Libguestfs - Feb 2013 - [PATCH 0/7] Fix SELinux security contexts so we can access shared disks (RHBZ#912499).

https://bugzilla.redhat.com/show_bug.cgi?id=912499
(especially comments 7 & 10)

This patch set is the final fix so that we can access disks in use by
other guests when SELinux and sVirt are enabled.

Previously such disks were inaccessible because sVirt labels the disks
with a random SELinux label to prevent other instances of qemu from
being able to read them.  So naturally the libguestfs appliance
(ie. qemu) cannot read these disks, not even if it is running as root.

The fix is to read that SELinux label from libvirt, then label the
libguestfs appliance the same way, so it can access the disks.

This is done by modifying the guestfs_add_domain call so that it reads
the label from libvirt, and passes the label through to the libvirt
back end which uses it when creating the appliance.  It's actually a
bit more complex than this because we have to label the overlay disks
and the appliance disk correctly.

These patches are only the final part of the whole fix.  I have
already pushed fixes to virt-df and virt-alignment-scan (34e77af1bf +
6e3aab2f0c + several dependent commits) so that these tools now use
just one appliance per guest, allowing us to label that appliance
correctly, because if an appliance is attached to multiple guests then
there is no single label that could be used.

As a result of the total complexity of this fix, I very much doubt
that it can be backported to 1.20.

Rich.

From: "Richard W.M. Jones" <rjones at redhat.com>

This is just code motion.
---
 src/launch-libvirt.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 6ad19de..7db2ce5 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -138,6 +138,7 @@ static int make_qcow2_overlay_for_drive (guestfs_h *g,
struct drive *drv);
 static void drive_free_priv (void *);
 static void set_socket_create_context (guestfs_h *g);
 static void clear_socket_create_context (guestfs_h *g);
+static void selinux_warning (guestfs_h *g, const char *func, const char
*selinux_op, const char *data);
 
 static int
 launch_libvirt (guestfs_h *g, const char *libvirt_uri)
@@ -588,30 +589,27 @@ set_socket_create_context (guestfs_h *g)
   context_t con;
 
   if (getcon (&scon) == -1) {
-    debug (g, "%s: getcon failed: %m", __func__);
+    selinux_warning (g, __func__, "getcon", NULL);
     return;
   }
 
   con = context_new (scon);
   if (!con) {
-    debug (g, "%s: context_new failed: %m", __func__);
+    selinux_warning (g, __func__, "context_new", scon);
     goto out1;
   }
 
   if (context_type_set (con, SOCKET_CONTEXT) == -1) {
-    debug (g, "%s: context_type_set failed: %m", __func__);
+    selinux_warning (g, __func__, "context_type_set", scon);
     goto out2;
   }
 
-#define SETSOCKCREATECON_WARNING_NOTICE "[you can ignore this UNLESS using
SELinux + sVirt]"
-
   /* Note that setsockcreatecon sets the per-thread socket creation
    * context (/proc/self/task/<tid>/attr/sockcreate) so this is
    * thread-safe.
    */
   if (setsockcreatecon (context_str (con)) == -1) {
-    debug (g, "%s: setsockcreatecon (%s) failed: %m %s",
-           __func__, context_str (con), SETSOCKCREATECON_WARNING_NOTICE);
+    selinux_warning (g, __func__, "setsockcreatecon", context_str
(con));
     goto out2;
   }
 
@@ -625,8 +623,7 @@ static void
 clear_socket_create_context (guestfs_h *g)
 {
   if (setsockcreatecon (NULL) == -1)
-    debug (g, "%s: setsockcreatecon (NULL) failed: %m %s", __func__,
-           SETSOCKCREATECON_WARNING_NOTICE);
+    selinux_warning (g, __func__, "setsockcreatecon",
"NULL");
 }
 
 #else /* !HAVE_LIBSELINUX */
@@ -1454,6 +1451,15 @@ libvirt_error (guestfs_h *g, const char *fs, ...)
   free (msg);
 }
 
+static void
+selinux_warning (guestfs_h *g, const char *func,
+                 const char *selinux_op, const char *data)
+{
+  debug (g, "%s: %s failed: %s: %m"
+         " [you can ignore this UNLESS using SELinux + sVirt]",
+         func, selinux_op, data ? data : "(none)");
+}
+
 /* This backend assumes virtio-scsi is available. */
 static int
 max_disks_libvirt (guestfs_h *g)
-- 
1.8.1.2

From: "Richard W.M. Jones" <rjones at redhat.com>

This internal API sets two SELinux labels in the handle (the process
label and the image label -- they are closely related).

