On 09/09/09 17:49, Richard W.M. Jones wrote:

...

From 759de3f6289967a3c9978b4e947a38a4585f404c Mon Sep 17 00:00:00 2001 From: Richard Jones<rjones@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