* fdisk: creating new partitions problem
From: Peter Hallberg @ 2011-05-10 14:49 UTC (permalink / raw)
To: util-linux-ng
When creating a new partition on a disk after an existing partition that ends unaligned there will be a few unused blocks in between them, now when creating yet another partition fdisk will sugest placing the new one in the small room between the existing partitions instead of after the last one.
^ permalink raw reply
* Re: [PATCH 1/1] dmesg: adds the ability to "follow" the klog ring buffer
From: Karel Zak @ 2011-05-04 12:43 UTC (permalink / raw)
To: Austen Dicken; +Cc: util-linux
In-Reply-To: <BANLkTikGoXaYdwvY3fZPGVmvBkGzT23xtw@mail.gmail.com>
On Tue, May 03, 2011 at 09:14:41AM -0500, Austen Dicken wrote:
> > Then you can use klogctl(2, ...). It waits until the kernel log buffer
> > is non-empty, so the last_line and usleep() will be unnecessary, and
> > the implementation will be pretty simple.
>
> Agreed. I have decided to change the patch to function this way,
> which simplifies things
> immensely.
Unfortunately, it seems that SYSLOG_ACTION_READ (aka klogctl(2, ...))
is not ideal solution too. Sorry.
The output is incomplete if another process(e.g. syslog daemon) is
reading the buffer. The buffer (and index into the buffer) is shared
for all processes, so it's possible to read the information only once.
For more details see kernel/printk.c (log_start index) in kernel
sources.
Anyway for debugging or on some non-standard boxes the '-f' option
could be still usable. I'd like to add --level= and --facility=
options, so dmesg(1) should be definitely better than the "cat
/proc/kmsg" command.
I did some changes to the patch to minimize overhead, see below. Maybe
it would be better to use open(/proc/kmsg) + read() instead of
klogctl(2, ...) for the -f option to minimize overhead in glibc.
Comments?
Karel
>From 8e2ae5130c8f03e8ab6e28f194e409f8c322daf8 Mon Sep 17 00:00:00 2001
From: Austen Dicken <cvpcsm@gmail.com>
Date: Tue, 3 May 2011 09:14:41 -0500
Subject: [PATCH] dmesg: adds the ability to "follow" (-f) the klog ring buffer
Co-Author: Karel Zak <kzak@redhat.com>
Signed-off-by: Austen Dicken <cvpcsm@gmail.com>
Signed-off-by: Karel Zak <kzak@redhat.com>
---
sys-utils/dmesg.1 | 5 +++
sys-utils/dmesg.c | 88 ++++++++++++++++++++++++++++++-----------------------
2 files changed, 55 insertions(+), 38 deletions(-)
diff --git a/sys-utils/dmesg.1 b/sys-utils/dmesg.1
index d7af1da..9b4471d 100644
--- a/sys-utils/dmesg.1
+++ b/sys-utils/dmesg.1
@@ -6,6 +6,7 @@ dmesg \- print or control the kernel ring buffer
.SH SYNOPSIS
.B dmesg
.RB [ \-c ]
+.RB [ \-f ]
.RB [ \-r ]
.RB [ \-n
.IR level ]
@@ -28,6 +29,10 @@ file to whoever can debug their problem.
.B \-c
Clear the ring buffer contents after printing.
.TP
+.B \-f
+Output ring buffer contents as they are added. The output could be incomplete
+if another process (e.g. syslog daemon) is reading the kernel log buffer.
+.TP
.B \-r
Print the raw message buffer, i.e., don't strip the log level prefixes.
.TP
diff --git a/sys-utils/dmesg.c b/sys-utils/dmesg.c
index c3e5659..2f6b30e 100644
--- a/sys-utils/dmesg.c
+++ b/sys-utils/dmesg.c
@@ -40,38 +40,41 @@
#include "nls.h"
#include "strutils.h"
#include "xalloc.h"
+#include "writeall.h"
static void __attribute__ ((noreturn)) usage(void)
{
fprintf(stderr,
- _("Usage: %s [-c] [-n level] [-r] [-s bufsize]\n"),
+ _("Usage: %s [-c] [-f] [-n level] [-r] [-s bufsize]\n"),
program_invocation_short_name);
exit(EXIT_FAILURE);
-
}
int main(int argc, char *argv[])
{
char *buf = NULL;
- int sz;
+ int sz = 0;
int bufsize = 0;
- int i;
int n;
int c;
int level = 0;
- int lastc;
int cmd = 3; /* Read all messages in the ring buffer */
int raw = 0;
+ int follow = 0;
setlocale(LC_ALL, "");
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
- while ((c = getopt(argc, argv, "crn:s:")) != -1) {
+ while ((c = getopt(argc, argv, "cfrn:s:")) != -1) {
switch (c) {
case 'c':
cmd = 4; /* Read and clear all messages */
break;
+ case 'f':
+ cmd = 2;
+ follow = 1;
+ break;
case 'n':
cmd = 8; /* Set level of messages */
level = strtol_or_err(optarg, _("failed to parse level"));
@@ -105,46 +108,55 @@ int main(int argc, char *argv[])
if (!bufsize) {
n = klogctl(10, NULL, 0); /* read ringbuffer size */
- if (n > 0)
+ if (n > 0) {
bufsize = n;
+ sz = bufsize + 8;
+ buf = xmalloc(sz * sizeof(char));
+ }
}
- if (bufsize) {
- sz = bufsize + 8;
- buf = xmalloc(sz * sizeof(char));
- n = klogctl(cmd, buf, sz);
- } else {
- sz = 16392;
- while (1) {
- buf = xmalloc(sz * sizeof(char));
- n = klogctl(3, buf, sz); /* read only */
- if (n != sz || sz > (1 << 28))
- break;
+ do {
+ if (bufsize && sz)
+ n = klogctl(cmd, buf, sz);
+ else {
free(buf);
- sz *= 4;
+ sz = 16392;
+ while (1) {
+ buf = xmalloc(sz * sizeof(char));
+ n = klogctl(3, buf, sz); /* read only */
+ if (n != sz || sz > (1 << 28))
+ break;
+ free(buf);
+ sz *= 4;
+ }
+ if (n > 0 && cmd == 4)
+ n = klogctl(cmd, buf, sz); /* read and clear */
}
- if (n > 0 && cmd == 4)
- n = klogctl(cmd, buf, sz); /* read and clear */
- }
-
- if (n < 0)
- err(EXIT_FAILURE, _("klogctl failed"));
-
- lastc = '\n';
- for (i = 0; i < n; i++) {
- if (!raw && (i == 0 || buf[i - 1] == '\n') && buf[i] == '<') {
- i++;
- while (isdigit(buf[i]))
- i++;
- if (buf[i] == '>')
- i++;
+ if (n < 0)
+ err(EXIT_FAILURE, _("klogctl failed"));
+ if (raw)
+ write_all(STDIN_FILENO, buf, n);
+ else {
+ int i;
+
+ for (i = 0; i < n; i++) {
+ if ((i == 0 || buf[i - 1] == '\n') &&
+ buf[i] == '<') {
+ i++;
+ while (isdigit(buf[i]))
+ i++;
+ if (buf[i] == '>')
+ i++;
+ }
+ putchar(buf[i]);
+ }
}
- lastc = buf[i];
- putchar(lastc);
- }
- if (lastc != '\n')
+ } while (follow);
+
+ if (sz && n > 0 && buf && buf[n - 1] != '\n')
putchar('\n');
+
free(buf);
return EXIT_SUCCESS;
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH 1/1] dmesg: adds the ability to "follow" the klog ring buffer
From: Austen Dicken @ 2011-05-03 14:14 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
In-Reply-To: <20110503125322.GK29492@nb.net.home>
On Tue, May 3, 2011 at 07:53, Karel Zak <kzak@redhat.com> wrote:
> On Fri, Apr 29, 2011 at 11:42:00AM -0500, Austen Dicken wrote:
>> adds the "-f" option to dmesg, which allows an underpriviledged
>> user to "follow" the kernel ring buffer, outputting new messages as they
>
> I like the "follow the kernel ring buffer" idea, but ...
>
>> appear. this is done non-destructively so that the current buffer remains
>> after execution.
>>
>> this also changes the log output to be by-line instead of by-character,
>> and saves the last line printed on each read of the buffer. if looping,
>> it will then read the buffer again, but not output until it finds the last
>> line that it printed.
>
> ... this seems like a problem to me.
>
> [...]
>
>> - for (i = 0; i < n; i++) {
>> - if (!raw && (i == 0 || buf[i - 1] == '\n') && buf[i] == '<') {
>> - i++;
>> - while (isdigit(buf[i]))
>> - i++;
>> - if (buf[i] == '>')
>> + this_line = xmalloc(n * sizeof(char));
>> + for (i = 0, j = 0; i < n; i++) {
>> + if (!raw && (i == 0 || buf[i - 1] == '\n') && buf[i] == '<') {
>> i++;
>> + while (isdigit(buf[i]))
>> + i++;
>> + if (buf[i] == '>')
>> + i++;
>> + }
>> + this_line[j] = buf[i];
>> +
>> + if (this_line[j] == '\n') {
>> + this_line[j] = '\0';
>> + if (last_line) {
>> + if (!strcmp(last_line, this_line)) {
>> + free(last_line);
>> + last_line = NULL;
>> + }
>> + } else
>> + printf("%s\n", this_line);
>
> It expects that the kernel log does not contain duplicate messages.
> Is it correct? I don't think so, I see:
>
> ata1.00: configured for UDMA/100
> ata1.00: configured for UDMA/100
>
> You have to compile kernel with CONFIG_PRINTK_TIME, otherwise your
> heuristic will not work properly.
I see what you mean. Yes my heuristic would not be able to function properly
in that way, so there's no reason to complicate things by going to line-by-line
output.
>
>> + usleep(200);
>> +
>> + } while (follow_loop);
>
> Hmm... sleep() in code usually means a poor design or API.
I was using sleep at the time since klogctl(3,...) doesn't block so
when no new data
was available the code was essentially busy-looping, but I agree it is
quite messy.
>
> What about to support -f (follow) for privileged users only?
>
> Then you can use klogctl(2, ...). It waits until the kernel log buffer
> is non-empty, so the last_line and usleep() will be unnecessary, and
> the implementation will be pretty simple.
Agreed. I have decided to change the patch to function this way,
which simplifies things
immensely.
>
>> + if (prev_sighandler)
>> + signal(SIGINT, prev_sighandler);
>> + else
>> + signal(SIGINT, SIG_DFL);
>
> Why do you need to restore the handler before return (exit)? It seems
> unnecessary.
I was restoring the old signal handler just for completeness, in the
same way that someone
might free() memory even though they are about to exit. The main
reason for that is mostly
so that in the future if more were to be added to this program then
there wouldn't be bugs
caused by incomplete closure of such things.
Regardless, the use of signal handlers is obsolete in my latest patch,
shown below.
>
> Karel
>
> --
> Karel Zak <kzak@redhat.com>
> http://karelzak.blogspot.com
>
So I have changed the implementation to use klogctl(2, ...) and remove
the signal handlers,
so one simply interrupts the program for the sake of interrupting the
program, no cleanup should
be required, so the signal handling is just extra.
"dmesg -f" now essentially would do the same as "cat /proc/kmsg" but
with the addition of
some of the nice formatting options that dmesg already provides
(removal of log levels for instance).
I was playing with the idea of having follow run klogctl(3, ...) first
to dump the current buffer, then
call klogctl(2, ...) after that to append new information (essentially
producing the originally intended
output), but decided against it as if an underprivileged user attempts
to use the option that would result
in the log dumping and then failing when switching to klogctl(2, ...),
which seemed confusing and messy.
It could have been modified to run a privileged command first, but
that seemed messy as well, so I'll leave
it at this for now.
~ Austen
--
sys-utils/dmesg.1 | 4 +++
sys-utils/dmesg.c | 73 +++++++++++++++++++++++++++++------------------------
2 files changed, 44 insertions(+), 33 deletions(-)
diff --git a/sys-utils/dmesg.1 b/sys-utils/dmesg.1
index d7af1da..bbdd462 100644
--- a/sys-utils/dmesg.1
+++ b/sys-utils/dmesg.1
@@ -6,6 +6,7 @@ dmesg \- print or control the kernel ring buffer
.SH SYNOPSIS
.B dmesg
.RB [ \-c ]
+.RB [ \-f ]
.RB [ \-r ]
.RB [ \-n
.IR level ]
@@ -28,6 +29,9 @@ file to whoever can debug their problem.
.B \-c
Clear the ring buffer contents after printing.
.TP
+.B \-f
+Output ring buffer contents as they are added.
+.TP
.B \-r
Print the raw message buffer, i.e., don't strip the log level prefixes.
.TP
diff --git a/sys-utils/dmesg.c b/sys-utils/dmesg.c
index c3e5659..e2f5933 100644
--- a/sys-utils/dmesg.c
+++ b/sys-utils/dmesg.c
@@ -44,7 +44,7 @@
static void __attribute__ ((noreturn)) usage(void)
{
fprintf(stderr,
- _("Usage: %s [-c] [-n level] [-r] [-s bufsize]\n"),
+ _("Usage: %s [-c] [-f] [-n level] [-r] [-s bufsize]\n"),
program_invocation_short_name);
exit(EXIT_FAILURE);
@@ -62,16 +62,21 @@ int main(int argc, char *argv[])
int lastc;
int cmd = 3; /* Read all messages in the ring buffer */
int raw = 0;
+ int follow = 0;
setlocale(LC_ALL, "");
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
- while ((c = getopt(argc, argv, "crn:s:")) != -1) {
+ while ((c = getopt(argc, argv, "cfrn:s:")) != -1) {
switch (c) {
case 'c':
cmd = 4; /* Read and clear all messages */
break;
+ case 'f':
+ cmd = 2;
+ follow = 1;
+ break;
case 'n':
cmd = 8; /* Set level of messages */
level = strtol_or_err(optarg, _("failed to parse level"));
@@ -109,43 +114,45 @@ int main(int argc, char *argv[])
bufsize = n;
}
- if (bufsize) {
- sz = bufsize + 8;
- buf = xmalloc(sz * sizeof(char));
- n = klogctl(cmd, buf, sz);
- } else {
- sz = 16392;
- while (1) {
+ do {
+ if (bufsize) {
+ sz = bufsize + 8;
buf = xmalloc(sz * sizeof(char));
- n = klogctl(3, buf, sz); /* read only */
- if (n != sz || sz > (1 << 28))
- break;
- free(buf);
- sz *= 4;
+ n = klogctl(cmd, buf, sz);
+ } else {
+ sz = 16392;
+ while (1) {
+ buf = xmalloc(sz * sizeof(char));
+ n = klogctl(3, buf, sz); /* read only */
+ if (n != sz || sz > (1 << 28))
+ break;
+ free(buf);
+ sz *= 4;
+ }
+
+ if (n > 0 && cmd == 4)
+ n = klogctl(cmd, buf, sz); /* read and clear */
}
- if (n > 0 && cmd == 4)
- n = klogctl(cmd, buf, sz); /* read and clear */
- }
-
- if (n < 0)
- err(EXIT_FAILURE, _("klogctl failed"));
+ if (n < 0)
+ err(EXIT_FAILURE, _("klogctl failed"));
- lastc = '\n';
- for (i = 0; i < n; i++) {
- if (!raw && (i == 0 || buf[i - 1] == '\n') && buf[i] == '<') {
- i++;
- while (isdigit(buf[i]))
- i++;
- if (buf[i] == '>')
+ lastc = '\n';
+ for (i = 0; i < n; i++) {
+ if (!raw && (i == 0 || buf[i - 1] == '\n') && buf[i] == '<') {
i++;
+ while (isdigit(buf[i]))
+ i++;
+ if (buf[i] == '>')
+ i++;
+ }
+ lastc = buf[i];
+ putchar(lastc);
}
- lastc = buf[i];
- putchar(lastc);
- }
- if (lastc != '\n')
- putchar('\n');
- free(buf);
+ if (lastc != '\n')
+ putchar('\n');
+ free(buf);
+ } while (follow);
return EXIT_SUCCESS;
}
^ permalink raw reply related
* Re: [PATCH 1/1] dmesg: adds the ability to "follow" the klog ring buffer
From: Karel Zak @ 2011-05-03 12:53 UTC (permalink / raw)
To: Austen Dicken; +Cc: util-linux
In-Reply-To: <BANLkTin=y5oaEU0913dOnjWhosiyXe3Zwg@mail.gmail.com>
On Fri, Apr 29, 2011 at 11:42:00AM -0500, Austen Dicken wrote:
> adds the "-f" option to dmesg, which allows an underpriviledged
> user to "follow" the kernel ring buffer, outputting new messages as they
I like the "follow the kernel ring buffer" idea, but ...
> appear. this is done non-destructively so that the current buffer remains
> after execution.
>
> this also changes the log output to be by-line instead of by-character,
> and saves the last line printed on each read of the buffer. if looping,
> it will then read the buffer again, but not output until it finds the last
> line that it printed.
... this seems like a problem to me.
[...]
> - for (i = 0; i < n; i++) {
> - if (!raw && (i == 0 || buf[i - 1] == '\n') && buf[i] == '<') {
> - i++;
> - while (isdigit(buf[i]))
> - i++;
> - if (buf[i] == '>')
> + this_line = xmalloc(n * sizeof(char));
> + for (i = 0, j = 0; i < n; i++) {
> + if (!raw && (i == 0 || buf[i - 1] == '\n') && buf[i] == '<') {
> i++;
> + while (isdigit(buf[i]))
> + i++;
> + if (buf[i] == '>')
> + i++;
> + }
> + this_line[j] = buf[i];
> +
> + if (this_line[j] == '\n') {
> + this_line[j] = '\0';
> + if (last_line) {
> + if (!strcmp(last_line, this_line)) {
> + free(last_line);
> + last_line = NULL;
> + }
> + } else
> + printf("%s\n", this_line);
It expects that the kernel log does not contain duplicate messages.
Is it correct? I don't think so, I see:
ata1.00: configured for UDMA/100
ata1.00: configured for UDMA/100
You have to compile kernel with CONFIG_PRINTK_TIME, otherwise your
heuristic will not work properly.
> + usleep(200);
> +
> + } while (follow_loop);
Hmm... sleep() in code usually means a poor design or API.
What about to support -f (follow) for privileged users only?
Then you can use klogctl(2, ...). It waits until the kernel log buffer
is non-empty, so the last_line and usleep() will be unnecessary, and
the implementation will be pretty simple.
> + if (prev_sighandler)
> + signal(SIGINT, prev_sighandler);
> + else
> + signal(SIGINT, SIG_DFL);
Why do you need to restore the handler before return (exit)? It seems
unnecessary.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* [PATCH 1/1] dmesg: adds the ability to "follow" the klog ring buffer
From: Austen Dicken @ 2011-04-29 16:42 UTC (permalink / raw)
To: util-linux
adds the "-f" option to dmesg, which allows an underpriviledged
user to "follow" the kernel ring buffer, outputting new messages as they
appear. this is done non-destructively so that the current buffer remains
after execution.
this also changes the log output to be by-line instead of by-character,
and saves the last line printed on each read of the buffer. if looping,
it will then read the buffer again, but not output until it finds the last
line that it printed.
---
sys-utils/dmesg.1 | 4 ++
sys-utils/dmesg.c | 147 ++++++++++++++++++++++++++++++++++++++++------------
2 files changed, 117 insertions(+), 34 deletions(-)
diff --git a/sys-utils/dmesg.1 b/sys-utils/dmesg.1
index d7af1da..bbdd462 100644
--- a/sys-utils/dmesg.1
+++ b/sys-utils/dmesg.1
@@ -6,6 +6,7 @@ dmesg \- print or control the kernel ring buffer
.SH SYNOPSIS
.B dmesg
.RB [ \-c ]
+.RB [ \-f ]
.RB [ \-r ]
.RB [ \-n
.IR level ]
@@ -28,6 +29,9 @@ file to whoever can debug their problem.
.B \-c
Clear the ring buffer contents after printing.
.TP
+.B \-f
+Output ring buffer contents as they are added.
+.TP
.B \-r
Print the raw message buffer, i.e., don't strip the log level prefixes.
.TP
diff --git a/sys-utils/dmesg.c b/sys-utils/dmesg.c
index c3e5659..3342b80 100644
--- a/sys-utils/dmesg.c
+++ b/sys-utils/dmesg.c
@@ -35,6 +35,8 @@
#include <stdlib.h>
#include <sys/klog.h>
#include <ctype.h>
+#include <signal.h>
+#include <unistd.h>
#include "c.h"
#include "nls.h"
@@ -44,34 +46,55 @@
static void __attribute__ ((noreturn)) usage(void)
{
fprintf(stderr,
- _("Usage: %s [-c] [-n level] [-r] [-s bufsize]\n"),
+ _("Usage: %s [-c] [-f] [-n level] [-r] [-s bufsize]\n"),
program_invocation_short_name);
exit(EXIT_FAILURE);
}
+typedef void (*sighandler_t)(int);
+static int follow_loop = 0;
+static void follow_sighandler(int signum) {
+ switch (signum) {
+ case SIGINT:
+ if(follow_loop) {
+ printf("\n");
+ follow_loop = 0;
+ }
+ break;
+ default:
+ break;
+ }
+}
+
int main(int argc, char *argv[])
{
char *buf = NULL;
int sz;
int bufsize = 0;
- int i;
+ int i, j;
int n;
int c;
int level = 0;
- int lastc;
int cmd = 3; /* Read all messages in the ring buffer */
int raw = 0;
+ int follow = 0;
+ char* last_line = NULL;
+ char* this_line = NULL;
+ sighandler_t prev_sighandler;
setlocale(LC_ALL, "");
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
- while ((c = getopt(argc, argv, "crn:s:")) != -1) {
+ while ((c = getopt(argc, argv, "cfrn:s:")) != -1) {
switch (c) {
case 'c':
cmd = 4; /* Read and clear all messages */
break;
+ case 'f':
+ follow = 1;
+ break;
case 'n':
cmd = 8; /* Set level of messages */
level = strtol_or_err(optarg, _("failed to parse level"));
@@ -103,49 +126,105 @@ int main(int argc, char *argv[])
return EXIT_SUCCESS;
}
+ if (follow) {
+ cmd = 3; // override command to just print the buffer
+ follow_loop = 1;
+ prev_sighandler = signal(SIGINT, follow_sighandler);
+ }
+
if (!bufsize) {
n = klogctl(10, NULL, 0); /* read ringbuffer size */
if (n > 0)
bufsize = n;
}
- if (bufsize) {
- sz = bufsize + 8;
- buf = xmalloc(sz * sizeof(char));
- n = klogctl(cmd, buf, sz);
- } else {
- sz = 16392;
- while (1) {
+ do {
+ if (bufsize) {
+ sz = bufsize + 8;
buf = xmalloc(sz * sizeof(char));
- n = klogctl(3, buf, sz); /* read only */
- if (n != sz || sz > (1 << 28))
- break;
- free(buf);
- sz *= 4;
+ n = klogctl(cmd, buf, sz);
+ } else {
+ sz = 16392;
+ while (1) {
+ buf = xmalloc(sz * sizeof(char));
+ n = klogctl(3, buf, sz); /* read only */
+ if (n != sz || sz > (1 << 28))
+ break;
+ free(buf);
+ sz *= 4;
+ }
+
+ if (n > 0 && cmd == 4)
+ n = klogctl(cmd, buf, sz); /* read and clear */
}
- if (n > 0 && cmd == 4)
- n = klogctl(cmd, buf, sz); /* read and clear */
- }
-
- if (n < 0)
- err(EXIT_FAILURE, _("klogctl failed"));
+ if (n < 0)
+ err(EXIT_FAILURE, _("klogctl failed"));
- lastc = '\n';
- for (i = 0; i < n; i++) {
- if (!raw && (i == 0 || buf[i - 1] == '\n') && buf[i] == '<') {
- i++;
- while (isdigit(buf[i]))
- i++;
- if (buf[i] == '>')
+ this_line = xmalloc(n * sizeof(char));
+ for (i = 0, j = 0; i < n; i++) {
+ if (!raw && (i == 0 || buf[i - 1] == '\n') && buf[i] == '<') {
i++;
+ while (isdigit(buf[i]))
+ i++;
+ if (buf[i] == '>')
+ i++;
+ }
+ this_line[j] = buf[i];
+
+ if (this_line[j] == '\n') {
+ this_line[j] = '\0';
+ if (last_line) {
+ if (!strcmp(last_line, this_line)) {
+ free(last_line);
+ last_line = NULL;
+ }
+ } else
+ printf("%s\n", this_line);
+
+ j = 0;
+ } else
+ j++;
+ }
+
+ // if we have stale data to print, then print it!
+ if (j && !last_line) {
+ this_line[j] = '\0';
+ printf("%s\n", this_line);
+ }
+
+ free(buf);
+
+ if (last_line) {
+ /* we traversed the whole log and never found
+ * the last line, so we didn't output anything.
+ * we clear last_line now so that next time we
+ * print the whole log */
+ free(last_line);
+ last_line = NULL;
+
+ /* free this_line as well */
+ free(this_line);
+ this_line = NULL;
+ } else {
+ last_line = this_line;
+ this_line = NULL;
}
- lastc = buf[i];
- putchar(lastc);
+ usleep(200);
+
+ } while (follow_loop);
+
+ if (follow) {
+ if (last_line) {
+ free(last_line);
+ last_line = NULL;
+ }
+
+ if (prev_sighandler)
+ signal(SIGINT, prev_sighandler);
+ else
+ signal(SIGINT, SIG_DFL);
}
- if (lastc != '\n')
- putchar('\n');
- free(buf);
return EXIT_SUCCESS;
}
^ permalink raw reply related
* Re: Mangle functions
From: Karel Zak @ 2011-04-26 11:12 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: util-linux-ng
In-Reply-To: <alpine.LNX.2.01.1104231718050.16111@obet.zrqbmnf.qr>
On Sat, Apr 23, 2011 at 05:20:01PM +0200, Jan Engelhardt wrote:
> I see a mnt_mangle function provided by libmount, but it seems to be
> meant for *tab columns only. Is there also a function that mangles the
> passed string such that it can be used as an option value, i.e. encodes
> the set " ,\t\n\\"?
Not sure if I good understand. Do you have any example?
Do you mean mount options? The library uses the mnt_mangle functions
for all strings that are read/write to/from the *tab files. You don't
have to care about " \t\n\\".
The problem is comma (','), it should not be used in the strings --
exception are SELinux mount options where comma is acceptable if the
option uses quotes (e.g. context="aaa,bbb,ccc").
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Mangle functions
From: Jan Engelhardt @ 2011-04-23 15:20 UTC (permalink / raw)
To: kzak; +Cc: util-linux-ng
Hej,
I see a mnt_mangle function provided by libmount, but it seems to be
meant for *tab columns only. Is there also a function that mangles the
passed string such that it can be used as an option value, i.e. encodes
the set " ,\t\n\\"?
^ permalink raw reply
* Re: [PATCH] getopt: [getopt.c] add static qualifiers
From: Karel Zak @ 2011-04-11 10:37 UTC (permalink / raw)
To: Olivier Mengué; +Cc: Davidlohr Bueso, util-linux
In-Reply-To: <BANLkTikG9pi0ER0jPvZDGWRxWu9Kg1+jAQ@mail.gmail.com>
On Sat, Apr 09, 2011 at 10:17:58AM +0200, Olivier Mengué wrote:
> I've noticed my patch has been committed in
> 0a626987e244b6fa30c18cfa86d12cd80b3977e8.
> Thanks.
>
> Le 29 mars 2011 01:39, Davidlohr Bueso <dave@gnu.org> a écrit :
> >
> > On Sun, 2011-03-27 at 18:29 +0200, Olivier Mengué wrote:
> > > -void *our_malloc(size_t size);
> > > -void *our_realloc(void *ptr, size_t size);
> >
> > It would be nice to use our xalloc lib for stuff like this.
>
> Working on a patch for that.
> However, note that xalloc error messages are not localized contrary to
> the current implementation.
Hmm.. it should be probably fixed.
> > > -void print_help(void)
> > > +static void print_help(void)
> >
> > Lately we've been standardizing this kind of things too.
>
> A real cleanup of this function would require to also merge the
> multiple fputs() calls. This would have impact on localization.
> What is the policy on changes having impact on localization? (Please,
> add the answer to README.devel too).
Don't care about localization, the strings will be translated before
release.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: [PATCH] getopt: [getopt.c] add static qualifiers
From: Olivier Mengué @ 2011-04-09 8:17 UTC (permalink / raw)
To: Davidlohr Bueso; +Cc: util-linux
In-Reply-To: <1301355571.2073.1.camel@offworld>
I've noticed my patch has been committed in
0a626987e244b6fa30c18cfa86d12cd80b3977e8.
Thanks.
Le 29 mars 2011 01:39, Davidlohr Bueso <dave@gnu.org> a écrit :
>
> On Sun, 2011-03-27 at 18:29 +0200, Olivier Mengué wrote:
> > -void *our_malloc(size_t size);
> > -void *our_realloc(void *ptr, size_t size);
>
> It would be nice to use our xalloc lib for stuff like this.
Working on a patch for that.
However, note that xalloc error messages are not localized contrary to
the current implementation.
> > -void print_help(void)
> > +static void print_help(void)
>
> Lately we've been standardizing this kind of things too.
A real cleanup of this function would require to also merge the
multiple fputs() calls. This would have impact on localization.
What is the policy on changes having impact on localization? (Please,
add the answer to README.devel too).
Olivier.
^ permalink raw reply
* [PATCH] mkfs.minix: add -{1,2,3} options
From: Davidlohr Bueso @ 2011-04-07 19:43 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
To be applied on top of the die removal patch.
From: Davidlohr Bueso <dave@gnu.org>
Date: Thu, 7 Apr 2011 16:39:37 -0300
Subject: [PATCH] mkfs.minix: add -{1,2,3} options
Specify by intuitive arguments which fs version to create. For v3 an exit message is shown that the feature is not yet available.
This patch also gets rid of the version2 variable, replaced by 'fs_version'.
Signed-off-by: Davidlohr Bueso <dave@gnu.org>
---
disk-utils/mkfs.minix.c | 26 ++++++++++++++++----------
1 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/disk-utils/mkfs.minix.c b/disk-utils/mkfs.minix.c
index 53b5900..ff50d4d 100644
--- a/disk-utils/mkfs.minix.c
+++ b/disk-utils/mkfs.minix.c
@@ -88,7 +88,7 @@
#define INODE_SIZE (sizeof(struct minix_inode))
#define INODE_SIZE2 (sizeof(struct minix2_inode))
-#define INODE_BLOCKS UPPER(INODES, (version2 ? MINIX2_INODES_PER_BLOCK \
+#define INODE_BLOCKS UPPER(INODES, (fs_version == 2 ? MINIX2_INODES_PER_BLOCK \
: MINIX_INODES_PER_BLOCK))
#define INODE_BUFFER_SIZE (INODE_BLOCKS * BLOCK_SIZE)
@@ -106,7 +106,7 @@ static int namelen = 30; /* default (changed to 30, per Linus's
suggestion, Sun Nov 21 08:05:07 1993) */
static int dirsize = 32;
static int magic = MINIX_SUPER_MAGIC2;
-static int version2 = 0;
+static int fs_version = 1; /* v1 by default */
static char root_block[BLOCK_SIZE] = "\0";
@@ -118,7 +118,7 @@ static char *super_block_buffer;
static char boot_block_buffer[512];
#define Super (*(struct minix_super_block *)super_block_buffer)
#define INODES ((unsigned long)Super.s_ninodes)
-#define ZONES ((unsigned long)(version2 ? Super.s_zones : Super.s_nzones))
+#define ZONES ((unsigned long)(fs_version == 2 ? Super.s_zones : Super.s_nzones))
#define IMAPS ((unsigned long)Super.s_imap_blocks)
#define ZMAPS ((unsigned long)Super.s_zmap_blocks)
#define FIRSTZONE ((unsigned long)Super.s_firstdatazone)
@@ -392,8 +392,8 @@ setup_tables(void) {
memset(boot_block_buffer,0,512);
Super.s_magic = magic;
Super.s_log_zone_size = 0;
- Super.s_max_size = version2 ? 0x7fffffff : (7+512+512*512)*1024;
- if (version2)
+ Super.s_max_size = fs_version == 2 ? 0x7fffffff : (7+512+512*512)*1024;
+ if (fs_version == 2)
Super.s_zones = BLOCKS;
else
Super.s_nzones = BLOCKS;
@@ -404,7 +404,7 @@ setup_tables(void) {
else
inodes = req_nr_inodes;
/* Round up inode count to fill block size */
- if (version2)
+ if (fs_version == 2)
inodes = ((inodes + MINIX2_INODES_PER_BLOCK - 1) &
~(MINIX2_INODES_PER_BLOCK - 1));
else
@@ -570,7 +570,7 @@ main(int argc, char ** argv) {
errx(8, _("bad inode size"), device_name);
opterr = 0;
- while ((i = getopt(argc, argv, "ci:l:n:v")) != -1)
+ while ((i = getopt(argc, argv, "ci:l:n:v123")) != -1)
switch (i) {
case 'c':
check=1; break;
@@ -592,9 +592,15 @@ main(int argc, char ** argv) {
namelen = i;
dirsize = i+2;
break;
+ case '1':
+ fs_version = 1;
+ break;
+ case '2':
case 'v':
- version2 = 1;
+ fs_version = 2;
break;
+ case '3':
+ errx(8, _("cannot create mkfs v3 - unsupported feature"));
default:
usage();
}
@@ -654,7 +660,7 @@ main(int argc, char ** argv) {
errx(8, _("will not try to make filesystem on '%s'"), device_name);
if (BLOCKS < 10)
errx(8, _("number of blocks too small"), device_name);
- if (version2) {
+ if (fs_version == 2) {
if (namelen == 14)
magic = MINIX2_SUPER_MAGIC;
else
@@ -667,7 +673,7 @@ main(int argc, char ** argv) {
check_blocks();
else if (listfile)
get_list_blocks(listfile);
- if (version2) {
+ if (fs_version == 2) {
make_root_inode2 ();
make_bad_inode2 ();
} else {
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 3/3] mkfs.minix: add support for minix3 fs
From: Davidlohr Bueso @ 2011-04-06 14:22 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux-ng
In-Reply-To: <20110406083627.GA6491@nb.net.home>
On Wed, 2011-04-06 at 10:36 +0200, Karel Zak wrote:
> On Thu, Mar 31, 2011 at 11:59:38AM -0300, Davidlohr Bueso wrote:
> > #define INODE_SIZE (sizeof(struct minix_inode))
...
>
> Oh.. Davidlohr, you forgot regression test ;-) What about to add
>
> mkfs.minix -{1,2,3}
>
> to tests/ts/minix/mkfs{1,2,3} ?
>
> It would be also nice to add MINIX3_SUPER_MAGIC to fsck.minix and
> print a warning that v3 is unsupported (yet?).
I had a feeling you would dislike the sloppy code (which was already a
part of mkfs.minix), so heck, let's invade it massively, like partx.
Like you mention, we need to go through several patches, which I will be
sending you over the next few days, this way it might be a bit easier to
merge.
Thanks,
Davidlohr
^ permalink raw reply
* [PATCH 3/3] mkfs.minix: add support for minix3 fs
From: Davidlohr Bueso @ 2011-03-31 14:59 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux-ng
Add Minix filesystem v3 support. This version allows up to 60 character name files, instead of 30 for older versions.
We can also specify the block size to be used (with the new -B option). Additionally, three new options were added to
specify the fs version to be created (-1, -2, -3).
Signed-off-by: Davidlohr Bueso <dave@gnu.org>
---
disk-utils/Makefile.am | 2 +-
disk-utils/mkfs.minix.c | 351 ++++++++++++++++++++++++++++++-----------------
2 files changed, 223 insertions(+), 130 deletions(-)
diff --git a/disk-utils/Makefile.am b/disk-utils/Makefile.am
index 5df8064..cd07169 100644
--- a/disk-utils/Makefile.am
+++ b/disk-utils/Makefile.am
@@ -1,6 +1,6 @@
include $(top_srcdir)/config/include-Makefile.am
-utils_common = $(top_srcdir)/lib/blkdev.c
+utils_common = $(top_srcdir)/lib/blkdev.c $(top_srcdir)/lib/strutils.c
if LINUX
utils_common += $(top_srcdir)/lib/linux_version.c
endif
diff --git a/disk-utils/mkfs.minix.c b/disk-utils/mkfs.minix.c
index 807a571..c5f1ac6 100644
--- a/disk-utils/mkfs.minix.c
+++ b/disk-utils/mkfs.minix.c
@@ -46,13 +46,19 @@
* 02.07.96 - Added small patch from Russell King to make the program a
* good deal more portable (janl@math.uio.no)
*
- * Usage: mkfs [-c | -l filename ] [-v] [-nXX] [-iXX] device [size-in-blocks]
+ * 22.03.11 - Added support for v3 filesystem, -1,-2,-3,-B options
+ * and overall general cleanups.
+ * (Davidlohr Bueso, dave@gnu.org)
+ *
+ * Usage: mkfs [-c | -l filename ] [-v] [-1 | -2 | -3] [-nXX] [-iXX] device [size-in-blocks]
*
* -c for readablility checking (SLOW!)
* -l for getting a list of bad blocks from a file.
* -n for namelength (currently the kernel only uses 14 or 30)
* -i for number of inodes
* -v for v2 filesystem
+ * -B specify block size (v3)
+ * -1,-2,-3 indicating the filesystem version to be created
*
* The device may be a block device or a image of one, but this isn't
* enforced (but it's not much fun on a character device :-).
@@ -70,13 +76,14 @@
#include <sys/stat.h>
#include <mntent.h>
#include <getopt.h>
-#include <err.h>
#include "blkdev.h"
#include "minix.h"
#include "nls.h"
#include "pathnames.h"
#include "bitops.h"
+#include "strutils.h"
+#include "xalloc.h"
#define MINIX_ROOT_INO 1
#define MINIX_BAD_INO 2
@@ -88,13 +95,11 @@
#define INODE_SIZE (sizeof(struct minix_inode))
#define INODE_SIZE2 (sizeof(struct minix2_inode))
-#define INODE_BLOCKS UPPER(INODES, (version2 ? MINIX2_INODES_PER_BLOCK \
- : MINIX_INODES_PER_BLOCK))
-#define INODE_BUFFER_SIZE (INODE_BLOCKS * BLOCK_SIZE)
+#define INODE_BLOCKS UPPER(INODES, inodes_per_block)
-#define BITS_PER_BLOCK (BLOCK_SIZE<<3)
+#define INODE_BUFFER_SIZE (INODE_BLOCKS * BLOCK_SIZE)
-#define MAX_INODES 65535
+#define BITS_PER_BLOCK (blksz<<3)
static char * program_name = "mkfs";
static char * device_name = NULL;
@@ -102,11 +107,17 @@ static int DEV = -1;
static unsigned long long BLOCKS = 0;
static int check = 0;
static int badblocks = 0;
-static int namelen = 30; /* default (changed to 30, per Linus's
- suggestion, Sun Nov 21 08:05:07 1993) */
+static unsigned long long blksz = 0;
+
+/*
+ * default (changed to 30, per Linus's
+ * suggestion, Sun Nov 21 08:05:07 1993)
+ * This should be changed in the future to 60,
+ * since v3 needs to be the default nowadays (2011)
+ */
+static int namelen = 30;
static int dirsize = 32;
static int magic = MINIX_SUPER_MAGIC2;
-static int version2 = 0;
static char root_block[BLOCK_SIZE] = "\0";
@@ -114,16 +125,21 @@ static char * inode_buffer = NULL;
#define Inode (((struct minix_inode *) inode_buffer)-1)
#define Inode2 (((struct minix2_inode *) inode_buffer)-1)
+static int inodes_per_block = 0;
+static int fs_version = 1; /* v1 enabled by default */
+
static char *super_block_buffer;
static char boot_block_buffer[512];
+
#define Super (*(struct minix_super_block *)super_block_buffer)
-#define INODES ((unsigned long)Super.s_ninodes)
-#define ZONES ((unsigned long)(version2 ? Super.s_zones : Super.s_nzones))
-#define IMAPS ((unsigned long)Super.s_imap_blocks)
-#define ZMAPS ((unsigned long)Super.s_zmap_blocks)
-#define FIRSTZONE ((unsigned long)Super.s_firstdatazone)
-#define ZONESIZE ((unsigned long)Super.s_log_zone_size)
-#define MAXSIZE ((unsigned long)Super.s_max_size)
+#define Super3 (*(struct minix3_super_block *)super_block_buffer)
+#define INODES ((unsigned long)(fs_version == 3 ? Super3.s_ninodes : Super.s_ninodes))
+#define ZONES ((unsigned long)(fs_version == 3 ? Super3.s_zones : (fs_version == 2 ? Super.s_zones : Super.s_nzones)))
+#define IMAPS ((unsigned long)(fs_version == 3 ? Super3.s_imap_blocks : Super.s_imap_blocks))
+#define ZMAPS ((unsigned long)(fs_version == 3 ? Super3.s_zmap_blocks : Super.s_zmap_blocks))
+#define FIRSTZONE ((unsigned long) (fs_version == 3 ? Super3.s_firstdatazone : Super.s_firstdatazone))
+#define ZONESIZE ((unsigned long)(fs_version == 3 ? Super3.s_log_zone_size : Super.s_log_zone_size))
+#define MAXSIZE ((unsigned long)(fs_version == 3 ? Super3.s_max_size : Super.s_max_size))
#define NORM_FIRSTZONE (2+IMAPS+ZMAPS+INODE_BLOCKS)
static char *inode_map;
@@ -141,6 +157,8 @@ static unsigned long req_nr_inodes = 0;
#define mark_zone(x) (setbit(zone_map,(x)-FIRSTZONE+1))
#define unmark_zone(x) (clrbit(zone_map,(x)-FIRSTZONE+1))
+#define INODE_MAX 65535
+
static void
die(char *str) {
fprintf(stderr, "%s: ", program_name);
@@ -149,10 +167,13 @@ die(char *str) {
exit(8);
}
-static void __attribute__((__noreturn__))
+static void
usage(void) {
- errx(16, _("Usage: %s [-c | -l filename] [-nXX] [-iXX] /dev/name [blocks]"),
- program_name);
+ fprintf(stderr, _("%s (%s)\n"), program_name, PACKAGE_STRING);
+ fprintf(stderr,
+ _("Usage: %s [-c | -l filename] [-nXX] [-iXX] /dev/name [blocks]\n"),
+ program_name);
+ exit(16);
}
/*
@@ -177,23 +198,28 @@ check_mount(void) {
die(_("%s is mounted; will not make a filesystem here!"));
}
-static void
+static void
write_tables(void) {
/* Mark the super block valid. */
- Super.s_state |= MINIX_VALID_FS;
- Super.s_state &= ~MINIX_ERROR_FS;
-
+ if (fs_version == 3) {
+ Super3.s_state |= MINIX_VALID_FS;
+ Super3.s_state &= ~MINIX_ERROR_FS;
+ } else {
+ Super.s_state |= MINIX_VALID_FS;
+ Super.s_state &= ~MINIX_ERROR_FS;
+ }
+
if (lseek(DEV, 0, SEEK_SET))
die(_("seek to boot block failed in write_tables"));
if (512 != write(DEV, boot_block_buffer, 512))
die(_("unable to clear boot sector"));
- if (BLOCK_SIZE != lseek(DEV, BLOCK_SIZE, SEEK_SET))
+ if (blksz != lseek(DEV, blksz, SEEK_SET))
die(_("seek failed in write_tables"));
- if (BLOCK_SIZE != write(DEV, super_block_buffer, BLOCK_SIZE))
+ if (blksz != write(DEV, super_block_buffer, blksz))
die(_("unable to write super-block"));
- if (IMAPS*BLOCK_SIZE != write(DEV,inode_map,IMAPS*BLOCK_SIZE))
+ if (IMAPS*blksz != write(DEV,inode_map,IMAPS*blksz))
die(_("unable to write inode map"));
- if (ZMAPS*BLOCK_SIZE != write(DEV,zone_map,ZMAPS*BLOCK_SIZE))
+ if (ZMAPS*blksz != write(DEV,zone_map,ZMAPS*blksz))
die(_("unable to write zone map"));
if (INODE_BUFFER_SIZE != write(DEV,inode_buffer,INODE_BUFFER_SIZE))
die(_("unable to write inodes"));
@@ -202,9 +228,9 @@ write_tables(void) {
static void
write_block(int blk, char * buffer) {
- if (blk*BLOCK_SIZE != lseek(DEV, blk*BLOCK_SIZE, SEEK_SET))
+ if (blk*blksz != lseek(DEV, blk*blksz, SEEK_SET))
die(_("seek failed in write_block"));
- if (BLOCK_SIZE != write(DEV, buffer, BLOCK_SIZE))
+ if (blksz != write(DEV, buffer, blksz))
die(_("write failed in write_block"));
}
@@ -250,8 +276,8 @@ make_bad_inode(void) {
struct minix_inode * inode = &Inode[MINIX_BAD_INO];
int i,j,zone;
int ind=0,dind=0;
- unsigned short ind_block[BLOCK_SIZE>>1];
- unsigned short dind_block[BLOCK_SIZE>>1];
+ unsigned short ind_block[blksz>>1];
+ unsigned short dind_block[blksz>>1];
#define NEXT_BAD (zone = next(zone))
@@ -261,7 +287,7 @@ make_bad_inode(void) {
inode->i_nlinks = 1;
inode->i_time = time(NULL);
inode->i_mode = S_IFREG + 0000;
- inode->i_size = badblocks*BLOCK_SIZE;
+ inode->i_size = badblocks*blksz;
zone = next(0);
for (i=0 ; i<7 ; i++) {
inode->i_zone[i] = zone;
@@ -269,18 +295,18 @@ make_bad_inode(void) {
goto end_bad;
}
inode->i_zone[7] = ind = get_free_block();
- memset(ind_block,0,BLOCK_SIZE);
+ memset(ind_block,0,blksz);
for (i=0 ; i<512 ; i++) {
ind_block[i] = zone;
if (!NEXT_BAD)
goto end_bad;
}
inode->i_zone[8] = dind = get_free_block();
- memset(dind_block,0,BLOCK_SIZE);
+ memset(dind_block,0,blksz);
for (i=0 ; i<512 ; i++) {
write_block(ind,(char *) ind_block);
dind_block[i] = ind = get_free_block();
- memset(ind_block,0,BLOCK_SIZE);
+ memset(ind_block,0,blksz);
for (j=0 ; j<512 ; j++) {
ind_block[j] = zone;
if (!NEXT_BAD)
@@ -296,12 +322,13 @@ end_bad:
}
static void
-make_bad_inode2 (void) {
+make_bad_inode2 (void)
+{
struct minix2_inode *inode = &Inode2[MINIX_BAD_INO];
int i, j, zone;
int ind = 0, dind = 0;
- unsigned long ind_block[BLOCK_SIZE >> 2];
- unsigned long dind_block[BLOCK_SIZE >> 2];
+ unsigned long ind_block[blksz >> 2];
+ unsigned long dind_block[blksz >> 2];
if (!badblocks)
return;
@@ -309,7 +336,7 @@ make_bad_inode2 (void) {
inode->i_nlinks = 1;
inode->i_atime = inode->i_mtime = inode->i_ctime = time (NULL);
inode->i_mode = S_IFREG + 0000;
- inode->i_size = badblocks * BLOCK_SIZE;
+ inode->i_size = badblocks * blksz;
zone = next (0);
for (i = 0; i < 7; i++) {
inode->i_zone[i] = zone;
@@ -317,18 +344,18 @@ make_bad_inode2 (void) {
goto end_bad;
}
inode->i_zone[7] = ind = get_free_block ();
- memset (ind_block, 0, BLOCK_SIZE);
+ memset (ind_block, 0, blksz);
for (i = 0; i < 256; i++) {
ind_block[i] = zone;
if (!NEXT_BAD)
goto end_bad;
}
inode->i_zone[8] = dind = get_free_block ();
- memset (dind_block, 0, BLOCK_SIZE);
+ memset (dind_block, 0, blksz);
for (i = 0; i < 256; i++) {
write_block (ind, (char *) ind_block);
dind_block[i] = ind = get_free_block ();
- memset (ind_block, 0, BLOCK_SIZE);
+ memset (ind_block, 0, blksz);
for (j = 0; j < 256; j++) {
ind_block[j] = zone;
if (!NEXT_BAD)
@@ -337,7 +364,7 @@ make_bad_inode2 (void) {
}
/* Could make triple indirect block here */
die (_("too many bad blocks"));
- end_bad:
+end_bad:
if (ind)
write_block (ind, (char *) ind_block);
if (dind)
@@ -345,9 +372,13 @@ make_bad_inode2 (void) {
}
static void
-make_root_inode(void) {
+make_root_inode(void)
+{
struct minix_inode * inode = &Inode[MINIX_ROOT_INO];
+
+
+
mark_inode(MINIX_ROOT_INO);
inode->i_zone[0] = get_free_block();
inode->i_nlinks = 2;
@@ -363,126 +394,154 @@ make_root_inode(void) {
inode->i_uid = getuid();
if (inode->i_uid)
inode->i_gid = getgid();
- write_block(inode->i_zone[0],root_block);
+ write_block(inode->i_zone[0], root_block);
}
-static void
-make_root_inode2 (void) {
+static void
+make_root_inode2 (void)
+{
struct minix2_inode *inode = &Inode2[MINIX_ROOT_INO];
mark_inode (MINIX_ROOT_INO);
inode->i_zone[0] = get_free_block ();
inode->i_nlinks = 2;
inode->i_atime = inode->i_mtime = inode->i_ctime = time (NULL);
+
if (badblocks)
inode->i_size = 3 * dirsize;
else {
root_block[2 * dirsize] = '\0';
- root_block[2 * dirsize + 1] = '\0';
- inode->i_size = 2 * dirsize;
+ root_block[2 * dirsize] = '\0';
+ if (fs_version == 2)
+ inode->i_size = 2 * dirsize;
}
inode->i_mode = S_IFDIR + 0755;
inode->i_uid = getuid();
if (inode->i_uid)
inode->i_gid = getgid();
+
write_block (inode->i_zone[0], root_block);
}
-static void
-setup_tables(void) {
+/* sets up the boot sector, superblock and inode tables */
+static void
+setup_tables(void)
+{
int i;
unsigned long inodes;
- super_block_buffer = calloc(1, BLOCK_SIZE);
- if (!super_block_buffer)
- die(_("unable to alloc buffer for superblock"));
-
+ super_block_buffer = xcalloc(1, blksz);
memset(boot_block_buffer,0,512);
- Super.s_magic = magic;
- Super.s_log_zone_size = 0;
- Super.s_max_size = version2 ? 0x7fffffff : (7+512+512*512)*1024;
- if (version2)
+
+ if (fs_version == 3) {
+ Super3.s_magic = magic;
+ Super3.s_log_zone_size = 0;
+ Super3.s_blocksize = blksz;
+ }
+
+ else {
+ Super.s_magic = magic;
+ Super.s_log_zone_size = 0;
+ }
+
+ if (fs_version == 3)
+ Super3.s_max_size = 2147483647L;
+ else
+ Super.s_max_size = fs_version == 2 ? 0x7fffffff : (7+512+512*512)*1024;
+
+ if (fs_version == 3)
+ Super3.s_zones = BLOCKS;
+ else if (fs_version == 2)
Super.s_zones = BLOCKS;
else
Super.s_nzones = BLOCKS;
-/* some magic nrs: 1 inode / 3 blocks */
+ /* some magic nrs: 1 inode / 3 blocks */
if ( req_nr_inodes == 0 )
inodes = BLOCKS/3;
else
inodes = req_nr_inodes;
/* Round up inode count to fill block size */
- if (version2)
+ if (fs_version == 2 || fs_version == 3)
inodes = ((inodes + MINIX2_INODES_PER_BLOCK - 1) &
~(MINIX2_INODES_PER_BLOCK - 1));
else
inodes = ((inodes + MINIX_INODES_PER_BLOCK - 1) &
~(MINIX_INODES_PER_BLOCK - 1));
- if (inodes > 65535)
- inodes = 65535;
- Super.s_ninodes = inodes;
-
- /* The old code here
- * ZMAPS = 0;
- * while (ZMAPS != UPPER(BLOCKS - NORM_FIRSTZONE + 1,BITS_PER_BLOCK))
- * ZMAPS = UPPER(BLOCKS - NORM_FIRSTZONE + 1,BITS_PER_BLOCK);
- * was no good, since it may loop. - aeb
- */
- Super.s_imap_blocks = UPPER(INODES + 1, BITS_PER_BLOCK);
- Super.s_zmap_blocks = UPPER(BLOCKS - (1+IMAPS+INODE_BLOCKS),
- BITS_PER_BLOCK+1);
- Super.s_firstdatazone = NORM_FIRSTZONE;
-
- inode_map = malloc(IMAPS * BLOCK_SIZE);
- zone_map = malloc(ZMAPS * BLOCK_SIZE);
- if (!inode_map || !zone_map)
- die(_("unable to allocate buffers for maps"));
- memset(inode_map,0xff,IMAPS * BLOCK_SIZE);
- memset(zone_map,0xff,ZMAPS * BLOCK_SIZE);
+
+ if (fs_version == 3)
+ Super3.s_ninodes = inodes;
+ else {
+ if (inodes > INODE_MAX)
+ inodes = INODE_MAX;
+ Super.s_ninodes = inodes;
+ }
+
+ if (fs_version == 3)
+ Super3.s_imap_blocks = UPPER(INODES + 1, BITS_PER_BLOCK);
+ else
+ Super.s_imap_blocks = UPPER(INODES + 1, BITS_PER_BLOCK);
+
+ if (fs_version == 3)
+ Super3.s_zmap_blocks = UPPER(BLOCKS - (1+IMAPS+INODE_BLOCKS),
+ BITS_PER_BLOCK+1);
+ else
+ Super.s_zmap_blocks = UPPER(BLOCKS - (1+IMAPS+INODE_BLOCKS),
+ BITS_PER_BLOCK+1);
+
+ if (fs_version == 3)
+ Super3.s_firstdatazone = NORM_FIRSTZONE;
+ else
+ Super.s_firstdatazone = NORM_FIRSTZONE;
+
+ inode_map = xmalloc(IMAPS * blksz);
+ zone_map = xmalloc(ZMAPS * blksz);
+ memset(inode_map,0xff,IMAPS * blksz);
+ memset(zone_map,0xff,ZMAPS * blksz);
for (i = FIRSTZONE ; i<ZONES ; i++)
unmark_zone(i);
for (i = MINIX_ROOT_INO ; i<=INODES ; i++)
unmark_inode(i);
- inode_buffer = malloc(INODE_BUFFER_SIZE);
- if (!inode_buffer)
- die(_("unable to allocate buffer for inodes"));
+
+ inode_buffer = xmalloc(INODE_BUFFER_SIZE);
memset(inode_buffer,0,INODE_BUFFER_SIZE);
- printf(_("%ld inodes\n"),INODES);
- printf(_("%ld blocks\n"),ZONES);
- printf(_("Firstdatazone=%ld (%ld)\n"),FIRSTZONE,NORM_FIRSTZONE);
- printf(_("Zonesize=%d\n"),BLOCK_SIZE<<ZONESIZE);
- printf(_("Maxsize=%ld\n\n"),MAXSIZE);
+
+ printf(_("%lu inodes\n"), INODES);
+ printf(_("%ld blocks\n"), ZONES);
+ printf(_("Firstdatazone=%ld (%ld)\n"), FIRSTZONE, NORM_FIRSTZONE);
+ printf(_("Zonesize=%llu\n"), blksz<<ZONESIZE);
+ printf(_("Maxsize=%ld\n\n"), MAXSIZE);
}
/*
* Perform a test of a block; return the number of
* blocks readable/writeable.
*/
-static long
-do_check(char * buffer, int try, unsigned int current_block) {
+static long do_check(char * buffer, int try, unsigned int current_block)
+{
long got;
/* Seek to the correct loc. */
- if (lseek(DEV, current_block * BLOCK_SIZE, SEEK_SET) !=
- current_block * BLOCK_SIZE ) {
- die(_("seek failed during testing of blocks"));
+ if (lseek(DEV, current_block * blksz, SEEK_SET) !=
+ current_block * blksz ) {
+ die(_("seek failed during testing of blocks"));
}
/* Try the read */
- got = read(DEV, buffer, try * BLOCK_SIZE);
+ got = read(DEV, buffer, try * blksz);
if (got < 0) got = 0;
- if (got & (BLOCK_SIZE - 1 )) {
+ if (got & (blksz - 1 )) {
printf(_("Weird values in do_check: probably bugs\n"));
}
- got /= BLOCK_SIZE;
+ got /= blksz;
return got;
}
static unsigned int currently_testing = 0;
-static void
-alarm_intr(int alnum) {
+static void alarm_intr(int alnum)
+{
if (currently_testing >= ZONES)
return;
signal(SIGALRM,alarm_intr);
@@ -493,8 +552,8 @@ alarm_intr(int alnum) {
fflush(stdout);
}
-static void
-check_blocks(void) {
+static void check_blocks(void)
+{
int try,got;
static char buffer[BLOCK_SIZE * TEST_BUFFER_BLOCKS];
@@ -502,8 +561,8 @@ check_blocks(void) {
signal(SIGALRM,alarm_intr);
alarm(5);
while (currently_testing < ZONES) {
- if (lseek(DEV,currently_testing*BLOCK_SIZE,SEEK_SET) !=
- currently_testing*BLOCK_SIZE)
+ if (lseek(DEV,currently_testing*blksz,SEEK_SET) !=
+ currently_testing*blksz)
die(_("seek failed in check_blocks"));
try = TEST_BUFFER_BLOCKS;
if (currently_testing + try > ZONES)
@@ -549,13 +608,10 @@ get_list_blocks(char *filename) {
printf(_("one bad block\n"));
}
-int
-main(int argc, char ** argv) {
- int i;
- char * tmp;
+int main(int argc, char ** argv) {
struct stat statbuf;
- char * listfile = NULL;
- char * p;
+ char *tmp, *p, *listfile = NULL;
+ int i, sectorsize; /* physical sector size in bytes */
if (argc && *argv)
program_name = *argv;
@@ -572,13 +628,8 @@ main(int argc, char ** argv) {
exit(0);
}
- if (INODE_SIZE * MINIX_INODES_PER_BLOCK != BLOCK_SIZE)
- die(_("bad inode size"));
- if (INODE_SIZE2 * MINIX2_INODES_PER_BLOCK != BLOCK_SIZE)
- die(_("bad inode size"));
-
opterr = 0;
- while ((i = getopt(argc, argv, "ci:l:n:v")) != -1)
+ while ((i = getopt(argc, argv, "ci:l:n:v123B:")) != -1)
switch (i) {
case 'c':
check=1; break;
@@ -596,12 +647,22 @@ main(int argc, char ** argv) {
else if (i == 30)
magic = MINIX_SUPER_MAGIC2;
else
- usage();
+ die(_("length must be either 14 or 30"));
namelen = i;
dirsize = i+2;
break;
- case 'v':
- version2 = 1;
+ case '2':
+ case 'v': /* kept for backwards compatiblitly */
+ fs_version = 2;
+ break;
+ case '1':
+ fs_version = 1;
+ break;
+ case '3':
+ fs_version = 3;
+ break;
+ case 'B':
+ blksz = strtol_or_err(optarg, "failed to parse the block size");
break;
default:
usage();
@@ -621,9 +682,9 @@ main(int argc, char ** argv) {
}
}
- if (!device_name) {
+ if (!device_name)
usage();
- }
+
check_mount(); /* is it already mounted? */
tmp = root_block;
*(short *)tmp = 1;
@@ -633,6 +694,7 @@ main(int argc, char ** argv) {
strcpy(tmp+2,"..");
tmp += dirsize;
*(short *)tmp = 2;
+
strcpy(tmp+2,".badblocks");
if (stat(device_name, &statbuf) < 0)
die(_("unable to stat %s"));
@@ -643,8 +705,6 @@ main(int argc, char ** argv) {
if (DEV<0)
die(_("unable to open %s"));
if (S_ISBLK(statbuf.st_mode)) {
- int sectorsize;
-
if (blkdev_get_sector_size(DEV, §orsize) == -1)
die(_("cannot determine sector size for %s"));
if (BLOCK_SIZE < sectorsize)
@@ -660,29 +720,62 @@ main(int argc, char ** argv) {
check=0;
} else if (statbuf.st_rdev == 0x0300 || statbuf.st_rdev == 0x0340)
die(_("will not try to make filesystem on '%s'"));
+
+ /* determine what block size to use, based on fs version and user input */
+ if (blksz && fs_version == 3) {
+ if (blksz%sectorsize || blksz < BLOCK_SIZE) {
+ fprintf(stderr, "block size must be multiple of sector (%d) "
+ "and at least %d bytes\n", sectorsize, BLOCK_SIZE);
+ die(_("specified block size illegal"));
+ }
+ if(blksz%INODE_SIZE2) {
+ fprintf(stderr, "block size must be a multiple of inode size (%lu bytes)\n",
+ INODE_SIZE2);
+ die(_("specified block size illegal"));
+ }
+ }
+ else /* use the default block size */
+ blksz = BLOCK_SIZE;
+
+ if (fs_version == 2 || fs_version == 3) {
+ inodes_per_block = ((blksz)/(sizeof(struct minix2_inode)));
+ if (INODE_SIZE2 * inodes_per_block != blksz)
+ die(_("bad inode size"));
+ }
+ else {
+ inodes_per_block = ((blksz)/(sizeof(struct minix_inode)));
+ if (INODE_SIZE * inodes_per_block != blksz)
+ die(_("bad inode size"));
+
+ }
+
if (BLOCKS < 10)
die(_("number of blocks too small"));
- if (version2) {
+
+ if (fs_version == 3)
+ magic = MINIX3_SUPER_MAGIC;
+ else if (fs_version == 2) {
if (namelen == 14)
magic = MINIX2_SUPER_MAGIC;
else
magic = MINIX2_SUPER_MAGIC2;
} else
- if (BLOCKS > MAX_INODES)
- BLOCKS = MAX_INODES;
+ if (BLOCKS > 65535)
+ BLOCKS = 65535;
setup_tables();
if (check)
check_blocks();
else if (listfile)
get_list_blocks(listfile);
- if (version2) {
+
+ if (fs_version == 2 || fs_version == 3) {
make_root_inode2 ();
make_bad_inode2 ();
} else {
make_root_inode();
make_bad_inode();
}
-
+
mark_good_blocks();
write_tables();
close(DEV);
--
1.7.1
^ permalink raw reply related
* [PATCH 2/3] mkfs.minix: document mfs3 changes
From: Davidlohr Bueso @ 2011-03-31 14:59 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux-ng
Update the mkfs.minix manpage for new options and amount of characters for a filename.
Signed-off-by: Davidlohr Bueso <dave@gnu.org>
---
disk-utils/mkfs.minix.8 | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/disk-utils/mkfs.minix.8 b/disk-utils/mkfs.minix.8
index 9359106..da503e0 100644
--- a/disk-utils/mkfs.minix.8
+++ b/disk-utils/mkfs.minix.8
@@ -1,4 +1,4 @@
-.\" Copyright 1992, 1993, 1994 Rickard E. Faith (faith@cs.unc.edu)
+.\" Copyright 1992, 1993, 1994 Rickard E. Faith (faith@cs.unc.edu) - updated by Davidlohr Bueso <dave@gnu.og> 2011.
.\" May be freely distributed.
.TH MKFS.MINIX 8 "2 July 1996" "Util-linux 2.6" "Linux System Administrator's Manual"
.SH NAME
@@ -46,12 +46,12 @@ are found, the count is printed.
.TP
.BI \-n " namelength"
Specify the maximum length of filenames.
-Currently, the only allowable values are 14 and 30.
+Currently, the only allowable values are 14, 30 and 60 (v3 specific).
The default is 30. Note that kernels older than 0.99p7
only accept namelength 14.
.TP
.BI \-i " inodecount"
-Specify the number of inodes for the filesystem.
+ Specify the number of inodes for the filesystem.
.TP
.BI \-l " filename"
Read the bad blocks list from
@@ -59,8 +59,17 @@ Read the bad blocks list from
The file has one bad block number per line. The count of bad blocks read
is printed.
.TP
-.B \-v
+.B \-1
+Make a Minix version 1 filesystem.
+.TP
+.B \-2, \-v
Make a Minix version 2 filesystem.
+.TP
+.B \-3
+Make a Minix version 3 filesystem.
+.TP
+.BI \-B " blocksize"
+Specify the block size to use (only for v3)
.SH "EXIT CODES"
The exit code returned by
.B mkfs.minix
--
1.7.1
^ permalink raw reply related
* [PATCH 1/3] mkfs.minix: add support for mkfs3
From: Davidlohr Bueso @ 2011-03-31 14:59 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux-ng
Add superblock layout for minix3 filesystem support.
Signed-off-by: Davidlohr Bueso <dave@gnu.org>
---
disk-utils/minix.h | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/disk-utils/minix.h b/disk-utils/minix.h
index 638565e..8952792 100644
--- a/disk-utils/minix.h
+++ b/disk-utils/minix.h
@@ -44,6 +44,24 @@ struct minix_super_block {
u32 s_zones;
};
+/* V3 minix super-block data on disk */
+struct minix3_super_block {
+ u32 s_ninodes;
+ u16 s_pad0;
+ u16 s_imap_blocks;
+ u16 s_zmap_blocks;
+ u16 s_firstdatazone;
+ u16 s_log_zone_size;
+ u16 s_pad1;
+ u32 s_max_size;
+ u32 s_zones;
+ u16 s_magic;
+ u16 s_pad2;
+ u16 s_blocksize;
+ u8 s_disk_version;
+ u16 s_state;
+};
+
#define BLOCK_SIZE_BITS 10
#define BLOCK_SIZE (1<<BLOCK_SIZE_BITS)
@@ -59,5 +77,6 @@ struct minix_super_block {
#define MINIX_SUPER_MAGIC2 0x138F /* minix fs, 30 char names */
#define MINIX2_SUPER_MAGIC 0x2468 /* minix V2 fs */
#define MINIX2_SUPER_MAGIC2 0x2478 /* minix V2 fs, 30 char names */
+#define MINIX3_SUPER_MAGIC 0x4d5a /* minix V3 fs (60 char names) */
#endif /* KERNEL_INCLUDES_ARE_CLEAN */
--
1.7.1
^ permalink raw reply related
* [RFC PATCH 0/3] mkfs.minix: add v3 support
From: Davidlohr Bueso @ 2011-03-31 14:59 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux-ng
Hi,
The following patches add support for the minix3 filesystem in
mkfs.minix. I have been able to successfully try this out on different
disks, loading different types of files (images, text, video, etc.). I
have also verified that stat(2) works fine on this and the regression
tests for util-linux pass. Since I am not a fs expert please try this
out thoroughly and let me know your opinion. Despite the ugliness of the
code (for years now), I tried not to be too invasive and not rewrite
large parts of it.
With this new version we can
* have up to 60 characters for filenames.
* specify the block size to be used.
Additionally, four options were added:
* -1, -2, -3 to specify what version of the fs to create
* -B to specify the block size
Thanks,
Davidlohr
^ permalink raw reply
* Re: [PATCH] umount: allow non-root umount of FUSE even if not in fstab
From: Karel Zak @ 2011-03-29 10:36 UTC (permalink / raw)
To: Petr Uzel; +Cc: util-linux, Miklos Szeredi
In-Reply-To: <20110329084623.GC26819@foxbat.suse.cz>
On Tue, Mar 29, 2011 at 10:46:23AM +0200, Petr Uzel wrote:
> > I believe more and more that we need something like
> >
> > /etc/mount.d/{fuse,foo,...}.conf
> >
> > where we can define such behavior for some filesystems.
>
> Hm, I fail to understand what (regarding to this 'issue') would
> you configure there. I can imagine e.g. per-fs-type default mount
> options or enable/disable 'user mounts' (-i) per fs-type.
>
> Is this what you think that should be configured via /etc/mount.d/*,
> or something else?
The mount(8) and umount(8) are very strict about user mounts and about
the way how it calls /sbin/[u]mount helpers. It was fine many years
ago, but now we have things like pam_mount, fuse, HAL/udisks where
mount/umount stuff is managed in a different way. It means that we
need exceptions... and exceptions suck.
I think about:
$ cat /etc/mount.d/fuse.conf
[usermount]
enabled = true
require_fstab_entry = false
require_helper = true
and yes, things like per-fs-type default mount options is another
possibility.
Maybe this is something what should be added to the libmount.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: [PATCH] umount: allow non-root umount of FUSE even if not in fstab
From: Karel Zak @ 2011-03-29 10:13 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Petr Uzel, util-linux
In-Reply-To: <1301388524.22846.86.camel@tucsk.pomaz.szeredi.hu>
On Tue, Mar 29, 2011 at 10:48:44AM +0200, Miklos Szeredi wrote:
> On Tue, 2011-03-29 at 10:28 +0200, Karel Zak wrote:
> > On Mon, Mar 28, 2011 at 04:23:20PM +0200, Miklos Szeredi wrote:
> > > On Mon, 2011-03-28 at 16:04 +0200, Petr Uzel wrote:
> > > > Hi all,
> > > >
> > > > I hacked the following patch with which it is possible to use
> > > > "umount $dir" instead of "fusermount -u $dir", which IMHO is an
> > > > improvement in usability. It seems to work (at least for me), however,
> > > > I have to admit that I don't like it very much, because:
> > > > - it complicates umount
> > > > - duplicates code from fusermount
> > >
> > > And this is not the only one that would have to be duplicated. The
> > > mount and umount races that were fixed in fusermount in recently and not
> > > so recently would also have to be added to util-linux, which would
> > > actually be a good thing, since in theory they could affect fstab based
> > > user mounts as well (though that is much more unlikely than with fuse,
> > > where the user chooses the mountpoint).
> >
> > Maybe we need to call umount2() with UMOUNT_NOFOLLOW flag for
> > non-root users in umount(8). I think it should be enough for
> > umount(8) (where almost all is controlled by system admin in fstab).
> >
> > See below. Comments?
>
> UMOUNT_NOFOLLOW is a good idea but not enough, it will only deal with
> last path component changing to a symlink. If previous path component
> is changed then UMOUNT_NOFOLLOW will not have any effect.
>
> What fusermount does is change cwd to the parent directory, check if cwd
> matches that of the intended path, and then umount with UMOUNT_NOFOLLOW.
Ah.. I see the code. Thanks.
It would be nice to have
mountat(dirfd, ...)
umountat(dirfd, ...)
syscalls to avoid any races :-)
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: [PATCH] umount: allow non-root umount of FUSE even if not in fstab
From: Karel Zak @ 2011-03-29 10:03 UTC (permalink / raw)
To: Petr Uzel; +Cc: Miklos Szeredi, util-linux
In-Reply-To: <20110329085601.GD26819@foxbat.suse.cz>
On Tue, Mar 29, 2011 at 10:56:01AM +0200, Petr Uzel wrote:
> On Tue, Mar 29, 2011 at 10:51:30AM +0200, Miklos Szeredi wrote:
> > On Tue, 2011-03-29 at 10:46 +0200, Petr Uzel wrote:
> > > > Wouldn't be better to always call /sbin/umount.fuse for non-roots (except
> > > > umount -i)?
> > >
> > > Yes, that's what I meant by umount helpers. The problem is that AFAICS
> > > fuse does not provide this helper.
> > >
> > > @Miklos: what do you think? Would it make sense to have umount.fuse
> > > (most likely as symlink to /sbin/mount.fuse)? I could look into it.
> >
> > Could umount.fuse allow umount of fuse filesystem even if not in fstab?
By default umount(8) requires an entry in /etc/fstab. This behaviour
could be changed by uhelper= mount option (the option has to be in
mtab). The solution based on uhelper= also sucks, because it requires
userspace specific mount option...
This is reason why I think that /etc/mount.d/fuse.conf would be better
than add an exception to the code or use uhelper= mount option. The
same problem we have with HAL (udisks) where non-root user stuff is
not in fstab.
> > If so, then I don't see any problem with providing /sbin/umount.fuse.
>
> OK, I'll try to look into it.
It will require the exception for fuse.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
* Re: [PATCH] umount: allow non-root umount of FUSE even if not in fstab
From: Petr Uzel @ 2011-03-29 8:56 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Karel Zak, util-linux
In-Reply-To: <1301388690.22846.88.camel@tucsk.pomaz.szeredi.hu>
[-- Attachment #1: Type: text/plain, Size: 810 bytes --]
On Tue, Mar 29, 2011 at 10:51:30AM +0200, Miklos Szeredi wrote:
> On Tue, 2011-03-29 at 10:46 +0200, Petr Uzel wrote:
> > > Wouldn't be better to always call /sbin/umount.fuse for non-roots (except
> > > umount -i)?
> >
> > Yes, that's what I meant by umount helpers. The problem is that AFAICS
> > fuse does not provide this helper.
> >
> > @Miklos: what do you think? Would it make sense to have umount.fuse
> > (most likely as symlink to /sbin/mount.fuse)? I could look into it.
>
> Could umount.fuse allow umount of fuse filesystem even if not in fstab?
Yes, as far as I understand.
> If so, then I don't see any problem with providing /sbin/umount.fuse.
OK, I'll try to look into it.
>
> Thanks,
> Miklos
Thanks,
Petr
--
Petr Uzel
IRC: ptr_uzl @ freenode
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] umount: allow non-root umount of FUSE even if not in fstab
From: Miklos Szeredi @ 2011-03-29 8:51 UTC (permalink / raw)
To: Petr Uzel; +Cc: Karel Zak, util-linux
In-Reply-To: <20110329084623.GC26819@foxbat.suse.cz>
On Tue, 2011-03-29 at 10:46 +0200, Petr Uzel wrote:
> > Wouldn't be better to always call /sbin/umount.fuse for non-roots (except
> > umount -i)?
>
> Yes, that's what I meant by umount helpers. The problem is that AFAICS
> fuse does not provide this helper.
>
> @Miklos: what do you think? Would it make sense to have umount.fuse
> (most likely as symlink to /sbin/mount.fuse)? I could look into it.
Could umount.fuse allow umount of fuse filesystem even if not in fstab?
If so, then I don't see any problem with providing /sbin/umount.fuse.
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH] umount: allow non-root umount of FUSE even if not in fstab
From: Miklos Szeredi @ 2011-03-29 8:48 UTC (permalink / raw)
To: Karel Zak; +Cc: Petr Uzel, util-linux
In-Reply-To: <20110329082850.GR21093@nb.net.home>
On Tue, 2011-03-29 at 10:28 +0200, Karel Zak wrote:
> On Mon, Mar 28, 2011 at 04:23:20PM +0200, Miklos Szeredi wrote:
> > On Mon, 2011-03-28 at 16:04 +0200, Petr Uzel wrote:
> > > Hi all,
> > >
> > > I hacked the following patch with which it is possible to use
> > > "umount $dir" instead of "fusermount -u $dir", which IMHO is an
> > > improvement in usability. It seems to work (at least for me), however,
> > > I have to admit that I don't like it very much, because:
> > > - it complicates umount
> > > - duplicates code from fusermount
> >
> > And this is not the only one that would have to be duplicated. The
> > mount and umount races that were fixed in fusermount in recently and not
> > so recently would also have to be added to util-linux, which would
> > actually be a good thing, since in theory they could affect fstab based
> > user mounts as well (though that is much more unlikely than with fuse,
> > where the user chooses the mountpoint).
>
> Maybe we need to call umount2() with UMOUNT_NOFOLLOW flag for
> non-root users in umount(8). I think it should be enough for
> umount(8) (where almost all is controlled by system admin in fstab).
>
> See below. Comments?
UMOUNT_NOFOLLOW is a good idea but not enough, it will only deal with
last path component changing to a symlink. If previous path component
is changed then UMOUNT_NOFOLLOW will not have any effect.
What fusermount does is change cwd to the parent directory, check if cwd
matches that of the intended path, and then umount with UMOUNT_NOFOLLOW.
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH] umount: allow non-root umount of FUSE even if not in fstab
From: Petr Uzel @ 2011-03-29 8:46 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux, Miklos Szeredi
In-Reply-To: <20110328220344.GQ21093@nb.net.home>
[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]
On Tue, Mar 29, 2011 at 12:03:44AM +0200, Karel Zak wrote:
> On Mon, Mar 28, 2011 at 04:04:56PM +0200, Petr Uzel wrote:
> > I have to admit that I don't like it very much, because:
>
> I don't like it too :-)
I'm not surprised :)
>
> > - it complicates umount
> > - duplicates code from fusermount
> > - should (???) be implemented using umount helpers
>
> [...]
>
> > + /* If this is fuse-based filesystem, allow user unmount even
> > + * if the FS is not in fstab.
> > + *
> > + * Based on fusermount code. */
> > + if (strcmp(mc->m.mnt_type, "fuse") == 0 ||
> > + strcmp(mc->m.mnt_type, "fuseblk") == 0 ||
> > + strncmp(mc->m.mnt_type, "fuse.", 5) == 0 ||
> > + strncmp(mc->m.mnt_type, "fuseblk.", 8) == 0) {
>
> Wouldn't be better to always call /sbin/umount.fuse for non-roots (except
> umount -i)?
Yes, that's what I meant by umount helpers. The problem is that AFAICS
fuse does not provide this helper.
@Miklos: what do you think? Would it make sense to have umount.fuse
(most likely as symlink to /sbin/mount.fuse)? I could look into it.
>
> I believe more and more that we need something like
>
> /etc/mount.d/{fuse,foo,...}.conf
>
> where we can define such behavior for some filesystems.
Hm, I fail to understand what (regarding to this 'issue') would
you configure there. I can imagine e.g. per-fs-type default mount
options or enable/disable 'user mounts' (-i) per fs-type.
Is this what you think that should be configured via /etc/mount.d/*,
or something else?
Thanks,
Petr
--
Petr Uzel
IRC: ptr_uzl @ freenode
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] umount: allow non-root umount of FUSE even if not in fstab
From: Karel Zak @ 2011-03-29 8:28 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Petr Uzel, util-linux
In-Reply-To: <1301322200.22846.68.camel@tucsk.pomaz.szeredi.hu>
On Mon, Mar 28, 2011 at 04:23:20PM +0200, Miklos Szeredi wrote:
> On Mon, 2011-03-28 at 16:04 +0200, Petr Uzel wrote:
> > Hi all,
> >
> > I hacked the following patch with which it is possible to use
> > "umount $dir" instead of "fusermount -u $dir", which IMHO is an
> > improvement in usability. It seems to work (at least for me), however,
> > I have to admit that I don't like it very much, because:
> > - it complicates umount
> > - duplicates code from fusermount
>
> And this is not the only one that would have to be duplicated. The
> mount and umount races that were fixed in fusermount in recently and not
> so recently would also have to be added to util-linux, which would
> actually be a good thing, since in theory they could affect fstab based
> user mounts as well (though that is much more unlikely than with fuse,
> where the user chooses the mountpoint).
Maybe we need to call umount2() with UMOUNT_NOFOLLOW flag for
non-root users in umount(8). I think it should be enough for
umount(8) (where almost all is controlled by system admin in fstab).
See below. Comments?
Karel
>From 5cf67485d23dc4547eb5e54cbe96cc60837e36af Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@redhat.com>
Date: Tue, 29 Mar 2011 10:19:56 +0200
Subject: [PATCH] umount: use UMOUNT_NOFOLLOW for non-root users
Signed-off-by: Karel Zak <kzak@redhat.com>
---
mount/umount.c | 44 ++++++++++++++++++++++++++++++++++++--------
1 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/mount/umount.c b/mount/umount.c
index 42671f4..0660b20 100644
--- a/mount/umount.c
+++ b/mount/umount.c
@@ -45,15 +45,22 @@ umount2(const char *path, int flags) {
}
#endif /* __NR_umount2 */
-#if !defined(MNT_FORCE)
-/* dare not try to include <linux/mount.h> -- lots of errors */
-#define MNT_FORCE 1
+#ifndef MNT_FORCE
+# define MNT_FORCE 0x00000001 /* Attempt to forcibily umount */
#endif
#endif /* MNT_FORCE */
-#if !defined(MNT_DETACH)
-#define MNT_DETACH 2
+#ifndef MNT_DETACH
+# define MNT_DETACH 0x00000002 /* Just detach from the tree */
+#endif
+
+#ifndef UMOUNT_NOFOLLOW
+# define UMOUNT_NOFOLLOW 0x00000008 /* Don't follow symlink on umount */
+#endif
+
+#ifndef UMOUNT_UNUSED
+#define UMOUNT_UNUSED 0x80000000 /* Flag guaranteed to be unused */
#endif
@@ -197,6 +204,21 @@ static void complain(int err, const char *dev) {
}
}
+/* Check whether the kernel supports UMOUNT_NOFOLLOW flag */
+static int umount_nofollow_support(void)
+{
+ int res = umount2("", UMOUNT_UNUSED);
+ if (res != -1 || errno != EINVAL)
+ return 0;
+
+ res = umount2("", UMOUNT_NOFOLLOW);
+ if (res != -1 || errno != ENOENT)
+ return 0;
+
+ return 1;
+}
+
+
/* Umount a single device. Return a status code, so don't exit
on a non-fatal error. We lock/unlock around each umount. */
static int
@@ -206,6 +228,7 @@ umount_one (const char *spec, const char *node, const char *type,
int isroot;
int res = 0;
int status;
+ int extra_flags = 0;
const char *loopdev;
int myloop = 0;
@@ -237,15 +260,18 @@ umount_one (const char *spec, const char *node, const char *type,
if (delloop && is_loop_device(spec))
myloop = 1;
+ if (restricted && umount_nofollow_support())
+ extra_flags |= UMOUNT_NOFOLLOW;
+
if (lazy) {
- res = umount2 (node, MNT_DETACH);
+ res = umount2 (node, MNT_DETACH | extra_flags);
if (res < 0)
umnt_err = errno;
goto writemtab;
}
if (force) { /* only supported for NFS */
- res = umount2 (node, MNT_FORCE);
+ res = umount2 (node, MNT_FORCE | extra_flags);
if (res == -1) {
int errsv = errno;
perror("umount2");
@@ -256,7 +282,9 @@ umount_one (const char *spec, const char *node, const char *type,
res = umount (node);
}
}
- } else
+ } else if (extra_flags)
+ res = umount2 (node, extra_flags);
+ else
res = umount (node);
if (res < 0)
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] umount: allow non-root umount of FUSE even if not in fstab
From: Petr Uzel @ 2011-03-29 8:08 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: util-linux
In-Reply-To: <1301322200.22846.68.camel@tucsk.pomaz.szeredi.hu>
[-- Attachment #1: Type: text/plain, Size: 1340 bytes --]
On Mon, Mar 28, 2011 at 04:23:20PM +0200, Miklos Szeredi wrote:
> On Mon, 2011-03-28 at 16:04 +0200, Petr Uzel wrote:
> > Hi all,
> >
> > I hacked the following patch with which it is possible to use
> > "umount $dir" instead of "fusermount -u $dir", which IMHO is an
> > improvement in usability. It seems to work (at least for me), however,
> > I have to admit that I don't like it very much, because:
> > - it complicates umount
> > - duplicates code from fusermount
>
> And this is not the only one that would have to be duplicated. The
> mount and umount races that were fixed in fusermount in recently and not
> so recently would also have to be added to util-linux, which would
> actually be a good thing, since in theory they could affect fstab based
> user mounts as well (though that is much more unlikely than with fuse,
> where the user chooses the mountpoint).
Do you mean this commit:
8b3a0c74a15e237eb4b7053774600f0ce3fff403
Fix race if two "fusermount -u" instances are run in parallel.
?
> > - should (???) be implemented using umount helpers
>
> The end goal is to implement permission checking for unprivileged mount
> and umount in the kernel.
OK, that sounds reasonable. Not something I'd have guts to look into,
though :)
Petr
--
Petr Uzel
IRC: ptr_uzl @ freenode
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] umount: allow non-root umount of FUSE even if not in fstab
From: Karel Zak @ 2011-03-28 22:03 UTC (permalink / raw)
To: Petr Uzel; +Cc: util-linux, Miklos Szeredi
In-Reply-To: <20110328140454.GA4790@foxbat.suse.cz>
On Mon, Mar 28, 2011 at 04:04:56PM +0200, Petr Uzel wrote:
> I have to admit that I don't like it very much, because:
I don't like it too :-)
> - it complicates umount
> - duplicates code from fusermount
> - should (???) be implemented using umount helpers
[...]
> + /* If this is fuse-based filesystem, allow user unmount even
> + * if the FS is not in fstab.
> + *
> + * Based on fusermount code. */
> + if (strcmp(mc->m.mnt_type, "fuse") == 0 ||
> + strcmp(mc->m.mnt_type, "fuseblk") == 0 ||
> + strncmp(mc->m.mnt_type, "fuse.", 5) == 0 ||
> + strncmp(mc->m.mnt_type, "fuseblk.", 8) == 0) {
Wouldn't be better to always call /sbin/umount.fuse for non-roots (except
umount -i)?
I believe more and more that we need something like
/etc/mount.d/{fuse,foo,...}.conf
where we can define such behavior for some filesystems.
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox