[Qemu-devel] [Bug 904308] Re: x86: BT/BTS/BTR/BTC: ZF flag is unaffected

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  x86: BT/BTS/BTR/BTC: ZF flag is unaffected

Status in QEMU:
  Expired

Bug description:
  Hello!

  Bug was found in qemu.git.
  See target-i386/translate.c:

  case 0x1ba: /* bt/bts/btr/btc Gv, im */
  ot = dflag + OT_WORD;
  modrm = ldub_code(s->pc++);
  op = (modrm >> 3) & 7;
  mod = (modrm >> 6) & 3;
  rm = (modrm & 7) | REX_B(s);
  if (mod != 3) {
  s->rip_offset = 1;
  gen_lea_modrm(s, modrm, _addr, _addr);
  gen_op_ld_T0_A0(ot + s->mem_index);
  } else {
  gen_op_mov_TN_reg(ot, 0, rm);
  }
  /* load shift */
  val = ldub_code(s->pc++);
  gen_op_movl_T1_im(val);
  if (op < 4)
  goto illegal_op;
  op -= 4;
  goto bt_op;
  case 0x1a3: /* bt Gv, Ev */
  op = 0;
  goto do_btx;
  case 0x1ab: /* bts */
  op = 1;
  goto do_btx;
  case 0x1b3: /* btr */
  op = 2;
  goto do_btx;
  case 0x1bb: /* btc */
  op = 3;
  do_btx:
  ot = dflag + OT_WORD;
  modrm = ldub_code(s->pc++);
  reg = ((modrm >> 3) & 7) | rex_r;
  mod = (modrm >> 6) & 3;
  rm = (modrm & 7) | REX_B(s);
  gen_op_mov_TN_reg(OT_LONG, 1, reg);
  if (mod != 3) {
  gen_lea_modrm(s, modrm, _addr, _addr);
  /* specific case: we need to add a displacement */
  gen_exts(ot, cpu_T[1]);
  tcg_gen_sari_tl(cpu_tmp0, cpu_T[1], 3 + ot);
  tcg_gen_shli_tl(cpu_tmp0, cpu_tmp0, ot);
  tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0);
  gen_op_ld_T0_A0(ot + s->mem_index);
  } else {
  gen_op_mov_TN_reg(ot, 0, rm);
  }
  bt_op:
  tcg_gen_andi_tl(cpu_T[1], cpu_T[1], (1 << (3 + ot)) - 1);
  switch(op) {
  case 0:
  tcg_gen_shr_tl(cpu_cc_src, cpu_T[0], cpu_T[1]);
  tcg_gen_movi_tl(cpu_cc_dst, 0);   
<< always set zf
  break;
  case 1:
  tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
  tcg_gen_movi_tl(cpu_tmp0, 1);
  tcg_gen_shl_tl(cpu_tmp0, cpu_tmp0, cpu_T[1]);
  tcg_gen_or_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
  break;
  case 2:
  tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
  tcg_gen_movi_tl(cpu_tmp0, 1);
  tcg_gen_shl_tl(cpu_tmp0, cpu_tmp0, cpu_T[1]);
  tcg_gen_not_tl(cpu_tmp0, cpu_tmp0);
  tcg_gen_and_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
  break;
  default:
  case 3:
  tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
  tcg_gen_movi_tl(cpu_tmp0, 1);
  tcg_gen_shl_tl(cpu_tmp0, cpu_tmp0, cpu_T[1]);
  tcg_gen_xor_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
  break;
  }
  s->cc_op = CC_OP_SARB + ot;
  if (op != 0) {
  if (mod != 3)
  gen_op_st_T0_A0(ot + s->mem_index);
  else
  gen_op_mov_reg_T0(ot, rm);
  tcg_gen_mov_tl(cpu_cc_src, cpu_tmp4);
  tcg_gen_movi_tl(cpu_cc_dst, 0);   
<< always set zf
  }
  break;

  always set zf...

  There is fixed patch.

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

Re: [Qemu-devel] No video for Windows 2000 guest

[Thomas Huth]

On 09.08.2017 20:12, Programmingkid wrote:
> 
>> On Aug 9, 2017, at 12:37 PM, Paolo Bonzini  wrote:
>>
>> On 09/08/2017 16:56, Programmingkid wrote:
>>> The default vga card not longer works with a Windows 2000 guest. All I see 
>>> is a black screen after a the Windows splash screen.
>>>
>>> This is the command-line I used: 
>>>
>>> qemu-system-i386 -hda Windows2000HD.qcow2 -boot c -m 512
>>>
>>> When using the -vga cirrus option video works. Testing was done with QEMU 
>>> v2.10.0 rc2. 
>>
>> Did it work in 2.9?
>>
>> Paolo
> 
> I haven't test version QEMU 2.9.0 but I did test version 2.8.0 and it has the 
> same problem. Starting up Windows 2000 in VGA mode allowed me to access the 
> operating system. Found out QEMU's default video controller is not 
> recognized. In the Device Manager I can see a yellow question mark for Video 
> Controller (VGA Compatible). I remember Windows 2000 being able to use the 
> default video card in QEMU in the past. I may be able to bisect this issue 
> after all.

I guess you'll end up with QEMU 2.1 as good version and 2.2 as the first
"bad" version. According the qemu-doc:

-vga type

Select type of VGA card to emulate. Valid values for type are

cirrus

Cirrus Logic GD5446 Video card. All Windows versions starting
from Windows 95 should recognize and use this graphic card. For
optimal performances, use 16 bit color depth in the guest and
the host OS. (This card was the default before QEMU 2.2)

std

Standard VGA card with Bochs VBE extensions. If your guest OS
supports the VESA 2.0 VBE extensions (e.g. Windows XP) and if
you want to use high resolution modes (>= 1280x1024x16) then you
should use this option. (This card is the default since QEMU
2.2)

Everything is in the documentation ;-)

 Thomas

Re: [Qemu-devel] [PATCH v3] 9pfs: fix dependencies

[Thomas Huth]

On 09.08.2017 18:30, Cornelia Huck wrote:
> Nothing in fsdev/ or hw/9pfs/ depends on pci; it should rather depend
> on CONFIG_VIRTFS and CONFIG_VIRTIO/CONFIG_XEN only.
> 
> Signed-off-by: Cornelia Huck 
> ---
> 
> v2->v3: switch dependencies to VIRTFS && (VIRTIO || XEN)
> add explict VIRTIO dependency for virtio-9p-device.o
> 
> ---
>  fsdev/Makefile.objs   | 9 +++--
>  hw/9pfs/Makefile.objs | 2 +-
>  hw/Makefile.objs  | 2 +-
>  3 files changed, 5 insertions(+), 8 deletions(-)

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] No video for Windows 2000 guest

[Programmingkid]

> On Aug 9, 2017, at 6:42 PM, Paolo Bonzini  wrote:
> 
> 
>> I haven't test version QEMU 2.9.0 but I did test version 2.8.0 and it has the
>> same problem. Starting up Windows 2000 in VGA mode allowed me to access the
>> operating system. Found out QEMU's default video controller is not
>> recognized. In the Device Manager I can see a yellow question mark for Video
>> Controller (VGA Compatible). I remember Windows 2000 being able to use the
>> default video card in QEMU in the past. I may be able to bisect this issue
>> after all.
> 
> Try going back to 2.7.0 and so on...
> 
> Paolo

I went back all the way to 2.4.1 to find a version that works. I've ran into 
problems with compiling QEMU a little beyond that. I'm seeing this error: 

fatal error: 
  'epoxy/egl.h' file not found
#include 

It will take me a while to figure out how to fix this issue. I did see some 
very interesting commits up ahead that involve VGA. If you look at this page: 
https://github.com/qemu/qemu/commits/stable-2.5, there are five patches that 
effect VGA. I'm thinking one of them might be the problem.

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

[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 uses new interfaces to bind/unbind msi.

Signed-off-by: Chao Gao 
Signed-off-by: Lan Tianyu 
---
 configure |  4 +++-
 hw/xen/xen_pt_msi.c   | 50 ---
 include/hw/i386/apic-msidef.h |  1 +
 include/hw/xen/xen_common.h   | 25 ++
 4 files changed, 66 insertions(+), 14 deletions(-)

diff --git a/configure b/configure
index dd73cce..92b56d3 100755
--- a/configure
+++ b/configure
@@ -2108,13 +2108,15 @@ EOF
 elif
 cat > $TMPC <
 #include 
 int main(void) {
+  xc_interface *xc = NULL;
   xenforeignmemory_handle *xfmem;
 
   xfmem = xenforeignmemory_open(0, 0);
   xenforeignmemory_map2(xfmem, 0, 0, 0, 0, 0, 0, 0);
-
+  xc_domain_update_msi_irq_remapping(xc, 0, 0, 0, 0, 0, 0);
   return 0;
 }
 EOF
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index ff9a79f..5c5d15a 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -163,16 +163,23 @@ static int msi_msix_update(XenPCIPassthroughState *s,
 int rc = 0;
 uint64_t table_addr = 0;
 
-XEN_PT_LOG(d, "Updating MSI%s with pirq %d gvec %#x gflags %#x"
-   " (entry: %#x)\n",
-   is_msix ? "-X" : "", pirq, gvec, gflags, msix_entry);
-
 if (is_msix) {
 table_addr = s->msix->mmio_base_addr;
 }
 
-rc = xc_domain_update_msi_irq(xen_xc, xen_domid, gvec,
-  pirq, gflags, table_addr);
+if (addr & MSI_ADDR_IF_MASK) {
+XEN_PT_LOG(d, "Updating MSI%s with addr %#" PRIx64 " data %#x\n",
+   is_msix ? "-X" : "", addr, data);
+rc = xc_domain_update_msi_irq_remapping(xen_xc, xen_domid, pirq,
+ d->devfn, data, addr, table_addr);
+} else {
+XEN_PT_LOG(d, "Updating MSI%s with pirq %d gvec %#x gflags %#x"
+   " (entry: %#x)\n",
+   is_msix ? "-X" : "", pirq, gvec, gflags, msix_entry);
+
+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,30 @@ 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_IF_MASK) {
+XEN_PT_LOG(d, "Unbinding of MSI%s . ( pirq: %d, data: %x, "
+   "addr: %#" PRIx64 ")\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: %#" PRIx64 ")\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 420b411..a2b52d9 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  0x000ff000
+#define  MSI_ADDR_IF_MASK   0x0010
 
 #endif /* HW_APIC_MSIDEF_H */
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 86c7f26..454ab6d 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -680,4 +680,29 @@ static inline int xengnttab_grant_copy(xengnttab_handle 
*xgt, uint32_t count,
 }
 #endif
 
+/* Xen before 4.10 */
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 41000
+
+static inline int 

[Qemu-devel] [PATCH V2 3/3] msi: Handle remappable format interrupt request

[Lan Tianyu]

From: Chao Gao 

According to VT-d spec Interrupt Remapping and Interrupt Posting ->
Interrupt Remapping -> Interrupt Request Formats On Intel 64
Platforms, fields of MSI data register have changed. This patch
avoids wrongly regarding a remappable format interrupt request as
an interrupt binded with an event channel.

Signed-off-by: Chao Gao 
Signed-off-by: Lan Tianyu 
Acked-by: Anthony PERARD 
---
 hw/i386/xen/xen-hvm.c | 8 +++-
 hw/pci/msi.c  | 5 +++--
 hw/pci/msix.c | 4 +++-
 hw/xen/xen_pt_msi.c   | 2 +-
 include/hw/xen/xen.h  | 2 +-
 stubs/xen-hvm.c   | 2 +-
 6 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index d9ccd5d..cbbce73 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -145,8 +145,14 @@ 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_addr_lo, uint32_t msi_data)
 {
+/* If the MSI address is configured in remapping format, the MSI will not
+ * be remapped into a pirq.
+ */
+if (msi_addr_lo & MSI_ADDR_IF_MASK) {
+return 0;
+}
 /* If vector is 0, the msi is remapped into a pirq, passed as
  * dest_id.
  */
diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index 5e05ce5..d05c876 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_long(dev->config + msi_address_lo_off(dev));
+if (xen_is_pirq_msi(addr_lo, data)) {
 return false;
 }
 
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 5078d3d..65cf00c 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -83,9 +83,11 @@ static bool msix_vector_masked(PCIDevice *dev, unsigned int 
vector, bool fmask)
 {
 unsigned offset = vector * PCI_MSIX_ENTRY_SIZE;
 uint8_t *data = >msix_table[offset + PCI_MSIX_ENTRY_DATA];
+uint8_t *addr_lo = >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(addr_lo),
+ pci_get_long(data))) {
 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 5c5d15a..82e13b2 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(addr, data)) {
 *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 7efcdaa..0d6c83e 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -34,7 +34,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_addr_lo, uint32_t msi_data);
 
 qemu_irq *xen_interrupt_controller_init(void);
 
diff --git a/stubs/xen-hvm.c b/stubs/xen-hvm.c
index 3ca6c51..aeb1592 100644
--- a/stubs/xen-hvm.c
+++ b/stubs/xen-hvm.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_addr_lo, uint32_t msi_data)
 {
 return 0;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 0/3] Qemu: Add Xen vIOMMU interrupt remapping function support

[Lan Tianyu]

This patchset is to deal with MSI interrupt remapping request when guest
updates MSI registers.

Chao Gao (3):
  i386/msi: Correct mask of destination ID in MSI address
  xen-pt: bind/unbind interrupt remapping format MSI
  msi: Handle remappable format interrupt request

 configure |  4 +++-
 hw/i386/xen/xen-hvm.c |  8 ++-
 hw/pci/msi.c  |  5 +++--
 hw/pci/msix.c |  4 +++-
 hw/xen/xen_pt_msi.c   | 52 +++
 include/hw/i386/apic-msidef.h |  3 ++-
 include/hw/xen/xen.h  |  2 +-
 include/hw/xen/xen_common.h   | 25 +
 stubs/xen-hvm.c   |  2 +-
 9 files changed, 83 insertions(+), 22 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH V2 1/3] i386/msi: Correct mask of destination ID in MSI address

[Lan Tianyu]

From: Chao Gao 

According to SDM 10.11.1, only [19:12] bits of MSI address are
Destination ID, change the mask to avoid ambiguity for VT-d spec
has used the bit 4 to indicate a remappable interrupt request.

Signed-off-by: Chao Gao 
Signed-off-by: Lan Tianyu 
Reviewed-by: Anthony PERARD 
Reviewed-by: Peter Xu 
---
 include/hw/i386/apic-msidef.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/i386/apic-msidef.h b/include/hw/i386/apic-msidef.h
index 8b4d4cc..420b411 100644
--- a/include/hw/i386/apic-msidef.h
+++ b/include/hw/i386/apic-msidef.h
@@ -26,6 +26,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_DEST_ID_MASK  0x000ff000
 
 #endif /* HW_APIC_MSIDEF_H */
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] target/ppc: 'PVR != host PVR' in KVM_SET_SREGS workaround

[David Gibson]

On Wed, Aug 09, 2017 at 05:43:46PM -0300, Daniel Henrique Barboza wrote:
> Commit d5fc133eed ("ppc: Rework CPU compatibility testing
> across migration") changed the way cpu_post_load behaves with
> the PVR setting, causing an unexpected bug in KVM-HV migrations
> between hosts that are compatible (POWER8 and POWER8E, for example).
> Even with pvr_match() returning true, the guest freezes right after
> cpu_post_load. The reason is that the guest kernel can't handle a
> different PVR value other that the running host in KVM_SET_SREGS.
> 
> In [1] it was discussed the possibility of a new KVM capability
> that would indicate that the guest kernel can handle a different
> PVR in KVM_SET_SREGS. Even if such feature is implemented, there is
> still the problem with older kernels that will not have this capability
> and will fail to migrate.
> 
> This patch implements a workaround for that scenario. If running
> with KVM, check if the guest kernel does not have the capability
> (named here as 'cap_ppc_pvr_compat'). If it doesn't, calls
> kvmppc_is_pr() to see if the guest is running in KVM-HV. If all this
> happens, set env->spr[SPR_PVR] to the same value as the current
> host PVR. This ensures that we allow migrations with 'close enough'
> PVRs to still work in KVM-HV but also makes the code ready for
> this new KVM capability when it is done.
> 
> A new function called 'kvmppc_pvr_workaround_required' was created
> to encapsulate the conditions said above and to avoid calling too
> many kvm.c internals inside cpu_post_load.
> 
> [1] https://lists.gnu.org/archive/html/qemu-ppc/2017-06/msg00503.html
> 
> Signed-off-by: Daniel Henrique Barboza 

Ugly, but necessary.  Applied to ppc-for-2.10.

> ---
>  target/ppc/kvm.c | 34 ++
>  target/ppc/kvm_ppc.h |  1 +
>  target/ppc/machine.c | 22 ++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 8571379..fb3ee43 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -90,6 +90,7 @@ static int cap_htm; /* Hardware transactional 
> memory support */
>  static int cap_mmu_radix;
>  static int cap_mmu_hash_v3;
>  static int cap_resize_hpt;
> +static int cap_ppc_pvr_compat;
>  
>  static uint32_t debug_inst_opcode;
>  
> @@ -147,6 +148,13 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>  cap_mmu_radix = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_RADIX);
>  cap_mmu_hash_v3 = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_HASH_V3);
>  cap_resize_hpt = kvm_vm_check_extension(s, KVM_CAP_SPAPR_RESIZE_HPT);
> +/*
> + * Note: setting it to false because there is not such capability
> + * in KVM at this moment.
> + *
> + * TODO: call kvm_vm_check_extension() with the right capability
> + * after the kernel starts implementing it.*/
> +cap_ppc_pvr_compat = false;
>  
>  if (!cap_interrupt_level) {
>  fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the 
> "
> @@ -2785,3 +2793,29 @@ void kvmppc_update_sdr1(target_ulong sdr1)
>  run_on_cpu(cs, kvmppc_pivot_hpt_cpu, RUN_ON_CPU_TARGET_PTR(sdr1));
>  }
>  }
> +
> +/*
> + * This is a helper function to detect a post migration scenario
> + * in which a guest, running as KVM-HV, freezes in cpu_post_load because
> + * the guest kernel can't handle a PVR value other than the actual host
> + * PVR in KVM_SET_SREGS, even if pvr_match() returns true.
> + *
> + * If we don't have cap_ppc_pvr_compat and we're not running in PR
> + * (so, we're HV), return true. The workaround itself is done in
> + * cpu_post_load.
> + *
> + * The order here is important: we'll only check for KVM PR as a
> + * fallback if the guest kernel can't handle the situation itself.
> + * We need to avoid as much as possible querying the running KVM type
> + * in QEMU level.
> + */
> +bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu)
> +{
> +CPUState *cs = CPU(cpu);
> +
> +if (cap_ppc_pvr_compat) {
> +return false;
> +}
> +
> +return !kvmppc_is_pr(cs->kvm_state);
> +}
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 6bc6fb3..381afe6 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -67,6 +67,7 @@ void kvmppc_check_papr_resize_hpt(Error **errp);
>  int kvmppc_resize_hpt_prepare(PowerPCCPU *cpu, target_ulong flags, int 
> shift);
>  int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, target_ulong flags, int shift);
>  void kvmppc_update_sdr1(target_ulong sdr1);
> +bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
>  
>  bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path);
>  
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index f578156..e545885 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -9,6 +9,7 @@
>  #include "mmu-hash64.h"
>  #include "migration/cpu.h"
>  #include "qapi/error.h"
> +#include "kvm_ppc.h"
>  
>  static int 

Re: [Qemu-devel] [PATCH v2 2/4] vhost-user-blk: introduce a new vhost-user-blk host device

[Liu, Changpeng]

