[libvirt] [PATCH] vbox: network: make sure driver not be NULL in virRegisterNetworkDriver
libvirtd will report below error if does not make sure driver not be NULL in virRegisterNetworkDriver $ libvirtd 2014-10-24 09:24:36.443+: 28876: info : libvirt version: 1.2.10 2014-10-24 09:24:36.443+: 28876: error : virRegisterNetworkDriver:549 : driver in virRegisterNetworkDriver must not be NULL 2014-10-24 09:24:36.443+: 28876: error : virDriverLoadModule:99 : Failed module registration vboxNetworkRegister Signed-off-by: Shanzhi Yu --- 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)) return -1; return 0; } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vbox: network: make sure driver not be NULL in virRegisterNetworkDriver
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+: 28876: info : libvirt version: 1.2.10 > 2014-10-24 09:24:36.443+: 28876: error : virRegisterNetworkDriver:549 : > driver in virRegisterNetworkDriver must not be NULL > 2014-10-24 09:24:36.443+: 28876: error : virDriverLoadModule:99 : Failed > module registration vboxNetworkRegister > > Signed-off-by: Shanzhi Yu > --- > 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) 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. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vbox: network: make sure driver not be NULL in virRegisterNetworkDriver
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+: 28876: info : libvirt version: 1.2.10 > > 2014-10-24 09:24:36.443+: 28876: error : virRegisterNetworkDriver:549 : > > driver in virRegisterNetworkDriver must not be NULL > > 2014-10-24 09:24:36.443+: 28876: error : virDriverLoadModule:99 : > > Failed module registration vboxNetworkRegister > > > > Signed-off-by: Shanzhi Yu > > --- > > 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vbox: network: make sure driver not be NULL in virRegisterNetworkDriver
On 10/24/2014 10:52 PM, Daniel P. Berrange wrote: 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+: 28876: info : libvirt version: 1.2.10 2014-10-24 09:24:36.443+: 28876: error : virRegisterNetworkDriver:549 : driver in virRegisterNetworkDriver must not be NULL 2014-10-24 09:24:36.443+: 28876: error : virDriverLoadModule:99 : Failed module registration vboxNetworkRegister Signed-off-by: Shanzhi Yu --- 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. Thanks for your review. I mean the && not ||. 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 -- Regards shyu -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vbox: network: make sure driver not be NULL in virRegisterNetworkDriver
On 10/24/2014 10:17 AM, Shanzhi Yu wrote: > >>> >>> 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. > > Thanks for your review. I mean the && not ||. > >> 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. Pushed with some minor grammar tweaks to the commit message. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
