[libvirt] [PATCH 0/6] lxc: Clean up lxcNetworkWalkCallback() method.

This series has a lots of cleanups to prepare lxc to handle network settings for verion 3.0 or higher. The new version is considering network indexes to define network interfaces. i.e.: "lxc.net.X.". As we need modular structures to support both versions, this method needs a cleanup before accepting new features for version 3. Obs: this version does not include any new feature for LXC 3.0 network. Julio Faracco (6): lxc: Create a separate method to handle IPv{4,6} outside parser. lxc: Create a new method called lxcNetworkParseDataEntry(). lxc: Converting full string entries in types only. lxc: Refactoring lxcNetworkWalkCallback() to a simple method. lxc: Create a method to initialize network data outisde. lxc: Converting 'if,else' logic into a 'switch,case'. src/lxc/lxc_native.c | 184 +++++++++++++++++++++++++++++-------------- src/lxc/lxc_native.h | 17 ++++ 2 files changed, 141 insertions(+), 60 deletions(-) -- 2.19.1

The new method called lxcNetworkParseDataIPs() is responsible to handle IPv{4,6} settings now. The idea is let lxcNetworkWalkCallback() method handle all entries related to network definition only. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 65 +++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 1eee3fc2bb..5bbbbf132c 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -552,6 +552,42 @@ lxcAddNetworkDefinition(lxcNetworkParseData *data) return -1; } +static int +lxcNetworkParseDataIPs(const char *name, virConfValuePtr value, lxcNetworkParseData *parseData) +{ + int family = AF_INET; + char **ipparts = NULL; + virNetDevIPAddrPtr ip = NULL; + + if (VIR_ALLOC(ip) < 0) + return -1; + + if (STREQ(name, "lxc.network.ipv6")) + family = AF_INET6; + + ipparts = virStringSplit(value->str, "/", 2); + if (virStringListLength((const char * const *)ipparts) != 2 || + virSocketAddrParse(&ip->address, ipparts[0], family) < 0 || + virStrToLong_ui(ipparts[1], NULL, 10, &ip->prefix) < 0) { + + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid CIDR address: '%s'"), value->str); + + virStringListFree(ipparts); + VIR_FREE(ip); + return -1; + } + + virStringListFree(ipparts); + + if (VIR_APPEND_ELEMENT(parseData->ips, parseData->nips, ip) < 0) { + VIR_FREE(ip); + return -1; + } + + return 0; +} + static int lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) { @@ -597,35 +633,8 @@ lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) parseData->name = value->str; else if (STREQ(name, "lxc.network.ipv4") || STREQ(name, "lxc.network.ipv6")) { - int family = AF_INET; - char **ipparts = NULL; - virNetDevIPAddrPtr ip = NULL; - - if (VIR_ALLOC(ip) < 0) + if (lxcNetworkParseDataIPs(name, value, parseData) < 0) return -1; - - if (STREQ(name, "lxc.network.ipv6")) - family = AF_INET6; - - ipparts = virStringSplit(value->str, "/", 2); - if (virStringListLength((const char * const *)ipparts) != 2 || - virSocketAddrParse(&ip->address, ipparts[0], family) < 0 || - virStrToLong_ui(ipparts[1], NULL, 10, &ip->prefix) < 0) { - - virReportError(VIR_ERR_INVALID_ARG, - _("Invalid CIDR address: '%s'"), value->str); - - virStringListFree(ipparts); - VIR_FREE(ip); - return -1; - } - - virStringListFree(ipparts); - - if (VIR_APPEND_ELEMENT(parseData->ips, parseData->nips, ip) < 0) { - VIR_FREE(ip); - return -1; - } } else if (STREQ(name, "lxc.network.ipv4.gateway")) { parseData->gateway_ipv4 = value->str; } else if (STREQ(name, "lxc.network.ipv6.gateway")) { -- 2.19.1

FWIW: No need for stop (e.g. '.') at end of each comment message... On 2/18/19 2:09 PM, Julio Faracco wrote:
The new method called lxcNetworkParseDataIPs() is responsible to handle IPv{4,6} settings now. The idea is let lxcNetworkWalkCallback() method handle all entries related to network definition only.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 65 +++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 1eee3fc2bb..5bbbbf132c 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -552,6 +552,42 @@ lxcAddNetworkDefinition(lxcNetworkParseData *data) return -1; }
+static int +lxcNetworkParseDataIPs(const char *name, virConfValuePtr value, lxcNetworkParseData *parseData)
When adding new functions we like to see 2 blank lines before/after each method and each argument for the method on it's own line. I've fixed all of this for you... Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]

