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

15 Apr 2015

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

    