Re: [libvirt] [PATCH] Take DryRun code out of virCommand* code flow

Since dry-run of cmd is just for tests now, it would be better to remove dry-run code and move them to testutils. Then the code flow in virCommand* could be more general. There are 3 steps in this patch: 1. Introduce a new global hook (of virExecHook type) which will be called in code flow just before the cmd->hook. The global hook is also called in child process. If it returns 1, the child process will exit with status in advance and the parent will process io and wait for the child normally. It prepares for registering dry-run and anything else. The virCommandSetPreExecHook is modified for registering both types of hooks. 2. Implement virTestSetDryRun with dry-run code in "testutils.c". It substitutes for virCommandSetDryRun. The virTestSetDryRun invokes virCommandSetPreExecHook to register a function on the global hook which will fill in cmdline buffer and call callback for tests. 3. Remove all dryrun code in "vircommand.c" and remove virCommandSetDryRun API. Diffs from old dry-run: The new global hook is called in child process. So dryrun-callback for tests should write to stdout/stderr when they need output something. Now the opaque argument for dryrun-callback cannot be inout. In "tests/viriscsitest.c", iface_created in opaque of callback is an inout argument. There's a bit complicated to transfer it between the child and its parent. So this patch use a temporary file to do that. Signed-off-by: Shi Lei <shi_lei(a)massclouds.com&gt; --- docs/internals/command.html.in | 33 +++++- src/libvirt_private.syms | 4 +- src/qemu/qemu_process.c | 8 +- src/util/vircommand.c | 151 ++++++++++++---------------- src/util/vircommand.h | 18 +++- src/util/vircommandpriv.h | 19 ++-- tests/networkxml2firewalltest.c | 7 +- tests/nwfilterebiptablestest.c | 32 +++--- tests/nwfilterxml2firewalltest.c | 6 +- tests/testutils.c | 210 +++++++++++++++++++++++++++++++++++++++ tests/testutils.h | 2 + tests/virfirewalltest.c | 71 ++++++------- tests/viriscsitest.c | 120 ++++++++++++++++------ tests/virkmodtest.c | 10 +- tests/virnetdevbandwidthtest.c | 6 +- 15 files changed, 482 insertions(+), 215 deletions(-) diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in index 43f51a4..44478f0 100644 --- a/docs/internals/command.html.in +++ b/docs/internals/command.html.in @@ -398,17 +398,40 @@ virCommandSetWorkingDirectory(cmd, LOCALSTATEDIR); <h3><a id="hooks">Any additional hooks</a></h3> <p> - If anything else is needed, it is possible to request a hook - function that is called in the child after the fork, as the + If anything else is needed, it is possible to request hook + functions that are called in the child after the fork, as the last thing before changing directories, dropping capabilities, - and executing the new process. If hook(opaque) returns - non-zero, then the child process will not be run. + and executing the new process. + + There are two types of hooks: + <ul> + <li>Global hook takes effect to all command. + Setting global hook if passing NULL to @cmd. + </li> + <li>Command hook takes effect only to the command. + Passing the pointer of the command to @cmd. + </li> + </ul> + + If hook(opaque) returns -1, then the child process will not be + run for error. And if it returns 1, the child process will exit + with status in advance and this mode is used for command dryrun. + + Since @hook runs in the child, it should be careful to avoid + any functions that are not async-signal-safe. + + For global hook, Passing NULL for @cmd, @hook and @opaque to + cancel the effect. </p> <pre> -virCommandSetPreExecHook(cmd, hook, opaque); +virCommandSetPreExecHook(NULL, hook, opaque); /* global hook */ +virCommandSetPreExecHook(cmd, hook, opaque); /* command hook */ + +virCommandSetPreExecHook(NULL, NULL, NULL); /* cancel global hook */ </pre>

void virCommandSetPreExecHook(virCommandPtr cmd, virExecHook hook, void *opaque) { - if (!cmd || cmd->has_error) + if (!cmd) { + /* Global hook */ + preExecHook = hook; + preExecOpaque = opaque; + return; + }