[Qemu-devel] [PATCH 1/2] ich9: add tco reset

2017-03-17 Thread Gerd Hoffmann
Without that the nmi watchdog fires after reboot and the
linux kernel is confused where the nmi comes from:

   Uhhuh. NMI received for unknown reason 20 on CPU 0.
   Do you have a strange power saving mode enabled?
   Dazed and confused, but trying to continue

Signed-off-by: Gerd Hoffmann 
---
 hw/acpi/ich9.c|  1 +
 hw/acpi/tco.c | 39 ---
 include/hw/acpi/tco.h |  1 +
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 5c279bb..4d03182 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -250,6 +250,7 @@ static void pm_reset(void *opaque)
 acpi_pm1_cnt_reset(&pm->acpi_regs);
 acpi_pm_tmr_reset(&pm->acpi_regs);
 acpi_gpe_reset(&pm->acpi_regs);
+acpi_pm_tco_reset(&pm->tco_regs);
 
 pm->smi_en = 0;
 if (!pm->smm_enabled) {
diff --git a/hw/acpi/tco.c b/hw/acpi/tco.c
index b4adac8..6e5fca8 100644
--- a/hw/acpi/tco.c
+++ b/hw/acpi/tco.c
@@ -214,27 +214,28 @@ static const MemoryRegionOps tco_io_ops = {
 .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+void acpi_pm_tco_reset(TCOIORegs *tr)
+{
+tr->tco.rld  = TCO_RLD_DEFAULT;
+tr->tco.din  = TCO_DAT_IN_DEFAULT;
+tr->tco.dout = TCO_DAT_OUT_DEFAULT;
+tr->tco.sts1 = TCO1_STS_DEFAULT;
+tr->tco.sts2 = TCO2_STS_DEFAULT;
+tr->tco.cnt1 = TCO1_CNT_DEFAULT;
+tr->tco.cnt2 = TCO2_CNT_DEFAULT;
+tr->tco.msg1 = TCO_MESSAGE1_DEFAULT;
+tr->tco.msg2 = TCO_MESSAGE2_DEFAULT;
+tr->tco.wdcnt= TCO_WDCNT_DEFAULT;
+tr->tco.tmr  = TCO_TMR_DEFAULT;
+tr->sw_irq_gen   = SW_IRQ_GEN_DEFAULT;
+tr->expire_time  = -1;
+tr->timeouts_no  = 0;
+}
+
 void acpi_pm_tco_init(TCOIORegs *tr, MemoryRegion *parent)
 {
-*tr = (TCOIORegs) {
-.tco = {
-.rld  = TCO_RLD_DEFAULT,
-.din  = TCO_DAT_IN_DEFAULT,
-.dout = TCO_DAT_OUT_DEFAULT,
-.sts1 = TCO1_STS_DEFAULT,
-.sts2 = TCO2_STS_DEFAULT,
-.cnt1 = TCO1_CNT_DEFAULT,
-.cnt2 = TCO2_CNT_DEFAULT,
-.msg1 = TCO_MESSAGE1_DEFAULT,
-.msg2 = TCO_MESSAGE2_DEFAULT,
-.wdcnt= TCO_WDCNT_DEFAULT,
-.tmr  = TCO_TMR_DEFAULT,
-},
-.sw_irq_gen= SW_IRQ_GEN_DEFAULT,
-.tco_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, tco_timer_expired, 
tr),
-.expire_time   = -1,
-.timeouts_no   = 0,
-};
+tr->tco_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, tco_timer_expired, tr);
+acpi_pm_tco_reset(tr);
 memory_region_init_io(&tr->io, memory_region_owner(parent),
   &tco_io_ops, tr, "sm-tco", ICH9_PMIO_TCO_LEN);
 memory_region_add_subregion(parent, ICH9_PMIO_TCO_RLD, &tr->io);
diff --git a/include/hw/acpi/tco.h b/include/hw/acpi/tco.h
index 52ad767..7d14043 100644
--- a/include/hw/acpi/tco.h
+++ b/include/hw/acpi/tco.h
@@ -74,6 +74,7 @@ typedef struct TCOIORegs {
 } TCOIORegs;
 
 /* tco.c */
+void acpi_pm_tco_reset(TCOIORegs *tr);
 void acpi_pm_tco_init(TCOIORegs *tr, MemoryRegion *parent);
 
 extern const VMStateDescription vmstate_tco_io_sts;
-- 
1.8.3.1




[Qemu-devel] [PATCH] cirrus: fix off-by-one in cirrus_bitblt_rop_bkwd_transp_*_16

2017-03-17 Thread Gerd Hoffmann
The switch from pointers to addresses (commit
026aeffcb4752054830ba203020ed6eb05bcaba8 and
ffaf857778286ca54e3804432a2369a279e73aa7) added
a off-by-one bug to 16bit backward blits.  Fix.

Reported-by: 李强 
Signed-off-by: Gerd Hoffmann 
---
 hw/display/cirrus_vga_rop.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/cirrus_vga_rop.h b/hw/display/cirrus_vga_rop.h
index c61a677..0841b9e 100644
--- a/hw/display/cirrus_vga_rop.h
+++ b/hw/display/cirrus_vga_rop.h
@@ -219,7 +219,7 @@ glue(glue(cirrus_bitblt_rop_bkwd_transp_, 
ROP_NAME),_16)(CirrusVGAState *s,
 srcpitch += bltwidth;
 for (y = 0; y < bltheight; y++) {
 for (x = 0; x < bltwidth; x+=2) {
-ROP_OP_TR_16(s, dstaddr, cirrus_src16(s, srcaddr), transp);
+ROP_OP_TR_16(s, dstaddr - 1, cirrus_src16(s, srcaddr - 1), transp);
 dstaddr -= 2;
 srcaddr -= 2;
 }
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH 1/2] ich9: add tco reset

2017-03-17 Thread Gerd Hoffmann
On Fr, 2017-03-17 at 08:09 +0100, Gerd Hoffmann wrote:
> Without that the nmi watchdog fires after reboot and the
> linux kernel is confused where the nmi comes from:
> 
>Uhhuh. NMI received for unknown reason 20 on CPU 0.
>Do you have a strange power saving mode enabled?
>Dazed and confused, but trying to continue

Oops scratch that.  Patch not working, must be something else, didn't
manage to root-cause this one yet ...

cheers,
  Gerd




[Qemu-devel] [PATCH] ui/egl-helpers: fix egl 1.5 display init

2017-03-17 Thread Gerd Hoffmann
Unfortunaly switching to getPlatformDisplayEXT isn't as easy as
implemented by 0ea1523fb6703aa0dcd65e66b59e96fec028e60a.  See the
longish comment for the complete story.

Cc: Frediano Ziglio 
Suggested-by: Hans de Goede 
Signed-off-by: Gerd Hoffmann 
---
 ui/egl-helpers.c | 53 +++--
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 584dd1b..48686cf 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -192,6 +192,51 @@ EGLSurface qemu_egl_init_surface_x11(EGLContext ectx, 
Window win)
 
 /* -- */
 
+/*
+ * Taken from glamor_egl.h from the Xorg xserver, which is MIT licensed
+ *
+ * Create an EGLDisplay from a native display type. This is a little quirky
+ * for a few reasons.
+ *
+ * 1: GetPlatformDisplayEXT and GetPlatformDisplay are the API you want to
+ * use, but have different function signatures in the third argument; this
+ * happens not to matter for us, at the moment, but it means epoxy won't alias
+ * them together.
+ *
+ * 2: epoxy 1.3 and earlier don't understand EGL client extensions, which
+ * means you can't call "eglGetPlatformDisplayEXT" directly, as the resolver
+ * will crash.
+ *
+ * 3: You can't tell whether you have EGL 1.5 at this point, because
+ * eglQueryString(EGL_VERSION) is a property of the display, which we don't
+ * have yet. So you have to query for extensions no matter what. Fortunately
+ * epoxy_has_egl_extension _does_ let you query for client extensions, so
+ * we don't have to write our own extension string parsing.
+ *
+ * 4. There is no EGL_KHR_platform_base to complement the EXT one, thus one
+ * needs to know EGL 1.5 is supported in order to use the eglGetPlatformDisplay
+ * function pointer.
+ * We can workaround this (circular dependency) by probing for the EGL 1.5
+ * platform extensions (EGL_KHR_platform_gbm and friends) yet it doesn't seem
+ * like mesa will be able to adverise these (even though it can do EGL 1.5).
+ */
+static EGLDisplay qemu_egl_get_display(void *native)
+{
+#ifdef EGL_PLATFORM_GBM_MESA
+/* In practise any EGL 1.5 implementation would support the EXT extension 
*/
+if (epoxy_has_egl_extension(NULL, "EGL_EXT_platform_base")) {
+PFNEGLGETPLATFORMDISPLAYEXTPROC getPlatformDisplayEXT =
+(void *) eglGetProcAddress("eglGetPlatformDisplayEXT");
+if (getPlatformDisplayEXT) {
+return getPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, native, NULL);
+}
+}
+#endif
+
+/* fallback */
+return eglGetDisplay(native);
+}
+
 int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug)
 {
 static const EGLint conf_att_gl[] = {
@@ -222,12 +267,8 @@ int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, 
bool debug)
 setenv("LIBGL_DEBUG", "verbose", true);
 }
 
-egl_dbg("eglGetDisplay (dpy %p) ...\n", dpy);
-#ifdef EGL_MESA_platform_gbm
-qemu_egl_display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, dpy, 
NULL);
-#else
-qemu_egl_display = eglGetDisplay(dpy);
-#endif
+egl_dbg("qemu_egl_get_display (dpy %p) ...\n", dpy);
+qemu_egl_display = qemu_egl_get_display(dpy);
 if (qemu_egl_display == EGL_NO_DISPLAY) {
 error_report("egl: eglGetDisplay failed");
 return -1;
-- 
1.8.3.1




Re: [Qemu-devel] [Qemu-ppc] [PATCH 48/77] ppc/pnv: Add model for Power8 PHB3 PCIe Host bridge

2017-03-17 Thread Cédric Le Goater
Ben,

Quick question,

> +/* This is called whenever the PHB LSI, MSI source ID register or
> + * the PBCQ irq filters are written.
> + */
> +void pnv_phb3_remap_irqs(PnvPhb3State *phb)
> +{
> +uint32_t local, global, count, mask, comp;
> +uint64_t baren;
> +
> +/* First check if we are enabled. Unlike real HW we don't separate TX 
> and RX
> + * so we enable if both are set
> + */
> +baren = phb->pbcq->nest_regs[PBCQ_NEST_BAR_EN];
> +if (!(baren & PBCQ_NEST_BAR_EN_IRSN_RX) ||
> +!(baren & PBCQ_NEST_BAR_EN_IRSN_TX)) {
> +phb->lsi_ics->offset = 0;
> +return;
> +}
> +
> +/* Grab local LSI source ID */
> +local = GETFIELD(PHB_LSI_SRC_ID, phb->regs[PHB_LSI_SOURCE_ID >> 3]) << 3;
> +
> +/* Grab global one and compare */
> +global = GETFIELD(PBCQ_NEST_LSI_SRC,
> +  phb->pbcq->nest_regs[PBCQ_NEST_LSI_SRC_ID]) << 3;
> +if (global != local) {
> +/* This happens during initialization, let's come back when we
> + * are properly configured
> + */
> +phb->lsi_ics->offset = 0;
> +return;
> +}
> +
> +/* Get the base on the powerbus */
> +comp = GETFIELD(PBCQ_NEST_IRSN_COMP,
> +phb->pbcq->nest_regs[PBCQ_NEST_IRSN_COMPARE]);
> +mask = GETFIELD(PBCQ_NEST_IRSN_COMP,
> +phb->pbcq->nest_regs[PBCQ_NEST_IRSN_MASK]);
> +count = ((~mask) + 1) & 0x7;
> +phb->total_irq = count;
> +
> +/* Sanity checks */
> +if ((global + 8) > count) {
> +DBG_MAP(phb, "LSIs out of reach: LSI base=%d total irq=%d",
> +global, count);
> +}
> +if (count > 2048) {
> +DBG_MAP(phb, "More interrupts than supported: %d", count);
> +}
> +if ((comp & mask) != comp) {
> +DBG_MAP(phb, "IRQ compare bits not in mask: comp=0x%x mask=0x%x",
> +comp, mask);
> +comp &= mask;
> +}
> +/* Setup LSI offset */
> +phb->lsi_ics->offset = comp + global;
> +
> +/* Setup MSI offset */
> +pnv_phb3_msi_update_config(phb->msis, comp, count);

I changed that to :

pnv_phb3_msi_update_config(phb->msis, comp, count - PHB_NUM_LSI);

else the IRQ numbers overlap with the LSI and I think this why we were 
uselessly looping on the EOI.

Correct ? 

C.
 
> +
> +DBG_MAP(phb, "Initialized for %d interrupts @0x%x, LSI off=%d",
> +count, comp, global);
> +}
> +




Re: [Qemu-devel] [PATCH for-2.10 6/8] ppc/pnv: Add cut down PSI bridge model and hookup external interrupt

2017-03-17 Thread Cédric Le Goater
On 03/17/2017 03:00 AM, David Gibson wrote:
> On Thu, Mar 16, 2017 at 02:52:17PM +0100, Cédric Le Goater wrote:
>> On 03/15/2017 07:16 AM, David Gibson wrote:
>>> On Wed, Mar 08, 2017 at 11:52:49AM +0100, Cédric Le Goater wrote:
 From: Benjamin Herrenschmidt 

 The PSI (Processor Service Interface) Controller is one of the engines
 of the "Bridge" unit which connects the different interfaces to the
 Power Processor.

 This adds just enough of the PSI bridge to handle various on-chip and
 the one external interrupt. The rest of PSI has to do with the link to
 the IBM FSP service processor which we don't plan to emulate (not used
 on OpenPower machines).

 Signed-off-by: Benjamin Herrenschmidt 
 [clg: - updated for qemu-2.9
   - changed the XSCOM interface to fit new model
   - QOMified the model
   - reworked set_xive and worked around a skiboot bug
   - removed the 'psi_mmio_to_xscom' mapping array ]
 Signed-off-by: Cédric Le Goater 
 ---
  hw/ppc/Makefile.objs   |   2 +-
  hw/ppc/pnv.c   |  35 ++-
  hw/ppc/pnv_psi.c   | 583 
 +
  include/hw/ppc/pnv.h   |   8 +
  include/hw/ppc/pnv_psi.h   |  61 +
  include/hw/ppc/pnv_xscom.h |   3 +
  6 files changed, 685 insertions(+), 7 deletions(-)
  create mode 100644 hw/ppc/pnv_psi.c
  create mode 100644 include/hw/ppc/pnv_psi.h

 diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
 index 001293423c8d..dc19ee17fa57 100644
 --- a/hw/ppc/Makefile.objs
 +++ b/hw/ppc/Makefile.objs
 @@ -6,7 +6,7 @@ obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o 
 spapr_rtas.o
  obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
  obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o
  # IBM PowerNV
 -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o
 +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o
  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
  obj-y += spapr_pci_vfio.o
  endif
 diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
 index 0ae11cc3a2ca..85b00bf235c6 100644
 --- a/hw/ppc/pnv.c
 +++ b/hw/ppc/pnv.c
 @@ -356,15 +356,22 @@ static void ppc_powernv_reset(void)
   * have a CPLD that will collect the SerIRQ and shoot them as a
   * single level interrupt to the P8 chip. So let's setup a hook
   * for doing just that.
 - *
 - * Note: The actual interrupt input isn't emulated yet, this will
 - * come with the PSI bridge model.
   */
  static void pnv_lpc_isa_irq_handler_cpld(void *opaque, int n, int level)
  {
 -/* We don't yet emulate the PSI bridge which provides the external
 - * interrupt, so just drop interrupts on the floor
 - */
 +PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine());
 +uint32_t old_state = pnv->cpld_irqstate;
 +PnvChip *chip = opaque;
 +
 +if (level) {
 +pnv->cpld_irqstate |= 1u << n;
 +} else {
 +pnv->cpld_irqstate &= ~(1u << n);
 +}
 +if (pnv->cpld_irqstate != old_state) {
 +pnv_psi_irq_set(&chip->psi, PSIHB_IRQ_EXTERNAL,
 +pnv->cpld_irqstate != 0);
 +}
  }
  
  static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level)
 @@ -702,6 +709,11 @@ static void pnv_chip_init(Object *obj)
  
  object_initialize(&chip->lpc, sizeof(chip->lpc), TYPE_PNV_LPC);
  object_property_add_child(obj, "lpc", OBJECT(&chip->lpc), NULL);
 +
 +object_initialize(&chip->psi, sizeof(chip->psi), TYPE_PNV_PSI);
 +object_property_add_child(obj, "psi", OBJECT(&chip->psi), NULL);
 +object_property_add_const_link(OBJECT(&chip->psi), "xics",
 +   OBJECT(qdev_get_machine()), 
 &error_abort);
  }
  
  static void pnv_chip_icp_realize(PnvChip *chip, Error **errp)
 @@ -722,6 +734,8 @@ static void pnv_chip_realize(DeviceState *dev, Error 
 **errp)
  char *typename = pnv_core_typename(pcc->cpu_model);
  size_t typesize = object_type_get_instance_size(typename);
  int i, core_hwid;
 +MachineState *machine = MACHINE(qdev_get_machine());
 +PnvMachineState *pnv = POWERNV_MACHINE(machine);
  
  if (!object_class_by_name(typename)) {
  error_setg(errp, "Unable to find PowerNV CPU Core '%s'", 
 typename);
 @@ -797,6 +811,15 @@ static void pnv_chip_realize(DeviceState *dev, Error 
 **errp)
  }
  g_free(typename);
  
 +
 +/* Processor Service Interface (PSI) Host Bridge */
 +object_property_set_bool(OBJECT(&chip->psi), true, "realized",
 + &error_fatal);
 +pnv_xscom_add_sub

Re: [Qemu-devel] [PATCH] cirrus: fix off-by-one in cirrus_bitblt_rop_bkwd_transp_*_16

2017-03-17 Thread 李强


> -Original Message-
> From: Qemu-devel
> [mailto:qemu-devel-bounces+liqiang6-s=360...@nongnu.org] On Behalf Of
> Gerd Hoffmann
> Sent: Friday, March 17, 2017 3:22 PM
> To: qemu-devel@nongnu.org
> Cc: Gerd Hoffmann
> Subject: [Qemu-devel] [PATCH] cirrus: fix off-by-one in
> cirrus_bitblt_rop_bkwd_transp_*_16
> 
> The switch from pointers to addresses (commit
> 026aeffcb4752054830ba203020ed6eb05bcaba8 and
> ffaf857778286ca54e3804432a2369a279e73aa7) added a off-by-one bug to
> 16bit backward blits.  Fix.
> 
> Reported-by: 李强 
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Li Qiang 

