Skip to content

Commit

Permalink
storage/lvm: do not activate unused LVM volumes
Browse files Browse the repository at this point in the history 
This prevents udev from accessing not currently used volumes, and also
makes LVM snapshot operations faster (less locking to do).

There is at least one unsolved problem here: inactive volumes do not
have size listed on `lvs` output. There is a band-aid by caching the
usage from the last time the volume was active, but it isn't enough for
many cases (at the very least - reporting disk usage of a VM that wasn't
started recently).

There may be also other places that assume existing volumes always have
relevant /dev/ node present.

Fixes QubesOS/qubes-issues#6184
  • Loading branch information
@marmarek
marmarek committed Jan 14, 2021
1 parent 006e7a9 commit 1c7364e
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 74 deletions.
106 changes: 70 additions & 36 deletions qubes/storage/lvm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,16 +223,21 @@ def _parse_lvm_cache(lvm_output):
line = line.decode().strip()
pool_name, pool_lv, name, size, usage_percent, attr, \
origin, metadata_size, metadata_percent = line.split(';', 8)
if '' in [pool_name, name, size, usage_percent]:
if '' in [pool_name, name, size]:
continue
name = pool_name + "/" + name
size = int(size[:-1]) # Remove 'B' suffix
usage = int(size / 100 * float(usage_percent))
if usage_percent:
usage = int(size / 100 * float(usage_percent))
else:
# keep cached usage for non-active volumes
usage = size_cache.get(name, {}).get('usage')
if metadata_size:
metadata_size = int(metadata_size[:-1])
if metadata_percent:
metadata_usage = int(metadata_size / 100 * float(metadata_percent))
else:
metadata_usage = None
metadata_usage = size_cache.get(name, {}).get('metadata_usage')
result[name] = {'size': size, 'usage': usage, 'pool_lv': pool_lv,
'attr': attr, 'origin': origin, 'metadata_size': metadata_size,
'metadata_usage': metadata_usage}
Expand Down Expand Up @@ -277,6 +282,7 @@ def init_cache_coro(log=logging.getLogger('qubes.storage.lvm')):
return _parse_lvm_cache(out)

size_cache_time = 0
size_cache = {}
size_cache = init_cache()


Expand Down Expand Up @@ -357,7 +363,7 @@ def size(self, _):

