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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list