[PATCH 2/2] Allow building vhost-user in BSD

Sergio Lopez posted 2 patches 3 years, 11 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, John G Johnson <john.g.johnson@oracle.com>, Alex Williamson <alex.williamson@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Thomas Huth <thuth@redhat.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, "Michael S. Tsirkin" <mst@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>
There is a newer version of this series
[PATCH 2/2] Allow building vhost-user in BSD
Posted by Sergio Lopez 3 years, 11 months ago
With the possibility of using pipefd as a replacement on operating
systems that doesn't support eventfd, vhost-user can also work on BSD
systems.

This change allows enabling vhost-user on BSD platforms too and
makes libvhost_user (which still depends on eventfd) a linux-only
feature.

Signed-off-by: Sergio Lopez <slp@redhat.com>
---
 configure   | 5 +++--
 meson.build | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index c56ed53ee3..93aa22e345 100755
--- a/configure
+++ b/configure
@@ -1659,8 +1659,9 @@ fi
 # vhost interdependencies and host support
 
 # vhost backends
-if test "$vhost_user" = "yes" && test "$linux" != "yes"; then
-  error_exit "vhost-user is only available on Linux"
+if test "$vhost_user" = "yes" && \
+    test "$linux" != "yes" && test "$bsd" != "yes" ; then
+  error_exit "vhost-user is only available on Linux and BSD"
 fi
 test "$vhost_vdpa" = "" && vhost_vdpa=$linux
 if test "$vhost_vdpa" = "yes" && test "$linux" != "yes"; then
diff --git a/meson.build b/meson.build
index 8df40bfac4..f2bc439c30 100644
--- a/meson.build
+++ b/meson.build
@@ -2701,7 +2701,7 @@ if have_system or have_user
 endif
 
 vhost_user = not_found
-if 'CONFIG_VHOST_USER' in config_host
+if targetos == 'linux' and 'CONFIG_VHOST_USER' in config_host
   libvhost_user = subproject('libvhost-user')
   vhost_user = libvhost_user.get_variable('vhost_user_dep')
 endif
-- 
2.35.1
Re: [PATCH 2/2] Allow building vhost-user in BSD
Posted by Paolo Bonzini 3 years, 11 months ago
On 3/2/22 12:36, Sergio Lopez wrote:
> With the possibility of using pipefd as a replacement on operating
> systems that doesn't support eventfd, vhost-user can also work on BSD
> systems.
> 
> This change allows enabling vhost-user on BSD platforms too and
> makes libvhost_user (which still depends on eventfd) a linux-only
> feature.
> 
> Signed-off-by: Sergio Lopez <slp@redhat.com>

I would just check for !windows.

Paolo

> ---
>   configure   | 5 +++--
>   meson.build | 2 +-
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index c56ed53ee3..93aa22e345 100755
> --- a/configure
> +++ b/configure
> @@ -1659,8 +1659,9 @@ fi
>   # vhost interdependencies and host support
>   
>   # vhost backends
> -if test "$vhost_user" = "yes" && test "$linux" != "yes"; then
> -  error_exit "vhost-user is only available on Linux"
> +if test "$vhost_user" = "yes" && \
> +    test "$linux" != "yes" && test "$bsd" != "yes" ; then
> +  error_exit "vhost-user is only available on Linux and BSD"
>   fi
>   test "$vhost_vdpa" = "" && vhost_vdpa=$linux
>   if test "$vhost_vdpa" = "yes" && test "$linux" != "yes"; then
> diff --git a/meson.build b/meson.build
> index 8df40bfac4..f2bc439c30 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2701,7 +2701,7 @@ if have_system or have_user
>   endif
>   
>   vhost_user = not_found
> -if 'CONFIG_VHOST_USER' in config_host
> +if targetos == 'linux' and 'CONFIG_VHOST_USER' in config_host
>     libvhost_user = subproject('libvhost-user')
>     vhost_user = libvhost_user.get_variable('vhost_user_dep')
>   endif
Re: [PATCH 2/2] Allow building vhost-user in BSD
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
On 2/3/22 18:10, Paolo Bonzini wrote:
> On 3/2/22 12:36, Sergio Lopez wrote:
>> With the possibility of using pipefd as a replacement on operating
>> systems that doesn't support eventfd, vhost-user can also work on BSD
>> systems.
>>
>> This change allows enabling vhost-user on BSD platforms too and
>> makes libvhost_user (which still depends on eventfd) a linux-only
>> feature.
>>
>> Signed-off-by: Sergio Lopez <slp@redhat.com>
> 
> I would just check for !windows.

What about Darwin / Haiku / Illumnos?
Re: [PATCH 2/2] Allow building vhost-user in BSD
Posted by Sergio Lopez 3 years, 11 months ago
On Wed, Mar 02, 2022 at 06:18:59PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/3/22 18:10, Paolo Bonzini wrote:
> > On 3/2/22 12:36, Sergio Lopez wrote:
> > > With the possibility of using pipefd as a replacement on operating
> > > systems that doesn't support eventfd, vhost-user can also work on BSD
> > > systems.
> > > 
> > > This change allows enabling vhost-user on BSD platforms too and
> > > makes libvhost_user (which still depends on eventfd) a linux-only
> > > feature.
> > > 
> > > Signed-off-by: Sergio Lopez <slp@redhat.com>
> > 
> > I would just check for !windows.
> 
> What about Darwin / Haiku / Illumnos?

