From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33861) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XTSZo-0003hl-UU for qemu-devel@nongnu.org; Mon, 15 Sep 2014 05:28:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XTSZh-0007N5-L6 for qemu-devel@nongnu.org; Mon, 15 Sep 2014 05:28:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44704) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XTSZh-0007Ml-DW for qemu-devel@nongnu.org; Mon, 15 Sep 2014 05:27:57 -0400 Date: Mon, 15 Sep 2014 10:27:53 +0100 From: Stefan Hajnoczi Message-ID: <20140915092753.GA32739@stefanha-thinkpad.redhat.com> References: <1409568798-2292-1-git-send-email-junmuzi@gmail.com> <20140905153343.GH27649@stefanha-thinkpad.redhat.com> <20140913155331.GA22493@localhost.localdomain> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="rwEMma7ioTxnRzrJ" Content-Disposition: inline In-Reply-To: <20140913155331.GA22493@localhost.localdomain> Subject: Re: [Qemu-devel] [PATCH v2] qcow2: add update refcount table realization for update_refcount List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jun Li Cc: kwolf@redhat.com, Stefan Hajnoczi , juli@redhat.com, famz@redhat.com, qemu-devel@nongnu.org --rwEMma7ioTxnRzrJ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Sep 13, 2014 at 11:53:58PM +0800, Jun Li wrote: > On Fri, 09/05 16:33, Stefan Hajnoczi wrote: > > On Mon, Sep 01, 2014 at 06:52:48PM +0800, Jun Li wrote: > >=20 > > How does this patch handle self-describing refcount blocks? I think > > they will keep the refcount block alive forever because your code will > > not decide to free them. > >=20 >=20 > Sorry, I have ignored self-describing refcount blocks. :) For this... > > This patch should also discard the refcount block if we decide to free > > it (in the same way that we discard at cluster_offset). > >=20 > > > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > > > index 43665b8..63f36e6 100644 > > > --- a/block/qcow2-refcount.c > > > +++ b/block/qcow2-refcount.c > > > @@ -586,6 +586,37 @@ static int QEMU_WARN_UNUSED_RESULT update_refcou= nt(BlockDriverState *bs, > > > if (refcount =3D=3D 0 && s->discard_passthrough[type]) { > > > update_refcount_discard(bs, cluster_offset, s->cluster_s= ize); > > > } > > > + > > > + /* When refcount block is NULL, update refcount table */ > > > + if (block_index =3D=3D 0) { > >=20 > > What is the purpose of block_index =3D=3D 0? >=20 > Here is want to reduce the probability of running the following code. Only > when block_index =3D=3D 0, we will run the following code to free refcoun= t block. =2E..and this reason, I consider this approach incomplete. The approach is unreliable because a change to refcount update ordering could change leak behavior. Either free refcount blocks to avoid leaks in all cases, or don't bother. Stefan --rwEMma7ioTxnRzrJ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUFrEZAAoJEJykq7OBq3PIBlkH/izC2Q4S55F3r+aE5pzSzAeh HrQlA8mYaH2bMJbZn+80G/xT1wMNizTkCu3DTzXdzL1UBGQ7AmkOMbN5EuQgdlg9 U8pOzoc+WlML04o4xQsCnO3ddJSQwSs3CUkN0yMpqL2fKBmDwt8BHEkMS60K94iX 9LR0Y8aEMGqHfq06/S6IET3yu2bhirS0tTMJg+v+doNG1Ve4PLtfSdSXvbfLlcGR n7rDDsNhHstjpf3XYmBJLldImmfcfWLH7/Y5wf/Exz3jj7TdtRMQjUqSsG+gzwki LTwmoxFBhjXDaKXPrQcvWqI3jBsZsjKvmTj0a4ZR1j6R062jjJuGuVuDhpQPE34= =aqcz -----END PGP SIGNATURE----- --rwEMma7ioTxnRzrJ--