[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v3 04/29] vhost: Add x-vhost-enable-shadow-vq qmp
From: |
Eugenio Perez Martin |
Subject: |
Re: [RFC v3 04/29] vhost: Add x-vhost-enable-shadow-vq qmp |
Date: |
Mon, 24 May 2021 09:13:39 +0200 |
On Fri, May 21, 2021 at 9:05 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> Eugenio Pérez <eperezma@redhat.com> writes:
>
> > Command to enable shadow virtqueue looks like:
> >
> > { "execute": "x-vhost-enable-shadow-vq",
> > "arguments": { "name": "dev0", "enable": true } }
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> > qapi/net.json | 22 ++++++++++++++++++++++
> > hw/virtio/vhost.c | 6 ++++++
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/qapi/net.json b/qapi/net.json
> > index c31748c87f..660feafdd2 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -77,6 +77,28 @@
> > ##
> > { 'command': 'netdev_del', 'data': {'id': 'str'} }
> >
> > +##
> > +# @x-vhost-enable-shadow-vq:
> > +#
> > +# Use vhost shadow virtqueue.
> > +#
> > +# @name: the device name of the VirtIO device
> > +#
> > +# @enable: true to use he alternate shadow VQ notification path
>
> Typo "he".
>
Thanks, I will fix it!
> What's a "notification path", and why should I care?
>
> Maybe
>
> # @enable: Enable alternate shadow VQ notification
>
Your description is more accurate at some point of the series, I will fix it.
> > +#
> > +# Returns: Error if failure, or 'no error' for success. Not found if vhost
> > is not enabled.
>
> This is confusing. What do you mean by "Not found"?
>
> If you mean DeviceNotFound:
>
> 1. Not actually true: qmp_x_vhost_enable_shadow_vq() always fails with
> GenericError. Perhaps later patches will change that.
>
Right, I left the documentation in an intermediate state. At this
point it will always return failure, and in future ones it depends on
some conditions as you may have seen.
If I carry the QMP command to future series, I will update the doc
accordly to every commit.
> 2. Do you really need to distinguish "vhost is not enabled" from other
> errors?
>
SVQ cannot work if the device backend is not vhost, like qemu VirtIO
dev. What I meant is that "qemu will only look for its name in the set
of vhost devices, so you will have a device not found if the device is
not a vhost one", which may not be 100% clear at first glance. Maybe
this wording is better?
> > +#
> > +# Since: 6.1
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "x-vhost-enable-shadow-vq", "arguments": { "name":
> > "virtio-net", "enable": false } }
>
> Please break the long line, e.g. like this:
>
> # -> { "execute": "x-vhost-enable-shadow-vq",
> # "arguments": { "name": "virtio-net", "enable": false } }
>
> We normally show output in examples, too.
>
Ok, I will fix both issues.
Thanks!
> > +#
> > +##
> > +{ 'command': 'x-vhost-enable-shadow-vq',
> > + 'data': {'name': 'str', 'enable': 'bool'},
> > + 'if': 'defined(CONFIG_VHOST_KERNEL)' }
> > +
> > ##
> > # @NetLegacyNicOptions:
> > #
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 40f9f64ebd..c4c1f80661 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -15,6 +15,7 @@
> >
> > #include "qemu/osdep.h"
> > #include "qapi/error.h"
> > +#include "qapi/qapi-commands-net.h"
> > #include "hw/virtio/vhost.h"
> > #include "qemu/atomic.h"
> > #include "qemu/range.h"
> > @@ -1831,3 +1832,8 @@ int vhost_net_set_backend(struct vhost_dev *hdev,
> >
> > return -1;
> > }
> > +
> > +void qmp_x_vhost_enable_shadow_vq(const char *name, bool enable, Error
> > **errp)
> > +{
> > + error_setg(errp, "Shadow virtqueue still not implemented");
> > +}
>
- [RFC v3 00/29] vDPA software assisted live migration, Eugenio Pérez, 2021/05/19
- [RFC v3 01/29] virtio: Add virtio_queue_is_host_notifier_enabled, Eugenio Pérez, 2021/05/19
- [RFC v3 02/29] vhost: Save masked_notifier state, Eugenio Pérez, 2021/05/19
- [RFC v3 03/29] vhost: Add VhostShadowVirtqueue, Eugenio Pérez, 2021/05/19
- [RFC v3 05/29] virtio: Add VIRTIO_F_QUEUE_STATE, Eugenio Pérez, 2021/05/19
- [RFC v3 04/29] vhost: Add x-vhost-enable-shadow-vq qmp, Eugenio Pérez, 2021/05/19
- [RFC v3 06/29] virtio-net: Honor VIRTIO_CONFIG_S_DEVICE_STOPPED, Eugenio Pérez, 2021/05/19
- [RFC v3 07/29] vhost: Route guest->host notification through shadow virtqueue, Eugenio Pérez, 2021/05/19
- [RFC v3 08/29] vhost: Route host->guest notification through shadow virtqueue, Eugenio Pérez, 2021/05/19
- [RFC v3 09/29] vhost: Avoid re-set masked notifier in shadow vq, Eugenio Pérez, 2021/05/19
- [RFC v3 10/29] virtio: Add vhost_shadow_vq_get_vring_addr, Eugenio Pérez, 2021/05/19
- [RFC v3 11/29] vhost: Add vhost_vring_pause operation, Eugenio Pérez, 2021/05/19
- [RFC v3 12/29] vhost: add vhost_kernel_vring_pause, Eugenio Pérez, 2021/05/19
- [RFC v3 13/29] vhost: Add vhost_get_iova_range operation, Eugenio Pérez, 2021/05/19