Re: [Qemu-devel] [PATCH v2 2/8] spapr: move the IRQ server number mapping under the machine

[David Gibson]

On Thu, Mar 16, 2017 at 03:35:06PM +0100, Cédric Le Goater wrote:
> This is the second step to abstract the IRQ 'server' number of the
> XICS layer. Now that the prereq cleanups have been done in the
> previous patch, we can move down the 'cpu_dt_id' to 'cpu_index'
> mapping in the sPAPR machine handler.
> 
> Signed-off-by: Cédric Le Goater 

Reviewed-by: David Gibson 

(excepting the tiny changes that will be necessary for suggested
changes in 1/8)

> ---
>  hw/intc/xics_spapr.c| 5 ++---
>  hw/ppc/spapr.c  | 3 ++-
>  hw/ppc/spapr_cpu_core.c | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 178b3adc8af7..9574cae14944 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -52,9 +52,8 @@ static target_ulong h_cppr(PowerPCCPU *cpu, 
> sPAPRMachineState *spapr,
>  static target_ulong h_ipi(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>target_ulong opcode, target_ulong *args)
>  {
> -target_ulong server = xics_get_cpu_index_by_dt_id(args[0]);
>  target_ulong mfrr = args[1];
> -ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), server);
> +ICPState *icp = xics_icp_get(XICS_FABRIC(spapr), args[0]);
>  
>  if (!icp) {
>  return H_PARAMETER;
> @@ -122,7 +121,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, 
> sPAPRMachineState *spapr,
>  }
>  
>  nr = rtas_ld(args, 0);
> -server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
> +server = rtas_ld(args, 1);
>  priority = rtas_ld(args, 2);
>  
>  if (!ics_valid_irq(ics, nr) || !xics_icp_get(XICS_FABRIC(spapr), server)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 6ee566d658f8..396490bc5dfc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3024,9 +3024,10 @@ static void spapr_ics_resend(XICSFabric *dev)
>  ics_resend(spapr->ics);
>  }
>  
> -static ICPState *spapr_icp_get(XICSFabric *xi, int server)
> +static ICPState *spapr_icp_get(XICSFabric *xi, int cpu_dt_id)
>  {
>  sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> +int server = xics_get_cpu_index_by_dt_id(cpu_dt_id);
>  
>  return (server < spapr->nr_servers) ? >icps[server] : NULL;
>  }
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 59f1cba6fba5..1d5e7fbeeb1f 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -81,7 +81,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
> PowerPCCPU *cpu,
>  }
>  }
>  
> -cpu->icp = xics_icp_get(xi, CPU(cpu)->cpu_index);
> +cpu->icp = xics_icp_get(xi, cpu->cpu_dt_id);
>  xics_cpu_setup(xi, cpu);
>  
>  qemu_register_reset(spapr_cpu_reset, cpu);

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 5/8] ppc/pnv: create the ICP and ICS objects under the machine

[David Gibson]

On Thu, Mar 16, 2017 at 03:35:09PM +0100, Cédric Le Goater wrote:
> Like this is done for the sPAPR machine, we use a simple array under
> the PowerNV machine to store the Interrupt Control Presenters (ICP)
> objects, one for each vCPU. This array is indexed by 'cpu_index' of
> the CPUState but the users will provide a core PIR number. The mapping
> is done in the icp_get() handler of the machine and is transparent to
> XICS.

I note that you don't actually seem to be setting the cpu's icp backlink.

> We use a list to hold the different Interrupt Control Sources (ICS)
> objects, as PowerNV needs to handle multiple sources: for PCI-E and
> for the Processor Service Interface (PSI).

This is reasonable for now, but I wonder if we could use the fact we
know the platform architecture to avoid having an explicit ics list.
IIUC there's one ICS per chip (are there some extras as well?), so we
could just iterate through the existing chip array, and check each
one's ICS.

