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.
> + </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 :|
