[libvirt] [PATCH v2 3/4] xen: fix PyGrub device order using boot/@order

[Philipp Hahn]

When PyGrub is used as the bootloader in Xen, it gets passed the first
bootable disk. Xend supports a "bootable"-flag for this, which was
previously unused.
In commit c2969ec7aec5c40519aadf422ab5c47a21938bff the bootable=1 flag
was used to re-order the disks when converting from SEXPR to XML, such
that on re-definition Xend would mark the first disk as bootable.
This got broken with commit c1a98d88255197a8446d08c0b1589861660e9064,
which reorders all disks according to their target-name, making it
impossible to change the boot order without changing the device names.

When converting from Xen-sexpr to xml, Xens boolean bootable-flag is
converted to libvirts cardinal bootIndex, tracking the order of disks as
returned by Xend. This satisfies the requirement of bootIndex being
unique.

When converting back to xen-sexpr, disks with an explicit bootIndex are
sorted ascending before all other disks and are marked as bootable.

Explicitly mark the first disk as bootable for compatibility with Xends
default behaviour.

Add capability "deviceboot" to Xen-PV-domains. This is not 100% correct,
since for example PyGrub only handles disk devices and not netboot.

Signed-off-by: Philipp Hahn <hahn at univention.de>
---
v2: explicitly sort disks by bootIndex
v2: Add deviceboot capability.
---
 src/xen/xen_hypervisor.c |    7 +++++++
 src/xenxs/xen_sxpr.c     |   39 ++++++++++++++++++++++++++++++---------
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 58ae6a3..faca121 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -2402,6 +2402,13 @@ xenHypervisorBuildCapabilities(virConnectPtr conn,
                                                    0,
                                                    1) == NULL)
                     goto no_memory;
+        } else { /* !hvm */
+            /* Limited suport of deviceboot Xen-PV domains using PyGrub */
+            if (virCapabilitiesAddGuestFeature(guest,
+                                               "deviceboot",
+                                               1,
+                                               0) == NULL)
+                    goto no_memory;
         }
 
     }
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index b78e316..9786ad3 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -335,6 +335,7 @@ xenParseSxprDisks(virDomainDefPtr def,
 {
     const struct sexpr *cur, *node;
     virDomainDiskDefPtr disk = NULL;
+    int bootIndex = 0;
 
     for (cur = root; cur->kind == SEXPR_CONS; cur = cur->u.s.cdr) {
         node = cur->u.s.car;
@@ -494,16 +495,13 @@ xenParseSxprDisks(virDomainDefPtr def,
                 strchr(mode, '!'))
                 disk->shared = 1;
 
+            if (STREQ_NULLABLE(bootable, "1"))
+                disk->bootIndex = ++bootIndex;
+
             if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0)
                 goto no_memory;
 
-            /* re-order disks if there is a bootable device */
-            if (STREQ_NULLABLE(bootable, "1")) {
-                def->disks[def->ndisks++] = def->disks[0];
-                def->disks[0] = disk;
-            } else {
-                def->disks[def->ndisks++] = disk;
-            }
+            def->disks[def->ndisks++] = disk;
             disk = NULL;
         }
     }
@@ -2043,6 +2041,7 @@ xenFormatSxpr(virConnectPtr conn,
     const char *tmp;
     char *bufout;
     int hvm = 0, i;
+    virDomainDiskDefPtr *disks = NULL;
 
     VIR_DEBUG("Formatting domain sexpr");
 
@@ -2315,10 +2314,31 @@ xenFormatSxpr(virConnectPtr conn,
         }
     }
 
+    /* Sort bootable disks in ascending order by bootIndex, other disks behind */
+    if (VIR_ALLOC_N(disks, def->ndisks) < 0)
+        goto error;
+    for (i = 0; i < def->ndisks; i++) {
+        if (def->disks[i]->bootIndex) {
+            int j;
+            /* move back later disks */
+            for (j = i;
+                 j > 0 && (disks[j - 1]->bootIndex == 0 ||
+                           disks[j - 1]->bootIndex > def->disks[i]->bootIndex);
+                 j--)
+                disks[j] = disks[j - 1];
+            disks[j] = def->disks[i];
+        } else {
+            /* append disks without explicit bootIndex to tail */
+            disks[i] = def->disks[i];
+        }
+    }
+
     for (i = 0 ; i < def->ndisks ; i++)
-        if (xenFormatSxprDisk(def->disks[i],
-                              &buf, hvm, xendConfigVersion, 0, -1) < 0)
+        if (xenFormatSxprDisk(disks[i],
+                              &buf, hvm, xendConfigVersion, 0,
+                              i == 0 || disks[i]->bootIndex > 0) < 0)
             goto error;
+    VIR_FREE(disks);
 
     for (i = 0 ; i < def->nnets ; i++)
         if (xenFormatSxprNet(conn, def->nets[i],
@@ -2349,6 +2369,7 @@ xenFormatSxpr(virConnectPtr conn,
     return bufout;
 
 error:
+    VIR_FREE(disks);
     virBufferFreeAndReset(&buf);
     return NULL;
 }
-- 
1.7.1