Fwd: [rt.cpan.org #49703] GNU df is broken. Allow the use of local non-gnu df if it exists.

0 views
Skip to first unread message

Brad Fitzpatrick

unread,
Oct 3, 2009, 11:30:56 AM10/3/09
to mog...@googlegroups.com
Do RT tickets also go to this list?  (I don't think so....)

Somebody want to look into this ticket?

---------- Forwarded message ----------
From: Paul Armstrong via RT <bug-mogil...@rt.cpan.org>
Date: Mon, Sep 14, 2009 at 3:52 PM
Subject: [rt.cpan.org #49703] GNU df is broken. Allow the use of local non-gnu df if it exists.
To:


Mon Sep 14 18:52:17 2009: Request 49703 was acted upon.
Transaction: Ticket created by psa
      Queue: mogilefs-server
    Subject: GNU df is broken. Allow the use of local non-gnu df if it exists.
  Broken in: 2.30
   Severity: Important
      Owner: Nobody
 Requestors: cp...@otoh.org
     Status: new
 Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=49703 >


Mogstored uses the -P flag to df due to GNU df's broken behavior with
splitting line output and requiring -P to do the right thing.

For those of us not using GNU df, it'd be nice to have mogstored just
work with our local df.

Here's the patch to make this work:

--- DiskUsage.pm.gnu    Mon Sep 14 22:32:53 2009
+++ DiskUsage.pm        Mon Sep 14 22:37:26 2009
@@ -36,7 +36,13 @@
    my $err = sub { warn "$_[0]\n"; };
    my $path = $ENV{MOG_DOCROOT};
    $path =~ s!/$!!;
+    my $gnu_df;

+    # gnu df is broken as it splits output onto multiple lines by default
+    if($^O eq 'Linux') {
+      $gnu_df = '-P';
+    }
+
    # find all devices below us
    my @devnum;
    if (opendir(D, $path)) {
@@ -47,7 +53,7 @@
    }

    foreach my $devnum (@devnum) {
-        my $rval = `df -P -l -k $path/$devnum`;
+        my $rval = `df $gnu_df -l -k $path/$devnum`;
        my $uperK = ($rval =~ /512-blocks/i) ? 2 : 1; # units per kB
        foreach my $l (split /\r?\n/, $rval) {
            next unless $l =~
/^(.+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(.+)\s+(.+)$/;

That patch does assume Linux is the only place to find GNU df, to be
really sure, the fix would be something more like the following
(although it does end up making 2 system calls instead of one each time)

--- DiskUsage.pm.gnu    Mon Sep 14 22:32:53 2009
+++ DiskUsage.pm        Mon Sep 14 22:49:26 2009
@@ -36,7 +36,14 @@
    my $err = sub { warn "$_[0]\n"; };
    my $path = $ENV{MOG_DOCROOT};
    $path =~ s!/$!!;
+    my $gnu_df;

+    # gnu df is broken as it splits output onto multiple lines by default
+    `df -P / 2>/dev/null >/dev/null`;
+    if($? eq 0) {
+      $gnu_df = '-P';
+    }
+
    # find all devices below us
    my @devnum;
    if (opendir(D, $path)) {
@@ -47,7 +54,7 @@
    }

    foreach my $devnum (@devnum) {
-        my $rval = `df -P -l -k $path/$devnum`;
+        my $rval = `df $gnu_df -l -k $path/$devnum`;
        my $uperK = ($rval =~ /512-blocks/i) ? 2 : 1; # units per kB
        foreach my $l (split /\r?\n/, $rval) {
            next unless $l =~
/^(.+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(.+)\s+(.+)$/;


--
Save our economy, demand finishing metric conversion from your federal
and state representatives.

dormando

unread,
Nov 15, 2009, 7:40:13 PM11/15/09
to mog...@googlegroups.com
I finally merged a version of this in r1356

Justin Azoff

unread,
Nov 15, 2009, 8:31:36 PM11/15/09
to mog...@googlegroups.com
On Sun, Nov 15, 2009 at 04:40:13PM -0800, dormando wrote:
> I finally merged a version of this in r1356
>
[...]

> > For those of us not using GNU df, it'd be nice to have mogstored just
> > work with our local df.

It seems this problem comes up over and over again. Should mogilefs just use
one of the many perl interfaces to statvfs? I proposed this a long time ago
but I forget what the reasons for continuing to parse df output were.

--
-- Justin Azoff
-- Network Performance Analyst

dormando

unread,
Nov 15, 2009, 9:08:57 PM11/15/09
to mog...@googlegroups.com
> It seems this problem comes up over and over again. Should mogilefs just use
> one of the many perl interfaces to statvfs? I proposed this a long time ago
> but I forget what the reasons for continuing to parse df output were.

Probably paranoia over adding (yet anther) dependency. I only recall this
coming up recently and me ignoring it until just now. If it comes up again
we can just replace it, I guess?

Reply all
Reply to author
Forward
0 new messages