[PATCHv2 0/2] Introduce vfs_min_size API to get minimum filesystem size.
Tried to make it in accordance with your comments. Difference to v1: Added reply_with_error where necessary. Changed name get_min_size -> vfs_min_size. Maxim Perevedentsev (2): New API: vfs_min_size Include resize2fs_P into vfs_min_size. daemon/Makefile.am | 1 + daemon/daemon.h | 2 ++ daemon/ext2.c | 45 ++++++++++++++++++++++++++----- daemon/fs-min-size.c | 49 ++++++++++++++++++++++++++++++++++ daemon/ntfs.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++ generator/actions.ml | 16 ++++++----- po/POTFILES | 1 + 7 files changed, 177 insertions(+), 12 deletions(-) create mode 100644 daemon/fs-min-size.c -- 1.8.3.1
This call provides the way to get minimum size of filesystem. This is needed for shrinking. The return units are bytes. --- daemon/Makefile.am | 1 + daemon/daemon.h | 1 + daemon/fs-min-size.c | 46 ++++++++++++++++++++++++++++++++ daemon/ntfs.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++ generator/actions.ml | 16 +++++++++++ po/POTFILES | 1 + src/MAX_PROC_NR | 2 +- 7 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 daemon/fs-min-size.c diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 4ea3c88..0a01a24 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -116,6 +116,7 @@ guestfsd_SOURCES = \ findfs.c \ fill.c \ find.c \ + fs-min-size.c \ fsck.c \ fstrim.c \ glob.c \ diff --git a/daemon/daemon.h b/daemon/daemon.h index 508691a..a690152 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -283,6 +283,7 @@ extern int btrfs_set_uuid_random (const char *device); /*-- in ntfs.c --*/ extern char *ntfs_get_label (const char *device); extern int ntfs_set_label (const char *device, const char *label); +extern int64_t ntfs_min_size (const char *device); /*-- in swap.c --*/ extern int swap_set_uuid (const char *device, const char *uuid); diff --git a/daemon/fs-min-size.c b/daemon/fs-min-size.c new file mode 100644 index 0000000..9c107d1 --- /dev/null +++ b/daemon/fs-min-size.c @@ -0,0 +1,46 @@ +/* libguestfs - the guestfsd daemon + * Copyright (C) 2015 Maxim Perevedentsev mperevedentsev@virtuozzo.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> + +#include "daemon.h" +#include "actions.h" + +int64_t +do_vfs_min_size(const mountable_t *mountable) +{ + int64_t r; + + /* How we set the label depends on the filesystem type. */ + CLEANUP_FREE char *vfs_type = do_vfs_type (mountable); + if (vfs_type == NULL) + return -1; + + else if (STREQ (vfs_type, "ntfs")) + r = ntfs_min_size (mountable->device); + + else + NOT_SUPPORTED (-1, "don't know how to get minimum size of '%s' filesystems", + vfs_type); + + return r; +} diff --git a/daemon/ntfs.c b/daemon/ntfs.c index 1ead159..b9a645a 100644 --- a/daemon/ntfs.c +++ b/daemon/ntfs.c @@ -153,6 +153,81 @@ do_ntfsresize_size (const char *device, int64_t size) return do_ntfsresize (device, size, 0); } +int64_t +ntfs_min_size (const char *device) +{ + CLEANUP_FREE char *err = NULL, *out = NULL; + CLEANUP_FREE_STRING_LIST char **lines = NULL; + int r; + size_t i; + char *p; + int64_t ret, volume_size = 0; + const char *size_pattern = "You might resize at ", + *full_pattern = "Volume is full", + *cluster_size_pattern = "Cluster size", + *volume_size_pattern = "Current volume size:"; + int is_full = 0; + int32_t cluster_size = 0; + + /* FS may be marked for check, so force ntfsresize */ + r = command (&out, &err, str_ntfsresize, "--info", "-ff", device, NULL); + + lines = split_lines (out); + if (lines == NULL) + return -1; + + if (verbose) { + for (i = 0; lines[i] != NULL; ++i) + fprintf (stderr, "ntfs_min_size: lines[%zu] = \"%s\"\n", i, lines[i]); + } + + if (r == -1) { + /* If volume is full, ntfsresize returns error. */ + for (i = 0; lines[i] != NULL; ++i) { + if (strstr (lines[i], full_pattern)) + is_full = 1; + else if ((p = strstr (lines[i], cluster_size_pattern))) { + if (sscanf (p + strlen(cluster_size_pattern), + "%*[ ]:%" SCNd32, &cluster_size) != 1) { + reply_with_error("Cannot parse cluster size"); + return -1; + } + } + else if ((p = strstr (lines[i], volume_size_pattern))) { + if (sscanf (p + strlen(volume_size_pattern), + "%" SCNd64, &volume_size) != 1) { + reply_with_error("Cannot parse volume size"); + return -1; + } + } + } + if (is_full) { + /* Ceil to cluster size */ + if (cluster_size == 0) { + reply_with_error("Bad cluster size"); + return -1; + } + return (volume_size + cluster_size - 1) / cluster_size * cluster_size; + } + + reply_with_error ("%s", err); + return -1; + } + + for (i = 0; lines[i] != NULL; ++i) { + if ((p = strstr (lines[i], size_pattern))) { + if (sscanf (p + strlen(size_pattern), "%" SCNd64, &ret) != 1) { + reply_with_error("Cannot parse minimum size"); + return -1; + } + return ret; + } + } + + reply_with_error("Minimum size not found. Check output format:\n%s", out); + return -1; +} + /* Takes optional arguments, consult optargs_bitmask. */ int do_ntfsfix (const char *device, int clearbadsectors) diff --git a/generator/actions.ml b/generator/actions.ml index 274ef3f..0646a16 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -12765,6 +12765,22 @@ Get the estimated minimum filesystem size of an ext2/3/4 filesystem in blocks. See also L<resize2fs(8)>." }; + { defaults with + name = "vfs_min_size"; added = (1, 31, 18); + style = RInt64 "sizeinbytes", [Mountable "mountable"], []; + proc_nr = Some 458; + tests = [ + InitPartition, IfAvailable "ntfsprogs", TestRun( + [["mkfs"; "ntfs"; "/dev/sda1"; ""; "NOARG"; ""; ""; "NOARG"]; + ["vfs_min_size"; "/dev/sda1"]]), []; + ]; + shortdesc = "get minimum filesystem size"; + longdesc = "\ +Get the minimum size of filesystem in bytes. +This is the minimum possible size for filesystem shrinking. + +See also L<ntfsresize(8)>." }; + ] (* Non-API meta-commands available only in guestfish. diff --git a/po/POTFILES b/po/POTFILES index cd2c437..d90772a 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -49,6 +49,7 @@ daemon/file.c daemon/fill.c daemon/find.c daemon/findfs.c +daemon/fs-min-size.c daemon/fsck.c daemon/fstrim.c daemon/glob.c diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index de2a00c..c92ddb6 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -457 +458 -- 1.8.3.1
On Monday 19 October 2015 17:05:02 Maxim Perevedentsev wrote:
This call provides the way to get minimum size of filesystem. This is needed for shrinking. The return units are bytes.
--- daemon/Makefile.am | 1 + daemon/daemon.h | 1 + daemon/fs-min-size.c | 46 ++++++++++++++++++++++++++++++++ daemon/ntfs.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++ generator/actions.ml | 16 +++++++++++ po/POTFILES | 1 + src/MAX_PROC_NR | 2 +- 7 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 daemon/fs-min-size.c
diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 4ea3c88..0a01a24 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -116,6 +116,7 @@ guestfsd_SOURCES = \ findfs.c \ fill.c \ find.c \ + fs-min-size.c \
Hm not really convinced by the file name chosen, but I cannot find better options. "size.c"? "fs-size.c"?
fsck.c \ fstrim.c \ glob.c \ diff --git a/daemon/daemon.h b/daemon/daemon.h index 508691a..a690152 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -283,6 +283,7 @@ extern int btrfs_set_uuid_random (const char *device); /*-- in ntfs.c --*/ extern char *ntfs_get_label (const char *device); extern int ntfs_set_label (const char *device, const char *label); +extern int64_t ntfs_min_size (const char *device);
/*-- in swap.c --*/ extern int swap_set_uuid (const char *device, const char *uuid); diff --git a/daemon/fs-min-size.c b/daemon/fs-min-size.c new file mode 100644 index 0000000..9c107d1 --- /dev/null +++ b/daemon/fs-min-size.c @@ -0,0 +1,46 @@ +/* libguestfs - the guestfsd daemon + * Copyright (C) 2015 Maxim Perevedentsev mperevedentsev@virtuozzo.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> + +#include "daemon.h" +#include "actions.h" + +int64_t +do_vfs_min_size(const mountable_t *mountable)
I'd be slightly more verbose here, and call the action like "vfs_minimum_size". (also a minor style nitpick: missing space between function name and the bracket, and in other few places in this patch)
+{ + int64_t r; + + /* How we set the label depends on the filesystem type. */ + CLEANUP_FREE char *vfs_type = do_vfs_type (mountable); + if (vfs_type == NULL) + return -1; + + else if (STREQ (vfs_type, "ntfs")) + r = ntfs_min_size (mountable->device); + + else + NOT_SUPPORTED (-1, "don't know how to get minimum size of '%s' filesystems", + vfs_type);
The documentation might better explicitly mention that unsupported filesystems will give ENOTSUP as errno on failure.
+ + return r; +} diff --git a/daemon/ntfs.c b/daemon/ntfs.c index 1ead159..b9a645a 100644 --- a/daemon/ntfs.c +++ b/daemon/ntfs.c @@ -153,6 +153,81 @@ do_ntfsresize_size (const char *device, int64_t size) return do_ntfsresize (device, size, 0); }
+int64_t +ntfs_min_size (const char *device) +{ + CLEANUP_FREE char *err = NULL, *out = NULL; + CLEANUP_FREE_STRING_LIST char **lines = NULL; + int r; + size_t i; + char *p; + int64_t ret, volume_size = 0;
The scope of "ret" could be reduced to only the if below where it is used.
+ const char *size_pattern = "You might resize at ", + *full_pattern = "Volume is full", + *cluster_size_pattern = "Cluster size", + *volume_size_pattern = "Current volume size:"; + int is_full = 0; + int32_t cluster_size = 0; + + /* FS may be marked for check, so force ntfsresize */ + r = command (&out, &err, str_ntfsresize, "--info", "-ff", device, NULL); + + lines = split_lines (out); + if (lines == NULL) + return -1; + + if (verbose) { + for (i = 0; lines[i] != NULL; ++i) + fprintf (stderr, "ntfs_min_size: lines[%zu] = \"%s\"\n", i, lines[i]); + } + + if (r == -1) { + /* If volume is full, ntfsresize returns error. */ + for (i = 0; lines[i] != NULL; ++i) { + if (strstr (lines[i], full_pattern))
Better use STRPREFIX here, which is what we use all around libguestfs.
+ is_full = 1; + else if ((p = strstr (lines[i], cluster_size_pattern))) { + if (sscanf (p + strlen(cluster_size_pattern), + "%*[ ]:%" SCNd32, &cluster_size) != 1) { + reply_with_error("Cannot parse cluster size"); + return -1; + } + } + else if ((p = strstr (lines[i], volume_size_pattern))) { + if (sscanf (p + strlen(volume_size_pattern), + "%" SCNd64, &volume_size) != 1) { + reply_with_error("Cannot parse volume size"); + return -1; + }
It sounds like these scans of the lines could be done using the analyze_line helper in btrfs.c (which would need to be moved as common code in guestfd.c). This way, the parsing of numbers could use xstrtoul/xstrtoull (see btrfs.c:do_btrfs_subvolume_list) instead of sscanf.
+ } + } + if (is_full) { + /* Ceil to cluster size */ + if (cluster_size == 0) { + reply_with_error("Bad cluster size"); + return -1; + } + return (volume_size + cluster_size - 1) / cluster_size * cluster_size;
Can you please explain (e.g. in a comment) what is this calculation for?
+ } + + reply_with_error ("%s", err); + return -1; + } + + for (i = 0; lines[i] != NULL; ++i) { + if ((p = strstr (lines[i], size_pattern))) { + if (sscanf (p + strlen(size_pattern), "%" SCNd64, &ret) != 1) { + reply_with_error("Cannot parse minimum size"); + return -1;
Same notes as above wrt using analyze_line and xstrtoul.
+ } + return ret; + } + } + + reply_with_error("Minimum size not found. Check output format:\n%s", out); + return -1; +} + /* Takes optional arguments, consult optargs_bitmask. */ int do_ntfsfix (const char *device, int clearbadsectors) diff --git a/generator/actions.ml b/generator/actions.ml index 274ef3f..0646a16 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -12765,6 +12765,22 @@ Get the estimated minimum filesystem size of an ext2/3/4 filesystem in blocks.
See also L<resize2fs(8)>." };
+ { defaults with + name = "vfs_min_size"; added = (1, 31, 18); + style = RInt64 "sizeinbytes", [Mountable "mountable"], []; + proc_nr = Some 458; + tests = [ + InitPartition, IfAvailable "ntfsprogs", TestRun( + [["mkfs"; "ntfs"; "/dev/sda1"; ""; "NOARG"; ""; ""; "NOARG"]; + ["vfs_min_size"; "/dev/sda1"]]), []; + ]; + shortdesc = "get minimum filesystem size"; + longdesc = "\ +Get the minimum size of filesystem in bytes. +This is the minimum possible size for filesystem shrinking. + +See also L<ntfsresize(8)>." }; + ]
(* Non-API meta-commands available only in guestfish. diff --git a/po/POTFILES b/po/POTFILES index cd2c437..d90772a 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -49,6 +49,7 @@ daemon/file.c daemon/fill.c daemon/find.c daemon/findfs.c +daemon/fs-min-size.c daemon/fsck.c daemon/fstrim.c daemon/glob.c diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index de2a00c..c92ddb6 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -457 +458 --
Thanks, -- Pino Toscano
On 10/19/2015 07:45 PM, Pino Toscano wrote:
On Monday 19 October 2015 17:05:02 Maxim Perevedentsev wrote:
@@ -116,6 +116,7 @@ guestfsd_SOURCES = \ findfs.c \ fill.c \ find.c \ + fs-min-size.c \ Hm not really convinced by the file name chosen, but I cannot find better options. "size.c"? "fs-size.c"? Neither can I. Current name, at least, gives a hint about what is inside. FS size is not so straightforward.
-- Your sincerely, Maxim Perevedentsev
On 10/19/2015 07:45 PM, Pino Toscano wrote:
On Monday 19 October 2015 17:05:02 Maxim Perevedentsev wrote:
+int64_t +ntfs_min_size (const char *device) +{ + CLEANUP_FREE char *err = NULL, *out = NULL; + CLEANUP_FREE_STRING_LIST char **lines = NULL; + int r; + size_t i; + char *p; + int64_t ret, volume_size = 0; The scope of "ret" could be reduced to only the if below where it is used. Looking through code, I decided that declaring variables in the beginning of function is part of code style, so I followed it. Should I also move volume/cluster size declarations to branch where they are used?
-- Your sincerely, Maxim Perevedentsev
On Tue, Oct 20, 2015 at 12:59:06PM +0300, Maxim Perevedentsev wrote:
On Monday 19 October 2015 17:05:02 Maxim Perevedentsev wrote:
+int64_t +ntfs_min_size (const char *device) +{ + CLEANUP_FREE char *err = NULL, *out = NULL; + CLEANUP_FREE_STRING_LIST char **lines = NULL; + int r; + size_t i; + char *p; + int64_t ret, volume_size = 0; The scope of "ret" could be reduced to only the if below where it is used. Looking through code, I decided that declaring variables in the beginning of function is part of code style, so I followed it. Should I also move volume/cluster size declarations to branch where
On 10/19/2015 07:45 PM, Pino Toscano wrote: they are used?
The code style is to put variable decls at the top. What Pino means here specifically is that the 'ret' could be placed inside the later loop, ie: + for (i = 0; lines[i] != NULL; ++i) { + if ((p = strstr (lines[i], size_pattern))) { int64_t ret; + if (sscanf (p + strlen(size_pattern), "%" SCNd64, &ret) != 1) { + reply_with_error("Cannot parse minimum size"); + return -1; + } + return ret; + } Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
On 10/19/2015 07:45 PM, Pino Toscano wrote:
On Monday 19 October 2015 17:05:02 Maxim Perevedentsev wrote:
+ const char *size_pattern = "You might resize at ", + *full_pattern = "Volume is full", + *cluster_size_pattern = "Cluster size", + *volume_size_pattern = "Current volume size:"; + int is_full = 0; + int32_t cluster_size = 0; + + /* FS may be marked for check, so force ntfsresize */ + r = command (&out, &err, str_ntfsresize, "--info", "-ff", device, NULL); + + lines = split_lines (out); + if (lines == NULL) + return -1; + + if (verbose) { + for (i = 0; lines[i] != NULL; ++i) + fprintf (stderr, "ntfs_min_size: lines[%zu] = \"%s\"\n", i, lines[i]); + } + + if (r == -1) { + /* If volume is full, ntfsresize returns error. */ + for (i = 0; lines[i] != NULL; ++i) { + if (strstr (lines[i], full_pattern)) Better use STRPREFIX here, which is what we use all around libguestfs. This is not at the beginning of line.
+ is_full = 1; + else if ((p = strstr (lines[i], cluster_size_pattern))) { + if (sscanf (p + strlen(cluster_size_pattern), + "%*[ ]:%" SCNd32, &cluster_size) != 1) { + reply_with_error("Cannot parse cluster size"); + return -1; + } + } + else if ((p = strstr (lines[i], volume_size_pattern))) { + if (sscanf (p + strlen(volume_size_pattern), + "%" SCNd64, &volume_size) != 1) { + reply_with_error("Cannot parse volume size"); + return -1; + } It sounds like these scans of the lines could be done using the analyze_line helper in btrfs.c (which would need to be moved as common code in guestfd.c). This way, the parsing of numbers could use xstrtoul/xstrtoull (see btrfs.c:do_btrfs_subvolume_list) instead of sscanf. This is the only place where it could be used for now. Does it worth moving the code? I guess this code won't be shorter or simpler when using analyze_line. + } + + reply_with_error ("%s", err); + return -1; + } + + for (i = 0; lines[i] != NULL; ++i) { + if ((p = strstr (lines[i], size_pattern))) { + if (sscanf (p + strlen(size_pattern), "%" SCNd64, &ret) != 1) { + reply_with_error("Cannot parse minimum size"); + return -1; Same notes as above wrt using analyze_line and xstrtoul. No separator here. The string is "You might resize at 1231231 bytes"
-- Your sincerely, Maxim Perevedentsev
On Tue, Oct 20, 2015 at 01:28:23PM +0300, Maxim Perevedentsev wrote:
On 10/19/2015 07:45 PM, Pino Toscano wrote:
On Monday 19 October 2015 17:05:02 Maxim Perevedentsev wrote:
+ const char *size_pattern = "You might resize at ", + *full_pattern = "Volume is full", + *cluster_size_pattern = "Cluster size", + *volume_size_pattern = "Current volume size:"; + int is_full = 0; + int32_t cluster_size = 0; + + /* FS may be marked for check, so force ntfsresize */ + r = command (&out, &err, str_ntfsresize, "--info", "-ff", device, NULL); + + lines = split_lines (out); + if (lines == NULL) + return -1; + + if (verbose) { + for (i = 0; lines[i] != NULL; ++i) + fprintf (stderr, "ntfs_min_size: lines[%zu] = \"%s\"\n", i, lines[i]); + } + + if (r == -1) { + /* If volume is full, ntfsresize returns error. */ + for (i = 0; lines[i] != NULL; ++i) { + if (strstr (lines[i], full_pattern)) Better use STRPREFIX here, which is what we use all around libguestfs. This is not at the beginning of line.
+ is_full = 1; + else if ((p = strstr (lines[i], cluster_size_pattern))) { + if (sscanf (p + strlen(cluster_size_pattern), + "%*[ ]:%" SCNd32, &cluster_size) != 1) { + reply_with_error("Cannot parse cluster size"); + return -1; + } + } + else if ((p = strstr (lines[i], volume_size_pattern))) { + if (sscanf (p + strlen(volume_size_pattern), + "%" SCNd64, &volume_size) != 1) { + reply_with_error("Cannot parse volume size"); + return -1; + } It sounds like these scans of the lines could be done using the analyze_line helper in btrfs.c (which would need to be moved as common code in guestfd.c). This way, the parsing of numbers could use xstrtoul/xstrtoull (see btrfs.c:do_btrfs_subvolume_list) instead of sscanf.
Although I think sscanf is fine, and certainly easier to use. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
--- daemon/daemon.h | 1 + daemon/ext2.c | 45 +++++++++++++++++++++++++++++++++++++++------ daemon/fs-min-size.c | 3 +++ generator/actions.ml | 20 ++++---------------- src/MAX_PROC_NR | 2 +- 5 files changed, 48 insertions(+), 23 deletions(-) diff --git a/daemon/daemon.h b/daemon/daemon.h index a690152..61b61e7 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -224,6 +224,7 @@ extern int sync_disks (void); #define EXT2_LABEL_MAX 16 extern int fstype_is_extfs (const char *fstype); extern int ext_set_uuid_random (const char *device); +extern int64_t ext_min_size (const char *device); /*-- in blkid.c --*/ extern char *get_blkid_tag (const char *device, const char *tag); diff --git a/daemon/ext2.c b/daemon/ext2.c index 0cd6a66..f00e081 100644 --- a/daemon/ext2.c +++ b/daemon/ext2.c @@ -279,8 +279,34 @@ do_resize2fs_M (const char *device) return 0; } +static int32_t +get_block_size (const char *device) +{ + CLEANUP_FREE_STRING_LIST char **params = NULL; + const char *block_pattern = "Block size"; + size_t i; + int32_t block_size; + + params = do_tune2fs_l (device); + if (params == NULL) + return -1; + + for (i = 0; params[i] != NULL; i += 2) { + if ((!strcmp (params[i], block_pattern))) { + if (sscanf (params[i + 1], "%" SCNd32, &block_size) != 1) { + reply_with_error("Cannot parse block size"); + return -1; + } + return block_size; + } + } + + reply_with_error("No block size in tune2fs_l output. Check format.") + return -1; +} + int64_t -do_resize2fs_P (const char *device) +ext_min_size (const char *device) { CLEANUP_FREE char *err = NULL, *out = NULL; CLEANUP_FREE_STRING_LIST char **lines = NULL; @@ -288,6 +314,7 @@ do_resize2fs_P (const char *device) size_t i; char *p; int64_t ret; + int32_t block_size; const char *pattern = "Estimated minimum size of the filesystem: "; r = command (&out, &err, str_resize2fs, "-P", device, NULL); @@ -301,16 +328,22 @@ do_resize2fs_P (const char *device) return -1; for (i = 0; lines[i] != NULL; ++i) { - if (verbose) - fprintf (stderr, "resize2fs_P: lines[%zu] = \"%s\"\n", i, lines[i]); - if ((p = strstr (lines[i], pattern))) { - if (sscanf (p + strlen(pattern), "%" SCNd64, &ret) != 1) + if (sscanf (p + strlen(pattern), "%" SCNd64, &ret) != 1) { + reply_with_error("Cannot parse min size"); + return -1; + } + if ((block_size = get_block_size (device)) == -1) return -1; - return ret; + if (verbose) { + fprintf(stderr, "Minimum size in blocks: %" SCNd64 \ + "\nBlock count: %" SCNd32 "\n", ret, block_size); + } + return ret * block_size; } } + reply_with_error("Minimum size not found. Check output format:\n%s", out); return -1; } diff --git a/daemon/fs-min-size.c b/daemon/fs-min-size.c index 9c107d1..8c7287f 100644 --- a/daemon/fs-min-size.c +++ b/daemon/fs-min-size.c @@ -35,6 +35,9 @@ do_vfs_min_size(const mountable_t *mountable) if (vfs_type == NULL) return -1; + else if (fstype_is_extfs (vfs_type)) + r = ext_min_size (mountable->device); + else if (STREQ (vfs_type, "ntfs")) r = ntfs_min_size (mountable->device); diff --git a/generator/actions.ml b/generator/actions.ml index 0646a16..22358ef 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -12752,24 +12752,12 @@ Only some filesystem types support setting UUIDs. To read the UUID on a filesystem, call C<guestfs_vfs_uuid>." }; { defaults with - name = "resize2fs_P"; added = (1, 31, 17); - style = RInt64 "sizeinblocks", [Device "device"], []; - proc_nr = Some 457; - tests = [ - InitBasicFS, Always, TestRun ( - [["resize2fs_P"; "/dev/sda1"]]), []; - ]; - shortdesc = "get minimum filesystem size in blocks"; - longdesc = "\ -Get the estimated minimum filesystem size of an ext2/3/4 filesystem in blocks. - -See also L<resize2fs(8)>." }; - - { defaults with name = "vfs_min_size"; added = (1, 31, 18); style = RInt64 "sizeinbytes", [Mountable "mountable"], []; - proc_nr = Some 458; + proc_nr = Some 457; tests = [ + InitBasicFS, Always, TestRun ( + [["vfs_min_size"; "/dev/sda1"]]), []; InitPartition, IfAvailable "ntfsprogs", TestRun( [["mkfs"; "ntfs"; "/dev/sda1"; ""; "NOARG"; ""; ""; "NOARG"]; ["vfs_min_size"; "/dev/sda1"]]), []; @@ -12779,7 +12767,7 @@ See also L<resize2fs(8)>." }; Get the minimum size of filesystem in bytes. This is the minimum possible size for filesystem shrinking. -See also L<ntfsresize(8)>." }; +See also L<ntfsresize(8)>, L<resize2fs(8)>." }; ] diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index c92ddb6..de2a00c 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -458 +457 -- 1.8.3.1
On Monday 19 October 2015 17:05:03 Maxim Perevedentsev wrote:
--- daemon/daemon.h | 1 + daemon/ext2.c | 45 +++++++++++++++++++++++++++++++++++++++------ daemon/fs-min-size.c | 3 +++ generator/actions.ml | 20 ++++---------------- src/MAX_PROC_NR | 2 +- 5 files changed, 48 insertions(+), 23 deletions(-)
diff --git a/daemon/daemon.h b/daemon/daemon.h index a690152..61b61e7 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -224,6 +224,7 @@ extern int sync_disks (void); #define EXT2_LABEL_MAX 16 extern int fstype_is_extfs (const char *fstype); extern int ext_set_uuid_random (const char *device); +extern int64_t ext_min_size (const char *device);
/*-- in blkid.c --*/ extern char *get_blkid_tag (const char *device, const char *tag); diff --git a/daemon/ext2.c b/daemon/ext2.c index 0cd6a66..f00e081 100644 --- a/daemon/ext2.c +++ b/daemon/ext2.c @@ -279,8 +279,34 @@ do_resize2fs_M (const char *device) return 0; }
+static int32_t +get_block_size (const char *device) +{ + CLEANUP_FREE_STRING_LIST char **params = NULL; + const char *block_pattern = "Block size"; + size_t i; + int32_t block_size; + + params = do_tune2fs_l (device);
This works, yes, although maybe there should be a way to run tune2fs -l and traverse its output to either build a return hash (i.e. what do_tune2fs_l currently does) or just lookup one or more keys (like needed here).
+ if (params == NULL) + return -1; + + for (i = 0; params[i] != NULL; i += 2) { + if ((!strcmp (params[i], block_pattern))) {
Please use STREQ, like use everywhere else in libguestfs. (and the extra brackets are not needed)
+ if (sscanf (params[i + 1], "%" SCNd32, &block_size) != 1) { + reply_with_error("Cannot parse block size");
This could use a simplier xstrtoul/xstrtoull.
+ return -1; + } + return block_size; + } + } + + reply_with_error("No block size in tune2fs_l output. Check format.")
"missing 'Block size' in tune2fs_l output"
+ return -1; +} + int64_t -do_resize2fs_P (const char *device) +ext_min_size (const char *device) { CLEANUP_FREE char *err = NULL, *out = NULL; CLEANUP_FREE_STRING_LIST char **lines = NULL; @@ -288,6 +314,7 @@ do_resize2fs_P (const char *device) size_t i; char *p; int64_t ret; + int32_t block_size; const char *pattern = "Estimated minimum size of the filesystem: ";
r = command (&out, &err, str_resize2fs, "-P", device, NULL); @@ -301,16 +328,22 @@ do_resize2fs_P (const char *device) return -1;
for (i = 0; lines[i] != NULL; ++i) { - if (verbose) - fprintf (stderr, "resize2fs_P: lines[%zu] = \"%s\"\n", i, lines[i]); - if ((p = strstr (lines[i], pattern))) {
STRPREFIX here.
- if (sscanf (p + strlen(pattern), "%" SCNd64, &ret) != 1) + if (sscanf (p + strlen(pattern), "%" SCNd64, &ret) != 1) {
Ditto (xstrtoul).
+ reply_with_error("Cannot parse min size"); + return -1; + } + if ((block_size = get_block_size (device)) == -1) return -1; - return ret; + if (verbose) { + fprintf(stderr, "Minimum size in blocks: %" SCNd64 \ + "\nBlock count: %" SCNd32 "\n", ret, block_size); + } + return ret * block_size;
Might better check ret for possible overflows before multiplying it with block_size.
} }
+ reply_with_error("Minimum size not found. Check output format:\n%s", out); return -1; }
diff --git a/daemon/fs-min-size.c b/daemon/fs-min-size.c index 9c107d1..8c7287f 100644 --- a/daemon/fs-min-size.c +++ b/daemon/fs-min-size.c @@ -35,6 +35,9 @@ do_vfs_min_size(const mountable_t *mountable) if (vfs_type == NULL) return -1;
+ else if (fstype_is_extfs (vfs_type)) + r = ext_min_size (mountable->device); + else if (STREQ (vfs_type, "ntfs")) r = ntfs_min_size (mountable->device);
diff --git a/generator/actions.ml b/generator/actions.ml index 0646a16..22358ef 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -12752,24 +12752,12 @@ Only some filesystem types support setting UUIDs. To read the UUID on a filesystem, call C<guestfs_vfs_uuid>." };
{ defaults with - name = "resize2fs_P"; added = (1, 31, 17); - style = RInt64 "sizeinblocks", [Device "device"], []; - proc_nr = Some 457; - tests = [ - InitBasicFS, Always, TestRun ( - [["resize2fs_P"; "/dev/sda1"]]), []; - ]; - shortdesc = "get minimum filesystem size in blocks"; - longdesc = "\ -Get the estimated minimum filesystem size of an ext2/3/4 filesystem in blocks. - -See also L<resize2fs(8)>." }; - - { defaults with name = "vfs_min_size"; added = (1, 31, 18); style = RInt64 "sizeinbytes", [Mountable "mountable"], []; - proc_nr = Some 458; + proc_nr = Some 457; tests = [ + InitBasicFS, Always, TestRun ( + [["vfs_min_size"; "/dev/sda1"]]), []; InitPartition, IfAvailable "ntfsprogs", TestRun( [["mkfs"; "ntfs"; "/dev/sda1"; ""; "NOARG"; ""; ""; "NOARG"]; ["vfs_min_size"; "/dev/sda1"]]), []; @@ -12779,7 +12767,7 @@ See also L<resize2fs(8)>." }; Get the minimum size of filesystem in bytes. This is the minimum possible size for filesystem shrinking.
-See also L<ntfsresize(8)>." }; +See also L<ntfsresize(8)>, L<resize2fs(8)>." };
]
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index c92ddb6..de2a00c 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -458 +457
Thanks, -- Pino Toscano
On 10/19/2015 07:58 PM, Pino Toscano wrote:
On Monday 19 October 2015 17:05:03 Maxim Perevedentsev wrote:
+static int32_t +get_block_size (const char *device) +{ + CLEANUP_FREE_STRING_LIST char **params = NULL; + const char *block_pattern = "Block size"; + size_t i; + int32_t block_size; + + params = do_tune2fs_l (device); This works, yes, although maybe there should be a way to run tune2fs -l and traverse its output to either build a return hash (i.e. what do_tune2fs_l currently does) or just lookup one or more keys (like needed here). We could extract running tune2fs -l to separate function returning char*, and parse parameters using strstr. It's terrible... I think the cost of calling external command is much higher than the cost of building hash.
To make things betters, we could extract line-splitting function, create 2 similar functions (1 building hash (as for now) and 1 comparing key to argument). I do not think we will save much if we avoid copying strings into hash. -- Your sincerely, Maxim Perevedentsev
Sorry, both patches should have v2 in subject. -- Your sincerely, Maxim Perevedentsev
participants (3)
-
Maxim Perevedentsev -
Pino Toscano -
Richard W.M. Jones