[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 4/5] qemu-img: Add convert --bitmaps option
From: |
Eric Blake |
Subject: |
Re: [PATCH v6 4/5] qemu-img: Add convert --bitmaps option |
Date: |
Tue, 26 May 2020 11:27:40 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
On 5/25/20 2:51 AM, Vladimir Sementsov-Ogievskiy wrote:
21.05.2020 22:21, Eric Blake wrote:
Make it easier to copy all the persistent bitmaps of (the top layer
of) a source image along with its guest-visible contents, by adding a
boolean flag for use with qemu-img convert. This is basically
shorthand, as the same effect could be accomplished with a series of
'qemu-img bitmap --add' and 'qemu-img bitmap --merge -b source'
commands, or by their corresponding QMP commands.
Note that this command will fail in the same scenarios where 'qemu-img
measure' omits a 'bitmaps size:' line, namely, when either the source
or the destination lacks persistent bitmap support altogether.
See also https://bugzilla.redhat.com/show_bug.cgi?id=1779893
While touching this, clean up a couple coding issues spotted in the
same function: an extra blank line, and merging back-to-back 'if
(!skip_create)' blocks.
Signed-off-by: Eric Blake <address@hidden>
---
@@ -2573,6 +2632,13 @@ static int img_convert(int argc, char **argv)
}
out_bs = blk_bs(s.target);
+ if (nbitmaps > 0 &&
!bdrv_supports_persistent_dirty_bitmap(out_bs)) {
We will not fail, if target doesn't support bitmaps, source supports
them but has no bitmaps? Doesn't seem to be a problem, but a bit less
strict than you write in commit message.
So, maybe, s/nbitmaps > 0/bitmaps/
In fact, nbitmaps is not needed at all (it was useful in earlier
iterations, but as the series has morphed, it is no longer buying me
anything useful).
+ error_report("Format driver '%s' does not support bitmaps",
+ out_fmt);
Hmm seems, out_fmt may be NULL at this point, consider the path:
const char *out_fmt = NULL
...
[no -O option]
--target-image-opts, so out_fmt doesn't default to "raw" but remains NULL
...
So, with s/out_fmt/out_bs->drv->format_name/ (and with or without
s/nbitmaps > 0/bitmaps/):
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Okay, I'm squashing this in, and adding your R-b. Pull request coming
shortly.
diff --git i/qemu-img.c w/qemu-img.c
index 8ecebe178890..d7e846e60742 100644
--- i/qemu-img.c
+++ w/qemu-img.c
@@ -2196,7 +2196,6 @@ static int img_convert(int argc, char **argv)
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 */
@@ -2565,10 +2564,8 @@ static int img_convert(int argc, char **argv)
}
}
- /* Determine how many bitmaps need copying */
+ /* Determine if bitmaps need copying */
if (bitmaps) {
- BdrvDirtyBitmap *bm;
-
if (s.src_num > 1) {
error_report("Copying bitmaps only possible with single
source");
ret = -1;
@@ -2579,11 +2576,6 @@ static int img_convert(int argc, char **argv)
ret = -1;
goto out;
}
- FOR_EACH_DIRTY_BITMAP(blk_bs(s.src[0]), bm) {
- if (bdrv_dirty_bitmap_get_persistence(bm)) {
- nbitmaps++;
- }
- }
}
/*
@@ -2632,9 +2624,9 @@ static int img_convert(int argc, char **argv)
}
out_bs = blk_bs(s.target);
- if (nbitmaps > 0 && !bdrv_supports_persistent_dirty_bitmap(out_bs)) {
+ if (bitmaps && !bdrv_supports_persistent_dirty_bitmap(out_bs)) {
error_report("Format driver '%s' does not support bitmaps",
- out_fmt);
+ out_bs->drv->format_name);
ret = -1;
goto out;
}
@@ -2700,7 +2692,7 @@ static int img_convert(int argc, char **argv)
ret = convert_do_copy(&s);
/* Now copy the bitmaps */
- if (nbitmaps > 0 && ret == 0) {
+ if (bitmaps && ret == 0) {
ret = convert_copy_bitmaps(blk_bs(s.src[0]), out_bs);
}
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- [PATCH v6 0/5] qemu-img: Add convert --bitmaps, Eric Blake, 2020/05/21
- [PATCH v6 1/5] iotests: Fix test 178, Eric Blake, 2020/05/21
- [PATCH v6 2/5] qcow2: Expose bitmaps' size during measure, Eric Blake, 2020/05/21
- [PATCH v6 3/5] qemu-img: Factor out code for merging bitmaps, Eric Blake, 2020/05/21
- [PATCH v6 4/5] qemu-img: Add convert --bitmaps option, Eric Blake, 2020/05/21
- [PATCH v6 5/5] iotests: Add test 291 to for qemu-img bitmap coverage, Eric Blake, 2020/05/21