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 <kw...@redhat.com> > > --- > > 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
pgpqmCxfcRpAK.pgp
Description: PGP signature