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

[Peter Krempa]

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 at 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.

-------------- 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/0536ea99/attachment-0001.sig>