> 
> Finally, to interface with the XICS layer which manipulates the ICP
> and ICS objects, we extend the PowerNV machine with an XICSFabric
> interface and its associated handlers.
> 
> Signed-off-by: Cédric Le Goater 
> ---
> 
>  Changes since v1:
> 
>  - handled pir-to-cpu_index mapping under icp_get 
>  - removed ics_eio handler
>  - changed ICP name indexing
>  - removed sysbus parenting of the ICP object
> 
>  hw/ppc/pnv.c  | 102 
> ++
>  include/hw/ppc/pnv.h  |   4 ++
>  include/hw/ppc/xics.h |   1 +
>  3 files changed, 107 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 3fa722af82e6..6ff01c3b84d5 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -33,7 +33,10 @@
>  #include "exec/address-spaces.h"
>  #include "qemu/cutils.h"
>  #include "qapi/visitor.h"
> +#include "monitor/monitor.h"
> +#include "hw/intc/intc.h"
>  
> +#include "hw/ppc/xics.h"
>  #include "hw/ppc/pnv_xscom.h"
>  
>  #include "hw/isa/isa.h"
> @@ -417,6 +420,26 @@ static void ppc_powernv_init(MachineState *machine)
>  machine->cpu_model = "POWER8";
>  }
>  
> +/* Create the Interrupt Control Presenters before the vCPUs */
> +pnv->nr_servers = pnv->num_chips * smp_cores * smp_threads;
> +pnv->icps = g_new0(PnvICPState, pnv->nr_servers);
> +for (i = 0; i < pnv->nr_servers; i++) {
> +PnvICPState *icp = >icps[i];
> +char name[32];
> +
> +/* TODO: fix ICP object name to be in sync with the core name */
> +snprintf(name, sizeof(name), "icp[%d]", i);
> +object_initialize(icp, sizeof(*icp), TYPE_PNV_ICP);
> +object_property_add_child(OBJECT(pnv), name, OBJECT(icp),
> +  _fatal);
> +object_property_add_const_link(OBJECT(icp), "xics", OBJECT(pnv),
> +   _fatal);
> +object_property_set_bool(OBJECT(icp), true, "realized", 
> _fatal);
> +}
> +
> +/* and the list of Interrupt Control Sources */
> +QLIST_INIT(>ics);
> +
>  /* Create the processor chips */
>  chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%s", machine->cpu_model);
>  if (!object_class_by_name(chip_typename)) {
> @@ -743,6 +766,58 @@ static void pnv_get_num_chips(Object *obj, Visitor *v, 
> const char *name,
>  visit_type_uint32(v, name, _MACHINE(obj)->num_chips, errp);
>  }
>  
> +static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
> +{
> +PnvMachineState *pnv = POWERNV_MACHINE(xi);
> +ICSState *ics;
> +
> +QLIST_FOREACH(ics, >ics, list) {
> +if (ics_valid_irq(ics, irq)) {
> +return ics;
> +}
> +}
> +return NULL;
> +}
> +
> +static void pnv_ics_resend(XICSFabric *xi)
> +{
> +PnvMachineState *pnv = POWERNV_MACHINE(xi);
> +ICSState *ics;
> +
> +QLIST_FOREACH(ics, >ics, list) {
> +ics_resend(ics);
> +}
> +}
> +
> +static PowerPCCPU *ppc_get_vcpu_by_pir(int pir)
> +{
> +CPUState *cs;
> +
> +CPU_FOREACH(cs) {
> +PowerPCCPU *cpu = POWERPC_CPU(cs);
> +CPUPPCState *env = >env;
> +
> +if (env->spr_cb[SPR_PIR].default_value == pir) {
> +return cpu;
> +}
> +}
> +
> +return NULL;
> +}
> +
> +static ICPState *pnv_icp_get(XICSFabric *xi, int pir)
> +{
> +PnvMachineState *pnv = POWERNV_MACHINE(xi);
> +PowerPCCPU *cpu = ppc_get_vcpu_by_pir(pir);
> +
> +if (!cpu) {
> +return NULL;
> +}
> +
> +assert(cpu->parent_obj.cpu_index < pnv->nr_servers);
> +return ICP(>icps[cpu->parent_obj.cpu_index]);
> +}
> +
>  static void pnv_set_num_chips(Object *obj, Visitor *v, const char *name,
>void *opaque, Error **errp)
>  {
> @@ -784,9 +859,27 @@ static void powernv_machine_class_props_init(ObjectClass 
> *oc)
>NULL);
>  }
>  
> +static void pnv_pic_print_info(InterruptStatsProvider *obj,
> +   Monitor *mon)
> +{
> +

Re: [Qemu-devel] [PATCH v2 8/8] ppc/pnv: add memory regions for the ICP registers

[David Gibson]

On Thu, Mar 16, 2017 at 03:35:12PM +0100, Cédric Le Goater wrote:
> This provides to a PowerNV chip (POWER8) access to the Interrupt
> Management area, which contains the registers of the Interrupt Control
> Presenters of each thread. These are used to accept, return, forward
> interrupts in the system.
> 
> This area is modeled with a per-chip container memory region holding
> all the ICP registers. Each thread of a chip is then associated with
> its ICP registers using a memory subregion indexed by its PIR number
> in the overall region.
> 
> The device tree is populated accordingly.
> 
> Signed-off-by: Cédric Le Goater 

Reviewed-by: David Gibson 

> ---
> 
>  Changes since v1:
> 
>  - added multichip support
>  - adapted to use PnvICPState object
> 
>  hw/ppc/pnv.c | 81 
> 
>  include/hw/ppc/pnv.h | 19 
>  2 files changed, 100 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 9c239932fb3a..f91e16955fa8 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -218,6 +218,43 @@ static void powernv_create_core_node(PnvChip *chip, 
> PnvCore *pc, void *fdt)
> servers_prop, sizeof(servers_prop;
>  }
>  
> +static void powernv_populate_icp(PnvChip *chip, void *fdt, uint32_t pir,
> + uint32_t nr_threads)
> +{
> +uint64_t addr = PNV_ICP_BASE(chip) | (pir << 12);
> +char *name;
> +const char compat[] = "IBM,power8-icp\0IBM,ppc-xicp";
> +uint32_t irange[2], i, rsize;
> +uint64_t *reg;
> +int offset;
> +
> +irange[0] = cpu_to_be32(pir);
> +irange[1] = cpu_to_be32(nr_threads);
> +
> +rsize = sizeof(uint64_t) * 2 * nr_threads;
> +reg = g_malloc(rsize);
> +for (i = 0; i < nr_threads; i++) {
> +reg[i * 2] = cpu_to_be64(addr | ((pir + i) * 0x1000));
> +reg[i * 2 + 1] = cpu_to_be64(0x1000);
> +}
> +
> +name = g_strdup_printf("interrupt-controller@%"PRIX64, addr);
> +offset = fdt_add_subnode(fdt, 0, name);
> +_FDT(offset);
> +g_free(name);
> +
> +_FDT((fdt_setprop(fdt, offset, "compatible", compat, sizeof(compat;
> +_FDT((fdt_setprop(fdt, offset, "reg", reg, rsize)));
> +_FDT((fdt_setprop_string(fdt, offset, "device_type",
> +  "PowerPC-External-Interrupt-Presentation")));
> +_FDT((fdt_setprop(fdt, offset, "interrupt-controller", NULL, 0)));
> +_FDT((fdt_setprop(fdt, offset, "ibm,interrupt-server-ranges",
> +   irange, sizeof(irange;
> +_FDT((fdt_setprop_cell(fdt, offset, "#interrupt-cells", 1)));
> +_FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 0)));
> +g_free(reg);
> +}
> +
>  static void powernv_populate_chip(PnvChip *chip, void *fdt)
>  {
>  PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> @@ -231,6 +268,10 @@ static void powernv_populate_chip(PnvChip *chip, void 
> *fdt)
>  PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
>  
>  powernv_create_core_node(chip, pnv_core, fdt);
> +
> +/* Interrupt Control Presenters (ICP). One per core. */
> +powernv_populate_icp(chip, fdt, pnv_core->pir,
> + CPU_CORE(pnv_core)->nr_threads);
>  }
>  
>  if (chip->ram_size) {
> @@ -663,6 +704,38 @@ static void pnv_chip_init(Object *obj)
>  object_property_add_child(obj, "lpc", OBJECT(>lpc), NULL);
>  }
>  
> +static void pnv_chip_icp_realize(PnvChip *chip, Error **errp)
> +{
> +PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> +char *typename = pnv_core_typename(pcc->cpu_model);
> +size_t typesize = object_type_get_instance_size(typename);
> +int i, j;
> +char *name;
> +XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
> +
> +name = g_strdup_printf("icp-%x", chip->chip_id);
> +memory_region_init(>icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
> +sysbus_init_mmio(SYS_BUS_DEVICE(chip), >icp_mmio);
> +g_free(name);
> +
> +sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
> +
> +/* Map the ICP registers for each thread */
> +for (i = 0; i < chip->nr_cores; i++) {
> +PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
> +int core_hwid = CPU_CORE(pnv_core)->core_id;
> +
> +for (j = 0; j < CPU_CORE(pnv_core)->nr_threads; j++) {
> +uint32_t pir = pcc->core_pir(chip, core_hwid) + j;
> +PnvICPState *icp = PNV_ICP(xics_icp_get(xi, pir));
> +
> +memory_region_add_subregion(>icp_mmio, pir << 12, 
> >mmio);
> +}
> +}
> +
> +g_free(typename);
> +}
> +
>  static void pnv_chip_realize(DeviceState *dev, Error **errp)
>  {
>  PnvChip *chip = PNV_CHIP(dev);
> @@ -729,6 +802,14 @@ static void pnv_chip_realize(DeviceState *dev, Error 
> **errp)
>  }
>  g_free(typename);
>  
> +/* Interrupt Management Area. This is the memory region holding
> + * all the 

Re: [Qemu-devel] [PATCH v2 7/8] ppc/pnv: link the CPUs to the machine XICSFabric

[David Gibson]

On Thu, Mar 16, 2017 at 03:35:11PM +0100, Cédric Le Goater wrote:
> This assigns the ICPState object to the CPU using the PIR number for
> lookups before calling the XICS layer to finish the job.
> 
> Signed-off-by: Cédric Le Goater 

Reviewed-by: David Gibson 

> ---
>  hw/ppc/pnv.c  |  2 ++
>  hw/ppc/pnv_core.c | 20 
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 6ff01c3b84d5..9c239932fb3a 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -714,6 +714,8 @@ static void pnv_chip_realize(DeviceState *dev, Error 
> **errp)
>  object_property_set_int(OBJECT(pnv_core),
>  pcc->core_pir(chip, core_hwid),
>  "pir", _fatal);
> +object_property_add_const_link(OBJECT(pnv_core), "xics",
> +   qdev_get_machine(), _fatal);
>  object_property_set_bool(OBJECT(pnv_core), true, "realized",
>   _fatal);
>  object_unref(OBJECT(pnv_core));
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index d79d530b4881..6ec1c9c0a831 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -25,6 +25,7 @@
>  #include "hw/ppc/pnv.h"
>  #include "hw/ppc/pnv_core.h"
>  #include "hw/ppc/pnv_xscom.h"
> +#include "hw/ppc/xics.h"
>  
>  static void powernv_cpu_reset(void *opaque)
>  {
> @@ -43,7 +44,7 @@ static void powernv_cpu_reset(void *opaque)
>  env->msr |= MSR_HVB; /* Hypervisor mode */
>  }
>  
> -static void powernv_cpu_init(PowerPCCPU *cpu, Error **errp)
> +static void powernv_cpu_init(PowerPCCPU *cpu, XICSFabric *xi, Error **errp)
>  {
>  CPUPPCState *env = >env;
>  int core_pir;
> @@ -63,6 +64,9 @@ static void powernv_cpu_init(PowerPCCPU *cpu, Error **errp)
>  cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ);
>  
>  qemu_register_reset(powernv_cpu_reset, cpu);
> +
> +cpu->icp = xics_icp_get(xi, pir->default_value);
> +xics_cpu_setup(xi, cpu);
>  }
>  
>  /*
> @@ -110,7 +114,7 @@ static const MemoryRegionOps pnv_core_xscom_ops = {
>  .endianness = DEVICE_BIG_ENDIAN,
>  };
>  
> -static void pnv_core_realize_child(Object *child, Error **errp)
> +static void pnv_core_realize_child(Object *child, XICSFabric *xi, Error 
> **errp)
>  {
>  Error *local_err = NULL;
>  CPUState *cs = CPU(child);
> @@ -122,7 +126,7 @@ static void pnv_core_realize_child(Object *child, Error 
> **errp)
>  return;
>  }
>  
> -powernv_cpu_init(cpu, _err);
> +powernv_cpu_init(cpu, xi, _err);
>  if (local_err) {
>  error_propagate(errp, local_err);
>  return;
> @@ -140,6 +144,14 @@ static void pnv_core_realize(DeviceState *dev, Error 
> **errp)
>  void *obj;
>  int i, j;
>  char name[32];
> +Object *xi;
> +
> +xi = object_property_get_link(OBJECT(dev), "xics", _err);
> +if (!xi) {
> +error_setg(errp, "%s: required link 'xics' not found: %s",
> +   __func__, error_get_pretty(local_err));
> +return;
> +}
>  
>  pc->threads = g_malloc0(size * cc->nr_threads);
>  for (i = 0; i < cc->nr_threads; i++) {
> @@ -160,7 +172,7 @@ static void pnv_core_realize(DeviceState *dev, Error 
> **errp)
>  for (j = 0; j < cc->nr_threads; j++) {
>  obj = pc->threads + j * size;
>  
> -pnv_core_realize_child(obj, _err);
> +pnv_core_realize_child(obj, XICS_FABRIC(xi), _err);
>  if (local_err) {
>  goto err;
>  }

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 6/8] ppc/pnv: add a helper to calculate MMIO addresses registers

[David Gibson]

On Thu, Mar 16, 2017 at 03:35:10PM +0100, Cédric Le Goater wrote:
> Some controllers (ICP, PSI) have a base register address which is
> calculated using the chip id.
> 
> Signed-off-by: Cédric Le Goater 

Reviewed-by: David Gibson 

> ---
>  include/hw/ppc/pnv.h | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index d6ef04771aff..cfd059fc49db 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -92,14 +92,24 @@ typedef struct PnvChipClass {
>  OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER9)
>  
>  /*
> - * This generates a HW chip id depending on an index:
> + * This generates a HW chip id depending on an index, as found on a
> + * two socket system with dual chip modules :
>   *
>   *0x0, 0x1, 0x10, 0x11
>   *
>   * 4 chips should be the maximum
> + *
> + * TODO: use a machine property to define the chip ids
>   */
>  #define PNV_CHIP_HWID(i) i) & 0x3e) << 3) | ((i) & 0x1))
>  
> +/*
> + * Converts back a HW chip id to an index. This is useful to calculate
> + * the MMIO addresses of some controllers which depend on the chip id.
> + */
> +#define PNV_CHIP_INDEX(chip)\
> +(((chip)->chip_id >> 2) * 2 + ((chip)->chip_id & 0x3))
> +
>  #define TYPE_POWERNV_MACHINE   MACHINE_TYPE_NAME("powernv")
>  #define POWERNV_MACHINE(obj) \
>  OBJECT_CHECK(PnvMachineState, (obj), TYPE_POWERNV_MACHINE)

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 3/8] ppc/xics: add a realize() handler to ICPStateClass

[David Gibson]

On Thu, Mar 16, 2017 at 03:35:07PM +0100, Cédric Le Goater wrote:
> It will be used by derived classes in PowerNV for customization.
> 
> Signed-off-by: Cédric Le Goater 

Reviewed-by: David Gibson 

> ---
>  hw/intc/xics.c| 5 +
>  include/hw/ppc/xics.h | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 5cde86ceb3bc..7cd842102265 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -348,6 +348,7 @@ static void icp_reset(void *dev)
>  static void icp_realize(DeviceState *dev, Error **errp)
>  {
>  ICPState *icp = ICP(dev);
> +ICPStateClass *icpc = ICP_GET_CLASS(dev);
>  Object *obj;
>  Error *err = NULL;
>  
> @@ -360,6 +361,10 @@ static void icp_realize(DeviceState *dev, Error **errp)
>  
>  icp->xics = XICS_FABRIC(obj);
>  
> +if (icpc->realize) {
> +icpc->realize(dev, errp);
> +}
> +
>  qemu_register_reset(icp_reset, dev);
>  }
>  
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 9a5e715fe553..0863e3a079f5 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -60,6 +60,7 @@ typedef struct XICSFabric XICSFabric;
>  struct ICPStateClass {
>  DeviceClass parent_class;
>  
> +void (*realize)(DeviceState *dev, Error **errp);
>  void (*pre_save)(ICPState *s);
>  int (*post_load)(ICPState *s, int version_id);
>  void (*cpu_setup)(ICPState *icp, PowerPCCPU *cpu);

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 4/8] ppc/pnv: add a PnvICPState object

[David Gibson]

On Thu, Mar 16, 2017 at 03:35:08PM +0100, Cédric Le Goater wrote:
> This provides a new ICPState object for the PowerNV machine (POWER8).
> Access to the Interrupt Management area is done though a memory
> region. It contains the registers of the Interrupt Control Presenters
> of each thread which are used to accept, return, forward interrupts in
> the system.
> 
> Signed-off-by: Cédric Le Goater 

Revieed-by: David Gibson 

> ---
> 
>  Changes since v1:
> 
>  - moved the memory region from PnvCore to a specific PnvICPState object
> 
>  hw/intc/Makefile.objs |   1 +
>  hw/intc/xics_pnv.c| 180 
> ++
>  include/hw/ppc/xics.h |  12 
>  3 files changed, 193 insertions(+)
>  create mode 100644 hw/intc/xics_pnv.c
> 
> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> index adedd0da5fd8..78426a7dafcd 100644
> --- a/hw/intc/Makefile.objs
> +++ b/hw/intc/Makefile.objs
> @@ -35,6 +35,7 @@ obj-$(CONFIG_SH4) += sh_intc.o
>  obj-$(CONFIG_XICS) += xics.o
>  obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o
>  obj-$(CONFIG_XICS_KVM) += xics_kvm.o
> +obj-$(CONFIG_POWERNV) += xics_pnv.o
>  obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
>  obj-$(CONFIG_S390_FLIC) += s390_flic.o
>  obj-$(CONFIG_S390_FLIC_KVM) += s390_flic_kvm.o
> diff --git a/hw/intc/xics_pnv.c b/hw/intc/xics_pnv.c
> new file mode 100644
> index ..68a3ef6097a6
> --- /dev/null
> +++ b/hw/intc/xics_pnv.c
> @@ -0,0 +1,180 @@
> +/*
> + * QEMU PowerPC PowerNV ICP model
> + *
> + * Copyright (c) 2016, IBM Corporation.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "sysemu/sysemu.h"
> +#include "qapi/error.h"
> +#include "qemu/log.h"
> +#include "hw/ppc/xics.h"
> +
> +static uint64_t pnv_icp_read(void *opaque, hwaddr addr, unsigned width)
> +{
> +ICPState *icp = ICP(opaque);
> +PnvICPState *picp = PNV_ICP(opaque);
> +bool byte0 = (width == 1 && (addr & 0x3) == 0);
> +uint64_t val = 0x;
> +
> +switch (addr & 0xffc) {
> +case 0: /* poll */
> +val = icp_ipoll(icp, NULL);
> +if (byte0) {
> +val >>= 24;
> +} else if (width != 4) {
> +goto bad_access;
> +}
> +break;
> +case 4: /* xirr */
> +if (byte0) {
> +val = icp_ipoll(icp, NULL) >> 24;
> +} else if (width == 4) {
> +val = icp_accept(icp);
> +} else {
> +goto bad_access;
> +}
> +break;
> +case 12:
> +if (byte0) {
> +val = icp->mfrr;
> +} else {
> +goto bad_access;
> +}
> +break;
> +case 16:
> +if (width == 4) {
> +val = picp->links[0];
> +} else {
> +goto bad_access;
> +}
> +break;
> +case 20:
> +if (width == 4) {
> +val = picp->links[1];
> +} else {
> +goto bad_access;
> +}
> +break;
> +case 24:
> +if (width == 4) {
> +val = picp->links[2];
> +} else {
> +goto bad_access;
> +}
> +break;
> +default:
> +bad_access:
> +qemu_log_mask(LOG_GUEST_ERROR, "XICS: Bad ICP access 0x%"
> +  HWADDR_PRIx"/%d\n", addr, width);
> +}
> +
> +return val;
> +}
> +
> +static void pnv_icp_write(void *opaque, hwaddr addr, uint64_t val,
> +  unsigned width)
> +{
> +ICPState *icp = ICP(opaque);
> +PnvICPState *picp = PNV_ICP(opaque);
> +bool byte0 = (width == 1 && (addr & 0x3) == 0);
> +
> +switch (addr & 0xffc) {
> +case 4: /* xirr */
> +if (byte0) {
> +icp_set_cppr(icp, val);
> +} else if (width == 4) {
> +icp_eoi(icp, val);
> +} else {
> +goto bad_access;
> +}
> +break;
> +case 12:
> +if (byte0) {
> +icp_set_mfrr(icp, val);
> +} else {
> +goto bad_access;
> +}
> +break;
> +case 16:
> +if (width == 4) {
> +picp->links[0] = val;
> +} else {
> +goto bad_access;
> +}
> +break;
> +case 20:
> +if (width == 4) {
> +

Re: [Qemu-devel] [PATCH v2 3/3] vfio-pci: process non fatal error of AER

[Cao jin]

On 03/22/2017 09:27 PM, Michael S. Tsirkin wrote:
> On Wed, Mar 22, 2017 at 06:36:52PM +0800, Cao jin wrote:
>> Make use of the non fatal error eventfd that the kernel module provide
>> to process the AER non fatal error. Fatal error still goes into the
>> legacy way which results in VM stop.
>>
>> Register the handler, wait for notification. Construct aer message and
>> pass it to root port on notification. Root port will trigger an interrupt
>> to signal guest, then guest driver will do the recovery.
>>
>> Signed-off-by: Dou Liyang 
>> Signed-off-by: Cao jin 
>> ---
>>  hw/vfio/pci.c  | 247 
>> +
>>  hw/vfio/pci.h  |   4 +
>>  linux-headers/linux/vfio.h |   2 +
>>  3 files changed, 253 insertions(+)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 3d0d005..4912bc6 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -2422,6 +2422,34 @@ static void vfio_populate_device(VFIOPCIDevice *vdev, 
>> Error **errp)
>>   "Could not enable error recovery for the device",
>>   vbasedev->name);
>>  }
>> +
>> +irq_info.index = VFIO_PCI_NON_FATAL_ERR_IRQ_INDEX;
>> +irq_info.count = 0; /* clear */
>> +ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, _info);
>> +if (ret) {
>> +/* This can fail for an old kernel or legacy PCI dev */
>> +trace_vfio_populate_device_get_irq_info_failure();
>> +} else if (irq_info.count == 1) {
>> +vdev->pci_aer_non_fatal = true;
>> +} else {
>> +error_report(WARN_PREFIX
>> + "Couldn't enable non fatal error recovery for the 
>> device",
>> + vbasedev->name);
> 
> when does this trigger?
> 
>> +}
>> +
>> +irq_info.index = VFIO_PCI_PASSIVE_RESET_IRQ_INDEX;
>> +irq_info.count = 0; /* clear */
>> +ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, _info);
>> +if (ret) {
>> +/* This can fail for an old kernel or legacy PCI dev */
>> +trace_vfio_populate_device_get_irq_info_failure();
>> +} else if (irq_info.count == 1) {
>> +vdev->passive_reset = true;
>> +} else {
>> +error_report(WARN_PREFIX
>> + "Don't support passive reset notification",
>> + vbasedev->name);
> 
> when does this happen?
> what does this message mean?
> 

Both are boilerplate code as err_notifier. They will be triggered by
running latest QEMU on older kernel.  Will drop these code & the flags

>> +}
>>  }
>>  
>>  static void vfio_put_device(VFIOPCIDevice *vdev)
>> @@ -2432,6 +2460,221 @@ static void vfio_put_device(VFIOPCIDevice *vdev)
>>  vfio_put_base_device(>vbasedev);
>>  }
>>  
>> +static void vfio_non_fatal_err_notifier_handler(void *opaque)
>> +{
>> +VFIOPCIDevice *vdev = opaque;
>> +PCIDevice *dev = >pdev;
>> +PCIEAERMsg msg = {
>> +.severity = PCI_ERR_ROOT_CMD_NONFATAL_EN,
>> +.source_id = (pci_bus_num(dev->bus) << 8) | dev->devfn,
>> +};
>> +
> 
> Should this just use pci_requester_id?
> 
> 
> At least Peter thought so.
> 
>> +if (!event_notifier_test_and_clear(>non_fatal_err_notifier)) {
>> +return;
>> +}
>> +
>> +/* Populate the aer msg and send it to root port */
>> +if (dev->exp.aer_cap) {
>> +uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
>> +uint32_t uncor_status;
>> +bool isfatal;
>> +
>> +uncor_status = vfio_pci_read_config(dev,
>> +dev->exp.aer_cap + PCI_ERR_UNCOR_STATUS, 4);
>> +if (!uncor_status) {
>> +return;
>> +}
>> +
>> +isfatal = uncor_status & pci_get_long(aer_cap + 
>> PCI_ERR_UNCOR_SEVER);
>> +if (isfatal) {
>> +goto stop;
>> +}
>> +
>> +error_report("%s sending non fatal event to root port. uncor status 
>> = "
>> + "0x%"PRIx32, vdev->vbasedev.name, uncor_status);
>> +pcie_aer_msg(dev, );
>> +return;
>> +}
>> +
>> +stop:
>> +/* Terminate the guest in case of fatal error */
>> +error_report("%s(%s) fatal error detected. Please collect any data"
>> +" possible and then kill the guest", __func__, 
>> vdev->vbasedev.name);
> 
> "Device detected a fatal error. VM stopped".
> 
> would be better IMO.
> 

Yes, user has no way to collect any data.

>> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
>> index 34e8b04..dcb0e0a 100644
>> --- a/hw/vfio/pci.h
>> +++ b/hw/vfio/pci.h
>> @@ -119,6 +119,8 @@ typedef struct VFIOPCIDevice {
>>  void *igd_opregion;
>>  PCIHostDeviceAddress host;
>>  EventNotifier err_notifier;
>> +EventNotifier non_fatal_err_notifier;
>> +EventNotifier passive_reset_notifier;
>>  EventNotifier req_notifier;
>>  int (*resetfn)(struct VFIOPCIDevice *);
>>  uint32_t vendor_id;
>> @@ -137,6 +139,8 @@ typedef struct 

[Qemu-devel] [Resend PATCH] migration: Fix colo hang in socket_accept_incoming_migration

[Wang Guang]

Due to the feature of accept channel does not support 
QIO_CHANNEL_FEATURE_SHUTDOWN.
when failover,channel_shutdown could not shut down the channel.
So the colo_process_incoming_thread will hang at recvmsg.
This patch just call qio_channel_socket_new to get channel,
Which set QIO_CHANNEL_FEATURE_SHUTDOWN already.

Signed-off-by: Wang Guang
Signed-off-by: zhanghailiang 
---
 io/channel-socket.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/io/channel-socket.c b/io/channel-socket.c
index f546c68..8344da4 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -331,16 +331,10 @@ qio_channel_socket_accept(QIOChannelSocket *ioc,
 {
 QIOChannelSocket *cioc;
 
-cioc = QIO_CHANNEL_SOCKET(object_new(TYPE_QIO_CHANNEL_SOCKET));
-cioc->fd = -1;
+cioc = qio_channel_socket_new();
 cioc->remoteAddrLen = sizeof(ioc->remoteAddr);
 cioc->localAddrLen = sizeof(ioc->localAddr);
 
-#ifdef WIN32
-QIO_CHANNEL(cioc)->event = CreateEvent(NULL, FALSE, FALSE, NULL);
-#endif
-
-
  retry:
 trace_qio_channel_socket_accept(ioc);
 cioc->fd = qemu_accept(ioc->fd, (struct sockaddr *)>remoteAddr,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] Fix colo hang in socket_accept_incoming_migration

[no-reply]

Hi,

This series failed build test on s390x host. Please find the details below.

Message-id: 1490236226-30248-1-git-send-email-wang.guan...@zte.com.cn
Type: series
Subject: [Qemu-devel] [PATCH] Fix colo hang in socket_accept_incoming_migration

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
echo -n "Using CC: "
realpath $CC
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] 
patchew/1490236226-30248-1-git-send-email-wang.guan...@zte.com.cn -> 
patchew/1490236226-30248-1-git-send-email-wang.guan...@zte.com.cn
 - [tag update]  patchew/20170322204844.446-1-f4...@amsat.org -> 
patchew/20170322204844.446-1-f4...@amsat.org
 - [tag update]  patchew/20170322210005.16533-1-kw...@redhat.com -> 
patchew/20170322210005.16533-1-kw...@redhat.com
Switched to a new branch 'test'
a583148 Fix colo hang in socket_accept_incoming_migration

=== OUTPUT BEGIN ===
=== ENV ===
XDG_SESSION_ID=73323
SHELL=/bin/sh
USER=fam
PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug
PATH=/usr/bin:/bin
PWD=/var/tmp/patchew-tester-tmp-13i7__w8/src
LANG=en_US.UTF-8
HOME=/home/fam
SHLVL=2
LOGNAME=fam
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus
XDG_RUNTIME_DIR=/run/user/1012
_=/usr/bin/env
=== PACKAGES ===
gpg-pubkey-873529b8-54e386ff
xz-libs-5.2.2-2.fc24.s390x
libacl-2.2.52-11.fc24.s390x
libxshmfence-1.2-3.fc24.s390x
cdparanoia-libs-10.2-21.fc24.s390x
ustr-1.0.4-21.fc24.s390x
giflib-4.1.6-15.fc24.s390x
libusb-0.1.5-7.fc24.s390x
trousers-lib-0.3.13-6.fc24.s390x
readline-devel-6.3-8.fc24.s390x
python-srpm-macros-3-10.fc25.noarch
ncurses-base-6.0-6.20160709.fc25.noarch
gmp-6.1.1-1.fc25.s390x
chkconfig-1.8-1.fc25.s390x
libidn-1.33-1.fc25.s390x
file-5.28-4.fc25.s390x
slang-2.3.0-7.fc25.s390x
avahi-libs-0.6.32-4.fc25.s390x
libsemanage-2.5-8.fc25.s390x
perl-Unicode-Normalize-1.25-365.fc25.s390x
perl-libnet-3.10-1.fc25.noarch
perl-Thread-Queue-3.11-1.fc25.noarch
perl-podlators-4.09-1.fc25.noarch
jasper-libs-1.900.13-1.fc25.s390x
graphite2-1.3.6-1.fc25.s390x
libblkid-2.28.2-1.fc25.s390x
pkgconfig-0.29.1-1.fc25.s390x
dbus-python-1.2.4-2.fc25.s390x
alsa-lib-1.1.1-2.fc25.s390x
libgnome-keyring-3.12.0-7.fc25.s390x
yum-metadata-parser-1.1.4-17.fc25.s390x
python3-3.5.2-4.fc25.s390x
python3-slip-dbus-0.6.4-4.fc25.noarch
python2-cssselect-0.9.2-1.fc25.noarch
python-backports-1.0-8.fc25.s390x
python-magic-5.28-4.fc25.noarch
python-pycparser-2.14-7.fc25.noarch
python-fedora-0.8.0-2.fc25.noarch
createrepo_c-libs-0.10.0-6.fc25.s390x
initscripts-9.69-1.fc25.s390x
plymouth-scripts-0.9.3-0.6.20160620git0e65b86c.fc25.s390x
cronie-1.5.1-2.fc25.s390x
python2-librepo-1.7.18-3.fc25.s390x
wget-1.18-2.fc25.s390x
python3-dnf-plugins-core-0.1.21-4.fc25.noarch
at-spi2-core-2.22.0-1.fc25.s390x
libXv-1.0.11-1.fc25.s390x
dhcp-client-4.3.5-1.fc25.s390x
python2-dnf-plugins-core-0.1.21-4.fc25.noarch
parted-3.2-21.fc25.s390x
python2-ndg_httpsclient-0.4.0-4.fc25.noarch
bash-completion-2.4-1.fc25.noarch
btrfs-progs-4.6.1-1.fc25.s390x
texinfo-6.1-3.fc25.s390x
perl-Filter-1.55-366.fc25.s390x
flex-2.6.0-3.fc25.s390x
libgcc-6.3.1-1.fc25.s390x
glib2-2.50.2-1.fc25.s390x
dbus-libs-1.11.8-1.fc25.s390x
libgomp-6.3.1-1.fc25.s390x
colord-libs-1.3.4-1.fc25.s390x
perl-Encode-2.88-5.fc25.s390x
gstreamer1-1.10.2-1.fc25.s390x
cracklib-2.9.6-4.fc25.s390x
rpm-build-libs-4.13.0-6.fc25.s390x
libobjc-6.3.1-1.fc25.s390x
pcre-devel-8.40-1.fc25.s390x
mariadb-config-10.1.20-1.fc25.s390x
gcc-6.3.1-1.fc25.s390x
mesa-libGL-13.0.3-1.fc25.s390x
python3-dnf-plugin-system-upgrade-0.7.1-4.fc25.noarch
bind-libs-9.10.4-4.P5.fc25.s390x
python-osbs-client-0.33-3.fc25.noarch
NetworkManager-1.4.4-3.fc25.s390x
audit-2.7.1-1.fc25.s390x
glibc-static-2.24-4.fc25.s390x
perl-Pod-Simple-3.35-1.fc25.noarch
gdb-7.12-36.fc25.s390x
python2-simplejson-3.10.0-1.fc25.s390x
python3-sssdconfig-1.14.2-2.fc25.noarch
texlive-lib-2016-30.20160520.fc25.s390x
boost-random-1.60.0-10.fc25.s390x
brltty-5.4-2.fc25.s390x
libref_array-0.1.5-29.fc25.s390x
librados2-10.2.4-2.fc25.s390x
gnutls-dane-3.5.8-1.fc25.s390x
systemtap-client-3.1-0.20160725git91bfb36.fc25.s390x
libXrender-devel-0.9.10-1.fc25.s390x
libXi-devel-1.7.8-2.fc25.s390x
texlive-pdftex-doc-svn41149-30.fc25.noarch
tcp_wrappers-7.6-83.fc25.s390x
javapackages-tools-4.7.0-6.1.fc25.noarch
texlive-kpathsea-bin-svn40473-30.20160520.fc25.s390x
texlive-url-svn32528.3.4-30.fc25.noarch
texlive-latex-fonts-svn2.0-30.fc25.noarch
texlive-mptopdf-bin-svn18674.0-30.20160520.fc25.noarch
texlive-underscore-svn18261.0-30.fc25.noarch

[Qemu-devel] [PATCH] Fix colo hang in socket_accept_incoming_migration

[Wang Guang]

Due to the feature of accept channel does not support 
QIO_CHANNEL_FEATURE_SHUTDOWN.
when failover,channel_shutdown could not shut down the channel.
So the colo_process_incoming_thread will hang at recvmsg.
This patch just call qio_channel_socket_new to get channel,
Which set QIO_CHANNEL_FEATURE_SHUTDOWN already.

Signed-off-by: Wang Guang
Signed-off-by: zhanghailiang 
---
 io/channel-socket.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/io/channel-socket.c b/io/channel-socket.c
index f546c68..8344da4 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -331,16 +331,10 @@ qio_channel_socket_accept(QIOChannelSocket *ioc,
 {
 QIOChannelSocket *cioc;
 
-cioc = QIO_CHANNEL_SOCKET(object_new(TYPE_QIO_CHANNEL_SOCKET));
-cioc->fd = -1;
+cioc = qio_channel_socket_new()
 cioc->remoteAddrLen = sizeof(ioc->remoteAddr);
 cioc->localAddrLen = sizeof(ioc->localAddr);
 
-#ifdef WIN32
-QIO_CHANNEL(cioc)->event = CreateEvent(NULL, FALSE, FALSE, NULL);
-#endif
-
-
  retry:
 trace_qio_channel_socket_accept(ioc);
 cioc->fd = qemu_accept(ioc->fd, (struct sockaddr *)>remoteAddr,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v5] vfio error recovery: kernel support

[Cao jin]

On 03/22/2017 09:10 PM, Michael S. Tsirkin wrote:
> Minor comments on commit log below.
> 
> On Wed, Mar 22, 2017 at 06:34:23PM +0800, Cao jin wrote:
>> From: "Michael S. Tsirkin" 
>>

> 
>> Signed-off-by: Michael S. Tsirkin 
>> Signed-off-by: Cao jin 
>> ---
>>
>> v5 changelog:
>> 1. Add another new eventfd passive_reset_trigger & the boilerplate code,
>>used in slot_reset. Add comment for slot_reset().
>> 2. Rewrite the commit log.
>>
>>  drivers/vfio/pci/vfio_pci.c | 49 
>> +++--
>>  drivers/vfio/pci/vfio_pci_intrs.c   | 38 
>>  drivers/vfio/pci/vfio_pci_private.h |  2 ++
>>  include/uapi/linux/vfio.h   |  2 ++
>>  4 files changed, 89 insertions(+), 2 deletions(-)
>>

> 
> 
>> +static pci_ers_result_t vfio_pci_aer_slot_reset(struct pci_dev *pdev)
>> +{
>> +struct vfio_pci_device *vdev;
>> +struct vfio_device *device;
>> +static pci_ers_result_t err = PCI_ERS_RESULT_NONE;
>> +
>> +device = vfio_device_get_from_dev(>dev);
>> +if (!device)
>> +goto err_dev;
>> +
>> +vdev = vfio_device_data(device);
>> +if (!vdev)
>> +goto err_data;
>> +
>> +mutex_lock(>igate);
>> +
>> +if (vdev->passive_reset_trigger)
>> +eventfd_signal(vdev->passive_reset_trigger, 1);
>> +else if (vdev->err_trigger)
>> +eventfd_signal(vdev->err_trigger, 1);
> 
> why is this chunk here? why not just do
> 
>   if (vdev->passive_reset_trigger)
>   eventfd_signal(vdev->passive_reset_trigger, 1);
> 
> without a fallback?
> 
> 

I thought it is one way of "passing maximum info to userspace and let it
decide."

-- 
Sincerely,
Cao jin

[Qemu-devel] [Bug 1675108] [NEW] Cocoa UI always crashes on startup

[Brendan Shanks]

Public bug reported:

Commit 8bb93c6f99a42c2e0943bc904b283cd622d302c5 ("ui/console: ensure
graphic updates don't race with TCG vCPUs") causes the graphic update to
run on a non-main thread, which Cocoa is not happy with. It crashes
immediately after startup.

$ i386-softmmu/qemu-system-i386 
2017-03-22 10:09:25.113 qemu-system-i386[15968:9538245] *** Terminating app due 
to uncaught exception 'NSInternalInconsistencyException', reason: 
'nextEventMatchingMask should only be called from the Main Thread!'
*** First throw call stack:
(
0   CoreFoundation  0x7fff91e72e7b 
__exceptionPreprocess + 171
1   libobjc.A.dylib 0x7fffa6a5ccad 
objc_exception_throw + 48
2   AppKit  0x7fff900953fd 
-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] 
+ 4471
3   qemu-system-i3860x000104f75a49 
cocoa_refresh + 233
4   qemu-system-i3860x000104e0312c 
process_queued_cpu_work + 140
5   qemu-system-i3860x000104d1a73c 
qemu_tcg_rr_cpu_thread_fn + 284
6   libsystem_pthread.dylib 0x7fffa7557aab 
_pthread_body + 180
7   libsystem_pthread.dylib 0x7fffa75579f7 
_pthread_body + 0
8   libsystem_pthread.dylib 0x7fffa75571fd thread_start 
+ 13
)
libc++abi.dylib: terminating with uncaught exception of type NSException
Abort trap: 6

System: macOS 10.12.3, Xcode 8.2.1

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1675108

Title:
  Cocoa UI always crashes on startup

Status in QEMU:
  New

Bug description:
  Commit 8bb93c6f99a42c2e0943bc904b283cd622d302c5 ("ui/console: ensure
  graphic updates don't race with TCG vCPUs") causes the graphic update
  to run on a non-main thread, which Cocoa is not happy with. It crashes
  immediately after startup.

  $ i386-softmmu/qemu-system-i386 
  2017-03-22 10:09:25.113 qemu-system-i386[15968:9538245] *** Terminating app 
due to uncaught exception 'NSInternalInconsistencyException', reason: 
'nextEventMatchingMask should only be called from the Main Thread!'
  *** First throw call stack:
  (
0   CoreFoundation  0x7fff91e72e7b 
__exceptionPreprocess + 171
1   libobjc.A.dylib 0x7fffa6a5ccad 
objc_exception_throw + 48
2   AppKit  0x7fff900953fd 
-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] 
+ 4471
3   qemu-system-i3860x000104f75a49 
cocoa_refresh + 233
4   qemu-system-i3860x000104e0312c 
process_queued_cpu_work + 140
5   qemu-system-i3860x000104d1a73c 
qemu_tcg_rr_cpu_thread_fn + 284
6   libsystem_pthread.dylib 0x7fffa7557aab 
_pthread_body + 180
7   libsystem_pthread.dylib 0x7fffa75579f7 
_pthread_body + 0
8   libsystem_pthread.dylib 0x7fffa75571fd thread_start 
+ 13
  )
  libc++abi.dylib: terminating with uncaught exception of type NSException
  Abort trap: 6

  System: macOS 10.12.3, Xcode 8.2.1

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1675108/+subscriptions

[Qemu-devel] help please how to use qemu.

[Richard Odell]

step by step instructions please

Re: [Qemu-devel] [PATCH] cryptodev: fix asserting single queue

[Gonglei (Arei)]

> 
> 
> On 03/22/2017 01:36 PM, Halil Pasic wrote:
> > We already check for queues == 1 in cryptodev_builtin_init and when that
> > is not true raise an error. But before that error is reported the
> > assertion in cryptodev_builtin_cleanup kicks in (because object is being
> > finalized and freed).
> >
> > Let's remove assert(queues == 1) form cryptodev_builtin_cleanup as it
> > does only harm and no good.
> >
> > Signed-off-by: Halil Pasic 
> 
> Sorry guys, I forgot to give credit to the reporter.
> 
> Reported-by: Boris Fiuczynski 

Applied, thanks!


> > ---
> >  backends/cryptodev-builtin.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
> > index 82a068e..137c7a6 100644
> > --- a/backends/cryptodev-builtin.c
> > +++ b/backends/cryptodev-builtin.c
> > @@ -359,8 +359,6 @@ static void cryptodev_builtin_cleanup(
> >  }
> >  }
> >
> > -assert(queues == 1);
> > -
> >  for (i = 0; i < queues; i++) {
> >  cc = backend->conf.peers.ccs[i];
> >  if (cc) {
> >

Re: [Qemu-devel] [PATCH for-2.9?] file-posix: Make bdrv_flush() failure permanent without O_DIRECT

[Eric Blake]

On 03/22/2017 04:00 PM, Kevin Wolf wrote:
> Success for bdrv_flush() means that all previously written data is safe
> on disk. For fdatasync(), the best semantics we can hope for on Linux
> (without O_DIRECT) is that all data that was written since the last call
> was successfully written back. Therefore, and because we can't redo all
> writes after a flush failure, we have to give up after a single
> fdatasync() failure. After this failure, we would never be able to make
> the promise that a successful bdrv_flush() makes.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/file-posix.c | 22 ++
>  1 file changed, 22 insertions(+)

Makes sense for 2.9 (it doesn't change the data loss, but alerts to the
user to the knowledge of data loss a lot sooner, perhaps before things
get even worse).

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"

[ashish mittal]

On Mon, Mar 20, 2017 at 5:55 AM, Daniel P. Berrange  wrote:
> On Fri, Mar 17, 2017 at 06:44:56PM -0700, ashish mittal wrote:
>> On Thu, Mar 16, 2017 at 5:29 PM, ashish mittal  wrote:
>> > On Mon, Mar 13, 2017 at 2:57 AM, Daniel P. Berrange  
>> > wrote:
>> >> On Tue, Mar 07, 2017 at 05:27:55PM -0800, ashish mittal wrote:
>> >>> Thanks! There is one more input I need some help with!
>> >>>
>> >>> VxHS network library opens a fixed number of connection channels to a
>> >>> given host, and all the vdisks (that connect to the same host) share
>> >>> these connection channels.
>> >>>
>> >>> Therefore, we need to open secure channels to a specific target host
>> >>> only once for the first vdisk that connects to that host. All the
>> >>> other vdisks that connect to the same target host will share the same
>> >>> set of secure communication channels.
>> >>>
>> >>> I hope the above scheme is acceptable?
>> >>>
>> >>> If yes, then we have a couple of options to implement this:
>> >>>
>> >>> (1) Accept the TLS credentials per vdisk using the previously
>> >>> discussed --object tls-creds-x509 mechanism. In this case, if more
>> >>> than one vdisk have the same host info, then we will use only the
>> >>> first one's creds to set up the secure connection, and ignore the
>> >>> others. vdisks that connect to different target hosts will use their
>> >>> individual tls-creds-x509 to set up the secure channels. This is, of
>> >>> course, not relevant for qemu-img/qemu-io as they can open only one
>> >>> vdisk at a time.
>> >>
>> >> It looks like option 1 here is the way to go. Just report an error if
>> >> there are multiple creds provided for the same host and they don't
>> >> match.
>> >>
>> >
>> > I have made changes to implement option 1 in the library (branch
>> > per_channel_ssl).
>> > Can you please help review it?
>> > https://github.com/VeritasHyperScale/libqnio/compare/per_channel_ssl
>> >
>> > Here's the changelog:
>> > (1) Changed code to not use instance UUID for setting up SSL context.
>> > (2) User is supposed to pass the cacert, client_key and client_cert
>> > files to iio_open(). These will be used to set up a per-channel secure 
>> > SSL
>> > connection to the server. All three values are needed to set up a
>> > secure connection.
>> > (3) If the secure channel to a particular host is already open, other
>> > block device connections to the same host will have to provide
>> > TLS/SSL credentials that match the original one.
>> > (4) Set default locations for trusted client CA certificates
>> >  based on user specified cacert file.
>> >
>> > NB - I have given steps to test SSL communication (using the supplied
>> > test client/server programs) in the commit log. I have not tested
>> > using qemu binary yet. Will run more tests in the days to come.
>> >
>> > qemu vxhs patch changes should be pretty straightforward, given that
>> > we have already discussed how to implement passing --object
>> > tls-creds-x509 per block device.
>> >
>> > Thanks!
>> >
>>
>> Update -
>> (1) Successfully tested SSL communication using qemu-io and the test server.
>> (2) Minor changes to per_channel_ssl branch.
>> (3) Created a pull request.
>>
>> Please review per convenience. Thanks!
>
> IIUC, on that branch the 'is_secure()' method is still looking for the
> directory /var/lib/libvxhs/secure to exist on the host. If that is not
> present, then it appears to be silently ignoring the SSL certs passed
> in from QEMU.
>
> IMHO it should enable TLS when 'cacert' passed to iio_open is not NULL,
> not relying on a magic directory to exist.
>

I have made changes per above to the library. Please see commits -

https://github.com/VeritasHyperScale/libqnio/commit/6c3261e9c9bb1350f4433a1ae4fcd98f7692cf39
https://github.com/VeritasHyperScale/libqnio/commit/502c74278457e6ac86a4ee4ad9102e56ff3be5d4

Commit log: Enable secure mode based on the SSL/TLS args passed in iio_open()

(1) Do not use /var/lib/libvxhs/secure to enable secure SSL mode on
the client side.
(2) Instead, enable SSL mode if the user passes TLS/SSL creds for
the block device on the qemu CLI.

Will be posting v10 qemu vxhs patch soon. v10 will work with the
latest library changes, and will support passing tls-creds-x509 creds
for every vxhs block device.


> Regards,
> Daniel
> --
> |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org  -o- http://virt-manager.org :|
> |: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH for-2.9?] file-posix: Make bdrv_flush() failure permanent without O_DIRECT

[Fam Zheng]

On Wed, 03/22 22:00, Kevin Wolf wrote:
> Success for bdrv_flush() means that all previously written data is safe
> on disk. For fdatasync(), the best semantics we can hope for on Linux
> (without O_DIRECT) is that all data that was written since the last call
> was successfully written back. Therefore, and because we can't redo all
> writes after a flush failure, we have to give up after a single
> fdatasync() failure. After this failure, we would never be able to make
> the promise that a successful bdrv_flush() makes.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/file-posix.c | 22 ++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 53febd3..beb7a4f 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -144,6 +144,7 @@ typedef struct BDRVRawState {
>  bool has_write_zeroes:1;
>  bool discard_zeroes:1;
>  bool use_linux_aio:1;
> +bool page_cache_inconsistent:1;
>  bool has_fallocate;
>  bool needs_alignment;
>  } BDRVRawState;
> @@ -824,10 +825,31 @@ static ssize_t handle_aiocb_ioctl(RawPosixAIOData 
> *aiocb)
>  
>  static ssize_t handle_aiocb_flush(RawPosixAIOData *aiocb)
>  {
> +BDRVRawState *s = aiocb->bs->opaque;
>  int ret;
>  
> +if (s->page_cache_inconsistent) {
> +return -EIO;
> +}
> +
>  ret = qemu_fdatasync(aiocb->aio_fildes);
>  if (ret == -1) {
> +/* There is no clear definition of the semantics of a failing 
> fsync(),
> + * so we may have to assume the worst. The sad truth is that this
> + * assumption is correct for Linux. Some pages are now probably 
> marked
> + * clean in the page cache even though they are inconsistent with the
> + * on-disk contents. The next fdatasync() call would succeed, but no
> + * further writeback attempt will be made. We can't get back to a 
> state
> + * in which we know what is on disk (we would have to rewrite
> + * everything that was touched since the last fdatasync() at least), 
> so
> + * make bdrv_flush() fail permanently. Given that the behaviour isn't
> + * really defined, I have little hope that other OSes are doing 
> better.
> + *
> + * Obviously, this doesn't affect O_DIRECT, which bypasses the page
> + * cache. */
> +if ((s->open_flags & O_DIRECT) == 0) {
> +s->page_cache_inconsistent = true;
> +}
>  return -errno;
>  }
>  return 0;
> -- 
> 2.9.3
> 
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH RESEND 2/3] device_tree: fix compiler warnings (clang 5)

[Marc-André Lureau]

On Thu, Mar 23, 2017 at 12:49 AM Philippe Mathieu-Daudé 
wrote:

> static code analyzer complain:
>
> device_tree.c:155:18: warning: Null pointer passed as an argument to a
> 'nonnull' parameter
> while ((de = readdir(d)) != NULL) {
>  ^~
>
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé 
>


Reviewed-by: Marc-André Lureau 



> ---
>  device_tree.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/device_tree.c b/device_tree.c
> index 6e06320830..a24ddff02b 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -148,6 +148,7 @@ static void read_fstree(void *fdt, const char *dirname)
>  d = opendir(dirname);
>  if (!d) {
>  error_setg(_fatal, "%s cannot open %s", __func__, dirname);
> +return;
>  }
>
>  while ((de = readdir(d)) != NULL) {
> --
> 2.11.0
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] rawhide gcc failures [was: Proposal for deprecating unsupported host OSes & architecutures]

[Peter Maydell]

On 22 March 2017 at 19:07, Eric Blake  wrote:
> Tangentially-related: do we officially support bleeding-edge OS builds?
> For example, current rawhide has a new-enough gcc that gives some
> (possibly-useful) new warnings (-Werror=format-truncation)

Depends what you mean by "support". Are we testing them
in our build/CI loop? Apparently not :-) Do we fix them?
Yep, certainly, because bleeding-edge libraries and
compilers will eventually be standard ones. Do we fix
them during the release process? Yes, but maybe if
the fix is very invasive somehow we might prefer to
postpone to the next release. Do we consider them
a release-critical bug that we'd spin an extra rc
for? I don't think so, because (a) for released tarballs
-Werror is not on by default (b) even for building from
git you can work around this with -disable-werror
(c) it's just an accident of timing if a new gcc drops
3 weeks before release rather than 3 weeks after, so
it's always possible that releases will end up being
built on compilers that warn about things in them.

More generally, I'm not going to go so far as to insist
that we have a build machine for every flavour of every
distro -- that would not usefully improve coverage I think.
Also I'm happy to be ad-hoc about exactly what we test
(my x86 Linux builds are just "the Ubuntu flavour my
desktop box happens to be running right now") because
being less ad-hoc feels like it would be work that
we don't really need to engage in.

thanks
-- PMM

Re: [Qemu-devel] [PATCH RESEND 3/3] qga: fix compiler warnings (clang 5)

[Michael Roth]

Quoting Philippe Mathieu-Daudé (2017-03-22 15:48:44)
> static code analyzer complain:
> 
> qga/commands-posix.c:2127:9: warning: Null pointer passed as an argument to a 
> 'nonnull' parameter
> closedir(dp);
> ^~~~
> 
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé 
> Reviewed-by: Marc-André Lureau 
> ---
>  qga/commands-posix.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 73d93eb5ce..8028141534 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2122,9 +2122,11 @@ static void transfer_memory_block(GuestMemoryBlock 
> *mem_blk, bool sys2memblk,
>   * we think this VM does not support online/offline memory block,
>   * any other solution?
>   */
> -if (!dp && errno == ENOENT) {
> -result->response =
> -GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
> +if (!dp) {
> +if (errno == ENOENT) {
> +result->response =
> +GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
> +}
>  goto out1;
>  }

I think this should be:

if (!dp) {
if (errno == ENOENT) {
result->response =
GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
} else {
result->response =
GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED;
}
goto out1;
}

otherwise we might set result->error_code while still indicating
success for the operation. That wasn't handled properly before your
patch either, it's just more apparent now.

>  closedir(dp);
> -- 
> 2.11.0
> 

[Qemu-devel] [PATCH for-2.9?] file-posix: Make bdrv_flush() failure permanent without O_DIRECT

[Kevin Wolf]

Success for bdrv_flush() means that all previously written data is safe
on disk. For fdatasync(), the best semantics we can hope for on Linux
(without O_DIRECT) is that all data that was written since the last call
was successfully written back. Therefore, and because we can't redo all
writes after a flush failure, we have to give up after a single
fdatasync() failure. After this failure, we would never be able to make
the promise that a successful bdrv_flush() makes.

Signed-off-by: Kevin Wolf 
---
 block/file-posix.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index 53febd3..beb7a4f 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -144,6 +144,7 @@ typedef struct BDRVRawState {
 bool has_write_zeroes:1;
 bool discard_zeroes:1;
 bool use_linux_aio:1;
+bool page_cache_inconsistent:1;
 bool has_fallocate;
 bool needs_alignment;
 } BDRVRawState;
@@ -824,10 +825,31 @@ static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
 
 static ssize_t handle_aiocb_flush(RawPosixAIOData *aiocb)
 {
+BDRVRawState *s = aiocb->bs->opaque;
 int ret;
 
+if (s->page_cache_inconsistent) {
+return -EIO;
+}
+
 ret = qemu_fdatasync(aiocb->aio_fildes);
 if (ret == -1) {
+/* There is no clear definition of the semantics of a failing fsync(),
+ * so we may have to assume the worst. The sad truth is that this
+ * assumption is correct for Linux. Some pages are now probably marked
+ * clean in the page cache even though they are inconsistent with the
+ * on-disk contents. The next fdatasync() call would succeed, but no
+ * further writeback attempt will be made. We can't get back to a state
+ * in which we know what is on disk (we would have to rewrite
+ * everything that was touched since the last fdatasync() at least), so
+ * make bdrv_flush() fail permanently. Given that the behaviour isn't
+ * really defined, I have little hope that other OSes are doing better.
+ *
+ * Obviously, this doesn't affect O_DIRECT, which bypasses the page
+ * cache. */
+if ((s->open_flags & O_DIRECT) == 0) {
+s->page_cache_inconsistent = true;
+}
 return -errno;
 }
 return 0;
-- 
2.9.3

[Qemu-devel] [PATCH RESEND 3/3] qga: fix compiler warnings (clang 5)

[Philippe Mathieu-Daudé]

static code analyzer complain:

qga/commands-posix.c:2127:9: warning: Null pointer passed as an argument to a 
'nonnull' parameter
closedir(dp);
^~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Marc-André Lureau 
---
 qga/commands-posix.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 73d93eb5ce..8028141534 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2122,9 +2122,11 @@ static void transfer_memory_block(GuestMemoryBlock 
*mem_blk, bool sys2memblk,
  * we think this VM does not support online/offline memory block,
  * any other solution?
  */
-if (!dp && errno == ENOENT) {
-result->response =
-GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
+if (!dp) {
+if (errno == ENOENT) {
+result->response =
+GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
+}
 goto out1;
 }
 closedir(dp);
-- 
2.11.0

[Qemu-devel] [PATCH RESEND 1/3] usb-ccid: make ccid_write_data_block() cope with null buffers

[Philippe Mathieu-Daudé]

static code analyzer complain:

hw/usb/dev-smartcard-reader.c:816:5: warning: Null pointer passed as an 
argument to a 'nonnull' parameter
memcpy(p->abData, data, len);
^~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Marc-André Lureau 
---
 hw/usb/dev-smartcard-reader.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 757b8b3f5a..c38a4e5886 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -799,8 +799,14 @@ static void ccid_write_parameters(USBCCIDState *s, 
CCID_Header *recv)
 static void ccid_write_data_block(USBCCIDState *s, uint8_t slot, uint8_t seq,
   const uint8_t *data, uint32_t len)
 {
-CCID_DataBlock *p = ccid_reserve_recv_buf(s, sizeof(*p) + len);
+CCID_DataBlock *p;
 
+if (len == 0) {
+return;
+}
+g_assert(data != NULL);
+
+p = ccid_reserve_recv_buf(s, sizeof(*p) + len);
 if (p == NULL) {
 return;
 }
-- 
2.11.0

[Qemu-devel] [PATCH RESEND 2/3] device_tree: fix compiler warnings (clang 5)

[Philippe Mathieu-Daudé]

static code analyzer complain:

device_tree.c:155:18: warning: Null pointer passed as an argument to a 
'nonnull' parameter
while ((de = readdir(d)) != NULL) {
 ^~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé 
---
 device_tree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/device_tree.c b/device_tree.c
index 6e06320830..a24ddff02b 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -148,6 +148,7 @@ static void read_fstree(void *fdt, const char *dirname)
 d = opendir(dirname);
 if (!d) {
 error_setg(_fatal, "%s cannot open %s", __func__, dirname);
+return;
 }
 
 while ((de = readdir(d)) != NULL) {
-- 
2.11.0

[Qemu-devel] [PATCH RESEND 0/3] easy-to-fix clang warnings

[Philippe Mathieu-Daudé]

This patchset fixes three easy-to-fix clang warnings.

Resent adding Marc-André Lureau's Reviewed-by and CC'ing qemu-trivial as
suggested by Markus Armbruster.

Philippe Mathieu-Daudé (3):
  usb-ccid: make ccid_write_data_block() cope with null buffers
  device_tree: fix compiler warnings (clang 5)
  qga: fix compiler warnings (clang 5)

 device_tree.c | 1 +
 hw/usb/dev-smartcard-reader.c | 8 +++-
 qga/commands-posix.c  | 8 +---
 3 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.11.0

Re: [Qemu-devel] q35 and sysbus devices

[Laszlo Ersek]

On 03/22/17 21:31, Eduardo Habkost wrote:
> Hi,
> 
> I am investigating the current status of has_dynamic_sysbus and
> sysbus device support on each of QEMU's machine types. The good
> news is that almost all has_dynamic_sysbus=1 machines have their
> own internal (often short) whitelist of supported sysbus device
> types, and automatically reject unsupported devices.
> 
> ...except for q35.
> 
> q35 currently accepts all sys-bus-device subtypes on "-device",
> and today this includes the following 23 devices:
> 
> * allwinner-ahci
> * amd-iommu
> * cfi.pflash01
> * esp
> * fw_cfg_io
> * fw_cfg_mem
> * generic-sdhci
> * hpet
> * intel-iommu
> * ioapic
> * isabus-bridge
> * kvmclock
> * kvm-ioapic
> * kvmvapic
> * SUNW,fdtwo
> * sysbus-ahci
> * sysbus-fdc
> * sysbus-ohci
> * unimplemented-device
> * virtio-mmio
> * xen-backend
> * xen-sysdev
> 
> My question is: do all those devices really make sense to be used
> with "-device" on q35?

I think fw_cfg_io and fw_cfg_mem should be board-only devices (no
-device switch).

Regarding cfi.pflash01, I think originally it would have been nice to
specify pflash chips with the modern (non-legacy) syntax, that is,
separate -drive if=none,file=... backend options combined with -device
cfi.pflash01,drive=... frontend options. However, that ship has sailed,
even libvirt uses -drive if=pflash for these, and given the purpose we
use pflash chips for, on Q35, I don't see much benefit in exposing
cfi.pflash01 with a naked -device *now*.

Re: virtio-mmio, I don't think that should be available on Q35 at all.

I can't comment on the rest.

Thanks
Laszlo

> Should we make q35 validate dynamic sysbus
> devices against a whitelist, like the other has_dynamic_sysbus
> machines?
> 
> I'm asking this because I will resume work on the
> "query-device-slots" command, which will report supported sysbus
> devices too. And I don't want the new command to report any
> devices that it shouldn't.
> 

[Qemu-devel] q35 and sysbus devices

[Eduardo Habkost]

Hi,

I am investigating the current status of has_dynamic_sysbus and
sysbus device support on each of QEMU's machine types. The good
news is that almost all has_dynamic_sysbus=1 machines have their
own internal (often short) whitelist of supported sysbus device
types, and automatically reject unsupported devices.

...except for q35.

q35 currently accepts all sys-bus-device subtypes on "-device",
and today this includes the following 23 devices:

* allwinner-ahci
* amd-iommu
* cfi.pflash01
* esp
* fw_cfg_io
* fw_cfg_mem
* generic-sdhci
* hpet
* intel-iommu
* ioapic
* isabus-bridge
* kvmclock
* kvm-ioapic
* kvmvapic
* SUNW,fdtwo
* sysbus-ahci
* sysbus-fdc
* sysbus-ohci
* unimplemented-device
* virtio-mmio
* xen-backend
* xen-sysdev

My question is: do all those devices really make sense to be used
with "-device" on q35? Should we make q35 validate dynamic sysbus
devices against a whitelist, like the other has_dynamic_sysbus
machines?

I'm asking this because I will resume work on the
"query-device-slots" command, which will report supported sysbus
devices too. And I don't want the new command to report any
devices that it shouldn't.

-- 
Eduardo

Re: [Qemu-devel] [BUG] user-to-root privesc inside VM via bad translation caching

[Richard Henderson]

On 03/23/2017 02:29 AM, Pranith Kumar wrote:

On Wed, Mar 22, 2017 at 11:21 AM, Peter Maydell
 wrote:

On 22 March 2017 at 15:14, Pranith Kumar  wrote:

On Wed, Mar 22, 2017 at 11:04 AM, Peter Maydell
 wrote:

This doesn't look right because it means we'll check
only after we've emitted all the code to do the
instruction operation, so the effect will be
"execute instruction, then take illegal-opcode
exception".



The pc is restored to original address (s->pc = pc_start), so the
exception will overwrite the generated illegal instruction and will be
executed first.


s->pc is the guest PC -- moving that backwards will
not do anything about the generated TCG IR that's
already been written. You'd need to rewind the
write pointer in the IR stream, which there is
no support for doing AFAIK.


Ah, OK. Thanks for the explanation. May be we should check the size of
the instruction while decoding the prefixes and error out once we
exceed the limit. We would not generate any IR code.


Yes.

It would not enforce a true limit of 15 bytes, since you can't know that until 
you've done the rest of the decode.  But you'd be able to say that no more than 
14 prefix + 1 opc + 6 modrm+sib+ofs + 4 immediate = 25 bytes is used.


Which does fix the bug.


r~

Re: [Qemu-devel] [RFC][PATCH 0/6] "bootonceindex" property

[Janne Huttunen]

On Wed, 2017-03-22 at 15:36 +0100, Laszlo Ersek wrote:
> 
> To my knowledge, currently the bootindex properties cannot be changed
> dynamically (for example with monitor commands) after they have been
> specified on the QEMU command line.

Yes they can, via QMP:

{'execute': 'qom-get', 'arguments': { 'path': 'scsi0.0/child[0]',
'property': 'bootindex' }}
{"return": 10}

{'execute': 'qom-set', 'arguments': { 'path': 'scsi0.0/child[0]',
'property': 'bootindex', 'value': 11 }}
{"return": {}}

{'execute': 'qom-get', 'arguments': { 'path': 'scsi0.0/child[0]',
'property': 'bootindex' }}
{"return": 11}


I have no idea if doing so breaks something, like e.g.
migration, but it definitely can be done.

> And, even if some settings can be changed, the question is what agent
> changes them. If the management layer (libvirt etc) changes them
> using
> monitor commands, then libvirt itself can keep track of that state,
> and
> then start the QEMU process on the destination  host with a suitably
> modified command line.
> 
> Whereas, if it is the guest that changes device state, memory state
> etc,
> then those things have to be part of the migration stream.

True.

> > If it
> > is, migrating the "bootonceindex" values the same way should
> > be sufficient, no?
> To my knowledge, "bootindex" properties are not migrated now, and
> they
> also need not to, because they never change at runtime.

Like demonstrated above, they can change, but of course
there might be an assumption that they won't change
"unexpectedly".

> If the second system sets a temporary boot order for the first system
> "every now and then", which I guess entails on-demand talking to the
> first system's management processor, then how can you replace that
> with
> a static QEMU command line (with the proposed bootonceindex
> property)?

I'm not. I'm using QMP to change the index dynamically.

> I'm not saying that the use case is without merit. My point is that
> this
> new property would introduce obscure interactions with other
> components
> in the virt stack around QEMU, and it could have a significant
> maintenance footprint, while the feature does look niche (to me
> anyway).

Maybe, and it is definitely good to analyze it. Like I
already admitted, I did not think about migration at all
and there may very well have been other things I have
overlooked too.

Re: [Qemu-devel] rawhide gcc failures [was: Proposal for deprecating unsupported host OSes & architecutures]

[Philippe Mathieu-Daudé]

so lovely...

as a start point for bleeding edge stuff this can go in CI as a 
complement of debian images in tests/docker/... using the official 
fedora:rawhide base image.


On 03/22/2017 04:07 PM, Eric Blake wrote:

On 03/16/2017 10:23 AM, Peter Maydell wrote:

OK, here's a concrete proposal for deprecating/dropping out of
date host OS and architecture support.

We'll put this in the ChangeLog 'Future incompatible changes'
section:
-
* Removal of support for untested host OS and architectures:


Tangentially-related: do we officially support bleeding-edge OS builds?
For example, current rawhide has a new-enough gcc that gives some
(possibly-useful) new warnings (-Werror=format-truncation) that fire
when formatting what can be easily proven to be larger than a
fixed-width buffer will hold.  If rawhide is not a current target, then
I don't need to spend any time on this (yet); but if rawhide builds ARE
supported, then we want this patched before 2.9:


  CC  block/blkdebug.o
block/blkdebug.c: In function ‘blkdebug_refresh_filename’:
block/blkdebug.c:693:31: error: ‘%s’ directive output may be truncated
writing up to 4095 bytes into a region of size 4086
[-Werror=format-truncation=]
  "blkdebug:%s:%s", s->config_file ?: "",
   ^~
block/blkdebug.c:692:9: note: ‘snprintf’ output 11 or more bytes
(assuming 4106) into a destination of size 4096
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
 ^~~~
  "blkdebug:%s:%s", s->config_file ?: "",
  ~~~
  bs->file->bs->exact_filename);
  ~
cc1: all warnings being treated as errors
make: *** [/home/dummy/qemu/rules.mak:69: block/blkdebug.o] Error 1
  CC  block/blkverify.o
block/blkverify.c: In function ‘blkverify_refresh_filename’:
block/blkverify.c:309:29: error: ‘%s’ directive output may be truncated
writing up to 4095 bytes into a region of size 4086
[-Werror=format-truncation=]
  "blkverify:%s:%s",
 ^~
block/blkverify.c:308:9: note: ‘snprintf’ output between 12 and 8202
bytes into a destination of size 4096
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
 ^~~~
  "blkverify:%s:%s",
  ~~
  bs->file->bs->exact_filename,
  ~
  s->test_file->bs->exact_filename);
  ~
cc1: all warnings being treated as errors
make: *** [/home/dummy/qemu/rules.mak:69: block/blkverify.o] Error 1
  CC  hw/usb/bus.o
hw/usb/bus.c: In function ‘usb_port_location’:
hw/usb/bus.c:410:66: error: ‘%d’ directive output may be truncated
writing between 1 and 11 bytes into a region of size between 0 and 15
[-Werror=format-truncation=]
 snprintf(downstream->path, sizeof(downstream->path), "%s.%d",
  ^~
hw/usb/bus.c:410:9: note: ‘snprintf’ output between 3 and 28 bytes into
a destination of size 16
 snprintf(downstream->path, sizeof(downstream->path), "%s.%d",
 ^
  upstream->path, portnr);
  ~~~
cc1: all warnings being treated as errors
make: *** [/home/dummy/qemu/rules.mak:69: hw/usb/bus.o] Error 1
  CC  net/slirp.o
net/slirp.c: In function ‘slirp_smb_cleanup’:
net/slirp.c:565:44: error: ‘%s’ directive output may be truncated
writing up to 127 bytes into a region of size 121
[-Werror=format-truncation=]
 snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir);
^~
net/slirp.c:565:9: note: ‘snprintf’ output between 8 and 135 bytes into
a destination of size 128
 snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir);
 ^~~
net/slirp.c: In function ‘slirp_smb’:
net/slirp.c:609:46: error: ‘%s’ directive output may be truncated
writing 8 bytes into a region of size between 0 and 127
[-Werror=format-truncation=]
 snprintf(smb_conf, sizeof(smb_conf), "%s/%s", s->smb_dir, "smb.conf");
  ^~   ~~
net/slirp.c:609:5: note: ‘snprintf’ output between 10 and 137 bytes into
a destination of size 128
 snprintf(smb_conf, sizeof(smb_conf), "%s/%s", s->smb_dir, "smb.conf");
 ^
net/slirp.c:654:55: error: ‘%s’ directive output may be truncated
writing up to 127 bytes into a region of size 110
[-Werror=format-truncation=]
 snprintf(smb_cmdline, sizeof(smb_cmdline), "%s -l %s -s %s",
   

Re: [Qemu-devel] [PATCH v3 0/8] qemu-img: add measure sub-command

[Nir Soffer]

On Wed, Mar 22, 2017 at 2:28 PM Stefan Hajnoczi  wrote:

> v3:
>  * Drop RFC, this is ready to go for QEMU 2.10
>  * Use "required size" instead of "required bytes" in qemu-img output for
>consistency [Nir]
>  * Clarify BlockMeasureInfo semantics [Max]
>  * Clarify bdrv_measure() opts argument and error handling [Nir]
>  * Handle -o backing_file= for qcow2 [Max]
>  * Handle snapshot options in qemu-img measure
>  * Probe input image for allocated data clusters for qcow2.  Didn't
> centralize
>this because there are format-specific aspects such as the
> cluster_size.  It
>may make sense to centralize it later (with a bit more complexity) if
>support is added to more formats.
>  * Add qemu-img(1) man page section for 'measure' sub-command [Max]
>  * Extend test case to cover additional scenarios [Nir]
>
> RFCv2:
>  * Publishing RFC again to discuss the new user-visible interfaces.  Code
> has
>changed quite a bit, I have not kept any Reviewed-by tags.
>  * Rename qemu-img sub-command "measure" and API bdrv_measure() [Nir]
>  * Report both "required bytes" and "fully allocated bytes" to handle the
> empty
>image file and prealloc use cases [Nir and Dan]
>  * Use bdrv_getlength() instead of bdrv_nb_sectors() [Berto]
>  * Rename "err" label "out" in qemu-img-cmds.c [Nir]
>  * Add basic qcow2 support, doesn't support qemu-img convert from existing
> files yet
>
> RFCv1:
>  * Publishing patch series with just raw support, no qcow2 yet.  Please
> review
>the command-line interface and let me know if you are happy with this
>approach.
>
> Users and management tools sometimes need to know the size required for a
> new
> disk image so that an LVM volume, SAN LUN, etc can be allocated ahead of
> time.
> Image formats like qcow2 have non-trivial metadata that makes it hard to
> estimate the exact size without knowledge of file format internals.
>
> This patch series introduces a new qemu-img sub-command that calculates the
> required size for both image creation and conversion scenarios.
>
> The conversion scenario is:
>
>   $ qemu-img measure -f raw -O qcow2 input.img
>   required size: 1327680
>   fully allocated size: 1074069504
>
> Here an existing image file is taken and the output includes the space
> required
> for data from the input image file.
>
> The creation scenario is:
>
>   $ qemu-img measure -O qcow2 --size 5G
>   required size: 327680
>   fully allocated size: 1074069504
>
> Stefan Hajnoczi (8):
>   block: add bdrv_measure() API
>   raw-format: add bdrv_measure() support
>   qcow2: extract preallocation calculation function
>   qcow2: extract image creation option parsing
>   qcow2: add bdrv_measure() support
>   qemu-img: add measure subcommand
>   qemu-iotests: support per-format golden output files
>   iotests: add test 178 for qemu-img measure
>
>  qapi/block-core.json |  25 +++
>  include/block/block.h|   4 +
>  include/block/block_int.h|   2 +
>  block.c  |  35 
>  block/qcow2.c| 362
> +--
>  block/raw-format.c   |  22 +++
>  qemu-img.c   | 227 
>  qemu-img-cmds.hx |   6 +
>  qemu-img.texi|  25 +++
>  tests/qemu-iotests/178   | 144 
>  tests/qemu-iotests/178.out.qcow2 | 242 ++
>  tests/qemu-iotests/178.out.raw   | 130 ++
>  tests/qemu-iotests/check |   5 +
>  tests/qemu-iotests/group |   1 +
>  14 files changed, 1136 insertions(+), 94 deletions(-)
>  create mode 100755 tests/qemu-iotests/178
>  create mode 100644 tests/qemu-iotests/178.out.qcow2
>  create mode 100644 tests/qemu-iotests/178.out.raw
>
> --
> 2.9.3
>

Stefan, is this available in some public git repo?

I would like to test it,  and downloading and applying 8 patches is lot of
work.
(I'm probably not using the right tools for this).

Nir

Re: [Qemu-devel] [Qemu-ppc] [Bug 1674925] Re: Qemu PPC64 kvm no display if --device virtio-gpu-pci is selected

[luigiburdo]

Hello Cédric,

with your last message you made me think about and make more test.

>The default machine for qemu-system-ppc64 is pseries.
yes usually with 2.8 i  boot  the VM without issue on G5 Quad  with the option  
-M  pseries from 2.1 to 2.5 with kvm-pr enabled.
i did the tests and with all pseries now on 2.9 i have the same issue.
example:
qemu-system-ppc64 --enable-kvm -cpu 970fx_v2.0 -m 1024 -M pseries-2.1
qemu-system-ppc64: KVM and IRQ_XICS capability must be present for in-kernel 
XICS

but no issue if i run with -M mac99 before 2.9 was not possible use it on 
qemu-system-ppc64
It means it will no possible anymore in future release of qemu use open 
firmware on powermacs any moore?


>I admit the message is not very clear, but the host kernel is
>using a dev config.

Im so sorry, i learn English by my self reading ml and on irc chatting
is too difficult where no one speak English  around.

>> On Qoriq if i pass the -cpu e500 (normal thing) all is working right qemu 
>> 2.9rc1
>> all boot and so and so.
>but you must be changing the machine right ?
not on Qoriq because it is book3e and is not so flexible like the G5 Quad who 
is book3s machine.
i can run qemu kvm only with emb hardware on Qoriq

>>On G5 the -cpu variable dont fix the issue.
>with which machine ?
On PowerMac G5 Quad 970MP it have similar hardware configuration of IBM 
intellistation power 285

> >I can build a new kernel release , usually mine dont have xics enabled 
> >because G5
>> dont have that feature, if needed i can enable it for testing.**
>Yes that would be interesting.

I will do ASAP just the time to build it .

Thank you really much for your time and patience.
Luigi


-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1674925

Title:
  Qemu PPC64 kvm no display if  --device virtio-gpu-pci is selected

Status in QEMU:
  New

Bug description:
  Hi,
  i did many tests on qemu 2.8 on my BE machines and i found an issue that i 
think was need to be reported

  Test Machines BE 970MP

  if i setup qemu with

  qemu-system-ppc64 -M 1024 --display sdl(or gtk),gl=on --device virtio-
  gpu-pci,virgl --enable-kvm and so and so

  result is doubled window one is vga other is virtio-gpu-pci without
  any start of the VM . pratically i dont have any output of openbios
  and on the virtual serial output

  the same issue i found is if i select:
  qemu-system-ppc64 -M 1024 --display gtk(or sdl) --device virtio-gpu-pci 
--enable-kvm and so and so

  
  i had been try to change all the -M types of all kind of pseries without any 
positive result.

  Ciao 
  Luigi

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1674925/+subscriptions

Re: [Qemu-devel] [PATCH 0/3] script for crash-testing -device

[Eduardo Habkost]

On Wed, Mar 22, 2017 at 01:00:49PM -0300, Eduardo Habkost wrote:
> This series adds scripts/device-crashtest.py, that can be used to
> crash-test -device with multiple machine/accel/device
> combinations.
> 
> The script found a few crashes on some machines/devices. A dump
> of existing cases can be seen here:
>   https://gist.github.com/ehabkost/503b0af0375f0d98d3e84017e8ca54eb
> 
> The script contains a whitelist that can also be useful as
> documentation of existing ways -device can fail or crash.
> 
> Note that the script takes a few hours to run on the default mode
> (testing all accel/machine/device combinations), but the "-r N"
> option can be used to make it only test N random samples.

Something I forgot to mention: I would like to run some subset of
these tests on "make check", but I don't know how we could choose
that subset. We could run, e.g., 100 random samples, but I am not
sure we really want to make "make check" non-deterministic.

Ideas?

-- 
Eduardo

[Qemu-devel] rawhide gcc failures [was: Proposal for deprecating unsupported host OSes & architecutures]

[Eric Blake]

On 03/16/2017 10:23 AM, Peter Maydell wrote:
> OK, here's a concrete proposal for deprecating/dropping out of
> date host OS and architecture support.
> 
> We'll put this in the ChangeLog 'Future incompatible changes'
> section:
> -
> * Removal of support for untested host OS and architectures:

Tangentially-related: do we officially support bleeding-edge OS builds?
For example, current rawhide has a new-enough gcc that gives some
(possibly-useful) new warnings (-Werror=format-truncation) that fire
when formatting what can be easily proven to be larger than a
fixed-width buffer will hold.  If rawhide is not a current target, then
I don't need to spend any time on this (yet); but if rawhide builds ARE
supported, then we want this patched before 2.9:


  CC  block/blkdebug.o
block/blkdebug.c: In function ‘blkdebug_refresh_filename’:
block/blkdebug.c:693:31: error: ‘%s’ directive output may be truncated
writing up to 4095 bytes into a region of size 4086
[-Werror=format-truncation=]
  "blkdebug:%s:%s", s->config_file ?: "",
   ^~
block/blkdebug.c:692:9: note: ‘snprintf’ output 11 or more bytes
(assuming 4106) into a destination of size 4096
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
 ^~~~
  "blkdebug:%s:%s", s->config_file ?: "",
  ~~~
  bs->file->bs->exact_filename);
  ~
cc1: all warnings being treated as errors
make: *** [/home/dummy/qemu/rules.mak:69: block/blkdebug.o] Error 1
  CC  block/blkverify.o
block/blkverify.c: In function ‘blkverify_refresh_filename’:
block/blkverify.c:309:29: error: ‘%s’ directive output may be truncated
writing up to 4095 bytes into a region of size 4086
[-Werror=format-truncation=]
  "blkverify:%s:%s",
 ^~
block/blkverify.c:308:9: note: ‘snprintf’ output between 12 and 8202
bytes into a destination of size 4096
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
 ^~~~
  "blkverify:%s:%s",
  ~~
  bs->file->bs->exact_filename,
  ~
  s->test_file->bs->exact_filename);
  ~
cc1: all warnings being treated as errors
make: *** [/home/dummy/qemu/rules.mak:69: block/blkverify.o] Error 1
  CC  hw/usb/bus.o
hw/usb/bus.c: In function ‘usb_port_location’:
hw/usb/bus.c:410:66: error: ‘%d’ directive output may be truncated
writing between 1 and 11 bytes into a region of size between 0 and 15
[-Werror=format-truncation=]
 snprintf(downstream->path, sizeof(downstream->path), "%s.%d",
  ^~
hw/usb/bus.c:410:9: note: ‘snprintf’ output between 3 and 28 bytes into
a destination of size 16
 snprintf(downstream->path, sizeof(downstream->path), "%s.%d",
 ^
  upstream->path, portnr);
  ~~~
cc1: all warnings being treated as errors
make: *** [/home/dummy/qemu/rules.mak:69: hw/usb/bus.o] Error 1
  CC  net/slirp.o
net/slirp.c: In function ‘slirp_smb_cleanup’:
net/slirp.c:565:44: error: ‘%s’ directive output may be truncated
writing up to 127 bytes into a region of size 121
[-Werror=format-truncation=]
 snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir);
^~
net/slirp.c:565:9: note: ‘snprintf’ output between 8 and 135 bytes into
a destination of size 128
 snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir);
 ^~~
net/slirp.c: In function ‘slirp_smb’:
net/slirp.c:609:46: error: ‘%s’ directive output may be truncated
writing 8 bytes into a region of size between 0 and 127
[-Werror=format-truncation=]
 snprintf(smb_conf, sizeof(smb_conf), "%s/%s", s->smb_dir, "smb.conf");
  ^~   ~~
net/slirp.c:609:5: note: ‘snprintf’ output between 10 and 137 bytes into
a destination of size 128
 snprintf(smb_conf, sizeof(smb_conf), "%s/%s", s->smb_dir, "smb.conf");
 ^
net/slirp.c:654:55: error: ‘%s’ directive output may be truncated
writing up to 127 bytes into a region of size 110
[-Werror=format-truncation=]
 snprintf(smb_cmdline, sizeof(smb_cmdline), "%s -l %s -s %s",
   ^~
net/slirp.c:654:5: note: ‘snprintf’ output 23 or more bytes (assuming
150) into a destination of size 128
 snprintf(smb_cmdline, sizeof(smb_cmdline), "%s -l %s -s %s",
 

Re: [Qemu-devel] [PATCH v3] qemu/thread: Add support for error reporting in qemu_thread_create

[Achilles Benetopoulos]

Thank you for the detailed review. The indentation errors mentioned
were pure carelesness on my part, sorry about that.

On 3/22/17 2:20 PM, Eric Blake wrote:
>> @@ -342,13 +343,19 @@ static void pci_edu_realize(PCIDevice *pdev, Error 
>> **errp) >>  { >>  EduState *edu = DO_UPCAST(EduState, pdev, pdev); >>
>>   uint8_t *pci_conf = pdev->config; >> +Error *local_err = NULL; >> >>   
>>timer_init_ms(>dma_timer, QEMU_CLOCK_VIRTUAL, edu_dma_timer, edu); 
>> >> >>  qemu_mutex_init(>thr_mutex); >>  
>> qemu_cond_init(>thr_cond); >>  qemu_thread_create(>thread, 
>> "edu", edu_fact_thread, >> -   edu, 
>> QEMU_THREAD_JOINABLE); >> +   edu, QEMU_THREAD_JOINABLE, 
>> _err); >> + >> +if (local_err) { >> +error_propagate(errp, 
>> local_err); >> +return; >> +} > > Looking at code like this, I 
>> wonder if it would be easier to make > qemu_thread_create() return a value, 
>> rather than being void.  Then you > could write: > > if 
>> (qemu_thread_create(..., errp) < 0) { > return; > } > > instead of 
>> having to futz around with local_err and error_propagate(). >

I don't know about it being easier, but it does seem like it would
make the code at the call site cleaner, when the function calling
qemu_thread_create was passed an error variable itself. However, in
all but four cases there is no preexisting error variable, so the code
would stay pretty much the same.

>> +++ b/hw/usb/ccid-card-emulated.c >> @@ -34,6 +34,7 @@ >> >>  #include 
>> "qemu/thread.h" >>  #include "sysemu/char.h" >> +#include "qapi/error.h" >>  
>> #include "ccid.h" >> >>  #define DPRINTF(card, lvl, fmt, ...) \ >> @@ -485,6 
>> +486,7 @@ static int emulated_initfn(CCIDCardState *base) >>  
>> EmulatedState *card = EMULATED_CCID_CARD(base); >>  VCardEmulError ret; 
>> >>  const EnumTable *ptable; >> +Error *err = NULL, *local_err = 
>> NULL; > > Huh? Why do you need two local error objects? One is generally 
>> sufficient. > >> >>  QSIMPLEQ_INIT(>event_list); >>  
>> QSIMPLEQ_INIT(>guest_apdu_list); >> @@ -541,9 +543,17 @@ static int 
>> emulated_initfn(CCIDCardState *base) >>  return -1; >>  } >> 
>>  qemu_thread_create(>event_thread_id, "ccid/event", event_thread, >> - 
>>   card, QEMU_THREAD_JOINABLE); >> +  
>>  card, QEMU_THREAD_JOINABLE, ); >> + >>  
>> qemu_thread_create(>apdu_thread_id, "ccid/apdu", handle_apdu_thread, >>
-   card, QEMU_THREAD_JOINABLE); >> +   
card, QEMU_THREAD_JOINABLE, _err); >> +error_propagate(, 
local_err); >> + >> +if (err) { >> +error_report_err(err); >> + 
   return -1; >> +} > > > If you used the return value, you could write: > 
> if (qemu_thread_create(..., ) < 0 || > qemu_thread_create(..., ) 
< 0) { > error_report_err(err); > return -1; > } > > without needing 
the second object.

Well, I wrote it this way because of a recommendation in the error.h
header, but yes, if we were to change the return type of
qemu_thread_create, then this would make sense.

>> +++ b/include/qemu/thread.h >> @@ -55,7 +55,7 @@ void 
>> qemu_event_destroy(QemuEvent *ev); >> >>  void qemu_thread_create(QemuThread 
>> *thread, const char *name, >>  void 
>> *(*start_routine)(void *), >> -void *arg, int mode); 
>> >> +void *arg, int mode, Error **errp); > > Hmm, we 
>> still haven't made it official recommended practice, but it's a > good idea 
>> to use 'git config diff.orderFile /path/to/file' in order to > hoist .h 
>> changes to the front of a diff (it makes diffs easier to review > when you 
>> see the interface change prior to the clients of the > interface).  I wish I 
>> had a better URL to point to on the topic, but > didn't want to spend time 
>> finding it in the mailing list archives at > this time.

Noted for future submissions.

>> +++ b/migration/ram.c >> @@ -357,6 +357,7 @@ void 
>> migrate_compress_threads_join(void) >>  void 
>> migrate_compress_threads_create(void) >>  { >>  int i, thread_count; >> 
>> +Error *err = NULL, *local_err = NULL; >> >>  if 
>> (!migrate_use_compression()) { >>  return; >> @@ -378,7 +379,16 @@ 
>> void migrate_compress_threads_create(void) >>  
>> qemu_cond_init(_param[i].cond); >>  
>> qemu_thread_create(compress_threads + i, "compress", >>  
>>do_data_compress, comp_param + i, >> -   
>> QEMU_THREAD_JOINABLE); >> +   QEMU_THREAD_JOINABLE, 
>> _err); >> + >> +if (local_err) { >> +
>> error_propagate(, local_err); >> +local_err = NULL; >> + 
>>} >> +} >> + >> +if (err) { >> +error_report_err(err); >> 
>>  } > > Another place that looks weird with two error variables.

I was on the fence about this 

Re: [Qemu-devel] [PULL v2 for-2.9 17/17] qapi: Fix QemuOpts visitor regression on unvisited input

[Markus Armbruster]

Eric Blake  writes:

> On 03/22/2017 12:48 PM, Markus Armbruster wrote:
>> From: Eric Blake 
>> 
>> An off-by-one in commit 15c2f669e meant that we were failing to
>> check for unparsed input in all QemuOpts visitors.  Recent testsuite
>> additions show that fixing the obvious bug with bogus fields will
>> also fix the case of an incomplete list visit; update the tests to
>> match the new behavior.
>> 
>> Simple testcase:
>> 
>> ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -numa 
>> node,size=1g
>> 
>> failed to diagnose that 'size' is not a valid argument to -numa, and
>> now once again reports:
>> 
>> qemu-system-x86_64: -numa node,size=1g: Invalid parameter 'size'
>> 
>> See also https://bugzilla.redhat.com/show_bug.cgi?id=1434666
>> 
>> CC: qemu-sta...@nongnu.org
>> Signed-off-by: Eric Blake 
>> Reviewed-by: Michael Roth 
>> Tested-by: Laurent Vivier 
>> Message-Id: <20170322144525.18964-4-ebl...@redhat.com>
>> Reviewed-by: Markus Armbruster 
>> [Fixup squashed in]
>> Signed-off-by: Markus Armbruster 
>
> Fixup squashed into the wrong patch. End result is the same, though, so
> not sure if it is worth a v3 pull request.

Rats...  thanks for paying attention!

Re: [Qemu-devel] [PATCH v4] xen: use libxendevice model to restrict operations

[Stefano Stabellini]

On Wed, 22 Mar 2017, Paul Durrant wrote:
> This patch adds a command-line option (-xen-domid-restrict) which will
> use the new libxendevicemodel API to restrict devicemodel [1] operations
> to the specified domid. (Such operations are not applicable to the xenpv
> machine type).
> 
> This patch also adds a tracepoint to allow successful enabling of the
> restriction to be monitored.
> 
> [1] I.e. operations issued by libxendevicemodel. Operation issued by other
> xen libraries (e.g. libxenforeignmemory) are currently still unrestricted
> but this will be rectified by subsequent patches.
> 
> Signed-off-by: Paul Durrant 

Reviewed-by: Stefano Stabellini 

Applied to my next branch

> ---
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: Paolo Bonzini 
> 
> NOTE: This is already re-based on Juergen Gross's patch "xen: use 5 digit
>   xen versions" and so should not be applied until after that patch
>   has been applied.
> 
> v4:
>  - Added missing quote
> 
> v3:
>  - Updated usage comment
> 
> v2:
>  - Log errno in tracepoint
> ---
>  hw/xen/trace-events |  1 +
>  include/hw/xen/xen.h|  1 +
>  include/hw/xen/xen_common.h | 20 
>  qemu-options.hx |  7 +++
>  vl.c|  8 
>  xen-hvm.c   |  8 
>  6 files changed, 45 insertions(+)
> 
> diff --git a/hw/xen/trace-events b/hw/xen/trace-events
> index c4fb6f1..5615dce 100644
> --- a/hw/xen/trace-events
> +++ b/hw/xen/trace-events
> @@ -11,3 +11,4 @@ xen_map_portio_range(uint32_t id, uint64_t start_addr, 
> uint64_t end_addr) "id: %
>  xen_unmap_portio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) 
> "id: %u start: %#"PRIx64" end: %#"PRIx64
>  xen_map_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id: %u 
> bdf: %02x.%02x.%02x"
>  xen_unmap_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id: 
> %u bdf: %02x.%02x.%02x"
> +xen_domid_restrict(int err) "err: %u"
> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> index 2b1733b..7efcdaa 100644
> --- a/include/hw/xen/xen.h
> +++ b/include/hw/xen/xen.h
> @@ -21,6 +21,7 @@ enum xen_mode {
>  
>  extern uint32_t xen_domid;
>  extern enum xen_mode xen_mode;
> +extern bool xen_domid_restrict;
>  
>  extern bool xen_allowed;
>  
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index df098c7..4f3bd35 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -152,6 +152,13 @@ static inline int xendevicemodel_set_mem_type(
>  return xc_hvm_set_mem_type(dmod, domid, mem_type, first_pfn, nr);
>  }
>  
> +static inline int xendevicemodel_restrict(
> +xendevicemodel_handle *dmod, domid_t domid)
> +{
> +errno = ENOTTY;
> +return -1;
> +}
> +
>  #else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40900 */
>  
>  #include 
> @@ -206,6 +213,19 @@ static inline int xen_modified_memory(domid_t domid, 
> uint64_t first_pfn,
>  return xendevicemodel_modified_memory(xen_dmod, domid, first_pfn, nr);
>  }
>  
> +static inline int xen_restrict(domid_t domid)
> +{
> +int rc = xendevicemodel_restrict(xen_dmod, domid);
> +
> +trace_xen_domid_restrict(errno);
> +
> +if (errno == ENOTTY) {
> +return 0;
> +}
> +
> +return rc;
> +}
> +
>  /* Xen 4.2 through 4.6 */
>  #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 40701
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 99af8ed..2043371 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3354,6 +3354,11 @@ DEF("xen-attach", 0, QEMU_OPTION_xen_attach,
>  "-xen-attach attach to existing xen domain\n"
>  "xend will use this when starting QEMU\n",
>  QEMU_ARCH_ALL)
> +DEF("xen-domid-restrict", 0, QEMU_OPTION_xen_domid_restrict,
> +"-xen-domid-restrict restrict set of available xen operations\n"
> +"to specified domain id. (Does not affect\n"
> +"xenpv machine type).\n",
> +QEMU_ARCH_ALL)
>  STEXI
>  @item -xen-domid @var{id}
>  @findex -xen-domid
> @@ -3366,6 +3371,8 @@ Warning: should not be used when xend is in use (XEN 
> only).
>  @findex -xen-attach
>  Attach to existing xen domain.
>  xend will use this when starting QEMU (XEN only).
> +@findex -xen-domid-restrict
> +Restrict set of available xen operations to specified domain id (XEN only).
>  ETEXI
>  
>  DEF("no-reboot", 0, QEMU_OPTION_no_reboot, \
> diff --git a/vl.c b/vl.c
> index 0b4ed52..f46e070 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -205,6 +205,7 @@ static NotifierList machine_init_done_notifiers =
>  bool xen_allowed;
>  uint32_t xen_domid;
>  enum xen_mode xen_mode = XEN_EMULATE;
> +bool xen_domid_restrict;
>  
>  static int has_defaults = 1;
>  static int default_serial = 1;
> @@ -3933,6 +3934,13 @@ int main(int argc, char **argv, char **envp)
>  

[Qemu-devel] [PULL v3 for-2.9 02/17] keyval: Improve some comments

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Message-Id: <1490014548-15083-3-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 util/keyval.c | 47 +++
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/util/keyval.c b/util/keyval.c
index f646b36..46cd540 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -21,22 +21,36 @@
  *
  * Semantics defined by reduction to JSON:
  *
- *   key-vals is a tree of objects and arrays rooted at object R
- *   where for each key-val = key-fragment . ... = val in key-vals
- *   R op key-fragment op ... = val'
- *   where (left-associative) op is
- * array subscript L[key-fragment] for numeric key-fragment
- * member reference L.key-fragment otherwise
- * val' is val with ',,' replaced by ','
- *   and only R may be empty.
+ *   key-vals specifies a JSON object, i.e. a tree whose root is an
+ *   object, inner nodes other than the root are objects or arrays,
+ *   and leaves are strings.
  *
- *   Duplicate keys are permitted; all but the last one are ignored.
+ *   Each key-val = key-fragment '.' ... '=' val specifies a path from
+ *   root to a leaf (left of '='), and the leaf's value (right of
+ *   '=').
  *
- *   The equations must have a solution.  Counter-example: a.b=1,a=2
- *   doesn't have one, because R.a must be an object to satisfy a.b=1
- *   and a string to satisfy a=2.
+ *   A path from the root is defined recursively:
+ *   L '.' key-fragment is a child of the node denoted by path L
+ *   key-fragment is a child of the tree root
+ *   If key-fragment is numeric, the parent is an array and the child
+ *   is its key-fragment-th member, counting from zero.
+ *   Else, the parent is an object, and the child is its member named
+ *   key-fragment.
  *
- * Key-fragments must be valid QAPI names or consist only of digits.
+ *   This constrains inner nodes to be either array or object.  The
+ *   constraints must be satisfiable.  Counter-example: a.b=1,a=2 is
+ *   not, because root.a must be an object to satisfy a.b=1 and a
+ *   string to satisfy a=2.
+ *
+ *   Array subscripts can occur in any order, but the set of
+ *   subscripts must not have gaps.  For instance, a.1=v is not okay,
+ *   because root.a[0] is missing.
+ *
+ *   If multiple key-val denote the same leaf, the last one determines
+ *   the value.
+ *
+ * Key-fragments must be valid QAPI names or consist only of decimal
+ * digits.
  *
  * The length of any key-fragment must be between 1 and 127.
  *
@@ -64,8 +78,8 @@
 
 /*
  * Convert @key to a list index.
- * Convert all leading digits to a (non-negative) number, capped at
- * INT_MAX.
+ * Convert all leading decimal digits to a (non-negative) number,
+ * capped at INT_MAX.
  * If @end is non-null, assign a pointer to the first character after
  * the number to *@end.
  * Else, fail if any characters follow.
@@ -337,7 +351,8 @@ static QObject *keyval_listify(QDict *cur, GSList 
*key_of_cur, Error **errp)
 }
 
 /*
- * Make a list from @elt[], reporting any missing elements.
+ * Make a list from @elt[], reporting the first missing element,
+ * if any.
  * If we dropped an index >= nelt in the previous loop, this loop
  * will run into the sentinel and report index @nelt missing.
  */
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 06/17] qapi: Drop excessive Make dependencies on qapi2texi.py

[Markus Armbruster]

When qapi2texi.py changes, we regenerate everything QAPI.  Screwed up
in commit 56e8bdd.

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-2-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 Makefile | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index f4f90df..6c359b2 100644
--- a/Makefile
+++ b/Makefile
@@ -392,7 +392,6 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
 gen-out-type = $(subst .,-,$(suffix $@))
 
 qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
-qapi-py += $(SRC_PATH)/scripts/qapi2texi.py
 
 qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
@@ -701,10 +700,12 @@ qemu-monitor-info.texi: $(SRC_PATH)/hmp-commands-info.hx 
$(SRC_PATH)/scripts/hxt
 qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > 
$@,"GEN","$@")
 
-docs/qemu-qmp-qapi.texi: $(qapi-modules) $(qapi-py)
+docs/qemu-qmp-qapi.texi docs/qemu-ga-qapi.texi: 
$(SRC_PATH)/scripts/qapi2texi.py $(qapi-py)
+
+docs/qemu-qmp-qapi.texi: $(qapi-modules)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > 
$@,"GEN","$@")
 
-docs/qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py)
+docs/qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > 
$@,"GEN","$@")
 
 qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi 
qemu-monitor-info.texi
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 09/17] tests/qapi-schema: Make test-qapi.py print docs again

[Markus Armbruster]

test-qapi.py used to print the internal representation of doc comments
(commit 3313b61).  This went away when we dropped the doc comments in
positive tests (commit 87c16dc).  Bring it back, because I'm going to
add real positive doc comment tests.

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-5-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 tests/qapi-schema/test-qapi.py | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index ef74e2c..c7724d3 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -55,3 +55,14 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
 
 schema = QAPISchema(sys.argv[1])
 schema.visit(QAPISchemaTestVisitor())
+
+for doc in schema.docs:
+if doc.symbol:
+print 'doc symbol=%s' % doc.symbol
+else:
+print 'doc freeform'
+print 'body=\n%s' % doc.body
+for arg, section in doc.args.iteritems():
+print 'arg=%s\n%s' % (arg, section)
+for section in doc.sections:
+print 'section=%s\n%s' % (section.name, section)
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 17/17] qapi: Fix QemuOpts visitor regression on unvisited input

[Markus Armbruster]

From: Eric Blake 

An off-by-one in commit 15c2f669e meant that we were failing to
check for unparsed input in all QemuOpts visitors.  Recent testsuite
additions show that fixing the obvious bug with bogus fields will
also fix the case of an incomplete list visit; update the tests to
match the new behavior.

Simple testcase:

./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -numa 
node,size=1g

failed to diagnose that 'size' is not a valid argument to -numa, and
now once again reports:

qemu-system-x86_64: -numa node,size=1g: Invalid parameter 'size'

See also https://bugzilla.redhat.com/show_bug.cgi?id=1434666

CC: qemu-sta...@nongnu.org
Signed-off-by: Eric Blake 
Reviewed-by: Michael Roth 
Tested-by: Laurent Vivier 
Message-Id: <20170322144525.18964-4-ebl...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 qapi/opts-visitor.c   |  6 +++---
 tests/test-opts-visitor.c | 13 -
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 026d25b..324b197 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -164,7 +164,7 @@ opts_check_struct(Visitor *v, Error **errp)
 GHashTableIter iter;
 GQueue *any;
 
-if (ov->depth > 0) {
+if (ov->depth > 1) {
 return;
 }
 
@@ -276,8 +276,8 @@ static void
 opts_check_list(Visitor *v, Error **errp)
 {
 /*
- * FIXME should set error when unvisited elements remain.  Mostly
- * harmless, as the generated visits always visit all elements.
+ * Unvisited list elements will be reported later when checking
+ * whether unvisited struct members remain.
  */
 }
 
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index 8e0dda5..23e8970 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -175,6 +175,7 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer 
test_data)
 static void
 test_opts_range_unvisited(void)
 {
+Error *err = NULL;
 intList *list = NULL;
 intList *tail;
 QemuOpts *opts;
@@ -199,10 +200,11 @@ test_opts_range_unvisited(void)
 g_assert_cmpint(tail->value, ==, 1);
 tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list));
 g_assert(tail);
-visit_check_list(v, _abort); /* BUG: unvisited tail not reported */
+visit_check_list(v, _abort); /* unvisited tail ignored until... */
 visit_end_list(v, (void **));
 
-visit_check_struct(v, _abort);
+visit_check_struct(v, ); /* ...here */
+error_free_or_abort();
 visit_end_struct(v, NULL);
 
 qapi_free_intList(list);
@@ -250,6 +252,7 @@ test_opts_range_beyond(void)
 static void
 test_opts_dict_unvisited(void)
 {
+Error *err = NULL;
 QemuOpts *opts;
 Visitor *v;
 UserDefOptions *userdef;
@@ -258,11 +261,11 @@ test_opts_dict_unvisited(void)
_abort);
 
 v = opts_visitor_new(opts);
-/* BUG: bogus should be diagnosed */
-visit_type_UserDefOptions(v, NULL, , _abort);
+visit_type_UserDefOptions(v, NULL, , );
+error_free_or_abort();
 visit_free(v);
 qemu_opts_del(opts);
-qapi_free_UserDefOptions(userdef);
+g_assert(!userdef);
 }
 
 int
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 10/17] tests/qapi-schema: Systematic positive doc comment tests