This new method is responsible to verify is the settings correspond to network entry. Right now, it is only verifying "lxc.network.", but in the future, it can be used to verify "lxc.net.X." too. Any other case would be rejected. On the other hand, the idea here is working only with types. If we know that entry is part of network settings, after we just need to know which type is. It keeps the hanlder simple. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 5bbbbf132c..c144f3c52e 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -648,6 +648,14 @@ lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) return 0; } +static int +lxcNetworkParseDataEntry(const char *name, virConfValuePtr value, lxcNetworkParseData *parseData) +{ + const char *suffix = STRSKIP(name, "lxc.network."); + + return lxcNetworkParseDataSuffix(suffix, value, parseData); +} + static int lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr properties) { -- 2.19.1

$SUBJ: lxc: Introduce lxcNetworkParseDataEntry On 2/18/19 2:09 PM, Julio Faracco wrote:
This new method is responsible to verify is the settings correspond to network entry. Right now, it is only verifying "lxc.network.", but in the future, it can be used to verify "lxc.net.X." too. Any other case would be rejected.
On the other hand, the idea here is working only with types. If we know that entry is part of network settings, after we just need to know which type is. It keeps the hanlder simple.
*handler
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 5bbbbf132c..c144f3c52e 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -648,6 +648,14 @@ lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) return 0; }
+static int +lxcNetworkParseDataEntry(const char *name, virConfValuePtr value, lxcNetworkParseData *parseData)
Same thoughts as before w/r/t method headers. However, because we require each patch to be able to compile separately, things won't work in this order as lxcNetworkParseDataEntry is a static method with no callers. What needs to be done is this move this above lxcNetworkWalkCallback and then merge in patch 4 to here and alter patch3 slightly. That means patch4 gets dropped. I can do this and alter the commit messages appropriately. Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]

