Libguestfs - Oct 2015 - Re: [PATCH 2/2] Include resize2fs_P into vfs_min_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

---
 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

Sorry, both patches should have v2 in subject.

-- 
Your sincerely,
Maxim Perevedentsev

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 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