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

[Osier Yang]

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