On 11/23/2010 04:49 PM, Eric Blake wrote: > This should address some of the review comments from the first round > (https://www.redhat.com/archives/libvir-list/2010-November/msg00803.html). > > Daniel P. Berrange (5): > Introduce new APIs for spawning processes > virCommand: docs for usage of new command APIs > Port hooks and iptables code to new command execution APIs > uml: convert to virCommand > Remove bogus includes > > Eric Blake (3): > util: add virVasprintf > util: fix saferead type > qemu: convert to virCommand Here's the interdiff from v1 (well, from v1 + untested uml patches), if it helps review. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
diff --git a/.x-sc_prohibit_asprintf b/.x-sc_prohibit_asprintf
index 614c238..d03b947 100644
--- a/.x-sc_prohibit_asprintf
+++ b/.x-sc_prohibit_asprintf
@@ -1,3 +1,5 @@
+ChangeLog
+^bootstrap.conf$
^gnulib/
^po/
-ChangeLog
+^src/util/util.c$
diff --git a/cfg.mk b/cfg.mk
index 2ac3c69..6312632 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -243,7 +243,7 @@ sc_prohibit_strncmp:
# Use virAsprintf rather than as'printf since *strp is undefined on error.
sc_prohibit_asprintf:
- @prohibit='\<a[s]printf\>' \
+ @prohibit='\<v?a[s]printf\>' \
halt='use virAsprintf, not as'printf \
$(_sc_search_regexp)
diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in
index 3d28cd4..c4fa800 100644
--- a/docs/internals/command.html.in
+++ b/docs/internals/command.html.in
@@ -111,6 +111,15 @@
</pre>
<p>
+ If an argument needs to be formatted as if by
+ <code>printf</code>:
+ </p>
+
+<pre>
+ virCommandAddArgFormat(cmd, "%d", count);
+</pre>
+
+ <p>
To add a entire NULL terminated array of arguments in one go,
there are two options.
</p>
@@ -124,8 +133,8 @@
</pre>
<p>
- This latter option can also be used at time of initial
- construction of the <code>virCommandPtr</code> object
+ This can also be done at the time of initial construction of
+ the <code>virCommandPtr</code> object:
</p>
<pre>
@@ -134,7 +143,9 @@
"--strict-order", "--except-interface",
"lo", "--domain", "localdomain", NULL
};
- virCommandPtr cmd = virCommandNewArgs(cmd, args);
+ virCommandPtr cmd1 = virCommandNewArgs(cmd, args);
+ virCommandPtr cmd2 = virCommandNewArgList("/usr/bin/dnsmasq",
+ "--domain", "localdomain", NULL);
</pre>
<h3><a name="env">Setting up the environment</a></h3>
@@ -233,23 +244,31 @@
<h3><a name="fds">Managing file handles</a></h3>
<p>
- To prevent unintended resource leaks to child processes,
- all open file handles will be closed in the child, and
- its stdin/out/err all connected to <code>/dev/null</code>.
- It is possible to allow an open file handle to be passed
- into the child:
+ To prevent unintended resource leaks to child processes, the
+ child defaults to closing all open file handles, and setting
+ stdin/out/err to <code>/dev/null</code>. It is possible to
+ allow an open file handle to be passed into the child, while
+ controlling whether that handle remains open in the parent or
+ guaranteeing that the handle will be closed in the parent after
+ either virCommandRun or virCommandFree.
</p>
<pre>
- virCommandPreserveFD(cmd, 5);
+ int sharedfd = open("cmd.log", "w+");
+ int childfd = open("conf.txt", "r");
+ virCommandPreserveFD(cmd, sharedfd);
+ virCommandTransferFD(cmd, childfd);
+ if (VIR_CLOSE(sharedfd) < 0)
+ goto cleanup;
</pre>
<p>
- With this file descriptor 5 in the current process remains
- open as file descriptor 5 in the child. For stdin/out/err
- it is usually necessary to map a file handle. To attach
- file descriptor 7 in the current process to stdin in the
- child:
+ With this, both file descriptors sharedfd and childfd in the
+ current process remain open as the same file descriptors in the
+ child. Meanwhile, after the child is spawned, sharedfd remains
+ open in the parent, while childfd is closed. For stdin/out/err
+ it is usually necessary to map a file handle. To attach file
+ descriptor 7 in the current process to stdin in the child:
</p>
<pre>
@@ -284,16 +303,30 @@
<p>
Once the command is running, <code>outfd</code>
and <code>errfd</code> will be initialized with
- valid file handles that can be read from.
+ valid file handles that can be read from. It is
+ permissible to pass the same pointer for both outfd
+ and errfd, in which case both standard streams in
+ the child will share the same fd in the parent.
</p>
+ <p>
+ Normally, file descriptors opened to collect output from a child
+ process perform blocking I/O, but the parent process can request
+ non-blocking mode:
+ </p>
+
+<pre>
+ virCommandNonblockingFDs(cmd);
+</pre>
+
<h3><a name="buffers">Feeding & capturing strings to/from the child</a></h3>
<p>
Often dealing with file handles for stdin/out/err
is unnecessarily complex. It is possible to specify
a string buffer to act as the data source for the
- child's stdin, if there are no embedded NUL bytes
+ child's stdin, if there are no embedded NUL bytes,
+ and if the command will be run with virCommandRun:
</p>
<pre>
@@ -304,7 +337,8 @@
<p>
Similarly it is possible to request that the child's
stdout/err be redirected into a string buffer, if the
- output is not expected to contain NUL bytes
+ output is not expected to contain NUL bytes, and if
+ the command will be run with virCommandRun:
</p>
<pre>
@@ -346,6 +380,31 @@
virCommandSetPreExecHook(cmd, hook, opaque);
</pre>
+ <h3><a name="logging">Logging commands</a></h3>
+
+ <p>
+ Sometimes, it is desirable to log what command will be run, or
+ even to use virCommand solely for creation of a single
+ consolidated string without running anything.
+ </p>
+
+<pre>
+ int logfd = ...;
+ char *timestamp = virTimestamp();
+ char *string = NULL;
+
+ dprintf(logfd, "%s: ", timestamp);
+ VIR_FREE(timestamp);
+ virCommandWriteArgLog(cmd, logfd);
+
+ string = virCommandToString(cmd);
+ if (string)
+ VIR_DEBUG("about to run %s", string);
+ VIR_FREE(string);
+ if (virCommandRun(cmd) < 0)
+ return -1;
+</pre>
+
<h3><a name="sync">Running commands synchronously</a></h3>
<p>
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 63795b3..d7dc7b1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -85,6 +85,7 @@ virCgroupSetSwapHardLimit;
# command.h
virCommandAddArg;
+virCommandAddArgFormat;
virCommandAddArgList;
virCommandAddArgPair;
virCommandAddArgSet;
@@ -96,7 +97,9 @@ virCommandClearCaps;
virCommandDaemonize;
virCommandFree;
virCommandNew;
+virCommandNewArgList;
virCommandNewArgs;
+virCommandNonblockingFDs;
virCommandPreserveFD;
virCommandRun;
virCommandRunAsync;
@@ -109,7 +112,10 @@ virCommandSetOutputFD;
virCommandSetPidFile;
virCommandSetPreExecHook;
virCommandSetWorkingDirectory;
+virCommandToString;
+virCommandTransferFD;
virCommandWait;
+virCommandWriteArgLog;
# conf.h
@@ -839,6 +845,7 @@ virStrToLong_ull;
virStrcpy;
virStrncpy;
virTimestamp;
+virVasprintf;
# uuid.h
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 35caccc..7c0db15 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1530,6 +1530,15 @@ int qemudExtractVersionInfo(const char *qemu,
if (retversion)
*retversion = 0;
+ /* Make sure the binary we are about to try exec'ing exists.
+ * Technically we could catch the exec() failure, but that's
+ * in a sub-process so it's hard to feed back a useful error.
+ */
+ if (access(qemu, X_OK) < 0) {
+ virReportSystemError(errno, _("Cannot find QEMU binary %s"), qemu);
+ return -1;
+ }
+
if (virExec(qemuarg, qemuenv, NULL,
&child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0)
return -1;
@@ -3942,43 +3951,35 @@ qemuBuildSmpArgStr(const virDomainDefPtr def,
* XXX 'conn' is only required to resolve network -> bridge name
* figure out how to remove this requirement some day
*/
-int qemudBuildCommandLine(virConnectPtr conn,
- struct qemud_driver *driver,
- virDomainDefPtr def,
- virDomainChrDefPtr monitor_chr,
- int monitor_json,
- unsigned long long qemuCmdFlags,
- const char ***retargv,
- const char ***retenv,
- int **vmfds,
- int *nvmfds,
- const char *migrateFrom,
- virDomainSnapshotObjPtr current_snapshot)
+virCommandPtr
+qemudBuildCommandLine(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainDefPtr def,
+ virDomainChrDefPtr monitor_chr,
+ bool monitor_json,
+ unsigned long long qemuCmdFlags,
+ const char *migrateFrom,
+ virDomainSnapshotObjPtr current_snapshot)
{
int i;
- char memory[50];
char boot[VIR_DOMAIN_BOOT_LAST+1];
struct utsname ut;
int disableKQEMU = 0;
int enableKQEMU = 0;
int disableKVM = 0;
int enableKVM = 0;
- int qargc = 0, qarga = 0;
- const char **qargv = NULL;
- int qenvc = 0, qenva = 0;
- const char **qenv = NULL;
const char *emulator;
char uuid[VIR_UUID_STRING_BUFLEN];
- char domid[50];
char *cpu;
char *smp;
int last_good_net = -1;
bool hasHwVirt = false;
+ virCommandPtr cmd;
uname_normalize(&ut);
if (qemuAssignDeviceAliases(def, qemuCmdFlags) < 0)
- return -1;
+ return NULL;
virUUIDFormat(def->uuid, uuid);
@@ -3990,7 +3991,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP)) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("TCP migration is not supported with this QEMU binary"));
- return -1;
+ return NULL;
}
} else if (STREQ(migrateFrom, "stdio")) {
if (qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC) {
@@ -3998,13 +3999,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
} else if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_KVM_STDIO)) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("STDIO migration is not supported with this QEMU binary"));
- return -1;
+ return NULL;
}
} else if (STRPREFIX(migrateFrom, "exec")) {
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC)) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("STDIO migration is not supported with this QEMU binary"));
- return -1;
+ return NULL;
}
}
}
@@ -4065,134 +4066,45 @@ int qemudBuildCommandLine(virConnectPtr conn,
break;
}
-#define ADD_ARG_SPACE \
- do { \
- if (qargc == qarga) { \
- qarga += 10; \
- if (VIR_REALLOC_N(qargv, qarga) < 0) \
- goto no_memory; \
- } \
- } while (0)
-
-#define ADD_ARG(thisarg) \
- do { \
- ADD_ARG_SPACE; \
- qargv[qargc++] = thisarg; \
- } while (0)
-
-#define ADD_ARG_LIT(thisarg) \
- do { \
- ADD_ARG_SPACE; \
- if ((qargv[qargc++] = strdup(thisarg)) == NULL) \
- goto no_memory; \
- } while (0)
-
-#define ADD_USBDISK(thisarg) \
- do { \
- ADD_ARG_LIT("-usbdevice"); \
- ADD_ARG_SPACE; \
- if ((virAsprintf((char **)&(qargv[qargc++]), \
- "disk:%s", thisarg)) == -1) { \
- goto no_memory; \
- } \
- } while (0)
-
-#define ADD_ENV_SPACE \
- do { \
- if (qenvc == qenva) { \
- qenva += 10; \
- if (VIR_REALLOC_N(qenv, qenva) < 0) \
- goto no_memory; \
- } \
- } while (0)
-
-#define ADD_ENV(thisarg) \
- do { \
- ADD_ENV_SPACE; \
- qenv[qenvc++] = thisarg; \
- } while (0)
-
-#define ADD_ENV_LIT(thisarg) \
- do { \
- ADD_ENV_SPACE; \
- if ((qenv[qenvc++] = strdup(thisarg)) == NULL) \
- goto no_memory; \
- } while (0)
-
-#define ADD_ENV_PAIR(envname, val) \
- do { \
- char *envval; \
- ADD_ENV_SPACE; \
- if (virAsprintf(&envval, "%s=%s", envname, val) < 0) \
- goto no_memory; \
- qenv[qenvc++] = envval; \
- } while (0)
-
- /* Make sure to unset or set all envvars in qemuxml2argvtest.c that
- * are copied here using this macro, otherwise the test may fail */
-#define ADD_ENV_COPY(envname) \
- do { \
- char *val = getenv(envname); \
- if (val != NULL) { \
- ADD_ENV_PAIR(envname, val); \
- } \
- } while (0)
+ cmd = virCommandNewArgList(emulator, "-S", NULL);
- /* Set '-m MB' based on maxmem, because the lower 'memory' limit
- * is set post-startup using the balloon driver. If balloon driver
- * is not supported, then they're out of luck anyway
- */
- snprintf(memory, sizeof(memory), "%lu", def->mem.max_balloon/1024);
- snprintf(domid, sizeof(domid), "%d", def->id);
-
- ADD_ENV_LIT("LC_ALL=C");
-
- ADD_ENV_COPY("LD_PRELOAD");
- ADD_ENV_COPY("LD_LIBRARY_PATH");
- ADD_ENV_COPY("PATH");
- ADD_ENV_COPY("HOME");
- ADD_ENV_COPY("USER");
- ADD_ENV_COPY("LOGNAME");
- ADD_ENV_COPY("TMPDIR");
-
- ADD_ARG_LIT(emulator);
- ADD_ARG_LIT("-S");
+ virCommandAddEnvPassCommon(cmd);
/* This should *never* be NULL, since we always provide
* a machine in the capabilities data for QEMU. So this
* check is just here as a safety in case the unexpected
* happens */
- if (def->os.machine) {
- ADD_ARG_LIT("-M");
- ADD_ARG_LIT(def->os.machine);
- }
+ if (def->os.machine)
+ virCommandAddArgList(cmd, "-M", def->os.machine, NULL);
if (qemuBuildCpuArgStr(driver, def, emulator, qemuCmdFlags,
&ut, &cpu, &hasHwVirt) < 0)
goto error;
if (cpu) {
- ADD_ARG_LIT("-cpu");
- ADD_ARG_LIT(cpu);
+ virCommandAddArgList(cmd, "-cpu", cpu, NULL);
VIR_FREE(cpu);
if ((qemuCmdFlags & QEMUD_CMD_FLAG_NESTING) &&
hasHwVirt)
- ADD_ARG_LIT("-enable-nesting");
+ virCommandAddArg(cmd, "-enable-nesting");
}
if (disableKQEMU)
- ADD_ARG_LIT("-no-kqemu");
- else if (enableKQEMU) {
- ADD_ARG_LIT("-enable-kqemu");
- ADD_ARG_LIT("-kernel-kqemu");
- }
+ virCommandAddArg(cmd, "-no-kqemu");
+ else if (enableKQEMU)
+ virCommandAddArgList(cmd, "-enable-kqemu", "-kernel-kqemu", NULL);
if (disableKVM)
- ADD_ARG_LIT("-no-kvm");
+ virCommandAddArg(cmd, "-no-kvm");
if (enableKVM)
- ADD_ARG_LIT("-enable-kvm");
- ADD_ARG_LIT("-m");
- ADD_ARG_LIT(memory);
+ virCommandAddArg(cmd, "-enable-kvm");
+
+ /* Set '-m MB' based on maxmem, because the lower 'memory' limit
+ * is set post-startup using the balloon driver. If balloon driver
+ * is not supported, then they're out of luck anyway
+ */
+ virCommandAddArg(cmd, "-m");
+ virCommandAddArgFormat(cmd, "%lu", def->mem.max_balloon / 1024);
if (def->mem.hugepage_backed) {
if (!driver->hugetlbfs_mount) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4210,43 +4122,38 @@ int qemudBuildCommandLine(virConnectPtr conn,
def->emulator);
goto error;
}
- ADD_ARG_LIT("-mem-prealloc");
- ADD_ARG_LIT("-mem-path");
- ADD_ARG_LIT(driver->hugepage_path);
+ virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path",
+ driver->hugepage_path, NULL);
}
- ADD_ARG_LIT("-smp");
+ virCommandAddArg(cmd, "-smp");
if (!(smp = qemuBuildSmpArgStr(def, qemuCmdFlags)))
goto error;
- ADD_ARG(smp);
+ virCommandAddArg(cmd, smp);
+ VIR_FREE(smp);
if (qemuCmdFlags & QEMUD_CMD_FLAG_NAME) {
- ADD_ARG_LIT("-name");
+ virCommandAddArg(cmd, "-name");
if (driver->setProcessName &&
(qemuCmdFlags & QEMUD_CMD_FLAG_NAME_PROCESS)) {
- char *name;
- if (virAsprintf(&name, "%s,process=qemu:%s",
- def->name, def->name) < 0)
- goto no_memory;
- ADD_ARG_LIT(name);
+ virCommandAddArgFormat(cmd, "%s,process=qemu:%s",
+ def->name, def->name);
} else {
- ADD_ARG_LIT(def->name);
+ virCommandAddArg(cmd, def->name);
}
}
- if (qemuCmdFlags & QEMUD_CMD_FLAG_UUID) {
- ADD_ARG_LIT("-uuid");
- ADD_ARG_LIT(uuid);
- }
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_UUID)
+ virCommandAddArgList(cmd, "-uuid", uuid, NULL);
if (def->virtType == VIR_DOMAIN_VIRT_XEN ||
STREQ(def->os.type, "xen") ||
STREQ(def->os.type, "linux")) {
if (qemuCmdFlags & QEMUD_CMD_FLAG_DOMID) {
- ADD_ARG_LIT("-domid");
- ADD_ARG_LIT(domid);
+ virCommandAddArg(cmd, "-domid");
+ virCommandAddArgFormat(cmd, "%d", def->id);
} else if (qemuCmdFlags & QEMUD_CMD_FLAG_XEN_DOMID) {
- ADD_ARG_LIT("-xen-attach");
- ADD_ARG_LIT("-xen-domid");
- ADD_ARG_LIT(domid);
+ virCommandAddArg(cmd, "-xen-attach");
+ virCommandAddArg(cmd, "-xen-domid");
+ virCommandAddArgFormat(cmd, "%d", def->id);
} else {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("qemu emulator '%s' does not support xen"),
@@ -4288,13 +4195,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
smbioscmd = qemuBuildSmbiosBiosStr(source);
if (smbioscmd != NULL) {
- ADD_ARG_LIT("-smbios");
- ADD_ARG(smbioscmd);
+ virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL);
+ VIR_FREE(smbioscmd);
}
smbioscmd = qemuBuildSmbiosSystemStr(source);
if (smbioscmd != NULL) {
- ADD_ARG_LIT("-smbios");
- ADD_ARG(smbioscmd);
+ virCommandAddArgList(cmd, "-smbios", smbioscmd, NULL);
+ VIR_FREE(smbioscmd);
}
}
}
@@ -4307,12 +4214,14 @@ int qemudBuildCommandLine(virConnectPtr conn,
* these defaults ourselves...
*/
if (!def->graphics)
- ADD_ARG_LIT("-nographic");
+ virCommandAddArg(cmd, "-nographic");
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
if (qemuCmdFlags & QEMUD_CMD_FLAG_NODEFCONFIG)
- ADD_ARG_LIT("-nodefconfig"); /* Disabling global config files */
- ADD_ARG_LIT("-nodefaults"); /* Disabling default guest devices */
+ virCommandAddArg(cmd,
+ "-nodefconfig"); /* Disable global config files */
+ virCommandAddArg(cmd,
+ "-nodefaults"); /* Disable default guest devices */
}
if (monitor_chr) {
@@ -4320,39 +4229,40 @@ int qemudBuildCommandLine(virConnectPtr conn,
/* Use -chardev if it's available */
if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) {
- ADD_ARG_LIT("-chardev");
+ virCommandAddArg(cmd, "-chardev");
if (!(chrdev = qemuBuildChrChardevStr(monitor_chr)))
goto error;
- ADD_ARG(chrdev);
+ virCommandAddArg(cmd, chrdev);
+ VIR_FREE(chrdev);
- ADD_ARG_LIT("-mon");
- if (monitor_json)
- ADD_ARG_LIT("chardev=monitor,mode=control");
- else
- ADD_ARG_LIT("chardev=monitor,mode=readline");
+ virCommandAddArg(cmd, "-mon");
+ virCommandAddArgFormat(cmd, "chardev=monitor,mode=%s",
+ monitor_json ? "control" : "readline");
} else {
const char *prefix = NULL;
if (monitor_json)
prefix = "control,";
- ADD_ARG_LIT("-monitor");
+ virCommandAddArg(cmd, "-monitor");
if (!(chrdev = qemuBuildChrArgStr(monitor_chr, prefix)))
goto error;
- ADD_ARG(chrdev);
+ virCommandAddArg(cmd, chrdev);
+ VIR_FREE(chrdev);
}
}
if (qemuCmdFlags & QEMUD_CMD_FLAG_RTC) {
const char *rtcopt;
- ADD_ARG_LIT("-rtc");
+ virCommandAddArg(cmd, "-rtc");
if (!(rtcopt = qemuBuildClockArgStr(&def->clock)))
goto error;
- ADD_ARG(rtcopt);
+ virCommandAddArg(cmd, rtcopt);
+ VIR_FREE(rtcopt);
} else {
switch (def->clock.offset) {
case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE:
- ADD_ARG_LIT("-localtime");
+ virCommandAddArg(cmd, "-localtime");
break;
case VIR_DOMAIN_CLOCK_OFFSET_UTC:
@@ -4368,7 +4278,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
}
if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE &&
def->clock.data.timezone) {
- ADD_ENV_PAIR("TZ", def->clock.data.timezone);
+ virCommandAddEnvPair(cmd, "TZ", def->clock.data.timezone);
}
for (i = 0; i < def->clock.ntimers; i++) {
@@ -4392,7 +4302,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
/* the default - do nothing */
break;
case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP:
- ADD_ARG_LIT("-rtc-td-hack");
+ virCommandAddArg(cmd, "-rtc-td-hack");
break;
case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE:
case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD:
@@ -4421,14 +4331,14 @@ int qemudBuildCommandLine(virConnectPtr conn,
/* delay is the default if we don't have kernel
(-no-kvm-pit), otherwise, the default is catchup. */
if (qemuCmdFlags & QEMUD_CMD_FLAG_NO_KVM_PIT)
- ADD_ARG_LIT("-no-kvm-pit-reinjection");
+ virCommandAddArg(cmd, "-no-kvm-pit-reinjection");
break;
case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP:
if (qemuCmdFlags & QEMUD_CMD_FLAG_NO_KVM_PIT) {
/* do nothing - this is default for kvm-pit */
} else if (qemuCmdFlags & QEMUD_CMD_FLAG_TDF) {
/* -tdf switches to 'catchup' with userspace pit. */
- ADD_ARG_LIT("-tdf");
+ virCommandAddArg(cmd, "-tdf");
} else {
/* can't catchup if we have neither pit mode */
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -4457,7 +4367,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (qemuCmdFlags & QEMUD_CMD_FLAG_NO_HPET) {
if (def->clock.timers[i]->present == 0)
- ADD_ARG_LIT("-no-hpet");
+ virCommandAddArg(cmd, "-no-hpet");
} else {
/* no hpet timer available. The only possible action
is to raise an error if present="yes" */
@@ -4472,10 +4382,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
if ((qemuCmdFlags & QEMUD_CMD_FLAG_NO_REBOOT) &&
def->onReboot != VIR_DOMAIN_LIFECYCLE_RESTART)
- ADD_ARG_LIT("-no-reboot");
+ virCommandAddArg(cmd, "-no-reboot");
if (!(def->features & (1 << VIR_DOMAIN_FEATURE_ACPI)))
- ADD_ARG_LIT("-no-acpi");
+ virCommandAddArg(cmd, "-no-acpi");
if (!def->os.bootloader) {
for (i = 0 ; i < def->os.nBootDevs ; i++) {
@@ -4499,7 +4409,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
}
if (def->os.nBootDevs) {
virBuffer boot_buf = VIR_BUFFER_INITIALIZER;
- ADD_ARG_LIT("-boot");
+ char *bootstr;
+ virCommandAddArg(cmd, "-boot");
boot[def->os.nBootDevs] = '\0';
@@ -4518,24 +4429,19 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto error;
}
- ADD_ARG(virBufferContentAndReset(&boot_buf));
+ bootstr = virBufferContentAndReset(&boot_buf);
+ virCommandAddArg(cmd, bootstr);
+ VIR_FREE(bootstr);
}
- if (def->os.kernel) {
- ADD_ARG_LIT("-kernel");
- ADD_ARG_LIT(def->os.kernel);
- }
- if (def->os.initrd) {
- ADD_ARG_LIT("-initrd");
- ADD_ARG_LIT(def->os.initrd);
- }
- if (def->os.cmdline) {
- ADD_ARG_LIT("-append");
- ADD_ARG_LIT(def->os.cmdline);
- }
+ if (def->os.kernel)
+ virCommandAddArgList(cmd, "-kernel", def->os.kernel, NULL);
+ if (def->os.initrd)
+ virCommandAddArgList(cmd, "-initrd", def->os.initrd, NULL);
+ if (def->os.cmdline)
+ virCommandAddArgList(cmd, "-append", def->os.cmdline, NULL);
} else {
- ADD_ARG_LIT("-bootloader");
- ADD_ARG_LIT(def->os.bootloader);
+ virCommandAddArgList(cmd, "-bootloader", def->os.bootloader, NULL);
}
for (i = 0 ; i < def->ndisks ; i++) {
@@ -4568,13 +4474,14 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto error;
}
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
char *devstr;
if (!(devstr = qemuBuildControllerDevStr(def->controllers[i])))
goto no_memory;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
}
}
@@ -4610,10 +4517,12 @@ int qemudBuildCommandLine(virConnectPtr conn,
if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) &&
!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
- ADD_USBDISK(disk->src);
+ virCommandAddArg(cmd, "-usbdevice");
+ virCommandAddArgFormat(cmd, "disk:%s", disk->src);
} else {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
- _("unsupported usb disk type for '%s'"), disk->src);
+ _("unsupported usb disk type for '%s'"),
+ disk->src);
goto error;
}
continue;
@@ -4634,10 +4543,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
break;
}
- ADD_ARG_LIT("-drive");
+ virCommandAddArg(cmd, "-drive");
- /* Unfortunately it is nt possible to use
- -device for floppys, or Xen paravirt
+ /* Unfortunately it is not possible to use
+ -device for floppies, or Xen paravirt
devices. Fortunately, those don't need
static PCI addresses, so we don't really
care that we can't use -device */
@@ -4648,24 +4557,23 @@ int qemudBuildCommandLine(virConnectPtr conn,
(withDeviceArg ? qemuCmdFlags :
(qemuCmdFlags & ~QEMUD_CMD_FLAG_DEVICE)))))
goto error;
- ADD_ARG(optstr);
+ virCommandAddArg(cmd, optstr);
+ VIR_FREE(optstr);
if (withDeviceArg) {
if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) {
- char *fdc;
- ADD_ARG_LIT("-global");
-
- if (virAsprintf(&fdc, "isa-fdc.drive%c=drive-%s",
- disk->info.addr.drive.unit ? 'B' : 'A',
- disk->info.alias) < 0)
- goto no_memory;
- ADD_ARG(fdc);
+ virCommandAddArg(cmd, "-global");
+ virCommandAddArgFormat(cmd, "isa-fdc.drive%c=drive-%s",
+ disk->info.addr.drive.unit
+ ? 'B' : 'A',
+ disk->info.alias);
} else {
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
if (!(optstr = qemuBuildDriveDevStr(disk)))
goto error;
- ADD_ARG(optstr);
+ virCommandAddArg(cmd, optstr);
+ VIR_FREE(optstr);
}
}
}
@@ -4677,10 +4585,12 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
- ADD_USBDISK(disk->src);
+ virCommandAddArg(cmd, "-usbdevice");
+ virCommandAddArgFormat(cmd, "disk:%s", disk->src);
} else {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
- _("unsupported usb disk type for '%s'"), disk->src);
+ _("unsupported usb disk type for '%s'"),
+ disk->src);
goto error;
}
continue;
@@ -4726,8 +4636,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
snprintf(file, PATH_MAX, "%s", disk->src);
}
- ADD_ARG_LIT(dev);
- ADD_ARG_LIT(file);
+ virCommandAddArgList(cmd, dev, file, NULL);
}
}
@@ -4736,15 +4645,17 @@ int qemudBuildCommandLine(virConnectPtr conn,
char *optstr;
virDomainFSDefPtr fs = def->fss[i];
- ADD_ARG_LIT("-fsdev");
+ virCommandAddArg(cmd, "-fsdev");
if (!(optstr = qemuBuildFSStr(fs, qemuCmdFlags)))
goto error;
- ADD_ARG(optstr);
+ virCommandAddArg(cmd, optstr);
+ VIR_FREE(optstr);
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
if (!(optstr = qemuBuildFSDevStr(fs)))
goto error;
- ADD_ARG(optstr);
+ virCommandAddArg(cmd, optstr);
+ VIR_FREE(optstr);
}
} else {
if (def->nfss) {
@@ -4756,10 +4667,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (!def->nnets) {
/* If we have -device, then we set -nodefault already */
- if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
- ADD_ARG_LIT("-net");
- ADD_ARG_LIT("none");
- }
+ if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))
+ virCommandAddArgList(cmd, "-net", "none", NULL);
} else {
for (i = 0 ; i < def->nnets ; i++) {
virDomainNetDefPtr net = def->nets[i];
@@ -4777,21 +4686,16 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK ||
net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- int tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags);
+ int tapfd = qemudNetworkIfaceConnect(conn, driver, net,
+ qemuCmdFlags);
if (tapfd < 0)
goto error;
- if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
- virDomainConfNWFilterTeardown(net);
- VIR_FORCE_CLOSE(tapfd);
- goto no_memory;
- }
-
last_good_net = i;
+ virCommandTransferFD(cmd, tapfd);
- (*vmfds)[(*nvmfds)++] = tapfd;
-
- if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name))
+ if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
+ tapfd) >= sizeof(tapfd_name))
goto no_memory;
} else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
int tapfd = qemudPhysIfaceConnect(conn, driver, net,
@@ -4800,17 +4704,11 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (tapfd < 0)
goto error;
- if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
- virDomainConfNWFilterTeardown(net);
- VIR_FORCE_CLOSE(tapfd);
- goto no_memory;
- }
-
last_good_net = i;
+ virCommandTransferFD(cmd, tapfd);
- (*vmfds)[(*nvmfds)++] = tapfd;
-
- if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name))
+ if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
+ tapfd) >= sizeof(tapfd_name))
goto no_memory;
}
@@ -4821,14 +4719,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
network device */
int vhostfd = qemudOpenVhostNet(net, qemuCmdFlags);
if (vhostfd >= 0) {
- if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
- VIR_FORCE_CLOSE(vhostfd);
- goto no_memory;
- }
+ virCommandTransferFD(cmd, vhostfd);
- (*vmfds)[(*nvmfds)++] = vhostfd;
- if (snprintf(vhostfd_name, sizeof(vhostfd_name), "%d", vhostfd)
- >= sizeof(vhostfd_name))
+ if (snprintf(vhostfd_name, sizeof(vhostfd_name), "%d",
+ vhostfd) >= sizeof(vhostfd_name))
goto no_memory;
}
}
@@ -4842,40 +4736,42 @@ int qemudBuildCommandLine(virConnectPtr conn,
*/
if ((qemuCmdFlags & QEMUD_CMD_FLAG_NETDEV) &&
(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
- ADD_ARG_LIT("-netdev");
+ virCommandAddArg(cmd, "-netdev");
if (!(host = qemuBuildHostNetStr(net, ',', vlan,
tapfd_name, vhostfd_name)))
goto error;
- ADD_ARG(host);
+ virCommandAddArg(cmd, host);
+ VIR_FREE(host);
}
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
if (!(nic = qemuBuildNicDevStr(net, vlan)))
goto error;
- ADD_ARG(nic);
+ virCommandAddArg(cmd, nic);
+ VIR_FREE(nic);
} else {
- ADD_ARG_LIT("-net");
+ virCommandAddArg(cmd, "-net");
if (!(nic = qemuBuildNicStr(net, "nic,", vlan)))
goto error;
- ADD_ARG(nic);
+ virCommandAddArg(cmd, nic);
+ VIR_FREE(nic);
}
if (!((qemuCmdFlags & QEMUD_CMD_FLAG_NETDEV) &&
(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))) {
- ADD_ARG_LIT("-net");
+ virCommandAddArg(cmd, "-net");
if (!(host = qemuBuildHostNetStr(net, ',', vlan,
tapfd_name, vhostfd_name)))
goto error;
- ADD_ARG(host);
+ virCommandAddArg(cmd, host);
+ VIR_FREE(host);
}
}
}
if (!def->nserials) {
/* If we have -device, then we set -nodefault already */
- if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
- ADD_ARG_LIT("-serial");
- ADD_ARG_LIT("none");
- }
+ if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))
+ virCommandAddArgList(cmd, "-serial", "none", NULL);
} else {
for (i = 0 ; i < def->nserials ; i++) {
virDomainChrDefPtr serial = def->serials[i];
@@ -4884,30 +4780,29 @@ int qemudBuildCommandLine(virConnectPtr conn,
/* Use -chardev with -device if they are available */
if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) &&
(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
- ADD_ARG_LIT("-chardev");
+ virCommandAddArg(cmd, "-chardev");
if (!(devstr = qemuBuildChrChardevStr(serial)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
- ADD_ARG_LIT("-device");
- if (virAsprintf(&devstr, "isa-serial,chardev=%s", serial->info.alias) < 0)
- goto no_memory;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, "-device");
+ virCommandAddArgFormat(cmd, "isa-serial,chardev=%s",
+ serial->info.alias);
} else {
- ADD_ARG_LIT("-serial");
+ virCommandAddArg(cmd, "-serial");
if (!(devstr = qemuBuildChrArgStr(serial, NULL)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
}
}
}
if (!def->nparallels) {
/* If we have -device, then we set -nodefault already */
- if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
- ADD_ARG_LIT("-parallel");
- ADD_ARG_LIT("none");
- }
+ if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE))
+ virCommandAddArgList(cmd, "-parallel", "none", NULL);
} else {
for (i = 0 ; i < def->nparallels ; i++) {
virDomainChrDefPtr parallel = def->parallels[i];
@@ -4916,20 +4811,21 @@ int qemudBuildCommandLine(virConnectPtr conn,
/* Use -chardev with -device if they are available */
if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) &&
(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
- ADD_ARG_LIT("-chardev");
+ virCommandAddArg(cmd, "-chardev");
if (!(devstr = qemuBuildChrChardevStr(parallel)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
- ADD_ARG_LIT("-device");
- if (virAsprintf(&devstr, "isa-parallel,chardev=%s", parallel->info.alias) < 0)
- goto no_memory;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, "-device");
+ virCommandAddArgFormat(cmd, "isa-parallel,chardev=%s",
+ parallel->info.alias);
} else {
- ADD_ARG_LIT("-parallel");
+ virCommandAddArg(cmd, "-parallel");
if (!(devstr = qemuBuildChrArgStr(parallel, NULL)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
}
}
}
@@ -4947,24 +4843,23 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto error;
}
- ADD_ARG_LIT("-chardev");
+ virCommandAddArg(cmd, "-chardev");
if (!(devstr = qemuBuildChrChardevStr(channel)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
char *addr = virSocketFormatAddr(channel->target.addr);
if (!addr)
goto error;
int port = virSocketGetPort(channel->target.addr);
- ADD_ARG_LIT("-netdev");
- if (virAsprintf(&devstr, "user,guestfwd=tcp:%s:%i,chardev=%s,id=user-%s",
- addr, port, channel->info.alias, channel->info.alias) < 0) {
- VIR_FREE(addr);
- goto no_memory;
- }
+ virCommandAddArg(cmd, "-netdev");
+ virCommandAddArgFormat(cmd,
+ "user,guestfwd=tcp:%s:%i,chardev=%s,id=user-%s",
+ addr, port, channel->info.alias,
+ channel->info.alias);
VIR_FREE(addr);
- ADD_ARG(devstr);
break;
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
@@ -4974,15 +4869,17 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto error;
}
- ADD_ARG_LIT("-chardev");
+ virCommandAddArg(cmd, "-chardev");
if (!(devstr = qemuBuildChrChardevStr(channel)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
if (!(devstr = qemuBuildVirtioSerialPortDevStr(channel)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
break;
}
}
@@ -5000,15 +4897,17 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto error;
}
- ADD_ARG_LIT("-chardev");
+ virCommandAddArg(cmd, "-chardev");
if (!(devstr = qemuBuildChrChardevStr(console)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
if (!(devstr = qemuBuildVirtioSerialPortDevStr(console)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
break;
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL:
@@ -5022,20 +4921,22 @@ int qemudBuildCommandLine(virConnectPtr conn,
}
}
- ADD_ARG_LIT("-usb");
+ virCommandAddArg(cmd, "-usb");
for (i = 0 ; i < def->ninputs ; i++) {
virDomainInputDefPtr input = def->inputs[i];
if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) {
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
char *optstr;
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
if (!(optstr = qemuBuildUSBInputDevStr(input)))
goto error;
- ADD_ARG(optstr);
+ virCommandAddArg(cmd, optstr);
+ VIR_FREE(optstr);
} else {
- ADD_ARG_LIT("-usbdevice");
- ADD_ARG_LIT(input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet");
+ virCommandAddArgList(cmd, "-usbdevice",
+ input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE
+ ? "mouse" : "tablet", NULL);
}
}
}
@@ -5079,7 +4980,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
virBufferAddLit(&opt, ",sasl");
if (driver->vncSASLdir)
- ADD_ENV_PAIR("SASL_CONF_DIR", driver->vncSASLdir);
+ virCommandAddEnvPair(cmd, "SASL_CONF_DIR",
+ driver->vncSASLdir);
/* TODO: Support ACLs later */
}
@@ -5094,11 +4996,11 @@ int qemudBuildCommandLine(virConnectPtr conn,
optstr = virBufferContentAndReset(&opt);
- ADD_ARG_LIT("-vnc");
- ADD_ARG(optstr);
+ virCommandAddArgList(cmd, "-vnc", optstr, NULL);
+ VIR_FREE(optstr);
if (def->graphics[0]->data.vnc.keymap) {
- ADD_ARG_LIT("-k");
- ADD_ARG_LIT(def->graphics[0]->data.vnc.keymap);
+ virCommandAddArgList(cmd, "-k", def->graphics[0]->data.vnc.keymap,
+ NULL);
}
/* Unless user requested it, set the audio backend to none, to
@@ -5106,45 +5008,33 @@ int qemudBuildCommandLine(virConnectPtr conn,
* security issues and might not work when using VNC.
*/
if (driver->vncAllowHostAudio) {
- ADD_ENV_COPY("QEMU_AUDIO_DRV");
+ virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
} else {
- ADD_ENV_LIT("QEMU_AUDIO_DRV=none");
+ virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none");
}
} else if ((def->ngraphics == 1) &&
def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
- char *xauth = NULL;
- char *display = NULL;
-
- if (def->graphics[0]->data.sdl.xauth &&
- virAsprintf(&xauth, "XAUTHORITY=%s",
- def->graphics[0]->data.sdl.xauth) < 0)
- goto no_memory;
- if (def->graphics[0]->data.sdl.display &&
- virAsprintf(&display, "DISPLAY=%s",
- def->graphics[0]->data.sdl.display) < 0) {
- VIR_FREE(xauth);
- goto no_memory;
- }
-
- if (xauth)
- ADD_ENV(xauth);
- if (display)
- ADD_ENV(display);
+ if (def->graphics[0]->data.sdl.xauth)
+ virCommandAddEnvPair(cmd, "XAUTHORITY",
+ def->graphics[0]->data.sdl.xauth);
+ if (def->graphics[0]->data.sdl.display)
+ virCommandAddEnvPair(cmd, "DISPLAY",
+ def->graphics[0]->data.sdl.display);
if (def->graphics[0]->data.sdl.fullscreen)
- ADD_ARG_LIT("-full-screen");
+ virCommandAddArg(cmd, "-full-screen");
/* If using SDL for video, then we should just let it
* use QEMU's host audio drivers, possibly SDL too
* User can set these two before starting libvirtd
*/
- ADD_ENV_COPY("QEMU_AUDIO_DRV");
- ADD_ENV_COPY("SDL_AUDIODRIVER");
+ virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV");
+ virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER");
/* New QEMU has this flag to let us explicitly ask for
* SDL graphics. This is better than relying on the
* default, since the default changes :-( */
if (qemuCmdFlags & QEMUD_CMD_FLAG_SDL)
- ADD_ARG_LIT("-sdl");
+ virCommandAddArg(cmd, "-sdl");
} else if ((def->ngraphics == 1) &&
def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
@@ -5197,16 +5087,15 @@ int qemudBuildCommandLine(virConnectPtr conn,
optstr = virBufferContentAndReset(&opt);
- ADD_ARG_LIT("-spice");
- ADD_ARG(optstr);
- if (def->graphics[0]->data.spice.keymap) {
- ADD_ARG_LIT("-k");
- ADD_ARG_LIT(def->graphics[0]->data.spice.keymap);
- }
+ virCommandAddArgList(cmd, "-spice", optstr, NULL);
+ VIR_FREE(optstr);
+ if (def->graphics[0]->data.spice.keymap)
+ virCommandAddArgList(cmd, "-k",
+ def->graphics[0]->data.spice.keymap, NULL);
/* SPICE includes native support for tunnelling audio, so we
* set the audio backend to point at SPICE's own driver
*/
- ADD_ENV_LIT("QEMU_AUDIO_DRV=spice");
+ virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice");
} else if ((def->ngraphics == 1)) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
@@ -5235,18 +5124,17 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto error;
}
- ADD_ARG_LIT("-vga");
- ADD_ARG_LIT(vgastr);
+ virCommandAddArgList(cmd, "-vga", vgastr, NULL);
}
} else {
switch (def->videos[0]->type) {
case VIR_DOMAIN_VIDEO_TYPE_VGA:
- ADD_ARG_LIT("-std-vga");
+ virCommandAddArg(cmd, "-std-vga");
break;
case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
- ADD_ARG_LIT("-vmwarevga");
+ virCommandAddArg(cmd, "-vmwarevga");
break;
case VIR_DOMAIN_VIDEO_TYPE_XEN:
@@ -5273,12 +5161,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto error;
}
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
if (!(str = qemuBuildVideoDevStr(def->videos[i])))
goto error;
- ADD_ARG(str);
+ virCommandAddArg(cmd, str);
+ VIR_FREE(str);
}
} else {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -5290,10 +5179,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
} else {
/* If we have -device, then we set -nodefault already */
if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) &&
- (qemuCmdFlags & QEMUD_CMD_FLAG_VGA)) {
- ADD_ARG_LIT("-vga");
- ADD_ARG_LIT("none");
- }
+ (qemuCmdFlags & QEMUD_CMD_FLAG_VGA))
+ virCommandAddArgList(cmd, "-vga", "none", NULL);
}
/* Add sound hardware */
@@ -5307,15 +5194,15 @@ int qemudBuildCommandLine(virConnectPtr conn,
* we don't need to set any PCI address on it, so we don't
* mind too much */
if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) {
- ADD_ARG_LIT("-soundhw");
- ADD_ARG_LIT("pcspk");
+ virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL);
} else {
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
if (!(str = qemuBuildSoundDevStr(sound)))
goto error;
- ADD_ARG(str);
+ virCommandAddArg(cmd, str);
+ VIR_FREE(str);
}
}
} else {
@@ -5338,8 +5225,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (i < (def->nsounds - 1))
strncat(modstr, ",", size--);
}
- ADD_ARG_LIT("-soundhw");
- ADD_ARG(modstr);
+ virCommandAddArgList(cmd, "-soundhw", modstr, NULL);
+ VIR_FREE(modstr);
}
}
@@ -5349,13 +5236,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
char *optstr;
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
optstr = qemuBuildWatchdogDevStr(watchdog);
if (!optstr)
goto error;
} else {
- ADD_ARG_LIT("-watchdog");
+ virCommandAddArg(cmd, "-watchdog");
const char *model = virDomainWatchdogModelTypeToString(watchdog->model);
if (!model) {
@@ -5367,7 +5254,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
if (!(optstr = strdup(model)))
goto no_memory;
}
- ADD_ARG(optstr);
+ virCommandAddArg(cmd, optstr);
+ VIR_FREE(optstr);
const char *action = virDomainWatchdogActionTypeToString(watchdog->action);
if (!action) {
@@ -5375,8 +5263,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
"%s", _("invalid watchdog action"));
goto error;
}
- ADD_ARG_LIT("-watchdog-action");
- ADD_ARG_LIT(action);
+ virCommandAddArgList(cmd, "-watchdog-action", action, NULL);
}
/* Add host passthrough hardware */
@@ -5389,15 +5276,17 @@ int qemudBuildCommandLine(virConnectPtr conn,
hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
} else {
- ADD_ARG_LIT("-usbdevice");
+ virCommandAddArg(cmd, "-usbdevice");
if (!(devstr = qemuBuildUSBHostdevUsbDevStr(hostdev)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
}
}
@@ -5416,26 +5305,22 @@ int qemudBuildCommandLine(virConnectPtr conn,
goto no_memory;
}
- if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) {
- VIR_FREE(configfd_name);
- VIR_FORCE_CLOSE(configfd);
- goto no_memory;
- }
-
- (*vmfds)[(*nvmfds)++] = configfd;
+ virCommandTransferFD(cmd, configfd);
}
}
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name);
VIR_FREE(configfd_name);
if (!devstr)
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
} else if (qemuCmdFlags & QEMUD_CMD_FLAG_PCIDEVICE) {
- ADD_ARG_LIT("-pcidevice");
+ virCommandAddArg(cmd, "-pcidevice");
if (!(devstr = qemuBuildPCIHostdevPCIDevStr(hostdev)))
goto error;
- ADD_ARG(devstr);
+ virCommandAddArg(cmd, devstr);
+ VIR_FREE(devstr);
} else {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("PCI device assignment is not supported by this version of qemu"));
@@ -5444,10 +5329,8 @@ int qemudBuildCommandLine(virConnectPtr conn,
}
}
- if (migrateFrom) {
- ADD_ARG_LIT("-incoming");
- ADD_ARG_LIT(migrateFrom);
- }
+ if (migrateFrom)
+ virCommandAddArgList(cmd, "-incoming", migrateFrom, NULL);
/* QEMU changed its default behavior to not include the virtio balloon
* device. Explicitly request it to ensure it will be present.
@@ -5465,76 +5348,43 @@ int qemudBuildCommandLine(virConnectPtr conn,
}
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
char *optstr;
- ADD_ARG_LIT("-device");
+ virCommandAddArg(cmd, "-device");
optstr = qemuBuildMemballoonDevStr(def->memballoon);
if (!optstr)
goto error;
- ADD_ARG(optstr);
+ virCommandAddArg(cmd, optstr);
+ VIR_FREE(optstr);
} else if (qemuCmdFlags & QEMUD_CMD_FLAG_BALLOON) {
- ADD_ARG_LIT("-balloon");
- ADD_ARG_LIT("virtio");
+ virCommandAddArgList(cmd, "-balloon", "virtio", NULL);
}
}
- if (current_snapshot && current_snapshot->def->active) {
- ADD_ARG_LIT("-loadvm");
- ADD_ARG_LIT(current_snapshot->def->name);
- }
+ if (current_snapshot && current_snapshot->def->active)
+ virCommandAddArgList(cmd, "-loadvm", current_snapshot->def->name,
+ NULL);
if (def->namespaceData) {
- qemuDomainCmdlineDefPtr cmd;
-
- cmd = def->namespaceData;
- for (i = 0; i < cmd->num_args; i++)
- ADD_ARG_LIT(cmd->args[i]);
- for (i = 0; i < cmd->num_env; i++) {
- if (cmd->env_value[i])
- ADD_ENV_PAIR(cmd->env_name[i], cmd->env_value[i]);
- else
- ADD_ENV_PAIR(cmd->env_name[i], "");
- }
- }
+ qemuDomainCmdlineDefPtr qemucmd;
- ADD_ARG(NULL);
- ADD_ENV(NULL);
+ qemucmd = def->namespaceData;
+ for (i = 0; i < qemucmd->num_args; i++)
+ virCommandAddArg(cmd, qemucmd->args[i]);
+ for (i = 0; i < qemucmd->num_env; i++)
+ virCommandAddEnvPair(cmd, qemucmd->env_name[i],
+ qemucmd->env_value[i]
+ ? qemucmd->env_value[i] : "");
+ }
- *retargv = qargv;
- *retenv = qenv;
- return 0;
+ return cmd;
no_memory:
virReportOOMError();
error:
for (i = 0; i <= last_good_net; i++)
virDomainConfNWFilterTeardown(def->nets[i]);
- if (vmfds &&
- *vmfds) {
- for (i = 0; i < *nvmfds; i++)
- VIR_FORCE_CLOSE((*vmfds)[i]);
- VIR_FREE(*vmfds);
- *nvmfds = 0;
- }
- if (qargv) {
- for (i = 0 ; i < qargc ; i++)
- VIR_FREE((qargv)[i]);
- VIR_FREE(qargv);
- }
- if (qenv) {
- for (i = 0 ; i < qenvc ; i++)
- VIR_FREE((qenv)[i]);
- VIR_FREE(qenv);
- }
- return -1;
-
-#undef ADD_ARG
-#undef ADD_ARG_LIT
-#undef ADD_ARG_SPACE
-#undef ADD_USBDISK
-#undef ADD_ENV
-#undef ADD_ENV_COPY
-#undef ADD_ENV_LIT
-#undef ADD_ENV_SPACE
+ virCommandFree(cmd);
+ return NULL;
}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 790ce98..24df871 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -25,6 +25,7 @@
# define __QEMUD_CONF_H
# include <config.h>
+# include <stdbool.h>
# include "ebtables.h"
# include "internal.h"
@@ -40,6 +41,7 @@
# include "cpu_conf.h"
# include "driver.h"
# include "bitmap.h"
+# include "command.h"
# define qemudDebug(fmt, ...) do {} while(0)
@@ -227,16 +229,12 @@ int qemudParseHelpStr (const char *qemu,
unsigned int *is_kvm,
unsigned int *kvm_version);
-int qemudBuildCommandLine (virConnectPtr conn,
+virCommandPtr qemudBuildCommandLine (virConnectPtr conn,
struct qemud_driver *driver,
virDomainDefPtr def,
virDomainChrDefPtr monitor_chr,
- int monitor_json,
+ bool monitor_json,
unsigned long long qemuCmdFlags,
- const char ***retargv,
- const char ***retenv,
- int **vmfds,
- int *nvmfds,
const char *migrateFrom,
virDomainSnapshotObjPtr current_snapshot)
ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f00d8a3..38d7bd2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3018,14 +3018,6 @@ qemuAssignPCIAddresses(virDomainDefPtr def)
int ret = -1;
unsigned long long qemuCmdFlags = 0;
qemuDomainPCIAddressSetPtr addrs = NULL;
- struct stat sb;
-
- if (stat(def->emulator, &sb) < 0) {
- virReportSystemError(errno,
- _("Cannot find QEMU binary %s"),
- def->emulator);
- goto cleanup;
- }
if (qemudExtractVersionInfo(def->emulator,
NULL,
@@ -3865,30 +3857,21 @@ static int qemudStartVMDaemon(virConnectPtr conn,
bool start_paused,
int stdin_fd,
const char *stdin_path) {
- const char **argv = NULL, **tmp;
- const char **progenv = NULL;
- int i, ret, runflags;
- struct stat sb;
- int *vmfds = NULL;
- int nvmfds = 0;
+ int ret;
unsigned long long qemuCmdFlags;
- fd_set keepfd;
- const char *emulator;
- pid_t child;
int pos = -1;
char ebuf[1024];
char *pidfile = NULL;
int logfile = -1;
char *timestamp;
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virCommandPtr cmd = NULL;
struct qemudHookData hookData;
hookData.conn = conn;
hookData.vm = vm;
hookData.driver = driver;
- FD_ZERO(&keepfd);
-
DEBUG0("Beginning VM startup process");
if (virDomainObjIsActive(vm)) {
@@ -3979,21 +3962,8 @@ static int qemudStartVMDaemon(virConnectPtr conn,
if ((logfile = qemudLogFD(driver, vm->def->name, false)) < 0)
goto cleanup;
- emulator = vm->def->emulator;
-
- /* Make sure the binary we are about to try exec'ing exists.
- * Technically we could catch the exec() failure, but that's
- * in a sub-process so its hard to feed back a useful error
- */
- if (stat(emulator, &sb) < 0) {
- virReportSystemError(errno,
- _("Cannot find QEMU binary %s"),
- emulator);
- goto cleanup;
- }
-
- DEBUG0("Determing emulator version");
- if (qemudExtractVersionInfo(emulator,
+ DEBUG0("Determining emulator version");
+ if (qemudExtractVersionInfo(vm->def->emulator,
NULL,
&qemuCmdFlags) < 0)
goto cleanup;
@@ -4062,10 +4032,10 @@ static int qemudStartVMDaemon(virConnectPtr conn,
DEBUG0("Building emulator command line");
vm->def->id = driver->nextvmid++;
- if (qemudBuildCommandLine(conn, driver, vm->def, priv->monConfig,
- priv->monJSON, qemuCmdFlags, &argv, &progenv,
- &vmfds, &nvmfds, migrateFrom,
- vm->current_snapshot) < 0)
+ if (!(cmd = qemudBuildCommandLine(conn, driver, vm->def, priv->monConfig,
+ priv->monJSON != 0, qemuCmdFlags,
+ migrateFrom,
+ vm->current_snapshot)))
goto cleanup;
if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir) < 0)
@@ -4100,50 +4070,28 @@ static int qemudStartVMDaemon(virConnectPtr conn,
VIR_FREE(timestamp);
}
- tmp = progenv;
-
- while (*tmp) {
- if (safewrite(logfile, *tmp, strlen(*tmp)) < 0)
- VIR_WARN("Unable to write envv to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
- if (safewrite(logfile, " ", 1) < 0)
- VIR_WARN("Unable to write envv to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
- tmp++;
- }
- tmp = argv;
- while (*tmp) {
- if (safewrite(logfile, *tmp, strlen(*tmp)) < 0)
- VIR_WARN("Unable to write argv to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
- if (safewrite(logfile, " ", 1) < 0)
- VIR_WARN("Unable to write argv to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
- tmp++;
- }
- if (safewrite(logfile, "\n", 1) < 0)
- VIR_WARN("Unable to write argv to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
+ virCommandWriteArgLog(cmd, logfile);
if ((pos = lseek(logfile, 0, SEEK_END)) < 0)
VIR_WARN("Unable to seek to end of logfile: %s",
virStrerror(errno, ebuf, sizeof ebuf));
- for (i = 0 ; i < nvmfds ; i++)
- FD_SET(vmfds[i], &keepfd);
-
VIR_DEBUG("Clear emulator capabilities: %d",
driver->clearEmulatorCapabilities);
- runflags = VIR_EXEC_NONBLOCK;
- if (driver->clearEmulatorCapabilities) {
- runflags |= VIR_EXEC_CLEAR_CAPS;
- }
-
- ret = virExecDaemonize(argv, progenv, &keepfd, &child,
- stdin_fd, &logfile, &logfile,
- runflags,
- qemudSecurityHook, &hookData,
- pidfile);
+ if (driver->clearEmulatorCapabilities)
+ virCommandClearCaps(cmd);
+
+ VIR_WARN("Executing %s", vm->def->emulator);
+ virCommandSetPreExecHook(cmd, qemudSecurityHook, &hookData);
+ virCommandSetInputFD(cmd, stdin_fd);
+ virCommandSetOutputFD(cmd, &logfile);
+ virCommandSetErrorFD(cmd, &logfile);
+ virCommandNonblockingFDs(cmd);
+ virCommandSetPidFile(cmd, pidfile);
+ virCommandDaemonize(cmd);
+
+ ret = virCommandRun(cmd, NULL);
+ VIR_WARN("Executing done %s", vm->def->emulator);
VIR_FREE(pidfile);
/* wait for qemu process to to show up */
@@ -4153,7 +4101,13 @@ static int qemudStartVMDaemon(virConnectPtr conn,
_("Domain %s didn't show up\n"), vm->def->name);
ret = -1;
}
+#if 0
} else if (ret == -2) {
+ /*
+ * XXX this is bogus. It isn't safe to set vm->pid = child
+ * because the child no longer exists.
+ */
+
/* The virExec process that launches the daemon failed. Pending on
* when it failed (we can't determine for sure), there may be
* extra info in the domain log (if the hook failed for example).
@@ -4163,30 +4117,16 @@ static int qemudStartVMDaemon(virConnectPtr conn,
*/
vm->pid = child;
ret = 0;
+#endif
}
if (migrateFrom)
start_paused = true;
vm->state = start_paused ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
- for (i = 0 ; argv[i] ; i++)
- VIR_FREE(argv[i]);
- VIR_FREE(argv);
-
- for (i = 0 ; progenv[i] ; i++)
- VIR_FREE(progenv[i]);
- VIR_FREE(progenv);
-
if (ret == -1) /* The VM failed to start; tear filters before taps */
virDomainConfVMNWFilterTeardown(vm);
- if (vmfds) {
- for (i = 0 ; i < nvmfds ; i++) {
- VIR_FORCE_CLOSE(vmfds[i]);
- }
- VIR_FREE(vmfds);
- }
-
if (ret == -1) /* The VM failed to start */
goto cleanup;
@@ -4240,6 +4180,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
if (virDomainObjSetDefTransient(driver->caps, vm) < 0)
goto cleanup;
+ virCommandFree(cmd);
VIR_FORCE_CLOSE(logfile);
return 0;
@@ -4248,9 +4189,9 @@ cleanup:
/* We jump here if we failed to start the VM for any reason, or
* if we failed to initialize the now running VM. kill it off and
* pretend we never started it */
- qemudShutdownVMDaemon(driver, vm, 0);
-
+ virCommandFree(cmd);
VIR_FORCE_CLOSE(logfile);
+ qemudShutdownVMDaemon(driver, vm, 0);
return -1;
}
@@ -7312,13 +7253,8 @@ static char *qemuDomainXMLToNative(virConnectPtr conn,
struct qemud_driver *driver = conn->privateData;
virDomainDefPtr def = NULL;
virDomainChrDef monConfig;
- const char *emulator;
unsigned long long qemuCmdFlags;
- struct stat sb;
- const char **retargv = NULL;
- const char **retenv = NULL;
- const char **tmp;
- virBuffer buf = VIR_BUFFER_INITIALIZER;
+ virCommandPtr cmd = NULL;
char *ret = NULL;
int i;
@@ -7368,70 +7304,26 @@ static char *qemuDomainXMLToNative(virConnectPtr conn,
def->graphics[i]->data.vnc.autoport)
def->graphics[i]->data.vnc.port = 5900;
}
- emulator = def->emulator;
-
- /* Make sure the binary we are about to try exec'ing exists.
- * Technically we could catch the exec() failure, but that's
- * in a sub-process so its hard to feed back a useful error
- */
- if (stat(emulator, &sb) < 0) {
- virReportSystemError(errno,
- _("Cannot find QEMU binary %s"),
- emulator);
- goto cleanup;
- }
- if (qemudExtractVersionInfo(emulator,
+ if (qemudExtractVersionInfo(def->emulator,
NULL,
- &qemuCmdFlags) < 0) {
- qemuReportError(VIR_ERR_INTERNAL_ERROR,
- _("Cannot determine QEMU argv syntax %s"),
- emulator);
+ &qemuCmdFlags) < 0)
goto cleanup;
- }
if (qemuPrepareMonitorChr(driver, &monConfig, def->name) < 0)
goto cleanup;
- if (qemudBuildCommandLine(conn, driver, def,
- &monConfig, 0, qemuCmdFlags,
- &retargv, &retenv,
- NULL, NULL, /* Don't want it to create TAP devices */
- NULL, NULL) < 0) {
- goto cleanup;
- }
-
- tmp = retenv;
- while (*tmp) {
- virBufferAdd(&buf, *tmp, strlen(*tmp));
- virBufferAddLit(&buf, " ");
- tmp++;
- }
- tmp = retargv;
- while (*tmp) {
- virBufferAdd(&buf, *tmp, strlen(*tmp));
- virBufferAddLit(&buf, " ");
- tmp++;
- }
-
- if (virBufferError(&buf)) {
- virBufferFreeAndReset(&buf);
- virReportOOMError();
+ if (!(cmd = qemudBuildCommandLine(conn, driver, def,
+ &monConfig, false, qemuCmdFlags,
+ NULL, NULL)))
goto cleanup;
- }
- ret = virBufferContentAndReset(&buf);
+ ret = virCommandToString(cmd);
cleanup:
qemuDriverUnlock(driver);
- for (tmp = retargv ; tmp && *tmp ; tmp++)
- VIR_FREE(*tmp);
- VIR_FREE(retargv);
-
- for (tmp = retenv ; tmp && *tmp ; tmp++)
- VIR_FREE(*tmp);
- VIR_FREE(retenv);
+ virCommandFree(cmd);
virDomainDefFree(def);
return ret;
}
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index 947a6d8..55b3ef6 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -308,7 +308,7 @@ error:
static char *
umlBuildCommandLineChr(virDomainChrDefPtr def,
const char *dev,
- fd_set *keepfd)
+ virCommandPtr cmd)
{
char *ret = NULL;
@@ -372,7 +372,7 @@ umlBuildCommandLineChr(virDomainChrDefPtr def,
VIR_FORCE_CLOSE(fd_out);
return NULL;
}
- FD_SET(fd_out, keepfd);
+ virCommandTransferFD(cmd, fd_out);
}
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
@@ -425,20 +425,17 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn,
virDomainObjPtr vm)
{
int i, j;
- char memory[50];
struct utsname ut;
virCommandPtr cmd;
uname(&ut);
- snprintf(memory, sizeof(memory), "%luK", vm->def->mem.cur_balloon);
-
cmd = virCommandNew(vm->def->os.kernel);
virCommandAddEnvPassCommon(cmd);
//virCommandAddArgPair(cmd, "con0", "fd:0,fd:1");
- virCommandAddArgPair(cmd, "mem", memory);
+ virCommandAddArgFormat(cmd, "mem=%luK", vm->def->mem.cur_balloon);
virCommandAddArgPair(cmd, "umid", vm->def->name);
virCommandAddArgPair(cmd, "uml_dir", driver->monitorDir);
@@ -468,7 +465,7 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn,
for (i = 0 ; i < UML_MAX_CHAR_DEVICE ; i++) {
char *ret = NULL;
if (i == 0 && vm->def->console)
- ret = umlBuildCommandLineChr(vm->def->console, "con", keepfd);
+ ret = umlBuildCommandLineChr(vm->def->console, "con", cmd);
if (!ret)
if (virAsprintf(&ret, "con%d=none", i) < 0)
goto no_memory;
@@ -483,7 +480,7 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn,
if (vm->def->serials[j]->target.port == i)
chr = vm->def->serials[j];
if (chr)
- ret = umlBuildCommandLineChr(chr, "ssl", keepfd);
+ ret = umlBuildCommandLineChr(chr, "ssl", cmd);
if (!ret)
if (virAsprintf(&ret, "ssl%d=none", i) < 0)
goto no_memory;
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 6c28c76..546e960 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -812,18 +812,11 @@ static int umlCleanupTapDevices(virConnectPtr conn ATTRIBUTE_UNUSED,
static int umlStartVMDaemon(virConnectPtr conn,
struct uml_driver *driver,
virDomainObjPtr vm) {
- const char **argv = NULL, **tmp;
- const char **progenv = NULL;
- int i, ret;
- pid_t pid;
+ int ret;
char *logfile;
int logfd = -1;
- struct stat sb;
- fd_set keepfd;
- char ebuf[1024];
umlDomainObjPrivatePtr priv = vm->privateData;
-
- FD_ZERO(&keepfd);
+ virCommandPtr cmd = NULL;
if (virDomainObjIsActive(vm)) {
umlReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -840,7 +833,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
* Technically we could catch the exec() failure, but that's
* in a sub-process so its hard to feed back a useful error
*/
- if (stat(vm->def->os.kernel, &sb) < 0) {
+ if (access(vm->def->os.kernel, X_OK) < 0) {
virReportSystemError(errno,
_("Cannot find UML kernel %s"),
vm->def->os.kernel);
@@ -877,67 +870,30 @@ static int umlStartVMDaemon(virConnectPtr conn,
return -1;
}
- if (umlBuildCommandLine(conn, driver, vm, &keepfd,
- &argv, &progenv) < 0) {
+ if (!(cmd = umlBuildCommandLine(conn, driver, vm))) {
VIR_FORCE_CLOSE(logfd);
virDomainConfVMNWFilterTeardown(vm);
umlCleanupTapDevices(conn, vm);
return -1;
}
- tmp = progenv;
- while (*tmp) {
- if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
- VIR_WARN("Unable to write envv to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
- if (safewrite(logfd, " ", 1) < 0)
- VIR_WARN("Unable to write envv to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
- tmp++;
- }
- tmp = argv;
- while (*tmp) {
- if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
- VIR_WARN("Unable to write argv to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
- if (safewrite(logfd, " ", 1) < 0)
- VIR_WARN("Unable to write argv to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
- tmp++;
- }
- if (safewrite(logfd, "\n", 1) < 0)
- VIR_WARN("Unable to write argv to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
+ virCommandWriteArgLog(cmd, logfd);
priv->monitor = -1;
- ret = virExecDaemonize(argv, progenv, &keepfd, &pid,
- -1, &logfd, &logfd,
- VIR_EXEC_CLEAR_CAPS,
- NULL, NULL, NULL);
+ virCommandClearCaps(cmd);
+ virCommandSetOutputFD(cmd, &logfd);
+ virCommandSetErrorFD(cmd, &logfd);
+ virCommandDaemonize(cmd);
+
+ ret = virCommandRun(cmd, NULL);
VIR_FORCE_CLOSE(logfd);
if (ret < 0)
goto cleanup;
ret = virDomainObjSetDefTransient(driver->caps, vm);
cleanup:
- /*
- * At the moment, the only thing that populates keepfd is
- * umlBuildCommandLineChr. We want to close every fd it opens.
- */
- for (i = 0; i < FD_SETSIZE; i++)
- if (FD_ISSET(i, &keepfd)) {
- int tmpfd = i;
- VIR_FORCE_CLOSE(tmpfd);
- }
-
- for (i = 0 ; argv[i] ; i++)
- VIR_FREE(argv[i]);
- VIR_FREE(argv);
-
- for (i = 0 ; progenv[i] ; i++)
- VIR_FREE(progenv[i]);
- VIR_FREE(progenv);
+ virCommandFree(cmd);
if (ret < 0) {
virDomainConfVMNWFilterTeardown(vm);
diff --git a/src/util/command.c b/src/util/command.c
index 71db2a1..948a957 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -33,6 +33,7 @@
#include "util.h"
#include "logging.h"
#include "files.h"
+#include "buf.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -54,7 +55,9 @@ struct _virCommand {
char *pwd;
/* XXX Use int[] if we ever need to support more than FD_SETSIZE fd's. */
- fd_set preserve;
+ fd_set preserve; /* FDs to pass to child. */
+ fd_set transfer; /* FDs to close in parent. */
+
unsigned int flags;
char *inbuf;
@@ -99,6 +102,7 @@ virCommandNewArgs(const char *const*args)
return NULL;
FD_ZERO(&cmd->preserve);
+ FD_ZERO(&cmd->transfer);
cmd->infd = cmd->outfd = cmd->errfd = -1;
cmd->inpipe = -1;
cmd->pid = -1;
@@ -113,27 +117,76 @@ virCommandNewArgs(const char *const*args)
return cmd;
}
+/*
+ * Create a new command with a NULL terminated
+ * list of args, starting with the binary to run
+ */
+virCommandPtr
+virCommandNewArgList(const char *binary, ...)
+{
+ virCommandPtr cmd = virCommandNew(binary);
+ va_list list;
+ const char *arg;
+
+ if (!cmd || cmd->has_error)
+ return NULL;
+
+ va_start(list, binary);
+ while ((arg = va_arg(list, const char *)) != NULL)
+ virCommandAddArg(cmd, arg);
+ va_end(list);
+ return cmd;
+}
+
/*
* Preserve the specified file descriptor in the child, instead of
- * closing it. FD must not be one of the three standard streams.
+ * closing it. FD must not be one of the three standard streams. If
+ * transfer is true, then fd will be closed in the parent after a call
+ * to Run/RunAsync/Free, otherwise caller is still responsible for fd.
*/
-void
-virCommandPreserveFD(virCommandPtr cmd, int fd)
+static void
+virCommandKeepFD(virCommandPtr cmd, int fd, bool transfer)
{
- if (!cmd || cmd->has_error)
+ if (!cmd)
return;
if (fd <= STDERR_FILENO || FD_SETSIZE <= fd) {
- cmd->has_error = -1;
+ if (!cmd->has_error)
+ cmd->has_error = -1;
VIR_DEBUG("cannot preserve %d", fd);
return;
}
FD_SET(fd, &cmd->preserve);
+ if (transfer)
+ FD_SET(fd, &cmd->transfer);
}
/*
+ * Preserve the specified file descriptor
+ * in the child, instead of closing it.
+ * The parent is still responsible for managing fd.
+ */
+void
+virCommandPreserveFD(virCommandPtr cmd, int fd)
+{
+ return virCommandKeepFD(cmd, fd, false);
+}
+
+/*
+ * Transfer the specified file descriptor
+ * to the child, instead of closing it.
+ * Close the fd in the parent during Run/RunAsync/Free.
+ */
+void
+virCommandTransferFD(virCommandPtr cmd, int fd)
+{
+ return virCommandKeepFD(cmd, fd, true);
+}
+
+
+/*
* Save the child PID in a pidfile
*/
void
@@ -192,6 +245,19 @@ virCommandDaemonize(virCommandPtr cmd)
}
/*
+ * Set FDs created by virCommandSetOutputFD and virCommandSetErrorFD
+ * as non-blocking in the parent.
+ */
+void
+virCommandNonblockingFDs(virCommandPtr cmd)
+{
+ if (!cmd || cmd->has_error)
+ return;
+
+ cmd->flags |= VIR_EXEC_NONBLOCK;
+}
+
+/*
* Add an environment variable to the child
* using separate name & value strings
*/
@@ -317,20 +383,24 @@ virCommandAddArg(virCommandPtr cmd, const char *val)
/*
- * Add "NAME=VAL" as a single command line argument to the child
+ * Add a command line argument created by a printf-style format
*/
void
-virCommandAddArgPair(virCommandPtr cmd, const char *name, const char *val)
+virCommandAddArgFormat(virCommandPtr cmd, const char *format, ...)
{
char *arg;
+ va_list list;
if (!cmd || cmd->has_error)
return;
- if (virAsprintf(&arg, "%s=%s", name, val) < 0) {
+ va_start(list, format);
+ if (virVasprintf(&arg, format, list) < 0) {
cmd->has_error = ENOMEM;
+ va_end(list);
return;
}
+ va_end(list);
/* Arg plus trailing NULL. */
if (VIR_RESIZE_N(cmd->args, cmd->maxargs, cmd->nargs, 1 + 1) < 0) {
@@ -343,6 +413,15 @@ virCommandAddArgPair(virCommandPtr cmd, const char *name, const char *val)
}
/*
+ * Add "NAME=VAL" as a single command line argument to the child
+ */
+void
+virCommandAddArgPair(virCommandPtr cmd, const char *name, const char *val)
+{
+ virCommandAddArgFormat(cmd, "%s=%s", name, val);
+}
+
+/*
* Add a NULL terminated list of args
*/
void
@@ -569,6 +648,89 @@ virCommandSetPreExecHook(virCommandPtr cmd, virExecHook hook, void *opaque)
/*
+ * Call after adding all arguments and environment settings, but before
+ * Run/RunAsync, to immediately output the environment and arguments of
+ * cmd to logfd. If virCommandRun cannot succeed (because of an
+ * out-of-memory condition while building cmd), nothing will be logged.
+ */
+void
+virCommandWriteArgLog(virCommandPtr cmd, int logfd)
+{
+ int ioError = 0;
+ size_t i;
+
+ /* Any errors will be reported later by virCommandRun, which means
+ * no command will be run, so there is nothing to log. */
+ if (!cmd || cmd->has_error)
+ return;
+
+ for (i = 0 ; i < cmd->nenv ; i++) {
+ if (safewrite(logfd, cmd->env[i], strlen(cmd->env[i])) < 0)
+ ioError = errno;
+ if (safewrite(logfd, " ", 1) < 0)
+ ioError = errno;
+ }
+ for (i = 0 ; i < cmd->nargs ; i++) {
+ if (safewrite(logfd, cmd->args[i], strlen(cmd->args[i])) < 0)
+ ioError = errno;
+ if (safewrite(logfd, i == cmd->nargs - 1 ? "\n" : " ", 1) < 0)
+ ioError = errno;
+ }
+
+ if (ioError) {
+ char ebuf[1024];
+ VIR_WARN("Unable to write command %s args to logfile: %s",
+ cmd->args[0], virStrerror(ioError, ebuf, sizeof ebuf));
+ }
+}
+
+
+/*
+ * Call after adding all arguments and environment settings, but before
+ * Run/RunAsync, to return a string representation of the environment and
+ * arguments of cmd. If virCommandRun cannot succeed (because of an
+ * out-of-memory condition while building cmd), NULL will be returned.
+ * Caller is responsible for freeing the resulting string.
+ */
+char *
+virCommandToString(virCommandPtr cmd)
+{
+ size_t i;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ /* Cannot assume virCommandRun will be called; so report the error
+ * now. If virCommandRun is called, it will report the same error. */
+ if (!cmd || cmd->has_error == -1) {
+ virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("invalid use of command API"));
+ return NULL;
+ }
+ if (cmd->has_error == ENOMEM) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ for (i = 0; i < cmd->nenv; i++) {
+ virBufferAdd(&buf, cmd->env[i], strlen(cmd->env[i]));
+ virBufferAddChar(&buf, ' ');
+ }
+ virBufferAdd(&buf, cmd->args[0], strlen(cmd->args[0]));
+ for (i = 1; i < cmd->nargs; i++) {
+ virBufferAddChar(&buf, ' ');
+ virBufferAdd(&buf, cmd->args[i], strlen(cmd->args[i]));
+ }
+
+ if (virBufferError(&buf)) {
+ virBufferFreeAndReset(&buf);
+ virReportOOMError();
+ return NULL;
+ }
+
+ return virBufferContentAndReset(&buf);
+}
+
+
+/*
* Manage input and output to the child process.
*/
static int
@@ -823,6 +985,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
{
int ret;
char *str;
+ int i;
if (!cmd || cmd->has_error == -1) {
virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -868,6 +1031,14 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
VIR_DEBUG("Command result %d, with PID %d",
ret, (int)cmd->pid);
+ for (i = STDERR_FILENO + 1; i < FD_SETSIZE; i++) {
+ if (FD_ISSET(i, &cmd->transfer)) {
+ int tmpfd = i;
+ VIR_FORCE_CLOSE(tmpfd);
+ FD_CLR(i, &cmd->transfer);
+ }
+ }
+
if (ret == 0 && pid)
*pid = cmd->pid;
@@ -941,6 +1112,13 @@ virCommandFree(virCommandPtr cmd)
if (!cmd)
return;
+ for (i = STDERR_FILENO + 1; i < FD_SETSIZE; i++) {
+ if (FD_ISSET(i, &cmd->transfer)) {
+ int tmpfd = i;
+ VIR_FORCE_CLOSE(tmpfd);
+ }
+ }
+
VIR_FORCE_CLOSE(cmd->outfd);
VIR_FORCE_CLOSE(cmd->errfd);
diff --git a/src/util/command.h b/src/util/command.h
index ca24869..9b04e68 100644
--- a/src/util/command.h
+++ b/src/util/command.h
@@ -39,18 +39,34 @@ virCommandPtr virCommandNew(const char *binary) ATTRIBUTE_NONNULL(1);
*/
virCommandPtr virCommandNewArgs(const char *const*args) ATTRIBUTE_NONNULL(1);
+/*
+ * Create a new command with a NULL terminated
+ * list of args, starting with the binary to run
+ */
+virCommandPtr virCommandNewArgList(const char *binary, ...)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL;
+
/* All error report from these setup APIs is
- * delayed until the Run/Exec/Wait methods
+ * delayed until the Run/RunAsync methods
*/
/*
* Preserve the specified file descriptor
- * in the child, instead of closing it
+ * in the child, instead of closing it.
+ * The parent is still responsible for managing fd.
*/
void virCommandPreserveFD(virCommandPtr cmd,
int fd);
/*
+ * Transfer the specified file descriptor
+ * to the child, instead of closing it.
+ * Close the fd in the parent during Run/RunAsync/Free.
+ */
+void virCommandTransferFD(virCommandPtr cmd,
+ int fd);
+
+/*
* Save the child PID in a pidfile
*/
void virCommandSetPidFile(virCommandPtr cmd,
@@ -74,6 +90,11 @@ void virCommandAllowCap(virCommandPtr cmd,
*/
void virCommandDaemonize(virCommandPtr cmd);
+/*
+ * Set FDs created by virCommandSetOutputFD and virCommandSetErrorFD
+ * as non-blocking in the parent.
+ */
+void virCommandNonblockingFDs(virCommandPtr cmd);
/*
* Add an environment variable to the child
@@ -106,6 +127,14 @@ void virCommandAddEnvPassCommon(virCommandPtr cmd);
*/
void virCommandAddArg(virCommandPtr cmd,
const char *val) ATTRIBUTE_NONNULL(2);
+
+/*
+ * Add a command line argument created by a printf-style format
+ */
+void virCommandAddArgFormat(virCommandPtr cmd,
+ const char *format, ...)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3);
+
/*
* Add a command line argument to the child
*/
@@ -179,6 +208,24 @@ void virCommandSetPreExecHook(virCommandPtr cmd,
void *opaque) ATTRIBUTE_NONNULL(2);
/*
+ * Call after adding all arguments and environment settings, but before
+ * Run/RunAsync, to immediately output the environment and arguments of
+ * cmd to logfd. If virCommandRun cannot succeed (because of an
+ * out-of-memory condition while building cmd), nothing will be logged.
+ */
+void virCommandWriteArgLog(virCommandPtr cmd,
+ int logfd);
+
+/*
+ * Call after adding all arguments and environment settings, but before
+ * Run/RunAsync, to return a string representation of the environment and
+ * arguments of cmd. If virCommandRun cannot succeed (because of an
+ * out-of-memory condition while building cmd), NULL will be returned.
+ * Caller is responsible for freeing the resulting string.
+ */
+char *virCommandToString(virCommandPtr cmd) ATTRIBUTE_RETURN_CHECK;
+
+/*
* Run the command and wait for completion.
* Returns -1 on any error executing the
* command. Returns 0 if the command executed,
diff --git a/src/util/hooks.c b/src/util/hooks.c
index 1139d88..5ba2036 100644
--- a/src/util/hooks.c
+++ b/src/util/hooks.c
@@ -250,12 +250,10 @@ virHookCall(int driver, const char *id, int op, int sub_op, const char *extra,
return(-1);
}
- cmd = virCommandNew(path);
+ cmd = virCommandNewArgList(path, id, opstr, subopstr, extra, NULL);
virCommandAddEnvPassCommon(cmd);
- virCommandAddArgList(cmd, id, opstr, subopstr, extra, NULL);
-
if (input)
virCommandSetInputBuffer(cmd, input);
diff --git a/src/util/util.c b/src/util/util.c
index a2582aa..f6050de 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -88,42 +88,44 @@ verify(sizeof(gid_t) <= sizeof (unsigned int) &&
__FUNCTION__, __LINE__, __VA_ARGS__)
/* Like read(), but restarts after EINTR */
-int saferead(int fd, void *buf, size_t count)
-{
- size_t nread = 0;
- while (count > 0) {
- ssize_t r = read(fd, buf, count);
- if (r < 0 && errno == EINTR)
- continue;
- if (r < 0)
- return r;
- if (r == 0)
- return nread;
- buf = (char *)buf + r;
- count -= r;
- nread += r;
- }
- return nread;
+ssize_t
+saferead(int fd, void *buf, size_t count)
+{
+ size_t nread = 0;
+ while (count > 0) {
+ ssize_t r = read(fd, buf, count);
+ if (r < 0 && errno == EINTR)
+ continue;
+ if (r < 0)
+ return r;
+ if (r == 0)
+ return nread;
+ buf = (char *)buf + r;
+ count -= r;
+ nread += r;
+ }
+ return nread;
}
/* Like write(), but restarts after EINTR */
-ssize_t safewrite(int fd, const void *buf, size_t count)
-{
- size_t nwritten = 0;
- while (count > 0) {
- ssize_t r = write(fd, buf, count);
-
- if (r < 0 && errno == EINTR)
- continue;
- if (r < 0)
- return r;
- if (r == 0)
- return nwritten;
- buf = (const char *)buf + r;
- count -= r;
- nwritten += r;
- }
- return nwritten;
+ssize_t
+safewrite(int fd, const void *buf, size_t count)
+{
+ size_t nwritten = 0;
+ while (count > 0) {
+ ssize_t r = write(fd, buf, count);
+
+ if (r < 0 && errno == EINTR)
+ continue;
+ if (r < 0)
+ return r;
+ if (r == 0)
+ return nwritten;
+ buf = (const char *)buf + r;
+ count -= r;
+ nwritten += r;
+ }
+ return nwritten;
}
#ifdef HAVE_POSIX_FALLOCATE
@@ -2197,6 +2199,22 @@ virParseVersionString(const char *str, unsigned long *version)
}
/**
+ * virVasprintf
+ *
+ * like glibc's vasprintf but makes sure *strp == NULL on failure
+ */
+int
+virVasprintf(char **strp, const char *fmt, va_list list)
+{
+ int ret;
+
+ if ((ret = vasprintf(strp, fmt, list)) == -1)
+ *strp = NULL;
+
+ return ret;
+}
+
+/**
* virAsprintf
*
* like glibc's_asprintf but makes sure *strp == NULL on failure
@@ -2208,10 +2226,7 @@ virAsprintf(char **strp, const char *fmt, ...)
int ret;
va_start(ap, fmt);
-
- if ((ret = vasprintf(strp, fmt, ap)) == -1)
- *strp = NULL;
-
+ ret = virVasprintf(strp, fmt, ap);
va_end(ap);
return ret;
}
diff --git a/src/util/util.h b/src/util/util.h
index a240d87..deaf8bb 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -31,12 +31,13 @@
# include <unistd.h>
# include <sys/select.h>
# include <sys/types.h>
+# include <stdarg.h>
# ifndef MIN
# define MIN(a, b) ((a) < (b) ? (a) : (b))
# endif
-int saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK;
+ssize_t saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK;
ssize_t safewrite(int fd, const void *buf, size_t count)
ATTRIBUTE_RETURN_CHECK;
int safezero(int fd, int flags, off_t offset, off_t len)
@@ -202,7 +203,10 @@ int virMacAddrCompare (const char *mac1, const char *mac2);
void virSkipSpaces(const char **str);
int virParseNumber(const char **str);
int virParseVersionString(const char *str, unsigned long *version);
-int virAsprintf(char **strp, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(2, 3);
+int virAsprintf(char **strp, const char *fmt, ...)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3);
+int virVasprintf(char **strp, const char *fmt, va_list list)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 0);
char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
ATTRIBUTE_RETURN_CHECK;
char *virStrcpy(char *dest, const char *src, size_t destbytes)
diff --git a/src/util/virtaudit.c b/src/util/virtaudit.c
index b630fce..e6bd07f 100644
--- a/src/util/virtaudit.c
+++ b/src/util/virtaudit.c
@@ -94,7 +94,7 @@ void virAuditSend(const char *file ATTRIBUTE_UNUSED, const char *func,
#endif
va_start(args, fmt);
- if (vasprintf(&str, fmt, args) < 0) {
+ if (virVasprintf(&str, fmt, args) < 0) {
VIR_WARN0("Out of memory while formatting audit message");
str = NULL;
}
diff --git a/tests/commanddata/test16.log b/tests/commanddata/test16.log
new file mode 100644
index 0000000..2433a4d
--- /dev/null
+++ b/tests/commanddata/test16.log
@@ -0,0 +1 @@
+A=B /bin/true C
diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index dc8998f..2ee9153 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -29,6 +29,7 @@
#include "internal.h"
#include "util.h"
#include "memory.h"
+#include "files.h"
static int envsort(const void *a, const void *b) {
@@ -102,7 +103,7 @@ int main(int argc, char **argv) {
strcpy(cwd, ".../commanddata");
fprintf(log, "CWD:%s\n", cwd);
- fclose(log);
+ VIR_FORCE_FCLOSE(log);
char buf[1024];
ssize_t got;
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 5479bfc..46d6ae5 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -25,6 +25,8 @@
#include <string.h>
#include <unistd.h>
#include <signal.h>
+#include <sys/stat.h>
+#include <fcntl.h>
#include "testutils.h"
#include "internal.h"
@@ -32,6 +34,7 @@
#include "util.h"
#include "memory.h"
#include "command.h"
+#include "files.h"
#ifndef __linux__
@@ -166,10 +169,9 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) {
int newfd1 = dup(STDERR_FILENO);
int newfd2 = dup(STDERR_FILENO);
int newfd3 = dup(STDERR_FILENO);
- close(newfd2);
virCommandPreserveFD(cmd, newfd1);
- virCommandPreserveFD(cmd, newfd3);
+ virCommandTransferFD(cmd, newfd3);
if (virCommandRun(cmd, NULL) < 0) {
virErrorPtr err = virGetLastError();
@@ -177,7 +179,16 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) {
return -1;
}
+ if (fcntl(newfd1, F_GETFL) < 0 ||
+ fcntl(newfd2, F_GETFL) < 0 ||
+ fcntl(newfd3, F_GETFL) >= 0) {
+ puts("fds in wrong state");
+ return -1;
+ }
+
virCommandFree(cmd);
+ VIR_FORCE_CLOSE(newfd1);
+ VIR_FORCE_CLOSE(newfd2);
return checkoutput("test3");
}
@@ -501,6 +512,52 @@ static int test15(const void *unused ATTRIBUTE_UNUSED) {
return checkoutput("test15");
}
+/*
+ * Don't run program; rather, log what would be run.
+ */
+static int test16(const void *unused ATTRIBUTE_UNUSED) {
+ virCommandPtr cmd = virCommandNew("/bin/true");
+ char *outactual = NULL;
+ const char *outexpect = "A=B /bin/true C";
+ int ret = -1;
+ int fd = -1;
+
+ virCommandAddEnvPair(cmd, "A", "B");
+ virCommandAddArg(cmd, "C");
+
+ if ((outactual = virCommandToString(cmd)) == NULL) {
+ virErrorPtr err = virGetLastError();
+ printf("Cannot convert to string: %s\n", err->message);
+ return -1;
+ }
+ if ((fd = open(abs_builddir "/commandhelper.log",
+ O_CREAT | O_TRUNC | O_WRONLY, 0600)) < 0) {
+ printf("Cannot open log file: %s\n", strerror (errno));
+ goto cleanup;
+ }
+ virCommandWriteArgLog(cmd, fd);
+ if (VIR_CLOSE(fd) < 0) {
+ printf("Cannot close log file: %s\n", strerror (errno));
+ goto cleanup;
+ }
+
+ virCommandFree(cmd);
+
+ if (checkoutput("test16") < 0)
+ goto cleanup;
+
+ if (!STREQ(outactual, outexpect)) {
+ virtTestDifference(stderr, outactual, outexpect);
+ goto cleanup;
+ }
+ ret = 0;
+
+cleanup:
+ VIR_FORCE_CLOSE(fd);
+ VIR_FREE(outactual);
+ return ret;
+}
+
static int
mymain(int argc, char **argv)
{
@@ -563,6 +620,7 @@ mymain(int argc, char **argv)
DO_TEST(test13);
DO_TEST(test14);
DO_TEST(test15);
+ DO_TEST(test16);
return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b149ef4..07dde54 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -25,28 +25,30 @@ static struct qemud_driver driver;
# define MAX_FILE 4096
static int testCompareXMLToArgvFiles(const char *xml,
- const char *cmd,
+ const char *cmdline,
unsigned long long extraFlags,
const char *migrateFrom,
bool expectError) {
char argvData[MAX_FILE];
char *expectargv = &(argvData[0]);
+ int len;
char *actualargv = NULL;
- const char **argv = NULL;
- const char **qenv = NULL;
- const char **tmp = NULL;
- int ret = -1, len;
+ int ret = -1;
unsigned long long flags;
virDomainDefPtr vmdef = NULL;
virDomainChrDef monitor_chr;
virConnectPtr conn;
char *log = NULL;
+ virCommandPtr cmd = NULL;
if (!(conn = virGetConnect()))
goto fail;
- if (virtTestLoadFile(cmd, &expectargv, MAX_FILE) < 0)
+ len = virtTestLoadFile(cmdline, &expectargv, MAX_FILE);
+ if (len < 0)
goto fail;
+ if (len && expectargv[len - 1] == '\n')
+ expectargv[len - 1] = '\0';
if (!(vmdef = virDomainDefParseFile(driver.caps, xml,
VIR_DOMAIN_XML_INACTIVE)))
@@ -85,10 +87,9 @@ static int testCompareXMLToArgvFiles(const char *xml,
free(virtTestLogContentAndReset());
- if (qemudBuildCommandLine(conn, &driver,
- vmdef, &monitor_chr, 0, flags,
- &argv, &qenv,
- NULL, NULL, migrateFrom, NULL) < 0)
+ if (!(cmd = qemudBuildCommandLine(conn, &driver,
+ vmdef, &monitor_chr, false, flags,
+ migrateFrom, NULL)))
goto fail;
if ((log = virtTestLogContentAndReset()) == NULL)
@@ -105,36 +106,8 @@ static int testCompareXMLToArgvFiles(const char *xml,
virResetLastError();
}
- len = 1; /* for trailing newline */
- tmp = qenv;
- while (*tmp) {
- len += strlen(*tmp) + 1;
- tmp++;
- }
-
- tmp = argv;
- while (*tmp) {
- len += strlen(*tmp) + 1;
- tmp++;
- }
- if ((actualargv = malloc(sizeof(*actualargv)*len)) == NULL)
+ if (!(actualargv = virCommandToString(cmd)))
goto fail;
- actualargv[0] = '\0';
- tmp = qenv;
- while (*tmp) {
- if (actualargv[0])
- strcat(actualargv, " ");
- strcat(actualargv, *tmp);
- tmp++;
- }
- tmp = argv;
- while (*tmp) {
- if (actualargv[0])
- strcat(actualargv, " ");
- strcat(actualargv, *tmp);
- tmp++;
- }
- strcat(actualargv, "\n");
if (STRNEQ(expectargv, actualargv)) {
virtTestDifference(stderr, expectargv, actualargv);
@@ -146,22 +119,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
fail:
free(log);
free(actualargv);
- if (argv) {
- tmp = argv;
- while (*tmp) {
- free(*(char**)tmp);
- tmp++;
- }
- free(argv);
- }
- if (qenv) {
- tmp = qenv;
- while (*tmp) {
- free(*(char**)tmp);
- tmp++;
- }
- free(qenv);
- }
+ virCommandFree(cmd);
virDomainDefFree(vmdef);
virUnrefConnect(conn);
return ret;
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list