Libguestfs - Oct 2017 - [PATCH v2 2/2] builder: Choose better weights in the planner.

v1 -> v2:

 - Removed the f_type field from StatVFS.statvfs structure.

 - New function StatVFS.filesystem_is_remote, written in C.
   [Thinking about it, this should probably be called
   ?is_network_filesystem?, but I can change that before
   pushing].

 - Use statvfs instead of fstatvfs, and statfs instead of fstatfs.

 - Rejigged the comments in builder/builder.ml to make them simpler
   and clearer.

Rich.

common/mlutils: Unix_utils.StatVFS.statvfs: This commit implements a
full-featured binding for the statvfs(3) function.

We then use this to reimplement the daemon statvfs API in OCaml.

Note that the Gnulib fallback is fixed in this commit.  It previously
referenced non-existent field names in the fs_usage struct.
---
 common/mlutils/unix_utils-c.c | 138 ++++++++++++++++++++++++++++---
 common/mlutils/unix_utils.ml  |  21 ++++-
 common/mlutils/unix_utils.mli |  23 +++++-
 daemon/Makefile.am            |  11 ++-
 daemon/statvfs.c              | 184 ------------------------------------------
 daemon/statvfs.ml             |  43 ++++++++++
 daemon/statvfs.mli            |  33 ++++++++
 docs/C_SOURCE_FILES           |   1 -
 generator/actions_core.ml     |   1 +
 m4/guestfs_libraries.m4       |   2 +
 po/POTFILES                   |   1 -
 sparsify/copying.ml           |   2 +-
 v2v/v2v.ml                    |   2 +-
 13 files changed, 256 insertions(+), 206 deletions(-)

diff --git a/common/mlutils/unix_utils-c.c b/common/mlutils/unix_utils-c.c
index 631f1fddd..657852ef2 100644
--- a/common/mlutils/unix_utils-c.c
+++ b/common/mlutils/unix_utils-c.c
@@ -27,7 +27,10 @@
 #include <fnmatch.h>
 #include <errno.h>
 #include <sys/types.h>
+
+#ifdef HAVE_SYS_STATVFS_H
 #include <sys/statvfs.h>
+#endif
 
 #if MAJOR_IN_MKDEV
 #include <sys/mkdev.h>
@@ -36,12 +39,18 @@
 /* else it's in sys/types.h, included above */
 #endif
 
+#ifdef HAVE_WINDOWS_H
+#include <windows.h>
+#endif
+
 #include <caml/alloc.h>
 #include <caml/fail.h>
 #include <caml/memory.h>
 #include <caml/mlvalues.h>
 #include <caml/unixsupport.h>
 
+#include "fsusage.h"
+
 extern value guestfs_int_mllib_dev_t_makedev (value majv, value minv);
 extern value guestfs_int_mllib_dev_t_major (value devv);
 extern value guestfs_int_mllib_dev_t_minor (value devv);
@@ -52,7 +61,7 @@ extern value guestfs_int_mllib_sync (value unitv);
 extern value guestfs_int_mllib_fsync_file (value filenamev);
 extern value guestfs_int_mllib_mkdtemp (value val_pattern);
 extern value guestfs_int_mllib_realpath (value pathv);
-extern value guestfs_int_mllib_statvfs_free_space (value pathv);
+extern value guestfs_int_mllib_statvfs_statvfs (value pathv);
 
 /* NB: This is a "noalloc" call. */
 value
@@ -207,20 +216,129 @@ guestfs_int_mllib_realpath (value pathv)
 }
 
 value
