On 01/26/2015 08:00 AM, Max Reitz wrote:
Although qemu-img already creates BlockBackends, it does not do accesses
to the images through them. This patch converts all of the bdrv_* calls
for which this is currently possible to blk_* calls. Most of the
remaining calls will probably stay bdrv_* calls because they really do
operate on the BDS level instead of the BB level.
Signed-off-by: Max Reitz <address@hidden>
---
qemu-img.c | 98 ++++++++++++++++++++++++++++++++++----------------------------
1 file changed, 54 insertions(+), 44 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 0b23c87..8b4139e 100644
--- a/qemu-img.c
@@ -1130,22 +1130,26 @@ static int img_compare(int argc, char **argv)
}
bs2 = blk_bs(blk2);
- buf1 = qemu_blockalign(bs1, IO_BUF_SIZE);
- buf2 = qemu_blockalign(bs2, IO_BUF_SIZE);
- total_sectors1 = bdrv_nb_sectors(bs1);
+ buf1 = blk_blockalign(blk1, IO_BUF_SIZE);
+ buf2 = blk_blockalign(blk2, IO_BUF_SIZE);
+ total_sectors1 = blk_getlength(blk1);
if (total_sectors1 < 0) {
error_report("Can't get size of %s: %s",
filename1, strerror(-total_sectors1));
ret = 4;
goto out;
}
- total_sectors2 = bdrv_nb_sectors(bs2);
+ total_sectors2 = blk_getlength(blk2);
The naming feels awkward; your conversion is now using bytes while the
old code was using sectors, so 'total_sectors2' feels weird...
if (total_sectors2 < 0) {
error_report("Can't get size of %s: %s",
filename2, strerror(-total_sectors2));
ret = 4;
goto out;
}
+
+ total_sectors1 /= BDRV_SECTOR_SIZE;
+ total_sectors2 /= BDRV_SECTOR_SIZE;
...at least you end up converting to sectors after all. But it makes me
wonder if you should have blk_nb_sectors(), and/or temporary
intermediate variables to avoid cross-unit confusion.
@@ -1476,13 +1480,14 @@ static int img_convert(int argc, char **argv)
goto out;
}
bs[bs_i] = blk_bs(blk[bs_i]);
- bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]);
+ bs_sectors[bs_i] = blk_getlength(blk[bs_i]);
if (bs_sectors[bs_i] < 0) {
error_report("Could not get size of %s: %s",
argv[optind + bs_i], strerror(-bs_sectors[bs_i]));
ret = -1;
goto out;
}
+ bs_sectors[bs_i] /= BDRV_SECTOR_SIZE;
Another instance of the confusion.
total_sectors += bs_sectors[bs_i];
}
@@ -1625,16 +1630,19 @@ static int img_convert(int argc, char **argv)
out_bs->bl.discard_alignment))
);
- buf = qemu_blockalign(out_bs, bufsectors * BDRV_SECTOR_SIZE);
+ buf = blk_blockalign(out_blk, bufsectors * BDRV_SECTOR_SIZE);
if (skip_create) {
- int64_t output_sectors = bdrv_nb_sectors(out_bs);
+ int64_t output_sectors = blk_getlength(out_blk);
if (output_sectors < 0) {
error_report("unable to get output image length: %s\n",
strerror(-output_sectors));
ret = -1;
goto out;
- } else if (output_sectors < total_sectors) {
+ }
+
+ output_sectors /= BDRV_SECTOR_SIZE;
+ if (output_sectors < total_sectors) {
And another.
@@ -2585,17 +2591,17 @@ static int img_rebase(int argc, char **argv)
uint8_t * buf_new;
float local_progress = 0;
- buf_old = qemu_blockalign(bs, IO_BUF_SIZE);
- buf_new = qemu_blockalign(bs, IO_BUF_SIZE);
+ buf_old = blk_blockalign(blk, IO_BUF_SIZE);
+ buf_new = blk_blockalign(blk, IO_BUF_SIZE);
- num_sectors = bdrv_nb_sectors(bs);
+ num_sectors = blk_getlength(blk);
if (num_sectors < 0) {
...
- if (bs_new_backing) {
- new_backing_num_sectors = bdrv_nb_sectors(bs_new_backing);
+
+ num_sectors /= BDRV_SECTOR_SIZE;
+ old_backing_num_sectors /= BDRV_SECTOR_SIZE;
and another.
I did not closely audit if there were any other conversions that should
have been made. Also, I suspect that a blk_nb_sectors() as a pre-req
patch would make this one feel cleaner if you respin and rebase.