March 2017 - Libguestfs - libguestfs List Archives

[PATCH 00/11] Various Coverity fixes
by Pino Toscano 03 Mar '17

03 Mar '17

Hi, this patch series fixes some issues discovered by Coverity. Most of them are memory leaks, usually on error; there are also invalid memory access issues. Thanks, Pino Toscano (11): java: link libguestfs_jni against libutils java: fix invalid memory access for FBuffer in struct lists daemon: tsk: properly use GUESTFS_MAX_CHUNK_SIZE edit: fix small memory leak on error java: fix possible memory leak on error fish: fully init the msghdr buffers tail: pass the right path for Windows guests ocaml: do not try to malloc 0 elements in get_all_event_callbacks python: do not try to malloc 0 elements in get_all_event_callbacks ruby: do not try to malloc 0 elements in get_all_event_callbacks java: do not try to malloc 0 elements in get_all_event_callbacks cat/tail.c | 2 +- common/edit/file-edit.c | 1 + daemon/sleuthkit.c | 2 +- fish/rc.c | 9 +++------ generator/java.ml | 8 +++----- java/Makefile.am | 4 +++- java/handle.c | 3 ++- ocaml/guestfs-c.c | 3 ++- python/handle.c | 3 ++- ruby/ext/guestfs/handle.c | 3 ++- 10 files changed, 20 insertions(+), 18 deletions(-) -- 2.9.3

2 16

[PATCH] rescue: Implement --mount and -i options.
by Richard W.M. Jones 03 Mar '17

03 Mar '17

Depends on the previous 5 patches that modified virt-rescue to work without direct mode: https://www.redhat.com/archives/libguestfs/2017-March/msg00017.html Rich.

1 1

[PATCH libguestfs] Use AC_HEADER_MAJOR to find definitions of major, minor, makedev.
by Richard W.M. Jones 03 Mar '17

03 Mar '17