-guestfs_int_mllib_statvfs_free_space (value pathv)
+guestfs_int_mllib_statvfs_statvfs (value pathv)
 {
   CAMLparam1 (pathv);
-  CAMLlocal1 (rv);
+  int64_t f_bsize;
+  int64_t f_frsize;
+  int64_t f_blocks;
+  int64_t f_bfree;
+  int64_t f_bavail;
+  int64_t f_files;
+  int64_t f_ffree;
+  int64_t f_favail;
+  int64_t f_fsid;
+  int64_t f_flag;
+  int64_t f_namemax;
+  CAMLlocal2 (rv, v);
+
+#ifdef HAVE_STATVFS
   struct statvfs buf;
-  int64_t free_space;
 
-  if (statvfs (String_val (pathv), &buf) == -1) {
-    perror ("statvfs");
-    caml_failwith ("statvfs");
-  }
+  if (statvfs (String_val (pathv), &buf) == -1)
+    unix_error (errno, (char *) "statvfs", pathv);
 
-  free_space = (int64_t) buf.f_bsize * buf.f_bavail;
-  rv = caml_copy_int64 (free_space);
+  f_bsize = buf.f_bsize;
+  f_frsize = buf.f_frsize;
+  f_blocks = buf.f_blocks;
+  f_bfree = buf.f_bfree;
+  f_bavail = buf.f_bavail;
+  f_files = buf.f_files;
+  f_ffree = buf.f_ffree;
+  f_favail = buf.f_favail;
+  f_fsid = buf.f_fsid;
+  f_flag = buf.f_flag;
+  f_namemax = buf.f_namemax;
+
+#else /* !HAVE_STATVFS */
+#  if WIN32
+  ULONGLONG free_bytes_available; /* for user - similar to bavail */
+  ULONGLONG total_number_of_bytes;
+  ULONGLONG total_number_of_free_bytes; /* for everyone - bfree */
+
+  if (!GetDiskFreeSpaceEx (String_val (pathv),
+                           (PULARGE_INTEGER) &free_bytes_available,
+                           (PULARGE_INTEGER) &total_number_of_bytes,
+                           (PULARGE_INTEGER) &total_number_of_free_bytes))
+    unix_error (EIO, (char *) "statvfs: GetDiskFreeSpaceEx", pathv);
+
+  /* XXX I couldn't determine how to get block size.  MSDN has a
+   * unhelpful hard-coded list here:
+   *   http://support.microsoft.com/kb/140365
+   * but this depends on the filesystem type, the size of the disk and
+   * the version of Windows.  So this code assumes the disk is NTFS
+   * and the version of Windows is >= Win2K.
+   */
+  if (total_number_of_bytes < UINT64_C (16) * 1024 * 1024 * 1024 * 1024)
+    f_bsize = 4096;
+  else if (total_number_of_bytes < UINT64_C (32) * 1024 * 1024 * 1024 *
1024)
+    f_bsize = 8192;
+  else if (total_number_of_bytes < UINT64_C (64) * 1024 * 1024 * 1024 *
1024)
+    f_bsize = 16384;
+  else if (total_number_of_bytes < UINT64_C (128) * 1024 * 1024 * 1024 *
1024)
+    f_bsize = 32768;
+  else
+    f_bsize = 65536;
+
+  /* As with stat, -1 indicates a field is not known. */
+  f_frsize = ret->bsize;
+  f_blocks = total_number_of_bytes / ret->bsize;
+  f_bfree = total_number_of_free_bytes / ret->bsize;
+  f_bavail = free_bytes_available / ret->bsize;
+  f_files = -1;
+  f_ffree = -1;
+  f_favail = -1;
+  f_fsid = -1;
+  f_flag = -1;
+  f_namemax = FILENAME_MAX;
+
+#  else /* !WIN32 */
+  /* This replacement function is provided by Gnulib. */
+  struct fs_usage fsu;
+
+  if (get_fs_usage (String_val (pathv), String_val (pathv), &fsu) == -1)
+    unix_error (EIO, (char *) "statvfs: get_fs_usage", pathv);
+
+  /* As with stat, -1 indicates a field is not known. */
+  f_bsize = fsu.fsu_bsize;
+  f_frsize = -1;
+  f_blocks = fsu.fsu_blocks;
+  f_bfree = fsu.fsu_bfree;
+  f_bavail = fsu.fsu_bavail;
+  f_files = fsu.fsu_files;
+  f_free = fsu.fsu_ffree;
+  f_favail = -1;
+  f_fsid = -1;
+  f_flag = -1;
+  f_namemax = -1;
+
+#  endif /* !WIN32 */
+#endif /* !HAVE_STATVFS */
+
+  /* Construct the return struct. */
+  rv = caml_alloc (11, 0);
+  v = caml_copy_int64 (f_bsize);
+  Store_field (rv, 0, v);
+  v = caml_copy_int64 (f_frsize);
+  Store_field (rv, 1, v);
+  v = caml_copy_int64 (f_blocks);
+  Store_field (rv, 2, v);
+  v = caml_copy_int64 (f_bfree);
+  Store_field (rv, 3, v);
+  v = caml_copy_int64 (f_bavail);
+  Store_field (rv, 4, v);
+  v = caml_copy_int64 (f_files);
+  Store_field (rv, 5, v);
+  v = caml_copy_int64 (f_ffree);
+  Store_field (rv, 6, v);
+  v = caml_copy_int64 (f_favail);
+  Store_field (rv, 7, v);
+  v = caml_copy_int64 (f_fsid);
+  Store_field (rv, 8, v);
+  v = caml_copy_int64 (f_flag);
+  Store_field (rv, 9, v);
+  v = caml_copy_int64 (f_namemax);
+  Store_field (rv, 10, v);
 
   CAMLreturn (rv);
 }
diff --git a/common/mlutils/unix_utils.ml b/common/mlutils/unix_utils.ml
index b135bcaee..085eff65b 100644
--- a/common/mlutils/unix_utils.ml
+++ b/common/mlutils/unix_utils.ml
@@ -16,6 +16,8 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *)
 
+open Std_utils
+
 module Dev_t = struct
   external makedev : int -> int -> int =
"guestfs_int_mllib_dev_t_makedev" "noalloc"
   external major : int -> int = "guestfs_int_mllib_dev_t_major"
