[Qemu-devel] [PULL 00/20] Block patches

[Qemu-devel] [PULL 00/20] Block patches

[Stefan Hajnoczi]

The following changes since commit 482b61844ae7c6df39df0b48ac90ffbc87bed7d2:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20160617' into staging (2016-06-17 16:16:37 +0100)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 5ab4b69ce29908b327a91966dc78ea0fd7424075:

  backup: follow AioContext change gracefully (2016-06-20 14:25:41 +0100)

----------------------------------------------------------------

----------------------------------------------------------------

Denis V. Lunev (3):
  block: fixed BdrvTrackedRequest filling in bdrv_co_discard
  block: fix race in bdrv_co_discard with drive-mirror
  block: process before_write_notifiers in bdrv_co_discard

Stefan Hajnoczi (17):
  libqos: use virtio_ids.h for device ID definitions
  libqos: drop duplicated PCI vendor ID definition
  libqos: drop duplicated virtio_config.h definitions
  libqos: drop duplicated virtio_ring.h bit definitions
  libqos: drop duplicated virtio_vring.h structs
  libqos: drop duplicated virtio_blk.h definitions
  libqos: drop duplicated virtio_scsi.h definitions
  libqos: drop duplicated virtio_pci.h definitions
  libqos: add qvirtqueue_cleanup()
  blockjob: move iostatus reset out of block_job_enter()
  blockjob: rename block_job_is_paused()
  blockjob: add pause points
  blockjob: add block_job_get_aio_context()
  block: use safe iteration over AioContext notifiers
  blockjob: add AioContext attached callback
  mirror: follow AioContext change gracefully
  backup: follow AioContext change gracefully

 block.c                    | 46 +++++++++++++++++-----
 block/backup.c             | 22 +++++++----
 block/io.c                 | 12 ++++--
 block/mirror.c             | 43 ++++++++++++++++----
 blockdev.c                 |  1 +
 blockjob.c                 | 97 ++++++++++++++++++++++++++++++++++++++++------
 include/block/block_int.h  |  2 +
 include/block/blockjob.h   | 51 ++++++++++++++++++------
 tests/libqos/virtio-mmio.c | 13 ++++++-
 tests/libqos/virtio-pci.c  | 60 +++++++++++++++++-----------
 tests/libqos/virtio-pci.h  | 17 --------
 tests/libqos/virtio.c      | 48 +++++++++++++----------
 tests/libqos/virtio.h      | 78 ++++++-------------------------------
 tests/virtio-blk-test.c    | 96 ++++++++++++++++++++-------------------------
 tests/virtio-net-test.c    | 12 +++---
 tests/virtio-scsi-test.c   | 55 ++++++++++----------------
 16 files changed, 381 insertions(+), 272 deletions(-)

-- 
2.5.5

[Qemu-devel] [PULL 01/20] libqos: use virtio_ids.h for device ID definitions

[Stefan Hajnoczi]

Avoid redefining device IDs.  Use the standard Linux headers that are
already in the source tree.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1462798061-30382-2-git-send-email-stefanha@redhat.com
---
 tests/libqos/virtio.h    | 9 ---------
 tests/virtio-blk-test.c  | 7 ++++---
 tests/virtio-net-test.c  | 5 +++--
 tests/virtio-scsi-test.c | 5 +++--
 4 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 0101278..af03793 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -19,15 +19,6 @@
 #define QVIRTIO_DRIVER          0x2
 #define QVIRTIO_DRIVER_OK       0x4
 
-#define QVIRTIO_NET_DEVICE_ID       0x1
-#define QVIRTIO_BLK_DEVICE_ID       0x2
-#define QVIRTIO_CONSOLE_DEVICE_ID   0x3
-#define QVIRTIO_RNG_DEVICE_ID       0x4
-#define QVIRTIO_BALLOON_DEVICE_ID   0x5
-#define QVIRTIO_RPMSG_DEVICE_ID     0x7
-#define QVIRTIO_SCSI_DEVICE_ID      0x8
-#define QVIRTIO_9P_DEVICE_ID        0x9
-
 #define QVIRTIO_F_NOTIFY_ON_EMPTY       0x01000000
 #define QVIRTIO_F_ANY_LAYOUT            0x08000000
 #define QVIRTIO_F_RING_INDIRECT_DESC    0x10000000
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 8272ba8..a0d2d83 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -18,6 +18,7 @@
 #include "libqos/malloc-pc.h"
 #include "libqos/malloc-generic.h"
 #include "qemu/bswap.h"
+#include "standard-headers/linux/virtio_ids.h"
 
 #define QVIRTIO_BLK_F_BARRIER       0x00000001
 #define QVIRTIO_BLK_F_SIZE_MAX      0x00000002
@@ -118,9 +119,9 @@ static QVirtioPCIDevice *virtio_blk_pci_init(QPCIBus *bus, int slot)
 {
     QVirtioPCIDevice *dev;
 
-    dev = qvirtio_pci_device_find(bus, QVIRTIO_BLK_DEVICE_ID);
+    dev = qvirtio_pci_device_find(bus, VIRTIO_ID_BLOCK);
     g_assert(dev != NULL);
-    g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID);
+    g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_BLOCK);
     g_assert_cmphex(dev->pdev->devfn, ==, ((slot << 3) | PCI_FN));
 
     qvirtio_pci_device_enable(dev);
@@ -732,7 +733,7 @@ static void mmio_basic(void)
 
     dev = qvirtio_mmio_init_device(MMIO_DEV_BASE_ADDR, MMIO_PAGE_SIZE);
     g_assert(dev != NULL);
-    g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID);
+    g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_BLOCK);
 
     qvirtio_reset(&qvirtio_mmio, &dev->vdev);
     qvirtio_set_acknowledge(&qvirtio_mmio, &dev->vdev);
diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index e5c1448..e49c13e 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -20,6 +20,7 @@
 #include "libqos/malloc-generic.h"
 #include "qemu/bswap.h"
 #include "hw/virtio/virtio-net.h"
+#include "standard-headers/linux/virtio_ids.h"
 
 #define PCI_SLOT_HP             0x06
 #define PCI_SLOT                0x04
@@ -39,9 +40,9 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
 {
     QVirtioPCIDevice *dev;
 
-    dev = qvirtio_pci_device_find(bus, QVIRTIO_NET_DEVICE_ID);
+    dev = qvirtio_pci_device_find(bus, VIRTIO_ID_NET);
     g_assert(dev != NULL);
-    g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_NET_DEVICE_ID);
+    g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_NET);
 
     qvirtio_pci_device_enable(dev);
     qvirtio_reset(&qvirtio_pci, &dev->vdev);
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 5f1a8ae..26bb0e3 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -17,6 +17,7 @@
 #include "libqos/malloc.h"
 #include "libqos/malloc-pc.h"
 #include "libqos/malloc-generic.h"
+#include "standard-headers/linux/virtio_ids.h"
 
 #define PCI_SLOT                0x02
 #define PCI_FN                  0x00
@@ -163,10 +164,10 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
     vs->alloc = pc_alloc_init();
     vs->bus = qpci_init_pc();
 
-    dev = qvirtio_pci_device_find(vs->bus, QVIRTIO_SCSI_DEVICE_ID);
+    dev = qvirtio_pci_device_find(vs->bus, VIRTIO_ID_SCSI);
     vs->dev = (QVirtioDevice *)dev;
     g_assert(dev != NULL);
-    g_assert_cmphex(vs->dev->device_type, ==, QVIRTIO_SCSI_DEVICE_ID);
+    g_assert_cmphex(vs->dev->device_type, ==, VIRTIO_ID_SCSI);
 
     qvirtio_pci_device_enable(dev);
     qvirtio_reset(&qvirtio_pci, vs->dev);
-- 
2.5.5

[Qemu-devel] [PULL 02/20] libqos: drop duplicated PCI vendor ID definition

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1462798061-30382-3-git-send-email-stefanha@redhat.com
---
 tests/libqos/virtio-pci.c | 3 ++-
 tests/libqos/virtio.h     | 2 --
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 9d45e20..dc867d7 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -16,6 +16,7 @@
 #include "libqos/malloc.h"
 #include "libqos/malloc-pc.h"
 
+#include "hw/pci/pci.h"
 #include "hw/pci/pci_regs.h"
 
 typedef struct QVirtioPCIForeachData {
@@ -263,7 +264,7 @@ void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
                                 .device_type = device_type,
                                 .user_data = data };
 
-    qpci_device_foreach(bus, QVIRTIO_VENDOR_ID, -1,
+    qpci_device_foreach(bus, PCI_VENDOR_ID_REDHAT_QUMRANET, -1,
                                 qvirtio_pci_foreach_callback, &d);
 }
 
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index af03793..e663bcf 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -12,8 +12,6 @@
 
 #include "libqos/malloc.h"
 
-#define QVIRTIO_VENDOR_ID       0x1AF4
-
 #define QVIRTIO_RESET           0x0
 #define QVIRTIO_ACKNOWLEDGE     0x1
 #define QVIRTIO_DRIVER          0x2
-- 
2.5.5

[Qemu-devel] [PULL 03/20] libqos: drop duplicated virtio_config.h definitions

[Stefan Hajnoczi]

Note that VIRTIO_F_ANY_LAYOUT and VIRTIO_F_NOTIFY_ON_EMPTY are bit
numbers in virtio_config.h but bit masks in qtest virtio.h.  Therefore
it's necessary to change users from X to (1u << X).

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1462798061-30382-4-git-send-email-stefanha@redhat.com
---
 tests/libqos/virtio.c   | 19 ++++++++++---------
 tests/libqos/virtio.h   |  9 ---------
 tests/virtio-blk-test.c |  6 ++++--
 3 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index d792635..e3f6194 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -10,6 +10,7 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "libqos/virtio.h"
+#include "standard-headers/linux/virtio_config.h"
 
 uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
                                                                 uint64_t addr)
