Bug#800845: autopkgtest: Add support for nested VMs

Thu Mar 3 01:01:39 UTC 2016

Control: clone -1 -2
Control: retitle -2 
Control: block -2 by -1 Properly support skipping nested VM tests in incompatible virt-subprocs
Control: severity -2 wishlist
Control: tags -2 - patch

Hi Martin,

Moving this to the front:

>>   I split this out from the first patch, because this is what you
>>   didn't like the first time around. I still think a new restriction is
>>   a good idea, in order to be able to distinguish between skipped tests
>>   and tests that were actually successful.
> 
> Right, getting proper SKIPs is desirable. But the new restriction
> would otherwise be rather pointless.
> 
> Maybe this is all trying to be overly abstract, and what we really
> want is just "Restrictions: virt-{qemu,lxc,lxd,schroot,...}"? Given
> that tests must be written in a very adt-virt-* specific way, they
> should also be able to just plain say which runner they are compatible
> with.
> 
> Let's get the other two patches in shape first, formalizing this new
> restriction isn't a blocker AFAICS, right?

No, it isn't. Cloning this bug to track the SKIP stuff.

On 03/02/2016 11:52 PM, Martin Pitt wrote:
> Christian Seiler [2016-02-26 17:00 +0100]:
>> So I've spent some time again on this and have separated this into
>> three patches:
> 
> Thanks for getting back to this!

I want to get this done, so I can actually start working on proper
integration tests. :)

>> 0001-adt-virt-qemu-Implement-support-for-nested-base-imag.patch
> 
> We are homing in for this one, it just still looks a bit hackish to
> me:
> 
>>   This adds the image specified to adt-virt-qemu as an additional read
>>   only virtio block device (with serial BASEIMAGE) to the VM (in
>>   contrast to my first patch, this is the actual data, not a qcow2
>>   image).
>>
>>   It sets the ADT_BASEIMAGE environment variable to that device.
> 
> I'm not sure why we need *both* that environment variable *and*
> /dev/baseimage. This doesn't generalize very well to other runners
> anyway. I'd be fine with just /dev/baseimage.

Yes, thinking about it some more, /dev/baseimage is completely
sufficient.

>>   It also does two things with udev in setup-testbed:
>>
>>     1) it creates an udev rule to create a symlink /dev/baseimage to
>>        the new image (ADT_BASEIMAGE remains the absolute path for now,
>>        though)
>>
>>     2) it modifies 60-persistent-storage.rules to skip blkid processing
>>        of UUID etc. for the base image, since the base image is likely
>>        the same as the main image, so UUIDs would match. Otherwise the
>>        /dev/disk/by-uuid symlinks would point to the base image drive.
> 
> This is quite "eww".. Isn't it enough to add
> OPTIONS+="link_priority=-1000" to 61-baseimage.rules so that the
> symlinks of the real root device always win?

Oh, I didn't know about link_priority (but it's documented, so my bad).
Just tried it (removed the 60-...rules hack, added the link priority),
and it works.

>>   The same as with my first patch: if multiple images are specified on
>>   the adt-virt-qemu command line, it is impossible to determine what
>>   an approrpriate base image is
> 
> It is -- it must be the first one. Any subsequent ones can only be
> readonly auxiliary ones, they don't get an overlay, etc. The only
> practical application for this is to provide an iso9660 image for
> cloud-init if you want to use off-the-shelf cloud images. But this
> isn't necessary with adt-buildvm-ubuntu-cloud or vmdebootstrap, and I
> think for this case you can safely ignore any additional drives.

Ah ok, thinking about it: all additional drives are read-only anyway,
so they can't really be part of the file system that does stuff (such
as /usr, because in test setups that needs to be r/w). Sorry, I was
thinking far too generically.

>> hence my patch doesn't set the base image in that case, though you
>> may specify one explicitly via the command line.
> 
> I don't mind whether or not it sets /dev/baseimage in this case -- I
> think it can't hurt really. But a new CLI option is over the top IMO,
> can we please just drop this?
> 
> So I think just adding 61-baseimage.rules, the extra -drive, and
> documentation in the manpage about /dev/baseimage should be all that's
> needed.

I've attached an updated patch that does just this.

>> 0002-adt-virt-qemu-Use-host-CPU-type-by-default.patch
>>
>>   Passes -cpu host to QEMU by default _unless_ --qemu-command= is
>>   specified. (If qemu-command is specified, it might be the case that
>>   someone wants to run a fully emulated different architecture, and
>>   then -cpu host will fail spectacularly.)
> 
> As I already wrote before, I really don't like this as a default, as
> it introduces unpredictability into testbeds. We've seen tests that
> behave differently with different CPU models (X.org's LLVM
> software render and mesa for example), and getting random test
> passes/failures depending on which hw you run it on is really
> non-obvious.

Isn't this then a bug of the testbed? Also note that if you run those
tests (e.g. mesa) in e.g. LXC - or on a dedicated machine (doesn't
Ubuntu have some bare metal machines where tests requiring machine
isolation are run on?), you get the same issues.

Also note that the CPU vendor id is not replaced by QEMU - so while
the CPU features (flags in /proc/cpuinfo) correspond to the emulated
CPU, the vendor name (GenuineIntel or AuthenticAMD) remains that of
the host - at least in KVM mode.

> However, I'd be totally fine with changing the default to
> "-cpu SandyBridge". This is what nova-compute-qemu uses by default
> (apparently), so this both provides a reasonably rich CPU architecture
> and increases compatibility to test runs on a cloud instance, and
> keeps predictability. WDYT?

I don't have AMD hardware to test here, but if I try to do it the
other way around, e.g. "-cpu Opteron_G5" on my Haswell box, nested
KVM doesn't work. Problem is that AMD and Intel have completely
different KVM support - AMD has something called 'svm', Intel calls
it's technology 'vmx'. And if we start to emulate SandyBridge,
nested KVM won't work on AMD at all. (CPU features are disabled if
the host CPU doesn't support them. There are some warnings present.)
That's why I went with -cpu host - it's very nice and simple. :-)

Of course, we could pick SandyBridge for Intel and something else
for AMD (no idea what would be a good idea there, I haven't followed
AMD for a while) and reduce the possible CPUs to just 2.

>>   This can be overridden by the --qemu-cpu option for adt-virt-qemu,
>>   which then replaces the value of QEMU's -cpu parameter. If
>>   --qemu-cpu=qemu-default is specified, the -cpu option is dropped
>>   completely.
> 
> This option should be used so seldomly that I think
> --qemu-options='-cpu blah' is fine.

Ok, just checked: QEMU is fine with -cpu twice, takes the last one.
Then it's not a problem, should drop that option.

> So for example, the tests will never run on Debian's and
> Ubuntu's production CI.

This is a tangent, but wasn't somebody working on qemu testbeds for
the Debian CI?

>> [CAVEAT for base image stuff]
>>   The first patch that adds the base image as an additional read-only
>>   drive has a slight problem if old images are used
> 
> What are "old" images?

So let's say you create an image with the commands from autopkgtest
3.19.3 (current sid), that doesn't have the udev rules to make sure
that the UUIDs don't get reused, then you might run into a problem.
So one needs to make sure to recreate the images with the new udev
rules to be on the safe side.

Regards,
Christian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-adt-virt-qemu-Implement-support-for-nested-base-imag.patch
Type: text/x-diff
Size: 4699 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/autopkgtest-devel/attachments/20160303/40f292eb/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/autopkgtest-devel/attachments/20160303/40f292eb/attachment.sig>