On Tue, Feb 19, 2019 at 12:31:41PM +0100, Kevin Wolf wrote: > Am 19.02.2019 um 12:06 hat Daniel P. Berrangé geschrieben: > > On Tue, Feb 19, 2019 at 10:37:16AM +0100, Kevin Wolf wrote: > > > Am 19.02.2019 um 10:04 hat Thomas Huth geschrieben: > > > > > > > > https://gitlab.com/huth/qemu/-/jobs/163680780 > > > > > > > > Some of them apparently need encryption to be enabled (as already > > > > mentioned by Cleber in his patch) - thus should they really be in the > > > > quick check, too? Or could they at least check whether QEMU has been > > > > built with encryption? > > > > > > The correct solution would be that they detect the situation > > > automatically and skip the test by calling _notrun. > > > > > > I'm not sure how to detect if a given QEMU binary supports encryption, > > > but Dan might know. > > > > It isn't easy & depends which encryption feature you're trying to use. > > > > For TLS related features you can do something gross like > > > > qemu-img info --object tls-creds-anon,id=dummy README 2>&1 > > test $? != 0 && exit 0 > > > > This relies on fact that 'tls-creds-anon' object type will report a > > runtime error during initialization if gnutls isn't enabled. > > > > For more general ciphers you pretty much have to just try the higher level > > feature and see if it fails. > > Actually, I think for test cases we should see 'qemu-img create' failing > and could just skip the test if it returns a non-zero exit code. > > But then I looked at Thomas' output again: > > --- /builds/huth/qemu/tests/qemu-iotests/188.out 2019-02-19 > 08:23:54.000000000 +0000 > +++ /builds/huth/qemu/tests/qemu-iotests/188.out.bad 2019-02-19 > 08:34:54.000000000 +0000 > @@ -1,4 +1,5 @@ > QA output created by 188 > +qemu-img: TEST_DIR/t.IMGFMT: No crypto library supporting PBKDF in this > build: Function not implemented > Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 > encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10 > > == reading whole image ==--- > /builds/huth/qemu/tests/qemu-iotests/188.out 2019-02-19 > 08:23:54.000000000 +0000 > > What is it actually doing there? There's clearly an error message, but > it almost looks like it's creating some kind of image anyway? The > following I/O works fine (i.e. this created image can even be opened > again with the luks driver), except that you can also access the image > with the wrong password. > > Is this a real bug in either qcow2 or luks?
It is an artifact of the way qcow2 image creation happens in multiple phases. qcow2_co_create first creates a minimal qcow2 file, and then opens it and updates it to add in the various extra features, including luks encryption. We fail to create the luks encryption, but enough of the qcow2 file has been created that it is able to still do plain text I/O. Essentially the problem is that qcow2_co_create() doesn't unlink() the partially created image when things fail. This is a generic problem which can affect any part of qcow2_co_create that might fail, but it is especially problematic with luks. The complication in fixing this is that can't just do an unlink() as we can't assume a local file. We need to have a bdrv_unlink() driver callback we can use to delegate to the right block driver APIs for deletion. This is something I think could be useful in general, to expose as a "qemu-img delete" command - especially for non-local formats like rbd, sheepdog, glusterfs the mgmt app can't simply run "rm $img". QEMU already knows how to talk to the native APis for these network drivers so can make life easier by exposing a 'delete' op. For luks it would be desirable to allow options to the 'delete' operation, for example to request scrubbing of the key material headers. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
