Merge lp:~vishvananda/nova/lp829020 into lp:~hudson-openstack/nova/trunk

[Vish Ishaya]

Fixes libvirt rescue to use the same strategy as xen. Use a new copy of the base image as the rescue image. It leaves the original rescue image flags in, so a hand picked rescue image can still be used if desired.

[Devin Carlen (devcamcar)]

You've got a merge conflict that needs to be resolved.

review: Needs Fixing

[lp:~vishvananda/nova/lp829020]

1543. By Vish Ishaya on 2011-09-14

write out xml for rescue

1544. By Vish Ishaya on 2011-09-14

merge trunk

[Vish Ishaya (vishvananda)]

devin: fixed

[Devin Carlen (devcamcar)]

lgtm

review: Approve

[Dan Prince (dan-prince)]

Hey Vish,

I'm getting some permissions issues when trying to go into rescue mode using libvirt:

2011-09-16 14:58:03,439 INFO nova.virt.libvirt_conn [-] instance instance-00000001: Creating image
2011-09-16 14:58:03,440 ERROR nova.exception [-] Uncaught exception
(nova.exception): TRACE: Traceback (most recent call last):
(nova.exception): TRACE: File "/usr/lib/pymodules/python2.6/nova/exception.py", line 98, in wrapped
(nova.exception): TRACE: return f(*args, **kw)
(nova.exception): TRACE: File "/usr/lib/pymodules/python2.6/nova/virt/libvirt/connection.py", line 564, in rescue
(nova.exception): TRACE: network_info=network_info)
(nova.exception): TRACE: File "/usr/lib/pymodules/python2.6/nova/virt/libvirt/connection.py", line 818, in _create_image
(nova.exception): TRACE: os.O_CREAT | os.O_WRONLY, 0660))
(nova.exception): TRACE: OSError: [Errno 13] Permission denied: '/var/lib/nova/instances/instance-00000001/console.log'

review: Needs Fixing

[lp:~vishvananda/nova/lp829020]

1545. By Vish Ishaya on 2011-09-16

fix permissions

[Dan Prince (dan-prince)]

This looks good.

review: Approve

=== modified file 'nova/virt/libvirt/connection.py'
--- nova/virt/libvirt/connection.py	2011-09-16 01:57:03 +0000
+++ nova/virt/libvirt/connection.py	2011-09-16 15:52:14 +0000
@@ -29,9 +29,9 @@
                 (default: kvm).
 :libvirt_uri:  Override for the default libvirt URI (depends on libvirt_type).
 :libvirt_xml_template:  Libvirt XML Template.
-:rescue_image_id:  Rescue ami image (default: ami-rescue).
-:rescue_kernel_id:  Rescue aki image (default: aki-rescue).
-:rescue_ramdisk_id:  Rescue ari image (default: ari-rescue).
+:rescue_image_id:  Rescue ami image (None = original image).
+:rescue_kernel_id:  Rescue aki image (None = original image).
+:rescue_ramdisk_id:  Rescue ari image (None = original image).
 :injected_network_template:  Template file for injected network
 :allow_same_net_traffic:  Whether to allow in project network traffic
 
@@ -84,9 +84,9 @@
 FLAGS = flags.FLAGS
 flags.DECLARE('live_migration_retry_count', 'nova.compute.manager')
 # TODO(vish): These flags should probably go into a shared location
-flags.DEFINE_string('rescue_image_id', 'ami-rescue', 'Rescue ami image')
-flags.DEFINE_string('rescue_kernel_id', 'aki-rescue', 'Rescue aki image')
-flags.DEFINE_string('rescue_ramdisk_id', 'ari-rescue', 'Rescue ari image')
+flags.DEFINE_string('rescue_image_id', None, 'Rescue ami image')
+flags.DEFINE_string('rescue_kernel_id', None, 'Rescue aki image')
+flags.DEFINE_string('rescue_ramdisk_id', None, 'Rescue ari image')
 flags.DEFINE_string('libvirt_xml_template',
                     utils.abspath('virt/libvirt.xml.template'),
                     'Libvirt XML Template')
@@ -468,7 +468,7 @@
         snapshot_ptr.delete(0)
 
     @exception.wrap_exception()
