-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add bond migration #57
Add bond migration #57
Conversation
rust/migrate-wicked/src/interface.rs
Outdated
if let Some(dummy) = &self.dummy { | ||
if let Some(address) = &dummy.address { | ||
connection.mac_address = MacAddress::from_str(address)?; | ||
} | ||
} else if let Some(ethernet) = &self.ethernet { | ||
if let Some(ethernet) = &self.ethernet { | ||
if let Some(address) = ðernet.address { | ||
connection.mac_address = MacAddress::from_str(address)?; | ||
} | ||
} | ||
|
||
if self.dummy.is_some() { | ||
if let Some(dummy) = &self.dummy { | ||
if let Some(address) = &dummy.address { | ||
connection.mac_address = MacAddress::from_str(address)?; | ||
} | ||
connection.config = model::ConnectionConfig::Dummy | ||
} else if let Some(bond) = &self.bond { | ||
if let Some(address) = &bond.address { | ||
connection.mac_address = MacAddress::from_str(address)?; | ||
} | ||
connection.config = bond.into() | ||
} else { | ||
connection.config = model::ConnectionConfig::Ethernet | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if let Some(dummy) = &self.dummy { | |
if let Some(address) = &dummy.address { | |
connection.mac_address = MacAddress::from_str(address)?; | |
} | |
} else if let Some(ethernet) = &self.ethernet { | |
if let Some(ethernet) = &self.ethernet { | |
if let Some(address) = ðernet.address { | |
connection.mac_address = MacAddress::from_str(address)?; | |
} | |
} | |
if self.dummy.is_some() { | |
if let Some(dummy) = &self.dummy { | |
if let Some(address) = &dummy.address { | |
connection.mac_address = MacAddress::from_str(address)?; | |
} | |
connection.config = model::ConnectionConfig::Dummy | |
} else if let Some(bond) = &self.bond { | |
if let Some(address) = &bond.address { | |
connection.mac_address = MacAddress::from_str(address)?; | |
} | |
connection.config = bond.into() | |
} else { | |
connection.config = model::ConnectionConfig::Ethernet | |
}; | |
if let Some(ethernet) = &self.ethernet { | |
if let Some(address) = ðernet.address { | |
connection.mac_address = MacAddress::from_str(address)?; | |
} | |
connection.config = model::ConnectionConfig::Ethernet | |
} else if let Some(dummy) = &self.dummy { | |
if let Some(address) = &dummy.address { | |
connection.mac_address = MacAddress::from_str(address)?; | |
} | |
connection.config = model::ConnectionConfig::Dummy | |
} else if let Some(bond) = &self.bond { | |
if let Some(address) = &bond.address { | |
connection.mac_address = MacAddress::from_str(address)?; | |
} | |
connection.config = bond.into() | |
}; |
and maybe some DRY before we repeat it for every connection type:
macro_rules! ADD_MAC {
($connection:expr, $config:expr) => {
if let Some(address) = &$config.address {
$connection.mac_address = MacAddress::from_str(address)?;
}
};
}
...
if let Some(ethernet) = &self.ethernet {
ADD_MAC!(connection, ethernet);
connection.config = model::ConnectionConfig::Ethernet
}
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT of
impl TryFrom<&Option<String>> for MacAddress {
type Error = InvalidMacAddress;
fn try_from(value: &Option<String>) -> Result<Self, Self::Error> {
match &value {
Some(str) => MacAddress::from_str(str),
None => Ok(Self::Unset)
}
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, should also work if you prefer it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I finally go with
#serde(default)
address: String,
which lead to empty string if it isn't in xml...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I finally go with
#serde(default) address: String,
which lead to empty string if it isn't in xml...
In that case you should also add
#[serde(skip_serializing_if = "String::is_empty")]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
valid point.
Lets see if this agama-project#983 gets accepted, then I go back to Option<String>
40addde
to
d5004b4
Compare
d5004b4
to
2f75de4
Compare
Problem
Upstream bonding was finally merged and now we can re-implement the migrate-wicked side.
Testing