> -Original Message-
> From: Marc-André Lureau [mailto:marcandre.lur...@redhat.com]
> Sent: Wednesday, August 9, 2017 11:39 PM
> To: Liu, Changpeng 
> Cc: qemu-devel@nongnu.org; stefa...@gmail.com; pbonz...@redhat.com;
> m...@redhat.com; fel...@nutanix.com; Harris, James R
> 
> Subject: Re: [PATCH v2 2/4] vhost-user-blk: introduce a new vhost-user-blk 
> host
> device
> 
> Hi
> 
> - Original Message -
> > This commit introduces a new vhost-user device for block, it uses a
> > chardev to connect with the backend, same with Qemu virito-blk device,
> > Guest OS still uses the virtio-blk frontend driver.
> >
> > To use it, start Qemu with command line like this:
> >
> > qemu-system-x86_64 \
> > -chardev socket,id=char0,path=/path/vhost.socket \
> > -device vhost-user-blk-pci,chardev=char0,num_queues=...
> >
> > Different with exist Qemu virtio-blk host device, it makes more easy
> > for users to implement their own I/O processing logic, such as all
> > user space I/O stack against hardware block device. It uses the new
> > vhost messages(VHOST_USER_GET_CONFIG) to get block virtio config
> > information from backend process.
> >
> > Signed-off-by: Changpeng Liu 
> > ---
> >  configure  |  11 ++
> >  hw/block/Makefile.objs |   3 +
> >  hw/block/vhost-user-blk.c  | 360
> >  +
> >  hw/virtio/virtio-pci.c |  55 ++
> >  hw/virtio/virtio-pci.h |  18 ++
> >  include/hw/virtio/vhost-user-blk.h |  40 +
> >  6 files changed, 487 insertions(+)
> >  create mode 100644 hw/block/vhost-user-blk.c
> >  create mode 100644 include/hw/virtio/vhost-user-blk.h
> >
> > diff --git a/configure b/configure
> > index dd73cce..1452c66 100755
> > --- a/configure
> > +++ b/configure
> > @@ -305,6 +305,7 @@ tcg="yes"
> >
> >  vhost_net="no"
> >  vhost_scsi="no"
> > +vhost_user_blk="no"
> >  vhost_vsock="no"
> >  vhost_user=""
> >  kvm="no"
> > @@ -779,6 +780,7 @@ Linux)
> >kvm="yes"
> >vhost_net="yes"
> >vhost_scsi="yes"
> > +  vhost_user_blk="yes"
> >vhost_vsock="yes"
> >QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers
> >$QEMU_INCLUDES"
> >supported_os="yes"
> > @@ -1136,6 +1138,10 @@ for opt do
> >;;
> >--enable-vhost-scsi) vhost_scsi="yes"
> >;;
> > +  --disable-vhost-user-blk) vhost_user_blk="no"
> > +  ;;
> > +  --enable-vhost-user-blk) vhost_user_blk="yes"
> > +  ;;
> 
> I suggest we don't add yet another configure option, but reuse the recently
> introduced --enable-vhost-user (that should cover all vhost-user devices for 
> now,
> but may learn to enable specific devices if needed in the future).
Yes, I noticed there is a new vhost-user configuration, sounds good to me if 
other devices
such as vhost-net and vhost-scsi also use the same configuration option.
> 
> >--disable-vhost-vsock) vhost_vsock="no"
> >;;
> >--enable-vhost-vsock) vhost_vsock="yes"
> > @@ -1506,6 +1512,7 @@ disabled with --disable-FEATURE, default is enabled if
> > available:
> >cap-ng  libcap-ng support
> >attrattr and xattr support
> >vhost-net   vhost-net acceleration support
> > +  vhost-user-blk  VM virtio-blk acceleration in user space
> >spice   spice
> >rbd rados block device (rbd)
> >libiscsiiscsi support
> > @@ -5365,6 +5372,7 @@ echo "posix_madvise $posix_madvise"
> >  echo "libcap-ng support $cap_ng"
> >  echo "vhost-net support $vhost_net"
> >  echo "vhost-scsi support $vhost_scsi"
> > +echo "vhost-user-blk support $vhost_user_blk"
> >  echo "vhost-vsock support $vhost_vsock"
> >  echo "vhost-user support $vhost_user"
> >  echo "Trace backends$trace_backends"
> > @@ -5776,6 +5784,9 @@ fi
> >  if test "$vhost_scsi" = "yes" ; then
> >echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak
> >  fi
> > +if test "$vhost_user_blk" = "yes" ; then
> > +  echo "CONFIG_VHOST_USER_BLK=y" >> $config_host_mak
> > +fi
> >  if test "$vhost_net" = "yes" -a "$vhost_user" = "yes"; then
> >echo "CONFIG_VHOST_NET_USED=y" >> $config_host_mak
> >  fi
> > diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
> > index e0ed980..4c19a58 100644
> > --- a/hw/block/Makefile.objs
> > +++ b/hw/block/Makefile.objs
> > @@ -13,3 +13,6 @@ obj-$(CONFIG_SH4) += tc58128.o
> >
> >  obj-$(CONFIG_VIRTIO) += virtio-blk.o
> >  obj-$(CONFIG_VIRTIO) += dataplane/
> > +ifeq ($(CONFIG_VIRTIO),y)
> > +obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk.o
> > +endif
> > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > new file mode 100644
> > index 000..8aa9fa9
> > --- /dev/null
> > +++ b/hw/block/vhost-user-blk.c
> > @@ -0,0 +1,360 @@
> > +/*
> > + * vhost-user-blk host device
> > + *
> > + * Copyright IBM, Corp. 2011
> > + * Copyright(C) 2017 Intel Corporation.
> > + *
> > + * Authors:
> > + *  Stefan 

[Qemu-devel] [PATCH] target/i386: fix packusdw in-place operation

[Joseph Myers]

The SSE4.1 packusdw instruction combines source and destination
vectors of signed 32-bit integers into a single vector of unsigned
16-bit integers, with unsigned saturation.  When the source and
destination are the same register, this means each 32-bit element of
that register is used twice as an input, to produce two of the 16-bit
output elements, and so if the operation is carried out
element-by-element in-place, no matter what the order in which it is
applied to the elements, the first element's operation will overwrite
some future input.  The helper for packssdw avoids this issue by
computing the result in a local temporary and copying it to the
destination at the end; this patch fixes the packusdw helper to do
likewise.  This fixes three gcc test failures in my GCC 6-based
testing.

Signed-off-by: Joseph Myers 

---

diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
index 16509d0..05b1701 100644
--- a/target/i386/ops_sse.h
+++ b/target/i386/ops_sse.h
@@ -1655,14 +1655,17 @@ SSE_HELPER_Q(helper_pcmpeqq, FCMPEQQ)
 
 void glue(helper_packusdw, SUFFIX)(CPUX86State *env, Reg *d, Reg *s)
 {
-d->W(0) = satuw((int32_t) d->L(0));
-d->W(1) = satuw((int32_t) d->L(1));
-d->W(2) = satuw((int32_t) d->L(2));
-d->W(3) = satuw((int32_t) d->L(3));
-d->W(4) = satuw((int32_t) s->L(0));
-d->W(5) = satuw((int32_t) s->L(1));
-d->W(6) = satuw((int32_t) s->L(2));
-d->W(7) = satuw((int32_t) s->L(3));
+Reg r;
+
+r.W(0) = satuw((int32_t) d->L(0));
+r.W(1) = satuw((int32_t) d->L(1));
+r.W(2) = satuw((int32_t) d->L(2));
+r.W(3) = satuw((int32_t) d->L(3));
+r.W(4) = satuw((int32_t) s->L(0));
+r.W(5) = satuw((int32_t) s->L(1));
+r.W(6) = satuw((int32_t) s->L(2));
+r.W(7) = satuw((int32_t) s->L(3));
+*d = r;
 }
 
 #define FMINSB(d, s) MIN((int8_t)d, (int8_t)s)

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [Qemu-devel] [PATCH 01/12] qemu-iotests: remove dead code

[Paolo Bonzini]

- Original Message -
> From: "Eric Blake" 
> To: "Paolo Bonzini" , qemu-devel@nongnu.org
> Cc: kw...@redhat.com, qemu-bl...@nongnu.org
> Sent: Thursday, August 10, 2017 12:18:54 AM
> Subject: Re: [Qemu-devel] [PATCH 01/12] qemu-iotests: remove dead code
> 
> On 08/09/2017 04:54 PM, Paolo Bonzini wrote:
> > This includes shell function, shell variables, command line options
> > (randomize.awk does not exist) and conditions that can never be true
> > (./qemu does not exist anymore).
> 
> Can we point to a commit id where we stopped making ./qemu?

commit 9aed1e036dc0de49d08d713f9e5c4655e94acb56
Author: Anthony Liguori 
Date:   Mon Aug 29 09:55:36 2011 -0500

Rename qemu -> qemu-system-i386

This has been discussed before in the past.  The special casing really 
makes no
sense anymore.  This seems like a good change to make for 1.0.

Signed-off-by: Anthony Liguori 

> Is it still worth supporting a local symlink?

Not sure who would have one...

Paolo

Re: [Qemu-devel] No video for Windows 2000 guest

[Paolo Bonzini]

> I haven't test version QEMU 2.9.0 but I did test version 2.8.0 and it has the
> same problem. Starting up Windows 2000 in VGA mode allowed me to access the
> operating system. Found out QEMU's default video controller is not
> recognized. In the Device Manager I can see a yellow question mark for Video
> Controller (VGA Compatible). I remember Windows 2000 being able to use the
> default video card in QEMU in the past. I may be able to bisect this issue
> after all.

Try going back to 2.7.0 and so on...

Paolo

Re: [Qemu-devel] [PATCH 02/12] qemu-iotests: get rid of AWK_PROG

[Philippe Mathieu-Daudé]

On 08/09/2017 06:55 PM, Paolo Bonzini wrote:

Signed-off-by: Paolo Bonzini 


Reviewed-by: Philippe Mathieu-Daudé 


---
  tests/qemu-iotests/check | 4 ++--
  tests/qemu-iotests/common| 2 +-
  tests/qemu-iotests/common.config | 3 ---
  3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 01fd5a26e5..5075029e89 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -128,7 +128,7 @@ tmp="${TEST_DIR}"/$$
  
  _wallclock()

  {
-date "+%H %M %S" | $AWK_PROG '{ print $1*3600 + $2*60 + $3 }'
+date "+%H %M %S" | awk '{ print $1*3600 + $2*60 + $3 }'
  }
  
  _timestamp()

@@ -147,7 +147,7 @@ _wrapup()
  if [ -f $TIMESTAMP_FILE -a -f $tmp.time ]
  then
  cat $TIMESTAMP_FILE $tmp.time \
-| $AWK_PROG '
+| awk '
  { t[$1] = $2 }
  END{ if (NR > 0) {
  for (i in t) print i " " t[i]
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 867918895b..130f647a4d 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -366,7 +366,7 @@ testlist options
  if $xpand
  then
  have_test_arg=true
-$AWK_PROG   
-export AWK_PROG="`set_prog_path awk`"

-[ "$AWK_PROG" = "" ] && _fatal "awk not found"
-
  if [ -z "$QEMU_PROG" ]; then
  export QEMU_PROG="`set_prog_path qemu`"
  fi

Re: [Qemu-devel] [PATCH 03/12] qemu-iotests: move "check" code out of common.rc

[Philippe Mathieu-Daudé]

On 08/09/2017 06:55 PM, Paolo Bonzini wrote:

Some functions in common.rc are never used by the tests.  Move
them out of that file and into common, which is already included
only by "check".

Code that actually *is* common to "check" and tests can be placed in
common.config.

Signed-off-by: Paolo Bonzini 


Reviewed-by: Philippe Mathieu-Daudé 


---
  tests/qemu-iotests/common| 30 ++-
  tests/qemu-iotests/common.config | 12 +++
  tests/qemu-iotests/common.rc | 45 
  3 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 130f647a4d..50720f080f 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -19,6 +19,29 @@
  # common procedures for QA scripts
  #
  
+_full_imgfmt_details()

+{
+if [ -n "$IMGOPTS" ]; then
+echo "$IMGFMT ($IMGOPTS)"
+else
+echo "$IMGFMT"
+fi
+}
+
+_full_imgproto_details()
+{
+echo "$IMGPROTO"
+}
+
+_full_platform_details()
+{
+os=`uname -s`
+host=`hostname -s`
+kernel=`uname -r`
+platform=`uname -m`
+echo "$os/$platform $host $kernel"
+}
+
  diff="diff -u"
  verbose=false
  debug=false
@@ -404,7 +427,12 @@ if [ "$IMGOPTSSYNTAX" != "true" ]; then
  fi
  
  # Set default options for qemu-img create -o if they were not specified

-_set_default_imgopts
+if [ "$IMGFMT" == "qcow2" ] && ! (echo "$IMGOPTS" | grep "compat=" > 
/dev/null); then
+IMGOPTS=$(_optstr_add "$IMGOPTS" "compat=1.1")
+fi
+if [ "$IMGFMT" == "luks" ] && ! (echo "$IMGOPTS" | grep "iter-time=" > 
/dev/null); then
+IMGOPTS=$(_optstr_add "$IMGOPTS" "iter-time=10")
+fi
  
  if [ -s $tmp.list ]

  then
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index 0f571d46eb..91da65f3dc 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -27,6 +27,9 @@ export PWD=`pwd`
  
  export _QEMU_HANDLE=0
  
+# make sure we have a standard umask

+umask 022
+
  # $1 = prog to look for, $2* = default pathnames if not found in $PATH
  set_prog_path()
  {
@@ -49,6 +52,15 @@ set_prog_path()
  return 1
  }
  
+_optstr_add()

+{
+if [ -n "$1" ]; then
+echo "$1,$2"
+else
+echo "$2"
+fi
+}
+
  _fatal()
  {
  echo "$*"
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index dd91a2b79a..6f6e03366f 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -50,9 +50,6 @@ then
  fi
  fi
  
-# make sure we have a standard umask

-umask 022
-
  if [ "$IMGOPTSSYNTAX" = "true" ]; then
  DRIVER="driver=$IMGFMT"
  if [ "$IMGFMT" = "luks" ]; then
@@ -94,25 +91,6 @@ else
  fi
  ORIG_TEST_IMG="$TEST_IMG"
  
-_optstr_add()

-{
-if [ -n "$1" ]; then
-echo "$1,$2"
-else
-echo "$2"
-fi
-}
-
-_set_default_imgopts()
-{
-if [ "$IMGFMT" == "qcow2" ] && ! (echo "$IMGOPTS" | grep "compat=" > 
/dev/null); then
-IMGOPTS=$(_optstr_add "$IMGOPTS" "compat=1.1")
-fi
-if [ "$IMGFMT" == "luks" ] && ! (echo "$IMGOPTS" | grep "iter-time=" > 
/dev/null); then
-IMGOPTS=$(_optstr_add "$IMGOPTS" "iter-time=10")
-fi
-}
-
  _use_sample_img()
  {
  SAMPLE_IMG_FILE="${1%\.bz2}"
@@ -428,28 +406,5 @@ _require_command()
  [ -x "$c" ] || _notrun "$1 utility required, skipped this test"
  }
  
-_full_imgfmt_details()

-{
-if [ -n "$IMGOPTS" ]; then
-echo "$IMGFMT ($IMGOPTS)"
-else
-echo "$IMGFMT"
-fi
-}
-
-_full_imgproto_details()
-{
-echo "$IMGPROTO"
-}
-
-_full_platform_details()
-{
-os=`uname -s`
-host=`hostname -s`
-kernel=`uname -r`
-platform=`uname -m`
-echo "$os/$platform $host $kernel"
-}
-
  # make sure this script returns success
  true

Re: [Qemu-devel] [PATCH 05/12] qemu-iotests: do not include common.rc in "check"

[Philippe Mathieu-Daudé]

On 08/09/2017 06:55 PM, Paolo Bonzini wrote:

It only provides functions used by the test programs.

Signed-off-by: Paolo Bonzini 


Reviewed-by: Philippe Mathieu-Daudé 


---
  tests/qemu-iotests/check |  6 --
  tests/qemu-iotests/common.rc | 13 +
  2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 5075029e89..1ef6d0ac3a 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -113,12 +113,6 @@ then
  _init_error "failed to source common.config"
  fi
  
-# we need common.rc

-if ! . "$source_iotests/common.rc"
-then
-_init_error "failed to source common.rc"
-fi
-
  # we need common
  . "$source_iotests/common"
  
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc

index ef5cdb3385..390aa81224 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -40,14 +40,11 @@ poke_file()
  printf "$3" | dd "of=$1" bs=1 "seek=$2" conv=notrunc &>/dev/null
  }
  
-# we need common.config

-if [ "$iam" != "check" ]
-then
-if ! . ./common.config
-then
-echo "$iam: failed to source common.config"
-exit 1
-fi
+
+if ! . ./common.config
+then
+echo "$iam: failed to source common.config"
+exit 1
  fi
  
  _fatal()

Re: [Qemu-devel] [PATCH 10/12] qemu-iotests: get rid of $iam

[Philippe Mathieu-Daudé]

On 08/09/2017 06:55 PM, Paolo Bonzini wrote:

The variable is almost unused, and one of the two uses is actually
uninitialized.

Signed-off-by: Paolo Bonzini 


Reviewed-by: Philippe Mathieu-Daudé 


---
  tests/qemu-iotests/check | 5 +
  tests/qemu-iotests/common.rc | 2 +-
  2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index e5d1ae3d92..dc60e17e6a 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -30,12 +30,9 @@ interrupt=true
  # by default don't output timestamps
  timestamp=${TIMESTAMP:=false}
  
-# generic initialization

-iam=check
-
  _init_error()
  {
-echo "$iam: $1" >&2
+echo "check: $1" >&2
  exit 1
  }
  
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc

index 7046a83974..d0aa22543e 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -43,7 +43,7 @@ poke_file()
  
  if ! . ./common.config

  then
-echo "$iam: failed to source common.config"
+echo "$0: failed to source common.config"
  exit 1
  fi
  

Re: [Qemu-devel] [PATCH 02/12] qemu-iotests: get rid of AWK_PROG

[Eric Blake]

On 08/09/2017 04:55 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini 
> ---
>  tests/qemu-iotests/check | 4 ++--
>  tests/qemu-iotests/common| 2 +-
>  tests/qemu-iotests/common.config | 3 ---
>  3 files changed, 3 insertions(+), 6 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 01/12] qemu-iotests: remove dead code

[Eric Blake]

On 08/09/2017 04:54 PM, Paolo Bonzini wrote:
> This includes shell function, shell variables, command line options
> (randomize.awk does not exist) and conditions that can never be true
> (./qemu does not exist anymore).

Can we point to a commit id where we stopped making ./qemu?  Is it still
worth supporting a local symlink?

> 
> Signed-off-by: Paolo Bonzini 
> ---
>  tests/qemu-iotests/check | 36 +
>  tests/qemu-iotests/common| 23 --
>  tests/qemu-iotests/common.config | 26 ---
>  tests/qemu-iotests/common.rc | 68 
> 
>  4 files changed, 1 insertion(+), 152 deletions(-)
> 
> -#
> -# - These can be added to $HOST_CONFIG_DIR (witch default to ./config)

Good riddance to the wrong word ('which' was intended)

Other than possibly still wanting ./qemu convenience, this looks like a
reasonable cleanup.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] OvmfPkg (PlatformPei?): supporting Qemu's nvdimm device

[Laszlo Ersek]

On 08/09/17 22:47, Rebecca Cran wrote:
> I've been investigating adding support for Qemu's nvdimm devices to
> OVMF.  I was thinking such support would go into PlatformPei, but it
> looks like I can only read the ACPI NFIT in the DXE phase. So, should
> Qemu be changed to add non-volatile memory to the e820 table, or
> should such memory be added during the DXE phase instead?

I don't have the slightest idea, sorry.

One thing that does look bad is that both edk2's

  MdeModulePkg/Universal/Disk/RamDiskDxe

and QEMU's

  hw/acpi/nvdimm.c

think they own the NVDIMM root device with, with _HID=ACPI0012.

I don't recall being consulted when QEMU's NVDIMM stuff was designed and
implemented, despite the fact that it uses the ACPI linker/loader
(apparently).

In general, using the ACPI linker/loader is a good recipe for making an
ACPI-defined feature "just work" under both SeaBIOS and OVMF, without
having to resort to specialized information channels such as new fw_cfg
files, or additional paravirt hardware. However, there are enough quirks
in edk2 / UEFI with regard to ACPI that, without special attention to
OVMF / edk2 at design time, things will likely break due to infighting
over ACPI ownership.

In RamDiskPublishNfit(), there is

//
// Assumption is made that if no NFIT is in the ACPI table, there is no
// NVDIMM root device in the \SB scope.
// Therefore, a NVDIMM root device will be reported via Secondary System
// Description Table (SSDT).
//
Status = RamDiskPublishSsdt ();

That assumption might or might not be good enough, when QEMU pushes down
its NVDIMM / NFIT stuff. I suggest reading "docs/specs/acpi_nvdimm.txt"
in the QEMU tree, and figuring out if that can work with OVMF's ACPI
linker/loader client, and the rest of the edk2 modules. The assumption
could also be affected by whether OvmfPkg/AcpiTableDxe installs QEMU's
NVDIM stuff before or after RamDiskDxe does its thing. I have no clue.

I apologize but I don't think I can help (or even find the bandwidth to
research or discuss this -- it's 00:13 local time and I'm reviewing
patches...)

I'm CC'ing Michael and Igor, maybe they can provide you with pointers.
(Also adding qemu-devel.)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v4 17/22] libqtest: Add qmp_args() helper

[Eric Blake]

On 08/09/2017 10:57 AM, Markus Armbruster wrote:

> I don't like the qmp_args name.  It's not about arguments, it's about
> sending a command with arguments.
> 
> I'm afraid I'm getting pretty thorougly confused by the evolving
> interface.  So I stop and think how it should look like when done.

At a bare minimum, I like your idea that we want:

FOO_send() # sends a message, does not wait for a reply
FOO_receive()  # reads one JSON object, returns QObject counterpart
FOO() # combo of FOO_send() and FOO_receive()

Then FOO_discard() is a convenient shorthand for QDECREF(FOO_receive()).

Which FOO do we need? Right now, we have 'qmp' for anything using the
global_qtest, 'qtest_qmp' for anything using an explicit QTestState (but
we're arguing that is overkill), and 'qmp_fd' for anything using a raw
fd (test-qga.c - and where I added 'qga' in 11/22, although that
addition was local rather than in libqtest.h).

I also just had an insight: it is possible to write a macro
qmp_send(...) that will expand to qmp_send_cmd(name) if there is only
one argument, but to qmp_send_cmdf(name, fmt, ...) if there is more than
one argument (and later expose a public qmp_send_cmdv(name, fmt,
va_list) as pair if it is needed, although right now it is not).
Perhaps we can even make the one-arg form expand to qmp_send_cmdf(name,
" "), if that's enough to silence gcc's -Wformat-zero-length, and if our
underlying routines are smart enough to recognize a single-space
argument as not being worthy of calling qobject_from_jsonf() (since
qobject_from_jsonf() aborts rather than returning NULL when presented
just whitespace).

In fact, having a macro qmp_send(cmd, ...) expand to
qmp_send_internal(cmd, " " __VA_ARGS__) might even do the trick without
preprocessor magic of checking whether __VA_ARGS__ contains a comma
(although it has the subtle limitation that if present, a format
argument MUST be a string literal because we now rely on string
concatenation with " " - although given gcc's -Wformat-nonliteral, we
are probably okay with that limitation).

So, with a nice macro, the bulk of the testsuite would just write in
this style:

qmp_send("foo");
qmp_send("foo", "{literal_args...}");
qmp_send("foo", "{'name':%s}", value);

for send without waiting, or:

obj = qmp("foo");
obj = qmp(foo", "{literal_args...}");
obj = qmp("foo", "{'name':%s}", value);

where the result matters.  And the names we choose for internal
implementation are no longer quite as important (although still helpful
to get right).

> 
> We need send and receive.  Convenience functions to do both, and to
> receive and throw away are nice to have.

Do we want both a convenience function to throw away a single read, AND
a function to send a command + throw away a read? Prior to this series,
we have qmp_discard_response() which is send + discard, but nothing that
does plain discard; although plain discard is a better building block
(precisely because then we don't have to duplicate all the different
send forms) - the convenience of send+receive in one call should be
limited to the most common case.

Also, the qmp_eventwait() is a nice convenience function for wait in a
loop until the received object matches a given name; whether we also
need the convenience of qmp_eventwait_ref() is a bit more debatable
(perhaps we could just as easily have qmp_event_wait() always return an
object, and require the caller to QDECREF() it, for one less function to
maintain).

> 
> We need qmp_raw().  We haven't found a need for a raw receive function.

Yes, qmp_raw() (or maybe it should be named qmp_send_raw(), depending on
whether the receive is coupled with it?) is our backdoor for sending
JSON that does not match the normal {'execute':'name','arguments':{}}
paradigm (and pretty much qmp-test.c, or maybe also test-qga.c, would be
the only clients).

[Side node - why can't we pick a consistent naming of our tests?  The
name 'FOO-test' is a bit nicer than 'test-FOO' when it comes to writing
.gitignore entries. Should we do a bulk cleanup rename to get all the
tests into one consistent naming style?]

> 
> Convenience functions to build commands and send are nice to have.  They
> come in pairs, without arguments, to avoid -Wno-format-zero-length
> (aside: that's one of the sillier warnings).

It's possible to alter CFLAGS to shut gcc up on that one while still
getting the rest of -Wformat, if we don't think it adds value to qemu.
My idea above is that you can use a macro to hide the worst of the
paired nature, so that the bulk of the testsuite can supply or omit an
arguments format string as desired.

> 
> We used to have almost everything with and without QTestState, but we're
> refusing to continue that self-flagellation.

And for v5, I think I'll reorder my series to get rid of QTestState
sooner, rather than later.

> 
> For every function taking ..., we want one taking va_list.

Under the hood, probably.  Whether the va_list form has to be exported
is a different question (we might be 

[Qemu-devel] [PATCH 12/12] qemu-iotests: merge "check" and "common"

[Paolo Bonzini]

"check" is full of qemu-iotests--specific details.  Separating it
from "common" does not make much sense anymore.

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/check  | 482 +++-
 tests/qemu-iotests/common | 501 --
 2 files changed, 480 insertions(+), 503 deletions(-)
 delete mode 100644 tests/qemu-iotests/common

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 28b3444963..6d4d01e432 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -110,8 +110,486 @@ then
 export QEMU_NBD_PROG="$build_root/qemu-nbd"
 fi
 
-# we need common
-. "$source_iotests/common"
+_full_imgfmt_details()
+{
+if [ -n "$IMGOPTS" ]; then
+echo "$IMGFMT ($IMGOPTS)"
+else
+echo "$IMGFMT"
+fi
+}
+
+_full_imgproto_details()
+{
+echo "$IMGPROTO"
+}
+
+_full_platform_details()
+{
+os=`uname -s`
+host=`hostname -s`
+kernel=`uname -r`
+platform=`uname -m`
+echo "$os/$platform $host $kernel"
+}
+
+# $1 = prog to look for
+set_prog_path()
+{
+p=`command -v $1 2> /dev/null`
+if [ -n "$p" -a -x "$p" ]; then
+realpath -- "$(type -p "$p")"
+else
+return 1
+fi
+}
+
+export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
+export QEMU_IMG_PROG=$(realpath -- "$(type -p "$QEMU_IMG_PROG")")
+export QEMU_IO_PROG=$(realpath -- "$(type -p "$QEMU_IO_PROG")")
+export QEMU_NBD_PROG=$(realpath -- "$(type -p "$QEMU_NBD_PROG")")
+if [ -z "$QEMU_VXHS_PROG" ]; then
+  export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
+fi
+
+if [ -z "$TEST_DIR" ]; then
+TEST_DIR=`pwd`/scratch
+fi
+
+if [ ! -e "$TEST_DIR" ]; then
+mkdir "$TEST_DIR"
+fi
+
+diff="diff -u"
+verbose=false
+debug=false
+group=false
+xgroup=false
+imgopts=false
+showme=false
+sortme=false
+expunge=true
+have_test_arg=false
+cachemode=false
+
+tmp="${TEST_DIR}"/$$
+rm -f $tmp.list $tmp.tmp $tmp.sed
+
+export IMGFMT=raw
+export IMGFMT_GENERIC=true
+export IMGPROTO=file
+export IMGOPTS=""
+export CACHEMODE="writeback"
+export QEMU_IO_OPTIONS=""
+export QEMU_IO_OPTIONS_NO_FMT=""
+export CACHEMODE_IS_DEFAULT=true
+export QEMU_OPTIONS="-nodefaults -machine accel=qtest"
+export VALGRIND_QEMU=
+export IMGKEYSECRET=
+export IMGOPTSSYNTAX=false
+
+default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
+default_alias_machine=$($QEMU_PROG -machine help | \
+   sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
+if [[ "$default_alias_machine" ]]; then
+default_machine="$default_alias_machine"
+fi
+
+export QEMU_DEFAULT_MACHINE="$default_machine"
+
+for r
+do
+
+if $group
+then
+# arg after -g
+group_list=`sed -n <"$source_iotests/group" -e 's/$/ /' -e 
"/^[0-9][0-9][0-9].* $r /"'{
+s/ .*//p
+}'`
+if [ -z "$group_list" ]
+then
+echo "Group \"$r\" is empty or not defined?"
+exit 1
+fi
+[ ! -s $tmp.list ] && touch $tmp.list
+for t in $group_list
+do
+if grep -s "^$t\$" $tmp.list >/dev/null
+then
+:
+else
+echo "$t" >>$tmp.list
+fi
+done
+group=false
+continue
+
+elif $xgroup
+then
+# arg after -x
+# Populate $tmp.list with all tests
+awk '/^[0-9]{3,}/ {print $1}' "${source_iotests}/group" > $tmp.list 
2>/dev/null
+group_list=`sed -n <"$source_iotests/group" -e 's/$/ /' -e 
"/^[0-9][0-9][0-9].* $r /"'{
+s/ .*//p
+}'`
+if [ -z "$group_list" ]
+then
+echo "Group \"$r\" is empty or not defined?"
+exit 1
+fi
+numsed=0
+rm -f $tmp.sed
+for t in $group_list
+do
+if [ $numsed -gt 100 ]
+then
+sed -f $tmp.sed <$tmp.list >$tmp.tmp
+mv $tmp.tmp $tmp.list
+numsed=0
+rm -f $tmp.sed
+fi
+echo "/^$t\$/d" >>$tmp.sed
+numsed=`expr $numsed + 1`
+done
+sed -f $tmp.sed <$tmp.list >$tmp.tmp
+mv $tmp.tmp $tmp.list
+xgroup=false
+continue
+
+elif $imgopts
+then
+IMGOPTS="$r"
+imgopts=false
+continue
+elif $cachemode
+then
+CACHEMODE="$r"
+CACHEMODE_IS_DEFAULT=false
+cachemode=false
+continue
+fi
+
+xpand=true
+case "$r"
+in
+
+-\? | -h | --help)# usage
+echo "Usage: $0 [options] [testlist]"'
+
+common options
+-v  verbose
+-d  debug
+
+image format options
+-rawtest raw (default)
+-bochs  test bochs
+-cloop  test cloop
+-parallels  test parallels
+-qcow   test qcow
+-qcow2  test qcow2
+-qedtest qed
+-vdi   

[Qemu-devel] [PATCH 11/12] qemu-iotests: include common.env and common.config early

[Paolo Bonzini]

They are now very small and they do not have external dependencies.  We
can source them as soon as we have their path.

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/check | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index dc60e17e6a..28b3444963 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -55,6 +55,18 @@ else
 build_iotests=$PWD
 fi
 
+# we need common.env
+if ! . "$build_iotests/common.env"
+then
+_init_error "failed to source common.env (make sure the qemu-iotests are 
run from tests/qemu-iotests in the build tree)"
+fi
+
+# we need common.config
+if ! . "$source_iotests/common.config"
+then
+_init_error "failed to source common.config"
+fi
+
 build_root="$build_iotests/../.."
 
 if [ -x "$build_iotests/socket_scm_helper" ]
@@ -98,18 +110,6 @@ then
 export QEMU_NBD_PROG="$build_root/qemu-nbd"
 fi
 
-# we need common.env
-if ! . "$build_iotests/common.env"
-then
-_init_error "failed to source common.env (make sure the qemu-iotests are 
run from tests/qemu-iotests in the build tree)"
-fi
-
-# we need common.config
-if ! . "$source_iotests/common.config"
-then
-_init_error "failed to source common.config"
-fi
-
 # we need common
 . "$source_iotests/common"
 
-- 
2.13.3

[Qemu-devel] [PATCH 04/12] qemu-iotests: limit non-_PROG-suffixed variables to common.rc

[Paolo Bonzini]

These are never used by "check", with one exception that does not need
$QEMU_OPTIONS.  Keep them in common.rc, which will be soon included only
by the tests.

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/039.out   | 10 ++---
 tests/qemu-iotests/061.out   |  4 +-
 tests/qemu-iotests/137.out   |  2 +-
 tests/qemu-iotests/common|  8 
 tests/qemu-iotests/common.config | 75 +
 tests/qemu-iotests/common.rc | 80 
 6 files changed, 90 insertions(+), 89 deletions(-)

diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index c6e0ac2da3..724d7b2508 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -11,7 +11,7 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
 exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
 else
 exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
@@ -50,7 +50,7 @@ read 512/512 bytes at offset 0
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
 exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
 else
 exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
@@ -68,7 +68,7 @@ incompatible_features 0x0
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
 exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
 else
 exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
@@ -91,7 +91,7 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
 exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
 else
 exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
@@ -105,7 +105,7 @@ Data may be corrupted, or further writes to the image may 
corrupt it.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
 exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
 else
 exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index a431b7f305..942485de99 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -57,7 +57,7 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
 exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
 else
 exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
@@ -219,7 +219,7 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
then
+./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; then
 exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
"$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
 else
 exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
diff --git a/tests/qemu-iotests/137.out b/tests/qemu-iotests/137.out
index c0e753483b..05efd74d17 100644
--- a/tests/qemu-iotests/137.out
+++ b/tests/qemu-iotests/137.out
@@ -31,7 +31,7 @@ Cache clean interval too big
 Unsupported value 'blubb' for qcow2 option 'overlap-check'. Allowed are any of 
the following: none, 

[Qemu-devel] [PATCH 01/12] qemu-iotests: remove dead code

[Paolo Bonzini]

This includes shell function, shell variables, command line options
(randomize.awk does not exist) and conditions that can never be true
(./qemu does not exist anymore).

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/check | 36 +
 tests/qemu-iotests/common| 23 --
 tests/qemu-iotests/common.config | 26 ---
 tests/qemu-iotests/common.rc | 68 
 4 files changed, 1 insertion(+), 152 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 2a55ec9ada..01fd5a26e5 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -65,8 +65,7 @@ then
 export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
 fi
 
-# if ./qemu exists, it should be prioritized and will be chosen by 
common.config
-if [[ -z "$QEMU_PROG" && ! -x './qemu' ]]
+if [[ -z "$QEMU_PROG" ]]
 then
 arch=$(uname -m 2> /dev/null)
 
@@ -123,12 +122,6 @@ fi
 # we need common
 . "$source_iotests/common"
 
-#if [ `id -u` -ne 0 ]
-#then
-#echo "check: QA must be run as root"
-#exit 1
-#fi
-
 TIMESTAMP_FILE=check.time-$IMGPROTO-$IMGFMT
 
 tmp="${TEST_DIR}"/$$
