Re: [PATCH 1/2 v3] VMD: remove add from switch configuration

2017-11-10 Thread Mike Larkin
On Wed, Nov 08, 2017 at 06:49:30PM -0800, Carlos Cardenas wrote:
> Remove configuration items on switches for:
> * adding static interfaces
> 
> Adding static interfaces are to be set in hostname.if.
> 
> Changed rule on rdomain:
> * vm->interface->rdomain takes precedence over sw->rdomain
> 
> Update examples/vm.conf and vm.conf manpage to reflect changes.
> 
> Comments? Ok?
> 
> Ok by reyk@
> 

committed, thanks!

-ml

> diff --git etc/examples/vm.conf etc/examples/vm.conf
> index f35235e3fba..d61ce8c4977 100644
> --- etc/examples/vm.conf
> +++ etc/examples/vm.conf
> @@ -10,15 +10,14 @@ sets="/var/www/htdocs/pub/OpenBSD/snapshots/amd64/"
>  #
>  
>  switch "uplink" {
> - # This interface will default to bridge0, but switch(4) is supported
> + # This switch will use bridge0, defined by /etc/hostname.bridge0, as
> + # the underlying interface.  switch(4) is also supported
>   #interface switch0
> -
> - # Add additional members
> - add em0
> + interface bridge0
>  }
>  
>  switch "local" {
> - add vether0
> + interface bridge1
>   down
>  }
>  
> diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
> index a0e96545923..18543a74911 100644
> --- usr.sbin/vmd/parse.y
> +++ usr.sbin/vmd/parse.y
> @@ -88,7 +88,6 @@ int  parse_disk(char *);
>  static struct vmop_create_params vmc;
>  static struct vm_create_params   *vcp;
>  static struct vmd_switch *vsw;
> -static struct vmd_if *vif;
>  static struct vmd_vm *vm;
>  static char   vsw_type[IF_NAMESIZE];
>  static intvcp_disable;
> @@ -193,7 +192,6 @@ switch: SWITCH string {
>   vsw->sw_id = env->vmd_nswitches + 1;
>   vsw->sw_name = $2;
>   vsw->sw_flags = VMIFF_UP;
> - TAILQ_INIT(>sw_ifs);
>  
>   vcp_disable = 0;
>   } '{' optnl switch_opts_l '}'   {
> @@ -224,21 +222,6 @@ switch_opts_l: switch_opts_l switch_opts nl
>  switch_opts  : disable   {
>   vcp_disable = $1;
>   }
> - | ADD string{
> - chartype[IF_NAMESIZE];
> -
> - if ((vif = calloc(1, sizeof(*vif))) == NULL)
> - fatal("could not allocate interface");
> -
> - if (priv_getiftype($2, type, NULL) == -1) {
> - yyerror("invalid interface: %s", $2);
> - free($2);
> - YYERROR;
> - }
> - vif->vif_name = $2;
> -
> - TAILQ_INSERT_TAIL(>sw_ifs, vif, vif_entry);
> - }
>   | GROUP string  {
>   if (priv_validgroup($2) == -1) {
>   yyerror("invalid group name: %s", $2);
> diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
> index d585bf75a99..c1d0c0e13be 100644
> --- usr.sbin/vmd/priv.c
> +++ usr.sbin/vmd/priv.c
> @@ -255,9 +255,18 @@ priv_validgroup(const char *name)
>  }
>  
>  /*
> - * Called from the process peer
> + * Called from the Parent process to setup vm interface(s)
> + * - ensure the interface has the description set (tracking purposes)
> + * - if interface is to be attached to a switch, attach it
> + * - check if rdomain is set on interface and switch
> + *   - if interface only or both, use interface rdomain
> + *   - if switch only, use switch rdomain
> + * - check if group is set on interface and switch
> + *   - if interface, add it
> + *   - if switch, add it
> + * - ensure the interface is up/down
> + * - if local interface, set address
>   */
> -
>  int
>  vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
>  {
> @@ -279,18 +288,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
>   sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name))
>   return (-1);
>  
> - /* Use the configured rdomain or get it from the process */
> - if (vif->vif_flags & VMIFF_RDOMAIN)
> - vfr.vfr_id = vif->vif_rdomain;
> - else
> - vfr.vfr_id = getrtable();
> - if (vfr.vfr_id != 0)
> - log_debug("%s: interface %s rdomain %u", __func__,
> - vfr.vfr_name, vfr.vfr_id);
> -
> - proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN,
> - , sizeof(vfr));
> -
>   /* Description can be truncated */
>   (void)snprintf(vfr.vfr_value, sizeof(vfr.vfr_value),
>   "vm%u-if%u-%s", vm->vm_vmid, i, vcp->vcp_name);
> @@ -301,8 +298,17 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
>   proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESCR,
>   , sizeof(vfr));
>  
> - /* Add interface to 

[PATCH 1/2 v3] VMD: remove add from switch configuration

2017-11-08 Thread Carlos Cardenas
Remove configuration items on switches for:
* adding static interfaces

Adding static interfaces are to be set in hostname.if.

Changed rule on rdomain:
* vm->interface->rdomain takes precedence over sw->rdomain

Update examples/vm.conf and vm.conf manpage to reflect changes.

Comments? Ok?

Ok by reyk@

diff --git etc/examples/vm.conf etc/examples/vm.conf
index f35235e3fba..d61ce8c4977 100644
--- etc/examples/vm.conf
+++ etc/examples/vm.conf
@@ -10,15 +10,14 @@ sets="/var/www/htdocs/pub/OpenBSD/snapshots/amd64/"
 #
 
 switch "uplink" {
-   # This interface will default to bridge0, but switch(4) is supported
+   # This switch will use bridge0, defined by /etc/hostname.bridge0, as
+   # the underlying interface.  switch(4) is also supported
#interface switch0
-
-   # Add additional members
-   add em0
+   interface bridge0
 }
 
 switch "local" {
-   add vether0
+   interface bridge1
down
 }
 
diff --git usr.sbin/vmd/parse.y usr.sbin/vmd/parse.y
index a0e96545923..18543a74911 100644
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -88,7 +88,6 @@ intparse_disk(char *);
 static struct vmop_create_params vmc;
 static struct vm_create_params *vcp;
 static struct vmd_switch   *vsw;
-static struct vmd_if   *vif;
 static struct vmd_vm   *vm;
 static char vsw_type[IF_NAMESIZE];
 static int  vcp_disable;
@@ -193,7 +192,6 @@ switch  : SWITCH string {
vsw->sw_id = env->vmd_nswitches + 1;
vsw->sw_name = $2;
vsw->sw_flags = VMIFF_UP;
-   TAILQ_INIT(>sw_ifs);
 
vcp_disable = 0;
} '{' optnl switch_opts_l '}'   {
@@ -224,21 +222,6 @@ switch_opts_l  : switch_opts_l switch_opts nl
 switch_opts: disable   {
vcp_disable = $1;
}
-   | ADD string{
-   chartype[IF_NAMESIZE];
-
-   if ((vif = calloc(1, sizeof(*vif))) == NULL)
-   fatal("could not allocate interface");
-
-   if (priv_getiftype($2, type, NULL) == -1) {
-   yyerror("invalid interface: %s", $2);
-   free($2);
-   YYERROR;
-   }
-   vif->vif_name = $2;
-
-   TAILQ_INSERT_TAIL(>sw_ifs, vif, vif_entry);
-   }
| GROUP string  {
if (priv_validgroup($2) == -1) {
yyerror("invalid group name: %s", $2);
diff --git usr.sbin/vmd/priv.c usr.sbin/vmd/priv.c
index d585bf75a99..c1d0c0e13be 100644
--- usr.sbin/vmd/priv.c
+++ usr.sbin/vmd/priv.c
@@ -255,9 +255,18 @@ priv_validgroup(const char *name)
 }
 
 /*
- * Called from the process peer
+ * Called from the Parent process to setup vm interface(s)
+ * - ensure the interface has the description set (tracking purposes)
+ * - if interface is to be attached to a switch, attach it
+ * - check if rdomain is set on interface and switch
+ *   - if interface only or both, use interface rdomain
+ *   - if switch only, use switch rdomain
+ * - check if group is set on interface and switch
+ *   - if interface, add it
+ *   - if switch, add it
+ * - ensure the interface is up/down
+ * - if local interface, set address
  */
