[Libguestfs] [PATCH 2/3] New API parameter: Add discard parameter to guestfs_add_drive_opts.
Richard W.M. Jones
rjones at redhat.com
Mon Mar 10 17:28:53 UTC 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 | 110 ++++++++++++++++++++++++++++++++++++++++++++++++-
src/launch-libvirt.c | 96 ++++++++++++++++++++++++++++++++++++++----
src/launch-uml.c | 6 +++
6 files changed, 316 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..df481a7 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, bool qemu15, bool qemu16);
/* utils.c */
extern int guestfs___validate_guid (const char *);
diff --git a/src/launch-direct.c b/src/launch-direct.c
index 1e84061..584c4de 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -509,6 +509,29 @@ 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;
+
+ 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, major, minor))
+ 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 +540,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 +1394,90 @@ 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".
+ *
+ * qemu15: qemu >= 1.5. This was the first version that supported
+ * the discard option on -drive at all.
+ *
+ * qemu16: qemu >= 1.6. This was the first version that supported
+ * unmap on qcow2 backing files.
+ */
+bool
+guestfs___discard_possible (guestfs_h *g, struct drive *drv,
+ bool qemu15, bool qemu16)
+{
+ 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 d86e2d2..1c46b57 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 backend_libvir
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);
@@ -1238,7 +1253,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;
}
@@ -1375,8 +1392,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;
}
@@ -1412,14 +1430,43 @@ 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 >= 1005000,
+ data->qemu_version >= 1006000))
+ 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;
@@ -1504,7 +1551,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)
@@ -1664,6 +1713,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
More information about the Libguestfs
mailing list