Skip to content

Commit

Permalink
libqos: use microseconds instead of iterations for virtio timeout
Browse files Browse the repository at this point in the history 
Some hosts are slow or overloaded so test execution takes a long time.
Test cases use timeouts to protect against an infinite loop stalling the
test forever (especially important in automated test setups).

Commit 6cd1405 ("libqos virtio:
Increase ISR timeout") increased the clock_step() value in an attempt to
lengthen the virtio interrupt wait timeout, but timeout failures are
still occuring on the Travis automated testing platform.

This is because clock_step() only affects the guest's virtual time.
Virtio requests can be bottlenecked on host disk I/O latency - which
cannot be improved by stepping the clock, so the fix was ineffective.

This patch changes the qvirtio_wait_queue_isr() and
qvirtio_wait_config_isr() timeout mechanism from loop iterations to
microseconds.  This way the test case can specify an absolute 30 second
timeout.  Number of loop iterations is not a reliable timeout mechanism
since the speed depends on many factors including host performance.

Tests should no longer timeout on overloaded Travis instances.

Cc: Marc Marí <[email protected]>
Reported-by: Peter Maydell <[email protected]>
Signed-off-by: Stefan Hajnoczi <[email protected]>
Signed-off-by: Peter Maydell <[email protected]>
  • Loading branch information
@stefanhaRH @pm215
stefanhaRH authored and pm215 committed Sep 29, 2014
1 parent e8c81b4 commit 7055626
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 43 deletions.
30 changes: 16 additions & 14 deletions tests/libqos/virtio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,18 @@ void qvirtio_set_driver_ok(const QVirtioBus *bus, QVirtioDevice *d)
QVIRTIO_DRIVER_OK | QVIRTIO_DRIVER | QVIRTIO_ACKNOWLEDGE);
}

bool qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice *d,
QVirtQueue *vq, uint64_t timeout)
void qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice *d,
QVirtQueue *vq, gint64 timeout_us)
{
do {
gint64 start_time = g_get_monotonic_time();

for (;;) {
clock_step(100);
if (bus->get_queue_isr_status(d, vq)) {
break; /* It has ended */
return;
}
} while (--timeout);

return timeout != 0;
g_assert(g_get_monotonic_time() - start_time <= timeout_us);
}
}

/* Wait for the status byte at given guest memory address to be set
Expand All @@ -113,17 +114,18 @@ uint8_t qvirtio_wait_status_byte_no_isr(const QVirtioBus *bus,
return val;
}

bool qvirtio_wait_config_isr(const QVirtioBus *bus, QVirtioDevice *d,
uint64_t timeout)
void qvirtio_wait_config_isr(const QVirtioBus *bus, QVirtioDevice *d,
gint64 timeout_us)
{
do {
gint64 start_time = g_get_monotonic_time();

for (;;) {
clock_step(100);
if (bus->get_config_isr_status(d)) {
break; /* It has ended */
return;
}
} while (--timeout);

return timeout != 0;
g_assert(g_get_monotonic_time() - start_time <= timeout_us);
}
}

void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr)
Expand Down
8 changes: 4 additions & 4 deletions tests/libqos/virtio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,15 @@ void qvirtio_set_acknowledge(const QVirtioBus *bus, QVirtioDevice *d);
void qvirtio_set_driver(const QVirtioBus *bus, QVirtioDevice *d);
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);
void qvirtio_wait_queue_isr(const QVirtioBus *bus, QVirtioDevice *d,
QVirtQueue *vq, gint64 timeout_us);
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);
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);

Expand Down
47 changes: 22 additions & 25 deletions tests/virtio-blk-test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
#define QVIRTIO_BLK_T_GET_ID 8

#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
Expand Down Expand Up @@ -184,8 +183,8 @@ static void pci_basic(void)
qvirtqueue_add(&vqpci->vq, req_addr + 528, 1, true, false);
qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);

g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT_US);
status = readb(req_addr + 528);
g_assert_cmpint(status, ==, 0);

Expand All @@ -206,8 +205,8 @@ static void pci_basic(void)

qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);

g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT_US);
status = readb(req_addr + 528);
g_assert_cmpint(status, ==, 0);

Expand All @@ -234,8 +233,8 @@ static void pci_basic(void)

qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);

g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT_US);
status = readb(req_addr + 528);
g_assert_cmpint(status, ==, 0);

Expand All @@ -257,8 +256,8 @@ static void pci_basic(void)

qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);

g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT_US);
status = readb(req_addr + 528);
g_assert_cmpint(status, ==, 0);

Expand Down Expand Up @@ -330,8 +329,8 @@ static void pci_indirect(void)
free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);

g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT_US);
status = readb(req_addr + 528);
g_assert_cmpint(status, ==, 0);

Expand All @@ -355,8 +354,8 @@ static void pci_indirect(void)
free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);

g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT_US);
status = readb(req_addr + 528);
g_assert_cmpint(status, ==, 0);

Expand Down Expand Up @@ -397,8 +396,7 @@ static void pci_config(void)

qmp("{ 'execute': 'block_resize', 'arguments': { 'device': 'drive0', "
" 'size': %d } }", n_size);
g_assert(qvirtio_wait_config_isr(&qvirtio_pci, &dev->vdev,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_config_isr(&qvirtio_pci, &dev->vdev, QVIRTIO_BLK_TIMEOUT_US);

capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
g_assert_cmpint(capacity, ==, n_size / 512);
Expand Down Expand Up @@ -453,8 +451,7 @@ static void pci_msix(void)
qmp("{ 'execute': 'block_resize', 'arguments': { 'device': 'drive0', "
" 'size': %d } }", n_size);

g_assert(qvirtio_wait_config_isr(&qvirtio_pci, &dev->vdev,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_config_isr(&qvirtio_pci, &dev->vdev, QVIRTIO_BLK_TIMEOUT_US);

capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
g_assert_cmpint(capacity, ==, n_size / 512);
Expand All @@ -474,8 +471,8 @@ static void pci_msix(void)
qvirtqueue_add(&vqpci->vq, req_addr + 528, 1, true, false);
qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);

g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT_US);

status = readb(req_addr + 528);
g_assert_cmpint(status, ==, 0);
Expand All @@ -498,8 +495,8 @@ static void pci_msix(void)
qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);


g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT_US);

status = readb(req_addr + 528);
g_assert_cmpint(status, ==, 0);
Expand Down Expand Up @@ -575,8 +572,8 @@ static void pci_idx(void)
qvirtqueue_add(&vqpci->vq, req_addr + 528, 1, true, false);
qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);

g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT_US);

/* Write request */
req.type = QVIRTIO_BLK_T_OUT;
Expand Down Expand Up @@ -619,8 +616,8 @@ static void pci_idx(void)
qvirtqueue_kick(&qvirtio_pci, &dev->vdev, &vqpci->vq, free_head);


g_assert(qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT));
qvirtio_wait_queue_isr(&qvirtio_pci, &dev->vdev, &vqpci->vq,
QVIRTIO_BLK_TIMEOUT_US);

status = readb(req_addr + 528);
g_assert_cmpint(status, ==, 0);
Expand Down

0 comments on commit 7055626

Please sign in to comment.