Skip to content

Commit

Permalink
qvm-volume: refuse to shrink volume unless --force option is used
Browse files Browse the repository at this point in the history
Right now Admin API backend will refuse to shrink volume anyway, but
we're planning to relax this restriction. Make sure the client side
(qvm-volume tool here, GUI VM settings already have this in place) will
employ appropriate safety check.

QubesOS/qubes-issues#3725
  • Loading branch information
marmarek committed Mar 20, 2018
1 parent 25803fd commit 70b15c2
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 8 deletions.
17 changes: 14 additions & 3 deletions doc/manpages/qvm-volume.rst
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,22 @@ Set property of given volume. Properties currently possible to change:

aliases: c, set, s

extend
resize
^^^^^^
| :command:`qvm-volume extend` [-h] [--verbose] [--quiet] *VMNAME:VOLUME* *NEW_SIZE*
| :command:`qvm-volume resize` [-h] [--force|-f] [--verbose] [--quiet] *VMNAME:VOLUME* *NEW_SIZE*
Extend the volume with *POOL_NAME:VOLUME_ID* TO *NEW_SIZE*
Resize the volume with *VMNAME:VOLUME* TO *NEW_SIZE*

If new size is smaller than current, the tool will refuse to continue unless
`--force` option is used. One should be very careful about that, because
shrinking volume without shrinking filesystem and other data inside first, will
surely end with data loss.

.. option:: -f, --force

Force operation even if new size is smaller than the current one.

aliases: extend

revert
^^^^^^
Expand Down
73 changes: 69 additions & 4 deletions qubesadmin/tests/tools/qvm_volume.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,18 @@ def test_010_extend(self):
self.app.expected_calls[
('testvm', 'admin.vm.volume.List', None, None)] = \
b'0\x00root\nprivate\n'
self.app.expected_calls[
('testvm', 'admin.vm.volume.Info', 'private', None)] = \
b'0\x00pool=lvm\n' \
b'vid=qubes_dom0/vm-testvm-private\n' \
b'size=2147483648\n' \
b'usage=10000000\n' \
b'rw=True\n' \
b'source=\n' \
b'save_on_stop=True\n' \
b'snap_on_start=False\n' \
b'revisions_to_keep=3\n' \
b'is_outdated=False\n'
self.app.expected_calls[
('testvm', 'admin.vm.volume.Resize', 'private', b'10737418240')] = \
b'0\x00'
Expand All @@ -169,15 +181,68 @@ def test_011_extend_error(self):
('testvm', 'admin.vm.volume.List', None, None)] = \
b'0\x00root\nprivate\n'
self.app.expected_calls[
('testvm', 'admin.vm.volume.Resize', 'private', b'1073741824')] = \
('testvm', 'admin.vm.volume.Info', 'private', None)] = \
b'0\x00pool=lvm\n' \
b'vid=qubes_dom0/vm-testvm-private\n' \
b'size=2147483648\n' \
b'usage=10000000\n' \
b'rw=True\n' \
b'source=\n' \
b'save_on_stop=True\n' \
b'snap_on_start=False\n' \
b'revisions_to_keep=3\n' \
b'is_outdated=False\n'
self.app.expected_calls[
('testvm', 'admin.vm.volume.Resize', 'private', b'10737418240')] = \
b'2\x00StoragePoolException\x00\x00Failed to resize volume: ' \
b'shrink not allowed\x00'
b'error: success\x00'
with qubesadmin.tests.tools.StderrBuffer() as stderr:
self.assertEqual(1,
qubesadmin.tools.qvm_volume.main(
['extend', 'testvm:private', '10GiB'],
app=self.app))
self.assertIn('error: success', stderr.getvalue())
self.assertAllCalled()

def test_012_extend_deny_shrink(self):
self.app.expected_calls[('dom0', 'admin.vm.List', None, None)] = \
b'0\x00testvm class=AppVM state=Running\n'
self.app.expected_calls[
('testvm', 'admin.vm.volume.List', None, None)] = \
b'0\x00root\nprivate\n'
self.app.expected_calls[
('testvm', 'admin.vm.volume.Info', 'private', None)] = \
b'0\x00pool=lvm\n' \
b'vid=qubes_dom0/vm-testvm-private\n' \
b'size=2147483648\n' \
b'usage=10000000\n' \
b'rw=True\n' \
b'source=\n' \
b'save_on_stop=True\n' \
b'snap_on_start=False\n' \
b'revisions_to_keep=3\n' \
b'is_outdated=False\n'
with qubesadmin.tests.tools.StderrBuffer() as stderr:
self.assertEqual(1,
qubesadmin.tools.qvm_volume.main(
['extend', 'testvm:private', '1GiB'],
['resize', 'testvm:private', '1GiB'],
app=self.app))
self.assertIn('shrink not allowed', stderr.getvalue())
self.assertIn('shrinking of private is disabled', stderr.getvalue())
self.assertAllCalled()

def test_013_resize_force_shrink(self):
self.app.expected_calls[('dom0', 'admin.vm.List', None, None)] = \
b'0\x00testvm class=AppVM state=Running\n'
self.app.expected_calls[
('testvm', 'admin.vm.volume.List', None, None)] = \
b'0\x00root\nprivate\n'
self.app.expected_calls[
('testvm', 'admin.vm.volume.Resize', 'private', b'1073741824')] = \
b'0\x00'
self.assertEqual(0,
qubesadmin.tools.qvm_volume.main(
['resize', '-f', 'testvm:private', '1GiB'],
app=self.app))
self.assertAllCalled()

def test_020_revert(self):
Expand Down
12 changes: 11 additions & 1 deletion qubesadmin/tools/qvm_volume.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,13 @@ def extend_volumes(args):
'''
volume = args.volume
size = qubesadmin.utils.parse_size(args.size)
if not args.force and size < volume.size:
raise qubesadmin.exc.StoragePoolException(
'For your own safety, shrinking of %s is'
' disabled (%d < %d). If you really know what you'
' are doing, resize filesystem manually first, then use `-f` '
'option.' %
(volume.name, size, volume.size))
volume.resize(size)


Expand Down Expand Up @@ -237,10 +244,13 @@ def init_revert_parser(sub_parsers):
def init_extend_parser(sub_parsers):
''' Add 'extend' action related options '''
extend_parser = sub_parsers.add_parser(
"extend", help="extend volume from domain")
"resize", aliases=('extend', ), help="resize volume for domain")
extend_parser.add_argument(metavar='VM:VOLUME', dest='volume',
action=qubesadmin.tools.VMVolumeAction)
extend_parser.add_argument('size', help='New size in bytes')
extend_parser.add_argument('--force', '-f', action='store_true',
help='Force operation, even if new size is smaller than the current '
'one')
extend_parser.set_defaults(func=extend_volumes)

def init_info_parser(sub_parsers):
Expand Down

0 comments on commit 70b15c2

Please sign in to comment.