[libvirt] [PATCH 01/12] qemu: Drop KVM assignment

Daniel Henrique Barboza danielhb413 at gmail.com
Tue Aug 20 17:49:16 UTC 2019



On 8/20/19 1:52 PM, Daniel Henrique Barboza wrote:
>
>
> On 8/20/19 11:30 AM, Michal Privoznik wrote:
>> KVM style of PCI devices assignment was dropped in kernel in
>> favor of vfio pci (see kernel commit v4.12-rc1~68^2~65). Since
>> vfio is around for quite some time now and is far superior
>> discourage people in using KVM style.
>>
>> Ideally, I'd make QEMU_CAPS_VFIO_PCI implicitly assumed but turns
>> out qemu-3.0.0 doesn't support vfio-pci device for RISC-V.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---


Tested-by: Daniel Henrique Barboza <danielhb413 at gmail.com>


After taking care of that commit message nit I've pointed out below:


Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>


>>   src/qemu/qemu_capabilities.c |  6 -----
>>   src/qemu/qemu_command.c      | 26 ++-------------------
>>   src/qemu/qemu_command.h      |  1 -
>>   src/qemu/qemu_driver.c       | 14 ++++--------
>>   src/qemu/qemu_hostdev.c      | 44 +++---------------------------------
>>   src/qemu/qemu_hostdev.h      |  1 -
>>   src/qemu/qemu_hotplug.c      | 20 ++--------------
>>   tests/domaincapstest.c       |  3 +--
>>   8 files changed, 12 insertions(+), 103 deletions(-)
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index c9677315ab..73300128ea 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -5338,7 +5338,6 @@ static int
>>   virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps,
>> virDomainCapsDeviceHostdevPtr hostdev)
>>   {
>> -    bool supportsPassthroughKVM = 
>> qemuHostdevHostSupportsPassthroughLegacy();
>>       bool supportsPassthroughVFIO = 
>> qemuHostdevHostSupportsPassthroughVFIO();
>>         hostdev->supported = VIR_TRISTATE_BOOL_YES;
>> @@ -5374,11 +5373,6 @@ 
>> virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps,
>> VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO);
>>       }
>>   -    if (supportsPassthroughKVM) {
>> -        VIR_DOMAIN_CAPS_ENUM_SET(hostdev->pciBackend,
>> - VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT,
>> - VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM);
>> -    }
>>       return 0;
>>   }
>>   diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index e3f4ef624b..f7b5430db8 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -4904,7 +4904,6 @@ char *
>>   qemuBuildPCIHostdevDevStr(const virDomainDef *def,
>>                             virDomainHostdevDefPtr dev,
>>                             unsigned int bootIndex, /* used iff 
>> dev->info->bootIndex == 0 */
>> -                          const char *configfd,
>>                             virQEMUCapsPtr qemuCaps)
>>   {
>>       virBuffer buf = VIR_BUFFER_INITIALIZER;
>> @@ -4913,16 +4912,11 @@ qemuBuildPCIHostdevDevStr(const virDomainDef 
>> *def,
>>         /* caller has to assign proper passthrough backend type */
>>       switch ((virDomainHostdevSubsysPCIBackendType)backend) {
>> -    case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM:
>> -        virBufferAddLit(&buf, "pci-assign");
>> -        if (configfd && *configfd)
>> -            virBufferAsprintf(&buf, ",configfd=%s", configfd);
>> -        break;
>> -
>>       case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO:
>>           virBufferAddLit(&buf, "vfio-pci");
>>           break;
>>   +    case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM:
>>       case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
>>       case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN:
>>       case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST:
>> @@ -5676,7 +5670,6 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
>>                   }
>>               }
>>   -            char *configfd_name = NULL;
>>               unsigned int bootIndex = hostdev->info->bootIndex;
>>                 /* bootNet will be non-0 if boot order was set and no 
>> other
>> @@ -5686,27 +5679,12 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
>>                   bootIndex = *bootHostdevNet;
>>                   *bootHostdevNet = 0;
>>               }
>> -            if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
>> -                int configfd = qemuOpenPCIConfig(hostdev);
>> -
>> -                if (configfd >= 0) {
>> -                    if (virAsprintf(&configfd_name, "%d", configfd) 
>> < 0) {
>> -                        VIR_FORCE_CLOSE(configfd);
>> -                        return -1;
>> -                    }
>> -
>> -                    virCommandPassFD(cmd, configfd,
>> - VIR_COMMAND_PASS_FD_CLOSE_PARENT);
>> -                }
>> -            }
>>                 if (qemuCommandAddExtDevice(cmd, hostdev->info) < 0)
>>                   return -1;
>>                 virCommandAddArg(cmd, "-device");
>> -            devstr = qemuBuildPCIHostdevDevStr(def, hostdev, bootIndex,
>> -                                               configfd_name, 
>> qemuCaps);
>> -            VIR_FREE(configfd_name);
>> +            devstr = qemuBuildPCIHostdevDevStr(def, hostdev, 
>> bootIndex, qemuCaps);
>>               if (!devstr)
>>                   return -1;
>>               virCommandAddArg(cmd, devstr);
>> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
>> index 7e2dc5a60a..e3983bedb2 100644
>> --- a/src/qemu/qemu_command.h
>> +++ b/src/qemu/qemu_command.h
>> @@ -148,7 +148,6 @@ char 
>> *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
>>   char *qemuBuildPCIHostdevDevStr(const virDomainDef *def,
>>                                   virDomainHostdevDefPtr dev,
>>                                   unsigned int bootIndex,
>> -                                const char *configfd,
>>                                   virQEMUCapsPtr qemuCaps);
>>     char *qemuBuildRNGDevStr(const virDomainDef *def,
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 11f97dbc65..eb373c14d7 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -13517,7 +13517,6 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
>>       int ret = -1;
>>       virNodeDeviceDefPtr def = NULL;
>>       char *xml = NULL;
>> -    bool legacy = qemuHostdevHostSupportsPassthroughLegacy();
>>       bool vfio = qemuHostdevHostSupportsPassthroughVFIO();
>>       virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
>>   @@ -13544,8 +13543,6 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr 
>> dev,
>>       if (!driverName) {
>>           if (vfio) {
>>               driverName = "vfio";
>> -        } else if (legacy) {
>> -            driverName = "kvm";
>>           } else {
>>               virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>>                              _("neither VFIO nor KVM device 
>> assignment is "
>
> Right here comes the error message:
>
>             virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>                            _("neither VFIO nor KVM device assignment is "
>                              "currently supported on this system"));
>             goto cleanup;
>
> I think this message should refer only to VFIO in this case - there's
> already a KVM device assignment message down below.
>
>
> Thanks,
>
>
> DHB
>
>
>> @@ -13563,13 +13560,10 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr 
>> dev,
>>           }
>>           virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO);
>>       } else if (STREQ(driverName, "kvm")) {
>> -        if (!legacy) {
>> -            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>> -                           _("KVM device assignment is currently not "
>> -                             "supported on this system"));
>> -            goto cleanup;
>> -        }
>> -        virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_KVM);
>> +        virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>> +                       _("KVM device assignment is currently not "
>> +                         "supported on this system"));
>> +        goto cleanup;
>>       } else {
>>           virReportError(VIR_ERR_INVALID_ARG,
>>                          _("unknown driver name '%s'"), driverName);
>> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
>> index 92b037e1ed..af41c32679 100644
>> --- a/src/qemu/qemu_hostdev.c
>> +++ b/src/qemu/qemu_hostdev.c
>> @@ -133,44 +133,11 @@ qemuHostdevHostSupportsPassthroughVFIO(void)
>>   }
>>     -#if HAVE_LINUX_KVM_H
>> -# include <linux/kvm.h>
>> -bool
>> -qemuHostdevHostSupportsPassthroughLegacy(void)
>> -{
>> -    int kvmfd = -1;
>> -    bool ret = false;
>> -
>> -    if ((kvmfd = open("/dev/kvm", O_RDONLY)) < 0)
>> -        goto cleanup;
>> -
>> -# ifdef KVM_CAP_IOMMU
>> -    if ((ioctl(kvmfd, KVM_CHECK_EXTENSION, KVM_CAP_IOMMU)) <= 0)
>> -        goto cleanup;
>> -
>> -    ret = true;
>> -# endif
>> -
>> - cleanup:
>> -    VIR_FORCE_CLOSE(kvmfd);
>> -
>> -    return ret;
>> -}
>> -#else
>> -bool
>> -qemuHostdevHostSupportsPassthroughLegacy(void)
>> -{
>> -    return false;
>> -}
>> -#endif
>> -
>> -
>>   static bool
>> qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDefPtr 
>> *hostdevs,
>>                                            size_t nhostdevs,
>>                                            virQEMUCapsPtr qemuCaps)
>>   {
>> -    bool supportsPassthroughKVM = 
>> qemuHostdevHostSupportsPassthroughLegacy();
>>       bool supportsPassthroughVFIO = 
>> qemuHostdevHostSupportsPassthroughVFIO();
>>       size_t i;
>>   @@ -189,8 +156,6 @@ 
>> qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDefPtr 
>> *hostdevs,
>>               if (supportsPassthroughVFIO &&
>>                   virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) {
>>                   *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO;
>> -            } else if (supportsPassthroughKVM) {
>> -                *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM;
>>               } else {
>>                   virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>                                  _("host doesn't support passthrough 
>> of "
>> @@ -209,12 +174,9 @@ 
>> qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDefPtr 
>> *hostdevs,
>>               break;
>>             case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM:
>> -            if (!supportsPassthroughKVM) {
>> -                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> -                               _("host doesn't support legacy PCI 
>> passthrough"));
>> -                return false;
>> -            }
>> -
>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                           _("host doesn't support legacy PCI 
>> passthrough"));
>> +            return false;
>>               break;
>>             case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN:
>> diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
>> index f6d76c1c2a..e99c204961 100644
>> --- a/src/qemu/qemu_hostdev.h
>> +++ b/src/qemu/qemu_hostdev.h
>> @@ -24,7 +24,6 @@
>>   #include "qemu_conf.h"
>>   #include "domain_conf.h"
>>   -bool qemuHostdevHostSupportsPassthroughLegacy(void);
>>   bool qemuHostdevHostSupportsPassthroughVFIO(void);
>>     int qemuHostdevUpdateActiveMediatedDevices(virQEMUDriverPtr driver,
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index d8be63b71c..63acb9c451 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -1465,8 +1465,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr 
>> driver,
>>       virDomainDeviceInfoPtr info = hostdev->info;
>>       int ret;
>>       char *devstr = NULL;
>> -    int configfd = -1;
>> -    char *configfd_name = NULL;
>>       bool releaseaddr = false;
>>       bool teardowncgroup = false;
>>       bool teardownlabel = false;
>> @@ -1547,13 +1545,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr 
>> driver,
>>       if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
>>           goto error;
>>       releaseaddr = true;
>> -    if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
>> -        configfd = qemuOpenPCIConfig(hostdev);
>> -        if (configfd >= 0) {
>> -            if (virAsprintf(&configfd_name, "fd-%s", info->alias) < 0)
>> -                goto error;
>> -        }
>> -    }
>>         if (!virDomainObjIsActive(vm)) {
>>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> @@ -1561,8 +1552,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr 
>> driver,
>>           goto error;
>>       }
>>   -    if (!(devstr = qemuBuildPCIHostdevDevStr(vm->def, hostdev, 0,
>> -                                             configfd_name, 
>> priv->qemuCaps)))
>> +    if (!(devstr = qemuBuildPCIHostdevDevStr(vm->def, hostdev, 0, 
>> priv->qemuCaps)))
>>           goto error;
>>         qemuDomainObjEnterMonitor(driver, vm);
>> @@ -1570,10 +1560,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr 
>> driver,
>>       if ((ret = qemuDomainAttachExtensionDevice(priv->mon, 
>> hostdev->info)) < 0)
>>           goto exit_monitor;
>>   -    if ((ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr,
>> -                                          configfd, configfd_name)) 
>> < 0) {
>> +    if ((ret = qemuMonitorAddDevice(priv->mon, devstr)) < 0)
>> ignore_value(qemuDomainDetachExtensionDevice(priv->mon, hostdev->info));
>> -    }
>>      exit_monitor:
>>       if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> @@ -1586,8 +1574,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr 
>> driver,
>>       vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
>>         VIR_FREE(devstr);
>> -    VIR_FREE(configfd_name);
>> -    VIR_FORCE_CLOSE(configfd);
>>         return 0;
>>   @@ -1607,8 +1593,6 @@ 
>> qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
>>       qemuHostdevReAttachPCIDevices(driver, vm->def->name, &hostdev, 1);
>>         VIR_FREE(devstr);
>> -    VIR_FREE(configfd_name);
>> -    VIR_FORCE_CLOSE(configfd);
>>      cleanup:
>>       return -1;
>> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
>> index 45ecfe61ac..77acefa6b0 100644
>> --- a/tests/domaincapstest.c
>> +++ b/tests/domaincapstest.c
>> @@ -111,8 +111,7 @@ fillQemuCaps(virDomainCapsPtr domCaps,
>>                                     cfg->nfirmwares) < 0)
>>           goto cleanup;
>>   -    /* The function above tries to query host's KVM & VFIO 
>> capabilities by
>> -     * calling qemuHostdevHostSupportsPassthroughLegacy() and
>> +    /* The function above tries to query host's VFIO capabilities by 
>> calling
>>        * qemuHostdevHostSupportsPassthroughVFIO() which, however, 
>> can't be
>>        * successfully mocked as they are not exposed as internal 
>> APIs. Therefore,
>>        * instead of mocking set the expected values here by hand. */
>




More information about the libvir-list mailing list