[libvirt] [PATCH] rng: Forbid to validate mismatched <disk> 'device' and 'type' attributes

[Ján Tomko]

On Fri, Apr 17, 2015 at 01:31:09PM +0200, Erik Skultety wrote:
> According to docs, using 'lun' as a value for device attribute is only valid
> with disk types 'block' and 'network'. However current RNG schema also allows
> a combination type='file' device='lun' which results in a successfull
> xml validation, but fails at qemuBuildCommandLine.
> Besides fixing the RNG schema, this patch also adds a qemuxml2argvtest
> for this case.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1210669
> ---
>  docs/schemas/domaincommon.rng                      | 42 ++++++++++++++--------
>  .../qemuxml2argv-disk-device-lun-type-invalid.xml  | 28 +++++++++++++++
>  tests/qemuxml2argvtest.c                           |  2 ++
>  3 files changed, 58 insertions(+), 14 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-device-lun-type-invalid.xml
> 

ACK if you split out the sgIO reference.

> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 03fd541..ee32b73 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1316,37 +1316,43 @@
...

> -            <attribute name="sgio">
> -              <choice>
> -                <value>filtered</value>
> -                <value>unfiltered</value>
> -              </choice>
> -            </attribute>
> +            <ref name="sgIO"/>

This hunk...

>            </optional>
> +          <interleave>
> +            <choice>
> +              <ref name="diskSourceNetwork"/>
> +              <ref name="diskSourceBlock"/>
> +            </choice>
> +            <ref name="diskSpecsExtra"/>
> +          </interleave>
>          </group>
>        </choice>
>        <optional>
>          <ref name="snapshot"/>
>        </optional>
> -      <interleave>
> -        <ref name="diskSource"/>
> -        <ref name="storageSourceExtra"/>
> -        <ref name="diskBackingChain"/>
> -      </interleave>
>      </element>
>    </define>
>  
> +  <define name="diskSpecsExtra">
> +    <interleave>
> +      <ref name="storageSourceExtra"/>
> +      <ref name="diskBackingChain"/>
> +    </interleave>
> +  </define>
> +
>    <define name="diskBackingChain">
>      <choice>
>        <ref name="diskBackingStore"/>
> @@ -5315,4 +5321,12 @@
>        <ref name="virYesNo"/>
>      </attribute>
>    </define>

> +  <define name="sgIO">
> +    <attribute name="sgio">
> +      <choice>
> +        <value>filtered</value>
> +        <value>unfiltered</value>
> +      </choice>
> +    </attribute>
> +  </define>

... and this hunk are not required to fix the bug and should be pushed
separately.

>  </grammar>

</review>

Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150417/6334d765/attachment-0001.sig>