This adds a discard parameter to the guestfs_add_drive_opts which approximately maps to the discard=ignore|unmap parameter supported by qemu. If discard is set to "enable" then we force discard=unmap (and try to fail if it is not possible). If discard is set to the more useful "besteffort" option, then we enable discard if possible. The default is "disable". --- generator/actions.ml | 32 +++++++++++++- src/drives.c | 38 +++++++++++++--- src/guestfs-internal.h | 9 ++++ src/launch-direct.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++- src/launch-libvirt.c | 99 ++++++++++++++++++++++++++++++++++++++--- src/launch-uml.c | 6 +++ 6 files changed, 287 insertions(+), 14 deletions(-) diff --git a/generator/actions.ml b/generator/actions.ml index a6ef194..b25194c 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -1286,7 +1286,7 @@ not all belong to a single logical operating system { defaults with name = "add_drive"; - style = RErr, [String "filename"], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"]; + style = RErr, [String "filename"], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"]; once_had_no_optargs = true; blocking = false; fish_alias = ["add"]; @@ -1504,6 +1504,36 @@ for scratch or temporary disks. =back +=item C<discard> + +Enable or disable discard (a.k.a. trim or unmap) support on this +drive. If enabled, operations such as C<guestfs_fstrim> will be able +to discard / make thin / punch holes in the underlying host file +or device. + +Possible discard settings are: + +=over 4 + +=item C<discard = \"disable\"> + +Disable discard support. This is the default. + +=item C<discard = \"enable\"> + +Enable discard support. Fail if discard is not possible. + +=item C<discard = \"besteffort\"> + +Enable discard support if possible, but don't fail if it is not +supported. + +Since not all backends and not all underlying systems support +discard, this is a good choice if you want to use discard if +possible, but don't mind if it doesn't work. + +=back + =back" }; { defaults with diff --git a/src/drives.c b/src/drives.c index 2c776a2..57c2471 100644 --- a/src/drives.c +++ b/src/drives.c @@ -61,6 +61,7 @@ struct drive_create_data { const char *name; const char *disk_label; const char *cachemode; + enum discard discard; }; /* Compile all the regular expressions once when the shared library is @@ -144,6 +145,7 @@ create_drive_file (guestfs_h *g, drv->name = data->name ? safe_strdup (g, data->name) : NULL; drv->disk_label = data->disk_label ? safe_strdup (g, data->disk_label) : NULL; drv->cachemode = data->cachemode ? safe_strdup (g, data->cachemode) : NULL; + drv->discard = data->discard; if (data->readonly) { if (create_overlay (g, drv) == -1) { @@ -178,6 +180,7 @@ create_drive_non_file (guestfs_h *g, drv->name = data->name ? safe_strdup (g, data->name) : NULL; drv->disk_label = data->disk_label ? safe_strdup (g, data->disk_label) : NULL; drv->cachemode = data->cachemode ? safe_strdup (g, data->cachemode) : NULL; + drv->discard = data->discard; if (data->readonly) { if (create_overlay (g, drv) == -1) { @@ -473,6 +476,7 @@ create_drive_dev_null (guestfs_h *g, return NULL; data->exportname = tmpfile; + data->discard = discard_disable; return create_drive_file (g, data); } @@ -511,8 +515,8 @@ free_drive_struct (struct drive *drv) free (drv); } -static const char * -protocol_to_string (enum drive_protocol protocol) +const char * +guestfs___drive_protocol_to_string (enum drive_protocol protocol) { switch (protocol) { case drive_protocol_file: return "file"; @@ -538,12 +542,12 @@ static char * drive_to_string (guestfs_h *g, const struct drive *drv) { return safe_asprintf - (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s", + (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s%s", drv->src.u.path, drv->readonly ? " readonly" : "", drv->src.format ? " format=" : "", drv->src.format ? : "", - protocol_to_string (drv->src.protocol), + guestfs___drive_protocol_to_string (drv->src.protocol), drv->iface ? " iface=" : "", drv->iface ? : "", drv->name ? " name=" : "", @@ -551,7 +555,9 @@ drive_to_string (guestfs_h *g, const struct drive *drv) drv->disk_label ? " label=" : "", drv->disk_label ? : "", drv->cachemode ? " cache=" : "", - drv->cachemode ? : ""); + drv->cachemode ? : "", + drv->discard == discard_disable ? "" : + drv->discard == discard_enable ? " discard=enable" : " discard=besteffort"); } /* Add struct drive to the g->drives vector at the given index. */ @@ -795,6 +801,28 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, data.cachemode = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_CACHEMODE_BITMASK ? optargs->cachemode : NULL; + if (optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK) { + if (STREQ (optargs->discard, "disable")) + data.discard = discard_disable; + else if (STREQ (optargs->discard, "enable")) + data.discard = discard_enable; + else if (STREQ (optargs->discard, "besteffort")) + data.discard = discard_besteffort; + else { + error (g, _("discard parameter must be 'disable', 'enable' or 'besteffort'")); + free_drive_servers (data.servers, data.nr_servers); + return -1; + } + } + else + data.discard = discard_disable; + + if (data.readonly && data.discard == discard_enable) { + error (g, _("discard support cannot be enabled on read-only drives")); + free_drive_servers (data.servers, data.nr_servers); + return -1; + } + if (data.format && !valid_format_iface (data.format)) { error (g, _("%s parameter is empty or contains disallowed characters"), "format"); diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 545146f..95b09cf 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -231,6 +231,12 @@ struct drive_source { char *secret; }; +enum discard { + discard_disable = 0, + discard_enable, + discard_besteffort, +}; + /* There is one 'struct drive' per drive, including hot-plugged drives. */ struct drive { /* Original source of the drive, eg. file:..., http:... */ @@ -252,6 +258,7 @@ struct drive { char *name; char *disk_label; char *cachemode; + enum discard discard; }; /* Extra hv parameters (from guestfs_config). */ @@ -713,6 +720,7 @@ extern size_t guestfs___checkpoint_drives (guestfs_h *g); extern void guestfs___rollback_drives (guestfs_h *g, size_t); extern void guestfs___add_dummy_appliance_drive (guestfs_h *g); extern void guestfs___free_drives (guestfs_h *g); +extern const char *guestfs___drive_protocol_to_string (enum drive_protocol protocol); /* appliance.c */ extern int guestfs___build_appliance (guestfs_h *g, char **kernel, char **dtb, char **initrd, char **appliance); @@ -832,6 +840,7 @@ extern void guestfs___cleanup_cmd_close (struct command **); /* launch-direct.c */ extern char *guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src); +extern bool guestfs___discard_possible (guestfs_h *g, struct drive *drv, unsigned long qemu_version); /* utils.c */ extern int guestfs___validate_guid (const char *); diff --git a/src/launch-direct.c b/src/launch-direct.c index 1e84061..3d478c7 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -509,6 +509,30 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) CLEANUP_FREE char *file = NULL, *escaped_file = NULL, *param = NULL; if (!drv->overlay) { + const char *discard_mode = ""; + int major = data->qemu_version_major, minor = data->qemu_version_minor; + unsigned long qemu_version = major * 1000000 + minor * 1000; + + switch (drv->discard) { + case discard_disable: + /* Since the default is always discard=ignore, don't specify it + * on the command line. This also avoids unnecessary breakage + * with qemu < 1.5 which didn't have the option at all. + */ + break; + case discard_enable: + if (!guestfs___discard_possible (g, drv, qemu_version)) + goto cleanup0; + /*FALLTHROUGH*/ + case discard_besteffort: + /* I believe from reading the code that this is always safe as + * long as qemu >= 1.5. + */ + if (major > 1 || (major == 1 && minor >= 5)) + discard_mode = ",discard=unmap"; + break; + } + /* Make the file= parameter. */ file = guestfs___drive_source_qemu_param (g, &drv->src); escaped_file = qemu_escape_param (g, file); @@ -517,10 +541,11 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) * the if=... at the end. */ param = safe_asprintf - (g, "file=%s%s,cache=%s%s%s%s%s,id=hd%zu", + (g, "file=%s%s,cache=%s%s%s%s%s%s,id=hd%zu", escaped_file, drv->readonly ? ",snapshot=on" : "", drv->cachemode ? drv->cachemode : "writeback", + discard_mode, drv->src.format ? ",format=" : "", drv->src.format ? drv->src.format : "", drv->disk_label ? ",serial=" : "", @@ -1370,6 +1395,96 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src) abort (); } +/* Test if discard is both supported by qemu AND possible with the + * underlying file or device. This returns 1 if discard is possible. + * It returns 0 if not possible and sets the error to the reason why. + * + * This function is called when the user set discard == "enable". + * + * qemu_version is the version of qemu in the form returned by libvirt: + * major * 1,000,000 + minor * 1,000 + release + */ +bool +guestfs___discard_possible (guestfs_h *g, struct drive *drv, + unsigned long qemu_version) +{ + /* qemu >= 1.5. This was the first version that supported the + * discard option on -drive at all. + */ + bool qemu15 = qemu_version >= 1005000; + /* qemu >= 1.6. This was the first version that supported unmap on + * qcow2 backing files. + */ + bool qemu16 = qemu_version >= 1006000; + + if (!qemu15) { + error (g, _("discard cannot be enabled on this drive: " + "qemu < 1.5")); + return false; + } + + /* If it's an overlay, discard is not possible (on the underlying + * file). This has probably been caught earlier since we already + * checked that the drive is !readonly. Nevertheless ... + */ + if (drv->overlay) { + error (g, _("discard cannot be enabled on this drive: " + "the drive has a read-only overlay")); + return false; + } + + /* Look at the source format. */ + if (drv->src.format == NULL) { + /* We could autodetect the format, but we don't ... yet. XXX */ + error (g, _("discard cannot be enabled on this drive: " + "you have to specify the format of the file")); + return false; + } + else if (STREQ (drv->src.format, "raw")) + /* OK */ ; + else if (STREQ (drv->src.format, "qcow2")) { + if (!qemu16) { + error (g, _("discard cannot be enabled on this drive: " + "qemu < 1.6 cannot do discard on qcow2 files")); + return false; + } + } + else { + /* It's possible in future other formats will support discard, but + * currently (qemu 1.7) none of them do. + */ + error (g, _("discard cannot be enabled on this drive: " + "qemu does not support discard for '%s' format files"), + drv->src.format); + return false; + } + + switch (drv->src.protocol) { + /* Protocols which support discard. */ + case drive_protocol_file: + case drive_protocol_gluster: + case drive_protocol_iscsi: + case drive_protocol_nbd: + case drive_protocol_rbd: + case drive_protocol_sheepdog: /* XXX depends on server version */ + break; + + /* Protocols which don't support discard. */ + case drive_protocol_ftp: + case drive_protocol_ftps: + case drive_protocol_http: + case drive_protocol_https: + case drive_protocol_ssh: + case drive_protocol_tftp: + error (g, _("discard cannot be enabled on this drive: " + "protocol '%s' does not support discard"), + guestfs___drive_protocol_to_string (drv->src.protocol)); + return false; + } + + return true; +} + static int shutdown_direct (guestfs_h *g, void *datav, int check_for_errors) { diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index da1d665..e0049eb 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -107,6 +107,7 @@ struct backend_libvirt_data { bool selinux_norelabel_disks; char name[DOMAIN_NAME_LEN]; /* random name */ bool is_kvm; /* false = qemu, true = kvm (from capabilities)*/ + unsigned long qemu_version; /* qemu version (from libvirt) */ }; /* Parameters passed to construct_libvirt_xml and subfunctions. We @@ -131,6 +132,7 @@ static xmlChar *construct_libvirt_xml (guestfs_h *g, const struct libvirt_xml_pa static void debug_appliance_permissions (guestfs_h *g); static void debug_socket_permissions (guestfs_h *g); static void libvirt_error (guestfs_h *g, const char *fs, ...) __attribute__((format (printf,2,3))); +static void libvirt_debug (guestfs_h *g, const char *fs, ...) __attribute__((format (printf,2,3))); static int is_custom_hv (guestfs_h *g); static int is_blk (const char *path); static void ignore_errors (void *ignore, virErrorPtr ignore2); @@ -283,6 +285,19 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) */ virConnSetErrorFunc (conn, NULL, ignore_errors); + /* Get hypervisor (hopefully qemu) version. */ + if (virConnectGetVersion (conn, &data->qemu_version) == 0) { + debug (g, "qemu version (reported by libvirt) = %lu (%lu.%lu.%lu)", + data->qemu_version, + data->qemu_version / 1000000UL, + data->qemu_version / 1000UL % 1000UL, + data->qemu_version % 1000UL); + } + else { + libvirt_debug (g, "unable to read qemu version from libvirt"); + data->qemu_version = 0; + } + if (g->verbose) guestfs___print_timestamped_message (g, "get libvirt capabilities"); @@ -789,7 +804,7 @@ static int construct_libvirt_xml_devices (guestfs_h *g, const struct libvirt_xml static int construct_libvirt_xml_qemu_cmdline (guestfs_h *g, const struct libvirt_xml_params *params, xmlTextWriterPtr xo); static int construct_libvirt_xml_disk (guestfs_h *g, const struct backend_libvirt_data *data, xmlTextWriterPtr xo, struct drive *drv, size_t drv_index); static int construct_libvirt_xml_disk_target (guestfs_h *g, xmlTextWriterPtr xo, size_t drv_index); -static int construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, xmlTextWriterPtr xo, const char *format, const char *cachemode); +static int construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, const struct backend_libvirt_data *data, struct drive *drv, xmlTextWriterPtr xo, const char *format, const char *cachemode, enum discard discard); static int construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr xo, size_t drv_index); static int construct_libvirt_xml_disk_source_hosts (guestfs_h *g, xmlTextWriterPtr xo, const struct drive_source *src); static int construct_libvirt_xml_disk_source_seclabel (guestfs_h *g, const struct backend_libvirt_data *data, xmlTextWriterPtr xo); @@ -1233,7 +1248,9 @@ construct_libvirt_xml_disk (guestfs_h *g, if (construct_libvirt_xml_disk_target (g, xo, drv_index) == -1) return -1; - if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2", "unsafe") + if (construct_libvirt_xml_disk_driver_qemu (g, data, drv, + xo, "qcow2", "unsafe", + discard_disable) == -1) return -1; } @@ -1370,8 +1387,9 @@ construct_libvirt_xml_disk (guestfs_h *g, return -1; } - if (construct_libvirt_xml_disk_driver_qemu (g, xo, format, - drv->cachemode ? : "writeback") + if (construct_libvirt_xml_disk_driver_qemu (g, data, drv, xo, format, + drv->cachemode ? : "writeback", + drv->discard) == -1) return -1; } @@ -1407,14 +1425,46 @@ construct_libvirt_xml_disk_target (guestfs_h *g, xmlTextWriterPtr xo, } static int -construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, xmlTextWriterPtr xo, +construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, + const struct backend_libvirt_data *data, + struct drive *drv, + xmlTextWriterPtr xo, const char *format, - const char *cachemode) + const char *cachemode, + enum discard discard) { + bool discard_unmap = false; + + /* When adding the appliance disk, we don't have a 'drv' struct. + * However the caller will use discard_disable, so we don't need it. + */ + assert (discard == discard_disable || drv != NULL); + + switch (discard) { + case discard_disable: + /* Since the default is always discard=ignore, don't specify it + * in the XML. + */ + break; + case discard_enable: + if (!guestfs___discard_possible (g, drv, data->qemu_version)) + return -1; + /*FALLTHROUGH*/ + case discard_besteffort: + /* I believe from reading the code that this is always safe as + * long as qemu >= 1.5. + */ + if (data->qemu_version >= 1005000) + discard_unmap = true; + break; + } + start_element ("driver") { attribute ("name", "qemu"); attribute ("type", format); attribute ("cache", cachemode); + if (discard_unmap) + attribute ("discard", "unmap"); } end_element (); return 0; @@ -1499,7 +1549,9 @@ construct_libvirt_xml_appliance (guestfs_h *g, attribute ("bus", "scsi"); } end_element (); - if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2", "unsafe") == -1) + if (construct_libvirt_xml_disk_driver_qemu (g, params->data, NULL, xo, + "qcow2", "unsafe", + discard_disable) == -1) return -1; if (construct_libvirt_xml_disk_address (g, xo, params->appliance_index) @@ -1659,6 +1711,39 @@ libvirt_error (guestfs_h *g, const char *fs, ...) free (msg); } +/* Same as 'libvirt_error' but calls debug instead. */ +static void +libvirt_debug (guestfs_h *g, const char *fs, ...) +{ + va_list args; + char *msg; + int len; + virErrorPtr err; + + if (!g->verbose) + return; + + va_start (args, fs); + len = vasprintf (&msg, fs, args); + va_end (args); + + if (len < 0) + msg = safe_asprintf (g, + _("%s: internal error forming error message"), + __func__); + + /* In all recent libvirt, this retrieves the thread-local error. */ + err = virGetLastError (); + if (err) + debug (g, "%s: %s [code=%d domain=%d]", + msg, err->message, err->code, err->domain); + else + debug (g, "%s", msg); + + /* NB. 'err' must not be freed! */ + free (msg); +} + #if HAVE_LIBSELINUX static void selinux_warning (guestfs_h *g, const char *func, diff --git a/src/launch-uml.c b/src/launch-uml.c index deda366..2a6ddaf 100644 --- a/src/launch-uml.c +++ b/src/launch-uml.c @@ -122,6 +122,12 @@ uml_supported (guestfs_h *g) _("uml backend does not support drives with 'label' parameter")); return false; } + /* Note that discard == "besteffort" is fine. */ + if (drv->discard == discard_enable) { + error (g, + _("uml backend does not support drives with 'discard' parameter set to 'enable'")); + return false; + } } return true; -- 1.8.5.3

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

