Re: [Qemu-devel] virtio-serial-pci very expensive during live migration
Copying Amit. Chris Friesen chris.frie...@windriver.com writes: Hi, I recently made the unfortunate discovery that virtio-serial-pci is quite expensive to stop/start during live migration. By default we support 32 ports, each of which uses 2 queues. In my case it takes 2-3ms per queue to disconnect on the source host, and another 2-3ms per queue to connect on the target host, for a total cost of 300ms. In our case this roughly tripled the outage times of a libvirt-based live migration, from 150ms to almost 500ms. It seems like the main problem is that we loop over all the queues, calling virtio_pci_set_host_notifier_internal() on each of them. That in turn calls memory_region_add_eventfd(), which calls memory_region_transaction_commit(), which scans over all the address spaces, which seems to take the vast majority of the time. Yes, setting the max_ports value to something smaller does help, but each port still adds 10-12ms to the overall live migration time, which is crazy. Is there anything that could be done to make this code more efficient? Could we tweak the API so that we add all the eventfds and then do a single commit at the end? Do we really need to scan the entire address space? I don't know the code well enough to answer that sort of question, but I'm hoping that one of you does. Thanks, Chris
Re: [Qemu-devel] [PATCH v1 13/22] target-arm: Register EL2 versions of ELR and SPSR
On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- target-arm/helper.c | 17 + 1 file changed, 17 insertions(+) diff --git a/target-arm/helper.c b/target-arm/helper.c index ba1830d..8efc340 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2078,6 +2078,19 @@ static const ARMCPRegInfo v8_cp_reginfo[] = { REGINFO_SENTINEL }; +static const ARMCPRegInfo v8_el2_cp_reginfo[] = { +{ .name = ELR_EL2, .state = ARM_CP_STATE_AA64, + .type = ARM_CP_NO_MIGRATE, + .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1, + .access = PL2_RW, + .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(2)]) }, +{ .name = SPSR_EL2, .state = ARM_CP_STATE_AA64, + .type = ARM_CP_NO_MIGRATE, + .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0, + .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) }, +REGINFO_SENTINEL +}; + static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { @@ -2321,6 +2334,10 @@ void register_cp_regs_for_features(ARMCPU *cpu) define_arm_cp_regs(cpu, v8_idregs); define_arm_cp_regs(cpu, v8_cp_reginfo); define_aarch64_debug_regs(cpu); + +if (arm_feature(env, ARM_FEATURE_EL2)) { +define_arm_cp_regs(cpu, v8_el2_cp_reginfo); +} I think this should be outside the if ARM_FEATURE_V8 for consistency. None of the other per-feature CP register defs are nested within the ifferry for their ARM version. Detecting the invalid combination of ARM_FEATURE_EL2 and pre V8 should probably be an assertion done in arm_cpu_realizefn(). Otherwise: Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com } if (arm_feature(env, ARM_FEATURE_MPU)) { /* These are the MPU registers prior to PMSAv6. Any new -- 1.8.3.2
[Qemu-devel] [PATCH 3/6] xics: disable flags reset on xics reset
Since islsi[] array has been merged into the ICSState struct, we must not reset flags as they tell if the interrupt is in use. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- hw/intc/xics.c | 4 +++- hw/intc/xics_kvm.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 9314654..7a64b2e 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -522,10 +522,12 @@ static void ics_reset(DeviceState *dev) ICSState *ics = ICS(dev); int i; -memset(ics-irqs, 0, sizeof(ICSIRQState) * ics-nr_irqs); for (i = 0; i ics-nr_irqs; i++) { +ics-irqs[i].server = 0; ics-irqs[i].priority = 0xff; ics-irqs[i].saved_priority = 0xff; +ics-irqs[i].status = 0; +/* Do not reset @flags as IRQ might be allocated */ } } diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c index 456fc2c..a322593 100644 --- a/hw/intc/xics_kvm.c +++ b/hw/intc/xics_kvm.c @@ -272,10 +272,12 @@ static void ics_kvm_reset(DeviceState *dev) ICSState *ics = ICS(dev); int i; -memset(ics-irqs, 0, sizeof(ICSIRQState) * ics-nr_irqs); for (i = 0; i ics-nr_irqs; i++) { +ics-irqs[i].server = 0; ics-irqs[i].priority = 0xff; ics-irqs[i].saved_priority = 0xff; +ics-irqs[i].status = 0; +/* Do not reset @flags as IRQ might be allocated */ } ics_set_kvm_state(ics, 1); -- 1.8.4.rc4
[Qemu-devel] [PATCH 5/6] spapr: remove @next_irq
This removes @next_irq from sPAPREnvironment which was used in old IRQ allocator as XICS is now responsible for IRQs and keeps track of allocated IRQs. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- hw/ppc/spapr.c | 3 +-- include/hw/ppc/spapr.h | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index db21515..a680e90 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -754,7 +754,7 @@ static const VMStateDescription vmstate_spapr = { .minimum_version_id = 1, .minimum_version_id_old = 1, .fields = (VMStateField []) { -VMSTATE_UINT32(next_irq, sPAPREnvironment), +VMSTATE_UNUSED(4), /* used to be @next_irq */ /* RTC offset */ VMSTATE_UINT64(rtc_offset, sPAPREnvironment), @@ -1158,7 +1158,6 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) /* Set up Interrupt Controller before we create the VCPUs */ spapr-icp = xics_system_init(smp_cpus * kvmppc_smt_threads() / smp_threads, XICS_IRQS); -spapr-next_irq = XICS_IRQ_BASE; /* init CPUs */ if (cpu_model == NULL) { diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index feb241a..f8d7326 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -27,7 +27,6 @@ typedef struct sPAPREnvironment { long rtas_size; void *fdt_skel; target_ulong entry_point; -uint32_t next_irq; uint64_t rtc_offset; bool has_graphics; -- 1.8.4.rc4
[Qemu-devel] [PATCH 6/6] xics: implement xics_ics_free()
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- hw/intc/xics.c| 24 include/hw/ppc/xics.h | 1 + trace-events | 2 ++ 3 files changed, 27 insertions(+) diff --git a/hw/intc/xics.c b/hw/intc/xics.c index faf304c..2316519 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -755,6 +755,30 @@ int xics_alloc_block(XICSState *icp, int server, int num, bool lsi, bool align) return first + ics-offset; } +static void ics_free(ICSState *ics, int srcno, int num) +{ +int i; + +for (i = srcno; i srcno + num; ++i) { +if (ICS_IRQ_FREE(ics, i)) { +trace_xics_ics_free_warn(ics - ics-icp-ics, i + ics-offset); +} +memset(ics-irqs[i], 0, sizeof(ICSIRQState)); +} +} + +void xics_free(XICSState *icp, int server, int irq, int num) +{ +ICSState *ics = icp-ics[server]; + +assert(server == 0); + +trace_xics_ics_free(ics - icp-ics, irq, num); +if (server = 0) { +ics_free(ics, irq - ics-offset, num); +} +} + /* * Guest interfaces */ diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index 0f01a21..e5d8f47 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -158,6 +158,7 @@ struct ICSIRQState { qemu_irq xics_get_qirq(XICSState *icp, int irq); int xics_alloc(XICSState *icp, int server, int irq_hint, bool lsi); int xics_alloc_block(XICSState *icp, int server, int num, bool lsi, bool align); +void xics_free(XICSState *icp, int server, int irq, int num); void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu); diff --git a/trace-events b/trace-events index 8cf8fb2..5ca126c 100644 --- a/trace-events +++ b/trace-events @@ -1179,6 +1179,8 @@ xics_alloc(int server, int irq) server#%d, irq %d xics_alloc_failed_hint(int server, int irq) server#%d, irq %d is already in use xics_alloc_failed_no_left(int server) server#%d, no irq left xics_alloc_block(int server, int first, int num, bool lsi, int align) server#%d, first irq %d, %d irqs, lsi=%d, alignnum %d +xics_ics_free(int server, int irq, int num) server#%d, first irq %d, %d irqs +xics_ics_free_warn(int server, int irq) server#%d, irq %d is already free # hw/ppc/spapr_iommu.c spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) liobn=%PRIx64 ioba=0x%PRIx64 tce=0x%PRIx64 ret=%PRId64 -- 1.8.4.rc4
[Qemu-devel] [PATCH 1/6] xics: add flags for interrupts
The existing interrupt allocation scheme in SPAPR assumes that interrupts are allocated at the start time, continously and the config will not change. However, there are cases when this is not going to work such as: 1. migration - we will have to have an ability to choose interrupt numbers for devices in the command line and this will create gaps in interrupt space. 2. PCI hotplug - interrupts from unplugged device need to be returned back to interrupt pool, otherwise we will quickly run out of interrupts. This replaces a separate lslsi[] array with a byte in the ICSIRQState struct and defines LSI and MSI flags. Neither of these flags set signals that the descriptor is not allocated and not in use. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- hw/intc/xics.c| 21 ++--- hw/intc/xics_kvm.c| 5 ++--- include/hw/ppc/xics.h | 5 - 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 64aabe7..1f89a00 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -438,7 +438,7 @@ static void ics_set_irq(void *opaque, int srcno, int val) { ICSState *ics = (ICSState *)opaque; -if (ics-islsi[srcno]) { +if (ics-irqs[srcno].flags XICS_FLAGS_LSI) { set_irq_lsi(ics, srcno, val); } else { set_irq_msi(ics, srcno, val); @@ -475,7 +475,7 @@ static void ics_write_xive(ICSState *ics, int nr, int server, trace_xics_ics_write_xive(nr, srcno, server, priority); -if (ics-islsi[srcno]) { +if (ics-irqs[srcno].flags XICS_FLAGS_LSI) { write_xive_lsi(ics, srcno); } else { write_xive_msi(ics, srcno); @@ -497,7 +497,7 @@ static void ics_resend(ICSState *ics) for (i = 0; i ics-nr_irqs; i++) { /* FIXME: filter by server#? */ -if (ics-islsi[i]) { +if (ics-irqs[i].flags XICS_FLAGS_LSI) { resend_lsi(ics, i); } else { resend_msi(ics, i); @@ -512,7 +512,7 @@ static void ics_eoi(ICSState *ics, int nr) trace_xics_ics_eoi(nr); -if (ics-islsi[srcno]) { +if (ics-irqs[srcno].flags XICS_FLAGS_LSI) { irq-status = ~XICS_STATUS_SENT; } } @@ -609,7 +609,6 @@ static void ics_realize(DeviceState *dev, Error **errp) return; } ics-irqs = g_malloc0(ics-nr_irqs * sizeof(ICSIRQState)); -ics-islsi = g_malloc0(ics-nr_irqs * sizeof(bool)); ics-qirqs = qemu_allocate_irqs(ics_set_irq, ics, ics-nr_irqs); } @@ -646,11 +645,19 @@ qemu_irq xics_get_qirq(XICSState *icp, int irq) return icp-ics-qirqs[irq - icp-ics-offset]; } +static void ics_set_irq_type(ICSState *ics, int srcno, bool lsi) +{ +ics-irqs[srcno].flags |= +lsi ? XICS_FLAGS_LSI : XICS_FLAGS_MSI; +} + void xics_set_irq_type(XICSState *icp, int irq, bool lsi) { -assert(ics_valid_irq(icp-ics, irq)); +ICSState *ics = icp-ics; -icp-ics-islsi[irq - icp-ics-offset] = lsi; +assert(ics_valid_irq(ics, irq)); + +ics_set_irq_type(ics, irq - ics-offset, lsi); } /* diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c index 09476ae..456fc2c 100644 --- a/hw/intc/xics_kvm.c +++ b/hw/intc/xics_kvm.c @@ -224,7 +224,7 @@ static int ics_set_kvm_state(ICSState *ics, int version_id) state |= KVM_XICS_MASKED; } -if (ics-islsi[i]) { +if (ics-irqs[i].flags XICS_FLAGS_LSI) { state |= KVM_XICS_LEVEL_SENSITIVE; if (irq-status XICS_STATUS_ASSERTED) { state |= KVM_XICS_PENDING; @@ -253,7 +253,7 @@ static void ics_kvm_set_irq(void *opaque, int srcno, int val) int rc; args.irq = srcno + ics-offset; -if (!ics-islsi[srcno]) { +if (ics-irqs[srcno].flags XICS_FLAGS_MSI) { if (!val) { return; } @@ -290,7 +290,6 @@ static void ics_kvm_realize(DeviceState *dev, Error **errp) return; } ics-irqs = g_malloc0(ics-nr_irqs * sizeof(ICSIRQState)); -ics-islsi = g_malloc0(ics-nr_irqs * sizeof(bool)); ics-qirqs = qemu_allocate_irqs(ics_kvm_set_irq, ics, ics-nr_irqs); } diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index 0d7673d..aad48cf 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -136,7 +136,6 @@ struct ICSState { uint32_t nr_irqs; uint32_t offset; qemu_irq *qirqs; -bool *islsi; ICSIRQState *irqs; XICSState *icp; }; @@ -150,6 +149,10 @@ struct ICSIRQState { #define XICS_STATUS_REJECTED 0x4 #define XICS_STATUS_MASKED_PENDING 0x8 uint8_t status; +/* @flags == 0 measn the interrupt is not allocated */ +#define XICS_FLAGS_LSI 0x1 +#define XICS_FLAGS_MSI 0x2 +uint8_t flags; }; qemu_irq xics_get_qirq(XICSState *icp, int irq); -- 1.8.4.rc4
[Qemu-devel] [PATCH 0/6] move interrupts from spapr to xics
The existing interrupt allocation scheme in SPAPR assumes that interrupts are allocated at the start time, continously and the config will not change. However, there are cases when this is not going to work such as: 1. migration - we will have to have an ability to choose interrupt numbers for devices in the command line and this will create gaps in interrupt space. 2. PCI hotplug - interrupts from unplugged device need to be returned back to interrupt pool, otherwise we will quickly run out of interrupts if we do PCI hotplug often. First this was posted as [PATCH 0/8] spapr: fix IOMMU and XICS/IRQs migration but since then we decided to migrate IOMMU in a different way and now it just about interrupts. This is also going to be used in spapr_pci's ibm,change-msi callback to release interrupts and reallocate them again. At the moment spapr_pci keeps a map of what it allocated for what device and it would be nice to get rid of that mapping too and use this patchset instead. Please comment. Thanks! Alexey Kardashevskiy (6): xics: add flags for interrupts xics: add find_server xics: disable flags reset on xics reset spapr: move interrupt allocator to xics spapr: remove @next_irq xics: implement xics_ics_free() hw/intc/xics.c | 150 + hw/intc/xics_kvm.c | 9 +-- hw/ppc/spapr.c | 70 +-- hw/ppc/spapr_events.c | 2 +- hw/ppc/spapr_pci.c | 6 +- hw/ppc/spapr_vio.c | 2 +- include/hw/ppc/spapr.h | 11 include/hw/ppc/xics.h | 9 ++- trace-events | 6 ++ 9 files changed, 162 insertions(+), 103 deletions(-) -- 1.8.4.rc4
Re: [Qemu-devel] [PATCH v1 16/22] target-arm: A64: Forbid ERET to unimplemented ELs
On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Check for EL2 support before returning to it. Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- target-arm/op_helper.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index 770c776..f1ae05e 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -411,12 +411,10 @@ void HELPER(exception_return)(CPUARMState *env) env-regs[15] = env-elr_el[ELR_EL_IDX(1)] ~0x1; } else { new_el = extract32(spsr, 2, 2); -if (new_el cur_el) { +if (new_el cur_el +|| (new_el == 2 !arm_feature(env, ARM_FEATURE_EL2))) { /* Disallow returns to higher ELs than the current one. */ -goto illegal_return; -} -if (new_el 1) { -/* Return to unimplemented EL */ +/* Disallow returns to unimplemented ELs. */ goto illegal_return; } if (extract32(spsr, 1, 1)) { -- 1.8.3.2
Re: [Qemu-devel] [PATCH v1 14/22] target-arm: Register EL3 versions of ELR and SPSR
On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com Same as last patch, Otherwise: Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- target-arm/helper.c | 16 1 file changed, 16 insertions(+) diff --git a/target-arm/helper.c b/target-arm/helper.c index 8efc340..65daeaf 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2091,6 +2091,19 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = { REGINFO_SENTINEL }; +static const ARMCPRegInfo v8_el3_cp_reginfo[] = { +{ .name = ELR_EL3, .state = ARM_CP_STATE_AA64, + .type = ARM_CP_NO_MIGRATE, + .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 1, + .access = PL3_RW, + .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(3)]) }, +{ .name = SPSR_EL3, .state = ARM_CP_STATE_AA64, + .type = ARM_CP_NO_MIGRATE, + .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 0, + .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[7]) }, +REGINFO_SENTINEL +}; + static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { @@ -2338,6 +2351,9 @@ void register_cp_regs_for_features(ARMCPU *cpu) if (arm_feature(env, ARM_FEATURE_EL2)) { define_arm_cp_regs(cpu, v8_el2_cp_reginfo); } +if (arm_feature(env, ARM_FEATURE_EL3)) { +define_arm_cp_regs(cpu, v8_el3_cp_reginfo); +} } if (arm_feature(env, ARM_FEATURE_MPU)) { /* These are the MPU registers prior to PMSAv6. Any new -- 1.8.3.2
Re: [Qemu-devel] [PATCH v1 15/22] target-arm: A64: Forbid ERET to increase the EL
On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- target-arm/op_helper.c | 5 + 1 file changed, 5 insertions(+) diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index dd9e4fc..770c776 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -389,6 +389,7 @@ void HELPER(exception_return)(CPUARMState *env) unsigned int spsr_idx = arm64_banked_spsr_index(1); uint32_t spsr = env-banked_spsr[spsr_idx]; int new_el, i; +int cur_el = arm_current_pl(env); if (env-pstate PSTATE_SP) { env-sp_el[1] = env-xregs[31]; @@ -410,6 +411,10 @@ void HELPER(exception_return)(CPUARMState *env) env-regs[15] = env-elr_el[ELR_EL_IDX(1)] ~0x1; } else { new_el = extract32(spsr, 2, 2); +if (new_el cur_el) { +/* Disallow returns to higher ELs than the current one. */ +goto illegal_return; +} if (new_el 1) { /* Return to unimplemented EL */ goto illegal_return; -- 1.8.3.2
Re: [Qemu-devel] [PATCH v3 14/14] qmp: Don't use error_is_set() to suppress additional errors
Eric Blake ebl...@redhat.com writes: On 05/02/2014 05:26 AM, Markus Armbruster wrote: Using error_is_set(errp) that way can sweep programming errors under the carpet when we get called incorrectly with an error set. encrypted_bdrv_it() does it, because there's no way to make bdrv_iterate() break its loop. Actually safe, because qmp_cont() clears the error before the loop. Clean it up anyway: replace bdrv_iterate() by bdrv_next(), break the loop on error. Replace both occurences, for consistency. s/occurences/occurrences/ Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com Fixing up the commit message typo is trivial and doesn't alter the positive review. Luiz, could you fix it up on commit, when you add Eric's R-by?
Re: [Qemu-devel] [PATCH v1 17/22] target-arm: A64: Generalize ERET to various ELs
On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Adds support for ERET to Aarch64 EL2 and 3. Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- target-arm/op_helper.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index f1ae05e..8494f7f 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -386,13 +386,14 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm) void HELPER(exception_return)(CPUARMState *env) { -unsigned int spsr_idx = arm64_banked_spsr_index(1); -uint32_t spsr = env-banked_spsr[spsr_idx]; -int new_el, i; int cur_el = arm_current_pl(env); +unsigned int spsr_idx = arm64_banked_spsr_index(cur_el); +uint32_t spsr; +int new_el, i; +spsr = env-banked_spsr[spsr_idx]; Why change to split declaration and assignment (amongst the other all-in-one's above)? Otherwise: Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com if (env-pstate PSTATE_SP) { -env-sp_el[1] = env-xregs[31]; +env-sp_el[cur_el] = env-xregs[31]; } else { env-sp_el[0] = env-xregs[31]; } @@ -428,7 +429,7 @@ void HELPER(exception_return)(CPUARMState *env) env-aarch64 = 1; pstate_write(env, spsr); env-xregs[31] = env-sp_el[new_el]; -env-pc = env-elr_el[ELR_EL_IDX(1)]; +env-pc = env-elr_el[ELR_EL_IDX(cur_el)]; } return; @@ -442,7 +443,7 @@ illegal_return: * no change to exception level, execution state or stack pointer */ env-pstate |= PSTATE_IL; -env-pc = env-elr_el[ELR_EL_IDX(1)]; +env-pc = env-elr_el[ELR_EL_IDX(cur_el)]; spsr = PSTATE_NZCV | PSTATE_DAIF; spsr |= pstate_read(env) ~(PSTATE_NZCV | PSTATE_DAIF); pstate_write(env, spsr); -- 1.8.3.2
Re: [Qemu-devel] [PATCH v1 18/22] target-arm: A64: Generalize update_spsel for the various ELs
On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- target-arm/internals.h | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/target-arm/internals.h b/target-arm/internals.h index 7c39946..5d802db 100644 --- a/target-arm/internals.h +++ b/target-arm/internals.h @@ -107,6 +107,7 @@ int arm_rmode_to_sf(int rmode); static inline void update_spsel(CPUARMState *env, uint32_t imm) { +unsigned int cur_el = arm_current_pl(env); /* Update PSTATE SPSel bit; this requires us to update the * working stack pointer in xregs[31]. */ @@ -115,17 +116,15 @@ static inline void update_spsel(CPUARMState *env, uint32_t imm) } env-pstate = deposit32(env-pstate, 0, 1, imm); -/* EL0 has no access rights to update SPSel, and this code - * assumes we are updating SP for EL1 while running as EL1. - */ -assert(arm_current_pl(env) == 1); +/* EL0 has no access rights to update SPSel. */ +assert(cur_el = 1 cur_el = 3); if (env-pstate PSTATE_SP) { /* Switch from using SP_EL0 to using SP_ELx */ env-sp_el[0] = env-xregs[31]; -env-xregs[31] = env-sp_el[1]; +env-xregs[31] = env-sp_el[cur_el]; } else { /* Switch from SP_EL0 to SP_ELx */ -env-sp_el[1] = env-xregs[31]; +env-sp_el[cur_el] = env-xregs[31]; env-xregs[31] = env-sp_el[0]; } } -- 1.8.3.2
Re: [Qemu-devel] [PATCH v1 20/22] target-arm: Make vbar_write writeback to any CPREG
On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- target-arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index 65daeaf..2406058 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -657,7 +657,7 @@ static void vbar_write(CPUARMState *env, const ARMCPRegInfo *ri, * contexts. (ARMv8 would permit us to do no masking at all, but ARMv7 * requires the bottom five bits to be RAZ/WI because they're UNK/SBZP.) */ -env-cp15.vbar_el[VBAR_EL_IDX(1)] = value ~0x1FULL; +CPREG_FIELD64(env, ri) = value ~0x1FULL; Use raw_write() to implement CP register writing (check vmsa_ttbr_write for example). Regards, Peter } static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri) -- 1.8.3.2
Re: [Qemu-devel] [PATCH v1 21/22] target-arm: A64: Register VBAR_EL2
On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- target-arm/helper.c | 5 + 1 file changed, 5 insertions(+) diff --git a/target-arm/helper.c b/target-arm/helper.c index 2406058..6e3f5fa 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2088,6 +2088,11 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = { .type = ARM_CP_NO_MIGRATE, .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0, .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[6]) }, +{ .name = VBAR_EL2, .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0, + .access = PL2_RW, .writefn = vbar_write, + .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[1]), This [1] is smoewhat misleading, and should either use the VBAR_EL_IDX macro or if changing over to always-four array, just [2]. Regards, Peter + .resetvalue = 0 }, REGINFO_SENTINEL }; -- 1.8.3.2
Re: [Qemu-devel] [PATCH v1 22/22] target-arm: A64: Register VBAR_EL3
On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- target-arm/helper.c | 5 + 1 file changed, 5 insertions(+) diff --git a/target-arm/helper.c b/target-arm/helper.c index 6e3f5fa..b6dac25 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2106,6 +2106,11 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = { .type = ARM_CP_NO_MIGRATE, .opc0 = 3, .opc1 = 6, .crn = 4, .crm = 0, .opc2 = 0, .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[7]) }, +{ .name = VBAR_EL3, .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 6, .crn = 12, .crm = 0, .opc2 = 0, + .access = PL3_RW, .writefn = vbar_write, + .fieldoffset = offsetof(CPUARMState, cp15.vbar_el[2]), Same comment as P21. Regards, Peter + .resetvalue = 0 }, REGINFO_SENTINEL }; -- 1.8.3.2
Re: [Qemu-devel] [PATCH] target-i386: fix handling of ZF in btx instructions
Il 06/05/2014 15:33, Dmitry Poletaev ha scritto: Eflags for bt/bts/btr/btc instructions compute as for shift(SAR) instructions. According to Intel A2 manual, for btx instructions zf is unaffected under any condition, but for SAR group, when evaluate eflags, we compute zf. This patch makes zf=0, it doesn't leave it unaffected. Paolo So I suggest add another group, specifically for computing eflags after btx instructions. Signed-off-by: Dmitry Poletaev poletaev-q...@yandex.ru diff --git a/target-i386/cc_helper.c b/target-i386/cc_helper.c index 05dd12b..272e2f1 100644 --- a/target-i386/cc_helper.c +++ b/target-i386/cc_helper.c @@ -168,6 +168,12 @@ target_ulong helper_cc_compute_all(target_ulong dst, target_ulong src1, case CC_OP_SHLL: return compute_all_shll(dst, src1); +case CC_OP_BTXB: +return compute_all_btxb(dst, src1); +case CC_OP_BTXW: +return compute_all_btxw(dst, src1); +case CC_OP_BTXL: +return compute_all_btxl(dst, src1); case CC_OP_SARB: return compute_all_sarb(dst, src1); case CC_OP_SARW: @@ -208,6 +214,8 @@ target_ulong helper_cc_compute_all(target_ulong dst, target_ulong src1, return compute_all_decq(dst, src1); case CC_OP_SHLQ: return compute_all_shlq(dst, src1); +case CC_OP_BTXQ: +return compute_all_btxq(dst, src1); case CC_OP_SARQ: return compute_all_sarq(dst, src1); case CC_OP_BMILGQ: @@ -234,6 +242,10 @@ target_ulong helper_cc_compute_c(target_ulong dst, target_ulong src1, return 0; case CC_OP_EFLAGS: +case CC_OP_BTXB: +case CC_OP_BTXW: +case CC_OP_BTXL: +case CC_OP_BTXQ: case CC_OP_SARB: case CC_OP_SARW: case CC_OP_SARL: diff --git a/target-i386/cc_helper_template.h b/target-i386/cc_helper_template.h index 607311f..04375f1 100644 --- a/target-i386/cc_helper_template.h +++ b/target-i386/cc_helper_template.h @@ -187,6 +187,19 @@ static int glue(compute_c_shl, SUFFIX)(DATA_TYPE dst, DATA_TYPE src1) return (src1 (DATA_BITS - 1)) CC_C; } +static int glue(compute_all_btx, SUFFIX)(DATA_TYPE dst, DATA_TYPE src1) +{ +int cf, pf, af, sf, of; + +cf = src1 CC_C; +pf = 0; /* undefined */ +af = 0; /* undefined */ +/* zf unaffected */ +sf = 0; /* undefined */ +of = 0; /* undefined */ +return cf | pf | af | sf | of; +} + static int glue(compute_all_sar, SUFFIX)(DATA_TYPE dst, DATA_TYPE src1) { int cf, pf, af, zf, sf, of; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 2a22a7d..506037d 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -660,6 +660,10 @@ typedef enum { CC_OP_SHLL, CC_OP_SHLQ, +CC_OP_BTXB, /* modify only C, CC_SRC.msb = C */ +CC_OP_BTXW, +CC_OP_BTXL, +CC_OP_BTXQ, CC_OP_SARB, /* modify all flags, CC_DST = res, CC_SRC.lsb = C */ CC_OP_SARW, CC_OP_SARL, diff --git a/target-i386/translate.c b/target-i386/translate.c index 02625e3..e77ba0b 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -200,6 +200,7 @@ static const uint8_t cc_op_live[CC_OP_NB] = { [CC_OP_INCB ... CC_OP_INCQ] = USES_CC_DST | USES_CC_SRC, [CC_OP_DECB ... CC_OP_DECQ] = USES_CC_DST | USES_CC_SRC, [CC_OP_SHLB ... CC_OP_SHLQ] = USES_CC_DST | USES_CC_SRC, +[CC_OP_BTXB ... CC_OP_BTXQ] = USES_CC_DST | USES_CC_SRC, [CC_OP_SARB ... CC_OP_SARQ] = USES_CC_DST | USES_CC_SRC, [CC_OP_BMILGB ... CC_OP_BMILGQ] = USES_CC_DST | USES_CC_SRC, [CC_OP_ADCX] = USES_CC_DST | USES_CC_SRC, @@ -6734,7 +6735,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, tcg_gen_xor_tl(cpu_T[0], cpu_T[0], cpu_tmp0); break; } -set_cc_op(s, CC_OP_SARB + ot); +set_cc_op(s, CC_OP_BTXB + ot); if (op != 0) { if (mod != 3) { gen_op_st_v(s, ot, cpu_T[0], cpu_A0);
[Qemu-devel] [PATCH 4/6] spapr: move interrupt allocator to xics
The current allocator returns IRQ numbers from a pool and does not support IRQs reuse in any form as it did not keep track of what it previously returned, it only keeps the last returned IRQ. Some use cases such as PCI hot(un)plug may require IRQ release and reallocation. This moves an allocator from SPAPR to XICS. This switches IRQ users to use new API. This uses LSI/MSI flags to know if interrupt is allocated. This removes xics_set_irq_type() as it is not used anymore. The interrupt release function will be posted as a separate patch. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- hw/intc/xics.c | 85 +++--- hw/ppc/spapr.c | 67 --- hw/ppc/spapr_events.c | 2 +- hw/ppc/spapr_pci.c | 6 ++-- hw/ppc/spapr_vio.c | 2 +- include/hw/ppc/spapr.h | 10 -- include/hw/ppc/xics.h | 3 +- trace-events | 4 +++ 8 files changed, 91 insertions(+), 88 deletions(-) diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 7a64b2e..faf304c 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -669,15 +669,90 @@ static void ics_set_irq_type(ICSState *ics, int srcno, bool lsi) lsi ? XICS_FLAGS_LSI : XICS_FLAGS_MSI; } -void xics_set_irq_type(XICSState *icp, int irq, bool lsi) +#define ICS_IRQ_FREE(ics, srcno) \ +(!((ics)-irqs[(srcno)].flags (XICS_FLAGS_LSI | XICS_FLAGS_MSI))) + +static int ics_find_free_block(ICSState *ics, int num, int alignnum) +{ +int first, i; + +for (first = 0; first ics-nr_irqs; first += alignnum) { +if (num (ics-nr_irqs - first)) { +return -1; +} +for (i = first; i first + num; ++i) { +if (!ICS_IRQ_FREE(ics, i)) { +break; +} +} +if (i == (first + num)) { +return first; +} +} + +return -1; +} + +int xics_alloc(XICSState *icp, int server, int irq_hint, bool lsi) { -int server = xics_find_server(icp, irq); -ICSState *ics; +ICSState *ics = icp-ics[server]; +int irq; -assert(server = 0); +if (irq_hint) { +assert(server == xics_find_server(icp, irq_hint)); +if (!ICS_IRQ_FREE(ics, irq_hint - ics-offset)) { +trace_xics_alloc_failed_hint(server, irq_hint); +return -1; +} +irq = irq_hint; +} else { +irq = ics_find_free_block(ics, 1, 1); +if (irq 0) { +trace_xics_alloc_failed_no_left(server); +return -1; +} +irq += ics-offset; +} -ics = icp-ics[server]; ics_set_irq_type(ics, irq - ics-offset, lsi); +trace_xics_alloc(server, irq); + +return irq; +} + +/* + * Allocate block of consequtive IRQs, returns a number of the first. + * If align==true, aligns the first IRQ number to num. + */ +int xics_alloc_block(XICSState *icp, int server, int num, bool lsi, bool align) +{ +int i, first = -1; +ICSState *ics = icp-ics[server]; + +assert(server == 0); +/* + * MSIMesage::data is used for storing VIRQ so + * it has to be aligned to num to support multiple + * MSI vectors. MSI-X is not affected by this. + * The hint is used for the first IRQ, the rest should + * be allocated continuously. + */ +if (align) { +assert((num == 1) || (num == 2) || (num == 4) || + (num == 8) || (num == 16) || (num == 32)); +first = ics_find_free_block(ics, num, num); +} else { +first = ics_find_free_block(ics, num, 1); +} + +if (first 0) { +for (i = first; i first + num; ++i) { +ics_set_irq_type(ics, i, lsi); +} +} +trace_xics_alloc_block(server, first, num, lsi, align); + +return first + ics-offset; } /* diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index a11e121..db21515 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -88,73 +88,6 @@ sPAPREnvironment *spapr; -int spapr_allocate_irq(int hint, bool lsi) -{ -int irq; - -if (hint) { -irq = hint; -if (hint = spapr-next_irq) { -spapr-next_irq = hint + 1; -} -/* FIXME: we should probably check for collisions somehow */ -} else { -irq = spapr-next_irq++; -} - -/* Configure irq type */ -if (!xics_get_qirq(spapr-icp, irq)) { -return 0; -} - -xics_set_irq_type(spapr-icp, irq, lsi); - -return irq; -} - -/* - * Allocate block of consequtive IRQs, returns a number of the first. - * If msi==true, aligns the first IRQ number to num. - */ -int spapr_allocate_irq_block(int num, bool lsi, bool msi) -{ -int first = -1; -int i, hint = 0; - -/* - * MSIMesage::data is used for storing VIRQ so - * it has to be aligned to num to support multiple - * MSI vectors. MSI-X is not affected by this. - * The hint is used for the first IRQ, the rest should - * be allocated continuously. - */ -if (msi) { -
Re: [Qemu-devel] [PATCH v3] block/raw-posix: Try both FIEMAP and SEEK_HOLE
Il 07/05/2014 00:18, Max Reitz ha scritto: The current version of raw-posix always uses ioctl(FS_IOC_FIEMAP) if FIEMAP is available; lseek with SEEK_HOLE/SEEK_DATA are not even compiled in in this case. However, there may be implementations which support the latter but not the former (e.g., NFSv4.2) as well as vice versa. To cover both cases, always try SEEK_HOLE/SEEK_DATA (as this will probably be covered by POSIX soon) and if that does not work, fall back to FIEMAP; and if that does not work either, treat everything as allocated. Signed-off-by: Max Reitz mre...@redhat.com --- v3: - use a tristate for keeping the information whether or not to use FIEMAP and/or SEEK_HOLE/SEEK_DATA [Eric] - fall through to another implementation (i.e. FIEMAP) if the first (i.e. SEEK_HOLE/SEEK_DATA) reports everything as allocated; this will not result in that implementation being skipped the next time, however [Eric] You need this if you use SEEK_HOLE/SEEK_DATA first, because it has a default implementation that returns a single non-hole for the entire file. But if you start with FIEMAP, you don't because it will return ENOTSUP instead of a single allocated extent. Paolo --- block/raw-posix.c | 182 +++--- 1 file changed, 131 insertions(+), 51 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 3ce026d..fd6bac6 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -123,6 +123,18 @@ #define MAX_BLOCKSIZE 4096 +/* In case there are multiple implementations for the same feature provided by + * the environment, this enumeration may be used to represent the status of + * these alternatives. */ +typedef enum ImplementationAlternativeStatus { +/* The status is (yet) unknown. */ +IMPLSTAT_UNKNOWN = 0, +/* This implementation is known to return correct results. */ +IMPLSTAT_WORKING, +/* This implementation is known not to return correct results. */ +IMPLSTAT_SKIP, +} ImplementationAlternativeStatus; + typedef struct BDRVRawState { int fd; int type; @@ -146,6 +158,12 @@ typedef struct BDRVRawState { bool has_discard:1; bool has_write_zeroes:1; bool discard_zeroes:1; +#if defined SEEK_HOLE defined SEEK_DATA +ImplementationAlternativeStatus seek_hole_status; +#endif +#ifdef CONFIG_FIEMAP +ImplementationAlternativeStatus fiemap_status; +#endif } BDRVRawState; typedef struct BDRVRawReopenState { @@ -1272,98 +1290,160 @@ static int raw_create(const char *filename, QEMUOptionParameter *options, return result; } -/* - * Returns true iff the specified sector is present in the disk image. Drivers - * not implementing the functionality are assumed to not support backing files, - * hence all their sectors are reported as allocated. - * - * If 'sector_num' is beyond the end of the disk image the return value is 0 - * and 'pnum' is set to 0. - * - * 'pnum' is set to the number of sectors (including and immediately following - * the specified sector) that are known to be in the same - * allocated/unallocated state. - * - * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes - * beyond the end of the disk image it will be clamped. - */ -static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs, -int64_t sector_num, -int nb_sectors, int *pnum) +static int64_t try_fiemap(BlockDriverState *bs, off_t start, off_t end, + off_t *data, off_t *hole, int nb_sectors, int *pnum) { -off_t start, data, hole; -int64_t ret; - -ret = fd_open(bs); -if (ret 0) { -return ret; -} - -start = sector_num * BDRV_SECTOR_SIZE; -ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start; - #ifdef CONFIG_FIEMAP - BDRVRawState *s = bs-opaque; +int64_t ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start; struct { struct fiemap fm; struct fiemap_extent fe; } f; +if (s-fiemap_status == IMPLSTAT_SKIP) { +return -ENOTSUP; +} + f.fm.fm_start = start; f.fm.fm_length = (int64_t)nb_sectors * BDRV_SECTOR_SIZE; f.fm.fm_flags = 0; f.fm.fm_extent_count = 1; f.fm.fm_reserved = 0; if (ioctl(s-fd, FS_IOC_FIEMAP, f) == -1) { -/* Assume everything is allocated. */ -*pnum = nb_sectors; -return ret; +s-fiemap_status = IMPLSTAT_SKIP; +return -errno; +} + +if (s-fiemap_status == IMPLSTAT_UNKNOWN) { +if (f.fm.fm_extent_count == 1 +f.fe.fe_logical == 0 f.fe.fe_length = end) +{ +/* FIEMAP returned a single extent spanning the entire file; maybe + * this was just a default response and therefore we cannot be sure + * whether it actually works; fall back to an alternative + * implementation. */ +return -ENOTSUP; +}
Re: [Qemu-devel] virtio-serial-pci very expensive during live migration
Il 06/05/2014 22:01, Chris Friesen ha scritto: It seems like the main problem is that we loop over all the queues, calling virtio_pci_set_host_notifier_internal() on each of them. That in turn calls memory_region_add_eventfd(), which calls memory_region_transaction_commit(), which scans over all the address spaces, which seems to take the vast majority of the time. Yes, you can wrap the entire loop with memory_region_transaction_begin and memory_region_transaction_commit. Can you try that? Paolo
[Qemu-devel] Configure virtio-scsi options via libvirt
Hi everyone, I would like be able to configure virtio-scsi options num_queues, max_sectors, and cmd_per_lun via libvirt. Are there any plans to have this support? -- Mike Perez
[Qemu-devel] [PATCH 2/6] xics: add find_server
PAPR allows having multiple interrupr servers. This adds a server lookup function and makes use of it. Since at the moment QEMU only supports a single server, no change in behaviour is expected. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- hw/intc/xics.c | 28 +++- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 1f89a00..9314654 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -635,14 +635,30 @@ static const TypeInfo ics_info = { /* * Exported functions */ +static int xics_find_server(XICSState *icp, int irq) +{ +int server; + +for (server = 0; server icp-nr_servers; ++server) { +ICSState *ics = icp-ics[server]; +if (ics_valid_irq(ics, irq)) { +return server; +} +} + +return -1; +} qemu_irq xics_get_qirq(XICSState *icp, int irq) { -if (!ics_valid_irq(icp-ics, irq)) { -return NULL; +int server = xics_find_server(icp, irq); + +if (server = 0) { +ICSState *ics = icp-ics[server]; +return ics-qirqs[irq - ics-offset]; } -return icp-ics-qirqs[irq - icp-ics-offset]; +return NULL; } static void ics_set_irq_type(ICSState *ics, int srcno, bool lsi) @@ -653,10 +669,12 @@ static void ics_set_irq_type(ICSState *ics, int srcno, bool lsi) void xics_set_irq_type(XICSState *icp, int irq, bool lsi) { -ICSState *ics = icp-ics; +int server = xics_find_server(icp, irq); +ICSState *ics; -assert(ics_valid_irq(ics, irq)); +assert(server = 0); +ics = icp-ics[server]; ics_set_irq_type(ics, irq - ics-offset, lsi); } -- 1.8.4.rc4
[Qemu-devel] [PATCH] spapr: pci: clean msi info when releasing it
In current code, we use phb-msi_table[ndev].nvec to indicate whether this msi entries are used by a device or not. So when unplug a pci device, we should reset nvec to zero. Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/ppc/spapr_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index cbef095..7b1dfe1 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -316,6 +316,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr, rtas_st(rets, 0, RTAS_OUT_HW_ERROR); return; } +phb-msi_table[ndev].nvec = 0; trace_spapr_pci_msi(Released MSIs, ndev, config_addr); rtas_st(rets, 0, RTAS_OUT_SUCCESS); rtas_st(rets, 1, 0); -- 1.8.1.4
Re: [Qemu-devel] [PATCH] pc: q35: disable CPU hotplug handler on machines less than 2.0
On Tue, May 06, 2014 at 03:09:04PM +0200, Igor Mammedov wrote: CPU hotplug handling for Q35 machine types was added only since 2.0 but commit wrongly c649983b58 added CPU hotplug handler to Q35 1.5 machine without adding ACPI support for it. As result user can create VCPU but guest couldn't use it since it hasn't any data on it (missing ACPI implementation). Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/pc_q35.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index c844dc2..687a397 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -308,7 +308,9 @@ static QEMUMachine pc_q35_machine_v2_0 = { .init = pc_q35_init, }; -#define PC_Q35_1_7_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS +#define PC_Q35_1_7_MACHINE_OPTIONS \ +PC_Q35_MACHINE_OPTIONS, \ +.hot_add_cpu = NULL static QEMUMachine pc_q35_machine_v1_7 = { PC_Q35_1_7_MACHINE_OPTIONS, @@ -342,9 +344,7 @@ static QEMUMachine pc_q35_machine_v1_5 = { }, }; -#define PC_Q35_1_4_MACHINE_OPTIONS \ -PC_Q35_1_6_MACHINE_OPTIONS, \ -.hot_add_cpu = NULL +#define PC_Q35_1_4_MACHINE_OPTIONS PC_Q35_1_6_MACHINE_OPTIONS If I'm looking at the correct code, pc-q35-1.6 still enables cpu hotplug: #define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS You can remove PC_Q35_1_6_MACHINE_OPTIONS and use PC_Q35_1_7_MACHINE_OPTIONS in all versions less then 1.7. static QEMUMachine pc_q35_machine_v1_4 = { PC_Q35_1_4_MACHINE_OPTIONS, -- 1.7.1
Re: [Qemu-devel] [PATCH] spapr: pci: clean msi info when releasing it
On 05/07/2014 04:51 PM, Liu Ping Fan wrote: In current code, we use phb-msi_table[ndev].nvec to indicate whether this msi entries are used by a device or not. So when unplug a pci device, we should reset nvec to zero. Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/ppc/spapr_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index cbef095..7b1dfe1 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -316,6 +316,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr, rtas_st(rets, 0, RTAS_OUT_HW_ERROR); return; } +phb-msi_table[ndev].nvec = 0; trace_spapr_pci_msi(Released MSIs, ndev, config_addr); rtas_st(rets, 0, RTAS_OUT_SUCCESS); rtas_st(rets, 1, 0); ibm,change-msi is called with 0 to disable MSIs. If later the guest decides to reenable MSI on the same device (rmmod + modprobe in the guest can do that I suppose), new block will be allocated because of this patch which is bad. And there is no PCI hotplug for SPAPR in upstream QEMU so this patch cannot possibly fix it :) -- Alexey
Re: [Qemu-devel] [PATCH v3 00/12] block/json: Add JSON protocol driver
On Tue, May 06, 2014 at 07:51:10PM +0200, Max Reitz wrote: On 06.05.2014 10:10, Stefan Hajnoczi wrote: On Mon, May 05, 2014 at 06:19:07PM +0200, Max Reitz wrote: On 10.04.2014 20:43, Max Reitz wrote: This series adds a passthrough JSON protocol block driver. Its filenames are JSON objects prefixed by json:. The objects are used as options for opening another block device which will be the child of the JSON device. Regarding this child device, the JSON driver behaves nearly the same as raw_bsd in that it is just a passthrough driver. The only difference is probably that the JSON driver identifies itself as a block filter, in contrast to raw_bsd. The purpose of this driver is that it may sometimes be desirable to specify options for a block device where only a filename can be given, e.g., for backing files. Using this should obviously be the exception, but it is nice to have if actually needed. Ping – I do understand that Kevin has reservations against this series, but as long as he doesn't explicitly ask me to reimplement this in bdrv_open() without an own block driver (which I'd more or less gladly do), I do not see issues why this series should not be merged. I haven't reviewed it further because it seems like a kludge (that we have to keep supporting once it's merged). Was hoping you and Kevin will come up with a long-term fix instead. Okay, if you think the same, I guess I'll have to rewrite this series. I agree that including this functionality in bdrv_open() is the nicer alternative from the user's point of view, whereas I think using a separate block driver results in nicer code. I'll rewrite this series and then we'll see how bad it actually looks. ;-) Thanks! Stefan
Re: [Qemu-devel] [PATCH 01/13] qapi: Update qapi-code-gen.txt example to match current code
Eric Blake ebl...@redhat.com writes: On 05/05/2014 12:49 AM, Markus Armbruster wrote: Eric Blake ebl...@redhat.com writes: Example: mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qmp-marshal.c The file was missing the example command line that generated this file; I figured it out: mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-commands.py \ --output-dir=qapi-generated --prefix=example- example-schema.json if you want to add that to this patch. Sure, why not, but if there's nothing else to correct in this series, I'd prefer a separate commit to a respin. Followup commit is just fine, if I don't turn up anything else while reviewing the rest of the series. Followup commit. because my fix conflicts with a patch in Luiz's queue. I'll wait for that to land, then post my fix on top.
[Qemu-devel] [PATCH v2 03/12] qapi: Remove unused Visitor callbacks start_handle(), end_handle()
These have never been called or implemented by anything, and their intended use is undocumented, like all of the visitor API. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/qapi/visitor-impl.h | 3 --- qapi/qapi-visit-core.c | 15 --- 2 files changed, 18 deletions(-) diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index f3fa420..166aadd 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -46,9 +46,6 @@ struct Visitor Error **errp); void (*end_optional)(Visitor *v, Error **errp); -void (*start_handle)(Visitor *v, void **obj, const char *kind, - const char *name, Error **errp); -void (*end_handle)(Visitor *v, Error **errp); void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp); void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp); void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp); diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 6451a21..1f7475c 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -17,21 +17,6 @@ #include qapi/visitor.h #include qapi/visitor-impl.h -void visit_start_handle(Visitor *v, void **obj, const char *kind, -const char *name, Error **errp) -{ -if (!error_is_set(errp) v-start_handle) { -v-start_handle(v, obj, kind, name, errp); -} -} - -void visit_end_handle(Visitor *v, Error **errp) -{ -if (!error_is_set(errp) v-end_handle) { -v-end_handle(v, errp); -} -} - void visit_start_struct(Visitor *v, void **obj, const char *kind, const char *name, size_t size, Error **errp) { -- 1.8.1.4
[Qemu-devel] [PATCH v2 01/12] qapi: Update qapi-code-gen.txt example to match current code
Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- docs/qapi-code-gen.txt | 146 ++--- 1 file changed, 90 insertions(+), 56 deletions(-) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index d78921f..923565e 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -223,11 +223,23 @@ Example: mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \ --output-dir=qapi-generated --prefix=example- example-schema.json mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.c -/* AUTOMATICALLY GENERATED, DO NOT MODIFY */ +[Uninteresting stuff omitted...] + +void qapi_free_UserDefOneList(UserDefOneList * obj) +{ +QapiDeallocVisitor *md; +Visitor *v; + +if (!obj) { +return; +} + +md = qapi_dealloc_visitor_new(); +v = qapi_dealloc_get_visitor(md); +visit_type_UserDefOneList(v, obj, NULL, NULL); +qapi_dealloc_visitor_cleanup(md); +} -#include qapi/qapi-dealloc-visitor.h -#include example-qapi-types.h -#include example-qapi-visit.h void qapi_free_UserDefOne(UserDefOne * obj) { @@ -245,31 +257,37 @@ Example: } mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.h -/* AUTOMATICALLY GENERATED, DO NOT MODIFY */ -#ifndef QAPI_GENERATED_EXAMPLE_QAPI_TYPES -#define QAPI_GENERATED_EXAMPLE_QAPI_TYPES +[Uninteresting stuff omitted...] + +#ifndef EXAMPLE_QAPI_TYPES_H +#define EXAMPLE_QAPI_TYPES_H -#include qapi/qapi-types-core.h +[Builtin types omitted...] typedef struct UserDefOne UserDefOne; typedef struct UserDefOneList { -UserDefOne *value; +union { +UserDefOne *value; +uint64_t padding; +}; struct UserDefOneList *next; } UserDefOneList; +[Functions on builtin types omitted...] + struct UserDefOne { int64_t integer; char * string; }; +void qapi_free_UserDefOneList(UserDefOneList * obj); void qapi_free_UserDefOne(UserDefOne * obj); #endif - === scripts/qapi-visit.py === Used to generate the visitor functions used to walk through and convert @@ -293,39 +311,63 @@ Example: mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-visit.py \ --output-dir=qapi-generated --prefix=example- example-schema.json mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.c -/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */ +[Uninteresting stuff omitted...] -#include example-qapi-visit.h +static void visit_type_UserDefOne_fields(Visitor *m, UserDefOne ** obj, Error **errp) +{ +Error *err = NULL; +visit_type_int(m, (*obj)-integer, integer, err); +visit_type_str(m, (*obj)-string, string, err); + +error_propagate(errp, err); +} void visit_type_UserDefOne(Visitor *m, UserDefOne ** obj, const char *name, Error **errp) { -visit_start_struct(m, (void **)obj, UserDefOne, name, sizeof(UserDefOne), errp); -visit_type_int(m, (obj *obj) ? (*obj)-integer : NULL, integer, errp); -visit_type_str(m, (obj *obj) ? (*obj)-string : NULL, string, errp); -visit_end_struct(m, errp); +if (!error_is_set(errp)) { +Error *err = NULL; +visit_start_struct(m, (void **)obj, UserDefOne, name, sizeof(UserDefOne), err); +if (!err) { +if (*obj) { +visit_type_UserDefOne_fields(m, obj, err); +error_propagate(errp, err); +err = NULL; +} +/* Always call end_struct if start_struct succeeded. */ +visit_end_struct(m, err); +} +error_propagate(errp, err); +} } void visit_type_UserDefOneList(Visitor *m, UserDefOneList ** obj, const char *name, Error **errp) { GenericList *i, **prev = (GenericList **)obj; - -visit_start_list(m, name, errp); - -for (; (i = visit_next_list(m, prev, errp)) != NULL; prev = i) { -UserDefOneList *native_i = (UserDefOneList *)i; -visit_type_UserDefOne(m, native_i-value, NULL, errp); +Error *err = NULL; + +if (!error_is_set(errp)) { +visit_start_list(m, name, err); +if (!err) { +for (; (i = visit_next_list(m, prev, err)) != NULL; prev = i) { +UserDefOneList *native_i = (UserDefOneList *)i; +visit_type_UserDefOne(m, native_i-value, NULL, err); +} +error_propagate(errp, err); +err = NULL; + +/* Always call end_list if start_list succeeded. */ +visit_end_list(m, err); +} +error_propagate(errp, err); } - -
[Qemu-devel] [PATCH v2 00/12] qapi: Purge error_is_set()
This is the sixth part, covering QAPI and its users. Luiz agreed to take this through his tree. PATCH 01-08 are preparatory cleanups. PATCH 09-11 fix misuses of the visitor API in hand-written code. Generated code uses the API correctly. PATCH 12 converts QAPI and its users to the common use of the error API, purging error_is_set() along the way. v1 has a PATCH 13 that drops error_is_set(). This depends on all five prior parts of the purge, of which only the first two have been committed already. Luiz asked me to drop it from this series. My series conflicts with LluÃs's qapi: Allow modularization of QAPI schema files and Amos's qapi: fix coding style in generated code, but the conflicts are trivial, and 3-way merge can take care of them. v2: * Fix pasto in commit messages of PATCH 10+11 [Eric] * Fix logic error in PATCH 12 [Eric] * Update copyright notice in PATCH 12 * Unbundle PATCH 13 Markus Armbruster (12): qapi: Update qapi-code-gen.txt example to match current code qapi: Normalize marshalling's visitor initialization and cleanup qapi: Remove unused Visitor callbacks start_handle(), end_handle() qapi: Replace start_optional()/end_optional() by optional() qapi-visit.py: Clean up confusing push_indent() / pop_indent() use qapi: Clean up shadowing of parameters and locals in inner scopes qapi-visit.py: Clean up a sloppy use of field prefix qapi: Un-inline visit of implicit struct hmp: Call visit_end_struct() after visit_start_struct() succeeds hw: Don't call visit_end_struct() after visit_start_struct() fails tests: Don't call visit_end_struct() after visit_start_struct() fails qapi: Replace uncommon use of the error API by the common one docs/qapi-code-gen.txt | 165 ++- hmp.c | 16 +-- hw/timer/mc146818rtc.c | 41 +- hw/virtio/virtio-balloon.c | 33 +++-- include/qapi/visitor-impl.h| 8 +- include/qapi/visitor.h | 5 +- qapi/opts-visitor.c| 5 +- qapi/qapi-visit-core.c | 259 +++-- qapi/qmp-input-visitor.c | 6 +- qapi/string-input-visitor.c| 6 +- scripts/qapi-commands.py | 89 - scripts/qapi-visit.py | 232 +++-- tests/test-qmp-input-strict.c | 28 +++- tests/test-qmp-input-visitor.c | 26 ++-- tests/test-qmp-output-visitor.c| 28 +++- tests/test-visitor-serialization.c | 26 +++- 16 files changed, 558 insertions(+), 415 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH v2 07/12] qapi-visit.py: Clean up a sloppy use of field prefix
generate_visit_struct_fields() generates the base type's struct member name both with and without the field prefix. Harmless, because the field prefix is always empty there: only unboxed complex members have a prefix, and those can't have a base type. Clean it up anyway. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- scripts/qapi-visit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 222ce62..23ae5f2 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -60,7 +60,7 @@ static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error * if base: ret += mcgen(''' -visit_start_implicit_struct(m, (void**) (*obj)-%(c_name)s, sizeof(%(type)s), err); +visit_start_implicit_struct(m, (void**) (*obj)-%(c_prefix)s%(c_name)s, sizeof(%(type)s), err); if (!err) { visit_type_%(type)s_fields(m, (*obj)-%(c_prefix)s%(c_name)s, err); error_propagate(errp, err); -- 1.8.1.4
[Qemu-devel] [PATCH v2 11/12] tests: Don't call visit_end_struct() after visit_start_struct() fails
When visit_start_struct() fails, visit_end_struct() must not be called. Three out of four visit_type_TestStruct() call it anyway. As far as I can tell, visit_start_struct() doesn't actually fail there. Fix them anyway. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- tests/test-qmp-input-strict.c | 18 +- tests/test-qmp-output-visitor.c| 18 +- tests/test-visitor-serialization.c | 18 +- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c index 449d285..ec798c2 100644 --- a/tests/test-qmp-input-strict.c +++ b/tests/test-qmp-input-strict.c @@ -72,14 +72,22 @@ typedef struct TestStruct static void visit_type_TestStruct(Visitor *v, TestStruct **obj, const char *name, Error **errp) { +Error *err = NULL; + visit_start_struct(v, (void **)obj, TestStruct, name, sizeof(TestStruct), - errp); + err); +if (err) { +goto out; +} + +visit_type_int(v, (*obj)-integer, integer, err); +visit_type_bool(v, (*obj)-boolean, boolean, err); +visit_type_str(v, (*obj)-string, string, err); -visit_type_int(v, (*obj)-integer, integer, errp); -visit_type_bool(v, (*obj)-boolean, boolean, errp); -visit_type_str(v, (*obj)-string, string, errp); +visit_end_struct(v, err); -visit_end_struct(v, errp); +out: +error_propagate(errp, err); } static void test_validate_struct(TestInputVisitorData *data, diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c index 2580f3d..dfd597c 100644 --- a/tests/test-qmp-output-visitor.c +++ b/tests/test-qmp-output-visitor.c @@ -176,14 +176,22 @@ typedef struct TestStruct static void visit_type_TestStruct(Visitor *v, TestStruct **obj, const char *name, Error **errp) { +Error *err = NULL; + visit_start_struct(v, (void **)obj, TestStruct, name, sizeof(TestStruct), - errp); + err); +if (err) { +goto out; +} + +visit_type_int(v, (*obj)-integer, integer, err); +visit_type_bool(v, (*obj)-boolean, boolean, err); +visit_type_str(v, (*obj)-string, string, err); -visit_type_int(v, (*obj)-integer, integer, errp); -visit_type_bool(v, (*obj)-boolean, boolean, errp); -visit_type_str(v, (*obj)-string, string, errp); +visit_end_struct(v, err); -visit_end_struct(v, errp); +out: +error_propagate(errp, err); } static void test_visitor_out_struct(TestOutputVisitorData *data, diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c index 8166cf1..85170e5 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -195,13 +195,21 @@ typedef struct TestStruct static void visit_type_TestStruct(Visitor *v, TestStruct **obj, const char *name, Error **errp) { -visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), errp); +Error *err= NULL; -visit_type_int(v, (*obj)-integer, integer, errp); -visit_type_bool(v, (*obj)-boolean, boolean, errp); -visit_type_str(v, (*obj)-string, string, errp); +visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), err); +if (err) { +goto out; +} + +visit_type_int(v, (*obj)-integer, integer, err); +visit_type_bool(v, (*obj)-boolean, boolean, err); +visit_type_str(v, (*obj)-string, string, err); + +visit_end_struct(v, err); -visit_end_struct(v, errp); +out: +error_propagate(errp, err); } static TestStruct *struct_create(void) -- 1.8.1.4
[Qemu-devel] [PATCH v2 08/12] qapi: Un-inline visit of implicit struct
In preparation of error handling changes. Bonus: generates less duplicated code. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- scripts/qapi-visit.py | 48 ++-- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 23ae5f2..3e161bf 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -17,6 +17,31 @@ import os import getopt import errno +implicit_structs = [] + +def generate_visit_implicit_struct(type): +global implicit_structs +if type in implicit_structs: +return '' +implicit_structs.append(type) +return mcgen(''' + +static void visit_type_implicit_%(c_type)s(Visitor *m, %(c_type)s **obj, Error **errp) +{ +Error *err = NULL; + +visit_start_implicit_struct(m, (void **)obj, sizeof(%(c_type)s), err); +if (!err) { +visit_type_%(c_type)s_fields(m, obj, err); +error_propagate(errp, err); +err = NULL; +visit_end_implicit_struct(m, err); +} +error_propagate(errp, err); +} +''', + c_type=type_name(type)) + def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base = None): substructs = [] ret = '' @@ -49,6 +74,9 @@ static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s **obj } ''') +if base: +ret += generate_visit_implicit_struct(base) + ret += mcgen(''' static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error **errp) @@ -60,13 +88,7 @@ static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error * if base: ret += mcgen(''' -visit_start_implicit_struct(m, (void**) (*obj)-%(c_prefix)s%(c_name)s, sizeof(%(type)s), err); -if (!err) { -visit_type_%(type)s_fields(m, (*obj)-%(c_prefix)s%(c_name)s, err); -error_propagate(errp, err); -err = NULL; -visit_end_implicit_struct(m, err); -} +visit_type_implicit_%(type)s(m, (*obj)-%(c_prefix)s%(c_name)s, err); ''', c_prefix=c_var(field_prefix), type=type_name(base), c_name=c_var('base')) @@ -292,6 +314,10 @@ def generate_visit_union(expr): del base_fields[discriminator] ret += generate_visit_struct_fields(name, , , base_fields) +if discriminator: +for key in members: +ret += generate_visit_implicit_struct(members[key]) + ret += mcgen(''' void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp) @@ -330,13 +356,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** if not discriminator: fmt = 'visit_type_%(c_type)s(m, (*obj)-%(c_name)s, data, err);' else: -fmt = '''visit_start_implicit_struct(m, (void**) (*obj)-%(c_name)s, sizeof(%(c_type)s), err); -if (!err) { -visit_type_%(c_type)s_fields(m, (*obj)-%(c_name)s, err); -error_propagate(errp, err); -err = NULL; -visit_end_implicit_struct(m, err); -}''' +fmt = 'visit_type_implicit_%(c_type)s(m, (*obj)-%(c_name)s, err);' enum_full_value = generate_enum_full_value(disc_type, key) ret += mcgen(''' -- 1.8.1.4
[Qemu-devel] [PATCH v2 05/12] qapi-visit.py: Clean up confusing push_indent() / pop_indent() use
Changing implicit indentation in the middle of generating a block makes following the code being generated unnecessarily hard. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- scripts/qapi-visit.py | 32 ++-- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index b38d62e..3eeb435 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -128,12 +128,14 @@ if (!err) { ''', name=full_name) +ret += mcgen(''' +/* Always call end_struct if start_struct succeeded. */ +visit_end_struct(m, err); +} +error_propagate(errp, err); +''') pop_indent() ret += mcgen(''' -/* Always call end_struct if start_struct succeeded. */ -visit_end_struct(m, err); -} -error_propagate(errp, err); } ''') return ret @@ -289,19 +291,15 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** ''', name=name) - -push_indent() push_indent() push_indent() if base: ret += mcgen(''' -visit_type_%(name)s_fields(m, obj, err); +visit_type_%(name)s_fields(m, obj, err); ''', name=name) -pop_indent() - if not discriminator: disc_key = type else: @@ -343,19 +341,17 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** } error_propagate(errp, err); err = NULL; -} ''') pop_indent() -ret += mcgen(''' -/* Always call end_struct if start_struct succeeded. */ -visit_end_struct(m, err); -} -error_propagate(errp, err); -} -''') +pop_indent() -pop_indent(); ret += mcgen(''' +} +/* Always call end_struct if start_struct succeeded. */ +visit_end_struct(m, err); +} +error_propagate(errp, err); +} } ''') -- 1.8.1.4
[Qemu-devel] [PATCH v2 02/12] qapi: Normalize marshalling's visitor initialization and cleanup
Input and output marshalling functions do it differently. Change them to work the same: initialize the I/O visitor, use it, clean it up, initialize the dealloc visitor, use it, clean it up. This delays dealloc visitor initialization in output marshalling functions, and input visitor cleanup in input marshalling functions. No functional change, but the latter will be convenient when I change the error handling. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- docs/qapi-code-gen.txt | 8 scripts/qapi-commands.py | 27 --- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 923565e..ac951ef 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -399,8 +399,8 @@ Example: static void qmp_marshal_output_my_command(UserDefOne * ret_in, QObject **ret_out, Error **errp) { -QapiDeallocVisitor *md = qapi_dealloc_visitor_new(); QmpOutputVisitor *mo = qmp_output_visitor_new(); +QapiDeallocVisitor *md; Visitor *v; v = qmp_output_get_visitor(mo); @@ -409,6 +409,7 @@ Example: *ret_out = qmp_output_get_qobject(mo); } qmp_output_visitor_cleanup(mo); +md = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(md); visit_type_UserDefOne(v, ret_in, unused, NULL); qapi_dealloc_visitor_cleanup(md); @@ -417,15 +418,13 @@ Example: static void qmp_marshal_input_my_command(QDict *args, QObject **ret, Error **errp) { UserDefOne * retval = NULL; -QmpInputVisitor *mi; +QmpInputVisitor *mi = qmp_input_visitor_new_strict(QOBJECT(args)); QapiDeallocVisitor *md; Visitor *v; UserDefOne * arg1 = NULL; -mi = qmp_input_visitor_new_strict(QOBJECT(args)); v = qmp_input_get_visitor(mi); visit_type_UserDefOne(v, arg1, arg1, errp); -qmp_input_visitor_cleanup(mi); if (error_is_set(errp)) { goto out; @@ -436,6 +435,7 @@ Example: } out: +qmp_input_visitor_cleanup(mi); md = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(md); visit_type_UserDefOne(v, arg1, arg1, NULL); diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 9734ab0..f56cc1c 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -69,16 +69,17 @@ def gen_marshal_output_call(name, ret_type): return return qmp_marshal_output_%s(retval, ret, errp); % c_fun(name) -def gen_visitor_input_containers_decl(args): +def gen_visitor_input_containers_decl(args, obj): ret = push_indent() if len(args) 0: ret += mcgen(''' -QmpInputVisitor *mi; +QmpInputVisitor *mi = qmp_input_visitor_new_strict(%(obj)s); QapiDeallocVisitor *md; Visitor *v; -''') +''', + obj=obj) pop_indent() return ret.rstrip() @@ -106,7 +107,7 @@ bool has_%(argname)s = false; pop_indent() return ret.rstrip() -def gen_visitor_input_block(args, obj, dealloc=False): +def gen_visitor_input_block(args, dealloc=False): ret = errparg = 'errp' @@ -118,15 +119,14 @@ def gen_visitor_input_block(args, obj, dealloc=False): if dealloc: errparg = 'NULL' ret += mcgen(''' +qmp_input_visitor_cleanup(mi); md = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(md); ''') else: ret += mcgen(''' -mi = qmp_input_visitor_new_strict(%(obj)s); v = qmp_input_get_visitor(mi); -''', - obj=obj) +''') for argname, argtype, optional, structured in parse_args(args): if optional: @@ -152,10 +152,6 @@ visit_end_optional(v, %(errp)s); ret += mcgen(''' qapi_dealloc_visitor_cleanup(md); ''') -else: -ret += mcgen(''' -qmp_input_visitor_cleanup(mi); -''') pop_indent() return ret.rstrip() @@ -166,8 +162,8 @@ def gen_marshal_output(name, args, ret_type, middle_mode): ret = mcgen(''' static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_out, Error **errp) { -QapiDeallocVisitor *md = qapi_dealloc_visitor_new(); QmpOutputVisitor *mo = qmp_output_visitor_new(); +QapiDeallocVisitor *md; Visitor *v; v = qmp_output_get_visitor(mo); @@ -176,6 +172,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_o *ret_out = qmp_output_get_qobject(mo); } qmp_output_visitor_cleanup(mo); +md = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(md); %(visitor)s(v, ret_in, unused, NULL); qapi_dealloc_visitor_cleanup(md); @@ -228,9 +225,9 @@ def gen_marshal_input(name, args, ret_type, middle_mode): %(visitor_input_block)s ''', - visitor_input_containers_decl=gen_visitor_input_containers_decl(args), +
[Qemu-devel] [PATCH v2 04/12] qapi: Replace start_optional()/end_optional() by optional()
Semantics of end_optional() differ subtly from the other end_FOO() callbacks: when start_FOO() succeeds, the matching end_FOO() gets called regardless of what happens in between. end_optional() gets called only when everything in between succeeds as well. Entirely undocumented, like all of the visitor API. The only user of Visitor Callback end_optional() never did anything, and was removed in commit 9f9ab46. I'm about to clean up error handling in the generated visitor code, and end_optional() is in my way. No users mean no test cases, and making non-trivial cleanup transformations without test cases doesn't strike me as a good idea. Drop end_optional(), and rename start_optional() to optional(). We can always go back to a pair of callbacks when we have an actual need. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- include/qapi/visitor-impl.h | 5 ++--- include/qapi/visitor.h | 5 ++--- qapi/opts-visitor.c | 5 ++--- qapi/qapi-visit-core.c | 15 --- qapi/qmp-input-visitor.c| 6 +++--- qapi/string-input-visitor.c | 6 +++--- scripts/qapi-commands.py| 5 ++--- scripts/qapi-visit.py | 3 +-- 8 files changed, 19 insertions(+), 31 deletions(-) diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 166aadd..ecc0183 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -42,9 +42,8 @@ struct Visitor Error **errp); /* May be NULL */ -void (*start_optional)(Visitor *v, bool *present, const char *name, - Error **errp); -void (*end_optional)(Visitor *v, Error **errp); +void (*optional)(Visitor *v, bool *present, const char *name, + Error **errp); void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp); void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp); diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 29da211..4a0178f 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -39,9 +39,8 @@ void visit_end_implicit_struct(Visitor *v, Error **errp); void visit_start_list(Visitor *v, const char *name, Error **errp); GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp); void visit_end_list(Visitor *v, Error **errp); -void visit_start_optional(Visitor *v, bool *present, const char *name, - Error **errp); -void visit_end_optional(Visitor *v, Error **errp); +void visit_optional(Visitor *v, bool *present, const char *name, +Error **errp); void visit_get_next_type(Visitor *v, int *obj, const int *qtypes, const char *name, Error **errp); void visit_type_enum(Visitor *v, int *obj, const char *strings[], diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 5d830a2..1632c54 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -483,8 +483,7 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp) static void -opts_start_optional(Visitor *v, bool *present, const char *name, - Error **errp) +opts_optional(Visitor *v, bool *present, const char *name, Error **errp) { OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v); @@ -527,7 +526,7 @@ opts_visitor_new(const QemuOpts *opts) /* type_number() is not filled in, but this is not the first visitor to * skip some mandatory methods... */ -ov-visitor.start_optional = opts_start_optional; +ov-visitor.optional = opts_optional; ov-opts_root = opts; diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 1f7475c..ffd7637 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -69,18 +69,11 @@ void visit_end_list(Visitor *v, Error **errp) v-end_list(v, errp); } -void visit_start_optional(Visitor *v, bool *present, const char *name, - Error **errp) +void visit_optional(Visitor *v, bool *present, const char *name, +Error **errp) { -if (!error_is_set(errp) v-start_optional) { -v-start_optional(v, present, name, errp); -} -} - -void visit_end_optional(Visitor *v, Error **errp) -{ -if (!error_is_set(errp) v-end_optional) { -v-end_optional(v, errp); +if (!error_is_set(errp) v-optional) { +v-optional(v, present, name, errp); } } diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index a2bed1e..d861206 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -286,8 +286,8 @@ static void qmp_input_type_number(Visitor *v, double *obj, const char *name, } } -static void qmp_input_start_optional(Visitor *v, bool *present, - const char *name, Error **errp) +static void qmp_input_optional(Visitor *v, bool *present, const char *name, + Error **errp) {
[Qemu-devel] [PATCH v2 10/12] hw: Don't call visit_end_struct() after visit_start_struct() fails
When visit_start_struct() fails, visit_end_struct() must not be called. rtc_get_date() and balloon_stats_all() call it anyway. As far as I can tell, they're only used with the string output visitor, which doesn't care. Fix them anyway. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- hw/timer/mc146818rtc.c | 23 +++ hw/virtio/virtio-balloon.c | 25 +++-- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c index 8509309..6c3e3b6 100644 --- a/hw/timer/mc146818rtc.c +++ b/hw/timer/mc146818rtc.c @@ -793,19 +793,26 @@ static const MemoryRegionOps cmos_ops = { static void rtc_get_date(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { +Error *err = NULL; RTCState *s = MC146818_RTC(obj); struct tm current_tm; rtc_update_time(s); rtc_get_time(s, current_tm); -visit_start_struct(v, NULL, struct tm, name, 0, errp); -visit_type_int32(v, current_tm.tm_year, tm_year, errp); -visit_type_int32(v, current_tm.tm_mon, tm_mon, errp); -visit_type_int32(v, current_tm.tm_mday, tm_mday, errp); -visit_type_int32(v, current_tm.tm_hour, tm_hour, errp); -visit_type_int32(v, current_tm.tm_min, tm_min, errp); -visit_type_int32(v, current_tm.tm_sec, tm_sec, errp); -visit_end_struct(v, errp); +visit_start_struct(v, NULL, struct tm, name, 0, err); +if (err) { +goto out; +} +visit_type_int32(v, current_tm.tm_year, tm_year, err); +visit_type_int32(v, current_tm.tm_mon, tm_mon, err); +visit_type_int32(v, current_tm.tm_mday, tm_mday, err); +visit_type_int32(v, current_tm.tm_hour, tm_hour, err); +visit_type_int32(v, current_tm.tm_min, tm_min, err); +visit_type_int32(v, current_tm.tm_sec, tm_sec, err); +visit_end_struct(v, err); + +out: +error_propagate(errp, err); } static void rtc_realizefn(DeviceState *dev, Error **errp) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 51f02eb..65d05c8 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -108,6 +108,7 @@ static void balloon_stats_poll_cb(void *opaque) static void balloon_stats_get_all(Object *obj, struct Visitor *v, void *opaque, const char *name, Error **errp) { +Error *err = NULL; VirtIOBalloon *s = opaque; int i; @@ -116,17 +117,29 @@ static void balloon_stats_get_all(Object *obj, struct Visitor *v, return; } -visit_start_struct(v, NULL, guest-stats, name, 0, errp); -visit_type_int(v, s-stats_last_update, last-update, errp); +visit_start_struct(v, NULL, guest-stats, name, 0, err); +if (err) { +goto out; +} + +visit_type_int(v, s-stats_last_update, last-update, err); -visit_start_struct(v, NULL, NULL, stats, 0, errp); +visit_start_struct(v, NULL, NULL, stats, 0, err); +if (err) { +goto out_end; +} + for (i = 0; i VIRTIO_BALLOON_S_NR; i++) { visit_type_int64(v, (int64_t *) s-stats[i], balloon_stat_names[i], - errp); + err); } -visit_end_struct(v, errp); +visit_end_struct(v, err); + +out_end: +visit_end_struct(v, err); -visit_end_struct(v, errp); +out: +error_propagate(errp, err); } static void balloon_stats_get_poll_interval(Object *obj, struct Visitor *v, -- 1.8.1.4
[Qemu-devel] [PATCH v2 09/12] hmp: Call visit_end_struct() after visit_start_struct() succeeds
When visit_start_struct() succeeds, visit_end_struct() must be called. hmp_object_add() doesn't when a member visit fails. As far as I can tell, the opts visitor copes okay with the misuse. Fix it anyway. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- hmp.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/hmp.c b/hmp.c index ad31ceb..a7cd59e 100644 --- a/hmp.c +++ b/hmp.c @@ -1384,6 +1384,7 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict) void hmp_object_add(Monitor *mon, const QDict *qdict) { Error *err = NULL; +Error *err_end = NULL; QemuOpts *opts; char *type = NULL; char *id = NULL; @@ -1407,24 +1408,23 @@ void hmp_object_add(Monitor *mon, const QDict *qdict) qdict_del(pdict, qom-type); visit_type_str(opts_get_visitor(ov), type, qom-type, err); if (err) { -goto out_clean; +goto out_end; } qdict_del(pdict, id); visit_type_str(opts_get_visitor(ov), id, id, err); if (err) { -goto out_clean; +goto out_end; } object_add(type, id, pdict, opts_get_visitor(ov), err); -if (err) { -goto out_clean; -} -visit_end_struct(opts_get_visitor(ov), err); -if (err) { + +out_end: +visit_end_struct(opts_get_visitor(ov), err_end); +if (!err err_end) { qmp_object_del(id, NULL); } - +error_propagate(err, err_end); out_clean: opts_visitor_cleanup(ov); -- 1.8.1.4
[Qemu-devel] [PATCH v2 12/12] qapi: Replace uncommon use of the error API by the common one
We commonly use the error API like this: err = NULL; foo(..., err); if (err) { goto out; } bar(..., err); Every error source is checked separately. The second function is only called when the first one succeeds. Both functions are free to pass their argument to error_set(). Because error_set() asserts no error has been set, this effectively means they must not be called with an error set. The qapi-generated code uses the error API differently: // *errp was initialized to NULL somewhere up the call chain frob(..., errp); gnat(..., errp); Errors accumulate in *errp: first error wins, subsequent errors get dropped. To make this work, the second function does nothing when called with an error set. Requires non-null errp, or else the second function can't see the first one fail. This usage has also bled into visitor tests, and two device model object property getters rtc_get_date() and balloon_stats_get_all(). With the accumulate technique, you need fewer error checks in callers, and buy that with an error check in every callee. Can be nice. However, mixing the two techniques is confusing. You can't use the accumulate technique with functions designed for the check separately technique. You can use the check separately technique with functions designed for the accumulate technique, but then error_set() can't catch you setting an error more than once. Standardize on the check separately technique for now, because it's overwhelmingly prevalent. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- docs/qapi-code-gen.txt | 87 -- hw/timer/mc146818rtc.c | 24 +++- hw/virtio/virtio-balloon.c | 12 +- qapi/qapi-visit-core.c | 231 - scripts/qapi-commands.py | 57 ++--- scripts/qapi-visit.py | 171 +-- tests/test-qmp-input-strict.c | 10 +- tests/test-qmp-input-visitor.c | 26 +++-- tests/test-qmp-output-visitor.c| 10 +- tests/test-visitor-serialization.c | 12 +- 10 files changed, 354 insertions(+), 286 deletions(-) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index ac951ef..eb32981 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -240,7 +240,6 @@ Example: qapi_dealloc_visitor_cleanup(md); } - void qapi_free_UserDefOne(UserDefOne * obj) { QapiDeallocVisitor *md; @@ -317,49 +316,54 @@ Example: { Error *err = NULL; visit_type_int(m, (*obj)-integer, integer, err); +if (err) { +goto out; +} visit_type_str(m, (*obj)-string, string, err); +if (err) { +goto out; +} +out: error_propagate(errp, err); } void visit_type_UserDefOne(Visitor *m, UserDefOne ** obj, const char *name, Error **errp) { -if (!error_is_set(errp)) { -Error *err = NULL; -visit_start_struct(m, (void **)obj, UserDefOne, name, sizeof(UserDefOne), err); -if (!err) { -if (*obj) { -visit_type_UserDefOne_fields(m, obj, err); -error_propagate(errp, err); -err = NULL; -} -/* Always call end_struct if start_struct succeeded. */ -visit_end_struct(m, err); +Error *err = NULL; + +visit_start_struct(m, (void **)obj, UserDefOne, name, sizeof(UserDefOne), err); +if (!err) { +if (*obj) { +visit_type_UserDefOne_fields(m, obj, errp); } -error_propagate(errp, err); +visit_end_struct(m, err); } +error_propagate(errp, err); } void visit_type_UserDefOneList(Visitor *m, UserDefOneList ** obj, const char *name, Error **errp) { -GenericList *i, **prev = (GenericList **)obj; Error *err = NULL; +GenericList *i, **prev; -if (!error_is_set(errp)) { -visit_start_list(m, name, err); -if (!err) { -for (; (i = visit_next_list(m, prev, err)) != NULL; prev = i) { -UserDefOneList *native_i = (UserDefOneList *)i; -visit_type_UserDefOne(m, native_i-value, NULL, err); -} -error_propagate(errp, err); -err = NULL; - -/* Always call end_list if start_list succeeded. */ -visit_end_list(m, err); -} -error_propagate(errp, err); +visit_start_list(m, name, err); +if (err) { +goto out; +} + +for (prev = (GenericList **)obj; + !err (i = visit_next_list(m, prev, err)) != NULL; + prev = i) { +UserDefOneList *native_i = (UserDefOneList *)i; +
[Qemu-devel] [PATCH v2 06/12] qapi: Clean up shadowing of parameters and locals in inner scopes
By un-inlining the visit of nested complex types. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- scripts/qapi-visit.py | 20 +--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 3eeb435..222ce62 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -35,6 +35,19 @@ def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base = nested_field_prefix = %s%s. % (field_prefix, argname) ret += generate_visit_struct_fields(name, nested_field_prefix, nested_fn_prefix, argentry) +ret += mcgen(''' + +static void visit_type_%(full_name)s_field_%(c_name)s(Visitor *m, %(name)s **obj, Error **errp) +{ +Error *err = NULL; +''', + name=name, full_name=full_name, c_name=c_var(argname)) +push_indent() +ret += generate_visit_struct_body(full_name, argname, argentry) +pop_indent() +ret += mcgen(''' +} +''') ret += mcgen(''' @@ -69,7 +82,10 @@ if ((*obj)-%(prefix)shas_%(c_name)s) { push_indent() if structured: -ret += generate_visit_struct_body(full_name, argname, argentry) +ret += mcgen(''' +visit_type_%(full_name)s_field_%(c_name)s(m, obj, err); +''', + full_name=full_name, c_name=c_var(argname)) else: ret += mcgen(''' visit_type_%(type)s(m, (*obj)-%(c_prefix)s%(c_name)s, %(name)s, err); @@ -106,8 +122,6 @@ if (!error_is_set(errp)) { if len(field_prefix): ret += mcgen(''' -Error **errp = err; /* from outer scope */ -Error *err = NULL; visit_start_struct(m, NULL, , %(name)s, 0, err); ''', name=name) -- 1.8.1.4
Re: [Qemu-devel] [PATCH v3] glib: fix g_poll early timeout on windows
On Fri, Apr 18, 2014 at 08:24:03PM +0400, Stanislav Vorobiov wrote: Please fix the following compiler warning with gcc 4.8.2: +} else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) || + ((int)res WAIT_OBJECT_0) || + (res = (WAIT_OBJECT_0 + nhandles))) { +break; +} util/oslib-win32.c: In function 'g_poll_fixed': util/oslib-win32.c:324:21: warning: comparison of unsigned expression 0 is always false [-Wtype-limits] ((int)res WAIT_OBJECT_0) || ^
Re: [Qemu-devel] [PATCH for-2.0 0/7] SMBus and tmp105 fixes
Il 02/04/2014 17:55, Michael S. Tsirkin ha scritto: smbus: allow returning an error from reads smbus: return -1 if nothing found at the given address pm_smbus: correctly report unclaimed cycles I've reviewed these and they look sane and safe for 2.0. mst, could you have a second look as PC maintainer and take them? I'd rather delay to 2.1. It's not a regression is it? Ping? Paolo
Re: [Qemu-devel] [PATCH v2] vmdk: Optimize cluster allocation
On Wed, May 07, 2014 at 09:45:17AM +0800, Fam Zheng wrote: On Tue, 05/06 10:32, Fam Zheng wrote: @@ -1110,12 +,20 @@ static int get_cluster_offset(BlockDriverState *bs, } /* Avoid the L2 tables update for the images that have snapshots. */ -*cluster_offset = bdrv_getlength(extent-file); +ret = bdrv_getlength(extent-file); +if (ret 0 || +ret ((extent-cluster_sectors BDRV_SECTOR_BITS) - 1)) { +return VMDK_ERROR; +} +*cluster_offset = ret; if (!extent-compressed) { -bdrv_truncate( -extent-file, -*cluster_offset + (extent-cluster_sectors 9) -); +ret = bdrv_write_zeroes(extent-file, +*cluster_offset BDRV_SECTOR_BITS, +extent-cluster_sectors, +0); Hi Stefan, By considering a bdrv_write_zeroes as a pre-write, it in general doubles the write for the whole image, so it's not a good solution. A better way would be removing the bdrv_truncate and require the caller to do full cluster write (with a bounce buffer if necessary). So let's drop this patch. Okay, thanks for investigating this. Stefan
Re: [Qemu-devel] Configure virtio-scsi options via libvirt
On Tue, May 06, 2014 at 04:13:50PM -0700, Mike Perez wrote: I would like be able to configure virtio-scsi options num_queues, max_sectors, and cmd_per_lun via libvirt. Are there any plans to have this support? Hi Mike, I'm not sure about the status of libvirt support for virtio-scsi options but in the meantime you can use qemu:commandline passthrough: http://blog.vmsplice.net/2011/04/how-to-pass-qemu-command-line-options.html Stefan
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
On Tue, 6 May 2014 19:37:22 +0100 Peter Maydell peter.mayd...@linaro.org wrote: On 5 May 2014 09:07, Greg Kurz gk...@linux.vnet.ibm.com wrote: POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR special purpose register to decide the endianness to use when entering interrupt handlers. When running a Linux guest, this provides a hint on the endianness used by the kernel. From a QEMU point of view, the information is needed for legacy virtio support and crash dump support as well. Do you care about the case of: * kernel bigendian Yes. FWIW, ppc64 is still widely used in big endian mode we don't want to break. * userspace littleendian (or vice-versa) We don't care about userspace here. We assume that virtio structures are owned by the guest kernel. * guest kernel passes virtio device through to guest userspace Not sure to understand... could you please point me to an example ? * guest userspace is doing the manipulation of the device Hmm... you mean we would have virtio drivers implemented in the guest userspace ? Does that exist ? Please elaborate. ? (Will Deacon just suggested this as a possibility on the kvm-arm mailing list...) Just discovered some virtio endian threads in the kvm-arm@ archives... I'll take some time to read. Also, are we documenting what the process should be for a virtio implementation to decide the endianness for a particular architecture? I assume we'd like kvmtool and QEMU to do the same thing rather than subtly different things... Sure ! thanks -- PMM Thanks. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 Anarchy is about taking complete responsibility for yourself. Alan Moore.
Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 2/4] ppc64-dump: Support dump for little endian ppc64
On Mon, 05 May 2014 13:04:35 +0200 Alexander Graf ag...@suse.de wrote: On 05/05/2014 10:05 AM, Greg Kurz wrote: From: Bharata B Rao bhar...@linux.vnet.ibm.com Fix ppc64 arch specific dump code to work correctly for little endian guests. We introduce a NoteFuncArg type to avoid adding extra arguments to all note functions. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com [ rebased on top of current master branch, introduced NoteFuncArg, use new cpu_to_dump{16,32,64} endian helpers, Greg Kurz gk...@linux.vnet.ibm.com ] Reviewed-by: Alexander Graf ag...@suse.de Signed-off-by: Greg Kurz gk...@linux.vnet.ibm.com --- Changes in v3: - better taste with the endian helpers naming target-ppc/arch_dump.c | 82 +--- 1 file changed, 49 insertions(+), 33 deletions(-) diff --git a/target-ppc/arch_dump.c b/target-ppc/arch_dump.c index 9dccf1a..5487b61 100644 --- a/target-ppc/arch_dump.c +++ b/target-ppc/arch_dump.c @@ -79,94 +79,109 @@ typedef struct noteStruct { } contents; } QEMU_PACKED Note; +typedef struct NoteFuncArg { +Note note; +DumpState *state; +} NoteFuncArg; -static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu) +static void ppc64_write_elf64_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu) { int i; uint64_t cr; struct PPC64ElfPrstatus *prstatus; struct PPC64UserRegStruct *reg; +Note *note = arg-note; +DumpState *s = arg-state; -note-hdr.n_type = cpu_to_be32(NT_PRSTATUS); +note-hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS); prstatus = note-contents.prstatus; memset(prstatus, 0, sizeof(*prstatus)); reg = prstatus-pr_reg; for (i = 0; i 32; i++) { -reg-gpr[i] = cpu_to_be64(cpu-env.gpr[i]); +reg-gpr[i] = cpu_to_dump64(s, cpu-env.gpr[i]); } -reg-nip = cpu_to_be64(cpu-env.nip); -reg-msr = cpu_to_be64(cpu-env.msr); -reg-ctr = cpu_to_be64(cpu-env.ctr); -reg-link = cpu_to_be64(cpu-env.lr); -reg-xer = cpu_to_be64(cpu_read_xer(cpu-env)); +reg-nip = cpu_to_dump64(s, cpu-env.nip); +reg-msr = cpu_to_dump64(s, cpu-env.msr); +reg-ctr = cpu_to_dump64(s, cpu-env.ctr); +reg-link = cpu_to_dump64(s, cpu-env.lr); +reg-xer = cpu_to_dump64(s, cpu_read_xer(cpu-env)); cr = 0; for (i = 0; i 8; i++) { cr |= (cpu-env.crf[i] 15) (4 * (7 - i)); } -reg-ccr = cpu_to_be64(cr); +reg-ccr = cpu_to_dump64(s, cr); } -static void ppc64_write_elf64_fpregset(Note *note, PowerPCCPU *cpu) +static void ppc64_write_elf64_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu) { int i; struct PPC64ElfFpregset *fpregset; +Note *note = arg-note; +DumpState *s = arg-state; -note-hdr.n_type = cpu_to_be32(NT_PRFPREG); +note-hdr.n_type = cpu_to_dump32(s, NT_PRFPREG); fpregset = note-contents.fpregset; memset(fpregset, 0, sizeof(*fpregset)); for (i = 0; i 32; i++) { -fpregset-fpr[i] = cpu_to_be64(cpu-env.fpr[i]); +fpregset-fpr[i] = cpu_to_dump64(s, cpu-env.fpr[i]); } -fpregset-fpscr = cpu_to_be64(cpu-env.fpscr); +fpregset-fpscr = cpu_to_dump64(s, cpu-env.fpscr); } -static void ppc64_write_elf64_vmxregset(Note *note, PowerPCCPU *cpu) +static void ppc64_write_elf64_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu) { int i; struct PPC64ElfVmxregset *vmxregset; +Note *note = arg-note; +DumpState *s = arg-state; -note-hdr.n_type = cpu_to_be32(NT_PPC_VMX); +note-hdr.n_type = cpu_to_dump32(s, NT_PPC_VMX); vmxregset = note-contents.vmxregset; memset(vmxregset, 0, sizeof(*vmxregset)); for (i = 0; i 32; i++) { -vmxregset-avr[i].u64[0] = cpu_to_be64(cpu-env.avr[i].u64[0]); -vmxregset-avr[i].u64[1] = cpu_to_be64(cpu-env.avr[i].u64[1]); +vmxregset-avr[i].u64[0] = cpu_to_dump64(s, cpu-env.avr[i].u64[0]); +vmxregset-avr[i].u64[1] = cpu_to_dump64(s, cpu-env.avr[i].u64[1]); Is this correct? Tom, could you please ack if it is? Alex Tom, Can you comment plz ? Thanks. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 Anarchy is about taking complete responsibility for yourself. Alan Moore.
Re: [Qemu-devel] [PATCH v2] vmdk: Optimize cluster allocation
Am 07.05.2014 um 03:45 hat Fam Zheng geschrieben: On Tue, 05/06 10:32, Fam Zheng wrote: On mounted NFS filesystem, ftruncate is much much slower than doing a zero write. Changing this significantly speeds up cluster allocation. Comparing by converting a cirros image (296M) to VMDK on an NFS mount point, over 1Gbe LAN: $ time qemu-img convert cirros-0.3.1.img /mnt/a.raw -O vmdk Before: real0m26.464s user0m0.133s sys 0m0.527s After: real0m2.120s user0m0.080s sys 0m0.197s Signed-off-by: Fam Zheng f...@redhat.com --- V2: Fix cluster_offset check. (Kevin) Signed-off-by: Fam Zheng f...@redhat.com --- block/vmdk.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 06a1f9f..98d2d56 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1037,6 +1037,7 @@ static int get_cluster_offset(BlockDriverState *bs, int min_index, i, j; uint32_t min_count, *l2_table; bool zeroed = false; +int64_t ret; if (m_data) { m_data-valid = 0; @@ -1110,12 +,20 @@ static int get_cluster_offset(BlockDriverState *bs, } /* Avoid the L2 tables update for the images that have snapshots. */ -*cluster_offset = bdrv_getlength(extent-file); +ret = bdrv_getlength(extent-file); +if (ret 0 || +ret ((extent-cluster_sectors BDRV_SECTOR_BITS) - 1)) { +return VMDK_ERROR; +} +*cluster_offset = ret; if (!extent-compressed) { -bdrv_truncate( -extent-file, -*cluster_offset + (extent-cluster_sectors 9) -); +ret = bdrv_write_zeroes(extent-file, +*cluster_offset BDRV_SECTOR_BITS, +extent-cluster_sectors, +0); Hi Stefan, By considering a bdrv_write_zeroes as a pre-write, it in general doubles the write for the whole image, so it's not a good solution. A better way would be removing the bdrv_truncate and require the caller to do full cluster write (with a bounce buffer if necessary). Doesn't get_whole_cluster() already ensure that you already write a full cluster to the image file? However, it might be better to not use bdrv_getlength() each time you need a new cluster, but instead use a field in VmdkExtent to keep the next free cluster offset (which is rounded up in vmdk_open). This will ensure that we don't overlap the next cluster allocation in case get_whole_cluster() fails halfway through. (In fact, the L2 table should only be updated after get_whole_cluster() has succeeded, but we can do both to be on the safe side...) Kevin
Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
Am 06.05.2014 um 23:03 hat Max Reitz geschrieben: On 06.05.2014 13:10, Jan Kiszka wrote: On 2014-05-06 12:19, Kevin Wolf wrote: The immediately visible effect of this patch is that it fixes committing a temporary snapshot to its backing file. Previously, it would fail with a permission denied error because bdrv_inherited_flags() forced the backing file to be read-only, ignoring the r/w reopen of bdrv_commit(). The bigger problem this releaved is that the original open flags must actually only be applied to the temporary snapshot, and the original image file must be treated as a backing file of the temporary snapshot and get the right flags for that. Reported-by: Jan Kiszka jan.kis...@web.de Signed-off-by: Kevin Wolf kw...@redhat.com --- block.c| 34 +++--- include/block/block.h | 2 +- tests/qemu-iotests/051 | 4 tests/qemu-iotests/051.out | 10 ++ 4 files changed, 34 insertions(+), 16 deletions(-) Works fine here! (For unknown reason, applying the iotest hunk didn't work for me, though.) The lines are too long and therefore split in this mail, they need to be joined manually before applying the patch. Perhaps the monitor should be changed to avoid printing so many useless control characters, then we'd hit the limit less often... Stefan, didn't you plan to do something like this? Or was it unrelated? Kevin
Re: [Qemu-devel] [libvirt] Configure virtio-scsi options via libvirt
On 05/07/2014 01:13 AM, Mike Perez wrote: Hi everyone, I would like be able to configure virtio-scsi options num_queues, max_sectors, and cmd_per_lun via libvirt. Are there any plans to have this support? num_queues is supported for virtio-scsi controllers since libvirt 1.0.5: controller type='scsi' index='0' model='virtio-scsi' driver queues='8'/ /controller http://libvirt.org/git/?p=libvirt.git;a=commit;h=d4bf0a9 Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] block: Fix bdrv_is_allocated() for short backing files
Am 06.05.2014 um 21:53 hat Max Reitz geschrieben: On 06.05.2014 15:30, Kevin Wolf wrote: bdrv_is_allocated() shouldn't return true for sectors that are unallocated, but after the end of a short backing file, even though such sectors are (correctly) marked as containing zeros. Signed-off-by: Kevin Wolf kw...@redhat.com --- block.c | 8 +--- include/block/block.h | 11 +++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c index c90c71a..d3a9906 100644 --- a/block.c +++ b/block.c @@ -3883,6 +3883,10 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, *pnum, pnum); } +if (ret (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) { +ret |= BDRV_BLOCK_ALLOCATED; +} + Shouldn't BDRV_BLOCK_ALLOCATED be set in the !bs-drv-bdrv_co_get_block_status case as well? It should. Thanks, I'll send a v2. if (!(ret BDRV_BLOCK_DATA) !(ret BDRV_BLOCK_ZERO)) { if (bdrv_unallocated_blocks_are_zero(bs)) { ret |= BDRV_BLOCK_ZERO; @@ -3959,9 +3963,7 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, if (ret 0) { return ret; } -return -(ret BDRV_BLOCK_DATA) || -((ret BDRV_BLOCK_ZERO) !bdrv_has_zero_init(bs)); +return (ret BDRV_BLOCK_ALLOCATED); } /* diff --git a/include/block/block.h b/include/block/block.h index 2fda81c..ad4c7e8 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -116,6 +116,8 @@ typedef enum { /* BDRV_BLOCK_DATA: data is read from bs-file or another file * BDRV_BLOCK_ZERO: sectors read as zero * BDRV_BLOCK_OFFSET_VALID: sector stored in bs-file as raw data + * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this + * layer (as opposed to the backing file) I guess this is above BDRV_BLOCK_RAW (albeit having a greater value) because it is not only used internally? (to pick up on the topic of OCD :-P) Because it felt right. This may or may not be equivalent. Kevin
[Qemu-devel] [PATCH v2] block: Fix bdrv_is_allocated() for short backing files
bdrv_is_allocated() shouldn't return true for sectors that are unallocated, but after the end of a short backing file, even though such sectors are (correctly) marked as containing zeros. Signed-off-by: Kevin Wolf kw...@redhat.com --- v2: - Set BDRV_BLOCK_ALLOCATED for !drv-bdrv_co_get_block_status (Max) block.c | 10 ++ include/block/block.h | 11 +++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/block.c b/block.c index c90c71a..65e8191 100644 --- a/block.c +++ b/block.c @@ -3864,7 +3864,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, if (!bs-drv-bdrv_co_get_block_status) { *pnum = nb_sectors; -ret = BDRV_BLOCK_DATA; +ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED; if (bs-drv-protocol_name) { ret |= BDRV_BLOCK_OFFSET_VALID | (sector_num * BDRV_SECTOR_SIZE); } @@ -3883,6 +3883,10 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, *pnum, pnum); } +if (ret (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) { +ret |= BDRV_BLOCK_ALLOCATED; +} + if (!(ret BDRV_BLOCK_DATA) !(ret BDRV_BLOCK_ZERO)) { if (bdrv_unallocated_blocks_are_zero(bs)) { ret |= BDRV_BLOCK_ZERO; @@ -3959,9 +3963,7 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, if (ret 0) { return ret; } -return -(ret BDRV_BLOCK_DATA) || -((ret BDRV_BLOCK_ZERO) !bdrv_has_zero_init(bs)); +return (ret BDRV_BLOCK_ALLOCATED); } /* diff --git a/include/block/block.h b/include/block/block.h index 2fda81c..ad4c7e8 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -116,6 +116,8 @@ typedef enum { /* BDRV_BLOCK_DATA: data is read from bs-file or another file * BDRV_BLOCK_ZERO: sectors read as zero * BDRV_BLOCK_OFFSET_VALID: sector stored in bs-file as raw data + * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this + * layer (as opposed to the backing file) * BDRV_BLOCK_RAW: used internally to indicate that the request * was answered by the raw driver and that one * should look in bs-file directly. @@ -137,10 +139,11 @@ typedef enum { * ftf not allocated or unknown offset, read as zero * fff not allocated or unknown offset, read from backing_hd */ -#define BDRV_BLOCK_DATA 1 -#define BDRV_BLOCK_ZERO 2 -#define BDRV_BLOCK_OFFSET_VALID 4 -#define BDRV_BLOCK_RAW 8 +#define BDRV_BLOCK_DATA 0x01 +#define BDRV_BLOCK_ZERO 0x02 +#define BDRV_BLOCK_OFFSET_VALID 0x04 +#define BDRV_BLOCK_RAW 0x08 +#define BDRV_BLOCK_ALLOCATED0x10 #define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK typedef enum { -- 1.8.3.1
Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
On Tue, May 06, 2014 at 12:19:10PM +0200, Kevin Wolf wrote: The immediately visible effect of this patch is that it fixes committing a temporary snapshot to its backing file. Previously, it would fail with a permission denied error because bdrv_inherited_flags() forced the backing file to be read-only, ignoring the r/w reopen of bdrv_commit(). The bigger problem this releaved is that the original open flags must actually only be applied to the temporary snapshot, and the original image file must be treated as a backing file of the temporary snapshot and get the right flags for that. Reported-by: Jan Kiszka jan.kis...@web.de Signed-off-by: Kevin Wolf kw...@redhat.com --- block.c| 34 +++--- include/block/block.h | 2 +- tests/qemu-iotests/051 | 4 tests/qemu-iotests/051.out | 10 ++ 4 files changed, 34 insertions(+), 16 deletions(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH v3] glib: fix g_poll early timeout on windows
Hi, Hm, but (int)res expression is not unsigned, it's signed. I've also had this warning, but with this expression: (res WAIT_OBJECT_0), that's why I put (int) there. Could it be that for some reason your compiler treats int and unsigned int, that would be really strange though... On 05/07/2014 12:02 PM, Stefan Hajnoczi wrote: On Fri, Apr 18, 2014 at 08:24:03PM +0400, Stanislav Vorobiov wrote: Please fix the following compiler warning with gcc 4.8.2: +} else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) || + ((int)res WAIT_OBJECT_0) || + (res = (WAIT_OBJECT_0 + nhandles))) { +break; +} util/oslib-win32.c: In function 'g_poll_fixed': util/oslib-win32.c:324:21: warning: comparison of unsigned expression 0 is always false [-Wtype-limits] ((int)res WAIT_OBJECT_0) || ^
[Qemu-devel] [PULL 1/3] s390x/helper: Fixed real-to-absolute address translation
From: Thomas Huth th...@linux.vnet.ibm.com The real-to-absolute address translation in mmu_translate() was missing the second part for translating the page at the prefix address back to the 0 page. And while we're at it, also moved the code into a separate helper function since this might come in handy for other parts of the code, too. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com Reviewed-by: Alexander Graf ag...@suse.de Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- target-s390x/helper.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/target-s390x/helper.c b/target-s390x/helper.c index aa628b8..ddf268e 100644 --- a/target-s390x/helper.c +++ b/target-s390x/helper.c @@ -170,6 +170,20 @@ static void trigger_page_fault(CPUS390XState *env, target_ulong vaddr, trigger_pgm_exception(env, type, ilen); } +/** + * Translate real address to absolute (= physical) + * address by taking care of the prefix mapping. + */ +static target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr) +{ +if (raddr 0x2000) { +return raddr + env-psa;/* Map the lowcore. */ +} else if (raddr = env-psa raddr env-psa + 0x2000) { +return raddr - env-psa;/* Map the 0 page. */ +} +return raddr; +} + static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, uint64_t asc, uint64_t asce, int level, target_ulong *raddr, int *flags, int rw) @@ -363,9 +377,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc, out: /* Convert real address - absolute address */ -if (*raddr 0x2000) { -*raddr = *raddr + env-psa; -} +*raddr = mmu_real2abs(env, *raddr); if (*raddr = ram_size) { sk = env-storage_keys[*raddr / TARGET_PAGE_SIZE]; -- 1.7.9.5
[Qemu-devel] [PULL 0/3] s390 fixes
The following changes since commit 9898370497da3f18e0c9555b65c858eabc78ab50: Merge remote-tracking branch 'remotes/kraxel/tags/pull-smbios-2' into staging (2014-05-06 12:23:05 +0100) are available in the git repository at: https://github.com/cohuck/qemu.git tags/s390x-20140507 for you to fetch changes up to 56bf1a8e90e76701428885656743515af53f19ef: s390x/css: Don't save orb in subchannel. (2014-05-07 10:17:35 +0200) Some improvements for s390. Two patches deal with address translation, one fixes a problem in the channel subsystem code. Cornelia Huck (1): s390x/css: Don't save orb in subchannel. Thomas Huth (2): s390x/helper: Fixed real-to-absolute address translation s390x/helper: Added format control bit to MMU translation hw/s390x/css.c| 21 +--- hw/s390x/css.h|1 - hw/s390x/virtio-ccw.c |1 - target-s390x/cpu.h|4 +++ target-s390x/helper.c | 88 + 5 files changed, 79 insertions(+), 36 deletions(-) -- 1.7.9.5
[Qemu-devel] [PULL 3/3] s390x/css: Don't save orb in subchannel.
Current css code saves the operation request block (orb) in the subchannel structure for later consumption by the start function handler. This might make sense for asynchronous execution of the start function (which qemu doesn't support), but not in our case; it would even be wrong since orb contains a reference to a local variable in the base ssch handler. Let's just pass the orb through the start function call chain for ssch; for rsch, we can pass NULL as the backend function does not use any information passed via the orb there. Acked-by: Christian Borntraeger borntrae...@de.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/css.c| 21 - hw/s390x/css.h|1 - hw/s390x/virtio-ccw.c |1 - 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 7074d2b..122cc7e 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -140,7 +140,6 @@ static void sch_handle_clear_func(SubchDev *sch) s-flags = ~SCSW_FLAGS_MASK_PNO; /* We always 'attempt to issue the clear signal', and we always succeed. */ -sch-orb = NULL; sch-channel_prog = 0x0; sch-last_cmd_valid = false; s-ctrl = ~SCSW_ACTL_CLEAR_PEND; @@ -163,7 +162,6 @@ static void sch_handle_halt_func(SubchDev *sch) path = 0x80; /* We always 'attempt to issue the halt signal', and we always succeed. */ -sch-orb = NULL; sch-channel_prog = 0x0; sch-last_cmd_valid = false; s-ctrl = ~SCSW_ACTL_HALT_PEND; @@ -317,12 +315,11 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr) return ret; } -static void sch_handle_start_func(SubchDev *sch) +static void sch_handle_start_func(SubchDev *sch, ORB *orb) { PMCW *p = sch-curr_status.pmcw; SCSW *s = sch-curr_status.scsw; -ORB *orb = sch-orb; int path; int ret; @@ -331,6 +328,7 @@ static void sch_handle_start_func(SubchDev *sch) if (!(s-ctrl SCSW_ACTL_SUSP)) { /* Look at the orb and try to execute the channel program. */ +assert(orb != NULL); /* resume does not pass an orb */ p-intparm = orb-intparm; if (!(orb-lpm path)) { /* Generate a deferred cc 3 condition. */ @@ -406,7 +404,7 @@ static void sch_handle_start_func(SubchDev *sch) * read/writes) asynchronous later on if we start supporting more than * our current very simple devices. */ -static void do_subchannel_work(SubchDev *sch) +static void do_subchannel_work(SubchDev *sch, ORB *orb) { SCSW *s = sch-curr_status.scsw; @@ -416,7 +414,7 @@ static void do_subchannel_work(SubchDev *sch) } else if (s-ctrl SCSW_FCTL_HALT_FUNC) { sch_handle_halt_func(sch); } else if (s-ctrl SCSW_FCTL_START_FUNC) { -sch_handle_start_func(sch); +sch_handle_start_func(sch, orb); } else { /* Cannot happen. */ return; @@ -594,7 +592,6 @@ int css_do_xsch(SubchDev *sch) SCSW_ACTL_SUSP); sch-channel_prog = 0x0; sch-last_cmd_valid = false; -sch-orb = NULL; s-dstat = 0; s-cstat = 0; ret = 0; @@ -618,7 +615,7 @@ int css_do_csch(SubchDev *sch) s-ctrl = ~(SCSW_CTRL_MASK_FCTL | SCSW_CTRL_MASK_ACTL); s-ctrl |= SCSW_FCTL_CLEAR_FUNC | SCSW_FCTL_CLEAR_FUNC; -do_subchannel_work(sch); +do_subchannel_work(sch, NULL); ret = 0; out: @@ -659,7 +656,7 @@ int css_do_hsch(SubchDev *sch) } s-ctrl |= SCSW_ACTL_HALT_PEND; -do_subchannel_work(sch); +do_subchannel_work(sch, NULL); ret = 0; out: @@ -721,13 +718,12 @@ int css_do_ssch(SubchDev *sch, ORB *orb) if (channel_subsys-chnmon_active) { css_update_chnmon(sch); } -sch-orb = orb; sch-channel_prog = orb-cpa; /* Trigger the start function. */ s-ctrl |= (SCSW_FCTL_START_FUNC | SCSW_ACTL_START_PEND); s-flags = ~SCSW_FLAGS_MASK_PNO; -do_subchannel_work(sch); +do_subchannel_work(sch, orb); ret = 0; out: @@ -957,7 +953,7 @@ int css_do_rsch(SubchDev *sch) } s-ctrl |= SCSW_ACTL_RESUME_PEND; -do_subchannel_work(sch); +do_subchannel_work(sch, NULL); ret = 0; out: @@ -1267,7 +1263,6 @@ void css_reset_sch(SubchDev *sch) sch-channel_prog = 0x0; sch-last_cmd_valid = false; -sch-orb = NULL; sch-thinint_active = false; } diff --git a/hw/s390x/css.h b/hw/s390x/css.h index e9b4405..220169e 100644 --- a/hw/s390x/css.h +++ b/hw/s390x/css.h @@ -76,7 +76,6 @@ struct SubchDev { hwaddr channel_prog; CCW1 last_cmd; bool last_cmd_valid; -ORB *orb; bool thinint_active; /* transport-provided data: */ int (*ccw_cb) (SubchDev *, CCW1); diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 2bf0af8..1cb4e2c 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -559,7 +559,6 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) /* Initialize subchannel structure. */
[Qemu-devel] [PULL 2/3] s390x/helper: Added format control bit to MMU translation
From: Thomas Huth th...@linux.vnet.ibm.com With the EDAT-1 facility, the MMU translation can stop at the segment table already, pointing to a 1 MB block. And while we're at it, move the page table entry handling to a separate function, too, as suggested by Alexander Graf. Acked-by: Alexander Graf ag...@suse.de Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- target-s390x/cpu.h|4 +++ target-s390x/helper.c | 70 - 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h index 41903a9..aad277a 100644 --- a/target-s390x/cpu.h +++ b/target-s390x/cpu.h @@ -270,6 +270,9 @@ typedef struct CPUS390XState { #define FLAG_MASK_64(PSW_MASK_64 32) #define FLAG_MASK_320x1000 +/* Control register 0 bits */ +#define CR0_EDAT0x0080ULL + static inline int cpu_mmu_index (CPUS390XState *env) { if (env-psw.mask PSW_MASK_PSTATE) { @@ -927,6 +930,7 @@ struct sysib_322 { #define _REGION_ENTRY_LENGTH0x03 /* region third length */ #define _SEGMENT_ENTRY_ORIGIN ~0x7ffULL /* segment table origin */ +#define _SEGMENT_ENTRY_FC 0x400 /* format control */ #define _SEGMENT_ENTRY_RO 0x200 /* page protection bit */ #define _SEGMENT_ENTRY_INV 0x20 /* invalid segment table entry */ diff --git a/target-s390x/helper.c b/target-s390x/helper.c index ddf268e..7c76fc1 100644 --- a/target-s390x/helper.c +++ b/target-s390x/helper.c @@ -184,6 +184,50 @@ static target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr) return raddr; } +/* Decode page table entry (normal 4KB page) */ +static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr, + uint64_t asc, uint64_t asce, + target_ulong *raddr, int *flags, int rw) +{ +if (asce _PAGE_INVALID) { +DPRINTF(%s: PTE=0x% PRIx64 invalid\n, __func__, asce); +trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw); +return -1; +} + +if (asce _PAGE_RO) { +*flags = ~PAGE_WRITE; +} + +*raddr = asce _ASCE_ORIGIN; + +PTE_DPRINTF(%s: PTE=0x% PRIx64 \n, __func__, asce); + +return 0; +} + +/* Decode EDAT1 segment frame absolute address (1MB page) */ +static int mmu_translate_sfaa(CPUS390XState *env, target_ulong vaddr, + uint64_t asc, uint64_t asce, target_ulong *raddr, + int *flags, int rw) +{ +if (asce _SEGMENT_ENTRY_INV) { +DPRINTF(%s: SEG=0x% PRIx64 invalid\n, __func__, asce); +trigger_page_fault(env, vaddr, PGM_SEGMENT_TRANS, asc, rw); +return -1; +} + +if (asce _SEGMENT_ENTRY_RO) { +*flags = ~PAGE_WRITE; +} + +*raddr = (asce 0xfff0ULL) | (vaddr 0xf); + +PTE_DPRINTF(%s: SEG=0x% PRIx64 \n, __func__, asce); + +return 0; +} + static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, uint64_t asc, uint64_t asce, int level, target_ulong *raddr, int *flags, int rw) @@ -243,28 +287,18 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr, PTE_DPRINTF(%s: 0x% PRIx64 + 0x% PRIx64 = 0x%016 PRIx64 \n, __func__, origin, offs, new_asce); -if (level != _ASCE_TYPE_SEGMENT) { +if (level == _ASCE_TYPE_SEGMENT) { +/* 4KB page */ +return mmu_translate_pte(env, vaddr, asc, new_asce, raddr, flags, rw); +} else if (level - 4 == _ASCE_TYPE_SEGMENT + (new_asce _SEGMENT_ENTRY_FC) (env-cregs[0] CR0_EDAT)) { +/* 1MB page */ +return mmu_translate_sfaa(env, vaddr, asc, new_asce, raddr, flags, rw); +} else { /* yet another region */ return mmu_translate_asce(env, vaddr, asc, new_asce, level - 4, raddr, flags, rw); } - -/* PTE */ -if (new_asce _PAGE_INVALID) { -DPRINTF(%s: PTE=0x% PRIx64 invalid\n, __func__, new_asce); -trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw); -return -1; -} - -if (new_asce _PAGE_RO) { -*flags = ~PAGE_WRITE; -} - -*raddr = new_asce _ASCE_ORIGIN; - -PTE_DPRINTF(%s: PTE=0x% PRIx64 \n, __func__, new_asce); - -return 0; } static int mmu_translate_asc(CPUS390XState *env, target_ulong vaddr, -- 1.7.9.5
Re: [Qemu-devel] [PATCH v3] glib: fix g_poll early timeout on windows
On 7 May 2014, at 09:36, Stanislav Vorobiov wrote: Hi, Hm, but (int)res expression is not unsigned, it's signed. I've also had this warning, but with this expression: (res WAIT_OBJECT_0), that's why I put (int) there. Could it be that for some reason your compiler treats int and unsigned int, that would be really strange though... I suspect the problem is that WAIT_OBJECT_0 is defined as an unsigned long: #define WAIT_OBJECT_0 ((STATUS_WAIT_0 ) + 0 ) #define STATUS_WAIT_0 ((DWORD)0xL) So IIRC under the 'usual conversions', and int compared with it will be cast to an unsigned long too. I think you want to cast WAIT_OBJECT_0 to a long or similar. Alex On 05/07/2014 12:02 PM, Stefan Hajnoczi wrote: On Fri, Apr 18, 2014 at 08:24:03PM +0400, Stanislav Vorobiov wrote: Please fix the following compiler warning with gcc 4.8.2: +} else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) || + ((int)res WAIT_OBJECT_0) || + (res = (WAIT_OBJECT_0 + nhandles))) { +break; +} util/oslib-win32.c: In function 'g_poll_fixed': util/oslib-win32.c:324:21: warning: comparison of unsigned expression 0 is always false [-Wtype-limits] ((int)res WAIT_OBJECT_0) || ^ -- Alex Bligh
Re: [Qemu-devel] [PATCH v1 01/22] target-arm: A64: Add friendly logging of PSTATE A and I flags
On 6 May 2014 07:08, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- target-arm/translate-a64.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index b62db4d..4f8246f 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -137,8 +137,10 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f, cpu_fprintf(f, ); } } -cpu_fprintf(f, PSTATE=%08x (flags %c%c%c%c)\n, +cpu_fprintf(f, PSTATE=%08x (flags %c%c%c%c%c%c)\n, psr, +psr PSTATE_A ? 'A' : '-', +psr PSTATE_I ? 'I' : '-', psr PSTATE_N ? 'N' : '-', psr PSTATE_Z ? 'Z' : '-', psr PSTATE_C ? 'C' : '-', Why A and I ? In particular in QEMU the A bit is always zero because we don't do System Errors (aka asynchronous external aborts), and it's weird to show I but not F. The idea of splitting out NZCV is really that (as with the A32/T32 state dump) they're the most useful bits for immediately figuring out code flow); anything else you can fish out of the hex value by hand if you really need it. I think you can make a case for decode only a small set of key bits or for completely decode the whole register, but I'm not sure adding only two more bits makes sense. thanks -- PMM
Re: [Qemu-devel] [PATCH] vl.c: remove init_clocks call from main
On Tue, May 06, 2014 at 04:59:53PM +0400, Kirill Batuzov wrote: Clocks are initialized in qemu_init_main_loop. They are not needed before it. Initializing them twice is not only unnecessary but is harmful: it results in memory leak and potentially can lead to a situation where different parts of QEMU use different sets of timers. To avoid it remove init_clocks call from main and add an assertion to qemu_clock_init that corresponding clock has not been initialized yet. Signed-off-by: Kirill Batuzov batuz...@ispras.ru --- qemu-timer.c |3 +++ vl.c |1 - 2 files changed, 3 insertions(+), 1 deletion(-) The init_clocks call was added to qemu_init_main_loop in commit 172061a0a0d98c974ea8d5ed715195237bc44225. init_clocks was made idempotent in prior commit 744ca8e3754e6808c6b5331d287adc533fca0ad3. Back then it was not possible to remove init_clocks call from main because rtc_clock variable was of type QEMUClock * and was used in option processing. So clocks needed to be initialized before command line options could be processed. Commit 7bf8fbde449600926370f744b2b3d9cc819ca74f removed idempotence property from init_clocks. Commit 884f17c235095af99b92dd8cd10bb824a5b0f777 removed the need to call init_clocks from main, but did not remove the call. Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH v2] vmdk: Optimize cluster allocation
On Wed, 05/07 10:20, Kevin Wolf wrote: Am 07.05.2014 um 03:45 hat Fam Zheng geschrieben: On Tue, 05/06 10:32, Fam Zheng wrote: On mounted NFS filesystem, ftruncate is much much slower than doing a zero write. Changing this significantly speeds up cluster allocation. Comparing by converting a cirros image (296M) to VMDK on an NFS mount point, over 1Gbe LAN: $ time qemu-img convert cirros-0.3.1.img /mnt/a.raw -O vmdk Before: real0m26.464s user0m0.133s sys 0m0.527s After: real0m2.120s user0m0.080s sys 0m0.197s Signed-off-by: Fam Zheng f...@redhat.com --- V2: Fix cluster_offset check. (Kevin) Signed-off-by: Fam Zheng f...@redhat.com --- block/vmdk.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 06a1f9f..98d2d56 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1037,6 +1037,7 @@ static int get_cluster_offset(BlockDriverState *bs, int min_index, i, j; uint32_t min_count, *l2_table; bool zeroed = false; +int64_t ret; if (m_data) { m_data-valid = 0; @@ -1110,12 +,20 @@ static int get_cluster_offset(BlockDriverState *bs, } /* Avoid the L2 tables update for the images that have snapshots. */ -*cluster_offset = bdrv_getlength(extent-file); +ret = bdrv_getlength(extent-file); +if (ret 0 || +ret ((extent-cluster_sectors BDRV_SECTOR_BITS) - 1)) { +return VMDK_ERROR; +} +*cluster_offset = ret; if (!extent-compressed) { -bdrv_truncate( -extent-file, -*cluster_offset + (extent-cluster_sectors 9) -); +ret = bdrv_write_zeroes(extent-file, +*cluster_offset BDRV_SECTOR_BITS, +extent-cluster_sectors, +0); Hi Stefan, By considering a bdrv_write_zeroes as a pre-write, it in general doubles the write for the whole image, so it's not a good solution. A better way would be removing the bdrv_truncate and require the caller to do full cluster write (with a bounce buffer if necessary). Doesn't get_whole_cluster() already ensure that you already write a full cluster to the image file? That one is actually called get_backing_cluster(), if you look at the code it has. :) However, it might be better to not use bdrv_getlength() each time you need a new cluster, but instead use a field in VmdkExtent to keep the next free cluster offset (which is rounded up in vmdk_open). Yes, indeed. We should do that. Thanks, Fam
Re: [Qemu-devel] [PATCH v1 16/22] target-arm: A64: Forbid ERET to unimplemented ELs
On 6 May 2014 07:08, Edgar E. Iglesias edgar.igles...@gmail.com wrote: From: Edgar E. Iglesias edgar.igles...@xilinx.com Check for EL2 support before returning to it. Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- target-arm/op_helper.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index 770c776..f1ae05e 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -411,12 +411,10 @@ void HELPER(exception_return)(CPUARMState *env) env-regs[15] = env-elr_el[ELR_EL_IDX(1)] ~0x1; } else { new_el = extract32(spsr, 2, 2); -if (new_el cur_el) { +if (new_el cur_el +|| (new_el == 2 !arm_feature(env, ARM_FEATURE_EL2))) { /* Disallow returns to higher ELs than the current one. */ -goto illegal_return; -} -if (new_el 1) { -/* Return to unimplemented EL */ +/* Disallow returns to unimplemented ELs. */ Merge the comments rather than having two one-liners one after the other, please. /* Disallow return to an EL which is unimplemented or higher * than the current one. */ thanks -- PMM
Re: [Qemu-devel] [PATCH] target-i386: fix handling of ZF in btx instructions
On 6 May 2014 14:33, Dmitry Poletaev poletaev-q...@yandex.ru wrote: Eflags for bt/bts/btr/btc instructions compute as for shift(SAR) instructions. According to Intel A2 manual, for btx instructions zf is unaffected under any condition, but for SAR group, when evaluate eflags, we compute zf. Isn't this the same bug that's fixed by Richard Henderson's patch from last month? http://patchwork.ozlabs.org/patch/337929/ thanks -- PMM
Re: [Qemu-devel] [PATCH for-2.0 0/7] SMBus and tmp105 fixes
On Wed, May 07, 2014 at 10:03:58AM +0200, Paolo Bonzini wrote: Il 02/04/2014 17:55, Michael S. Tsirkin ha scritto: smbus: allow returning an error from reads smbus: return -1 if nothing found at the given address pm_smbus: correctly report unclaimed cycles I've reviewed these and they look sane and safe for 2.0. mst, could you have a second look as PC maintainer and take them? I'd rather delay to 2.1. It's not a regression is it? Ping? Paolo Applied, thanks!
Re: [Qemu-devel] [PATCH v6 0/2] apic: bump emulated lapic version to 0x14
On Tue, May 06, 2014 at 11:17:23AM -0400, Gabriel L. Somlo wrote: This patch set changes the software-emulated local apic version to 0x14 starting with pc machine types 2.1 and newer. This should be particularly helpful when running OS X guests with TCG, since XNU appears to have a hardcoded requirement that lapic version = 0x14. Changelog: v6: - rebased to apply cleanly (no fuzz) against latest qemu git - opportunity to practice dealing with Acked-by and Reviewed-by :) v5: convert lapic version to uint8_t (only 8 bits dedicated to implementation version in the apic version register, according to the Intel spec). v4: - split into a two-patch series with cover letter - 1/2: - introduces empty 2.0 compat_props - depends on 3458b2b075f92f163ccb9a1f24733eb5705947f0 to add 2.1 machine type and move aliases (now already upstream, but not at the time v4 went out :) - 2/2: - adds lapic version as a machine property defaulting to 0x14 - set to 0x11 in compat_props for machines 2.0 and older v3 and older: single patch, lapic version is global, no cover letter Thanks again, Gabriel Applied, thanks for your patience. PS. Funny, now that I'm getting close to having figured out the qemu contributor netiquette, I'm just about done submitting all the changes I set out to contribute... :) Gabriel L. Somlo (2): pc: add compat_props placeholder for 2.0 machine type pic: use emulated lapic version 0x14 on pc machines = 2.1 hw/i386/pc_piix.c | 4 hw/i386/pc_q35.c| 4 hw/intc/apic.c | 2 +- hw/intc/apic_common.c | 1 + include/hw/i386/apic_internal.h | 1 + include/hw/i386/pc.h| 12 6 files changed, 23 insertions(+), 1 deletion(-) -- 1.9.0
Re: [Qemu-devel] [PATCH v2] vmdk: Optimize cluster allocation
Am 07.05.2014 um 10:57 hat Fam Zheng geschrieben: On Wed, 05/07 10:20, Kevin Wolf wrote: Am 07.05.2014 um 03:45 hat Fam Zheng geschrieben: On Tue, 05/06 10:32, Fam Zheng wrote: On mounted NFS filesystem, ftruncate is much much slower than doing a zero write. Changing this significantly speeds up cluster allocation. Comparing by converting a cirros image (296M) to VMDK on an NFS mount point, over 1Gbe LAN: $ time qemu-img convert cirros-0.3.1.img /mnt/a.raw -O vmdk Before: real0m26.464s user0m0.133s sys 0m0.527s After: real0m2.120s user0m0.080s sys 0m0.197s Signed-off-by: Fam Zheng f...@redhat.com --- V2: Fix cluster_offset check. (Kevin) Signed-off-by: Fam Zheng f...@redhat.com --- block/vmdk.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 06a1f9f..98d2d56 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1037,6 +1037,7 @@ static int get_cluster_offset(BlockDriverState *bs, int min_index, i, j; uint32_t min_count, *l2_table; bool zeroed = false; +int64_t ret; if (m_data) { m_data-valid = 0; @@ -1110,12 +,20 @@ static int get_cluster_offset(BlockDriverState *bs, } /* Avoid the L2 tables update for the images that have snapshots. */ -*cluster_offset = bdrv_getlength(extent-file); +ret = bdrv_getlength(extent-file); +if (ret 0 || +ret ((extent-cluster_sectors BDRV_SECTOR_BITS) - 1)) { +return VMDK_ERROR; +} +*cluster_offset = ret; if (!extent-compressed) { -bdrv_truncate( -extent-file, -*cluster_offset + (extent-cluster_sectors 9) -); +ret = bdrv_write_zeroes(extent-file, +*cluster_offset BDRV_SECTOR_BITS, +extent-cluster_sectors, +0); Hi Stefan, By considering a bdrv_write_zeroes as a pre-write, it in general doubles the write for the whole image, so it's not a good solution. A better way would be removing the bdrv_truncate and require the caller to do full cluster write (with a bounce buffer if necessary). Doesn't get_whole_cluster() already ensure that you already write a full cluster to the image file? That one is actually called get_backing_cluster(), if you look at the code it has. :) Right, it doesn't do anything without a backing file. This is different from qcow2, whose mechanism I assumed without reading the code in detail. :-) I think it would make sense to rewrite get_whole_cluster() to write the cluster for both image with a backing file and standalone images; just that without a backing file it would use memset() to fill the buffer instead of bdrv_read(). Not sure how easy it would be, but it might be an opportunity to also change it to write only those parts of the cluster that aren't written to anyway by the cluster. Kevin
Re: [Qemu-devel] [PATCH v3] glib: fix g_poll early timeout on windows
Hi, Yes it's probably the cause, thanks. On 05/07/2014 12:49 PM, Alex Bligh wrote: On 7 May 2014, at 09:36, Stanislav Vorobiov wrote: Hi, Hm, but (int)res expression is not unsigned, it's signed. I've also had this warning, but with this expression: (res WAIT_OBJECT_0), that's why I put (int) there. Could it be that for some reason your compiler treats int and unsigned int, that would be really strange though... I suspect the problem is that WAIT_OBJECT_0 is defined as an unsigned long: #define WAIT_OBJECT_0 ((STATUS_WAIT_0 ) + 0 ) #define STATUS_WAIT_0 ((DWORD)0xL) So IIRC under the 'usual conversions', and int compared with it will be cast to an unsigned long too. I think you want to cast WAIT_OBJECT_0 to a long or similar. Alex On 05/07/2014 12:02 PM, Stefan Hajnoczi wrote: On Fri, Apr 18, 2014 at 08:24:03PM +0400, Stanislav Vorobiov wrote: Please fix the following compiler warning with gcc 4.8.2: +} else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) || + ((int)res WAIT_OBJECT_0) || + (res = (WAIT_OBJECT_0 + nhandles))) { +break; +} util/oslib-win32.c: In function 'g_poll_fixed': util/oslib-win32.c:324:21: warning: comparison of unsigned expression 0 is always false [-Wtype-limits] ((int)res WAIT_OBJECT_0) || ^
[Qemu-devel] [PATCH v4] glib: fix g_poll early timeout on windows
From: Sangho Park sangho1206.p...@samsung.com g_poll has a problem on Windows when using timeouts 10ms, in glib/gpoll.c: /* If not, and we have a significant timeout, poll again with * timeout then. Note that this will return indication for only * one event, or only for messages. We ignore timeouts less than * ten milliseconds as they are mostly pointless on Windows, the * MsgWaitForMultipleObjectsEx() call will timeout right away * anyway. */ if (retval == 0 (timeout == INFINITE || timeout = 10)) retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout); so whenever g_poll is called with timeout 10ms it does a quick poll instead of wait, this causes significant performance degradation of QEMU, thus we should use WaitForMultipleObjectsEx directly Signed-off-by: Stanislav Vorobiov s.vorob...@samsung.com --- include/glib-compat.h | 19 + include/qemu-common.h | 12 -- util/oslib-win32.c| 112 + 3 files changed, 131 insertions(+), 12 deletions(-) diff --git a/include/glib-compat.h b/include/glib-compat.h index 8aa77af..1280fb2 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -24,4 +24,23 @@ static inline guint g_timeout_add_seconds(guint interval, GSourceFunc function, } #endif +#ifdef _WIN32 +/* + * g_poll has a problem on Windows when using + * timeouts 10ms, so use wrapper. + */ +#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout) +gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout); +#elif !GLIB_CHECK_VERSION(2, 20, 0) +/* + * Glib before 2.20.0 doesn't implement g_poll, so wrap it to compile properly + * on older systems. + */ +static inline gint g_poll(GPollFD *fds, guint nfds, gint timeout) +{ +GMainContext *ctx = g_main_context_default(); +return g_main_context_get_poll_func(ctx)(fds, nfds, timeout); +} +#endif + #endif diff --git a/include/qemu-common.h b/include/qemu-common.h index a998e8d..3f3fd60 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -124,18 +124,6 @@ int qemu_main(int argc, char **argv, char **envp); void qemu_get_timedate(struct tm *tm, int offset); int qemu_timedate_diff(struct tm *tm); -#if !GLIB_CHECK_VERSION(2, 20, 0) -/* - * Glib before 2.20.0 doesn't implement g_poll, so wrap it to compile properly - * on older systems. - */ -static inline gint g_poll(GPollFD *fds, guint nfds, gint timeout) -{ -GMainContext *ctx = g_main_context_default(); -return g_main_context_get_poll_func(ctx)(fds, nfds, timeout); -} -#endif - /** * is_help_option: * @s: string to test diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 93f7d35..69552f7 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -238,3 +238,115 @@ char *qemu_get_exec_dir(void) { return g_strdup(exec_dir); } + +/* + * g_poll has a problem on Windows when using + * timeouts 10ms, in glib/gpoll.c: + * + * // If not, and we have a significant timeout, poll again with + * // timeout then. Note that this will return indication for only + * // one event, or only for messages. We ignore timeouts less than + * // ten milliseconds as they are mostly pointless on Windows, the + * // MsgWaitForMultipleObjectsEx() call will timeout right away + * // anyway. + * + * if (retval == 0 (timeout == INFINITE || timeout = 10)) + * retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout); + * + * So whenever g_poll is called with timeout 10ms it does + * a quick poll instead of wait, this causes significant performance + * degradation of QEMU, thus we should use WaitForMultipleObjectsEx + * directly + */ +gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout) +{ +guint i; +HANDLE handles[MAXIMUM_WAIT_OBJECTS]; +gint nhandles = 0; +int num_completed = 0; + +for (i = 0; i nfds; i++) { +gint j; + +if (fds[i].fd = 0) { +continue; +} + +/* don't add same handle several times + */ +for (j = 0; j nhandles; j++) { +if (handles[j] == (HANDLE)fds[i].fd) { +break; +} +} + +if (j == nhandles) { +if (nhandles == MAXIMUM_WAIT_OBJECTS) { +fprintf(stderr, Too many handles to wait for!\n); +break; +} else { +handles[nhandles++] = (HANDLE)fds[i].fd; +} +} +} + +for (i = 0; i nfds; ++i) { +fds[i].revents = 0; +} + +if (timeout == -1) { +timeout = INFINITE; +} + +if (nhandles == 0) { +if (timeout == INFINITE) { +return -1; +} else { +SleepEx(timeout, TRUE); +return 0; +} +} + +while (1) { +DWORD res; +gint j; + +res = WaitForMultipleObjectsEx(nhandles, handles, FALSE, +timeout, TRUE); + +if (res == WAIT_FAILED) { +for (i = 0; i nfds; ++i) { +
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
On 7 May 2014 09:14, Greg Kurz gk...@linux.vnet.ibm.com wrote: On Tue, 6 May 2014 19:37:22 +0100 Peter Maydell peter.mayd...@linaro.org wrote: On 5 May 2014 09:07, Greg Kurz gk...@linux.vnet.ibm.com wrote: POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR special purpose register to decide the endianness to use when entering interrupt handlers. When running a Linux guest, this provides a hint on the endianness used by the kernel. From a QEMU point of view, the information is needed for legacy virtio support and crash dump support as well. Do you care about the case of: * kernel bigendian Yes. FWIW, ppc64 is still widely used in big endian mode we don't want to break. * userspace littleendian (or vice-versa) We don't care about userspace here. We assume that virtio structures are owned by the guest kernel. * guest kernel passes virtio device through to guest userspace Not sure to understand... could you please point me to an example ? Consider PCI passthrough of a virtio device to userspace Linux or to a nested KVM/QEMU instance running inside the outermost KVM. It's a bit of an odd corner case, but we should either accommodate it or definitely say it's not expected to work consistently across architectures I think. thanks -- PMM
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
On 07.05.14 10:14, Greg Kurz wrote: On Tue, 6 May 2014 19:37:22 +0100 Peter Maydell peter.mayd...@linaro.org wrote: On 5 May 2014 09:07, Greg Kurz gk...@linux.vnet.ibm.com wrote: POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR special purpose register to decide the endianness to use when entering interrupt handlers. When running a Linux guest, this provides a hint on the endianness used by the kernel. From a QEMU point of view, the information is needed for legacy virtio support and crash dump support as well. Do you care about the case of: * kernel bigendian Yes. FWIW, ppc64 is still widely used in big endian mode we don't want to break. * userspace littleendian (or vice-versa) We don't care about userspace here. We assume that virtio structures are owned by the guest kernel. * guest kernel passes virtio device through to guest userspace Not sure to understand... could you please point me to an example ? * guest userspace is doing the manipulation of the device Hmm... you mean we would have virtio drivers implemented in the guest userspace ? Does that exist ? Please elaborate. Virtio bypasses the IOMMU by design, so user space drivers don't make sense here :). I don't think we should overengineer hacks for legacy virtio. Alex
Re: [Qemu-devel] [PATCH 00/35] pc: ACPI memory hotplug
Hello Igor, while testing your patchset i came to a very stupid problem. I wanted to test migration and it cames out that the migration works fine after plugging in memory only if i run the target VM without the -daemonize option. If i enable the -daemonize option the target vm tries to read from non readable memory. proc maps shows: 7f9334021000-7f933800 ---p 00:00 0 where it tries to read from. Also the memory layout is different in daemonize mode than in non daemonize mode. Stefan Am 04.04.2014 15:36, schrieb Igor Mammedov: What's new since v7: * Per Andreas' suggestion dropped DIMMBus concept. * Added hotplug binding for bus-less devices * DIMM device is split to backend and frontend. Therefore following command/options were added for supporting it: For memory-ram backend: CLI: -object-add memory-ram, with options: 'id' and 'size' For dimm frontend: option size became readonly, pulling it's size from attached backend added option memdev for specifying backend by 'id' * dropped support for 32 bit guests * failed hotplug action doesn't consume 1 slot anymore * vaious fixes adressing reviewer's comments most of them in ACPI part --- This series allows to hotplug 'arbitrary' DIMM devices specifying size, NUMA node mapping (guest side), slot and address where to map it, at runtime. Due to ACPI limitation there is need to specify a number of possible DIMM devices. For this task -m option was extended to support following format: -m [mem=]RamSize[,slots=N,maxmem=M] To allow memory hotplug user must specify a pair of additional parameters: 'slots' - number of possible increments 'maxmem' - max possible total memory size QEMU is allowed to use, including RamSize. minimal monitor command syntax to hotplug DIMM device: object_add memory-ram,id=memX,size=1G device_add dimm,id=dimmX,memdev=memX DIMM device provides following properties that could be used with device_add / -device to alter default behavior: id- unique string identifying device [mandatory] slot - number in range [0-slots) [optional], if not specified the first free slot is used node - NUMA node id [optional] (default: 0) size - amount of memory to add, readonly derived from backing memdev start - guest's physical address where to plug DIMM [optional], if not specified the first gap in hotplug memory region that fits DIMM is used -device option could be used for adding potentially hotunplugable DIMMs and also for specifying hotplugged DIMMs in migration case. Tested guests: - RHEL 6x64 - Windows 2012DCx64 - Windows 2008DCx64 Known limitations/bugs/TODOs: - hot-remove is not supported, yet - max number of supported DIMM devices 255 (due to ACPI object name limit), could be increased creating several containers and putting DIMMs there. (exercise for future) - e820 table doesn't include DIMM devices added with -device / (or after reboot devices added with device_add) - Windows 2008 remembers DIMM configuration, so if DIMM with other start/size is added into the same slot, it refuses to use it insisting on old mapping. QEMU git tree for testing is available at: https://github.com/imammedo/qemu/commits/memory-hotplug-v8 Example QEMU cmd line: qemu-system-x86_64 -enable-kvm -monitor unix:/tmp/mon,server,nowait \ -m 4096,slots=4,maxmem=8G guest.img PS: Windows guest requires SRAT table for hotplug to work so add an extra option: -numa node to QEMU command line. Igor Mammedov (34): vl: convert -m to QemuOpts object_add: allow completion handler to get canonical path add memdev backend infrastructure vl.c: extend -m option to support options for memory hotplug add pc-{i440fx,q35}-2.1 machine types pc: create custom generic PC machine type qdev: hotplug for buss-less devices qdev: expose DeviceState.hotplugged field as a property dimm: implement dimm device abstraction memory: add memory_region_is_mapped() API dimm: do not allow to set already busy memdev pc: initialize memory hotplug address space pc: exit QEMU if slots 256 pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS pc: add memory hotplug handler to PC_MACHINE dimm: add busy address check and address auto-allocation dimm: add busy slot check and slot auto-allocation acpi: rename cpu_hotplug_defs.h to acpi_defs.h acpi: memory hotplug ACPI hardware implementation trace: add acpi memory hotplug IO region events trace: add DIMM slot address allocation for target-i386 acpi:piix4: make plug/unlug callbacks generic acpi:piix4: add memory hotplug handling pc: ich9 lpc: make it work with global/compat properties acpi:ich9: add memory hotplug handling pc: migrate piix4 ich9 MemHotplugState pc: propagate memory hotplug event to ACPI device
[Qemu-devel] [PATCH net v1 1/4] net: cadence_gem: Fix Tx descriptor update
The local variable desc was being used to read-modify-write the first descriptor (of a multi-desc packet) upon packet completion. desc however continues to be used by the code as the current descriptor. Give this first desc RMW it's own local variable to avoid trampling. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/net/cadence_gem.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index e34b25e..1c36e48 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -917,14 +917,16 @@ static void gem_transmit(GemState *s) /* Last descriptor for this packet; hand the whole thing off */ if (tx_desc_get_last(desc)) { +unsigneddesc_first[2]; + /* Modify the 1st descriptor of this packet to be owned by * the processor. */ cpu_physical_memory_read(s-tx_desc_addr, - (uint8_t *)desc[0], sizeof(desc)); -tx_desc_set_used(desc); + (uint8_t *)desc_first[0], sizeof(desc)); +tx_desc_set_used(desc_first); cpu_physical_memory_write(s-tx_desc_addr, - (uint8_t *)desc[0], sizeof(desc)); + (uint8_t *)desc_first[0], sizeof(desc)); /* Advance the hardare current descriptor past this packet */ if (tx_desc_get_wrap(desc)) { s-tx_desc_addr = s-regs[GEM_TXQBASE]; -- 1.9.2.1.g06c4abd
[Qemu-devel] [PATCH net v1 0/4] Cadence GEM patches
Hi Stefan, Peter, Found a bug in the Cadence GEM. Fixed in P1. Have some follow us trivials as well (P2-4). Regards, Peter Peter Crosthwaite (4): net: cadence_gem: Fix Tx descriptor update net: cadence_gem: Add Tx descriptor fetch printf net: cadence_gem: Fix top comment net: cadence_gem: Comment spelling sweep hw/net/cadence_gem.c | 25 +++-- 1 file changed, 15 insertions(+), 10 deletions(-) -- 1.9.2.1.g06c4abd
Re: [Qemu-devel] [PATCH 5/7] monitor: Add set_link arguments completion.
On Sun, Apr 27, 2014 at 05:00:06PM +0100, Hani Benhabiles wrote: Make it possible to query all net clients without specifying an ID when calling qemu_find_net_clients_except(). This also adds the add_completion_option() function which is to be used for other commands completions as well. Signed-off-by: Hani Benhabiles h...@linux.com --- hmp-commands.hx | 1 + hmp.h | 1 + monitor.c | 34 ++ net/net.c | 2 +- 4 files changed, 37 insertions(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
[Qemu-devel] [PATCH net v1 2/4] net: cadence_gem: Add Tx descriptor fetch printf
Add a debug printf for TX descriptor fetching. This helpful to anyone needing to debug TX ring buffer traversal. Its also now consistent with the RX code which has a similar printf. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/net/cadence_gem.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index 1c36e48..f999886 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -886,6 +886,8 @@ static void gem_transmit(GemState *s) /* read current descriptor */ packet_desc_addr = s-tx_desc_addr; + +DB_PRINT(read descriptor 0x%x\n, (unsigned)packet_desc_addr); cpu_physical_memory_read(packet_desc_addr, (uint8_t *)desc[0], sizeof(desc)); /* Handle all descriptors owned by hardware */ @@ -968,6 +970,7 @@ static void gem_transmit(GemState *s) } else { packet_desc_addr += 8; } +DB_PRINT(read descriptor 0x%x\n, (unsigned)packet_desc_addr); cpu_physical_memory_read(packet_desc_addr, (uint8_t *)desc[0], sizeof(desc)); } -- 1.9.2.1.g06c4abd
[Qemu-devel] [PATCH net v1 3/4] net: cadence_gem: Fix top comment
To indicate Cadence GEM not Xilinx. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/net/cadence_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index f999886..7d13e7c 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -1,5 +1,5 @@ /* - * QEMU Xilinx GEM emulation + * QEMU Cadence GEM emulation * * Copyright (c) 2011 Xilinx, Inc. * -- 1.9.2.1.g06c4abd
[Qemu-devel] [PATCH net v1 4/4] net: cadence_gem: Comment spelling sweep
Fix some typos in comments. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/net/cadence_gem.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index 7d13e7c..4e49f07 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -50,7 +50,7 @@ #define GEM_IER (0x0028/4) /* Interrupt Enable reg */ #define GEM_IDR (0x002C/4) /* Interrupt Disable reg */ #define GEM_IMR (0x0030/4) /* Interrupt Mask reg */ -#define GEM_PHYMNTNC (0x0034/4) /* Phy Maintaince reg */ +#define GEM_PHYMNTNC (0x0034/4) /* Phy Maintenance reg */ #define GEM_RXPAUSE (0x0038/4) /* RX Pause Time reg */ #define GEM_TXPAUSE (0x003C/4) /* TX Pause Time reg */ #define GEM_TXPARTIALSF (0x0040/4) /* TX Partial Store and Forward */ @@ -150,7 +150,7 @@ #define GEM_NWCTRL_LOCALLOOP 0x0002 /* Local Loopback */ #define GEM_NWCFG_STRIP_FCS0x0002 /* Strip FCS field */ -#define GEM_NWCFG_LERR_DISC0x0001 /* Discard RX frames with lenth err */ +#define GEM_NWCFG_LERR_DISC0x0001 /* Discard RX frames with len err */ #define GEM_NWCFG_BUFF_OFST_M 0xC000 /* Receive buffer offset mask */ #define GEM_NWCFG_BUFF_OFST_S 14 /* Receive buffer offset shift */ #define GEM_NWCFG_UCAST_HASH 0x0080 /* accept unicast if hash match */ @@ -397,7 +397,7 @@ const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }; */ static void gem_init_register_masks(GemState *s) { -/* Mask of register bits which are read only*/ +/* Mask of register bits which are read only */ memset(s-regs_ro[0], 0, sizeof(s-regs_ro)); s-regs_ro[GEM_NWCTRL] = 0xFFF8; s-regs_ro[GEM_NWSTATUS] = 0x; @@ -720,7 +720,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size) int crc_offset; /* The application wants the FCS field, which QEMU does not provide. - * We must try and caclculate one. + * We must try and calculate one. */ memcpy(rxbuf, buf, size); @@ -877,7 +877,7 @@ static void gem_transmit(GemState *s) DB_PRINT(\n); -/* The packet we will hand off to qemu. +/* The packet we will hand off to QEMU. * Packets scattered across multiple descriptors are gathered to this * one contiguous buffer first. */ @@ -929,7 +929,7 @@ static void gem_transmit(GemState *s) tx_desc_set_used(desc_first); cpu_physical_memory_write(s-tx_desc_addr, (uint8_t *)desc_first[0], sizeof(desc)); -/* Advance the hardare current descriptor past this packet */ +/* Advance the hardware current descriptor past this packet */ if (tx_desc_get_wrap(desc)) { s-tx_desc_addr = s-regs[GEM_TXQBASE]; } else { -- 1.9.2.1.g06c4abd
Re: [Qemu-devel] [PATCH 6/7] monitor: Add netdev_add type argument completion.
On Sun, Apr 27, 2014 at 05:00:07PM +0100, Hani Benhabiles wrote: Also update the command's documentation. Signed-off-by: Hani Benhabiles h...@linux.com --- hmp-commands.hx | 3 ++- hmp.h | 1 + monitor.c | 15 +++ 3 files changed, 18 insertions(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] [PATCH v2] vmdk: Optimize cluster allocation
On Wed, 05/07 11:06, Kevin Wolf wrote: Am 07.05.2014 um 10:57 hat Fam Zheng geschrieben: On Wed, 05/07 10:20, Kevin Wolf wrote: Am 07.05.2014 um 03:45 hat Fam Zheng geschrieben: On Tue, 05/06 10:32, Fam Zheng wrote: On mounted NFS filesystem, ftruncate is much much slower than doing a zero write. Changing this significantly speeds up cluster allocation. Comparing by converting a cirros image (296M) to VMDK on an NFS mount point, over 1Gbe LAN: $ time qemu-img convert cirros-0.3.1.img /mnt/a.raw -O vmdk Before: real0m26.464s user0m0.133s sys 0m0.527s After: real0m2.120s user0m0.080s sys 0m0.197s Signed-off-by: Fam Zheng f...@redhat.com --- V2: Fix cluster_offset check. (Kevin) Signed-off-by: Fam Zheng f...@redhat.com --- block/vmdk.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 06a1f9f..98d2d56 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1037,6 +1037,7 @@ static int get_cluster_offset(BlockDriverState *bs, int min_index, i, j; uint32_t min_count, *l2_table; bool zeroed = false; +int64_t ret; if (m_data) { m_data-valid = 0; @@ -1110,12 +,20 @@ static int get_cluster_offset(BlockDriverState *bs, } /* Avoid the L2 tables update for the images that have snapshots. */ -*cluster_offset = bdrv_getlength(extent-file); +ret = bdrv_getlength(extent-file); +if (ret 0 || +ret ((extent-cluster_sectors BDRV_SECTOR_BITS) - 1)) { +return VMDK_ERROR; +} +*cluster_offset = ret; if (!extent-compressed) { -bdrv_truncate( -extent-file, -*cluster_offset + (extent-cluster_sectors 9) -); +ret = bdrv_write_zeroes(extent-file, +*cluster_offset BDRV_SECTOR_BITS, +extent-cluster_sectors, +0); Hi Stefan, By considering a bdrv_write_zeroes as a pre-write, it in general doubles the write for the whole image, so it's not a good solution. A better way would be removing the bdrv_truncate and require the caller to do full cluster write (with a bounce buffer if necessary). Doesn't get_whole_cluster() already ensure that you already write a full cluster to the image file? That one is actually called get_backing_cluster(), if you look at the code it has. :) Right, it doesn't do anything without a backing file. This is different from qcow2, whose mechanism I assumed without reading the code in detail. :-) I think it would make sense to rewrite get_whole_cluster() to write the cluster for both image with a backing file and standalone images; just that without a backing file it would use memset() to fill the buffer instead of bdrv_read(). Not sure how easy it would be, but it might be an opportunity to also change it to write only those parts of the cluster that aren't written to anyway by the cluster. I think that shouldn't be hard . I'll make the change and send another patch later. Thanks, Fam
Re: [Qemu-devel] [RFC PATCH v2 00/16] visitor+BER migration format
* Markus Armbruster (arm...@redhat.com) wrote: Michael S. Tsirkin m...@redhat.com writes: On Tue, May 06, 2014 at 07:58:07PM +0100, Dr. David Alan Gilbert wrote: * Markus Armbruster (arm...@redhat.com) wrote: Michael S. Tsirkin m...@redhat.com writes: snip OK but for a new machine type, let's default to BER, right? I see no reason to keep supporting when non-BER when -M specifies 2.1 compatibility, do you? I fail to see the relation between machine type and migration's wire encoding. New machine types are a useful but not definitive line in the sand. If you enable something/change the default on a new machine type you know it won't break any existing users since there aren't any. Dve The purpose of machine types is to keep the guest ABI stable. I don't like tacking random crap unrelated to guest ABI to machine types. They're hard enough to grasp for users as they are. Exactly. And on the other hand, someone enabling old machine type and doing live migration is likely to want to be compatible with old qemu wrt migration. Machine types let you migrate to a newer QEMU (forward migration) without messing up the guest ABI. Migrating to an older QEMU (backward migration) basically doesn't work, and as long as that's the case, picking the older wire format by default is worthless. Anyway, we seem to have had a long conversation about the least complicated part of this patch set. I'd love some thoughts on the actual visitor interface, which IMHO is the bit that's actually messy and needs some rethinking). Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 7/7] monitor: Add netdev_del id argument completion.
On Sun, Apr 27, 2014 at 05:00:08PM +0100, Hani Benhabiles wrote: Signed-off-by: Hani Benhabiles h...@linux.com --- hmp-commands.hx | 1 + hmp.h | 1 + monitor.c | 26 ++ 3 files changed, 28 insertions(+) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Re: [Qemu-devel] [PATCH 3/4] block: Allow JSON filenames
Am 06.05.2014 um 21:57 hat Eric Blake geschrieben: On 05/06/2014 01:30 PM, Max Reitz wrote: If the filename given to bdrv_open() is prefixed with json:, parse the rest as a JSON object and use the result as the options QDict. Signed-off-by: Max Reitz mre...@redhat.com --- block.c | 41 + 1 file changed, 41 insertions(+) /* * Opens a disk image (raw, qcow2, vmdk, ...) * @@ -1337,6 +1364,20 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, options = qdict_new(); } +if (filename g_str_has_prefix(filename, json:)) { +QDict *json_options = parse_json_filename(filename, local_err); +if (local_err) { +ret = -EINVAL; +goto fail; +} + +qdict_join(options, json_options, true); +assert(qdict_size(json_options) == 0); Would it be better to pass false to qdict_join(), and then raise an error if the user specified conflicting options? For example (untested, just typing off the top of my head here), -drive file='json:{driver:qcow2,file.filename:foo,backing.file.driver:raw}',backing.file.driver=qcow2 looks like it specifies conflicting backing.file.driver options. Passing true means that qdict_join silently overwrites the value in options to instead be the value in the json string; passing false means you could flag the user error. Isn't the more realistic case, that 'file' is actually the backing file string stored in an image, and the overwriting option comes from the command line? In this case, I think we want to allow overriding the option stored in the qcow2 file. Kevin pgpwYyU85KJK3.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
On 7 May 2014 10:09, Alexander Graf ag...@suse.de wrote: I don't think we should overengineer hacks for legacy virtio. Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes? thanks -- PMM
Re: [Qemu-devel] [SeaBIOS [PATCH V5 0/2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Thu, Apr 10, 2014 at 09:27:44PM +0300, Marcel Apfelbaum wrote: v4 - v5 - Addressed Michael S. Tsirkin's comments (patch 2/2): - Open-coded pci_config_is_reserved() method. v3 - v4: - Addressed Kevin O'Connor's comments: - Refactored a for loop in patch 1/2. - Addressed Michael S. Tsirkin's comments (patch 2/2): - Removed not needed method - Test only base registers (dropped limits tests) - Renamed a helper method - Used 0xFF to test if the memory is reserved - Simplified code in pci_bridge_has_region - I did keep the code that restores base's address as I don't want to modify the registers in a 'query' method. (as replied on the mail thread) v2 - v3: - Addressed Michael S. Tsirkin's comments: - I/O and Prefetchable Memory are optional. Do not allocate ranges if they are not implemented (2/2). - Note that 2/2 patch can be seen as a separate fix. However, it is related to ranges reservation. v1 - v2: - Thanks Gerd Hoffmann for the review. - Addressed Michael S. Tsirkin's comments: - Limit capabilities query to 256 iterations, to make sure we don't get into an infinite loop with a broken device. If a pci-2-pci bridge supports hot-plug functionality but there are no devices connected to it, reserve IO/mem in order to be able to attach devices later. Do not waste space, use minimum allowed. Marcel Apfelbaum (2): hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached hw/pci: check if pci2pci bridges implement optional limit registers src/fw/pciinit.c | 12 +--- src/hw/pci.c | 45 + src/hw/pci.h | 10 ++ 3 files changed, 60 insertions(+), 7 deletions(-) It would be nice to have a seabios release with these patches included in QEMU: make it easier for people to use hotplug. Gerd? -- 1.8.3.1
Re: [Qemu-devel] [SeaBIOS [PATCH V5 0/2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
Hi, It would be nice to have a seabios release with these patches included in QEMU: make it easier for people to use hotplug. New release from master is planned already. cheers, Gerd
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
Am 07.05.2014 um 11:26 schrieb Peter Maydell peter.mayd...@linaro.org: On 7 May 2014 10:09, Alexander Graf ag...@suse.de wrote: I don't think we should overengineer hacks for legacy virtio. Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes? The interrupt endianness for book3s PPC. Since that's an arch specific thing I think we should just make the determination mechanism arch dependent and list it in the spec. Booke for example would be vastly different since there is no global LE flag - it's a bit in the TLB entries. Alex
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
On 7 May 2014 10:37, Alexander Graf ag...@suse.de wrote: Am 07.05.2014 um 11:26 schrieb Peter Maydell peter.mayd...@linaro.org: On 7 May 2014 10:09, Alexander Graf ag...@suse.de wrote: I don't think we should overengineer hacks for legacy virtio. Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes? The interrupt endianness for book3s PPC. Since that's an arch specific thing I think we should just make the determination mechanism arch dependent and list it in the spec. Booke for example would be vastly different since there is no global LE flag - it's a bit in the TLB entries. Sure, but I think we should also state the general principle we're aiming to implement with the arch specific detail, so that people figuring out what a new arch should do have a guide to follow. thanks -- PMM
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
Am 07.05.2014 um 11:26 schrieb Peter Maydell peter.mayd...@linaro.org: On 7 May 2014 10:09, Alexander Graf ag...@suse.de wrote: I don't think we should overengineer hacks for legacy virtio. Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes? I just realized we're talking about virtio in a non-virtio thread. This patch set is about core dump support which is different from virtio bi-endian support. While both may end up at the same logic, I don't like the idea to mix them. This function is PPC internal. Alex thanks -- PMM
Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian
Am 07.05.2014 um 11:40 schrieb Peter Maydell peter.mayd...@linaro.org: On 7 May 2014 10:37, Alexander Graf ag...@suse.de wrote: Am 07.05.2014 um 11:26 schrieb Peter Maydell peter.mayd...@linaro.org: On 7 May 2014 10:09, Alexander Graf ag...@suse.de wrote: I don't think we should overengineer hacks for legacy virtio. Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes? The interrupt endianness for book3s PPC. Since that's an arch specific thing I think we should just make the determination mechanism arch dependent and list it in the spec. Booke for example would be vastly different since there is no global LE flag - it's a bit in the TLB entries. Sure, but I think we should also state the general principle we're aiming to implement with the arch specific detail, so that people figuring out what a new arch should do have a guide to follow. The general principle is Try to find the Linux guest compile time endianness :). Alex
Re: [Qemu-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Hi, all Ping...anyone? Thanks! Best regards, -Gonglei -Original Message- From: Gonglei (Arei) Sent: Sunday, May 04, 2014 5:25 PM To: qemu-devel@nongnu.org; xen-de...@lists.xen.org Cc: ian.campb...@citrix.com; jbeul...@suse.com; stefano.stabell...@eu.citrix.com; fabio.fant...@m2r.biz; anthony.per...@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei (UVP); Gonglei (Arei) Subject: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching From: Gaowei gao.gao...@huawei.com In Xen platform, after using upstream qemu, the all of pci devices will show hotplug in the windows guest. In this situation, the windows guest may occur blue screen when VM' user click the icon of VGA card for trying unplug VGA card. However, we don't hope VM's user can do such dangerous operation, and showing all pci devices inside the guest OS is unfriendly. This is done by runtime patching: - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that this has the same checksum, but is ignored by OSPM. - At compile time, look for these methods in ASL source,find the matching AML, and store the offsets of these methods in a table named aml_ej0_data. - At run time, go over aml_ej0_data, check which slots not support hotplug and patch the ACPI table, replacing _EJ0 with EJ0_. Signed-off-by: Gaowei gao.gao...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- v3-v2: reformat on the latest master v2-v1: rework by Jan Beulich's suggestion: - adjust the code style tools/firmware/hvmloader/acpi/Makefile | 44 ++- tools/firmware/hvmloader/acpi/acpi2_0.h| 4 + tools/firmware/hvmloader/acpi/build.c | 22 ++ tools/firmware/hvmloader/acpi/dsdt.asl | 1 + tools/firmware/hvmloader/acpi/mk_dsdt.c| 2 + tools/firmware/hvmloader/ovmf.c| 6 +- tools/firmware/hvmloader/rombios.c | 4 + tools/firmware/hvmloader/seabios.c | 8 + tools/firmware/hvmloader/tools/acpi_extract.py | 308 + .../hvmloader/tools/acpi_extract_preprocess.py | 41 +++ 10 files changed, 428 insertions(+), 12 deletions(-) create mode 100644 tools/firmware/hvmloader/tools/acpi_extract.py create mode 100644 tools/firmware/hvmloader/tools/acpi_extract_preprocess.py diff --git a/tools/firmware/hvmloader/acpi/Makefile b/tools/firmware/hvmloader/acpi/Makefile index 2c50851..f79ecc3 100644 --- a/tools/firmware/hvmloader/acpi/Makefile +++ b/tools/firmware/hvmloader/acpi/Makefile @@ -24,30 +24,46 @@ OBJS = $(patsubst %.c,%.o,$(C_SRC)) CFLAGS += $(CFLAGS_xeninclude) vpath iasl $(PATH) +vpath python $(PATH) + +.DELETE_ON_ERROR:$(filter dsdt_%.c,$(C_SRC)) + all: acpi.a ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl iasl -vs -p $* -tc $ - sed -e 's/AmlCode/$*/g' $*.hex $@ + sed -e 's/AmlCode/$*/g' $*.hex $@.tmp + $(call move-if-changed,$@.tmp $@) rm -f $*.hex $*.aml mk_dsdt: mk_dsdt.c $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt - awk 'NR 1 {print s} {s=$$0}' $ $@ - ./mk_dsdt --dm-version qemu-xen $@ + awk 'NR 1 {print s} {s=$$0}' $ $@.tmp + sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@.tmp + ./mk_dsdt --dm-version qemu-xen $@.tmp + sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' $@.tmp + sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' $@.tmp + $(call move-if-changed,$@.tmp $@) # NB. awk invocation is a portable alternative to 'head -n -1' dsdt_%cpu.asl: dsdt.asl mk_dsdt - awk 'NR 1 {print s} {s=$$0}' $ $@ - ./mk_dsdt --maxcpu $* $@ + awk 'NR 1 {print s} {s=$$0}' $ $@.tmp + sed -i 's/AmlCode/dsdt_$*cpu/g' $@.tmp + ./mk_dsdt --maxcpu $* $@.tmp + $(call move-if-changed,$@.tmp $@) -$(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl - iasl -vs -p $* -tc $*.asl - sed -e 's/AmlCode/$*/g' $*.hex $@ - echo int $*_len=sizeof($*); $@ - rm -f $*.aml $*.hex +$(filter dsdt_%.c,$(C_SRC)): %.c: iasl python %.asl + cpp -P $*.asl $*.asl.i.orig + python ../tools/acpi_extract_preprocess.py $*.asl.i.orig $*.asl.i + iasl -vs -l -tc -p $* $*.asl.i + python ../tools/acpi_extract.py $*.lst $@.tmp + echo int $*_len=sizeof($*); $@.tmp + if grep -q $*_aml_ej0_name $@.tmp; then echo int $*_aml_ej0_name_len=sizeof($*_aml_ej0_name); $@.tmp; fi + if grep -q $*_aml_adr_dword $@.tmp; then echo int $*_aml_adr_dword_len=sizeof($*_aml_adr_dword); $@.tmp;fi + $(call move-if-changed,$@.tmp $@) + rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst iasl: @echo @@ -57,6 +73,12 @@ iasl: @echo @exit 1 +python: + @echo + @echo python is needed + @echo +
Re: [Qemu-devel] [RFC PATCH v2 05/16] Header/constant/types fixes for visitors
On Wed, Apr 23, 2014 at 05:37:38PM +0100, Dr. David Alan Gilbert (git) wrote: From: Dr. David Alan Gilbert dgilb...@redhat.com Move constants around and add types to allow file structure to move into visitors. Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com --- arch_init.c | 12 include/migration/migration.h | 17 + include/migration/vmstate.h | 20 +--- include/qemu/typedefs.h | 4 ++-- 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/arch_init.c b/arch_init.c index 60c975d..73b9303 100644 --- a/arch_init.c +++ b/arch_init.c @@ -110,18 +110,6 @@ static bool mig_throttle_on; static int dirty_rate_high_cnt; static void check_guest_throttling(void); -/***/ -/* ram save/restore */ - -#define RAM_SAVE_FLAG_FULL 0x01 /* Obsolete, not used anymore */ -#define RAM_SAVE_FLAG_COMPRESS 0x02 -#define RAM_SAVE_FLAG_MEM_SIZE 0x04 -#define RAM_SAVE_FLAG_PAGE 0x08 -#define RAM_SAVE_FLAG_EOS 0x10 -#define RAM_SAVE_FLAG_CONTINUE 0x20 -#define RAM_SAVE_FLAG_XBZRLE 0x40 -/* 0x80 is reserved in migration.h start with 0x100 next */ - static struct defconfig_file { const char *filename; /* Indicates it is an user config file (disabled by -no-user-config) */ diff --git a/include/migration/migration.h b/include/migration/migration.h index 3e1e6c7..825 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -165,7 +165,16 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags); * side. This lets before_ram_iterate/after_ram_iterate add * transport-specific sections to the RAM migration data. */ +/* ram save/restore */ +#define RAM_SAVE_FLAG_FULL 0x01 /* Obsolete, not used anymore */ +#define RAM_SAVE_FLAG_COMPRESS 0x02 +#define RAM_SAVE_FLAG_MEM_SIZE 0x04 +#define RAM_SAVE_FLAG_PAGE 0x08 +#define RAM_SAVE_FLAG_EOS 0x10 +#define RAM_SAVE_FLAG_CONTINUE 0x20 +#define RAM_SAVE_FLAG_XBZRLE 0x40 #define RAM_SAVE_FLAG_HOOK 0x80 +#define RAM_SAVE_FLAG_MASK0x1ff #define RAM_SAVE_CONTROL_NOT_SUPP -1000 #define RAM_SAVE_CONTROL_DELAYED -2000 @@ -174,4 +183,12 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, ram_addr_t offset, size_t size, int *bytes_sent); +typedef struct { +uint64_t addr; +uint16_t flags; +char idstr[256]; +char ch; /* Used for filled pages (normally 0 fill) */ +size_t len; /* Uses include xbzrle's data len */ +} ramsecentry_header; + RamSecEntryHeader? and maybe we should make this 256 a named constant too. #endif diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index e7e1705..a5e4b0b 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -26,6 +26,7 @@ #ifndef QEMU_VMSTATE_H #define QEMU_VMSTATE_H 1 +#include qemu/typedefs.h #ifndef CONFIG_USER_ONLY #include migration/qemu-file.h #endif @@ -49,15 +50,27 @@ typedef struct SaveVMHandlers { * use data that is local to the migration thread or protected * by other locks. */ -int (*save_live_iterate)(QEMUFile *f, void *opaque); +int (*save_live_iterate)(Visitor *v, void *opaque); /* This runs outside the iothread lock! */ -int (*save_live_setup)(QEMUFile *f, void *opaque); -uint64_t (*save_live_pending)(QEMUFile *f, void *opaque, uint64_t max_size); +int (*save_live_setup)(Visitor *v, void *opaque); +uint64_t (*save_live_pending)(void *opaque, uint64_t max_size); LoadStateHandler *load_state; } SaveVMHandlers; +/* This is the data used to identify a section as passed + * into the section version of the compat sequence visitor + * (TODO: Probably want to move the whole name lookup into there + *and keep the section_id wrapped inside the binary visitor) + */ +typedef struct SectionHeader { +uint32_t section_id; +uint32_t instance_id; /* Below only used for full version */ +uint32_t version_id; +char idstr[256]; +} SectionHeader; + int register_savevm(DeviceState *dev, const char *idstr, int instance_id, @@ -134,6 +147,7 @@ struct VMStateDescription { void (*pre_save)(void *opaque); VMStateField *fields; const VMStateSubsection *subsections; +uint32_t ber_tag; }; #ifdef CONFIG_USER_ONLY diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index bf8daac..3fea88e 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -10,8 +10,6 @@ typedef struct QEMUBH QEMUBH; typedef struct AioContext AioContext; -typedef struct Visitor Visitor; - struct Monitor; typedef struct Monitor Monitor; typedef struct MigrationParams MigrationParams; @@ -39,6 +37,7 @@
Re: [Qemu-devel] [RFC PATCH v2 01/16] Visitor: Add methods for migration format use
On Wed, Apr 23, 2014 at 05:37:34PM +0100, Dr. David Alan Gilbert (git) wrote: From: Dr. David Alan Gilbert dgilb...@redhat.com array types From https://lists.gnu.org/archive/html/qemu-devel/2011-09/msg02465.html str256 type For the upto 256byte strings QEMU commonly uses for IDs buffer type For a blob of data that the caller wants to deliver whole (e.g. a page of RAM or block of disk) Load/save flags to let a user perform pre-save/post-load checking An accessor to get the underlying QEMUFile* (for compatibility) compat-sequences Provide enough information for a visitor providing compatibility with the old format to generate it's byte stream, while allowing a new visitor to do something sensible. destroy Allows the caller to destroy a visitor without knowing what type of visitor it is. Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com --- include/qapi/visitor-impl.h | 23 + include/qapi/visitor.h | 51 + qapi/qapi-visit-core.c | 80 + 3 files changed, 154 insertions(+) diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index f3fa420..10cdbf7 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -15,6 +15,9 @@ #include qapi/error.h #include qapi/visitor.h +#define VISITOR_SAVING (10) +#define VISITOR_LOADING (11) + struct Visitor { /* Must be set */ @@ -29,6 +32,10 @@ struct Visitor void (*start_list)(Visitor *v, const char *name, Error **errp); GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp); void (*end_list)(Visitor *v, Error **errp); +void (*start_array)(Visitor *v, void **obj, const char *name, +size_t elem_count, size_t elem_size, Error **errp); +void (*next_array)(Visitor *v, Error **errp); +void (*end_array)(Visitor *v, Error **errp); void (*type_enum)(Visitor *v, int *obj, const char *strings[], const char *kind, const char *name, Error **errp); @@ -38,6 +45,7 @@ struct Visitor void (*type_int)(Visitor *v, int64_t *obj, const char *name, Error **errp); void (*type_bool)(Visitor *v, bool *obj, const char *name, Error **errp); void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp); +void (*type_str256)(Visitor *v, char *obj, const char *name, Error **errp); void (*type_number)(Visitor *v, double *obj, const char *name, Error **errp); @@ -49,6 +57,8 @@ struct Visitor void (*start_handle)(Visitor *v, void **obj, const char *kind, const char *name, Error **errp); void (*end_handle)(Visitor *v, Error **errp); +void (*type_buffer)(Visitor *v, void *data, size_t len, bool async, +const char *name, Error **errp); void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp); void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp); void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp); @@ -59,6 +69,19 @@ struct Visitor void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp); /* visit_type_size() falls back to (*type_uint64)() if type_size is unset */ void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp); + +void (*destroy)(Visitor *v, Error **errp); + +void (*start_sequence_compat)(Visitor *v, const char *name, + Visit_seq_compat_mode compat_mode, + void *opaque, Error **errp); +void (*end_sequence_compat)(Visitor *v, const char *name, +Visit_seq_compat_mode compat_mode, +Error **errp); + +QEMUFile* (*get_qemufile)(Visitor *v); + +uint64_t flags; }; void input_type_enum(Visitor *v, int *obj, const char *strings[], diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 29da211..70c20df 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -39,11 +39,22 @@ void visit_end_implicit_struct(Visitor *v, Error **errp); void visit_start_list(Visitor *v, const char *name, Error **errp); GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp); void visit_end_list(Visitor *v, Error **errp); +void visit_start_array(Visitor *v, void **obj, const char *name, + size_t elem_count, size_t elem_size, Error **errp); +void visit_next_array(Visitor *v, Error **errp); +void visit_end_array(Visitor *v, Error **errp); + void visit_start_optional(Visitor *v, bool *present, const char *name, Error **errp); void visit_end_optional(Visitor *v, Error **errp); void visit_get_next_type(Visitor *v, int
[Qemu-devel] [PATCH v27 01/33] QemuOpts: move find_desc_by_name ahead for later calling
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com Signed-off-by: Chunyan Liu cy...@suse.com --- util/qemu-option.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index 8bbc3ad..808aef4 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -173,6 +173,20 @@ static void parse_option_number(const char *name, const char *value, } } +static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc, +const char *name) +{ +int i; + +for (i = 0; desc[i].name != NULL; i++) { +if (strcmp(desc[i].name, name) == 0) { +return desc[i]; +} +} + +return NULL; +} + void parse_option_size(const char *name, const char *value, uint64_t *ret, Error **errp) { @@ -637,20 +651,6 @@ static bool opts_accepts_any(const QemuOpts *opts) return opts-list-desc[0].name == NULL; } -static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc, -const char *name) -{ -int i; - -for (i = 0; desc[i].name != NULL; i++) { -if (strcmp(desc[i].name, name) == 0) { -return desc[i]; -} -} - -return NULL; -} - int qemu_opt_unset(QemuOpts *opts, const char *name) { QemuOpt *opt = qemu_opt_find(opts, name); -- 1.7.12.4
[Qemu-devel] [PATCH v27 00/33] replace QEMUOptionParameter with QemuOpts
This patch series is to replace QEMUOptionParameter with QemuOpts, so that only one Qemu Option structure is kept in QEMU code. --- Changes to v26: * Following Eric's comment, backward split 2/33, 3/33. (repurpose qemu_opts_print first, add def_value_str to QemuOptDesc later). * Fix memory free in qemu_opts_append to solve iotest issue. 10/33 * Following Eric's comment, remove the end '.' in error message. And update qemu-iotests .out file. 12/33 * Following Eric's comment, fix memory free in vvfat.c 13/33 * Following Eric's comment, split qcow2 patch into two. 19/33, 20/33: export qemu_opt_find first, add qcow2 driver patch later. * rebase to git master All patches are also available from: https://github.com/chunyanliu/qemu/commits/QemuOpts Chunyan Liu (33): QemuOpts: move find_desc_by_name ahead for later calling QemuOpts: repurpose qemu_opts_print to replace print_option_parameters QemuOpts: add def_value_str to QemuOptDesc qapi: output def_value_str when query command line options QemuOpts: change opt-name|str from (const char *) to (char *) QemuOpts: move qemu_opt_del ahead for later calling QemuOpts: add qemu_opt_get_*_del functions for replace work QemuOpts: add qemu_opts_print_help to replace print_option_help QemuOpts: add conversion between QEMUOptionParameter to QemuOpts QemuOpts: add qemu_opts_append to replace append_option_parameters QemuOpts: check NULL input for qemu_opts_del change block layer to support both QemuOpts and QEMUOptionParamter vvfat.c: handle cross_driver's create_options and create_opts cow.c: replace QEMUOptionParameter with QemuOpts gluster.c: replace QEMUOptionParameter with QemuOpts iscsi.c: replace QEMUOptionParameter with QemuOpts nfs.c: replace QEMUOptionParameter with QemuOpts qcow.c: replace QEMUOptionParameter with QemuOpts QemuOpts: export qemu_opt_find qcow2.c: replace QEMUOptionParameter with QemuOpts qed.c: replace QEMUOptionParameter with QemuOpts raw-posix.c: replace QEMUOptionParameter with QemuOpts raw-win32.c: replace QEMUOptionParameter with QemuOpts raw_bsd.c: replace QEMUOptionParameter with QemuOpts rbd.c: replace QEMUOptionParameter with QemuOpts sheepdog.c: replace QEMUOptionParameter with QemuOpts ssh.c: replace QEMUOptionParameter with QemuOpts vdi.c: replace QEMUOptionParameter with QemuOpts vhdx.c: replace QEMUOptionParameter with QemuOpts vmdk.c: replace QEMUOptionParameter with QemuOpts vpc.c: replace QEMUOptionParameter with QemuOpts cleanup QEMUOptionParameter QemuOpts: cleanup tmp 'allocated' member from QemuOptsList block.c| 99 block/cow.c| 52 ++-- block/gluster.c| 73 +++--- block/iscsi.c | 32 ++- block/nfs.c| 10 +- block/qcow.c | 72 +++--- block/qcow2.c | 259 ++-- block/qed.c| 112 + block/qed.h| 3 +- block/raw-posix.c | 55 ++--- block/raw-win32.c | 38 +-- block/raw_bsd.c| 25 +- block/rbd.c| 61 +++-- block/sheepdog.c | 102 block/ssh.c| 30 ++- block/vdi.c| 71 +++--- block/vhdx.c | 97 block/vhdx.h | 1 + block/vmdk.c | 121 +- block/vpc.c| 60 ++--- block/vvfat.c | 13 +- include/block/block.h | 7 +- include/block/block_int.h | 9 +- include/qemu/option.h | 53 +--- include/qemu/option_int.h | 4 +- qapi-schema.json | 5 +- qapi/opts-visitor.c| 10 +- qemu-img.c | 89 +++ qmp-commands.hx| 2 + tests/qemu-iotests/049.out | 2 +- tests/qemu-iotests/061.out | 2 +- util/qemu-config.c | 4 + util/qemu-option.c | 588 - 33 files changed, 1033 insertions(+), 1128 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH v27 05/33] QemuOpts: change opt-name|str from (const char *) to (char *)
qemu_opt_del() already assumes that all QemuOpt instances contain malloc'd name and value; but it had to cast away const because opts_start_struct() was doing its own thing and using static storage instead. By using the correct type and malloced strings everywhere, the usage of this struct becomes clearer. Reviewed-by: Eric Blake ebl...@redhat.com Reviewed-by: Leandro Dorileo l...@dorileo.org Signed-off-by: Chunyan Liu cy...@suse.com --- include/qemu/option_int.h | 4 ++-- qapi/opts-visitor.c | 10 +++--- util/qemu-option.c| 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h index 8212fa4..6432c1a 100644 --- a/include/qemu/option_int.h +++ b/include/qemu/option_int.h @@ -30,8 +30,8 @@ #include qemu/error-report.h struct QemuOpt { -const char *name; -const char *str; +char *name; +char *str; const QemuOptDesc *desc; union { diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 5d830a2..7a64f4e 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -143,8 +143,8 @@ opts_start_struct(Visitor *v, void **obj, const char *kind, if (ov-opts_root-id != NULL) { ov-fake_id_opt = g_malloc0(sizeof *ov-fake_id_opt); -ov-fake_id_opt-name = id; -ov-fake_id_opt-str = ov-opts_root-id; +ov-fake_id_opt-name = g_strdup(id); +ov-fake_id_opt-str = g_strdup(ov-opts_root-id); opts_visitor_insert(ov-unprocessed_opts, ov-fake_id_opt); } } @@ -177,7 +177,11 @@ opts_end_struct(Visitor *v, Error **errp) } g_hash_table_destroy(ov-unprocessed_opts); ov-unprocessed_opts = NULL; -g_free(ov-fake_id_opt); +if (ov-fake_id_opt) { +g_free(ov-fake_id_opt-name); +g_free(ov-fake_id_opt-str); +g_free(ov-fake_id_opt); +} ov-fake_id_opt = NULL; } diff --git a/util/qemu-option.c b/util/qemu-option.c index 2be6995..69cdf3f 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -664,8 +664,8 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp) static void qemu_opt_del(QemuOpt *opt) { QTAILQ_REMOVE(opt-opts-head, opt, next); -g_free((/* !const */ char*)opt-name); -g_free((/* !const */ char*)opt-str); +g_free(opt-name); +g_free(opt-str); g_free(opt); } -- 1.7.12.4
[Qemu-devel] [PATCH v27 10/33] QemuOpts: add qemu_opts_append to replace append_option_parameters
For later merge .create_opts of drv and proto_drv in qemu-img commands. Signed-off-by: Chunyan Liu cy...@suse.com --- Changes: * fix memory free: if (param) { - g_free(list); + qemu_opts_free(list); //free allocated memory too } include/qemu/option.h | 5 util/qemu-option.c| 67 +++ 2 files changed, 72 insertions(+) diff --git a/include/qemu/option.h b/include/qemu/option.h index e98e8ef..44d9961 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -176,5 +176,10 @@ void qemu_opts_print_help(QemuOptsList *list); void qemu_opts_free(QemuOptsList *list); QEMUOptionParameter *opts_to_params(QemuOpts *opts); QemuOptsList *params_to_opts(QEMUOptionParameter *list); +/* FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts. + */ +QemuOptsList *qemu_opts_append(QemuOptsList *dst, + QemuOptsList *list, + QEMUOptionParameter *param); #endif diff --git a/util/qemu-option.c b/util/qemu-option.c index 93ffcd5..e15723a 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -1498,3 +1498,70 @@ void qemu_opts_free(QemuOptsList *list) g_free(list); } + +/* Realloc dst option list and append options either from an option list (list) + * or a QEMUOptionParameter (param) to it. dst could be NULL or a malloced list. + * FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts. + */ +QemuOptsList *qemu_opts_append(QemuOptsList *dst, + QemuOptsList *list, + QEMUOptionParameter *param) +{ +size_t num_opts, num_dst_opts; +QemuOptDesc *desc; +bool need_init = false; + +assert(!(list param)); +if (!param !list) { +return dst; +} + +if (param) { +list = params_to_opts(param); +} + +/* If dst is NULL, after realloc, some area of dst should be initialized + * before adding options to it. + */ +if (!dst) { +need_init = true; +} + +num_opts = count_opts_list(dst); +num_dst_opts = num_opts; +num_opts += count_opts_list(list); +dst = g_realloc(dst, sizeof(QemuOptsList) + +(num_opts + 1) * sizeof(QemuOptDesc)); +if (need_init) { +dst-name = NULL; +dst-implied_opt_name = NULL; +QTAILQ_INIT(dst-head); +dst-allocated = true; +dst-merge_lists = false; +} +dst-desc[num_dst_opts].name = NULL; + +/* (const char *) members of result dst are malloced, need free. */ +assert(dst-allocated); +/* append list-desc to dst-desc */ +if (list) { +desc = list-desc; +while (desc desc-name) { +if (find_desc_by_name(dst-desc, desc-name) == NULL) { +dst-desc[num_dst_opts].name = g_strdup(desc-name); +dst-desc[num_dst_opts].type = desc-type; +dst-desc[num_dst_opts].help = g_strdup(desc-help); +dst-desc[num_dst_opts].def_value_str = + g_strdup(desc-def_value_str); +num_dst_opts++; +dst-desc[num_dst_opts].name = NULL; +} +desc++; +} +} + +if (param) { +qemu_opts_free(list); +} +return dst; +} -- 1.7.12.4
[Qemu-devel] [PATCH v27 04/33] qapi: output def_value_str when query command line options
Change qapi interfaces to output the newly added def_value_str when querying command line options. Reviewed-by: Stefan Hajnoczi stefa...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com Reviewed-by: Leandro Dorileo l...@dorileo.org Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com Signed-off-by: Chunyan Liu cy...@suse.com --- qapi-schema.json | 5 - qmp-commands.hx| 2 ++ util/qemu-config.c | 4 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/qapi-schema.json b/qapi-schema.json index 0b00427..880829d 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4088,12 +4088,15 @@ # # @help: #optional human readable text string, not suitable for parsing. # +# @default: #optional default value string (since 2.1) +# # Since 1.5 ## { 'type': 'CommandLineParameterInfo', 'data': { 'name': 'str', 'type': 'CommandLineParameterType', -'*help': 'str' } } +'*help': 'str', +'*default': 'str' } } ## # @CommandLineOptionInfo: diff --git a/qmp-commands.hx b/qmp-commands.hx index ed3ab92..1271332 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2895,6 +2895,8 @@ Each array entry contains the following: or 'size') - help: human readable description of the parameter (json-string, optional) +- default: default value string for the parameter + (json-string, optional) Example: diff --git a/util/qemu-config.c b/util/qemu-config.c index f4e4f38..ba375c0 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -82,6 +82,10 @@ static CommandLineParameterInfoList *query_option_descs(const QemuOptDesc *desc) info-has_help = true; info-help = g_strdup(desc[i].help); } +if (desc[i].def_value_str) { +info-has_q_default = true; +info-q_default = g_strdup(desc[i].def_value_str); +} entry = g_malloc0(sizeof(*entry)); entry-value = info; -- 1.7.12.4
[Qemu-devel] [PATCH v27 06/33] QemuOpts: move qemu_opt_del ahead for later calling
In later patch, qemu_opt_get_del functions will be added, they will first get the option value, then call qemu_opt_del to remove the option from opt list. To prepare for that purpose, move qemu_opt_del ahead first. Reviewed-by: Eric Blake ebl...@redhat.com Reviewed-by: Leandro Dorileo l...@dorileo.org Signed-off-by: Chunyan Liu cy...@suse.com --- util/qemu-option.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index 69cdf3f..4d2d4d1 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -567,6 +567,14 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name) return NULL; } +static void qemu_opt_del(QemuOpt *opt) +{ +QTAILQ_REMOVE(opt-opts-head, opt, next); +g_free(opt-name); +g_free(opt-str); +g_free(opt); +} + const char *qemu_opt_get(QemuOpts *opts, const char *name) { QemuOpt *opt = qemu_opt_find(opts, name); @@ -661,14 +669,6 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp) } } -static void qemu_opt_del(QemuOpt *opt) -{ -QTAILQ_REMOVE(opt-opts-head, opt, next); -g_free(opt-name); -g_free(opt-str); -g_free(opt); -} - static bool opts_accepts_any(const QemuOpts *opts) { return opts-list-desc[0].name == NULL; -- 1.7.12.4
[Qemu-devel] [PATCH v27 02/33] QemuOpts: repurpose qemu_opts_print to replace print_option_parameters
Currently this function is not used anywhere. In later patches, it will replace print_option_parameters. To avoid print info changes, change qemu_opts_print from fprintf stderr to printf, and remove last printf. Signed-off-by: Chunyan Liu cy...@suse.com --- Changes: * backward split than v26 (2/33, 3/33). include/qemu/option.h | 2 +- util/qemu-option.c| 10 -- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/include/qemu/option.h b/include/qemu/option.h index 8c0ac34..1077b69 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -156,7 +156,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict); void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp); typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque); -int qemu_opts_print(QemuOpts *opts, void *dummy); +void qemu_opts_print(QemuOpts *opts); int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void *opaque, int abort_on_failure); diff --git a/util/qemu-option.c b/util/qemu-option.c index 808aef4..290ed54 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -895,17 +895,15 @@ void qemu_opts_del(QemuOpts *opts) g_free(opts); } -int qemu_opts_print(QemuOpts *opts, void *dummy) +void qemu_opts_print(QemuOpts *opts) { QemuOpt *opt; -fprintf(stderr, %s: %s:, opts-list-name, -opts-id ? opts-id : noid); +printf(%s: %s:, opts-list-name, + opts-id ? opts-id : noid); QTAILQ_FOREACH(opt, opts-head, next) { -fprintf(stderr, %s=\%s\, opt-name, opt-str); +printf( %s=\%s\, opt-name, opt-str); } -fprintf(stderr, \n); -return 0; } static int opts_do_parse(QemuOpts *opts, const char *params, -- 1.7.12.4
[Qemu-devel] [PATCH v27 03/33] QemuOpts: add def_value_str to QemuOptDesc
Add def_value_str (default value) to QemuOptDesc, to replace function of the default value in QEMUOptionParameter. Improve qemu_opts_get_* functions: if find opt, return opt-str; otherwise, if desc-def_value_str is set, return desc-def_value_str; otherwise, return input defval. Improve qemu_opts_print: if option is set, print opt-str; otherwise, if desc-def_value_str is set, print it. Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com Signed-off-by: Chunyan Liu cy...@suse.com --- Changes: * backward split than v26. (2/33, 3/33) include/qemu/option.h | 1 + util/qemu-option.c| 56 --- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/include/qemu/option.h b/include/qemu/option.h index 1077b69..c3b0a91 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -99,6 +99,7 @@ typedef struct QemuOptDesc { const char *name; enum QemuOptType type; const char *help; +const char *def_value_str; } QemuOptDesc; struct QemuOptsList { diff --git a/util/qemu-option.c b/util/qemu-option.c index 290ed54..2be6995 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -570,6 +570,13 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name) const char *qemu_opt_get(QemuOpts *opts, const char *name) { QemuOpt *opt = qemu_opt_find(opts, name); + +if (!opt) { +const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name); +if (desc desc-def_value_str) { +return desc-def_value_str; +} +} return opt ? opt-str : NULL; } @@ -589,8 +596,13 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) { QemuOpt *opt = qemu_opt_find(opts, name); -if (opt == NULL) +if (opt == NULL) { +const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name); +if (desc desc-def_value_str) { +parse_option_bool(name, desc-def_value_str, defval, error_abort); +} return defval; +} assert(opt-desc opt-desc-type == QEMU_OPT_BOOL); return opt-value.boolean; } @@ -599,8 +611,14 @@ uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval) { QemuOpt *opt = qemu_opt_find(opts, name); -if (opt == NULL) +if (opt == NULL) { +const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name); +if (desc desc-def_value_str) { +parse_option_number(name, desc-def_value_str, defval, +error_abort); +} return defval; +} assert(opt-desc opt-desc-type == QEMU_OPT_NUMBER); return opt-value.uint; } @@ -609,8 +627,13 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval) { QemuOpt *opt = qemu_opt_find(opts, name); -if (opt == NULL) +if (opt == NULL) { +const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name); +if (desc desc-def_value_str) { +parse_option_size(name, desc-def_value_str, defval, error_abort); +} return defval; +} assert(opt-desc opt-desc-type == QEMU_OPT_SIZE); return opt-value.uint; } @@ -898,11 +921,30 @@ void qemu_opts_del(QemuOpts *opts) void qemu_opts_print(QemuOpts *opts) { QemuOpt *opt; +QemuOptDesc *desc = opts-list-desc; -printf(%s: %s:, opts-list-name, - opts-id ? opts-id : noid); -QTAILQ_FOREACH(opt, opts-head, next) { -printf( %s=\%s\, opt-name, opt-str); +if (desc[0].name == NULL) { +QTAILQ_FOREACH(opt, opts-head, next) { +printf(%s=\%s\ , opt-name, opt-str); +} +return; +} +for (; desc desc-name; desc++) { +const char *value; +QemuOpt *opt = qemu_opt_find(opts, desc-name); + +value = opt ? opt-str : desc-def_value_str; +if (!value) { +continue; +} +if (desc-type == QEMU_OPT_STRING) { +printf(%s='%s' , desc-name, value); +} else if ((desc-type == QEMU_OPT_SIZE || +desc-type == QEMU_OPT_NUMBER) opt) { +printf(%s=% PRId64 , desc-name, opt-value.uint); +} else { +printf(%s=%s , desc-name, value); +} } } -- 1.7.12.4