@@ -146,12 +139,6 @@ _timestamp()
 
 _wrapup()
 {
-# for hangcheck ...
-# remove files that were used by hangcheck
-#
-[ -f "${TEST_DIR}"/check.pid ] && rm -rf "${TEST_DIR}"/check.pid
-[ -f "${TEST_DIR}"/check.sts ] && rm -rf "${TEST_DIR}"/check.sts
-
 if $showme
 then
 :
@@ -207,24 +194,6 @@ END{ if (NR > 0) {
 
 trap "_wrapup; exit \$status" 0 1 2 3 15
 
-# for hangcheck ...
-# Save pid of check in a well known place, so that hangcheck can be sure it
-# has the right pid (getting the pid from ps output is not reliable enough).
-#
-rm -rf "${TEST_DIR}"/check.pid
-echo $$ > "${TEST_DIR}"/check.pid
-
-# for hangcheck ...
-# Save the status of check in a well known place, so that hangcheck can be
-# sure to know where check is up to (getting test number from ps output is
-# not reliable enough since the trace stuff has been introduced).
-#
-rm -rf "${TEST_DIR}"/check.sts
-echo "preamble" > "${TEST_DIR}"/check.sts
-
-# don't leave old full output behind on a clean run
-rm -f check.full
-
 [ -f $TIMESTAMP_FILE ] || touch $TIMESTAMP_FILE
 
 FULL_IMGFMT_DETAILS=`_full_imgfmt_details`
@@ -287,9 +256,6 @@ do
 fi
 rm -f core $seq.notrun
 
-# for hangcheck ...
-echo "$seq" > "${TEST_DIR}"/check.sts
-
 start=`_wallclock`
 $timestamp && printf %s "[$(date "+%T")]"
 
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index d34c11c056..867918895b 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -19,17 +19,6 @@
 # common procedures for QA scripts
 #
 
-_setenvironment()
-{
-MSGVERB="text:action"
-export MSGVERB
-}
-
-rm -f "$OUTPUT_DIR/$iam.out"
-_setenvironment
-
-check=${check-true}
-
 diff="diff -u"
 verbose=false
 debug=false
@@ -40,7 +29,6 @@ showme=false
 sortme=false
 expunge=true
 have_test_arg=false
-randomize=false
 cachemode=false
 rm -f $tmp.list $tmp.tmp $tmp.sed
 
@@ -170,7 +158,6 @@ other options
 -n  show me, do not run tests
 -o options  -o options to pass to qemu-img create/convert
 -T  output timestamps
--r  randomize test order
 -c mode cache mode
 
 testlist options
@@ -327,11 +314,6 @@ testlist options
 cachemode=true
 xpand=false
 ;;
--r)# randomize test order
-randomize=true
-xpand=false
-;;
-
 -T)# turn on timestamp output
 timestamp=true
 xpand=false
@@ -445,11 +427,6 @@ fi
 list=`sort $tmp.list`
 rm -f $tmp.list $tmp.tmp $tmp.sed
 
-if $randomize
-then
-list=`echo $list | awk -f randomize.awk`
-fi
-
 [ "$QEMU" = "" ] && _fatal "qemu not found"
 [ "$QEMU_IMG" = "" ] && _fatal "qemu-img not found"
 [ "$QEMU_IO" = "" ] && _fatal "qemu-io not found"
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index e0883a0c65..b599c72211 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -15,33 +15,14 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .
 #
-#
-# setup and check for config parameters, and in particular
-#
-# EMAIL -   email of the script runner.
-# TEST_DIR -scratch test directory
-#
-# - These can be added to $HOST_CONFIG_DIR (witch default to ./config)
-#   below or a separate local configuration file can be used (using
-#   the HOST_OPTIONS variable).
-# - This script is shared by the stress test system and the auto-qa
-#   system (includes both regression test and benchmark components).
-# - this script shouldn't make any assertions about filesystem
-#   validity 

[Qemu-devel] [PATCH 10/12] qemu-iotests: get rid of $iam

[Paolo Bonzini]

The variable is almost unused, and one of the two uses is actually
uninitialized.

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/check | 5 +
 tests/qemu-iotests/common.rc | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index e5d1ae3d92..dc60e17e6a 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -30,12 +30,9 @@ interrupt=true
 # by default don't output timestamps
 timestamp=${TIMESTAMP:=false}
 
-# generic initialization
-iam=check
-
 _init_error()
 {
-echo "$iam: $1" >&2
+echo "check: $1" >&2
 exit 1
 }
 
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 7046a83974..d0aa22543e 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -43,7 +43,7 @@ poke_file()
 
 if ! . ./common.config
 then
-echo "$iam: failed to source common.config"
+echo "$0: failed to source common.config"
 exit 1
 fi
 
-- 
2.13.3

[Qemu-devel] [PATCH 09/12] qemu-iotests: do not search for binaries in the current directory

[Paolo Bonzini]

Looking in the build root is enough.

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/check | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 5b9c0c8ade..e5d1ae3d92 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -86,17 +86,17 @@ then
 fi
 fi
 
-if [[ -z $QEMU_IMG_PROG && -x "$build_root/qemu-img" && ! -x './qemu-img' ]]
+if [[ -z $QEMU_IMG_PROG && -x "$build_root/qemu-img" ]]
 then
 export QEMU_IMG_PROG="$build_root/qemu-img"
 fi
 
-if [[ -z $QEMU_IO_PROG && -x "$build_root/qemu-io" && ! -x './qemu-io' ]]
+if [[ -z $QEMU_IO_PROG && -x "$build_root/qemu-io" ]]
 then
 export QEMU_IO_PROG="$build_root/qemu-io"
 fi
 
-if [[ -z $QEMU_NBD_PROG && -x "$build_root/qemu-nbd" && ! -x './qemu-nbd' ]]
+if [[ -z $QEMU_NBD_PROG && -x "$build_root/qemu-nbd" ]]
 then
 export QEMU_NBD_PROG="$build_root/qemu-nbd"
 fi
-- 
2.13.3

[Qemu-devel] [PATCH 05/12] qemu-iotests: do not include common.rc in "check"

[Paolo Bonzini]

It only provides functions used by the test programs.

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/check |  6 --
 tests/qemu-iotests/common.rc | 13 +
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 5075029e89..1ef6d0ac3a 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -113,12 +113,6 @@ then
 _init_error "failed to source common.config"
 fi
 
-# we need common.rc
-if ! . "$source_iotests/common.rc"
-then
-_init_error "failed to source common.rc"
-fi
-
 # we need common
 . "$source_iotests/common"
 
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index ef5cdb3385..390aa81224 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -40,14 +40,11 @@ poke_file()
 printf "$3" | dd "of=$1" bs=1 "seek=$2" conv=notrunc &>/dev/null
 }
 
-# we need common.config
-if [ "$iam" != "check" ]
-then
-if ! . ./common.config
-then
-echo "$iam: failed to source common.config"
-exit 1
-fi
+
+if ! . ./common.config
+then
+echo "$iam: failed to source common.config"
+exit 1
 fi
 
 _fatal()
-- 
2.13.3

[Qemu-devel] [PATCH 08/12] qemu-iotests: fix uninitialized variable

[Paolo Bonzini]

The function is used in "common" but defined only after the file
is sourced.

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/check  | 2 --
 tests/qemu-iotests/common | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 1ef6d0ac3a..5b9c0c8ade 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -118,8 +118,6 @@ fi
 
 TIMESTAMP_FILE=check.time-$IMGPROTO-$IMGFMT
 
-tmp="${TEST_DIR}"/$$
-
 _wallclock()
 {
 date "+%H %M %S" | awk '{ print $1*3600 + $2*60 + $3 }'
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index adf427501e..35e4d0f87b 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -80,6 +80,8 @@ sortme=false
 expunge=true
 have_test_arg=false
 cachemode=false
+
+tmp="${TEST_DIR}"/$$
 rm -f $tmp.list $tmp.tmp $tmp.sed
 
 export IMGFMT=raw
-- 
2.13.3

[Qemu-devel] [PATCH 02/12] qemu-iotests: get rid of AWK_PROG

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/check | 4 ++--
 tests/qemu-iotests/common| 2 +-
 tests/qemu-iotests/common.config | 3 ---
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 01fd5a26e5..5075029e89 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -128,7 +128,7 @@ tmp="${TEST_DIR}"/$$
 
 _wallclock()
 {
-date "+%H %M %S" | $AWK_PROG '{ print $1*3600 + $2*60 + $3 }'
+date "+%H %M %S" | awk '{ print $1*3600 + $2*60 + $3 }'
 }
 
 _timestamp()
@@ -147,7 +147,7 @@ _wrapup()
 if [ -f $TIMESTAMP_FILE -a -f $tmp.time ]
 then
 cat $TIMESTAMP_FILE $tmp.time \
-| $AWK_PROG '
+| awk '
 { t[$1] = $2 }
 END{ if (NR > 0) {
 for (i in t) print i " " t[i]
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 867918895b..130f647a4d 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -366,7 +366,7 @@ testlist options
 if $xpand
 then
 have_test_arg=true
-$AWK_PROG 

[Qemu-devel] [PATCH 07/12] qemu-iotests: disintegrate more parts of common.config

[Paolo Bonzini]

Split "check" parts from tests part.

For the directory setup, the actual computation of directories goes
in "check", while the sanity checks go in the tests.

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/common| 24 
 tests/qemu-iotests/common.config | 49 
 tests/qemu-iotests/common.qemu   |  1 +
 tests/qemu-iotests/common.rc | 25 
 4 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 33557abe6c..adf427501e 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -61,6 +61,14 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
   export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
 fi
 
+if [ -z "$TEST_DIR" ]; then
+TEST_DIR=`pwd`/scratch
+fi
+
+if [ ! -e "$TEST_DIR" ]; then
+mkdir "$TEST_DIR"
+fi
+
 diff="diff -u"
 verbose=false
 debug=false
@@ -87,6 +95,15 @@ export VALGRIND_QEMU=
 export IMGKEYSECRET=
 export IMGOPTSSYNTAX=false
 
+default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
+default_alias_machine=$($QEMU_PROG -machine help | \
+   sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
+if [[ "$default_alias_machine" ]]; then
+default_machine="$default_alias_machine"
+fi
+
+export QEMU_DEFAULT_MACHINE="$default_machine"
+
 for r
 do
 
@@ -453,6 +470,13 @@ if [ "$IMGFMT" == "luks" ] && ! (echo "$IMGOPTS" | grep 
"iter-time=" > /dev/null
 IMGOPTS=$(_optstr_add "$IMGOPTS" "iter-time=10")
 fi
 
+if [ -z "$SAMPLE_IMG_DIR" ]; then
+SAMPLE_IMG_DIR="$source_iotests/sample_images"
+fi
+
+export TEST_DIR
+export SAMPLE_IMG_DIR
+
 if [ -s $tmp.list ]
 then
 # found some valid test numbers ... this is good
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index 9d535415b5..dd2ffe034d 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -25,8 +25,6 @@ HOSTOS=`uname -s`
 
 export PWD=`pwd`
 
-export _QEMU_HANDLE=0
-
 # make sure we have a standard umask
 umask 022
 
@@ -39,52 +37,5 @@ _optstr_add()
 fi
 }
 
-QEMU_IMG_EXTRA_ARGS=
-if [ "$IMGOPTSSYNTAX" = "true" ]; then
-QEMU_IMG_EXTRA_ARGS="--image-opts $QEMU_IMG_EXTRA_ARGS"
-if [ -n "$IMGKEYSECRET" ]; then
-QEMU_IMG_EXTRA_ARGS="--object secret,id=keysec0,data=$IMGKEYSECRET 
$QEMU_IMG_EXTRA_ARGS"
-fi
-fi
-export QEMU_IMG_EXTRA_ARGS
-
-
-default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
-default_alias_machine=$($QEMU_PROG -machine help | \
-   sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
-if [[ "$default_alias_machine" ]]; then
-default_machine="$default_alias_machine"
-fi
-
-export QEMU_DEFAULT_MACHINE="$default_machine"
-
-if [ -z "$TEST_DIR" ]; then
-TEST_DIR=`pwd`/scratch
-fi
-
-QEMU_TEST_DIR="${TEST_DIR}"
-
-if [ ! -e "$TEST_DIR" ]; then
-mkdir "$TEST_DIR"
-fi
-
-if [ ! -d "$TEST_DIR" ]; then
-echo "common.config: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
-exit 1
-fi
-
-export TEST_DIR
-
-if [ -z "$SAMPLE_IMG_DIR" ]; then
-SAMPLE_IMG_DIR="$source_iotests/sample_images"
-fi
-
-if [ ! -d "$SAMPLE_IMG_DIR" ]; then
-echo "common.config: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a 
directory"
-exit 1
-fi
-
-export SAMPLE_IMG_DIR
-
 # make sure this script returns success
 true
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index 7645f1dc72..9f9aecc9df 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -31,6 +31,7 @@ QEMU_FIFO_IN="${QEMU_TEST_DIR}/qmp-in-$$"
 QEMU_FIFO_OUT="${QEMU_TEST_DIR}/qmp-out-$$"
 
 QEMU_HANDLE=0
+export _QEMU_HANDLE=0
 
 # If bash version is >= 4.1, these will be overwritten and dynamic
 # file descriptor values assigned.
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 390aa81224..7046a83974 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -129,6 +129,10 @@ export QEMU_VXHS=_qemu_vxhs_wrapper
 
 if [ "$IMGOPTSSYNTAX" = "true" ]; then
 DRIVER="driver=$IMGFMT"
+QEMU_IMG_EXTRA_ARGS="--image-opts $QEMU_IMG_EXTRA_ARGS"
+if [ -n "$IMGKEYSECRET" ]; then
+QEMU_IMG_EXTRA_ARGS="--object secret,id=keysec0,data=$IMGKEYSECRET 
$QEMU_IMG_EXTRA_ARGS"
+fi
 if [ "$IMGFMT" = "luks" ]; then
 DRIVER="$DRIVER,key-secret=keysec0"
 fi
@@ -148,6 +152,7 @@ if [ "$IMGOPTSSYNTAX" = "true" ]; then
 
TEST_IMG="$DRIVER,file.driver=$IMGPROTO,file.filename=$TEST_DIR/t.$IMGFMT"
 fi
 else
+QEMU_IMG_EXTRA_ARGS=
 if [ "$IMGPROTO" = "file" ]; then
 TEST_IMG=$TEST_DIR/t.$IMGFMT
 elif [ "$IMGPROTO" = "nbd" ]; then
@@ -168,6 +173,26 @@ else
 fi
 ORIG_TEST_IMG="$TEST_IMG"
 
+if [ -z "$TEST_DIR" ]; then
+TEST_DIR=`pwd`/scratch
+fi
+
+QEMU_TEST_DIR="${TEST_DIR}"
+
+if [ ! -e "$TEST_DIR" ]; then
+mkdir "$TEST_DIR"
+fi
+
+if [ ! -d 

[Qemu-devel] [PATCH 06/12] qemu-iotests: do not do useless search for QEMU_*_PROG

[Paolo Bonzini]

With the exception of qnio_server, all the variables are initialized
by "check" prior to "common" being sourced.  They cannot be empty.
Only the "realpath" invocation is useful, and can be done just once
in "check" rather than in the tests.

For qnio_server, move the detection to "common", simplifying
set_prog_path to stop handling the unused second argument, and
embedding the "realpath" pass.

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/common| 19 ++
 tests/qemu-iotests/common.config | 54 
 2 files changed, 19 insertions(+), 54 deletions(-)

diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index f58e56fc40..33557abe6c 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -42,6 +42,25 @@ _full_platform_details()
 echo "$os/$platform $host $kernel"
 }
 
+# $1 = prog to look for
+set_prog_path()
+{
+p=`command -v $1 2> /dev/null`
+if [ -n "$p" -a -x "$p" ]; then
+realpath -- "$(type -p "$p")"
+else
+return 1
+fi
+}
+
+export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
+export QEMU_IMG_PROG=$(realpath -- "$(type -p "$QEMU_IMG_PROG")")
+export QEMU_IO_PROG=$(realpath -- "$(type -p "$QEMU_IO_PROG")")
+export QEMU_NBD_PROG=$(realpath -- "$(type -p "$QEMU_NBD_PROG")")
+if [ -z "$QEMU_VXHS_PROG" ]; then
+  export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
+fi
+
 diff="diff -u"
 verbose=false
 debug=false
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index ee10c23672..9d535415b5 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -30,28 +30,6 @@ export _QEMU_HANDLE=0
 # make sure we have a standard umask
 umask 022
 
-# $1 = prog to look for, $2* = default pathnames if not found in $PATH
-set_prog_path()
-{
-p=`command -v $1 2> /dev/null`
-if [ -n "$p" -a -x "$p" ]; then
-echo $p
-return 0
-fi
-p=$1
-
-shift
-for f; do
-if [ -x $f ]; then
-echo $f
-return 0
-fi
-done
-
-echo ""
-return 1
-}
-
 _optstr_add()
 {
 if [ -n "$1" ]; then
@@ -61,38 +39,6 @@ _optstr_add()
 fi
 }
 
-if [ -z "$QEMU_PROG" ]; then
-export QEMU_PROG="`set_prog_path qemu`"
-fi
-
-if [ -z "$QEMU_IMG_PROG" ]; then
-export QEMU_IMG_PROG="`set_prog_path qemu-img`"
-fi
-
-if [ -z "$QEMU_IO_PROG" ]; then
-export QEMU_IO_PROG="`set_prog_path qemu-io`"
-fi
-
-if [ -z "$QEMU_NBD_PROG" ]; then
-export QEMU_NBD_PROG="`set_prog_path qemu-nbd`"
-fi
-
-if [ -z "$QEMU_VXHS_PROG" ]; then
-export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
-fi
-
-export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
-export QEMU_IMG_PROG=$(realpath -- "$(type -p "$QEMU_IMG_PROG")")
-export QEMU_IO_PROG=$(realpath -- "$(type -p "$QEMU_IO_PROG")")
-export QEMU_NBD_PROG=$(realpath -- "$(type -p "$QEMU_NBD_PROG")")
-
-# This program is not built as part of qemu but (possibly) provided by the
-# system, so it may not be present at all
-if [ -n "$QEMU_VXHS_PROG" ]; then
-export QEMU_VXHS_PROG=$(realpath -- "$(type -p "$QEMU_VXHS_PROG")")
-fi
-
-
 QEMU_IMG_EXTRA_ARGS=
 if [ "$IMGOPTSSYNTAX" = "true" ]; then
 QEMU_IMG_EXTRA_ARGS="--image-opts $QEMU_IMG_EXTRA_ARGS"
-- 
2.13.3

[Qemu-devel] [PATCH 03/12] qemu-iotests: move "check" code out of common.rc

[Paolo Bonzini]

Some functions in common.rc are never used by the tests.  Move
them out of that file and into common, which is already included
only by "check".

Code that actually *is* common to "check" and tests can be placed in
common.config.

Signed-off-by: Paolo Bonzini 
---
 tests/qemu-iotests/common| 30 ++-
 tests/qemu-iotests/common.config | 12 +++
 tests/qemu-iotests/common.rc | 45 
 3 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 130f647a4d..50720f080f 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -19,6 +19,29 @@
 # common procedures for QA scripts
 #
 
+_full_imgfmt_details()
+{
+if [ -n "$IMGOPTS" ]; then
+echo "$IMGFMT ($IMGOPTS)"
+else
+echo "$IMGFMT"
+fi
+}
+
+_full_imgproto_details()
+{
+echo "$IMGPROTO"
+}
+
+_full_platform_details()
+{
+os=`uname -s`
+host=`hostname -s`
+kernel=`uname -r`
+platform=`uname -m`
+echo "$os/$platform $host $kernel"
+}
+
 diff="diff -u"
 verbose=false
 debug=false
@@ -404,7 +427,12 @@ if [ "$IMGOPTSSYNTAX" != "true" ]; then
 fi
 
 # Set default options for qemu-img create -o if they were not specified
-_set_default_imgopts
+if [ "$IMGFMT" == "qcow2" ] && ! (echo "$IMGOPTS" | grep "compat=" > 
/dev/null); then
+IMGOPTS=$(_optstr_add "$IMGOPTS" "compat=1.1")
+fi
+if [ "$IMGFMT" == "luks" ] && ! (echo "$IMGOPTS" | grep "iter-time=" > 
/dev/null); then
+IMGOPTS=$(_optstr_add "$IMGOPTS" "iter-time=10")
+fi
 
 if [ -s $tmp.list ]
 then
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index 0f571d46eb..91da65f3dc 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -27,6 +27,9 @@ export PWD=`pwd`
 
 export _QEMU_HANDLE=0
 
+# make sure we have a standard umask
+umask 022
+
 # $1 = prog to look for, $2* = default pathnames if not found in $PATH
 set_prog_path()
 {
@@ -49,6 +52,15 @@ set_prog_path()
 return 1
 }
 
+_optstr_add()
+{
+if [ -n "$1" ]; then
+echo "$1,$2"
+else
+echo "$2"
+fi
+}
+
 _fatal()
 {
 echo "$*"
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index dd91a2b79a..6f6e03366f 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -50,9 +50,6 @@ then
 fi
 fi
 
-# make sure we have a standard umask
-umask 022
-
 if [ "$IMGOPTSSYNTAX" = "true" ]; then
 DRIVER="driver=$IMGFMT"
 if [ "$IMGFMT" = "luks" ]; then
@@ -94,25 +91,6 @@ else
 fi
 ORIG_TEST_IMG="$TEST_IMG"
 
-_optstr_add()
-{
-if [ -n "$1" ]; then
-echo "$1,$2"
-else
-echo "$2"
-fi
-}
-
-_set_default_imgopts()
-{
-if [ "$IMGFMT" == "qcow2" ] && ! (echo "$IMGOPTS" | grep "compat=" > 
/dev/null); then
-IMGOPTS=$(_optstr_add "$IMGOPTS" "compat=1.1")
-fi
-if [ "$IMGFMT" == "luks" ] && ! (echo "$IMGOPTS" | grep "iter-time=" > 
/dev/null); then
-IMGOPTS=$(_optstr_add "$IMGOPTS" "iter-time=10")
-fi
-}
-
 _use_sample_img()
 {
 SAMPLE_IMG_FILE="${1%\.bz2}"
@@ -428,28 +406,5 @@ _require_command()
 [ -x "$c" ] || _notrun "$1 utility required, skipped this test"
 }
 
-_full_imgfmt_details()
-{
-if [ -n "$IMGOPTS" ]; then
-echo "$IMGFMT ($IMGOPTS)"
-else
-echo "$IMGFMT"
-fi
-}
-
-_full_imgproto_details()
-{
-echo "$IMGPROTO"
-}
-
-_full_platform_details()
-{
-os=`uname -s`
-host=`hostname -s`
-kernel=`uname -r`
-platform=`uname -m`
-echo "$os/$platform $host $kernel"
-}
-
 # make sure this script returns success
 true
-- 
2.13.3

[Qemu-devel] [PATCH 00/12] cleanup qemu-iotests

[Paolo Bonzini]

The purpose of this series is to separate the "check" sources from
the tests.  After these patches, common.config is reduced to simple
shell initialization, and common.rc is only included by the tests.

Along the way, a lot of dead code is removed too.

Paolo

Paolo Bonzini (12):
  qemu-iotests: remove dead code
  qemu-iotests: get rid of AWK_PROG
  qemu-iotests: move "check" code out of common.rc
  qemu-iotests: limit non-_PROG-suffixed variables to common.rc
  qemu-iotests: do not include common.rc in "check"
  qemu-iotests: do not do useless search for QEMU_*_PROG
  qemu-iotests: disintegrate more parts of common.config
  qemu-iotests: fix uninitialized variable
  qemu-iotests: do not search for binaries in the current directory
  qemu-iotests: get rid of $iam
  qemu-iotests: include common.env and common.config early
  qemu-iotests: merge "check" and "common"

 tests/qemu-iotests/039.out   |  10 +-
 tests/qemu-iotests/061.out   |   4 +-
 tests/qemu-iotests/137.out   |   2 +-
 tests/qemu-iotests/check | 551 ++-
 tests/qemu-iotests/common| 459 
 tests/qemu-iotests/common.config | 205 +--
 tests/qemu-iotests/common.qemu   |   1 +
 tests/qemu-iotests/common.rc | 225 
 8 files changed, 615 insertions(+), 842 deletions(-)
 delete mode 100644 tests/qemu-iotests/common

-- 
2.13.3

[Qemu-devel] [PATCH v3 1/2] tests/vhost-user-bridge: disable debug output by default

[Jens Freimann]

From: Jens Freimann 

vhost-user-bridge prints out a lot of information, including dumps
of all transmitted data. When called from a testcase this output
clutters the actual test results, so let's make the default no debug
output.

Reviewed-by: Maxime Coquelin 
Signed-off-by: Jens Freimann 
---
 tests/vhost-user-bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
index 1e5b5ca3da..93d95353d7 100644
--- a/tests/vhost-user-bridge.c
+++ b/tests/vhost-user-bridge.c
@@ -34,7 +34,7 @@
 #include "standard-headers/linux/virtio_net.h"
 #include "contrib/libvhost-user/libvhost-user.h"
 
-#define VHOST_USER_BRIDGE_DEBUG 1
+#define VHOST_USER_BRIDGE_DEBUG 0
 
 #define DPRINT(...) \
 do { \
-- 
2.13.3

[Qemu-devel] [PATCH v3 0/2] tests/pxe-test: add testcase using vhost-user-bridge

[Jens Freimann]

This implements a testcase for pxe-test using the vhost-user interface. Spawn a
vhost-user-bridge process and connect it to the qemu process.

changes v2->v3:
- fixed build on aarch64 by removing a flag for g_spawn_async() that is
  not even needed. Patch builds on my system for target aarch64 now

changes v1->v2:

- new patch 4/5. Necessary to make qtest_add_abrt_handler work. 
- get rid of hugepagefs specific code (mst)
- use htonl and INADDR_LOOPBACK to set remote address (Stefan Hajnoczi)
- add qtest abort handler (Stefan Hajnoczi)
- spawn vhost-user-bridge process with flag DO_NOT_REAP_CHILD and add a child
  watch function (Stefan Hajnoczi)
  Killing the vubr process manually at the end of the test and watching
  its termination with the child watch function. 
- clean up pxe-test-disk images 
- add Jason Wang to Cc

regards,
Jens


Jens Freimann (2):
  tests/vhost-user-bridge: disable debug output by default
  tests/pxe-test: add testcase using vhost-user-bridge

 tests/Makefile.include|   4 +-
 tests/pxe-test.c  | 114 +-
 tests/vhost-user-bridge.c |   2 +-
 3 files changed, 117 insertions(+), 3 deletions(-)

-- 
2.13.3

Re: [Qemu-devel] [PATCH 18/47] MAINTAINERS: add missing TCG entry

[Philippe Mathieu-Daudé]

[I added mails of other person who reply to this series, this mail is 
not directly addressed to Alex]


On 08/09/2017 11:38 AM, Alex Bennée wrote:

Philippe Mathieu-Daudé  writes:

[...]

Do you think there should be another entry in "Block QAPI, monitor,
command line"?
or this file (and related include) rather deserves an own section
(possibly tagged Odd Fixes) to unburden Richard?


Well the default for un-matched files is qemu-devel right? It would be
nice for it to be properly maintained but that requires someone to
step-up to the plate.


I wonder if I'm understanding correctly what the MAINTAINERS file is for 
and how to use it.


From an submitter view I feel a bit confused. I thought 
./get_maintainer.pl would give me the list of person to email the 
changes I did on some file from the repository. This script seems 
correctly named, I'm looking for some ./get_reviewers.pl instead, to 
know who I'v to keep updated, apart from the ./get_maintainer.pl.


currently we have:
"M: Mail patches to: FullName "
Does this imply FullName is a maintainer?
If so is it ok I do this change:

-M: Mail patches to: FullName 
+M: Maintainer: FullName 
+   These maintainers must be mailed on patches.
 R: Designated reviewer: FullName 
These reviewers should be CCed on patches.


actual default for un-matched: "recent contributors" + qemu-devel@

  $ ./scripts/get_maintainer.pl -f disas.c
  get_maintainer.pl: No maintainers found, printing recent contributors.
  get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.
  Peter Maydell  (commit_signer:2/3=67%)
  Richard Henderson  (commit_signer:1/3=33%)
  Thomas Huth  (commit_signer:1/3=33%)
  Michael Tokarev  (commit_signer:1/3=33%)
  Julian Brown  (commit_signer:1/3=33%)
  qemu-devel@nongnu.org (open list:All patches CC here)

I find the un-matched "recent contributors" list often confuse, due to 
files being moved, header updated, checkpatch indented.
Most of the time you get the list of queue maintainers since they accept 
patches and sign-of the pull request.


Having to use 'git log --follow' and 'git blame' to figure out is not 
bad, to be aware of who modified this file before you, but there are 
some files I already hit 3 times in different series, and wondered about 
how avoid those manual steps.


Anyway I now understand these recent contributors are not maintainers 
but no-designated reviewers, unwilling to be maintainers (else they'd 
have added a section/entry by themselves).


I also understand why Fam said it sounds weird to add an new section as 
"Orphan".


For the linux-headers case, if people want to be notified of changes the 
easiest seems to modify the update-linux-headers.sh script.


I'll review the whole series thinking differently and resend when 2.11 
opens.


Regards,

Phil.

[Qemu-devel] [PATCH v2 4/5] qcow2: Drop debugging dump_refcounts()

[Eric Blake]

It's been #if 0'd since its introduction in 2006, commit 585f8587.
We can revive dead code if we need it, but in the meantime, it has
bit-rotted (for example, not checking for failure in bdrv_getlength()).

Signed-off-by: Eric Blake 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Jeff Cody 
Reviewed-by: Kevin Wolf 
Reviewed-by: John Snow 
---
 block/qcow2.c | 21 -
 1 file changed, 21 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index d7c600b5a2..99407403ea 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3798,27 +3798,6 @@ static ImageInfoSpecific 
*qcow2_get_specific_info(BlockDriverState *bs)
 return spec_info;
 }

-#if 0
-static void dump_refcounts(BlockDriverState *bs)
-{
-BDRVQcow2State *s = bs->opaque;
-int64_t nb_clusters, k, k1, size;
-int refcount;
-
-size = bdrv_getlength(bs->file->bs);
-nb_clusters = size_to_clusters(s, size);
-for(k = 0; k < nb_clusters;) {
-k1 = k;
-refcount = get_refcount(bs, k);
-k++;
-while (k < nb_clusters && get_refcount(bs, k) == refcount)
-k++;
-printf("%" PRId64 ": refcount=%d nb=%" PRId64 "\n", k, refcount,
-   k - k1);
-}
-}
-#endif
-
 static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
   int64_t pos)
 {
-- 
2.13.4

[Qemu-devel] [PATCH] target/ppc: 'PVR != host PVR' in KVM_SET_SREGS workaround

[Daniel Henrique Barboza]

Commit d5fc133eed ("ppc: Rework CPU compatibility testing
across migration") changed the way cpu_post_load behaves with
the PVR setting, causing an unexpected bug in KVM-HV migrations
between hosts that are compatible (POWER8 and POWER8E, for example).
Even with pvr_match() returning true, the guest freezes right after
cpu_post_load. The reason is that the guest kernel can't handle a
different PVR value other that the running host in KVM_SET_SREGS.

In [1] it was discussed the possibility of a new KVM capability
that would indicate that the guest kernel can handle a different
PVR in KVM_SET_SREGS. Even if such feature is implemented, there is
still the problem with older kernels that will not have this capability
and will fail to migrate.

This patch implements a workaround for that scenario. If running
with KVM, check if the guest kernel does not have the capability
(named here as 'cap_ppc_pvr_compat'). If it doesn't, calls
kvmppc_is_pr() to see if the guest is running in KVM-HV. If all this
happens, set env->spr[SPR_PVR] to the same value as the current
host PVR. This ensures that we allow migrations with 'close enough'
PVRs to still work in KVM-HV but also makes the code ready for
this new KVM capability when it is done.

A new function called 'kvmppc_pvr_workaround_required' was created
to encapsulate the conditions said above and to avoid calling too
many kvm.c internals inside cpu_post_load.

[1] https://lists.gnu.org/archive/html/qemu-ppc/2017-06/msg00503.html

Signed-off-by: Daniel Henrique Barboza 
---
 target/ppc/kvm.c | 34 ++
 target/ppc/kvm_ppc.h |  1 +
 target/ppc/machine.c | 22 ++
 3 files changed, 57 insertions(+)

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 8571379..fb3ee43 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -90,6 +90,7 @@ static int cap_htm; /* Hardware transactional 
memory support */
 static int cap_mmu_radix;
 static int cap_mmu_hash_v3;
 static int cap_resize_hpt;
+static int cap_ppc_pvr_compat;
 
 static uint32_t debug_inst_opcode;
 
@@ -147,6 +148,13 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 cap_mmu_radix = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_RADIX);
 cap_mmu_hash_v3 = kvm_vm_check_extension(s, KVM_CAP_PPC_MMU_HASH_V3);
 cap_resize_hpt = kvm_vm_check_extension(s, KVM_CAP_SPAPR_RESIZE_HPT);
+/*
+ * Note: setting it to false because there is not such capability
+ * in KVM at this moment.
+ *
+ * TODO: call kvm_vm_check_extension() with the right capability
+ * after the kernel starts implementing it.*/
+cap_ppc_pvr_compat = false;
 
 if (!cap_interrupt_level) {
 fprintf(stderr, "KVM: Couldn't find level irq capability. Expect the "
@@ -2785,3 +2793,29 @@ void kvmppc_update_sdr1(target_ulong sdr1)
 run_on_cpu(cs, kvmppc_pivot_hpt_cpu, RUN_ON_CPU_TARGET_PTR(sdr1));
 }
 }
+
+/*
+ * This is a helper function to detect a post migration scenario
+ * in which a guest, running as KVM-HV, freezes in cpu_post_load because
+ * the guest kernel can't handle a PVR value other than the actual host
+ * PVR in KVM_SET_SREGS, even if pvr_match() returns true.
+ *
+ * If we don't have cap_ppc_pvr_compat and we're not running in PR
+ * (so, we're HV), return true. The workaround itself is done in
+ * cpu_post_load.
+ *
+ * The order here is important: we'll only check for KVM PR as a
+ * fallback if the guest kernel can't handle the situation itself.
+ * We need to avoid as much as possible querying the running KVM type
+ * in QEMU level.
+ */
+bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu)
+{
+CPUState *cs = CPU(cpu);
+
+if (cap_ppc_pvr_compat) {
+return false;
+}
+
+return !kvmppc_is_pr(cs->kvm_state);
+}
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 6bc6fb3..381afe6 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -67,6 +67,7 @@ void kvmppc_check_papr_resize_hpt(Error **errp);
 int kvmppc_resize_hpt_prepare(PowerPCCPU *cpu, target_ulong flags, int shift);
 int kvmppc_resize_hpt_commit(PowerPCCPU *cpu, target_ulong flags, int shift);
 void kvmppc_update_sdr1(target_ulong sdr1);
+bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
 
 bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path);
 
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index f578156..e545885 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -9,6 +9,7 @@
 #include "mmu-hash64.h"
 #include "migration/cpu.h"
 #include "qapi/error.h"
+#include "kvm_ppc.h"
 
 static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
 {
@@ -250,6 +251,27 @@ static int cpu_post_load(void *opaque, int version_id)
 }
 }
 
+/*
+ * If we're running with KVM HV, there is a chance that the guest
+ * is running with KVM HV and its kernel does not have the
+ * capability of dealing with a different PVR other than this
+ * 

Re: [Qemu-devel] [PATCH 1/2] IDE: Do not flush empty CDROM drives

[Eric Blake]

On 08/09/2017 11:02 AM, Stefan Hajnoczi wrote:
> The block backend changed in a way that flushing empty CDROM drives now
> crashes.  Amend IDE to avoid doing so until the root problem can be
> addressed for 2.11.
> 
> Original patch by John Snow .
> 
> Reported-by: Kieron Shorrock 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  hw/ide/core.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 0b48b64d3a..bea39536b0 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -1063,7 +1063,15 @@ static void ide_flush_cache(IDEState *s)
>  s->status |= BUSY_STAT;
>  ide_set_retry(s);
>  block_acct_start(blk_get_stats(s->blk), >acct, 0, BLOCK_ACCT_FLUSH);
> -s->pio_aiocb = blk_aio_flush(s->blk, ide_flush_cb, s);
> +
> +if (blk_bs(s->blk)) {
> +s->pio_aiocb = blk_aio_flush(s->blk, ide_flush_cb, s);
> +} else {
> +/* XXX blk_aio_flush() crashes when blk_bs(blk) is NULL, remove this
> + * temporary workaround when blk_aio_*() functions handle NULL 
> blk_bs.
> + */
> +ide_flush_cb(s, 0);
> +}
>  }
>  
>  static void ide_cfata_metadata_inquiry(IDEState *s)
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH for-2.10 v2 0/5] More bdrv_getlength() fixes

