On Mon, May 14, 2018 at 16:27:17 -0400, John Ferlan wrote: > > > 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 <pkre...@redhat.com> > > --- > > 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.
Yes, my mailserver rejected sending of the next patch so I've retried it manually without the CC and messed up selecting which ones I should resend. > > > 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? It should never be saved in persistent XML since the parser+validator will not allow parsing it. Since we don't modify the persistent XML in this case it's not a problem. The validation code is not called for the status XML so that should be covered as well. > > 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 ? Hmm, yeah we can add that too.
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list