From 76f4599460768b95172ba4e61fbe317462c1cf3f Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Mon, 16 Sep 2024 12:05:33 -0400 Subject: [PATCH] virtxml: Don't require `options` at XML action time Adjust cli.py `run_parser` and similar functions to take the parservalue directly, rather than passing in the whole `options` structure. This makes it easier to reason about what the virt-xml action functions are actually working with. Signed-off-by: Cole Robinson --- virtinst/cli.py | 12 ++++++------ virtinst/virtinstall.py | 8 +++----- virtinst/virtxml.py | 31 +++++++++++++++++-------------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/virtinst/cli.py b/virtinst/cli.py index 3d6097510..eed11b57f 100644 --- a/virtinst/cli.py +++ b/virtinst/cli.py @@ -1616,12 +1616,12 @@ class ParserXML(VirtCLIParser): super()._parse(inst) -def parse_xmlcli(guest, options): +def parse_xmlcli(guest, parservalue): """ Parse --xml option string into XMLManualAction instances and append to guest.xml_actions. """ - for optstr in options.xml: + for optstr in parservalue: inst = guest.xml_actions.new() ParserXML(optstr).parse(inst) guest.xml_actions.append(inst) @@ -4876,16 +4876,15 @@ class ParserLaunchSecurity(VirtCLIParser): # Public virt parser APIs # ########################### -def run_parser(options, guest, parserclass, editinst=None): +def run_parser(guest, parserclass, parservalue, editinst=None): """ Lookup the cli options.* string associated with the passed in Parser* class, and parse its values into the passed guest instance, or editinst for some virt-xml usage. """ ret = [] - optstr_list = xmlutil.listify(getattr(options, parserclass.cli_arg_name)) - for optstr in optstr_list: + for optstr in xmlutil.listify(parservalue): parserobj = parserclass(optstr, guest=guest, editing=bool(editinst)) parseret = parserobj.parse(editinst) ret += xmlutil.listify(parseret) @@ -4896,7 +4895,8 @@ def run_parser(options, guest, parserclass, editinst=None): def run_all_parsers(options, guest): ret = [] for parserclass in VIRT_PARSERS: - ret += run_parser(options, guest, parserclass) + parservalue = getattr(options, parserclass.cli_arg_name) + ret += run_parser(guest, parserclass, parservalue) return ret diff --git a/virtinst/virtinstall.py b/virtinst/virtinstall.py index 130c8e28e..065f8847b 100644 --- a/virtinst/virtinstall.py +++ b/virtinst/virtinstall.py @@ -631,10 +631,8 @@ def _build_options_guest(conn, options): # We do these two parser bit early, since Installer setup will # depend on them, but delay the rest to later, since things like # disk naming can depend on Installer operations - cli.run_parser(options, guest, cli.ParserBoot) - options.boot = None - cli.run_parser(options, guest, cli.ParserMetadata) - options.metadata = None + cli.run_parser(guest, cli.ParserBoot, options.boot) + cli.run_parser(guest, cli.ParserMetadata, options.metadata) # Call set_capabilities_defaults explicitly here rather than depend # on set_defaults calling it. Installer setup needs filled in values. @@ -666,7 +664,7 @@ def build_guest_instance(conn, options): # default disk paths are generated based on VM name set_cli_default_name(guest) cli.run_all_parsers(options, guest) - cli.parse_xmlcli(guest, options) + cli.parse_xmlcli(guest, options.xml) set_cli_defaults(options, guest) installer.set_install_defaults(guest) diff --git a/virtinst/virtxml.py b/virtinst/virtxml.py index 2e7676cb1..202e87ca7 100644 --- a/virtinst/virtxml.py +++ b/virtinst/virtxml.py @@ -44,11 +44,11 @@ def get_diff(origxml, newxml): return diff -def set_os_variant(options, guest): - if options.os_variant is None: +def set_os_variant(guest, os_variant): + if os_variant is None: return - osdata = cli.parse_os_variant(options.os_variant) + osdata = cli.parse_os_variant(os_variant) if osdata.get_name(): guest.set_os_name(osdata.get_name()) @@ -231,12 +231,13 @@ def _find_objects_to_edit(guest, action_name, editval, parserclass): return inst -def action_edit(action, guest, options): +def action_edit(action, guest): parserclass = action.parserclass + parservalue = action.parservalue selector = action.selector if parserclass is cli.ParserXML: - cli.parse_xmlcli(guest, options) + cli.parse_xmlcli(guest, parservalue) return [] if parserclass.guest_propname: @@ -252,22 +253,23 @@ def action_edit(action, guest, options): devs = [] for editinst in xmlutil.listify(inst): - devs += cli.run_parser(options, guest, parserclass, + devs += cli.run_parser(guest, parserclass, parservalue, editinst=editinst) return devs -def action_add_device(action, guest, options, input_devs): +def action_add_device(action, guest, os_variant, input_devs): parserclass = action.parserclass + parservalue = action.parservalue - set_os_variant(options, guest) + set_os_variant(guest, os_variant) if input_devs: for dev in input_devs: guest.add_device(dev) devs = input_devs else: - devs = cli.run_parser(options, guest, parserclass) + devs = cli.run_parser(guest, parserclass, parservalue) for dev in devs: dev.set_defaults(guest) @@ -287,10 +289,11 @@ def action_remove_device(action, guest): return devs -def action_build_xml(action, guest, options): +def action_build_xml(action, guest): parserclass = action.parserclass + parservalue = action.parservalue - devs = cli.run_parser(options, guest, parserclass) + devs = cli.run_parser(guest, parserclass, parservalue) for dev in devs: dev.set_defaults(guest) return devs @@ -298,11 +301,11 @@ def action_build_xml(action, guest, options): def perform_action(action, guest, options, input_devs): if action.is_add_device: - return action_add_device(action, guest, options, input_devs) + return action_add_device(action, guest, options.os_variant, input_devs) if action.is_remove_device: return action_remove_device(action, guest) if action.is_edit: - return action_edit(action, guest, options) + return action_edit(action, guest) raise xmlutil.DevError( "perform_action() incorrectly called with action_name=%s" % action.action_name) @@ -566,7 +569,7 @@ def main(conn=None): vm_is_running = bool(active_xmlobj) if action.is_build_xml: - built_devs = action_build_xml(action, inactive_xmlobj, options) + built_devs = action_build_xml(action, inactive_xmlobj) for dev in built_devs: # pylint: disable=no-member print_stdout(xmlutil.unindent_device_xml(dev.get_xml()))