Re: [PATCH v3 25/33] tools/xenstored: move all socket handling into posix.c

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 05/02/2024 09:21, Juergen Gross wrote:
> On 26.01.24 17:22, Julien Grall wrote:
> > Hi Juergen,
> >
> > On 04/01/2024 09:00, Juergen Gross wrote:
> > > All of the socket handling is needed only when running as daemon.
> > >
> > > Move it into posix.c, allowing to remove the NO_SOCKETS macro.
> > >
> > > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > > ---
> > > V3:
> > > - new patch
> > > ---
> > >   tools/xenstored/Makefile.common |   4 -
> > >   tools/xenstored/core.c          | 156 +-------------------------------
> > >   tools/xenstored/core.h          |   8 +-
> > >   tools/xenstored/domain.c        |   9 +-
> > >   tools/xenstored/minios.c        |   6 ++
> > >   tools/xenstored/posix.c         | 153 +++++++++++++++++++++++++++++++
> > >   6 files changed, 167 insertions(+), 169 deletions(-)
> > >
> > > diff --git a/tools/xenstored/Makefile.common 
> > > b/tools/xenstored/Makefile.common
> > > index 189ab81b8d..ef63ef650c 100644
> > > --- a/tools/xenstored/Makefile.common
> > > +++ b/tools/xenstored/Makefile.common
> > > @@ -16,10 +16,6 @@ CFLAGS += $(CFLAGS_libxenctrl)
> > >   CFLAGS += $(CFLAGS_libxenguest)
> > >   CFLAGS += $(CFLAGS_libxentoolcore)
> > > -ifdef CONFIG_STUBDOM
> > > -CFLAGS += -DNO_SOCKETS=1
> > > -endif
> > > -
> > >   $(XENSTORED_OBJS-y): CFLAGS += $(CFLAGS_libxengnttab)
> > >   xenstored.a: $(XENSTORED_OBJS-y)
> > > diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
> > > index 650c1d1574..8ff1faccef 100644
> > > --- a/tools/xenstored/core.c
> > > +++ b/tools/xenstored/core.c
> > > @@ -20,10 +20,6 @@
> > >   #include <sys/types.h>
> > >   #include <sys/stat.h>
> > >   #include <poll.h>
> > > -#ifndef NO_SOCKETS
> > > -#include <sys/socket.h>
> > > -#include <sys/un.h>
> > > -#endif
> > >   #include <sys/time.h>
> > >   #include <time.h>
> > >   #include <unistd.h>
> > > @@ -61,7 +57,7 @@ static unsigned int current_array_size;
> > >   static unsigned int nr_fds;
> > >   static unsigned int delayed_requests;
> > > -static int sock = -1;
> > > +int sock = -1;
> > Similar comment to 'fds' on the previous patch. This name is quite 
> > generic. Can we either pass it as an argument (or return) or rename it?
> Turns out this patch now makes the "fds" solution more complicated: I'd 
> need to
> add fds as an additional parameter to the .can_write() and .can_read() 
> callbacks
> of struct interface_funcs.
>
> I can do that, but an alternative would be to just rename fds to e.g. 
> poll_fds.
> Do you have any preference?
The last one seems to be the simplest approach. And I am fine with the 
renaming. This can be improved in the future.
Cheers,

--
Julien Grall