| Message ID | 7a23c4e794c0df4bd1c7b7aabf61f5985ff56396.1458340818.git.crobinso@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/19/2016 03:04 PM, Roman Bogorodskiy wrote: > 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()); > + } > + } > Ah, sorry for not looking close enough at KillPainfully return codes. That sounds good to me, I was mostly just looking for candidates to convert to virGetLastErrorMessage() :) Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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);