1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] conf: avoid reporting errors when network driver is disabled

Daniel P. Berrangé posted 1 patch 6 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180323114626.24883-1-berrange@redhat.com
Test syntax-check failed
src/conf/domain_conf.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
[libvirt] [PATCH] conf: avoid reporting errors when network driver is disabled
Posted by Daniel P. Berrangé 6 years ago 
In previous releases all these methods were a no-op if the network
driver is disabled. These helper methods are called unconditionally for
all types of network interface, so must be no-ops if missing. Other code
will already generate an error if the network driver is disabled and a
NIC with type=network is used.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/conf/domain_conf.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c8d051fa9f..79d6bd378e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28979,10 +28979,13 @@ int
 virDomainNetAllocateActualDevice(virDomainDefPtr dom,
                                  virDomainNetDefPtr iface)
 {
+    /* Just silently ignore if network driver isn't present. If something
+     * has tried to use a NIC with type=network, other code will already
+     * cause an error. This ensures type=bridge doesn't break when
+     * network driver is compiled out.
+     */
     if (!netAllocate) {
-        virReportError(VIR_ERR_NO_SUPPORT, "%s",
-                       _("Network device allocation not available"));
-        return -1;
+        return 0;
     }
 
     return netAllocate(dom, iface);
@@ -28993,8 +28996,6 @@ virDomainNetNotifyActualDevice(virDomainDefPtr dom,
                                virDomainNetDefPtr iface)
 {
     if (!netNotify) {
-        virReportError(VIR_ERR_NO_SUPPORT, "%s",
-                       _("Network device notification not available"));
         return;
     }
 
@@ -29007,9 +29008,7 @@ virDomainNetReleaseActualDevice(virDomainDefPtr dom,
                                 virDomainNetDefPtr iface)
 {
     if (!netRelease) {
-        virReportError(VIR_ERR_NO_SUPPORT, "%s",
-                       _("Network device release not available"));
-        return -1;
+        return 0;
     }
 
     return netRelease(dom, iface);
@@ -29020,9 +29019,7 @@ virDomainNetBandwidthChangeAllowed(virDomainNetDefPtr iface,
                                    virNetDevBandwidthPtr newBandwidth)
 {
     if (!netBandwidthChangeAllowed) {
-        virReportError(VIR_ERR_NO_SUPPORT, "%s",
-                       _("Network device bandwidth change query not available"));
-        return -1;
+        return 0;
     }
 
     return netBandwidthChangeAllowed(iface, newBandwidth);
@@ -29033,9 +29030,7 @@ virDomainNetBandwidthUpdate(virDomainNetDefPtr iface,
                             virNetDevBandwidthPtr newBandwidth)
 {
     if (!netBandwidthUpdate) {
-        virReportError(VIR_ERR_NO_SUPPORT, "%s",
-                       _("Network device bandwidth update not available"));
-        return -1;
+        return 0;
     }
 
     return netBandwidthUpdate(iface, newBandwidth);
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: avoid reporting errors when network driver is disabled
Posted by Laine Stump 6 years ago 
On 03/23/2018 07:46 AM, Daniel P. Berrangé wrote:
> In previous releases all these methods were a no-op if the network
> driver is disabled. These helper methods are called unconditionally for
> all types of network interface, so must be no-ops if missing. Other code
> will already generate an error if the network driver is disabled and a
> NIC with type=network is used.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: Laine Stump <laine@laine.org>

I've been thinking about this the last few hours. The end-game of making
the network driver available separately is to have all of the
functionality for setting up network interfaces be in the network
driver, including creation of tap devices and attaching them to bridges,
adding iptables rules, setting QoS. Once this is done, it will no longer
be possible to build with --without-network and have a functionally
useful libvirtd. Either we will have to redefine what
"--without-network" means (so that it just applies to the "virtual
networks" part of the network driver), or remove that option.

> ---
>  src/conf/domain_conf.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c8d051fa9f..79d6bd378e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -28979,10 +28979,13 @@ int
>  virDomainNetAllocateActualDevice(virDomainDefPtr dom,
>                                   virDomainNetDefPtr iface)
>  {
> +    /* Just silently ignore if network driver isn't present. If something
> +     * has tried to use a NIC with type=network, other code will already
> +     * cause an error. This ensures type=bridge doesn't break when
> +     * network driver is compiled out.
> +     */
>      if (!netAllocate) {
> -        virReportError(VIR_ERR_NO_SUPPORT, "%s",
> -                       _("Network device allocation not available"));
> -        return -1;
> +        return 0;
>      }
>  
>      return netAllocate(dom, iface);
> @@ -28993,8 +28996,6 @@ virDomainNetNotifyActualDevice(virDomainDefPtr dom,
>                                 virDomainNetDefPtr iface)
>  {
>      if (!netNotify) {
> -        virReportError(VIR_ERR_NO_SUPPORT, "%s",
> -                       _("Network device notification not available"));
>          return;
>      }
>  
> @@ -29007,9 +29008,7 @@ virDomainNetReleaseActualDevice(virDomainDefPtr dom,
>                                  virDomainNetDefPtr iface)
>  {
>      if (!netRelease) {
> -        virReportError(VIR_ERR_NO_SUPPORT, "%s",
> -                       _("Network device release not available"));
> -        return -1;
> +        return 0;
>      }
>  
>      return netRelease(dom, iface);
> @@ -29020,9 +29019,7 @@ virDomainNetBandwidthChangeAllowed(virDomainNetDefPtr iface,
>                                     virNetDevBandwidthPtr newBandwidth)
>  {
>      if (!netBandwidthChangeAllowed) {
> -        virReportError(VIR_ERR_NO_SUPPORT, "%s",
> -                       _("Network device bandwidth change query not available"));
> -        return -1;
> +        return 0;
>      }
>  
>      return netBandwidthChangeAllowed(iface, newBandwidth);
> @@ -29033,9 +29030,7 @@ virDomainNetBandwidthUpdate(virDomainNetDefPtr iface,
>                              virNetDevBandwidthPtr newBandwidth)
>  {
>      if (!netBandwidthUpdate) {
> -        virReportError(VIR_ERR_NO_SUPPORT, "%s",
> -                       _("Network device bandwidth update not available"));
> -        return -1;
> +        return 0;
>      }
>  
>      return netBandwidthUpdate(iface, newBandwidth);


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: avoid reporting errors when network driver is disabled
Posted by Daniel P. Berrangé 6 years ago 
On Fri, Mar 23, 2018 at 11:27:37AM -0400, Laine Stump wrote:
> On 03/23/2018 07:46 AM, Daniel P. Berrangé wrote:
> > In previous releases all these methods were a no-op if the network
> > driver is disabled. These helper methods are called unconditionally for
> > all types of network interface, so must be no-ops if missing. Other code
> > will already generate an error if the network driver is disabled and a
> > NIC with type=network is used.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Reviewed-by: Laine Stump <laine@laine.org>
> 
> I've been thinking about this the last few hours. The end-game of making
> the network driver available separately is to have all of the
> functionality for setting up network interfaces be in the network
> driver, including creation of tap devices and attaching them to bridges,
> adding iptables rules, setting QoS. Once this is done, it will no longer
> be possible to build with --without-network and have a functionally
> useful libvirtd. Either we will have to redefine what
> "--without-network" means (so that it just applies to the "virtual
> networks" part of the network driver), or remove that option.

NB, I had only been anticipating using virtnetworkd for TAP device setup
from the unprivileged libvirtd, no change to privileged libvirtd. Having
said that though, is would be compelling to only have 1 codepath to care
about. So if we do what you mention here, I think we would make sure that
--without-network just disables the virtual network functionality.


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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.