If using the libvirt attach-method with SELinux and sVirt, then this
will cause the following XML to be added to the appliance definition:

<seclabel type=static model=selinux relabel=yes>
  <label>[LABEL HERE]</label>
  <imagelabel>[IMAGELABEL HERE]</imagelabel>
</seclabel>

It is ignored by other attach-methods.
---
 generator/actions.ml   | 12 ++++++++++++
 src/guestfs-internal.h |  2 ++
 src/handle.c           |  2 ++
 src/launch-libvirt.c   | 36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+)

diff --git a/generator/actions.ml b/generator/actions.ml
index 8a8e3ff..59e667d 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -2694,6 +2694,18 @@ the default.  Else C</var/tmp> is the
default." };
     longdesc = "\
 Get the directory used by the handle to store the appliance cache." };
 
+  { defaults with
+    name = "internal_set_libvirt_selinux_label";
+    style = RErr, [String "label"; String "imagelabel"],
[];
+    blocking = false;
+    visibility = VInternal;
+    shortdesc = "set SELinux label used by the libvirt attach
method";
+    longdesc = "\
+This internal function sets the SELinux security label (in
+reality, two labels: the process label and the image label)
+used by the appliance when the libvirt attach method is selected
+(it is ignored by other attach methods)." };
+
 ]
 
 (* daemon_functions are any functions which cause some action
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index e1a7d31..78e2bf5 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -322,6 +322,8 @@ struct guestfs_h
     virDomainPtr dom;           /* libvirt domain */
   } virt;
 #endif
+  char *virt_selinux_label;
+  char *virt_selinux_imagelabel;
 };
 
 /* Per-filesystem data stored for inspect_os. */
diff --git a/src/handle.c b/src/handle.c
index c630daf..2f44632 100644
--- a/src/handle.c
+++ b/src/handle.c
@@ -326,6 +326,8 @@ guestfs_close (guestfs_h *g)
 
   if (g->pda)
     hash_free (g->pda);
+  free (g->virt_selinux_label);
+  free (g->virt_selinux_imagelabel);
   free (g->tmpdir);
   free (g->env_tmpdir);
   free (g->int_tmpdir);
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 7db2ce5..0a59cb6 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -855,6 +855,31 @@ construct_libvirt_xml_seclabel (guestfs_h *g,
                                            BAD_CAST "none"));
     XMLERROR (-1, xmlTextWriterEndElement (xo));
   }
+  else if (g->virt_selinux_label) {
+    /* Enable sVirt and pass a custom <seclabel/> inherited from the
+     * original libvirt domain (when guestfs_add_domain was called).
+     * https://bugzilla.redhat.com/show_bug.cgi?id=912499#c7
+     */
+    XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"seclabel"));
+    XMLERROR (-1,
+              xmlTextWriterWriteAttribute (xo, BAD_CAST "type",
+                                           BAD_CAST "static"));
+    XMLERROR (-1,
+              xmlTextWriterWriteAttribute (xo, BAD_CAST "model",
+                                           BAD_CAST "selinux"));
+    XMLERROR (-1,
+              xmlTextWriterWriteAttribute (xo, BAD_CAST "relabel",
+                                           BAD_CAST "yes"));
+    XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "label"));
+    XMLERROR (-1, xmlTextWriterWriteString (xo,
+                                            BAD_CAST
g->virt_selinux_label));
+    XMLERROR (-1, xmlTextWriterEndElement (xo));
+    XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"imagelabel"));
+    XMLERROR (-1, xmlTextWriterWriteString (xo,
+                                            BAD_CAST
g->virt_selinux_imagelabel));
+    XMLERROR (-1, xmlTextWriterEndElement (xo));
+    XMLERROR (-1, xmlTextWriterEndElement (xo));
+  }
 
   return 0;
 }
@@ -1603,3 +1628,14 @@ struct attach_ops attach_ops_libvirt = {
 };
 
 #endif /* no libvirt or libxml2 at compile time */
+
+int
+guestfs__internal_set_libvirt_selinux_label (guestfs_h *g, const char *label,
+                                             const char *imagelabel)
+{
+  free (g->virt_selinux_label);
+  g->virt_selinux_label = safe_strdup (g, label);
+  free (g->virt_selinux_imagelabel);
+  g->virt_selinux_imagelabel = safe_strdup (g, imagelabel);
+  return 0;
+}
-- 
1.8.1.2

From: "Richard W.M. Jones" <rjones at redhat.com>

If set, this causes <seclabel model=selinux relabel=no> to be added to
the disk element in the libvirt XML.

It has no effect *except* on the libvirt attach method when SELinux
and sVirt is being used.
---
 generator/actions.ml   |  8 +++++++-
 src/guestfs-internal.h |  1 +
 src/launch-libvirt.c   | 20 ++++++++++++++++++++
 src/launch.c           | 21 ++++++++++++++-------
 4 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/generator/actions.ml b/generator/actions.ml
index 59e667d..ececa7b 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -1247,7 +1247,7 @@ not all belong to a single logical operating system
 
   { defaults with
     name = "add_drive";
-    style = RErr, [String "filename"], [OBool "readonly";
OString "format"; OString "iface"; OString "name";
OString "label"];
+    style = RErr, [String "filename"], [OBool "readonly";
OString "format"; OString "iface"; OString "name";
OString "label"; OBool "selinuxnorelabel"];
     once_had_no_optargs = true;
     blocking = false;
     fish_alias = ["add"];
@@ -1319,6 +1319,12 @@ the drive will also be named
C</dev/disk/guestfs/I<label>>.
 
 See L<guestfs(3)/DISK LABELS>.
 
+=item C<selinuxnorelabel>
+
+If set, this option (only applicable when using the libvirt
+attach-method with SELinux and sVirt) causes libvirt to I<not>
+relabel the disk.
+
 =back" };
 
   { defaults with
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 78e2bf5..9a468dc 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -119,6 +119,7 @@ struct drive {
   char *name;
   char *disk_label;
   bool use_cache_none;
+  int selinuxnorelabel;
 
   void *priv;                   /* Data used by attach method. */
   void (*free_priv) (void *);
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 0a59cb6..45950e2 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -1035,6 +1035,16 @@ construct_libvirt_xml_disk (guestfs_h *g,
     XMLERROR (-1,
               xmlTextWriterWriteAttribute (xo, BAD_CAST "file",
                                            BAD_CAST drv_priv->path));
+    if (drv->selinuxnorelabel) {
+      XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"seclabel"));
+      XMLERROR (-1,
+                xmlTextWriterWriteAttribute (xo, BAD_CAST "model",
+                                             BAD_CAST "selinux"));
+      XMLERROR (-1,
+                xmlTextWriterWriteAttribute (xo, BAD_CAST "relabel",
+                                             BAD_CAST "no"));
+      XMLERROR (-1, xmlTextWriterEndElement (xo));
+    }
     XMLERROR (-1, xmlTextWriterEndElement (xo));
   }
   else {
@@ -1046,6 +1056,16 @@ construct_libvirt_xml_disk (guestfs_h *g,
     XMLERROR (-1,
               xmlTextWriterWriteAttribute (xo, BAD_CAST "dev",
                                            BAD_CAST drv_priv->path));
+    if (drv->selinuxnorelabel) {
+      XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"seclabel"));
+      XMLERROR (-1,
+                xmlTextWriterWriteAttribute (xo, BAD_CAST "model",
+                                             BAD_CAST "selinux"));
+      XMLERROR (-1,
+                xmlTextWriterWriteAttribute (xo, BAD_CAST "relabel",
+                                             BAD_CAST "no"));
+      XMLERROR (-1, xmlTextWriterEndElement (xo));
+    }
     XMLERROR (-1, xmlTextWriterEndElement (xo));
   }
 
