On Fri, Mar 26, 2010 at 10:53:01AM -0600, Eric Blake wrote:
On 03/26/2010 09:43 AM, Daniel Veillard wrote:
+ */ + if ((virHooksFound == -1) || + ((driver == VIR_HOOK_DRIVER_DAEMON) && + (op == VIR_HOOK_DAEMON_OP_RELOAD))) + virHookInitialize(); + ... + /* + * Convenience macros borrowed from qemudBuildCommandLine() + */
Duplicating the definition of all these helpers is a bit of a long distraction in the middle of this function; is it worth a helper file, where we could #include "command_line_builder.h" to get all these helpers defined, and to reduce the duplication from qemudBuildCommandLine by having the macros in one place?
I'd like to kill off all these macros completely, and instead introduce a formal API. In a similar style to virBuffer, have a statically initialized struct, APIs to add args & env variables, and a final API to check whether any OOM errors occurred. Then a thin wrapper for virExec/virRun to actually execute it typedef struct { int hasError; char **argv; char **env; } virCommandInfo; virCommandInfo cmd = VIR_COMMAND_INFO_INITIALIZER; virCommandInfoAddArg(info, "/usr/bin/kvm"); virCommandInfoCopyEnv(info, "PATH"); virCommandInfoSetEnv(info, "LANG", "C"); virCommandInfoAddArg(info, "--foo"); virCommandInfoAddArg(info, "--bar"); if (virCommandInfoHasError(info)) virReportOOMError(); virRunCommand(info); Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|