Re: [libvirt] [PATCH v2 3/4] xen: fix PyGrub device order using boot/@order
On Thu, Nov 03, 2011 at 11:21:24 +0100, Jiri Denemark wrote:
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, 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(a)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(-)
ACK
Actually, I'm changing my mind. After reviewing patch 4/4, seeing you also
added deviceboot for hvm guest capabilities and included it in both SEXPRs and
XMLs for hvm domains, and realizing why you most likely did that, I think you
should fix the code to be consistent with this. You do that because Xen hvm
boots from the first disk in SEXPR and you want to preserve that order while
doing SEXPR->XML->SEXPR, right? If so, I think you should add deviceboot
feature for both hvm and pv guests, but hvm should forbid bootindex > 1 since
it is not supported by xen (yet?). This way even users could influence what
should be the first disk that Xen will boot from when creating a new domain.
BTW, should xen{Parse,Format}XM be changed to do similar thing? That is
reorder disk device according to bootindex when creating xm config files?
Jirka