Re: [PATCH 5/8] vhost-user: enable frontends on any POSIX system

[Stefano Garzarella <sgarzare@redhat.com>]

On Wed, Oct 28, 2026 at 08:33:30PM +0100, Tim Pambor wrote:
>On Wed, Oct 28, 2026 at 7:44 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>
>> On Wed, Oct 28, 2026 at 02:23:55PM +0100, Stefano Garzarella wrote:
>> > On Wed, Oct 28, 2026 at 08:33:46AM -0400, Stefan Hajnoczi wrote:
>> > > On Wed, Oct 28, 2026 at 09:32:05AM +0100, Stefano Garzarella wrote:
>> > > > On Tue, Oct 27, 2026 at 03:24:45PM -0400, Stefan Hajnoczi wrote:
>> > > > > On Thu, Oct 22, 2026 at 03:47:26PM +0200, Tim Pambor wrote:
>> > > > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> > > > > > index 6e05c995f1..788913953d 100644
>> > > > > > --- a/hw/net/vhost_net.c
>> > > > > > +++ b/hw/net/vhost_net.c
>> > > > > > @@ -33,7 +33,13 @@
>> > > > > >  #include "standard-headers/linux/virtio_ring.h"
>> > > > > >  #include "hw/virtio/vhost.h"
>> > > > > >  #include "hw/virtio/virtio-bus.h"
>> > > > > > +#if defined(__linux__)
>> > > > > >  #include "linux-headers/linux/vhost.h"
>> > > > > > +#endif
>> > > > > > +
>> > > > > > +#ifndef VHOST_FILE_UNBIND
>> > > > > > +#define VHOST_FILE_UNBIND -1
>> > > > > > +#endif
>> > > > >
>> > > > > This constant is for hw/virtio/vhost.c's vhost_net_set_backend() API. It
>> > > > > should be defined along with that API. Define it alongside
>> > > > > vhost_net_set_backend() in hw/virtio/vhost.h and call it
>> > > > > VHOST_NET_FILE_UNBIND?
>> > > > >
>> > > >
>> > > > That constant is defined by a Linux header (see
>> > > > linux-headers/linux/vhost.h). Are you suggesting to add another constant
>> > > > that is defined with VHOST_FILE_UNBIND on Linux and -1 on other platforms?
>> > >
>> > > Looking again, we should probably just use -1 instead of a constant
>> >
>> > Yep, this would be fine too.
>> > I don't remember why I did in this way, maybe to keep the diff smaller
>> > enough, but I don't have a strong opinion, we can avoid this ifndef and use
>> > -1 everywhere if you prefer that.
>>
>> IMO VHOST_FILE_UNBIND doesn't belong in vhost_net.c, so it would be nice
>> to avoid the #ifndef.
>
>So, for v2, is it ok if I replace the one usage of VHOST_FILE_UNBIND in
>vhost_net.c with -1 and remove the #ifndef?

Yep.

>
>> > > since include/standard-headers/linux/vhost_types.h doesn't even define a
>> > > constant:
>> > >
>> > >  struct vhost_vring_file {
>> > >     unsigned int index;
>> > >     int fd; /* Pass -1 to unbind from file. */
>> > >
>> > >  };
>> > >
>> > > It's strange that VHOST_FILE_UNBIND was defined in <linux/vhost.h>
>> > > instead of <linux/vhost_types.h> where struct vhost_vring_file is
>> > > defined.
>> >
>> > Oh, VHOST_FILE_UNBIND was introduced after struct vhost_vring_file, with
>> > commit 776f395004d8 ("vhost_vdpa: Support config interrupt in vdpa").
>> >
>> > But I agree that should be moved to vhost_types.h. BTW <linux/vhost.h>
>> > already includes <linux/vhost_types.h>, so maybe we can fix it without too
>> > much trouble.
>>
>> Yes, that change seems safe and will improve things in the long term.
>
>Or should I send a patch to linux adding VHOST_FILE_UNBIND to vhost_types.h
>and then drop it from vhost.h and remove the #ifndef?

This is orthogonal, the issue this patch was fixing was to build on 
no-Linux systems, so changing Linux headers will not help.

This is more a cleanup, if you can send also that patch to Linux (which 
is unrelated to this work) would be nice, otherwise let me know and I 
can do that.

Note: files under linux-headers/ are imported from Linux, can't be 
changed here.

Thanks,
Stefano