Re: [Qemu-devel] virtio-serial-pci very expensive during live migration

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Alexey Kardashevskiy
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

2014-05-07 Thread Alexey Kardashevskiy
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()

2014-05-07 Thread Alexey Kardashevskiy
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

2014-05-07 Thread Alexey Kardashevskiy
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

2014-05-07 Thread Alexey Kardashevskiy
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Paolo Bonzini

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

2014-05-07 Thread Alexey Kardashevskiy
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

2014-05-07 Thread Paolo Bonzini

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

2014-05-07 Thread Paolo Bonzini

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

2014-05-07 Thread Mike Perez
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

2014-05-07 Thread Alexey Kardashevskiy
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

2014-05-07 Thread Liu Ping Fan
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

2014-05-07 Thread Hu Tao
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

2014-05-07 Thread Alexey Kardashevskiy
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

2014-05-07 Thread Stefan Hajnoczi
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

2014-05-07 Thread Markus Armbruster
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()

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Markus Armbruster
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()

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Markus Armbruster
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()

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Markus Armbruster
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

2014-05-07 Thread Stefan Hajnoczi
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

2014-05-07 Thread Paolo Bonzini

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

2014-05-07 Thread Stefan Hajnoczi
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

2014-05-07 Thread Stefan Hajnoczi
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

2014-05-07 Thread Greg Kurz
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

2014-05-07 Thread Greg Kurz
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

2014-05-07 Thread Kevin Wolf
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

2014-05-07 Thread Kevin Wolf
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

2014-05-07 Thread Ján Tomko
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

2014-05-07 Thread Kevin Wolf
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

2014-05-07 Thread Kevin Wolf
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

2014-05-07 Thread Stefan Hajnoczi
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

2014-05-07 Thread Stanislav Vorobiov
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

2014-05-07 Thread Cornelia Huck
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

2014-05-07 Thread Cornelia Huck
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.

2014-05-07 Thread Cornelia Huck
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

2014-05-07 Thread Cornelia Huck
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

2014-05-07 Thread Alex Bligh

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

2014-05-07 Thread Peter Maydell
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

2014-05-07 Thread Stefan Hajnoczi
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

2014-05-07 Thread Fam Zheng
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

2014-05-07 Thread Peter Maydell
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

2014-05-07 Thread Peter Maydell
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

2014-05-07 Thread Michael S. Tsirkin
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

2014-05-07 Thread Michael S. Tsirkin
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

2014-05-07 Thread Kevin Wolf
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

2014-05-07 Thread Stanislav Vorobiov
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

2014-05-07 Thread Stanislav Vorobiov
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

2014-05-07 Thread Peter Maydell
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

2014-05-07 Thread Alexander Graf


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

2014-05-07 Thread Stefan Priebe - Profihost AG
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Peter Crosthwaite

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.

2014-05-07 Thread Stefan Hajnoczi
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Peter Crosthwaite
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

2014-05-07 Thread Peter Crosthwaite
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.

2014-05-07 Thread Stefan Hajnoczi
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

2014-05-07 Thread Fam Zheng
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

2014-05-07 Thread Dr. David Alan Gilbert
* 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.

2014-05-07 Thread Stefan Hajnoczi
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

2014-05-07 Thread Kevin Wolf
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

2014-05-07 Thread Peter Maydell
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

2014-05-07 Thread Michael S. Tsirkin
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

2014-05-07 Thread Gerd Hoffmann
  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

2014-05-07 Thread Alexander Graf


 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

2014-05-07 Thread Peter Maydell
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

2014-05-07 Thread Alexander Graf


 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

2014-05-07 Thread Alexander Graf


 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

2014-05-07 Thread Gonglei (Arei)
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

2014-05-07 Thread Michael S. Tsirkin
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

2014-05-07 Thread Michael S. Tsirkin
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

2014-05-07 Thread Chunyan Liu
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

2014-05-07 Thread Chunyan Liu
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 *)

2014-05-07 Thread Chunyan Liu
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

2014-05-07 Thread Chunyan Liu
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

2014-05-07 Thread Chunyan Liu
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

2014-05-07 Thread Chunyan Liu
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

2014-05-07 Thread Chunyan Liu
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

2014-05-07 Thread Chunyan Liu
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




  1   2   3   4   >