From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47406) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aMErv-0001Gk-Ef for qemu-devel@nongnu.org; Thu, 21 Jan 2016 08:01:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aMErp-00074Z-Dk for qemu-devel@nongnu.org; Thu, 21 Jan 2016 08:01:43 -0500 Date: Thu, 21 Jan 2016 21:01:19 +0800 From: Fam Zheng Message-ID: <20160121130119.GA22797@ad.usersys.redhat.com> References: <1453311539-1193-1-git-send-email-berrange@redhat.com> <1453311539-1193-11-git-send-email-berrange@redhat.com> <20160121091208.GB31470@ad.usersys.redhat.com> <20160121110230.GI19835@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160121110230.GI19835@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 10/17] block: add generic full disk encryption driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: Kevin Wolf , qemu-devel@nongnu.org, qemu-block@nongnu.org On Thu, 01/21 11:02, Daniel P. Berrange wrote: > On Thu, Jan 21, 2016 at 05:12:08PM +0800, Fam Zheng wrote: > > On Wed, 01/20 17:38, Daniel P. Berrange wrote: > > > + /* XXX Should we treat size as being total physical size > > > + * of the image (ie payload + encryption header), or just > > > + * the logical size of the image (ie payload). If the latter > > > + * then we need to extend 'size' to include the header > > > + * size */ > > > > The latter. :) > > Ok > > > > + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort); > > > +#define BLOCK_CRYPTO_DRIVER(name, format) \ > > > + static int block_crypto_probe_ ## name(const uint8_t *buf, \ > > > + int buf_size, \ > > > + const char *filename) { \ > > > + return block_crypto_probe_generic(format, \ > > > + buf, buf_size, filename); \ > > > + } \ > > > + \ > > > + static int block_crypto_open_ ## name(BlockDriverState *bs, \ > > > + QDict *options, \ > > > + int flags, \ > > > + Error **errp) \ > > > + { \ > > > + return block_crypto_open_generic(format, \ > > > + &block_crypto_runtime_opts_ ## name, \ > > > + bs, options, flags, errp); \ > > > + } \ > > > + \ > > > + static int block_crypto_create_ ## name(const char *filename, \ > > > + QemuOpts *opts, \ > > > + Error **errp) \ > > > + { \ > > > + return block_crypto_create_generic(format, \ > > > + filename, opts, errp); \ > > > + } \ > > > + \ > > > + BlockDriver bdrv_crypto_ ## name = { \ > > > + .format_name = #name, \ > > > + .instance_size = sizeof(BlockCrypto), \ > > > + .bdrv_probe = block_crypto_probe_ ## name, \ > > > + .bdrv_open = block_crypto_open_ ## name, \ > > > + .bdrv_close = block_crypto_close, \ > > > + .bdrv_create = block_crypto_create_ ## name, \ > > > + .create_opts = &block_crypto_create_opts_ ## name, \ > > > + \ > > > + .bdrv_co_readv = block_crypto_co_readv, \ > > > + .bdrv_co_writev = block_crypto_co_writev, \ > > > + .bdrv_getlength = block_crypto_getlength, \ > > > + } > > > + > > > +BLOCK_CRYPTO_DRIVER(luks, Q_CRYPTO_BLOCK_FORMAT_LUKS); > > > > Personally I really prefer a preprocessed version, for the ease of grep. > > I'm not sure I understand what you mean by a preprocessed version - could > you expand on that. I mean don't use macro concatenation and use plain symbols like in other block drivers. BlockDriver bdrv_crypto_luks = { .format_name = "luks", .instance_size = sizeof(BlockCrypto), .bdrv_probe = block_crypto_probe_luks, .bdrv_open = block_crypto_open_luks, ... mostly because it's easier to grep (or for refactoring with tools). But I can't how repeatitive this would be (I can see the "don't repeat yourself" with your approach). There is only one BLOCK_CRYPTO_DRIVER instance in this series. This is probably bikeshedding. Fam