Note this requires either the following fix in autoconf: http://git.savannah.gnu.org/cgit/autoconf.git/commit/?id=e17a30e98 OR gnulib sys_types module plus gnulib commit a512e041120e9012e69afa2f5c3adc196ec4999a (any gnulib more recent than Sep 2016) which corrects the AC_HEADER_MAJOR macro in a similar way. --- bootstrap | 1 + cat/ls.c | 7 +++++++ daemon/mknod.c | 6 ++++++ diff/diff.c | 7 +++++++ lib/fuse.c | 6 ++++++ m4/guestfs_libraries.m4 | 3 +++ mllib/unix_utils-c.c | 8 +++++++- p2v/main.c | 6 ++++++ 8 files changed, 43 insertions(+), 1 deletion(-) diff --git a/bootstrap b/bootstrap index 037d07e..faa10a3 100755 --- a/bootstrap +++ b/bootstrap @@ -92,6 +92,7 @@ strerror strndup symlinkat sys_select +sys_types sys_wait vasprintf vc-list-files diff --git a/cat/ls.c b/cat/ls.c index e0b5ff8..7568a5f 100644 --- a/cat/ls.c +++ b/cat/ls.c @@ -31,7 +31,14 @@ #include <assert.h> #include <time.h> #include <libintl.h> + +#if MAJOR_IN_MKDEV +#include <sys/mkdev.h> +#elif MAJOR_IN_SYSMACROS #include <sys/sysmacros.h> +#else +#include <sys/types.h> +#endif #include "human.h" #include "getprogname.h" diff --git a/daemon/mknod.c b/daemon/mknod.c index f8640ea..35dad15 100644 --- a/daemon/mknod.c +++ b/daemon/mknod.c @@ -25,7 +25,13 @@ #include <fcntl.h> #include <sys/types.h> #include <sys/stat.h> + +#if MAJOR_IN_MKDEV +#include <sys/mkdev.h> +#elif MAJOR_IN_SYSMACROS #include <sys/sysmacros.h> +/* else it's in sys/types.h, included above */ +#endif #include "guestfs_protocol.h" #include "daemon.h" diff --git a/diff/diff.c b/diff/diff.c index fc31ad3..8c045ad 100644 --- a/diff/diff.c +++ b/diff/diff.c @@ -32,7 +32,14 @@ #include <time.h> #include <libintl.h> #include <sys/wait.h> + +#if MAJOR_IN_MKDEV +#include <sys/mkdev.h> +#elif MAJOR_IN_SYSMACROS #include <sys/sysmacros.h> +#else +#include <sys/types.h> +#endif #include "c-ctype.h" #include "human.h" diff --git a/lib/fuse.c b/lib/fuse.c index 98bbc8d..7f80c49 100644 --- a/lib/fuse.c +++ b/lib/fuse.c @@ -26,7 +26,13 @@ #include <sys/wait.h> #include <string.h> #include <libintl.h> + +#if MAJOR_IN_MKDEV +#include <sys/mkdev.h> +#elif MAJOR_IN_SYSMACROS #include <sys/sysmacros.h> +/* else it's in sys/types.h, included above */ +#endif #if HAVE_FUSE /* See <attr/xattr.h> */ diff --git a/m4/guestfs_libraries.m4 b/m4/guestfs_libraries.m4 index 0882d27..c265b29 100644 --- a/m4/guestfs_libraries.m4 +++ b/m4/guestfs_libraries.m4 @@ -82,6 +82,9 @@ AC_CHECK_FUNCS([\ statvfs \ sync]) +dnl Which header file defines major, minor, makedev. +AC_HEADER_MAJOR + dnl Check for UNIX_PATH_MAX, creating a custom one if not available. AC_MSG_CHECKING([for UNIX_PATH_MAX]) AC_COMPILE_IFELSE([ diff --git a/mllib/unix_utils-c.c b/mllib/unix_utils-c.c index 1a4c3d7..f5aaaf6 100644 --- a/mllib/unix_utils-c.c +++ b/mllib/unix_utils-c.c @@ -27,9 +27,15 @@ #include <fnmatch.h> #include <errno.h> #include <sys/types.h> -#include <sys/sysmacros.h> #include <sys/statvfs.h> +#if MAJOR_IN_MKDEV +#include <sys/mkdev.h> +#elif MAJOR_IN_SYSMACROS +#include <sys/sysmacros.h> +/* else it's in sys/types.h, included above */ +#endif + #include <caml/alloc.h> #include <caml/fail.h> #include <caml/memory.h> diff --git a/p2v/main.c b/p2v/main.c index 42d3bbb..af14240 100644 --- a/p2v/main.c +++ b/p2v/main.c @@ -32,7 +32,13 @@ #include <libintl.h> #include <sys/types.h> #include <sys/stat.h> + +#if MAJOR_IN_MKDEV +#include <sys/mkdev.h> +#elif MAJOR_IN_SYSMACROS #include <sys/sysmacros.h> +/* else it's in sys/types.h, included above */ +#endif /* errors in <gtk.h> */ #pragma GCC diagnostic push -- 2.9.3

2 2

[PATCH] erlang: Rename 'message' to something less generic.
by Richard W.M. Jones 03 Mar '17

03 Mar '17

It's not possible to define an action which takes a parameter called 'message' because the Erlang bindings use that as the name of an internal variable. Solve this by renaming the Erlang internal variable. Fixes commit 84763d7fca3668c62ee3fe53d0e00a5a672f687b. --- generator/erlang.ml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/generator/erlang.ml b/generator/erlang.ml index 3753835..d0d2198 100644 --- a/generator/erlang.ml +++ b/generator/erlang.ml @@ -192,7 +192,7 @@ and generate_erlang_actions_h () = extern guestfs_h *g; -extern ETERM *dispatch (ETERM *message); +extern ETERM *dispatch (ETERM *args_tuple); extern int atom_equals (ETERM *atom, const char *name); extern ETERM *make_error (const char *funname); extern ETERM *unknown_optarg (const char *funname, ETERM *optargname); @@ -205,7 +205,7 @@ extern int get_bool (ETERM *term); extern int get_int (ETERM *term); extern int64_t get_int64 (ETERM *term); -#define ARG(i) (ERL_TUPLE_ELEMENT(message,(i)+1)) +#define ARG(i) (ERL_TUPLE_ELEMENT(args_tuple,(i)+1)) "; @@ -229,7 +229,7 @@ extern int64_t get_int64 (ETERM *term); List.iter ( fun { name = name } -> - pr "ETERM *run_%s (ETERM *message);\n" name + pr "ETERM *run_%s (ETERM *args_tuple);\n" name ) (actions |> external_functions |> sort); pr "\n"; @@ -355,7 +355,7 @@ instead of erl_interface. c_function = c_function; c_optarg_prefix = c_optarg_prefix } -> pr "\n"; pr "ETERM *\n"; - pr "run_%s (ETERM *message)\n" name; + pr "run_%s (ETERM *args_tuple)\n" name; pr "{\n"; iteri ( @@ -546,11 +546,11 @@ instead of erl_interface. #include \"actions.h\" ETERM * -dispatch (ETERM *message) +dispatch (ETERM *args_tuple) { ETERM *fun; - fun = ERL_TUPLE_ELEMENT (message, 0); + fun = ERL_TUPLE_ELEMENT (args_tuple, 0); /* XXX We should use gperf here. */ "; @@ -558,7 +558,7 @@ dispatch (ETERM *message) List.iter ( fun { name = name; style = ret, args, optargs } -> pr "if (atom_equals (fun, \"%s\"))\n" name; - pr " return run_%s (message);\n" name; + pr " return run_%s (args_tuple);\n" name; pr " else "; ) (actions |> external_functions |> sort); -- 2.9.3

