On 3/4/19 8:54 PM, Julio Faracco wrote:
This commit refactor the code logic to introduce new array structures instead of single lxcNetworkParseData struct.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 94 +++++++++++++++----------------------------- 1 file changed, 32 insertions(+), 62 deletions(-)
lxc/lxc_native.c:679:27: error: unused variable 'networks' [-Werror,-Wunused-variable] lxcNetworkParseArray *networks = data; ^ 1 error generated. make[5]: *** [Makefile:10902: lxc/libvirt_driver_lxc_impl_la-lxc_native.lo] Error 1
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index bf82cd1e98..a6afbbe865 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -484,7 +484,7 @@ lxcAddNetworkRouteDefinition(const char *address, }
static int -lxcAddNetworkDefinition(lxcNetworkParseData *data) +lxcAddNetworkDefinition(virDomainDefPtr def, lxcNetworkParseDataPtr data)
One argument per line... Still I think too much going on at a time... Removal of the @def from lxcNetworkParseData should be separated.
{ virDomainNetDefPtr net = NULL; virDomainHostdevDefPtr hostdev = NULL; @@ -532,9 +532,9 @@ lxcAddNetworkDefinition(lxcNetworkParseData *data) &hostdev->source.caps.u.net.ip.nroutes) < 0) goto error;
- if (VIR_EXPAND_N(data->def->hostdevs, data->def->nhostdevs, 1) < 0) + if (VIR_EXPAND_N(def->hostdevs, def->nhostdevs, 1) < 0) goto error; - data->def->hostdevs[data->def->nhostdevs - 1] = hostdev; + def->hostdevs[def->nhostdevs - 1] = hostdev; } else { if (!(net = lxcCreateNetDef(data->type, data->link, data->mac, data->flag, data->macvlanmode, @@ -556,9 +556,9 @@ lxcAddNetworkDefinition(lxcNetworkParseData *data) &net->guestIP.nroutes) < 0) goto error;
- if (VIR_EXPAND_N(data->def->nets, data->def->nnets, 1) < 0) + if (VIR_EXPAND_N(def->nets, def->nnets, 1) < 0) goto error; - data->def->nets[data->def->nnets - 1] = net; + def->nets[def->nnets - 1] = net; }
return 1; @@ -572,44 +572,10 @@ lxcAddNetworkDefinition(lxcNetworkParseData *data) return -1; }
- -static int -lxcNetworkParseDataType(virConfValuePtr value, - lxcNetworkParseData *parseData) -{
Another example of too much happening at one time... Strange to see this "go away"
- 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) + lxcNetworkParseDataPtr parseData)
This hunk would be in previous patch
{ int family = AF_INET; char **ipparts = NULL; @@ -648,14 +614,13 @@ lxcNetworkParseDataIPs(const char *name, static int lxcNetworkParseDataSuffix(const char *entry, virConfValuePtr value, - lxcNetworkParseData *parseData) + lxcNetworkParseDataPtr parseData)
Same - previous patch
{ int elem = virLXCNetworkConfigEntryTypeFromString(entry);
switch (elem) { case VIR_LXC_NETWORK_CONFIG_TYPE: - if (lxcNetworkParseDataType(value, parseData) < 0) - return -1; + parseData->type = value->str;
It's confusing why lxcNetworkParseDataType is no longer necessary
break; case VIR_LXC_NETWORK_CONFIG_LINK: parseData->link = value->str; @@ -700,7 +665,7 @@ lxcNetworkParseDataSuffix(const char *entry, static int lxcNetworkParseDataEntry(const char *name, virConfValuePtr value, - lxcNetworkParseData *parseData) + lxcNetworkParseDataPtr parseData)
Again, previous patch
{ const char *suffix = STRSKIP(name, "lxc.network.");
@@ -711,7 +676,8 @@ lxcNetworkParseDataEntry(const char *name, static int lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) { - lxcNetworkParseData *parseData = data; + lxcNetworkParseArray *networks = data;
As the compiler tells you @networks is not used
+ lxcNetworkParseDataPtr parseData = NULL;
If you took this one patch at a time, then @data "used" to be a pointer to a structure of mostly empty data with a filled in @def and this changes it to a NULL structure, which doesn't feel quite right. That means lxcNetworkParseDataEntry gets called w/ NULL parameter in 3rd parameter. Going to stop here and wait for the next series... I think removing the @networks and @privnet from _lxcNetworkParseData should be one step and then the removal of @def another step. John
if (STRPREFIX(name, "lxc.network.")) return lxcNetworkParseDataEntry(name, value, parseData); @@ -724,26 +690,26 @@ lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr properties) { int status; int result = -1; - size_t i; - lxcNetworkParseData data = {def, NULL, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, 0, - NULL, NULL, true, 0}; + size_t i, j; + bool privnet = true; + lxcNetworkParseArray nets = {NULL, 0};
- if (virConfWalk(properties, lxcNetworkWalkCallback, &data) < 0) + if (virConfWalk(properties, lxcNetworkWalkCallback, &nets) < 0) goto error;
+ for (i = 0; i < nets.nnetworks; i++) { + lxcNetworkParseDataPtr data = nets.data[i];
- /* Add the last network definition found */ - status = lxcAddNetworkDefinition(&data); + /* Add network definitions */ + status = lxcAddNetworkDefinition(def, data);
- if (status < 0) - goto error; - else if (status > 0) - data.networks++; - else if (data.type != NULL && STREQ(data.type, "none")) - data.privnet = false; + if (status < 0) + goto error; + else if (data->type != NULL && STREQ(data->type, "none")) + privnet = false; + }
- if (data.networks == 0 && data.privnet) { + if (nets.nnetworks == 0 && privnet) { /* When no network type is provided LXC only adds loopback */ def->features[VIR_DOMAIN_FEATURE_PRIVNET] = VIR_TRISTATE_SWITCH_ON; } @@ -752,9 +718,13 @@ lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr properties) return result;
error: - for (i = 0; i < data.nips; i++) - VIR_FREE(data.ips[i]); - VIR_FREE(data.ips); + for (i = 0; i < nets.nnetworks; i++) { + for (j = 0; j < nets.data[i]->nips; j++) + VIR_FREE(nets.data[i]->ips[j]); + VIR_FREE(nets.data[i]->ips); + } + for (i = 0; i < nets.nnetworks; i++) + VIR_FREE(nets.data[i]); return -1; }