From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:38461) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QpJcR-0000ZA-N3 for qemu-devel@nongnu.org; Fri, 05 Aug 2011 08:35:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QpJcN-0003j1-ON for qemu-devel@nongnu.org; Fri, 05 Aug 2011 08:35:15 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:60905) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QpJcN-0003iu-Lt for qemu-devel@nongnu.org; Fri, 05 Aug 2011 08:35:11 -0400 Received: by gxk26 with SMTP id 26so79704gxk.4 for ; Fri, 05 Aug 2011 05:35:10 -0700 (PDT) Message-ID: <4E3BE37C.8070403@codemonkey.ws> Date: Fri, 05 Aug 2011 07:35:08 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1312538771-11472-1-git-send-email-kwolf@redhat.com> In-Reply-To: <1312538771-11472-1-git-send-email-kwolf@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3] qcow2: Fix L1 table size after bdrv_snapshot_goto List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: stefanha@gmail.com, hahn@univention.de, qemu-devel@nongnu.org, freddy77@gmail.com On 08/05/2011 05:06 AM, Kevin Wolf wrote: > When loading an internal snapshot whose L1 table is smaller than the current L1 > table, the size of the current L1 would be shrunk to the snapshot's L1 size in > memory, but not on disk. This lead to incorrect refcount updates and eventuelly > to image corruption. > > Instead of writing the new L1 size to disk, this simply retains the bigger L1 > size that is currently in use and makes sure that the unused part is zeroed. > > Signed-off-by: Kevin Wolf > Tested-by: Philipp Hahn Applied to master and stable-0.15. Thanks. Regards, Anthony Liguori > --- > block/qcow2-snapshot.c | 16 +++++++++++----- > 1 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c > index 74823a5..e32bcf0 100644 > --- a/block/qcow2-snapshot.c > +++ b/block/qcow2-snapshot.c > @@ -317,7 +317,8 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) > { > BDRVQcowState *s = bs->opaque; > QCowSnapshot *sn; > - int i, snapshot_index, l1_size2; > + int i, snapshot_index; > + int cur_l1_bytes, sn_l1_bytes; > > snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id); > if (snapshot_index< 0) > @@ -330,14 +331,19 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) > if (qcow2_grow_l1_table(bs, sn->l1_size, true)< 0) > goto fail; > > - s->l1_size = sn->l1_size; > - l1_size2 = s->l1_size * sizeof(uint64_t); > + cur_l1_bytes = s->l1_size * sizeof(uint64_t); > + sn_l1_bytes = sn->l1_size * sizeof(uint64_t); > + > + if (cur_l1_bytes> sn_l1_bytes) { > + memset(s->l1_table + sn->l1_size, 0, cur_l1_bytes - sn_l1_bytes); > + } > + > /* copy the snapshot l1 table to the current l1 table */ > if (bdrv_pread(bs->file, sn->l1_table_offset, > - s->l1_table, l1_size2) != l1_size2) > + s->l1_table, sn_l1_bytes)< 0) > goto fail; > if (bdrv_pwrite_sync(bs->file, s->l1_table_offset, > - s->l1_table, l1_size2)< 0) > + s->l1_table, cur_l1_bytes)< 0) > goto fail; > for(i = 0;i< s->l1_size; i++) { > be64_to_cpus(&s->l1_table[i]);