On Mon, Jan 12, 2004 at 09:30:12PM +0100, Sander van Vliet wrote: > I have attempted to create a new disk-alive metric. Currently it only > works on Linux. A short description of what it, disk_alive_func(), > does: > 1 Get a list of filesystems via /proc/mounts
If you split this out into a separate function, it could be implemented
on each platform and the rest of the code could probably be made
portable.
> 2 Iterate through the list and for local filesystems call check_alive_func()
>
> check_alive_func does the following:
> 1 Get the size of the disk
This should probably also be done at startup time. On most disks this
won't change at runtime. :-)
> 2 Generate a random number smaller than the disksize
> 3 Open the disk
You should do the open in the metric init code (see the way I open
/dev/kmem in the FreeBSD init code). This allows you to drop root
privs so you lessen the degree to which the system in vulnerable if
ganglia has a buffer overflow.
> 4 Seek to that number of bytes
> 5 Read 4 bytes of data starting at that position
For portability, you should probably try to read one aligned block.
This is required on FreeBSD and DragonFly BSD where there are no block
devices.
> 6 Close the disk
> The function checks for errors at every step. If at any step there is a
> failure the function returns the value 0 and disk_alive_func will set it's
> return value to "failure".
>
> There is still a lot of work needed. For instance there is no check
> whatsoever if the read or open call enters an interruptable sleep
> (waiting for IO access). Also there is a problem with the file
> permissions on the devices. On most Linux distro's the user nobody has
> by default no access to those devices.
Cool. This looks like a good start. I've got more comments in the code
below. Thanks for working on this. It's a feature I've been wanting.
>
> Any comments are welcome. Patches are included below.
>
> - Sander van Vliet
> [EMAIL PROTECTED]
> +31-6-30281912
>
> === PATCHES === PATCHES ===
> --- old.key_metrics.h 2003-10-26 00:57:44.000000000 +0200
> +++ key_metrics.h 2004-01-12 20:43:27.000000000 +0100
> @@ -59,6 +59,11 @@
> disk_total,
> disk_free,
> part_max_used,
> + /*
> + * added by sander
> + * new disk alive metric
> + */
> + disk_alive,
> #endif
> #ifdef HPUX
> cpu_intr,
>
> --- old.metric.h 2003-10-26 00:57:44.000000000 +0200
> +++ metric.h 2004-01-12 20:43:27.000000000 +0100
> @@ -74,6 +74,13 @@
> extern g_val_t disk_free_func(void);
> extern g_val_t part_max_used_func(void);
>
> +/* these are for the new disk alive metric and are _not_ tested.
> + * the only test was to see if it reported that all disks were alive }:->
> + * added by sander, [EMAIL PROTECTED]
> + */
> +extern g_val_t disk_alive_func(void);
> +int check_alive_func(char device[256], char mount[256]);
> +
> #endif
>
> #ifdef HPUX
> @@ -198,6 +205,7 @@
> * The amount of disk space could change - hot-swap, mounts, etc. check:
> 30-60min. */
> KEY(disk_total), 1, 1800, 3600, 900, 1200, g_double, "GB", "%.3f" },
> KEY(disk_free), 1, 30, 40, 120, 180, g_double, "GB", "%.3f" },
> +KEY(disk_alive), 1, 15, 20, 60, 90, g_string, "", "%s"},
> KEY(part_max_used), 1, 30, 40, 120, 180, g_float, "%", "%.1f" }
>
> #endif
> @@ -237,6 +245,7 @@
> KEY(part_max_used), 1, 30, 40, 120, 180, g_float, "%", "%.1f" }
>
> #endif
> +
> };
>
> #endif /* METRIC_H */
>
> --- old.linux.c 2003-10-29 21:41:38.000000000 +0100
> +++ linux.c 2004-01-12 20:43:34.000000000 +0100
> @@ -1,5 +1,7 @@
> #include <time.h>
> #include <unistd.h>
> +#include <string.h>
> +#include <errno.h>
> #include "ganglia.h"
> #include "metric_typedefs.h"
>
> @@ -1046,3 +1048,112 @@
> return val;
> }
>
> +/* Report if a disk has failed.
> + * returns "failure" or "all ok"
> + */
> +g_val_t
> +disk_alive_func( void )
> +{
> + FILE *mounts;
> + size_t size;
> + char procline[256];
> + char mount[128], device[128], type[32];
> + g_val_t val;
> + int rc;
> +
> + mounts = fopen(MOUNTS,"r");
> + if(!mounts) {
> + debug_msg("disk_alive_func() Could not open mounts file %s. Are
> we on the
> righs OS?\n",MOUNTS);
> + snprintf(val.str, MAX_G_STRING_SIZE, "error");
> + return val;
> + }
> +
> + /* skip redundant checks in the while-loop */
> + size = sizeof(procline);
> +
> + /* set the default return value */
> + snprintf(val.str, MAX_G_STRING_SIZE, "all ok");
> +
> + while(fgets(procline, size, mounts)) {
> + rc = sscanf(procline, "%s %s %s", device, mount, type);
> + if(rc > 0) {
> + if(remote_mount(device, type)) {
> + /* Skip remove devices */
> + continue;
> + } else if(strncmp(device,"/dev/",5) != 0) {
> + /* Skip everything not starting with /dev/ */
> + continue;
> + } else if(strncmp(device,"/dev/root",9) == 0) {
> + /* This is a fix for SuSE */
> + continue;
> + } else {
> + /* check_alive_func is below */
> + if(check_alive_func(device,mount) == 0) {
> + debug_msg("disk_alive_func() Device %s
> failed!", device);
> + snprintf(val.str, MAX_G_STRING_SIZE,
> "failure");
> + /* bail out on first failure */
> + break;
> + }
> + }
> + }
> + }
> +
> + /* clean up */
> + fclose(mounts);
> +
> + return val;
> +}
> +
> +/* Read 4 bytes of data from a random position
> + * return 0 on failure 1 on success
> + */
> +int
> +check_alive_func(char device[256], char mount[256])
Use "const char *" not fixed length arrays.
> +{
> + void *data;
> + struct fs_usage fsu;
> + int fd, random, disk_total;
random and disk_total should be size_t's. Mere int's are way too small for
many disks these days.
> +
> + /* get the disk size */
> + /* note: we use blocks. This way we are always within the
> + bounds of the disk size */
> + get_fs_usage((char *)mount,(char *)device,&fsu);
> + disk_total = fsu.fsu_blocks;
> +
> + /* check if we can open the disk */
> + fd = open(device,O_RDONLY);
> + if(fd < 0) {
> + /* error: unable to open */
> + return 0;
> + }
> +
> + /* initialize the seed */
> + srandom(time(NULL) + getpid());
Do this at startup. Calling it more then once hurts more then it helps.
> +
> + /* get a random number, keep going till we get one that fits */
> + while(1) {
> + random = rand();
> + if(random < disk_total) {
> + break;
> + }
> + }
rand() is seeded by srand not srandom so either change the srandom call
or use random(). It's probably best to use random() due to the crappy
behavior of rand()'s low-order bits.
Why not random = random() % disk_total rather then looping?
> +
> + /* seek to the random position */
> + if(lseek(fd,random,SEEK_SET) < 0) {
> + /* unable to seek */
> + return 0;
> + }
> +
> + /* read 4 bytes */
> + data = (void *)malloc(4);
A static allocation on the stack or (if using blocks sized reads) a
preallocated buffer in init.
> + if(read(fd,data,4) < 4) {
> + /* unable to read */
> + return 0;
> + }
> +
> + /* clean up */
> + free(data);
> + close(fd);
> +
> + return 1;
> +}
Thanks,
Brooks
--
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4
pgpTrnAPKhCaA.pgp
Description: PGP signature
