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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Jul 25, 2016 at 04:16:51PM +0200, Juergen Gross wrote:
> 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)."
> 

OK. I'm convinced now.

Wei.

> 
> Juergen
> 

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