Surprisingly, nothing special is happening here. If we are the
first to use the managed helper then spawn it. If not, we're
almost done.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_hotplug.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_process.c | 38 +++++++++++++++++++++-----
 src/qemu/qemu_process.h |  7 +++++
 3 files changed, 110 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e0a5300f0..8cc0b631d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -348,6 +348,58 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
 }
 
 
+static int
+qemuBuildPRDefInfoProps(virDomainObjPtr vm,
+                        virDomainDiskDefPtr disk,
+                        virJSONValuePtr *prmgrProps,
+                        const char **prAlias,
+                        const char **prPath)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    qemuDomainStorageSourcePrivatePtr srcPriv;
+    virJSONValuePtr props = NULL;
+    int ret = -1;
+
+    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
+
+    *prmgrProps = NULL;
+
+    if (priv->prPid != (pid_t) -1 ||
+        !srcPriv->prd ||
+        !srcPriv->prd->alias)
+        return 0;
+
+    if (virJSONValueObjectCreate(&props,
+                                 "s:path", srcPriv->prd->path,
+                                 NULL) < 0)
+        goto cleanup;
+
+    if (qemuProcessSetupOnePRDaemon(vm, disk) < 0)
+        goto cleanup;
+
+    *prAlias = srcPriv->prd->alias;
+    *prPath = srcPriv->prd->path;
+    *prmgrProps = props;
+    props = NULL;
+    ret = 0;
+ cleanup:
+    virJSONValueFree(props);
+    return ret;
+}
+
+
+static void
+qemuDestroyPRDefObject(virDomainObjPtr vm,
+                       const char *alias,
+                       const char *path)
+{
+    if (!alias)
+        return;
+
+    qemuProcessKillPRDaemon(vm, path, false);
+}
+
+
 /**
  * qemuDomainAttachDiskGeneric:
  *
@@ -365,12 +417,16 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
     char *devstr = NULL;
     char *drivestr = NULL;
     char *drivealias = NULL;
+    const char *prAlias = NULL;
+    const char *prPath = NULL;
     bool driveAdded = false;
     bool secobjAdded = false;
     bool encobjAdded = false;
+    bool prmgrAdded = false;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     virJSONValuePtr secobjProps = NULL;
     virJSONValuePtr encobjProps = NULL;
+    virJSONValuePtr prmgrProps = NULL;
     qemuDomainStorageSourcePrivatePtr srcPriv;
     qemuDomainSecretInfoPtr secinfo = NULL;
     qemuDomainSecretInfoPtr encinfo = NULL;
@@ -403,6 +459,9 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
                                       disk->info.alias) < 0)
         goto error;
 
+    if (qemuBuildPRDefInfoProps(vm, disk, &prmgrProps, &prAlias, &prPath) < 0)
+        goto error;
+
     if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
         goto error;
 
@@ -435,6 +494,15 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
         encobjAdded = true;
     }
 
+    if (prmgrProps) {
+        rv = qemuMonitorAddObject(priv->mon, "pr-manager-helper", prAlias,
+                                  prmgrProps);
+        prmgrProps = NULL; /* qemuMonitorAddObject consumes */
+        if (rv < 0)
+            goto exit_monitor;
+        prmgrAdded = true;
+    }
+
     if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
         goto exit_monitor;
     driveAdded = true;
@@ -455,6 +523,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
  cleanup:
     virJSONValueFree(secobjProps);
     virJSONValueFree(encobjProps);
+    virJSONValueFree(prmgrProps);
     qemuDomainSecretDiskDestroy(disk);
     VIR_FREE(devstr);
     VIR_FREE(drivestr);
@@ -472,6 +541,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
         ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias));
     if (encobjAdded)
         ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias));
+    if (prmgrAdded)
+        ignore_value(qemuMonitorDelObject(priv->mon, prAlias));
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         ret = -2;
     virErrorRestore(&orig_err);
@@ -481,6 +552,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
  error:
     qemuDomainDelDiskSrcTLSObject(driver, vm, disk->src);
     ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true));
