Remove AoE, Clean up volume code

 * Removes Ata Over Ethernet
 * Adds drivers to libvirt for volumes
 * Adds initialize_connection and terminate_connection to volume api
 * Passes connection info back through volume api

Change-Id: I1b1626f40bebe8466ab410fb174683293c7c474f

4 files changed, 203 insertions, 250 deletions

diff --git a/nova/volume/api.py b/nova/volume/api.py
index d9c082514..34103a1f3 100644
--- a/nova/volume/api.py
+++ b/nova/volume/api.py
@@ -23,7 +23,6 @@ Handles all requests relating to volumes.
 
 from eventlet import greenthread
 
-from nova import db
 from nova import exception
 from nova import flags
 from nova import log as logging
@@ -180,12 +179,49 @@ class API(base.Base):
         if volume['status'] == "available":
             raise exception.ApiError(_("Volume is already detached"))
 
-    def remove_from_compute(self, context, volume_id, host):
+    def remove_from_compute(self, context, instance_id, volume_id, host):
         """Remove volume from specified compute host."""
         rpc.call(context,
                  self.db.queue_get_for(context, FLAGS.compute_topic, host),
-                 {"method": "remove_volume",
-                  "args": {'volume_id': volume_id}})
+                 {"method": "remove_volume_connection",
+                  "args": {'instance_id': instance_id,
+                           'volume_id': volume_id}})
+
+    def attach(self, context, volume_id, instance_id, mountpoint):
+        volume = self.get(context, volume_id)
+        host = volume['host']
+        queue = self.db.queue_get_for(context, FLAGS.volume_topic, host)
+        return rpc.call(context, queue,
+                        {"method": "attach_volume",
+                         "args": {"volume_id": volume_id,
+                                  "instance_id": instance_id,
+                                  "mountpoint": mountpoint}})
+
+    def detach(self, context, volume_id):
+        volume = self.get(context, volume_id)
+        host = volume['host']
+        queue = self.db.queue_get_for(context, FLAGS.volume_topic, host)
+        return rpc.call(context, queue,
+                 {"method": "detach_volume",
+                  "args": {"volume_id": volume_id}})
+
+    def initialize_connection(self, context, volume_id, address):
+        volume = self.get(context, volume_id)
+        host = volume['host']
+        queue = self.db.queue_get_for(context, FLAGS.volume_topic, host)
+        return rpc.call(context, queue,
+                        {"method": "initialize_connection",
+                         "args": {"volume_id": volume_id,
+                                  "address": address}})
+
+    def terminate_connection(self, context, volume_id, address):
+        volume = self.get(context, volume_id)
+        host = volume['host']
+        queue = self.db.queue_get_for(context, FLAGS.volume_topic, host)
+        return rpc.call(context, queue,
+                        {"method": "terminate_connection",
+                         "args": {"volume_id": volume_id,
+                                  "address": address}})
 
     def _create_snapshot(self, context, volume_id, name, description,
                          force=False):
diff --git a/nova/volume/driver.py b/nova/volume/driver.py
index e5bb498ed..2692f5e9c 100644
--- a/nova/volume/driver.py
+++ b/nova/volume/driver.py
@@ -20,8 +20,8 @@ Drivers for volumes.
 
 """
 
-import time
 import os
+import time
 from xml.etree import ElementTree
 
 from nova import exception
@@ -35,25 +35,17 @@ LOG = logging.getLogger("nova.volume.driver")
 FLAGS = flags.FLAGS
 flags.DEFINE_string('volume_group', 'nova-volumes',
                     'Name for the VG that will contain exported volumes')
-flags.DEFINE_string('aoe_eth_dev', 'eth0',
-                    'Which device to export the volumes on')
 flags.DEFINE_string('num_shell_tries', 3,
                     'number of times to attempt to run flakey shell commands')
 flags.DEFINE_string('num_iscsi_scan_tries', 3,
                     'number of times to rescan iSCSI target to find volume')
-flags.DEFINE_integer('num_shelves',
-                    100,
-                    'Number of vblade shelves')
-flags.DEFINE_integer('blades_per_shelf',
-                    16,
-                    'Number of vblade blades per shelf')
 flags.DEFINE_integer('iscsi_num_targets',
                     100,
                     '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', '$my_ip',
-                    'discover volumes on the ip that starts with this prefix')
+flags.DEFINE_string('iscsi_ip_address', '$my_ip',
+                    'use this ip for iscsi')
 flags.DEFINE_string('rbd_pool', 'rbd',
                     'the rbd pool in which volumes are stored')
 
@@ -202,16 +194,16 @@ class VolumeDriver(object):
         """Removes an export for a logical volume."""
         raise NotImplementedError()
 
-    def discover_volume(self, context, volume):
-        """Discover volume on a remote host."""
+    def check_for_export(self, context, volume_id):
+        """Make sure volume is exported."""
         raise NotImplementedError()
 
-    def undiscover_volume(self, volume):
-        """Undiscover volume on a remote host."""
+    def initialize_connection(self, volume, address):
+        """Allow connection to ip and return connection info."""
         raise NotImplementedError()
 
-    def check_for_export(self, context, volume_id):
-        """Make sure volume is exported."""
+    def terminate_connection(self, volume, address):
+        """Disallow connection from ip"""
         raise NotImplementedError()
 
     def get_volume_stats(self, refresh=False):
@@ -220,128 +212,6 @@ class VolumeDriver(object):
         return None
 
 
-class AOEDriver(VolumeDriver):
-    """WARNING! Deprecated. This driver will be removed in Essex. Its use
-    is not recommended.
-
-    Implements AOE specific volume commands."""
-
-    def __init__(self, *args, **kwargs):
-        LOG.warn(_("AOEDriver is deprecated and will be removed in Essex"))
-        super(AOEDriver, self).__init__(*args, **kwargs)
-
-    def ensure_export(self, context, volume):
-        # NOTE(vish): we depend on vblade-persist for recreating exports
-        pass
-
-    def _ensure_blades(self, context):
-        """Ensure that blades have been created in datastore."""
-        total_blades = FLAGS.num_shelves * FLAGS.blades_per_shelf
-        if self.db.export_device_count(context) >= total_blades:
-            return
-        for shelf_id in xrange(FLAGS.num_shelves):
-            for blade_id in xrange(FLAGS.blades_per_shelf):
-                dev = {'shelf_id': shelf_id, 'blade_id': blade_id}
-                self.db.export_device_create_safe(context, dev)
-
-    def create_export(self, context, volume):
-        """Creates an export for a logical volume."""
-        self._ensure_blades(context)
-        (shelf_id,
-         blade_id) = self.db.volume_allocate_shelf_and_blade(context,
-                                                             volume['id'])
-        self._try_execute(
-                 'vblade-persist', 'setup',
-                 shelf_id,
-                 blade_id,
-                 FLAGS.aoe_eth_dev,
-                 "/dev/%s/%s" %
-                 (FLAGS.volume_group,
-                  volume['name']),
-                 run_as_root=True)
-        # NOTE(vish): The standard _try_execute does not work here
-        #             because these methods throw errors if other
-        #             volumes on this host are in the process of
-        #             being created.  The good news is the command
-        #             still works for the other volumes, so we
-        #             just wait a bit for the current volume to
-        #             be ready and ignore any errors.
-        time.sleep(2)
-        self._execute('vblade-persist', 'auto', 'all',
-                      check_exit_code=False, run_as_root=True)
-        self._execute('vblade-persist', 'start', 'all',
-                      check_exit_code=False, run_as_root=True)
-
-    def remove_export(self, context, volume):
-        """Removes an export for a logical volume."""
-        (shelf_id,
-         blade_id) = self.db.volume_get_shelf_and_blade(context,
-                                                        volume['id'])
-        self._try_execute('vblade-persist', 'stop',
-                          shelf_id, blade_id, run_as_root=True)
-        self._try_execute('vblade-persist', 'destroy',
-                          shelf_id, blade_id, run_as_root=True)
-
-    def discover_volume(self, context, _volume):
-        """Discover volume on a remote host."""
-        (shelf_id,
-         blade_id) = self.db.volume_get_shelf_and_blade(context,
-                                                        _volume['id'])
-        self._execute('aoe-discover', run_as_root=True)
-        out, err = self._execute('aoe-stat', check_exit_code=False,
-                                 run_as_root=True)
-        device_path = 'e%(shelf_id)d.%(blade_id)d' % locals()
-        if out.find(device_path) >= 0:
-            return "/dev/etherd/%s" % device_path
-        else:
-            return
-
-    def undiscover_volume(self, _volume):
-        """Undiscover volume on a remote host."""
-        pass
-
-    def check_for_export(self, context, volume_id):
-        """Make sure volume is exported."""
-        (shelf_id,
-         blade_id) = self.db.volume_get_shelf_and_blade(context,
-                                                        volume_id)
-        cmd = ('vblade-persist', 'ls', '--no-header')
-        out, _err = self._execute(*cmd, run_as_root=True)
-        exported = False
-        for line in out.split('\n'):
-            param = line.split(' ')
-            if len(param) == 6 and param[0] == str(shelf_id) \
-                    and param[1] == str(blade_id) and param[-1] == "run":
-                exported = True
-                break
-        if not exported:
-            # Instance will be terminated in this case.
-            desc = _("Cannot confirm exported volume id:%(volume_id)s. "
-                     "vblade process for e%(shelf_id)s.%(blade_id)s "
-                     "isn't running.") % locals()
-            raise exception.ProcessExecutionError(out, _err, cmd=cmd,
-                                                  description=desc)
-
-
-class FakeAOEDriver(AOEDriver):
-    """Logs calls instead of executing."""
-
-    def __init__(self, *args, **kwargs):
-        super(FakeAOEDriver, self).__init__(execute=self.fake_execute,
-                                            sync_exec=self.fake_execute,
-                                            *args, **kwargs)
-
-    def check_for_setup_error(self):
-        """No setup necessary in fake mode."""
-        pass
-
-    @staticmethod
-    def fake_execute(cmd, *_args, **_kwargs):
-        """Execute that simply logs the command."""
-        LOG.debug(_("FAKE AOE: %s"), cmd)
-        return (None, None)
-
-
 class ISCSIDriver(VolumeDriver):
     """Executes commands relating to ISCSI volumes.
 
