machine: query machine properties rather than qemu opts
cover machine: query machine properties rather than qemu opts
Commit e79d5a6 ("machine: remove qemu_machine_opts global list") removed option descriptions from the -machine QemuOptsList to avoid repeating MachineState's QOM properties.
This results in a Qemu crash if a non string option is queried using qemu opts. Fix this by querying machine properties through designated wrappers.
I hope I didn't miss anything.
Comments are appreciated as always.
Thanks,
Marcel
Marcel Apfelbaum (8):
machine: query iommu machine property rather than qemu opts hw/machine: kernel-irqchip property support for allowed/required
machine: query kernel-irqchip machine property rather than qemu opts
kvm: add machine state to kvm_arch_init
machine: query kvm-shadow-mem machine property rather than qemu opts
machine: query phandle-start machine property rather than qemu opts
machine: query dump-guest-core machine property rather than qemu opts
machine: query mem-merge machine property rather than qemu opts
device_tree.c | 5 ++--- exec.c | 6 +++--- hw/core/machine.c | 52 +++++++++++++++++++++++++++++++++++++++++++--------- hw/pci-host/q35.c | 2 +- hw/ppc/e500.c | 16 +++++----------- hw/ppc/spapr.c | 16 ++++++---------- include/hw/boards.h | 10 +++++++++- include/sysemu/kvm.h | 2 +- kvm-all.c | 8 ++++---- target-arm/kvm.c | 2 +- target-i386/kvm.c | 5 ++--- target-mips/kvm.c | 2 +- target-ppc/kvm.c | 2 +- target-s390x/kvm.c | 2 +- 14 files changed, 80 insertions(+), 50 deletions(-)
Signature · 1 line
2.1.0
PATCH 1/8 machine: query iommu machine property rather than qemu opts
Fixes a QEMU crash when passing iommu parameter in command line.
--- hw/core/machine.c | 5 +++++ hw/pci-host/q35.c | 2 +- include/hw/boards.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-)
hw/core/machine.c +5 -0
@@ -403,6 +403,11 @@ bool machine_usb(MachineState *machine)return machine->usb; } +bool machine_iommu (MachineState *machine) +{ +return machine->iommu; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,
hw/pci-host/q35.c +1 -1
@@ -415,7 +415,7 @@ static int mch_init(PCIDevice *d) PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE); }/* Intel IOMMU (VT-d) */ -if (qemu_opt_get_bool (qemu_get_machine_opts (),"iommu" , false)) { +if (machine_iommu (current_machine)) {mch_init_dmar (mch); }return 0 ;
include/hw/boards.h +1 -0
@@ -66,6 +66,7 @@ MachineClass *find_default_machine(void);extern MachineState *current_machine;bool machine_usb (MachineState *machine); +bool machine_iommu (MachineState *machine);/** * MachineClass:
Signature · 1 line
2.1.0
Re: [Qemu-devel] [PATCH 1/8] machine: query iommu machine property rather than qemu opts
Quoted 15 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing iommu parameter in command line.
Signed-off-by Marcel Apfelbaum <[email protected]>--- hw/core/machine.c | 5 +++++ hw/pci-host/q35.c | 2 +- include/hw/boards.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-)hw/core/machine.c +0
-0@@ -403,6 +403,11 @@ bool machine_usb(MachineState *machine)return machine->usb;
hw/core/machine.c bool machine_usb(MachineState *machine) · line 413
Quoted 10 lines
hw/core/machine.c +5
-0} +bool machine_iommu (MachineState *machine) +{ +return machine->iommu; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,
What does this buy us over a straight current_machine->iommu?
Same for the other wrapper functions.
[...]
Re: [Qemu-devel] [PATCH 1/8] machine: query iommu machine property rather than qemu opts
Quoted 19 lines · Markus Armbruster
Quoted 17 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing iommu parameter in command line.
Signed-off-by Marcel Apfelbaum <[email protected]>--- hw/core/machine.c | 5 +++++ hw/pci-host/q35.c | 2 +- include/hw/boards.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-)hw/core/machine.c +0
-0@@ -403,6 +403,11 @@ bool machine_usb(MachineState *machine)return machine->usb; }
Quoted 10 lines
hw/core/machine.c bool machine_usb(MachineState *machine) · line 413
Quoted 8 lines
hw/core/machine.c +5
-0+bool machine_iommu (MachineState *machine) +{ +return machine->iommu; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,What does this buy us over a straight
current_machine->iommu?
Following QOM guidelines/conventions all object fields are private to machine files only. The *only* ways that they can be exposed are:
1. A wrapper
2. object_property_get...
I chose the wrapper because the other variant would be really ugly and should be used only on a generic code.
This is the real reason I used this, but pure theoretical speaking using wrappers will give us the opportunity to change machine_iommu implementation without the need to change the call sites.
To wrap it up, I just followed previous comments I received on QOM handling.
Thanks,
Marcel
Quoted 5 lines
Same for the other wrapper functions.
[...]
Re: [Qemu-devel] [PATCH 1/8] machine: query iommu machine property rather than qemu opts
Quoted 35 lines · Marcel Apfelbaum
Quoted 19 lines · Markus Armbruster
Quoted 17 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing iommu parameter in command line.
Signed-off-by Marcel Apfelbaum <[email protected]>--- hw/core/machine.c | 5 +++++ hw/pci-host/q35.c | 2 +- include/hw/boards.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-)hw/core/machine.c +0
-0@@ -403,6 +403,11 @@ bool machine_usb(MachineState *machine)return machine->usb; }Quoted 10 lines
hw/core/machine.c bool machine_usb(MachineState *machine) · line 413
Quoted 8 lines
hw/core/machine.c +5
-0+bool machine_iommu (MachineState *machine) +{ +return machine->iommu; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,What does this buy us over a straight
current_machine->iommu?Following QOM guidelines/conventions all object fields are private to machine files only. The *only* ways that they can be exposed are:
1. A wrapper
2.object_property_get...
Quoted 6 lines
I chose the wrapper because the other variant would be really ugly and should be used only on a generic code.
This is the real reason I used this, but pure theoretical speaking using wrappers will give us the opportunity to change
machine_iommuimplementation without the need to change the call sites.
So we pay the notational and cognitive overhead of wrappers now and going forward to maybe some day save us fixing up a couple of machine->iommu instances.
A theory that makes sense for interfacing complex machinery with non-trivial implementation details becomes silly when applied to a couple of straightforward configuration flags. As Doug McIlroy observed, "KISS became MICAHI: make it complicated and hide it."
Quoted 1 line
To wrap it up, I just followed previous comments I received on QOM handling.
In other words, it's somebody else's fault. I'll shut up now ;)
[...]
Re: [Qemu-devel] [PATCH 1/8] machine: query iommu machine property rather than qemu opts
In reply to Marcel ApfelbaumQuoted 3 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing iommu parameter in command line.
Signed-off-by Marcel Apfelbaum <[email protected]>
Please amend commit message:
Running
x86_64-softmmu/qemu-system-x86_64 -machine pc,iommu=on -enable-kvm
leads to crash:
qemu-system-x86_64: qemu/util/qemu-option.c:387: qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed. Aborted (core dumped)
This happens because the commit e79d5a6 ("machine: remove qemu_machine_opts global list") removed the global option descriptions and moved them to MachineState's QOM properties.
Fix this by querying machine properties through designated wrappers.
Quoted 48 lines
--- hw/core/machine.c | 5 +++++ hw/pci-host/q35.c | 2 +- include/hw/boards.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-)hw/core/machine.c +5
-0@@ -403,6 +403,11 @@ bool machine_usb(MachineState *machine)return machine->usb; } +bool machine_iommu (MachineState *machine) +{ +return machine->iommu; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,hw/pci-host/q35.c +1
-1@@ -415,7 +415,7 @@ static int mch_init(PCIDevice *d) PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE); }/* Intel IOMMU (VT-d) */ -if (qemu_opt_get_bool (qemu_get_machine_opts (),"iommu" , false)) { +if (machine_iommu (current_machine)) {mch_init_dmar (mch); }return 0 ;include/hw/boards.h +1
-0@@ -66,6 +66,7 @@ MachineClass *find_default_machine(void);extern MachineState *current_machine;bool machine_usb (MachineState *machine); +bool machine_iommu (MachineState *machine);/** * MachineClass:
PATCH 2/8 hw/machine: kernel-irqchip property support for allowed/required
In reply to Marcel ApfelbaumThe code using kernel-irqchip property requires 'allowed/required' functionality. Replace machine's kernel_irqchip field with two fields representing the new functionality and expose them through wrappers.
--- hw/core/machine.c | 24 +++++++++++++++--------- include/hw/boards.h | 5 ++++- 2 files changed, 19 insertions(+), 10 deletions(-)
hw/core/machine.c +15 -9
@@ -31,18 +31,12 @@ static void machine_set_accel(Object *obj, const char *value, Error **errp) ms->accel =g_strdup (value); } -static bool machine_get_kernel_irqchip (Object *obj,Error **errp) -{ -MachineState *ms =MACHINE (obj); - -return ms->kernel_irqchip; -} -static void machine_set_kernel_irqchip (Object *obj,bool value,Error **errp) {MachineState *ms =MACHINE (obj); - ms->kernel_irqchip = value; + ms->kernel_irqchip_allowed = value; + ms->kernel_irqchip_required = value; } static void machine_get_kvm_shadow_mem(Object *obj, Visitor *v,
@@ -289,13 +283,15 @@ static void machine_initfn(Object *obj) {MachineState *ms =MACHINE (obj); + ms->kernel_irqchip_allowed = true; +object_property_add_str (obj,"accel" , machine_get_accel, machine_set_accel,NULL );object_property_set_description (obj,"accel" ,"Accelerator list" ,NULL );object_property_add_bool (obj,"kernel-irqchip" , - machine_get_kernel_irqchip, +NULL , machine_set_kernel_irqchip,NULL ); object_property_set_description(obj, "kernel-irqchip",
@@ -408,6 +404,16 @@ bool machine_iommu(MachineState *machine)return machine->iommu; } +bool machine_kernel_irqchip_allowed (MachineState *machine) +{ +return machine->kernel_irqchip_allowed; +} + +bool machine_kernel_irqchip_required (MachineState *machine) +{ +return machine->kernel_irqchip_required; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,
include/hw/boards.h +4 -1
@@ -67,6 +67,8 @@ extern MachineState *current_machine;bool machine_usb (MachineState *machine);bool machine_iommu (MachineState *machine); +bool machine_kernel_irqchip_allowed (MachineState *machine); +bool machine_kernel_irqchip_required (MachineState *machine);/** * MachineClass:
@@ -125,7 +127,8 @@ struct MachineState {/*< public >*/ char *accel; -bool kernel_irqchip; +bool kernel_irqchip_allowed; +bool kernel_irqchip_required;int kvm_shadow_mem;char *dtb;char *dumpdtb;
Signature · 1 line
2.1.0
PATCH 3/8 machine: query kernel-irqchip machine property rather than qemu opts
In reply to Marcel ApfelbaumFixes a QEMU crash when passing kernel_irqchip parameter in command line.
--- hw/ppc/e500.c | 16 +++++----------- hw/ppc/spapr.c | 16 ++++++---------- kvm-all.c | 6 +++--- 3 files changed, 14 insertions(+), 24 deletions(-)
hw/ppc/e500.c +5 -11
@@ -731,8 +731,8 @@ static DeviceState *ppce500_init_mpic_kvm(PPCE500Params *params,return dev; } -static qemu_irq *ppce500_init_mpic (PPCE500Params *params,MemoryRegion *ccsr, -qemu_irq **irqs) +static qemu_irq *ppce500_init_mpic (MachineState *machine,PPCE500Params *params, +MemoryRegion *ccsr,qemu_irq **irqs) {qemu_irq *mpic;DeviceState *dev =NULL ;
@@ -742,17 +742,11 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params *params, MemoryRegion *ccsr, mpic =g_new0 (qemu_irq,256 );if (kvm_enabled ()) { -QemuOpts *machine_opts =qemu_get_machine_opts (); -bool irqchip_allowed =qemu_opt_get_bool (machine_opts, -"kernel_irqchip" , true); -bool irqchip_required =qemu_opt_get_bool (machine_opts, -"kernel_irqchip" , false); - -if (irqchip_allowed) { +if (machine_kernel_irqchip_allowed (machine)) { dev =ppce500_init_mpic_kvm (params, irqs); } -if (irqchip_required && !dev) { +if (machine_kernel_irqchip_required (machine) && !dev) {fprintf (stderr,"%s: irqchip requested but unavailable\n" , __func__);abort ();
@@ -876,7 +870,7 @@ void ppce500_init(MachineState *machine, PPCE500Params *params)memory_region_add_subregion (address_space_mem, params->ccsrbar_base, ccsr_addr_space); - mpic =ppce500_init_mpic (params, ccsr_addr_space, irqs); + mpic =ppce500_init_mpic (machine, params, ccsr_addr_space, irqs);/* Serial */ if (serial_hds[0 ]) {
hw/ppc/spapr.c +6 -10
@@ -123,21 +123,16 @@ static XICSState *try_create_xics(const char *type, int nr_servers,return XICS_COMMON (dev); } -static XICSState *xics_system_init (int nr_servers,int nr_irqs) +static XICSState *xics_system_init (MachineState *machine, +int nr_servers,int nr_irqs) {XICSState *icp =NULL ; -if (kvm_enabled ()) { -QemuOpts *machine_opts =qemu_get_machine_opts (); -bool irqchip_allowed =qemu_opt_get_bool (machine_opts, -"kernel_irqchip" , true); -bool irqchip_required =qemu_opt_get_bool (machine_opts, -"kernel_irqchip" , false); -if (irqchip_allowed) { +if (machine_kernel_irqchip_allowed (machine)) { icp =try_create_xics (TYPE_KVM_XICS , nr_servers, nr_irqs); } -if (irqchip_required && !icp) { +if (machine_kernel_irqchip_required (machine) && !icp) {perror ("Failed to create in-kernel XICS\n" );abort (); }
@@ -1420,7 +1415,8 @@ static void ppc_spapr_init(MachineState *machine) }/* Set up Interrupt Controller before we create the VCPUs */ - spapr->icp =xics_system_init (smp_cpus *kvmppc_smt_threads () / smp_threads, + spapr->icp =xics_system_init (machine, + smp_cpus *kvmppc_smt_threads () / smp_threads,XICS_IRQS );/* init CPUs */
kvm-all.c +3 -3
@@ -1360,11 +1360,11 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq) false); } -static int kvm_irqchip_create (KVMState *s) +static int kvm_irqchip_create (MachineState *machine,KVMState *s) {int ret; -if (!qemu_opt_get_bool (qemu_get_machine_opts (),"kernel_irqchip" , true) || +if (!machine_kernel_irqchip_allowed (machine) || (!kvm_check_extension (s,KVM_CAP_IRQCHIP ) && (kvm_vm_enable_cap (s,KVM_CAP_S390_IRQCHIP ,0 ) <0 ))) {return 0 ;
@@ -1603,7 +1603,7 @@ static int kvm_init(MachineState *ms) goto err; } - ret =kvm_irqchip_create (s); + ret =kvm_irqchip_create (ms, s);if (ret <0 ) { goto err; }
Signature · 1 line
2.1.0
Re: [Qemu-devel] [PATCH 3/8] machine: query kernel-irqchip machine property rather than qemu opts
Quoted 1 line · Marcel Apfelbaum
Fixes a QEMU crash when passing
kernel_irqchipparameter in command line.
Please amend commit message:
Running
x86_64-softmmu/qemu-system-x86_64 -machine pc,kernel_irqchip=on -enable-kvm
leads to crash:
qemu-system-x86_64: qemu/util/qemu-option.c:387: qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed. Aborted (core dumped)
This happens because the commit e79d5a6 ("machine: remove qemu_machine_opts global list") removed the global option descriptions and moved them to MachineState's QOM properties.
Fix this by querying machine properties through designated wrappers.
Quoted 120 lines
Signed-off-by Marcel Apfelbaum <[email protected]>--- hw/ppc/e500.c | 16 +++++----------- hw/ppc/spapr.c | 16 ++++++---------- kvm-all.c | 6 +++--- 3 files changed, 14 insertions(+), 24 deletions(-)hw/ppc/e500.c +5
-11@@ -731,8 +731,8 @@ static DeviceState *ppce500_init_mpic_kvm(PPCE500Params *params,return dev; } -static qemu_irq *ppce500_init_mpic (PPCE500Params *params,MemoryRegion *ccsr, -qemu_irq **irqs) +static qemu_irq *ppce500_init_mpic (MachineState *machine,PPCE500Params *params, +MemoryRegion *ccsr,qemu_irq **irqs) {qemu_irq *mpic;DeviceState *dev =NULL ;@@ -742,17 +742,11 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params *params, MemoryRegion *ccsr, mpic =g_new0 (qemu_irq,256 );if (kvm_enabled ()) { -QemuOpts *machine_opts =qemu_get_machine_opts (); -bool irqchip_allowed =qemu_opt_get_bool (machine_opts, -"kernel_irqchip" , true); -bool irqchip_required =qemu_opt_get_bool (machine_opts, -"kernel_irqchip" , false); - -if (irqchip_allowed) { +if (machine_kernel_irqchip_allowed (machine)) { dev =ppce500_init_mpic_kvm (params, irqs); } -if (irqchip_required && !dev) { +if (machine_kernel_irqchip_required (machine) && !dev) {fprintf (stderr,"%s: irqchip requested but unavailable\n" , __func__);abort ();@@ -876,7 +870,7 @@ void ppce500_init(MachineState *machine, PPCE500Params *params)memory_region_add_subregion (address_space_mem, params->ccsrbar_base, ccsr_addr_space); - mpic =ppce500_init_mpic (params, ccsr_addr_space, irqs); + mpic =ppce500_init_mpic (machine, params, ccsr_addr_space, irqs);/* Serial */ if (serial_hds[0 ]) {hw/ppc/spapr.c +6
-10@@ -123,21 +123,16 @@ static XICSState *try_create_xics(const char *type, int nr_servers,return XICS_COMMON (dev); } -static XICSState *xics_system_init (int nr_servers,int nr_irqs) +static XICSState *xics_system_init (MachineState *machine, +int nr_servers,int nr_irqs) {XICSState *icp =NULL ; -if (kvm_enabled ()) { -QemuOpts *machine_opts =qemu_get_machine_opts (); -bool irqchip_allowed =qemu_opt_get_bool (machine_opts, -"kernel_irqchip" , true); -bool irqchip_required =qemu_opt_get_bool (machine_opts, -"kernel_irqchip" , false); -if (irqchip_allowed) { +if (machine_kernel_irqchip_allowed (machine)) { icp =try_create_xics (TYPE_KVM_XICS , nr_servers, nr_irqs); } -if (irqchip_required && !icp) { +if (machine_kernel_irqchip_required (machine) && !icp) {perror ("Failed to create in-kernel XICS\n" );abort (); }@@ -1420,7 +1415,8 @@ static void ppc_spapr_init(MachineState *machine) }/* Set up Interrupt Controller before we create the VCPUs */ - spapr->icp =xics_system_init (smp_cpus *kvmppc_smt_threads () / smp_threads, + spapr->icp =xics_system_init (machine, + smp_cpus *kvmppc_smt_threads () / smp_threads,XICS_IRQS );/* init CPUs */ kvm-all.c +3
-3@@ -1360,11 +1360,11 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, EventNotifier *n, int virq) false); } -static int kvm_irqchip_create (KVMState *s) +static int kvm_irqchip_create (MachineState *machine,KVMState *s) {int ret; -if (!qemu_opt_get_bool (qemu_get_machine_opts (),"kernel_irqchip" , true) || +if (!machine_kernel_irqchip_allowed (machine) || (!kvm_check_extension (s,KVM_CAP_IRQCHIP ) && (kvm_vm_enable_cap (s,KVM_CAP_S390_IRQCHIP ,0 ) <0 ))) {return 0 ;@@ -1603,7 +1603,7 @@ static int kvm_init(MachineState *ms) goto err; } - ret =kvm_irqchip_create (s); + ret =kvm_irqchip_create (ms, s);if (ret <0 ) { goto err; }
PATCH 4/8 kvm: add machine state to kvm_arch_init
In reply to Marcel ApfelbaumNeeded to query machine's properties.
--- include/sysemu/kvm.h | 2 +- kvm-all.c | 2 +- target-arm/kvm.c | 2 +- target-i386/kvm.c | 2 +- target-mips/kvm.c | 2 +- target-ppc/kvm.c | 2 +- target-s390x/kvm.c | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-)
include/sysemu/kvm.h +1 -1
@@ -259,7 +259,7 @@ int kvm_arch_get_registers(CPUState *cpu);int kvm_arch_put_registers (CPUState *cpu,int level); -int kvm_arch_init (KVMState *s); +int kvm_arch_init (MachineState *ms,KVMState *s);int kvm_arch_init_vcpu (CPUState *cpu);
kvm-all.c +1 -1
@@ -1598,7 +1598,7 @@ static int kvm_init(MachineState *ms) kvm_resamplefds_allowed = (kvm_check_extension (s,KVM_CAP_IRQFD_RESAMPLE ) >0 ); - ret =kvm_arch_init (s); + ret =kvm_arch_init (ms, s);if (ret <0 ) { goto err; }
target-arm/kvm.c +1 -1
@@ -150,7 +150,7 @@ static const TypeInfo host_arm_cpu_type_info = { .class_size = sizeof(ARMHostCPUClass), }; -int kvm_arch_init (KVMState *s) +int kvm_arch_init (MachineState *ms,KVMState *s) {/* For ARM interrupt delivery is always asynchronous, * whether we are using an in-kernel VGIC or not.
target-i386/kvm.c +1 -1
@@ -840,7 +840,7 @@ static int kvm_get_supported_msrs(KVMState *s)return ret; } -int kvm_arch_init (KVMState *s) +int kvm_arch_init (MachineState *ms,KVMState *s) {uint64_t identity_base =0xfffbc000 ;uint64_t shadow_mem;
target-mips/kvm.c +1 -1
@@ -40,7 +40,7 @@ unsigned long kvm_arch_vcpu_id(CPUState *cs)return cs->cpu_index; } -int kvm_arch_init (KVMState *s) +int kvm_arch_init (MachineState *ms,KVMState *s) {/* MIPS has 128 signals */ kvm_set_sigmask_len (s,16 );
target-ppc/kvm.c +1 -1
@@ -95,7 +95,7 @@ static void kvm_kick_cpu(void *opaque)static int kvm_ppc_register_host_cpu_type (void ); -int kvm_arch_init (KVMState *s) +int kvm_arch_init (MachineState *ms,KVMState *s) { cap_interrupt_unset =kvm_check_extension (s,KVM_CAP_PPC_UNSET_IRQ ); cap_interrupt_level =kvm_check_extension (s,KVM_CAP_PPC_IRQ_LEVEL );
target-s390x/kvm.c +1 -1
@@ -172,7 +172,7 @@ static void kvm_s390_enable_cmma(KVMState *s)trace_kvm_enable_cmma (rc); } -int kvm_arch_init (KVMState *s) +int kvm_arch_init (MachineState *ms,KVMState *s) { cap_sync_regs =kvm_check_extension (s,KVM_CAP_SYNC_REGS ); cap_async_pf =kvm_check_extension (s,KVM_CAP_ASYNC_PF );
Signature · 1 line
2.1.0
PATCH 5/8 machine: query kvm-shadow-mem machine property rather than qemu opts
In reply to Marcel ApfelbaumFixes a QEMU crash when passing kvm_shadow_mem parameter in command line.
--- hw/core/machine.c | 6 ++++++ include/hw/boards.h | 1 + target-i386/kvm.c | 3 +-- 3 files changed, 8 insertions(+), 2 deletions(-)
hw/core/machine.c +6 -0
@@ -284,6 +284,7 @@ static void machine_initfn(Object *obj)MachineState *ms =MACHINE (obj); ms->kernel_irqchip_allowed = true; + ms->kvm_shadow_mem =-1 ;object_property_add_str (obj,"accel" , machine_get_accel, machine_set_accel,NULL );
@@ -414,6 +415,11 @@ bool machine_kernel_irqchip_required(MachineState *machine)return machine->kernel_irqchip_required; } +int machine_kvm_shadow_mem (MachineState *machine) +{ +return machine->kvm_shadow_mem; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,
include/hw/boards.h +1 -0
@@ -69,6 +69,7 @@ bool machine_usb(MachineState *machine);bool machine_iommu (MachineState *machine);bool machine_kernel_irqchip_allowed (MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine); +int machine_kvm_shadow_mem (MachineState *machine);/** * MachineClass:
target-i386/kvm.c +1 -2
@@ -890,8 +890,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) }qemu_register_reset (kvm_unpoison_all,NULL ); - shadow_mem =qemu_opt_get_size (qemu_get_machine_opts (), -"kvm_shadow_mem" ,-1 ); + shadow_mem =machine_kvm_shadow_mem (ms);if (shadow_mem !=-1 ) { shadow_mem /=4096 ; ret =kvm_vm_ioctl (s,KVM_SET_NR_MMU_PAGES , shadow_mem);
Signature · 1 line
2.1.0
Re: [Qemu-devel] [PATCH 5/8] machine: query kvm-shadow-mem machine property rather than qemu opts
Quoted 3 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing
kvm_shadow_memparameter in command line.Signed-off-by Marcel Apfelbaum <[email protected]>
Please amend commit message:
Commit e79d5a6 ("machine: remove qemu_machine_opts global list")
removed the global option descriptions and moved them to MachineState's QOM properties.
Query kvm-shadow-mem by accessing machine properties through designated wrappers.
Quoted 57 lines
--- hw/core/machine.c | 6 ++++++ include/hw/boards.h | 1 + target-i386/kvm.c | 3 +-- 3 files changed, 8 insertions(+), 2 deletions(-)hw/core/machine.c +6
-0@@ -284,6 +284,7 @@ static void machine_initfn(Object *obj)MachineState *ms =MACHINE (obj); ms->kernel_irqchip_allowed = true; + ms->kvm_shadow_mem =-1 ;object_property_add_str (obj,"accel" , machine_get_accel, machine_set_accel,NULL );@@ -414,6 +415,11 @@ bool machine_kernel_irqchip_required(MachineState *machine)return machine->kernel_irqchip_required; } +int machine_kvm_shadow_mem (MachineState *machine) +{ +return machine->kvm_shadow_mem; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,include/hw/boards.h +1
-0@@ -69,6 +69,7 @@ bool machine_usb(MachineState *machine);bool machine_iommu (MachineState *machine);bool machine_kernel_irqchip_allowed (MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine); +int machine_kvm_shadow_mem (MachineState *machine);/** * MachineClass: target-i386/kvm.c +1
-2@@ -890,8 +890,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) }qemu_register_reset (kvm_unpoison_all,NULL ); - shadow_mem =qemu_opt_get_size (qemu_get_machine_opts (), -"kvm_shadow_mem" ,-1 ); + shadow_mem =machine_kvm_shadow_mem (ms);if (shadow_mem !=-1 ) { shadow_mem /=4096 ; ret =kvm_vm_ioctl (s,KVM_SET_NR_MMU_PAGES , shadow_mem);
Quoted 5 lines · Marcel Apfelbaum
Commit e79d5a6 ("machine: remove
qemu_machine_optsglobal list") removed option descriptions from the -machine QemuOptsList to avoid repeating MachineState's QOM properties.This results in a Qemu crash if a non string option is queried using qemu opts. Fix this by querying machine properties through designated wrappers.
Wrappers vs. direct access to fields is just a matter of taste, so
PATCH 6/8 machine: query phandle-start machine property rather than qemu opts
In reply to Marcel ApfelbaumFixes a QEMU crash when passing phandle_start parameter in command line.
--- device_tree.c | 5 ++--- hw/core/machine.c | 5 +++++ include/hw/boards.h | 1 + 3 files changed, 8 insertions(+), 3 deletions(-)
device_tree.c +2 -3
@@ -24,7 +24,7 @@#include "sysemu/device_tree.h" #include "sysemu/sysemu.h" #include "hw/loader.h" -#include "qemu/option.h" +#include "hw/boards.h" #include "qemu/config-file.h" #include <libfdt.h>
@@ -245,8 +245,7 @@ uint32_t qemu_fdt_alloc_phandle(void *fdt)* which phandle id to start allocting phandles. */ if (!phandle) { - phandle =qemu_opt_get_number (qemu_get_machine_opts (), -"phandle_start" ,0 ); + phandle =machine_phandle_start (current_machine); }if (!phandle) {
hw/core/machine.c +5 -0
@@ -420,6 +420,11 @@ int machine_kvm_shadow_mem(MachineState *machine)return machine->kvm_shadow_mem; } +int machine_phandle_start (MachineState *machine) +{ +return machine->phandle_start; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,
include/hw/boards.h +1 -0
@@ -70,6 +70,7 @@ bool machine_iommu(MachineState *machine);bool machine_kernel_irqchip_allowed (MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine); +int machine_phandle_start (MachineState *machine);/** * MachineClass:
Signature · 1 line
2.1.0
Re: [Qemu-devel] [PATCH 6/8] machine: query phandle-start machine property rather than qemu opts
Quoted 3 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing
phandle_startparameter in command line.Signed-off-by Marcel Apfelbaum <[email protected]>
Please amend commit message:
Commit e79d5a6 ("machine: remove qemu_machine_opts global list")
removed the global option descriptions and moved them to MachineState's QOM properties.
Query phandle-start by accephandle-startssing machine properties through designated wrappers.
Quoted 58 lines
--- device_tree.c | 5 ++--- hw/core/machine.c | 5 +++++ include/hw/boards.h | 1 + 3 files changed, 8 insertions(+), 3 deletions(-)device_tree.c +2
-3@@ -24,7 +24,7 @@#include "sysemu/device_tree.h" #include "sysemu/sysemu.h" #include "hw/loader.h" -#include "qemu/option.h" +#include "hw/boards.h" #include "qemu/config-file.h" #include <libfdt.h> @@ -245,8 +245,7 @@ uint32_t qemu_fdt_alloc_phandle(void *fdt)* which phandle id to start allocting phandles. */ if (!phandle) { - phandle =qemu_opt_get_number (qemu_get_machine_opts (), -"phandle_start" ,0 ); + phandle =machine_phandle_start (current_machine); }if (!phandle) {hw/core/machine.c +5
-0@@ -420,6 +420,11 @@ int machine_kvm_shadow_mem(MachineState *machine)return machine->kvm_shadow_mem; } +int machine_phandle_start (MachineState *machine) +{ +return machine->phandle_start; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,include/hw/boards.h +1
-0@@ -70,6 +70,7 @@ bool machine_iommu(MachineState *machine);bool machine_kernel_irqchip_allowed (MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine); +int machine_phandle_start (MachineState *machine);/** * MachineClass:
Re: [Qemu-devel] [PATCH 6/8] machine: query phandle-start machine property rather than qemu opts
Quoted 4 lines · Marcel Apfelbaum
Quoted 3 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing
phandle_startparameter in command line.Signed-off-by Marcel Apfelbaum <[email protected]>
Quoted 9 lines
Please amend commit message:
Commit e79d5a6 ("machine: remove
qemu_machine_optsglobal list")
removed the global option descriptions and moved them to MachineState's QOM properties.Query phandle-start by accephandle-startssing machine properties through designated wrappers.
Signed-off-by Marcel Apfelbaum <[email protected]>
Sorry for the typo:
Commit e79d5a6 ("machine: remove qemu_machine_opts global list")
removed the global option descriptions and moved them to MachineState's QOM properties.
Query phandle-start by accessing machine properties through designated wrappers.
Quoted 60 lines
Quoted 59 lines
--- device_tree.c | 5 ++--- hw/core/machine.c | 5 +++++ include/hw/boards.h | 1 + 3 files changed, 8 insertions(+), 3 deletions(-)device_tree.c +2
-3@@ -24,7 +24,7 @@#include "sysemu/device_tree.h" #include "sysemu/sysemu.h" #include "hw/loader.h" -#include "qemu/option.h" +#include "hw/boards.h" #include "qemu/config-file.h" #include <libfdt.h> @@ -245,8 +245,7 @@ uint32_t qemu_fdt_alloc_phandle(void *fdt)* which phandle id to start allocting phandles. */ if (!phandle) { - phandle =qemu_opt_get_number (qemu_get_machine_opts (), -"phandle_start" ,0 ); + phandle =machine_phandle_start (current_machine); }if (!phandle) {hw/core/machine.c +5
-0@@ -420,6 +420,11 @@ int machine_kvm_shadow_mem(MachineState *machine)return machine->kvm_shadow_mem; } +int machine_phandle_start (MachineState *machine) +{ +return machine->phandle_start; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,include/hw/boards.h +1
-0@@ -70,6 +70,7 @@ bool machine_iommu(MachineState *machine);bool machine_kernel_irqchip_allowed (MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine); +int machine_phandle_start (MachineState *machine);/** * MachineClass:
Re: [Qemu-devel] [PATCH 6/8] machine: query phandle-start machine property rather than qemu opts
In reply to Marcel ApfelbaumQuoted 10 lines · Marcel Apfelbaum
Quoted 3 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing
phandle_startparameter in command line.Signed-off-by Marcel Apfelbaum <[email protected]>Please amend commit message:
Commit e79d5a6 ("machine: remove
qemu_machine_optsglobal list")
removed the global option descriptions and moved them to MachineState's QOM properties.Query phandle-start by accephandle-startssing machine properties through designated wrappers.
What does this mean?
Quoted 2 lines
Signed-off-by Marcel Apfelbaum <[email protected]>
I don't think this makes it clearer.
Maybe just given an example on how to reproduce the crash?
Quoted 59 lines
Quoted 59 lines
--- device_tree.c | 5 ++--- hw/core/machine.c | 5 +++++ include/hw/boards.h | 1 + 3 files changed, 8 insertions(+), 3 deletions(-)device_tree.c +2
-3@@ -24,7 +24,7 @@#include "sysemu/device_tree.h" #include "sysemu/sysemu.h" #include "hw/loader.h" -#include "qemu/option.h" +#include "hw/boards.h" #include "qemu/config-file.h" #include <libfdt.h> @@ -245,8 +245,7 @@ uint32_t qemu_fdt_alloc_phandle(void *fdt)* which phandle id to start allocting phandles. */ if (!phandle) { - phandle =qemu_opt_get_number (qemu_get_machine_opts (), -"phandle_start" ,0 ); + phandle =machine_phandle_start (current_machine); }if (!phandle) {hw/core/machine.c +5
-0@@ -420,6 +420,11 @@ int machine_kvm_shadow_mem(MachineState *machine)return machine->kvm_shadow_mem; } +int machine_phandle_start (MachineState *machine) +{ +return machine->phandle_start; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,include/hw/boards.h +1
-0@@ -70,6 +70,7 @@ bool machine_iommu(MachineState *machine);bool machine_kernel_irqchip_allowed (MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine); +int machine_phandle_start (MachineState *machine);/** * MachineClass:
Re: [Qemu-devel] [PATCH 6/8] machine: query phandle-start machine property rather than qemu opts
Quoted 5 lines · Michael S. Tsirkin
Quoted 4 lines · Marcel Apfelbaum
Quoted 3 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing
phandle_startparameter in command line.Signed-off-by Marcel Apfelbaum <[email protected]>
Quoted 8 lines
Quoted 6 lines
Please amend commit message:
Commit e79d5a6 ("machine: remove
qemu_machine_optsglobal list")
removed the global option descriptions and moved them to MachineState's QOM properties.Query phandle-start by accephandle-startssing machine properties through designated wrappers.
What does this mean?
Please look at the fixed mail (sent a few seconds later)
Thanks,
Marcel
Quoted 67 lines
Quoted 3 lines
Signed-off-by Marcel Apfelbaum <[email protected]>I don't think this makes it clearer.
Maybe just given an example on how to reproduce the crash?Quoted 59 lines
Quoted 59 lines
--- device_tree.c | 5 ++--- hw/core/machine.c | 5 +++++ include/hw/boards.h | 1 + 3 files changed, 8 insertions(+), 3 deletions(-)device_tree.c +2
-3@@ -24,7 +24,7 @@#include "sysemu/device_tree.h" #include "sysemu/sysemu.h" #include "hw/loader.h" -#include "qemu/option.h" +#include "hw/boards.h" #include "qemu/config-file.h" #include <libfdt.h> @@ -245,8 +245,7 @@ uint32_t qemu_fdt_alloc_phandle(void *fdt)* which phandle id to start allocting phandles. */ if (!phandle) { - phandle =qemu_opt_get_number (qemu_get_machine_opts (), -"phandle_start" ,0 ); + phandle =machine_phandle_start (current_machine); }if (!phandle) {hw/core/machine.c +5
-0@@ -420,6 +420,11 @@ int machine_kvm_shadow_mem(MachineState *machine)return machine->kvm_shadow_mem; } +int machine_phandle_start (MachineState *machine) +{ +return machine->phandle_start; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,include/hw/boards.h +1
-0@@ -70,6 +70,7 @@ bool machine_iommu(MachineState *machine);bool machine_kernel_irqchip_allowed (MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine); +int machine_phandle_start (MachineState *machine);/** * MachineClass:
PATCH 7/8 machine: query dump-guest-core machine property rather than qemu opts
In reply to Marcel ApfelbaumFixes a QEMU crash when passing dump_guest_core parameter in command line.
--- exec.c | 4 ++-- hw/core/machine.c | 6 ++++++ include/hw/boards.h | 1 + 3 files changed, 9 insertions(+), 2 deletions(-)
exec.c +2 -2
@@ -26,6 +26,7 @@#include "cpu.h" #include "tcg.h" #include "hw/hw.h" +#include "hw/boards.h" #include "hw/qdev.h" #include "qemu/osdep.h" #include "sysemu/kvm.h"
@@ -1213,8 +1214,7 @@ static void qemu_ram_setup_dump(void *addr, ram_addr_t size)int ret;/* Use MADV_DONTDUMP, if user doesn't want the guest memory in the core */ -if (!qemu_opt_get_bool (qemu_get_machine_opts (), -"dump-guest-core" , true)) { +if (!machine_dump_guest_core (current_machine)) { ret =qemu_madvise (addr, size,QEMU_MADV_DONTDUMP );if (ret) {perror ("qemu_madvise" );
hw/core/machine.c +6 -0
@@ -285,6 +285,7 @@ static void machine_initfn(Object *obj) ms->kernel_irqchip_allowed = true; ms->kvm_shadow_mem =-1 ; + ms->dump_guest_core = true;object_property_add_str (obj,"accel" , machine_get_accel, machine_set_accel,NULL );
@@ -425,6 +426,11 @@ int machine_phandle_start(MachineState *machine)return machine->phandle_start; } +bool machine_dump_guest_core (MachineState *machine) +{ +return machine->dump_guest_core; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,
include/hw/boards.h +1 -0
@@ -71,6 +71,7 @@ bool machine_kernel_irqchip_allowed(MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine);int machine_phandle_start (MachineState *machine); +bool machine_dump_guest_core (MachineState *machine);/** * MachineClass:
Signature · 1 line
2.1.0
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.
Explain that, please?
Quoted 21 lines
Signed-off-by Marcel Apfelbaum <[email protected]>--- exec.c | 4 ++-- hw/core/machine.c | 6 ++++++ include/hw/boards.h | 1 + 3 files changed, 9 insertions(+), 2 deletions(-)exec.c +1
-0@@ -26,6 +26,7 @@#include "cpu.h" #include "tcg.h" #include "hw/hw.h" +#include "hw/boards.h" #include "hw/qdev.h" #include "qemu/osdep.h" #include "sysemu/kvm.h"
exec.c static void qemu_ram_setup_dump(void *addr, ram_addr_t size) · line 1217
Quoted 6 lines
exec.c +1
-2@@ -1213,8 +1214,7 @@ static void qemu_ram_setup_dump(void *addr, ram_addr_t size)int ret;/* Use MADV_DONTDUMP, if user doesn't want the guest memory in the core */ -if (!qemu_opt_get_bool (qemu_get_machine_opts (), -"dump-guest-core" , true)) { +if (!machine_dump_guest_core (current_machine)) {
linux-user doesn't have a current_machine, and machine.c shouldn't even be compiled in, so I'm guessing this function is #ifdef'ed, which means the header should be #ifdef'ed, too. hw/boards.h has no business being in *-user.
Regards,
Andreas
Quoted 40 lines
exec.c +0
-0ret =qemu_madvise (addr, size,QEMU_MADV_DONTDUMP );if (ret) {perror ("qemu_madvise" );hw/core/machine.c +6
-0@@ -285,6 +285,7 @@ static void machine_initfn(Object *obj) ms->kernel_irqchip_allowed = true; ms->kvm_shadow_mem =-1 ; + ms->dump_guest_core = true;object_property_add_str (obj,"accel" , machine_get_accel, machine_set_accel,NULL );@@ -425,6 +426,11 @@ int machine_phandle_start(MachineState *machine)return machine->phandle_start; } +bool machine_dump_guest_core (MachineState *machine) +{ +return machine->dump_guest_core; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,include/hw/boards.h +1
-0@@ -71,6 +71,7 @@ bool machine_kernel_irqchip_allowed(MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine);int machine_phandle_start (MachineState *machine); +bool machine_dump_guest_core (MachineState *machine);/** * MachineClass:
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
Quoted 6 lines · Andreas Färber
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
Quoted 82 lines
Quoted 21 lines
Signed-off-by Marcel Apfelbaum <[email protected]>--- exec.c | 4 ++-- hw/core/machine.c | 6 ++++++ include/hw/boards.h | 1 + 3 files changed, 9 insertions(+), 2 deletions(-)exec.c +1
-0@@ -26,6 +26,7 @@#include "cpu.h" #include "tcg.h" #include "hw/hw.h" +#include "hw/boards.h" #include "hw/qdev.h" #include "qemu/osdep.h" #include "sysemu/kvm.h" exec.c static void qemu_ram_setup_dump(void *addr, ram_addr_t size) · line 1217
Quoted 6 lines
exec.c +1
-2@@ -1213,8 +1214,7 @@ static void qemu_ram_setup_dump(void *addr, ram_addr_t size)int ret;/* Use MADV_DONTDUMP, if user doesn't want the guest memory in the core */ -if (!qemu_opt_get_bool (qemu_get_machine_opts (), -"dump-guest-core" , true)) { +if (!machine_dump_guest_core (current_machine)) {linux-user doesn't have a
current_machine, andmachine.cshouldn't even be compiled in, so I'm guessing this function is #ifdef'ed, which means the header should be #ifdef'ed, too.hw/boards.hhas no business being in *-user.Regards,
AndreasQuoted 40 lines
exec.c +0
-0ret =qemu_madvise (addr, size,QEMU_MADV_DONTDUMP );if (ret) {perror ("qemu_madvise" );hw/core/machine.c +6
-0@@ -285,6 +285,7 @@ static void machine_initfn(Object *obj) ms->kernel_irqchip_allowed = true; ms->kvm_shadow_mem =-1 ; + ms->dump_guest_core = true;object_property_add_str (obj,"accel" , machine_get_accel, machine_set_accel,NULL );@@ -425,6 +426,11 @@ int machine_phandle_start(MachineState *machine)return machine->phandle_start; } +bool machine_dump_guest_core (MachineState *machine) +{ +return machine->dump_guest_core; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,include/hw/boards.h +1
-0@@ -71,6 +71,7 @@ bool machine_kernel_irqchip_allowed(MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine);int machine_phandle_start (MachineState *machine); +bool machine_dump_guest_core (MachineState *machine);/** * MachineClass: Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
Quoted 6 lines · Andreas Färber
Quoted 5 lines
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.
Quoted 7 lines
Quoted 1 line
Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
No, since the patch is not in qemu.git (it builds!) it is not too late to fix it, nor too late to ask why a patch that introduces a breakage does what it does. (Moving the info from the cover letter into the commit message would've been a good idea, Marcel.)
All QEMU patches are supposed to be bisectable. It's our job as maintainers to build-test each. If you do that 1 month later, that's not my fault.
Regards,
Andreas
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
Quoted 12 lines · Andreas Färber
Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
Quoted 11 lines · Andreas Färber
Quoted 6 lines
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
Quoted 6 lines
Quoted 2 lines
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
No, since the patch is not in qemu.git (it builds!) it is not too late to fix it, nor too late to ask why a patch that introduces a breakage does what it does.
Getting review that late is decidedly suboptimal, but no excuse to invoke maintainer privilege to ram the patch through unchanged.
Cosmetic issues can be tidied up on top. The ongoing review may produce nothing but cosmetic issues, but we don't know that, yet.
Commit messages can't be tidied up on top, and they're dirt cheap to improve right in place, so let's do that, please.
[...]
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
Quoted 21 lines · Markus Armbruster
Quoted 12 lines · Andreas Färber
Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
Quoted 11 lines · Andreas Färber
Quoted 6 lines
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
Quoted 6 lines
Quoted 2 lines
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
No, since the patch is not in qemu.git (it builds!) it is not too late to fix it, nor too late to ask why a patch that introduces a breakage does what it does.
Quoted 5 lines
Getting review that late is decidedly suboptimal, but no excuse to invoke maintainer privilege to ram the patch through unchanged.
Cosmetic issues can be tidied up on top. The ongoing review may produce nothing but cosmetic issues, but we don't know that, yet.
Cool, review is good. What I wanted to say though is that I'm not holding up a patchset that's been around for a month just because of cosmetics and basic questions.
So I intend to send pull request this evening - I don't think we want to live with known crashers any longer - crashes waste tester's time.
Quoted 4 lines
Commit messages can't be tidied up on top, and they're dirt cheap to improve right in place, so let's do that, please.
[...]
Sure. Marcel, can you pls supply the command line that produces the crash? I'll include that.
Signature · 1 line
MST
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
Quoted 34 lines · Michael S. Tsirkin
Quoted 21 lines · Markus Armbruster
Quoted 12 lines · Andreas Färber
Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
Quoted 11 lines · Andreas Färber
Quoted 6 lines
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
Quoted 6 lines
Quoted 2 lines
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
No, since the patch is not in qemu.git (it builds!) it is not too late to fix it, nor too late to ask why a patch that introduces a breakage does what it does.
Quoted 5 lines
Getting review that late is decidedly suboptimal, but no excuse to invoke maintainer privilege to ram the patch through unchanged.
Cosmetic issues can be tidied up on top. The ongoing review may produce nothing but cosmetic issues, but we don't know that, yet.
Cool, review is good. What I wanted to say though is that I'm not holding up a patchset that's been around for a month just because of cosmetics and basic questions.
So I intend to send pull request this evening - I don't think we want to live with known crashers any longer - crashes waste tester's time.
Quoted 7 lines
Quoted 4 lines
Commit messages can't be tidied up on top, and they're dirt cheap to improve right in place, so let's do that, please.
[...]
Sure. Marcel, can you pls supply the command line that produces the crash? I'll include that.
Sure you just have to use the option:
qemu-bin ... -machine pc,dump-guest-core=on
x86_64-softmmu/qemu-system-x86_64 -machine pc,dump-guest-core=on
qemu-system-x86_64: qemu/util/qemu-option.c:387: qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed. Aborted (core dumped)
Thanks,
Marcel
Quoted 1 line
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
In reply to Andreas FärberQuoted 12 lines · Andreas Färber
Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
Quoted 11 lines · Andreas Färber
Quoted 6 lines
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
Quoted 6 lines
Quoted 2 lines
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
No, since the patch is not in qemu.git (it builds!) it is not too late to fix it, nor too late to ask why a patch that introduces a breakage does what it does.
I tried to say that I'm not holding this patch set up because there are some basic questions. Paolo reviewed it and gave an ack. If others want to re-start review 1 month afterwards, that's fine, but I don't want to defer pull request with this any longer. If someone can quickly spot a serious non-cosmetic problem there, that's another matter, and would make me defer the pull request.
Quoted 2 lines
(Moving the info from the cover letter into the commit message would've been a good idea, Marcel.)
I can tweak commit messages, sure, since that does not require re-testing it all.
Quoted 6 lines
All QEMU patches are supposed to be bisectable. It's our job as maintainers to build-test each. If you do that 1 month later, that's not my fault.
Regards,
Andreas
I have this patch in my tree and there's
no bisect issue, just test-built before and after this patch. That's because I had the ifdefs in boards.h which you and Peter objected to, but that is about cosmetics, I fixed that with a patch on top to hopefully make you both happy.
Don't take my word for it, you can check out my tree and verify, that would be very wellcome.
Quoted 4 lines
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
Am 11.03.2015 um 09:56 schrieb Michael S. Tsirkin:
Quoted 40 lines · Andreas Färber
Quoted 12 lines
Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
Quoted 11 lines · Andreas Färber
Quoted 6 lines
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
Quoted 6 lines
Quoted 2 lines
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
No, since the patch is not in qemu.git (it builds!) it is not too late to fix it, nor too late to ask why a patch that introduces a breakage does what it does.
I tried to say that I'm not holding this patch set up because there are some basic questions. Paolo reviewed it and gave an ack. If others want to re-start review 1 month afterwards, that's fine, but I don't want to defer pull request with this any longer. If someone can quickly spot a serious non-cosmetic problem there, that's another matter, and would make me defer the pull request.
Quoted 2 lines
(Moving the info from the cover letter into the commit message would've been a good idea, Marcel.)
I can tweak commit messages, sure, since that does not require re-testing it all.
Quoted 4 lines
All QEMU patches are supposed to be bisectable. It's our job as maintainers to build-test each. If you do that 1 month later, that's not my fault.
Quoted 8 lines
Quoted 2 lines
Regards,
AndreasI have this patch in my tree and there's
no bisect issue, just test-built before and after this patch. That's because I had the ifdefs inboards.hwhich you and Peter objected to, but that is about cosmetics, I fixed that with a patch on top to hopefully make you both happy.
All I was asking for is, please squash the patch(es) that fix(es) the build issue. In particular if you applied the patch just yesterday when we complained. We've been required to, so I expect the same rules to apply to everyone.
In order to propose a better fix I tried to understand what the patch is fixing, that's all. If an improvement of the commit message comes out of that, good, but that was not the main purpose.
Thanks,
Andreas
P.S. I was sick most of February and my Chromebook has a broken DRM driver, not allowing for much bedside-hacking. ;)
Quoted 8 lines
Don't take my word for it, you can check out my tree and verify, that would be very wellcome.
Quoted 4 lines
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
Quoted 59 lines · Andreas Färber
Am 11.03.2015 um 09:56 schrieb Michael S. Tsirkin:
Quoted 40 lines · Andreas Färber
Quoted 12 lines
Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
Quoted 11 lines · Andreas Färber
Quoted 6 lines
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
Quoted 6 lines
Quoted 2 lines
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
No, since the patch is not in qemu.git (it builds!) it is not too late to fix it, nor too late to ask why a patch that introduces a breakage does what it does.
I tried to say that I'm not holding this patch set up because there are some basic questions. Paolo reviewed it and gave an ack. If others want to re-start review 1 month afterwards, that's fine, but I don't want to defer pull request with this any longer. If someone can quickly spot a serious non-cosmetic problem there, that's another matter, and would make me defer the pull request.
Quoted 2 lines
(Moving the info from the cover letter into the commit message would've been a good idea, Marcel.)
I can tweak commit messages, sure, since that does not require re-testing it all.
Quoted 4 lines
All QEMU patches are supposed to be bisectable. It's our job as maintainers to build-test each. If you do that 1 month later, that's not my fault.
Quoted 8 lines
Quoted 2 lines
Regards,
AndreasI have this patch in my tree and there's
no bisect issue, just test-built before and after this patch. That's because I had the ifdefs inboards.hwhich you and Peter objected to, but that is about cosmetics, I fixed that with a patch on top to hopefully make you both happy.All I was asking for is, please squash the patch(es) that fix(es) the build issue. In particular if you applied the patch just yesterday when we complained. We've been required to, so I expect the same rules to apply to everyone.
In order to propose a better fix I tried to understand what the patch is fixing, that's all. If an improvement of the commit message comes out of that, good, but that was not the main purpose.
Quoted 5 lines
Thanks,
Andreas
P.S. I was sick most of February and my Chromebook has a broken DRM driver, not allowing for much bedside-hacking. ;)
Hi Andreas,
I hope you are feeling better now!
The main issue I see here (and believe me is not the reviews, they are always welcomed!) is that more than a month ago several developers complained about these crashes. I stopped what I was doing and posted a series ASAP that was almost immediately reviewed by Paolo.
I pinged twice already and nobody did anything about it. Michael took it because nobody else did and now we have a situation:
"No good deed goes unpunished"
Now, we need a way to not let this happen.
I am afraid that next time I will not get lucky and nobody will take the patches :(.
Thanks,
Marcel
Quoted 11 lines
Quoted 9 lines
Don't take my word for it, you can check out my tree and verify, that would be very wellcome.
Quoted 4 lines
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
In reply to Andreas FärberQuoted 44 lines · Andreas Färber
Am 11.03.2015 um 09:56 schrieb Michael S. Tsirkin:
Quoted 43 lines · Andreas Färber
Quoted 12 lines
Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
Quoted 11 lines · Andreas Färber
Quoted 6 lines
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
Quoted 6 lines
Quoted 2 lines
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
No, since the patch is not in qemu.git (it builds!) it is not too late to fix it, nor too late to ask why a patch that introduces a breakage does what it does.
I tried to say that I'm not holding this patch set up because there are some basic questions. Paolo reviewed it and gave an ack. If others want to re-start review 1 month afterwards, that's fine, but I don't want to defer pull request with this any longer. If someone can quickly spot a serious non-cosmetic problem there, that's another matter, and would make me defer the pull request.
Quoted 2 lines
(Moving the info from the cover letter into the commit message would've been a good idea, Marcel.)
I can tweak commit messages, sure, since that does not require re-testing it all.
Quoted 6 lines
All QEMU patches are supposed to be bisectable. It's our job as maintainers to build-test each. If you do that 1 month later, that's not my fault.
Regards,
Andreas
Quoted 8 lines
Quoted 5 lines
I have this patch in my tree and there's
no bisect issue, just test-built before and after this patch. That's because I had the ifdefs inboards.hwhich you and Peter objected to, but that is about cosmetics, I fixed that with a patch on top to hopefully make you both happy.All I was asking for is, please squash the patch(es) that fix(es) the build issue.
Hmm as far as I can see, there's no build issue. My patch is required before Marcel's one. Then there's another one by me to address the comments you had.
I could squash them all but this just messes up attribution, bisect build is fine, and I verified that.
Quoted 8 lines
In particular if you applied the patch just yesterday when we complained. We've been required to, so I expect the same rules to apply to everyone.
In order to propose a better fix I tried to understand what the patch is fixing, that's all. If an improvement of the commit message comes out of that, good, but that was not the main purpose.
Quoted 5 lines
Thanks,
Andreas
P.S. I was sick most of February and my Chromebook has a broken DRM driver, not allowing for much bedside-hacking. ;)
I certainly didn't intend to blame anyone, sorry if it sounded like that. I was merely saying, it's been on review for a while, got ack and not objections, so let's apply it unless someone sees significant issues, I don't want to hold it up until all questions are answered.
Quoted 14 lines
Quoted 8 lines
Don't take my word for it, you can check out my tree and verify, that would be very wellcome.
Quoted 4 lines
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
In reply to Michael S. TsirkinQuoted 21 lines · Michael S. Tsirkin
Quoted 12 lines · Andreas Färber
Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
Quoted 11 lines · Andreas Färber
Quoted 6 lines
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
Quoted 6 lines
Quoted 2 lines
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
No, since the patch is not in qemu.git (it builds!) it is not too late to fix it, nor too late to ask why a patch that introduces a breakage does what it does.
Quoted 7 lines
I tried to say that I'm not holding this patch set up because there are some basic questions. Paolo reviewed it and gave an ack. If others want to re-start review 1 month afterwards, that's fine, but I don't want to defer pull request with this any longer. If someone can quickly spot a serious non-cosmetic problem there, that's another matter, and would make me defer the pull request.
Okay, sounds much more reasonable than your first reply. Thanks for clarifying.
[...]
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
In reply to Andreas FärberQuoted 12 lines · Andreas Färber
Am 10.03.2015 um 22:24 schrieb Michael S. Tsirkin:
Quoted 11 lines · Andreas Färber
Quoted 6 lines
Hi,
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 1 line
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Explain that, please?
Pls note the submission date. It's 1 month late to ask for basic clarifications.
Quoted 7 lines
Quoted 2 lines
I've merged the patches, I'll fix up issues such as prettifying includes by adding patches on top.
No, since the patch is not in qemu.git (it builds!) it is not too late to fix it, nor too late to ask why a patch that introduces a breakage does what it does. (Moving the info from the cover letter into the commit message would've been a good idea, Marcel.)
Hi Andreas,
Agreed.
Thanks,
Marcel
Quoted 8 lines
All QEMU patches are supposed to be bisectable. It's our job as maintainers to build-test each. If you do that 1 month later, that's not my fault.
Regards,
Andreas
Re: [Qemu-devel] [PATCH 7/8] machine: query dump-guest-core machine property rather than qemu opts
In reply to Marcel ApfelbaumQuoted 3 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing
dump_guest_coreparameter in command line.Signed-off-by Marcel Apfelbaum <[email protected]>
Please amend commit message:
Running
qemu-bin ... -machine pc,dump-guest-core=on
leads to crash:
x86_64-softmmu/qemu-system-x86_64 -machine pc,dump-guest-core=on
qemu-system-x86_64: qemu/util/qemu-option.c:387: qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed.
Aborted (core dumped)This happens because the commit e79d5a6 ("machine: remove qemu_machine_opts global list") removed the global option descriptions and moved them to MachineState's QOM properties.
Fix this by querying machine properties through designated wrappers.
Quoted 65 lines
--- exec.c | 4 ++-- hw/core/machine.c | 6 ++++++ include/hw/boards.h | 1 + 3 files changed, 9 insertions(+), 2 deletions(-)exec.c +2
-2@@ -26,6 +26,7 @@#include "cpu.h" #include "tcg.h" #include "hw/hw.h" +#include "hw/boards.h" #include "hw/qdev.h" #include "qemu/osdep.h" #include "sysemu/kvm.h" @@ -1213,8 +1214,7 @@ static void qemu_ram_setup_dump(void *addr, ram_addr_t size)int ret;/* Use MADV_DONTDUMP, if user doesn't want the guest memory in the core */ -if (!qemu_opt_get_bool (qemu_get_machine_opts (), -"dump-guest-core" , true)) { +if (!machine_dump_guest_core (current_machine)) { ret =qemu_madvise (addr, size,QEMU_MADV_DONTDUMP );if (ret) {perror ("qemu_madvise" );hw/core/machine.c +6
-0@@ -285,6 +285,7 @@ static void machine_initfn(Object *obj) ms->kernel_irqchip_allowed = true; ms->kvm_shadow_mem =-1 ; + ms->dump_guest_core = true;object_property_add_str (obj,"accel" , machine_get_accel, machine_set_accel,NULL );@@ -425,6 +426,11 @@ int machine_phandle_start(MachineState *machine)return machine->phandle_start; } +bool machine_dump_guest_core (MachineState *machine) +{ +return machine->dump_guest_core; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,include/hw/boards.h +1
-0@@ -71,6 +71,7 @@ bool machine_kernel_irqchip_allowed(MachineState *machine);bool machine_kernel_irqchip_required (MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine);int machine_phandle_start (MachineState *machine); +bool machine_dump_guest_core (MachineState *machine);/** * MachineClass:
PATCH 8/8 machine: query mem-merge machine property rather than qemu opts
In reply to Marcel ApfelbaumFixes a QEMU crash when passing mem_merge parameter in command line.
--- exec.c | 2 +- hw/core/machine.c | 6 ++++++ include/hw/boards.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-)
exec.c +1 -1
@@ -1277,7 +1277,7 @@ void qemu_ram_unset_idstr(ram_addr_t addr)static int memory_try_enable_merging (void *addr,size_t len) { -if (!qemu_opt_get_bool (qemu_get_machine_opts (),"mem-merge" , true)) { +if (!machine_mem_merge (current_machine)) {/* disabled by the user */ return 0 ; }
hw/core/machine.c +6 -0
@@ -286,6 +286,7 @@ static void machine_initfn(Object *obj) ms->kernel_irqchip_allowed = true; ms->kvm_shadow_mem =-1 ; ms->dump_guest_core = true; + ms->mem_merge = true;object_property_add_str (obj,"accel" , machine_get_accel, machine_set_accel,NULL );
@@ -431,6 +432,11 @@ bool machine_dump_guest_core(MachineState *machine)return machine->dump_guest_core; } +bool machine_mem_merge (MachineState *machine) +{ +return machine->mem_merge; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,
include/hw/boards.h +1 -0
@@ -72,6 +72,7 @@ bool machine_kernel_irqchip_required(MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine);int machine_phandle_start (MachineState *machine);bool machine_dump_guest_core (MachineState *machine); +bool machine_mem_merge (MachineState *machine);/** * MachineClass:
Signature · 1 line
2.1.0
Re: [Qemu-devel] [PATCH 8/8] machine: query mem-merge machine property rather than qemu opts
Quoted 3 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing
mem_mergeparameter in command line.Signed-off-by Marcel Apfelbaum <[email protected]>
Can you rebase this one on top of my pci branch pls?
Quoted 57 lines
--- exec.c | 2 +- hw/core/machine.c | 6 ++++++ include/hw/boards.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-)exec.c +1
-1@@ -1277,7 +1277,7 @@ void qemu_ram_unset_idstr(ram_addr_t addr)static int memory_try_enable_merging (void *addr,size_t len) { -if (!qemu_opt_get_bool (qemu_get_machine_opts (),"mem-merge" , true)) { +if (!machine_mem_merge (current_machine)) {/* disabled by the user */ return 0 ; }hw/core/machine.c +6
-0@@ -286,6 +286,7 @@ static void machine_initfn(Object *obj) ms->kernel_irqchip_allowed = true; ms->kvm_shadow_mem =-1 ; ms->dump_guest_core = true; + ms->mem_merge = true;object_property_add_str (obj,"accel" , machine_get_accel, machine_set_accel,NULL );@@ -431,6 +432,11 @@ bool machine_dump_guest_core(MachineState *machine)return machine->dump_guest_core; } +bool machine_mem_merge (MachineState *machine) +{ +return machine->mem_merge; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,include/hw/boards.h +1
-0@@ -72,6 +72,7 @@ bool machine_kernel_irqchip_required(MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine);int machine_phandle_start (MachineState *machine);bool machine_dump_guest_core (MachineState *machine); +bool machine_mem_merge (MachineState *machine);/** * MachineClass: Signature · 1 line
2.1.0
Re: [Qemu-devel] [PATCH 8/8] machine: query mem-merge machine property rather than qemu opts
Quoted 6 lines · Michael S. Tsirkin
Quoted 3 lines · Marcel Apfelbaum
Fixes a QEMU crash when passing
mem_mergeparameter in command line.Signed-off-by Marcel Apfelbaum <[email protected]>Can you rebase this one on top of my pci branch pls?
I'll submit again only 8/8 after rebase.
Thanks,
Marcel
Quoted 58 lines
Quoted 58 lines
--- exec.c | 2 +- hw/core/machine.c | 6 ++++++ include/hw/boards.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-)exec.c +1
-1@@ -1277,7 +1277,7 @@ void qemu_ram_unset_idstr(ram_addr_t addr)static int memory_try_enable_merging (void *addr,size_t len) { -if (!qemu_opt_get_bool (qemu_get_machine_opts (),"mem-merge" , true)) { +if (!machine_mem_merge (current_machine)) {/* disabled by the user */ return 0 ; }hw/core/machine.c +6
-0@@ -286,6 +286,7 @@ static void machine_initfn(Object *obj) ms->kernel_irqchip_allowed = true; ms->kvm_shadow_mem =-1 ; ms->dump_guest_core = true; + ms->mem_merge = true;object_property_add_str (obj,"accel" , machine_get_accel, machine_set_accel,NULL );@@ -431,6 +432,11 @@ bool machine_dump_guest_core(MachineState *machine)return machine->dump_guest_core; } +bool machine_mem_merge (MachineState *machine) +{ +return machine->mem_merge; +} +static const TypeInfo machine_info = { .name =TYPE_MACHINE , .parent =TYPE_OBJECT ,include/hw/boards.h +1
-1@@ -72,6 +72,7 @@ bool machine_kernel_irqchip_required(MachineState *machine);int machine_kvm_shadow_mem (MachineState *machine);int machine_phandle_start (MachineState *machine);bool machine_dump_guest_core (MachineState *machine); +bool machine_mem_merge (MachineState *machine);/** * MachineClass: -- 2.1.0
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 12 lines
Commit e79d5a6 ("machine: remove
qemu_machine_optsglobal list") removed option descriptions from the -machine QemuOptsList to avoid repeating MachineState's QOM properties.This results in a Qemu crash if a non string option is queried using qemu opts. Fix this by querying machine properties through designated wrappers.
I hope I didn't miss anything.
Comments are appreciated as always.Thanks,
Marcel
Quoted 9 lines
Marcel Apfelbaum (8):
machine: query iommu machine property rather than qemu opts hw/machine: kernel-irqchip property support for allowed/required
machine: query kernel-irqchip machine property rather than qemu opts
kvm: add machine state tokvm_arch_init
machine: query kvm-shadow-mem machine property rather than qemu opts
machine: query phandle-start machine property rather than qemu opts
machine: query dump-guest-core machine property rather than qemu opts
machine: query mem-merge machine property rather than qemu opts
In general this seems to work.
I have a question, though:
What I like is a way to do some wrappers in the specific machines. For example, we plan to add
static inline void s390_machine_initfn(Object *obj)
{
object_property_add_bool(obj, "aes-key-wrap",
machine_get_aes_key_wrap,
machine_set_aes_key_wrap, NULL);
object_property_set_description(obj, "aes-key-wrap",
"enable/disable AES key wrapping using the CPACF wrapping key",
NULL);
object_property_add_bool(obj, "dea-key-wrap",
machine_get_dea_key_wrap,
machine_set_dea_key_wrap, NULL);
object_property_set_description(obj, "dea-key-wrap",
"enable/disable DEA key wrapping using the CPACF wrapping key",
NULL);
}Previously we used
if (qemu_opt_get_bool(opts, "aes-key-wrap", false))
if target-s390/kvm.c
to query that.
Now, these options are pretty specific to s390 and adding them to hw/core/machine.c to create wrappers seems strange. So implementing them in hw/s390/s390-virtio-ccw.c seems a much better place.
Would a function there that does gets S390_CCW_MACHINE(current_machine)->aes_key_wrap considered ok, or do we need to pollute hw/core/machine.c with architecture specific options?
Christian
PS: The same is somewhat true for qemu-options.hx. Having such specific machine option in the global help offers room for improvement
Quoted 23 lines · Christian Borntraeger
Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
Quoted 12 lines
Commit e79d5a6 ("machine: remove
qemu_machine_optsglobal list") removed option descriptions from the -machine QemuOptsList to avoid repeating MachineState's QOM properties.This results in a Qemu crash if a non string option is queried using qemu opts. Fix this by querying machine properties through designated wrappers.
I hope I didn't miss anything.
Comments are appreciated as always.Thanks,
MarcelQuoted 9 lines
Marcel Apfelbaum (8):
machine: query iommu machine property rather than qemu opts hw/machine: kernel-irqchip property support for allowed/required
machine: query kernel-irqchip machine property rather than qemu opts
kvm: add machine state tokvm_arch_init
machine: query kvm-shadow-mem machine property rather than qemu opts
machine: query phandle-start machine property rather than qemu opts
machine: query dump-guest-core machine property rather than qemu opts
machine: query mem-merge machine property rather than qemu opts
Quoted 1 line
In general this seems to work.
Thanks!
Quoted 6 lines
I have a question, though:
What I like is a way to do some wrappers in the specific machines. For example, we plan to add
static inline void
s390_machine_initfn(Object *obj)
Quoted 14 lines
{
object_property_add_bool(obj, "aes-key-wrap",
machine_get_aes_key_wrap,
machine_set_aes_key_wrap, NULL);
object_property_set_description(obj, "aes-key-wrap",
"enable/disable AES key wrapping using the CPACF wrapping key",
NULL);
object_property_add_bool(obj, "dea-key-wrap",
machine_get_dea_key_wrap,
machine_set_dea_key_wrap, NULL);
object_property_set_description(obj, "dea-key-wrap",
"enable/disable DEA key wrapping using the CPACF wrapping key",
NULL);
}OK, You add 2 QOM properties to TYPE_S390_CCW_MACHINE instances.
Of course your setters/getters need to save the property values into an actual field, so you will need a S390State (derived from MachineState)
typedef struct {
MachineState parent;
bool aes_key_wrap;
...
} S390State
Quoted 10 lines
Previously we used
if (qemu_opt_get_bool(opts, "aes-key-wrap", false))
iftarget-s390/kvm.c
to query that.Now, these options are pretty specific to s390 and adding them to
hw/core/machine.cto create wrappers seems strange.
Completely agree. This is the reason we wanted options(properties) per machine type.
So implementing them in hw/s390/s390-virtio-ccw.c
Quoted 1 line
seems a much better place.
Indeed.
Quoted 3 lines
Would a function there that does gets S390_CCW_MACHINE(current_machine)->aes_key_wrap considered ok, or do we need to pollute
hw/core/machine.cwith architecture specific options?
We surely don't add this to hw/core/machine.c because is specific to TYPE_S390_CCW_MACHINE.
Let's say you want to use this property in kvm_arch_init of target-s390x/kvm.c.
- After this series you already have an instance of MachineState initialized with your new properties.
- My assumption is that TYPE_S390_CCW_MACHINE is the only s390 machine or the base type of all s390 machines.
You have three options here:
1. Use QOM infrastucture:
bool aes_key_wrap = object_property_get_bool(OBJECT(machine), "aes-key-wrap", NULL);
2. Add a wrapper somewhere in include/hw/s390x/
that gets MachineState, cast it into S390State and return the field value.
3. Directly downcast MachineState to S390State and get the value.
* All of the above use my assumption that all s390 machines derive from this one.
Quoted 5 lines
Christian
PS: The same is somewhat true for qemu-options.hx. Having such specific machine option in the global help offers room for improvement
Can you please elaborate? I didn't fully understand what exactly are you referring to.
Hope I helped,
Marcel
Quoted 2 lines
Am 04.02.2015 um 22:35 schrieb Marcel Apfelbaum:
[...]
Quoted 10 lines
Quoted 3 lines
Would a function there that does gets S390_CCW_MACHINE(current_machine)->aes_key_wrap considered ok, or do we need to pollute
hw/core/machine.cwith architecture specific options?We surely don't add this to
hw/core/machine.cbecause is specific toTYPE_S390_CCW_MACHINE.Let's say you want to use this property in
kvm_arch_initoftarget-s390x/kvm.c.
- After this series you already have an instance of MachineState initialized with your new properties.
- My assumption is thatTYPE_S390_CCW_MACHINEis the only s390 machine or the base type of all s390 machines.
You have three options here:
1. Use QOM infrastucture:
Quoted 5 lines
bool aes_key_wrap = object_property_get_bool(OBJECT(machine), "aes-key-wrap", NULL);2. Add a wrapper somewhere in
include/hw/s390x/
that gets MachineState, cast it into S390State and return the field value.
3. Directly downcast MachineState to S390State and get the value.
* All of the above use my assumption that all s390 machines derive from this one.
Yes, we derive CCW_MACHINE from the base machine type. Thanks
Quoted 7 lines
Quoted 6 lines
Christian
PS: The same is somewhat true for qemu-options.hx. Having such specific machine option in the global help offers room for improvement
Can you please elaborate? I didn't fully understand what exactly are you referring to.
lets take for example vmport. This is only relevant for x86, but it seems that there is nothing like QEMU_ARCH_ALL QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 for machine properties. Now that we actually move this into the backends, we could do it here as well.
Quoted 2 lines
Hope I helped,
Marcel
Absolutely
Quoted 8 lines · Marcel Apfelbaum
Commit e79d5a6 ("machine: remove
qemu_machine_optsglobal list") removed option descriptions from the -machine QemuOptsList to avoid repeating MachineState's QOM properties.This results in a Qemu crash if a non string option is queried using qemu opts. Fix this by querying machine properties through designated wrappers.
I hope I didn't miss anything.
Comments are appreciated as always.
Ping
Hi, who can take this series?
It solves some command line bugs and people were pretty annoyed about it.
Thanks,
Marcel
Quoted 30 lines
Thanks,
MarcelMarcel Apfelbaum (8):
machine: query iommu machine property rather than qemu opts hw/machine: kernel-irqchip property support for allowed/required
machine: query kernel-irqchip machine property rather than qemu opts
kvm: add machine state tokvm_arch_init
machine: query kvm-shadow-mem machine property rather than qemu opts
machine: query phandle-start machine property rather than qemu opts
machine: query dump-guest-core machine property rather than qemu opts
machine: query mem-merge machine property rather than qemu optsdevice_tree.c | 5 ++--- exec.c | 6 +++--- hw/core/machine.c | 52 +++++++++++++++++++++++++++++++++++++++++++--------- hw/pci-host/q35.c | 2 +- hw/ppc/e500.c | 16 +++++----------- hw/ppc/spapr.c | 16 ++++++---------- include/hw/boards.h | 10 +++++++++- include/sysemu/kvm.h | 2 +- kvm-all.c | 8 ++++---- target-arm/kvm.c | 2 +- target-i386/kvm.c | 5 ++--- target-mips/kvm.c | 2 +- target-ppc/kvm.c | 2 +- target-s390x/kvm.c | 2 +- 14 files changed, 80 insertions(+), 50 deletions(-)
Quoted 10 lines · Marcel Apfelbaum
Quoted 8 lines · Marcel Apfelbaum
Commit e79d5a6 ("machine: remove
qemu_machine_optsglobal list") removed option descriptions from the -machine QemuOptsList to avoid repeating MachineState's QOM properties.This results in a Qemu crash if a non string option is queried using qemu opts. Fix this by querying machine properties through designated wrappers.
I hope I didn't miss anything.
Comments are appreciated as always.Ping
Ping ping :)
Thanks,
Marcel
Quoted 38 lines
Hi, who can take this series?
It solves some command line bugs and people were pretty annoyed about it.Thanks,
MarcelQuoted 30 lines
Thanks,
MarcelMarcel Apfelbaum (8):
machine: query iommu machine property rather than qemu opts hw/machine: kernel-irqchip property support for allowed/required
machine: query kernel-irqchip machine property rather than qemu opts
kvm: add machine state tokvm_arch_init
machine: query kvm-shadow-mem machine property rather than qemu opts
machine: query phandle-start machine property rather than qemu opts
machine: query dump-guest-core machine property rather than qemu opts
machine: query mem-merge machine property rather than qemu optsdevice_tree.c | 5 ++--- exec.c | 6 +++--- hw/core/machine.c | 52 +++++++++++++++++++++++++++++++++++++++++++--------- hw/pci-host/q35.c | 2 +- hw/ppc/e500.c | 16 +++++----------- hw/ppc/spapr.c | 16 ++++++---------- include/hw/boards.h | 10 +++++++++- include/sysemu/kvm.h | 2 +- kvm-all.c | 8 ++++---- target-arm/kvm.c | 2 +- target-i386/kvm.c | 5 ++--- target-mips/kvm.c | 2 +- target-ppc/kvm.c | 2 +- target-s390x/kvm.c | 2 +- 14 files changed, 80 insertions(+), 50 deletions(-)