diff --git a/src/launch.c b/src/launch.c
index 7c37667..518ecd7 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -88,7 +88,7 @@ static struct drive *
 create_drive_struct (guestfs_h *g, const char *path,
                      int readonly, const char *format,
                      const char *iface, const char *name,
-                     const char *disk_label,
+                     const char *disk_label, int selinuxnorelabel,
                      int use_cache_none)
 {
   struct drive *drv = safe_malloc (g, sizeof (struct drive));
@@ -100,6 +100,7 @@ create_drive_struct (guestfs_h *g, const char *path,
   drv->name = name ? safe_strdup (g, name) : NULL;
   drv->disk_label = disk_label ? safe_strdup (g, disk_label) : NULL;
   drv->use_cache_none = use_cache_none;
+  drv->selinuxnorelabel = selinuxnorelabel;
   drv->priv = drv->free_priv = NULL;
 
   return drv;
@@ -111,7 +112,7 @@ guestfs___add_dummy_appliance_drive (guestfs_h *g)
 {
   struct drive *drv;
 
-  drv = create_drive_struct (g, "", 0, NULL, NULL, NULL, NULL, 0);
+  drv = create_drive_struct (g, "", 0, NULL, NULL, NULL, NULL, 0, 0);
   add_drive_to_handle (g, drv);
 }
 
@@ -276,7 +277,8 @@ add_drive (guestfs_h *g, struct drive *drv)
  */
 static int
 add_null_drive (guestfs_h *g, int readonly, const char *format,
-                const char *iface, const char *name, const char *disk_label)
+                const char *iface, const char *name, const char *disk_label,
+                int selinuxnorelabel)
 {
   char *tmpfile = NULL;
   int fd = -1, r;
@@ -310,7 +312,8 @@ add_null_drive (guestfs_h *g, int readonly, const char
*format,
     goto err;
   }
 
-  drv = create_drive_struct (g, tmpfile, readonly, format, iface, name,
disk_label, 0);
+  drv = create_drive_struct (g, tmpfile, readonly, format, iface, name,
+                             disk_label, selinuxnorelabel, 0);
   r = add_drive (g, drv);
   free (tmpfile);
   if (r == -1) {
@@ -337,6 +340,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   const char *name;
   const char *disk_label;
   int use_cache_none;
+  int selinuxnorelabel;
   struct drive *drv;
 
   if (strchr (filename, ':') != NULL) {
@@ -355,6 +359,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
          ? optargs->name : NULL;
   disk_label = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_LABEL_BITMASK
          ? optargs->label : NULL;
+  selinuxnorelabel = optargs->bitmask &
GUESTFS_ADD_DRIVE_OPTS_SELINUXNORELABEL_BITMASK
+         ? optargs->selinuxnorelabel : 0;
 
   if (format && !valid_format_iface (format)) {
     error (g, _("%s parameter is empty or contains disallowed
characters"),
@@ -372,7 +378,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   }
 
   if (STREQ (filename, "/dev/null"))
-    return add_null_drive (g, readonly, format, iface, name, disk_label);
+    return add_null_drive (g, readonly, format, iface, name, disk_label,
+                           selinuxnorelabel);
 
   /* For writable files, see if we can use cache=none.  This also
    * checks for the existence of the file.  For readonly we have
@@ -389,8 +396,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
     }
   }
 
-  drv = create_drive_struct (g, filename, readonly, format, iface, name,
disk_label,
-                             use_cache_none);
+  drv = create_drive_struct (g, filename, readonly, format, iface, name,
+                             disk_label, selinuxnorelabel, use_cache_none);
   if (add_drive (g, drv) == -1) {
     free_drive_struct (drv);
     return -1;
-- 
1.8.1.2

From: "Richard W.M. Jones" <rjones at redhat.com>

When a disk is opened readonly, the libvirt attach-method privately
creates a qcow2 overlay on top.

This commit lets that overlay get an SELinux label, and sets it to the
label specified by guestfs_internal_set_libvirt_selinux_label.

We have to adjust the SELinux label (which is a process label) to make
it applicable to images.  We do this by changing the role from
'system_r' to 'object_r', and the type from 'svirt_t' to
'svirt_image_t'.

The above only applies to the libvirt attach-method.
---
 src/launch-libvirt.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 45950e2..5c14155 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -133,8 +133,8 @@ static int is_custom_qemu (guestfs_h *g);
 static int is_blk (const char *path);
 static int random_chars (char *ret, size_t len);
 static void ignore_errors (void *ignore, virErrorPtr ignore2);
-static char *make_qcow2_overlay (guestfs_h *g, const char *path, const char
*format);
-static int make_qcow2_overlay_for_drive (guestfs_h *g, struct drive *drv);
+static char *make_qcow2_overlay (guestfs_h *g, const char *path, const char
*format, bool selinux_label);
+static int make_qcow2_overlay_for_drive (guestfs_h *g, struct drive *drv, bool
selinux_label);
 static void drive_free_priv (void *);
 static void set_socket_create_context (guestfs_h *g);
 static void clear_socket_create_context (guestfs_h *g);
@@ -235,13 +235,13 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
    * Note that appliance can be NULL if using the old-style appliance.
    */
   if (appliance) {
-    params.appliance_overlay = make_qcow2_overlay (g, appliance,
"raw");
+    params.appliance_overlay = make_qcow2_overlay (g, appliance,
"raw", false);
     if (!params.appliance_overlay)
       goto cleanup;
   }
 
   ITER_DRIVES (g, i, drv) {
-    if (make_qcow2_overlay_for_drive (g, drv) == -1)
+    if (make_qcow2_overlay_for_drive (g, drv, true) == -1)
       goto cleanup;
   }
 
@@ -1350,7 +1350,8 @@ ignore_errors (void *ignore, virErrorPtr ignore2)
 
 /* Create a temporary qcow2 overlay on top of 'path'. */
 static char *
-make_qcow2_overlay (guestfs_h *g, const char *path, const char *format)
+make_qcow2_overlay (guestfs_h *g, const char *path, const char *format,
+                    bool selinux_label)
 {
   char *tmpfile = NULL;
   CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g);
@@ -1381,6 +1382,15 @@ make_qcow2_overlay (guestfs_h *g, const char *path, const
char *format)
     goto error;
   }
 
+#if HAVE_LIBSELINUX
+  if (selinux_label && g->virt_selinux_imagelabel) {
+    debug (g, "setting SELinux label on %s to %s",
+           tmpfile, g->virt_selinux_imagelabel);
+    if (setfilecon (tmpfile, g->virt_selinux_imagelabel) == -1)
+      selinux_warning (g, __func__, "setfilecon", tmpfile);
+  }
+#endif
+
   return tmpfile;               /* caller frees */
 
  error:
@@ -1390,7 +1400,8 @@ make_qcow2_overlay (guestfs_h *g, const char *path, const
char *format)
 }
 
 static int
-make_qcow2_overlay_for_drive (guestfs_h *g, struct drive *drv)
+make_qcow2_overlay_for_drive (guestfs_h *g, struct drive *drv,
+                              bool selinux_label)
 {
   char *path;
   struct drive_libvirt *drv_priv;
@@ -1414,7 +1425,7 @@ make_qcow2_overlay_for_drive (guestfs_h *g, struct drive
*drv)
     drv_priv->format = drv->format ? safe_strdup (g, drv->format) :
NULL;
   }
   else {
-    drv_priv->path = make_qcow2_overlay (g, path, drv->format);
+    drv_priv->path = make_qcow2_overlay (g, path, drv->format,
selinux_label);
     free (path);
     if (!drv_priv->path)
       return -1;
@@ -1531,7 +1542,7 @@ hot_add_drive_libvirt (guestfs_h *g, struct drive *drv,
size_t drv_index)
   /* Create overlay for read-only drive.  This works around lack of
    * support for <transient/> disks in libvirt.
    */
-  if (make_qcow2_overlay_for_drive (g, drv) == -1)
+  if (make_qcow2_overlay_for_drive (g, drv, true) == -1)
     return -1;
 
   /* Create the XML for the new disk. */
-- 
1.8.1.2

From: "Richard W.M. Jones" <rjones at redhat.com>

This is just code motion.
---
 src/libvirt-domain.c | 71 +++++++++++++++++++++++++---------------------------
 1 file changed, 34 insertions(+), 37 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index ab776b3..3f5b1ed 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -40,7 +40,8 @@
 
 #if defined(HAVE_LIBVIRT) && defined(HAVE_LIBXML2)
 
-static ssize_t for_each_disk (guestfs_h *g, virDomainPtr dom, int (*f)
(guestfs_h *g, const char *filename, const char *format, int readonly, void
*data), void *data);
+static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom);
+static ssize_t for_each_disk (guestfs_h *g, xmlDocPtr doc, int (*f) (guestfs_h
*g, const char *filename, const char *format, int readonly, void *data), void
*data);
 
 static void
 ignore_errors (void *ignore, virErrorPtr ignore2)
@@ -167,6 +168,7 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
   enum readonlydisk readonlydisk = readonlydisk_write;
   size_t ckp;
   struct add_disk_data data;
+  CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL;
 
   readonly      optargs->bitmask &
GUESTFS___ADD_LIBVIRT_DOM_READONLY_BITMASK
@@ -228,6 +230,10 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
     }
   }
 
+  /* Domain XML. */
+  if ((doc = get_domain_xml (g, dom)) == NULL)
+    return -1;
+
   /* Add the disks. */
   data.optargs.bitmask = 0;
   data.readonly = readonly;
@@ -241,7 +247,7 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
    * all disks are added or none are added.
    */
   ckp = guestfs___checkpoint_drives (g);
-  r = for_each_disk (g, dom, add_disk, &data);
+  r = for_each_disk (g, doc, add_disk, &data);
   if (r == -1)
     guestfs___rollback_drives (g, ckp);
 
@@ -303,29 +309,15 @@ static int
 connect_live (guestfs_h *g, virDomainPtr dom)
 {
   int i;
-  virErrorPtr err;
   CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL;
   CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL;
   CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL;
-  CLEANUP_FREE char *xml = NULL, *path = NULL, *attach_method = NULL;
+  CLEANUP_FREE char *path = NULL, *attach_method = NULL;
   xmlNodeSetPtr nodes;
 
   /* Domain XML. */
-  xml = virDomainGetXMLDesc (dom, 0);
-
-  if (!xml) {
-    err = virGetLastError ();
-    error (g, _("error reading libvirt XML information: %s"),
-           err->message);
+  if ((doc = get_domain_xml (g, dom)) == NULL)
     return -1;
-  }
-
-  /* Parse XML to document. */
-  doc = xmlParseMemory (xml, strlen (xml));
-  if (doc == NULL) {
-    error (g, _("unable to parse XML information returned by
libvirt"));
-    return -1;
-  }
 
   xpathCtx = xmlXPathNewContext (doc);
   if (xpathCtx == NULL) {
@@ -388,7 +380,7 @@ connect_live (guestfs_h *g, virDomainPtr dom)
  */
 static ssize_t
 for_each_disk (guestfs_h *g,
-               virDomainPtr dom,
+               xmlDocPtr doc,
                int (*f) (guestfs_h *g,
                          const char *filename, const char *format,
                          int readonly,
@@ -396,31 +388,13 @@ for_each_disk (guestfs_h *g,
                void *data)
 {
   size_t i, nr_added = 0, nr_nodes;
-  virErrorPtr err;
-  CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL;
   CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL;
   CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL;
-  CLEANUP_FREE char *xml = NULL;
   xmlNodeSetPtr nodes;
 
-  /* Domain XML. */
-  xml = virDomainGetXMLDesc (dom, 0);
-
-  if (!xml) {
-    err = virGetLastError ();
-    error (g, _("error reading libvirt XML information: %s"),
err->message);
-    return -1;
-  }
-
   /* Now the horrible task of parsing out the fields we need from the XML.
    * http://www.xmlsoft.org/examples/xpath1.c
    */
-  doc = xmlParseMemory (xml, strlen (xml));
-  if (doc == NULL) {
-    error (g, _("unable to parse XML information returned by
libvirt"));
-    return -1;
-  }
-
   xpathCtx = xmlXPathNewContext (doc);
   if (xpathCtx == NULL) {
     error (g, _("unable to create new XPath context"));
@@ -533,6 +507,29 @@ for_each_disk (guestfs_h *g,
   return nr_added;
 }
 
+static xmlDocPtr
+get_domain_xml (guestfs_h *g, virDomainPtr dom)
+{
+  virErrorPtr err;
+  xmlDocPtr doc;
+
+  CLEANUP_FREE char *xml = virDomainGetXMLDesc (dom, 0);
+  if (!xml) {
+    err = virGetLastError ();
+    error (g, _("error reading libvirt XML information: %s"),
err->message);
+    return NULL;
+  }
+
+  /* Parse the domain XML into an XML document. */
+  doc = xmlParseMemory (xml, strlen (xml));
+  if (doc == NULL) {
+    error (g, _("unable to parse XML information returned by
libvirt"));
+    return NULL;
+  }
+
+  return doc;
+}
+
 #else /* no libvirt or libxml2 at compile time */
 
 #define NOT_IMPL(r)                                                     \
-- 
1.8.1.2

From: "Richard W.M. Jones" <rjones at redhat.com>

When adding a domain (ie. guestfs_add_domain), read the SELinux
<label/> and <imagelabel/> from the guest and use them for the
appliance.  The appliance is statically labelled the same as the
guest, so it is able to read its disks.

However tell libvirt not to try relabelling the disks, to prevent
libvirt from disturbing the existing labels on the disks (in
particular when the libvirt connection is closed, we don't want
libvirt to try to restore some other label on the disks).
---
 src/libvirt-domain.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 3f5b1ed..15be611 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -20,6 +20,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <assert.h>
 
 #ifdef HAVE_LIBVIRT
@@ -42,6 +43,7 @@
 
 static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom);
 static ssize_t for_each_disk (guestfs_h *g, xmlDocPtr doc, int (*f) (guestfs_h
*g, const char *filename, const char *format, int readonly, void *data), void
*data);
+static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char
**label_rtn, char **imagelabel_rtn);
 
 static void
 ignore_errors (void *ignore, virErrorPtr ignore2)
@@ -169,6 +171,8 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
   size_t ckp;
   struct add_disk_data data;
   CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL;
+  CLEANUP_FREE char *label = NULL, *imagelabel = NULL;
+  int selinuxnorelabel;
 
   readonly      optargs->bitmask &
GUESTFS___ADD_LIBVIRT_DOM_READONLY_BITMASK
@@ -234,6 +238,16 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
   if ((doc = get_domain_xml (g, dom)) == NULL)
     return -1;
 
+  /* Find and pass the SELinux security label to the libvirt back end. */
+  if (libvirt_selinux_label (g, doc, &label, &imagelabel) == -1)
+    return -1;
+  if (label && imagelabel) {
+    guestfs_internal_set_libvirt_selinux_label (g, label, imagelabel);
+    selinuxnorelabel = 1;
+  }
+  else
+    selinuxnorelabel = 0;
+
   /* Add the disks. */
   data.optargs.bitmask = 0;
   data.readonly = readonly;
@@ -242,6 +256,10 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
     data.optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_IFACE_BITMASK;
     data.optargs.iface = iface;
   }
+  if (selinuxnorelabel) {
+    data.optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SELINUXNORELABEL_BITMASK;
+    data.optargs.selinuxnorelabel = 1;
+  }
 
   /* Checkpoint the command line around the operation so that either
    * all disks are added or none are added.
@@ -374,6 +392,66 @@ connect_live (guestfs_h *g, virDomainPtr dom)
   return guestfs_set_attach_method (g, attach_method);
 }
 
+/* Find the <seclabel/> element in the libvirt XML, and if it exists
+ * get the SELinux process label and image label from it.
+ *
+ * The reason for all this is because of sVirt:
+ * https://bugzilla.redhat.com/show_bug.cgi?id=912499#c7
+ */
+static int
+libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc,
+                       char **label_rtn, char **imagelabel_rtn)
+{
+  CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL;
+  CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL;
+  xmlNodeSetPtr nodes;
+  size_t nr_nodes;
+  xmlNodePtr node, child;
+  bool gotlabel = 0, gotimagelabel = 0;
+
+  xpathCtx = xmlXPathNewContext (doc);
+  if (xpathCtx == NULL) {
+    error (g, _("unable to create new XPath context"));
+    return -1;
+  }
+
+  /* This gives us a set of all the <seclabel/> nodes, hopefully only
one. */
+  xpathObj = xmlXPathEvalExpression (BAD_CAST "/domain/seclabel",
+                                     xpathCtx);
+  if (xpathObj == NULL) {
+    error (g, _("unable to evaluate XPath expression"));
+    return -1;
+  }
+
+  nodes = xpathObj->nodesetval;
+  nr_nodes = nodes->nodeNr;
+
+  if (nr_nodes == 0 || nr_nodes > 1)
+    return 0;
+
+  node = nodes->nodeTab[0];
+  if (node->type != XML_ELEMENT_NODE) {
+    error (g, _("expected <seclabel/> to be an XML element"));
+    return -1;
+  }
+
+  /* Find the <label/> and <imagelabel/> child nodes. */
+  for (child = node->children; child != NULL; child = child->next) {
+    if (!gotlabel && STREQ ((char *) child->name,
"label")) {
+      /* Get the label content. */
+      *label_rtn = (char *) xmlNodeGetContent (child);
+      gotlabel = 1;
+    }
+    if (!gotimagelabel && STREQ ((char *) child->name,
"imagelabel")) {
+      /* Get the imagelabel content. */
+      *imagelabel_rtn = (char *) xmlNodeGetContent (child);
+      gotimagelabel = 1;
+    }
+  }
+
+  return 0;
+}
+
 /* The callback function 'f' is called once for each disk.
  *
  * Returns number of disks, or -1 if there was an error.
-- 
1.8.1.2

From: "Richard W.M. Jones" <rjones at redhat.com>

This is just code motion.
---
 src/libvirt-domain.c | 138 +++++++++++++++++++++++++--------------------------
 1 file changed, 69 insertions(+), 69 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 15be611..22d9803 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -323,75 +323,6 @@ add_disk (guestfs_h *g,
   return guestfs__add_drive_opts (g, filename, &optargs);
 }
 
-static int
-connect_live (guestfs_h *g, virDomainPtr dom)
-{
-  int i;
-  CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL;
-  CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL;
-  CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL;
-  CLEANUP_FREE char *path = NULL, *attach_method = NULL;
-  xmlNodeSetPtr nodes;
-
-  /* Domain XML. */
-  if ((doc = get_domain_xml (g, dom)) == NULL)
-    return -1;
-
-  xpathCtx = xmlXPathNewContext (doc);
-  if (xpathCtx == NULL) {
-    error (g, _("unable to create new XPath context"));
-    return -1;
-  }
-
-  /* This gives us a set of all the <channel> nodes related to the
-   * guestfsd virtio-serial channel.
-   */
-  xpathObj = xmlXPathEvalExpression (BAD_CAST
-      "//devices/channel[@type=\"unix\" and "
-                        "./source/@mode=\"bind\" and "
-                        "./source/@path and "
-                        "./target/@type=\"virtio\" and "
-                       
"./target/@name=\"org.libguestfs.channel.0\"]",
-                                     xpathCtx);
-  if (xpathObj == NULL) {
-    error (g, _("unable to evaluate XPath expression"));
-    return -1;
-  }
-
-  nodes = xpathObj->nodesetval;
-  for (i = 0; i < nodes->nodeNr; ++i) {
-    CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppath = NULL;
-    xmlAttrPtr attr;
-
-    /* See note in function above. */
-    xpathCtx->node = nodes->nodeTab[i];
-
-    /* The path is in <source path=..> attribute. */
-    xppath = xmlXPathEvalExpression (BAD_CAST "./source/@path",
xpathCtx);
-    if (xppath == NULL ||
-        xppath->nodesetval == NULL ||
-        xppath->nodesetval->nodeNr == 0) {
-      xmlXPathFreeObject (xppath);
-      continue;                 /* no type attribute, skip it */
-    }
-    assert (xppath->nodesetval->nodeTab[0]);
-    assert (xppath->nodesetval->nodeTab[0]->type ==
XML_ATTRIBUTE_NODE);
-    attr = (xmlAttrPtr) xppath->nodesetval->nodeTab[0];
-    path = (char *) xmlNodeListGetString (doc, attr->children, 1);
-    break;
-  }
-
-  if (path == NULL) {
-    error (g, _("this guest has no libvirt <channel> definition for
guestfsd\n"
-                "See ATTACHING TO RUNNING DAEMONS in guestfs(3) for
further information."));
-    return -1;
-  }
-
-  /* Got a path. */
-  attach_method = safe_asprintf (g, "unix:%s", path);
-  return guestfs_set_attach_method (g, attach_method);
-}
-
 /* Find the <seclabel/> element in the libvirt XML, and if it exists
  * get the SELinux process label and image label from it.
  *
@@ -585,6 +516,75 @@ for_each_disk (guestfs_h *g,
   return nr_added;
 }
 
+static int
+connect_live (guestfs_h *g, virDomainPtr dom)
+{
+  int i;
+  CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL;
+  CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL;
+  CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL;
+  CLEANUP_FREE char *path = NULL, *attach_method = NULL;
+  xmlNodeSetPtr nodes;
+
+  /* Domain XML. */
+  if ((doc = get_domain_xml (g, dom)) == NULL)
+    return -1;
+
+  xpathCtx = xmlXPathNewContext (doc);
+  if (xpathCtx == NULL) {
+    error (g, _("unable to create new XPath context"));
+    return -1;
+  }
+
+  /* This gives us a set of all the <channel> nodes related to the
+   * guestfsd virtio-serial channel.
+   */
+  xpathObj = xmlXPathEvalExpression (BAD_CAST
+      "//devices/channel[@type=\"unix\" and "
+                        "./source/@mode=\"bind\" and "
+                        "./source/@path and "
+                        "./target/@type=\"virtio\" and "
+                       
"./target/@name=\"org.libguestfs.channel.0\"]",
+                                     xpathCtx);
+  if (xpathObj == NULL) {
+    error (g, _("unable to evaluate XPath expression"));
+    return -1;
+  }
+
+  nodes = xpathObj->nodesetval;
+  for (i = 0; i < nodes->nodeNr; ++i) {
+    CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppath = NULL;
+    xmlAttrPtr attr;
+
+    /* See note in function above. */
+    xpathCtx->node = nodes->nodeTab[i];
+
+    /* The path is in <source path=..> attribute. */
+    xppath = xmlXPathEvalExpression (BAD_CAST "./source/@path",
xpathCtx);
+    if (xppath == NULL ||
+        xppath->nodesetval == NULL ||
+        xppath->nodesetval->nodeNr == 0) {
+      xmlXPathFreeObject (xppath);
+      continue;                 /* no type attribute, skip it */
+    }
+    assert (xppath->nodesetval->nodeTab[0]);
+    assert (xppath->nodesetval->nodeTab[0]->type ==
XML_ATTRIBUTE_NODE);
+    attr = (xmlAttrPtr) xppath->nodesetval->nodeTab[0];
+    path = (char *) xmlNodeListGetString (doc, attr->children, 1);
+    break;
+  }
+
+  if (path == NULL) {
+    error (g, _("this guest has no libvirt <channel> definition for
guestfsd\n"
+                "See ATTACHING TO RUNNING DAEMONS in guestfs(3) for
further information."));
+    return -1;
+  }
+
+  /* Got a path. */
+  attach_method = safe_asprintf (g, "unix:%s", path);
+  return guestfs_set_attach_method (g, attach_method);
+}
+
 static xmlDocPtr
 get_domain_xml (guestfs_h *g, virDomainPtr dom)
 {
-- 
1.8.1.2