[Eric Blake]

We already have a lot of bdrv_getlength() fixes in -rc2; so I think
this is still okay for -rc3.

v1 was here (with a typo'd subject line):
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01226.html

Since v1:
- patch 1: fix error message capitalization (Kevin, R-b kept)
- fix locking bug in original patch 2 (Kevin)
- split original patch 2 into two parts: signature update, and
added error checking (Kevin)
- check for unlikely integer overflow before bdrv_truncate (Jeff)

001/5:[0002] [FC] 'vpc: Check failure of bdrv_getlength()'
002/5:[down] 'qcow: Change signature of get_cluster_offset()'
003/5:[0048] [FC] 'qcow: Check failure of bdrv_getlength() and bdrv_truncate()'
004/5:[] [--] 'qcow2: Drop debugging dump_refcounts()'
005/5:[] [--] 'qcow2: Check failure of bdrv_getlength()'

Eric Blake (5):
  vpc: Check failure of bdrv_getlength()
  qcow: Change signature of get_cluster_offset()
  qcow: Check failure of bdrv_getlength() and bdrv_truncate()
  qcow2: Drop debugging dump_refcounts()
  qcow2: Check failure of bdrv_getlength()

 block/qcow.c  | 165 ++
 block/qcow2.c |  26 ++---
 block/vpc.c   |   9 +++-
 3 files changed, 110 insertions(+), 90 deletions(-)

-- 
2.13.4

[Qemu-devel] [PATCH v2 2/5] qcow: Change signature of get_cluster_offset()

[Eric Blake]

The old signature has an ambiguous meaning for a return of 0:
either no allocation was requested or necessary, or an error
occurred (but any errno associated with the error is lost to
the caller, which then has to assume EIO).

Better is to follow the example of qcow2, by changing the
signature to have a separate return value that cleanly
distinguishes between failure and success, along with a
parameter that cleanly holds a 64-bit value.  Then update all
callers.

While auditing that all return paths return a negative errno
(rather than -1), I also simplified places where we can pass
NULL rather than a local Error that just gets thrown away.

Suggested-by: Kevin Wolf 
Signed-off-by: Eric Blake 
---
 block/qcow.c | 135 +--
 1 file changed, 76 insertions(+), 59 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index c08cdc4a7b..d07bef6306 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -347,19 +347,21 @@ static int qcow_reopen_prepare(BDRVReopenState *state,
  * 'compressed_size'. 'compressed_size' must be > 0 and <
  * cluster_size
  *
- * return 0 if not allocated.
+ * return 0 if not allocated, 1 if *result is assigned, and negative
+ * errno on failure.
  */
-static uint64_t get_cluster_offset(BlockDriverState *bs,
-   uint64_t offset, int allocate,
-   int compressed_size,
-   int n_start, int n_end)
+static int get_cluster_offset(BlockDriverState *bs,
+  uint64_t offset, int allocate,
+  int compressed_size,
+  int n_start, int n_end, uint64_t *result)
 {
 BDRVQcowState *s = bs->opaque;
-int min_index, i, j, l1_index, l2_index;
+int min_index, i, j, l1_index, l2_index, ret;
 uint64_t l2_offset, *l2_table, cluster_offset, tmp;
 uint32_t min_count;
 int new_l2_table;

+*result = 0;
 l1_index = offset >> (s->l2_bits + s->cluster_bits);
 l2_offset = s->l1_table[l1_index];
 new_l2_table = 0;
@@ -373,10 +375,12 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 /* update the L1 entry */
 s->l1_table[l1_index] = l2_offset;
 tmp = cpu_to_be64(l2_offset);
-if (bdrv_pwrite_sync(bs->file,
-s->l1_table_offset + l1_index * sizeof(tmp),
-, sizeof(tmp)) < 0)
-return 0;
+ret = bdrv_pwrite_sync(bs->file,
+   s->l1_table_offset + l1_index * sizeof(tmp),
+   , sizeof(tmp));
+if (ret < 0) {
+return ret;
+}
 new_l2_table = 1;
 }
 for(i = 0; i < L2_CACHE_SIZE; i++) {
@@ -403,14 +407,17 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 l2_table = s->l2_cache + (min_index << s->l2_bits);
 if (new_l2_table) {
 memset(l2_table, 0, s->l2_size * sizeof(uint64_t));
-if (bdrv_pwrite_sync(bs->file, l2_offset, l2_table,
-s->l2_size * sizeof(uint64_t)) < 0)
-return 0;
+ret = bdrv_pwrite_sync(bs->file, l2_offset, l2_table,
+   s->l2_size * sizeof(uint64_t));
+if (ret < 0) {
+return ret;
+}
 } else {
-if (bdrv_pread(bs->file, l2_offset, l2_table,
-   s->l2_size * sizeof(uint64_t)) !=
-s->l2_size * sizeof(uint64_t))
-return 0;
+ret = bdrv_pread(bs->file, l2_offset, l2_table,
+ s->l2_size * sizeof(uint64_t));
+if (ret < 0) {
+return ret;
+}
 }
 s->l2_cache_offsets[min_index] = l2_offset;
 s->l2_cache_counts[min_index] = 1;
@@ -427,16 +434,18 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 /* if the cluster is already compressed, we must
decompress it in the case it is not completely
overwritten */
-if (decompress_cluster(bs, cluster_offset) < 0)
-return 0;
+if (decompress_cluster(bs, cluster_offset) < 0) {
+return -EIO;
+}
 cluster_offset = bdrv_getlength(bs->file->bs);
 cluster_offset = (cluster_offset + s->cluster_size - 1) &
 ~(s->cluster_size - 1);
 /* write the cluster content */
-if (bdrv_pwrite(bs->file, cluster_offset, s->cluster_cache,
-s->cluster_size) !=
-s->cluster_size)
-return -1;
+ret = bdrv_pwrite(bs->file, cluster_offset, s->cluster_cache,
+  s->cluster_size);
+if (ret < 0) {
+return ret;
+}
 } else {
 cluster_offset = bdrv_getlength(bs->file->bs);
 if (allocate == 1) {
@@ -454,20 +463,19 @@ static uint64_t 

[Qemu-devel] [PATCH v2 5/5] qcow2: Check failure of bdrv_getlength()

[Eric Blake]

qcow2_co_pwritev_compressed() should not call bdrv_truncate()
if determining the size failed.

Reported-by: Markus Armbruster 
Signed-off-by: Eric Blake 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Jeff Cody 
Reviewed-by: Kevin Wolf 
Reviewed-by: John Snow 
---
 block/qcow2.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 99407403ea..40ba26c111 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3282,12 +3282,15 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, 
uint64_t offset,
 z_stream strm;
 int ret, out_len;
 uint8_t *buf, *out_buf;
-uint64_t cluster_offset;
+int64_t cluster_offset;

 if (bytes == 0) {
 /* align end of file to a sector boundary to ease reading with
sector based I/Os */
 cluster_offset = bdrv_getlength(bs->file->bs);
+if (cluster_offset < 0) {
+return cluster_offset;
+}
 return bdrv_truncate(bs->file, cluster_offset, PREALLOC_MODE_OFF, 
NULL);
 }

-- 
2.13.4

[Qemu-devel] [NOTFORMERGE PATCH for-2.11 v3 16/16] travis/osx: build using bleeding Xcode

[Philippe Mathieu-Daudé]

test 'unstable' Xcode (9beta4) ...
OS X 10.13 (darwin16.7.0)
Apple LLVM version 9.0.0 (clang-900.0.31)

see https://docs.travis-ci.com/user/reference/osx/#OS-X-Version

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index cd59570062..16c1dee150 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -130,6 +130,11 @@ matrix:
   # newer stable available, OS X 10.12
   osx_image: xcode8.3
   compiler: clang
+- env: CONFIG=""
+  os: osx
+  # unstable
+  osx_image: xcode9
+  compiler: clang
 # Plain Trusty System Build
 - env: CONFIG="--disable-linux-user"
   sudo: required
-- 
2.13.3

[Qemu-devel] [PATCH v2 1/5] vpc: Check failure of bdrv_getlength()

[Eric Blake]

vpc_open() was checking for bdrv_getlength() failure in one, but
not the other, location.

Reported-by: Markus Armbruster 
Signed-off-by: Eric Blake 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Jeff Cody 
Reviewed-by: John Snow 

---
v2: error message tweak [Kevin]
---
 block/vpc.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/block/vpc.c b/block/vpc.c
index 574879ba7c..82911ebead 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -219,6 +219,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
 uint64_t pagetable_size;
 int disk_type = VHD_DYNAMIC;
 int ret;
+int64_t bs_size;

 bs->file = bdrv_open_child(NULL, options, "file", bs, _file,
false, errp);
@@ -411,7 +412,13 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
 }
 }

-if (s->free_data_block_offset > bdrv_getlength(bs->file->bs)) {
+bs_size = bdrv_getlength(bs->file->bs);
+if (bs_size < 0) {
+error_setg_errno(errp, -bs_size, "Unable to learn image size");
+ret = bs_size;
+goto fail;
+}
+if (s->free_data_block_offset > bs_size) {
 error_setg(errp, "block-vpc: free_data_block_offset points after "
  "the end of file. The image has been truncated.");
 ret = -EINVAL;
-- 
2.13.4

[Qemu-devel] [PATCH v2 3/5] qcow: Check failure of bdrv_getlength() and bdrv_truncate()

[Eric Blake]

Omitting the check for whether bdrv_getlength() and bdrv_truncate()
failed meant that it was theoretically possible to return an
incorrect offset to the caller.  More likely, conditions for either
of these functions to fail would also cause one of our other calls
(such as bdrv_pread() or bdrv_pwrite_sync()) to also fail, but
auditing that we are safe is difficult compared to just patching
things to always forward on the error rather than ignoring it.

Use osdep.h macros instead of open-coded rounding while in the
area.

Reported-by: Markus Armbruster 
Signed-off-by: Eric Blake 

---
v2: Rebase to new signature to not corrupt compressed clusters, and
fix lock handling [Kevin], check for truncate overflow [Jeff]
---
 block/qcow.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index d07bef6306..f450b00cfc 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -357,7 +357,8 @@ static int get_cluster_offset(BlockDriverState *bs,
 {
 BDRVQcowState *s = bs->opaque;
 int min_index, i, j, l1_index, l2_index, ret;
-uint64_t l2_offset, *l2_table, cluster_offset, tmp;
+int64_t l2_offset;
+uint64_t *l2_table, cluster_offset, tmp;
 uint32_t min_count;
 int new_l2_table;

@@ -370,8 +371,11 @@ static int get_cluster_offset(BlockDriverState *bs,
 return 0;
 /* allocate a new l2 entry */
 l2_offset = bdrv_getlength(bs->file->bs);
+if (l2_offset < 0) {
+return l2_offset;
+}
 /* round to cluster size */
-l2_offset = (l2_offset + s->cluster_size - 1) & ~(s->cluster_size - 1);
+l2_offset = QEMU_ALIGN_UP(l2_offset, s->cluster_size);
 /* update the L1 entry */
 s->l1_table[l1_index] = l2_offset;
 tmp = cpu_to_be64(l2_offset);
@@ -438,8 +442,10 @@ static int get_cluster_offset(BlockDriverState *bs,
 return -EIO;
 }
 cluster_offset = bdrv_getlength(bs->file->bs);
-cluster_offset = (cluster_offset + s->cluster_size - 1) &
-~(s->cluster_size - 1);
+if ((int64_t) cluster_offset < 0) {
+return cluster_offset;
+}
+cluster_offset = QEMU_ALIGN_UP(cluster_offset, s->cluster_size);
 /* write the cluster content */
 ret = bdrv_pwrite(bs->file, cluster_offset, s->cluster_cache,
   s->cluster_size);
@@ -448,12 +454,20 @@ static int get_cluster_offset(BlockDriverState *bs,
 }
 } else {
 cluster_offset = bdrv_getlength(bs->file->bs);
+if ((int64_t) cluster_offset < 0) {
+return cluster_offset;
+}
 if (allocate == 1) {
 /* round to cluster size */
-cluster_offset = (cluster_offset + s->cluster_size - 1) &
-~(s->cluster_size - 1);
-bdrv_truncate(bs->file, cluster_offset + s->cluster_size,
-  PREALLOC_MODE_OFF, NULL);
+cluster_offset = QEMU_ALIGN_UP(cluster_offset, 
s->cluster_size);
+if (cluster_offset + s->cluster_size > INT64_MAX) {
+return -E2BIG;
+}
+ret = bdrv_truncate(bs->file, cluster_offset + s->cluster_size,
+PREALLOC_MODE_OFF, NULL);
+if (ret < 0) {
+return ret;
+}
 /* if encrypted, we must initialize the cluster
content which won't be written */
 if (bs->encrypted &&
-- 
2.13.4

[Qemu-devel] [RFC PATCH for-2.11 v3 15/16] travis/osx: build using more Xcode versions

[Philippe Mathieu-Daudé]

currently default builder is based on Xcode 7.3.1:
OS X 10.11 (darwin14.5.0)
Apple LLVM version 7.3.0

add the oldest available Xcode (6.4):
OS X 10.10 (darwin14.5.0)
Apple LLVM 6.1.0 (based on LLVM 3.6.0svn)

and the newer available Xcode (8.3.3):
OS X 10.12 (darwin16.6.0)
Apple LLVM version 8.1.0

see https://docs.travis-ci.com/user/reference/osx/#OS-X-Version

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 12 
 1 file changed, 12 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 7c93a10c5f..cd59570062 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -117,6 +117,18 @@ matrix:
   compiler: gcc
 - env: CONFIG=""
   os: osx
+  # older available: OS X 10.10
+  osx_image: xcode6.4
+  compiler: clang
+- env: CONFIG=""
+  os: osx
+  # default: OS X 10.11.6
+  osx_image: xcode7.3
+  compiler: clang
+- env: CONFIG=""
+  os: osx
+  # newer stable available, OS X 10.12
+  osx_image: xcode8.3
   compiler: clang
 # Plain Trusty System Build
 - env: CONFIG="--disable-linux-user"
-- 
2.13.3

[Qemu-devel] [RFC PATCH for-2.11 v3 13/16] travis: use gcc-6 sanitizers

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 24f62fb7cf..1da542f99f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -185,8 +185,8 @@ matrix:
 - ubuntu-toolchain-r-test
   packages:
 # Extra toolchains
-- gcc-5
-- g++-5
+- gcc-6
+- g++-6
 # Build dependencies
 - glusterfs-common
 - libaio-dev
@@ -228,8 +228,8 @@ matrix:
   language: generic
   compiler: none
   env:
-- COMPILER_NAME=gcc CXX=g++-5 CC=gcc-5
-- CONFIG="--cc=gcc-5 --cxx=g++-5 --disable-pie --disable-linux-user"
+- COMPILER_NAME=gcc CXX=g++-6 CC=gcc-6
+- CONFIG="--disable-pie --disable-linux-user"
 - TEST_CMD=""
   before_script:
 - ./configure ${CONFIG} --extra-cflags="-g3 -O0 -fsanitize=thread 
-fuse-ld=gold" || (cat config.log; exit 1)
-- 
2.13.3

[Qemu-devel] [RFC PATCH for-2.11 v3 14/16] travis: allow_failures for gprof/gcov

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 1da542f99f..7c93a10c5f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -87,6 +87,7 @@ before_script:
 script:
   - make ${MAKEFLAGS} && ${TEST_CMD}
 matrix:
+  fast_finish: true
   include:
 # We manually include builds which we disable "make check" for, they also
 # are the faster jobs (no testing).
@@ -233,3 +234,8 @@ matrix:
 - TEST_CMD=""
   before_script:
 - ./configure ${CONFIG} --extra-cflags="-g3 -O0 -fsanitize=thread 
-fuse-ld=gold" || (cat config.log; exit 1)
+  allow_failures:
+- env: CONFIG="--enable-gprof --disable-pie"
+  compiler: gcc
+- env: CONFIG="--enable-gcov --disable-pie"
+  compiler: gcc
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 05/16] travis: retry if llvm.org timeouts

[Philippe Mathieu-Daudé]

example of failure: https://travis-ci.org/qemu/qemu/jobs/243232857

$ sudo apt-get update -qq
W: Failed to fetch 
http://llvm.org/apt/trusty/dists/llvm-toolchain-trusty-3.9/Release.gpg  
Connection failed
E: Some index files failed to download. They have been ignored, or old ones 
used instead.
The command "sudo apt-get update -qq" failed and exited with 100 during .
Your build has been stopped.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
---
 .travis.yml | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index d622d8c433..82c1819c93 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -143,10 +143,10 @@ matrix:
 - COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9
 - CONFIG="--disable-linux-user --cc=clang-3.9 --cxx=clang++-3.9"
   before_install:
-- wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo 
apt-key add -
-- sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty 
llvm-toolchain-trusty-3.9 main'
-- sudo apt-get update -qq
-- sudo apt-get install -qq -y clang-3.9
+- travis_retry wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key 
| sudo apt-key add -
+- travis_retry sudo apt-add-repository -y 'deb 
http://llvm.org/apt/trusty llvm-toolchain-trusty-3.9 main'
+- travis_retry sudo apt-get update -qq
+- travis_retry sudo apt-get install -qq -y clang-3.9
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
 - git submodule update --init --recursive
@@ -163,10 +163,10 @@ matrix:
 - COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9
 - CONFIG="--disable-system --cc=clang-3.9 --cxx=clang++-3.9"
   before_install:
-- wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo 
apt-key add -
-- sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty 
llvm-toolchain-trusty-3.9 main'
-- sudo apt-get update -qq
-- sudo apt-get install -qq -y clang-3.9
+- travis_retry wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key 
| sudo apt-key add -
+- travis_retry sudo apt-add-repository -y 'deb 
http://llvm.org/apt/trusty llvm-toolchain-trusty-3.9 main'
+- travis_retry sudo apt-get update -qq
+- travis_retry sudo apt-get install -qq -y clang-3.9
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
 - git submodule update --init --recursive
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 10/16] travis: check ./configure outcome, dump config.log on failure

[Philippe Mathieu-Daudé]

also de-duplicate 'before_script'

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 72aad5ca35..88ca78c775 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -79,7 +79,7 @@ before_install:
   - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
   - travis_retry git submodule update --init --recursive
 before_script:
-  - ./configure ${CONFIG}
+  - ./configure ${CONFIG} || (cat config.log; exit 1)
 script:
   - make ${MAKEFLAGS} && ${TEST_CMD}
 matrix:
@@ -155,8 +155,6 @@ matrix:
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
 - travis_retry git submodule update --init --recursive
-  before_script:
-- ./configure ${CONFIG} || cat config.log
 # Trusty Linux User build with latest stable clang
 - sudo: required
   addons:
@@ -175,8 +173,6 @@ matrix:
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
 - travis_retry git submodule update --init --recursive
-  before_script:
-- ./configure ${CONFIG} || cat config.log
 # Using newer GCC with sanitizers
 - addons:
 apt:
@@ -232,4 +228,4 @@ matrix:
 - CONFIG="--cc=gcc-5 --cxx=g++-5 --disable-pie --disable-linux-user"
 - TEST_CMD=""
   before_script:
-- ./configure ${CONFIG} --extra-cflags="-g3 -O0 -fsanitize=thread 
-fuse-ld=gold" || cat config.log
+- ./configure ${CONFIG} --extra-cflags="-g3 -O0 -fsanitize=thread 
-fuse-ld=gold" || (cat config.log; exit 1)
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 11/16] travis/osx: use readable YAML

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 88ca78c775..4bf69b0116 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -74,8 +74,10 @@ git:
   # we want to do this ourselves
   submodules: false
 before_install:
-  - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update ; fi
-  - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew install libffi gettext glib 
pixman ; fi
+  - if [ "$TRAVIS_OS_NAME" == "osx" ]; then
+brew update ;
+brew install libffi gettext glib pixman ;
+fi
   - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
   - travis_retry git submodule update --init --recursive
 before_script:
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 12/16] travis/osx: silent texinfo warnings

[Philippe Mathieu-Daudé]

  $ make info
GEN qemu-doc.html
  qemu-doc.texi:8: warning: unrecognized encoding name `UTF-8'.
GEN qemu-doc.txt
  qemu-doc.texi:8: warning: unrecognized encoding name `UTF-8'.

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 4bf69b0116..24f62fb7cf 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -74,9 +74,11 @@ git:
   # we want to do this ourselves
   submodules: false
 before_install:
+  # silent texinfo warnings 
(https://github.com/xiaohanyu/oh-my-emacs/blob/c664894e2f1c1cb0f95a9f2da88d41b00f190856/core/ome-basic.org#homebrew)
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then
 brew update ;
-brew install libffi gettext glib pixman ;
+brew install libffi gettext glib pixman texinfo ;
+brew link texinfo --force ;
 fi
   - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
   - travis_retry git submodule update --init --recursive
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 04/16] travis: increase S3 cache timeout

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.travis.yml b/.travis.yml
index 311f25eb28..d622d8c433 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -6,6 +6,7 @@ compiler:
   - gcc
 cache:
   ccache: true
+  timeout: 1200 # https://docs.travis-ci.com/user/caching#setting-the-timeout
 addons:
   apt:
 packages:
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 07/16] travis: split the gprof/gcov job

[Philippe Mathieu-Daudé]

gcov generates lot of output and often trigger job limit timeout

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
---
 .travis.yml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 196412f5be..e56c928bef 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -87,7 +87,9 @@ matrix:
 - env: CONFIG=""
   compiler: clang
 # gprof/gcov are GCC features
-- env: CONFIG="--enable-gprof --enable-gcov --disable-pie"
+- env: CONFIG="--enable-gprof --disable-pie"
+  compiler: gcc
+- env: CONFIG="--enable-gcov --disable-pie"
   compiler: gcc
 # We manually include builds which we disable "make check" for
 - env: CONFIG="--enable-debug --enable-tcg-interpreter"
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 08/16] travis: reorder matrix to speedup failure

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 36 +++-
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index e56c928bef..57462b0471 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -64,11 +64,11 @@ env:
 - MAKEFLAGS="-j3"
   matrix:
 - CONFIG=""
-- CONFIG="--enable-debug --enable-debug-tcg --enable-trace-backends=log"
 - CONFIG="--disable-linux-aio --disable-cap-ng --disable-attr 
--disable-brlapi --disable-uuid --disable-libusb"
 - CONFIG="--enable-modules"
 - CONFIG="--with-coroutine=ucontext"
 - CONFIG="--with-coroutine=sigaltstack"
+- CONFIG="--enable-debug --enable-debug-tcg --enable-trace-backends=log"
 git:
   # we want to do this ourselves
   submodules: false
@@ -83,30 +83,32 @@ script:
   - make ${MAKEFLAGS} && ${TEST_CMD}
 matrix:
   include:
+# We manually include builds which we disable "make check" for, they also
+# are the faster jobs (no testing).
+- env: CONFIG="--disable-tcg"
+   TEST_CMD=""
+  compiler: gcc
+- env: CONFIG="--enable-trace-backends=ust"
+   TEST_CMD=""
+  compiler: gcc
+- env: CONFIG="--enable-debug --enable-tcg-interpreter"
+   TEST_CMD=""
+  compiler: gcc
+- env: CONFIG="--enable-trace-backends=simple"
+   TEST_CMD=""
+  compiler: gcc
+- env: CONFIG="--enable-trace-backends=ftrace"
+   TEST_CMD=""
+  compiler: gcc
 # Test with CLang for compile portability
 - env: CONFIG=""
   compiler: clang
+
 # gprof/gcov are GCC features
 - env: CONFIG="--enable-gprof --disable-pie"
   compiler: gcc
 - env: CONFIG="--enable-gcov --disable-pie"
   compiler: gcc
-# We manually include builds which we disable "make check" for
-- env: CONFIG="--enable-debug --enable-tcg-interpreter"
-   TEST_CMD=""
-  compiler: gcc
-- env: CONFIG="--enable-trace-backends=simple"
-   TEST_CMD=""
-  compiler: gcc
-- env: CONFIG="--enable-trace-backends=ftrace"
-   TEST_CMD=""
-  compiler: gcc
-- env: CONFIG="--enable-trace-backends=ust"
-   TEST_CMD=""
-  compiler: gcc
-- env: CONFIG="--disable-tcg"
-   TEST_CMD=""
-  compiler: gcc
 - env: CONFIG=""
   os: osx
   compiler: clang
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 03/16] travis: enable multiple caching features

[Philippe Mathieu-Daudé]

(this eases git workflow)

see https://docs.travis-ci.com/user/caching#enabling-multiple-caching-features

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
---
 .travis.yml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index c8fecb584a..311f25eb28 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -4,7 +4,8 @@ python:
   - "2.4"
 compiler:
   - gcc
-cache: ccache
+cache:
+  ccache: true
 addons:
   apt:
 packages:
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 09/16] travis: reduce git clone depth

[Philippe Mathieu-Daudé]

see https://docs.travis-ci.com/user/customizing-the-build#Git-Clone-Depth

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.travis.yml b/.travis.yml
index 57462b0471..72aad5ca35 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -70,6 +70,7 @@ env:
 - CONFIG="--with-coroutine=sigaltstack"
 - CONFIG="--enable-debug --enable-debug-tcg --enable-trace-backends=log"
 git:
+  depth: 3
   # we want to do this ourselves
   submodules: false
 before_install:
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 06/16] travis: retry when git submodules initialization fails

