From c5a2313ba173568087d78f76cc0258e7a353830b Mon Sep 17 00:00:00 2001

From: Eric Blake <eblake@redhat.com>

Date: Fri, 6 Aug 2021 15:07:49 -0400

Subject: [PATCH 26/39] qemu-img: Add --skip-broken-bitmaps for 'convert

 --bitmaps'

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

RH-Author: Miroslav Rezanina <mrezanin@redhat.com>

RH-MergeRequest: 35: Synchronize with RHEL-AV 8.5 release 28 to RHEL 9

RH-Commit: [3/4] 4b7203c66367c601f9710bbcd91bdbdd56f0f8bd (mrezanin/centos-src-qemu-kvm)

RH-Bugzilla: 1957194

RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>

The point of 'qemu-img convert --bitmaps' is to be a convenience for

actions that are already possible through a string of smaller

'qemu-img bitmap' sub-commands.  One situation not accounted for

already is that if a source image contains an inconsistent bitmap (for

example, because a qemu process died abruptly before flushing bitmap

state), the user MUST delete those inconsistent bitmaps before

anything else useful can be done with the image.

We don't want to delete inconsistent bitmaps by default: although a

corrupt bitmap is only a loss of optimization rather than a corruption

of user-visible data, it is still nice to require the user to opt in

to the fact that they are aware of the loss of the bitmap.  Still,

requiring the user to check 'qemu-img info' to see whether bitmaps are

consistent, then use 'qemu-img bitmap --remove' to remove offenders,

all before using 'qemu-img convert', is a lot more work than just

adding a knob 'qemu-img convert --bitmaps --skip-broken-bitmaps' which

opts in to skipping the broken bitmaps.

After testing the new option, also demonstrate the way to manually fix

things (either deleting bad bitmaps, or re-creating them as empty) so

that it is possible to convert without the option.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1946084

Signed-off-by: Eric Blake <eblake@redhat.com>

Message-Id: <20210709153951.2801666-4-eblake@redhat.com>

[eblake: warning message tweak, test enhancements]

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

(cherry picked from commit 955171e4417bf39edb5503e694501e082a757731)

Signed-off-by: Eric Blake <eblake@redhat.com>

Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 docs/tools/qemu-img.rst                       |  8 ++++-

 qemu-img.c                                    | 29 +++++++++++----

 tests/qemu-iotests/tests/qemu-img-bitmaps     | 16 ++++++++-

 tests/qemu-iotests/tests/qemu-img-bitmaps.out | 35 ++++++++++++++++++-

 4 files changed, 79 insertions(+), 9 deletions(-)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst

index c9efcfaefc..3df6277d6a 100644

--- a/docs/tools/qemu-img.rst

+++ b/docs/tools/qemu-img.rst

@@ -414,7 +414,7 @@ Command description:

   4

     Error on reading data

-.. 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] [-r RATE_LIMIT] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME

+.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps [--skip-broken-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] [-r RATE_LIMIT] [-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

@@ -456,6 +456,12 @@ Command description:

   *NUM_COROUTINES* specifies how many coroutines work in parallel during

   the convert process (defaults to 8).

+  Use of ``--bitmaps`` requests that any persistent bitmaps present in

+  the original are also copied to the destination.  If any bitmap is

+  inconsistent in the source, the conversion will fail unless

+  ``--skip-broken-bitmaps`` is also specified to copy only the

+  consistent bitmaps.

+

 .. option:: create [--object OBJECTDEF] [-q] [-f FMT] [-b BACKING_FILE] [-F BACKING_FMT] [-u] [-o OPTIONS] FILENAME [SIZE]

   Create the new disk image *FILENAME* of size *SIZE* and format

diff --git a/qemu-img.c b/qemu-img.c

index 7684684bfa..75bab32416 100644

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -82,6 +82,7 @@ enum {

     OPTION_MERGE = 274,

     OPTION_BITMAPS = 275,

     OPTION_FORCE = 276,

+    OPTION_SKIP_BROKEN = 277,

 };

 typedef enum OutputFormat {

@@ -2099,7 +2100,7 @@ static int convert_do_copy(ImgConvertState *s)

 }

 /* Check that bitmaps can be copied, or output an error */

-static int convert_check_bitmaps(BlockDriverState *src)

+static int convert_check_bitmaps(BlockDriverState *src, bool skip_broken)

 {

     BdrvDirtyBitmap *bm;

@@ -2111,17 +2112,19 @@ static int convert_check_bitmaps(BlockDriverState *src)

         if (!bdrv_dirty_bitmap_get_persistence(bm)) {

             continue;

         }

-        if (bdrv_dirty_bitmap_inconsistent(bm)) {

+        if (!skip_broken && bdrv_dirty_bitmap_inconsistent(bm)) {

             error_report("Cannot copy inconsistent bitmap '%s'",

                          bdrv_dirty_bitmap_name(bm));

-            error_printf("Try 'qemu-img bitmap --remove' to delete it\n");

+            error_printf("Try --skip-broken-bitmaps, or "

+                         "use 'qemu-img bitmap --remove' to delete it\n");

             return -1;

         }

     }

     return 0;

 }

-static int convert_copy_bitmaps(BlockDriverState *src, BlockDriverState *dst)

+static int convert_copy_bitmaps(BlockDriverState *src, BlockDriverState *dst,

+                                bool skip_broken)

 {

     BdrvDirtyBitmap *bm;

     Error *err = NULL;

@@ -2133,6 +2136,10 @@ static int convert_copy_bitmaps(BlockDriverState *src, BlockDriverState *dst)

             continue;

         }

         name = bdrv_dirty_bitmap_name(bm);

+        if (skip_broken && bdrv_dirty_bitmap_inconsistent(bm)) {

+            warn_report("Skipping inconsistent bitmap '%s'", name);

+            continue;

+        }

         qmp_block_dirty_bitmap_add(dst->node_name, name,

                                    true, bdrv_dirty_bitmap_granularity(bm),

                                    true, true,

@@ -2188,6 +2195,7 @@ static int img_convert(int argc, char **argv)

     bool force_share = false;

     bool explict_min_sparse = false;

     bool bitmaps = false;

+    bool skip_broken = false;

     int64_t rate_limit = 0;

     ImgConvertState s = (ImgConvertState) {

@@ -2209,6 +2217,7 @@ static int img_convert(int argc, char **argv)

             {"salvage", no_argument, 0, OPTION_SALVAGE},

             {"target-is-zero", no_argument, 0, OPTION_TARGET_IS_ZERO},

             {"bitmaps", no_argument, 0, OPTION_BITMAPS},

+            {"skip-broken-bitmaps", no_argument, 0, OPTION_SKIP_BROKEN},

             {0, 0, 0, 0}

         };

         c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WUr:",

@@ -2337,6 +2346,9 @@ static int img_convert(int argc, char **argv)

         case OPTION_BITMAPS:

             bitmaps = true;

             break;

+        case OPTION_SKIP_BROKEN:

+            skip_broken = true;

+            break;

         }

     }

@@ -2344,6 +2356,11 @@ static int img_convert(int argc, char **argv)

         out_fmt = "raw";

     }

+    if (skip_broken && !bitmaps) {

+        error_report("Use of --skip-broken-bitmaps requires --bitmaps");

+        goto fail_getopt;

+    }

+

     if (s.compressed && s.copy_range) {

         error_report("Cannot enable copy offloading when -c is used");

         goto fail_getopt;

@@ -2573,7 +2590,7 @@ static int img_convert(int argc, char **argv)

             ret = -1;

             goto out;

         }

-        ret = convert_check_bitmaps(blk_bs(s.src[0]));

+        ret = convert_check_bitmaps(blk_bs(s.src[0]), skip_broken);

         if (ret < 0) {

             goto out;

         }

@@ -2698,7 +2715,7 @@ static int img_convert(int argc, char **argv)

     /* Now copy the bitmaps */

     if (bitmaps && ret == 0) {

-        ret = convert_copy_bitmaps(blk_bs(s.src[0]), out_bs);

+        ret = convert_copy_bitmaps(blk_bs(s.src[0]), out_bs, skip_broken);

     }

 out:

diff --git a/tests/qemu-iotests/tests/qemu-img-bitmaps b/tests/qemu-iotests/tests/qemu-img-bitmaps

index 09c3d395d1..7a3fe8c3d3 100755

--- a/tests/qemu-iotests/tests/qemu-img-bitmaps

+++ b/tests/qemu-iotests/tests/qemu-img-bitmaps

@@ -144,7 +144,21 @@ _img_info --format-specific | _filter_irrelevant_img_info

 echo

 $QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG" "$TEST_IMG.copy" &&

     echo "unexpected success"

-TEST_IMG=$TEST_IMG.copy _img_info --format-specific \

+TEST_IMG="$TEST_IMG.copy" _img_info --format-specific \

+    | _filter_irrelevant_img_info

+# Skipping the broken bitmaps works,...

+echo

+$QEMU_IMG convert --bitmaps --skip-broken-bitmaps \

+    -O qcow2 "$TEST_IMG" "$TEST_IMG.copy"

+TEST_IMG="$TEST_IMG.copy" _img_info --format-specific \

+    | _filter_irrelevant_img_info

+# ...as does removing them

+echo

+_rm_test_img "$TEST_IMG.copy"

+$QEMU_IMG bitmap --remove "$TEST_IMG" b0

+$QEMU_IMG bitmap --remove --add "$TEST_IMG" b2

+$QEMU_IMG convert --bitmaps -O qcow2 "$TEST_IMG" "$TEST_IMG.copy"

+TEST_IMG="$TEST_IMG.copy" _img_info --format-specific \

     | _filter_irrelevant_img_info

 # success, all done

diff --git a/tests/qemu-iotests/tests/qemu-img-bitmaps.out b/tests/qemu-iotests/tests/qemu-img-bitmaps.out

index 1e32833bf1..7a7429e320 100644

--- a/tests/qemu-iotests/tests/qemu-img-bitmaps.out

+++ b/tests/qemu-iotests/tests/qemu-img-bitmaps.out

@@ -145,6 +145,39 @@ Format specific information:

     corrupt: false

 qemu-img: Cannot copy inconsistent bitmap 'b0'

-Try 'qemu-img bitmap --remove' to delete it

+Try --skip-broken-bitmaps, or use 'qemu-img bitmap --remove' to delete it

 qemu-img: Could not open 'TEST_DIR/t.IMGFMT.copy': Could not open 'TEST_DIR/t.IMGFMT.copy': No such file or directory

+

+qemu-img: warning: Skipping inconsistent bitmap 'b0'

+qemu-img: warning: Skipping inconsistent bitmap 'b2'

+image: TEST_DIR/t.IMGFMT.copy

+file format: IMGFMT

+virtual size: 10 MiB (10485760 bytes)

+cluster_size: 65536

+Format specific information:

+    bitmaps:

+        [0]:

+            flags:

+                [0]: auto

+            name: b4

+            granularity: 65536

+    corrupt: false

+

+image: TEST_DIR/t.IMGFMT.copy

+file format: IMGFMT

+virtual size: 10 MiB (10485760 bytes)

+cluster_size: 65536

+Format specific information:

+    bitmaps:

+        [0]:

+            flags:

+                [0]: auto

+            name: b4

+            granularity: 65536

+        [1]:

+            flags:

+                [0]: auto

+            name: b2

+            granularity: 65536

+    corrupt: false

 *** done

-- 

2.27.0