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/ > exclusive. > --- > docs/formatdomain.html.in | 13 +++++++++- > docs/schemas/domaincommon.rng | 52 ++++++++++++++++++++++++++++------------ > 2 files changed, 48 insertions(+), 17 deletions(-) > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 8e234fd..8c7c682 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -1395,7 +1395,18 @@ > rawio='yes', rawio capability will be enabled for all disks in > the domain (because, in the case of QEMU, this capability can > only be set on a per-process basis). This attribute is only > - valid when device is "lun". > + valid when device is "lun". NB, <code>rawio</code> intends to > + confine the capability per-device, however, current QEMU > + implementation gives the domain process broader capability > + than that (per-process basis, affects all the domain disks). > + To confine the capability as much as possible for QEMU driver > + as this stage, <code>cdbfilter</code> is recommended. > + The optional <code>cdbfilter</code> attribute > + (<span class="since">since 1.0.1</span>) indicates whether the > + kernel will filter unprivileged SG_IO for the disk, valid settings > + are "yes" or "no" (defaults to "yes"). Note that it's exclusive > + with attribute <code>rawio</code>; Same with <code>rawio</code>, > + <code>cdbfilter</code> is only valid for device 'lun'. Hmm, this doesn't seem like the smartest of designs. If I'm understanding you right, we have: 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 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. > The optional <code>snapshot</code> attribute indicates the default > behavior of the disk during disk snapshots: "internal" > requires a file format such as qcow2 that can store both the > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 14344e2..c8cdfd7 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -971,24 +971,44 @@ > --> > <define name="disk"> > <element name="disk"> > - <optional> > - <attribute name="device"> > - <choice> > - <value>floppy</value> > - <value>disk</value> > - <value>cdrom</value> > - <value>lun</value> > - </choice> > - </attribute> > - </optional> > - <optional> > - <attribute name="rawio"> > + <choice> > + <group> > + <optional> > + <attribute name="device"> > + <choice> > + <value>floppy</value> > + <value>disk</value> > + <value>cdrom</value> > + </choice> > + </attribute> > + </optional> > + </group> I like that you split this up, to enforce that rawio only appears with device='lun'. However... > + <group> > + <optional> > + <attribute name="device"> > + <value>lun</value> > + </attribute> > + </optional> This device='lun' should not be <optional>, since the default when device= is omitted is 'disk' (that is, unless you want the absence of device= and the presence of rawio='...' to imply a lun, but I think we should require an explicit use of lun before allowing rawio=). > <choice> > - <value>yes</value> > - <value>no</value> > + <optional> > + <attribute name="rawio"> > + <choice> > + <value>yes</value> > + <value>no</value> > + </choice> > + </attribute> > + </optional> > + <optional> > + <attribute name="cdbfilter"> > + <choice> > + <value>yes</value> > + <value>no</value> > + </choice> And this part would need a major rework if you go with my idea of a tri-state attribute value for rawio instead of introducing yet another attribute. > + </attribute> > + </optional> > </choice> > - </attribute> > - </optional> > + </group> > + </choice> > <optional> > <ref name="snapshot"/> > </optional> > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list