Merge lp:~zulcss/nova/nova-lxc into lp:~hudson-openstack/nova/trunk
- nova-lxc
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Soren Hansen |
Approved revision: | 816 |
Merged at revision: | 909 |
Proposed branch: | lp:~zulcss/nova/nova-lxc |
Merge into: | lp:~hudson-openstack/nova/trunk |
Diff against target: |
262 lines (+121/-12) 5 files modified
Authors (+1/-0) nova/tests/test_virt.py (+43/-0) nova/virt/disk.py (+35/-0) nova/virt/libvirt.xml.template (+21/-10) nova/virt/libvirt_conn.py (+21/-2) |
To merge this branch: | bzr merge lp:~zulcss/nova/nova-lxc |
Related bugs: | |
Related blueprints: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
termie (community) | Approve | ||
Thierry Carrez (community) | ffe | Approve | |
Soren Hansen (community) | Approve | ||
Rick Clark (community) | Approve | ||
Jay Pipes | Pending | ||
Vish Ishaya | Pending | ||
Review via email: mp+55260@code.launchpad.net |
This proposal supersedes a proposal from 2011-03-28.
Commit message
Description of the change
This branch adds support for linux containers (LXC) to nova. It uses the libvirt LXC driver to start and stop the instance.
Vish Ishaya (vishvananda) wrote : Posted in a previous version of this proposal | # |
Vish Ishaya (vishvananda) wrote : Posted in a previous version of this proposal | # |
Ok, I think I figured this out mostly. I created an ubuntu image using lxc-ubuntu and it seems to work properly. The busybox image launches properly as well but doesn't seem to be running sshd. I think having a ttylinux/busybox version that runs sshd and cloud-init would be very useful for testing.
Jay Pipes (jaypipes) wrote : Posted in a previous version of this proposal | # |
Hi Chuck!
149 + utils.execute(
Does the above need a sudo?
Also, I note that there are no test cases included for this new functionality. Could we add some?
Cheers!
jay
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> Hi Chuck!
>
> 149 + utils.execute(
>
> Does the above need a sudo?
>
> Also, I note that there are no test cases included for this new functionality.
> Could we add some?
>
> Cheers!
> jay
Hi Jay,
It doesnt need sudo because its running as the nova user. Also the tests were added in revision 753.
chuck
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> While I'm very excited about using lxc with nova, I am unable to get this to
> work. The instance reports being launched correctly but doesn't seem to
> actually succeed
> virsh --connect lxc:/// list shows nothing and I get the following error in
> /var/log/
> 17:09:05.465: error : lxcFdForward:231 : read of fd 7 failed: Input/output
> error
> Can we get some sort of instructions as to the setup necessary to make this
> work. Does it require a special image? Does the image have to match the host
> os? Do we need to do special setup to get it to work? apt-get install lxc?
> anything else?
>
> FWIW I am testing using the ttylinux (ami-tty). Maverick is my host os.
Right Ive added some documentation. It can be found at:
You will also need a newer version of libvirt (preferably the one from natty).
chuck
Vish Ishaya (vishvananda) wrote : Posted in a previous version of this proposal | # |
Ok, we definitely need to add the new libvirt to our ppa to support older installs.
I'm still working on getting this running properly. I'll let you know if i run into any issues.
Vish
On Feb 28, 2011, at 10:59 AM, Chuck Short wrote:
>> While I'm very excited about using lxc with nova, I am unable to get this to
>> work. The instance reports being launched correctly but doesn't seem to
>> actually succeed
>> virsh --connect lxc:/// list shows nothing and I get the following error in
>> /var/log/
>> 17:09:05.465: error : lxcFdForward:231 : read of fd 7 failed: Input/output
>> error
>> Can we get some sort of instructions as to the setup necessary to make this
>> work. Does it require a special image? Does the image have to match the host
>> os? Do we need to do special setup to get it to work? apt-get install lxc?
>> anything else?
>>
>> FWIW I am testing using the ttylinux (ami-tty). Maverick is my host os.
>
> Right Ive added some documentation. It can be found at:
>
> http://
>
> You will also need a newer version of libvirt (preferably the one from natty).
>
> chuck
> --
> https:/
> You are reviewing the proposed merge of lp:~zulcss/nova/nova-lxc into lp:nova.
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
You don't seem to ever free the loopback you set up in setup_container?
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
2011/3/7 Chuck Short <email address hidden>:
> http://
AFAICT, that will still leak loopback devices (and corresponding
mapped partitions)?
--
Soren Hansen | http://
Ubuntu Developer | http://
OpenStack Developer | http://
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> 2011/3/7 Chuck Short <email address hidden>:
> > http://
>
> AFAICT, that will still leak loopback devices (and corresponding
> mapped partitions)?
>
> --
> Soren Hansen | http://
> Ubuntu Developer | http://
> OpenStack Developer | http://
Ok I think I cleared this up in the last commit.
chuck
Nachi Ueno (nati-ueno) wrote : Posted in a previous version of this proposal | # |
LXC support on Nova is pretty cool!
Would you please add procedure in http://
Jay Pipes (jaypipes) wrote : Posted in a previous version of this proposal | # |
lgtm.
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
> > 2011/3/7 Chuck Short <email address hidden>:
> > > http://
> > AFAICT, that will still leak loopback devices (and corresponding
> > mapped partitions)?
> Ok I think I cleared this up in the last commit.
Sorry, I don't see it. Can you explain how the partition devices and loopback devices get cleaned up? It's not obvious to me, I'm afraid.
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
2011/3/14 Chuck Short <email address hidden>:
> Soren,
>
> Please see:
>
> http://
I did, but I still don't see it. Ok, from the top:
=== modified file 'nova/virt/disk.py'
--- nova/virt/disk.py 2011-03-12 23:53:10 +0000
+++ nova/virt/disk.py 2011-03-14 20:56:29 +0000
@@ -115,6 +115,39 @@
+def setup_container
+ """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)
Ok, so here you attach the disk image through nbd or losetup.
+ try:
+ if not partition is None:
+ # create partition
+ utils.execute('sudo kpartx -a %s' % device)
This needs to be changed to utils.execute(
Also, why is this needed? nbd devices should benefit from the kernel's
built in partition detection. If it's for non-nbd devices, make it
conditional.
+ mapped_device = '/dev/mapper/%p%s' % (device.
+ partition)
+ else:
+ mapped_device = device
+
+ utils.execute('sudo mount %s %s' %(mapped_device, container_dir))
This too needs to be changed to pass individual argv elements separately.
+
+ except Exception as e:
+ LOG.warn(_('Unable to mount container'))
+ if not partition is None:
+ # remove partitions
+ utils.execute('sudo kpartx -s %s' % device)
What does "kpartx -s" do?
+ _unlink_
+
+def destroy_
+ """Destroy the container once it terminates"""
+ try:
+ utils.execute('sudo umount %s/rootfs' % target)
Here you unmount the rootfs. What about the mapped partitions from
kparts and the nbd device. Where do they get cleaned up?
+ image = os.path.
'' + 'disk')
What's this for?
+ except Exception as e:
This is scary. Can you make this more specific (and you can leave off
the "as e" bit if you're not using the exception for anything.
--
Soren Hansen | http://
Ubuntu Developer | http://
OpenStack Developer | http://
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
> === 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_
>
>
> +def setup_container
> + """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(
> + if err:
This will always evaluate as True. utils.execute returns a 2-tuple.
> + raise exception.
> + % err)
> + _unlink_
> +
> +def destroy_
> + """Destroy the container once it terminates"""
> + try:
> + container_dir = '%s/rootfs' % target
> + utils.execute(
> + finally:
> + image = os.path.
> + out, err = utils.execute(
> + device = out.strip()
> + if err:
> + raise execption.
> + %err)
> + utils.execute(
> +
> +
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?
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
Ok, Chuck just pointed out to me that he stopped using kpartx altogether, which I completely missed in my last review, so here's another review where I stop being an idiot.
> === 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_
>
>
> +def setup_container
> + """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(
What's mapped_device? I'm not really sure what the assumptions here are. In what format are you expecting these LXC images to be distributed?
> + if err:
The comment from my previous review still applies here.
> + raise exception.
> + % err)
> + _unlink_
> +
> +def destroy_
> + """Destroy the container once it terminates"""
> + try:
> + container_dir = '%s/rootfs' % target
> + utils.execute(
> + finally:
> + image = os.path.
> + out, err = utils.execute(
I still think this will create another loopback device..
> + device = out.strip()
> + if err:
> + raise execption.
> + %err)
> + utils.execute(
...and then remove it again?
With regard to unit tests, it would be nice if they'd check:
* if os/type was set to exe,
* if os/init was set to /sbin/init,
* and that that filesystem was set correctly.
After all, the number of conditionals in that template is making it a bit difficult to verify by just looking at it.
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> Ok, Chuck just pointed out to me that he stopped using kpartx altogether,
> which I completely missed in my last review, so here's another review where I
> stop being an idiot.
>
> > === 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_
> >
> >
> > +def setup_container
> > + """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(
>
> What's mapped_device? I'm not really sure what the assumptions here are. In
> what format are you expecting these LXC images to be distributed?
Ok well spotted. The assumption is that they are using a regular UEC image. LXC container support has already been added in the latest imgaes.
>
> > + if err:
>
> The comment from my previous review still applies here.
>
> > + raise exception.
> > + % err)
> > + _unlink_
> > +
> > +def destroy_
> > + """Destroy the container once it terminates"""
> > + try:
> > + container_dir = '%s/rootfs' % target
> > + utils.execute(
> > + finally:
> > + image = os.path.
> 'disk')
> > + out, err = utils.execute(
> image)
>
> I still think this will create another loopback device..
>
It wont, its getting the information associated with the associated image.
> > + device = out.strip()
> > + if err:
> > + raise execption.
> > + %err)
> > + utils.execute(
>
> ...and then remove it again?
>
Deleting it when the image is shutdown.
>
> With regard to unit tests, it would be nice if they'd check:
> * if os/type was set to exe,
> * if os/init was set to /sbin/init,
> * and that that filesystem was set correctly.
>
Sure I can do that.
> After all, the number of conditionals in that template is making it a bit
> difficult to verify by just looking at it.
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
2011/3/15 Chuck Short <email address hidden>:
>> What's mapped_device? I'm not really sure what the assumptions here are. In
>> what format are you expecting these LXC images to be distributed?
>
> Ok well spotted. The assumption is that they are using a regular UEC image. LXC container support has already been added in the latest imgaes.
So only raw disk images are supported? Ok.
>>> + raise exception.
>>> + % err)
>>> + _unlink_
>>> +
>>> +def destroy_
>>> + """Destroy the container once it terminates"""
>>> + try:
>>> + container_dir = '%s/rootfs' % target
>>> + utils.execute(
>>> + finally:
>>> + image = os.path.
>> 'disk')
>>> + out, err = utils.execute(
>> image)
>> I still think this will create another loopback device..
> It wont, its getting the information associated with the associated image
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
My review seems to have gotten cut short. :( /me retries
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
These are the missing bits:
>> I still think this will create another loopback device..
> It wont, its getting the information associated with the associated image.
What am I doing wrong/differently here:
[2011-03-15 13:45:03] soren@lenny:
of=testimg.img bs=512 count=1k
1024+0 blokke ind
1024+0 blokke ud
524288 byte (524 kB) kopieret, 0,0075592 s, 69,4 MB/s
[2011-03-15 13:45:15] soren@lenny:
--find --show $(pwd)/testimg.img
[sudo] password for soren:
/dev/loop0
[2011-03-15 13:45:38] soren@lenny:
--find --show $(pwd)/testimg.img
/dev/loop1
[2011-03-15 13:45:38] soren@lenny:
--find --show $(pwd)/testimg.img
/dev/loop2
[2011-03-15 13:45:39] soren@lenny:
--find --show $(pwd)/testimg.img
/dev/loop3
[2011-03-15 13:45:39] soren@lenny:
--find --show $(pwd)/testimg.img
/dev/loop4
[2011-03-15 13:45:39] soren@lenny:
--find --show $(pwd)/testimg.img
/dev/loop5
[2011-03-15 13:45:40] soren@lenny:
--find --show $(pwd)/testimg.img
/dev/loop6
[2011-03-15 13:45:40] soren@lenny:
--find --show $(pwd)/testimg.img
/dev/loop7
Each time I get a new loopback device.
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
In my defense, it was must lovelier formatted when I sent it the first time. :-/
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> In my defense, it was must lovelier formatted when I sent it the first time.
> :-/
Hah..
I think I addressed your comments, it should be ready to go now.
chuck
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
=======
ERROR: Failure: SyntaxError (can't assign to function call (test_virt.py, line 262))
-------
Traceback (most recent call last):
File "/usr/lib/
addr.filename, addr.module)
File "/usr/lib/
return self.importFrom
File "/usr/lib/
mod = load_module(
File "/home/
for i (check, expected_result) in enumerate(check):
SyntaxError: can't assign to function call
=======
FAIL: test_authors_
-------
Traceback (most recent call last):
File "/home/
'%r not listed in Authors' % missing)
AssertionError: set([u'<email address hidden>']) not listed in Authors
-------
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> =======
> ERROR: Failure: SyntaxError (can't assign to function call (test_virt.py, line
> 262))
> -------
> Traceback (most recent call last):
> File "/usr/lib/
> loadTestsFromName
> addr.filename, addr.module)
> File "/usr/lib/
> importFromPath
> return self.importFrom
> File "/usr/lib/
> importFromDir
> mod = load_module(
> File "/home/
> for i (check, expected_result) in enumerate(check):
> SyntaxError: can't assign to function call
>
> =======
> FAIL: test_authors_
> -------
> Traceback (most recent call last):
> File "/home/
> in test_authors_
> '%r not listed in Authors' % missing)
> AssertionError: set([u'<email address hidden>']) not listed in Authors
>
> -------
Ok thats fixed now...test suite runs cleanly now.
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
nova/tests/
Use 4 spaces per indentation level.
For really old code that you don't want to mess up, you can continue to
use 8-space tabs.
Okay: a = 1
Okay: if a == 0:\n a = 1
E111: a = 1
Okay: for item in items:\n pass
E112: for item in items:\npass
Okay: a = 1\nb = 2
E113: a = 1\n b = 2
nova/virt/
^
JCR: Trailing whitespace is superfluous.
FBM: Except when it occurs as part of a blank line (i.e. the line is
nothing but whitespace). According to Python docs[1] a line with only
whitespace is considered a blank line, and is to be ignored. However,
matching a blank line to its indentation level avoids mistakenly
pasting code into the standard Python interpreter.
[1] http://
The warning returned varies on whether the line itself is blank, for easier
filtering for those who want to indent their blank lines.
Okay: spam(1)
W291: spam(1)\s
W293: class Foo(object):\n \n bang = 12
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> nova/tests/
> self.assertEqua
> ^
> Use 4 spaces per indentation level.
>
> For really old code that you don't want to mess up, you can continue to
> use 8-space tabs.
>
> Okay: a = 1
> Okay: if a == 0:\n a = 1
> E111: a = 1
>
> Okay: for item in items:\n pass
> E112: for item in items:\npass
>
> Okay: a = 1\nb = 2
> E113: a = 1\n b = 2
> nova/virt/
>
> ^
> JCR: Trailing whitespace is superfluous.
> FBM: Except when it occurs as part of a blank line (i.e. the line is
> nothing but whitespace). According to Python docs[1] a line with only
> whitespace is considered a blank line, and is to be ignored. However,
> matching a blank line to its indentation level avoids mistakenly
> terminating a multi-line statement (e.g. class declaration) when
> pasting code into the standard Python interpreter.
>
> [1] http://
> lines
>
> The warning returned varies on whether the line itself is blank, for
> easier
> filtering for those who want to indent their blank lines.
>
> Okay: spam(1)
> W291: spam(1)\s
> W293: class Foo(object):\n \n bang = 12
And thats fixed.
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
> === modified file 'Authors'
> --- Authors 2011-03-15 18:19:47 +0000
> +++ Authors 2011-03-17 12:08:41 +0000
> @@ -11,6 +11,7 @@
> Chmouel Boudjnah <email address hidden>
> Chris Behrens <email address hidden>
> Christian Berendt <email address hidden>
> +Chuck Short <email address hidden>
If you want a different e-mail here, you can look at the .mailmap file.
> Cory Wright <email address hidden>
> Dan Prince <email address hidden>
> David Pravec <email address hidden>
>
> === modified file 'nova/tests/
> --- nova/tests/
> +++ nova/tests/
[...]
> @@ -302,8 +342,9 @@
> (lambda t: t.find(
> 'path')
> (lambda t: t.find(
> +
> if rescue:
> - common_checks += [
> + common_checks = [
Why? That will effectively disable a bunch of checks.
> (lambda t: t.findall(
> 'file')
> (lambda t: t.findall(
> @@ -326,14 +367,14 @@
> xml = conn.to_
> tree = xml_to_tree(xml)
> for i, (check, expected_result) in enumerate(checks):
> - self.assertEqua
> - expected_result,
> - '%s failed check %d' % (xml, i))
> + self.assertEqua
> + expected_result,
> + '%s failed check %d' % (xml, i))
What's going on here?
>
> for i, (check, expected_result) in enumerate(
> - self.assertEqua
> - expected_result,
> - '%s failed common check %d' % (xml, i))
> + self.assertEqua
> + expected_result,
> + '%s failed common check %d' % (xml, i))
...and here?
> === modified file 'nova/virt/disk.py'
> --- nova/virt/disk.py 2011-03-14 17:59:41 +0000
> +++ nova/virt/disk.py 2011-03-17 12:08:41 +0000
> @@ -23,6 +23,7 @@
> """
>
> import os
> +import string
> import tempfile
> import time
>
> @@ -115,6 +116,36 @@
> _unlink_
>
>
> +def setup_container
> + """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(
> + if err:
> + raise exception.
> + % err)
> + _unlink_
> +
> +
> +def destroy_
> + """Destroy the container once it termin...
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
> > nova/tests/
[...]
> > nova/virt/
> And thats fixed.
Um.. No. Maybe you forgot to commit/push?
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> > === modified file 'Authors'
> > --- Authors 2011-03-15 18:19:47 +0000
> > +++ Authors 2011-03-17 12:08:41 +0000
> > @@ -11,6 +11,7 @@
> > Chmouel Boudjnah <email address hidden>
> > Chris Behrens <email address hidden>
> > Christian Berendt <email address hidden>
> > +Chuck Short <email address hidden>
>
> If you want a different e-mail here, you can look at the .mailmap file.
>
> > Cory Wright <email address hidden>
> > Dan Prince <email address hidden>
> > David Pravec <email address hidden>
> >
> > === modified file 'nova/tests/
> > --- nova/tests/
> > +++ nova/tests/
> [...]
> > @@ -302,8 +342,9 @@
> > (lambda t: t.find(
> > 'path')
> > (lambda t: t.find(
> > +
> > if rescue:
> > - common_checks += [
> > + common_checks = [
>
> Why? That will effectively disable a bunch of checks.
This was a left over when I was trying to add tests for lxc.
>
> > (lambda t: t.findall(
> > 'file')
> > (lambda t: t.findall(
> > @@ -326,14 +367,14 @@
> > xml = conn.to_
> > tree = xml_to_tree(xml)
> > for i, (check, expected_result) in enumerate(checks):
> > - self.assertEqua
> > - expected_result,
> > - '%s failed check %d' % (xml, i))
> > + self.assertEqua
> > + expected_result,
> > + '%s failed check %d' % (xml, i))
>
> What's going on here?
ditto
> >
> > for i, (check, expected_result) in enumerate(
> > - self.assertEqua
> > - expected_result,
> > - '%s failed common check %d' % (xml, i))
> > + self.assertEqua
> > + expected_result,
> > + '%s failed common check %d' % (xml, i))
>
> ...and here?
done
> > === modified file 'nova/virt/disk.py'
> > --- nova/virt/disk.py 2011-03-14 17:59:41 +0000
> > +++ nova/virt/disk.py 2011-03-17 12:08:41 +0000
> > @@ -23,6 +23,7 @@
> > """
> >
> > import os
> > +import string
> > import tempfile
> > import time
> >
> > @@ -115,6 +116,36 @@
> > _unlink_
> >
> >
> > +def setup_container
> > + """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(
> > + if err:
> > + raise except...
Rick Clark (dendrobates) wrote : Posted in a previous version of this proposal | # |
I've tested this and it works for me. I am approving this, but I want to make sure that Soren is satisfied that all his concerns were addressed before merging it.
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
> === modified file 'nova/virt/disk.py'
> --- nova/virt/disk.py 2011-03-17 15:29:04 +0000
> +++ nova/virt/disk.py 2011-03-24 10:26:05 +0000
> @@ -115,6 +115,37 @@
> _unlink_
>
>
> +def setup_container
You don't use the partition kwarg, so don't pass it. It's confusing.
> + """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(
> + if err:
> + raise exception.
> + % err)
> + _unlink_
> +
> +
> +def destroy_
> + """Destroy the container once it terminates
> +
> + It will umount the container that is mounted, try to find the loopback
> + device associated with the container and delete it.
> + """
> + try:
> + container_dir = '%s/rootfs' % target
> + utils.execute(
> + finally:
> + for loop in utils.popen('sudo losetup -a').readlines():
There's no such things as utils.popen.
> + if instance['name'] in loop:
> + device = loop.split(loop, ':')
> + utils.execute(
> +
> +
> def _link_device(image, nbd):
> """Link image to device using loopback or nbd"""
> if nbd:
>
> === modified file 'nova/virt/
> --- nova/virt/
> +++ nova/virt/
> @@ -2,6 +2,12 @@
> <name>$
> <memory>
> <os>
> +#if $type == 'lxc'
> + #set $disk_prefix = ''
> + #set $disk_bus = ''
> + <type>exe</type>
> + <init>/
> +#else
> #if $type == 'uml'
> #set $disk_prefix = 'ubd'
> #set $disk_bus = 'uml'
> @@ -37,6 +43,7 @@
> <boot dev="hd" />
> #end if
> #end if
> + #end if
Your indentations don't line up.
> #end if
> </os>
> <features>
> @@ -44,6 +51,12 @@
> </features>
> <vcpu>$
> <devices>
> +#if $type == 'lxc'
> + <filesystem type='mount'>
> + <source dir='${
> + <target dir='/'/>
> + </filesystem>
> +#else
You're still missing a unit test for this. What's the difficulty?
> #if $getVar('rescue', False)
> <disk type='file'>
> <driver type='$
> @@ -68,6 +81,7 @@
> <target dev='${
> </disk>
> #end if
> + #end if
Again, odd indentation. It's hard enough to read as it is. Please make sure that #if's line up with their corresponding "#end if"s.
> === modified file 'nova/virt/
> --- nova/virt/
> +++ nova/virt/
> @@ -20,7 +20,7 @@
> """
> A connection to a hypervisor through libvirt.
>
> -Supports KVM, ...
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
> === modified file 'nova/virt/disk.py'
> --- nova/virt/disk.py 2011-03-14 17:59:41 +0000
> +++ nova/virt/disk.py 2011-03-24 22:32:44 +0000
> @@ -115,6 +115,38 @@
> _unlink_
>
>
> +def setup_container
Still accepting an unused partition argument.
> + """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(
> + 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?
> + raise exception.
> + % err)
> + _unlink_
> +
> +
> +def destroy_
> + """Destroy the container once it terminates
> +
> + It will umount the container that is mounted, try to find the loopback
> + device associated with the container and delete it.
> + """
> + try:
> + container_dir = '%s/rootfs' % target
> + utils.execute(
> + finally:
> + out, err = utils('sudo', 'losetup', '-a')
There's no function named utils.
> + for loop in out.splitlines():
> + if instance['name'] in loop:
> + device = loop.split(loop, ':')
> + utils.execute(
> +
> +
> def _link_device(image, nbd):
> """Link image to device using loopback or nbd"""
> if nbd:
>
> === modified file 'nova/virt/
> --- nova/virt/
> +++ nova/virt/
Ok, if you take what you have there now and apply this patch:
http://
...then it's fine. Indentation-wise, at least.
> === modified file 'nova/virt/
> --- nova/virt/
> +++ nova/virt/
> @@ -720,6 +734,11 @@
> disk.inject_
> partition=
> nbd=FLAGS.
> +
> + if FLAGS.libvirt_type == 'lxc':
> + disk.setup_
> + container_
> + partition=
...and still passing the unused partition argument.
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
This is a very cool feature, by the way. I'm really looking forward to getting it in. We're close :)
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> > === modified file 'nova/virt/disk.py'
> > --- nova/virt/disk.py 2011-03-14 17:59:41 +0000
> > +++ nova/virt/disk.py 2011-03-24 22:32:44 +0000
> > @@ -115,6 +115,38 @@
> > _unlink_
> >
> >
> > +def setup_container
>
> Still accepting an unused partition argument.
>
Fixed
> > + """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(
> > + 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
>
> > + raise exception.
> > + % err)
> > + _unlink_
> > +
> > +
>
> > +def destroy_
> > + """Destroy the container once it terminates
> > +
> > + It will umount the container that is mounted, try to find the loopback
> > + device associated with the container and delete it.
> > + """
> > + try:
> > + container_dir = '%s/rootfs' % target
> > + utils.execute(
> > + finally:
> > + out, err = utils('sudo', 'losetup', '-a')
>
> There's no function named utils.
>
fixed
> > + for loop in out.splitlines():
> > + if instance['name'] in loop:
> > + device = loop.split(loop, ':')
> > + utils.execute(
> > +
> > +
> > def _link_device(image, nbd):
> > """Link image to device using loopback or nbd"""
> > if nbd:
> >
>
> > === modified file 'nova/virt/
> > --- nova/virt/
> > +++ nova/virt/
>
> Ok, if you take what you have there now and apply this patch:
>
> http://
>
> ...then it's fine. Indentation-wise, at least.
>
> > === modified file 'nova/virt/
> > --- nova/virt/
> > +++ nova/virt/
> > @@ -720,6 +734,11 @@
> > disk.inject_
> > partition=
> > nbd=FLAGS.
> > +
> > + if FLAGS.libvirt_type == 'lxc':
> > + disk.setup_
> > + container_
> > + partition=
>
> ...and still passing the unused partition argument.
One or more two things, I havent added the test for the filesystem because i havent f...
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
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(
>> > + 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?
> 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?
--
Soren Hansen | http://
Ubuntu Developer | http://
OpenStack Developer | http://
Thierry Carrez (ttx) wrote : Posted in a previous version of this proposal | # |
Interesting feature, self-contained, almost there: FFE granted for late merging by Tuesday EOD
termie (termie) wrote : Posted in a previous version of this proposal | # |
- you've got extra whitespaces at lines: 33, 38
- 41: you should be using self.flags(
- your test seems to rely on libvirt being available which is not always the case, perhaps you should skip the test if libvirt is not available? otherwise it doesn't seem like it is testing anything.
- at line 51 you should move line 52 up to that line and indent line 53 to to match it
- 71: please add punctuation to your docstring
- 81: missing a space on your indent
- in the new xml template you appear to have removed a feature related to multiple nics, what was the reasoning behind that?
other than that stuff, looks pretty cool :)
Vish Ishaya (vishvananda) wrote : Posted in a previous version of this proposal | # |
termie mentioned this, but it appears that the following is all due to a bad merge:
<interface type='bridge'>
255 - <source bridge=
256 - <mac address=
257 + <source bridge=
258 + <mac address=
259 <!-- <model type='virtio'/> CANT RUN virtio network right now -->
260 - <filterref filter=
261 - <parameter name="IP" value="
262 - <parameter name="DHCPSERVER" value="
263 -#if $getVar(
264 - ${nic.extra_params}
265 + <filterref filter=
266 + <parameter name="IP" value="
267 + <parameter name="DHCPSERVER" value="
268 +#if $getVar(
269 + ${extra_params}
270 #end if
271 -#if $getVar(
272 - <parameter name="RASERVER" value="
273 +#if $getVar(
274 + <parameter name="RASERVER" value="
275 #end if
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> 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(
> >> > + 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://
> Ubuntu Developer | http://
> OpenStack Developer | http://
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> termie mentioned this, but it appears that the following is all due to a bad
> merge:
>
> <interface type='bridge'>
> 255 - <source bridge=
> 256 - <mac address=
> 257 + <source bridge=
> 258 + <mac address=
> 259 <!-- <model type='virtio'/> CANT RUN virtio network
> right now -->
> 260 - <filterref filter=
> 261 - <parameter name="IP" value="
> 262 - <parameter name="DHCPSERVER"
> value="
> 263 -#if $getVar(
> 264 - ${nic.extra_params}
> 265 + <filterref filter=
> 266 + <parameter name="IP" value="
> 267 + <parameter name="DHCPSERVER" value="
> />
> 268 +#if $getVar(
> 269 + ${extra_params}
> 270 #end if
> 271 -#if $getVar(
> 272 - <parameter name="RASERVER" value="
> />
> 273 +#if $getVar(
> 274 + <parameter name="RASERVER" value="
> 275 #end if
It was a bad merge, I have corrected it.
Vish Ishaya (vishvananda) wrote : Posted in a previous version of this proposal | # |
Everything looks good except for your setup and destroy container:
1) nbd should be based on FLAGS.use_
2) destroy container should use _unlink_device so it can destroy using nbd or loopback. Looks like there is some trickiness with how to keep track of the device that the image is using, but you need to support an nbd version and a loopback version.
NBD may be a performance issue, so an alternative solution is to force use_cow_images to be False if we are using lxc, and change the line in setup_container to nbd=False (without the quotes)
Vish Ishaya (vishvananda) : Posted in a previous version of this proposal | # |
termie (termie) wrote : Posted in a previous version of this proposal | # |
- extra whitespace in your fixed_ip dict in test_virt.py, and just under it in db.fixed_
- please add punctuation to your docstrings
Soren Hansen (soren) wrote : Posted in a previous version of this proposal | # |
> + device = _link_device(image, nbd)
> + out, err = utils.execute(
> + if err:
> + raise exception.
> + % err)
> + _unlink_
If mount gives a non-zero exit code, it'll raise an exception. You
should catch and handle that, too.
I think that's all I have to add that hasn't already been pointed out.
That said, I still haven't actually tested it, but we're at a point now
where, if you address these couple of things, I'm actually willing to
believe it'll behave :), and if not, help fix it up.
Chuck Short (zulcss) wrote : Posted in a previous version of this proposal | # |
> Everything looks good except for your setup and destroy container:
>
> 1) nbd should be based on FLAGS.use_
> "False" (which is a string btw so it will evaluate to True)
>
> 2) destroy container should use _unlink_device so it can destroy using nbd or
> loopback. Looks like there is some trickiness with how to keep track of the
> device that the image is using, but you need to support an nbd version and a
> loopback version.
>
> NBD may be a performance issue, so an alternative solution is to force
> use_cow_images to be False if we are using lxc, and change the line in
> setup_container to nbd=False (without the quotes)
I addressed this in the last commit. It should be ready to go now.
chuck
Soren Hansen (soren) wrote : | # |
> === modified file 'nova/virt/disk.py'
> --- nova/virt/disk.py 2011-03-24 22:39:39 +0000
> +++ nova/virt/disk.py 2011-03-29 11:48:17 +0000
> @@ -116,6 +116,41 @@
> _unlink_
>
>
> +def setup_container
> + """Setup the LXC container
> +
> + It will mount the loopback image to the container directory in order
> + to create the root filesystem for the container.
> +
> + LXC does not support qcow2 images yet.
> + """
> + device = _link_device(image, nbd)
> + out, err = utils.execute(
> + if err:
> + raise exception.
> + % err)
> + _unlink_
Again, utils.execute raises an exception if it gives a non-zero exit code. You should catch and handle that.
Your call to _unlink_device will never happen due to your raising an exception immediately before.
Chuck Short (zulcss) wrote : | # |
> > === modified file 'nova/virt/disk.py'
> > --- nova/virt/disk.py 2011-03-24 22:39:39 +0000
> > +++ nova/virt/disk.py 2011-03-29 11:48:17 +0000
> > @@ -116,6 +116,41 @@
> > _unlink_
> >
> >
> > +def setup_container
> > + """Setup the LXC container
> > +
> > + It will mount the loopback image to the container directory in order
> > + to create the root filesystem for the container.
> > +
> > + LXC does not support qcow2 images yet.
> > + """
> > + device = _link_device(image, nbd)
> > + out, err = utils.execute(
> > + if err:
> > + raise exception.
> > + % err)
> > + _unlink_
>
> Again, utils.execute raises an exception if it gives a non-zero exit code. You
> should catch and handle that.
>
> Your call to _unlink_device will never happen due to your raising an exception
> immediately before.
I changed it to a try/except block.
chuck
Rick Clark (dendrobates) wrote : | # |
Thanks for all the iterations, Chuck. I think this fixes all the outstanding issues.
termie (termie) wrote : | # |
You still have extra whitespaces in your dicts, the line numbers i mentioned earlier are still applicable.
You also still have not yet added punctuation to your docstrings.
Soren Hansen (soren) wrote : | # |
This looks good now. Thanks for this work!
- 816. By Chuck Short
-
Style fixes
Preview Diff
1 | === modified file 'Authors' |
2 | --- Authors 2011-03-25 13:38:57 +0000 |
3 | +++ Authors 2011-03-29 19:49:51 +0000 |
4 | @@ -12,6 +12,7 @@ |
5 | Chmouel Boudjnah <chmouel@chmouel.com> |
6 | Chris Behrens <cbehrens@codestud.com> |
7 | Christian Berendt <berendt@b1-systems.de> |
8 | +Chuck Short <zulcss@ubuntu.com> |
9 | Cory Wright <corywright@gmail.com> |
10 | Dan Prince <dan.prince@rackspace.com> |
11 | David Pravec <David.Pravec@danix.org> |
12 | |
13 | === modified file 'nova/tests/test_virt.py' |
14 | --- nova/tests/test_virt.py 2011-03-25 00:16:53 +0000 |
15 | +++ nova/tests/test_virt.py 2011-03-29 19:49:51 +0000 |
16 | @@ -225,6 +225,49 @@ |
17 | self._check_xml_and_uri(instance_data, expect_kernel=True, |
18 | expect_ramdisk=True, rescue=True) |
19 | |
20 | + def test_lxc_container_and_uri(self): |
21 | + instance_data = dict(self.test_instance) |
22 | + self._check_xml_and_container(instance_data) |
23 | + |
24 | + def _check_xml_and_container(self, instance): |
25 | + user_context = context.RequestContext(project=self.project, |
26 | + user=self.user) |
27 | + instance_ref = db.instance_create(user_context, instance) |
28 | + host = self.network.get_network_host(user_context.elevated()) |
29 | + network_ref = db.project_get_network(context.get_admin_context(), |
30 | + self.project.id) |
31 | + |
32 | + fixed_ip = {'address': self.test_ip, |
33 | + 'network_id': network_ref['id']} |
34 | + |
35 | + ctxt = context.get_admin_context() |
36 | + fixed_ip_ref = db.fixed_ip_create(ctxt, fixed_ip) |
37 | + db.fixed_ip_update(ctxt, self.test_ip, |
38 | + {'allocated': True, |
39 | + 'instance_id': instance_ref['id']}) |
40 | + |
41 | + self.flags(libvirt_type='lxc') |
42 | + conn = libvirt_conn.LibvirtConnection(True) |
43 | + |
44 | + uri = conn.get_uri() |
45 | + self.assertEquals(uri, 'lxc:///') |
46 | + |
47 | + xml = conn.to_xml(instance_ref) |
48 | + tree = xml_to_tree(xml) |
49 | + |
50 | + check = [ |
51 | + (lambda t: t.find('.').get('type'), 'lxc'), |
52 | + (lambda t: t.find('./os/type').text, 'exe'), |
53 | + (lambda t: t.find('./devices/filesystem/target').get('dir'), '/')] |
54 | + |
55 | + for i, (check, expected_result) in enumerate(check): |
56 | + self.assertEqual(check(tree), |
57 | + expected_result, |
58 | + '%s failed common check %d' % (xml, i)) |
59 | + |
60 | + target = tree.find('./devices/filesystem/source').get('dir') |
61 | + self.assertTrue(len(target) > 0) |
62 | + |
63 | def _check_xml_and_uri(self, instance, expect_ramdisk, expect_kernel, |
64 | rescue=False): |
65 | user_context = context.RequestContext(project=self.project, |
66 | |
67 | === modified file 'nova/virt/disk.py' |
68 | --- nova/virt/disk.py 2011-03-24 22:39:39 +0000 |
69 | +++ nova/virt/disk.py 2011-03-29 19:49:51 +0000 |
70 | @@ -116,6 +116,41 @@ |
71 | _unlink_device(device, nbd) |
72 | |
73 | |
74 | +def setup_container(image, container_dir=None, nbd=False): |
75 | + """Setup the LXC container. |
76 | + |
77 | + It will mount the loopback image to the container directory in order |
78 | + to create the root filesystem for the container. |
79 | + |
80 | + LXC does not support qcow2 images yet. |
81 | + """ |
82 | + try: |
83 | + device = _link_device(image, nbd) |
84 | + utils.execute('sudo', 'mount', device, container_dir) |
85 | + except Exception, exn: |
86 | + LOG.exception(_('Failed to mount filesystem: %s'), exn) |
87 | + _unlink_device(device, nbd) |
88 | + |
89 | + |
90 | +def destroy_container(target, instance, nbd=False): |
91 | + """Destroy the container once it terminates. |
92 | + |
93 | + It will umount the container that is mounted, try to find the loopback |
94 | + device associated with the container and delete it. |
95 | + |
96 | + LXC does not support qcow2 images yet. |
97 | + """ |
98 | + try: |
99 | + container_dir = '%s/rootfs' % target |
100 | + utils.execute('sudo', 'umount', container_dir) |
101 | + finally: |
102 | + out, err = utils.execute('sudo', 'losetup', '-a') |
103 | + for loop in out.splitlines(): |
104 | + if instance['name'] in loop: |
105 | + device = loop.split(loop, ':') |
106 | + _unlink_device(device, nbd) |
107 | + |
108 | + |
109 | def _link_device(image, nbd): |
110 | """Link image to device using loopback or nbd""" |
111 | if nbd: |
112 | |
113 | === modified file 'nova/virt/libvirt.xml.template' |
114 | --- nova/virt/libvirt.xml.template 2011-03-23 18:56:24 +0000 |
115 | +++ nova/virt/libvirt.xml.template 2011-03-29 19:49:51 +0000 |
116 | @@ -2,7 +2,12 @@ |
117 | <name>${name}</name> |
118 | <memory>${memory_kb}</memory> |
119 | <os> |
120 | -#if $type == 'uml' |
121 | +#if $type == 'lxc' |
122 | + #set $disk_prefix = '' |
123 | + #set $disk_bus = '' |
124 | + <type>exe</type> |
125 | + <init>/sbin/init</init> |
126 | +#else if $type == 'uml' |
127 | #set $disk_prefix = 'ubd' |
128 | #set $disk_bus = 'uml' |
129 | <type>uml</type> |
130 | @@ -44,7 +49,13 @@ |
131 | </features> |
132 | <vcpu>${vcpus}</vcpu> |
133 | <devices> |
134 | -#if $getVar('rescue', False) |
135 | +#if $type == 'lxc' |
136 | + <filesystem type='mount'> |
137 | + <source dir='${basepath}/rootfs'/> |
138 | + <target dir='/'/> |
139 | + </filesystem> |
140 | +#else |
141 | + #if $getVar('rescue', False) |
142 | <disk type='file'> |
143 | <driver type='${driver_type}'/> |
144 | <source file='${basepath}/disk.rescue'/> |
145 | @@ -55,18 +66,19 @@ |
146 | <source file='${basepath}/disk'/> |
147 | <target dev='${disk_prefix}b' bus='${disk_bus}'/> |
148 | </disk> |
149 | -#else |
150 | + #else |
151 | <disk type='file'> |
152 | <driver type='${driver_type}'/> |
153 | <source file='${basepath}/disk'/> |
154 | <target dev='${disk_prefix}a' bus='${disk_bus}'/> |
155 | </disk> |
156 | - #if $getVar('local', False) |
157 | - <disk type='file'> |
158 | - <driver type='${driver_type}'/> |
159 | - <source file='${basepath}/disk.local'/> |
160 | - <target dev='${disk_prefix}b' bus='${disk_bus}'/> |
161 | - </disk> |
162 | + #if $getVar('local', False) |
163 | + <disk type='file'> |
164 | + <driver type='${driver_type}'/> |
165 | + <source file='${basepath}/disk.local'/> |
166 | + <target dev='${disk_prefix}b' bus='${disk_bus}'/> |
167 | + </disk> |
168 | + #end if |
169 | #end if |
170 | #end if |
171 | |
172 | @@ -87,7 +99,6 @@ |
173 | </filterref> |
174 | </interface> |
175 | #end for |
176 | - |
177 | <!-- The order is significant here. File must be defined first --> |
178 | <serial type="file"> |
179 | <source path='${basepath}/console.log'/> |
180 | |
181 | === modified file 'nova/virt/libvirt_conn.py' |
182 | --- nova/virt/libvirt_conn.py 2011-03-28 17:47:11 +0000 |
183 | +++ nova/virt/libvirt_conn.py 2011-03-29 19:49:51 +0000 |
184 | @@ -20,7 +20,7 @@ |
185 | """ |
186 | A connection to a hypervisor through libvirt. |
187 | |
188 | -Supports KVM, QEMU, UML, and XEN. |
189 | +Supports KVM, LXC, QEMU, UML, and XEN. |
190 | |
191 | **Related Flags** |
192 | |
193 | @@ -86,7 +86,7 @@ |
194 | flags.DEFINE_string('libvirt_type', |
195 | 'kvm', |
196 | 'Libvirt domain type (valid options are: ' |
197 | - 'kvm, qemu, uml, xen)') |
198 | + 'kvm, lxc, qemu, uml, xen)') |
199 | flags.DEFINE_string('libvirt_uri', |
200 | '', |
201 | 'Override the default libvirt URI (which is dependent' |
202 | @@ -266,6 +266,8 @@ |
203 | uri = FLAGS.libvirt_uri or 'uml:///system' |
204 | elif FLAGS.libvirt_type == 'xen': |
205 | uri = FLAGS.libvirt_uri or 'xen:///' |
206 | + elif FLAGS.libvirt_type == 'lxc': |
207 | + uri = FLAGS.libvirt_uri or 'lxc:///' |
208 | else: |
209 | uri = FLAGS.libvirt_uri or 'qemu:///system' |
210 | return uri |
211 | @@ -344,6 +346,8 @@ |
212 | instance_name = instance['name'] |
213 | LOG.info(_('instance %(instance_name)s: deleting instance files' |
214 | ' %(target)s') % locals()) |
215 | + if FLAGS.libvirt_type == 'lxc': |
216 | + disk.destroy_container(target, instance, nbd=FLAGS.use_cow_images) |
217 | if os.path.exists(target): |
218 | shutil.rmtree(target) |
219 | |
220 | @@ -625,6 +629,9 @@ |
221 | instance['name']) |
222 | data = self._flush_xen_console(virsh_output) |
223 | fpath = self._append_to_file(data, console_log) |
224 | + elif FLAGS.libvirt_type == 'lxc': |
225 | + # LXC is also special |
226 | + LOG.info(_("Unable to read LXC console")) |
227 | else: |
228 | fpath = console_log |
229 | |
230 | @@ -738,6 +745,10 @@ |
231 | f.write(libvirt_xml) |
232 | f.close() |
233 | |
234 | + if FLAGS.libvirt_type == 'lxc': |
235 | + container_dir = '%s/rootfs' % basepath(suffix='') |
236 | + utils.execute('mkdir', '-p', container_dir) |
237 | + |
238 | # NOTE(vish): No need add the suffix to console.log |
239 | os.close(os.open(basepath('console.log', ''), |
240 | os.O_CREAT | os.O_WRONLY, 0660)) |
241 | @@ -797,6 +808,9 @@ |
242 | if not inst['kernel_id']: |
243 | target_partition = "1" |
244 | |
245 | + if FLAGS.libvirt_type == 'lxc': |
246 | + target_partition = None |
247 | + |
248 | key = str(inst['key_data']) |
249 | net = None |
250 | |
251 | @@ -842,6 +856,11 @@ |
252 | disk.inject_data(basepath('disk'), key, net, |
253 | partition=target_partition, |
254 | nbd=FLAGS.use_cow_images) |
255 | + |
256 | + if FLAGS.libvirt_type == 'lxc': |
257 | + disk.setup_container(basepath('disk'), |
258 | + container_dir=container_dir, |
259 | + nbd=FLAGS.use_cow_images) |
260 | except Exception as e: |
261 | # This could be a windows image, or a vmdk format disk |
262 | LOG.warn(_('instance %(inst_name)s: ignoring error injecting' |
While I'm very excited about using lxc with nova, I am unable to get this to work. The instance reports being launched correctly but doesn't seem to actually succeed libvirt/ lxc/instance- 00000001. log
virsh --connect lxc:/// list shows nothing and I get the following error in
/var/log/
17:09:05.465: error : lxcFdForward:231 : read of fd 7 failed: Input/output error
Can we get some sort of instructions as to the setup necessary to make this work. Does it require a special image? Does the image have to match the host os? Do we need to do special setup to get it to work? apt-get install lxc? anything else?
FWIW I am testing using the ttylinux (ami-tty). Maverick is my host os.