[PATCH v2 3/3] qemu: Do not erase duplicate devices from namespace if error occurs

[Michal Prívozník]

On 7/14/21 4:46 PM, Kristina Hanicova wrote:
> If the attempt to attach a device failed, we erased the
> unattached device from the namespace. This resulted in erasing an
> already attached device in case of a duplicate. We need to check
> for existing file in the namespace in order to determine erasing
> it in case of a failure.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1780508
> 
> Signed-off-by: Kristina Hanicova <khanicov at redhat.com>
> ---
>  src/qemu/qemu_domain.c    |  4 +--
>  src/qemu/qemu_hotplug.c   | 27 +++++++------------
>  src/qemu/qemu_namespace.c | 55 +++++++++++++++++++++++----------------
>  src/qemu/qemu_namespace.h | 18 ++++++++-----
>  src/qemu/qemu_process.c   |  2 +-
>  5 files changed, 55 insertions(+), 51 deletions(-)
> 

> diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
> index eb048a2faa..46ee95b8c8 100644
> --- a/src/qemu/qemu_namespace.c
> +++ b/src/qemu/qemu_namespace.c

> @@ -1235,7 +1236,8 @@ qemuNamespacePrepareOneItem(qemuNamespaceMknodData *data,
>  
>  static int
>  qemuNamespaceMknodPaths(virDomainObj *vm,
> -                        GSList *paths)
> +                        GSList *paths,
> +                        bool *created)
>  {
>      qemuDomainObjPrivate *priv = vm->privateData;
>      virQEMUDriver *driver = priv->driver;
> @@ -1280,15 +1282,13 @@ qemuNamespaceMknodPaths(virDomainObj *vm,
>      if (qemuSecurityPreFork(driver->securityManager) < 0)
>          goto cleanup;
>  
> -    if (virProcessRunInMountNamespace(vm->pid,
> -                                      qemuNamespaceMknodHelper,
> -                                      &data) < 0) {
> -        qemuSecurityPostFork(driver->securityManager);
> -        goto cleanup;
> -    }
> +    ret = virProcessRunInMountNamespace(vm->pid, qemuNamespaceMknodHelper,
> +                                        &data);
> +    if (ret == 0 && created != NULL)
> +        *created = true;
> +
>      qemuSecurityPostFork(driver->securityManager);
>  

Here it's better if qemuSecurityPostFork() is called before the if().
The reason we have PreFork() and PostFork() calls is to fight
async-signal unsafe functions; Anyway - PreFork() locks
driver->securityManager() and only PostFork() unlocks it. And it's just
virProcessRunInMountNamespace() that's in critical section, not if().
Micro optimization, I know.

Michal