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

[Daniel Henrique Barboza]

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 <mpriv...@redhat.com>
---

Tested-by: Daniel Henrique Barboza <danielhb...@gmail.com>


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


Reviewed-by: Daniel Henrique Barboza <danielhb...@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. */

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list