On 7/7/21 12:46 PM, Kristina Hanicova wrote: > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1780508 > > Signed-off-by: Kristina Hanicova <khanicov@xxxxxxxxxx> > --- I know writing commit messages is hard, but I think you can describe the problem and the proposed solution briefly. > src/qemu/qemu_hotplug.c | 3 +-- > src/qemu/qemu_namespace.c | 10 ++++++++-- > src/qemu/qemu_namespace.h | 3 ++- > 3 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index d2a354d026..7002623924 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -3225,9 +3225,8 @@ qemuDomainAttachInputDevice(virQEMUDriver *driver, > if (qemuBuildInputDevStr(&devstr, vm->def, input, priv->qemuCaps) < 0) > goto cleanup; > > - if (qemuDomainNamespaceSetupInput(vm, input) < 0) > + if (qemuDomainNamespaceSetupInput(vm, input, &teardowndevice) < 0) > goto cleanup; > - teardowndevice = true; > > if (qemuSetupInputCgroup(vm, input) < 0) > goto cleanup; > diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c > index 154968acbd..22bdda229a 100644 > --- a/src/qemu/qemu_namespace.c > +++ b/src/qemu/qemu_namespace.c > @@ -1600,9 +1600,11 @@ qemuDomainNamespaceTeardownRNG(virDomainObj *vm, > > int > qemuDomainNamespaceSetupInput(virDomainObj *vm, > - virDomainInputDef *input) > + virDomainInputDef *input, > + bool *created) > { > g_autoptr(virGSListString) paths = NULL; > + int ret = 0; > > if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) > return 0; > @@ -1610,8 +1612,12 @@ qemuDomainNamespaceSetupInput(virDomainObj *vm, > if (qemuDomainSetupInput(input, &paths) < 0) > return -1; > > - if (qemuNamespaceMknodPaths(vm, paths) < 0) > + if ((ret = qemuNamespaceMknodPaths(vm, paths)) < 0) > return -1; > + > + if (ret == 0) > + *created = true; > + > return 0; > } > > diff --git a/src/qemu/qemu_namespace.h b/src/qemu/qemu_namespace.h > index 771d7873ef..5d9af123a9 100644 > --- a/src/qemu/qemu_namespace.h > +++ b/src/qemu/qemu_namespace.h > @@ -80,7 +80,8 @@ int qemuDomainNamespaceTeardownRNG(virDomainObj *vm, > virDomainRNGDef *rng); > > int qemuDomainNamespaceSetupInput(virDomainObj *vm, > - virDomainInputDef *input); > + virDomainInputDef *input, > + bool *created); > > int qemuDomainNamespaceTeardownInput(virDomainObj *vm, > virDomainInputDef *input); > So this fixes just <input/>. I know that we talked about other types and that QEMU can open them multiple times, but I still thought that you were going to make the rest of qemuDomainNamespaceSetup*() follow this Input(). I mean, even though we did not see it in our testing, but the hotplug code can still experience an error (for various reasons) in which case we shouldn't try to remove the device from the namespace. For instance: when hotplugging an USB device qemuDomainAttachHostUSBDevice() is called and if say qemuSetupHostdevCgroup() failed then the device would be removed from namespace even though we might not have created it. Having said that, I wonder if we can save a few lines of code if this @created argument was set in qemuNamespaceMknodPaths() instead of having the same pattern (if ret==0 then *created=true;) copied into all qemuDomainNamespaceSetup*() functions. But I don't mind. Michal
