Re: [Xen-devel] [PATCH v5 02/14] libxenstore.so: add support for systemd

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-05-21 at 18:24 +0200, Luis R. Rodriguez wrote:
> On Wed, May 21, 2014 at 03:35:19PM +0100, Ian Campbell wrote:
> > On Tue, 2014-05-20 at 05:31 -0700, Luis R. Rodriguez wrote:
> > > From: "Luis R. Rodriguez" <mcgrof@xxxxxxxx>
> > > 
> > > This adds support for systemd into libxenstore.so to enable usage
> > > through cxenstored and oxenstored. The way we provide support for
> > > systemd is to *not* compile systemd with -lsystemd-daemon but instead
> > > to look for libsystemd-daemon.so at run time if the binary was compiled
> > > with support for systemd.
> > 
> > This is not what either Ian or I intended to suggest. It is perfectly
> > fine for the binary to be dynamically linked against -lsystemd-daemon in
> > the normal way (and for the resulting binary packages to depend on the
> > libsystemd-daemon package etc) if the headers etc are present at compile
> > time.
> > 
> > What we were after was for the actual use of systemd to be runtime not
> > compile time. IOW it is fine for xenstored to require the library to be
> > installed, but not that systemd be the init which is in use.
> 
> In order to work dynamically *and* automatically at run time, that is
> to let the binary figure out what is best, the dynamic linker was the
> best choice. The reason is that we have to consider a binary that
> can run on systems that do not have the systemd libraries present,

This is not a requirement. If the systemd libraries were present at
build time then it is acceptable to require them at runtime even if
systemd is not actually in use. (Of course other than he
"is_systemd_being_used" function nothing in the library really ought to
get called in this case)

> or that have them but didn't use sytemd as the init process.

That case must work but if it required dlopen then something is horribly
broken somewhere.

>  This solution
> covers all those cases.
> 
> > > diff --git a/tools/xenstore/xs_systemd.c b/tools/xenstore/xs_systemd.c
> > > new file mode 100644
> > > index 0000000..814e0fc
> > > --- /dev/null
> > > +++ b/tools/xenstore/xs_systemd.c
> > 
> > > +
> > > +/*
> > > + * We list stdin, stdout and stderr simply for documentation purposes
> > > + * and to help our array size fit the number of expected sockets we
> > > + * as sd_listen_fds() will return 5 for example if you set the socket
> > > + * service with 2 sockets.
> > 
> > Please can we get rid of this list (which is bad enough in itself but
> > the three spurious entries are ludicrous)
> >
> > and just
> > #define SOCKET_RW_INDEX SD_LISTEN_FDS_START
> > #define SOCKET_RO_INDEX SDL_LISTEN_FDS_START + 1
> > etc and use those for lookups, as I described in
> > <1399971222.11314.27.camel@xxxxxxxxxxxxxxxxxxxxxx>?
> 
> Did you see the mode? Its an example of how different preferences can
> be tucked away under it, but we can always statically peg associations.

I have no idea what you are trying to ask/say here...

> The purpose of the list was also to allow a switch on the type to
> alternate on sd_is_socket_*() calls but of course if we're not growing
> the xenstore implementation this means we only have to deal with one
> type and can use that statically.

...or here.

If socket activation is enabled then both of the things we are passed
should be unix domain sockets so I don't understand why you would need
to "alternate" on anything there either. And if a new thing were added
to the list then its index would imply that it was whatever type it must
be, because you'd have written it in the spec.

> Up to you, just consider the above and let me know.

The list should go.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel