* [PATCH 0/4] btrfs-progs: improve output of btrfs subvolume list command
@ 2015-10-02 16:41 axel
2015-10-02 16:41 ` [PATCH 1/4] btrfs-progs: add -A option for subvolume list (print all available information) axel
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: axel @ 2015-10-02 16:41 UTC (permalink / raw)
To: linux-btrfs; +Cc: Axel Burri
Improvements to "subvolume list" command, with the goal to improve
machine-readability. While the first three patches simply add new
options (-A and --time-format), the last patch [PATCH 4/4] is more
intrusive and changes the output format of the '-t' option. Consider
it "my view of how tabular output of a linux command should look
like" (see also comments on the commit).
Axel Burri (4):
btrfs-progs: add -A option for subvolume list (print all available
information)
btrfs-progs: add "flags" column for subvolume list (shows "readonly"
flag with -A)
btrfs-progs: add option "--time-format=short|iso|unix|locale" to
subvolume list
btrfs-progs: change -t option for subvolume list to print a simple
space-separated table (making it machine-readable)
Documentation/btrfs-subvolume.asciidoc | 5 ++
btrfs-list.c | 119 +++++++++++++++++++++++----------
btrfs-list.h | 2 +
cmds-subvolume.c | 16 ++++-
4 files changed, 104 insertions(+), 38 deletions(-)
--
2.4.9
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] btrfs-progs: add -A option for subvolume list (print all available information)
2015-10-02 16:41 [PATCH 0/4] btrfs-progs: improve output of btrfs subvolume list command axel
@ 2015-10-02 16:41 ` axel
2015-10-02 16:41 ` [PATCH 2/4] btrfs-progs: add "flags" column for subvolume list (shows "readonly" flag with -A) axel
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: axel @ 2015-10-02 16:41 UTC (permalink / raw)
To: linux-btrfs; +Cc: Axel Burri
Signed-off-by: Axel Burri <axel@tty0.ch>
---
Documentation/btrfs-subvolume.asciidoc | 2 ++
cmds-subvolume.c | 6 +++++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc b/Documentation/btrfs-subvolume.asciidoc
index c187fd8..afbec83 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -114,6 +114,8 @@ print the UUID of the subvolume.
print the parent uuid of subvolumes (and snapshots).
-R::::
print the UUID of the sent subvolume, where the subvolume is the result of a receive operation
+-A::::
+print all available subvolume information.
-t::::
print the result as a table.
-s::::
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 82173c0..26a08f1 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -428,6 +428,7 @@ static const char * const cmd_subvol_list_usage[] = {
"-u print the uuid of subvolumes (and snapshots)",
"-q print the parent uuid of the snapshots",
"-R print the uuid of the received snapshots",
+ "-A print all available subvolume information",
"-t print the result as a table",
"-s list snapshots only in the filesystem",
"-r list readonly subvolumes (including snapshots)",
@@ -471,7 +472,7 @@ static int cmd_subvol_list(int argc, char **argv)
};
c = getopt_long(argc, argv,
- "acdgopqsurRG:C:t", long_options, NULL);
+ "acdgopqsurRAG:C:t", long_options, NULL);
if (c < 0)
break;
@@ -515,6 +516,9 @@ static int cmd_subvol_list(int argc, char **argv)
case 'R':
btrfs_list_setup_print_column(BTRFS_LIST_RUUID);
break;
+ case 'A':
+ btrfs_list_setup_print_column(BTRFS_LIST_ALL);
+ break;
case 'r':
flags |= BTRFS_ROOT_SUBVOL_RDONLY;
break;
--
2.4.9
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] btrfs-progs: add "flags" column for subvolume list (shows "readonly" flag with -A)
2015-10-02 16:41 [PATCH 0/4] btrfs-progs: improve output of btrfs subvolume list command axel
2015-10-02 16:41 ` [PATCH 1/4] btrfs-progs: add -A option for subvolume list (print all available information) axel
@ 2015-10-02 16:41 ` axel
2015-10-02 16:41 ` [PATCH 3/4] btrfs-progs: add option "--time-format=short|iso|unix|locale" to subvolume list axel
2015-10-02 16:41 ` [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable) axel
3 siblings, 0 replies; 15+ messages in thread
From: axel @ 2015-10-02 16:41 UTC (permalink / raw)
To: linux-btrfs; +Cc: Axel Burri
Signed-off-by: Axel Burri <axel@tty0.ch>
---
btrfs-list.c | 14 ++++++++++++++
btrfs-list.h | 1 +
2 files changed, 15 insertions(+)
diff --git a/btrfs-list.c b/btrfs-list.c
index 7529e11..ff337f9 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -80,6 +80,11 @@ static struct {
.need_print = 0,
},
{
+ .name = "flags",
+ .column_name = "Flags",
+ .need_print = 0,
+ },
+ {
.name = "parent_uuid",
.column_name = "Parent UUID",
.need_print = 0,
@@ -1388,6 +1393,15 @@ static void print_subvolume_column(struct root_info *subv,
uuid_unparse(subv->ruuid, uuidparse);
printf("%s", uuidparse);
break;
+ case BTRFS_LIST_FLAGS:
+ if (subv->flags == 0) {
+ printf("-");
+ } else {
+ /* comma-separated list of all available flags */
+ if(subv->flags & BTRFS_ROOT_SUBVOL_RDONLY)
+ printf("readonly");
+ }
+ break;
case BTRFS_LIST_PATH:
BUG_ON(!subv->full_path);
printf("%s", subv->full_path);
diff --git a/btrfs-list.h b/btrfs-list.h
index 13f44c3..397eb3e 100644
--- a/btrfs-list.h
+++ b/btrfs-list.h
@@ -119,6 +119,7 @@ enum btrfs_list_column_enum {
BTRFS_LIST_PARENT,
BTRFS_LIST_TOP_LEVEL,
BTRFS_LIST_OTIME,
+ BTRFS_LIST_FLAGS,
BTRFS_LIST_PUUID,
BTRFS_LIST_RUUID,
BTRFS_LIST_UUID,
--
2.4.9
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] btrfs-progs: add option "--time-format=short|iso|unix|locale" to subvolume list
2015-10-02 16:41 [PATCH 0/4] btrfs-progs: improve output of btrfs subvolume list command axel
2015-10-02 16:41 ` [PATCH 1/4] btrfs-progs: add -A option for subvolume list (print all available information) axel
2015-10-02 16:41 ` [PATCH 2/4] btrfs-progs: add "flags" column for subvolume list (shows "readonly" flag with -A) axel
@ 2015-10-02 16:41 ` axel
2015-10-02 16:41 ` [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable) axel
3 siblings, 0 replies; 15+ messages in thread
From: axel @ 2015-10-02 16:41 UTC (permalink / raw)
To: linux-btrfs; +Cc: Axel Burri
Affects time format of "otime". Supporting unix time and ISO8601 makes
the output of "subvolume list" machine-readable.
Default is "short", keeping default output as it was before (I suggest
changing the default to "iso", as with "short" is it not clear to the
user if localtime or gmtime is printed, and it does not contain
whitespace).
Signed-off-by: Axel Burri <axel@tty0.ch>
---
Documentation/btrfs-subvolume.asciidoc | 3 ++
btrfs-list.c | 65 +++++++++++++++++++++++++++++-----
btrfs-list.h | 1 +
cmds-subvolume.c | 10 ++++++
4 files changed, 70 insertions(+), 9 deletions(-)
diff --git a/Documentation/btrfs-subvolume.asciidoc b/Documentation/btrfs-subvolume.asciidoc
index afbec83..b4a8c68 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -137,6 +137,9 @@ you can add \'\+' or \'-' in front of each items, \'+' means ascending,
for --sort you can combine some items together by \',', just like
-sort=+ogen,-gen,path,rootid.
+--time-format=short|iso|unix|locale::::
+print times in format: short, ISO 8601, unix (seconds), or current locale.
+
*set-default* <id> <path>::
Set the subvolume of the filesystem <path> which is mounted as
default.
diff --git a/btrfs-list.c b/btrfs-list.c
index ff337f9..f8396e7 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -114,6 +114,15 @@ static struct {
static btrfs_list_filter_func all_filter_funcs[];
static btrfs_list_comp_func all_comp_funcs[];
+enum btrfs_list_time_format_enum {
+ BTRFS_LIST_TIME_FORMAT_SHORT,
+ BTRFS_LIST_TIME_FORMAT_ISO,
+ BTRFS_LIST_TIME_FORMAT_UNIX,
+ BTRFS_LIST_TIME_FORMAT_LOCALE,
+};
+
+static enum btrfs_list_time_format_enum btrfs_list_time_format = BTRFS_LIST_TIME_FORMAT_SHORT;
+
void btrfs_list_setup_print_column(enum btrfs_list_column_enum column)
{
int i;
@@ -1338,10 +1347,35 @@ static int __list_subvol_fill_paths(int fd, struct root_lookup *root_lookup)
return 0;
}
+static void print_timestamp(time_t time)
+{
+ char tstr[256];
+ struct tm tm;
+
+ switch(btrfs_list_time_format) {
+ case BTRFS_LIST_TIME_FORMAT_SHORT:
+ localtime_r(&time, &tm);
+ strftime(tstr, 256, "%Y-%m-%d %X", &tm);
+ break;
+ case BTRFS_LIST_TIME_FORMAT_ISO:
+ localtime_r(&time, &tm);
+ strftime(tstr, 256, "%FT%T%z", &tm);
+ break;
+ case BTRFS_LIST_TIME_FORMAT_UNIX:
+ gmtime_r(&time, &tm);
+ strftime(tstr, 256, "%s", &tm);
+ break;
+ case BTRFS_LIST_TIME_FORMAT_LOCALE:
+ localtime_r(&time, &tm);
+ strftime(tstr, 256, "%c", &tm);
+ break;
+ }
+ printf("%s", tstr);
+}
+
static void print_subvolume_column(struct root_info *subv,
enum btrfs_list_column_enum column)
{
- char tstr[256];
char uuidparse[BTRFS_UUID_UNPARSED_SIZE];
BUG_ON(column >= BTRFS_LIST_ALL || column < 0);
@@ -1363,14 +1397,10 @@ static void print_subvolume_column(struct root_info *subv,
printf("%llu", subv->top_id);
break;
case BTRFS_LIST_OTIME:
- if (subv->otime) {
- struct tm tm;
-
- localtime_r(&subv->otime, &tm);
- strftime(tstr, 256, "%Y-%m-%d %X", &tm);
- } else
- strcpy(tstr, "-");
- printf("%s", tstr);
+ if (subv->otime)
+ print_timestamp(subv->otime);
+ else
+ printf("-");
break;
case BTRFS_LIST_UUID:
if (uuid_is_null(subv->uuid))
@@ -1918,6 +1948,23 @@ int btrfs_list_parse_filter_string(char *opt_arg,
return 0;
}
+int btrfs_list_parse_time_format(const char *format)
+{
+ if (strcmp(format, "short") == 0)
+ btrfs_list_time_format = BTRFS_LIST_TIME_FORMAT_SHORT;
+ else if (strcmp(format, "unix") == 0)
+ btrfs_list_time_format = BTRFS_LIST_TIME_FORMAT_UNIX;
+ else if (strcmp(format, "locale") == 0)
+ btrfs_list_time_format = BTRFS_LIST_TIME_FORMAT_LOCALE;
+ else if (strcmp(optarg, "iso") == 0)
+ btrfs_list_time_format = BTRFS_LIST_TIME_FORMAT_ISO;
+ else {
+ fprintf(stderr, "Unknown time format %s\n", format);
+ return 1;
+ };
+ return 0;
+}
+
int btrfs_list_get_path_rootid(int fd, u64 *treeid)
{
int ret;
diff --git a/btrfs-list.h b/btrfs-list.h
index 397eb3e..dd2eebf 100644
--- a/btrfs-list.h
+++ b/btrfs-list.h
@@ -159,6 +159,7 @@ int btrfs_list_parse_sort_string(char *optarg,
int btrfs_list_parse_filter_string(char *optarg,
struct btrfs_list_filter_set **filters,
enum btrfs_list_filter_enum type);
+int btrfs_list_parse_time_format(const char *format);
void btrfs_list_setup_print_column(enum btrfs_list_column_enum column);
struct btrfs_list_filter_set *btrfs_list_alloc_filter_set(void);
void btrfs_list_free_filter_set(struct btrfs_list_filter_set *filter_set);
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 26a08f1..fefefac 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -443,6 +443,8 @@ static const char * const cmd_subvol_list_usage[] = {
" list the subvolume in order of gen, ogen, rootid or path",
" you also can add '+' or '-' in front of each items.",
" (+:ascending, -:descending, ascending default)",
+ "--time-format=short|iso|unix|locale",
+ " print timestamps in given format",
NULL,
};
@@ -468,6 +470,7 @@ static int cmd_subvol_list(int argc, char **argv)
int c;
static const struct option long_options[] = {
{"sort", required_argument, NULL, 'S'},
+ {"time-format", required_argument, NULL, 'T'},
{NULL, 0, NULL, 0}
};
@@ -551,6 +554,13 @@ static int cmd_subvol_list(int argc, char **argv)
goto out;
}
break;
+ case 'T':
+ ret = btrfs_list_parse_time_format(optarg);
+ if (ret) {
+ uerr = 1;
+ goto out;
+ }
+ break;
default:
uerr = 1;
--
2.4.9
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable)
2015-10-02 16:41 [PATCH 0/4] btrfs-progs: improve output of btrfs subvolume list command axel
` (2 preceding siblings ...)
2015-10-02 16:41 ` [PATCH 3/4] btrfs-progs: add option "--time-format=short|iso|unix|locale" to subvolume list axel
@ 2015-10-02 16:41 ` axel
2015-10-03 9:56 ` Goffredo Baroncelli
3 siblings, 1 reply; 15+ messages in thread
From: axel @ 2015-10-02 16:41 UTC (permalink / raw)
To: linux-btrfs; +Cc: Axel Burri
Old implementation used tabs "\t", and tried to work around problems
by guessing amount of tabs needed (e.g. "\t\t" after top level", with
buggy output as soon as empty uuids are printed). This will never work
correctly, as tab width is a user-defined setting in the terminal.
Keep it simple and don't reimplement the wheel, for nice tabular
output we have the "column" command from util-linux:
btrfs subvolume list -t <path> | column -t
Signed-off-by: Axel Burri <axel@tty0.ch>
---
btrfs-list.c | 40 ++++++++++++----------------------------
1 file changed, 12 insertions(+), 28 deletions(-)
diff --git a/btrfs-list.c b/btrfs-list.c
index f8396e7..c09257a 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -45,12 +45,12 @@ struct root_lookup {
};
static struct {
- char *name;
+ char *name; /* machine-readable column identifier: [a-z_]+ */
char *column_name;
int need_print;
} btrfs_list_columns[] = {
{
- .name = "ID",
+ .name = "id",
.column_name = "ID",
.need_print = 0,
},
@@ -70,7 +70,7 @@ static struct {
.need_print = 0,
},
{
- .name = "top level",
+ .name = "top_level",
.column_name = "Top Level",
.need_print = 0,
},
@@ -1465,13 +1465,10 @@ static void print_single_volume_info_table(struct root_info *subv)
if (!btrfs_list_columns[i].need_print)
continue;
- print_subvolume_column(subv, i);
-
- if (i != BTRFS_LIST_PATH)
- printf("\t");
+ if (i != 0)
+ printf(" ");
- if (i == BTRFS_LIST_TOP_LEVEL)
- printf("\t");
+ print_subvolume_column(subv, i);
}
printf("\n");
}
@@ -1496,30 +1493,17 @@ static void print_single_volume_info_default(struct root_info *subv)
static void print_all_volume_info_tab_head(void)
{
int i;
- int len;
- char barrier[20];
-
- for (i = 0; i < BTRFS_LIST_ALL; i++) {
- if (btrfs_list_columns[i].need_print)
- printf("%s\t", btrfs_list_columns[i].name);
-
- if (i == BTRFS_LIST_ALL-1)
- printf("\n");
- }
for (i = 0; i < BTRFS_LIST_ALL; i++) {
- memset(barrier, 0, sizeof(barrier));
+ if (!btrfs_list_columns[i].need_print)
+ continue;
- if (btrfs_list_columns[i].need_print) {
- len = strlen(btrfs_list_columns[i].name);
- while (len--)
- strcat(barrier, "-");
+ if (i != 0)
+ printf(" ");
- printf("%s\t", barrier);
- }
- if (i == BTRFS_LIST_ALL-1)
- printf("\n");
+ printf(btrfs_list_columns[i].name);
}
+ printf("\n");
}
static void print_all_volume_info(struct root_lookup *sorted_tree,
--
2.4.9
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable)
2015-10-02 16:41 ` [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable) axel
@ 2015-10-03 9:56 ` Goffredo Baroncelli
2015-10-03 10:06 ` Goffredo Baroncelli
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Goffredo Baroncelli @ 2015-10-03 9:56 UTC (permalink / raw)
To: linux-btrfs
On 2015-10-02 18:41, axel@tty0.ch wrote:
> Old implementation used tabs "\t", and tried to work around problems
> by guessing amount of tabs needed (e.g. "\t\t" after top level", with
> buggy output as soon as empty uuids are printed). This will never work
> correctly, as tab width is a user-defined setting in the terminal.
Why not use string_table() and table_*() functions ?
>
> Keep it simple and don't reimplement the wheel, for nice tabular
> output we have the "column" command from util-linux:
>
> btrfs subvolume list -t <path> | column -t
>
> Signed-off-by: Axel Burri <axel@tty0.ch>
> ---
> btrfs-list.c | 40 ++++++++++++----------------------------
> 1 file changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/btrfs-list.c b/btrfs-list.c
> index f8396e7..c09257a 100644
> --- a/btrfs-list.c
> +++ b/btrfs-list.c
> @@ -45,12 +45,12 @@ struct root_lookup {
> };
>
> static struct {
> - char *name;
> + char *name; /* machine-readable column identifier: [a-z_]+ */
> char *column_name;
> int need_print;
> } btrfs_list_columns[] = {
> {
> - .name = "ID",
> + .name = "id",
> .column_name = "ID",
> .need_print = 0,
> },
> @@ -70,7 +70,7 @@ static struct {
> .need_print = 0,
> },
> {
> - .name = "top level",
> + .name = "top_level",
> .column_name = "Top Level",
> .need_print = 0,
> },
> @@ -1465,13 +1465,10 @@ static void print_single_volume_info_table(struct root_info *subv)
> if (!btrfs_list_columns[i].need_print)
> continue;
>
> - print_subvolume_column(subv, i);
> -
> - if (i != BTRFS_LIST_PATH)
> - printf("\t");
> + if (i != 0)
> + printf(" ");
>
> - if (i == BTRFS_LIST_TOP_LEVEL)
> - printf("\t");
> + print_subvolume_column(subv, i);
> }
> printf("\n");
> }
> @@ -1496,30 +1493,17 @@ static void print_single_volume_info_default(struct root_info *subv)
> static void print_all_volume_info_tab_head(void)
> {
> int i;
> - int len;
> - char barrier[20];
> -
> - for (i = 0; i < BTRFS_LIST_ALL; i++) {
> - if (btrfs_list_columns[i].need_print)
> - printf("%s\t", btrfs_list_columns[i].name);
> -
> - if (i == BTRFS_LIST_ALL-1)
> - printf("\n");
> - }
>
> for (i = 0; i < BTRFS_LIST_ALL; i++) {
> - memset(barrier, 0, sizeof(barrier));
> + if (!btrfs_list_columns[i].need_print)
> + continue;
>
> - if (btrfs_list_columns[i].need_print) {
> - len = strlen(btrfs_list_columns[i].name);
> - while (len--)
> - strcat(barrier, "-");
> + if (i != 0)
> + printf(" ");
>
> - printf("%s\t", barrier);
> - }
> - if (i == BTRFS_LIST_ALL-1)
> - printf("\n");
> + printf(btrfs_list_columns[i].name);
> }
> + printf("\n");
> }
>
> static void print_all_volume_info(struct root_lookup *sorted_tree,
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable)
2015-10-03 9:56 ` Goffredo Baroncelli
@ 2015-10-03 10:06 ` Goffredo Baroncelli
2015-10-03 10:17 ` Axel Burri
[not found] ` <560FA944.3050606@digint.ch>
2 siblings, 0 replies; 15+ messages in thread
From: Goffredo Baroncelli @ 2015-10-03 10:06 UTC (permalink / raw)
To: axel; +Cc: linux-btrfs
On 2015-10-03 11:56, Goffredo Baroncelli wrote:
> On 2015-10-02 18:41, axel@tty0.ch wrote:
>> Old implementation used tabs "\t", and tried to work around problems
>> by guessing amount of tabs needed (e.g. "\t\t" after top level", with
>> buggy output as soon as empty uuids are printed). This will never work
>> correctly, as tab width is a user-defined setting in the terminal.
>
>
> Why not use string_table() and table_*() functions ?
Hem sorry, .... create_table() and table_*() functions (see string-table.h)
[...]
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable)
2015-10-03 9:56 ` Goffredo Baroncelli
2015-10-03 10:06 ` Goffredo Baroncelli
@ 2015-10-03 10:17 ` Axel Burri
[not found] ` <560FA944.3050606@digint.ch>
2 siblings, 0 replies; 15+ messages in thread
From: Axel Burri @ 2015-10-03 10:17 UTC (permalink / raw)
To: linux-btrfs
On 2015-10-03 11:56, Goffredo Baroncelli wrote:
> On 2015-10-02 18:41, axel@tty0.ch wrote:
>> Old implementation used tabs "\t", and tried to work around problems
>> by guessing amount of tabs needed (e.g. "\t\t" after top level", with
>> buggy output as soon as empty uuids are printed). This will never work
>> correctly, as tab width is a user-defined setting in the terminal.
>
>
> Why not use string_table() and table_*() functions ?
string_table(), as well as all table functions by nature, needs to know
the maximum size of all cells in a row before printing, and therefore
buffers all the output before printing. It would eat up a lot of memory
for large tables (it is not unusual to have 1000+ subvolumes in btrfs if
you make heavy use of snapshotting). Furthermore, it would slow down
things by not printing the output linewise.
>
>>
>> Keep it simple and don't reimplement the wheel, for nice tabular
>> output we have the "column" command from util-linux:
>>
>> btrfs subvolume list -t <path> | column -t
>>
>> Signed-off-by: Axel Burri <axel@tty0.ch>
>> ---
>> btrfs-list.c | 40 ++++++++++++----------------------------
>> 1 file changed, 12 insertions(+), 28 deletions(-)
>>
>> diff --git a/btrfs-list.c b/btrfs-list.c
>> index f8396e7..c09257a 100644
>> --- a/btrfs-list.c
>> +++ b/btrfs-list.c
>> @@ -45,12 +45,12 @@ struct root_lookup {
>> };
>>
>> static struct {
>> - char *name;
>> + char *name; /* machine-readable column identifier: [a-z_]+ */
>> char *column_name;
>> int need_print;
>> } btrfs_list_columns[] = {
>> {
>> - .name = "ID",
>> + .name = "id",
>> .column_name = "ID",
>> .need_print = 0,
>> },
>> @@ -70,7 +70,7 @@ static struct {
>> .need_print = 0,
>> },
>> {
>> - .name = "top level",
>> + .name = "top_level",
>> .column_name = "Top Level",
>> .need_print = 0,
>> },
>> @@ -1465,13 +1465,10 @@ static void print_single_volume_info_table(struct root_info *subv)
>> if (!btrfs_list_columns[i].need_print)
>> continue;
>>
>> - print_subvolume_column(subv, i);
>> -
>> - if (i != BTRFS_LIST_PATH)
>> - printf("\t");
>> + if (i != 0)
>> + printf(" ");
>>
>> - if (i == BTRFS_LIST_TOP_LEVEL)
>> - printf("\t");
>> + print_subvolume_column(subv, i);
>> }
>> printf("\n");
>> }
>> @@ -1496,30 +1493,17 @@ static void print_single_volume_info_default(struct root_info *subv)
>> static void print_all_volume_info_tab_head(void)
>> {
>> int i;
>> - int len;
>> - char barrier[20];
>> -
>> - for (i = 0; i < BTRFS_LIST_ALL; i++) {
>> - if (btrfs_list_columns[i].need_print)
>> - printf("%s\t", btrfs_list_columns[i].name);
>> -
>> - if (i == BTRFS_LIST_ALL-1)
>> - printf("\n");
>> - }
>>
>> for (i = 0; i < BTRFS_LIST_ALL; i++) {
>> - memset(barrier, 0, sizeof(barrier));
>> + if (!btrfs_list_columns[i].need_print)
>> + continue;
>>
>> - if (btrfs_list_columns[i].need_print) {
>> - len = strlen(btrfs_list_columns[i].name);
>> - while (len--)
>> - strcat(barrier, "-");
>> + if (i != 0)
>> + printf(" ");
>>
>> - printf("%s\t", barrier);
>> - }
>> - if (i == BTRFS_LIST_ALL-1)
>> - printf("\n");
>> + printf(btrfs_list_columns[i].name);
>> }
>> + printf("\n");
>> }
>>
>> static void print_all_volume_info(struct root_lookup *sorted_tree,
>>
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable)
[not found] ` <560FA944.3050606@digint.ch>
@ 2015-10-03 17:41 ` Goffredo Baroncelli
2015-10-04 3:37 ` Duncan
0 siblings, 1 reply; 15+ messages in thread
From: Goffredo Baroncelli @ 2015-10-03 17:41 UTC (permalink / raw)
To: Axel Burri; +Cc: linux-btrfs
On 2015-10-03 12:09, Axel Burri wrote:
>
>
> On 2015-10-03 11:56, Goffredo Baroncelli wrote:
>> On 2015-10-02 18:41, axel@tty0.ch wrote:
>>> Old implementation used tabs "\t", and tried to work around problems
>>> by guessing amount of tabs needed (e.g. "\t\t" after top level", with
>>> buggy output as soon as empty uuids are printed). This will never work
>>> correctly, as tab width is a user-defined setting in the terminal.
>>
>>
>> Why not use string_table() and table_*() functions ?
>
> string_table(), as well as all table functions by nature, needs to know
> the maximum size of all cells in a row before printing, and therefore
> buffers all the output before printing. It would eat up a lot of memory
> for large tables (it is not unusual to have 1000+ subvolumes in btrfs if
> you make heavy use of snapshotting). Furthermore, it would slow down
> things by not printing the output linewise.
Assuming 200bytes per row (== subvolume) x 1000 subvolumes = 200kB... I don't think that this could be a problem, nor in terms of memory used nor in terms of speed: if you have 1000+ subvolumes the most time consuming activity is traversing the filesystem looking at the snapshot...
>
>>
>>>
>>> Keep it simple and don't reimplement the wheel, for nice tabular
>>> output we have the "column" command from util-linux:
>>>
>>> btrfs subvolume list -t <path> | column -t
>>>
>>> Signed-off-by: Axel Burri <axel@tty0.ch>
>>> ---
>>> btrfs-list.c | 40 ++++++++++++----------------------------
>>> 1 file changed, 12 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/btrfs-list.c b/btrfs-list.c
>>> index f8396e7..c09257a 100644
>>> --- a/btrfs-list.c
>>> +++ b/btrfs-list.c
>>> @@ -45,12 +45,12 @@ struct root_lookup {
>>> };
>>>
>>> static struct {
>>> - char *name;
>>> + char *name; /* machine-readable column identifier: [a-z_]+ */
>>> char *column_name;
>>> int need_print;
>>> } btrfs_list_columns[] = {
>>> {
>>> - .name = "ID",
>>> + .name = "id",
>>> .column_name = "ID",
>>> .need_print = 0,
>>> },
>>> @@ -70,7 +70,7 @@ static struct {
>>> .need_print = 0,
>>> },
>>> {
>>> - .name = "top level",
>>> + .name = "top_level",
>>> .column_name = "Top Level",
>>> .need_print = 0,
>>> },
>>> @@ -1465,13 +1465,10 @@ static void print_single_volume_info_table(struct root_info *subv)
>>> if (!btrfs_list_columns[i].need_print)
>>> continue;
>>>
>>> - print_subvolume_column(subv, i);
>>> -
>>> - if (i != BTRFS_LIST_PATH)
>>> - printf("\t");
>>> + if (i != 0)
>>> + printf(" ");
>>>
>>> - if (i == BTRFS_LIST_TOP_LEVEL)
>>> - printf("\t");
>>> + print_subvolume_column(subv, i);
>>> }
>>> printf("\n");
>>> }
>>> @@ -1496,30 +1493,17 @@ static void print_single_volume_info_default(struct root_info *subv)
>>> static void print_all_volume_info_tab_head(void)
>>> {
>>> int i;
>>> - int len;
>>> - char barrier[20];
>>> -
>>> - for (i = 0; i < BTRFS_LIST_ALL; i++) {
>>> - if (btrfs_list_columns[i].need_print)
>>> - printf("%s\t", btrfs_list_columns[i].name);
>>> -
>>> - if (i == BTRFS_LIST_ALL-1)
>>> - printf("\n");
>>> - }
>>>
>>> for (i = 0; i < BTRFS_LIST_ALL; i++) {
>>> - memset(barrier, 0, sizeof(barrier));
>>> + if (!btrfs_list_columns[i].need_print)
>>> + continue;
>>>
>>> - if (btrfs_list_columns[i].need_print) {
>>> - len = strlen(btrfs_list_columns[i].name);
>>> - while (len--)
>>> - strcat(barrier, "-");
>>> + if (i != 0)
>>> + printf(" ");
>>>
>>> - printf("%s\t", barrier);
>>> - }
>>> - if (i == BTRFS_LIST_ALL-1)
>>> - printf("\n");
>>> + printf(btrfs_list_columns[i].name);
>>> }
>>> + printf("\n");
>>> }
>>>
>>> static void print_all_volume_info(struct root_lookup *sorted_tree,
>>>
>>
>>
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable)
2015-10-03 17:41 ` Goffredo Baroncelli
@ 2015-10-04 3:37 ` Duncan
2015-10-04 14:34 ` Goffredo Baroncelli
0 siblings, 1 reply; 15+ messages in thread
From: Duncan @ 2015-10-04 3:37 UTC (permalink / raw)
To: linux-btrfs
Goffredo Baroncelli posted on Sat, 03 Oct 2015 19:41:33 +0200 as
excerpted:
> On 2015-10-03 12:09, Axel Burri wrote:
>>
>>
>> On 2015-10-03 11:56, Goffredo Baroncelli wrote:
>>> On 2015-10-02 18:41, axel@tty0.ch wrote:
>>>> Old implementation used tabs "\t", and tried to work around problems
>>>> by guessing amount of tabs needed (e.g. "\t\t" after top level", with
>>>> buggy output as soon as empty uuids are printed). This will never
>>>> work correctly, as tab width is a user-defined setting in the
>>>> terminal.
>>>
>>>
>>> Why not use string_table() and table_*() functions ?
>>
>> string_table(), as well as all table functions by nature, needs to know
>> the maximum size of all cells in a row before printing, and therefore
>> buffers all the output before printing. It would eat up a lot of memory
>> for large tables (it is not unusual to have 1000+ subvolumes in btrfs
>> if you make heavy use of snapshotting). Furthermore, it would slow down
>> things by not printing the output linewise.
>
>
> Assuming 200bytes per row (== subvolume) x 1000 subvolumes = 200kB... I
> don't think that this could be a problem, nor in terms of memory used
> nor in terms of speed: if you have 1000+ subvolumes the most time
> consuming activity is traversing the filesystem looking at the
> snapshot...
Perhaps unfortunately, scaling to millions of snapshots/subvolumes really
*is* expected by some people. You'd be surprised at the number of folks
that setup automated per-minute snapshotting with no automated thinning,
and expect to be able to keep several years' worth of snapshots, and then
wonder why btrfs maintenance commands such as balance take weeks/months...
5 years * 365 days/year * 24 hours/day * 60 minutes/hour * 1 snapshot/
minute = ...
(years, days, hours, minutes cancel out, leaving snapshots... yeah, the
math should be right)
2.6 million plus snapshots. And that's just one subvolume. Multiply
that by a half dozen subvolumes...
Over 15 million snapshots. Multiply that by 200 bytes/snapshot...
Several gigs of buffer memory, now.
Obviously btrfs doesn't scale to that level now, and if it did, someone
making the mistake of trying to get a listing of millions of snapshots
would very likely change their mind before even hitting 10%...
But that's why actually processing line-by-line is important, so they'll
actually /see/ what happened and ctrl-C it, instead of the program
aborting as it runs into (for example) the 32-bit user/kernel memory
barrier, without printing anything useful...
The line-by-line way... "Oops, that's waayyy too many snapshots to be
practical, maybe I should start thinning..."
The buffer-it-all-way... "Oops, there's a bug in the program; it crashed."
--
Duncan - List replies preferred. No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master." Richard Stallman
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable)
2015-10-04 3:37 ` Duncan
@ 2015-10-04 14:34 ` Goffredo Baroncelli
2015-10-05 15:08 ` Axel Burri
[not found] ` <56129171.4040200@digint.ch>
0 siblings, 2 replies; 15+ messages in thread
From: Goffredo Baroncelli @ 2015-10-04 14:34 UTC (permalink / raw)
To: Duncan; +Cc: linux-btrfs
On 2015-10-04 05:37, Duncan wrote:
> Goffredo Baroncelli posted on Sat, 03 Oct 2015 19:41:33 +0200 as
> excerpted:
>
>> On 2015-10-03 12:09, Axel Burri wrote:
>>>
>>>
>>> On 2015-10-03 11:56, Goffredo Baroncelli wrote:
>>>> On 2015-10-02 18:41, axel@tty0.ch wrote:
>>>>> Old implementation used tabs "\t", and tried to work around problems
>>>>> by guessing amount of tabs needed (e.g. "\t\t" after top level", with
>>>>> buggy output as soon as empty uuids are printed). This will never
>>>>> work correctly, as tab width is a user-defined setting in the
>>>>> terminal.
>>>>
>>>>
>>>> Why not use string_table() and table_*() functions ?
>>>
>>> string_table(), as well as all table functions by nature, needs to know
>>> the maximum size of all cells in a row before printing, and therefore
>>> buffers all the output before printing. It would eat up a lot of memory
>>> for large tables (it is not unusual to have 1000+ subvolumes in btrfs
>>> if you make heavy use of snapshotting). Furthermore, it would slow down
>>> things by not printing the output linewise.
>>
>>
>> Assuming 200bytes per row (== subvolume) x 1000 subvolumes = 200kB... I
>> don't think that this could be a problem, nor in terms of memory used
>> nor in terms of speed: if you have 1000+ subvolumes the most time
>> consuming activity is traversing the filesystem looking at the
>> snapshot...
>
> Perhaps unfortunately, scaling to millions of snapshots/subvolumes really
> *is* expected by some people. You'd be surprised at the number of folks
> that setup automated per-minute snapshotting with no automated thinning,
> and expect to be able to keep several years' worth of snapshots, and then
> wonder why btrfs maintenance commands such as balance take weeks/months...
[...]
> Obviously btrfs doesn't scale to that level now, and if it did, someone
> making the mistake of trying to get a listing of millions of snapshots
> would very likely change their mind before even hitting 10%...
>
> But that's why actually processing line-by-line is important, so they'll
> actually /see/ what happened and ctrl-C it, instead of the program
> aborting as it runs into (for example) the 32-bit user/kernel memory
> barrier, without printing anything useful...
Please Ducan, read the code.
*TODAY* "btrfs list" does a scan of all subvolumes storing information in memory !
This is the most time consuming activity (think traversing a filesystem with millions of snapshots)
*Then* "btrfs list" print the info.
So you are already blocked at the screen until all subvolume are read. And I repeat (re)processing the information requires less time than reading the information from the disk.
[....]
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable)
2015-10-04 14:34 ` Goffredo Baroncelli
@ 2015-10-05 15:08 ` Axel Burri
[not found] ` <56129171.4040200@digint.ch>
1 sibling, 0 replies; 15+ messages in thread
From: Axel Burri @ 2015-10-05 15:08 UTC (permalink / raw)
Cc: linux-btrfs
On 2015-10-04 16:34, Goffredo Baroncelli wrote:
> On 2015-10-04 05:37, Duncan wrote:
>> Goffredo Baroncelli posted on Sat, 03 Oct 2015 19:41:33 +0200 as
>> excerpted:
>>
>>> On 2015-10-03 12:09, Axel Burri wrote:
>>>>
>>>>
>>>> On 2015-10-03 11:56, Goffredo Baroncelli wrote:
>>>>> On 2015-10-02 18:41, axel@tty0.ch wrote:
>>>>>> Old implementation used tabs "\t", and tried to work around problems
>>>>>> by guessing amount of tabs needed (e.g. "\t\t" after top level", with
>>>>>> buggy output as soon as empty uuids are printed). This will never
>>>>>> work correctly, as tab width is a user-defined setting in the
>>>>>> terminal.
>>>>>
>>>>>
>>>>> Why not use string_table() and table_*() functions ?
>>>>
>>>> string_table(), as well as all table functions by nature, needs to know
>>>> the maximum size of all cells in a row before printing, and therefore
>>>> buffers all the output before printing. It would eat up a lot of memory
>>>> for large tables (it is not unusual to have 1000+ subvolumes in btrfs
>>>> if you make heavy use of snapshotting). Furthermore, it would slow down
>>>> things by not printing the output linewise.
>>>
>>>
>>> Assuming 200bytes per row (== subvolume) x 1000 subvolumes = 200kB... I
>>> don't think that this could be a problem, nor in terms of memory used
>>> nor in terms of speed: if you have 1000+ subvolumes the most time
>>> consuming activity is traversing the filesystem looking at the
>>> snapshot...
>>
>> Perhaps unfortunately, scaling to millions of snapshots/subvolumes really
>> *is* expected by some people. You'd be surprised at the number of folks
>> that setup automated per-minute snapshotting with no automated thinning,
>> and expect to be able to keep several years' worth of snapshots, and then
>> wonder why btrfs maintenance commands such as balance take weeks/months...
> [...]
>> Obviously btrfs doesn't scale to that level now, and if it did, someone
>> making the mistake of trying to get a listing of millions of snapshots
>> would very likely change their mind before even hitting 10%...
>>
>> But that's why actually processing line-by-line is important, so they'll
>> actually /see/ what happened and ctrl-C it, instead of the program
>> aborting as it runs into (for example) the 32-bit user/kernel memory
>> barrier, without printing anything useful...
>
> Please Ducan, read the code.
>
> *TODAY* "btrfs list" does a scan of all subvolumes storing information in memory !
>
> This is the most time consuming activity (think traversing a filesystem with millions of snapshots)
>
> *Then* "btrfs list" print the info.
>
> So you are already blocked at the screen until all subvolume are read. And I repeat (re)processing the information requires less time than reading the information from the disk.
>
> [....]
>
A quick look at the code shows me that Goffredo is right here, as
__list_subvol_search() always fetches ALL data from
BTRFS_IOC_TREE_SEARCH, putting it into a rbtree for later processing
(assemble full paths, sorting).
While there is certainly room for improvements here (assuming that
BTRFS_IOC_TREE_SEARCH returns objectid's in sorted order, it would
definitively be possible to produce line-by-line output), the code looks
pretty elegant the way it is.
I still don't think it is wise to bloat things further just for printing
nice tables. My impression is that "btrfs subvolume list" is
human-readable enough without the '-t' flag, while the output with '-t'
flag is much more machine-readable-friendly, and thus should have the
highest possible performance. e.g.:
btrfs sub list -t / | (read th; while read $th ; do echo $gen; done)
btrfs sub list -t | column -t
Again, this is just my opinion, being a "unix-purist". Maybe a good
compromise would be to use a single "\t" instead of " " as column delimiter.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable)
[not found] ` <56129171.4040200@digint.ch>
@ 2015-10-05 15:42 ` Goffredo Baroncelli
2015-10-05 16:58 ` Axel Burri
[not found] ` <5612B30A.9030308@tty0.ch>
0 siblings, 2 replies; 15+ messages in thread
From: Goffredo Baroncelli @ 2015-10-05 15:42 UTC (permalink / raw)
To: Axel Burri; +Cc: Duncan, linux-btrfs
Hi Axel,
On 2015-10-05 17:04, Axel Burri wrote:
[...]
>
> A quick look at the code shows me that Goffredo is right here, as
> __list_subvol_search() always fetches ALL data from
> BTRFS_IOC_TREE_SEARCH, putting it into a rbtree for later processing
> (assemble full paths, sorting).
>
> While there is certainly room for improvements here (assuming that
> BTRFS_IOC_TREE_SEARCH returns objectid's in sorted order, it would
> definitively be possible to produce line-by-line output), the code looks
> pretty elegant the way it is.
>
> I still don't think it is wise to bloat things further just for printing
> nice tables. My impression is that "btrfs subvolume list" is
> human-readable enough without the '-t' flag, while the output with '-t'
> flag is much more machine-readable-friendly, and thus should have the
> highest possible performance. e.g.:
I disagree, the "-t" mode is for the human; the non '-t' mode would be for machine (if you take the space as separator and the first word as key, with the exception of the path which starts from the "path " word and ends to the end of line); unfortunately "top level" is an exception. Anyway btrfs is not very machine-friendly (sic).
Compare the two output below:
$ sudo btrfs sub list -a /
ID 257 gen 44309 top level 5 path <FS_TREE>/debian
ID 289 gen 17415 top level 257 path debian/var/lib/machines
ID 298 gen 35434 top level 5 path <FS_TREE>/debian-i32
ID 299 gen 43961 top level 5 path <FS_TREE>/boot
ID 300 gen 39452 top level 5 path <FS_TREE>/debian-jessie
$ sudo btrfs sub list -at /
ID gen top level path
-- --- --------- ----
257 44310 5 <FS_TREE>/debian
289 17415 257 debian/var/lib/machines
298 35434 5 <FS_TREE>/debian-i32
299 43961 5 <FS_TREE>/boot
300 39452 5 <FS_TREE>/debian-jessie
I think that is easy to declare the latter more "human" friendly.
BTW with the use of the table_* function the output become:
$ sudo ./btrfs sub list -at /
ID gen top level path
=== ===== ========= =======================
257 44311 5 <FS_TREE>/debian
289 17415 257 debian/var/lib/machines
298 35434 5 <FS_TREE>/debian-i32
299 43961 5 <FS_TREE>/boot
300 39452 5 <FS_TREE>/debian-jessie
$ sudo ./btrfs sub list -aptcguqR /
ID gen cgen parent top level parent_uuid received_uuid uuid path
=== ===== ===== ====== ========= =========== ============= ==================================== =======================
257 44313 7 5 5 - - 840c86cf-e78b-d54a-ab38-66662858812d <FS_TREE>/debian
289 17415 17415 257 257 - - 8b857250-3a3e-754d-810e-57342bbb2f56 debian/var/lib/machines
298 35434 35399 5 5 - - 1f38049b-b153-d741-b903-d2de6fd7b3fd <FS_TREE>/debian-i32
299 43961 35512 5 5 - - f9d52b6b-a6d1-8c45-a6cd-ddb68cf58062 <FS_TREE>/boot
300 39452 37744 5 5 - - 026e44bd-66d4-e341-9a14-0124acf79793 <FS_TREE>/debian-jessie
I right aligned each field with the exception of the path (which is left aligned).
I already prepared the patch which convert "btrfs sub list -t" to use the table_* function. If you want I can send it to you to extend/replace your patch #4.
BR
G.Baroncelli
>
> btrfs sub list -t / | (read th; while read $th ; do echo $gen; done)
> btrfs sub list -t | column -t
>
> Again, this is just my opinion, being a "unix-purist". Maybe a good
> compromise would be to use a single "\t" instead of " " as column delimiter.
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable)
2015-10-05 15:42 ` Goffredo Baroncelli
@ 2015-10-05 16:58 ` Axel Burri
[not found] ` <5612B30A.9030308@tty0.ch>
1 sibling, 0 replies; 15+ messages in thread
From: Axel Burri @ 2015-10-05 16:58 UTC (permalink / raw)
To: kreijack; +Cc: Duncan, linux-btrfs
On 2015-10-05 17:42, Goffredo Baroncelli wrote:
> Hi Axel,
>
> On 2015-10-05 17:04, Axel Burri wrote:
> [...]
>> I still don't think it is wise to bloat things further just for printing
>> nice tables. My impression is that "btrfs subvolume list" is
>> human-readable enough without the '-t' flag, while the output with '-t'
>> flag is much more machine-readable-friendly, and thus should have the
>> highest possible performance. e.g.:
>
> I disagree, the "-t" mode is for the human; the non '-t' mode would be for machine (if you take the space as separator and the first word as key, with the exception of the path which starts from the "path " word and ends to the end of line); unfortunately "top level" is an exception. Anyway btrfs is not very machine-friendly (sic).
That's what my patches are all about in the first place, to make it more
machine-friendly :)
> Compare the two output below:
>
> $ sudo btrfs sub list -a /
> ID 257 gen 44309 top level 5 path <FS_TREE>/debian
> ID 289 gen 17415 top level 257 path debian/var/lib/machines
> ID 298 gen 35434 top level 5 path <FS_TREE>/debian-i32
> ID 299 gen 43961 top level 5 path <FS_TREE>/boot
> ID 300 gen 39452 top level 5 path <FS_TREE>/debian-jessie
"grep-friendly"
>
> $ sudo btrfs sub list -at /
> ID gen top level path
> -- --- --------- ----
> 257 44310 5 <FS_TREE>/debian
> 289 17415 257 debian/var/lib/machines
> 298 35434 5 <FS_TREE>/debian-i32
> 299 43961 5 <FS_TREE>/boot
> 300 39452 5 <FS_TREE>/debian-jessie
>
>
> I think that is easy to declare the latter more "human" friendly.
Well yes, but in some ways the latter is also more machine-friendly, as
there is less data to pipe and a whitespace-separated list is much
easier to parse (of course the same applies to your table implementation
below).
Problems here are:
- "top level" (whitespace, addressed in [PATCH 4/4])
- timestamp (-s) "YYYY-MM-DD HH:MM:SS" (whitspace and no timezone,
addressed in [PATCH 3/4])
- path can contain whitespace (not _that_ important as it's always
printed last).
>
> BTW with the use of the table_* function the output become:
>
> $ sudo ./btrfs sub list -at /
> ID gen top level path
> === ===== ========= =======================
> 257 44311 5 <FS_TREE>/debian
> 289 17415 257 debian/var/lib/machines
> 298 35434 5 <FS_TREE>/debian-i32
> 299 43961 5 <FS_TREE>/boot
> 300 39452 5 <FS_TREE>/debian-jessie
>
> $ sudo ./btrfs sub list -aptcguqR /
> ID gen cgen parent top level parent_uuid received_uuid uuid path
> === ===== ===== ====== ========= =========== ============= ==================================== =======================
> 257 44313 7 5 5 - - 840c86cf-e78b-d54a-ab38-66662858812d <FS_TREE>/debian
> 289 17415 17415 257 257 - - 8b857250-3a3e-754d-810e-57342bbb2f56 debian/var/lib/machines
> 298 35434 35399 5 5 - - 1f38049b-b153-d741-b903-d2de6fd7b3fd <FS_TREE>/debian-i32
> 299 43961 35512 5 5 - - f9d52b6b-a6d1-8c45-a6cd-ddb68cf58062 <FS_TREE>/boot
> 300 39452 37744 5 5 - - 026e44bd-66d4-e341-9a14-0124acf79793 <FS_TREE>/debian-jessie
>
> I right aligned each field with the exception of the path (which is left aligned).
> I already prepared the patch which convert "btrfs sub list -t" to use the table_* function. If you want I can send it to you to extend/replace your patch #4.
Looks promising, although I'd still not recommend it because of the
increased memory/cpu requirements.
^ permalink raw reply [flat|nested] 15+ messages in thread
* btrfs machine readable output [was Re: btrfs patches]
[not found] ` <5612B30A.9030308@tty0.ch>
@ 2015-10-05 20:09 ` Goffredo Baroncelli
0 siblings, 0 replies; 15+ messages in thread
From: Goffredo Baroncelli @ 2015-10-05 20:09 UTC (permalink / raw)
To: Axel Burri, David Sterba; +Cc: linux-btrfs
Hi Axel,
I add CC:linux-btrfs because I think that what you are saying is important and it was discussed in ML.
[we are talking about the Axel patches to "btrfs sub list" command]
On 2015-10-05 19:27, Axel Burri wrote:
> Hi Goffredo
>
> Don't get me wrong, I don't want to blame you (or anyone), actually I'm
> only arguing what I think is best for btrfs. I started to write those
> patches because btrfs-scriptability is pretty bad (there is also "btrfs
> sub show" which has pretty un-parseable output"), and I think this is
> very important for the success of btrfs, and no-one seemed to address this.
In the past, when I developed the "btrfs fi usage/btrfs dev usage" I put a lot of effort to have an output easily machine readable: one data per line, key without space; I remember the battle to have in the key "_" instead of space :-)
However other people changed that to a more (human) readable output.
May be that I was wrong; I don't know.
However for sure, if I look to other projects:
- systemd: it uses some btrfs capability, but it uses the ioctl directly.
- snapper: it uses ioctl directly
So I have to conclude that it is not common to use btrfs (as progs) directly.
Few times I noticed somebodies are working on a libbtrfs. May be this is a better approach.
>
> I know that nowadays people expect to get shiny output from command-line
> tools, but IMHO it is much more important to have consistent
> machine-readable output before even thinking of human readability (since
> there are so many unix commands to translate it, e.g. "columns"). Now
> this is sadly not the case with btrfs-progs, and it's hard to change it
> when people are used to it. Note that when writing the patches I was
> also thinking of introducing some "--machine-readable" option, but this
> seemed so wrong to me...
I think that "--machine-readable" (or similar) is a better approach: the output of the btrfs command in the past is changed even drastically (give a look to my work on the mkfs.btrfs); but we could force us to have an output "backward compatible" and machine (programmer) friendly if a switch like "--machine-readable" is used.
[...]
>
> Regards,
>
> - Axel
BR
Goffredo
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-10-05 20:09 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-02 16:41 [PATCH 0/4] btrfs-progs: improve output of btrfs subvolume list command axel
2015-10-02 16:41 ` [PATCH 1/4] btrfs-progs: add -A option for subvolume list (print all available information) axel
2015-10-02 16:41 ` [PATCH 2/4] btrfs-progs: add "flags" column for subvolume list (shows "readonly" flag with -A) axel
2015-10-02 16:41 ` [PATCH 3/4] btrfs-progs: add option "--time-format=short|iso|unix|locale" to subvolume list axel
2015-10-02 16:41 ` [PATCH 4/4] btrfs-progs: change -t option for subvolume list to print a simple space-separated table (making it machine-readable) axel
2015-10-03 9:56 ` Goffredo Baroncelli
2015-10-03 10:06 ` Goffredo Baroncelli
2015-10-03 10:17 ` Axel Burri
[not found] ` <560FA944.3050606@digint.ch>
2015-10-03 17:41 ` Goffredo Baroncelli
2015-10-04 3:37 ` Duncan
2015-10-04 14:34 ` Goffredo Baroncelli
2015-10-05 15:08 ` Axel Burri
[not found] ` <56129171.4040200@digint.ch>
2015-10-05 15:42 ` Goffredo Baroncelli
2015-10-05 16:58 ` Axel Burri
[not found] ` <5612B30A.9030308@tty0.ch>
2015-10-05 20:09 ` btrfs machine readable output [was Re: btrfs patches] Goffredo Baroncelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).