Re: [RFC v3 4/6] qmp: add QMP command x-debug-virtio-queue-status

[Jason Wang <jasowang@redhat.com>]

On 2020/5/7 下午7:49, Laurent Vivier wrote:
> This new command shows internal status of a VirtQueue.
> (vrings and indexes).
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>


It looks to me that packed virtqueue is not supported. It's better to 
add them in the future.


> ---
>   hw/virtio/virtio-stub.c |  6 +++
>   hw/virtio/virtio.c      | 35 +++++++++++++++
>   qapi/virtio.json        | 98 +++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 139 insertions(+)
>
> diff --git a/hw/virtio/virtio-stub.c b/hw/virtio/virtio-stub.c
> index ddb592f72eee..3c1bf172acf6 100644
> --- a/hw/virtio/virtio-stub.c
> +++ b/hw/virtio/virtio-stub.c
> @@ -17,3 +17,9 @@ VirtioStatus *qmp_x_debug_virtio_status(const char* path, Error **errp)
>   {
>       return qmp_virtio_unsupported(errp);
>   }
> +
> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
> +                                                 uint16_t queue, Error **errp)
> +{
> +    return qmp_virtio_unsupported(errp);
> +}
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 59bf6ef651a6..57552bf05014 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3877,6 +3877,41 @@ static VirtIODevice *virtio_device_find(const char *path)
>       return NULL;
>   }
>   
> +VirtQueueStatus *qmp_x_debug_virtio_queue_status(const char *path,
> +                                                 uint16_t queue, Error **errp)
> +{
> +    VirtIODevice *vdev;
> +    VirtQueueStatus *status;
> +
> +    vdev = virtio_device_find(path);
> +    if (vdev == NULL) {
> +        error_setg(errp, "Path %s is not a VirtIO device", path);
> +        return NULL;
> +    }
> +
> +    if (queue >= VIRTIO_QUEUE_MAX || !virtio_queue_get_num(vdev, queue)) {
> +        error_setg(errp, "Invalid virtqueue number %d", queue);
> +        return NULL;
> +    }
> +
> +    status = g_new0(VirtQueueStatus, 1);
> +    status->queue_index = vdev->vq[queue].queue_index;
> +    status->inuse = vdev->vq[queue].inuse;
> +    status->vring_num = vdev->vq[queue].vring.num;
> +    status->vring_num_default = vdev->vq[queue].vring.num_default;
> +    status->vring_align = vdev->vq[queue].vring.align;
> +    status->vring_desc = vdev->vq[queue].vring.desc;
> +    status->vring_avail = vdev->vq[queue].vring.avail;
> +    status->vring_used = vdev->vq[queue].vring.used;
> +    status->last_avail_idx = vdev->vq[queue].last_avail_idx;


This might not be correct when vhost is used.

We may consider to sync last_avail_idx from vhost backends here?

Thanks


> +    status->shadow_avail_idx = vdev->vq[queue].shadow_avail_idx;
> +    status->used_idx = vdev->vq[queue].used_idx;
> +    status->signalled_used = vdev->vq[queue].signalled_used;
> +    status->signalled_used_valid = vdev->vq[queue].signalled_used_valid;
> +
> +    return status;
> +}
> +
>   #define CONVERT_FEATURES(type, map)                \
>       ({                                           \
>           type *list = NULL;                         \
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 69dd107d0c9b..43c234a9fc69 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -308,3 +308,101 @@
>     'data': { 'path': 'str' },
>     'returns': 'VirtioStatus'
>   }
> +
> +##
> +# @VirtQueueStatus:
> +#
> +# Status of a VirtQueue
> +#
> +# @queue-index: VirtQueue queue_index
> +#
> +# @inuse: VirtQueue inuse
> +#
> +# @vring-num: VirtQueue vring.num
> +#
> +# @vring-num-default: VirtQueue vring.num_default
> +#
> +# @vring-align: VirtQueue vring.align
> +#
> +# @vring-desc: VirtQueue vring.desc
> +#
> +# @vring-avail: VirtQueue vring.avail
> +#
> +# @vring-used: VirtQueue vring.used
> +#
> +# @last-avail-idx: VirtQueue last_avail_idx
> +#
> +# @shadow-avail-idx: VirtQueue shadow_avail_idx
> +#
> +# @used-idx: VirtQueue used_idx
> +#
> +# @signalled-used: VirtQueue signalled_used
> +#
> +# @signalled-used-valid: VirtQueue signalled_used_valid
> +#
> +# Since: 5.1
> +#
> +##
> +
> +{ 'struct': 'VirtQueueStatus',
> +  'data': {
> +    'queue-index': 'uint16',
> +    'inuse': 'uint32',
> +    'vring-num': 'int',
> +    'vring-num-default': 'int',
> +    'vring-align': 'int',
> +    'vring-desc': 'uint64',
> +    'vring-avail': 'uint64',
> +    'vring-used': 'uint64',
> +    'last-avail-idx': 'uint16',
> +    'shadow-avail-idx': 'uint16',
> +    'used-idx': 'uint16',
> +    'signalled-used': 'uint16',
> +    'signalled-used-valid': 'uint16'
> +  }
> +}
> +
> +##
> +# @x-debug-virtio-queue-status:
> +#
> +# Return the status of a given VirtQueue
> +#
> +# @path: QOBject path of the VirtIODevice
> +#
> +# @queue: queue number to examine
> +#
> +# Returns: Status of the VirtQueue
> +#
> +# Since: 5.1
> +#
> +# Example:
> +#
> +# -> { "execute": "x-debug-virtio-queue-status",
> +#      "arguments": {
> +#          "path": "/machine/peripheral-anon/device[3]/virtio-backend",
> +#          "queue": 0
> +#      }
> +#   }
> +# <- { "return": {
> +#      "signalled-used": 373,
> +#      "inuse": 0,
> +#      "vring-desc": 864411648,
> +#      "vring-num-default": 256,
> +#      "signalled-used-valid": 1,
> +#      "vring-avail": 864415744,
> +#      "last-avail-idx": 373,
> +#      "queue-index": 0,
> +#      "vring-used": 864416320,
> +#      "shadow-avail-idx": 619,
> +#      "used-idx": 373,
> +#      "vring-num": 256,
> +#      "vring-align": 4096
> +#      }
> +#    }
> +#
> +##
> +
> +{ 'command': 'x-debug-virtio-queue-status',
> +  'data': { 'path': 'str', 'queue': 'uint16' },
> +  'returns': 'VirtQueueStatus'
> +}