[libvirt] [PATCH 1/2] qemu: refactor graphics code to not hardcode a single display

The check for a single display remains so no new functionality is added. --- Concerning both patches, I've tested running with all three simultaneously (sdl+spice+vnc) and with spice+vnc, using a single qxl device. src/qemu/qemu_command.c | 2425 ++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +- 2 files changed, 1259 insertions(+), 1236 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1e96982..f9e4d4d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4410,171 +4410,499 @@ error: return -1; } -/* - * Constructs a argv suitable for launching qemu with config defined - * for a given virtual machine. - * - * XXX 'conn' is only required to resolve network -> bridge name - * figure out how to remove this requirement some day - */ -virCommandPtr -qemuBuildCommandLine(virConnectPtr conn, - struct qemud_driver *driver, - virDomainDefPtr def, - virDomainChrSourceDefPtr monitor_chr, - bool monitor_json, - qemuCapsPtr caps, - const char *migrateFrom, - int migrateFd, - virDomainSnapshotObjPtr snapshot, - enum virNetDevVPortProfileOp vmop) +enum { + OK=0, + ERROR=1, + NO_MEMORY=2, +}; + +static int +qemuBuildGraphicsCommandLine(struct qemud_driver *driver, + virCommandPtr cmd, + virDomainDefPtr def, + qemuCapsPtr caps, + virDomainGraphicsDefPtr graphics) { - int i, j; - struct utsname ut; - int disableKQEMU = 0; - int enableKQEMU = 0; - int disableKVM = 0; - int enableKVM = 0; - const char *emulator; - char uuid[VIR_UUID_STRING_BUFLEN]; - char *cpu; - char *smp; - int last_good_net = -1; - bool hasHwVirt = false; - virCommandPtr cmd = NULL; - bool emitBootindex = false; - int usbcontroller = 0; - bool usblegacy = false; - uname_normalize(&ut); - int contOrder[] = { - /* We don't add an explicit IDE or FD controller because the - * provided PIIX4 device already includes one. It isn't possible to - * remove the PIIX4. */ - VIR_DOMAIN_CONTROLLER_TYPE_USB, - VIR_DOMAIN_CONTROLLER_TYPE_SCSI, - VIR_DOMAIN_CONTROLLER_TYPE_SATA, - VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, - VIR_DOMAIN_CONTROLLER_TYPE_CCID, - }; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + virBuffer opt = VIR_BUFFER_INITIALIZER; - VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " - "caps=%p migrateFrom=%s migrateFD=%d " - "snapshot=%p vmop=%d", - conn, driver, def, monitor_chr, monitor_json, - caps, migrateFrom, migrateFd, snapshot, vmop); + if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vnc graphics are not supported with this QEMU")); + return ERROR; + } - virUUIDFormat(def->uuid, uuid); + if (graphics->data.vnc.socket || + driver->vncAutoUnixSocket) { - emulator = def->emulator; + if (!graphics->data.vnc.socket && + virAsprintf(&graphics->data.vnc.socket, + "%s/%s.vnc", driver->libDir, def->name) == -1) { + return NO_MEMORY; + } - /* - * do not use boot=on for drives when not using KVM since this - * is not supported at all in upstream QEmu. - */ - if (qemuCapsGet(caps, QEMU_CAPS_KVM) && - (def->virtType == VIR_DOMAIN_VIRT_QEMU)) - qemuCapsClear(caps, QEMU_CAPS_DRIVE_BOOT); + virBufferAsprintf(&opt, "unix:%s", + graphics->data.vnc.socket); - switch (def->virtType) { - case VIR_DOMAIN_VIRT_QEMU: - if (qemuCapsGet(caps, QEMU_CAPS_KQEMU)) - disableKQEMU = 1; - if (qemuCapsGet(caps, QEMU_CAPS_KVM)) - disableKVM = 1; - break; + } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + bool escapeAddr; + int ret; - case VIR_DOMAIN_VIRT_KQEMU: - if (qemuCapsGet(caps, QEMU_CAPS_KVM)) - disableKVM = 1; + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break; - if (qemuCapsGet(caps, QEMU_CAPS_ENABLE_KQEMU)) { - enableKQEMU = 1; - } else if (!qemuCapsGet(caps, QEMU_CAPS_KQEMU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("the QEMU binary %s does not support kqemu"), - emulator); - goto error; - } - break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + return 1; + } + if (ret < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + return 1; + } + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it will + * show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + return 1; + break; + } - case VIR_DOMAIN_VIRT_KVM: - if (qemuCapsGet(caps, QEMU_CAPS_KQEMU)) - disableKQEMU = 1; + if (!listenAddr) + listenAddr = driver->vncListen; - if (qemuCapsGet(caps, QEMU_CAPS_ENABLE_KVM)) { - enableKVM = 1; - } else if (!qemuCapsGet(caps, QEMU_CAPS_KVM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("the QEMU binary %s does not support kvm"), - emulator); - goto error; + escapeAddr = strchr(listenAddr, ':') != NULL; + if (escapeAddr) + virBufferAsprintf(&opt, "[%s]", listenAddr); + else + virBufferAdd(&opt, listenAddr, -1); + virBufferAsprintf(&opt, ":%d", + graphics->data.vnc.port - 5900); + + VIR_FREE(netAddr); + } else { + virBufferAsprintf(&opt, "%d", + graphics->data.vnc.port - 5900); } - break; - case VIR_DOMAIN_VIRT_XEN: - /* XXX better check for xenner */ - break; + if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { + if (graphics->data.vnc.auth.passwd || + driver->vncPassword) + virBufferAddLit(&opt, ",password"); - default: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("the QEMU binary %s does not support %s"), - emulator, virDomainVirtTypeToString(def->virtType)); - break; - } + if (driver->vncTLS) { + virBufferAddLit(&opt, ",tls"); + if (driver->vncTLSx509verify) { + virBufferAsprintf(&opt, ",x509verify=%s", + driver->vncTLSx509certdir); + } else { + virBufferAsprintf(&opt, ",x509=%s", + driver->vncTLSx509certdir); + } + } - cmd = virCommandNew(emulator); + if (driver->vncSASL) { + virBufferAddLit(&opt, ",sasl"); - virCommandAddEnvPassCommon(cmd); + if (driver->vncSASLdir) + virCommandAddEnvPair(cmd, "SASL_CONF_DIR", + driver->vncSASLdir); - if (qemuCapsGet(caps, QEMU_CAPS_NAME)) { - virCommandAddArg(cmd, "-name"); - if (driver->setProcessName && - qemuCapsGet(caps, QEMU_CAPS_NAME_PROCESS)) { - virCommandAddArgFormat(cmd, "%s,process=qemu:%s", - def->name, def->name); + /* TODO: Support ACLs later */ + } + } + + virCommandAddArg(cmd, "-vnc"); + virCommandAddArgBuffer(cmd, &opt); + if (graphics->data.vnc.keymap) { + virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap, + NULL); + } + + /* Unless user requested it, set the audio backend to none, to + * prevent it opening the host OS audio devices, since that causes + * security issues and might not work when using VNC. + */ + if (driver->vncAllowHostAudio) { + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); } else { - virCommandAddArg(cmd, def->name); + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + } + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { + if (qemuCapsGet(caps, QEMU_CAPS_0_10) && + !qemuCapsGet(caps, QEMU_CAPS_SDL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("sdl not supported by '%s'"), + def->emulator); + return 1; } - } - virCommandAddArg(cmd, "-S"); /* freeze CPU */ - if (qemuBuildMachineArgStr(cmd, def, caps) < 0) - goto error; + if (graphics->data.sdl.xauth) + virCommandAddEnvPair(cmd, "XAUTHORITY", + graphics->data.sdl.xauth); + if (graphics->data.sdl.display) + virCommandAddEnvPair(cmd, "DISPLAY", + graphics->data.sdl.display); + if (graphics->data.sdl.fullscreen) + virCommandAddArg(cmd, "-full-screen"); - if (qemuBuildCpuArgStr(driver, def, emulator, caps, - &ut, &cpu, &hasHwVirt, !!migrateFrom) < 0) - goto error; + /* 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 + */ + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); + virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER"); - if (cpu) { - virCommandAddArgList(cmd, "-cpu", cpu, NULL); - VIR_FREE(cpu); + /* 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 (qemuCapsGet(caps, QEMU_CAPS_SDL)) + virCommandAddArg(cmd, "-sdl"); - if (qemuCapsGet(caps, QEMU_CAPS_NESTING) && - hasHwVirt) - virCommandAddArg(cmd, "-enable-nesting"); - } + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + int ret; + int defaultMode = graphics->data.spice.defaultMode; - if (disableKQEMU) - virCommandAddArg(cmd, "-no-kqemu"); - else if (enableKQEMU) - virCommandAddArgList(cmd, "-enable-kqemu", "-kernel-kqemu", NULL); - if (disableKVM) - virCommandAddArg(cmd, "-no-kvm"); - if (enableKVM) - virCommandAddArg(cmd, "-enable-kvm"); + if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice graphics are not supported with this QEMU")); + return 1; + } - if (def->os.loader) { - virCommandAddArg(cmd, "-bios"); - virCommandAddArg(cmd, def->os.loader); - } + virBufferAsprintf(&opt, "port=%u", graphics->data.spice.port); - /* 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. Update the - * XML to reflect our rounding. - */ - virCommandAddArg(cmd, "-m"); - def->mem.max_balloon = VIR_DIV_UP(def->mem.max_balloon, 1024) * 1024; + if (graphics->data.spice.tlsPort > 0) { + if (!driver->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice TLS port set in XML configuration," + " but TLS is disabled in qemu.conf")); + return 1; + } + virBufferAsprintf(&opt, ",tls-port=%u", + graphics->data.spice.tlsPort); + } + + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + return 1; + } + if (ret < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + return 1; + } + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it will + * show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + return 1; + break; + } + + if (!listenAddr) + listenAddr = driver->spiceListen; + if (listenAddr) + virBufferAsprintf(&opt, ",addr=%s", listenAddr); + + VIR_FREE(netAddr); + + int mm = graphics->data.spice.mousemode; + if (mm) { + switch (mm) { + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: + virBufferAsprintf(&opt, ",agent-mouse=off"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: + virBufferAsprintf(&opt, ",agent-mouse=on"); + break; + default: + break; + } + } + + /* In the password case we set it via monitor command, to avoid + * making it visible on CLI, so there's no use of password=XXX + * in this bit of the code */ + if (!graphics->data.spice.auth.passwd && + !driver->spicePassword) + virBufferAddLit(&opt, ",disable-ticketing"); + + if (driver->spiceTLS) + virBufferAsprintf(&opt, ",x509-dir=%s", + driver->spiceTLSx509certdir); + + switch (defaultMode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + virBufferAsprintf(&opt, ",tls-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + /* nothing */ + break; + } + + for (int i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { + int mode = graphics->data.spice.channels[i]; + switch (mode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + if (!driver->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf")); + return 1; + } + virBufferAsprintf(&opt, ",tls-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); + break; + } + } + if (graphics->data.spice.image) + virBufferAsprintf(&opt, ",image-compression=%s", + virDomainGraphicsSpiceImageCompressionTypeToString(graphics->data.spice.image)); + if (graphics->data.spice.jpeg) + virBufferAsprintf(&opt, ",jpeg-wan-compression=%s", + virDomainGraphicsSpiceJpegCompressionTypeToString(graphics->data.spice.jpeg)); + if (graphics->data.spice.zlib) + virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s", + virDomainGraphicsSpiceZlibCompressionTypeToString(graphics->data.spice.zlib)); + if (graphics->data.spice.playback) + virBufferAsprintf(&opt, ",playback-compression=%s", + virDomainGraphicsSpicePlaybackCompressionTypeToString(graphics->data.spice.playback)); + if (graphics->data.spice.streaming) + virBufferAsprintf(&opt, ",streaming-video=%s", + virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming)); + if (graphics->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) + virBufferAddLit(&opt, ",disable-copy-paste"); + + if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) { + /* If qemu supports seamless migration turn it + * unconditionally on. If migration destination + * doesn't support it, it fallbacks to previous + * migration algorithm silently. */ + virBufferAddLit(&opt, ",seamless-migration=on"); + } + + virCommandAddArg(cmd, "-spice"); + virCommandAddArgBuffer(cmd, &opt); + if (graphics->data.spice.keymap) + virCommandAddArgList(cmd, "-k", + graphics->data.spice.keymap, NULL); + /* SPICE includes native support for tunnelling audio, so we + * set the audio backend to point at SPICE's own driver + */ + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); + + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported graphics type '%s'"), + virDomainGraphicsTypeToString(graphics->type)); + return 1; + } + return 0; +} + +/* + * Constructs a argv suitable for launching qemu with config defined + * for a given virtual machine. + * + * XXX 'conn' is only required to resolve network -> bridge name + * figure out how to remove this requirement some day + */ +virCommandPtr +qemuBuildCommandLine(virConnectPtr conn, + struct qemud_driver *driver, + virDomainDefPtr def, + virDomainChrSourceDefPtr monitor_chr, + bool monitor_json, + qemuCapsPtr caps, + const char *migrateFrom, + int migrateFd, + virDomainSnapshotObjPtr snapshot, + enum virNetDevVPortProfileOp vmop) +{ + int i, j; + struct utsname ut; + int disableKQEMU = 0; + int enableKQEMU = 0; + int disableKVM = 0; + int enableKVM = 0; + const char *emulator; + char uuid[VIR_UUID_STRING_BUFLEN]; + char *cpu; + char *smp; + int last_good_net = -1; + bool hasHwVirt = false; + virCommandPtr cmd = NULL; + bool emitBootindex = false; + int usbcontroller = 0; + bool usblegacy = false; + uname_normalize(&ut); + int contOrder[] = { + /* We don't add an explicit IDE or FD controller because the + * provided PIIX4 device already includes one. It isn't possible to + * remove the PIIX4. */ + VIR_DOMAIN_CONTROLLER_TYPE_USB, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI, + VIR_DOMAIN_CONTROLLER_TYPE_SATA, + VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, + VIR_DOMAIN_CONTROLLER_TYPE_CCID, + }; + + VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " + "caps=%p migrateFrom=%s migrateFD=%d " + "snapshot=%p vmop=%d", + conn, driver, def, monitor_chr, monitor_json, + caps, migrateFrom, migrateFd, snapshot, vmop); + + virUUIDFormat(def->uuid, uuid); + + emulator = def->emulator; + + /* + * do not use boot=on for drives when not using KVM since this + * is not supported at all in upstream QEmu. + */ + if (qemuCapsGet(caps, QEMU_CAPS_KVM) && + (def->virtType == VIR_DOMAIN_VIRT_QEMU)) + qemuCapsClear(caps, QEMU_CAPS_DRIVE_BOOT); + + switch (def->virtType) { + case VIR_DOMAIN_VIRT_QEMU: + if (qemuCapsGet(caps, QEMU_CAPS_KQEMU)) + disableKQEMU = 1; + if (qemuCapsGet(caps, QEMU_CAPS_KVM)) + disableKVM = 1; + break; + + case VIR_DOMAIN_VIRT_KQEMU: + if (qemuCapsGet(caps, QEMU_CAPS_KVM)) + disableKVM = 1; + + if (qemuCapsGet(caps, QEMU_CAPS_ENABLE_KQEMU)) { + enableKQEMU = 1; + } else if (!qemuCapsGet(caps, QEMU_CAPS_KQEMU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the QEMU binary %s does not support kqemu"), + emulator); + goto error; + } + break; + + case VIR_DOMAIN_VIRT_KVM: + if (qemuCapsGet(caps, QEMU_CAPS_KQEMU)) + disableKQEMU = 1; + + if (qemuCapsGet(caps, QEMU_CAPS_ENABLE_KVM)) { + enableKVM = 1; + } else if (!qemuCapsGet(caps, QEMU_CAPS_KVM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the QEMU binary %s does not support kvm"), + emulator); + goto error; + } + break; + + case VIR_DOMAIN_VIRT_XEN: + /* XXX better check for xenner */ + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the QEMU binary %s does not support %s"), + emulator, virDomainVirtTypeToString(def->virtType)); + break; + } + + cmd = virCommandNew(emulator); + + virCommandAddEnvPassCommon(cmd); + + if (qemuCapsGet(caps, QEMU_CAPS_NAME)) { + virCommandAddArg(cmd, "-name"); + if (driver->setProcessName && + qemuCapsGet(caps, QEMU_CAPS_NAME_PROCESS)) { + virCommandAddArgFormat(cmd, "%s,process=qemu:%s", + def->name, def->name); + } else { + virCommandAddArg(cmd, def->name); + } + } + virCommandAddArg(cmd, "-S"); /* freeze CPU */ + + if (qemuBuildMachineArgStr(cmd, def, caps) < 0) + goto error; + + if (qemuBuildCpuArgStr(driver, def, emulator, caps, + &ut, &cpu, &hasHwVirt, !!migrateFrom) < 0) + goto error; + + if (cpu) { + virCommandAddArgList(cmd, "-cpu", cpu, NULL); + VIR_FREE(cpu); + + if (qemuCapsGet(caps, QEMU_CAPS_NESTING) && + hasHwVirt) + virCommandAddArg(cmd, "-enable-nesting"); + } + + if (disableKQEMU) + virCommandAddArg(cmd, "-no-kqemu"); + else if (enableKQEMU) + virCommandAddArgList(cmd, "-enable-kqemu", "-kernel-kqemu", NULL); + if (disableKVM) + virCommandAddArg(cmd, "-no-kvm"); + if (enableKVM) + virCommandAddArg(cmd, "-enable-kvm"); + + if (def->os.loader) { + virCommandAddArg(cmd, "-bios"); + virCommandAddArg(cmd, def->os.loader); + } + + /* 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. Update the + * XML to reflect our rounding. + */ + virCommandAddArg(cmd, "-m"); + def->mem.max_balloon = VIR_DIV_UP(def->mem.max_balloon, 1024) * 1024; virCommandAddArgFormat(cmd, "%llu", def->mem.max_balloon / 1024); if (def->mem.hugepage_backed) { if (!driver->hugetlbfs_mount) { @@ -4954,1231 +5282,924 @@ qemuBuildCommandLine(virConnectPtr conn, break; case VIR_DOMAIN_BOOT_NET: boot[i] = 'n'; - break; - default: - boot[i] = 'c'; - break; - } - } - boot[def->os.nBootDevs] = '\0'; - - virBufferAsprintf(&boot_buf, "%s", boot); - boot_nparams++; - } - - if (def->os.bootmenu) { - if (qemuCapsGet(caps, QEMU_CAPS_BOOT_MENU)) { - if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); - - if (def->os.bootmenu == VIR_DOMAIN_BOOT_MENU_ENABLED) - virBufferAsprintf(&boot_buf, "menu=on"); - else - virBufferAsprintf(&boot_buf, "menu=off"); - } else { - /* We cannot emit an error when bootmenu is enabled but - * unsupported because of backward compatibility */ - VIR_WARN("bootmenu is enabled but not " - "supported by this QEMU binary"); - } - } - - if (def->os.bios.rt_set) { - if (!qemuCapsGet(caps, QEMU_CAPS_REBOOT_TIMEOUT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("reboot timeout is not supported " - "by this QEMU binary")); - goto error; - } - - if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); - - virBufferAsprintf(&boot_buf, - "reboot-timeout=%d", - def->os.bios.rt_delay); - } - - if (boot_nparams > 0) { - virCommandAddArg(cmd, "-boot"); - - if (boot_nparams < 2 || emitBootindex) { - virCommandAddArgBuffer(cmd, &boot_buf); - } else { - virCommandAddArgFormat(cmd, - "order=%s", - virBufferContentAndReset(&boot_buf)); - } - } - - 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 { - virCommandAddArgList(cmd, "-bootloader", def->os.bootloader, NULL); - } - - for (i = 0 ; i < def->ndisks ; i++) { - virDomainDiskDefPtr disk = def->disks[i]; - - if (disk->driverName != NULL && - !STREQ(disk->driverName, "qemu")) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported driver name '%s' for disk '%s'"), - disk->driverName, disk->src); - goto error; - } - } - - if (qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) { - for (i = 0; i < def->ncontrollers; i++) { - virDomainControllerDefPtr cont = def->controllers[i]; - - if (cont->type != contOrder[j]) - continue; - - /* Also, skip USB controllers with type none.*/ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && - cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) { - usbcontroller = -1; /* mark we don't want a controller */ - continue; - } - - /* Only recent QEMU implements a SATA (AHCI) controller */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) { - if (!qemuCapsGet(caps, QEMU_CAPS_ICH9_AHCI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("SATA is not supported with this " - "QEMU binary")); - goto error; - } else { - char *devstr; - - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildControllerDevStr(def, cont, - caps, NULL))) - goto error; - - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && - cont->model == -1 && - !qemuCapsGet(caps, QEMU_CAPS_PIIX3_USB_UHCI)) { - if (usblegacy) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Multiple legacy USB controllers are " - "not supported")); - goto error; - } - usblegacy = true; - } else { - virCommandAddArg(cmd, "-device"); - - char *devstr; - if (!(devstr = qemuBuildControllerDevStr(def, cont, caps, - &usbcontroller))) - goto error; - - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - } - } - } - - if (usbcontroller == 0) - virCommandAddArg(cmd, "-usb"); - - for (i = 0 ; i < def->nhubs ; i++) { - virDomainHubDefPtr hub = def->hubs[i]; - char *optstr; - - virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildHubDevStr(hub, caps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } - - /* If QEMU supports -drive param instead of old -hda, -hdb, -cdrom .. */ - if (qemuCapsGet(caps, QEMU_CAPS_DRIVE)) { - int bootCD = 0, bootFloppy = 0, bootDisk = 0; - - if ((qemuCapsGet(caps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex)) { - /* bootDevs will get translated into either bootindex=N or boot=on - * depending on what qemu supports */ - for (i = 0 ; i < def->os.nBootDevs ; i++) { - switch (def->os.bootDevs[i]) { - case VIR_DOMAIN_BOOT_CDROM: - bootCD = i + 1; - break; - case VIR_DOMAIN_BOOT_FLOPPY: - bootFloppy = i + 1; - break; - case VIR_DOMAIN_BOOT_DISK: - bootDisk = i + 1; - break; - } - } - } - - for (i = 0 ; i < def->ndisks ; i++) { - char *optstr; - int bootindex = 0; - virDomainDiskDefPtr disk = def->disks[i]; - int withDeviceArg = 0; - bool deviceFlagMasked = false; - - /* Unless we have -device, then USB disks need special - handling */ - if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && - !qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - virCommandAddArg(cmd, "-usbdevice"); - virCommandAddArgFormat(cmd, "disk:%s", disk->src); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported usb disk type for '%s'"), - disk->src); - goto error; - } - continue; - } - - switch (disk->device) { - case VIR_DOMAIN_DISK_DEVICE_CDROM: - bootindex = bootCD; - bootCD = 0; - break; - case VIR_DOMAIN_DISK_DEVICE_FLOPPY: - bootindex = bootFloppy; - bootFloppy = 0; - break; - case VIR_DOMAIN_DISK_DEVICE_DISK: - case VIR_DOMAIN_DISK_DEVICE_LUN: - bootindex = bootDisk; - bootDisk = 0; - break; - } - - virCommandAddArg(cmd, "-drive"); - - /* 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 */ - if (qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - if (disk->bus != VIR_DOMAIN_DISK_BUS_XEN) { - withDeviceArg = 1; - } else { - qemuCapsClear(caps, QEMU_CAPS_DEVICE); - deviceFlagMasked = true; + break; + default: + boot[i] = 'c'; + break; } } - optstr = qemuBuildDriveStr(conn, disk, - emitBootindex ? false : !!bootindex, - caps); - if (deviceFlagMasked) - qemuCapsSet(caps, QEMU_CAPS_DEVICE); - if (!optstr) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - - if (!emitBootindex) - bootindex = 0; - else if (disk->info.bootIndex) - bootindex = disk->info.bootIndex; + boot[def->os.nBootDevs] = '\0'; - if (withDeviceArg) { - if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "isa-fdc.drive%c=drive-%s", - disk->info.addr.drive.unit - ? 'B' : 'A', - disk->info.alias); + virBufferAsprintf(&boot_buf, "%s", boot); + boot_nparams++; + } - if (bootindex) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "isa-fdc.bootindex%c=%d", - disk->info.addr.drive.unit - ? 'B' : 'A', - bootindex); - } - } else { - virCommandAddArg(cmd, "-device"); + if (def->os.bootmenu) { + if (qemuCapsGet(caps, QEMU_CAPS_BOOT_MENU)) { + if (boot_nparams++) + virBufferAddChar(&boot_buf, ','); - if (!(optstr = qemuBuildDriveDevStr(def, disk, bootindex, - caps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } + if (def->os.bootmenu == VIR_DOMAIN_BOOT_MENU_ENABLED) + virBufferAsprintf(&boot_buf, "menu=on"); + else + virBufferAsprintf(&boot_buf, "menu=off"); + } else { + /* We cannot emit an error when bootmenu is enabled but + * unsupported because of backward compatibility */ + VIR_WARN("bootmenu is enabled but not " + "supported by this QEMU binary"); } } - } else { - for (i = 0 ; i < def->ndisks ; i++) { - char dev[NAME_MAX]; - char *file; - const char *fmt; - virDomainDiskDefPtr disk = def->disks[i]; - if ((disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) && - (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { + if (def->os.bios.rt_set) { + if (!qemuCapsGet(caps, QEMU_CAPS_REBOOT_TIMEOUT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("tray status 'open' is invalid for " - "block type disk")); + _("reboot timeout is not supported " + "by this QEMU binary")); goto error; } - if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - virCommandAddArg(cmd, "-usbdevice"); - virCommandAddArgFormat(cmd, "disk:%s", disk->src); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported usb disk type for '%s'"), - disk->src); - goto error; - } - continue; - } + if (boot_nparams++) + virBufferAddChar(&boot_buf, ','); - if (STREQ(disk->dst, "hdc") && - disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - if (disk->src) { - snprintf(dev, NAME_MAX, "-%s", "cdrom"); - } else { - continue; - } - } else { - if (STRPREFIX(disk->dst, "hd") || - STRPREFIX(disk->dst, "fd")) { - snprintf(dev, NAME_MAX, "-%s", disk->dst); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported disk type '%s'"), disk->dst); - goto error; - } - } + virBufferAsprintf(&boot_buf, + "reboot-timeout=%d", + def->os.bios.rt_delay); + } - if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { - /* QEMU only supports magic FAT format for now */ - if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported disk driver type for '%s'"), - virStorageFileFormatTypeToString(disk->format)); - goto error; - } - if (!disk->readonly) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create virtual FAT disks in read-write mode")); - goto error; - } - if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - fmt = "fat:floppy:%s"; - else - fmt = "fat:%s"; + if (boot_nparams > 0) { + virCommandAddArg(cmd, "-boot"); - if (virAsprintf(&file, fmt, disk->src) < 0) { - goto no_memory; - } - } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { - switch (disk->protocol) { - case VIR_DOMAIN_DISK_PROTOCOL_NBD: - if (disk->nhosts != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("NBD accepts only one host")); - goto error; - } - if (virAsprintf(&file, "nbd:%s:%s,", disk->hosts->name, - disk->hosts->port) < 0) { - goto no_memory; - } - break; - case VIR_DOMAIN_DISK_PROTOCOL_RBD: - { - virBuffer opt = VIR_BUFFER_INITIALIZER; - if (qemuBuildRBDString(conn, disk, &opt) < 0) - goto error; - if (virBufferError(&opt)) { - virReportOOMError(); - goto error; - } - file = virBufferContentAndReset(&opt); - } - break; - case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: - if (disk->nhosts == 0) { - if (virAsprintf(&file, "sheepdog:%s,", disk->src) < 0) { - goto no_memory; - } - } else { - /* only one host is supported now */ - if (virAsprintf(&file, "sheepdog:%s:%s:%s,", - disk->hosts->name, disk->hosts->port, - disk->src) < 0) { - goto no_memory; - } - } - break; - } + if (boot_nparams < 2 || emitBootindex) { + virCommandAddArgBuffer(cmd, &boot_buf); } else { - if (!(file = strdup(disk->src))) { - goto no_memory; - } + virCommandAddArgFormat(cmd, + "order=%s", + virBufferContentAndReset(&boot_buf)); } - - /* Don't start with source if the tray is open for - * CDROM and Floppy device. - */ - if (!((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || - disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && - disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) - virCommandAddArgList(cmd, dev, file, NULL); - VIR_FREE(file); } - } - - if (qemuCapsGet(caps, QEMU_CAPS_FSDEV)) { - for (i = 0 ; i < def->nfss ; i++) { - char *optstr; - virDomainFSDefPtr fs = def->fss[i]; - - virCommandAddArg(cmd, "-fsdev"); - if (!(optstr = qemuBuildFSStr(fs, caps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildFSDevStr(fs, caps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } + 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 { - if (def->nfss) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("filesystem passthrough not supported by this QEMU")); - goto error; - } + virCommandAddArgList(cmd, "-bootloader", def->os.bootloader, NULL); } - if (!def->nnets) { - /* If we have -device, then we set -nodefault already */ - if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-net", "none", NULL); - } else { - int bootNet = 0; + for (i = 0 ; i < def->ndisks ; i++) { + virDomainDiskDefPtr disk = def->disks[i]; - if (emitBootindex) { - /* convert <boot dev='network'/> to bootindex since we didn't emit - * -boot n - */ - for (i = 0 ; i < def->os.nBootDevs ; i++) { - if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_NET) { - bootNet = i + 1; - break; - } - } + if (disk->driverName != NULL && + !STREQ(disk->driverName, "qemu")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported driver name '%s' for disk '%s'"), + disk->driverName, disk->src); + goto error; } + } - for (i = 0 ; i < def->nnets ; i++) { - virDomainNetDefPtr net = def->nets[i]; - char *nic, *host; - char tapfd_name[50] = ""; - char vhostfd_name[50] = ""; - int vlan; - int bootindex = bootNet; - int actualType; - - bootNet = 0; - if (!bootindex) - bootindex = net->info.bootIndex; + if (qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { + for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) { + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; - /* VLANs are not used with -netdev, so don't record them */ - if (qemuCapsGet(caps, QEMU_CAPS_NETDEV) && - qemuCapsGet(caps, QEMU_CAPS_DEVICE)) - vlan = -1; - else - vlan = i; + if (cont->type != contOrder[j]) + continue; - /* If appropriate, grab a physical device from the configured - * network's pool of devices, or resolve bridge device name - * to the one defined in the network definition. - */ - if (networkAllocateActualDevice(net) < 0) - goto error; + /* Also, skip USB controllers with type none.*/ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) { + usbcontroller = -1; /* mark we don't want a controller */ + continue; + } - actualType = virDomainNetGetActualType(net); - if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { - if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { - virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); - virDomainHostdevDefPtr found; - /* For a network with <forward mode='hostdev'>, there is a need to - * add the newly minted hostdev to the hostdevs array. - */ - if (qemuAssignDeviceHostdevAlias(def, hostdev, - (def->nhostdevs-1)) < 0) { + /* Only recent QEMU implements a SATA (AHCI) controller */ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) { + if (!qemuCapsGet(caps, QEMU_CAPS_ICH9_AHCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("SATA is not supported with this " + "QEMU binary")); goto error; - } + } else { + char *devstr; - if (virDomainHostdevFind(def, hostdev, &found) < 0) { - if (virDomainHostdevInsert(def, hostdev) < 0) { - virReportOOMError(); - goto error; - } - if (qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid, - &hostdev, 1) < 0) { + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildControllerDevStr(def, cont, + caps, NULL))) goto error; - } + + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } - else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("PCI device %04x:%02x:%02x.%x " - "allocated from network %s is already " - "in use by domain %s"), - hostdev->source.subsys.u.pci.domain, - hostdev->source.subsys.u.pci.bus, - hostdev->source.subsys.u.pci.slot, - hostdev->source.subsys.u.pci.function, - net->data.network.name, - def->name); + } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && + cont->model == -1 && + !qemuCapsGet(caps, QEMU_CAPS_PIIX3_USB_UHCI)) { + if (usblegacy) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Multiple legacy USB controllers are " + "not supported")); goto error; } + usblegacy = true; + } else { + virCommandAddArg(cmd, "-device"); + + char *devstr; + if (!(devstr = qemuBuildControllerDevStr(def, cont, caps, + &usbcontroller))) + goto error; + + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } - continue; } + } + } - if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { - /* - * If type='bridge' then we attempt to allocate the tap fd here only if - * running under a privilged user or -netdev bridge option is not - * supported. - */ - if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - driver->privileged || - (!qemuCapsGet(caps, QEMU_CAPS_NETDEV_BRIDGE))) { - int tapfd = qemuNetworkIfaceConnect(def, conn, driver, net, - caps); - if (tapfd < 0) - goto error; + if (usbcontroller == 0) + virCommandAddArg(cmd, "-usb"); - last_good_net = i; - virCommandTransferFD(cmd, tapfd); + for (i = 0 ; i < def->nhubs ; i++) { + virDomainHubDefPtr hub = def->hubs[i]; + char *optstr; - if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", - tapfd) >= sizeof(tapfd_name)) - goto no_memory; - } - } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { - int tapfd = qemuPhysIfaceConnect(def, driver, net, - caps, vmop); - if (tapfd < 0) - goto error; + virCommandAddArg(cmd, "-device"); + if (!(optstr = qemuBuildHubDevStr(hub, caps))) + goto error; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } - last_good_net = i; - virCommandTransferFD(cmd, tapfd); + /* If QEMU supports -drive param instead of old -hda, -hdb, -cdrom .. */ + if (qemuCapsGet(caps, QEMU_CAPS_DRIVE)) { + int bootCD = 0, bootFloppy = 0, bootDisk = 0; - if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", - tapfd) >= sizeof(tapfd_name)) - goto no_memory; + if ((qemuCapsGet(caps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex)) { + /* bootDevs will get translated into either bootindex=N or boot=on + * depending on what qemu supports */ + for (i = 0 ; i < def->os.nBootDevs ; i++) { + switch (def->os.bootDevs[i]) { + case VIR_DOMAIN_BOOT_CDROM: + bootCD = i + 1; + break; + case VIR_DOMAIN_BOOT_FLOPPY: + bootFloppy = i + 1; + break; + case VIR_DOMAIN_BOOT_DISK: + bootDisk = i + 1; + break; + } } + } - if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || - actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { - /* Attempt to use vhost-net mode for these types of - network device */ - int vhostfd; + for (i = 0 ; i < def->ndisks ; i++) { + char *optstr; + int bootindex = 0; + virDomainDiskDefPtr disk = def->disks[i]; + int withDeviceArg = 0; + bool deviceFlagMasked = false; - if (qemuOpenVhostNet(def, net, caps, &vhostfd) < 0) + /* Unless we have -device, then USB disks need special + handling */ + if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && + !qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + virCommandAddArg(cmd, "-usbdevice"); + virCommandAddArgFormat(cmd, "disk:%s", disk->src); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported usb disk type for '%s'"), + disk->src); goto error; - if (vhostfd >= 0) { - virCommandTransferFD(cmd, vhostfd); - - if (snprintf(vhostfd_name, sizeof(vhostfd_name), "%d", - vhostfd) >= sizeof(vhostfd_name)) - goto no_memory; } + continue; } - /* Possible combinations: - * - * 1. Old way: -net nic,model=e1000,vlan=1 -net tap,vlan=1 - * 2. Semi-new: -device e1000,vlan=1 -net tap,vlan=1 - * 3. Best way: -netdev type=tap,id=netdev1 -device e1000,id=netdev1 - * - * NB, no support for -netdev without use of -device - */ - if (qemuCapsGet(caps, QEMU_CAPS_NETDEV) && - qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-netdev"); - if (!(host = qemuBuildHostNetStr(net, driver, caps, - ',', vlan, tapfd_name, - vhostfd_name))) - goto error; - virCommandAddArg(cmd, host); - VIR_FREE(host); + + switch (disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + bootindex = bootCD; + bootCD = 0; + break; + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + bootindex = bootFloppy; + bootFloppy = 0; + break; + case VIR_DOMAIN_DISK_DEVICE_DISK: + case VIR_DOMAIN_DISK_DEVICE_LUN: + bootindex = bootDisk; + bootDisk = 0; + break; } + + virCommandAddArg(cmd, "-drive"); + + /* 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 */ if (qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-device"); - nic = qemuBuildNicDevStr(net, vlan, bootindex, caps); - if (!nic) - goto error; - virCommandAddArg(cmd, nic); - VIR_FREE(nic); - } else { - virCommandAddArg(cmd, "-net"); - if (!(nic = qemuBuildNicStr(net, "nic,", vlan))) - goto error; - virCommandAddArg(cmd, nic); - VIR_FREE(nic); - } - if (!(qemuCapsGet(caps, QEMU_CAPS_NETDEV) && - qemuCapsGet(caps, QEMU_CAPS_DEVICE))) { - virCommandAddArg(cmd, "-net"); - if (!(host = qemuBuildHostNetStr(net, driver, caps, - ',', vlan, tapfd_name, - vhostfd_name))) - goto error; - virCommandAddArg(cmd, host); - VIR_FREE(host); + if (disk->bus != VIR_DOMAIN_DISK_BUS_XEN) { + withDeviceArg = 1; + } else { + qemuCapsClear(caps, QEMU_CAPS_DEVICE); + deviceFlagMasked = true; + } } - } - } + optstr = qemuBuildDriveStr(conn, disk, + emitBootindex ? false : !!bootindex, + caps); + if (deviceFlagMasked) + qemuCapsSet(caps, QEMU_CAPS_DEVICE); + if (!optstr) + goto error; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + + if (!emitBootindex) + bootindex = 0; + else if (disk->info.bootIndex) + bootindex = disk->info.bootIndex; - if (def->nsmartcards) { - /* -device usb-ccid was already emitted along with other - * controllers. For now, qemu handles only one smartcard. */ - virDomainSmartcardDefPtr smartcard = def->smartcards[0]; - char *devstr; - virBuffer opt = VIR_BUFFER_INITIALIZER; - const char *database; + if (withDeviceArg) { + if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "isa-fdc.drive%c=drive-%s", + disk->info.addr.drive.unit + ? 'B' : 'A', + disk->info.alias); - if (def->nsmartcards > 1 || - smartcard->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID || - smartcard->info.addr.ccid.controller != 0 || - smartcard->info.addr.ccid.slot != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks multiple smartcard " - "support")); - virBufferFreeAndReset(&opt); - goto error; - } + if (bootindex) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "isa-fdc.bootindex%c=%d", + disk->info.addr.drive.unit + ? 'B' : 'A', + bootindex); + } + } else { + virCommandAddArg(cmd, "-device"); - switch (smartcard->type) { - case VIR_DOMAIN_SMARTCARD_TYPE_HOST: - if (!qemuCapsGet(caps, QEMU_CAPS_CHARDEV) || - !qemuCapsGet(caps, QEMU_CAPS_CCID_EMULATED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); - goto error; + if (!(optstr = qemuBuildDriveDevStr(def, disk, bootindex, + caps))) + goto error; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } } + } + } else { + for (i = 0 ; i < def->ndisks ; i++) { + char dev[NAME_MAX]; + char *file; + const char *fmt; + virDomainDiskDefPtr disk = def->disks[i]; - virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated"); - break; - - case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: - if (!qemuCapsGet(caps, QEMU_CAPS_CHARDEV) || - !qemuCapsGet(caps, QEMU_CAPS_CCID_EMULATED)) { + if ((disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK) && + (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); + _("tray status 'open' is invalid for " + "block type disk")); goto error; } - virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates"); - for (j = 0; j < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; j++) { - if (strchr(smartcard->data.cert.file[j], ',')) { - virBufferFreeAndReset(&opt); - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid certificate name: %s"), - smartcard->data.cert.file[j]); + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + virCommandAddArg(cmd, "-usbdevice"); + virCommandAddArgFormat(cmd, "disk:%s", disk->src); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported usb disk type for '%s'"), + disk->src); goto error; } - virBufferAsprintf(&opt, ",cert%d=%s", j + 1, - smartcard->data.cert.file[j]); + continue; } - if (smartcard->data.cert.database) { - if (strchr(smartcard->data.cert.database, ',')) { - virBufferFreeAndReset(&opt); - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid database name: %s"), - smartcard->data.cert.database); - goto error; + + if (STREQ(disk->dst, "hdc") && + disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { + if (disk->src) { + snprintf(dev, NAME_MAX, "-%s", "cdrom"); + } else { + continue; } - database = smartcard->data.cert.database; } else { - database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; + if (STRPREFIX(disk->dst, "hd") || + STRPREFIX(disk->dst, "fd")) { + snprintf(dev, NAME_MAX, "-%s", disk->dst); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported disk type '%s'"), disk->dst); + goto error; + } } - virBufferAsprintf(&opt, ",database=%s", database); - break; - case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - if (!qemuCapsGet(caps, QEMU_CAPS_CHARDEV) || - !qemuCapsGet(caps, QEMU_CAPS_CCID_PASSTHRU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard " - "passthrough mode support")); - goto error; - } + if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { + /* QEMU only supports magic FAT format for now */ + if (disk->format > 0 && disk->format != VIR_STORAGE_FILE_FAT) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported disk driver type for '%s'"), + virStorageFileFormatTypeToString(disk->format)); + goto error; + } + if (!disk->readonly) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot create virtual FAT disks in read-write mode")); + goto error; + } + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + fmt = "fat:floppy:%s"; + else + fmt = "fat:%s"; - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, - smartcard->info.alias, - caps))) { - virBufferFreeAndReset(&opt); - goto error; + if (virAsprintf(&file, fmt, disk->src) < 0) { + goto no_memory; + } + } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { + switch (disk->protocol) { + case VIR_DOMAIN_DISK_PROTOCOL_NBD: + if (disk->nhosts != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NBD accepts only one host")); + goto error; + } + if (virAsprintf(&file, "nbd:%s:%s,", disk->hosts->name, + disk->hosts->port) < 0) { + goto no_memory; + } + break; + case VIR_DOMAIN_DISK_PROTOCOL_RBD: + { + virBuffer opt = VIR_BUFFER_INITIALIZER; + if (qemuBuildRBDString(conn, disk, &opt) < 0) + goto error; + if (virBufferError(&opt)) { + virReportOOMError(); + goto error; + } + file = virBufferContentAndReset(&opt); + } + break; + case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: + if (disk->nhosts == 0) { + if (virAsprintf(&file, "sheepdog:%s,", disk->src) < 0) { + goto no_memory; + } + } else { + /* only one host is supported now */ + if (virAsprintf(&file, "sheepdog:%s:%s:%s,", + disk->hosts->name, disk->hosts->port, + disk->src) < 0) { + goto no_memory; + } + } + break; + } + } else { + if (!(file = strdup(disk->src))) { + goto no_memory; + } } - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - virBufferAsprintf(&opt, "ccid-card-passthru,chardev=char%s", - smartcard->info.alias); - break; - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected smartcard type %d"), - smartcard->type); - virBufferFreeAndReset(&opt); - goto error; + /* Don't start with source if the tray is open for + * CDROM and Floppy device. + */ + if (!((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || + disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && + disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) + virCommandAddArgList(cmd, dev, file, NULL); + VIR_FREE(file); } - virCommandAddArg(cmd, "-device"); - virBufferAsprintf(&opt, ",id=%s,bus=ccid0.0", smartcard->info.alias); - virCommandAddArgBuffer(cmd, &opt); } - if (!def->nserials) { - /* If we have -device, then we set -nodefault already */ - if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-serial", "none", NULL); - } else { - for (i = 0 ; i < def->nserials ; i++) { - virDomainChrDefPtr serial = def->serials[i]; - char *devstr; - - /* Use -chardev with -device if they are available */ - if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV) && - qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&serial->source, - serial->info.alias, - caps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); + if (qemuCapsGet(caps, QEMU_CAPS_FSDEV)) { + for (i = 0 ; i < def->nfss ; i++) { + char *optstr; + virDomainFSDefPtr fs = def->fss[i]; - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildChrDeviceStr(serial, caps, - def->os.arch, - def->os.machine))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } else { - virCommandAddArg(cmd, "-serial"); - if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } + virCommandAddArg(cmd, "-fsdev"); + if (!(optstr = qemuBuildFSStr(fs, caps))) + goto error; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + + virCommandAddArg(cmd, "-device"); + if (!(optstr = qemuBuildFSDevStr(fs, caps))) + goto error; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } + } else { + if (def->nfss) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("filesystem passthrough not supported by this QEMU")); + goto error; } } - if (!def->nparallels) { + if (!def->nnets) { /* If we have -device, then we set -nodefault already */ if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-parallel", "none", NULL); + virCommandAddArgList(cmd, "-net", "none", NULL); } else { - for (i = 0 ; i < def->nparallels ; i++) { - virDomainChrDefPtr parallel = def->parallels[i]; - char *devstr; - - /* Use -chardev with -device if they are available */ - if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV) && - qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(¶llel->source, - parallel->info.alias, - caps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); + int bootNet = 0; - virCommandAddArg(cmd, "-device"); - virCommandAddArgFormat(cmd, "isa-parallel,chardev=char%s,id=%s", - parallel->info.alias, - parallel->info.alias); - } else { - virCommandAddArg(cmd, "-parallel"); - if (!(devstr = qemuBuildChrArgStr(¶llel->source, NULL))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); + if (emitBootindex) { + /* convert <boot dev='network'/> to bootindex since we didn't emit + * -boot n + */ + for (i = 0 ; i < def->os.nBootDevs ; i++) { + if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_NET) { + bootNet = i + 1; + break; + } } } - } - - for (i = 0 ; i < def->nchannels ; i++) { - virDomainChrDefPtr channel = def->channels[i]; - char *devstr; - char *addr; - int port; - switch (channel->targetType) { - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: - if (!qemuCapsGet(caps, QEMU_CAPS_CHARDEV) || - !qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("guestfwd requires QEMU to support -chardev & -device")); - goto error; - } + for (i = 0 ; i < def->nnets ; i++) { + virDomainNetDefPtr net = def->nets[i]; + char *nic, *host; + char tapfd_name[50] = ""; + char vhostfd_name[50] = ""; + int vlan; + int bootindex = bootNet; + int actualType; - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, - channel->info.alias, - caps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); + bootNet = 0; + if (!bootindex) + bootindex = net->info.bootIndex; - addr = virSocketAddrFormat(channel->target.addr); - if (!addr) - goto error; - port = virSocketAddrGetPort(channel->target.addr); + /* VLANs are not used with -netdev, so don't record them */ + if (qemuCapsGet(caps, QEMU_CAPS_NETDEV) && + qemuCapsGet(caps, QEMU_CAPS_DEVICE)) + vlan = -1; + else + vlan = i; - virCommandAddArg(cmd, "-netdev"); - virCommandAddArgFormat(cmd, - "user,guestfwd=tcp:%s:%i,chardev=char%s,id=user-%s", - addr, port, channel->info.alias, - channel->info.alias); - VIR_FREE(addr); - break; + /* If appropriate, grab a physical device from the configured + * network's pool of devices, or resolve bridge device name + * to the one defined in the network definition. + */ + if (networkAllocateActualDevice(net) < 0) + goto error; - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: - if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio channel requires QEMU to support -device")); - goto error; - } + actualType = virDomainNetGetActualType(net); + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); + virDomainHostdevDefPtr found; + /* For a network with <forward mode='hostdev'>, there is a need to + * add the newly minted hostdev to the hostdevs array. + */ + if (qemuAssignDeviceHostdevAlias(def, hostdev, + (def->nhostdevs-1)) < 0) { + goto error; + } - if (qemuCapsGet(caps, QEMU_CAPS_DEVICE_SPICEVMC) && - channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { - /* spicevmc was originally introduced via a -device - * with a backend internal to qemu; although we prefer - * the newer -chardev interface. */ - ; - } else { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, - channel->info.alias, - caps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); + if (virDomainHostdevFind(def, hostdev, &found) < 0) { + if (virDomainHostdevInsert(def, hostdev) < 0) { + virReportOOMError(); + goto error; + } + if (qemuPrepareHostdevPCIDevices(driver, def->name, def->uuid, + &hostdev, 1) < 0) { + goto error; + } + } + else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("PCI device %04x:%02x:%02x.%x " + "allocated from network %s is already " + "in use by domain %s"), + hostdev->source.subsys.u.pci.domain, + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function, + net->data.network.name, + def->name); + goto error; + } + } + continue; } - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildVirtioSerialPortDevStr(channel, - caps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - break; - } - } - - /* Explicit console devices */ - for (i = 0 ; i < def->nconsoles ; i++) { - virDomainChrDefPtr console = def->consoles[i]; - char *devstr; + if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { + /* + * If type='bridge' then we attempt to allocate the tap fd here only if + * running under a privilged user or -netdev bridge option is not + * supported. + */ + if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + driver->privileged || + (!qemuCapsGet(caps, QEMU_CAPS_NETDEV_BRIDGE))) { + int tapfd = qemuNetworkIfaceConnect(def, conn, driver, net, + caps); + if (tapfd < 0) + goto error; - switch (console->targetType) { - case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: - if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio channel requires QEMU to support -device")); - goto error; - } + last_good_net = i; + virCommandTransferFD(cmd, tapfd); - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&console->source, - console->info.alias, - caps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); + if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", + tapfd) >= sizeof(tapfd_name)) + goto no_memory; + } + } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + int tapfd = qemuPhysIfaceConnect(def, driver, net, + caps, vmop); + if (tapfd < 0) + goto error; - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildVirtioSerialPortDevStr(console, - caps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - break; + last_good_net = i; + virCommandTransferFD(cmd, tapfd); - case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: - break; + if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", + tapfd) >= sizeof(tapfd_name)) + goto no_memory; + } - default: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported console target type %s"), - NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType))); - goto error; - } - } + if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + /* Attempt to use vhost-net mode for these types of + network device */ + int vhostfd; - for (i = 0 ; i < def->ninputs ; i++) { - virDomainInputDefPtr input = def->inputs[i]; + if (qemuOpenVhostNet(def, net, caps, &vhostfd) < 0) + goto error; + if (vhostfd >= 0) { + virCommandTransferFD(cmd, vhostfd); - if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { + if (snprintf(vhostfd_name, sizeof(vhostfd_name), "%d", + vhostfd) >= sizeof(vhostfd_name)) + goto no_memory; + } + } + /* Possible combinations: + * + * 1. Old way: -net nic,model=e1000,vlan=1 -net tap,vlan=1 + * 2. Semi-new: -device e1000,vlan=1 -net tap,vlan=1 + * 3. Best way: -netdev type=tap,id=netdev1 -device e1000,id=netdev1 + * + * NB, no support for -netdev without use of -device + */ + if (qemuCapsGet(caps, QEMU_CAPS_NETDEV) && + qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { + virCommandAddArg(cmd, "-netdev"); + if (!(host = qemuBuildHostNetStr(net, driver, caps, + ',', vlan, tapfd_name, + vhostfd_name))) + goto error; + virCommandAddArg(cmd, host); + VIR_FREE(host); + } if (qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - char *optstr; virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildUSBInputDevStr(input, caps))) + nic = qemuBuildNicDevStr(net, vlan, bootindex, caps); + if (!nic) goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); + virCommandAddArg(cmd, nic); + VIR_FREE(nic); } else { - virCommandAddArgList(cmd, "-usbdevice", - input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE - ? "mouse" : "tablet", NULL); + virCommandAddArg(cmd, "-net"); + if (!(nic = qemuBuildNicStr(net, "nic,", vlan))) + goto error; + virCommandAddArg(cmd, nic); + VIR_FREE(nic); + } + if (!(qemuCapsGet(caps, QEMU_CAPS_NETDEV) && + qemuCapsGet(caps, QEMU_CAPS_DEVICE))) { + virCommandAddArg(cmd, "-net"); + if (!(host = qemuBuildHostNetStr(net, driver, caps, + ',', vlan, tapfd_name, + vhostfd_name))) + goto error; + virCommandAddArg(cmd, host); + VIR_FREE(host); } } } - if (def->ngraphics > 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("only 1 graphics device is supported")); - goto error; - } - - if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + if (def->nsmartcards) { + /* -device usb-ccid was already emitted along with other + * controllers. For now, qemu handles only one smartcard. */ + virDomainSmartcardDefPtr smartcard = def->smartcards[0]; + char *devstr; virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *database; - if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) { + if (def->nsmartcards > 1 || + smartcard->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID || + smartcard->info.addr.ccid.controller != 0 || + smartcard->info.addr.ccid.slot != 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("vnc graphics are not supported with this QEMU")); + _("this QEMU binary lacks multiple smartcard " + "support")); + virBufferFreeAndReset(&opt); goto error; } - if (def->graphics[0]->data.vnc.socket || - driver->vncAutoUnixSocket) { - - if (!def->graphics[0]->data.vnc.socket && - virAsprintf(&def->graphics[0]->data.vnc.socket, - "%s/%s.vnc", driver->libDir, def->name) == -1) { - goto no_memory; + switch (smartcard->type) { + case VIR_DOMAIN_SMARTCARD_TYPE_HOST: + if (!qemuCapsGet(caps, QEMU_CAPS_CHARDEV) || + !qemuCapsGet(caps, QEMU_CAPS_CCID_EMULATED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard host " + "mode support")); + goto error; } - virBufferAsprintf(&opt, "unix:%s", - def->graphics[0]->data.vnc.socket); - - } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { - const char *listenNetwork; - const char *listenAddr = NULL; - char *netAddr = NULL; - bool escapeAddr; - int ret; + virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated"); + break; - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); - break; + case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: + if (!qemuCapsGet(caps, QEMU_CAPS_CHARDEV) || + !qemuCapsGet(caps, QEMU_CAPS_CCID_EMULATED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard host " + "mode support")); + goto error; + } - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { + virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates"); + for (j = 0; j < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; j++) { + if (strchr(smartcard->data.cert.file[j], ',')) { + virBufferFreeAndReset(&opt); virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); + _("invalid certificate name: %s"), + smartcard->data.cert.file[j]); goto error; } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); + virBufferAsprintf(&opt, ",cert%d=%s", j + 1, + smartcard->data.cert.file[j]); + } + if (smartcard->data.cert.database) { + if (strchr(smartcard->data.cert.database, ',')) { + virBufferFreeAndReset(&opt); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid database name: %s"), + smartcard->data.cert.database); goto error; } - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0, - listenAddr, -1, false) < 0) - goto error; - break; + database = smartcard->data.cert.database; + } else { + database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; } + virBufferAsprintf(&opt, ",database=%s", database); + break; - if (!listenAddr) - listenAddr = driver->vncListen; - - escapeAddr = strchr(listenAddr, ':') != NULL; - if (escapeAddr) - virBufferAsprintf(&opt, "[%s]", listenAddr); - else - virBufferAdd(&opt, listenAddr, -1); - virBufferAsprintf(&opt, ":%d", - def->graphics[0]->data.vnc.port - 5900); - - VIR_FREE(netAddr); - } else { - virBufferAsprintf(&opt, "%d", - def->graphics[0]->data.vnc.port - 5900); - } - - if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { - if (def->graphics[0]->data.vnc.auth.passwd || - driver->vncPassword) - virBufferAddLit(&opt, ",password"); - - if (driver->vncTLS) { - virBufferAddLit(&opt, ",tls"); - if (driver->vncTLSx509verify) { - virBufferAsprintf(&opt, ",x509verify=%s", - driver->vncTLSx509certdir); - } else { - virBufferAsprintf(&opt, ",x509=%s", - driver->vncTLSx509certdir); - } + case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: + if (!qemuCapsGet(caps, QEMU_CAPS_CHARDEV) || + !qemuCapsGet(caps, QEMU_CAPS_CCID_PASSTHRU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard " + "passthrough mode support")); + goto error; } - if (driver->vncSASL) { - virBufferAddLit(&opt, ",sasl"); - - if (driver->vncSASLdir) - virCommandAddEnvPair(cmd, "SASL_CONF_DIR", - driver->vncSASLdir); - - /* TODO: Support ACLs later */ + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, + smartcard->info.alias, + caps))) { + virBufferFreeAndReset(&opt); + goto error; } - } + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); - virCommandAddArg(cmd, "-vnc"); - virCommandAddArgBuffer(cmd, &opt); - if (def->graphics[0]->data.vnc.keymap) { - virCommandAddArgList(cmd, "-k", def->graphics[0]->data.vnc.keymap, - NULL); - } + virBufferAsprintf(&opt, "ccid-card-passthru,chardev=char%s", + smartcard->info.alias); + break; - /* Unless user requested it, set the audio backend to none, to - * prevent it opening the host OS audio devices, since that causes - * security issues and might not work when using VNC. - */ - if (driver->vncAllowHostAudio) { - virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); - } else { - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - } - } else if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { - if (qemuCapsGet(caps, QEMU_CAPS_0_10) && - !qemuCapsGet(caps, QEMU_CAPS_SDL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("sdl not supported by '%s'"), - def->emulator); + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected smartcard type %d"), + smartcard->type); + virBufferFreeAndReset(&opt); goto error; } + virCommandAddArg(cmd, "-device"); + virBufferAsprintf(&opt, ",id=%s,bus=ccid0.0", smartcard->info.alias); + virCommandAddArgBuffer(cmd, &opt); + } - 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) - virCommandAddArg(cmd, "-full-screen"); + if (!def->nserials) { + /* If we have -device, then we set -nodefault already */ + if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) + virCommandAddArgList(cmd, "-serial", "none", NULL); + } else { + for (i = 0 ; i < def->nserials ; i++) { + virDomainChrDefPtr serial = def->serials[i]; + char *devstr; - /* 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 - */ - virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); - virCommandAddEnvPass(cmd, "SDL_AUDIODRIVER"); + /* Use -chardev with -device if they are available */ + if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV) && + qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&serial->source, + serial->info.alias, + caps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildChrDeviceStr(serial, caps, + def->os.arch, + def->os.machine))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } else { + virCommandAddArg(cmd, "-serial"); + if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + } + } - /* 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 (qemuCapsGet(caps, QEMU_CAPS_SDL)) - virCommandAddArg(cmd, "-sdl"); + if (!def->nparallels) { + /* If we have -device, then we set -nodefault already */ + if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) + virCommandAddArgList(cmd, "-parallel", "none", NULL); + } else { + for (i = 0 ; i < def->nparallels ; i++) { + virDomainChrDefPtr parallel = def->parallels[i]; + char *devstr; - } else if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virBuffer opt = VIR_BUFFER_INITIALIZER; - const char *listenNetwork; - const char *listenAddr = NULL; - char *netAddr = NULL; - int ret; - int defaultMode = def->graphics[0]->data.spice.defaultMode; + /* Use -chardev with -device if they are available */ + if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV) && + qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(¶llel->source, + parallel->info.alias, + caps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); - if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice graphics are not supported with this QEMU")); - goto error; + virCommandAddArg(cmd, "-device"); + virCommandAddArgFormat(cmd, "isa-parallel,chardev=char%s,id=%s", + parallel->info.alias, + parallel->info.alias); + } else { + virCommandAddArg(cmd, "-parallel"); + if (!(devstr = qemuBuildChrArgStr(¶llel->source, NULL))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } } + } - virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port); + for (i = 0 ; i < def->nchannels ; i++) { + virDomainChrDefPtr channel = def->channels[i]; + char *devstr; + char *addr; + int port; - if (def->graphics[0]->data.spice.tlsPort > 0) { - if (!driver->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice TLS port set in XML configuration," - " but TLS is disabled in qemu.conf")); + switch (channel->targetType) { + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: + if (!qemuCapsGet(caps, QEMU_CAPS_CHARDEV) || + !qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("guestfwd requires QEMU to support -chardev & -device")); goto error; } - virBufferAsprintf(&opt, ",tls-port=%u", - def->graphics[0]->data.spice.tlsPort); - } - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&channel->source, + channel->info.alias, + caps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + addr = virSocketAddrFormat(channel->target.addr); + if (!addr) + goto error; + port = virSocketAddrGetPort(channel->target.addr); + + virCommandAddArg(cmd, "-netdev"); + virCommandAddArgFormat(cmd, + "user,guestfwd=tcp:%s:%i,chardev=char%s,id=user-%s", + addr, port, channel->info.alias, + channel->info.alias); + VIR_FREE(addr); break; - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: + if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio channel requires QEMU to support -device")); goto error; } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); - goto error; + + if (qemuCapsGet(caps, QEMU_CAPS_DEVICE_SPICEVMC) && + channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { + /* spicevmc was originally introduced via a -device + * with a backend internal to qemu; although we prefer + * the newer -chardev interface. */ + ; + } else { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&channel->source, + channel->info.alias, + caps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); } - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0, - listenAddr, -1, false) < 0) - goto error; + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildVirtioSerialPortDevStr(channel, + caps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); break; } + } - if (!listenAddr) - listenAddr = driver->spiceListen; - if (listenAddr) - virBufferAsprintf(&opt, ",addr=%s", listenAddr); - - VIR_FREE(netAddr); + /* Explicit console devices */ + for (i = 0 ; i < def->nconsoles ; i++) { + virDomainChrDefPtr console = def->consoles[i]; + char *devstr; - int mm = def->graphics[0]->data.spice.mousemode; - if (mm) { - switch (mm) { - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: - virBufferAsprintf(&opt, ",agent-mouse=off"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: - virBufferAsprintf(&opt, ",agent-mouse=on"); - break; - default: - break; + switch (console->targetType) { + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: + if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio channel requires QEMU to support -device")); + goto error; } - } - - /* In the password case we set it via monitor command, to avoid - * making it visible on CLI, so there's no use of password=XXX - * in this bit of the code */ - if (!def->graphics[0]->data.spice.auth.passwd && - !driver->spicePassword) - virBufferAddLit(&opt, ",disable-ticketing"); - if (driver->spiceTLS) - virBufferAsprintf(&opt, ",x509-dir=%s", - driver->spiceTLSx509certdir); + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&console->source, + console->info.alias, + caps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); - switch (defaultMode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - virBufferAsprintf(&opt, ",tls-channel=default"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAsprintf(&opt, ",plaintext-channel=default"); + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildVirtioSerialPortDevStr(console, + caps))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: - /* nothing */ + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported console target type %s"), + NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType))); + goto error; } + } - for (i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { - int mode = def->graphics[0]->data.spice.channels[i]; - switch (mode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - if (!driver->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf")); + for (i = 0 ; i < def->ninputs ; i++) { + virDomainInputDefPtr input = def->inputs[i]; + + if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { + if (qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { + char *optstr; + virCommandAddArg(cmd, "-device"); + if (!(optstr = qemuBuildUSBInputDevStr(input, caps))) goto error; - } - virBufferAsprintf(&opt, ",tls-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAsprintf(&opt, ",plaintext-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); - break; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } else { + virCommandAddArgList(cmd, "-usbdevice", + input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE + ? "mouse" : "tablet", NULL); } } - if (def->graphics[0]->data.spice.image) - virBufferAsprintf(&opt, ",image-compression=%s", - virDomainGraphicsSpiceImageCompressionTypeToString(def->graphics[0]->data.spice.image)); - if (def->graphics[0]->data.spice.jpeg) - virBufferAsprintf(&opt, ",jpeg-wan-compression=%s", - virDomainGraphicsSpiceJpegCompressionTypeToString(def->graphics[0]->data.spice.jpeg)); - if (def->graphics[0]->data.spice.zlib) - virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s", - virDomainGraphicsSpiceZlibCompressionTypeToString(def->graphics[0]->data.spice.zlib)); - if (def->graphics[0]->data.spice.playback) - virBufferAsprintf(&opt, ",playback-compression=%s", - virDomainGraphicsSpicePlaybackCompressionTypeToString(def->graphics[0]->data.spice.playback)); - if (def->graphics[0]->data.spice.streaming) - virBufferAsprintf(&opt, ",streaming-video=%s", - virDomainGraphicsSpiceStreamingModeTypeToString(def->graphics[0]->data.spice.streaming)); - if (def->graphics[0]->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) - virBufferAddLit(&opt, ",disable-copy-paste"); - - if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) { - /* If qemu supports seamless migration turn it - * unconditionally on. If migration destination - * doesn't support it, it fallbacks to previous - * migration algorithm silently. */ - virBufferAddLit(&opt, ",seamless-migration=on"); - } - - virCommandAddArg(cmd, "-spice"); - virCommandAddArgBuffer(cmd, &opt); - 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 - */ - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); + } - } else if ((def->ngraphics == 1)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported graphics type '%s'"), - virDomainGraphicsTypeToString(def->graphics[0]->type)); + if (def->ngraphics > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("only 1 graphics device is supported")); goto error; } + for (i = 0 ; i < def->ngraphics ; ++i) { + switch (qemuBuildGraphicsCommandLine(driver, cmd, def, caps, + def->graphics[i])) { + case ERROR: + goto error; + case NO_MEMORY: + goto no_memory; + } + } if (def->nvideos > 0) { if (qemuCapsGet(caps, QEMU_CAPS_VGA)) { if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_XEN) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d8cf4c3..2a77290 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2081,16 +2081,17 @@ qemuProcessInitPasswords(virConnectPtr conn, int ret = 0; qemuDomainObjPrivatePtr priv = vm->privateData; - if (vm->def->ngraphics == 1) { - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + for (int i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { ret = qemuDomainChangeGraphicsPasswords(driver, vm, VIR_DOMAIN_GRAPHICS_TYPE_VNC, - &vm->def->graphics[0]->data.vnc.auth, + &graphics->data.vnc.auth, driver->vncPassword); - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { ret = qemuDomainChangeGraphicsPasswords(driver, vm, VIR_DOMAIN_GRAPHICS_TYPE_SPICE, - &vm->def->graphics[0]->data.spice.auth, + &graphics->data.spice.auth, driver->spicePassword); } } @@ -3484,21 +3485,22 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Ensuring no historical cgroup is lying around"); qemuRemoveCgroup(driver, vm, 1); - if (vm->def->ngraphics == 1) { - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - !vm->def->graphics[0]->data.vnc.socket && - vm->def->graphics[0]->data.vnc.autoport) { + for (i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !graphics->data.vnc.socket && + graphics->data.vnc.autoport) { int port = qemuProcessNextFreePort(driver, driver->remotePortMin); if (port < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for VNC")); goto cleanup; } - vm->def->graphics[0]->data.vnc.port = port; - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + graphics->data.vnc.port = port; + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { int port = -1; - if (vm->def->graphics[0]->data.spice.autoport || - vm->def->graphics[0]->data.spice.port == -1) { + if (graphics->data.spice.autoport || + graphics->data.spice.port == -1) { port = qemuProcessNextFreePort(driver, driver->remotePortMin); if (port < 0) { @@ -3507,13 +3509,13 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } - vm->def->graphics[0]->data.spice.port = port; + graphics->data.spice.port = port; } if (driver->spiceTLS && - (vm->def->graphics[0]->data.spice.autoport || - vm->def->graphics[0]->data.spice.tlsPort == -1)) { + (graphics->data.spice.autoport || + graphics->data.spice.tlsPort == -1)) { int tlsPort = qemuProcessNextFreePort(driver, - vm->def->graphics[0]->data.spice.port + 1); + graphics->data.spice.port + 1); if (tlsPort < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for SPICE TLS")); @@ -3521,20 +3523,19 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } - vm->def->graphics[0]->data.spice.tlsPort = tlsPort; + graphics->data.spice.tlsPort = tlsPort; } } - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virDomainGraphicsDefPtr graphics = vm->def->graphics[0]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { if (graphics->nListens == 0) { if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0) { virReportOOMError(); goto cleanup; } graphics->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) graphics->listens[0].address = strdup(driver->vncListen); else graphics->listens[0].address = strdup(driver->spiceListen); @@ -4150,19 +4151,20 @@ retry: qemuProcessRemoveDomainStatus(driver, vm); - /* Remove VNC port from port reservation bitmap, but only if it was - reserved by the driver (autoport=yes) + /* Remove VNC and Spice ports from port reservation bitmap, but only if + they were reserved by the driver (autoport=yes) */ - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - vm->def->graphics[0]->data.vnc.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.vnc.port); - } - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - vm->def->graphics[0]->data.spice.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.port); - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.tlsPort); + for (i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + graphics->data.vnc.autoport) { + qemuProcessReturnPort(driver, graphics->data.vnc.port); + } + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + graphics->data.spice.autoport) { + qemuProcessReturnPort(driver, graphics->data.spice.port); + qemuProcessReturnPort(driver, graphics->data.spice.tlsPort); + } } vm->taint = 0; -- 1.8.0

--- src/qemu/qemu_command.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f9e4d4d..fcdf60c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6186,9 +6186,26 @@ qemuBuildCommandLine(virConnectPtr conn, } if (def->ngraphics > 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("only 1 graphics device is supported")); - goto error; + int sdl = 0, vnc = 0, spice = 0; + for (i = 0 ; i < def->ngraphics ; ++i) { + switch (def->graphics[i]->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + ++sdl; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + ++vnc; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + ++spice; + break; + } + } + if (sdl > 1 || vnc > 1 || spice > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("only 1 graphics device of each type " + "(sdl, vnc, spice) is supported")); + goto error; + } } for (i = 0 ; i < def->ngraphics ; ++i) { -- 1.8.0

On Thu, Nov 08, 2012 at 09:48:55AM +0100, Alon Levy wrote:
The check for a single display remains so no new functionality is added. ---
Concerning both patches, I've tested running with all three simultaneously (sdl+spice+vnc) and with spice+vnc, using a single qxl device.
src/qemu/qemu_command.c | 2425 ++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +- 2 files changed, 1259 insertions(+), 1236 deletions(-)
I'm not really sure what has happened here, but you have got an absolutely enourmous diff here. I would expect this patch to only affect the small part of qemuBuildCommandLine that actually deals with graphics devices. As it is now, it is impossible to review this patch I'm afraid :-( Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 11/08/12 15:04, Daniel P. Berrange wrote:
On Thu, Nov 08, 2012 at 09:48:55AM +0100, Alon Levy wrote:
The check for a single display remains so no new functionality is added. ---
Concerning both patches, I've tested running with all three simultaneously (sdl+spice+vnc) and with spice+vnc, using a single qxl device.
src/qemu/qemu_command.c | 2425 ++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +- 2 files changed, 1259 insertions(+), 1236 deletions(-)
I'm not really sure what has happened here, but you have got an absolutely enourmous diff here. I would expect this patch to only affect the small part of qemuBuildCommandLine that actually deals with graphics devices. As it is now, it is impossible to review this patch I'm afraid :-(
Daniel
It was a code split, where git diff got mad. With the patience algorithm the patch looks much better. Peter

On Thu, Nov 08, 2012 at 09:48:55AM +0100, Alon Levy wrote:
The check for a single display remains so no new functionality is added. ---
Concerning both patches, I've tested running with all three simultaneously (sdl+spice+vnc) and with spice+vnc, using a single qxl device.
src/qemu/qemu_command.c | 2425 ++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +- 2 files changed, 1259 insertions(+), 1236 deletions(-)
I'm not really sure what has happened here, but you have got an absolutely enourmous diff here. I would expect this patch to only affect the small part of qemuBuildCommandLine that actually deals with graphics devices. As it is now, it is impossible to review this patch I'm afraid :-(
This is simply a refactoring, I've extracted the whole block inside qemuBuildCommandLine dealing with the graphics elements that qemu supports (vnc, sdl, spice) to qemuBuildGraphicsCommandLine. git diff is not very viewable since it looks at too few context lines, maybe it can be tweaked to give an easier view.
Daniel -- |: http://berrange.com -o- | http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- | http://virt-manager.org :| |: http://autobuild.org -o- | http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- | http://live.gnome.org/gtk-vnc :|

On 11/08/2012 07:04 AM, Daniel P. Berrange wrote:
On Thu, Nov 08, 2012 at 09:48:55AM +0100, Alon Levy wrote:
The check for a single display remains so no new functionality is added. ---
Concerning both patches, I've tested running with all three simultaneously (sdl+spice+vnc) and with spice+vnc, using a single qxl device.
src/qemu/qemu_command.c | 2425 ++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +- 2 files changed, 1259 insertions(+), 1236 deletions(-)
I'm not really sure what has happened here, but you have got an absolutely enourmous diff here. I would expect this patch to only affect the small part of qemuBuildCommandLine that actually deals with graphics devices. As it is now, it is impossible to review this patch I'm afraid :-(
'git diff --patience' to the rescue: $ git diff HEAD^ --stat src/qemu/qemu_command.c | 2425 ++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +- 2 files changed, 1259 insertions(+), 1236 deletions(-) $ git diff HEAD^ --stat --patience src/qemu/qemu_command.c | 647 +++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +++--- 2 files changed, 370 insertions(+), 347 deletions(-) I'll send the patience version of the patch as a followup for easier review. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 11/08/2012 07:04 AM, Daniel P. Berrange wrote:
On Thu, Nov 08, 2012 at 09:48:55AM +0100, Alon Levy wrote:
The check for a single display remains so no new functionality is added. ---
Concerning both patches, I've tested running with all three simultaneously (sdl+spice+vnc) and with spice+vnc, using a single qxl device.
src/qemu/qemu_command.c | 2425 ++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +- 2 files changed, 1259 insertions(+), 1236 deletions(-)
I'm not really sure what has happened here, but you have got an absolutely enourmous diff here. I would expect this patch to only affect the small part of qemuBuildCommandLine that actually deals with graphics devices. As it is now, it is impossible to review this patch I'm afraid :-(
'git diff --patience' to the rescue:
$ git diff HEAD^ --stat src/qemu/qemu_command.c | 2425 ++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +- 2 files changed, 1259 insertions(+), 1236 deletions(-) $ git diff HEAD^ --stat --patience src/qemu/qemu_command.c | 647 +++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +++--- 2 files changed, 370 insertions(+), 347 deletions(-)
I'll send the patience version of the patch as a followup for easier review.
Thanks!
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: Alon Levy <alevy@redhat.com> The check for a single display remains so no new functionality is added. --- No change to the patch itself, just the use of the --patience flag to make review much easier. src/qemu/qemu_command.c | 647 +++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +++--- 2 files changed, 370 insertions(+), 347 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1e96982..f9e4d4d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4410,6 +4410,334 @@ error: return -1; } +enum { + OK=0, + ERROR=1, + NO_MEMORY=2, +}; + +static int +qemuBuildGraphicsCommandLine(struct qemud_driver *driver, + virCommandPtr cmd, + virDomainDefPtr def, + qemuCapsPtr caps, + virDomainGraphicsDefPtr graphics) +{ + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + virBuffer opt = VIR_BUFFER_INITIALIZER; + + if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vnc graphics are not supported with this QEMU")); + return ERROR; + } + + if (graphics->data.vnc.socket || + driver->vncAutoUnixSocket) { + + if (!graphics->data.vnc.socket && + virAsprintf(&graphics->data.vnc.socket, + "%s/%s.vnc", driver->libDir, def->name) == -1) { + return NO_MEMORY; + } + + virBufferAsprintf(&opt, "unix:%s", + graphics->data.vnc.socket); + + } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + bool escapeAddr; + int ret; + + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + return 1; + } + if (ret < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + return 1; + } + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it will + * show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + return 1; + break; + } + + if (!listenAddr) + listenAddr = driver->vncListen; + + escapeAddr = strchr(listenAddr, ':') != NULL; + if (escapeAddr) + virBufferAsprintf(&opt, "[%s]", listenAddr); + else + virBufferAdd(&opt, listenAddr, -1); + virBufferAsprintf(&opt, ":%d", + graphics->data.vnc.port - 5900); + + VIR_FREE(netAddr); + } else { + virBufferAsprintf(&opt, "%d", + graphics->data.vnc.port - 5900); + } + + if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { + if (graphics->data.vnc.auth.passwd || + driver->vncPassword) + virBufferAddLit(&opt, ",password"); + + if (driver->vncTLS) { + virBufferAddLit(&opt, ",tls"); + if (driver->vncTLSx509verify) { + virBufferAsprintf(&opt, ",x509verify=%s", + driver->vncTLSx509certdir); + } else { + virBufferAsprintf(&opt, ",x509=%s", + driver->vncTLSx509certdir); + } + } + + if (driver->vncSASL) { + virBufferAddLit(&opt, ",sasl"); + + if (driver->vncSASLdir) + virCommandAddEnvPair(cmd, "SASL_CONF_DIR", + driver->vncSASLdir); + + /* TODO: Support ACLs later */ + } + } + + virCommandAddArg(cmd, "-vnc"); + virCommandAddArgBuffer(cmd, &opt); + if (graphics->data.vnc.keymap) { + virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap, + NULL); + } + + /* Unless user requested it, set the audio backend to none, to + * prevent it opening the host OS audio devices, since that causes + * security issues and might not work when using VNC. + */ + if (driver->vncAllowHostAudio) { + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); + } else { + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + } + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { + if (qemuCapsGet(caps, QEMU_CAPS_0_10) && + !qemuCapsGet(caps, QEMU_CAPS_SDL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("sdl not supported by '%s'"), + def->emulator); + return 1; + } + + if (graphics->data.sdl.xauth) + virCommandAddEnvPair(cmd, "XAUTHORITY", + graphics->data.sdl.xauth); + if (graphics->data.sdl.display) + virCommandAddEnvPair(cmd, "DISPLAY", + graphics->data.sdl.display); + if (graphics->data.sdl.fullscreen) + 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 + */ + 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 (qemuCapsGet(caps, QEMU_CAPS_SDL)) + virCommandAddArg(cmd, "-sdl"); + + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + int ret; + int defaultMode = graphics->data.spice.defaultMode; + + if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice graphics are not supported with this QEMU")); + return 1; + } + + virBufferAsprintf(&opt, "port=%u", graphics->data.spice.port); + + if (graphics->data.spice.tlsPort > 0) { + if (!driver->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice TLS port set in XML configuration," + " but TLS is disabled in qemu.conf")); + return 1; + } + virBufferAsprintf(&opt, ",tls-port=%u", + graphics->data.spice.tlsPort); + } + + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + return 1; + } + if (ret < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + return 1; + } + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it will + * show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + return 1; + break; + } + + if (!listenAddr) + listenAddr = driver->spiceListen; + if (listenAddr) + virBufferAsprintf(&opt, ",addr=%s", listenAddr); + + VIR_FREE(netAddr); + + int mm = graphics->data.spice.mousemode; + if (mm) { + switch (mm) { + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: + virBufferAsprintf(&opt, ",agent-mouse=off"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: + virBufferAsprintf(&opt, ",agent-mouse=on"); + break; + default: + break; + } + } + + /* In the password case we set it via monitor command, to avoid + * making it visible on CLI, so there's no use of password=XXX + * in this bit of the code */ + if (!graphics->data.spice.auth.passwd && + !driver->spicePassword) + virBufferAddLit(&opt, ",disable-ticketing"); + + if (driver->spiceTLS) + virBufferAsprintf(&opt, ",x509-dir=%s", + driver->spiceTLSx509certdir); + + switch (defaultMode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + virBufferAsprintf(&opt, ",tls-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + /* nothing */ + break; + } + + for (int i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { + int mode = graphics->data.spice.channels[i]; + switch (mode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + if (!driver->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf")); + return 1; + } + virBufferAsprintf(&opt, ",tls-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); + break; + } + } + if (graphics->data.spice.image) + virBufferAsprintf(&opt, ",image-compression=%s", + virDomainGraphicsSpiceImageCompressionTypeToString(graphics->data.spice.image)); + if (graphics->data.spice.jpeg) + virBufferAsprintf(&opt, ",jpeg-wan-compression=%s", + virDomainGraphicsSpiceJpegCompressionTypeToString(graphics->data.spice.jpeg)); + if (graphics->data.spice.zlib) + virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s", + virDomainGraphicsSpiceZlibCompressionTypeToString(graphics->data.spice.zlib)); + if (graphics->data.spice.playback) + virBufferAsprintf(&opt, ",playback-compression=%s", + virDomainGraphicsSpicePlaybackCompressionTypeToString(graphics->data.spice.playback)); + if (graphics->data.spice.streaming) + virBufferAsprintf(&opt, ",streaming-video=%s", + virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming)); + if (graphics->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) + virBufferAddLit(&opt, ",disable-copy-paste"); + + if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) { + /* If qemu supports seamless migration turn it + * unconditionally on. If migration destination + * doesn't support it, it fallbacks to previous + * migration algorithm silently. */ + virBufferAddLit(&opt, ",seamless-migration=on"); + } + + virCommandAddArg(cmd, "-spice"); + virCommandAddArgBuffer(cmd, &opt); + if (graphics->data.spice.keymap) + virCommandAddArgList(cmd, "-k", + graphics->data.spice.keymap, NULL); + /* SPICE includes native support for tunnelling audio, so we + * set the audio backend to point at SPICE's own driver + */ + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); + + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported graphics type '%s'"), + virDomainGraphicsTypeToString(graphics->type)); + return 1; + } + return 0; +} + /* * Constructs a argv suitable for launching qemu with config defined * for a given virtual machine. @@ -5863,322 +6191,15 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - virBuffer opt = VIR_BUFFER_INITIALIZER; - - if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("vnc graphics are not supported with this QEMU")); + for (i = 0 ; i < def->ngraphics ; ++i) { + switch (qemuBuildGraphicsCommandLine(driver, cmd, def, caps, + def->graphics[i])) { + case ERROR: goto error; + case NO_MEMORY: + goto no_memory; } - - if (def->graphics[0]->data.vnc.socket || - driver->vncAutoUnixSocket) { - - if (!def->graphics[0]->data.vnc.socket && - virAsprintf(&def->graphics[0]->data.vnc.socket, - "%s/%s.vnc", driver->libDir, def->name) == -1) { - goto no_memory; - } - - virBufferAsprintf(&opt, "unix:%s", - def->graphics[0]->data.vnc.socket); - - } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { - const char *listenNetwork; - const char *listenAddr = NULL; - char *netAddr = NULL; - bool escapeAddr; - int ret; - - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); - goto error; - } - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0, - listenAddr, -1, false) < 0) - goto error; - break; - } - - if (!listenAddr) - listenAddr = driver->vncListen; - - escapeAddr = strchr(listenAddr, ':') != NULL; - if (escapeAddr) - virBufferAsprintf(&opt, "[%s]", listenAddr); - else - virBufferAdd(&opt, listenAddr, -1); - virBufferAsprintf(&opt, ":%d", - def->graphics[0]->data.vnc.port - 5900); - - VIR_FREE(netAddr); - } else { - virBufferAsprintf(&opt, "%d", - def->graphics[0]->data.vnc.port - 5900); - } - - if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { - if (def->graphics[0]->data.vnc.auth.passwd || - driver->vncPassword) - virBufferAddLit(&opt, ",password"); - - if (driver->vncTLS) { - virBufferAddLit(&opt, ",tls"); - if (driver->vncTLSx509verify) { - virBufferAsprintf(&opt, ",x509verify=%s", - driver->vncTLSx509certdir); - } else { - virBufferAsprintf(&opt, ",x509=%s", - driver->vncTLSx509certdir); - } - } - - if (driver->vncSASL) { - virBufferAddLit(&opt, ",sasl"); - - if (driver->vncSASLdir) - virCommandAddEnvPair(cmd, "SASL_CONF_DIR", - driver->vncSASLdir); - - /* TODO: Support ACLs later */ - } - } - - virCommandAddArg(cmd, "-vnc"); - virCommandAddArgBuffer(cmd, &opt); - if (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 - * prevent it opening the host OS audio devices, since that causes - * security issues and might not work when using VNC. - */ - if (driver->vncAllowHostAudio) { - virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); - } else { - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - } - } else if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { - if (qemuCapsGet(caps, QEMU_CAPS_0_10) && - !qemuCapsGet(caps, QEMU_CAPS_SDL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("sdl not supported by '%s'"), - def->emulator); - goto error; - } - - 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) - 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 - */ - 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 (qemuCapsGet(caps, QEMU_CAPS_SDL)) - virCommandAddArg(cmd, "-sdl"); - - } else if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virBuffer opt = VIR_BUFFER_INITIALIZER; - const char *listenNetwork; - const char *listenAddr = NULL; - char *netAddr = NULL; - int ret; - int defaultMode = def->graphics[0]->data.spice.defaultMode; - - if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice graphics are not supported with this QEMU")); - goto error; - } - - virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port); - - if (def->graphics[0]->data.spice.tlsPort > 0) { - if (!driver->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice TLS port set in XML configuration," - " but TLS is disabled in qemu.conf")); - goto error; - } - virBufferAsprintf(&opt, ",tls-port=%u", - def->graphics[0]->data.spice.tlsPort); - } - - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); - goto error; - } - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0, - listenAddr, -1, false) < 0) - goto error; - break; - } - - if (!listenAddr) - listenAddr = driver->spiceListen; - if (listenAddr) - virBufferAsprintf(&opt, ",addr=%s", listenAddr); - - VIR_FREE(netAddr); - - int mm = def->graphics[0]->data.spice.mousemode; - if (mm) { - switch (mm) { - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: - virBufferAsprintf(&opt, ",agent-mouse=off"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: - virBufferAsprintf(&opt, ",agent-mouse=on"); - break; - default: - break; - } - } - - /* In the password case we set it via monitor command, to avoid - * making it visible on CLI, so there's no use of password=XXX - * in this bit of the code */ - if (!def->graphics[0]->data.spice.auth.passwd && - !driver->spicePassword) - virBufferAddLit(&opt, ",disable-ticketing"); - - if (driver->spiceTLS) - virBufferAsprintf(&opt, ",x509-dir=%s", - driver->spiceTLSx509certdir); - - switch (defaultMode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - virBufferAsprintf(&opt, ",tls-channel=default"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAsprintf(&opt, ",plaintext-channel=default"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: - /* nothing */ - break; - } - - for (i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { - int mode = def->graphics[0]->data.spice.channels[i]; - switch (mode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - if (!driver->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf")); - goto error; - } - virBufferAsprintf(&opt, ",tls-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAsprintf(&opt, ",plaintext-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); - break; - } - } - if (def->graphics[0]->data.spice.image) - virBufferAsprintf(&opt, ",image-compression=%s", - virDomainGraphicsSpiceImageCompressionTypeToString(def->graphics[0]->data.spice.image)); - if (def->graphics[0]->data.spice.jpeg) - virBufferAsprintf(&opt, ",jpeg-wan-compression=%s", - virDomainGraphicsSpiceJpegCompressionTypeToString(def->graphics[0]->data.spice.jpeg)); - if (def->graphics[0]->data.spice.zlib) - virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s", - virDomainGraphicsSpiceZlibCompressionTypeToString(def->graphics[0]->data.spice.zlib)); - if (def->graphics[0]->data.spice.playback) - virBufferAsprintf(&opt, ",playback-compression=%s", - virDomainGraphicsSpicePlaybackCompressionTypeToString(def->graphics[0]->data.spice.playback)); - if (def->graphics[0]->data.spice.streaming) - virBufferAsprintf(&opt, ",streaming-video=%s", - virDomainGraphicsSpiceStreamingModeTypeToString(def->graphics[0]->data.spice.streaming)); - if (def->graphics[0]->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) - virBufferAddLit(&opt, ",disable-copy-paste"); - - if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) { - /* If qemu supports seamless migration turn it - * unconditionally on. If migration destination - * doesn't support it, it fallbacks to previous - * migration algorithm silently. */ - virBufferAddLit(&opt, ",seamless-migration=on"); - } - - virCommandAddArg(cmd, "-spice"); - virCommandAddArgBuffer(cmd, &opt); - 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 - */ - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); - - } else if ((def->ngraphics == 1)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported graphics type '%s'"), - virDomainGraphicsTypeToString(def->graphics[0]->type)); - goto error; } - if (def->nvideos > 0) { if (qemuCapsGet(caps, QEMU_CAPS_VGA)) { if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_XEN) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5bd042d..7a671dd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2079,16 +2079,17 @@ qemuProcessInitPasswords(virConnectPtr conn, int ret = 0; qemuDomainObjPrivatePtr priv = vm->privateData; - if (vm->def->ngraphics == 1) { - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + for (int i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { ret = qemuDomainChangeGraphicsPasswords(driver, vm, VIR_DOMAIN_GRAPHICS_TYPE_VNC, - &vm->def->graphics[0]->data.vnc.auth, + &graphics->data.vnc.auth, driver->vncPassword); - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { ret = qemuDomainChangeGraphicsPasswords(driver, vm, VIR_DOMAIN_GRAPHICS_TYPE_SPICE, - &vm->def->graphics[0]->data.spice.auth, + &graphics->data.spice.auth, driver->spicePassword); } } @@ -3482,21 +3483,22 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Ensuring no historical cgroup is lying around"); qemuRemoveCgroup(driver, vm, 1); - if (vm->def->ngraphics == 1) { - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - !vm->def->graphics[0]->data.vnc.socket && - vm->def->graphics[0]->data.vnc.autoport) { + for (i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !graphics->data.vnc.socket && + graphics->data.vnc.autoport) { int port = qemuProcessNextFreePort(driver, driver->remotePortMin); if (port < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for VNC")); goto cleanup; } - vm->def->graphics[0]->data.vnc.port = port; - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + graphics->data.vnc.port = port; + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { int port = -1; - if (vm->def->graphics[0]->data.spice.autoport || - vm->def->graphics[0]->data.spice.port == -1) { + if (graphics->data.spice.autoport || + graphics->data.spice.port == -1) { port = qemuProcessNextFreePort(driver, driver->remotePortMin); if (port < 0) { @@ -3505,13 +3507,13 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } - vm->def->graphics[0]->data.spice.port = port; + graphics->data.spice.port = port; } if (driver->spiceTLS && - (vm->def->graphics[0]->data.spice.autoport || - vm->def->graphics[0]->data.spice.tlsPort == -1)) { + (graphics->data.spice.autoport || + graphics->data.spice.tlsPort == -1)) { int tlsPort = qemuProcessNextFreePort(driver, - vm->def->graphics[0]->data.spice.port + 1); + graphics->data.spice.port + 1); if (tlsPort < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for SPICE TLS")); @@ -3519,20 +3521,19 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } - vm->def->graphics[0]->data.spice.tlsPort = tlsPort; + graphics->data.spice.tlsPort = tlsPort; } } - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virDomainGraphicsDefPtr graphics = vm->def->graphics[0]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { if (graphics->nListens == 0) { if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0) { virReportOOMError(); goto cleanup; } graphics->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) graphics->listens[0].address = strdup(driver->vncListen); else graphics->listens[0].address = strdup(driver->spiceListen); @@ -4148,19 +4149,20 @@ retry: qemuProcessRemoveDomainStatus(driver, vm); - /* Remove VNC port from port reservation bitmap, but only if it was - reserved by the driver (autoport=yes) + /* Remove VNC and Spice ports from port reservation bitmap, but only if + they were reserved by the driver (autoport=yes) */ - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - vm->def->graphics[0]->data.vnc.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.vnc.port); - } - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - vm->def->graphics[0]->data.spice.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.port); - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.tlsPort); + for (i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + graphics->data.vnc.autoport) { + qemuProcessReturnPort(driver, graphics->data.vnc.port); + } + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + graphics->data.spice.autoport) { + qemuProcessReturnPort(driver, graphics->data.spice.port); + qemuProcessReturnPort(driver, graphics->data.spice.tlsPort); + } } vm->taint = 0; -- 1.7.11.7

hmm, I'm not using the enum everywhere, I'll send again after fixing (and use --patience). ----- Original Message -----
From: Alon Levy <alevy@redhat.com>
The check for a single display remains so no new functionality is added. ---
No change to the patch itself, just the use of the --patience flag to make review much easier.
src/qemu/qemu_command.c | 647 +++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +++--- 2 files changed, 370 insertions(+), 347 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1e96982..f9e4d4d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4410,6 +4410,334 @@ error: return -1; }
+enum { + OK=0, + ERROR=1, + NO_MEMORY=2, +}; + +static int +qemuBuildGraphicsCommandLine(struct qemud_driver *driver, + virCommandPtr cmd, + virDomainDefPtr def, + qemuCapsPtr caps, + virDomainGraphicsDefPtr graphics) +{ + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + virBuffer opt = VIR_BUFFER_INITIALIZER; + + if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vnc graphics are not supported with this QEMU")); + return ERROR; + } + + if (graphics->data.vnc.socket || + driver->vncAutoUnixSocket) { + + if (!graphics->data.vnc.socket && + virAsprintf(&graphics->data.vnc.socket, + "%s/%s.vnc", driver->libDir, def->name) == -1) { + return NO_MEMORY; + } + + virBufferAsprintf(&opt, "unix:%s", + graphics->data.vnc.socket); + + } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + bool escapeAddr; + int ret; + + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + return 1; + } + if (ret < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + return 1; + } + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it will + * show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + return 1; + break; + } + + if (!listenAddr) + listenAddr = driver->vncListen; + + escapeAddr = strchr(listenAddr, ':') != NULL; + if (escapeAddr) + virBufferAsprintf(&opt, "[%s]", listenAddr); + else + virBufferAdd(&opt, listenAddr, -1); + virBufferAsprintf(&opt, ":%d", + graphics->data.vnc.port - 5900); + + VIR_FREE(netAddr); + } else { + virBufferAsprintf(&opt, "%d", + graphics->data.vnc.port - 5900); + } + + if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { + if (graphics->data.vnc.auth.passwd || + driver->vncPassword) + virBufferAddLit(&opt, ",password"); + + if (driver->vncTLS) { + virBufferAddLit(&opt, ",tls"); + if (driver->vncTLSx509verify) { + virBufferAsprintf(&opt, ",x509verify=%s", + driver->vncTLSx509certdir); + } else { + virBufferAsprintf(&opt, ",x509=%s", + driver->vncTLSx509certdir); + } + } + + if (driver->vncSASL) { + virBufferAddLit(&opt, ",sasl"); + + if (driver->vncSASLdir) + virCommandAddEnvPair(cmd, "SASL_CONF_DIR", + driver->vncSASLdir); + + /* TODO: Support ACLs later */ + } + } + + virCommandAddArg(cmd, "-vnc"); + virCommandAddArgBuffer(cmd, &opt); + if (graphics->data.vnc.keymap) { + virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap, + NULL); + } + + /* Unless user requested it, set the audio backend to none, to + * prevent it opening the host OS audio devices, since that causes + * security issues and might not work when using VNC. + */ + if (driver->vncAllowHostAudio) { + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); + } else { + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + } + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { + if (qemuCapsGet(caps, QEMU_CAPS_0_10) && + !qemuCapsGet(caps, QEMU_CAPS_SDL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("sdl not supported by '%s'"), + def->emulator); + return 1; + } + + if (graphics->data.sdl.xauth) + virCommandAddEnvPair(cmd, "XAUTHORITY", + graphics->data.sdl.xauth); + if (graphics->data.sdl.display) + virCommandAddEnvPair(cmd, "DISPLAY", + graphics->data.sdl.display); + if (graphics->data.sdl.fullscreen) + 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 + */ + 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 (qemuCapsGet(caps, QEMU_CAPS_SDL)) + virCommandAddArg(cmd, "-sdl"); + + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + int ret; + int defaultMode = graphics->data.spice.defaultMode; + + if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice graphics are not supported with this QEMU")); + return 1; + } + + virBufferAsprintf(&opt, "port=%u", graphics->data.spice.port); + + if (graphics->data.spice.tlsPort > 0) { + if (!driver->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice TLS port set in XML configuration," + " but TLS is disabled in qemu.conf")); + return 1; + } + virBufferAsprintf(&opt, ",tls-port=%u", + graphics->data.spice.tlsPort); + } + + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + return 1; + } + if (ret < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + return 1; + } + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it will + * show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + return 1; + break; + } + + if (!listenAddr) + listenAddr = driver->spiceListen; + if (listenAddr) + virBufferAsprintf(&opt, ",addr=%s", listenAddr); + + VIR_FREE(netAddr); + + int mm = graphics->data.spice.mousemode; + if (mm) { + switch (mm) { + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: + virBufferAsprintf(&opt, ",agent-mouse=off"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: + virBufferAsprintf(&opt, ",agent-mouse=on"); + break; + default: + break; + } + } + + /* In the password case we set it via monitor command, to avoid + * making it visible on CLI, so there's no use of password=XXX + * in this bit of the code */ + if (!graphics->data.spice.auth.passwd && + !driver->spicePassword) + virBufferAddLit(&opt, ",disable-ticketing"); + + if (driver->spiceTLS) + virBufferAsprintf(&opt, ",x509-dir=%s", + driver->spiceTLSx509certdir); + + switch (defaultMode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + virBufferAsprintf(&opt, ",tls-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + /* nothing */ + break; + } + + for (int i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { + int mode = graphics->data.spice.channels[i]; + switch (mode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + if (!driver->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf")); + return 1; + } + virBufferAsprintf(&opt, ",tls-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); + break; + } + } + if (graphics->data.spice.image) + virBufferAsprintf(&opt, ",image-compression=%s", + virDomainGraphicsSpiceImageCompressionTypeToString(graphics->data.spice.image)); + if (graphics->data.spice.jpeg) + virBufferAsprintf(&opt, ",jpeg-wan-compression=%s", + virDomainGraphicsSpiceJpegCompressionTypeToString(graphics->data.spice.jpeg)); + if (graphics->data.spice.zlib) + virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s", + virDomainGraphicsSpiceZlibCompressionTypeToString(graphics->data.spice.zlib)); + if (graphics->data.spice.playback) + virBufferAsprintf(&opt, ",playback-compression=%s", + virDomainGraphicsSpicePlaybackCompressionTypeToString(graphics->data.spice.playback)); + if (graphics->data.spice.streaming) + virBufferAsprintf(&opt, ",streaming-video=%s", + virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming)); + if (graphics->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) + virBufferAddLit(&opt, ",disable-copy-paste"); + + if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) { + /* If qemu supports seamless migration turn it + * unconditionally on. If migration destination + * doesn't support it, it fallbacks to previous + * migration algorithm silently. */ + virBufferAddLit(&opt, ",seamless-migration=on"); + } + + virCommandAddArg(cmd, "-spice"); + virCommandAddArgBuffer(cmd, &opt); + if (graphics->data.spice.keymap) + virCommandAddArgList(cmd, "-k", + graphics->data.spice.keymap, NULL); + /* SPICE includes native support for tunnelling audio, so we + * set the audio backend to point at SPICE's own driver + */ + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); + + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported graphics type '%s'"), + virDomainGraphicsTypeToString(graphics->type)); + return 1; + } + return 0; +} + /* * Constructs a argv suitable for launching qemu with config defined * for a given virtual machine. @@ -5863,322 +6191,15 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
- if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - virBuffer opt = VIR_BUFFER_INITIALIZER; - - if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("vnc graphics are not supported with this QEMU")); + for (i = 0 ; i < def->ngraphics ; ++i) { + switch (qemuBuildGraphicsCommandLine(driver, cmd, def, caps, + def->graphics[i])) { + case ERROR: goto error; + case NO_MEMORY: + goto no_memory; } - - if (def->graphics[0]->data.vnc.socket || - driver->vncAutoUnixSocket) { - - if (!def->graphics[0]->data.vnc.socket && - virAsprintf(&def->graphics[0]->data.vnc.socket, - "%s/%s.vnc", driver->libDir, def->name) == -1) { - goto no_memory; - } - - virBufferAsprintf(&opt, "unix:%s", - def->graphics[0]->data.vnc.socket); - - } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { - const char *listenNetwork; - const char *listenAddr = NULL; - char *netAddr = NULL; - bool escapeAddr; - int ret; - - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); - goto error; - } - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0, - listenAddr, -1, false) < 0) - goto error; - break; - } - - if (!listenAddr) - listenAddr = driver->vncListen; - - escapeAddr = strchr(listenAddr, ':') != NULL; - if (escapeAddr) - virBufferAsprintf(&opt, "[%s]", listenAddr); - else - virBufferAdd(&opt, listenAddr, -1); - virBufferAsprintf(&opt, ":%d", - def->graphics[0]->data.vnc.port - 5900); - - VIR_FREE(netAddr); - } else { - virBufferAsprintf(&opt, "%d", - def->graphics[0]->data.vnc.port - 5900); - } - - if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { - if (def->graphics[0]->data.vnc.auth.passwd || - driver->vncPassword) - virBufferAddLit(&opt, ",password"); - - if (driver->vncTLS) { - virBufferAddLit(&opt, ",tls"); - if (driver->vncTLSx509verify) { - virBufferAsprintf(&opt, ",x509verify=%s", - driver->vncTLSx509certdir); - } else { - virBufferAsprintf(&opt, ",x509=%s", - driver->vncTLSx509certdir); - } - } - - if (driver->vncSASL) { - virBufferAddLit(&opt, ",sasl"); - - if (driver->vncSASLdir) - virCommandAddEnvPair(cmd, "SASL_CONF_DIR", - driver->vncSASLdir); - - /* TODO: Support ACLs later */ - } - } - - virCommandAddArg(cmd, "-vnc"); - virCommandAddArgBuffer(cmd, &opt); - if (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 - * prevent it opening the host OS audio devices, since that causes - * security issues and might not work when using VNC. - */ - if (driver->vncAllowHostAudio) { - virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); - } else { - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - } - } else if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { - if (qemuCapsGet(caps, QEMU_CAPS_0_10) && - !qemuCapsGet(caps, QEMU_CAPS_SDL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("sdl not supported by '%s'"), - def->emulator); - goto error; - } - - 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) - 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 - */ - 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 (qemuCapsGet(caps, QEMU_CAPS_SDL)) - virCommandAddArg(cmd, "-sdl"); - - } else if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virBuffer opt = VIR_BUFFER_INITIALIZER; - const char *listenNetwork; - const char *listenAddr = NULL; - char *netAddr = NULL; - int ret; - int defaultMode = def->graphics[0]->data.spice.defaultMode; - - if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice graphics are not supported with this QEMU")); - goto error; - } - - virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port); - - if (def->graphics[0]->data.spice.tlsPort > 0) { - if (!driver->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice TLS port set in XML configuration," - " but TLS is disabled in qemu.conf")); - goto error; - } - virBufferAsprintf(&opt, ",tls-port=%u", - def->graphics[0]->data.spice.tlsPort); - } - - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); - goto error; - } - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0, - listenAddr, -1, false) < 0) - goto error; - break; - } - - if (!listenAddr) - listenAddr = driver->spiceListen; - if (listenAddr) - virBufferAsprintf(&opt, ",addr=%s", listenAddr); - - VIR_FREE(netAddr); - - int mm = def->graphics[0]->data.spice.mousemode; - if (mm) { - switch (mm) { - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: - virBufferAsprintf(&opt, ",agent-mouse=off"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: - virBufferAsprintf(&opt, ",agent-mouse=on"); - break; - default: - break; - } - } - - /* In the password case we set it via monitor command, to avoid - * making it visible on CLI, so there's no use of password=XXX - * in this bit of the code */ - if (!def->graphics[0]->data.spice.auth.passwd && - !driver->spicePassword) - virBufferAddLit(&opt, ",disable-ticketing"); - - if (driver->spiceTLS) - virBufferAsprintf(&opt, ",x509-dir=%s", - driver->spiceTLSx509certdir); - - switch (defaultMode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - virBufferAsprintf(&opt, ",tls-channel=default"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAsprintf(&opt, ",plaintext-channel=default"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: - /* nothing */ - break; - } - - for (i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { - int mode = def->graphics[0]->data.spice.channels[i]; - switch (mode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - if (!driver->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf")); - goto error; - } - virBufferAsprintf(&opt, ",tls-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAsprintf(&opt, ",plaintext-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); - break; - } - } - if (def->graphics[0]->data.spice.image) - virBufferAsprintf(&opt, ",image-compression=%s", - virDomainGraphicsSpiceImageCompressionTypeToString(def->graphics[0]->data.spice.image)); - if (def->graphics[0]->data.spice.jpeg) - virBufferAsprintf(&opt, ",jpeg-wan-compression=%s", - virDomainGraphicsSpiceJpegCompressionTypeToString(def->graphics[0]->data.spice.jpeg)); - if (def->graphics[0]->data.spice.zlib) - virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s", - virDomainGraphicsSpiceZlibCompressionTypeToString(def->graphics[0]->data.spice.zlib)); - if (def->graphics[0]->data.spice.playback) - virBufferAsprintf(&opt, ",playback-compression=%s", - virDomainGraphicsSpicePlaybackCompressionTypeToString(def->graphics[0]->data.spice.playback)); - if (def->graphics[0]->data.spice.streaming) - virBufferAsprintf(&opt, ",streaming-video=%s", - virDomainGraphicsSpiceStreamingModeTypeToString(def->graphics[0]->data.spice.streaming)); - if (def->graphics[0]->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) - virBufferAddLit(&opt, ",disable-copy-paste"); - - if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) { - /* If qemu supports seamless migration turn it - * unconditionally on. If migration destination - * doesn't support it, it fallbacks to previous - * migration algorithm silently. */ - virBufferAddLit(&opt, ",seamless-migration=on"); - } - - virCommandAddArg(cmd, "-spice"); - virCommandAddArgBuffer(cmd, &opt); - 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 - */ - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); - - } else if ((def->ngraphics == 1)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported graphics type '%s'"), - virDomainGraphicsTypeToString(def->graphics[0]->type)); - goto error; } - if (def->nvideos > 0) { if (qemuCapsGet(caps, QEMU_CAPS_VGA)) { if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_XEN) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5bd042d..7a671dd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2079,16 +2079,17 @@ qemuProcessInitPasswords(virConnectPtr conn, int ret = 0; qemuDomainObjPrivatePtr priv = vm->privateData;
- if (vm->def->ngraphics == 1) { - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + for (int i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { ret = qemuDomainChangeGraphicsPasswords(driver, vm, VIR_DOMAIN_GRAPHICS_TYPE_VNC, - &vm->def->graphics[0]->data.vnc.auth, + &graphics->data.vnc.auth, driver->vncPassword); - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { ret = qemuDomainChangeGraphicsPasswords(driver, vm, VIR_DOMAIN_GRAPHICS_TYPE_SPICE, - &vm->def->graphics[0]->data.spice.auth, + &graphics->data.spice.auth, driver->spicePassword); } } @@ -3482,21 +3483,22 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Ensuring no historical cgroup is lying around"); qemuRemoveCgroup(driver, vm, 1);
- if (vm->def->ngraphics == 1) { - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - !vm->def->graphics[0]->data.vnc.socket && - vm->def->graphics[0]->data.vnc.autoport) { + for (i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !graphics->data.vnc.socket && + graphics->data.vnc.autoport) { int port = qemuProcessNextFreePort(driver, driver->remotePortMin); if (port < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for VNC")); goto cleanup; } - vm->def->graphics[0]->data.vnc.port = port; - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + graphics->data.vnc.port = port; + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { int port = -1; - if (vm->def->graphics[0]->data.spice.autoport || - vm->def->graphics[0]->data.spice.port == -1) { + if (graphics->data.spice.autoport || + graphics->data.spice.port == -1) { port = qemuProcessNextFreePort(driver, driver->remotePortMin);
if (port < 0) { @@ -3505,13 +3507,13 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; }
- vm->def->graphics[0]->data.spice.port = port; + graphics->data.spice.port = port; } if (driver->spiceTLS && - (vm->def->graphics[0]->data.spice.autoport || - vm->def->graphics[0]->data.spice.tlsPort == -1)) { + (graphics->data.spice.autoport || + graphics->data.spice.tlsPort == -1)) { int tlsPort = qemuProcessNextFreePort(driver, - vm->def->graphics[0]->data.spice.port + 1); + graphics->data.spice.port + 1); if (tlsPort < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for SPICE TLS")); @@ -3519,20 +3521,19 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; }
- vm->def->graphics[0]->data.spice.tlsPort = tlsPort; + graphics->data.spice.tlsPort = tlsPort; } }
- if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virDomainGraphicsDefPtr graphics = vm->def->graphics[0]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { if (graphics->nListens == 0) { if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0) { virReportOOMError(); goto cleanup; } graphics->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) graphics->listens[0].address = strdup(driver->vncListen); else graphics->listens[0].address = strdup(driver->spiceListen); @@ -4148,19 +4149,20 @@ retry:
qemuProcessRemoveDomainStatus(driver, vm);
- /* Remove VNC port from port reservation bitmap, but only if it was - reserved by the driver (autoport=yes) + /* Remove VNC and Spice ports from port reservation bitmap, but only if + they were reserved by the driver (autoport=yes) */ - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - vm->def->graphics[0]->data.vnc.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.vnc.port); - } - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - vm->def->graphics[0]->data.spice.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.port); - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.tlsPort); + for (i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + graphics->data.vnc.autoport) { + qemuProcessReturnPort(driver, graphics->data.vnc.port); + } + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + graphics->data.spice.autoport) { + qemuProcessReturnPort(driver, graphics->data.spice.port); + qemuProcessReturnPort(driver, graphics->data.spice.tlsPort); + } }
vm->taint = 0; -- 1.7.11.7

On Fri, Nov 09, 2012 at 05:14:10 -0500, Alon Levy wrote:
hmm, I'm not using the enum everywhere, I'll send again after fixing (and use --patience).
Actually it's be better to just get rid of the enum completely and rather follow libvirt's convention of returning -1 on error and 0 on success. And call virReportOOMError() directly from qemuBuildGraphicsCommandLine so that you don't have to care about returning different error values. Jirka

The check for a single display remains so no new functionality is added. --- v2 changes: removed enum, use virReportOOMError directly. use --patience added a one line label fix patch. The second patch changes a string that needs to be translated - how do I go about it? src/qemu/qemu_command.c | 641 +++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +++--- 2 files changed, 364 insertions(+), 347 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1e96982..ba99c7a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4410,6 +4410,331 @@ error: return -1; } +static int +qemuBuildGraphicsCommandLine(struct qemud_driver *driver, + virCommandPtr cmd, + virDomainDefPtr def, + qemuCapsPtr caps, + virDomainGraphicsDefPtr graphics) +{ + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + virBuffer opt = VIR_BUFFER_INITIALIZER; + + if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vnc graphics are not supported with this QEMU")); + goto error; + } + + if (graphics->data.vnc.socket || + driver->vncAutoUnixSocket) { + + if (!graphics->data.vnc.socket && + virAsprintf(&graphics->data.vnc.socket, + "%s/%s.vnc", driver->libDir, def->name) == -1) { + goto no_memory; + } + + virBufferAsprintf(&opt, "unix:%s", + graphics->data.vnc.socket); + + } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + bool escapeAddr; + int ret; + + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + goto error; + } + if (ret < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + goto error; + } + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it will + * show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + goto error; + break; + } + + if (!listenAddr) + listenAddr = driver->vncListen; + + escapeAddr = strchr(listenAddr, ':') != NULL; + if (escapeAddr) + virBufferAsprintf(&opt, "[%s]", listenAddr); + else + virBufferAdd(&opt, listenAddr, -1); + virBufferAsprintf(&opt, ":%d", + graphics->data.vnc.port - 5900); + + VIR_FREE(netAddr); + } else { + virBufferAsprintf(&opt, "%d", + graphics->data.vnc.port - 5900); + } + + if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { + if (graphics->data.vnc.auth.passwd || + driver->vncPassword) + virBufferAddLit(&opt, ",password"); + + if (driver->vncTLS) { + virBufferAddLit(&opt, ",tls"); + if (driver->vncTLSx509verify) { + virBufferAsprintf(&opt, ",x509verify=%s", + driver->vncTLSx509certdir); + } else { + virBufferAsprintf(&opt, ",x509=%s", + driver->vncTLSx509certdir); + } + } + + if (driver->vncSASL) { + virBufferAddLit(&opt, ",sasl"); + + if (driver->vncSASLdir) + virCommandAddEnvPair(cmd, "SASL_CONF_DIR", + driver->vncSASLdir); + + /* TODO: Support ACLs later */ + } + } + + virCommandAddArg(cmd, "-vnc"); + virCommandAddArgBuffer(cmd, &opt); + if (graphics->data.vnc.keymap) { + virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap, + NULL); + } + + /* Unless user requested it, set the audio backend to none, to + * prevent it opening the host OS audio devices, since that causes + * security issues and might not work when using VNC. + */ + if (driver->vncAllowHostAudio) { + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); + } else { + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + } + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { + if (qemuCapsGet(caps, QEMU_CAPS_0_10) && + !qemuCapsGet(caps, QEMU_CAPS_SDL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("sdl not supported by '%s'"), + def->emulator); + goto error; + } + + if (graphics->data.sdl.xauth) + virCommandAddEnvPair(cmd, "XAUTHORITY", + graphics->data.sdl.xauth); + if (graphics->data.sdl.display) + virCommandAddEnvPair(cmd, "DISPLAY", + graphics->data.sdl.display); + if (graphics->data.sdl.fullscreen) + 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 + */ + 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 (qemuCapsGet(caps, QEMU_CAPS_SDL)) + virCommandAddArg(cmd, "-sdl"); + + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + int ret; + int defaultMode = graphics->data.spice.defaultMode; + + if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice graphics are not supported with this QEMU")); + goto error; + } + + virBufferAsprintf(&opt, "port=%u", graphics->data.spice.port); + + if (graphics->data.spice.tlsPort > 0) { + if (!driver->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice TLS port set in XML configuration," + " but TLS is disabled in qemu.conf")); + return 1; + } + virBufferAsprintf(&opt, ",tls-port=%u", + graphics->data.spice.tlsPort); + } + + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + goto error; + } + if (ret < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + goto error; + } + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it will + * show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + goto error; + break; + } + + if (!listenAddr) + listenAddr = driver->spiceListen; + if (listenAddr) + virBufferAsprintf(&opt, ",addr=%s", listenAddr); + + VIR_FREE(netAddr); + + int mm = graphics->data.spice.mousemode; + if (mm) { + switch (mm) { + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: + virBufferAsprintf(&opt, ",agent-mouse=off"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: + virBufferAsprintf(&opt, ",agent-mouse=on"); + break; + default: + break; + } + } + + /* In the password case we set it via monitor command, to avoid + * making it visible on CLI, so there's no use of password=XXX + * in this bit of the code */ + if (!graphics->data.spice.auth.passwd && + !driver->spicePassword) + virBufferAddLit(&opt, ",disable-ticketing"); + + if (driver->spiceTLS) + virBufferAsprintf(&opt, ",x509-dir=%s", + driver->spiceTLSx509certdir); + + switch (defaultMode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + virBufferAsprintf(&opt, ",tls-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + /* nothing */ + break; + } + + for (int i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { + int mode = graphics->data.spice.channels[i]; + switch (mode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + if (!driver->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf")); + return 1; + } + virBufferAsprintf(&opt, ",tls-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); + break; + } + } + if (graphics->data.spice.image) + virBufferAsprintf(&opt, ",image-compression=%s", + virDomainGraphicsSpiceImageCompressionTypeToString(graphics->data.spice.image)); + if (graphics->data.spice.jpeg) + virBufferAsprintf(&opt, ",jpeg-wan-compression=%s", + virDomainGraphicsSpiceJpegCompressionTypeToString(graphics->data.spice.jpeg)); + if (graphics->data.spice.zlib) + virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s", + virDomainGraphicsSpiceZlibCompressionTypeToString(graphics->data.spice.zlib)); + if (graphics->data.spice.playback) + virBufferAsprintf(&opt, ",playback-compression=%s", + virDomainGraphicsSpicePlaybackCompressionTypeToString(graphics->data.spice.playback)); + if (graphics->data.spice.streaming) + virBufferAsprintf(&opt, ",streaming-video=%s", + virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming)); + if (graphics->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) + virBufferAddLit(&opt, ",disable-copy-paste"); + + if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) { + /* If qemu supports seamless migration turn it + * unconditionally on. If migration destination + * doesn't support it, it fallbacks to previous + * migration algorithm silently. */ + virBufferAddLit(&opt, ",seamless-migration=on"); + } + + virCommandAddArg(cmd, "-spice"); + virCommandAddArgBuffer(cmd, &opt); + if (graphics->data.spice.keymap) + virCommandAddArgList(cmd, "-k", + graphics->data.spice.keymap, NULL); + /* SPICE includes native support for tunnelling audio, so we + * set the audio backend to point at SPICE's own driver + */ + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); + + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported graphics type '%s'"), + virDomainGraphicsTypeToString(graphics->type)); + goto error; + } +no_memory: + virReportOOMError(); +error: + return 0; +} + /* * Constructs a argv suitable for launching qemu with config defined * for a given virtual machine. @@ -5863,322 +6188,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - virBuffer opt = VIR_BUFFER_INITIALIZER; - - if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("vnc graphics are not supported with this QEMU")); + for (i = 0 ; i < def->ngraphics ; ++i) { + if (qemuBuildGraphicsCommandLine(driver, cmd, def, caps, + def->graphics[i]) == -1) { goto error; } - - if (def->graphics[0]->data.vnc.socket || - driver->vncAutoUnixSocket) { - - if (!def->graphics[0]->data.vnc.socket && - virAsprintf(&def->graphics[0]->data.vnc.socket, - "%s/%s.vnc", driver->libDir, def->name) == -1) { - goto no_memory; - } - - virBufferAsprintf(&opt, "unix:%s", - def->graphics[0]->data.vnc.socket); - - } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { - const char *listenNetwork; - const char *listenAddr = NULL; - char *netAddr = NULL; - bool escapeAddr; - int ret; - - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); - goto error; - } - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0, - listenAddr, -1, false) < 0) - goto error; - break; - } - - if (!listenAddr) - listenAddr = driver->vncListen; - - escapeAddr = strchr(listenAddr, ':') != NULL; - if (escapeAddr) - virBufferAsprintf(&opt, "[%s]", listenAddr); - else - virBufferAdd(&opt, listenAddr, -1); - virBufferAsprintf(&opt, ":%d", - def->graphics[0]->data.vnc.port - 5900); - - VIR_FREE(netAddr); - } else { - virBufferAsprintf(&opt, "%d", - def->graphics[0]->data.vnc.port - 5900); - } - - if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { - if (def->graphics[0]->data.vnc.auth.passwd || - driver->vncPassword) - virBufferAddLit(&opt, ",password"); - - if (driver->vncTLS) { - virBufferAddLit(&opt, ",tls"); - if (driver->vncTLSx509verify) { - virBufferAsprintf(&opt, ",x509verify=%s", - driver->vncTLSx509certdir); - } else { - virBufferAsprintf(&opt, ",x509=%s", - driver->vncTLSx509certdir); - } - } - - if (driver->vncSASL) { - virBufferAddLit(&opt, ",sasl"); - - if (driver->vncSASLdir) - virCommandAddEnvPair(cmd, "SASL_CONF_DIR", - driver->vncSASLdir); - - /* TODO: Support ACLs later */ - } - } - - virCommandAddArg(cmd, "-vnc"); - virCommandAddArgBuffer(cmd, &opt); - if (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 - * prevent it opening the host OS audio devices, since that causes - * security issues and might not work when using VNC. - */ - if (driver->vncAllowHostAudio) { - virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); - } else { - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - } - } else if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { - if (qemuCapsGet(caps, QEMU_CAPS_0_10) && - !qemuCapsGet(caps, QEMU_CAPS_SDL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("sdl not supported by '%s'"), - def->emulator); - goto error; - } - - 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) - 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 - */ - 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 (qemuCapsGet(caps, QEMU_CAPS_SDL)) - virCommandAddArg(cmd, "-sdl"); - - } else if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virBuffer opt = VIR_BUFFER_INITIALIZER; - const char *listenNetwork; - const char *listenAddr = NULL; - char *netAddr = NULL; - int ret; - int defaultMode = def->graphics[0]->data.spice.defaultMode; - - if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice graphics are not supported with this QEMU")); - goto error; - } - - virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port); - - if (def->graphics[0]->data.spice.tlsPort > 0) { - if (!driver->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice TLS port set in XML configuration," - " but TLS is disabled in qemu.conf")); - goto error; - } - virBufferAsprintf(&opt, ",tls-port=%u", - def->graphics[0]->data.spice.tlsPort); - } - - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); - goto error; - } - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0, - listenAddr, -1, false) < 0) - goto error; - break; - } - - if (!listenAddr) - listenAddr = driver->spiceListen; - if (listenAddr) - virBufferAsprintf(&opt, ",addr=%s", listenAddr); - - VIR_FREE(netAddr); - - int mm = def->graphics[0]->data.spice.mousemode; - if (mm) { - switch (mm) { - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: - virBufferAsprintf(&opt, ",agent-mouse=off"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: - virBufferAsprintf(&opt, ",agent-mouse=on"); - break; - default: - break; - } - } - - /* In the password case we set it via monitor command, to avoid - * making it visible on CLI, so there's no use of password=XXX - * in this bit of the code */ - if (!def->graphics[0]->data.spice.auth.passwd && - !driver->spicePassword) - virBufferAddLit(&opt, ",disable-ticketing"); - - if (driver->spiceTLS) - virBufferAsprintf(&opt, ",x509-dir=%s", - driver->spiceTLSx509certdir); - - switch (defaultMode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - virBufferAsprintf(&opt, ",tls-channel=default"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAsprintf(&opt, ",plaintext-channel=default"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: - /* nothing */ - break; - } - - for (i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { - int mode = def->graphics[0]->data.spice.channels[i]; - switch (mode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - if (!driver->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf")); - goto error; - } - virBufferAsprintf(&opt, ",tls-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAsprintf(&opt, ",plaintext-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); - break; - } - } - if (def->graphics[0]->data.spice.image) - virBufferAsprintf(&opt, ",image-compression=%s", - virDomainGraphicsSpiceImageCompressionTypeToString(def->graphics[0]->data.spice.image)); - if (def->graphics[0]->data.spice.jpeg) - virBufferAsprintf(&opt, ",jpeg-wan-compression=%s", - virDomainGraphicsSpiceJpegCompressionTypeToString(def->graphics[0]->data.spice.jpeg)); - if (def->graphics[0]->data.spice.zlib) - virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s", - virDomainGraphicsSpiceZlibCompressionTypeToString(def->graphics[0]->data.spice.zlib)); - if (def->graphics[0]->data.spice.playback) - virBufferAsprintf(&opt, ",playback-compression=%s", - virDomainGraphicsSpicePlaybackCompressionTypeToString(def->graphics[0]->data.spice.playback)); - if (def->graphics[0]->data.spice.streaming) - virBufferAsprintf(&opt, ",streaming-video=%s", - virDomainGraphicsSpiceStreamingModeTypeToString(def->graphics[0]->data.spice.streaming)); - if (def->graphics[0]->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) - virBufferAddLit(&opt, ",disable-copy-paste"); - - if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) { - /* If qemu supports seamless migration turn it - * unconditionally on. If migration destination - * doesn't support it, it fallbacks to previous - * migration algorithm silently. */ - virBufferAddLit(&opt, ",seamless-migration=on"); - } - - virCommandAddArg(cmd, "-spice"); - virCommandAddArgBuffer(cmd, &opt); - 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 - */ - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); - - } else if ((def->ngraphics == 1)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported graphics type '%s'"), - virDomainGraphicsTypeToString(def->graphics[0]->type)); - goto error; } - if (def->nvideos > 0) { if (qemuCapsGet(caps, QEMU_CAPS_VGA)) { if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_XEN) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d8cf4c3..2a77290 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2081,16 +2081,17 @@ qemuProcessInitPasswords(virConnectPtr conn, int ret = 0; qemuDomainObjPrivatePtr priv = vm->privateData; - if (vm->def->ngraphics == 1) { - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + for (int i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { ret = qemuDomainChangeGraphicsPasswords(driver, vm, VIR_DOMAIN_GRAPHICS_TYPE_VNC, - &vm->def->graphics[0]->data.vnc.auth, + &graphics->data.vnc.auth, driver->vncPassword); - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { ret = qemuDomainChangeGraphicsPasswords(driver, vm, VIR_DOMAIN_GRAPHICS_TYPE_SPICE, - &vm->def->graphics[0]->data.spice.auth, + &graphics->data.spice.auth, driver->spicePassword); } } @@ -3484,21 +3485,22 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Ensuring no historical cgroup is lying around"); qemuRemoveCgroup(driver, vm, 1); - if (vm->def->ngraphics == 1) { - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - !vm->def->graphics[0]->data.vnc.socket && - vm->def->graphics[0]->data.vnc.autoport) { + for (i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !graphics->data.vnc.socket && + graphics->data.vnc.autoport) { int port = qemuProcessNextFreePort(driver, driver->remotePortMin); if (port < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for VNC")); goto cleanup; } - vm->def->graphics[0]->data.vnc.port = port; - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + graphics->data.vnc.port = port; + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { int port = -1; - if (vm->def->graphics[0]->data.spice.autoport || - vm->def->graphics[0]->data.spice.port == -1) { + if (graphics->data.spice.autoport || + graphics->data.spice.port == -1) { port = qemuProcessNextFreePort(driver, driver->remotePortMin); if (port < 0) { @@ -3507,13 +3509,13 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } - vm->def->graphics[0]->data.spice.port = port; + graphics->data.spice.port = port; } if (driver->spiceTLS && - (vm->def->graphics[0]->data.spice.autoport || - vm->def->graphics[0]->data.spice.tlsPort == -1)) { + (graphics->data.spice.autoport || + graphics->data.spice.tlsPort == -1)) { int tlsPort = qemuProcessNextFreePort(driver, - vm->def->graphics[0]->data.spice.port + 1); + graphics->data.spice.port + 1); if (tlsPort < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for SPICE TLS")); @@ -3521,20 +3523,19 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } - vm->def->graphics[0]->data.spice.tlsPort = tlsPort; + graphics->data.spice.tlsPort = tlsPort; } } - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virDomainGraphicsDefPtr graphics = vm->def->graphics[0]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { if (graphics->nListens == 0) { if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0) { virReportOOMError(); goto cleanup; } graphics->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) graphics->listens[0].address = strdup(driver->vncListen); else graphics->listens[0].address = strdup(driver->spiceListen); @@ -4150,19 +4151,20 @@ retry: qemuProcessRemoveDomainStatus(driver, vm); - /* Remove VNC port from port reservation bitmap, but only if it was - reserved by the driver (autoport=yes) + /* Remove VNC and Spice ports from port reservation bitmap, but only if + they were reserved by the driver (autoport=yes) */ - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - vm->def->graphics[0]->data.vnc.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.vnc.port); - } - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - vm->def->graphics[0]->data.spice.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.port); - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.tlsPort); + for (i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + graphics->data.vnc.autoport) { + qemuProcessReturnPort(driver, graphics->data.vnc.port); + } + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + graphics->data.spice.autoport) { + qemuProcessReturnPort(driver, graphics->data.spice.port); + qemuProcessReturnPort(driver, graphics->data.spice.tlsPort); + } } vm->taint = 0; -- 1.8.0