[Philippe Mathieu-Daudé]

example of failure: https://travis-ci.org/philmd/qemu/jobs/245612939

  $ git submodule update --init --recursive
  [...]
  Submodule 'pixman' (git://anongit.freedesktop.org/pixman) registered for path 
'pixman'
  Cloning into 'pixman'...
  fatal: unable to connect to anongit.freedesktop.org:
  anongit.freedesktop.org[0: 131.252.210.161]: errno=Connection timed out
  anongit.freedesktop.org[1: 2610:10:20:722:a800:ff:fe24:61cf]: errno=Network 
is unreachable
  Clone of 'git://anongit.freedesktop.org/pixman' into submodule path 'pixman' 
failed
  The command "git submodule update --init --recursive" failed and exited with 
1 during .
  Your build has been stopped.

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 82c1819c93..196412f5be 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -76,7 +76,7 @@ before_install:
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew update ; fi
   - if [ "$TRAVIS_OS_NAME" == "osx" ]; then brew install libffi gettext glib 
pixman ; fi
   - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
-  - git submodule update --init --recursive
+  - travis_retry git submodule update --init --recursive
 before_script:
   - ./configure ${CONFIG}
 script:
@@ -119,7 +119,7 @@ matrix:
 - sudo apt-get update -qq
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
-- git submodule update --init --recursive
+- travis_retry git submodule update --init --recursive
 # Plain Trusty Linux User Build
 - env: CONFIG="--disable-system"
   sudo: required
@@ -131,7 +131,7 @@ matrix:
 - sudo apt-get update -qq
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
-- git submodule update --init --recursive
+- travis_retry git submodule update --init --recursive
 # Trusty System build with latest stable clang
 - sudo: required
   addons:
@@ -149,7 +149,7 @@ matrix:
 - travis_retry sudo apt-get install -qq -y clang-3.9
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
-- git submodule update --init --recursive
+- travis_retry git submodule update --init --recursive
   before_script:
 - ./configure ${CONFIG} || cat config.log
 # Trusty Linux User build with latest stable clang
@@ -169,7 +169,7 @@ matrix:
 - travis_retry sudo apt-get install -qq -y clang-3.9
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
-- git submodule update --init --recursive
+- travis_retry git submodule update --init --recursive
   before_script:
 - ./configure ${CONFIG} || cat config.log
 # Using newer GCC with sanitizers
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 00/16] travis: speedup to reduce failures

[Philippe Mathieu-Daudé]

These patches try to improve our Travis CI usage (few failures the last days):
- lower false negative rate (mostly network issues),
- improved code coverage,
- speedup failure detection.

dropped since v2 (will follow as a separate series)
- multicore parallelism use
- cache use

Patch 1 is expected to enter /master as of yesterday.
Travis warned few months ago:
  "On Wednesday, June 21st 2017, we are going to update all our Ubuntu Trusty
  14.04 images."

Patches 4-6 try to avoid aborting a job on network failure.

Patches 7-9 intend to speedup jobs.

Patch 10 gives hint when ./configure fails, patch 12 silent build warnings.

RFC patch 13 uses upgraded GCC v5 -> v6.
RFC patch 14 don't fails the build if the slow gprof/gcov job fail.

Patches 15, 16 add more OSX VMs/toolchains (older, newer, bleeding edge).

Regards,

Phil.

Peter Maydell (1):
  travis: install more library dependencies

Philippe Mathieu-Daudé (15):
  travis: update sudo-enabled Trusty images
  travis: enable multiple caching features
  travis: increase S3 cache timeout
  travis: retry if llvm.org timeouts
  travis: retry when git submodules initialization fails
  travis: split the gprof/gcov job
  travis: reorder matrix to speedup failure
  travis: reduce git clone depth
  travis: check ./configure outcome, dump config.log on failure
  travis/osx: use readable YAML
  travis/osx: silent texinfo warnings
  RFC travis: use gcc-6 sanitizers
  RFC travis: allow_failures for gprof/gcov
  RFC travis/osx: build using more Xcode versions
  NOTFORMERGE travis/osx: build using bleeding Xcode

 .travis.yml | 134 +++-
 1 file changed, 97 insertions(+), 37 deletions(-)

-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 01/16] travis: update sudo-enabled Trusty images

[Philippe Mathieu-Daudé]

see https://blog.travis-ci.com/2017-06-19-trusty-updates-2017-Q2
and https://blog.travis-ci.com/2017-06-21-trusty-updates-2017-Q2-launch

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 4 
 1 file changed, 4 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index f583839755..b9cafe3d40 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -98,6 +98,7 @@ matrix:
   sudo: required
   addons:
   dist: trusty
+  group: deprecated-2017Q2
   compiler: gcc
   before_install:
 - sudo apt-get update -qq
@@ -109,6 +110,7 @@ matrix:
   sudo: required
   addons:
   dist: trusty
+  group: deprecated-2017Q2
   compiler: gcc
   before_install:
 - sudo apt-get update -qq
@@ -119,6 +121,7 @@ matrix:
 - sudo: required
   addons:
   dist: trusty
+  group: deprecated-2017Q2
   language: generic
   compiler: none
   env:
@@ -138,6 +141,7 @@ matrix:
 - sudo: required
   addons:
   dist: trusty
+  group: deprecated-2017Q2
   language: generic
   compiler: none
   env:
-- 
2.13.3

[Qemu-devel] [PATCH for-2.11 v3 02/16] travis: install more library dependencies

[Philippe Mathieu-Daudé]

From: Peter Maydell 

Update the travis list of library packages to install so that
our build tests cover more of our code base.

Signed-off-by: Peter Maydell 
Reviewed-by: Alex Bennée 
[PMD: added more dependencies:
 glusterfs-common, libbz2-dev, libncursesw5-dev, libnfs-dev]
Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Alex Bennée 
---
 .travis.yml | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index b9cafe3d40..c8fecb584a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -9,30 +9,43 @@ addons:
   apt:
 packages:
   # Build dependencies
+  - glusterfs-common
   - libaio-dev
   - libattr1-dev
+  - libbluetooth-dev
   - libbrlapi-dev
+  - libbz2-dev
+  - libcap-dev
   - libcap-ng-dev
   - libgnutls-dev
   - libgtk-3-dev
   - libiscsi-dev
   - liblttng-ust-dev
+  - liblzo2-dev
   - libnfs-dev
   - libncurses5-dev
+  - libncursesw5-dev
+  - libnfs-dev
   - libnss3-dev
   - libpixman-1-dev
   - libpng12-dev
   - librados-dev
+  - librdmacm-dev
   - libsdl1.2-dev
   - libseccomp-dev
+  - libsnappy-dev
   - libspice-protocol-dev
   - libspice-server-dev
   - libssh2-1-dev
   - liburcu-dev
   - libusb-1.0-0-dev
+  - libvde-dev
   - libvte-2.90-dev
+  - libxen-dev
+  - nettle-dev
   - sparse
   - uuid-dev
+  - xfslibs-dev
 
 # The channel name "irc.oftc.net#qemu" is encrypted against qemu/qemu
 # to prevent IRC notifications from forks. This was created using:
@@ -168,30 +181,43 @@ matrix:
 - gcc-5
 - g++-5
 # Build dependencies
+- glusterfs-common
 - libaio-dev
 - libattr1-dev
+- libbluetooth-dev
 - libbrlapi-dev
+- libbz2-dev
+- libcap-dev
 - libcap-ng-dev
 - libgnutls-dev
 - libgtk-3-dev
 - libiscsi-dev
 - liblttng-ust-dev
+- liblzo2-dev
 - libnfs-dev
 - libncurses5-dev
+- libncursesw5-dev
+- libnfs-dev
 - libnss3-dev
 - libpixman-1-dev
 - libpng12-dev
 - librados-dev
+- librdmacm-dev
 - libsdl1.2-dev
 - libseccomp-dev
+- libsnappy-dev
 - libspice-protocol-dev
 - libspice-server-dev
 - libssh2-1-dev
 - liburcu-dev
 - libusb-1.0-0-dev
+- libvde-dev
 - libvte-2.90-dev
+- libxen-dev
+- nettle-dev
 - sparse
 - uuid-dev
+- xfslibs-dev
   language: generic
   compiler: none
   env:
-- 
2.13.3

[Qemu-devel] [PATCH] QEMU Backup Tool

[Ishani Chugh]

qemu-backup will be a command-line tool for performing full and
incremental disk backups on running VMs. It is intended as a
reference implementation for management stack and backup developers
to see QEMU's backup features in action. The tool writes details of
guest in a configuration file and the data is retrieved from the file
while creating a backup. The location of config file can be set as an
environment variable QEMU_BACKUP_CONFIG. The usage is as follows:

Add a guest
python qemu-backup.py guest add --guest  --qmp 

Add a drive for backup in a specified guest
python qemu-backup.py drive add --guest  --id  [--target 
]

Create backup of the added drives:
python qemu-backup.py backup --guest 

List all guest configs in configuration file:
python qemu-backup.py guest list

Restore operation
python qemu-backup.py restore --guest 

Remove a guest
python qemu-backup.py guest remove --guest 


Signed-off-by: Ishani Chugh 
---
 contrib/backup/qemu-backup.py | 217 +++---
 1 file changed, 141 insertions(+), 76 deletions(-)

diff --git a/contrib/backup/qemu-backup.py b/contrib/backup/qemu-backup.py
index 9c3dc53..9bbbdb7 100644
--- a/contrib/backup/qemu-backup.py
+++ b/contrib/backup/qemu-backup.py
@@ -1,22 +1,54 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# 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 .
+#
+
 """
 This file is an implementation of backup tool
 """
+from __future__ import print_function
 from argparse import ArgumentParser
 import os
 import errno
 from socket import error as socket_error
-import configparser
+try:
+import configparser
+except ImportError:
+import ConfigParser as configparser
 import sys
-sys.path.append('../../scripts/qmp')
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..',
+ 'scripts', 'qmp'))
 from qmp import QEMUMonitorProtocol
 
 
 class BackupTool(object):
 """BackupTool Class"""
-def __init__(self, config_file='backup.ini'):
-self.config_file = config_file
+def __init__(self,
+ config_file=os.path.expanduser('~')+'/.qemu/backup/config'):
+if "QEMU_BACKUP_CONFIG" in os.environ:
+self.config_file = os.environ["QEMU_BACKUP_CONFIG"]
+else:
+self.config_file = config_file
+try:
+if not os.path.isdir(os.path.expanduser('~')+'/.qemu/backup'):
+os.makedirs(os.path.expanduser('~')+'/.qemu/backup')
+except:
+print("Cannot find the config file", file=sys.stderr)
+exit(1)
 self.config = configparser.ConfigParser()
 self.config.read(self.config_file)
 
@@ -24,66 +56,70 @@ class BackupTool(object):
 """
 Writes configuration to ini file.
 """
-with open(self.config_file, 'w') as config_file:
-self.config.write(config_file)
+config_file = open(self.config_file+".tmp", 'w')
+self.config.write(config_file)
+config_file.flush()
+os.fsync(config_file.fileno())
+config_file.close()
+os.rename(self.config_file+".tmp", self.config_file)
 
-def get_socket_path(self, socket_path, tcp):
+def get_socket_address(self, socket_address):
 """
 Return Socket address in form of string or tuple
 """
-if tcp is False:
-return os.path.abspath(socket_path)
-return (socket_path.split(':')[0], int(socket_path.split(':')[1]))
+if socket_address.startswith('tcp'):
+return (socket_address.split(':')[1],
+int(socket_address.split(':')[2]))
+return socket_address.split(':',2)[1]
 
-def __full_backup(self, guest_name):
+def _full_backup(self, guest_name):
 """
 Performs full backup of guest
 """
 if guest_name not in self.config.sections():
-print ("Cannot find specified guest")
-return
-if self.is_guest_running(guest_name, self.config[guest_name]['qmp'],
- self.config[guest_name]['tcp']) is False:
-return
+print ("Cannot find specified guest", file=sys.stderr)
+exit(1)
+
+self.verify_guest_running(guest_name)
 connection = 

Re: [Qemu-devel] No video for Windows 2000 guest

[Michael S. Tsirkin]

On Wed, Aug 09, 2017 at 09:05:54PM +0300, Michael S. Tsirkin wrote:
> On Wed, Aug 09, 2017 at 01:54:23PM -0400, Programmingkid wrote:
> > 
> > > On Aug 9, 2017, at 1:18 PM, Michael S. Tsirkin  wrote:
> > > 
> > > On Wed, Aug 09, 2017 at 06:37:12PM +0200, Paolo Bonzini wrote:
> > >> On 09/08/2017 16:56, Programmingkid wrote:
> > >>> The default vga card not longer works with a Windows 2000 guest. All I 
> > >>> see is a black screen after a the Windows splash screen.
> > >>> 
> > >>> This is the command-line I used: 
> > >>> 
> > >>> qemu-system-i386 -hda Windows2000HD.qcow2 -boot c -m 512
> > >>> 
> > >>> When using the -vga cirrus option video works. Testing was done with 
> > >>> QEMU v2.10.0 rc2. 
> > >> 
> > >> Did it work in 2.9?
> > >> 
> > >> Paolo
> > > 
> > > Generally bisect is extremely helpful to debug these issues.
> > 
> > I tried but the acpi issue kept Windows 2000 from booting. 
> 
> You can just revert that on top of each bisect.

IOW after each bisect go
git revert 77af8a2b
test it
then reset back and continue with bisect

Re: [Qemu-devel] [PATCH v2 3/4] contrib/libvhost-user: enable virtio config space messages

[Marc-André Lureau]

Hi

