Am 14.05.2019 um 13:13 hat Daniel P. Berrangé geschrieben: > On Mon, Mar 12, 2018 at 04:02:14PM +0100, Kevin Wolf wrote: > > Everything that refers to the protocol layer or QemuOpts is moved out of > > block_crypto_create_generic(), so that the remaining function is > > suitable to be called by a .bdrv_co_create implementation. > > > > LUKS is the only driver that actually implements the old interface, and > > we don't intend to use it in any new drivers, so put the moved out code > > directly into a LUKS function rather than creating a generic > > intermediate one. > > > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > > Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> > > Reviewed-by: Eric Blake <ebl...@redhat.com> > > --- > > block/crypto.c | 95 > > +++++++++++++++++++++++++++++++++++++--------------------- > > 1 file changed, 61 insertions(+), 34 deletions(-) > > > Reviving a year old commit... > > The LUKS driver doesn't implement preallocation during create. > > Before this commit this would be reported > > $ qemu-img create -f luks --object secret,id=sec0,data=base -o > key-secret=sec0 base.luks 1G -o preallocation=full > Formatting 'base.luks', fmt=luks size=1073741824 key-secret=sec0 > preallocation=full > qemu-img: base.luks: Parameter 'preallocation' is unexpected > > After this commit, there is no error reported - it just silently > ignores the preallocation=full option. > > I'm a bit lost in block layer understanding where is the right > place to fix the error reporting in this case.
Hmm, this looked strange at first, but I see now where the difference is. In the old state, crypto used qemu_opts_to_qdict() and then fed all options to its own visitor. I'm not sure whether I can clearly call this a bug, but it's different from other format drivers, which parse all options they know and pass the rest to the protocol driver. The new version was converted to use qemu_opts_to_qdict_filtered() like in all other drivers, so we got the same behaviour in crypto: Unknown options are passed to the protocol. As it happens, file-posix does support 'preallocation', so the operation will return success now. Of course, passing the preallocation to the protocol level is questionable for non-raw image formats, and completely useless when you create the protocol level with size 0 and you'll call blk_truncate() later. I think we would get back to the old state if you just changed the qemu_opts_to_qdict_filtered() call back to qemu_opts_to_qdict(). But it make the driver inconsistent with other drivers again. Maybe the best way to solve this would be to just implement preallocation. It should be as easy as adding a 'preallocation' create option (in QAPI and block_crypto_create_opts_luks), putting the PreallocMode into BlockCryptoCreateData and then passing it to the blk_truncate() call in block_crypto_init_func(). Kevin
