[libvirt] [PATCH] domain: don't try to interpret <driver> as virtio config for hostdev interfaces

This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1046337 The <driver> type attribute of an interface is interpreted in two different ways depending on the <interface> type - if the interface is type='hostdev', then the driver name describes which backend to use for the hostdev device assignment (vfio or kvm), but if the interface is any emulated type *and* the model type is "virtio", then the driver name can be "vhost" or "qemu", telling which backend qemu should use to communicate with the emulated device. The problem comes when someone has defined a an interface like this (which is accepted by the parser as long as no <driver name='xxx'/> is specified): <interface type='hostdev'> ... <model type='virtio'/> ... </interface> As libvirt storing this definition in the domain's status, the driver name is automatically filled in with the backend that was automatically decided by libvirt, so it stores this in the status: <interface type='hostdev'> ... <driver name='vfio'/> ... <model type='virtio'/> ... </interface> This isn't noticed until the next time libvirtd is restarted - as it is reading the status of all domains, it encounters the above interface definition, logs an error: internal error: Unknown interface <driver name='vfio'> has been specified and fails to reload the domain status, so the domain is marked as inactive. The solution is to stop the parser from interpreting <driver> attributes as if the device was an emulated virtio device, when it is actually a hostdev. (Although the bug has existed since vfio support was added, it has just recently become more apparent because libvirt previously didn't automatically set the driver name for hostdev interfaces in the domain status to vfio/kvm as it does since commit f094aa, first appearing in v1.1.4.) --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 80bc85f..e65f3e3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6752,7 +6752,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, model = NULL; } - if (def->model && STREQ(def->model, "virtio")) { + if (def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && + STREQ_NULLABLE(def->model, "virtio")) { if (backend != NULL) { int name; if ((name = virDomainNetBackendTypeFromString(backend)) < 0 || -- 1.8.3.1

On 12/24/2013 09:26 AM, Laine Stump wrote:
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1046337
The <driver> type attribute of an interface is interpreted in two different ways depending on the <interface> type - if the interface is type='hostdev', then the driver name describes which backend to use for the hostdev device assignment (vfio or kvm), but if the interface is any emulated type *and* the model type is "virtio", then the driver name can be "vhost" or "qemu", telling which backend qemu should use to communicate with the emulated device.
This isn't noticed until the next time libvirtd is restarted - as it is reading the status of all domains, it encounters the above interface definition, logs an error:
internal error: Unknown interface <driver name='vfio'> has been specified
and fails to reload the domain status, so the domain is marked as inactive.
Always annoying when that happens.
The solution is to stop the parser from interpreting <driver> attributes as if the device was an emulated virtio device, when it is actually a hostdev.
(Although the bug has existed since vfio support was added, it has just recently become more apparent because libvirt previously didn't automatically set the driver name for hostdev interfaces in the domain status to vfio/kvm as it does since commit f094aa, first appearing in v1.1.4.)
Thanks for the analysis - in spite of taking a long time to write, a long commit message does make it easier to validate that the patch is right. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Laine Stump