[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 12/21] mirror: Switch mirror_do_read() to byt
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v4 12/21] mirror: Switch mirror_do_read() to byte-based |
Date: |
Thu, 6 Jul 2017 15:30:17 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 05.07.2017 um 23:08 hat Eric Blake geschrieben:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based. Convert another internal
> function (no semantic change).
>
> Signed-off-by: Eric Blake <address@hidden>
> Reviewed-by: John Snow <address@hidden>
> Reviewed-by: Jeff Cody <address@hidden>
> - /* The sector range must meet granularity because:
> + assert(bytes <= s->buf_size);
> + /* The range will be sector-aligned because:
> * 1) Caller passes in aligned values;
> - * 2) mirror_cow_align is used only when target cluster is larger. */
> - assert(!(sector_num % sectors_per_chunk));
> - nb_chunks = DIV_ROUND_UP(nb_sectors, sectors_per_chunk);
> + * 2) mirror_cow_align is used only when target cluster is larger.
> + * But it might not be cluster-aligned at end-of-file. */
> + assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
> + nb_chunks = DIV_ROUND_UP(bytes, s->granularity);
The assertion in the old code was about sector_num (i.e. that the start
of the range is cluster-aligned), not about nb_sectors, so while you add
a new assertion, it is asserting something different. This explains
why you had to switch to sector aligned even though the semantics
shouldn't be changed by this patch.
Is this intentional or did you confuse sector_num and nb_sectors? I
think we can just have both assertions.
The rest of the patch looks okay.
Kevin
- [Qemu-devel] [PATCH v4 02/21] trace: Show blockjob actions via bytes, not sectors, (continued)
- [Qemu-devel] [PATCH v4 02/21] trace: Show blockjob actions via bytes, not sectors, Eric Blake, 2017/07/05
- [Qemu-devel] [PATCH v4 05/21] stream: Switch stream_run() to byte-based, Eric Blake, 2017/07/05
- [Qemu-devel] [PATCH v4 08/21] mirror: Switch MirrorBlockJob to byte-based, Eric Blake, 2017/07/05
- [Qemu-devel] [PATCH v4 09/21] mirror: Switch mirror_do_zero_or_discard() to byte-based, Eric Blake, 2017/07/05
- [Qemu-devel] [PATCH v4 11/21] mirror: Switch mirror_cow_align() to byte-based, Eric Blake, 2017/07/05
- [Qemu-devel] [PATCH v4 12/21] mirror: Switch mirror_do_read() to byte-based, Eric Blake, 2017/07/05
- Re: [Qemu-devel] [PATCH v4 12/21] mirror: Switch mirror_do_read() to byte-based,
Kevin Wolf <=
- [Qemu-devel] [PATCH v4 13/21] mirror: Switch mirror_iteration() to byte-based, Eric Blake, 2017/07/05
- [Qemu-devel] [PATCH v4 14/21] block: Drop unused bdrv_round_sectors_to_clusters(), Eric Blake, 2017/07/05
- [Qemu-devel] [PATCH v4 15/21] backup: Switch BackupBlockJob to byte-based, Eric Blake, 2017/07/05
- [Qemu-devel] [PATCH v4 16/21] backup: Switch block_backup.h to byte-based, Eric Blake, 2017/07/05