From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39615) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aStHl-0007n8-S9 for qemu-devel@nongnu.org; Mon, 08 Feb 2016 16:23:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aStHk-0002Kq-Py for qemu-devel@nongnu.org; Mon, 08 Feb 2016 16:23:53 -0500 References: <1453311539-1193-1-git-send-email-berrange@redhat.com> <1453311539-1193-17-git-send-email-berrange@redhat.com> From: Eric Blake Message-ID: <56B9075C.5060406@redhat.com> Date: Mon, 8 Feb 2016 14:23:40 -0700 MIME-Version: 1.0 In-Reply-To: <1453311539-1193-17-git-send-email-berrange@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="5NfkbgRi262eTKqilhLBTKndvK0hLpRAl" Subject: Re: [Qemu-devel] [PATCH v2 16/17] block: remove all encryption handling APIs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Kevin Wolf , Fam Zheng , qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --5NfkbgRi262eTKqilhLBTKndvK0hLpRAl Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/20/2016 10:38 AM, Daniel P. Berrange wrote: > Now that all encryption keys must be provided upfront via > the QCryptoSecret API and associated block driver properties > there is no need for any explicit encryption handling APIs > in the block layer. Encryption can be handled transparently > within the block driver. We only retain an API for querying > whether an image is encrypted or not, since that is a > potentially useful piece of metadata to report to the user. >=20 > Signed-off-by: Daniel P. Berrange > --- > @@ -2190,7 +2181,6 @@ void bdrv_close(BlockDriverState *bs) > bs->backing_format[0] =3D '\0'; > bs->total_sectors =3D 0; > bs->encrypted =3D 0; > - bs->valid_key =3D 0; It would be nice if this patch (or maybe a followup) switched bs->encrypted to bool, rather than int. > -void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)= > -{ > - if (key) { > - if (!bdrv_is_encrypted(bs)) { > - error_setg(errp, "Node '%s' is not encrypted", > - bdrv_get_device_or_node_name(bs)); > - } else if (bdrv_set_key(bs, key) < 0) { > - error_setg(errp, QERR_INVALID_PASSWORD); If I'm not mistaken, this was the only use of QERR_INVALID_PASSWORD; please also nuke it from include/qapi/qmp/qerror.h. > - } > - } else { > - if (bdrv_key_required(bs)) { > - error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED, > - "'%s' (%s) is encrypted", Likewise, this looks like the last use of ERROR_CLASS_DEVICE_ENCRYPTED (since 15/17 nuked the only client in hmp.c that cared about the error class); it would be nice to nuke the remains in include/qapi/error.h and in qapi/common.json. > +++ b/blockdev.c > @@ -2261,24 +2257,8 @@ void qmp_block_passwd(bool has_device, const cha= r *device, > bool has_node_name, const char *node_name, > const char *password, Error **errp) > { > - Error *local_err =3D NULL; > - BlockDriverState *bs; > - AioContext *aio_context; > - > - bs =3D bdrv_lookup_bs(has_device ? device : NULL, > - has_node_name ? node_name : NULL, > - &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > - return; > - } > - > - aio_context =3D bdrv_get_aio_context(bs); > - aio_context_acquire(aio_context); > - > - bdrv_add_key(bs, password, errp); > - > - aio_context_release(aio_context); > + error_setg_errno(errp, -ENOSYS, > + "Setting block passwords directly is no longer su= pported"); We should document in qapi/block-core.json that the QMP 'block_passwd' command is deprecated, and will be removed in a future release (but I suspect we don't want to completely remove it in 2.6, so that we at least have time to find clients that were relying on it and should be using the new methods). > +++ b/include/block/block_int.h > @@ -374,7 +374,6 @@ struct BlockDriverState { > int read_only; /* if true, the media is read only */ > int open_flags; /* flags used to open the file, re-used for re-ope= n */ > int encrypted; /* if true, the media is encrypted */ > - int valid_key; /* if true, a valid encryption key has been set */ > int sg; /* if true, the device is a /dev/sg* */ > int copy_on_read; /* if true, copy read backing sectors into image= > note this is a reference count */ Hmm - several variables that are only 'true' or 'false' and should be typed 'bool'. Only 'encrypted' is a candidate for change in this patch; 'sg' is unrelated. And 'copy_on_read' should be tweaked to document 'nonzero', not 'true', since it is used as a reference count rather than bool. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --5NfkbgRi262eTKqilhLBTKndvK0hLpRAl 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/ iQEcBAEBCAAGBQJWuQdcAAoJEKeha0olJ0NqwhgH/1tfq/K8abLwtmIY5Z7NyXSR /0wAgNefH9Pf0wCye/a6CHy5HidqenqP4MUiBgTSIpxTMb8RRod1payYaIum2NRW pFSxsUiTuiH8Fw4XzDUBOzsbY85+HVfdl8oQlYWpLZci/jBd20T9POBuH4JFlfnI ioJB4FrZI3dGI2VhphAkQcGDKsircJu9ybSBRc0ZzUq6xz2kAS48cfi7zd3w9iMk YANeXUEYYpsymP5J2V1Kq48FVtabzisZ/hFhU14wgUrEgwgEhjvf6Szb45KH3L/N rSptqQ3SoEIM5jhdCAPTSL05CAhQyWvHPtlSzIbpeMOzoFHyTw7n3DnD4F28Dcw= =tMSP -----END PGP SIGNATURE----- --5NfkbgRi262eTKqilhLBTKndvK0hLpRAl--