[libvirt] [PATCH] conf: avoid reporting errors when network driver is disabled
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
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);
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 :|
participants (2)
-
Daniel P. Berrangé -
Laine Stump