Xen only supports a single type of PCI hostdev assignment, so it is superfluous to have <driver name='xen'/> peppered throughout the config. It *is* necessary to have the driver type explicitly set in the hosdev object before calling into the hypervisor-agnostic "hostdev manager" though (otherwise the hostdev manager doesn't know whether it should do Xen-specific setup, or VFIO-specific setup). Historically, the Xen driver has checked for "default" driver type (i.e. not set in the XML), and set it to "xen', during the XML postparse, thus guaranteeing that it will be set by the time the object is sent to the hostdev manager at runtime, but also setting it so early that a simple round-trip of parse-format results in the XML always containing an explicit <driver name='xen'/>, even if that wasn't specified in the original XML. The QEMU driver *doesn't* set driver.type during postparse though; instead, it waits until domain startup time (or device attach time for hotplug), and sets the driver.type then. The result is that a parse-format round trip of the XML in the QEMU driver *doesn't* add in the <driver name='vfio'/>. This patch modifies the Xen code to behave similarly to the QEMU code - the PostParse just checks for a driver.type that isn't supported by the Xen driver, and any explicit setting to "xen" is deferred until domain runtime rather than during the postparse. This delayed setting of driver.type of course results in slightly different xml2xml parse-format results, so the unit test data is modified accordingly. Signed-off-by: Laine Stump <laine@xxxxxxxxxx> --- src/libxl/libxl_domain.c | 65 +++++++++++++++---- src/libxl/libxl_driver.c | 25 ++++--- tests/libxlxml2domconfigdata/moredevs-hvm.xml | 1 - tests/xlconfigdata/test-fullvirt-pci.xml | 2 - tests/xmconfigdata/test-pci-dev-syntax.xml | 2 - tests/xmconfigdata/test-pci-devs.xml | 2 - 6 files changed, 69 insertions(+), 28 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 22482adfa6..ecdf57e655 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -160,8 +160,15 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDef *dev, if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - pcisrc->driver.type == VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_DEFAULT) - pcisrc->driver.type = VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_XEN; + pcisrc->driver.type != VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_DEFAULT && + pcisrc->driver.type != VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_XEN) { + + /* Xen only supports "Xen" style of hostdev, of course! */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("XEN does not support device assignment mode '%1$s'"), + virDeviceHostdevPCIDriverTypeToString(pcisrc->driver.type)); + return -1; + } } if (dev->type == VIR_DOMAIN_DEVICE_VIDEO) { @@ -986,18 +993,9 @@ libxlNetworkPrepareDevices(virDomainDef *def) if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV && net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { /* Each type='hostdev' network device must also have a - * corresponding entry in the hostdevs array. For netdevs - * that are hardcoded as type='hostdev', this is already - * done by the parser, but for those allocated from a - * network / determined at runtime, we need to do it - * separately. + * corresponding entry in the hostdevs array. */ virDomainHostdevDef *hostdev = virDomainNetGetActualHostdev(net); - virDomainHostdevSubsysPCI *pcisrc = &hostdev->source.subsys.u.pci; - - if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - pcisrc->driver.type = VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_XEN; if (virDomainHostdevInsert(def, hostdev) < 0) return -1; @@ -1007,6 +1005,46 @@ libxlNetworkPrepareDevices(virDomainDef *def) return 0; } + +static int +libxlHostdevPrepareDevices(virDomainDef *def) +{ + size_t i; + + for (i = 0; i < def->nhostdevs; i++) { + virDomainHostdevDef *hostdev = def->hostdevs[i]; + virDomainHostdevSubsysPCI *pcisrc = &hostdev->source.subsys.u.pci; + + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + pcisrc->driver.type == VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_DEFAULT) { + + + pcisrc->driver.type = VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_XEN; + + /* Q: Why do we set an explicit driverType here even + * though Xen (currently) only supports one driverType for + * PCI hostdev assignment? + * + * A: Setting the driver type *in the domain XML config* + * is optional (and actually pointless at the time of + * writing, since each hypervisor only supports a single + * type of PCI hostdev device assignment). But the + * hypervisor-agnostic virHostdevPrepareDomainDevices(), + * which is called immediately after this function in + * order to do any driver-related setup (e.g. bind the + * host device to a driver kernel driver), requires that + * the driverType be explicitly set (otherwise it wouldn't + * know whether to bind to the Xen driver or VFIO driver, + * for example). + */ + } + } + + return 0; +} + + static void libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback) { @@ -1174,6 +1212,9 @@ libxlDomainStartPrepare(libxlDriverPrivate *driver, if (libxlNetworkPrepareDevices(vm->def) < 0) goto error; + if (libxlHostdevPrepareDevices(vm->def) < 0) + goto error; + if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME, vm->def, hostdev_flags) < 0) goto error; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 7f12b11b8d..1636233346 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3087,6 +3087,22 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivate *driver, VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1); + /* The only supported driverType for Xen is + * VIR_DOMAIN_HOSTDEV_PCI_DRIVER_TYPE_XEN, which normally isn't + * set in the config (because it doesn't need to be), but it does + * need to be set for the impending call to + * virHostdevPreparePCIDevices() + */ + if (pcisrc->driver.type == VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_DEFAULT) + pcisrc->driver.type = VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_XEN; + + if (pcisrc->driver.type != VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_XEN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("XEN does not support device assignment mode '%1$s'"), + virDeviceHostdevPCIDriverTypeToString(pcisrc->driver.type)); + goto cleanup; + } + if (virHostdevPreparePCIDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME, vm->def->name, vm->def->uuid, &hostdev, 1, 0) < 0) @@ -3395,15 +3411,6 @@ libxlDomainAttachNetDevice(libxlDriverPrivate *driver, if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { virDomainHostdevDef *hostdev = virDomainNetGetActualHostdev(net); - virDomainHostdevSubsysPCI *pcisrc = &hostdev->source.subsys.u.pci; - - /* For those just allocated from a network pool whose driver type is - * still VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_DEFAULT, we need to set - * driver type correctly. - */ - if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - pcisrc->driver.type = VIR_DEVICE_HOSTDEV_PCI_DRIVER_TYPE_XEN; /* This is really a "smart hostdev", so it should be attached * as a hostdev (the hostdev code will reach over into the diff --git a/tests/libxlxml2domconfigdata/moredevs-hvm.xml b/tests/libxlxml2domconfigdata/moredevs-hvm.xml index 89ad80631d..64eeeff889 100644 --- a/tests/libxlxml2domconfigdata/moredevs-hvm.xml +++ b/tests/libxlxml2domconfigdata/moredevs-hvm.xml @@ -43,7 +43,6 @@ </interface> <interface type='hostdev' managed='yes'> <mac address='00:16:3e:2e:e7:fc'/> - <driver name='xen'/> <source> <address type='pci' domain='0x0000' bus='0x0a' slot='0x10' function='0x0'/> </source> diff --git a/tests/xlconfigdata/test-fullvirt-pci.xml b/tests/xlconfigdata/test-fullvirt-pci.xml index 75aa6ac25b..6826d14b9e 100644 --- a/tests/xlconfigdata/test-fullvirt-pci.xml +++ b/tests/xlconfigdata/test-fullvirt-pci.xml @@ -37,13 +37,11 @@ <model type='cirrus' vram='8192' heads='1' primary='yes'/> </video> <hostdev mode='subsystem' type='pci' managed='no'> - <driver name='xen'/> <source> <address domain='0x0000' bus='0x01' slot='0x1a' function='0x1'/> </source> </hostdev> <hostdev mode='subsystem' type='pci' managed='no'> - <driver name='xen'/> <source writeFiltering='no'> <address domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> </source> diff --git a/tests/xmconfigdata/test-pci-dev-syntax.xml b/tests/xmconfigdata/test-pci-dev-syntax.xml index 5d5d29c61c..1d5d857072 100644 --- a/tests/xmconfigdata/test-pci-dev-syntax.xml +++ b/tests/xmconfigdata/test-pci-dev-syntax.xml @@ -55,13 +55,11 @@ <model type='cirrus' vram='8192' heads='1' primary='yes'/> </video> <hostdev mode='subsystem' type='pci' managed='no'> - <driver name='xen'/> <source> <address domain='0x0001' bus='0x0c' slot='0x1b' function='0x2'/> </source> </hostdev> <hostdev mode='subsystem' type='pci' managed='no'> - <driver name='xen'/> <source> <address domain='0x0000' bus='0x01' slot='0x13' function='0x0'/> </source> diff --git a/tests/xmconfigdata/test-pci-devs.xml b/tests/xmconfigdata/test-pci-devs.xml index 5d5d29c61c..1d5d857072 100644 --- a/tests/xmconfigdata/test-pci-devs.xml +++ b/tests/xmconfigdata/test-pci-devs.xml @@ -55,13 +55,11 @@ <model type='cirrus' vram='8192' heads='1' primary='yes'/> </video> <hostdev mode='subsystem' type='pci' managed='no'> - <driver name='xen'/> <source> <address domain='0x0001' bus='0x0c' slot='0x1b' function='0x2'/> </source> </hostdev> <hostdev mode='subsystem' type='pci' managed='no'> - <driver name='xen'/> <source> <address domain='0x0000' bus='0x01' slot='0x13' function='0x0'/> </source> -- 2.41.0 _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx