From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52884) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aT7U2-0005Qn-AD for qemu-devel@nongnu.org; Tue, 09 Feb 2016 07:33:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aT7U0-00015u-W8 for qemu-devel@nongnu.org; Tue, 09 Feb 2016 07:33:30 -0500 Date: Tue, 9 Feb 2016 12:33:19 +0000 From: "Daniel P. Berrange" Message-ID: <20160209123319.GG24614@redhat.com> References: <1453311539-1193-1-git-send-email-berrange@redhat.com> <1453311539-1193-14-git-send-email-berrange@redhat.com> <56B8FAD2.2090404@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <56B8FAD2.2090404@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 13/17] qcow: make encrypt_sectors encrypt in place Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Kevin Wolf , Fam Zheng , qemu-devel@nongnu.org, qemu-block@nongnu.org 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 > > --- > > 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 :|