[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Wed, Apr 15, 2015 at 04:25:11PM +0530, Prerna Saxena wrote:

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.


Well, that's now.  If new QEMU comes with new machine type, we'll have
to fix libvirt to work with that.  I was trying to look into the
future a bit.

Anyway, there'll be a v3, so that's going to be the place to make
discussions.

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]