[PATCH v12 05/27] tools/libxenevtchn: add possibility to not close file descriptor on exec

From: Juergen Gross <jgross@suse.com>
To: xen-devel@lists.xenproject.org
Cc: Juergen Gross <jgross@suse.com>, Ian Jackson <iwj@xenproject.org>,
	Wei Liu <wl@xen.org>, Julien Grall <jgrall@amazon.com>
Subject: [PATCH v12 05/27] tools/libxenevtchn: add possibility to not close file descriptor on exec
Date: Fri, 15 Jan 2021 09:29:38 +0100	[thread overview]
Message-ID: <20210115083000.14186-6-jgross@suse.com> (raw)
In-Reply-To: <20210115083000.14186-1-jgross@suse.com>

Today the file descriptor for the access of the event channel driver
is being closed in case of exec(2). For the support of live update of
a daemon using libxenevtchn this can be problematic, so add a way to
keep that file descriptor open.

Add support of a flag XENEVTCHN_NO_CLOEXEC for xenevtchn_open() which
will result in _not_ setting O_CLOEXEC when opening the event channel
driver node.

The caller can then obtain the file descriptor via xenevtchn_fd().

Add an alternative open function xenevtchn_fdopen() which takes that
file descriptor as an additional parameter. This allows to allocate a
xenevtchn_handle and to associate it with that file descriptor.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wl@xen.org>
Reviewed-by: Julien Grall <jgrall@amazon.com>
---
V7:
- new patch

V8:
- some minor comments by Julien Grall addressed

V11:
- rename to xenevtchn_fdopen() (Andrew Cooper)

V12:
- better comments in header file (Andrew Cooper)
- reject flags != 0 if fdopen (Andrew Cooper)
- style (Andrew Cooper)
---
 tools/include/xenevtchn.h          | 50 +++++++++++++++---------
 tools/libs/evtchn/Makefile         |  2 +-
 tools/libs/evtchn/core.c           | 62 ++++++++++++++++++++++--------
 tools/libs/evtchn/freebsd.c        |  7 +++-
 tools/libs/evtchn/libxenevtchn.map |  4 ++
 tools/libs/evtchn/linux.c          |  6 ++-
 tools/libs/evtchn/minios.c         |  4 ++
 7 files changed, 97 insertions(+), 38 deletions(-)

diff --git a/tools/include/xenevtchn.h b/tools/include/xenevtchn.h
index 3e9b6e7323..10dbe1a06e 100644
--- a/tools/include/xenevtchn.h
+++ b/tools/include/xenevtchn.h
@@ -42,35 +42,47 @@ struct xentoollog_logger;
  */
 
 /*
- * Return a handle to the event channel driver, or NULL on failure, in
- * which case errno will be set appropriately.
+ * Opens the evtchn device node.  Return a handle to the event channel
+ * driver, or NULL on failure, in which case errno will be set
+ * appropriately.
  *
- * Note: After fork(2) a child process must not use any opened evtchn
- * handle inherited from their parent, nor access any grant mapped
- * areas associated with that handle.
+ * On fork(2):
  *
- * The child must open a new handle if they want to interact with
- * evtchn.
+ *   After fork, a child process must not use any opened evtchn handle
+ *   inherited from their parent.  This includes operations such as
+ *   poll() on the underlying file descriptor.  Calling xenevtchn_close()
+ *   is the only safe operation on a xenevtchn_handle which has been
+ *   inherited.
  *
- * Calling exec(2) in a child will safely (and reliably) reclaim any
- * allocated resources via a xenevtchn_handle in the parent.
+ *   The child must open a new handle if they want to interact with
+ *   evtchn.
  *
- * A child which does not call exec(2) may safely call
- * xenevtchn_close() on a xenevtchn_handle inherited from their
- * parent. This will attempt to reclaim any resources associated with
- * that handle. Note that in some implementations this reclamation may
- * not be completely effective, in this case any affected resources
- * remain allocated.
+ * On exec(2):
  *
- * Calling xenevtchn_close() is the only safe operation on a
- * xenevtchn_handle which has been inherited.
+ *   Wherever possible, the device node will be opened with O_CLOEXEC,
+ *   so it is not inherited by the subsequent program.
+ *
+ *   However the XENEVTCHN_NO_CLOEXEC flag may be used to avoid opening
+ *   the device node with O_CLOEXEC.  This is intended for use by
+ *   daemons which support a self-reexec method of updating themselves.
+ *
+ *   In this case, the updated daemon should pass the underlying file
+ *   descriptor it inherited to xenevtchn_fdopen() to reconstruct the
+ *   library handle.
  */
-/* Currently no flags are defined */
+
+/* Don't set O_CLOEXEC when opening event channel driver node. */
+#define XENEVTCHN_NO_CLOEXEC (1U <<0)
+
 xenevtchn_handle *xenevtchn_open(struct xentoollog_logger *logger,
                                  unsigned int flags);
 
+/* Flag XENEVTCHN_NO_CLOEXEC is rejected by xenevtchn_fdopen(). */
+xenevtchn_handle *xenevtchn_fdopen(struct xentoollog_logger *logger,
+                                    int fd, unsigned open_flags);
+
 /*
- * Close a handle previously allocated with xenevtchn_open().
+ * Close a handle previously allocated with xenevtchn_{,fd}open().
  */
 int xenevtchn_close(xenevtchn_handle *xce);
 
diff --git a/tools/libs/evtchn/Makefile b/tools/libs/evtchn/Makefile
index ad01a17b3d..b8c37b5b97 100644
--- a/tools/libs/evtchn/Makefile
+++ b/tools/libs/evtchn/Makefile
@@ -2,7 +2,7 @@ XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
 MAJOR    = 1
