Re: [libvirt] [PATCH] qemuProcessHandleMonitorEOF: Disable namespace for domain

On Fri, Mar 10, 2017 at 01:41:23PM +0100, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1430634 > > If a qemu process has died, we get EOF on its monitor. At this > point, since qemu process was the only one running in the > namespace kernel has already cleaned the namespace up. Any > attempt of ours to enter it has to fail. > > This really happened in the bug linked above. We've tried to > attach a disk to qemu and while we were in the monitor talking to > qemu it just died. Therefore our code tried to do some roll back > (e.g. deny the device in cgroups again, restore labels, etc.). > However, during the roll back (esp. when restoring labels) we > still thought that domain has a namespace. So we used secdriver's > transactions. This failed as there is no namespace to enter. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/qemu/qemu_domain.c | 25 +++++++++++++++++++++++++ > src/qemu/qemu_domain.h | 3 +++ > src/qemu/qemu_process.c | 4 ++++ > 3 files changed, 32 insertions(+) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 1a42fcf1b..d5833b026 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -201,6 +201,22 @@ qemuDomainEnableNamespace(virDomainObjPtr vm, > } > > > +static void > +qemuDomainDisableNamespace(virDomainObjPtr vm, > + qemuDomainNamespace ns) > +{ > + qemuDomainObjPrivatePtr priv = vm->privateData; > + > + if (priv->namespaces) { > + ignore_value(virBitmapClearBit(priv->namespaces, ns)); > + if (virBitmapIsAllClear(priv->namespaces)) { > + virBitmapFree(priv->namespaces); > + priv->namespaces = NULL; > + } > + } > +} > + This function is written in a way that... > @@ -7805,6 +7821,15 @@ qemuDomainCreateNamespace(virQEMUDriverPtr driver, > } > > > +void > +qemuDomainDestroyNamespace(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, > + virDomainObjPtr vm) > +{ > + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) > + qemuDomainDisableNamespace(vm, QEMU_DOMAIN_NS_MOUNT); > +} > + ...this wrapper over it is kinda useless. But your call on whether to keep it (I get there are some "consistency" and "naming" reasons).

ACK to this, although I feel like commit 3e6839d4e801 should be reverted, but that's up for a discussion. It does not make *any* difference now, but I just feel like it's cleaner that way.