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

[Osier Yang]

On 13/03/14 15:50, Jiri Denemark wrote:
> On Wed, Mar 12, 2014 at 23:13:24 +0800, Osier Yang wrote:
>> 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.
> OK, makes sense. ACK with this change then.
>

Thanks, pushed.

Osier