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

[Osier Yang]

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.