> ---
>  hw/display/cirrus_vga_rop.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/cirrus_vga_rop.h b/hw/display/cirrus_vga_rop.h index
> c61a677..0841b9e 100644
> --- a/hw/display/cirrus_vga_rop.h
> +++ b/hw/display/cirrus_vga_rop.h
> @@ -219,7 +219,7 @@ glue(glue(cirrus_bitblt_rop_bkwd_transp_,
> ROP_NAME),_16)(CirrusVGAState *s,
>  srcpitch += bltwidth;
>  for (y = 0; y < bltheight; y++) {
>  for (x = 0; x < bltwidth; x+=2) {
> -ROP_OP_TR_16(s, dstaddr, cirrus_src16(s, srcaddr), transp);
> +ROP_OP_TR_16(s, dstaddr - 1, cirrus_src16(s, srcaddr - 1),
> + transp);
>  dstaddr -= 2;
>  srcaddr -= 2;
>  }
> --
> 1.8.3.1
> 



Re: [Qemu-devel] [PATCH] ui/egl-helpers: fix egl 1.5 display init

2017-03-17 Thread Marc-André Lureau
On Fri, Mar 17, 2017 at 12:12 PM Gerd Hoffmann  wrote:

> Unfortunaly switching to getPlatformDisplayEXT isn't as easy as
> implemented by 0ea1523fb6703aa0dcd65e66b59e96fec028e60a.  See the
> longish comment for the complete story.
>
> Cc: Frediano Ziglio 
> Suggested-by: Hans de Goede 
> Signed-off-by: Gerd Hoffmann 
> ---



Reviewed-by: Marc-André Lureau 



>  ui/egl-helpers.c | 53
> +++--
>  1 file changed, 47 insertions(+), 6 deletions(-)
>
> diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
> index 584dd1b..48686cf 100644
> --- a/ui/egl-helpers.c
> +++ b/ui/egl-helpers.c
> @@ -192,6 +192,51 @@ EGLSurface qemu_egl_init_surface_x11(EGLContext ectx,
> Window win)
>
>  /* --
> */
>
> +/*
> + * Taken from glamor_egl.h from the Xorg xserver, which is MIT licensed
> + *
> + * Create an EGLDisplay from a native display type. This is a little
> quirky
> + * for a few reasons.
> + *
> + * 1: GetPlatformDisplayEXT and GetPlatformDisplay are the API you want to
> + * use, but have different function signatures in the third argument; this
> + * happens not to matter for us, at the moment, but it means epoxy won't
> alias
> + * them together.
> + *
> + * 2: epoxy 1.3 and earlier don't understand EGL client extensions, which
> + * means you can't call "eglGetPlatformDisplayEXT" directly, as the
> resolver
> + * will crash.
> + *
> + * 3: You can't tell whether you have EGL 1.5 at this point, because
> + * eglQueryString(EGL_VERSION) is a property of the display, which we
> don't
> + * have yet. So you have to query for extensions no matter what.
> Fortunately
> + * epoxy_has_egl_extension _does_ let you query for client extensions, so
> + * we don't have to write our own extension string parsing.
> + *
> + * 4. There is no EGL_KHR_platform_base to complement the EXT one, thus
> one
> + * needs to know EGL 1.5 is supported in order to use the
> eglGetPlatformDisplay
> + * function pointer.
> + * We can workaround this (circular dependency) by probing for the EGL 1.5
> + * platform extensions (EGL_KHR_platform_gbm and friends) yet it doesn't
> seem
> + * like mesa will be able to adverise these (even though it can do EGL
> 1.5).
> + */
> +static EGLDisplay qemu_egl_get_display(void *native)
> +{
> +#ifdef EGL_PLATFORM_GBM_MESA
> +/* In practise any EGL 1.5 implementation would support the EXT
> extension */
> +if (epoxy_has_egl_extension(NULL, "EGL_EXT_platform_base")) {
> +PFNEGLGETPLATFORMDISPLAYEXTPROC getPlatformDisplayEXT =
> +(void *) eglGetProcAddress("eglGetPlatformDisplayEXT");
> +if (getPlatformDisplayEXT) {
> +return getPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, native,
> NULL);
> +}
> +}
> +#endif
> +
> +/* fallback */
> +return eglGetDisplay(native);
> +}
> +
>  int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug)
>  {
>  static const EGLint conf_att_gl[] = {
> @@ -222,12 +267,8 @@ int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool
> gles, bool debug)
>  setenv("LIBGL_DEBUG", "verbose", true);
>  }
>
> -egl_dbg("eglGetDisplay (dpy %p) ...\n", dpy);
> -#ifdef EGL_MESA_platform_gbm
> -qemu_egl_display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA,
> dpy, NULL);
> -#else
> -qemu_egl_display = eglGetDisplay(dpy);
> -#endif
> +egl_dbg("qemu_egl_get_display (dpy %p) ...\n", dpy);
> +qemu_egl_display = qemu_egl_get_display(dpy);
>  if (qemu_egl_display == EGL_NO_DISPLAY) {
>  error_report("egl: eglGetDisplay failed");
>  return -1;
> --
> 1.8.3.1
>
>
> --
Marc-André Lureau


Re: [Qemu-devel] Proposal for deprecating unsupported host OSes & architecutures

2017-03-17 Thread Markus Armbruster
"Daniel P. Berrange"  writes:

> On Thu, Mar 16, 2017 at 03:55:13PM +, Peter Maydell wrote:
>> On 16 March 2017 at 15:46, Daniel P. Berrange  wrote:
>> > On Thu, Mar 16, 2017 at 03:23:45PM +, 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:
>> >>
>> >> The QEMU Project intends to drop support in a future release for any
>> >> host OS or architecture which we do not have access to a build and test
>> >> machine for. This affects the following host OSes:
>> >>  * Native CYGWIN building
>> >>  * GNU/kFreeBSD
>> >>  * FreeBSD
>> >>  * DragonFly BSD
>> >>  * NetBSD
>> >>  * OpenBSD
>> >>  * Solaris
>> >>  * AIX
>> >>  * Haiku
>> >> and the following host CPU architectures:
>> >>  * ia64
>> >>  * sparc
>> >>
>> >> Specifically, if we do not have a build and test system available
>> >> to us by the time we release QEMU 2.10, we will remove support in the
>> >> release that follows 2.10.
>> >> -
>> >>
>> >> I'm not sure here if we want to just have this as a bald list,
>> >> or to have some kind of two tier setup with OSes we expect to
>> >> dump in one tier and OSes where we're really trolling for a build
>> >> machine in the other tier (the "unlikely to dump" category would
>> >> get most of the BSD variants in it). Putting out a changelog
>> >> that says "we're gonna drop all the BSDs" seems like it might
>> >> produce a lot of yelling?
>> >
>> > I think it depends on the level of bit-rot we are aware of, and
>> > whether we expect anyone is likely to fix the bit-rot should it
>> > be discovered.
>> >
>> > Simply not having a build machine for QEMU CI doesn't imply that
>> > it is totally broken, and even if some pieces are broken, it
>> > doesn't imply that QEMU is unusable.
>> 
>> No, but it does imply that our CI is missing a big chunk.
>> Realistically, for the BSDs where I want to get to is "we
>> have BSD coverage in our CI setup". The problem at the moment
>> is that we (presumably) have BSD users but we have basically
>> no BSD developers active upstream, which in my view is not
>> a very long-term satisfactory situation.
>> 
>> > IOW, I think there is a reasonable 3 tier set here
>> >
>> >  1. Stuff we actively test builds & thus guarantee will work for
>> > any QEMU release going forward.
>> >
>> >  2. Stuff we don't actively test, but generally assume is mostly
>> > working, and likely to be fixed if & when problems are found
>> >
>> >  3. Stuff we don't actively test,  assume is probably broken
>> > and unlikely to be fixed if reported
>> >
>> > Stuff in tier 3 should be candidate for deletion. Stuff in tier
>> > 2 shouldn't be removed, but it might drop into tier 3 at some
>> > point if people stop caring about fixing problems when found.
>> > Conversely tier 2 might rise to tier 1 if CI turns up.
>> 
>> I don't really want a tier 2. Either we support it enough
>> to at least be able to run "make && make check" on some
>> representative system, or we don't support it at all.
>> Code which we have but are really reluctant to touch because
>> we don't even test it builds (like bsd-user/) is really bad
>> for preventing cleanups.
>
> IMHO we should not be afraid of cleaning up code in such cases.
> If bsd-user accidentally breaks because we clean up some other
> parts of QEMU, so be it. If someone cares they'll step forward,
> if not, it'll be a sign that it its material for tier 3 & thus
> eventual removal.
>
> I'm just pretty wary of gratuitously deleting stuff that still
> has a reasonable chance of being functional, simply because
> we lack CI testing.

That's pre-git thinking :)

I've done a fair amount of cleanup work.  Having to touch something you
can't test *always* deters.  Working CI is the rock-bottom level of "can
test".

Sure, "if somebody cares, they'll step forward" to fix whatever I break.
Eventually.  But I'd like to turn this argument inside out: if somebody
cares, they can bloody well make CI work, so it doesn't break so easily.
If they discover they care only after we dropped support, they can
bloody well fish its code out of git.

I'm willing to tolerate the odd host-specific configure snippet without
CI.  But no conditional compilation, be it via Make or #if.



Re: [Qemu-devel] Proposal for deprecating unsupported host OSes & architecutures

2017-03-17 Thread Thomas Huth
On 16.03.2017 17:52, Paolo Bonzini wrote:
> 
> 
> On 16/03/2017 16:55, Peter Maydell wrote:
>>> IOW, I think there is a reasonable 3 tier set here
>>>
>>>  1. Stuff we actively test builds & thus guarantee will work for
>>> any QEMU release going forward.
>>>
>>>  2. Stuff we don't actively test, but generally assume is mostly
>>> working, and likely to be fixed if & when problems are found
>>>
>>>  3. Stuff we don't actively test,  assume is probably broken
>>> and unlikely to be fixed if reported
>>>
>>> Stuff in tier 3 should be candidate for deletion. Stuff in tier
>>> 2 shouldn't be removed, but it might drop into tier 3 at some
>>> point if people stop caring about fixing problems when found.
>>> Conversely tier 2 might rise to tier 1 if CI turns up.
>>
>> I don't really want a tier 2. Either we support it enough
>> to at least be able to run "make && make check" on some
>> representative system, or we don't support it at all.
>> Code which we have but are really reluctant to touch because
>> we don't even test it builds (like bsd-user/) is really bad
>> for preventing cleanups.
> 
> I think we should further differentiate between bsd-user/ and softmmu.
> System emulation is just another program where we mostly compile to C
> standard + POSIX or C standard + Win32.  There are certainly places
> where we use Linux-specific extensions but it's not that special.
> Neither BSD nor Solaris are particularly hard to support there.
> 
> On the other hand, bsd-user is extremely BSD specific, and ought to have
> CI.  I think there should be a tier 2 for system emulation (which
> doesn't mean that anything there shouldn't be moved to tier 3 and
> eventually removed), but there shouldn't be a tier 2 for user-mode
> emulation.
> 
> In particular, I believe that we should remove bsd-user from 2.10 unless
> the downstream BSD port is merged back (and CI is provided).  There is
> no point in keeping the current half-baked code without thread support.

I think you made a good point here.
So "+1" from my side to remove "bsd-user" and "tcg/ia64" in QEMU 2.10
or 2.11 (unless someone speaks up and provides maintainence, of course).

... and I think we should add a message to the configure script for 2.9
when somebody tries to use these subsystems, so that the removal does
not happen without warning first?

 Thomas




Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

2017-03-17 Thread Sebastian Ott
On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> For future code reuse purpose, this decouples the cio code with
> the ccw device specific parts from ccw_device_cancel_halt_clear,
> and makes a new common I/O interface named cio_cancel_halt_clear.

What would the user of cio_cancel_halt_clear be?

Sebastian




[Qemu-devel] [PATCH] vnc: fix a qio-channel leak

2017-03-17 Thread Marc-André Lureau
Spotted by ASAN.

Signed-off-by: Marc-André Lureau 
---
 ui/vnc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index 8bfb1e0685..6e93b883b5 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3677,6 +3677,7 @@ static int vnc_display_listen_addr(VncDisplay *vd,
 qio_channel_set_name(QIO_CHANNEL(sioc), name);
 if (qio_channel_socket_listen_sync(
 sioc, rawaddrs[i], listenerr == NULL ? &listenerr : NULL) < 0) 
{
+object_unref(OBJECT(sioc));
 continue;
 }
 listening = true;
-- 
2.12.0.191.gc5d8de91d




Re: [Qemu-devel] [PATCH v4 02/16] s390: cio: export more interfaces

2017-03-17 Thread Sebastian Ott
On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> Export the common I/O interfaces those are needed by an I/O
> subchannel driver to actually talk to the subchannel.

Which I/O subchannel driver are you talking about? I know of just one
and it can't be build as a module.

Sebastian




Re: [Qemu-devel] [Qemu-block] [PATCH for-2.9] block: Propagate error in bdrv_open_backing_file

2017-03-17 Thread Alberto Garcia
On Fri 17 Mar 2017 02:56:30 AM CET, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  block.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/block.c b/block.c
> index cb57370..6621167 100644
> --- a/block.c
> +++ b/block.c
> @@ -2025,6 +2025,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
> *parent_options,
>  bdrv_set_backing_hd(bs, backing_hd, &local_err);
>  bdrv_unref(backing_hd);
>  if (local_err) {
> +error_propagate(errp, local_err);
>  ret = -EINVAL;
>  goto free_exit;
>  }

Reviewed-by: Alberto Garcia 

Berto



Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

2017-03-17 Thread Dong Jia Shi
* Sebastian Ott  [2017-03-17 10:26:51 +0100]:

> On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> > For future code reuse purpose, this decouples the cio code with
> > the ccw device specific parts from ccw_device_cancel_halt_clear,
> > and makes a new common I/O interface named cio_cancel_halt_clear.
> 
> What would the user of cio_cancel_halt_clear be?
Hi Sebastian,

We are trying to introduce a new I/O subchannel driver to enable usage
of I/O subchannels via VFIO-CCW, so we can passthru a subchannel to a
KVM guest. And that new vfio-ccw subchannel driver is the user. :>

> 
> Sebastian

-- 
Dong Jia




Re: [Qemu-devel] [PATCH v2] qemu-ga: obey LISTEN_PID when using systemd socket activation

2017-03-17 Thread Stefan Hajnoczi
On Thu, Mar 16, 2017 at 05:36:40PM +0100, Paolo Bonzini wrote:
> qemu-ga's socket activation support was not obeying the LISTEN_PID
> environment variable, which avoids that a process uses a socket-activation
> file descriptor meant for its parent.
> 
> Mess can for example ensue if a process forks a children before consuming
> the socket-activation file descriptor and therefore setting O_CLOEXEC
> on it.
> 
> Luckily, qemu-nbd also got socket activation code, and its copy does
> support LISTEN_PID.  Some extra fixups are needed to ensure that the
> code can be used for both, but that's what this patch does.  The
> main change is to replace get_listen_fds's "consume" argument with
> the FIRST_SOCKET_ACTIVATION_FD macro from the qemu-nbd code.

I intentionally wrote qga socket activation this way.  It allows socket
activation to work together with daemonization.  That combination is
probably not very useful so it's fine to get rid of it.

Please add an error message in qga/main.c if socket activation is used
in combination with the -d/--daemonize flag.

> Cc: "Richard W.M. Jones" 
> Cc: Stefan Hajnoczi 
> Reviewed-by: Daniel P. Berrange 
> Signed-off-by: Paolo Bonzini 
> ---
>  include/qemu/systemd.h |  26 +
>  qemu-nbd.c | 100 
> -
>  qga/main.c |  51 +++--
>  util/Makefile.objs |   1 +
>  util/systemd.c |  77 +
>  5 files changed, 125 insertions(+), 130 deletions(-)
>  create mode 100644 include/qemu/systemd.h
>  create mode 100644 util/systemd.c
> 
> diff --git a/include/qemu/systemd.h b/include/qemu/systemd.h
> new file mode 100644
> index 000..e6a877e
> --- /dev/null
> +++ b/include/qemu/systemd.h
> @@ -0,0 +1,26 @@
> +/*
> + * systemd socket activation support
> + *
> + * Copyright 2017 Red Hat, Inc. and/or its affiliates
> + *
> + * Authors:
> + *  Richard W.M. Jones 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef QEMU_SYSTEMD_H
> +#define QEMU_SYSTEMD_H 1
> +
> +#define FIRST_SOCKET_ACTIVATION_FD 3 /* defined by systemd ABI */
> +
> +/*
> + * Check if socket activation was requested via use of the
> + * LISTEN_FDS and LISTEN_PID environment variables.
> + *
> + * Returns 0 if no socket activation, or the number of FDs.
> + */
> +unsigned int check_socket_activation(void);
> +
> +#endif
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index e4fede6..e080fb7 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -28,6 +28,7 @@
>  #include "qemu/config-file.h"
>  #include "qemu/bswap.h"
>  #include "qemu/log.h"
> +#include "qemu/systemd.h"
>  #include "block/snapshot.h"
>  #include "qapi/util.h"
>  #include "qapi/qmp/qstring.h"
> @@ -474,98 +475,6 @@ static void setup_address_and_port(const char **address, 
> const char **port)
>  }
>  }
>  
> -#define FIRST_SOCKET_ACTIVATION_FD 3 /* defined by systemd ABI */
> -
> -#ifndef _WIN32
> -/*
> - * Check if socket activation was requested via use of the
> - * LISTEN_FDS and LISTEN_PID environment variables.
> - *
> - * Returns 0 if no socket activation, or the number of FDs.
> - */
> -static unsigned int check_socket_activation(void)
> -{
> -const char *s;
> -unsigned long pid;
> -unsigned long nr_fds;
> -unsigned int i;
> -int fd;
> -int err;
> -
> -s = getenv("LISTEN_PID");
> -if (s == NULL) {
> -return 0;
> -}
> -err = qemu_strtoul(s, NULL, 10, &pid);
> -if (err) {
> -if (verbose) {
> -fprintf(stderr, "malformed %s environment variable (ignored)\n",
> -"LISTEN_PID");
> -}
> -return 0;
> -}
> -if (pid != getpid()) {
> -if (verbose) {
> -fprintf(stderr, "%s was not for us (ignored)\n",
> -"LISTEN_PID");
> -}
> -return 0;
> -}
> -
> -s = getenv("LISTEN_FDS");
> -if (s == NULL) {
> -return 0;
> -}
> -err = qemu_strtoul(s, NULL, 10, &nr_fds);
> -if (err) {
> -if (verbose) {
> -fprintf(stderr, "malformed %s environment variable (ignored)\n",
> -"LISTEN_FDS");
> -}
> -return 0;
> -}
> -assert(nr_fds <= UINT_MAX);
> -
> -/* A limitation of current qemu-nbd is that it can only listen on
> - * a single socket.  When that limitation is lifted, we can change
> - * this function to allow LISTEN_FDS > 1, and remove the assertion
> - * in the main function below.
> - */
> -if (nr_fds > 1) {
> -error_report("qemu-nbd does not support socket activation with %s > 
> 1",
> - "LISTEN_FDS");
> -exit(EXIT_FAILURE);
> -}
> -
> -/* So these are not passed to any child processes we might start. */
> -unsetenv("LISTEN_FDS");
> -unsetenv("LISTEN_PID");
> -
> -/* So the file descript

Re: [Qemu-devel] [PATCH v4 02/16] s390: cio: export more interfaces

2017-03-17 Thread Dong Jia Shi
* Sebastian Ott  [2017-03-17 10:29:31 +0100]:

> On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> > Export the common I/O interfaces those are needed by an I/O
> > subchannel driver to actually talk to the subchannel.
> 
> Which I/O subchannel driver are you talking about? I know of just one
> and it can't be build as a module.
As I replied to you in another email, there will be a new I/O subchannel
driver which is named as vfio-ccw subchannel driver. That driver could
be built as a module, thus it needs these interfaces be exported.

> 
> Sebastian

-- 
Dong Jia




Re: [Qemu-devel] [PATCH] qemu-img: show help for invalid global options

2017-03-17 Thread Stefan Hajnoczi
On Thu, Mar 16, 2017 at 03:07:29AM +0100, Max Reitz wrote:
> On 13.03.2017 06:11, Stefan Hajnoczi wrote:
> > The qemu-img sub-command executes regardless of invalid global options:
> > 
> >   $ qemu-img --foo info test.img
> >   qemu-img: unrecognized option '--foo'
> >   image: test.img
> >   ...
> > 
> > The unrecognized option warning may be missed by the user.  This can
> > hide incorrect command-lines in scripts and confuse users.
> > 
> > This patch prints the help information and terminates instead of
> > executing the sub-command.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  qemu-img.c | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Well, now you get blown away by a wall of text and spotting what went
> wrong is actually not quite simple. Maybe we should follow the way of
> the coreutils, that is:
> 
> qemu-img: unrecognized option '--foo'
> Try 'qemu-img --help' for more information.
> 
> ?

Sure, I'll add another patch to improve that.

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v4 01/16] s390: cio: introduce cio_cancel_halt_clear

2017-03-17 Thread Sebastian Ott
On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> * Sebastian Ott  [2017-03-17 10:26:51 +0100]:
> 
> > On Fri, 17 Mar 2017, Dong Jia Shi wrote:
> > > For future code reuse purpose, this decouples the cio code with
> > > the ccw device specific parts from ccw_device_cancel_halt_clear,
> > > and makes a new common I/O interface named cio_cancel_halt_clear.
> > 
> > What would the user of cio_cancel_halt_clear be?
> Hi Sebastian,
> 
> We are trying to introduce a new I/O subchannel driver to enable usage
> of I/O subchannels via VFIO-CCW, so we can passthru a subchannel to a
> KVM guest. And that new vfio-ccw subchannel driver is the user. :>

OK, thanks for the explanation. May I suggest that you put these patches
in the series that actually introduces the new subchannel driver.

Sebastian




Re: [Qemu-devel] [PATCH 23/31] ram: Move migration_bitmap_rcu into RAMState

2017-03-17 Thread Dr. David Alan Gilbert
* Juan Quintela (quint...@redhat.com) wrote:
> Once there, rename the type to be shorter.
> 
> Signed-off-by: Juan Quintela 
> ---
>  migration/ram.c | 79 
> ++---
>  1 file changed, 42 insertions(+), 37 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index c14293c..d39d185 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -132,6 +132,19 @@ out:
>  return ret;
>  }
>  
> +struct RAMBitmap {
> +struct rcu_head rcu;
> +/* Main migration bitmap */
> +unsigned long *bmap;
> +/* bitmap of pages that haven't been sent even once
> + * only maintained and used in postcopy at the moment
> + * where it's used to send the dirtymap at the start
> + * of the postcopy phase
> + */
> +unsigned long *unsentmap;
> +};
> +typedef struct RAMBitmap RAMBitmap;
> +

I'm OK with this; although I can see the idea of naming it BitmapRcu,
given that the actual bmap is inside that and most of the rest of the type
is just the rcu wrapper.

Reviewed-by: Dr. David Alan Gilbert 