2 1

[PATCH] generator: Move some deprecated functions to actions_core_deprecated.ml.
by Richard W.M. Jones 03 Mar '17

03 Mar '17

Fixes commit 97773d2bbee8e28830fd689deef9e9f63ce0c18e. --- generator/actions_core.ml | 83 ------------------------------------ generator/actions_core_deprecated.ml | 83 ++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 83 deletions(-) diff --git a/generator/actions_core.ml b/generator/actions_core.ml index e662827..ed89f74 100644 --- a/generator/actions_core.ml +++ b/generator/actions_core.ml @@ -1595,38 +1595,6 @@ Note that this call does not add the new disk to the handle. You may need to call C<guestfs_add_drive_opts> separately." }; { defaults with - name = "stat"; added = (1, 9, 2); - style = RStruct ("statbuf", "stat"), [Pathname "path"], []; - deprecated_by = Some "statns"; - tests = [ - InitISOFS, Always, TestResult ( - [["stat"; "/empty"]], "ret->size == 0"), [] - ]; - shortdesc = "get file information"; - longdesc = "\ -Returns file information for the given C<path>. - -This is the same as the L<stat(2)> system call." }; - - { defaults with - name = "lstat"; added = (1, 9, 2); - style = RStruct ("statbuf", "stat"), [Pathname "path"], []; - deprecated_by = Some "lstatns"; - tests = [ - InitISOFS, Always, TestResult ( - [["lstat"; "/empty"]], "ret->size == 0"), [] - ]; - shortdesc = "get file information for a symbolic link"; - longdesc = "\ -Returns file information for the given C<path>. - -This is the same as C<guestfs_stat> except that if C<path> -is a symbolic link, then the link is stat-ed, not the file it -refers to. - -This is the same as the L<lstat(2)> system call." }; - - { defaults with name = "c_pointer"; added = (1, 29, 17); style = RInt64 "ptr", [], []; fish_output = Some FishOutputHexadecimal; @@ -2259,28 +2227,6 @@ This creates an LVM logical volume called C<logvol> on the volume group C<volgroup>, with C<size> megabytes." }; { defaults with - name = "write_file"; added = (0, 0, 8); - style = RErr, [Pathname "path"; String "content"; Int "size"], []; - protocol_limit_warning = true; deprecated_by = Some "write"; - (* Regression test for RHBZ#597135. *) - tests = [ - InitScratchFS, Always, TestLastFail - [["write_file"; "/write_file"; "abc"; "10000"]], [] - ]; - shortdesc = "create a file"; - longdesc = "\ -This call creates a file called C<path>. The contents of the -file is the string C<content> (which can contain any 8 bit data), -with length C<size>. - -As a special case, if C<size> is C<0> -then the length is calculated using C<strlen> (so in this case -the content cannot contain embedded ASCII NULs). - -I<NB.> Owing to a bug, writing content containing ASCII NUL -characters does I<not> work, even if the length is specified." }; - - { defaults with name = "umount"; added = (0, 0, 8); style = RErr, [Dev_or_Path "pathordevice"], [OBool "force"; OBool "lazyunmount"]; fish_alias = ["unmount"]; @@ -5479,26 +5425,6 @@ calls to associate logical volumes and volume groups. See also C<guestfs_vgpvuuids>." }; { defaults with - name = "copy_size"; added = (1, 0, 87); - style = RErr, [Dev_or_Path "src"; Dev_or_Path "dest"; Int64 "size"], []; - progress = true; deprecated_by = Some "copy_device_to_device"; - tests = [ - InitScratchFS, Always, TestResult ( - [["mkdir"; "/copy_size"]; - ["write"; "/copy_size/src"; "hello, world"]; - ["copy_size"; "/copy_size/src"; "/copy_size/dest"; "5"]; - ["read_file"; "/copy_size/dest"]], - "compare_buffers (ret, size, \"hello\", 5) == 0"), [] - ]; - shortdesc = "copy size bytes from source to destination using dd"; - longdesc = "\ -This command copies exactly C<size> bytes from one source device -or file C<src> to another destination device or file C<dest>. - -Note this will fail if the source is too short or if the destination -is not large enough." }; - - { defaults with name = "zero_device"; added = (1, 3, 1); style = RErr, [Device "device"], []; progress = true; @@ -5785,15 +5711,6 @@ This command is the same as C<guestfs_pvresize> except that it allows you to specify the new size (in bytes) explicitly." }; { defaults with - name = "ntfsresize_size"; added = (1, 3, 14); - style = RErr, [Device "device"; Int64 "size"], []; - optional = Some "ntfsprogs"; deprecated_by = Some "ntfsresize"; - shortdesc = "resize an NTFS filesystem (with size)"; - longdesc = "\ -This command is the same as C<guestfs_ntfsresize> except that it -allows you to specify the new size (in bytes) explicitly." }; - - { defaults with name = "available_all_groups"; added = (1, 3, 15); style = RStringList "groups", [], []; tests = [ diff --git a/generator/actions_core_deprecated.ml b/generator/actions_core_deprecated.ml index 3c9b890..b8cca79 100644 --- a/generator/actions_core_deprecated.ml +++ b/generator/actions_core_deprecated.ml @@ -105,6 +105,38 @@ list a directory contents without making many round-trips. See also C<guestfs_lxattrlist> for a similarly efficient call for getting extended attributes." }; + { defaults with + name = "stat"; added = (1, 9, 2); + style = RStruct ("statbuf", "stat"), [Pathname "path"], []; + deprecated_by = Some "statns"; + tests = [ + InitISOFS, Always, TestResult ( + [["stat"; "/empty"]], "ret->size == 0"), [] + ]; + shortdesc = "get file information"; + longdesc = "\ +Returns file information for the given C<path>. + +This is the same as the L<stat(2)> system call." }; + + { defaults with + name = "lstat"; added = (1, 9, 2); + style = RStruct ("statbuf", "stat"), [Pathname "path"], []; + deprecated_by = Some "lstatns"; + tests = [ + InitISOFS, Always, TestResult ( + [["lstat"; "/empty"]], "ret->size == 0"), [] + ]; + shortdesc = "get file information for a symbolic link"; + longdesc = "\ +Returns file information for the given C<path>. + +This is the same as C<guestfs_stat> except that if C<path> +is a symbolic link, then the link is stat-ed, not the file it +refers to. + +This is the same as the L<lstat(2)> system call." }; + ] let daemon_functions = [ @@ -751,4 +783,55 @@ List the files in F<directory> in the format of 'ls -laZ'. This command is mostly useful for interactive sessions. It is I<not> intended that you try to parse the output string." }; + { defaults with + name = "write_file"; added = (0, 0, 8); + style = RErr, [Pathname "path"; String "content"; Int "size"], []; + protocol_limit_warning = true; deprecated_by = Some "write"; + (* Regression test for RHBZ#597135. *) + tests = [ + InitScratchFS, Always, TestLastFail + [["write_file"; "/write_file"; "abc"; "10000"]], [] + ]; + shortdesc = "create a file"; + longdesc = "\ +This call creates a file called C<path>. The contents of the +file is the string C<content> (which can contain any 8 bit data), +with length C<size>. + +As a special case, if C<size> is C<0> +then the length is calculated using C<strlen> (so in this case +the content cannot contain embedded ASCII NULs). + +I<NB.> Owing to a bug, writing content containing ASCII NUL +characters does I<not> work, even if the length is specified." }; + + { defaults with + name = "copy_size"; added = (1, 0, 87); + style = RErr, [Dev_or_Path "src"; Dev_or_Path "dest"; Int64 "size"], []; + progress = true; deprecated_by = Some "copy_device_to_device"; + tests = [ + InitScratchFS, Always, TestResult ( + [["mkdir"; "/copy_size"]; + ["write"; "/copy_size/src"; "hello, world"]; + ["copy_size"; "/copy_size/src"; "/copy_size/dest"; "5"]; + ["read_file"; "/copy_size/dest"]], + "compare_buffers (ret, size, \"hello\", 5) == 0"), [] + ]; + shortdesc = "copy size bytes from source to destination using dd"; + longdesc = "\ +This command copies exactly C<size> bytes from one source device +or file C<src> to another destination device or file C<dest>. + +Note this will fail if the source is too short or if the destination +is not large enough." }; + + { defaults with + name = "ntfsresize_size"; added = (1, 3, 14); + style = RErr, [Device "device"; Int64 "size"], []; + optional = Some "ntfsprogs"; deprecated_by = Some "ntfsresize"; + shortdesc = "resize an NTFS filesystem (with size)"; + longdesc = "\ +This command is the same as C<guestfs_ntfsresize> except that it +allows you to specify the new size (in bytes) explicitly." }; + ] -- 2.9.3

2 1

[PATCH supermin 0/2] Use AC_HEADER_MAJOR to find definitions of major, minor, makedev.
by Richard W.M. Jones 03 Mar '17

03 Mar '17

Quite a complex fix in the end, see the comment in the second commit. Rich.

2 3

[PATCH 0/5] Fix virt-rescue.
by Richard W.M. Jones 03 Mar '17

03 Mar '17

This fixes the main issues in virt-rescue and is usable. There are some enhancements which could be made (in follow up work): - An escape sequence and escape commands that could be handled by virt-rescue, eg. to shut down the appliance, mount or unmount filesystems. - `virt-rescue -i' could be implemented cleanly by performing the right API calls before handing control to the rescue shell. Rich.

1 5

[PATCH] parted: add more udev_settle calls.
by Dawid Zamirski 03 Mar '17

03 Mar '17

add udev_settle calls to print_partition_table and sgdisk_info_extract_field because the inspect-os calls guestfs_part_get_parttype and guestfs_part_get_gpt_guid for all parition devices found and this causes intermittent with opening block devices that are certainly present yet RESOLVE_DEVICE macro would fail wiht ENOENT. --- daemon/parted.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/daemon/parted.c b/daemon/parted.c index 124d1e9..03e83cb 100644 --- a/daemon/parted.c +++ b/daemon/parted.c @@ -320,6 +320,8 @@ print_partition_table (const char *device, bool add_m_option) CLEANUP_FREE char *err = NULL; int r; + udev_settle (); + if (add_m_option) r = command (&out, &err, str_parted, "-m", "-s", "--", device, "unit", "b", @@ -328,6 +330,9 @@ print_partition_table (const char *device, bool add_m_option) r = command (&out, &err, str_parted, "-s", "--", device, "unit", "b", "print", NULL); + + udev_settle (); + if (r == -1) { int errcode = 0; @@ -665,6 +670,8 @@ sgdisk_info_extract_field (const char *device, int partnum, const char *field, return NULL; } + udev_settle (); + CLEANUP_FREE char *err = NULL; int r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR, str_sgdisk, device, "-i", partnum_str, NULL); @@ -674,6 +681,8 @@ sgdisk_info_extract_field (const char *device, int partnum, const char *field, return NULL; } + udev_settle (); + CLEANUP_FREE_STRING_LIST char **lines = split_lines (err); if (lines == NULL) { reply_with_error ("'%s %s -i %i' returned no output", -- 2.9.3

2 2

[PATCH WIP 0/5] Fix virt-rescue.
by Richard W.M. Jones 03 Mar '17

03 Mar '17

This set of patches fixes virt-rescue rather cleanly. In particular the problems with handling ^C are completely fixed. Work still to be done before this can go upstream: - Shutdown doesn't work properly if you exit the shell. At the moment to exit you must do 'reboot -f'. Future improvements: - An escape sequence and escape commands that could be handled by virt-rescue, eg. to shut down the appliance, mount or unmount filesystems. - `virt-rescue -i' could be implemented cleanly by performing the right API calls before handling control to the rescue shell. I'm not intending the do the future stuff just yet. Rich.

1 5