On Monday 13 April 2015 07:50 PM, Daniel P. Berrange wrote:
> On Mon, Apr 13, 2015 at 04:14:53PM +0200, Martin Kletzander wrote:
>> Use the same pattern as there is for x86 machines.
>>
>> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
>> ---
>> docs/schemas/domaincommon.rng | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 03fd541..80b30df 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -384,7 +384,9 @@
>> </optional>
>> <optional>
>> <attribute name="machine">
>> - <value>sun4m</value>
>> + <data type="string">
>> + <param name="pattern">[a-zA-Z0-9_\.\-]+</param>
>> + </data>
>> </attribute>
>> </optional>
>> </group>
> I think you could probably simplify this all much more. All these
> architecture specific blocks of machine type names should just be
> deleted and so this:
>
> <define name="ostypehvm">
> <element name="type">
> <optional>
> <choice>
> <ref name="hvmx86"/>
> <ref name="hvmmips"/>
> <ref name="hvmsparc"/>
> <ref name="hvmppc"/>
> <ref name="hvmppc64"/>
> <ref name="hvms390"/>
> <ref name="hvmarm"/>
> <ref name="hvmaarch64"/>
> </choice>
> </optional>
> <value>hvm</value>
> </element>
> </define>
>
> Would simplify to just
>
> <define name="ostypehvm">
> <element name="type">
> <optional>
> <attribute name="arch">
> <choice>
> <value>i686</value>
> ....others...
> </choice>
> </attribute>
> </optional>
> <optional>
> <attribute name="machine">
> <data type="string">
> <param name="pattern">[a-zA-Z0-9_\.\-]+</param>
> </data>
> </attribute>
> </optional>
> </element>
> </define>
Hi Martin, Daniel,
I have not been able to try this patch, it fails with this error :
error: internal error: Unable to parse RNG /test-libvirt/share/libvirt/schemas/domain.rng: Reference osexe has no matching definition
Internal found no define for ref osexe
However, had some concerns purely by looking at this patch. This change is very x86-centric, it does not respect other architectures.
I think the rationale for simplifying domaincommon.rng would have been to group all types that obey this pattern string:
<param name="pattern">[a-zA-Z0-9_\.\-]+</param>
However, this regex does not conform to machine types for _all_ architectures.
As an example, see this :
<define name="hvms390">
<group>
<optional>
<attribute name="arch">
<choice>
<value>s390</value>
<value>s390x</value>
</choice>
</attribute>
</optional>
<optional>
<attribute name="machine">
<choice>
<value>s390</value>
<value>s390-virtio</value>
<value>s390-ccw</value>
<value>s390-ccw-virtio</value>
</choice>
</attribute>
</optional>
</group>
</define>
The s390 arch only allows four machine names : "s390", "s390-virtio", "s390-ccw", "s390-ccw-virtio".
With the patch you suggest, even a string such as "abcdefg" will become a legitimate machine type for s390x, which seems like an odd thing.
Likewise, ppc64[le] architecture allows only strings such as pseries, pseries-2.1, pseries-2.2 ..
This patch will allow any random machine name, which seems somewhat odd to me.
Regards,
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list