For the 'alloc' command, accepting an offset in bytes but a length
in sectors, and reporting output in sectors, is confusing. Do
everything in bytes, and adjust the expected output accordingly.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
qemu-io-cmds.c | 30 ++++++++++++++++++------------
tests/qemu-iotests/019.out | 8 ++++----
tests/qemu-iotests/179 | 4 ++--
tests/qemu-iotests/179.out | 4 ++--
tests/qemu-iotests/common.pattern | 2 +-
5 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 9e023a4..df7297f 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1760,7 +1760,7 @@ out:
static int alloc_f(BlockBackend *blk, int argc, char **argv)
{
BlockDriverState *bs = blk_bs(blk);
- int64_t offset, sector_num, nb_sectors, remaining;
+ int64_t offset, sector_num, nb_sectors, remaining, bytes;
char s1[64];
int num, ret;
int64_t sum_alloc;
@@ -1776,18 +1776,24 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv)
}
if (argc == 3) {
- nb_sectors = cvtnum(argv[2]);
- if (nb_sectors < 0) {
- print_cvtnum_err(nb_sectors, argv[2]);
+ bytes = cvtnum(argv[2]);
+ if (bytes < 0) {
+ print_cvtnum_err(bytes, argv[2]);
return 0;
- } else if (nb_sectors > INT_MAX) {
- printf("length argument cannot exceed %d, given %s\n",
- INT_MAX, argv[2]);
+ } else if (bytes > INT_MAX * BDRV_SECTOR_SIZE) {
+ printf("length argument cannot exceed %llu, given %s\n",
+ INT_MAX * BDRV_SECTOR_SIZE, argv[2]);
return 0;
}
} else {
- nb_sectors = 1;
+ bytes = BDRV_SECTOR_SIZE;
}
+ if (bytes & 0x1ff) {
+ printf("bytes %" PRId64 " is not sector aligned\n",
+ bytes);
+ return 0;
+ }
+ nb_sectors = bytes >> BDRV_SECTOR_BITS;
remaining = nb_sectors;
sum_alloc = 0;
@@ -1811,8 +1817,8 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv)
cvtstr(offset, s1, sizeof(s1));
- printf("%"PRId64"/%"PRId64" sectors allocated at offset %s\n",
- sum_alloc, nb_sectors, s1);
+ printf("%"PRId64"/%"PRId64" bytes allocated at offset %s\n",
+ sum_alloc << BDRV_SECTOR_BITS, nb_sectors << BDRV_SECTOR_BITS, s1);
return 0;
}
@@ -1822,8 +1828,8 @@ static const cmdinfo_t alloc_cmd = {
.argmin = 1,
.argmax = 2,
.cfunc = alloc_f,
- .args = "off [sectors]",
- .oneline = "checks if a sector is present in the file",
+ .args = "offset [bytes]",
+ .oneline = "checks if offset is allocated in the file",
};
diff --git a/tests/qemu-iotests/019.out b/tests/qemu-iotests/019.out
index 0124264..17a7c03 100644
--- a/tests/qemu-iotests/019.out
+++ b/tests/qemu-iotests/019.out
@@ -542,8 +542,8 @@ Testing conversion with -B TEST_DIR/t.IMGFMT.base
Checking if backing clusters are allocated when they shouldn't
-0/128 sectors allocated at offset 1 MiB
-0/128 sectors allocated at offset 4.001 GiB
+0/65536 bytes allocated at offset 1 MiB
+0/65536 bytes allocated at offset 4.001 GiB
Reading
=== IO: pattern 42
@@ -1086,8 +1086,8 @@ Testing conversion with -o backing_file=TEST_DIR/t.IMGFMT.base
Checking if backing clusters are allocated when they shouldn't
-0/128 sectors allocated at offset 1 MiB
-0/128 sectors allocated at offset 4.001 GiB
+0/65536 bytes allocated at offset 1 MiB
+0/65536 bytes allocated at offset 4.001 GiB
Reading
=== IO: pattern 42
diff --git a/tests/qemu-iotests/179 b/tests/qemu-iotests/179
index 338a45d..44541e1 100755
--- a/tests/qemu-iotests/179
+++ b/tests/qemu-iotests/179
@@ -62,12 +62,12 @@ $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
# But not requesting unmap must result in allocation (whether a cluster
# allocation in compat=0.10 or a flag allocation in compat=1.1).
$QEMU_IO -c "write -z 5M 1M" "$TEST_IMG.base" | _filter_qemu_io
-$QEMU_IO -c "alloc 5M $((1024*1024 / 512))" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "alloc 5M 1M" "$TEST_IMG.base" | _filter_qemu_io
# Presence of a backing file overrides permission to unmap. Again,
# compat=0.10 images allocate, while compat=1.1 images set zero flag.
$QEMU_IO -c "write -z -u 7M 1M" "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "alloc 7M $((1024 * 1024 / 512))" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "alloc 7M 1M" "$TEST_IMG" | _filter_qemu_io
# Final check that images are still sane.
TEST_IMG="$TEST_IMG.base" _check_test_img
diff --git a/tests/qemu-iotests/179.out b/tests/qemu-iotests/179.out
index fc97b19..18ecf0f 100644
--- a/tests/qemu-iotests/179.out
+++ b/tests/qemu-iotests/179.out
@@ -13,10 +13,10 @@ wrote 1048576/1048576 bytes at offset 3145728
[{ "start": 0, "length": 16777216, "depth": 1, "zero": true, "data": false}]
wrote 1048576/1048576 bytes at offset 5242880
1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-2048/2048 sectors allocated at offset 5 MiB
+1048576/1048576 bytes allocated at offset 5 MiB
wrote 1048576/1048576 bytes at offset 7340032
1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-2048/2048 sectors allocated at offset 7 MiB
+1048576/1048576 bytes allocated at offset 7 MiB
No errors were found on the image.
No errors were found on the image.
*** done
diff --git a/tests/qemu-iotests/common.pattern b/tests/qemu-iotests/common.pattern
index ddfbca1..34f4a8d 100644
--- a/tests/qemu-iotests/common.pattern
+++ b/tests/qemu-iotests/common.pattern
@@ -18,7 +18,7 @@
function do_is_allocated() {
local start=$1
- local size=$(( $2 / 512))
+ local size=$2
local step=$3
local count=$4
--
2.9.3
Hi Eric,
On 04/10/2017 10:17 PM, Eric Blake wrote:
> For the 'alloc' command, accepting an offset in bytes but a length
> in sectors, and reporting output in sectors, is confusing. Do
> everything in bytes, and adjust the expected output accordingly.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> qemu-io-cmds.c | 30 ++++++++++++++++++------------
> tests/qemu-iotests/019.out | 8 ++++----
> tests/qemu-iotests/179 | 4 ++--
> tests/qemu-iotests/179.out | 4 ++--
> tests/qemu-iotests/common.pattern | 2 +-
> 5 files changed, 27 insertions(+), 21 deletions(-)
>
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 9e023a4..df7297f 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -1760,7 +1760,7 @@ out:
> static int alloc_f(BlockBackend *blk, int argc, char **argv)
> {
> BlockDriverState *bs = blk_bs(blk);
> - int64_t offset, sector_num, nb_sectors, remaining;
> + int64_t offset, sector_num, nb_sectors, remaining, bytes;
> char s1[64];
> int num, ret;
> int64_t sum_alloc;
> @@ -1776,18 +1776,24 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv)
> }
>
> if (argc == 3) {
> - nb_sectors = cvtnum(argv[2]);
> - if (nb_sectors < 0) {
> - print_cvtnum_err(nb_sectors, argv[2]);
> + bytes = cvtnum(argv[2]);
> + if (bytes < 0) {
> + print_cvtnum_err(bytes, argv[2]);
> return 0;
> - } else if (nb_sectors > INT_MAX) {
> - printf("length argument cannot exceed %d, given %s\n",
> - INT_MAX, argv[2]);
> + } else if (bytes > INT_MAX * BDRV_SECTOR_SIZE) {
> + printf("length argument cannot exceed %llu, given %s\n",
> + INT_MAX * BDRV_SECTOR_SIZE, argv[2]);
> return 0;
> }
> } else {
> - nb_sectors = 1;
> + bytes = BDRV_SECTOR_SIZE;
> }
> + if (bytes & 0x1ff) {
This macro is self-explanatory:
if(!QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE))
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> + printf("bytes %" PRId64 " is not sector aligned\n",
> + bytes);
> + return 0;
> + }
> + nb_sectors = bytes >> BDRV_SECTOR_BITS;
>
> remaining = nb_sectors;
> sum_alloc = 0;
> @@ -1811,8 +1817,8 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv)
>
> cvtstr(offset, s1, sizeof(s1));
>
> - printf("%"PRId64"/%"PRId64" sectors allocated at offset %s\n",
> - sum_alloc, nb_sectors, s1);
> + printf("%"PRId64"/%"PRId64" bytes allocated at offset %s\n",
> + sum_alloc << BDRV_SECTOR_BITS, nb_sectors << BDRV_SECTOR_BITS, s1);
> return 0;
> }
>
> @@ -1822,8 +1828,8 @@ static const cmdinfo_t alloc_cmd = {
> .argmin = 1,
> .argmax = 2,
> .cfunc = alloc_f,
> - .args = "off [sectors]",
> - .oneline = "checks if a sector is present in the file",
> + .args = "offset [bytes]",
> + .oneline = "checks if offset is allocated in the file",
> };
>
>
> diff --git a/tests/qemu-iotests/019.out b/tests/qemu-iotests/019.out
> index 0124264..17a7c03 100644
> --- a/tests/qemu-iotests/019.out
> +++ b/tests/qemu-iotests/019.out
> @@ -542,8 +542,8 @@ Testing conversion with -B TEST_DIR/t.IMGFMT.base
>
> Checking if backing clusters are allocated when they shouldn't
>
> -0/128 sectors allocated at offset 1 MiB
> -0/128 sectors allocated at offset 4.001 GiB
> +0/65536 bytes allocated at offset 1 MiB
> +0/65536 bytes allocated at offset 4.001 GiB
> Reading
>
> === IO: pattern 42
> @@ -1086,8 +1086,8 @@ Testing conversion with -o backing_file=TEST_DIR/t.IMGFMT.base
>
> Checking if backing clusters are allocated when they shouldn't
>
> -0/128 sectors allocated at offset 1 MiB
> -0/128 sectors allocated at offset 4.001 GiB
> +0/65536 bytes allocated at offset 1 MiB
> +0/65536 bytes allocated at offset 4.001 GiB
> Reading
>
> === IO: pattern 42
> diff --git a/tests/qemu-iotests/179 b/tests/qemu-iotests/179
> index 338a45d..44541e1 100755
> --- a/tests/qemu-iotests/179
> +++ b/tests/qemu-iotests/179
> @@ -62,12 +62,12 @@ $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
> # But not requesting unmap must result in allocation (whether a cluster
> # allocation in compat=0.10 or a flag allocation in compat=1.1).
> $QEMU_IO -c "write -z 5M 1M" "$TEST_IMG.base" | _filter_qemu_io
> -$QEMU_IO -c "alloc 5M $((1024*1024 / 512))" "$TEST_IMG.base" | _filter_qemu_io
> +$QEMU_IO -c "alloc 5M 1M" "$TEST_IMG.base" | _filter_qemu_io
>
> # Presence of a backing file overrides permission to unmap. Again,
> # compat=0.10 images allocate, while compat=1.1 images set zero flag.
> $QEMU_IO -c "write -z -u 7M 1M" "$TEST_IMG" | _filter_qemu_io
> -$QEMU_IO -c "alloc 7M $((1024 * 1024 / 512))" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "alloc 7M 1M" "$TEST_IMG" | _filter_qemu_io
>
> # Final check that images are still sane.
> TEST_IMG="$TEST_IMG.base" _check_test_img
> diff --git a/tests/qemu-iotests/179.out b/tests/qemu-iotests/179.out
> index fc97b19..18ecf0f 100644
> --- a/tests/qemu-iotests/179.out
> +++ b/tests/qemu-iotests/179.out
> @@ -13,10 +13,10 @@ wrote 1048576/1048576 bytes at offset 3145728
> [{ "start": 0, "length": 16777216, "depth": 1, "zero": true, "data": false}]
> wrote 1048576/1048576 bytes at offset 5242880
> 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -2048/2048 sectors allocated at offset 5 MiB
> +1048576/1048576 bytes allocated at offset 5 MiB
> wrote 1048576/1048576 bytes at offset 7340032
> 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -2048/2048 sectors allocated at offset 7 MiB
> +1048576/1048576 bytes allocated at offset 7 MiB
> No errors were found on the image.
> No errors were found on the image.
> *** done
> diff --git a/tests/qemu-iotests/common.pattern b/tests/qemu-iotests/common.pattern
> index ddfbca1..34f4a8d 100644
> --- a/tests/qemu-iotests/common.pattern
> +++ b/tests/qemu-iotests/common.pattern
> @@ -18,7 +18,7 @@
>
> function do_is_allocated() {
> local start=$1
> - local size=$(( $2 / 512))
> + local size=$2
> local step=$3
> local count=$4
>
On 04/10/2017 09:37 PM, Philippe Mathieu-Daudé wrote:
> Hi Eric,
>
> On 04/10/2017 10:17 PM, Eric Blake wrote:
>> For the 'alloc' command, accepting an offset in bytes but a length
>> in sectors, and reporting output in sectors, is confusing. Do
>> everything in bytes, and adjust the expected output accordingly.
>>
>> }
>> } else {
>> - nb_sectors = 1;
>> + bytes = BDRV_SECTOR_SIZE;
>> }
>> + if (bytes & 0x1ff) {
>
> This macro is self-explanatory:
>
> if(!QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE))
Indeed; but I was going for copy-and-paste consistency with the rest of
the file that also open-codes this. A separate cleanup patch for all of
them would qualify as trivial.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
© 2016 - 2026 Red Hat, Inc.