@@ -54,28 +55,28 @@ QVirtQueue *qvirtqueue_setup(const QVirtioBus *bus, QVirtioDevice *d,
 
 void qvirtio_reset(const QVirtioBus *bus, QVirtioDevice *d)
 {
-    bus->set_status(d, QVIRTIO_RESET);
-    g_assert_cmphex(bus->get_status(d), ==, QVIRTIO_RESET);
+    bus->set_status(d, 0);
+    g_assert_cmphex(bus->get_status(d), ==, 0);
 }
 
 void qvirtio_set_acknowledge(const QVirtioBus *bus, QVirtioDevice *d)
 {
-    bus->set_status(d, bus->get_status(d) | QVIRTIO_ACKNOWLEDGE);
-    g_assert_cmphex(bus->get_status(d), ==, QVIRTIO_ACKNOWLEDGE);
+    bus->set_status(d, bus->get_status(d) | VIRTIO_CONFIG_S_ACKNOWLEDGE);
+    g_assert_cmphex(bus->get_status(d), ==, VIRTIO_CONFIG_S_ACKNOWLEDGE);
 }
 
 void qvirtio_set_driver(const QVirtioBus *bus, QVirtioDevice *d)
 {
-    bus->set_status(d, bus->get_status(d) | QVIRTIO_DRIVER);
+    bus->set_status(d, bus->get_status(d) | VIRTIO_CONFIG_S_DRIVER);
     g_assert_cmphex(bus->get_status(d), ==,
-                                    QVIRTIO_DRIVER | QVIRTIO_ACKNOWLEDGE);
+                    VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_ACKNOWLEDGE);
 }
 
 void qvirtio_set_driver_ok(const QVirtioBus *bus, QVirtioDevice *d)
 {
-    bus->set_status(d, bus->get_status(d) | QVIRTIO_DRIVER_OK);
-    g_assert_cmphex(bus->get_status(d), ==,
-                QVIRTIO_DRIVER_OK | QVIRTIO_DRIVER | QVIRTIO_ACKNOWLEDGE);
+    bus->set_status(d, bus->get_status(d) | VIRTIO_CONFIG_S_DRIVER_OK);
+    g_assert_cmphex(bus->get_status(d), ==, VIRTIO_CONFIG_S_DRIVER_OK |
+                    VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_ACKNOWLEDGE);
 }
 
 void qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice *d,
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index e663bcf..993ae0e 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -12,13 +12,6 @@
 
 #include "libqos/malloc.h"
 
-#define QVIRTIO_RESET           0x0
-#define QVIRTIO_ACKNOWLEDGE     0x1
-#define QVIRTIO_DRIVER          0x2
-#define QVIRTIO_DRIVER_OK       0x4
-
-#define QVIRTIO_F_NOTIFY_ON_EMPTY       0x01000000
-#define QVIRTIO_F_ANY_LAYOUT            0x08000000
 #define QVIRTIO_F_RING_INDIRECT_DESC    0x10000000
 #define QVIRTIO_F_RING_EVENT_IDX        0x20000000
 #define QVIRTIO_F_BAD_FEATURE           0x40000000
@@ -27,8 +20,6 @@
 #define QVRING_DESC_F_WRITE     0x2
 #define QVRING_DESC_F_INDIRECT  0x4
 
-#define QVIRTIO_F_NOTIFY_ON_EMPTY       0x01000000
-#define QVIRTIO_F_ANY_LAYOUT            0x08000000
 #define QVIRTIO_F_RING_INDIRECT_DESC    0x10000000
 #define QVIRTIO_F_RING_EVENT_IDX        0x20000000
 #define QVIRTIO_F_BAD_FEATURE           0x40000000
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index a0d2d83..84eb056 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -19,6 +19,7 @@
 #include "libqos/malloc-generic.h"
 #include "qemu/bswap.h"
 #include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_config.h"
 
 #define QVIRTIO_BLK_F_BARRIER       0x00000001
 #define QVIRTIO_BLK_F_SIZE_MAX      0x00000002
@@ -239,7 +240,7 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
 
     guest_free(alloc, req_addr);
 
-    if (features & QVIRTIO_F_ANY_LAYOUT) {
+    if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
         /* Write and read with 2 descriptor layout */
         /* Write request */
         req.type = QVIRTIO_BLK_T_OUT;
@@ -606,7 +607,8 @@ static void pci_idx(void)
     features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
                             QVIRTIO_F_RING_INDIRECT_DESC |
-                            QVIRTIO_F_NOTIFY_ON_EMPTY | QVIRTIO_BLK_F_SCSI);
+                            (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
+                            QVIRTIO_BLK_F_SCSI);
     qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
 
     vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
-- 
2.5.5

[Qemu-devel] [PULL 04/20] libqos: drop duplicated virtio_ring.h bit definitions

[Stefan Hajnoczi]

Note that virtio_ring.h defines feature bits using their bit number:

  #define VIRTIO_RING_F_INDIRECT_DESC     28

On the other hand libqos virtio.h uses the bit mask:

  #define QVIRTIO_F_RING_INDIRECT_DESC    0x10000000

The patch makes the necessary adjustments.

I have used "1u << BITMASK" instead of "1ULL << BITMASK" because the
64-bit feature fields are not implemented in libqos virtio.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1462798061-30382-5-git-send-email-stefanha@redhat.com
---
 tests/libqos/virtio-mmio.c |  5 +++--
 tests/libqos/virtio-pci.c  |  5 +++--
 tests/libqos/virtio.c      | 13 +++++++------
 tests/libqos/virtio.h      | 14 --------------
 tests/virtio-blk-test.c    | 18 +++++++++++-------
 tests/virtio-net-test.c    |  5 +++--
 6 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/tests/libqos/virtio-mmio.c b/tests/libqos/virtio-mmio.c
index e15b480..ccb92d9 100644
--- a/tests/libqos/virtio-mmio.c
+++ b/tests/libqos/virtio-mmio.c
@@ -13,6 +13,7 @@
 #include "libqos/virtio-mmio.h"
 #include "libqos/malloc.h"
 #include "libqos/malloc-generic.h"
+#include "standard-headers/linux/virtio_ring.h"
 
 static uint8_t qvirtio_mmio_config_readb(QVirtioDevice *d, uint64_t addr)
 {
@@ -135,8 +136,8 @@ static QVirtQueue *qvirtio_mmio_virtqueue_setup(QVirtioDevice *d,
     vq->free_head = 0;
     vq->num_free = vq->size;
     vq->align = dev->page_size;
-    vq->indirect = (dev->features & QVIRTIO_F_RING_INDIRECT_DESC) != 0;
-    vq->event = (dev->features & QVIRTIO_F_RING_EVENT_IDX) != 0;
+    vq->indirect = (dev->features & (1u << VIRTIO_RING_F_INDIRECT_DESC)) != 0;
+    vq->event = (dev->features & (1u << VIRTIO_RING_F_EVENT_IDX)) != 0;
 
     writel(dev->addr + QVIRTIO_MMIO_QUEUE_NUM, vq->size);
 
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index dc867d7..2418b7d 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -15,6 +15,7 @@
 #include "libqos/pci-pc.h"
 #include "libqos/malloc.h"
 #include "libqos/malloc-pc.h"
+#include "standard-headers/linux/virtio_ring.h"
 
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_regs.h"
@@ -212,8 +213,8 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     vqpci->vq.free_head = 0;
     vqpci->vq.num_free = vqpci->vq.size;
     vqpci->vq.align = QVIRTIO_PCI_ALIGN;
-    vqpci->vq.indirect = (feat & QVIRTIO_F_RING_INDIRECT_DESC) != 0;
-    vqpci->vq.event = (feat & QVIRTIO_F_RING_EVENT_IDX) != 0;
+    vqpci->vq.indirect = (feat & (1u << VIRTIO_RING_F_INDIRECT_DESC)) != 0;
+    vqpci->vq.event = (feat & (1u << VIRTIO_RING_F_EVENT_IDX)) != 0;
 
     vqpci->msix_entry = -1;
     vqpci->msix_addr = 0;
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index e3f6194..0a3e05b 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -11,6 +11,7 @@
 #include "libqtest.h"
 #include "libqos/virtio.h"
 #include "standard-headers/linux/virtio_config.h"
+#include "standard-headers/linux/virtio_ring.h"
 
 uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
                                                                 uint64_t addr)
@@ -172,7 +173,7 @@ QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
         /* indirect->desc[i].addr */
         writeq(indirect->desc + (16 * i), 0);
         /* indirect->desc[i].flags */
-        writew(indirect->desc + (16 * i) + 12, QVRING_DESC_F_NEXT);
+        writew(indirect->desc + (16 * i) + 12, VRING_DESC_F_NEXT);
         /* indirect->desc[i].next */
         writew(indirect->desc + (16 * i) + 14, i + 1);
     }
@@ -190,7 +191,7 @@ void qvring_indirect_desc_add(QVRingIndirectDesc *indirect, uint64_t data,
     flags = readw(indirect->desc + (16 * indirect->index) + 12);
 
     if (write) {
-        flags |= QVRING_DESC_F_WRITE;
+        flags |= VRING_DESC_F_WRITE;
     }
 
     /* indirect->desc[indirect->index].addr */
@@ -210,11 +211,11 @@ uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, bool write,
     vq->num_free--;
 
     if (write) {
-        flags |= QVRING_DESC_F_WRITE;
+        flags |= VRING_DESC_F_WRITE;
     }
 
     if (next) {
-        flags |= QVRING_DESC_F_NEXT;
+        flags |= VRING_DESC_F_NEXT;
     }
 
     /* vq->desc[vq->free_head].addr */
@@ -241,7 +242,7 @@ uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect)
     writel(vq->desc + (16 * vq->free_head) + 8,
                                         sizeof(QVRingDesc) * indirect->elem);
     /* vq->desc[vq->free_head].flags */
-    writew(vq->desc + (16 * vq->free_head) + 12, QVRING_DESC_F_INDIRECT);
+    writew(vq->desc + (16 * vq->free_head) + 12, VRING_DESC_F_INDIRECT);
 
     return vq->free_head++; /* Return and increase, in this order */
 }
@@ -267,7 +268,7 @@ void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
                                 (sizeof(struct QVRingUsedElem) * vq->size));
 
     /* < 1 because we add elements to avail queue one by one */
