Re: [PATCH] vbox: network: make sure driver not be NULL in virRegisterNetworkDriver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 24, 2014 at 08:47:47AM -0600, Eric Blake wrote:
> On 10/24/2014 03:31 AM, Shanzhi Yu wrote:
> > libvirtd will report below error if does not make sure driver not be NULL
> > in virRegisterNetworkDriver
> > 
> > $ libvirtd
> > 2014-10-24 09:24:36.443+0000: 28876: info : libvirt version: 1.2.10
> > 2014-10-24 09:24:36.443+0000: 28876: error : virRegisterNetworkDriver:549 : driver in virRegisterNetworkDriver must not be NULL
> > 2014-10-24 09:24:36.443+0000: 28876: error : virDriverLoadModule:99 : Failed module registration vboxNetworkRegister
> > 
> > Signed-off-by: Shanzhi Yu <shyu@xxxxxxxxxx>
> > ---
> >  src/vbox/vbox_driver.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/vbox/vbox_driver.c b/src/vbox/vbox_driver.c
> > index 743a488..ff69069 100644
> > --- a/src/vbox/vbox_driver.c
> > +++ b/src/vbox/vbox_driver.c
> > @@ -152,7 +152,7 @@ int vboxNetworkRegister(void)
> >      if (VBoxCGlueInit(&uVersion) == 0)
> >          networkDriver = vboxGetNetworkDriver(uVersion);
> >  
> > -    if (virRegisterNetworkDriver(networkDriver) < 0)
> > +    if ((networkDriver != NULL) && (virRegisterNetworkDriver(networkDriver) < 0)) 
> 
> Over-parenthesized.  Sufficient to write:
> 
> if (networkDriver && virRegisterNetworkDriver(networkDriver) < 0)

ACK to this.

> 
> Or did you botch the logic, and really mean:
> 
> if (!networkDriver || virRegisterNetworkDriver(networkDriver) < 0)
> 
> Furthermore, Dan's recent patch series will probably overhaul all of
> this anyways, so it may be easier to just wait for his patches to land.

No, the logic in the patch is correct. We don't want an error if the
networkDriver is NULL. We just want to continue running without
any error in that case.

We should push this now because we'll want to cherry pick it to stable
branches.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]