On Thu, Jan 13, 2011 at 01:45:28AM -0500, Laine Stump wrote:
This patch is in response to
https://bugzilla.redhat.com/show_bug.cgi?id=643050
The existing libvirt support for the vhost-net backend to the virtio network driver happens automatically - if the vhost-net device is available, it is always enabled, otherwise the standard userland virtio backend is used.
This patch makes it possible to force whether or not vhost-net is used with a bit of XML. Adding a <driver> element to the interface XML, eg:
<interface type="network"> <model type="virtio"/> <driver name="vhost"/>
will force use of vhost-net (if it's not available, the domain will fail to start). if driver name="qemu", vhost-net will not be used even if it is available.
If there is no <driver name='xxx'/> in the config, libvirt will revert to the pre-existing automatic behavior - use vhost-net if it's available, and userland backend if vhost-net isn't available. ---
Note that I don't really like the "name='vhost|qemu'" nomenclature, but am including it here just to get the patches on the list. I could live with it this way, or with any of the following (anyone have a strong opinion?) (note that in all cases, nothing specified means "try to use vhost, but fallback to userland if necessary")
vhost='on|off' vhost='required|disabled' mode='vhost|qemu' mode='kernel|user' backend='kernel|user'
I wanted 'name=' becasue that matches <driver> usage in <disk>. This rules out the first options completely, since we can just play with attribute values. kernel|user is nice, except when QEMU invent vhost2, and now 'kernel' is no longer unique :-( Also we used 'name=qemu' already in <disk> to refer to the in-QEMU disk backend, and there's likely to be a 'vhost' backend for disks too in the future. So in summary I think 'name=vhost|qemu' is the best optionl.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b4df38c..04ed502 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -184,6 +184,10 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST, "internal", "direct")
+VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, + "qemu", + "vhost") + VIR_ENUM_IMPL(virDomainChrChannelTarget, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, "guestfwd", @@ -2289,6 +2293,7 @@ virDomainNetDefParseXML(virCapsPtr caps, char *address = NULL; char *port = NULL; char *model = NULL; + char *backend = NULL; char *filter = NULL; char *internal = NULL; char *devaddr = NULL; @@ -2371,6 +2376,8 @@ virDomainNetDefParseXML(virCapsPtr caps, script = virXMLPropString(cur, "path"); } else if (xmlStrEqual (cur->name, BAD_CAST "model")) { model = virXMLPropString(cur, "type"); + } else if (xmlStrEqual (cur->name, BAD_CAST "driver")) { + backend = virXMLPropString(cur, "name"); } else if (xmlStrEqual (cur->name, BAD_CAST "filterref")) { filter = virXMLPropString(cur, "filter"); VIR_FREE(filterparams); @@ -2558,6 +2565,18 @@ virDomainNetDefParseXML(virCapsPtr caps, model = NULL; }
+ if ((backend != NULL) && + (def->model && STREQ(def->model, "virtio"))) { + int b; + if ((b = virDomainNetBackendTypeFromString(backend)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("Unkown interface <driver name='%s'> has been specified"), + backend); + goto error; + } + def->backend = b; + def->backend_specified = 1; + } if (filter != NULL) { switch (def->type) { case VIR_DOMAIN_NET_TYPE_ETHERNET: @@ -2584,6 +2603,7 @@ cleanup: VIR_FREE(script); VIR_FREE(bridge); VIR_FREE(model); + VIR_FREE(backend); VIR_FREE(filter); VIR_FREE(type); VIR_FREE(internal); @@ -6275,9 +6295,14 @@ virDomainNetDefFormat(virBufferPtr buf, if (def->ifname) virBufferEscapeString(buf, " <target dev='%s'/>\n", def->ifname); - if (def->model) + if (def->model) { virBufferEscapeString(buf, " <model type='%s'/>\n", def->model); + if (STREQ(def->model, "virtio") && def->backend_specified) { + virBufferVSprintf(buf, " <driver name='%s'/>\n", + virDomainNetBackendTypeToString(def->backend)); + } + } if (def->filter) { virBufferEscapeString(buf, " <filterref filter='%s'", def->filter); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a459a22..451ccad 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -292,6 +292,13 @@ enum virDomainNetType { VIR_DOMAIN_NET_TYPE_LAST, };
+/* the backend driver used for virtio interfaces */ +enum virDomainNetBackendType { + VIR_DOMAIN_NET_BACKEND_TYPE_QEMU, /* userland */ + VIR_DOMAIN_NET_BACKEND_TYPE_VHOST, /* kernel */ + + VIR_DOMAIN_NET_BACKEND_TYPE_LAST, +};
/* the mode type for macvtap devices */ enum virDomainNetdevMacvtapType { @@ -310,6 +317,8 @@ struct _virDomainNetDef { enum virDomainNetType type; unsigned char mac[VIR_MAC_BUFLEN]; char *model; + enum virDomainNetBackendType backend; + int backend_specified : 1;
I don't really like this pattern since it is at odds with the way we handle defaults elsewhere which is to include the default as the first enum value, eg enum virDomainNetBackendType { VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT, /* Try best available */ VIR_DOMAIN_NET_BACKEND_TYPE_QEMU, /* userland */ VIR_DOMAIN_NET_BACKEND_TYPE_VHOST, /* kernel */ VIR_DOMAIN_NET_BACKEND_TYPE_LAST, }; And then we do if (def->backend) ...print XML... when formatting the XML, so that the 'name=default' never actually appears in the XML output. Regards, Daniel