[libvirt] [PATCHv8 1/7] bhyve: Support /domain/bootloader configuration for non-FreeBSD guests.

Michal Privoznik mprivozn at redhat.com
Tue Nov 11 15:40:23 UTC 2014


On 11.11.2014 16:17, Conrad Meyer wrote:
> On Tue, Nov 11, 2014 at 9:52 AM, Michal Privoznik <mprivozn at 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.

Well, if one is modifying XML to change booloader_args already he can 
change the memory amount there too. What I'm trying to say is, we should 
add '-m def->mem.max_balloon' unconditionally. Or do you intend to give 
users so much freedom? I worried it can bite us later when libvirt fails 
to see the real amount of memory that domain has.

>
>>> +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.

Agreed. This can be addressed later, when needed.

Michal




More information about the libvir-list mailing list