[libvirt] [PATCH v2 10/10] qemu_hotplug: Relabel memdev

[John Ferlan]

On 08/11/2016 09:26 AM, Michal Privoznik wrote:
> Now that we have APIs for relabel memdevs on hotplug, fill in the
> missing implementation in qemu hotplug code.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 

Note: Patches 6-9 have an implicit ACK - they seem to be fairly
standard.  Although what about apparmour?

> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 6ba0b8e..afabbda 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1861,6 +1861,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>      int id;
>      int ret = -1;
>      int rv;
> +    bool restoreLabel = false;
>  
>      qemuDomainMemoryDeviceAlignSize(vm->def, mem);
>  
> @@ -1893,6 +1894,11 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>          goto removedef;
>      }
>  
> +    if (virSecurityManagerSetMemoryLabel(driver->securityManager,
> +                                         vm->def, mem) < 0)
> +        goto cleanup;
> +    restoreLabel = true;
> +
>      qemuDomainObjEnterMonitor(driver, vm);
>      rv = qemuMonitorAddObject(priv->mon, backendType, objalias, props);
>      props = NULL; /* qemuMonitorAddObject consumes */
> @@ -1945,6 +1951,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>          mem = NULL;
>          goto audit;
>      }
> +    if (mem && restoreLabel &&

Coverity notes that checking for mem here is unnecessary.  It dereffed
at the top and there is no way to get to the exit_monitor label after
the mem = NULL.

> +        virSecurityManagerRestoreMemoryLabel(driver->securityManager,
> +                                             vm->def, mem) < 0)
> +        VIR_WARN("Unable to restore security label on memdev");

In any case, if this does stay within this label, I think it should move
to inside the 'orig_err' code...

The question becomes, if the qemuDomainObjExitMonitor fails, should the
Restore be called as well. Part of me says yes, but then it's noted in
the failure to ExitMonitor that we cannot touch mem, so we're SOL.

John
>  
>   removedef:
>      if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
> @@ -3141,6 +3151,10 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver,
>      if ((idx = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
>          virDomainMemoryRemove(vm->def, idx);
>  
> +    if (virSecurityManagerRestoreMemoryLabel(driver->securityManager,
> +                                             vm->def, mem) < 0)
> +        VIR_WARN("Unable to restore security label on memdev");
> +
>      virDomainMemoryDefFree(mem);
>  
>      /* fix the balloon size */
>