Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 00/18] Add discard support.
This still isn't working at the moment. See: http://marc.info/?t=139457409300003&r=1&w=2 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. Rich.
Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 01/18] daemon: fstrim: When debugging, capture and print fstrim -v output.
---
daemon/fstrim.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/daemon/fstrim.c b/daemon/fstrim.c
index cb844d1..bf9dad8 100644
--- a/daemon/fstrim.c
+++ b/daemon/fstrim.c
@@ -46,7 +46,7 @@ do_fstrim (const char *path,
const char *argv[MAX_ARGS];
size_t i = 0;
char offset_s[64], length_s[64], mfe_s[64];
- CLEANUP_FREE char *err = NULL;
+ CLEANUP_FREE char *out = NULL, *err = NULL;
int r;
ADD_ARG (argv, i, str_fstrim);
@@ -84,14 +84,21 @@ do_fstrim (const char *path,
ADD_ARG (argv, i, mfe_s);
}
+ /* When running in debug mode, use -v, capture stdout and print it below. */
+ if (verbose)
+ ADD_ARG (argv, i, "-v");
+
ADD_ARG (argv, i, path);
ADD_ARG (argv, i, NULL);
- r = commandv (NULL, &err, argv);
+ r = commandv (&out, &err, argv);
if (r == -1) {
reply_with_error ("%s", err);
return -1;
}
+ if (verbose)
+ fprintf (stderr, "%s\n", out);
+
return 0;
}
--
1.8.5.3
Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 02/18] launch: libvirt: Move the is_kvm flag (derived from libvirt capabilities) to backend data struct.
This is just rearranging the data between structs. There should be no
functional change.
---
src/launch-libvirt.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index fd6e6d2..da1d665 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -106,6 +106,7 @@ struct backend_libvirt_data {
char *selinux_imagelabel;
bool selinux_norelabel_disks;
char name[DOMAIN_NAME_LEN]; /* random name */
+ bool is_kvm; /* false = qemu, true = kvm (from
capabilities)*/
};
/* Parameters passed to construct_libvirt_xml and subfunctions. We
@@ -122,11 +123,10 @@ struct libvirt_xml_params {
char guestfsd_path[UNIX_PATH_MAX]; /* paths to sockets */
char console_path[UNIX_PATH_MAX];
bool enable_svirt; /* false if we decided to disable sVirt */
- bool is_kvm; /* false = qemu, true = kvm */
bool current_proc_is_root; /* true = euid is root */
};
-static int parse_capabilities (guestfs_h *g, const char *capabilities_xml,
struct libvirt_xml_params *params);
+static int parse_capabilities (guestfs_h *g, const char *capabilities_xml,
struct backend_libvirt_data *data);
static xmlChar *construct_libvirt_xml (guestfs_h *g, const struct
libvirt_xml_params *params);
static void debug_appliance_permissions (guestfs_h *g);
static void debug_socket_permissions (guestfs_h *g);
@@ -299,7 +299,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
if (g->verbose)
guestfs___print_timestamped_message (g, "parsing capabilities
XML");
- if (parse_capabilities (g, capabilities_xml, ¶ms) == -1)
+ if (parse_capabilities (g, capabilities_xml, data) == -1)
goto cleanup;
/* Locate and/or build the appliance. */
@@ -575,7 +575,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
static int
parse_capabilities (guestfs_h *g, const char *capabilities_xml,
- struct libvirt_xml_params *params)
+ struct backend_libvirt_data *data)
{
CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL;
CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL;
@@ -654,9 +654,9 @@ parse_capabilities (guestfs_h *g, const char
*capabilities_xml,
force_tcg = guestfs___get_backend_setting_bool (g, "force_tcg");
if (!force_tcg)
- params->is_kvm = seen_kvm;
+ data->is_kvm = seen_kvm;
else
- params->is_kvm = 0;
+ data->is_kvm = 0;
return 0;
}
@@ -929,7 +929,7 @@ construct_libvirt_xml_domain (guestfs_h *g,
xmlTextWriterPtr xo)
{
start_element ("domain") {
- attribute ("type", params->is_kvm ? "kvm" :
"qemu");
+ attribute ("type", params->data->is_kvm ? "kvm" :
"qemu");
attribute_ns ("xmlns", "qemu", NULL,
"http://libvirt.org/schemas/domain/qemu/1.0");
@@ -987,7 +987,7 @@ construct_libvirt_xml_cpu (guestfs_h *g,
* Only do this with KVM. It is broken in subtle ways on TCG, and
* fairly pointless anyway.
*/
- if (params->is_kvm) {
+ if (params->data->is_kvm) {
start_element ("cpu") {
attribute ("mode", "host-passthrough");
start_element ("model") {
@@ -1039,7 +1039,7 @@ construct_libvirt_xml_boot (guestfs_h *g,
/* Linux kernel command line. */
flags = 0;
- if (!params->is_kvm)
+ if (!params->data->is_kvm)
flags |= APPLIANCE_COMMAND_LINE_IS_TCG;
cmdline = guestfs___appliance_command_line (g, params->appliance_dev,
flags);
--
1.8.5.3
Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 03/18] 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 | 107 +++++++++++++++++++++++++++++---------------
src/guestfs-internal.h | 9 ++++
src/launch-direct.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++-
src/launch-libvirt.c | 94 ++++++++++++++++++++++++++++++++++++---
src/launch-uml.c | 6 +++
6 files changed, 321 insertions(+), 44 deletions(-)
diff --git a/generator/actions.ml b/generator/actions.ml
index a6ef194..b25194c 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -1286,7 +1286,7 @@ not all belong to a single logical operating system
{ defaults with
name = "add_drive";
- style = RErr, [String "filename"], [OBool "readonly";
OString "format"; OString "iface"; OString "name";
OString "label"; OString "protocol"; OStringList
"server"; OString "username"; OString "secret";
OString "cachemode"];
+ style = RErr, [String "filename"], [OBool "readonly";
OString "format"; OString "iface"; OString "name";
OString "label"; OString "protocol"; OStringList
"server"; OString "username"; OString "secret";
OString "cachemode"; OString "discard"];
once_had_no_optargs = true;
blocking = false;
fish_alias = ["add"];
@@ -1504,6 +1504,36 @@ for scratch or temporary disks.
=back
+=item C<discard>
+
+Enable or disable discard (a.k.a. trim or unmap) support on this
+drive. If enabled, operations such as C<guestfs_fstrim> will be able
+to discard / make thin / punch holes in the underlying host file
+or device.
+
+Possible discard settings are:
+
+=over 4
+
+=item C<discard = \"disable\">
+
+Disable discard support. This is the default.
+
+=item C<discard = \"enable\">
+
+Enable discard support. Fail if discard is not possible.
+
+=item C<discard = \"besteffort\">
+
+Enable discard support if possible, but don't fail if it is not
+supported.
+
+Since not all backends and not all underlying systems support
+discard, this is a good choice if you want to use discard if
+possible, but don't mind if it doesn't work.
+
+=back
+
=back" };
{ defaults with
diff --git a/src/drives.c b/src/drives.c
index 2c85b52..68e37f7 100644
--- a/src/drives.c
+++ b/src/drives.c
@@ -115,7 +115,8 @@ static struct drive *
create_drive_file (guestfs_h *g, const char *path,
bool readonly, const char *format,
const char *iface, const char *name,
- const char *disk_label, const char *cachemode)
+ const char *disk_label, const char *cachemode,
+ enum discard discard)
{
struct drive *drv = safe_calloc (g, 1, sizeof *drv);
@@ -128,6 +129,7 @@ create_drive_file (guestfs_h *g, const char *path,
drv->name = name ? safe_strdup (g, name) : NULL;
drv->disk_label = disk_label ? safe_strdup (g, disk_label) : NULL;
drv->cachemode = cachemode ? safe_strdup (g, cachemode) : NULL;
+ drv->discard = discard;
if (readonly) {
if (create_overlay (g, drv) == -1) {
@@ -151,7 +153,8 @@ create_drive_non_file (guestfs_h *g,
const char *username, const char *secret,
bool readonly, const char *format,
const char *iface, const char *name,
- const char *disk_label, const char *cachemode)
+ const char *disk_label, const char *cachemode,
+ enum discard discard)
{
struct drive *drv = safe_calloc (g, 1, sizeof *drv);
@@ -168,6 +171,7 @@ create_drive_non_file (guestfs_h *g,
drv->name = name ? safe_strdup (g, name) : NULL;
drv->disk_label = disk_label ? safe_strdup (g, disk_label) : NULL;
drv->cachemode = cachemode ? safe_strdup (g, cachemode) : NULL;
+ drv->discard = discard;
if (readonly) {
if (create_overlay (g, drv) == -1) {
@@ -191,7 +195,8 @@ create_drive_curl (guestfs_h *g,
const char *username, const char *secret,
bool readonly, const char *format,
const char *iface, const char *name,
- const char *disk_label, const char *cachemode)
+ const char *disk_label, const char *cachemode,
+ enum discard discard)
{
if (secret != NULL) {
error (g, _("curl: you cannot specify a secret with this
protocol"));
@@ -223,7 +228,7 @@ create_drive_curl (guestfs_h *g,
servers, nr_servers, exportname,
username, secret,
readonly, format, iface, name, disk_label,
- cachemode);
+ cachemode, discard);
}
static struct drive *
@@ -233,7 +238,8 @@ create_drive_gluster (guestfs_h *g,
const char *username, const char *secret,
bool readonly, const char *format,
const char *iface, const char *name,
- const char *disk_label, const char *cachemode)
+ const char *disk_label, const char *cachemode,
+ enum discard discard)
{
if (username != NULL) {
error (g, _("gluster: you cannot specify a username with this
protocol"));
@@ -263,7 +269,7 @@ create_drive_gluster (guestfs_h *g,
servers, nr_servers, exportname,
username, secret,
readonly, format, iface, name, disk_label,
- cachemode);
+ cachemode, discard);
}
static int
@@ -285,7 +291,8 @@ create_drive_nbd (guestfs_h *g,
const char *username, const char *secret,
bool readonly, const char *format,
const char *iface, const char *name,
- const char *disk_label, const char *cachemode)
+ const char *disk_label, const char *cachemode,
+ enum discard discard)
{
if (username != NULL) {
error (g, _("nbd: you cannot specify a username with this
protocol"));
@@ -308,7 +315,7 @@ create_drive_nbd (guestfs_h *g,
servers, nr_servers, exportname,
username, secret,
readonly, format, iface, name, disk_label,
- cachemode);
+ cachemode, discard);
}
static struct drive *
@@ -318,7 +325,8 @@ create_drive_rbd (guestfs_h *g,
const char *username, const char *secret,
bool readonly, const char *format,
const char *iface, const char *name,
- const char *disk_label, const char *cachemode)
+ const char *disk_label, const char *cachemode,
+ enum discard discard)
{
size_t i;
@@ -348,7 +356,7 @@ create_drive_rbd (guestfs_h *g,
servers, nr_servers, exportname,
username, secret,
readonly, format, iface, name, disk_label,
- cachemode);
+ cachemode, discard);
}
static struct drive *
@@ -358,7 +366,8 @@ create_drive_sheepdog (guestfs_h *g,
const char *username, const char *secret,
bool readonly, const char *format,
const char *iface, const char *name,
- const char *disk_label, const char *cachemode)
+ const char *disk_label, const char *cachemode,
+ enum discard discard)
{
size_t i;
@@ -397,7 +406,7 @@ create_drive_sheepdog (guestfs_h *g,
servers, nr_servers, exportname,
username, secret,
readonly, format, iface, name, disk_label,
- cachemode);
+ cachemode, discard);
}
static struct drive *
@@ -407,7 +416,8 @@ create_drive_ssh (guestfs_h *g,
const char *username, const char *secret,
bool readonly, const char *format,
const char *iface, const char *name,
- const char *disk_label, const char *cachemode)
+ const char *disk_label, const char *cachemode,
+ enum discard discard)
{
if (secret != NULL) {
error (g, _("ssh: you cannot specify a secret with this
protocol"));
@@ -444,7 +454,7 @@ create_drive_ssh (guestfs_h *g,
servers, nr_servers, exportname,
username, secret,
readonly, format, iface, name, disk_label,
- cachemode);
+ cachemode, discard);
}
static struct drive *
@@ -454,7 +464,8 @@ create_drive_iscsi (guestfs_h *g,
const char *username, const char *secret,
bool readonly, const char *format,
const char *iface, const char *name,
- const char *disk_label, const char *cachemode)
+ const char *disk_label, const char *cachemode,
+ enum discard discard)
{
if (username != NULL) {
error (g, _("iscsi: you cannot specify a username with this
protocol"));
@@ -491,7 +502,7 @@ create_drive_iscsi (guestfs_h *g,
servers, nr_servers, exportname,
username, secret,
readonly, format, iface, name, disk_label,
- cachemode);
+ cachemode, discard);
}
/* Traditionally you have been able to use /dev/null as a filename, as
@@ -528,14 +539,15 @@ create_drive_dev_null (guestfs_h *g, bool readonly, const
char *format,
return NULL;
return create_drive_file (g, tmpfile, readonly, format, iface, name,
- disk_label, 0);
+ disk_label, NULL, discard_disable);
}
static struct drive *
create_drive_dummy (guestfs_h *g)
{
/* A special drive struct that is used as a dummy slot for the appliance. */
- return create_drive_file (g, "", 0, NULL, NULL, NULL, NULL, 0);
+ return create_drive_file (g, "", 0, NULL, NULL, NULL, NULL, NULL,
+ discard_disable);
}
static void
@@ -563,8 +575,8 @@ free_drive_struct (struct drive *drv)
free (drv);
}
-static const char *
-protocol_to_string (enum drive_protocol protocol)
+const char *
+guestfs___drive_protocol_to_string (enum drive_protocol protocol)
{
switch (protocol) {
case drive_protocol_file: return "file";
@@ -590,12 +602,12 @@ static char *
drive_to_string (guestfs_h *g, const struct drive *drv)
{
return safe_asprintf
- (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s",
+ (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s%s",
drv->src.u.path,
drv->readonly ? " readonly" : "",
drv->src.format ? " format=" : "",
drv->src.format ? : "",
- protocol_to_string (drv->src.protocol),
+ guestfs___drive_protocol_to_string (drv->src.protocol),
drv->iface ? " iface=" : "",
drv->iface ? : "",
drv->name ? " name=" : "",
@@ -603,7 +615,9 @@ drive_to_string (guestfs_h *g, const struct drive *drv)
drv->disk_label ? " label=" : "",
drv->disk_label ? : "",
drv->cachemode ? " cache=" : "",
- drv->cachemode ? : "");
+ drv->cachemode ? : "",
+ drv->discard == discard_disable ? "" :
+ drv->discard == discard_enable ? " discard=enable" : "
discard=besteffort");
}
/* Add struct drive to the g->drives vector at the given index. */
@@ -824,6 +838,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
const char *username;
const char *secret;
const char *cachemode;
+ enum discard discard;
struct drive *drv;
size_t i, drv_index;
@@ -852,6 +867,28 @@ guestfs__add_drive_opts (guestfs_h *g, const char
*filename,
cachemode = optargs->bitmask &
GUESTFS_ADD_DRIVE_OPTS_CACHEMODE_BITMASK
? optargs->cachemode : NULL;
+ if (optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK) {
+ if (STREQ (optargs->discard, "disable"))
+ discard = discard_disable;
+ else if (STREQ (optargs->discard, "enable"))
+ discard = discard_enable;
+ else if (STREQ (optargs->discard, "besteffort"))
+ discard = discard_besteffort;
+ else {
+ error (g, _("discard parameter must be 'disable',
'enable' or 'besteffort'"));
+ free_drive_servers (servers, nr_servers);
+ return -1;
+ }
+ }
+ else
+ discard = discard_disable;
+
+ if (readonly && discard == discard_enable) {
+ error (g, _("discard support cannot be enabled on read-only
drives"));
+ free_drive_servers (servers, nr_servers);
+ return -1;
+ }
+
if (format && !valid_format_iface (format)) {
error (g, _("%s parameter is empty or contains disallowed
characters"),
"format");
@@ -904,7 +941,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
}
drv = create_drive_file (g, filename, readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
}
else if (STREQ (protocol, "ftp")) {
@@ -912,71 +949,71 @@ guestfs__add_drive_opts (guestfs_h *g, const char
*filename,
servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else if (STREQ (protocol, "ftps")) {
drv = create_drive_curl (g, drive_protocol_ftps,
servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else if (STREQ (protocol, "gluster")) {
drv = create_drive_gluster (g, servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else if (STREQ (protocol, "http")) {
drv = create_drive_curl (g, drive_protocol_http,
servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else if (STREQ (protocol, "https")) {
drv = create_drive_curl (g, drive_protocol_https,
servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else if (STREQ (protocol, "iscsi")) {
drv = create_drive_iscsi (g, servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else if (STREQ (protocol, "nbd")) {
drv = create_drive_nbd (g, servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else if (STREQ (protocol, "rbd")) {
drv = create_drive_rbd (g, servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else if (STREQ (protocol, "sheepdog")) {
drv = create_drive_sheepdog (g, servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else if (STREQ (protocol, "ssh")) {
drv = create_drive_ssh (g, servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else if (STREQ (protocol, "tftp")) {
drv = create_drive_curl (g, drive_protocol_tftp,
servers, nr_servers, filename,
username, secret,
readonly, format, iface, name,
- disk_label, cachemode);
+ disk_label, cachemode, discard);
}
else {
error (g, _("unknown protocol '%s'"), protocol);
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 545146f..0f3a928 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -231,6 +231,12 @@ struct drive_source {
char *secret;
};
+enum discard {
+ discard_disable = 0,
+ discard_enable = 1,
+ discard_besteffort = 2,
+};
+
/* There is one 'struct drive' per drive, including hot-plugged drives.
*/
struct drive {
/* Original source of the drive, eg. file:..., http:... */
@@ -252,6 +258,7 @@ struct drive {
char *name;
char *disk_label;
char *cachemode;
+ enum discard discard;
};
/* Extra hv parameters (from guestfs_config). */
@@ -713,6 +720,7 @@ extern size_t guestfs___checkpoint_drives (guestfs_h *g);
extern void guestfs___rollback_drives (guestfs_h *g, size_t);
extern void guestfs___add_dummy_appliance_drive (guestfs_h *g);
extern void guestfs___free_drives (guestfs_h *g);
+extern const char *guestfs___drive_protocol_to_string (enum drive_protocol
protocol);
/* appliance.c */
extern int guestfs___build_appliance (guestfs_h *g, char **kernel, char **dtb,
char **initrd, char **appliance);
@@ -832,6 +840,7 @@ extern void guestfs___cleanup_cmd_close (struct command **);
/* launch-direct.c */
extern char *guestfs___drive_source_qemu_param (guestfs_h *g, const struct
drive_source *src);
+extern bool guestfs___discard_possible (guestfs_h *g, struct drive *drv,
unsigned long qemu_version);
/* utils.c */
extern int guestfs___validate_guid (const char *);
diff --git a/src/launch-direct.c b/src/launch-direct.c
index 1e84061..3d478c7 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -509,6 +509,30 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
CLEANUP_FREE char *file = NULL, *escaped_file = NULL, *param = NULL;
if (!drv->overlay) {
+ const char *discard_mode = "";
+ int major = data->qemu_version_major, minor =
data->qemu_version_minor;
+ unsigned long qemu_version = major * 1000000 + minor * 1000;
+
+ switch (drv->discard) {
+ case discard_disable:
+ /* Since the default is always discard=ignore, don't specify it
+ * on the command line. This also avoids unnecessary breakage
+ * with qemu < 1.5 which didn't have the option at all.
+ */
+ break;
+ case discard_enable:
+ if (!guestfs___discard_possible (g, drv, qemu_version))
+ goto cleanup0;
+ /*FALLTHROUGH*/
+ case discard_besteffort:
+ /* I believe from reading the code that this is always safe as
+ * long as qemu >= 1.5.
+ */
+ if (major > 1 || (major == 1 && minor >= 5))
+ discard_mode = ",discard=unmap";
+ break;
+ }
+
/* Make the file= parameter. */
file = guestfs___drive_source_qemu_param (g, &drv->src);
escaped_file = qemu_escape_param (g, file);
@@ -517,10 +541,11 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
* the if=... at the end.
*/
param = safe_asprintf
- (g, "file=%s%s,cache=%s%s%s%s%s,id=hd%zu",
+ (g, "file=%s%s,cache=%s%s%s%s%s%s,id=hd%zu",
escaped_file,
drv->readonly ? ",snapshot=on" : "",
drv->cachemode ? drv->cachemode : "writeback",
+ discard_mode,
drv->src.format ? ",format=" : "",
drv->src.format ? drv->src.format : "",
drv->disk_label ? ",serial=" : "",
@@ -1370,6 +1395,96 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const
struct drive_source *src)
abort ();
}
+/* Test if discard is both supported by qemu AND possible with the
+ * underlying file or device. This returns 1 if discard is possible.
+ * It returns 0 if not possible and sets the error to the reason why.
+ *
+ * This function is called when the user set discard == "enable".
+ *
+ * qemu_version is the version of qemu in the form returned by libvirt:
+ * major * 1,000,000 + minor * 1,000 + release
+ */
+bool
+guestfs___discard_possible (guestfs_h *g, struct drive *drv,
+ unsigned long qemu_version)
+{
+ /* qemu >= 1.5. This was the first version that supported the
+ * discard option on -drive at all.
+ */
+ bool qemu15 = qemu_version >= 1005000;
+ /* qemu >= 1.6. This was the first version that supported unmap on
+ * qcow2 backing files.
+ */
+ bool qemu16 = qemu_version >= 1006000;
+
+ if (!qemu15) {
+ error (g, _("discard cannot be enabled on this drive: "
+ "qemu < 1.5"));
+ return false;
+ }
+
+ /* If it's an overlay, discard is not possible (on the underlying
+ * file). This has probably been caught earlier since we already
+ * checked that the drive is !readonly. Nevertheless ...
+ */
+ if (drv->overlay) {
+ error (g, _("discard cannot be enabled on this drive: "
+ "the drive has a read-only overlay"));
+ return false;
+ }
+
+ /* Look at the source format. */
+ if (drv->src.format == NULL) {
+ /* We could autodetect the format, but we don't ... yet. XXX */
+ error (g, _("discard cannot be enabled on this drive: "
+ "you have to specify the format of the file"));
+ return false;
+ }
+ else if (STREQ (drv->src.format, "raw"))
+ /* OK */ ;
+ else if (STREQ (drv->src.format, "qcow2")) {
+ if (!qemu16) {
+ error (g, _("discard cannot be enabled on this drive: "
+ "qemu < 1.6 cannot do discard on qcow2 files"));
+ return false;
+ }
+ }
+ else {
+ /* It's possible in future other formats will support discard, but
+ * currently (qemu 1.7) none of them do.
+ */
+ error (g, _("discard cannot be enabled on this drive: "
+ "qemu does not support discard for '%s' format
files"),
+ drv->src.format);
+ return false;
+ }
+
+ switch (drv->src.protocol) {
+ /* Protocols which support discard. */
+ case drive_protocol_file:
+ case drive_protocol_gluster:
+ case drive_protocol_iscsi:
+ case drive_protocol_nbd:
+ case drive_protocol_rbd:
+ case drive_protocol_sheepdog: /* XXX depends on server version */
+ break;
+
+ /* Protocols which don't support discard. */
+ case drive_protocol_ftp:
+ case drive_protocol_ftps:
+ case drive_protocol_http:
+ case drive_protocol_https:
+ case drive_protocol_ssh:
+ case drive_protocol_tftp:
+ error (g, _("discard cannot be enabled on this drive: "
+ "protocol '%s' does not support discard"),
+ guestfs___drive_protocol_to_string (drv->src.protocol));
+ return false;
+ }
+
+ return true;
+}
+
static int
shutdown_direct (guestfs_h *g, void *datav, int check_for_errors)
{
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index da1d665..1f5a79a 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -107,6 +107,7 @@ struct backend_libvirt_data {
bool selinux_norelabel_disks;
char name[DOMAIN_NAME_LEN]; /* random name */
bool is_kvm; /* false = qemu, true = kvm (from
capabilities)*/
+ unsigned long qemu_version; /* qemu version (from libvirt) */
};
/* Parameters passed to construct_libvirt_xml and subfunctions. We
@@ -131,6 +132,7 @@ static xmlChar *construct_libvirt_xml (guestfs_h *g, const
struct libvirt_xml_pa
static void debug_appliance_permissions (guestfs_h *g);
static void debug_socket_permissions (guestfs_h *g);
static void libvirt_error (guestfs_h *g, const char *fs, ...)
__attribute__((format (printf,2,3)));
+static void libvirt_debug (guestfs_h *g, const char *fs, ...)
__attribute__((format (printf,2,3)));
static int is_custom_hv (guestfs_h *g);
static int is_blk (const char *path);
static void ignore_errors (void *ignore, virErrorPtr ignore2);
@@ -283,6 +285,19 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
*/
virConnSetErrorFunc (conn, NULL, ignore_errors);
+ /* Get hypervisor (hopefully qemu) version. */
+ if (virConnectGetVersion (conn, &data->qemu_version) == 0) {
+ debug (g, "qemu version (reported by libvirt) = %lu
(%lu.%lu.%lu)",
+ data->qemu_version,
+ data->qemu_version / 1000000UL,
+ data->qemu_version / 1000UL % 1000UL,
+ data->qemu_version % 1000UL);
+ }
+ else {
+ libvirt_debug (g, "unable to read qemu version from libvirt");
+ data->qemu_version = 0;
+ }
+
if (g->verbose)
guestfs___print_timestamped_message (g, "get libvirt
capabilities");
@@ -789,7 +804,7 @@ static int construct_libvirt_xml_devices (guestfs_h *g,
const struct libvirt_xml
static int construct_libvirt_xml_qemu_cmdline (guestfs_h *g, const struct
libvirt_xml_params *params, xmlTextWriterPtr xo);
static int construct_libvirt_xml_disk (guestfs_h *g, const struct
backend_libvirt_data *data, xmlTextWriterPtr xo, struct drive *drv, size_t
drv_index);
static int construct_libvirt_xml_disk_target (guestfs_h *g, xmlTextWriterPtr
xo, size_t drv_index);
-static int construct_libvirt_xml_disk_driver_qemu (guestfs_h *g,
xmlTextWriterPtr xo, const char *format, const char *cachemode);
+static int construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, const struct
backend_libvirt_data *data, struct drive *drv, xmlTextWriterPtr xo, const char
*format, const char *cachemode, enum discard discard);
static int construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr
xo, size_t drv_index);
static int construct_libvirt_xml_disk_source_hosts (guestfs_h *g,
xmlTextWriterPtr xo, const struct drive_source *src);
static int construct_libvirt_xml_disk_source_seclabel (guestfs_h *g, const
struct backend_libvirt_data *data, xmlTextWriterPtr xo);
@@ -1233,7 +1248,9 @@ construct_libvirt_xml_disk (guestfs_h *g,
if (construct_libvirt_xml_disk_target (g, xo, drv_index) == -1)
return -1;
- if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2",
"unsafe")
+ if (construct_libvirt_xml_disk_driver_qemu (g, NULL, NULL,
+ xo, "qcow2",
"unsafe",
+ discard_disable)
== -1)
return -1;
}
@@ -1370,8 +1387,9 @@ construct_libvirt_xml_disk (guestfs_h *g,
return -1;
}
- if (construct_libvirt_xml_disk_driver_qemu (g, xo, format,
- drv->cachemode ? :
"writeback")
+ if (construct_libvirt_xml_disk_driver_qemu (g, data, drv, xo, format,
+ drv->cachemode ? :
"writeback",
+ drv->discard)
== -1)
return -1;
}
@@ -1407,14 +1425,41 @@ construct_libvirt_xml_disk_target (guestfs_h *g,
xmlTextWriterPtr xo,
}
static int
-construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, xmlTextWriterPtr xo,
+construct_libvirt_xml_disk_driver_qemu (guestfs_h *g,
+ const struct backend_libvirt_data
*data,
+ struct drive *drv,
+ xmlTextWriterPtr xo,
const char *format,
- const char *cachemode)
+ const char *cachemode,
+ enum discard discard)
{
+ bool discard_unmap = false;
+
+ switch (discard) {
+ case discard_disable:
+ /* Since the default is always discard=ignore, don't specify it
+ * in the XML.
+ */
+ break;
+ case discard_enable:
+ if (!guestfs___discard_possible (g, drv, data->qemu_version))
+ return -1;
+ /*FALLTHROUGH*/
+ case discard_besteffort:
+ /* I believe from reading the code that this is always safe as
+ * long as qemu >= 1.5.
+ */
+ if (data->qemu_version >= 1005000)
+ discard_unmap = true;
+ break;
+ }
+
start_element ("driver") {
attribute ("name", "qemu");
attribute ("type", format);
attribute ("cache", cachemode);
+ if (discard_unmap)
+ attribute ("discard", "unmap");
} end_element ();
return 0;
@@ -1499,7 +1544,9 @@ construct_libvirt_xml_appliance (guestfs_h *g,
attribute ("bus", "scsi");
} end_element ();
- if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2",
"unsafe") == -1)
+ if (construct_libvirt_xml_disk_driver_qemu (g, NULL, NULL, xo,
+ "qcow2",
"unsafe",
+ discard_disable) == -1)
return -1;
if (construct_libvirt_xml_disk_address (g, xo, params->appliance_index)
@@ -1659,6 +1706,39 @@ libvirt_error (guestfs_h *g, const char *fs, ...)
free (msg);
}
+/* Same as 'libvirt_error' but calls debug instead. */
+static void
+libvirt_debug (guestfs_h *g, const char *fs, ...)
+{
+ va_list args;
+ char *msg;
+ int len;
+ virErrorPtr err;
+
+ if (!g->verbose)
+ return;
+
+ va_start (args, fs);
+ len = vasprintf (&msg, fs, args);
+ va_end (args);
+
+ if (len < 0)
+ msg = safe_asprintf (g,
+ _("%s: internal error forming error
message"),
+ __func__);
+
+ /* In all recent libvirt, this retrieves the thread-local error. */
+ err = virGetLastError ();
+ if (err)
+ debug (g, "%s: %s [code=%d domain=%d]",
+ msg, err->message, err->code, err->domain);
+ else
+ debug (g, "%s", msg);
+
+ /* NB. 'err' must not be freed! */
+ free (msg);
+}
+
#if HAVE_LIBSELINUX
static void
selinux_warning (guestfs_h *g, const char *func,
diff --git a/src/launch-uml.c b/src/launch-uml.c
index deda366..2a6ddaf 100644
--- a/src/launch-uml.c
+++ b/src/launch-uml.c
@@ -122,6 +122,12 @@ uml_supported (guestfs_h *g)
_("uml backend does not support drives with 'label'
parameter"));
return false;
}
+ /* Note that discard == "besteffort" is fine. */
+ if (drv->discard == discard_enable) {
+ error (g,
+ _("uml backend does not support drives with 'discard'
parameter set to 'enable'"));
+ return false;
+ }
}
return true;
--
1.8.5.3
Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 04/18] 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 5b2ec57..7b2ab8a 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -15,6 +15,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-11 23:13 UTC
[Libguestfs] [PATCH v2 05/18] 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-11 23:13 UTC
[Libguestfs] [PATCH v2 06/18] tests: Add tests of discard support.
Test that both fstrim and blkdiscard work in reality, end-to-end.
---
Makefile.am | 1 +
configure.ac | 1 +
tests/discard/Makefile.am | 27 +++++++++
tests/discard/test-blkdiscard.pl | 111 +++++++++++++++++++++++++++++++++++
tests/discard/test-fstrim.pl | 124 +++++++++++++++++++++++++++++++++++++++
5 files changed, 264 insertions(+)
create mode 100644 tests/discard/Makefile.am
create mode 100755 tests/discard/test-blkdiscard.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..c1ff68b
--- /dev/null
+++ b/tests/discard/Makefile.am
@@ -0,0 +1,27 @@
+# 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-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..22602b8
--- /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 "surprising result: full size <= original size"
+ 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 "looks like the blkdiscard operation did not work"
+ 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..dea5203
--- /dev/null
+++ b/tests/discard/test-fstrim.pl
@@ -0,0 +1,124 @@
+#!/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");
+$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 "surprising result: full size <= original size"
+ 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 "looks like the fstrim operation did not work"
+ if $trimmed_size >= $full_size;
--
1.8.5.3
Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 07/18] 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-11 23:13 UTC
[Libguestfs] [PATCH v2 08/18] 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-11 23:13 UTC
[Libguestfs] [PATCH v2 09/18] 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-11 23:13 UTC
[Libguestfs] [PATCH v2 10/18] 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-11 23:13 UTC
[Libguestfs] [PATCH v2 11/18] sparsify: Remove unused variable definition.
--- sparsify/sparsify.ml | 3 --- 1 file changed, 3 deletions(-) diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml index 212a23f..b124406 100644 --- a/sparsify/sparsify.ml +++ b/sparsify/sparsify.ml @@ -296,9 +296,6 @@ let () in Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint) -(* Get the size in bytes of the input disk. *) -let insize = g#blockdev_getsize64 "/dev/sda" - (* Write zeroes for non-ignored filesystems that we are able to mount, * and selected swap partitions. *) -- 1.8.5.3
Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 12/18] sparsify: Capture any exceptions and display nicer error messages.
This is just code motion, there is no functional change.
---
sparsify/sparsify.ml | 418 ++++++++++++++++++++++++++-------------------------
1 file changed, 217 insertions(+), 201 deletions(-)
diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml
index b124406..e79fe78 100644
--- a/sparsify/sparsify.ml
+++ b/sparsify/sparsify.ml
@@ -31,68 +31,69 @@ external statvfs_free_space : string -> int64
let () = Random.self_init ()
-(* Command line argument parsing. *)
let prog = Filename.basename Sys.executable_name
let error fs = error ~prog fs
-let indisk, outdisk, check_tmpdir, compress, convert, debug_gc,
- format, ignores, machine_readable,
- option, quiet, verbose, trace, zeroes - let display_version () - printf
"virt-sparsify %s\n" Config.package_version;
- exit 0
- in
+let main () + (* Command line argument parsing. *)
+ let indisk, outdisk, check_tmpdir, compress, convert, debug_gc,
+ format, ignores, machine_readable,
+ option, quiet, verbose, trace, zeroes + let display_version () +
printf "virt-sparsify %s\n" Config.package_version;
+ exit 0
+ in
- let add xs s = xs := s :: !xs in
+ let add xs s = xs := s :: !xs in
- let check_tmpdir = ref `Warn in
- let set_check_tmpdir = function
- | "ignore" | "i" -> check_tmpdir := `Ignore
- | "continue" | "cont" | "c" ->
check_tmpdir := `Continue
- | "warn" | "warning" | "w" -> check_tmpdir
:= `Warn
- | "fail" | "f" | "error" -> check_tmpdir
:= `Fail
- | str ->
- eprintf (f_"--check-tmpdir: unknown argument `%s'\n") str;
- exit 1
- in
+ let check_tmpdir = ref `Warn in
+ let set_check_tmpdir = function
+ | "ignore" | "i" -> check_tmpdir := `Ignore
+ | "continue" | "cont" | "c" ->
check_tmpdir := `Continue
+ | "warn" | "warning" | "w" ->
check_tmpdir := `Warn
+ | "fail" | "f" | "error" -> check_tmpdir
:= `Fail
+ | str ->
+ eprintf (f_"--check-tmpdir: unknown argument `%s'\n")
str;
+ exit 1
+ in
- let compress = ref false in
- let convert = ref "" in
- let debug_gc = ref false in
- let format = ref "" in
- let ignores = ref [] in
- let machine_readable = ref false in
- let option = ref "" in
- let quiet = ref false in
- let verbose = ref false in
- let trace = ref false in
- let zeroes = ref [] in
+ let compress = ref false in
+ let convert = ref "" in
+ let debug_gc = ref false in
+ let format = ref "" in
+ let ignores = ref [] in
+ let machine_readable = ref false in
+ let option = ref "" in
+ let quiet = ref false in
+ let verbose = ref false in
+ let trace = ref false in
+ let zeroes = ref [] in
- let ditto = " -\"-" in
- let argspec = Arg.align [
- "--check-tmpdir", Arg.String set_check_tmpdir,
"ignore|..." ^ " " ^ s_"Check there is enough space in
$TMPDIR";
- "--compress", Arg.Set compress, " " ^
s_"Compressed output format";
- "--convert", Arg.Set_string convert, s_"format" ^
" " ^ s_"Format of output disk (default: same as input)";
- "--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";
- "--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";
- "-q", Arg.Set quiet, " " ^
s_"Quiet output";
- "--quiet", Arg.Set quiet, ditto;
- "-v", Arg.Set verbose, " " ^
s_"Enable debugging messages";
- "--verbose", Arg.Set verbose, ditto;
- "-V", Arg.Unit display_version, " " ^
s_"Display version and exit";
- "--version", Arg.Unit display_version, ditto;
- "-x", Arg.Set trace, " " ^
s_"Enable tracing of libguestfs calls";
- "--zero", Arg.String (add zeroes), s_"fs" ^ "
" ^ s_"Zero filesystem";
- ] in
- long_options := argspec;
- let disks = ref [] in
- let anon_fun s = disks := s :: !disks in
- let usage_msg - sprintf (f_"\
+ let ditto = " -\"-" in
+ let argspec = Arg.align [
+ "--check-tmpdir", Arg.String set_check_tmpdir,
"ignore|..." ^ " " ^ s_"Check there is enough space in
$TMPDIR";
+ "--compress", Arg.Set compress, " " ^
s_"Compressed output format";
+ "--convert", Arg.Set_string convert, s_"format" ^
" " ^ s_"Format of output disk (default: same as input)";
+ "--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";
+ "--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";
+ "-q", Arg.Set quiet, " " ^
s_"Quiet output";
+ "--quiet", Arg.Set quiet, ditto;
+ "-v", Arg.Set verbose, " " ^
s_"Enable debugging messages";
+ "--verbose", Arg.Set verbose, ditto;
+ "-V", Arg.Unit display_version, " " ^
s_"Display version and exit";
+ "--version", Arg.Unit display_version, ditto;
+ "-x", Arg.Set trace, " " ^
s_"Enable tracing of libguestfs calls";
+ "--zero", Arg.String (add zeroes), s_"fs" ^
" " ^ s_"Zero filesystem";
+ ] in
+ long_options := argspec;
+ let disks = ref [] in
+ let anon_fun s = disks := s :: !disks in
+ let usage_msg + sprintf (f_"\
%s: sparsify a virtual machine disk
virt-sparsify [--options] indisk outdisk
@@ -100,118 +101,114 @@ let indisk, outdisk, check_tmpdir, compress, convert,
debug_gc,
A short summary of the options is given below. For detailed help please
read the man page virt-sparsify(1).
")
- prog in
- Arg.parse argspec anon_fun usage_msg;
+ prog in
+ Arg.parse argspec anon_fun usage_msg;
- (* Dereference the rest of the args. *)
- let check_tmpdir = !check_tmpdir in
- let compress = !compress in
- let convert = match !convert with "" -> None | str -> Some
str in
- let debug_gc = !debug_gc in
- let format = match !format with "" -> None | str -> Some str
in
- let ignores = List.rev !ignores in
- let machine_readable = !machine_readable in
- let option = match !option with "" -> None | str -> Some str
in
- let quiet = !quiet in
- let verbose = !verbose in
- let trace = !trace in
- let zeroes = List.rev !zeroes in
+ (* Dereference the rest of the args. *)
+ let check_tmpdir = !check_tmpdir in
+ let compress = !compress in
+ let convert = match !convert with "" -> None | str -> Some
str in
+ let debug_gc = !debug_gc in
+ let format = match !format with "" -> None | str -> Some
str in
+ let ignores = List.rev !ignores in
+ let machine_readable = !machine_readable in
+ let option = match !option with "" -> None | str -> Some
str in
+ let quiet = !quiet in
+ let verbose = !verbose in
+ let trace = !trace in
+ let zeroes = List.rev !zeroes in
- (* No arguments and machine-readable mode? Print out some facts
- * about what this binary supports.
- *)
- if !disks = [] && machine_readable then (
- printf "virt-sparsify\n";
- printf "linux-swap\n";
- printf "zero\n";
- printf "check-tmpdir\n";
- let g = new G.guestfs () in
- g#add_drive "/dev/null";
- g#launch ();
- if g#feature_available [| "ntfsprogs"; "ntfs3g" |] then
- printf "ntfs\n";
- if g#feature_available [| "btrfs" |] then
- printf "btrfs\n";
- exit 0
- );
+ (* No arguments and machine-readable mode? Print out some facts
+ * about what this binary supports.
+ *)
+ if !disks = [] && machine_readable then (
+ printf "virt-sparsify\n";
+ printf "linux-swap\n";
+ printf "zero\n";
+ printf "check-tmpdir\n";
+ let g = new G.guestfs () in
+ g#add_drive "/dev/null";
+ g#launch ();
+ if g#feature_available [| "ntfsprogs"; "ntfs3g" |]
then
+ printf "ntfs\n";
+ if g#feature_available [| "btrfs" |] then
+ printf "btrfs\n";
+ exit 0
+ );
- (* Verify we got exactly 2 disks. *)
- let indisk, outdisk - match List.rev !disks with
- | [indisk; outdisk] -> indisk, outdisk
- | _ ->
+ (* Verify we got exactly 2 disks. *)
+ let indisk, outdisk + match List.rev !disks with
+ | [indisk; outdisk] -> indisk, outdisk
+ | _ ->
error "usage is: %s [--options] indisk outdisk" prog in
- (* Simple-minded check that the user isn't trying to use the
- * same disk for input and output.
- *)
- if indisk = outdisk then
- error (f_"you cannot use the same disk image for input and
output");
+ (* Simple-minded check that the user isn't trying to use the
+ * same disk for input and output.
+ *)
+ 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
- indisk
- else
- Sys.getcwd () // indisk in
+ (* 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;
+ (* 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;
+ if is_char_device outdisk then
+ error (f_"output '%s' cannot be a character device, it must
be a regular file")
+ outdisk;
- indisk, outdisk, check_tmpdir, compress, convert,
- debug_gc, format, ignores, machine_readable,
- option, quiet, verbose, trace, zeroes
+ indisk, outdisk, check_tmpdir, compress, convert,
+ debug_gc, format, ignores, machine_readable,
+ option, quiet, verbose, trace, zeroes in
-(* Once we have got past argument parsing and start to create
- * temporary files (including the potentially massive overlay file), we
- * need to catch SIGINT (^C) and exit cleanly so the temporary file
- * goes away. Note that we don't delete temporaries in the signal
- * handler.
- *)
-let () + (* Once we have got past argument parsing and start to create
+ * temporary files (including the potentially massive overlay file), we
+ * need to catch SIGINT (^C) and exit cleanly so the temporary file
+ * goes away. Note that we don't delete temporaries in the signal
+ * handler.
+ *)
let do_sigint _ = exit 1 in
- Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint)
+ Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint);
-(* What should the output format be? If the user specified an
- * input format, use that, else detect it from the source image.
- *)
-let output_format - match convert with
- | Some fmt -> fmt (* user specified output conversion *)
- | None ->
- match format with
- | Some fmt -> fmt (* user specified input format, use that *)
+ (* What should the output format be? If the user specified an
+ * input format, use that, else detect it from the source image.
+ *)
+ let output_format + match convert with
+ | Some fmt -> fmt (* user specified output conversion *)
| None ->
- (* Don't know, so we must autodetect. *)
- match (new G.guestfs ())#disk_format indisk with
- | "unknown" ->
- error (f_"cannot detect input disk format; use the --format
parameter")
- | fmt -> fmt
+ match format with
+ | Some fmt -> fmt (* user specified input format, use that *)
+ | None ->
+ (* Don't know, so we must autodetect. *)
+ match (new G.guestfs ())#disk_format indisk with
+ | "unknown" ->
+ error (f_"cannot detect input disk format; use the --format
parameter")
+ | fmt -> fmt in
-(* Compression is not supported by raw output (RHBZ#852194). *)
-let () + (* Compression is not supported by raw output (RHBZ#852194). *)
if output_format = "raw" && compress then
- error (f_"--compress cannot be used for raw output. Remove this
option or use --convert qcow2.")
+ error (f_"--compress cannot be used for raw output. Remove this
option or use --convert qcow2.");
-(* Get virtual size of the input disk. *)
-let virtual_size = (new G.guestfs ())#disk_virtual_size indisk
-let () + (* Get virtual size of the input disk. *)
+ let virtual_size = (new G.guestfs ())#disk_virtual_size indisk in
if not quiet then
printf (f_"Input disk virtual size = %Ld bytes (%s)\n%!")
- virtual_size (human_size virtual_size)
+ virtual_size (human_size virtual_size);
-(* Check there is enough space in $TMPDIR. *)
-let tmpdir = Filename.temp_dir_name
+ (* Check there is enough space in $TMPDIR. *)
+ let tmpdir = Filename.temp_dir_name in
-let () let print_warning () let free_space = statvfs_free_space tmpdir
in
let extra_needed = virtual_size -^ free_space in
@@ -236,7 +233,7 @@ You can ignore this warning or change it to a hard failure
using the
) else false
in
- match check_tmpdir with
+ (match check_tmpdir with
| `Ignore -> ()
| `Continue -> ignore (print_warning ())
| `Warn ->
@@ -249,57 +246,54 @@ You can ignore this warning or change it to a hard failure
using the
eprintf "Exiting because --check-tmpdir=fail was set.\n%!";
exit 2
)
+ );
-let () if not quiet then
- printf (f_"Create overlay file in %s to protect source disk
...\n%!") tmpdir
+ printf (f_"Create overlay file in %s to protect source disk
...\n%!") tmpdir;
-(* Create the temporary overlay file. *)
-let overlaydisk - let tmp = Filename.temp_file "sparsify"
".qcow2" in
- unlink_on_exit tmp;
+ (* Create the temporary overlay file. *)
+ let overlaydisk + let tmp = Filename.temp_file "sparsify"
".qcow2" in
+ unlink_on_exit tmp;
- (* Create it with the indisk as the backing file. *)
- (* XXX Old code used to:
- * - detect if compat=1.1 was supported
- * - add lazy_refcounts option
- *)
- (new G.guestfs ())#disk_create
- ~backingfile:indisk ?backingformat:format ~compat:"1.1"
- tmp "qcow2" Int64.minus_one;
+ (* Create it with the indisk as the backing file. *)
+ (* XXX Old code used to:
+ * - detect if compat=1.1 was supported
+ * - add lazy_refcounts option
+ *)
+ (new G.guestfs ())#disk_create
+ ~backingfile:indisk ?backingformat:format ~compat:"1.1"
+ tmp "qcow2" Int64.minus_one;
- tmp
+ tmp in
-let () if not quiet then
- printf (f_"Examine source disk ...\n%!")
+ printf (f_"Examine source disk ...\n%!");
-(* Connect to libguestfs. *)
-let g - let g = new G.guestfs () in
- if trace then g#set_trace true;
- if verbose then g#set_verbose true;
+ (* Connect to libguestfs. *)
+ let g + let g = new G.guestfs () in
+ if trace then g#set_trace true;
+ if verbose then g#set_verbose true;
- (* Note that the temporary overlay disk is always qcow2 format. *)
- g#add_drive ~format:"qcow2" ~readonly:false
~cachemode:"unsafe" overlaydisk;
+ (* Note that the temporary overlay disk is always qcow2 format. *)
+ g#add_drive ~format:"qcow2" ~readonly:false
~cachemode:"unsafe" overlaydisk;
- if not quiet then Progress.set_up_progress_bar ~machine_readable g;
- g#launch ();
+ if not quiet then Progress.set_up_progress_bar ~machine_readable g;
+ g#launch ();
- g
+ g in
-(* Modify SIGINT handler (set first above) to cancel the handle. *)
-let () + (* Modify SIGINT handler (set first above) to cancel the handle. *)
let do_sigint _ g#user_cancel ();
exit 1
in
- Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint)
+ Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint);
-(* Write zeroes for non-ignored filesystems that we are able to mount,
- * and selected swap partitions.
- *)
-let () + (* Write zeroes for 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
@@ -356,10 +350,9 @@ let ()
g#umount_all ()
)
- ) filesystems
+ ) filesystems;
-(* Fill unused space in volume groups. *)
-let () + (* Fill unused space in volume groups. *)
let vgs = g#vgs () in
let vgs = Array.to_list vgs in
let vgs = List.sort compare vgs in
@@ -382,22 +375,19 @@ let () g#lvremove lvdev
)
)
- ) vgs
+ ) vgs;
-(* Don't need libguestfs now. *)
-let () + (* Don't need libguestfs now. *)
g#shutdown ();
- g#close ()
+ g#close ();
-(* Modify SIGINT handler (set first above) to just exit. *)
-let () + (* Modify SIGINT handler (set first above) to just exit. *)
let do_sigint _ = exit 1 in
- Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint)
+ Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint);
-(* Now run qemu-img convert which copies the overlay to the
- * destination and automatically does sparsification.
- *)
-let () + (* Now run qemu-img convert which copies the overlay to the
+ * destination and automatically does sparsification.
+ *)
if not quiet then
printf (f_"Copy to destination and make sparse ...\n%!");
@@ -412,16 +402,42 @@ let () if verbose then
printf "%s\n%!" cmd;
if Sys.command cmd <> 0 then
- error (f_"external command failed: %s") cmd
+ error (f_"external command failed: %s") cmd;
-(* Finished. *)
-let () + (* Finished. *)
if not quiet then (
print_newline ();
wrap (s_"Sparsify operation completed with no errors. Before deleting
the old disk, carefully check that the target disk boots and works
correctly.\n");
);
if debug_gc then
- Gc.compact ();
+ Gc.compact ()
- exit 0
+let () + try main ()
+ with
+ | Unix.Unix_error (code, fname, "") -> (* from a syscall *)
+ eprintf (f_"%s: error: %s: %s\n") prog fname (Unix.error_message
code);
+ exit 1
+ | Unix.Unix_error (code, fname, param) -> (* from a syscall *)
+ eprintf (f_"%s: error: %s: %s: %s\n") prog fname
(Unix.error_message code)
+ param;
+ exit 1
+ | G.Error msg -> (* from libguestfs *)
+ eprintf (f_"%s: libguestfs error: %s\n") prog msg;
+ exit 1
+ | Failure msg -> (* from failwith/failwithf *)
+ eprintf (f_"%s: failure: %s\n") prog msg;
+ exit 1
+ | Invalid_argument msg -> (* probably should never happen *)
+ eprintf (f_"%s: internal error: invalid argument: %s\n") prog
msg;
+ exit 1
+ | Assert_failure (file, line, char) -> (* should never happen *)
+ eprintf (f_"%s: internal error: assertion failed at %s, line %d, char
%d\n") prog file line char;
+ exit 1
+ | Not_found -> (* should never happen *)
+ eprintf (f_"%s: internal error: Not_found exception was
thrown\n") prog;
+ exit 1
+ | exn -> (* something not matched above *)
+ eprintf (f_"%s: exception: %s\n") prog (Printexc.to_string exn);
+ exit 1
--
1.8.5.3
Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 13/18] sparsify: Remove unused 'open' directive.
--- sparsify/sparsify.ml | 1 - 1 file changed, 1 deletion(-) diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml index e79fe78..80c875c 100644 --- a/sparsify/sparsify.ml +++ b/sparsify/sparsify.ml @@ -17,7 +17,6 @@ *) open Unix -open Scanf open Printf open Common_gettext.Gettext -- 1.8.5.3
Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 14/18] sparsify: Move command line parsing code to separate file.
This is just code motion.
---
po/POTFILES-ml | 1 +
sparsify/Makefile.am | 2 +
sparsify/cmdline.ml | 160 +++++++++++++++++++++++++++++++++++++++++++++++++++
sparsify/sparsify.ml | 137 +------------------------------------------
4 files changed, 165 insertions(+), 135 deletions(-)
create mode 100644 sparsify/cmdline.ml
diff --git a/po/POTFILES-ml b/po/POTFILES-ml
index 32e4206..ca29c25 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -31,6 +31,7 @@ mllib/timezone.ml
mllib/uRI.ml
mllib/urandom.ml
resize/resize.ml
+sparsify/cmdline.ml
sparsify/sparsify.ml
sysprep/main.ml
sysprep/sysprep_operation.ml
diff --git a/sparsify/Makefile.am b/sparsify/Makefile.am
index 516069a..3a6a011 100644
--- a/sparsify/Makefile.am
+++ b/sparsify/Makefile.am
@@ -26,6 +26,7 @@ CLEANFILES = *~ *.cmi *.cmo *.cmx *.cmxa *.o virt-sparsify
# Alphabetical order.
SOURCES = \
+ cmdline.ml \
sparsify.ml \
statvfs-c.c
@@ -42,6 +43,7 @@ deps = \
$(top_builddir)/mllib/progress.cmx \
$(top_builddir)/mllib/config.cmx \
statvfs-c.o \
+ cmdline.cmx \
sparsify.cmx
if HAVE_OCAMLOPT
diff --git a/sparsify/cmdline.ml b/sparsify/cmdline.ml
new file mode 100644
index 0000000..d803ab1
--- /dev/null
+++ b/sparsify/cmdline.ml
@@ -0,0 +1,160 @@
+(* 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.
+ *)
+
+(* Command line argument parsing. *)
+
+open Printf
+
+open Common_gettext.Gettext
+open Common_utils
+
+let prog = Filename.basename Sys.executable_name
+let error fs = error ~prog fs
+
+let parse_cmdline () + let display_version () + printf "virt-sparsify
%s\n" Config.package_version;
+ exit 0
+ in
+
+ let add xs s = xs := s :: !xs in
+
+ let check_tmpdir = ref `Warn in
+ let set_check_tmpdir = function
+ | "ignore" | "i" -> check_tmpdir := `Ignore
+ | "continue" | "cont" | "c" ->
check_tmpdir := `Continue
+ | "warn" | "warning" | "w" -> check_tmpdir
:= `Warn
+ | "fail" | "f" | "error" -> check_tmpdir
:= `Fail
+ | str ->
+ eprintf (f_"--check-tmpdir: unknown argument `%s'\n") str;
+ exit 1
+ in
+
+ let compress = ref false in
+ let convert = ref "" in
+ let debug_gc = ref false in
+ let format = ref "" in
+ let ignores = ref [] in
+ let machine_readable = ref false in
+ let option = ref "" in
+ let quiet = ref false in
+ let verbose = ref false in
+ let trace = ref false in
+ let zeroes = ref [] in
+
+ let ditto = " -\"-" in
+ let argspec = Arg.align [
+ "--check-tmpdir", Arg.String set_check_tmpdir,
"ignore|..." ^ " " ^ s_"Check there is enough space in
$TMPDIR";
+ "--compress", Arg.Set compress, " " ^
s_"Compressed output format";
+ "--convert", Arg.Set_string convert, s_"format" ^
" " ^ s_"Format of output disk (default: same as input)";
+ "--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";
+ "--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";
+ "-q", Arg.Set quiet, " " ^
s_"Quiet output";
+ "--quiet", Arg.Set quiet, ditto;
+ "-v", Arg.Set verbose, " " ^
s_"Enable debugging messages";
+ "--verbose", Arg.Set verbose, ditto;
+ "-V", Arg.Unit display_version, " " ^
s_"Display version and exit";
+ "--version", Arg.Unit display_version, ditto;
+ "-x", Arg.Set trace, " " ^
s_"Enable tracing of libguestfs calls";
+ "--zero", Arg.String (add zeroes), s_"fs" ^ "
" ^ s_"Zero filesystem";
+ ] in
+ long_options := argspec;
+ let disks = ref [] in
+ let anon_fun s = disks := s :: !disks in
+ let usage_msg + sprintf (f_"\
+%s: sparsify a virtual machine disk
+
+ virt-sparsify [--options] indisk outdisk
+
+A short summary of the options is given below. For detailed help please
+read the man page virt-sparsify(1).
+")
+ prog in
+ Arg.parse argspec anon_fun usage_msg;
+
+ (* Dereference the rest of the args. *)
+ let check_tmpdir = !check_tmpdir in
+ let compress = !compress in
+ let convert = match !convert with "" -> None | str -> Some
str in
+ let debug_gc = !debug_gc in
+ let format = match !format with "" -> None | str -> Some str
in
+ let ignores = List.rev !ignores in
+ let machine_readable = !machine_readable in
+ let option = match !option with "" -> None | str -> Some str
in
+ let quiet = !quiet in
+ let verbose = !verbose in
+ let trace = !trace in
+ let zeroes = List.rev !zeroes in
+
+ (* No arguments and machine-readable mode? Print out some facts
+ * about what this binary supports.
+ *)
+ if !disks = [] && machine_readable then (
+ printf "virt-sparsify\n";
+ printf "linux-swap\n";
+ printf "zero\n";
+ printf "check-tmpdir\n";
+ let g = new G.guestfs () in
+ g#add_drive "/dev/null";
+ g#launch ();
+ if g#feature_available [| "ntfsprogs"; "ntfs3g" |] then
+ printf "ntfs\n";
+ if g#feature_available [| "btrfs" |] then
+ printf "btrfs\n";
+ exit 0
+ );
+
+ (* Verify we got exactly 2 disks. *)
+ let indisk, outdisk + match List.rev !disks with
+ | [indisk; outdisk] -> indisk, outdisk
+ | _ ->
+ error "usage is: %s [--options] indisk outdisk" prog in
+
+ (* Simple-minded check that the user isn't trying to use the
+ * same disk for input and output.
+ *)
+ 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
+ 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, outdisk, check_tmpdir, compress, convert,
+ debug_gc, format, ignores, machine_readable,
+ option, quiet, verbose, trace, zeroes
diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml
index 80c875c..c9692d4 100644
--- a/sparsify/sparsify.ml
+++ b/sparsify/sparsify.ml
@@ -24,151 +24,18 @@ open Common_gettext.Gettext
module G = Guestfs
open Common_utils
+open Cmdline
external statvfs_free_space : string -> int64
"virt_sparsify_statvfs_free_space"
let () = Random.self_init ()
-let prog = Filename.basename Sys.executable_name
-let error fs = error ~prog fs
-
let main () - (* Command line argument parsing. *)
let indisk, outdisk, check_tmpdir, compress, convert, debug_gc,
format, ignores, machine_readable,
option, quiet, verbose, trace, zeroes - let display_version () -
printf "virt-sparsify %s\n" Config.package_version;
- exit 0
- in
-
- let add xs s = xs := s :: !xs in
-
- let check_tmpdir = ref `Warn in
- let set_check_tmpdir = function
- | "ignore" | "i" -> check_tmpdir := `Ignore
- | "continue" | "cont" | "c" ->
check_tmpdir := `Continue
- | "warn" | "warning" | "w" ->
check_tmpdir := `Warn
- | "fail" | "f" | "error" -> check_tmpdir
:= `Fail
- | str ->
- eprintf (f_"--check-tmpdir: unknown argument `%s'\n")
str;
- exit 1
- in
-
- let compress = ref false in
- let convert = ref "" in
- let debug_gc = ref false in
- let format = ref "" in
- let ignores = ref [] in
- let machine_readable = ref false in
- let option = ref "" in
- let quiet = ref false in
- let verbose = ref false in
- let trace = ref false in
- let zeroes = ref [] in
-
- let ditto = " -\"-" in
- let argspec = Arg.align [
- "--check-tmpdir", Arg.String set_check_tmpdir,
"ignore|..." ^ " " ^ s_"Check there is enough space in
$TMPDIR";
- "--compress", Arg.Set compress, " " ^
s_"Compressed output format";
- "--convert", Arg.Set_string convert, s_"format" ^
" " ^ s_"Format of output disk (default: same as input)";
- "--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";
- "--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";
- "-q", Arg.Set quiet, " " ^
s_"Quiet output";
- "--quiet", Arg.Set quiet, ditto;
- "-v", Arg.Set verbose, " " ^
s_"Enable debugging messages";
- "--verbose", Arg.Set verbose, ditto;
- "-V", Arg.Unit display_version, " " ^
s_"Display version and exit";
- "--version", Arg.Unit display_version, ditto;
- "-x", Arg.Set trace, " " ^
s_"Enable tracing of libguestfs calls";
- "--zero", Arg.String (add zeroes), s_"fs" ^
" " ^ s_"Zero filesystem";
- ] in
- long_options := argspec;
- let disks = ref [] in
- let anon_fun s = disks := s :: !disks in
- let usage_msg - sprintf (f_"\
-%s: sparsify a virtual machine disk
-
- virt-sparsify [--options] indisk outdisk
-
-A short summary of the options is given below. For detailed help please
-read the man page virt-sparsify(1).
-")
- prog in
- Arg.parse argspec anon_fun usage_msg;
-
- (* Dereference the rest of the args. *)
- let check_tmpdir = !check_tmpdir in
- let compress = !compress in
- let convert = match !convert with "" -> None | str -> Some
str in
- let debug_gc = !debug_gc in
- let format = match !format with "" -> None | str -> Some
str in
- let ignores = List.rev !ignores in
- let machine_readable = !machine_readable in
- let option = match !option with "" -> None | str -> Some
str in
- let quiet = !quiet in
- let verbose = !verbose in
- let trace = !trace in
- let zeroes = List.rev !zeroes in
-
- (* No arguments and machine-readable mode? Print out some facts
- * about what this binary supports.
- *)
- if !disks = [] && machine_readable then (
- printf "virt-sparsify\n";
- printf "linux-swap\n";
- printf "zero\n";
- printf "check-tmpdir\n";
- let g = new G.guestfs () in
- g#add_drive "/dev/null";
- g#launch ();
- if g#feature_available [| "ntfsprogs"; "ntfs3g" |]
then
- printf "ntfs\n";
- if g#feature_available [| "btrfs" |] then
- printf "btrfs\n";
- exit 0
- );
-
- (* Verify we got exactly 2 disks. *)
- let indisk, outdisk - match List.rev !disks with
- | [indisk; outdisk] -> indisk, outdisk
- | _ ->
- error "usage is: %s [--options] indisk outdisk" prog in
-
- (* Simple-minded check that the user isn't trying to use the
- * same disk for input and output.
- *)
- 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
- 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, outdisk, check_tmpdir, compress, convert,
- debug_gc, format, ignores, machine_readable,
- option, quiet, verbose, trace, zeroes in
+ parse_cmdline () in
(* Once we have got past argument parsing and start to create
* temporary files (including the potentially massive overlay file), we
--
1.8.5.3
Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 15/18] sparsify: Refactor command line parsing to pass back a mode.
This is just code motion, but sets the ground-work for adding a second
mode (in-place image modification).
---
sparsify/cmdline.ml | 10 +++++++---
sparsify/sparsify.ml | 24 ++++++++++++++++--------
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/sparsify/cmdline.ml b/sparsify/cmdline.ml
index d803ab1..b714a9e 100644
--- a/sparsify/cmdline.ml
+++ b/sparsify/cmdline.ml
@@ -26,6 +26,10 @@ open Common_utils
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
+and check_t = [`Ignore|`Continue|`Warn|`Fail]
+
let parse_cmdline () let display_version () printf "virt-sparsify
%s\n" Config.package_version;
@@ -155,6 +159,6 @@ read the man page virt-sparsify(1).
error (f_"output '%s' cannot be a character device, it must be
a regular file")
outdisk;
- indisk, outdisk, check_tmpdir, compress, convert,
- debug_gc, format, ignores, machine_readable,
- option, quiet, verbose, trace, zeroes
+ indisk, debug_gc, format, ignores, machine_readable,
+ quiet, verbose, trace, zeroes,
+ Mode_copying (outdisk, check_tmpdir, compress, convert, option)
diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml
index c9692d4..0091d95 100644
--- a/sparsify/sparsify.ml
+++ b/sparsify/sparsify.ml
@@ -31,12 +31,23 @@ external statvfs_free_space : string -> int64
let () = Random.self_init ()
-let main () - let indisk, outdisk, check_tmpdir, compress, convert, debug_gc,
- format, ignores, machine_readable,
- option, quiet, verbose, trace, zeroes +let rec main () + let indisk,
debug_gc, format, ignores, machine_readable,
+ quiet, verbose, trace, zeroes, mode parse_cmdline () in
+ (match mode with
+ | Mode_copying (outdisk, check_tmpdir, compress, convert, option) ->
+ copying indisk outdisk check_tmpdir compress convert
+ format ignores machine_readable option quiet verbose trace zeroes
+ );
+
+ if debug_gc then
+ Gc.compact ()
+
+and copying indisk outdisk check_tmpdir compress convert
+ format ignores machine_readable option quiet verbose trace zeroes +
(* Once we have got past argument parsing and start to create
* temporary files (including the potentially massive overlay file), we
* need to catch SIGINT (^C) and exit cleanly so the temporary file
@@ -274,10 +285,7 @@ You can ignore this warning or change it to a hard failure
using the
if not quiet then (
print_newline ();
wrap (s_"Sparsify operation completed with no errors. Before deleting
the old disk, carefully check that the target disk boots and works
correctly.\n");
- );
-
- if debug_gc then
- Gc.compact ()
+ )
let () try main ()
--
1.8.5.3
Richard W.M. Jones
2014-Mar-11 23:13 UTC
[Libguestfs] [PATCH v2 16/18] sparsify: Move copying-mode code to a separate file.
This is just code motion.
---
po/POTFILES-ml | 1 +
sparsify/Makefile.am | 2 +
sparsify/copying.ml | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++
sparsify/sparsify.ml | 247 +--------------------------------------------
4 files changed, 280 insertions(+), 246 deletions(-)
create mode 100644 sparsify/copying.ml
diff --git a/po/POTFILES-ml b/po/POTFILES-ml
index ca29c25..fd2e886 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -32,6 +32,7 @@ mllib/uRI.ml
mllib/urandom.ml
resize/resize.ml
sparsify/cmdline.ml
+sparsify/copying.ml
sparsify/sparsify.ml
sysprep/main.ml
sysprep/sysprep_operation.ml
diff --git a/sparsify/Makefile.am b/sparsify/Makefile.am
index 3a6a011..2f32093 100644
--- a/sparsify/Makefile.am
+++ b/sparsify/Makefile.am
@@ -27,6 +27,7 @@ CLEANFILES = *~ *.cmi *.cmo *.cmx *.cmxa *.o virt-sparsify
# Alphabetical order.
SOURCES = \
cmdline.ml \
+ copying.ml \
sparsify.ml \
statvfs-c.c
@@ -44,6 +45,7 @@ deps = \
$(top_builddir)/mllib/config.cmx \
statvfs-c.o \
cmdline.cmx \
+ copying.cmx \
sparsify.cmx
if HAVE_OCAMLOPT
diff --git a/sparsify/copying.ml b/sparsify/copying.ml
new file mode 100644
index 0000000..2765cf4
--- /dev/null
+++ b/sparsify/copying.ml
@@ -0,0 +1,276 @@
+(* 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 traditional virt-sparsify mode: We copy from a
+ * source disk to a destination disk.
+ *)
+
+open Unix
+open Printf
+
+open Common_gettext.Gettext
+
+module G = Guestfs
+
+open Common_utils
+open Cmdline
+
+external statvfs_free_space : string -> int64 +
"virt_sparsify_statvfs_free_space"
+
+let run indisk outdisk check_tmpdir compress convert
+ format ignores machine_readable option quiet verbose trace zeroes +
+ (* Once we have got past argument parsing and start to create
+ * temporary files (including the potentially massive overlay file), we
+ * need to catch SIGINT (^C) and exit cleanly so the temporary file
+ * goes away. Note that we don't delete temporaries in the signal
+ * handler.
+ *)
+ let do_sigint _ = exit 1 in
+ Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint);
+
+ (* What should the output format be? If the user specified an
+ * input format, use that, else detect it from the source image.
+ *)
+ let output_format + match convert with
+ | Some fmt -> fmt (* user specified output conversion *)
+ | None ->
+ match format with
+ | Some fmt -> fmt (* user specified input format, use that *)
+ | None ->
+ (* Don't know, so we must autodetect. *)
+ match (new G.guestfs ())#disk_format indisk with
+ | "unknown" ->
+ error (f_"cannot detect input disk format; use the --format
parameter")
+ | fmt -> fmt in
+
+ (* Compression is not supported by raw output (RHBZ#852194). *)
+ if output_format = "raw" && compress then
+ error (f_"--compress cannot be used for raw output. Remove this
option or use --convert qcow2.");
+
+ (* Get virtual size of the input disk. *)
+ let virtual_size = (new G.guestfs ())#disk_virtual_size indisk in
+ if not quiet then
+ printf (f_"Input disk virtual size = %Ld bytes (%s)\n%!")
+ virtual_size (human_size virtual_size);
+
+ (* Check there is enough space in $TMPDIR. *)
+ let tmpdir = Filename.temp_dir_name in
+
+ let print_warning () + let free_space = statvfs_free_space tmpdir in
+ let extra_needed = virtual_size -^ free_space in
+ if extra_needed > 0L then (
+ eprintf (f_"\
+
+WARNING: There may not be enough free space on %s.
+You may need to set TMPDIR to point to a directory with more free space.
+
+Max needed: %s. Free: %s. May need another %s.
+
+Note this is an overestimate. If the guest disk is full of data
+then not as much free space would be required.
+
+You can ignore this warning or change it to a hard failure using the
+--check-tmpdir=(ignore|continue|warn|fail) option. See virt-sparsify(1).
+
+%!")
+ tmpdir (human_size virtual_size)
+ (human_size free_space) (human_size extra_needed);
+ true
+ ) else false
+ in
+
+ (match check_tmpdir with
+ | `Ignore -> ()
+ | `Continue -> ignore (print_warning ())
+ | `Warn ->
+ if print_warning () then (
+ eprintf "Press RETURN to continue or ^C to quit.\n%!";
+ ignore (read_line ())
+ );
+ | `Fail ->
+ if print_warning () then (
+ eprintf "Exiting because --check-tmpdir=fail was set.\n%!";
+ exit 2
+ )
+ );
+
+ if not quiet then
+ printf (f_"Create overlay file in %s to protect source disk
...\n%!") tmpdir;
+
+ (* Create the temporary overlay file. *)
+ let overlaydisk + let tmp = Filename.temp_file "sparsify"
".qcow2" in
+ unlink_on_exit tmp;
+
+ (* Create it with the indisk as the backing file. *)
+ (* XXX Old code used to:
+ * - detect if compat=1.1 was supported
+ * - add lazy_refcounts option
+ *)
+ (new G.guestfs ())#disk_create
+ ~backingfile:indisk ?backingformat:format ~compat:"1.1"
+ tmp "qcow2" Int64.minus_one;
+
+ tmp in
+
+ if not quiet then
+ printf (f_"Examine source disk ...\n%!");
+
+ (* Connect to libguestfs. *)
+ let g + let g = new G.guestfs () in
+ if trace then g#set_trace true;
+ if verbose then g#set_verbose true;
+
+ (* Note that the temporary overlay disk is always qcow2 format. *)
+ g#add_drive ~format:"qcow2" ~readonly:false
~cachemode:"unsafe" overlaydisk;
+
+ if not quiet then Progress.set_up_progress_bar ~machine_readable g;
+ g#launch ();
+
+ g in
+
+ (* Modify SIGINT handler (set first above) to cancel the handle. *)
+ let do_sigint _ + g#user_cancel ();
+ exit 1
+ in
+ Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint);
+
+ (* Write zeroes for 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;
+
+ g#zero_device fs
+ ) else (
+ let mounted + try g#mount fs "/"; true
+ with _ -> false in
+
+ if mounted then (
+ if not quiet then
+ printf (f_"Fill free space in %s with zero ...\n%!")
fs;
+
+ g#zero_free_space "/"
+ ) 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#zero_device fs;
+ if g#pwrite_device fs header 0L <> 4096 then
+ error (f_"pwrite: short write restoring swap partition
header")
+ )
+ )
+ );
+
+ g#umount_all ()
+ )
+ ) filesystems;
+
+ (* Fill 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_"Fill free space in volgroup %s with zero
...\n%!") vg;
+
+ g#zero_device lvdev;
+ g#sync ();
+ g#lvremove lvdev
+ )
+ )
+ ) vgs;
+
+ (* Don't need libguestfs now. *)
+ g#shutdown ();
+ g#close ();
+
+ (* Modify SIGINT handler (set first above) to just exit. *)
+ let do_sigint _ = exit 1 in
+ Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint);
+
+ (* Now run qemu-img convert which copies the overlay to the
+ * destination and automatically does sparsification.
+ *)
+ if not quiet then
+ printf (f_"Copy to destination and make sparse ...\n%!");
+
+ let cmd + sprintf "qemu-img convert -f qcow2 -O %s%s%s %s %s"
+ (Filename.quote output_format)
+ (if compress then " -c" else "")
+ (match option with
+ | None -> ""
+ | Some option -> " -o " ^ Filename.quote option)
+ (Filename.quote overlaydisk) (Filename.quote outdisk) in
+ if verbose then
+ printf "%s\n%!" cmd;
+ if Sys.command cmd <> 0 then
+ error (f_"external command failed: %s") cmd;
+
+ (* Finished. *)
+ if not quiet then (
+ print_newline ();
+ wrap (s_"Sparsify operation completed with no errors. Before deleting
the old disk, carefully check that the target disk boots and works
correctly.\n");
+ )
diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml
index 0091d95..faefb23 100644
--- a/sparsify/sparsify.ml
+++ b/sparsify/sparsify.ml
@@ -26,9 +26,6 @@ module G = Guestfs
open Common_utils
open Cmdline
-external statvfs_free_space : string -> int64 -
"virt_sparsify_statvfs_free_space"
-
let () = Random.self_init ()
let rec main () @@ -38,255 +35,13 @@ let rec main ()
(match mode with
| Mode_copying (outdisk, check_tmpdir, compress, convert, option) ->
- copying indisk outdisk check_tmpdir compress convert
+ Copying.run indisk outdisk check_tmpdir compress convert
format ignores machine_readable option quiet verbose trace zeroes
);
if debug_gc then
Gc.compact ()
-and copying indisk outdisk check_tmpdir compress convert
- format ignores machine_readable option quiet verbose trace zeroes -
- (* Once we have got past argument parsing and start to create
- * temporary files (including the potentially massive overlay file), we
- * need to catch SIGINT (^C) and exit cleanly so the temporary file
- * goes away. Note that we don't delete temporaries in the signal
- * handler.
- *)
- let do_sigint _ = exit 1 in
- Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint);
-
- (* What should the output format be? If the user specified an
- * input format, use that, else detect it from the source image.
- *)
- let output_format - match convert with
- | Some fmt -> fmt (* user specified output conversion *)
- | None ->
- match format with
- | Some fmt -> fmt (* user specified input format, use that *)
- | None ->
- (* Don't know, so we must autodetect. *)
- match (new G.guestfs ())#disk_format indisk with
- | "unknown" ->
- error (f_"cannot detect input disk format; use the --format
parameter")
- | fmt -> fmt in
-
- (* Compression is not supported by raw output (RHBZ#852194). *)
- if output_format = "raw" && compress then
- error (f_"--compress cannot be used for raw output. Remove this
option or use --convert qcow2.");
-
- (* Get virtual size of the input disk. *)
- let virtual_size = (new G.guestfs ())#disk_virtual_size indisk in
- if not quiet then
- printf (f_"Input disk virtual size = %Ld bytes (%s)\n%!")
- virtual_size (human_size virtual_size);
-
- (* Check there is enough space in $TMPDIR. *)
- let tmpdir = Filename.temp_dir_name in
-
- let print_warning () - let free_space = statvfs_free_space tmpdir in
- let extra_needed = virtual_size -^ free_space in
- if extra_needed > 0L then (
- eprintf (f_"\
-
-WARNING: There may not be enough free space on %s.
-You may need to set TMPDIR to point to a directory with more free space.
-
-Max needed: %s. Free: %s. May need another %s.
-
-Note this is an overestimate. If the guest disk is full of data
-then not as much free space would be required.
-
-You can ignore this warning or change it to a hard failure using the
---check-tmpdir=(ignore|continue|warn|fail) option. See virt-sparsify(1).
-
-%!")
- tmpdir (human_size virtual_size)
- (human_size free_space) (human_size extra_needed);
- true
- ) else false
- in
-
- (match check_tmpdir with
- | `Ignore -> ()
- | `Continue -> ignore (print_warning ())
- | `Warn ->
- if print_warning () then (
- eprintf "Press RETURN to continue or ^C to quit.\n%!";
- ignore (read_line ())
- );
- | `Fail ->
- if print_warning () then (
- eprintf "Exiting because --check-tmpdir=fail was set.\n%!";
- exit 2
- )
- );
-
- if not quiet then
- printf (f_"Create overlay file in %s to protect source disk
...\n%!") tmpdir;
-
- (* Create the temporary overlay file. *)
- let overlaydisk - let tmp = Filename.temp_file "sparsify"
".qcow2" in
- unlink_on_exit tmp;
-
- (* Create it with the indisk as the backing file. *)
- (* XXX Old code used to:
- * - detect if compat=1.1 was supported
- * - add lazy_refcounts option
- *)
- (new G.guestfs ())#disk_create
- ~backingfile:indisk ?backingformat:format ~compat:"1.1"
- tmp "qcow2" Int64.minus_one;
-
- tmp in
-
- if not quiet then
- printf (f_"Examine source disk ...\n%!");
-
- (* Connect to libguestfs. *)
- let g - let g = new G.guestfs () in
- if trace then g#set_trace true;
- if verbose then g#set_verbose true;
-
- (* Note that the temporary overlay disk is always qcow2 format. *)
- g#add_drive ~format:"qcow2" ~readonly:false
~cachemode:"unsafe" overlaydisk;
-
- if not quiet then Progress.set_up_progress_bar ~machine_readable g;
- g#launch ();
-
- g in
-
- (* Modify SIGINT handler (set first above) to cancel the handle. *)
- let do_sigint _ - g#user_cancel ();
- exit 1
- in
- Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint);
-
- (* Write zeroes for 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;
-
- g#zero_device fs
- ) else (
- let mounted - try g#mount fs "/"; true
- with _ -> false in
-
- if mounted then (
- if not quiet then
- printf (f_"Fill free space in %s with zero ...\n%!")
fs;
-
- g#zero_free_space "/"
- ) 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#zero_device fs;
- if g#pwrite_device fs header 0L <> 4096 then
- error (f_"pwrite: short write restoring swap partition
header")
- )
- )
- );
-
- g#umount_all ()
- )
- ) filesystems;
-
- (* Fill 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_"Fill free space in volgroup %s with zero
...\n%!") vg;
-
- g#zero_device lvdev;
- g#sync ();
- g#lvremove lvdev
- )
- )
- ) vgs;
-
- (* Don't need libguestfs now. *)
- g#shutdown ();
- g#close ();
-
- (* Modify SIGINT handler (set first above) to just exit. *)
- let do_sigint _ = exit 1 in
- Sys.set_signal Sys.sigint (Sys.Signal_handle do_sigint);
-
- (* Now run qemu-img convert which copies the overlay to the
- * destination and automatically does sparsification.
- *)
- if not quiet then
- printf (f_"Copy to destination and make sparse ...\n%!");
-
- let cmd - sprintf "qemu-img convert -f qcow2 -O %s%s%s %s %s"
- (Filename.quote output_format)
- (if compress then " -c" else "")
- (match option with
- | None -> ""
- | Some option -> " -o " ^ Filename.quote option)
- (Filename.quote overlaydisk) (Filename.quote outdisk) in
- if verbose then
- printf "%s\n%!" cmd;
- if Sys.command cmd <> 0 then
- error (f_"external command failed: %s") cmd;
-
- (* Finished. *)
- if not quiet then (
- print_newline ();
- wrap (s_"Sparsify operation completed with no errors. Before deleting
the old disk, carefully check that the target disk boots and works
correctly.\n");
- )
-
let () try main ()
with
--
1.8.5.3
Richard W.M. Jones
2014-Mar-11 23:14 UTC
[Libguestfs] [PATCH v2 17/18] 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 fd2e886..2c37d91 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -33,6 +33,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-11 23:14 UTC
[Libguestfs] [PATCH v2 18/18] 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
Pino Toscano
2014-Mar-12 09:45 UTC
Re: [Libguestfs] [PATCH v2 03/18] New API parameter: Add discard parameter to guestfs_add_drive_opts.
On Tuesday 11 March 2014 23:13:46 Richard W.M. Jones wrote:> 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); >(and others) It seems like these functions, albeit internal and private to this file, could need some help in avoid the over-long list of parameters, which all need to be changed every time there's an argument change (just like now). I'll do a small refactor.> --- 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, > +};I guess the manual numbering is not needed.> > - 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; > }... and ...> @@ -1499,7 +1544,9 @@ construct_libvirt_xml_appliance (guestfs_h *g, > attribute ("bus", "scsi"); > } end_element (); > > - if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2", "unsafe") == -1) > + if (construct_libvirt_xml_disk_driver_qemu (g, NULL, NULL, xo, > + "qcow2", "unsafe", > + discard_disable) == -1) return -1; > > if (construct_libvirt_xml_disk_address (g, xo,Even if passing discard_disable makes sure that now "data" and "drv" are not used within construct_libvirt_xml_disk_driver_qemu, wouldn't be better to pass them anyway if possible, so further code might use them (at least "data") safely? -- Pino Toscano
On Tuesday 11 March 2014 23:13:43 Richard W.M. Jones wrote:> This still isn't working at the moment. See: > http://marc.info/?t=139457409300003&r=1&w=2 > > 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.Do you plan to wait to solve the issues (wrt the linux-ext4 discussion) before pushing the series? There are few commits (01, 02, 11, 12, 13, 14, 15, 16) which IMHO could go even right now (so I ACK them). -- Pino Toscano
Richard W.M. Jones
2014-Mar-12 13:44 UTC
Re: [Libguestfs] [PATCH v2 00/18] Add discard support.
On Wed, Mar 12, 2014 at 10:53:06AM +0100, Pino Toscano wrote:> On Tuesday 11 March 2014 23:13:43 Richard W.M. Jones wrote: > > This still isn't working at the moment. See: > > http://marc.info/?t=139457409300003&r=1&w=2 > > > > 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. > > Do you plan to wait to solve the issues (wrt the linux-ext4 discussion) > before pushing the series?I just fixed it. There is a stupid mistake in daemon/fstrim.c:>From edee98c2e44838bf4cd997886fbabd308d5e379f Mon Sep 17 00:00:00 2001From: "Richard W.M. Jones" <rjones@redhat.com> Date: Wed, 12 Mar 2014 13:41:39 +0000 Subject: [PATCH] daemon: fstrim: Fix fstrim so it trims the correct filesystem. We didn't call sysroot_path, so it was trimming the appliance instead of the guest filesystem. --- daemon/fstrim.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/daemon/fstrim.c b/daemon/fstrim.c index bf9dad8..2aad155 100644 --- a/daemon/fstrim.c +++ b/daemon/fstrim.c @@ -46,6 +46,7 @@ do_fstrim (const char *path, const char *argv[MAX_ARGS]; size_t i = 0; char offset_s[64], length_s[64], mfe_s[64]; + CLEANUP_FREE char *buf = NULL; CLEANUP_FREE char *out = NULL, *err = NULL; int r; @@ -88,7 +89,13 @@ do_fstrim (const char *path, if (verbose) ADD_ARG (argv, i, "-v"); - ADD_ARG (argv, i, path); + buf = sysroot_path (path); + if (!buf) { + reply_with_error ("malloc"); + return -1; + } + + ADD_ARG (argv, i, buf); ADD_ARG (argv, i, NULL); r = commandv (&out, &err, argv); -- 1.8.5.3> There are few commits (01, 02, 11, 12, 13, 14, 15, 16) which IMHO could > go even right now (so I ACK them).I'm going to read your other comments, and I'll post a v3 of this series. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#)