Re: [libvirt] [PATCH] replace ppce500v2 with ppce500

[Daniel P. Berrange]

On Tue, Mar 19, 2013 at 09:05:14PM -0600, Eric Blake wrote:
> On 03/19/2013 08:13 PM, Olivia Yin wrote:
> > ---
> >  docs/schemas/domaincommon.rng| 2 +-
> 
> Your commit message is sparse.  This is a good place to explain WHY a
> maintainer should apply your patch.
> 
> On my Fedora 18 box, I see:
> $ qemu-ppc64 -cpu help | grep e500
> PowerPC e500_v10 PVR 80200010
> PowerPC e500_v20 PVR 80200020
> PowerPC e500v1   PVR 80200020
> PowerPC e500v2_v10   PVR 80210010
> PowerPC e500v2_v20   PVR 80210020
> PowerPC e500v2_v21   PVR 80210021
> PowerPC e500v2_v22   PVR 80210022
> PowerPC e500v2   PVR 80210022
> PowerPC e500 PVR 80210022
> PowerPC e500v2_v30   PVR 80210030
> PowerPC e500mc   PVR 80230020
> 
> Doesn't that mean that both 'ppce500' AND 'ppce500v2' (and several
> others) should all be valid machine names?  If anything, we should
> either relax the RNG grammar to allow freeform names (since the code
> wasn't really checking), or we should enhance the grammar and C code to
> call out the entire list of values that qemu supports.  With as many ppc
> variants as qemu seems to support, trying to maintain a list seems like
> duplicated effort where we will constantly be behind.  Therefore, I
> think the best action is to do a mix: relax the grammar to allow any
> string (the way we already do for x86), but then do a validation in the
> C code that the string chosen by the user is supported by qemu (instead
> of the current approach of not caring about the string).

The list of machine names is something we dynamically extract from
QEMU, so the RNG shouldn't be trying to hardcode any list. We shuld
make the RNG allow 

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

Re: [libvirt] [PATCH] replace ppce500v2 with ppce500

[Eric Blake]

On 03/19/2013 08:13 PM, Olivia Yin wrote:
> ---
>  docs/schemas/domaincommon.rng| 2 +-

Your commit message is sparse.  This is a good place to explain WHY a
maintainer should apply your patch.

On my Fedora 18 box, I see:
$ qemu-ppc64 -cpu help | grep e500
PowerPC e500_v10 PVR 80200010
PowerPC e500_v20 PVR 80200020
PowerPC e500v1   PVR 80200020
PowerPC e500v2_v10   PVR 80210010
PowerPC e500v2_v20   PVR 80210020
PowerPC e500v2_v21   PVR 80210021
PowerPC e500v2_v22   PVR 80210022
PowerPC e500v2   PVR 80210022
PowerPC e500 PVR 80210022
PowerPC e500v2_v30   PVR 80210030
PowerPC e500mc   PVR 80230020

Doesn't that mean that both 'ppce500' AND 'ppce500v2' (and several
others) should all be valid machine names?  If anything, we should
either relax the RNG grammar to allow freeform names (since the code
wasn't really checking), or we should enhance the grammar and C code to
call out the entire list of values that qemu supports.  With as many ppc
variants as qemu seems to support, trying to maintain a list seems like
duplicated effort where we will constantly be behind.  Therefore, I
think the best action is to do a mix: relax the grammar to allow any
string (the way we already do for x86), but then do a validation in the
C code that the string chosen by the user is supported by qemu (instead
of the current approach of not caring about the string).

I'm not yet convinced that this patch should be applied.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

[libvirt] [PATCH] replace ppce500v2 with ppce500

[Olivia Yin]

---
 docs/schemas/domaincommon.rng| 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml  | 2 +-
 tests/testutilsqemu.c| 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 90647df..5a871d6 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -367,7 +367,7 @@
 g3beige
 mac99
 prep
-ppce500v2
+ppce500
   
 
   
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args 
b/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
index 93e8f9c..1ffd8be 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
@@ -1,5 +1,5 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
-/usr/bin/qemu-system-ppc -S -M ppce500v2 -m 256 -smp 1 -nographic \
+/usr/bin/qemu-system-ppc -S -M ppce500 -m 256 -smp 1 -nographic \
 -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
 -kernel /media/ram/uImage -initrd /media/ram/ramdisk \
 -append 'root=/dev/ram rw console=ttyS0,115200' -dtb /media/ram/test.dtb \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml
index 3674621..04f0eb6 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml
@@ -5,7 +5,7 @@
   262144
   1
   
-hvm
+hvm
 /media/ram/uImage
 /media/ram/ramdisk
 root=/dev/ram rw console=ttyS0,115200
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index db15ee6..35d6158 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -97,7 +97,7 @@ static int testQemuAddPPCGuest(virCapsPtr caps)
 static const char *machine[] = { "g3beige",
  "mac99",
  "prep",
- "ppce500v2" };
+ "ppce500" };
 virCapsGuestMachinePtr *machines = NULL;
 virCapsGuestPtr guest;
 
-- 
1.8.0


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