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

[Juergen Gross <jgross@xxxxxxxx>]

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?

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.

> 
>>  
>> -    CAMLreturn(ret);
>> +       CAMLreturn(ret);
>>  }
>>  
>>  CAMLprim value ocaml_sd_notify_ready(value ignore)
>>  {
>> -    CAMLparam1(ignore);
>> -    CAMLlocal1(ret);
>> +       CAMLparam1(ignore);
>> +       CAMLlocal1(ret);
>>  
>> -    ret = Val_int(0);
>> +       ret = Val_int(0);
>>  
>> -    sd_notify(1, "READY=1");
>> +       sd_notify(1, "READY=1");
>>  
>> -    CAMLreturn(ret);
>> +       CAMLreturn(ret);
> 
> It seems that you have introduced quite a few white space changes.
> If you really want to change tabs to spaces, please do that in a
> separate patch.

Oops, didn't mean to do this. Sorry.


Juergen

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