diff mbox series

[10/24] Make libs/evtchn build on NetBSD

Message ID 20201214163623.2127-11-bouyer@netbsd.org (mailing list archive)
State New, archived
Headers show
Series NetBSD fixes | expand

Commit Message

Manuel Bouyer Dec. 14, 2020, 4:36 p.m. UTC 
---
 tools/libs/evtchn/netbsd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Roger Pau Monne Dec. 29, 2020, 11:52 a.m. UTC | #1 
On Mon, Dec 14, 2020 at 05:36:09PM +0100, Manuel Bouyer wrote:
> ---
>  tools/libs/evtchn/netbsd.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
> index 8b8545d2f9..6d4ce28011 100644
> --- a/tools/libs/evtchn/netbsd.c
> +++ b/tools/libs/evtchn/netbsd.c
> @@ -25,10 +25,10 @@
>  
>  #include <sys/ioctl.h>
>  
> -#include <xen/sys/evtchn.h>
> -
>  #include "private.h"
>  
> +#include <xen/xenio3.h>
> +
>  #define EVTCHN_DEV_NAME  "/dev/xenevt"
>  
>  int osdep_evtchn_open(xenevtchn_handle *xce)
> @@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
>      int fd = xce->fd;
>      evtchn_port_t port;
>  
> -    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
> +    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
>          return -1;
>  
>      return port;
> @@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
>  int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
>  {
>      int fd = xce->fd;
> -    return write_exact(fd, (char *)&port, sizeof(port));
> +    return write(fd, (char *)&port, sizeof(port));

I'm afraid we will need some context as to why {read/write}_exact
doesn't work here.

Thanks, Roger.
Manuel Bouyer Jan. 4, 2021, 10:26 a.m. UTC | #2 
On Tue, Dec 29, 2020 at 12:52:43PM +0100, Roger Pau Monné wrote:
> On Mon, Dec 14, 2020 at 05:36:09PM +0100, Manuel Bouyer wrote:
> > ---
> >  tools/libs/evtchn/netbsd.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
> > index 8b8545d2f9..6d4ce28011 100644
> > --- a/tools/libs/evtchn/netbsd.c
> > +++ b/tools/libs/evtchn/netbsd.c
> > @@ -25,10 +25,10 @@
> >  
> >  #include <sys/ioctl.h>
> >  
> > -#include <xen/sys/evtchn.h>
> > -
> >  #include "private.h"
> >  
> > +#include <xen/xenio3.h>
> > +
> >  #define EVTCHN_DEV_NAME  "/dev/xenevt"
> >  
> >  int osdep_evtchn_open(xenevtchn_handle *xce)
> > @@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> >      int fd = xce->fd;
> >      evtchn_port_t port;
> >  
> > -    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
> > +    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
> >          return -1;
> >  
> >      return port;
> > @@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> >  int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
> >  {
> >      int fd = xce->fd;
> > -    return write_exact(fd, (char *)&port, sizeof(port));
> > +    return write(fd, (char *)&port, sizeof(port));
> 
> I'm afraid we will need some context as to why {read/write}_exact
> doesn't work here.

It just doesn't exists on NetBSD
Roger Pau Monne Jan. 4, 2021, 5:15 p.m. UTC | #3 
On Mon, Jan 04, 2021 at 11:26:45AM +0100, Manuel Bouyer wrote:
> On Tue, Dec 29, 2020 at 12:52:43PM +0100, Roger Pau Monné wrote:
> > On Mon, Dec 14, 2020 at 05:36:09PM +0100, Manuel Bouyer wrote:
> > > ---
> > >  tools/libs/evtchn/netbsd.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
> > > index 8b8545d2f9..6d4ce28011 100644
> > > --- a/tools/libs/evtchn/netbsd.c
> > > +++ b/tools/libs/evtchn/netbsd.c
> > > @@ -25,10 +25,10 @@
> > >  
> > >  #include <sys/ioctl.h>
> > >  
> > > -#include <xen/sys/evtchn.h>
> > > -
> > >  #include "private.h"
> > >  
> > > +#include <xen/xenio3.h>
> > > +
> > >  #define EVTCHN_DEV_NAME  "/dev/xenevt"
> > >  
> > >  int osdep_evtchn_open(xenevtchn_handle *xce)
> > > @@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > >      int fd = xce->fd;
> > >      evtchn_port_t port;
> > >  
> > > -    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
> > > +    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
> > >          return -1;
> > >  
> > >      return port;
> > > @@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > >  int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
> > >  {
> > >      int fd = xce->fd;
> > > -    return write_exact(fd, (char *)&port, sizeof(port));
> > > +    return write(fd, (char *)&port, sizeof(port));
> > 
> > I'm afraid we will need some context as to why {read/write}_exact
> > doesn't work here.
> 
> It just doesn't exists on NetBSD

But those are not part of libc or any external library, they are
implemented in tools/libs/ctrl/xc_private.c and should be available to
the NetBSD build AFAICT.

They are just helpers build on top of the standard read/write calls.

Roger.
Manuel Bouyer Jan. 10, 2021, 12:22 p.m. UTC | #4 
On Mon, Jan 04, 2021 at 06:15:24PM +0100, Roger Pau Monné wrote:
> On Mon, Jan 04, 2021 at 11:26:45AM +0100, Manuel Bouyer wrote:
> > On Tue, Dec 29, 2020 at 12:52:43PM +0100, Roger Pau Monné wrote:
> > > On Mon, Dec 14, 2020 at 05:36:09PM +0100, Manuel Bouyer wrote:
> > > > ---
> > > >  tools/libs/evtchn/netbsd.c | 8 ++++----
> > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
> > > > index 8b8545d2f9..6d4ce28011 100644
> > > > --- a/tools/libs/evtchn/netbsd.c
> > > > +++ b/tools/libs/evtchn/netbsd.c
> > > > @@ -25,10 +25,10 @@
> > > >  
> > > >  #include <sys/ioctl.h>
> > > >  
> > > > -#include <xen/sys/evtchn.h>
> > > > -
> > > >  #include "private.h"
> > > >  
> > > > +#include <xen/xenio3.h>
> > > > +
> > > >  #define EVTCHN_DEV_NAME  "/dev/xenevt"
> > > >  
> > > >  int osdep_evtchn_open(xenevtchn_handle *xce)
> > > > @@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > > >      int fd = xce->fd;
> > > >      evtchn_port_t port;
> > > >  
> > > > -    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
> > > > +    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
> > > >          return -1;
> > > >  
> > > >      return port;
> > > > @@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > > >  int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
> > > >  {
> > > >      int fd = xce->fd;
> > > > -    return write_exact(fd, (char *)&port, sizeof(port));
> > > > +    return write(fd, (char *)&port, sizeof(port));
> > > 
> > > I'm afraid we will need some context as to why {read/write}_exact
> > > doesn't work here.
> > 
> > It just doesn't exists on NetBSD
> 
> But those are not part of libc or any external library, they are
> implemented in tools/libs/ctrl/xc_private.c and should be available to
> the NetBSD build AFAICT.
> 
> They are just helpers build on top of the standard read/write calls.

Yes, I misremembered (I have this patch for a long time, since 4.11 at last,
maybe even older).
Anyway the build fails with:
netbsd.c: In function 'xenevtchn_pending':
netbsd.c:134:10: error: implicit declaration of function 'read_exact'; did you mean 'readlinkat'? [-Werror=implicit-function-declaration]

The only header where I see this function defined is
tools/libs/ctrl/xc_private.h, so I would need something like
#include "../../ctrl/xc_private.h"
but this doesn't look right.

I didn't find where other OSes are getting the prototype from (or maybe
they just have this -Werror turned off ?)

Anyway I think NetBSD doesn't need this read_exact/write_exact thing,
the underlying pseudo-device won't to partial read/write.
Roger Pau Monne Jan. 11, 2021, 5:22 p.m. UTC | #5 
On Sun, Jan 10, 2021 at 01:22:50PM +0100, Manuel Bouyer wrote:
> On Mon, Jan 04, 2021 at 06:15:24PM +0100, Roger Pau Monné wrote:
> > On Mon, Jan 04, 2021 at 11:26:45AM +0100, Manuel Bouyer wrote:
> > > On Tue, Dec 29, 2020 at 12:52:43PM +0100, Roger Pau Monné wrote:
> > > > On Mon, Dec 14, 2020 at 05:36:09PM +0100, Manuel Bouyer wrote:
> > > > > ---
> > > > >  tools/libs/evtchn/netbsd.c | 8 ++++----
> > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
> > > > > index 8b8545d2f9..6d4ce28011 100644
> > > > > --- a/tools/libs/evtchn/netbsd.c
> > > > > +++ b/tools/libs/evtchn/netbsd.c
> > > > > @@ -25,10 +25,10 @@
> > > > >  
> > > > >  #include <sys/ioctl.h>
> > > > >  
> > > > > -#include <xen/sys/evtchn.h>
> > > > > -
> > > > >  #include "private.h"
> > > > >  
> > > > > +#include <xen/xenio3.h>
> > > > > +
> > > > >  #define EVTCHN_DEV_NAME  "/dev/xenevt"
> > > > >  
> > > > >  int osdep_evtchn_open(xenevtchn_handle *xce)
> > > > > @@ -131,7 +131,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > > > >      int fd = xce->fd;
> > > > >      evtchn_port_t port;
> > > > >  
> > > > > -    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
> > > > > +    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
> > > > >          return -1;
> > > > >  
> > > > >      return port;
> > > > > @@ -140,7 +140,7 @@ xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
> > > > >  int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
> > > > >  {
> > > > >      int fd = xce->fd;
> > > > > -    return write_exact(fd, (char *)&port, sizeof(port));
> > > > > +    return write(fd, (char *)&port, sizeof(port));
> > > > 
> > > > I'm afraid we will need some context as to why {read/write}_exact
> > > > doesn't work here.
> > > 
> > > It just doesn't exists on NetBSD
> > 
> > But those are not part of libc or any external library, they are
> > implemented in tools/libs/ctrl/xc_private.c and should be available to
> > the NetBSD build AFAICT.
> > 
> > They are just helpers build on top of the standard read/write calls.
> 
> Yes, I misremembered (I have this patch for a long time, since 4.11 at last,
> maybe even older).
> Anyway the build fails with:
> netbsd.c: In function 'xenevtchn_pending':
> netbsd.c:134:10: error: implicit declaration of function 'read_exact'; did you mean 'readlinkat'? [-Werror=implicit-function-declaration]
> 
> The only header where I see this function defined is
> tools/libs/ctrl/xc_private.h, so I would need something like
> #include "../../ctrl/xc_private.h"
> but this doesn't look right.
> 
> I didn't find where other OSes are getting the prototype from (or maybe
> they just have this -Werror turned off ?)
> 
> Anyway I think NetBSD doesn't need this read_exact/write_exact thing,
> the underlying pseudo-device won't to partial read/write.

The usage of {read/write}_exact there is indeed a mistake, when the
evtchn library was split from libxc no one realized that the
{read/write}_exact where no longer available to that code.

Could you please add:

Fixes: b7f76a699dc ('tools: Refactor /dev/xen/evtchn wrappers into libxenevtchn.')

To the commit message?

And also:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.
diff mbox series

Patch

diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
index 8b8545d2f9..6d4ce28011 100644
--- a/tools/libs/evtchn/netbsd.c
+++ b/tools/libs/evtchn/netbsd.c
@@ -25,10 +25,10 @@ 
 
 #include <sys/ioctl.h>
 
-#include <xen/sys/evtchn.h>
-
 #include "private.h"
 
+#include <xen/xenio3.h>
+
 #define EVTCHN_DEV_NAME  "/dev/xenevt"
 
 int osdep_evtchn_open(xenevtchn_handle *xce)
@@ -131,7 +131,7 @@  xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
     int fd = xce->fd;
     evtchn_port_t port;
 
-    if ( read_exact(fd, (char *)&port, sizeof(port)) == -1 )
+    if ( read(fd, (char *)&port, sizeof(port)) == -1 )
         return -1;
 
     return port;
@@ -140,7 +140,7 @@  xenevtchn_port_or_error_t xenevtchn_pending(xenevtchn_handle *xce)
 int xenevtchn_unmask(xenevtchn_handle *xce, evtchn_port_t port)
 {
     int fd = xce->fd;
-    return write_exact(fd, (char *)&port, sizeof(port));
+    return write(fd, (char *)&port, sizeof(port));
 }
 
 /*