[libvirt] [PATCH v2 12/14] qemu_hotplug: Relabel memdev

[John Ferlan]

On 02/27/2017 08:19 AM, Michal Privoznik wrote:
> Now that we have APIs for relabel memdevs on hotplug, fill in the
> missing implementation in qemu hotplug code.
> 
> The qemuSecurity wrappers might look like overkill for now,
> because qemu namespace code does not deal with the nvdimms yet.
> Nor does our cgroup code.  But hey, there's cgroup_device_acl
> variable in qemu.conf. If users add their /dev/pmem* device in
> there, the device is allowed in cgroups and created in the
> namespace so they can successfully passthrough it to the domain.
> It doesn't look like overkill after all, does it?
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_hotplug.c  | 13 +++++++++++
>  src/qemu/qemu_security.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_security.h |  8 +++++++
>  3 files changed, 77 insertions(+)
> 

There's a git am -3 conflict here:

Applying: qemu_hotplug: Relabel memdev
error: patch failed: src/qemu/qemu_security.h:60
error: src/qemu/qemu_security.h: patch does not apply
Patch failed at 0012 qemu_hotplug: Relabel memdev


> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index f2299f66e..7e837a422 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2191,6 +2191,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>      char *objalias = NULL;
>      const char *backendType;
>      bool objAdded = false;
> +    bool teardownlabel = false;
>      virJSONValuePtr props = NULL;
>      virObjectEventPtr event;
>      int id;
> @@ -2232,6 +2233,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>          goto removedef;
>      }
>  
> +    if (qemuSecuritySetMemoryLabel(driver, vm, mem) < 0)
> +        goto removedef;
> +    teardownlabel = true;
> +

I almost wonder if this should go before the virDomainMemoryInsert which
results in callers jumping to removedef.

Secondary should the failure and teardown for this and the subsequent
objects have a separate label which can save/restore the error that
occurred so that I don't have to go check the called function to see if
any of the functions it calls (and so on) would perhaps overwrite the
error that caused the initial failure.

The other "oddity" would be that by going to removedef, one jumps to
audit rather than code before the AdjustMaxMemLock which would go to
cleanup and avoid the audit.  I dunno - I guess I'd expect the audit to
occur, but we don't seem to be consistent within hotplug overall.

John

>      qemuDomainObjEnterMonitor(driver, vm);
>      rv = qemuMonitorAddObject(priv->mon, backendType, objalias, props);
>      props = NULL; /* qemuMonitorAddObject consumes */
> @@ -2266,6 +2271,11 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>   audit:
>      virDomainAuditMemory(vm, oldmem, newmem, "update", ret == 0);
>   cleanup:
> +    if (mem && ret < 0) {
> +        if (teardownlabel && qemuSecurityRestoreMemoryLabel(driver, vm, mem) < 0)
> +            VIR_WARN("Unable to restore security label on memdev");
> +    }
> +
>      virObjectUnref(cfg);
>      VIR_FREE(devstr);
>      VIR_FREE(objalias);
> @@ -3726,6 +3736,9 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver,
>      if ((idx = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
>          virDomainMemoryRemove(vm->def, idx);
>  
> +    if (qemuSecurityRestoreMemoryLabel(driver, vm, mem) < 0)
> +        VIR_WARN("Unable to restore security label on memdev");
> +
>      virDomainMemoryDefFree(mem);
>  
>      /* fix the balloon size */
> diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
> index f2931976b..61934f990 100644
> --- a/src/qemu/qemu_security.c
> +++ b/src/qemu/qemu_security.c
> @@ -245,3 +245,59 @@ qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver,
>      virSecurityManagerTransactionAbort(driver->securityManager);
>      return ret;
>  }
> +
> +
> +int
> +qemuSecuritySetMemoryLabel(virQEMUDriverPtr driver,
> +                           virDomainObjPtr vm,
> +                           virDomainMemoryDefPtr mem)
> +{
> +    int ret = -1;
> +
> +    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
> +        virSecurityManagerTransactionStart(driver->securityManager) < 0)
> +        goto cleanup;
> +
> +    if (virSecurityManagerSetMemoryLabel(driver->securityManager,
> +                                         vm->def,
> +                                         mem) < 0)
> +        goto cleanup;
> +
> +    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
> +        virSecurityManagerTransactionCommit(driver->securityManager,
> +                                            vm->pid) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> + cleanup:
> +    virSecurityManagerTransactionAbort(driver->securityManager);
> +    return ret;
> +}
> +
> +
> +int
> +qemuSecurityRestoreMemoryLabel(virQEMUDriverPtr driver,
> +                               virDomainObjPtr vm,
> +                               virDomainMemoryDefPtr mem)
> +{
> +    int ret = -1;
> +
> +    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
> +        virSecurityManagerTransactionStart(driver->securityManager) < 0)
> +        goto cleanup;
> +
> +    if (virSecurityManagerRestoreMemoryLabel(driver->securityManager,
> +                                             vm->def,
> +                                             mem) < 0)
> +        goto cleanup;
> +
> +    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT) &&
> +        virSecurityManagerTransactionCommit(driver->securityManager,
> +                                            vm->pid) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> + cleanup:
> +    virSecurityManagerTransactionAbort(driver->securityManager);
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h
> index 54638908d..04a0b82d4 100644
> --- a/src/qemu/qemu_security.h
> +++ b/src/qemu/qemu_security.h
> @@ -60,4 +60,12 @@ int qemuSecuritySetHostdevLabel(virQEMUDriverPtr driver,
>  int qemuSecurityRestoreHostdevLabel(virQEMUDriverPtr driver,
>                                      virDomainObjPtr vm,
>                                      virDomainHostdevDefPtr hostdev);
> +
> +int qemuSecuritySetMemoryLabel(virQEMUDriverPtr driver,
> +                               virDomainObjPtr vm,
> +                               virDomainMemoryDefPtr mem);
> +
> +int qemuSecurityRestoreMemoryLabel(virQEMUDriverPtr driver,
> +                                   virDomainObjPtr vm,
> +                                   virDomainMemoryDefPtr mem);
>  #endif /* __QEMU_SECURITY_H__ */
>