[libvirt] [PATCH 10/13] qemu: command: Move check whether PR manager object props need to be built

Peter Krempa pkrempa at redhat.com
Tue May 15 15:53:23 UTC 2018


On Mon, May 14, 2018 at 16:46:15 -0400, John Ferlan wrote:
> 
> 
> On 05/14/2018 06:45 AM, Peter Krempa wrote:
> > Move it out of the formatter function and let the caller decide this.
> 
> s/formatter/format/
> 
> I cannot recall our current consistency...  Some times we seem to have
> the lowest routine make the singular check and other times we have the
> caller make the check.
> 
> > 
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> >  src/qemu/qemu_command.c | 9 +++------
> >  src/qemu/qemu_hotplug.c | 3 +++
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 7df9979cb2..c38dde5a60 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -9687,9 +9687,6 @@ qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk,
> >      *propsret = NULL;
> >      *aliasret = NULL;
> > 
> > -    if (!disk->src->pr)
> > -        return 0;
> > -
> >      if (virStoragePRDefIsManaged(disk->src->pr)) {
> >          if (VIR_STRDUP(alias, qemuDomainGetManagedPRAlias()) < 0)
> >              goto cleanup;
> > @@ -9725,6 +9722,9 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd,
> >      for (i = 0; i < def->ndisks; i++) {
> >          const virDomainDiskDef *disk = def->disks[i];
> > 
> > +        if (!disk->src->pr)
> > +            continue;
> > +
> >          if (virStoragePRDefIsManaged(disk->src->pr)) {
> >              if (managedAdded)
> >                  continue;
> > @@ -9735,9 +9735,6 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd,
> >          if (qemuBuildPRManagerInfoProps(disk, &props, &alias) < 0)
> >              goto cleanup;
> > 
> > -        if (!props)
> > -            continue;
> > -
> 
> This one seems unrelated... although at this point I'm not sure how it
> could happen...  I'd say keep it as is unless Michal had some other
> reason or maybe remembers what it was there for originally.

It actually is related. Prior to this patch qemuBuildPRManagerInfoProps
either filled props or not depending on the configuration.

This patch is actually changing that so that it always returns props
and moves the duty of checking whether it is needed to the caller.

In the current situation props will ever be set to null when
qemuBuildPRManagerInfoProps returns -1 due to allocation failure.
-------------- 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/20180515/3792668a/attachment-0001.sig>


More information about the libvir-list mailing list