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

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. --- src/bhyve/bhyve_command.c | 49 ++++++++++++++++++++-- .../bhyvexml2argv-bhyveload-bootorder.args | 10 +++++ .../bhyvexml2argv-bhyveload-bootorder.ldargs | 3 ++ .../bhyvexml2argv-bhyveload-bootorder.xml | 30 +++++++++++++ .../bhyvexml2argv-bhyveload-bootorder1.args | 10 +++++ .../bhyvexml2argv-bhyveload-bootorder1.ldargs | 3 ++ .../bhyvexml2argv-bhyveload-bootorder1.xml | 29 +++++++++++++ .../bhyvexml2argv-bhyveload-bootorder2.args | 9 ++++ .../bhyvexml2argv-bhyveload-bootorder2.ldargs | 3 ++ .../bhyvexml2argv-bhyveload-bootorder2.xml | 24 +++++++++++ .../bhyvexml2argv-bhyveload-bootorder3.args | 10 +++++ .../bhyvexml2argv-bhyveload-bootorder3.ldargs | 3 ++ .../bhyvexml2argv-bhyveload-bootorder3.xml | 30 +++++++++++++ tests/bhyvexml2argvtest.c | 4 ++ 14 files changed, 214 insertions(+), 3 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.xml diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 6576029..1e5d4c7 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -522,6 +522,48 @@ virBhyveProcessBuildGrubbhyveCmd(virDomainDefPtr def, return cmd; } +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; + } + } + + 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]; + } + } + + 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")); return virBhyveProcessBuildBhyveloadCmd(def, disk); } else if (strstr(def->os.bootloader, "grub-bhyve") != NULL) { diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.args new file mode 100644 index 0000000..01a0290 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img \ +-s 4:0,ahci-cd,/tmp/cdrom.iso bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.ldargs new file mode 100644 index 0000000..24e0bc2 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/cdrom.iso bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.xml new file mode 100644 index 0000000..487caf0 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder.xml @@ -0,0 +1,30 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <boot dev='cdrom'/> + <boot dev='hd'/> + </os> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='file' type='raw'/> + <source file='/tmp/cdrom.iso'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.args new file mode 100644 index 0000000..01a0290 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img \ +-s 4:0,ahci-cd,/tmp/cdrom.iso bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.ldargs new file mode 100644 index 0000000..32538b5 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.xml new file mode 100644 index 0000000..6ea4631 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder1.xml @@ -0,0 +1,29 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='file' type='raw'/> + <source file='/tmp/cdrom.iso'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.args new file mode 100644 index 0000000..88de179 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.args @@ -0,0 +1,9 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.ldargs new file mode 100644 index 0000000..32538b5 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.xml new file mode 100644 index 0000000..f086a9e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder2.xml @@ -0,0 +1,24 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <boot dev='cdrom'/> + <boot dev='hd'/> + </os> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.args new file mode 100644 index 0000000..01a0290 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img \ +-s 4:0,ahci-cd,/tmp/cdrom.iso bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.ldargs new file mode 100644 index 0000000..24e0bc2 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/cdrom.iso bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.xml new file mode 100644 index 0000000..c3edcdf --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder3.xml @@ -0,0 +1,30 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <boot order='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='file' type='raw'/> + <source file='/tmp/cdrom.iso'/> + <target dev='hda' bus='sata'/> + <boot order='1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 3e57a78..bea4caa 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -130,6 +130,10 @@ mymain(void) DO_TEST("grub-defaults"); DO_TEST("grub-bootorder"); DO_TEST("grub-bootorder2"); + DO_TEST("bhyveload-bootorder"); + DO_TEST("bhyveload-bootorder1"); + DO_TEST("bhyveload-bootorder2"); + DO_TEST("bhyveload-bootorder3"); DO_TEST("bhyveload-explicitargs"); DO_TEST("custom-loader"); DO_TEST("disk-cdrom-grub"); -- 2.4.6

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 :|

Daniel P. Berrange wrote:
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.
The reason that I did it this way was because my original intention was to make it work with virt-manager smoother. However, I think you're right, it's better to error on configuration we cannot properly support instead of letting users have false expectation. It will make creation of new VM via virt-manager fail with the default configuration, but it's not critical because user still have to modify some hardware settings anyway to make it work with bhyve (i.e.: drop vnc and pty console). Thanks for the review, I'll upload v2 shortly. Roman Bogorodskiy
participants (2)
-
Daniel P. Berrange
-
Roman Bogorodskiy