>  /* State of RAM for migration */
>  struct RAMState {
>  /* Last block that we have visited searching for dirty pages */
> @@ -180,6 +193,8 @@ struct RAMState {
>  uint64_t migration_dirty_pages;
>  /* protects modification of the bitmap */
>  QemuMutex bitmap_mutex;
> +/* Ram Bitmap protected by RCU */
> +RAMBitmap *ram_bitmap;
>  };
>  typedef struct RAMState RAMState;
>  
> @@ -236,18 +251,6 @@ struct PageSearchStatus {
>  };
>  typedef struct PageSearchStatus PageSearchStatus;
>  
> -static struct BitmapRcu {
> -struct rcu_head rcu;
> -/* Main migration bitmap */
> -unsigned long *bmap;
> -/* bitmap of pages that haven't been sent even once
> - * only maintained and used in postcopy at the moment
> - * where it's used to send the dirtymap at the start
> - * of the postcopy phase
> - */
> -unsigned long *unsentmap;
> -} *migration_bitmap_rcu;
> -
>  struct CompressParam {
>  bool done;
>  bool quit;
> @@ -554,7 +557,7 @@ ram_addr_t migration_bitmap_find_dirty(RAMState *rs, 
> RAMBlock *rb,
>  
>  unsigned long next;
>  
> -bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap;
> +bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap;
>  if (rs->ram_bulk_stage && nr > base) {
>  next = nr + 1;
>  } else {
> @@ -569,7 +572,7 @@ static inline bool migration_bitmap_clear_dirty(RAMState 
> *rs, ram_addr_t addr)
>  {
>  bool ret;
>  int nr = addr >> TARGET_PAGE_BITS;
> -unsigned long *bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap;
> +unsigned long *bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap;
>  
>  ret = test_and_clear_bit(nr, bitmap);
>  
> @@ -583,7 +586,7 @@ static void migration_bitmap_sync_range(RAMState *rs, 
> ram_addr_t start,
>  ram_addr_t length)
>  {
>  unsigned long *bitmap;
> -bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap;
> +bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap;
>  rs-> migration_dirty_pages +=
>  cpu_physical_memory_sync_dirty_bitmap(bitmap, start, length);
>  }
> @@ -1115,14 +1118,14 @@ static bool get_queued_page(RAMState *rs, 
> MigrationState *ms, PageSearchStatus *
>   */
>  if (block) {
>  unsigned long *bitmap;
> -bitmap = atomic_rcu_read(&migration_bitmap_rcu)->bmap;
> +bitmap = atomic_rcu_read(&rs->ram_bitmap)->bmap;
>  dirty = test_bit(*ram_addr_abs >> TARGET_PAGE_BITS, bitmap);
>  if (!dirty) {
>  trace_get_queued_page_not_dirty(
>  block->idstr, (uint64_t)offset,
>  (uint64_t)*ram_addr_abs,
>  test_bit(*ram_addr_abs >> TARGET_PAGE_BITS,
> - atomic_rcu_read(&migration_bitmap_rcu)->unsentmap));
> + atomic_rcu_read(&rs->ram_bitmap)->unsentmap));
>  } else {
>  trace_get_queued_page(block->idstr,
>(uint64_t)offset,
> @@ -1276,7 +1279,7 @@ static int ram_save_target_page(RAMState *rs, 
> MigrationState *ms, QEMUFile *f,
>  if (res < 0) {
>  return res;
>  }
> -unsentmap = atomic_rcu_read(&migration_bitmap_rcu)->unsentmap;
> +unsentmap = atomic_rcu_read(&rs->ram_bitmap)->unsentmap;
>  if (unsentmap) {
>  clear_bit(dirty_ram_abs >> TARGET_PAGE_BITS, unsentmap);
>  }
> @@ -1440,7 +1443,7 @@ void free_xbzrle_decoded_buf(void)
>  xbzrle_decoded_buf = NULL;
>  }
>  
> -static void migration_bitmap_free(struct BitmapRcu *bmap)
> +static void migration_bitmap_free(struct RAMBitmap *bmap)
>  {
>  g_free(bmap->bmap);
>  g_free(bmap->unsentmap);
> @@ -1449,11 +1452,13 @@ static void migration_bitmap_free(struct BitmapRcu 
> *bmap)
>  
>  static void ram_migration_cleanup(void *opaque)

Re: [Qemu-devel] [PATCH 25/31] ram: Use the RAMState bytes_transferred parameter

2017-03-17 Thread Dr. David Alan Gilbert
* Juan Quintela (quint...@redhat.com) wrote:
> Somewhere it was passed by reference, just use it from RAMState.
> 
> Signed-off-by: Juan Quintela 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  migration/ram.c | 77 
> -
>  1 file changed, 27 insertions(+), 50 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index f9933b2..9c9533d 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -477,12 +477,10 @@ static void xbzrle_cache_zero_page(RAMState *rs, 
> ram_addr_t current_addr)
>   * @block: block that contains the page we want to send
>   * @offset: offset inside the block for the page
>   * @last_stage: if we are at the completion stage
> - * @bytes_transferred: increase it with the number of transferred bytes
>   */
>  static int save_xbzrle_page(QEMUFile *f, RAMState *rs, uint8_t 
> **current_data,
>  ram_addr_t current_addr, RAMBlock *block,
> -ram_addr_t offset, bool last_stage,
> -uint64_t *bytes_transferred)
> +ram_addr_t offset, bool last_stage)
>  {
>  int encoded_len = 0, bytes_xbzrle;
>  uint8_t *prev_cached_page;
> @@ -538,7 +536,7 @@ static int save_xbzrle_page(QEMUFile *f, RAMState *rs, 
> uint8_t **current_data,
>  bytes_xbzrle += encoded_len + 1 + 2;
>  rs->xbzrle_pages++;
>  rs->xbzrle_bytes += bytes_xbzrle;
> -*bytes_transferred += bytes_xbzrle;
> +rs->bytes_transferred += bytes_xbzrle;
>  
>  return 1;
>  }
> @@ -701,20 +699,18 @@ static void migration_bitmap_sync(RAMState *rs)
>   * @block: block that contains the page we want to send
>   * @offset: offset inside the block for the page
>   * @p: pointer to the page
> - * @bytes_transferred: increase it with the number of transferred bytes
>   */
>  static int save_zero_page(RAMState *rs, QEMUFile *f, RAMBlock *block,
> -  ram_addr_t offset,
> -  uint8_t *p, uint64_t *bytes_transferred)
> +  ram_addr_t offset, uint8_t *p)
>  {
>  int pages = -1;
>  
>  if (is_zero_range(p, TARGET_PAGE_SIZE)) {
>  rs->zero_pages++;
> -*bytes_transferred += save_page_header(f, block,
> -   offset | 
> RAM_SAVE_FLAG_COMPRESS);
> +rs->bytes_transferred += save_page_header(f, block,
> +  offset | 
> RAM_SAVE_FLAG_COMPRESS);
>  qemu_put_byte(f, 0);
> -*bytes_transferred += 1;
> +rs->bytes_transferred += 1;
>  pages = 1;
>  }
>  
> @@ -745,11 +741,9 @@ static void ram_release_pages(MigrationState *ms, const 
> char *block_name,
>   * @block: block that contains the page we want to send
>   * @offset: offset inside the block for the page
>   * @last_stage: if we are at the completion stage
> - * @bytes_transferred: increase it with the number of transferred bytes
>   */
>  static int ram_save_page(RAMState *rs, MigrationState *ms, QEMUFile *f,
> - PageSearchStatus *pss, bool last_stage,
> - uint64_t *bytes_transferred)
> + PageSearchStatus *pss, bool last_stage)
>  {
>  int pages = -1;
>  uint64_t bytes_xmit;
> @@ -767,7 +761,7 @@ static int ram_save_page(RAMState *rs, MigrationState 
> *ms, QEMUFile *f,
>  ret = ram_control_save_page(f, block->offset,
> offset, TARGET_PAGE_SIZE, &bytes_xmit);
>  if (bytes_xmit) {
> -*bytes_transferred += bytes_xmit;
> +rs->bytes_transferred += bytes_xmit;
>  pages = 1;
>  }
>  
> @@ -787,7 +781,7 @@ static int ram_save_page(RAMState *rs, MigrationState 
> *ms, QEMUFile *f,
>  }
>  }
>  } else {
> -pages = save_zero_page(rs, f, block, offset, p, bytes_transferred);
> +pages = save_zero_page(rs, f, block, offset, p);
>  if (pages > 0) {
>  /* Must let xbzrle know, otherwise a previous (now 0'd) cached
>   * page would be stale
> @@ -797,7 +791,7 @@ static int ram_save_page(RAMState *rs, MigrationState 
> *ms, QEMUFile *f,
>  } else if (!rs->ram_bulk_stage &&
> !migration_in_postcopy(ms) && migrate_use_xbzrle()) {
>  pages = save_xbzrle_page(f, rs, &p, current_addr, block,
> - offset, last_stage, bytes_transferred);
> + offset, last_stage);
>  if (!last_stage) {
>  /* Can't send this cached data async, since the cache page
>   * might get updated before it gets to the wire
> @@ -809,7 +803,7 @@ static int ram_save_page(RAMState *rs, MigrationState 
> *ms, QEMUFile *f,
>  
>  /* XBZRLE overflow or normal page */
>  if (pages == -1) {
> -*bytes_transferred += save_page_header(f, block,
> +rs->byt

Re: [Qemu-devel] Proposal for deprecating unsupported host OSes & architecutures

2017-03-17 Thread Peter Maydell
On 16 March 2017 at 18:59, Dr. David Alan Gilbert  wrote:
> I build-test the FreeBSD when I do migration pulls; given it's
> just a VM it's not too hard; my main reason is that I use it as
> a proxy that gives it a good chance to get past your MacOS build.

Yeah, I have no in principle objection to using a VM here. It's
just that the last couple of times I've tried it I've messed
around for an hour or so and failed to get a VM setup that
Just Works (ie serial console without graphics window, network
setup that doesn't expose the VM to the external network but still
lets me ssh into it, etc). If we don't have an externally
maintained machine we can use then I think good wiki documentation
on how to set up the VM locally is a minimum.

thanks
-- PMM



Re: [Qemu-devel] Proposal for deprecating unsupported host OSes & architecutures

2017-03-17 Thread Daniel P. Berrange
On Fri, Mar 17, 2017 at 10:09:51AM +0100, Thomas Huth wrote:
> On 16.03.2017 17:52, Paolo Bonzini wrote:
> > 
> > 
> > On 16/03/2017 16:55, Peter Maydell wrote:
> >>> IOW, I think there is a reasonable 3 tier set here
> >>>
> >>>  1. Stuff we actively test builds & thus guarantee will work for
> >>> any QEMU release going forward.
> >>>
> >>>  2. Stuff we don't actively test, but generally assume is mostly
> >>> working, and likely to be fixed if & when problems are found
> >>>
> >>>  3. Stuff we don't actively test,  assume is probably broken
> >>> and unlikely to be fixed if reported
> >>>
> >>> Stuff in tier 3 should be candidate for deletion. Stuff in tier
> >>> 2 shouldn't be removed, but it might drop into tier 3 at some
> >>> point if people stop caring about fixing problems when found.
> >>> Conversely tier 2 might rise to tier 1 if CI turns up.
> >>
> >> I don't really want a tier 2. Either we support it enough
> >> to at least be able to run "make && make check" on some
> >> representative system, or we don't support it at all.
> >> Code which we have but are really reluctant to touch because
> >> we don't even test it builds (like bsd-user/) is really bad
> >> for preventing cleanups.
> > 
> > I think we should further differentiate between bsd-user/ and softmmu.
> > System emulation is just another program where we mostly compile to C
> > standard + POSIX or C standard + Win32.  There are certainly places
> > where we use Linux-specific extensions but it's not that special.
> > Neither BSD nor Solaris are particularly hard to support there.
> > 
> > On the other hand, bsd-user is extremely BSD specific, and ought to have
> > CI.  I think there should be a tier 2 for system emulation (which
> > doesn't mean that anything there shouldn't be moved to tier 3 and
> > eventually removed), but there shouldn't be a tier 2 for user-mode
> > emulation.
> > 
> > In particular, I believe that we should remove bsd-user from 2.10 unless
> > the downstream BSD port is merged back (and CI is provided).  There is
> > no point in keeping the current half-baked code without thread support.
> 
> I think you made a good point here.
> So "+1" from my side to remove "bsd-user" and "tcg/ia64" in QEMU 2.10
> or 2.11 (unless someone speaks up and provides maintainence, of course).

In the mail thread two months back Sean Bruno did suggest he might like
to just start over with bsd-user:

  https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00171.html

So perhaps someone should just ping him to see if he objects to us
deleting bsd-usr now (on off chance he's got patches nearly ready to
fix it)

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] vnc: fix a qio-channel leak

2017-03-17 Thread Daniel P. Berrange
On Fri, Mar 17, 2017 at 01:28:02PM +0400, Marc-André Lureau wrote:
> Spotted by ASAN.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  ui/vnc.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Daniel P. Berrange 

> diff --git a/ui/vnc.c b/ui/vnc.c
> index 8bfb1e0685..6e93b883b5 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3677,6 +3677,7 @@ static int vnc_display_listen_addr(VncDisplay *vd,
>  qio_channel_set_name(QIO_CHANNEL(sioc), name);
>  if (qio_channel_socket_listen_sync(
>  sioc, rawaddrs[i], listenerr == NULL ? &listenerr : NULL) < 
> 0) {
> +object_unref(OBJECT(sioc));
>  continue;
>  }
>  listening = true;

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 27/31] ram: Move last_req_rb to RAMState

2017-03-17 Thread Dr. David Alan Gilbert
* Juan Quintela (quint...@redhat.com) wrote:
> It was on MigrationState when it is only used inside ram.c for
> postcopy.  Problem is that we need to access it without being able to
> pass it RAMState directly.
> 
> Signed-off-by: Juan Quintela 
> ---
>  include/migration/migration.h | 2 --
>  migration/migration.c | 1 -
>  migration/ram.c   | 6 --
>  3 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 84cef4b..e032fb0 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -189,8 +189,6 @@ struct MigrationState
>  /* Queue of outstanding page requests from the destination */
>  QemuMutex src_page_req_mutex;
>  QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) 
> src_page_requests;
> -/* The RAMBlock used in the last src_page_request */
> -RAMBlock *last_req_rb;

Should this be kept together with src_page_req_mutex and src_page_requests?

Dave

>  /* The semaphore is used to notify COLO thread that failover is finished 
> */
>  QemuSemaphore colo_exit_sem;
>  
> diff --git a/migration/migration.c b/migration/migration.c
> index 46645b6..4f19382 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1114,7 +1114,6 @@ MigrationState *migrate_init(const MigrationParams 
> *params)
>  s->postcopy_after_devices = false;
>  s->postcopy_requests = 0;
>  s->migration_thread_running = false;
> -s->last_req_rb = NULL;
>  error_free(s->error);
>  s->error = NULL;
>  
> diff --git a/migration/ram.c b/migration/ram.c
> index e7db39c..50ca1da 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -197,6 +197,8 @@ struct RAMState {
>  QemuMutex bitmap_mutex;
>  /* Ram Bitmap protected by RCU */
>  RAMBitmap *ram_bitmap;
> +/* The RAMBlock used in the last src_page_request */
> +RAMBlock *last_req_rb;
>  };
>  typedef struct RAMState RAMState;
>  
> @@ -1190,7 +1192,7 @@ int ram_save_queue_pages(MigrationState *ms, const char 
> *rbname,
>  rcu_read_lock();
>  if (!rbname) {
>  /* Reuse last RAMBlock */
> -ramblock = ms->last_req_rb;
> +ramblock = ram_state.last_req_rb;
>  
>  if (!ramblock) {
>  /*
> @@ -1208,7 +1210,7 @@ int ram_save_queue_pages(MigrationState *ms, const char 
> *rbname,
>  error_report("ram_save_queue_pages no block '%s'", rbname);
>  goto err;
>  }
> -ms->last_req_rb = ramblock;
> +ram_state.last_req_rb = ramblock;
>  }
>  trace_ram_save_queue_pages(ramblock->idstr, start, len);
>  if (start+len > ramblock->used_length) {
> -- 
> 2.9.3
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] Proposal for deprecating unsupported host OSes & architecutures

2017-03-17 Thread Peter Maydell
On 17 March 2017 at 10:12, Daniel P. Berrange  wrote:
> In the mail thread two months back Sean Bruno did suggest he might like
> to just start over with bsd-user:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00171.html
>
> So perhaps someone should just ping him to see if he objects to us
> deleting bsd-usr now (on off chance he's got patches nearly ready to
> fix it)

It's because Sean is (on-and-off) still around that I'm not
very inclined to delete bsd-user. We actually have somebody
who cares about the code, which means we should move it into
tier 1, not dump it into tier 3 and delete.

thanks
-- PMM



[Qemu-devel] [Bug 1672365] Re: nested 9pfs read fail

2017-03-17 Thread Leo Gaspard
Hmm, actually it looks like a kernel in kernel panic always takes 100%
CPU (just got another unrelated one), so I guess it's not necessarily a
qemu bug but can arise from anywhere in the stack. I'm marking the bug
as invalid as a consequence.

** Changed in: qemu
   Status: New => Invalid

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

Title:
  nested 9pfs read fail

Status in QEMU:
  Invalid

Bug description:
  tl;dr: A virtfs read fails. The init being on this virtfs (mounted by
  the initrd), the linux kernel guest is unable to boot, and kernel
  panics. The fact that qemu still takes 100%cpu after the kernel panic
  makes me think it's a qemu bug.

  Here is the setup (some hashes replaced with "..."):
   * A (NixOS) host system, with /nix/store as a btrfs on lvm on cryptsetup
   * Running a qemu-kvm NixOS guest, with /nix/.ro-store as a virtfs mapping to 
host /nix/store:
  ```
  exec /nix/store/...-qemu-x86-only-for-vm-tests-2.8.0/bin/qemu-kvm \
  -name test \
  -m 8192 \
  -cpu kvm64 \
  -net nic,vlan=0,model=virtio -net user,vlan=0 \
  -virtfs local,path=/nix/store,security_model=none,mount_tag=store \
  -virtfs 
local,path=/tmp/nix-vm/xchg,security_model=none,mount_tag=xchg \
  -virtfs 
local,path=/tmp/nix-vm/xchg,security_model=none,mount_tag=shared \
  -drive 
index=0,id=drive1,file=/home/ekleog/nixos/test.qcow2,if=virtio,cache=writeback,werror=report
 \
  -kernel /nix/store/...-nixos-system-test-17.09.git.deee8da/kernel \
  -initrd /nix/store/...-nixos-system-test-17.09.git.deee8da/initrd \
  -append "$(cat 
/nix/store/...-nixos-system-test-17.09.git.deee8da/kernel-params) 
init=/nix/store/...-nixos-system-test-17.09.git.deee8da/init 
regInfo=/nix/store/...-reginfo" \
  -vga std -usbdevice tablet
  ```
   * With /nix/store as an overlayfs between /nix/.ro-store and /nix/.rw-store
   * Running a qemu-kvm NixOS guest, with /nix/.ro-store as a virtfs mapping to 
host /nix/store/...-vm-nginx-store:
  ```
  /nix/store/...-qemu-2.8.0/bin/qemu-kvm \
-name nginx -m 128 -smp 2 -cpu kvm64 \
-nographic -serial 
unix:"/var/lib/vm/consoles/nginx/socket.unix",server,nowait \
-drive file="/var/lib/vm/images/nginx.img",if=virtio,media=disk \
-virtfs 
local,path="/nix/store/...-vm-nginx-store",security_model=none,mount_tag=store \
-netdev type=tap,id=net0,ifname=vm-nginx,script=no,dscript=no -device 
virtio-net-pci,netdev=net0,mac=56:00:00:00:00:01 \
-kernel /nix/store/...-nixos-system-nginx-17.09.git.deee8da/kernel \
-initrd /nix/store/...-nixos-system-nginx-17.09.git.deee8da/initrd \
-append "$(cat 
/nix/store/...-nixos-system-nginx-17.09.git.deee8da/kernel-params) 
init=/nix/store/...-nixos-system-nginx-17.09.git.deee8da/init console=ttyS0 
boot.shell_on_fail" \
-virtfs 
local,path="/var/lib/vm/persist/nginx/home",security_model=mapped-xattr,mount_tag="shared1"
 \
-virtfs 
local,path="/var/lib",security_model=mapped-xattr,mount_tag="shared2" \
-virtfs local,path="/tmp",security_model=mapped-xattr,mount_tag="shared3"
  ```
   * With /nix/store as an overlayfs between /nix/.ro-store and /nix/.rw-store
   * With init in /nix/store

  What happens is that at boot time the inner VM doesn't manage to read
  the init script after the initrd has mounted the 9pfs and overlayfs.

  What makes me think it's a qemu bug is that htop in the outer VM shows
  the inner VM's qemu as using 100% cpu, despite the fact the kernel
  it's running is under kernel panic, thus doing nothing.

  What do you think about this?

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



Re: [Qemu-devel] [PATCH 29/31] ram: Remove dirty_bytes_rate

2017-03-17 Thread Dr. David Alan Gilbert
* Juan Quintela (quint...@redhat.com) wrote:
> It can be recalculated from dirty_pages_rate.
> 
> Signed-off-by: Juan Quintela 
> ---
>  include/migration/migration.h | 1 -
>  migration/migration.c | 5 ++---
>  migration/ram.c   | 1 -
>  3 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 54a1a4f..42b9edf 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -167,7 +167,6 @@ struct MigrationState
>  int64_t downtime;
>  int64_t expected_downtime;
>  int64_t dirty_pages_rate;
> -int64_t dirty_bytes_rate;
>  bool enabled_capabilities[MIGRATION_CAPABILITY__MAX];
>  int64_t xbzrle_cache_size;
>  int64_t setup_time;
> diff --git a/migration/migration.c b/migration/migration.c
> index 09d02be..2f8c440 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1107,7 +1107,6 @@ MigrationState *migrate_init(const MigrationParams 
> *params)
>  s->downtime = 0;
>  s->expected_downtime = 0;
>  s->dirty_pages_rate = 0;
> -s->dirty_bytes_rate = 0;
>  s->setup_time = 0;
>  s->start_postcopy = false;
>  s->postcopy_after_devices = false;
> @@ -1999,8 +1998,8 @@ static void *migration_thread(void *opaque)
>bandwidth, max_size);
>  /* if we haven't sent anything, we don't want to recalculate
> 1 is a small enough number for our purposes */
> -if (s->dirty_bytes_rate && transferred_bytes > 1) {
> -s->expected_downtime = s->dirty_bytes_rate / bandwidth;
> +if (s->dirty_pages_rate && transferred_bytes > 1) {
> +s->expected_downtime = s->dirty_pages_rate * (1ul << 
> qemu_target_page_bits())/ bandwidth;

The line got a bit long, please wrap.

Other than that,

Reviewed-by: Dr. David Alan Gilbert 

>  }
>  
>  qemu_file_reset_rate_limit(s->to_dst_file);
> diff --git a/migration/ram.c b/migration/ram.c
> index 4563e3d..1006e60 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -687,7 +687,6 @@ static void migration_bitmap_sync(RAMState *rs)
>  }
>  s->dirty_pages_rate = rs->num_dirty_pages_period * 1000
>  / (end_time - rs->start_time);
> -s->dirty_bytes_rate = s->dirty_pages_rate * TARGET_PAGE_SIZE;
>  rs->start_time = end_time;
>  rs->num_dirty_pages_period = 0;
>  }
> -- 
> 2.9.3
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH for-2.9] block: Propagate error in bdrv_open_backing_file

2017-03-17 Thread Kevin Wolf
Am 17.03.2017 um 02:56 hat Fam Zheng geschrieben:
> Signed-off-by: Fam Zheng 

Thanks, applied to the block branch.

Kevin



Re: [Qemu-devel] Proposal for deprecating unsupported host OSes & architecutures

2017-03-17 Thread Thomas Huth
On 17.03.2017 11:15, Peter Maydell wrote:
> On 17 March 2017 at 10:12, Daniel P. Berrange  wrote:
>> In the mail thread two months back Sean Bruno did suggest he might like
>> to just start over with bsd-user:
>>
>>   https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00171.html
>>
>> So perhaps someone should just ping him to see if he objects to us
>> deleting bsd-usr now (on off chance he's got patches nearly ready to
>> fix it)
> 
> It's because Sean is (on-and-off) still around that I'm not
> very inclined to delete bsd-user. We actually have somebody
> who cares about the code, which means we should move it into
> tier 1, not dump it into tier 3 and delete.

OK, fair, but even Sean was suggesting to purge the old code first ...
so maybe we should just do so and start with a new bsd-user folder when
he is ready?

 Thomas




[Qemu-devel] [PATCH for-2.9] configure: Warn that the support for ia64 is going to be removed

2017-03-17 Thread Thomas Huth
Access to an ia64 machine is hard to get these days, and we lack a
machine for continuous integration testing, so it is very likely
that we remove support for the ia64 TCG backend soon.

Signed-off-by: Thomas Huth 
---
 configure | 5 +
 1 file changed, 5 insertions(+)

diff --git a/configure b/configure
index 99d8bec..3ff8999 100755
--- a/configure
+++ b/configure
@@ -499,6 +499,11 @@ elif check_define __mips__ ; then
   cpu="mips"
 elif check_define __ia64__ ; then
   cpu="ia64"
+  echo "*** Note: The QEMU Project intends to drop support for ia64 in a near"
+  echo "*** future release since we do not have a machine for continuous"
+  echo "*** integration testing. Please get in touch via qemu-devel@nongnu.org"
+  echo "*** if you want to maintain support and can provide a test machine."
+  sleep 10
 elif check_define __s390__ ; then
   if check_define __s390x__ ; then
 cpu="s390x"
-- 
1.8.3.1




[Qemu-devel] [PATCH] checkpatch: allow longer lines for logging functions

2017-03-17 Thread Greg Kurz
Commit f1e155bbf863a removed a bunch of stuff that really don't make sense
outside the linux kernel. An exception though is logging functions: it is
convenient to be able to grep error messages in the code. For this to work,
error strings mustn't be broken down on multiple lines, and therefore are
likely to overcome the 90 characters limit, and make checkpatch unhappy.

This patch reverts the change for logging functions and adapts it to QEMU.

Signed-off-by: Greg Kurz 
---
 scripts/checkpatch.pl |8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f0845429342a..cc2796238170 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -192,6 +192,11 @@ our $typeTypedefs = qr{(?x:
 | QEMUBH# all uppercase
 )};
 
+our $logFunctions = qr{(?x:
+   error_report|
+   error_printf
+)};
+
 our @typeList = (
qr{void},
qr{(?:unsigned\s+)?char},
@@ -1341,7 +1346,8 @@ sub process {
 
 #90 column limit
if ($line =~ /^\+/ &&
-   !($line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) &&
+   !($line =~ 
/^\+\s*$logFunctions\s*\(\s*(?:([^"]*))?"[X\t]*"\s*(?:,|\)\s*;)\s*$/ ||
+ $line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) &&
$length > 80)
{
if ($length > 90) {




[Qemu-devel] [PATCH v2] 9pfs: don't try to flush self and avoid QEMU hang on reset

2017-03-17 Thread Greg Kurz
According to the 9P spec [*], when a client wants to cancel a pending I/O
request identified by a given tag (uint16), it must send a Tflush message
and wait for the server to respond with a Rflush message before reusing this
tag for another I/O. The server may still send a completion message for the
I/O if it wasn't actually cancelled but the Rflush message must arrive after
that.

QEMU hence waits for the flushed PDU to complete before sending the Rflush
message back to the client.

If a client sends 'Tflush tag oldtag' and tag == oldtag, QEMU will then
allocate a PDU identified by tag, find it in the PDU list and wait for
this same PDU to complete... i.e. wait for a completion that will never
happen. This causes a tag and ring slot leak in the guest, and a PDU
leak in QEMU, all of them limited by the maximal number of PDUs (128).
But, worse, this causes QEMU to hang on device reset since v9fs_reset()
wants to drain all pending I/O.

This insane behavior is likely to denote a bug in the client, and it would
deserve an Rerror message to be sent back. Unfortunately, the protocol
allows it and requires all flush requests to suceed (only a Tflush response
is expected).

The only option is to detect when we have to handle a self-referencing
flush request and report success to the client right away.

[*] http://man.cat-v.org/plan_9/5/flush

Reported-by: Al Viro 
Signed-off-by: Greg Kurz 
---
v2: print out a warning
---
 hw/9pfs/9p.c |   12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 76c9247c777d..b8c0b993580c 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -2353,7 +2353,7 @@ static void coroutine_fn v9fs_flush(void *opaque)
 ssize_t err;
 int16_t tag;
 size_t offset = 7;
-V9fsPDU *cancel_pdu;
+V9fsPDU *cancel_pdu = NULL;
 V9fsPDU *pdu = opaque;
 V9fsState *s = pdu->s;
 
@@ -2364,9 +2364,13 @@ static void coroutine_fn v9fs_flush(void *opaque)
 }
 trace_v9fs_flush(pdu->tag, pdu->id, tag);
 
-QLIST_FOREACH(cancel_pdu, &s->active_list, next) {
-if (cancel_pdu->tag == tag) {
-break;
+if (pdu->tag == tag) {
+error_report("Warning: the guest sent a self-referencing 9P flush 
request");
+} else {
+QLIST_FOREACH(cancel_pdu, &s->active_list, next) {
+if (cancel_pdu->tag == tag) {
+break;
+}
 }
 }
 if (cancel_pdu) {




Re: [Qemu-devel] [PATCH 30/31] ram: move dirty_pages_rate to RAMState

2017-03-17 Thread Dr. David Alan Gilbert
* Juan Quintela (quint...@redhat.com) wrote:
> Treat it like the rest of ram stats counters.  Export its value the
> same way.  As an added bonus, no more MigrationState used in
> migration_bitmap_sync();
> 
> Signed-off-by: Juan Quintela 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  include/migration/migration.h |  2 +-
>  migration/migration.c |  7 +++
>  migration/ram.c   | 12 +---
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 42b9edf..43bdf86 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -166,7 +166,6 @@ struct MigrationState
>  int64_t total_time;
>  int64_t downtime;
>  int64_t expected_downtime;
> -int64_t dirty_pages_rate;
>  bool enabled_capabilities[MIGRATION_CAPABILITY__MAX];
>  int64_t xbzrle_cache_size;
>  int64_t setup_time;
> @@ -269,6 +268,7 @@ uint64_t ram_bytes_remaining(void);
>  uint64_t ram_bytes_transferred(void);
>  uint64_t ram_bytes_total(void);
>  uint64_t ram_dirty_sync_count(void);
> +uint64_t ram_dirty_pages_rate(void);
>  void free_xbzrle_decoded_buf(void);
>  
>  void acct_update_position(QEMUFile *f, size_t size, bool zero);
> diff --git a/migration/migration.c b/migration/migration.c
> index 2f8c440..0a70d55 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -650,7 +650,7 @@ static void populate_ram_info(MigrationInfo *info, 
> MigrationState *s)
>  
>  if (s->state != MIGRATION_STATUS_COMPLETED) {
>  info->ram->remaining = ram_bytes_remaining();
> -info->ram->dirty_pages_rate = s->dirty_pages_rate;
> +info->ram->dirty_pages_rate = ram_dirty_pages_rate();
>  }
>  }
>  
> @@ -1106,7 +1106,6 @@ MigrationState *migrate_init(const MigrationParams 
> *params)
>  s->mbps = 0.0;
>  s->downtime = 0;
>  s->expected_downtime = 0;
> -s->dirty_pages_rate = 0;
>  s->setup_time = 0;
>  s->start_postcopy = false;
>  s->postcopy_after_devices = false;
> @@ -1998,8 +1997,8 @@ static void *migration_thread(void *opaque)
>bandwidth, max_size);
>  /* if we haven't sent anything, we don't want to recalculate
> 1 is a small enough number for our purposes */
> -if (s->dirty_pages_rate && transferred_bytes > 1) {
> -s->expected_downtime = s->dirty_pages_rate * (1ul << 
> qemu_target_page_bits())/ bandwidth;
> +if (ram_dirty_pages_rate() && transferred_bytes > 1) {
> +s->expected_downtime = ram_dirty_pages_rate() * (1ul << 
> qemu_target_page_bits())/ bandwidth;
>  }
>  
>  qemu_file_reset_rate_limit(s->to_dst_file);
> diff --git a/migration/ram.c b/migration/ram.c
> index 1006e60..b85f58f 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -193,6 +193,8 @@ struct RAMState {
>  uint64_t migration_dirty_pages;
>  /* total number of bytes transferred */
>  uint64_t bytes_transferred;
> +/* number of dirtied pages in the last second */
> +uint64_t dirty_pages_rate;
>  /* protects modification of the bitmap */
>  QemuMutex bitmap_mutex;
>  /* Ram Bitmap protected by RCU */
> @@ -254,6 +256,11 @@ uint64_t ram_dirty_sync_count(void)
>  return ram_state.bitmap_sync_count;
>  }
>  
> +uint64_t ram_dirty_pages_rate(void)
> +{
> +return ram_state.dirty_pages_rate;
> +}
> +
>  /* used by the search for pages to send */
>  struct PageSearchStatus {
>  /* Current block being searched */
> @@ -624,7 +631,6 @@ static void migration_bitmap_sync(RAMState *rs)
>  {
>  RAMBlock *block;
>  uint64_t num_dirty_pages_init = rs->migration_dirty_pages;
> -MigrationState *s = migrate_get_current();
>  int64_t end_time;
>  int64_t bytes_xfer_now;
>  
> @@ -664,7 +670,7 @@ static void migration_bitmap_sync(RAMState *rs)
> throttling */
>  bytes_xfer_now = ram_bytes_transferred();
>  
> -if (s->dirty_pages_rate &&
> +if (rs->dirty_pages_rate &&
> (rs->num_dirty_pages_period * TARGET_PAGE_SIZE >
> (bytes_xfer_now - rs->bytes_xfer_prev)/2) &&
> (rs->dirty_rate_high_cnt++ >= 2)) {
> @@ -685,7 +691,7 @@ static void migration_bitmap_sync(RAMState *rs)
>  rs->iterations_prev = rs->iterations;
>  rs->xbzrle_cache_miss_prev = rs->xbzrle_cache_miss;
>  }
> -s->dirty_pages_rate = rs->num_dirty_pages_period * 1000
> +rs->dirty_pages_rate = rs->num_dirty_pages_period * 1000
>  / (end_time - rs->start_time);
>  rs->start_time = end_time;
>  rs->num_dirty_pages_period = 0;
> -- 
> 2.9.3
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



[Qemu-devel] [PATCH v2 0/3] qemu-img: improve qemu-img getopt error messages

2017-03-17 Thread Stefan Hajnoczi
v2:
 * Print short help to avoid obscuring error messages [Max]

This series improves getopt error messages.  Unrecognized global options were
skipped rather than causing qemu-img to exit as expected.  Also avoid printing
the full help text because it obscures the actual error message.

Stefan Hajnoczi (3):
  qemu-img: show help for invalid global options
  qemu-img: fix switch indentation in img_amend()
  qemu-img: print short help on getopt failure

 qemu-img.c | 196 ++---
 1 file changed, 137 insertions(+), 59 deletions(-)

-- 
2.9.3




[Qemu-devel] [PATCH v2 2/3] qemu-img: fix switch indentation in img_amend()

2017-03-17 Thread Stefan Hajnoczi
QEMU coding style indents 'case' to the same level as the 'switch'
statement:

  switch (foo) {
  case 1:

Fix this coding style violation so checkpatch.pl doesn't complain about
the next patch.

Signed-off-by: Stefan Hajnoczi 
---
 qemu-img.c | 82 +++---
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ce293a4..c7ffabb 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3500,47 +3500,47 @@ static int img_amend(int argc, char **argv)
 }
 
 switch (c) {
-case 'h':
-case '?':
-help();
-break;
-case 'o':
-if (!is_valid_option_list(optarg)) {
-error_report("Invalid option list: %s", optarg);
-ret = -1;
-goto out_no_progress;
-}
-if (!options) {
-options = g_strdup(optarg);
-} else {
-char *old_options = options;
-options = g_strdup_printf("%s,%s", options, optarg);
-g_free(old_options);
-}
-break;
-case 'f':
-fmt = optarg;
-break;
-case 't':
-cache = optarg;
-break;
-case 'p':
-progress = true;
-break;
-case 'q':
-quiet = true;
-break;
-case OPTION_OBJECT:
-opts = qemu_opts_parse_noisily(&qemu_object_opts,
-   optarg, true);
-if (!opts) {
-ret = -1;
-goto out_no_progress;
-}
-break;
-case OPTION_IMAGE_OPTS:
-image_opts = true;
-break;
+case 'h':
+case '?':
+help();
+break;
+case 'o':
+if (!is_valid_option_list(optarg)) {
+error_report("Invalid option list: %s", optarg);
+ret = -1;
+goto out_no_progress;
+}
+if (!options) {
+options = g_strdup(optarg);
+} else {
+char *old_options = options;
+options = g_strdup_printf("%s,%s", options, optarg);
+g_free(old_options);
+}
+break;
+case 'f':
+fmt = optarg;
+break;
+case 't':
+cache = optarg;
+break;
+case 'p':
+progress = true;
+break;
+case 'q':
+quiet = true;
+break;
+case OPTION_OBJECT:
+opts = qemu_opts_parse_noisily(&qemu_object_opts,
+   optarg, true);
+if (!opts) {
+ret = -1;
+goto out_no_progress;
+}
+break;
+case OPTION_IMAGE_OPTS:
+image_opts = true;
+break;
 }
 }
 
-- 
2.9.3




[Qemu-devel] [PATCH v2 1/3] qemu-img: show help for invalid global options

2017-03-17 Thread Stefan Hajnoczi
The qemu-img sub-command executes regardless of invalid global options:

  $ qemu-img --foo info test.img
  qemu-img: unrecognized option '--foo'
  image: test.img
  ...

The unrecognized option warning may be missed by the user.  This can
hide incorrect command-lines in scripts and confuse users.

This patch prints the help information and terminates instead of
executing the sub-command.

Signed-off-by: Stefan Hajnoczi 
---
 qemu-img.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qemu-img.c b/qemu-img.c
index 98b836b..ce293a4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4339,6 +4339,7 @@ int main(int argc, char **argv)
 while ((c = getopt_long(argc, argv, "+hVT:", long_options, NULL)) != -1) {
 switch (c) {
 case 'h':
+case '?':
 help();
 return 0;
 case 'V':
-- 
2.9.3




[Qemu-devel] callout to *file in bdrv_co_get_block_status

2017-03-17 Thread Peter Lieven
Hi,


I tried to debug why qemu-img convert with a VMDK source laying on a tmpfs is 
horrible slow.

For some reason a lseek on a tmpfs is slow. Strictly speaking the lseek in 
find_allocation in file-posix.c

is slow.


When qemu-img convert iterates over all sectors of a VMDK file to check their 
allocation status it ends

up checking allocation status of all allocated sectors due to the following 
condition in bdrv_co_get_block_status:


if (*file && *file != bs &&
(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
(ret & BDRV_BLOCK_OFFSET_VALID)) {
BlockDriverState *file2;
int file_pnum;
ret2 = bdrv_co_get_block_status(*file, ret >> BDRV_SECTOR_BITS,
*pnum, &file_pnum, &file2);
if (ret2 >= 0) {
/* Ignore errors.  This is just providing extra information, it
 * is useful but not necessary.
 */
if (!file_pnum) {
/* !file_pnum indicates an offset at or beyond the EOF; it is
 * perfectly valid for the format block driver to point to such
 * offsets, so catch it and mark everything as zero */
ret |= BDRV_BLOCK_ZERO;
} else {
/* Limit request to the range reported by the protocol driver */
*pnum = file_pnum;
ret |= (ret2 & BDRV_BLOCK_ZERO);
}
}
}


Does anybody remember for what circumstances this case this was added? In case 
of an container format

like VMDK or QCOW2 shouldn't we trust the information from the l2 tables in the 
VMDK or QCOW2?


Thanks,

Peter




[Qemu-devel] [PATCH v2 3/3] qemu-img: print short help on getopt failure

2017-03-17 Thread Stefan Hajnoczi
Printing the full help output obscures the error message for an invalid
command-line option or missing argument.

Before this patch:

  $ ./qemu-img --foo
  ...pages of output...

After this patch:

  $ ./qemu-img --foo
  qemu-img: unrecognized option '--foo'
  Try 'qemu-img --help' for more information

This patch adds the getopt ':' character so that it can distinguish
between missing arguments and unrecognized options.  This helps provide
more detailed error messages.

Suggested-by: Max Reitz 
Signed-off-by: Stefan Hajnoczi 
---
 qemu-img.c | 117 ++---
 1 file changed, 97 insertions(+), 20 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index c7ffabb..b220cf7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -88,6 +88,16 @@ static void QEMU_NORETURN GCC_FMT_ATTR(1, 2) 
error_exit(const char *fmt, ...)
 exit(EXIT_FAILURE);
 }
 
+static void QEMU_NORETURN missing_argument(const char *option)
+{
+error_exit("missing argument for option '%s'", option);
+}
+
+static void QEMU_NORETURN unrecognized_option(const char *option)
+{
+error_exit("unrecognized option '%s'", option);
+}
+
 /* Please keep in synch with qemu-img.texi */
 static void QEMU_NORETURN help(void)
 {
@@ -406,13 +416,18 @@ static int img_create(int argc, char **argv)
 {"object", required_argument, 0, OPTION_OBJECT},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, "F:b:f:he6o:q",
+c = getopt_long(argc, argv, ":F:b:f:he6o:q",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 switch(c) {
+case ':':
+missing_argument(argv[optind - 1]);
+break;
 case '?':
+unrecognized_option(argv[optind - 1]);
+break;
 case 'h':
 help();
 break;
@@ -651,13 +666,18 @@ static int img_check(int argc, char **argv)
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, "hf:r:T:q",
+c = getopt_long(argc, argv, ":hf:r:T:q",
 long_options, &option_index);
 if (c == -1) {
 break;
 }
 switch(c) {
+case ':':
+missing_argument(argv[optind - 1]);
+break;
 case '?':
+unrecognized_option(argv[optind - 1]);
+break;
 case 'h':
 help();
 break;
@@ -855,13 +875,18 @@ static int img_commit(int argc, char **argv)
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, "f:ht:b:dpq",
+c = getopt_long(argc, argv, ":f:ht:b:dpq",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 switch(c) {
+case ':':
+missing_argument(argv[optind - 1]);
+break;
 case '?':
+unrecognized_option(argv[optind - 1]);
+break;
 case 'h':
 help();
 break;
@@ -1190,13 +1215,18 @@ static int img_compare(int argc, char **argv)
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, "hf:F:T:pqs",
+c = getopt_long(argc, argv, ":hf:F:T:pqs",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 switch (c) {
+case ':':
+missing_argument(argv[optind - 1]);
+break;
 case '?':
+unrecognized_option(argv[optind - 1]);
+break;
 case 'h':
 help();
 break;
@@ -1926,13 +1956,18 @@ static int img_convert(int argc, char **argv)
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, "hf:O:B:ce6o:s:l:S:pt:T:qnm:W",
+c = getopt_long(argc, argv, ":hf:O:B:ce6o:s:l:S:pt:T:qnm:W",
 long_options, NULL);
 if (c == -1) {
 break;
 }
 switch(c) {
+case ':':
+missing_argument(argv[optind - 1]);
+break;
 case '?':
+unrecognized_option(argv[optind - 1]);
+break;
 case 'h':
 help();
 break;
@@ -2502,13 +2537,18 @@ static int img_info(int argc, char **argv)
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {0, 0, 0, 0}
 };
-c = getopt_long(argc, argv, "f:h",
+c = getopt_long(argc, argv, ":f:h",
 long_options, &option_index);
 if (c == -1) {
 break;
 }
 switch(c) {
+case ':':
+missing_argument(argv[optind - 1]);
+break;
 case '?':
+unrecognized_option(argv[opt

Re: [Qemu-devel] [PATCH] checkpatch: allow longer lines for logging functions

2017-03-17 Thread no-reply
Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 148974701986.29545.5414999102981738774.stgit@bahia
Subject: [Qemu-devel] [PATCH] checkpatch: allow longer lines for logging 
functions
Type: series

=== 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/148974701986.29545.5414999102981738774.stgit@bahia 
-> patchew/148974701986.29545.5414999102981738774.stgit@bahia
 * [new tag] 
patchew/148974744281.30636.17973264008285415592.stgit@bahia -> 
patchew/148974744281.30636.17973264008285415592.stgit@bahia
Switched to a new branch 'test'
390d7c4 checkpatch: allow longer lines for logging functions

=== OUTPUT BEGIN ===
Checking PATCH 1/1: checkpatch: allow longer lines for logging functions...
ERROR: line over 90 characters
#38: FILE: scripts/checkpatch.pl:1349:
+   !($line =~ 
/^\+\s*$logFunctions\s*\(\s*(?:([^"]*))?"[X\t]*"\s*(?:,|\)\s*;)\s*$/ ||

total: 1 errors, 0 warnings, 20 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.

=== 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

Re: [Qemu-devel] Proposal for deprecating unsupported host OSes & architecutures

2017-03-17 Thread Peter Maydell
On 17 March 2017 at 10:30, Thomas Huth  wrote:
> On 17.03.2017 11:15, Peter Maydell wrote:
>> On 17 March 2017 at 10:12, Daniel P. Berrange  wrote:
>>> In the mail thread two months back Sean Bruno did suggest he might like
>>> to just start over with bsd-user:
>>>
>>>   https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00171.html
>>>
>>> So perhaps someone should just ping him to see if he objects to us
>>> deleting bsd-usr now (on off chance he's got patches nearly ready to
>>> fix it)
>>
>> It's because Sean is (on-and-off) still around that I'm not
>> very inclined to delete bsd-user. We actually have somebody
>> who cares about the code, which means we should move it into
>> tier 1, not dump it into tier 3 and delete.
>
> OK, fair, but even Sean was suggesting to purge the old code first ...
> so maybe we should just do so and start with a new bsd-user folder when
> he is ready?

That's not so great for the other BSDs, and I'm not convinced that
"here's a huge pile of code" is going to be easier to review than
the incremental changes.

(It's the "what's easiest to review" part I think is most important.)

thanks
-- PMM



Re: [Qemu-devel] [PATCH for-2.9] configure: Warn that the support for ia64 is going to be removed

2017-03-17 Thread Peter Maydell
On 17 March 2017 at 10:34, Thomas Huth  wrote:
> Access to an ia64 machine is hard to get these days, and we lack a
> machine for continuous integration testing, so it is very likely
> that we remove support for the ia64 TCG backend soon.
>
> Signed-off-by: Thomas Huth 
> ---
>  configure | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/configure b/configure
> index 99d8bec..3ff8999 100755
> --- a/configure
> +++ b/configure
> @@ -499,6 +499,11 @@ elif check_define __mips__ ; then
>cpu="mips"
>  elif check_define __ia64__ ; then
>cpu="ia64"
> +  echo "*** Note: The QEMU Project intends to drop support for ia64 in a 
> near"
> +  echo "*** future release since we do not have a machine for continuous"
> +  echo "*** integration testing. Please get in touch via 
> qemu-devel@nongnu.org"
> +  echo "*** if you want to maintain support and can provide a test machine."
> +  sleep 10
>  elif check_define __s390__ ; then
>if check_define __s390x__ ; then
>  cpu="s390x"
> --
> 1.8.3.1

I have a more general patch for this that supports marking several
host CPUs and OSes as deprecated. I just haven't posted it yet
because I'm not sure which ones we want to mark as deprecated.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v2] qemu-ga: obey LISTEN_PID when using systemd socket activation

2017-03-17 Thread Paolo Bonzini


On 17/03/2017 10:41, Stefan Hajnoczi wrote:
>> Luckily, qemu-nbd also got socket activation code, and its copy does
>> support LISTEN_PID.  Some extra fixups are needed to ensure that the
>> code can be used for both, but that's what this patch does.  The
>> main change is to replace get_listen_fds's "consume" argument with
>> the FIRST_SOCKET_ACTIVATION_FD macro from the qemu-nbd code.
> 
> I intentionally wrote qga socket activation this way.  It allows socket
> activation to work together with daemonization.  That combination is
> probably not very useful so it's fine to get rid of it.

This works with my patch (and I've tested it now), because it looks up
LISTEN_FDS only once, way before daemonization.

The run_agent function grew a parameter that tells it to use
FIRST_SOCKET_ACTIVATION_FD as the file descriptor, without requiring
another look at the environment.  (And because daemonization forks but
doesn't exec, setting the cloexec flag doesn't get in the way either).

Thanks,

Paolo

> Please add an error message in qga/main.c if socket activation is used
> in combination with the -d/--daemonize flag.



Re: [Qemu-devel] callout to *file in bdrv_co_get_block_status

2017-03-17 Thread Paolo Bonzini


On 17/03/2017 11:45, Peter Lieven wrote:
> Hi,
> 
> 
> I tried to debug why qemu-img convert with a VMDK source laying on a tmpfs is 
> horrible slow.
> 
> For some reason a lseek on a tmpfs is slow. Strictly speaking the lseek in 
> find_allocation in file-posix.c
> 
> is slow.
> 
> 
> When qemu-img convert iterates over all sectors of a VMDK file to check their 
> allocation status it ends
> 
> up checking allocation status of all allocated sectors due to the following 
> condition in bdrv_co_get_block_status:
> 
> 
> if (*file && *file != bs &&
> (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
> (ret & BDRV_BLOCK_OFFSET_VALID)) {
> BlockDriverState *file2;
> int file_pnum;
> ret2 = bdrv_co_get_block_status(*file, ret >> BDRV_SECTOR_BITS,
> *pnum, &file_pnum, &file2);
> if (ret2 >= 0) {
> /* Ignore errors.  This is just providing extra information, it
>  * is useful but not necessary.
>  */
> if (!file_pnum) {
> /* !file_pnum indicates an offset at or beyond the EOF; it is
>  * perfectly valid for the format block driver to point to 
> such
>  * offsets, so catch it and mark everything as zero */
> ret |= BDRV_BLOCK_ZERO;
> } else {
> /* Limit request to the range reported by the protocol driver 
> */
> *pnum = file_pnum;
> ret |= (ret2 & BDRV_BLOCK_ZERO);
> }
> }
> }
> 
> 
> Does anybody remember for what circumstances this case this was added? In 
> case of an container format
> 
> like VMDK or QCOW2 shouldn't we trust the information from the l2 tables in 
> the VMDK or QCOW2?

It provides additional information, for example it works better with
prealloc=metadata.

Paolo



[Qemu-devel] [PATCH] configure: Warn about deprecated hosts

2017-03-17 Thread Peter Maydell
We plan to drop support in a future QEMU release for host OSes
and host architectures for which we have no test machine where
we can build and run tests. For the 2.9 release, make configure
print a warning if it is run on such a host, so that the user
has some warning of the plans and can volunteer to help us
maintain the port if they need it to continue to function.

This commit flags up as deprecated the CPU architectures:
 * ia64
 * sparc
 * anything which we don't have a TCG port for
   (and which was presumably using TCI)
and the OSes:
 * Cygwin
 * GNU/kFreeBSD
 * FreeBSD
 * DragonFly BSD
 * NetBSD
 * OpenBSD
 * Solaris
 * AIX
 * Haiku

It also makes entirely unrecognized host OS strings be
rejected rather than treated as if they were Linux (which
likely never worked).

Signed-off-by: Peter Maydell 
---
This list is definitely too all-encompassing, and we should
move at least some of the BSDs into "not-deprecated".
I'm posting the patch for the moment for code review on the
logic and as a placeholder.
---
 configure | 48 ++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 75c7c35..5ddc985 100755
--- a/configure
+++ b/configure
@@ -321,6 +321,9 @@ tcmalloc="no"
 jemalloc="no"
 replication="yes"
 
+supported_cpu="no"
+supported_os="no"
+
 # parse CC options first
 for opt do
   optarg=$(expr "x$opt" : 'x[^=]*=\(.*\)')
@@ -517,23 +520,32 @@ ARCH=
 # Normalise host CPU name and set ARCH.
 # Note that this case should only have supported host CPUs, not guests.
 case "$cpu" in
-  ia64|ppc|ppc64|s390|s390x|sparc64|x32)
+  ppc|ppc64|s390|s390x|x32)
+cpu="$cpu"
+supported_cpu="yes"
+  ;;
+  ia64|sparc64)
 cpu="$cpu"
   ;;
   i386|i486|i586|i686|i86pc|BePC)
 cpu="i386"
+supported_cpu="yes"
   ;;
   x86_64|amd64)
 cpu="x86_64"
+supported_cpu="yes"
   ;;
   armv*b|armv*l|arm)
 cpu="arm"
+supported_cpu="yes"
   ;;
   aarch64)
 cpu="aarch64"
+supported_cpu="yes"
   ;;
   mips*)
 cpu="mips"
+supported_cpu="yes"
   ;;
   sparc|sun4[cdmuv])
 cpu="sparc"
@@ -568,6 +580,7 @@ MINGW32*)
   else
 audio_drv_list=""
   fi
+  supported_os="yes"
 ;;
 GNU/kFreeBSD)
   bsd="yes"
@@ -626,6 +639,7 @@ Darwin)
   # won't work when we're compiling with gcc as a C compiler.
   QEMU_CFLAGS="-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
   HOST_VARIANT_DIR="darwin"
+  supported_os="yes"
 ;;
 SunOS)
   solaris="yes"
@@ -672,7 +686,7 @@ Haiku)
   QEMU_CFLAGS="-DB_USE_POSITIVE_POSIX_ERRORS $QEMU_CFLAGS"
   LIBS="-lposix_error_mapper -lnetwork $LIBS"
 ;;
-*)
+Linux)
   audio_drv_list="oss"
   audio_possible_drivers="oss alsa sdl pa"
   linux="yes"
@@ -682,6 +696,10 @@ Haiku)
   vhost_scsi="yes"
   vhost_vsock="yes"
   QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers 
$QEMU_INCLUDES"
+  supported_os="yes"
+;;
+*)
+  error_exit "Unsupported host OS $targetos"
 ;;
 esac
 
@@ -5095,6 +5113,32 @@ if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
 fi
 
+if test "$supported_cpu" = "no"; then
+echo
+echo "WARNING: SUPPORT FOR THIS HOST CPU WILL GO AWAY IN FUTURE RELEASES!"
+echo
+echo "CPU host architecture $cpu support is not currently maintained."
+echo "The QEMU project intends to remove support for this host CPU in"
+echo "a future release if nobody volunteers to maintain it and to"
+echo "provide a build host for our continuous integration setup."
+echo "configure has succeeded and you can continue to build, but"
+echo "if you care about QEMU on this platform you should contact"
+echo "us upstream at qemu-devel@nongnu.org."
+fi
+
+if test "$supported_os" = "no"; then
+echo
+echo "WARNING: SUPPORT FOR THIS HOST OS WILL GO AWAY IN FUTURE RELEASES!"
+echo
+echo "CPU host OS $targetos support is not currently maintained."
+echo "The QEMU project intends to remove support for this host CPU in"
+echo "a future release if nobody volunteers to maintain it and to"
+echo "provide a build host for our continuous integration setup."
+echo "configure has succeeded and you can continue to build, but"
+echo "if you care about QEMU on this platform you should contact"
+echo "us upstream at qemu-devel@nongnu.org."
+fi
+
 config_host_mak="config-host.mak"
 
 echo "# Automatically generated by configure - do not modify" 
>config-all-disas.mak
-- 
2.7.4




Re: [Qemu-devel] [PATCH v3 1/9] xen: do not build backends for targets that do not support xen

2017-03-17 Thread Greg Kurz
On Thu, 16 Mar 2017 13:01:50 -0700
Stefano Stabellini  wrote:

> Change Makefile.objs to use CONFIG_XEN instead of CONFIG_XEN_BACKEND, so
> that the Xen backends are only built for targets that support Xen.
> 
> Set CONFIG_XEN in the toplevel Makefile to ensure that files that are
> built only once pick up Xen support properly.
> 
> Signed-off-by: Stefano Stabellini 
> CC: gr...@kaod.org
> CC: pbonz...@redhat.com
> CC: peter.mayd...@linaro.org
> CC: r...@twiddle.net
> CC: stefa...@redhat.com
> ---

Tested-by: Greg Kurz 
Reviewed-by: Greg Kurz 

>  Makefile | 1 +
>  hw/block/Makefile.objs   | 2 +-
>  hw/char/Makefile.objs| 2 +-
>  hw/display/Makefile.objs | 2 +-
>  hw/net/Makefile.objs | 2 +-
>  hw/usb/Makefile.objs | 2 +-
>  hw/xen/Makefile.objs | 2 +-
>  7 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1c4c04f..b246138 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -26,6 +26,7 @@ endif
>  
>  CONFIG_SOFTMMU := $(if $(filter %-softmmu,$(TARGET_DIRS)),y)
>  CONFIG_USER_ONLY := $(if $(filter %-user,$(TARGET_DIRS)),y)
> +CONFIG_XEN := $(CONFIG_XEN_BACKEND)
>  CONFIG_ALL=y
>  -include config-all-devices.mak
>  -include config-all-disas.mak
> diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
> index d4c3ab7..e0ed980 100644
> --- a/hw/block/Makefile.objs
> +++ b/hw/block/Makefile.objs
> @@ -4,7 +4,7 @@ common-obj-$(CONFIG_SSI_M25P80) += m25p80.o
>  common-obj-$(CONFIG_NAND) += nand.o
>  common-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o
>  common-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o
> -common-obj-$(CONFIG_XEN_BACKEND) += xen_disk.o
> +common-obj-$(CONFIG_XEN) += xen_disk.o
>  common-obj-$(CONFIG_ECC) += ecc.o
>  common-obj-$(CONFIG_ONENAND) += onenand.o
>  common-obj-$(CONFIG_NVME_PCI) += nvme.o
> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> index 6ea76fe..725fdc4 100644
> --- a/hw/char/Makefile.objs
> +++ b/hw/char/Makefile.objs
> @@ -7,7 +7,7 @@ common-obj-$(CONFIG_SERIAL_ISA) += serial-isa.o
>  common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
>  common-obj-$(CONFIG_VIRTIO) += virtio-console.o
>  common-obj-$(CONFIG_XILINX) += xilinx_uartlite.o
> -common-obj-$(CONFIG_XEN_BACKEND) += xen_console.o
> +common-obj-$(CONFIG_XEN) += xen_console.o
>  common-obj-$(CONFIG_CADENCE) += cadence_uart.o
>  
>  obj-$(CONFIG_EXYNOS4) += exynos4210_uart.o
> diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
> index 063889b..3d02e8b 100644
> --- a/hw/display/Makefile.objs
> +++ b/hw/display/Makefile.objs
> @@ -5,7 +5,7 @@ common-obj-$(CONFIG_JAZZ_LED) += jazz_led.o
>  common-obj-$(CONFIG_PL110) += pl110.o
>  common-obj-$(CONFIG_SSD0303) += ssd0303.o
>  common-obj-$(CONFIG_SSD0323) += ssd0323.o
> -common-obj-$(CONFIG_XEN_BACKEND) += xenfb.o
> +common-obj-$(CONFIG_XEN) += xenfb.o
>  
>  common-obj-$(CONFIG_VGA_PCI) += vga-pci.o
>  common-obj-$(CONFIG_VGA_ISA) += vga-isa.o
> diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
> index 610ed3e..6a95d92 100644
> --- a/hw/net/Makefile.objs
> +++ b/hw/net/Makefile.objs
> @@ -1,5 +1,5 @@
>  common-obj-$(CONFIG_DP8393X) += dp8393x.o
> -common-obj-$(CONFIG_XEN_BACKEND) += xen_nic.o
> +common-obj-$(CONFIG_XEN) += xen_nic.o
>  
>  # PCI network cards
>  common-obj-$(CONFIG_NE2000_PCI) += ne2000.o
> diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
> index 98b5c9d..5958be8 100644
> --- a/hw/usb/Makefile.objs
> +++ b/hw/usb/Makefile.objs
> @@ -40,5 +40,5 @@ common-obj-$(CONFIG_USB_REDIR) += redirect.o quirks.o
>  common-obj-y += $(patsubst %,host-%.o,$(HOST_USB))
>  
>  ifeq ($(CONFIG_USB_LIBUSB),y)
> -common-obj-$(CONFIG_XEN_BACKEND) += xen-usb.o
> +common-obj-$(CONFIG_XEN) += xen-usb.o
>  endif
> diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
> index 591cdc2..4be3ec9 100644
> --- a/hw/xen/Makefile.objs
> +++ b/hw/xen/Makefile.objs
> @@ -1,5 +1,5 @@
>  # xen backend driver support
> -common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o xen_pvdev.o
> +common-obj-$(CONFIG_XEN) += xen_backend.o xen_devconfig.o xen_pvdev.o
>  
>  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
>  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o 
> xen_pt_graphics.o xen_pt_msi.o



pgpKrP5LT5pbR.pgp
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v3 1/9] xen: do not build backends for targets that do not support xen

2017-03-17 Thread Paolo Bonzini


On 16/03/2017 21:01, Stefano Stabellini wrote:
> Change Makefile.objs to use CONFIG_XEN instead of CONFIG_XEN_BACKEND, so
> that the Xen backends are only built for targets that support Xen.
> 
> Set CONFIG_XEN in the toplevel Makefile to ensure that files that are
> built only once pick up Xen support properly.
> 
> Signed-off-by: Stefano Stabellini 
> CC: gr...@kaod.org
> CC: pbonz...@redhat.com
> CC: peter.mayd...@linaro.org
> CC: r...@twiddle.net
> CC: stefa...@redhat.com
> ---
>  Makefile | 1 +
>  hw/block/Makefile.objs   | 2 +-
>  hw/char/Makefile.objs| 2 +-
>  hw/display/Makefile.objs | 2 +-
>  hw/net/Makefile.objs | 2 +-
>  hw/usb/Makefile.objs | 2 +-
>  hw/xen/Makefile.objs | 2 +-
>  7 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1c4c04f..b246138 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -26,6 +26,7 @@ endif
>  
>  CONFIG_SOFTMMU := $(if $(filter %-softmmu,$(TARGET_DIRS)),y)
>  CONFIG_USER_ONLY := $(if $(filter %-user,$(TARGET_DIRS)),y)
> +CONFIG_XEN := $(CONFIG_XEN_BACKEND)
>  CONFIG_ALL=y
>  -include config-all-devices.mak
>  -include config-all-disas.mak
> diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
> index d4c3ab7..e0ed980 100644
> --- a/hw/block/Makefile.objs
> +++ b/hw/block/Makefile.objs
> @@ -4,7 +4,7 @@ common-obj-$(CONFIG_SSI_M25P80) += m25p80.o
>  common-obj-$(CONFIG_NAND) += nand.o
>  common-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o
>  common-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o
> -common-obj-$(CONFIG_XEN_BACKEND) += xen_disk.o
> +common-obj-$(CONFIG_XEN) += xen_disk.o
>  common-obj-$(CONFIG_ECC) += ecc.o
>  common-obj-$(CONFIG_ONENAND) += onenand.o
>  common-obj-$(CONFIG_NVME_PCI) += nvme.o
> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> index 6ea76fe..725fdc4 100644
> --- a/hw/char/Makefile.objs
> +++ b/hw/char/Makefile.objs
> @@ -7,7 +7,7 @@ common-obj-$(CONFIG_SERIAL_ISA) += serial-isa.o
>  common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
>  common-obj-$(CONFIG_VIRTIO) += virtio-console.o
>  common-obj-$(CONFIG_XILINX) += xilinx_uartlite.o
> -common-obj-$(CONFIG_XEN_BACKEND) += xen_console.o
> +common-obj-$(CONFIG_XEN) += xen_console.o
>  common-obj-$(CONFIG_CADENCE) += cadence_uart.o
>  
>  obj-$(CONFIG_EXYNOS4) += exynos4210_uart.o
> diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
> index 063889b..3d02e8b 100644
> --- a/hw/display/Makefile.objs
> +++ b/hw/display/Makefile.objs
> @@ -5,7 +5,7 @@ common-obj-$(CONFIG_JAZZ_LED) += jazz_led.o
>  common-obj-$(CONFIG_PL110) += pl110.o
>  common-obj-$(CONFIG_SSD0303) += ssd0303.o
>  common-obj-$(CONFIG_SSD0323) += ssd0323.o
> -common-obj-$(CONFIG_XEN_BACKEND) += xenfb.o
> +common-obj-$(CONFIG_XEN) += xenfb.o
>  
>  common-obj-$(CONFIG_VGA_PCI) += vga-pci.o
>  common-obj-$(CONFIG_VGA_ISA) += vga-isa.o
> diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
> index 610ed3e..6a95d92 100644
> --- a/hw/net/Makefile.objs
> +++ b/hw/net/Makefile.objs
> @@ -1,5 +1,5 @@
>  common-obj-$(CONFIG_DP8393X) += dp8393x.o
> -common-obj-$(CONFIG_XEN_BACKEND) += xen_nic.o
> +common-obj-$(CONFIG_XEN) += xen_nic.o
>  
>  # PCI network cards
>  common-obj-$(CONFIG_NE2000_PCI) += ne2000.o
> diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
> index 98b5c9d..5958be8 100644
> --- a/hw/usb/Makefile.objs
> +++ b/hw/usb/Makefile.objs
> @@ -40,5 +40,5 @@ common-obj-$(CONFIG_USB_REDIR) += redirect.o quirks.o
>  common-obj-y += $(patsubst %,host-%.o,$(HOST_USB))
>  
>  ifeq ($(CONFIG_USB_LIBUSB),y)
> -common-obj-$(CONFIG_XEN_BACKEND) += xen-usb.o
> +common-obj-$(CONFIG_XEN) += xen-usb.o
>  endif
> diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
> index 591cdc2..4be3ec9 100644
> --- a/hw/xen/Makefile.objs
> +++ b/hw/xen/Makefile.objs
> @@ -1,5 +1,5 @@
>  # xen backend driver support
> -common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o xen_pvdev.o
> +common-obj-$(CONFIG_XEN) += xen_backend.o xen_devconfig.o xen_pvdev.o
>  
>  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
>  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o 
> xen_pt_graphics.o xen_pt_msi.o
> 

I'm queuing this for 2.9 as a bugfix, adding Xen things to various
softcore emulators makes no sense.

Paolo



Re: [Qemu-devel] callout to *file in bdrv_co_get_block_status

2017-03-17 Thread Peter Lieven
Am 17.03.2017 um 11:59 schrieb Paolo Bonzini:
>
> On 17/03/2017 11:45, Peter Lieven wrote:
>> Hi,
>>
>>
>> I tried to debug why qemu-img convert with a VMDK source laying on a tmpfs 
>> is horrible slow.
>>
>> For some reason a lseek on a tmpfs is slow. Strictly speaking the lseek in 
>> find_allocation in file-posix.c
>>
>> is slow.
>>
>>
>> When qemu-img convert iterates over all sectors of a VMDK file to check 
>> their allocation status it ends
>>
>> up checking allocation status of all allocated sectors due to the following 
>> condition in bdrv_co_get_block_status:
>>
>>
>> if (*file && *file != bs &&
>> (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
>> (ret & BDRV_BLOCK_OFFSET_VALID)) {
>> BlockDriverState *file2;
>> int file_pnum;
>> ret2 = bdrv_co_get_block_status(*file, ret >> BDRV_SECTOR_BITS,
>> *pnum, &file_pnum, &file2);
>> if (ret2 >= 0) {
>> /* Ignore errors.  This is just providing extra information, it
>>  * is useful but not necessary.
>>  */
>> if (!file_pnum) {
>> /* !file_pnum indicates an offset at or beyond the EOF; it is
>>  * perfectly valid for the format block driver to point to 
>> such
>>  * offsets, so catch it and mark everything as zero */
>> ret |= BDRV_BLOCK_ZERO;
>> } else {
>> /* Limit request to the range reported by the protocol 
>> driver */
>> *pnum = file_pnum;
>> ret |= (ret2 & BDRV_BLOCK_ZERO);
>> }
>> }
>> }
>>
>>
>> Does anybody remember for what circumstances this case this was added? In 
>> case of an container format
>>
>> like VMDK or QCOW2 shouldn't we trust the information from the l2 tables in 
>> the VMDK or QCOW2?
> It provides additional information, for example it works better with
> prealloc=metadata.

Okay, understood. Can you imagine of a away to conditionally avoid this second 
callout? In my case we have an additional
lseek for each cluster. For a 20GB file this are approx. 327k calls to lseek. 
And if the file has no preallocated metadata
it will likely not improve anything. And even if the metadata is prealloced 
what is the allocation status of the clusters?

Peter



Re: [Qemu-devel] [PATCH v3 2/9] xen: import ring.h from xen

2017-03-17 Thread Greg Kurz
On Thu, 16 Mar 2017 13:01:51 -0700
Stefano Stabellini  wrote:

> Do not use the ring.h header installed on the system. Instead, import
> the header into the QEMU codebase. This avoids problems when QEMU is
> built against a Xen version too old to provide all the ring macros.
> 
> Signed-off-by: Stefano Stabellini 
> CC: anthony.per...@citrix.com
> CC: jgr...@suse.com
> ---
> NB: The new macros have not been committed to Xen yet. Do not apply this
> patch until they do.
> ---

There are many trailing spaces in include/hw/xen/io/ring.h, but I guess this
is acceptable for an imported header.

Reviewed-by: Greg Kurz 

> ---
>  hw/block/xen_blkif.h |   2 +-
>  hw/usb/xen-usb.c |   2 +-
>  include/hw/xen/io/ring.h | 455 
> +++
>  3 files changed, 457 insertions(+), 2 deletions(-)
>  create mode 100644 include/hw/xen/io/ring.h
> 
> diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h
> index 3300b6f..3e6e1ea 100644
> --- a/hw/block/xen_blkif.h
> +++ b/hw/block/xen_blkif.h
> @@ -1,7 +1,7 @@
>  #ifndef XEN_BLKIF_H
>  #define XEN_BLKIF_H
>  
> -#include 
> +#include "hw/xen/io/ring.h"
>  #include 
>  #include 
>  
> diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
> index 8e676e6..370b3d9 100644
> --- a/hw/usb/xen-usb.c
> +++ b/hw/usb/xen-usb.c
> @@ -33,7 +33,7 @@
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qstring.h"
>  
> -#include 
> +#include "hw/xen/io/ring.h"
>  #include 
>  
>  /*
> diff --git a/include/hw/xen/io/ring.h b/include/hw/xen/io/ring.h
> new file mode 100644
> index 000..cf01fc3
> --- /dev/null
> +++ b/include/hw/xen/io/ring.h
> @@ -0,0 +1,455 @@
> +/**
> + * ring.h
> + * 
> + * Shared producer-consumer ring macros.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, 
> and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Tim Deegan and Andrew Warfield November 2004.
> + */
> +
> +#ifndef __XEN_PUBLIC_IO_RING_H__
> +#define __XEN_PUBLIC_IO_RING_H__
> +
> +#if __XEN_INTERFACE_VERSION__ < 0x00030208
> +#define xen_mb()  mb()
> +#define xen_rmb() rmb()
> +#define xen_wmb() wmb()
> +#endif
> +
> +typedef unsigned int RING_IDX;
> +
> +/* Round a 32-bit unsigned constant down to the nearest power of two. */
> +#define __RD2(_x)  (((_x) & 0x0002) ? 0x2  : ((_x) & 
> 0x1))
> +#define __RD4(_x)  (((_x) & 0x000c) ? __RD2((_x)>>2)<<2: __RD2(_x))
> +#define __RD8(_x)  (((_x) & 0x00f0) ? __RD4((_x)>>4)<<4: __RD4(_x))
> +#define __RD16(_x) (((_x) & 0xff00) ? __RD8((_x)>>8)<<8: __RD8(_x))
> +#define __RD32(_x) (((_x) & 0x) ? __RD16((_x)>>16)<<16 : __RD16(_x))
> +
> +/*
> + * Calculate size of a shared ring, given the total available space for the
> + * ring and indexes (_sz), and the name tag of the request/response 
> structure.
> + * A ring contains as many entries as will fit, rounded down to the nearest 
> + * power of two (so we can mask with (size-1) to loop around).
> + */
> +#define __CONST_RING_SIZE(_s, _sz) \
> +(__RD32(((_sz) - offsetof(struct _s##_sring, ring)) / \
> + sizeof(((struct _s##_sring *)0)->ring[0])))
> +/*
> + * The same for passing in an actual pointer instead of a name tag.
> + */
> +#define __RING_SIZE(_s, _sz) \
> +(__RD32(((_sz) - (long)(_s)->ring + (long)(_s)) / sizeof((_s)->ring[0])))
> +
> +/*
> + * Macros to make the correct C datatypes for a new kind of ring.
> + * 
> + * To make a new ring datatype, you need to have two message structures,
> + * let's say request_t, and response_t already defined.
> + *
> + * In a header where you want the ring datatype declared, you then do:
> + *
> + * DEFINE_RING_TYPES(mytag, request_t, response_t);
> + *
> + * These expand out to give you a set of types, as you can see below.
> + * The most important of these are:
> + * 
> + * mytag_sring_t  - The shared ring.
> + * mytag_front_ring_t - The '

Re: [Qemu-devel] callout to *file in bdrv_co_get_block_status

2017-03-17 Thread Paolo Bonzini


On 17/03/2017 12:11, Peter Lieven wrote:
>>> like VMDK or QCOW2 shouldn't we trust the information from the l2 tables in 
>>> the VMDK or QCOW2?
>> It provides additional information, for example it works better with
>> prealloc=metadata.
> Okay, understood. Can you imagine of a away to conditionally avoid this 
> second callout? In my case we have an additional
> lseek for each cluster. For a 20GB file this are approx. 327k calls to lseek. 
> And if the file has no preallocated metadata
> it will likely not improve anything. And even if the metadata is prealloced 
> what is the allocation status of the clusters?

If the metadata is preallocated, cluster will (or should) show up as
zero, speeding up the copy.

Paolo



Re: [Qemu-devel] [PATCH v3 3/9] 9p: introduce a type for the 9p header

2017-03-17 Thread Greg Kurz
On Thu, 16 Mar 2017 13:01:52 -0700
Stefano Stabellini  wrote:

> Use the new type in virtio-9p-device.
> 
> Signed-off-by: Stefano Stabellini 
> CC: anthony.per...@citrix.com
> CC: jgr...@suse.com
> CC: Aneesh Kumar K.V 
> CC: Greg Kurz 
> ---

Reviewed-by: Greg Kurz 

>  hw/9pfs/9p.h   | 6 ++
>  hw/9pfs/virtio-9p-device.c | 6 +-
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index b7e8362..5312d8a 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -119,6 +119,12 @@ static inline char *rpath(FsContext *ctx, const char 
> *path)
>  typedef struct V9fsPDU V9fsPDU;
>  struct V9fsState;
>  
> +typedef struct {
> +uint32_t size_le;
> +uint8_t id;
> +uint16_t tag_le;
> +} QEMU_PACKED P9MsgHeader;
> +
>  struct V9fsPDU
>  {
>  uint32_t size;
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 27a4a32..3782f43 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -46,11 +46,7 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue 
> *vq)
>  VirtQueueElement *elem;
>  
>  while ((pdu = pdu_alloc(s))) {
> -struct {
> -uint32_t size_le;
> -uint8_t id;
> -uint16_t tag_le;
> -} QEMU_PACKED out;
> +P9MsgHeader out;
>  
>  elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
>  if (!elem) {



pgpxZOtkivrdS.pgp
Description: OpenPGP digital signature


Re: [Qemu-devel] callout to *file in bdrv_co_get_block_status

2017-03-17 Thread Peter Lieven
Am 17.03.2017 um 12:16 schrieb Paolo Bonzini:
>
> On 17/03/2017 12:11, Peter Lieven wrote:
 like VMDK or QCOW2 shouldn't we trust the information from the l2 tables 
 in the VMDK or QCOW2?
>>> It provides additional information, for example it works better with
>>> prealloc=metadata.
>> Okay, understood. Can you imagine of a away to conditionally avoid this 
>> second callout? In my case we have an additional
>> lseek for each cluster. For a 20GB file this are approx. 327k calls to 
>> lseek. And if the file has no preallocated metadata
>> it will likely not improve anything. And even if the metadata is prealloced 
>> what is the allocation status of the clusters?
> If the metadata is preallocated, cluster will (or should) show up as
> zero, speeding up the copy.

Okay, in this case the second call out to *file will not happen. It only 
happens if the metadata says it contains data.
So where does it actually help?

The condition is: (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) && (ret & 
BDRV_BLOCK_OFFSET_VALID))

So from my view it can only have any effect if the metadata returns 
BDRV_BLOCK_DATA, but the protocol driver returns
BDRV_BLOCK_ZERO.

This can only happen if I partially write to a cluster, or am I wrong here?

Peter



Re: [Qemu-devel] callout to *file in bdrv_co_get_block_status

2017-03-17 Thread Fam Zheng
On Fri, 03/17 12:16, Paolo Bonzini wrote:
> 
> 
> On 17/03/2017 12:11, Peter Lieven wrote:
> >>> like VMDK or QCOW2 shouldn't we trust the information from the l2 tables 
> >>> in the VMDK or QCOW2?
> >> It provides additional information, for example it works better with
> >> prealloc=metadata.
> > Okay, understood. Can you imagine of a away to conditionally avoid this 
> > second callout? In my case we have an additional
> > lseek for each cluster. For a 20GB file this are approx. 327k calls to 
> > lseek. And if the file has no preallocated metadata
> > it will likely not improve anything. And even if the metadata is prealloced 
> > what is the allocation status of the clusters?
> 
> If the metadata is preallocated, cluster will (or should) show up as
> zero, speeding up the copy.

I think from qemu-img convert's perspective, it doesn't care about the *file
status if the metadata already speaks, because, like you said, the data shows up
as zeroes.

In other words I think this can be optimized.

Fam



Re: [Qemu-devel] [PATCH] checkpatch: allow longer lines for logging functions

2017-03-17 Thread Greg Kurz
On Fri, 17 Mar 2017 03:45:56 -0700 (PDT)
no-re...@patchew.org wrote:

> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Message-id: 148974701986.29545.5414999102981738774.stgit@bahia
> Subject: [Qemu-devel] [PATCH] checkpatch: allow longer lines for logging 
> functions
> Type: series
> 
> === 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/148974701986.29545.5414999102981738774.stgit@bahia -> 
> patchew/148974701986.29545.5414999102981738774.stgit@bahia
>  * [new tag] 
> patchew/148974744281.30636.17973264008285415592.stgit@bahia -> 
> patchew/148974744281.30636.17973264008285415592.stgit@bahia
> Switched to a new branch 'test'
> 390d7c4 checkpatch: allow longer lines for logging functions
> 
> === OUTPUT BEGIN ===
> Checking PATCH 1/1: checkpatch: allow longer lines for logging functions...
> ERROR: line over 90 characters

Ha ha ! Should checkpatch allow long regexp lines in itself ? ;)

> #38: FILE: scripts/checkpatch.pl:1349:
> + !($line =~ 
> /^\+\s*$logFunctions\s*\(\s*(?:([^"]*))?"[X\t]*"\s*(?:,|\)\s*;)\s*$/ ||
> 
> total: 1 errors, 0 warnings, 20 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.
> 
> === 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


pgp2tH0mj8iPN.pgp
Description: OpenPGP digital signature


[Qemu-devel] [PATCH for-2.9] virtio: Fix error handling in virtio_bus_device_plugged

2017-03-17 Thread Fam Zheng
For one thing we shouldn't continue if an error happened, for the other
two steps failing can cause an abort() in error_setg because we reuse
the same errp blindly.

Add error handling checks to fix both issues.

Signed-off-by: Fam Zheng 
---
 hw/virtio/virtio-bus.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index a886011..ef76919 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "qemu/error-report.h"
+#include "qapi/error.h"
 #include "hw/qdev.h"
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio.h"
@@ -48,21 +49,34 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error 
**errp)
 VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
 bool has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+Error *local_err = NULL;
 
 DPRINTF("%s: plug device.\n", qbus->name);
 
 if (klass->pre_plugged != NULL) {
-klass->pre_plugged(qbus->parent, errp);
+klass->pre_plugged(qbus->parent, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
 }
 
 /* Get the features of the plugged device. */
 assert(vdc->get_features != NULL);
 vdev->host_features = vdc->get_features(vdev, vdev->host_features,
-errp);
+&local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
 
 if (klass->device_plugged != NULL) {
 klass->device_plugged(qbus->parent, errp);
 }
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
 
 if (klass->get_dma_as != NULL && has_iommu) {
 virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
-- 
2.9.3




[Qemu-devel] [RFC PATCH 1/4] I440: Allow adding sysbus devices with -device on I440

2017-03-17 Thread Lan Tianyu
From: Chao Gao 

xen-viommu will be a sysbus device and the device model will
be enabled via "-device" parameter.

Signed-off-by: Chao Gao 
Signed-off-by: Lan Tianyu 
---
 hw/i386/pc_piix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a07dc81..3289593 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -436,6 +436,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
 m->hot_add_cpu = pc_hot_add_cpu;
 m->default_machine_opts = "firmware=bios-256k.bin";
 m->default_display = "std";
+m->has_dynamic_sysbus = true;
 }
 
 static void pc_i440fx_2_7_machine_options(MachineClass *m)
-- 
1.8.3.1




[Qemu-devel] [RFC PATCH 0/4] Qemu: Add Xen vIOMMU support

2017-03-17 Thread Lan Tianyu
This patchset is to add Xen vIOMMU device model and handle
irq remapping stuffs. Xen vIOMMU emulation is in the Xen hypervisor
and the new device module in Qemu works as hypercall wrappers to
create and destroy vIOMMU in hypervisor.

Xen only supports emulated I440 and so we enable vIOMMU with emulated
I440 chipset. This works on Linux and Windows guest.


Chao Gao (4):
  I440: Allow adding sysbus devices with -device on I440
  Xen: add a dummy vIOMMU to create/destroy vIOMMU in Xen
  xen-pt: bind/unbind interrupt remapping format MSI
  msi: taking interrupt format into consideration during judging a pirq
is binded with a event channel

 hw/i386/pc_piix.c |   1 +
 hw/pci/msi.c  |   5 +-
 hw/pci/msix.c |   4 +-
 hw/xen/Makefile.objs  |   1 +
 hw/xen/xen_pt_msi.c   |  38 ++
 hw/xen/xen_viommu.c   | 116 ++
 include/hw/i386/apic-msidef.h |   1 +
 include/hw/xen/xen.h  |   2 +-
 xen-hvm-stub.c|   2 +-
 xen-hvm.c |   7 ++-
 10 files changed, 162 insertions(+), 15 deletions(-)
 create mode 100644 hw/xen/xen_viommu.c

-- 
1.8.3.1




[Qemu-devel] [RFC PATCH 3/4] xen-pt: bind/unbind interrupt remapping format MSI

2017-03-17 Thread Lan Tianyu
From: Chao Gao 

If a vIOMMU is exposed to guest, guest will configure the msi to remapping
format. The original code isn't suitable to the new format. A new pair
bind/unbind interfaces are added for this usage. This patch recognizes
this case and use new interfaces to bind/unbind msi.

Signed-off-by: Chao Gao 
Signed-off-by: Lan Tianyu 
---
 hw/xen/xen_pt_msi.c   | 36 
 include/hw/i386/apic-msidef.h |  1 +
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index 62add06..8b0d7fc 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -161,6 +161,7 @@ static int msi_msix_update(XenPCIPassthroughState *s,
 uint8_t gvec = msi_vector(data);
 uint32_t gflags = msi_gflags(data, addr);
 int rc = 0;
+bool ir = !!(addr & MSI_ADDR_IM_MASK);
 uint64_t table_addr = 0;
 
 XEN_PT_LOG(d, "Updating MSI%s with pirq %d gvec %#x gflags %#x"
@@ -171,8 +172,14 @@ static int msi_msix_update(XenPCIPassthroughState *s,
 table_addr = s->msix->mmio_base_addr;
 }
 
-rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
+if (ir) {
+rc = xc_domain_update_msi_irq_remapping(xen_xc, xen_domid, pirq,
+d->devfn, data, addr, table_addr);
+}
+else {
+rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
   pirq, gflags, table_addr);
+}
 
 if (rc) {
 XEN_PT_ERR(d, "Updating of MSI%s failed. (err: %d)\n",
@@ -204,13 +211,26 @@ static int msi_msix_disable(XenPCIPassthroughState *s,
 }
 
 if (is_binded) {
-XEN_PT_LOG(d, "Unbind MSI%s with pirq %d, gvec %#x\n",
-   is_msix ? "-X" : "", pirq, gvec);
-rc = xc_domain_unbind_msi_irq(xen_xc, xen_domid, gvec, pirq, gflags);
-if (rc) {
-XEN_PT_ERR(d, "Unbinding of MSI%s failed. (err: %d, pirq: %d, 
gvec: %#x)\n",
-   is_msix ? "-X" : "", errno, pirq, gvec);
-return rc;
+if ( addr & MSI_ADDR_IM_MASK ) {
+XEN_PT_LOG(d, "Unbinding of MSI%s . ( pirq: %d, data: %x, addr: 
%lx)\n",
+   is_msix ? "-X" : "", pirq, data, addr);
+rc = xc_domain_unbind_msi_irq_remapping(xen_xc, xen_domid, pirq,
+d->devfn, data, addr);
+if (rc) {
+XEN_PT_ERR(d, "Unbinding of MSI%s . (error: %d, pirq: %d, 
data: %x, addr: %lx)\n",
+   is_msix ? "-X" : "", rc, pirq, data, addr);
+return rc;
+}
+
+} else {
+XEN_PT_LOG(d, "Unbind MSI%s with pirq %d, gvec %#x\n",
+   is_msix ? "-X" : "", pirq, gvec);
+rc = xc_domain_unbind_msi_irq(xen_xc, xen_domid, gvec, pirq, 
gflags);
+if (rc) {
+XEN_PT_ERR(d, "Unbinding of MSI%s failed. (err: %d, pirq: %d, 
gvec: %#x)\n",
+   is_msix ? "-X" : "", errno, pirq, gvec);
+return rc;
+}
 }
 }
 
diff --git a/include/hw/i386/apic-msidef.h b/include/hw/i386/apic-msidef.h
index 8b4d4cc..08b584f 100644
--- a/include/hw/i386/apic-msidef.h
+++ b/include/hw/i386/apic-msidef.h
@@ -27,5 +27,6 @@
 #define MSI_ADDR_DEST_ID_SHIFT  12
 #define MSI_ADDR_DEST_IDX_SHIFT 4
 #define  MSI_ADDR_DEST_ID_MASK  0x000
+#define  MSI_ADDR_IM_MASK   0x0010
 
 #endif /* HW_APIC_MSIDEF_H */
-- 
1.8.3.1




[Qemu-devel] [RFC PATCH 2/4] Xen: add a dummy vIOMMU to create/destroy vIOMMU in Xen

2017-03-17 Thread Lan Tianyu
From: Chao Gao 

Since adding a dynamic sysbus device is forbidden, so choose TYPE_DEVICE
as parent class.

Signed-off-by: Chao Gao 
Signed-off-by: Lan Tianyu 
---
 hw/xen/Makefile.objs |   1 +
 hw/xen/xen_viommu.c  | 116 +++
 2 files changed, 117 insertions(+)
 create mode 100644 hw/xen/xen_viommu.c

diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
index d367094..e37d808 100644
--- a/hw/xen/Makefile.objs
+++ b/hw/xen/Makefile.objs
@@ -3,3 +3,4 @@ common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o 
xen_devconfig.o
 
 obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
 obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o 
xen_pt_graphics.o xen_pt_msi.o
+obj-$(CONFIG_XEN) += xen_viommu.o
diff --git a/hw/xen/xen_viommu.c b/hw/xen/xen_viommu.c
new file mode 100644
index 000..9bf9158
--- /dev/null
+++ b/hw/xen/xen_viommu.c
@@ -0,0 +1,116 @@
+/*
+ * Xen virtual IOMMU (virtual VT-D)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program 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 General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+
+#include "hw/qdev-core.h"
+#include "hw/sysbus.h"
+#include "hw/xen/xen.h"
+#include "hw/xen/xen_backend.h"
+
+#define TYPE_XEN_VIOMMU_DEVICE "xen_viommu"
+#define  XEN_VIOMMU_DEVICE(obj) \
+OBJECT_CHECK(XenVIOMMUState, (obj), TYPE_XEN_VIOMMU_DEVICE)
+
+typedef struct XenVIOMMUState XenVIOMMUState;
+
+struct XenVIOMMUState {
+DeviceState dev;
+uint32_t id;
+uint64_t cap;
+uint64_t base_addr;
+};
+
+static void xen_viommu_realize(DeviceState *dev, Error **errp)
+{
+int rc;
+uint64_t cap;
+char *dom;
+char viommu_path[1024];
+XenVIOMMUState *s = XEN_VIOMMU_DEVICE(dev);
+
+s->id = -1;
+
+/* Read vIOMMU attributes from Xenstore. */
+dom = xs_get_domain_path(xenstore, xen_domid);
+snprintf(viommu_path, sizeof(viommu_path), "%s/viommu", dom);
+rc = xenstore_read_uint64(viommu_path, "base_addr", &s->base_addr);  
+if (rc) {
+error_report("Can't get base address of vIOMMU");
+exit(1);
+}
+
+rc = xenstore_read_uint64(viommu_path, "cap", &s->cap);
+if (rc) {
+error_report("Can't get capabilities of vIOMMU");
+exit(1);
+}
+
+rc = xc_viommu_query_cap(xen_xc, xen_domid, &cap);
+if (rc) {
+exit(1);
+}
+
+
+if ((cap & s->cap) != cap) {
+error_report("xen: Unsupported capability %lx", s->cap);
+exit(1);
+}
+
+rc = xc_viommu_create(xen_xc, xen_domid, s->base_addr, s->cap, &s->id);
+if (rc) {
+s->id = -1;
+error_report("xen: failed(%d) to create viommu ", rc);
+exit(1);
+}
+}
+
+static void xen_viommu_instance_finalize(Object *o)
+{
+int rc;
+XenVIOMMUState *s = XEN_VIOMMU_DEVICE(o);
+
+if (s->id != -1) {
+rc = xc_viommu_destroy(xen_xc, xen_domid, s->id); 
+if (rc) {
+error_report("xen: failed(%d) to destroy viommu ", rc);
+}
+}
+}
+
+static void xen_viommu_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+dc->hotpluggable = false;
+dc->realize = xen_viommu_realize;
+}
+
+static const TypeInfo xen_viommu_info = {
+.name  = TYPE_XEN_VIOMMU_DEVICE,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(XenVIOMMUState),
+.instance_finalize = xen_viommu_instance_finalize,
+.class_init= xen_viommu_class_init,
+};
+
+static void xen_viommu_register_types(void)
+{
+type_register_static(&xen_viommu_info); 
+}
+
+type_init(xen_viommu_register_types);
-- 
1.8.3.1




[Qemu-devel] [RFC PATCH 4/4] msi: taking interrupt format into consideration during judging a pirq is binded with a event channel

2017-03-17 Thread Lan Tianyu
From: Chao Gao 

As remapping format interrupt has been introduced, the vector in msi remapping
format can also be 0, same as a interrupt is binded with a event channel.
So we can't just use whether vector is 0 or not to judge a msi is binded
to a event channel or not.

This patch takes the msi interrupt format into consideration.

Signed-off-by: Chao Gao 
Signed-off-by: Lan Tianyu 
---
 hw/pci/msi.c | 5 +++--
 hw/pci/msix.c| 4 +++-
 hw/xen/xen_pt_msi.c  | 2 +-
 include/hw/xen/xen.h | 2 +-
 xen-hvm-stub.c   | 2 +-
 xen-hvm.c| 7 ++-
 6 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a87b227..8d1ac9e 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -289,7 +289,7 @@ void msi_reset(PCIDevice *dev)
 static bool msi_is_masked(const PCIDevice *dev, unsigned int vector)
 {
 uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
-uint32_t mask, data;
+uint32_t mask, data, addr_lo;
 bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
 assert(vector < PCI_MSI_VECTORS_MAX);
 
@@ -298,7 +298,8 @@ static bool msi_is_masked(const PCIDevice *dev, unsigned 
int vector)
 }
 
 data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
-if (xen_is_pirq_msi(data)) {
+addr_lo = pci_get_word(dev->config + msi_address_lo_off(dev));
+if (xen_is_pirq_msi(data, addr_lo)) {
 return false;
 }
 
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 0ec1cb1..6b8045a 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -81,9 +81,11 @@ static bool msix_vector_masked(PCIDevice *dev, unsigned int 
vector, bool fmask)
 {
 unsigned offset = vector * PCI_MSIX_ENTRY_SIZE;
 uint8_t *data = &dev->msix_table[offset + PCI_MSIX_ENTRY_DATA];
+uint8_t *addr_lo = &dev->msix_table[offset + PCI_MSIX_ENTRY_LOWER_ADDR];
 /* MSIs on Xen can be remapped into pirqs. In those cases, masking
  * and unmasking go through the PV evtchn path. */
-if (xen_enabled() && xen_is_pirq_msi(pci_get_long(data))) {
+if (xen_enabled() && xen_is_pirq_msi(pci_get_long(data),
+ pci_get_long(addr_lo))) {
 return false;
 }
 return fmask || dev->msix_table[offset + PCI_MSIX_ENTRY_VECTOR_CTRL] &
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index 8b0d7fc..f799fed 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -114,7 +114,7 @@ static int msi_msix_setup(XenPCIPassthroughState *s,
 
 assert((!is_msix && msix_entry == 0) || is_msix);
 
-if (xen_is_pirq_msi(data)) {
+if (xen_is_pirq_msi(data, (uint32_t)(addr & 0x))) {
 *ppirq = msi_ext_dest_id(addr >> 32) | msi_dest_id(addr);
 if (!*ppirq) {
 /* this probably identifies an misconfiguration of the guest,
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index a8f3afb..c15beb5 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -33,7 +33,7 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
 void xen_piix3_set_irq(void *opaque, int irq_num, int level);
 void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
-int xen_is_pirq_msi(uint32_t msi_data);
+int xen_is_pirq_msi(uint32_t msi_data, uint32_t msi_addr_lo);
 
 qemu_irq *xen_interrupt_controller_init(void);
 
diff --git a/xen-hvm-stub.c b/xen-hvm-stub.c
index c500325..dae421c 100644
--- a/xen-hvm-stub.c
+++ b/xen-hvm-stub.c
@@ -31,7 +31,7 @@ void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
 {
 }
 
-int xen_is_pirq_msi(uint32_t msi_data)
+int xen_is_pirq_msi(uint32_t msi_data, uint32_t msi_addr_lo)
 {
 return 0;
 }
diff --git a/xen-hvm.c b/xen-hvm.c
index 2f348ed..9e78b23 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -146,8 +146,13 @@ void xen_piix_pci_write_config_client(uint32_t address, 
uint32_t val, int len)
 }
 }
 
-int xen_is_pirq_msi(uint32_t msi_data)
+int xen_is_pirq_msi(uint32_t msi_data, uint32_t msi_addr_lo)
 {
+/* If msi address is configurate to remapping format, the msi will not
+ * remapped into a pirq.
+ */
+if ( msi_addr_lo & 0x10 )
+return 0;
 /* If vector is 0, the msi is remapped into a pirq, passed as
  * dest_id.
  */
-- 
1.8.3.1




Re: [Qemu-devel] [RFC PATCH 0/4] Qemu: Add Xen vIOMMU support

2017-03-17 Thread no-reply
Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 1489750157-17401-1-git-send-email-tianyu@intel.com
Subject: [Qemu-devel] [RFC PATCH 0/4] Qemu: Add Xen vIOMMU support
Type: series

=== 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/1489750157-17401-1-git-send-email-tianyu@intel.com -> 
patchew/1489750157-17401-1-git-send-email-tianyu@intel.com
Switched to a new branch 'test'
0f16022 msi: taking interrupt format into consideration during judging a pirq 
is binded with a event channel
62b6353 xen-pt: bind/unbind interrupt remapping format MSI
0900f2b Xen: add a dummy vIOMMU to create/destroy vIOMMU in Xen
7cb3e06 I440: Allow adding sysbus devices with -device on I440

=== OUTPUT BEGIN ===
Checking PATCH 1/4: I440: Allow adding sysbus devices with -device on I440...
Checking PATCH 2/4: Xen: add a dummy vIOMMU to create/destroy vIOMMU in Xen...
ERROR: trailing whitespace
#75: FILE: hw/xen/xen_viommu.c:48:
+$

ERROR: trailing whitespace
#79: FILE: hw/xen/xen_viommu.c:52:
+rc = xenstore_read_uint64(viommu_path, "base_addr", &s->base_addr);  $

ERROR: trailing whitespace
#116: FILE: hw/xen/xen_viommu.c:89:
+rc = xc_viommu_destroy(xen_xc, xen_domid, s->id); $

ERROR: trailing whitespace
#140: FILE: hw/xen/xen_viommu.c:113:
+type_register_static(&xen_viommu_info); $

total: 4 errors, 0 warnings, 120 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/4: xen-pt: bind/unbind interrupt remapping format MSI...
ERROR: else should follow close brace '}'
#36: FILE: hw/xen/xen_pt_msi.c:179:
+}
+else {

ERROR: space prohibited after that open parenthesis '('
#54: FILE: hw/xen/xen_pt_msi.c:214:
+if ( addr & MSI_ADDR_IM_MASK ) {

ERROR: space prohibited before that close parenthesis ')'
#54: FILE: hw/xen/xen_pt_msi.c:214:
+if ( addr & MSI_ADDR_IM_MASK ) {

WARNING: line over 80 characters
#55: FILE: hw/xen/xen_pt_msi.c:215:
+XEN_PT_LOG(d, "Unbinding of MSI%s . ( pirq: %d, data: %x, addr: 
%lx)\n",

ERROR: line over 90 characters
#60: FILE: hw/xen/xen_pt_msi.c:220:
+XEN_PT_ERR(d, "Unbinding of MSI%s . (error: %d, pirq: %d, 
data: %x, addr: %lx)\n",

WARNING: line over 80 characters
#68: FILE: hw/xen/xen_pt_msi.c:228:
+rc = xc_domain_unbind_msi_irq(xen_xc, xen_domid, gvec, pirq, 
gflags);

ERROR: line over 90 characters
#70: FILE: hw/xen/xen_pt_msi.c:230:
+XEN_PT_ERR(d, "Unbinding of MSI%s failed. (err: %d, pirq: %d, 
gvec: %#x)\n",

total: 5 errors, 2 warnings, 61 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 4/4: msi: taking interrupt format into consideration during 
judging a pirq is binded with a event channel...
ERROR: space prohibited after that open parenthesis '('
#111: FILE: xen-hvm.c:154:
+if ( msi_addr_lo & 0x10 )

ERROR: space prohibited before that close parenthesis ')'
#111: FILE: xen-hvm.c:154:
+if ( msi_addr_lo & 0x10 )

ERROR: braces {} are necessary for all arms of this statement
#111: FILE: xen-hvm.c:154:
+if ( msi_addr_lo & 0x10 )
[...]

total: 3 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.

=== 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

Re: [Qemu-devel] [PATCH] configure: Warn about deprecated hosts

2017-03-17 Thread Daniel P. Berrange
On Fri, Mar 17, 2017 at 11:08:22AM +, Peter Maydell wrote:
> We plan to drop support in a future QEMU release for host OSes
> and host architectures for which we have no test machine where
> we can build and run tests. For the 2.9 release, make configure
> print a warning if it is run on such a host, so that the user
> has some warning of the plans and can volunteer to help us
> maintain the port if they need it to continue to function.
> 
> This commit flags up as deprecated the CPU architectures:
>  * ia64
>  * sparc
>  * anything which we don't have a TCG port for
>(and which was presumably using TCI)

This seems to imply that if we remove supported for these architectures,
then TCI is no longer required either, or would there be other reasons
to want to keep TCI around, even when all host archs have a TCG port ? 

> and the OSes:
>  * Cygwin
>  * GNU/kFreeBSD
>  * FreeBSD
>  * DragonFly BSD
>  * NetBSD
>  * OpenBSD
>  * Solaris
>  * AIX
>  * Haiku
> 
> It also makes entirely unrecognized host OS strings be
> rejected rather than treated as if they were Linux (which
> likely never worked).
> 
> Signed-off-by: Peter Maydell 
> ---
> This list is definitely too all-encompassing, and we should
> move at least some of the BSDs into "not-deprecated".
> I'm posting the patch for the moment for code review on the
> logic and as a placeholder.

FreeBSD is the one where I most frequently see developers engaging across
the overall virt tools stack (QEMU/libvirt/virt-manager/OpenStack).
GNU/kFreeBSD has come up a few times in libvirt, and I don't recall seeing
anyone mentioning the remaining BSDs - though perhaps thats because the
FreeBSD port fixes are enough to also make other BSDs work.

> diff --git a/configure b/configure
> index 75c7c35..5ddc985 100755
> --- a/configure
> +++ b/configure
> @@ -321,6 +321,9 @@ tcmalloc="no"
>  jemalloc="no"
>  replication="yes"
>  
> +supported_cpu="no"
> +supported_os="no"
> +
>  # parse CC options first
>  for opt do
>optarg=$(expr "x$opt" : 'x[^=]*=\(.*\)')
> @@ -517,23 +520,32 @@ ARCH=
>  # Normalise host CPU name and set ARCH.
>  # Note that this case should only have supported host CPUs, not guests.
>  case "$cpu" in
> -  ia64|ppc|ppc64|s390|s390x|sparc64|x32)
> +  ppc|ppc64|s390|s390x|x32)
> +cpu="$cpu"
> +supported_cpu="yes"
> +  ;;
> +  ia64|sparc64)
>  cpu="$cpu"
>;;
>i386|i486|i586|i686|i86pc|BePC)
>  cpu="i386"
> +supported_cpu="yes"
>;;
>x86_64|amd64)
>  cpu="x86_64"
> +supported_cpu="yes"
>;;
>armv*b|armv*l|arm)
>  cpu="arm"
> +supported_cpu="yes"
>;;
>aarch64)
>  cpu="aarch64"
> +supported_cpu="yes"
>;;
>mips*)
>  cpu="mips"
> +supported_cpu="yes"
>;;
>sparc|sun4[cdmuv])
>  cpu="sparc"
> @@ -568,6 +580,7 @@ MINGW32*)
>else
>  audio_drv_list=""
>fi
> +  supported_os="yes"
>  ;;
>  GNU/kFreeBSD)
>bsd="yes"
> @@ -626,6 +639,7 @@ Darwin)
># won't work when we're compiling with gcc as a C compiler.
>QEMU_CFLAGS="-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS"
>HOST_VARIANT_DIR="darwin"
> +  supported_os="yes"
>  ;;
>  SunOS)
>solaris="yes"
> @@ -672,7 +686,7 @@ Haiku)
>QEMU_CFLAGS="-DB_USE_POSITIVE_POSIX_ERRORS $QEMU_CFLAGS"
>LIBS="-lposix_error_mapper -lnetwork $LIBS"
>  ;;
> -*)
> +Linux)
>audio_drv_list="oss"
>audio_possible_drivers="oss alsa sdl pa"
>linux="yes"
> @@ -682,6 +696,10 @@ Haiku)
>vhost_scsi="yes"
>vhost_vsock="yes"
>QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers 
> $QEMU_INCLUDES"
> +  supported_os="yes"
> +;;
> +*)
> +  error_exit "Unsupported host OS $targetos"
>  ;;
>  esac
>  
> @@ -5095,6 +5113,32 @@ if test "$sdl_too_old" = "yes"; then
>  echo "-> Your SDL version is too old - please upgrade to have SDL support"
>  fi
>  
> +if test "$supported_cpu" = "no"; then
> +echo
> +echo "WARNING: SUPPORT FOR THIS HOST CPU WILL GO AWAY IN FUTURE 
> RELEASES!"
> +echo
> +echo "CPU host architecture $cpu support is not currently maintained."
> +echo "The QEMU project intends to remove support for this host CPU in"
> +echo "a future release if nobody volunteers to maintain it and to"
> +echo "provide a build host for our continuous integration setup."
> +echo "configure has succeeded and you can continue to build, but"
> +echo "if you care about QEMU on this platform you should contact"
> +echo "us upstream at qemu-devel@nongnu.org."
> +fi
> +
> +if test "$supported_os" = "no"; then
> +echo
> +echo "WARNING: SUPPORT FOR THIS HOST OS WILL GO AWAY IN FUTURE RELEASES!"
> +echo
> +echo "CPU host OS $targetos support is not currently maintained."
> +echo "The QEMU project intends to remove support for this host CPU in"
> +echo "a future release if nobody volunteers to maintain it and to"
> +echo "provide a build host for our continuous integration setup."
> +echo "configure has succeeded and you can conti

Re: [Qemu-devel] [PATCH] configure: Warn about deprecated hosts

2017-03-17 Thread Peter Maydell
On 17 March 2017 at 11:49, Daniel P. Berrange  wrote:
> On Fri, Mar 17, 2017 at 11:08:22AM +, Peter Maydell wrote:
>> We plan to drop support in a future QEMU release for host OSes
>> and host architectures for which we have no test machine where
>> we can build and run tests. For the 2.9 release, make configure
>> print a warning if it is run on such a host, so that the user
>> has some warning of the plans and can volunteer to help us
>> maintain the port if they need it to continue to function.
>>
>> This commit flags up as deprecated the CPU architectures:
>>  * ia64
>>  * sparc
>>  * anything which we don't have a TCG port for
>>(and which was presumably using TCI)
>
> This seems to imply that if we remove supported for these architectures,
> then TCI is no longer required either, or would there be other reasons
> to want to keep TCI around, even when all host archs have a TCG port ?

I've never really seen the point in TCI, but others do, which
is why it's in the tree.

>> This list is definitely too all-encompassing, and we should
>> move at least some of the BSDs into "not-deprecated".
>> I'm posting the patch for the moment for code review on the
>> logic and as a placeholder.
>
> FreeBSD is the one where I most frequently see developers engaging across
> the overall virt tools stack (QEMU/libvirt/virt-manager/OpenStack).
> GNU/kFreeBSD has come up a few times in libvirt, and I don't recall seeing
> anyone mentioning the remaining BSDs - though perhaps thats because the
> FreeBSD port fixes are enough to also make other BSDs work.

Yeah. I'm just struggling with setting up a FreeBSD VM so we
can compile test that. Interesting that GNU/kFreeBSD has users.

thanks
-- PMM



Re: [Qemu-devel] [PATCH for-2.9] thread-pool: add missing qemu_bh_cancel in completion function

2017-03-17 Thread Kevin Wolf
Am 16.03.2017 um 17:02 hat Peter Lieven geschrieben:
> commit 3c80ca15 fixed a deadlock scenarion with nested aio_poll invocations.
> 
> However, the rescheduling of the completion BH introcuded unnecessary spinning
> in the main-loop. On very fast file backends this can even lead to the
> "WARNING: I/O thread spun for 1000 iterations" message popping up.
> 
> Callgrind reports about 3-4% less instructions with this patch running
> qemu-img bench on a ramdisk based VMDK file.
> 
> Fixes: 3c80ca158c96ff902a30883a8933e755988948b1
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Peter Lieven 

Thanks, applied to the block branch.

Kevin



Re: [Qemu-devel] [PATCH for-2.9] virtio: Fix error handling in virtio_bus_device_plugged

2017-03-17 Thread Cornelia Huck
On Fri, 17 Mar 2017 19:33:53 +0800
Fam Zheng  wrote:

> For one thing we shouldn't continue if an error happened, for the other
> two steps failing can cause an abort() in error_setg because we reuse
> the same errp blindly.
> 
> Add error handling checks to fix both issues.
> 
> Signed-off-by: Fam Zheng 
> ---
>  hw/virtio/virtio-bus.c | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck 




Re: [Qemu-devel] [PATCH] configure: Warn about deprecated hosts

2017-03-17 Thread Daniel P. Berrange
On Fri, Mar 17, 2017 at 11:52:01AM +, Peter Maydell wrote:
> On 17 March 2017 at 11:49, Daniel P. Berrange  wrote:
> > On Fri, Mar 17, 2017 at 11:08:22AM +, Peter Maydell wrote:
> >> We plan to drop support in a future QEMU release for host OSes
> >> and host architectures for which we have no test machine where
> >> we can build and run tests. For the 2.9 release, make configure
> >> print a warning if it is run on such a host, so that the user
> >> has some warning of the plans and can volunteer to help us
> >> maintain the port if they need it to continue to function.
> >>
> >> This commit flags up as deprecated the CPU architectures:
> >>  * ia64
> >>  * sparc
> >>  * anything which we don't have a TCG port for
> >>(and which was presumably using TCI)
> >
> > This seems to imply that if we remove supported for these architectures,
> > then TCI is no longer required either, or would there be other reasons
> > to want to keep TCI around, even when all host archs have a TCG port ?
> 
> I've never really seen the point in TCI, but others do, which
> is why it's in the tree.
> 
> >> This list is definitely too all-encompassing, and we should
> >> move at least some of the BSDs into "not-deprecated".
> >> I'm posting the patch for the moment for code review on the
> >> logic and as a placeholder.
> >
> > FreeBSD is the one where I most frequently see developers engaging across
> > the overall virt tools stack (QEMU/libvirt/virt-manager/OpenStack).
> > GNU/kFreeBSD has come up a few times in libvirt, and I don't recall seeing
> > anyone mentioning the remaining BSDs - though perhaps thats because the
> > FreeBSD port fixes are enough to also make other BSDs work.
> 
> Yeah. I'm just struggling with setting up a FreeBSD VM so we
> can compile test that. Interesting that GNU/kFreeBSD has users.

I wouldn't go so far as to say GNU/kFreeBSD has users. It had a few people
who have noticed bits of libvirt didn't compile at times, but I'm not sure
if they actually used the result, or were just building libvirt for sake of
having full platform coverage in Debian packages.

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] virtio: Fix error handling in virtio_bus_device_plugged

2017-03-17 Thread Andrew Jones
On Fri, Mar 17, 2017 at 07:33:53PM +0800, Fam Zheng wrote:
> For one thing we shouldn't continue if an error happened, for the other
> two steps failing can cause an abort() in error_setg because we reuse
> the same errp blindly.
> 
> Add error handling checks to fix both issues.
> 
> Signed-off-by: Fam Zheng 
> ---
>  hw/virtio/virtio-bus.c | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index a886011..ef76919 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -25,6 +25,7 @@
>  #include "qemu/osdep.h"
>  #include "hw/hw.h"
>  #include "qemu/error-report.h"
> +#include "qapi/error.h"
>  #include "hw/qdev.h"
>  #include "hw/virtio/virtio-bus.h"
>  #include "hw/virtio/virtio.h"
> @@ -48,21 +49,34 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error 
> **errp)
>  VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
>  VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>  bool has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
> +Error *local_err = NULL;
>  
>  DPRINTF("%s: plug device.\n", qbus->name);
>  
>  if (klass->pre_plugged != NULL) {
> -klass->pre_plugged(qbus->parent, errp);
> +klass->pre_plugged(qbus->parent, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
>  }
>  
>  /* Get the features of the plugged device. */
>  assert(vdc->get_features != NULL);
>  vdev->host_features = vdc->get_features(vdev, vdev->host_features,
> -errp);
> +&local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
>  
>  if (klass->device_plugged != NULL) {
>  klass->device_plugged(qbus->parent, errp);

Did you intend to change this errp to &local_err, as well? If not,
then the body of the below hunk is unreachable, as local_err can
never be non-null there.

Thanks,
drew

>  }
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
>  
>  if (klass->get_dma_as != NULL && has_iommu) {
>  virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
> -- 
> 2.9.3
> 
> 



Re: [Qemu-devel] [PATCH for-2.9] virtio: Fix error handling in virtio_bus_device_plugged

2017-03-17 Thread Cornelia Huck
On Fri, 17 Mar 2017 13:18:16 +0100
Andrew Jones  wrote:

> On Fri, Mar 17, 2017 at 07:33:53PM +0800, Fam Zheng wrote:
> > For one thing we shouldn't continue if an error happened, for the other
> > two steps failing can cause an abort() in error_setg because we reuse
> > the same errp blindly.
> > 
> > Add error handling checks to fix both issues.
> > 
> > Signed-off-by: Fam Zheng 
> > ---
> >  hw/virtio/virtio-bus.c | 18 --
> >  1 file changed, 16 insertions(+), 2 deletions(-)

> >  
> >  if (klass->device_plugged != NULL) {
> >  klass->device_plugged(qbus->parent, errp);
> 
> Did you intend to change this errp to &local_err, as well? If not,
> then the body of the below hunk is unreachable, as local_err can
> never be non-null there.

Oh, I managed to overlook this.

So my r-b applies if this is changed to use local_err :)

> 
> Thanks,
> drew
> 
> >  }
> > +if (local_err) {
> > +error_propagate(errp, local_err);
> > +return;
> > +}
> >  
> >  if (klass->get_dma_as != NULL && has_iommu) {
> >  virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
> > -- 
> > 2.9.3
> > 
> > 
> 
> 




Re: [Qemu-devel] [PATCH for-2.9] virtio: Fix error handling in virtio_bus_device_plugged

2017-03-17 Thread Fam Zheng
On Fri, 03/17 13:18, Andrew Jones wrote:
> On Fri, Mar 17, 2017 at 07:33:53PM +0800, Fam Zheng wrote:
> > For one thing we shouldn't continue if an error happened, for the other
> > two steps failing can cause an abort() in error_setg because we reuse
> > the same errp blindly.
> > 
> > Add error handling checks to fix both issues.
> > 
> > Signed-off-by: Fam Zheng 
> > ---
> >  hw/virtio/virtio-bus.c | 18 --
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> > index a886011..ef76919 100644
> > --- a/hw/virtio/virtio-bus.c
> > +++ b/hw/virtio/virtio-bus.c
> > @@ -25,6 +25,7 @@
> >  #include "qemu/osdep.h"
> >  #include "hw/hw.h"
> >  #include "qemu/error-report.h"
> > +#include "qapi/error.h"
> >  #include "hw/qdev.h"
> >  #include "hw/virtio/virtio-bus.h"
> >  #include "hw/virtio/virtio.h"
> > @@ -48,21 +49,34 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, 
> > Error **errp)
> >  VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
> >  VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> >  bool has_iommu = virtio_host_has_feature(vdev, 
> > VIRTIO_F_IOMMU_PLATFORM);
> > +Error *local_err = NULL;
> >  
> >  DPRINTF("%s: plug device.\n", qbus->name);
> >  
> >  if (klass->pre_plugged != NULL) {
> > -klass->pre_plugged(qbus->parent, errp);
> > +klass->pre_plugged(qbus->parent, &local_err);
> > +if (local_err) {
> > +error_propagate(errp, local_err);
> > +return;
> > +}
> >  }
> >  
> >  /* Get the features of the plugged device. */
> >  assert(vdc->get_features != NULL);
> >  vdev->host_features = vdc->get_features(vdev, vdev->host_features,
> > -errp);
> > +&local_err);
> > +if (local_err) {
> > +error_propagate(errp, local_err);
> > +return;
> > +}
> >  
> >  if (klass->device_plugged != NULL) {
> >  klass->device_plugged(qbus->parent, errp);
> 
> Did you intend to change this errp to &local_err, as well? If not,
> then the body of the below hunk is unreachable, as local_err can
> never be non-null there.

Yes, this is an overlook.

Fam

> 
> Thanks,
> drew
> 
> >  }
> > +if (local_err) {
> > +error_propagate(errp, local_err);
> > +return;
> > +}
> >  
> >  if (klass->get_dma_as != NULL && has_iommu) {
> >  virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM);
> > -- 
> > 2.9.3
> > 
> > 



[Qemu-devel] [PATCH for-2.9 v2] virtio: Fix error handling in virtio_bus_device_plugged

2017-03-17 Thread Fam Zheng
For one thing we shouldn't continue if an error happened, for the other
two steps failing can cause an abort() in error_setg because we reuse
the same errp blindly.

Add error handling checks to fix both issues.

Signed-off-by: Fam Zheng 

---

v2: Don't forget the last errp -> &local_err. [Drew]
---
 hw/virtio/virtio-bus.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index a886011..3042232 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -25,6 +25,7 @@
 #include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "qemu/error-report.h"
+#include "qapi/error.h"
 #include "hw/qdev.h"
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio.h"
@@ -48,20 +49,33 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error 
**errp)
 VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
 VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
 bool has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+Error *local_err = NULL;
 
 DPRINTF("%s: plug device.\n", qbus->name);
 
 if (klass->pre_plugged != NULL) {
-klass->pre_plugged(qbus->parent, errp);
+klass->pre_plugged(qbus->parent, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
 }
 
 /* Get the features of the plugged device. */
 assert(vdc->get_features != NULL);
 vdev->host_features = vdc->get_features(vdev, vdev->host_features,
-errp);
+&local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
 
 if (klass->device_plugged != NULL) {
-klass->device_plugged(qbus->parent, errp);
+klass->device_plugged(qbus->parent, &local_err);
+}
+if (local_err) {
+error_propagate(errp, local_err);
+return;
 }
 
 if (klass->get_dma_as != NULL && has_iommu) {
-- 
2.9.3




Re: [Qemu-devel] [PATCH] configure: Warn about deprecated hosts

2017-03-17 Thread Andrew Jones
On Fri, Mar 17, 2017 at 11:49:53AM +, Daniel P. Berrange wrote:
> On Fri, Mar 17, 2017 at 11:08:22AM +, Peter Maydell wrote:
> > This list is definitely too all-encompassing, and we should
> > move at least some of the BSDs into "not-deprecated".
> > I'm posting the patch for the moment for code review on the
> > logic and as a placeholder.
> 
> FreeBSD is the one where I most frequently see developers engaging across
> the overall virt tools stack (QEMU/libvirt/virt-manager/OpenStack).
> GNU/kFreeBSD has come up a few times in libvirt, and I don't recall seeing
> anyone mentioning the remaining BSDs - though perhaps thats because the
> FreeBSD port fixes are enough to also make other BSDs work.

OpenBSD just came up this week regarding kvm-unit-tests. Someone is
attempting to test vmd, the openbsd hypervisor, using the openbsd port
of qemu and kvm-unit-tests.

(just pointing that out, not making a deprecate/don't-deprecate statement)

Thanks,
drew



Re: [Qemu-devel] [PATCH for-2.9 v2] virtio: Fix error handling in virtio_bus_device_plugged

2017-03-17 Thread Cornelia Huck
On Fri, 17 Mar 2017 20:32:42 +0800
Fam Zheng  wrote:

> For one thing we shouldn't continue if an error happened, for the other
> two steps failing can cause an abort() in error_setg because we reuse
> the same errp blindly.
> 
> Add error handling checks to fix both issues.
> 
> Signed-off-by: Fam Zheng 
> 
> ---
> 
> v2: Don't forget the last errp -> &local_err. [Drew]
> ---
>  hw/virtio/virtio-bus.c | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)

Reviewed-by: Cornelia Huck 




Re: [Qemu-devel] [PATCH for-2.9 v2] virtio: Fix error handling in virtio_bus_device_plugged

2017-03-17 Thread Andrew Jones
On Fri, Mar 17, 2017 at 08:32:42PM +0800, Fam Zheng wrote:
> For one thing we shouldn't continue if an error happened, for the other
> two steps failing can cause an abort() in error_setg because we reuse
> the same errp blindly.
> 
> Add error handling checks to fix both issues.
> 
> Signed-off-by: Fam Zheng 
> 
> ---
> 
> v2: Don't forget the last errp -> &local_err. [Drew]
> ---
>  hw/virtio/virtio-bus.c | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index a886011..3042232 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -25,6 +25,7 @@
>  #include "qemu/osdep.h"
>  #include "hw/hw.h"
>  #include "qemu/error-report.h"
> +#include "qapi/error.h"
>  #include "hw/qdev.h"
>  #include "hw/virtio/virtio-bus.h"
>  #include "hw/virtio/virtio.h"
> @@ -48,20 +49,33 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error 
> **errp)
>  VirtioBusClass *klass = VIRTIO_BUS_GET_CLASS(bus);
>  VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>  bool has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
> +Error *local_err = NULL;
>  
>  DPRINTF("%s: plug device.\n", qbus->name);
>  
>  if (klass->pre_plugged != NULL) {
> -klass->pre_plugged(qbus->parent, errp);
> +klass->pre_plugged(qbus->parent, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
>  }
>  
>  /* Get the features of the plugged device. */
>  assert(vdc->get_features != NULL);
>  vdev->host_features = vdc->get_features(vdev, vdev->host_features,
> -errp);
> +&local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
>  
>  if (klass->device_plugged != NULL) {
> -klass->device_plugged(qbus->parent, errp);
> +klass->device_plugged(qbus->parent, &local_err);
> +}
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
>  }
>  
>  if (klass->get_dma_as != NULL && has_iommu) {
> -- 
> 2.9.3
> 
>

Reviewed-by: Andrew Jones 



Re: [Qemu-devel] [PATCH 13/16] migration: Create thread infrastructure for multifd recv side

2017-03-17 Thread Dr. David Alan Gilbert
* Paolo Bonzini (pbonz...@redhat.com) wrote:
> 
> 
> On 13/03/2017 13:44, Juan Quintela wrote:
> >  case RAM_SAVE_FLAG_MULTIFD_PAGE:
> >  fd_num = qemu_get_be16(f);
> > -if (fd_num != 0) {
> > -/* this is yet an unused variable, changed later */
> > -fd_num = fd_num;
> > -}
> > +multifd_recv_page(host, fd_num);
> >  qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
> >  break;
> 
> I still believe this design is a mistake.

Is it a use of a separate FD carrying all of the flags/addresses that
you object to?

Dave

> Paolo
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PULL 0/2] submodule-update queue 20170303

2017-03-17 Thread James Hanley
Thanks - that seemed to resolve the issue.  Will the synchronization
scripts be updated for all repositories? I was also seeing the same issue
when references for submodules roms/SLOF and roms/seabios were updated.
-Jim

On Wed, Mar 15, 2017 at 1:02 PM, Jeff Cody  wrote:

> On Wed, Mar 15, 2017 at 04:51:07PM +, Peter Maydell wrote:
> > On 15 March 2017 at 15:30, James Hanley  wrote:
> > > It would appear that the HTTP versions of the repo URLs are /not/
> > > referencing the same backend
> > >
> > > jim@jim-VirtualBox:~$ git ls-remote http://git.qemu-project.org/
> git/dtc.git
> > > 3b9c97093d6e1067f4d24d2bff32f3dd24e0751e HEAD
> > > 827ac8eca016c39b13fc916bbdb16f9f2fe3c34c refs/heads/expressions
> > > 3b9c97093d6e1067f4d24d2bff32f3dd24e0751e refs/heads/master
> > > 17dab7023959256224e800dd77cae57d8ecfaec0 refs/heads/multi-v1-tags
> > > 5426757b714e4c142da488fe685220b732f69d7b refs/heads/testing
> > > 827ac8eca016c39b13fc916bbdb16f9f2fe3c34c refs/remotes/github/
> expressions
> > > 3b9c97093d6e1067f4d24d2bff32f3dd24e0751e refs/remotes/github/master
> > > 17dab7023959256224e800dd77cae57d8ecfaec0 refs/remotes/github/multi-v1-
> tags
> > > 5426757b714e4c142da488fe685220b732f69d7b refs/remotes/github/testing
> > > 827ac8eca016c39b13fc916bbdb16f9f2fe3c34c refs/remotes/origin/
> expressions
> > > 120775eb1cf39f8dcecd695c3ff1cfef8aeb669d refs/remotes/origin/master
> > > 17dab7023959256224e800dd77cae57d8ecfaec0 refs/remotes/origin/multi-v1-
> tags
> > > f5aa792d81f5911eff088e4f88c0cd0a11ea9ca0 refs/tags/dwg-last
> > > 0a1018321b08f89d0f1942c77802aa777a82d437 refs/tags/v1.0.0
> > > 5cb1fbdd7cf82e1909e27c81073cf3272cb63fa3 refs/tags/v1.0.0^{}
> > > 8e4751ca3600a2d82365e7e9d806f2bab9b81d56 refs/tags/v1.0.0-rc1
> > > 74ce242bf3307c7ec77b9ddfff443c247ac8c0a3 refs/tags/v1.0.0-rc1^{}
> > > 38738612dec55c0262de2192cbe655f499b8c5de refs/tags/v1.1.0
> > > 202863e4dd681d17c06a82943f49485bf7860633 refs/tags/v1.1.0^{}
> > > 2d38c152a6cbcd6fcd7a2f2535d3bc8860c975f9 refs/tags/v1.1.0-rc1
> > > 7364cc79b5fa11e416dce01802139bc87d690118 refs/tags/v1.1.0-rc1^{}
> > > 427b0062114703674688aa581d13499b1b2da896 refs/tags/v1.2.0
> > > 52c356d81b1b5b5426f53655e782c37793c3637e refs/tags/v1.2.0^{}
> > > 33ea8e2705c6905edcabda65dfa92af56716056b refs/tags/v1.2.0-rc1
> > > f8bf4bfc8796b46e6086a52f0cd6c1f9ed58645a refs/tags/v1.2.0-rc1^{}
> > > a532a5d2148e6a644bb56f7aa3d29297d19e30de refs/tags/v1.2.0-rc2
> > > 17773b0e5148c5ae281ee21492c871292cb7de20 refs/tags/v1.2.0-rc2^{}
> > > 00e38ce99a600e146aa20eac082b8d7d8ec70711 refs/tags/v1.3.0
> > > bc895d6d09695d05ceb8b52486ffe861d6cfbdde refs/tags/v1.3.0^{}
> > > 6d109a2e4885896d2665d4bbcc5bc985110b0950 refs/tags/v1.4.0
> > > 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf refs/tags/v1.4.0^{}
> > > 29a9b5177c0bc192f7881b940932f903aca9c360 refs/tags/v1.4.1
> > > 302fca9f4c283e1994cf0a5a9ce1cf43ca15e6d2 refs/tags/v1.4.1^{}
> > > 24ec6e01bca89179d744d836fe94f2b459abd03d refs/tags/v1.4.2
> > > ec02b34c05be04f249ffaaca4b666f5246877dea refs/tags/v1.4.2^{}
> > > jim@jim-VirtualBox:~$
> > >
> > >
> > > Note that the tags for 1.4.3 and 1.4.4 are missing... Are the tags not
> > > "fetched" with the synchronization?
> >
> > Jeff, Stefan -- is something odd with the HTTP versions here?
> >
>
> Hi,
>
> Yes - the post-update hooks to populate the info/refs file (needed for
> http)
> is  not invokled when we update from our script.  I just did it manually
> for
> dtc.git, and I am now updating the script so that this will happen
> automatically.
>
> (The tags should now be present)
>
> Thanks,
> -Jeff
>


Re: [Qemu-devel] [PATCH] configure: Warn about deprecated hosts

2017-03-17 Thread Andrea Bolognani
On Fri, 2017-03-17 at 11:56 +, Daniel P. Berrange wrote:
> > Yeah. I'm just struggling with setting up a FreeBSD VM so we
> > can compile test that. Interesting that GNU/kFreeBSD has users.
> 
> I wouldn't go so far as to say GNU/kFreeBSD has users. It had a few people
> who have noticed bits of libvirt didn't compile at times, but I'm not sure
> if they actually used the result, or were just building libvirt for sake of
> having full platform coverage in Debian packages.

GNU/kFreeBSD is no longer a release architecture as of
Debian 8.

-- 
Andrea Bolognani / Red Hat / Virtualization



[Qemu-devel] [PULL 3/8] file-posix: Don't leak fd in hdev_get_max_segments

2017-03-17 Thread Kevin Wolf
From: Fam Zheng 

This fixes a leaked fd introduced in commit 9103f1ce.

Signed-off-by: Fam Zheng 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 block/file-posix.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index ac6bd9f..53febd3 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -703,6 +703,9 @@ static int hdev_get_max_segments(const struct stat *st)
 }
 
 out:
+if (fd != -1) {
+close(fd);
+}
 g_free(sysfspath);
 return ret;
 #else
-- 
1.8.3.1




[Qemu-devel] [PULL 4/8] block: Always call bdrv_child_check_perm first

2017-03-17 Thread Kevin Wolf
From: Fam Zheng 

bdrv_child_set_perm alone is not very usable because the caller must
call bdrv_child_check_perm first. This is already encapsulated
conveniently in bdrv_child_try_set_perm, so remove the other prototypes
from the header and fix the one wrong caller, block/mirror.c.

Signed-off-by: Fam Zheng 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 block.c   | 13 +
 block/mirror.c|  6 --
 include/block/block_int.h |  4 
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index cb57370..a77e8a0 100644
--- a/block.c
+++ b/block.c
@@ -1393,6 +1393,11 @@ static int bdrv_fill_options(QDict **options, const char 
*filename,
 return 0;
 }
 
+static int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
+ GSList *ignore_children, Error **errp);
+static void bdrv_child_abort_perm_update(BdrvChild *c);
+static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
+
 /*
  * Check whether permissions on this node can be changed in a way that
  * @cumulative_perms and @cumulative_shared_perms are the new cumulative
@@ -1615,8 +1620,8 @@ static int bdrv_check_update_perm(BlockDriverState *bs, 
uint64_t new_used_perm,
 
 /* Needs to be followed by a call to either bdrv_child_set_perm() or
  * bdrv_child_abort_perm_update(). */
-int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
-  GSList *ignore_children, Error **errp)
+static int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
+ GSList *ignore_children, Error **errp)
 {
 int ret;
 
@@ -1627,7 +1632,7 @@ int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, 
uint64_t shared,
 return ret;
 }
 
-void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared)
+static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared)
 {
 uint64_t cumulative_perms, cumulative_shared_perms;
 
@@ -1639,7 +1644,7 @@ void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, 
uint64_t shared)
 bdrv_set_perm(c->bs, cumulative_perms, cumulative_shared_perms);
 }
 
-void bdrv_child_abort_perm_update(BdrvChild *c)
+static void bdrv_child_abort_perm_update(BdrvChild *c)
 {
 bdrv_abort_perm_update(c->bs);
 }
diff --git a/block/mirror.c b/block/mirror.c
index 4f3a5cb..ca4baa5 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -574,7 +574,8 @@ static void mirror_exit(BlockJob *job, void *opaque)
  * valid. Also give up permissions on mirror_top_bs->backing, which might
  * block the removal. */
 block_job_remove_all_bdrv(job);
-bdrv_child_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL);
+bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
+&error_abort);
 bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort);
 
 /* We just changed the BDS the job BB refers to (with either or both of the
@@ -1245,7 +1246,8 @@ fail:
 block_job_unref(&s->common);
 }
 
-bdrv_child_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL);
+bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
+&error_abort);
 bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort);
 }
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 6c699ac..59400bd 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -889,10 +889,6 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState 
*child_bs,
   void *opaque, Error **errp);
 void bdrv_root_unref_child(BdrvChild *child);
 
-int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
-  GSList *ignore_children, Error **errp);
-void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
-void bdrv_child_abort_perm_update(BdrvChild *c);
 int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
 Error **errp);
 
-- 
1.8.3.1




[Qemu-devel] [PULL 6/8] block: Propagate error in bdrv_open_backing_file

2017-03-17 Thread Kevin Wolf
From: Fam Zheng 

Signed-off-by: Fam Zheng 
Reviewed-by: Alberto Garcia 
Signed-off-by: Kevin Wolf 
---
 block.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block.c b/block.c
index a77e8a0..e538084 100644
--- a/block.c
+++ b/block.c
@@ -2030,6 +2030,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*parent_options,
 bdrv_set_backing_hd(bs, backing_hd, &local_err);
 bdrv_unref(backing_hd);
 if (local_err) {
+error_propagate(errp, local_err);
 ret = -EINVAL;
 goto free_exit;
 }
-- 
1.8.3.1




[Qemu-devel] [PULL 7/8] thread-pool: add missing qemu_bh_cancel in completion function

2017-03-17 Thread Kevin Wolf
From: Peter Lieven 

commit 3c80ca15 fixed a deadlock scenarion with nested aio_poll invocations.

However, the rescheduling of the completion BH introcuded unnecessary spinning
in the main-loop. On very fast file backends this can even lead to the
"WARNING: I/O thread spun for 1000 iterations" message popping up.

Callgrind reports about 3-4% less instructions with this patch running
qemu-img bench on a ramdisk based VMDK file.

Fixes: 3c80ca158c96ff902a30883a8933e755988948b1
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven 
Signed-off-by: Kevin Wolf 
---
 util/thread-pool.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/util/thread-pool.c b/util/thread-pool.c
index ce6cd30..610646d 100644
--- a/util/thread-pool.c
+++ b/util/thread-pool.c
@@ -188,6 +188,13 @@ restart:
 aio_context_release(pool->ctx);
 elem->common.cb(elem->common.opaque, elem->ret);
 aio_context_acquire(pool->ctx);
+
+/* We can safely cancel the completion_bh here regardless of 
someone
+ * else having scheduled it meanwhile because we reenter the
+ * completion function anyway (goto restart).
+ */
+qemu_bh_cancel(pool->completion_bh);
+
 qemu_aio_unref(elem);
 goto restart;
 } else {
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH v3 5/9] xen/9pfs: connect to the frontend

2017-03-17 Thread Greg Kurz
On Thu, 16 Mar 2017 13:01:54 -0700
Stefano Stabellini  wrote:

> Write the limits of the backend to xenstore. Connect to the frontend.
> Upon connection, allocate the rings according to the protocol
> specification.
> 
> Initialize a QEMUBH to schedule work upon receiving an event channel
> notification from the frontend.
> 
> Signed-off-by: Stefano Stabellini 
> CC: anthony.per...@citrix.com
> CC: jgr...@suse.com
> CC: Aneesh Kumar K.V 
> CC: Greg Kurz 
> ---
>  hw/9pfs/xen-9p-backend.c | 182 
> ++-
>  1 file changed, 181 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
> index 92bb805..3fd20ff 100644
> --- a/hw/9pfs/xen-9p-backend.c
> +++ b/hw/9pfs/xen-9p-backend.c
> @@ -23,8 +23,39 @@
>  #define XEN_9PFS_RING_SIZE  XEN_FLEX_RING_SIZE(XEN_9PFS_RING_ORDER)
>  DEFINE_XEN_FLEX_RING_AND_INTF(xen_9pfs);
>  
> +#define VERSIONS "1"
> +#define MAX_RINGS 8
> +#define MAX_RING_ORDER 8
> +
> +typedef struct Xen9pfsRing {
> +struct Xen9pfsDev *priv;
> +
> +int ref;
> +xenevtchn_handle   *evtchndev;
> +int evtchn;
> +int local_port;
> +struct xen_9pfs_data_intf *intf;
> +unsigned char *data;
> +struct xen_9pfs_data ring;
> +
> +QEMUBH *bh;
> +
> +/* local copies, so that we can read/write PDU data directly from
> + * the ring */
> +RING_IDX out_cons, out_size, in_cons;
> +bool inprogress;
> +} Xen9pfsRing;
> +
>  typedef struct Xen9pfsDev {
>  struct XenDevice xendev;  /* must be first */
> +V9fsState state;
> +char *path;
> +char *security_model;
> +char *tag;
> +char *id;
> +
> +int num_rings;
> +Xen9pfsRing *rings;
>  } Xen9pfsDev;
>  
>  static ssize_t xen_9pfs_pdu_vmarshal(V9fsPDU *pdu,
> @@ -73,22 +104,171 @@ static int xen_9pfs_init(struct XenDevice *xendev)
>  return 0;
>  }
>  
> +static void xen_9pfs_bh(void *opaque)
> +{
> +}
> +
> +static void xen_9pfs_evtchn_event(void *opaque)
> +{
> +}
> +
>  static int xen_9pfs_free(struct XenDevice *xendev)
>  {
> -return -1;
> +int i;
> +Xen9pfsDev *xen_9pdev = container_of(xendev, Xen9pfsDev, xendev);
> +
> +if (xen_9pdev->id != NULL) {
> +g_free(xen_9pdev->id);
> +}
> +if (xen_9pdev->tag != NULL) {
> +g_free(xen_9pdev->tag);
> +}
> +if (xen_9pdev->path != NULL) {
> +g_free(xen_9pdev->path);
> +}
> +if (xen_9pdev->security_model != NULL) {
> +g_free(xen_9pdev->security_model);
> +}

You don't need the if's since g_free(NULL) is legal.

> +
> +for (i = 0; i < xen_9pdev->num_rings; i++) {
> +if (xen_9pdev->rings[i].data != NULL) {
> +xengnttab_unmap(xen_9pdev->xendev.gnttabdev,
> +xen_9pdev->rings[i].data,
> +(1 << XEN_9PFS_RING_ORDER));
> +}
> +if (xen_9pdev->rings[i].intf != NULL) {
> +xengnttab_unmap(xen_9pdev->xendev.gnttabdev,
> +xen_9pdev->rings[i].intf,
> +1);
> +}
> +if (xen_9pdev->rings[i].evtchndev > 0) {
> +qemu_set_fd_handler(xenevtchn_fd(xen_9pdev->rings[i].evtchndev),
> +NULL, NULL, NULL);
> +xenevtchn_unbind(xen_9pdev->rings[i].evtchndev,
> + xen_9pdev->rings[i].local_port);
> +}
> +if (xen_9pdev->rings[i].bh != NULL) {
> +qemu_bh_delete(xen_9pdev->rings[i].bh);
> +}
> +}
> +g_free(xen_9pdev->rings);
> +return 0;
>  }
>  
>  static int xen_9pfs_connect(struct XenDevice *xendev)
>  {
> +int i;
> +Xen9pfsDev *xen_9pdev = container_of(xendev, Xen9pfsDev, xendev);
> +V9fsState *s = &xen_9pdev->state;
> +QemuOpts *fsdev;
> +
> +if (xenstore_read_fe_int(&xen_9pdev->xendev, "num-rings",
> + &xen_9pdev->num_rings) == -1 ||
> +xen_9pdev->num_rings > MAX_RINGS || xen_9pdev->num_rings < 1) {
> +return -1;
> +}
> +
> +xen_9pdev->rings = g_malloc0(xen_9pdev->num_rings * sizeof(Xen9pfsRing));
> +for (i = 0; i < xen_9pdev->num_rings; i++) {
> +char *str;
> +
> +xen_9pdev->rings[i].priv = xen_9pdev;
> +xen_9pdev->rings[i].evtchn = -1;
> +xen_9pdev->rings[i].local_port = -1;
> +
> +str = g_strdup_printf("ring-ref%u", i);
> +if (xenstore_read_fe_int(&xen_9pdev->xendev, str,
> + &xen_9pdev->rings[i].ref) == -1) {
> +goto out;
> +}
> +g_free(str);
> +str = g_strdup_printf("event-channel-%u", i);
> +if (xenstore_read_fe_int(&xen_9pdev->xendev, str,
> + &xen_9pdev->rings[i].evtchn) == -1) {
> +goto out;
> +}
> +g_free(str);
> +
> +xen_9pdev->rings[i].intf =  xengnttab_map_grant_ref(
> +xen_9pdev->xendev.gnttabdev,
> +xen_9pdev->xendev.dom,
> +xen_

Re: [Qemu-devel] [PATCH] configure: Warn about deprecated hosts

2017-03-17 Thread Thomas Huth
On 17.03.2017 12:52, Peter Maydell wrote:
> On 17 March 2017 at 11:49, Daniel P. Berrange  wrote:
>> On Fri, Mar 17, 2017 at 11:08:22AM +, Peter Maydell wrote:
>>> We plan to drop support in a future QEMU release for host OSes
>>> and host architectures for which we have no test machine where
>>> we can build and run tests. For the 2.9 release, make configure
>>> print a warning if it is run on such a host, so that the user
>>> has some warning of the plans and can volunteer to help us
>>> maintain the port if they need it to continue to function.

I like your patch! ... but instead of completely aborting with
"Unsupported host OS" when an unexpected host operating system has been
detected, I'd maybe only print a warning and continue the configure
process - in case it is a POSIX-compatible system, there is at least a
small chance that QEMU can be compiled there.

>>> This commit flags up as deprecated the CPU architectures:
>>>  * ia64
>>>  * sparc
>>>  * anything which we don't have a TCG port for
>>>(and which was presumably using TCI)
>>
>> This seems to imply that if we remove supported for these architectures,
>> then TCI is no longer required either, or would there be other reasons
>> to want to keep TCI around, even when all host archs have a TCG port ?
> 
> I've never really seen the point in TCI, but others do, which
> is why it's in the tree.

TCI is sometimes useful when debugging problems with TCG backends - you
then have something else to compare the behavior with. I think it might
also be useful when porting QEMU to new architectures.

So could you maybe change your patch that it does not warn when the user
has run configure with the "--enable-tcg-interpreter" option?

>>> This list is definitely too all-encompassing, and we should
>>> move at least some of the BSDs into "not-deprecated".

Right, I just had a look at the changelog, and people contributed at
least patches for FreeBSD and OpenBSD last year, so I'd move at least
those two to the "not-deprecated" list (and I guess at least the other
BSDs are also close enough to these two to be kept).

 Thomas




[Qemu-devel] [PULL 2/8] replication: clarify permissions

2017-03-17 Thread Kevin Wolf
From: Changlong Xie 

Even if hidden_disk, secondary_disk are backing files, they all need
write permissions in replication scenario. Otherwise we will encouter
below exceptions on secondary side during adding nbd server:

{'execute': 'nbd-server-add', 'arguments': {'device': 'colo-disk', 'writable': 
true } }
{"error": {"class": "GenericError", "desc": "Conflicts with use by 
hidden-qcow2-driver as 'backing', which does not allow 'write' on 
sec-qcow2-driver-for-nbd"}}

CC: Zhang Hailiang 
CC: Zhang Chen 
CC: Wen Congyang 
Signed-off-by: Changlong Xie 
Signed-off-by: Kevin Wolf 
---
 block/replication.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/block/replication.c b/block/replication.c
index 22f170f..bf3c395 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -155,6 +155,18 @@ static void replication_close(BlockDriverState *bs)
 replication_remove(s->rs);
 }
 
+static void replication_child_perm(BlockDriverState *bs, BdrvChild *c,
+   const BdrvChildRole *role,
+   uint64_t perm, uint64_t shared,
+   uint64_t *nperm, uint64_t *nshared)
+{
+*nperm = *nshared = BLK_PERM_CONSISTENT_READ \
+| BLK_PERM_WRITE \
+| BLK_PERM_WRITE_UNCHANGED;
+
+return;
+}
+
 static int64_t replication_getlength(BlockDriverState *bs)
 {
 return bdrv_getlength(bs->file->bs);
@@ -660,7 +672,7 @@ BlockDriver bdrv_replication = {
 
 .bdrv_open  = replication_open,
 .bdrv_close = replication_close,
-.bdrv_child_perm= bdrv_filter_default_perms,
+.bdrv_child_perm= replication_child_perm,
 
 .bdrv_getlength = replication_getlength,
 .bdrv_co_readv  = replication_co_readv,
-- 
1.8.3.1




[Qemu-devel] [PULL 1/8] file-posix: clean up max_segments buffer termination

2017-03-17 Thread Kevin Wolf
From: Stefan Hajnoczi 

The following pattern is unsafe:

  char buf[32];
  ret = read(fd, buf, sizeof(buf));
  ...
  buf[ret] = 0;

If read(2) returns 32 then a byte beyond the end of the buffer is
zeroed.

In practice this buffer overflow does not occur because the sysfs
max_segments file only contains an unsigned short + '\n'.  The string is
always shorter than 32 bytes.

Regardless, avoid this pattern because static analysis tools might
complain and it could lead to real buffer overflows if copy-pasted
elsewhere in the codebase.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Kevin Wolf 
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index c4c0663..ac6bd9f 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -686,7 +686,7 @@ static int hdev_get_max_segments(const struct stat *st)
 goto out;
 }
 do {
-ret = read(fd, buf, sizeof(buf));
+ret = read(fd, buf, sizeof(buf) - 1);
 } while (ret == -1 && errno == EINTR);
 if (ret < 0) {
 ret = -errno;
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] configure: Warn about deprecated hosts

2017-03-17 Thread Peter Maydell
On 17 March 2017 at 13:19, Thomas Huth  wrote:
> On 17.03.2017 12:52, Peter Maydell wrote:
>> On 17 March 2017 at 11:49, Daniel P. Berrange  wrote:
>>> On Fri, Mar 17, 2017 at 11:08:22AM +, Peter Maydell wrote:
 We plan to drop support in a future QEMU release for host OSes
 and host architectures for which we have no test machine where
 we can build and run tests. For the 2.9 release, make configure
 print a warning if it is run on such a host, so that the user
 has some warning of the plans and can volunteer to help us
 maintain the port if they need it to continue to function.
>
> I like your patch! ... but instead of completely aborting with
> "Unsupported host OS" when an unexpected host operating system has been
> detected, I'd maybe only print a warning and continue the configure
> process - in case it is a POSIX-compatible system, there is at least a
> small chance that QEMU can be compiled there.

That code path tries to enable all the Linux specific stuff
(including KVM and linux-user and so on) and puts linux-headers/
on the include path. It seems very unlikely that it will actually
work even if in theory a generic-POSIX OS might be able to build
QEMU somehow.

This kind of tiny-corner-case stuff is exactly why I want
to drop any attempt to build on an OS or architecture we don't
actually know about. The 99.9% probability is that no user of
QEMU has successfully gone down that code path in configure for
anything that isn't Linux; and yet here we are arguing about what
its behaviour should be.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v2 2/3] qemu-img: fix switch indentation in img_amend()

2017-03-17 Thread Philippe Mathieu-Daudé

On 03/17/2017 07:45 AM, Stefan Hajnoczi wrote:

QEMU coding style indents 'case' to the same level as the 'switch'
statement:

  switch (foo) {
  case 1:

Fix this coding style violation so checkpatch.pl doesn't complain about
the next patch.

Signed-off-by: Stefan Hajnoczi 


Reviewed-by: Philippe Mathieu-Daudé 


---
 qemu-img.c | 82 +++---
 1 file changed, 41 insertions(+), 41 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index ce293a4..c7ffabb 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3500,47 +3500,47 @@ static int img_amend(int argc, char **argv)
 }

 switch (c) {
-case 'h':
-case '?':
-help();
-break;
-case 'o':
-if (!is_valid_option_list(optarg)) {
-error_report("Invalid option list: %s", optarg);
-ret = -1;
-goto out_no_progress;
-}
-if (!options) {
-options = g_strdup(optarg);
-} else {
-char *old_options = options;
-options = g_strdup_printf("%s,%s", options, optarg);
-g_free(old_options);
-}
-break;
-case 'f':
-fmt = optarg;
-break;
-case 't':
-cache = optarg;
-break;
-case 'p':
-progress = true;
-break;
-case 'q':
-quiet = true;
-break;
-case OPTION_OBJECT:
-opts = qemu_opts_parse_noisily(&qemu_object_opts,
-   optarg, true);
-if (!opts) {
-ret = -1;
-goto out_no_progress;
-}
-break;
-case OPTION_IMAGE_OPTS:
-image_opts = true;
-break;
+case 'h':
+case '?':
+help();
+break;
+case 'o':
+if (!is_valid_option_list(optarg)) {
+error_report("Invalid option list: %s", optarg);
+ret = -1;
+goto out_no_progress;
+}
+if (!options) {
+options = g_strdup(optarg);
+} else {
+char *old_options = options;
+options = g_strdup_printf("%s,%s", options, optarg);
+g_free(old_options);
+}
+break;
+case 'f':
+fmt = optarg;
+break;
+case 't':
+cache = optarg;
+break;
+case 'p':
+progress = true;
+break;
+case 'q':
+quiet = true;
+break;
+case OPTION_OBJECT:
+opts = qemu_opts_parse_noisily(&qemu_object_opts,
+   optarg, true);
+if (!opts) {
+ret = -1;
+goto out_no_progress;
+}
+break;
+case OPTION_IMAGE_OPTS:
+image_opts = true;
+break;
 }
 }






[Qemu-devel] [PULL 5/8] blockdev: fix bitmap clear undo

2017-03-17 Thread Kevin Wolf
From: John Snow 

Only undo the action if we actually prepared the action.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index f1f49bd..c5b2c2c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2047,7 +2047,9 @@ static void block_dirty_bitmap_clear_abort(BlkActionState 
*common)
 BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
  common, common);
 
-bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup);
+if (state->backup) {
+bdrv_undo_clear_dirty_bitmap(state->bitmap, state->backup);
+}
 }
 
 static void block_dirty_bitmap_clear_commit(BlkActionState *common)
-- 
1.8.3.1




[Qemu-devel] [PULL 0/8] Block layer fixes for 2.9.0-rc1

2017-03-17 Thread Kevin Wolf
The following changes since commit 272d7dee5951f926fad1911f2f072e5915cdcba0:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-cirrus-20170316-1' 
into staging (2017-03-16 16:40:44 +)

are available in the git repository at:


  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 11f0f5e553cff93d5fc77d67b98c40adba12b729:

  Merge remote-tracking branch 'mreitz/tags/pull-block-2017-03-17' into 
queue-block (2017-03-17 13:03:44 +0100)



Block layer fixes for 2.9.0-rc1


Changlong Xie (1):
  replication: clarify permissions

Fam Zheng (3):
  file-posix: Don't leak fd in hdev_get_max_segments
  block: Always call bdrv_child_check_perm first
  block: Propagate error in bdrv_open_backing_file

John Snow (1):
  blockdev: fix bitmap clear undo

Kevin Wolf (1):
  Merge remote-tracking branch 'mreitz/tags/pull-block-2017-03-17' into 
queue-block

Paolo Bonzini (1):
  block: quiesce AioContext when detaching from it

Peter Lieven (1):
  thread-pool: add missing qemu_bh_cancel in completion function

Stefan Hajnoczi (1):
  file-posix: clean up max_segments buffer termination

 block.c   | 21 +
 block/file-posix.c|  5 -
 block/mirror.c|  6 --
 block/replication.c   | 14 +-
 blockdev.c|  4 +++-
 include/block/block_int.h |  4 
 util/thread-pool.c|  7 +++
 7 files changed, 48 insertions(+), 13 deletions(-)



[Qemu-devel] [PULL 8/8] block: quiesce AioContext when detaching from it

2017-03-17 Thread Kevin Wolf
From: Paolo Bonzini 

While it is true that bdrv_set_aio_context only works on a single
BlockDriverState subtree (see commit message for 53ec73e, "block: Use
bdrv_drain to replace uncessary bdrv_drain_all", 2015-07-07), it works
at the AioContext level rather than the BlockDriverState level.

Therefore, it is also necessary to trigger pending bottom halves too,
even if no requests are pending.

For NBD this ensures that the aio_co_schedule of a previous call to
nbd_attach_aio_context is completed before detaching from the old
AioContext; it fixes qemu-iotest 094.  Another similar bug happens
when the VM is stopped and the virtio-blk dataplane irqfd is torn down.
In this case it's possible that guest I/O gets stuck if notify_guest_bh
was scheduled but doesn't run.

Calling aio_poll from another AioContext is safe if non-blocking; races
such as the one mentioned in the commit message for c9d1a56 ("block:
only call aio_poll on the current thread's AioContext", 2016-10-28)
are a concern for blocking calls.

I considered other options, including:

- moving the bs->wakeup mechanism to AioContext, and letting the caller
check.  This might work for virtio which has a clear place to wakeup
(notify_place_bh) and check the condition (virtio_blk_data_plane_stop).
For aio_co_schedule I couldn't find a clear place to check the condition.

- adding a dummy oneshot bottom half and waiting for it to trigger.
This has the complication that bottom half list is LIFO for historical
reasons.  There were performance issues caused by bottom half ordering
in the past, so I decided against it for 2.9.

Fixes: 99723548561978da8ef44cf804fb7912698f5d88
Reported-by: Max Reitz 
Reported-by: Halil Pasic 
Tested-by: Halil Pasic 
Signed-off-by: Paolo Bonzini 
Message-id: 2017031457.14464-2-pbonz...@redhat.com
Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 
---
 block.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/block.c b/block.c
index e538084..6e906ec 100644
--- a/block.c
+++ b/block.c
@@ -4350,8 +4350,15 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
 
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
 {
+AioContext *ctx;
+
 bdrv_drain(bs); /* ensure there are no in-flight requests */
 
+ctx = bdrv_get_aio_context(bs);
+while (aio_poll(ctx, false)) {
+/* wait for all bottom halves to execute */
+}
+
 bdrv_detach_aio_context(bs);
 
 /* This function executes in the old AioContext so acquire the new one in
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] configure: Warn about deprecated hosts

2017-03-17 Thread Peter Maydell
On 17 March 2017 at 13:19, Thomas Huth  wrote:
> So could you maybe change your patch that it does not warn when the user
> has run configure with the "--enable-tcg-interpreter" option?

Nope. I specifically don't want to enable support for TCI on random
who-knows-what-this-is architectures. It eats up too much time
causing people to worry about "what if this is some weird thing"
when they're trying to refactor code, when in practice nobody
is really using QEMU on oddball host CPUs.

If somebody wants to support QEMU as a TCI-only setup on a
new host architecture, they can submit a patch to configure
that specifically recognizes the host architecture (even
if we don't have a tcg backend for it).

thanks
-- PMM



Re: [Qemu-devel] [PULL 0/2] submodule-update queue 20170303

2017-03-17 Thread Jeff Cody
On Fri, Mar 17, 2017 at 09:06:20AM -0400, James Hanley wrote:
>Thanks - that seemed to resolve the issue.  Will the synchronization
>scripts be updated for all repositories? I was also seeing the same issue
>when references for submodules roms/SLOF and roms/seabios were updated.
>-Jim

Hi,

Thanks for the verification.  Yes, the scripts have been updated for all
repositories.

Thanks,
Jeff





Re: [Qemu-devel] [PATCH v3 7/9] xen/9pfs: implement in/out_iov_from_pdu and vmarshal/vunmarshal

2017-03-17 Thread Greg Kurz
On Thu, 16 Mar 2017 13:01:56 -0700
Stefano Stabellini  wrote:

> Implement xen_9pfs_init_in/out_iov_from_pdu and
> xen_9pfs_pdu_vmarshal/vunmarshall by creating new sg pointing to the
> data on the ring.
> 
> This is safe as we only handle one request per ring at any given time.
> 
> Signed-off-by: Stefano Stabellini 
> CC: anthony.per...@citrix.com
> CC: jgr...@suse.com
> CC: Aneesh Kumar K.V 
> CC: Greg Kurz 
> ---
>  hw/9pfs/xen-9p-backend.c | 92 
> ++--
>  1 file changed, 90 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
> index f757591..0a1eb34 100644
> --- a/hw/9pfs/xen-9p-backend.c
> +++ b/hw/9pfs/xen-9p-backend.c
> @@ -58,12 +58,78 @@ typedef struct Xen9pfsDev {
>  Xen9pfsRing *rings;
>  } Xen9pfsDev;
>  
> +static void xen_9pfs_in_sg(Xen9pfsRing *ring,
> +   struct iovec *in_sg,
> +   int *num,
> +   uint32_t idx,
> +   uint32_t size)
> +{
> +RING_IDX cons, prod, masked_prod, masked_cons;
> +
> +cons = ring->intf->in_cons;
> +prod = ring->intf->in_prod;
> +xen_rmb();
> +masked_prod = xen_9pfs_mask(prod, XEN_9PFS_RING_SIZE);
> +masked_cons = xen_9pfs_mask(cons, XEN_9PFS_RING_SIZE);
> +
> +if (masked_prod < masked_cons) {
> +in_sg[0].iov_base = ring->ring.in + masked_prod;
> +in_sg[0].iov_len = masked_cons - masked_prod;
> +*num = 1;
> +} else {
> +in_sg[0].iov_base = ring->ring.in + masked_prod;
> +in_sg[0].iov_len = XEN_9PFS_RING_SIZE - masked_prod;
> +in_sg[1].iov_base = ring->ring.in;
> +in_sg[1].iov_len = masked_cons;
> +*num = 2;
> +}
> +}
> +
> +static void xen_9pfs_out_sg(Xen9pfsRing *ring,
> +struct iovec *out_sg,
> +int *num,
> +uint32_t idx)
> +{
> +RING_IDX cons, prod, masked_prod, masked_cons;
> +
> +cons = ring->intf->out_cons;
> +prod = ring->intf->out_prod;
> +xen_rmb();
> +masked_prod = xen_9pfs_mask(prod, XEN_9PFS_RING_SIZE);
> +masked_cons = xen_9pfs_mask(cons, XEN_9PFS_RING_SIZE);
> +
> +if (masked_cons < masked_prod) {
> +out_sg[0].iov_base = ring->ring.out + masked_cons;
> +out_sg[0].iov_len = ring->out_size;
> +*num = 1;
> +} else {
> +if (ring->out_size > (XEN_9PFS_RING_SIZE - masked_cons)) {
> +out_sg[0].iov_base = ring->ring.out + masked_cons;
> +out_sg[0].iov_len = XEN_9PFS_RING_SIZE - masked_cons;
> +out_sg[1].iov_base = ring->ring.out;
> +out_sg[1].iov_len = ring->out_size -
> +(XEN_9PFS_RING_SIZE - masked_cons);
> +*num = 2;
> +} else {
> +out_sg[0].iov_base = ring->ring.out + masked_cons;
> +out_sg[0].iov_len = ring->out_size;
> +*num = 1;
> +}
> +}
> +}
> +
>  static ssize_t xen_9pfs_pdu_vmarshal(V9fsPDU *pdu,
>   size_t offset,
>   const char *fmt,
>   va_list ap)
>  {
> -return 0;
> +Xen9pfsDev *xen_9pfs = container_of(pdu->s, Xen9pfsDev, state);
> +struct iovec in_sg[2];
> +int num;
> +
> +xen_9pfs_in_sg(&xen_9pfs->rings[pdu->tag % xen_9pfs->num_rings],
> +   in_sg, &num, pdu->idx, ROUND_UP(offset + 128, 512));
> +return v9fs_iov_vmarshal(in_sg, num, offset, 0, fmt, ap);
>  }
>  
>  static ssize_t xen_9pfs_pdu_vunmarshal(V9fsPDU *pdu,
> @@ -71,13 +137,27 @@ static ssize_t xen_9pfs_pdu_vunmarshal(V9fsPDU *pdu,
> const char *fmt,
> va_list ap)
>  {
> -return 0;
> +Xen9pfsDev *xen_9pfs = container_of(pdu->s, Xen9pfsDev, state);
> +struct iovec out_sg[2];
> +int num;
> +
> +xen_9pfs_out_sg(&xen_9pfs->rings[pdu->tag % xen_9pfs->num_rings],
> +out_sg, &num, pdu->idx);
> +return v9fs_iov_vunmarshal(out_sg, num, offset, 0, fmt, ap);
>  }
>  
>  static void xen_9pfs_init_out_iov_from_pdu(V9fsPDU *pdu,
> struct iovec **piov,
> unsigned int *pniov)
>  {
> +Xen9pfsDev *xen_9pfs = container_of(pdu->s, Xen9pfsDev, state);
> +Xen9pfsRing *ring = &xen_9pfs->rings[pdu->tag % xen_9pfs->num_rings];
> +struct iovec *sg = g_malloc0(sizeof(*sg) * 2);

I had overlooked in v2, sorry for that. Where does *sg gets freed ?

> +int num;
> +
> +xen_9pfs_out_sg(ring, sg, &num, pdu->idx);
> +*piov = sg;
> +*pniov = num;
>  }
>  
>  static void xen_9pfs_init_in_iov_from_pdu(V9fsPDU *pdu,
> @@ -85,6 +165,14 @@ static void xen_9pfs_init_in_iov_from_pdu(V9fsPDU *pdu,
>unsigned int *pniov,
>

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

2017-03-17 Thread Paul Durrant
This patch adds a command-line option (-xen-domid-restrict) which will
use the new libxendevicemodel API to restrict devicemodel operations to
the specified domid.

This patch also adds a tracepoint to allow successful enabling of the
restriction to be monitored.

Signed-off-by: Paul Durrant 
---
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.
---
 hw/xen/trace-events |  1 +
 include/hw/xen/xen.h|  1 +
 include/hw/xen/xen_common.h | 23 +++
 qemu-options.hx |  6 ++
 vl.c|  8 
 xen-hvm.c   |  8 
 6 files changed, 47 insertions(+)

diff --git a/hw/xen/trace-events b/hw/xen/trace-events
index c4fb6f1..a5b5e8b 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(void) ""
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..5962bc4 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,22 @@ 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);
+
+if (rc == 0) {
+trace_xen_domid_restrict();
+return 0;
+}
+
+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..4aab077 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3354,6 +3354,10 @@ 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\n",
+QEMU_ARCH_ALL)
 STEXI
 @item -xen-domid @var{id}
 @findex -xen-domid
@@ -3366,6 +3370,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)
 }
 xen_mode = XEN_ATTACH;
 break;
+case QEMU_OPTION_xen_domid_restrict:
+if (!(xen_available())) {
+error_report("Option not supported for this target");
+exit(1);
+}
+xen_domid_restrict = true;
+break;
 case QEMU_OPTION_trace:
 g_free(trace_file);
 trace_file = trace_opt_parse(optarg);
diff --git a/xen-hvm.c b/xen-hvm.c
index 4b928cf..335e263 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -1226,6 +1226,14 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion 
**ram_memory)
 goto err;
 }
 
+if (xen_domid_restrict) {
+rc = xen_restrict(xen_domid);
+if (rc < 0) {
+error_report("failed to restrict: error %d", errno);
+goto err;
+}
+}
+
 

Re: [Qemu-devel] [PATCH] configure: Warn about deprecated hosts

2017-03-17 Thread Thomas Huth
On 17.03.2017 14:22, Peter Maydell wrote:
> On 17 March 2017 at 13:19, Thomas Huth  wrote:
>> On 17.03.2017 12:52, Peter Maydell wrote:
>>> On 17 March 2017 at 11:49, Daniel P. Berrange  wrote:
 On Fri, Mar 17, 2017 at 11:08:22AM +, Peter Maydell wrote:
> We plan to drop support in a future QEMU release for host OSes
> and host architectures for which we have no test machine where
> we can build and run tests. For the 2.9 release, make configure
> print a warning if it is run on such a host, so that the user
> has some warning of the plans and can volunteer to help us
> maintain the port if they need it to continue to function.
>>
>> I like your patch! ... but instead of completely aborting with
>> "Unsupported host OS" when an unexpected host operating system has been
>> detected, I'd maybe only print a warning and continue the configure
>> process - in case it is a POSIX-compatible system, there is at least a
>> small chance that QEMU can be compiled there.
> 
> That code path tries to enable all the Linux specific stuff
> (including KVM and linux-user and so on) and puts linux-headers/
> on the include path. It seems very unlikely that it will actually
> work even if in theory a generic-POSIX OS might be able to build
> QEMU somehow.

Ah, sorry, you got me slightly wrong. I think it is a good idea that you
replaced the "*)" with "Linux)" in your patch. I rather meant to simply
do "echo" instead of "error_exit" in the new "*)" case.

I now just tried to build QEMU with such a "fake" generic POSIX system
(by hacking configure to not recognize "Linux"), and seems like QEMU
properly compiles in that case - but linking fails because net/tap.c is
missing a tap backend.

So you're right, currently it is not possible to build QEMU on unknown
host systems, i.e. the "error_exit" is indeed justified there (though I
think it would be maybe be possible to also add stubs for missing tap
functions to make it work).

 Thomas




Re: [Qemu-devel] [PULL 0/8] Block layer fixes for 2.9.0-rc1

2017-03-17 Thread Peter Maydell
On 17 March 2017 at 13:15, Kevin Wolf  wrote:
> The following changes since commit 272d7dee5951f926fad1911f2f072e5915cdcba0:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/pull-cirrus-20170316-1' 
> into staging (2017-03-16 16:40:44 +)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 11f0f5e553cff93d5fc77d67b98c40adba12b729:
>
>   Merge remote-tracking branch 'mreitz/tags/pull-block-2017-03-17' into 
> queue-block (2017-03-17 13:03:44 +0100)
>
> 
>
> Block layer fixes for 2.9.0-rc1
>
> 

Applied, thanks.

-- PMM



[Qemu-devel] [Bug 1673722] [NEW] Reading register at offset. It is not fully implemented warning make VM impossible to use

2017-03-17 Thread Julien Duponchelle
Public bug reported:

Hi,

Since this commit:
https://github.com/qemu/qemu/commit/bc0f0674f037a01f2ce0870ad6270a356a7a8347

We can no longer use the IOSvL2 image from Cisco. The problem is we got a lot 
of warning message saying:
e1000: Reading register at offset: 0x2410. It is not fully implemented. 

User got so much of this warning that they can't use the VM.

Thanks for the help

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: e1000

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

Title:
  Reading register at offset. It is not fully implemented warning make
  VM impossible to use

Status in QEMU:
  New

Bug description:
  Hi,

  Since this commit:
  https://github.com/qemu/qemu/commit/bc0f0674f037a01f2ce0870ad6270a356a7a8347

  We can no longer use the IOSvL2 image from Cisco. The problem is we got a lot 
of warning message saying:
  e1000: Reading register at offset: 0x2410. It is not fully implemented. 

  User got so much of this warning that they can't use the VM.

  Thanks for the help

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



Re: [Qemu-devel] [PATCH] qemu: Fix -version with configure --with-pkgversion

2017-03-17 Thread Thomas Huth
On 16.03.2017 10:00, Jordan Justen wrote:
> This appears to have regressed in 67a1de0d19.
> 
> When the configure --with-pkgversion=foo option is used, the output
> from -version will look like:
> 
> QEMU emulator version 2.8.90(foo)
> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
> 
> After this patch, it will be:
> 
> QEMU emulator version 2.8.90 (foo)
> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
> 
> Cc: Paolo Bonzini 
> Cc: Fam Zheng 
> Fixes: https://bugs.launchpad.net/bugs/1673373
> Cc: 1673...@bugs.launchpad.net
> Signed-off-by: Jordan Justen 
> ---
>  Makefile  | 2 +-
>  configure | 2 +-
>  vl.c  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1c4c04f6f2..c9df119798 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -289,7 +289,7 @@ qemu-version.h: FORCE
>   printf '"$(PKGVERSION)"\n'; \
>   else \
>   if test -d .git; then \
> - printf '" ('; \
> + printf '"('; \
>   git describe --match 'v*' 2>/dev/null | tr -d 
> '\n'; \
>   if ! git diff-index --quiet HEAD &>/dev/null; 
> then \
>   printf -- '-dirty'; \
> diff --git a/configure b/configure
> index 99d8bece70..e94b06b5fc 100755
> --- a/configure
> +++ b/configure
> @@ -1004,7 +1004,7 @@ for opt do
>;;
>--disable-blobs) blobs="no"
>;;
> -  --with-pkgversion=*) pkgversion=" ($optarg)"
> +  --with-pkgversion=*) pkgversion="($optarg)"
>;;
>--with-coroutine=*) coroutine="$optarg"
>;;
> diff --git a/vl.c b/vl.c
> index 0b4ed5241c..3e60e61905 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1904,7 +1904,7 @@ static void main_loop(void)
>  
>  static void version(void)
>  {
> -printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION "\n"
> +printf("QEMU emulator version " QEMU_VERSION " " QEMU_PKGVERSION "\n"
> QEMU_COPYRIGHT "\n");
>  }

Reviewed-by: Thomas Huth 




Re: [Qemu-devel] [PATCH] vnc: fix a qio-channel leak

2017-03-17 Thread Philippe Mathieu-Daudé

On 03/17/2017 07:14 AM, Daniel P. Berrange wrote:

On Fri, Mar 17, 2017 at 01:28:02PM +0400, Marc-André Lureau wrote:

Spotted by ASAN.

Signed-off-by: Marc-André Lureau 
---
 ui/vnc.c | 1 +
 1 file changed, 1 insertion(+)


Reviewed-by: Daniel P. Berrange 



Reviewed-by: Philippe Mathieu-Daudé 


diff --git a/ui/vnc.c b/ui/vnc.c
index 8bfb1e0685..6e93b883b5 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3677,6 +3677,7 @@ static int vnc_display_listen_addr(VncDisplay *vd,
 qio_channel_set_name(QIO_CHANNEL(sioc), name);
 if (qio_channel_socket_listen_sync(
 sioc, rawaddrs[i], listenerr == NULL ? &listenerr : NULL) < 0) 
{
+object_unref(OBJECT(sioc));
 continue;
 }
 listening = true;


Regards,
Daniel





Re: [Qemu-devel] [PATCH] ui/egl-helpers: fix egl 1.5 display init

2017-03-17 Thread Philippe Mathieu-Daudé

On 03/17/2017 05:53 AM, Marc-André Lureau wrote:

On Fri, Mar 17, 2017 at 12:12 PM Gerd Hoffmann  wrote:


Unfortunaly switching to getPlatformDisplayEXT isn't as easy as
implemented by 0ea1523fb6703aa0dcd65e66b59e96fec028e60a.  See the
longish comment for the complete story.

Cc: Frediano Ziglio 
Suggested-by: Hans de Goede 
Signed-off-by: Gerd Hoffmann 
---




Reviewed-by: Marc-André Lureau 



Reviewed-by: Philippe Mathieu-Daudé 


 ui/egl-helpers.c | 53
+++--
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 584dd1b..48686cf 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -192,6 +192,51 @@ EGLSurface qemu_egl_init_surface_x11(EGLContext ectx,
Window win)

 /* --
*/

+/*
+ * Taken from glamor_egl.h from the Xorg xserver, which is MIT licensed
+ *
+ * Create an EGLDisplay from a native display type. This is a little
quirky
+ * for a few reasons.
+ *
+ * 1: GetPlatformDisplayEXT and GetPlatformDisplay are the API you want to
+ * use, but have different function signatures in the third argument; this
+ * happens not to matter for us, at the moment, but it means epoxy won't
alias
+ * them together.
+ *
+ * 2: epoxy 1.3 and earlier don't understand EGL client extensions, which
+ * means you can't call "eglGetPlatformDisplayEXT" directly, as the
resolver
+ * will crash.
+ *
+ * 3: You can't tell whether you have EGL 1.5 at this point, because
+ * eglQueryString(EGL_VERSION) is a property of the display, which we
don't
+ * have yet. So you have to query for extensions no matter what.
Fortunately
+ * epoxy_has_egl_extension _does_ let you query for client extensions, so
+ * we don't have to write our own extension string parsing.
+ *
+ * 4. There is no EGL_KHR_platform_base to complement the EXT one, thus
one
+ * needs to know EGL 1.5 is supported in order to use the
eglGetPlatformDisplay
+ * function pointer.
+ * We can workaround this (circular dependency) by probing for the EGL 1.5
+ * platform extensions (EGL_KHR_platform_gbm and friends) yet it doesn't
seem
+ * like mesa will be able to adverise these (even though it can do EGL
1.5).
+ */
+static EGLDisplay qemu_egl_get_display(void *native)
+{
+#ifdef EGL_PLATFORM_GBM_MESA
+/* In practise any EGL 1.5 implementation would support the EXT
extension */
+if (epoxy_has_egl_extension(NULL, "EGL_EXT_platform_base")) {
+PFNEGLGETPLATFORMDISPLAYEXTPROC getPlatformDisplayEXT =
+(void *) eglGetProcAddress("eglGetPlatformDisplayEXT");
+if (getPlatformDisplayEXT) {
+return getPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, native,
NULL);
+}
+}
+#endif
+
+/* fallback */
+return eglGetDisplay(native);
+}
+
 int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug)
 {
 static const EGLint conf_att_gl[] = {
@@ -222,12 +267,8 @@ int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool
gles, bool debug)
 setenv("LIBGL_DEBUG", "verbose", true);
 }

-egl_dbg("eglGetDisplay (dpy %p) ...\n", dpy);
-#ifdef EGL_MESA_platform_gbm
-qemu_egl_display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA,
dpy, NULL);
-#else
-qemu_egl_display = eglGetDisplay(dpy);
-#endif
+egl_dbg("qemu_egl_get_display (dpy %p) ...\n", dpy);
+qemu_egl_display = qemu_egl_get_display(dpy);
 if (qemu_egl_display == EGL_NO_DISPLAY) {
 error_report("egl: eglGetDisplay failed");
 return -1;
--
1.8.3.1


--

Marc-André Lureau





Re: [Qemu-devel] [PATCH] qemu: Fix -version with configure --with-pkgversion

2017-03-17 Thread Peter Maydell
On 16 March 2017 at 09:00, Jordan Justen  wrote:
> This appears to have regressed in 67a1de0d19.
>
> When the configure --with-pkgversion=foo option is used, the output
> from -version will look like:
>
> QEMU emulator version 2.8.90(foo)
> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
>
> After this patch, it will be:
>
> QEMU emulator version 2.8.90 (foo)
> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
>
> Cc: Paolo Bonzini 
> Cc: Fam Zheng 
> Fixes: https://bugs.launchpad.net/bugs/1673373
> Cc: 1673...@bugs.launchpad.net
> Signed-off-by: Jordan Justen 
> ---
>  Makefile  | 2 +-
>  configure | 2 +-
>  vl.c  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 1c4c04f6f2..c9df119798 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -289,7 +289,7 @@ qemu-version.h: FORCE
> printf '"$(PKGVERSION)"\n'; \
> else \
> if test -d .git; then \
> -   printf '" ('; \
> +   printf '"('; \
> git describe --match 'v*' 2>/dev/null | tr -d 
> '\n'; \
> if ! git diff-index --quiet HEAD &>/dev/null; 
> then \
> printf -- '-dirty'; \
> diff --git a/configure b/configure
> index 99d8bece70..e94b06b5fc 100755
> --- a/configure
> +++ b/configure
> @@ -1004,7 +1004,7 @@ for opt do
>;;
>--disable-blobs) blobs="no"
>;;
> -  --with-pkgversion=*) pkgversion=" ($optarg)"
> +  --with-pkgversion=*) pkgversion="($optarg)"
>;;
>--with-coroutine=*) coroutine="$optarg"
>;;
> diff --git a/vl.c b/vl.c
> index 0b4ed5241c..3e60e61905 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1904,7 +1904,7 @@ static void main_loop(void)
>
>  static void version(void)
>  {
> -printf("QEMU emulator version " QEMU_VERSION QEMU_PKGVERSION "\n"
> +printf("QEMU emulator version " QEMU_VERSION " " QEMU_PKGVERSION "\n"
> QEMU_COPYRIGHT "\n");
>  }

This is not the only place where we assemble a string from
QEMU_VERSION QEMU_PKGVERSION, so if you want to change from the
original approach where QEMU_PKGVERSION had a space in it then
you need to change the other places too.

Also it looks like we return QEMU_PKGVERSION as part of the
QMP qmp_query_version() code, so we should check to see what
the expected behaviour there is regarding having the space
or not.

I think when I wrote those printfs I was expecting that
QEMU_PKGVERSION might be a "-something" kind of string,
and so did whoever wrote the commit log for 67a1de0d19.
However looking at git history it seems to have always been
a " (something)" format, so obviously some confusion here...

thanks
-- PMM



  1   2   >