Skip to content

Commit

Permalink
hw/virtio/vhost-user: don't suppress F_CONFIG when supported
Browse files Browse the repository at this point in the history
Previously we would silently suppress VHOST_USER_PROTOCOL_F_CONFIG
during the protocol negotiation if the QEMU stub hadn't implemented
the vhost_dev_config_notifier. However this isn't the only way we can
handle config messages, the existing vdc->get/set_config can do this
as well.

Lightly re-factor the code to check for both potential methods and
instead of silently squashing the feature error out. It is unlikely
that a vhost-user backend expecting to handle CONFIG messages will
behave correctly if they never get sent.

Fixes: 1c3e5a2 ("vhost-user: back SET/GET_CONFIG requests with a protocol feature")
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Message-Id: <20220321153037.3622127-13-alex.bennee@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
  • Loading branch information
stsquad authored and mstsirkin committed May 16, 2022
1 parent 2735199 commit 5653493
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 13 deletions.
1 change: 1 addition & 0 deletions hw/scsi/vhost-user-scsi.c
Expand Up @@ -121,6 +121,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
vsc->dev.backend_features = 0;
vqs = vsc->dev.vqs;

s->vhost_user.supports_config = true;
ret = vhost_dev_init(&vsc->dev, &s->vhost_user,
VHOST_BACKEND_TYPE_USER, 0, errp);
if (ret < 0) {
Expand Down
46 changes: 33 additions & 13 deletions hw/virtio/vhost-user.c
Expand Up @@ -1949,14 +1949,15 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
Error **errp)
{
uint64_t features, protocol_features, ram_slots;
uint64_t features, ram_slots;
struct vhost_user *u;
VhostUserState *vus = (VhostUserState *) opaque;
int err;

assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);

u = g_new0(struct vhost_user, 1);
u->user = opaque;
u->user = vus;
u->dev = dev;
dev->opaque = u;

Expand All @@ -1967,6 +1968,10 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
}

if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
bool supports_f_config = vus->supports_config ||
(dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
uint64_t protocol_features;

dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;

err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
Expand All @@ -1976,19 +1981,34 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
return -EPROTO;
}

dev->protocol_features =
protocol_features & VHOST_USER_PROTOCOL_FEATURE_MASK;

if (!dev->config_ops || !dev->config_ops->vhost_dev_config_notifier) {
/* Don't acknowledge CONFIG feature if device doesn't support it */
dev->protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
} else if (!(protocol_features &
(1ULL << VHOST_USER_PROTOCOL_F_CONFIG))) {
error_setg(errp, "Device expects VHOST_USER_PROTOCOL_F_CONFIG "
"but backend does not support it.");
return -EINVAL;
/*
* We will use all the protocol features we support - although
* we suppress F_CONFIG if we know QEMUs internal code can not support
* it.
*/
protocol_features &= VHOST_USER_PROTOCOL_FEATURE_MASK;

if (supports_f_config) {
if (!virtio_has_feature(protocol_features,
VHOST_USER_PROTOCOL_F_CONFIG)) {
error_setg(errp, "vhost-user device %s expecting "
"VHOST_USER_PROTOCOL_F_CONFIG but the vhost-user backend does "
"not support it.", dev->vdev->name);
return -EPROTO;
}
} else {
if (virtio_has_feature(protocol_features,
VHOST_USER_PROTOCOL_F_CONFIG)) {
warn_reportf_err(*errp, "vhost-user backend supports "
"VHOST_USER_PROTOCOL_F_CONFIG for "
"device %s but QEMU does not.",
dev->vdev->name);
protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG);
}
}

/* final set of protocol features */
dev->protocol_features = protocol_features;
err = vhost_user_set_protocol_features(dev, dev->protocol_features);
if (err < 0) {
error_setg_errno(errp, EPROTO, "vhost_backend_init failed");
Expand Down
1 change: 1 addition & 0 deletions include/hw/virtio/vhost-user.h
Expand Up @@ -22,6 +22,7 @@ typedef struct VhostUserState {
CharBackend *chr;
VhostUserHostNotifier notifier[VIRTIO_QUEUE_MAX];
int memory_slots;
bool supports_config;
} VhostUserState;

bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp);
Expand Down

0 comments on commit 5653493

Please sign in to comment.