lvm2-commits September 2016

lvm2-commits@lists.fedorahosted.org

7 participants
101 discussions

master - lvmlockd: fix segfault in error path
by David Teigland 28 Sep '16

28 Sep '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=49a1c4d4b0f2c5fb… Commit: 49a1c4d4b0f2c5fbc83a1ad4bfde59a366049b2a Parent: bd96036835e088de6a59811834b5fa0747274a62 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Wed Sep 28 13:29:55 2016 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Wed Sep 28 13:29:55 2016 -0500 lvmlockd: fix segfault in error path The log_debug statement was ignoring the NULL vg error case. --- lib/cache/lvmetad.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c index 727d3d3..8e9bb1f 100644 --- a/lib/cache/lvmetad.c +++ b/lib/cache/lvmetad.c @@ -2061,7 +2061,8 @@ scan_more: dm_config_destroy(vgmeta_ret); } out: - log_debug_lvmetad("Rescan VG %s done (seqno %u).", vg_ret->name, vg_ret->seqno); + if (vg_ret) + log_debug_lvmetad("Rescan VG %s done (seqno %u).", vg_ret->name, vg_ret->seqno); return vg_ret; }

1 0

master - lvmdbusd: Use 'pv_missing' column instead of '[unknown]'
by tasleson 28 Sep '16

28 Sep '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=bd96036835e088de… Commit: bd96036835e088de6a59811834b5fa0747274a62 Parent: be5eb995d356610b0748b05b65f74c34934526e4 Author: Tony Asleson <tasleson(a)redhat.com> AuthorDate: Wed Sep 28 11:18:10 2016 -0500 Committer: Tony Asleson <tasleson(a)redhat.com> CommitterDate: Wed Sep 28 12:07:48 2016 -0500 lvmdbusd: Use 'pv_missing' column instead of '[unknown]' Previously using '[unknown]' for PV device path comparison which is incorrect as it's not always true. --- daemons/lvmdbusd/cmdhandler.py | 4 ++-- daemons/lvmdbusd/lvmdb.py | 7 +++++++ daemons/lvmdbusd/objectmanager.py | 8 +++++--- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/daemons/lvmdbusd/cmdhandler.py b/daemons/lvmdbusd/cmdhandler.py index c8c7baa..3cb2cbd 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'] @@ -522,7 +522,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: 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/objectmanager.py b/daemons/lvmdbusd/objectmanager.py index 3284e1c..71149a5 100644 --- a/daemons/lvmdbusd/objectmanager.py +++ b/daemons/lvmdbusd/objectmanager.py @@ -320,10 +320,12 @@ class ObjectManager(AutomatedProperties): # 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]'. When - # we see the lvm_id as such we will re-assign to None + # 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 \ - lvm_id == '[unknown]': + cfg.db.pv_missing(uuid): lvm_id = None # Lets check for the uuid first

1 0

master - lvmdbusd: Remove extraneous parameter
by tasleson 28 Sep '16

28 Sep '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=be5eb995d356610b… Commit: be5eb995d356610b0748b05b65f74c34934526e4 Parent: a93616cf66b15591f741dc5f1450c12bc34b3785 Author: Tony Asleson <tasleson(a)redhat.com> AuthorDate: Wed Sep 28 11:16:49 2016 -0500 Committer: Tony Asleson <tasleson(a)redhat.com> CommitterDate: Wed Sep 28 12:05:44 2016 -0500 lvmdbusd: Remove extraneous parameter Remove 'gen_new' parameter as it's not needed, correct documentation. --- daemons/lvmdbusd/lv.py | 9 +++------ daemons/lvmdbusd/manager.py | 6 ++---- daemons/lvmdbusd/objectmanager.py | 30 +++++++++++++++++------------- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/daemons/lvmdbusd/lv.py b/daemons/lvmdbusd/lv.py index 455f371..2a8a3fb 100644 --- a/daemons/lvmdbusd/lv.py +++ b/daemons/lvmdbusd/lv.py @@ -101,8 +101,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 +130,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 @@ -160,8 +158,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 = '/' diff --git a/daemons/lvmdbusd/manager.py b/daemons/lvmdbusd/manager.py index 0a5fe73..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!") @@ -164,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 d55ca99..3284e1c 100644 --- a/daemons/lvmdbusd/objectmanager.py +++ b/daemons/lvmdbusd/objectmanager.py @@ -209,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): """ @@ -289,23 +289,27 @@ class ObjectManager(AutomatedProperties): 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 # Check for Manager.LookUpByLvmId query, we cannot @@ -335,7 +339,7 @@ class ObjectManager(AutomatedProperties): self._uuid_verify(path, uuid, lvm_id) else: # We have exhausted all lookups, let's create if we can - if gen_new: + if path_create: path = path_create() self._lookup_add(None, path, lvm_id, uuid)