-    if ((flags & QVRING_USED_F_NO_NOTIFY) == 0 &&
+    if ((flags & VRING_USED_F_NO_NOTIFY) == 0 &&
                             (!vq->event || (uint16_t)(idx-avail_event) < 1)) {
         bus->virtqueue_kick(d, vq);
     }
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 993ae0e..2af8036 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -12,22 +12,8 @@
 
 #include "libqos/malloc.h"
 
-#define QVIRTIO_F_RING_INDIRECT_DESC    0x10000000
-#define QVIRTIO_F_RING_EVENT_IDX        0x20000000
 #define QVIRTIO_F_BAD_FEATURE           0x40000000
 
-#define QVRING_DESC_F_NEXT      0x1
-#define QVRING_DESC_F_WRITE     0x2
-#define QVRING_DESC_F_INDIRECT  0x4
-
-#define QVIRTIO_F_RING_INDIRECT_DESC    0x10000000
-#define QVIRTIO_F_RING_EVENT_IDX        0x20000000
-#define QVIRTIO_F_BAD_FEATURE           0x40000000
-
-#define QVRING_AVAIL_F_NO_INTERRUPT     1
-
-#define QVRING_USED_F_NO_NOTIFY     1
-
 typedef struct QVirtioDevice {
     /* Device type */
     uint16_t device_type;
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 84eb056..8efe3d6 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -20,6 +20,7 @@
 #include "qemu/bswap.h"
 #include "standard-headers/linux/virtio_ids.h"
 #include "standard-headers/linux/virtio_config.h"
+#include "standard-headers/linux/virtio_ring.h"
 
 #define QVIRTIO_BLK_F_BARRIER       0x00000001
 #define QVIRTIO_BLK_F_SIZE_MAX      0x00000002
@@ -183,7 +184,8 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
 
     features = qvirtio_get_features(bus, dev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
-                    QVIRTIO_F_RING_INDIRECT_DESC | QVIRTIO_F_RING_EVENT_IDX |
+                    (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+                    (1u << VIRTIO_RING_F_EVENT_IDX) |
                             QVIRTIO_BLK_F_SCSI);
     qvirtio_set_features(bus, dev, features);
 
@@ -349,9 +351,10 @@ static void pci_indirect(void)
     g_assert_cmpint(capacity, ==, TEST_IMAGE_SIZE / 512);
 
     features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
-    g_assert_cmphex(features & QVIRTIO_F_RING_INDIRECT_DESC, !=, 0);
-    features = features & ~(QVIRTIO_F_BAD_FEATURE | QVIRTIO_F_RING_EVENT_IDX |
-                                                            QVIRTIO_BLK_F_SCSI);
+    g_assert_cmphex(features & (1u << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
+    features = features & ~(QVIRTIO_F_BAD_FEATURE |
+                            (1u << VIRTIO_RING_F_EVENT_IDX) |
+                            QVIRTIO_BLK_F_SCSI);
     qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
 
     alloc = pc_alloc_init();
@@ -491,8 +494,9 @@ static void pci_msix(void)
 
     features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
-                            QVIRTIO_F_RING_INDIRECT_DESC |
-                            QVIRTIO_F_RING_EVENT_IDX | QVIRTIO_BLK_F_SCSI);
+                            (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+                            (1u << VIRTIO_RING_F_EVENT_IDX) |
+                            QVIRTIO_BLK_F_SCSI);
     qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
 
     vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
@@ -606,7 +610,7 @@ static void pci_idx(void)
 
     features = qvirtio_get_features(&qvirtio_pci, &dev->vdev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
-                            QVIRTIO_F_RING_INDIRECT_DESC |
+                            (1u << VIRTIO_RING_F_INDIRECT_DESC) |
                             (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
                             QVIRTIO_BLK_F_SCSI);
     qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index e49c13e..1712450 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -21,6 +21,7 @@
 #include "qemu/bswap.h"
 #include "hw/virtio/virtio-net.h"
 #include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_ring.h"
 
 #define PCI_SLOT_HP             0x06
 #define PCI_SLOT                0x04
@@ -70,8 +71,8 @@ static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
 
     features = qvirtio_get_features(bus, dev);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
-                            QVIRTIO_F_RING_INDIRECT_DESC |
-                            QVIRTIO_F_RING_EVENT_IDX);
+                            (1u << VIRTIO_RING_F_INDIRECT_DESC) |
+                            (1u << VIRTIO_RING_F_EVENT_IDX));
     qvirtio_set_features(bus, dev, features);
 
     qvirtio_set_driver_ok(bus, dev);
-- 
2.5.5

[Qemu-devel] [PULL 05/20] libqos: drop duplicated virtio_vring.h structs

[Stefan Hajnoczi]

The descriptor element, used, and avail vring structs are defined in
virtio_ring.h.  There is no need to duplicate them in libqos virtio.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1462798061-30382-6-git-send-email-stefanha@redhat.com
---
 tests/libqos/virtio.c | 10 +++++-----
 tests/libqos/virtio.h | 39 +++++++--------------------------------
 2 files changed, 12 insertions(+), 37 deletions(-)

diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index 0a3e05b..f79d2d1 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -135,7 +135,7 @@ void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr)
     int i;
 
     vq->desc = addr;
-    vq->avail = vq->desc + vq->size*sizeof(QVRingDesc);
+    vq->avail = vq->desc + vq->size * sizeof(struct vring_desc);
     vq->used = (uint64_t)((vq->avail + sizeof(uint16_t) * (3 + vq->size)
         + vq->align - 1) & ~(vq->align - 1));
 
@@ -156,7 +156,7 @@ void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr)
     /* vq->used->flags */
     writew(vq->used, 0);
     /* vq->used->avail_event */
-    writew(vq->used+2+(sizeof(struct QVRingUsedElem)*vq->size), 0);
+    writew(vq->used + 2 + sizeof(struct vring_used_elem) * vq->size, 0);
 }
 
 QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
@@ -167,7 +167,7 @@ QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
 
     indirect->index = 0;
     indirect->elem = elem;
-    indirect->desc = guest_alloc(alloc, sizeof(QVRingDesc)*elem);
+    indirect->desc = guest_alloc(alloc, sizeof(struct vring_desc) * elem);
 
     for (i = 0; i < elem - 1; ++i) {
         /* indirect->desc[i].addr */
@@ -240,7 +240,7 @@ uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect)
     writeq(vq->desc + (16 * vq->free_head), indirect->desc);
     /* vq->desc[vq->free_head].len */
     writel(vq->desc + (16 * vq->free_head) + 8,
-                                        sizeof(QVRingDesc) * indirect->elem);
+           sizeof(struct vring_desc) * indirect->elem);
     /* vq->desc[vq->free_head].flags */
     writew(vq->desc + (16 * vq->free_head) + 12, VRING_DESC_F_INDIRECT);
 
@@ -265,7 +265,7 @@ void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
     /* Must read after idx is updated */
     flags = readw(vq->avail);
     avail_event = readw(vq->used + 4 +
-                                (sizeof(struct QVRingUsedElem) * vq->size));
+                                sizeof(struct vring_used_elem) * vq->size);
 
     /* < 1 because we add elements to avail queue one by one */
     if ((flags & VRING_USED_F_NO_NOTIFY) == 0 &&
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 2af8036..c73fd8c 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -11,6 +11,7 @@
 #define LIBQOS_VIRTIO_H
 
 #include "libqos/malloc.h"
+#include "standard-headers/linux/virtio_ring.h"
 
 #define QVIRTIO_F_BAD_FEATURE           0x40000000
 
@@ -19,36 +20,10 @@ typedef struct QVirtioDevice {
     uint16_t device_type;
 } QVirtioDevice;
 
-typedef struct QVRingDesc {
-    uint64_t addr;
-    uint32_t len;
-    uint16_t flags;
-    uint16_t next;
-} QVRingDesc;
-
-typedef struct QVRingAvail {
-    uint16_t flags;
-    uint16_t idx;
-    uint16_t ring[0]; /* This is an array of uint16_t */
-    uint16_t used_event;
-} QVRingAvail;
-
-typedef struct QVRingUsedElem {
-    uint32_t id;
-    uint32_t len;
-} QVRingUsedElem;
-
-typedef struct QVRingUsed {
-    uint16_t flags;
-    uint16_t idx;
-    QVRingUsedElem ring[0]; /* This is an array of QVRingUsedElem structs */
-    uint16_t avail_event;
-} QVRingUsed;
-
 typedef struct QVirtQueue {
-    uint64_t desc; /* This points to an array of QVRingDesc */
-    uint64_t avail; /* This points to a QVRingAvail */
-    uint64_t used; /* This points to a QVRingDesc */
+    uint64_t desc; /* This points to an array of struct vring_desc */
+    uint64_t avail; /* This points to a struct vring_avail */
+    uint64_t used; /* This points to a struct vring_desc */
     uint16_t index;
     uint32_t size;
     uint32_t free_head;
@@ -59,7 +34,7 @@ typedef struct QVirtQueue {
 } QVirtQueue;
 
 typedef struct QVRingIndirectDesc {
-    uint64_t desc; /* This points to an array fo QVRingDesc */
+    uint64_t desc; /* This points to an array fo struct vring_desc */
     uint16_t index;
     uint16_t elem;
 } QVRingIndirectDesc;
@@ -110,9 +85,9 @@ typedef struct QVirtioBus {
 
 static inline uint32_t qvring_size(uint32_t num, uint32_t align)
 {
-    return ((sizeof(struct QVRingDesc) * num + sizeof(uint16_t) * (3 + num)
+    return ((sizeof(struct vring_desc) * num + sizeof(uint16_t) * (3 + num)
         + align - 1) & ~(align - 1))
-        + sizeof(uint16_t) * 3 + sizeof(struct QVRingUsedElem) * num;
+        + sizeof(uint16_t) * 3 + sizeof(struct vring_used_elem) * num;
 }
 
 uint8_t qvirtio_config_readb(const QVirtioBus *bus, QVirtioDevice *d,
-- 
2.5.5

[Qemu-devel] [PULL 06/20] libqos: drop duplicated virtio_blk.h definitions

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1462798061-30382-7-git-send-email-stefanha@redhat.com
---
 tests/virtio-blk-test.c | 50 ++++++++++++++++---------------------------------
 1 file changed, 16 insertions(+), 34 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 8efe3d6..3920038 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -21,25 +21,7 @@
 #include "standard-headers/linux/virtio_ids.h"
 #include "standard-headers/linux/virtio_config.h"
 #include "standard-headers/linux/virtio_ring.h"
-
-#define QVIRTIO_BLK_F_BARRIER       0x00000001
-#define QVIRTIO_BLK_F_SIZE_MAX      0x00000002
-#define QVIRTIO_BLK_F_SEG_MAX       0x00000004
-#define QVIRTIO_BLK_F_GEOMETRY      0x00000010
-#define QVIRTIO_BLK_F_RO            0x00000020
-#define QVIRTIO_BLK_F_BLK_SIZE      0x00000040
-#define QVIRTIO_BLK_F_SCSI          0x00000080
-#define QVIRTIO_BLK_F_WCE           0x00000200
-#define QVIRTIO_BLK_F_TOPOLOGY      0x00000400
-#define QVIRTIO_BLK_F_CONFIG_WCE    0x00000800
-
-#define QVIRTIO_BLK_T_IN            0
-#define QVIRTIO_BLK_T_OUT           1
-#define QVIRTIO_BLK_T_SCSI_CMD      2
-#define QVIRTIO_BLK_T_SCSI_CMD_OUT  3
-#define QVIRTIO_BLK_T_FLUSH         4
-#define QVIRTIO_BLK_T_FLUSH_OUT     5
-#define QVIRTIO_BLK_T_GET_ID        8
+#include "standard-headers/linux/virtio_blk.h"
 
 #define TEST_IMAGE_SIZE         (64 * 1024 * 1024)
 #define QVIRTIO_BLK_TIMEOUT_US  (30 * 1000 * 1000)
@@ -186,14 +168,14 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
                     (1u << VIRTIO_RING_F_INDIRECT_DESC) |
                     (1u << VIRTIO_RING_F_EVENT_IDX) |
-                            QVIRTIO_BLK_F_SCSI);
+                    (1u << VIRTIO_BLK_F_SCSI));
     qvirtio_set_features(bus, dev, features);
 
     qvirtio_set_driver_ok(bus, dev);
 
     /* Write and read with 3 descriptor layout */
     /* Write request */
-    req.type = QVIRTIO_BLK_T_OUT;
+    req.type = VIRTIO_BLK_T_OUT;
     req.ioprio = 1;
     req.sector = 0;
     req.data = g_malloc0(512);
@@ -216,7 +198,7 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
     guest_free(alloc, req_addr);
 
     /* Read request */
-    req.type = QVIRTIO_BLK_T_IN;
+    req.type = VIRTIO_BLK_T_IN;
     req.ioprio = 1;
     req.sector = 0;
     req.data = g_malloc0(512);
@@ -245,7 +227,7 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
     if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
         /* Write and read with 2 descriptor layout */
         /* Write request */
-        req.type = QVIRTIO_BLK_T_OUT;
+        req.type = VIRTIO_BLK_T_OUT;
         req.ioprio = 1;
         req.sector = 1;
         req.data = g_malloc0(512);
@@ -266,7 +248,7 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
         guest_free(alloc, req_addr);
 
         /* Read request */
-        req.type = QVIRTIO_BLK_T_IN;
+        req.type = VIRTIO_BLK_T_IN;
         req.ioprio = 1;
         req.sector = 1;
         req.data = g_malloc0(512);
@@ -354,7 +336,7 @@ static void pci_indirect(void)
     g_assert_cmphex(features & (1u << VIRTIO_RING_F_INDIRECT_DESC), !=, 0);
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
                             (1u << VIRTIO_RING_F_EVENT_IDX) |
-                            QVIRTIO_BLK_F_SCSI);
+                            (1u << VIRTIO_BLK_F_SCSI));
     qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
 
     alloc = pc_alloc_init();
@@ -363,7 +345,7 @@ static void pci_indirect(void)
     qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
 
     /* Write request */
-    req.type = QVIRTIO_BLK_T_OUT;
+    req.type = VIRTIO_BLK_T_OUT;
     req.ioprio = 1;
     req.sector = 0;
     req.data = g_malloc0(512);
@@ -388,7 +370,7 @@ static void pci_indirect(void)
     guest_free(alloc, req_addr);
 
     /* Read request */
-    req.type = QVIRTIO_BLK_T_IN;
+    req.type = VIRTIO_BLK_T_IN;
     req.ioprio = 1;
     req.sector = 0;
     req.data = g_malloc0(512);
@@ -496,7 +478,7 @@ static void pci_msix(void)
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
                             (1u << VIRTIO_RING_F_INDIRECT_DESC) |
                             (1u << VIRTIO_RING_F_EVENT_IDX) |
-                            QVIRTIO_BLK_F_SCSI);
+                            (1u << VIRTIO_BLK_F_SCSI));
     qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
 
     vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
