Skip to content

Commit

Permalink
vhost: Distinguish errors in vhost_backend_init()
Browse files Browse the repository at this point in the history 
Instead of just returning 0/-1 and letting the caller make up a
meaningless error message, add an Error parameter to allow reporting the
real error and switch to 0/-errno so that different kind of errors can
be distinguished in the caller.

Specifically, in vhost-user, EPROTO is used for all errors that relate
to the connection itself, whereas other error codes are used for errors
relating to the content of the connection. This will allow us later to
automatically reconnect when the connection goes away, without ending up
in an endless loop if it's a permanent error in the configuration.

Signed-off-by: Kevin Wolf <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Stefano Garzarella <[email protected]>
Reviewed-by: Raphael Norwitz <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
  • Loading branch information
@kevmw
kevmw committed Jun 30, 2021
1 parent a6945f2 commit 28770ff
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 29 deletions.
2 changes: 1 addition & 1 deletion hw/virtio/vhost-backend.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int request,
return ioctl(fd, request, arg);
}

static int vhost_kernel_init(struct vhost_dev *dev, void *opaque)
static int vhost_kernel_init(struct vhost_dev *dev, void *opaque, Error **errp)
{
assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL);

Expand Down
41 changes: 21 additions & 20 deletions hw/virtio/vhost-user.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1856,7 +1856,8 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
return 0;
}

static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque)
static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
Error **errp)
{
uint64_t features, protocol_features, ram_slots;
struct vhost_user *u;
Expand All @@ -1871,7 +1872,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque)

err = vhost_user_get_features(dev, &features);
if (err < 0) {
return err;
return -EPROTO;
}

if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
Expand All @@ -1880,7 +1881,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque)
err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
&protocol_features);
if (err < 0) {
return err;
return -EPROTO;
}

dev->protocol_features =
Expand All @@ -1891,27 +1892,27 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque)
dev->protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
} else if (!(protocol_features &
(1ULL << VHOST_USER_PROTOCOL_F_CONFIG))) {
error_report("Device expects VHOST_USER_PROTOCOL_F_CONFIG "
"but backend does not support it.");
return -1;
error_setg(errp, "Device expects VHOST_USER_PROTOCOL_F_CONFIG "
"but backend does not support it.");
return -EINVAL;
}

err = vhost_user_set_protocol_features(dev, dev->protocol_features);
if (err < 0) {
return err;
return -EPROTO;
}

/* query the max queues we support if backend supports Multiple Queue */
if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)) {
err = vhost_user_get_u64(dev, VHOST_USER_GET_QUEUE_NUM,
&dev->max_queues);
if (err < 0) {
return err;
return -EPROTO;
}
}
if (dev->num_queues && dev->max_queues < dev->num_queues) {
error_report("The maximum number of queues supported by the "
"backend is %" PRIu64, dev->max_queues);
error_setg(errp, "The maximum number of queues supported by the "
"backend is %" PRIu64, dev->max_queues);
return -EINVAL;
}

Expand All @@ -1920,9 +1921,9 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque)
VHOST_USER_PROTOCOL_F_SLAVE_REQ) &&
virtio_has_feature(dev->protocol_features,
VHOST_USER_PROTOCOL_F_REPLY_ACK))) {
error_report("IOMMU support requires reply-ack and "
"slave-req protocol features.");
return -1;
error_setg(errp, "IOMMU support requires reply-ack and "
"slave-req protocol features.");
return -EINVAL;
}

/* get max memory regions if backend supports configurable RAM slots */
Expand All @@ -1932,15 +1933,15 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque)
} else {
err = vhost_user_get_max_memslots(dev, &ram_slots);
if (err < 0) {
return err;
return -EPROTO;
}

if (ram_slots < u->user->memory_slots) {
error_report("The backend specified a max ram slots limit "
"of %" PRIu64", when the prior validated limit was %d. "
"This limit should never decrease.", ram_slots,
u->user->memory_slots);
return -1;
error_setg(errp, "The backend specified a max ram slots limit "
"of %" PRIu64", when the prior validated limit was "
"%d. This limit should never decrease.", ram_slots,
u->user->memory_slots);
return -EINVAL;
}

u->user->memory_slots = MIN(ram_slots, VHOST_USER_MAX_RAM_SLOTS);
Expand All @@ -1958,7 +1959,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque)
if (dev->vq_index == 0) {
err = vhost_setup_slave_channel(dev);
if (err < 0) {
return err;
return -EPROTO;
}
}

Expand Down
2 changes: 1 addition & 1 deletion hw/virtio/vhost-vdpa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status)
vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &s);
}

static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque)
static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
{
struct vhost_vdpa *v;
assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA);
Expand Down
13 changes: 7 additions & 6 deletions hw/virtio/vhost.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1289,19 +1289,21 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
VhostBackendType backend_type, uint32_t busyloop_timeout,
Error **errp)
{
ERRP_GUARD();
uint64_t features;
int i, r, n_initialized_vqs = 0;
Error *local_err = NULL;

hdev->vdev = NULL;
hdev->migration_blocker = NULL;

r = vhost_set_backend_type(hdev, backend_type);
assert(r >= 0);

r = hdev->vhost_ops->vhost_backend_init(hdev, opaque);
r = hdev->vhost_ops->vhost_backend_init(hdev, opaque, errp);
if (r < 0) {
error_setg(errp, "vhost_backend_init failed");
if (!*errp) {
error_setg_errno(errp, -r, "vhost_backend_init failed");
}
goto fail;
}

Expand Down Expand Up @@ -1369,9 +1371,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
}

if (hdev->migration_blocker != NULL) {
r = migrate_add_blocker(hdev->migration_blocker, &local_err);
if (local_err) {
error_propagate(errp, local_err);
r = migrate_add_blocker(hdev->migration_blocker, errp);
if (*errp) {
error_free(hdev->migration_blocker);
goto fail_busyloop;
}
Expand Down
3 changes: 2 additions & 1 deletion include/hw/virtio/vhost-backend.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ struct vhost_scsi_target;
struct vhost_iotlb_msg;
struct vhost_virtqueue;

typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque);
typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque,
Error **errp);
typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev);
typedef int (*vhost_backend_memslots_limit)(struct vhost_dev *dev);

Expand Down

0 comments on commit 28770ff

Please sign in to comment.