From: Jeff Cody <jcody@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org, famz@redhat.com, mreitz@redhat.com,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH for-2.6 v3 1/3] mirror: Don't extend the last sub-chunk
Date: Wed, 20 Apr 2016 10:47:30 -0400 [thread overview]
Message-ID: <20160420144730.GA1109@localhost.localdomain> (raw)
In-Reply-To: <1461162766-907-2-git-send-email-kwolf@redhat.com>
On Wed, Apr 20, 2016 at 04:32:44PM +0200, Kevin Wolf wrote:
> From: Fam Zheng <famz@redhat.com>
>
> The last sub-chunk is rounded up to the copy granularity in the target
> image, resulting in a larger size than the source.
>
> Add a function to clip the copied sectors to the end.
>
> This undoes the "wrong" changes to tests/qemu-iotests/109.out in
> e5b43573e28. The remaining two offset changes are okay.
>
> [ kwolf: Use DIV_ROUND_UP to calculate nb_chunks now ]
>
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/mirror.c | 19 +++++++++++++++----
> tests/qemu-iotests/109.out | 44 ++++++++++++++++++++++----------------------
> 2 files changed, 37 insertions(+), 26 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 9df1fae..d56e30e 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -108,7 +108,7 @@ static void mirror_iteration_done(MirrorOp *op, int ret)
>
> sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
> chunk_num = op->sector_num / sectors_per_chunk;
> - nb_chunks = op->nb_sectors / sectors_per_chunk;
> + nb_chunks = DIV_ROUND_UP(op->nb_sectors, sectors_per_chunk);
> bitmap_clear(s->in_flight_bitmap, chunk_num, nb_chunks);
> if (ret >= 0) {
> if (s->cow_bitmap) {
> @@ -161,6 +161,14 @@ static void mirror_read_complete(void *opaque, int ret)
> mirror_write_complete, op);
> }
>
> +static inline void mirror_clip_sectors(MirrorBlockJob *s,
> + int64_t sector_num,
> + int *nb_sectors)
> +{
> + *nb_sectors = MIN(*nb_sectors,
> + s->bdev_length / BDRV_SECTOR_SIZE - sector_num);
> +}
> +
> /* Round sector_num and/or nb_sectors to target cluster if COW is needed, and
> * return the offset of the adjusted tail sector against original. */
> static int mirror_cow_align(MirrorBlockJob *s,
> @@ -189,6 +197,9 @@ static int mirror_cow_align(MirrorBlockJob *s,
> s->target_cluster_sectors);
> }
> }
> + /* Clipping may result in align_nb_sectors unaligned to chunk boundary, but
> + * that doesn't matter because it's already the end of source image. */
> + mirror_clip_sectors(s, align_sector_num, &align_nb_sectors);
>
> ret = align_sector_num + align_nb_sectors - (*sector_num + *nb_sectors);
> *sector_num = align_sector_num;
> @@ -231,9 +242,8 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
> /* The sector range must meet granularity because:
> * 1) Caller passes in aligned values;
> * 2) mirror_cow_align is used only when target cluster is larger. */
> - assert(!(nb_sectors % sectors_per_chunk));
> assert(!(sector_num % sectors_per_chunk));
> - nb_chunks = nb_sectors / sectors_per_chunk;
> + nb_chunks = DIV_ROUND_UP(nb_sectors, sectors_per_chunk);
>
> while (s->buf_free_count < nb_chunks) {
> trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
> @@ -384,6 +394,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
> }
> }
>
> + mirror_clip_sectors(s, sector_num, &io_sectors);
> switch (mirror_method) {
> case MIRROR_METHOD_COPY:
> io_sectors = mirror_do_read(s, sector_num, io_sectors);
> @@ -399,7 +410,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
> }
> assert(io_sectors);
> sector_num += io_sectors;
> - nb_chunks -= io_sectors / sectors_per_chunk;
> + nb_chunks -= DIV_ROUND_UP(io_sectors, sectors_per_chunk);
> delay_ns += ratelimit_calculate_delay(&s->limit, io_sectors);
> }
> return delay_ns;
> diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
> index b3358de..38bc073 100644
> --- a/tests/qemu-iotests/109.out
> +++ b/tests/qemu-iotests/109.out
> @@ -10,14 +10,14 @@ Automatically detecting the format is dangerous for raw images, write operations
> Specify the 'raw' format explicitly to remove the restrictions.
> {"return": {}}
> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 65536, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 1024, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> {"return": []}
> read 65536/65536 bytes at offset 0
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> {"return": {}}
> {"return": {}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 65536, "offset": 65536, "speed": 0, "type": "mirror"}}
> -{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 65536, "offset": 65536, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 1024, "offset": 1024, "speed": 0, "type": "mirror"}}
> +{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 1024, "offset": 1024, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> Warning: Image size mismatch!
> Images are identical.
>
> @@ -31,14 +31,14 @@ Automatically detecting the format is dangerous for raw images, write operations
> Specify the 'raw' format explicitly to remove the restrictions.
> {"return": {}}
> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 262144, "offset": 65536, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 197120, "offset": 512, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> {"return": []}
> read 65536/65536 bytes at offset 0
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> {"return": {}}
> {"return": {}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 262144, "offset": 262144, "speed": 0, "type": "mirror"}}
> -{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 262144, "offset": 262144, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 197120, "offset": 197120, "speed": 0, "type": "mirror"}}
> +{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 197120, "offset": 197120, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> Warning: Image size mismatch!
> Images are identical.
>
> @@ -73,14 +73,14 @@ Automatically detecting the format is dangerous for raw images, write operations
> Specify the 'raw' format explicitly to remove the restrictions.
> {"return": {}}
> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 65536, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 1024, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> {"return": []}
> read 65536/65536 bytes at offset 0
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> {"return": {}}
> {"return": {}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 65536, "offset": 65536, "speed": 0, "type": "mirror"}}
> -{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 65536, "offset": 65536, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 1024, "offset": 1024, "speed": 0, "type": "mirror"}}
> +{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 1024, "offset": 1024, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> Warning: Image size mismatch!
> Images are identical.
>
> @@ -115,14 +115,14 @@ Automatically detecting the format is dangerous for raw images, write operations
> Specify the 'raw' format explicitly to remove the restrictions.
> {"return": {}}
> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 65536, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> {"return": []}
> read 65536/65536 bytes at offset 0
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> {"return": {}}
> {"return": {}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 65536, "offset": 65536, "speed": 0, "type": "mirror"}}
> -{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 65536, "offset": 65536, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2560, "offset": 2560, "speed": 0, "type": "mirror"}}
> +{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2560, "offset": 2560, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> Warning: Image size mismatch!
> Images are identical.
>
> @@ -135,14 +135,14 @@ Automatically detecting the format is dangerous for raw images, write operations
> Specify the 'raw' format explicitly to remove the restrictions.
> {"return": {}}
> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 65536, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> {"return": []}
> read 65536/65536 bytes at offset 0
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> {"return": {}}
> {"return": {}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 65536, "offset": 65536, "speed": 0, "type": "mirror"}}
> -{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 65536, "offset": 65536, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2560, "offset": 2560, "speed": 0, "type": "mirror"}}
> +{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2560, "offset": 2560, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> Image resized.
> Warning: Image size mismatch!
> Images are identical.
> @@ -198,14 +198,14 @@ Automatically detecting the format is dangerous for raw images, write operations
> Specify the 'raw' format explicitly to remove the restrictions.
> {"return": {}}
> {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 65536, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2048, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
> {"return": []}
> read 65536/65536 bytes at offset 0
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> {"return": {}}
> {"return": {}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 65536, "offset": 65536, "speed": 0, "type": "mirror"}}
> -{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 65536, "offset": 65536, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2048, "offset": 2048, "speed": 0, "type": "mirror"}}
> +{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2048, "offset": 2048, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> Image resized.
> Warning: Image size mismatch!
> Images are identical.
> @@ -218,14 +218,14 @@ WARNING: Image format was not specified for 'TEST_DIR/t.raw' and probing guessed
> Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
> Specify the 'raw' format explicitly to remove the restrictions.
> {"return": {}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 65536, "offset": 65536, "speed": 0, "type": "mirror"}}
> -{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 65536, "offset": 65536, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 512, "offset": 512, "speed": 0, "type": "mirror"}}
> +{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 512, "offset": 512, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> Warning: Image size mismatch!
> Images are identical.
> {"return": {}}
> {"return": {}}
> -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 65536, "offset": 65536, "speed": 0, "type": "mirror"}}
> -{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 65536, "offset": 65536, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 512, "offset": 512, "speed": 0, "type": "mirror"}}
> +{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 512, "offset": 512, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
> Warning: Image size mismatch!
> Images are identical.
> *** done
> --
> 1.8.3.1
>
Reviewed-by: Jeff Cody <jcody@redhat.com>
next prev parent reply other threads:[~2016-04-20 14:47 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-20 14:32 [Qemu-devel] [PATCH for-2.6 v3 0/3] block: Fix drive-mirror with image size unaligned with granularity Kevin Wolf
2016-04-20 14:32 ` [Qemu-devel] [PATCH for-2.6 v3 1/3] mirror: Don't extend the last sub-chunk Kevin Wolf
2016-04-20 14:46 ` Max Reitz
2016-04-20 14:47 ` Jeff Cody [this message]
2016-04-20 14:32 ` [Qemu-devel] [PATCH for-2.6 v3 2/3] iotests: Add iotests.image_size Kevin Wolf
2016-04-20 14:48 ` Jeff Cody
2016-04-20 14:32 ` [Qemu-devel] [PATCH for-2.6 v3 3/3] iotests: Test case for drive-mirror with unaligned image size Kevin Wolf
2016-04-20 14:49 ` Max Reitz
2016-04-20 14:50 ` Jeff Cody
2016-04-20 14:51 ` [Qemu-devel] [PATCH for-2.6 v3 0/3] block: Fix drive-mirror with image size unaligned with granularity Jeff Cody
2016-04-20 14:59 ` Jeff Cody
2016-04-21 2:12 ` Fam Zheng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160420144730.GA1109@localhost.localdomain \
--to=jcody@redhat.com \
--cc=famz@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).