On Sun, May 12, 2019 at 08:50:51PM +0200, Anton Lindqvist wrote:
> On Wed, May 08, 2019 at 01:02:10PM -0700, Mike Larkin wrote:
> > On Wed, May 08, 2019 at 09:41:42PM +0200, Anton Lindqvist wrote:
> > > On Wed, May 08, 2019 at 07:19:45PM +0200, Reyk Floeter wrote:
> > > > On Wed, May 08, 2019 at 06:47:53PM +0200, Anton Lindqvist wrote:
> > > > > Hi,
> > > > > A first stab at adding support for option `-B device' to vm.conf(5).
> > > > > With the diff below, I'm able to add a dedicated VM to be used with
> > > > > autoinstall(5):
> > > > >
> > > > > vm "amd64-install" {
> > > > > disable
> > > > > boot $snapshots "bsd.rd"
> > > > > disk $home "amd64.qcow2"
> > > > > boot-device net
> > > > > local interface
> > > > > }
> > > > >
> > > > > The name `boot-device' is of course up for debate. Also, should allow
> > > > > support
> > > > > also be considered?
> > > > >
> > > > > Comments? OK?
> > > > >
> > > >
> > > > Nice, thanks for adding this. When I did vm.conf vs vmctl, I always
> > > > liked to have vm.conf feature-complete or even to have it as a
> > > > superset of vmctl - it is just nicer to add complex options with a
> > > > grammar than command line flags (yay getsubopt(3)!).
> > > >
> > > > Could you adjust the manpage based on vmctl(8)? It is much more
> > > > verbose and why should vm.conf(5) lack the details?
> > > >
> > > > For the grammar, I think it should be done without the "-"...
> > > > - boot "bsd.foo"
> > > > - boot device net
> > > >
> > > > ...as the parser should be able to handle this. We should have made
> > > > it more flexible but I guess we don't want to change the grammar now?
> > > > - boot image "bsd.foo"
> > > > - boot device net
> > >
> > > Thanks for the feedback, new diff:
> > >
> > > * Grammar changed to `boot device' according to feedback
> > >
> > > * Manual tweaks, thanks jmc@ and sthen@
> > >
> > > * I also took a stab at incorporating more of details from the vmctl
> > > manual. This part could probably be improved...
> > >
> >
> > I like the diff, but there is a larger issue lurking ... :)
> >
> > When I shuffled the man page bits last week, I just moved wording around,
> > and
> > actually didn't notice we call out PXE in the documentation. The problem
> > here
> > is that we don't actually support PXE in -current. I have a diff in my tree
> > that fixes this but it's not committed yet. It breaks Linux guests, and
> > every
> > time I think I've nailed it, something else breaks. I had intended to commit
> > it after AsisBSDcon last month but then I ran into that bug.
> >
> > When claudio committed the "net boot" changes last year, he was pretty clear
> > what it was intended to do:
> >
> > ----------------------------
> > revision 1.7
> > date: 2018/12/06 09:20:06; author: claudio; state: Exp; lines: +7 -6;
> > commitid: 3sAzaMHdn0pTPnpR;
> > Make it possible to define the bootdevice in vmd. This information is used
> > currently only when booting a OpenBSD kernel. If VMBOOTDEV_NET is used the
> > internal dhcp server will pass "auto_install" as boot file to the client and
> > the boot loader passes the MAC of the first interface to the kernel to
> > indicate
> > PXE booting. Adding boot order support to SeaBIOS is not yet implemented.
> > Ok ccardenas@
> >
> > So what this does is sets up the bootarg page to tell the kernel that it
> > had been booted from PXE (even though it wasn't). That's used in conjunction
> > with the vmd-dhcp server to provide auto_install options automatically to
> > the VM. The fact that the term "PXE" was included in the commit may have
> > misled some people to think this actually means PXE is supported now, which
> > it is not.
> >
> > So, while the diff is good, we should probably clean up the documentation
> > to reflect what we really can and cannot do. Once I clean up iPXE and
> > (really) get it working, we can go back and rev the documentation again.
> >
> > Claudio, please correct me if I am wrong in the interpretation.
>
> Good call. Here's an attempt to document the current implementation. If
> we can agree upon something, vmctl(8) should also be updated
> accordingly.
>
Reads ok to me.
ok mlarkin
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/parse.y,v
> retrieving revision 1.51
> diff -u -p -r1.51 parse.y
> --- parse.y 11 May 2019 19:55:14 -0000 1.51
> +++ parse.y 12 May 2019 18:46:56 -0000
> @@ -120,12 +120,13 @@ typedef struct {
>
>
> %token INCLUDE ERROR
> -%token ADD ALLOW BOOT CDROM DISABLE DISK DOWN ENABLE FORMAT GROUP INET6
> -%token INSTANCE INTERFACE LLADDR LOCAL LOCKED MEMORY NIFS OWNER PATH
> PREFIX
> -%token RDOMAIN SIZE SOCKET SWITCH UP VM VMID
> +%token ADD ALLOW BOOT CDROM DEVICE DISABLE DISK DOWN ENABLE FORMAT
> GROUP
> +%token INET6 INSTANCE INTERFACE LLADDR LOCAL LOCKED MEMORY NET NIFS
> OWNER
> +%token PATH PREFIX RDOMAIN SIZE SOCKET SWITCH UP VM VMID
> %token <v.number> NUMBER
> %token <v.string> STRING
> %type <v.lladdr> lladdr
> +%type <v.number> bootdevice
> %type <v.number> disable
> %type <v.number> image_format
> %type <v.number> local
> @@ -457,6 +458,9 @@ vm_opts : disable {
> }
> vmc.vmc_flags |= VMOP_CREATE_KERNEL;
> }
> + | BOOT DEVICE bootdevice {
> + vmc.vmc_bootdevice = $3;
> + }
> | CDROM string {
> if (vcp->vcp_cdrom[0] != '\0') {
> yyerror("cdrom specified more than once");
> @@ -704,6 +708,11 @@ disable : ENABLE { $$ =
> 0; }
> | DISABLE { $$ = 1; }
> ;
>
> +bootdevice : CDROM { $$ = VMBOOTDEV_CDROM; }
> + | DISK { $$ = VMBOOTDEV_DISK; }
> + | NET { $$ = VMBOOTDEV_NET; }
> + ;
> +
> optcomma : ','
> |
> ;
> @@ -757,6 +766,7 @@ lookup(char *s)
> { "allow", ALLOW },
> { "boot", BOOT },
> { "cdrom", CDROM },
> + { "device", DEVICE },
> { "disable", DISABLE },
> { "disk", DISK },
> { "down", DOWN },
> @@ -773,6 +783,7 @@ lookup(char *s)
> { "local", LOCAL },
> { "locked", LOCKED },
> { "memory", MEMORY },
> + { "net", NET },
> { "owner", OWNER },
> { "prefix", PREFIX },
> { "rdomain", RDOMAIN },
> Index: vm.conf.5
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/vm.conf.5,v
> retrieving revision 1.42
> diff -u -p -r1.42 vm.conf.5
> --- vm.conf.5 7 Mar 2019 18:54:06 -0000 1.42
> +++ vm.conf.5 12 May 2019 18:46:56 -0000
> @@ -144,6 +144,37 @@ See
> Kernel or BIOS image to load when booting the VM.
> If not specified, the default is to boot using the BIOS image in
> .Pa /etc/firmware/vmm-bios .
> +.It Cm boot device Ar device
> +Force VM to boot from
> +.Ar device .
> +Valid values are:
> +.Bl -tag -width "cdrom"
> +.It Ar cdrom
> +Boot the ISO image file specified using the
> +.Ic cdrom
> +parameter.
> +.It Ar disk
> +Boot from the disk image file specified using the
> +.Ic disk
> +parameter.
> +.It Ar net
> +Boot the kernel specified using the
> +.Ic boot
> +parameter as if the VM was network booted.
> +In addition, the DHCP lease will advertise
> +.Dq auto_install
> +in the bootfile option making it suitable for use with
> +.Xr autoinstall 8 .
> +Note, this not to be confused with
> +.Xr pxeboot 8
> +but rather a simulated network boot.
> +.El
> +.Pp
> +Currently
> +.Ar disk
> +and
> +.Ar cdrom
> +only work with VMs booted using BIOS.
> .It Cm cdrom Ar path
> ISO image file.
> .It Cm enable
>