Subject: config(8) -- attributes with dependencies
To: None <tech-kern@netbsd.org>
From: Jason R Thorpe <thorpej@wasabisystems.com>
List: tech-kern
Date: 10/09/2002 13:11:17
--4Ckj6UjgE2iN1+kY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Right now, attributes cannot have other attributes.  At first, this
kind of makes sense, but it would be nice to say, for example, that
an interface attribute itself has other attributes (more accurately,
depencies).

Where I discovered the need for this was in the SCSI code.  The
SCSI code is pulled in if the "scsi" attribute is selected.  But
the "scsibus" code doesn't carry the "scsi" attribute -- it attaches
to it.  So, a kernel with "scsibus" is able to link as a side-effect
of a SCSI HBA driver also being in the kernel.

Or more natural way to specify this is to define a "scsi_core"
attribute, which, when selected, pulls in the core SCSI code.
But now for a SCSI HBA driver to select that code, you need to
either say:

dev/scsipi/scsi_base.c		scsi | scsi_core

...or you need to say that "scsi depends on scsi_core".

I have implemented support for the latter :-)  This allows attributes
to express dependencies on other attributes (so long as none of the
dependencies is an interface attribute -- that is not allowed).  This
causes the dependencies of any selected attribute to also be selected.

Attached is the change to config, and also a change to conf/files
and dev/scsipi/files.scsipi which uses this change.

Note that while I did add a simple check for circular dependencies,
I don't think it's actually possible to create one using the current
config grammar, which is good :-)

Anyway, with this change, I can make another trivial change (which
I have already implemented, actually :-) to config(8) to allow devices
to be orphaned in the config file... I will post about that separately.

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

--4Ckj6UjgE2iN1+kY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=config-patch

Index: defs.h
===================================================================
RCS file: /cvsroot/syssrc/usr.sbin/config/defs.h,v
retrieving revision 1.7
diff -c -r1.7 defs.h
*** defs.h	2002/09/26 04:07:36	1.7
--- defs.h	2002/10/09 15:52:18
***************
*** 153,158 ****
--- 153,160 ----
  	int	a_loclen;		/* length of above list */
  	struct	nvlist *a_devs;		/* children */
  	struct	nvlist *a_refs;		/* parents */
+ 	struct	nvlist *a_deps;		/* we depend on these other attrs */
+ 	int	a_expanding;		/* to detect cycles in attr graph */
  };
  
  /*
Index: gram.y
===================================================================
RCS file: /cvsroot/syssrc/usr.sbin/config/gram.y,v
retrieving revision 1.36
diff -c -r1.36 gram.y
*** gram.y	2002/09/11 06:20:09	1.36
--- gram.y	2002/10/09 15:52:18
***************
*** 266,274 ****
  	device_major			{ do_devsw = 1; } |
  	include |
  	prefix |
! 	DEVCLASS WORD			{ (void)defattr($2, NULL, 1); } |
  	DEFFS fsoptfile_opt deffses	{ deffilesystem($2, $3); } |
! 	DEFINE WORD interface_opt	{ (void)defattr($2, $3, 0); } |
  	DEFOPT optfile_opt defopts defoptdeps
  					{ defoption($2, $3, $4); } |
  	DEFFLAG optfile_opt defopts defoptdeps
--- 266,275 ----
  	device_major			{ do_devsw = 1; } |
  	include |
  	prefix |
! 	DEVCLASS WORD			{ (void)defattr($2, NULL, NULL, 1); } |
  	DEFFS fsoptfile_opt deffses	{ deffilesystem($2, $3); } |
! 	DEFINE WORD interface_opt attrs_opt
! 					{ (void)defattr($2, $3, $4, 0); } |
  	DEFOPT optfile_opt defopts defoptdeps
  					{ defoption($2, $3, $4); } |
  	DEFFLAG optfile_opt defopts defoptdeps
Index: sem.c
===================================================================
RCS file: /cvsroot/syssrc/usr.sbin/config/sem.c,v
retrieving revision 1.33
diff -c -r1.33 sem.c
*** sem.c	2002/09/26 04:07:36	1.33
--- sem.c	2002/10/09 15:52:21
***************
*** 180,199 ****
  }
  
  /*
!  * Define an attribute, optionally with an interface (a locator list).
   * Since an empty locator list is logically different from "no interface",
   * all locator lists include a dummy head node, which we discard here.
   */
  int
