[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.7 3/8] libqos: drop duplicated virtio_conf
From: |
Marc Marí |
Subject: |
Re: [Qemu-devel] [PATCH for-2.7 3/8] libqos: drop duplicated virtio_config.h definitions |
Date: |
Sun, 8 May 2016 20:40:36 +0200 |
On Sun, 8 May 2016 20:22:53 +0200
Marc Marí <address@hidden> wrote:
> On Mon, 25 Apr 2016 13:46:08 +0100
> Stefan Hajnoczi <address@hidden> wrote:
>
> > Signed-off-by: Stefan Hajnoczi <address@hidden>
> > ---
> > tests/libqos/virtio.c | 19 ++++++++++---------
> > tests/libqos/virtio.h | 9 ---------
> > tests/virtio-blk-test.c | 5 +++--
> > 3 files changed, 13 insertions(+), 20 deletions(-)
> >
> > diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
> > index 613dece..ee9e892 100644
> > --- a/tests/libqos/virtio.c
> > +++ b/tests/libqos/virtio.c
> > @@ -11,6 +11,7 @@
> > #include <glib.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) @@ -55,28 +56,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 e663bcf..993ae0e 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 02107a6..bfeffc4 100644
> > --- a/tests/virtio-blk-test.c
> > +++ b/tests/virtio-blk-test.c
> > @@ -20,6 +20,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
> > @@ -240,7 +241,7 @@ static void test_basic(const QVirtioBus *bus,
> > QVirtioDevice *dev,
> > guest_free(alloc, req_addr);
> >
> > - if (features & QVIRTIO_F_ANY_LAYOUT) {
> > + if (features & VIRTIO_F_ANY_LAYOUT) {
> > /* Write and read with 2 descriptor layout */
> > /* Write request */
> > req.type = QVIRTIO_BLK_T_OUT;
> > @@ -607,7 +608,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 |
> > - QVIRTIO_F_NOTIFY_ON_EMPTY |
> > QVIRTIO_BLK_F_SCSI);
> > + 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,
>
> In standard-headers/linux/virtio_config.h, there is;
>
> #define VIRTIO_F_NOTIFY_ON_EMPTY 24
> #define VIRTIO_F_ANY_LAYOUT 27
>
> whereas in tests/libqos.h, there is:
>
> #define QVIRTIO_F_NOTIFY_ON_EMPTY 0x01000000
> #define QVIRTIO_F_ANY_LAYOUT 0x08000000
>
> It is necessary to use 2 << VIRTIO_F_NOTIFY_ON_EMPTY to make the
> change properly.
>
I meant 1 << VIRTIO_F_NOTIFY_ON_EMPTY, obviously.
And what I say is half corrected in the next patch. I think it would be
nicer to make the changes directly in this patch.
In any case, this line:
> if (features & VIRTIO_F_ANY_LAYOUT) {
is not corrected in any patch
Marc