"noalloc"
@@ -61,6 +63,21 @@ module Realpath = struct
 end
 
 module StatVFS = struct
-  external free_space : string -> int64 -   
"guestfs_int_mllib_statvfs_free_space"
+  type statvfs = {
+    f_bsize : int64;
+    f_frsize : int64;
+    f_blocks : int64;
+    f_bfree : int64;
+    f_bavail : int64;
+    f_files : int64;
+    f_ffree : int64;
+    f_favail : int64;
+    f_fsid : int64;
+    f_flag : int64;
+    f_namemax : int64;
+  }
+  external statvfs : string -> statvfs +   
"guestfs_int_mllib_statvfs_statvfs"
+
+  let free_space { f_bsize = bsize; f_bavail = bavail } = bsize *^ bavail
 end
diff --git a/common/mlutils/unix_utils.mli b/common/mlutils/unix_utils.mli
index f4f8ca578..bd182129c 100644
--- a/common/mlutils/unix_utils.mli
+++ b/common/mlutils/unix_utils.mli
@@ -94,7 +94,26 @@ module Realpath : sig
 end
 
 module StatVFS : sig
-  val free_space : string -> int64
-  (** [free_space path] returns the free space available on the
+  type statvfs = {
+    f_bsize : int64;            (** Filesystem block size *)
+    f_frsize : int64;           (** Fragment size *)
+    f_blocks : int64;           (** Size of fs in f_frsize units *)
+    f_bfree : int64;            (** Number of free blocks *)
+    f_bavail : int64;           (** Number of free blocks for non-root *)
+    f_files : int64;            (** Number of inodes *)
+    f_ffree : int64;            (** Number of free inodes *)
+    f_favail : int64;           (** Number of free inodes for non-root *)
+    f_fsid : int64;             (** Filesystem ID *)
+    f_flag : int64;             (** Mount flags *)
+    f_namemax : int64;          (** Maximum length of filenames *)
+  }
+  val statvfs : string -> statvfs
+  (** This calls [statvfs(3)] on the path parameter.
+
+      In case of non-Linux or non-POSIX, this call is emulated as best
+      we can with missing fields returned as [-1]. *)
+
+  val free_space : statvfs -> int64
+  (** [free_space (statvfs path)] returns the free space available on the
       filesystem that contains [path], in bytes. *)
 end
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 61cfb5caa..070fe4ddc 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -154,7 +154,6 @@ guestfsd_SOURCES = \
 	sleuthkit.c \
 	squashfs.c \
 	stat.c \
-	statvfs.c \
 	strings.c \
 	structs-cleanups.c \
 	structs-cleanups.h \
@@ -274,6 +273,7 @@ SOURCES_MLI = \
 	optgroups.mli \
 	parted.mli \
 	realpath.mli \
+	statvfs.mli \
 	structs.mli \
 	sysroot.mli \
 	utils.mli
@@ -300,6 +300,7 @@ SOURCES_ML = \
 	parted.ml \
 	listfs.ml \
 	realpath.ml \
+	statvfs.ml \
 	inspect_types.ml \
 	inspect_utils.ml \
 	inspect_fs_unix_fstab.ml \
@@ -348,8 +349,10 @@ camldaemon.o: $(OBJECTS)
 	$(OCAMLFIND) $(BEST) -output-obj -o $@ \
 	    $(OCAMLFLAGS) $(OCAMLPACKAGES) \
 	    -linkpkg \
-	    mlaugeas.$(MLARCHIVE) mlpcre.$(MLARCHIVE) \
-	    mlcutils.$(MLARCHIVE) mlstdutils.$(MLARCHIVE) \
+	    mlaugeas.$(MLARCHIVE) \
+	    mlpcre.$(MLARCHIVE) \
+	    mlstdutils.$(MLARCHIVE) \
+	    mlcutils.$(MLARCHIVE) \
 	    $(OBJECTS)
 
 # OCaml dependencies.
@@ -399,8 +402,8 @@ endif
 
 OCAMLLINKFLAGS = \
 	mlpcre.$(MLARCHIVE) \
-	mlcutils.$(MLARCHIVE) \
 	mlstdutils.$(MLARCHIVE) \
+	mlcutils.$(MLARCHIVE) \
 	mlaugeas.$(MLARCHIVE) \
 	$(LINK_CUSTOM_OCAMLC_ONLY)
 
diff --git a/daemon/statvfs.c b/daemon/statvfs.c
deleted file mode 100644
index 987537d1f..000000000
--- a/daemon/statvfs.c
+++ /dev/null
@@ -1,184 +0,0 @@
-/* libguestfs - the guestfsd daemon
- * Copyright (C) 2009 Red Hat Inc.
- *
- * 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>
-
-#ifdef HAVE_WINDOWS_H
-#include <windows.h>
-#endif
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <sys/types.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include <stdint.h>
-
-#ifdef HAVE_SYS_STATVFS_H
-#include <sys/statvfs.h>
-#endif
-
-#include <fsusage.h>
-
-#include "guestfs_protocol.h"
-#include "daemon.h"
-#include "actions.h"
-
-guestfs_int_statvfs *
-do_statvfs (const char *path)
-{
-#ifdef HAVE_STATVFS
-  int r;
-  guestfs_int_statvfs *ret;
-  struct statvfs statbuf;
-
-  CHROOT_IN;
-  r = statvfs (path, &statbuf);
-  CHROOT_OUT;
-
-  if (r == -1) {
-    reply_with_perror ("statvfs");
-    return NULL;
-  }
-
-  ret = malloc (sizeof *ret);
-  if (ret == NULL) {
-    reply_with_perror ("malloc");
-    return NULL;
-  }
-
-  ret->bsize = statbuf.f_bsize;
-  ret->frsize = statbuf.f_frsize;
-  ret->blocks = statbuf.f_blocks;
-  ret->bfree = statbuf.f_bfree;
-  ret->bavail = statbuf.f_bavail;
-  ret->files = statbuf.f_files;
-  ret->ffree = statbuf.f_ffree;
-  ret->favail = statbuf.f_favail;
-  ret->fsid = statbuf.f_fsid;
-  ret->flag = statbuf.f_flag;
-  ret->namemax = statbuf.f_namemax;
-
-  return ret;
-
-#else /* !HAVE_STATVFS */
-#  if WIN32
-
-  CLEANUP_FREE char *disk = NULL;
-  guestfs_int_statvfs *ret;
-  ULONGLONG free_bytes_available; /* for user - similar to bavail */
-  ULONGLONG total_number_of_bytes;
-  ULONGLONG total_number_of_free_bytes; /* for everyone - bfree */
-
-  disk = sysroot_path (path);
-  if (!disk) {
-    reply_with_perror ("malloc");
-    return NULL;
-  }
-
-  if (!GetDiskFreeSpaceEx (disk,
-                           (PULARGE_INTEGER) &free_bytes_available,
-                           (PULARGE_INTEGER) &total_number_of_bytes,
-                           (PULARGE_INTEGER) &total_number_of_free_bytes))
{
-    reply_with_perror ("GetDiskFreeSpaceEx");
-    return NULL;
-  }
-
-  ret = malloc (sizeof *ret);
-  if (ret == NULL) {
-    reply_with_perror ("malloc");
-    return NULL;
-  }
-
-  /* XXX I couldn't determine how to get block size.  MSDN has a
-   * unhelpful hard-coded list here:
-   *   http://support.microsoft.com/kb/140365
-   * but this depends on the filesystem type, the size of the disk and
-   * the version of Windows.  So this code assumes the disk is NTFS
-   * and the version of Windows is >= Win2K.
-   */
-  if (total_number_of_bytes < UINT64_C (16) * 1024 * 1024 * 1024 * 1024)
-    ret->bsize = 4096;
-  else if (total_number_of_bytes < UINT64_C (32) * 1024 * 1024 * 1024 *
1024)
-    ret->bsize = 8192;
-  else if (total_number_of_bytes < UINT64_C (64) * 1024 * 1024 * 1024 *
1024)
-    ret->bsize = 16384;
-  else if (total_number_of_bytes < UINT64_C (128) * 1024 * 1024 * 1024 *
1024)
-    ret->bsize = 32768;
-  else
-    ret->bsize = 65536;
-
-  /* As with stat, -1 indicates a field is not known. */
-  ret->frsize = ret->bsize;
-  ret->blocks = total_number_of_bytes / ret->bsize;
-  ret->bfree = total_number_of_free_bytes / ret->bsize;
-  ret->bavail = free_bytes_available / ret->bsize;
-  ret->files = -1;
-  ret->ffree = -1;
-  ret->favail = -1;
-  ret->fsid = -1;
-  ret->flag = -1;
-  ret->namemax = FILENAME_MAX;
-
-  return ret;
-
-#  else /* !WIN32 */
-
-  CLEANUP_FREE char *disk = NULL;
-  int r;
-  guestfs_int_statvfs *ret;
-  struct fs_usage fsu;
-
-  disk = sysroot_path (path);
-  if (!disk) {
-    reply_with_perror ("malloc");
-    return NULL;
-  }
-
-  r = get_fs_usage (disk, disk, &fsu);
-
-  if (r == -1) {
-    reply_with_perror ("get_fs_usage: %s", path);
-    return NULL;
-  }
-
-  ret = malloc (sizeof *ret);
-  if (ret == NULL) {
-    reply_with_perror ("malloc");
-    return NULL;
-  }
-
-  /* As with stat, -1 indicates a field is not known. */
-  ret->bsize = fsu.f_bsize;
-  ret->frsize = -1;
-  ret->blocks = fsu.f_blocks;
-  ret->bfree = fsu.f_bfree;
-  ret->bavail = fsu.f_bavail;
-  ret->files = fsu.f_files;
-  ret->ffree = fsu.f_ffree;
-  ret->favail = -1;
-  ret->fsid = -1;
-  ret->flag = -1;
-  ret->namemax = -1;
-
-  return ret;
-
-#  endif /* !WIN32 */
-#endif /* !HAVE_STATVFS */
-}
diff --git a/daemon/statvfs.ml b/daemon/statvfs.ml
new file mode 100644
index 000000000..af3434a98
--- /dev/null
+++ b/daemon/statvfs.ml
@@ -0,0 +1,43 @@
+(* guestfs-inspection
+ * Copyright (C) 2009-2017 Red Hat Inc.
+ *
+ * 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.
+ *)
+
+open Unix_utils
+
+include Structs
+
+let statvfs path +  let chroot = Chroot.create ~name:(Printf.sprintf
"statvfs: %s" path) () in
+  let r = Chroot.f chroot StatVFS.statvfs path in
+
+  (* [r : Unix_utils.StatVFS.statvfs] is superficially similar to
+   * the guestfs [statvfs] structure, but not the same.  We have to
+   * copy the fields.
+   *)
+  {
+    bsize = r.StatVFS.f_bsize;
+    frsize = r.StatVFS.f_frsize;
+    blocks = r.StatVFS.f_blocks;
+    bfree = r.StatVFS.f_bfree;
+    bavail = r.StatVFS.f_bavail;
+    files = r.StatVFS.f_files;
+    ffree = r.StatVFS.f_ffree;
+    favail = r.StatVFS.f_favail;
+    fsid = r.StatVFS.f_fsid;
+    flag = r.StatVFS.f_flag;
+    namemax = r.StatVFS.f_namemax;
+  }
diff --git a/daemon/statvfs.mli b/daemon/statvfs.mli
new file mode 100644
index 000000000..163ea9a15
--- /dev/null
+++ b/daemon/statvfs.mli
@@ -0,0 +1,33 @@
+(* guestfs-inspection
+ * Copyright (C) 2009-2017 Red Hat Inc.
+ *
+ * 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.
+ *)
+
+type statvfs = {
+  bsize : int64;
+  frsize : int64;
+  blocks : int64;
+  bfree : int64;
+  bavail : int64;
+  files : int64;
+  ffree : int64;
+  favail : int64;
+  fsid : int64;
+  flag : int64;
+  namemax : int64;
+}
+
+val statvfs : string -> statvfs
diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES
index 41bb6c9eb..f3ee61e0d 100644
--- a/docs/C_SOURCE_FILES
+++ b/docs/C_SOURCE_FILES
@@ -166,7 +166,6 @@ daemon/sleep.c
 daemon/sleuthkit.c
 daemon/squashfs.c
 daemon/stat.c
