[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#800845: autopkgtest: Add support for nested VMs

Hey Christian,

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!

> 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

>   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.

>   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?

>   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.

> 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

> 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

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 do like the "only specify -cpu if we use the default qemu" part

>   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.

> 0003-Add-needs-baseimage-restriction.patch

I already said that I feel that this is quite a stretch of what
Restrictions: should do -- they should apply to any kind of testbed,
and this concept doesn't generalize at all to schroot, lxc, ssh, etc.

On second thought maybe a more generic name would be
"needs-nested-virt"? qemu, LXC/LXD, and schroot can be nested in
principle (although we don't yet implement it for the latter ones),
while ssh and null would never provide this capability. LXC/schroots
wouldn't serve images, but you could e. g. provide a read-only bind
mount inside the container/schroot with the root fs.

But -- on third thought :-) -- Even with that tests will still be
specific to which adt-virt-* you use, as you have to do completely
different steps (and even have completely different test depends!) for
doing the nesting. So in the end, your test will always have a
/dev/baseimage with -qemu, and it can't work anyway with any other
runner. So for example, the tests will never run on Debian's and
Ubuntu's production CI.

>   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

Let's get the other two patches in shape first, formalizing this new
restriction isn't a blocker AFAICS, right?

>   (Of course, without a base image, one could do vmdebootstrap within
>   the outer VM, but that would be a huge waste of resources

Indeed, yes. Let's not encourage this :-)

>   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?

> since UUIDs are the same for filesystems on the main virtio drive
> and the base image "drive" (because udev processing of the base
> image "drive" is not disabled), partitions on both drives will have
> the same UUID, and apparently (at least on sid) udev selects the
> second drive for the symlinks, so /dev/disk/by-uuid/* will point to
> /dev/vdb*.

This is pretty much exactly what link_priority is for, so I'd hope it
works correctly with that?


Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/autopkgtest-devel/attachments/20160302/57d86f58/attachment.sig>

Reply to: