Re: [PATCH 2/3] qemu: Process the hostdev "rawio" setting

Michal Privoznik <mprivozn@xxxxxxxxxx> · Thu, 18 Sep 2014 18:50:10 +0200

On 10.09.2014 01:40, John Ferlan wrote:
Mimic the "Disk" processing for 'rawio', but for a scsi_host hostdev
lun device.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
  src/qemu/qemu_domain.c  | 21 +++++++++++++++++++++
  src/qemu/qemu_domain.h  |  4 ++++
  src/qemu/qemu_driver.c  |  1 +
  src/qemu/qemu_process.c | 20 +++++++++++++++++++-
  4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 306ff10..166fadb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1715,6 +1715,10 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver,
      for (i = 0; i < obj->def->ndisks; i++)
          qemuDomainObjCheckDiskTaint(driver, obj, obj->def->disks[i], logFD);

+    for (i = 0; i < obj->def->nhostdevs; i++)
+        qemuDomainObjCheckHostdevTaint(driver, obj, obj->def->hostdevs[i],
+                                       logFD);
+
      for (i = 0; i < obj->def->nnets; i++)
          qemuDomainObjCheckNetTaint(driver, obj, obj->def->nets[i], logFD);

@@ -1741,6 +1745,23 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
  }


+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver,
+                                    virDomainObjPtr obj,
+                                    virDomainHostdevDefPtr hostdev,
+                                    int logFD)
+{
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);

This @cfg is not used anywhere in the function. Well, technically is - 
it's unref'd but besides that, it isn't. Is it a leftover from previous 
code of yours that you were testing?

+    virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+
+    if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
+        scsisrc->rawio == 1)

If you follow my advice from 1/3 then this would be 
s/1/VIR_TRISTATE_BOOL_YES/

+            qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES,
+                               logFD);
+
+    virObjectUnref(cfg);
+}
+
+
  void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver,
                                  virDomainObjPtr obj,
                                  virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index f353d90..7aebb0f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -286,6 +286,10 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
                                   virDomainObjPtr obj,
                                   virDomainDiskDefPtr disk,
                                   int logFD);
+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver,
+                                    virDomainObjPtr obj,
+                                    virDomainHostdevDefPtr disk,
+                                    int logFD);
  void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver,
                                  virDomainObjPtr obj,
                                  virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d724eeb..78ecb3e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6548,6 +6548,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
          break;

      case VIR_DOMAIN_DEVICE_HOSTDEV:
+        qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, -1);
          ret = qemuDomainAttachHostDevice(dom->conn, driver, vm,
                                           dev->data.hostdev);
          if (!ret)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b1d8a32..3544716 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn,
      struct qemuProcessHookData hookData;
      unsigned long cur_balloon;
      size_t i;
+    bool rawio_set = false;
      char *nodeset = NULL;
      virBitmapPtr nodemask = NULL;
      unsigned int stop_flags;
@@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn,
          virDomainDeviceDef dev;
          virDomainDiskDefPtr disk = vm->def->disks[i];

-        if (vm->def->disks[i]->rawio == 1)
+        if (vm->def->disks[i]->rawio == 1) {
  #ifdef CAP_SYS_RAWIO
              virCommandAllowCap(cmd, CAP_SYS_RAWIO);
  #else
              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                             _("Raw I/O is not supported on this platform"));
  #endif
+            rawio_set = true;
+        }

Interesting. So if rawio is requested we shout an error but don't fail 
actually. I think we are missing 'goto cleanup' here.


          dev.type = VIR_DOMAIN_DEVICE_DISK;
          dev.data.disk = disk;
@@ -4139,6 +4142,21 @@ int qemuProcessStart(virConnectPtr conn,
              goto cleanup;
      }

+    /* If rawio not already set, check hostdevs as well */
+    if (!rawio_set) {
+        for (i = 0; i < vm->def->nhostdevs; i++) {
+            virDomainHostdevSubsysSCSIPtr scsisrc =
+                &vm->def->hostdevs[i]->source.subsys.u.scsi;
+            if (scsisrc->rawio == 1)
+#ifdef CAP_SYS_RAWIO
+                virCommandAllowCap(cmd, CAP_SYS_RAWIO);
+#else
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("Raw I/O is not supported on this platform"));
+#endif

And here too then.

+        }
+    }
+
      virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData);
      virCommandSetMaxProcesses(cmd, cfg->maxProcesses);
      virCommandSetMaxFiles(cmd, cfg->maxFiles);


Michal

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







