[Libguestfs] [PATCH v2 03/18] New API parameter: Add discard parameter to guestfs_add_drive_opts.

Tuesday, 11 March 2014

This adds a discard parameter to the guestfs_add_drive_opts which
approximately maps to the discard=ignore|unmap parameter supported by
qemu.

If discard is set to "enable" then we force discard=unmap (and try to
fail if it is not possible).  If discard is set to the more useful
"besteffort" option, then we enable discard if possible.  The default
is "disable".
---
 generator/actions.ml   |  32 +++++++++++++-
 src/drives.c           | 107 +++++++++++++++++++++++++++++---------------
 src/guestfs-internal.h |   9 ++++
 src/launch-direct.c    | 117 ++++++++++++++++++++++++++++++++++++++++++++++++-
 src/launch-libvirt.c   |  94 ++++++++++++++++++++++++++++++++++++---
 src/launch-uml.c       |   6 +++
 6 files changed, 321 insertions(+), 44 deletions(-)

diff --git a/generator/actions.ml b/generator/actions.ml
index a6ef194..b25194c 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -1286,7 +1286,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"; OString "protocol"; OStringList "server"; OString
"username"; OString "secret"; OString "cachemode"];
+    style = RErr, [String "filename"], [OBool "readonly"; OString
"format"; OString "iface"; OString "name"; OString
"label"; OString "protocol"; OStringList "server"; OString
"username"; OString "secret"; OString "cachemode"; OString
"discard"];
     once_had_no_optargs = true;
     blocking = false;
     fish_alias = ["add"];
@@ -1504,6 +1504,36 @@ for scratch or temporary disks.
 
 =back
 
