On Tue, Aug 30, 2022 at 12:48:49PM +0100, Daniel P. Berrangé wrote:
On Wed, Aug 17, 2022 at 02:50:39PM +0200, Martin Kletzander wrote:This represents an interface connected to a VMWare Distributed Switch, previously obscured as a dummy interface. Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> --- docs/formatdomain.rst | 30 +++++++++++---- src/ch/ch_monitor.c | 1 + src/conf/domain_conf.c | 64 +++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++ src/conf/netdev_bandwidth_conf.c | 1 + src/conf/schemas/domaincommon.rng | 23 +++++++++++ src/libxl/libxl_conf.c | 1 + src/libxl/xen_common.c | 1 + src/lxc/lxc_controller.c | 1 + src/lxc/lxc_driver.c | 3 ++ src/lxc/lxc_process.c | 2 + src/qemu/qemu_command.c | 4 ++ src/qemu/qemu_domain.c | 1 + src/qemu/qemu_hotplug.c | 3 ++ src/qemu/qemu_interface.c | 2 + src/qemu/qemu_process.c | 2 + src/qemu/qemu_validate.c | 1 + src/vmx/vmx.c | 1 + tools/virsh-domain.c | 1 + 19 files changed, 141 insertions(+), 8 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index ed0d9c19593b..3a8aa96fdc0a 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -5244,22 +5244,36 @@ Dummy network interface ^^^^^^^^^^^^^^^^^^^^^^^ An unconnected network interface sounds pretty pointless, but can show up for -example with VMWare when libvirt does not have any more information to provide. -Two such scenarios are currently known: +example with VMWare without any specified network to be connected to. +:since:`Since 8.7.0` -1) network interface exists, but is not connected to any existing network -2) the interface is connected to something known as VMWare Distributed Switch +:: + + ... + <devices> + <interface type='dummy'> + <mac address='52:54:00:22:c9:42'/> + </interface> + </devices> + ... -The difference between these two is not (yet?) discoverable by libvirt, so at -least the information gathered from the hypervisor is provided in the -element. :since:`Since 8.7.0` +VMWare Distributed Switch +^^^^^^^^^^^^^^^^^^^^^^^^^ + +Interface can be connected to VMWare Distributed Switch, but since libvirt +cannot provide information about that architecture, the information presented +here is only what can be gathered from the VM configuration. VMs with this +interface type can be created, so that editing of the XML works properly, +however libvirt cannot guarantee that any changes in these parameters will be +valid in the hypervisor. :since:`Since 8.7.0` :: ... <devices> - <interface type='dummy'> + <interface type='vds'> <mac address='52:54:00:22:c9:42'/> + <source switchid='12345678-1234-1234-1234-123456789abc' portid='6' portgroupid='pg-4321' connectionid='12345'/> </interface> </devices> ...@@ -8899,6 +8905,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *vhost_path = NULL; g_autofree char *tap = NULL; g_autofree char *vhost = NULL; + g_autofree char *switchid = NULL; + g_autofree char *connectionid = NULL; const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL; if (!(def = virDomainNetDefNew(xmlopt))) @@ -8932,6 +8940,13 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, portid = virXMLPropString(source_node, "portid"); } + if (def->type == VIR_DOMAIN_NET_TYPE_VDS) { + switchid = virXMLPropString(source_node, "switchid"); + portid = virXMLPropString(source_node, "portid"); + portgroup = virXMLPropString(source_node, "portgroupid"); + connectionid = virXMLPropString(source_node, "connectionid"); + } + if (def->type == VIR_DOMAIN_NET_TYPE_INTERNAL) internal = virXMLPropString(source_node, "name"); @@ -9313,6 +9328,36 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, } break; + case VIR_DOMAIN_NET_TYPE_VDS: + if (!switchid) { + virReportError(VIR_ERR_XML_ERROR, + _("Missing source switchid for interface type '%s'"), + virDomainNetTypeToString(def->type)); + goto error; + } + + if (virUUIDParse(switchid, def->data.vds.switch_id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse switchid '%s'"), switchid); + goto error; + }Report if switch id is missing, which is good.+ + if (virStrToLong_ll(portid, NULL, 0, &def->data.vds.port_id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse portid '%s'"), portid); + goto error; + } + + if (virStrToLong_ll(connectionid, NULL, 0, &def->data.vds.connection_id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse connectionid '%s'"), connectionid); + goto error; + }AFAICT, this may well crash if port id or connedtion id are missing, since it ends up passing NULL to strtol which is not defined to allow NULL IIUC.+ + def->data.vds.portgroup_id = g_steal_pointer(&portgroup);Allows portgroup id to be missing+ + break; + case VIR_DOMAIN_NET_TYPE_ETHERNET: case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_DUMMY: @@ -9495,6 +9540,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, case VIR_DOMAIN_NET_TYPE_HOSTDEV: case VIR_DOMAIN_NET_TYPE_UDP: case VIR_DOMAIN_NET_TYPE_DUMMY: + case VIR_DOMAIN_NET_TYPE_VDS: case VIR_DOMAIN_NET_TYPE_VDPA: break; case VIR_DOMAIN_NET_TYPE_LAST: @@ -23683,7 +23729,21 @@ virDomainNetDefFormat(virBuffer *buf, def->data.vdpa.devicepath); sourceLines++; } + break; + + case VIR_DOMAIN_NET_TYPE_VDS: { + char switchidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(def->data.vds.switch_id, switchidstr); + virBufferEscapeString(buf, "<source switchid='%s'", switchidstr); + virBufferAsprintf(buf, " portid='%lld'", def->data.vds.port_id); + virBufferEscapeString(buf, " portgroupid='%s'", def->data.vds.portgroup_id); + virBufferAsprintf(buf, " connectionid='%lld'", def->data.vds.connection_id);Based on this logic switch id, port id and connection id are mandatory, but missing portgroup_id would be handled gracefully.+ + sourceLines++; + break; + } case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_DUMMY:diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 5d530f957b0d..7f6ea1d8887d 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -3440,6 +3440,29 @@ <ref name="interface-options"/> </group> + <group> + <attribute name="type"> + <value>vds</value> + </attribute> + <interleave> + <element name="source"> + <attribute name="switchid"> + <ref name="UUID"/> + </attribute> + <attribute name="portid"> + <data type="long"/> + </attribute> + <attribute name="portgroupid"> + <data type="string"/> + </attribute> + <attribute name="connectionid"> + <data type="long"/> + </attribute>Indicates all attributes are mandatory even though the code gracefully handles port group ID being missing. Overall there's some error checking missing when parsing I believe, or this needs relaxing for port group ID.
Definitely more checks, I sent them now so that we can make it before release. Thanks for finding this out.
+ </element> + <ref name="interface-options"/> + </interleave> + </group> + </choice> <optional> <attribute name="trustGuestRxFilters">With 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 :|
Attachment:
signature.asc
Description: PGP signature
