Re: [PATCH v2 2/2] bhyve: bhyveload: respect boot dev and boot order

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jan 02, 2016 at 11:53:00PM +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.
> ---
>  src/bhyve/bhyve_command.c                          | 92 ++++++++++++++++++++--
>  .../bhyvexml2argv-bhyveload-bootorder.args         | 10 +++
>  .../bhyvexml2argv-bhyveload-bootorder.ldargs       |  3 +
>  .../bhyvexml2argv-bhyveload-bootorder.xml          | 29 +++++++
>  .../bhyvexml2argv-bhyveload-bootorder1.args        | 10 +++
>  .../bhyvexml2argv-bhyveload-bootorder1.ldargs      |  3 +
>  .../bhyvexml2argv-bhyveload-bootorder1.xml         | 29 +++++++
>  .../bhyvexml2argv-bhyveload-bootorder2.xml         | 23 ++++++
>  .../bhyvexml2argv-bhyveload-bootorder3.args        | 10 +++
>  .../bhyvexml2argv-bhyveload-bootorder3.ldargs      |  3 +
>  .../bhyvexml2argv-bhyveload-bootorder3.xml         | 29 +++++++
>  .../bhyvexml2argv-bhyveload-bootorder4.xml         | 30 +++++++
>  .../bhyvexml2argv-bhyveload-bootorder5.xml         | 30 +++++++
>  .../bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.xml |  1 +
>  tests/bhyvexml2argvtest.c                          | 75 +++++++++++++-----
>  15 files changed, 352 insertions(+), 25 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.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
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder4.xml
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-bootorder5.xml
> 
> diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> index 5f3055d..8ae3de1 100644
> --- a/src/bhyve/bhyve_command.c
> +++ b/src/bhyve/bhyve_command.c
> @@ -522,22 +522,100 @@ virBhyveProcessBuildGrubbhyveCmd(virDomainDefPtr def,
>      return cmd;
>  }
>  
> -virCommandPtr
> -virBhyveProcessBuildLoadCmd(virConnectPtr conn, virDomainDefPtr def,
> -                            const char *devmap_file, char **devicesmap_out)
> +static virDomainDiskDefPtr
> +virBhyveGetBootDisk(virConnectPtr conn, virDomainDefPtr def)
>  {
> -    virDomainDiskDefPtr disk;
> +    size_t i;
> +    virDomainDiskDefPtr match = NULL;
> +    int boot_dev = -1;
>  
>      if (def->ndisks < 1) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("domain should have at least one disk defined"));
> +                       _("Domain should have at least one disk defined"));
> +        return NULL;
> +    }
> +
> +    if (def->os.nBootDevs > 1) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("Only one boot device is supported"));
>          return NULL;
> +    } else if (def->os.nBootDevs == 1) {
> +        switch (def->os.bootDevs[0]) {
> +        case VIR_DOMAIN_BOOT_CDROM:
> +            boot_dev = VIR_DOMAIN_DISK_DEVICE_CDROM;
> +            break;
> +        case VIR_DOMAIN_BOOT_DISK:
> +            boot_dev = VIR_DOMAIN_DISK_DEVICE_DISK;
> +            break;
> +        default:
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("Cannot boot from device %s"),
> +                           virDomainBootTypeToString(def->os.bootDevs[0]));
> +            return NULL;
> +        }
>      }
>  
> +    if (boot_dev != -1) {
> +        /* If boot_dev is set, we return the first device of
> +         * the request type */
> +        for (i = 0; i < def->ndisks; i++) {
> +            if (!virBhyveUsableDisk(conn, def->disks[i]))
> +                continue;
> +
> +            if (def->disks[i]->device == boot_dev)
> +                match = def->disks[i];

Don't you need to have a 'break' here, otherwise you'll carry
on matching against the 2nd, 3rd, ... disk of that type

> +        }
> +
> +        if (match == NULL) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("Cannot find boot device of requested type %s"),
> +                           virDomainBootTypeToString(def->os.bootDevs[0]));
> +            return NULL;
> +        }

ACK with that fix above, since rest looks fine.

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]