From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56599) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzJr0-0001F6-0s for qemu-devel@nongnu.org; Tue, 24 Jun 2014 02:05:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WzJqt-0007Hr-Nm for qemu-devel@nongnu.org; Tue, 24 Jun 2014 02:05:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41647) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzJqt-0007HH-Gq for qemu-devel@nongnu.org; Tue, 24 Jun 2014 02:05:07 -0400 Date: Tue, 24 Jun 2014 14:05:17 +0800 From: Fam Zheng Message-ID: <20140624060517.GA4490@T430.redhat.com> References: <1403535322-3346-1-git-send-email-marc.mari.barcelo@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1403535322-3346-1-git-send-email-marc.mari.barcelo@gmail.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC] Functions bus_foreach and device_find from libqos virtio API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marc =?iso-8859-1?Q?Mar=ED?= Cc: Paolo Bonzini , qemu-devel@nongnu.org, Stefan Hajnoczi On Mon, 06/23 16:55, Marc Mar=ED 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 +=3D tests/libqos/i2c.o > libqos-pc-obj-y =3D $(libqos-obj-y) tests/libqos/pci-pc.o > libqos-pc-obj-y +=3D tests/libqos/malloc-pc.o > libqos-omap-obj-y =3D $(libqos-obj-y) tests/libqos/i2c-omap.o > +libqos-virtio-obj-y =3D $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqo= s/virtio-pci.o $(libqos-pc-obj-y) has $(libqos-obj-y) itself, two lines above. So no nee= d 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=ED > + * > + * 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=ED > + * > + * 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 =3D { > + .notify =3D qvirtio_pci_notify, > + .get_config =3D qvirtio_pci_get_config, > + .set_config =3D qvirtio_pci_set_config, > + .get_features =3D qvirtio_pci_get_features, > + .get_status =3D qvirtio_pci_get_status, > + .set_status =3D qvirtio_pci_set_status, > + .reset =3D qvirtio_pci_reset, > + .query_isr =3D qvirtio_pci_query_isr, > + .bus_foreach =3D qvirtio_pci_foreach, > + .device_find =3D 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 m= ake the implementation functions static, because they are already accessed th= rough 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 > +/*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 Lin= ux's. I > +think QEMU implementation is better, because it will be easier to conn= ect the > +QEMU Virtqueue with the Libaos VirtQueue. > + > +The functions to use the VirtQueue are the ones I think necessary in L= ibqos, 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=ED > * > * 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 @@ > =20 > #include > #include > +#include > +#include > +#include > #include "libqtest.h" > -#include "qemu/osdep.h" > +#include "libqos/virtio.h" > +/*#include "qemu/osdep.h"*/ and this. > =20 > -/* Tests only initialization so far. TODO: Replace with functional tes= ts */ > -static void pci_nop(void) > +#define TEST_IMAGE_SIZE (64 * 1024 * 1024) > + > +static char tmp_path[] =3D "/tmp/qtest.XXXXXX"; > +extern QVirtioBus qvirtio_pci; > + > +static void pci_basic(void) > { > + QVirtioDevice *dev; > + dev =3D qvirtio_pci.device_find(VIRTIO_BLK_DEVICE_ID); > + fprintf(stderr, "Device: %x %x %x\n", > + dev->device_id, dev->location, dev->device= _type); > } > =20 > 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 ... > =20 > g_test_init(&argc, &argv, NULL); > - qtest_add_func("/virtio/blk/pci/nop", pci_nop); > + qtest_add_func("/virtio/blk/pci/basic", pci_basic); > =20 > - qtest_start("-drive id=3Ddrv0,if=3Dnone,file=3D/dev/null " > - "-device virtio-blk-pci,drive=3Ddrv0"); > + /* Create a temporary raw image */ > + fd =3D mkstemp(tmp_path); > + g_assert_cmpint(fd, >=3D, 0); > + ret =3D ftruncate(fd, TEST_IMAGE_SIZE); > + g_assert_cmpint(ret, =3D=3D, 0); > + close(fd); > + > + sprintf(test_start, "-drive if=3Dnone,id=3Ddrive0,file=3D%s " > + "-device virtio-blk-pci,drive=3Ddrive0", tmp_path)= ; ... here. Also please use snprintf. > + qtest_start(test_start); > ret =3D g_test_run(); > =20 > qtest_end(); > =20 > + /* Cleanup */ > + unlink(tmp_path); > + > return ret; > } > --=20 > 1.7.10.4 >=20 >=20 Fam