Re: [PATCH-for-8.0 v2 6/6] hw/virtio: Extract QMP related code virtio-qmp.c

[Philippe Mathieu-Daudé]

On 19/12/22 14:31, Jonah Palmer wrote:
> On 12/13/22 06:17, Philippe Mathieu-Daudé wrote:
> > The monitor decoders are the only functions using the CONFIG_xxx
> > definitions declared in the target specific CONFIG_DEVICES header.
> >
> > Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> > ---
> >   hw/virtio/meson.build  |   2 +-
> >   hw/virtio/virtio-qmp.c | 659 +++++++++++++++++++++++++++++++++++++++++
> >   hw/virtio/virtio-qmp.h |  20 ++
> >   hw/virtio/virtio.c     | 635 +--------------------------------------
> >   4 files changed, 682 insertions(+), 634 deletions(-)
> >   create mode 100644 hw/virtio/virtio-qmp.c
> >   create mode 100644 hw/virtio/virtio-qmp.h

> I haven't tried this myself, but is there a reason why we're not also including
> the 'qmp_decode_vring_desc_flags' function here?

Because we use VirtQueue as an opaque forward-declared structure.

The structure is declared in hw/virtio/virtio.c, which is where
the internal fields are accessed.

If I move qmp_x_query_virtio_queue_status() and
qmp_x_query_virtio_queue_element(), which calls
qmp_decode_vring_desc_flags(), I get:

../hw/virtio/virtio-qmp.c:874:35: error: subscript of pointer to 
incomplete type 'VirtQueue' (aka 'struct VirtQueue')
    status->queue_index = vdev->vq[queue].queue_index;
                          ~~~~~~~~^
include/hw/virtio/virtio.h:39:8: note: forward declaration of 'struct 
VirtQueue'
struct VirtQueue;
       ^

Now you are right the QOM-generic functions (qmp_x_query_virtio,
qmp_x_query_virtio_status and qmp_x_query_virtio_vhost_queue_status)
can be moved.

Thanks for your review,

Phil.