more bugfixes, flag for local volumes

6 files changed, 27 insertions, 12 deletions

diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py
index 0df4d3710..c53ce6f5e 100644
--- a/nova/api/ec2/__init__.py
+++ b/nova/api/ec2/__init__.py
@@ -238,6 +238,7 @@ class Executor(wsgi.Application):
             return self._error(req, type(ex).__name__, str(ex))
 
     def _error(self, req, code, message):
+        logging.error("%s: %s", code, message)
         resp = webob.Response()
         resp.status = 400
         resp.headers['Content-Type'] = 'text/xml'
diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index a1899c47f..7a057396c 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -467,7 +467,7 @@ class CloudController(object):
         instance_data = None
         if volume.get('instance', None):
             internal_id = volume['instance']['internal_id']
-            ec2_id = internal_id_to_ec2_id(internal_id)
+            instance_ec2_id = internal_id_to_ec2_id(internal_id)
             instance_data = '%s[%s]' % (instance_ec2_id,
                                         volume['instance']['host'])
         v = {}
@@ -522,7 +522,10 @@ class CloudController(object):
                   "args": {"topic": FLAGS.volume_topic,
                            "volume_id": volume_ref['id']}})
 
-        return {'volumeSet': [self._format_volume(context, volume_ref)]}
+        # TODO(vish): Instance should be None at db layer instead of
+        #             trying to lazy load, but for now we turn it into
+        #             a dict to avoid an error.
+        return {'volumeSet': [self._format_volume(context, dict(volume_ref))]}
 
     def attach_volume(self, context, volume_id, instance_id, device, **kwargs):
         volume_ref = db.volume_get_by_ec2_id(context, volume_id)
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 3b3208fea..116bf11cc 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -113,7 +113,7 @@ class ComputeManager(manager.Manager):
         instance_ref = self.db.instance_get(context, instance_id)
         volumes = instance_ref.get('volumes', []) or []
         for volume in volumes:
-            self.detach_volume(instance_id, volume['id'])
+            self.detach_volume(context, instance_id, volume['id'])
         if instance_ref['state'] == power_state.SHUTOFF:
             self.db.instance_destroy(context, instance_id)
             raise exception.Error('trying to destroy already destroyed'
@@ -176,6 +176,8 @@ class ComputeManager(manager.Manager):
                                     instance_id,
                                     mountpoint)
         except Exception:
+            logging.debug("instance %s: attach failed to %s, removing export",
+                          instance_id, mountpoint)
             yield self.volume_manager.remove_compute_volume(context,
                                                             volume_id)
             raise
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 18d837e6b..b0adc3a2a 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -82,6 +82,12 @@ class NovaBase(object):
     def __getitem__(self, key):
         return getattr(self, key)
 
+    def get(self, key, default=None):
+        try:
+            return getattr(self, key)
+        except AttributeError, KeyError:
+            return default
+
     def __iter__(self):
         self._i = iter(object_mapper(self).columns)
         return self
diff --git a/nova/volume/driver.py b/nova/volume/driver.py
index eff56d9c6..bffe4d6b5 100644
--- a/nova/volume/driver.py
+++ b/nova/volume/driver.py
@@ -49,8 +49,8 @@ flags.DEFINE_integer('iscsi_target_ids',
                     'Number of iscsi target ids per host')
 flags.DEFINE_string('iscsi_target_prefix', 'iqn.2010-10.org.openstack:',
                     'prefix for iscsi volumes')
-flags.DEFINE_string('iscsi_ip_prefix', '127.0.0',
-                    'only connect to the specified ip')
+flags.DEFINE_string('iscsi_ip_prefix', '127.0',
+                    'discover volumes on the ip that starts with this prefix')
 
 
 class VolumeDriver(object):
@@ -107,6 +107,7 @@ class VolumeDriver(object):
 
     @defer.inlineCallbacks
     def local_path(self, volume):
+        yield  # NOTE(vish): stops deprecation warning
         defer.returnValue("/dev/%s/%s" % (FLAGS.volume_group, volume['name']))
 
     def ensure_export(self, context, volume):
@@ -261,7 +262,7 @@ class ISCSIDriver(VolumeDriver):
     @defer.inlineCallbacks
     def remove_export(self, context, volume):
         """Removes an export for a logical volume"""
-        target_id = self.db.volume_get_target_id(context, volume['name'])
+        target_id = self.db.volume_get_target_id(context, volume['id'])
         yield self._execute("sudo ietadm --op delete --tid=%s "
                             "--lun=0" % target_id)
         yield self._execute("sudo ietadm --op delete --tid=%s" %
@@ -282,7 +283,7 @@ class ISCSIDriver(VolumeDriver):
     def discover_volume(self, volume):
         """Discover volume on a remote host"""
         (iscsi_name,
-         iscsi_portal) = yield self._get_name_and_portal(volume['id'],
+         iscsi_portal) = yield self._get_name_and_portal(volume['name'],
                                                          volume['host'])
         yield self._execute("sudo iscsiadm -m node -T %s -p %s --login" %
                             (iscsi_name, iscsi_portal))
diff --git a/nova/volume/manager.py b/nova/volume/manager.py
index f6146efe9..bc49e28ee 100644
--- a/nova/volume/manager.py
+++ b/nova/volume/manager.py
@@ -39,6 +39,8 @@ flags.DEFINE_string('storage_availability_zone',
                     'availability zone of this service')
 flags.DEFINE_string('volume_driver', 'nova.volume.driver.ISCSIDriver',
                     'Driver to use for volume creation')
+flags.DEFINE_boolean('use_local_volumes', True,
+                     'if True, will not discover local volumes')
 
 
 class VolumeManager(manager.Manager):
@@ -61,7 +63,7 @@ class VolumeManager(manager.Manager):
         volumes = self.db.volume_get_all_by_host(ctxt, self.host)
         logging.debug("Re-exporting %s volumes", len(volumes))
         for volume in volumes:
-            self.driver.ensure_export(context, volume)
+            self.driver.ensure_export(ctxt, volume)
 
     @defer.inlineCallbacks
     def create_volume(self, context, volume_id):
@@ -100,6 +102,8 @@ class VolumeManager(manager.Manager):
             raise exception.Error("Volume is still attached")
         if volume_ref['host'] != self.host:
             raise exception.Error("Volume is not local to this node")
+        logging.debug("volume %s: removing export", volume_ref['name'])
+        yield self.driver.remove_export(context, volume_ref)
         logging.debug("volume %s: deleting", volume_ref['name'])
         yield self.driver.delete_volume(volume_ref)
         self.db.volume_destroy(context, volume_id)
@@ -114,8 +118,7 @@ class VolumeManager(manager.Manager):
         """
         context = context.elevated()
         volume_ref = self.db.volume_get(context, volume_id)
-        if volume_ref['host'] == self.host:
-            # NOTE(vish): No need to discover local volumes.
+        if volume_ref['host'] == self.host and FLAGS.use_local_volumes:
             path = yield self.driver.local_path(volume_ref)
         else:
             path = yield self.driver.discover_volume(volume_ref)
@@ -126,8 +129,7 @@ class VolumeManager(manager.Manager):
         """Remove remote volume on compute host """
         context = context.elevated()
         volume_ref = self.db.volume_get(context, volume_id)
-        if volume_ref['host'] == self.host:
-            # NOTE(vish): No need to undiscover local volumes.
+        if volume_ref['host'] == self.host and FLAGS.use_local_volumes:
             defer.returnValue(True)
         else:
             yield self.driver.undiscover_volume(volume_ref)