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

[Cole Robinson]

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