--- src/qemu/qemu_command.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba99c7a..a1a4523 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6183,9 +6183,26 @@ qemuBuildCommandLine(virConnectPtr conn, } if (def->ngraphics > 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("only 1 graphics device is supported")); - goto error; + int sdl = 0, vnc = 0, spice = 0; + for (i = 0 ; i < def->ngraphics ; ++i) { + switch (def->graphics[i]->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + ++sdl; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + ++vnc; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + ++spice; + break; + } + } + if (sdl > 1 || vnc > 1 || spice > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("only 1 graphics device of each type " + "(sdl, vnc, spice) is supported")); + goto error; + } } for (i = 0 ; i < def->ngraphics ; ++i) { -- 1.8.0

On Sat, Nov 10, 2012 at 02:40:24 +0100, Alon Levy wrote:
--- src/qemu/qemu_command.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba99c7a..a1a4523 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6183,9 +6183,26 @@ qemuBuildCommandLine(virConnectPtr conn, }
if (def->ngraphics > 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("only 1 graphics device is supported")); - goto error; + int sdl = 0, vnc = 0, spice = 0; + for (i = 0 ; i < def->ngraphics ; ++i) { + switch (def->graphics[i]->type) { + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: + ++sdl; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: + ++vnc; + break; + case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + ++spice; + break; + } + } + if (sdl > 1 || vnc > 1 || spice > 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("only 1 graphics device of each type " + "(sdl, vnc, spice) is supported")); + goto error; + } }
We don't need the extra if (def->ngraphics > 1) around this code. ACK with that removed. Jirka

