[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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).

>              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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]