[Libguestfs] [PATCH] Add command trace functionality
Matthew Booth
mbooth at redhat.com
Thu Sep 10 14:02:25 UTC 2009
On 09/09/09 17:49, Richard W.M. Jones wrote:
>> From 759de3f6289967a3c9978b4e947a38a4585f404c Mon Sep 17 00:00:00 2001
> From: Richard Jones<rjones at trick.home.annexia.org>
> Date: Wed, 9 Sep 2009 17:48:30 +0100
> Subject: [PATCH] Add command trace functionality.
>
> Enable this by calling guestfs_trace (handle, 1) or by
> setting the LIBGUESTFS_TRACE=1 environment variable.
> ---
> guestfish.pod | 4 +++
> guestfs.pod | 5 +++
> src/generator.ml | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/guestfs.c | 17 ++++++++++++
> 4 files changed, 100 insertions(+), 0 deletions(-)
>
> diff --git a/guestfish.pod b/guestfish.pod
> index 5427b23..3afc967 100644
> --- a/guestfish.pod
> +++ b/guestfish.pod
> @@ -575,6 +575,10 @@ Set the default qemu binary that libguestfs uses. If not set, then
> the qemu which was found at compile time by the configure script is
> used.
>
> +=item LIBGUESTFS_TRACE
> +
> +Set C<LIBGUESTFS_TRACE=1> to enable command traces.
> +
> =item PAGER
>
> The C<more> command uses C<$PAGER> as the pager. If not
> diff --git a/guestfs.pod b/guestfs.pod
> index d8e4da3..b8379d0 100644
> --- a/guestfs.pod
> +++ b/guestfs.pod
> @@ -983,6 +983,11 @@ used.
>
> See also L</QEMU WRAPPERS> above.
>
> +=item LIBGUESTFS_TRACE
> +
> +Set C<LIBGUESTFS_TRACE=1> to enable command traces. This
> +has the same effect as calling C<guestfs_set_trace (handle, 1)>.
> +
> =item TMPDIR
>
> Location of temporary directory, defaults to C</tmp>.
> diff --git a/src/generator.ml b/src/generator.ml
> index 765cb16..6184890 100755
> --- a/src/generator.ml
> +++ b/src/generator.ml
> @@ -805,6 +805,32 @@ is passed to the appliance at boot time. See C<guestfs_set_selinux>.
> For more information on the architecture of libguestfs,
> see L<guestfs(3)>.");
>
> + ("set_trace", (RErr, [Bool "trace"]), -1, [FishAlias "trace"],
> + [InitNone, Always, TestOutputTrue (
> + [["set_trace"; "true"];
> + ["get_trace"]])],
> + "enable or disable command traces",
> + "\
> +If the command trace flag is set to 1, then commands are
> +printed on stdout before they are executed in a format
> +which is very similar to the one used by guestfish. In
> +other words, you can run a program with this enabled, and
> +you will get out a script which you can feed to guestfish
> +to perform the same set of actions.
> +
> +If you want to trace C API calls into libguestfs (and
> +other libraries) then possibly a better way is to use
> +the external ltrace(1) command.
> +
> +Command traces are disabled unless the environment variable
> +C<LIBGUESTFS_TRACE> is defined and set to C<1>.");
> +
> + ("get_trace", (RBool "trace", []), -1, [],
> + [],
> + "get command trace enabled flag",
> + "\
> +Return the command trace flag.");
> +
> ]
>
> (* daemon_functions are any functions which cause some action
> @@ -4630,6 +4656,52 @@ check_state (guestfs_h *g, const char *caller)
>
> ";
>
> + (* Generate code to generate guestfish call traces. *)
> + let trace_call shortname style =
> + pr " if (guestfs__get_trace (g)) {\n";
> +
> + let needs_i =
> + List.exists (function
> + | StringList _ | DeviceList _ -> true
> + | _ -> false) (snd style) in
> + if needs_i then (
> + pr " int i;\n";
> + pr "\n"
> + );
> +
> + pr " printf (\"%%s\", \"%s\");\n" shortname;
> + List.iter (
> + function
> + | String n (* strings *)
> + | Device n
> + | Pathname n
> + | Dev_or_Path n
> + | FileIn n
> + | FileOut n ->
> + (* guestfish doesn't support string escaping, so neither do we *)
> + pr " printf (\" \\\"%%s\\\"\", %s);\n" n
> + | OptString n -> (* string option *)
> + pr " if (%s) printf (\" \\\"%%s\\\"\", %s);\n" n n;
> + pr " else printf (\" null\");\n"
> + | StringList n
> + | DeviceList n -> (* string list *)
> + pr " printf (\"\\\"\");\n";
There's a missing leading space in the above line.
> + pr " for (i = 0; %s[i]; ++i) {\n" n;
> + pr " if (i> 0) putchar (' ');\n";
> + pr " printf (\"%%s\", %s[i]);\n" n;
> + pr " }\n";
> + pr " printf (\"\\\"\");\n";
As discussed on the list, this results in commands which can't be
executed in guestfish because argument groupings are lost when arguments
contain spaces. While this is a limitation of guestfish rather than this
patch, a fix in guestfish will require a modification to this patch. Can
we hold off on this until guestfish is fixed?
> + | Bool n -> (* boolean *)
> + pr " if (%s) printf (\" true\");\n" n;
> + pr " else printf (\" false\");\n"
> + | Int n -> (* int *)
> + pr " printf (\" %%d\", %s);\n" n
> + ) (snd style);
> + pr " printf (\"\\n\");\n";
> + pr " }\n";
> + pr "\n";
> + in
> +
> (* For non-daemon functions, generate a wrapper around each function. *)
> List.iter (
> fun (shortname, style, _, _, _, _, _) ->
> @@ -4638,6 +4710,7 @@ check_state (guestfs_h *g, const char *caller)
> generate_prototype ~extern:false ~semicolon:false ~newline:true
> ~handle:"g" name style;
> pr "{\n";
> + trace_call shortname style;
> pr " return guestfs__%s " shortname;
> generate_c_call_args ~handle:"g" style;
> pr ";\n";
> @@ -4745,6 +4818,7 @@ check_state (guestfs_h *g, const char *caller)
> pr " guestfs_main_loop *ml = guestfs_get_main_loop (g);\n";
> pr " int serial;\n";
> pr "\n";
> + trace_call shortname style;
> pr " if (check_state (g, \"%s\") == -1) return %s;\n" name error_code;
> pr " guestfs_set_busy (g);\n";
> pr "\n";
> diff --git a/src/guestfs.c b/src/guestfs.c
> index 571205f..98d99b8 100644
> --- a/src/guestfs.c
> +++ b/src/guestfs.c
> @@ -170,6 +170,7 @@ struct guestfs_h
> int cmdline_size;
>
> int verbose;
> + int trace;
> int autosync;
>
> char *path; /* Path to kernel, initrd. */
> @@ -238,6 +239,9 @@ guestfs_create (void)
> str = getenv ("LIBGUESTFS_DEBUG");
> g->verbose = str != NULL&& strcmp (str, "1") == 0;
>
> + str = getenv ("LIBGUESTFS_TRACE");
> + g->trace = str != NULL&& strcmp (str, "1") == 0;
> +
> str = getenv ("LIBGUESTFS_PATH");
> g->path = str != NULL ? strdup (str) : strdup (GUESTFS_DEFAULT_PATH);
> if (!g->path) goto error;
> @@ -734,6 +738,19 @@ guestfs__version (guestfs_h *g)
> return r;
> }
>
> +int
> +guestfs__set_trace (guestfs_h *g, int t)
> +{
> + g->trace = !!t;
> + return 0;
> +}
> +
> +int
> +guestfs__get_trace (guestfs_h *g)
> +{
> + return g->trace;
> +}
> +
> /* Add a string to the current command line. */
> static void
> incr_cmdline_size (guestfs_h *g)
> -- 1.6.2.5
I really like this. It *almost* works in practise. I've found 2 limitations:
String list splitting doesn't work in guestfish. See note above and
expect a patch.
It spits out internal state changes. Look at this output:
add_drive "/var/lib/virt-snapshot/images/RHEL39FV32-hda-1252577522.qcow2"
config "-drive"
"file=/var/lib/virt-snapshot/images/RHEL39FV32-hda-1252577522.qcow2,cache=off,if=ide"
add_cdrom "/tmp/4BQ6oiKKAJ.iso"
config "-cdrom" "/tmp/4BQ6oiKKAJ.iso"
set_selinux true
launch
wait_ready
list_partitions
is_ready
set_busy
end_busy
pvs
is_ready
set_busy
end_busy
lvs
is_ready
set_busy
end_busy
file "/dev/sda1"
is_ready
We also discussed this on the list. I don't think this is a problem with
this patch as such, but that state transition functions are part of the
external API. These need to be removed from *all* bindings, including
guestfish.
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
More information about the Libguestfs
mailing list