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@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list > Thank you for review, I moved the hunks into a separate commit and pushed both. Erik -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list