[Markus Armbruster]

We have a number of negative tests, but we don't have systematic
positive coverage.  Fix that.

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-6-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 tests/Makefile.include  |  13 ++-
 tests/qapi-schema/doc-good.err  |   0
 tests/qapi-schema/doc-good.exit |   1 +
 tests/qapi-schema/doc-good.json | 136 ++
 tests/qapi-schema/doc-good.out  | 148 
 tests/qapi-schema/doc-good.texi | 243 
 6 files changed, 537 insertions(+), 4 deletions(-)
 create mode 100644 tests/qapi-schema/doc-good.err
 create mode 100644 tests/qapi-schema/doc-good.exit
 create mode 100644 tests/qapi-schema/doc-good.json
 create mode 100644 tests/qapi-schema/doc-good.out
 create mode 100644 tests/qapi-schema/doc-good.texi

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 86f9490..f3de81f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -379,6 +379,7 @@ qapi-schema += doc-duplicated-since.json
 qapi-schema += doc-empty-arg.json
 qapi-schema += doc-empty-section.json
 qapi-schema += doc-empty-symbol.json
+qapi-schema += doc-good.json
 qapi-schema += doc-interleaved-section.json
 qapi-schema += doc-invalid-end.json
 qapi-schema += doc-invalid-end2.json
