Code review comment for lp:~zulcss/nova/nova-lxc

Revision history for this message
Soren Hansen (soren) wrote :

> === modified file 'nova/virt/disk.py'
> --- nova/virt/disk.py 2011-03-12 23:53:10 +0000
> +++ nova/virt/disk.py 2011-03-15 10:46:53 +0000
> @@ -115,6 +115,34 @@
> _unlink_device(device, nbd)
>
>
> +def setup_container(image, container_dir=None, partition=None, nbd=False):
> + """Setup the LXC container
> +
> + It will mount the loopback image to the container directory in order
> + to create the root filesystem for the container
> + """
> + device = _link_device(image, nbd)
> + err = utils.execute('sudo', 'mount', mapped_device, container_dir)
> + if err:

This will always evaluate as True. utils.execute returns a 2-tuple.

> + raise exception.Error(_('Failed to mount filesystem: %s')
> + % err)
> + _unlink_device(device, nbd)

> +
> +def destroy_container(target, instance, nbd=False):
> + """Destroy the container once it terminates"""
> + try:
> + container_dir = '%s/rootfs' % target
> + utils.execute('sudo', 'umount', container_dir)
> + finally:
> + image = os.path.join(FLAGS.instances_path, instance['name'], '' + 'disk')
> + out, err = utils.execute('sudo', 'losetup', '--find', '--show', image)
> + device = out.strip()
> + if err:
> + raise execption.Error(_('Could not find loopback image: %s')
> + %err)
> + utils.execute('sudo', 'losetup', '--detach', device)
> +
> +

You're still not clearing out the mapped partitions that you create with kpartx. Instead you seem to be creating an additional loopback device or is that not what losetup --find does?

review: Needs Fixing

« Back to merge proposal