Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-17 Thread Stefan Hajnoczi
On Tue, 17 Oct 2023 at 10:38, Albert Esteve wrote: > On Tue, Oct 17, 2023 at 12:57 PM Stefan Hajnoczi wrote: >> >> On Tue, 17 Oct 2023 at 04:26, Albert Esteve wrote: Thanks for considering my feedback! > There is not. In a first thought, I assumed that the backends will be in > charge > of cl

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-17 Thread Albert Esteve
On Tue, Oct 17, 2023 at 12:57 PM Stefan Hajnoczi wrote: > On Tue, 17 Oct 2023 at 04:26, Albert Esteve wrote: > > > > Hi! > > > > Thanks for the patch, and sorry for not noticing the flag had already > been assigned. The patch looks good to me! > > Hi Albert, > I looked at the shared object code

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-17 Thread Stefan Hajnoczi
On Tue, 17 Oct 2023 at 04:26, Albert Esteve wrote: > > Hi! > > Thanks for the patch, and sorry for not noticing the flag had already been > assigned. The patch looks good to me! Hi Albert, I looked at the shared object code for the first time: - There are memory leaks in virtio_add_dmabuf() and

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-17 Thread Albert Esteve
Hi! Thanks for the patch, and sorry for not noticing the flag had already been assigned. The patch looks good to me! BR, Albert On Tue, Oct 17, 2023 at 9:54 AM Viresh Kumar wrote: > On 17-10-23, 09:51, Hanna Czenczek wrote: > > Not that I’m really opposed to that, but I don’t see the problem w

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-17 Thread Viresh Kumar
On 17-10-23, 10:14, Hanna Czenczek wrote: > On 17.10.23 09:53, Viresh Kumar wrote: > > On 17-10-23, 09:51, Hanna Czenczek wrote: > > > Not that I’m really opposed to that, but I don’t see the problem with just > > > doing that in the same work that makes qemu actually use this flag, > > > exactly

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-17 Thread Hanna Czenczek
On 17.10.23 09:53, Viresh Kumar wrote: On 17-10-23, 09:51, Hanna Czenczek wrote: Not that I’m really opposed to that, but I don’t see the problem with just doing that in the same work that makes qemu actually use this flag, exactly because it’s just a -1/+1 change. I can send a v2, but should I

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-17 Thread Viresh Kumar
On 17-10-23, 09:51, Hanna Czenczek wrote: > Not that I’m really opposed to that, but I don’t see the problem with just > doing that in the same work that makes qemu actually use this flag, exactly > because it’s just a -1/+1 change. > > I can send a v2, but should I do the same for libvhost-user a

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-17 Thread Hanna Czenczek
On 17.10.23 07:36, Viresh Kumar wrote: On 16-10-23, 12:40, Alex Bennée wrote: Viresh Kumar writes: On 16-10-23, 11:45, Manos Pitsidianakis wrote: On Mon, 16 Oct 2023 11:32, Hanna Czenczek wrote: diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h index 9f9ddf878d..

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-16 Thread Viresh Kumar
On 16-10-23, 12:40, Alex Bennée wrote: > > Viresh Kumar writes: > > > On 16-10-23, 11:45, Manos Pitsidianakis wrote: > >> On Mon, 16 Oct 2023 11:32, Hanna Czenczek wrote: > >> > diff --git a/include/hw/virtio/vhost-user.h > >> > b/include/hw/virtio/vhost-user.h > >> > index 9f9ddf878d..1d412143

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-16 Thread Stefan Hajnoczi
Reviewed-by: Stefan Hajnoczi On Mon, 16 Oct 2023 at 04:33, Hanna Czenczek wrote: > > The VHOST_USER_PROTOCOL_F_XEN_MMAP feature bit was defined in > f21e95ee97d, which has been part of qemu's 8.1.0 release. However, it > seems it was never added to qemu's code, but it is well possible that it >

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-16 Thread Manos Pitsidianakis
On Mon, 16 Oct 2023 13:32, Viresh Kumar wrote: Perhaps because we will never use it from Qemu code ? It'd be nice if downstream users of the vhost-user protocol can fetch and use the header verbatim. For example, for automatically generating bindings.

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-16 Thread Alex Bennée
Viresh Kumar writes: > On 16-10-23, 11:45, Manos Pitsidianakis wrote: >> On Mon, 16 Oct 2023 11:32, Hanna Czenczek wrote: >> > diff --git a/include/hw/virtio/vhost-user.h >> > b/include/hw/virtio/vhost-user.h >> > index 9f9ddf878d..1d4121431b 100644 >> > --- a/include/hw/virtio/vhost-user.h >>

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-16 Thread Hanna Czenczek
On 16.10.23 10:45, Manos Pitsidianakis wrote: On Mon, 16 Oct 2023 11:32, Hanna Czenczek wrote: diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h index 9f9ddf878d..1d4121431b 100644 --- a/include/hw/virtio/vhost-user.h +++ b/include/hw/virtio/vhost-user.h @@ -29,7 +29

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-16 Thread Viresh Kumar
On 16-10-23, 11:45, Manos Pitsidianakis wrote: > On Mon, 16 Oct 2023 11:32, Hanna Czenczek wrote: > > diff --git a/include/hw/virtio/vhost-user.h > > b/include/hw/virtio/vhost-user.h > > index 9f9ddf878d..1d4121431b 100644 > > --- a/include/hw/virtio/vhost-user.h > > +++ b/include/hw/virtio/vhost-

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-16 Thread Stefano Garzarella
On Mon, Oct 16, 2023 at 10:32:01AM +0200, Hanna Czenczek wrote: The VHOST_USER_PROTOCOL_F_XEN_MMAP feature bit was defined in f21e95ee97d, which has been part of qemu's 8.1.0 release. However, it seems it was never added to qemu's code, but it is well possible that it is already used by differen

Re: [PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-16 Thread Manos Pitsidianakis
On Mon, 16 Oct 2023 11:32, Hanna Czenczek wrote: diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h index 9f9ddf878d..1d4121431b 100644 --- a/include/hw/virtio/vhost-user.h +++ b/include/hw/virtio/vhost-user.h @@ -29,7 +29,8 @@ enum VhostUserProtocolFeature { VHOST

[PATCH] vhost-user: Fix protocol feature bit conflict

2023-10-16 Thread Hanna Czenczek
The VHOST_USER_PROTOCOL_F_XEN_MMAP feature bit was defined in f21e95ee97d, which has been part of qemu's 8.1.0 release. However, it seems it was never added to qemu's code, but it is well possible that it is already used by different front-ends outside of qemu (i.e., Xen). VHOST_USER_PROTOCOL_F_S