Code review comment for lp:~justin-fathomdb/nova/raw-disk-images

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

I'm cool with Cheetah. I've used it before and I don't know of another templating system that's supposed to be the canonical one.

As for the udev network thing: Ubuntu will refrain from storing the MAC->interface-name mapping if the MAC address is "locally administered". nova.utils.generate_mac() generates such a MAC. If you can somehow get VirtualBox to accept that MAC, you should be able to remove that hack.

> @defer.inlineCallbacks
> -def inject_data(image, key=None, net=None, partition=None, execute=None):
> +def inject_data( image, key=None, net=None, dns=None,
> + remove_network_udev=False,
> + partition=None, execute=None):

PEP-8 says not to put whitespace immediately inside parentheses.

=== modified file 'nova/virt/libvirt_conn.py'
--- nova/virt/libvirt_conn.py 2010-08-18 21:14:24 +0000
+++ nova/virt/libvirt_conn.py 2010-08-24 09:44:45 +0000
@@ -260,13 +275,28 @@
     def toXml(self, instance):
         # TODO(termie): cache?
         logging.debug("Starting the toXML method")
+ template_contents = open(FLAGS.libvirt_xml_template).read()
         xml_info = instance.datamodel.copy()
         # TODO(joshua): Make this xml express the attached disks as well

Since I added UML capabilities to Nova, the template to use is already
put into self.libvirt_xml in __init__. Please use that instead, as
it's based on a different template if libvirt_type is "uml". Also, the
changes you made to libvirt.qemu.xml.template need to be done similarly
for libvirt.uml.xml.template.

Other than that, it looks good, and I'm really looking forward to seeing
this land! :)

review: Needs Fixing

« Back to merge proposal