@@ -607,6 +608,9 @@ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json 
$(SRC_PATH)/scripts/qapi-int
$(gen-out-type) -o tests -p "test-" $<, \
"GEN","$@")
 
+tests/qapi-schema/doc-good.test.texi: 
$(SRC_PATH)/tests/qapi-schema/doc-good.json $(SRC_PATH)/scripts/qapi2texi.py 
$(qapi-py)
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > 
$@,"GEN","$@")
+
 tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o 
$(test-qapi-obj-y)
 tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o 
$(test-qapi-obj-y)
 tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y)
@@ -856,9 +860,6 @@ QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) = 
tests/qemu-iotests/socket_scm_helper$(EXE
 check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh 
qemu-img$(EXESUF) qemu-io$(EXESUF) $(QEMU_IOTESTS_HELPERS-y)
$<
 
-.PHONY: check-tests/test-qapi.py
-check-tests/test-qapi.py: tests/test-qapi.py
-
 .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
 $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: 
$(SRC_PATH)/%.json
$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
@@ -871,10 +872,14 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): 
check-%.json: $(SRC_PATH)/%.json
@perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q 
$(SRC_PATH)/$*.err -
@diff -q $(SRC_PATH)/$*.exit $*.test.exit
 
+.PHONY: check-tests/qapi-schema/doc-good.texi
+check-tests/qapi-schema/doc-good.texi: tests/qapi-schema/doc-good.test.texi
+   @diff -q $(SRC_PATH)/tests/qapi-schema/doc-good.texi $<
+
 # Consolidated targets
 
 .PHONY: check-qapi-schema check-qtest check-unit check check-clean
