From c081ed8c82b6cd6bf891fa0ad8defa89fd6d8684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Mon, 20 Apr 2020 03:08:24 +0200 Subject: [PATCH] Enable caching in qvm-ls and qvm-prefs Both tools issue a large number of Admin API calls and greatly benefit from a cache filled with a single per-vm Admin API call (admin.vm.property.GetAll). In case of qvm-ls, this also saves multiple admin.vm.CurrentState calls (power state is given in the admin.vm.List response too). QubesOS/qubes-issues#3293 --- qubesadmin/tests/tools/qubes_prefs.py | 8 ++- qubesadmin/tests/tools/qvm_ls.py | 78 +++++++++++++-------------- qubesadmin/tests/tools/qvm_prefs.py | 10 ++-- qubesadmin/tools/qvm_ls.py | 4 ++ qubesadmin/tools/qvm_prefs.py | 3 ++ 5 files changed, 50 insertions(+), 53 deletions(-) diff --git a/qubesadmin/tests/tools/qubes_prefs.py b/qubesadmin/tests/tools/qubes_prefs.py index 9d1ebf4..853e5cb 100644 --- a/qubesadmin/tests/tools/qubes_prefs.py +++ b/qubesadmin/tests/tools/qubes_prefs.py @@ -29,11 +29,9 @@ class TC_00_qubes_prefs(qubesadmin.tests.QubesTestCase): ('dom0', 'admin.property.List', None, None)] = \ b'0\x00prop1\nprop2\n' self.app.expected_calls[ - ('dom0', 'admin.property.Get', 'prop1', None)] = \ - b'0\x00default=True type=str value1' - self.app.expected_calls[ - ('dom0', 'admin.property.Get', 'prop2', None)] = \ - b'0\x00default=False type=str value2' + ('dom0', 'admin.property.GetAll', None, None)] = \ + b'0\x00prop1 default=True type=str value1\n' \ + b'prop2 default=False type=str value2\n' with qubesadmin.tests.tools.StdoutBuffer() as stdout: self.assertEqual(0, qubesadmin.tools.qubes_prefs.main([], app=self.app)) self.assertEqual(stdout.getvalue(), diff --git a/qubesadmin/tests/tools/qvm_ls.py b/qubesadmin/tests/tools/qvm_ls.py index 75f8418..ab27575 100644 --- a/qubesadmin/tests/tools/qvm_ls.py +++ b/qubesadmin/tests/tools/qvm_ls.py @@ -278,42 +278,34 @@ class TC_90_List_with_qubesd_calls(qubesadmin.tests.QubesTestCase): b'0\x00vm1 class=AppVM state=Running\n' \ b'template1 class=TemplateVM state=Halted\n' \ b'sys-net class=AppVM state=Running\n' - self.app.expected_calls[ - ('vm1', 'admin.vm.CurrentState', None, None)] = \ - b'0\x00power_state=Running' - self.app.expected_calls[ - ('sys-net', 'admin.vm.CurrentState', None, None)] = \ - b'0\x00power_state=Running' - self.app.expected_calls[ - ('template1', 'admin.vm.CurrentState', None, None)] = \ - b'0\x00power_state=Halted' props = { - 'label': b'type=label green', - 'template': b'type=vm template1', - 'netvm': b'type=vm sys-net', + 'label': 'type=label green', + 'template': 'type=vm template1', + 'netvm': 'type=vm sys-net', # 'virt_mode': b'type=str pv', } - for key, value in props.items(): - self.app.expected_calls[ - ('vm1', 'admin.vm.property.Get', key, None)] = \ - b'0\x00default=True ' + value - - # setup template1 - props['label'] = b'type=label black' - for key, value in props.items(): - self.app.expected_calls[ - ('template1', 'admin.vm.property.Get', key, None)] = \ - b'0\x00default=True ' + value self.app.expected_calls[ - ('template1', 'admin.vm.property.Get', 'template', None)] = \ - b'' # request refused - no such property + ('vm1', 'admin.vm.property.GetAll', None, None)] = \ + b'0\x00' + ''.join( + '{} default=True {}\n'.format(key, value) + for key, value in props.items()).encode() # setup sys-net - props['label'] = b'type=label red' - for key, value in props.items(): - self.app.expected_calls[ - ('sys-net', 'admin.vm.property.Get', key, None)] = \ - b'0\x00default=True ' + value + props['label'] = 'type=label red' + self.app.expected_calls[ + ('sys-net', 'admin.vm.property.GetAll', None, None)] = \ + b'0\x00' + ''.join( + '{} default=True {}\n'.format(key, value) + for key, value in props.items()).encode() + + # setup template1 + props['label'] = 'type=label black' + del props['template'] + self.app.expected_calls[ + ('template1', 'admin.vm.property.GetAll', None, None)] = \ + b'0\x00' + ''.join( + '{} default=True {}\n'.format(key, value) + for key, value in props.items()).encode() with qubesadmin.tests.tools.StdoutBuffer() as stdout: qubesadmin.tools.qvm_ls.main([], app=self.app) @@ -337,22 +329,24 @@ class TC_90_List_with_qubesd_calls(qubesadmin.tests.QubesTestCase): ('sys-net', 'admin.vm.CurrentState', None, None)] = \ b'0\x00power_state=Running' props = { - 'label': b'type=label green', - 'template': b'type=vm template1', - 'netvm': b'type=vm sys-net', + 'label': 'type=label green', + 'template': 'type=vm template1', + 'netvm': 'type=vm sys-net', # 'virt_mode': b'type=str pv', } - for key, value in props.items(): - self.app.expected_calls[ - ('vm1', 'admin.vm.property.Get', key, None)] = \ - b'0\x00default=True ' + value + self.app.expected_calls[ + ('vm1', 'admin.vm.property.GetAll', None, None)] = \ + b'0\x00' + ''.join( + '{} default=True {}\n'.format(key, value) + for key, value in props.items()).encode() # setup sys-net - props['label'] = b'type=label red' - for key, value in props.items(): - self.app.expected_calls[ - ('sys-net', 'admin.vm.property.Get', key, None)] = \ - b'0\x00default=True ' + value + props['label'] = 'type=label red' + self.app.expected_calls[ + ('sys-net', 'admin.vm.property.GetAll', None, None)] = \ + b'0\x00' + ''.join( + '{} default=True {}\n'.format(key, value) + for key, value in props.items()).encode() with qubesadmin.tests.tools.StdoutBuffer() as stdout: qubesadmin.tools.qvm_ls.main(['vm1', 'sys-net'], app=self.app) diff --git a/qubesadmin/tests/tools/qvm_prefs.py b/qubesadmin/tests/tools/qvm_prefs.py index 4d1e3ea..d0bb172 100644 --- a/qubesadmin/tests/tools/qvm_prefs.py +++ b/qubesadmin/tests/tools/qvm_prefs.py @@ -33,11 +33,9 @@ class TC_00_qvm_prefs(qubesadmin.tests.QubesTestCase): ('dom0', 'admin.vm.property.List', None, None)] = \ b'0\x00prop1\nprop2\n' self.app.expected_calls[ - ('dom0', 'admin.vm.property.Get', 'prop1', None)] = \ - b'0\x00default=True type=str value1' - self.app.expected_calls[ - ('dom0', 'admin.vm.property.Get', 'prop2', None)] = \ - b'0\x00default=False type=str value2' + ('dom0', 'admin.vm.property.GetAll', None, None)] = \ + b'0\x00prop1 default=True type=str value1\n' \ + b'prop2 default=False type=str value2\n' with qubesadmin.tests.tools.StdoutBuffer() as stdout: self.assertEqual(0, qubesadmin.tools.qvm_prefs.main([ 'dom0'], app=self.app)) @@ -46,7 +44,7 @@ class TC_00_qvm_prefs(qubesadmin.tests.QubesTestCase): 'prop2 - value2\n') with qubesadmin.tests.tools.StdoutBuffer() as stdout: self.assertEqual(0, qubesadmin.tools.qvm_prefs.main([ - 'dom0','--hide-default'], app=self.app)) + 'dom0', '--hide-default'], app=self.app)) self.assertEqual(stdout.getvalue(), 'prop2 - value2\n') self.assertAllCalled() diff --git a/qubesadmin/tools/qvm_ls.py b/qubesadmin/tools/qvm_ls.py index d0eb961..e3a8270 100644 --- a/qubesadmin/tools/qvm_ls.py +++ b/qubesadmin/tools/qvm_ls.py @@ -664,6 +664,10 @@ def main(args=None, app=None): parser.print_error(str(e)) return 1 + # fetch all the properties with one Admin API call, instead of issuing + # one call per property + args.app.cache_enabled = True + if args.raw_list: args.raw_data = True args.fields = 'name' diff --git a/qubesadmin/tools/qvm_prefs.py b/qubesadmin/tools/qvm_prefs.py index fc91492..d8ae4c9 100644 --- a/qubesadmin/tools/qvm_prefs.py +++ b/qubesadmin/tools/qvm_prefs.py @@ -93,6 +93,9 @@ def process_actions(parser, args, target): return 0 if args.property is None: + # fetch all the properties with one Admin API call, instead of issuing + # one call per property + args.app.cache_enabled = True properties = target.property_list() width = max(len(prop) for prop in properties)