@@ -515,7 +497,7 @@ static void pci_msix(void)
     g_assert_cmpint(capacity, ==, n_size / 512);
 
     /* Write request */
-    req.type = QVIRTIO_BLK_T_OUT;
+    req.type = VIRTIO_BLK_T_OUT;
     req.ioprio = 1;
     req.sector = 0;
     req.data = g_malloc0(512);
@@ -539,7 +521,7 @@ static void pci_msix(void)
     guest_free(alloc, req_addr);
 
     /* Read request */
-    req.type = QVIRTIO_BLK_T_IN;
+    req.type = VIRTIO_BLK_T_IN;
     req.ioprio = 1;
     req.sector = 0;
     req.data = g_malloc0(512);
@@ -612,7 +594,7 @@ static void pci_idx(void)
     features = features & ~(QVIRTIO_F_BAD_FEATURE |
                             (1u << VIRTIO_RING_F_INDIRECT_DESC) |
                             (1u << VIRTIO_F_NOTIFY_ON_EMPTY) |
-                            QVIRTIO_BLK_F_SCSI);
+                            (1u << VIRTIO_BLK_F_SCSI));
     qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
 
     vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
@@ -622,7 +604,7 @@ static void pci_idx(void)
     qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
 
     /* Write request */
-    req.type = QVIRTIO_BLK_T_OUT;
+    req.type = VIRTIO_BLK_T_OUT;
     req.ioprio = 1;
     req.sector = 0;
     req.data = g_malloc0(512);
@@ -641,7 +623,7 @@ static void pci_idx(void)
                            QVIRTIO_BLK_TIMEOUT_US);
 
     /* Write request */
-    req.type = QVIRTIO_BLK_T_OUT;
+    req.type = VIRTIO_BLK_T_OUT;
     req.ioprio = 1;
     req.sector = 1;
     req.data = g_malloc0(512);
@@ -667,7 +649,7 @@ static void pci_idx(void)
     guest_free(alloc, req_addr);
 
     /* Read request */
-    req.type = QVIRTIO_BLK_T_IN;
+    req.type = VIRTIO_BLK_T_IN;
     req.ioprio = 1;
     req.sector = 1;
     req.data = g_malloc0(512);
-- 
2.5.5

[Qemu-devel] [PULL 07/20] libqos: drop duplicated virtio_scsi.h definitions

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1462798061-30382-8-git-send-email-stefanha@redhat.com
---
 tests/virtio-scsi-test.c | 45 +++++++++++++++------------------------------
 1 file changed, 15 insertions(+), 30 deletions(-)

diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 26bb0e3..be0f760 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -18,11 +18,11 @@
 #include "libqos/malloc-pc.h"
 #include "libqos/malloc-generic.h"
 #include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_scsi.h"
 
 #define PCI_SLOT                0x02
 #define PCI_FN                  0x00
 #define QVIRTIO_SCSI_TIMEOUT_US (1 * 1000 * 1000)
-#define CDB_SIZE 32
 
 #define MAX_NUM_QUEUES 64
 
@@ -34,24 +34,6 @@ typedef struct {
     QVirtQueue *vq[MAX_NUM_QUEUES + 2];
 } QVirtIOSCSI;
 
-typedef struct {
-    uint8_t lun[8];
-    int64_t tag;
-    uint8_t task_attr;
-    uint8_t prio;
-    uint8_t crn;
-    uint8_t cdb[CDB_SIZE];
-} QEMU_PACKED QVirtIOSCSICmdReq;
-
-typedef struct {
-    uint32_t sense_len;
-    uint32_t resid;
-    uint16_t status_qualifier;
-    uint8_t status;
-    uint8_t response;
-    uint8_t sense[96];
-} QEMU_PACKED QVirtIOSCSICmdResp;
-
 static void qvirtio_scsi_start(const char *extra_opts)
 {
     char *cmdline;
@@ -100,11 +82,11 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
                                       const uint8_t *data_in,
                                       size_t data_in_len,
                                       uint8_t *data_out, size_t data_out_len,
-                                      QVirtIOSCSICmdResp *resp_out)
+                                      struct virtio_scsi_cmd_resp *resp_out)
 {
     QVirtQueue *vq;
-    QVirtIOSCSICmdReq req = { { 0 } };
-    QVirtIOSCSICmdResp resp = { .response = 0xff, .status = 0xff };
+    struct virtio_scsi_cmd_req req = { { 0 } };
+    struct virtio_scsi_cmd_resp resp = { .response = 0xff, .status = 0xff };
     uint64_t req_addr, resp_addr, data_in_addr = 0, data_out_addr = 0;
     uint8_t response;
     uint32_t free_head;
@@ -113,7 +95,7 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
 
     req.lun[0] = 1; /* Select LUN */
     req.lun[1] = 1; /* Select target 1 */
-    memcpy(req.cdb, cdb, CDB_SIZE);
+    memcpy(req.cdb, cdb, VIRTIO_SCSI_CDB_SIZE);
 
     /* XXX: Fix endian if any multi-byte field in req/resp is used */
 
@@ -138,7 +120,8 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
     qvirtqueue_kick(&qvirtio_pci, vs->dev, vq, free_head);
     qvirtio_wait_queue_isr(&qvirtio_pci, vs->dev, vq, QVIRTIO_SCSI_TIMEOUT_US);
 
-    response = readb(resp_addr + offsetof(QVirtIOSCSICmdResp, response));
+    response = readb(resp_addr +
+                     offsetof(struct virtio_scsi_cmd_resp, response));
 
     if (resp_out) {
         memread(resp_addr, resp_out, sizeof(*resp_out));
@@ -153,10 +136,10 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
 
 static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
 {
-    const uint8_t test_unit_ready_cdb[CDB_SIZE] = {};
+    const uint8_t test_unit_ready_cdb[VIRTIO_SCSI_CDB_SIZE] = {};
     QVirtIOSCSI *vs;
     QVirtioPCIDevice *dev;
-    QVirtIOSCSICmdResp resp;
+    struct virtio_scsi_cmd_resp resp;
     void *addr;
     int i;
 
@@ -239,10 +222,12 @@ static void test_unaligned_write_same(void)
     QVirtIOSCSI *vs;
     uint8_t buf1[512] = { 0 };
     uint8_t buf2[512] = { 1 };
-    const uint8_t write_same_cdb_1[CDB_SIZE] = { 0x41, 0x00, 0x00, 0x00, 0x00,
-                                               0x01, 0x00, 0x00, 0x02, 0x00 };
-    const uint8_t write_same_cdb_2[CDB_SIZE] = { 0x41, 0x00, 0x00, 0x00, 0x00,
-                                               0x01, 0x00, 0x33, 0x00, 0x00 };
+    const uint8_t write_same_cdb_1[VIRTIO_SCSI_CDB_SIZE] = {
+        0x41, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x02, 0x00
+    };
+    const uint8_t write_same_cdb_2[VIRTIO_SCSI_CDB_SIZE] = {
+        0x41, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x33, 0x00, 0x00
+    };
 
     qvirtio_scsi_start("-drive file=blkdebug::null-co://,if=none,id=dr1"
                        ",format=raw,file.align=4k "
-- 
2.5.5

[Qemu-devel] [PULL 08/20] libqos: drop duplicated virtio_pci.h definitions

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1462798061-30382-9-git-send-email-stefanha@redhat.com
---
 tests/libqos/virtio-pci.c | 42 ++++++++++++++++++++++--------------------
 tests/libqos/virtio-pci.h | 17 -----------------
 tests/virtio-blk-test.c   | 11 ++++++-----
 tests/virtio-scsi-test.c  |  3 ++-
 4 files changed, 30 insertions(+), 43 deletions(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 2418b7d..3b2b59b 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -16,6 +16,7 @@
 #include "libqos/malloc.h"
 #include "libqos/malloc-pc.h"
 #include "standard-headers/linux/virtio_ring.h"
+#include "standard-headers/linux/virtio_pci.h"
 
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_regs.h"
@@ -103,31 +104,31 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
 static uint32_t qvirtio_pci_get_features(QVirtioDevice *d)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    return qpci_io_readl(dev->pdev, dev->addr + QVIRTIO_PCI_DEVICE_FEATURES);
+    return qpci_io_readl(dev->pdev, dev->addr + VIRTIO_PCI_HOST_FEATURES);
 }
 
 static void qvirtio_pci_set_features(QVirtioDevice *d, uint32_t features)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_PCI_GUEST_FEATURES, features);
+    qpci_io_writel(dev->pdev, dev->addr + VIRTIO_PCI_GUEST_FEATURES, features);
 }
 
 static uint32_t qvirtio_pci_get_guest_features(QVirtioDevice *d)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    return qpci_io_readl(dev->pdev, dev->addr + QVIRTIO_PCI_GUEST_FEATURES);
+    return qpci_io_readl(dev->pdev, dev->addr + VIRTIO_PCI_GUEST_FEATURES);
 }
 
 static uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_DEVICE_STATUS);
+    return qpci_io_readb(dev->pdev, dev->addr + VIRTIO_PCI_STATUS);
 }
 
 static void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t status)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_PCI_DEVICE_STATUS, status);
+    qpci_io_writeb(dev->pdev, dev->addr + VIRTIO_PCI_STATUS, status);
 }
 
 static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
@@ -151,7 +152,7 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
             }
         }
     } else {
-        return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_ISR_STATUS) & 1;
+        return qpci_io_readb(dev->pdev, dev->addr + VIRTIO_PCI_ISR) & 1;
     }
 }
 
@@ -175,26 +176,26 @@ static bool qvirtio_pci_get_config_isr_status(QVirtioDevice *d)
             }
         }
     } else {
-        return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_PCI_ISR_STATUS) & 2;
+        return qpci_io_readb(dev->pdev, dev->addr + VIRTIO_PCI_ISR) & 2;
     }
 }
 
 static void qvirtio_pci_queue_select(QVirtioDevice *d, uint16_t index)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_SELECT, index);
+    qpci_io_writeb(dev->pdev, dev->addr + VIRTIO_PCI_QUEUE_SEL, index);
 }
 
 static uint16_t qvirtio_pci_get_queue_size(QVirtioDevice *d)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    return qpci_io_readw(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_SIZE);
+    return qpci_io_readw(dev->pdev, dev->addr + VIRTIO_PCI_QUEUE_NUM);
 }
 
 static void qvirtio_pci_set_queue_address(QVirtioDevice *d, uint32_t pfn)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_ADDRESS, pfn);
+    qpci_io_writel(dev->pdev, dev->addr + VIRTIO_PCI_QUEUE_PFN, pfn);
 }
 
 static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
@@ -212,7 +213,7 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     vqpci->vq.size = qvirtio_pci_get_queue_size(d);
     vqpci->vq.free_head = 0;
     vqpci->vq.num_free = vqpci->vq.size;
-    vqpci->vq.align = QVIRTIO_PCI_ALIGN;
+    vqpci->vq.align = VIRTIO_PCI_VRING_ALIGN;
     vqpci->vq.indirect = (feat & (1u << VIRTIO_RING_F_INDIRECT_DESC)) != 0;
     vqpci->vq.event = (feat & (1u << VIRTIO_RING_F_EVENT_IDX)) != 0;
 
@@ -226,9 +227,10 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     /* Check power of 2 */
     g_assert_cmpint(vqpci->vq.size & (vqpci->vq.size - 1), ==, 0);
 
-    addr = guest_alloc(alloc, qvring_size(vqpci->vq.size, QVIRTIO_PCI_ALIGN));
+    addr = guest_alloc(alloc, qvring_size(vqpci->vq.size,
+                                          VIRTIO_PCI_VRING_ALIGN));
     qvring_init(alloc, &vqpci->vq, addr);
-    qvirtio_pci_set_queue_address(d, vqpci->vq.desc / QVIRTIO_PCI_ALIGN);
+    qvirtio_pci_set_queue_address(d, vqpci->vq.desc / VIRTIO_PCI_VRING_ALIGN);
 
     return &vqpci->vq;
 }
@@ -236,7 +238,7 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
 static void qvirtio_pci_virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    qpci_io_writew(dev->pdev, dev->addr + QVIRTIO_PCI_QUEUE_NOTIFY, vq->index);
+    qpci_io_writew(dev->pdev, dev->addr + VIRTIO_PCI_QUEUE_NOTIFY, vq->index);
 }
 
 const QVirtioBus qvirtio_pci = {
@@ -316,9 +318,9 @@ void qvirtqueue_pci_msix_setup(QVirtioPCIDevice *d, QVirtQueuePCI *vqpci,
                                         control & ~PCI_MSIX_ENTRY_CTRL_MASKBIT);
 
     qvirtio_pci_queue_select(&d->vdev, vqpci->vq.index);
-    qpci_io_writew(d->pdev, d->addr + QVIRTIO_PCI_MSIX_QUEUE_VECTOR, entry);
-    vector = qpci_io_readw(d->pdev, d->addr + QVIRTIO_PCI_MSIX_QUEUE_VECTOR);
-    g_assert_cmphex(vector, !=, QVIRTIO_MSI_NO_VECTOR);
+    qpci_io_writew(d->pdev, d->addr + VIRTIO_MSI_QUEUE_VECTOR, entry);
+    vector = qpci_io_readw(d->pdev, d->addr + VIRTIO_MSI_QUEUE_VECTOR);
+    g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
 }
 
 void qvirtio_pci_set_msix_configuration_vector(QVirtioPCIDevice *d,
@@ -348,7 +350,7 @@ void qvirtio_pci_set_msix_configuration_vector(QVirtioPCIDevice *d,
     qpci_io_writel(d->pdev, addr + PCI_MSIX_ENTRY_VECTOR_CTRL,
                                         control & ~PCI_MSIX_ENTRY_CTRL_MASKBIT);
 
-    qpci_io_writew(d->pdev, d->addr + QVIRTIO_PCI_MSIX_CONF_VECTOR, entry);
-    vector = qpci_io_readw(d->pdev, d->addr + QVIRTIO_PCI_MSIX_CONF_VECTOR);
-    g_assert_cmphex(vector, !=, QVIRTIO_MSI_NO_VECTOR);
+    qpci_io_writew(d->pdev, d->addr + VIRTIO_MSI_CONFIG_VECTOR, entry);
+    vector = qpci_io_readw(d->pdev, d->addr + VIRTIO_MSI_CONFIG_VECTOR);
+    g_assert_cmphex(vector, !=, VIRTIO_MSI_NO_VECTOR);
 }
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 8f0e52a..efcac2d 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -13,23 +13,6 @@
 #include "libqos/virtio.h"
 #include "libqos/pci.h"
 
-#define QVIRTIO_PCI_DEVICE_FEATURES         0x00
-#define QVIRTIO_PCI_GUEST_FEATURES          0x04
-#define QVIRTIO_PCI_QUEUE_ADDRESS           0x08
-#define QVIRTIO_PCI_QUEUE_SIZE              0x0C
-#define QVIRTIO_PCI_QUEUE_SELECT            0x0E
-#define QVIRTIO_PCI_QUEUE_NOTIFY            0x10
-#define QVIRTIO_PCI_DEVICE_STATUS           0x12
-#define QVIRTIO_PCI_ISR_STATUS              0x13
-#define QVIRTIO_PCI_MSIX_CONF_VECTOR        0x14
-#define QVIRTIO_PCI_MSIX_QUEUE_VECTOR       0x16
-#define QVIRTIO_PCI_DEVICE_SPECIFIC_MSIX    0x18
-#define QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX 0x14
-
-#define QVIRTIO_PCI_ALIGN   4096
-
-#define QVIRTIO_MSI_NO_VECTOR   0xFFFF
-
 typedef struct QVirtioPCIDevice {
     QVirtioDevice vdev;
     QPCIDevice *pdev;
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 3920038..9d27be5 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -22,6 +22,7 @@
 #include "standard-headers/linux/virtio_config.h"
 #include "standard-headers/linux/virtio_ring.h"
 #include "standard-headers/linux/virtio_blk.h"
+#include "standard-headers/linux/virtio_pci.h"
 
 #define TEST_IMAGE_SIZE         (64 * 1024 * 1024)
 #define QVIRTIO_BLK_TIMEOUT_US  (30 * 1000 * 1000)
@@ -291,7 +292,7 @@ static void pci_basic(void)
                                                                     alloc, 0);
 
     /* MSI-X is not enabled */
-    addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
+    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
 
     test_basic(&qvirtio_pci, &dev->vdev, alloc, &vqpci->vq,
                                                     (uint64_t)(uintptr_t)addr);
@@ -326,7 +327,7 @@ static void pci_indirect(void)
     dev = virtio_blk_pci_init(bus, PCI_SLOT);
 
     /* MSI-X is not enabled */
-    addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
+    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
 
     capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
                                                     (uint64_t)(uintptr_t)addr);
@@ -421,7 +422,7 @@ static void pci_config(void)
     dev = virtio_blk_pci_init(bus, PCI_SLOT);
 
     /* MSI-X is not enabled */
-    addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
+    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
 
     capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
                                                     (uint64_t)(uintptr_t)addr);
@@ -468,7 +469,7 @@ static void pci_msix(void)
     qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
 
     /* MSI-X is enabled */
-    addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_MSIX;
+    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
 
     capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
                                                     (uint64_t)(uintptr_t)addr);
@@ -584,7 +585,7 @@ static void pci_idx(void)
     qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
 
     /* MSI-X is enabled */
-    addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_MSIX;
+    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
 
     capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev,
                                                     (uint64_t)(uintptr_t)addr);
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index be0f760..3938163 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -18,6 +18,7 @@
 #include "libqos/malloc-pc.h"
 #include "libqos/malloc-generic.h"
 #include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_pci.h"
 #include "standard-headers/linux/virtio_scsi.h"
 
 #define PCI_SLOT                0x02
@@ -157,7 +158,7 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
     qvirtio_set_acknowledge(&qvirtio_pci, vs->dev);
     qvirtio_set_driver(&qvirtio_pci, vs->dev);
 
-    addr = dev->addr + QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX;
+    addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
     vs->num_queues = qvirtio_config_readl(&qvirtio_pci, vs->dev,
                                           (uint64_t)(uintptr_t)addr);
 
-- 
2.5.5

[Qemu-devel] [PULL 09/20] libqos: add qvirtqueue_cleanup()

[Stefan Hajnoczi]

qvirtqueue_setup() allocates the vring and virtqueue state.  So far
there has been no function to free it.  Callers have been using
guest_free() for the vring but forgot to free the QVirtQueue state.

This patch solves the memory leak by introducing qvirtqueue_cleanup().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqos/virtio-mmio.c |  8 ++++++++
 tests/libqos/virtio-pci.c  | 10 ++++++++++
 tests/libqos/virtio.c      |  6 ++++++
 tests/libqos/virtio.h      |  5 +++++
 tests/virtio-blk-test.c    | 10 +++++-----
 tests/virtio-net-test.c    |  2 +-
 tests/virtio-scsi-test.c   |  2 +-
 7 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/tests/libqos/virtio-mmio.c b/tests/libqos/virtio-mmio.c
index ccb92d9..0cab38f 100644
--- a/tests/libqos/virtio-mmio.c
+++ b/tests/libqos/virtio-mmio.c
@@ -154,6 +154,13 @@ static QVirtQueue *qvirtio_mmio_virtqueue_setup(QVirtioDevice *d,
     return vq;
 }
 
+static void qvirtio_mmio_virtqueue_cleanup(QVirtQueue *vq,
+                                           QGuestAllocator *alloc)
+{
+    guest_free(alloc, vq->desc);
+    g_free(vq);
+}
+
 static void qvirtio_mmio_virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
 {
     QVirtioMMIODevice *dev = (QVirtioMMIODevice *)d;
@@ -176,6 +183,7 @@ const QVirtioBus qvirtio_mmio = {
     .get_queue_size = qvirtio_mmio_get_queue_size,
     .set_queue_address = qvirtio_mmio_set_queue_address,
     .virtqueue_setup = qvirtio_mmio_virtqueue_setup,
+    .virtqueue_cleanup = qvirtio_mmio_virtqueue_cleanup,
     .virtqueue_kick = qvirtio_mmio_virtqueue_kick,
 };
 
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 3b2b59b..18b92b9 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -235,6 +235,15 @@ static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
     return &vqpci->vq;
 }
 
+static void qvirtio_pci_virtqueue_cleanup(QVirtQueue *vq,
+                                          QGuestAllocator *alloc)
+{
+    QVirtQueuePCI *vqpci = container_of(vq, QVirtQueuePCI, vq);
+
+    guest_free(alloc, vq->desc);
+    g_free(vqpci);
+}
+
 static void qvirtio_pci_virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
@@ -257,6 +266,7 @@ const QVirtioBus qvirtio_pci = {
     .get_queue_size = qvirtio_pci_get_queue_size,
     .set_queue_address = qvirtio_pci_set_queue_address,
     .virtqueue_setup = qvirtio_pci_virtqueue_setup,
+    .virtqueue_cleanup = qvirtio_pci_virtqueue_cleanup,
     .virtqueue_kick = qvirtio_pci_virtqueue_kick,
 };
 
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index f79d2d1..d8c2970 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -54,6 +54,12 @@ QVirtQueue *qvirtqueue_setup(const QVirtioBus *bus, QVirtioDevice *d,
     return bus->virtqueue_setup(d, alloc, index);
 }
 
+void qvirtqueue_cleanup(const QVirtioBus *bus, QVirtQueue *vq,
+                        QGuestAllocator *alloc)
+{
+    return bus->virtqueue_cleanup(vq, alloc);
+}
+
 void qvirtio_reset(const QVirtioBus *bus, QVirtioDevice *d)
 {
     bus->set_status(d, 0);
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index c73fd8c..0250842 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -79,6 +79,9 @@ typedef struct QVirtioBus {
     QVirtQueue *(*virtqueue_setup)(QVirtioDevice *d, QGuestAllocator *alloc,
                                                                 uint16_t index);
 
+    /* Free virtqueue resources */
+    void (*virtqueue_cleanup)(QVirtQueue *vq, QGuestAllocator *alloc);
+
     /* Notify changes in virtqueue */
     void (*virtqueue_kick)(QVirtioDevice *d, QVirtQueue *vq);
 } QVirtioBus;
@@ -118,6 +121,8 @@ void qvirtio_wait_config_isr(const QVirtioBus *bus, QVirtioDevice *d,
                              gint64 timeout_us);
 QVirtQueue *qvirtqueue_setup(const QVirtioBus *bus, QVirtioDevice *d,
                                         QGuestAllocator *alloc, uint16_t index);
+void qvirtqueue_cleanup(const QVirtioBus *bus, QVirtQueue *vq,
+                        QGuestAllocator *alloc);
 
 void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr);
 QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 9d27be5..4ab14d5 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -298,7 +298,7 @@ static void pci_basic(void)
                                                     (uint64_t)(uintptr_t)addr);
 
     /* End test */
-    guest_free(alloc, vqpci->vq.desc);
+    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
     pc_alloc_uninit(alloc);
     qvirtio_pci_device_disable(dev);
     g_free(dev);
@@ -401,7 +401,7 @@ static void pci_indirect(void)
     guest_free(alloc, req_addr);
 
     /* End test */
-    guest_free(alloc, vqpci->vq.desc);
+    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
     pc_alloc_uninit(alloc);
     qvirtio_pci_device_disable(dev);
     g_free(dev);
@@ -552,7 +552,7 @@ static void pci_msix(void)
     guest_free(alloc, req_addr);
 
     /* End test */
-    guest_free(alloc, vqpci->vq.desc);
+    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
     pc_alloc_uninit(alloc);
     qpci_msix_disable(dev->pdev);
     qvirtio_pci_device_disable(dev);
@@ -679,7 +679,7 @@ static void pci_idx(void)
     guest_free(alloc, req_addr);
 
     /* End test */
-    guest_free(alloc, vqpci->vq.desc);
+    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
     pc_alloc_uninit(alloc);
     qpci_msix_disable(dev->pdev);
     qvirtio_pci_device_disable(dev);
@@ -745,7 +745,7 @@ static void mmio_basic(void)
     g_assert_cmpint(capacity, ==, n_size / 512);
 
     /* End test */
-    guest_free(alloc, vq->desc);
+    qvirtqueue_cleanup(&qvirtio_mmio, vq, alloc);
     generic_alloc_uninit(alloc);
     g_free(dev);
     test_end();
diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 1712450..0d2c63f 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -229,7 +229,7 @@ static void pci_basic(gconstpointer data)
 
     /* End test */
     close(sv[0]);
-    guest_free(alloc, tx->vq.desc);
+    qvirtqueue_cleanup(&qvirtio_pci, &tx->vq, alloc);
     pc_alloc_uninit(alloc);
     qvirtio_pci_device_disable(dev);
     g_free(dev);
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 3938163..cbe5dcc 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -58,7 +58,7 @@ static void qvirtio_scsi_pci_free(QVirtIOSCSI *vs)
     int i;
 
     for (i = 0; i < vs->num_queues + 2; i++) {
-        guest_free(vs->alloc, vs->vq[i]->desc);
+        qvirtqueue_cleanup(&qvirtio_pci, vs->vq[i], vs->alloc);
     }
     pc_alloc_uninit(vs->alloc);
     qvirtio_pci_device_disable(container_of(vs->dev, QVirtioPCIDevice, vdev));
-- 
2.5.5

[Qemu-devel] [PULL 10/20] block: fixed BdrvTrackedRequest filling in bdrv_co_discard

[Stefan Hajnoczi]

From: "Denis V. Lunev" <den@openvz.org>

The request area is specified in bytes, not in sectors.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1466093381-6120-2-git-send-email-den@openvz.org
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/io.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/io.c b/block/io.c
index ebdb9d8..3527d91 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2347,8 +2347,8 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
         return 0;
     }
 
-    tracked_request_begin(&req, bs, sector_num, nb_sectors,
-                          BDRV_TRACKED_DISCARD);
+    tracked_request_begin(&req, bs, sector_num << BDRV_SECTOR_BITS,
+                          nb_sectors << BDRV_SECTOR_BITS, BDRV_TRACKED_DISCARD);
     bdrv_set_dirty(bs, sector_num, nb_sectors);
 
     max_discard = MIN_NON_ZERO(bs->bl.max_discard, BDRV_REQUEST_MAX_SECTORS);
-- 
2.5.5

[Qemu-devel] [PULL 11/20] block: fix race in bdrv_co_discard with drive-mirror

[Stefan Hajnoczi]

From: "Denis V. Lunev" <den@openvz.org>

Actually we must set dirty bitmap dirty after we have written all our
zeroes for correct processing in drive mirror code. In the other case
we can face not zeroes in this area in mirror_iteration.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1466093381-6120-3-git-send-email-den@openvz.org
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/io.c b/block/io.c
index 3527d91..8ab6f95 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2349,7 +2349,6 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
 
     tracked_request_begin(&req, bs, sector_num << BDRV_SECTOR_BITS,
                           nb_sectors << BDRV_SECTOR_BITS, BDRV_TRACKED_DISCARD);
-    bdrv_set_dirty(bs, sector_num, nb_sectors);
 
     max_discard = MIN_NON_ZERO(bs->bl.max_discard, BDRV_REQUEST_MAX_SECTORS);
     while (nb_sectors > 0) {
@@ -2398,6 +2397,8 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
     }
     ret = 0;
 out:
+    bdrv_set_dirty(bs, req.offset >> BDRV_SECTOR_BITS,
+                   req.bytes >> BDRV_SECTOR_BITS);
     tracked_request_end(&req);
     return ret;
 }
-- 
2.5.5

[Qemu-devel] [PULL 12/20] block: process before_write_notifiers in bdrv_co_discard

[Stefan Hajnoczi]

From: "Denis V. Lunev" <den@openvz.org>

This is mandatory for correct backup creation. In the other case the
content under this area would be lost.

Dirty bits are set exactly like in bdrv_aligned_pwritev, i.e. they are set
even if notifier has returned a error.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1466093381-6120-4-git-send-email-den@openvz.org
CC: Fam Zheng <famz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/io.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/io.c b/block/io.c
index 8ab6f95..7cf3645 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2350,6 +2350,11 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
     tracked_request_begin(&req, bs, sector_num << BDRV_SECTOR_BITS,
                           nb_sectors << BDRV_SECTOR_BITS, BDRV_TRACKED_DISCARD);
 
+    ret = notifier_with_return_list_notify(&bs->before_write_notifiers, &req);
+    if (ret < 0) {
+        goto out;
+    }
+
     max_discard = MIN_NON_ZERO(bs->bl.max_discard, BDRV_REQUEST_MAX_SECTORS);
     while (nb_sectors > 0) {
         int ret;
-- 
2.5.5

[Qemu-devel] [PULL 13/20] blockjob: move iostatus reset out of block_job_enter()

[Stefan Hajnoczi]

The QMP block-job-resume command and cancellation may want to reset the
job's iostatus.  The next patches add a user who does not want to reset
iostatus so move it up to block_job_enter() callers.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 1466096189-6477-2-git-send-email-stefanha@redhat.com
---
 blockdev.c | 1 +
 blockjob.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index c9a0068..7299312 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3811,6 +3811,7 @@ void qmp_block_job_resume(const char *device, Error **errp)
 
     job->user_paused = false;
     trace_qmp_block_job_resume(job);
+    block_job_iostatus_reset(job);
     block_job_resume(job);
     aio_context_release(aio_context);
 }
diff --git a/blockjob.c b/blockjob.c
index 01b896b..5137dce 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -269,7 +269,6 @@ void block_job_resume(BlockJob *job)
 
 void block_job_enter(BlockJob *job)
 {
-    block_job_iostatus_reset(job);
     if (job->co && !job->busy) {
         qemu_coroutine_enter(job->co, NULL);
     }
@@ -278,6 +277,7 @@ void block_job_enter(BlockJob *job)
 void block_job_cancel(BlockJob *job)
 {
     job->cancelled = true;
+    block_job_iostatus_reset(job);
     block_job_enter(job);
 }
 
-- 
2.5.5

[Qemu-devel] [PULL 14/20] blockjob: rename block_job_is_paused()

[Stefan Hajnoczi]

The block_job_is_paused() function name is not great because callers
only use it to determine whether pausing has been requested.  Rename it
to highlight those semantics and remove it from the public header file
as there are no external callers.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 1466096189-6477-3-git-send-email-stefanha@redhat.com
---
 blockjob.c               | 6 +++---
 include/block/blockjob.h | 9 ---------
 2 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 5137dce..21cc3ee 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -252,7 +252,7 @@ void block_job_pause(BlockJob *job)
     job->pause_count++;
 }
 
-bool block_job_is_paused(BlockJob *job)
+static bool block_job_should_pause(BlockJob *job)
 {
     return job->pause_count > 0;
 }
@@ -361,11 +361,11 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
     }
 
     job->busy = false;
-    if (!block_job_is_paused(job)) {
+    if (!block_job_should_pause(job)) {
         co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns);
     }
     /* The job can be paused while sleeping, so check this again */
-    if (block_job_is_paused(job)) {
+    if (block_job_should_pause(job)) {
         qemu_coroutine_yield();
     }
     job->busy = true;
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 00ac418..8fcecf9 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -348,15 +348,6 @@ void block_job_event_completed(BlockJob *job, const char *msg);
 void block_job_event_ready(BlockJob *job);
 
 /**
- * block_job_is_paused:
- * @job: The job being queried.
- *
- * Returns whether the job is currently paused, or will pause
- * as soon as it reaches a sleeping point.
- */
-bool block_job_is_paused(BlockJob *job);
-
-/**
  * block_job_cancel_sync:
  * @job: The job to be canceled.
  *
-- 
2.5.5

[Qemu-devel] [PULL 15/20] blockjob: add pause points

[Stefan Hajnoczi]

Block jobs are coroutines that usually perform I/O but sometimes also
sleep or yield.  Currently only sleeping or yielded block jobs can be
paused.  This means jobs that do not sleep or yield (using
block_job_yield()) are unaffected by block_job_pause().

Add block_job_pause_point() so that block jobs can mark quiescent points
that are suitable for pausing.  This solves the problem that it can take
a block job a long time to pause if it is performing a long series of
I/O operations.

Transitioning to paused state involves a .pause()/.resume() callback.
These callbacks are used to ensure that I/O and event loop activity has
ceased while the job is at a pause point.

Note that this patch introduces a stricter pause state than previously.
The job->busy flag was incorrectly documented as a quiescent state
without I/O pending.  This is violated by any job that has I/O pending
across sleep or block_job_yield(), like the mirror block job.

[Add missing block_job_should_pause() check to avoid deadlock after
job->driver->pause() in block_job_pause_point().
--Stefan]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 1466096189-6477-4-git-send-email-stefanha@redhat.com
---
 blockjob.c               | 38 +++++++++++++++++++++++++++++++++-----
 include/block/blockjob.h | 35 ++++++++++++++++++++++++++++++++---
 2 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 21cc3ee..874b573 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -257,6 +257,32 @@ static bool block_job_should_pause(BlockJob *job)
     return job->pause_count > 0;
 }
 
+void coroutine_fn block_job_pause_point(BlockJob *job)
+{
+    if (!block_job_should_pause(job)) {
+        return;
+    }
+    if (block_job_is_cancelled(job)) {
+        return;
+    }
+
+    if (job->driver->pause) {
+        job->driver->pause(job);
+    }
+
+    if (block_job_should_pause(job) && !block_job_is_cancelled(job)) {
+        job->paused = true;
+        job->busy = false;
+        qemu_coroutine_yield(); /* wait for block_job_resume() */
+        job->busy = true;
+        job->paused = false;
+    }
+
+    if (job->driver->resume) {
+        job->driver->resume(job);
+    }
+}
+
 void block_job_resume(BlockJob *job)
 {
     assert(job->pause_count > 0);
@@ -364,11 +390,9 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
     if (!block_job_should_pause(job)) {
         co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns);
     }
-    /* The job can be paused while sleeping, so check this again */
-    if (block_job_should_pause(job)) {
-        qemu_coroutine_yield();
-    }
     job->busy = true;
+
+    block_job_pause_point(job);
 }
 
 void block_job_yield(BlockJob *job)
@@ -381,8 +405,12 @@ void block_job_yield(BlockJob *job)
     }
 
     job->busy = false;
-    qemu_coroutine_yield();
+    if (!block_job_should_pause(job)) {
+        qemu_coroutine_yield();
+    }
     job->busy = true;
+
+    block_job_pause_point(job);
 }
 
 BlockJobInfo *block_job_query(BlockJob *job)
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 8fcecf9..7739f37 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -70,6 +70,20 @@ typedef struct BlockJobDriver {
      * never both.
      */
     void (*abort)(BlockJob *job);
+
+    /**
+     * If the callback is not NULL, it will be invoked when the job transitions
+     * into the paused state.  Paused jobs must not perform any asynchronous
+     * I/O or event loop activity.  This callback is used to quiesce jobs.
+     */
+    void coroutine_fn (*pause)(BlockJob *job);
+
+    /**
+     * If the callback is not NULL, it will be invoked when the job transitions
+     * out of the paused state.  Any asynchronous I/O or event loop activity
+     * should be restarted from this callback.
+     */
+    void coroutine_fn (*resume)(BlockJob *job);
 } BlockJobDriver;
 
 /**
@@ -119,13 +133,19 @@ struct BlockJob {
     bool user_paused;
 
     /**
-     * Set to false by the job while it is in a quiescent state, where
-     * no I/O is pending and the job has yielded on any condition
-     * that is not detected by #aio_poll, such as a timer.
+     * Set to false by the job while the coroutine has yielded and may be
+     * re-entered by block_job_enter().  There may still be I/O or event loop
+     * activity pending.
      */
     bool busy;
 
     /**
+     * Set to true by the job while it is in a quiescent state, where
+     * no I/O or event loop activity is pending.
+     */
+    bool paused;
+
+    /**
      * Set to true when the job is ready to be completed.
      */
     bool ready;
@@ -299,6 +319,15 @@ bool block_job_is_cancelled(BlockJob *job);
 BlockJobInfo *block_job_query(BlockJob *job);
 
 /**
+ * block_job_pause_point:
+ * @job: The job that is ready to pause.
+ *
+ * Pause now if block_job_pause() has been called.  Block jobs that perform
+ * lots of I/O must call this between requests so that the job can be paused.
+ */
+void coroutine_fn block_job_pause_point(BlockJob *job);
+
+/**
  * block_job_pause:
  * @job: The job to be paused.
  *
-- 
2.5.5

[Qemu-devel] [PULL 16/20] blockjob: add block_job_get_aio_context()

[Stefan Hajnoczi]

Add a helper function to document why block jobs sometimes run in the
QEMU main loop and to avoid code duplication in a following patch.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 1466096189-6477-5-git-send-email-stefanha@redhat.com
---
 blockjob.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 874b573..096a1b8 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -60,6 +60,17 @@ BlockJob *block_job_next(BlockJob *job)
     return QLIST_NEXT(job, job_list);
 }
 
+/* Normally the job runs in its BlockBackend's AioContext.  The exception is
+ * block_job_defer_to_main_loop() where it runs in the QEMU main loop.  Code
+ * that supports both cases uses this helper function.
+ */
+static AioContext *block_job_get_aio_context(BlockJob *job)
+{
+    return job->deferred_to_main_loop ?
+           qemu_get_aio_context() :
+           blk_get_aio_context(job->blk);
+}
+
 void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
                        int64_t speed, BlockCompletionFunc *cb,
                        void *opaque, Error **errp)
@@ -337,9 +348,7 @@ static int block_job_finish_sync(BlockJob *job,
         return -EBUSY;
     }
     while (!job->completed) {
-        aio_poll(job->deferred_to_main_loop ? qemu_get_aio_context() :
-                                              blk_get_aio_context(job->blk),
-                 true);
+        aio_poll(block_job_get_aio_context(job), true);
     }
     ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
     block_job_unref(job);
-- 
2.5.5

[Qemu-devel] [PULL 17/20] block: use safe iteration over AioContext notifiers

[Stefan Hajnoczi]

It's possible that an AioContext notifier user was close to finishing
when .detach_aio_context() or .attached_aio_context() is called.  In
that case they may call bdrv_remove_aio_context_notifier() during the
callback.

Use safe iteration to avoid crashing when the notifier list is modified
during iteration.  We must not only handle the case where the current
aio notifier is removed during a callback but also the one where any
other aio notifier is removed.

The next patch adds an AioContext notifier for block jobs and they
really could be terminating just as .detach_aio_context() is invoked.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 1466096189-6477-6-git-send-email-stefanha@redhat.com
---
 block.c                   | 46 ++++++++++++++++++++++++++++++++++++----------
 include/block/block_int.h |  2 ++
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index b331eb9..c0ccc27 100644
--- a/block.c
+++ b/block.c
@@ -3609,18 +3609,34 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs)
     return bs->aio_context;
 }
 
