Re: [libvirt] [PATCH] qemuDomainNamespaceTeardownHostdev: Drop useless check

On 09/07/2018 12:52 AM, John Ferlan wrote: > > > On 09/06/2018 12:16 PM, Michal Privoznik wrote: >> There is no need to check if @npaths is not zero. Let's >> qemuDomainNamespaceUnlinkPaths() handle that. >> >> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >> --- >> src/qemu/qemu_domain.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> > > At the cost of a possible unnecessary, but perhaps expensive call to > qemuDomainGetPreservedMounts when npaths == 0?

Sure. But at least with my patch we are consistent. If it really bothers us, we can have a check at the beginning of qemuDomainNamespaceMknodPaths() and qemuDomainNamespaceUnlinkPaths(), right after namespace check to return early if npaths is zero. > > I think if you add a "filter" of npaths == 0, then return 0 in > qemuDomainNamespaceUnlinkPaths, then that'd be a good thing... > Okay. > > Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; > > John > > I also wonder if the : > > if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) > return 0; > > that's "duplicated" in qemuDomainNamespaceTeardownHostdev and > qemuDomainNamespaceUnlinkPaths could be "reworked"... > Oh sure it could. We have two sets of functions apparently: one does the check themselves and return early (e.g. qemuDomainNamespaceSetupDisk()) and the other leave it to qemuDomainNamespaceMknodPaths() to return return early (e.g. qemuDomainNamespaceSetupMemory()).

Michal -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list