From 8ac9e205bd516d9cedbf28852d77c14ce977b74b Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 9 May 2016 13:47:34 +0100 Subject: [PATCH 01/20] libqos: use virtio_ids.h for device ID definitions Avoid redefining device IDs. Use the standard Linux headers that are already in the source tree. Signed-off-by: Stefan Hajnoczi 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 01012787b8..af03793870 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 8272ba8c42..a0d2d83644 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 e5c144818e..e49c13ea5f 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 5f1a8aefeb..26bb0e312d 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); From 7ad1e708e68426b4deaeb975636396ac4d6266b1 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 9 May 2016 13:47:35 +0100 Subject: [PATCH 02/20] libqos: drop duplicated PCI vendor ID definition Signed-off-by: Stefan Hajnoczi 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 9d45e2028e..dc867d7888 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 af03793870..e663bcf3d1 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 From 1373a4c2568282a8f998a778f09d9d628afcd7fd Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 9 May 2016 13:47:36 +0100 Subject: [PATCH 03/20] libqos: drop duplicated virtio_config.h definitions 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 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 d792635340..e3f6194527 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 e663bcf3d1..993ae0efe5 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 a0d2d83644..84eb056305 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, From ee3b850a70613a9e0cedef8c79f76e9ee45cbef3 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 9 May 2016 13:47:37 +0100 Subject: [PATCH 04/20] libqos: drop duplicated virtio_ring.h bit definitions 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 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 e15b480ab8..ccb92d9eef 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 dc867d7888..2418b7d86f 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 e3f6194527..0a3e05b3df 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 993ae0efe5..2af8036285 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 84eb056305..8efe3d6efb 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 e49c13ea5f..17124509a3 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); From 780b11a097ada54306d43d6e64450e8d7e7a76e4 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 9 May 2016 13:47:38 +0100 Subject: [PATCH 05/20] libqos: drop duplicated virtio_vring.h structs 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 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 0a3e05b3df..f79d2d14ec 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 2af8036285..c73fd8c24a 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, From 4565a3e02998d1e12e3218fa230c5f29fd2bcd2e Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 9 May 2016 13:47:39 +0100 Subject: [PATCH 06/20] libqos: drop duplicated virtio_blk.h definitions Signed-off-by: Stefan Hajnoczi 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 8efe3d6efb..3920038609 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); From 74f079a7eea9719e444b1e5162743ae1673a9813 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 9 May 2016 13:47:40 +0100 Subject: [PATCH 07/20] libqos: drop duplicated virtio_scsi.h definitions Signed-off-by: Stefan Hajnoczi 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 26bb0e312d..be0f760cec 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 " From c75f4c061bacad0c41708bf17d526fac72314ad0 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 9 May 2016 13:47:41 +0100 Subject: [PATCH 08/20] libqos: drop duplicated virtio_pci.h definitions Signed-off-by: Stefan Hajnoczi 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 2418b7d86f..3b2b59bcf8 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 8f0e52ad47..efcac2d3de 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 3920038609..9d27be571a 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 be0f760cec..3938163393 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); From f1d3b99154138741161fc52f5a8c373bf71613c6 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 5 May 2016 16:53:35 +0100 Subject: [PATCH 09/20] libqos: add qvirtqueue_cleanup() 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 --- 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 ccb92d9eef..0cab38f296 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 3b2b59bcf8..18b92b95dc 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 f79d2d14ec..d8c2970de7 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 c73fd8c24a..0250842bf2 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 9d27be571a..4ab14d5d36 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 17124509a3..0d2c63f8ab 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 3938163393..cbe5dcca24 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)); From 3a36e474f2ca74ebd4f36044823a625389dfef01 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Thu, 16 Jun 2016 19:09:39 +0300 Subject: [PATCH 10/20] block: fixed BdrvTrackedRequest filling in bdrv_co_discard The request area is specified in bytes, not in sectors. Signed-off-by: Denis V. Lunev Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Fam Zheng Reviewed-by: Eric Blake Message-id: 1466093381-6120-2-git-send-email-den@openvz.org CC: Stefan Hajnoczi CC: Kevin Wolf CC: Max Reitz Signed-off-by: Stefan Hajnoczi --- block/io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/io.c b/block/io.c index ebdb9d834c..3527d91aa0 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); From 968d8b0627d3585d6b82da4239b9adf98614ab7c Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Thu, 16 Jun 2016 19:09:40 +0300 Subject: [PATCH 11/20] block: fix race in bdrv_co_discard with drive-mirror 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 Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Fam Zheng Reviewed-by: Eric Blake Message-id: 1466093381-6120-3-git-send-email-den@openvz.org CC: Stefan Hajnoczi CC: Kevin Wolf CC: Max Reitz Signed-off-by: Stefan Hajnoczi --- block/io.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/io.c b/block/io.c index 3527d91aa0..8ab6f95477 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; } From ec050f77a549b300ee444634bccd9ec05d134c4d Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Thu, 16 Jun 2016 19:09:41 +0300 Subject: [PATCH 12/20] block: process before_write_notifiers in bdrv_co_discard 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 Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Fam Zheng Reviewed-by: Eric Blake Message-id: 1466093381-6120-4-git-send-email-den@openvz.org CC: Fam Zheng CC: Stefan Hajnoczi CC: Kevin Wolf CC: Max Reitz Signed-off-by: Stefan Hajnoczi --- block/io.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/block/io.c b/block/io.c index 8ab6f95477..7cf3645ade 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; From 17bd51f936ac0719ef7a93fb77e30313b55c83b5 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 16 Jun 2016 17:56:22 +0100 Subject: [PATCH 13/20] blockjob: move iostatus reset out of block_job_enter() 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 Reviewed-by: Fam Zheng Reviewed-by: Paolo Bonzini 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 c9a0068cd6..7299312632 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 01b896b7e7..5137dce808 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); } From a7f3b7ff03a4712b9fc1089cc568eea7296af069 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 16 Jun 2016 17:56:23 +0100 Subject: [PATCH 14/20] blockjob: rename block_job_is_paused() 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 Reviewed-by: Paolo Bonzini Reviewed-by: Fam Zheng 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 5137dce808..21cc3ee332 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 00ac4184cc..8fcecf9a79 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -347,15 +347,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. From fc9c0a9c4b2c07cf2b8683f2617af584f14c93e7 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 16 Jun 2016 17:56:24 +0100 Subject: [PATCH 15/20] blockjob: add pause points 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 Reviewed-by: Paolo Bonzini Reviewed-by: Fam Zheng 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 21cc3ee332..874b573bd5 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 8fcecf9a79..7739f37c3a 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,12 +133,18 @@ 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. */ @@ -298,6 +318,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. From 9f6bc648c40da61a50ea1f51048368faeea5d9a1 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 16 Jun 2016 17:56:25 +0100 Subject: [PATCH 16/20] blockjob: add block_job_get_aio_context() 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 Reviewed-by: Paolo Bonzini Reviewed-by: Fam Zheng 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 874b573bd5..096a1b8715 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); From e8a095dadb70e2ea6d5169d261920db3747bfa45 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 16 Jun 2016 17:56:26 +0100 Subject: [PATCH 17/20] block: use safe iteration over AioContext notifiers 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 Reviewed-by: Paolo Bonzini Reviewed-by: Fam Zheng 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 b331eb9d38..c0ccc27c73 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 688c6be009..205715600b 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 From 463e0be101cb5a78ca6ee517d58604c3f3637bcd Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 16 Jun 2016 17:56:27 +0100 Subject: [PATCH 18/20] blockjob: add AioContext attached callback 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 Reviewed-by: Paolo Bonzini Reviewed-by: Fam Zheng 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 096a1b8715..90c4e262b0 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 7739f37c3a..7dc720c82b 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; /** From 565ac01f8d35236844dd0257a185f81425cd4b6a Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 16 Jun 2016 17:56:28 +0100 Subject: [PATCH 19/20] mirror: follow AioContext change gracefully 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 Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini Reviewed-by: Fam Zheng Message-id: 1466096189-6477-8-git-send-email-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi --- block/mirror.c | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 075384a9cf..a04ed9c7a4 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, From 5ab4b69ce29908b327a91966dc78ea0fd7424075 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 16 Jun 2016 17:56:29 +0100 Subject: [PATCH 20/20] backup: follow AioContext change gracefully 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 Reviewed-by: Paolo Bonzini Reviewed-by: Fam Zheng 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 feeb9f8bf2..581269b29a 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);