Re: [PATCH 09/13] qemu: process: Change semantics of functions starting PR daemon

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 14, 2018 at 16:33:58 -0400, John Ferlan wrote:
> 
> 
> On 05/14/2018 06:45 AM, Peter Krempa wrote:
> > Libvirt only manages one PR daemon. This means that we don't need to
> > pass the 'disk' object and also rename the functions dealing with this
> > so that it's obvious we only deal with the managed PR daemon.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat st.com>
>                                              ^^^^
> 
> Something strange happened here.

Yeah, I've messed up while editing the commit message somehow. It also
broke sending of the patches later on.

> 
> 
> > ---
> >  src/qemu/qemu_hotplug.c |  6 +++---
> >  src/qemu/qemu_process.c | 37 ++++++++++++++++---------------------
> >  src/qemu/qemu_process.h |  5 ++---
> >  3 files changed, 21 insertions(+), 27 deletions(-)
> > 
> 
> [...]
> 
> >  int
> > -qemuProcessStartPRDaemon(virDomainObjPtr vm,
> > -                         const virDomainDiskDef *disk)
> > +qemuProcessStartManagedPRDaemon(virDomainObjPtr vm)
> >  {
> >      qemuDomainObjPrivatePtr priv = vm->privateData;
> >      virQEMUDriverPtr driver = priv->driver;
> > @@ -2640,10 +2639,6 @@ qemuProcessStartPRDaemon(virDomainObjPtr vm,
> >      const unsigned long long timeout = 500000; /* ms */
> >      int ret = -1;
> > 
> > -    if (!virStoragePRDefIsManaged(disk->src->pr) ||
> > -        priv->prDaemonRunning)
> > -        return 0;
> > -
> >      cfg = virQEMUDriverGetConfig(driver);
> > 
> >      if (!virFileIsExecutable(cfg->prHelperName)) {
> > @@ -2734,7 +2729,7 @@ qemuProcessStartPRDaemon(virDomainObjPtr vm,
> >          goto cleanup;
> > 
> >      priv->prDaemonRunning = true;
> > -    ret = 1;
> > +    ret = 0;
> 
> Unrelated, but since callers only care about < 0, no big deal...   I'm
> assuming this is a holder from some other path which used the function
> for a return value (e.g. like qemuDomainMaybeStartPRDaemon) at some
> point during development.
> 
> 
> No big deal - I don't expect this to be extracted, but was just paying
> attention ;-)

The code path was actually exactly used in the case when
qemuDomainMaybeStartPRDaemon called into this function. Since the logic
determining whether it was started was extracted into that function (in
trimmed context) it now returns the correct thing there which is
expected and qemuProcessStartManagedPRDaemon does not have to do this
any more.

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux