Em qua., 29 de jan. de 2020 às 08:38, Daniel P. Berrangé <berrange@redhat.com> escreveu:
On Tue, Jan 28, 2020 at 10:54:08PM -0300, Julio Faracco wrote:
Struct lxcNetworkParseData is being used as a single pointer which iterates through LXC config lines. It means that it will be applied as a network each time that a new type appears. After, the same struct is used to populate a new network interface. This commit changes this logic to multiple lxcNetworkParseData to move this strcuture to an array. It makes more sense if we are using indexes to fill interface settings. This is better to improve code clarity.
This commit still introduces *Legacy() functions to keep support of network old style definitions.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_native.c | 129 +++++++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 61 deletions(-)
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index dd2345c324..b101848c09 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c
@@ -702,35 +703,41 @@ static int lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr properties) { int status; - size_t i; - lxcNetworkParseData data = {def, NULL, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, 0, - NULL, NULL, true, 0}; + bool privnet = true; + size_t i, j; + lxcNetworkParseDataArray networks = {0, NULL}; + + networks.parseData = g_new0(lxcNetworkParseDataPtr, 1);
- if (virConfWalk(properties, lxcNetworkWalkCallback, &data) < 0) + if (virConfWalk(properties, lxcNetworkWalkCallback, &networks) < 0) goto error;
+ for (i = 0; i < networks.ndata; i++) { + lxcNetworkParseDataPtr data = networks.parseData[i]; + data->def = def;
- /* Add the last network definition found */ - status = lxcAddNetworkDefinition(&data); + status = lxcAddNetworkDefinition(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 (networks.ndata == 0 && privnet) { /* When no network type is provided LXC only adds loopback */ def->features[VIR_DOMAIN_FEATURE_PRIVNET] = VIR_TRISTATE_SWITCH_ON; } return 0;
error: - for (i = 0; i < data.nips; i++) - VIR_FREE(data.ips[i]); - VIR_FREE(data.ips); + for (i = 0; i < networks.ndata; i++) { + lxcNetworkParseDataPtr data = networks.parseData[i]; + for (j = 0; j < data->nips; j++) + VIR_FREE(data->ips[j]); + VIR_FREE(data->ips); + } + VIR_FREE(networks.parseData); return -1; }
Unfortunately I noticed some memory leaks in this method reported by valgrind
Check it with this:
./run valgrind --leak-check=full --show-possibly-lost=no \ ./tests/lxcconf2xmltest
Essentially we're not free'ing c in the success code path, only the failure code path. The failure codepath also fails to free the 'lxcNetworkParseDataPtr' instance.
I think the main problem here with success code path is loosing IPs when we free networks.parseData. The failure code path is not relevant. We can free everything.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|