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

[Juergen Gross <jgross@xxxxxxxx>]

On 25/07/16 16:11, Wei Liu wrote:
> On Mon, Jul 25, 2016 at 04:06:11PM +0200, Juergen Gross wrote:
>> 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."
> 
> That's a different thing from the systemd-notify utility, isn't it?
> 
> I read man systemd-notify in this case:
> 
>   EXIT STATUS
>          On success, 0 is returned, a non-zero failure code otherwise.
> 
> I'm not too sure about the relationship between systemd-notify and
> sd_notify, but I bet it is more than just a call to sd_notify.

The systemd-notify man-page tells us:

"This is mostly just a wrapper around sd_notify() and makes this
functionality available to shell scripts. For details see sd_notify(3)."


Juergen


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