1 0

master - lvmdbusd: Add diagnostic validate for look ups
by tasleson 27 Sep '16

27 Sep '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=a93616cf66b15591… Commit: a93616cf66b15591f741dc5f1450c12bc34b3785 Parent: d906fd520129d639c7cad27c24f88836d133f9ad Author: Tony Asleson <tasleson(a)redhat.com> AuthorDate: Tue Sep 27 10:08:23 2016 -0500 Committer: Tony Asleson <tasleson(a)redhat.com> CommitterDate: Tue Sep 27 13:28:54 2016 -0500 lvmdbusd: Add diagnostic validate for look ups Make sure that the lookup tables don't have extranoues stuff in them. --- daemons/lvmdbusd/manager.py | 6 +++++- daemons/lvmdbusd/objectmanager.py | 28 +++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/daemons/lvmdbusd/manager.py b/daemons/lvmdbusd/manager.py index b7d7b10..0a5fe73 100644 --- a/daemons/lvmdbusd/manager.py +++ b/daemons/lvmdbusd/manager.py @@ -115,6 +115,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 +126,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, diff --git a/daemons/lvmdbusd/objectmanager.py b/daemons/lvmdbusd/objectmanager.py index f491df4..d55ca99 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, pv_obj_path_generate +from .utils import log_debug, pv_obj_path_generate, log_error from .automatedproperties import AutomatedProperties @@ -70,6 +71,31 @@ 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

1 0

master - lvmdbustest.py: Make sure to test for hidden lookups
by tasleson 27 Sep '16

27 Sep '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=d906fd520129d639… Commit: d906fd520129d639c7cad27c24f88836d133f9ad Parent: 063265eacda95dbc1567e647734a000c8d68c8b7 Author: Tony Asleson <tasleson(a)redhat.com> AuthorDate: Mon Sep 26 22:03:12 2016 -0500 Committer: Tony Asleson <tasleson(a)redhat.com> CommitterDate: Tue Sep 27 13:28:54 2016 -0500 lvmdbustest.py: Make sure to test for hidden lookups Test both vgname/[hidden] and vgname/hidden forms --- test/dbus/lvmdbustest.py | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/test/dbus/lvmdbustest.py b/test/dbus/lvmdbustest.py index 7a2c44f..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)

1 0

master - lvmdbusd: Allow PV device names to be '[unknown]'
by tasleson 27 Sep '16

