Re: [libvirt] [PATCH] xen: fix PyGrub device order using boot/@order

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, the exact order of bootable disks is lost, since Xend only stores a boolean flag. If multiple disks are marked bootable, the behaviour is undefined. Adapt all Xen-sexpr tests to now contain the extra '(bootbale 0)' flag. It must be explicitly set, otherwise Xend remembers the old state and only ever adds the bootable indicator. diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index d44b0dc..d571c72 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; } } @@ -1736,6 +1734,12 @@ xenFormatSxprDisk(virDomainDiskDefPtr def, virBufferAddLit(buf, "(mode 'w!')"); else virBufferAddLit(buf, "(mode 'w')"); + + if (def->bootIndex) + virBufferAddLit(buf, "(bootable 1)"); + else + virBufferAddLit(buf, "(bootable 0)"); + if (def->transient) { XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED, _("transient disks not supported yet"));