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

[Erik Skultety]

On 04/17/2015 03:10 PM, Ján Tomko wrote:
> 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
> 
> 
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 
Thank you for review, I moved the hunks into a separate commit and
pushed both.
Erik