27 Sep '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=063265eacda95dbc… Commit: 063265eacda95dbc1567e647734a000c8d68c8b7 Parent: a882eb2b3b6eba6a482d4dcb1079a3ffa615fb6a Author: Tony Asleson <tasleson(a)redhat.com> AuthorDate: Mon Sep 26 22:02:08 2016 -0500 Committer: Tony Asleson <tasleson(a)redhat.com> CommitterDate: Tue Sep 27 13:28:54 2016 -0500 lvmdbusd: Allow PV device names to be '[unknown]' When a PV device is missing lvm will return '[unknown]' for the device path. The object manager keeps a hash table lookup for uuid and for PV's device name. When we had multiple PVs with the same device path we we only had 1 key in the table for the lvm id (device path). This caused a problem when the PV device transitioned from '[unknown]' to known as any subsequent transitions would cause an exception: Traceback (most recent call last): File "/usr/lib/python3.5/site-packages/lvmdbusd/request.py", line 66, in run_cmd result = self.method(*self.arguments) File "/usr/lib/python3.5/site-packages/lvmdbusd/manager.py", line 205, in _pv_scan cfg.load() File "/usr/lib/python3.5/site-packages/lvmdbusd/fetch.py", line 24, in load cache_refresh=False)[1] File "/usr/lib/python3.5/site-packages/lvmdbusd/pv.py", line 48, in load_pvs emit_signal, cache_refresh) File "/usr/lib/python3.5/site-packages/lvmdbusd/loader.py", line 80, in common cfg.om.remove_object(cfg.om.get_object_by_path(k), True) File "/usr/lib/python3.5/site-packages/lvmdbusd/objectmanager.py", line 153, in remove_object self._lookup_remove(path) File "/usr/lib/python3.5/site-packages/lvmdbusd/objectmanager.py", line 97, in _lookup_remove del self._id_to_object_path[lvm_id] KeyError: '[unknown]' when trying to delete a key that wasn't present. In this case we don't add a lookup key for the device path and the PV can only be located by UUID. Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1379357 --- daemons/lvmdbusd/objectmanager.py | 35 +++++++++++++++++++++++++++-------- 1 files changed, 27 insertions(+), 8 deletions(-) diff --git a/daemons/lvmdbusd/objectmanager.py b/daemons/lvmdbusd/objectmanager.py index 55f1729..f491df4 100644 --- a/daemons/lvmdbusd/objectmanager.py +++ b/daemons/lvmdbusd/objectmanager.py @@ -13,7 +13,7 @@ import traceback import dbus import os from . import cfg -from .utils import log_debug +from .utils import log_debug, pv_obj_path_generate from .automatedproperties import AutomatedProperties @@ -75,7 +75,7 @@ class ObjectManager(AutomatedProperties): 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 +85,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 +99,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): @@ -206,7 +216,8 @@ 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 and uuid not in self._id_to_object_path: obj = self.get_object_by_path(path) @@ -221,12 +232,14 @@ class ObjectManager(AutomatedProperties): :param lvm_id: lvm_id to verify :return: None """ - # This gets called when we found an object based on lvm_id, ensure - # uuid is correct too, as they can change + # 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 @@ -277,6 +290,12 @@ class ObjectManager(AutomatedProperties): # 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]'. When + # we see the lvm_id as such we will re-assign to None + if path_create == pv_obj_path_generate and \ + lvm_id == '[unknown]': + lvm_id = None + # Lets check for the uuid first path = self._id_lookup(uuid) if path:

1 0

master - lvmdbusd: Clean up objectmanager.get_object_path_by_uuid_lvm_id
by tasleson 27 Sep '16

27 Sep '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=a882eb2b3b6eba6a… Commit: a882eb2b3b6eba6a482d4dcb1079a3ffa615fb6a Parent: ec076b1df239dff91377a6e19ea76a4047b04b32 Author: Tony Asleson <tasleson(a)redhat.com> AuthorDate: Mon Sep 26 21:56:11 2016 -0500 Committer: Tony Asleson <tasleson(a)redhat.com> CommitterDate: Tue Sep 27 13:28:54 2016 -0500 lvmdbusd: Clean up objectmanager.get_object_path_by_uuid_lvm_id This method grew crufty with a number of stuble bugs. Refactor and simplify. --- daemons/lvmdbusd/objectmanager.py | 106 +++++++++++++++++++++--------------- 1 files changed, 62 insertions(+), 44 deletions(-) diff --git a/daemons/lvmdbusd/objectmanager.py b/daemons/lvmdbusd/objectmanager.py index 0b810d8..55f1729 100644 --- a/daemons/lvmdbusd/objectmanager.py +++ b/daemons/lvmdbusd/objectmanager.py @@ -208,19 +208,46 @@ class ObjectManager(AutomatedProperties): # This gets called when we found an object based on lvm_id, ensure # uuid is correct too, as they can change 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 lvm_id, ensure + # uuid is correct too, as they can change + 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, @@ -240,44 +267,35 @@ class ObjectManager(AutomatedProperties): if gen_new: assert 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 + + # 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 gen_new: + 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

1 0

master - lvmdbustest.py: Make env variable work
by tasleson 27 Sep '16

27 Sep '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=ec076b1df239dff9… Commit: ec076b1df239dff91377a6e19ea76a4047b04b32 Parent: 1bc546269a7d2cae156827e282715410a4967d51 Author: Tony Asleson <tasleson(a)redhat.com> AuthorDate: Mon Sep 26 21:52:08 2016 -0500 Committer: Tony Asleson <tasleson(a)redhat.com> CommitterDate: Tue Sep 27 13:28:45 2016 -0500 lvmdbustest.py: Make env variable work --- test/dbus/lvmdbustest.py | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/test/dbus/lvmdbustest.py b/test/dbus/lvmdbustest.py index 31902c4..7a2c44f 100755 --- a/test/dbus/lvmdbustest.py +++ b/test/dbus/lvmdbustest.py @@ -1294,7 +1294,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')

1 0

master - lvconvert: Disable thin pool raid conversion while active.
by Alasdair Kergon 27 Sep '16

27 Sep '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=1bc546269a7d2cae… Commit: 1bc546269a7d2cae156827e282715410a4967d51 Parent: 56c90ffa5e7d7c69c5230ac3bbee3ed03e7bae62 Author: Alasdair G Kergon <agk(a)redhat.com> AuthorDate: Tue Sep 27 18:22:54 2016 +0100 Committer: Alasdair G Kergon <agk(a)redhat.com> CommitterDate: Tue Sep 27 18:22:54 2016 +0100 lvconvert: Disable thin pool raid conversion while active. Works if the pool is inactive. Activation code doesn't notice a new raid dependency in on-disk metadata when a thin LV is already active. https://bugzilla.redhat.com/1365286 --- WHATS_NEW | 1 + lib/metadata/raid_manip.c | 5 +++++ 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index aa6c0c6..b0140be 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.02.167 - ====================================== + Disable lvconvert of thin pool to raid while active. Disable systemd service start rate limiting for lvm2-pvscan@.service. Version 2.02.166 - 26th September 2016 diff --git a/lib/metadata/raid_manip.c b/lib/metadata/raid_manip.c index deb88a2..f8a3c46 100644 --- a/lib/metadata/raid_manip.c +++ b/lib/metadata/raid_manip.c @@ -776,6 +776,11 @@ static int _raid_add_images_without_commit(struct logical_volume *lv, return 0; } + if (lv_is_active(lv_lock_holder(lv)) && (old_count == 1) && (lv_is_thin_pool_data(lv) || lv_is_thin_pool_metadata(lv))) { + log_error("Can't add image to active thin pool LV %s yet. Deactivate first.", display_lvname(lv)); + return 0; + } + if (!archive(lv->vg)) return_0;

1 0

master - libdm: fix dm_stats_delete_region() backwards compat
by Bryn Reeves 27 Sep '16

27 Sep '16

Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=56c90ffa5e7d7c69… Commit: 56c90ffa5e7d7c69c5230ac3bbee3ed03e7bae62 Parent: 6ec8854fdb051b092d5e262dc6c6d4c2ea075cd1 Author: Bryn M. Reeves <bmr(a)redhat.com> AuthorDate: Tue Sep 27 17:46:52 2016 +0100 Committer: Bryn M. Reeves <bmr(a)redhat.com> CommitterDate: Tue Sep 27 17:58:05 2016 +0100 libdm: fix dm_stats_delete_region() backwards compat The dm_stats_delete_region() call removes a region from the bound device, and, if the region is grouped, from the group leader group descriptor stored in aux_data. To do this requires a listed handle: previous versions of the library do not since no dependencies exist between regions without grouping. This leads to strange behaviour when a command built against an old version of the library is used with one supporting groups. Deleting a region with dmstats succeeds, but logs errors: # dmstats list Name RgID RgSta RgSiz #Areas ArSize ProgID vg_hex-root 0 0 1.00g 1 1.00g dmstats vg_hex-root 1 1.00g 1.00g 1 1.00g dmstats vg_hex-root 2 2.00g 1.00g 1 1.00g dmstats # dmstats delete --regionid 2 vg_hex/root Region ID 2 does not exist Could not delete statistics region. Command failed # dmstats list Name RgID RgSta RgSiz #Areas ArSize ProgID vg_hex-root 0 0 1.00g 1 1.00g dmstats vg_hex-root 1 1.00g 1.00g 1 1.00g dmstats This happens because the call to dm_stats_delete_region() is inside a dm_stats_walk_*() iterator: upon entry to the call, the iterator is at its end conditions and about to terminate. Due to the call to dm_stats_list() inside the function, it returns with an iterator at the beginning of a walk and performs a further iteration before exiting. This final loop makes a further attempt to delete the (already deleted) region, leading to the confusing error messages. --- WHATS_NEW_DM | 1 + libdm/libdm-stats.c | 48 +++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/WHATS_NEW_DM b/WHATS_NEW_DM index b048f7a..ed07307 100644 --- a/WHATS_NEW_DM +++ b/WHATS_NEW_DM @@ -1,5 +1,6 @@ Version 1.02.136 - ====================================== + Fix 'dmstats delete' with dmsetup older than v1.02.129 Fix stats walk segfault with dmsetup older than v1.02.129 Version 1.02.135 - 26th September 2016 diff --git a/libdm/libdm-stats.c b/libdm/libdm-stats.c index 75e0144..a033861 100644 --- a/libdm/libdm-stats.c +++ b/libdm/libdm-stats.c @@ -1550,7 +1550,7 @@ out: int dm_stats_walk_end(struct dm_stats *dms) { - if (!dms || !dms->regions) + if (!dms) return 1; if (_stats_walk_end(dms, &dms->cur_flags, @@ -2020,26 +2020,47 @@ int dm_stats_delete_region(struct dm_stats *dms, uint64_t region_id) { char msg[STATS_MSG_BUF_LEN]; struct dm_task *dmt; + int listed = 0; if (!_stats_bound(dms)) return_0; - if (!dms->regions && !dm_stats_list(dms, dms->program_id)) { + /* + * To correctly delete a region, that may be part of a group, a + * listed handle is required, since the region may need to be + * removed from another region's group descriptor; earlier + * versions of the region deletion interface do not have this + * requirement since there are no dependencies between regions. + * + * Listing a previously unlisted handle has numerous + * side-effects on other calls and operations (e.g. stats + * walks), especially when returning to a function that depends + * on the state of the region table, or statistics cursor. + * + * To avoid changing the semantics of the API, and the need for + * a versioned symbol, maintain a flag indicating when a listing + * has been carried out, and drop the region table before + * returning. + * + * This ensures compatibility with programs compiled against + * earlier versions of libdm. + */ + if (!dms->regions && !(listed = dm_stats_list(dms, dms->program_id))) { log_error("Could not obtain region list while deleting " "region ID " FMTu64, region_id); - return 0; + goto bad; } if (!dm_stats_get_nr_areas(dms)) { log_error("Could not delete region ID " FMTu64 ": " "no regions found", region_id); - return 0; + goto bad; } /* includes invalid and special region_id values */ if (!dm_stats_region_present(dms, region_id)) { log_error("Region ID " FMTu64 " does not exist", region_id); - return 0; + goto bad; } if(_stats_region_is_grouped(dms, region_id)) @@ -2047,12 +2068,12 @@ int dm_stats_delete_region(struct dm_stats *dms, uint64_t region_id) log_error("Could not remove region ID " FMTu64 " from " "group ID " FMTu64, region_id, dms->regions[region_id].group_id); - return 0; + goto bad; } if (!dm_snprintf(msg, sizeof(msg), "@stats_delete " FMTu64, region_id)) { log_error("Could not prepare @stats_delete message."); - return 0; + goto bad; } dmt = _stats_send_message(dms, msg); @@ -2060,10 +2081,19 @@ int dm_stats_delete_region(struct dm_stats *dms, uint64_t region_id) return_0; dm_task_destroy(dmt); - /* wipe region and mark as not present */ - _stats_region_destroy(&dms->regions[region_id]); + if (!listed) + /* wipe region and mark as not present */ + _stats_region_destroy(&dms->regions[region_id]); + else + /* return handle to prior state */ + _stats_regions_destroy(dms); return 1; +bad: + if (listed) + _stats_regions_destroy(dms); + + return 0; } int dm_stats_clear_region(struct dm_stats *dms, uint64_t region_id)

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

lvm2-commits September 2016