Re: [libvirt] [PATCH] qemu: Forbid "sgio" support for SCSI generic host device

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.