[libvirt] [PATCH] virsh: report errors in virshInit()

Daniel P. Berrange posted 1 patch 7 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170419104306.11644-1-berrange@redhat.com
tools/virsh.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
[libvirt] [PATCH] virsh: report errors in virshInit()
Posted by Daniel P. Berrange 7 years ago
There are several functions in virshInit which can fail, especially
when running win32 builds under WINE. Currently virsh just exits
without reporting what error happened.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 tools/virsh.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 31e23bd..90f8125 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -358,16 +358,22 @@ virshInit(vshControl *ctl)
     /* set up the library error handler */
     virSetErrorFunc(NULL, vshErrorHandler);
 
-    if (virEventRegisterDefaultImpl() < 0)
+    if (virEventRegisterDefaultImpl() < 0) {
+        vshReportError(ctl);
         return false;
+    }
 
-    if (virThreadCreate(&ctl->eventLoop, true, vshEventLoop, ctl) < 0)
+    if (virThreadCreate(&ctl->eventLoop, true, vshEventLoop, ctl) < 0) {
+        vshReportError(ctl);
         return false;
+    }
     ctl->eventLoopStarted = true;
 
     if ((ctl->eventTimerId = virEventAddTimeout(-1, vshEventTimeout, ctl,
-                                                NULL)) < 0)
+                                                NULL)) < 0) {
+        vshReportError(ctl);
         return false;
+    }
 
     if (ctl->connname) {
         /* Connecting to a named connection must succeed, but we delay
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: report errors in virshInit()
Posted by John Ferlan 7 years ago

On 04/19/2017 06:43 AM, Daniel P. Berrange wrote:
> There are several functions in virshInit which can fail, especially
> when running win32 builds under WINE. Currently virsh just exits
> without reporting what error happened.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  tools/virsh.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 

Why not a "goto error;" instead where error: does the
vshReportError(ctl); return false;

ACK either way

John

> diff --git a/tools/virsh.c b/tools/virsh.c
> index 31e23bd..90f8125 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -358,16 +358,22 @@ virshInit(vshControl *ctl)
>      /* set up the library error handler */
>      virSetErrorFunc(NULL, vshErrorHandler);
>  
> -    if (virEventRegisterDefaultImpl() < 0)
> +    if (virEventRegisterDefaultImpl() < 0) {
> +        vshReportError(ctl);
>          return false;
> +    }
>  
> -    if (virThreadCreate(&ctl->eventLoop, true, vshEventLoop, ctl) < 0)
> +    if (virThreadCreate(&ctl->eventLoop, true, vshEventLoop, ctl) < 0) {
> +        vshReportError(ctl);
>          return false;
> +    }
>      ctl->eventLoopStarted = true;
>  
>      if ((ctl->eventTimerId = virEventAddTimeout(-1, vshEventTimeout, ctl,
> -                                                NULL)) < 0)
> +                                                NULL)) < 0) {
> +        vshReportError(ctl);
>          return false;
> +    }
>  
>      if (ctl->connname) {
>          /* Connecting to a named connection must succeed, but we delay
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: report errors in virshInit()
Posted by Daniel P. Berrange 7 years ago
On Wed, Apr 19, 2017 at 08:10:22PM -0400, John Ferlan wrote:
> 
> 
> On 04/19/2017 06:43 AM, Daniel P. Berrange wrote:
> > There are several functions in virshInit which can fail, especially
> > when running win32 builds under WINE. Currently virsh just exits
> > without reporting what error happened.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  tools/virsh.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> 
> Why not a "goto error;" instead where error: does the
> vshReportError(ctl); return false;
> 
> ACK either way

I realize I accidentally pushed this patch already yesterday when
pushing the RBD build breaker fix.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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