NetBSD Problem Report #51334
From ef@math.uni-bonn.de Mon Jul 11 18:10:22 2016
Return-Path: <ef@math.uni-bonn.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 55BDA7A3DB
for <gnats-bugs@gnats.NetBSD.org>; Mon, 11 Jul 2016 18:10:22 +0000 (UTC)
Message-Id: <20160711165109.430865EDB@jade.math.uni-bonn.de>
Date: Mon, 11 Jul 2016 18:51:09 +0200 (CEST)
From: ef@math.uni-bonn.de
Reply-To: ef@math.uni-bonn.de
To: gnats-bugs@gnats.NetBSD.org
Subject: collectd disk plugin NetBSD implementation correction
X-Send-Pr-Version: 3.95
>Number: 51334
>Category: pkg
>Synopsis: collectd disk plugin NetBSD implementation correction
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: pkg-manager
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Mon Jul 11 18:15:00 +0000 2016
>Closed-Date: Thu Dec 17 15:57:00 +0000 2020
>Last-Modified: Thu Dec 17 15:57:00 +0000 2020
>Originator: Edgar Fuß
>Release: pkgsrc-2016Q1
>Organization:
Mathematisches Institut der Universität Bonn
>Environment:
>Description:
The NetBSD implementation of the collectd disk plugin (sysutils/collectd/patches/patch-src_disk.c) is overly complicated, and mostly wrong. This is probably due to defiencies in the collect documentation.
For a DERIVE Data source, the plugin simply provides the new value; it's the daemon's business to compute the change rate.
It doesn't make sense on NetBSD to try to compute disk_time (the average time per read/write op) in a PenguinOS compatible way. There's no data source for separate read and write times spent by the disc.
However, it's trivial to pass disk_io_time (the disc's busy ratio).
>How-To-Repeat:
Use collectd's disk plugin on a NetBSD machine. Inspect disk_time.
>Fix:
Replace patches/patch-src_disk.c with the following (and re-compute patch checksums and bump PKG_REVISION):
$NetBSD: patch-src_disk.c $
Provide a port to NetBSD.
--- src/disk.c.orig 2015-03-10 14:14:45.000000000 +0000
+++ src/disk.c
@@ -124,6 +124,35 @@ static int numdisk;
static int pnumdisk;
/* #endif HAVE_PERFSTAT */
+#elif HAVE_SYSCTL && KERNEL_NETBSD
+
+#include <sys/sysctl.h>
+#include <sys/iostat.h>
+
+typedef struct diskstats {
+ char *name;
+
+ u_int poll_count;
+
+ derive_t read_ops;
+ derive_t write_ops;
+
+ derive_t read_bytes;
+ derive_t write_bytes;
+
+ derive_t avg_io_time;
+
+ struct io_sysctl stats;
+
+ struct diskstats *next;
+} diskstats_t;
+
+static diskstats_t *disklist;
+static struct io_sysctl *drives = NULL;
+static size_t ndrive = 0;
+
+/* #endif HAVE_SYSCTL && KERNEL_NETBSD */
+
#else
# error "No applicable input method."
#endif
@@ -241,7 +270,34 @@ static int disk_init (void)
continue;
ksp[numdisk++] = ksp_chain;
}
-#endif /* HAVE_LIBKSTAT */
+/* #endif HAVE_LIBKSTAT */
+
+#elif HAVE_SYSCTL && KERNEL_NETBSD
+ int mib[3];
+ size_t size;
+
+ /* figure out number of drives */
+ mib[0] = CTL_HW;
+ mib[1] = HW_IOSTATS;
+ mib[2] = sizeof(struct io_sysctl);
+ if (sysctl(mib, 3, NULL, &size, NULL, 0) == -1) {
+ ERROR ("disk plugin: sysctl for ndrives failed");
+ return -1;
+ }
+ ndrive = size / sizeof(struct io_sysctl);
+
+ if (size == 0 ) {
+ ERROR ("disk plugin: no drives found");
+ return -1;
+ }
+ drives = (struct io_sysctl *)malloc(size);
+ if (drives == NULL) {
+ ERROR ("disk plugin: memory allocation failure");
+ return -1;
+ }
+
+#endif /* HAVE_SYSCTL && KERNEL_NETBSD */
+
return (0);
} /* int disk_init */
@@ -929,7 +985,129 @@ static int disk_read (void)
write_time *= ((double)(_system_configuration.Xint)/(double)(_system_configuration.Xfrac)) / 1000000.0;
disk_submit (stat_disk[i].name, "disk_time", read_time, write_time);
}
-#endif /* defined(HAVE_PERFSTAT) */
+/* #endif defined(HAVE_PERFSTAT) */
+
+#elif HAVE_SYSCTL && KERNEL_NETBSD
+ int mib[3];
+ size_t size, i, nndrive;
+ diskstats_t *ds, *pre_ds;
+ char *output_name;
+
+ u_int64_t ops;
+ u_int64_t delta_t;
+
+ /* figure out number of drives */
+ mib[0] = CTL_HW;
+ mib[1] = HW_IOSTATS;
+ mib[2] = sizeof(struct io_sysctl);
+ if (sysctl(mib, 3, NULL, &size, NULL, 0) == -1) {
+ ERROR ("disk plugin: sysctl for ndrives failed");
+ return -1;
+ }
+ nndrive = size / sizeof(struct io_sysctl);
+
+ if (size == 0 ) {
+ ERROR ("disk plugin: no drives found");
+ return -1;
+ }
+ /* number of drives changed, reallocate buffer */
+ if (nndrive != ndrive) {
+ drives = (struct io_sysctl *)realloc(drives, size);
+ if (drives == NULL) {
+ ERROR ("disk plugin: memory allocation failure");
+ return -1;
+ }
+ ndrive = nndrive;
+ }
+
+ /* get stats for all drives */
+ mib[0] = CTL_HW;
+ mib[1] = HW_IOSTATS;
+ mib[2] = sizeof(struct io_sysctl);
+ if (sysctl(mib, 3, drives, &size, NULL, 0) == -1) {
+ ERROR ("disk plugin: sysctl for drive stats failed");
+ return -1;
+ }
+
+ for (i = 0; i < ndrive; i++) {
+
+ if (drives[i].type != IOSTAT_DISK)
+ continue;
+
+ /* find drive stats, if present */
+ for (ds = disklist, pre_ds = disklist;
+ ds != NULL;
+ pre_ds = ds, ds = ds->next) {
+ if (strcmp (drives[i].name, ds->name) == 0)
+ break;
+ }
+ if (ds == NULL) { /* not found; allocate & link in */
+ if ((ds = calloc(1, sizeof(diskstats_t))) == NULL)
+ continue;
+ if ((ds->name = strdup(drives[i].name)) == NULL) {
+ free(ds);
+ continue;
+ }
+ if (pre_ds == NULL)
+ disklist = ds;
+ else
+ pre_ds->next = ds;
+ }
+
+ ds->poll_count++;
+ if (ds->poll_count <= 2)
+ {
+ DEBUG ("disk plugin: (ds->poll_count = %i) <= "
+ "(min_poll_count = 2); => Not writing.",
+ ds->poll_count);
+ ds->stats = drives[i]; /* but save base values */
+ continue;
+ }
+ ds->read_ops = drives[i].rxfer - ds->stats.rxfer;
+ ds->write_ops = drives[i].wxfer - ds->stats.wxfer;
+ ds->read_bytes = drives[i].rbytes - ds->stats.rbytes;
+ ds->write_bytes = drives[i].wbytes - ds->stats.wbytes;
+
+ /* Need this dance because of unsigned values... */
+ if (drives[i].time_usec < ds->stats.time_usec) {
+ delta_t = ((drives[i].time_sec - 1 -
+ ds->stats.time_sec) * 1000) +
+ ((drives[i].time_usec + 1000000 -
+ ds->stats.time_usec) / 1000);
+ } else {
+ delta_t = ((drives[i].time_sec -
+ ds->stats.time_sec) * 1000) +
+ ((drives[i].time_usec -
+ ds->stats.time_usec) / 1000);
+ }
+
+ ops = ds->read_ops + ds->write_ops;
+ if (ops == 0) {
+ DEBUG ("disk plugin: read + write ops == 0, "
+ "not writing");
+ continue;
+ }
+
+ ds->avg_io_time = delta_t / ops;
+
+ output_name = drives[i].name;
+
+ if ((ds->read_bytes != 0) || (ds->write_bytes != 0))
+ disk_submit (output_name, "disk_octets",
+ ds->read_bytes, ds->write_bytes);
+
+ if ((ds->read_ops != 0) || (ds->write_ops != 0))
+ disk_submit (output_name, "disk_ops",
+ ds->read_ops, ds->write_ops);
+
+ if (ds->avg_io_time != 0)
+ disk_submit (output_name, "disk_time",
+ ds->avg_io_time, ds->avg_io_time);
+
+ ds->stats = drives[i];
+ }
+
+#endif /* HAVE_SYSCTL && KERNEL_NETBSD */
return (0);
} /* int disk_read */
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: pkg-manager->fhajny
Responsible-Changed-By: wiz@NetBSD.org
Responsible-Changed-When: Mon, 11 Jul 2016 21:46:59 +0000
Responsible-Changed-Why:
Over to maintainer.
From: Edgar =?iso-8859-1?B?RnXf?= <ef@math.uni-bonn.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/51334: collectd disk plugin NetBSD implementation correction
Date: Wed, 25 Oct 2017 19:16:23 +0200
--bKyqfOwhbdpXa4YI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Unfortunately, the old, non-functional disk plugin patch has been ported to the new collectd version (mainly formatting changes) instead of the functional one in the PR.
So here's a functional patch for the new collectd version. I hope I haven't mangled it.
--bKyqfOwhbdpXa4YI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="patch-src_disk.c"
$NetBSD: patch-src_disk.c,v $
Provide a working port to NetBSD.
--- src/disk.c.orig 2017-10-06 19:32:22.000000000 +0200
+++ src/disk.c 2017-10-09 14:13:56.000000000 +0200
@@ -135,6 +135,16 @@
static int pnumdisk;
/* #endif HAVE_PERFSTAT */
+#elif HAVE_SYSCTL && KERNEL_NETBSD
+
+#include <sys/sysctl.h>
+#include <sys/iostat.h>
+
+static struct io_sysctl *drives = NULL;
+static size_t ndrive = 0;
+
+/* #endif HAVE_SYSCTL && KERNEL_NETBSD */
+
#else
#error "No applicable input method."
#endif
@@ -253,7 +263,31 @@
continue;
ksp[numdisk++] = ksp_chain;
}
-#endif /* HAVE_LIBKSTAT */
+/* #endif HAVE_LIBKSTAT */
+#elif HAVE_SYSCTL && KERNEL_NETBSD
+ int mib[3];
+ size_t size;
+
+ /* figure out number of drives */
+ mib[0] = CTL_HW;
+ mib[1] = HW_IOSTATS;
+ mib[2] = sizeof(struct io_sysctl);
+ if (sysctl(mib, 3, NULL, &size, NULL, 0) == -1) {
+ ERROR ("disk plugin: sysctl for ndrives failed");
+ return -1;
+ }
+ ndrive = size / sizeof(struct io_sysctl);
+
+ if (size == 0 ) {
+ ERROR ("disk plugin: no drives found");
+ return -1;
+ }
+ drives = (struct io_sysctl *)malloc(size);
+ if (drives == NULL) {
+ ERROR ("disk plugin: memory allocation failure");
+ return -1;
+ }
+#endif /* HAVE_SYSCTL && KERNEL_NETBSD */
return (0);
} /* int disk_init */
@@ -284,7 +318,7 @@
plugin_dispatch_values(&vl);
} /* void disk_submit */
-#if KERNEL_FREEBSD || KERNEL_LINUX
+#if KERNEL_FREEBSD || (HAVE_SYSCTL && KERNEL_NETBSD) || KERNEL_LINUX
static void submit_io_time(char const *plugin_instance, derive_t io_time,
derive_t weighted_time) {
value_list_t vl = VALUE_LIST_INIT;
@@ -300,7 +334,7 @@
plugin_dispatch_values(&vl);
} /* void submit_io_time */
-#endif /* KERNEL_FREEBSD || KERNEL_LINUX */
+#endif /* KERNEL_FREEBSD || (HAVE_SYSCTL && KERNEL_NETBSD) || KERNEL_LINUX */
#if KERNEL_LINUX
static void submit_in_progress(char const *disk_name, gauge_t in_progress) {
@@ -1018,7 +1052,58 @@
1000000.0;
disk_submit(stat_disk[i].name, "disk_time", read_time, write_time);
}
-#endif /* defined(HAVE_PERFSTAT) */
+/* #endif defined(HAVE_PERFSTAT) */
+#elif HAVE_SYSCTL && KERNEL_NETBSD
+ int mib[3];
+ size_t size, i, nndrive;
+
+ /* figure out number of drives */
+ mib[0] = CTL_HW;
+ mib[1] = HW_IOSTATS;
+ mib[2] = sizeof(struct io_sysctl);
+ if (sysctl(mib, 3, NULL, &size, NULL, 0) == -1) {
+ ERROR ("disk plugin: sysctl for ndrives failed");
+ return -1;
+ }
+ nndrive = size / sizeof(struct io_sysctl);
+
+ if (size == 0 ) {
+ ERROR ("disk plugin: no drives found");
+ return -1;
+ }
+ /* number of drives changed, reallocate buffer */
+ if (nndrive != ndrive) {
+ drives = (struct io_sysctl *)realloc(drives, size);
+ if (drives == NULL) {
+ ERROR ("disk plugin: memory allocation failure");
+ return -1;
+ }
+ ndrive = nndrive;
+ }
+
+ /* get stats for all drives */
+ mib[0] = CTL_HW;
+ mib[1] = HW_IOSTATS;
+ mib[2] = sizeof(struct io_sysctl);
+ if (sysctl(mib, 3, drives, &size, NULL, 0) == -1) {
+ ERROR ("disk plugin: sysctl for drive stats failed");
+ return -1;
+ }
+
+ for (i = 0; i < ndrive; i++) {
+ if (drives[i].type != IOSTAT_DISK)
+ continue;
+ if (ignorelist_match(ignorelist, drives[i].name))
+ continue;
+
+ disk_submit(drives[i].name, "disk_octets",
+ drives[i].rbytes, drives[i].wbytes);
+ disk_submit(drives[i].name, "disk_ops",
+ drives[i].rxfer, drives[i].wxfer);
+ submit_io_time(drives[i].name,
+ drives[i].time_sec * 1000 + drives[i].time_usec / 1000, 0);
+ }
+#endif /* HAVE_SYSCTL && KERNEL_NETBSD */
return (0);
} /* int disk_read */
--bKyqfOwhbdpXa4YI--
From: Edgar =?iso-8859-1?B?RnXf?= <ef@math.uni-bonn.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: pkg/51334 (collectd disk plugin NetBSD implementation correction)
Date: Fri, 7 Sep 2018 10:16:08 +0200
And once again, the included, dysfunctional disk plugin patch for NetBSD has been ported to a newer version of collectd.
And once again, I ported my working disk plugin for NetBSD to a newer version of collectd.
Sigh.
$NetBSD: patch-src_disk.c,v $
Provide a working port to NetBSD.
--- src/disk.c.orig 2018-08-30 15:04:48.000000000 +0200
+++ src/disk.c 2018-08-30 15:04:49.000000000 +0200
@@ -135,6 +135,16 @@ static int numdisk;
static int pnumdisk;
/* #endif HAVE_PERFSTAT */
+#elif HAVE_SYSCTL && KERNEL_NETBSD
+
+#include <sys/sysctl.h>
+#include <sys/iostat.h>
+
+static struct io_sysctl *drives = NULL;
+static size_t ndrive = 0;
+
+/* #endif HAVE_SYSCTL && KERNEL_NETBSD */
+
#else
#error "No applicable input method."
#endif
@@ -253,7 +263,31 @@ static int disk_init(void) {
continue;
ksp[numdisk++] = ksp_chain;
}
-#endif /* HAVE_LIBKSTAT */
+/* #endif HAVE_LIBKSTAT */
+#elif HAVE_SYSCTL && KERNEL_NETBSD
+ int mib[3];
+ size_t size;
+
+ /* figure out number of drives */
+ mib[0] = CTL_HW;
+ mib[1] = HW_IOSTATS;
+ mib[2] = sizeof(struct io_sysctl);
+ if (sysctl(mib, 3, NULL, &size, NULL, 0) == -1) {
+ ERROR ("disk plugin: sysctl for ndrives failed");
+ return -1;
+ }
+ ndrive = size / sizeof(struct io_sysctl);
+
+ if (size == 0 ) {
+ ERROR ("disk plugin: no drives found");
+ return -1;
+ }
+ drives = (struct io_sysctl *)malloc(size);
+ if (drives == NULL) {
+ ERROR ("disk plugin: memory allocation failure");
+ return -1;
+ }
+#endif /* HAVE_SYSCTL && KERNEL_NETBSD */
return 0;
} /* int disk_init */
@@ -284,7 +318,7 @@ static void disk_submit(const char *plug
plugin_dispatch_values(&vl);
} /* void disk_submit */
-#if KERNEL_FREEBSD || KERNEL_LINUX
+#if KERNEL_FREEBSD || (HAVE_SYSCTL && KERNEL_NETBSD) || KERNEL_LINUX
static void submit_io_time(char const *plugin_instance, derive_t io_time,
derive_t weighted_time) {
value_list_t vl = VALUE_LIST_INIT;
@@ -300,7 +334,7 @@ static void submit_io_time(char const *p
plugin_dispatch_values(&vl);
} /* void submit_io_time */
-#endif /* KERNEL_FREEBSD || KERNEL_LINUX */
+#endif /* KERNEL_FREEBSD || (HAVE_SYSCTL && KERNEL_NETBSD) || KERNEL_LINUX */
#if KERNEL_LINUX
static void submit_in_progress(char const *disk_name, gauge_t in_progress) {
@@ -1017,7 +1051,58 @@ static int disk_read(void) {
1000000.0;
disk_submit(stat_disk[i].name, "disk_time", read_time, write_time);
}
-#endif /* defined(HAVE_PERFSTAT) */
+/* #endif defined(HAVE_PERFSTAT) */
+#elif HAVE_SYSCTL && KERNEL_NETBSD
+ int mib[3];
+ size_t size, i, nndrive;
+
+ /* figure out number of drives */
+ mib[0] = CTL_HW;
+ mib[1] = HW_IOSTATS;
+ mib[2] = sizeof(struct io_sysctl);
+ if (sysctl(mib, 3, NULL, &size, NULL, 0) == -1) {
+ ERROR ("disk plugin: sysctl for ndrives failed");
+ return -1;
+ }
+ nndrive = size / sizeof(struct io_sysctl);
+
+ if (size == 0 ) {
+ ERROR ("disk plugin: no drives found");
+ return -1;
+ }
+ /* number of drives changed, reallocate buffer */
+ if (nndrive != ndrive) {
+ drives = (struct io_sysctl *)realloc(drives, size);
+ if (drives == NULL) {
+ ERROR ("disk plugin: memory allocation failure");
+ return -1;
+ }
+ ndrive = nndrive;
+ }
+
+ /* get stats for all drives */
+ mib[0] = CTL_HW;
+ mib[1] = HW_IOSTATS;
+ mib[2] = sizeof(struct io_sysctl);
+ if (sysctl(mib, 3, drives, &size, NULL, 0) == -1) {
+ ERROR ("disk plugin: sysctl for drive stats failed");
+ return -1;
+ }
+
+ for (i = 0; i < ndrive; i++) {
+ if (drives[i].type != IOSTAT_DISK)
+ continue;
+ if (ignorelist_match(ignorelist, drives[i].name))
+ continue;
+
+ disk_submit(drives[i].name, "disk_octets",
+ drives[i].rbytes, drives[i].wbytes);
+ disk_submit(drives[i].name, "disk_ops",
+ drives[i].rxfer, drives[i].wxfer);
+ submit_io_time(drives[i].name,
+ drives[i].time_sec * 1000 + drives[i].time_usec / 1000, 0);
+ }
+#endif /* HAVE_SYSCTL && KERNEL_NETBSD */
return 0;
} /* int disk_read */
Responsible-Changed-From-To: fhajny->pkg-manager
Responsible-Changed-By: wiz@NetBSD.org
Responsible-Changed-When: Wed, 27 May 2020 19:39:08 +0000
Responsible-Changed-Why:
Maintainer was reset
From: Edgar =?iso-8859-1?B?RnXf?= <ef@math.uni-bonn.de>
To: gnats-bugs@gnats.netbsd.org
Cc:
Subject: Re: pkg/51334: collectd disk plugin NetBSD implementation correction
Date: Tue, 1 Dec 2020 19:24:19 +0100
For the record, the (corrected) disk plugin (plus more of he@'s NetBSD implementations and an nfs plugin of my own) have been incorporated into collectd 5.11.0, which has been imported (via wip/me) to pkgsrc.
From: Benny Siegert <bsiegert@gmail.com>
To: gnats-bugs@netbsd.org
Cc: pkg-manager@netbsd.org, gnats-admin@netbsd.org, pkgsrc-bugs@netbsd.org,
ef@math.uni-bonn.de
Subject: Re: pkg/51334: collectd disk plugin NetBSD implementation
correction
Date: Wed, 2 Dec 2020 08:25:19 +0000 (UTC)
> For the record, the (corrected) disk plugin (plus more of he@'s NetBSD
> implementations and an nfs plugin of my own) have been incorporated into
> collectd 5.11.0, which has been imported (via wip/me) to pkgsrc.
So this PR can be closed then?
--
Benny
State-Changed-From-To: open->closed
State-Changed-By: bsiegert@NetBSD.org
State-Changed-When: Thu, 17 Dec 2020 15:57:00 +0000
State-Changed-Why:
Submitter says this is fixed
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home