-check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y))
+check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) 
check-tests/qapi-schema/doc-good.texi
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 check-unit: $(patsubst %,check-%, $(check-unit-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
diff --git a/tests/qapi-schema/doc-good.err b/tests/qapi-schema/doc-good.err
new file mode 100644
index 000..e69de29
diff --git a/tests/qapi-schema/doc-good.exit b/tests/qapi-schema/doc-good.exit
new file mode 100644
index 000..573541a
--- /dev/null
+++ b/tests/qapi-schema/doc-good.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
new file mode 100644
index 000..cfdc0a8
--- /dev/null
+++ b/tests/qapi-schema/doc-good.json
@@ -0,0 +1,136 @@
+# -*- Mode: Python -*-
+# Positive QAPI doc comment tests
+
+{ 'pragma': { 'doc-required': true } }
+
+##
+# = Section
+#
+# == Subsection
+#
+# *strong* _with emphasis_
+# @var {in braces}
+# * List item one
+# - Two, multiple
+#   lines
+#
+# 3. Three
+# Still in list
+#
+#   Not in list
+# - Second list
+# Note: still in list
+#
+# Note: not in list
+# 1. Third list
+#is numbered
+#
+# - another item
+#
+# | example
+# | multiple lines
+#
+# Returns: the King
+# Since: the first age
+# Notes:
+#
+# 1. Lorem ipsum dolor sit amet
+#
+# 2. Ut enim ad minim veniam
+#
+# Duis aute irure dolor
+#
+# Example:
+#
+# -> in
+# <- out
+# Examples:
+# - *verbatim*
+# - {braces}
+##
+
+##
+# @Enum:
+# == Produces *invalid* texinfo
+# @one: The _one_ {and only}
+#
+# @two is undocumented
+##
+{ 'enum': 'Enum', 'data': [ 'one', 'two' ] }
+
+##
+# @Base:
+# @base1:
+#   the first member
+##
+{ 'struct': 'Base', 'data': { 'base1': 'Enum' } }
+
+##
+# @Variant1:
+# A paragraph
+#
+# Another paragraph (but no @var: line)

[Qemu-devel] [PULL v3 for-2.9 14/17] test-qobject-input-visitor: Cover visit_type_uint64()

[Markus Armbruster]

The new test demonstrates known bugs: integers between INT64_MAX+1 and
UINT64_MAX rejected, and integers between INT64_MIN and -1 are
accepted modulo 2^64.

Signed-off-by: Markus Armbruster 
Message-Id: <1490118290-6133-1-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 tests/test-qobject-input-visitor.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/tests/test-qobject-input-visitor.c 
b/tests/test-qobject-input-visitor.c
index 6eb48fe..f965743 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -116,6 +116,34 @@ static void test_visitor_in_int(TestInputVisitorData *data,
 g_assert_cmpint(res, ==, value);
 }
 
+static void test_visitor_in_uint(TestInputVisitorData *data,
+const void *unused)
+{
+Error *err = NULL;
+uint64_t res = 0;
+int value = 42;
+Visitor *v;
+
+v = visitor_input_test_init(data, "%d", value);
+
+visit_type_uint64(v, NULL, , _abort);
+g_assert_cmpuint(res, ==, (uint64_t)value);
+
+/* BUG: value between INT64_MIN and -1 accepted modulo 2^64 */
+
+v = visitor_input_test_init(data, "%d", -value);
+
+visit_type_uint64(v, NULL, , _abort);
+g_assert_cmpuint(res, ==, (uint64_t)-value);
+
+/* BUG: value between INT64_MAX+1 and UINT64_MAX rejected */
+
+v = visitor_input_test_init(data, "18446744073709551574");
+
+visit_type_uint64(v, NULL, , );
+error_free_or_abort();
+}
+
 static void test_visitor_in_int_overflow(TestInputVisitorData *data,
  const void *unused)
 {
@@ -1225,6 +1253,8 @@ int main(int argc, char **argv)
 
 input_visitor_test_add("/visitor/input/int",
NULL, test_visitor_in_int);
+input_visitor_test_add("/visitor/input/uint",
+   NULL, test_visitor_in_uint);
 input_visitor_test_add("/visitor/input/int_overflow",
NULL, test_visitor_in_int_overflow);
 input_visitor_test_add("/visitor/input/int_keyval",
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 00/17] QAPI patches for 2017-03-22

[Markus Armbruster]

v3 (third time's a charm)
* Actually squash it into 16/17, not 17/17
v2:
* Leak fix squashed into 16/17

The following changes since commit 940a8ce075e3408742a4edcabfd6c2a15e2539eb:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2017-03-20 16:34:26 +)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2017-03-22-v3

for you to fetch changes up to 21f88d021d0d2b4ecee8f6cd6ca63a943a3ce71d:

  qapi: Fix QemuOpts visitor regression on unvisited input (2017-03-22 19:24:34 
+0100)


QAPI patches for 2017-03-22


Eric Blake (3):
  tests: Expose regression in QemuOpts visitor
  qom: Avoid unvisited 'id'/'qom-type' in user_creatable_add_opts
  qapi: Fix QemuOpts visitor regression on unvisited input

Markus Armbruster (14):
  test-keyval: Tweaks to improve list coverage
  keyval: Improve some comments
  test-keyval: Cover alternate and 'any' type
  keyval: Document issues with 'any' and alternate types
  MAINTAINERS: Add myself for files I touched recently
  qapi: Drop excessive Make dependencies on qapi2texi.py
  qapi2texi: Fix to actually fail when 'doc-required' is false
  qapi: Drop unused QAPIDoc member optional
  tests/qapi-schema: Make test-qapi.py print docs again
  tests/qapi-schema: Systematic positive doc comment tests
  qapi2texi: Fix translation of *strong* and _emphasized_
  qapi: Fix string input visitor regression for empty lists
  Revert "hostmem: fix QEMU crash by 'info memdev'"
  test-qobject-input-visitor: Cover visit_type_uint64()

 MAINTAINERS|  11 ++
 Makefile   |   7 +-
 backends/hostmem.c |  22 ++--
 qapi/opts-visitor.c|   6 +-
 qapi/string-input-visitor.c|   4 +
 qom/object_interfaces.c|   8 +-
 scripts/qapi.py|   1 -
 scripts/qapi2texi.py   |   5 +-
 tests/Makefile.include |  15 ++-
 tests/qapi-schema/doc-good.err |   0
 tests/qapi-schema/doc-good.exit|   1 +
 tests/qapi-schema/doc-good.json| 136 +
 tests/qapi-schema/doc-good.out | 148 ++
 tests/qapi-schema/doc-good.texi| 243 +
 tests/qapi-schema/test-qapi.py |  11 ++
 tests/test-keyval.c|  59 -
 tests/test-opts-visitor.c  |  27 -
 tests/test-qobject-input-visitor.c |  30 +
 tests/test-string-input-visitor.c  |  11 +-
 util/keyval.c  |  57 ++---
 20 files changed, 747 insertions(+), 55 deletions(-)
 create mode 100644 tests/qapi-schema/doc-good.err
 create mode 100644 tests/qapi-schema/doc-good.exit
 create mode 100644 tests/qapi-schema/doc-good.json
 create mode 100644 tests/qapi-schema/doc-good.out
 create mode 100644 tests/qapi-schema/doc-good.texi

-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 11/17] qapi2texi: Fix translation of *strong* and _emphasized_

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-7-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 scripts/qapi2texi.py| 4 ++--
 tests/qapi-schema/doc-good.texi | 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 5c4db78..9e01500 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -35,12 +35,12 @@ EXAMPLE_FMT = """@example
 
 def subst_strong(doc):
 """Replaces *foo* by @strong{foo}"""
-return re.sub(r'\*([^*\n]+)\*', r'@emph{\1}', doc)
+return re.sub(r'\*([^*\n]+)\*', r'@strong{\1}', doc)
 
 
 def subst_emph(doc):
 """Replaces _foo_ by @emph{foo}"""
-return re.sub(r'\b_([^_\n]+)_\b', r' @emph{\1} ', doc)
+return re.sub(r'\b_([^_\n]+)_\b', r'@emph{\1}', doc)
 
 
 def subst_vars(doc):
diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi
index 1160aaf..c410626 100644
--- a/tests/qapi-schema/doc-good.texi
+++ b/tests/qapi-schema/doc-good.texi
@@ -2,7 +2,7 @@
 
 @subsection Subsection
 
-@emph{strong}  @emph{with emphasis} 
+@strong{strong} @emph{with emphasis}
 @code{var} @{in braces@}
 @itemize @bullet
 @item
@@ -67,7 +67,7 @@ Example:
 Examples:
 @itemize @minus
 @item
-@emph{verbatim}
+@strong{verbatim}
 @item
 @{braces@}
 @end itemize
@@ -76,12 +76,12 @@ Examples:
 
 @deftp {Enum} Enum
 
-@subsection Produces @emph{invalid} texinfo
+@subsection Produces @strong{invalid} texinfo
 
 @b{Values:}
 @table @asis
 @item @code{one}
-The  @emph{one}  @{and only@}
+The @emph{one} @{and only@}
 @item @code{two}
 Not documented
 @end table
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 16/17] qom: Avoid unvisited 'id'/'qom-type' in user_creatable_add_opts

[Markus Armbruster]

From: Eric Blake 

A regression in commit 15c2f669e caused us to silently ignore
excess input to the QemuOpts visitor.  Later, commit ea4641
accidentally abused that situation, by removing "qom-type" and
"id" from the corresponding QDict but leaving them defined in
the QemuOpts, when using the pair of containers to create a
user-defined object. Note that since we are already traversing
two separate items (a QDict and a QemuOpts), we are already
able to flag bogus arguments, as in:

$ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -object 
memory-backend-ram,id=mem1,size=4k,bogus=huh
qemu-system-x86_64: -object memory-backend-ram,id=mem1,size=4k,bogus=huh: 
Property '.bogus' not found

So the only real concern is that when we re-enable strict checking
in the QemuOpts visitor, we do not want to start flagging the two
leftover keys as unvisited.  Rearrange the code to clean out the
QemuOpts listing in advance, rather than removing items from the
QDict.  Since "qom-type" is usually an automatic implicit default,
we don't have to restore it (this does mean that once instantiated,
QemuOpts is not necessarily an accurate representation of the
original command line - but this is not the first place to do that);
however "id" has to be put back (requiring us to cast away a const).

[As a side note, hmp_object_add() turns a QDict into a QemuOpts,
then calls user_creatable_add_opts() which converts QemuOpts into
a new QDict. There are probably a lot of wasteful conversions like
this, but cleaning them up is a much bigger task than the immediate
regression fix.]

CC: qemu-sta...@nongnu.org
Signed-off-by: Eric Blake 
Message-Id: <20170322144525.18964-3-ebl...@redhat.com>
Tested-by: Laurent Vivier 
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 qom/object_interfaces.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 03a95c3..9c271ad 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -114,7 +114,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error 
**errp)
 QDict *pdict;
 Object *obj;
 const char *id = qemu_opts_id(opts);
-const char *type = qemu_opt_get(opts, "qom-type");
+char *type = qemu_opt_get_del(opts, "qom-type");
 
 if (!type) {
 error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
@@ -122,17 +122,19 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error 
**errp)
 }
 if (!id) {
 error_setg(errp, QERR_MISSING_PARAMETER, "id");
+g_free(type);
 return NULL;
 }
 
+qemu_opts_set_id(opts, NULL);
 pdict = qemu_opts_to_qdict(opts, NULL);
-qdict_del(pdict, "qom-type");
-qdict_del(pdict, "id");
 
 v = opts_visitor_new(opts);
 obj = user_creatable_add_type(type, id, pdict, v, errp);
 visit_free(v);
 
+qemu_opts_set_id(opts, (char *) id);
+g_free(type);
 QDECREF(pdict);
 return obj;
 }
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 05/17] MAINTAINERS: Add myself for files I touched recently

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Message-Id: <1490014548-15083-6-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 MAINTAINERS | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 779c429..c60235e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1231,6 +1231,15 @@ M: Samuel Thibault 
 S: Maintained
 F: backends/baum.c
 
+Command line option argument parsing
+M: Markus Armbruster 
+S: Supported
+F: include/qemu/option.h
+F: tests/test-keyval.c
+F: tests/test-qemu-opts.c
+F: util/keyval.c
+F: util/qemu-option.c
+
 Coverity model
 M: Markus Armbruster 
 S: Supported
@@ -1365,7 +1374,9 @@ X: include/qapi/qmp/
 F: include/qapi/qmp/dispatch.h
 F: tests/qapi-schema/
 F: tests/test-*-visitor.c
+F: tests/test-qapi-*.c
 F: tests/test-qmp-*.c
+F: tests/test-visitor-serialization.c
 F: scripts/qapi*
 F: docs/qapi*
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 08/17] qapi: Drop unused QAPIDoc member optional

[Markus Armbruster]

Unused since commit aa964b7 "qapi2texi: Convert to QAPISchemaVisitor"

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-4-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 scripts/qapi.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e88c047..6c4d554 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -106,7 +106,6 @@ class QAPIDoc(object):
 self.name = name
 # the list of lines for this section
 self.content = []
-self.optional = False
 
 def append(self, line):
 self.content.append(line)
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 15/17] tests: Expose regression in QemuOpts visitor

[Markus Armbruster]

From: Eric Blake 

Commit 15c2f669e broke the ability of the QemuOpts visitor to
flag extra input parameters, but the regression went unnoticed
because of missing testsuite coverage.  Add a test to cover this;
take the approach already used in 9cb8ef3 of adding a test that
passes (to avoid breaking bisection) but marks with BUG the
behavior that we don't like, so that the actual impact of the
fix in a later patch is easier to see.

CC: qemu-sta...@nongnu.org
Signed-off-by: Eric Blake 
Reviewed-by: Michael Roth 
Message-Id: <20170322144525.18964-2-ebl...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 tests/test-opts-visitor.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index 2238f8e..8e0dda5 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -247,6 +247,24 @@ test_opts_range_beyond(void)
 qemu_opts_del(opts);
 }
 
+static void
+test_opts_dict_unvisited(void)
+{
+QemuOpts *opts;
+Visitor *v;
+UserDefOptions *userdef;
+
+opts = qemu_opts_parse(qemu_find_opts("userdef"), "i64x=0,bogus=1", false,
+   _abort);
+
+v = opts_visitor_new(opts);
+/* BUG: bogus should be diagnosed */
+visit_type_UserDefOptions(v, NULL, , _abort);
+visit_free(v);
+qemu_opts_del(opts);
+qapi_free_UserDefOptions(userdef);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -343,6 +361,8 @@ main(int argc, char **argv)
 g_test_add_func("/visitor/opts/range/beyond",
 test_opts_range_beyond);
 
+g_test_add_func("/visitor/opts/dict/unvisited", test_opts_dict_unvisited);
+
 g_test_run();
 return 0;
 }
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 13/17] Revert "hostmem: fix QEMU crash by 'info memdev'"

[Markus Armbruster]

This reverts commit 1454d33f0507cb54d62ed80f494884157c9e7130.

The string input visitor regression fixed in the previous commit made
visit_type_uint16List() fail on empty input.  query_memdev() calls it
via object_property_get_uint16List().  Because it doesn't expect it to
fail, it passes _abort, and duly crashes.

Commit 1454d33 "fixes" this crash by making
host_memory_backend_get_host_nodes() return a list containing just
MAX_NODES instead of the empty list.  Papers over the regression, and
leads to bogus "info memdev" output, as shown below; revert.

I suspect that if we had bisected the crash back then, we would have
found and fixed the actual bug instead of papering over it.

To reproduce, run HMP command "info memdev" with

$ qemu-system-x86_64 --nodefaults -S -display none -monitor stdio -object 
memory-backend-ram,id=mem1,size=4k

With this commit, "info memdev" prints

memory backend: mem1
  size:  4096
  merge: true
  dump: true
  prealloc: false
  policy: default
  host nodes:

exactly like before commit 74f24cb.

Between commit 1454d33 and this commit, it prints

memory backend: mem1
  size:  4096
  merge: true
  dump: true
  prealloc: false
  policy: default
  host nodes: 128

The last line is bogus.

Between commit 74f24cb and 1454d33, it crashes like this:

Unexpected error in parse_str() at 
/work/armbru/tmp/qemu/qapi/string-input-visitor.c:126:
Parameter 'null' expects an int64 value or range
Aborted (core dumped)

Cc: Xiao Guangrong 
Cc: Paolo Bonzini 
Signed-off-by: Markus Armbruster 
Message-Id: <1490026424-11330-3-git-send-email-arm...@redhat.com>
Reviewed-by: Michael Roth 
Reviewed-by: Eric Blake 
---
 backends/hostmem.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 162c218..89feb9e 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -64,14 +64,6 @@ out:
 error_propagate(errp, local_err);
 }
 
-static uint16List **host_memory_append_node(uint16List **node,
-unsigned long value)
-{
- *node = g_malloc0(sizeof(**node));
- (*node)->value = value;
- return &(*node)->next;
-}
-
 static void
 host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
@@ -82,23 +74,25 @@ host_memory_backend_get_host_nodes(Object *obj, Visitor *v, 
const char *name,
 unsigned long value;
 
 value = find_first_bit(backend->host_nodes, MAX_NODES);
-
-node = host_memory_append_node(node, value);
-
 if (value == MAX_NODES) {
-goto out;
+return;
 }
 
+*node = g_malloc0(sizeof(**node));
+(*node)->value = value;
+node = &(*node)->next;
+
 do {
 value = find_next_bit(backend->host_nodes, MAX_NODES, value + 1);
 if (value == MAX_NODES) {
 break;
 }
 
-node = host_memory_append_node(node, value);
+*node = g_malloc0(sizeof(**node));
+(*node)->value = value;
+node = &(*node)->next;
 } while (true);
 
-out:
 visit_type_uint16List(v, name, _nodes, errp);
 }
 
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 03/17] test-keyval: Cover alternate and 'any' type

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Message-Id: <1490014548-15083-4-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 tests/Makefile.include |  2 +-
 tests/test-keyval.c| 53 ++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 402e71c..86f9490 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -736,7 +736,7 @@ tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o 
$(test-util-obj-y) \
$(chardev-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): 
tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o $(test-util-obj-y)
-tests/test-keyval$(EXESUF): tests/test-keyval.o $(test-util-obj-y)
+tests/test-keyval$(EXESUF): tests/test-keyval.o $(test-util-obj-y) 
$(test-qapi-obj-y)
 tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o 
$(test-block-obj-y)
 tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y)
 tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index e97f6d5..ba19560 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -14,6 +14,7 @@
 #include "qapi/error.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi/qobject-input-visitor.h"
+#include "test-qapi-visit.h"
 #include "qemu/cutils.h"
 #include "qemu/option.h"
 
@@ -608,6 +609,56 @@ static void test_keyval_visit_optional(void)
 visit_free(v);
 }
 
+static void test_keyval_visit_alternate(void)
+{
+Error *err = NULL;
+Visitor *v;
+QDict *qdict;
+AltNumStr *ans;
+AltNumInt *ani;
+
+/*
+ * Can't do scalar alternate variants other than string.  You get
+ * the string variant if there is one, else an error.
+ */
+qdict = keyval_parse("a=1,b=2", NULL, _abort);
+v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+QDECREF(qdict);
+visit_start_struct(v, NULL, NULL, 0, _abort);
+visit_type_AltNumStr(v, "a", , _abort);
+g_assert_cmpint(ans->type, ==, QTYPE_QSTRING);
+g_assert_cmpstr(ans->u.s, ==, "1");
+visit_type_AltNumInt(v, "a", , );
+error_free_or_abort();
+visit_end_struct(v, NULL);
+visit_free(v);
+}
+
+static void test_keyval_visit_any(void)
+{
+Visitor *v;
+QDict *qdict;
+QObject *any;
+QList *qlist;
+QString *qstr;
+
+qdict = keyval_parse("a.0=null,a.1=1", NULL, _abort);
+v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+QDECREF(qdict);
+visit_start_struct(v, NULL, NULL, 0, _abort);
+visit_type_any(v, "a", , _abort);
+qlist = qobject_to_qlist(any);
+g_assert(qlist);
+qstr = qobject_to_qstring(qlist_pop(qlist));
+g_assert_cmpstr(qstring_get_str(qstr), ==, "null");
+qstr = qobject_to_qstring(qlist_pop(qlist));
+g_assert_cmpstr(qstring_get_str(qstr), ==, "1");
+g_assert(qlist_empty(qlist));
+visit_check_struct(v, _abort);
+visit_end_struct(v, NULL);
+visit_free(v);
+}
+
 int main(int argc, char *argv[])
 {
 g_test_init(, , NULL);
@@ -619,6 +670,8 @@ int main(int argc, char *argv[])
 g_test_add_func("/keyval/visit/dict", test_keyval_visit_dict);
 g_test_add_func("/keyval/visit/list", test_keyval_visit_list);
 g_test_add_func("/keyval/visit/optional", test_keyval_visit_optional);
+g_test_add_func("/keyval/visit/alternate", test_keyval_visit_alternate);
+g_test_add_func("/keyval/visit/any", test_keyval_visit_any);
 g_test_run();
 return 0;
 }
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 01/17] test-keyval: Tweaks to improve list coverage

[Markus Armbruster]

We have a negative test case for a list index with leading zero.  Add
positive ones.

Tweak the test case for list index greater or equal the number of
elements: test "equal" instead of "greater" to guard against
off-by-one mistakes.

Signed-off-by: Markus Armbruster 
Message-Id: <1490014548-15083-2-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 tests/test-keyval.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index 71288b0..e97f6d5 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -218,14 +218,14 @@ static void test_keyval_parse_list(void)
 QDECREF(qdict);
 
 /* Multiple indexes, last one wins */
-qdict = keyval_parse("list.1=goner,list.0=null,list.1=eins,list.2=zwei",
+qdict = keyval_parse("list.1=goner,list.0=null,list.01=eins,list.2=zwei",
  NULL, _abort);
 g_assert_cmpint(qdict_size(qdict), ==, 1);
 check_list012(qdict_get_qlist(qdict, "list"));
 QDECREF(qdict);
 
 /* List at deeper nesting */
-qdict = keyval_parse("a.list.1=eins,a.list.0=null,a.list.2=zwei",
+qdict = keyval_parse("a.list.1=eins,a.list.00=null,a.list.2=zwei",
  NULL, _abort);
 g_assert_cmpint(qdict_size(qdict), ==, 1);
 sub_qdict = qdict_get_qdict(qdict, "a");
@@ -242,7 +242,7 @@ static void test_keyval_parse_list(void)
 g_assert(!qdict);
 
 /* Missing list indexes */
-qdict = keyval_parse("list.2=lonely", NULL, );
+qdict = keyval_parse("list.1=lonely", NULL, );
 error_free_or_abort();
 g_assert(!qdict);
 qdict = keyval_parse("list.0=null,list.2=eins,list.02=zwei", NULL, );
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 12/17] qapi: Fix string input visitor regression for empty lists

[Markus Armbruster]

Visiting a list when input is the empty string should result in an
empty list, not an error.  Noticed when commit 3d089ce belatedly added
tests, but simply accepted as weird then.  It's actually a regression:
broken in commit 74f24cb, v2.7.0.  Fix it, and throw in another test
case for empty string.

Signed-off-by: Markus Armbruster 
Message-Id: <1490026424-11330-2-git-send-email-arm...@redhat.com>
Reviewed-by: Michael Roth 
Reviewed-by: Eric Blake 
---
 qapi/string-input-visitor.c   |  4 
 tests/test-string-input-visitor.c | 11 ---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 806b01ae..c089491 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -54,6 +54,10 @@ static int parse_str(StringInputVisitor *siv, const char 
*name, Error **errp)
 return 0;
 }
 
+if (!*str) {
+return 0;
+}
+
 do {
 errno = 0;
 start = strtoll(str, , 0);
diff --git a/tests/test-string-input-visitor.c 
b/tests/test-string-input-visitor.c
index 6db850b..79313a7 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -63,6 +63,11 @@ static void test_visitor_in_int(TestInputVisitorData *data,
 
 visit_type_int(v, NULL, , );
 error_free_or_abort();
+
+v = visitor_input_test_init(data, "");
+
+visit_type_int(v, NULL, , );
+error_free_or_abort();
 }
 
 static void check_ilist(Visitor *v, int64_t *expected, size_t n)
@@ -140,11 +145,11 @@ static void test_visitor_in_intList(TestInputVisitorData 
*data,
 v = visitor_input_test_init(data, "18446744073709551615");
 check_ulist(v, expect4, ARRAY_SIZE(expect4));
 
-/* Empty list is invalid (weird) */
+/* Empty list */
 
 v = visitor_input_test_init(data, "");
-visit_type_int64List(v, NULL, , );
-error_free_or_abort();
+visit_type_int64List(v, NULL, , _abort);
+g_assert(!res);
 
 /* Not a list */
 
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 04/17] keyval: Document issues with 'any' and alternate types

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Message-Id: <1490014548-15083-5-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 util/keyval.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/util/keyval.c b/util/keyval.c
index 46cd540..93d5db6 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -61,6 +61,16 @@
  * "key absent" already means "optional object/array absent", which
  * isn't the same as "empty object/array present".
  *
+ * Design flaw: scalar values can only be strings; there is no way to
+ * denote numbers, true, false or null.  The special QObject input
+ * visitor returned by qobject_input_visitor_new_keyval() mostly hides
+ * this by automatically converting strings to the type the visitor
+ * expects.  Breaks down for alternate types and type 'any', where the
+ * visitor's expectation isn't clear.  Code visiting such types needs
+ * to do the conversion itself, but only when using this keyval
+ * visitor.  Awkward.  Alternate types without a string member don't
+ * work at all.
+ *
  * Additional syntax for use with an implied key:
  *
  *   key-vals-ik  = val-no-key [ ',' key-vals ]
-- 
2.7.4

[Qemu-devel] [PULL v3 for-2.9 07/17] qapi2texi: Fix to actually fail when 'doc-required' is false

[Markus Armbruster]

Messed up in commit bc52d03.

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-3-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 scripts/qapi2texi.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 8eed11a..5c4db78 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -292,6 +292,7 @@ def main(argv):
 if not qapi.doc_required:
 print >>sys.stderr, ("%s: need pragma 'doc-required' "
  "to generate documentation" % argv[0])
+sys.exit(1)
 print texi_schema(schema)
 
 
-- 
2.7.4

Re: [Qemu-devel] [Xen-devel] [PATCH v4 0/8] xen/9pfs: introduce the Xen 9pfs backend

[Stefano Stabellini]

On Wed, 22 Mar 2017, Greg Kurz wrote:
> On Tue, 21 Mar 2017 13:14:02 -0700 (PDT)
> Stefano Stabellini  wrote:
> 
> > On Tue, 21 Mar 2017, Greg Kurz wrote:
> > > On Mon, 20 Mar 2017 11:18:46 -0700 (PDT)
> > > Stefano Stabellini  wrote:
> > >   
> > > > Hi all,
> > > > 
> > > > This patch series implements a new transport for 9pfs, aimed at Xen
> > > > systems.
> > > > 
> > > > The transport is based on a traditional Xen frontend and backend drivers
> > > > pair. This patch series implements the backend, which typically runs in
> > > > Dom0. I sent another series to implement the frontend in Linux
> > > > (http://marc.info/?l=linux-kernel=148883047125960=2).
> > > > 
> > > > The backend complies to the Xen transport for 9pfs specification
> > > > version 1, available here:
> > > > 
> > > > https://xenbits.xen.org/docs/unstable/misc/9pfs.html
> > > > 
> > > > 
> > > > Changes in v4:
> > > > - add reviewed-bys
> > > > - remove useless if(NULL) checks around g_free
> > > > - g_free g_malloc'ed sgs
> > > > - remove XEN_9PFS_RING_ORDER, make the ring order dynamic per ring,
> > > >   reading the ring_order field in xen_9pfs_data_intf
> > > > - remove patch not to build Xen backends on non-Xen capable targets
> > > >   because it is already upstream
> > > >   
> > > 
> > > Hi Stefano,
> > > 
> > > This looks good to me. Do you want these patches to go through my 9p
> > > tree or through your xen tree ?  
> > 
> > Thanks Greg! It can work both ways. If you have any changes in your queue
> > that could conflict with this, it's best to go via your tree.
> > 
> > Otherwise, I'll merge it in mine, so that I can keep an eye on the
> > correspondent Xen changes to the header files and make sure they are in
> > sync (specifically http://marc.info/?l=qemu-devel=149003412910278).
> > 
> 
> I don't have any conflicting patches on my side. Please merge this in your
> tree (as well as the MAINTAINERS patch).

All right, thanks! I'll add a reviewed-by you on all patches if for you
is OK.


> > 
> > >  Also, I guess you may want to add
> > > F: hw/9pfs/xen-9p-backend.c to the Xen section in MAINTAINERS.  
> > 
> > I'll send a patch to be applied on top of the series
> > 
> > 
> > > --
> > > Greg
> > >   
> > > > Changes in v3:
> > > > - do not build backends for targets that do not support xen
> > > > - remove xen_9pfs.h, merge its content into xen-9p-backend.c
> > > > - remove xen_9pfs_header, introduce P9MsgHeader
> > > > - use le32_to_cpu to access P9MsgHeader fields
> > > > - many coding style fixes
> > > > - run checkpatch on all patches
> > > > - add check if num_rings < 1
> > > > - use g_strdup_printf
> > > > - free fsdev_id in xen_9pfs_free
> > > > - add comments
> > > > 
> > > > Changes in v2:
> > > > - fix coding style
> > > > - compile xen-9p-backend.c if CONFIG_XEN_BACKEND
> > > > - add patch to set CONFIG_XEN_BACKEND only for the right targets
> > > > - add review-bys
> > > > 
> > > > 
> > > > Stefano Stabellini (8):
> > > >   xen: import ring.h from xen
> > > >   9p: introduce a type for the 9p header
> > > >   xen/9pfs: introduce Xen 9pfs backend
> > > >   xen/9pfs: connect to the frontend
> > > >   xen/9pfs: receive requests from the frontend
> > > >   xen/9pfs: implement in/out_iov_from_pdu and vmarshal/vunmarshal
> > > >   xen/9pfs: send responses back to the frontend
> > > >   xen/9pfs: build and register Xen 9pfs backend
> > > > 
> > > >  hw/9pfs/9p.h |   6 +
> > > >  hw/9pfs/Makefile.objs|   1 +
> > > >  hw/9pfs/virtio-9p-device.c   |   6 +-
> > > >  hw/9pfs/xen-9p-backend.c | 444 
> > > > +
> > > >  hw/block/xen_blkif.h |   2 +-
> > > >  hw/usb/xen-usb.c |   2 +-
> > > >  hw/xen/xen_backend.c |   3 +
> > > >  include/hw/xen/io/ring.h | 455 
> > > > +++
> > > >  include/hw/xen/xen_backend.h |   3 +
> > > >  9 files changed, 915 insertions(+), 7 deletions(-)
> > > >  create mode 100644 hw/9pfs/xen-9p-backend.c
> > > >  create mode 100644 include/hw/xen/io/ring.h  
> > > 
> > >   
> 
> 

Re: [Qemu-devel] [Qemu-ppc] [Bug 1674925] Re: Qemu PPC64 kvm no display if --device virtio-gpu-pci is selected

[Cédric Le Goater]

Hello Luigi,

On 03/22/2017 06:49 PM, luigi burdo wrote:

> i have the 4.11rc1 . onfedora 25 ppc 64 on both machine Qoriq and on G5Quad.
> 
> On the 2.8 this issue isnt presentbut
> I did the test o Qoriq  e5500 a book3e processor and on 2.8 if i made:
> 
> qemu-system-ppc64 --enable-kvm the true result is:
> qemu-system-ppc64: Unable to find CPU definition: host
> 
> on qemu 2.9 rc1
> 
> ./qemu-system-ppc64 --enable-kvm i have :
> qemu-system-ppc64: KVM and IRQ_XICS capability must be present for in-kernel 
> XICS

The default machine for qemu-system-ppc64 is pseries. 

The error message for QEMU 2.8 means that it fails to initialize 
the machine in ppc_cpu_parse_features() which is a bit late. 
For QEMU 2.9, it fails at the right beginning in xics_system_init(), 
before the CPUs are initialized, which is better, I think.

I admit the message is not very clear, but the host kernel is 
using a dev config.

> On Qoriq if i pass the -cpu e500 (normal thing) all is working right qemu 
> 2.9rc1 
> all boot and so and so.

but you must be changing the machine right ? 

> On G5 the -cpu variable dont fix the issue.

with which machine ? 


> I can build a new kernel release , usually mine dont have xics enabled 
> because G5 
> dont have that feature, if needed i can enable it for testing.**

Yes that would be interesting. 

Thanks,

C. 

> 
> Hope my english is understandable.
> 
> ciao
> 
> Luigi
> 
> 
> --
> *Da:* Qemu-ppc  per 
> conto di Cédric Le Goater 
> *Inviato:* mercoledì 22 marzo 2017 18.29
> *A:* Thomas Huth; Bug 1674925; qemu-devel@nongnu.org
> *Cc:* qemu-...@nongnu.org
> *Oggetto:* Re: [Qemu-ppc] [Qemu-devel] [Bug 1674925] Re: Qemu PPC64 kvm no 
> display if --device virtio-gpu-pci is selected
>  
> On 03/22/2017 03:15 PM, Thomas Huth wrote:
>> On 22.03.2017 14:35, luigiburdo wrote:
>>> Hi Thomas with 2.9 rc1 i have this with --enable-kvm
>>>
>>> emu-system-ppc64 --enable-kvm
>>> qemu-system-ppc64: KVM and IRQ_XICS capability must be present for 
>>> in-kernel XICS
>>>
>>> and the qemu dont run.
>> 
>> Does it exit, or just hang afterwards? Was this with or without --device
>> virtio-gpu-pci option? Do you get any output if you run QEMU with
>> "-nographic" instead?
> 
> I guess this is an issue with the host kernel. Which one are you running ?
> 
> C.
> 
> 

Re: [Qemu-devel] [PATCH v1 2/2] reduce qemu's heap Rss size from 12252kB to 2752KB

[Xu, Anthony]

> So please send it to the list with Signed-off-by line.
Thanks, 

> 
> DPRINTF("handle_mmio\n");
> /* Called outside BQL */
> address_space_rw(_space_memory,
>  run->mmio.phys_addr, attrs,
>  run->mmio.data,
>  run->mmio.len,
>  run->mmio.is_write);
> 
> and adding a simple assert() would have quickly disproved your theory.
You are right here.
If it is a PCI bar write, a memory region operation(del/add) may 
be called inside address_space_rw, and 
memory_region_transaction_commit will be called.
If address_space_rw is called  without the global lock, 
memory_region_transaction_commit is called with the global lock.
 It conflicts with what you said before.

>No, you don't need to check it.  Most functions (in this case,
>memory_region_transaction_commit) can only be called under the global lock.

And if two vcpus program the different PCI bars at the same time 
(it is unlikely, but QEMU should not assume it), without the global lock, 
region operations may be called at the same time. 
Are memory_region_del_subregion and 
memory_region_add_subregion_overlap thread-safe?

> It's not a fix if the code is not thread-safe anymore!  But I think you
> have the answer now as to why you cannot use synchronize_rcu.

Can you elaborate why the code is not thread-safe?

Thanks
Anthony

Re: [Qemu-devel] [PATCH v2 for-2.9] block: Declare blockdev-add and blockdev-del supported

[Markus Armbruster]

Max Reitz  writes:

> On 22.03.2017 17:20, Markus Armbruster wrote:
>> It's been a long journey, but here we are.
>> 
>> The supported blockdev-add is not compatible to its experimental
>> predecessors; bump all Since: tags to 2.9.
>> 
>> x-blockdev-remove-medium, x-blockdev-insert-medium and
>> x-blockdev-change need a bit more work, so leave them alone for now.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>> v2:
>> * Another "command is considered experimental" comment dropped [Max]
>> * More Since: tags updated to 2.9, commit message improved, R-bys dropped
>
> Do you want to do this recursively? There are things such as
> BlkdebugSetStateOptions or ReplicationMode which are only used by the
> corresponding BlockdevOptions*, so they too are basically only supported
> as of 2.9.

Fixup appended.

> I don't have a strong opinion, but I think I personally wouldn't bother
> with any of these structures and just update the tag for blockdev-add
> itself...

Kevin, what about you?

Since: tags on types are kind of redundant, as types become part of the
public interface only via commands and events.  The real Since: is the
oldest command or event exposing it.  But explaining that is too
complicated.  It's better to tell people to always add Since: tags.

Now if the QAPI generators could tell them when to add and when not to
add Since: tags...

> You being fine with me would be enough to convince me, so:
>
> Reviewed-by: Max Reitz 

Thanks!



diff --git a/qapi/block-core.json b/qapi/block-core.json
index bd39b3f..f938316 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2053,7 +2053,7 @@
 # @ignore:  Ignore the request
 # @unmap:   Forward as an unmap request
 #
-# Since: 1.7
+# Since: 2.9
 ##
 { 'enum': 'BlockdevDiscardOptions',
   'data': [ 'ignore', 'unmap' ] }
@@ -2082,7 +2082,7 @@
 # @threads: Use qemu's thread pool
 # @native:  Use native AIO backend (only Linux and Windows)
 #
-# Since: 1.7
+# Since: 2.9
 ##
 { 'enum': 'BlockdevAioOptions',
   'data': [ 'threads', 'native' ] }
@@ -2097,7 +2097,7 @@
 # @no-flush:ignore any flush requests for the device (default:
 #   false)
 #
-# Since: 1.7
+# Since: 2.9
 ##
 { 'struct': 'BlockdevCacheOptions',
   'data': { '*direct': 'bool',
@@ -2229,7 +2229,7 @@
 #
 # @all: Perform all available overlap checks
 #
-# Since: 2.2
+# Since: 2.9
 ##
 { 'enum': 'Qcow2OverlapCheckMode',
   'data': [ 'none', 'constant', 'cached', 'all' ] }
@@ -2244,7 +2244,7 @@
 # @template: Specifies a template mode which can be adjusted using the other
 #flags, defaults to 'cached'
 #
-# Since: 2.2
+# Since: 2.9
 ##
 { 'struct': 'Qcow2OverlapCheckFlags',
   'data': { '*template':   'Qcow2OverlapCheckMode',
@@ -2268,7 +2268,7 @@
 #
 # @mode:named mode which chooses a specific set of flags
 #
-# Since: 2.2
+# Since: 2.9
 ##
 { 'alternate': 'Qcow2OverlapChecks',
   'data': { 'flags': 'Qcow2OverlapCheckFlags',
@@ -2349,7 +2349,7 @@
 #
 # Trigger events supported by blkdebug.
 #
-# Since: 2.0
+# Since: 2.9
 ##
 { 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
   'data': [ 'l1_update', 'l1_grow_alloc_table', 'l1_grow_write_table',
@@ -2389,7 +2389,7 @@
 #
 # @immediately: fail immediately; defaults to false
 #
-# Since: 2.0
+# Since: 2.9
 ##
 { 'struct': 'BlkdebugInjectErrorOptions',
   'data': { 'event': 'BlkdebugEvent',
@@ -2412,7 +2412,7 @@
 # @new_state:   the state identifier blkdebug is supposed to assume if
 #   this event is triggered
 #
-# Since: 2.0
+# Since: 2.9
 ##
 { 'struct': 'BlkdebugSetStateOptions',
   'data': { 'event': 'BlkdebugEvent',
@@ -2468,7 +2468,7 @@
 #
 # @fifo: read only from the first child that has not failed
 #
-# Since: 2.2
+# Since: 2.9
 ##
 { 'enum': 'QuorumReadPattern', 'data': [ 'quorum', 'fifo' ] }
 
@@ -2674,7 +2674,7 @@
 #
 # @secondary: Secondary mode, receive the vm's state from primary QEMU.
 #
-# Since: 2.8
+# Since: 2.9
 ##
 { 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }
 
@@ -2703,7 +2703,7 @@
 #
 # @inet:TCP transport
 #
-# Since: 2.8
+# Since: 2.9
 ##
 { 'enum': 'NFSTransport',
   'data': [ 'inet' ] }
@@ -2717,7 +2717,7 @@
 #
 # @host:host address for NFS server
 #
-# Since: 2.8
+# Since: 2.9
 ##
 { 'struct': 'NFSServer',
   'data': { 'type': 'NFSTransport',

Re: [Qemu-devel] [PATCH 2/2] configure: use pkg-config for obtaining xen version

[Stefano Stabellini]

On Wed, 22 Mar 2017, Juergen Gross wrote:
> On 21/03/17 19:54, Stefano Stabellini wrote:
> > On Tue, 21 Mar 2017, Juergen Gross wrote:
> >> On 17/03/17 19:33, Stefano Stabellini wrote:
> >>> On Fri, 17 Mar 2017, Juergen Gross wrote:
>  On 16/03/17 21:20, Stefano Stabellini wrote:
> > On Thu, 16 Mar 2017, Juergen Gross wrote:
> >> Instead of trying to guess the Xen version to use by compiling various
> >> test programs first just ask the system via pkg-config. Only if it
> >> can't return the version fall back to the test program scheme.
> >
> > That's OK, but why did you remove the Xen unstable test?
> 
>  >From Xen 4.9 on pkg-config will return the needed information. There is
>  no longer a need for a test program to determine the Xen version. After
>  all this was the main objective of my series adding the pkg-config
>  files to Xen.
> >>>
> >>> I was going to say something like "yeah, but is pkg-config always
> >>> available?" In reality, QEMU already has pkg-config as build
> >>> dependency, so I guess there is no problem with that.
> >>>
> >>> Please add a note about this to the commit message.
> >>>
> >>
> >> Okay.
> > 
> > Sorry to point this out only now, and I realize that it might be
> > unimportant for production builds, but it is important to me, and
> > developers in general, to be able to test a single QEMU tree against a
> > number of Xen trees (all releases from 4.3 onward).
> > 
> > With this change (specifically dropping the 4.9 build test), out of tree
> > builds don't work anymore. I would like to be able to do:
> > 
> > ./configure --enable-xen --target-list=i386-softmmu \
> > --extra-cflags="-I$DIR/tools/include \
> > -I$DIR/tools/libs/toollog/include \
> > -I$DIR/tools/libs/evtchn/include \
> > -I$DIR/tools/libs/gnttab/include \
> > -I$DIR/tools/libs/foreignmemory/include \
> > -I$DIR/tools/libs/devicemodel/include \
> > -I$DIR/tools/libxc/include \
> > -I$DIR/tools/xenstore/include \
> > -I$DIR/tools/xenstore/compat/include" \
> > --extra-ldflags="-L$DIR/tools/libxc \
> > -L$DIR/tools/xenstore \
> > -L$DIR/tools/libs/evtchn \
> > -L$DIR/tools/libs/gnttab \
> > -L$DIR/tools/libs/foreignmemory \
> > -L$DIR/tools/libs/devicemodel \
> > -Wl,-rpath-link=$DIR/tools/libs/toollog \
> > -Wl,-rpath-link=$DIR/tools/libs/evtchn \
> > -Wl,-rpath-link=$DIR/tools/libs/gnttab \
> > -Wl,-rpath-link=$DIR/tools/libs/call \
> > -Wl,-rpath-link=$DIR/tools/libs/foreignmemory \
> > -Wl,-rpath-link=$DIR/tools/libs/devicemodel" \
> > --disable-kvm 
> > make
> > 
> > And the make should succeed. Is there a way to do that with pkg-config?
> 
> Sure, for Xen 4.9 just do:
> 
> PKG_CONFIG_PATH=$(DIR)/tools/pkg-config ./configure \
>   --enable-xen --target-list=i386-softmmu \
>   --disable-kvm
> make

Yes, that works, thanks! I committed it to my next branch adding
"pkg-config, which is already a build dependency of QEMU, will be used
exclusively to determine the Xen version from Xen 4.9 onward." to the
commit message.

Re: [Qemu-devel] [PATCH v2 for-2.9] block: Declare blockdev-add and blockdev-del supported

[Eric Blake]

On 03/22/2017 12:13 PM, Max Reitz wrote:
> On 22.03.2017 17:20, Markus Armbruster wrote:
>> It's been a long journey, but here we are.
>>
>> The supported blockdev-add is not compatible to its experimental
>> predecessors; bump all Since: tags to 2.9.
>>
>> x-blockdev-remove-medium, x-blockdev-insert-medium and
>> x-blockdev-change need a bit more work, so leave them alone for now.
>>
>> Signed-off-by: Markus Armbruster 
>> ---
>> v2:
>> * Another "command is considered experimental" comment dropped [Max]
>> * More Since: tags updated to 2.9, commit message improved, R-bys dropped
> 
> Do you want to do this recursively? There are things such as
> BlkdebugSetStateOptions or ReplicationMode which are only used by the
> corresponding BlockdevOptions*, so they too are basically only supported
> as of 2.9.
> 
> I don't have a strong opinion, but I think I personally wouldn't bother
> with any of these structures and just update the tag for blockdev-add
> itself...

I also don't have a strong opinion, and think it may actually be a bit
more time consuming to audit which types are recursively used only by
blockdev-add (and no where else) than to just leave them be.  But I'm
also okay if you like bumping the numbers to the point where the struct
was marked stable, and where we have to start thinking about back-compat
ramifications when modifying the struct.

> 
> You being fine with me would be enough to convince me, so:
> 
> Reviewed-by: Max Reitz 

Likewise.

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL v2 for-2.9 10/17] tests/qapi-schema: Systematic positive doc comment tests

[Markus Armbruster]

We have a number of negative tests, but we don't have systematic
positive coverage.  Fix that.

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-6-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 tests/Makefile.include  |  13 ++-
 tests/qapi-schema/doc-good.err  |   0
 tests/qapi-schema/doc-good.exit |   1 +
 tests/qapi-schema/doc-good.json | 136 ++
 tests/qapi-schema/doc-good.out  | 148 
 tests/qapi-schema/doc-good.texi | 243 
 6 files changed, 537 insertions(+), 4 deletions(-)
 create mode 100644 tests/qapi-schema/doc-good.err
 create mode 100644 tests/qapi-schema/doc-good.exit
 create mode 100644 tests/qapi-schema/doc-good.json
 create mode 100644 tests/qapi-schema/doc-good.out
 create mode 100644 tests/qapi-schema/doc-good.texi

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 86f9490..f3de81f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -379,6 +379,7 @@ qapi-schema += doc-duplicated-since.json
 qapi-schema += doc-empty-arg.json
 qapi-schema += doc-empty-section.json
 qapi-schema += doc-empty-symbol.json
+qapi-schema += doc-good.json
 qapi-schema += doc-interleaved-section.json
 qapi-schema += doc-invalid-end.json
 qapi-schema += doc-invalid-end2.json
@@ -607,6 +608,9 @@ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json 
$(SRC_PATH)/scripts/qapi-int
$(gen-out-type) -o tests -p "test-" $<, \
"GEN","$@")
 
+tests/qapi-schema/doc-good.test.texi: 
$(SRC_PATH)/tests/qapi-schema/doc-good.json $(SRC_PATH)/scripts/qapi2texi.py 
$(qapi-py)
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > 
$@,"GEN","$@")
+
 tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o 
$(test-qapi-obj-y)
 tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o 
$(test-qapi-obj-y)
 tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y)
@@ -856,9 +860,6 @@ QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) = 
tests/qemu-iotests/socket_scm_helper$(EXE
 check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh 
qemu-img$(EXESUF) qemu-io$(EXESUF) $(QEMU_IOTESTS_HELPERS-y)
$<
 
-.PHONY: check-tests/test-qapi.py
-check-tests/test-qapi.py: tests/test-qapi.py
-
 .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
 $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: 
$(SRC_PATH)/%.json
$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
@@ -871,10 +872,14 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): 
check-%.json: $(SRC_PATH)/%.json
@perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q 
$(SRC_PATH)/$*.err -
@diff -q $(SRC_PATH)/$*.exit $*.test.exit
 
+.PHONY: check-tests/qapi-schema/doc-good.texi
+check-tests/qapi-schema/doc-good.texi: tests/qapi-schema/doc-good.test.texi
+   @diff -q $(SRC_PATH)/tests/qapi-schema/doc-good.texi $<
+
 # Consolidated targets
 
 .PHONY: check-qapi-schema check-qtest check-unit check check-clean
-check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y))
+check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) 
check-tests/qapi-schema/doc-good.texi
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 check-unit: $(patsubst %,check-%, $(check-unit-y))
 check-block: $(patsubst %,check-%, $(check-block-y))
diff --git a/tests/qapi-schema/doc-good.err b/tests/qapi-schema/doc-good.err
new file mode 100644
index 000..e69de29
diff --git a/tests/qapi-schema/doc-good.exit b/tests/qapi-schema/doc-good.exit
new file mode 100644
index 000..573541a
--- /dev/null
+++ b/tests/qapi-schema/doc-good.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
new file mode 100644
index 000..cfdc0a8
--- /dev/null
+++ b/tests/qapi-schema/doc-good.json
@@ -0,0 +1,136 @@
+# -*- Mode: Python -*-
+# Positive QAPI doc comment tests
+
+{ 'pragma': { 'doc-required': true } }
+
+##
+# = Section
+#
+# == Subsection
+#
+# *strong* _with emphasis_
+# @var {in braces}
+# * List item one
+# - Two, multiple
+#   lines
+#
+# 3. Three
+# Still in list
+#
+#   Not in list
+# - Second list
+# Note: still in list
+#
+# Note: not in list
+# 1. Third list
+#is numbered
+#
+# - another item
+#
+# | example
+# | multiple lines
+#
+# Returns: the King
+# Since: the first age
+# Notes:
+#
+# 1. Lorem ipsum dolor sit amet
+#
+# 2. Ut enim ad minim veniam
+#
+# Duis aute irure dolor
+#
+# Example:
+#
+# -> in
+# <- out
+# Examples:
+# - *verbatim*
+# - {braces}
+##
+
+##
+# @Enum:
+# == Produces *invalid* texinfo
+# @one: The _one_ {and only}
+#
+# @two is undocumented
+##
+{ 'enum': 'Enum', 'data': [ 'one', 'two' ] }
+
+##
+# @Base:
+# @base1:
+#   the first member
+##
+{ 'struct': 'Base', 'data': { 'base1': 'Enum' } }
+
+##
+# @Variant1:
+# A paragraph
+#
+# Another paragraph (but no @var: line)

[Qemu-devel] [PULL v2 for-2.9 17/17] qapi: Fix QemuOpts visitor regression on unvisited input

[Markus Armbruster]

From: Eric Blake 

An off-by-one in commit 15c2f669e meant that we were failing to
check for unparsed input in all QemuOpts visitors.  Recent testsuite
additions show that fixing the obvious bug with bogus fields will
also fix the case of an incomplete list visit; update the tests to
match the new behavior.

Simple testcase:

./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -numa 
node,size=1g

failed to diagnose that 'size' is not a valid argument to -numa, and
now once again reports:

qemu-system-x86_64: -numa node,size=1g: Invalid parameter 'size'

See also https://bugzilla.redhat.com/show_bug.cgi?id=1434666

CC: qemu-sta...@nongnu.org
Signed-off-by: Eric Blake 
Reviewed-by: Michael Roth 
Tested-by: Laurent Vivier 
Message-Id: <20170322144525.18964-4-ebl...@redhat.com>
Reviewed-by: Markus Armbruster 
[Fixup squashed in]
Signed-off-by: Markus Armbruster 
---
 qapi/opts-visitor.c   |  6 +++---
 qom/object_interfaces.c   |  1 +
 tests/test-opts-visitor.c | 13 -
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 026d25b..324b197 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -164,7 +164,7 @@ opts_check_struct(Visitor *v, Error **errp)
 GHashTableIter iter;
 GQueue *any;
 
-if (ov->depth > 0) {
+if (ov->depth > 1) {
 return;
 }
 
@@ -276,8 +276,8 @@ static void
 opts_check_list(Visitor *v, Error **errp)
 {
 /*
- * FIXME should set error when unvisited elements remain.  Mostly
- * harmless, as the generated visits always visit all elements.
+ * Unvisited list elements will be reported later when checking
+ * whether unvisited struct members remain.
  */
 }
 
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index cc9a694..9c271ad 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -122,6 +122,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error 
**errp)
 }
 if (!id) {
 error_setg(errp, QERR_MISSING_PARAMETER, "id");
+g_free(type);
 return NULL;
 }
 
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index 8e0dda5..23e8970 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -175,6 +175,7 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer 
test_data)
 static void
 test_opts_range_unvisited(void)
 {
+Error *err = NULL;
 intList *list = NULL;
 intList *tail;
 QemuOpts *opts;
@@ -199,10 +200,11 @@ test_opts_range_unvisited(void)
 g_assert_cmpint(tail->value, ==, 1);
 tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list));
 g_assert(tail);
-visit_check_list(v, _abort); /* BUG: unvisited tail not reported */
+visit_check_list(v, _abort); /* unvisited tail ignored until... */
 visit_end_list(v, (void **));
 
-visit_check_struct(v, _abort);
+visit_check_struct(v, ); /* ...here */
+error_free_or_abort();
 visit_end_struct(v, NULL);
 
 qapi_free_intList(list);
@@ -250,6 +252,7 @@ test_opts_range_beyond(void)
 static void
 test_opts_dict_unvisited(void)
 {
+Error *err = NULL;
 QemuOpts *opts;
 Visitor *v;
 UserDefOptions *userdef;
@@ -258,11 +261,11 @@ test_opts_dict_unvisited(void)
_abort);
 
 v = opts_visitor_new(opts);
-/* BUG: bogus should be diagnosed */
-visit_type_UserDefOptions(v, NULL, , _abort);
+visit_type_UserDefOptions(v, NULL, , );
+error_free_or_abort();
 visit_free(v);
 qemu_opts_del(opts);
-qapi_free_UserDefOptions(userdef);
+g_assert(!userdef);
 }
 
 int
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 15/17] tests: Expose regression in QemuOpts visitor

[Markus Armbruster]

From: Eric Blake 

Commit 15c2f669e broke the ability of the QemuOpts visitor to
flag extra input parameters, but the regression went unnoticed
because of missing testsuite coverage.  Add a test to cover this;
take the approach already used in 9cb8ef3 of adding a test that
passes (to avoid breaking bisection) but marks with BUG the
behavior that we don't like, so that the actual impact of the
fix in a later patch is easier to see.

CC: qemu-sta...@nongnu.org
Signed-off-by: Eric Blake 
Reviewed-by: Michael Roth 
Message-Id: <20170322144525.18964-2-ebl...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 tests/test-opts-visitor.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
index 2238f8e..8e0dda5 100644
--- a/tests/test-opts-visitor.c
+++ b/tests/test-opts-visitor.c
@@ -247,6 +247,24 @@ test_opts_range_beyond(void)
 qemu_opts_del(opts);
 }
 
+static void
+test_opts_dict_unvisited(void)
+{
+QemuOpts *opts;
+Visitor *v;
+UserDefOptions *userdef;
+
+opts = qemu_opts_parse(qemu_find_opts("userdef"), "i64x=0,bogus=1", false,
+   _abort);
+
+v = opts_visitor_new(opts);
+/* BUG: bogus should be diagnosed */
+visit_type_UserDefOptions(v, NULL, , _abort);
+visit_free(v);
+qemu_opts_del(opts);
+qapi_free_UserDefOptions(userdef);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -343,6 +361,8 @@ main(int argc, char **argv)
 g_test_add_func("/visitor/opts/range/beyond",
 test_opts_range_beyond);
 
+g_test_add_func("/visitor/opts/dict/unvisited", test_opts_dict_unvisited);
+
 g_test_run();
 return 0;
 }
-- 
2.7.4

Re: [Qemu-devel] [PULL v2 for-2.9 17/17] qapi: Fix QemuOpts visitor regression on unvisited input

[Eric Blake]

On 03/22/2017 12:48 PM, Markus Armbruster wrote:
> From: Eric Blake 
> 
> An off-by-one in commit 15c2f669e meant that we were failing to
> check for unparsed input in all QemuOpts visitors.  Recent testsuite
> additions show that fixing the obvious bug with bogus fields will
> also fix the case of an incomplete list visit; update the tests to
> match the new behavior.
> 
> Simple testcase:
> 
> ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -numa 
> node,size=1g
> 
> failed to diagnose that 'size' is not a valid argument to -numa, and
> now once again reports:
> 
> qemu-system-x86_64: -numa node,size=1g: Invalid parameter 'size'
> 
> See also https://bugzilla.redhat.com/show_bug.cgi?id=1434666
> 
> CC: qemu-sta...@nongnu.org
> Signed-off-by: Eric Blake 
> Reviewed-by: Michael Roth 
> Tested-by: Laurent Vivier 
> Message-Id: <20170322144525.18964-4-ebl...@redhat.com>
> Reviewed-by: Markus Armbruster 
> [Fixup squashed in]
> Signed-off-by: Markus Armbruster 

Fixup squashed into the wrong patch. End result is the same, though, so
not sure if it is worth a v3 pull request.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL v2 for-2.9 16/17] qom: Avoid unvisited 'id'/'qom-type' in user_creatable_add_opts

[Markus Armbruster]

From: Eric Blake 

A regression in commit 15c2f669e caused us to silently ignore
excess input to the QemuOpts visitor.  Later, commit ea4641
accidentally abused that situation, by removing "qom-type" and
"id" from the corresponding QDict but leaving them defined in
the QemuOpts, when using the pair of containers to create a
user-defined object. Note that since we are already traversing
two separate items (a QDict and a QemuOpts), we are already
able to flag bogus arguments, as in:

$ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio -object 
memory-backend-ram,id=mem1,size=4k,bogus=huh
qemu-system-x86_64: -object memory-backend-ram,id=mem1,size=4k,bogus=huh: 
Property '.bogus' not found

So the only real concern is that when we re-enable strict checking
in the QemuOpts visitor, we do not want to start flagging the two
leftover keys as unvisited.  Rearrange the code to clean out the
QemuOpts listing in advance, rather than removing items from the
QDict.  Since "qom-type" is usually an automatic implicit default,
we don't have to restore it (this does mean that once instantiated,
QemuOpts is not necessarily an accurate representation of the
original command line - but this is not the first place to do that);
however "id" has to be put back (requiring us to cast away a const).

[As a side note, hmp_object_add() turns a QDict into a QemuOpts,
then calls user_creatable_add_opts() which converts QemuOpts into
a new QDict. There are probably a lot of wasteful conversions like
this, but cleaning them up is a much bigger task than the immediate
regression fix.]

CC: qemu-sta...@nongnu.org
Signed-off-by: Eric Blake 
Message-Id: <20170322144525.18964-3-ebl...@redhat.com>
Tested-by: Laurent Vivier 
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 qom/object_interfaces.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 03a95c3..cc9a694 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -114,7 +114,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error 
**errp)
 QDict *pdict;
 Object *obj;
 const char *id = qemu_opts_id(opts);
-const char *type = qemu_opt_get(opts, "qom-type");
+char *type = qemu_opt_get_del(opts, "qom-type");
 
 if (!type) {
 error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
@@ -125,14 +125,15 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error 
**errp)
 return NULL;
 }
 
+qemu_opts_set_id(opts, NULL);
 pdict = qemu_opts_to_qdict(opts, NULL);
-qdict_del(pdict, "qom-type");
-qdict_del(pdict, "id");
 
 v = opts_visitor_new(opts);
 obj = user_creatable_add_type(type, id, pdict, v, errp);
 visit_free(v);
 
+qemu_opts_set_id(opts, (char *) id);
+g_free(type);
 QDECREF(pdict);
 return obj;
 }
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 13/17] Revert "hostmem: fix QEMU crash by 'info memdev'"

[Markus Armbruster]

This reverts commit 1454d33f0507cb54d62ed80f494884157c9e7130.

The string input visitor regression fixed in the previous commit made
visit_type_uint16List() fail on empty input.  query_memdev() calls it
via object_property_get_uint16List().  Because it doesn't expect it to
fail, it passes _abort, and duly crashes.

Commit 1454d33 "fixes" this crash by making
host_memory_backend_get_host_nodes() return a list containing just
MAX_NODES instead of the empty list.  Papers over the regression, and
leads to bogus "info memdev" output, as shown below; revert.

I suspect that if we had bisected the crash back then, we would have
found and fixed the actual bug instead of papering over it.

To reproduce, run HMP command "info memdev" with

$ qemu-system-x86_64 --nodefaults -S -display none -monitor stdio -object 
memory-backend-ram,id=mem1,size=4k

With this commit, "info memdev" prints

memory backend: mem1
  size:  4096
  merge: true
  dump: true
  prealloc: false
  policy: default
  host nodes:

exactly like before commit 74f24cb.

Between commit 1454d33 and this commit, it prints

memory backend: mem1
  size:  4096
  merge: true
  dump: true
  prealloc: false
  policy: default
  host nodes: 128

The last line is bogus.

Between commit 74f24cb and 1454d33, it crashes like this:

Unexpected error in parse_str() at 
/work/armbru/tmp/qemu/qapi/string-input-visitor.c:126:
Parameter 'null' expects an int64 value or range
Aborted (core dumped)

Cc: Xiao Guangrong 
Cc: Paolo Bonzini 
Signed-off-by: Markus Armbruster 
Message-Id: <1490026424-11330-3-git-send-email-arm...@redhat.com>
Reviewed-by: Michael Roth 
Reviewed-by: Eric Blake 
---
 backends/hostmem.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 162c218..89feb9e 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -64,14 +64,6 @@ out:
 error_propagate(errp, local_err);
 }
 
-static uint16List **host_memory_append_node(uint16List **node,
-unsigned long value)
-{
- *node = g_malloc0(sizeof(**node));
- (*node)->value = value;
- return &(*node)->next;
-}
-
 static void
 host_memory_backend_get_host_nodes(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
@@ -82,23 +74,25 @@ host_memory_backend_get_host_nodes(Object *obj, Visitor *v, 
const char *name,
 unsigned long value;
 
 value = find_first_bit(backend->host_nodes, MAX_NODES);
-
-node = host_memory_append_node(node, value);
-
 if (value == MAX_NODES) {
-goto out;
+return;
 }
 
+*node = g_malloc0(sizeof(**node));
+(*node)->value = value;
+node = &(*node)->next;
+
 do {
 value = find_next_bit(backend->host_nodes, MAX_NODES, value + 1);
 if (value == MAX_NODES) {
 break;
 }
 
-node = host_memory_append_node(node, value);
+*node = g_malloc0(sizeof(**node));
+(*node)->value = value;
+node = &(*node)->next;
 } while (true);
 
-out:
 visit_type_uint16List(v, name, _nodes, errp);
 }
 
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 05/17] MAINTAINERS: Add myself for files I touched recently

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Message-Id: <1490014548-15083-6-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 MAINTAINERS | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 779c429..c60235e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1231,6 +1231,15 @@ M: Samuel Thibault 
 S: Maintained
 F: backends/baum.c
 
+Command line option argument parsing
+M: Markus Armbruster 
+S: Supported
+F: include/qemu/option.h
+F: tests/test-keyval.c
+F: tests/test-qemu-opts.c
+F: util/keyval.c
+F: util/qemu-option.c
+
 Coverity model
 M: Markus Armbruster 
 S: Supported
@@ -1365,7 +1374,9 @@ X: include/qapi/qmp/
 F: include/qapi/qmp/dispatch.h
 F: tests/qapi-schema/
 F: tests/test-*-visitor.c
+F: tests/test-qapi-*.c
 F: tests/test-qmp-*.c
+F: tests/test-visitor-serialization.c
 F: scripts/qapi*
 F: docs/qapi*
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
-- 
2.7.4

Re: [Qemu-devel] [Qemu-ppc] [Bug 1674925] Re: Qemu PPC64 kvm no display if --device virtio-gpu-pci is selected

[luigiburdo]

Hi Cédric,

i have the 4.11 rc1 . on fedora 25 ppc 64 on both machine Qoriq and on
G5 Quad.

On the 2.8 this issue isnt present but
I did the test o Qoriq  e5500 a book3e processor and on 2.8 if i made:

qemu-system-ppc64 --enable-kvm the true result is:
qemu-system-ppc64: Unable to find CPU definition: host

on qemu 2.9 rc1

./qemu-system-ppc64 --enable-kvm i have :
qemu-system-ppc64: KVM and IRQ_XICS capability must be present for in-kernel 
XICS

On Qoriq if i pass the -cpu e500 (normal thing) all is working right
qemu 2.9rc1 all boot and so and so.

On G5 the -cpu variable dont fix the issue.

I can build a new kernel release , usually mine dont have xics enabled
because G5 dont have that feature, if needed i can enable it for
testing.


Hope my english is understandable.

ciao

Luigi


Da: Qemu-ppc  per conto 
di Cédric Le Goater 
Inviato: mercoledì 22 marzo 2017 18.29
A: Thomas Huth; Bug 1674925; qemu-devel@nongnu.org
Cc: qemu-...@nongnu.org
Oggetto: Re: [Qemu-ppc] [Qemu-devel] [Bug 1674925] Re: Qemu PPC64 kvm no 
display if --device virtio-gpu-pci is selected

On 03/22/2017 03:15 PM, Thomas Huth wrote:
> On 22.03.2017 14:35, luigiburdo wrote:
>> Hi Thomas with 2.9 rc1 i have this with --enable-kvm
>>
>> emu-system-ppc64 --enable-kvm
>> qemu-system-ppc64: KVM and IRQ_XICS capability must be present for in-kernel 
>> XICS
>>
>> and the qemu dont run.
>
> Does it exit, or just hang afterwards? Was this with or without --device
> virtio-gpu-pci option? Do you get any output if you run QEMU with
> "-nographic" instead?

I guess this is an issue with the host kernel. Which one are you running
?

C.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1674925

Title:
  Qemu PPC64 kvm no display if  --device virtio-gpu-pci is selected

Status in QEMU:
  New

Bug description:
  Hi,
  i did many tests on qemu 2.8 on my BE machines and i found an issue that i 
think was need to be reported

  Test Machines BE 970MP

  if i setup qemu with

  qemu-system-ppc64 -M 1024 --display sdl(or gtk),gl=on --device virtio-
  gpu-pci,virgl --enable-kvm and so and so

  result is doubled window one is vga other is virtio-gpu-pci without
  any start of the VM . pratically i dont have any output of openbios
  and on the virtual serial output

  the same issue i found is if i select:
  qemu-system-ppc64 -M 1024 --display gtk(or sdl) --device virtio-gpu-pci 
--enable-kvm and so and so

  
  i had been try to change all the -M types of all kind of pseries without any 
positive result.

  Ciao 
  Luigi

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1674925/+subscriptions

[Qemu-devel] [PULL v2 for-2.9 02/17] keyval: Improve some comments

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Message-Id: <1490014548-15083-3-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 util/keyval.c | 47 +++
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/util/keyval.c b/util/keyval.c
index f646b36..46cd540 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -21,22 +21,36 @@
  *
  * Semantics defined by reduction to JSON:
  *
- *   key-vals is a tree of objects and arrays rooted at object R
- *   where for each key-val = key-fragment . ... = val in key-vals
- *   R op key-fragment op ... = val'
- *   where (left-associative) op is
- * array subscript L[key-fragment] for numeric key-fragment
- * member reference L.key-fragment otherwise
- * val' is val with ',,' replaced by ','
- *   and only R may be empty.
+ *   key-vals specifies a JSON object, i.e. a tree whose root is an
+ *   object, inner nodes other than the root are objects or arrays,
+ *   and leaves are strings.
  *
- *   Duplicate keys are permitted; all but the last one are ignored.
+ *   Each key-val = key-fragment '.' ... '=' val specifies a path from
+ *   root to a leaf (left of '='), and the leaf's value (right of
+ *   '=').
  *
- *   The equations must have a solution.  Counter-example: a.b=1,a=2
- *   doesn't have one, because R.a must be an object to satisfy a.b=1
- *   and a string to satisfy a=2.
+ *   A path from the root is defined recursively:
+ *   L '.' key-fragment is a child of the node denoted by path L
+ *   key-fragment is a child of the tree root
+ *   If key-fragment is numeric, the parent is an array and the child
+ *   is its key-fragment-th member, counting from zero.
+ *   Else, the parent is an object, and the child is its member named
+ *   key-fragment.
  *
- * Key-fragments must be valid QAPI names or consist only of digits.
+ *   This constrains inner nodes to be either array or object.  The
+ *   constraints must be satisfiable.  Counter-example: a.b=1,a=2 is
+ *   not, because root.a must be an object to satisfy a.b=1 and a
+ *   string to satisfy a=2.
+ *
+ *   Array subscripts can occur in any order, but the set of
+ *   subscripts must not have gaps.  For instance, a.1=v is not okay,
+ *   because root.a[0] is missing.
+ *
+ *   If multiple key-val denote the same leaf, the last one determines
+ *   the value.
+ *
+ * Key-fragments must be valid QAPI names or consist only of decimal
+ * digits.
  *
  * The length of any key-fragment must be between 1 and 127.
  *
@@ -64,8 +78,8 @@
 
 /*
  * Convert @key to a list index.
- * Convert all leading digits to a (non-negative) number, capped at
- * INT_MAX.
+ * Convert all leading decimal digits to a (non-negative) number,
+ * capped at INT_MAX.
  * If @end is non-null, assign a pointer to the first character after
  * the number to *@end.
  * Else, fail if any characters follow.
@@ -337,7 +351,8 @@ static QObject *keyval_listify(QDict *cur, GSList 
*key_of_cur, Error **errp)
 }
 
 /*
- * Make a list from @elt[], reporting any missing elements.
+ * Make a list from @elt[], reporting the first missing element,
+ * if any.
  * If we dropped an index >= nelt in the previous loop, this loop
  * will run into the sentinel and report index @nelt missing.
  */
-- 
2.7.4

[Qemu-devel] [PATCH] clear pending status before calling memory commit

[Xu, Anthony]

clear pending status before calling memory commit.
Otherwise when memory_region_finalize is called, 
memory_region_transaction_depth is 0 and 
memory_region_update_pending is true. 
That's wrong.


Signed-off -by: Anthony Xu 



diff --git a/memory.c b/memory.c
index 64b0a60..4c95aaf 100644
--- a/memory.c
+++ b/memory.c
@@ -906,12 +906,6 @@ void memory_region_transaction_begin(void)
 ++memory_region_transaction_depth;
 }

-static void memory_region_clear_pending(void)
-{
-memory_region_update_pending = false;
-ioeventfd_update_pending = false;
-}
-
 void memory_region_transaction_commit(void)
 {
 AddressSpace *as;
@@ -927,14 +921,14 @@ void memory_region_transaction_commit(void)
 QTAILQ_FOREACH(as, _spaces, address_spaces_link) {
 address_space_update_topology(as);
 }
-
+memory_region_update_pending = false;
 MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
 } else if (ioeventfd_update_pending) {
 QTAILQ_FOREACH(as, _spaces, address_spaces_link) {
 address_space_update_ioeventfds(as);
 }
+ioeventfd_update_pending = false;
 }
-memory_region_clear_pending();
}
 }

[Qemu-devel] [PULL v2 for-2.9 04/17] keyval: Document issues with 'any' and alternate types

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Message-Id: <1490014548-15083-5-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 util/keyval.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/util/keyval.c b/util/keyval.c
index 46cd540..93d5db6 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -61,6 +61,16 @@
  * "key absent" already means "optional object/array absent", which
  * isn't the same as "empty object/array present".
  *
+ * Design flaw: scalar values can only be strings; there is no way to
+ * denote numbers, true, false or null.  The special QObject input
+ * visitor returned by qobject_input_visitor_new_keyval() mostly hides
+ * this by automatically converting strings to the type the visitor
+ * expects.  Breaks down for alternate types and type 'any', where the
+ * visitor's expectation isn't clear.  Code visiting such types needs
+ * to do the conversion itself, but only when using this keyval
+ * visitor.  Awkward.  Alternate types without a string member don't
+ * work at all.
+ *
  * Additional syntax for use with an implied key:
  *
  *   key-vals-ik  = val-no-key [ ',' key-vals ]
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 01/17] test-keyval: Tweaks to improve list coverage

[Markus Armbruster]

We have a negative test case for a list index with leading zero.  Add
positive ones.

Tweak the test case for list index greater or equal the number of
elements: test "equal" instead of "greater" to guard against
off-by-one mistakes.

Signed-off-by: Markus Armbruster 
Message-Id: <1490014548-15083-2-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 tests/test-keyval.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index 71288b0..e97f6d5 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -218,14 +218,14 @@ static void test_keyval_parse_list(void)
 QDECREF(qdict);
 
 /* Multiple indexes, last one wins */
-qdict = keyval_parse("list.1=goner,list.0=null,list.1=eins,list.2=zwei",
+qdict = keyval_parse("list.1=goner,list.0=null,list.01=eins,list.2=zwei",
  NULL, _abort);
 g_assert_cmpint(qdict_size(qdict), ==, 1);
 check_list012(qdict_get_qlist(qdict, "list"));
 QDECREF(qdict);
 
 /* List at deeper nesting */
-qdict = keyval_parse("a.list.1=eins,a.list.0=null,a.list.2=zwei",
+qdict = keyval_parse("a.list.1=eins,a.list.00=null,a.list.2=zwei",
  NULL, _abort);
 g_assert_cmpint(qdict_size(qdict), ==, 1);
 sub_qdict = qdict_get_qdict(qdict, "a");
@@ -242,7 +242,7 @@ static void test_keyval_parse_list(void)
 g_assert(!qdict);
 
 /* Missing list indexes */
-qdict = keyval_parse("list.2=lonely", NULL, );
+qdict = keyval_parse("list.1=lonely", NULL, );
 error_free_or_abort();
 g_assert(!qdict);
 qdict = keyval_parse("list.0=null,list.2=eins,list.02=zwei", NULL, );
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 03/17] test-keyval: Cover alternate and 'any' type

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Message-Id: <1490014548-15083-4-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 tests/Makefile.include |  2 +-
 tests/test-keyval.c| 53 ++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 402e71c..86f9490 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -736,7 +736,7 @@ tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o 
$(test-util-obj-y) \
$(chardev-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): 
tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o $(test-util-obj-y)
-tests/test-keyval$(EXESUF): tests/test-keyval.o $(test-util-obj-y)
+tests/test-keyval$(EXESUF): tests/test-keyval.o $(test-util-obj-y) 
$(test-qapi-obj-y)
 tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o 
$(test-block-obj-y)
 tests/test-netfilter$(EXESUF): tests/test-netfilter.o $(qtest-obj-y)
 tests/test-filter-mirror$(EXESUF): tests/test-filter-mirror.o $(qtest-obj-y)
diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index e97f6d5..ba19560 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -14,6 +14,7 @@
 #include "qapi/error.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi/qobject-input-visitor.h"
+#include "test-qapi-visit.h"
 #include "qemu/cutils.h"
 #include "qemu/option.h"
 
@@ -608,6 +609,56 @@ static void test_keyval_visit_optional(void)
 visit_free(v);
 }
 
+static void test_keyval_visit_alternate(void)
+{
+Error *err = NULL;
+Visitor *v;
+QDict *qdict;
+AltNumStr *ans;
+AltNumInt *ani;
+
+/*
+ * Can't do scalar alternate variants other than string.  You get
+ * the string variant if there is one, else an error.
+ */
+qdict = keyval_parse("a=1,b=2", NULL, _abort);
+v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+QDECREF(qdict);
+visit_start_struct(v, NULL, NULL, 0, _abort);
+visit_type_AltNumStr(v, "a", , _abort);
+g_assert_cmpint(ans->type, ==, QTYPE_QSTRING);
+g_assert_cmpstr(ans->u.s, ==, "1");
+visit_type_AltNumInt(v, "a", , );
+error_free_or_abort();
+visit_end_struct(v, NULL);
+visit_free(v);
+}
+
+static void test_keyval_visit_any(void)
+{
+Visitor *v;
+QDict *qdict;
+QObject *any;
+QList *qlist;
+QString *qstr;
+
+qdict = keyval_parse("a.0=null,a.1=1", NULL, _abort);
+v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+QDECREF(qdict);
+visit_start_struct(v, NULL, NULL, 0, _abort);
+visit_type_any(v, "a", , _abort);
+qlist = qobject_to_qlist(any);
+g_assert(qlist);
+qstr = qobject_to_qstring(qlist_pop(qlist));
+g_assert_cmpstr(qstring_get_str(qstr), ==, "null");
+qstr = qobject_to_qstring(qlist_pop(qlist));
+g_assert_cmpstr(qstring_get_str(qstr), ==, "1");
+g_assert(qlist_empty(qlist));
+visit_check_struct(v, _abort);
+visit_end_struct(v, NULL);
+visit_free(v);
+}
+
 int main(int argc, char *argv[])
 {
 g_test_init(, , NULL);
@@ -619,6 +670,8 @@ int main(int argc, char *argv[])
 g_test_add_func("/keyval/visit/dict", test_keyval_visit_dict);
 g_test_add_func("/keyval/visit/list", test_keyval_visit_list);
 g_test_add_func("/keyval/visit/optional", test_keyval_visit_optional);
+g_test_add_func("/keyval/visit/alternate", test_keyval_visit_alternate);
+g_test_add_func("/keyval/visit/any", test_keyval_visit_any);
 g_test_run();
 return 0;
 }
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 12/17] qapi: Fix string input visitor regression for empty lists

[Markus Armbruster]

Visiting a list when input is the empty string should result in an
empty list, not an error.  Noticed when commit 3d089ce belatedly added
tests, but simply accepted as weird then.  It's actually a regression:
broken in commit 74f24cb, v2.7.0.  Fix it, and throw in another test
case for empty string.

Signed-off-by: Markus Armbruster 
Message-Id: <1490026424-11330-2-git-send-email-arm...@redhat.com>
Reviewed-by: Michael Roth 
Reviewed-by: Eric Blake 
---
 qapi/string-input-visitor.c   |  4 
 tests/test-string-input-visitor.c | 11 ---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 806b01ae..c089491 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -54,6 +54,10 @@ static int parse_str(StringInputVisitor *siv, const char 
*name, Error **errp)
 return 0;
 }
 
+if (!*str) {
+return 0;
+}
+
 do {
 errno = 0;
 start = strtoll(str, , 0);
diff --git a/tests/test-string-input-visitor.c 
b/tests/test-string-input-visitor.c
index 6db850b..79313a7 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -63,6 +63,11 @@ static void test_visitor_in_int(TestInputVisitorData *data,
 
 visit_type_int(v, NULL, , );
 error_free_or_abort();
+
+v = visitor_input_test_init(data, "");
+
+visit_type_int(v, NULL, , );
+error_free_or_abort();
 }
 
 static void check_ilist(Visitor *v, int64_t *expected, size_t n)
@@ -140,11 +145,11 @@ static void test_visitor_in_intList(TestInputVisitorData 
*data,
 v = visitor_input_test_init(data, "18446744073709551615");
 check_ulist(v, expect4, ARRAY_SIZE(expect4));
 
-/* Empty list is invalid (weird) */
+/* Empty list */
 
 v = visitor_input_test_init(data, "");
-visit_type_int64List(v, NULL, , );
-error_free_or_abort();
+visit_type_int64List(v, NULL, , _abort);
+g_assert(!res);
 
 /* Not a list */
 
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 08/17] qapi: Drop unused QAPIDoc member optional

[Markus Armbruster]

Unused since commit aa964b7 "qapi2texi: Convert to QAPISchemaVisitor"

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-4-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 scripts/qapi.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index e88c047..6c4d554 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -106,7 +106,6 @@ class QAPIDoc(object):
 self.name = name
 # the list of lines for this section
 self.content = []
-self.optional = False
 
 def append(self, line):
 self.content.append(line)
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 06/17] qapi: Drop excessive Make dependencies on qapi2texi.py

[Markus Armbruster]

When qapi2texi.py changes, we regenerate everything QAPI.  Screwed up
in commit 56e8bdd.

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-2-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 Makefile | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index f4f90df..6c359b2 100644
--- a/Makefile
+++ b/Makefile
@@ -392,7 +392,6 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
 gen-out-type = $(subst .,-,$(suffix $@))
 
 qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
-qapi-py += $(SRC_PATH)/scripts/qapi2texi.py
 
 qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
@@ -701,10 +700,12 @@ qemu-monitor-info.texi: $(SRC_PATH)/hmp-commands-info.hx 
$(SRC_PATH)/scripts/hxt
 qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > 
$@,"GEN","$@")
 
-docs/qemu-qmp-qapi.texi: $(qapi-modules) $(qapi-py)
+docs/qemu-qmp-qapi.texi docs/qemu-ga-qapi.texi: 
$(SRC_PATH)/scripts/qapi2texi.py $(qapi-py)
+
+docs/qemu-qmp-qapi.texi: $(qapi-modules)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > 
$@,"GEN","$@")
 
-docs/qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py)
+docs/qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > 
$@,"GEN","$@")
 
 qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi 
qemu-monitor-info.texi
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 00/17] QAPI patches for 2017-03-22

[Markus Armbruster]

v2:
* Leak fix squashed into 16/17

The following changes since commit 940a8ce075e3408742a4edcabfd6c2a15e2539eb:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2017-03-20 16:34:26 +)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2017-03-22-v2

for you to fetch changes up to 02bbae70011a95676408731e6710eba93d2e3ebf:

  qapi: Fix QemuOpts visitor regression on unvisited input (2017-03-22 18:39:44 
+0100)


QAPI patches for 2017-03-22


Eric Blake (3):
  tests: Expose regression in QemuOpts visitor
  qom: Avoid unvisited 'id'/'qom-type' in user_creatable_add_opts
  qapi: Fix QemuOpts visitor regression on unvisited input

Markus Armbruster (14):
  test-keyval: Tweaks to improve list coverage
  keyval: Improve some comments
  test-keyval: Cover alternate and 'any' type
  keyval: Document issues with 'any' and alternate types
  MAINTAINERS: Add myself for files I touched recently
  qapi: Drop excessive Make dependencies on qapi2texi.py
  qapi2texi: Fix to actually fail when 'doc-required' is false
  qapi: Drop unused QAPIDoc member optional
  tests/qapi-schema: Make test-qapi.py print docs again
  tests/qapi-schema: Systematic positive doc comment tests
  qapi2texi: Fix translation of *strong* and _emphasized_
  qapi: Fix string input visitor regression for empty lists
  Revert "hostmem: fix QEMU crash by 'info memdev'"
  test-qobject-input-visitor: Cover visit_type_uint64()

 MAINTAINERS|  11 ++
 Makefile   |   7 +-
 backends/hostmem.c |  22 ++--
 qapi/opts-visitor.c|   6 +-
 qapi/string-input-visitor.c|   4 +
 qom/object_interfaces.c|   8 +-
 scripts/qapi.py|   1 -
 scripts/qapi2texi.py   |   5 +-
 tests/Makefile.include |  15 ++-
 tests/qapi-schema/doc-good.err |   0
 tests/qapi-schema/doc-good.exit|   1 +
 tests/qapi-schema/doc-good.json| 136 +
 tests/qapi-schema/doc-good.out | 148 ++
 tests/qapi-schema/doc-good.texi| 243 +
 tests/qapi-schema/test-qapi.py |  11 ++
 tests/test-keyval.c|  59 -
 tests/test-opts-visitor.c  |  27 -
 tests/test-qobject-input-visitor.c |  30 +
 tests/test-string-input-visitor.c  |  11 +-
 util/keyval.c  |  57 ++---
 20 files changed, 747 insertions(+), 55 deletions(-)
 create mode 100644 tests/qapi-schema/doc-good.err
 create mode 100644 tests/qapi-schema/doc-good.exit
 create mode 100644 tests/qapi-schema/doc-good.json
 create mode 100644 tests/qapi-schema/doc-good.out
 create mode 100644 tests/qapi-schema/doc-good.texi

-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 14/17] test-qobject-input-visitor: Cover visit_type_uint64()

[Markus Armbruster]

The new test demonstrates known bugs: integers between INT64_MAX+1 and
UINT64_MAX rejected, and integers between INT64_MIN and -1 are
accepted modulo 2^64.

Signed-off-by: Markus Armbruster 
Message-Id: <1490118290-6133-1-git-send-email-arm...@redhat.com>
Reviewed-by: Eric Blake 
---
 tests/test-qobject-input-visitor.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/tests/test-qobject-input-visitor.c 
b/tests/test-qobject-input-visitor.c
index 6eb48fe..f965743 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -116,6 +116,34 @@ static void test_visitor_in_int(TestInputVisitorData *data,
 g_assert_cmpint(res, ==, value);
 }
 
+static void test_visitor_in_uint(TestInputVisitorData *data,
+const void *unused)
+{
+Error *err = NULL;
+uint64_t res = 0;
+int value = 42;
+Visitor *v;
+
+v = visitor_input_test_init(data, "%d", value);
+
+visit_type_uint64(v, NULL, , _abort);
+g_assert_cmpuint(res, ==, (uint64_t)value);
+
+/* BUG: value between INT64_MIN and -1 accepted modulo 2^64 */
+
+v = visitor_input_test_init(data, "%d", -value);
+
+visit_type_uint64(v, NULL, , _abort);
+g_assert_cmpuint(res, ==, (uint64_t)-value);
+
+/* BUG: value between INT64_MAX+1 and UINT64_MAX rejected */
+
+v = visitor_input_test_init(data, "18446744073709551574");
+
+visit_type_uint64(v, NULL, , );
+error_free_or_abort();
+}
+
 static void test_visitor_in_int_overflow(TestInputVisitorData *data,
  const void *unused)
 {
@@ -1225,6 +1253,8 @@ int main(int argc, char **argv)
 
 input_visitor_test_add("/visitor/input/int",
NULL, test_visitor_in_int);
+input_visitor_test_add("/visitor/input/uint",
+   NULL, test_visitor_in_uint);
 input_visitor_test_add("/visitor/input/int_overflow",
NULL, test_visitor_in_int_overflow);
 input_visitor_test_add("/visitor/input/int_keyval",
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 09/17] tests/qapi-schema: Make test-qapi.py print docs again

[Markus Armbruster]

test-qapi.py used to print the internal representation of doc comments
(commit 3313b61).  This went away when we dropped the doc comments in
positive tests (commit 87c16dc).  Bring it back, because I'm going to
add real positive doc comment tests.

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-5-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 tests/qapi-schema/test-qapi.py | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index ef74e2c..c7724d3 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -55,3 +55,14 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
 
 schema = QAPISchema(sys.argv[1])
 schema.visit(QAPISchemaTestVisitor())
+
+for doc in schema.docs:
+if doc.symbol:
+print 'doc symbol=%s' % doc.symbol
+else:
+print 'doc freeform'
+print 'body=\n%s' % doc.body
+for arg, section in doc.args.iteritems():
+print 'arg=%s\n%s' % (arg, section)
+for section in doc.sections:
+print 'section=%s\n%s' % (section.name, section)
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 11/17] qapi2texi: Fix translation of *strong* and _emphasized_

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-7-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 scripts/qapi2texi.py| 4 ++--
 tests/qapi-schema/doc-good.texi | 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 5c4db78..9e01500 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -35,12 +35,12 @@ EXAMPLE_FMT = """@example
 
 def subst_strong(doc):
 """Replaces *foo* by @strong{foo}"""
-return re.sub(r'\*([^*\n]+)\*', r'@emph{\1}', doc)
+return re.sub(r'\*([^*\n]+)\*', r'@strong{\1}', doc)
 
 
 def subst_emph(doc):
 """Replaces _foo_ by @emph{foo}"""
-return re.sub(r'\b_([^_\n]+)_\b', r' @emph{\1} ', doc)
+return re.sub(r'\b_([^_\n]+)_\b', r'@emph{\1}', doc)
 
 
 def subst_vars(doc):
diff --git a/tests/qapi-schema/doc-good.texi b/tests/qapi-schema/doc-good.texi
index 1160aaf..c410626 100644
--- a/tests/qapi-schema/doc-good.texi
+++ b/tests/qapi-schema/doc-good.texi
@@ -2,7 +2,7 @@
 
 @subsection Subsection
 
-@emph{strong}  @emph{with emphasis} 
+@strong{strong} @emph{with emphasis}
 @code{var} @{in braces@}
 @itemize @bullet
 @item
@@ -67,7 +67,7 @@ Example:
 Examples:
 @itemize @minus
 @item
-@emph{verbatim}
+@strong{verbatim}
 @item
 @{braces@}
 @end itemize
@@ -76,12 +76,12 @@ Examples:
 
 @deftp {Enum} Enum
 
-@subsection Produces @emph{invalid} texinfo
+@subsection Produces @strong{invalid} texinfo
 
 @b{Values:}
 @table @asis
 @item @code{one}
-The  @emph{one}  @{and only@}
+The @emph{one} @{and only@}
 @item @code{two}
 Not documented
 @end table
-- 
2.7.4

[Qemu-devel] [PULL v2 for-2.9 07/17] qapi2texi: Fix to actually fail when 'doc-required' is false

[Markus Armbruster]

Messed up in commit bc52d03.

Signed-off-by: Markus Armbruster 
Message-Id: <1490015515-25851-3-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau 
---
 scripts/qapi2texi.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 8eed11a..5c4db78 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -292,6 +292,7 @@ def main(argv):
 if not qapi.doc_required:
 print >>sys.stderr, ("%s: need pragma 'doc-required' "
  "to generate documentation" % argv[0])
+sys.exit(1)
 print texi_schema(schema)
 
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH] fixup! qom: Avoid unvisited 'id'/'qom-type' in user_creatable_add_opts

[Eric Blake]

On 03/22/2017 12:35 PM, no-re...@patchew.org wrote:
> Hi,
> 
> This series failed build test on s390x host. Please find the details below.
> 
> Type: series
> Subject: [Qemu-devel] [PATCH] fixup! qom: Avoid unvisited 'id'/'qom-type' in 
> user_creatable_add_opts
> Message-id: 20170322173023.22654-1-ebl...@redhat.com
> 

> /var/tmp/patchew-tester-tmp-5drz5kus/src/qom/object_interfaces.c: In function 
> ‘user_creatable_add_opts’:
> /var/tmp/patchew-tester-tmp-5drz5kus/src/qom/object_interfaces.c:125:16: 
> error: passing argument 1 of ‘g_free’ discards ‘const’ qualifier from pointer 
> target type [-Werror=discarded-qualifiers]
>  g_free(type);
> ^~~~

Silly patchew. Maybe we should teach you that any patch with 'fixup!' in
the title is _probably_ not going to compile on its own, because it is a
fixup to be sqaushed to an (as-yet) uncommitted patch.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Assertion failure taking external snapshot with virtio drive + iothread

[Ed Swierk]

On Wed, Mar 22, 2017 at 2:19 AM, Fam Zheng  wrote:
> On Tue, 03/21 06:05, Ed Swierk wrote:
>> Actually running snapshot_blkdev command in the text monitor doesn't
>> trigger this assertion (I mixed up my notes). Instead it's triggered
>> by the following sequence in qmp-shell:
>>
>> (QEMU) blockdev-snapshot-sync device=drive0 format=qcow2
>> snapshot-file=/x/snap1.qcow2
>> {"return": {}}
>> (QEMU) block-commit device=drive0
>> {"return": {}}
>> (QEMU) block-job-complete device=drive0
>> {"return": {}}
>>
>> > Is there a backtrace?
>>
>> #0  0x73757067 in raise () from /lib/x86_64-linux-gnu/libc.so.6
>> #1  0x73758448 in abort () from /lib/x86_64-linux-gnu/libc.so.6
>> #2  0x73750266 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
>> #3  0x73750312 in __assert_fail () from 
>> /lib/x86_64-linux-gnu/libc.so.6
>> #4  0x55b4b0bb in bdrv_drain_recurse
>> (bs=bs@entry=0x57bd6010)  at /x/qemu/block/io.c:164
>> #5  0x55b4b7ad in bdrv_drained_begin (bs=0x57bd6010)  at
>> /x/qemu/block/io.c:231
>> #6  0x55b4b802 in bdrv_parent_drained_begin
>> (bs=0x568c1a00)  at /x/qemu/block/io.c:53
>> #7  bdrv_drained_begin (bs=bs@entry=0x568c1a00)  at 
>> /x/qemu/block/io.c:228
>> #8  0x55b4be1e in bdrv_co_drain_bh_cb (opaque=0x7fff9aaece40)
>> at /x/qemu/block/io.c:190
>> #9  0x55bb431e in aio_bh_call (bh=0x5750e5f0)  at
>> /x/qemu/util/async.c:90
>> #10 aio_bh_poll (ctx=ctx@entry=0x56718090)  at /x/qemu/util/async.c:118
>> #11 0x55bb72eb in aio_poll (ctx=0x56718090,
>> blocking=blocking@entry=true)  at /x/qemu/util/aio-posix.c:682
>> #12 0x559443ce in iothread_run (opaque=0x56717b80)  at
>> /x/qemu/iothread.c:59
>> #13 0x73ad50a4 in start_thread () from
>> /lib/x86_64-linux-gnu/libpthread.so.0
>> #14 0x7380a87d in clone () from /lib/x86_64-linux-gnu/libc.so.6
>
> Hmm, looks like a separate bug to me. In addition please apply this (the
> assertion here is correct I think, but all callers are not audited yet):
>
> diff --git a/block.c b/block.c
> index 6e906ec..447d908 100644
> --- a/block.c
> +++ b/block.c
> @@ -1737,6 +1737,9 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
>  {
>  BlockDriverState *old_bs = child->bs;
>
> +if (old_bs && new_bs) {
> +assert(bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs));
> +}
>  if (old_bs) {
>  if (old_bs->quiesce_counter && child->role->drained_end) {
>  child->role->drained_end(child);
> diff --git a/block/mirror.c b/block/mirror.c
> index ca4baa5..a23ca9e 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1147,6 +1147,7 @@ static void mirror_start_job(const char *job_id, 
> BlockDriverState *bs,
>  return;
>  }
>  mirror_top_bs->total_sectors = bs->total_sectors;
> +bdrv_set_aio_context(mirror_top_bs, bdrv_get_aio_context(bs));
>
>  /* bdrv_append takes ownership of the mirror_top_bs reference, need to 
> keep
>   * it alive until block_job_create() even if bs has no parent. */

With this patch, I'm seeing either assertions or hangs when I run
blockdev-snapshot-sync, block-commit and block-job-complete
repeatedly. The exact assertion seems to depend on timing and/or what
combination of your other patches I apply. They include:

/x/qemu/hw/virtio/virtio.c:212: vring_get_region_caches: Assertion
`caches != ((void *)0)' failed.

/x/qemu/block/mirror.c:350: mirror_iteration: Assertion `sector_num >=
0' failed.

/x/qemu/block/mirror.c:865: mirror_run: Assertion
`((>tracked_requests)->lh_first == ((void *)0))' failed.

We don't appear to be converging on a solution here. Perhaps I should
instead focus on implementing automated tests so that you or anyone
else can easily reproduce these problems. The only tricky part is
extending qemu-iotests to include booting a guest to generate block IO
and trigger race conditions, but I have some ideas about how to do
this with a minimal (< 5 MB) Linux kernel+rootfs.

--Ed

Re: [Qemu-devel] [PATCH] fixup! qom: Avoid unvisited 'id'/'qom-type' in user_creatable_add_opts

[no-reply]

Hi,

This series failed build test on s390x host. Please find the details below.

Type: series
Subject: [Qemu-devel] [PATCH] fixup! qom: Avoid unvisited 'id'/'qom-type' in 
user_creatable_add_opts
Message-id: 20170322173023.22654-1-ebl...@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
echo "=== ENV ==="
env
echo "=== PACKAGES ==="
rpm -qa
echo "=== TEST BEGIN ==="
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
echo -n "Using CC: "
realpath $CC
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/1490198748-4753-1-git-send-email-arm...@redhat.com 
-> patchew/1490198748-4753-1-git-send-email-arm...@redhat.com
 - [tag update]  patchew/1490199642-5521-1-git-send-email-arm...@redhat.com 
-> patchew/1490199642-5521-1-git-send-email-arm...@redhat.com
 * [new tag] patchew/20170322173023.22654-1-ebl...@redhat.com -> 
patchew/20170322173023.22654-1-ebl...@redhat.com
Switched to a new branch 'test'
c7ade23 fixup! qom: Avoid unvisited 'id'/'qom-type' in user_creatable_add_opts

=== OUTPUT BEGIN ===
=== ENV ===
XDG_SESSION_ID=71875
SHELL=/bin/sh
USER=fam
PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug
PATH=/usr/bin:/bin
PWD=/var/tmp/patchew-tester-tmp-5drz5kus/src
LANG=en_US.UTF-8
HOME=/home/fam
SHLVL=2
LOGNAME=fam
DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus
XDG_RUNTIME_DIR=/run/user/1012
_=/usr/bin/env
=== PACKAGES ===
gpg-pubkey-873529b8-54e386ff
xz-libs-5.2.2-2.fc24.s390x
libacl-2.2.52-11.fc24.s390x
libxshmfence-1.2-3.fc24.s390x
cdparanoia-libs-10.2-21.fc24.s390x
ustr-1.0.4-21.fc24.s390x
giflib-4.1.6-15.fc24.s390x
libusb-0.1.5-7.fc24.s390x
trousers-lib-0.3.13-6.fc24.s390x
readline-devel-6.3-8.fc24.s390x
python-srpm-macros-3-10.fc25.noarch
ncurses-base-6.0-6.20160709.fc25.noarch
gmp-6.1.1-1.fc25.s390x
chkconfig-1.8-1.fc25.s390x
libidn-1.33-1.fc25.s390x
file-5.28-4.fc25.s390x
slang-2.3.0-7.fc25.s390x
avahi-libs-0.6.32-4.fc25.s390x
libsemanage-2.5-8.fc25.s390x
perl-Unicode-Normalize-1.25-365.fc25.s390x
perl-libnet-3.10-1.fc25.noarch
perl-Thread-Queue-3.11-1.fc25.noarch
perl-podlators-4.09-1.fc25.noarch
jasper-libs-1.900.13-1.fc25.s390x
graphite2-1.3.6-1.fc25.s390x
libblkid-2.28.2-1.fc25.s390x
pkgconfig-0.29.1-1.fc25.s390x
dbus-python-1.2.4-2.fc25.s390x
alsa-lib-1.1.1-2.fc25.s390x
libgnome-keyring-3.12.0-7.fc25.s390x
yum-metadata-parser-1.1.4-17.fc25.s390x
python3-3.5.2-4.fc25.s390x
python3-slip-dbus-0.6.4-4.fc25.noarch
python2-cssselect-0.9.2-1.fc25.noarch
python-backports-1.0-8.fc25.s390x
python-magic-5.28-4.fc25.noarch
python-pycparser-2.14-7.fc25.noarch
python-fedora-0.8.0-2.fc25.noarch
createrepo_c-libs-0.10.0-6.fc25.s390x
initscripts-9.69-1.fc25.s390x
plymouth-scripts-0.9.3-0.6.20160620git0e65b86c.fc25.s390x
cronie-1.5.1-2.fc25.s390x
python2-librepo-1.7.18-3.fc25.s390x
wget-1.18-2.fc25.s390x
python3-dnf-plugins-core-0.1.21-4.fc25.noarch
at-spi2-core-2.22.0-1.fc25.s390x
libXv-1.0.11-1.fc25.s390x
dhcp-client-4.3.5-1.fc25.s390x
python2-dnf-plugins-core-0.1.21-4.fc25.noarch
parted-3.2-21.fc25.s390x
python2-ndg_httpsclient-0.4.0-4.fc25.noarch
bash-completion-2.4-1.fc25.noarch
btrfs-progs-4.6.1-1.fc25.s390x
texinfo-6.1-3.fc25.s390x
perl-Filter-1.55-366.fc25.s390x
flex-2.6.0-3.fc25.s390x
libgcc-6.3.1-1.fc25.s390x
glib2-2.50.2-1.fc25.s390x
dbus-libs-1.11.8-1.fc25.s390x
libgomp-6.3.1-1.fc25.s390x
colord-libs-1.3.4-1.fc25.s390x
perl-Encode-2.88-5.fc25.s390x
gstreamer1-1.10.2-1.fc25.s390x
cracklib-2.9.6-4.fc25.s390x
rpm-build-libs-4.13.0-6.fc25.s390x
libobjc-6.3.1-1.fc25.s390x
pcre-devel-8.40-1.fc25.s390x
mariadb-config-10.1.20-1.fc25.s390x
gcc-6.3.1-1.fc25.s390x
mesa-libGL-13.0.3-1.fc25.s390x
python3-dnf-plugin-system-upgrade-0.7.1-4.fc25.noarch
bind-libs-9.10.4-4.P5.fc25.s390x
python-osbs-client-0.33-3.fc25.noarch
NetworkManager-1.4.4-3.fc25.s390x
audit-2.7.1-1.fc25.s390x
glibc-static-2.24-4.fc25.s390x
perl-Pod-Simple-3.35-1.fc25.noarch
gdb-7.12-36.fc25.s390x
python2-simplejson-3.10.0-1.fc25.s390x
python3-sssdconfig-1.14.2-2.fc25.noarch
texlive-lib-2016-30.20160520.fc25.s390x
boost-random-1.60.0-10.fc25.s390x
brltty-5.4-2.fc25.s390x
libref_array-0.1.5-29.fc25.s390x
librados2-10.2.4-2.fc25.s390x
gnutls-dane-3.5.8-1.fc25.s390x
systemtap-client-3.1-0.20160725git91bfb36.fc25.s390x
libXrender-devel-0.9.10-1.fc25.s390x
libXi-devel-1.7.8-2.fc25.s390x
texlive-pdftex-doc-svn41149-30.fc25.noarch
tcp_wrappers-7.6-83.fc25.s390x
javapackages-tools-4.7.0-6.1.fc25.noarch
texlive-kpathsea-bin-svn40473-30.20160520.fc25.s390x
texlive-url-svn32528.3.4-30.fc25.noarch
texlive-latex-fonts-svn2.0-30.fc25.noarch
texlive-mptopdf-bin-svn18674.0-30.20160520.fc25.noarch

[Qemu-devel] [PULL for-2.9 0/4] Block patches

[Jeff Cody]

The following changes since commit 55a19ad8b2d0797e3a8fe90ab99a9bb713824059:

  Update version for v2.9.0-rc1 release (2017-03-21 17:13:29 +)

are available in the git repository at:

  https://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request

for you to fetch changes up to 600ac6a0ef5c06418446ef2f37407bddcc51b21c:

  blockjob: add devops to blockjob backends (2017-03-22 13:26:27 -0400)


Block patches for 2.9


John Snow (3):
  blockjob: add block_job_start_shim
  block-backend: add drained_begin / drained_end ops
  blockjob: add devops to blockjob backends

Paolo Bonzini (1):
  blockjob: avoid recursive AioContext locking

 block/block-backend.c  | 24 ++--
 blockjob.c | 63 --
 include/sysemu/block-backend.h |  8 ++
 3 files changed, 79 insertions(+), 16 deletions(-)

-- 
2.9.3

[Qemu-devel] [PULL for-2.9 3/4] block-backend: add drained_begin / drained_end ops

[Jeff Cody]

From: John Snow 

Allow block backends to forward drain requests to their devices/users.
The initial intended purpose for this patch is to allow BBs to forward
requests along to BlockJobs, which will want to pause if their associated
BB has entered a drained region.

Signed-off-by: John Snow 
Reviewed-by: Jeff Cody 
Message-id: 20170316212351.13797-3-js...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/block-backend.c  | 24 ++--
 include/sysemu/block-backend.h |  8 
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 5742c09..0b63773 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -65,6 +65,8 @@ struct BlockBackend {
 bool allow_write_beyond_eof;
 
 NotifierList remove_bs_notifiers, insert_bs_notifiers;
+
+int quiesce_counter;
 };
 
 typedef struct BlockBackendAIOCB {
@@ -699,12 +701,17 @@ void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps 
*ops,
  void *opaque)
 {
 /* All drivers that use blk_set_dev_ops() are qdevified and we want to keep
- * it that way, so we can assume blk->dev is a DeviceState if blk->dev_ops
- * is set. */
+ * it that way, so we can assume blk->dev, if present, is a DeviceState if
+ * blk->dev_ops is set. Non-device users may use dev_ops without device. */
 assert(!blk->legacy_dev);
 
 blk->dev_ops = ops;
 blk->dev_opaque = opaque;
+
+/* Are we currently quiesced? Should we enforce this right now? */
+if (blk->quiesce_counter && ops->drained_begin) {
+ops->drained_begin(opaque);
+}
 }
 
 /*
@@ -1870,6 +1877,12 @@ static void blk_root_drained_begin(BdrvChild *child)
 {
 BlockBackend *blk = child->opaque;
 
+if (++blk->quiesce_counter == 1) {
+if (blk->dev_ops && blk->dev_ops->drained_begin) {
+blk->dev_ops->drained_begin(blk->dev_opaque);
+}
+}
+
 /* Note that blk->root may not be accessible here yet if we are just
  * attaching to a BlockDriverState that is drained. Use child instead. */
 
@@ -1881,7 +1894,14 @@ static void blk_root_drained_begin(BdrvChild *child)
 static void blk_root_drained_end(BdrvChild *child)
 {
 BlockBackend *blk = child->opaque;
+assert(blk->quiesce_counter);
 
 assert(blk->public.io_limits_disabled);
 --blk->public.io_limits_disabled;
+
+if (--blk->quiesce_counter == 0) {
+if (blk->dev_ops && blk->dev_ops->drained_end) {
+blk->dev_ops->drained_end(blk->dev_opaque);
+}
+}
 }
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 096c17f..7462228 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -58,6 +58,14 @@ typedef struct BlockDevOps {
  * Runs when the size changed (e.g. monitor command block_resize)
  */
 void (*resize_cb)(void *opaque);
+/*
+ * Runs when the backend receives a drain request.
+ */
+void (*drained_begin)(void *opaque);
+/*
+ * Runs when the backend's last drain request ends.
+ */
+void (*drained_end)(void *opaque);
 } BlockDevOps;
 
 /* This struct is embedded in (the private) BlockBackend struct and contains
-- 
2.9.3

[Qemu-devel] [PULL for-2.9 4/4] blockjob: add devops to blockjob backends

[Jeff Cody]

From: John Snow 

This lets us hook into drained_begin and drained_end requests from the
backend level, which is particularly useful for making sure that all
jobs associated with a particular node (whether the source or the target)
receive a drain request.

Suggested-by: Kevin Wolf 
Signed-off-by: John Snow 
Reviewed-by: Jeff Cody 
Message-id: 20170316212351.13797-4-js...@redhat.com
Signed-off-by: Jeff Cody 
---
 blockjob.c | 29 -
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 0e9ed03..9b619f385 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -68,6 +68,23 @@ static const BdrvChildRole child_job = {
 .stay_at_node   = true,
 };
 
+static void block_job_drained_begin(void *opaque)
+{
+BlockJob *job = opaque;
+block_job_pause(job);
+}
+
+static void block_job_drained_end(void *opaque)
+{
+BlockJob *job = opaque;
+block_job_resume(job);
+}
+
+static const BlockDevOps block_job_dev_ops = {
+.drained_begin = block_job_drained_begin,
+.drained_end = block_job_drained_end,
+};
+
 BlockJob *block_job_next(BlockJob *job)
 {
 if (!job) {
@@ -205,11 +222,6 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 }
 
 job = g_malloc0(driver->instance_size);
-error_setg(>blocker, "block device is in use by block job: %s",
-   BlockJobType_lookup[driver->job_type]);
-block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, _abort);
-bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
-
 job->driver= driver;
 job->id= g_strdup(job_id);
 job->blk   = blk;
@@ -219,8 +231,15 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 job->paused= true;
 job->pause_count   = 1;
 job->refcnt= 1;
+
+error_setg(>blocker, "block device is in use by block job: %s",
+   BlockJobType_lookup[driver->job_type]);
+block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, _abort);
 bs->job = job;
 
+blk_set_dev_ops(blk, _job_dev_ops, job);
+bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
+
 QLIST_INSERT_HEAD(_jobs, job, job_list);
 
 blk_add_aio_context_notifier(blk, block_job_attached_aio_context,
-- 
2.9.3

[Qemu-devel] [PULL for-2.9 2/4] blockjob: add block_job_start_shim

[Jeff Cody]

From: John Snow 

The purpose of this shim is to allow us to pause pre-started jobs.
The purpose of *that* is to allow us to buffer a pause request that
will be able to take effect before the job ever does any work, allowing
us to create jobs during a quiescent state (under which they will be
automatically paused), then resuming the jobs after the critical section
in any order, either:

(1) -block_job_start
-block_job_resume (via e.g. drained_end)

(2) -block_job_resume (via e.g. drained_end)
-block_job_start

The problem that requires a startup wrapper is the idea that a job must
start in the busy=true state only its first time-- all subsequent entries
require busy to be false, and the toggling of this state is otherwise
handled during existing pause and yield points.

The wrapper simply allows us to mandate that a job can "start," set busy
to true, then immediately pause only if necessary. We could avoid
requiring a wrapper, but all jobs would need to do it, so it's been
factored out here.

Signed-off-by: John Snow 
Reviewed-by: Jeff Cody 
Message-id: 20170316212351.13797-2-js...@redhat.com
Signed-off-by: Jeff Cody 
---
 blockjob.c | 26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 2159df7..0e9ed03 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -250,16 +250,28 @@ static bool block_job_started(BlockJob *job)
 return job->co;
 }
 
+/**
+ * All jobs must allow a pause point before entering their job proper. This
+ * ensures that jobs can be paused prior to being started, then resumed later.
+ */
+static void coroutine_fn block_job_co_entry(void *opaque)
+{
+BlockJob *job = opaque;
+
+assert(job && job->driver && job->driver->start);
+block_job_pause_point(job);
+job->driver->start(job);
+}
+
 void block_job_start(BlockJob *job)
 {
 assert(job && !block_job_started(job) && job->paused &&
-   !job->busy && job->driver->start);
-job->co = qemu_coroutine_create(job->driver->start, job);
-if (--job->pause_count == 0) {
-job->paused = false;
-job->busy = true;
-qemu_coroutine_enter(job->co);
-}
+   job->driver && job->driver->start);
+job->co = qemu_coroutine_create(block_job_co_entry, job);
+job->pause_count--;
+job->busy = true;
+job->paused = false;
+qemu_coroutine_enter(job->co);
 }
 
 void block_job_ref(BlockJob *job)
-- 
2.9.3

[Qemu-devel] [PULL for-2.9 1/4] blockjob: avoid recursive AioContext locking

[Jeff Cody]

From: Paolo Bonzini 

Streaming or any other block job hangs when performed on a block device
that has a non-default iothread.  This happens because the AioContext
is acquired twice by block_job_defer_to_main_loop_bh and then released
only once by BDRV_POLL_WHILE.  (Insert rants on recursive mutexes, which
unfortunately are a temporary but necessary evil for iothreads at the
moment).

Luckily, the reason for the double acquisition is simple; the function
acquires the AioContext for both the job iothread and the BDS iothread,
in case the BDS iothread was changed while the job was running.  It
is therefore enough to skip the second acquisition when the two
AioContexts are one and the same.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Eric Blake 
Reviewed-by: Jeff Cody 
Message-id: 1490118490-5597-1-git-send-email-pbonz...@redhat.com
Signed-off-by: Jeff Cody 
---
 blockjob.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 69126af..2159df7 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -755,12 +755,16 @@ static void block_job_defer_to_main_loop_bh(void *opaque)
 
 /* Fetch BDS AioContext again, in case it has changed */
 aio_context = blk_get_aio_context(data->job->blk);
-aio_context_acquire(aio_context);
+if (aio_context != data->aio_context) {
+aio_context_acquire(aio_context);
+}
 
 data->job->deferred_to_main_loop = false;
 data->fn(data->job, data->opaque);
 
-aio_context_release(aio_context);
+if (aio_context != data->aio_context) {
+aio_context_release(aio_context);
+}
 
 aio_context_release(data->aio_context);
 
-- 
2.9.3

[Qemu-devel] [PATCH] fixup! qom: Avoid unvisited 'id'/'qom-type' in user_creatable_add_opts

[Eric Blake]

CC: qemu-sta...@nongnu.org
Signed-off-by: Eric Blake 

---

Markus already issued the pull request, so if that goes in before
he can squash this for a v2, then I'll have to turn this into a
full-blown patch.

v3.5: avoid memory leak when id is not present
v3: enhance commit message
v2: new patch
---
 qom/object_interfaces.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index cc9a694..9c271ad 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -122,6 +122,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error 
**errp)
 }
 if (!id) {
 error_setg(errp, QERR_MISSING_PARAMETER, "id");
+g_free(type);
 return NULL;
 }

-- 
2.9.3

Re: [Qemu-devel] [Bug 1674925] Re: Qemu PPC64 kvm no display if --device virtio-gpu-pci is selected

[Cédric Le Goater]

On 03/22/2017 03:15 PM, Thomas Huth wrote:
> On 22.03.2017 14:35, luigiburdo wrote:
>> Hi Thomas with 2.9 rc1 i have this with --enable-kvm
>>
>> emu-system-ppc64 --enable-kvm
>> qemu-system-ppc64: KVM and IRQ_XICS capability must be present for in-kernel 
>> XICS
>>
>> and the qemu dont run.
> 
> Does it exit, or just hang afterwards? Was this with or without --device
> virtio-gpu-pci option? Do you get any output if you run QEMU with
> "-nographic" instead?

I guess this is an issue with the host kernel. Which one are you running ? 

C.

Re: [Qemu-devel] [PULL for-2.9 16/17] qom: Avoid unvisited 'id'/'qom-type' in user_creatable_add_opts

[Eric Blake]

On 03/22/2017 11:05 AM, Markus Armbruster wrote:
> From: Eric Blake 
> 
> A regression in commit 15c2f669e caused us to silently ignore
> excess input to the QemuOpts visitor.  Later, commit ea4641
> accidentally abused that situation, by removing "qom-type" and
> "id" from the corresponding QDict but leaving them defined in
> the QemuOpts, when using the pair of containers to create a
> user-defined object. Note that since we are already traversing
> two separate items (a QDict and a QemuOpts), we are already
> able to flag bogus arguments, as in:
> 

> +++ b/qom/object_interfaces.c
> @@ -114,7 +114,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error 
> **errp)
>  QDict *pdict;
>  Object *obj;
>  const char *id = qemu_opts_id(opts);
> -const char *type = qemu_opt_get(opts, "qom-type");
> +char *type = qemu_opt_get_del(opts, "qom-type");
>  
>  if (!type) {
>  error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
> @@ -125,14 +125,15 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error 
> **errp)
>  return NULL;
>  }

Shoot - I missed the memory leak of type if id is not present. I'll post
the fixup, but depending on timing, it may have to be a separate patch
rather than a v2 pull request.

>  
> +qemu_opts_set_id(opts, NULL);
>  pdict = qemu_opts_to_qdict(opts, NULL);
> -qdict_del(pdict, "qom-type");
> -qdict_del(pdict, "id");
>  
>  v = opts_visitor_new(opts);
>  obj = user_creatable_add_type(type, id, pdict, v, errp);
>  visit_free(v);
>  
> +qemu_opts_set_id(opts, (char *) id);
> +g_free(type);
>  QDECREF(pdict);
>  return obj;
>  }
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 0/3] block: pause block jobs for bdrv_drain_begin/end

[Jeff Cody]

On Thu, Mar 16, 2017 at 02:28:47PM -0700, no-re...@patchew.org wrote:
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Subject: [Qemu-devel] [PATCH v2 0/3] block: pause block jobs for 
> bdrv_drain_begin/end
> Message-id: 20170316212351.13797-1-js...@redhat.com
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> 
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
> 
> # Useful git options
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> 
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; 
> then
> failed=1
> echo
> fi
> n=$((n+1))
> done
> 
> exit $failed
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag] patchew/20170316212351.13797-1-js...@redhat.com -> 
> patchew/20170316212351.13797-1-js...@redhat.com
> Switched to a new branch 'test'
> 1cca6f3 blockjob: add devops to blockjob backends
> 864d906 block-backend: add drained_begin / drained_end ops
> 5e4f22d blockjob: add block_job_start_shim
> 
> === OUTPUT BEGIN ===
> Checking PATCH 1/3: blockjob: add block_job_start_shim...
> Checking PATCH 2/3: block-backend: add drained_begin / drained_end ops...
> ERROR: suspect code indent for conditional statements (8, 14)
> #70: FILE: block/block-backend.c:1903:
> +if (blk->dev_ops && blk->dev_ops->drained_end) {
> +  blk->dev_ops->drained_end(blk->dev_opaque);
> 
> total: 1 errors, 0 warnings, 67 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 3/3: blockjob: add devops to blockjob backends...
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-de...@freelists.org


Fixed the patchew nit, and:

Thanks,

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc.git block

-Jeff

Re: [Qemu-devel] [PATCH v2 for-2.9] block: Declare blockdev-add and blockdev-del supported

[Max Reitz]

On 22.03.2017 17:20, Markus Armbruster wrote:
> It's been a long journey, but here we are.
> 
> The supported blockdev-add is not compatible to its experimental
> predecessors; bump all Since: tags to 2.9.
> 
> x-blockdev-remove-medium, x-blockdev-insert-medium and
> x-blockdev-change need a bit more work, so leave them alone for now.
> 
> Signed-off-by: Markus Armbruster 
> ---
> v2:
> * Another "command is considered experimental" comment dropped [Max]
> * More Since: tags updated to 2.9, commit message improved, R-bys dropped

Do you want to do this recursively? There are things such as
BlkdebugSetStateOptions or ReplicationMode which are only used by the
corresponding BlockdevOptions*, so they too are basically only supported
as of 2.9.

I don't have a strong opinion, but I think I personally wouldn't bother
with any of these structures and just update the tag for blockdev-add
itself...

You being fine with me would be enough to convince me, so:

Reviewed-by: Max Reitz 

> 
>  blockdev.c |  4 +--
>  qapi/block-core.json   | 69 
> ++
>  tests/qemu-iotests/139 |  8 +++---
>  tests/qemu-iotests/141 |  4 +--
>  tests/qemu-iotests/147 |  2 +-
>  5 files changed, 34 insertions(+), 53 deletions(-)



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] qemu-ga static compilation

[Stefan Hajnoczi]

On Tue, Mar 21, 2017 at 11:23:39AM +0200, Sameeh Jubran wrote:
> Just to make it clear,
> 
> Stefan you are going to send a patch right?

I see you've sent a patch.  Please go ahead with it.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.9?] block/file-posix.c: Fix unused variable warning on OpenBSD

[Max Reitz]

On 20.03.2017 19:33, Peter Maydell wrote:
> On OpenBSD none of the ioctls probe_logical_blocksize() tries
> exist, so the variable sector_size is unused. Refactor the
> code to avoid this (and reduce the duplicated code).
> 
> Signed-off-by: Peter Maydell 
> ---
> The alternative would be to move the variable so it was
> local to a code block inside each #ifdef, but this seemed
> a bit nicer anyway.
> 
> Tentatively tagged 'for-2.9' just because this is the only
> warning in the OpenBSD build, but I don't insist on it.

I'm fine with targeting 2.9.

> I've opted to retain the existing behaviour of "try every
> ioctl available and use the last one that works" rather
> than "stop as soon as something worked".
> 
> ---
>  block/file-posix.c | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 53febd3..b980d23 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -219,28 +219,28 @@ static int probe_logical_blocksize(int fd, unsigned int 
> *sector_size_p)
>  {
>  unsigned int sector_size;
>  bool success = false;
> +int i;
>  
>  errno = ENOTSUP;
> -
> -/* Try a few ioctls to get the right size */
> +unsigned long ioctl_list[] = {
>  #ifdef BLKSSZGET
> -if (ioctl(fd, BLKSSZGET, _size) >= 0) {
> -*sector_size_p = sector_size;
> -success = true;
> -}
> +BLKSSZGET,
>  #endif
>  #ifdef DKIOCGETBLOCKSIZE
> -if (ioctl(fd, DKIOCGETBLOCKSIZE, _size) >= 0) {
> -*sector_size_p = sector_size;
> -success = true;
> -}
> +DKIOCGETBLOCKSIZE,
>  #endif
>  #ifdef DIOCGSECTORSIZE
> -if (ioctl(fd, DIOCGSECTORSIZE, _size) >= 0) {
> -*sector_size_p = sector_size;
> -success = true;
> -}
> +DIOCGSECTORSIZE,
>  #endif
> +};
> +
> +/* Try a few ioctls to get the right size */
> +for (i = 0; i < ARRAY_SIZE(ioctl_list); i++) {

On 32-bit, though:

./block/file-posix.c: In function ‘probe_logical_blocksize’:
./qemu/block/file-posix.c:229:19: error: comparison of unsigned
expression < 0 is always false [-Werror=type-limits]

A cast (int)ARRAY_SIZE() would fix that (I had the same problem once...).

Max

> +if (ioctl(fd, ioctl_list[i], _size) >= 0) {
> +*sector_size_p = sector_size;
> +success = true;
> +}
> +}
>  
>  return success ? 0 : -errno;
>  }
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 14/16] block/qcow2: falloc/full preallocating growth

[Stefan Hajnoczi]

On Mon, Mar 20, 2017 at 04:15:59PM +0100, Max Reitz wrote:
> On 20.03.2017 12:29, Stefan Hajnoczi wrote:
> > On Mon, Mar 13, 2017 at 10:41:15PM +0100, Max Reitz wrote:
> >> Implement the preallocation modes falloc and full for growing qcow2
> >> images.
> >>
> >> Signed-off-by: Max Reitz 
> >> ---
> >>  block/qcow2.c | 36 +++-
> >>  1 file changed, 35 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/block/qcow2.c b/block/qcow2.c
> >> index 80fb815b15..b6b08d70da 100644
> >> --- a/block/qcow2.c
> >> +++ b/block/qcow2.c
> >> @@ -2604,7 +2604,9 @@ static int qcow2_truncate(BlockDriverState *bs, 
> >> int64_t offset,
> >>  int64_t new_l1_size;
> >>  int ret;
> >>  
> >> -if (prealloc != PREALLOC_MODE_OFF && prealloc != 
> >> PREALLOC_MODE_METADATA) {
> >> +if (prealloc != PREALLOC_MODE_OFF && prealloc != 
> >> PREALLOC_MODE_METADATA &&
> >> +prealloc != PREALLOC_MODE_FALLOC && prealloc != 
> >> PREALLOC_MODE_FULL)
> > 
> > Now all cases are covered so this if statement can be dropped.  If you
> > are worried about new preallocation modes being added in the future then
> > the error_setg() can be moved into the switch statement's default case.
> 
> No, because the switch comes after we have already grown the L1 table.
> That wouldn't be so nice.

Oops :)

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] pc: q35: bump max_cpus to INT32_MAX

[Radim Krčmář]

2017-03-22 18:06+0200, Michael S. Tsirkin:
> On Wed, Mar 22, 2017 at 04:59:06PM +0100, Radim Krčmář wrote:
>> QEMU does not allocate based on machine's max_cpus, but only uses it to
>> limit the maximum selected by user and the actual limit of VCPUs is
>> enfoced by other components:
>>  - machine without vIOMMU ends at 255 VCPUs
>>  - TCG currently doesn't support x2APIC, so it also ends below 256
>>  - KVM with vIOMMU cannot exceed the KVM limit (currently 288)
>> 
>> Using a big value should bring no drawbacks and the benefit is that
>> components (especially out-of-tree KVM) can bump their limits without
>> touching machine's max_cpus.
>> 
>> max_cpus is unsigned, but it is treated as signed at least in printf and
>> the other two billion VCPU won't be needed for a while, so we can ignore
>> possible bugs by using signed max.
>> 
>> Signed-off-by: Radim Krčmář 
>> ---
>>   Should the 2.9 machine type still have 288?
> 
> It doesn't look like a bugfix to me.
> So if we can defer past 2.9 that would be best IMO.

Sure, I'll wait until the 2.10 machine type pops up.

Thanks.