Re: [PATCH 10/24] Make libs/evtchn build on NetBSD

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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@xxxxxxxxxx>

Thanks, Roger.