-daemon/statvfs.c
 daemon/strings.c
 daemon/structs-cleanups.c
 daemon/structs-cleanups.h
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index 81cb99ea8..604620673 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -2319,6 +2319,7 @@ See also: C<guestfs_sh_lines>" };
   { defaults with
     name = "statvfs"; added = (1, 9, 2);
     style = RStruct ("statbuf", "statvfs"), [String
(Pathname, "path")], [];
+    impl = OCaml "Statvfs.statvfs";
     tests = [
       InitISOFS, Always, TestResult (
         [["statvfs"; "/"]], "ret->namemax ==
255"), []
diff --git a/m4/guestfs_libraries.m4 b/m4/guestfs_libraries.m4
index b8fa1446a..ddd9e2b7f 100644
--- a/m4/guestfs_libraries.m4
+++ b/m4/guestfs_libraries.m4
@@ -48,10 +48,12 @@ AC_CHECK_HEADERS([\
     sys/inotify.h \
     sys/resource.h \
     sys/socket.h \
+    sys/statfs.h \
     sys/statvfs.h \
     sys/time.h \
     sys/types.h \
     sys/un.h \
+    sys/vfs.h \
     sys/wait.h \
     windows.h \
     sys/xattr.h])
diff --git a/po/POTFILES b/po/POTFILES
index 40c0708a0..a049d66fe 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -149,7 +149,6 @@ daemon/sleep.c
 daemon/sleuthkit.c
 daemon/squashfs.c
 daemon/stat.c
-daemon/statvfs.c
 daemon/strings.c
 daemon/structs-cleanups.c
 daemon/stubs-0.c
diff --git a/sparsify/copying.ml b/sparsify/copying.ml
index 7d004b550..0c0b27c94 100644
--- a/sparsify/copying.ml
+++ b/sparsify/copying.ml
@@ -99,7 +99,7 @@ let run indisk outdisk check_tmpdir compress convert
           virtual_size (human_size virtual_size);
 
     let print_warning () -      let free_space = StatVFS.free_space tmpdir in
+      let free_space = StatVFS.free_space (StatVFS.statvfs tmpdir) in
       let extra_needed = virtual_size -^ free_space in
       if extra_needed > 0L then (
         warning (f_"\
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 74ba66a3d..f54838e5e 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -272,7 +272,7 @@ and overlay_dir = (open_guestfs ())#get_cachedir ()
  * guestfs appliance which is also stored here.
  *)
 and check_host_free_space () -  let free_space = StatVFS.free_space overlay_dir
in
+  let free_space = StatVFS.free_space (StatVFS.statvfs overlay_dir) in
   debug "check_host_free_space: overlay_dir=%s free_space=%Ld"
         overlay_dir free_space;
   if free_space < 1_073_741_824L then
-- 
2.13.2

---
 builder/builder.ml            | 84 +++++++++++++++++++++++++++++++++----------
 common/mlutils/unix_utils-c.c | 27 ++++++++++++++
 common/mlutils/unix_utils.ml  |  3 ++
 common/mlutils/unix_utils.mli |  4 +++
 m4/guestfs_libraries.m4       |  1 +
 5 files changed, 100 insertions(+), 19 deletions(-)

diff --git a/builder/builder.ml b/builder/builder.ml
index d8e625f68..fd19aa7d9 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -414,12 +414,59 @@ let main ()      let is_not t = not (is t) in
     let remove = List.remove_assoc in
     let ret = ref [] in
-    let tr task weight otags = push_front (task, weight, otags) ret in
 
-    (* XXX Weights are not very smartly chosen.  At the moment I'm
-     * using a range [0..100] where 0 = free and 100 = expensive.  We
-     * could estimate weights better by looking at file sizes.
+    (* The scheme for weights ranges from 0 = free to 100 = most expensive:
+     *
+     *    0 = free operations like renaming a file in the same directory
+     *   10 = in-place conversions (like [qemu-img resize])
+     *   20 = copy or move a file between two local filesystems
+     *   30 = copy and convert a file between two local filesystems
+     *   40 = copy a file within the same local filesystem
+     *   50 = copy and convert a file within the same local filesystem
+     *   80 = copy, move, convert if source or target is on remote filesystem
+     *  100 = complex operations like virt-resize
+     *
+     * Copies and moves across different local filesystems are
+     * cheaper than copies within the same filesystem.  The
+     * theory because less bandwith is available if both source
+     * and destination hit the same device (except in the special
+     * case of moving within a filesystem which is free).
+     *
+     * We could estimate weights better by looking at file sizes.
      *)
+    let weight task otags +      let infile = List.assoc `Filename itags
+      and outfile = List.assoc `Filename otags in
+
+      (* If infile/outfile don't exist, get the containing directory. *)
+      let infile +        if Sys.file_exists infile then infile else
Filename.dirname infile in
+      let outfile +        if Sys.file_exists outfile then outfile else
Filename.dirname outfile in
+
+      match task with
+      | `Virt_resize -> 100     (* virt-resize is a special case*)
+      | (`Copy|`Move|`Pxzcat|`Disk_resize|`Convert) as task ->
+         if StatVFS.filesystem_is_remote infile ||
+            StatVFS.filesystem_is_remote outfile
+         then 80 (* NFS etc. *)
+         else (
+           let inst = lstat infile in
+           let outst = lstat outfile in
+           let across = inst.st_dev <> outst.st_dev in
+           match task, across with
+           | `Move, false -> 0     (* rename in same filesystem *)
+           | `Disk_resize, _ -> 10 (* in-place conversion *)
+           | `Move, true -> 20     (* move between two filesystems *)
+           | `Copy, true -> 20     (* copy between two filesystems *)
+           | (`Pxzcat|`Convert), true -> 30 (* convert between two local
fses*)
+           | `Copy, false -> 40    (* copy within same filesystem *)
+           | (`Pxzcat|`Convert), false -> 50 (* convert with same local fs*)
+         )
+    in
+
+    (* Add a transition to the returned list. *)
+    let tr task otags = push_front (task, weight task otags, otags) ret in
 
     (* Since the final plan won't run in parallel, we don't only need
      * to choose unique tempfiles per transition, so this is OK:
@@ -431,17 +478,16 @@ let main ()       * thing a copy does is to remove the
template tag (since it's always
      * copied out of the cache directory).
      *)
-    tr `Copy 50 ((`Filename, output_filename) :: remove `Template itags);
-    tr `Copy 50 ((`Filename, tempfile) :: remove `Template itags);
+    tr `Copy ((`Filename, output_filename) :: remove `Template itags);
+    tr `Copy ((`Filename, tempfile) :: remove `Template itags);
 
     (* We can rename a file instead of copying, but don't rename the
-     * cache copy!  (XXX Also this is not free if copying across
-     * filesystems)
+     * cache copy!
      *)
     if is_not `Template then (
       if not output_is_block_dev then
-        tr `Rename 0 ((`Filename, output_filename) :: itags);
-      tr `Rename 0 ((`Filename, tempfile) :: itags);
+        tr `Move ((`Filename, output_filename) :: itags);
+      tr `Move ((`Filename, tempfile) :: itags)
     );
 
     if is `XZ then (
@@ -449,9 +495,9 @@ let main ()         * to the output file or to a temp file.
        *)
       if not output_is_block_dev then
-        tr `Pxzcat 80
+        tr `Pxzcat
           ((`Filename, output_filename) :: remove `XZ (remove `Template
