Re: [Xen-devel] [PATCH v3 4/4] tools: make xenstore domain easy configurable

[Juergen Gross <jgross@xxxxxxxx>]

On 25/07/16 16:01, Wei Liu wrote:
> On Mon, Jul 25, 2016 at 03:56:17PM +0200, Juergen Gross wrote:
>> On 25/07/16 15:43, Wei Liu wrote:
>>> On Fri, Jul 22, 2016 at 05:09:31PM +0200, Juergen Gross wrote:
>>> [...]
>>>> diff --git a/tools/hotplug/Linux/launch-xenstore.in 
>>>> b/tools/hotplug/Linux/launch-xenstore.in
>>>> index 2bd9f64..fdfa33a 100644
>>>> --- a/tools/hotplug/Linux/launch-xenstore.in
>>>> +++ b/tools/hotplug/Linux/launch-xenstore.in
>>>> @@ -48,18 +48,40 @@ test_xenstore && exit 0
>>>>  
>>>>  test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . 
>>>> @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
>>>>  
>>>> -test -z "$XENSTORED_ROOTDIR" && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
>>>> -rm -f "$XENSTORED_ROOTDIR"/tdb* 2>/dev/null
>>>> -test -z "$XENSTORED_TRACE" || XENSTORED_ARGS=" -T 
>>>> @XEN_LOG_DIR@/xenstored-trace.log"
>>>> +[ "$XENSTORETYPE" = "" ] && XENSTORETYPE=daemon
>>>> +
>>>> +/bin/mkdir -p @XEN_RUN_DIR@
>>>> +
>>>> +[ "$XENSTORETYPE" = "daemon" ] && {
>>>> +  [ -z "$XENSTORED_ROOTDIR" ] && XENSTORED_ROOTDIR="@XEN_LIB_STORED@"
>>>> +  /bin/rm -f $XENSTORED_ROOTDIR/tdb* 2>/dev/null
>>>> +  [ -z "$XENSTORED_TRACE" ] || XENSTORED_ARGS="$XENSTORED_ARGS -T 
>>>> @XEN_LOG_DIR@/xenstored-trace.log"
>>>> +  [ -z "$XENSTORED" ] && XENSTORED=@XENSTORED@
>>>> +  [ -x "$XENSTORED" ] || {
>>>> +          echo "No xenstored found"
>>>> +          exit 1
>>>> +  }
>>>>  
>>>> -if [ -n "$XENSTORED" ] ; then
>>>>    echo -n Starting $XENSTORED...
>>>>    $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS
>>>> -else
>>>> -  echo "No xenstored found"
>>>> -  exit 1
>>>> -fi
>>>>  
>>>> -timeout_xenstore $XENSTORED || exit 1
>>>> +  systemd-notify --booted 2>/dev/null || timeout_xenstore $XENSTORED || 
>>>> exit 1
>>>>  
>>>> -exit 0
>>>> +  exit 0
>>>> +}
>>>> +
>>>> +[ "$XENSTORETYPE" = "domain" ] && {
>>>> +  [ -z "$XENSTORE_DOMAIN_KERNEL" ] && 
>>>> XENSTORE_DOMAIN_KERNEL=@LIBEXEC@/boot/xenstore-stubdom.gz
>>>> +  XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --kernel 
>>>> $XENSTORE_DOMAIN_KERNEL"
>>>> +  [ -z "$XENSTORE_DOMAIN_SIZE" ] && XENSTORE_DOMAIN_SIZE=8
>>>> +  XENSTORE_DOMAIN_ARGS="$XENSTORE_DOMAIN_ARGS --memory 
>>>> $XENSTORE_DOMAIN_SIZE"
>>>> +
>>>> +  echo -n Starting $XENSTORE_DOMAIN_KERNEL...
>>>> +  ${LIBEXEC_BIN}/init-xenstore-domain $XENSTORE_DOMAIN_ARGS || exit 1
>>>> +  systemd-notify --ready 2>/dev/null
>>>
>>> Please test if there is systemd-notify before trying to invoke it and
>>> then you can properly log the failure of the invocation.
>>
>> I thought about that. What would be the purpose doing so? Following
>> cases are possible:
>>
> 
> This
> 
>> - system is booted under control of systemd, systemd-notify is found:
>>   everything is nice, systemd receives the notification it is waiting
>>   for
>>
> 
> Here you assume systemd-notify always succeed. It can fail due to some
> reasons. That's what its manpage suggests.
> 
> We need to handle this.

May I repeat the sd_notify() man page here?

"In order to support both, init systems that implement this scheme and
those which do not, it is generally recommended to ignore the return
value of this call."

Juergen

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