[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: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/5] qcow2: Make copy_sectors() byte based |
Date: |
Tue, 7 Jun 2016 11:12:47 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 07.06.2016 um 05:47 hat Eric Blake geschrieben:
> 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...
Yes, thanks for catching that.
> > 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?
You mean I should actually test encrypted images? *cough* (I know I did
test something with encryption, but maybe that was only after converting
reads.)
Anyway, missing ~ indeed.
> > + 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.
Kevin
pgpWxvqbKc9Pc.pgp
Description: PGP signature