[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/5] qcow2: Make copy_sectors() byte based
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/5] qcow2: Make copy_sectors() byte based |
Date: |
Mon, 6 Jun 2016 21:47:35 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 06/06/2016 08:59 AM, Kevin Wolf wrote:
> This will allow copy on write operations where the overwritten part of
> the cluster is not aligned to sector boundaries.
>
> Also rename the function because it has nothing to do with sectors any
> more.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> block/qcow2-cluster.c | 54
> +++++++++++++++++++++++++--------------------------
> 1 file changed, 26 insertions(+), 28 deletions(-)
>
>
> if (bs->encrypted) {
> Error *err = NULL;
> + int sector = (cluster_offset + offset_in_cluster) >>
> BDRV_SECTOR_BITS;
Potentially the wrong type...
> assert(s->cipher);
> - if (qcow2_encrypt_sectors(s, start_sect + n_start,
> - iov.iov_base, iov.iov_base, n,
> - true, &err) < 0) {
> + assert((offset_in_cluster & BDRV_SECTOR_MASK) == 0);
Why is this one true? If I have a cluster of 4 sectors, why must
offset_in_cluster fall within only the first of those sectors? Are you
missing a ~, to instead be asserting that offset_in_cluster is
sector-aligned?
> + assert((bytes & ~BDRV_SECTOR_MASK) == 0);
This one looks correct, stating that the number of bytes to copy is a
sector multiple.
> + if (qcow2_encrypt_sectors(s, sector, iov.iov_base, iov.iov_base,
> + bytes >> BDRV_SECTOR_BITS, true, &err) <
> 0) {
...since encryption allows a 64-bit sector number for the case where the
image is larger than 2T.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature