[libvirt] [PATCH] drvbhyve: Use boot-order for grub-bhyve boot device

[Michal Privoznik]

On 12.11.2014 23:31, Conrad Meyer wrote:
> Rather than just picking the first CD (or failing that, HDD) we come
> across, if the user has picked a boot device ordering with <boot
> order=''>, respect that (and just try to boot the lowest-index device).
> 
> Adds two sets of tests to bhyve2xmlargv; 'grub-bootorder' shows that we
> pick a user-specified device over the first device in the domain;
> 'grub-bootorder2' shows that we pick the first (lowest index) device.
> ---
> This is a follow-up to the 'Add non-FreeBSD guest support to Bhyve driver'
> patch series to fix the grub-bhyve automagic configuration to respect <boot
> order=''> in the domain. (Requested by both Roman and Michal, I believe.)
> ---
>   docs/drvbhyve.html.in                              |  9 +--
>   src/bhyve/bhyve_command.c                          | 64 ++++++++++++++++------
>   .../bhyvexml2argv-grub-bootorder.args              |  6 ++
>   .../bhyvexml2argv-grub-bootorder.devmap            |  1 +
>   .../bhyvexml2argv-grub-bootorder.ldargs            |  2 +
>   .../bhyvexml2argv-grub-bootorder.xml               | 36 ++++++++++++
>   .../bhyvexml2argv-grub-bootorder2.args             |  6 ++
>   .../bhyvexml2argv-grub-bootorder2.devmap           |  1 +
>   .../bhyvexml2argv-grub-bootorder2.ldargs           |  2 +
>   .../bhyvexml2argv-grub-bootorder2.xml              | 38 +++++++++++++
>   tests/bhyvexml2argvtest.c                          |  2 +
>   11 files changed, 146 insertions(+), 21 deletions(-)
>   create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.args
>   create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.devmap
>   create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.ldargs
>   create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder.xml
>   create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.args
>   create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.devmap
>   create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.ldargs
>   create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-bootorder2.xml
> 
> diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in
> index bd4b35e..5479511 100644
> --- a/docs/drvbhyve.html.in
> +++ b/docs/drvbhyve.html.in
> @@ -234,10 +234,11 @@ management.</p>
>   <p>It's possible to boot non-FreeBSD guests by specifying an explicit
>   bootloader, e.g. <code>grub-bhyve(1)</code>. Arguments to the bootloader may be
>   specified as well. If the bootloader is <code>grub-bhyve</code> and arguments
> -are omitted, libvirt will try and boot the first disk in the domain (either
> -<code>cdrom</code>- or <code>disk</code>-type devices). If the disk type is
> -<code>disk</code>, it will attempt to boot from the first partition in the disk
> -image.</p>
> +are omitted, libvirt will try and infer boot ordering from user-supplied
> +<boot order='N'> configuration in the domain. Failing that, it will boot
> +the first disk in the domain (either <code>cdrom</code>- or
> +<code>disk</code>-type devices). If the disk type is <code>disk</code>, it will
> +attempt to boot from the first partition in the disk image.</p>
>   
>   <pre>
>     ...
> diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> index 26d4797..6e3bf03 100644
> --- a/src/bhyve/bhyve_command.c
> +++ b/src/bhyve/bhyve_command.c
> @@ -381,38 +381,62 @@ virBhyveUsableDisk(virConnectPtr conn, virDomainDiskDefPtr disk)
>       return true;
>   }
>   
> +static void
> +virBhyveFormatGrubDevice(virBufferPtr devicemap, virDomainDiskDefPtr def)
> +{
> +
> +    if (def->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
> +        virBufferAsprintf(devicemap, "(cd) %s\n",
> +                          virDomainDiskGetSource(def));
> +    else
> +        virBufferAsprintf(devicemap, "(hd0) %s\n",
> +                          virDomainDiskGetSource(def));
> +}
> +
>   static virCommandPtr
>   virBhyveProcessBuildGrubbhyveCmd(virDomainDefPtr def,
>                                    virConnectPtr conn,
>                                    const char *devmap_file,
>                                    char **devicesmap_out)
>   {
> -    virDomainDiskDefPtr disk, cd;
> +    virDomainDiskDefPtr hdd, cd, userdef, diskdef;
>       virBuffer devicemap;
>       virCommandPtr cmd;
> +    int best_idx;
>       size_t i;
>   
>       if (def->os.bootloaderArgs != NULL)
>           return virBhyveProcessBuildCustomLoaderCmd(def);
>   
> +    best_idx = INT_MAX;
>       devicemap = (virBuffer)VIR_BUFFER_INITIALIZER;
>   
> -    /* Search disk list for CD or HDD device. */
> -    cd = disk = NULL;
> +    /* Search disk list for CD or HDD device. We'll respect <boot order=''> if
> +     * present and otherwise pick the first CD or failing that HDD we come
> +     * across. */
> +    cd = hdd = userdef = NULL;
>       for (i = 0; i < def->ndisks; i++) {
>           if (!virBhyveUsableDisk(conn, def->disks[i]))
>               continue;
>   
> +        diskdef = def->disks[i];
> +
> +        if (diskdef->info.bootIndex && diskdef->info.bootIndex < best_idx) {
> +            userdef = diskdef;
> +            best_idx = userdef->info.bootIndex;
> +            continue;
> +        }
> +
>           if (cd == NULL &&
>               def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
> -            cd = def->disks[i];
> -            VIR_INFO("Picking %s as boot CD", virDomainDiskGetSource(cd));
> +            cd = diskdef;
> +            VIR_INFO("Picking %s as CD", virDomainDiskGetSource(cd));
>           }
>   
> -        if (disk == NULL &&
> +        if (hdd == NULL &&
>               def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> -            disk = def->disks[i];
> -            VIR_INFO("Picking %s as HDD", virDomainDiskGetSource(disk));
> +            hdd = diskdef;
> +            VIR_INFO("Picking %s as HDD", virDomainDiskGetSource(hdd));
>           }
>       }

I think we need to honor boot order for CD-ROMs too. I mean, if you have two CD-ROMS like this:

    <disk type='file' device='cdrom'>
      <driver name='file' type='raw'/>
      <source file='/tmp/freebsd1.iso'/>
      <target dev='hda' bus='sata'/>
      <boot order='2'/>
    </disk>
    <disk type='file' device='cdrom'>
      <driver name='file' type='raw'/>
      <source file='/tmp/freebsd2.iso'/>
      <target dev='hda' bus='sata'/>
      <boot order='1'/>
    </disk>

I'd expect to boot from /tmp/freebsd2.iso. Although, on the other hand, there's only onde device we can boot from, right? If that's the case, your code works too. Or am I just off the page?

Michal