itags));
-      tr `Pxzcat 80
+      tr `Pxzcat
         ((`Filename, tempfile) :: remove `XZ (remove `Template itags));
     )
     else (
@@ -462,11 +508,11 @@ let main ()        let old_size = Int64.of_string
(List.assoc `Size itags) in
       let headroom = 256L *^ 1024L *^ 1024L in
       if output_size >= old_size +^ headroom then (
-        tr `Virt_resize 100
+        tr `Virt_resize
           ((`Size, Int64.to_string output_size) ::
               (`Filename, output_filename) ::
               (`Format, output_format) :: (remove `Template itags));
-        tr `Virt_resize 100
+        tr `Virt_resize
           ((`Size, Int64.to_string output_size) ::
               (`Filename, tempfile) ::
               (`Format, output_format) :: (remove `Template itags))
@@ -485,15 +531,15 @@ let main ()         *)
       else if output_size > old_size && is_not `Template
               && List.mem_assoc `Format itags then
-        tr `Disk_resize 60 ((`Size, Int64.to_string output_size) :: itags);
+        tr `Disk_resize ((`Size, Int64.to_string output_size) :: itags);
 
       (* qemu-img convert is always possible, and quicker.  It doesn't
        * resize, but it does change the format.
        *)
-      tr `Convert 60
+      tr `Convert
         ((`Filename, output_filename) :: (`Format, output_format) ::
             (remove `Template itags));
-      tr `Convert 60
+      tr `Convert
         ((`Filename, tempfile) :: (`Format, output_format) ::
             (remove `Template itags));
     );
@@ -528,7 +574,7 @@ let main ()      in
     let print_task = function
       | `Copy -> printf "cp"
-      | `Rename -> printf "mv"
+      | `Move -> printf "mv"
       | `Pxzcat -> printf "pxzcat"
       | `Virt_resize -> printf "virt-resize"
       | `Disk_resize -> printf "qemu-img resize"
@@ -570,7 +616,7 @@ let main ()        let cmd = [ "cp"; ifile; ofile
] in
       if run_command cmd <> 0 then exit 1
 
-    | itags, `Rename, otags ->
+    | itags, `Move, otags ->
       let ifile = List.assoc `Filename itags in
       let ofile = List.assoc `Filename otags in
       let cmd = [ "mv"; ifile; ofile ] in
diff --git a/common/mlutils/unix_utils-c.c b/common/mlutils/unix_utils-c.c
index 657852ef2..340fdec44 100644
--- a/common/mlutils/unix_utils-c.c
+++ b/common/mlutils/unix_utils-c.c
@@ -28,10 +28,18 @@
 #include <errno.h>
 #include <sys/types.h>
 
+#ifdef HAVE_SYS_STATFS_H
+#include <sys/statfs.h>
+#endif
+
 #ifdef HAVE_SYS_STATVFS_H
 #include <sys/statvfs.h>
 #endif
 
+#ifdef HAVE_SYS_VFS_H
+#include <sys/vfs.h>
+#endif
+
 #if MAJOR_IN_MKDEV
 #include <sys/mkdev.h>
 #elif MAJOR_IN_SYSMACROS
@@ -62,6 +70,7 @@ extern value guestfs_int_mllib_fsync_file (value filenamev);
 extern value guestfs_int_mllib_mkdtemp (value val_pattern);
 extern value guestfs_int_mllib_realpath (value pathv);
 extern value guestfs_int_mllib_statvfs_statvfs (value pathv);
+extern value guestfs_int_mllib_statvfs_filesystem_is_remote (value pathv);
 
 /* NB: This is a "noalloc" call. */
 value
@@ -342,3 +351,21 @@ guestfs_int_mllib_statvfs_statvfs (value pathv)
 
   CAMLreturn (rv);
 }
+
+/* NB: This is a "noalloc" call. */
+value
+guestfs_int_mllib_statvfs_filesystem_is_remote (value pathv)
+{
+#ifdef HAVE_STATFS
+  struct statfs buf;
+
+  if (statfs (String_val (pathv), &buf) == -1)
+    unix_error (errno, (char *) "statvfs", pathv);
+
+  return Val_bool (buf.f_type == 0xff534d42 /* CIFS_MAGIC_NUMBER */ ||
+                   buf.f_type == 0x6969 /* NFS_SUPER_MAGIC */ ||
+                   buf.f_type == 0x517b /* SMB_SUPER_MAGIC */);
+#else
+  return Val_bool (0);
+#endif
+}
diff --git a/common/mlutils/unix_utils.ml b/common/mlutils/unix_utils.ml
index 085eff65b..e516ba8bb 100644
--- a/common/mlutils/unix_utils.ml
+++ b/common/mlutils/unix_utils.ml
@@ -80,4 +80,7 @@ module StatVFS = struct
     "guestfs_int_mllib_statvfs_statvfs"
 
   let free_space { f_bsize = bsize; f_bavail = bavail } = bsize *^ bavail