- Original Message -
> Enable VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG/VHOST_USER_SET_CONFIG_FD
> messages in libvhost-user library, users can implement their own I/O target
> based on the library. This enable the virtio config space delivered between
> Qemu host device and the I/O target, also event notifier is added in case
> of virtio config space changed.
> 
> Signed-off-by: Changpeng Liu 
> ---
>  contrib/libvhost-user/libvhost-user.c | 51
>  +++
>  contrib/libvhost-user/libvhost-user.h | 14 ++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/contrib/libvhost-user/libvhost-user.c
> b/contrib/libvhost-user/libvhost-user.c
> index 9efb9da..002cf15 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -63,6 +63,9 @@ vu_request_to_string(int req)
>  REQ(VHOST_USER_SET_VRING_ENABLE),
>  REQ(VHOST_USER_SEND_RARP),
>  REQ(VHOST_USER_INPUT_GET_CONFIG),
> +REQ(VHOST_USER_GET_CONFIG),
> +REQ(VHOST_USER_SET_CONFIG),
> +REQ(VHOST_USER_SET_CONFIG_FD),
>  REQ(VHOST_USER_MAX),
>  };
>  #undef REQ
> @@ -744,6 +747,43 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
>  }
>  
>  static bool
> +vu_get_config(VuDev *dev, VhostUserMsg *vmsg)
> +{
> +if (dev->iface->get_config) {
> +dev->iface->get_config(dev, vmsg->payload.config, vmsg->size);

better check the return value on error to avoid sending garbage back to master.

> +}
> +
> +return true;
> +}
> +
> +static bool
> +vu_set_config(VuDev *dev, VhostUserMsg *vmsg)
> +{
> +if (dev->iface->set_config) {
> +dev->iface->set_config(dev, vmsg->payload.config, vmsg->size);

you could perhaps make that function return void instead (since error isn't 
reported to master)


> +}
> +
> +return false;
> +}
> +
> +static bool
> +vu_set_config_fd(VuDev *dev, VhostUserMsg *vmsg)
> +{
> +   if (vmsg->fd_num != 1) {
> +vu_panic(dev, "Invalid config_fd message");
> +return false;
> +}
> +
> +if (dev->config_fd != -1) {
> +close(dev->config_fd);
> +}
> +dev->config_fd = vmsg->fds[0];
> +DPRINT("Got config_fd: %d\n", vmsg->fds[0]);
> +
> +return false;
> +}
> +
> +static bool
>  vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
>  {
>  int do_reply = 0;
> @@ -806,6 +846,12 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
>  return vu_get_queue_num_exec(dev, vmsg);
>  case VHOST_USER_SET_VRING_ENABLE:
>  return vu_set_vring_enable_exec(dev, vmsg);
> +case VHOST_USER_GET_CONFIG:
> +return vu_get_config(dev, vmsg);
> +case VHOST_USER_SET_CONFIG:
> +return vu_set_config(dev, vmsg);
> +case VHOST_USER_SET_CONFIG_FD:
> +return vu_set_config_fd(dev, vmsg);
>  default:
>  vmsg_close_fds(vmsg);
>  vu_panic(dev, "Unhandled request: %d", vmsg->request);
> @@ -878,6 +924,10 @@ vu_deinit(VuDev *dev)
>  
>  vu_close_log(dev);
>  
> +if (dev->config_fd != -1) {
> +close(dev->config_fd);
> +}
> +
>  if (dev->sock != -1) {
>  close(dev->sock);
>  }
> @@ -907,6 +957,7 @@ vu_init(VuDev *dev,
>  dev->remove_watch = remove_watch;
>  dev->iface = iface;
>  dev->log_call_fd = -1;
> +dev->config_fd = -1;
>  for (i = 0; i < VHOST_MAX_NR_VIRTQUEUE; i++) {
>  dev->vq[i] = (VuVirtq) {
>  .call_fd = -1, .kick_fd = -1, .err_fd = -1,
> diff --git a/contrib/libvhost-user/libvhost-user.h
> b/contrib/libvhost-user/libvhost-user.h
> index 53ef222..899dee1 100644
> --- a/contrib/libvhost-user/libvhost-user.h
> +++ b/contrib/libvhost-user/libvhost-user.h
> @@ -30,6 +30,8 @@
>  
>  #define VHOST_MEMORY_MAX_NREGIONS 8
>  
> +#define VHOST_USER_MAX_CONFIG_SIZE 256
> +
>  enum VhostUserProtocolFeature {
>  VHOST_USER_PROTOCOL_F_MQ = 0,
>  VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
> @@ -62,6 +64,9 @@ typedef enum VhostUserRequest {
>  VHOST_USER_SET_VRING_ENABLE = 18,
>  VHOST_USER_SEND_RARP = 19,
>  VHOST_USER_INPUT_GET_CONFIG = 20,
> +VHOST_USER_GET_CONFIG = 24,
> +VHOST_USER_SET_CONFIG = 25,
> +VHOST_USER_SET_CONFIG_FD = 26,
>  VHOST_USER_MAX
>  } VhostUserRequest;
>  
> @@ -105,6 +110,7 @@ typedef struct VhostUserMsg {
>  struct vhost_vring_addr addr;
>  VhostUserMemory memory;
>  VhostUserLog log;
> +uint8_t config[VHOST_USER_MAX_CONFIG_SIZE];
>  } payload;
>  
>  int fds[VHOST_MEMORY_MAX_NREGIONS];
> @@ -132,6 +138,9 @@ typedef void (*vu_set_features_cb) (VuDev *dev, uint64_t
> features);
>  typedef int (*vu_process_msg_cb) (VuDev *dev, VhostUserMsg *vmsg,
>int *do_reply);
>  typedef void (*vu_queue_set_started_cb) (VuDev *dev, int qidx, bool
>  started);
> +typedef int (*vu_get_config_cb) (VuDev *dev, uint8_t *config, size_t len);
> +typedef int (*vu_set_config_cb) 

Re: [Qemu-devel] [PATCH v2 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk sample application

[Marc-André Lureau]

Hi

- Original Message -
> This commit introcudes a vhost-user-blk backend device, it uses UNIX
> domain socket to communicate with Qemu. The vhost-user-blk sample
> application should be used with Qemu vhost-user-blk-pci device.
> 
> To use it, complie with:
> make vhost-user-blk
> 
> and start like this:
> vhost-user-blk -b /dev/sdb -s /path/vhost.socket

I guess it could be a regular file instead (fallocate/trunc to desired size).

> 
> Signed-off-by: Changpeng Liu 
> ---
>  .gitignore  |   1 +
>  Makefile|   3 +
>  Makefile.objs   |   2 +
>  contrib/vhost-user-blk/Makefile.objs|   1 +
>  contrib/vhost-user-blk/vhost-user-blk.c | 735
>  
>  5 files changed, 742 insertions(+)
>  create mode 100644 contrib/vhost-user-blk/Makefile.objs
>  create mode 100644 contrib/vhost-user-blk/vhost-user-blk.c
> 
> diff --git a/.gitignore b/.gitignore
> index cf65316..dbe5c13 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -51,6 +51,7 @@
>  /module_block.h
>  /vscclient
>  /vhost-user-scsi
> +/vhost-user-blk
>  /fsdev/virtfs-proxy-helper
>  *.[1-9]
>  *.a
> diff --git a/Makefile b/Makefile
> index 97a58a0..e68e339 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -270,6 +270,7 @@ dummy := $(call unnest-vars,, \
>  ivshmem-server-obj-y \
>  libvhost-user-obj-y \
>  vhost-user-scsi-obj-y \
> +vhost-user-blk-obj-y \
>  qga-vss-dll-obj-y \
>  block-obj-y \
>  block-obj-m \
> @@ -478,6 +479,8 @@ ivshmem-server$(EXESUF): $(ivshmem-server-obj-y)
> $(COMMON_LDADDS)
>  endif
>  vhost-user-scsi$(EXESUF): $(vhost-user-scsi-obj-y)
>   $(call LINK, $^)
> +vhost-user-blk$(EXESUF): $(vhost-user-blk-obj-y)
> + $(call LINK, $^)
>  
>  module_block.h: $(SRC_PATH)/scripts/modules/module_block.py config-host.mak
>   $(call quiet-command,$(PYTHON) $< $@ \
> diff --git a/Makefile.objs b/Makefile.objs
> index 24a4ea0..6b81548 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -114,6 +114,8 @@ vhost-user-scsi.o-cflags := $(LIBISCSI_CFLAGS)
>  vhost-user-scsi.o-libs := $(LIBISCSI_LIBS)
>  vhost-user-scsi-obj-y = contrib/vhost-user-scsi/
>  vhost-user-scsi-obj-y += contrib/libvhost-user/libvhost-user.o
> +vhost-user-blk-obj-y = contrib/vhost-user-blk/
> +vhost-user-blk-obj-y += contrib/libvhost-user/libvhost-user.o
>  
>  ##
>  trace-events-subdirs =
> diff --git a/contrib/vhost-user-blk/Makefile.objs
> b/contrib/vhost-user-blk/Makefile.objs
> new file mode 100644
> index 000..72e2cdc
> --- /dev/null
> +++ b/contrib/vhost-user-blk/Makefile.objs
> @@ -0,0 +1 @@
> +vhost-user-blk-obj-y = vhost-user-blk.o
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c
> b/contrib/vhost-user-blk/vhost-user-blk.c

My bad I didn't review vhost-user-scsi.c and you reproduce a lot of code here.

Imho, there is no need for memory allocation failure check; it's a test app & 
glib will terminate if allocation fails anyway.

I should also say that libvhost-user is supposed to be glib-free, and it's not 
fully (it is 99%). That also create some confusion I believe. And some docs is 
lacking.

> new file mode 100644
> index 000..9b90164
> --- /dev/null
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -0,0 +1,735 @@
> +/*
> + * vhost-user-blk sample application
> + *
> + * Copyright IBM, Corp. 2007
> + * Copyright (c) 2016 Nutanix Inc. All rights reserved.
> + * Copyright (c) 2017 Intel Corporation. All rights reserved.
> + *
> + * Author:
> + *  Anthony Liguori 
> + *  Felipe Franciosi 
> + *  Changpeng Liu 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 only.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/virtio/virtio-blk.h"
> +#include "contrib/libvhost-user/libvhost-user.h"
> +
> +#include 
> +
> +/* Small compat shim from glib 2.32 */
> +#ifndef G_SOURCE_CONTINUE
> +#define G_SOURCE_CONTINUE TRUE
> +#endif
> +#ifndef G_SOURCE_REMOVE
> +#define G_SOURCE_REMOVE FALSE
> +#endif
> +

Should probably be in glib-compat.h

> +/* And this is the final byte of request*/
> +#define VIRTIO_BLK_S_OK 0
> +#define VIRTIO_BLK_S_IOERR 1
> +#define VIRTIO_BLK_S_UNSUPP 2
> +
> +typedef struct vhost_blk_dev {
> +VuDev vu_dev;
> +int server_sock;
> +int blk_fd;
> +struct virtio_blk_config blkcfg;
> +char *blk_name;
> +GMainLoop *loop;
> +GTree *fdmap;   /* fd -> gsource context id */

why a tree? I would rather have hashmap, or even a fixed size array, since the 
app isn't supposed to have so many FD open...

> +} vhost_blk_dev_t;
> +
> +typedef struct vhost_blk_request {
> +VuVirtqElement *elem;
> +int64_t sector_num;
> +size_t size;

Re: [Qemu-devel] No video for Windows 2000 guest

[Programmingkid]

> On Aug 9, 2017, at 12:37 PM, Paolo Bonzini  wrote:
> 
> On 09/08/2017 16:56, Programmingkid wrote:
>> The default vga card not longer works with a Windows 2000 guest. All I see 
>> is a black screen after a the Windows splash screen.
>> 
>> This is the command-line I used: 
>> 
>> qemu-system-i386 -hda Windows2000HD.qcow2 -boot c -m 512
>> 
>> When using the -vga cirrus option video works. Testing was done with QEMU 
>> v2.10.0 rc2. 
> 
> Did it work in 2.9?
> 
> Paolo

I haven't test version QEMU 2.9.0 but I did test version 2.8.0 and it has the 
same problem. Starting up Windows 2000 in VGA mode allowed me to access the 
operating system. Found out QEMU's default video controller is not recognized. 
In the Device Manager I can see a yellow question mark for Video Controller 
(VGA Compatible). I remember Windows 2000 being able to use the default video 
card in QEMU in the past. I may be able to bisect this issue after all.

Re: [Qemu-devel] No video for Windows 2000 guest

[Michael S. Tsirkin]

On Wed, Aug 09, 2017 at 01:54:23PM -0400, Programmingkid wrote:
> 
> > On Aug 9, 2017, at 1:18 PM, Michael S. Tsirkin  wrote:
> > 
> > On Wed, Aug 09, 2017 at 06:37:12PM +0200, Paolo Bonzini wrote:
> >> On 09/08/2017 16:56, Programmingkid wrote:
> >>> The default vga card not longer works with a Windows 2000 guest. All I 
> >>> see is a black screen after a the Windows splash screen.
> >>> 
> >>> This is the command-line I used: 
> >>> 
> >>> qemu-system-i386 -hda Windows2000HD.qcow2 -boot c -m 512
> >>> 
> >>> When using the -vga cirrus option video works. Testing was done with QEMU 
> >>> v2.10.0 rc2. 
> >> 
> >> Did it work in 2.9?
> >> 
> >> Paolo
> > 
> > Generally bisect is extremely helpful to debug these issues.
> 
> I tried but the acpi issue kept Windows 2000 from booting. 

You can just revert that on top of each bisect.

Re: [Qemu-devel] No video for Windows 2000 guest

[Programmingkid]

> On Aug 9, 2017, at 1:18 PM, Michael S. Tsirkin  wrote:
> 
> On Wed, Aug 09, 2017 at 06:37:12PM +0200, Paolo Bonzini wrote:
>> On 09/08/2017 16:56, Programmingkid wrote:
>>> The default vga card not longer works with a Windows 2000 guest. All I see 
>>> is a black screen after a the Windows splash screen.
>>> 
>>> This is the command-line I used: 
>>> 
>>> qemu-system-i386 -hda Windows2000HD.qcow2 -boot c -m 512
>>> 
>>> When using the -vga cirrus option video works. Testing was done with QEMU 
>>> v2.10.0 rc2. 
>> 
>> Did it work in 2.9?
>> 
>> Paolo
> 
> Generally bisect is extremely helpful to debug these issues.

I tried but the acpi issue kept Windows 2000 from booting. 

Re: [Qemu-devel] [PATCH v4 5/5] docs: update documentation considering PCIE-PCI bridge

[Laszlo Ersek]

On 08/09/17 18:52, Aleksandr Bezzubikov wrote:
> 2017-08-09 13:18 GMT+03:00 Laszlo Ersek :
>> On 08/08/17 21:21, Aleksandr Bezzubikov wrote:
>>> 2017-08-08 18:11 GMT+03:00 Laszlo Ersek :
 one comment below

 On 08/05/17 22:27, Aleksandr Bezzubikov wrote:

> +Capability layout (defined in include/hw/pci/pci_bridge.h):
> +
> +uint8_t id; Standard PCI capability header field
> +uint8_t next;   Standard PCI capability header field
> +uint8_t len;Standard PCI vendor-specific capability header field
> +
> +uint8_t type;   Red Hat vendor-specific capability type
> +List of currently existing types:
> +QEMU_RESERVE = 1
> +
> +
> +uint32_t bus_res;   Minimum number of buses to reserve
> +
> +uint64_t io;IO space to reserve
> +uint64_t memNon-prefetchable memory to reserve
> +uint64_t mem_pref;  Prefetchable memory to reserve

 (I apologize if I missed any concrete points from the past messages
 regarding this structure.)

 How is the firmware supposed to know whether the prefetchable MMIO
 reservation should be made in 32-bit or 64-bit address space? If we
 reserve prefetchable MMIO outside of the 32-bit address space, then
 hot-plugging a device without 64-bit MMIO support could fail.

 My earlier request, to distinguish "prefetchable_32" from
 "prefetchable_64" (mutually exclusively), was so that firmware would
 know whether to restrict the MMIO reservation to 32-bit address
 space.
>>>
>>> IIUC now (in SeaBIOS at least) we just assign this PREF registers
>>> unconditionally,
>>> so the decision about the mode can be made basing on !=0
>>> UPPER_PREF_LIMIT register.
>>> My idea was the same - we can just check if the value doesn't fit into
>>> 16-bit (PREF_LIMIT reg size, 32-bit MMIO). Do we really need separate
>>> fields for that?
>>
>> The PciBusDxe driver in edk2 tracks 32-bit and 64-bit MMIO resources
>> separately from each other, and other (independent) logic exists in it
>> that, on some conditions, allocates 64-bit MMIO BARs from 32-bit address
>> space. This is just to say that the distinction is intentional in
>> PciBusDxe.
>>
>> Furthermore, the Platform Init spec v1.6 says the following (this is
>> what OVMF will have to comply with, in the "platform hook" called by
>> PciBusDxe):
>>
>>> 12.6 PCI Hot Plug PCI Initialization Protocol
>>> EFI_PCI_HOT_PLUG_INIT_PROTOCOL.GetResourcePadding()
>>> ...
>>> Padding  The amount of resource padding that is required by the PCI
>>>  bus under the control of the specified HPC. Because the
>>>  caller does not know the size of this buffer, this buffer is
>>>  allocated by the callee and freed by the caller.
>>> ...
>>> The padding is returned in the form of ACPI (2.0 & 3.0) resource
>>> descriptors. The exact definition of each of the fields is the same as
>>> in the
>>> EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL.SubmitResources()
>>> function. See the section 10.8 for the definition of this function.
>>
>> Following that pointer:
>>
>>> 10.8 PCI HostBridge Code Definitions
>>> 10.8.2 PCI Host Bridge Resource Allocation Protocol
>>>
>>> Table 8. ACPI 2.0 & 3.0 QWORD Address Space Descriptor Usage
>>>
>>> ByteByteData  Description
>>> Offset  Length
>>> ...
>>> 0x030x01  Resource type:
>>> 0: Memory range
>>> 1: I/O range
>>> 2: Bus number range
>>> ...
>>> 0x050x01  Type-specific flags. Ignored except as defined
>>>   in Table 3-3 and Table 3-4 below.
>>>
>>> 0x060x08  Address Space Granularity. Used to differentiate
>>>   between a 32-bit memory request and a 64-bit
>>>   memory request. For a 32-bit memory request,
>>>   this field should be set to 32. For a 64-bit
>>>   memory request, this field should be set to 64.
>>>   Ignored for I/O and bus resource requests.
>>>   Ignored during GetProposedResources().
>>
>> The "Table 3-3" and "Table 3-4" references under "Type-specific flags"
>> are out of date (spec bug); in reality those are:
>> - Table 10. I/O Resource Flag (Resource Type = 1) Usage,
>> - Table 11. Memory Resource Flag (Resource Type = 0) Usage.
>>
>> The latter is relevant here:
>>
>>> Table 11. Memory Resource Flag (Resource Type = 0) Usage
>>>
>>> Bits  Meaning
>>> ...
>>> Bit[2:1]  _MEM. Memory attributes.
>>>   Value and Meaning:
>>> 0 The memory is nonprefetchable.
>>> 1 Invalid.
>>> 2 Invalid.
>>> 3 The memory is prefetchable.
>>>   Note: The interpretation of these bits is somewhat different
>>>   from the ACPI 

Re: [Qemu-devel] No video for Windows 2000 guest

[Michael S. Tsirkin]

On Wed, Aug 09, 2017 at 06:37:12PM +0200, Paolo Bonzini wrote:
> On 09/08/2017 16:56, Programmingkid wrote:
> > The default vga card not longer works with a Windows 2000 guest. All I see 
> > is a black screen after a the Windows splash screen.
> > 
> > This is the command-line I used: 
> > 
> > qemu-system-i386 -hda Windows2000HD.qcow2 -boot c -m 512
> > 
> > When using the -vga cirrus option video works. Testing was done with QEMU 
> > v2.10.0 rc2. 
> 
> Did it work in 2.9?
> 
> Paolo

Generally bisect is extremely helpful to debug these issues.

-- 
MST

Re: [Qemu-devel] Qemu and 32 PCIe devices

[Michael S. Tsirkin]

On Wed, Aug 09, 2017 at 09:26:11AM +0200, Paolo Bonzini wrote:
> On 09/08/2017 03:06, Laszlo Ersek wrote:
> >>   20.14%  qemu-system-x86_64  [.] render_memory_region
> >>   17.14%  qemu-system-x86_64  [.] subpage_register
> >>   10.31%  qemu-system-x86_64  [.] int128_add
> >>7.86%  qemu-system-x86_64  [.] addrrange_end
> >>7.30%  qemu-system-x86_64  [.] int128_ge
> >>4.89%  qemu-system-x86_64  [.] int128_nz
> >>3.94%  qemu-system-x86_64  [.] phys_page_compact
> >>2.73%  qemu-system-x86_64  [.] phys_map_node_alloc
> 
> Yes, this is the O(n^3) thing.  An optimized build should be faster
> because int128 operations will be inlined and become much more efficient.
> 
> > With this patch, I only tested the "93 devices" case, as the slowdown
> > became visible to the naked eye from the trace messages, as the firmware
> > enabled more and more BARs / command registers (and inversely, the
> > speedup was perceivable when the firmware disabled more and more BARs /
> > command registers).
> 
> This is an interesting observation, and it's expected.  Looking at the
> O(n^3) complexity more in detail you have N operations, where the "i"th
> operates on "i" DMA address spaces, all of which have at least "i"
> memory regions (at least 1 BAR per device).
> 
> So the total cost is sum i=1..N i^2 = N(N+1)(2N+1)/6 = O(n^3).
> Expressing it as a sum shows why it gets slower as time progresses.
> 
> The solution is to note that those "i" address spaces are actually all
> the same, so we can get it down to sum i=1..N i = N(N+1)/2 = O(n^2).
> 
> Thanks,
> 
> Paolo

We'll probably run into more issues with the vIOMMU but I guess we
can look into it later.

Resolving addresses lazily somehow might be interesting. And would
the caching work that went in a while ago but got disabled
since we couldn't iron out all the small issues
help go in that direction somehow?

-- 
MST

Re: [Qemu-devel] [PATCH v2 2/4] vhost-user-blk: introduce a new vhost-user-blk host device

[Michael S. Tsirkin]

I only had time for a quick look. More review when
you repost after release.


On Thu, Aug 10, 2017 at 06:12:29PM +0800, Changpeng Liu wrote:
> This commit introduces a new vhost-user device for block, it uses a
> chardev to connect with the backend, same with Qemu virito-blk device,
> Guest OS still uses the virtio-blk frontend driver.
> 
> To use it, start Qemu with command line like this:
> 
> qemu-system-x86_64 \
> -chardev socket,id=char0,path=/path/vhost.socket \
> -device vhost-user-blk-pci,chardev=char0,num_queues=...
> 
> Different with exist Qemu virtio-blk host device, it makes more easy
> for users to implement their own I/O processing logic, such as all
> user space I/O stack against hardware block device. It uses the new
> vhost messages(VHOST_USER_GET_CONFIG) to get block virtio config
> information from backend process.

I took a quick look. I think I would prefer a more direct approach
where qemu is more of a driver. So user specifies properties and
they get sent to backend at init time. Only handle geometry changes
specially.

> 
> Signed-off-by: Changpeng Liu 
> ---
>  configure  |  11 ++
>  hw/block/Makefile.objs |   3 +
>  hw/block/vhost-user-blk.c  | 360 
> +
>  hw/virtio/virtio-pci.c |  55 ++
>  hw/virtio/virtio-pci.h |  18 ++
>  include/hw/virtio/vhost-user-blk.h |  40 +
>  6 files changed, 487 insertions(+)
>  create mode 100644 hw/block/vhost-user-blk.c
>  create mode 100644 include/hw/virtio/vhost-user-blk.h
> 
> diff --git a/configure b/configure
> index dd73cce..1452c66 100755
> --- a/configure
> +++ b/configure
> @@ -305,6 +305,7 @@ tcg="yes"
>  
>  vhost_net="no"
>  vhost_scsi="no"
> +vhost_user_blk="no"
>  vhost_vsock="no"
>  vhost_user=""
>  kvm="no"
> @@ -779,6 +780,7 @@ Linux)
>kvm="yes"
>vhost_net="yes"
>vhost_scsi="yes"
> +  vhost_user_blk="yes"
>vhost_vsock="yes"
>QEMU_INCLUDES="-I\$(SRC_PATH)/linux-headers -I$(pwd)/linux-headers 
> $QEMU_INCLUDES"
>supported_os="yes"
> @@ -1136,6 +1138,10 @@ for opt do
>;;
>--enable-vhost-scsi) vhost_scsi="yes"
>;;
> +  --disable-vhost-user-blk) vhost_user_blk="no"
> +  ;;
> +  --enable-vhost-user-blk) vhost_user_blk="yes"
> +  ;;
>--disable-vhost-vsock) vhost_vsock="no"
>;;
>--enable-vhost-vsock) vhost_vsock="yes"
> @@ -1506,6 +1512,7 @@ disabled with --disable-FEATURE, default is enabled if 
> available:
>cap-ng  libcap-ng support
>attrattr and xattr support
>vhost-net   vhost-net acceleration support
> +  vhost-user-blk  VM virtio-blk acceleration in user space
>spice   spice
>rbd rados block device (rbd)
>libiscsiiscsi support
> @@ -5365,6 +5372,7 @@ echo "posix_madvise $posix_madvise"
>  echo "libcap-ng support $cap_ng"
>  echo "vhost-net support $vhost_net"
>  echo "vhost-scsi support $vhost_scsi"
> +echo "vhost-user-blk support $vhost_user_blk"
>  echo "vhost-vsock support $vhost_vsock"
>  echo "vhost-user support $vhost_user"
>  echo "Trace backends$trace_backends"
> @@ -5776,6 +5784,9 @@ fi
>  if test "$vhost_scsi" = "yes" ; then
>echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak
>  fi
> +if test "$vhost_user_blk" = "yes" ; then
> +  echo "CONFIG_VHOST_USER_BLK=y" >> $config_host_mak
> +fi
>  if test "$vhost_net" = "yes" -a "$vhost_user" = "yes"; then
>echo "CONFIG_VHOST_NET_USED=y" >> $config_host_mak
>  fi
> diff --git a/hw/block/Makefile.objs b/hw/block/Makefile.objs
> index e0ed980..4c19a58 100644
> --- a/hw/block/Makefile.objs
> +++ b/hw/block/Makefile.objs
> @@ -13,3 +13,6 @@ obj-$(CONFIG_SH4) += tc58128.o
>  
>  obj-$(CONFIG_VIRTIO) += virtio-blk.o
>  obj-$(CONFIG_VIRTIO) += dataplane/
> +ifeq ($(CONFIG_VIRTIO),y)
> +obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk.o
> +endif
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> new file mode 100644
> index 000..8aa9fa9
> --- /dev/null
> +++ b/hw/block/vhost-user-blk.c
> @@ -0,0 +1,360 @@
> +/*
> + * vhost-user-blk host device
> + *
> + * Copyright IBM, Corp. 2011
> + * Copyright(C) 2017 Intel Corporation.
> + *
> + * Authors:
> + *  Stefan Hajnoczi 
> + *  Changpeng Liu 
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu/error-report.h"
> +#include "qemu/typedefs.h"
> +#include "qemu/cutils.h"
> +#include "qom/object.h"
> +#include "hw/qdev-core.h"
> +#include "hw/virtio/vhost.h"
> +#include "hw/virtio/vhost-user-blk.h"
> +#include "hw/virtio/virtio.h"
> +#include "hw/virtio/virtio-bus.h"
> +#include "hw/virtio/virtio-access.h"
> +
> +static const int user_feature_bits[] = {
> +VIRTIO_BLK_F_SIZE_MAX,
> +VIRTIO_BLK_F_SEG_MAX,
> +   

Re: [Qemu-devel] [PATCH v2 0/4] *** Introduce a new vhost-user-blk host device to Qemu ***

[Michael S. Tsirkin]

On Thu, Aug 10, 2017 at 06:12:27PM +0800, Changpeng Liu wrote:
> Althrough virtio scsi specification was designed as a replacement for 
> virtio_blk,
> there are still many users using virtio_blk. Qemu 2.9 introduced a new device
> vhost user scsi which can process I/O in user space for virtio_scsi, this 
> commit
> introduces a new vhost user block host device, which can support virtio_blk in
> Guest OS, and I/O processing in another I/O target.
> 
> Due to the limitation for virtio_blk specification, virtio_blk device cannot 
> get
> block information such as capacity, block size etc via the specification, 
> several
> new vhost user messages were added to support deliver virtio config space
> information between Qemu and I/O target, 
> VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG
> messages used for get/set config space from/to I/O target, 
> VHOST_USER_SET_CONFIG_FD
> was added for event notifier in case the change of virtio config space. Also, 
> those
> messages can be used for vhost device live migration as well.

As we are busy wrapping up a QEMU release, please remember to repost after the
release.

> Changpeng Liu (4):
>   vhost-user: add new vhost user messages to support virtio config space
>   vhost-user-blk: introduce a new vhost-user-blk host device
>   contrib/libvhost-user: enable virtio config space messages
>   contrib/vhost-user-blk: introduce a vhost-user-blk sample application
> 
>  .gitignore  |   1 +
>  Makefile|   3 +
>  Makefile.objs   |   2 +
>  configure   |  11 +
>  contrib/libvhost-user/libvhost-user.c   |  51 +++
>  contrib/libvhost-user/libvhost-user.h   |  14 +
>  contrib/vhost-user-blk/Makefile.objs|   1 +
>  contrib/vhost-user-blk/vhost-user-blk.c | 735 
> 
>  docs/interop/vhost-user.txt |  31 ++
>  hw/block/Makefile.objs  |   3 +
>  hw/block/vhost-user-blk.c   | 360 
>  hw/virtio/vhost-user.c  |  86 
>  hw/virtio/vhost.c   |  63 +++
>  hw/virtio/virtio-pci.c  |  55 +++
>  hw/virtio/virtio-pci.h  |  18 +
>  include/hw/virtio/vhost-backend.h   |   8 +
>  include/hw/virtio/vhost-user-blk.h  |  40 ++
>  include/hw/virtio/vhost.h   |  16 +
>  18 files changed, 1498 insertions(+)
>  create mode 100644 contrib/vhost-user-blk/Makefile.objs
>  create mode 100644 contrib/vhost-user-blk/vhost-user-blk.c
>  create mode 100644 hw/block/vhost-user-blk.c
>  create mode 100644 include/hw/virtio/vhost-user-blk.h
> 
> -- 
> 1.9.3

Re: [Qemu-devel] [PATCH v3] 9pfs: fix dependencies

[Greg Kurz]

On Wed,  9 Aug 2017 18:30:19 +0200
Cornelia Huck  wrote:

> Nothing in fsdev/ or hw/9pfs/ depends on pci; it should rather depend
> on CONFIG_VIRTFS and CONFIG_VIRTIO/CONFIG_XEN only.
> 
> Signed-off-by: Cornelia Huck 
> ---
> 

I think we're ok now. Thanks for fixing that!

IIUC, this patch will be needed when your zpci cleanup patches get merged.
I suggest you add it to your series with my:

Acked-by: Greg Kurz 

Cheers,

--
Greg

> v2->v3: switch dependencies to VIRTFS && (VIRTIO || XEN)
> add explict VIRTIO dependency for virtio-9p-device.o
> 
> ---
>  fsdev/Makefile.objs   | 9 +++--
>  hw/9pfs/Makefile.objs | 2 +-
>  hw/Makefile.objs  | 2 +-
>  3 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
> index 659df6e187..fb38017c0b 100644
> --- a/fsdev/Makefile.objs
> +++ b/fsdev/Makefile.objs
> @@ -1,10 +1,7 @@
> -ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
>  # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
> -# only pull in the actual virtio-9p device if we also enabled virtio.
> -common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> -else
> -common-obj-y = qemu-fsdev-dummy.o
> -endif
> +# only pull in the actual 9p backend if we also enabled virtio or xen.
> +common-obj-$(call land,$(CONFIG_VIRTFS),$(call 
> lor,$(CONFIG_VIRTIO),$(CONFIG_XEN))) = qemu-fsdev.o 9p-marshal.o 
> 9p-iov-marshal.o
> +common-obj-$(call lnot,$(call land,$(CONFIG_VIRTFS),$(call 
> lor,$(CONFIG_VIRTIO),$(CONFIG_XEN = qemu-fsdev-dummy.o
>  common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
>  
>  # Toplevel always builds this; targets without virtio will put it in
> diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
> index cab5e942ed..fd90b62900 100644
> --- a/hw/9pfs/Makefile.objs
> +++ b/hw/9pfs/Makefile.objs
> @@ -7,4 +7,4 @@ common-obj-$(CONFIG_OPEN_BY_HANDLE) +=  9p-handle.o
>  common-obj-y += 9p-proxy.o
>  common-obj-$(CONFIG_XEN) += xen-9p-backend.o
>  
> -obj-y += virtio-9p-device.o
> +obj-$(CONFIG_VIRTIO) += virtio-9p-device.o
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index a2c61f6b09..cf4cb2010b 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -1,4 +1,4 @@
> -devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call 
> land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
> +devices-dirs-$(call land,$(CONFIG_VIRTFS),$(call 
> lor,$(CONFIG_VIRTIO),$(CONFIG_XEN))) += 9pfs/
>  devices-dirs-$(CONFIG_SOFTMMU) += acpi/
>  devices-dirs-$(CONFIG_SOFTMMU) += adc/
>  devices-dirs-$(CONFIG_SOFTMMU) += audio/



pgppmuImXiszR.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 5/5] docs: update documentation considering PCIE-PCI bridge

[Aleksandr Bezzubikov]

2017-08-09 13:18 GMT+03:00 Laszlo Ersek :
> On 08/08/17 21:21, Aleksandr Bezzubikov wrote:
>> 2017-08-08 18:11 GMT+03:00 Laszlo Ersek :
>>> one comment below
>>>
>>> On 08/05/17 22:27, Aleksandr Bezzubikov wrote:
>>>
 +Capability layout (defined in include/hw/pci/pci_bridge.h):
 +
 +uint8_t id; Standard PCI capability header field
 +uint8_t next;   Standard PCI capability header field
 +uint8_t len;Standard PCI vendor-specific capability header field
 +
 +uint8_t type;   Red Hat vendor-specific capability type
 +List of currently existing types:
 +QEMU_RESERVE = 1
 +
 +
 +uint32_t bus_res;   Minimum number of buses to reserve
 +
 +uint64_t io;IO space to reserve
 +uint64_t memNon-prefetchable memory to reserve
 +uint64_t mem_pref;  Prefetchable memory to reserve
>>>
>>> (I apologize if I missed any concrete points from the past messages
>>> regarding this structure.)
>>>
>>> How is the firmware supposed to know whether the prefetchable MMIO
>>> reservation should be made in 32-bit or 64-bit address space? If we
>>> reserve prefetchable MMIO outside of the 32-bit address space, then
>>> hot-plugging a device without 64-bit MMIO support could fail.
>>>
>>> My earlier request, to distinguish "prefetchable_32" from
>>> "prefetchable_64" (mutually exclusively), was so that firmware would
>>> know whether to restrict the MMIO reservation to 32-bit address
>>> space.
>>
>> IIUC now (in SeaBIOS at least) we just assign this PREF registers
>> unconditionally,
>> so the decision about the mode can be made basing on !=0
>> UPPER_PREF_LIMIT register.
>> My idea was the same - we can just check if the value doesn't fit into
>> 16-bit (PREF_LIMIT reg size, 32-bit MMIO). Do we really need separate
>> fields for that?
>
> The PciBusDxe driver in edk2 tracks 32-bit and 64-bit MMIO resources
> separately from each other, and other (independent) logic exists in it
> that, on some conditions, allocates 64-bit MMIO BARs from 32-bit address
> space. This is just to say that the distinction is intentional in
> PciBusDxe.
>
> Furthermore, the Platform Init spec v1.6 says the following (this is
> what OVMF will have to comply with, in the "platform hook" called by
> PciBusDxe):
>
>> 12.6 PCI Hot Plug PCI Initialization Protocol
>> EFI_PCI_HOT_PLUG_INIT_PROTOCOL.GetResourcePadding()
>> ...
>> Padding  The amount of resource padding that is required by the PCI
>>  bus under the control of the specified HPC. Because the
>>  caller does not know the size of this buffer, this buffer is
>>  allocated by the callee and freed by the caller.
>> ...
>> The padding is returned in the form of ACPI (2.0 & 3.0) resource
>> descriptors. The exact definition of each of the fields is the same as
>> in the
>> EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL.SubmitResources()
>> function. See the section 10.8 for the definition of this function.
>
> Following that pointer:
>
>> 10.8 PCI HostBridge Code Definitions
>> 10.8.2 PCI Host Bridge Resource Allocation Protocol
>>
>> Table 8. ACPI 2.0 & 3.0 QWORD Address Space Descriptor Usage
>>
>> ByteByteData  Description
>> Offset  Length
>> ...
>> 0x030x01  Resource type:
>> 0: Memory range
>> 1: I/O range
>> 2: Bus number range
>> ...
>> 0x050x01  Type-specific flags. Ignored except as defined
>>   in Table 3-3 and Table 3-4 below.
>>
>> 0x060x08  Address Space Granularity. Used to differentiate
>>   between a 32-bit memory request and a 64-bit
>>   memory request. For a 32-bit memory request,
>>   this field should be set to 32. For a 64-bit
>>   memory request, this field should be set to 64.
>>   Ignored for I/O and bus resource requests.
>>   Ignored during GetProposedResources().
>
> The "Table 3-3" and "Table 3-4" references under "Type-specific flags"
> are out of date (spec bug); in reality those are:
> - Table 10. I/O Resource Flag (Resource Type = 1) Usage,
> - Table 11. Memory Resource Flag (Resource Type = 0) Usage.
>
> The latter is relevant here:
>
>> Table 11. Memory Resource Flag (Resource Type = 0) Usage
>>
>> Bits  Meaning
>> ...
>> Bit[2:1]  _MEM. Memory attributes.
>>   Value and Meaning:
>> 0 The memory is nonprefetchable.
>> 1 Invalid.
>> 2 Invalid.
>> 3 The memory is prefetchable.
>>   Note: The interpretation of these bits is somewhat different
>>   from the ACPI Specification. According to the ACPI
>>   Specification, a value of 0 implies noncacheable memory and
>>   the value of 3 indicates prefetchable and 

Re: [Qemu-devel] [PATCH RFC 0/6] QEMU: kvm: cleanup kvm_slot handling

[Paolo Bonzini]

On 09/08/2017 15:33, David Hildenbrand wrote:
> If I am not missing something important here, we can heavily simplify
> the kvm_slot code. Flatview will make sure that we don't have to deal
> with overlapping slots. E.g. when a memory section is resized, we are
> first notified about the removal and then about the new memory section.
> 
> So basically, we can directly always map one memory section to one
> kvm slot (if the fixed up size is > 0).
> 
> Only very briefly tested. Will do some more testing if we agree that this
> is the right thing to do.

Yes, it all looks very sane.

Paolo

> David Hildenbrand (6):
>   kvm: require JOIN_MEMORY_REGIONS_WORKS
>   kvm: factor out alignment of memory section
>   kvm: use start + size for memory ranges
>   kvm: we never have overlapping slots in kvm_set_phys_mem()
>   kvm: kvm_log_start/stop are only called with known sections
>   kvm: kvm_log_sync() is only called with known memory sections
> 
>  accel/kvm/kvm-all.c | 276 
> +---
>  1 file changed, 89 insertions(+), 187 deletions(-)
> 

Re: [Qemu-devel] [PATCH v5 10/17] migration: Create ram_multifd_page

[Paolo Bonzini]

On 08/08/2017 21:14, Dr. David Alan Gilbert wrote:
>> There is no barrier there that I can see.  I know that it probably work
>> on x86, but in others?  I think that it *** HERE  we need that
>> memory barrier that we don't have.
> Yes, I think that's smp_mb_release() - and you have to do an
> smp_mb_acquire after reading the pages->num before accessing the iov.

Yes, I think that's correct.

Paolo

> (Probably worth checking with Paolo).
> Or just stick with mutex's.
> 
> 

Re: [Qemu-devel] [PATCH v5 12/17] migration: Send the fd number which we are going to use for this page

[Paolo Bonzini]

On 17/07/2017 15:42, Juan Quintela wrote:
> We are still sending the page through the main channel, that would
> change later in the series
> 
> Signed-off-by: Juan Quintela 
> ---
>  migration/ram.c | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 90e1bcb..ac0742f 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -568,7 +568,7 @@ static int multifd_send_page(uint8_t *address)
>  qemu_mutex_unlock(>mutex);
>  qemu_sem_post(>sem);
>  
> -return 0;
> +return i;
>  }
>  
>  struct MultiFDRecvParams {
> @@ -1143,6 +1143,7 @@ static int ram_multifd_page(RAMState *rs, 
> PageSearchStatus *pss,
>  bool last_stage)
>  {
>  int pages;
> +uint16_t fd_num;
>  uint8_t *p;
>  RAMBlock *block = pss->block;
>  ram_addr_t offset = pss->page << TARGET_PAGE_BITS;
> @@ -1154,8 +1155,10 @@ static int ram_multifd_page(RAMState *rs, 
> PageSearchStatus *pss,
>  ram_counters.transferred +=
>  save_page_header(rs, rs->f, block,
>   offset | RAM_SAVE_FLAG_MULTIFD_PAGE);
> +fd_num = multifd_send_page(p);
> +qemu_put_be16(rs->f, fd_num);
> +ram_counters.transferred += 2; /* size of fd_num */
>  qemu_put_buffer(rs->f, p, TARGET_PAGE_SIZE);
> -multifd_send_page(p);
>  ram_counters.transferred += TARGET_PAGE_SIZE;
>  pages = 1;
>  ram_counters.normal++;
> @@ -2905,6 +2908,7 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> version_id)
>  while (!postcopy_running && !ret && !(flags & RAM_SAVE_FLAG_EOS)) {
>  ram_addr_t addr, total_ram_bytes;
>  void *host = NULL;
> +uint16_t fd_num;
>  uint8_t ch;
>  
>  addr = qemu_get_be64(f);
> @@ -3015,6 +3019,11 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> version_id)
>  break;
>  
>  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;
> +}
>  qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
>  break;
>  
> 


I'm still not convinced of doing this instead of just treating all
sockets equivalently (and flushing them all when the main socket is told
that there is a new block).

Paolo

Re: [Qemu-devel] [PATCH v4 21/22] libqtest: Drop now-unused qmp()

[Eric Blake]

On 08/09/2017 11:01 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> All callers have been converted to a form of qmp_cmd() or
>> qmp_args() that takes the command name with less boilerplate.
>> Therefore, we also know that all commands are using
>> interpolation, and can remove an assertion.
>>
>> This also means that we have fixed the testsuite to comply with
>> -Wformat checking on the strings being interpolated for qmp()
>> (similar to what we previously did for strings used in hmp(), and
>> matching the checking present on qobject_from_jsonf()).
>>
>> Signed-off-by: Eric Blake 
>> ---

>> - * A round trip through QObject is only needed if % interpolation
>> - * is used.  We interpolate through QObject rather than sprintf in
>> - * order to escape strings properly.
>> + * A round trip through QObject (and not sprintf) is needed
>> + * because % interpolation is used, and we must escape strings
>> + * properly.
>>   */
>> -if (!strchr(fmt, '%')) {
>> -qmp_fd_send(s->qmp_fd, fmt);
>> -return;
>> -}
>> +assert(strchr(fmt, '%'));
> 
> What exactly is wrong with a @fmt that doesn't contain '%'?

Nothing, so much as proving to myself that I indeed converted all the
qmp() calls.  As before, the assertion is not vital to the series, and
can be omitted on the respin.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 19/22] libqtest: Add qmp_args_dict() helper

