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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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.