@@ -445,7 +315,7 @@ class ISCSIDriver(VolumeDriver):
                                     '-t', 'sendtargets', '-p', volume['host'],
                                     run_as_root=True)
         for target in out.splitlines():
-            if FLAGS.iscsi_ip_prefix in target and volume_name in target:
+            if FLAGS.iscsi_ip_address in target and volume_name in target:
                 return target
         return None
 
@@ -462,6 +332,8 @@ class ISCSIDriver(VolumeDriver):
 
         :target_portal:    the portal of the iSCSI target
 
+        :volume_id:    the id of the volume (currently used by xen)
+
         :auth_method:, :auth_username:, :auth_password:
 
             the authentication details. Right now, either auth_method is not
@@ -491,6 +363,7 @@ class ISCSIDriver(VolumeDriver):
 
         iscsi_portal = iscsi_target.split(",")[0]
 
+        properties['volume_id'] = volume['id']
         properties['target_iqn'] = iscsi_name
         properties['target_portal'] = iscsi_portal
 
@@ -519,64 +392,32 @@ class ISCSIDriver(VolumeDriver):
                          '-v', property_value)
         return self._run_iscsiadm(iscsi_properties, iscsi_command)
 
-    def discover_volume(self, context, volume):
-        """Discover volume on a remote host."""
-        iscsi_properties = self._get_iscsi_properties(volume)
-
-        if not iscsi_properties['target_discovered']:
-            self._run_iscsiadm(iscsi_properties, ('--op', 'new'))
+    def initialize_connection(self, volume, address):
+        """Initializes the connection and returns connection info.
+
+        The iscsi driver returns a driver_volume_type of 'iscsi'.
+        The format of the driver data is defined in _get_iscsi_properties.
+        Example return value:
+            {
+                'driver_volume_type': 'iscsi'
+                'data': {
+                    'target_discovered': True,
+                    'target_iqn': 'iqn.2010-10.org.openstack:volume-00000001',
+                    'target_portal': '127.0.0.0.1:3260',
+                    'volume_id': 1,
+                }
+            }
 
-        if iscsi_properties.get('auth_method'):
-            self._iscsiadm_update(iscsi_properties,
-                                  "node.session.auth.authmethod",
-                                  iscsi_properties['auth_method'])
-            self._iscsiadm_update(iscsi_properties,
-                                  "node.session.auth.username",
-                                  iscsi_properties['auth_username'])
-            self._iscsiadm_update(iscsi_properties,
-                                  "node.session.auth.password",
-                                  iscsi_properties['auth_password'])
-
-        self._run_iscsiadm(iscsi_properties, ("--login", ))
-
-        self._iscsiadm_update(iscsi_properties, "node.startup", "automatic")
-
-        mount_device = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-0" %
-                        (iscsi_properties['target_portal'],
-                         iscsi_properties['target_iqn']))
-
-        # The /dev/disk/by-path/... node is not always present immediately
-        # TODO(justinsb): This retry-with-delay is a pattern, move to utils?
-        tries = 0
-        while not os.path.exists(mount_device):
-            if tries >= FLAGS.num_iscsi_scan_tries:
-                raise exception.Error(_("iSCSI device not found at %s") %
-                                      (mount_device))
-
-            LOG.warn(_("ISCSI volume not yet found at: %(mount_device)s. "
-                       "Will rescan & retry.  Try number: %(tries)s") %
-                     locals())
-
-            # The rescan isn't documented as being necessary(?), but it helps
-            self._run_iscsiadm(iscsi_properties, ("--rescan", ))
-
-            tries = tries + 1
-            if not os.path.exists(mount_device):
-                time.sleep(tries ** 2)
-
-        if tries != 0:
-            LOG.debug(_("Found iSCSI node %(mount_device)s "
-                        "(after %(tries)s rescans)") %
-                      locals())
-
-        return mount_device
+        """
 
