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@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