On Thu, 16 Feb 2023 14:32:48 +0100
Michal Privoznik <mprivozn(a)redhat.com> wrote:

...
  When passt is started, it daemonizes itself by
default. There's
 no point in having our virCommand module daemonize it too.
 
 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
 ---
  src/qemu/qemu_passt.c | 1 -
  1 file changed, 1 deletion(-)
 
 diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
 index 78830fdc26..adc69fc052 100644
 --- a/src/qemu/qemu_passt.c
 +++ b/src/qemu/qemu_passt.c
 @@ -156,7 +156,6 @@ qemuPasstStart(virDomainObj *vm,
      virCommandClearCaps(cmd);
      virCommandSetPidFile(cmd, pidfile);
      virCommandSetErrorFD(cmd, &errfd);
 -    virCommandDaemonize(cmd);
  
      virCommandAddArgList(cmd,
                           "--one-off", 
For what it's worth,

Reviewed-by: Stefano Brivio <sbrivio(a)redhat.com>

-- 
Stefano

On 2/16/23 8:32 AM, Michal Privoznik wrote:
...
  When starting passt, it may write something onto its
stderr
 (convincing it to print even more is addressed later). Pass this
 string we read to user.
 
 Since we're not daemonizing passt anymore (see previous commit),
 we can let virCommand module do all the heavy lifting and switch
 to virCommandSetErrorBuffer() instead of reading error from an
 FD.
 
 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> 
Reviewed-by: Laine Stump <laine(a)redhat.com>

(also a part of the reverted patch)

...
  ---
   src/qemu/qemu_passt.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
 index adc69fc052..c082c149cd 100644
 --- a/src/qemu/qemu_passt.c
 +++ b/src/qemu/qemu_passt.c
 @@ -144,18 +144,18 @@ qemuPasstStart(virDomainObj *vm,
       g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net);
       g_autoptr(virCommand) cmd = NULL;
       g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
 +    g_autofree char *errbuf = NULL;
       char macaddr[VIR_MAC_STRING_BUFLEN];
       size_t i;
       pid_t pid = (pid_t) -1;
       int exitstatus = 0;
       int cmdret = 0;
 -    VIR_AUTOCLOSE errfd = -1;
   
       cmd = virCommandNew(PASST);
   
       virCommandClearCaps(cmd);
       virCommandSetPidFile(cmd, pidfile);
 -    virCommandSetErrorFD(cmd, &errfd);
 +    virCommandSetErrorBuffer(cmd, &errbuf);
   
       virCommandAddArgList(cmd,
                            "--one-off",
 @@ -266,7 +266,7 @@ qemuPasstStart(virDomainObj *vm,
   
       if (cmdret < 0 || exitstatus != 0) {
           virReportError(VIR_ERR_INTERNAL_ERROR,
 -                       _("Could not start 'passt'. exitstatus: %d"),
exitstatus);
 +                       _("Could not start 'passt': %s"),
NULLSTR(errbuf));
           goto error;
       }
    

On 2/16/23 7:32 AM, Michal Privoznik wrote:
...
  Passt has '--stderr' argument which makes it
report error onto
 stderr rather to system log. Unfortunately, it's currently
 impossible to use both '--log-file' and '--stderr', so pass the
 latter only if the former isn't passed. Then, use the stderr to
 produce more user friendly error message on failed start.
 
 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
 ---
   src/qemu/qemu_passt.c | 22 +++++++++++++++++++---
   1 file changed, 19 insertions(+), 3 deletions(-)
 
 diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
 index c082c149cd..881205449b 100644
 --- a/src/qemu/qemu_passt.c
 +++ b/src/qemu/qemu_passt.c
 @@ -171,8 +171,13 @@ qemuPasstStart(virDomainObj *vm,
       if (net->sourceDev)
           virCommandAddArgList(cmd, "--interface", net->sourceDev, NULL);
   
 -    if (net->backend.logFile)
 +    if (net->backend.logFile) {
           virCommandAddArgList(cmd, "--log-file", net->backend.logFile,
NULL);
 +    } else {
 +        /* By default, passt logs into system logger. But we are interested
 +         * into errors too. Make it print errors onto stderr. */ 
s/into/in the/ ?

...
  +        virCommandAddArg(cmd, "--stderr");
 +    }
   
       /* Add IP address info */
       for (i = 0; i < net->guestIP.nips; i++) {
 @@ -265,8 +270,19 @@ qemuPasstStart(virDomainObj *vm,
           goto error;
   
       if (cmdret < 0 || exitstatus != 0) {
 -        virReportError(VIR_ERR_INTERNAL_ERROR,
 -                       _("Could not start 'passt': %s"),
NULLSTR(errbuf));
 +        if (STRNEQ_NULLABLE(errbuf, "")) {
 +            virReportError(VIR_ERR_INTERNAL_ERROR,
 +                           _("Could not start 'passt': %s"),
errbuf);
 +        } else if (net->backend.logFile) {
 +            virReportError(VIR_ERR_INTERNAL_ERROR,
 +                           _("Could not start 'passt': look into %s for
error"),
 +                           net->backend.logFile);
 +        } else {
 +            virReportError(VIR_ERR_INTERNAL_ERROR,
 +                           _("Could not start 'passt': exit status =
'%d'"),
 +                           exitstatus);
 +        }
 +
           goto error;
       }
    

On Thu, Feb 16, 2023 at 05:21:48PM +0100, Michal Prívozník wrote:
...
  On 2/16/23 16:42, Jonathon Jongsma wrote:
  On 2/16/23 7:32 AM, Michal Privoznik wrote:
  Passt has '--stderr' argument which
makes it report error onto
 stderr rather to system log. Unfortunately, it's currently
 impossible to use both '--log-file' and '--stderr', so pass the
 latter only if the former isn't passed. Then, use the stderr to
 produce more user friendly error message on failed start.

 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
 ---
   src/qemu/qemu_passt.c | 22 +++++++++++++++++++---
   1 file changed, 19 insertions(+), 3 deletions(-)

 diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
 index c082c149cd..881205449b 100644
 --- a/src/qemu/qemu_passt.c
 +++ b/src/qemu/qemu_passt.c
 @@ -171,8 +171,13 @@ qemuPasstStart(virDomainObj *vm,
       if (net->sourceDev)
           virCommandAddArgList(cmd, "--interface", net->sourceDev, NULL);
   -    if (net->backend.logFile)
 +    if (net->backend.logFile) {
           virCommandAddArgList(cmd, "--log-file",
 net->backend.logFile, NULL);
 +    } else {
 +        /* By default, passt logs into system logger. But we are
 interested
 +         * into errors too. Make it print errors onto stderr. */ 
 
 s/into/in the/ ? 
 
 Honestly, I have no idea. I'm not a native speaker. 
I am, and "in the" definitely seems more correct here.

...
  Maybe it's 'print
 onto paper' but 'print into a stream'? 
Fwiw, s/onto/to/ also reads slightly better.

...
  
 Anyway, fixed locally. Thanks.
 
 Michal
  
-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

On 2/16/23 17:07, Stefano Brivio wrote:
...
  On Thu, 16 Feb 2023 14:32:50 +0100
 Michal Privoznik <mprivozn(a)redhat.com> wrote:
 
  Passt has '--stderr' argument which
makes it report error onto
 stderr rather to system log. Unfortunately, it's currently
 impossible to use both '--log-file' and '--stderr', so pass the
 latter only if the former isn't passed. Then, use the stderr to
 produce more user friendly error message on failed start.

 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
 ---
  src/qemu/qemu_passt.c | 22 +++++++++++++++++++---
  1 file changed, 19 insertions(+), 3 deletions(-)

 diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
 index c082c149cd..881205449b 100644
 --- a/src/qemu/qemu_passt.c
 +++ b/src/qemu/qemu_passt.c
 @@ -171,8 +171,13 @@ qemuPasstStart(virDomainObj *vm,
      if (net->sourceDev)
          virCommandAddArgList(cmd, "--interface", net->sourceDev, NULL);
  
 -    if (net->backend.logFile)
 +    if (net->backend.logFile) {
          virCommandAddArgList(cmd, "--log-file", net->backend.logFile,
NULL);
 +    } else {
 +        /* By default, passt logs into system logger. But we are interested
 +         * into errors too. Make it print errors onto stderr. */
 +        virCommandAddArg(cmd, "--stderr");
 +    } 
 
 There's no need for this, see my previous email, archived at:
   https://listman.redhat.com/archives/libvir-list/2023-February/237880.html
 
   
      /* Add IP address info */
      for (i = 0; i < net->guestIP.nips; i++) {
 @@ -265,8 +270,19 @@ qemuPasstStart(virDomainObj *vm,
          goto error;
  
      if (cmdret < 0 || exitstatus != 0) {
 -        virReportError(VIR_ERR_INTERNAL_ERROR,
 -                       _("Could not start 'passt': %s"),
NULLSTR(errbuf));
 +        if (STRNEQ_NULLABLE(errbuf, "")) {
 +            virReportError(VIR_ERR_INTERNAL_ERROR,
 +                           _("Could not start 'passt': %s"),
errbuf);
 +        } else if (net->backend.logFile) {
 +            virReportError(VIR_ERR_INTERNAL_ERROR,
 +                           _("Could not start 'passt': look into %s for
error"),
 +                           net->backend.logFile); 
 
 ...and this won't be needed either, with Laine's series for passt. It
 might actually be a bit misleading.
 
  +        } else {
 +            virReportError(VIR_ERR_INTERNAL_ERROR,
 +                           _("Could not start 'passt': exit status =
'%d'"),
 +                           exitstatus);
 +        }
 +
          goto error;
      }
   
 
 So all in all I think this is unnecessary, but also kind of harmless.
  
Except those patches are not merged yet. And as we are getting close to
the release I'd like to make this work with what we have now. We've been
burned plenty of times with QEMU to learn our lesson. We've merged
patches that were based not on QEMU's git, but some patches on top
thinking - yeah, the API won't change. And then it did.

Now we don't require a release (which would be ideal), but at least for
patches to be merged. When they get merged then yeah, this can be reworked.

Michal

On 2/16/23 11:27 AM, Michal Prívozník wrote:
...
  On 2/16/23 17:07, Stefano Brivio wrote:
  On Thu, 16 Feb 2023 14:32:50 +0100
 Michal Privoznik <mprivozn(a)redhat.com> wrote:

  Passt has '--stderr' argument which
makes it report error onto
 stderr rather to system log. Unfortunately, it's currently
 impossible to use both '--log-file' and '--stderr', so pass the
 latter only if the former isn't passed. Then, use the stderr to
 produce more user friendly error message on failed start.

 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
 ---
   src/qemu/qemu_passt.c | 22 +++++++++++++++++++---
   1 file changed, 19 insertions(+), 3 deletions(-)

 diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
 index c082c149cd..881205449b 100644
 --- a/src/qemu/qemu_passt.c
 +++ b/src/qemu/qemu_passt.c
 @@ -171,8 +171,13 @@ qemuPasstStart(virDomainObj *vm,
       if (net->sourceDev)
           virCommandAddArgList(cmd, "--interface", net->sourceDev, NULL);
   
 -    if (net->backend.logFile)
 +    if (net->backend.logFile) {
           virCommandAddArgList(cmd, "--log-file", net->backend.logFile,
NULL);
 +    } else {
 +        /* By default, passt logs into system logger. But we are interested
 +         * into errors too. Make it print errors onto stderr. */
 +        virCommandAddArg(cmd, "--stderr");
 +    }
 
 There's no need for this, see my previous email, archived at:
    https://listman.redhat.com/archives/libvir-list/2023-February/237880.html

    
       /* Add IP address info */
       for (i = 0; i < net->guestIP.nips; i++) {
 @@ -265,8 +270,19 @@ qemuPasstStart(virDomainObj *vm,
           goto error;
   
       if (cmdret < 0 || exitstatus != 0) {
 -        virReportError(VIR_ERR_INTERNAL_ERROR,
 -                       _("Could not start 'passt': %s"),
NULLSTR(errbuf));
 +        if (STRNEQ_NULLABLE(errbuf, "")) {
 +            virReportError(VIR_ERR_INTERNAL_ERROR,
 +                           _("Could not start 'passt': %s"),
errbuf);
 +        } else if (net->backend.logFile) {
 +            virReportError(VIR_ERR_INTERNAL_ERROR,
 +                           _("Could not start 'passt': look into %s for
error"),
 +                           net->backend.logFile);
 
 ...and this won't be needed either, with Laine's series for passt. It
 might actually be a bit misleading.

  +        } else {
 +            virReportError(VIR_ERR_INTERNAL_ERROR,
 +                           _("Could not start 'passt': exit status =
'%d'"),
 +                           exitstatus);
 +        }
 +
           goto error;
       }
   
 
 So all in all I think this is unnecessary, but also kind of harmless.

 
 
 Except those patches are not merged yet. And as we are getting close to
 the release I'd like to make this work with what we have now. We've been
 burned plenty of times with QEMU to learn our lesson. We've merged
 patches that were based not on QEMU's git, but some patches on top
 thinking - yeah, the API won't change. And then it did.
 
The patches are in passt, not QEMU, and Stefano will be cutting a new 
passt release "within a day or two".

...
  
 Now we don't require a release (which would be ideal), but at least for
 patches to be merged. When they get merged then yeah, this can be reworked.
 

On 2/16/23 17:07, Stefano Brivio wrote:
...
  On Thu, 16 Feb 2023 14:32:51 +0100
 Michal Privoznik <mprivozn(a)redhat.com> wrote:
 
  There are two places where we kill passt:

 1) qemuPasstStop() - called transitively from qemuProcessStop(),
 2) qemuPasstStart() - after failed start.

 Now, the code from 2) lack error preservation (so if there's
 another error during cleanup we might overwrite the original
 error). Therefore, move the internals of qemuPasstStop() into a
 separate function and call it from both places.

 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
 ---
  src/qemu/qemu_passt.c | 23 +++++++++++++----------
  1 file changed, 13 insertions(+), 10 deletions(-)

 diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
 index 881205449b..a4cc9e7166 100644
 --- a/src/qemu/qemu_passt.c
 +++ b/src/qemu/qemu_passt.c
 @@ -102,11 +102,9 @@ qemuPasstAddNetProps(virDomainObj *vm,
  }
  
  
 -void
 -qemuPasstStop(virDomainObj *vm,
 -              virDomainNetDef *net)
 +static void
 +qemuPasstKill(const char *pidfile) 
 
 A minor comment, should you respin: I think it should be made clear that
 this is not the expected/normal way in which passt will terminate --
 here or in the next patch. Removing the PID file is nice, but that's
 (usually) about it. 

I can adjust the commit message.

...
  
   {
 -    g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
      virErrorPtr orig_err;
  
      virErrorPreserveLast(&orig_err);
 @@ -118,6 +116,16 @@ qemuPasstStop(virDomainObj *vm,
  }
  
  
 +void
 +qemuPasstStop(virDomainObj *vm,
 +              virDomainNetDef *net)
 +{
 +    g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
 +
 +    qemuPasstKill(pidfile);
 +}
 +
 +
  int
  qemuPasstSetupCgroup(virDomainObj *vm,
                       virDomainNetDef *net,
 @@ -147,7 +155,6 @@ qemuPasstStart(virDomainObj *vm,
      g_autofree char *errbuf = NULL;
      char macaddr[VIR_MAC_STRING_BUFLEN];
      size_t i;
 -    pid_t pid = (pid_t) -1;
      int exitstatus = 0;
      int cmdret = 0;
  
 @@ -289,10 +296,6 @@ qemuPasstStart(virDomainObj *vm,
      return 0;
  
   error:
 -    ignore_value(virPidFileReadPathIfLocked(pidfile, &pid));
 -    if (pid != -1)
 -        virProcessKillPainfully(pid, true);
 -    unlink(pidfile);
 -
 +    qemuPasstKill(pidfile); 
 
 ...what takes care of terminating passt in case qemu doesn't start, now?
 The fact that the process is in the cgroup, right? 
No, it's qemuPasstKill(). Starting a guest is done in
qemuProcessStart(). In here, qemuProcessLaunch() ->
qemuExtDevicesStart() -> qemuPasstStart() is called. Now, in the top
most parent (qemuProcessStart()) - you can see the 'stop' label in which
qemuProcessStop() -> qemuExtDevicesStop() -> qemuPasstStop() is called.

We could go the extra step and use either virCgroupKillRecursive() or
virCgroupKillPainfully() to kill all processes within the CGroup. BUT,
the driver runs on different OSes than Linux (e.g. FreeBSD) or on Linux
with no CGroups.

Michal

On 2/16/23 8:32 AM, Michal Privoznik wrote:
...
  There are two places where we kill passt:
 
 1) qemuPasstStop() - called transitively from qemuProcessStop(),
 2) qemuPasstStart() - after failed start.
 
 Now, the code from 2) lack error preservation (so if there's
 another error during cleanup we might overwrite the original
 error). Therefore, move the internals of qemuPasstStop() into a
 separate function and call it from both places.
 
 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> 
Reviewed-by: Laine Stump <laine(a)redhat.com>

...
  ---
   src/qemu/qemu_passt.c | 23 +++++++++++++----------
   1 file changed, 13 insertions(+), 10 deletions(-)
 
 diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
 index 881205449b..a4cc9e7166 100644
 --- a/src/qemu/qemu_passt.c
 +++ b/src/qemu/qemu_passt.c
 @@ -102,11 +102,9 @@ qemuPasstAddNetProps(virDomainObj *vm,
   }
   
   
 -void
 -qemuPasstStop(virDomainObj *vm,
 -              virDomainNetDef *net)
 +static void
 +qemuPasstKill(const char *pidfile)
   {
 -    g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
       virErrorPtr orig_err;
   
       virErrorPreserveLast(&orig_err);
 @@ -118,6 +116,16 @@ qemuPasstStop(virDomainObj *vm,
   }
   
   
 +void
 +qemuPasstStop(virDomainObj *vm,
 +              virDomainNetDef *net)
 +{
 +    g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
 +
 +    qemuPasstKill(pidfile);
 +}
 +
 +
   int
   qemuPasstSetupCgroup(virDomainObj *vm,
                        virDomainNetDef *net,
 @@ -147,7 +155,6 @@ qemuPasstStart(virDomainObj *vm,
       g_autofree char *errbuf = NULL;
       char macaddr[VIR_MAC_STRING_BUFLEN];
       size_t i;
 -    pid_t pid = (pid_t) -1;
       int exitstatus = 0;
       int cmdret = 0;
   
 @@ -289,10 +296,6 @@ qemuPasstStart(virDomainObj *vm,
       return 0;
   
    error:
 -    ignore_value(virPidFileReadPathIfLocked(pidfile, &pid));
 -    if (pid != -1)
 -        virProcessKillPainfully(pid, true);
 -    unlink(pidfile);
 -
 +    qemuPasstKill(pidfile);
       return -1;
   } 

On 2/16/23 8:32 AM, Michal Privoznik wrote:
...
  The way we start passt currently is: we use
 virCommandSetPidFile() to use our virCommand machinery to acquire
 the PID file and leak opened FD into passt. Then, we use
 virPidFile*() APIs to read the PID file (which is needed when
 placing it into CGroups or killing it). But this does not fly
 really because passt daemonizes itself. Thus the process we
 started dies soon and thus the PID file is closed and unlocked.
 
 We could work around this by passing '--foreground' argument, but
 that weakens passt as it can't create new PID namespace (because
 it doesn't fork()).
 
 The solution is to let passt write the PID file, but since it
 does not lock the file and closes it as soon as it is written, we
 have to switch to those virPidFile APIs which don't expect PID
 file to be locked. 
So *this* is the part that was functionally wrong after my earlier patch 
was applied - I had switched to using an externally-generated pidfile, 
but was still using the APIs that should have only been used for 
pidfiles created by libvirt.

(You had mentioned some sort of cgroup issue last week. Did that solve 
itself? I never saw the problem, and passt has been starting/stopping 
fine for me all along (both before and after I changed the daemonizing) 
as long as selinux is disabled - still need to fix that).

...
  
 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> 
Reviewed-by: Laine Stump <laine(a)redhat.com>

...
  ---
   src/qemu/qemu_passt.c | 11 +++++++----
   1 file changed, 7 insertions(+), 4 deletions(-)
 
 diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
 index a4cc9e7166..47f4b5fcae 100644
 --- a/src/qemu/qemu_passt.c
 +++ b/src/qemu/qemu_passt.c
 @@ -72,7 +72,7 @@ qemuPasstGetPid(virDomainObj *vm,
   {
       g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
   
 -    return virPidFileReadPathIfLocked(pidfile, pid);
 +    return virPidFileReadPath(pidfile, pid);
   }
   
   
 @@ -106,11 +106,14 @@ static void
   qemuPasstKill(const char *pidfile)
   {
       virErrorPtr orig_err;
 +    pid_t pid = 0;
   
       virErrorPreserveLast(&orig_err);
   
 -    if (virPidFileForceCleanupPath(pidfile) < 0)
 -        VIR_WARN("Unable to kill passt process");
 +    ignore_value(virPidFileReadPath(pidfile, &pid));
 +    if (pid != 0)
 +        virProcessKillPainfully(pid, true);
 +    unlink(pidfile);
   
       virErrorRestore(&orig_err);
   }
 @@ -161,13 +164,13 @@ qemuPasstStart(virDomainObj *vm,
       cmd = virCommandNew(PASST);
   
       virCommandClearCaps(cmd);
 -    virCommandSetPidFile(cmd, pidfile);
       virCommandSetErrorBuffer(cmd, &errbuf);
   
       virCommandAddArgList(cmd,
                            "--one-off",
                            "--socket", passtSocketName,
                            "--mac-addr", virMacAddrFormat(&net->mac,
macaddr),
 +                         "--pid", pidfile,
                            NULL);
   
       if (net->mtu) { 

On 2/16/23 17:35, Laine Stump wrote:
...
  On 2/16/23 8:32 AM, Michal Privoznik wrote:
  This is a v2 of:

 https://listman.redhat.com/archives/libvir-list/2023-February/237731.html

 diff to v1:
 - Merged patches that were ACKed in v1,
 - Dropped 4/4 from the original series (the one that sets --foreground),
    and implemented a different approach

 Michal Prívozník (5):
    qemu_passt: Avoid double daemonizing passt
    qemu_passt: Report passt's error on failed start
    qemu_passt: Make passt report errors to stderr whenever possible
    qemu_passt: Deduplicate passt killing code
    qemu_passt: Let passt write the PID file
 
 
 This is everything that was in the patch I sent last week, with the
 following additions
 
 1) adding NULLSTR() around the reference to errbuf in patch 2/5
 
 2) adding "--stderr" to the commandline in patch 3/5 (which I found to
 be unnecessary in my testing - as Stefano says everything goes to stderr
 until passt has completed its init anyway)
 
 3) the other bit of patch 3/5 which adds an extra message telling the
 user to look into the designated logfile for the error - this is
 unnecessary (and actually now counter-productive, as it forces you to
 look elsewhere for the error when you wouldn't have needed to) because
 of patches I've sent to passt.
 
 4)  patch 4/5 that is a cleanup de-duplicating code
 
 5) patch 5 changes additional code (that I didn't touch in my patch) to
 use virPidFileReadPath() instead of virPidFileReadPathIfLocked(), and
 virProcessKillPainfully() instead of the higher level
 virPidFileForceCleanupPath().
 
 So it all seems fine (except the error reporting stuff), but why revert
 a patch only to push back the same changes in a deconstructed fashion
 plus some fixups, rather than just posting a followup or two?
 
Yeah, I realized this too and I'm sorry. My original intention was to
fix this in a completely different way (as my last patch from v1
demonstrates) and that was incompatible with yours.

Michal