From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55428) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsI1P-0005pX-4H for qemu-devel@nongnu.org; Tue, 12 May 2015 17:47:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YsI1O-0007pp-3E for qemu-devel@nongnu.org; Tue, 12 May 2015 17:47:27 -0400 Message-ID: <555274E3.6060906@redhat.com> Date: Tue, 12 May 2015 15:47:15 -0600 From: Eric Blake MIME-Version: 1.0 References: <1431105726-3682-1-git-send-email-kwolf@redhat.com> <1431105726-3682-21-git-send-email-kwolf@redhat.com> In-Reply-To: <1431105726-3682-21-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="4FOUhXFxKtPEe7T5rEF1DFCEDPPraXdN2" 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: Kevin Wolf , qemu-block@nongnu.org Cc: mreitz@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --4FOUhXFxKtPEe7T5rEF1DFCEDPPraXdN2 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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 se= t > 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 > -/* 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; 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. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --4FOUhXFxKtPEe7T5rEF1DFCEDPPraXdN2 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVUnTjAAoJEKeha0olJ0NqHnAIAIFnVT/LlHPav+jvxYt/m0/N UL5I9g85OA+uhwhj6d5oldd+PFqWXG9wG9K4j94TGAmQnVPZ8xZUo4SYJXbmOhYV fL97Si6ZJwm8LW0bLF+9D3MAhLfkxkOUzmE48mbK0cshm5i2mFdfVBYt7xl6b+Cr WXPn0e4OaUiELUSa4VQ3YU8Dc+RAnIbyMeXeZrzW2MANZQtOR76B5voValS+SNph pue9DbFpA1yJlLlVY2o91tbs8GyBkzEEtF4LCwT3qfcN7qGpzGDK5XI3S7GUUlzR b3P6IHIeLA6Q42HIaszHTIYQmOfHGr/mICAwOUY7pBCTNyUzgFKGoDXM8D+BVL8= =XhLl -----END PGP SIGNATURE----- --4FOUhXFxKtPEe7T5rEF1DFCEDPPraXdN2--