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: 1c3e5a2617 ("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>
---
- we can't check for get_config/set_config as the stack squashed vdev
- use vhost-user-state to transmit this
---
include/hw/virtio/vhost-user.h | 1 +
hw/scsi/vhost-user-scsi.c | 1 +
hw/virtio/vhost-user.c | 46 ++++++++++++++++++++++++----------
3 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index e44a41bb70..6e0e8a71a3 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -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);
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 1b2f7eed98..9be21d07ee 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -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) {
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index b27b8c56e2..6ce082861b 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -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;
@@ -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,
@@ -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");
--
2.30.2
On Mon, Mar 21, 2022 at 03:30:36PM +0000, Alex Bennée wrote:
> 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.
Could you give an example where the problem is encountered please?
> 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.
Hmm but are you sure? Most devices work mostly fine without CONFIG
messages, there's a chance a backend set this flag just in case
without much thought ...
> Fixes: 1c3e5a2617 ("vhost-user: back SET/GET_CONFIG requests with a protocol feature")
I'm not sure whether something is broken or this is a cleanup patch.
Fixes tag means "if you have 1c3e5a2617 you should pick this patch", so
cleanups don't need a fixes: tag.
> 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>
>
> ---
> - we can't check for get_config/set_config as the stack squashed vdev
> - use vhost-user-state to transmit this
> ---
> include/hw/virtio/vhost-user.h | 1 +
> hw/scsi/vhost-user-scsi.c | 1 +
> hw/virtio/vhost-user.c | 46 ++++++++++++++++++++++++----------
> 3 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
> index e44a41bb70..6e0e8a71a3 100644
> --- a/include/hw/virtio/vhost-user.h
> +++ b/include/hw/virtio/vhost-user.h
> @@ -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);
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index 1b2f7eed98..9be21d07ee 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -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) {
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index b27b8c56e2..6ce082861b 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -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;
>
> @@ -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,
> @@ -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");
> --
> 2.30.2
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Mon, Mar 21, 2022 at 03:30:36PM +0000, Alex Bennée wrote:
>> 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.
>
> Could you give an example where the problem is encountered please?
Well I only came across it when I realised by rpmb implementation wasn't
sending config updates to the vhost-user daemon because it only
implemented set/get_config methods.
I think vhost-user-scsi suffers from this but I'm not able to test it as
it's not clear how to do it. The vhost-user-scsi daemon want to attach
to real SCSI devices rather than a file for the block device. I guess I
need to somehow add "fake" scsi nodes to my host system which SCSI
commands can be passed to?
When was the last time vhost-user-scsi was tested with anything?
>> 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.
>
> Hmm but are you sure? Most devices work mostly fine without CONFIG
> messages, there's a chance a backend set this flag just in case
> without much thought ...
But that would be a bug right? Certainly a mismatch if something really
does want to see config messages. Again RPMB needs this because it's the
vhost-user daemon that knows the size of the device.
>> Fixes: 1c3e5a2617 ("vhost-user: back SET/GET_CONFIG requests with a protocol feature")
>
> I'm not sure whether something is broken or this is a cleanup patch.
> Fixes tag means "if you have 1c3e5a2617 you should pick this patch", so
> cleanups don't need a fixes: tag.
No I think it's broken, we just didn't notice because as you say most
devices don't need to care.
>
>
>> 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>
>>
>> ---
>> - we can't check for get_config/set_config as the stack squashed vdev
>> - use vhost-user-state to transmit this
>> ---
>> include/hw/virtio/vhost-user.h | 1 +
>> hw/scsi/vhost-user-scsi.c | 1 +
>> hw/virtio/vhost-user.c | 46 ++++++++++++++++++++++++----------
>> 3 files changed, 35 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
>> index e44a41bb70..6e0e8a71a3 100644
>> --- a/include/hw/virtio/vhost-user.h
>> +++ b/include/hw/virtio/vhost-user.h
>> @@ -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);
>> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
>> index 1b2f7eed98..9be21d07ee 100644
>> --- a/hw/scsi/vhost-user-scsi.c
>> +++ b/hw/scsi/vhost-user-scsi.c
>> @@ -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) {
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index b27b8c56e2..6ce082861b 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -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;
>>
>> @@ -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,
>> @@ -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");
>> --
>> 2.30.2
--
Alex Bennée
On Tue, Mar 22, 2022 at 03:54:32PM +0000, Alex Bennée wrote:
>
> "Michael S. Tsirkin" <mst@redhat.com> writes:
>
> > On Mon, Mar 21, 2022 at 03:30:36PM +0000, Alex Bennée wrote:
> >> 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.
> >
> > Could you give an example where the problem is encountered please?
>
> Well I only came across it when I realised by rpmb implementation wasn't
> sending config updates to the vhost-user daemon because it only
> implemented set/get_config methods.
>
> I think vhost-user-scsi suffers from this but I'm not able to test it as
> it's not clear how to do it. The vhost-user-scsi daemon want to attach
> to real SCSI devices rather than a file for the block device. I guess I
> need to somehow add "fake" scsi nodes to my host system which SCSI
> commands can be passed to?
>
> When was the last time vhost-user-scsi was tested with anything?
It's for passthrough mostly, yes. I think you can use tcm_loop
on top of loopback on top of a file. Didn't try myself.
> >> 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.
> >
> > Hmm but are you sure? Most devices work mostly fine without CONFIG
> > messages, there's a chance a backend set this flag just in case
> > without much thought ...
>
> But that would be a bug right? Certainly a mismatch if something really
> does want to see config messages. Again RPMB needs this because it's the
> vhost-user daemon that knows the size of the device.
Could you pls clarify how do you create a config that's broken with RPMB?
> >> Fixes: 1c3e5a2617 ("vhost-user: back SET/GET_CONFIG requests with a protocol feature")
> >
> > I'm not sure whether something is broken or this is a cleanup patch.
> > Fixes tag means "if you have 1c3e5a2617 you should pick this patch", so
> > cleanups don't need a fixes: tag.
>
> No I think it's broken, we just didn't notice because as you say most
> devices don't need to care.
I was mostly asking about the fixes tag. If no existing user cares then
it's best to avoid the fixes tag, it's a hint for backporters which
versions need the patch.
> >
> >
> >> 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>
> >>
> >> ---
> >> - we can't check for get_config/set_config as the stack squashed vdev
> >> - use vhost-user-state to transmit this
> >> ---
> >> include/hw/virtio/vhost-user.h | 1 +
> >> hw/scsi/vhost-user-scsi.c | 1 +
> >> hw/virtio/vhost-user.c | 46 ++++++++++++++++++++++++----------
> >> 3 files changed, 35 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
> >> index e44a41bb70..6e0e8a71a3 100644
> >> --- a/include/hw/virtio/vhost-user.h
> >> +++ b/include/hw/virtio/vhost-user.h
> >> @@ -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);
> >> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> >> index 1b2f7eed98..9be21d07ee 100644
> >> --- a/hw/scsi/vhost-user-scsi.c
> >> +++ b/hw/scsi/vhost-user-scsi.c
> >> @@ -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) {
> >> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >> index b27b8c56e2..6ce082861b 100644
> >> --- a/hw/virtio/vhost-user.c
> >> +++ b/hw/virtio/vhost-user.c
> >> @@ -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;
> >>
> >> @@ -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,
> >> @@ -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");
> >> --
> >> 2.30.2
>
>
> --
> Alex Bennée
© 2016 - 2026 Red Hat, Inc.