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

Bug#800845: autopkgtest: Add support for nested VMs



Hey Christian,

thanks for working on this! Indeed I once had a patch to adt-virt-qemu
which would make its own image available as /dev/qemu_image or
something such, but I can't find it now for the life of me.

Christian Seiler [2015-10-04 13:51 +0200]:
> as per our discussion on the autopkgtest mailing list [1], I'd like to
> be able to have autopkgtest support nested VMs for testing of network
> clients in the kernel such as NFS, CIFS, iSCSI, NBD, etc.

Back then I actually had that mostly working, except that the nested
qemu was really unstable. But in later releases QEMU got much beteter,
and your trick to set the emulated CPU to the host CPU might just be
the remaining brick :-)

>  - have a copy of the qcow2 base image in the test environment

OOI, is this merely an implementation detail, or do you really need
qcow2? If we'd export this as a block device (symlink /dev/qemu_image
-> /dev/xdb or whatever it will actually end up like), you avoid
introducing an assumption about the "outside" image format. This block
device would be readonly, so that you can only use it in overlay mode
for the nested QEMU.

> I've attached two patches that implement the necessary changes to
> autopkgtest.

I'll comment on the actual patches, that's easier.

>     Note that nested KVM will also require a module option to be set.
>     (nested=1 for either the kvm_intel or kvm_amd module.) Setting
>     -cpu host has no negative side effects even if nested=0 is set on
>     the host - then the kvm_* modules will not load in the VM and KVM
>     will simply be not available - same as without -cpu host.

Where does this need to be set, on the host, or in the outer QEMU
instance? The patch doesn't cover this bit, and it should at least be
documented.

> --- a/doc/README.package-tests.rst
> +++ b/doc/README.package-tests.rst
> @@ -202,6 +202,26 @@ needs-recommends
>      Enable installation of recommended packages in apt for the test
>      dependencies. This does not affect build dependencies.
>  
> +needs-qcow2-baseimage
> +    The test needs to have a read-only qcow2 base image available so it
> +    may create an overlay and start a qemu/KVM virtual machine inside
> +    the test environment.

This is a way too specific restriction that I want to make this
official and unchangeable API for eternity. If you want to do the same
with LXC or nspawn etc., we'd have to come up with similarly
complicated and specific names.

Something like "needs-nesting" would be more general, but it's still
not sufficient to tell the testbed everything it needs to know, in
particular where the base image is. With QEMU and LXC exporting a
block device to the testbed should work. But unless you are
super-careful, tests which make use of that will be written for one
specific adt runner (i. e. -qemu).

Can we start small with not declaring a particular restriction (and
corresponding test bed capabilitiy), and instead the test just skips
itself if it doesn't find $ADT_QCOW2_BASEIMAGE, or /dev/qemu_image,
etc.?

> --- a/virt-subproc/adt-virt-qemu
> +++ b/virt-subproc/adt-virt-qemu
> @@ -80,6 +80,8 @@ def parse_args():
>                          help='Enable debugging output')
>      parser.add_argument('--qemu-options',
>                          help='Pass through arguments to QEMU command.')
> +    parser.add_argument('--nested-qcow2-baseimage',
> +                        help='qcow2 VM base image for use inside the VM (nested VMs)')

I don't think we need this. This should be really cheap to set up, so
just always make it available.

>  def prepare_overlay():
> @@ -274,6 +278,10 @@ EOF
>  def make_auxverb(shared_dir):
>      '''Create auxverb script'''
>  
> +    envvars = ''
> +    if args.nested_qcow2_baseimage != None:
> +        envvars += 'export ADT_QCOW2_BASEIMAGE=/dev/vd%c ; ' % chr(ord('a') + len(args.image))

As I said above, could this instead be added as an extra readonly
-drive, and you add an extra command to provide a symlink
/dev/qemu_image to it? Then the test doesn't have to make an
assumption about the image format any more, and this can be changed in
the future.

> @@ -475,6 +485,13 @@ def hook_open():
>      for i, image in enumerate(args.image[1:]):
>          argv.append('-drive')
>          argv.append('file=%s,if=virtio,index=%i,readonly' % (image, i + 1))
> +    if args.nested_qcow2_baseimage != None:
> +        # export base image as drive (it's the easiest way to get
> +        # it into QEMU), but make sure format=raw is set, because
> +        # we want the VM to be able to see it as a qcow2 image and
> +        # not have QEMU interpret it
> +        argv.append('-drive')
> +        argv.append('file=%s,if=virtio,index=%i,readonly,format=raw' % (args.nested_qcow2_baseimage, len(args.image)))

Right, like that, but as a real drive, not a block device which isn't
actually one. :-)

>  def hook_capabilities():
> -    global normal_user
> +    global normal_user, args
>      caps = ['revert', 'revert-full-system', 'root-on-testbed',
>              'isolation-machine', 'reboot']
>      # disabled, see hook_downtmp()
>      # caps.append('downtmp-host=%s' % os.path.join(workdir, 'shared', 'tmp'))
>      if normal_user:
>          caps.append('suggested-normal-user=' + normal_user)
> +    if args.nested_qcow2_baseimage != None:
> +        caps.append('provides-qcow2-baseimage')
>      return caps

See above, this should then go, or maybe in the future become a
"nesting" or so.

> --- a/virt-subproc/adt-virt-qemu
> +++ b/virt-subproc/adt-virt-qemu
> @@ -495,6 +495,10 @@ def hook_open():
>  
>      if os.path.exists('/dev/kvm'):
>          argv.append('-enable-kvm')
> +        # emulate host CPU so that nested KVM might work (if it's
> +        # enabled)
> +        argv.append('-cpu')
> +        argv.append('host')

We've seen test regressions/hangs/failures because of changing CPU
models, in things like mesa or LLVM. I wouldn't like to do this by
default as it introduces additional
unpredictability/unreproducibility. But turning that into an option
and documenting that it is recommended for nesting is okay for me. (I
don't think it's strictly necessary -- with current QEMU and kernels
nesting seems to work reasonably well with the default emulated CPU).

Thanks!

Martin
-- 
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: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/autopkgtest-devel/attachments/20151005/39f236ba/attachment.sig>


Reply to: