From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:43330) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gw4Kp-0006mN-NY for qemu-devel@nongnu.org; Tue, 19 Feb 2019 07:17:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gw4Ko-0006uA-HF for qemu-devel@nongnu.org; Tue, 19 Feb 2019 07:17:15 -0500 Date: Tue, 19 Feb 2019 13:16:57 +0100 From: Kevin Wolf Message-ID: <20190219121657.GL4727@localhost.localdomain> References: <1550058881-16351-1-git-send-email-thuth@redhat.com> <3374c532-c885-d26e-2d34-0454943c3905@redhat.com> <28c77705-33c2-f10b-9dae-331bc15c9596@redhat.com> <20190219075333.GA4727@localhost.localdomain> <20190219093716.GF4727@localhost.localdomain> <20190219110626.GC7154@redhat.com> <20190219113141.GJ4727@localhost.localdomain> <20190219120128.GF7154@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20190219120128.GF7154@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] Failing iotests in CI (was: Add a gitlab-ci file for Continuous Integration testing on Gitlab) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Daniel =?iso-8859-1?Q?P=2E_Berrang=E9?= Cc: Thomas Huth , Cleber Rosa , qemu-devel@nongnu.org, Fam Zheng , Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= , Alex =?iso-8859-1?Q?Benn=E9e?= , Qemu-block , =?iso-8859-1?Q?Marc-Andr=E9?= Lureau Am 19.02.2019 um 13:01 hat Daniel P. Berrang=E9 geschrieben: > On Tue, Feb 19, 2019 at 12:31:41PM +0100, Kevin Wolf wrote: > > Am 19.02.2019 um 12:06 hat Daniel P. Berrang=E9 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: > > > > >=20 > > > > > https://gitlab.com/huth/qemu/-/jobs/163680780 > > > > >=20 > > > > > Some of them apparently need encryption to be enabled (as alrea= dy > > > > > 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? > > > >=20 > > > > The correct solution would be that they detect the situation > > > > automatically and skip the test by calling _notrun. > > > >=20 > > > > I'm not sure how to detect if a given QEMU binary supports encryp= tion, > > > > but Dan might know. > > >=20 > > > It isn't easy & depends which encryption feature you're trying to u= se. > > >=20 > > > For TLS related features you can do something gross like > > >=20 > > > qemu-img info --object tls-creds-anon,id=3Ddummy README 2>&1 > > > test $? !=3D 0 && exit 0 > > >=20 > > > This relies on fact that 'tls-creds-anon' object type will report a > > > runtime error during initialization if gnutls isn't enabled. > > >=20 > > > For more general ciphers you pretty much have to just try the highe= r level > > > feature and see if it fails. > >=20 > > Actually, I think for test cases we should see 'qemu-img create' fail= ing > > and could just skip the test if it returns a non-zero exit code. > >=20 > > But then I looked at Thomas' output again: > >=20 > > --- /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 0= 8: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=3DIMGFMT size=3D16777216 enc= rypt.format=3Dluks encrypt.key-secret=3Dsec0 encrypt.iter-time=3D10 > >=20 > > =3D=3D reading whole image =3D=3D--- /builds/huth/qemu/tests/qem= u-iotests/188.out 2019-02-19 08:23:54.000000000 +0000 > >=20 > > What is it actually doing there? There's clearly an error message, bu= t > > 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 imag= e > > with the wrong password. > >=20 > > Is this a real bug in either qcow2 or luks? >=20 > 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. But why doesn't qcow2_update_options_prepare() fail then? If you pass encryption options like 188 does, but the image isn't encrypted, then the function at least attempts to error out. Where is this failing? > 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. >=20 > 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. .bdrv_co_create can't unlink at all, because that would be undoing something that it didn't even do itself. If I created a file (local or on a remote server that is accessed over a network protocol) and my blockdev-create command to create the qcow2 layer fails, I certainly don't expect the resource that I manually created to go away. What .bdrv_co_create could and probably should do is make the header invalid so that it's still recognised as qcow2 (to avoid probing it as raw), but opening it fails. > 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. >=20 > For luks it would be desirable to allow options to the 'delete' > operation, for example to request scrubbing of the key material > headers. This could still be useful, just not necessarily in the context of blockdev-create. It could actually be used in 'qemu-img create', which creates both the protocol layer and the format layer. Kevin