Re: [libvirt] [PATCHv8 1/7] bhyve: Support /domain/bootloader configuration for non-FreeBSD guests.
On Tue, Nov 11, 2014 at 9:52 AM, Michal Privoznik <mprivozn(a)redhat.com> wrote:
On 08.11.2014 17:48, Conrad Meyer wrote:
> +static virCommandPtr
> +virBhyveProcessBuildBhyveloadCmd(virDomainDefPtr def, virDomainDiskDefPtr
> disk)
> {
> virCommandPtr cmd;
> - virDomainDiskDefPtr disk;
>
> - if (def->ndisks < 1) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("domain should have at least one disk
> defined"));
> + cmd = virCommandNew(BHYVELOAD);
> +
> + if (def->os.bootloaderArgs == NULL) {
> + VIR_DEBUG("bhyveload with default arguments");
> +
> + /* Memory (MB) */
> + virCommandAddArg(cmd, "-m");
> + virCommandAddArgFormat(cmd, "%llu",
> + VIR_DIV_UP(def->mem.max_balloon, 1024));
One of the things I'm worried about is, if the boot args are not specified
we properly add memory configured in domain XML.
> +
> + /* Image path */
> + virCommandAddArg(cmd, "-d");
> + virCommandAddArg(cmd, virDomainDiskGetSource(disk));
> +
> + /* VM name */
> + virCommandAddArg(cmd, def->name);
> + } else {
> + VIR_DEBUG("bhyveload with arguments");
> + virAppendBootloaderArgs(cmd, def);
> + }
> +
However, IIUC the memory amount can be overridden with boot args. Is that
expected?
Yes. If you use bootloader_args, you get exactly what you ask for and no more.
> +static virCommandPtr
> +virBhyveProcessBuildGrubbhyveCmd(virDomainDefPtr def,
> + virConnectPtr conn,
> + const char *devmap_file,
> + char **devicesmap_out)
> +{
> + virDomainDiskDefPtr disk, cd;
> + virBuffer devicemap;
> + virCommandPtr cmd;
> + size_t i;
> +
> + if (def->os.bootloaderArgs != NULL)
> + return virBhyveProcessBuildCustomLoaderCmd(def);
> +
> + devicemap = (virBuffer)VIR_BUFFER_INITIALIZER;
> +
> + /* Search disk list for CD or HDD device. */
> + cd = disk = NULL;
> + for (i = 0; i < def->ndisks; i++) {
> + if (!virBhyveUsableDisk(conn, def->disks[i]))
> + 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));
> + }
> +
> + if (disk == NULL &&
> + def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> + disk = def->disks[i];
> + VIR_INFO("Picking %s as HDD", virDomainDiskGetSource(disk));
> + }
Can we utilize <boot order='X'/> attribute here?
This has come up before and the answer is yes, but I'd prefer to do so
in a later patch series; this one is already growing unwieldy.
> + }
> +
> + cmd = virCommandNew(def->os.bootloader);
> +
> + VIR_DEBUG("grub-bhyve with default arguments");
> +
> + if (devicesmap_out != NULL) {
> + /* Grub device.map (just for boot) */
> + if (disk != NULL)
> + virBufferAsprintf(&devicemap, "(hd0) %s\n",
> + virDomainDiskGetSource(disk));
> +
> + if (cd != NULL)
> + virBufferAsprintf(&devicemap, "(cd) %s\n",
> + virDomainDiskGetSource(cd));
> +
> + *devicesmap_out = virBufferContentAndReset(&devicemap);
> + }
> +
> + if (cd != NULL) {
> + virCommandAddArg(cmd, "--root");
> + virCommandAddArg(cmd, "cd");
> + } else {
> + virCommandAddArg(cmd, "--root");
> + virCommandAddArg(cmd, "hd0,msdos1");
> + }
> +
> + virCommandAddArg(cmd, "--device-map");
> + virCommandAddArg(cmd, devmap_file);
> +
> + /* Memory in MB */
> + virCommandAddArg(cmd, "--memory");
> virCommandAddArgFormat(cmd, "%llu",
> VIR_DIV_UP(def->mem.max_balloon, 1024));
>
> - /* Image path */
> - virCommandAddArg(cmd, "-d");
> - virCommandAddArg(cmd, virDomainDiskGetSource(disk));
> -
> /* VM name */
> virCommandAddArg(cmd, def->name);
>
> return cmd;
> }
Otherwise looking good.
Michal
Thanks!
Conrad