Re: [PATCH 5/8 v8] qemu: set unpriv_sgio when domain starting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2012年12月15日 05:36, Eric Blake wrote:
On 12/14/2012 08:43 AM, Osier Yang wrote:
Note that cdbfilter is not exactly match unpriv_sgio's value.
VIR_DOMAIN_DISK_CDB_FILTER_YES mean the kernel will filter the
SCSI commands, which maps to 0 of unpriv_sgio;
VIR_DOMAIN_DISK_CDB_FILTER_NO maps to 1 of unpriv_sgio.

Again, I find this confusing to have negative logic, and think making
rawio a tri-state will make it easier to reason about which disks have
allowed unpriv_sgio vs. the default of being filtered.

---
  src/qemu/qemu_process.c |   11 ++++++++++-
  1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c3ecf6f..bcea0ff 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3730,10 +3730,19 @@ int qemuProcessStart(virConnectPtr conn,
                             _("Raw I/O is not supported on this platform"));
  #endif

-       if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK&&  disk->shared) {
+        if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK&&  disk->shared) {

The re-indent could be pushed independently and prior to the release
(or, if it was introduced earlier in the series, then fix that earlier
patch).

Okay.


              if (qemuAddSharedDisk(driver->sharedDisks, disk->src)<  0)
                  goto cleanup;
          }
+
+        /* Set sysfs unpriv_sgio if cdbfilter is specified */
+        if (disk->cdbfilter) {
+            if (virSetDeviceUnprivSGIO(disk->src, NULL,
+                                       (disk->cdbfilter ==
+                                        VIR_DOMAIN_DISK_CDB_FILTER_NO)
+                                        ? 1 : 0)<  0)
+                goto cleanup;
+        }

Would it be any simpler to write:

virSetDeviceUnprivSGIO(disk->src, NULL,
                        disk->cdbfilter != VIR_DOMAIN_DISK_CDB_FILTER_NO)<  0

but you may be rewriting this anyways.

Yeah, it's better.


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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]