Richard W.M. Jones
2017-Oct-04 13:31 UTC
[Libguestfs] [PATCH v3 0/2] builder: Choose better weights in the planner.
v2 -> v3: - Drop gnulib fallback.
Richard W.M. Jones
2017-Oct-04 13:31 UTC
[Libguestfs] [PATCH v3 1/2] daemon: Reimplement statvfs API in OCaml, drop gnulib fallback.
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 dropped in this commit. It previously referenced non-existent field names in the fs_usage struct so it didn't work. Also it's not necessary as POSIX has supported statvfs(3) since 2001, it's supported in *BSD, macOS > 10.4, and there is already a Windows fallback. --- bootstrap | 1 - common/mlutils/unix_utils-c.c | 118 ++++++++++++++++++++++++--- 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/.gitignore | 1 - m4/guestfs_libraries.m4 | 2 + po/POTFILES | 1 - sparsify/copying.ml | 2 +- v2v/v2v.ml | 2 +- 15 files changed, 236 insertions(+), 208 deletions(-) diff --git a/bootstrap b/bootstrap index 4e3d4bc51..bb665c6a8 100755 --- a/bootstrap +++ b/bootstrap @@ -49,7 +49,6 @@ dup3 error filevercmp fstatat -fsusage fts full-read full-write diff --git a/common/mlutils/unix_utils-c.c b/common/mlutils/unix_utils-c.c index 631f1fddd..94097e95f 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,6 +39,10 @@ /* 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> @@ -52,7 +59,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 +214,111 @@ 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 */ +#error "no statvfs or equivalent function available" +# 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/.gitignore b/m4/.gitignore index a84b22e5c..6b9c4c667 100644 --- a/m4/.gitignore +++ b/m4/.gitignore @@ -62,7 +62,6 @@ /fseeko.m4 /fstatat.m4 /fstat.m4 -/fsusage.m4 /ftell.m4 /ftello.m4 /ftruncate.m4 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
Richard W.M. Jones
2017-Oct-04 13:31 UTC
[Libguestfs] [PATCH v3 2/2] builder: Choose better weights in the planner.
--- builder/builder.ml | 82 +++++++++++++++++++++++++++++++++---------- 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, 98 insertions(+), 19 deletions(-) diff --git a/builder/builder.ml b/builder/builder.ml index d8e625f68..90ff4645a 100644 --- a/builder/builder.ml +++ b/builder/builder.ml @@ -414,12 +414,57 @@ 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.is_network_filesystem infile || + StatVFS.is_network_filesystem outfile + then 80 (* NFS etc. *) + else ( + let across = (lstat infile).st_dev <> (lstat outfile).st_dev in + match task, across with + | `Move, false -> 0 (* rename in same filesystem *) + | `Disk_resize, _ -> 10 (* in-place conversion *) + | `Move, true (* move or copy between two filesystems *) + | `Copy, true -> 20 + | (`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 +476,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 +493,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 +506,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 +529,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 +572,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 +614,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 94097e95f..a43921741 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 @@ -60,6 +68,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_is_network_filesystem (value pathv); /* NB: This is a "noalloc" call. */ value @@ -322,3 +331,21 @@ guestfs_int_mllib_statvfs_statvfs (value pathv) CAMLreturn (rv); } + +/* NB: This is a "noalloc" call. */ +value +guestfs_int_mllib_statvfs_is_network_filesystem (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..6912282c8 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 is_network_filesystem : string -> bool + "guestfs_int_mllib_statvfs_is_network_filesystem" "noalloc" end diff --git a/common/mlutils/unix_utils.mli b/common/mlutils/unix_utils.mli index bd182129c..4df72aa55 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 is_network_filesystem : string -> bool + (** [is_network_filesystem path] returns true if [path] is located on + a network 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
Apparently Analagous Threads
- [PATCH v2 0/2] builder: Choose better weights in the planner.
- [PATCH 0/2] builder: Choose better weights in the planner.
- [PATCH 0/3] v2v: vCenter: Remove proxy environment variables
- [PATCH] mllib: Add a binding for realpath(3).
- virtdf outputs on host differs from df in guest