Skip to content

Commit 46a2e5a

Browse files
authored
NAS-135584 / 25.10 / Gracefully handle edge case when we are missing separator (#16498)
This commit adds changes to gracefully handle the case when we are missing a required separator in mount info line. Basically what happened is that it seems on boot after upgrade for this user (we saw an earlier ticket as well with the same issue), somewhere a call to `catalog.config` was called. Now this call made use of `filesystem.mountinfo` which was using `getmntinfo` call. The function `getmntinfo` raised an `IndexError` (more on this later), which was propagated above to ``` @Private async def _get_or_insert(self, datastore, options): try: return await self.middleware.call('datastore.config', datastore, options) except IndexError: async with get_or_insert_lock: try: return await self.middleware.call('datastore.config', datastore, options) except IndexError: await self.middleware.call('datastore.insert', datastore, {}) return await self.middleware.call('datastore.config', datastore, options) ``` which resulted in a duplicate entry being inserted to the database which was an invalid entry as we have unique constraint on `LABEL` for this specific table and another subsequent call to `catalog.config` resulted in another entry being added but that failing which resulted in the user not being able to utilize apps and we catching it. Now coming back to `getmntinfo`, basically what happens is that we expect `-` separator to be there in each line for `/proc/self/mountinfo` as documented in https://www.man7.org/linux/man-pages/man5/proc_pid_mountinfo.5.html as well. However it appears we might have a race condition where this is not the case which is why it results in an index error. ``` def __mntent_dict(line): mnt_id, parent_id, maj_min, root, mp, opts, extra = line.split(" ", 6) fstype, mnt_src, super_opts = extra.split(' - ')[1].split() ``` So the changes done here make sure that whenever these functions are called, if we do end up raising an `IndexError` on some line, we make sure that that we just silently skip that line. Full traceback below: ``` File "/usr/lib/python3/dist-packages/middlewared/service/config_service.py", line 110, in config return await self._get_or_insert(self._config.datastore, options) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/middlewared/service/config_service.py", line 130, in _get_or_insert return await self.middleware.call('datastore.config', datastore, options) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/concurrent/futures/thread.py", line 58, in run result = self.fn(*self.args, **self.kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/middlewared/api/base/decorator.py", line 96, in wrapped result = func(*args) ^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/middlewared/plugins/filesystem.py", line 135, in mount_info mntinfo = getmntinfo() ^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/middlewared/utils/mount.py", line 131, in getmntinfo __iter_mountinfo(dev_id=dev_id, callback=__parse_to_dev, private_data=info) File "/usr/lib/python3/dist-packages/middlewared/utils/mount.py", line 87, in __iter_mountinfo callback(line, private_data) File "/usr/lib/python3/dist-packages/middlewared/utils/mount.py", line 34, in __parse_to_dev entry = __mntent_dict(line) ^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/middlewared/utils/mount.py", line 11, in __mntent_dict fstype, mnt_src, super_opts = extra.split(' - ')[1].split() ~~~~~~~~~~~~~~~~~~^^^ IndexError: list index out of range ```
1 parent 93430c3 commit 46a2e5a

File tree

2 files changed

+28
-12
lines changed

2 files changed

+28
-12
lines changed

src/middlewared/middlewared/plugins/filesystem.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,20 @@ def is_dataset_path(self, path):
133133

134134
@filterable_api_method(private=True)
135135
def mount_info(self, filters, options):
136-
mntinfo = getmntinfo()
136+
retries = 1
137+
for i in range(retries):
138+
try:
139+
# getmntinfo() may be weird sometimes i.e check NAS-135584 where we might have a malformed line
140+
# in mountinfo
141+
mntinfo = getmntinfo()
142+
except Exception as e:
143+
if i >= retries:
144+
raise CallError(f'Unable to retrieve mount information: {e}')
145+
146+
time.sleep(0.25)
147+
else:
148+
break
149+
137150
return filter_list(list(mntinfo.values()), filters, options)
138151

139152
@api_method(FilesystemMkdirArgs, FilesystemMkdirResult, roles=['FILESYSTEM_DATA_WRITE'])

src/middlewared/middlewared/utils/mount.py

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,20 +71,23 @@ def __iter_mountinfo(dev_id=None, mnt_id=None, callback=None, private_data=None)
7171

7272
with open('/proc/self/mountinfo') as f:
7373
for line in f:
74-
if maj_min:
75-
if line.find(maj_min) == -1:
76-
continue
74+
try:
75+
if maj_min:
76+
if line.find(maj_min) == -1:
77+
continue
7778

78-
callback(line, private_data)
79-
break
80-
elif mnt_id is not None:
81-
if not line.startswith(mount_id):
82-
continue
79+
callback(line, private_data)
80+
break
81+
elif mnt_id is not None:
82+
if not line.startswith(mount_id):
83+
continue
8384

84-
callback(line, private_data)
85-
break
85+
callback(line, private_data)
86+
break
8687

87-
callback(line, private_data)
88+
callback(line, private_data)
89+
except Exception as e:
90+
raise RuntimeError(f'Failed to parse {line!r} line: {e}')
8891

8992

9093
def getmntinfo(dev_id=None, mnt_id=None):

0 commit comments

Comments
 (0)