+static void bdrv_do_remove_aio_context_notifier(BdrvAioNotifier *ban)
+{
+    QLIST_REMOVE(ban, list);
+    g_free(ban);
+}
+
 void bdrv_detach_aio_context(BlockDriverState *bs)
 {
-    BdrvAioNotifier *baf;
+    BdrvAioNotifier *baf, *baf_tmp;
     BdrvChild *child;
 
     if (!bs->drv) {
         return;
     }
 
-    QLIST_FOREACH(baf, &bs->aio_notifiers, list) {
-        baf->detach_aio_context(baf->opaque);
+    assert(!bs->walking_aio_notifiers);
+    bs->walking_aio_notifiers = true;
+    QLIST_FOREACH_SAFE(baf, &bs->aio_notifiers, list, baf_tmp) {
+        if (baf->deleted) {
+            bdrv_do_remove_aio_context_notifier(baf);
+        } else {
+            baf->detach_aio_context(baf->opaque);
+        }
     }
+    /* Never mind iterating again to check for ->deleted.  bdrv_close() will
+     * remove remaining aio notifiers if we aren't called again.
+     */
+    bs->walking_aio_notifiers = false;
 
     if (bs->drv->bdrv_detach_aio_context) {
         bs->drv->bdrv_detach_aio_context(bs);
@@ -3635,7 +3651,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
 void bdrv_attach_aio_context(BlockDriverState *bs,
                              AioContext *new_context)
 {
-    BdrvAioNotifier *ban;
+    BdrvAioNotifier *ban, *ban_tmp;
     BdrvChild *child;
 
     if (!bs->drv) {
@@ -3651,9 +3667,16 @@ void bdrv_attach_aio_context(BlockDriverState *bs,
         bs->drv->bdrv_attach_aio_context(bs, new_context);
     }
 
-    QLIST_FOREACH(ban, &bs->aio_notifiers, list) {
-        ban->attached_aio_context(new_context, ban->opaque);
+    assert(!bs->walking_aio_notifiers);
+    bs->walking_aio_notifiers = true;
+    QLIST_FOREACH_SAFE(ban, &bs->aio_notifiers, list, ban_tmp) {
+        if (ban->deleted) {
+            bdrv_do_remove_aio_context_notifier(ban);
+        } else {
+            ban->attached_aio_context(new_context, ban->opaque);
+        }
     }
+    bs->walking_aio_notifiers = false;
 }
 
 void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
@@ -3695,11 +3718,14 @@ void bdrv_remove_aio_context_notifier(BlockDriverState *bs,
     QLIST_FOREACH_SAFE(ban, &bs->aio_notifiers, list, ban_next) {
         if (ban->attached_aio_context == attached_aio_context &&
             ban->detach_aio_context   == detach_aio_context   &&
-            ban->opaque               == opaque)
+            ban->opaque               == opaque               &&
+            ban->deleted              == false)
         {
-            QLIST_REMOVE(ban, list);
-            g_free(ban);
-
+            if (bs->walking_aio_notifiers) {
+                ban->deleted = true;
+            } else {
+                bdrv_do_remove_aio_context_notifier(ban);
+            }
             return;
         }
     }
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 688c6be..2057156 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -361,6 +361,7 @@ typedef struct BdrvAioNotifier {
     void (*detach_aio_context)(void *opaque);
 
     void *opaque;
+    bool deleted;
 
     QLIST_ENTRY(BdrvAioNotifier) list;
 } BdrvAioNotifier;
@@ -427,6 +428,7 @@ struct BlockDriverState {
      * BDS may register themselves in this list to be notified of changes
      * regarding this BDS's context */
     QLIST_HEAD(, BdrvAioNotifier) aio_notifiers;
+    bool walking_aio_notifiers; /* to make removal during iteration safe */
 
     char filename[PATH_MAX];
     char backing_file[PATH_MAX]; /* if non zero, the image is a diff of
-- 
2.5.5

[Qemu-devel] [PULL 18/20] blockjob: add AioContext attached callback

[Stefan Hajnoczi]

Block jobs that use additional BDSes or event loop resources need a
callback to get their affairs in order when the AioContext is switched.

Simple block jobs don't need an attach callback, they automatically work
thanks to the generic attach/detach notifiers that this patch adds.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 1466096189-6477-7-git-send-email-stefanha@redhat.com
---
 blockjob.c               | 38 ++++++++++++++++++++++++++++++++++++++
 include/block/blockjob.h |  7 +++++++
 2 files changed, 45 insertions(+)

diff --git a/blockjob.c b/blockjob.c
index 096a1b8..90c4e26 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -71,6 +71,38 @@ static AioContext *block_job_get_aio_context(BlockJob *job)
            blk_get_aio_context(job->blk);
 }
 
+static void block_job_attached_aio_context(AioContext *new_context,
+                                           void *opaque)
+{
+    BlockJob *job = opaque;
+
+    if (job->driver->attached_aio_context) {
+        job->driver->attached_aio_context(job, new_context);
+    }
+
+    block_job_resume(job);
+}
+
+static void block_job_detach_aio_context(void *opaque)
+{
+    BlockJob *job = opaque;
+
+    /* In case the job terminates during aio_poll()... */
+    block_job_ref(job);
+
+    block_job_pause(job);
+
+    if (!job->paused) {
+        /* If job is !job->busy this kicks it into the next pause point. */
+        block_job_enter(job);
+    }
+    while (!job->paused && !job->completed) {
+        aio_poll(block_job_get_aio_context(job), true);
+    }
+
+    block_job_unref(job);
+}
+
 void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
                        int64_t speed, BlockCompletionFunc *cb,
                        void *opaque, Error **errp)
@@ -103,6 +135,9 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
 
     QLIST_INSERT_HEAD(&block_jobs, job, job_list);
 
+    blk_add_aio_context_notifier(blk, block_job_attached_aio_context,
+                                 block_job_detach_aio_context, job);
+
     /* Only set speed when necessary to avoid NotSupported error */
     if (speed != 0) {
         Error *local_err = NULL;
@@ -128,6 +163,9 @@ void block_job_unref(BlockJob *job)
         BlockDriverState *bs = blk_bs(job->blk);
         bs->job = NULL;
         bdrv_op_unblock_all(bs, job->blocker);
+        blk_remove_aio_context_notifier(job->blk,
+                                        block_job_attached_aio_context,
+                                        block_job_detach_aio_context, job);
         blk_unref(job->blk);
         error_free(job->blocker);
         g_free(job->id);
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 7739f37..7dc720c 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -84,6 +84,13 @@ typedef struct BlockJobDriver {
      * should be restarted from this callback.
      */
     void coroutine_fn (*resume)(BlockJob *job);
+
+    /*
+     * If the callback is not NULL, it will be invoked before the job is
+     * resumed in a new AioContext.  This is the place to move any resources
+     * besides job->blk to the new AioContext.
+     */
+    void (*attached_aio_context)(BlockJob *job, AioContext *new_context);
 } BlockJobDriver;
 
 /**
-- 
2.5.5

[Qemu-devel] [PULL 19/20] mirror: follow AioContext change gracefully

[Stefan Hajnoczi]

Add block_job_pause_point() calls to mark quiescent points and make sure
to complete in-flight requests when switching AioContexts.

This patch solves undefined behavior in the mirror block job when the
BDS AioContext is changed by dataplane.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 1466096189-6477-8-git-send-email-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/mirror.c | 43 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 075384a..a04ed9c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -331,6 +331,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
         mirror_wait_for_io(s);
     }
 
+    block_job_pause_point(&s->common);
+
     /* Find the number of consective dirty chunks following the first dirty
      * one, and wait for in flight requests in them. */
     while (nb_chunks * sectors_per_chunk < (s->buf_size >> BDRV_SECTOR_BITS)) {
@@ -582,6 +584,8 @@ static void coroutine_fn mirror_run(void *opaque)
             if (now - last_pause_ns > SLICE_TIME) {
                 last_pause_ns = now;
                 block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, 0);
+            } else {
+                block_job_pause_point(&s->common);
             }
 
             if (block_job_is_cancelled(&s->common)) {
@@ -613,6 +617,8 @@ static void coroutine_fn mirror_run(void *opaque)
             goto immediate_exit;
         }
 
+        block_job_pause_point(&s->common);
+
         cnt = bdrv_get_dirty_count(s->dirty_bitmap);
         /* s->common.offset contains the number of bytes already processed so
          * far, cnt is the number of dirty sectors remaining and
@@ -795,18 +801,39 @@ static void mirror_complete(BlockJob *job, Error **errp)
     block_job_enter(&s->common);
 }
 
+/* There is no matching mirror_resume() because mirror_run() will begin
+ * iterating again when the job is resumed.
+ */
+static void coroutine_fn mirror_pause(BlockJob *job)
+{
+    MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+
+    mirror_drain(s);
+}
+
+static void mirror_attached_aio_context(BlockJob *job, AioContext *new_context)
+{
+    MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+
+    blk_set_aio_context(s->target, new_context);
+}
+
 static const BlockJobDriver mirror_job_driver = {
-    .instance_size = sizeof(MirrorBlockJob),
-    .job_type      = BLOCK_JOB_TYPE_MIRROR,
-    .set_speed     = mirror_set_speed,
-    .complete      = mirror_complete,
+    .instance_size          = sizeof(MirrorBlockJob),
+    .job_type               = BLOCK_JOB_TYPE_MIRROR,
+    .set_speed              = mirror_set_speed,
+    .complete               = mirror_complete,
+    .pause                  = mirror_pause,
+    .attached_aio_context   = mirror_attached_aio_context,
 };
 
 static const BlockJobDriver commit_active_job_driver = {
-    .instance_size = sizeof(MirrorBlockJob),
-    .job_type      = BLOCK_JOB_TYPE_COMMIT,
-    .set_speed     = mirror_set_speed,
-    .complete      = mirror_complete,
+    .instance_size          = sizeof(MirrorBlockJob),
+    .job_type               = BLOCK_JOB_TYPE_COMMIT,
+    .set_speed              = mirror_set_speed,
+    .complete               = mirror_complete,
+    .pause                  = mirror_pause,
+    .attached_aio_context   = mirror_attached_aio_context,
 };
 
 static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
-- 
2.5.5

[Qemu-devel] [PULL 20/20] backup: follow AioContext change gracefully

[Stefan Hajnoczi]

Move s->target to the new AioContext when there is an AioContext change.

The backup_run() coroutine does not use asynchronous I/O so there is no
need to wait for in-flight requests in a BlockJobDriver->pause()
callback.

Guest writes are intercepted by the backup job.  Treat them as guest
activity and do it even while the job is paused.  This is necessary
since the only alternative would be to fail a job that experienced guest
writes during pause once the job is resumed.  In practice the guest
writes don't interfere with AioContext switching since bdrv_drain() is
used by bdrv_set_aio_context().

Loops already contain pause points because of block_job_sleep_ns() calls
in the yield_and_check() helper function.  It is necessary to convert a
raw qemu_coroutine_yield() to block_job_yield() so the
MIRROR_SYNC_MODE_NONE case can pause.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 1466096189-6477-9-git-send-email-stefanha@redhat.com
---
 block/backup.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index feeb9f8..581269b 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -246,12 +246,20 @@ static void backup_abort(BlockJob *job)
     }
 }
 
+static void backup_attached_aio_context(BlockJob *job, AioContext *aio_context)
+{
+    BackupBlockJob *s = container_of(job, BackupBlockJob, common);
+
+    blk_set_aio_context(s->target, aio_context);
+}
+
 static const BlockJobDriver backup_job_driver = {
-    .instance_size  = sizeof(BackupBlockJob),
-    .job_type       = BLOCK_JOB_TYPE_BACKUP,
-    .set_speed      = backup_set_speed,
-    .commit         = backup_commit,
-    .abort          = backup_abort,
+    .instance_size          = sizeof(BackupBlockJob),
+    .job_type               = BLOCK_JOB_TYPE_BACKUP,
+    .set_speed              = backup_set_speed,
+    .commit                 = backup_commit,
+    .abort                  = backup_abort,
+    .attached_aio_context   = backup_attached_aio_context,
 };
 
 static BlockErrorAction backup_error_action(BackupBlockJob *job,
@@ -392,9 +400,7 @@ static void coroutine_fn backup_run(void *opaque)
         while (!block_job_is_cancelled(&job->common)) {
             /* Yield until the job is cancelled.  We just let our before_write
              * notify callback service CoW requests. */
-            job->common.busy = false;
-            qemu_coroutine_yield();
-            job->common.busy = true;
+            block_job_yield(&job->common);
         }
     } else if (job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
         ret = backup_run_incremental(job);
-- 
2.5.5

Re: [Qemu-devel] [PULL 00/20] Block patches

[Peter Maydell]

On 20 June 2016 at 15:05, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit 482b61844ae7c6df39df0b48ac90ffbc87bed7d2:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20160617' into staging (2016-06-17 16:16:37 +0100)
>
> are available in the git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 5ab4b69ce29908b327a91966dc78ea0fd7424075:
>
>   backup: follow AioContext change gracefully (2016-06-20 14:25:41 +0100)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM