virt-xml: Denest and comment code flow a bit

Add lots of early exits to remove implicit fall through
behavior that I find hard to follow

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("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="Either update or start")
+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")

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,
+        inactive_xmlobj = virtinst.Guest(conn, parsexml=options.stdinxml)
-                parsexml=options.stdinxml)
+    vm_is_running = bool(active_xmlobj)
-
-    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))
 
     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)
-        devs, action = prepare_changes(inactive_xmlobj, options, parserclass)
+    if not options.define:
-        if options.define:
+        if options.start:
-            dom = define_changes(conn, inactive_xmlobj,
+            start_domain_transient(conn, inactive_xmlobj, devs,
-                                 devs, action, options.confirm)
+                                   action, options.confirm)
-            if dom and options.start:
+        return 0
-                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)
 
-    if not options.update and not options.define and not options.start:
+    dom = define_changes(conn, inactive_xmlobj,
-        prepare_changes(inactive_xmlobj, options, parserclass)
+                         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