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

Cole Robinson wrote: 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()); + } + } PS Actually, bhyve at some point (or even from the start) started to support triggering ACPI shutdown by sending SIGTERM. So I think I'll actually need to implement domainShutdown that only sends SIGTERM and drop a warning when virProcessKillPainfully() returns 1 in domainDestroy as it's sort of implied by design. Roman Bogorodskiy