On Thu, Jan 21, 2016 at 09:01:19PM +0800, Fam Zheng wrote: > 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.
Yeah, thinking about it, my macro approach is probably overkill for the forseeable future anyway. I was future proofing wrt possible addition of trucrypt support, but that's soo far off its not something i really need worry about now. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|