[Eric Blake]

On 08/09/2017 10:59 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> Leaving interpolation into JSON to qobject_from_jsonf() is more
>> robust than building QMP input manually; however, we have a few
>> places where code is already creating a QDict to interpolate
>> individual arguments, which cannot be reproduced with the
>> qobject_from_jsonf() parser.  Expose a public wrapper
>> qmp_args_dict() for the internal helper qmp_args_dict_async()
>> that we needed earlier for qmp_args(), and fix a test to use
>> the new helper.
>>
>> Signed-off-by: Eric Blake 
>> ---

>> +++ b/tests/device-introspect-test.c
>> @@ -36,8 +36,7 @@ static QList *qom_list_types(const char *implements, bool 
>> abstract)
>>  if (implements) {
>>  qdict_put_str(args, "implements", implements);
>>  }
>> -resp = qmp("{'execute': 'qom-list-types',"
>> -   " 'arguments': %p }", args);
>> +resp = qmp_args_dict("qom-list-types", args);
>>  g_assert(qdict_haskey(resp, "return"));
>>  ret = qdict_get_qlist(resp, "return");
>>  QINCREF(ret);
> 
> If we had five of these, the helper would be worth its keep.

This patch only  has one client, but 20/22 adds another.  Is having 2
clients sufficient to keep it (not quite the 5 that makes it obvious,
but still a good reuse of code)?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [for-2.10 PATCH v4] 9pfs: local: fix fchmodat_nofollow() limitations

[Greg Kurz]

This function has to ensure it doesn't follow a symlink that could be used
to escape the virtfs directory. This could be easily achieved if fchmodat()
on linux honored the AT_SYMLINK_NOFOLLOW flag as described in POSIX, but
it doesn't. There was a tentative to implement a new fchmodat2() syscall
with the correct semantics:

https://patchwork.kernel.org/patch/9596301/

but it didn't gain much momentum. Also it was suggested to look at an O_PATH
based solution in the first place.

The current implementation covers most use-cases, but it notably fails if:
- the target path has access rights equal to  (openat() returns EPERM),
  => once you've done chmod() on a file, you can never chmod() again
- the target path is UNIX domain socket (openat() returns ENXIO)
  => bind() of UNIX domain sockets fails if the file is on 9pfs

The solution is to use O_PATH: openat() now succeeds in both cases, and we
can ensure the path isn't a symlink with fstat(). The associated entry in
"/proc/self/fd" can hence be safely passed to the regular chmod() syscall.

The previous behavior is kept for older systems that don't have O_PATH.

Signed-off-by: Greg Kurz 
Reviewed-by: Eric Blake 
---
v4: - fixed #if condition
- moved out: label above #endif
- fixed typo in changelog
- added Eric's r-b

v3: - O_PATH in a separate block of code
- added a reference to the fchmodat2() tentative in the changelog

v2: - renamed OPENAT_DIR_O_PATH to O_PATH_9P_UTIL and use it as a replacement
  for O_PATH to avoid build breaks on O_PATH-less systems
- keep current behavior for O_PATH-less systems
- added comments
- TODO in 2.11: add _nofollow suffix to openat_dir() and openat_file()
---
 hw/9pfs/9p-local.c |   43 ---
 hw/9pfs/9p-util.h  |   24 +++-
 2 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 6e478f4765ef..d9ef57d343c9 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -333,17 +333,27 @@ update_map_file:
 
 static int fchmodat_nofollow(int dirfd, const char *name, mode_t mode)
 {
+struct stat stbuf;
 int fd, ret;
 
 /* FIXME: this should be handled with fchmodat(AT_SYMLINK_NOFOLLOW).
- * Unfortunately, the linux kernel doesn't implement it yet. As an
- * alternative, let's open the file and use fchmod() instead. This
- * may fail depending on the permissions of the file, but it is the
- * best we can do to avoid TOCTTOU. We first try to open read-only
- * in case name points to a directory. If that fails, we try write-only
- * in case name doesn't point to a directory.
+ * Unfortunately, the linux kernel doesn't implement it yet.
  */
-fd = openat_file(dirfd, name, O_RDONLY, 0);
+
+ /* First, we clear non-racing symlinks out of the way. */
+if (fstatat(dirfd, name, , AT_SYMLINK_NOFOLLOW)) {
+return -1;
+}
+if (S_ISLNK(stbuf.st_mode)) {
+errno = ELOOP;
+return -1;
+}
+
+/* Access modes are ignored when O_PATH is supported. We try O_RDONLY and
+ * O_WRONLY for old-systems that don't support O_PATH.
+ */
+fd = openat_file(dirfd, name, O_RDONLY | O_PATH_9P_UTIL, 0);
+#if O_PATH_9P_UTIL == 0
 if (fd == -1) {
 /* In case the file is writable-only and isn't a directory. */
 if (errno == EACCES) {
@@ -357,6 +367,25 @@ static int fchmodat_nofollow(int dirfd, const char *name, 
mode_t mode)
 return -1;
 }
 ret = fchmod(fd, mode);
+#else
+/* Now we handle racing symlinks. */
+ret = fstat(fd, );
+if (ret) {
+goto out;
+}
+if (S_ISLNK(stbuf.st_mode)) {
+errno = ELOOP;
+ret = -1;
+goto out;
+}
+
+{
+char *proc_path = g_strdup_printf("/proc/self/fd/%d", fd);
+ret = chmod(proc_path, mode);
+g_free(proc_path);
+}
+out:
+#endif
 close_preserve_errno(fd);
 return ret;
 }
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 91299a24b8af..dc0d2e29aa3b 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -13,6 +13,12 @@
 #ifndef QEMU_9P_UTIL_H
 #define QEMU_9P_UTIL_H
 
+#ifdef O_PATH
+#define O_PATH_9P_UTIL O_PATH
+#else
+#define O_PATH_9P_UTIL 0
+#endif
+
 static inline void close_preserve_errno(int fd)
 {
 int serrno = errno;
@@ -22,13 +28,8 @@ static inline void close_preserve_errno(int fd)
 
 static inline int openat_dir(int dirfd, const char *name)
 {
-#ifdef O_PATH
-#define OPENAT_DIR_O_PATH O_PATH
-#else
-#define OPENAT_DIR_O_PATH 0
-#endif
 return openat(dirfd, name,
-  O_DIRECTORY | O_RDONLY | O_NOFOLLOW | OPENAT_DIR_O_PATH);
+  O_DIRECTORY | O_RDONLY | O_NOFOLLOW | O_PATH_9P_UTIL);
 }
 
 static inline int openat_file(int dirfd, const char *name, int flags,
@@ -43,9 +44,14 @@ static inline int openat_file(int dirfd, const char *name, 
int flags,
 }
 
 serrno = 

Re: [Qemu-devel] [PATCH v4 16/22] libqtest: Add qmp_cmd() helper

[Eric Blake]

On 08/09/2017 10:40 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> Now that we've asserted that all of our interpolated QMP commands
>> include 'execute', we can reduce some of the caller boilerplate
>> by providing a helpr function to wrap commands with no arguments
> 
> helper
> 
> I don't get the dependency on asserting "contains 'execute'".

As mentioned elsewhere, the assertions helped me make sure I converted
all qmp() callers, but I'm fine not having it (and therefore adjusting
this commit message) in the next spin.

>> +void qmp_cmd_async(const char *cmd)
>> +{
>> +qtest_qmp_send(global_qtest, "{'execute':%s}", cmd);
>> +}
>> +
> 
> Hmm.  A possibly saner naming scheme:
> 
> FOO_send(): send a command
> FOO_receive(): receive a reply
> FOO: both

Yes, I like it.  That means s/FOO_async/FOO_send/.  And to some extent,
I already did that - as the name qmp_cmd() was temporary until I could
get rid of all older qmp() semantics, and then end with s/qmp_cmd/qmp/
in 22/22.  And since I'm already touching pretty much every client, it's
no worse churn to do a sane rename in the process.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] No video for Windows 2000 guest

[Paolo Bonzini]

On 09/08/2017 16:56, Programmingkid wrote:
> The default vga card not longer works with a Windows 2000 guest. All I see is 
> a black screen after a the Windows splash screen.
> 
> This is the command-line I used: 
> 
> qemu-system-i386 -hda Windows2000HD.qcow2 -boot c -m 512
> 
> When using the -vga cirrus option video works. Testing was done with QEMU 
> v2.10.0 rc2. 

Did it work in 2.9?

Paolo

Re: [Qemu-devel] >256 Virtio-net-pci hotplug Devices

[Kinsella, Ray]

Marcel,

The findings are pretty consistent with what I identified.
Although it looks like SeaBIOS fairs better than UEFI.

Thanks for the headsup, will reply on the thread itself.

Ray K

-Original Message-
From: Marcel Apfelbaum [mailto:mar...@redhat.com] 
Sent: Wednesday, August 9, 2017 3:53 AM
To: Kinsella, Ray ; Kevin O'Connor 
Cc: Tan, Jianfeng ; seab...@seabios.org; Michael 
Tsirkin ; qemu-devel@nongnu.org; Gerd Hoffmann 

Subject: Re: [Qemu-devel] >256 Virtio-net-pci hotplug Devices

On 07/08/2017 22:00, Kinsella, Ray wrote:
> Hi Marcel,
> 

Hi Ray,

Please have a look on this thread, I think Laszlo and Paolo
found the root cause.
 https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01368.html
It seems hot-plugging the devices would not help.

Thanks,
MArcel

> Yup - I am using Seabios by default.
> I took all the measures from the Kernel time reported in syslog.
> As Seabios wasn't exhibiting any obvious scaling problem.
> 
> Ray K
> 
> -Original Message-
> From: Marcel Apfelbaum [mailto:mar...@redhat.com]
> Sent: Wednesday, August 2, 2017 5:43 AM
> To: Kinsella, Ray ; Kevin O'Connor 
> 
> Cc: Tan, Jianfeng ; seab...@seabios.org; Michael 
> Tsirkin ; qemu-devel@nongnu.org; Gerd Hoffmann 
> 
> Subject: Re: [Qemu-devel] >256 Virtio-net-pci hotplug Devices
> 
> It is an issue worth looking into it, one more question, all the measurements 
> are from OS boot? Do you use SeaBIOS?
> No problems with the firmware?
> 
> Thanks,
> Marcel
> 
> 

Re: [Qemu-devel] [PATCH v4 15/22] libqtest: Delete qtest_qmp() wrappers

[Eric Blake]

On 08/09/2017 10:34 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> None of our tests were directly using qtest_qmp() and friends;
>> even tests like postcopy-test.c that manage multiple connections
>> get along just fine changing global_qtest as needed (other than
>> one callsite where it forgot to use the inlined form).  It's
>> also annoying that we have qmp_async() but qtest_async_qmp(),
>> with inconsistent naming for tracing through the wrappers.
>>

> What about all the other functions taking a QTestState?  Aren't they
> just as silly?

What's left after this patch:

- qtest_init()
  qtest_init_without_qmp_handshake()
  qtest_quit()
necessary for setting up parallel state.

and then a lot of functions that have static inline wrappers (for
example, qmp_receive(), inb(), ...).

So yes, I could additionally get rid of more wrappers and have even more
functions implicitly depend on global_qtest.

> 
> Having two of every function is tiresome, but consistent.
> 
> Having just one is easier to maintain, so if it serves our needs,
> possibly with the occasional state switch, I like it.
> 
> What I don't like is a mix of the two.

Okay, I'll prune even harder in the next revision.  Deleting cruft is fun!

>> +++ b/tests/libqtest.c
>> @@ -233,9 +233,10 @@ QTestState *qtest_init(const char *extra_args)
>>  QDict *greeting;
>>
>>  /* Read the QMP greeting and then do the handshake */
>> -greeting = qtest_qmp_receive(s);
>> +greeting = qmp_fd_receive(s->qmp_fd);
> 
> Why doesn't this become qmp_receive()?

Because in THIS version of the patch, qmp_receive() is still a static
inline wrapper that calls qtest_qmp_receive(global_qtest) - but
global_qtest is not set here.  (If I delete qtest_qmp_receive() and have
qmp_receive() not be static inline, then we STILL want to operate
directly on the just-created s->qmp_fd rather than assuming that
global_qtest == s, when in the body of qtest_init).

>> @@ -446,7 +447,7 @@ QDict *qtest_qmp_receive(QTestState *s)
>>   * Internal code that converts from interpolated JSON into a message
>>   * to send over the wire, without waiting for a reply.
>>   */
>> -static void qmp_fd_sendv(int fd, const char *fmt, va_list ap)
>> +static void qtest_qmp_sendv(QTestState *s, const char *fmt, va_list ap)
> 
> Why this move in the other direction?

Because it fixes the disparity you pointed out in 12/22 about
qmp_fd_sendv() no longer being a sane pairing to qmp_fd_send(), AND
because I needed the .../va_list pairing to work in order for...

>> -char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap)
>> +static char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap)
>>  {
>>  char *cmd;
>>  QDict *resp;
>>  char *ret;
>>
>>  cmd = g_strdup_vprintf(fmt, ap);
>> -resp = qtest_qmp(s, "{'execute': 'human-monitor-command',"
>> - " 'arguments': {'command-line': %s}}",
>> - cmd);
>> +qtest_qmp_send(s, "{'execute': 'human-monitor-command',"
>> +   " 'arguments': {'command-line': %s}}", cmd);
>> +resp = qtest_qmp_receive(s);

...this to work.  So now my sane pairing is
qtest_qmp_send()/qtest_qmp_sendv() - although your argument that
qtest_qmp_sendf() might have been a nicer name for the ... form may
still have merit - at least any time the sendv() form is in a public
header.  Then again, by the end of the series, I managed to get rid of
all va_list in libqtest.h, needing it only in libqtest.c.

>> @@ -889,7 +854,7 @@ void qmp_async(const char *fmt, ...)
>>  va_list ap;
>>
>>  va_start(ap, fmt);
>> -qtest_async_qmpv(global_qtest, fmt, ap);
>> +qtest_qmp_sendv(global_qtest, fmt, ap);
>>  va_end(ap);
>>  }
> 
> Hmm.  Before this patch, qmp_async() is the ... buddy of va_list
> qmp_fd_sendv().  If we keep qmp_fd_sendv(), they should be named
> accordingly.

What name to use, though?  By the end of the series, we have
qmp_async(...) but no public va_list form.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [for-2.10 PATCH v3] 9pfs: local: fix fchmodat_nofollow() limitations

[Greg Kurz]

On Wed, 9 Aug 2017 11:19:42 -0500
Eric Blake  wrote:

> On 08/09/2017 11:00 AM, Greg Kurz wrote:
> > This function has to ensure it doesn't follow a symlink that could be used
> > to escape the virtfs directory. This could be easily achieved if fchmodat()
> > on linux honored the AT_SYMLINK_NOFOLLOW flag as described in POSIX, but
> > it doesn't. There was a tentative to implement a new fchmodat2() syscall
> > with the correct semantics:
> > 
> > https://patchwork.kernel.org/patch/9596301/
> > 
> > but it didn't gain much momentum. Also it was suggested to look at a O_PATH 
> >  
> 
> s/a O_PATH/an O_PATH/
> 

Fixed.

> > based solution in the first place.
> > 
> > The current implementation covers most use-cases, but it notably fails if:
> > - the target path has access rights equal to  (openat() returns EPERM), 
> >  
> >   => once you've done chmod() on a file, you can never chmod() again  
> > - the target path is UNIX domain socket (openat() returns ENXIO)  
> >   => bind() of UNIX domain sockets fails if the file is on 9pfs  
> > 
> > The solution is to use O_PATH: openat() now succeeds in both cases, and we
> > can ensure the path isn't a symlink with fstat(). The associated entry in
> > "/proc/self/fd" can hence be safely passed to the regular chmod() syscall.  
> 
> My late-breaking question from v2 remains: fstat() on O_PATH only works

Yeah I saw your mail just after sending the v3 :)

> in kernel 3.6 and newer; are we worried about kernels in the window of
> 2.6.39 (when O_PATH was introduced) and 3.5?  Or at this point, are we
> reasonably sure that platforms are either too old for O_PATH at all
> (Hello RHEL 6, with 2.6.32), or else new enough that we aren't going to
> have spurious failures due to fstat() not doing what we want?
> 
> I don't actually know the failure mode of fstat() on kernel 3.5, so if
> someone cares about that working (presumably because they are on a
> platform with such a kernel), please speak up. (Or even run my test
> program included on the v1 thread, to show us what happens)
> 

That seems reasonable to me.

> > +fd = openat_file(dirfd, name, O_RDONLY | O_PATH_9P_UTIL, 0);
> > +#ifndef O_PATH  
> 
> Please make this '#if O_PATH' or even '#if O_PATH_9P_UTIL'; as it might
> be feasible for someone to
> 
> #ifndef O_PATH
> #define O_PATH 0
> #endif
> 
> where the macro is defined but the feature is not present, messing up
> our code if we only check for a definition.
> 

Ok, I'll do that.

> > +#else
> > +/* Now we handle racing symlinks. */
> > +ret = fstat(fd, );
> > +if (ret) {
> > +goto out;  
> 
> This may leave errno at an unusual value for fchmodat(), if we are on
> kernel 3.5.  But until someone speaks up that it matters, I'm okay
> saving any cleanup work in that area for a followup patch.
> 

Agreed.

> > +}
> > +if (S_ISLNK(stbuf.st_mode)) {
> > +errno = ELOOP;
> > +ret = -1;
> > +goto out;
> > +}
> > +
> > +{
> > +char *proc_path = g_strdup_printf("/proc/self/fd/%d", fd);
> > +ret = chmod(proc_path, mode);
> > +g_free(proc_path);
> > +}
> > +#endif
> > +out:  
> 
> Swap these two lines - your only use of 'goto out' are under the O_PATH
> branch, and therefore you get a compilation failure about unused label
> on older glibc.
> 

Oops.

> With the #if condition fixed and the scope of the #endif fixed,
> 
> Reviewed-by: Eric Blake 
> 

Thanks !


pgpuADwN891y9.pgp
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3] 9pfs: fix dependencies

[Cornelia Huck]

Nothing in fsdev/ or hw/9pfs/ depends on pci; it should rather depend
on CONFIG_VIRTFS and CONFIG_VIRTIO/CONFIG_XEN only.

Signed-off-by: Cornelia Huck 
---

v2->v3: switch dependencies to VIRTFS && (VIRTIO || XEN)
add explict VIRTIO dependency for virtio-9p-device.o

---
 fsdev/Makefile.objs   | 9 +++--
 hw/9pfs/Makefile.objs | 2 +-
 hw/Makefile.objs  | 2 +-
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
index 659df6e187..fb38017c0b 100644
--- a/fsdev/Makefile.objs
+++ b/fsdev/Makefile.objs
@@ -1,10 +1,7 @@
-ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
 # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