--- po/POTFILES-ml | 1 + sparsify/Makefile.am | 2 + sparsify/cmdline.ml | 77 +++++++++++++++++------- sparsify/in_place.ml | 143 +++++++++++++++++++++++++++++++++++++++++++++ sparsify/sparsify.ml | 3 + sparsify/virt-sparsify.pod | 63 +++++++++++++++++--- 6 files changed, 259 insertions(+), 30 deletions(-) create mode 100644 sparsify/in_place.ml diff --git a/po/POTFILES-ml b/po/POTFILES-ml index 899a460..ed96697 100644 --- a/po/POTFILES-ml +++ b/po/POTFILES-ml @@ -35,6 +35,7 @@ mllib/urandom.ml resize/resize.ml sparsify/cmdline.ml sparsify/copying.ml +sparsify/in_place.ml sparsify/sparsify.ml sysprep/main.ml sysprep/sysprep_operation.ml diff --git a/sparsify/Makefile.am b/sparsify/Makefile.am index 2f32093..811c131 100644 --- a/sparsify/Makefile.am +++ b/sparsify/Makefile.am @@ -28,6 +28,7 @@ CLEANFILES = *~ *.cmi *.cmo *.cmx *.cmxa *.o virt-sparsify SOURCES = \ cmdline.ml \ copying.ml \ + in_place.ml \ sparsify.ml \ statvfs-c.c @@ -46,6 +47,7 @@ deps = \ statvfs-c.o \ cmdline.cmx \ copying.cmx \ + in_place.cmx \ sparsify.cmx if HAVE_OCAMLOPT diff --git a/sparsify/cmdline.ml b/sparsify/cmdline.ml index b714a9e..d25e9e9 100644 --- a/sparsify/cmdline.ml +++ b/sparsify/cmdline.ml @@ -27,7 +27,8 @@ let prog = Filename.basename Sys.executable_name let error fs = error ~prog fs type mode_t = - Mode_copying of string * check_t * bool * string option * string option +| Mode_copying of string * check_t * bool * string option * string option +| Mode_in_place and check_t = [`Ignore|`Continue|`Warn|`Fail] let parse_cmdline () = @@ -54,6 +55,7 @@ let parse_cmdline () = let debug_gc = ref false in let format = ref "" in let ignores = ref [] in + let in_place = ref false in let machine_readable = ref false in let option = ref "" in let quiet = ref false in @@ -69,6 +71,8 @@ let parse_cmdline () = "--debug-gc", Arg.Set debug_gc, " " ^ s_"Debug GC and memory allocations"; "--format", Arg.Set_string format, s_"format" ^ " " ^ s_"Format of input disk"; "--ignore", Arg.String (add ignores), s_"fs" ^ " " ^ s_"Ignore filesystem"; + "--in-place", Arg.Set in_place, " " ^ s_"Modify the disk image in-place"; + "--inplace", Arg.Set in_place, ditto; "--long-options", Arg.Unit display_long_options, " " ^ s_"List long options"; "--machine-readable", Arg.Set machine_readable, " " ^ s_"Make output machine readable"; "-o", Arg.Set_string option, s_"option" ^ " " ^ s_"Add qemu-img options"; @@ -90,6 +94,8 @@ let parse_cmdline () = virt-sparsify [--options] indisk outdisk + virt-sparsify [--options] --in-place disk + A short summary of the options is given below. For detailed help please read the man page virt-sparsify(1). ") @@ -103,6 +109,7 @@ read the man page virt-sparsify(1). let debug_gc = !debug_gc in let format = match !format with "" -> None | str -> Some str in let ignores = List.rev !ignores in + let in_place = !in_place in let machine_readable = !machine_readable in let option = match !option with "" -> None | str -> Some str in let quiet = !quiet in @@ -118,6 +125,7 @@ read the man page virt-sparsify(1). printf "linux-swap\n"; printf "zero\n"; printf "check-tmpdir\n"; + printf "in-place\n"; let g = new G.guestfs () in g#add_drive "/dev/null"; g#launch (); @@ -128,12 +136,14 @@ read the man page virt-sparsify(1). exit 0 ); - (* Verify we got exactly 2 disks. *) + (* Verify we got exactly 1 or 2 disks, depending on the mode. *) let indisk, outdisk = - match List.rev !disks with - | [indisk; outdisk] -> indisk, outdisk + match in_place, List.rev !disks with + | false, [indisk; outdisk] -> indisk, outdisk + | true, [disk] -> disk, "" | _ -> - error "usage is: %s [--options] indisk outdisk" prog in + error "usage is: %s [--options] indisk outdisk OR %s --in-place disk" + prog prog in (* Simple-minded check that the user isn't trying to use the * same disk for input and output. @@ -141,24 +151,49 @@ read the man page virt-sparsify(1). if indisk = outdisk then error (f_"you cannot use the same disk image for input and output"); - (* The input disk must be an absolute path, so we can store the name - * in the overlay disk. - *) let indisk = - if not (Filename.is_relative indisk) then + if not in_place then ( + (* The input disk must be an absolute path, so we can store the name + * in the overlay disk. + *) + let indisk = + if not (Filename.is_relative indisk) then + indisk + else + Sys.getcwd () // indisk in + + (* Check the output is not a block or char special (RHBZ#1056290). *) + if is_block_device outdisk then + error (f_"output '%s' cannot be a block device, it must be a regular file") + outdisk; + + if is_char_device outdisk then + error (f_"output '%s' cannot be a character device, it must be a regular file") + outdisk; + indisk + ) + else ( (* --in-place checks *) + if check_tmpdir <> `Warn then + error (f_"you cannot use --in-place and --check-tmpdir options together"); + + if compress then + error (f_"you cannot use --in-place and --compress options together"); + + if convert <> None then + error (f_"you cannot use --in-place and --convert options together"); + + if option <> None then + error (f_"you cannot use --in-place and -o options together"); + + indisk + ) in + + let mode = + if not in_place then + Mode_copying (outdisk, check_tmpdir, compress, convert, option) else - Sys.getcwd () // indisk in - - (* Check the output is not a block or char special (RHBZ#1056290). *) - if is_block_device outdisk then - error (f_"output '%s' cannot be a block device, it must be a regular file") - outdisk; - - if is_char_device outdisk then - error (f_"output '%s' cannot be a character device, it must be a regular file") - outdisk; + Mode_in_place in indisk, debug_gc, format, ignores, machine_readable, - quiet, verbose, trace, zeroes, - Mode_copying (outdisk, check_tmpdir, compress, convert, option) + quiet, verbose, trace, zeroes, mode diff --git a/sparsify/in_place.ml b/sparsify/in_place.ml new file mode 100644 index 0000000..9f46ad3 --- /dev/null +++ b/sparsify/in_place.ml @@ -0,0 +1,143 @@ +(* virt-sparsify + * Copyright (C) 2011-2014 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + *) + +(* This is the virt-sparsify --in-place mode. *) + +open Unix +open Printf + +open Common_gettext.Gettext + +module G = Guestfs + +open Common_utils +open Cmdline + +let run disk format ignores machine_readable quiet verbose trace zeroes = + (* Connect to libguestfs. *) + let g = new G.guestfs () in + if trace then g#set_trace true; + if verbose then g#set_verbose true; + + (* XXX Current limitation of the API. Can remove this hunk in future. *) + let format = + match format with + | Some _ -> format + | None -> Some (g#disk_format disk) in + + g#add_drive ?format ~discard:"enable" disk; + + if not quiet then Progress.set_up_progress_bar ~machine_readable g; + g#launch (); + + (* Discard non-ignored filesystems that we are able to mount, and + * selected swap partitions. + *) + let filesystems = g#list_filesystems () in + let filesystems = List.map fst filesystems in + let filesystems = List.sort compare filesystems in + + let is_ignored fs = + let fs = g#canonical_device_name fs in + List.exists (fun fs' -> fs = g#canonical_device_name fs') ignores + in + + List.iter ( + fun fs -> + if not (is_ignored fs) then ( + if List.mem fs zeroes then ( + if not quiet then + printf (f_"Zeroing %s ...\n%!") fs; + + if not (g#blkdiscardzeroes fs) then + g#zero_device fs; + g#blkdiscard fs + ) else ( + let mounted = + try g#mount_options "discard" fs "/"; true + with _ -> false in + + if mounted then ( + if not quiet then + printf (f_"Trimming %s ...\n%!") fs; + + g#fstrim "/" + ) else ( + let is_linux_x86_swap = + (* Look for the signature for Linux swap on i386. + * Location depends on page size, so it definitely won't + * work on non-x86 architectures (eg. on PPC, page size is + * 64K). Also this avoids hibernated swap space: in those, + * the signature is moved to a different location. + *) + try g#pread_device fs 10 4086L = "SWAPSPACE2" + with _ -> false in + + if is_linux_x86_swap then ( + if not quiet then + printf (f_"Clearing Linux swap on %s ...\n%!") fs; + + (* Don't use mkswap. Just preserve the header containing + * the label, UUID and swap format version (libguestfs + * mkswap may differ from guest's own). + *) + let header = g#pread_device fs 4096 0L in + g#blkdiscard fs; + if g#pwrite_device fs header 0L <> 4096 then + error (f_"pwrite: short write restoring swap partition header") + ) + ) + ); + + g#umount_all () + ) + ) filesystems; + + (* Discard unused space in volume groups. *) + let vgs = g#vgs () in + let vgs = Array.to_list vgs in + let vgs = List.sort compare vgs in + List.iter ( + fun vg -> + if not (List.mem vg ignores) then ( + let lvname = string_random8 () in + let lvdev = "/dev/" ^ vg ^ "/" ^ lvname in + + let created = + try g#lvcreate_free lvname vg 100; true + with _ -> false in + + if created then ( + if not quiet then + printf (f_"Discard space in volgroup %s ...\n%!") vg; + + g#blkdiscard lvdev; + g#sync (); + g#lvremove lvdev + ) + ) + ) vgs; + + g#shutdown (); + g#close (); + + (* Finished. *) + if not quiet then ( + print_newline (); + wrap (s_"Sparsify in-place operation completed with no errors.\n"); + ) diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml index faefb23..f148296 100644 --- a/sparsify/sparsify.ml +++ b/sparsify/sparsify.ml @@ -37,6 +37,9 @@ let rec main () = | Mode_copying (outdisk, check_tmpdir, compress, convert, option) -> Copying.run indisk outdisk check_tmpdir compress convert format ignores machine_readable option quiet verbose trace zeroes + | Mode_in_place -> + In_place.run indisk format ignores machine_readable + quiet verbose trace zeroes ); if debug_gc then diff --git a/sparsify/virt-sparsify.pod b/sparsify/virt-sparsify.pod index 84e94c5..c12a15f 100644 --- a/sparsify/virt-sparsify.pod +++ b/sparsify/virt-sparsify.pod @@ -8,6 +8,8 @@ virt-sparsify - Make a virtual machine disk sparse virt-sparsify [--options] indisk outdisk + virt-sparsify [--options] --in-place disk + =head1 DESCRIPTION Virt-sparsify is a tool which can make a virtual machine disk (or any @@ -45,13 +47,6 @@ such as C<du -sh>. It can make a huge difference: =item * -Virt-sparsify does not do in-place modifications. It copies from a -source image to a destination image, leaving the source unchanged. -I<Check that the sparsification was successful before deleting the -source image>. - -=item * - The virtual machine I<must be shut down> before using this tool. =item * @@ -60,6 +55,9 @@ Virt-sparsify may require up to 2x the virtual size of the source disk image (1 temporary copy + 1 destination image). This is in the worst case and usually much less space is required. +If you are using the I<--in-place> option, then large amounts of +temporary space are B<not> required. + =item * Virt-sparsify cannot resize disk images. To do that, use @@ -105,6 +103,11 @@ to ignore (don't zero free space on) certain filesystems by doing: See L<virt-filesystems(1)> to get a list of filesystems within a disk image. +Since virt-sparsify E<ge> 1.26, you can now sparsify a disk image +in place by doing: + + virt-sparsify --in-place disk.img + =head1 OPTIONS =over 4 @@ -147,11 +150,15 @@ B<fail> and exit. =back +You cannot use this option and I<--in-place> together. + =item B<--compress> Compress the output file. This I<only> works if the output format is C<qcow2>. +You cannot use this option and I<--in-place> together. + =item B<--convert> raw =item B<--convert> qcow2 @@ -171,6 +178,8 @@ then virt-sparsify doesn't need to try to guess the input format. For fine-tuning the output format, see: I<--compress>, I<-o>. +You cannot use this option and I<--in-place> together. + =item B<--debug-gc> Debug garbage collection and memory allocation. This is only useful @@ -191,14 +200,23 @@ ensure the format is always specified. =item B<--ignore> volgroup -Ignore the named filesystem. Free space on the filesystem will not be +Ignore the named filesystem. + +When not using I<--in-place>: Free space on the filesystem will not be zeroed, but existing blocks of zeroes will still be sparsified. +When using I<--in-place>, the filesystem is ignored completely. + In the second form, this ignores the named volume group. Use the volume group name without the C</dev/> prefix, eg. I<--ignore vg_foo> You can give this option multiple times. +=item B<--in-place> + +Do in-place sparsification instead of copying sparsification. +See L</IN-PLACE SPARSIFICATION> below. + =item B<--machine-readable> This option is used to make the output more machine friendly @@ -217,6 +235,8 @@ them with commas, eg: virt-sparsify --convert qcow2 \ -o cluster_size=512,preallocation=metadata ... +You cannot use this option and I<--in-place> together. + =item B<-q> =item B<--quiet> @@ -249,6 +269,28 @@ excellent! You can give this option multiple times. =back +=head1 IN-PLACE SPARSIFICATION + +Since virt-sparsify E<ge> 1.26, the tool is able to do in-place +sparsification (instead of copying from an input disk to an output +disk). This is more efficient. However it requires special support +in libguestfs, the kernel and qemu, and it is not able to recover +quite as much space as copying sparsification. So in-place +sparsification is considered to be experimental at this time. + +To use this mode, specify a disk image which will be modified in +place: + + virt-sparsify --in-place disk.img + +Some options are not compatible with this mode: I<--convert>, +I<--compress> and I<-o> because they require wholesale disk format +changes; I<--check-tmpdir> because large amounts of temporary space +are not required. + +In-place sparsification works using discard (a.k.a trim or unmap) +support. + =head1 MACHINE READABLE OUTPUT The I<--machine-readable> option can be used to make the output more @@ -332,6 +374,9 @@ size of the tmpfs mountpoint, eg: mount -o remount,size=10G /tmp +If you are using the I<--in-place> option, then large amounts of +temporary space are B<not> required. + =back For other environment variables, see L<guestfs(3)/ENVIRONMENT VARIABLES>. @@ -355,4 +400,4 @@ Richard W.M. Jones L<http://people.redhat.com/~rjones/> =head1 COPYRIGHT -Copyright (C) 2011-2012 Red Hat Inc. +Copyright (C) 2011-2014 Red Hat Inc. -- 1.8.5.3