-    def undiscover_volume(self, volume):
-        """Undiscover volume on a remote host."""
         iscsi_properties = self._get_iscsi_properties(volume)
-        self._iscsiadm_update(iscsi_properties, "node.startup", "manual")
-        self._run_iscsiadm(iscsi_properties, ("--logout", ))
-        self._run_iscsiadm(iscsi_properties, ('--op', 'delete'))
+        return {
+            'driver_volume_type': 'iscsi',
+            'data': iscsi_properties
+        }
+
+    def terminate_connection(self, volume, address):
+        pass
 
     def check_for_export(self, context, volume_id):
         """Make sure volume is exported."""
@@ -605,12 +446,13 @@ class FakeISCSIDriver(ISCSIDriver):
         """No setup necessary in fake mode."""
         pass
 
-    def discover_volume(self, context, volume):
-        """Discover volume on a remote host."""
-        return "/dev/disk/by-path/volume-id-%d" % volume['id']
+    def initialize_connection(self, volume, address):
+        return {
+            'driver_volume_type': 'iscsi',
+            'data': {}
+        }
 
-    def undiscover_volume(self, volume):
-        """Undiscover volume on a remote host."""
+    def terminate_connection(self, volume, address):
         pass
 
     @staticmethod
@@ -675,12 +517,15 @@ class RBDDriver(VolumeDriver):
         """Removes an export for a logical volume"""
         pass
 
-    def discover_volume(self, context, volume):
-        """Discover volume on a remote host"""
-        return "rbd:%s/%s" % (FLAGS.rbd_pool, volume['name'])
+    def initialize_connection(self, volume, address):
+        return {
+            'driver_volume_type': 'rbd',
+            'data': {
+                'name': '%s/%s' % (FLAGS.rbd_pool, volume['name'])
+            }
+        }
 
-    def undiscover_volume(self, volume):
-        """Undiscover volume on a remote host"""
+    def terminate_connection(self, volume, address):
         pass
 
 
@@ -738,12 +583,15 @@ class SheepdogDriver(VolumeDriver):
         """Removes an export for a logical volume"""
         pass
 
-    def discover_volume(self, context, volume):
-        """Discover volume on a remote host"""
-        return "sheepdog:%s" % volume['name']
+    def initialize_connection(self, volume, address):
+        return {
+            'driver_volume_type': 'sheepdog',
+            'data': {
+                'name': volume['name']
+            }
+        }
 
-    def undiscover_volume(self, volume):
-        """Undiscover volume on a remote host"""
+    def terminate_connection(self, volume, address):
         pass
 
 
@@ -772,11 +620,11 @@ class LoggingVolumeDriver(VolumeDriver):
     def remove_export(self, context, volume):
         self.log_action('remove_export', volume)
 
-    def discover_volume(self, context, volume):
-        self.log_action('discover_volume', volume)
+    def initialize_connection(self, volume, address):
+        self.log_action('initialize_connection', volume)
 
-    def undiscover_volume(self, volume):
-        self.log_action('undiscover_volume', volume)
+    def terminate_connection(self, volume, address):
+        self.log_action('terminate_connection', volume)
 
     def check_for_export(self, context, volume_id):
         self.log_action('check_for_export', volume_id)