It should work on every system providing pipe() or pipe2(), so I guess
Paolo's right, every platform except Windows. FWIW, I already tested
it with Darwin.

Thanks,
Sergio.
Re: [PATCH 2/2] Allow building vhost-user in BSD
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
On 2/3/22 18:31, Sergio Lopez wrote:
> On Wed, Mar 02, 2022 at 06:18:59PM +0100, Philippe Mathieu-Daudé wrote:
>> On 2/3/22 18:10, Paolo Bonzini wrote:
>>> On 3/2/22 12:36, Sergio Lopez wrote:
>>>> With the possibility of using pipefd as a replacement on operating
>>>> systems that doesn't support eventfd, vhost-user can also work on BSD
>>>> systems.
>>>>
>>>> This change allows enabling vhost-user on BSD platforms too and
>>>> makes libvhost_user (which still depends on eventfd) a linux-only
>>>> feature.
>>>>
>>>> Signed-off-by: Sergio Lopez <slp@redhat.com>
>>>
>>> I would just check for !windows.
>>
>> What about Darwin / Haiku / Illumnos?
> 
> It should work on every system providing pipe() or pipe2(), so I guess
> Paolo's right, every platform except Windows. FWIW, I already tested
> it with Darwin.

Wow, nice.

So maybe simply check for pipe/pipe2 rather than !windows?

Re: [PATCH 2/2] Allow building vhost-user in BSD
Posted by Daniel P. Berrangé 3 years, 11 months ago
On Wed, Mar 02, 2022 at 06:38:07PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/3/22 18:31, Sergio Lopez wrote:
> > On Wed, Mar 02, 2022 at 06:18:59PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 2/3/22 18:10, Paolo Bonzini wrote:
> > > > On 3/2/22 12:36, Sergio Lopez wrote:
> > > > > With the possibility of using pipefd as a replacement on operating
> > > > > systems that doesn't support eventfd, vhost-user can also work on BSD
> > > > > systems.
> > > > > 
> > > > > This change allows enabling vhost-user on BSD platforms too and
> > > > > makes libvhost_user (which still depends on eventfd) a linux-only
> > > > > feature.
> > > > > 
> > > > > Signed-off-by: Sergio Lopez <slp@redhat.com>
> > > > 
> > > > I would just check for !windows.
> > > 
> > > What about Darwin / Haiku / Illumnos?
> > 
> > It should work on every system providing pipe() or pipe2(), so I guess
> > Paolo's right, every platform except Windows. FWIW, I already tested
> > it with Darwin.
> 
> Wow, nice.
> 
> So maybe simply check for pipe/pipe2 rather than !windows?

NB that would make the check more fragile.

We already use pipe/pipe2 without checking for it, because its
usage is confined to oslib-posix.c and we know all POSIX
OS have it. There is no impl at all of qemu_pipe in oslib-win.c
and the declaration is masked out too in the header file.

Thus if we check for pipe2 and windows did ever implement it,
then we would actually break the windows build due to qemu_pipe
not existing. 

IOW, checking !windows matches our logic for picking oslib-posix.c
in builds and so is better than checking for pipe directly.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH 2/2] Allow building vhost-user in BSD
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
On 2/3/22 18:55, Daniel P. Berrangé wrote:
> On Wed, Mar 02, 2022 at 06:38:07PM +0100, Philippe Mathieu-Daudé wrote:
>> On 2/3/22 18:31, Sergio Lopez wrote:
>>> On Wed, Mar 02, 2022 at 06:18:59PM +0100, Philippe Mathieu-Daudé wrote:
>>>> On 2/3/22 18:10, Paolo Bonzini wrote:
>>>>> On 3/2/22 12:36, Sergio Lopez wrote:
>>>>>> With the possibility of using pipefd as a replacement on operating
>>>>>> systems that doesn't support eventfd, vhost-user can also work on BSD
>>>>>> systems.
>>>>>>
>>>>>> This change allows enabling vhost-user on BSD platforms too and
>>>>>> makes libvhost_user (which still depends on eventfd) a linux-only
>>>>>> feature.
>>>>>>
>>>>>> Signed-off-by: Sergio Lopez <slp@redhat.com>
>>>>>
>>>>> I would just check for !windows.
>>>>
>>>> What about Darwin / Haiku / Illumnos?
>>>
>>> It should work on every system providing pipe() or pipe2(), so I guess
>>> Paolo's right, every platform except Windows. FWIW, I already tested
>>> it with Darwin.
>>
>> Wow, nice.
>>
>> So maybe simply check for pipe/pipe2 rather than !windows?
> 
> NB that would make the check more fragile.
> 
> We already use pipe/pipe2 without checking for it, because its
> usage is confined to oslib-posix.c and we know all POSIX
> OS have it. There is no impl at all of qemu_pipe in oslib-win.c
> and the declaration is masked out too in the header file.
> 
> Thus if we check for pipe2 and windows did ever implement it,
> then we would actually break the windows build due to qemu_pipe
> not existing.
> 
> IOW, checking !windows matches our logic for picking oslib-posix.c
> in builds and so is better than checking for pipe directly.

