vhost-user-snd: Use virtio_get_config_size()

[PATCH] vhost-user-snd: Use virtio_get_config_size()

Posted by Matias Ezequiel Vara Larsen 12 months ago

Use virtio_get_config_size() rather than sizeof(struct
virtio_snd_config) for the config_size in the vhost-user-snd frontend.
The frontend shall rely on device features for the size of the device
configuration space. This fixes an issue introduced by commit ab0c7fb2
in which the optional field `control` is added to the virtio_snd_config
structure. This breaks vhost-user-device backends that do not implement
the `controls` field.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2805
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
---
 hw/virtio/vhost-user-snd.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-user-snd.c b/hw/virtio/vhost-user-snd.c
index 8610370af8..8da4309470 100644
--- a/hw/virtio/vhost-user-snd.c
+++ b/hw/virtio/vhost-user-snd.c
@@ -16,6 +16,18 @@
 #include "standard-headers/linux/virtio_ids.h"
 #include "standard-headers/linux/virtio_snd.h"
 
+static const VirtIOFeature feature_sizes[] = {
+    {.flags = 1ULL << VIRTIO_SND_F_CTLS,
+    .end = endof(struct virtio_snd_config, controls)},
+    {}
+};
+
+static const VirtIOConfigSizeParams cfg_size_params = {
+    .min_size = endof(struct virtio_snd_config, chmaps),
+    .max_size = sizeof(struct virtio_snd_config),
+    .feature_sizes = feature_sizes
+};
+
 static const VMStateDescription vu_snd_vmstate = {
     .name = "vhost-user-snd",
     .unmigratable = 1,
@@ -23,16 +35,20 @@ static const VMStateDescription vu_snd_vmstate = {
 
 static const Property vsnd_properties[] = {
     DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
+    DEFINE_PROP_BIT64("config-controls", VHostUserBase,
+                      parent_obj.host_features, VIRTIO_SND_F_CTLS, false),
 };
 
 static void vu_snd_base_realize(DeviceState *dev, Error **errp)
 {
     VHostUserBase *vub = VHOST_USER_BASE(dev);
     VHostUserBaseClass *vubs = VHOST_USER_BASE_GET_CLASS(dev);
+    VirtIODevice *vdev = &vub->parent_obj;
 
     vub->virtio_id = VIRTIO_ID_SOUND;
     vub->num_vqs = 4;
-    vub->config_size = sizeof(struct virtio_snd_config);
+    vub->config_size = virtio_get_config_size(&cfg_size_params,
+                                              vdev->host_features);
     vub->vq_size = 64;
 
     vubs->parent_realize(dev, errp);
-- 
2.42.0

Re: [PATCH] vhost-user-snd: Use virtio_get_config_size()

Posted by Stefano Garzarella 12 months ago

For the title, what about this?

   vhost-user-snd: fix incorrect config_size computation

Or something like that, just to make clear that we are fixing a
real issue.

On Thu, Feb 13, 2025 at 02:25:13PM +0100, Matias Ezequiel Vara Larsen 
wrote:
>Use virtio_get_config_size() rather than sizeof(struct
>virtio_snd_config) for the config_size in the vhost-user-snd frontend.
>The frontend shall rely on device features for the size of the device
>configuration space. This fixes an issue introduced by commit ab0c7fb2

When we refer to a commit it's a good practice to put both the sha1, but
also the title, like this:
This fixes an issue introduced by commit ab0c7fb22b ("linux-headers:
update to current kvm/next") ...

>in which the optional field `control` is added to the virtio_snd_config

s/control/controls

I would also specify here that the presence of `controls` in the config
space depends on VIRTIO_SND_F_CTLS, citing the specification:

5.14.4 Device Configuration Layout
  ...
  controls
     (driver-read-only) indicates a total number of all available control
     elements if VIRTIO_SND_F_CTLS has been negotiated.

>structure. This breaks vhost-user-device backends that do not implement
>the `controls` field.
>

I'd suggest to add the fixes tag:

Fixes: ab0c7fb22b ("linux-headers: update to current kvm/next")

And maybe also:

Cc: qemu-stable@nongnu.org

>Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2805
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
>---
> hw/virtio/vhost-user-snd.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
>diff --git a/hw/virtio/vhost-user-snd.c b/hw/virtio/vhost-user-snd.c
>index 8610370af8..8da4309470 100644
>--- a/hw/virtio/vhost-user-snd.c
>+++ b/hw/virtio/vhost-user-snd.c
>@@ -16,6 +16,18 @@
> #include "standard-headers/linux/virtio_ids.h"
> #include "standard-headers/linux/virtio_snd.h"
>
>+static const VirtIOFeature feature_sizes[] = {
>+    {.flags = 1ULL << VIRTIO_SND_F_CTLS,
>+    .end = endof(struct virtio_snd_config, controls)},
>+    {}
>+};
>+
>+static const VirtIOConfigSizeParams cfg_size_params = {
>+    .min_size = endof(struct virtio_snd_config, chmaps),
>+    .max_size = sizeof(struct virtio_snd_config),
>+    .feature_sizes = feature_sizes
>+};
>+
> static const VMStateDescription vu_snd_vmstate = {
>     .name = "vhost-user-snd",
>     .unmigratable = 1,
>@@ -23,16 +35,20 @@ static const VMStateDescription vu_snd_vmstate = {
>
> static const Property vsnd_properties[] = {
>     DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
>+    DEFINE_PROP_BIT64("config-controls", VHostUserBase,

In almost all other virtio/vhost-user devices, the property name does 
not have the prefix `config-`, but usually the thing after F_, in this 
case CTLS is cryptic, so IMO just `controls` should be fine.

The only example I found is `config-wce` for vhost-user-blk, but in that 
case the feature is actually called VIRTIO_BLK_F_CONFIG_WCE.

Thanks,
Stefano

>+                      parent_obj.host_features, VIRTIO_SND_F_CTLS, false),
> };
>
> static void vu_snd_base_realize(DeviceState *dev, Error **errp)
> {
>     VHostUserBase *vub = VHOST_USER_BASE(dev);
>     VHostUserBaseClass *vubs = VHOST_USER_BASE_GET_CLASS(dev);
>+    VirtIODevice *vdev = &vub->parent_obj;
>
>     vub->virtio_id = VIRTIO_ID_SOUND;
>     vub->num_vqs = 4;
>-    vub->config_size = sizeof(struct virtio_snd_config);
>+    vub->config_size = virtio_get_config_size(&cfg_size_params,
>+                                              vdev->host_features);
>     vub->vq_size = 64;
>
>     vubs->parent_realize(dev, errp);
>-- 
>2.42.0
>

Re: [PATCH] vhost-user-snd: Use virtio_get_config_size()

Posted by Matias Ezequiel Vara Larsen 11 months, 4 weeks ago

On Thu, Feb 13, 2025 at 04:31:41PM +0100, Stefano Garzarella wrote:
> For the title, what about this?
> 
>   vhost-user-snd: fix incorrect config_size computation
> 
> Or something like that, just to make clear that we are fixing a
> real issue.
> 
> On Thu, Feb 13, 2025 at 02:25:13PM +0100, Matias Ezequiel Vara Larsen wrote:
> > Use virtio_get_config_size() rather than sizeof(struct
> > virtio_snd_config) for the config_size in the vhost-user-snd frontend.
> > The frontend shall rely on device features for the size of the device
> > configuration space. This fixes an issue introduced by commit ab0c7fb2
> 
> When we refer to a commit it's a good practice to put both the sha1, but
> also the title, like this:
> This fixes an issue introduced by commit ab0c7fb22b ("linux-headers:
> update to current kvm/next") ...
> 
> > in which the optional field `control` is added to the virtio_snd_config
> 
> s/control/controls
> 
> I would also specify here that the presence of `controls` in the config
> space depends on VIRTIO_SND_F_CTLS, citing the specification:
> 
> 5.14.4 Device Configuration Layout
>  ...
>  controls
>     (driver-read-only) indicates a total number of all available control
>     elements if VIRTIO_SND_F_CTLS has been negotiated.
> 
> > structure. This breaks vhost-user-device backends that do not implement
> > the `controls` field.
> > 
> 
> I'd suggest to add the fixes tag:
> 
> Fixes: ab0c7fb22b ("linux-headers: update to current kvm/next")
> 
> And maybe also:
> 
> Cc: qemu-stable@nongnu.org
> 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2805
> > Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> > Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > ---
> > hw/virtio/vhost-user-snd.c | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/virtio/vhost-user-snd.c b/hw/virtio/vhost-user-snd.c
> > index 8610370af8..8da4309470 100644
> > --- a/hw/virtio/vhost-user-snd.c
> > +++ b/hw/virtio/vhost-user-snd.c
> > @@ -16,6 +16,18 @@
> > #include "standard-headers/linux/virtio_ids.h"
> > #include "standard-headers/linux/virtio_snd.h"
> > 
> > +static const VirtIOFeature feature_sizes[] = {
> > +    {.flags = 1ULL << VIRTIO_SND_F_CTLS,
> > +    .end = endof(struct virtio_snd_config, controls)},
> > +    {}
> > +};
> > +
> > +static const VirtIOConfigSizeParams cfg_size_params = {
> > +    .min_size = endof(struct virtio_snd_config, chmaps),
> > +    .max_size = sizeof(struct virtio_snd_config),
> > +    .feature_sizes = feature_sizes
> > +};
> > +
> > static const VMStateDescription vu_snd_vmstate = {
> >     .name = "vhost-user-snd",
> >     .unmigratable = 1,
> > @@ -23,16 +35,20 @@ static const VMStateDescription vu_snd_vmstate = {
> > 
> > static const Property vsnd_properties[] = {
> >     DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
> > +    DEFINE_PROP_BIT64("config-controls", VHostUserBase,
> 
> In almost all other virtio/vhost-user devices, the property name does not
> have the prefix `config-`, but usually the thing after F_, in this case CTLS
> is cryptic, so IMO just `controls` should be fine.
> 
> The only example I found is `config-wce` for vhost-user-blk, but in that
> case the feature is actually called VIRTIO_BLK_F_CONFIG_WCE.
> 

Thanks for the review. I will address all the comments in the next
version.

Matias

Re: [PATCH] vhost-user-snd: Use virtio_get_config_size()

Posted by Stefano Garzarella 12 months ago

Unrelated to this patch, but since we are talking about
VIRTIO_SND_F_CTLS, I think it would be good to send a patch to Linux to
make it clear that `controls` depends on VIRTIO_SND_F_CTLS.

For example in linux/include/uapi/linux/virtio_blk.h we have:

struct virtio_blk_config {
	/* The capacity (in 512-byte sectors). */
	__virtio64 capacity;
	/* The maximum segment size (if VIRTIO_BLK_F_SIZE_MAX) */
	__virtio32 size_max;
	/* The maximum number of segments (if VIRTIO_BLK_F_SEG_MAX) */
	__virtio32 seg_max;
	....

So I suggest something like this:

diff --git a/include/uapi/linux/virtio_snd.h b/include/uapi/linux/virtio_snd.h
index 5f4100c2cf04..a4cfb9f6561a 100644
--- a/include/uapi/linux/virtio_snd.h
+++ b/include/uapi/linux/virtio_snd.h
@@ -25,7 +25,7 @@ struct virtio_snd_config {
        __le32 streams;
        /* # of available channel maps */
        __le32 chmaps;
-       /* # of available control elements */
+       /* # of available control elements (if VIRTIO_SND_F_CTLS) */
        __le32 controls;
 };

Thanks,
Stefano


On Thu, 13 Feb 2025 at 16:31, Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> For the title, what about this?
>
>    vhost-user-snd: fix incorrect config_size computation
>
> Or something like that, just to make clear that we are fixing a
> real issue.
>
> On Thu, Feb 13, 2025 at 02:25:13PM +0100, Matias Ezequiel Vara Larsen
> wrote:
> >Use virtio_get_config_size() rather than sizeof(struct
> >virtio_snd_config) for the config_size in the vhost-user-snd frontend.
> >The frontend shall rely on device features for the size of the device
> >configuration space. This fixes an issue introduced by commit ab0c7fb2
>
> When we refer to a commit it's a good practice to put both the sha1, but
> also the title, like this:
> This fixes an issue introduced by commit ab0c7fb22b ("linux-headers:
> update to current kvm/next") ...
>
> >in which the optional field `control` is added to the virtio_snd_config
>
> s/control/controls
>
> I would also specify here that the presence of `controls` in the config
> space depends on VIRTIO_SND_F_CTLS, citing the specification:
>
> 5.14.4 Device Configuration Layout
>   ...
>   controls
>      (driver-read-only) indicates a total number of all available control
>      elements if VIRTIO_SND_F_CTLS has been negotiated.
>
> >structure. This breaks vhost-user-device backends that do not implement
> >the `controls` field.
> >
>
> I'd suggest to add the fixes tag:
>
> Fixes: ab0c7fb22b ("linux-headers: update to current kvm/next")
>
> And maybe also:
>
> Cc: qemu-stable@nongnu.org
>
> >Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2805
> >Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> >Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> >---
> > hw/virtio/vhost-user-snd.c | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/virtio/vhost-user-snd.c b/hw/virtio/vhost-user-snd.c
> >index 8610370af8..8da4309470 100644
> >--- a/hw/virtio/vhost-user-snd.c
> >+++ b/hw/virtio/vhost-user-snd.c
> >@@ -16,6 +16,18 @@
> > #include "standard-headers/linux/virtio_ids.h"
> > #include "standard-headers/linux/virtio_snd.h"
> >
> >+static const VirtIOFeature feature_sizes[] = {
> >+    {.flags = 1ULL << VIRTIO_SND_F_CTLS,
> >+    .end = endof(struct virtio_snd_config, controls)},
> >+    {}
> >+};
> >+
> >+static const VirtIOConfigSizeParams cfg_size_params = {
> >+    .min_size = endof(struct virtio_snd_config, chmaps),
> >+    .max_size = sizeof(struct virtio_snd_config),
> >+    .feature_sizes = feature_sizes
> >+};
> >+
> > static const VMStateDescription vu_snd_vmstate = {
> >     .name = "vhost-user-snd",
> >     .unmigratable = 1,
> >@@ -23,16 +35,20 @@ static const VMStateDescription vu_snd_vmstate = {
> >
> > static const Property vsnd_properties[] = {
> >     DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
> >+    DEFINE_PROP_BIT64("config-controls", VHostUserBase,
>
> In almost all other virtio/vhost-user devices, the property name does
> not have the prefix `config-`, but usually the thing after F_, in this
> case CTLS is cryptic, so IMO just `controls` should be fine.
>
> The only example I found is `config-wce` for vhost-user-blk, but in that
> case the feature is actually called VIRTIO_BLK_F_CONFIG_WCE.
>
> Thanks,
> Stefano
>
> >+                      parent_obj.host_features, VIRTIO_SND_F_CTLS, false),
> > };
> >
> > static void vu_snd_base_realize(DeviceState *dev, Error **errp)
> > {
> >     VHostUserBase *vub = VHOST_USER_BASE(dev);
> >     VHostUserBaseClass *vubs = VHOST_USER_BASE_GET_CLASS(dev);
> >+    VirtIODevice *vdev = &vub->parent_obj;
> >
> >     vub->virtio_id = VIRTIO_ID_SOUND;
> >     vub->num_vqs = 4;
> >-    vub->config_size = sizeof(struct virtio_snd_config);
> >+    vub->config_size = virtio_get_config_size(&cfg_size_params,
> >+                                              vdev->host_features);
> >     vub->vq_size = 64;
> >
> >     vubs->parent_realize(dev, errp);
> >--
> >2.42.0
> >

Re: [PATCH] vhost-user-snd: Use virtio_get_config_size()

Posted by Stefano Garzarella 12 months ago

On Thu, 13 Feb 2025 at 16:43, Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> Unrelated to this patch, but since we are talking about
> VIRTIO_SND_F_CTLS, I think it would be good to send a patch to Linux to
> make it clear that `controls` depends on VIRTIO_SND_F_CTLS.

Patch posted here:
https://lore.kernel.org/virtualization/20250213161825.139952-1-sgarzare@redhat.com/T/#u

Stefano

>
> For example in linux/include/uapi/linux/virtio_blk.h we have:
>
> struct virtio_blk_config {
>         /* The capacity (in 512-byte sectors). */
>         __virtio64 capacity;
>         /* The maximum segment size (if VIRTIO_BLK_F_SIZE_MAX) */
>         __virtio32 size_max;
>         /* The maximum number of segments (if VIRTIO_BLK_F_SEG_MAX) */
>         __virtio32 seg_max;
>         ....
>
> So I suggest something like this:
>
> diff --git a/include/uapi/linux/virtio_snd.h b/include/uapi/linux/virtio_snd.h
> index 5f4100c2cf04..a4cfb9f6561a 100644
> --- a/include/uapi/linux/virtio_snd.h
> +++ b/include/uapi/linux/virtio_snd.h
> @@ -25,7 +25,7 @@ struct virtio_snd_config {
>         __le32 streams;
>         /* # of available channel maps */
>         __le32 chmaps;
> -       /* # of available control elements */
> +       /* # of available control elements (if VIRTIO_SND_F_CTLS) */
>         __le32 controls;
>  };
>
> Thanks,
> Stefano
>
>
> On Thu, 13 Feb 2025 at 16:31, Stefano Garzarella <sgarzare@redhat.com> wrote:
> >
> > For the title, what about this?
> >
> >    vhost-user-snd: fix incorrect config_size computation
> >
> > Or something like that, just to make clear that we are fixing a
> > real issue.
> >
> > On Thu, Feb 13, 2025 at 02:25:13PM +0100, Matias Ezequiel Vara Larsen
> > wrote:
> > >Use virtio_get_config_size() rather than sizeof(struct
> > >virtio_snd_config) for the config_size in the vhost-user-snd frontend.
> > >The frontend shall rely on device features for the size of the device
> > >configuration space. This fixes an issue introduced by commit ab0c7fb2
> >
> > When we refer to a commit it's a good practice to put both the sha1, but
> > also the title, like this:
> > This fixes an issue introduced by commit ab0c7fb22b ("linux-headers:
> > update to current kvm/next") ...
> >
> > >in which the optional field `control` is added to the virtio_snd_config
> >
> > s/control/controls
> >
> > I would also specify here that the presence of `controls` in the config
> > space depends on VIRTIO_SND_F_CTLS, citing the specification:
> >
> > 5.14.4 Device Configuration Layout
> >   ...
> >   controls
> >      (driver-read-only) indicates a total number of all available control
> >      elements if VIRTIO_SND_F_CTLS has been negotiated.
> >
> > >structure. This breaks vhost-user-device backends that do not implement
> > >the `controls` field.
> > >
> >
> > I'd suggest to add the fixes tag:
> >
> > Fixes: ab0c7fb22b ("linux-headers: update to current kvm/next")
> >
> > And maybe also:
> >
> > Cc: qemu-stable@nongnu.org
> >
> > >Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2805
> > >Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> > >Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > >---
> > > hw/virtio/vhost-user-snd.c | 18 +++++++++++++++++-
> > > 1 file changed, 17 insertions(+), 1 deletion(-)
> > >
> > >diff --git a/hw/virtio/vhost-user-snd.c b/hw/virtio/vhost-user-snd.c
> > >index 8610370af8..8da4309470 100644
> > >--- a/hw/virtio/vhost-user-snd.c
> > >+++ b/hw/virtio/vhost-user-snd.c
> > >@@ -16,6 +16,18 @@
> > > #include "standard-headers/linux/virtio_ids.h"
> > > #include "standard-headers/linux/virtio_snd.h"
> > >
> > >+static const VirtIOFeature feature_sizes[] = {
> > >+    {.flags = 1ULL << VIRTIO_SND_F_CTLS,
> > >+    .end = endof(struct virtio_snd_config, controls)},
> > >+    {}
> > >+};
> > >+
> > >+static const VirtIOConfigSizeParams cfg_size_params = {
> > >+    .min_size = endof(struct virtio_snd_config, chmaps),
> > >+    .max_size = sizeof(struct virtio_snd_config),
> > >+    .feature_sizes = feature_sizes
> > >+};
> > >+
> > > static const VMStateDescription vu_snd_vmstate = {
> > >     .name = "vhost-user-snd",
> > >     .unmigratable = 1,
> > >@@ -23,16 +35,20 @@ static const VMStateDescription vu_snd_vmstate = {
> > >
> > > static const Property vsnd_properties[] = {
> > >     DEFINE_PROP_CHR("chardev", VHostUserBase, chardev),
> > >+    DEFINE_PROP_BIT64("config-controls", VHostUserBase,
> >
> > In almost all other virtio/vhost-user devices, the property name does
> > not have the prefix `config-`, but usually the thing after F_, in this
> > case CTLS is cryptic, so IMO just `controls` should be fine.
> >
> > The only example I found is `config-wce` for vhost-user-blk, but in that
> > case the feature is actually called VIRTIO_BLK_F_CONFIG_WCE.
> >
> > Thanks,
> > Stefano
> >
> > >+                      parent_obj.host_features, VIRTIO_SND_F_CTLS, false),
> > > };
> > >
> > > static void vu_snd_base_realize(DeviceState *dev, Error **errp)
> > > {
> > >     VHostUserBase *vub = VHOST_USER_BASE(dev);
> > >     VHostUserBaseClass *vubs = VHOST_USER_BASE_GET_CLASS(dev);
> > >+    VirtIODevice *vdev = &vub->parent_obj;
> > >
> > >     vub->virtio_id = VIRTIO_ID_SOUND;
> > >     vub->num_vqs = 4;
> > >-    vub->config_size = sizeof(struct virtio_snd_config);
> > >+    vub->config_size = virtio_get_config_size(&cfg_size_params,
> > >+                                              vdev->host_features);
> > >     vub->vq_size = 64;
> > >
> > >     vubs->parent_realize(dev, errp);
> > >--
> > >2.42.0
> > >

Re: [PATCH] vhost-user-snd: Use virtio_get_config_size()

Posted by Matias Ezequiel Vara Larsen 11 months, 4 weeks ago

On Thu, Feb 13, 2025 at 05:22:22PM +0100, Stefano Garzarella wrote:
> On Thu, 13 Feb 2025 at 16:43, Stefano Garzarella <sgarzare@redhat.com> wrote:
> >
> > Unrelated to this patch, but since we are talking about
> > VIRTIO_SND_F_CTLS, I think it would be good to send a patch to Linux to
> > make it clear that `controls` depends on VIRTIO_SND_F_CTLS.
> 
> Patch posted here:
> https://lore.kernel.org/virtualization/20250213161825.139952-1-sgarzare@redhat.com/T/#u
> 
> Stefano

Thanks!

Re: [PATCH] vhost-user-snd: Use virtio_get_config_size()

Posted by Philippe Mathieu-Daudé 12 months ago

On 13/2/25 14:25, Matias Ezequiel Vara Larsen wrote:
> Use virtio_get_config_size() rather than sizeof(struct
> virtio_snd_config) for the config_size in the vhost-user-snd frontend.
> The frontend shall rely on device features for the size of the device
> configuration space. This fixes an issue introduced by commit ab0c7fb2
> in which the optional field `control` is added to the virtio_snd_config
> structure. This breaks vhost-user-device backends that do not implement
> the `controls` field.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2805
> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> ---
>   hw/virtio/vhost-user-snd.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)

¡¡Bien ahí!!

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] vhost-user-snd: Use virtio_get_config_size()

Posted by Matias Ezequiel Vara Larsen 11 months, 4 weeks ago

On Thu, Feb 13, 2025 at 04:07:47PM +0100, Philippe Mathieu-Daudé wrote:
> On 13/2/25 14:25, Matias Ezequiel Vara Larsen wrote:
> > Use virtio_get_config_size() rather than sizeof(struct
> > virtio_snd_config) for the config_size in the vhost-user-snd frontend.
> > The frontend shall rely on device features for the size of the device
> > configuration space. This fixes an issue introduced by commit ab0c7fb2
> > in which the optional field `control` is added to the virtio_snd_config
> > structure. This breaks vhost-user-device backends that do not implement
> > the `controls` field.
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2805
> > Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> > Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > ---
> >   hw/virtio/vhost-user-snd.c | 18 +++++++++++++++++-
> >   1 file changed, 17 insertions(+), 1 deletion(-)
> 
> ¡¡Bien ahí!!
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

Thanks! ;)