[libvirt] [PATCH] schema: Allow multiple machines for sparc VMs

[Prerna Saxena]

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 at 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