-# only pull in the actual virtio-9p device if we also enabled virtio.
-common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
-else
-common-obj-y = qemu-fsdev-dummy.o
-endif
+# only pull in the actual 9p backend if we also enabled virtio or xen.
+common-obj-$(call land,$(CONFIG_VIRTFS),$(call 
lor,$(CONFIG_VIRTIO),$(CONFIG_XEN))) = qemu-fsdev.o 9p-marshal.o 
9p-iov-marshal.o
+common-obj-$(call lnot,$(call land,$(CONFIG_VIRTFS),$(call 
lor,$(CONFIG_VIRTIO),$(CONFIG_XEN = qemu-fsdev-dummy.o
 common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
 
 # Toplevel always builds this; targets without virtio will put it in
diff --git a/hw/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
index cab5e942ed..fd90b62900 100644
--- a/hw/9pfs/Makefile.objs
+++ b/hw/9pfs/Makefile.objs
@@ -7,4 +7,4 @@ common-obj-$(CONFIG_OPEN_BY_HANDLE) +=  9p-handle.o
 common-obj-y += 9p-proxy.o
 common-obj-$(CONFIG_XEN) += xen-9p-backend.o
 
-obj-y += virtio-9p-device.o
+obj-$(CONFIG_VIRTIO) += virtio-9p-device.o
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index a2c61f6b09..cf4cb2010b 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -1,4 +1,4 @@
-devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call 
land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
+devices-dirs-$(call land,$(CONFIG_VIRTFS),$(call 
lor,$(CONFIG_VIRTIO),$(CONFIG_XEN))) += 9pfs/
 devices-dirs-$(CONFIG_SOFTMMU) += acpi/
 devices-dirs-$(CONFIG_SOFTMMU) += adc/
 devices-dirs-$(CONFIG_SOFTMMU) += audio/
-- 
2.13.4

Re: [Qemu-devel] [for-2.10 PATCH v3] 9pfs: local: fix fchmodat_nofollow() limitations

[Eric Blake]

On 08/09/2017 11:00 AM, Greg Kurz wrote:
> This function has to ensure it doesn't follow a symlink that could be used
> to escape the virtfs directory. This could be easily achieved if fchmodat()
> on linux honored the AT_SYMLINK_NOFOLLOW flag as described in POSIX, but
> it doesn't. There was a tentative to implement a new fchmodat2() syscall
> with the correct semantics:
> 
> https://patchwork.kernel.org/patch/9596301/
> 
> but it didn't gain much momentum. Also it was suggested to look at a O_PATH

s/a O_PATH/an O_PATH/

> based solution in the first place.
> 
> The current implementation covers most use-cases, but it notably fails if:
> - the target path has access rights equal to  (openat() returns EPERM),
>   => once you've done chmod() on a file, you can never chmod() again
> - the target path is UNIX domain socket (openat() returns ENXIO)
>   => bind() of UNIX domain sockets fails if the file is on 9pfs
> 
> The solution is to use O_PATH: openat() now succeeds in both cases, and we
> can ensure the path isn't a symlink with fstat(). The associated entry in
> "/proc/self/fd" can hence be safely passed to the regular chmod() syscall.

My late-breaking question from v2 remains: fstat() on O_PATH only works
in kernel 3.6 and newer; are we worried about kernels in the window of
2.6.39 (when O_PATH was introduced) and 3.5?  Or at this point, are we
reasonably sure that platforms are either too old for O_PATH at all
(Hello RHEL 6, with 2.6.32), or else new enough that we aren't going to
have spurious failures due to fstat() not doing what we want?

I don't actually know the failure mode of fstat() on kernel 3.5, so if
someone cares about that working (presumably because they are on a
platform with such a kernel), please speak up. (Or even run my test
program included on the v1 thread, to show us what happens)

> +fd = openat_file(dirfd, name, O_RDONLY | O_PATH_9P_UTIL, 0);
> +#ifndef O_PATH

Please make this '#if O_PATH' or even '#if O_PATH_9P_UTIL'; as it might
be feasible for someone to

#ifndef O_PATH
#define O_PATH 0
#endif

where the macro is defined but the feature is not present, messing up
our code if we only check for a definition.

> +#else
> +/* Now we handle racing symlinks. */
> +ret = fstat(fd, );
> +if (ret) {
> +goto out;

This may leave errno at an unusual value for fchmodat(), if we are on
kernel 3.5.  But until someone speaks up that it matters, I'm okay
saving any cleanup work in that area for a followup patch.

> +}
> +if (S_ISLNK(stbuf.st_mode)) {
> +errno = ELOOP;
> +ret = -1;
> +goto out;
> +}
> +
> +{
> +char *proc_path = g_strdup_printf("/proc/self/fd/%d", fd);
> +ret = chmod(proc_path, mode);
> +g_free(proc_path);
> +}
> +#endif
> +out:

Swap these two lines - your only use of 'goto out' are under the O_PATH
branch, and therefore you get a compilation failure about unused label
on older glibc.

With the #if condition fixed and the scope of the #endif fixed,

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 for 2.10] iotests: fix 185

[Eric Blake]

On 08/09/2017 10:57 AM, Vladimir Sementsov-Ogievskiy wrote:

 This is because quite happens before first mirror request is done
>>> s/quite/quit/
>>>
 (and, in specified case, even before block-job len field is set).
 To prevent it let's just add a sleep for 0.3 seconds before quite.
>> Here, as well.
>>
>> Maybe:
>>
>> This is because, under heavy load, the quit can happen before the first
>> iteration of the mirror request has occurred.  To make sure we've had
>> time to iterate, let's just add a sleep for 0.3 seconds before quitting.
>>
>>
"return"
+# If we don't sleep here 'quit' command may be handled before
 +# the first mirror iteration is done
 +sleep 0.5
>> The commit message disagrees with the code (.3 vs. .5) - which one is
>> correct?
>>
> Let it be .5, as I've tested it.

With a correct commit message (maintainer can touch it up),
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 4/4] block-backend: test flush op on empty backend

[Kevin Wolf]

Am 08.08.2017 um 19:57 hat John Snow geschrieben:
> From: Kevin Wolf 
> 
> Signed-off-by: Kevin Wolf 
> Signed-off-by: John Snow 
> ---
>  tests/Makefile.include |  2 ++
>  tests/test-block-backend.c | 62 
> ++
>  2 files changed, 64 insertions(+)
>  create mode 100644 tests/test-block-backend.c
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index eb4895f..153494b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -56,6 +56,7 @@ check-unit-y += tests/test-hbitmap$(EXESUF)
>  gcov-files-test-hbitmap-y = blockjob.c
>  check-unit-y += tests/test-blockjob$(EXESUF)
>  check-unit-y += tests/test-blockjob-txn$(EXESUF)
> +check-unit-y += tests/test-block-backend$(EXESUF)
>  check-unit-y += tests/test-x86-cpuid$(EXESUF)
>  # all code tested by test-x86-cpuid is inside topology.h
>  gcov-files-test-x86-cpuid-y =
> @@ -556,6 +557,7 @@ tests/test-aio-multithread$(EXESUF): 
> tests/test-aio-multithread.o $(test-block-o
>  tests/test-throttle$(EXESUF): tests/test-throttle.o $(test-block-obj-y)
>  tests/test-blockjob$(EXESUF): tests/test-blockjob.o $(test-block-obj-y) 
> $(test-util-obj-y)
>  tests/test-blockjob-txn$(EXESUF): tests/test-blockjob-txn.o 
> $(test-block-obj-y) $(test-util-obj-y)
> +tests/test-block-backend$(EXESUF): tests/test-block-backend.o 
> $(test-block-obj-y) $(test-util-obj-y)
>  tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y)
>  tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
>  tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) 
> $(test-crypto-obj-y)
> diff --git a/tests/test-block-backend.c b/tests/test-block-backend.c
> new file mode 100644
> index 000..5348781
> --- /dev/null
> +++ b/tests/test-block-backend.c
> @@ -0,0 +1,62 @@
> +/*
> + * BlockBackend tests
> + *
> + * Copyright (c) 2017 Kevin Wolf 
> + *
> + * 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.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "block/block.h"
> +#include "sysemu/block-backend.h"
> +#include "qapi/error.h"
> +
> +static void test_drain_aio_error_flush_cb(void *opaque, int ret)
> +{
> +bool *completed = opaque;
> +
> +g_assert(ret == -ENOMEDIUM);
> +*completed = true;
> +}
> +
> +static void test_drain_aio_error(void)
> +{
> +BlockBackend *blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
> +BlockAIOCB *acb;
> +bool completed = false;
> +
> +acb = blk_aio_flush(blk, test_drain_aio_error_flush_cb, );
> +g_assert(acb != NULL);
> +g_assert(completed == false);
> +
> +blk_drain(blk);
> +g_assert(completed == true);
> +}

Locally, I added a second test for blk_drain_all():

static void test_drain_all_aio_error(void)
{
BlockBackend *blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
BlockAIOCB *acb;
bool completed = false;

acb = blk_aio_flush(blk, test_drain_aio_error_flush_cb, );
g_assert(acb != NULL);
g_assert(completed == false);

blk_drain_all();
g_assert(completed == true);

blk_unref(blk);
}

> +int main(int argc, char **argv)
> +{
> +bdrv_init();
> +qemu_init_main_loop(_abort);
> +
> +g_test_init(, , NULL);
> +
> +g_test_add_func("/block-backend/drain_aio_error", test_drain_aio_error);
> +
> +return g_test_run();
> +}

Kevin

[Qemu-devel] [PATCH 2/2] IDE: test flush on empty CDROM

[Stefan Hajnoczi]

From: Kevin Wolf 

Signed-off-by: Kevin Wolf 
Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Signed-off-by: Stefan Hajnoczi 
---
 tests/ide-test.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index bfd79ddbdc..aa9de065fc 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -689,6 +689,24 @@ static void test_flush_nodev(void)
 ide_test_quit();
 }
 
+static void test_flush_empty_drive(void)
+{
+QPCIDevice *dev;
+QPCIBar bmdma_bar, ide_bar;
+
+ide_test_start("-device ide-cd,bus=ide.0");
+dev = get_pci_device(_bar, _bar);
+
+/* FLUSH CACHE command on device 0 */
+qpci_io_writeb(dev, ide_bar, reg_device, 0);
+qpci_io_writeb(dev, ide_bar, reg_command, CMD_FLUSH_CACHE);
+
+/* Just testing that qemu doesn't crash... */
+
+free_pci_device(dev);
+ide_test_quit();
+}
+
 static void test_pci_retry_flush(void)
 {
 test_retry_flush("pc");
@@ -954,6 +972,7 @@ int main(int argc, char **argv)
 
 qtest_add_func("/ide/flush", test_flush);
 qtest_add_func("/ide/flush/nodev", test_flush_nodev);
+qtest_add_func("/ide/flush/empty_drive", test_flush_empty_drive);
 qtest_add_func("/ide/flush/retry_pci", test_pci_retry_flush);
 qtest_add_func("/ide/flush/retry_isa", test_isa_retry_flush);
 
-- 
2.13.3

[Qemu-devel] [PATCH 1/2] IDE: Do not flush empty CDROM drives

[Stefan Hajnoczi]

The block backend changed in a way that flushing empty CDROM drives now
crashes.  Amend IDE to avoid doing so until the root problem can be
addressed for 2.11.

Original patch by John Snow .

Reported-by: Kieron Shorrock 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 0b48b64d3a..bea39536b0 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1063,7 +1063,15 @@ static void ide_flush_cache(IDEState *s)
 s->status |= BUSY_STAT;
 ide_set_retry(s);
 block_acct_start(blk_get_stats(s->blk), >acct, 0, BLOCK_ACCT_FLUSH);
-s->pio_aiocb = blk_aio_flush(s->blk, ide_flush_cb, s);
+
+if (blk_bs(s->blk)) {
+s->pio_aiocb = blk_aio_flush(s->blk, ide_flush_cb, s);
+} else {
+/* XXX blk_aio_flush() crashes when blk_bs(blk) is NULL, remove this
+ * temporary workaround when blk_aio_*() functions handle NULL blk_bs.
+ */
+ide_flush_cb(s, 0);
+}
 }
 
 static void ide_cfata_metadata_inquiry(IDEState *s)
-- 
2.13.3

[Qemu-devel] [PATCH 0/2] IDE: Do not flush empty drives

[Stefan Hajnoczi]

John Snow is offline until Monday, QEMU 2.10-rc3 is due to be tagged on
Tuesday, and so I've taken two patches John sent for 2.10 and addressed review
comments.

Kevin Wolf (1):
  IDE: test flush on empty CDROM

Stefan Hajnoczi (1):
  IDE: Do not flush empty CDROM drives

 hw/ide/core.c| 10 +-
 tests/ide-test.c | 19 +++
 2 files changed, 28 insertions(+), 1 deletion(-)

-- 
2.13.3

Re: [Qemu-devel] [PATCH v2] 9pfs: fix dependencies

[Cornelia Huck]

On Wed, 9 Aug 2017 14:10:01 +0200
Greg Kurz  wrote:

> On Wed, 9 Aug 2017 13:06:14 +0200
> Cornelia Huck  wrote:

> > Should the condition be VIRTFS && (VIRTIO || XEN), then?  
> 
> That's what I was beginning to think as well :)

OK, here's what should work:

- fsdev/Makefile.objs needs to check for VIRTFS && (VIRTIO || XEN)
- hw/Makefile.objs needs to check for VIRTFS && (VIRTIO || XEN) before
  building hw/9pfs/
- hw/9pfs/Makefile.objs needs a new VIRTIO check for the virtio device

I'm preparing a patch.

Re: [Qemu-devel] [PATCH 3/4] block-backend: shift in-flight counter to BB from BDS

[Kevin Wolf]

Am 08.08.2017 um 19:57 hat John Snow geschrieben:
> From: Kevin Wolf 
> 
> This allows us to detect errors in cache flushing (ENOMEDIUM)
> without choking on a null dereference because we assume that
> blk_bs(bb) is always defined.
> 
> Signed-off-by: Kevin Wolf 
> Signed-off-by: John Snow 
> ---
>  block.c   |  2 +-
>  block/block-backend.c | 40 ++--
>  2 files changed, 35 insertions(+), 7 deletions(-)
> 
> diff --git a/block.c b/block.c
> index ce9cce7..834b836 100644
> --- a/block.c
> +++ b/block.c
> @@ -4476,7 +4476,7 @@ out:
>  
>  AioContext *bdrv_get_aio_context(BlockDriverState *bs)
>  {
> -return bs->aio_context;
> +return bs ? bs->aio_context : qemu_get_aio_context();
>  }

This should probably be a separate patch; it's not really related to
moving the in-flight counter, but fixes another NULL dereference in
blk_aio_prwv().

>  void bdrv_coroutine_enter(BlockDriverState *bs, Coroutine *co)
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 968438c..efd7e92 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -68,6 +68,9 @@ struct BlockBackend {
>  NotifierList remove_bs_notifiers, insert_bs_notifiers;
>  
>  int quiesce_counter;
> +
> +/* Number of in-flight requests. Accessed with atomic ops. */
> +unsigned int in_flight;
>  };
>  
>  typedef struct BlockBackendAIOCB {
> @@ -1109,6 +1112,16 @@ int blk_make_zero(BlockBackend *blk, BdrvRequestFlags 
> flags)
>  return bdrv_make_zero(blk->root, flags);
>  }
>  
> +static void blk_inc_in_flight(BlockBackend *blk)
> +{
> +atomic_inc(>in_flight);
> +}
> +
> +static void blk_dec_in_flight(BlockBackend *blk)
> +{
> +atomic_dec(>in_flight);
> +}
> +
>  static void error_callback_bh(void *opaque)
>  {
>  struct BlockBackendAIOCB *acb = opaque;
> @@ -1147,7 +1160,7 @@ static const AIOCBInfo blk_aio_em_aiocb_info = {
>  static void blk_aio_complete(BlkAioEmAIOCB *acb)
>  {
>  if (acb->has_returned) {
> -bdrv_dec_in_flight(acb->common.bs);
> +blk_dec_in_flight(acb->rwco.blk);
>  acb->common.cb(acb->common.opaque, acb->rwco.ret);
>  qemu_aio_unref(acb);
>  }
> @@ -1168,7 +1181,7 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, 
> int64_t offset, int bytes,
>  BlkAioEmAIOCB *acb;
>  Coroutine *co;
>  
> -bdrv_inc_in_flight(blk_bs(blk));
> +blk_inc_in_flight(blk);
>  acb = blk_aio_get(_aio_em_aiocb_info, blk, cb, opaque);
>  acb->rwco = (BlkRwCo) {
>  .blk= blk,
> @@ -1405,14 +1418,28 @@ int blk_flush(BlockBackend *blk)
>  
>  void blk_drain(BlockBackend *blk)
>  {
> -if (blk_bs(blk)) {
> -bdrv_drain(blk_bs(blk));
> +AioContext *ctx = blk_get_aio_context(blk);
> +
> +while (atomic_read(>in_flight)) {
> +aio_context_acquire(ctx);
> +aio_poll(ctx, false);
> +aio_context_release(ctx);
> +
> +if (blk_bs(blk)) {
> +bdrv_drain(blk_bs(blk));
> +}
>  }
>  }
>  
>  void blk_drain_all(void)
>  {
> -bdrv_drain_all();
> +BlockBackend *blk = NULL;
> +
> +bdrv_drain_all_begin();
> +while ((blk = blk_all_next(blk)) != NULL) {
> +blk_drain(blk);
> +}
> +bdrv_drain_all_end();
>  }

We still need to check that everyone who should call blk_drain_all()
rather than bdrv_drain_all() actually does so.

>  void blk_set_on_error(BlockBackend *blk, BlockdevOnError on_read_error,
> @@ -1453,10 +1480,11 @@ static void send_qmp_error_event(BlockBackend *blk,
>   bool is_read, int error)
>  {
>  IoOperationType optype;
> +BlockDriverState *bs = blk_bs(blk);
>  
>  optype = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE;
>  qapi_event_send_block_io_error(blk_name(blk),
> -   bdrv_get_node_name(blk_bs(blk)), optype,
> +   bs ? bdrv_get_node_name(bs) : "", optype,
> action, blk_iostatus_is_enabled(blk),
> error == ENOSPC, strerror(error),
> _abort);

And this is another independent NULL dereference fix.

Kevin

Re: [Qemu-devel] [for-2.10 PATCH v2] 9pfs: local: fix fchmodat_nofollow() limitations

[Greg Kurz]

On Wed, 9 Aug 2017 10:59:46 -0500
Eric Blake  wrote:

> On 08/09/2017 10:22 AM, Greg Kurz wrote:
> 
> >>>
> >>> The solution is to use O_PATH: openat() now succeeds in both cases, and we
> >>> can ensure the path isn't a symlink with fstat(). The associated entry in
> >>> "/proc/self/fd" can hence be safely passed to the regular chmod() 
> >>> syscall.
> >>
> >> Hey - should we point this out as a viable solution to the glibc folks,
> >> since their current user-space emulation of AT_SYMLINK_NOFOLLOW is broken?
> >>  
> > 
> > Probably. What's the best way to do that ?  
> 
> I've added a comment to
> https://sourceware.org/bugzilla/show_bug.cgi?id=14578; you'll also want
> to point to the lkml discussion in that bug.  And reading that bug, it
> also looks like your hack with /proc/self/fd has been proposed by Rich
> Felker since 2013! (although fstat() didn't work until Linux 3.6, even
> though O_PATH predates that time) - so there is that one additional
> concern of whether we need to cater to the window of kernels where
> O_PATH exists but fstat() on that fd can't learn whether we opened a
> symlink.
> 

BTW, what happens with fstat() and O_PATH before Linux 3.6 ? Does it
fail or does it return something wrong in th stat buf ?


pgp64UsrRX43U.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 21/22] libqtest: Drop now-unused qmp()

[Markus Armbruster]

Eric Blake  writes:

> All callers have been converted to a form of qmp_cmd() or
> qmp_args() that takes the command name with less boilerplate.
> Therefore, we also know that all commands are using
> interpolation, and can remove an assertion.
>
> This also means that we have fixed the testsuite to comply with
> -Wformat checking on the strings being interpolated for qmp()
> (similar to what we previously did for strings used in hmp(), and
> matching the checking present on qobject_from_jsonf()).
>
> Signed-off-by: Eric Blake 
> ---
>  tests/libqtest.h | 20 +---
>  tests/libqtest.c | 32 
>  2 files changed, 5 insertions(+), 47 deletions(-)
>
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 193adf1eb9..04b36a7b11 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -457,15 +457,6 @@ static inline void qtest_end(void)
>  }
>
>  /**
> - * qmp:
> - * @fmt...: QMP message to send to qemu; formats arguments through
> - * json-lexer.c (only understands '%(PRI[ud]64|(l|ll)?[du]|[ipsf%])').
> - *
> - * Sends a QMP message to QEMU and returns the response.
> - */
> -QDict *qmp(const char *fmt, ...);
> -
> -/**
>   * qmp_raw:
>   * @msg: Raw QMP message to send to qemu.
>   *
> @@ -474,15 +465,6 @@ QDict *qmp(const char *fmt, ...);
>  QDict *qmp_raw(const char *msg);
>
>  /**
> - * qmp_async:
> - * @fmt...: QMP message to send to qemu; formats arguments through
> - * json-lexer.c (only understands '%(PRI[ud]64|(l|ll)?[du]|[ipsf%])').
> - *
> - * Sends a QMP message to QEMU and leaves the response in the stream.
> - */
> -void qmp_async(const char *fmt, ...);
> -
> -/**
>   * qmp_cmd:
>   * @cmd: QMP command, with no arguments.
>   *
> @@ -530,7 +512,7 @@ void qmp_args_async(const char *cmd, const char *fmt, 
> ...) GCC_FMT_ATTR(2, 3);
>  /**
>   * qmp_discard_response:
>   *
> - * Read and discard a QMP response, typically after qmp_async().
> + * Read and discard a QMP response, typically after qmp_cmd_async().
>   */
>  void qmp_discard_response(void);
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 5012ecf929..4597d4ac66 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -453,17 +453,12 @@ static void qtest_qmp_sendv(QTestState *s, const char 
> *fmt, va_list ap)
>  QString *qstr;
>  const char *str;
>
> -assert(strstr(fmt, "execute"));
> -
>  /*
> - * A round trip through QObject is only needed if % interpolation
> - * is used.  We interpolate through QObject rather than sprintf in
> - * order to escape strings properly.
> + * A round trip through QObject (and not sprintf) is needed
> + * because % interpolation is used, and we must escape strings
> + * properly.
>   */
> -if (!strchr(fmt, '%')) {
> -qmp_fd_send(s->qmp_fd, fmt);
> -return;
> -}
> +assert(strchr(fmt, '%'));

What exactly is wrong with a @fmt that doesn't contain '%'?

>
>  qobj = qobject_from_jsonv(fmt, ap);
>  qstr = qobject_to_json(qobj);
> @@ -833,31 +828,12 @@ void qtest_memset(QTestState *s, uint64_t addr, uint8_t 
> pattern, size_t size)
>  qtest_rsp(s, 0);
>  }
>
> -QDict *qmp(const char *fmt, ...)
> -{
> -va_list ap;
> -
> -va_start(ap, fmt);
> -qtest_qmp_sendv(global_qtest, fmt, ap);
> -va_end(ap);
> -return qtest_qmp_receive(global_qtest);
> -}
> -
>  QDict *qmp_raw(const char *msg)
>  {
>  qmp_fd_send(global_qtest->qmp_fd, msg);
>  return qtest_qmp_receive(global_qtest);
>  }
>
> -void qmp_async(const char *fmt, ...)
> -{
> -va_list ap;
> -
> -va_start(ap, fmt);
> -qtest_qmp_sendv(global_qtest, fmt, ap);
> -va_end(ap);
> -}
> -
>  QDict *qmp_cmd(const char *cmd)
>  {
>  qmp_cmd_async(cmd);

[Qemu-devel] [for-2.10 PATCH v3] 9pfs: local: fix fchmodat_nofollow() limitations

[Greg Kurz]

This function has to ensure it doesn't follow a symlink that could be used
to escape the virtfs directory. This could be easily achieved if fchmodat()
on linux honored the AT_SYMLINK_NOFOLLOW flag as described in POSIX, but
it doesn't. There was a tentative to implement a new fchmodat2() syscall
with the correct semantics:

https://patchwork.kernel.org/patch/9596301/

but it didn't gain much momentum. Also it was suggested to look at a O_PATH
based solution in the first place.

The current implementation covers most use-cases, but it notably fails if:
- the target path has access rights equal to  (openat() returns EPERM),
  => once you've done chmod() on a file, you can never chmod() again
- the target path is UNIX domain socket (openat() returns ENXIO)
  => bind() of UNIX domain sockets fails if the file is on 9pfs

The solution is to use O_PATH: openat() now succeeds in both cases, and we
can ensure the path isn't a symlink with fstat(). The associated entry in
"/proc/self/fd" can hence be safely passed to the regular chmod() syscall.

The previous behavior is kept for older systems that don't have O_PATH.

Signed-off-by: Greg Kurz 
---
v3: - O_PATH in a separate block of code
- added a reference to the fchmodat2() tentative in the changelog

v2: - renamed OPENAT_DIR_O_PATH to O_PATH_9P_UTIL and use it as a replacement
  for O_PATH to avoid build breaks on O_PATH-less systems
- keep current behavior for O_PATH-less systems
- added comments
- TODO in 2.11: add _nofollow suffix to openat_dir() and openat_file()
---
 hw/9pfs/9p-local.c |   43 ---
 hw/9pfs/9p-util.h  |   24 +++-
 2 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 6e478f4765ef..1e20d4881089 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -333,17 +333,27 @@ update_map_file:
 
 static int fchmodat_nofollow(int dirfd, const char *name, mode_t mode)
 {
+struct stat stbuf;
 int fd, ret;
 
 /* FIXME: this should be handled with fchmodat(AT_SYMLINK_NOFOLLOW).
- * Unfortunately, the linux kernel doesn't implement it yet. As an
- * alternative, let's open the file and use fchmod() instead. This
- * may fail depending on the permissions of the file, but it is the
- * best we can do to avoid TOCTTOU. We first try to open read-only
- * in case name points to a directory. If that fails, we try write-only
- * in case name doesn't point to a directory.
+ * Unfortunately, the linux kernel doesn't implement it yet.
  */
-fd = openat_file(dirfd, name, O_RDONLY, 0);
+
+ /* First, we clear non-racing symlinks out of the way. */
+if (fstatat(dirfd, name, , AT_SYMLINK_NOFOLLOW)) {
+return -1;
+}
+if (S_ISLNK(stbuf.st_mode)) {
+errno = ELOOP;
+return -1;
+}
+
+/* Access modes are ignored when O_PATH is supported. We try O_RDONLY and
+ * O_WRONLY for old-systems that don't support O_PATH.
+ */
+fd = openat_file(dirfd, name, O_RDONLY | O_PATH_9P_UTIL, 0);
+#ifndef O_PATH
 if (fd == -1) {
 /* In case the file is writable-only and isn't a directory. */
 if (errno == EACCES) {
@@ -357,6 +367,25 @@ static int fchmodat_nofollow(int dirfd, const char *name, 
mode_t mode)
 return -1;
 }
 ret = fchmod(fd, mode);
+#else
+/* Now we handle racing symlinks. */
+ret = fstat(fd, );
+if (ret) {
+goto out;
+}
+if (S_ISLNK(stbuf.st_mode)) {
+errno = ELOOP;
+ret = -1;
+goto out;
+}
+
+{
+char *proc_path = g_strdup_printf("/proc/self/fd/%d", fd);
+ret = chmod(proc_path, mode);
+g_free(proc_path);
+}
+#endif
+out:
 close_preserve_errno(fd);
 return ret;
 }
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 91299a24b8af..dc0d2e29aa3b 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -13,6 +13,12 @@
 #ifndef QEMU_9P_UTIL_H
 #define QEMU_9P_UTIL_H
 
+#ifdef O_PATH
+#define O_PATH_9P_UTIL O_PATH
+#else
+#define O_PATH_9P_UTIL 0
+#endif
+
 static inline void close_preserve_errno(int fd)
 {
 int serrno = errno;
@@ -22,13 +28,8 @@ static inline void close_preserve_errno(int fd)
 
 static inline int openat_dir(int dirfd, const char *name)
 {
-#ifdef O_PATH
-#define OPENAT_DIR_O_PATH O_PATH
-#else
-#define OPENAT_DIR_O_PATH 0
-#endif
 return openat(dirfd, name,
-  O_DIRECTORY | O_RDONLY | O_NOFOLLOW | OPENAT_DIR_O_PATH);
+  O_DIRECTORY | O_RDONLY | O_NOFOLLOW | O_PATH_9P_UTIL);
 }
 
 static inline int openat_file(int dirfd, const char *name, int flags,
@@ -43,9 +44,14 @@ static inline int openat_file(int dirfd, const char *name, 
int flags,
 }
 
 serrno = errno;
-/* O_NONBLOCK was only needed to open the file. Let's drop it. */
-ret = fcntl(fd, F_SETFL, flags);
-assert(!ret);
+/* O_NONBLOCK was only needed to 

Re: [Qemu-devel] [for-2.10 PATCH v2] 9pfs: local: fix fchmodat_nofollow() limitations

[Eric Blake]

On 08/09/2017 10:22 AM, Greg Kurz wrote:

>>>
>>> The solution is to use O_PATH: openat() now succeeds in both cases, and we
>>> can ensure the path isn't a symlink with fstat(). The associated entry in
>>> "/proc/self/fd" can hence be safely passed to the regular chmod() syscall.  
>>
>> Hey - should we point this out as a viable solution to the glibc folks,
>> since their current user-space emulation of AT_SYMLINK_NOFOLLOW is broken?
>>
> 
> Probably. What's the best way to do that ?

I've added a comment to
https://sourceware.org/bugzilla/show_bug.cgi?id=14578; you'll also want
to point to the lkml discussion in that bug.  And reading that bug, it
also looks like your hack with /proc/self/fd has been proposed by Rich
Felker since 2013! (although fstat() didn't work until Linux 3.6, even
though O_PATH predates that time) - so there is that one additional
concern of whether we need to cater to the window of kernels where
O_PATH exists but fstat() on that fd can't learn whether we opened a
symlink.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature