qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v6 02/10] qmp: Add block-dirty-bitmap-add and bl


From: John Snow
Subject: Re: [Qemu-devel] [PATCH v6 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
Date: Wed, 19 Nov 2014 15:47:25 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0



On 11/07/2014 07:24 AM, Vladimir Sementsov-Ogievskiy wrote:
+    if (!name || name[0] == '\0') {
Isn't is better to move "name[0] == '\0'" check to
bdrv_create_dirty_bitmap, near existed name checking?

Hm, OK, but then we still need to check for the presence of a name in these functions, so we're still going to be checking the validity of the name in both places regardless.

Probably not worth the effort. If somebody adds an empty string bitmap using bdrv_dirty_bitmap_create, it will at least be the only one named as such, and I don't think it will create any problems.

+        if (granularity < 512 || is_power_of_2(granularity)) {
+            error_setg(errp, "Granularity must be power of 2 "
+                             "and greater than 512");
+            return;
+        }
+    } else {
+        granularity = 65536;
+    }
Why not using something like DEFAULT_CLUSTER_SIZE, as in block/qcow2.h ?


I don't want to tie this to explicitly qcow2, but on a suggestion from Fam, I think I will look at how block/mirror.c does this and tie it to the current cluster size -- which, if it's qcow2 -- will accomplish what you want, here.

--
—js



reply via email to

[Prev in Thread] Current Thread [Next in Thread]