CA-209401: reattempt_on_boot_attach: call attach-static-vdis start in…

…stead of restart

In the original definition of the init script, "restart" is defined as just a
shortcut for "start", and "start" is itself idempotent. The
reattempt_on_boot_attach delibrately does another "(re)start" attempt based on
this fact as an insurance resort.

But with systemd now in place, the semantics of "start/stop/restart" depends on
not only the definition of cmdlet entries themselves but also the service's
current status as in systemd's book. So a "restart" (even if it's just defined
as a nic for "start") usually ended with a autonomous "stop" action first,
given if systemd believes the service has already been running at that point.

Interestingly, the previous "stop" cmdlet was a non-op, so even after we moved
to systemd, the "restart" semantics happened to remaine the same. But this is
not the case any more, as we've now added the "stop" implementation.

The autonomous "stop" would now cause troubles, because xhad might have
been holding one of the static-vdi at that point as the service started
succeeds previously. The extra "stop" action would then hung indefinitely. This
also contradicts with our intention, which is just to "ensure" the static vdis
attaching was successful , rather than detach and rettach them once again.

This patch fixes this by calling "start" instead of "restart". It fits the
systemd model as a double checker. If systemd believes the service is already
running, this will be a non-op. To do this, we also modified the "start"
implementation to return proper error code in case of failures.

Nonethess, to control service running/stopping directly from xapi was not a
clean model to begin with. But at the stage, this is probably the least
interruptive changes we can make to let things continue to work.

Signed-off-by: Zheng Li <[email protected]>

ocaml/xapi/static_vdis.ml

@@ -92,6 +92,6 @@ let gc () =
 let reattempt_on_boot_attach () =
 	let script = "attach-static-vdis" in
 	try
-		ignore(Helpers.call_script "/sbin/service" [ script; "restart" ])
+		ignore(Helpers.call_script "/sbin/service" [ script; "start" ])
 	with e ->
-		warn "Attempt to reattach static VDIs via '%s restart' failed: %s" script (ExnHelper.string_of_exn e)
+		warn "Attempt to reattach static VDIs via '%s start' failed: %s" script (ExnHelper.string_of_exn e)

scripts/init.d-attach-static-vdis

@@ -32,16 +32,19 @@ clear_stale_state(){
 }
 
 attach_all(){
+	RC=0
 	ALL=$(ls -1 ${STATE_DIR})
 
 	for i in ${ALL}; do
 	    UUID=$(cat ${STATE_DIR}/${i}/vdi-uuid)
 	    logger "Attempting to attach VDI: ${UUID}"
 	    OUTPUT=$(/opt/xensource/bin/static-vdis attach ${UUID} 2>&1)
 	    if [ $? -ne 0 ]; then
+	       RC=1
 	       logger "Attempt to attach VDI: ${UUID} failed -- skipping (Error was: ${OUTPUT})"
 	    fi
 	done
+	return $RC
 }
 
 detach_all(){
@@ -61,8 +64,9 @@ start() {
 	echo -n $"Attempting to attach all statically-configured VDIs"
 	clear_stale_state
 	attach_all
+	RC=$?
 	echo 
-	return 0
+	return $RC
 }