From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50652) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtfOV-0000Ki-9k for qemu-devel@nongnu.org; Wed, 26 Nov 2014 11:24:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtfOQ-0002Gb-AG for qemu-devel@nongnu.org; Wed, 26 Nov 2014 11:24:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40032) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtfOQ-0002GL-3S for qemu-devel@nongnu.org; Wed, 26 Nov 2014 11:24:38 -0500 Message-ID: <5475FEC0.3090306@redhat.com> Date: Wed, 26 Nov 2014 09:24:32 -0700 From: Eric Blake MIME-Version: 1.0 References: <1416487488-8423-1-git-send-email-mreitz@redhat.com> <1416487488-8423-2-git-send-email-mreitz@redhat.com> In-Reply-To: <1416487488-8423-2-git-send-email-mreitz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="NSfh51w51plHCOwLVfrhv47Ppd8D6AmOB" Subject: Re: [Qemu-devel] [PATCH 1/3] blockdev: Add read-only option to change-blockdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-devel@nongnu.org Cc: Kevin Wolf , Peter Lieven , Markus Armbruster , Stefan Hajnoczi , Luiz Capitulino This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --NSfh51w51plHCOwLVfrhv47Ppd8D6AmOB Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/20/2014 05:44 AM, Max Reitz wrote: > Add an option to qmp_change_blockdev() which allows changing the > read-only status of the block device to be changed. >=20 > Some drives do not have a inherently fixed read-only status; for > instance, floppy disks can be set read-only or writable independently o= f > the drive. Some users may find it useful to be able to therefore change= > the read-only status of a block device when changing the medium. >=20 > Signed-off-by: Max Reitz > --- > blockdev.c | 41 ++++++++++++++++++++++++++++++++++++++-= -- > include/sysemu/blockdev.h | 3 ++- > qapi-schema.json | 20 ++++++++++++++++++++ > qmp.c | 3 ++- > 4 files changed, 62 insertions(+), 5 deletions(-) >=20 > =20 > - qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp)= ; > + qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, &err)= ; > + > + if (err) { > + if (read_only =3D=3D BLOCKDEV_CHANGE_READ_ONLY_MODE_AUTO) { > + error_free(err); > + err =3D NULL; > + > + /* RDWR did not work, try RO now */ > + bdrv_flags &=3D ~BDRV_O_RDWR; > + qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NUL= L, errp); > + } else { > + error_propagate(errp, err); > + } Umm, why are you propagating the error here manually, when it was previously propagated as part of the fall-through into the out: label? Particularly since the second open_encrypted call still relies on fallthrough for propagating the error? I think this should be simplified to: if (err && read_only =3D=3D BLOCKDEV_CHANGE_READ_ONLY_MODE_AUTO) { error_free(err); err =3D NULL; /* RDWR did not work, try RO now */ bdrv_flags &=3D ~BDRV_O_RDWR; qmp_bdrv_open_encrypted(...); } Otherwise, looks okay to me. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --NSfh51w51plHCOwLVfrhv47Ppd8D6AmOB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJUdf7AAAoJEKeha0olJ0NqD/MIAIBViJGMCXCmJLafzkkcHJwT 28FrFCnjId20e2NUb5A6ktrB25jjcCR9mj1lF2ukVSmdcwg4OeY09W9bMwFGCw8z YkBuPRT4TCWV3Gp/bD4PXcOZ8hlJxx8odDkaYBriz5tSeQatgf4oIajmWuLJ3iFx w84/j3KxRI8XC+diy/x2vGsL56hkS4GNEdjrPzHG5gnW+cOuUkcC4WWI3aMwrifJ D+8TXc3p6w9dt7GBaV0r87PBirxJ7CSwzfNO6w/zeAFq/x0O+Qkv+I8J7Qh8ukEx iNC15tU/G7Qn8r4s3ztp9rS53pcn3ECYhwv8Yk3Wm2c20NRcsZ9nUmbPWA2772c= =jfhx -----END PGP SIGNATURE----- --NSfh51w51plHCOwLVfrhv47Ppd8D6AmOB--