Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-06 Thread Aneesh Kumar K.V
Jan Kiszka jan.kis...@siemens.com writes:

 On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Targets like ppc64 support different typed of KVM, one which use
 hypervisor mode and the other which doesn't. Add a new machine
 property kvm_type that helps in selecting the respective ones
 We also add a new QEMUMachine callback get_vm_type that helps
 in mapping the string representation of kvm type specified.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

 ...

 diff --git a/vl.c b/vl.c
 index 4e709d5..7ecc581 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
  .name = usb,
  .type = QEMU_OPT_BOOL,
  .help = Set on/off to enable/disable usb,
 +},{
 +.name = kvm_type,
 +.type = QEMU_OPT_STRING,
 +.help = Set to kvm type to be used in create vm ioctl,

 This does not sound like an appropriate documentation for an enduser.


Any other suggestion for that ? It is actually a string which will be
parsed by the machine opt callback, and the return value is used as the
argument to create vm ioctl. 

-aneesh




Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-06 Thread Jan Kiszka
On 2013-11-06 16:08, Aneesh Kumar K.V wrote:
 Jan Kiszka jan.kis...@siemens.com writes:
 
 On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

 Targets like ppc64 support different typed of KVM, one which use
 hypervisor mode and the other which doesn't. Add a new machine
 property kvm_type that helps in selecting the respective ones
 We also add a new QEMUMachine callback get_vm_type that helps
 in mapping the string representation of kvm type specified.

 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

 ...

 diff --git a/vl.c b/vl.c
 index 4e709d5..7ecc581 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
  .name = usb,
  .type = QEMU_OPT_BOOL,
  .help = Set on/off to enable/disable usb,
 +},{
 +.name = kvm_type,
 +.type = QEMU_OPT_STRING,
 +.help = Set to kvm type to be used in create vm ioctl,

 This does not sound like an appropriate documentation for an enduser.

 
 Any other suggestion for that ? It is actually a string which will be
 parsed by the machine opt callback, and the return value is used as the
 argument to create vm ioctl. 

Maybe something like this: Specifies the KVM virtualization mode (xxx,
yyy, whatever)

No user is interesting in how this information gets transfered to the
KVM core. That's an irrelavant implementation detail.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-04 Thread Alexander Graf

On 07.10.2013, at 18:53, Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com 
wrote:

 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Targets like ppc64 support different typed of KVM, one which use
 hypervisor mode and the other which doesn't. Add a new machine
 property kvm_type that helps in selecting the respective ones
 We also add a new QEMUMachine callback get_vm_type that helps
 in mapping the string representation of kvm type specified.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 ---
 hw/ppc/e500plat.c  |  2 ++
 hw/ppc/mac_newworld.c  |  2 ++
 hw/ppc/mac_oldworld.c  |  2 ++
 hw/ppc/ppc440_bamboo.c |  2 ++
 hw/ppc/spapr.c | 19 +++
 hw/ppc/vmtype.h| 18 ++
 include/hw/boards.h|  3 +++
 include/hw/xen/xen.h   |  3 ++-
 include/sysemu/kvm.h   |  4 ++--
 include/sysemu/qtest.h |  5 +++--
 kvm-all.c  | 16 +---
 kvm-stub.c |  4 +++-
 qtest.c|  2 +-
 vl.c   | 17 +++--
 xen-all.c  |  2 +-
 xen-stub.c |  2 +-
 16 files changed, 85 insertions(+), 18 deletions(-)
 create mode 100644 hw/ppc/vmtype.h
 
 diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
 index 2e964b2..3e53e85 100644
 --- a/hw/ppc/e500plat.c
 +++ b/hw/ppc/e500plat.c
 @@ -17,6 +17,7 @@
 #include hw/pci/pci.h
 #include hw/ppc/openpic.h
 #include kvm_ppc.h
 +#include vmtype.h
 
 static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
 {
 @@ -51,6 +52,7 @@ static QEMUMachine e500plat_machine = {
 .desc = generic paravirt e500 platform,
 .init = e500plat_init,
 .max_cpus = 32,
 +.get_vm_type = pr_get_vm_type,

It should be called kvm_type, like the machine opt.

Apart from that I like the patch :). But it needs to be ack'ed by Gleb / Paolo.


Alex




Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-04 Thread Jan Kiszka
On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Targets like ppc64 support different typed of KVM, one which use
 hypervisor mode and the other which doesn't. Add a new machine
 property kvm_type that helps in selecting the respective ones
 We also add a new QEMUMachine callback get_vm_type that helps
 in mapping the string representation of kvm type specified.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

...

 diff --git a/vl.c b/vl.c
 index 4e709d5..7ecc581 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
  .name = usb,
  .type = QEMU_OPT_BOOL,
  .help = Set on/off to enable/disable usb,
 +},{
 +.name = kvm_type,
 +.type = QEMU_OPT_STRING,
 +.help = Set to kvm type to be used in create vm ioctl,

This does not sound like an appropriate documentation for an enduser.

OTOH, I do not recall right now how these help strings can be obtained
at all. One could intuitively try -machine sometype,?, but that's
not implemented. Anyone?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-04 Thread Alexander Graf

On 04.11.2013, at 14:28, Jan Kiszka jan.kis...@siemens.com wrote:

 On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Targets like ppc64 support different typed of KVM, one which use
 hypervisor mode and the other which doesn't. Add a new machine
 property kvm_type that helps in selecting the respective ones
 We also add a new QEMUMachine callback get_vm_type that helps
 in mapping the string representation of kvm type specified.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 ...
 
 diff --git a/vl.c b/vl.c
 index 4e709d5..7ecc581 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
 .name = usb,
 .type = QEMU_OPT_BOOL,
 .help = Set on/off to enable/disable usb,
 +},{
 +.name = kvm_type,
 +.type = QEMU_OPT_STRING,
 +.help = Set to kvm type to be used in create vm ioctl,
 
 This does not sound like an appropriate documentation for an enduser.
 
 OTOH, I do not recall right now how these help strings can be obtained
 at all. One could intuitively try -machine sometype,?, but that's
 not implemented. Anyone?

They should definitely show up in the man page. But yes, -machine kvm_type=? 
would be helpful as well.

As for -machine ? we are have a problem orthogonal to this patch. I agree that 
we need some way to programmatically list all machine options.


Alex




Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-04 Thread Jan Kiszka
On 2013-11-04 14:30, Alexander Graf wrote:
 
 On 04.11.2013, at 14:28, Jan Kiszka jan.kis...@siemens.com wrote:
 
 On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

 Targets like ppc64 support different typed of KVM, one which use
 hypervisor mode and the other which doesn't. Add a new machine
 property kvm_type that helps in selecting the respective ones
 We also add a new QEMUMachine callback get_vm_type that helps
 in mapping the string representation of kvm type specified.

 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

 ...

 diff --git a/vl.c b/vl.c
 index 4e709d5..7ecc581 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
 .name = usb,
 .type = QEMU_OPT_BOOL,
 .help = Set on/off to enable/disable usb,
 +},{
 +.name = kvm_type,
 +.type = QEMU_OPT_STRING,
 +.help = Set to kvm type to be used in create vm ioctl,

 This does not sound like an appropriate documentation for an enduser.

 OTOH, I do not recall right now how these help strings can be obtained
 at all. One could intuitively try -machine sometype,?, but that's
 not implemented. Anyone?
 
 They should definitely show up in the man page. But yes, -machine kvm_type=? 
 would be helpful as well.

The man page is not generate from this C code, is it?

 
 As for -machine ? we are have a problem orthogonal to this patch. I agree 
 that we need some way to programmatically list all machine options.

I'm not saying that this is an issue of this patch, just that I wonder
how that .help message can be made visisble at at besides with the help
of an editor.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-04 Thread Alexander Graf

On 04.11.2013, at 14:32, Jan Kiszka jan.kis...@siemens.com wrote:

 On 2013-11-04 14:30, Alexander Graf wrote:
 
 On 04.11.2013, at 14:28, Jan Kiszka jan.kis...@siemens.com wrote:
 
 On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Targets like ppc64 support different typed of KVM, one which use
 hypervisor mode and the other which doesn't. Add a new machine
 property kvm_type that helps in selecting the respective ones
 We also add a new QEMUMachine callback get_vm_type that helps
 in mapping the string representation of kvm type specified.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 ...
 
 diff --git a/vl.c b/vl.c
 index 4e709d5..7ecc581 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
.name = usb,
.type = QEMU_OPT_BOOL,
.help = Set on/off to enable/disable usb,
 +},{
 +.name = kvm_type,
 +.type = QEMU_OPT_STRING,
 +.help = Set to kvm type to be used in create vm ioctl,
 
 This does not sound like an appropriate documentation for an enduser.
 
 OTOH, I do not recall right now how these help strings can be obtained
 at all. One could intuitively try -machine sometype,?, but that's
 not implemented. Anyone?
 
 They should definitely show up in the man page. But yes, -machine kvm_type=? 
 would be helpful as well.
 
 The man page is not generate from this C code, is it?

No, it has to be part of the patch :).

 
 
 As for -machine ? we are have a problem orthogonal to this patch. I agree 
 that we need some way to programmatically list all machine options.
 
 I'm not saying that this is an issue of this patch, just that I wonder
 how that .help message can be made visisble at at besides with the help
 of an editor.

I think we need both. We also need help on both levels: machine options and 
arguments to machine options.


Alex




Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-04 Thread Andreas Färber
Hi,

Am 04.11.2013 14:30, schrieb Alexander Graf:
 I agree that we need some way to programmatically list all machine options.

I wonder if it would make sense to mirror all -machine options as
dynamic properties on /machine? :)
Their addition could still be driven by the declarative format.
We could then easily not only list but also inspect the values of those
options at runtime.

It wouldn't immediately solve Jan's documentation problem, but that's
more general than just -machine, isn't it?

CC'ing Anthony and Paolo.

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-04 Thread Paolo Bonzini
Il 04/11/2013 18:41, Andreas Färber ha scritto:
  I agree that we need some way to programmatically list all machine options.
 I wonder if it would make sense to mirror all -machine options as
 dynamic properties on /machine? :)
 Their addition could still be driven by the declarative format.
 We could then easily not only list but also inspect the values of those
 options at runtime.

Yes, I agree.  Perhaps not all of them however; probably not accel= and
probably not kvm_type too.  In fact, what I don't like about this patch
is that it doesn't really feel right to put it in -machine.  On the
other hand we definitely do not want to split it further to -accel.  Oh
well, it seems like designing the right command-line interface is the
hardest part of doing QEMU.

Paolo

 It wouldn't immediately solve Jan's documentation problem, but that's
 more general than just -machine, isn't it?
 
 CC'ing Anthony and Paolo.




[Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-10-07 Thread Aneesh Kumar K.V
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

Targets like ppc64 support different typed of KVM, one which use
hypervisor mode and the other which doesn't. Add a new machine
property kvm_type that helps in selecting the respective ones
We also add a new QEMUMachine callback get_vm_type that helps
in mapping the string representation of kvm type specified.

Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
---
 hw/ppc/e500plat.c  |  2 ++
 hw/ppc/mac_newworld.c  |  2 ++
 hw/ppc/mac_oldworld.c  |  2 ++
 hw/ppc/ppc440_bamboo.c |  2 ++
 hw/ppc/spapr.c | 19 +++
 hw/ppc/vmtype.h| 18 ++
 include/hw/boards.h|  3 +++
 include/hw/xen/xen.h   |  3 ++-
 include/sysemu/kvm.h   |  4 ++--
 include/sysemu/qtest.h |  5 +++--
 kvm-all.c  | 16 +---
 kvm-stub.c |  4 +++-
 qtest.c|  2 +-
 vl.c   | 17 +++--
 xen-all.c  |  2 +-
 xen-stub.c |  2 +-
 16 files changed, 85 insertions(+), 18 deletions(-)
 create mode 100644 hw/ppc/vmtype.h

diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index 2e964b2..3e53e85 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -17,6 +17,7 @@
 #include hw/pci/pci.h
 #include hw/ppc/openpic.h
 #include kvm_ppc.h
+#include vmtype.h
 
 static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
 {
@@ -51,6 +52,7 @@ static QEMUMachine e500plat_machine = {
 .desc = generic paravirt e500 platform,
 .init = e500plat_init,
 .max_cpus = 32,
+.get_vm_type = pr_get_vm_type,
 };
 
 static void e500plat_machine_init(void)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 5e79575..efe9b25 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -68,6 +68,7 @@
 #include sysemu/blockdev.h
 #include exec/address-spaces.h
 #include hw/sysbus.h
+#include vmtype.h
 
 #define MAX_IDE_BUS 2
 #define CFG_ADDR 0xf510
@@ -478,6 +479,7 @@ static QEMUMachine core99_machine = {
 .init = ppc_core99_init,
 .max_cpus = MAX_CPUS,
 .default_boot_order = cd,
+.get_vm_type = pr_get_vm_type,
 };
 
 static void core99_machine_init(void)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 2f27754..aefb521 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -42,6 +42,7 @@
 #include kvm_ppc.h
 #include sysemu/blockdev.h
 #include exec/address-spaces.h
+#include vmtype.h
 
 #define MAX_IDE_BUS 2
 #define CFG_ADDR 0xf510
@@ -351,6 +352,7 @@ static QEMUMachine heathrow_machine = {
 .is_default = 1,
 #endif
 .default_boot_order = cd, /* TOFIX cad when Mac floppy is implemented 
*/
+.get_vm_type = pr_get_vm_type,
 };
 
 static void heathrow_machine_init(void)
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 655e499..16b755e 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -28,6 +28,7 @@
 #include ppc405.h
 #include sysemu/sysemu.h
 #include hw/sysbus.h
+#include vmtype.h
 
 #define BINARY_DEVICE_TREE_FILE bamboo.dtb
 
@@ -296,6 +297,7 @@ static QEMUMachine bamboo_machine = {
 .name = bamboo,
 .desc = bamboo,
 .init = bamboo_init,
+.get_vm_type = pr_get_vm_type,
 };
 
 static void bamboo_machine_init(void)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 004184d..4a23b6a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1337,6 +1337,24 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
 assert(spapr-fdt_skel != NULL);
 }
 
+static int spapr_get_vm_type(const char *vm_type)
+{
+if (!vm_type) {
+return 0;
+}
+
+if (!strcmp(vm_type, HV)) {
+return 1;
+}
+
+if (!strcmp(vm_type, PR)) {
+return 2;
+}
+
+hw_error(Unknown kvm_type specified '%s', vm_type);
+exit(1);
+}
+
 static QEMUMachine spapr_machine = {
 .name = pseries,
 .desc = pSeries Logical Partition (PAPR compliant),
@@ -1347,6 +1365,7 @@ static QEMUMachine spapr_machine = {
 .max_cpus = MAX_CPUS,
 .no_parallel = 1,
 .default_boot_order = NULL,
+.get_vm_type = spapr_get_vm_type,
 };
 
 static void spapr_machine_init(void)
diff --git a/hw/ppc/vmtype.h b/hw/ppc/vmtype.h
new file mode 100644
index 000..f99a491
--- /dev/null
+++ b/hw/ppc/vmtype.h
@@ -0,0 +1,18 @@
+#ifndef PPC_VMTYPE_H
+#define PPC_VMTYPE_H
+
+static inline int pr_get_vm_type(const char *vm_type)
+{
+if (!vm_type) {
+return 0;
+}
+
+if (!strcmp(vm_type, PR)) {
+return 2;
+}
+
+hw_error(Unknown kvm_type specified '%s', vm_type);
+exit(1);
+}
+
+#endif
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 5a7ae9f..2130488 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -21,6 +21,8 @@ typedef void QEMUMachineResetFunc(void);
 
 typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);
 
+typedef int QEMUMachineGetVmTypeFunc(const char *arg);
+
 typedef struct QEMUMachine {
 const char *name;
 const char