Re: [libvirt] [PATCH 3/4] domain_conf: Introduce <mtu/> to <interface/>

On 01/24/2017 10:40 AM, Michal Privoznik wrote: > So far we allow to set MTU for libvirt networks. However, not all > domain interfaces have to be plugged into a libvirt network and > even if they are, they might want to have a different MTU (e.g. > for testing purposes). ... although setting an MTU larger than a bridge's current MTU will result in the tap device MTU being lowered back to the bridge's MTU anyway, and setting an MTU smaller than the bridge's current MTU will result in the bridge's MTU being clamped to that new lower value. (Still, I think it's reasonable to expect someone someday will want to do that for some reason, so might as well allow it...) > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > docs/formatdomain.html.in | 19 ++++++++ > docs/schemas/domaincommon.rng | 3 ++ > docs/schemas/networkcommon.rng | 9 ++++ > src/conf/domain_conf.c | 10 +++++ > src/conf/domain_conf.h | 1 + > src/qemu/qemu_domain.c | 29 ++++++++++++ > src/qemu/qemu_domain.h | 2 + > tests/qemuxml2argvdata/qemuxml2argv-net-mtu.xml | 60 > +++++++++++++++++++++++++ > 8 files changed, 133 insertions(+) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-mtu.xml >

> diff --git a/docs/schemas/networkcommon.rng > b/docs/schemas/networkcommon.rng > index a334b83e3..26995556d 100644 > --- a/docs/schemas/networkcommon.rng > +++ b/docs/schemas/networkcommon.rng > @@ -260,10 +260,19 @@ > </optional> > </element> > </define> > + > <define name="macTableManager"> > <choice> > <value>kernel</value> > <value>libvirt</value> > </choice> > </define> > + > + <define name="mtu"> > + <element name="mtu"> > + <attribute name="size"> > + <ref name="unsignedShort"/> > + </attribute> > + </element> > + </define> Well, if you're going to add this, then I should use it in my patches, or I should do it and you use it in yours. Our relative timezones (and the fact that I haven't respun my patches yet) dictate that you should push first (anyway, I'm realizing I'll need to add an extra patch to mine for another issue - retrieving the MTU from the network in the case that it's not specified in the domain's interface element directly, similar to what we do with vlan tags).

> </grammar> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 26bb0fdd0..a2b72cb9c 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -10050,6 +10050,12 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr > xmlopt, > goto error; > } > + if (virXPathUInt("string(./mtu/@size)", ctxt, &def->mtu) < -1) { > + virReportError(VIR_ERR_XML_ERROR, "%s", > + _("malformed mtu size")); > + goto error; > + } > + > cleanup: > ctxt->node = oldnode; > VIR_FREE(macaddr); > @@ -21769,6 +21775,10 @@ virDomainNetDefFormat(virBufferPtr buf, > virBufferAsprintf(buf, "<link state='%s'/>\n", > > virDomainNetInterfaceLinkStateTypeToString(def->linkstate)); > } > + > + if (def->mtu) > + virBufferAsprintf(buf, "<mtu size='%u'/>\n", def->mtu); > + > if (virDomainDeviceInfoFormat(buf, &def->info, > flags | > VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT > | VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) > < 0) > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 78a3db4e1..4d830c51d 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1009,6 +1009,7 @@ struct _virDomainNetDef { > virNetDevVlan vlan; > int trustGuestRxFilters; /* enum virTristateBool */ > int linkstate; > + unsigned int mtu; > }; > /* Used for prefix of ifname of any network name generated > dynamically > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 2ed45ab17..26ca89930 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -2868,6 +2868,14 @@ qemuDomainDeviceDefValidate(const > virDomainDeviceDef *dev, > _("rx_queue_size has to be a power of > two")); > goto cleanup; > } > + > + if (net->mtu && > + !qemuDomainNetSupportsMTU(net->type)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("setting MTU on interface type %s is not > supported yet"), > + virDomainNetTypeToString(net->type)); > + goto cleanup; > + } > } > ret = 0; > @@ -6529,6 +6537,27 @@ qemuDomainSupportsNetdev(virDomainDefPtr def, > return virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV); > } > +bool > +qemuDomainNetSupportsMTU(virDomainNetType type) > +{ > + switch (type) { > + case VIR_DOMAIN_NET_TYPE_USER: > + case VIR_DOMAIN_NET_TYPE_ETHERNET: > + case VIR_DOMAIN_NET_TYPE_VHOSTUSER: > + case VIR_DOMAIN_NET_TYPE_SERVER: > + case VIR_DOMAIN_NET_TYPE_CLIENT: > + case VIR_DOMAIN_NET_TYPE_MCAST: > + case VIR_DOMAIN_NET_TYPE_NETWORK: > + case VIR_DOMAIN_NET_TYPE_BRIDGE: > + case VIR_DOMAIN_NET_TYPE_INTERNAL: > + case VIR_DOMAIN_NET_TYPE_DIRECT: > + case VIR_DOMAIN_NET_TYPE_HOSTDEV: > + case VIR_DOMAIN_NET_TYPE_UDP: > + case VIR_DOMAIN_NET_TYPE_LAST: > + break; > + } > + return false; > +} > int > qemuDomainNetVLAN(virDomainNetDefPtr def) > diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h > index 88b586972..041149167 100644 > --- a/src/qemu/qemu_domain.h > +++ b/src/qemu/qemu_domain.h > @@ -708,6 +708,8 @@ bool qemuDomainSupportsNetdev(virDomainDefPtr def, > virQEMUCapsPtr qemuCaps, > virDomainNetDefPtr net); > +bool qemuDomainNetSupportsMTU(virDomainNetType type); > + > int qemuDomainNetVLAN(virDomainNetDefPtr def); > int qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-mtu.xml > b/tests/qemuxml2argvdata/qemuxml2argv-net-mtu.xml > new file mode 100644 > index 000000000..606322463 > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-mtu.xml You added this test case, but didn't reference it in qemuxml2xmltest.c (or add the corresponding file in qemuxml2xmloutdata). Ah, I see you did add it in patch 4/4, but I think the xml2xml test should be added in the same patch that the conf changes are made.

ACK, if you move the xml2xml test case to this patch.