[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 <mprivozn at redhat.com>
> ---
>  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..ded666633 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(&props,
> +                                 "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:
>   *
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180212/edeb17fa/attachment-0001.sig>