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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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?

>  
> -     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.

Wei.

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