[libvirt] [PATCH 3/8] docs: Add docs and rng schema for new XML cdbfilter

Osier Yang jyang at redhat.com
Sun Dec 16 18:29:37 UTC 2012


On 2012年12月15日 04:47, Eric Blake wrote:
> On 12/14/2012 01:33 PM, Eric Blake wrote:
>> On 12/13/2012 12:05 PM, Osier Yang wrote:
>>> Since "rawio" and "cdbfilter" are only valid for "lun", this
>>> groups them together; And since both of them intend to allow
>>> the unprivledged user to use the SG_IO commands, they must be
>>
>> s/unprivledged/unprivileged/
>
>> rawio     cdbfilter      result
>> missing   missing        kernel prevents SG_IO
>> missing   no             kernel prevents SG_IO
>> missing   yes            SG_IO allowed for disk
>> no        missing        kernel prevents SG_IO
>> no        no             kernel prevents SG_IO
>> no        yes            SG_IO allowed for disk
>> yes       missing        SG_IO allowed for process
>> yes       no             SG_IO allowed for process
>> yes       yes            error
>
> This table shows another reason why I don't like your naming - the
> defaults are screwy, when an omitted rawio means 'no', but an omitted
> cdbfilter means 'yes'.  Corrected, the table looks like:
>
> rawio     cdbfilter      result
> missing   missing        kernel prevents SG_IO
> missing   no             SG_IO allowed for disk
> missing   yes            kernel prevents SG_IO
> no        missing        kernel prevents SG_IO
> no        no             error? or SG_IO allowed for disk?
> no        yes            error? or kernel prevents SG_IO?
> yes       missing        SG_IO allowed for process
> yes       no             error
> yes       yes            error? or kernel prevents SG_IO?
>
>> Why not simplify things, and have a single attribute rawio, with
>> multiple values?
>>
>> rawio        result
>> missing      kernel prevents SG_IO
>> no           kernel prevents SG_IO
>> yes          SG_IO allowed for process
>> cdb          SG_IO allowed for disk
>>
>> where we document that 'yes' works on more kernel versions than 'cdb',
>> but that 'cdb' (new to 1.0.1) is more secure.
>
> Oh, another thing, what does 'cdb' mean?  It happens to be an acronym
> that matches the kernel implementation,

Yes, it's abstraction of "Command Descriptor Block", see:

http://en.wikipedia.org/wiki/SCSI_CDB

> but it doesn't convey much
> meaning on its own.  Given that our existing rawio='yes' merely turns on
> SG_IO via a capability for the entire process,

However, the "rawio" tag is designed for disk-basis. Though current
qemu implementation turns on the capability for the whole process,
it's just a qemu implementation. And it's still possible to hack
'rawio' to enable the SG_IO capabilty for single disk in future.
This is Daniel's thought, and I agreed with it. See:

https://www.redhat.com/archives/libvir-list/2012-November/msg01077.html


> and the new filtering
> method enables per-disk whitelisting of which disks get to use SG_IO
> passthrough, would it be any better to document things as:
>
> no|yes|disk

As explained above, "rawio" tag is already designed for disk-basic,
and the underlying implementation for "rawio" and the new sysfs knob
is complete different; And as far as I get from the kernel patches,
I guess it could only filter sub-set of the commands in future, not
simply filtering all or not filtering all. That means we probably will
have to add new values in future.

For above reasons, I'm afraid that mixing them up will lead us
into a mess. So I'm inclined to use a new tag, and document
it as a different approach with "rawio" to enable SG_IO.

For the tag "name", I don't have better idea except "cdbfilter",
though I agreed that it doesn't convey meaning much on its own.

>
> and where we additionally allow 'process' as a synonym for 'yes' on
> input (for back-compat, we have to output 'yes', not 'process').  That
> is, I don't know that exposing the term 'cdb' buys us any understanding
> into what is really going on.
>

Thanks,

Osier




More information about the libvir-list mailing list