[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v2 13/17] qcow: make encrypt_sectors encrypt in
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-block] [PATCH v2 13/17] qcow: make encrypt_sectors encrypt in place |
Date: |
Tue, 9 Feb 2016 12:33:19 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Mon, Feb 08, 2016 at 01:30:10PM -0700, Eric Blake wrote:
> On 01/20/2016 10:38 AM, Daniel P. Berrange wrote:
> > Instead of requiring separate input/output buffers for
> > encrypting data, change encrypt_sectors() to assume
> > use of a single buffer, encrypting in place. One current
> > caller all uses the same buffer for input/output already
> > and the other two callers are easily converted todo so.
>
> s/todo/to do/
>
> >
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> > block/qcow.c | 31 ++++++++++---------------------
> > 1 file changed, 10 insertions(+), 21 deletions(-)
> >
>
> > @@ -453,13 +447,12 @@ static uint64_t get_cluster_offset(BlockDriverState
> > *bs,
> > uint64_t start_sect;
> > assert(s->cipher);
> > start_sect = (offset & ~(s->cluster_size - 1)) >> 9;
> > - memset(s->cluster_data + 512, 0x00, 512);
> > + memset(s->cluster_data, 0x00, 512);
> > for(i = 0; i < s->cluster_sectors; i++) {
>
> As long as you're touching near here, is it worth adding a space before '('?
>
> > @@ -672,7 +665,6 @@ static coroutine_fn int qcow_co_writev(BlockDriverState
> > *bs, int64_t sector_num,
> > BDRVQcowState *s = bs->opaque;
> > int index_in_cluster;
> > uint64_t cluster_offset;
> > - const uint8_t *src_buf;
> > int ret = 0, n;
> > uint8_t *cluster_data = NULL;
> > struct iovec hd_iov;
> > @@ -715,18 +707,15 @@ static coroutine_fn int
> > qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
> > if (!cluster_data) {
> > cluster_data = g_malloc0(s->cluster_size);
> > }
> > - if (encrypt_sectors(s, sector_num, cluster_data, buf,
> > + if (encrypt_sectors(s, sector_num, buf,
> > n, true, &err) < 0) {
> > error_free(err);
> > ret = -EIO;
> > break;
> > }
> > - src_buf = cluster_data;
>
> cluster_data is now unused; you've got a wasted g_malloc0() here and
> g_free() later on.
Oh, good point.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|