Re: [libvirt] [PATCH 2/3] lxc: Turn @veths into a string list in virLXCProcessStart

On 07/27/2018 10:37 AM, Erik Skultety wrote: > On Thu, Jul 26, 2018 at 05:36:28PM +0200, Michal Privoznik wrote: >> This way it will be easier to use autofree. >> >> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >> --- >> src/lxc/lxc_process.c | 24 +++++++++--------------- >> 1 file changed, 9 insertions(+), 15 deletions(-) >> >> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c >> index d021a890f7..3ac39d598c 100644 >> --- a/src/lxc/lxc_process.c >> +++ b/src/lxc/lxc_process.c >> @@ -514,8 +514,7 @@ static int virLXCProcessSetupNamespaces(virConnectPtr conn, >> * virLXCProcessSetupInterfaces: >> * @conn: pointer to connection >> * @def: pointer to virtual machine structure >> - * @nveths: number of interfaces >> - * @veths: interface names >> + * @veths: string list of interface names >> * >> * Sets up the container interfaces by creating the veth device pairs and >> * attaching the parent end to the appropriate bridge. The container end >> @@ -525,7 +524,6 @@ static int virLXCProcessSetupNamespaces(virConnectPtr conn, >> */ >> static int virLXCProcessSetupInterfaces(virConnectPtr conn, >> virDomainDefPtr def, >> - size_t *nveths, >> char ***veths) >> { >> int ret = -1; >> @@ -534,6 +532,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, >> virDomainNetDefPtr net; >> virDomainNetType type; >> >> + *veths = NULL; >> + >> for (i = 0; i < def->nnets; i++) { >> char *veth = NULL; >> virNetDevBandwidthPtr actualBandwidth; >> @@ -549,9 +549,6 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, >> if (virDomainNetAllocateActualDevice(def, net) < 0) >> goto cleanup; >> >> - if (VIR_EXPAND_N(*veths, *nveths, 1) < 0) >> - goto cleanup; > > Contrary to what I said in the previous patch, I hadn't realized we were > expanding a list by 1 in a loop before I looked at this patch. That is very > inefficient and we'll keep doing that. Now I'm biased towards tracking the size > of the list so that we can do VIR_EXPAND_N(*veths, 0, def->nnets) and then just > add new elements, of course that would require tweaking the virStringListAdd > more. > Hold on. I'm not quite sure I follow. Are you suggesting to pre-allocate the string list and ditch virStringListAdd completely? Something like this?

diff --git i/src/lxc/lxc_process.c w/src/lxc/lxc_process.c index 6eef17d1ce..33c806630b 100644 --- i/src/lxc/lxc_process.c +++ w/src/lxc/lxc_process.c @@ -532,7 +532,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, virDomainNetDefPtr net; virDomainNetType type; - *veths = NULL; + if (VIR_ALLOC_N(*veths, def->nnets + 1) < 0) + return -1; for (i = 0; i < def->nnets; i++) { char *veth = NULL; @@ -601,8 +602,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, } } - if (virStringListAdd(veths, veth) < 0) - goto cleanup; + (*veths)[i] = veth; if (VIR_STRDUP(def->nets[i]->ifname_guest_actual, veth) < 0) goto cleanup; Michal -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list