Re: [Xen-devel] [PATCH v3 1/4] tools: remove systemd xenstore socket definitions

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Jul 25, 2016 at 06:33:35AM +0200, Juergen Gross wrote:
> On 22/07/16 20:51, Wei Liu wrote:
> > On Fri, Jul 22, 2016 at 08:49:17PM +0200, Juergen Gross wrote:
> >> On 22/07/16 18:31, Wei Liu wrote:
> >>> Only skim-read this patch, will do proper review later.
> >>>
> >>> On Fri, Jul 22, 2016 at 05:09:28PM +0200, Juergen Gross wrote:
> >>> [...]
> >>>>  CAMLprim value ocaml_launched_by_systemd(value ignore)
> >>>>  {
> >>>> -        CAMLparam1(ignore);
> >>>> -        CAMLlocal1(ret);
> >>>> +       CAMLparam1(ignore);
> >>>> +       CAMLlocal1(ret);
> >>>>  
> >>>> -        ret = Val_false;
> >>>> +       ret = Val_false;
> >>>>  
> >>>> -        if (sd_listen_fds(0) > 0)
> >>>> -                ret = Val_true;
> >>>> +       if (sd_booted() > 0)
> >>>> +               ret = Val_true;
> >>>
> >>> I think this may be problematic.
> >>>
> >>> sd_booted returns true if system is booted with systemd, but it has no
> >>> bearing whether this particular process is launched by systemd.
> >>>
> >>> IIRC using sd_booted would cause oxenstored thinks it is launched by
> >>> systemd even if the user launches it by hand in a shell. That caused
> >>> it's initialisation to fail.  81d758afca7c3c1e3ccbd78154b33d64fd7757fb
> >>> was written to address that issue.
> >>>
> >>> So, what would happen if you start oxenstored by hand with your patch
> >>> apply? Maybe we can just remove this launched_by_systemd check all
> >>> together -- i.e. we always call sd_notify?
> 
> Sure we could. I'll remove the checks in both xenstored variants if
> nobody objects.
> 
> >>
> >> So you are concerned sd_notify() will be called too often, but you are
> >> suggesting to call it always? I don't understand your concerns then.
> >>
> > 
> > No, my concern is that you won't be able to start oxenstored from
> > command line manually if you boot with systemd. At least that was the
> > bug that caused me to write that patch.
> 
> I believe the main problem was xenstored not calling daemonize() in that
> case, right? This problem is being remove with my patch as daemonize()
> will be called always. The systemd service file is modified to reflect
> this change in behavior.
> 

I'm afraid I can't remember all the details. But as long as you can
launch [o/c]xenstored by hand I think we're fine.

Wei.

> 
> Juergen

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