diff --git a/tests/clitest.py b/tests/clitest.py index 85fa58e6..611f9d7c 100644 --- a/tests/clitest.py +++ b/tests/clitest.py @@ -1085,7 +1085,7 @@ vixml = App("virt-xml") c = vixml.add_category("misc", "") c.add_valid("--help") # basic --help test c.add_valid("--sound=? --tpm=?") # basic introspection test -c.add_valid("test-state-shutoff --edit --update --boot menu=on") # --update with inactive VM, should work but warn +c.add_valid("test-state-shutoff --edit --update --boot menu=on", grep="The VM is not running") # --update with inactive VM, should work but warn c.add_valid("test-for-virtxml --edit --graphics password=foo --update --confirm", input_text="no\nno\n") # prompt exiting c.add_valid("test-for-virtxml --edit --cpu host-passthrough --no-define --start --confirm", input_text="no") # transient prompt exiting c.add_valid("test-for-virtxml --edit --metadata name=test-for-virtxml", grep="requested changes will have no effect") @@ -1093,8 +1093,8 @@ c.add_invalid("test --edit 2 --events on_poweroff=destroy", grep="'--edit 2' doe c.add_invalid("test --os-variant fedora26 --edit --cpu host-passthrough", grep="--os-variant is not supported") c.add_invalid("test-for-virtxml --os-variant fedora26 --remove-device --disk 1", grep="--os-variant is not supported") c.add_invalid("--build-xml --os-variant fedora26 --disk path=foo", grep="--os-variant is not supported") -c.add_invalid("domain-idontexist --cpu host-passthrough --start", grep="Could not find domain") -c.add_invalid("test-state-shutoff --edit --update --boot menu=on --start", grep="Either update or start") +c.add_invalid("domain-idontexist --edit --cpu host-passthrough --start", grep="Could not find domain") +c.add_invalid("test-state-shutoff --edit --update --boot menu=on --start", grep="Cannot mix --update") c.add_invalid("test --edit --update --events on_poweroff=destroy", grep="Don't know how to --update for --events") c.add_invalid("--edit --cpu host-passthrough --confirm", input_file=(XMLDIR + "/virtxml-stdin-edit.xml"), grep="Can't use --confirm with stdin") c.add_invalid("--edit --cpu host-passthrough --update", input_file=(XMLDIR + "/virtxml-stdin-edit.xml"), grep="Can't use --update with stdin") diff --git a/virt-xml b/virt-xml index 5a7b40ba..4bcd145c 100755 --- a/virt-xml +++ b/virt-xml @@ -436,9 +436,6 @@ def main(conn=None): options.quiet = False cli.setupLogging("virt-xml", options.debug, options.quiet) - if options.update and options.start: - fail(_("Either update or start a domain")) - if cli.check_option_introspection(options): return 0 @@ -453,6 +450,10 @@ def main(conn=None): else: fail(_("A domain must be specified")) + # Default to --define, unless: + # --no-define explicitly specified + # --print-* option is used + # XML input came from stdin if not options.print_xml and not options.print_diff: if options.stdinxml: if not options.define: @@ -463,6 +464,21 @@ def main(conn=None): if options.confirm and not options.print_xml: options.print_diff = True + # Ensure only one of these actions wash specified + # --edit + # --remove-device + # --add-device + # --build-xml + check_action_collision(options) + + # Ensure there wasn't more than one device/xml config option + # specified. So reject '--disk X --network X' + parserclass = check_xmlopt_collision(options) + + if options.update and not parserclass.guest_propname: + fail(_("Don't know how to --update for --%s") % + (parserclass.cli_arg_name)) + conn = cli.getConnection(options.connect, conn) domain = None @@ -472,15 +488,8 @@ def main(conn=None): domain, inactive_xmlobj, active_xmlobj = get_domain_and_guest( conn, options.domain) else: - inactive_xmlobj = virtinst.Guest(conn, - parsexml=options.stdinxml) - - check_action_collision(options) - parserclass = check_xmlopt_collision(options) - - if options.update and not parserclass.guest_propname: - fail(_("Don't know how to --update for --%s") % - (parserclass.cli_arg_name)) + inactive_xmlobj = virtinst.Guest(conn, parsexml=options.stdinxml) + vm_is_running = bool(active_xmlobj) if options.build_xml: devs = action_build_xml(conn, options, parserclass, inactive_xmlobj) @@ -489,35 +498,48 @@ def main(conn=None): print_stdout(dev.get_xml()) return 0 + performed_update = False if options.update: - if active_xmlobj: + if options.update and options.start: + fail(_("Cannot mix --update and --start")) + + if vm_is_running: devs, action = prepare_changes(active_xmlobj, options, parserclass) update_changes(domain, devs, action, options.confirm) + performed_update = True else: logging.warning( _("The VM is not running, --update is inapplicable.")) + if not options.define: + # --update and --no-define passed, so we are done + # It's hard to hit this case with the test suite + return 0 # pragma: no cover - if options.define or options.start: - devs, action = prepare_changes(inactive_xmlobj, options, parserclass) - if options.define: - dom = define_changes(conn, inactive_xmlobj, - devs, action, options.confirm) - if dom and options.start: - try: - dom.create() - except libvirt.libvirtError as e: # pragma: no cover - fail(_("Failed starting domain '%s': %s") % (inactive_xmlobj.name, e)) - print_stdout(_("Domain '%s' started successfully.") % - inactive_xmlobj.name) - elif not options.update and active_xmlobj and dom: - print_stdout( - _("Changes will take effect after the domain is fully powered off.")) - else: - dom = start_domain_transient(conn, inactive_xmlobj, devs, - action, options.confirm) + devs, action = prepare_changes(inactive_xmlobj, options, parserclass) + if not options.define: + if options.start: + start_domain_transient(conn, inactive_xmlobj, devs, + action, options.confirm) + return 0 - if not options.update and not options.define and not options.start: - prepare_changes(inactive_xmlobj, options, parserclass) + dom = define_changes(conn, inactive_xmlobj, + devs, action, options.confirm) + if not dom: + # --confirm user said 'no' + return 0 + + if options.start: + try: + dom.create() + except libvirt.libvirtError as e: # pragma: no cover + fail(_("Failed starting domain '%s': %s") % ( + inactive_xmlobj.name, e)) + print_stdout(_("Domain '%s' started successfully.") % + inactive_xmlobj.name) + + elif vm_is_running and not performed_update: + print_stdout( + _("Changes will take effect after the domain is fully powered off.")) return 0