[Libguestfs] [PATCH] build: Require qemu >= 1.3.0 and yajl.

Require qemu >= 1.3.0, the first version that supported `qemu-img --output=json'. This means we require yajl (for parsing the JSON output of qemu-img), and that in turn has consequences elsewhere. --- README | 10 +- builder/Makefile.am | 2 - builder/sources.ml | 9 +- builder/yajl-c.c | 30 ----- builder/yajl.ml | 2 - builder/yajl.mli | 4 - builder/yajl_tests.ml | 2 - daemon/ldm.c | 10 -- m4/guestfs_libraries.m4 | 9 +- src/guestfs-internal.h | 6 - src/info.c | 349 +++--------------------------------------------- 11 files changed, 25 insertions(+), 408 deletions(-) diff --git a/README b/README index bf5542a..b3009a0 100644 --- a/README +++ b/README @@ -52,9 +52,9 @@ The full requirements are described below. | This installs the disk management tools required by the appliance. The | | list below is *additional* packages needed on the host. | +--------------+-------------+---+-----------------------------------------+ -| qemu | 1.2.0 | R | 1.1 may work, but has broken virtio-scsi| +| qemu | 1.3.0 | R | 1.1 may work, but has broken virtio-scsi| +--------------+-------------+---+-----------------------------------------+ -| qemu-img | | R | >= 2.2.0 is required for virt-v2v but | +| qemu-img | 1.3.0 | R | >= 2.2.0 is required for virt-v2v but | | | | | optional elsewhere | +--------------+-------------+---+-----------------------------------------+ | kernel | 2.6.34 | R | Make sure the following are enabled | @@ -115,6 +115,9 @@ The full requirements are described below. | xz | | R | Used to compress disk images. | | | | | Used by virt-builder for compression. | +--------------+-------------+---+-----------------------------------------+ +| yajl | 2.0.4 | R | JSON parser for parsing output of | +| | | | ldmtool and qemu-img info commands. | ++--------------+-------------+---+-----------------------------------------+ | po4a | |R/O| Required if compiling from git. | | | | | Optional if compiling from tarball. | | | | | For localizing man pages. | @@ -154,9 +157,6 @@ The full requirements are described below. +--------------+-------------+---+-----------------------------------------+ | sd-journal | | O | systemd journal library | +--------------+-------------+---+-----------------------------------------+ -| yajl | 2.0.4 | O | JSON parser for parsing output of | -| | | | ldmtool and qemu-img info commands. | -+--------------+-------------+---+-----------------------------------------+ | gdisk | | O | GPT disk support. | +--------------+-------------+---+-----------------------------------------+ | netpbm | | O | Render icons from guests. | diff --git a/builder/Makefile.am b/builder/Makefile.am index a2509d7..ab27442 100644 --- a/builder/Makefile.am +++ b/builder/Makefile.am @@ -286,9 +286,7 @@ TESTS = \ test-virt-index-validate.sh check_PROGRAMS = -if HAVE_YAJL TESTS += test-virt-builder-list-simplestreams.sh -endif if ENABLE_APPLIANCE TESTS += test-virt-builder.sh diff --git a/builder/sources.ml b/builder/sources.ml index 149db6f..37027d6 100644 --- a/builder/sources.ml +++ b/builder/sources.ml @@ -83,14 +83,7 @@ let parse_conf file = try (match (List.assoc ("format", None) fields) with | "native" | "" -> FormatNative - | "simplestreams" as fmt -> - if not (Yajl.yajl_is_available ()) then ( - if verbose () then ( - eprintf (f_"%s: repository type '%s' not supported (missing YAJL support), skipping it\n") prog fmt; - ); - invalid_arg fmt - ) else - FormatSimpleStreams + | "simplestreams" -> FormatSimpleStreams | fmt -> if verbose () then ( eprintf (f_"%s: unknown repository type '%s' in %s, skipping it\n") prog fmt file; diff --git a/builder/yajl-c.c b/builder/yajl-c.c index 194fa34..f34196e 100644 --- a/builder/yajl-c.c +++ b/builder/yajl-c.c @@ -23,18 +23,13 @@ #include <caml/memory.h> #include <caml/mlvalues.h> -#if HAVE_YAJL #include <yajl/yajl_tree.h> -#endif #include <stdio.h> #include <string.h> #define Val_none (Val_int (0)) -value virt_builder_yajl_is_available (value unit); - -#if HAVE_YAJL value virt_builder_yajl_tree_parse (value stringv); static value @@ -95,13 +90,6 @@ convert_yajl_value (yajl_val val, int level) } value -virt_builder_yajl_is_available (value unit) -{ - /* NB: noalloc */ - return Val_true; -} - -value virt_builder_yajl_tree_parse (value stringv) { CAMLparam1 (stringv); @@ -124,21 +112,3 @@ virt_builder_yajl_tree_parse (value stringv) CAMLreturn (rv); } - -#else -value virt_builder_yajl_tree_parse (value stringv) __attribute__((noreturn)); - -value -virt_builder_yajl_is_available (value unit) -{ - /* NB: noalloc */ - return Val_false; -} - -value -virt_builder_yajl_tree_parse (value stringv) -{ - caml_invalid_argument ("virt-builder was compiled without yajl support"); -} - -#endif diff --git a/builder/yajl.ml b/builder/yajl.ml index f2d5c2b..00e4dac 100644 --- a/builder/yajl.ml +++ b/builder/yajl.ml @@ -25,6 +25,4 @@ type yajl_val = | Yajl_array of yajl_val array | Yajl_bool of bool -external yajl_is_available : unit -> bool = "virt_builder_yajl_is_available" "noalloc" - external yajl_tree_parse : string -> yajl_val = "virt_builder_yajl_tree_parse" diff --git a/builder/yajl.mli b/builder/yajl.mli index aaa9389..9771e53 100644 --- a/builder/yajl.mli +++ b/builder/yajl.mli @@ -25,9 +25,5 @@ type yajl_val = | Yajl_array of yajl_val array | Yajl_bool of bool -val yajl_is_available : unit -> bool -(** Is YAJL built in? If not, calling any of the other yajl_* - functions will result in an error. *) - val yajl_tree_parse : string -> yajl_val (** Parse the JSON string. *) diff --git a/builder/yajl_tests.ml b/builder/yajl_tests.ml index 344a8db..f5a44f2 100644 --- a/builder/yajl_tests.ml +++ b/builder/yajl_tests.ml @@ -134,6 +134,4 @@ let suite = ] let () = - if not (yajl_is_available ()) then - exit 77; run_test_tt_main suite diff --git a/daemon/ldm.c b/daemon/ldm.c index 3705aa4..71cdf46 100644 --- a/daemon/ldm.c +++ b/daemon/ldm.c @@ -26,16 +26,12 @@ #include <glob.h> #include <string.h> -#if HAVE_YAJL #include <yajl/yajl_tree.h> -#endif #include "daemon.h" #include "actions.h" #include "optgroups.h" -#if HAVE_YAJL - GUESTFSD_EXT_CMD(str_ldmtool, ldmtool); int @@ -441,9 +437,3 @@ do_ldmtool_volume_partitions (const char *diskgroup, const char *volume) return parse_json_get_object_string_list (out, "partitions", __func__, "ldmtool show volume"); } - -#else /* !HAVE_YAJL */ - -OPTGROUP_LDM_NOT_AVAILABLE - -#endif diff --git a/m4/guestfs_libraries.m4 b/m4/guestfs_libraries.m4 index 0187c20..c5a4a01 100644 --- a/m4/guestfs_libraries.m4 +++ b/m4/guestfs_libraries.m4 @@ -261,13 +261,8 @@ LIBS="$LIBS $LIBXML2_LIBS" AC_CHECK_FUNCS([xmlBufferDetach]) LIBS="$old_LIBS" -dnl Check for yajl JSON library (optional). -PKG_CHECK_MODULES([YAJL], [yajl >= 2.0.4], [ - AC_SUBST([YAJL_CFLAGS]) - AC_SUBST([YAJL_LIBS]) - AC_DEFINE([HAVE_YAJL],[1],[Define to 1 if you have yajl.]) -],[AC_MSG_WARN([yajl not found, some features will be disabled])]) -AM_CONDITIONAL([HAVE_YAJL],[test "x$YAJL_LIBS" != "x"]) +dnl Check for yajl JSON library (required). +PKG_CHECK_MODULES([YAJL], [yajl >= 2.0.4]) dnl Check for C++ (optional, we just use this to test the header works). AC_PROG_CXX diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 6ba056e..26dde67 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -477,12 +477,6 @@ struct guestfs_h */ int unique; - /* In src/info.c: Use new (JSON) or old (human) qemu-img info parser. */ - int qemu_img_info_parser; -#define QEMU_IMG_INFO_UNKNOWN_PARSER 0 -#define QEMU_IMG_INFO_NEW_PARSER 1 -#define QEMU_IMG_INFO_OLD_PARSER 2 - /*** Protocol. ***/ struct connection *conn; /* Connection to appliance. */ int msg_next_serial; diff --git a/src/info.c b/src/info.c index 616ef50..9791730 100644 --- a/src/info.c +++ b/src/info.c @@ -38,92 +38,31 @@ #include <sys/resource.h> #endif -#if HAVE_YAJL #include <yajl/yajl_tree.h> -#endif #include "guestfs.h" #include "guestfs-internal.h" #include "guestfs-internal-actions.h" -static int which_parser (guestfs_h *g); -static char *get_disk_format (guestfs_h *g, const char *filename); -static int64_t get_disk_virtual_size (guestfs_h *g, const char *filename); -static int get_disk_has_backing_file (guestfs_h *g, const char *filename); -#if HAVE_YAJL +#ifdef HAVE_ATTRIBUTE_CLEANUP +#define CLEANUP_YAJL_TREE_FREE __attribute__((cleanup(cleanup_yajl_tree_free))) + +static void +cleanup_yajl_tree_free (void *ptr) +{ + yajl_tree_free (* (yajl_val *) ptr); +} + +#else +#define CLEANUP_YAJL_TREE_FREE +#endif + static yajl_val get_json_output (guestfs_h *g, const char *filename); -#endif -static char *old_parser_disk_format (guestfs_h *g, const char *filename); -static int64_t old_parser_disk_virtual_size (guestfs_h *g, const char *filename); -static int old_parser_disk_has_backing_file (guestfs_h *g, const char *filename); static void set_child_rlimits (struct command *); char * guestfs_impl_disk_format (guestfs_h *g, const char *filename) { - switch (which_parser (g)) { - case QEMU_IMG_INFO_NEW_PARSER: - return get_disk_format (g, filename); - case QEMU_IMG_INFO_OLD_PARSER: - return old_parser_disk_format (g, filename); - case QEMU_IMG_INFO_UNKNOWN_PARSER: - abort (); - } - - abort (); -} - -int64_t -guestfs_impl_disk_virtual_size (guestfs_h *g, const char *filename) -{ - switch (which_parser (g)) { - case QEMU_IMG_INFO_NEW_PARSER: - return get_disk_virtual_size (g, filename); - case QEMU_IMG_INFO_OLD_PARSER: - return old_parser_disk_virtual_size (g, filename); - case QEMU_IMG_INFO_UNKNOWN_PARSER: - abort (); - } - - abort (); -} - -int -guestfs_impl_disk_has_backing_file (guestfs_h *g, const char *filename) -{ - switch (which_parser (g)) { - case QEMU_IMG_INFO_NEW_PARSER: - return get_disk_has_backing_file (g, filename); - case QEMU_IMG_INFO_OLD_PARSER: - return old_parser_disk_has_backing_file (g, filename); - case QEMU_IMG_INFO_UNKNOWN_PARSER: - abort (); - } - - abort (); -} - -#if HAVE_YAJL - -# ifdef HAVE_ATTRIBUTE_CLEANUP -# define CLEANUP_YAJL_TREE_FREE __attribute__((cleanup(cleanup_yajl_tree_free))) - -static void -cleanup_yajl_tree_free (void *ptr) -{ - yajl_tree_free (* (yajl_val *) ptr); -} - -# else -# define CLEANUP_YAJL_TREE_FREE -# endif -#endif - -static char * -get_disk_format (guestfs_h *g, const char *filename) -{ -#if HAVE_YAJL - size_t i, len; CLEANUP_YAJL_TREE_FREE yajl_val tree = get_json_output (g, filename); @@ -150,17 +89,11 @@ get_disk_format (guestfs_h *g, const char *filename) bad_type: error (g, _("qemu-img info: JSON output did not contain 'format' key")); return NULL; - -#else /* !HAVE_YAJL */ - abort (); -#endif /* !HAVE_YAJL */ } -static int64_t -get_disk_virtual_size (guestfs_h *g, const char *filename) +int64_t +guestfs_impl_disk_virtual_size (guestfs_h *g, const char *filename) { -#if HAVE_YAJL - size_t i, len; CLEANUP_YAJL_TREE_FREE yajl_val tree = get_json_output (g, filename); @@ -189,17 +122,11 @@ get_disk_virtual_size (guestfs_h *g, const char *filename) bad_type: error (g, _("qemu-img info: JSON output did not contain 'virtual-size' key")); return -1; - -#else /* !HAVE_YAJL */ - abort (); -#endif /* !HAVE_YAJL */ } -static int -get_disk_has_backing_file (guestfs_h *g, const char *filename) +int +guestfs_impl_disk_has_backing_file (guestfs_h *g, const char *filename) { -#if HAVE_YAJL - size_t i, len; CLEANUP_YAJL_TREE_FREE yajl_val tree = get_json_output (g, filename); @@ -227,14 +154,8 @@ get_disk_has_backing_file (guestfs_h *g, const char *filename) bad_type: error (g, _("qemu-img info: JSON output was not an object")); return -1; - -#else /* !HAVE_YAJL */ - abort (); -#endif /* !HAVE_YAJL */ } -#if HAVE_YAJL - /* Run 'qemu-img info --output json filename', and parse the output * as JSON, returning a JSON tree and handling errors. */ @@ -332,242 +253,6 @@ parse_json (guestfs_h *g, void *treevp, const char *input, size_t len) } } -static void help_contains_output_json (guestfs_h *g, void *datav, const char *help_line, size_t len); - -/* Choose new (JSON) or old (human) parser? */ -static int -which_parser (guestfs_h *g) -{ - if (g->qemu_img_info_parser == QEMU_IMG_INFO_UNKNOWN_PARSER) { - int qemu_img_supports_json = 0; - CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g); - - guestfs_int_cmd_add_arg (cmd, "qemu-img"); - guestfs_int_cmd_add_arg (cmd, "--help"); - guestfs_int_cmd_set_stdout_callback (cmd, - help_contains_output_json, - &qemu_img_supports_json, 0); - guestfs_int_cmd_run (cmd); - /* ignore return code, which would usually be 1 */ - - if (qemu_img_supports_json) - g->qemu_img_info_parser = QEMU_IMG_INFO_NEW_PARSER; - else - g->qemu_img_info_parser = QEMU_IMG_INFO_OLD_PARSER; - } - - debug (g, "%s: g->qemu_img_info_parser = %d", - __func__, g->qemu_img_info_parser); - - return g->qemu_img_info_parser; -} - -static void -help_contains_output_json (guestfs_h *g, void *datav, - const char *help_line, size_t len) -{ - if (strstr (help_line, "--output") != NULL && - strstr (help_line, "json") != NULL) { - * (int *) datav = 1; - } -} - -#else /* !HAVE_YAJL */ - -/* With no YAJL, only the old parser is available. */ -static int -which_parser (guestfs_h *g) -{ - return g->qemu_img_info_parser = QEMU_IMG_INFO_OLD_PARSER; -} - -#endif /* !HAVE_YAJL */ - -/*---------------------------------------------------------------------- - * This is the old parser for the old / human-readable output of - * qemu-img info, ONLY used if EITHER you've got an old version of - * qemu-img, OR you're not using yajl. It is highly recommended that - * you upgrade qemu-img and install yajl so that you can use the new, - * secure JSON parser above. - */ - -static int old_parser_run_qemu_img_info (guestfs_h *g, const char *filename, cmd_stdout_callback cb, void *data); - -/* NB: For security reasons, the check_* callbacks MUST bail - * after seeing the first line that matches /^backing file: /. See: - * https://lists.gnu.org/archive/html/qemu-devel/2012-09/msg00137.html - */ - -struct old_parser_check_data { - int stop, failed; - union { - char *ret; - int reti; - int64_t reti64; - }; -}; - -static void old_parser_check_disk_format (guestfs_h *g, void *data, const char *line, size_t len); -static void old_parser_check_disk_virtual_size (guestfs_h *g, void *data, const char *line, size_t len); -static void old_parser_check_disk_has_backing_file (guestfs_h *g, void *data, const char *line, size_t len); - -static char * -old_parser_disk_format (guestfs_h *g, const char *filename) -{ - struct old_parser_check_data data; - - memset (&data, 0, sizeof data); - - if (old_parser_run_qemu_img_info (g, filename, - old_parser_check_disk_format, - &data) == -1) { - free (data.ret); - return NULL; - } - - if (data.ret == NULL) - data.ret = safe_strdup (g, "unknown"); - - return data.ret; -} - -static void -old_parser_check_disk_format (guestfs_h *g, void *datav, - const char *line, size_t len) -{ - struct old_parser_check_data *data = datav; - const char *p; - - if (data->stop) - return; - - if (STRPREFIX (line, "backing file: ")) { - data->stop = 1; - return; - } - - if (STRPREFIX (line, "file format: ")) { - p = &line[13]; - data->ret = safe_strdup (g, p); - data->stop = 1; - } -} - -static int64_t -old_parser_disk_virtual_size (guestfs_h *g, const char *filename) -{ - struct old_parser_check_data data; - - memset (&data, 0, sizeof data); - - if (old_parser_run_qemu_img_info (g, filename, - old_parser_check_disk_virtual_size, - &data) == -1) - return -1; - - if (data.failed) - error (g, _("%s: cannot detect virtual size of disk image"), filename); - - return data.reti64; -} - -static void -old_parser_check_disk_virtual_size (guestfs_h *g, void *datav, - const char *line, size_t len) -{ - struct old_parser_check_data *data = datav; - const char *p; - - if (data->stop) - return; - - if (STRPREFIX (line, "backing file: ")) { - data->stop = 1; - return; - } - - if (STRPREFIX (line, "virtual size: ")) { - /* "virtual size: 500M (524288000 bytes)\n" */ - p = &line[14]; - p = strchr (p, ' '); - if (!p || p[1] != '(' || sscanf (&p[2], "%" SCNi64, &data->reti64) != 1) - data->failed = 1; - data->stop = 1; - } -} - -static int -old_parser_disk_has_backing_file (guestfs_h *g, const char *filename) -{ - struct old_parser_check_data data; - - memset (&data, 0, sizeof data); - - if (old_parser_run_qemu_img_info (g, filename, - old_parser_check_disk_has_backing_file, - &data) == -1) - return -1; - - return data.reti; -} - -static void -old_parser_check_disk_has_backing_file (guestfs_h *g, void *datav, - const char *line, size_t len) -{ - struct old_parser_check_data *data = datav; - - if (data->stop) - return; - - if (STRPREFIX (line, "backing file: ")) { - data->reti = 1; - data->stop = 1; - } -} - -static int -old_parser_run_qemu_img_info (guestfs_h *g, const char *filename, - cmd_stdout_callback fn, void *data) -{ - CLEANUP_FREE char *abs_filename = NULL; - CLEANUP_FREE char *safe_filename = NULL; - CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g); - int r; - - if (guestfs_int_lazy_make_tmpdir (g) == -1) - return -1; - - safe_filename = safe_asprintf (g, "%s/format.%d", g->tmpdir, ++g->unique); - - /* 'filename' must be an absolute path so we can link to it. */ - abs_filename = realpath (filename, NULL); - if (abs_filename == NULL) { - perrorf (g, "realpath"); - return -1; - } - - if (symlink (abs_filename, safe_filename) == -1) { - perrorf (g, "symlink"); - return -1; - } - - guestfs_int_cmd_add_arg (cmd, "qemu-img"); - guestfs_int_cmd_add_arg (cmd, "info"); - guestfs_int_cmd_add_arg (cmd, safe_filename); - guestfs_int_cmd_set_stdout_callback (cmd, fn, data, 0); - set_child_rlimits (cmd); - r = guestfs_int_cmd_run (cmd); - if (r == -1) - return -1; - if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { - guestfs_int_external_command_failed (g, r, "qemu-img info", filename); - return -1; - } - - return 0; -} - static void set_child_rlimits (struct command *cmd) { -- 2.5.0