All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
@ 2013-10-07 16:53 Aneesh Kumar K.V
  2013-11-04 12:28 ` Alexander Graf
  2013-11-04 13:28 ` Jan Kiszka
  0 siblings, 2 replies; 10+ messages in thread
From: Aneesh Kumar K.V @ 2013-10-07 16:53 UTC (permalink / raw)
  To: agraf, paulus; +Cc: qemu-ppc, qemu-devel, Aneesh Kumar K.V

From: "Aneesh Kumar K.V" <aneesh.kumar@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.kumar@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 0xf0000510
@@ -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 0xf0000510
@@ -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 0000000..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 *alias;
@@ -28,6 +30,7 @@ typedef struct QEMUMachine {
     QEMUMachineInitFunc *init;
     QEMUMachineResetFunc *reset;
     QEMUMachineHotAddCPUFunc *hot_add_cpu;
+    QEMUMachineGetVmTypeFunc *get_vm_type;
     BlockInterfaceType block_default_type;
     int max_cpus;
     unsigned int no_serial:1,
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index e1f88bf..acc3d74 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -36,7 +36,8 @@ void xen_cmos_set_s3_resume(void *opaque, int irq, int level);
 
 qemu_irq *xen_interrupt_controller_init(void);
 
-int xen_init(void);
+typedef struct QEMUMachine QEMUMachine;
+int xen_init(QEMUMachine *machine);
 int xen_hvm_init(MemoryRegion **ram_memory);
 void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
 
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 9bbe3db..f25caec 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -142,8 +142,8 @@ typedef struct KVMState KVMState;
 extern KVMState *kvm_state;
 
 /* external API */
-
-int kvm_init(void);
+typedef struct QEMUMachine QEMUMachine;
+int kvm_init(QEMUMachine *machine);
 
 int kvm_has_sync_mmu(void);
 int kvm_has_vcpu_events(void);
diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index 9a0c6b3..d71343d 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -31,7 +31,8 @@ static inline int qtest_available(void)
     return 1;
 }
 
-int qtest_init(void);
+typedef struct QEMUMachine QEMUMachine;
+int qtest_init(QEMUMachine *machine);
 #else
 static inline bool qtest_enabled(void)
 {
@@ -43,7 +44,7 @@ static inline int qtest_available(void)
     return 0;
 }
 
-static inline int qtest_init(void)
+static inline int qtest_init(QEMUMachine *machine)
 {
     return 0;
 }
diff --git a/kvm-all.c b/kvm-all.c
index b87215c..606de41 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -35,6 +35,8 @@
 #include "qemu/event_notifier.h"
 #include "trace.h"
 
+#include "hw/boards.h"
+
 /* This check must be after config-host.h is included */
 #ifdef CONFIG_EVENTFD
 #include <sys/eventfd.h>
@@ -1342,7 +1344,7 @@ static int kvm_max_vcpus(KVMState *s)
     return 4;
 }
 
-int kvm_init(void)
+int kvm_init(QEMUMachine *machine)
 {
     static const char upgrade_note[] =
         "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
@@ -1350,8 +1352,9 @@ int kvm_init(void)
     KVMState *s;
     const KVMCapabilityInfo *missing_cap;
     int ret;
-    int i;
+    int i, type = 0;
     int max_vcpus;
+    const char *kvm_type;
 
     s = g_malloc0(sizeof(KVMState));
 
@@ -1407,7 +1410,14 @@ int kvm_init(void)
         goto err;
     }
 
-    s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
+    kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm_type");
+    if (machine->get_vm_type) {
+        type = machine->get_vm_type(kvm_type);
+    } else if (kvm_type) {
+        fprintf(stderr, "Invalid argument kvm_type=%s\n", kvm_type);
+        goto err;
+    }
+    s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, type);
     if (s->vmfd < 0) {
 #ifdef TARGET_S390X
         fprintf(stderr, "Please add the 'switch_amode' kernel parameter to "
diff --git a/kvm-stub.c b/kvm-stub.c
index 548f471..ccb7b8c 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -19,6 +19,8 @@
 #include "hw/pci/msi.h"
 #endif
 
+#include "hw/boards.h"
+
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
@@ -33,7 +35,7 @@ int kvm_init_vcpu(CPUState *cpu)
     return -ENOSYS;
 }
 
-int kvm_init(void)
+int kvm_init(QEMUMachine *machine)
 {
     return -ENOSYS;
 }
diff --git a/qtest.c b/qtest.c
index 584c707..ef3c473 100644
--- a/qtest.c
+++ b/qtest.c
@@ -502,7 +502,7 @@ static void qtest_event(void *opaque, int event)
     }
 }
 
