Re: [libvirt] [PATCH] schema: Allow multiple machines for sparc VMs
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(a)redhat.com>
> ---
> 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