+    qemuDestroyPRDefObject(vm, prAlias, prPath);
     goto cleanup;
 }
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b876d293a..cb160727c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2556,16 +2556,40 @@ qemuProcessResctrlCreate(virQEMUDriverPtr driver,
 }
 
 
-static void
-qemuProcessKillPRDaemon(virDomainObjPtr vm)
+void
+qemuProcessKillPRDaemon(virDomainObjPtr vm,
+                        const char *socketPath,
+                        bool force)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    size_t nmanaged = 0;
+    size_t i;
 
     if (priv->prPid == (pid_t) -1)
         return;
 
-    virProcessKillPainfully(priv->prPid, true);
-    priv->prPid = (pid_t) -1;
+    for (i = 0; i < vm->def->ndisks; i++) {
+        qemuDomainStorageSourcePrivatePtr srcPriv;
+        virDomainDiskDefPtr disk = vm->def->disks[i];
+
+        if (!virStoragePRDefIsManaged(disk->src->pr))
+            continue;
+
+        nmanaged++;
+
+        srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
+        if (!socketPath)
+            socketPath = srcPriv->prd->path;
+    }
+
+    if (force || nmanaged <= 1) {
+        virProcessKillPainfully(priv->prPid, true);
+        priv->prPid = (pid_t) -1;
+        if (socketPath &&
+            unlink(socketPath) < 0 &&
+            errno != ENOENT)
+            VIR_WARN("Unable to remove pr helper socket %s", socketPath);
+    }
 }
 
 
@@ -2593,7 +2617,7 @@ qemuProcessSetupOnePRDaemonHook(void *opaque)
 }
 
 
-static int
+int
 qemuProcessSetupOnePRDaemon(virDomainObjPtr vm,
                             virDomainDiskDefPtr disk)
 {
@@ -2713,7 +2737,7 @@ qemuProcessSetupPRDaemon(virDomainObjPtr vm)
     ret = 0;
  cleanup:
     if (ret < 0)
-        qemuProcessKillPRDaemon(vm);
+        qemuProcessKillPRDaemon(vm, NULL, true);
     return ret;
 }
 
@@ -6812,7 +6836,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
         VIR_FREE(vm->def->seclabels[i]->imagelabel);
     }
 
-    qemuProcessKillPRDaemon(vm);
+    qemuProcessKillPRDaemon(vm, NULL, true);
 
     qemuHostdevReAttachDomainDevices(driver, vm->def);
 
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 274111567..fab45eb2d 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -203,4 +203,11 @@ int qemuProcessRefreshDisks(virQEMUDriverPtr driver,
                             virDomainObjPtr vm,
                             qemuDomainAsyncJob asyncJob);
 
+int qemuProcessSetupOnePRDaemon(virDomainObjPtr vm,
+                                virDomainDiskDefPtr disk);
+
+void qemuProcessKillPRDaemon(virDomainObjPtr vm,
+                             const char *socketPath,
+                             bool force);
+
 #endif /* __QEMU_PROCESS_H__ */
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, Mar 14, 2018 at 17:05:39 +0100, Michal Privoznik wrote:
> Surprisingly, nothing special is happening here. If we are the
> first to use the managed helper then spawn it. If not, we're
> almost done.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_process.c | 38 +++++++++++++++++++++-----
>  src/qemu/qemu_process.h |  7 +++++
>  3 files changed, 110 insertions(+), 7 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index e0a5300f0..8cc0b631d 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -348,6 +348,58 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>  }
>  
>  
> +static int
> +qemuBuildPRDefInfoProps(virDomainObjPtr vm,
> +                        virDomainDiskDefPtr disk,
> +                        virJSONValuePtr *prmgrProps,
> +                        const char **prAlias,
> +                        const char **prPath)

It looks like this function is doing a bit too much for the name. It's
not only creating the JSON props of the object but setting up the
daemon. That looks like should be separated.

> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    qemuDomainStorageSourcePrivatePtr srcPriv;
> +    virJSONValuePtr props = NULL;
> +    int ret = -1;
> +
> +    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
> +
> +    *prmgrProps = NULL;
> +
> +    if (priv->prPid != (pid_t) -1 ||
> +        !srcPriv->prd ||
> +        !srcPriv->prd->alias)
> +        return 0;

So, this will exit early if you have the managed daemon, so you will not
add the object for the unmanaged option if hotplugging?

As said above, this function does too much. It should be split and then
you will not have these problems.

> +    if (virJSONValueObjectCreate(&props,
> +                                 "s:path", srcPriv->prd->path,
> +                                 NULL) < 0)
> +        goto cleanup;
> +
> +    if (qemuProcessSetupOnePRDaemon(vm, disk) < 0)
> +        goto cleanup;
> +
> +    *prAlias = srcPriv->prd->alias;
> +    *prPath = srcPriv->prd->path;
> +    *prmgrProps = props;
> +    props = NULL;
> +    ret = 0;
> + cleanup:
> +    virJSONValueFree(props);
> +    return ret;
> +}
> +
> +
> +static void
> +qemuDestroyPRDefObject(virDomainObjPtr vm,
> +                       const char *alias,
> +                       const char *path)
> +{
> +    if (!alias)
> +        return;
> +
> +    qemuProcessKillPRDaemon(vm, path, false);

This does not make much sense. You only ever need to kill the managed
one, so passing random path is weird. This should be called only for the
managed daemon.

> +}
> +
> +
>  /**
>   * qemuDomainAttachDiskGeneric:
>   *

[...]

> @@ -403,6 +459,9 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
>                                        disk->info.alias) < 0)
>          goto error;
>  
> +    if (qemuBuildPRDefInfoProps(vm, disk, &prmgrProps, &prAlias, &prPath) < 0)
> +        goto error;
> +
>      if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
>          goto error;
>  
> @@ -435,6 +494,15 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
>          encobjAdded = true;
>      }
>  
> +    if (prmgrProps) {
> +        rv = qemuMonitorAddObject(priv->mon, "pr-manager-helper", prAlias,
> +                                  prmgrProps);
> +        prmgrProps = NULL; /* qemuMonitorAddObject consumes */
> +        if (rv < 0)
> +            goto exit_monitor;
> +        prmgrAdded = true;
> +    }
> +
>      if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
>          goto exit_monitor;
>      driveAdded = true;

[...]

> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index b876d293a..cb160727c 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2556,16 +2556,40 @@ qemuProcessResctrlCreate(virQEMUDriverPtr driver,
>  }
>  
>  
> -static void
> -qemuProcessKillPRDaemon(virDomainObjPtr vm)
> +void
> +qemuProcessKillPRDaemon(virDomainObjPtr vm,
> +                        const char *socketPath,

socketPath seems useless here ...

> +                        bool force)
>  {
>      qemuDomainObjPrivatePtr priv = vm->privateData;
> +    size_t nmanaged = 0;
> +    size_t i;
>  
>      if (priv->prPid == (pid_t) -1)
>          return;
>  
> -    virProcessKillPainfully(priv->prPid, true);
> -    priv->prPid = (pid_t) -1;
> +    for (i = 0; i < vm->def->ndisks; i++) {
> +        qemuDomainStorageSourcePrivatePtr srcPriv;
> +        virDomainDiskDefPtr disk = vm->def->disks[i];
> +
> +        if (!virStoragePRDefIsManaged(disk->src->pr))
> +            continue;
> +
> +        nmanaged++;
> +
> +        srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
> +        if (!socketPath)
> +            socketPath = srcPriv->prd->path;
> +    }
> +
> +    if (force || nmanaged <= 1) {
> +        virProcessKillPainfully(priv->prPid, true);

since this will be true only for the managed daemons, thus you'll have
the socket path from the definition.

> +        priv->prPid = (pid_t) -1;
> +        if (socketPath &&
> +            unlink(socketPath) < 0 &&
> +            errno != ENOENT)
> +            VIR_WARN("Unable to remove pr helper socket %s", socketPath);
> +    }
>  }
>  
>  
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list