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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-05-21 at 19:29 +0200, Luis R. Rodriguez wrote:
> On Wed, May 21, 2014 at 05:39:31PM +0100, Ian Campbell wrote:
> > 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)
> 
> It was not clear that this was not a requirement and hence the use of
> dlopen() and dlsym(). Now that that work is done though up to you guys
> to decide what you want, this option works now, I've tested it, so
> just let me know.
> 
> I believe the existing use integration provides the most flexibility.

Please drop dlopen and just link against the libraries.

> > > 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...
> 
> There are seval uses of the structure I defined, one is the making it
> easier to call sd_is_socket_unix() with already prepared data,

Practically all the arguments to which are static in this case though.
(Once you ignore the totally useless stdio entries)

>  the other
> use case is to chmod() the socket on the path given that while systemd
> service unit files does support specifying permissions on the sockets
> its a wide set permission setting, so changes between permissions must
> be handled manually right now. See the use of chmod on 
> xs_claim_active_socket() 

I don't think you need a table just to select between 0600 and 0660 out
of a choice of two sockets. At most a switch is needed, even an if would
do for now.

> > so I don't understand why you would need
> > to "alternate" on anything there either.
> 
> What I mean by alternating on different sd_is_socket_*() calls is if we were 
> to
> grow xenstored with different type of sockets and make the code a bit more
> dynamic. In such case we'd just throw a switch on the active_socket->type and
> call the appropriate sd_is_socket_*().

No we wouldn't, we'd throw a switch over the index into the lookup
function and handle it there.

Ian.


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