-
 int
 vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
 {
@@ -279,18 +288,6 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
sizeof(vfr.vfr_name)) >= sizeof(vfr.vfr_name))
return (-1);
 
-   /* Use the configured rdomain or get it from the process */
-   if (vif->vif_flags & VMIFF_RDOMAIN)
-   vfr.vfr_id = vif->vif_rdomain;
-   else
-   vfr.vfr_id = getrtable();
-   if (vfr.vfr_id != 0)
-   log_debug("%s: interface %s rdomain %u", __func__,
-   vfr.vfr_name, vfr.vfr_id);
-
-   proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFRDOMAIN,
-   , sizeof(vfr));
-
/* Description can be truncated */
(void)snprintf(vfr.vfr_value, sizeof(vfr.vfr_value),
"vm%u-if%u-%s", vm->vm_vmid, i, vcp->vcp_name);
@@ -301,8 +298,17 @@ vm_priv_ifconfig(struct privsep *ps, struct vmd_vm *vm)
proc_compose(ps, PROC_PRIV, IMSG_VMDOP_PRIV_IFDESCR,
, sizeof(vfr));
 
-   /* Add interface to bridge/switch */
-   if ((vsw = switch_getbyname(vif->vif_switch)) != NULL) {
+   /* set default rdomain */
+   vfr.vfr_id = getrtable();
+
+   vsw = switch_getbyname(vif->vif_switch);
+
+