Re: patch - cgdconfig do_all mode gracefully ignore detached devices

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Sun, 10 Nov 2019 00:52:46 +0000
    From:        Jason High <jhigh%netbsd.org@localhost>
    Message-ID:  <20191110005246.GA29989%homeworld.netbsd.org@localhost>

  | When using cgdconfig in do_all mode (-C|-U), it will attempt to
  | configure/unconfigure all devices specified in cgd.conf regardless of
  | whether the device is attached or not

I can see the utility of this change for -C (though it should be enabled
by another option, so people who don't expect unattached devices get errors
when it happens, rather than simply having the cgdconfig -C silently do
nothing), but I cannot imagione its utility with -U - if the device has
been previously configured, then we should be able to unconfigure it (even
if it was detached in the meantime).   (If the -U code does not test that the
device has been configured, before attempting to unconfigure it, then
that is what ought to be fixed.)

Apart from that, try the following for your verify_attached() function.
This version does the sysctl stuff just once, rather than repeating it
for every device to be verified, doesn't duplicate the path arg, and
returns "attached" instead of "not attached" in case of a "should not
happen" type error occurring (so when it does, if it ever does, the
rest of the code won't simply silently ignore everything).   An alternative
would be to simply exit(1) if any of those errors happens (or just use
err() instead of perror()).

Note: this code is 100% illusory, it hasn't even been near a compiler,
let alone tested, so caveat emptor.   It is a little closer to NetBSD KNF
though.

kre

/*
 * check if opath backing device is attached
 * returns 1 if backing device is attached
 * otherwise returns 0
 */
static int 
verify_attached(const char *path)
{
	static const int mib[2] = { CTL_HW, HW_DISKNAMES };
	static char *devices = NULL;
	static int max = 0;
	char *dev;
	char *b;
	size_t len;

	if (path == NULL)
		return 0;

	if (devices == NULL) {
		/*
		 * get attached disk devices
		 *
		 * In case of failure here (shouldn't ever happen)
		 * indicate that the device is attached, rather than
		 * is not, so an attempt will be made to config it,
		 * rather than simply skipping everything.
		 */

		/* find out how much space we need first */
		if (sysctl(mib, 2, NULL, &len, NULL, 0) == -1) {
			perror("sysctl");
			return 1;
		}
		devices = (char *)calloc(len, sizeof(char));
		if (devices == NULL) {
			perror("calloc");
			return 1;
		}
		/* then get attached disk device (driver) names */
		if (sysctl(mib, 2, devices, &len, NULL, 0) == -1) {
			perror("sysctl");
			return 1;
		}	

		max = getmaxpartitions();
		if (max == -1) {
			perror("getmaxpartitions");
			free(devices);
			devices = NULL;
			return 1;
		}
		max += 'a';
	}

	if ((b = strrchr(path, '/')) != NULL)
		b++;
	else
		b = path;

	len = strlen(b);

	/* if suffixed with parition ident, truncate */
	if (b[len - 1] >= 'a' && b[len - 1] <= max)
		len--;

	/* iterate over device list for match */
	for (dev = devices; *dev != '\0'; ) {
		if (strncmp(dev, b, len) == 0 &&
		    (dev[len] == '\0' || dev[len] == ' '))
			return 1;
		if ((dev = strchr(dev, ' ')) == NULL)
			break;
		while (*dev == ' ')	/* XXX: loop probably not needed */
			dev++;		/* but this is, once at least */
	}
	return 0;
}