From 69dd45d382f241389ff19983a4f12404f0a63662 Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Sat, 22 Jan 2011 04:57:11 +0000 Subject: Refactoring _destroy into steps --- nova/virt/xenapi/vmops.py | 68 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 22 deletions(-) (limited to 'nova') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 6c2fd6a68..7ae93b243 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -252,41 +252,65 @@ class VMOps(object): raise RuntimeError(resp_dict['message']) return resp_dict['message'] - def destroy(self, instance): - """Destroy VM instance""" - vm = VMHelper.lookup(self._session, instance.name) - return self._destroy(instance, vm, shutdown=True) + def _shutdown(self, instance, vm): + """Shutdown an instance """ + try: + task = self._session.call_xenapi('Async.VM.hard_shutdown', vm) + self._session.wait_for_task(instance.id, task) + except self.XenAPI.Failure, exc: + LOG.exception(exc) - def _destroy(self, instance, vm, shutdown=True): - """ Destroy VM instance """ - if vm is None: - # Don't complain, just return. This lets us clean up instances - # that have already disappeared from the underlying platform. - return - # Get the VDIs related to the VM + def _destroy_vdis(self, instance, vm): + """Destroys all VDIs associated with a VM """ vdis = VMHelper.lookup_vm_vdis(self._session, vm) - if shutdown: + + if not vdis: + return + + for vdi in vdis: try: - task = self._session.call_xenapi('Async.VM.hard_shutdown', vm) + task = self._session.call_xenapi('Async.VDI.destroy', vdi) self._session.wait_for_task(instance.id, task) except self.XenAPI.Failure, exc: LOG.exception(exc) - # Disk clean-up - if vdis: - for vdi in vdis: - try: - task = self._session.call_xenapi('Async.VDI.destroy', vdi) - self._session.wait_for_task(instance.id, task) - except self.XenAPI.Failure, exc: - LOG.exception(exc) - # VM Destroy + def _destroy_vm(self, instance, vm): + """Destroys a VM record """ try: task = self._session.call_xenapi('Async.VM.destroy', vm) self._session.wait_for_task(instance.id, task) except self.XenAPI.Failure, exc: LOG.exception(exc) + def destroy(self, instance): + """ + Destroy VM instance + + This is the method exposed by xenapi_conn.destroy(). The rest of the + destroy_* methods are internal. + """ + vm = VMHelper.lookup(self._session, instance.name) + return self._destroy(instance, vm, shutdown=True) + + def _destroy(self, instance, vm, shutdown=True): + """ + Destroys VM instance by performing: + + 1. A shutdown if requested + 2. Destroying associated VDIs + 3. Destroying that actual VM record + """ + if vm is None: + # Don't complain, just return. This lets us clean up instances + # that have already disappeared from the underlying platform. + return + + if shutdown: + self._shutdown(instance, vm) + + self._destroy_vdis(instance, vm) + self._destroy_vm(instance, vm) + def _wait_with_callback(self, instance_id, task, callback): ret = None try: -- cgit From f5b051bbd0858294157f911ecbb04542fadbb38d Mon Sep 17 00:00:00 2001 From: Rick Harris Date: Sat, 22 Jan 2011 04:59:58 +0000 Subject: Skip shutdown if already halted --- nova/virt/xenapi/vmops.py | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'nova') diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 7ae93b243..2ccd1ec24 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -254,6 +254,12 @@ class VMOps(object): def _shutdown(self, instance, vm): """Shutdown an instance """ + state = self.get_info(instance['name'])['state'] + if state == power_state.SHUTDOWN: + LOG.warn(_("VM %(vm)s already halted, skipping shutdown...") % + locals()) + return + try: task = self._session.call_xenapi('Async.VM.hard_shutdown', vm) self._session.wait_for_task(instance.id, task) -- cgit From 354c16138abed0e4c43a4b0e0e21dfe7d2106a7a Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 22 Jan 2011 15:14:33 -0800 Subject: add test and fix describe regions --- nova/api/ec2/cloud.py | 8 ++++---- nova/flags.py | 2 +- nova/tests/test_cloud.py | 11 +++++++++++ 3 files changed, 16 insertions(+), 5 deletions(-) (limited to 'nova') diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index f63ec9085..43b9a88e1 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -252,18 +252,18 @@ class CloudController(object): regions = [] for region in FLAGS.region_list: name, _sep, host = region.partition('=') - endpoint = '%s://%s:%s%s' % (FLAGS.ec2_prefix, + endpoint = '%s://%s:%s%s' % (FLAGS.ec2_scheme, host, FLAGS.ec2_port, - FLAGS.ec2_suffix) + FLAGS.ec2_path) regions.append({'regionName': name, 'regionEndpoint': endpoint}) else: regions = [{'regionName': 'nova', - 'regionEndpoint': '%s://%s:%s%s' % (FLAGS.ec2_prefix, + 'regionEndpoint': '%s://%s:%s%s' % (FLAGS.ec2_scheme, FLAGS.ec2_host, FLAGS.ec2_port, - FLAGS.ec2_suffix)}] + FLAGS.ec2_path)}] return {'regionInfo': regions} def describe_snapshots(self, diff --git a/nova/flags.py b/nova/flags.py index 81e2e36f9..43bc174d2 100644 --- a/nova/flags.py +++ b/nova/flags.py @@ -218,7 +218,7 @@ def _get_my_ip(): DEFINE_string('my_ip', _get_my_ip(), 'host ip address') DEFINE_list('region_list', [], - 'list of region=url pairs separated by commas') + 'list of region=fqdn pairs separated by commas') DEFINE_string('connection_type', 'libvirt', 'libvirt, xenapi or fake') DEFINE_string('aws_access_key_id', 'admin', 'AWS Access ID') DEFINE_string('aws_secret_access_key', 'admin', 'AWS Access Key') diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 771b1fcc0..4d7639339 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -87,6 +87,17 @@ class CloudTestCase(test.TestCase): # NOTE(vish): create depends on pool, so just call helper directly return cloud._gen_key(self.context, self.context.user.id, name) + def test_describe_regions(self): + """Makes sure describe regions runs without raising an exception""" + result = self.cloud.describe_regions(self.context) + self.assertEqual(len(result['regionInfo']), 1) + regions = FLAGS.region_list + FLAGS.region_list = ["one=test_host1", "two=test_host2"] + result = self.cloud.describe_regions(self.context) + print result + self.assertEqual(len(result['regionInfo']), 2) + regions = FLAGS.region_list + def test_describe_addresses(self): """Makes sure describe addresses runs without raising an exception""" address = "10.10.10.10" -- cgit From 1fc00fd38629735352dd887573f3a4fca8946601 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 22 Jan 2011 15:17:43 -0800 Subject: remove extra print --- nova/tests/test_cloud.py | 1 - 1 file changed, 1 deletion(-) (limited to 'nova') diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 4d7639339..5e456e615 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -94,7 +94,6 @@ class CloudTestCase(test.TestCase): regions = FLAGS.region_list FLAGS.region_list = ["one=test_host1", "two=test_host2"] result = self.cloud.describe_regions(self.context) - print result self.assertEqual(len(result['regionInfo']), 2) regions = FLAGS.region_list -- cgit From fb4638f1cbe17ce0c2e0eb4580f81f3cb787de0f Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 22 Jan 2011 17:46:05 -0800 Subject: fix reversed assignment --- nova/tests/test_cloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nova') diff --git a/nova/tests/test_cloud.py b/nova/tests/test_cloud.py index 5e456e615..445cc6e8b 100644 --- a/nova/tests/test_cloud.py +++ b/nova/tests/test_cloud.py @@ -95,7 +95,7 @@ class CloudTestCase(test.TestCase): FLAGS.region_list = ["one=test_host1", "two=test_host2"] result = self.cloud.describe_regions(self.context) self.assertEqual(len(result['regionInfo']), 2) - regions = FLAGS.region_list + FLAGS.region_list = regions def test_describe_addresses(self): """Makes sure describe addresses runs without raising an exception""" -- cgit