From 6187529119ab51a6df7e30ef5190757ee0feca5e Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 3 Aug 2010 15:04:38 -0700 Subject: vblade commands randomly toss stuff into stderr, ignore it --- nova/volume/service.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'nova/volume') diff --git a/nova/volume/service.py b/nova/volume/service.py index e12f675a7..9dd63e88f 100644 --- a/nova/volume/service.py +++ b/nova/volume/service.py @@ -227,11 +227,7 @@ class Volume(datastore.BasicModel): @defer.inlineCallbacks def destroy(self): - try: - yield self._remove_export() - except Exception as ex: - logging.debug("Ingnoring failure to remove export %s" % ex) - pass + yield self._remove_export() yield self._delete_lv() super(Volume, self).destroy() @@ -250,7 +246,7 @@ class Volume(datastore.BasicModel): def _delete_lv(self): yield process.simple_execute( "sudo lvremove -f %s/%s" % (FLAGS.volume_group, - self['volume_id'])) + self['volume_id']), error_ok=1) @defer.inlineCallbacks def _setup_export(self): @@ -275,10 +271,10 @@ class Volume(datastore.BasicModel): def _remove_export(self): yield process.simple_execute( "sudo vblade-persist stop %s %s" % (self['shelf_id'], - self['blade_id'])) + self['blade_id']), error_ok=1) yield process.simple_execute( "sudo vblade-persist destroy %s %s" % (self['shelf_id'], - self['blade_id'])) + self['blade_id']), error_ok=1) class FakeVolume(Volume): -- cgit From d79fd0df0bf9c59483b30c0d8c3a811580a1ee39 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 5 Aug 2010 04:31:21 -0700 Subject: Changed volumes to use a pool instead of globbing filesystem for concurrency reasons. Fixed broken tests. --- nova/volume/service.py | 102 ++++++++++++++++++++++++++----------------------- 1 file changed, 55 insertions(+), 47 deletions(-) (limited to 'nova/volume') diff --git a/nova/volume/service.py b/nova/volume/service.py index 9dd63e88f..9c52ee469 100644 --- a/nova/volume/service.py +++ b/nova/volume/service.py @@ -22,12 +22,8 @@ destroying persistent storage volumes, ala EBS. Currently uses Ata-over-Ethernet. """ -import glob import logging import os -import shutil -import socket -import tempfile from twisted.internet import defer @@ -47,9 +43,6 @@ 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('storage_name', - socket.gethostname(), - 'name of this service') flags.DEFINE_integer('first_shelf_id', utils.last_octet(utils.get_my_ip()) * 10, 'AoE starting shelf_id for this service') @@ -59,9 +52,9 @@ flags.DEFINE_integer('last_shelf_id', flags.DEFINE_string('aoe_export_dir', '/var/lib/vblade-persist/vblades', 'AoE directory where exports are created') -flags.DEFINE_integer('slots_per_shelf', +flags.DEFINE_integer('blades_per_shelf', 16, - 'Number of AoE slots per shelf') + 'Number of AoE blades per shelf') flags.DEFINE_string('storage_availability_zone', 'nova', 'availability zone of this service') @@ -69,7 +62,7 @@ flags.DEFINE_boolean('fake_storage', False, 'Should we make real storage volumes to attach?') -class NoMoreVolumes(exception.Error): +class NoMoreBlades(exception.Error): pass def get_volume(volume_id): @@ -77,8 +70,9 @@ def get_volume(volume_id): volume_class = Volume if FLAGS.fake_storage: volume_class = FakeVolume - if datastore.Redis.instance().sismember('volumes', volume_id): - return volume_class(volume_id=volume_id) + vol = volume_class.lookup(volume_id) + if vol: + return vol raise exception.Error("Volume does not exist") class VolumeService(service.Service): @@ -91,18 +85,9 @@ class VolumeService(service.Service): super(VolumeService, self).__init__() self.volume_class = Volume if FLAGS.fake_storage: - FLAGS.aoe_export_dir = tempfile.mkdtemp() self.volume_class = FakeVolume self._init_volume_group() - def __del__(self): - # TODO(josh): Get rid of this destructor, volumes destroy themselves - if FLAGS.fake_storage: - try: - shutil.rmtree(FLAGS.aoe_export_dir) - except Exception, err: - pass - @defer.inlineCallbacks @validate.rangetest(size=(0, 1000)) def create_volume(self, size, user_id, project_id): @@ -113,8 +98,6 @@ class VolumeService(service.Service): """ logging.debug("Creating volume of size: %s" % (size)) vol = yield self.volume_class.create(size, user_id, project_id) - datastore.Redis.instance().sadd('volumes', vol['volume_id']) - datastore.Redis.instance().sadd('volumes:%s' % (FLAGS.storage_name), vol['volume_id']) logging.debug("restarting exports") yield self._restart_exports() defer.returnValue(vol['volume_id']) @@ -134,13 +117,11 @@ class VolumeService(service.Service): def delete_volume(self, volume_id): logging.debug("Deleting volume with id of: %s" % (volume_id)) vol = get_volume(volume_id) - if vol['status'] == "attached": + if vol['attach_status'] == "attached": raise exception.Error("Volume is still attached") - if vol['node_name'] != FLAGS.storage_name: + if vol['node_name'] != FLAGS.node_name: raise exception.Error("Volume is not local to this node") yield vol.destroy() - datastore.Redis.instance().srem('volumes', vol['volume_id']) - datastore.Redis.instance().srem('volumes:%s' % (FLAGS.storage_name), vol['volume_id']) defer.returnValue(True) @defer.inlineCallbacks @@ -172,14 +153,15 @@ class Volume(datastore.BasicModel): return self.volume_id def default_state(self): - return {"volume_id": self.volume_id} + return {"volume_id": self.volume_id, + "node_name": "unassigned"} @classmethod @defer.inlineCallbacks def create(cls, size, user_id, project_id): volume_id = utils.generate_uid('vol') vol = cls(volume_id) - vol['node_name'] = FLAGS.storage_name + vol['node_name'] = FLAGS.node_name vol['size'] = size vol['user_id'] = user_id vol['project_id'] = project_id @@ -225,10 +207,31 @@ class Volume(datastore.BasicModel): self['attach_status'] = "detached" self.save() + def save(self): + is_new = self.is_new_record() + super(Volume, self).save() + if is_new: + redis = datastore.Redis.instance() + key = self.__devices_key + # TODO(vish): these should be added by admin commands + more = redis.scard(self._redis_association_name("node", + self['node_name'])) + if (not redis.exists(key) and not more): + for shelf_id in range(FLAGS.first_shelf_id, + FLAGS.last_shelf_id + 1): + for blade_id in range(FLAGS.blades_per_shelf): + redis.sadd(key, "%s.%s" % (shelf_id, blade_id)) + self.associate_with("node", self['node_name']) + @defer.inlineCallbacks def destroy(self): yield self._remove_export() yield self._delete_lv() + self.unassociate_with("node", self['node_name']) + if self.get('shelf_id', None) and self.get('blade_id', None): + redis = datastore.Redis.instance() + key = self.__devices_key + redis.sadd(key, "%s.%s" % (self['shelf_id'], self['blade_id'])) super(Volume, self).destroy() @defer.inlineCallbacks @@ -248,17 +251,26 @@ class Volume(datastore.BasicModel): "sudo lvremove -f %s/%s" % (FLAGS.volume_group, self['volume_id']), error_ok=1) + @property + def __devices_key(self): + return 'volume_devices:%s' % FLAGS.node_name + @defer.inlineCallbacks def _setup_export(self): - (shelf_id, blade_id) = get_next_aoe_numbers() + redis = datastore.Redis.instance() + key = self.__devices_key + device = redis.spop(key) + if not device: + raise NoMoreBlades() + (shelf_id, blade_id) = device.split('.') self['aoe_device'] = "e%s.%s" % (shelf_id, blade_id) self['shelf_id'] = shelf_id self['blade_id'] = blade_id self.save() - yield self._exec_export() + yield self._exec_setup_export() @defer.inlineCallbacks - def _exec_export(self): + def _exec_setup_export(self): yield process.simple_execute( "sudo vblade-persist setup %s %s %s /dev/%s/%s" % (self['shelf_id'], @@ -269,6 +281,13 @@ class Volume(datastore.BasicModel): @defer.inlineCallbacks def _remove_export(self): + if not self.get('shelf_id', None) or not self.get('blade_id', None): + defer.returnValue(False) + yield self._exec_remove_export() + defer.returnValue(True) + + @defer.inlineCallbacks + def _exec_remove_export(self): yield process.simple_execute( "sudo vblade-persist stop %s %s" % (self['shelf_id'], self['blade_id']), error_ok=1) @@ -277,29 +296,18 @@ class Volume(datastore.BasicModel): self['blade_id']), error_ok=1) + class FakeVolume(Volume): def _create_lv(self): pass - def _exec_export(self): + def _exec_setup_export(self): fname = os.path.join(FLAGS.aoe_export_dir, self['aoe_device']) f = file(fname, "w") f.close() - def _remove_export(self): - pass + def _exec_remove_export(self): + os.unlink(os.path.join(FLAGS.aoe_export_dir, self['aoe_device'])) def _delete_lv(self): pass - -def get_next_aoe_numbers(): - for shelf_id in xrange(FLAGS.first_shelf_id, FLAGS.last_shelf_id + 1): - aoes = glob.glob("%s/e%s.*" % (FLAGS.aoe_export_dir, shelf_id)) - if not aoes: - blade_id = 0 - else: - blade_id = int(max([int(a.rpartition('.')[2]) for a in aoes])) + 1 - if blade_id < FLAGS.slots_per_shelf: - logging.debug("Next shelf.blade is %s.%s", shelf_id, blade_id) - return (shelf_id, blade_id) - raise NoMoreVolumes() -- cgit From a33dce2da8dc8e25d0943732adfa6b14b1e48c7b Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Fri, 6 Aug 2010 15:48:46 -0700 Subject: a few more commands were putting output on stderr. In general, exceptions on stderr output seems like a bad idea --- nova/volume/service.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'nova/volume') diff --git a/nova/volume/service.py b/nova/volume/service.py index 9c52ee469..66163a812 100644 --- a/nova/volume/service.py +++ b/nova/volume/service.py @@ -128,8 +128,8 @@ class VolumeService(service.Service): def _restart_exports(self): if FLAGS.fake_storage: return - yield process.simple_execute("sudo vblade-persist auto all") - # NOTE(vish): this command sometimes sends output to stderr for warnings + # NOTE(vish): these commands sometimes sends output to stderr for warnings + yield process.simple_execute("sudo vblade-persist auto all", error_ok=1) yield process.simple_execute("sudo vblade-persist start all", error_ok=1) @defer.inlineCallbacks @@ -243,7 +243,8 @@ class Volume(datastore.BasicModel): yield process.simple_execute( "sudo lvcreate -L %s -n %s %s" % (sizestr, self['volume_id'], - FLAGS.volume_group)) + FLAGS.volume_group), + error_ok=1) @defer.inlineCallbacks def _delete_lv(self): @@ -277,7 +278,7 @@ class Volume(datastore.BasicModel): self['blade_id'], FLAGS.aoe_eth_dev, FLAGS.volume_group, - self['volume_id'])) + self['volume_id']), error_ok=1) @defer.inlineCallbacks def _remove_export(self): -- cgit From d508418214016d5c00aa8d304f9498f5b99a960b Mon Sep 17 00:00:00 2001 From: andy Date: Mon, 16 Aug 2010 14:16:21 +0200 Subject: rather comprehensive style fixes --- nova/volume/service.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'nova/volume') diff --git a/nova/volume/service.py b/nova/volume/service.py index 66163a812..104bafe90 100644 --- a/nova/volume/service.py +++ b/nova/volume/service.py @@ -65,6 +65,7 @@ flags.DEFINE_boolean('fake_storage', False, class NoMoreBlades(exception.Error): pass + def get_volume(volume_id): """ Returns a redis-backed volume object """ volume_class = Volume @@ -75,6 +76,7 @@ def get_volume(volume_id): return vol raise exception.Error("Volume does not exist") + class VolumeService(service.Service): """ There is one VolumeNode running on each host. @@ -142,6 +144,7 @@ class VolumeService(service.Service): "sudo vgcreate %s %s" % (FLAGS.volume_group, FLAGS.storage_dev)) + class Volume(datastore.BasicModel): def __init__(self, volume_id=None): @@ -297,7 +300,6 @@ class Volume(datastore.BasicModel): self['blade_id']), error_ok=1) - class FakeVolume(Volume): def _create_lv(self): pass -- cgit