On 26/10/17(Thu) 16:23, Carlos Cardenas wrote: > * Require interface name to be defined for switches in vm.conf > ** Requires user to create bridge(4) beforehand > * Remove code to create bridges on the fly > * Add code to ensure bridge really exists > * Update manpage switch and example sections
What's the motivation to ask people to create a bridge(4) beforehand? > diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y > index 55a9b0c7acc..a0b21fa8ef1 100644 > --- usr.sbin/vmd/parse.y > +++ usr.sbin/vmd/parse.y > @@ -90,7 +90,6 @@ static struct vm_create_params *vcp; > static struct vmd_switch *vsw; > static struct vmd_if *vif; > static struct vmd_vm *vm; > -static unsigned int vsw_unit; > static char vsw_type[IF_NAMESIZE]; > static int vcp_disable; > static size_t vcp_nnics; > @@ -194,12 +193,17 @@ switch : SWITCH string { > vsw->sw_id = env->vmd_nswitches + 1; > vsw->sw_name = $2; > vsw->sw_flags = VMIFF_UP; > - snprintf(vsw->sw_ifname, sizeof(vsw->sw_ifname), > - "%s%u", vsw_type, vsw_unit++); > TAILQ_INIT(&vsw->sw_ifs); > > vcp_disable = 0; > } '{' optnl switch_opts_l '}' { > + if (strnlen(vsw->sw_ifname, > + sizeof(vsw->sw_ifname)) == 0) { > + yyerror("switch \"%s\" is missing interface > name", > + vsw->sw_name); > + YYERROR; > + } > + > if (vcp_disable) { > log_debug("%s:%d: switch \"%s\"" > " skipped (disabled)", > @@ -244,13 +248,12 @@ switch_opts : disable { > vsw->sw_group = $2; > } > | INTERFACE string { > - if (priv_getiftype($2, vsw_type, &vsw_unit) == -1 || > + if (priv_getiftype($2, vsw_type, NULL) == -1 || > priv_findname(vsw_type, vmd_descsw) == -1) { > yyerror("invalid switch interface: %s", $2); > free($2); > YYERROR; > } > - vsw_unit++; > > if (strlcpy(vsw->sw_ifname, $2, > sizeof(vsw->sw_ifname)) >= sizeof(vsw->sw_ifname)) { > diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c > index ef42549d105..d66bdcc9b4f 100644 > --- usr.sbin/vmd/priv.c > +++ usr.sbin/vmd/priv.c > @@ -87,8 +87,8 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, struct > imsg *imsg) > > switch (imsg->hdr.type) { > case IMSG_VMDOP_PRIV_IFDESCR: > - case IMSG_VMDOP_PRIV_IFCREATE: > case IMSG_VMDOP_PRIV_IFRDOMAIN: > + case IMSG_VMDOP_PRIV_IFEXISTS: > case IMSG_VMDOP_PRIV_IFADD: > case IMSG_VMDOP_PRIV_IFUP: > case IMSG_VMDOP_PRIV_IFDOWN: > @@ -118,13 +118,6 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, > struct imsg *imsg) > if (ioctl(env->vmd_fd, SIOCSIFDESCR, &ifr) < 0) > log_warn("SIOCSIFDESCR"); > break; > - case IMSG_VMDOP_PRIV_IFCREATE: > - /* Create the bridge if it doesn't exist */ > - strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name)); > - if (ioctl(env->vmd_fd, SIOCIFCREATE, &ifr) < 0 && > - errno != EEXIST) > - log_warn("SIOCIFCREATE"); > - break; > case IMSG_VMDOP_PRIV_IFRDOMAIN: > strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name)); > ifr.ifr_rdomainid = vfr.vfr_id; > @@ -145,6 +138,13 @@ priv_dispatch_parent(int fd, struct privsep_proc *p, > struct imsg *imsg) > errno != EEXIST) > log_warn("SIOCBRDGADD"); > break; > + case IMSG_VMDOP_PRIV_IFEXISTS: > + /* Determine if bridge/switch exists */ > + strlcpy(ifr.ifr_name, vfr.vfr_name, sizeof(ifr.ifr_name)); > + if (ioctl(env->vmd_fd, SIOCGIFFLAGS, &ifr) < 0) > + fatalx("%s: bridge \"%s\" does not exist", > + __func__, vfr.vfr_name); > + break; > case IMSG_VMDOP_PRIV_IFUP: > case IMSG_VMDOP_PRIV_IFDOWN: > /* Set the interface status */ > @@ -319,10 +319,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm) > log_debug("%s: interface %s add %s", __func__, > vfbr.vfr_name, vfbr.vfr_value); > > - proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFCREATE, > - &vfbr, sizeof(vfbr)); > - proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN, > - &vfbr, sizeof(vfbr)); > proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFADD, > &vfbr, sizeof(vfbr)); > } else if (vif->vif_switch != NULL) > @@ -398,7 +394,8 @@ vm_priv_brconfig(struct privsep *ps, struct vmd_switch > *vsw) > sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name)) > return (-1); > > - proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFCREATE, > + // ensure bridge/switch exists > + proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFEXISTS, > &vfr, sizeof(vfr)); > > /* Use the configured rdomain or get it from the process */ > diff --git usr.sbin/vmd/vm.conf.5 usr.sbin/vmd/vm.conf.5 > index 0927fbfecf9..f4bfc2f215b 100644 > --- usr.sbin/vmd/vm.conf.5 > +++ usr.sbin/vmd/vm.conf.5 > @@ -279,11 +279,6 @@ Set the > or > .Xr bridge 4 > network interface of this switch. > -If not specified, > -.Ar bridge0 > -will be used where the interface unit will be incremented for each switch, > -e.g.\& > -.Ar bridge0 , bridge1 , ... > If the type is changed to > .Ar switch0 , > it will be used for each following switch. > @@ -318,6 +313,7 @@ vm "vm2.example.com" { > Create the switch "uplink" with an additional physical network interface: > .Bd -literal -offset indent > switch "uplink" { > + interface bridge0 > add em0 > } > .Ed > diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h > index 4b7b5f70495..e02c2ea7882 100644 > --- usr.sbin/vmd/vmd.h > +++ usr.sbin/vmd/vmd.h > @@ -94,7 +94,7 @@ enum imsg_type { > IMSG_VMDOP_RELOAD, > IMSG_VMDOP_PRIV_IFDESCR, > IMSG_VMDOP_PRIV_IFADD, > - IMSG_VMDOP_PRIV_IFCREATE, > + IMSG_VMDOP_PRIV_IFEXISTS, > IMSG_VMDOP_PRIV_IFUP, > IMSG_VMDOP_PRIV_IFDOWN, > IMSG_VMDOP_PRIV_IFGROUP, > -- > 2.14.2 >