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

[Jiri Denemark]

On Mon, Oct 31, 2011 at 13:39:09 +0100, Philipp Hahn wrote:
> 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"));

Is there any way you can pass the exact order to Xen? Multiple devices with
bootIndex are allowed (the indexes are unique of course) to specify booting
preference and by setting (bootable 1) anytime bootIndex != 0 makes all such
devices bootable in unspecified order. If only one bootable device is
supported by Xen, you should probably emit (bootable 1) only for bootIndex = 1
and maybe even forbid more than one device with bootIndex.

You also need to add 'deviceboot' feature to guest capabilities XML so that
apps know they can use <boot order='n'/> when creating this kind of guests.

Jirka