From 180e3c30dee577dd44b3f1ae5bb16de2459b1d23 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Fri, 29 Aug 2014 18:19:13 +0100 Subject: [PATCH] Add support for qemu copy_on_read feature. When drives are added with this flag, reads from the backing file are copied into the overlay, improving performance of reads to the same area of disk (at the expense of local storage). This is excellent for reading remote / slow sources like HTTP. --- generator/actions.ml | 15 ++++++++++++--- src/drives.c | 13 +++++++++++-- src/guestfs-internal-frontend.h | 2 ++ src/guestfs-internal.h | 1 + src/launch-direct.c | 3 ++- src/launch-libvirt.c | 13 ++++++++----- src/libvirt-domain.c | 16 ++++++++++++++++ 7 files changed, 52 insertions(+), 11 deletions(-) diff --git a/generator/actions.ml b/generator/actions.ml index fdb35920f..37cd227e2 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -1290,7 +1290,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"; OString "discard"]; + 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"; OBool "copyonread"]; once_had_no_optargs = true; blocking = false; fish_alias = ["add"]; @@ -1538,6 +1538,15 @@ possible, but don't mind if it doesn't work. =back +=item C + +The boolean parameter C enables copy-on-read support. +This only affects disk formats which have backing files, and causes +reads to be stored in the overlay layer, speeding up multiple reads +of the same area of disk. + +The default is false. + =back" }; { defaults with @@ -1580,7 +1589,7 @@ not part of the formal API and can be removed or changed at any time." }; { defaults with name = "add_domain"; - style = RInt "nrdisks", [String "dom"], [OString "libvirturi"; OBool "readonly"; OString "iface"; OBool "live"; OBool "allowuuid"; OString "readonlydisk"; OString "cachemode"; OString "discard"]; + style = RInt "nrdisks", [String "dom"], [OString "libvirturi"; OBool "readonly"; OString "iface"; OBool "live"; OBool "allowuuid"; OString "readonlydisk"; OString "cachemode"; OString "discard"; OBool "copyonread"]; fish_alias = ["domain"]; config_only = true; shortdesc = "add the disk(s) from a named libvirt domain"; longdesc = "\ @@ -1672,7 +1681,7 @@ C." }; This interface is not quite baked yet. -- RWMJ 2010-11-11 { defaults with name = "add_libvirt_dom"; - style = RInt "nrdisks", [Pointer ("virDomainPtr", "dom")], [Bool "readonly"; String "iface"; Bool "live"; String "readonlydisk"; OString "cachemode"; OString "discard"]; + style = RInt "nrdisks", [Pointer ("virDomainPtr", "dom")], [Bool "readonly"; String "iface"; Bool "live"; String "readonlydisk"; OString "cachemode"; OString "discard"; OBool "copyonread"]; in_fish = false; shortdesc = "add the disk(s) from a libvirt domain"; longdesc = "\ diff --git a/src/drives.c b/src/drives.c index b92599657..307f267f9 100644 --- a/src/drives.c +++ b/src/drives.c @@ -62,6 +62,7 @@ struct drive_create_data { const char *disk_label; const char *cachemode; enum discard discard; + bool copyonread; }; /* Compile all the regular expressions once when the shared library is @@ -146,6 +147,7 @@ create_drive_file (guestfs_h *g, 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; + drv->copyonread = data->copyonread; if (data->readonly) { if (create_overlay (g, drv) == -1) { @@ -181,6 +183,7 @@ create_drive_non_file (guestfs_h *g, 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; + drv->copyonread = data->copyonread; if (data->readonly) { if (create_overlay (g, drv) == -1) { @@ -467,6 +470,7 @@ create_drive_dev_null (guestfs_h *g, data->exportname = tmpfile; data->discard = discard_disable; + data->copyonread = false; return create_drive_file (g, data); } @@ -532,7 +536,7 @@ 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%s", + (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s%s%s", drv->src.u.path, drv->readonly ? " readonly" : "", drv->src.format ? " format=" : "", @@ -547,7 +551,8 @@ drive_to_string (guestfs_h *g, const struct drive *drv) drv->cachemode ? " cache=" : "", drv->cachemode ? : "", drv->discard == discard_disable ? "" : - drv->discard == discard_enable ? " discard=enable" : " discard=besteffort"); + drv->discard == discard_enable ? " discard=enable" : " discard=besteffort", + drv->copyonread ? " copyonread" : ""); } /* Add struct drive to the g->drives vector at the given index. */ @@ -814,6 +819,10 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, else data.discard = discard_disable; + data.copyonread = + optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_COPYONREAD_BITMASK + ? optargs->copyonread : false; + 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); diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h index 20331c8e6..5c5d957d8 100644 --- a/src/guestfs-internal-frontend.h +++ b/src/guestfs-internal-frontend.h @@ -152,6 +152,8 @@ struct guestfs___add_libvirt_dom_argv { const char *cachemode; #define GUESTFS___ADD_LIBVIRT_DOM_DISCARD_BITMASK (UINT64_C(1)<<5) const char *discard; +#define GUESTFS___ADD_LIBVIRT_DOM_COPYONREAD_BITMASK (UINT64_C(1)<<6) + int copyonread; }; 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/guestfs-internal.h b/src/guestfs-internal.h index 2e9a1d15e..69ea2dc36 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -262,6 +262,7 @@ struct drive { char *disk_label; char *cachemode; enum discard discard; + bool copyonread; }; /* Extra hv parameters (from guestfs_config). */ diff --git a/src/launch-direct.c b/src/launch-direct.c index 3a8f2b2ab..5301176a0 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -528,7 +528,7 @@ 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%s,id=hd%zu", + (g, "file=%s%s,cache=%s%s%s%s%s%s%s,id=hd%zu", escaped_file, drv->readonly ? ",snapshot=on" : "", drv->cachemode ? drv->cachemode : "writeback", @@ -537,6 +537,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) drv->src.format ? drv->src.format : "", drv->disk_label ? ",serial=" : "", drv->disk_label ? drv->disk_label : "", + drv->copyonread ? ",copy-on-read=on" : "", i); } else { diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index f86ff74f5..9bce91fd6 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -817,7 +817,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, 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_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, bool copyonread); 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); @@ -1277,7 +1277,7 @@ construct_libvirt_xml_disk (guestfs_h *g, if (construct_libvirt_xml_disk_driver_qemu (g, data, drv, xo, "qcow2", "unsafe", - discard_disable) + discard_disable, false) == -1) return -1; } @@ -1416,7 +1416,7 @@ construct_libvirt_xml_disk (guestfs_h *g, if (construct_libvirt_xml_disk_driver_qemu (g, data, drv, xo, format, drv->cachemode ? : "writeback", - drv->discard) + drv->discard, false) == -1) return -1; } @@ -1458,7 +1458,8 @@ construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, xmlTextWriterPtr xo, const char *format, const char *cachemode, - enum discard discard) + enum discard discard, + bool copyonread) { bool discard_unmap = false; @@ -1492,6 +1493,8 @@ construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, attribute ("cache", cachemode); if (discard_unmap) attribute ("discard", "unmap"); + if (copyonread) + attribute ("copy_on_read", "on"); } end_element (); return 0; @@ -1578,7 +1581,7 @@ construct_libvirt_xml_appliance (guestfs_h *g, if (construct_libvirt_xml_disk_driver_qemu (g, params->data, NULL, xo, "qcow2", "unsafe", - discard_disable) == -1) + discard_disable, false) == -1) return -1; if (construct_libvirt_xml_disk_address (g, xo, params->appliance_index) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index d148d4aaa..7e0a0d9f5 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -65,6 +65,7 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name, const char *iface; const char *cachemode; const char *discard; + bool copyonread; struct guestfs___add_libvirt_dom_argv optargs2 = { .bitmask = 0 }; libvirturi = optargs->bitmask & GUESTFS_ADD_DOMAIN_LIBVIRTURI_BITMASK @@ -83,6 +84,8 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name, ? optargs->cachemode : NULL; discard = optargs->bitmask & GUESTFS_ADD_DOMAIN_DISCARD_BITMASK ? optargs->discard : NULL; + copyonread = optargs->bitmask & GUESTFS_ADD_DOMAIN_COPYONREAD_BITMASK + ? optargs->copyonread : false; if (live && readonly) { error (g, _("you cannot set both live and readonly flags")); @@ -143,6 +146,10 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name, optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_DISCARD_BITMASK; optargs2.discard = discard; } + if (copyonread) { + optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_COPYONREAD_BITMASK; + optargs2.copyonread = copyonread; + } r = guestfs___add_libvirt_dom (g, dom, &optargs2); @@ -179,6 +186,7 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom, const char *iface; const char *cachemode; const char *discard; + bool copyonread; int live; /* Default for back-compat reasons: */ enum readonlydisk readonlydisk = readonlydisk_write; @@ -220,6 +228,10 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom, optargs->bitmask & GUESTFS___ADD_LIBVIRT_DOM_DISCARD_BITMASK ? optargs->discard : NULL; + copyonread = + optargs->bitmask & GUESTFS___ADD_LIBVIRT_DOM_COPYONREAD_BITMASK + ? optargs->copyonread : false; + if (live && readonly) { error (g, _("you cannot set both live and readonly flags")); return -1; @@ -289,6 +301,10 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom, data.optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK; data.optargs.discard = discard; } + if (copyonread) { + data.optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_COPYONREAD_BITMASK; + data.optargs.copyonread = copyonread; + } /* Checkpoint the command line around the operation so that either * all disks are added or none are added.