On 05/14/2018 06:41 AM, Peter Krempa wrote:
> Rather than always checking which path to use pre-assign it when
> preparing storage source.
>
> This reduces the need to pass 'vm' around too much. For later use the
> path can be retrieved from the status XML.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> src/qemu/qemu_command.c | 18 +++++-------------
> src/qemu/qemu_command.h | 3 +--
> src/qemu/qemu_domain.c | 35 ++++++++++++++++++++++-------------
> src/qemu/qemu_domain.h | 3 +--
> src/qemu/qemu_hotplug.c | 2 +-
> src/qemu/qemu_process.c | 2 +-
> 6 files changed, 31 insertions(+), 32 deletions(-)
>
Strange this patch got posted twice once w/ your own CC and once without.
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 2bdba7734a..7df9979cb2 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9667,7 +9667,6 @@ qemuBuildPanicCommandLine(virCommandPtr cmd,
>
> /**
> * qemuBuildPRManagerInfoProps:
> - * @vm: domain object
> * @disk: disk definition
> * @propsret: Returns JSON object containing properties of the pr-manager-helper object
> * @aliasret: alias of the pr-manager-helper object
> @@ -9678,12 +9677,10 @@ qemuBuildPanicCommandLine(virCommandPtr cmd,
> * -1 on failure (with error message set).
> */
> int
> -qemuBuildPRManagerInfoProps(virDomainObjPtr vm,
> - const virDomainDiskDef *disk,
> +qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk,
> virJSONValuePtr *propsret,
> char **aliasret)
> {
> - char *socketPath = NULL;
> char *alias = NULL;
> int ret = -1;
>
> @@ -9693,9 +9690,6 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm,
> if (!disk->src->pr)
> return 0;
>
> - if (!(socketPath = qemuDomainGetPRSocketPath(vm, disk->src->pr)))
> - return ret;
> -
> if (virStoragePRDefIsManaged(disk->src->pr)) {
> if (VIR_STRDUP(alias, qemuDomainGetManagedPRAlias()) < 0)
> goto cleanup;
> @@ -9705,7 +9699,7 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm,
> }
>
> if (virJSONValueObjectCreate(propsret,
> - "s:path", socketPath,
> + "s:path", disk->src->pr->path,
> NULL) < 0)
> goto cleanup;
>
> @@ -9713,14 +9707,12 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm,
> ret = 0;
> cleanup:
> VIR_FREE(alias);
> - VIR_FREE(socketPath);
> return ret;
> }
>
>
> static int
> -qemuBuildMasterPRCommandLine(virDomainObjPtr vm,
> - virCommandPtr cmd,
> +qemuBuildMasterPRCommandLine(virCommandPtr cmd,
> const virDomainDef *def)
> {
> size_t i;
> @@ -9740,7 +9732,7 @@ qemuBuildMasterPRCommandLine(virDomainObjPtr vm,
> managedAdded = true;
> }
>
> - if (qemuBuildPRManagerInfoProps(vm, disk, &props, &alias) < 0)
> + if (qemuBuildPRManagerInfoProps(disk, &props, &alias) < 0)
> goto cleanup;
>
> if (!props)
> @@ -9934,7 +9926,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
> if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0)
> goto error;
>
> - if (qemuBuildMasterPRCommandLine(vm, cmd, def) < 0)
> + if (qemuBuildMasterPRCommandLine(cmd, def) < 0)
Rather than @vm - what was really desired is/was @priv, which is already
passed for qemuBuildMasterKeyCommandLine and could be for this too...
Thus not requiring the hunks that change path in disk->src->pr->path.
It is a static path beyond the priv->libDir part.
> goto error;
>
> if (enableFips)
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index da1fe679fe..621592cd79 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -55,8 +55,7 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver,
> int **nicindexes);
>
> /* Generate the object properties for pr-manager */
> -int qemuBuildPRManagerInfoProps(virDomainObjPtr vm,
> - const virDomainDiskDef *disk,
> +int qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk,
> virJSONValuePtr *propsret,
> char **alias);
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index eaa796260c..92217e66fe 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -11930,6 +11930,22 @@ qemuDomainPrepareDiskCachemode(virDomainDiskDefPtr disk)
> }
>
>
> +static int
> +qemuDomainPrepareStorageSourcePR(virStorageSourcePtr src,
> + qemuDomainObjPrivatePtr priv)
> +{
> + if (!src->pr)
> + return 0;
> +
> + if (virStoragePRDefIsManaged(src->pr)) {
> + if (!(src->pr->path = qemuDomainGetManagedPRSocketPath(priv)))
> + return -1;
> + }
While I understand the eventual goal - assigning a path here would seem
to be "contrary" to qemuDomainValidateStorageSource in the previous
patch. IOW: ->path should not be provided for managed reservations.
Doesn't that mean from this point forward we need to be careful to not
save the resulting path for the persistent XML? Or am I lost in the
weeds again?
If this is to stay, then is this perhaps where :
<reservations managed='yes'>
<source type='unix' path='/somepath/ux.sck' mode='client'/>
</reservations>
from patch 12 should be included for
tests/qemustatusxml2xmldata/modern-in.xml ?
John
> +
> + return 0;
> +}
> +
> +
> int
> qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
> qemuDomainObjPrivatePtr priv,
> @@ -11946,6 +11962,9 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
> if (qemuDomainPrepareDiskSourceChain(disk, NULL, cfg, priv->qemuCaps) < 0)
> return -1;
>
> + if (qemuDomainPrepareStorageSourcePR(disk->src, priv) < 0)
> + return -1;
> +
> return 0;
> }
>
> @@ -12051,22 +12070,12 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
>
>
> char *
> -qemuDomainGetPRSocketPath(virDomainObjPtr vm,
> - virStoragePRDefPtr pr)
> +qemuDomainGetManagedPRSocketPath(qemuDomainObjPrivatePtr priv)
> {
> - qemuDomainObjPrivatePtr priv = vm->privateData;
> - const char *defaultAlias = NULL;
> char *ret = NULL;
>
> - if (!pr)
> - return NULL;
> -
> - if (virStoragePRDefIsManaged(pr)) {
> - defaultAlias = qemuDomainGetManagedPRAlias();
> - ignore_value(virAsprintf(&ret, "%s/%s.sock", priv->libDir, defaultAlias));
> - } else {
> - ignore_value(VIR_STRDUP(ret, pr->path));
> - }
> + ignore_value(virAsprintf(&ret, "%s/%s.sock", priv->libDir,
> + qemuDomainGetManagedPRAlias()));
>
> return ret;
> }
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 09969f606a..40d1d095a3 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -1006,7 +1006,6 @@ qemuDomainDiskCachemodeFlags(int cachemode,
> bool *direct,
> bool *noflush);
>
> -char * qemuDomainGetPRSocketPath(virDomainObjPtr vm,
> - virStoragePRDefPtr pr);
> +char * qemuDomainGetManagedPRSocketPath(qemuDomainObjPrivatePtr priv);
>
> #endif /* __QEMU_DOMAIN_H__ */
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 39c457e607..77d37e5ef6 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -401,7 +401,7 @@ qemuMaybeBuildPRManagerInfoProps(virDomainObjPtr vm,
> return 0;
> }
>
> - return qemuBuildPRManagerInfoProps(vm, disk, propsret, aliasret);
> + return qemuBuildPRManagerInfoProps(disk, propsret, aliasret);
> }
>
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index a280784764..42b91b39ac 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2659,7 +2659,7 @@ qemuProcessStartPRDaemon(virDomainObjPtr vm,
> if ((pidfd = virPidFileAcquirePath(pidfile, false, -1)) < 0)
> goto cleanup;
>
> - if (!(socketPath = qemuDomainGetPRSocketPath(vm, disk->src->pr)))
> + if (!(socketPath = qemuDomainGetManagedPRSocketPath(priv)))
> goto cleanup;
>
> /* Remove stale socket */
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
