[8/9] RFC: golang/xenlight: Notify xenlight of SIGCHLD

Commit Message

George Dunlap Dec. 27, 2019, 4:32 p.m. UTC

libxl forks external processes and waits for them to complete; it
therefore needs to be notified when children exit.

In absence of instructions to the contrary, libxl sets up its own
SIGCHLD handlers.

Golang always unmasks and handles SIGCHLD itself.  libxl thankfully
notices this and throws an assert() rather than clobbering SIGCHLD
handlers.

Tell libxl that we'll be responsible for getting SIGCHLD notifications
to it.  Arrange for a channel in the context to receive notifications
on SIGCHLD, and set up a goroutine that will pass these on to libxl.

Signed-off-by: George Dunlap <[e-mail]>
---
I have no idea if this is actually the right way to go about this; in
particular, libxl_event.h's comment on this function refers to the
comment on `libxl_childproc_reaped`, which says:

 * May NOT be called from within a signal handler which might
 * interrupt any libxl operation.  The application will almost
 * certainly need to use the self-pipe trick (or a working pselect or
 * ppoll) to implement this.

I don't have a good way of knowing whether the
goproc-receiving-a-channel satisfies this requirement or not.  It
seems to work, in the sense that the pygrub process works fine.  But
it gets stuck a bit further on, looks like waiting for the disk; and a

Patch

diff of the output between `xl create` and the golang create seems to
indicate that xenstore watches aren't being delivered.  Not sure if
that's explicitly do to SIGCHLD, or due to some other side effect of
setting libxl_sigchld_owner_mainloop, or something else entirely.

CC: Nick Rosbrook <[e-mail]>
---
 tools/golang/xenlight/xenlight.go | 34 +++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index e115592257..f70a4c6d96 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -33,6 +33,9 @@  import "C"
 
 import (
 	"fmt"
+	"os"
+	"os/signal"
+	"syscall"
 	"unsafe"
 )
 
@@ -77,8 +80,23 @@  func (e Error) Error() string {
 
 // Context represents a libxl_ctx.
 type Context struct {
-	ctx    *C.libxl_ctx
-	logger *C.xentoollog_logger_stdiostream
+	ctx     *C.libxl_ctx
+	logger  *C.xentoollog_logger_stdiostream
+	sigchld chan os.Signal
+}
+
+// Golang always unmasks SIGCHLD, and internally has ways of
+// distributing SIGCHLD to multiple recipients.  libxl has provision
+// for this model: just tell it when a SIGCHLD happened, and it will
+// look after its own processes.
+//
+// This should "play nicely" with other users of SIGCHLD as long as
+// they don't reap libxl's processes.
+func sigchldHandler(ctx *Context) {
+	for _ = range ctx.sigchld {
+		// Can we spin up another goroutine for this?
+		C.libxl_childproc_sigchld_occurred(ctx.ctx)
+	}
 }
 
 // NewContext returns a new Context.
@@ -93,6 +111,15 @@  func NewContext() (*Context, error) {
 		return nil, Error(ret)
 	}
 
+	ctx.sigchld = make(chan os.Signal, 2)
+	signal.Notify(ctx.sigchld, syscall.SIGCHLD)
+
+	go sigchldHandler(&ctx)
+
+	C.libxl_childproc_setmode(ctx.ctx,
+		&C.libxl_childproc_hooks{chldowner: C.libxl_sigchld_owner_mainloop},
+		nil)
+
 	return &ctx, nil
 }
 
@@ -102,6 +129,9 @@  func (ctx *Context) Close() error {
 	ctx.ctx = nil
 	C.xtl_logger_destroy((*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
 
+	signal.Stop(ctx.sigchld)
+	close(ctx.sigchld)
+
 	if ret != 0 {
 		return Error(ret)
 	}