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

Revision history for this message
Chuck Short (zulcss) wrote :

> 2011/3/25 Chuck Short <email address hidden>:
> >> > +    """Setup the LXC container
> >> > +
> >> > +    It will mount the loopback image to the container directory in order
> >> > +    to create the root filesystem for the container
> >> > +    """
> >> > +    nbd = False
> >> > +    device = _link_device(image, nbd)
> >> > +    err = utils.execute('sudo', 'mount', device, container_dir)
> >> > +    if err:
> >>
> >> As I've pointed out before, utils.execute returns a 2-tuple. 2-tuples
> evaluate
> >> to True, so as far as I can see, this will always evaluate to True?
> > There is the case where the nbd module is not loaded when the device is
> created so probably the the check be something like: (pseudocode)
> >
> > if device:
> >  raise exception
>
> You've lost me entirely. My point is this: utils.execute returns a
> 2-tuple (or raises an exception). The 2-tuple consists of what was
> output on the process' stdout and stderr, respectively. You're storing
> this 2-tuple in a single variable, so "if err" will, as far as I can
> see, always evaluate to True? If that is so, aren't you always
> unmounting your container immediately after mounting it?

I see what you are saying now I have corrected this now.

>
> > One or more two things, I havent added the test for the filesystem because i
> havent figured out how to access the chunk in the xml.
>
> There are a number of examples of poking around in the XML in the
> libvirt tests already. There are even checks that verify that the disk
> paths are set correctly based on whether it's a rescue system or not.
> Can you share what you've tried and we can try to work it out together?

I didnt think of looking at the libvirt tests. I believe I have added the tests that you have wanted.

> --
> Soren Hansen        | http://linux2go.dk/
> Ubuntu Developer    | http://www.ubuntu.com/
> OpenStack Developer | http://www.openstack.org/

« Back to merge proposal