[virt-tools-list] [virt-manager PATCH v2] virt-xml: refactor the handling of --define and --update options

[Pavel Hrdina]

The code was wrong in many ways.  The main issue was, that for live
updates we were using config XML instead of live XML.

This patch fixes the --update and --define options to work properly as
described in man page.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1192875

Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
---

changes from v1:
 - fixed an issue, when you don't use --define nor --update options but only one
 of the --print-* options

 virt-xml | 62 ++++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/virt-xml b/virt-xml
index 744af3d..5dbe2e1 100755
--- a/virt-xml
+++ b/virt-xml
@@ -295,6 +295,33 @@ def update_changes(domain, devs, action, confirm):
             print_stdout("")
 
 
+def prepare_changes(xmlobj, options, parsermap, parserobj):
+    origxml = xmlobj.get_xml_config()
+
+    if options.edit != -1:
+        devs = action_edit(xmlobj, options, parsermap, parserobj)
+        action = "update"
+
+    elif options.add_device:
+        devs = action_add_device(xmlobj, options, parsermap, parserobj)
+        action = "hotplug"
+
+    elif options.remove_device:
+        devs = action_remove_device(xmlobj, options, parsermap, parserobj)
+        action = "hotunplug"
+
+    newxml = xmlobj.get_xml_config()
+    diff = get_diff(origxml, newxml)
+
+    if options.print_diff:
+        if diff:
+            print_stdout(diff)
+    elif options.print_xml:
+        print_stdout(newxml)
+
+    return devs, action
+
+
 #######################
 # CLI option handling #
 #######################
@@ -410,10 +437,6 @@ def main(conn=None):
     elif not options.build_xml:
         inactive_xmlobj = _make_guest(conn, options.stdinxml)
 
-    origxml = None
-    if inactive_xmlobj:
-        origxml = inactive_xmlobj.get_xml_config()
-
     check_action_collision(options)
     parserobj = check_xmlopt_collision(options, parsermap)
 
@@ -421,42 +444,25 @@ def main(conn=None):
         fail(_("Don't know how to --update for --%s") %
              (parserobj.cli_arg_name))
 
-    if options.edit != -1:
-        devs = action_edit(inactive_xmlobj, options, parsermap, parserobj)
-        action = "update"
-
-    elif options.add_device:
-        devs = action_add_device(inactive_xmlobj, options,
-                                 parsermap, parserobj)
-        action = "hotplug"
-
-    elif options.remove_device:
-        devs = action_remove_device(inactive_xmlobj, options,
-                                    parsermap, parserobj)
-        action = "hotunplug"
-
-    elif options.build_xml:
+    if options.build_xml:
         devs = action_build_xml(conn, options, parsermap, parserobj)
         for dev in util.listify(devs):
             print_stdout(dev.get_xml_config())
         return 0
 
-    newxml = inactive_xmlobj.get_xml_config()
-    diff = get_diff(origxml, newxml)
-
-    if options.print_diff:
-        if diff:
-            print_stdout(diff)
-    elif options.print_xml:
-        print_stdout(newxml)
-
     if options.update and active_xmlobj:
+        devs, action = prepare_changes(active_xmlobj, options,
+                                       parsermap, parserobj)
         update_changes(domain, devs, action, options.confirm)
     if options.define:
+        devs, action = prepare_changes(inactive_xmlobj, options,
+                                       parsermap, parserobj)
         define_changes(conn, inactive_xmlobj, devs, action, options.confirm)
         if not options.update and active_xmlobj:
             print_stdout(
                 _("Changes will take effect after the next domain shutdown."))
+    if not options.update and not options.define:
+        prepare_changes(inactive_xmlobj, options, parsermap, parserobj)
 
     return 0
 
-- 
2.4.5