Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 00/10] Add discard support.
This set of patches: - Adds new APIs to support discard in libguestfs. - Adds discard support to virt-format. - Adds discard support to virt-sysprep. - Implements virt-sparsify --in-place. This is now working, after fixing the rather stupid bug in fstrim. I've pushed the ones which were ACKed previously + the fstrim fix. Rich.
Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 01/10] New API parameter: Add discard parameter to guestfs_add_drive_opts.
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 | 38 +++++++++++++---
src/guestfs-internal.h | 9 ++++
src/launch-direct.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++-
src/launch-libvirt.c | 99 ++++++++++++++++++++++++++++++++++++++---
src/launch-uml.c | 6 +++
6 files changed, 287 insertions(+), 14 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 2c776a2..57c2471 100644
--- a/src/drives.c
+++ b/src/drives.c
@@ -61,6 +61,7 @@ struct drive_create_data {
const char *name;
const char *disk_label;
const char *cachemode;
+ enum discard discard;
};
/* Compile all the regular expressions once when the shared library is
@@ -144,6 +145,7 @@ create_drive_file (guestfs_h *g,
drv->name = data->name ? safe_strdup (g, data->name) : NULL;
drv->disk_label = data->disk_label ? safe_strdup (g,
data->disk_label) : NULL;
drv->cachemode = data->cachemode ? safe_strdup (g, data->cachemode)
: NULL;
+ drv->discard = data->discard;
if (data->readonly) {
if (create_overlay (g, drv) == -1) {
@@ -178,6 +180,7 @@ create_drive_non_file (guestfs_h *g,
drv->name = data->name ? safe_strdup (g, data->name) : NULL;
drv->disk_label = data->disk_label ? safe_strdup (g,
data->disk_label) : NULL;
drv->cachemode = data->cachemode ? safe_strdup (g, data->cachemode)
: NULL;
+ drv->discard = data->discard;
if (data->readonly) {
if (create_overlay (g, drv) == -1) {
@@ -473,6 +476,7 @@ create_drive_dev_null (guestfs_h *g,
return NULL;
data->exportname = tmpfile;
+ data->discard = discard_disable;
return create_drive_file (g, data);
}
@@ -511,8 +515,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";
@@ -538,12 +542,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=" : "",
@@ -551,7 +555,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. */
@@ -795,6 +801,28 @@ guestfs__add_drive_opts (guestfs_h *g, const char
*filename,
data.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"))
+ data.discard = discard_disable;
+ else if (STREQ (optargs->discard, "enable"))
+ data.discard = discard_enable;
+ else if (STREQ (optargs->discard, "besteffort"))
+ data.discard = discard_besteffort;
+ else {
+ error (g, _("discard parameter must be 'disable',
'enable' or 'besteffort'"));
+ free_drive_servers (data.servers, data.nr_servers);
+ return -1;
+ }
+ }
+ else
+ data.discard = discard_disable;
+
+ if (data.readonly && data.discard == discard_enable) {
+ error (g, _("discard support cannot be enabled on read-only
drives"));
+ free_drive_servers (data.servers, data.nr_servers);
+ return -1;
+ }
+
if (data.format && !valid_format_iface (data.format)) {
error (g, _("%s parameter is empty or contains disallowed
characters"),
"format");
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 545146f..95b09cf 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,
+ discard_besteffort,
+};
+
/* 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..e0049eb 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, data, drv,
+ 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,46 @@ 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;
+
+ /* When adding the appliance disk, we don't have a 'drv' struct.
+ * However the caller will use discard_disable, so we don't need it.
+ */
+ assert (discard == discard_disable || drv != NULL);
+
+ 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 +1549,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, params->data, NULL, xo,
+ "qcow2",
"unsafe",
+ discard_disable) == -1)
return -1;
if (construct_libvirt_xml_disk_address (g, xo, params->appliance_index)
@@ -1659,6 +1711,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
Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 02/10] New API: blkdiscard - discard all blocks on a block device.
---
daemon/Makefile.am | 1 +
daemon/blkdiscard.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++
generator/actions.ml | 15 ++++++++++
po/POTFILES | 1 +
src/MAX_PROC_NR | 2 +-
5 files changed, 95 insertions(+), 1 deletion(-)
create mode 100644 daemon/blkdiscard.c
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index e64caca..b18f9ff 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -83,6 +83,7 @@ guestfsd_SOURCES = \
available.c \
augeas.c \
base64.c \
+ blkdiscard.c \
blkid.c \
blockdev.c \
btrfs.c \
diff --git a/daemon/blkdiscard.c b/daemon/blkdiscard.c
new file mode 100644
index 0000000..26d5f7d
--- /dev/null
+++ b/daemon/blkdiscard.c
@@ -0,0 +1,77 @@
+/* libguestfs - the guestfsd daemon
+ * Copyright (C) 2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include <linux/fs.h>
+
+#include "daemon.h"
+#include "actions.h"
+
+/*
http://rwmj.wordpress.com/2014/03/11/blkdiscard-blkzeroout-blkdiscardzeroes-blksecdiscard/
*/
+
+int
+do_blkdiscard (const char *device)
+{
+ /* XXX We could read /sys/block/<device>/queue/discard_* in order to
+ * determine if discard is supported and the largest request size we
+ * are allowed to make. However:
+ *
+ * (1) Mapping the device name to /sys/block/<device> is quite hard
+ * (cf. the lv_canonical function in daemon/lvm.c)
+ *
+ * (2) We don't really need to do this in modern libguestfs since
+ * we're very likely to be using virtio-scsi, which supports
+ * arbitrary block discards.
+ *
+ * Let's wait to see if it causes a problem in real world
+ * situations.
+ */
+ uint64_t range[2];
+ int64_t size;
+ int fd;
+
+ size = do_blockdev_getsize64 (device);
+ if (size == -1)
+ return -1;
+
+ range[0] = 0;
+ range[1] = (uint64_t) size;
+
+ fd = open (device, O_WRONLY|O_CLOEXEC);
+ if (fd == -1) {
+ reply_with_perror ("open: %s", device);
+ return -1;
+ }
+
+ if (ioctl (fd, BLKDISCARD, range) == -1) {
+ reply_with_perror ("ioctl: %s: BLKDISCARD", device);
+ close (fd);
+ return -1;
+ }
+
+ close (fd);
+ return 0;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index b25194c..beb1182 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -11818,6 +11818,21 @@ device C<device>. Note that partitions are
numbered from 1.
The partition name can only be read on certain types of partition
table. This works on C<gpt> but not on C<mbr> partitions." };
+ { defaults with
+ name = "blkdiscard";
+ style = RErr, [Device "device"], [];
+ proc_nr = Some 417;
+ shortdesc = "discard all blocks on a device";
+ longdesc = "\
+This discards all blocks on the block device C<device>, giving
+the free space back to the host.
+
+This operation requires support in libguestfs, the host filesystem,
+qemu and the host kernel. If this support isn't present it may give
+an error or even appear to run but do nothing. You must also
+set the C<discard> attribute on the block device (see
+C<guestfs_add_drive_opts>)." };
+
]
(* Non-API meta-commands available only in guestfish.
diff --git a/po/POTFILES b/po/POTFILES
index 5aeeb2e..ecdbae4 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -16,6 +16,7 @@ daemon/acl.c
daemon/augeas.c
daemon/available.c
daemon/base64.c
+daemon/blkdiscard.c
daemon/blkid.c
daemon/blockdev.c
daemon/btrfs.c
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 1c105f1..53c86ff 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-416
+417
--
1.8.5.3
Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 03/10] New API: blkdiscardzeroes - do discarded blocks read back as zeroes?
---
daemon/blkdiscard.c | 23 +++++++++++++++++++++++
generator/actions.ml | 13 +++++++++++++
src/MAX_PROC_NR | 2 +-
3 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/daemon/blkdiscard.c b/daemon/blkdiscard.c
index 26d5f7d..0651788 100644
--- a/daemon/blkdiscard.c
+++ b/daemon/blkdiscard.c
@@ -75,3 +75,26 @@ do_blkdiscard (const char *device)
close (fd);
return 0;
}
+
+int
+do_blkdiscardzeroes (const char *device)
+{
+ int fd;
+ unsigned int arg;
+
+ fd = open (device, O_RDONLY|O_CLOEXEC);
+ if (fd == -1) {
+ reply_with_perror ("open: %s", device);
+ return -1;
+ }
+
+ if (ioctl (fd, BLKDISCARDZEROES, &arg) == -1) {
+ reply_with_perror ("ioctl: %s: BLKDISCARDZEROES", device);
+ close (fd);
+ return -1;
+ }
+
+ close (fd);
+
+ return arg != 0;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index beb1182..4e2c9e0 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -11833,6 +11833,19 @@ an error or even appear to run but do nothing. You
must also
set the C<discard> attribute on the block device (see
C<guestfs_add_drive_opts>)." };
+ { defaults with
+ name = "blkdiscardzeroes";
+ style = RBool "zeroes", [Device "device"], [];
+ proc_nr = Some 418;
+ shortdesc = "return true if discarded blocks are read as zeroes";
+ longdesc = "\
+This call returns true if blocks on C<device> that have been
+discarded by a call to C<guestfs_blkdiscard> are returned as
+blocks of zero bytes when read the next time.
+
+If it returns false, then it may be that discarded blocks are
+read as stale or random data." };
+
]
(* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 53c86ff..29aae8e 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-417
+418
--
1.8.5.3
Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 04/10] tests: Add tests of discard support.
Test that blkdiscard, -o discard, and fstrim work in reality, end-to-end.
---
Makefile.am | 1 +
configure.ac | 1 +
tests/discard/Makefile.am | 28 +++++++++
tests/discard/test-blkdiscard.pl | 111 ++++++++++++++++++++++++++++++++++
tests/discard/test-discard.pl | 117 ++++++++++++++++++++++++++++++++++++
tests/discard/test-fstrim.pl | 125 +++++++++++++++++++++++++++++++++++++++
6 files changed, 383 insertions(+)
create mode 100644 tests/discard/Makefile.am
create mode 100755 tests/discard/test-blkdiscard.pl
create mode 100755 tests/discard/test-discard.pl
create mode 100755 tests/discard/test-fstrim.pl
diff --git a/Makefile.am b/Makefile.am
index 020628e..11b12c2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -46,6 +46,7 @@ SUBDIRS += tests/events
SUBDIRS += tests/parallel
SUBDIRS += tests/create
SUBDIRS += tests/disks
+SUBDIRS += tests/discard
SUBDIRS += tests/mountable
SUBDIRS += tests/network
SUBDIRS += tests/lvm
diff --git a/configure.ac b/configure.ac
index 35460e2..3785ea8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1673,6 +1673,7 @@ AC_CONFIG_FILES([Makefile
tests/charsets/Makefile
tests/create/Makefile
tests/data/Makefile
+ tests/discard/Makefile
tests/disks/Makefile
tests/disks/test-qemu-drive-libvirt.xml
tests/disk-labels/Makefile
diff --git a/tests/discard/Makefile.am b/tests/discard/Makefile.am
new file mode 100644
index 0000000..e109628
--- /dev/null
+++ b/tests/discard/Makefile.am
@@ -0,0 +1,28 @@
+# libguestfs
+# Copyright (C) 2014 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+include $(top_srcdir)/subdir-rules.mk
+
+TESTS = \
+ test-blkdiscard.pl \
+ test-discard.pl \
+ test-fstrim.pl
+
+TESTS_ENVIRONMENT = $(top_builddir)/run --test
+
+EXTRA_DIST = \
+ $(TESTS)
diff --git a/tests/discard/test-blkdiscard.pl b/tests/discard/test-blkdiscard.pl
new file mode 100755
index 0000000..3caafbc
--- /dev/null
+++ b/tests/discard/test-blkdiscard.pl
@@ -0,0 +1,111 @@
+#!/usr/bin/perl
+# Copyright (C) 2014 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test that blkdiscard works.
+
+use strict;
+use warnings;
+
+use Sys::Guestfs;
+
+# Since we read error messages, we want to ensure they are printed
+# in English, hence:
+$ENV{"LANG"} = "C";
+
+$| = 1;
+
+if ($ENV{SKIP_TEST_BLKDISCARD_PL}) {
+ print "$0: skipped test because environment variable is set\n";
+ exit 77;
+}
+
+my $g = Sys::Guestfs->new ();
+
+# Discard is only supported when using qemu.
+if ($g->get_backend () ne "libvirt" &&
+ $g->get_backend () !~ /^libvirt:/ &&
+ $g->get_backend () ne "direct") {
+ print "$0: skipped test because discard is only supported when using
qemu\n";
+ exit 77;
+}
+
+# You can set this to "raw" or "qcow2".
+my $format = "raw";
+
+my $size = 5 * 1024 * 1024;
+
+my $disk;
+my @args;
+if ($format eq "raw") {
+ $disk = "test-blkdiscard.img";
+ @args = ( preallocation => "sparse" );
+} elsif ($format eq "qcow2") {
+ $disk = "test-blkdiscard.qcow2";
+ @args = ( preallocation => "off", compat => "1.1"
);
+} else {
+ die "$0: invalid disk format: $format\n";
+}
+
+# Create a disk and add it with discard enabled. This is allowed to
+# fail, eg because qemu is too old, but libguestfs must tell us that
+# it failed (since we're using 'enable', not 'besteffort').
+$g->disk_create ($disk, $format, $size, @args);
+END { unlink ($disk); };
+
+eval {
+ $g->add_drive ($disk, format => $format, readonly => 0, discard
=> "enable");
+ $g->launch ();
+};
+if ($@) {
+ if ($@ =~ /discard cannot be enabled on this drive/) {
+ # This is OK. Libguestfs says it's not possible to enable
+ # discard on this drive (eg. because qemu is too old). Print
+ # the reason and skip the test.
+ print "$0: skipped test: $@\n";
+ exit 77;
+ }
+ die # propagate the unexpected error
+}
+
+# At this point we've got a disk which claims to support discard.
+# Let's test that theory.
+
+my $orig_size = (stat ($disk))[12];
+print "original size:\t$orig_size (blocks)\n";
+
+# Fill the block device with some random data.
+
+$g->copy_device_to_device ("/dev/urandom", "/dev/sda",
size => $size);
+$g->sync ();
+
+my $full_size = (stat ($disk))[12];
+print "full size:\t$full_size (blocks)\n";
+
+die "$0: surprising result: full size <= original size\n"
+ if $full_size <= $orig_size;
+
+# Discard the data on the device.
+
+$g->blkdiscard ("/dev/sda");
+$g->shutdown ();
+$g->close ();
+
+my $trimmed_size = (stat ($disk))[12];
+print "trimmed size:\t$trimmed_size (blocks)\n";
+
+die "$0: looks like the blkdiscard operation did not work\n"
+ if $trimmed_size >= $full_size;
diff --git a/tests/discard/test-discard.pl b/tests/discard/test-discard.pl
new file mode 100755
index 0000000..3f350b3
--- /dev/null
+++ b/tests/discard/test-discard.pl
@@ -0,0 +1,117 @@
+#!/usr/bin/perl
+# Copyright (C) 2014 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test that -o discard works.
+
+use strict;
+use warnings;
+
+use Sys::Guestfs;
+
+# Since we read error messages, we want to ensure they are printed
+# in English, hence:
+$ENV{"LANG"} = "C";
+
+$| = 1;
+
+if ($ENV{SKIP_TEST_DISCARD_PL}) {
+ print "$0: skipped test because environment variable is set\n";
+ exit 77;
+}
+
+my $g = Sys::Guestfs->new ();
+
+# Discard is only supported when using qemu.
+if ($g->get_backend () ne "libvirt" &&
+ $g->get_backend () !~ /^libvirt:/ &&
+ $g->get_backend () ne "direct") {
+ print "$0: skipped test because discard is only supported when using
qemu\n";
+ exit 77;
+}
+
+# You can set this to "raw" or "qcow2".
+my $format = "raw";
+
+# Size needs to be at least 32 MB so we can fit an ext4 filesystem on it.
+my $size = 64 * 1024 * 1024;
+
+my $disk;
+my @args;
+if ($format eq "raw") {
+ $disk = "test-discard.img";
+ @args = ( preallocation => "sparse" );
+} elsif ($format eq "qcow2") {
+ $disk = "test-discard.qcow2";
+ @args = ( preallocation => "off", compat => "1.1"
);
+} else {
+ die "$0: invalid disk format: $format\n";
+}
+
+# Create a disk and add it with discard enabled. This is allowed to
+# fail, eg because qemu is too old, but libguestfs must tell us that
+# it failed (since we're using 'enable', not 'besteffort').
+$g->disk_create ($disk, $format, $size, @args);
+END { unlink ($disk); };
+
+eval {
+ $g->add_drive ($disk, format => $format, readonly => 0, discard
=> "enable");
+ $g->launch ();
+};
+if ($@) {
+ if ($@ =~ /discard cannot be enabled on this drive/) {
+ # This is OK. Libguestfs says it's not possible to enable
+ # discard on this drive (eg. because qemu is too old). Print
+ # the reason and skip the test.
+ print "$0: skipped test: $@\n";
+ exit 77;
+ }
+ die # propagate the unexpected error
+}
+
+# At this point we've got a disk which claims to support discard.
+# Let's test that theory.
+
+my $orig_size = (stat ($disk))[12];
+print "original size:\t$orig_size (blocks)\n";
+#system "du -sh $disk";
+
+# Write a filesystem onto the disk and fill it with data.
+
+$g->mkfs ("ext4", "/dev/sda");
+$g->mount_options ("discard", "/dev/sda",
"/");
+$g->fill (33, 10000000, "/data");
+$g->sync ();
+
+my $full_size = (stat ($disk))[12];
+print "full size:\t$full_size (blocks)\n";
+#system "du -sh $disk";
+
+die "$0: surprising result: full size <= original size\n"
+ if $full_size <= $orig_size;
+
+# Remove the file and check the filesystem is trimmed automatically.
+
+$g->rm ("/data");
+$g->sync ();
+$g->close ();
+
+my $trimmed_size = (stat ($disk))[12];
+print "trimmed size:\t$trimmed_size (blocks)\n";
+#system "du -sh $disk";
+
+die "$0: looks like the -o discard mount option did not work\n"
+ if $trimmed_size >= $full_size;
diff --git a/tests/discard/test-fstrim.pl b/tests/discard/test-fstrim.pl
new file mode 100755
index 0000000..54451b8
--- /dev/null
+++ b/tests/discard/test-fstrim.pl
@@ -0,0 +1,125 @@
+#!/usr/bin/perl
+# Copyright (C) 2014 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test that fstrim works.
+
+use strict;
+use warnings;
+
+use Sys::Guestfs;
+
+# Since we read error messages, we want to ensure they are printed
+# in English, hence:
+$ENV{"LANG"} = "C";
+
+$| = 1;
+
+if ($ENV{SKIP_TEST_FSTRIM_PL}) {
+ print "$0: skipped test because environment variable is set\n";
+ exit 77;
+}
+
+my $g = Sys::Guestfs->new ();
+
+# Discard is only supported when using qemu.
+if ($g->get_backend () ne "libvirt" &&
+ $g->get_backend () !~ /^libvirt:/ &&
+ $g->get_backend () ne "direct") {
+ print "$0: skipped test because discard is only supported when using
qemu\n";
+ exit 77;
+}
+
+# You can set this to "raw" or "qcow2".
+my $format = "raw";
+
+# Size needs to be at least 32 MB so we can fit an ext4 filesystem on it.
+my $size = 64 * 1024 * 1024;
+
+my $disk;
+my @args;
+if ($format eq "raw") {
+ $disk = "test-fstrim.img";
+ @args = ( preallocation => "sparse" );
+} elsif ($format eq "qcow2") {
+ $disk = "test-fstrim.qcow2";
+ @args = ( preallocation => "off", compat => "1.1"
);
+} else {
+ die "$0: invalid disk format: $format\n";
+}
+
+# Create a disk and add it with discard enabled. This is allowed to
+# fail, eg because qemu is too old, but libguestfs must tell us that
+# it failed (since we're using 'enable', not 'besteffort').
+$g->disk_create ($disk, $format, $size, @args);
+END { unlink ($disk); };
+
+eval {
+ $g->add_drive ($disk, format => $format, readonly => 0, discard
=> "enable");
+ $g->launch ();
+};
+if ($@) {
+ if ($@ =~ /discard cannot be enabled on this drive/) {
+ # This is OK. Libguestfs says it's not possible to enable
+ # discard on this drive (eg. because qemu is too old). Print
+ # the reason and skip the test.
+ print "$0: skipped test: $@\n";
+ exit 77;
+ }
+ die # propagate the unexpected error
+}
+
+# Is fstrim available in the appliance?
+unless ($g->feature_available (["fstrim"])) {
+ print "$0: skipped test because fstrim is not available\n";
+ exit 77;
+}
+
+# At this point we've got a disk which claims to support discard.
+# Let's test that theory.
+
+my $orig_size = (stat ($disk))[12];
+print "original size:\t$orig_size (blocks)\n";
+#system "du -sh $disk";
+
+# Write a filesystem onto the disk and fill it with data.
+
+$g->mkfs ("ext4", "/dev/sda");
+# Use nodiscard here so the 'rm' below doesn't discard data.
+$g->mount_options ("nodiscard", "/dev/sda",
"/");
+$g->fill (33, 10000000, "/data");
+$g->sync ();
+
+my $full_size = (stat ($disk))[12];
+print "full size:\t$full_size (blocks)\n";
+#system "du -sh $disk";
+
+die "$0: surprising result: full size <= original size\n"
+ if $full_size <= $orig_size;
+
+# Remove the file and then try to trim the filesystem.
+
+$g->rm ("/data");
+$g->fstrim ("/");
+$g->sync ();
+$g->close ();
+
+my $trimmed_size = (stat ($disk))[12];
+print "trimmed size:\t$trimmed_size (blocks)\n";
+#system "du -sh $disk";
+
+die "$0: looks like the fstrim operation did not work\n"
+ if $trimmed_size >= $full_size;
--
1.8.5.3
Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 05/10] Pass cachemode parameter from add-domain to add-drive.
Allow callers to specify that all the disks from a domain are added
with a specific cachemode (instead of always having to use the
default, writeback).
---
generator/actions.ml | 4 ++--
src/guestfs-internal-frontend.h | 2 ++
src/libvirt-domain.c | 16 ++++++++++++++++
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/generator/actions.ml b/generator/actions.ml
index 4e2c9e0..597138c 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -1576,7 +1576,7 @@ not part of the formal API and can be removed or changed
at any time." };
{ defaults with
name = "add_domain";
- style = RInt "nrdisks", [String "dom"], [OString
"libvirturi"; OBool "readonly"; OString "iface";
OBool "live"; OBool "allowuuid"; OString
"readonlydisk"];
+ style = RInt "nrdisks", [String "dom"], [OString
"libvirturi"; OBool "readonly"; OString "iface";
OBool "live"; OBool "allowuuid"; OString
"readonlydisk"; OString "cachemode"];
fish_alias = ["domain"]; config_only = true;
shortdesc = "add the disk(s) from a named libvirt domain";
longdesc = "\
@@ -1668,7 +1668,7 @@ C<guestfs_add_drive_opts>." };
This interface is not quite baked yet. -- RWMJ 2010-11-11
{ defaults with
name = "add_libvirt_dom";
- style = RInt "nrdisks", [Pointer ("virDomainPtr",
"dom")], [Bool "readonly"; String "iface"; Bool
"live"; String "readonlydisk"];
+ style = RInt "nrdisks", [Pointer ("virDomainPtr",
"dom")], [Bool "readonly"; String "iface"; Bool
"live"; String "readonlydisk"; OString
"cachemode"];
in_fish = false;
shortdesc = "add the disk(s) from a libvirt domain";
longdesc = "\
diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h
index 3b0a480..80ebf50 100644
--- a/src/guestfs-internal-frontend.h
+++ b/src/guestfs-internal-frontend.h
@@ -142,6 +142,8 @@ struct guestfs___add_libvirt_dom_argv {
int live;
#define GUESTFS___ADD_LIBVIRT_DOM_READONLYDISK_BITMASK (UINT64_C(1)<<3)
const char *readonlydisk;
+#define GUESTFS___ADD_LIBVIRT_DOM_CACHEMODE_BITMASK (UINT64_C(1)<<4)
+ const char *cachemode;
};
extern GUESTFS_DLL_PUBLIC int guestfs___add_libvirt_dom (guestfs_h *g,
virDomainPtr dom, const struct guestfs___add_libvirt_dom_argv *optargs);
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 414e996..3a1c691 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -63,6 +63,7 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name,
int allowuuid;
const char *readonlydisk;
const char *iface;
+ const char *cachemode;
struct guestfs___add_libvirt_dom_argv optargs2 = { .bitmask = 0 };
libvirturi = optargs->bitmask & GUESTFS_ADD_DOMAIN_LIBVIRTURI_BITMASK
@@ -77,6 +78,8 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name,
? optargs->allowuuid : 0;
readonlydisk = optargs->bitmask &
GUESTFS_ADD_DOMAIN_READONLYDISK_BITMASK
? optargs->readonlydisk : NULL;
+ cachemode = optargs->bitmask & GUESTFS_ADD_DOMAIN_CACHEMODE_BITMASK
+ ? optargs->cachemode : NULL;
if (live && readonly) {
error (g, _("you cannot set both live and readonly flags"));
@@ -129,6 +132,10 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name,
optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_READONLYDISK_BITMASK;
optargs2.readonlydisk = readonlydisk;
}
+ if (cachemode) {
+ optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_CACHEMODE_BITMASK;
+ optargs2.cachemode = cachemode;
+ }
r = guestfs___add_libvirt_dom (g, dom, &optargs2);
@@ -163,6 +170,7 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
ssize_t r;
int readonly;
const char *iface;
+ const char *cachemode;
int live;
/* Default for back-compat reasons: */
enum readonlydisk readonlydisk = readonlydisk_write;
@@ -196,6 +204,10 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
}
}
+ cachemode + optargs->bitmask &
GUESTFS___ADD_LIBVIRT_DOM_CACHEMODE_BITMASK
+ ? optargs->cachemode : NULL;
+
if (live && readonly) {
error (g, _("you cannot set both live and readonly flags"));
return -1;
@@ -256,6 +268,10 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
data.optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_IFACE_BITMASK;
data.optargs.iface = iface;
}
+ if (cachemode) {
+ data.optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_CACHEMODE_BITMASK;
+ data.optargs.cachemode = cachemode;
+ }
/* Checkpoint the command line around the operation so that either
* all disks are added or none are added.
--
1.8.5.3
Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 06/10] Pass discard parameter from add-domain to add-drive.
Allow callers to specify that all the disks from a domain are added
with a specific discard mode (instead of always having discard
disabled).
---
generator/actions.ml | 4 ++--
src/guestfs-internal-frontend.h | 2 ++
src/libvirt-domain.c | 16 ++++++++++++++++
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/generator/actions.ml b/generator/actions.ml
index 597138c..742df6a 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -1576,7 +1576,7 @@ not part of the formal API and can be removed or changed
at any time." };
{ defaults with
name = "add_domain";
- style = RInt "nrdisks", [String "dom"], [OString
"libvirturi"; OBool "readonly"; OString "iface";
OBool "live"; OBool "allowuuid"; OString
"readonlydisk"; OString "cachemode"];
+ style = RInt "nrdisks", [String "dom"], [OString
"libvirturi"; OBool "readonly"; OString "iface";
OBool "live"; OBool "allowuuid"; OString
"readonlydisk"; OString "cachemode"; OString
"discard"];
fish_alias = ["domain"]; config_only = true;
shortdesc = "add the disk(s) from a named libvirt domain";
longdesc = "\
@@ -1668,7 +1668,7 @@ C<guestfs_add_drive_opts>." };
This interface is not quite baked yet. -- RWMJ 2010-11-11
{ defaults with
name = "add_libvirt_dom";
- style = RInt "nrdisks", [Pointer ("virDomainPtr",
"dom")], [Bool "readonly"; String "iface"; Bool
"live"; String "readonlydisk"; OString
"cachemode"];
+ style = RInt "nrdisks", [Pointer ("virDomainPtr",
"dom")], [Bool "readonly"; String "iface"; Bool
"live"; String "readonlydisk"; OString
"cachemode"; OString "discard"];
in_fish = false;
shortdesc = "add the disk(s) from a libvirt domain";
longdesc = "\
diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h
index 80ebf50..6bf0a94 100644
--- a/src/guestfs-internal-frontend.h
+++ b/src/guestfs-internal-frontend.h
@@ -144,6 +144,8 @@ struct guestfs___add_libvirt_dom_argv {
const char *readonlydisk;
#define GUESTFS___ADD_LIBVIRT_DOM_CACHEMODE_BITMASK (UINT64_C(1)<<4)
const char *cachemode;
+#define GUESTFS___ADD_LIBVIRT_DOM_DISCARD_BITMASK (UINT64_C(1)<<5)
+ const char *discard;
};
extern GUESTFS_DLL_PUBLIC int guestfs___add_libvirt_dom (guestfs_h *g,
virDomainPtr dom, const struct guestfs___add_libvirt_dom_argv *optargs);
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 3a1c691..cadae3e 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -64,6 +64,7 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name,
const char *readonlydisk;
const char *iface;
const char *cachemode;
+ const char *discard;
struct guestfs___add_libvirt_dom_argv optargs2 = { .bitmask = 0 };
libvirturi = optargs->bitmask & GUESTFS_ADD_DOMAIN_LIBVIRTURI_BITMASK
@@ -80,6 +81,8 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name,
? optargs->readonlydisk : NULL;
cachemode = optargs->bitmask & GUESTFS_ADD_DOMAIN_CACHEMODE_BITMASK
? optargs->cachemode : NULL;
+ discard = optargs->bitmask & GUESTFS_ADD_DOMAIN_DISCARD_BITMASK
+ ? optargs->discard : NULL;
if (live && readonly) {
error (g, _("you cannot set both live and readonly flags"));
@@ -136,6 +139,10 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name,
optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_CACHEMODE_BITMASK;
optargs2.cachemode = cachemode;
}
+ if (discard) {
+ optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_DISCARD_BITMASK;
+ optargs2.discard = discard;
+ }
r = guestfs___add_libvirt_dom (g, dom, &optargs2);
@@ -171,6 +178,7 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
int readonly;
const char *iface;
const char *cachemode;
+ const char *discard;
int live;
/* Default for back-compat reasons: */
enum readonlydisk readonlydisk = readonlydisk_write;
@@ -208,6 +216,10 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
optargs->bitmask & GUESTFS___ADD_LIBVIRT_DOM_CACHEMODE_BITMASK
? optargs->cachemode : NULL;
+ discard + optargs->bitmask &
GUESTFS___ADD_LIBVIRT_DOM_DISCARD_BITMASK
+ ? optargs->discard : NULL;
+
if (live && readonly) {
error (g, _("you cannot set both live and readonly flags"));
return -1;
@@ -272,6 +284,10 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
data.optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_CACHEMODE_BITMASK;
data.optargs.cachemode = cachemode;
}
+ if (discard) {
+ data.optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK;
+ data.optargs.discard = discard;
+ }
/* Checkpoint the command line around the operation so that either
* all disks are added or none are added.
--
1.8.5.3
Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 07/10] virt-format: Discard the data on the disks.
If possible, this means that the host will be able to reclaim most of
the space used by formatted disks.
---
fish/options.c | 5 +++++
fish/options.h | 1 +
format/format.c | 13 +++++++++++++
3 files changed, 19 insertions(+)
diff --git a/fish/options.c b/fish/options.c
index cfdbdfe..80b71ec 100644
--- a/fish/options.c
+++ b/fish/options.c
@@ -134,6 +134,10 @@ add_drives_handle (guestfs_h *g, struct drv *drv, char
next_drive)
ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_CACHEMODE_BITMASK;
ad_optargs.cachemode = drv->a.cachemode;
}
+ if (drv->a.discard) {
+ ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK;
+ ad_optargs.discard = drv->a.discard;
+ }
r = guestfs_add_drive_opts_argv (g, drv->a.filename, &ad_optargs);
if (r == -1)
@@ -279,6 +283,7 @@ free_drives (struct drv *drv)
free (drv->a.filename);
/* a.format is an optarg, so don't free it */
/* a.cachemode is a static string, so don't free it */
+ /* a.discard is a static string, so don't free it */
break;
case drv_uri:
free (drv->uri.path);
diff --git a/fish/options.h b/fish/options.h
index a7b8277..dbf9163 100644
--- a/fish/options.h
+++ b/fish/options.h
@@ -61,6 +61,7 @@ struct drv {
char *filename; /* disk filename */
const char *format; /* format (NULL == autodetect) */
const char *cachemode;/* cachemode (NULL == default) */
+ const char *discard; /* discard (NULL == disable) */
} a;
struct {
char *path; /* disk path */
diff --git a/format/format.c b/format/format.c
index cff0d82..684bffd 100644
--- a/format/format.c
+++ b/format/format.c
@@ -188,6 +188,11 @@ main (int argc, char *argv[])
case 'a':
OPTION_a;
+
+ /* Enable discard on all drives added on the command line. */
+ assert (drvs != NULL);
+ assert (drvs->type == drv_a);
+ drvs->a.discard = "besteffort";
break;
case 'v':
@@ -342,6 +347,14 @@ do_format (void)
}
}
+ /* Send TRIM/UNMAP to all block devices, to give back the space to
+ * the host. However don't fail if this doesn't work.
+ */
+ guestfs_push_error_handler (g, NULL, NULL);
+ for (i = 0; devices[i] != NULL; ++i)
+ guestfs_blkdiscard (g, devices[i]);
+ guestfs_pop_error_handler (g);
+
if (do_rescan (devices))
return 1; /* which means, reopen the handle and retry */
--
1.8.5.3
Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 08/10] sysprep: Add disks with discard = "besteffort".
Since virt-sysprep tends to delete a lot of files, adding discard
support to it makes some sense.
Note that this probably won't have any effect for most filesystems
since:
(a) ext4 mounts also need to use -o discard,
(b) ext4, and maybe others, require you to call fstrim explicitly,
they don't discard automatically (except for userspace tools like
mkfs.ext4 but that doesn't apply in this case).
---
sysprep/main.ml | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sysprep/main.ml b/sysprep/main.ml
index 6f631d5..9047a74 100644
--- a/sysprep/main.ml
+++ b/sysprep/main.ml
@@ -198,7 +198,11 @@ read the man page virt-sysprep(1).
fun (g : Guestfs.guestfs) readonly ->
let allowuuid = true in
let readonlydisk = "ignore" (* ignore CDs, data drives *) in
- ignore (g#add_domain ~readonly ?libvirturi ~allowuuid ~readonlydisk
dom)
+ let discard = if readonly then None else Some "besteffort" in
+ ignore (g#add_domain
+ ~readonly ?discard
+ ?libvirturi ~allowuuid ~readonlydisk
+ dom)
| _, Some _ ->
eprintf (f_"%s: you cannot give -a and -d options together\n")
prog;
eprintf (f_"Read virt-sysprep(1) man page for further
information.\n");
@@ -209,7 +213,11 @@ read the man page virt-sysprep(1).
fun (uri, format) ->
let { URI.path = path; protocol = protocol;
server = server; username = username } = uri in
- g#add_drive ~readonly ?format ~protocol ?server ?username path
+ let discard = if readonly then None else Some
"besteffort" in
+ g#add_drive
+ ~readonly ?discard
+ ?format ~protocol ?server ?username
+ path
) files
in
--
1.8.5.3
Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 09/10] sparsify: Add virt-sparsify --in-place mode.
---
po/POTFILES-ml | 1 +
sparsify/Makefile.am | 2 +
sparsify/cmdline.ml | 77 +++++++++++++++++-------
sparsify/in_place.ml | 143 +++++++++++++++++++++++++++++++++++++++++++++
sparsify/sparsify.ml | 3 +
sparsify/virt-sparsify.pod | 63 +++++++++++++++++---
6 files changed, 259 insertions(+), 30 deletions(-)
create mode 100644 sparsify/in_place.ml
diff --git a/po/POTFILES-ml b/po/POTFILES-ml
index 899a460..ed96697 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -35,6 +35,7 @@ mllib/urandom.ml
resize/resize.ml
sparsify/cmdline.ml
sparsify/copying.ml
+sparsify/in_place.ml
sparsify/sparsify.ml
sysprep/main.ml
sysprep/sysprep_operation.ml
diff --git a/sparsify/Makefile.am b/sparsify/Makefile.am
index 2f32093..811c131 100644
--- a/sparsify/Makefile.am
+++ b/sparsify/Makefile.am
@@ -28,6 +28,7 @@ CLEANFILES = *~ *.cmi *.cmo *.cmx *.cmxa *.o virt-sparsify
SOURCES = \
cmdline.ml \
copying.ml \
+ in_place.ml \
sparsify.ml \
statvfs-c.c
@@ -46,6 +47,7 @@ deps = \
statvfs-c.o \
cmdline.cmx \
copying.cmx \
+ in_place.cmx \
sparsify.cmx
if HAVE_OCAMLOPT
diff --git a/sparsify/cmdline.ml b/sparsify/cmdline.ml
index b714a9e..d25e9e9 100644
--- a/sparsify/cmdline.ml
+++ b/sparsify/cmdline.ml
@@ -27,7 +27,8 @@ let prog = Filename.basename Sys.executable_name
let error fs = error ~prog fs
type mode_t - Mode_copying of string * check_t * bool * string option * string
option
+| Mode_copying of string * check_t * bool * string option * string option
+| Mode_in_place
and check_t = [`Ignore|`Continue|`Warn|`Fail]
let parse_cmdline () @@ -54,6 +55,7 @@ let parse_cmdline () let debug_gc =
ref false in
let format = ref "" in
let ignores = ref [] in
+ let in_place = ref false in
let machine_readable = ref false in
let option = ref "" in
let quiet = ref false in
@@ -69,6 +71,8 @@ let parse_cmdline () "--debug-gc", Arg.Set
debug_gc, " " ^ s_"Debug GC and memory allocations";
"--format", Arg.Set_string format, s_"format" ^
" " ^ s_"Format of input disk";
"--ignore", Arg.String (add ignores), s_"fs" ^ "
" ^ s_"Ignore filesystem";
+ "--in-place", Arg.Set in_place, " " ^
s_"Modify the disk image in-place";
+ "--inplace", Arg.Set in_place, ditto;
"--long-options", Arg.Unit display_long_options, " " ^
s_"List long options";
"--machine-readable", Arg.Set machine_readable, " " ^
s_"Make output machine readable";
"-o", Arg.Set_string option, s_"option" ^
" " ^ s_"Add qemu-img options";
@@ -90,6 +94,8 @@ let parse_cmdline ()
virt-sparsify [--options] indisk outdisk
+ virt-sparsify [--options] --in-place disk
+
A short summary of the options is given below. For detailed help please
read the man page virt-sparsify(1).
")
@@ -103,6 +109,7 @@ read the man page virt-sparsify(1).
let debug_gc = !debug_gc in
let format = match !format with "" -> None | str -> Some str
in
let ignores = List.rev !ignores in
+ let in_place = !in_place in
let machine_readable = !machine_readable in
let option = match !option with "" -> None | str -> Some str
in
let quiet = !quiet in
@@ -118,6 +125,7 @@ read the man page virt-sparsify(1).
printf "linux-swap\n";
printf "zero\n";
printf "check-tmpdir\n";
+ printf "in-place\n";
let g = new G.guestfs () in
g#add_drive "/dev/null";
g#launch ();
@@ -128,12 +136,14 @@ read the man page virt-sparsify(1).
exit 0
);
- (* Verify we got exactly 2 disks. *)
+ (* Verify we got exactly 1 or 2 disks, depending on the mode. *)
let indisk, outdisk - match List.rev !disks with
- | [indisk; outdisk] -> indisk, outdisk
+ match in_place, List.rev !disks with
+ | false, [indisk; outdisk] -> indisk, outdisk
+ | true, [disk] -> disk, ""
| _ ->
- error "usage is: %s [--options] indisk outdisk" prog in
+ error "usage is: %s [--options] indisk outdisk OR %s --in-place
disk"
+ prog prog in
(* Simple-minded check that the user isn't trying to use the
* same disk for input and output.
@@ -141,24 +151,49 @@ read the man page virt-sparsify(1).
if indisk = outdisk then
error (f_"you cannot use the same disk image for input and
output");
- (* The input disk must be an absolute path, so we can store the name
- * in the overlay disk.
- *)
let indisk - if not (Filename.is_relative indisk) then
+ if not in_place then (
+ (* The input disk must be an absolute path, so we can store the name
+ * in the overlay disk.
+ *)
+ let indisk + if not (Filename.is_relative indisk) then
+ indisk
+ else
+ Sys.getcwd () // indisk in
+
+ (* Check the output is not a block or char special (RHBZ#1056290). *)
+ if is_block_device outdisk then
+ error (f_"output '%s' cannot be a block device, it must be
a regular file")
+ outdisk;
+
+ if is_char_device outdisk then
+ error (f_"output '%s' cannot be a character device, it
must be a regular file")
+ outdisk;
+
indisk
+ )
+ else ( (* --in-place checks *)
+ if check_tmpdir <> `Warn then
+ error (f_"you cannot use --in-place and --check-tmpdir options
together");
+
+ if compress then
+ error (f_"you cannot use --in-place and --compress options
together");
+
+ if convert <> None then
+ error (f_"you cannot use --in-place and --convert options
together");
+
+ if option <> None then
+ error (f_"you cannot use --in-place and -o options
together");
+
+ indisk
+ ) in
+
+ let mode + if not in_place then
+ Mode_copying (outdisk, check_tmpdir, compress, convert, option)
else
- Sys.getcwd () // indisk in
-
- (* Check the output is not a block or char special (RHBZ#1056290). *)
- if is_block_device outdisk then
- error (f_"output '%s' cannot be a block device, it must be a
regular file")
- outdisk;
-
- if is_char_device outdisk then
- error (f_"output '%s' cannot be a character device, it must be
a regular file")
- outdisk;
+ Mode_in_place in
indisk, debug_gc, format, ignores, machine_readable,
- quiet, verbose, trace, zeroes,
- Mode_copying (outdisk, check_tmpdir, compress, convert, option)
+ quiet, verbose, trace, zeroes, mode
diff --git a/sparsify/in_place.ml b/sparsify/in_place.ml
new file mode 100644
index 0000000..9f46ad3
--- /dev/null
+++ b/sparsify/in_place.ml
@@ -0,0 +1,143 @@
+(* virt-sparsify
+ * Copyright (C) 2011-2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+(* This is the virt-sparsify --in-place mode. *)
+
+open Unix
+open Printf
+
+open Common_gettext.Gettext
+
+module G = Guestfs
+
+open Common_utils
+open Cmdline
+
+let run disk format ignores machine_readable quiet verbose trace zeroes + (*
Connect to libguestfs. *)
+ let g = new G.guestfs () in
+ if trace then g#set_trace true;
+ if verbose then g#set_verbose true;
+
+ (* XXX Current limitation of the API. Can remove this hunk in future. *)
+ let format + match format with
+ | Some _ -> format
+ | None -> Some (g#disk_format disk) in
+
+ g#add_drive ?format ~discard:"enable" disk;
+
+ if not quiet then Progress.set_up_progress_bar ~machine_readable g;
+ g#launch ();
+
+ (* Discard non-ignored filesystems that we are able to mount, and
+ * selected swap partitions.
+ *)
+ let filesystems = g#list_filesystems () in
+ let filesystems = List.map fst filesystems in
+ let filesystems = List.sort compare filesystems in
+
+ let is_ignored fs + let fs = g#canonical_device_name fs in
+ List.exists (fun fs' -> fs = g#canonical_device_name fs')
ignores
+ in
+
+ List.iter (
+ fun fs ->
+ if not (is_ignored fs) then (
+ if List.mem fs zeroes then (
+ if not quiet then
+ printf (f_"Zeroing %s ...\n%!") fs;
+
+ if not (g#blkdiscardzeroes fs) then
+ g#zero_device fs;
+ g#blkdiscard fs
+ ) else (
+ let mounted + try g#mount_options "discard" fs
"/"; true
+ with _ -> false in
+
+ if mounted then (
+ if not quiet then
+ printf (f_"Trimming %s ...\n%!") fs;
+
+ g#fstrim "/"
+ ) else (
+ let is_linux_x86_swap + (* Look for the signature for
Linux swap on i386.
+ * Location depends on page size, so it definitely won't
+ * work on non-x86 architectures (eg. on PPC, page size is
+ * 64K). Also this avoids hibernated swap space: in those,
+ * the signature is moved to a different location.
+ *)
+ try g#pread_device fs 10 4086L = "SWAPSPACE2"
+ with _ -> false in
+
+ if is_linux_x86_swap then (
+ if not quiet then
+ printf (f_"Clearing Linux swap on %s ...\n%!") fs;
+
+ (* Don't use mkswap. Just preserve the header containing
+ * the label, UUID and swap format version (libguestfs
+ * mkswap may differ from guest's own).
+ *)
+ let header = g#pread_device fs 4096 0L in
+ g#blkdiscard fs;
+ if g#pwrite_device fs header 0L <> 4096 then
+ error (f_"pwrite: short write restoring swap partition
header")
+ )
+ )
+ );
+
+ g#umount_all ()
+ )
+ ) filesystems;
+
+ (* Discard unused space in volume groups. *)
+ let vgs = g#vgs () in
+ let vgs = Array.to_list vgs in
+ let vgs = List.sort compare vgs in
+ List.iter (
+ fun vg ->
+ if not (List.mem vg ignores) then (
+ let lvname = string_random8 () in
+ let lvdev = "/dev/" ^ vg ^ "/" ^ lvname in
+
+ let created + try g#lvcreate_free lvname vg 100; true
+ with _ -> false in
+
+ if created then (
+ if not quiet then
+ printf (f_"Discard space in volgroup %s ...\n%!") vg;
+
+ g#blkdiscard lvdev;
+ g#sync ();
+ g#lvremove lvdev
+ )
+ )
+ ) vgs;
+
+ g#shutdown ();
+ g#close ();
+
+ (* Finished. *)
+ if not quiet then (
+ print_newline ();
+ wrap (s_"Sparsify in-place operation completed with no
errors.\n");
+ )
diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml
index faefb23..f148296 100644
--- a/sparsify/sparsify.ml
+++ b/sparsify/sparsify.ml
@@ -37,6 +37,9 @@ let rec main () | Mode_copying (outdisk, check_tmpdir,
compress, convert, option) ->
Copying.run indisk outdisk check_tmpdir compress convert
format ignores machine_readable option quiet verbose trace zeroes
+ | Mode_in_place ->
+ In_place.run indisk format ignores machine_readable
+ quiet verbose trace zeroes
);
if debug_gc then
diff --git a/sparsify/virt-sparsify.pod b/sparsify/virt-sparsify.pod
index 84e94c5..c12a15f 100644
--- a/sparsify/virt-sparsify.pod
+++ b/sparsify/virt-sparsify.pod
@@ -8,6 +8,8 @@ virt-sparsify - Make a virtual machine disk sparse
virt-sparsify [--options] indisk outdisk
+ virt-sparsify [--options] --in-place disk
+
=head1 DESCRIPTION
Virt-sparsify is a tool which can make a virtual machine disk (or any
@@ -45,13 +47,6 @@ such as C<du -sh>. It can make a huge difference:
=item *
-Virt-sparsify does not do in-place modifications. It copies from a
-source image to a destination image, leaving the source unchanged.
-I<Check that the sparsification was successful before deleting the
-source image>.
-
-=item *
-
The virtual machine I<must be shut down> before using this tool.
=item *
@@ -60,6 +55,9 @@ Virt-sparsify may require up to 2x the virtual size of the
source disk
image (1 temporary copy + 1 destination image). This is in the worst
case and usually much less space is required.
+If you are using the I<--in-place> option, then large amounts of
+temporary space are B<not> required.
+
=item *
Virt-sparsify cannot resize disk images. To do that, use
@@ -105,6 +103,11 @@ to ignore (don't zero free space on) certain
filesystems by doing:
See L<virt-filesystems(1)> to get a list of filesystems within a disk
image.
+Since virt-sparsify E<ge> 1.26, you can now sparsify a disk image
+in place by doing:
+
+ virt-sparsify --in-place disk.img
+
=head1 OPTIONS
=over 4
@@ -147,11 +150,15 @@ B<fail> and exit.
=back
+You cannot use this option and I<--in-place> together.
+
=item B<--compress>
Compress the output file. This I<only> works if the output format is
C<qcow2>.
+You cannot use this option and I<--in-place> together.
+
=item B<--convert> raw
=item B<--convert> qcow2
@@ -171,6 +178,8 @@ then virt-sparsify doesn't need to try to guess the
input format.
For fine-tuning the output format, see: I<--compress>, I<-o>.
+You cannot use this option and I<--in-place> together.
+
=item B<--debug-gc>
Debug garbage collection and memory allocation. This is only useful
@@ -191,14 +200,23 @@ ensure the format is always specified.
=item B<--ignore> volgroup
-Ignore the named filesystem. Free space on the filesystem will not be
+Ignore the named filesystem.
+
+When not using I<--in-place>: Free space on the filesystem will not be
zeroed, but existing blocks of zeroes will still be sparsified.
+When using I<--in-place>, the filesystem is ignored completely.
+
In the second form, this ignores the named volume group. Use the
volume group name without the C</dev/> prefix, eg. I<--ignore
vg_foo>
You can give this option multiple times.
+=item B<--in-place>
+
+Do in-place sparsification instead of copying sparsification.
+See L</IN-PLACE SPARSIFICATION> below.
+
=item B<--machine-readable>
This option is used to make the output more machine friendly
@@ -217,6 +235,8 @@ them with commas, eg:
virt-sparsify --convert qcow2 \
-o cluster_size=512,preallocation=metadata ...
+You cannot use this option and I<--in-place> together.
+
=item B<-q>
=item B<--quiet>
@@ -249,6 +269,28 @@ excellent! You can give this option multiple times.
=back
+=head1 IN-PLACE SPARSIFICATION
+
+Since virt-sparsify E<ge> 1.26, the tool is able to do in-place
+sparsification (instead of copying from an input disk to an output
+disk). This is more efficient. However it requires special support
+in libguestfs, the kernel and qemu, and it is not able to recover
+quite as much space as copying sparsification. So in-place
+sparsification is considered to be experimental at this time.
+
+To use this mode, specify a disk image which will be modified in
+place:
+
+ virt-sparsify --in-place disk.img
+
+Some options are not compatible with this mode: I<--convert>,
+I<--compress> and I<-o> because they require wholesale disk format
+changes; I<--check-tmpdir> because large amounts of temporary space
+are not required.
+
+In-place sparsification works using discard (a.k.a trim or unmap)
+support.
+
=head1 MACHINE READABLE OUTPUT
The I<--machine-readable> option can be used to make the output more
@@ -332,6 +374,9 @@ size of the tmpfs mountpoint, eg:
mount -o remount,size=10G /tmp
+If you are using the I<--in-place> option, then large amounts of
+temporary space are B<not> required.
+
=back
For other environment variables, see L<guestfs(3)/ENVIRONMENT VARIABLES>.
@@ -355,4 +400,4 @@ Richard W.M. Jones
L<http://people.redhat.com/~rjones/>
=head1 COPYRIGHT
-Copyright (C) 2011-2012 Red Hat Inc.
+Copyright (C) 2011-2014 Red Hat Inc.
--
1.8.5.3
Richard W.M. Jones
2014-Mar-12 14:26 UTC
[Libguestfs] [PATCH v3 10/10] sparsify: Add a test of the virt-sparsify --in-place option.
---
sparsify/Makefile.am | 7 +++-
sparsify/test-virt-sparsify-in-place.sh | 69 +++++++++++++++++++++++++++++++++
2 files changed, 74 insertions(+), 2 deletions(-)
create mode 100755 sparsify/test-virt-sparsify-in-place.sh
diff --git a/sparsify/Makefile.am b/sparsify/Makefile.am
index 811c131..5e1b382 100644
--- a/sparsify/Makefile.am
+++ b/sparsify/Makefile.am
@@ -20,7 +20,8 @@ include $(top_srcdir)/subdir-rules.mk
EXTRA_DIST = \
$(SOURCES) \
virt-sparsify.pod \
- test-virt-sparsify.sh
+ test-virt-sparsify.sh \
+ test-virt-sparsify-in-place.sh
CLEANFILES = *~ *.cmi *.cmo *.cmx *.cmxa *.o virt-sparsify
@@ -133,7 +134,9 @@ CLEANFILES += stamp-virt-sparsify.pod
TESTS_ENVIRONMENT = $(top_builddir)/run --test
if ENABLE_APPLIANCE
-TESTS = test-virt-sparsify.sh
+TESTS = \
+ test-virt-sparsify.sh \
+ test-virt-sparsify-in-place.sh
endif ENABLE_APPLIANCE
check-valgrind:
diff --git a/sparsify/test-virt-sparsify-in-place.sh
b/sparsify/test-virt-sparsify-in-place.sh
new file mode 100755
index 0000000..56311a0
--- /dev/null
+++ b/sparsify/test-virt-sparsify-in-place.sh
@@ -0,0 +1,69 @@
+#!/bin/bash -
+# libguestfs virt-sparsify --in-place test script
+# Copyright (C) 2011-2014 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+export LANG=C
+set -e
+
+if [ -n "$SKIP_TEST_VIRT_SPARSIFY_IN_PLACE_SH" ]; then
+ echo "$0: skipping test (environment variable set)"
+ exit 77
+fi
+
+if [ "$(../fish/guestfish get-backend)" = "uml" ]; then
+ echo "$0: skipping test because uml backend does not support
discard"
+ exit 77
+fi
+
+rm -f test-virt-sparsify-in-place.img
+
+# Create a filesystem, fill it with data, then delete the data. Then
+# prove that sparsifying it reduces the size of the final filesystem.
+
+$VG ../fish/guestfish \
+ -N
test-virt-sparsify-in-place.img=bootrootlv:/dev/VG/LV:ext4:ext4:400M:32M:gpt
<<EOF
+mount /dev/VG/LV /
+mkdir /boot
+mount /dev/sda1 /boot
+fill 1 300M /big
+fill 1 10M /boot/big
+sync
+rm /big
+rm /boot/big
+umount-all
+EOF
+
+size_before=$(du -s test-virt-sparsify-in-place.img | awk '{print $1}')
+
+$VG ./virt-sparsify --debug-gc --in-place test-virt-sparsify-in-place.img
+
+size_after=$(du -s test-virt-sparsify-in-place.img | awk '{print $1}')
+
+echo "test virt-sparsify: $size_before K -> $size_after K"
+
+if [ $size_before -lt 310000 ]; then
+ echo "test virt-sparsify --in-place: size_before ($size_before) too
small"
+ exit 1
+fi
+
+if [ $size_after -gt 15000 ]; then
+ echo "test virt-sparsify --in-place: size_after ($size_after) too
large"
+ echo "sparsification failed"
+ exit 1
+fi
+
+rm test-virt-sparsify-in-place.img
--
1.8.5.3
On Wednesday 12 March 2014 14:26:07 Richard W.M. Jones wrote:> This set of patches: > > - Adds new APIs to support discard in libguestfs. > > - Adds discard support to virt-format. > > - Adds discard support to virt-sysprep. > > - Implements virt-sparsify --in-place. > > This is now working, after fixing the rather stupid bug in fstrim. > > I've pushed the ones which were ACKed previously + the fstrim fix.Sounds good, just a couple of trivial questions/notes: - blkdiscard and blkdiscardzeroes look quite similar although one is an operation and the other is a "query", would it be possible to give them clearer names? Or it is better to have them more close to what they actually do? - is there any way to check at runtime for the availability of those two functions in the kernel/fs, and maybe advertise that as "feature" (guestfs_available, etc)? -- Pino Toscano
Richard W.M. Jones
2014-Mar-12 16:44 UTC
Re: [Libguestfs] [PATCH v3 00/10] Add discard support.
On Wed, Mar 12, 2014 at 05:19:19PM +0100, Pino Toscano wrote:> On Wednesday 12 March 2014 14:26:07 Richard W.M. Jones wrote: > > This set of patches: > > > > - Adds new APIs to support discard in libguestfs. > > > > - Adds discard support to virt-format. > > > > - Adds discard support to virt-sysprep. > > > > - Implements virt-sparsify --in-place. > > > > This is now working, after fixing the rather stupid bug in fstrim. > > > > I've pushed the ones which were ACKed previously + the fstrim fix. > > Sounds good, just a couple of trivial questions/notes: > > - blkdiscard and blkdiscardzeroes look quite similar although one is an > operation and the other is a "query", would it be possible to give > them clearer names? Or it is better to have them more close to what > they actually do?I'd prefer to keep these names similar to the Linux concepts (which is how we've done other APIs).> - is there any way to check at runtime for the availability of those two > functions in the kernel/fs, and maybe advertise that as "feature" > (guestfs_available, etc)?Good idea. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v