Re: [libvirt] [PATCH 13/14] qemu_hotplug: Hotplug of reservations

[Peter Krempa]

On Thu, Jan 18, 2018 at 17:04:45 +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 
> ---
>  src/qemu/qemu_hotplug.c | 97 
> +
>  1 file changed, 97 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 6b245bd6a..ded33 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -350,6 +350,84 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
>  }
>  
>  
> +static int
> +qemuBuildPRDefInfoProps(virDomainObjPtr vm,
> +virDomainDiskDefPtr disk,
> +virJSONValuePtr *prmgrProps,
> +const char **prAlias)
> +{
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +qemuDomainStorageSourcePrivatePtr srcPriv;
> +qemuDomainDiskPRObjectPtr tmp;
> +virJSONValuePtr props = NULL;
> +int ret = -1;

This function used in conjucntion with the JSON->commandline formatter
should be used instead of qemuBuildMasterPRCommandLineHelper so that we
don't have multiple implementations which need to be kept in sync ...


> +
> +srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
> +
> +*prmgrProps = NULL;
> +
> +if (!priv->prHelpers ||
> +!srcPriv->prAlias ||
> +!(tmp = virHashLookup(priv->prHelpers, srcPriv->prAlias)))
> +return 0;
> +
> +if (qemuDomainGetPRUsageCount(vm->def, srcPriv->prAlias) != 0) {
> +/* We're not the first ones to use pr-manager with that
> + * alias.  Return early and leave @prmgrProps NULL so
> + * that caller knows this. */
> +return 0;
> +}

Obviously you'll still need a wrapper for this logic ...


> +
> +if (virJSONValueObjectCreate(,
> + "s:path", tmp->path,
> + NULL) < 0)

... but we should not have two different implementations of this.

> +goto cleanup;
> +
> +if (qemuProcessSetupPRDaemon(vm, tmp, srcPriv->prAlias) < 0)
> +goto cleanup;
> +
> +*prAlias = srcPriv->prAlias;
> +*prmgrProps = props;
> +props = NULL;
> +ret = 0;
> + cleanup:
> +virJSONValueFree(props);
> +return ret;
> +}
> +
> +
> +static void
> +qemuDestroyPRDefObject(virDomainObjPtr vm,
> +   virDomainDiskDefPtr disk)
> +{
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +qemuDomainStorageSourcePrivatePtr srcPriv;
> +qemuDomainDiskPRObjectPtr tmp;
> +
> +srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
> +
> +if (!priv->prHelpers ||
> +!srcPriv->prAlias ||
> +!(tmp = virHashLookup(priv->prHelpers, srcPriv->prAlias)))
> +return;
> +
> +if (qemuDomainGetPRUsageCount(vm->def, srcPriv->prAlias) > 1) {
> +/* The function might return one or even zero if the
> + * pr-manager is unused depending whether the disk was
> + * added to domain def already or not. Anyway, if the
> + * return value is greater than one we are certain that
> + * there's another disk using it so return early. */

Shouldn't we use a different approach rather than comments like this?


> +return;
> +}
> +
> +/* This also kills the pr-manager daemon. See
> + * qemuDomainDiskPRObjectHashFree. */
> +virHashRemoveEntry(priv->prHelpers, srcPriv->prAlias);
> +
> +VIR_FREE(srcPriv->prAlias);
> +}
> +
> +
>  /**
>   * qemuDomainAttachDiskGeneric:
>   *


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 13/14] qemu_hotplug: Hotplug of reservations

[Michal Privoznik]

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 
---
 src/qemu/qemu_hotplug.c | 97 +
 1 file changed, 97 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6b245bd6a..ded33 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -350,6 +350,84 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
 }
 
 
+static int
+qemuBuildPRDefInfoProps(virDomainObjPtr vm,
+virDomainDiskDefPtr disk,
+virJSONValuePtr *prmgrProps,
+const char **prAlias)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuDomainStorageSourcePrivatePtr srcPriv;
+qemuDomainDiskPRObjectPtr tmp;
+virJSONValuePtr props = NULL;
+int ret = -1;
+
+srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
+
+*prmgrProps = NULL;
+
+if (!priv->prHelpers ||
+!srcPriv->prAlias ||
+!(tmp = virHashLookup(priv->prHelpers, srcPriv->prAlias)))
+return 0;
+
+if (qemuDomainGetPRUsageCount(vm->def, srcPriv->prAlias) != 0) {
+/* We're not the first ones to use pr-manager with that
+ * alias.  Return early and leave @prmgrProps NULL so
+ * that caller knows this. */
+return 0;
+}
+
+if (virJSONValueObjectCreate(,
+ "s:path", tmp->path,
+ NULL) < 0)
+goto cleanup;
+
+if (qemuProcessSetupPRDaemon(vm, tmp, srcPriv->prAlias) < 0)
+goto cleanup;
+
+*prAlias = srcPriv->prAlias;
+*prmgrProps = props;
+props = NULL;
+ret = 0;
+ cleanup:
+virJSONValueFree(props);
+return ret;
+}
+
+
+static void
+qemuDestroyPRDefObject(virDomainObjPtr vm,
+   virDomainDiskDefPtr disk)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuDomainStorageSourcePrivatePtr srcPriv;
+qemuDomainDiskPRObjectPtr tmp;
+
+srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
+
+if (!priv->prHelpers ||
+!srcPriv->prAlias ||
+!(tmp = virHashLookup(priv->prHelpers, srcPriv->prAlias)))
+return;
+
+if (qemuDomainGetPRUsageCount(vm->def, srcPriv->prAlias) > 1) {
+/* The function might return one or even zero if the
+ * pr-manager is unused depending whether the disk was
+ * added to domain def already or not. Anyway, if the
+ * return value is greater than one we are certain that
+ * there's another disk using it so return early. */
+return;
+}
+
+/* This also kills the pr-manager daemon. See
+ * qemuDomainDiskPRObjectHashFree. */
+virHashRemoveEntry(priv->prHelpers, srcPriv->prAlias);
+
+VIR_FREE(srcPriv->prAlias);
+}
+
+
 /**
  * qemuDomainAttachDiskGeneric:
  *
@@ -368,12 +446,15 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn,
 char *devstr = NULL;
 char *drivestr = NULL;
 char *drivealias = NULL;
+const char *prAlias = 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;
@@ -406,6 +487,9 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn,
   disk->info.alias) < 0)
 goto error;
 
+if (qemuBuildPRDefInfoProps(vm, disk, , ) < 0)
+goto error;
+
 if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
 goto error;
 
@@ -438,6 +522,15 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn,
 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;
@@ -458,6 +551,7 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn,
  cleanup:
 virJSONValueFree(secobjProps);
 virJSONValueFree(encobjProps);
+virJSONValueFree(prmgrProps);
 qemuDomainSecretDiskDestroy(disk);
 VIR_FREE(devstr);
 VIR_FREE(drivestr);
@@ -475,6 +569,8 @@ qemuDomainAttachDiskGeneric(virConnectPtr conn,
 ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias));
 if (encobjAdded)
 ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias));
+if (prmgrAdded)
+