@@ -906,6 +754,58 @@ class ZadaraBEDriver(ISCSIDriver):
 
         LOG.debug(_("VSA BE delete_volume for %s suceeded"), volume['name'])
 
+    def _discover_volume(self, context, volume):
+        """Discover volume on a remote host."""
+        iscsi_properties = self._get_iscsi_properties(volume)
+
+        if not iscsi_properties['target_discovered']:
+            self._run_iscsiadm(iscsi_properties, ('--op', 'new'))
+
+        if iscsi_properties.get('auth_method'):
+            self._iscsiadm_update(iscsi_properties,
+                                  "node.session.auth.authmethod",
+                                  iscsi_properties['auth_method'])
+            self._iscsiadm_update(iscsi_properties,
+                                  "node.session.auth.username",
+                                  iscsi_properties['auth_username'])
+            self._iscsiadm_update(iscsi_properties,
+                                  "node.session.auth.password",
+                                  iscsi_properties['auth_password'])
+
+        self._run_iscsiadm(iscsi_properties, ("--login", ))
+
+        self._iscsiadm_update(iscsi_properties, "node.startup", "automatic")
+
+        mount_device = ("/dev/disk/by-path/ip-%s-iscsi-%s-lun-0" %
+                        (iscsi_properties['target_portal'],
+                         iscsi_properties['target_iqn']))
+
+        # The /dev/disk/by-path/... node is not always present immediately
+        # TODO(justinsb): This retry-with-delay is a pattern, move to utils?
+        tries = 0
+        while not os.path.exists(mount_device):
+            if tries >= FLAGS.num_iscsi_scan_tries:
+                raise exception.Error(_("iSCSI device not found at %s") %
+                                      (mount_device))
+
+            LOG.warn(_("ISCSI volume not yet found at: %(mount_device)s. "
+                       "Will rescan & retry.  Try number: %(tries)s") %
+                     locals())
+
+            # The rescan isn't documented as being necessary(?), but it helps
+            self._run_iscsiadm(iscsi_properties, ("--rescan", ))
+
+            tries = tries + 1
+            if not os.path.exists(mount_device):
+                time.sleep(tries ** 2)
+
+        if tries != 0:
+            LOG.debug(_("Found iSCSI node %(mount_device)s "
+                        "(after %(tries)s rescans)") %
+                      locals())
+
+        return mount_device
+
     def local_path(self, volume):
         if self._not_vsa_volume_or_drive(volume):
             return super(ZadaraBEDriver, self).local_path(volume)
@@ -913,7 +813,10 @@ class ZadaraBEDriver(ISCSIDriver):
         if self._is_vsa_volume(volume):
             LOG.debug(_("\tFE VSA Volume %s local path call - call discover"),
                         volume['name'])
-            return super(ZadaraBEDriver, self).discover_volume(None, volume)
+            # NOTE(vish): Copied discover from iscsi_driver since it is used
+            #             but this should probably be refactored into a common
+            #             area because it is used in libvirt driver.
+            return self._discover_volume(None, volume)
 
         raise exception.Error(_("local_path not supported"))
 
diff --git a/nova/volume/manager.py b/nova/volume/manager.py
index caa5298d4..613924e7f 100644
--- a/nova/volume/manager.py
+++ b/nova/volume/manager.py
@@ -28,20 +28,17 @@ intact.
 :volume_topic:  What :mod:`rpc` topic to listen to (default: `volume`).
 :volume_manager:  The module name of a class derived from
                   :class:`manager.Manager` (default:
-                  :class:`nova.volume.manager.AOEManager`).
+                  :class:`nova.volume.manager.Manager`).
 :storage_availability_zone:  Defaults to `nova`.
-:volume_driver:  Used by :class:`AOEManager`.  Defaults to
-                 :class:`nova.volume.driver.AOEDriver`.
-:num_shelves:  Number of shelves for AoE (default: 100).
-:num_blades:  Number of vblades per shelf to allocate AoE storage from
-              (default: 16).
+:volume_driver:  Used by :class:`Manager`.  Defaults to
+                 :class:`nova.volume.driver.ISCSIDriver`.
 :volume_group:  Name of the group that will contain exported volumes (default:
                 `nova-volumes`)