+=item C<discard>
+
+Enable or disable discard (a.k.a. trim or unmap) support on this
+drive.  If enabled, operations such as C<guestfs_fstrim> will be able
+to discard / make thin / punch holes in the underlying host file
+or device.
+
+Possible discard settings are:
+
+=over 4
+
+=item C<discard = \"disable\">
+
+Disable discard support.  This is the default.
+
+=item C<discard = \"enable\">
+
+Enable discard support.  Fail if discard is not possible.
+
+=item C<discard = \"besteffort\">
+
+Enable discard support if possible, but don't fail if it is not
+supported.
+
+Since not all backends and not all underlying systems support
+discard, this is a good choice if you want to use discard if
+possible, but don't mind if it doesn't work.
+
+=back
+
 =back" };
 
   { defaults with
diff --git a/src/drives.c b/src/drives.c
index 2c85b52..68e37f7 100644
--- a/src/drives.c
+++ b/src/drives.c
@@ -115,7 +115,8 @@ static struct drive *
 create_drive_file (guestfs_h *g, const char *path,
                    bool readonly, const char *format,
                    const char *iface, const char *name,
-                   const char *disk_label, const char *cachemode)
+                   const char *disk_label, const char *cachemode,
+                   enum discard discard)
 {
   struct drive *drv = safe_calloc (g, 1, sizeof *drv);
 
@@ -128,6 +129,7 @@ create_drive_file (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->cachemode = cachemode ? safe_strdup (g, cachemode) : NULL;
+  drv->discard = discard;
 
   if (readonly) {
     if (create_overlay (g, drv) == -1) {
@@ -151,7 +153,8 @@ create_drive_non_file (guestfs_h *g,
                        const char *username, const char *secret,
                        bool readonly, const char *format,
                        const char *iface, const char *name,
-                       const char *disk_label, const char *cachemode)
+                       const char *disk_label, const char *cachemode,
+                       enum discard discard)
 {
   struct drive *drv = safe_calloc (g, 1, sizeof *drv);
 
@@ -168,6 +171,7 @@ create_drive_non_file (guestfs_h *g,
   drv->name = name ? safe_strdup (g, name) : NULL;
   drv->disk_label = disk_label ? safe_strdup (g, disk_label) : NULL;
   drv->cachemode = cachemode ? safe_strdup (g, cachemode) : NULL;
+  drv->discard = discard;
 
   if (readonly) {
     if (create_overlay (g, drv) == -1) {
@@ -191,7 +195,8 @@ create_drive_curl (guestfs_h *g,
                    const char *username, const char *secret,
                    bool readonly, const char *format,
                    const char *iface, const char *name,
-                   const char *disk_label, const char *cachemode)
+                   const char *disk_label, const char *cachemode,
+                   enum discard discard)
 {
   if (secret != NULL) {
     error (g, _("curl: you cannot specify a secret with this protocol"));
@@ -223,7 +228,7 @@ create_drive_curl (guestfs_h *g,
                                 servers, nr_servers, exportname,
                                 username, secret,
                                 readonly, format, iface, name, disk_label,
-                                cachemode);
+                                cachemode, discard);
 }
 
 static struct drive *
@@ -233,7 +238,8 @@ create_drive_gluster (guestfs_h *g,
                       const char *username, const char *secret,
                       bool readonly, const char *format,
                       const char *iface, const char *name,
-                      const char *disk_label, const char *cachemode)
+                      const char *disk_label, const char *cachemode,
+                      enum discard discard)
 {
   if (username != NULL) {
     error (g, _("gluster: you cannot specify a username with this protocol"));
@@ -263,7 +269,7 @@ create_drive_gluster (guestfs_h *g,
                                 servers, nr_servers, exportname,
                                 username, secret,
                                 readonly, format, iface, name, disk_label,
-                                cachemode);
+                                cachemode, discard);
 }
 
 static int
@@ -285,7 +291,8 @@ create_drive_nbd (guestfs_h *g,
                   const char *username, const char *secret,
                   bool readonly, const char *format,
                   const char *iface, const char *name,
-                  const char *disk_label, const char *cachemode)
+                  const char *disk_label, const char *cachemode,
+                  enum discard discard)
 {
   if (username != NULL) {
     error (g, _("nbd: you cannot specify a username with this protocol"));
@@ -308,7 +315,7 @@ create_drive_nbd (guestfs_h *g,
                                 servers, nr_servers, exportname,
                                 username, secret,
                                 readonly, format, iface, name, disk_label,
-                                cachemode);
+                                cachemode, discard);
 }
 
 static struct drive *
@@ -318,7 +325,8 @@ create_drive_rbd (guestfs_h *g,
                   const char *username, const char *secret,
                   bool readonly, const char *format,
                   const char *iface, const char *name,
-                  const char *disk_label, const char *cachemode)
+                  const char *disk_label, const char *cachemode,
+                  enum discard discard)
 {
   size_t i;
 
@@ -348,7 +356,7 @@ create_drive_rbd (guestfs_h *g,
                                 servers, nr_servers, exportname,
                                 username, secret,
                                 readonly, format, iface, name, disk_label,
-                                cachemode);
+                                cachemode, discard);
 }
 
 static struct drive *
@@ -358,7 +366,8 @@ create_drive_sheepdog (guestfs_h *g,
                        const char *username, const char *secret,
                        bool readonly, const char *format,
                        const char *iface, const char *name,
-                       const char *disk_label, const char *cachemode)
+                       const char *disk_label, const char *cachemode,
+                       enum discard discard)
 {
   size_t i;
 
@@ -397,7 +406,7 @@ create_drive_sheepdog (guestfs_h *g,
                                 servers, nr_servers, exportname,
                                 username, secret,
                                 readonly, format, iface, name, disk_label,
-                                cachemode);
+                                cachemode, discard);
 }
 
 static struct drive *
@@ -407,7 +416,8 @@ create_drive_ssh (guestfs_h *g,
                   const char *username, const char *secret,
                   bool readonly, const char *format,
                   const char *iface, const char *name,
-                  const char *disk_label, const char *cachemode)
+                  const char *disk_label, const char *cachemode,
+                  enum discard discard)
 {
   if (secret != NULL) {
     error (g, _("ssh: you cannot specify a secret with this protocol"));
@@ -444,7 +454,7 @@ create_drive_ssh (guestfs_h *g,
                                 servers, nr_servers, exportname,
                                 username, secret,
                                 readonly, format, iface, name, disk_label,
-                                cachemode);
+                                cachemode, discard);
 }
 
 static struct drive *
@@ -454,7 +464,8 @@ create_drive_iscsi (guestfs_h *g,
                     const char *username, const char *secret,
                     bool readonly, const char *format,
                     const char *iface, const char *name,
-                    const char *disk_label, const char *cachemode)
+                    const char *disk_label, const char *cachemode,
+                    enum discard discard)
 {
   if (username != NULL) {
     error (g, _("iscsi: you cannot specify a username with this protocol"));
@@ -491,7 +502,7 @@ create_drive_iscsi (guestfs_h *g,
                                 servers, nr_servers, exportname,
                                 username, secret,
                                 readonly, format, iface, name, disk_label,
-                                cachemode);
+                                cachemode, discard);
 }
 
 /* Traditionally you have been able to use /dev/null as a filename, as
@@ -528,14 +539,15 @@ create_drive_dev_null (guestfs_h *g, bool readonly, const char
*format,
     return NULL;
 
   return create_drive_file (g, tmpfile, readonly, format, iface, name,
-                            disk_label, 0);
+                            disk_label, NULL, discard_disable);
 }
 
 static struct drive *
 create_drive_dummy (guestfs_h *g)
 {
   /* A special drive struct that is used as a dummy slot for the appliance. */
-  return create_drive_file (g, "", 0, NULL, NULL, NULL, NULL, 0);
+  return create_drive_file (g, "", 0, NULL, NULL, NULL, NULL, NULL,
+                            discard_disable);
 }
 
 static void
@@ -563,8 +575,8 @@ free_drive_struct (struct drive *drv)
   free (drv);
 }
 
-static const char *
-protocol_to_string (enum drive_protocol protocol)
+const char *
+guestfs___drive_protocol_to_string (enum drive_protocol protocol)
 {
   switch (protocol) {
   case drive_protocol_file: return "file";
@@ -590,12 +602,12 @@ static char *
 drive_to_string (guestfs_h *g, const struct drive *drv)
 {
   return safe_asprintf
-    (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s",
+    (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s%s",
      drv->src.u.path,
      drv->readonly ? " readonly" : "",
      drv->src.format ? " format=" : "",
      drv->src.format ? : "",
-     protocol_to_string (drv->src.protocol),
+     guestfs___drive_protocol_to_string (drv->src.protocol),
      drv->iface ? " iface=" : "",
      drv->iface ? : "",
      drv->name ? " name=" : "",
@@ -603,7 +615,9 @@ drive_to_string (guestfs_h *g, const struct drive *drv)
      drv->disk_label ? " label=" : "",
      drv->disk_label ? : "",
      drv->cachemode ? " cache=" : "",
-     drv->cachemode ? : "");
+     drv->cachemode ? : "",
+     drv->discard == discard_disable ? "" :
+     drv->discard == discard_enable ? " discard=enable" : "
discard=besteffort");
 }
 
 /* Add struct drive to the g->drives vector at the given index. */
@@ -824,6 +838,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   const char *username;
   const char *secret;
   const char *cachemode;
+  enum discard discard;
   struct drive *drv;
   size_t i, drv_index;
 
@@ -852,6 +867,28 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   cachemode = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_CACHEMODE_BITMASK
     ? optargs->cachemode : NULL;
 
+  if (optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK) {
+    if (STREQ (optargs->discard, "disable"))
+      discard = discard_disable;
+    else if (STREQ (optargs->discard, "enable"))
+      discard = discard_enable;
+    else if (STREQ (optargs->discard, "besteffort"))
+      discard = discard_besteffort;
+    else {
+      error (g, _("discard parameter must be 'disable', 'enable' or
'besteffort'"));
+      free_drive_servers (servers, nr_servers);
+      return -1;
+    }
+  }
+  else
+    discard = discard_disable;
+
+  if (readonly && discard == discard_enable) {
+    error (g, _("discard support cannot be enabled on read-only drives"));
+    free_drive_servers (servers, nr_servers);
+    return -1;
+  }
+
   if (format && !valid_format_iface (format)) {
     error (g, _("%s parameter is empty or contains disallowed characters"),
            "format");
@@ -904,7 +941,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
       }
 
       drv = create_drive_file (g, filename, readonly, format, iface, name,
-                               disk_label, cachemode);
+                               disk_label, cachemode, discard);
     }
   }
   else if (STREQ (protocol, "ftp")) {
@@ -912,71 +949,71 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
                              servers, nr_servers, filename,
                              username, secret,
                              readonly, format, iface, name,
-                             disk_label, cachemode);
+                             disk_label, cachemode, discard);
   }
   else if (STREQ (protocol, "ftps")) {
     drv = create_drive_curl (g, drive_protocol_ftps,
                              servers, nr_servers, filename,
                              username, secret,
                              readonly, format, iface, name,
-                             disk_label, cachemode);
+                             disk_label, cachemode, discard);
   }
   else if (STREQ (protocol, "gluster")) {
     drv = create_drive_gluster (g, servers, nr_servers, filename,
                                 username, secret,
                                 readonly, format, iface, name,
-                                disk_label, cachemode);
+                                disk_label, cachemode, discard);
   }
   else if (STREQ (protocol, "http")) {
     drv = create_drive_curl (g, drive_protocol_http,
                              servers, nr_servers, filename,
                              username, secret,
                              readonly, format, iface, name,
-                             disk_label, cachemode);
+                             disk_label, cachemode, discard);
   }
   else if (STREQ (protocol, "https")) {
     drv = create_drive_curl (g, drive_protocol_https,
                              servers, nr_servers, filename,
                              username, secret,
                              readonly, format, iface, name,
-                             disk_label, cachemode);
+                             disk_label, cachemode, discard);
   }
   else if (STREQ (protocol, "iscsi")) {
     drv = create_drive_iscsi (g, servers, nr_servers, filename,
                               username, secret,
                               readonly, format, iface, name,
-                              disk_label, cachemode);
+                              disk_label, cachemode, discard);
   }
   else if (STREQ (protocol, "nbd")) {
     drv = create_drive_nbd (g, servers, nr_servers, filename,
                             username, secret,
                             readonly, format, iface, name,
-                            disk_label, cachemode);
+                            disk_label, cachemode, discard);
   }
   else if (STREQ (protocol, "rbd")) {
     drv = create_drive_rbd (g, servers, nr_servers, filename,
                             username, secret,
                             readonly, format, iface, name,
-                            disk_label, cachemode);
+                            disk_label, cachemode, discard);
   }
   else if (STREQ (protocol, "sheepdog")) {
     drv = create_drive_sheepdog (g, servers, nr_servers, filename,
                                  username, secret,
                                  readonly, format, iface, name,
-                                 disk_label, cachemode);
+                                 disk_label, cachemode, discard);
   }
   else if (STREQ (protocol, "ssh")) {
     drv = create_drive_ssh (g, servers, nr_servers, filename,
                             username, secret,
                             readonly, format, iface, name,
-                            disk_label, cachemode);
+                            disk_label, cachemode, discard);
   }
   else if (STREQ (protocol, "tftp")) {
     drv = create_drive_curl (g, drive_protocol_tftp,
                              servers, nr_servers, filename,
                              username, secret,
                              readonly, format, iface, name,
-                             disk_label, cachemode);
+                             disk_label, cachemode, discard);
   }
   else {
     error (g, _("unknown protocol '%s'"), protocol);
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 545146f..0f3a928 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -231,6 +231,12 @@ struct drive_source {
   char *secret;
 };
 
+enum discard {
+  discard_disable = 0,
+  discard_enable = 1,
+  discard_besteffort = 2,
+};
+
 /* There is one 'struct drive' per drive, including hot-plugged drives. */
 struct drive {
   /* Original source of the drive, eg. file:..., http:... */
@@ -252,6 +258,7 @@ struct drive {
   char *name;
   char *disk_label;
   char *cachemode;
+  enum discard discard;
 };
 
 /* Extra hv parameters (from guestfs_config). */
@@ -713,6 +720,7 @@ extern size_t guestfs___checkpoint_drives (guestfs_h *g);
 extern void guestfs___rollback_drives (guestfs_h *g, size_t);
 extern void guestfs___add_dummy_appliance_drive (guestfs_h *g);
 extern void guestfs___free_drives (guestfs_h *g);
+extern const char *guestfs___drive_protocol_to_string (enum drive_protocol protocol);
 
 /* appliance.c */
 extern int guestfs___build_appliance (guestfs_h *g, char **kernel, char **dtb, char
**initrd, char **appliance);
@@ -832,6 +840,7 @@ extern void guestfs___cleanup_cmd_close (struct command **);
 
 /* launch-direct.c */
 extern char *guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source
*src);
+extern bool guestfs___discard_possible (guestfs_h *g, struct drive *drv, unsigned long
qemu_version);
 
 /* utils.c */
 extern int guestfs___validate_guid (const char *);
diff --git a/src/launch-direct.c b/src/launch-direct.c
index 1e84061..3d478c7 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -509,6 +509,30 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
     CLEANUP_FREE char *file = NULL, *escaped_file = NULL, *param = NULL;
 
     if (!drv->overlay) {
+      const char *discard_mode = "";
+      int major = data->qemu_version_major, minor = data->qemu_version_minor;
+      unsigned long qemu_version = major * 1000000 + minor * 1000;
+
+      switch (drv->discard) {
+      case discard_disable:
+        /* Since the default is always discard=ignore, don't specify it
+         * on the command line.  This also avoids unnecessary breakage
+         * with qemu < 1.5 which didn't have the option at all.
+         */
+        break;
+      case discard_enable:
+        if (!guestfs___discard_possible (g, drv, qemu_version))
+          goto cleanup0;
+        /*FALLTHROUGH*/
+      case discard_besteffort:
+        /* I believe from reading the code that this is always safe as
+         * long as qemu >= 1.5.
+         */
+        if (major > 1 || (major == 1 && minor >= 5))
+          discard_mode = ",discard=unmap";
+        break;
+      }
+
       /* Make the file= parameter. */
       file = guestfs___drive_source_qemu_param (g, &drv->src);
       escaped_file = qemu_escape_param (g, file);
@@ -517,10 +541,11 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
        * the if=... at the end.
        */
       param = safe_asprintf
-        (g, "file=%s%s,cache=%s%s%s%s%s,id=hd%zu",
+        (g, "file=%s%s,cache=%s%s%s%s%s%s,id=hd%zu",
          escaped_file,
          drv->readonly ? ",snapshot=on" : "",
          drv->cachemode ? drv->cachemode : "writeback",
+         discard_mode,
          drv->src.format ? ",format=" : "",
          drv->src.format ? drv->src.format : "",
          drv->disk_label ? ",serial=" : "",
@@ -1370,6 +1395,96 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct
drive_source *src)
   abort ();
 }
 
+/* Test if discard is both supported by qemu AND possible with the
+ * underlying file or device.  This returns 1 if discard is possible.
+ * It returns 0 if not possible and sets the error to the reason why.
+ *
+ * This function is called when the user set discard == "enable".
+ *
+ * qemu_version is the version of qemu in the form returned by libvirt:
+ * major * 1,000,000 + minor * 1,000 + release
+ */
+bool
+guestfs___discard_possible (guestfs_h *g, struct drive *drv,
+                            unsigned long qemu_version)
+{
+  /* qemu >= 1.5.  This was the first version that supported the
+   * discard option on -drive at all.
+   */
+  bool qemu15 = qemu_version >= 1005000;
+  /* qemu >= 1.6.  This was the first version that supported unmap on
+   * qcow2 backing files.
+   */
+  bool qemu16 = qemu_version >= 1006000;
+
+  if (!qemu15) {
+    error (g, _("discard cannot be enabled on this drive: "
+                "qemu < 1.5"));
+    return false;
+  }
+
+  /* If it's an overlay, discard is not possible (on the underlying
+   * file).  This has probably been caught earlier since we already
+   * checked that the drive is !readonly.  Nevertheless ...
+   */
+  if (drv->overlay) {
+    error (g, _("discard cannot be enabled on this drive: "
+                "the drive has a read-only overlay"));
+    return false;
+  }
+
+  /* Look at the source format. */
+  if (drv->src.format == NULL) {
+    /* We could autodetect the format, but we don't ... yet. XXX */
+    error (g, _("discard cannot be enabled on this drive: "
+                "you have to specify the format of the file"));
+    return false;
+  }
+  else if (STREQ (drv->src.format, "raw"))
+    /* OK */ ;
+  else if (STREQ (drv->src.format, "qcow2")) {
+    if (!qemu16) {
+      error (g, _("discard cannot be enabled on this drive: "
+                  "qemu < 1.6 cannot do discard on qcow2 files"));
+    return false;
+    }
+  }
+  else {
+    /* It's possible in future other formats will support discard, but
+     * currently (qemu 1.7) none of them do.
+     */
+    error (g, _("discard cannot be enabled on this drive: "
+                "qemu does not support discard for '%s' format
files"),
+           drv->src.format);
+    return false;
+  }
+
+  switch (drv->src.protocol) {
+    /* Protocols which support discard. */
+  case drive_protocol_file:
+  case drive_protocol_gluster:
+  case drive_protocol_iscsi:
+  case drive_protocol_nbd:
+  case drive_protocol_rbd:
+  case drive_protocol_sheepdog: /* XXX depends on server version */
+    break;
+
+    /* Protocols which don't support discard. */
+  case drive_protocol_ftp:
+  case drive_protocol_ftps:
+  case drive_protocol_http:
+  case drive_protocol_https:
+  case drive_protocol_ssh:
+  case drive_protocol_tftp:
+    error (g, _("discard cannot be enabled on this drive: "
+                "protocol '%s' does not support discard"),
+           guestfs___drive_protocol_to_string (drv->src.protocol));
+    return false;
+  }
+
+  return true;
+}
+
 static int
 shutdown_direct (guestfs_h *g, void *datav, int check_for_errors)
 {
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index da1d665..1f5a79a 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -107,6 +107,7 @@ struct backend_libvirt_data {
   bool selinux_norelabel_disks;
   char name[DOMAIN_NAME_LEN];   /* random name */
   bool is_kvm;                  /* false = qemu, true = kvm (from capabilities)*/
+  unsigned long qemu_version;   /* qemu version (from libvirt) */
 };
 
 /* Parameters passed to construct_libvirt_xml and subfunctions.  We
@@ -131,6 +132,7 @@ static xmlChar *construct_libvirt_xml (guestfs_h *g, const struct
libvirt_xml_pa
 static void debug_appliance_permissions (guestfs_h *g);
 static void debug_socket_permissions (guestfs_h *g);
 static void libvirt_error (guestfs_h *g, const char *fs, ...) __attribute__((format
(printf,2,3)));
+static void libvirt_debug (guestfs_h *g, const char *fs, ...) __attribute__((format
(printf,2,3)));
 static int is_custom_hv (guestfs_h *g);
 static int is_blk (const char *path);
 static void ignore_errors (void *ignore, virErrorPtr ignore2);
@@ -283,6 +285,19 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
    */
   virConnSetErrorFunc (conn, NULL, ignore_errors);
 
+  /* Get hypervisor (hopefully qemu) version. */
+  if (virConnectGetVersion (conn, &data->qemu_version) == 0) {
+    debug (g, "qemu version (reported by libvirt) = %lu (%lu.%lu.%lu)",
+           data->qemu_version,
+           data->qemu_version / 1000000UL,
+           data->qemu_version / 1000UL % 1000UL,
+           data->qemu_version % 1000UL);
+  }
+  else {
+    libvirt_debug (g, "unable to read qemu version from libvirt");
+    data->qemu_version = 0;
+  }
+
   if (g->verbose)
     guestfs___print_timestamped_message (g, "get libvirt capabilities");
 
@@ -789,7 +804,7 @@ static int construct_libvirt_xml_devices (guestfs_h *g, const struct
libvirt_xml
 static int construct_libvirt_xml_qemu_cmdline (guestfs_h *g, const struct
libvirt_xml_params *params, xmlTextWriterPtr xo);
 static int construct_libvirt_xml_disk (guestfs_h *g, const struct backend_libvirt_data
*data, xmlTextWriterPtr xo, struct drive *drv, size_t drv_index);
 static int construct_libvirt_xml_disk_target (guestfs_h *g, xmlTextWriterPtr xo, size_t
drv_index);
-static int construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, xmlTextWriterPtr xo,
const char *format, const char *cachemode);
+static int construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, const struct
backend_libvirt_data *data, struct drive *drv, xmlTextWriterPtr xo, const char *format,
const char *cachemode, enum discard discard);
 static int construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr xo, size_t
drv_index);
 static int construct_libvirt_xml_disk_source_hosts (guestfs_h *g, xmlTextWriterPtr xo,
const struct drive_source *src);
 static int construct_libvirt_xml_disk_source_seclabel (guestfs_h *g, const struct
backend_libvirt_data *data, xmlTextWriterPtr xo);
@@ -1233,7 +1248,9 @@ construct_libvirt_xml_disk (guestfs_h *g,
       if (construct_libvirt_xml_disk_target (g, xo, drv_index) == -1)
         return -1;
 
-      if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2",
"unsafe")
+      if (construct_libvirt_xml_disk_driver_qemu (g, NULL, NULL,
+                                                  xo, "qcow2",
"unsafe",
+                                                  discard_disable)
           == -1)
         return -1;
     }
@@ -1370,8 +1387,9 @@ construct_libvirt_xml_disk (guestfs_h *g,
         return -1;
       }
 
-      if (construct_libvirt_xml_disk_driver_qemu (g, xo, format,
-                                                  drv->cachemode ? :
"writeback")
+      if (construct_libvirt_xml_disk_driver_qemu (g, data, drv, xo, format,
+                                                  drv->cachemode ? :
"writeback",
+                                                  drv->discard)
           == -1)
         return -1;
     }
@@ -1407,14 +1425,41 @@ construct_libvirt_xml_disk_target (guestfs_h *g, xmlTextWriterPtr
xo,
 }
 
 static int
-construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, xmlTextWriterPtr xo,
+construct_libvirt_xml_disk_driver_qemu (guestfs_h *g,
+                                        const struct backend_libvirt_data *data,
+                                        struct drive *drv,
+                                        xmlTextWriterPtr xo,
                                         const char *format,
-                                        const char *cachemode)
+                                        const char *cachemode,
+                                        enum discard discard)
 {
+  bool discard_unmap = false;
+
+  switch (discard) {
+  case discard_disable:
+    /* Since the default is always discard=ignore, don't specify it
+     * in the XML.
+     */
+    break;
+  case discard_enable:
+    if (!guestfs___discard_possible (g, drv, data->qemu_version))
+      return -1;
+    /*FALLTHROUGH*/
+  case discard_besteffort:
+    /* I believe from reading the code that this is always safe as
+     * long as qemu >= 1.5.
+     */
+    if (data->qemu_version >= 1005000)
+      discard_unmap = true;
+    break;
+  }
+
   start_element ("driver") {
     attribute ("name", "qemu");
     attribute ("type", format);
     attribute ("cache", cachemode);
+    if (discard_unmap)
+      attribute ("discard", "unmap");
   } end_element ();
 
   return 0;
@@ -1499,7 +1544,9 @@ construct_libvirt_xml_appliance (guestfs_h *g,
       attribute ("bus", "scsi");
     } end_element ();
 
-    if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2",
"unsafe") == -1)
+    if (construct_libvirt_xml_disk_driver_qemu (g, NULL, NULL, xo,
+                                                "qcow2", "unsafe",
+                                                discard_disable) == -1)
       return -1;
 
     if (construct_libvirt_xml_disk_address (g, xo, params->appliance_index)
@@ -1659,6 +1706,39 @@ libvirt_error (guestfs_h *g, const char *fs, ...)
   free (msg);
 }
 