-int qtest_init(void)
+int qtest_init(QEMUMachine *machine)
 {
     CharDriverState *chr;
 
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",
         },
+
         { /* End of list */ }
     },
 };
@@ -2608,7 +2613,7 @@ static QEMUMachine *machine_parse(const char *name)
     exit(!name || !is_help_option(name));
 }
 
-static int tcg_init(void)
+static int tcg_init(QEMUMachine *machine)
 {
     tcg_exec_init(tcg_tb_size * 1024 * 1024);
     return 0;
@@ -2618,7 +2623,7 @@ static struct {
     const char *opt_name;
     const char *name;
     int (*available)(void);
-    int (*init)(void);
+    int (*init)(QEMUMachine *);
     bool *allowed;
 } accel_list[] = {
     { "tcg", "tcg", tcg_available, tcg_init, &tcg_allowed },
@@ -2627,7 +2632,7 @@ static struct {
     { "qtest", "QTest", qtest_available, qtest_init, &qtest_allowed },
 };
 
-static int configure_accelerator(void)
+static int configure_accelerator(QEMUMachine *machine)
 {
     const char *p;
     char buf[10];
@@ -2654,7 +2659,7 @@ static int configure_accelerator(void)
                     continue;
                 }
                 *(accel_list[i].allowed) = true;
-                ret = accel_list[i].init();
+                ret = accel_list[i].init(machine);
                 if (ret < 0) {
                     init_failed = true;
                     fprintf(stderr, "failed to initialize %s: %s\n",
@@ -4037,10 +4042,10 @@ int main(int argc, char **argv, char **envp)
         exit(0);
     }
 
-    configure_accelerator();
+    configure_accelerator(machine);
 
     if (!qtest_enabled() && qtest_chrdev) {
-        qtest_init();
+        qtest_init(machine);
     }
 
     machine_opts = qemu_get_machine_opts();
diff --git a/xen-all.c b/xen-all.c
index 839f14f..ac3654b 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -1000,7 +1000,7 @@ static void xen_exit_notifier(Notifier *n, void *data)
     xs_daemon_close(state->xenstore);
 }
 
-int xen_init(void)
+int xen_init(QEMUMachine *machine)
 {
     xen_xc = xen_xc_interface_open(0, 0, 0);
     if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
diff --git a/xen-stub.c b/xen-stub.c
index ad189a6..59927cb 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -47,7 +47,7 @@ qemu_irq *xen_interrupt_controller_init(void)
     return NULL;
 }
 
-int xen_init(void)
+int xen_init(QEMUMachine *machine)
 {
     return -ENOSYS;
 }
-- 
1.8.1.2

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
  2013-10-07 16:53 [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type Aneesh Kumar K.V
@ 2013-11-04 12:28 ` Alexander Graf
  2013-11-04 13:28 ` Jan Kiszka
  1 sibling, 0 replies; 10+ messages in thread
From: Alexander Graf @ 2013-11-04 12:28 UTC (permalink / raw)
  To: Aneesh Kumar K.V
  Cc: list@suse.de:PReP, Paul Mackerras, QEMU Developers, gleb Natapov,
	Paolo Bonzini


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

> From: "Aneesh Kumar K.V" <aneesh.kumar@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.kumar@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
  2013-10-07 16:53 [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type Aneesh Kumar K.V
  2013-11-04 12:28 ` Alexander Graf
@ 2013-11-04 13:28 ` Jan Kiszka
  2013-11-04 13:30   ` Alexander Graf
  2013-11-06 15:08   ` Aneesh Kumar K.V
  1 sibling, 2 replies; 10+ messages in thread
From: Jan Kiszka @ 2013-11-04 13:28 UTC (permalink / raw)
  To: Aneesh Kumar K.V, agraf, paulus; +Cc: qemu-ppc, qemu-devel

On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@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.kumar@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
  2013-11-04 13:28 ` Jan Kiszka
@ 2013-11-04 13:30   ` Alexander Graf
  2013-11-04 13:32     ` Jan Kiszka
  2013-11-04 17:41     ` Andreas Färber
  2013-11-06 15:08   ` Aneesh Kumar K.V
  1 sibling, 2 replies; 10+ messages in thread
From: Alexander Graf @ 2013-11-04 13:30 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: list@suse.de:PReP, Paul Mackerras, Aneesh Kumar K.V, QEMU Developers


On 04.11.2013, at 14:28, Jan Kiszka <jan.kiszka@siemens.com> wrote:

> On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
>> From: "Aneesh Kumar K.V" <aneesh.kumar@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.kumar@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
  2013-11-04 13:30   ` Alexander Graf
@ 2013-11-04 13:32     ` Jan Kiszka
  2013-11-04 13:55       ` Alexander Graf
  2013-11-04 17:41     ` Andreas Färber
  1 sibling, 1 reply; 10+ messages in thread
From: Jan Kiszka @ 2013-11-04 13:32 UTC (permalink / raw)
  To: Alexander Graf
  Cc: list@suse.de:PReP, Paul Mackerras, Aneesh Kumar K.V, QEMU Developers

On 2013-11-04 14:30, Alexander Graf wrote:
> 
> On 04.11.2013, at 14:28, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 
>> On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
>>> From: "Aneesh Kumar K.V" <aneesh.kumar@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.kumar@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
  2013-11-04 13:32     ` Jan Kiszka
@ 2013-11-04 13:55       ` Alexander Graf
  0 siblings, 0 replies; 10+ messages in thread
From: Alexander Graf @ 2013-11-04 13:55 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: list@suse.de:PReP, Paul Mackerras, Aneesh Kumar K.V, QEMU Developers


On 04.11.2013, at 14:32, Jan Kiszka <jan.kiszka@siemens.com> wrote:

> On 2013-11-04 14:30, Alexander Graf wrote:
>> 
>> On 04.11.2013, at 14:28, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> 
>>> On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@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.kumar@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
  2013-11-04 13:30   ` Alexander Graf
  2013-11-04 13:32     ` Jan Kiszka
@ 2013-11-04 17:41     ` Andreas Färber
  2013-11-04 17:57       ` Paolo Bonzini
  1 sibling, 1 reply; 10+ messages in thread
From: Andreas Färber @ 2013-11-04 17:41 UTC (permalink / raw)
  To: Alexander Graf, Jan Kiszka
  Cc: qemu-ppc, QEMU Developers, Paul Mackerras, Aneesh Kumar K.V,
	Anthony Liguori, Paolo Bonzini

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
  2013-11-04 17:41     ` Andreas Färber
@ 2013-11-04 17:57       ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2013-11-04 17:57 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Jan Kiszka, QEMU Developers, Alexander Graf, qemu-ppc,
	Aneesh Kumar K.V, Anthony Liguori, Paul Mackerras

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
  2013-11-04 13:28 ` Jan Kiszka
  2013-11-04 13:30   ` Alexander Graf
@ 2013-11-06 15:08   ` Aneesh Kumar K.V
  2013-11-07  7:27     ` Jan Kiszka
  1 sibling, 1 reply; 10+ messages in thread
From: Aneesh Kumar K.V @ 2013-11-06 15:08 UTC (permalink / raw)
  To: Jan Kiszka, agraf, paulus; +Cc: qemu-ppc, qemu-devel

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
>> From: "Aneesh Kumar K.V" <aneesh.kumar@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.kumar@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
  2013-11-06 15:08   ` Aneesh Kumar K.V
@ 2013-11-07  7:27     ` Jan Kiszka
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2013-11-07  7:27 UTC (permalink / raw)
  To: Aneesh Kumar K.V, agraf, paulus; +Cc: qemu-ppc, qemu-devel

On 2013-11-06 16:08, Aneesh Kumar K.V wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
>>> From: "Aneesh Kumar K.V" <aneesh.kumar@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.kumar@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-11-07  7:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-07 16:53 [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type Aneesh Kumar K.V
2013-11-04 12:28 ` Alexander Graf
2013-11-04 13:28 ` Jan Kiszka
2013-11-04 13:30   ` Alexander Graf
2013-11-04 13:32     ` Jan Kiszka
2013-11-04 13:55       ` Alexander Graf
2013-11-04 17:41     ` Andreas Färber
2013-11-04 17:57       ` Paolo Bonzini
2013-11-06 15:08   ` Aneesh Kumar K.V
2013-11-07  7:27     ` Jan Kiszka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.