CVS log for src/sbin/raidctl/rf_configure.c

Up to [cvs.NetBSD.org] / src / sbin / raidctl

Request diff between arbitrary revisions

Keyword substitution: kv
Default branch: MAIN

PR bin/56917

If getfsspecname() fails that will usually mean that a NAME=wedge or
ROOT.x partition is unabailable.   raidframe specified unavailable
partitions as "absent" so in this case, pass "absent" rather than the
unaltered NAME= or ROOT.x string, which the kernel has no clue what
do do with, and doesn't configure the raid at all.

Fix some config file parsing.

First, and what got me started on this set of cleanups, the queue
length in the "queue" section (START queue) is limited to what will
fit in a char without losing accuracy (I tried setting it to 200,
rather than the more common (universal?) 100 and found that the
value configured into the array was -56 instead.

Why the value needs to be passed through a char variable I have no
idea (it is an int in the filesystem raidframe headers) - but that's
the way it is done, and changing it would be an ABI change I believe
(and so need versioning to alter) and that isn't worth it for this
(or not now, IMO).

Instead check that the value in the char is the same value as was
read from the config file, and complain if not.   Those of you with
unsigned chars will be able to have queue lengths up to 255, the
rest of us are limited to 127.

While looking at that, I noticed some code that obviously fails to
understand that scanf("%s") will never return a string containing
spaces, and proceeded to attempt to remove trailing spaces from the
result ... amusingly, after having used the result for its intended
purpose (non existent trailing spaces unremoved), after which that
buffer was never used again.   That code is now gone (but for now,
just #if 0'd rather than actually deleted - it should be cleaned up
sometime).

Then I saw some other issues with how the config was parsed - a
simple (unbounded) scanf("%s") into a buffer, which hypothetically
might not be large enough (not a security issue really, raidctl has
no special privs, and it isn't likely that root could easily be
tricked into running it on a bogus config file - or not without
looking first anyway, and a huge long string would rather stand
out).   Bound the string length to something reasonable, and
assert() that the buffer is big enough to contain it.

Lastly, in the event of one particular detected error in the
config file, the code would write a warning, but then just go
ahead and use the bad data (or nothing perhaps) anyway - a
failure of logic flow (unlikely to have ever happened, everyone
seems to simply copy the sample config from the man page, and
make minor adjustments as needed).

If any of these changes make any difference to anyone (except
me with my attempt to make longer queues - for no particularly
well thought out reason), I'd be very surprised.

In the previous (and some earlier) version(s) of raidctl.c
the following comment appeared:

	/*
	 * After NetBSD 9, convert this to not output the numRow's value,
	 * which is no longer required or ever used.
	 */

We are after NetBSD 9 (well after).   The change requested in that
comment is made here, and the comment is thus removed.

A couple of places in rf_configure.c where a value for the "rows"
parameter was output in an error message (always simply as the
constant 0) have also been updated (those messages will no longer
include "row 0", which they always said previously).   One of them
was also slightly reworded to be clearer what problem it was
experiencing (when it said 'unable to get device file' it meant
it was unable to locate the name for the device in the config file,
not that it was found, and there was some other problem with it).

avoid trying to printf() a NULL as %s.  fixes likely bug.

Pull up following revision(s) (requested by nakayama in ticket #1019):

	sbin/raidctl/rf_configure.h: revision 1.2
	sbin/raidctl/rf_configure.c: revision 1.27
	sbin/raidctl/rf_configure.c: revision 1.28
	sbin/raidctl/rf_configure.c: revision 1.29
	sbin/raidctl/raidctl.8: revision 1.73
	sbin/raidctl/rf_configure.c: revision 1.30
	sbin/raidctl/rf_configure.c: revision 1.31
	sbin/raidctl/rf_configure.c: revision 1.32

support NAME=<wedge name> syntax for disks and spares

 -

stop using magic constants
wrap long lines
use warn{,x}
make static
knf

 -

White space and comment formatting.   NFC.

 -

With char bug[SIZE]  using sizeof(bug[0]) is kind of boring, use
sizeof(bug) instead...

 -

Avoid needless pointer calisthenics: &foo[0] -> foo

 -

Several more cleanups:
1. Don't force use of "for" when "while" works better.
2. No need to check c != '\0' when we also check (c == ' ' || c == '\t')
3. Use the size of the buffer we're using, rather than a different one
   (not really a concern, they're the same size)
4. Don't use fscanf() to read file data, use fgets() & sscanf().
5. After using a pointer as a char *, validate alignment before switching
   to int * (can only fail if kernel #define gets set stupidly)   Or #6...
6. Validate sparemap file name isn't too long for assigned space.
7. recognise that strlen() returns size_t - don't shove it into an int.
8. On out of mem, be more clear which allocation failed in warning msg.

ATF tests all pass.   But I don't think they use sparemap files.

implement 32-bit compat support for raidframe.

convert several raidframe ioctls to be bitsize idempotent so that
they work the same in 32 and 64 bit worlds, allowing netbsd32 to
configure and query raid properly.  remove useless 'row' in a few
places.  add COMPAT_80 and put the old ioctls there.

raidframeio.h:
  RAIDFRAME_TEST_ACC
  - remove, unused
  RAIDFRAME_GET_COMPONENT_LABEL
  - convert to label not pointer to label
  RAIDFRAME_CHECK_RECON_STATUS_EXT
  RAIDFRAME_CHECK_PARITYREWRITE_STATUS_EXT
  RAIDFRAME_CHECK_COPYBACK_STATUS_EXT
  - convert to progress info not pointer to info
  RAIDFRAME_GET_INFO
  - version entirely.
raidframevar.h:
  - rf_recon_req{} has row, flags and raidPtr removed (they're
    not a useful part of this interface.)
  - RF_Config_s{} and RF_DeviceConfig_s{} have numRow/rows removed.
  - RF_RaidDisk_s{} is re-ordered slightly to fix alignment
    padding - the actual data was already OK.
  - InstallSpareTable() loses row argument

rf_compat32.c has code for RF_Config_s{} in 32 bit mode, used
by RAIDFRAME_CONFIGURE and RAIDFRAME_GET_INFO32.

rf_compat80.c has code for rf_recon_req{}, RF_RaidDisk_s{} and
RF_DeviceConfig_s{} to handle RAIDFRAME_FAIL_DISK,
RAIDFRAME_GET_COMPONENT_LABEL, RAIDFRAME_CHECK_RECON_STATUS_EXT,
RAIDFRAME_CHECK_PARITYREWRITE_STATUS_EXT,
RAIDFRAME_CHECK_COPYBACK_STATUS_EXT, RAIDFRAME_GET_INFO.

move several of the per-ioctl code blocks into separate functions.

add rf_recon_req_internal{} to replace old usage of global
rf_recon_req{} that had unused void * in the structure, ruining
it's 32/64 bit ABI.

add missing case for RAIDFRAME_GET_INFO50.

adjust raid tests to use the new .conf format, and add a case to
test the old method as well.

raidctl:
deal with lack of 'row' members in a couple of places.
fail request no longer takes row.
handle "START array" sections with just "numCol numSpare", ie
no "numRow" specified.  for now, generate old-style configuration
but update raidctl.8 to specify the new style (keeping reference
to the old style.)

note that: RF_ComponentLabel_s::{row,num_rows} and
RF_SingleComponent_s::row are obsolete but not removed yet.

Several more cleanups:
1. Don't force use of "for" when "while" works better.
2. No need to check c != '\0' when we also check (c == ' ' || c == '\t')
3. Use the size of the buffer we're using, rather than a different one
   (not really a concern, they're the same size)
4. Don't use fscanf() to read file data, use fgets() & sscanf().
5. After using a pointer as a char *, validate alignment before switching
   to int * (can only fail if kernel #define gets set stupidly)   Or #6...
6. Validate sparemap file name isn't too long for assigned space.
7. recognise that strlen() returns size_t - don't shove it into an int.
8. On out of mem, be more clear which allocation failed in warning msg.

ATF tests all pass.   But I don't think they use sparemap files.

Avoid needless pointer calisthenics: &foo[0] -> foo

With char bug[SIZE]  using sizeof(bug[0]) is kind of boring, use
sizeof(bug) instead...

White space and comment formatting.   NFC.

stop using magic constants
wrap long lines
use warn{,x}
make static
knf

support NAME=<wedge name> syntax for disks and spares

PR/50921: David Binderman: Fix memory leak

use warn/err appropriately.

Sync with HEAD.

Third (and last) commit. See http://mail-index.netbsd.org/source-changes/2009/05/13/msg221222.html

fix sign-compare issue

close fp in several error paths, closing Coverity CIDs 1690, 1692.

"go" by go.

Kill __P(), use ANSI function declarations; WARNS=3.

Cleanup a bit by nuking RF_Malloc macro, and add in some error
checking for the case where we can't allocate memory.  (Not like
anyone uses this part of the code anyway.)

Strip off trailing \n's and spaces from configuration file lines.
Makes RAIDframe more forgiving if you accidentally add a space after
things like device names.  Fixed at long last after a reminder over
the weekend.

How about we use the 'len' parameter to rf_get_next_nonblank_line()
instead of assuming all buffers are 256 bytes.

use bouded string ops

Add NetBSD RCS Ids.

sscanf overrun

rf_configure.c
 - remove dependence on stuff in sys/dev/raidframe
 - now rely on <dev/raidframe/*>
 - bring in some needed prototypes for local functions
 - nuke RF_ASSERT's.
 - drag in some needed RF_ERRORMSG's

raidctl.c
 - rely on <dev/raidframe/*>
 - welcome to the New Way of doing RAIDframe #includes.

(No functional changes.)

remove redundant prototypes.

Move parts of a RAID type mapping table from rf_layout.c to rf_configure.c.
This means we don't need to compile or link with rf_layout.c here.

Check return value of fgets. Approved by Greg Oster.

Pullup sbin string format fixes [is].
See "cvs log" for explicit revision numbers per file, from sommerfeld.

Pull up revision 1.11 (via patch, requested by is):
  Format string cleanup.

Format string cleanups by Bill Sommerfeld.

Apply indent.

Build with WARNS=2.

No need to include rf_sys.h.

Move two functions from rf_strutils.c into rf_configure.c, removing
the need for rf_strutils.c.

Nuke unused code.

Add missing NetBSD RCS ID's!  Thanks to SAITOH Masanobu (msaitoh@netbsd.org)
for pointing this out.

Take care of a few minor things due to the major RAIDframe cleanup.

Again, no functionality changes.

recover from errno.h changes

fix compile errors on the alpha.

RAIDframe, version 1.1, from the Parallel Data Laboratory at
Carnegie Mellon University.  Full RAID implementation, including
levels 0, 1, 4, 5, 6, parity logging, and a few other goodies.
Ported to NetBSD by Greg Oster.

raidctl is our userland configuration tool for RAIDframe.