Re: [PATCH 1/7] vhost: Add Error parameter to vhost_dev_init()
On Wed, Jun 09, 2021 at 05:46:52PM +0200, Kevin Wolf wrote:
> This allows callers to return better error messages instead of making
> one up while the real error ends up on stderr. Most callers can
> immediately make use of this because they already have an Error
> parameter themselves. The others just keep printing the error with
> error_report_err().
>
> Signed-off-by: Kevin Wolf
Reviewed-by: Raphael Norwitz
> ---
> include/hw/virtio/vhost.h| 2 +-
> backends/cryptodev-vhost.c | 5 -
> backends/vhost-user.c| 4 ++--
> hw/block/vhost-user-blk.c| 4 ++--
> hw/net/vhost_net.c | 6 +-
> hw/scsi/vhost-scsi.c | 4 +---
> hw/scsi/vhost-user-scsi.c| 4 +---
> hw/virtio/vhost-user-fs.c| 3 +--
> hw/virtio/vhost-user-vsock.c | 3 +--
> hw/virtio/vhost-vsock.c | 3 +--
> hw/virtio/vhost.c| 16 ++--
> 11 files changed, 29 insertions(+), 25 deletions(-)
>
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 21a9a52088..2d7aaad67b 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -104,7 +104,7 @@ struct vhost_net {
>
> int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> VhostBackendType backend_type,
> - uint32_t busyloop_timeout);
> + uint32_t busyloop_timeout, Error **errp);
> void vhost_dev_cleanup(struct vhost_dev *hdev);
> int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
> void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
> diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
> index 8231e7f1bc..bc13e466b4 100644
> --- a/backends/cryptodev-vhost.c
> +++ b/backends/cryptodev-vhost.c
> @@ -52,6 +52,7 @@ cryptodev_vhost_init(
> {
> int r;
> CryptoDevBackendVhost *crypto;
> +Error *local_err = NULL;
>
> crypto = g_new(CryptoDevBackendVhost, 1);
> crypto->dev.max_queues = 1;
> @@ -66,8 +67,10 @@ cryptodev_vhost_init(
> /* vhost-user needs vq_index to initiate a specific queue pair */
> crypto->dev.vq_index = crypto->cc->queue_index * crypto->dev.nvqs;
>
> -r = vhost_dev_init(&crypto->dev, options->opaque, options->backend_type,
> 0);
> +r = vhost_dev_init(&crypto->dev, options->opaque, options->backend_type,
> 0,
> + &local_err);
> if (r < 0) {
> +error_report_err(local_err);
> goto fail;
> }
>
> diff --git a/backends/vhost-user.c b/backends/vhost-user.c
> index b366610e16..10b39992d2 100644
> --- a/backends/vhost-user.c
> +++ b/backends/vhost-user.c
> @@ -48,9 +48,9 @@ vhost_user_backend_dev_init(VhostUserBackend *b,
> VirtIODevice *vdev,
> b->dev.nvqs = nvqs;
> b->dev.vqs = g_new0(struct vhost_virtqueue, nvqs);
>
> -ret = vhost_dev_init(&b->dev, &b->vhost_user, VHOST_BACKEND_TYPE_USER,
> 0);
> +ret = vhost_dev_init(&b->dev, &b->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
> + errp);
> if (ret < 0) {
> -error_setg_errno(errp, -ret, "vhost initialization failed");
> return -1;
> }
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index c6210fad0c..0cb56baefb 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -332,9 +332,9 @@ static int vhost_user_blk_connect(DeviceState *dev, Error
> **errp)
>
> vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>
> -ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER,
> 0);
> +ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
> + errp);
> if (ret < 0) {
> -error_setg_errno(errp, -ret, "vhost initialization failed");
> return ret;
> }
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 44c1ed92dc..447b119f85 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -22,6 +22,7 @@
> #include "standard-headers/linux/vhost_types.h"
> #include "hw/virtio/virtio-net.h"
> #include "net/vhost_net.h"
> +#include "qapi/error.h"
> #include "qemu/error-report.h"
> #include "qemu/main-loop.h"
>
> @@ -157,6 +158,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
> bool backend_kernel = options->backend_type == VHOST_BACKEND_TYPE_KERNEL;
> struct vhost_net *net = g_new0(struct vhost_net, 1);
> uint64_t features = 0;
> +Error *local_err = NULL;
>
> if (!options->net_backend) {
> fprintf(stderr, "vhost-net requires net backend to be setup\n");
> @@ -187,8 +189,10 @@ struct vhost_net *vhost_net_init(VhostNetOptions
> *options)
> }
>
> r = vhost_dev_init(&net->dev, options->opaque,
> - options->backend_type, options->busyloop_timeout);
> + options->backend_type, options->busyloop_timeout,
> + &local_err);
> if (r < 0) {
> +error_report_err(local_
Re: [PATCH 1/7] vhost: Add Error parameter to vhost_dev_init()
On Wed, Jun 09, 2021 at 05:46:52PM +0200, Kevin Wolf wrote: This allows callers to return better error messages instead of making one up while the real error ends up on stderr. Most callers can immediately make use of this because they already have an Error parameter themselves. The others just keep printing the error with error_report_err(). Signed-off-by: Kevin Wolf --- include/hw/virtio/vhost.h| 2 +- backends/cryptodev-vhost.c | 5 - backends/vhost-user.c| 4 ++-- hw/block/vhost-user-blk.c| 4 ++-- hw/net/vhost_net.c | 6 +- hw/scsi/vhost-scsi.c | 4 +--- hw/scsi/vhost-user-scsi.c| 4 +--- hw/virtio/vhost-user-fs.c| 3 +-- hw/virtio/vhost-user-vsock.c | 3 +-- hw/virtio/vhost-vsock.c | 3 +-- hw/virtio/vhost.c| 16 ++-- 11 files changed, 29 insertions(+), 25 deletions(-) Reviewed-by: Stefano Garzarella
[PATCH 1/7] vhost: Add Error parameter to vhost_dev_init()
This allows callers to return better error messages instead of making
one up while the real error ends up on stderr. Most callers can
immediately make use of this because they already have an Error
parameter themselves. The others just keep printing the error with
error_report_err().
Signed-off-by: Kevin Wolf
---
include/hw/virtio/vhost.h| 2 +-
backends/cryptodev-vhost.c | 5 -
backends/vhost-user.c| 4 ++--
hw/block/vhost-user-blk.c| 4 ++--
hw/net/vhost_net.c | 6 +-
hw/scsi/vhost-scsi.c | 4 +---
hw/scsi/vhost-user-scsi.c| 4 +---
hw/virtio/vhost-user-fs.c| 3 +--
hw/virtio/vhost-user-vsock.c | 3 +--
hw/virtio/vhost-vsock.c | 3 +--
hw/virtio/vhost.c| 16 ++--
11 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 21a9a52088..2d7aaad67b 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -104,7 +104,7 @@ struct vhost_net {
int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
VhostBackendType backend_type,
- uint32_t busyloop_timeout);
+ uint32_t busyloop_timeout, Error **errp);
void vhost_dev_cleanup(struct vhost_dev *hdev);
int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
index 8231e7f1bc..bc13e466b4 100644
--- a/backends/cryptodev-vhost.c
+++ b/backends/cryptodev-vhost.c
@@ -52,6 +52,7 @@ cryptodev_vhost_init(
{
int r;
CryptoDevBackendVhost *crypto;
+Error *local_err = NULL;
crypto = g_new(CryptoDevBackendVhost, 1);
crypto->dev.max_queues = 1;
@@ -66,8 +67,10 @@ cryptodev_vhost_init(
/* vhost-user needs vq_index to initiate a specific queue pair */
crypto->dev.vq_index = crypto->cc->queue_index * crypto->dev.nvqs;
-r = vhost_dev_init(&crypto->dev, options->opaque, options->backend_type,
0);
+r = vhost_dev_init(&crypto->dev, options->opaque, options->backend_type, 0,
+ &local_err);
if (r < 0) {
+error_report_err(local_err);
goto fail;
}
diff --git a/backends/vhost-user.c b/backends/vhost-user.c
index b366610e16..10b39992d2 100644
--- a/backends/vhost-user.c
+++ b/backends/vhost-user.c
@@ -48,9 +48,9 @@ vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice
*vdev,
b->dev.nvqs = nvqs;
b->dev.vqs = g_new0(struct vhost_virtqueue, nvqs);
-ret = vhost_dev_init(&b->dev, &b->vhost_user, VHOST_BACKEND_TYPE_USER, 0);
+ret = vhost_dev_init(&b->dev, &b->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
+ errp);
if (ret < 0) {
-error_setg_errno(errp, -ret, "vhost initialization failed");
return -1;
}
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index c6210fad0c..0cb56baefb 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -332,9 +332,9 @@ static int vhost_user_blk_connect(DeviceState *dev, Error
**errp)
vhost_dev_set_config_notifier(&s->dev, &blk_ops);
-ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0);
+ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
+ errp);
if (ret < 0) {
-error_setg_errno(errp, -ret, "vhost initialization failed");
return ret;
}
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 44c1ed92dc..447b119f85 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -22,6 +22,7 @@
#include "standard-headers/linux/vhost_types.h"
#include "hw/virtio/virtio-net.h"
#include "net/vhost_net.h"
+#include "qapi/error.h"
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
@@ -157,6 +158,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
bool backend_kernel = options->backend_type == VHOST_BACKEND_TYPE_KERNEL;
struct vhost_net *net = g_new0(struct vhost_net, 1);
uint64_t features = 0;
+Error *local_err = NULL;
if (!options->net_backend) {
fprintf(stderr, "vhost-net requires net backend to be setup\n");
@@ -187,8 +189,10 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
}
r = vhost_dev_init(&net->dev, options->opaque,
- options->backend_type, options->busyloop_timeout);
+ options->backend_type, options->busyloop_timeout,
+ &local_err);
if (r < 0) {
+error_report_err(local_err);
goto fail;
}
if (backend_kernel) {
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 4d70fa036b..8c611bfd2d 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -219,10 +219,8 @@ static void vhost_scsi_realize(DeviceState *dev, Error
**errp)
vsc->dev.backend_features = 0;
ret = vhost_d