OK I see, thanks.

Re: [PATCH 2/2] Allow building vhost-user in BSD
Posted by Paolo Bonzini 3 years, 11 months ago
On 3/2/22 18:38, Philippe Mathieu-Daudé wrote:
> On 2/3/22 18:31, Sergio Lopez wrote:
>> On Wed, Mar 02, 2022 at 06:18:59PM +0100, Philippe Mathieu-Daudé wrote:
>>> On 2/3/22 18:10, Paolo Bonzini wrote:
>>>> On 3/2/22 12:36, Sergio Lopez wrote:
>>>>> With the possibility of using pipefd as a replacement on operating
>>>>> systems that doesn't support eventfd, vhost-user can also work on BSD
>>>>> systems.
>>>>>
>>>>> This change allows enabling vhost-user on BSD platforms too and
>>>>> makes libvhost_user (which still depends on eventfd) a linux-only
>>>>> feature.
>>>>>
>>>>> Signed-off-by: Sergio Lopez <slp@redhat.com>
>>>>
>>>> I would just check for !windows.
>>>
>>> What about Darwin / Haiku / Illumnos?
>>
>> It should work on every system providing pipe() or pipe2(), so I guess
>> Paolo's right, every platform except Windows. FWIW, I already tested
>> it with Darwin.
> 
> Wow, nice.
> 
> So maybe simply check for pipe/pipe2 rather than !windows?

What you really need is not pipes, but AF_UNIX.

Paolo

Re: [PATCH 2/2] Allow building vhost-user in BSD
Posted by Daniel P. Berrangé 3 years, 11 months ago
On Wed, Mar 02, 2022 at 07:05:32PM +0100, Paolo Bonzini wrote:
> On 3/2/22 18:38, Philippe Mathieu-Daudé wrote:
> > On 2/3/22 18:31, Sergio Lopez wrote:
> > > On Wed, Mar 02, 2022 at 06:18:59PM +0100, Philippe Mathieu-Daudé wrote:
> > > > On 2/3/22 18:10, Paolo Bonzini wrote:
> > > > > On 3/2/22 12:36, Sergio Lopez wrote:
> > > > > > With the possibility of using pipefd as a replacement on operating
> > > > > > systems that doesn't support eventfd, vhost-user can also work on BSD
> > > > > > systems.
> > > > > > 
> > > > > > This change allows enabling vhost-user on BSD platforms too and
> > > > > > makes libvhost_user (which still depends on eventfd) a linux-only
> > > > > > feature.
> > > > > > 
> > > > > > Signed-off-by: Sergio Lopez <slp@redhat.com>
> > > > > 
> > > > > I would just check for !windows.
> > > > 
> > > > What about Darwin / Haiku / Illumnos?
> > > 
> > > It should work on every system providing pipe() or pipe2(), so I guess
> > > Paolo's right, every platform except Windows. FWIW, I already tested
> > > it with Darwin.
> > 
> > Wow, nice.
> > 
> > So maybe simply check for pipe/pipe2 rather than !windows?
> 
> What you really need is not pipes, but AF_UNIX.

Recent Windows has AF_UNIX so don't check for that ! What you really
need is AF_UNIX and FD passing and pipes and probably more POSIX
only features ...

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH 2/2] Allow building vhost-user in BSD
Posted by Sergio Lopez 3 years, 11 months ago
On Wed, Mar 02, 2022 at 06:38:07PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/3/22 18:31, Sergio Lopez wrote:
> > On Wed, Mar 02, 2022 at 06:18:59PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 2/3/22 18:10, Paolo Bonzini wrote:
> > > > On 3/2/22 12:36, Sergio Lopez wrote:
> > > > > With the possibility of using pipefd as a replacement on operating
> > > > > systems that doesn't support eventfd, vhost-user can also work on BSD
> > > > > systems.
> > > > > 
> > > > > This change allows enabling vhost-user on BSD platforms too and
> > > > > makes libvhost_user (which still depends on eventfd) a linux-only
> > > > > feature.
> > > > > 
> > > > > Signed-off-by: Sergio Lopez <slp@redhat.com>
> > > > 
> > > > I would just check for !windows.
> > > 
> > > What about Darwin / Haiku / Illumnos?
> > 
> > It should work on every system providing pipe() or pipe2(), so I guess
> > Paolo's right, every platform except Windows. FWIW, I already tested
> > it with Darwin.
> 
> Wow, nice.
> 
> So maybe simply check for pipe/pipe2 rather than !windows?
> 

Is it worth it? In "configure", CONFIG_POSIX is set to "y" if the
target isn't "mingw32", and CONFIG_POSIX brings "util/oslib-posix.c"
into the build, which expects to have either pipe or pipe2.

Thanks,
Sergio.