This commit removes the full network entry setting: "lxc.network.X" to type only. Like "type", "name", "flags", etc. So, here no matter if the settings is "lxc.network.X" or "lxc.net.X.Y". Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index c144f3c52e..ed50415a93 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -562,7 +562,7 @@ lxcNetworkParseDataIPs(const char *name, virConfValuePtr value, lxcNetworkParseD if (VIR_ALLOC(ip) < 0) return -1; - if (STREQ(name, "lxc.network.ipv6")) + if (STREQ(name, "ipv6")) family = AF_INET6; ipparts = virStringSplit(value->str, "/", 2); @@ -589,12 +589,11 @@ lxcNetworkParseDataIPs(const char *name, virConfValuePtr value, lxcNetworkParseD } static int -lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) +lxcNetworkParseDataSuffix(const char *name, virConfValuePtr value, lxcNetworkParseData *parseData) { - lxcNetworkParseData *parseData = data; int status; - if (STREQ(name, "lxc.network.type")) { + if (STREQ(name, "type")) { virDomainDefPtr def = parseData->def; size_t networks = parseData->networks; bool privnet = parseData->privnet; @@ -619,30 +618,31 @@ lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) /* Keep the new value */ parseData->type = value->str; } - else if (STREQ(name, "lxc.network.link")) + else if (STREQ(name, "link")) parseData->link = value->str; - else if (STREQ(name, "lxc.network.hwaddr")) + else if (STREQ(name, "hwaddr")) parseData->mac = value->str; - else if (STREQ(name, "lxc.network.flags")) + else if (STREQ(name, "flags")) parseData->flag = value->str; - else if (STREQ(name, "lxc.network.macvlan.mode")) + else if (STREQ(name, "macvlan.mode")) parseData->macvlanmode = value->str; - else if (STREQ(name, "lxc.network.vlan.id")) + else if (STREQ(name, "vlan.id")) parseData->vlanid = value->str; - else if (STREQ(name, "lxc.network.name")) + else if (STREQ(name, "name")) parseData->name = value->str; - else if (STREQ(name, "lxc.network.ipv4") || - STREQ(name, "lxc.network.ipv6")) { + else if (STREQ(name, "ipv4") || + STREQ(name, "ipv6")) { if (lxcNetworkParseDataIPs(name, value, parseData) < 0) return -1; - } else if (STREQ(name, "lxc.network.ipv4.gateway")) { + } else if (STREQ(name, "ipv4.gateway")) { parseData->gateway_ipv4 = value->str; - } else if (STREQ(name, "lxc.network.ipv6.gateway")) { + } else if (STREQ(name, "ipv6.gateway")) { parseData->gateway_ipv6 = value->str; - } else if (STRPREFIX(name, "lxc.network")) { + } else { VIR_WARN("Unhandled network property: %s = %s", name, value->str); + return -1; } return 0; -- 2.19.1

$SUBJ: lxc: Introduce lxcNetworkParseDataSuffix On 2/18/19 2:09 PM, Julio Faracco wrote:
This commit removes the full network entry setting: "lxc.network.X" to type only. Like "type", "name", "flags", etc. So, here no matter if the settings is "lxc.network.X" or "lxc.net.X.Y".
Replace last sentence w/ This will handle entries regardless of whether they are prefixed by "lxc.network." (today) or "lxc.net.X" (the future).
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]

Hi John, I'm seeing clang warnings after applying patches. Clang is complaining about this line:
- if (STREQ(name, "lxc.network.type")) { + if (STREQ(name, "type")) { virDomainDefPtr def = parseData->def;
I think it is irrelevant because there is no way to get a NULL pointer from case structure. During my tests, I'm being redirected to "Unhandled network property". Technically, only lxcNetworkParseDataSuffix is using this method. Btw, here is a possible fix for that... --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -612,7 +612,7 @@ lxcNetworkParseDataIPs(const char *name, if (VIR_ALLOC(ip) < 0) return -1; - if (STREQ(name, "ipv6")) + if (STREQ_NULLABLE(name, "ipv6")) family = AF_INET6; ipparts = virStringSplit(value->str, "/", 2); -- Julio Cesar Faracco

On 2/26/19 9:08 AM, Julio Faracco wrote:
Hi John,
I'm seeing clang warnings after applying patches. Clang is complaining about this line:
- if (STREQ(name, "lxc.network.type")) { + if (STREQ(name, "type")) { virDomainDefPtr def = parseData->def;
I think it is irrelevant because there is no way to get a NULL pointer from case structure. During my tests, I'm being redirected to "Unhandled network property". Technically, only lxcNetworkParseDataSuffix is using this method. Btw, here is a possible fix for that...
Hmm.. I don't typically compile with clang, although I know there are a few on the team that do and they haven't posted anything yet. I did see on the #virt channel that travis-ci's bot complained: https://travis-ci.org/libvirt/libvirt/builds/498459437 However, in looking at the output, it wasn't a compilation failure, rather it was a failure to 'mv mr.gmo-t mr.gmo' it seems which would be unrelated. I also just installed clang 7.0.1, ran CC=clang sh autogen.sh, and then did a build, but didn't see any failure. If anything - I would think a well placed sa_assert(suffix) in lxcNetworkParseDataEntry would assert to the compiler that suffix isn't NULL. Of course the fact that lxcNetworkParseDataIPs got called means that virLXCNetworkConfigEntryTypeFromString had a valid string. So, perhaps it's an older clang compiler that's issuing the message. John
--- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -612,7 +612,7 @@ lxcNetworkParseDataIPs(const char *name, if (VIR_ALLOC(ip) < 0) return -1;
- if (STREQ(name, "ipv6")) + if (STREQ_NULLABLE(name, "ipv6")) family = AF_INET6;
ipparts = virStringSplit(value->str, "/", 2);
-- Julio Cesar Faracco

The method lxcNetworkWalkCallback() needs to be simple enough to handle both possible network settings with indexes or the simple one. It is better the decouple the whole algorithm to parse data to only parse which entry type libvirt is handling. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index ed50415a93..95e08c18f4 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -656,6 +656,19 @@ lxcNetworkParseDataEntry(const char *name, virConfValuePtr value, lxcNetworkPars return lxcNetworkParseDataSuffix(suffix, value, parseData); } +static int +lxcNetworkWalkCallback(const char *name, + virConfValuePtr value, + void *data) +{ + lxcNetworkParseData *parseData = data; + + if (STRPREFIX(name, "lxc.network.")) + return lxcNetworkParseDataEntry(name, value, parseData); + + return 0; +} + static int lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr properties) { -- 2.19.1

On 2/18/19 2:09 PM, Julio Faracco wrote:
The method lxcNetworkWalkCallback() needs to be simple enough to handle both possible network settings with indexes or the simple one. It is better the decouple the whole algorithm to parse data to only parse which entry type libvirt is handling.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
This merges w/ patch2 as discussed there. John [...]

This method has the same idea of the method to parse IPv{4,6} data. The method lxcNetworkParseDataInit() is responsible to initialize network settings outside handler. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 56 +++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 95e08c18f4..25e35e93dd 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -552,6 +552,37 @@ lxcAddNetworkDefinition(lxcNetworkParseData *data) return -1; } +static int +lxcNetworkParseDataInit(virConfValuePtr value, lxcNetworkParseData *parseData) +{ + virDomainDefPtr def = parseData->def; + size_t networks = parseData->networks; + bool privnet = parseData->privnet; + int status; + + /* Store the previous NIC */ + status = lxcAddNetworkDefinition(parseData); + + if (status < 0) + return -1; + else if (status > 0) + networks++; + else if (parseData->type != NULL && STREQ(parseData->type, "none")) + privnet = false; + + /* clean NIC to store a new one */ + memset(parseData, 0, sizeof(*parseData)); + + parseData->def = def; + parseData->networks = networks; + parseData->privnet = privnet; + + /* Keep the new value */ + parseData->type = value->str; + + return 0; +} + static int lxcNetworkParseDataIPs(const char *name, virConfValuePtr value, lxcNetworkParseData *parseData) { @@ -591,32 +622,9 @@ lxcNetworkParseDataIPs(const char *name, virConfValuePtr value, lxcNetworkParseD static int lxcNetworkParseDataSuffix(const char *name, virConfValuePtr value, lxcNetworkParseData *parseData) { - int status; - if (STREQ(name, "type")) { - virDomainDefPtr def = parseData->def; - size_t networks = parseData->networks; - bool privnet = parseData->privnet; - - /* Store the previous NIC */ - status = lxcAddNetworkDefinition(parseData); - - if (status < 0) + if (lxcNetworkParseDataInit(value, parseData) < 0) return -1; - else if (status > 0) - networks++; - else if (parseData->type != NULL && STREQ(parseData->type, "none")) - privnet = false; - - /* clean NIC to store a new one */ - memset(parseData, 0, sizeof(*parseData)); - - parseData->def = def; - parseData->networks = networks; - parseData->privnet = privnet; - - /* Keep the new value */ - parseData->type = value->str; } else if (STREQ(name, "link")) parseData->link = value->str; -- 2.19.1

$SUBJ: lxc: Introduce lxcNetworkParseDataType [see below for name change] On 2/18/19 2:09 PM, Julio Faracco wrote:
This method has the same idea of the method to parse IPv{4,6} data. The method lxcNetworkParseDataInit() is responsible to initialize network settings outside handler.
Extract out the network "type" processing into it's own method rather than inline within lxcNetworkParseDataSuffix.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 56 +++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 24 deletions(-)
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 95e08c18f4..25e35e93dd 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -552,6 +552,37 @@ lxcAddNetworkDefinition(lxcNetworkParseData *data) return -1; }
+static int +lxcNetworkParseDataInit(virConfValuePtr value, lxcNetworkParseData *parseData)
Similar w/r/t blank lines and 2 lines per argument I think this is better named lxcNetworkParseDataType Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]

The structure used to handle network entries was based on 'if,else' conditions. This commit converts this ugly structure into a switch to clearify each option of the handler. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 58 ++++++++++++++++++++++++++++++++------------ src/lxc/lxc_native.h | 17 +++++++++++++ 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 25e35e93dd..c746c443da 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -35,6 +35,20 @@ VIR_LOG_INIT("lxc.lxc_native"); +VIR_ENUM_IMPL(virLXCNetworkConfigEntry, VIR_LXC_NETWORK_CONFIG_LAST, + "name", + "type", + "link", + "hwaddr", + "flags", + "macvlan.mode", + "vlan.id", + "ipv4", + "ipv4.gateway", + "ipv6", + "ipv6.gateway" +); + static virDomainFSDefPtr lxcCreateFSDef(int type, const char *src, @@ -620,35 +634,47 @@ lxcNetworkParseDataIPs(const char *name, virConfValuePtr value, lxcNetworkParseD } static int -lxcNetworkParseDataSuffix(const char *name, virConfValuePtr value, lxcNetworkParseData *parseData) +lxcNetworkParseDataSuffix(const char *entry, virConfValuePtr value, lxcNetworkParseData *parseData) { - if (STREQ(name, "type")) { + int type = virLXCNetworkConfigEntryTypeFromString(entry); + + switch (type) { + case VIR_LXC_NETWORK_CONFIG_TYPE: if (lxcNetworkParseDataInit(value, parseData) < 0) return -1; - } - else if (STREQ(name, "link")) + break; + case VIR_LXC_NETWORK_CONFIG_LINK: parseData->link = value->str; - else if (STREQ(name, "hwaddr")) + break; + case VIR_LXC_NETWORK_CONFIG_HWADDR: parseData->mac = value->str; - else if (STREQ(name, "flags")) + break; + case VIR_LXC_NETWORK_CONFIG_FLAGS: parseData->flag = value->str; - else if (STREQ(name, "macvlan.mode")) + break; + case VIR_LXC_NETWORK_CONFIG_MACVLAN_MODE: parseData->macvlanmode = value->str; - else if (STREQ(name, "vlan.id")) + break; + case VIR_LXC_NETWORK_CONFIG_VLAN_ID: parseData->vlanid = value->str; - else if (STREQ(name, "name")) + break; + case VIR_LXC_NETWORK_CONFIG_NAME: parseData->name = value->str; - else if (STREQ(name, "ipv4") || - STREQ(name, "ipv6")) { - if (lxcNetworkParseDataIPs(name, value, parseData) < 0) + break; + case VIR_LXC_NETWORK_CONFIG_IPV4: + case VIR_LXC_NETWORK_CONFIG_IPV6: + if (lxcNetworkParseDataIPs(entry, value, parseData) < 0) return -1; - } else if (STREQ(name, "ipv4.gateway")) { + break; + case VIR_LXC_NETWORK_CONFIG_IPV4_GATEWAY: parseData->gateway_ipv4 = value->str; - } else if (STREQ(name, "ipv6.gateway")) { + break; + case VIR_LXC_NETWORK_CONFIG_IPV6_GATEWAY: parseData->gateway_ipv6 = value->str; - } else { + break; + default: VIR_WARN("Unhandled network property: %s = %s", - name, + entry, value->str); return -1; } diff --git a/src/lxc/lxc_native.h b/src/lxc/lxc_native.h index 86f5163e12..0939be346d 100644 --- a/src/lxc/lxc_native.h +++ b/src/lxc/lxc_native.h @@ -25,6 +25,23 @@ # define LXC_CONFIG_FORMAT "lxc-tools" +typedef enum { + VIR_LXC_NETWORK_CONFIG_NAME, + VIR_LXC_NETWORK_CONFIG_TYPE, + VIR_LXC_NETWORK_CONFIG_LINK, + VIR_LXC_NETWORK_CONFIG_HWADDR, + VIR_LXC_NETWORK_CONFIG_FLAGS, + VIR_LXC_NETWORK_CONFIG_MACVLAN_MODE, + VIR_LXC_NETWORK_CONFIG_VLAN_ID, + VIR_LXC_NETWORK_CONFIG_IPV4, + VIR_LXC_NETWORK_CONFIG_IPV4_GATEWAY, + VIR_LXC_NETWORK_CONFIG_IPV6, + VIR_LXC_NETWORK_CONFIG_IPV6_GATEWAY, + VIR_LXC_NETWORK_CONFIG_LAST, +} virLXCNetworkConfigEntry; + +VIR_ENUM_DECL(virLXCNetworkConfigEntry); + virDomainDefPtr lxcParseConfigString(const char *config, virCapsPtr caps, virDomainXMLOptionPtr xmlopt); -- 2.19.1

On 2/18/19 2:09 PM, Julio Faracco wrote:
The structure used to handle network entries was based on 'if,else' conditions. This commit converts this ugly structure into a switch to clearify each option of the handler.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 58 ++++++++++++++++++++++++++++++++------------ src/lxc/lxc_native.h | 17 +++++++++++++ 2 files changed, 59 insertions(+), 16 deletions(-)
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 25e35e93dd..c746c443da 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -35,6 +35,20 @@
VIR_LOG_INIT("lxc.lxc_native");
+VIR_ENUM_IMPL(virLXCNetworkConfigEntry, VIR_LXC_NETWORK_CONFIG_LAST, + "name", + "type", + "link", + "hwaddr", + "flags", + "macvlan.mode", + "vlan.id", + "ipv4", + "ipv4.gateway", + "ipv6", + "ipv6.gateway" +); + static virDomainFSDefPtr lxcCreateFSDef(int type, const char *src, @@ -620,35 +634,47 @@ lxcNetworkParseDataIPs(const char *name, virConfValuePtr value, lxcNetworkParseD }
static int -lxcNetworkParseDataSuffix(const char *name, virConfValuePtr value, lxcNetworkParseData *parseData) +lxcNetworkParseDataSuffix(const char *entry, virConfValuePtr value, lxcNetworkParseData *parseData)
same w/r/t empty lines and argument on each line.
{ - if (STREQ(name, "type")) { + int type = virLXCNetworkConfigEntryTypeFromString(entry);
s/int type/int elem/ @entry was another option, but @elem I think is better than @type since "type" is a field. Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]
participants (2)
-
John Ferlan
-
Julio Faracco