[libvirt] [PATCH 5/8] Convert QEMU driver to use virCommand APIs

Switch the QEMU driver over to using the virCommand APIs instead of passing around char **argv, **env. * src/qemu/qemu_conf.c, src/qemu/qemu_conf.h, src/qemu/qemu_driver.c: Convert to virCommand --- src/locking/lock_manager.c | 2 + src/qemu/qemu_conf.c | 632 +++++++++++++++++++------------------------- src/qemu/qemu_conf.h | 21 +- src/qemu/qemu_driver.c | 195 +++----------- 4 files changed, 324 insertions(+), 526 deletions(-) diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index dc5e7d8..6bd2844 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -32,6 +32,8 @@ #include <stdlib.h> #include <unistd.h> +#include "configmake.h" + #define VIR_FROM_THIS VIR_FROM_LOCKING #define virLockError(code, ...) \ diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index dce9a63..ec255c9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1554,12 +1554,24 @@ int qemudExtractVersionInfo(const char *qemu, int ret = -1, status; unsigned int version, is_kvm, kvm_version; unsigned long long flags = 0; + struct stat sb; if (retflags) *retflags = 0; 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 its hard to feed back a useful error + */ + if (stat(qemu, &sb) < 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; @@ -3972,18 +3984,14 @@ 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, + int monitor_json, + unsigned long long qemuCmdFlags, + const char *migrateFrom, + virDomainSnapshotObjPtr current_snapshot) { int i; char memory[50]; @@ -3993,10 +4001,6 @@ int qemudBuildCommandLine(virConnectPtr conn, 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]; @@ -4004,11 +4008,12 @@ int qemudBuildCommandLine(virConnectPtr conn, 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); @@ -4020,7 +4025,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) { @@ -4028,13 +4033,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; } } } @@ -4095,78 +4100,11 @@ 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 = virCommandNew(emulator); + if (!cmd) { + virReportOOMError(); + return NULL; + } /* Set '-m MB' based on maxmem, because the lower 'memory' limit * is set post-startup using the balloon driver. If balloon driver @@ -4175,26 +4113,17 @@ int qemudBuildCommandLine(virConnectPtr conn, snprintf(memory, sizeof(memory), "%lu", def->mem.max_balloon/1024); snprintf(domid, sizeof(domid), "%d", def->id); - ADD_ENV_LIT("LC_ALL=C"); + virCommandAddEnvPassCommon(cmd); - 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"); + virCommandAddArg(cmd, "-S"); /* 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); + virCommandAddArg(cmd, "-M"); + virCommandAddArg(cmd, def->os.machine); } if (qemuBuildCpuArgStr(driver, def, emulator, qemuCmdFlags, @@ -4202,27 +4131,27 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; if (cpu) { - ADD_ARG_LIT("-cpu"); - ADD_ARG_LIT(cpu); + virCommandAddArg(cmd, "-cpu"); + virCommandAddArg(cmd, cpu); 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"); + virCommandAddArg(cmd, "-no-kqemu"); else if (enableKQEMU) { - ADD_ARG_LIT("-enable-kqemu"); - ADD_ARG_LIT("-kernel-kqemu"); + virCommandAddArg(cmd, "-enable-kqemu"); + virCommandAddArg(cmd, "-kernel-kqemu"); } 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"); + virCommandAddArg(cmd, "-m"); + virCommandAddArg(cmd, memory); if (def->mem.hugepage_backed) { if (!driver->hugetlbfs_mount) { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -4240,43 +4169,44 @@ int qemudBuildCommandLine(virConnectPtr conn, def->emulator); goto error; } - ADD_ARG_LIT("-mem-prealloc"); - ADD_ARG_LIT("-mem-path"); - ADD_ARG_LIT(driver->hugepage_path); + virCommandAddArg(cmd, "-mem-prealloc"); + virCommandAddArg(cmd, "-mem-path"); + virCommandAddArg(cmd, driver->hugepage_path); } - 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); + virCommandAddArg(cmd, 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); + virCommandAddArg(cmd, "-uuid"); + virCommandAddArg(cmd, uuid); } 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"); + virCommandAddArg(cmd, domid); } 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"); + virCommandAddArg(cmd, domid); } else { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("qemu emulator '%s' does not support xen"), @@ -4318,13 +4248,15 @@ int qemudBuildCommandLine(virConnectPtr conn, smbioscmd = qemuBuildSmbiosBiosStr(source); if (smbioscmd != NULL) { - ADD_ARG_LIT("-smbios"); - ADD_ARG(smbioscmd); + virCommandAddArg(cmd, "-smbios"); + virCommandAddArg(cmd, smbioscmd); + VIR_FREE(smbioscmd); } smbioscmd = qemuBuildSmbiosSystemStr(source); if (smbioscmd != NULL) { - ADD_ARG_LIT("-smbios"); - ADD_ARG(smbioscmd); + virCommandAddArg(cmd, "-smbios"); + virCommandAddArg(cmd, smbioscmd); + VIR_FREE(smbioscmd); } } } @@ -4337,12 +4269,12 @@ 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"); /* Disabling global config files */ + virCommandAddArg(cmd, "-nodefaults"); /* Disabling default guest devices */ } if (monitor_chr) { @@ -4350,39 +4282,42 @@ 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"); + virCommandAddArg(cmd, "-mon"); if (monitor_json) - ADD_ARG_LIT("chardev=monitor,mode=control"); + virCommandAddArg(cmd, "chardev=monitor,mode=control"); else - ADD_ARG_LIT("chardev=monitor,mode=readline"); + virCommandAddArg(cmd, "chardev=monitor,mode=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: @@ -4397,9 +4332,8 @@ 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); - } + def->clock.data.timezone) + virCommandAddEnvPair(cmd, "TZ", def->clock.data.timezone); for (i = 0; i < def->clock.ntimers; i++) { switch (def->clock.timers[i]->name) { @@ -4422,7 +4356,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: @@ -4451,14 +4385,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, @@ -4487,7 +4421,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" */ @@ -4502,10 +4436,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++) { @@ -4529,7 +4463,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'; @@ -4548,24 +4483,26 @@ 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); + virCommandAddArg(cmd, "-kernel"); + virCommandAddArg(cmd, def->os.kernel); } if (def->os.initrd) { - ADD_ARG_LIT("-initrd"); - ADD_ARG_LIT(def->os.initrd); + virCommandAddArg(cmd, "-initrd"); + virCommandAddArg(cmd, def->os.initrd); } if (def->os.cmdline) { - ADD_ARG_LIT("-append"); - ADD_ARG_LIT(def->os.cmdline); + virCommandAddArg(cmd, "-append"); + virCommandAddArg(cmd, def->os.cmdline); } } else { - ADD_ARG_LIT("-bootloader"); - ADD_ARG_LIT(def->os.bootloader); + virCommandAddArg(cmd, "-bootloader"); + virCommandAddArg(cmd, def->os.bootloader); } for (i = 0 ; i < def->ndisks ; i++) { @@ -4598,13 +4535,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); } } @@ -4640,7 +4578,8 @@ 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"); + virCommandAddArg(cmd, disk->src); } else { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported usb disk type for '%s'"), disk->src); @@ -4664,7 +4603,7 @@ 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 @@ -4678,24 +4617,27 @@ 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"); + virCommandAddArg(cmd, "-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, fdc); + VIR_FREE(fdc); } else { - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(optstr = qemuBuildDriveDevStr(disk))) goto error; - ADD_ARG(optstr); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); } } } @@ -4707,7 +4649,8 @@ 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"); + virCommandAddArg(cmd, disk->src); } else { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported usb disk type for '%s'"), disk->src); @@ -4756,8 +4699,8 @@ int qemudBuildCommandLine(virConnectPtr conn, snprintf(file, PATH_MAX, "%s", disk->src); } - ADD_ARG_LIT(dev); - ADD_ARG_LIT(file); + virCommandAddArg(cmd, dev); + virCommandAddArg(cmd, file); } } @@ -4766,15 +4709,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) { @@ -4787,8 +4732,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"); + virCommandAddArg(cmd, "-net"); + virCommandAddArg(cmd, "none"); } } else { for (i = 0 ; i < def->nnets ; i++) { @@ -4811,15 +4756,9 @@ 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; - (*vmfds)[(*nvmfds)++] = tapfd; + virCommandPreserveFD(cmd, tapfd); if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name)) goto no_memory; @@ -4830,15 +4769,9 @@ 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; - (*vmfds)[(*nvmfds)++] = tapfd; + virCommandPreserveFD(cmd, tapfd); if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name)) goto no_memory; @@ -4851,12 +4784,8 @@ 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; - } + virCommandPreserveFD(cmd, vhostfd); - (*vmfds)[(*nvmfds)++] = vhostfd; if (snprintf(vhostfd_name, sizeof(vhostfd_name), "%d", vhostfd) goto no_memory; @@ -4872,30 +4801,34 @@ 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); } } } @@ -4903,8 +4836,8 @@ int qemudBuildCommandLine(virConnectPtr conn, 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"); + virCommandAddArg(cmd, "-serial"); + virCommandAddArg(cmd, "none"); } } else { for (i = 0 ; i < def->nserials ; i++) { @@ -4914,20 +4847,23 @@ 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"); + virCommandAddArg(cmd, "-device"); if (virAsprintf(&devstr, "isa-serial,chardev=%s", serial->info.alias) < 0) goto no_memory; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } else { - ADD_ARG_LIT("-serial"); + virCommandAddArg(cmd, "-serial"); if (!(devstr = qemuBuildChrArgStr(serial, NULL))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } } } @@ -4935,8 +4871,8 @@ int qemudBuildCommandLine(virConnectPtr conn, 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"); + virCommandAddArg(cmd, "-parallel"); + virCommandAddArg(cmd, "none"); } } else { for (i = 0 ; i < def->nparallels ; i++) { @@ -4946,20 +4882,23 @@ 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"); + virCommandAddArg(cmd, "-device"); if (virAsprintf(&devstr, "isa-parallel,chardev=%s", parallel->info.alias) < 0) goto no_memory; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } else { - ADD_ARG_LIT("-parallel"); + virCommandAddArg(cmd, "-parallel"); if (!(devstr = qemuBuildChrArgStr(parallel, NULL))) goto error; - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } } } @@ -4977,24 +4916,26 @@ 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"); + virCommandAddArg(cmd, "-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; } VIR_FREE(addr); - ADD_ARG(devstr); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); break; case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: @@ -5004,15 +4945,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; } } @@ -5030,15 +4973,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: @@ -5052,20 +4997,21 @@ 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"); + virCommandAddArg(cmd, "-usbdevice"); + virCommandAddArg(cmd, input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet"); } } } @@ -5109,7 +5055,7 @@ 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 */ } @@ -5124,11 +5070,12 @@ int qemudBuildCommandLine(virConnectPtr conn, optstr = virBufferContentAndReset(&opt); - ADD_ARG_LIT("-vnc"); - ADD_ARG(optstr); + virCommandAddArg(cmd, "-vnc"); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); if (def->graphics[0]->data.vnc.keymap) { - ADD_ARG_LIT("-k"); - ADD_ARG_LIT(def->graphics[0]->data.vnc.keymap); + virCommandAddArg(cmd, "-k"); + virCommandAddArg(cmd, def->graphics[0]->data.vnc.keymap); } /* Unless user requested it, set the audio backend to none, to @@ -5136,45 +5083,34 @@ 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) + 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.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.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) { @@ -5227,16 +5163,17 @@ int qemudBuildCommandLine(virConnectPtr conn, optstr = virBufferContentAndReset(&opt); - ADD_ARG_LIT("-spice"); - ADD_ARG(optstr); + virCommandAddArg(cmd, "-spice"); + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); if (def->graphics[0]->data.spice.keymap) { - ADD_ARG_LIT("-k"); - ADD_ARG_LIT(def->graphics[0]->data.spice.keymap); + virCommandAddArg(cmd, "-k"); + virCommandAddArg(cmd, def->graphics[0]->data.spice.keymap); } /* 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, @@ -5265,18 +5202,18 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT("-vga"); - ADD_ARG_LIT(vgastr); + virCommandAddArg(cmd, "-vga"); + virCommandAddArg(cmd, vgastr); } } 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: @@ -5303,12 +5240,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, @@ -5321,8 +5259,8 @@ int qemudBuildCommandLine(virConnectPtr conn, /* 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"); + virCommandAddArg(cmd, "-vga"); + virCommandAddArg(cmd, "none"); } } @@ -5337,15 +5275,16 @@ 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"); + virCommandAddArg(cmd, "-soundhw"); + virCommandAddArg(cmd, "pcspk"); } else { - ADD_ARG_LIT("-device"); + virCommandAddArg(cmd, "-device"); if (!(str = qemuBuildSoundDevStr(sound))) goto error; - ADD_ARG(str); + virCommandAddArg(cmd, str); + VIR_FREE(str); } } } else { @@ -5368,8 +5307,9 @@ int qemudBuildCommandLine(virConnectPtr conn, if (i < (def->nsounds - 1)) strncat(modstr, ",", size--); } - ADD_ARG_LIT("-soundhw"); - ADD_ARG(modstr); + virCommandAddArg(cmd, "-soundhw"); + virCommandAddArg(cmd, modstr); + VIR_FREE(modstr); } } @@ -5379,13 +5319,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) { @@ -5397,7 +5337,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) { @@ -5405,8 +5346,8 @@ int qemudBuildCommandLine(virConnectPtr conn, "%s", _("invalid watchdog action")); goto error; } - ADD_ARG_LIT("-watchdog-action"); - ADD_ARG_LIT(action); + virCommandAddArg(cmd, "-watchdog-action"); + virCommandAddArg(cmd, action); } /* Add host passthrough hardware */ @@ -5419,15 +5360,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); } } @@ -5446,26 +5389,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; + virCommandPreserveFD(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")); @@ -5475,8 +5414,8 @@ int qemudBuildCommandLine(virConnectPtr conn, } if (migrateFrom) { - ADD_ARG_LIT("-incoming"); - ADD_ARG_LIT(migrateFrom); + virCommandAddArg(cmd, "-incoming"); + virCommandAddArg(cmd, migrateFrom); } /* QEMU changed its default behavior to not include the virtio balloon @@ -5495,76 +5434,47 @@ 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"); + virCommandAddArg(cmd, "-balloon"); + virCommandAddArg(cmd, "virtio"); } } if (current_snapshot && current_snapshot->def->active) { - ADD_ARG_LIT("-loadvm"); - ADD_ARG_LIT(current_snapshot->def->name); + virCommandAddArg(cmd, "-loadvm"); + virCommandAddArg(cmd, current_snapshot->def->name); } 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]); + qemuDomainCmdlineDefPtr qemucmd; + + qemucmd = def->namespaceData; + for (i = 0; i < qemucmd->num_args; i++) + virCommandAddArg(cmd, qemucmd->args[i]); + for (i = 0; i < qemucmd->num_env; i++) { + if (qemucmd->env_value[i]) + virCommandAddEnvPair(cmd, qemucmd->env_name[i], qemucmd->env_value[i]); else - ADD_ENV_PAIR(cmd->env_name[i], ""); + virCommandAddEnvPair(cmd, qemucmd->env_name[i], ""); } } - ADD_ARG(NULL); - ADD_ENV(NULL); - - *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 3789ed2..b006e75 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -41,6 +41,7 @@ # include "driver.h" # include "bitmap.h" # include "locking/lock_manager.h" +# include "command.h" # define qemudDebug(fmt, ...) do {} while(0) @@ -230,18 +231,14 @@ int qemudParseHelpStr (const char *qemu, unsigned int *is_kvm, unsigned int *kvm_version); -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, + int monitor_json, + unsigned long long qemuCmdFlags, + const char *migrateFrom, + virDomainSnapshotObjPtr current_snapshot) ATTRIBUTE_NONNULL(1); /* With vlan == -1, use netdev syntax, else old hostnet */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9dcf02d..0b0b6fe 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3017,14 +3017,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, @@ -3881,16 +3873,8 @@ 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; @@ -3898,6 +3882,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, char *timestamp; qemuDomainObjPrivatePtr priv = vm->privateData; virDomainLockPtr lock = NULL; + virCommandPtr cmd = NULL; struct qemudHookData hookData; hookData.conn = conn; @@ -3905,8 +3890,6 @@ static int qemudStartVMDaemon(virConnectPtr conn, hookData.driver = driver; hookData.lockState = NULL; /* XXX add lock state from migration source ! */ - FD_ZERO(&keepfd); - DEBUG0("Beginning VM startup process"); if (virDomainObjIsActive(vm)) { @@ -3987,21 +3970,8 @@ static int qemudStartVMDaemon(virConnectPtr conn, if ((logfile = qemudLogFD(driver, vm->def->name)) < 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, + if (qemudExtractVersionInfo(vm->def->emulator, NULL, &qemuCmdFlags) < 0) goto cleanup; @@ -4070,10 +4040,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, qemuCmdFlags, + migrateFrom, + vm->current_snapshot))) goto cleanup; if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir) < 0) @@ -4108,70 +4078,46 @@ 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; - } - - VIR_WARN("Executing %s", argv[0]); - ret = virExecDaemonize(argv, progenv, &keepfd, &child, - stdin_fd, &logfile, &logfile, - runflags, - qemudSecurityHook, &hookData, - pidfile); - VIR_WARN("Executing done %s", argv[0]); + /** XXXX runflags = VIR_EXEC_NONBLOCK; */ + 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); + virCommandSetPidFile(cmd, pidfile); + virCommandDaemonize(cmd); + + ret = virCommandRun(cmd, NULL); + VIR_WARN("Executing done %s", vm->def->emulator); VIR_FREE(pidfile); /* XXX this is bollocks. Need a sync point */ sleep(5); - VIR_WARN("Locking %s", argv[0]); + VIR_WARN("Locking %s", vm->def->emulator); if (!(lock = virDomainLockForStartup(driver->contentLockManager, driver->metadataLockManager, NULL, /* XXX lock state */ vm))) goto cleanup; - VIR_WARN("Labelling %s", argv[0]); + VIR_WARN("Labelling %s", vm->def->emulator); DEBUG0("Setting domain security labels"); if (virSecurityManagerSetAllLabel(driver->securityManager, vm, stdin_path) < 0) goto cleanup; - VIR_WARN("All done %s", argv[0]); + VIR_WARN("All done %s", vm->def->emulator); /* wait for qemu process to to show up */ @@ -4181,6 +4127,11 @@ static int qemudStartVMDaemon(virConnectPtr conn, _("Domain %s didn't show up\n"), vm->def->name); ret = -1; } +#if 0 + /* + * XXX this is bogus. It isn't safe to set vm->pid = chidl + * because child no longer exists. + */ } else if (ret == -2) { /* The virExec process that launches the daemon failed. Pending on * when it failed (we can't determine for sure), there may be @@ -4191,30 +4142,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; @@ -4264,6 +4201,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, goto cleanup; virDomainLockReleaseAndFree(lock); + virCommandFree(cmd); VIR_FORCE_CLOSE(logfile); @@ -4274,14 +4212,15 @@ cleanup: * that will re-aquire the lock in order to perform relabelling */ virDomainLockReleaseAndFree(lock); + virCommandFree(cmd); + + VIR_FORCE_CLOSE(logfile); /* 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); - VIR_FORCE_CLOSE(logfile); - return -1; } @@ -7312,13 +7251,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,69 +7302,24 @@ 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, 0, 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); virDomainDefFree(def); return ret; -- 1.7.2.3