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

2015-04-15 Thread Martin Kletzander

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 mklet...@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
-  valuesun4m/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
  valuehvm/value
/element
  /define

Would simplify to just

  define name=ostypehvm
element name=type
  optional
attribute name=arch
  choice
valuei686/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
   values390/value
   values390x/value
 /choice
   /attribute
 /optional
 optional
   attribute name=machine
 choice
   values390/value
   values390-virtio/value
   values390-ccw/value
   values390-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.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2015-04-15 Thread 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 mklet...@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
 -  valuesun4m/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
   valuehvm/value
 /element
   /define

 Would simplify to just

   define name=ostypehvm
 element name=type
   optional
 attribute name=arch
   choice
 valuei686/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
values390/value
values390x/value
  /choice
/attribute
  /optional
  optional
attribute name=machine
  choice
values390/value
values390-virtio/value
values390-ccw/value
values390-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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2015-04-15 Thread John Ferlan
...snip...
 I have not been able to try this patch, it fails with this error :
 

There's a v2 :

http://www.redhat.com/archives/libvir-list/2015-April/msg00503.html

Although it appears that it too has a RNG issue according to what Jan
just posted.

John

 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
 values390/value
 values390x/value
   /choice
 /attribute
   /optional
   optional
 attribute name=machine
   choice
 values390/value
 values390-virtio/value
 values390-ccw/value
 values390-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,
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2015-04-13 Thread Martin Kletzander
Use the same pattern as there is for x86 machines.

Signed-off-by: Martin Kletzander mklet...@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
-  valuesun4m/value
+  data type=string
+param name=pattern[a-zA-Z0-9_\.\-]+/param
+  /data
 /attribute
   /optional
 /group
-- 
2.3.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


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

2015-04-13 Thread Daniel P. Berrange
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 mklet...@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
 -  valuesun4m/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
  valuehvm/value
/element
  /define

Would simplify to just

  define name=ostypehvm
element name=type
  optional
attribute name=arch
  choice
valuei686/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


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list