[libvirt] [PATCH] bhyve: bhyveload: respect boot dev and boot order

[Daniel P. Berrange]

On Sun, Dec 13, 2015 at 06:41:21AM +0300, Roman Bogorodskiy wrote:
> Make bhyveload respect boot order as specified by os.boot section of the
> domain XML or by "boot order" for specific devices. As bhyve does not
> support a real boot order specification right now, it's just about
> choosing a single device to boot from.

So if bhyve only lets you specify a single device to boot from,
then we should report an error to the user if they provide more
than one <boot> element in the XML.


> +static virDomainDiskDefPtr
> +virBhyveGetBootDisk(virConnectPtr conn, virDomainDefPtr def)
> +{
> +    size_t i;
> +    virDomainDiskDefPtr match = NULL;
> +    int best_index = INT_MAX;
> +    int boot_cdrom = 0, boot_disk = 0;
> +
> +    if (def->ndisks == 0)
> +        return NULL;
> +
> +    for (i = 0; i < def->os.nBootDevs; i++) {
> +        switch (def->os.bootDevs[i]) {
> +        case VIR_DOMAIN_BOOT_CDROM:
> +            boot_cdrom = i;
> +            break;
> +        case VIR_DOMAIN_BOOT_DISK:
> +            boot_disk = i;
> +            break;
> +        }
> +    }

IOW instead of this, we should just do

   int boot_dev = -1;
   if (def->ndisks > 1) {
       virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                      _("Only one boot device is supported");
       return NULL;
   }

   switch (def->os.bootDevs[0]) {
      case VIR_DOMAIN_BOOT_DISK:
         boot_dev = VIR_DOMAIN_DISK_DEVICE_DISK;
	 break;
      case VIR_DOMAIN_BOOT_CDROM:
         boot_dev = VIR_DOMAIN_DISK_DEVICE_CDROM;
	 break;
      default:
          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                       _("Cannot boot from device %s"),
		        virDOmainBootDeviceTypeToString(def->os.bootDevs[0]));
         return NULL;
   }

Now look for the first disk where disk->device == boot_dev
and use that to boot from.

Note however that '<boot ...>' only takes effect if you have
*not* got any boot index values specified on devices. ie
they are considered mutually exclusive.

http://libvirt.org/formatdomain.html#elementsOSBIOS

  "The boot element and per-device boot elements are mutually exclusive."

So now in this loop:

> +    for (i = 0; i < def->ndisks; i++) {
> +        int bootIndex;
> +        if (!virBhyveUsableDisk(conn, def->disks[i]))
> +            continue;
> +
> +        bootIndex = def->disks[i]->info.bootIndex;
> +        if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM)
> +            bootIndex += boot_cdrom;
> +        else if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK)
> +            bootIndex += boot_disk;
> +
> +        if (bootIndex < best_index) {
> +            best_index = bootIndex;
> +            match = def->disks[i];
> +        }
> +    }

If 'boot_dev != -1' then you should report an error if
you see any 'bootIndex != -1'.

If 'boot_dev == -1', then you should have exactly one
device with a bootIndex != -1. If you see multiple
devices with a boot index set, you should again report
an VIR_ERR_CONFIG_UNSUPPORTED to inform the user that
their config cannot be honoured.

> +
> +    return match;
> +}
> +
>  virCommandPtr
>  virBhyveProcessBuildLoadCmd(virConnectPtr conn, virDomainDefPtr def,
>                              const char *devmap_file, char **devicesmap_out)
> @@ -535,10 +577,11 @@ virBhyveProcessBuildLoadCmd(virConnectPtr conn, virDomainDefPtr def,
>      }
>  
>      if (def->os.bootloader == NULL) {
> -        disk = def->disks[0];
> +        disk = virBhyveGetBootDisk(conn, def);
>  
> -        if (!virBhyveUsableDisk(conn, disk))
> -            return NULL;
> +        if (disk == NULL)
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("no bootable disks found"));

Push that error message into virBhyveGetBootDisk as there are multiple
different errors to report now.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|