[libvirt] [PATCH] virsh: Try to keep printed XML pretty with change-media

[Martin Kletzander]

When adding a new media with change-media and --print-xml, let's try
making it more readable and nice.

Before:
<disk type="file" device="cdrom">
...
      <target dev="hdb" bus="ide"/>
      <address type="drive" controller="0" bus="0" target="0" unit="1"/>
    <source file="/tmp/a.iso"/></disk>

After:
<disk type="file" device="cdrom">
...
      <source file="/tmp/a.iso"/>
      <target dev="hdb" bus="ide"/>
      <address type="drive" controller="0" bus="0" target="0" unit="1"/>
    </disk>

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

Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
---
 tools/virsh-domain.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index bd00785622b2..16b01d3dc631 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11566,7 +11566,10 @@ virshUpdateDiskXML(xmlNodePtr disk_node,
                    const char *target,
                    virshUpdateDiskXMLType type)
 {
+    xmlNodePtr tmp = NULL;
     xmlNodePtr source = NULL;
+    xmlNodePtr target_node = NULL;
+    xmlNodePtr text_node = NULL;
     char *device_type = NULL;
     char *ret = NULL;
     char *startupPolicy = NULL;
@@ -11583,9 +11586,31 @@ virshUpdateDiskXML(xmlNodePtr disk_node,
     }

     /* find the current source subelement */
-    for (source = disk_node->children; source; source = source->next) {
-        if (source->type == XML_ELEMENT_NODE &&
-            xmlStrEqual(source->name, BAD_CAST "source"))
+    for (tmp = disk_node->children; tmp; tmp = tmp->next) {
+        /*
+         * Safe the last text node before the <target/>.  The
+         * reasoning behind this is that the target node will be
+         * present in this case and also has a proper indentation.
+         */
+        if (!target_node && tmp->type == XML_TEXT_NODE)
+            text_node = tmp;
+
+        /*
+         * We need only element nodes from now on.
+         */
+        if (tmp->type != XML_ELEMENT_NODE)
+            continue;
+
+        if (xmlStrEqual(tmp->name, BAD_CAST "source"))
+            source = tmp;
+
+        if (xmlStrEqual(tmp->name, BAD_CAST "target"))
+            target_node = tmp;
+
+        /*
+         * We've found all we needed.
+         */
+        if (source && target_node)
             break;
     }

@@ -11637,7 +11662,22 @@ virshUpdateDiskXML(xmlNodePtr disk_node,

         if (startupPolicy)
             xmlNewProp(source, BAD_CAST "startupPolicy", BAD_CAST startupPolicy);
-        xmlAddChild(disk_node, source);
+
+        /*
+         * So that the output XML looks nice in case anyone calls
+         * 'change-media' with '--print-xml', let's attach the source
+         * before target...
+         */
+        xmlAddPrevSibling(target_node, source);
+
+        /*
+         * ... and duplicate the text node doing the indentation just
+         * so it's more easily readable.  And don't make it fatal.
+         */
+        if ((tmp = xmlCopyNode(text_node, 0))) {
+            if (!xmlAddPrevSibling(target_node, tmp))
+                xmlFreeNode(tmp);
+        }
     }

     if (!(ret = virXMLNodeToString(NULL, disk_node))) {
-- 
2.6.3