diff mbox

[RFC] Functions bus_foreach and device_find from libqos virtio API

Message ID 1403535322-3346-1-git-send-email-marc.mari.barcelo@gmail.com
State New
Headers show

Commit Message

Marc Marí June 23, 2014, 2:55 p.m. UTC
Virtio header has been changed to compile and work with a real device.
Functions bus_foreach and device_find have been implemented for PCI.
Virtio-blk test case now opens a fake device.

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 tests/Makefile            |    3 +-
 tests/libqos/virtio-pci.c |  123 +++++++++++++++++++++++++++++++++
 tests/libqos/virtio-pci.h |   40 +++++++++++
 tests/libqos/virtio.h     |  167 +++++++++++++++++++++++++++++++++++++++++++++
 tests/virtio-blk-test.c   |   38 +++++++++--
 5 files changed, 364 insertions(+), 7 deletions(-)
 create mode 100644 tests/libqos/virtio-pci.c
 create mode 100644 tests/libqos/virtio-pci.h
 create mode 100644 tests/libqos/virtio.h

Comments

Stefan Hajnoczi June 24, 2014, 4:41 a.m. UTC | #1
On Mon, Jun 23, 2014 at 04:55:22PM +0200, Marc Marí wrote:
> +static QVirtioDevice *qpcidevice_to_qvirtiodevice(QPCIDevice *dpci)
> +{
> +    QVirtioDevice *dvirtio;
> +    dvirtio = g_malloc0(sizeof(*dvirtio));
> +
> +    if (dpci) {
> +        dvirtio->device_id = qpci_config_readw(dpci, PCI_DEVICE_ID);
> +        dvirtio->device_type = qpci_config_readw(dpci, PCI_SUBSYSTEM_ID);
> +        dvirtio->location = dpci->devfn;
> +        dvirtio->vq = g_malloc0(sizeof(QVirtQueue));

QVirtioDevice doesn't know about PCI, so device_id and location are not
appropriate - at least as public fields that callers are expected to
access.  It's fine to stash away the PCI-specific information, probably
by defining a QVirtioPCIDevice struct that embeds QVirtioDevice:

typedef struct {
    QVirtioDevice vdev;
    QPCIDevice *pdev;
} QVirtioPCIDevice;

Then virtio-pci functions can cast from the generic QVirtioDevice back
to the PCI-specific QVirtioPCIDevice:

static void qvirtio_pci_notify(QVirtioDevice *vdev)
{
    QVirtioPCIDevice *dev = (QVirtioPCIDevice*)vdev;

    qpci_io_writew(dev->pdev, ...);
}

> +void qvirtio_pci_foreach(uint16_t device_type,
> +                void (*func)(QVirtioDevice *d, void *data), void *data)
> +{
> +    QPCIBus *bus;
> +    QPCIDevice *dev;
> +    int slot;
> +    int fn;
> +
> +    bus = qpci_init_pc();

This might be a design flaw in qpci_init_pc().  Not sure it's a good
idea to have a local PCI bus for every qvirtio_pci_foreach() call.

Tests should be able to operate on different PCI busses, if the machine
has several, but devices on the same bus should probably share a QPCIBus
instance.

> +    for (slot = 0; slot < 32; slot++) {
> +        for (fn = 0; fn < 8; fn++) {
> +            dev = g_malloc0(sizeof(*dev));
> +            dev->bus = bus;
> +            dev->devfn = QPCI_DEVFN(slot, fn);
> +
> +            if (qpci_config_readw(dev, PCI_VENDOR_ID) == 0xFFFF) {
> +                g_free(dev);
> +                continue;
> +            }
> +
> +            if (device_type != (uint16_t)-1 &&
> +                qpci_config_readw(dev, PCI_SUBSYSTEM_ID) != device_type) {
> +                continue;
> +            }
> +
> +            func(qpcidevice_to_qvirtiodevice(dev), data);
> +        }
> +    }

Can you reuse qpci_device_foreach() instead of duplicating this code?

> +uint64_t qvring_desc_addr(uint64_t desc_pa, int i);
> +uint32_t qvring_desc_len(uint64_t desc_pa, int i);
> +uint16_t qvring_desc_flags(uint64_t desc_pa, int i);
> +uint16_t qvring_desc_next(uint64_t desc_pa, int i);
> +uint16_t qvring_avail_flags(QVirtQueue *vq);
> +uint16_t qvring_avail_idx(QVirtQueue *vq);
> +uint16_t qvring_avail_ring(QVirtQueue *vq, int i);
> +uint16_t qvring_used_event(QVirtQueue *vq);
> +void qvring_used_ring_id(QVirtQueue *vq, int i, uint32_t val);
> +void qvring_used_ring_len(QVirtQueue *vq, int i, uint32_t val);
> +uint16_t qvring_used_idx(QVirtQueue *vq);
> +void qvring_used_idx_set(QVirtQueue *vq, uint16_t val);
> +void qvring_used_flags_set_bit(QVirtQueue *vq, int mask);
> +void qvring_used_flags_unset_bit(QVirtQueue *vq, int mask);
> +void qvring_avail_event(QVirtQueue *vq, uint16_t val);
> +
> +void qvirtqueue_push(QVirtQueue *vq, const QVirtQueueElement *elem,
> +                                                        unsigned int len);
> +void qvirtqueue_flush(QVirtQueue *vq, unsigned int count);
> +void qvirtqueue_fill(QVirtQueue *vq, const QVirtQueueElement *elem,
> +                                        unsigned int len, unsigned int idx);
> +void qvirtqueue_pop(QVirtQueue *vq, QVirtQueueElement *elem);
> +void qvirtqueue_map_sg(/*struct iovec *sg,*/ uint64_t *addr,
> +    size_t num_sg, int is_write);
> +void qvirtio_notify(QVirtioDevice *vdev, QVirtQueue *vq);
> +int qvirtio_queue_ready(QVirtQueue *vq);
> +int qvirtio_queue_empty(QVirtQueue *vq);

Please drop all the unimplemented functions and unused structs from the
next patch revision.  Let's focus on one step at a time:
1. Finding devices
2. Device reset, setting status field bits, feature bit negotiation
3. Notification (qtest->QEMU kick, QEMU->qtest interrupt)
4. Virtqueues
5. Simple virtio-blk tests (e.g. read a block from the disk) to prove it
   works

I'm suggesting you send the next patch revision without RFC and just
with a test case for finding devices (no unimplemented stuff).  That way
you can send new patches each week and have them merged incrementally,
rather than pushing along one big series that keeps growing until the
very end.

> +
> +#endif
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index d53f875..4f5b1e0 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -2,6 +2,7 @@
>   * QTest testcase for VirtIO Block Device
>   *
>   * Copyright (c) 2014 SUSE LINUX Products GmbH
> + * Copyright (c) 2014 Marc Marí
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>   * See the COPYING file in the top-level directory.
> @@ -9,26 +10,51 @@
>  
>  #include <glib.h>
>  #include <string.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <stdio.h>
>  #include "libqtest.h"
> -#include "qemu/osdep.h"
> +#include "libqos/virtio.h"
> +/*#include "qemu/osdep.h"*/
>  
> -/* Tests only initialization so far. TODO: Replace with functional tests */
> -static void pci_nop(void)
> +#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
> +
> +static char tmp_path[] = "/tmp/qtest.XXXXXX";
> +extern QVirtioBus qvirtio_pci;
> +
> +static void pci_basic(void)
>  {
> +    QVirtioDevice *dev;
> +    dev = qvirtio_pci.device_find(VIRTIO_BLK_DEVICE_ID);
> +    fprintf(stderr, "Device: %x %x %x\n",
> +                            dev->device_id, dev->location, dev->device_type);
>  }
>  
>  int main(int argc, char **argv)
>  {
>      int ret;
> +    int fd;
> +    char test_start[100];
>  
>      g_test_init(&argc, &argv, NULL);
> -    qtest_add_func("/virtio/blk/pci/nop", pci_nop);
> +    qtest_add_func("/virtio/blk/pci/basic", pci_basic);
>  
> -    qtest_start("-drive id=drv0,if=none,file=/dev/null "
> -                "-device virtio-blk-pci,drive=drv0");
> +    /* Create a temporary raw image */
> +    fd = mkstemp(tmp_path);
> +    g_assert_cmpint(fd, >=, 0);
> +    ret = ftruncate(fd, TEST_IMAGE_SIZE);
> +    g_assert_cmpint(ret, ==, 0);
> +    close(fd);
> +
> +    sprintf(test_start, "-drive if=none,id=drive0,file=%s "
> +                    "-device virtio-blk-pci,drive=drive0", tmp_path);
> +    qtest_start(test_start);

Every test case should probably start a fresh QEMU with a fresh disk
image.  That way no device register state or disk image state can affect
later test cases.
Fam Zheng June 24, 2014, 6:05 a.m. UTC | #2
On Mon, 06/23 16:55, Marc Marí wrote:
> diff --git a/tests/Makefile b/tests/Makefile
> index 4caf7de..b85a036 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -282,6 +282,7 @@ libqos-obj-y += tests/libqos/i2c.o
>  libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
>  libqos-pc-obj-y += tests/libqos/malloc-pc.o
>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
> +libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio-pci.o

$(libqos-pc-obj-y) has $(libqos-obj-y) itself, two lines above. So no need to
add again.
> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
> new file mode 100644
> index 0000000..62c238f
> --- /dev/null
> +++ b/tests/libqos/virtio-pci.c
> @@ -0,0 +1,123 @@
> +/*
> + * libqos virtio definitions

PCI

> + *
> + * Copyright (c) 2014 Marc Marí
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
> new file mode 100644
> index 0000000..d92bcf2
> --- /dev/null
> +++ b/tests/libqos/virtio-pci.h
> @@ -0,0 +1,40 @@
> +/*
> + * libqos virtio PCI definitions
> + *
> + * Copyright (c) 2014 Marc Marí
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef LIBQOS_VIRTIO_PCI_H
> +#define LIBQOS_VIRTIO_PCI_H
> +
> +#include "libqos/virtio.h"
> +
> +void qvirtio_pci_notify(QVirtioDevice *d, uint16_t vector);
> +void qvirtio_pci_get_config(QVirtioDevice *d, void *config);
> +void qvirtio_pci_set_config(QVirtioDevice *d, void *config);
> +uint32_t qvirtio_pci_get_features(QVirtioDevice *d);
> +uint8_t qvirtio_pci_get_status(QVirtioDevice *d);
> +void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t val);
> +void qvirtio_pci_reset(QVirtioDevice *d);
> +uint8_t qvirtio_pci_query_isr(QVirtioDevice *d);
> +void qvirtio_pci_foreach(uint16_t device_id,
> +                void (*func)(QVirtioDevice *d, void *data), void *data);
> +QVirtioDevice *qvirtio_pci_device_find(uint16_t device_id);
> +
> +const QVirtioBus qvirtio_pci = {
> +    .notify = qvirtio_pci_notify,
> +    .get_config = qvirtio_pci_get_config,
> +    .set_config = qvirtio_pci_set_config,
> +    .get_features = qvirtio_pci_get_features,
> +    .get_status = qvirtio_pci_get_status,
> +    .set_status = qvirtio_pci_set_status,
> +    .reset = qvirtio_pci_reset,
> +    .query_isr = qvirtio_pci_query_isr,
> +    .bus_foreach = qvirtio_pci_foreach,
> +    .device_find = qvirtio_pci_device_find,
> +};

Each source file to include this header will define its own copy of
qvirtio_pci. Please move this to tests/libqos/virtio-pci.c. And can you make
the implementation functions static, because they are already accessed through
members of qvirtio_pci?

> +
> +#endif
> diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
> new file mode 100644
> index 0000000..838aae4
> --- /dev/null
> +++ b/tests/libqos/virtio.h

<snip>

> +/*QVirtioBus *qvirtio_pci_init();
> +QVirtioBus *qvirtio_mmio_init();
> +QVirtioBus *qvirtio_ccw_init();*/

When you drop RFC, please drop this,

> +
> +/*
> +I'm not sure what implementation of VirtQueue is better, QEMU's or Linux's. I
> +think QEMU implementation is better, because it will be easier to connect the
> +QEMU Virtqueue with the Libaos VirtQueue.
> +
> +The functions to use the VirtQueue are the ones I think necessary in Libqos, but
> +probably there are some missing and some others that are not necessary.
> +*/

and this,

> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index d53f875..4f5b1e0 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -2,6 +2,7 @@
>   * QTest testcase for VirtIO Block Device
>   *
>   * Copyright (c) 2014 SUSE LINUX Products GmbH
> + * Copyright (c) 2014 Marc Marí
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>   * See the COPYING file in the top-level directory.
> @@ -9,26 +10,51 @@
>  
>  #include <glib.h>
>  #include <string.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <stdio.h>
>  #include "libqtest.h"
> -#include "qemu/osdep.h"
> +#include "libqos/virtio.h"
> +/*#include "qemu/osdep.h"*/

and this.

>  
> -/* Tests only initialization so far. TODO: Replace with functional tests */
> -static void pci_nop(void)
> +#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
> +
> +static char tmp_path[] = "/tmp/qtest.XXXXXX";
> +extern QVirtioBus qvirtio_pci;
> +
> +static void pci_basic(void)
>  {
> +    QVirtioDevice *dev;
> +    dev = qvirtio_pci.device_find(VIRTIO_BLK_DEVICE_ID);
> +    fprintf(stderr, "Device: %x %x %x\n",
> +                            dev->device_id, dev->location, dev->device_type);
>  }
>  
>  int main(int argc, char **argv)
>  {
>      int ret;
> +    int fd;
> +    char test_start[100];

Depending on length of tmp_path, this looks quite close to an overflow ...

>  
>      g_test_init(&argc, &argv, NULL);
> -    qtest_add_func("/virtio/blk/pci/nop", pci_nop);
> +    qtest_add_func("/virtio/blk/pci/basic", pci_basic);
>  
> -    qtest_start("-drive id=drv0,if=none,file=/dev/null "
> -                "-device virtio-blk-pci,drive=drv0");
> +    /* Create a temporary raw image */
> +    fd = mkstemp(tmp_path);
> +    g_assert_cmpint(fd, >=, 0);
> +    ret = ftruncate(fd, TEST_IMAGE_SIZE);
> +    g_assert_cmpint(ret, ==, 0);
> +    close(fd);
> +
> +    sprintf(test_start, "-drive if=none,id=drive0,file=%s "
> +                    "-device virtio-blk-pci,drive=drive0", tmp_path);

... here. Also please use snprintf.

> +    qtest_start(test_start);
>      ret = g_test_run();
>  
>      qtest_end();
>  
> +    /* Cleanup */
> +    unlink(tmp_path);
> +
>      return ret;
>  }
> -- 
> 1.7.10.4
> 
> 

Fam
Marc Marí June 24, 2014, 8:02 a.m. UTC | #3
Hello

> > +    for (slot = 0; slot < 32; slot++) {
> > +        for (fn = 0; fn < 8; fn++) {
> > +            dev = g_malloc0(sizeof(*dev));
> > +            dev->bus = bus;
> > +            dev->devfn = QPCI_DEVFN(slot, fn);
> > +
> > +            if (qpci_config_readw(dev, PCI_VENDOR_ID) == 0xFFFF) {
> > +                g_free(dev);
> > +                continue;
> > +            }
> > +
> > +            if (device_type != (uint16_t)-1 &&
> > +                qpci_config_readw(dev, PCI_SUBSYSTEM_ID) !=
> > device_type) {
> > +                continue;
> > +            }
> > +
> > +            func(qpcidevice_to_qvirtiodevice(dev), data);
> > +        }
> > +    }
> 
> Can you reuse qpci_device_foreach() instead of duplicating this code?

I've been thinking and I don't know how to join this definition:
void qvirtio_pci_foreach(uint16_t device_id, void (*func)(QVirtioDevice
*d, void *data), void *data);

With this definition:
void qpci_device_foreach(QPCIBus *bus, int vendor_id, int device_id,
void (*func)(QPCIDevice *dev, int devfn, void *data), void *data)

The function parameter lacks one parameter (which is PCI specific)

Any idea?

> > +
> > +#endif
> > diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> > index d53f875..4f5b1e0 100644
> > --- a/tests/virtio-blk-test.c
> > +++ b/tests/virtio-blk-test.c
> > @@ -2,6 +2,7 @@
> >   * QTest testcase for VirtIO Block Device
> >   *
> >   * Copyright (c) 2014 SUSE LINUX Products GmbH
> > + * Copyright (c) 2014 Marc Marí
> >   *
> >   * This work is licensed under the terms of the GNU GPL, version 2
> > or later.
> >   * See the COPYING file in the top-level directory.
> > @@ -9,26 +10,51 @@
> >  
> >  #include <glib.h>
> >  #include <string.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <stdio.h>
> >  #include "libqtest.h"
> > -#include "qemu/osdep.h"
> > +#include "libqos/virtio.h"
> > +/*#include "qemu/osdep.h"*/
> >  
> > -/* Tests only initialization so far. TODO: Replace with functional
> > tests */ -static void pci_nop(void)
> > +#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
> > +
> > +static char tmp_path[] = "/tmp/qtest.XXXXXX";
> > +extern QVirtioBus qvirtio_pci;
> > +
> > +static void pci_basic(void)
> >  {
> > +    QVirtioDevice *dev;
> > +    dev = qvirtio_pci.device_find(VIRTIO_BLK_DEVICE_ID);
> > +    fprintf(stderr, "Device: %x %x %x\n",
> > +                            dev->device_id, dev->location,
> > dev->device_type); }
> >  
> >  int main(int argc, char **argv)
> >  {
> >      int ret;
> > +    int fd;
> > +    char test_start[100];
> >  
> >      g_test_init(&argc, &argv, NULL);
> > -    qtest_add_func("/virtio/blk/pci/nop", pci_nop);
> > +    qtest_add_func("/virtio/blk/pci/basic", pci_basic);
> >  
> > -    qtest_start("-drive id=drv0,if=none,file=/dev/null "
> > -                "-device virtio-blk-pci,drive=drv0");
> > +    /* Create a temporary raw image */
> > +    fd = mkstemp(tmp_path);
> > +    g_assert_cmpint(fd, >=, 0);
> > +    ret = ftruncate(fd, TEST_IMAGE_SIZE);
> > +    g_assert_cmpint(ret, ==, 0);
> > +    close(fd);
> > +
> > +    sprintf(test_start, "-drive if=none,id=drive0,file=%s "
> > +                    "-device virtio-blk-pci,drive=drive0",
> > tmp_path);
> > +    qtest_start(test_start);
> 
> Every test case should probably start a fresh QEMU with a fresh disk
> image.  That way no device register state or disk image state can
> affect later test cases.

Is it better to create a new temporary file (and so, re-run qemu), or
just truncate every time?
Marc Marí June 24, 2014, 8:11 a.m. UTC | #4
Hello

> > -/* Tests only initialization so far. TODO: Replace with functional
> > tests */ -static void pci_nop(void)
> > +#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
> > +
> > +static char tmp_path[] = "/tmp/qtest.XXXXXX";
> > +extern QVirtioBus qvirtio_pci;
> > +
> > +static void pci_basic(void)
> >  {
> > +    QVirtioDevice *dev;
> > +    dev = qvirtio_pci.device_find(VIRTIO_BLK_DEVICE_ID);
> > +    fprintf(stderr, "Device: %x %x %x\n",
> > +                            dev->device_id, dev->location,
> > dev->device_type); }
> >  
> >  int main(int argc, char **argv)
> >  {
> >      int ret;
> > +    int fd;
> > +    char test_start[100];
> 
> Depending on length of tmp_path, this looks quite close to an
> overflow ...
> 
> >  
> >      g_test_init(&argc, &argv, NULL);
> > -    qtest_add_func("/virtio/blk/pci/nop", pci_nop);
> > +    qtest_add_func("/virtio/blk/pci/basic", pci_basic);
> >  
> > -    qtest_start("-drive id=drv0,if=none,file=/dev/null "
> > -                "-device virtio-blk-pci,drive=drv0");
> > +    /* Create a temporary raw image */
> > +    fd = mkstemp(tmp_path);
> > +    g_assert_cmpint(fd, >=, 0);
> > +    ret = ftruncate(fd, TEST_IMAGE_SIZE);
> > +    g_assert_cmpint(ret, ==, 0);
> > +    close(fd);
> > +
> > +    sprintf(test_start, "-drive if=none,id=drive0,file=%s "
> > +                    "-device virtio-blk-pci,drive=drive0",
> > tmp_path);
> 
> ... here. Also please use snprintf.
> 

tmp_path is defined as global, and has always the same size
(/tmp/qtest.XXXXXX, where the X will be replaced by a temporary name).

But I'll change to snprintf for security.
diff mbox

Patch

diff --git a/tests/Makefile b/tests/Makefile
index 4caf7de..b85a036 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -282,6 +282,7 @@  libqos-obj-y += tests/libqos/i2c.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
+libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio-pci.o
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
@@ -302,7 +303,7 @@  tests/eepro100-test$(EXESUF): tests/eepro100-test.o
 tests/vmxnet3-test$(EXESUF): tests/vmxnet3-test.o
 tests/ne2000-test$(EXESUF): tests/ne2000-test.o
 tests/virtio-balloon-test$(EXESUF): tests/virtio-balloon-test.o
-tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o
+tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o $(libqos-virtio-obj-y)
 tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o
 tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o
 tests/virtio-scsi-test$(EXESUF): tests/virtio-scsi-test.o
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
new file mode 100644
index 0000000..62c238f
--- /dev/null
+++ b/tests/libqos/virtio-pci.c
@@ -0,0 +1,123 @@ 
+/*
+ * libqos virtio definitions
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+#include <stdio.h>
+#include "libqtest.h"
+#include "libqos/virtio.h"
+#include "libqos/virtio-pci.h"
+#include "libqos/pci.h"
+#include "libqos/pci-pc.h"
+
+#include "hw/pci/pci_regs.h"
+
+static QVirtioDevice *qpcidevice_to_qvirtiodevice(QPCIDevice *dpci)
+{
+    QVirtioDevice *dvirtio;
+    dvirtio = g_malloc0(sizeof(*dvirtio));
+
+    if (dpci) {
+        dvirtio->device_id = qpci_config_readw(dpci, PCI_DEVICE_ID);
+        dvirtio->device_type = qpci_config_readw(dpci, PCI_SUBSYSTEM_ID);
+        dvirtio->location = dpci->devfn;
+        dvirtio->vq = g_malloc0(sizeof(QVirtQueue));
+    }
+
+    return dvirtio;
+}
+
+static void qvirtio_pci_assign_device(QVirtioDevice *d, void *data)
+{
+    QVirtioDevice *dev = data;
+    dev->device_id      = d->device_id;
+    dev->location       = d->location;
+    dev->device_type    = d->device_type;
+    dev->vq = d->vq;
+}
+
+void qvirtio_pci_notify(QVirtioDevice *d, uint16_t vector)
+{
+
+}
+
+void qvirtio_pci_get_config(QVirtioDevice *d, void *config)
+{
+
+}
+
+void qvirtio_pci_set_config(QVirtioDevice *d, void *config)
+{
+
+}
+
+uint32_t qvirtio_pci_get_features(QVirtioDevice *d)
+{
+    return 0;
+}
+
+uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
+{
+    return 0;
+}
+
+void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t val)
+{
+
+}
+
+void qvirtio_pci_reset(QVirtioDevice *d)
+{
+
+}
+
+uint8_t qvirtio_pci_query_isr(QVirtioDevice *d)
+{
+    return 0;
+}
+
+void qvirtio_pci_foreach(uint16_t device_type,
+                void (*func)(QVirtioDevice *d, void *data), void *data)
+{
+    QPCIBus *bus;
+    QPCIDevice *dev;
+    int slot;
+    int fn;
+
+    bus = qpci_init_pc();
+
+    for (slot = 0; slot < 32; slot++) {
+        for (fn = 0; fn < 8; fn++) {
+            dev = g_malloc0(sizeof(*dev));
+            dev->bus = bus;
+            dev->devfn = QPCI_DEVFN(slot, fn);
+
+            if (qpci_config_readw(dev, PCI_VENDOR_ID) == 0xFFFF) {
+                g_free(dev);
+                continue;
+            }
+
+            if (device_type != (uint16_t)-1 &&
+                qpci_config_readw(dev, PCI_SUBSYSTEM_ID) != device_type) {
+                continue;
+            }
+
+            func(qpcidevice_to_qvirtiodevice(dev), data);
+        }
+    }
+}
+
+QVirtioDevice *qvirtio_pci_device_find(uint16_t device_type)
+{
+    QVirtioDevice *dev;
+
+    dev = g_malloc0(sizeof(*dev));
+    qvirtio_pci_foreach(device_type, qvirtio_pci_assign_device, dev);
+
+    return dev;
+}
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
new file mode 100644
index 0000000..d92bcf2
--- /dev/null
+++ b/tests/libqos/virtio-pci.h
@@ -0,0 +1,40 @@ 
+/*
+ * libqos virtio PCI definitions
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef LIBQOS_VIRTIO_PCI_H
+#define LIBQOS_VIRTIO_PCI_H
+
+#include "libqos/virtio.h"
+
+void qvirtio_pci_notify(QVirtioDevice *d, uint16_t vector);
+void qvirtio_pci_get_config(QVirtioDevice *d, void *config);
+void qvirtio_pci_set_config(QVirtioDevice *d, void *config);
+uint32_t qvirtio_pci_get_features(QVirtioDevice *d);
+uint8_t qvirtio_pci_get_status(QVirtioDevice *d);
+void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t val);
+void qvirtio_pci_reset(QVirtioDevice *d);
+uint8_t qvirtio_pci_query_isr(QVirtioDevice *d);
+void qvirtio_pci_foreach(uint16_t device_id,
+                void (*func)(QVirtioDevice *d, void *data), void *data);
+QVirtioDevice *qvirtio_pci_device_find(uint16_t device_id);
+
+const QVirtioBus qvirtio_pci = {
+    .notify = qvirtio_pci_notify,
+    .get_config = qvirtio_pci_get_config,
+    .set_config = qvirtio_pci_set_config,
+    .get_features = qvirtio_pci_get_features,
+    .get_status = qvirtio_pci_get_status,
+    .set_status = qvirtio_pci_set_status,
+    .reset = qvirtio_pci_reset,
+    .query_isr = qvirtio_pci_query_isr,
+    .bus_foreach = qvirtio_pci_foreach,
+    .device_find = qvirtio_pci_device_find,
+};
+
+#endif
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
new file mode 100644
index 0000000..838aae4
--- /dev/null
+++ b/tests/libqos/virtio.h
@@ -0,0 +1,167 @@ 
+/*
+ * libqos virtio definitions
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef LIBQOS_VIRTIO_H
+#define LIBQOS_VIRTIO_H
+
+#define VIRTQUEUE_MAX_SIZE 1024
+#define VIRTIO_VENDOR_ID 0x1AF4
+
+#define VIRTIO_NET_DEVICE_ID 0x1
+#define VIRTIO_BLK_DEVICE_ID 0x2
+
+typedef struct QVirtioDevice QVirtioDevice;
+typedef struct QVirtQueue QVirtQueue;
+typedef struct QVRing QVRing;
+
+typedef struct QVirtioDevice {
+    /* Device id */
+    uint16_t device_id;
+
+    /* Device location (depends on transport) */
+    int location;
+
+    /* Device type */
+    uint16_t device_type;
+
+    /* VQs associated with the device */
+    QVirtQueue *vq;
+} QVirtioDevice;
+
+typedef struct QVirtioBus {
+    /* Send a notification IRQ to the device */
+    void (*notify)(QVirtioDevice *d, uint16_t vector);
+
+    /* Get configuration of the device */
+    void (*get_config)(QVirtioDevice *d, void *config);
+
+    /* Set configuration of the device */
+    void (*set_config)(QVirtioDevice *d, void *config);
+
+    /* Get features of the device */
+    uint32_t (*get_features)(QVirtioDevice *d);
+
+    /* Get status of the device */
+    uint8_t (*get_status)(QVirtioDevice *d);
+
+    /* Set status of the device  */
+    void (*set_status)(QVirtioDevice *d, uint8_t val);
+
+    /* Reset the device */
+    void (*reset)(QVirtioDevice *d);
+
+    /* Check pending IRQs */
+    uint8_t (*query_isr)(QVirtioDevice *d);
+
+    /* Loop all devices, applying a function to all, or the one specified */
+    void (*bus_foreach)(uint16_t device_t,
+                void (*func)(QVirtioDevice *d, void *data), void *data);
+
+    /* Find and return a device */
+    QVirtioDevice *(*device_find)(uint16_t device_type);
+} QVirtioBus;
+
+/*QVirtioBus *qvirtio_pci_init();
+QVirtioBus *qvirtio_mmio_init();
+QVirtioBus *qvirtio_ccw_init();*/
+
+/*
+I'm not sure what implementation of VirtQueue is better, QEMU's or Linux's. I
+think QEMU implementation is better, because it will be easier to connect the
+QEMU Virtqueue with the Libaos VirtQueue.
+
+The functions to use the VirtQueue are the ones I think necessary in Libqos, but
+probably there are some missing and some others that are not necessary.
+*/
+
+typedef struct QVRing {
+    unsigned int num;
+    unsigned int align;
+    uint64_t desc;
+    uint64_t avail;
+    uint64_t used;
+} QVRing;
+
+struct QVirtQueue {
+    QVRing vring;
+    uint64_t pa;
+    uint16_t last_avail_idx;
+    uint16_t signalled_used;
+    bool signalled_used_valid;
+    bool notification;
+    uint16_t queue_index;
+    int inuse;
+    uint16_t vector;
+    void (*handle_output)(QVirtioDevice *d, QVirtQueue *vq);
+    QVirtioDevice *dev;
+};
+
+typedef struct QVirtQueueElement {
+    unsigned int index;
+    unsigned int out_num;
+    unsigned int in_num;
+    uint64_t in_addr[VIRTQUEUE_MAX_SIZE];
+    uint64_t out_addr[VIRTQUEUE_MAX_SIZE];
+    /*struct iovec in_sg[VIRTQUEUE_MAX_SIZE];*/
+    /*struct iovec out_sg[VIRTQUEUE_MAX_SIZE];*/
+} QVirtQueueElement;
+
+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];
+} VRingAvail;
+
+typedef struct QVRingUsedElem {
+    uint32_t id;
+    uint32_t len;
+} QVRingUsedElem;
+
+typedef struct QVRingUsed  {
+    uint16_t flags;
+    uint16_t idx;
+    QVRingUsedElem ring[0];
+} QVRingUsed;
+
+uint64_t qvring_desc_addr(uint64_t desc_pa, int i);
+uint32_t qvring_desc_len(uint64_t desc_pa, int i);
+uint16_t qvring_desc_flags(uint64_t desc_pa, int i);
+uint16_t qvring_desc_next(uint64_t desc_pa, int i);
+uint16_t qvring_avail_flags(QVirtQueue *vq);
+uint16_t qvring_avail_idx(QVirtQueue *vq);
+uint16_t qvring_avail_ring(QVirtQueue *vq, int i);
+uint16_t qvring_used_event(QVirtQueue *vq);
+void qvring_used_ring_id(QVirtQueue *vq, int i, uint32_t val);
+void qvring_used_ring_len(QVirtQueue *vq, int i, uint32_t val);
+uint16_t qvring_used_idx(QVirtQueue *vq);
+void qvring_used_idx_set(QVirtQueue *vq, uint16_t val);
+void qvring_used_flags_set_bit(QVirtQueue *vq, int mask);
+void qvring_used_flags_unset_bit(QVirtQueue *vq, int mask);
+void qvring_avail_event(QVirtQueue *vq, uint16_t val);
+
+void qvirtqueue_push(QVirtQueue *vq, const QVirtQueueElement *elem,
+                                                        unsigned int len);
+void qvirtqueue_flush(QVirtQueue *vq, unsigned int count);
+void qvirtqueue_fill(QVirtQueue *vq, const QVirtQueueElement *elem,
+                                        unsigned int len, unsigned int idx);
+void qvirtqueue_pop(QVirtQueue *vq, QVirtQueueElement *elem);
+void qvirtqueue_map_sg(/*struct iovec *sg,*/ uint64_t *addr,
+    size_t num_sg, int is_write);
+void qvirtio_notify(QVirtioDevice *vdev, QVirtQueue *vq);
+int qvirtio_queue_ready(QVirtQueue *vq);
+int qvirtio_queue_empty(QVirtQueue *vq);
+
+#endif
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index d53f875..4f5b1e0 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -2,6 +2,7 @@ 
  * QTest testcase for VirtIO Block Device
  *
  * Copyright (c) 2014 SUSE LINUX Products GmbH
+ * Copyright (c) 2014 Marc Marí
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -9,26 +10,51 @@ 
 
 #include <glib.h>
 #include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
 #include "libqtest.h"
-#include "qemu/osdep.h"
+#include "libqos/virtio.h"
+/*#include "qemu/osdep.h"*/
 
-/* Tests only initialization so far. TODO: Replace with functional tests */
-static void pci_nop(void)
+#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
+
+static char tmp_path[] = "/tmp/qtest.XXXXXX";
+extern QVirtioBus qvirtio_pci;
+
+static void pci_basic(void)
 {
+    QVirtioDevice *dev;
+    dev = qvirtio_pci.device_find(VIRTIO_BLK_DEVICE_ID);
+    fprintf(stderr, "Device: %x %x %x\n",
+                            dev->device_id, dev->location, dev->device_type);
 }
 
 int main(int argc, char **argv)
 {
     int ret;
+    int fd;
+    char test_start[100];
 
     g_test_init(&argc, &argv, NULL);
-    qtest_add_func("/virtio/blk/pci/nop", pci_nop);
+    qtest_add_func("/virtio/blk/pci/basic", pci_basic);
 
-    qtest_start("-drive id=drv0,if=none,file=/dev/null "
-                "-device virtio-blk-pci,drive=drv0");
+    /* Create a temporary raw image */
+    fd = mkstemp(tmp_path);
+    g_assert_cmpint(fd, >=, 0);
+    ret = ftruncate(fd, TEST_IMAGE_SIZE);
+    g_assert_cmpint(ret, ==, 0);
+    close(fd);
+
+    sprintf(test_start, "-drive if=none,id=drive0,file=%s "
+                    "-device virtio-blk-pci,drive=drive0", tmp_path);
+    qtest_start(test_start);
     ret = g_test_run();
 
     qtest_end();
 
+    /* Cleanup */
+    unlink(tmp_path);
+
     return ret;
 }