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.

-ml

> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/parse.y,v
> retrieving revision 1.50
> diff -u -p -r1.50 parse.y
> --- parse.y   13 Feb 2019 22:57:08 -0000      1.50
> +++ parse.y   8 May 2019 19:36:04 -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
> @@ -456,6 +457,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");
> @@ -703,6 +707,11 @@ disable          : ENABLE                        { $$ = 
> 0; }
>               | DISABLE                       { $$ = 1; }
>               ;
>  
> +bootdevice   : CDROM                         { $$ = VMBOOTDEV_CDROM; }
> +             | DISK                          { $$ = VMBOOTDEV_DISK; }
> +             | NET                           { $$ = VMBOOTDEV_NET; }
> +             ;
> +
>  optcomma     : ','
>               |
>               ;
> @@ -756,6 +765,7 @@ lookup(char *s)
>               { "allow",              ALLOW },
>               { "boot",               BOOT },
>               { "cdrom",              CDROM },
> +             { "device",             DEVICE },
>               { "disable",            DISABLE },
>               { "disk",               DISK },
>               { "down",               DOWN },
> @@ -772,6 +782,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 8 May 2019 19:36:04 -0000
> @@ -144,6 +144,34 @@ 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
> +Perform a PXE boot using the first network interface specified using the
> +.Ic interface
> +parameter.
> +.El
> +.Pp
> +Currently
> +.Ar net
> +is only supported when booting a kernel specified using
> +.Ic boot
> +while
> +.Ar disk
> +and
> +.Ar cdrom
> +only work with VMs booted using BIOS.
>  .It Cm cdrom Ar path
>  ISO image file.
>  .It Cm enable
> 

Reply via email to