bhyve: process: Log error message when kill fails

Message ID 7a23c4e794c0df4bd1c7b7aabf61f5985ff56396.1458340818.git.crobinso@redhat.com
State New
Headers show

Commit Message

Cole Robinson March 18, 2016, 10:41 p.m.
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(-)

-- 
2.5.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Cole Robinson March 19, 2016, 7:07 p.m. | #1
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

Patch hide | download patch | download mbox

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);