! defattr(const char *name, struct nvlist *locs, int devclass)
  {
! 	struct attr *a;
  	struct nvlist *nv;
  	int len;
  
  	if (locs != NULL && devclass)
  		panic("defattr(%s): locators and devclass", name);
  
  	a = emalloc(sizeof *a);
  	if (ht_insert(attrtab, name, a)) {
  		free(a);
--- 180,220 ----
  }
  
  /*
!  * Define an attribute, optionally with an interface (a locator list)
!  * and a set of attribute-dependencies.
!  *
!  * Attribute dependencies MAY NOT be interface attributes.
!  *
   * Since an empty locator list is logically different from "no interface",
   * all locator lists include a dummy head node, which we discard here.
   */
  int
! defattr(const char *name, struct nvlist *locs, struct nvlist *deps,
!     int devclass)
  {
! 	struct attr *a, *dep;
  	struct nvlist *nv;
  	int len;
  
  	if (locs != NULL && devclass)
  		panic("defattr(%s): locators and devclass", name);
  
+ 	if (deps != NULL && devclass)
+ 		panic("defattr(%s): dependencies and devclass", name);
+ 
+ 	/*
+ 	 * If this attribute depends on any others, make sure none of
+ 	 * the dependencies are interface attributes.
+ 	 */
+ 	for (nv = deps; nv != NULL; nv = nv->nv_next) {
+ 		dep = nv->nv_ptr;
+ 		if (dep->a_iattr) {
+ 			error("`%s' dependency `%s' is an interface attribute",
+ 			    name, dep->a_name);
+ 			return (1);
+ 		}
+ 	}
+ 
  	a = emalloc(sizeof *a);
  	if (ht_insert(attrtab, name, a)) {
  		free(a);
***************
*** 235,240 ****
--- 256,267 ----
  	a->a_loclen = len;
  	a->a_devs = NULL;
  	a->a_refs = NULL;
+ 	a->a_deps = deps;
+ 	a->a_expanding = 0;
+ 
+ 	/* Expand the attribute to check for cycles in the graph. */
+ 	expandattr(a, NULL);
+ 
  	return (0);
  }
  
***************
*** 315,321 ****
  	if (loclist != NULL) {
  		nv = loclist;
  		loclist = NULL;	/* defattr disposes of them for us */
! 		if (defattr(dev->d_name, nv, 0))
  			goto bad;
  		attrs = newnv(dev->d_name, NULL, getattr(dev->d_name), 0,
  		    attrs);
--- 342,348 ----
  	if (loclist != NULL) {
  		nv = loclist;
  		loclist = NULL;	/* defattr disposes of them for us */
! 		if (defattr(dev->d_name, nv, NULL, 0))
  			goto bad;
  		attrs = newnv(dev->d_name, NULL, getattr(dev->d_name), 0,
  		    attrs);
***************
*** 540,545 ****
--- 567,602 ----
  }
  
  /*
+  * Recursively expand an attribute and its dependencies, checking for
+  * cycles, and invoking a callback for each attribute found.
+  */
+ void
+ expandattr(struct attr *a, void (*callback)(struct attr *))
+ {
+ 	struct nvlist *nv;
+ 	struct attr *dep;
+ 
+ 	if (a->a_expanding) {
+ 		error("circular dependency on attribute `%s'", a->a_name);
+ 		return;
+ 	}
+ 
+ 	a->a_expanding = 1;
+ 
+ 	/* First expand all of this attribute's dependencies. */
+ 	for (nv = a->a_deps; nv != NULL; nv = nv->nv_next) {
+ 		dep = nv->nv_ptr;
+ 		expandattr(dep, callback);
+ 	}
+ 
+ 	/* ...and now invoke the callback for ourself. */
+ 	if (callback != NULL)
+ 		(*callback)(a);
+ 
+ 	a->a_expanding = 0;
+ }
+ 
+ /*
   * Set the major device number for a device, so that it can be used
   * as a root/dumps "on" device in a configuration.
   */
***************
*** 1230,1235 ****
--- 1287,1299 ----
  	return (0);
  }
  
+ static void
+ selectattr(struct attr *a)
+ {
+ 
+ 	(void)ht_insert(selecttab, a->a_name, (char *)a->a_name);
+ }
+ 
  /*
   * We have an instance of the base foo, so select it and all its
   * attributes for "optional foo".
***************
*** 1243,1256 ****
  	(void)ht_insert(selecttab, d->d_name, (char *)d->d_name);
  	for (nv = d->d_attrs; nv != NULL; nv = nv->nv_next) {
  		a = nv->nv_ptr;
! 		(void)ht_insert(selecttab, a->a_name, (char *)a->a_name);
  	}
  	if (da != NULL) {
  		(void)ht_insert(selecttab, da->d_name, (char *)da->d_name);
  		for (nv = da->d_attrs; nv != NULL; nv = nv->nv_next) {
  			a = nv->nv_ptr;
! 			(void)ht_insert(selecttab, a->a_name,
! 			    (char *)a->a_name);
  		}
  	}
  }
--- 1307,1319 ----
  	(void)ht_insert(selecttab, d->d_name, (char *)d->d_name);
  	for (nv = d->d_attrs; nv != NULL; nv = nv->nv_next) {
  		a = nv->nv_ptr;
! 		expandattr(a, selectattr);
  	}
  	if (da != NULL) {
  		(void)ht_insert(selecttab, da->d_name, (char *)da->d_name);
  		for (nv = da->d_attrs; nv != NULL; nv = nv->nv_next) {
  			a = nv->nv_ptr;
! 			expandattr(a, selectattr);
  		}
  	}
  }
Index: sem.h
===================================================================
RCS file: /cvsroot/syssrc/usr.sbin/config/sem.h,v
retrieving revision 1.15
diff -c -r1.15 sem.h
*** sem.h	2002/09/06 13:24:19	1.15
--- sem.h	2002/10/09 15:52:21
***************
*** 49,61 ****
  void		setdefmaxusers(int, int, int);
  void		setmaxusers(int);
  void		setident(const char *);
! int		defattr(const char *, struct nvlist *, int);
  void		defdev(struct devbase *, struct nvlist *, struct nvlist *, int);
  void		defdevattach(struct deva *, struct devbase *, struct nvlist *,
  			     struct nvlist *);
  struct devbase *getdevbase(const char *);
  struct deva    *getdevattach(const char *);
  struct attr    *getattr(const char *);
  void		setmajor(struct devbase *, int);
  void		addconf(struct config *);
  void		setconf(struct nvlist **, const char *, struct nvlist *);
--- 49,62 ----
  void		setdefmaxusers(int, int, int);
  void		setmaxusers(int);
  void		setident(const char *);
! int		defattr(const char *, struct nvlist *, struct nvlist *, int);
  void		defdev(struct devbase *, struct nvlist *, struct nvlist *, int);
  void		defdevattach(struct deva *, struct devbase *, struct nvlist *,
  			     struct nvlist *);
  struct devbase *getdevbase(const char *);
  struct deva    *getdevattach(const char *);
  struct attr    *getattr(const char *);
+ void		expandattr(struct attr *, void (*)(struct attr *));
  void		setmajor(struct devbase *, int);
  void		addconf(struct config *);
  void		setconf(struct nvlist **, const char *, struct nvlist *);

--4Ckj6UjgE2iN1+kY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=files-patch

Index: conf/files
===================================================================
RCS file: /cvsroot/syssrc/sys/conf/files,v
retrieving revision 1.557
diff -c -r1.557 files
*** conf/files	2002/10/05 15:16:10	1.557
--- conf/files	2002/10/09 16:00:49
***************
*** 164,170 ****
  define	midibus		{ }
  define	midisyn
  define	wdc_base
! define	scsi		{[channel = -1]}
  define	ata		{[channel = -1], [drive = -1]}
  define	atapi		{[channel = -1]}
  define	irbus		{ }
--- 164,171 ----
  define	midibus		{ }
  define	midisyn
  define	wdc_base
! define	scsi_core
! define	scsi		{[channel = -1]}: scsi_core
  define	ata		{[channel = -1], [drive = -1]}
  define	atapi		{[channel = -1]}
  define	irbus		{ }
Index: dev/scsipi/files.scsipi
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/scsipi/files.scsipi,v
retrieving revision 1.32
diff -c -r1.32 files.scsipi
*** dev/scsipi/files.scsipi	2002/05/16 02:59:57	1.32
--- dev/scsipi/files.scsipi	2002/10/09 16:00:49
***************
*** 14,28 ****
  				SCSIPI_DEBUG_LUN
  				SCSIPI_DEBUG_FLAGS
  
! file	dev/scsipi/scsipiconf.c		scsi | atapibus
! file	dev/scsipi/scsipi_base.c	scsi | atapibus
! file	dev/scsipi/scsipi_ioctl.c	scsi | atapibus
! file	dev/scsipi/scsipi_verbose.c	(scsi | atapibus) & scsiverbose
! file	dev/scsipi/scsi_base.c		scsi 
  file	dev/scsipi/atapi_base.c		atapibus
  file	dev/scsipi/atapi_wdc.c		atapibus & wdc_base
  
! device	scsibus {target = -1, lun = -1}
  attach	scsibus at scsi
  file	dev/scsipi/scsiconf.c		scsibus			needs-flag
  
--- 14,28 ----
  				SCSIPI_DEBUG_LUN
  				SCSIPI_DEBUG_FLAGS
  
! file	dev/scsipi/scsipiconf.c		scsi_core | atapibus
! file	dev/scsipi/scsipi_base.c	scsi_core | atapibus
! file	dev/scsipi/scsipi_ioctl.c	scsi_core | atapibus
! file	dev/scsipi/scsipi_verbose.c	(scsi_core | atapibus) & scsiverbose
! file	dev/scsipi/scsi_base.c		scsi_core 
  file	dev/scsipi/atapi_base.c		atapibus
  file	dev/scsipi/atapi_wdc.c		atapibus & wdc_base
  
! device	scsibus {target = -1, lun = -1}: scsi_core
  attach	scsibus at scsi
  file	dev/scsipi/scsiconf.c		scsibus			needs-flag
  

--4Ckj6UjgE2iN1+kY--