[Virtio-fs] [RFC PATCH v3 20/20] hw/virtio: allow vhost-user-device to be driven by backend
Albert Esteve
aesteve at redhat.com
Fri Sep 1 10:00:44 UTC 2023
On Mon, Jul 10, 2023 at 6:44 PM Alex Bennée <alex.bennee at linaro.org> wrote:
> Instead of requiring all the information up front allow the
> vhost_dev_init to complete and then see what information we have from
> the backend.
>
> This does change the order around somewhat.
>
> Signed-off-by: Alex Bennée <alex.bennee at linaro.org>
> ---
> hw/virtio/vhost-user-device.c | 45 +++++++++++++++++++++++++----------
> 1 file changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/hw/virtio/vhost-user-device.c b/hw/virtio/vhost-user-device.c
> index 0109d4829d..b30b6265fb 100644
> --- a/hw/virtio/vhost-user-device.c
> +++ b/hw/virtio/vhost-user-device.c
> @@ -243,7 +243,6 @@ static void vub_device_realize(DeviceState *dev, Error
> **errp)
> {
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> VHostUserBase *vub = VHOST_USER_BASE(dev);
> - int ret;
>
> if (!vub->chardev.chr) {
> error_setg(errp, "vhost-user-device: missing chardev");
> @@ -254,13 +253,43 @@ static void vub_device_realize(DeviceState *dev,
> Error **errp)
> return;
> }
>
> + if (vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
> + VHOST_BACKEND_TYPE_USER, 0, errp)!=0) {
> + error_setg(errp, "vhost-user-device: unable to start connection");
> + return;
> + }
> +
> + if (vub->vhost_dev.specs.device_id) {
> + if (vub->virtio_id && vub->virtio_id !=
> vub->vhost_dev.specs.device_id) {
> + error_setg(errp, "vhost-user-device: backend id %d doesn't
> match cli %d",
> + vub->vhost_dev.specs.device_id, vub->virtio_id);
> + return;
> + }
> + vub->virtio_id = vub->vhost_dev.specs.device_id;
> + }
> +
> if (!vub->virtio_id) {
> - error_setg(errp, "vhost-user-device: need to define device id");
> + error_setg(errp, "vhost-user-device: need to define or be told
> device id");
> return;
> }
>
> + if (vub->vhost_dev.specs.min_vqs) {
> + if (vub->num_vqs) {
> + if (vub->num_vqs < vub->vhost_dev.specs.min_vqs ||
> + vub->num_vqs > vub->vhost_dev.specs.max_vqs) {
> + error_setg(errp,
> + "vhost-user-device: selected nvqs (%d) out of
> bounds (%d->%d)",
> + vub->num_vqs,
> + vub->vhost_dev.specs.min_vqs,
> vub->vhost_dev.specs.max_vqs);
> + return;
> + }
> + } else {
> + vub->num_vqs = vub->vhost_dev.specs.min_vqs;
> + }
> + }
> +
> if (!vub->num_vqs) {
> - vub->num_vqs = 1; /* reasonable default? */
> + error_setg(errp, "vhost-user-device: need to define number of
> vqs");
> }
>
> /*
> @@ -287,16 +316,6 @@ static void vub_device_realize(DeviceState *dev,
> Error **errp)
> virtio_add_queue(vdev, 4, vub_handle_output));
> }
>
> - vub->vhost_dev.nvqs = vub->num_vqs;
>
Who sets `vub->vhost_dev.nvqs` after removing this line?
Why having `vub->num_vqs` in the first place? In vub_start for example we
still
use `vub->vhost_dev.nvqs`, and we pass `vhost_dev` to other functions that
use its `nvqs`, so `num_vqs` is redundant and requires a logic to
copy/initialise `vhost_dev.nvqs`.
Maybe it would be better to initialse `nvqs` through a function, in the
device file, instead of doing:
`vub->num_vqs = 2;`
We could have:
`vub_set_nvqs(vub, 2);`
Or something along those lines. And the function will have all the internal
logic in this commit, i.e.,
checking the boundaries, setting the `vhost_dev.nvqs` value, printing the
error, etc.
So we can save the extra variable, and the logic to copy the value to the
device.
> -
> - /* connect to backend */
> - ret = vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
> - VHOST_BACKEND_TYPE_USER, 0, errp);
> -
> - if (ret < 0) {
> - do_vhost_user_cleanup(vdev, vub);
> - }
> -
> qemu_chr_fe_set_handlers(&vub->chardev, NULL, NULL, vub_event, NULL,
> dev, NULL, true);
> }
> --
> 2.39.2
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/virtio-fs/attachments/20230901/e1ca3dcf/attachment.htm>
More information about the Virtio-fs
mailing list