Message ID | 1412005212-18266-2-git-send-email-stefanha@redhat.com |
---|---|
State | New |
Headers | show |
On 29 September 2014 16:40, Stefan Hajnoczi <stefanha@redhat.com> wrote: > The virtio event_index feature lets the device driver tell the device > how many requests to process before raising the next interrupt. > virtio-blk-test.c tries to verify that the device does not raise an > interrupt unnecessarily. > > Unfortunately the test has a race condition. It spins checking for an > interrupt up to 100 times and then assumes the request has finished. On > a slow host the I/O request could still be in flight and the test would > fail. > > This patch waits for the request to complete, or until a 30-second > timeout is reached. If an interrupt is raised while waiting the test > fails since the device was not supposed to raise interrupts. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Hi; just noticed this breaks 'make check' build on MacOSX: /Users/pm215/src/qemu/tests/libqos/virtio.c:84:25: warning: implicit declaration of function 'g_get_monotonic_time' is invalid in C99 [-Wimplicit-function-declaration] gint64 start_time = g_get_monotonic_time(); ^ (and subsequent linker error). g_get_monotonic_time() only appeared in glib 2.28, and our minimum is 2.12. thanks -- PMM
On 11 October 2014 12:43, Peter Maydell <peter.maydell@linaro.org> wrote: > Hi; just noticed this breaks 'make check' build on MacOSX: > > /Users/pm215/src/qemu/tests/libqos/virtio.c:84:25: warning: implicit > declaration of function > 'g_get_monotonic_time' is invalid in C99 [-Wimplicit-function-declaration] > gint64 start_time = g_get_monotonic_time(); > ^ > (and subsequent linker error). > > g_get_monotonic_time() only appeared in glib 2.28, and our > minimum is 2.12. vhost-user-test.c has a helper function that has a fallback for not having g_get_monotonic_time(); we probably want to put that somewhere more generally accessible. -- PMM
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c index 9b6de2c..009325d 100644 --- a/tests/libqos/virtio.c +++ b/tests/libqos/virtio.c @@ -91,6 +91,28 @@ bool qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice *d, return timeout != 0; } +/* Wait for the status byte at given guest memory address to be set + * + * The virtqueue interrupt must not be raised, making this useful for testing + * event_index functionality. + */ +uint8_t qvirtio_wait_status_byte_no_isr(const QVirtioBus *bus, + QVirtioDevice *d, + QVirtQueue *vq, + uint64_t addr, + gint64 timeout_us) +{ + gint64 start_time = g_get_monotonic_time(); + uint8_t val; + + while ((val = readb(addr)) == 0xff) { + clock_step(100); + g_assert(!bus->get_queue_isr_status(d, vq)); + g_assert(g_get_monotonic_time() - start_time <= timeout_us); + } + return val; +} + bool qvirtio_wait_config_isr(const QVirtioBus *bus, QVirtioDevice *d, uint64_t timeout) { diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h index 70b3376..bc7518e 100644 --- a/tests/libqos/virtio.h +++ b/tests/libqos/virtio.h @@ -162,6 +162,11 @@ void qvirtio_set_driver_ok(const QVirtioBus *bus, QVirtioDevice *d); bool qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq, uint64_t timeout); +uint8_t qvirtio_wait_status_byte_no_isr(const QVirtioBus *bus, + QVirtioDevice *d, + QVirtQueue *vq, + uint64_t addr, + gint64 timeout_us); bool qvirtio_wait_config_isr(const QVirtioBus *bus, QVirtioDevice *d, uint64_t timeout); QVirtQueue *qvirtqueue_setup(const QVirtioBus *bus, QVirtioDevice *d, diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c index 588666c..0e3bfa7 100644 --- a/tests/virtio-blk-test.c +++ b/tests/virtio-blk-test.c @@ -42,6 +42,7 @@ #define TEST_IMAGE_SIZE (64 * 1024 * 1024) #define QVIRTIO_BLK_TIMEOUT 100 +#define QVIRTIO_BLK_TIMEOUT_US (30 * 1000 * 1000) #define PCI_SLOT 0x04 #define PCI_FN 0x00 @@ -595,10 +596,9 @@ static void pci_idx(void) qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head); /* No notification expected */ - g_assert(!qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq, - QVIRTIO_BLK_TIMEOUT)); - - status = readb(req_addr + 528); + status = qvirtio_wait_status_byte_no_isr(&qvirtio_pci, &dev->vdev, + &vqpci->vq, req_addr + 528, + QVIRTIO_BLK_TIMEOUT_US); g_assert_cmpint(status, ==, 0); guest_free(alloc, req_addr);
The virtio event_index feature lets the device driver tell the device how many requests to process before raising the next interrupt. virtio-blk-test.c tries to verify that the device does not raise an interrupt unnecessarily. Unfortunately the test has a race condition. It spins checking for an interrupt up to 100 times and then assumes the request has finished. On a slow host the I/O request could still be in flight and the test would fail. This patch waits for the request to complete, or until a 30-second timeout is reached. If an interrupt is raised while waiting the test fails since the device was not supposed to raise interrupts. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- tests/libqos/virtio.c | 22 ++++++++++++++++++++++ tests/libqos/virtio.h | 5 +++++ tests/virtio-blk-test.c | 8 ++++---- 3 files changed, 31 insertions(+), 4 deletions(-)