[libvirt] [PATCH 0/7] net/qemu: move vlan/bandwidth validation out of network driver
As usual, the first 6 patches are just torquing stuff around to make the last patch simple (except that patch 1 makes interface validation errors more useful). 2-5 are just converting a bunch of accessor functions to tak/return const pointers, since that's what's available in the places where I wanted to use them. This does actually fix a documented bug: https://bugzilla.redhat.com/1741121 (that one is about vlan tag validation. There may also be one about bandwidth, but I didn't see it right away, so I gave up, as is my custom). Laine Stump (7): qemu: add mac address to error messages in qemuDomainValidateActualNetDef conf: make virDomainNetGetActualVlan arg/return val const conf: make virDomainNetGetActualBandwidth arg/return value const conf: return a const from virDomainNetGetActualVirtPortProfile conf: change args/return values of remaining virDomainNetGetActual*() to const conf: add hypervisor agnostic, domain start-time, validation function for NetDef net/qemu: move vlan/bandwidth validation out of network driver src/conf/domain_conf.c | 78 ++++++++++++++++++++++++---- src/conf/domain_conf.h | 21 ++++---- src/conf/netdev_bandwidth_conf.c | 2 +- src/conf/netdev_bandwidth_conf.h | 2 +- src/conf/netdev_vport_profile_conf.c | 2 +- src/conf/netdev_vport_profile_conf.h | 2 +- src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 6 +-- src/libxl/libxl_domain.c | 4 ++ src/libxl/libxl_driver.c | 4 ++ src/libxl/xen_common.c | 4 +- src/lxc/lxc_driver.c | 8 ++- src/lxc/lxc_process.c | 16 +++--- src/network/bridge_driver.c | 37 ------------- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 35 ++++++++----- src/qemu/qemu_hotplug.c | 6 +-- src/qemu/qemu_migration_cookie.c | 2 +- src/qemu/qemu_process.c | 2 +- src/util/virhostdev.c | 8 +-- src/util/virnetdev.c | 4 +- src/util/virnetdev.h | 2 +- src/util/virnetdevbandwidth.c | 6 +-- src/util/virnetdevbandwidth.h | 4 +- src/util/virnetdevmacvlan.c | 20 +++---- src/util/virnetdevmacvlan.h | 10 ++-- src/util/virnetdevmidonet.c | 4 +- src/util/virnetdevmidonet.h | 4 +- src/util/virnetdevopenvswitch.c | 8 +-- src/util/virnetdevopenvswitch.h | 6 +-- src/util/virnetdevtap.c | 12 ++--- src/util/virnetdevtap.h | 12 ++--- src/util/virnetdevvportprofile.c | 12 ++--- src/util/virnetdevvportprofile.h | 12 ++--- tests/bhyvexml2argvmock.c | 4 +- 35 files changed, 206 insertions(+), 156 deletions(-) -- 2.21.0
This makes it easier to understand which interface's config caused the error. Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_domain.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3e8da13794..b2e8759da7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5384,8 +5384,11 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net, * time), but only if the validations would SUCCEED for * type='network'.) */ + char macstr[VIR_MAC_STRING_BUFLEN]; virDomainNetType actualType = virDomainNetGetActualType(net); + virMacAddrFormat(&net->mac, macstr); + /* Only tap/macvtap devices support multiqueue. */ if (net->driver.virtio.queues > 0) { @@ -5395,17 +5398,18 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net, actualType == VIR_DOMAIN_NET_TYPE_ETHERNET || actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("multiqueue network is not supported for: %s"), - virDomainNetTypeToString(actualType)); + _("interface %s - multiqueue network is not supported for: %s"), + macstr, virDomainNetTypeToString(actualType)); return -1; } if (net->driver.virtio.queues > 1 && actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("multiqueue network is not supported for vhost-user " - "with this QEMU binary")); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("interface %s - multiqueue network is " + "not supported for vhost-user with this QEMU binary"), + macstr); return -1; } } @@ -5417,21 +5421,22 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net, */ if (net->filter) { virNetDevVPortProfilePtr vport = virDomainNetGetActualVirtPortProfile(net); + if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("filterref is not supported for " + _("interface %s - filterref is not supported for " "network interfaces of type %s"), - virDomainNetTypeToString(actualType)); + macstr, virDomainNetTypeToString(actualType)); return -1; } if (vport && vport->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) { /* currently none of the defined virtualport types support iptables */ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("filterref is not supported for " + _("interface %s - filterref is not supported for " "network interfaces with virtualport type %s"), - virNetDevVPortTypeToString(vport->virtPortType)); + macstr, virNetDevVPortTypeToString(vport->virtPortType)); return -1; } } @@ -5441,8 +5446,8 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net, actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Custom tap device path is not supported for: %s"), - virDomainNetTypeToString(actualType)); + _("interface %s - custom tap device path is not supported for: %s"), + macstr, virDomainNetTypeToString(actualType)); return -1; } -- 2.21.0
This is needed if we want to call the function when the virDomainNetDef* we have is a const. Since virDomainNetGetActualVlan returns a pointer to memory that is within the virDomainNetDefPtr arg, the returned pointer must also be made const. This leads to a cascade of other virNetDevVlanPtr's that must be changed to "const virNetDevVlan *". Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 6 +++--- src/conf/domain_conf.h | 2 +- src/libxl/libxl_conf.c | 2 +- src/libxl/xen_common.c | 2 +- src/util/virhostdev.c | 2 +- src/util/virnetdev.c | 4 ++-- src/util/virnetdev.h | 2 +- src/util/virnetdevmacvlan.c | 4 ++-- src/util/virnetdevmacvlan.h | 2 +- src/util/virnetdevopenvswitch.c | 6 +++--- src/util/virnetdevopenvswitch.h | 4 ++-- src/util/virnetdevtap.c | 6 +++--- src/util/virnetdevtap.h | 6 +++--- tests/bhyvexml2argvmock.c | 2 +- 14 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 80e19a15df..c0122dfd07 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29710,10 +29710,10 @@ virDomainNetGetActualBandwidth(virDomainNetDefPtr iface) return iface->bandwidth; } -virNetDevVlanPtr -virDomainNetGetActualVlan(virDomainNetDefPtr iface) +const virNetDevVlan * +virDomainNetGetActualVlan(const virDomainNetDef *iface) { - virNetDevVlanPtr vlan = &iface->vlan; + const virNetDevVlan *vlan = &iface->vlan; /* if there is an ActualNetDef, *always* return * its vlan rather than the NetDef's vlan. diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a481389925..9ca3a5bf06 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3213,7 +3213,7 @@ virNetDevVPortProfilePtr virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface); virNetDevBandwidthPtr virDomainNetGetActualBandwidth(virDomainNetDefPtr iface); -virNetDevVlanPtr virDomainNetGetActualVlan(virDomainNetDefPtr iface); +const virNetDevVlan *virDomainNetGetActualVlan(const virDomainNetDef *iface); bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); const char *virDomainNetGetModelString(const virDomainNetDef *net); int virDomainNetSetModelString(virDomainNetDefPtr et, diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 3f08f0f710..89122826fc 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1218,7 +1218,7 @@ libxlMakeNic(virDomainDefPtr def, virConnectPtr conn = NULL; virNetDevBandwidthPtr actual_bw; virNetDevVPortProfilePtr port_profile; - virNetDevVlanPtr virt_vlan; + const virNetDevVlan *virt_vlan; virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i; const char *script = NULL; diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index cc9039a0cf..6e77921333 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -1681,7 +1681,7 @@ xenFormatNet(virConnectPtr conn, case VIR_DOMAIN_NET_TYPE_BRIDGE: { virNetDevVPortProfilePtr port_profile = virDomainNetGetActualVirtPortProfile(net); - virNetDevVlanPtr virt_vlan = virDomainNetGetActualVlan(net); + const virNetDevVlan *virt_vlan = virDomainNetGetActualVlan(net); const char *script = net->script; size_t i; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 010eb551a9..223a78bd34 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -501,7 +501,7 @@ virHostdevSetNetConfig(virDomainHostdevDefPtr hostdev, const unsigned char *uuid) { g_autofree char *linkdev = NULL; - virNetDevVlanPtr vlan; + const virNetDevVlan *vlan; virNetDevVPortProfilePtr virtPort; int vf = -1; bool port_profile_associate = true; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 5391f0030b..7adc69905d 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2178,7 +2178,7 @@ virNetDevReadNetConfig(const char *linkdev, int vf, int virNetDevSetNetConfig(const char *linkdev, int vf, const virMacAddr *adminMAC, - virNetDevVlanPtr vlan, + const virNetDevVlan *vlan, const virMacAddr *MAC, bool setVlan) { @@ -2401,7 +2401,7 @@ int virNetDevSetNetConfig(const char *linkdev G_GNUC_UNUSED, int vf G_GNUC_UNUSED, const virMacAddr *adminMAC G_GNUC_UNUSED, - virNetDevVlanPtr vlan G_GNUC_UNUSED, + const virNetDevVlan *vlan G_GNUC_UNUSED, const virMacAddr *MAC G_GNUC_UNUSED, bool setVlan G_GNUC_UNUSED) { diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index b5a5214bcb..24b41498ed 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -253,7 +253,7 @@ virNetDevReadNetConfig(const char *linkdev, int vf, int virNetDevSetNetConfig(const char *linkdev, int vf, const virMacAddr *adminMAC, - virNetDevVlanPtr vlan, + const virNetDevVlan *vlan, const virMacAddr *MAC, bool setVLan) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 2ccd83d1bb..c530cb75ed 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -895,7 +895,7 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested, const virMacAddr *macaddress, const char *linkdev, virNetDevMacVLanMode mode, - virNetDevVlanPtr vlan, + const virNetDevVlan *vlan, const unsigned char *vmuuid, virNetDevVPortProfilePtr virtPortProfile, char **ifnameResult, @@ -1224,7 +1224,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname G_GNUC_UNUSED, const virMacAddr *macaddress G_GNUC_UNUSED, const char *linkdev G_GNUC_UNUSED, virNetDevMacVLanMode mode G_GNUC_UNUSED, - virNetDevVlanPtr vlan G_GNUC_UNUSED, + const virNetDevVlan *vlan G_GNUC_UNUSED, const unsigned char *vmuuid G_GNUC_UNUSED, virNetDevVPortProfilePtr virtPortProfile G_GNUC_UNUSED, char **res_ifname G_GNUC_UNUSED, diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h index 30038a45b5..f455ec3384 100644 --- a/src/util/virnetdevmacvlan.h +++ b/src/util/virnetdevmacvlan.h @@ -76,7 +76,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname, const virMacAddr *macaddress, const char *linkdev, virNetDevMacVLanMode mode, - virNetDevVlanPtr vlan, + const virNetDevVlan *vlan, const unsigned char *vmuuid, virNetDevVPortProfilePtr virtPortProfile, char **res_ifname, diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 1d2bbf9dd7..9aa2c5ad84 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -68,7 +68,7 @@ virNetDevOpenvswitchAddTimeout(virCommandPtr cmd) * Returns 0 in case of success or -1 in case of failure. */ static int -virNetDevOpenvswitchConstructVlans(virCommandPtr cmd, virNetDevVlanPtr virtVlan) +virNetDevOpenvswitchConstructVlans(virCommandPtr cmd, const virNetDevVlan *virtVlan) { int ret = -1; size_t i = 0; @@ -136,7 +136,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, const virMacAddr *macaddr, const unsigned char *vmuuid, virNetDevVPortProfilePtr ovsport, - virNetDevVlanPtr virtVlan) + const virNetDevVlan *virtVlan) { char macaddrstr[VIR_MAC_STRING_BUFLEN]; char ifuuidstr[VIR_UUID_STRING_BUFLEN]; @@ -547,7 +547,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, * Returns 0 in case of success or -1 in case of failure. */ int virNetDevOpenvswitchUpdateVlan(const char *ifname, - virNetDevVlanPtr virtVlan) + const virNetDevVlan *virtVlan) { g_autoptr(virCommand) cmd = NULL; diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 2e15384b5f..dddb21a5a2 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -33,7 +33,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const virMacAddr *macaddr, const unsigned char *vmuuid, virNetDevVPortProfilePtr ovsport, - virNetDevVlanPtr virtVlan) + const virNetDevVlan *virtVlan) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) G_GNUC_WARN_UNUSED_RESULT; @@ -65,5 +65,5 @@ int virNetDevOpenvswitchGetVhostuserIfname(const char *path, ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT G_GNUC_NO_INLINE; int virNetDevOpenvswitchUpdateVlan(const char *ifname, - virNetDevVlanPtr virtVlan) + const virNetDevVlan *virtVlan) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index fe4f55f8fc..fa1626d6ba 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -503,7 +503,7 @@ virNetDevTapAttachBridge(const char *tapname, const virMacAddr *macaddr, const unsigned char *vmuuid, virNetDevVPortProfilePtr virtPortProfile, - virNetDevVlanPtr virtVlan, + const virNetDevVlan *virtVlan, unsigned int mtu, unsigned int *actualMTU) { @@ -575,7 +575,7 @@ virNetDevTapReattachBridge(const char *tapname, const virMacAddr *macaddr, const unsigned char *vmuuid, virNetDevVPortProfilePtr virtPortProfile, - virNetDevVlanPtr virtVlan, + const virNetDevVlan *virtVlan, unsigned int mtu, unsigned int *actualMTU) { @@ -661,7 +661,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, int *tapfd, size_t tapfdSize, virNetDevVPortProfilePtr virtPortProfile, - virNetDevVlanPtr virtVlan, + const virNetDevVlan *virtVlan, virNetDevCoalescePtr coalesce, unsigned int mtu, unsigned int *actualMTU, diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index cbc799bdbe..4fc734fd5e 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -64,7 +64,7 @@ virNetDevTapAttachBridge(const char *tapname, const virMacAddr *macaddr, const unsigned char *vmuuid, virNetDevVPortProfilePtr virtPortProfile, - virNetDevVlanPtr virtVlan, + const virNetDevVlan *virtVlan, unsigned int mtu, unsigned int *actualMTU) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) @@ -76,7 +76,7 @@ virNetDevTapReattachBridge(const char *tapname, const virMacAddr *macaddr, const unsigned char *vmuuid, virNetDevVPortProfilePtr virtPortProfile, - virNetDevVlanPtr virtVlan, + const virNetDevVlan *virtVlan, unsigned int mtu, unsigned int *actualMTU) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) @@ -90,7 +90,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, int *tapfd, size_t tapfdSize, virNetDevVPortProfilePtr virtPortProfile, - virNetDevVlanPtr virtVlan, + const virNetDevVlan *virtVlan, virNetDevCoalescePtr coalesce, unsigned int mtu, unsigned int *actualMTU, diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c index b0d38187f9..ac4ffe8596 100644 --- a/tests/bhyvexml2argvmock.c +++ b/tests/bhyvexml2argvmock.c @@ -27,7 +27,7 @@ int virNetDevTapCreateInBridgePort(const char *brname G_GNUC_UNUSED, int *tapfd G_GNUC_UNUSED, size_t tapfdSize G_GNUC_UNUSED, virNetDevVPortProfilePtr virtPortProfile G_GNUC_UNUSED, - virNetDevVlanPtr virtVlan G_GNUC_UNUSED, + const virNetDevVlan *virtVlan G_GNUC_UNUSED, virNetDevCoalescePtr coalesce G_GNUC_UNUSED, unsigned int mtu G_GNUC_UNUSED, unsigned int *actualMTU G_GNUC_UNUSED, -- 2.21.0
In this case, the virNetDevBandwidthPtr that is returned is not to a region within the virDomainNetDef arg, but points elsewhere (the NetDef has the pointer, not the entire object), so technically it's not necessary to make the return value a const, but it's a bit disingenuous to *not* do it. Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 4 ++-- src/conf/domain_conf.h | 4 ++-- src/conf/netdev_bandwidth_conf.c | 2 +- src/conf/netdev_bandwidth_conf.h | 2 +- src/libxl/libxl_conf.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/lxc/lxc_process.c | 4 ++-- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_hotplug.c | 4 ++-- src/util/virnetdevbandwidth.c | 6 +++--- src/util/virnetdevbandwidth.h | 4 ++-- 11 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c0122dfd07..76e036c48e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29698,8 +29698,8 @@ virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface) } } -virNetDevBandwidthPtr -virDomainNetGetActualBandwidth(virDomainNetDefPtr iface) +const virNetDevBandwidth * +virDomainNetGetActualBandwidth(const virDomainNetDef *iface) { /* if there is an ActualNetDef, *always* return * its bandwidth rather than the NetDef's bandwidth. diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9ca3a5bf06..44d5fb8e16 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3211,8 +3211,8 @@ int virDomainNetGetActualDirectMode(virDomainNetDefPtr iface); virDomainHostdevDefPtr virDomainNetGetActualHostdev(virDomainNetDefPtr iface); virNetDevVPortProfilePtr virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface); -virNetDevBandwidthPtr -virDomainNetGetActualBandwidth(virDomainNetDefPtr iface); +const virNetDevBandwidth * +virDomainNetGetActualBandwidth(const virDomainNetDef *iface); const virNetDevVlan *virDomainNetGetActualVlan(const virDomainNetDef *iface); bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); const char *virDomainNetGetModelString(const virDomainNetDef *net); diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index 9af2173b7b..0a27735e71 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -260,7 +260,7 @@ virNetDevBandwidthRateFormat(virNetDevBandwidthRatePtr def, * Returns 0 on success, else -1. */ int -virNetDevBandwidthFormat(virNetDevBandwidthPtr def, +virNetDevBandwidthFormat(const virNetDevBandwidth *def, unsigned int class_id, virBufferPtr buf) { diff --git a/src/conf/netdev_bandwidth_conf.h b/src/conf/netdev_bandwidth_conf.h index 898b2fb428..0004e48a4a 100644 --- a/src/conf/netdev_bandwidth_conf.h +++ b/src/conf/netdev_bandwidth_conf.h @@ -29,7 +29,7 @@ int virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth, xmlNodePtr node, bool allowFloor) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; -int virNetDevBandwidthFormat(virNetDevBandwidthPtr def, +int virNetDevBandwidthFormat(const virNetDevBandwidth *def, unsigned int class_id, virBufferPtr buf); diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 89122826fc..b0d67c7b83 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1216,7 +1216,7 @@ libxlMakeNic(virDomainDefPtr def, virDomainNetType actual_type = virDomainNetGetActualType(l_nic); virNetworkPtr network = NULL; virConnectPtr conn = NULL; - virNetDevBandwidthPtr actual_bw; + const virNetDevBandwidth *actual_bw; virNetDevVPortProfilePtr port_profile; const virNetDevVlan *virt_vlan; virBuffer buf = VIR_BUFFER_INITIALIZER; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 41a6a446bd..34c70240cb 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3829,7 +3829,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, virLXCDomainObjPrivatePtr priv = vm->privateData; int ret = -1; virDomainNetType actualType; - virNetDevBandwidthPtr actualBandwidth; + const virNetDevBandwidth *actualBandwidth; char *veth = NULL; if (!priv->initpid) { diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 450053d163..323f3632c0 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -338,7 +338,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn, char *ret = NULL; char *res_ifname = NULL; virLXCDriverPtr driver = conn->privateData; - virNetDevBandwidthPtr bw; + const virNetDevBandwidth *bw; virNetDevVPortProfilePtr prof; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); const char *linkdev = virDomainNetGetActualDirectDev(net); @@ -557,7 +557,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, for (i = 0; i < def->nnets; i++) { char *veth = NULL; - virNetDevBandwidthPtr actualBandwidth; + const virNetDevBandwidth *actualBandwidth; /* If appropriate, grab a physical device from the configured * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f24013f9eb..548cba2aec 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8397,7 +8397,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, char **vhostfdName = NULL; g_autofree char *slirpfdName = NULL; virDomainNetType actualType = virDomainNetGetActualType(net); - virNetDevBandwidthPtr actualBandwidth; + const virNetDevBandwidth *actualBandwidth; bool requireNicdev = false; qemuSlirpPtr slirp; size_t i; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1fa5e6e0f8..4cbe7b0060 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1146,7 +1146,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, bool releaseaddr = false; bool iface_connected = false; virDomainNetType actualType; - virNetDevBandwidthPtr actualBandwidth; + const virNetDevBandwidth *actualBandwidth; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virDomainCCWAddressSetPtr ccwaddrs = NULL; size_t i; @@ -3824,7 +3824,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, } if (needBandwidthSet) { - virNetDevBandwidthPtr newb = virDomainNetGetActualBandwidth(newdev); + const virNetDevBandwidth *newb = virDomainNetGetActualBandwidth(newdev); if (newb) { if (virNetDevBandwidthSet(newdev->ifname, newb, false, diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index b90bd55d32..3f3785cb65 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -190,7 +190,7 @@ virNetDevBandwidthManipulateFilter(const char *ifname, */ int virNetDevBandwidthSet(const char *ifname, - virNetDevBandwidthPtr bandwidth, + const virNetDevBandwidth *bandwidth, bool hierarchical_class, bool swapped) { @@ -485,8 +485,8 @@ virNetDevBandwidthCopy(virNetDevBandwidthPtr *dest, } bool -virNetDevBandwidthEqual(virNetDevBandwidthPtr a, - virNetDevBandwidthPtr b) +virNetDevBandwidthEqual(const virNetDevBandwidth *a, + const virNetDevBandwidth *b) { if (!a && !b) return true; diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h index 19323c5ed2..871d0c962c 100644 --- a/src/util/virnetdevbandwidth.h +++ b/src/util/virnetdevbandwidth.h @@ -39,7 +39,7 @@ struct _virNetDevBandwidth { void virNetDevBandwidthFree(virNetDevBandwidthPtr def); int virNetDevBandwidthSet(const char *ifname, - virNetDevBandwidthPtr bandwidth, + const virNetDevBandwidth *bandwidth, bool hierarchical_class, bool swapped) G_GNUC_WARN_UNUSED_RESULT; @@ -48,7 +48,7 @@ int virNetDevBandwidthCopy(virNetDevBandwidthPtr *dest, const virNetDevBandwidth *src) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; -bool virNetDevBandwidthEqual(virNetDevBandwidthPtr a, virNetDevBandwidthPtr b); +bool virNetDevBandwidthEqual(const virNetDevBandwidth *a, const virNetDevBandwidth *b); int virNetDevBandwidthPlug(const char *brname, virNetDevBandwidthPtr net_bandwidth, -- 2.21.0
This also isn't required (due to the vportprofile being stored in the NetDef as a pointer rather than being directly contained), but it seemed dishonest to not mark it as const (and thus permit users to modify its contents) Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 2 +- src/conf/netdev_vport_profile_conf.c | 2 +- src/conf/netdev_vport_profile_conf.h | 2 +- src/libxl/libxl_conf.c | 2 +- src/libxl/xen_common.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/lxc/lxc_process.c | 8 ++++---- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_migration_cookie.c | 2 +- src/qemu/qemu_process.c | 2 +- src/util/virhostdev.c | 6 +++--- src/util/virnetdevmacvlan.c | 16 ++++++++-------- src/util/virnetdevmacvlan.h | 8 ++++---- src/util/virnetdevmidonet.c | 4 ++-- src/util/virnetdevmidonet.h | 4 ++-- src/util/virnetdevopenvswitch.c | 2 +- src/util/virnetdevopenvswitch.h | 2 +- src/util/virnetdevtap.c | 6 +++--- src/util/virnetdevtap.h | 6 +++--- src/util/virnetdevvportprofile.c | 12 ++++++------ src/util/virnetdevvportprofile.h | 12 ++++++------ tests/bhyvexml2argvmock.c | 2 +- 24 files changed, 55 insertions(+), 55 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 76e036c48e..af8c0af7c7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29665,7 +29665,7 @@ virDomainNetGetActualHostdev(virDomainNetDefPtr iface) return NULL; } -virNetDevVPortProfilePtr +const virNetDevVPortProfile * virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface) { switch (iface->type) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 44d5fb8e16..b08202b092 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3209,7 +3209,7 @@ int virDomainNetGetActualBridgeMACTableManager(virDomainNetDefPtr iface); const char *virDomainNetGetActualDirectDev(virDomainNetDefPtr iface); int virDomainNetGetActualDirectMode(virDomainNetDefPtr iface); virDomainHostdevDefPtr virDomainNetGetActualHostdev(virDomainNetDefPtr iface); -virNetDevVPortProfilePtr +const virNetDevVPortProfile * virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface); const virNetDevBandwidth * virDomainNetGetActualBandwidth(const virDomainNetDef *iface); diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c index 660478d128..1dd8439adb 100644 --- a/src/conf/netdev_vport_profile_conf.c +++ b/src/conf/netdev_vport_profile_conf.c @@ -197,7 +197,7 @@ virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags) int -virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort, +virNetDevVPortProfileFormat(const virNetDevVPortProfile *virtPort, virBufferPtr buf) { enum virNetDevVPortProfile type; diff --git a/src/conf/netdev_vport_profile_conf.h b/src/conf/netdev_vport_profile_conf.h index a2989af81e..6a99a42ee8 100644 --- a/src/conf/netdev_vport_profile_conf.h +++ b/src/conf/netdev_vport_profile_conf.h @@ -40,5 +40,5 @@ virNetDevVPortProfilePtr virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags); int -virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort, +virNetDevVPortProfileFormat(const virNetDevVPortProfile *virtPort, virBufferPtr buf); diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index b0d67c7b83..113999b7b5 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1217,7 +1217,7 @@ libxlMakeNic(virDomainDefPtr def, virNetworkPtr network = NULL; virConnectPtr conn = NULL; const virNetDevBandwidth *actual_bw; - virNetDevVPortProfilePtr port_profile; + const virNetDevVPortProfile *port_profile; const virNetDevVlan *virt_vlan; virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i; diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 6e77921333..140a122fa1 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -1680,7 +1680,7 @@ xenFormatNet(virConnectPtr conn, switch (net->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: { - virNetDevVPortProfilePtr port_profile = virDomainNetGetActualVirtPortProfile(net); + const virNetDevVPortProfile *port_profile = virDomainNetGetActualVirtPortProfile(net); const virNetDevVlan *virt_vlan = virDomainNetGetActualVlan(net); const char *script = net->script; size_t i; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 34c70240cb..1a1239b1bd 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4351,7 +4351,7 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm, int detachidx, ret = -1; virDomainNetType actualType; virDomainNetDefPtr detach = NULL; - virNetDevVPortProfilePtr vport = NULL; + const virNetDevVPortProfile *vport = NULL; virErrorPtr save_err = NULL; if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 323f3632c0..1a565b6358 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -163,7 +163,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, { size_t i; virLXCDomainObjPrivatePtr priv = vm->privateData; - virNetDevVPortProfilePtr vport = NULL; + const virNetDevVPortProfile *vport = NULL; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); virConnectPtr conn = NULL; @@ -284,7 +284,7 @@ virLXCProcessSetupInterfaceTap(virDomainDefPtr vm, char *ret = NULL; char *parentVeth; char *containerVeth = NULL; - virNetDevVPortProfilePtr vport = virDomainNetGetActualVirtPortProfile(net); + const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net); VIR_DEBUG("calling vethCreate()"); parentVeth = net->ifname; @@ -339,7 +339,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn, char *res_ifname = NULL; virLXCDriverPtr driver = conn->privateData; const virNetDevBandwidth *bw; - virNetDevVPortProfilePtr prof; + const virNetDevVPortProfile *prof; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); const char *linkdev = virDomainNetGetActualDirectDev(net); unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_IFUP; @@ -645,7 +645,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, virErrorPreserveLast(&save_err); for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr iface = def->nets[i]; - virNetDevVPortProfilePtr vport = virDomainNetGetActualVirtPortProfile(iface); + const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(iface); if (vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) ignore_value(virNetDevOpenvswitchRemovePort( virDomainNetGetActualBridgeName(iface), diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b2e8759da7..0181b62602 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5420,7 +5420,7 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net, * a <virtualport> element in the config) */ if (net->filter) { - virNetDevVPortProfilePtr vport = virDomainNetGetActualVirtPortProfile(net); + const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net); if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4cbe7b0060..8c5f516ab2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1108,7 +1108,7 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, static void qemuDomainNetDeviceVportRemove(virDomainNetDefPtr net) { - virNetDevVPortProfilePtr vport = virDomainNetGetActualVirtPortProfile(net); + const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net); const char *brname; if (!vport) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index f625c7086b..a3de9180ea 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -243,7 +243,7 @@ qemuMigrationCookieNetworkAlloc(virQEMUDriverPtr driver G_GNUC_UNUSED, for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr netptr; - virNetDevVPortProfilePtr vport; + const virNetDevVPortProfile *vport; netptr = def->nets[i]; vport = virDomainNetGetActualVirtPortProfile(netptr); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7e0dc26658..a387341c8e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7384,7 +7384,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; virErrorPtr orig_err; virDomainDefPtr def; - virNetDevVPortProfilePtr vport = NULL; + const virNetDevVPortProfile *vport = NULL; size_t i; char *timestamp; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 223a78bd34..8922a01d4b 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -406,7 +406,7 @@ virHostdevIsVFIODevice(const virDomainHostdevDef *hostdev) static int virHostdevNetConfigVirtPortProfile(const char *linkdev, int vf, - virNetDevVPortProfilePtr virtPort, + const virNetDevVPortProfile *virtPort, const virMacAddr *macaddr, const unsigned char *uuid, bool associate) @@ -502,7 +502,7 @@ virHostdevSetNetConfig(virDomainHostdevDefPtr hostdev, { g_autofree char *linkdev = NULL; const virNetDevVlan *vlan; - virNetDevVPortProfilePtr virtPort; + const virNetDevVPortProfile *virtPort; int vf = -1; bool port_profile_associate = true; @@ -552,7 +552,7 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev, g_autofree virMacAddrPtr MAC = NULL; g_autofree virMacAddrPtr adminMAC = NULL; g_autoptr(virNetDevVlan) vlan = NULL; - virNetDevVPortProfilePtr virtPort; + const virNetDevVPortProfile *virtPort; int vf = -1; bool port_profile_associate = false; diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index c530cb75ed..e343dad923 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -828,7 +828,7 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname, const virMacAddr *macaddress, const char *linkdev, const unsigned char *vmuuid, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, virNetDevVPortProfileOp vmOp) { virNetlinkCallbackDataPtr calld = NULL; @@ -897,7 +897,7 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested, virNetDevMacVLanMode mode, const virNetDevVlan *vlan, const unsigned char *vmuuid, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, char **ifnameResult, virNetDevVPortProfileOp vmOp, char *stateDir, @@ -1094,7 +1094,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, const virMacAddr *macaddr, const char *linkdev, int mode, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, char *stateDir) { int ret = 0; @@ -1149,7 +1149,7 @@ int virNetDevMacVLanRestartWithVPortProfile(const char *cr_ifname, const virMacAddr *macaddress, const char *linkdev, const unsigned char *vmuuid, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, virNetDevVPortProfileOp vmOp) { int rc = 0; @@ -1226,7 +1226,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname G_GNUC_UNUSED, virNetDevMacVLanMode mode G_GNUC_UNUSED, const virNetDevVlan *vlan G_GNUC_UNUSED, const unsigned char *vmuuid G_GNUC_UNUSED, - virNetDevVPortProfilePtr virtPortProfile G_GNUC_UNUSED, + const virNetDevVPortProfile *virtPortProfile G_GNUC_UNUSED, char **res_ifname G_GNUC_UNUSED, virNetDevVPortProfileOp vmop G_GNUC_UNUSED, char *stateDir G_GNUC_UNUSED, @@ -1243,7 +1243,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname G_GNUC_UNUSED, const virMacAddr *macaddress G_GNUC_UNUSED, const char *linkdev G_GNUC_UNUSED, int mode G_GNUC_UNUSED, - virNetDevVPortProfilePtr virtPortProfile G_GNUC_UNUSED, + const virNetDevVPortProfile *virtPortProfile G_GNUC_UNUSED, char *stateDir G_GNUC_UNUSED) { virReportSystemError(ENOSYS, "%s", @@ -1255,7 +1255,7 @@ int virNetDevMacVLanRestartWithVPortProfile(const char *cr_ifname G_GNUC_UNUSED, const virMacAddr *macaddress G_GNUC_UNUSED, const char *linkdev G_GNUC_UNUSED, const unsigned char *vmuuid G_GNUC_UNUSED, - virNetDevVPortProfilePtr virtPortProfile G_GNUC_UNUSED, + const virNetDevVPortProfile *virtPortProfile G_GNUC_UNUSED, virNetDevVPortProfileOp vmOp G_GNUC_UNUSED) { virReportSystemError(ENOSYS, "%s", @@ -1267,7 +1267,7 @@ int virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname G_GNUC_UNUSE const virMacAddr *macaddress G_GNUC_UNUSED, const char *linkdev G_GNUC_UNUSED, const unsigned char *vmuuid G_GNUC_UNUSED, - virNetDevVPortProfilePtr virtPortProfile G_GNUC_UNUSED, + const virNetDevVPortProfile *virtPortProfile G_GNUC_UNUSED, virNetDevVPortProfileOp vmOp G_GNUC_UNUSED) { virReportSystemError(ENOSYS, "%s", diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h index f455ec3384..fc1bb018a2 100644 --- a/src/util/virnetdevmacvlan.h +++ b/src/util/virnetdevmacvlan.h @@ -78,7 +78,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname, virNetDevMacVLanMode mode, const virNetDevVlan *vlan, const unsigned char *vmuuid, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, char **res_ifname, virNetDevVPortProfileOp vmop, char *stateDir, @@ -101,7 +101,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, const virMacAddr *macaddress, const char *linkdev, int mode, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, char *stateDir) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(6) G_GNUC_WARN_UNUSED_RESULT; @@ -110,7 +110,7 @@ int virNetDevMacVLanRestartWithVPortProfile(const char *cr_ifname, const virMacAddr *macaddress, const char *linkdev, const unsigned char *vmuuid, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, virNetDevVPortProfileOp vmOp) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; @@ -119,7 +119,7 @@ int virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname, const virMacAddr *macaddress, const char *linkdev, const unsigned char *vmuuid, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, virNetDevVPortProfileOp vmOp) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/util/virnetdevmidonet.c b/src/util/virnetdevmidonet.c index e50cceaef5..354350ce1b 100644 --- a/src/util/virnetdevmidonet.c +++ b/src/util/virnetdevmidonet.c @@ -37,7 +37,7 @@ */ int virNetDevMidonetBindPort(const char *ifname, - virNetDevVPortProfilePtr virtualport) + const virNetDevVPortProfile *virtualport) { int ret = -1; virCommandPtr cmd = NULL; @@ -71,7 +71,7 @@ virNetDevMidonetBindPort(const char *ifname, * Returns 0 in case of success or -1 in case of failure. */ int -virNetDevMidonetUnbindPort(virNetDevVPortProfilePtr virtualport) +virNetDevMidonetUnbindPort(const virNetDevVPortProfile *virtualport) { int ret = -1; virCommandPtr cmd = NULL; diff --git a/src/util/virnetdevmidonet.h b/src/util/virnetdevmidonet.h index 61927c3edd..87ac3d74a1 100644 --- a/src/util/virnetdevmidonet.h +++ b/src/util/virnetdevmidonet.h @@ -23,8 +23,8 @@ int virNetDevMidonetBindPort(const char *ifname, - virNetDevVPortProfilePtr virtualport) + const virNetDevVPortProfile *virtualport) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; -int virNetDevMidonetUnbindPort(virNetDevVPortProfilePtr virtualport) +int virNetDevMidonetUnbindPort(const virNetDevVPortProfile *virtualport) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 9aa2c5ad84..81898057b1 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -135,7 +135,7 @@ virNetDevOpenvswitchConstructVlans(virCommandPtr cmd, const virNetDevVlan *virtV int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, const virMacAddr *macaddr, const unsigned char *vmuuid, - virNetDevVPortProfilePtr ovsport, + const virNetDevVPortProfile *ovsport, const virNetDevVlan *virtVlan) { char macaddrstr[VIR_MAC_STRING_BUFLEN]; diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index dddb21a5a2..c9ea592058 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -32,7 +32,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, const virMacAddr *macaddr, const unsigned char *vmuuid, - virNetDevVPortProfilePtr ovsport, + const virNetDevVPortProfile *ovsport, const virNetDevVlan *virtVlan) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index fa1626d6ba..d012c5e210 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -502,7 +502,7 @@ virNetDevTapAttachBridge(const char *tapname, const char *brname, const virMacAddr *macaddr, const unsigned char *vmuuid, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, unsigned int mtu, unsigned int *actualMTU) @@ -574,7 +574,7 @@ virNetDevTapReattachBridge(const char *tapname, const char *brname, const virMacAddr *macaddr, const unsigned char *vmuuid, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, unsigned int mtu, unsigned int *actualMTU) @@ -660,7 +660,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, const char *tunpath, int *tapfd, size_t tapfdSize, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, virNetDevCoalescePtr coalesce, unsigned int mtu, diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index 4fc734fd5e..cae8e61861 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -63,7 +63,7 @@ virNetDevTapAttachBridge(const char *tapname, const char *brname, const virMacAddr *macaddr, const unsigned char *vmuuid, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, unsigned int mtu, unsigned int *actualMTU) @@ -75,7 +75,7 @@ virNetDevTapReattachBridge(const char *tapname, const char *brname, const virMacAddr *macaddr, const unsigned char *vmuuid, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, unsigned int mtu, unsigned int *actualMTU) @@ -89,7 +89,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, const char *tunpath, int *tapfd, size_t tapfdSize, - virNetDevVPortProfilePtr virtPortProfile, + const virNetDevVPortProfile *virtPortProfile, const virNetDevVlan *virtVlan, virNetDevCoalescePtr coalesce, unsigned int mtu, diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 6442c2a2cf..593bd6e2d6 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -85,7 +85,7 @@ enum virNetDevVPortProfileLinkOp { #endif bool -virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a, virNetDevVPortProfilePtr b) +virNetDevVPortProfileEqual(const virNetDevVPortProfile *a, const virNetDevVPortProfile *b) { /* NULL resistant */ if (!a && !b) @@ -226,7 +226,7 @@ virNetDevVPortProfileCheckComplete(virNetDevVPortProfilePtr virtport, * an error is logged and -1 is returned. */ int -virNetDevVPortProfileCheckNoExtras(virNetDevVPortProfilePtr virtport) +virNetDevVPortProfileCheckNoExtras(const virNetDevVPortProfile *virtport) { const char *extra = NULL; @@ -283,7 +283,7 @@ virNetDevVPortProfileCheckNoExtras(virNetDevVPortProfilePtr virtport) */ static int virNetDevVPortProfileMerge(virNetDevVPortProfilePtr orig, - virNetDevVPortProfilePtr mods) + const virNetDevVPortProfile *mods) { enum virNetDevVPortProfile otype; @@ -423,9 +423,9 @@ virNetDevVPortProfileMerge(virNetDevVPortProfilePtr orig, */ int virNetDevVPortProfileMerge3(virNetDevVPortProfilePtr *result, - virNetDevVPortProfilePtr fromInterface, - virNetDevVPortProfilePtr fromNetwork, - virNetDevVPortProfilePtr fromPortgroup) + const virNetDevVPortProfile *fromInterface, + const virNetDevVPortProfile *fromNetwork, + const virNetDevVPortProfile *fromPortgroup) { int ret = -1; *result = NULL; diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index 5fdd019a09..88a42a1816 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -77,19 +77,19 @@ struct _virNetDevVPortProfile { }; -bool virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a, - virNetDevVPortProfilePtr b); +bool virNetDevVPortProfileEqual(const virNetDevVPortProfile *a, + const virNetDevVPortProfile *b); int virNetDevVPortProfileCopy(virNetDevVPortProfilePtr *dst, const virNetDevVPortProfile *src); int virNetDevVPortProfileCheckComplete(virNetDevVPortProfilePtr virtport, bool generateMissing); -int virNetDevVPortProfileCheckNoExtras(virNetDevVPortProfilePtr virtport); +int virNetDevVPortProfileCheckNoExtras(const virNetDevVPortProfile *virtport); int virNetDevVPortProfileMerge3(virNetDevVPortProfilePtr *result, - virNetDevVPortProfilePtr fromInterface, - virNetDevVPortProfilePtr fromNetwork, - virNetDevVPortProfilePtr fromPortgroup); + const virNetDevVPortProfile *fromInterface, + const virNetDevVPortProfile *fromNetwork, + const virNetDevVPortProfile *fromPortgroup); int virNetDevVPortProfileAssociate(const char *ifname, const virNetDevVPortProfile *virtPort, diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c index ac4ffe8596..2a552f9f47 100644 --- a/tests/bhyvexml2argvmock.c +++ b/tests/bhyvexml2argvmock.c @@ -26,7 +26,7 @@ int virNetDevTapCreateInBridgePort(const char *brname G_GNUC_UNUSED, const char *tunpath G_GNUC_UNUSED, int *tapfd G_GNUC_UNUSED, size_t tapfdSize G_GNUC_UNUSED, - virNetDevVPortProfilePtr virtPortProfile G_GNUC_UNUSED, + const virNetDevVPortProfile *virtPortProfile G_GNUC_UNUSED, const virNetDevVlan *virtVlan G_GNUC_UNUSED, virNetDevCoalescePtr coalesce G_GNUC_UNUSED, unsigned int mtu G_GNUC_UNUSED, -- 2.21.0
These all just return a scalar value, so there's no daisy-chained fallout from changing them, and they can easily be combined in a single patch. Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 10 +++++----- src/conf/domain_conf.h | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index af8c0af7c7..01a1a74946 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29606,7 +29606,7 @@ virDomainNetGetActualType(const virDomainNetDef *iface) } const char * -virDomainNetGetActualBridgeName(virDomainNetDefPtr iface) +virDomainNetGetActualBridgeName(const virDomainNetDef *iface) { if (iface->type == VIR_DOMAIN_NET_TYPE_BRIDGE) return iface->data.bridge.brname; @@ -29619,7 +29619,7 @@ virDomainNetGetActualBridgeName(virDomainNetDefPtr iface) } int -virDomainNetGetActualBridgeMACTableManager(virDomainNetDefPtr iface) +virDomainNetGetActualBridgeMACTableManager(const virDomainNetDef *iface) { if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK && iface->data.network.actual && @@ -29630,7 +29630,7 @@ virDomainNetGetActualBridgeMACTableManager(virDomainNetDefPtr iface) } const char * -virDomainNetGetActualDirectDev(virDomainNetDefPtr iface) +virDomainNetGetActualDirectDev(const virDomainNetDef *iface) { if (iface->type == VIR_DOMAIN_NET_TYPE_DIRECT) return iface->data.direct.linkdev; @@ -29642,7 +29642,7 @@ virDomainNetGetActualDirectDev(virDomainNetDefPtr iface) } int -virDomainNetGetActualDirectMode(virDomainNetDefPtr iface) +virDomainNetGetActualDirectMode(const virDomainNetDef *iface) { if (iface->type == VIR_DOMAIN_NET_TYPE_DIRECT) return iface->data.direct.mode; @@ -29729,7 +29729,7 @@ virDomainNetGetActualVlan(const virDomainNetDef *iface) bool -virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface) +virDomainNetGetActualTrustGuestRxFilters(const virDomainNetDef *iface) { if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK && iface->data.network.actual) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b08202b092..4a7010e8a5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3204,17 +3204,17 @@ int virDomainGraphicsListenAppendSocket(virDomainGraphicsDefPtr def, ATTRIBUTE_NONNULL(1); virDomainNetType virDomainNetGetActualType(const virDomainNetDef *iface); -const char *virDomainNetGetActualBridgeName(virDomainNetDefPtr iface); -int virDomainNetGetActualBridgeMACTableManager(virDomainNetDefPtr iface); -const char *virDomainNetGetActualDirectDev(virDomainNetDefPtr iface); -int virDomainNetGetActualDirectMode(virDomainNetDefPtr iface); +const char *virDomainNetGetActualBridgeName(const virDomainNetDef *iface); +int virDomainNetGetActualBridgeMACTableManager(const virDomainNetDef *iface); +const char *virDomainNetGetActualDirectDev(const virDomainNetDef *iface); +int virDomainNetGetActualDirectMode(const virDomainNetDef *iface); virDomainHostdevDefPtr virDomainNetGetActualHostdev(virDomainNetDefPtr iface); const virNetDevVPortProfile * virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface); const virNetDevBandwidth * virDomainNetGetActualBandwidth(const virDomainNetDef *iface); const virNetDevVlan *virDomainNetGetActualVlan(const virDomainNetDef *iface); -bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); +bool virDomainNetGetActualTrustGuestRxFilters(const virDomainNetDef *iface); const char *virDomainNetGetModelString(const virDomainNetDef *net); int virDomainNetSetModelString(virDomainNetDefPtr et, const char *model); -- 2.21.0
<interface> devices (virDomainNetDef) are a bit different from other types of devices in that their actual type may come from a network (in the form of a port connection), and that doesn't happen until the domain is started. This means that any validation of an <interface> at parse time needs to be a bit liberal in what it accepts - when type='network', you could think that something is/isn't allowed, but once the domain is started and a port is created by the configured network, the opposite might be true. To solve this problem hypervisor drivers need to do an extra validation step when the domain is being started. I recently (commit 3cff23f7, libvirt 5.7.0) added a function to peform such validation for all interfaces to the QEMU driver - qemuDomainValidateActualNetDef() - but while that function is a good single point to call for the multiple places that need to "start" an interface (domain startup, device hotplug, device update), it can't be called by the other hypervisor drivers, since 1) it's in the QEMU driver, and 2) it contains some checks specific to QEMU. For validation that applies to network devices on *all* hypervisors, we need yet another interface validation function that can be called by any hypervisor driver (not just QEMU) right after its network port has been created during domain startup or hotplug. This patch adds that function - virDomainNetDefRuntimeValidate(), in the conf directory, and calls it in appropriate places in the QEMU, lxc, and libxl drivers. The new function, virDomainNetDefRuntimeValidate(), should contain network device validation that 1) is hypervisor agnostic, and 2) can't be done until we know the "actual type" of an interface. There is no framework for validation at domain startup as there is for post-parse validation, but I don't want to create a whole elaborate system that will only be used by one type of device. For that reason, I just made a single function that should be called directly from the hypervisors, when they are initializing interfaces to start a domain, right after conditionally allocating the network port (and regardless of whether or not that was actually needed). In the case of the QEMU driver, qemuDomainValidateActualNetDef() is already called in all the appropriate places, so we can just call the new function from there. In the case of the other hypervisors, we search for virDomainNetAllocateActualDevice() (which is the hypervisor-agnostic function that calls virNetworkPortCreateXML()), and add the call to our new function right after that. The new function itself could be plunked down into many places in the code, but we already have 3 validation functions for network devices in 2 different places (not counting any basic validation done in virDomainNetDefParseXML() itself): 1) post-parse hypervisor-agnostic (virDomainNetDefValidate() - domain_conf.c:6145) 2) post-parse hypervisor-specific (qemuDomainDeviceDefValidateNetwork() - qemu_domain.c:5498) 3) domain-start hypervisor-specific (qemuDomainValidateActualNetDef() - qemu_domain.c:5390) I placed (3) right next to (2) when I added it, specifically to avoid spreading validation all over the code. For the same reason, I decided to put this new function right next to (1) - this way if someone needs to add validation specific to qemu, they go to one location, and if they need to add validation applying to everyone, they go to the other. It looks a bit strange to have a public function in between a bunch of statics, but I think it's better than the alternative of further fragmentation. (I'm open to other ideas though, of course.) Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 22 ++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/libxl/libxl_domain.c | 4 ++++ src/libxl/libxl_driver.c | 4 ++++ src/lxc/lxc_driver.c | 4 ++++ src/lxc/lxc_process.c | 4 ++++ src/qemu/qemu_domain.c | 6 ++++++ 8 files changed, 48 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 01a1a74946..c063f40a4c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6094,6 +6094,28 @@ virDomainRedirdevDefValidate(const virDomainDef *def, } +int +virDomainNetDefRuntimeValidate(const virDomainNetDef *net G_GNUC_UNUSED) +{ + /* Unlike virDomainNetDefValidate(), which is a static function + * called internally to this file, virDomainActualNetDefValidate() + * is a public function that can be called from a hypervisor after + * it has completely setup the NetDef for use by a domain, + * including possibly allocating a port from the network driver + * (which could change the effective/"actual" type of the NetDef, + * thus changing what should/shouldn't be allowed by validation). + * + * This function should contain validations not specific to a + * particular hypervisor (e.g. whether or not specifying bandwidth + * is allowed for a type of interface), but *not* + * hypervisor-specific things. + */ + + return 0; + +} + + static int virDomainNetDefValidate(const virDomainNetDef *net) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4a7010e8a5..dc0226e6da 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2802,6 +2802,9 @@ int virDomainDefValidate(virDomainDefPtr def, unsigned int parseFlags, virDomainXMLOptionPtr xmlopt); +int +virDomainNetDefRuntimeValidate(const virDomainNetDef *net); + static inline bool virDomainObjIsActive(virDomainObjPtr dom) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 12cb3b5bf7..811d01c4f5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -479,6 +479,7 @@ virDomainNetDefClear; virDomainNetDefFormat; virDomainNetDefFree; virDomainNetDefNew; +virDomainNetDefRuntimeValidate; virDomainNetDefToNetworkPort; virDomainNetFind; virDomainNetFindByName; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index d79c3c1ed7..57d2a4aeb5 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1065,6 +1065,10 @@ libxlNetworkPrepareDevices(virDomainDefPtr def) goto cleanup; } + /* final validation now that actual type is known */ + if (virDomainNetDefRuntimeValidate(net) < 0) + return -1; + actualType = virDomainNetGetActualType(net); if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV && net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 5cc6b87b8c..323cb92c04 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3425,6 +3425,10 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, goto cleanup; } + /* final validation now that actual type is known */ + if (virDomainNetDefRuntimeValidate(net) < 0) + return -1; + actualType = virDomainNetGetActualType(net); if (virDomainHasNet(vm->def, net)) { diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1a1239b1bd..8d519d282a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3860,6 +3860,10 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, virObjectUnref(netconn); } + /* final validation now that actual type is known */ + if (virDomainNetDefRuntimeValidate(net) < 0) + return -1; + actualType = virDomainNetGetActualType(net); switch (actualType) { diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 1a565b6358..13547abec4 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -574,6 +574,10 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, goto cleanup; } + /* final validation now that actual type is known */ + if (virDomainNetDefRuntimeValidate(net) < 0) + return -1; + type = virDomainNetGetActualType(net); switch (type) { case VIR_DOMAIN_NET_TYPE_NETWORK: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0181b62602..a8a7907546 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5389,6 +5389,12 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net, virMacAddrFormat(&net->mac, macstr); + /* hypervisor-agnostic validation */ + if (virDomainNetDefRuntimeValidate(net) < 0) + return -1; + + /* QEMU-specific validation */ + /* Only tap/macvtap devices support multiqueue. */ if (net->driver.virtio.queues > 0) { -- 2.21.0
On 10/22/19 9:24 PM, Laine Stump wrote:
<interface> devices (virDomainNetDef) are a bit different from other types of devices in that their actual type may come from a network (in the form of a port connection), and that doesn't happen until the domain is started. This means that any validation of an <interface> at parse time needs to be a bit liberal in what it accepts - when type='network', you could think that something is/isn't allowed, but once the domain is started and a port is created by the configured network, the opposite might be true.
To solve this problem hypervisor drivers need to do an extra validation step when the domain is being started. I recently (commit 3cff23f7, libvirt 5.7.0) added a function to peform such validation for all interfaces to the QEMU driver - qemuDomainValidateActualNetDef() - but while that function is a good single point to call for the multiple places that need to "start" an interface (domain startup, device hotplug, device update), it can't be called by the other hypervisor drivers, since 1) it's in the QEMU driver, and 2) it contains some checks specific to QEMU. For validation that applies to network devices on *all* hypervisors, we need yet another interface validation function that can be called by any hypervisor driver (not just QEMU) right after its network port has been created during domain startup or hotplug. This patch adds that function - virDomainNetDefRuntimeValidate(), in the conf directory, and calls it in appropriate places in the QEMU, lxc, and libxl drivers.
The new function, virDomainNetDefRuntimeValidate(), should contain network device validation that 1) is hypervisor agnostic, and 2) can't be done until we know the "actual type" of an interface.
There is no framework for validation at domain startup as there is for post-parse validation, but I don't want to create a whole elaborate system that will only be used by one type of device. For that reason, I just made a single function that should be called directly from the hypervisors, when they are initializing interfaces to start a domain, right after conditionally allocating the network port (and regardless of whether or not that was actually needed). In the case of the QEMU driver, qemuDomainValidateActualNetDef() is already called in all the appropriate places, so we can just call the new function from there. In the case of the other hypervisors, we search for virDomainNetAllocateActualDevice() (which is the hypervisor-agnostic function that calls virNetworkPortCreateXML()), and add the call to our new function right after that.
The new function itself could be plunked down into many places in the code, but we already have 3 validation functions for network devices in 2 different places (not counting any basic validation done in virDomainNetDefParseXML() itself):
1) post-parse hypervisor-agnostic (virDomainNetDefValidate() - domain_conf.c:6145) 2) post-parse hypervisor-specific (qemuDomainDeviceDefValidateNetwork() - qemu_domain.c:5498) 3) domain-start hypervisor-specific (qemuDomainValidateActualNetDef() - qemu_domain.c:5390)
I placed (3) right next to (2) when I added it, specifically to avoid spreading validation all over the code. For the same reason, I decided to put this new function right next to (1) - this way if someone needs to add validation specific to qemu, they go to one location, and if they need to add validation applying to everyone, they go to the other. It looks a bit strange to have a public function in between a bunch of statics, but I think it's better than the alternative of further fragmentation. (I'm open to other ideas though, of course.)
Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 22 ++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/libxl/libxl_domain.c | 4 ++++ src/libxl/libxl_driver.c | 4 ++++ src/lxc/lxc_driver.c | 4 ++++ src/lxc/lxc_process.c | 4 ++++ src/qemu/qemu_domain.c | 6 ++++++ 8 files changed, 48 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 01a1a74946..c063f40a4c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6094,6 +6094,28 @@ virDomainRedirdevDefValidate(const virDomainDef *def, }
+int +virDomainNetDefRuntimeValidate(const virDomainNetDef *net G_GNUC_UNUSED) +{ + /* Unlike virDomainNetDefValidate(), which is a static function + * called internally to this file, virDomainActualNetDefValidate() + * is a public function that can be called from a hypervisor after + * it has completely setup the NetDef for use by a domain, + * including possibly allocating a port from the network driver + * (which could change the effective/"actual" type of the NetDef, + * thus changing what should/shouldn't be allowed by validation). + *
The comment here claims the function is named virDomainActualNetDefValidate. I think that naming is more consistent rather than invent a new 'runtime' naming scheme, event though runtime is more semanticly descriptive. (though TBH I wouldn't mind s/actual/runtime/ in the codebase anyways, the 'actual' naming always confuses me) Comment should be fixed either way, I'll leave it up to you whether to actually rename the function. Series: Reviewed-by: Cole Robinson <crobinso@redhat.com> I think there's a conflict in patch 7, git am gave me some confusing results, so make sure the code you are moving is incorporating and possible new changes that landed there since this was posted. - Cole
On 11/14/19 5:20 PM, Cole Robinson wrote:
On 10/22/19 9:24 PM, Laine Stump wrote:
+int +virDomainNetDefRuntimeValidate(const virDomainNetDef *net G_GNUC_UNUSED) +{ + /* Unlike virDomainNetDefValidate(), which is a static function + * called internally to this file, virDomainActualNetDefValidate() + * is a public function that can be called from a hypervisor after + * it has completely setup the NetDef for use by a domain, + * including possibly allocating a port from the network driver + * (which could change the effective/"actual" type of the NetDef, + * thus changing what should/shouldn't be allowed by validation). + *
The comment here claims the function is named virDomainActualNetDefValidate. I think that naming is more consistent rather than invent a new 'runtime' naming scheme, event though runtime is more semanticly descriptive. (though TBH I wouldn't mind s/actual/runtime/ in the codebase anyways, the 'actual' naming always confuses me)
Well, "actual" is describing *what* it is, and "runtime" is describing *when* it is. In most cases I've been fine with the "actual" name, but this time it didn't seem to adequately convey the usefulness of the function, so I decided to try something else out, and missed the function name in the comment.
Comment should be fixed either way, I'll leave it up to you whether to actually rename the function.
Thinking about it more, I guess it's better to remain consistent everywhere. Maybe someday someone will find some other adjective that works better than actual in all circumstances (it might even be "runtime", my personal mental jury is still out), and will switch all occurrences at once.
Series:
Reviewed-by: Cole Robinson <crobinso@redhat.com>
I think there's a conflict in patch 7, git am gave me some confusing results, so make sure the code you are moving is incorporating and possible new changes that landed there since this was posted.
Yeah, I had rebased and was going to resend, but you reviewed the original before I got around to it.
In the past the network driver was (mistakenly) being called for all interfaces, not just those of type='network', and so it had a chance to validate all interface configs after the actual type of the interface was known. But since the network driver has been more completely/properly separated from qemu, the network driver isn't called during the startup of any interfaces except those with type='network', so this validation no longer takes place for, e.g. <interface type='bridge'> (or direct, etc). This in turn meant that a config could erroneously specify a vlan tag, or bandwidth settings, for a type of interface that didn't support it, and the domain would start without complaint, just silently ignoring those settings. This patch moves those validation checks out of the network driver, and into virDomainNetDefRuntimeValidate() so they will be done for all interfaces, not just type='network'. https://bugzilla.redhat.com/1741121 Signed-off-by: Laine Stump <laine@redhat.com> --- src/conf/domain_conf.c | 36 +++++++++++++++++++++++++++++++++++- src/network/bridge_driver.c | 37 ------------------------------------- 2 files changed, 35 insertions(+), 38 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c063f40a4c..59a75e67ed 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6110,9 +6110,43 @@ virDomainNetDefRuntimeValidate(const virDomainNetDef *net G_GNUC_UNUSED) * is allowed for a type of interface), but *not* * hypervisor-specific things. */ + char macstr[VIR_MAC_STRING_BUFLEN]; + virDomainNetType actualType = virDomainNetGetActualType(net); + const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net); + const virNetDevBandwidth *bandwidth = virDomainNetGetActualBandwidth(net); - return 0; + virMacAddrFormat(&net->mac, macstr); + if (virDomainNetGetActualVlan(net)) { + /* vlan configuration via libvirt is only supported for PCI + * Passthrough SR-IOV devices (hostdev or macvtap passthru + * mode) and openvswitch bridges. Otherwise log an error and + * fail + */ + if (!(actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV || + (actualType == VIR_DOMAIN_NET_TYPE_DIRECT && + virDomainNetGetActualDirectMode(net) == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) || + (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE && + vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("interface %s - vlan tag not supported for this connection type"), + macstr); + return -1; + } + } + + /* bandwidth configuration via libvirt is not supported for + * hostdev network devices + */ + if (bandwidth && actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("interface %s - bandwidth settings are not supported " + "for hostdev interfaces"), + macstr); + return -1; + } + + return 0; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 85e568e888..1c47485b9d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4793,43 +4793,6 @@ networkAllocatePort(virNetworkObjPtr obj, if (virNetDevVPortProfileCheckComplete(port->virtPortProfile, true) < 0) goto cleanup; - /* make sure that everything now specified for the device is - * actually supported on this type of network. NB: network, - * netdev, and iface->data.network.actual may all be NULL. - */ - VIR_DEBUG("Sanity check port config"); - - if (port->vlan.nTags) { - /* vlan configuration via libvirt is only supported for PCI - * Passthrough SR-IOV devices (hostdev or macvtap passthru - * mode) and openvswitch bridges. Otherwise log an error and - * fail - */ - if (!(port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI || - (port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_DIRECT && - port->plug.direct.mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) || - (port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE && - port->virtPortProfile && - port->virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("an interface connecting to network '%s' " - "is requesting a vlan tag, but that is not " - "supported for this type of network"), - netdef->name); - goto cleanup; - } - } - - /* bandwidth configuration via libvirt is not supported for - * hostdev network devices - */ - if (port->bandwidth && port->plugtype == VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("bandwidth settings are not supported " - "for hostdev interfaces")); - goto cleanup; - } - netdef->connections++; if (dev) dev->connections++; -- 2.21.0
ping. On 10/22/19 9:24 PM, Laine Stump wrote:
As usual, the first 6 patches are just torquing stuff around to make the last patch simple (except that patch 1 makes interface validation errors more useful). 2-5 are just converting a bunch of accessor functions to tak/return const pointers, since that's what's available in the places where I wanted to use them.
This does actually fix a documented bug:
https://bugzilla.redhat.com/1741121
(that one is about vlan tag validation. There may also be one about bandwidth, but I didn't see it right away, so I gave up, as is my custom).
Laine Stump (7): qemu: add mac address to error messages in qemuDomainValidateActualNetDef conf: make virDomainNetGetActualVlan arg/return val const conf: make virDomainNetGetActualBandwidth arg/return value const conf: return a const from virDomainNetGetActualVirtPortProfile conf: change args/return values of remaining virDomainNetGetActual*() to const conf: add hypervisor agnostic, domain start-time, validation function for NetDef net/qemu: move vlan/bandwidth validation out of network driver
src/conf/domain_conf.c | 78 ++++++++++++++++++++++++---- src/conf/domain_conf.h | 21 ++++---- src/conf/netdev_bandwidth_conf.c | 2 +- src/conf/netdev_bandwidth_conf.h | 2 +- src/conf/netdev_vport_profile_conf.c | 2 +- src/conf/netdev_vport_profile_conf.h | 2 +- src/libvirt_private.syms | 1 + src/libxl/libxl_conf.c | 6 +-- src/libxl/libxl_domain.c | 4 ++ src/libxl/libxl_driver.c | 4 ++ src/libxl/xen_common.c | 4 +- src/lxc/lxc_driver.c | 8 ++- src/lxc/lxc_process.c | 16 +++--- src/network/bridge_driver.c | 37 ------------- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 35 ++++++++----- src/qemu/qemu_hotplug.c | 6 +-- src/qemu/qemu_migration_cookie.c | 2 +- src/qemu/qemu_process.c | 2 +- src/util/virhostdev.c | 8 +-- src/util/virnetdev.c | 4 +- src/util/virnetdev.h | 2 +- src/util/virnetdevbandwidth.c | 6 +-- src/util/virnetdevbandwidth.h | 4 +- src/util/virnetdevmacvlan.c | 20 +++---- src/util/virnetdevmacvlan.h | 10 ++-- src/util/virnetdevmidonet.c | 4 +- src/util/virnetdevmidonet.h | 4 +- src/util/virnetdevopenvswitch.c | 8 +-- src/util/virnetdevopenvswitch.h | 6 +-- src/util/virnetdevtap.c | 12 ++--- src/util/virnetdevtap.h | 12 ++--- src/util/virnetdevvportprofile.c | 12 ++--- src/util/virnetdevvportprofile.h | 12 ++--- tests/bhyvexml2argvmock.c | 4 +- 35 files changed, 206 insertions(+), 156 deletions(-)
participants (3)
-
Cole Robinson -
Laine Stump -
Laine Stump