diff mbox

[for-2.7,3/8] libqos: drop duplicated virtio_config.h definitions

Message ID 1461588373-26486-4-git-send-email-stefanha@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Hajnoczi April 25, 2016, 12:46 p.m. UTC 
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqos/virtio.c   | 19 ++++++++++---------
 tests/libqos/virtio.h   |  9 ---------
 tests/virtio-blk-test.c |  5 +++--
 3 files changed, 13 insertions(+), 20 deletions(-)

Comments

Marc Marí May 8, 2016, 6:22 p.m. UTC | #1 
On Mon, 25 Apr 2016 13:46:08 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  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.

Marc
Marc Marí May 8, 2016, 6:40 p.m. UTC | #2 
On Sun, 8 May 2016 20:22:53 +0200
Marc Marí <marc.mari.barcelo@gmail.com> wrote:

> On Mon, 25 Apr 2016 13:46:08 +0100
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  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
Stefan Hajnoczi May 9, 2016, 11:46 a.m. UTC | #3 
On Sun, May 08, 2016 at 08:40:36PM +0200, Marc Marí wrote:
> On Sun, 8 May 2016 20:22:53 +0200
> Marc Marí <marc.mari.barcelo@gmail.com> wrote:
> 
> > On Mon, 25 Apr 2016 13:46:08 +0100
> > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > 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

Thanks for pointing this out, I thought I had caught all instances where
(1 << X) and X differ.

Will fix in the next revision.

Stefan
diff mbox

Patch

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,