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
