On 01/13/2011 03:59 PM, Eric Blake wrote:
On 01/12/2011 11:45 PM, 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,
...
(So far the strongest opinion has been for the current
"name='vhost|qemu'")
Also, using name= as the attribute name matches the fact that we already
have<driver name='xyz'> for<disk>.
Yep, I'm in the club :-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index a524e4b..6d0654d 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -1005,6 +1005,19 @@
</element>
</optional>
<optional>
+<element name="driver">
+<optional>
+<attribute name="name">
Thinking aloud: This permits<driver/> with no attributes, which looks
weird. For comparison with the<disk> element, the rng requires the
driver element has to have at least one attribute from the set (name,
cache), which means name is optional there. Which means on the one
hand,<disk> will never have<driver/> without attributes, but on the
other hand, why should name be mandatory here if it is optional there.
I guess that means this is fine to keep the<optional> here.
And especially since the rng is only used in tests that validate the
rng, I'm inclined to leave it optional...
@@ -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"),
s/Unkown/Unknown/
Right. I think this is the result of the copy-paste source I chose
(search for "Unkown"). I need to send a patch for that.
+ backend);
+ goto error;
+ }
+ def->backend = b;
+ def->backend_specified = 1;
It seems like our debate on IRC has been whether this two-variable
tracking is better than a three-way enum value. If we go with the
three-way enum (default, qemu, vhost), then you only need one variable,
but this line of code needs one extra check that rejects
STREQ(backend,"default") (or else the XML parsing would silently accept
name='default' but discard it).
Good idea. I chose to generate an error on name='default'. Shortens up
the code, eliminates non-zero default; an all around win!
+ *vhostfd = open("/dev/vhost-net", O_RDWR, 0);
The third argument is not necessary, since the second does not include
O_CREAT.
Okay. That's actually pre-existing code that I moved slightly, but I'll
use this oppurtunity to fix it.
I can live with the patch as-is (once you fix the spelling and open()
nits), but if you have time, I wouldn't mind a v2 with the approach of a
tri-state enum. So, you have a weak:
V2 coming up!
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list