From owner-freebsd-virtualization@FreeBSD.ORG Fri Oct 24 04:59:09 2014 Return-Path: Delivered-To: freebsd-virtualization@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 6C84195D for ; Fri, 24 Oct 2014 04:59:09 +0000 (UTC) Received: from mail-lb0-x22e.google.com (mail-lb0-x22e.google.com [IPv6:2a00:1450:4010:c04::22e]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D560A3B6 for ; Fri, 24 Oct 2014 04:59:08 +0000 (UTC) Received: by mail-lb0-f174.google.com with SMTP id p9so1919072lbv.5 for ; Thu, 23 Oct 2014 21:59:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=A/XwH9BvhfwSyp/HuLMj3ALaW2Rsl/Xd93vC3M6bLFE=; b=OnCSWTQpXTR/bdAFPv6zezA1N3Sh7wtyzhGDnQYIImBl99QzLQGt3P9QFzzPutGSRD 6wYDjyLrvTv++iPnKPEkRJIievsYA0BUx9tl00dVKRLIBhKh6V5/EZkOXfLocqSEpXap tMy1G9t1svpJV1l7+DhR7Es+5ABwbigvMVtvXi9Sf7UVNEc1BWXh5t03iJKSmDCBoWkw On7ChsfgX1EJaHwmkbEcEGR55caM3Ub7tYxUFyRShIEREuFfwTeYFWk762y5X8hpjsrF 5cnTpPGfc1IxNZ4CEce0TKSy3csZvciWw/J7PI/xF7nrq5SKUJ1t84lJoPnTOEFJvsXB gQ5Q== X-Received: by 10.152.30.33 with SMTP id p1mr1801286lah.78.1414126746664; Thu, 23 Oct 2014 21:59:06 -0700 (PDT) Received: from dev.san.ru (dev.san.ru. [88.147.129.60]) by mx.google.com with ESMTPSA id lk5sm1451595lac.45.2014.10.23.21.59.05 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 23 Oct 2014 21:59:06 -0700 (PDT) Date: Fri, 24 Oct 2014 08:58:45 +0400 From: Roman Bogorodskiy To: Conrad Meyer Subject: Re: [PATCHv2 1/2] bhyve: Support /domain/bootloader configuration for non-FreeBSD guests. Message-ID: <20141024045844.GA12422@dev.san.ru> References: <1414106923-32313-1-git-send-email-cse.cem@gmail.com> <1414106923-32313-2-git-send-email-cse.cem@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <1414106923-32313-2-git-send-email-cse.cem@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: libvir-list@redhat.com, Conrad Meyer , freebsd-virtualization@freebsd.org X-BeenThere: freebsd-virtualization@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "Discussion of various virtualization techniques FreeBSD supports." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Oct 2014 04:59:09 -0000 Conrad Meyer wrote: > We still default to bhyveloader(1) if no explicit bootloader > configuration is supplied in the domain. >=20 > If the /domain/bootloader looks like grub-bhyve and the user doesn't > supply /domain/bootloader_args, we make an intelligent guess and try > chainloading the first partition on the disk. >=20 > Caveats: > - We can't install from CD without explicit bootloader_args. > - We leave a device.map file lying around in /tmp. I don't see a good > way not to do so without reworking the API somewhat. > > Sponsored by: EMC / Isilon storage division Hi Conrad. Thanks for the patch. As for the /tmp, probably storing it somewhere in BHYVE_STATE_DIR would be more clean. However, I'm concerned about libvirt dealing with the grub-bhyve specifics as it involves some assumptions about its behaviour ahd has limitations, like the one you pointed with installing from CD. My idea was to add the following behaviour: * bootloader specified and bootloader_args specified -- use that as is * bootloader not specified, bootloader_args specified -- call bhyveload $bootloader_args $default_bhyveloader_args * bootloader and bootloader_args not set -- use defaults (current behaviour) * bootloader specified, bootloader_args missing -- fail here Actually, I've implemented that already, but didn't send because it needs some more testing. The reason of my concern about adding a special handling of grub-bhyve case that, as I've mentioned, we have to make assumptions and there are also caveats you mentioned. Probably it'd worth to wait for bhyve to provide single-step VM boot, i.e. UEFI support, that should appear soon anyway. There are some comments inline. > Signed-off-by: Conrad Meyer > --- > docs/drvbhyve.html.in | 28 ++++++++++-- > docs/formatdomain.html.in | 4 +- > po/libvirt.pot | 4 ++ > src/bhyve/bhyve_command.c | 107 ++++++++++++++++++++++++++++++++++++++++= +----- > 4 files changed, 128 insertions(+), 15 deletions(-) >=20 > diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in > index 39afdf5..6e85800 100644 > --- a/docs/drvbhyve.html.in > +++ b/docs/drvbhyve.html.in > @@ -37,8 +37,7 @@ bhyve+ssh://root@example.com/system (remote access, SSH= tunnelled) >

Example config

>

> The bhyve driver in libvirt is in its early stage and under active devel= opment. So it supports > -only limited number of features bhyve provides. All the supported featur= es could be found > -in this sample domain XML. > +only limited number of features bhyve provides. >

> =20 >

> @@ -50,8 +49,8 @@ up to 31 PCI devices. > =20 > 

>  <domain type=3D'bhyve'>
> -  <name>bhyve</name>
> -  <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> +    <name>bhyve</name>
> +    <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
>      <memory>219136</memory>
>      <currentMemory>219136</currentMemory>
>      <vcpu>1</vcpu>
> @@ -157,5 +156,26 @@ An example of domain XML device entry for that will =
look like:

>  
Please refer to the Storage documentation for more details on storage
>  management.

> =20
> +
Using grub2-bhyve or Alternative Bootloaders

> +
> +
It's possible to boot non-FreeBSD guests by specifying an explicit
> +bootloader, e.g. grub-bhyve(1). Arguments to the bootloader=
 may be
> +specified as well. If no arguments are given and bootloader is
> +grub-bhyve, libvirt will try and boot from the first partit=
ion of
> +the disk image.

> +
> +
> +  ...
> +    <bootloader>/usr/local/sbin/grub-bhyve</bootloader>
> +    <bootloader_args>...</bootloader_args>
> +  ...
> +

> +
> +
(Of course, to install from a CD a user will have to supply explicit
> +arguments to grub-bhyve.)

> +
> +
Caveat: bootloader_args does not support any quoting.
> +Filenames, etc, must not have spaces or they will be tokenized incorrect=
ly.

> +
>    
>  
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 0099ce7..b7b6c46 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -217,7 +217,9 @@
>        a BIOS, and instead the host is responsible to kicking off the
>        operating system boot. This may use a pseudo-bootloader in the
>        host to provide an interface to choose a kernel for the guest.
> -      An example is pygrub with Xen.
> +      An example is pygrub with Xen. The Bhyve hypervisor
> +      also uses a host bootloader, either bhyveload or
> +      grub-bhyve.
>      

> =20
>  
> diff --git a/po/libvirt.pot b/po/libvirt.pot
> index 0b44ad7..d8c9a4d 100644
> --- a/po/libvirt.pot
> +++ b/po/libvirt.pot
> @@ -851,6 +851,10 @@ msgstr ""
>  msgid "domain should have at least one disk defined"
>  msgstr ""
> =20
> +#: src/bhyve/bhyve_command.c:407
> +msgid "Custom loader requires explicit %s configuration"
> +msgstr ""
> +
>  #: src/bhyve/bhyve_device.c:50
>  msgid "PCI bus 0 slot 1 is reserved for the implicit LPC PCI-ISA bridge"
>  msgstr ""
> diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> index bea4a59..99956ae 100644
> --- a/src/bhyve/bhyve_command.c
> +++ b/src/bhyve/bhyve_command.c
> @@ -300,6 +300,7 @@ virBhyveProcessBuildLoadCmd(virConnectPtr conn,
>  {
>      virCommandPtr cmd;
>      virDomainDiskDefPtr disk;
> +    bool bhyveload, grub_bhyve;
> =20
>      if (def->ndisks < 1) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -326,19 +327,105 @@ virBhyveProcessBuildLoadCmd(virConnectPtr conn,
>          return NULL;
>      }
> =20
> -    cmd =3D virCommandNew(BHYVELOAD);
> +    if (def->os.bootloader =3D=3D NULL) {
> +        bhyveload =3D true;
> +        grub_bhyve =3D false;
> +        cmd =3D virCommandNew(BHYVELOAD);
> +    } else {
> +        bhyveload =3D false;
> +        if (strstr(def->os.bootloader, "grub-bhyve") =3D=3D 0)
> +            grub_bhyve =3D true;
> +        cmd =3D virCommandNew(def->os.bootloader);
> +    }
> =20
> -    /* Memory */
> -    virCommandAddArg(cmd, "-m");
> -    virCommandAddArgFormat(cmd, "%llu",
> -                           VIR_DIV_UP(def->mem.max_balloon, 1024));
> +    if (bhyveload && def->os.bootloaderArgs =3D=3D NULL) {
> +        VIR_DEBUG("%s: bhyveload with default arguments", __func__);
> +
> +        /* Memory (MB) */
> +        virCommandAddArg(cmd, "-m");
> +        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);
> +    } else if (grub_bhyve && def->os.bootloaderArgs =3D=3D NULL) {
> +        char tmpmapfile[128] =3D "/tmp/grub-bhyve-device.map_XXXXXX";
> +        FILE *f;
> +        int fd;
> +
> +        VIR_DEBUG("%s: grub-bhyve with default arguments", __func__);
> +
> +        /*
> +         * XXX Default grub-bhyve has some BIG caveats, but MAY work for=
 some
> +         * typical configurations. In particular:
> +         *
> +         *   - Can't create a new VM this way (no CD, no boot from CD)
> +         *   - Assumes a GRUB install on hd0,msdos1
> +         */
> =20
> -    /* Image path */
> -    virCommandAddArg(cmd, "-d");
> -    virCommandAddArg(cmd, virDomainDiskGetSource(disk));
> +        /* XXX cleanup this file. */
> +        fd =3D mkstemp(tmpmapfile);

This yields an error in 'syntax-check'. Running 'make syntax-check'
allows to avoid some of the general code and formatting issues.

> +        if (fd < 0) {
> +                virReportError(VIR_ERR_OPEN_FAILED, tmpmapfile);
> +                goto error;
> +        }
> +
> +        f =3D VIR_FDOPEN(fd, "wb+");
> +        if (f =3D=3D NULL) {
> +                VIR_FORCE_CLOSE(fd);
> +                virReportError(VIR_ERR_OPEN_FAILED, tmpmapfile);
> +                goto error;
> +        }
> +
> +        /* Grub device.map */
> +        fprintf(f, "(hd0) %s\n", virDomainDiskGetSource(disk));
> +        /* XXX CDs would look like: "(cd0) /path/to/CD" */
> +
> +        if (VIR_FCLOSE(f) < 0) {
> +                virReportSystemError(errno, "%s", _("failed to close fil=
e"));
> +                goto error;
> +        }
> =20
> -    /* VM name */
> -    virCommandAddArg(cmd, def->name);
> +
> +        virCommandAddArg(cmd, "--device-map");
> +        virCommandAddArg(cmd, tmpmapfile);
> +
> +        /* Memory in MB */
> +        virCommandAddArg(cmd, "--memory");
> +        virCommandAddArgFormat(cmd, "%llu",
> +                               VIR_DIV_UP(def->mem.max_balloon, 1024));
> +
> +        /* To boot from CD, "cd0" here. */
> +        virCommandAddArg(cmd, "--root");
> +        virCommandAddArg(cmd, "hd0,msdos1");
> +
> +        /* VM name */
> +        virCommandAddArg(cmd, def->name);
> +    } else if (def->os.bootloaderArgs =3D=3D NULL) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Custom loader requires explicit %s configurati=
on"),
> +                       "bootloader_args");
> +        goto error;
> +    } else {
> +        char **blargs, **arg;
> +
> +        VIR_DEBUG("%s: custom loader '%s' with arguments", __func__,
> +                  def->os.bootloader);
> +
> +        /* XXX: Handle quoted? */
> +        blargs =3D virStringSplit(def->os.bootloaderArgs, " ", 0);
> +        for (arg =3D blargs; *arg; arg++)
> +                virCommandAddArg(cmd, *arg);
> +        virStringFreeList(blargs);
> +    }
> =20
>      return cmd;
> +
> +error:
> +    virCommandFree(cmd);
> +    return NULL;
>  }
> --=20
> 1.9.3
>=20
> _______________________________________________
> freebsd-virtualization@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
> To unsubscribe, send any mail to "freebsd-virtualization-unsubscribe@free=
bsd.org"

Thanks,

Roman Bogorodskiy