-    def reboot(self, instance, network_info, reboot_type):
+    def reboot(self, instance, network_info, reboot_type=None, xml=None):
         """Reboot a virtual machine, given an instance reference.
 
         This method actually destroys and re-creates the domain to ensure the
@@ -479,7 +479,9 @@
         # NOTE(itoumsn): Use XML delived from the running instance
         # instead of using to_xml(instance, network_info). This is almost
         # the ultimate stupid workaround.
-        xml = virt_dom.XMLDesc(0)
+        if not xml:
+            xml = virt_dom.XMLDesc(0)
+
         # NOTE(itoumsn): self.shutdown() and wait instead of self.destroy() is
         # better because we cannot ensure flushing dirty buffers
         # in the guest OS. But, in case of KVM, shutdown() does not work...
@@ -543,43 +545,42 @@
         data recovery.
 
         """
-        self.destroy(instance, network_info, cleanup=False)
+
+        virt_dom = self._conn.lookupByName(instance['name'])
+        unrescue_xml = virt_dom.XMLDesc(0)
+        unrescue_xml_path = os.path.join(FLAGS.instances_path,
+                                         instance['name'],
+                                         'unrescue.xml')
+        f = open(unrescue_xml_path, 'w')
+        f.write(unrescue_xml)
+        f.close()
 
         xml = self.to_xml(instance, network_info, rescue=True)
-        rescue_images = {'image_id': FLAGS.rescue_image_id,
-                         'kernel_id': FLAGS.rescue_kernel_id,
-                         'ramdisk_id': FLAGS.rescue_ramdisk_id}
-        self._create_image(context, instance, xml, '.rescue', rescue_images)
-        self._create_new_domain(xml)
-
-        def _wait_for_rescue():
-            """Called at an interval until the VM is running again."""
-            instance_name = instance['name']
-
-            try:
-                state = self.get_info(instance_name)['state']
-            except exception.NotFound:
-                msg = _("During reboot, %s disappeared.") % instance_name
-                LOG.error(msg)
-                raise utils.LoopingCallDone
-
-            if state == power_state.RUNNING:
-                msg = _("Instance %s rescued successfully.") % instance_name
-                LOG.info(msg)
-                raise utils.LoopingCallDone
-
-        timer = utils.LoopingCall(_wait_for_rescue)
-        return timer.start(interval=0.5, now=True)
+        rescue_images = {
+            'image_id': FLAGS.rescue_image_id or instance['image_ref'],
+            'kernel_id': FLAGS.rescue_kernel_id or instance['kernel_id'],
+            'ramdisk_id': FLAGS.rescue_ramdisk_id or instance['ramdisk_id'],
+        }
+        self._create_image(context, instance, xml, '.rescue', rescue_images,
+                           network_info=network_info)
+        self.reboot(instance, network_info, xml=xml)
 
     @exception.wrap_exception()
-    def unrescue(self, instance, network_info):
+    def unrescue(self, instance, callback, network_info):
         """Reboot the VM which is being rescued back into primary images.
 
         Because reboot destroys and re-creates instances, unresue should
         simply call reboot.
 
         """
-        self.reboot(instance, network_info)
+        unrescue_xml_path = os.path.join(FLAGS.instances_path,
+                                         instance['name'],
+                                         'unrescue.xml')
+        f = open(unrescue_xml_path)
+        unrescue_xml = f.read()
+        f.close()
+        os.remove(unrescue_xml_path)
+        self.reboot(instance, network_info, xml=unrescue_xml)
 
     @exception.wrap_exception()
     def poll_rescued_instances(self, timeout):
@@ -814,8 +815,10 @@
             utils.execute('mkdir', '-p', container_dir)
 
         # NOTE(vish): No need add the suffix to console.log
-        os.close(os.open(basepath('console.log', ''),
-                         os.O_CREAT | os.O_WRONLY, 0660))
+        console_log = basepath('console.log', '')
+        if os.path.exists(console_log):
+            utils.execute('chown', os.getuid(), console_log, run_as_root=True)
+        os.close(os.open(console_log, os.O_CREAT | os.O_WRONLY, 0660))
 
         if not disk_images:
             disk_images = {'image_id': inst['image_ref'],