daemons/lvmdbusd/cmdhandler.py | 12 ++- daemons/lvmdbusd/lv.py | 48 ++++++++-- daemons/lvmdbusd/lvmdb.py | 7 ++ daemons/lvmdbusd/manager.py | 12 ++- daemons/lvmdbusd/objectmanager.py | 193 ++++++++++++++++++++++++++------------ test/dbus/lvmdbustest.py | 13 ++- 6 files changed, 203 insertions(+), 82 deletions(-) diff --git a/daemons/lvmdbusd/cmdhandler.py b/daemons/lvmdbusd/cmdhandler.py index c8c7baa..4bdbee5 100644 --- a/daemons/lvmdbusd/cmdhandler.py +++ b/daemons/lvmdbusd/cmdhandler.py @@ -469,7 +469,7 @@ def lvm_full_report_json(): 'pv_used', 'dev_size', 'pv_mda_size', 'pv_mda_free', 'pv_ba_start', 'pv_ba_size', 'pe_start', 'pv_pe_count', 'pv_pe_alloc_count', 'pv_attr', 'pv_tags', 'vg_name', - 'vg_uuid'] + 'vg_uuid', 'pv_missing'] pv_seg_columns = ['pvseg_start', 'pvseg_size', 'segtype', 'pv_uuid', 'lv_uuid', 'pv_name'] @@ -485,7 +485,9 @@ def lvm_full_report_json(): 'vg_name', 'pool_lv_uuid', 'pool_lv', 'origin_uuid', 'origin', 'data_percent', 'lv_attr', 'lv_tags', 'vg_uuid', 'lv_active', 'data_lv', - 'metadata_lv', 'lv_parent', 'lv_role', 'lv_layout'] + 'metadata_lv', 'lv_parent', 'lv_role', 'lv_layout', + 'snap_percent', 'metadata_percent', 'copy_percent', + 'sync_percent', 'lv_metadata_size', 'move_pv', 'move_pv_uuid'] lv_seg_columns = ['seg_pe_ranges', 'segtype', 'lv_uuid'] @@ -522,7 +524,7 @@ def pv_retrieve_with_segs(device=None): 'pv_used', 'dev_size', 'pv_mda_size', 'pv_mda_free', 'pv_ba_start', 'pv_ba_size', 'pe_start', 'pv_pe_count', 'pv_pe_alloc_count', 'pv_attr', 'pv_tags', 'vg_name', - 'vg_uuid', 'pvseg_start', 'pvseg_size', 'segtype'] + 'vg_uuid', 'pvseg_start', 'pvseg_size', 'segtype', 'pv_missing'] # Lvm has some issues where it returns failure when querying pvs when other # operations are in process, see: @@ -735,7 +737,9 @@ def lv_retrieve_with_segments(): 'origin', 'data_percent', 'lv_attr', 'lv_tags', 'vg_uuid', 'lv_active', 'data_lv', 'metadata_lv', 'seg_pe_ranges', 'segtype', 'lv_parent', - 'lv_role', 'lv_layout'] + 'lv_role', 'lv_layout', + 'snap_percent', 'metadata_percent', 'copy_percent', + 'sync_percent', 'lv_metadata_size', 'move_pv', 'move_pv_uuid'] cmd = _dc('lvs', ['-a', '-o', ','.join(columns)]) rc, out, err = call(cmd) diff --git a/daemons/lvmdbusd/lv.py b/daemons/lvmdbusd/lv.py index 455f371..c356799 100644 --- a/daemons/lvmdbusd/lv.py +++ b/daemons/lvmdbusd/lv.py @@ -81,7 +81,14 @@ def lvs_state_retrieve(selection, cache_refresh=True): n32(l['data_percent']), l['lv_attr'], l['lv_tags'], l['lv_active'], l['data_lv'], l['metadata_lv'], l['segtype'], l['lv_role'], - l['lv_layout'])) + l['lv_layout'], + n32(l['snap_percent']), + n32(l['metadata_percent']), + n32(l['copy_percent']), + n32(l['sync_percent']), + n(l['lv_metadata_size']), + l['move_pv'], + l['move_pv_uuid'])) return rc @@ -101,8 +108,7 @@ class LvState(State): rc = [] for pv in sorted(cfg.db.lv_contained_pv(uuid)): (pv_uuid, pv_name, pv_segs) = pv - pv_obj = cfg.om.get_object_path_by_uuid_lvm_id( - pv_uuid, pv_name, gen_new=False) + pv_obj = cfg.om.get_object_path_by_uuid_lvm_id(pv_uuid, pv_name) segs_decorate = [] for i in pv_segs: @@ -131,8 +137,7 @@ class LvState(State): for l in cfg.db.hidden_lvs(self.Uuid): full_name = "%s/%s" % (vg_name, l[1]) - op = cfg.om.get_object_path_by_uuid_lvm_id( - l[0], full_name, gen_new=False) + op = cfg.om.get_object_path_by_uuid_lvm_id(l[0], full_name) assert op rc.append(dbus.ObjectPath(op)) return rc @@ -140,7 +145,9 @@ class LvState(State): def __init__(self, Uuid, Name, Path, SizeBytes, vg_name, vg_uuid, pool_lv_uuid, PoolLv, origin_uuid, OriginLv, DataPercent, Attr, Tags, active, - data_lv, metadata_lv, segtypes, role, layout): + data_lv, metadata_lv, segtypes, role, layout, SnapPercent, + MetaDataPercent, CopyPercent, SyncPercent, MetaDataSizeBytes, + move_pv, move_pv_uuid): utils.init_class_from_arguments(self) # The segtypes is possibly an array with potentially dupes or a single @@ -160,8 +167,7 @@ class LvState(State): gen = utils.lv_object_path_method(Name, (Attr, layout, role)) self.PoolLv = cfg.om.get_object_path_by_uuid_lvm_id( - pool_lv_uuid, '%s/%s' % (vg_name, PoolLv), - gen) + pool_lv_uuid, '%s/%s' % (vg_name, PoolLv), gen) else: self.PoolLv = '/' @@ -217,13 +223,20 @@ class LvState(State): @utils.dbus_property(LV_COMMON_INTERFACE, 'Name', 's') @utils.dbus_property(LV_COMMON_INTERFACE, 'Path', 's') @utils.dbus_property(LV_COMMON_INTERFACE, 'SizeBytes', 't') -@utils.dbus_property(LV_COMMON_INTERFACE, 'DataPercent', 'u') @utils.dbus_property(LV_COMMON_INTERFACE, 'SegType', 'as') @utils.dbus_property(LV_COMMON_INTERFACE, 'Vg', 'o') @utils.dbus_property(LV_COMMON_INTERFACE, 'OriginLv', 'o') @utils.dbus_property(LV_COMMON_INTERFACE, 'PoolLv', 'o') @utils.dbus_property(LV_COMMON_INTERFACE, 'Devices', "a(oa(tts))") @utils.dbus_property(LV_COMMON_INTERFACE, 'HiddenLvs', "ao") +@utils.dbus_property(LV_COMMON_INTERFACE, 'Attr', 's') +@utils.dbus_property(LV_COMMON_INTERFACE, 'DataPercent', 'u') +@utils.dbus_property(LV_COMMON_INTERFACE, 'SnapPercent', 'u') +@utils.dbus_property(LV_COMMON_INTERFACE, 'DataPercent', 'u') +@utils.dbus_property(LV_COMMON_INTERFACE, 'MetaDataPercent', 'u') +@utils.dbus_property(LV_COMMON_INTERFACE, 'CopyPercent', 'u') +@utils.dbus_property(LV_COMMON_INTERFACE, 'SyncPercent', 'u') +@utils.dbus_property(LV_COMMON_INTERFACE, 'MetaDataSizeBytes', 't') class LvCommon(AutomatedProperties): _Tags_meta = ("as", LV_COMMON_INTERFACE) _Roles_meta = ("as", LV_COMMON_INTERFACE) @@ -239,12 +252,25 @@ class LvCommon(AutomatedProperties): _FixedMinor_meta = ('b', LV_COMMON_INTERFACE) _ZeroBlocks_meta = ('b', LV_COMMON_INTERFACE) _SkipActivation_meta = ('b', LV_COMMON_INTERFACE) + _MovePv_meta = ('o', LV_COMMON_INTERFACE) + + def _get_move_pv(self): + path = None + + # It's likely that the move_pv is empty + if self.state.move_pv_uuid and self.state.move_pv: + path = cfg.om.get_object_path_by_uuid_lvm_id( + self.state.move_pv_uuid, self.state.move_pv) + if not path: + path = '/' + return path # noinspection PyUnusedLocal,PyPep8Naming def __init__(self, object_path, object_state): super(LvCommon, self).__init__(object_path, lvs_state_retrieve) self.set_interface(LV_COMMON_INTERFACE) self.state = object_state + self._move_pv = self._get_move_pv() @property def VolumeType(self): @@ -354,6 +380,10 @@ class LvCommon(AutomatedProperties): def Active(self): return dbus.Boolean(self.state.active == "active") + @property + def MovePv(self): + return dbus.ObjectPath(self._move_pv) + # noinspection PyPep8Naming class Lv(LvCommon): diff --git a/daemons/lvmdbusd/lvmdb.py b/daemons/lvmdbusd/lvmdb.py index ac60a80..9529e03 100755 --- a/daemons/lvmdbusd/lvmdb.py +++ b/daemons/lvmdbusd/lvmdb.py @@ -433,6 +433,12 @@ class DataStore(object): rc.append(self.pvs[self.pv_path_to_uuid[s]]) return rc + def pv_missing(self, pv_uuid): + if pv_uuid in self.pvs: + if self.pvs[pv_uuid]['pv_missing'] == '': + return False + return True + def fetch_vgs(self, vg_name): if not vg_name: return self.vgs.values() @@ -516,6 +522,7 @@ if __name__ == "__main__": print("PVS") for v in ds.pvs.values(): pp.pprint(v) + print('PV missing is %s' % ds.pv_missing(v['pv_uuid'])) print("VGS") for v in ds.vgs.values(): diff --git a/daemons/lvmdbusd/manager.py b/daemons/lvmdbusd/manager.py index b7d7b10..e377442 100644 --- a/daemons/lvmdbusd/manager.py +++ b/daemons/lvmdbusd/manager.py @@ -37,8 +37,7 @@ class Manager(AutomatedProperties): # Check to see if we are already trying to create a PV for an existing # PV - pv = cfg.om.get_object_path_by_uuid_lvm_id( - device, device, None, False) + pv = cfg.om.get_object_path_by_uuid_lvm_id(device, device) if pv: raise dbus.exceptions.DBusException( MANAGER_INTERFACE, "PV Already exists!") @@ -115,6 +114,10 @@ class Manager(AutomatedProperties): # This is a diagnostic and should not be run in normal operation, so # lets remove the log entries for refresh as it's implied. + + # Run an internal diagnostic on the object manager look up tables + lc = cfg.om.validate_lookups() + rc = cfg.load(log=False) if rc != 0: @@ -122,7 +125,7 @@ class Manager(AutomatedProperties): 'bg_black', 'fg_light_red') else: utils.log_debug('Manager.Refresh - exit %d' % (rc)) - return rc + return rc + lc @dbus.service.method( dbus_interface=MANAGER_INTERFACE, @@ -160,8 +163,7 @@ class Manager(AutomatedProperties): :param key: The lookup value :return: Return the object path. If object not found you will get '/' """ - p = cfg.om.get_object_path_by_uuid_lvm_id( - key, key, gen_new=False) + p = cfg.om.get_object_path_by_uuid_lvm_id(key, key) if p: return p return '/' diff --git a/daemons/lvmdbusd/objectmanager.py b/daemons/lvmdbusd/objectmanager.py index 0b810d8..71149a5 100644 --- a/daemons/lvmdbusd/objectmanager.py +++ b/daemons/lvmdbusd/objectmanager.py @@ -12,8 +12,9 @@ import threading import traceback import dbus import os +import copy from . import cfg -from .utils import log_debug +from .utils import log_debug, pv_obj_path_generate, log_error from .automatedproperties import AutomatedProperties @@ -70,12 +71,37 @@ class ObjectManager(AutomatedProperties): log_debug(('SIGNAL: InterfacesRemoved(%s, %s)' % (str(object_path), str(interface_list)))) + def validate_lookups(self): + with self.rlock: + tmp_lookups = copy.deepcopy(self._id_to_object_path) + + # iterate over all we know, removing from the copy. If all is well + # we will have zero items left over + for path, md in self._objects.items(): + obj, lvm_id, uuid = md + + if lvm_id: + assert path == tmp_lookups[lvm_id] + del tmp_lookups[lvm_id] + + if uuid: + assert path == tmp_lookups[uuid] + del tmp_lookups[uuid] + + rc = len(tmp_lookups) + if rc: + # Error condition + log_error("_id_to_object_path has extraneous lookups!") + for key, path in tmp_lookups.items(): + log_error("Key= %s, path= %s" % (key, path)) + return rc + def _lookup_add(self, obj, path, lvm_id, uuid): """ Store information about what we added to the caches so that we can remove it cleanly :param obj: The dbus object we are storing - :param lvm_id: The user name for the asset + :param lvm_id: The lvm id for the asset :param uuid: The uuid for the asset :return: """ @@ -85,7 +111,12 @@ class ObjectManager(AutomatedProperties): self._lookup_remove(path) self._objects[path] = (obj, lvm_id, uuid) - self._id_to_object_path[lvm_id] = path + + # Make sure we have one or the other + assert lvm_id or uuid + + if lvm_id: + self._id_to_object_path[lvm_id] = path if uuid: self._id_to_object_path[uuid] = path @@ -94,8 +125,13 @@ class ObjectManager(AutomatedProperties): # Note: Only called internally, lock implied if obj_path in self._objects: (obj, lvm_id, uuid) = self._objects[obj_path] - del self._id_to_object_path[lvm_id] - del self._id_to_object_path[uuid] + + if lvm_id in self._id_to_object_path: + del self._id_to_object_path[lvm_id] + + if uuid in self._id_to_object_path: + del self._id_to_object_path[uuid] + del self._objects[obj_path] def lookup_update(self, dbus_obj, new_uuid, new_lvm_id): @@ -173,7 +209,7 @@ class ObjectManager(AutomatedProperties): def get_object_by_uuid_lvm_id(self, uuid, lvm_id): with self.rlock: return self.get_object_by_path( - self.get_object_path_by_uuid_lvm_id(uuid, lvm_id, None, False)) + self.get_object_path_by_uuid_lvm_id(uuid, lvm_id)) def get_object_by_lvm_id(self, lvm_id): """ @@ -206,78 +242,111 @@ class ObjectManager(AutomatedProperties): :return: None """ # This gets called when we found an object based on lvm_id, ensure - # uuid is correct too, as they can change + # uuid is correct too, as they can change. There is no durable + # non-changeable name in lvm if lvm_id != uuid: - if uuid not in self._id_to_object_path: + if uuid and uuid not in self._id_to_object_path: obj = self.get_object_by_path(path) self._lookup_add(obj, path, lvm_id, uuid) - def _return_lookup(self, uuid, lvm_identifier): + def _lvm_id_verify(self, path, uuid, lvm_id): """ - We found an identifier, so lets return the path to the found object - :param uuid: The lvm uuid - :param lvm_identifier: The lvm_id used to find object - :return: + Ensure lvm_id is present for a successful uuid lookup + NOTE: Internal call, assumes under object manager lock + :param path: Path to object we looked up + :param uuid: uuid used to find object + :param lvm_id: lvm_id to verify + :return: None """ - path = self._id_to_object_path[lvm_identifier] - self._uuid_verify(path, uuid, lvm_identifier) + # This gets called when we found an object based on uuid, ensure + # lvm_id is correct too, as they can change. There is no durable + # non-changeable name in lvm + if lvm_id != uuid: + if lvm_id and lvm_id not in self._id_to_object_path: + obj = self.get_object_by_path(path) + self._lookup_add(obj, path, lvm_id, uuid) + + def _id_lookup(self, the_id): + path = None + + if the_id: + # The _id_to_object_path contains hash keys for everything, so + # uuid and lvm_id + if the_id in self._id_to_object_path: + path = self._id_to_object_path[the_id] + else: + if "/" in the_id: + if the_id.startswith('/'): + # We could have a pv device path lookup that failed, + # lets try canonical form and try again. + canonical = os.path.realpath(the_id) + if canonical in self._id_to_object_path: + path = self._id_to_object_path[canonical] + else: + vg, lv = the_id.split("/", 1) + int_lvm_id = vg + "/" + ("[%s]" % lv) + if int_lvm_id in self._id_to_object_path: + path = self._id_to_object_path[int_lvm_id] return path - def get_object_path_by_uuid_lvm_id(self, uuid, lvm_id, path_create=None, - gen_new=True): + def get_object_path_by_uuid_lvm_id(self, uuid, lvm_id, path_create=None): """ - For a given lvm asset return the dbus object registered to it. If the - object is not found and gen_new == True and path_create is a valid - function we will create a new path, register it and return it. - :param uuid: The uuid for the lvm object - :param lvm_id: The lvm name - :param path_create: If true create an object path if not found - :param gen_new: The function used to create the new path + For a given lvm asset return the dbus object path registered for it. + This method first looks up by uuid and then by lvm_id. You + can search by just one by setting uuid == lvm_id (uuid or lvm_id). + If the object is not found and path_create is a not None, the + path_create function will be called to create a new object path and + register it with the object manager for the specified uuid & lvm_id. + Note: If path create is not None, uuid and lvm_id cannot be equal + :param uuid: The uuid for the lvm object we are searching for + :param lvm_id: The lvm name (eg. pv device path, vg name, lv full name) + :param path_create: If not None, create the path using this function if + we fail to find the object by uuid or lvm_id. + :returns None if lvm asset not found and path_create == None otherwise + a valid dbus object path """ with self.rlock: assert lvm_id assert uuid - if gen_new: - assert path_create + if path_create: + assert uuid != lvm_id - path = None - - if lvm_id in self._id_to_object_path: - self._return_lookup(uuid, lvm_id) - - if "/" in lvm_id: - vg, lv = lvm_id.split("/", 1) - int_lvm_id = vg + "/" + ("[%s]" % lv) - if int_lvm_id in self._id_to_object_path: - self._return_lookup(uuid, int_lvm_id) - elif lvm_id.startswith('/'): - # We could have a pv device path lookup that failed, - # lets try canonical form and try again. - canonical = os.path.realpath(lvm_id) - if canonical in self._id_to_object_path: - self._return_lookup(uuid, canonical) - - if uuid and uuid in self._id_to_object_path: - # If we get here it indicates that we found the object, but - # the lvm_id lookup failed. In the case of a rename, the uuid - # will be correct, but the lvm_id will be wrong and vise versa. - # If the lvm_id does not equal the uuid, lets fix up the table - # so that lookups will be handled correctly. - path = self._id_to_object_path[uuid] - - # In some cases we are looking up by one or the other, don't - # update when they are the same. - if uuid != lvm_id: - obj = self.get_object_by_path(path) - self._lookup_add(obj, path, lvm_id, uuid) + # Check for Manager.LookUpByLvmId query, we cannot + # check/verify/update the uuid and lvm_id lookups so don't! + if uuid == lvm_id: + path = self._id_lookup(lvm_id) else: - if gen_new: - path = path_create() - self._lookup_add(None, path, lvm_id, uuid) - - # pprint('get_object_path_by_lvm_id(%s, %s, %s, %s: return %s' % - # (uuid, lvm_id, str(path_create), str(gen_new), path)) + # We have a uuid and a lvm_id we can do sanity checks to ensure + # that they are consistent + + # If a PV is missing it's device path is '[unknown]' or some + # other text derivation of unknown. When we find that a PV is + # missing we will clear out the lvm_id as it's likely not unique + # and thus not useful and potentially harmful for lookups. + if path_create == pv_obj_path_generate and \ + cfg.db.pv_missing(uuid): + lvm_id = None + + # Lets check for the uuid first + path = self._id_lookup(uuid) + if path: + # Verify the lvm_id is sane + self._lvm_id_verify(path, uuid, lvm_id) + else: + # Unable to find by UUID, lets lookup by lvm_id + path = self._id_lookup(lvm_id) + if path: + # Verify the uuid is sane + self._uuid_verify(path, uuid, lvm_id) + else: + # We have exhausted all lookups, let's create if we can + if path_create: + path = path_create() + self._lookup_add(None, path, lvm_id, uuid) + + # print('get_object_path_by_lvm_id(%s, %s, %s, %s: return %s' % + # (uuid, lvm_id, str(path_create), str(gen_new), path)) return path diff --git a/test/dbus/lvmdbustest.py b/test/dbus/lvmdbustest.py index 31902c4..819d096 100755 --- a/test/dbus/lvmdbustest.py +++ b/test/dbus/lvmdbustest.py @@ -337,8 +337,16 @@ class TestDbusService(unittest.TestCase): if len(h_lv.HiddenLvs) > 0: self._verify_hidden_lookups(h_lv, vgname) - # print("Hidden check %s %s" % (h, h_lv.Name)) full_name = "%s/%s" % (vgname, h_lv.Name) + # print("Hidden check %s" % (full_name)) + lookup_path = mgr.LookUpByLvmId(full_name) + self.assertTrue(lookup_path != '/') + self.assertTrue(lookup_path == h_lv.object_path) + + # Lets's strip off the '[ ]' and make sure we can find + full_name = "%s/%s" % (vgname, h_lv.Name[1:-1]) + # print("Hidden check %s" % (full_name)) + lookup_path = mgr.LookUpByLvmId(full_name) self.assertTrue(lookup_path != '/') self.assertTrue(lookup_path == h_lv.object_path) @@ -1294,7 +1302,8 @@ if __name__ == '__main__': # Default to no lvm shell set_execution(False) - if test_shell == 0: + if int(test_shell) == 0: + print('\n Shortened fork & exec test ***\n') unittest.main(exit=True) else: print('\n *** Testing fork & exec *** \n')