* [PATCH 0/3] qemu-img: Add convert --bitmaps
@ 2020-04-16 14:51 Eric Blake
2020-04-16 14:51 ` [PATCH 1/3] blockdev: Split off basic bitmap operations for qemu-img Eric Blake
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Eric Blake @ 2020-04-16 14:51 UTC (permalink / raw)
To: qemu-devel; +Cc: nsoffer, jsnow, qemu-block
Without this series, the process for copying one qcow2 image to
another including all of its bitmaps involves running qemu and doing
the copying by hand with a series of QMP commands. This makes the
process a bit more convenient.
I still think that someday we will need a 'qemu-img bitmap' with
various subcommands for manipulating bitmaps within an offline image,
but in the meantime, this seems like a useful addition on its own.
Series can also be downloaded at:
https://repo.or.cz/qemu/ericb.git/shortlog/refs/tags/qemu-img-bitmaps-v1
Eric Blake (3):
blockdev: Split off basic bitmap operations for qemu-img
qemu-img: Add convert --bitmaps option
iotests: Add test 291 to for qemu-img convert --bitmaps
docs/tools/qemu-img.rst | 6 +-
Makefile.objs | 2 +-
include/sysemu/blockdev.h | 10 ++
blockbitmaps.c | 217 +++++++++++++++++++++++++++++++++++++
blockdev.c | 184 -------------------------------
qemu-img.c | 81 +++++++++++++-
MAINTAINERS | 1 +
qemu-img-cmds.hx | 4 +-
tests/qemu-iotests/291 | 143 ++++++++++++++++++++++++
tests/qemu-iotests/291.out | 56 ++++++++++
tests/qemu-iotests/group | 1 +
11 files changed, 514 insertions(+), 191 deletions(-)
create mode 100644 blockbitmaps.c
create mode 100755 tests/qemu-iotests/291
create mode 100644 tests/qemu-iotests/291.out
--
2.26.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] blockdev: Split off basic bitmap operations for qemu-img
2020-04-16 14:51 [PATCH 0/3] qemu-img: Add convert --bitmaps Eric Blake
@ 2020-04-16 14:51 ` Eric Blake
2020-04-16 14:51 ` [PATCH 2/3] qemu-img: Add convert --bitmaps option Eric Blake
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2020-04-16 14:51 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Max Reitz, nsoffer, jsnow
The next patch wants to teach qemu how to copy a bitmap from one qcow2
file to another. But blockdev.o is too heavyweight to link into
qemu-img, so it's time to split off the bare bones of what we will
need into a new file blockbitmaps.o. Transactions are not needed in
qemu-img (if things fail while creating the new image, the fix is to
delete the botched copy, rather than worrying about atomic rollback).
For now, I stuck to just the minimum code motion (add and merge); we
could instead decide to move everything bitmap-related that does not
also pull in transactions (delete, enable, disable).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
Makefile.objs | 2 +-
include/sysemu/blockdev.h | 10 ++
blockbitmaps.c | 217 ++++++++++++++++++++++++++++++++++++++
blockdev.c | 184 --------------------------------
MAINTAINERS | 1 +
5 files changed, 229 insertions(+), 185 deletions(-)
create mode 100644 blockbitmaps.c
diff --git a/Makefile.objs b/Makefile.objs
index a7c967633acf..44e30fa9a6e3 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,7 +14,7 @@ chardev-obj-y = chardev/
authz-obj-y = authz/
block-obj-y = nbd/
-block-obj-y += block.o blockjob.o job.o
+block-obj-y += block.o blockbitmaps.o blockjob.o job.o
block-obj-y += block/ scsi/
block-obj-y += qemu-io-cmds.o
block-obj-$(CONFIG_REPLICATION) += replication.o
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index a86d99b3d875..95cfeb29bc0a 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -57,4 +57,14 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type,
Error **errp);
+BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
+ const char *name,
+ BlockDriverState **pbs,
+ Error **errp);
+BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
+ const char *target,
+ BlockDirtyBitmapMergeSourceList *bitmaps,
+ HBitmap **backup, Error **errp);
+
+
#endif
diff --git a/blockbitmaps.c b/blockbitmaps.c
new file mode 100644
index 000000000000..0d334d82006d
--- /dev/null
+++ b/blockbitmaps.c
@@ -0,0 +1,217 @@
+/*
+ * QEMU host block device bitmaps
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ *
+ * This file incorporates work covered by the following copyright and
+ * permission notice:
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+
+#include "sysemu/blockdev.h"
+#include "block/block.h"
+#include "qapi/qapi-commands-block.h"
+#include "qapi/error.h"
+
+/**
+ * block_dirty_bitmap_lookup:
+ * Return a dirty bitmap (if present), after validating
+ * the node reference and bitmap names.
+ *
+ * @node: The name of the BDS node to search for bitmaps
+ * @name: The name of the bitmap to search for
+ * @pbs: Output pointer for BDS lookup, if desired. Can be NULL.
+ * @errp: Output pointer for error information. Can be NULL.
+ *
+ * @return: A bitmap object on success, or NULL on failure.
+ */
+BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
+ const char *name,
+ BlockDriverState **pbs,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+
+ if (!node) {
+ error_setg(errp, "Node cannot be NULL");
+ return NULL;
+ }
+ if (!name) {
+ error_setg(errp, "Bitmap name cannot be NULL");
+ return NULL;
+ }
+ bs = bdrv_lookup_bs(node, node, NULL);
+ if (!bs) {
+ error_setg(errp, "Node '%s' not found", node);
+ return NULL;
+ }
+
+ bitmap = bdrv_find_dirty_bitmap(bs, name);
+ if (!bitmap) {
+ error_setg(errp, "Dirty bitmap '%s' not found", name);
+ return NULL;
+ }
+
+ if (pbs) {
+ *pbs = bs;
+ }
+
+ return bitmap;
+}
+
+void qmp_block_dirty_bitmap_add(const char *node, const char *name,
+ bool has_granularity, uint32_t granularity,
+ bool has_persistent, bool persistent,
+ bool has_disabled, bool disabled,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+ AioContext *aio_context;
+
+ if (!name || name[0] == '\0') {
+ error_setg(errp, "Bitmap name cannot be empty");
+ return;
+ }
+
+ bs = bdrv_lookup_bs(node, node, errp);
+ if (!bs) {
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ if (has_granularity) {
+ if (granularity < 512 || !is_power_of_2(granularity)) {
+ error_setg(errp, "Granularity must be power of 2 "
+ "and at least 512");
+ goto out;
+ }
+ } else {
+ /* Default to cluster size, if available: */
+ granularity = bdrv_get_default_bitmap_granularity(bs);
+ }
+
+ if (!has_persistent) {
+ persistent = false;
+ }
+
+ if (!has_disabled) {
+ disabled = false;
+ }
+
+ if (persistent &&
+ !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
+ {
+ goto out;
+ }
+
+ bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
+ if (bitmap == NULL) {
+ goto out;
+ }
+
+ if (disabled) {
+ bdrv_disable_dirty_bitmap(bitmap);
+ }
+
+ bdrv_dirty_bitmap_set_persistence(bitmap, persistent);
+
+out:
+ aio_context_release(aio_context);
+}
+
+BdrvDirtyBitmap *do_block_dirty_bitmap_merge(
+ const char *node, const char *target,
+ BlockDirtyBitmapMergeSourceList *bitmaps,
+ HBitmap **backup, Error **errp)
+{
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *dst, *src, *anon;
+ BlockDirtyBitmapMergeSourceList *lst;
+ Error *local_err = NULL;
+
+ dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
+ if (!dst) {
+ return NULL;
+ }
+
+ anon = bdrv_create_dirty_bitmap(bs, bdrv_dirty_bitmap_granularity(dst),
+ NULL, errp);
+ if (!anon) {
+ return NULL;
+ }
+
+ for (lst = bitmaps; lst; lst = lst->next) {
+ switch (lst->value->type) {
+ const char *name, *node;
+ case QTYPE_QSTRING:
+ name = lst->value->u.local;
+ src = bdrv_find_dirty_bitmap(bs, name);
+ if (!src) {
+ error_setg(errp, "Dirty bitmap '%s' not found", name);
+ dst = NULL;
+ goto out;
+ }
+ break;
+ case QTYPE_QDICT:
+ node = lst->value->u.external.node;
+ name = lst->value->u.external.name;
+ src = block_dirty_bitmap_lookup(node, name, NULL, errp);
+ if (!src) {
+ dst = NULL;
+ goto out;
+ }
+ break;
+ default:
+ abort();
+ }
+
+ bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ dst = NULL;
+ goto out;
+ }
+ }
+
+ /* Merge into dst; dst is unchanged on failure. */
+ bdrv_merge_dirty_bitmap(dst, anon, backup, errp);
+
+ out:
+ bdrv_release_dirty_bitmap(anon);
+ return dst;
+}
+
+void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
+ BlockDirtyBitmapMergeSourceList *bitmaps,
+ Error **errp)
+{
+ do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
+}
diff --git a/blockdev.c b/blockdev.c
index 5faddaa7052f..ff6f5d38fcd5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1185,53 +1185,6 @@ out_aio_context:
return NULL;
}
-/**
- * block_dirty_bitmap_lookup:
- * Return a dirty bitmap (if present), after validating
- * the node reference and bitmap names.
- *
- * @node: The name of the BDS node to search for bitmaps
- * @name: The name of the bitmap to search for
- * @pbs: Output pointer for BDS lookup, if desired. Can be NULL.
- * @errp: Output pointer for error information. Can be NULL.
- *
- * @return: A bitmap object on success, or NULL on failure.
- */
-static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
- const char *name,
- BlockDriverState **pbs,
- Error **errp)
-{
- BlockDriverState *bs;
- BdrvDirtyBitmap *bitmap;
-
- if (!node) {
- error_setg(errp, "Node cannot be NULL");
- return NULL;
- }
- if (!name) {
- error_setg(errp, "Bitmap name cannot be NULL");
- return NULL;
- }
- bs = bdrv_lookup_bs(node, node, NULL);
- if (!bs) {
- error_setg(errp, "Node '%s' not found", node);
- return NULL;
- }
-
- bitmap = bdrv_find_dirty_bitmap(bs, name);
- if (!bitmap) {
- error_setg(errp, "Dirty bitmap '%s' not found", name);
- return NULL;
- }
-
- if (pbs) {
- *pbs = bs;
- }
-
- return bitmap;
-}
-
/* New and old BlockDriverState structs for atomic group operations */
typedef struct BlkActionState BlkActionState;
@@ -2171,11 +2124,6 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common)
}
}
-static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(
- const char *node, const char *target,
- BlockDirtyBitmapMergeSourceList *bitmaps,
- HBitmap **backup, Error **errp);
-
static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
Error **errp)
{
@@ -2441,69 +2389,6 @@ void qmp_block_passwd(bool has_device, const char *device,
"Setting block passwords directly is no longer supported");
}
-void qmp_block_dirty_bitmap_add(const char *node, const char *name,
- bool has_granularity, uint32_t granularity,
- bool has_persistent, bool persistent,
- bool has_disabled, bool disabled,
- Error **errp)
-{
- BlockDriverState *bs;
- BdrvDirtyBitmap *bitmap;
- AioContext *aio_context;
-
- if (!name || name[0] == '\0') {
- error_setg(errp, "Bitmap name cannot be empty");
- return;
- }
-
- bs = bdrv_lookup_bs(node, node, errp);
- if (!bs) {
- return;
- }
-
- aio_context = bdrv_get_aio_context(bs);
- aio_context_acquire(aio_context);
-
- if (has_granularity) {
- if (granularity < 512 || !is_power_of_2(granularity)) {
- error_setg(errp, "Granularity must be power of 2 "
- "and at least 512");
- goto out;
- }
- } else {
- /* Default to cluster size, if available: */
- granularity = bdrv_get_default_bitmap_granularity(bs);
- }
-
- if (!has_persistent) {
- persistent = false;
- }
-
- if (!has_disabled) {
- disabled = false;
- }
-
- if (persistent &&
- !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
- {
- goto out;
- }
-
- bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
- if (bitmap == NULL) {
- goto out;
- }
-
- if (disabled) {
- bdrv_disable_dirty_bitmap(bitmap);
- }
-
- bdrv_dirty_bitmap_set_persistence(bitmap, persistent);
-
-out:
- aio_context_release(aio_context);
-}
-
static BdrvDirtyBitmap *do_block_dirty_bitmap_remove(
const char *node, const char *name, bool release,
BlockDriverState **bitmap_bs, Error **errp)
@@ -2609,75 +2494,6 @@ void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
bdrv_disable_dirty_bitmap(bitmap);
}
-static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(
- const char *node, const char *target,
- BlockDirtyBitmapMergeSourceList *bitmaps,
- HBitmap **backup, Error **errp)
-{
- BlockDriverState *bs;
- BdrvDirtyBitmap *dst, *src, *anon;
- BlockDirtyBitmapMergeSourceList *lst;
- Error *local_err = NULL;
-
- dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
- if (!dst) {
- return NULL;
- }
-
- anon = bdrv_create_dirty_bitmap(bs, bdrv_dirty_bitmap_granularity(dst),
- NULL, errp);
- if (!anon) {
- return NULL;
- }
-
- for (lst = bitmaps; lst; lst = lst->next) {
- switch (lst->value->type) {
- const char *name, *node;
- case QTYPE_QSTRING:
- name = lst->value->u.local;
- src = bdrv_find_dirty_bitmap(bs, name);
- if (!src) {
- error_setg(errp, "Dirty bitmap '%s' not found", name);
- dst = NULL;
- goto out;
- }
- break;
- case QTYPE_QDICT:
- node = lst->value->u.external.node;
- name = lst->value->u.external.name;
- src = block_dirty_bitmap_lookup(node, name, NULL, errp);
- if (!src) {
- dst = NULL;
- goto out;
- }
- break;
- default:
- abort();
- }
-
- bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- dst = NULL;
- goto out;
- }
- }
-
- /* Merge into dst; dst is unchanged on failure. */
- bdrv_merge_dirty_bitmap(dst, anon, backup, errp);
-
- out:
- bdrv_release_dirty_bitmap(anon);
- return dst;
-}
-
-void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
- BlockDirtyBitmapMergeSourceList *bitmaps,
- Error **errp)
-{
- do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
-}
-
BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
const char *name,
Error **errp)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8cbc1fac2bfc..769cd357d281 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1989,6 +1989,7 @@ T: git https://github.com/jnsnow/qemu.git jobs
Block QAPI, monitor, command line
M: Markus Armbruster <armbru@redhat.com>
S: Supported
+F: blockbitmaps.c
F: blockdev.c
F: blockdev-hmp-cmds.c
F: block/qapi.c
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] qemu-img: Add convert --bitmaps option
2020-04-16 14:51 [PATCH 0/3] qemu-img: Add convert --bitmaps Eric Blake
2020-04-16 14:51 ` [PATCH 1/3] blockdev: Split off basic bitmap operations for qemu-img Eric Blake
@ 2020-04-16 14:51 ` Eric Blake
2020-04-16 14:51 ` [PATCH 3/3] iotests: Add test 291 to for qemu-img convert --bitmaps Eric Blake
2020-04-16 18:20 ` [PATCH 0/3] qemu-img: Add " Nir Soffer
3 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2020-04-16 14:51 UTC (permalink / raw)
To: qemu-devel; +Cc: nsoffer, Kevin Wolf, jsnow, qemu-block, Max Reitz
Make it easier to copy all the persistent bitmaps of a source image
along with the contents, by adding a boolean flag for use with
qemu-img convert.
See also https://bugzilla.redhat.com/show_bug.cgi?id=1779893
Signed-off-by: Eric Blake <eblake@redhat.com>
---
docs/tools/qemu-img.rst | 6 ++-
qemu-img.c | 81 +++++++++++++++++++++++++++++++++++++++--
qemu-img-cmds.hx | 4 +-
3 files changed, 85 insertions(+), 6 deletions(-)
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 0080f83a76c9..8c4d85e0b835 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -186,6 +186,10 @@ Parameters to convert subcommand:
.. program:: qemu-img-convert
+.. option:: --bitmaps
+
+ Additionally copy all bitmaps
+
.. option:: -n
Skip the creation of the target volume
@@ -373,7 +377,7 @@ Command description:
4
Error on reading data
-.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
+.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
Convert the disk image *FILENAME* or a snapshot *SNAPSHOT_PARAM*
to disk image *OUTPUT_FILENAME* using format *OUTPUT_FMT*. It can
diff --git a/qemu-img.c b/qemu-img.c
index 821cbf610e5f..6541357179c2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -28,6 +28,7 @@
#include "qemu-common.h"
#include "qemu-version.h"
#include "qapi/error.h"
+#include "qapi/qapi-commands-block-core.h"
#include "qapi/qapi-visit-block-core.h"
#include "qapi/qobject-output-visitor.h"
#include "qapi/qmp/qjson.h"
@@ -71,6 +72,7 @@ enum {
OPTION_SHRINK = 266,
OPTION_SALVAGE = 267,
OPTION_TARGET_IS_ZERO = 268,
+ OPTION_BITMAPS = 269,
};
typedef enum OutputFormat {
@@ -176,6 +178,7 @@ static void QEMU_NORETURN help(void)
" hiding corruption that has already occurred.\n"
"\n"
"Parameters to convert subcommand:\n"
+ " '--bitmaps' copies all persistent bitmaps to destination\n"
" '-m' specifies how many coroutines work in parallel during the convert\n"
" process (defaults to 8)\n"
" '-W' allow to write to the target out of order rather than sequential\n"
@@ -2054,6 +2057,47 @@ static int convert_do_copy(ImgConvertState *s)
return s->ret;
}
+static int convert_copy_bitmaps(BlockDriverState *src, BlockDriverState *dst)
+{
+ BdrvDirtyBitmap *bm;
+ Error *err = NULL;
+ BlockDirtyBitmapMergeSource *merge;
+ BlockDirtyBitmapMergeSourceList *list;
+
+ FOR_EACH_DIRTY_BITMAP(src, bm) {
+ const char *name;
+
+ if (!bdrv_dirty_bitmap_get_persistence(bm)) {
+ continue;
+ }
+ name = bdrv_dirty_bitmap_name(bm);
+ qmp_block_dirty_bitmap_add(dst->node_name, name,
+ true, bdrv_dirty_bitmap_granularity(bm),
+ true, true,
+ true, !bdrv_dirty_bitmap_enabled(bm),
+ &err);
+ if (err) {
+ error_reportf_err(err, "Failed to create bitmap %s: ", name);
+ return -1;
+ }
+
+ merge = g_new0(BlockDirtyBitmapMergeSource, 1);
+ merge->type = QTYPE_QDICT;
+ merge->u.external.node = g_strdup(src->node_name);
+ merge->u.external.name = g_strdup(name);
+ list = g_new0(BlockDirtyBitmapMergeSourceList, 1);
+ list->value = merge;
+ qmp_block_dirty_bitmap_merge(dst->node_name, name, list, &err);
+ qapi_free_BlockDirtyBitmapMergeSourceList(list);
+ if (err) {
+ error_reportf_err(err, "Failed to populate bitmap %s: ", name);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
#define MAX_BUF_SECTORS 32768
static int img_convert(int argc, char **argv)
@@ -2075,6 +2119,8 @@ static int img_convert(int argc, char **argv)
int64_t ret = -EINVAL;
bool force_share = false;
bool explict_min_sparse = false;
+ bool bitmaps = false;
+ size_t nbitmaps = 0;
ImgConvertState s = (ImgConvertState) {
/* Need at least 4k of zeros for sparse detection */
@@ -2094,6 +2140,7 @@ static int img_convert(int argc, char **argv)
{"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
{"salvage", no_argument, 0, OPTION_SALVAGE},
{"target-is-zero", no_argument, 0, OPTION_TARGET_IS_ZERO},
+ {"bitmaps", no_argument, 0, OPTION_BITMAPS},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
@@ -2225,6 +2272,9 @@ static int img_convert(int argc, char **argv)
*/
s.has_zero_init = true;
break;
+ case OPTION_BITMAPS:
+ bitmaps = true;
+ break;
}
}
@@ -2286,7 +2336,6 @@ static int img_convert(int argc, char **argv)
goto fail_getopt;
}
-
/* ret is still -EINVAL until here */
ret = bdrv_parse_cache_mode(src_cache, &src_flags, &src_writethrough);
if (ret < 0) {
@@ -2446,6 +2495,28 @@ static int img_convert(int argc, char **argv)
}
}
+ /* Determine how many bitmaps need copying */
+ if (bitmaps) {
+ BdrvDirtyBitmap *bm;
+
+ if (s.src_num > 1) {
+ error_report("Copying bitmaps only possible with single source");
+ ret = -1;
+ goto out;
+ }
+ FOR_EACH_DIRTY_BITMAP(blk_bs(s.src[0]), bm) {
+ if (bdrv_dirty_bitmap_get_persistence(bm)) {
+ nbitmaps++;
+ }
+ }
+ if (nbitmaps > 0 && drv && !drv->bdrv_co_can_store_new_dirty_bitmap) {
+ error_report("Format driver '%s' does not support bitmaps",
+ out_fmt);
+ ret = -1;
+ goto out;
+ }
+ }
+
/*
* The later open call will need any decryption secrets, and
* bdrv_create() will purge "opts", so extract them now before
@@ -2454,9 +2525,7 @@ static int img_convert(int argc, char **argv)
if (!skip_create) {
open_opts = qdict_new();
qemu_opt_foreach(opts, img_add_key_secrets, open_opts, &error_abort);
- }
- if (!skip_create) {
/* Create the new image */
ret = bdrv_create(drv, out_filename, opts, &local_err);
if (ret < 0) {
@@ -2553,6 +2622,12 @@ static int img_convert(int argc, char **argv)
}
ret = convert_do_copy(&s);
+
+ /* Now copy the bitmaps */
+ if (nbitmaps > 0 && ret == 0) {
+ ret = convert_copy_bitmaps(blk_bs(s.src[0]), out_bs);
+ }
+
out:
if (!ret) {
qemu_progress_print(100, 0);
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index c9c54de1df40..37cb36335218 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -39,9 +39,9 @@ SRST
ERST
DEF("convert", img_convert,
- "convert [--object objectdef] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
+ "convert [--object objectdef] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] [--salvage] filename [filename2 [...]] output_filename")
SRST
-.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-m NUM_COROUTINES] [-W] [--salvage] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
+.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-m NUM_COROUTINES] [-W] [--salvage] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
ERST
DEF("create", img_create,
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] iotests: Add test 291 to for qemu-img convert --bitmaps
2020-04-16 14:51 [PATCH 0/3] qemu-img: Add convert --bitmaps Eric Blake
2020-04-16 14:51 ` [PATCH 1/3] blockdev: Split off basic bitmap operations for qemu-img Eric Blake
2020-04-16 14:51 ` [PATCH 2/3] qemu-img: Add convert --bitmaps option Eric Blake
@ 2020-04-16 14:51 ` Eric Blake
2020-04-16 18:20 ` [PATCH 0/3] qemu-img: Add " Nir Soffer
3 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2020-04-16 14:51 UTC (permalink / raw)
To: qemu-devel; +Cc: nsoffer, Kevin Wolf, jsnow, qemu-block, Max Reitz
Add a new test covering the feature added in the previous patch.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/qemu-iotests/291 | 143 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/291.out | 56 +++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 200 insertions(+)
create mode 100755 tests/qemu-iotests/291
create mode 100644 tests/qemu-iotests/291.out
diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/291
new file mode 100755
index 000000000000..dfdcc8e352c8
--- /dev/null
+++ b/tests/qemu-iotests/291
@@ -0,0 +1,143 @@
+#!/usr/bin/env bash
+#
+# Test qemu-img convert --bitmaps
+#
+# Copyright (C) 2018-2020 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, see <http://www.gnu.org/licenses/>.
+#
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+ nbd_server_stop
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.nbd
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+_require_command QEMU_NBD
+
+do_run_qemu()
+{
+ echo Testing: "$@"
+ $QEMU -nographic -qmp stdio -serial none "$@"
+ echo
+}
+
+run_qemu()
+{
+ do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp \
+ | _filter_qemu | _filter_imgfmt \
+ | _filter_actual_image_size | _filter_qemu_io
+}
+
+# Create initial image and populate two bitmaps: one active, one inactive
+_make_test_img 10M
+run_qemu <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "blockdev-add",
+ "arguments": {
+ "driver": "$IMGFMT",
+ "node-name": "n",
+ "file": {
+ "driver": "file",
+ "filename": "$TEST_IMG"
+ }
+ }
+}
+{ "execute": "human-monitor-command",
+ "arguments": {
+ "command-line": 'qemu-io n "write 0 1M"'
+ }
+}
+{ "execute": "block-dirty-bitmap-add",
+ "arguments": {
+ "node": "n",
+ "name": "b1",
+ "persistent": true,
+ "granularity": 65536
+ }
+}
+{ "execute": "human-monitor-command",
+ "arguments": {
+ "command-line": 'qemu-io n "write 1M 1M"'
+ }
+}
+{ "execute": "transaction",
+ "arguments": {
+ "actions": [
+ { "type": "block-dirty-bitmap-disable",
+ "data": {
+ "node": "n",
+ "name": "b1"
+ } },
+ { "type": "block-dirty-bitmap-add",
+ "data": {
+ "node": "n",
+ "name": "b2",
+ "persistent": true
+ } }
+ ]
+ }
+}
+{ "execute": "human-monitor-command",
+ "arguments": {
+ "command-line": 'qemu-io n "write 2M 1M"'
+ }
+}
+{ "execute": "quit" }
+EOF
+
+echo
+echo "=== Bitmap preservation not possible to non-qcow2 ==="
+echo
+
+mv "$TEST_IMG" "$TEST_IMG.orig"
+$QEMU_IMG convert --bitmaps -O raw "$TEST_IMG.orig" "$TEST_IMG"
+
+echo
+echo "=== convert with bitmap preservation ==="
+echo
+
+$QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG.orig" "$TEST_IMG"
+$QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific
+
+echo
+echo "=== check bitmap contents ==="
+echo
+
+IMG="driver=nbd,server.type=unix,server.path=$nbd_unix_socket"
+nbd_server_start_unix_socket -r -f qcow2 -B b1 "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+ "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b1" | _filter_qemu_img_map
+nbd_server_start_unix_socket -r -f qcow2 -B b2 "$TEST_IMG"
+$QEMU_IMG map --output=json --image-opts \
+ "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b2" | _filter_qemu_img_map
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
new file mode 100644
index 000000000000..3e826f93ca82
--- /dev/null
+++ b/tests/qemu-iotests/291.out
@@ -0,0 +1,56 @@
+QA output created by 291
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=10485760
+Testing:
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+{"return": {}}
+wrote 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+{"return": {}}
+wrote 1048576/1048576 bytes at offset 2097152
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": ""}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
+
+
+=== Bitmap preservation not possible to non-qcow2 ===
+
+qemu-img: Format driver 'raw' does not support bitmaps
+
+=== convert with bitmap preservation ===
+
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 10 MiB (10485760 bytes)
+disk size: 3.39 MiB
+Format specific information:
+ compat: 1.1
+ lazy refcounts: false
+ bitmaps:
+ [0]:
+ flags:
+ [0]: auto
+ name: b2
+ granularity: 65536
+ [1]:
+ flags:
+ name: b1
+ granularity: 65536
+ refcount bits: 16
+ corrupt: false
+
+=== check bitmap contents ===
+
+[{ "start": 0, "length": 1048576, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 1048576, "length": 1048576, "depth": 0, "zero": false, "data": false},
+{ "start": 2097152, "length": 8388608, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+[{ "start": 0, "length": 2097152, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 2097152, "length": 1048576, "depth": 0, "zero": false, "data": false},
+{ "start": 3145728, "length": 7340032, "depth": 0, "zero": false, "data": true, "offset": OFFSET}]
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 435dccd5af90..8e9b9513a091 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -297,3 +297,4 @@
288 quick
289 rw quick
290 rw auto quick
+291 rw quick
--
2.26.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] qemu-img: Add convert --bitmaps
2020-04-16 14:51 [PATCH 0/3] qemu-img: Add convert --bitmaps Eric Blake
` (2 preceding siblings ...)
2020-04-16 14:51 ` [PATCH 3/3] iotests: Add test 291 to for qemu-img convert --bitmaps Eric Blake
@ 2020-04-16 18:20 ` Nir Soffer
2020-04-16 19:31 ` Eric Blake
3 siblings, 1 reply; 6+ messages in thread
From: Nir Soffer @ 2020-04-16 18:20 UTC (permalink / raw)
To: Eric Blake
Cc: Eyal Shenitzky, John Snow, QEMU Developers, qemu-block, Benny Zlotnik
On Thu, Apr 16, 2020 at 5:51 PM Eric Blake <eblake@redhat.com> wrote:
>
> Without this series, the process for copying one qcow2 image to
> another including all of its bitmaps involves running qemu and doing
> the copying by hand with a series of QMP commands. This makes the
> process a bit more convenient.
This seems good for copying an image chain from one storage to another,
but I think we need a similar --bitmaps option to qemu-img measure to make
this really useful.
Here is example use case showing how qemu-img measure is related:
Source chain:
/dev/vg1/base
/dev/vg1/top
Destination chain:
/dev/vg2/base
/dev/vg2/top
We create empty lvs with the same name on destination storage (/dev/vg2).
We measure the base lv using qemu-img measure for creating the target lv:
qemu-img measure -f qcow2 -O qcow2 /dev/vg1/base
lvcreate -L required_size /dev/vg2/base
qemu-img create -f qcow2 /dev/vg2/base 10g
For the top lv we use the current size of the source lv - I think we
should measure it instead but
I'm not sure if qemu-img measure supports measuring a single image in a chain
(maybe -o backing_file?).
lvcreate -L current_size /dev/vg2/top
qemu-img create -f qcow2 -b /dev/vg2/base -F qcow2 /dev/vg2/top 10g
And then convert the lvs one by one:
qemu-img convert -f qcow2 -O qcow2 -n --bitmaps /dev/vg1/base /dev/vg2/base
qemu-img convert -f qcow2 -O qcow2 -n --bitmaps -B /dev/vg2/base
/dev/vg1/top /dev/vg2/top
The first copy may fail with ENOSPC since qemu-img measure of the base
does not consider the
bitmaps in the required size.
So I think we need to add a similar --bitmaps option to qemu-img
measure, hopefully reusing the
same code to find and estimate the size of the bitmaps.
Maybe we can estimate the size using qemu-img info --bitmaps, but I
think the right way to
do this is in qemu-img measure.
We have also another use case when we collapsed an image chain to single image:
Source chain:
/dev/vg1/base
/dev/vg1/top
Destination:
/dev/vg2/collapsed
In this case we measure the size of the entire chain (/dev/vg1/base <-
/dev/vg1/top) and create
/dev/vg2/collapsed in the correct size, and then we convert the chain using:
qemu-img convert /dev/vg1/top /dev/vg2/collapsed
Currently we use this for exporting images, for example when creating
templates, or as a simple
backup. In this case we don't need to copy the bitmaps in the target
image - this is a new image
not used by any VM. Copying the bitmaps may also be non-trivial since
we may have the bitmaps
with the same names in several layers (e.g. result of live snapshot).
So I think using --bitmaps should be disabled when doing this kind of
convert. We can handle this
on our side easily, but I think this should fail or log a warning on
qemu-img, or require merging of
bitmaps with same names during the copy. I did not check if you
already handle this.
Finally we also have a use case when we copy the chain as is to new or
same storage, but
we create a new vm. In this case I don't think the backup history
makes sense for the new
vm, so we don't need to copy the bitmaps.
I will review the rest of the patches next week and can maybe give
this some testing.
Nir
> I still think that someday we will need a 'qemu-img bitmap' with
> various subcommands for manipulating bitmaps within an offline image,
> but in the meantime, this seems like a useful addition on its own.
>
> Series can also be downloaded at:
> https://repo.or.cz/qemu/ericb.git/shortlog/refs/tags/qemu-img-bitmaps-v1
>
> Eric Blake (3):
> blockdev: Split off basic bitmap operations for qemu-img
> qemu-img: Add convert --bitmaps option
> iotests: Add test 291 to for qemu-img convert --bitmaps
>
> docs/tools/qemu-img.rst | 6 +-
> Makefile.objs | 2 +-
> include/sysemu/blockdev.h | 10 ++
> blockbitmaps.c | 217 +++++++++++++++++++++++++++++++++++++
> blockdev.c | 184 -------------------------------
> qemu-img.c | 81 +++++++++++++-
> MAINTAINERS | 1 +
> qemu-img-cmds.hx | 4 +-
> tests/qemu-iotests/291 | 143 ++++++++++++++++++++++++
> tests/qemu-iotests/291.out | 56 ++++++++++
> tests/qemu-iotests/group | 1 +
> 11 files changed, 514 insertions(+), 191 deletions(-)
> create mode 100644 blockbitmaps.c
> create mode 100755 tests/qemu-iotests/291
> create mode 100644 tests/qemu-iotests/291.out
>
> --
> 2.26.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] qemu-img: Add convert --bitmaps
2020-04-16 18:20 ` [PATCH 0/3] qemu-img: Add " Nir Soffer
@ 2020-04-16 19:31 ` Eric Blake
0 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2020-04-16 19:31 UTC (permalink / raw)
To: Nir Soffer
Cc: Benny Zlotnik, qemu-block, Markus Armbruster, QEMU Developers,
Eyal Shenitzky, John Snow
(adding Markus for a CLI question, look for [*])
On 4/16/20 1:20 PM, Nir Soffer wrote:
> On Thu, Apr 16, 2020 at 5:51 PM Eric Blake <eblake@redhat.com> wrote:
>>
>> Without this series, the process for copying one qcow2 image to
>> another including all of its bitmaps involves running qemu and doing
>> the copying by hand with a series of QMP commands. This makes the
>> process a bit more convenient.
>
> This seems good for copying an image chain from one storage to another,
> but I think we need a similar --bitmaps option to qemu-img measure to make
> this really useful.
>
> Here is example use case showing how qemu-img measure is related:
>
> Source chain:
> /dev/vg1/base
> /dev/vg1/top
>
> Destination chain:
> /dev/vg2/base
> /dev/vg2/top
>
> We create empty lvs with the same name on destination storage (/dev/vg2).
>
> We measure the base lv using qemu-img measure for creating the target lv:
>
> qemu-img measure -f qcow2 -O qcow2 /dev/vg1/base
> lvcreate -L required_size /dev/vg2/base
> qemu-img create -f qcow2 /dev/vg2/base 10g
>
> For the top lv we use the current size of the source lv - I think we
> should measure it instead but
> I'm not sure if qemu-img measure supports measuring a single image in a chain
> (maybe -o backing_file?).
qemu-measure --image-opts should be able to measure a single image by
specifying image opts that purposefully treat the image as standalone
rather than with its normal backing file included. Let's see if I can
whip up an example:
$ qemu-img create -f qcow2 img.base 100M
Formatting 'img.base', fmt=qcow2 size=104857600 cluster_size=65536
lazy_refcounts=off refcount_bits=16
$ qemu-io -f qcow2 -c 'w 0 25m' img.base
wrote 26214400/26214400 bytes at offset 0
25 MiB, 1 ops; 00.24 sec (103.405 MiB/sec and 4.1362 ops/sec)
$ qemu-img create -f qcow2 -F qcow2 -b img.base img.top
Formatting 'img.top', fmt=qcow2 size=104857600 backing_file=img.base
backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ qemu-io -f qcow2 -c 'w 25m 25m' img.top
wrote 26214400/26214400 bytes at offset 26214400
25 MiB, 1 ops; 00.24 sec (103.116 MiB/sec and 4.1247 ops/sec)
$ qemu-img measure -f qcow2 -O qcow2 img.base
required size: 26542080
fully allocated size: 105185280
required size: 52756480
fully allocated size: 105185280
Okay, I can reproduce what you are seeing - measuring the top image
defaults to measuring the full allocation of the entire chain, rather
than the allocation of just the top image. And now with --image-opts to
the rescue:
$ qemu-img measure --image-opts -O qcow2
driver=qcow2,backing=,file.driver=file,file.filename=img.top
qemu-img: warning: Use of "backing": "" is deprecated; use "backing":
null instead
required size: 26542080
fully allocated size: 105185280
There you go - by forcing qemu to treat the overlay image as though it
had no backing, you can then measure that image in isolation.
(*) Hmm - that warning about backing="" being deprecated is annoying,
but I don't know any other way to use dotted command line syntax and
still express that we want a QMP null. I tried to see if I could inject
an alternative backing driver, such as null-co, but was met with errors:
$ ./qemu-img measure --image-opts -O qcow2
driver=qcow2,backing.driver=null-co,file.driver=file,file.filename=img.top
qemu-img: Could not open
'driver=qcow2,backing.driver=null-co,file.driver=file,file.filename=img.top':
Could not open backing file: The only allowed filename for this driver
is 'null-co://'
$ ./qemu-img measure --image-opts -O qcow2
driver=qcow2,backing.driver=null-co,backing.file=null-co://,file.driver=file,file.filename=img.top
qemu-img: Could not open
'driver=qcow2,backing.driver=null-co,backing.file=null-co://,file.driver=file,file.filename=img.top':
Could not open backing file: The only allowed filename for this driver
is 'null-co://'
$ ./qemu-img measure --image-opts -O qcow2
driver=qcow2,backing.driver=null-co,backing.file.filename=null-co://,file.driver=file,file.filename=img.top
qemu-img: Could not open
'driver=qcow2,backing.driver=null-co,backing.file.filename=null-co://,file.driver=file,file.filename=img.top':
Could not open backing file: Block protocol 'null-co' doesn't support
the option 'file.filename'
We don't want to support "" in the QMP syntax forever, but if the CLI
syntax has to handle the empty string specially in order to get null
passed to the QMP code, then so be it.
I also tried, but failed, to use JSON syntax. I don't know why we
haven't wired up --image-opts to use JSON syntax yet.
$ qemu-img measure --image-opts -O qcow2 '{"driver":"qcow2", "backing":null,
"file":{"driver":"file", "filename":"img.top"}}'
qemu-img: Could not open '{"driver":"qcow2", "backing":null,
"file":{"driver":"file", "filename":"img.top"}}': Cannot find
device={"driver":"qcow2" nor node_name={"driver":"qcow2"
I guess there's always the pseudo-json protocol:
$ qemu-img measure -O qcow2 'json:{"driver":"qcow2", "backing":null,
"file":{"driver":"file", "filename":"img.top"}}'
required size: 26542080
fully allocated size: 105185280
>
> lvcreate -L current_size /dev/vg2/top
> qemu-img create -f qcow2 -b /dev/vg2/base -F qcow2 /dev/vg2/top 10g
>
> And then convert the lvs one by one:
>
> qemu-img convert -f qcow2 -O qcow2 -n --bitmaps /dev/vg1/base /dev/vg2/base
> qemu-img convert -f qcow2 -O qcow2 -n --bitmaps -B /dev/vg2/base
> /dev/vg1/top /dev/vg2/top
>
> The first copy may fail with ENOSPC since qemu-img measure of the base
> does not consider the
> bitmaps in the required size.
Yes, that's a good argument for adding 'qemu-img measure --bitmaps'. In
the meantime...
>
> So I think we need to add a similar --bitmaps option to qemu-img
> measure, hopefully reusing the
> same code to find and estimate the size of the bitmaps.
>
> Maybe we can estimate the size using qemu-img info --bitmaps,
...you are correct, this works today. Well, 'qemu-img info --bitmaps'
doesn't exist, but 'qemu-img info' does output the number of bitmaps, as
well as their granularity, and if you also assume that each bitmap is
sized to match the virtual image size, you can compute the estimated
space occupied by those bitmaps.
> but I
> think the right way to
> do this is in qemu-img measure.
Yes, even though we have existing multi-step code, being able to do it
in a single step is justification for the improvement (although you may
still end up having to code the multi-step mode yourself if you can't
guarantee new-enough qemu-img - the addition of 'qemu-img measure
--bitmaps' won't land until at least 4 months from now with qemu 5.1).
Not to mention that the estimation computation (image size / granularity
rounded up to the next cluster size, summed over all bitmaps) is hairy
enough that it shouldn't have to be reimplemented by multiple layers of
software.
>
> We have also another use case when we collapsed an image chain to single image:
>
> Source chain:
> /dev/vg1/base
> /dev/vg1/top
>
> Destination:
> /dev/vg2/collapsed
>
> In this case we measure the size of the entire chain (/dev/vg1/base <-
> /dev/vg1/top) and create
> /dev/vg2/collapsed in the correct size, and then we convert the chain using:
>
> qemu-img convert /dev/vg1/top /dev/vg2/collapsed
When I submitted my v1 patch, I had only tested with a single source
image to a single destination. But now that you mention flattening,
it's easy enough for me to test of what happens (tl;dr: my patch only
looks at bitmaps in the active layer of the source):
$ qemu-img create -f qcow2 img.base 100M
Formatting 'img.base', fmt=qcow2 size=104857600 cluster_size=65536
lazy_refcounts=off refcount_bits=16
$ qemu-img create -f qcow2 -F qcow2 -b img.base img.top
Formatting 'img.top', fmt=qcow2 size=104857600 backing_file=img.base
backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ qemu-kvm --nographic --nodefaults --qmp stdio
{'execute':'qmp_capabilities'}
{'execute':'blockdev-add','arguments':{'driver':'qcow2',
'node-name':'base','file':{'driver':'file','filename':'img.base'}}}
{'execute':'blockdev-add','arguments':{'driver':'qcow2',
'node-name':'top','file':{'driver':'file','filename':'img.top'},
'backing':'base'}}
{'execute':'block-dirty-bitmap-add','arguments':{'node':'base',
'name':'b1','persistent':true}}
{'execute':'block-dirty-bitmap-add','arguments':{'node':'base',
'name':'b2','persistent':true}}
{'execute':'block-dirty-bitmap-add','arguments':{'node':'top',
'name':'b2','persistent':true}}
{'execute':'block-dirty-bitmap-add','arguments':{'node':'top',
'name':'b3','persistent':true}}
{'execute':'quit'}
$ qemu-img convert -f qcow2 -O qcow2 --bitmaps img.top img.flat
$ qemu-img info img.flat
image: img.flat
file format: qcow2
virtual size: 100 MiB (104857600 bytes)
disk size: 208 KiB
cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: false
bitmaps:
[0]:
flags:
[0]: auto
name: b3
granularity: 65536
[1]:
flags:
[0]: auto
name: b2
granularity: 65536
refcount bits: 16
corrupt: false
It only copied the bitmaps from the active layer, not all bitmaps from
all layers. If we want more complicated handling (such as whether to
pull in bitmaps deeper in the backing chain, rename bitmaps, policies on
merging duplicated bitmap names across nodes vs. failure, etc), we
really should be designing 'qemu-img bitmap' that gives full
command-line control over all sorts of bitmap operations, rather than
trying to further overload 'qemu-img convert'.
>
> Currently we use this for exporting images, for example when creating
> templates, or as a simple
> backup. In this case we don't need to copy the bitmaps in the target
> image - this is a new image
> not used by any VM. Copying the bitmaps may also be non-trivial since
> we may have the bitmaps
> with the same names in several layers (e.g. result of live snapshot).
>
> So I think using --bitmaps should be disabled when doing this kind of
> convert. We can handle this
> on our side easily, but I think this should fail or log a warning on
> qemu-img, or require merging of
> bitmaps with same names during the copy. I did not check if you
> already handle this.
My patch only copies top-most bitmaps. If you are using qemu-img
convert to flatten a chain, we'll need something else to control what to
do with bitmaps across that chain.
>
> Finally we also have a use case when we copy the chain as is to new or
> same storage, but
> we create a new vm. In this case I don't think the backup history
> makes sense for the new
> vm, so we don't need to copy the bitmaps.
>
> I will review the rest of the patches next week and can maybe give
> this some testing.
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-16 19:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-16 14:51 [PATCH 0/3] qemu-img: Add convert --bitmaps Eric Blake
2020-04-16 14:51 ` [PATCH 1/3] blockdev: Split off basic bitmap operations for qemu-img Eric Blake
2020-04-16 14:51 ` [PATCH 2/3] qemu-img: Add convert --bitmaps option Eric Blake
2020-04-16 14:51 ` [PATCH 3/3] iotests: Add test 291 to for qemu-img convert --bitmaps Eric Blake
2020-04-16 18:20 ` [PATCH 0/3] qemu-img: Add " Nir Soffer
2020-04-16 19:31 ` Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.