Re: [libvirt] [PATCH] bhyve: process: Log error message when kill fails

Cole Robinson wrote: > virProcessKillPainfully will raise a libvirt error, so log it. We > can drop the manual pid logging since ProcessKill always reports > the pid in its error message. > --- > src/bhyve/bhyve_process.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c > index 14588a9..f2ec712 100644 > --- a/src/bhyve/bhyve_process.c > +++ b/src/bhyve/bhyve_process.c > @@ -283,9 +283,8 @@ virBhyveProcessStop(bhyveConnPtr driver, > > /* First, try to kill 'bhyve' process */ > if (virProcessKillPainfully(vm->pid, true) != 0) > - VIR_WARN("Failed to gracefully stop bhyve VM '%s' (pid: %d)", > - vm->def->name, > - (int)vm->pid); > + VIR_WARN("Failed to gracefully stop bhyve VM '%s': %s", > + vm->def->name, virGetLastErrorMessage()); > > /* Cleanup network interfaces */ > bhyveNetCleanup(vm); If we want to call virGetLastErrorMessage() here, the check should be a little more complex because virProcessKillPainfully() could return 1 if it failed to kill with SIGTERM and killed with SIGKILL and in this case it doesn't set error. What do you think about this bit? diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index f2ec712..133589a 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -263,6 +263,7 @@ virBhyveProcessStop(bhyveConnPtr driver, virDomainShutoffReason reason) { int ret = -1; + int kill_ret = -1; virCommandPtr cmd = NULL; bhyveDomainObjPrivatePtr priv = vm->privateData; @@ -282,9 +283,17 @@ virBhyveProcessStop(bhyveConnPtr driver, bhyveMonitorClose(priv->mon); /* First, try to kill 'bhyve' process */ - if (virProcessKillPainfully(vm->pid, true) != 0) - VIR_WARN("Failed to gracefully stop bhyve VM '%s': %s", - vm->def->name, virGetLastErrorMessage()); + kill_ret = virProcessKillPainfully(vm->pid, true); + if (kill_ret != 0) { + if (kill_ret == 1) { + VIR_WARN("Failed to gracefully stop bhyve VM '%s' (pid: %d)", + vm->def->name, + (int)vm->pid); + } else { + VIR_WARN("Failed to kill bhyve process for VM '%s': %s", + vm->def->name, virGetLastErrorMessage()); + } + }