On Sat, Nov 10, 2012 at 15:20:24 +0100, Jiri Denemark wrote:
On Sat, Nov 10, 2012 at 02:40:24 +0100, Alon Levy wrote:
ACK with that removed.
Actually, the ACK might have been a bit premature? What QEMU version is needed to support multiple graphics devices? And what happens if we try to use them with older version? Will we get a reasonable error message? Jirka

On Mon, Nov 12, 2012 at 10:23:18AM +0100, Jiri Denemark wrote:
On Sat, Nov 10, 2012 at 15:20:24 +0100, Jiri Denemark wrote:
On Sat, Nov 10, 2012 at 02:40:24 +0100, Alon Levy wrote:
ACK with that removed.
Actually, the ACK might have been a bit premature? What QEMU version is needed to support multiple graphics devices? And what happens if we try to use them with older version? Will we get a reasonable error message?
IIRC this was introduced at the same time that the explicit '-sdl' arg was added - previously SDL was implicit if no -vnc was given, so to allow multiple graphics, they needed to add the explicit -sdl arg. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Sat, Nov 10, 2012 at 15:20:24 +0100, Jiri Denemark wrote:
On Sat, Nov 10, 2012 at 02:40:24 +0100, Alon Levy wrote:
ACK with that removed.
Actually, the ACK might have been a bit premature? What QEMU version is needed to support multiple graphics devices? And what happens if we try to use them with older version? Will we get a reasonable error message?
I'm not sure what version of qemu supports this - it has been for a while, but maybe it's safest to say 1.3.0 and if anyone wants it to work for earlier versions they can send a patch.
Jirka

--- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a1a4523..02a28b3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -788,7 +788,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, qemuCapsPtr caps) return 0; - no_memory: +no_memory: virReportOOMError(); return -1; } -- 1.8.0

On Sat, Nov 10, 2012 at 02:40:25 +0100, Alon Levy wrote:
--- src/qemu/qemu_command.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a1a4523..02a28b3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -788,7 +788,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, qemuCapsPtr caps)
return 0;
- no_memory: +no_memory: virReportOOMError(); return -1; }
ACK Jirka

On Sat, Nov 10, 2012 at 02:40:23 +0100, Alon Levy wrote:
The check for a single display remains so no new functionality is added. --- v2 changes: removed enum, use virReportOOMError directly. use --patience added a one line label fix patch.
The second patch changes a string that needs to be translated - how do I go about it?
src/qemu/qemu_command.c | 641 +++++++++++++++++++++++++----------------------- src/qemu/qemu_process.c | 70 +++--- 2 files changed, 364 insertions(+), 347 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1e96982..ba99c7a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4410,6 +4410,331 @@ error: return -1; }
+static int +qemuBuildGraphicsCommandLine(struct qemud_driver *driver, + virCommandPtr cmd, + virDomainDefPtr def, + qemuCapsPtr caps, + virDomainGraphicsDefPtr graphics) +{ + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + virBuffer opt = VIR_BUFFER_INITIALIZER; + + if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vnc graphics are not supported with this QEMU")); + goto error; + } + + if (graphics->data.vnc.socket || + driver->vncAutoUnixSocket) { + + if (!graphics->data.vnc.socket && + virAsprintf(&graphics->data.vnc.socket, + "%s/%s.vnc", driver->libDir, def->name) == -1) { + goto no_memory; + } + + virBufferAsprintf(&opt, "unix:%s", + graphics->data.vnc.socket); + + } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + bool escapeAddr; + int ret; + + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + goto error; + } + if (ret < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + goto error; + } + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it will + * show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + goto error; + break; + } + + if (!listenAddr) + listenAddr = driver->vncListen; + + escapeAddr = strchr(listenAddr, ':') != NULL; + if (escapeAddr) + virBufferAsprintf(&opt, "[%s]", listenAddr); + else + virBufferAdd(&opt, listenAddr, -1); + virBufferAsprintf(&opt, ":%d", + graphics->data.vnc.port - 5900); + + VIR_FREE(netAddr); + } else { + virBufferAsprintf(&opt, "%d", + graphics->data.vnc.port - 5900); + } + + if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { + if (graphics->data.vnc.auth.passwd || + driver->vncPassword) + virBufferAddLit(&opt, ",password"); + + if (driver->vncTLS) { + virBufferAddLit(&opt, ",tls"); + if (driver->vncTLSx509verify) { + virBufferAsprintf(&opt, ",x509verify=%s", + driver->vncTLSx509certdir); + } else { + virBufferAsprintf(&opt, ",x509=%s", + driver->vncTLSx509certdir); + } + } + + if (driver->vncSASL) { + virBufferAddLit(&opt, ",sasl"); + + if (driver->vncSASLdir) + virCommandAddEnvPair(cmd, "SASL_CONF_DIR", + driver->vncSASLdir); + + /* TODO: Support ACLs later */ + } + } + + virCommandAddArg(cmd, "-vnc"); + virCommandAddArgBuffer(cmd, &opt); + if (graphics->data.vnc.keymap) { + virCommandAddArgList(cmd, "-k", graphics->data.vnc.keymap, + NULL); + } + + /* Unless user requested it, set the audio backend to none, to + * prevent it opening the host OS audio devices, since that causes + * security issues and might not work when using VNC. + */ + if (driver->vncAllowHostAudio) { + virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); + } else { + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); + } + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { + if (qemuCapsGet(caps, QEMU_CAPS_0_10) && + !qemuCapsGet(caps, QEMU_CAPS_SDL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("sdl not supported by '%s'"), + def->emulator); + goto error; + } + + if (graphics->data.sdl.xauth) + virCommandAddEnvPair(cmd, "XAUTHORITY", + graphics->data.sdl.xauth); + if (graphics->data.sdl.display) + virCommandAddEnvPair(cmd, "DISPLAY", + graphics->data.sdl.display); + if (graphics->data.sdl.fullscreen) + 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 + */ + 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 (qemuCapsGet(caps, QEMU_CAPS_SDL)) + virCommandAddArg(cmd, "-sdl"); + + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *listenNetwork; + const char *listenAddr = NULL; + char *netAddr = NULL; + int ret; + int defaultMode = graphics->data.spice.defaultMode; + + if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice graphics are not supported with this QEMU")); + goto error; + } + + virBufferAsprintf(&opt, "port=%u", graphics->data.spice.port); + + if (graphics->data.spice.tlsPort > 0) { + if (!driver->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice TLS port set in XML configuration," + " but TLS is disabled in qemu.conf")); + return 1;
This should be goto error;
+ } + virBufferAsprintf(&opt, ",tls-port=%u", + graphics->data.spice.tlsPort); + } + + switch (virDomainGraphicsListenGetType(graphics, 0)) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); + break; + + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); + if (!listenNetwork) + break; + ret = networkGetNetworkAddress(listenNetwork, &netAddr); + if (ret <= -2) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); + goto error; + } + if (ret < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("listen network '%s' had no usable address"), + listenNetwork); + goto error; + } + listenAddr = netAddr; + /* store the address we found in the <graphics> element so it will + * show up in status. */ + if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) + goto error; + break; + } + + if (!listenAddr) + listenAddr = driver->spiceListen; + if (listenAddr) + virBufferAsprintf(&opt, ",addr=%s", listenAddr); + + VIR_FREE(netAddr); + + int mm = graphics->data.spice.mousemode; + if (mm) { + switch (mm) { + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: + virBufferAsprintf(&opt, ",agent-mouse=off"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: + virBufferAsprintf(&opt, ",agent-mouse=on"); + break; + default: + break; + } + } + + /* In the password case we set it via monitor command, to avoid + * making it visible on CLI, so there's no use of password=XXX + * in this bit of the code */ + if (!graphics->data.spice.auth.passwd && + !driver->spicePassword) + virBufferAddLit(&opt, ",disable-ticketing"); + + if (driver->spiceTLS) + virBufferAsprintf(&opt, ",x509-dir=%s", + driver->spiceTLSx509certdir); + + switch (defaultMode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + virBufferAsprintf(&opt, ",tls-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + /* nothing */ + break; + } + + for (int i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { + int mode = graphics->data.spice.channels[i]; + switch (mode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + if (!driver->spiceTLS) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf")); + return 1;
goto error;
+ } + virBufferAsprintf(&opt, ",tls-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=%s", + virDomainGraphicsSpiceChannelNameTypeToString(i)); + break; + } + } + if (graphics->data.spice.image) + virBufferAsprintf(&opt, ",image-compression=%s", + virDomainGraphicsSpiceImageCompressionTypeToString(graphics->data.spice.image)); + if (graphics->data.spice.jpeg) + virBufferAsprintf(&opt, ",jpeg-wan-compression=%s", + virDomainGraphicsSpiceJpegCompressionTypeToString(graphics->data.spice.jpeg)); + if (graphics->data.spice.zlib) + virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s", + virDomainGraphicsSpiceZlibCompressionTypeToString(graphics->data.spice.zlib)); + if (graphics->data.spice.playback) + virBufferAsprintf(&opt, ",playback-compression=%s", + virDomainGraphicsSpicePlaybackCompressionTypeToString(graphics->data.spice.playback)); + if (graphics->data.spice.streaming) + virBufferAsprintf(&opt, ",streaming-video=%s", + virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming)); + if (graphics->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) + virBufferAddLit(&opt, ",disable-copy-paste"); + + if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) { + /* If qemu supports seamless migration turn it + * unconditionally on. If migration destination + * doesn't support it, it fallbacks to previous + * migration algorithm silently. */ + virBufferAddLit(&opt, ",seamless-migration=on"); + } + + virCommandAddArg(cmd, "-spice"); + virCommandAddArgBuffer(cmd, &opt); + if (graphics->data.spice.keymap) + virCommandAddArgList(cmd, "-k", + graphics->data.spice.keymap, NULL); + /* SPICE includes native support for tunnelling audio, so we + * set the audio backend to point at SPICE's own driver + */ + virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); + + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported graphics type '%s'"), + virDomainGraphicsTypeToString(graphics->type)); + goto error; + } +no_memory: + virReportOOMError(); +error: + return 0; +} +
The end of this function is wrong, it would always report no memory and return success. The following is how it should like: return 0; no_memory: virReportOOMError(); error: return -1; Alternatively, all ``goto error'' statements could be replaced with ``return -1'' but this way minimizes modifications to the code being moved so I'm fine with it as-is.
/* * Constructs a argv suitable for launching qemu with config defined * for a given virtual machine. @@ -5863,322 +6188,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; }
- if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - virBuffer opt = VIR_BUFFER_INITIALIZER; - - if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("vnc graphics are not supported with this QEMU")); + for (i = 0 ; i < def->ngraphics ; ++i) { + if (qemuBuildGraphicsCommandLine(driver, cmd, def, caps, + def->graphics[i]) == -1) {
"def->graphics[i]" should be aligned with "driver" above.
goto error; } - - if (def->graphics[0]->data.vnc.socket || - driver->vncAutoUnixSocket) { - - if (!def->graphics[0]->data.vnc.socket && - virAsprintf(&def->graphics[0]->data.vnc.socket, - "%s/%s.vnc", driver->libDir, def->name) == -1) { - goto no_memory; - } - - virBufferAsprintf(&opt, "unix:%s", - def->graphics[0]->data.vnc.socket); - - } else if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { - const char *listenNetwork; - const char *listenAddr = NULL; - char *netAddr = NULL; - bool escapeAddr; - int ret; - - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); - goto error; - } - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0, - listenAddr, -1, false) < 0) - goto error; - break; - } - - if (!listenAddr) - listenAddr = driver->vncListen; - - escapeAddr = strchr(listenAddr, ':') != NULL; - if (escapeAddr) - virBufferAsprintf(&opt, "[%s]", listenAddr); - else - virBufferAdd(&opt, listenAddr, -1); - virBufferAsprintf(&opt, ":%d", - def->graphics[0]->data.vnc.port - 5900); - - VIR_FREE(netAddr); - } else { - virBufferAsprintf(&opt, "%d", - def->graphics[0]->data.vnc.port - 5900); - } - - if (qemuCapsGet(caps, QEMU_CAPS_VNC_COLON)) { - if (def->graphics[0]->data.vnc.auth.passwd || - driver->vncPassword) - virBufferAddLit(&opt, ",password"); - - if (driver->vncTLS) { - virBufferAddLit(&opt, ",tls"); - if (driver->vncTLSx509verify) { - virBufferAsprintf(&opt, ",x509verify=%s", - driver->vncTLSx509certdir); - } else { - virBufferAsprintf(&opt, ",x509=%s", - driver->vncTLSx509certdir); - } - } - - if (driver->vncSASL) { - virBufferAddLit(&opt, ",sasl"); - - if (driver->vncSASLdir) - virCommandAddEnvPair(cmd, "SASL_CONF_DIR", - driver->vncSASLdir); - - /* TODO: Support ACLs later */ - } - } - - virCommandAddArg(cmd, "-vnc"); - virCommandAddArgBuffer(cmd, &opt); - if (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 - * prevent it opening the host OS audio devices, since that causes - * security issues and might not work when using VNC. - */ - if (driver->vncAllowHostAudio) { - virCommandAddEnvPass(cmd, "QEMU_AUDIO_DRV"); - } else { - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=none"); - } - } else if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { - if (qemuCapsGet(caps, QEMU_CAPS_0_10) && - !qemuCapsGet(caps, QEMU_CAPS_SDL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("sdl not supported by '%s'"), - def->emulator); - goto error; - } - - 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) - 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 - */ - 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 (qemuCapsGet(caps, QEMU_CAPS_SDL)) - virCommandAddArg(cmd, "-sdl"); - - } else if ((def->ngraphics == 1) && - def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virBuffer opt = VIR_BUFFER_INITIALIZER; - const char *listenNetwork; - const char *listenAddr = NULL; - char *netAddr = NULL; - int ret; - int defaultMode = def->graphics[0]->data.spice.defaultMode; - - if (!qemuCapsGet(caps, QEMU_CAPS_SPICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice graphics are not supported with this QEMU")); - goto error; - } - - virBufferAsprintf(&opt, "port=%u", def->graphics[0]->data.spice.port); - - if (def->graphics[0]->data.spice.tlsPort > 0) { - if (!driver->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice TLS port set in XML configuration," - " but TLS is disabled in qemu.conf")); - goto error; - } - virBufferAsprintf(&opt, ",tls-port=%u", - def->graphics[0]->data.spice.tlsPort); - } - - switch (virDomainGraphicsListenGetType(def->graphics[0], 0)) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); - break; - - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - listenNetwork = virDomainGraphicsListenGetNetwork(def->graphics[0], 0); - if (!listenNetwork) - break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); - if (ret <= -2) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); - goto error; - } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); - goto error; - } - listenAddr = netAddr; - /* store the address we found in the <graphics> element so it will - * show up in status. */ - if (virDomainGraphicsListenSetAddress(def->graphics[0], 0, - listenAddr, -1, false) < 0) - goto error; - break; - } - - if (!listenAddr) - listenAddr = driver->spiceListen; - if (listenAddr) - virBufferAsprintf(&opt, ",addr=%s", listenAddr); - - VIR_FREE(netAddr); - - int mm = def->graphics[0]->data.spice.mousemode; - if (mm) { - switch (mm) { - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER: - virBufferAsprintf(&opt, ",agent-mouse=off"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT: - virBufferAsprintf(&opt, ",agent-mouse=on"); - break; - default: - break; - } - } - - /* In the password case we set it via monitor command, to avoid - * making it visible on CLI, so there's no use of password=XXX - * in this bit of the code */ - if (!def->graphics[0]->data.spice.auth.passwd && - !driver->spicePassword) - virBufferAddLit(&opt, ",disable-ticketing"); - - if (driver->spiceTLS) - virBufferAsprintf(&opt, ",x509-dir=%s", - driver->spiceTLSx509certdir); - - switch (defaultMode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - virBufferAsprintf(&opt, ",tls-channel=default"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAsprintf(&opt, ",plaintext-channel=default"); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: - /* nothing */ - break; - } - - for (i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { - int mode = def->graphics[0]->data.spice.channels[i]; - switch (mode) { - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: - if (!driver->spiceTLS) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spice secure channels set in XML configuration, but TLS is disabled in qemu.conf")); - goto error; - } - virBufferAsprintf(&opt, ",tls-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); - break; - case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: - virBufferAsprintf(&opt, ",plaintext-channel=%s", - virDomainGraphicsSpiceChannelNameTypeToString(i)); - break; - } - } - if (def->graphics[0]->data.spice.image) - virBufferAsprintf(&opt, ",image-compression=%s", - virDomainGraphicsSpiceImageCompressionTypeToString(def->graphics[0]->data.spice.image)); - if (def->graphics[0]->data.spice.jpeg) - virBufferAsprintf(&opt, ",jpeg-wan-compression=%s", - virDomainGraphicsSpiceJpegCompressionTypeToString(def->graphics[0]->data.spice.jpeg)); - if (def->graphics[0]->data.spice.zlib) - virBufferAsprintf(&opt, ",zlib-glz-wan-compression=%s", - virDomainGraphicsSpiceZlibCompressionTypeToString(def->graphics[0]->data.spice.zlib)); - if (def->graphics[0]->data.spice.playback) - virBufferAsprintf(&opt, ",playback-compression=%s", - virDomainGraphicsSpicePlaybackCompressionTypeToString(def->graphics[0]->data.spice.playback)); - if (def->graphics[0]->data.spice.streaming) - virBufferAsprintf(&opt, ",streaming-video=%s", - virDomainGraphicsSpiceStreamingModeTypeToString(def->graphics[0]->data.spice.streaming)); - if (def->graphics[0]->data.spice.copypaste == VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_NO) - virBufferAddLit(&opt, ",disable-copy-paste"); - - if (qemuCapsGet(caps, QEMU_CAPS_SEAMLESS_MIGRATION)) { - /* If qemu supports seamless migration turn it - * unconditionally on. If migration destination - * doesn't support it, it fallbacks to previous - * migration algorithm silently. */ - virBufferAddLit(&opt, ",seamless-migration=on"); - } - - virCommandAddArg(cmd, "-spice"); - virCommandAddArgBuffer(cmd, &opt); - 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 - */ - virCommandAddEnvString(cmd, "QEMU_AUDIO_DRV=spice"); - - } else if ((def->ngraphics == 1)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported graphics type '%s'"), - virDomainGraphicsTypeToString(def->graphics[0]->type)); - goto error; } - if (def->nvideos > 0) { if (qemuCapsGet(caps, QEMU_CAPS_VGA)) { if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_XEN) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d8cf4c3..2a77290 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2081,16 +2081,17 @@ qemuProcessInitPasswords(virConnectPtr conn, int ret = 0; qemuDomainObjPrivatePtr priv = vm->privateData;
- if (vm->def->ngraphics == 1) { - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + for (int i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) { ret = qemuDomainChangeGraphicsPasswords(driver, vm, VIR_DOMAIN_GRAPHICS_TYPE_VNC, - &vm->def->graphics[0]->data.vnc.auth, + &graphics->data.vnc.auth, driver->vncPassword); - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { ret = qemuDomainChangeGraphicsPasswords(driver, vm, VIR_DOMAIN_GRAPHICS_TYPE_SPICE, - &vm->def->graphics[0]->data.spice.auth, + &graphics->data.spice.auth, driver->spicePassword); } } @@ -3484,21 +3485,22 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Ensuring no historical cgroup is lying around"); qemuRemoveCgroup(driver, vm, 1);
- if (vm->def->ngraphics == 1) { - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - !vm->def->graphics[0]->data.vnc.socket && - vm->def->graphics[0]->data.vnc.autoport) { + for (i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + !graphics->data.vnc.socket && + graphics->data.vnc.autoport) { int port = qemuProcessNextFreePort(driver, driver->remotePortMin); if (port < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for VNC")); goto cleanup; } - vm->def->graphics[0]->data.vnc.port = port; - } else if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + graphics->data.vnc.port = port; + } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { int port = -1; - if (vm->def->graphics[0]->data.spice.autoport || - vm->def->graphics[0]->data.spice.port == -1) { + if (graphics->data.spice.autoport || + graphics->data.spice.port == -1) { port = qemuProcessNextFreePort(driver, driver->remotePortMin);
if (port < 0) { @@ -3507,13 +3509,13 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; }
- vm->def->graphics[0]->data.spice.port = port; + graphics->data.spice.port = port; } if (driver->spiceTLS && - (vm->def->graphics[0]->data.spice.autoport || - vm->def->graphics[0]->data.spice.tlsPort == -1)) { + (graphics->data.spice.autoport || + graphics->data.spice.tlsPort == -1)) { int tlsPort = qemuProcessNextFreePort(driver, - vm->def->graphics[0]->data.spice.port + 1); + graphics->data.spice.port + 1); if (tlsPort < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find an unused port for SPICE TLS")); @@ -3521,20 +3523,19 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; }
- vm->def->graphics[0]->data.spice.tlsPort = tlsPort; + graphics->data.spice.tlsPort = tlsPort; } }
- if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virDomainGraphicsDefPtr graphics = vm->def->graphics[0]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { if (graphics->nListens == 0) { if (VIR_EXPAND_N(graphics->listens, graphics->nListens, 1) < 0) { virReportOOMError(); goto cleanup; } graphics->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; - if (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) graphics->listens[0].address = strdup(driver->vncListen); else graphics->listens[0].address = strdup(driver->spiceListen); @@ -4150,19 +4151,20 @@ retry:
qemuProcessRemoveDomainStatus(driver, vm);
- /* Remove VNC port from port reservation bitmap, but only if it was - reserved by the driver (autoport=yes) + /* Remove VNC and Spice ports from port reservation bitmap, but only if + they were reserved by the driver (autoport=yes) */ - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - vm->def->graphics[0]->data.vnc.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.vnc.port); - } - if ((vm->def->ngraphics == 1) && - vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - vm->def->graphics[0]->data.spice.autoport) { - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.port); - qemuProcessReturnPort(driver, vm->def->graphics[0]->data.spice.tlsPort); + for (i = 0 ; i < vm->def->ngraphics; ++i) { + virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && + graphics->data.vnc.autoport) { + qemuProcessReturnPort(driver, graphics->data.vnc.port); + } + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + graphics->data.spice.autoport) { + qemuProcessReturnPort(driver, graphics->data.spice.port); + qemuProcessReturnPort(driver, graphics->data.spice.tlsPort); + } }
vm->taint = 0;
I'd prefer if the refactoring of qemuBuildCommandLine was separated from removing the limit for number of graphics cards. But since I already reviewed the patch and I don't want to do that again, I'm giving a formal ACK to this version (with the small issues fixed, of course). Since you don't have commit rights, I'll fix the issues and push the patches. Jirka

[snip]
I'd prefer if the refactoring of qemuBuildCommandLine was separated from removing the limit for number of graphics cards. But since I already reviewed the patch and I don't want to do that again, I'm giving a formal ACK to this version (with the small issues fixed, of course).
Since you don't have commit rights, I'll fix the issues and push the patches.
Thanks for the review, it's all correct, I missed all those returns, and got the default path wrong - seems libvirtd doesn't mind the no memory report and just continued, since it didn't affect my testing. Regarding the split I did leave a second patch that removes the limit check - so the first patch converts if blocks to a single iteration loop.
Jirka
participants (5)
-
Alon Levy
-
Daniel P. Berrange
-
Eric Blake
-
Jiri Denemark
-
Peter Krempa