-MINOR    = 1
+MINOR    = 2
 
 SRCS-y                 += core.c
 SRCS-$(CONFIG_Linux)   += linux.c
diff --git a/tools/libs/evtchn/core.c b/tools/libs/evtchn/core.c
index d3cc93e98f..abebf72559 100644
--- a/tools/libs/evtchn/core.c
+++ b/tools/libs/evtchn/core.c
@@ -30,18 +30,10 @@ static int all_restrict_cb(Xentoolcore__Active_Handle *ah, domid_t domid)
     return xenevtchn_restrict(xce, domid);
 }
 
-xenevtchn_handle *xenevtchn_open(xentoollog_logger *logger, unsigned int flags)
+static xenevtchn_handle *xenevtchn_alloc_handle(xentoollog_logger *logger)
 {
-    xenevtchn_handle *xce;
-    int rc;
-
-    if ( flags )
-    {
-        errno = EINVAL;
-        return NULL;
-    }
+    xenevtchn_handle *xce = malloc(sizeof(*xce));
 
-    xce = malloc(sizeof(*xce));
     if ( !xce )
         return NULL;
 
@@ -60,21 +52,59 @@ xenevtchn_handle *xenevtchn_open(xentoollog_logger *logger, unsigned int flags)
             goto err;
     }
 
+    return xce;
+
+err:
+    xenevtchn_close(xce);
+    return NULL;
+}
+
+xenevtchn_handle *xenevtchn_open(xentoollog_logger *logger, unsigned int flags)
+{
+    xenevtchn_handle *xce;
+    int rc;
+
+    if ( flags & ~XENEVTCHN_NO_CLOEXEC )
+    {
+        errno = EINVAL;
+        return NULL;
+    }
+
+    xce = xenevtchn_alloc_handle(logger);
+    if ( !xce )
+        return NULL;
+
     rc = osdep_evtchn_open(xce, flags);
     if ( rc < 0 )
         goto err;
 
     return xce;
 
- err:
-    xentoolcore__deregister_active_handle(&xce->tc_ah);
-    osdep_evtchn_close(xce);
-    xtl_logger_destroy(xce->logger_tofree);
-    free(xce);
-
+err:
+    xenevtchn_close(xce);
     return NULL;
 }
 
+xenevtchn_handle *xenevtchn_fdopen(struct xentoollog_logger *logger,
+                                   int fd, unsigned int flags)
+{
+    xenevtchn_handle *xce;
+
+    if ( flags )
+    {
+        errno = EINVAL;
+        return NULL;
+    }
+
+    xce = xenevtchn_alloc_handle(logger);
+    if ( !xce )
+        return NULL;
+
+    xce->fd = fd;
+
+    return xce;
+}
+
 int xenevtchn_close(xenevtchn_handle *xce)
 {
     int rc;
diff --git a/tools/libs/evtchn/freebsd.c b/tools/libs/evtchn/freebsd.c
index bb601f350f..7427ab2408 100644
--- a/tools/libs/evtchn/freebsd.c
+++ b/tools/libs/evtchn/freebsd.c
@@ -33,8 +33,13 @@
 
 int osdep_evtchn_open(xenevtchn_handle *xce, unsigned int flags)
 {
-    int fd = open(EVTCHN_DEV, O_RDWR|O_CLOEXEC);
+    int open_flags = O_RDWR;
+    int fd;
 
+    if ( !(flags & XENEVTCHN_NO_CLOEXEC) )
+        open_flags |= O_CLOEXEC;
+
+    fd = open(EVTCHN_DEV, open_flags);
     if ( fd == -1 )
         return -1;
 
diff --git a/tools/libs/evtchn/libxenevtchn.map b/tools/libs/evtchn/libxenevtchn.map
index 33a38f953a..4c180ea65d 100644
--- a/tools/libs/evtchn/libxenevtchn.map
+++ b/tools/libs/evtchn/libxenevtchn.map
@@ -21,3 +21,7 @@ VERS_1.1 {
 	global:
 		xenevtchn_restrict;
 } VERS_1.0;
+VERS_1.2 {
+	global:
+		xenevtchn_fdopen;
+} VERS_1.1;
diff --git a/tools/libs/evtchn/linux.c b/tools/libs/evtchn/linux.c
index 62adc0e574..60bb75a791 100644
--- a/tools/libs/evtchn/linux.c
+++ b/tools/libs/evtchn/linux.c
@@ -36,8 +36,12 @@
 
 int osdep_evtchn_open(xenevtchn_handle *xce, unsigned int flags)
 {
-    int fd = open("/dev/xen/evtchn", O_RDWR|O_CLOEXEC);
+    int open_flags = O_RDWR;
+    int fd;
 
+    if ( !(flags & XENEVTCHN_NO_CLOEXEC) )
+        open_flags |= O_CLOEXEC;
+    fd = open("/dev/xen/evtchn", open_flags);
     if ( fd == -1 )
         return -1;
 
diff --git a/tools/libs/evtchn/minios.c b/tools/libs/evtchn/minios.c
index 3afc8e2517..57f712ade3 100644
--- a/tools/libs/evtchn/minios.c
+++ b/tools/libs/evtchn/minios.c
@@ -69,6 +69,10 @@ static void port_dealloc(struct evtchn_port_info *port_info)
     free(port_info);
 }
 
+/*
+ * XENEVTCHN_NO_CLOEXEC is being ignored, as there is no exec() call supported
+ * in Mini-OS.
+ */
 int osdep_evtchn_open(xenevtchn_handle *xce, unsigned int flags)
 {
     int fd = alloc_fd(FTYPE_EVTCHN);
-- 
2.26.2


