Re: [PATCHv2 3/7] Introduce qemuDomainNamespaceMknodPaths

[Ján Tomko <jtomko@xxxxxxxxxx>]

On Thu, Dec 07, 2017 at 07:51:40AM -0500, John Ferlan wrote:
> On 12/01/2017 11:50 AM, Ján Tomko wrote:
> > Separate the logic of creating devices from their gathering.
> >
> > Use this new function in qemuDomainNamespaceSetupHostdev and
> > qemuDomainNamespaceSetupDisk.
> > ---
> >  src/qemu/qemu_domain.c | 74 ++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 44 insertions(+), 30 deletions(-)
> >
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index d7f4b3fd7..f1c664248 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -9925,16 +9925,18 @@ qemuDomainDetachDeviceUnlink(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> >  }
>
> [...]
>
> > +
> > +
> > +int
> > +qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> > +                             virDomainObjPtr vm,
> > +                             virStorageSourcePtr src)
> > +{
> > +    virStorageSourcePtr next;
> > +    char **paths = NULL;
> > +    size_t npaths;
> > +    int ret = -1;
> > +
> > +    if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
> > +        return 0;
> > +
> >      for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) {
> >          if (virStorageSourceIsEmpty(next) ||
> >              !virStorageSourceIsLocalStorage(next)) {
> > @@ -9952,17 +9983,16 @@ qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver,
> >              continue;
> >          }
> >
> > -        if (qemuDomainAttachDeviceMknod(driver,
> > -                                        vm,
> > -                                        next->path,
> > -                                        devMountsPath, ndevMountsPath) < 0)
> > +        if (VIR_APPEND_ELEMENT_COPY(paths, npaths, next->path) < 0)
>
> Coverity has noted to me this morning that @npaths is uninitialized...

Thanks, fixed.

> >              goto cleanup;
> >      }
> >
> > +    if (qemuDomainNamespaceMknodPaths(vm, (const char **)paths, npaths) < 0)
> > +        return -1;
> > +
> >      ret = 0;
> >   cleanup:
> > -    virStringListFreeCount(devMountsPath, ndevMountsPath);
> > -    virObjectUnref(cfg);
> > +    VIR_FREE(paths);
>
> And while not noted by Coverity, shouldn't this be a :
>
> virStringListFree(paths);

No, the paths are owned by the respective StorageSources.

Jan

> John

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list