+
+  external filesystem_is_remote : string -> bool +   
"guestfs_int_mllib_statvfs_filesystem_is_remote" "noalloc"
 end
diff --git a/common/mlutils/unix_utils.mli b/common/mlutils/unix_utils.mli
index bd182129c..b73db8a1e 100644
--- a/common/mlutils/unix_utils.mli
+++ b/common/mlutils/unix_utils.mli
@@ -116,4 +116,8 @@ module StatVFS : sig
   val free_space : statvfs -> int64
   (** [free_space (statvfs path)] returns the free space available on the
       filesystem that contains [path], in bytes. *)
+
+  val filesystem_is_remote : string -> bool
+  (** [filesystem_is_remote path] returns true if [path] is located on
+      a remote filesystem such as NFS or CIFS. *)
 end
diff --git a/m4/guestfs_libraries.m4 b/m4/guestfs_libraries.m4
index ddd9e2b7f..5b3983fae 100644
--- a/m4/guestfs_libraries.m4
+++ b/m4/guestfs_libraries.m4
@@ -82,6 +82,7 @@ AC_CHECK_FUNCS([\
     setrlimit \
     setxattr \
     sigaction \
+    statfs \
     statvfs \
     sync])
 
-- 
2.13.2

On Tuesday, 3 October 2017 14:24:13 CEST Richard W.M. Jones
wrote:
> common/mlutils: Unix_utils.StatVFS.statvfs: This commit implements a
> full-featured binding for the statvfs(3) function.
> 
> We then use this to reimplement the daemon statvfs API in OCaml.
> 
> Note that the Gnulib fallback is fixed in this commit.  It previously
> referenced non-existent field names in the fs_usage struct.
TBH I'd just drop the gnulib fallback (i.e. the fsusage module):
- (f)statvfs is POSIX since 2001
- Linux, macOS, and *BSDs have been supporting it already for years
  (decades, even)
- Windows (even if untested for long) has the fallback code already

This way there's less dead code around, and one gnulib module less.

-- 
Pino Toscano

On Tuesday, 3 October 2017 14:24:14 CEST Richard W.M. Jones
wrote:
> ---
>  builder/builder.ml            | 84
+++++++++++++++++++++++++++++++++----------
Cannot comment on the actual logic in the planner, since I admit I did
not have an in-depth look at it.
> diff --git a/common/mlutils/unix_utils-c.c b/common/mlutils/unix_utils-c.c
> index 657852ef2..340fdec44 100644
> --- a/common/mlutils/unix_utils-c.c
> +++ b/common/mlutils/unix_utils-c.c
> @@ -28,10 +28,18 @@
>  #include <errno.h>
>  #include <sys/types.h>
>  
> +#ifdef HAVE_SYS_STATFS_H
> +#include <sys/statfs.h>
> +#endif
> +
>  #ifdef HAVE_SYS_STATVFS_H
>  #include <sys/statvfs.h>
>  #endif
>  
> +#ifdef HAVE_SYS_VFS_H
> +#include <sys/vfs.h>
> +#endif
> +
>  #if MAJOR_IN_MKDEV
>  #include <sys/mkdev.h>
>  #elif MAJOR_IN_SYSMACROS
> @@ -62,6 +70,7 @@ extern value guestfs_int_mllib_fsync_file (value
filenamev);
>  extern value guestfs_int_mllib_mkdtemp (value val_pattern);
>  extern value guestfs_int_mllib_realpath (value pathv);
>  extern value guestfs_int_mllib_statvfs_statvfs (value pathv);
> +extern value guestfs_int_mllib_statvfs_filesystem_is_remote (value pathv);
>  
>  /* NB: This is a "noalloc" call. */
>  value
> @@ -342,3 +351,21 @@ guestfs_int_mllib_statvfs_statvfs (value pathv)
>  
>    CAMLreturn (rv);
>  }
> +
> +/* NB: This is a "noalloc" call. */
> +value
> +guestfs_int_mllib_statvfs_filesystem_is_remote (value pathv)
> +{
> +#ifdef HAVE_STATFS
> +  struct statfs buf;
> +
> +  if (statfs (String_val (pathv), &buf) == -1)
> +    unix_error (errno, (char *) "statvfs", pathv);
> +
> +  return Val_bool (buf.f_type == 0xff534d42 /* CIFS_MAGIC_NUMBER */ ||
> +                   buf.f_type == 0x6969 /* NFS_SUPER_MAGIC */ ||
> +                   buf.f_type == 0x517b /* SMB_SUPER_MAGIC */);
Using linux/magic.h + the constants from it should avoid the hardcoded
numbers, and possibly allow more remote filesystems.  Not a big issue
otherwise, anyway.

-- 
Pino Toscano