-:aoe_eth_dev:  Device name the volumes will be exported on (default: `eth0`).
-:num_shell_tries:  Number of times to attempt to run AoE commands (default: 3)
+:num_shell_tries:  Number of times to attempt to run commands (default: 3)
 
 """
 
+import sys
 
 from nova import context
 from nova import exception
@@ -126,10 +123,11 @@ class VolumeManager(manager.SchedulerDependentManager):
             if model_update:
                 self.db.volume_update(context, volume_ref['id'], model_update)
         except Exception:
+            exc_info = sys.exc_info()
             self.db.volume_update(context,
                                   volume_ref['id'], {'status': 'error'})
             self._notify_vsa(context, volume_ref, 'error')
-            raise
+            raise exc_info
 
         now = utils.utcnow()
         self.db.volume_update(context,
@@ -181,10 +179,11 @@ class VolumeManager(manager.SchedulerDependentManager):
                                   {'status': 'available'})
             return True
         except Exception:
+            exc_info = sys.exc_info()
             self.db.volume_update(context,
                                   volume_ref['id'],
                                   {'status': 'error_deleting'})
-            raise
+            raise exc_info
 
         self.db.volume_destroy(context, volume_id)
         LOG.debug(_("volume %s: deleted successfully"), volume_ref['name'])
@@ -233,26 +232,44 @@ class VolumeManager(manager.SchedulerDependentManager):
         LOG.debug(_("snapshot %s: deleted successfully"), snapshot_ref['name'])
         return True
 
-    def setup_compute_volume(self, context, volume_id):
-        """Setup remote volume on compute host.
-
-        Returns path to device."""
-        context = context.elevated()
+    def attach_volume(self, context, volume_id, instance_id, mountpoint):
+        """Updates db to show volume is attached"""
+        # TODO(vish): refactor this into a more general "reserve"
+        self.db.volume_attached(context,
+                                volume_id,
+                                instance_id,
+                                mountpoint)
+
+    def detach_volume(self, context, volume_id):
+        """Updates db to show volume is detached"""
+        # TODO(vish): refactor this into a more general "unreserve"
+        self.db.volume_detached(context, volume_id)
+
+    def initialize_connection(self, context, volume_id, address):
+        """Initialize volume to be connected from address.
+
+        This method calls the driver initialize_connection and returns
+        it to the caller.  The driver is responsible for doing any
+        necessary security setup and returning a connection_info dictionary
+        in the following format:
+            {'driver_volume_type': driver_volume_type
+             'data': data}
+
+        driver_volume_type: a string to identify the type of volume.  This
+                           can be used by the calling code to determine the
+                           strategy for connecting to the volume. This could
+                           be 'iscsi', 'rdb', 'sheepdog', etc.
+        data: this is the data that the calling code will use to connect
+              to the volume. Keep in mind that this will be serialized to
+              json in various places, so it should not contain any non-json
+              data types.
+        """
         volume_ref = self.db.volume_get(context, volume_id)
-        if volume_ref['host'] == self.host and FLAGS.use_local_volumes:
-            path = self.driver.local_path(volume_ref)
-        else:
-            path = self.driver.discover_volume(context, volume_ref)
-        return path
-
-    def remove_compute_volume(self, context, volume_id):
-        """Remove remote volume on compute host."""
-        context = context.elevated()
+        return self.driver.initialize_connection(volume_ref, address)
+
+    def terminate_connection(self, context, volume_id, address):
         volume_ref = self.db.volume_get(context, volume_id)
-        if volume_ref['host'] == self.host and FLAGS.use_local_volumes:
-            return True
-        else:
-            self.driver.undiscover_volume(volume_ref)
+        self.driver.terminate_connection(volume_ref, address)
 
     def check_for_export(self, context, instance_id):
         """Make sure whether volume is exported."""
diff --git a/nova/volume/san.py b/nova/volume/san.py
index 9532c8116..490605976 100644
--- a/nova/volume/san.py
+++ b/nova/volume/san.py
@@ -61,9 +61,6 @@ class SanISCSIDriver(ISCSIDriver):
     def _build_iscsi_target_name(self, volume):
         return "%s%s" % (FLAGS.iscsi_target_prefix, volume['name'])
 
-    # discover_volume is still OK
-    # undiscover_volume is still OK
-
     def _connect_to_ssh(self):
         ssh = paramiko.SSHClient()
         #TODO(justinsb): We need a better SSH key policy