From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55781) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsSvk-0002UK-LY for qemu-devel@nongnu.org; Wed, 13 May 2015 05:26:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YsSvj-0000hY-O6 for qemu-devel@nongnu.org; Wed, 13 May 2015 05:26:20 -0400 Date: Wed, 13 May 2015 11:26:12 +0200 From: Kevin Wolf Message-ID: <20150513092612.GE4263@noname.str.redhat.com> References: <1431105726-3682-1-git-send-email-kwolf@redhat.com> <1431105726-3682-21-git-send-email-kwolf@redhat.com> <555274E3.6060906@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="k4f25fnPtRuIRUb3" Content-Disposition: inline In-Reply-To: <555274E3.6060906@redhat.com> Subject: Re: [Qemu-devel] [PATCH 20/34] qcow2: Support updating driver-specific options in reopen List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: mreitz@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org, armbru@redhat.com --k4f25fnPtRuIRUb3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 12.05.2015 um 23:47 hat Eric Blake geschrieben: > On 05/08/2015 11:21 AM, Kevin Wolf wrote: > > For updating the cache sizes or disabling lazy refcounts there is a bit > > more to do than just changing the variables, but otherwise we're all set > > for changing options during bdrv_reopen(). > >=20 > > Just implement the missing pieces and hook the functions up in > > bdrv_reopen(). > >=20 > > Signed-off-by: Kevin Wolf > > --- > > block/qcow2.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++= +++----- > > 1 file changed, 64 insertions(+), 6 deletions(-) > >=20 >=20 > > -/* We have no actual commit/abort logic for qcow2, but we need to writ= e out any > > - * unwritten data if we reopen read-only. */ > > static int qcow2_reopen_prepare(BDRVReopenState *state, > > BlockReopenQueue *queue, Error **errp) > > { > > + Qcow2ReopenState *r; > > int ret; > > =20 > > + r =3D g_new0(Qcow2ReopenState, 1); > > + state->opaque =3D r; > > + > > + ret =3D qcow2_update_options_prepare(state->bs, r, state->options, > > + state->flags, errp); > > + if (ret < 0) { > > + goto fail; > > + } > > + > > + /* We need to write out any unwritten data if we reopen read-only.= */ > > if ((state->flags & BDRV_O_RDWR) =3D=3D 0) { > > ret =3D bdrv_flush(state->bs); > > if (ret < 0) { > > - return ret; > > + goto fail; > > } > > =20 > > ret =3D qcow2_mark_clean(state->bs); > > if (ret < 0) { > > - return ret; > > + goto fail; > > } > > } > > =20 > > return 0; > > + > > +fail: > > + qcow2_update_options_abort(state->bs, r); > > + return ret; >=20 > Doesn't this leak r? That is, you only free r if _commit or _abort is > reached, but my understanding of transaction semantics is that we only > guarantee that one of those is reached if _prepare succeeded. Yes, it does. Thanks for catching that. Kevin --k4f25fnPtRuIRUb3 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJVUxi0AAoJEH8JsnLIjy/WlbQP/jfoAMn6bSBag18nCNYuEA8J gUwYLRnpmh+h6N0tW13P4vlN8jh8m6fA3PhdvPD8MOtY4jMuJclFf6+uR23lsPNx E6nF77N+LeUqRnKm9NyNy/ekdhNUKCPoYZ1AeZk54qEp+sOdD0NxXpHbxU+96XST RbdrHOBEvlmskhZC/sdvhMR1SK6XLDJwHA48e/7EU06as3tKt5akuZm4a5psOB/9 NL6/vV4UWrz1Cb6Dvb4VBmqyJyFahuGz4OjCCY2jjuqrBEpa+yG18h2zNgCp8gql Tl7L54ZtFz7vNxsrDz2W90H1ZLRgDm+oKEFXUvDCcohMhjBc18J4Gfjl2sj2zOQG P8ciZYZL5BAtj2px305VxDL4c+h2pg2AOEdJJx2ezUuKnWbFXVQ3/Zwd0DaeulT2 El0mXgelJfpiaV3/tCOQsKCP9TCte7VZIKnT9uCeNqfUmBYXvu0rEgGwv7oa8Gnc GWVccr2jRfEvzVNctfV7jvc3gQ72QVVKsEb4hsQH55+IkBHJwcLGZwqWWCmRPpy1 YnpRRGP7Nv3d19hR9EOj7gWmjrpHhPS1QGoyVntPPmu0kaccifl5uwswmOFRQZ7s B0o6vDHkLHvWS+rN7V2QWGmju0P4mvAybSLss14trmtyA3a8Hi3QU6EEzInb4F2Q Tedz9GH4Q45kYpXKogS2 =jaRD -----END PGP SIGNATURE----- --k4f25fnPtRuIRUb3--