On 12/03/14 21:54, Jiri Denemark wrote:
On Fri, Mar 07, 2014 at 22:23:26 +0800, Osier Yang wrote:
The kernel didn't support the unprivileged SGIO for SCSI generic
device finally, and since it's unknow whether the way to support
unprivileged SGIO for SCSI generic device will be similar as for
SCSI block device or not, even it's simliar (I.e. via sysfs, for
SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio,
for example), the file name might be different, So it's better not
guess what it should be like currently.

This patch removes the related code (mainly about the "shareable"
checking on the "sgio" setting, it's not supported at all, why
we leave checking code there? :-), and error out if "sgio" is
specified in the domain config.
---
  src/qemu/qemu_conf.c | 87 ++++++++++++----------------------------------------
  1 file changed, 20 insertions(+), 67 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 2c397b0..ad6348d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
...
@@ -1135,22 +1102,15 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
      } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
          hostdev = dev->data.hostdev;
- if (!hostdev->shareable ||
-            !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-              hostdev->source.subsys.type == 
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI))
-            return 0;
In the follow-up patch, you recovered just the

     hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI

part. Shouldn't the other checks in this condition remain?

-
-        if (!(hostdev_name = virSCSIDeviceGetDevName(NULL,
-                                                     
hostdev->source.subsys.u.scsi.adapter,
-                                                     
hostdev->source.subsys.u.scsi.bus,
-                                                     
hostdev->source.subsys.u.scsi.target,
-                                                     
hostdev->source.subsys.u.scsi.unit)))
-            goto cleanup;
-
-        if (virAsprintf(&hostdev_path, "/dev/%s", hostdev_name) < 0)
+        if (hostdev->source.subsys.u.scsi.sgio) {
In other words, should this be something like

     if (hostdev->shareable &&
         hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
         hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
         hostdev->source.subsys.u.scsi.sgio) {



Should be:

If (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
    hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
    hostdev->source.subsys.u.scsi.sgio) {

Regardless of whether the SCSI generic device is shareable or not. We won't support the unpiv_sgio setting. I lost the SUBSYS checking in the follow up patch indeed. If no
other issue, I can squash it in when pushing.   Thanks for the reviewing.

Osier

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

Reply via email to