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

[Juergen Gross <jgross@xxxxxxxx>]

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?

In case we pass fds as a parameter, I'd be inclined to make it even local to the
main() function.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature