--- 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

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

On Wed, Mar 12, 2014 at 10:45:17AM +0100, Pino Toscano wrote: Yes, there's definitely some refactoring which would help with parameter passing in this file. I set discard_disable = 0 to be safe (I think the C standard implies it anyway), because we rely on calloc to set the default value. I'll remove the other two in v3 of this series. Unfortunately in the second call we don't have 'drv' (because we're adding the appliance disk which has no associated drv struct). However I will add an assert & a comment into construct_libvirt_xml_disk_driver_qemu which should mean we won't break that function in future. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/

--- 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

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

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

--- 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

--- 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

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

--- 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