+/* Same as 'libvirt_error' but calls debug instead. */
+static void
+libvirt_debug (guestfs_h *g, const char *fs, ...)
+{
+  va_list args;
+  char *msg;
+  int len;
+  virErrorPtr err;
+
+  if (!g->verbose)
+    return;
+
+  va_start (args, fs);
+  len = vasprintf (&msg, fs, args);
+  va_end (args);
+
+  if (len < 0)
+    msg = safe_asprintf (g,
+                         _("%s: internal error forming error message"),
+                         __func__);
+
+  /* In all recent libvirt, this retrieves the thread-local error. */
+  err = virGetLastError ();
+  if (err)
+    debug (g, "%s: %s [code=%d domain=%d]",
+           msg, err->message, err->code, err->domain);
+  else
+    debug (g, "%s", msg);
+
+  /* NB. 'err' must not be freed! */
+  free (msg);
+}
+
 #if HAVE_LIBSELINUX
 static void
 selinux_warning (guestfs_h *g, const char *func,
diff --git a/src/launch-uml.c b/src/launch-uml.c
index deda366..2a6ddaf 100644
--- a/src/launch-uml.c
+++ b/src/launch-uml.c
@@ -122,6 +122,12 @@ uml_supported (guestfs_h *g)
              _("uml backend does not support drives with 'label'
parameter"));
       return false;
     }
+    /* Note that discard == "besteffort" is fine. */
+    if (drv->discard == discard_enable) {
+      error (g,
+             _("uml backend does not support drives with 'discard' parameter
set to 'enable'"));
+      return false;
+    }
   }
 
   return true;
-- 
1.8.5.3


    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

[Libguestfs] [PATCH v2 03/18] New API parameter: Add discard parameter to guestfs_add_drive_opts.