[PATCH] New API: resize2fs_P
Added support for getting ext2/3/4 filesystem minimum size. This is needed primarily for shrinking images. --- daemon/ext2.c | 35 +++++++++++++++++++++++++++++++++++ generator/actions.ml | 10 ++++++++++ src/MAX_PROC_NR | 2 +- 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/daemon/ext2.c b/daemon/ext2.c index ebaf0f0..3dae40f 100644 --- a/daemon/ext2.c +++ b/daemon/ext2.c @@ -279,6 +279,41 @@ do_resize2fs_M (const char *device) return 0; } +int64_t +do_resize2fs_P (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; + char pattern[] = "Estimated minimum size of the filesystem: "; + + r = command (&out, &err, str_resize2fs, "-P", device, NULL); + if (r == -1) { + reply_with_error ("%s", err); + return -1; + } + + lines = split_lines (out); + if (lines == NULL) + 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), "%" SCNu64, &ret) != 1) + return -1; + return ret; + } + } + + return -1; +} + /* Takes optional arguments, consult optargs_bitmask. */ int do_e2fsck (const char *device, diff --git a/generator/actions.ml b/generator/actions.ml index a76b466..7e2bfac 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -12751,6 +12751,16 @@ 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; + shortdesc = "get minimum filesystem size in blocks"; + longdesc = "\ +Get the minimum filesystem size in blocks. + +See also L<resize2fs(8)>." }; + ] (* Non-API meta-commands available only in guestfish. diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index 8d38505..de2a00c 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -456 +457 -- 1.8.3.1
On Tue, Oct 13, 2015 at 06:26:25PM +0300, Maxim Perevedentsev wrote:
Added support for getting ext2/3/4 filesystem minimum size. This is needed primarily for shrinking images.
--- daemon/ext2.c | 35 +++++++++++++++++++++++++++++++++++ generator/actions.ml | 10 ++++++++++ src/MAX_PROC_NR | 2 +- 3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/daemon/ext2.c b/daemon/ext2.c index ebaf0f0..3dae40f 100644 --- a/daemon/ext2.c +++ b/daemon/ext2.c @@ -279,6 +279,41 @@ do_resize2fs_M (const char *device) return 0; }
+int64_t +do_resize2fs_P (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; + char pattern[] = "Estimated minimum size of the filesystem: "; + + r = command (&out, &err, str_resize2fs, "-P", device, NULL); + if (r == -1) { + reply_with_error ("%s", err); + return -1; + } + + lines = split_lines (out); + if (lines == NULL) + 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), "%" SCNu64, &ret) != 1) + return -1; + return ret; + } + } + + return -1; +} + /* Takes optional arguments, consult optargs_bitmask. */ int do_e2fsck (const char *device, diff --git a/generator/actions.ml b/generator/actions.ml index a76b466..7e2bfac 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -12751,6 +12751,16 @@ 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; + shortdesc = "get minimum filesystem size in blocks"; + longdesc = "\ +Get the minimum filesystem size in blocks. + +See also L<resize2fs(8)>." }; + ]
(* Non-API meta-commands available only in guestfish. diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index 8d38505..de2a00c 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -456 +457
Looks fine to me. Pino: any comment? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
On Tue, Oct 13, 2015 at 11:14:29PM +0100, Richard W.M. Jones wrote:
+ if ((p = strstr (lines[i], pattern))) { + if (sscanf (p + strlen(pattern), "%" SCNu64, &ret) != 1)
Actually this fails to compile: ext2.c: In function 'do_resize2fs_P': ext2.c:308:40: error: format '%lu' expects argument of type 'long unsigned int *', but argument 3 has type 'int64_t * {aka long int *}' [-Werror=format=] if (sscanf (p + strlen(pattern), "%" SCNu64, &ret) != 1) It'd be good to have a test as well, ie. a tests = ... section in generator/actions.ml. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
On Tue, Oct 13, 2015 at 06:26:25PM +0300, Maxim Perevedentsev wrote: Another couple of things ...
+ char pattern[] = "Estimated minimum size of the filesystem: ";
This should be 'const char *pattern'.
+ if ((p = strstr (lines[i], pattern))) { + if (sscanf (p + strlen(pattern), "%" SCNu64, &ret) != 1) + return -1; + return ret;
There's a case for multiplying the return size by the block size of the filesystem, thus returning the size in bytes. It might make the API easier to use. Not sure how hard that is, or whether it actually makes the API 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/
There's a case for multiplying the return size by the block size of the filesystem, thus returning the size in bytes. It might make the API easier to use. Not sure how hard that is, or whether it actually makes the API easier to use.
Rich. This multiplication may confuse users who got used to "normal" resize2fs -P behavior (blocks) due to the name of API command. I'm going to implement similar functionality for other filesystems (ntfs, btrfs, xfs). I think we should preserve original command's behavior (well-documented, obviously!) and leave the conversion between blocks, megabytes, etc to user. Maybe (later) we could implement aggregated call (getMinFsSizeInBytes()) to perform all this conversions.
-- Your sincerely, Maxim Perevedentsev
Hi, On Wednesday 14 October 2015 12:53:26 Maxim Perevedentsev wrote:
This multiplication may confuse users who got used to "normal" resize2fs -P behavior (blocks) due to the name of API command. I'm going to implement similar functionality for other filesystems (ntfs, btrfs, xfs).
Then please implement a single function for all these filesystems, just like set_label, set_uuid, etc. Having to call a different function to get the same information depending on the filesystem is only making artificial barriers in users of the API (which then have to do the function choice at runtime on their own).
I think we should preserve original command's behavior (well-documented, obviously!) and leave the conversion between blocks, megabytes, etc to user. Maybe (later) we could implement aggregated call (getMinFsSizeInBytes()) to perform all this conversions.
Please note the API is there also to provide some coherency. -- Pino Toscano
On Wed, Oct 14, 2015 at 06:29:41PM +0200, Pino Toscano wrote:
Hi,
On Wednesday 14 October 2015 12:53:26 Maxim Perevedentsev wrote:
This multiplication may confuse users who got used to "normal" resize2fs -P behavior (blocks) due to the name of API command. I'm going to implement similar functionality for other filesystems (ntfs, btrfs, xfs).
Then please implement a single function for all these filesystems, just like set_label, set_uuid, etc. Having to call a different function to get the same information depending on the filesystem is only making artificial barriers in users of the API (which then have to do the function choice at runtime on their own).
I think we should preserve original command's behavior (well-documented, obviously!) and leave the conversion between blocks, megabytes, etc to user. Maybe (later) we could implement aggregated call (getMinFsSizeInBytes()) to perform all this conversions.
Please note the API is there also to provide some coherency.
I generally agree with Pino that it would be better to have a single API that can handle any filesystem type, and returns the estimated min size in bytes. Note that we can remove and change new APIs until 1.32 (next stable) is released, and after that we keep them forever. I expect to release 1.32 some time in late December 2015. 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
participants (3)
-
Maxim Perevedentsev -
Pino Toscano -
Richard W.M. Jones