@asyncio.coroutine
def _reset(self):
''' Resets a volatile volume '''
""" Resets a volatile volume. Leave it in an active state """
assert not self.snap_on_start and not self.save_on_stop, \
"Not a volatile volume"
self.log.debug('Resetting volatile %s', self.vid)
Expand All @@ -367,7 +373,7 @@ def _reset(self):
except qubes.storage.StoragePoolException:
pass
# pylint: disable=protected-access
cmd = ['create', self.pool._pool_id, self.vid.split('/')[1],
cmd = ['create-activate', self.pool._pool_id, self.vid.split('/')[1],
str(self.size)]
yield from qubes_lvm_coro(cmd, self.log)

Expand Down Expand Up @@ -420,7 +426,7 @@ def _commit(self, vid_to_commit=None, keep=False):
vid_to_commit = self._vid_snap

assert self._lock.locked()
if not os.path.exists('/dev/' + vid_to_commit):
if vid_to_commit not in size_cache:
# nothing to commit
return

Expand Down Expand Up @@ -466,21 +472,21 @@ def create(self):
def remove(self):
assert self.vid
try:
if os.path.exists('/dev/' + self._vid_snap):
if self._vid_snap in size_cache:
cmd = ['remove', self._vid_snap]
yield from qubes_lvm_coro(cmd, self.log)
except AttributeError:
pass

try:
if os.path.exists('/dev/' + self._vid_import):
if self._vid_import in size_cache:
cmd = ['remove', self._vid_import]
yield from qubes_lvm_coro(cmd, self.log)
except AttributeError:
pass

yield from self._remove_revisions(self.revisions.keys())
if not os.path.exists(self.path):
if self._vid_current not in size_cache:
return
cmd = ['remove', self.path]
yield from qubes_lvm_coro(cmd, self.log)
Expand All @@ -491,10 +497,22 @@ def remove(self):
def export(self):
''' Returns an object that can be `open()`. '''
# make sure the device node is available
qubes_lvm(['activate', self.path], self.log)
qubes_lvm(['activate', self._vid_current], self.log)
devpath = self.path
return devpath

def export_end(self, path):
''' Returns an object that can be `open()`. '''
# release device node
assert path == '/dev/' + self._vid_current
try:
qubes_lvm(['deactivate', path[len('/dev/'):]], self.log)
except subprocess.CalledProcessError as e:
self.log.warning('Cannot deactivate %s - failed with code %d',
self._vid_current, e.returncode)
# device may be in use by another export
pass

@qubes.storage.Volume.locked
@asyncio.coroutine
def import_volume(self, src_volume):
Expand All @@ -513,19 +531,20 @@ def import_volume(self, src_volume):
src_volume.pool.thin_pool == self.pool.thin_pool: # NOQA
yield from self._commit(src_volume.path[len('/dev/'):], keep=True)
else:
cmd = ['create',
cmd = ['create-activate',
self.pool._pool_id, # pylint: disable=protected-access
self._vid_import.split('/')[1],
str(src_volume.size)]
yield from qubes_lvm_coro(cmd, self.log)
yield from reset_cache_coro()
src_path = yield from qubes.utils.coro_maybe(src_volume.export())
try:
cmd = ['dd', 'if=' + src_path, 'of=/dev/' + self._vid_import,
'conv=sparse', 'status=none', 'bs=128K']
if not os.access('/dev/' + self._vid_import, os.W_OK) or \
not os.access(src_path, os.R_OK):
cmd.insert(0, 'sudo')

print(cmd)
p = yield from asyncio.create_subprocess_exec(*cmd)
yield from p.wait()
finally:
Expand All @@ -551,7 +570,7 @@ def import_data(self, size):
format(self.vid))
self.abort_if_import_in_progress()
# pylint: disable=protected-access
cmd = ['create', self.pool._pool_id, self._vid_import.split('/')[1],
cmd = ['create-activate', self.pool._pool_id, self._vid_import.split('/')[1],
str(size)]
yield from qubes_lvm_coro(cmd, self.log)
yield from reset_cache_coro()
Expand All @@ -562,19 +581,21 @@ def import_data(self, size):
@asyncio.coroutine
def import_data_end(self, success):
'''Either commit imported data, or discard temporary volume'''
if not os.path.exists('/dev/' + self._vid_import):
print(self._vid_import)
print(list(size_cache.keys()))
if self._vid_import not in size_cache:
raise qubes.storage.StoragePoolException(
'No import operation in progress on {}'.format(self.vid))
if success:
yield from self._commit(self._vid_import)
else:
cmd = ['remove', self._vid_import]
yield from qubes_lvm_coro(cmd, self.log)
yield from reset_cache_coro()

def abort_if_import_in_progress(self):
try:
devpath = '/dev/' + self._vid_import
if os.path.exists(devpath):
if self._vid_import in size_cache:
raise qubes.storage.StoragePoolException(
'Import operation in progress on {}'.format(self.vid))
except AttributeError: # self._vid_import
Expand All @@ -583,7 +604,9 @@ def abort_if_import_in_progress(self):

def is_dirty(self):
if self.save_on_stop:
return os.path.exists('/dev/' + self._vid_snap)
print('is_dirty({}): {}'.format(self._vid_snap, list(size_cache.keys())))
subprocess.check_output(['sudo', 'lvs'])
return self._vid_snap in size_cache
return False

def is_outdated(self):
Expand All @@ -605,9 +628,9 @@ def revert(self, revision=None):
if revision is None:
revision = \
max(self.revisions.items(), key=_revision_sort_key)[0]
old_path = '/dev/' + self.vid + '-' + revision
if not os.path.exists(old_path):
msg = "Volume {!s} has no {!s}".format(self, old_path)
old_vid = self.vid + '-' + revision
if old_vid not in size_cache:
msg = "Volume {!s} has no {!s}".format(self, old_vid)
raise qubes.storage.StoragePoolException(msg)

if self.vid in size_cache:
Expand Down Expand Up @@ -640,31 +663,40 @@ def resize(self, size):
return

if self.is_dirty():
print('dirty')
cmd = ['extend', self._vid_snap, str(size)]
yield from qubes_lvm_coro(cmd, self.log)
elif hasattr(self, '_vid_import') and \
os.path.exists('/dev/' + self._vid_import):
self._vid_import in size_cache:
print('in import')
cmd = ['extend', self._vid_import, str(size)]
yield from qubes_lvm_coro(cmd, self.log)
elif self.save_on_stop and not self.snap_on_start:
print('normal')
cmd = ['extend', self._vid_current, str(size)]
yield from qubes_lvm_coro(cmd, self.log)

print('done')
self._size = size
yield from reset_cache_coro()

@asyncio.coroutine
def _snapshot(self):
"""
Make an *active* snapshot of the source volume.
:return:
"""
try:
cmd = ['remove', self._vid_snap]
yield from qubes_lvm_coro(cmd, self.log)
except: # pylint: disable=bare-except
pass

if self.source is None:
cmd = ['clone', self._vid_current, self._vid_snap]
cmd = ['clone-activate', self._vid_current, self._vid_snap]
else:
cmd = ['clone', self.source.path, self._vid_snap]
cmd = ['clone-activate', self.source.path, self._vid_snap]
yield from qubes_lvm_coro(cmd, self.log)

@qubes.storage.Volume.locked
Expand All @@ -675,6 +707,9 @@ def start(self):
if self.snap_on_start or self.save_on_stop:
if not self.save_on_stop or not self.is_dirty():
yield from self._snapshot()
else:
cmd = ['activate', self._vid_snap]
yield from qubes_lvm_coro(cmd, self.log)
else:
yield from self._reset()
finally:
Expand Down Expand Up @@ -706,12 +741,7 @@ def verify(self):
vid = self.source.path[len('/dev/'):]
else:
vid = self._vid_current
try:
vol_info = size_cache[vid]
if vol_info['attr'][4] != 'a':
raise qubes.storage.StoragePoolException(
'volume {} not active'.format(vid))
except KeyError:
if vid not in size_cache:
raise qubes.storage.StoragePoolException(
'volume {} missing'.format(vid))
return True
Expand Down Expand Up @@ -755,25 +785,28 @@ def _get_lvm_cmdline(cmd):
:return array of str appropriate for subprocess.Popen
'''
action = cmd[0]
activate = '-ay' if '-activate' in action else '-an'
if action == 'remove':
lvm_cmd = ['lvremove', '-f', cmd[1]]
elif action == 'clone':
lvm_cmd = ['lvcreate', '-kn', '-ay', '-s', cmd[1], '-n', cmd[2]]
elif action == 'create':
lvm_cmd = ['lvcreate', '-T', cmd[1], '-kn', '-ay', '-n', cmd[2], '-V',
str(cmd[3]) + 'B']
elif action in ('clone', 'clone-activate'):
lvm_cmd = ['lvcreate', '-K', '-ky', activate, '-s', cmd[1], '-n', cmd[2]]
elif action in ('create', 'create-activate'):
lvm_cmd = ['lvcreate', '-T', cmd[1], '-K', '-ky', activate, '-n', cmd[2],
'-V', str(cmd[3]) + 'B']
elif action == 'extend':
size = int(cmd[2]) / (1024 * 1024)
lvm_cmd = ["lvextend", "-L%s" % size, cmd[1]]
elif action == 'activate':
lvm_cmd = ['lvchange', '-ay', cmd[1]]
lvm_cmd = ['lvchange', '-K', '-ay', cmd[1]]
elif action == 'deactivate':
lvm_cmd = ['lvchange', '-an', cmd[1]]
elif action == 'rename':
lvm_cmd = ['lvrename', cmd[1], cmd[2]]
else:
raise NotImplementedError('unsupported action: ' + action)
if lvm_is_very_old:
# old lvm in trusty image used there does not support -k option
lvm_cmd = [x for x in lvm_cmd if x != '-kn']
lvm_cmd = [x for x in lvm_cmd if x not in ('-kn', '-ky')]
if os.getuid() != 0:
cmd = ['sudo', 'lvm'] + lvm_cmd
else:
Expand Down Expand Up @@ -825,6 +858,7 @@ def qubes_lvm_coro(cmd, log=logging.getLogger('qubes.storage.lvm')):
close_fds=True, env=environ)
_, _ = yield from p.communicate()
cmd = _get_lvm_cmdline(cmd)
print('calling {}'.format(' '.join(cmd)))
p = yield from asyncio.create_subprocess_exec(*cmd,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
Expand Down
Loading

0 comments on commit 1c7364e

Please sign in to comment.