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

Wei.

> 
> Juergen

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