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

[Juergen Gross <jgross@suse.com>]

On 25/07/16 13:05, Wei Liu wrote:
> 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.

Verified for both xenstored variants.


Juergen


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