On 01/25/2017 03:46 PM, Laine Stump wrote:
> 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@xxxxxxxxxx>
>> ---
>> 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).
I think we are already doing that: virNetDevTapCreateInBridgePort()
calls virNetDevSetMTUFromDevice() which sets MTU on the new tap
interface from the one that bridge has.
>
>> </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.
True and usually I require the xml2xml test to be in the same patch as
the conf/ changes. However, this is an exception: because in the post
parse callback I check whether there's an implementation available for
given interface type. Since there is no implementation available in this
patch, <mtu size=''/> is not allowed and xml2xml test would just error out.
>
> ACK, if you move the xml2xml test case to this patch.
>
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
