[libvirt] [PATCH 0/10] Convert over to use new -device args everywhere

QEMU has introduced a new command line arg called -device for specifying the guest device config. This allows separation from the host device config. libvirt needs to use this new syntax so that we can specify static PCI addresses for devices (the old syntax did not allow this). This patch series is the quick conversion I've done so far. The most obvious thing still lacking here is new test cases. I want to wait until this series merges with the device addressing series before adding those, since one set of tests can cover both patch series. In addition the handling of disk drivers is not quite correct. We need the disk controller patches myself/Wolfgang have worked on in order to specify the disk bus/unit instead of index.

To enable it to be called from multiple locations, split out the code for building the -drive arg string * src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Add qemuBuildDriveStr for building -drive arg string --- src/qemu/qemu_conf.c | 188 +++++++++++++++++++++++++++---------------------- src/qemu/qemu_conf.h | 7 ++ 2 files changed, 111 insertions(+), 84 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 86172c6..61e719d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1313,6 +1313,108 @@ qemuAssignNetNames(virDomainDefPtr def, return 0; } +#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" + +static int +qemuSafeSerialParamValue(virConnectPtr conn, + const char *value) +{ + if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen (value)) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("driver serial '%s' contains unsafe characters"), + value); + return -1; + } + + return 0; +} + + +int +qemuBuildDriveStr(virConnectPtr conn, + virDomainDiskDefPtr disk, + int bootable, + int qemuCmdFlags, + char **str) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); + int idx = virDiskNameToIndex(disk->dst); + + if (idx < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unsupported disk type '%s'"), disk->dst); + goto error; + } + + if (disk->src) { + if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { + /* QEMU only supports magic FAT format for now */ + if (disk->driverType && + STRNEQ(disk->driverType, "fat")) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unsupported disk driver type for '%s'"), + disk->driverType); + goto error; + } + if (!disk->readonly) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot create virtual FAT disks in read-write mode")); + goto error; + } + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + virBufferVSprintf(&opt, "file=fat:floppy:%s,", disk->src); + else + virBufferVSprintf(&opt, "file=fat:%s,", disk->src); + } else { + virBufferVSprintf(&opt, "file=%s,", disk->src); + } + } + virBufferVSprintf(&opt, "if=%s", bus); + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) + virBufferAddLit(&opt, ",media=cdrom"); + virBufferVSprintf(&opt, ",index=%d", idx); + if (bootable && + disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) + virBufferAddLit(&opt, ",boot=on"); + if (disk->driverType && + disk->type != VIR_DOMAIN_DISK_TYPE_DIR && + qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) + virBufferVSprintf(&opt, ",format=%s", disk->driverType); + if (disk->serial && + (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_SERIAL)) { + if (qemuSafeSerialParamValue(conn, disk->serial) < 0) + goto error; + virBufferVSprintf(&opt, ",serial=%s", disk->serial); + } + + if (disk->cachemode) { + const char *mode = + (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_CACHE_V2) ? + qemuDiskCacheV2TypeToString(disk->cachemode) : + qemuDiskCacheV1TypeToString(disk->cachemode); + + virBufferVSprintf(&opt, ",cache=%s", mode); + } else if (disk->shared && !disk->readonly) { + virBufferAddLit(&opt, ",cache=off"); + } + + if (virBufferError(&opt)) { + virReportOOMError(conn); + goto error; + } + + *str = virBufferContentAndReset(&opt); + return 0; + +error: + virBufferFreeAndReset(&opt); + *str = NULL; + return -1; +} + + int qemuBuildNicStr(virConnectPtr conn, virDomainNetDefPtr net, @@ -1563,23 +1665,6 @@ static void qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, } } -#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \ - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" - -static int -qemuSafeSerialParamValue(virConnectPtr conn, - const char *value) -{ - if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen (value)) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("driver serial '%s' contains unsafe characters"), - value); - return -1; - } - - return 0; -} - /* * Constructs a argv suitable for launching qemu with config defined * for a given virtual machine. @@ -1983,12 +2068,9 @@ int qemudBuildCommandLine(virConnectPtr conn, } for (i = 0 ; i < def->ndisks ; i++) { - virBuffer opt = VIR_BUFFER_INITIALIZER; char *optstr; int bootable = 0; virDomainDiskDefPtr disk = def->disks[i]; - int idx = virDiskNameToIndex(disk->dst); - const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { @@ -2003,12 +2085,6 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_SPACE; - if (idx < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("unsupported disk type '%s'"), disk->dst); - goto error; - } - switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: bootable = bootCD; @@ -2024,64 +2100,8 @@ int qemudBuildCommandLine(virConnectPtr conn, break; } - if (disk->src) { - if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { - /* QEMU only supports magic FAT format for now */ - if (disk->driverType && - STRNEQ(disk->driverType, "fat")) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("unsupported disk driver type for '%s'"), - disk->driverType); - goto error; - } - if (!disk->readonly) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create virtual FAT disks in read-write mode")); - goto error; - } - if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferVSprintf(&opt, "file=fat:floppy:%s,", disk->src); - else - virBufferVSprintf(&opt, "file=fat:%s,", disk->src); - } else { - virBufferVSprintf(&opt, "file=%s,", disk->src); - } - } - virBufferVSprintf(&opt, "if=%s", bus); - if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) - virBufferAddLit(&opt, ",media=cdrom"); - virBufferVSprintf(&opt, ",index=%d", idx); - if (bootable && - disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) - virBufferAddLit(&opt, ",boot=on"); - if (disk->driverType && - disk->type != VIR_DOMAIN_DISK_TYPE_DIR && - qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) - virBufferVSprintf(&opt, ",format=%s", disk->driverType); - if (disk->serial && - (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_SERIAL)) { - if (qemuSafeSerialParamValue(conn, disk->serial) < 0) - goto error; - virBufferVSprintf(&opt, ",serial=%s", disk->serial); - } - - if (disk->cachemode) { - const char *mode = - (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_CACHE_V2) ? - qemuDiskCacheV2TypeToString(disk->cachemode) : - qemuDiskCacheV1TypeToString(disk->cachemode); - - virBufferVSprintf(&opt, ",cache=%s", mode); - } else if (disk->shared && !disk->readonly) { - virBufferAddLit(&opt, ",cache=off"); - } - - if (virBufferError(&opt)) { - virBufferFreeAndReset(&opt); - goto no_memory; - } - - optstr = virBufferContentAndReset(&opt); + if (qemuBuildDriveStr(conn, disk, bootable, qemuCmdFlags, &optstr) < 0) + goto error; if ((qargv[qargc++] = strdup("-drive")) == NULL) { VIR_FREE(optstr); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 248677a..cd59d4c 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -191,6 +191,13 @@ int qemuBuildNicStr (virConnectPtr conn, int vlan, char **str); + +int qemuBuildDriveStr (virConnectPtr conn, + virDomainDiskDefPtr disk, + int bootable, + int qemuCmdFlags, + char **str); + int qemudNetworkIfaceConnect (virConnectPtr conn, struct qemud_driver *driver, virDomainNetDefPtr net, -- 1.6.5.2

On Tue, Dec 15, 2009 at 03:14:41PM +0000, Daniel P. Berrange wrote:
To enable it to be called from multiple locations, split out the code for building the -drive arg string
* src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Add qemuBuildDriveStr for building -drive arg string --- src/qemu/qemu_conf.c | 188 +++++++++++++++++++++++++++---------------------- src/qemu/qemu_conf.h | 7 ++ 2 files changed, 111 insertions(+), 84 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 86172c6..61e719d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1313,6 +1313,108 @@ qemuAssignNetNames(virDomainDefPtr def, return 0; }
+#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" + +static int +qemuSafeSerialParamValue(virConnectPtr conn, + const char *value) +{ + if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen (value)) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("driver serial '%s' contains unsafe characters"), + value); + return -1; + } + + return 0; +} + + +int +qemuBuildDriveStr(virConnectPtr conn, + virDomainDiskDefPtr disk, + int bootable, + int qemuCmdFlags, + char **str) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); + int idx = virDiskNameToIndex(disk->dst); + + if (idx < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unsupported disk type '%s'"), disk->dst); + goto error; + } + + if (disk->src) { + if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { + /* QEMU only supports magic FAT format for now */ + if (disk->driverType && + STRNEQ(disk->driverType, "fat")) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unsupported disk driver type for '%s'"), + disk->driverType); + goto error; + } + if (!disk->readonly) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot create virtual FAT disks in read-write mode")); + goto error; + } + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + virBufferVSprintf(&opt, "file=fat:floppy:%s,", disk->src); + else + virBufferVSprintf(&opt, "file=fat:%s,", disk->src); + } else { + virBufferVSprintf(&opt, "file=%s,", disk->src); + } + } + virBufferVSprintf(&opt, "if=%s", bus); + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) + virBufferAddLit(&opt, ",media=cdrom"); + virBufferVSprintf(&opt, ",index=%d", idx); + if (bootable && + disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) + virBufferAddLit(&opt, ",boot=on"); + if (disk->driverType && + disk->type != VIR_DOMAIN_DISK_TYPE_DIR && + qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) + virBufferVSprintf(&opt, ",format=%s", disk->driverType); + if (disk->serial && + (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_SERIAL)) { + if (qemuSafeSerialParamValue(conn, disk->serial) < 0) + goto error; + virBufferVSprintf(&opt, ",serial=%s", disk->serial); + } + + if (disk->cachemode) { + const char *mode = + (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_CACHE_V2) ? + qemuDiskCacheV2TypeToString(disk->cachemode) : + qemuDiskCacheV1TypeToString(disk->cachemode); + + virBufferVSprintf(&opt, ",cache=%s", mode); + } else if (disk->shared && !disk->readonly) { + virBufferAddLit(&opt, ",cache=off"); + } + + if (virBufferError(&opt)) { + virReportOOMError(conn); + goto error; + } + + *str = virBufferContentAndReset(&opt); + return 0; + +error: + virBufferFreeAndReset(&opt); + *str = NULL; + return -1; +} + + int qemuBuildNicStr(virConnectPtr conn, virDomainNetDefPtr net, @@ -1563,23 +1665,6 @@ static void qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, } }
-#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \ - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" - -static int -qemuSafeSerialParamValue(virConnectPtr conn, - const char *value) -{ - if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen (value)) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("driver serial '%s' contains unsafe characters"), - value); - return -1; - } - - return 0; -} - /* * Constructs a argv suitable for launching qemu with config defined * for a given virtual machine. @@ -1983,12 +2068,9 @@ int qemudBuildCommandLine(virConnectPtr conn, }
for (i = 0 ; i < def->ndisks ; i++) { - virBuffer opt = VIR_BUFFER_INITIALIZER; char *optstr; int bootable = 0; virDomainDiskDefPtr disk = def->disks[i]; - int idx = virDiskNameToIndex(disk->dst); - const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { @@ -2003,12 +2085,6 @@ int qemudBuildCommandLine(virConnectPtr conn,
ADD_ARG_SPACE;
- if (idx < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("unsupported disk type '%s'"), disk->dst); - goto error; - } - switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: bootable = bootCD; @@ -2024,64 +2100,8 @@ int qemudBuildCommandLine(virConnectPtr conn, break; }
- if (disk->src) { - if (disk->type == VIR_DOMAIN_DISK_TYPE_DIR) { - /* QEMU only supports magic FAT format for now */ - if (disk->driverType && - STRNEQ(disk->driverType, "fat")) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("unsupported disk driver type for '%s'"), - disk->driverType); - goto error; - } - if (!disk->readonly) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create virtual FAT disks in read-write mode")); - goto error; - } - if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferVSprintf(&opt, "file=fat:floppy:%s,", disk->src); - else - virBufferVSprintf(&opt, "file=fat:%s,", disk->src); - } else { - virBufferVSprintf(&opt, "file=%s,", disk->src); - } - } - virBufferVSprintf(&opt, "if=%s", bus); - if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) - virBufferAddLit(&opt, ",media=cdrom"); - virBufferVSprintf(&opt, ",index=%d", idx); - if (bootable && - disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) - virBufferAddLit(&opt, ",boot=on"); - if (disk->driverType && - disk->type != VIR_DOMAIN_DISK_TYPE_DIR && - qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) - virBufferVSprintf(&opt, ",format=%s", disk->driverType); - if (disk->serial && - (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_SERIAL)) { - if (qemuSafeSerialParamValue(conn, disk->serial) < 0) - goto error; - virBufferVSprintf(&opt, ",serial=%s", disk->serial); - } - - if (disk->cachemode) { - const char *mode = - (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_CACHE_V2) ? - qemuDiskCacheV2TypeToString(disk->cachemode) : - qemuDiskCacheV1TypeToString(disk->cachemode); - - virBufferVSprintf(&opt, ",cache=%s", mode); - } else if (disk->shared && !disk->readonly) { - virBufferAddLit(&opt, ",cache=off"); - } - - if (virBufferError(&opt)) { - virBufferFreeAndReset(&opt); - goto no_memory; - } - - optstr = virBufferContentAndReset(&opt); + if (qemuBuildDriveStr(conn, disk, bootable, qemuCmdFlags, &optstr) < 0) + goto error;
if ((qargv[qargc++] = strdup("-drive")) == NULL) { VIR_FREE(optstr); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 248677a..cd59d4c 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -191,6 +191,13 @@ int qemuBuildNicStr (virConnectPtr conn, int vlan, char **str);
+ +int qemuBuildDriveStr (virConnectPtr conn, + virDomainDiskDefPtr disk, + int bootable, + int qemuCmdFlags, + char **str); + int qemudNetworkIfaceConnect (virConnectPtr conn, struct qemud_driver *driver, virDomainNetDefPtr net,
ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

The current character device syntax uses either -serial tty,path=/dev/ttyS2 Or -chardev tty,id=serial0,path=/dev/ttyS2 -serial chardev:serial0 With the new -device support, we now prefer -chardev file,id=serial0,path=/tmp/serial.log -device isa-serial,chardev=serial0 This patch changes the existing -chardev syntax to use this new scheme, and fallbacks to the old plain -serial syntax for old QEMU. The monitor device changes to -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor In addition, this patch adds --nodefaults, which kills off the default serial, parallel, vga and nic devices. THis avoids the need for us to explicitly turn each off --- src/qemu/qemu_conf.c | 74 +++++++++++++------- src/qemu/qemu_conf.h | 1 + .../qemuxml2argv-channel-guestfwd.args | 2 +- .../qemuxml2argv-console-compat-chardev.args | 2 +- .../qemuxml2argv-parallel-tcp-chardev.args | 2 +- .../qemuxml2argv-serial-dev-chardev.args | 2 +- .../qemuxml2argv-serial-file-chardev.args | 2 +- .../qemuxml2argv-serial-many-chardev.args | 2 +- .../qemuxml2argv-serial-pty-chardev.args | 2 +- .../qemuxml2argv-serial-tcp-chardev.args | 2 +- .../qemuxml2argv-serial-tcp-telnet-chardev.args | 2 +- .../qemuxml2argv-serial-udp-chardev.args | 2 +- .../qemuxml2argv-serial-unix-chardev.args | 2 +- .../qemuxml2argv-serial-vc-chardev.args | 2 +- tests/qemuxml2argvtest.c | 26 ++++---- 15 files changed, 74 insertions(+), 51 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 61e719d..16e8d2c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -909,6 +909,8 @@ static unsigned int qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_MEM_PATH; if (strstr(help, "-chardev")) flags |= QEMUD_CMD_FLAG_CHARDEV; + if (strstr(help, "-device")) + flags |= QEMUD_CMD_FLAG_DEVICE; if (version >= 9000) flags |= QEMUD_CMD_FLAG_VNC_COLON; @@ -1945,6 +1947,9 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!def->graphics) ADD_ARG_LIT("-nographic"); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) + ADD_ARG_LIT("-nodefaults"); + if (monitor_chr) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -1959,26 +1964,32 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-chardev"); ADD_ARG(virBufferContentAndReset(&buf)); - if (monitor_json) - virBufferAddLit(&buf, "control,"); + virBufferAddLit(&buf, "chardev=monitor"); - virBufferAddLit(&buf, "chardev:monitor"); - } + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto no_memory; + } - else { + ADD_ARG_LIT("-mon"); + if (monitor_json) + ADD_ARG_LIT("chardev=monitor,mode=control"); + else + ADD_ARG_LIT("chardev=monitor,mode=readline"); + } else { if (monitor_json) virBufferAddLit(&buf, "control,"); qemudBuildCommandLineChrDevStr(monitor_chr, &buf); - } - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto no_memory; + } - ADD_ARG_LIT("-monitor"); - ADD_ARG_LIT(virBufferContentAndReset(&buf)); + ADD_ARG_LIT("-monitor"); + ADD_ARG(virBufferContentAndReset(&buf)); + } } if (def->localtime) @@ -2172,8 +2183,11 @@ int qemudBuildCommandLine(virConnectPtr conn, } if (!def->nnets) { - ADD_ARG_LIT("-net"); - ADD_ARG_LIT("none"); + /* If we have -device, then we set -nodefault alrady */ + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + ADD_ARG_LIT("-net"); + ADD_ARG_LIT("none"); + } } else { for (i = 0 ; i < def->nnets ; i++) { virDomainNetDefPtr net = def->nets[i]; @@ -2232,15 +2246,19 @@ int qemudBuildCommandLine(virConnectPtr conn, } if (!def->nserials) { - ADD_ARG_LIT("-serial"); - ADD_ARG_LIT("none"); + /* If we have -device, then we set -nodefault alrady */ + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + ADD_ARG_LIT("-serial"); + ADD_ARG_LIT("none"); + } } else { for (i = 0 ; i < def->nserials ; i++) { virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainChrDefPtr serial = def->serials[i]; - /* Use -chardev if it's available */ - if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) { + /* Use -chardev with -device if they are available */ + if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) && + (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { char id[16]; if (snprintf(id, sizeof(id), "serial%i", i) > sizeof(id)) @@ -2255,13 +2273,13 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-chardev"); ADD_ARG(virBufferContentAndReset(&buf)); - virBufferVSprintf(&buf, "chardev:%s", id); + virBufferVSprintf(&buf, "isa-serial,chardev=%s", id); if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); goto no_memory; } - ADD_ARG_LIT("-serial"); + ADD_ARG_LIT("-device"); ADD_ARG(virBufferContentAndReset(&buf)); } @@ -2279,15 +2297,19 @@ int qemudBuildCommandLine(virConnectPtr conn, } if (!def->nparallels) { - ADD_ARG_LIT("-parallel"); - ADD_ARG_LIT("none"); + /* If we have -device, then we set -nodefault alrady */ + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + ADD_ARG_LIT("-parallel"); + ADD_ARG_LIT("none"); + } } else { for (i = 0 ; i < def->nparallels ; i++) { virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainChrDefPtr parallel = def->parallels[i]; - /* Use -chardev if it's available */ - if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) { + /* Use -chardev with -device if they are available */ + if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) && + (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { char id[16]; if (snprintf(id, sizeof(id), "parallel%i", i) > sizeof(id)) @@ -2302,13 +2324,13 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-chardev"); ADD_ARG(virBufferContentAndReset(&buf)); - virBufferVSprintf(&buf, "chardev:%s", id); + virBufferVSprintf(&buf, "isa-parallel,chardev=%s", id); if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); goto no_memory; } - ADD_ARG_LIT("-parallel"); + ADD_ARG_LIT("-device"); ADD_ARG(virBufferContentAndReset(&buf)); } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index cd59d4c..840b749 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -76,6 +76,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_ENABLE_KVM = (1 << 23), /* Is the -enable-kvm flag available to "enable KVM full virtualization support" */ QEMUD_CMD_FLAG_0_12 = (1 << 24), QEMUD_CMD_FLAG_MONITOR_JSON = QEMUD_CMD_FLAG_0_12, /* JSON mode for monitor */ + QEMUD_CMD_FLAG_DEVICE = (1 << 25), /* Is the new -chardev arg available */ }; /* Main driver state */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args index deaff92..0e02207 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-guestfwd.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -chardev pipe,id=channel0,path=/tmp/guestfwd -net user,guestfwd=tcp:10.0.2.1:4600-chardev:channel0 -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev pipe,id=channel0,path=/tmp/guestfwd -net user,guestfwd=tcp:10.0.2.1:4600-chardev:channel0 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-chardev.args index fbb94f4..5957622 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -chardev pty,id=serial0 -serial chardev:serial0 -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev pty,id=serial0 -device isa-serial,chardev=serial0 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp-chardev.args index 45d1759..d17a16a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -chardev socket,id=parallel0,host=127.0.0.1,port=9999,server,nowait -parallel chardev:parallel0 -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev socket,id=parallel0,host=127.0.0.1,port=9999,server,nowait -device isa-parallel,chardev=parallel0 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev.args index 3036f13..fb2e13b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -chardev tty,id=serial0,path=/dev/ttyS2 -serial chardev:serial0 -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev tty,id=serial0,path=/dev/ttyS2 -device isa-serial,chardev=serial0 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-file-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-file-chardev.args index 1dcec2b..83bf35d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-file-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-file-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -chardev file,id=serial0,path=/tmp/serial.log -serial chardev:serial0 -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev file,id=serial0,path=/tmp/serial.log -device isa-serial,chardev=serial0 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-many-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-many-chardev.args index dd98fcb..27a29ba 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-many-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-many-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -chardev pty,id=serial0 -serial chardev:serial0 -chardev file,id=serial1,path=/tmp/serial.log -serial chardev:serial1 -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev pty,id=serial0 -device isa-serial,chardev=serial0 -chardev file,id=serial1,path=/tmp/serial.log -device isa-serial,chardev=serial1 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.args index fbb94f4..5957622 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -chardev pty,id=serial0 -serial chardev:serial0 -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev pty,id=serial0 -device isa-serial,chardev=serial0 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-chardev.args index 50cfeb0..c4bfea8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -chardev socket,id=serial0,host=127.0.0.1,port=9999 -serial chardev:serial0 -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev socket,id=serial0,host=127.0.0.1,port=9999 -device isa-serial,chardev=serial0 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet-chardev.args index 86fa8af..d25bed3 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -chardev socket,id=serial0,host=127.0.0.1,port=9999,telnet,server,nowait -serial chardev:serial0 -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev socket,id=serial0,host=127.0.0.1,port=9999,telnet,server,nowait -device isa-serial,chardev=serial0 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-udp-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-udp-chardev.args index 45421a4..f598f17 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-udp-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-udp-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -chardev udp,id=serial0,host=127.0.0.1,port=9998,localaddr=127.0.0.1,localport=9999 -serial chardev:serial0 -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev udp,id=serial0,host=127.0.0.1,port=9998,localaddr=127.0.0.1,localport=9999 -device isa-serial,chardev=serial0 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-unix-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-unix-chardev.args index f291156..071839a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-unix-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-unix-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -chardev socket,id=serial0,path=/tmp/serial.sock -serial chardev:serial0 -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev socket,id=serial0,path=/tmp/serial.sock -device isa-serial,chardev=serial0 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-vc-chardev.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-vc-chardev.args index a200225..0fcf8d8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-vc-chardev.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-vc-chardev.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -monitor chardev:monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -chardev vc,id=serial0 -serial chardev:serial0 -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -chardev vc,id=serial0 -device isa-serial,chardev=serial0 -usb diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3b0aa2b..189832e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -273,19 +273,19 @@ mymain(int argc, char **argv) DO_TEST("parallel-tcp", 0); DO_TEST("console-compat", 0); - DO_TEST("serial-vc-chardev", QEMUD_CMD_FLAG_CHARDEV); - DO_TEST("serial-pty-chardev", QEMUD_CMD_FLAG_CHARDEV); - DO_TEST("serial-dev-chardev", QEMUD_CMD_FLAG_CHARDEV); - DO_TEST("serial-file-chardev", QEMUD_CMD_FLAG_CHARDEV); - DO_TEST("serial-unix-chardev", QEMUD_CMD_FLAG_CHARDEV); - DO_TEST("serial-tcp-chardev", QEMUD_CMD_FLAG_CHARDEV); - DO_TEST("serial-udp-chardev", QEMUD_CMD_FLAG_CHARDEV); - DO_TEST("serial-tcp-telnet-chardev", QEMUD_CMD_FLAG_CHARDEV); - DO_TEST("serial-many-chardev", QEMUD_CMD_FLAG_CHARDEV); - DO_TEST("parallel-tcp-chardev", QEMUD_CMD_FLAG_CHARDEV); - DO_TEST("console-compat-chardev", QEMUD_CMD_FLAG_CHARDEV); - - DO_TEST("channel-guestfwd", QEMUD_CMD_FLAG_CHARDEV); + DO_TEST("serial-vc-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("serial-pty-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("serial-dev-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("serial-file-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("serial-unix-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("serial-tcp-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("serial-udp-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("serial-tcp-telnet-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("serial-many-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("parallel-tcp-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("console-compat-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + + DO_TEST("channel-guestfwd", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); DO_TEST("sound", 0); -- 1.6.5.2

On Tue, Dec 15, 2009 at 03:14:42PM +0000, Daniel P. Berrange wrote:
The current character device syntax uses either
-serial tty,path=/dev/ttyS2
Or
-chardev tty,id=serial0,path=/dev/ttyS2 -serial chardev:serial0
With the new -device support, we now prefer
-chardev file,id=serial0,path=/tmp/serial.log -device isa-serial,chardev=serial0
This patch changes the existing -chardev syntax to use this new scheme, and fallbacks to the old plain -serial syntax for old QEMU.
The monitor device changes to
-chardev socket,id=monitor,path=/tmp/test-monitor,server,nowait -mon chardev=monitor
In addition, this patch adds --nodefaults, which kills off the default serial, parallel, vga and nic devices. THis avoids the need for us to explicitly turn each off --- src/qemu/qemu_conf.c | 74 +++++++++++++------- src/qemu/qemu_conf.h | 1 + .../qemuxml2argv-channel-guestfwd.args | 2 +- .../qemuxml2argv-console-compat-chardev.args | 2 +- .../qemuxml2argv-parallel-tcp-chardev.args | 2 +- .../qemuxml2argv-serial-dev-chardev.args | 2 +- .../qemuxml2argv-serial-file-chardev.args | 2 +- .../qemuxml2argv-serial-many-chardev.args | 2 +- .../qemuxml2argv-serial-pty-chardev.args | 2 +- .../qemuxml2argv-serial-tcp-chardev.args | 2 +- .../qemuxml2argv-serial-tcp-telnet-chardev.args | 2 +- .../qemuxml2argv-serial-udp-chardev.args | 2 +- .../qemuxml2argv-serial-unix-chardev.args | 2 +- .../qemuxml2argv-serial-vc-chardev.args | 2 +- tests/qemuxml2argvtest.c | 26 ++++---- 15 files changed, 74 insertions(+), 51 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 61e719d..16e8d2c 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -909,6 +909,8 @@ static unsigned int qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_MEM_PATH; if (strstr(help, "-chardev")) flags |= QEMUD_CMD_FLAG_CHARDEV; + if (strstr(help, "-device")) + flags |= QEMUD_CMD_FLAG_DEVICE;
hum ... shouldn't we look for a space of some sort after the option strings, to make sure we match the right string. Not specific to this but might be safer.
if (version >= 9000) flags |= QEMUD_CMD_FLAG_VNC_COLON; @@ -1945,6 +1947,9 @@ int qemudBuildCommandLine(virConnectPtr conn, if (!def->graphics) ADD_ARG_LIT("-nographic");
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) + ADD_ARG_LIT("-nodefaults"); + if (monitor_chr) { virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -1959,26 +1964,32 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-chardev"); ADD_ARG(virBufferContentAndReset(&buf));
- if (monitor_json) - virBufferAddLit(&buf, "control,"); + virBufferAddLit(&buf, "chardev=monitor");
- virBufferAddLit(&buf, "chardev:monitor"); - } + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto no_memory; + }
- else { + ADD_ARG_LIT("-mon"); + if (monitor_json) + ADD_ARG_LIT("chardev=monitor,mode=control"); + else + ADD_ARG_LIT("chardev=monitor,mode=readline"); + } else { if (monitor_json) virBufferAddLit(&buf, "control,");
qemudBuildCommandLineChrDevStr(monitor_chr, &buf); - }
- if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto no_memory; + }
- ADD_ARG_LIT("-monitor"); - ADD_ARG_LIT(virBufferContentAndReset(&buf)); + ADD_ARG_LIT("-monitor"); + ADD_ARG(virBufferContentAndReset(&buf)); + } }
if (def->localtime) @@ -2172,8 +2183,11 @@ int qemudBuildCommandLine(virConnectPtr conn, }
if (!def->nnets) { - ADD_ARG_LIT("-net"); - ADD_ARG_LIT("none"); + /* If we have -device, then we set -nodefault alrady */ + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + ADD_ARG_LIT("-net"); + ADD_ARG_LIT("none"); + } } else { for (i = 0 ; i < def->nnets ; i++) { virDomainNetDefPtr net = def->nets[i]; @@ -2232,15 +2246,19 @@ int qemudBuildCommandLine(virConnectPtr conn, }
if (!def->nserials) { - ADD_ARG_LIT("-serial"); - ADD_ARG_LIT("none"); + /* If we have -device, then we set -nodefault alrady */ + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + ADD_ARG_LIT("-serial"); + ADD_ARG_LIT("none"); + } } else { for (i = 0 ; i < def->nserials ; i++) { virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainChrDefPtr serial = def->serials[i];
- /* Use -chardev if it's available */ - if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) { + /* Use -chardev with -device if they are available */ + if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) && + (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { char id[16];
if (snprintf(id, sizeof(id), "serial%i", i) > sizeof(id)) @@ -2255,13 +2273,13 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-chardev"); ADD_ARG(virBufferContentAndReset(&buf));
- virBufferVSprintf(&buf, "chardev:%s", id); + virBufferVSprintf(&buf, "isa-serial,chardev=%s", id); if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); goto no_memory; }
- ADD_ARG_LIT("-serial"); + ADD_ARG_LIT("-device"); ADD_ARG(virBufferContentAndReset(&buf)); }
@@ -2279,15 +2297,19 @@ int qemudBuildCommandLine(virConnectPtr conn, }
if (!def->nparallels) { - ADD_ARG_LIT("-parallel"); - ADD_ARG_LIT("none"); + /* If we have -device, then we set -nodefault alrady */ + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + ADD_ARG_LIT("-parallel"); + ADD_ARG_LIT("none"); + } } else { for (i = 0 ; i < def->nparallels ; i++) { virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainChrDefPtr parallel = def->parallels[i];
- /* Use -chardev if it's available */ - if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) { + /* Use -chardev with -device if they are available */ + if ((qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) && + (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { char id[16];
if (snprintf(id, sizeof(id), "parallel%i", i) > sizeof(id)) @@ -2302,13 +2324,13 @@ int qemudBuildCommandLine(virConnectPtr conn, ADD_ARG_LIT("-chardev"); ADD_ARG(virBufferContentAndReset(&buf));
- virBufferVSprintf(&buf, "chardev:%s", id); + virBufferVSprintf(&buf, "isa-parallel,chardev=%s", id); if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); goto no_memory; }
- ADD_ARG_LIT("-parallel"); + ADD_ARG_LIT("-device"); ADD_ARG(virBufferContentAndReset(&buf)); }
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index cd59d4c..840b749 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -76,6 +76,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_ENABLE_KVM = (1 << 23), /* Is the -enable-kvm flag available to "enable KVM full virtualization support" */ QEMUD_CMD_FLAG_0_12 = (1 << 24), QEMUD_CMD_FLAG_MONITOR_JSON = QEMUD_CMD_FLAG_0_12, /* JSON mode for monitor */ + QEMUD_CMD_FLAG_DEVICE = (1 << 25), /* Is the new -chardev arg available */ };
/* Main driver state */
I'm starting to worry about exhausting the enum size ... we are getting close, and something allowing for the continuous expansion of QEmu options will be needed soon ! ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

The current syntax for watchdogs is -watchdog i6300esb The new syntax will now be -device i6300esb --- src/qemu/qemu_conf.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 16e8d2c..79b7b00 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2574,7 +2574,10 @@ int qemudBuildCommandLine(virConnectPtr conn, "%s", _("invalid watchdog model")); goto error; } - ADD_ARG_LIT("-watchdog"); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) + ADD_ARG_LIT("-device"); + else + ADD_ARG_LIT("-watchdog"); ADD_ARG_LIT(model); const char *action = virDomainWatchdogActionTypeToString(watchdog->action); -- 1.6.5.2

On Tue, Dec 15, 2009 at 03:14:43PM +0000, Daniel P. Berrange wrote:
The current syntax for watchdogs is
-watchdog i6300esb
The new syntax will now be
-device i6300esb --- src/qemu/qemu_conf.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 16e8d2c..79b7b00 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2574,7 +2574,10 @@ int qemudBuildCommandLine(virConnectPtr conn, "%s", _("invalid watchdog model")); goto error; } - ADD_ARG_LIT("-watchdog"); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) + ADD_ARG_LIT("-device"); + else + ADD_ARG_LIT("-watchdog"); ADD_ARG_LIT(model);
const char *action = virDomainWatchdogActionTypeToString(watchdog->action);
ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

The current syntax for audio devices is a horrible multiplexed arg -soundhw sb16,pcspk,ac97 The new syntax is -device sb16 -device ac97 --- src/qemu/qemu_conf.c | 72 +++++++++++++++++++++++++++++++++++++------------ 1 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 79b7b00..8c44a93 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1538,6 +1538,28 @@ qemuBuildHostNetStr(virConnectPtr conn, return 0; } +static int +qemuBuildSoundDevStr(virConnectPtr conn, + virDomainSoundDefPtr sound, + char **str) +{ + const char *model = virDomainSoundModelTypeToString(sound->model); + + if (STREQ(model, "es1370")) + model = "ES1370"; + else if (STREQ(model, "ac97")) + model = "AC97"; + + *str = strdup(model); + if (!(*str)) { + virReportOOMError(conn); + return -1; + } + + return 0; +} + + /* This function outputs a -chardev command line option which describes only the * host side of the character device */ static void qemudBuildCommandLineChrDevChardevStr(virDomainChrDefPtr dev, @@ -2542,27 +2564,41 @@ int qemudBuildCommandLine(virConnectPtr conn, /* Add sound hardware */ if (def->nsounds) { - int size = 100; - char *modstr; - if (VIR_ALLOC_N(modstr, size+1) < 0) - goto no_memory; + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + for (i = 0 ; i < def->nsounds ; i++) { + virDomainSoundDefPtr sound = def->sounds[i]; + char *str = NULL; - for (i = 0 ; i < def->nsounds && size > 0 ; i++) { - virDomainSoundDefPtr sound = def->sounds[i]; - const char *model = virDomainSoundModelTypeToString(sound->model); - if (!model) { - VIR_FREE(modstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid sound model")); - goto error; + ADD_ARG_LIT("-device"); + + if (qemuBuildSoundDevStr(conn, sound, &str) < 0) + goto error; + + ADD_ARG(str); + } + } else { + int size = 100; + char *modstr; + if (VIR_ALLOC_N(modstr, size+1) < 0) + goto no_memory; + + for (i = 0 ; i < def->nsounds && size > 0 ; i++) { + virDomainSoundDefPtr sound = def->sounds[i]; + const char *model = virDomainSoundModelTypeToString(sound->model); + if (!model) { + VIR_FREE(modstr); + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("invalid sound model")); + goto error; + } + strncat(modstr, model, size); + size -= strlen(model); + if (i < (def->nsounds - 1)) + strncat(modstr, ",", size--); } - strncat(modstr, model, size); - size -= strlen(model); - if (i < (def->nsounds - 1)) - strncat(modstr, ",", size--); + ADD_ARG_LIT("-soundhw"); + ADD_ARG(modstr); } - ADD_ARG_LIT("-soundhw"); - ADD_ARG(modstr); } /* Add watchdog hardware */ -- 1.6.5.2

On Tue, Dec 15, 2009 at 03:14:44PM +0000, Daniel P. Berrange wrote:
The current syntax for audio devices is a horrible multiplexed arg
-soundhw sb16,pcspk,ac97
The new syntax is
-device sb16 -device ac97 --- src/qemu/qemu_conf.c | 72 +++++++++++++++++++++++++++++++++++++------------ 1 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 79b7b00..8c44a93 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1538,6 +1538,28 @@ qemuBuildHostNetStr(virConnectPtr conn, return 0; }
+static int +qemuBuildSoundDevStr(virConnectPtr conn, + virDomainSoundDefPtr sound, + char **str) +{ + const char *model = virDomainSoundModelTypeToString(sound->model); + + if (STREQ(model, "es1370")) + model = "ES1370"; + else if (STREQ(model, "ac97")) + model = "AC97";
I'm not sure I understand why those 2 strings need to be upper-cased
+ *str = strdup(model); + if (!(*str)) { + virReportOOMError(conn); + return -1; + } + + return 0; +} + + /* This function outputs a -chardev command line option which describes only the * host side of the character device */ static void qemudBuildCommandLineChrDevChardevStr(virDomainChrDefPtr dev, @@ -2542,27 +2564,41 @@ int qemudBuildCommandLine(virConnectPtr conn,
/* Add sound hardware */ if (def->nsounds) { - int size = 100; - char *modstr; - if (VIR_ALLOC_N(modstr, size+1) < 0) - goto no_memory; + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + for (i = 0 ; i < def->nsounds ; i++) { + virDomainSoundDefPtr sound = def->sounds[i]; + char *str = NULL;
- for (i = 0 ; i < def->nsounds && size > 0 ; i++) { - virDomainSoundDefPtr sound = def->sounds[i]; - const char *model = virDomainSoundModelTypeToString(sound->model); - if (!model) { - VIR_FREE(modstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid sound model")); - goto error; + ADD_ARG_LIT("-device"); + + if (qemuBuildSoundDevStr(conn, sound, &str) < 0) + goto error; + + ADD_ARG(str); + } + } else { + int size = 100; + char *modstr; + if (VIR_ALLOC_N(modstr, size+1) < 0) + goto no_memory; + + for (i = 0 ; i < def->nsounds && size > 0 ; i++) { + virDomainSoundDefPtr sound = def->sounds[i]; + const char *model = virDomainSoundModelTypeToString(sound->model); + if (!model) { + VIR_FREE(modstr); + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("invalid sound model")); + goto error; + } + strncat(modstr, model, size); + size -= strlen(model); + if (i < (def->nsounds - 1)) + strncat(modstr, ",", size--); } - strncat(modstr, model, size); - size -= strlen(model); - if (i < (def->nsounds - 1)) - strncat(modstr, ",", size--); + ADD_ARG_LIT("-soundhw"); + ADD_ARG(modstr); } - ADD_ARG_LIT("-soundhw"); - ADD_ARG(modstr); }
/* Add watchdog hardware */
ACK, the indentation messed the diff, making it hard to check the old code was still unchanged, looks fine, new code is way simpler ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Thu, Dec 17, 2009 at 10:06:22AM +0100, Daniel Veillard wrote:
On Tue, Dec 15, 2009 at 03:14:44PM +0000, Daniel P. Berrange wrote:
The current syntax for audio devices is a horrible multiplexed arg
-soundhw sb16,pcspk,ac97
The new syntax is
-device sb16 -device ac97 --- src/qemu/qemu_conf.c | 72 +++++++++++++++++++++++++++++++++++++------------ 1 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 79b7b00..8c44a93 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1538,6 +1538,28 @@ qemuBuildHostNetStr(virConnectPtr conn, return 0; }
+static int +qemuBuildSoundDevStr(virConnectPtr conn, + virDomainSoundDefPtr sound, + char **str) +{ + const char *model = virDomainSoundModelTypeToString(sound->model); + + if (STREQ(model, "es1370")) + model = "ES1370"; + else if (STREQ(model, "ac97")) + model = "AC97";
I'm not sure I understand why those 2 strings need to be upper-cased
That's just the way QEMU wants these two devices named :-( Everything else in QMEU is lowercase :-( Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

The current preferred syntax for disk drives uses -drive file=/vms/plain.qcow,if=virtio,index=0,boot=on,format=qcow The new syntax splits this up into a pair of linked args -drive file=/vms/plain.qcow,if=none,id=virtio-0,index=0,format=qcow2 -device virtio-blk-pci,drive=virtio-0 NB, the 'index=0' bit here in the new args is technically wrong. This will be fixed when the disk-controller /addressing patches merge with this. --- src/qemu/qemu_conf.c | 83 ++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8c44a93..2480df4 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1373,10 +1373,16 @@ qemuBuildDriveStr(virConnectPtr conn, virBufferVSprintf(&opt, "file=%s,", disk->src); } } - virBufferVSprintf(&opt, "if=%s", bus); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + virBufferAddLit(&opt, "if=none,"); + virBufferVSprintf(&opt, "id=%s-%d", bus, idx); + } else { + virBufferVSprintf(&opt, "if=%s", bus); + } if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) virBufferAddLit(&opt, ",media=cdrom"); - virBufferVSprintf(&opt, ",index=%d", idx); + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) + virBufferVSprintf(&opt, ",index=%d", idx); if (bootable && disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) virBufferAddLit(&opt, ",boot=on"); @@ -1416,6 +1422,48 @@ error: return -1; } +static int +qemuBuildDriveDevStr(virConnectPtr conn, + virDomainDiskDefPtr disk, + char **str) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); + int idx = virDiskNameToIndex(disk->dst); + + if (idx < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unsupported disk type '%s'"), disk->dst); + goto error; + } + + switch (disk->bus) { + case VIR_DOMAIN_DISK_BUS_IDE: + virBufferAddLit(&opt, "ide-drive"); + break; + case VIR_DOMAIN_DISK_BUS_SCSI: + virBufferAddLit(&opt, "scsi-disk"); + break; + case VIR_DOMAIN_DISK_BUS_VIRTIO: + virBufferAddLit(&opt, "virtio-blk-pci"); + break; + + default: + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unsupported disk bus '%s' with device setup"), bus); + goto error; + } + virBufferVSprintf(&opt, ",drive=%s-%d", bus, idx); + + *str = virBufferContentAndReset(&opt); + return 0; + +error: + virBufferFreeAndReset(&opt); + *str = NULL; + return -1; +} + int qemuBuildNicStr(virConnectPtr conn, @@ -2104,6 +2152,7 @@ int qemudBuildCommandLine(virConnectPtr conn, char *optstr; int bootable = 0; virDomainDiskDefPtr disk = def->disks[i]; + int withDeviceArg = 0; if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { @@ -2116,8 +2165,6 @@ int qemudBuildCommandLine(virConnectPtr conn, continue; } - ADD_ARG_SPACE; - switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: bootable = bootCD; @@ -2133,14 +2180,32 @@ int qemudBuildCommandLine(virConnectPtr conn, break; } - if (qemuBuildDriveStr(conn, disk, bootable, qemuCmdFlags, &optstr) < 0) + /* Unfortunately it is nt possible to use + -device for floppys, 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 ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + (disk->bus != VIR_DOMAIN_DISK_BUS_FDC) && + (disk->bus != VIR_DOMAIN_DISK_BUS_XEN)) + withDeviceArg = 1; + + ADD_ARG_LIT("-drive"); + + if (qemuBuildDriveStr(conn, disk, bootable, + (withDeviceArg ? qemuCmdFlags : + (qemuCmdFlags & ~QEMUD_CMD_FLAG_DEVICE)), + &optstr) < 0) goto error; + ADD_ARG(optstr); - if ((qargv[qargc++] = strdup("-drive")) == NULL) { - VIR_FREE(optstr); - goto no_memory; + if (withDeviceArg) { + ADD_ARG_LIT("-device"); + + if (qemuBuildDriveDevStr(conn, disk, &optstr) < 0) + goto error; + ADD_ARG(optstr); } - ADD_ARG(optstr); } } else { for (i = 0 ; i < def->ndisks ; i++) { -- 1.6.5.2

On Tue, Dec 15, 2009 at 03:14:45PM +0000, Daniel P. Berrange wrote:
The current preferred syntax for disk drives uses
-drive file=/vms/plain.qcow,if=virtio,index=0,boot=on,format=qcow
The new syntax splits this up into a pair of linked args
-drive file=/vms/plain.qcow,if=none,id=virtio-0,index=0,format=qcow2 -device virtio-blk-pci,drive=virtio-0
NB, the 'index=0' bit here in the new args is technically wrong. This will be fixed when the disk-controller /addressing patches merge with this. --- src/qemu/qemu_conf.c | 83 ++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 74 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8c44a93..2480df4 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1373,10 +1373,16 @@ qemuBuildDriveStr(virConnectPtr conn, virBufferVSprintf(&opt, "file=%s,", disk->src); } } - virBufferVSprintf(&opt, "if=%s", bus); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + virBufferAddLit(&opt, "if=none,"); + virBufferVSprintf(&opt, "id=%s-%d", bus, idx); + } else { + virBufferVSprintf(&opt, "if=%s", bus); + } if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) virBufferAddLit(&opt, ",media=cdrom"); - virBufferVSprintf(&opt, ",index=%d", idx); + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) + virBufferVSprintf(&opt, ",index=%d", idx); if (bootable && disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) virBufferAddLit(&opt, ",boot=on"); @@ -1416,6 +1422,48 @@ error: return -1; }
+static int +qemuBuildDriveDevStr(virConnectPtr conn, + virDomainDiskDefPtr disk, + char **str) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); + int idx = virDiskNameToIndex(disk->dst); + + if (idx < 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unsupported disk type '%s'"), disk->dst); + goto error; + } + + switch (disk->bus) { + case VIR_DOMAIN_DISK_BUS_IDE: + virBufferAddLit(&opt, "ide-drive"); + break; + case VIR_DOMAIN_DISK_BUS_SCSI: + virBufferAddLit(&opt, "scsi-disk"); + break; + case VIR_DOMAIN_DISK_BUS_VIRTIO: + virBufferAddLit(&opt, "virtio-blk-pci"); + break; + + default: + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("unsupported disk bus '%s' with device setup"), bus); + goto error; + } + virBufferVSprintf(&opt, ",drive=%s-%d", bus, idx); + + *str = virBufferContentAndReset(&opt); + return 0; + +error: + virBufferFreeAndReset(&opt); + *str = NULL; + return -1; +} +
int qemuBuildNicStr(virConnectPtr conn, @@ -2104,6 +2152,7 @@ int qemudBuildCommandLine(virConnectPtr conn, char *optstr; int bootable = 0; virDomainDiskDefPtr disk = def->disks[i]; + int withDeviceArg = 0;
if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { @@ -2116,8 +2165,6 @@ int qemudBuildCommandLine(virConnectPtr conn, continue; }
- ADD_ARG_SPACE; - switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: bootable = bootCD; @@ -2133,14 +2180,32 @@ int qemudBuildCommandLine(virConnectPtr conn, break; }
- if (qemuBuildDriveStr(conn, disk, bootable, qemuCmdFlags, &optstr) < 0) + /* Unfortunately it is nt possible to use + -device for floppys, 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 ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && + (disk->bus != VIR_DOMAIN_DISK_BUS_FDC) && + (disk->bus != VIR_DOMAIN_DISK_BUS_XEN)) + withDeviceArg = 1; + + ADD_ARG_LIT("-drive"); + + if (qemuBuildDriveStr(conn, disk, bootable, + (withDeviceArg ? qemuCmdFlags : + (qemuCmdFlags & ~QEMUD_CMD_FLAG_DEVICE)), + &optstr) < 0) goto error; + ADD_ARG(optstr);
- if ((qargv[qargc++] = strdup("-drive")) == NULL) { - VIR_FREE(optstr); - goto no_memory; + if (withDeviceArg) { + ADD_ARG_LIT("-device"); + + if (qemuBuildDriveDevStr(conn, disk, &optstr) < 0) + goto error; + ADD_ARG(optstr); } - ADD_ARG(optstr); } } else { for (i = 0 ; i < def->ndisks ; i++) {
ACK, the number of different cases get scary Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

The current syntax uses a pair of args -net nic,macaddr=52:54:00:56:6c:55,vlan=3,model=pcnet,name=pcnet.0 -net user,vlan=3,name=user.2 The new syntax does not need the vlan craziness anymore, and so has a simplified pair of args -netdev user,id=user.2 -device pcnet,netdev=user.2,mac=52:54:00:56:6c:55 --- src/qemu/qemu_conf.c | 166 +++++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 137 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2480df4..464f582 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1490,6 +1490,35 @@ qemuBuildNicStr(virConnectPtr conn, return 0; } +static int +qemuBuildNicDevStr(virConnectPtr conn, + virDomainNetDefPtr net, + char **str) +{ + const char *nic; + + if (!net->model) { + nic = "rtl8139"; + } else if (STREQ(net->model, "virtio")) { + nic = "virtio-net-pci"; + } else { + nic = net->model; + } + + if (virAsprintf(str, + "%s,netdev=%s,mac=%02x:%02x:%02x:%02x:%02x:%02x", + nic, + net->hostnet_name, + net->mac[0], net->mac[1], + net->mac[2], net->mac[3], + net->mac[4], net->mac[5]) < 0) { + virReportOOMError(conn); + return -1; + } + + return 0; +} + int qemuBuildHostNetStr(virConnectPtr conn, virDomainNetDefPtr net, @@ -1587,6 +1616,87 @@ qemuBuildHostNetStr(virConnectPtr conn, } static int +qemuBuildNetDevStr(virConnectPtr conn, + virDomainNetDefPtr net, + const char *tapfd, + char **str) +{ + switch (net->type) { + case VIR_DOMAIN_NET_TYPE_NETWORK: + case VIR_DOMAIN_NET_TYPE_BRIDGE: + if (virAsprintf(str, "tap,fd=%s,id=%s", + tapfd, net->hostnet_name) < 0) { + virReportOOMError(conn); + return -1; + } + break; + + case VIR_DOMAIN_NET_TYPE_ETHERNET: + { + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferAddLit(&buf, "tap"); + if (net->ifname) + virBufferVSprintf(&buf, ",ifname=%s", net->ifname); + if (net->data.ethernet.script) + virBufferVSprintf(&buf, ",script=%s", + net->data.ethernet.script); + if (net->hostnet_name) + virBufferVSprintf(&buf, ",id=%s", + net->hostnet_name); + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(conn); + return -1; + } + + *str = virBufferContentAndReset(&buf); + } + break; + + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_MCAST: + { + const char *mode = NULL; + + switch (net->type) { + case VIR_DOMAIN_NET_TYPE_CLIENT: + mode = "connect"; + break; + case VIR_DOMAIN_NET_TYPE_SERVER: + mode = "listen"; + break; + case VIR_DOMAIN_NET_TYPE_MCAST: + mode = "mcast"; + break; + } + + if (virAsprintf(str, "socket,%s=%s:%d,id=%s", + mode, + net->data.socket.address, + net->data.socket.port, + net->hostnet_name) < 0) { + virReportOOMError(conn); + return -1; + } + } + break; + + case VIR_DOMAIN_NET_TYPE_USER: + default: + if (virAsprintf(str, "user,id=%s", + net->hostnet_name) < 0) { + virReportOOMError(conn); + return -1; + } + break; + } + + return 0; +} + +static int qemuBuildSoundDevStr(virConnectPtr conn, virDomainSoundDefPtr sound, char **str) @@ -2279,26 +2389,9 @@ int qemudBuildCommandLine(virConnectPtr conn, for (i = 0 ; i < def->nnets ; i++) { virDomainNetDefPtr net = def->nets[i]; char *nic, *host; - char *tapfd_name = NULL; + char tapfd_name[50]; net->vlan = i; - - ADD_ARG_SPACE; - if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) && - qemuAssignNetNames(def, net) < 0) - goto no_memory; - - if (qemuBuildNicStr(conn, net, "nic,", net->vlan, &nic) < 0) - goto error; - - if ((qargv[qargc++] = strdup("-net")) == NULL) { - VIR_FREE(nic); - goto no_memory; - } - ADD_ARG(nic); - - - ADD_ARG_SPACE; if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK || net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { int tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags); @@ -2312,23 +2405,38 @@ int qemudBuildCommandLine(virConnectPtr conn, (*tapfds)[(*ntapfds)++] = tapfd; - if (virAsprintf(&tapfd_name, "%d", tapfd) < 0) + if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name)) goto no_memory; } - if (qemuBuildHostNetStr(conn, net, ',', - net->vlan, tapfd_name, &host) < 0) { - VIR_FREE(tapfd_name); - goto error; + if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) || + (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + if (qemuAssignNetNames(def, net) < 0) + goto no_memory; } - if ((qargv[qargc++] = strdup("-net")) == NULL) { - VIR_FREE(host); - goto no_memory; - } - ADD_ARG(host); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ADD_ARG_LIT("-netdev"); + if (qemuBuildNetDevStr(conn, net, tapfd_name, &host) < 0) + goto error; + ADD_ARG(host); + + ADD_ARG_LIT("-device"); + if (qemuBuildNicDevStr(conn, net, &nic) < 0) + goto error; + ADD_ARG(nic); + } else { + ADD_ARG_LIT("-net"); + if (qemuBuildNicStr(conn, net, "nic,", net->vlan, &nic) < 0) + goto error; + ADD_ARG(nic); - VIR_FREE(tapfd_name); + ADD_ARG_LIT("-net"); + if (qemuBuildHostNetStr(conn, net, ',', + net->vlan, tapfd_name, &host) < 0) + goto error; + ADD_ARG(host); + } } } -- 1.6.5.2

On Tue, Dec 15, 2009 at 03:14:46PM +0000, Daniel P. Berrange wrote:
The current syntax uses a pair of args
-net nic,macaddr=52:54:00:56:6c:55,vlan=3,model=pcnet,name=pcnet.0 -net user,vlan=3,name=user.2
The new syntax does not need the vlan craziness anymore, and so has a simplified pair of args
-netdev user,id=user.2 -device pcnet,netdev=user.2,mac=52:54:00:56:6c:55 --- src/qemu/qemu_conf.c | 166 +++++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 137 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2480df4..464f582 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1490,6 +1490,35 @@ qemuBuildNicStr(virConnectPtr conn, return 0; }
+static int +qemuBuildNicDevStr(virConnectPtr conn, + virDomainNetDefPtr net, + char **str) +{ + const char *nic; + + if (!net->model) { + nic = "rtl8139";
Weren't we supposed to default to something more "performant" in recent QEmus like e1000 or similar ? but I have no idea if this is safe (old Windows may not have default drivers)
+ } else if (STREQ(net->model, "virtio")) { + nic = "virtio-net-pci"; + } else { + nic = net->model; + } + + if (virAsprintf(str, + "%s,netdev=%s,mac=%02x:%02x:%02x:%02x:%02x:%02x", + nic, + net->hostnet_name, + net->mac[0], net->mac[1], + net->mac[2], net->mac[3], + net->mac[4], net->mac[5]) < 0) { + virReportOOMError(conn); + return -1; + } + + return 0; +} + int qemuBuildHostNetStr(virConnectPtr conn, virDomainNetDefPtr net, @@ -1587,6 +1616,87 @@ qemuBuildHostNetStr(virConnectPtr conn, }
static int +qemuBuildNetDevStr(virConnectPtr conn, + virDomainNetDefPtr net, + const char *tapfd, + char **str) +{ + switch (net->type) { + case VIR_DOMAIN_NET_TYPE_NETWORK: + case VIR_DOMAIN_NET_TYPE_BRIDGE: + if (virAsprintf(str, "tap,fd=%s,id=%s", + tapfd, net->hostnet_name) < 0) { + virReportOOMError(conn); + return -1; + } + break; + + case VIR_DOMAIN_NET_TYPE_ETHERNET: + { + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferAddLit(&buf, "tap"); + if (net->ifname) + virBufferVSprintf(&buf, ",ifname=%s", net->ifname); + if (net->data.ethernet.script) + virBufferVSprintf(&buf, ",script=%s", + net->data.ethernet.script); + if (net->hostnet_name) + virBufferVSprintf(&buf, ",id=%s", + net->hostnet_name); + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(conn); + return -1; + } + + *str = virBufferContentAndReset(&buf); + } + break; + + case VIR_DOMAIN_NET_TYPE_CLIENT: + case VIR_DOMAIN_NET_TYPE_SERVER: + case VIR_DOMAIN_NET_TYPE_MCAST: + { + const char *mode = NULL; + + switch (net->type) { + case VIR_DOMAIN_NET_TYPE_CLIENT: + mode = "connect"; + break; + case VIR_DOMAIN_NET_TYPE_SERVER: + mode = "listen"; + break; + case VIR_DOMAIN_NET_TYPE_MCAST: + mode = "mcast"; + break; + } + + if (virAsprintf(str, "socket,%s=%s:%d,id=%s", + mode, + net->data.socket.address, + net->data.socket.port, + net->hostnet_name) < 0) { + virReportOOMError(conn); + return -1; + } + } + break; + + case VIR_DOMAIN_NET_TYPE_USER: + default: + if (virAsprintf(str, "user,id=%s", + net->hostnet_name) < 0) { + virReportOOMError(conn); + return -1; + } + break; + } + + return 0; +} + +static int qemuBuildSoundDevStr(virConnectPtr conn, virDomainSoundDefPtr sound, char **str) @@ -2279,26 +2389,9 @@ int qemudBuildCommandLine(virConnectPtr conn, for (i = 0 ; i < def->nnets ; i++) { virDomainNetDefPtr net = def->nets[i]; char *nic, *host; - char *tapfd_name = NULL; + char tapfd_name[50];
net->vlan = i; - - ADD_ARG_SPACE; - if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) && - qemuAssignNetNames(def, net) < 0) - goto no_memory; - - if (qemuBuildNicStr(conn, net, "nic,", net->vlan, &nic) < 0) - goto error; - - if ((qargv[qargc++] = strdup("-net")) == NULL) { - VIR_FREE(nic); - goto no_memory; - } - ADD_ARG(nic); - - - ADD_ARG_SPACE; if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK || net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { int tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags); @@ -2312,23 +2405,38 @@ int qemudBuildCommandLine(virConnectPtr conn,
(*tapfds)[(*ntapfds)++] = tapfd;
- if (virAsprintf(&tapfd_name, "%d", tapfd) < 0) + if (snprintf(tapfd_name, sizeof(tapfd_name), "%d", tapfd) >= sizeof(tapfd_name)) goto no_memory; }
- if (qemuBuildHostNetStr(conn, net, ',', - net->vlan, tapfd_name, &host) < 0) { - VIR_FREE(tapfd_name); - goto error; + if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) || + (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + if (qemuAssignNetNames(def, net) < 0) + goto no_memory; }
- if ((qargv[qargc++] = strdup("-net")) == NULL) { - VIR_FREE(host); - goto no_memory; - } - ADD_ARG(host); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ADD_ARG_LIT("-netdev"); + if (qemuBuildNetDevStr(conn, net, tapfd_name, &host) < 0) + goto error; + ADD_ARG(host); + + ADD_ARG_LIT("-device"); + if (qemuBuildNicDevStr(conn, net, &nic) < 0) + goto error; + ADD_ARG(nic); + } else { + ADD_ARG_LIT("-net"); + if (qemuBuildNicStr(conn, net, "nic,", net->vlan, &nic) < 0) + goto error; + ADD_ARG(nic);
- VIR_FREE(tapfd_name); + ADD_ARG_LIT("-net"); + if (qemuBuildHostNetStr(conn, net, ',', + net->vlan, tapfd_name, &host) < 0) + goto error; + ADD_ARG(host); + } } }
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

The previous syntax was severely limited in its options -usbdevice disk:/home/berrange/output.img The new syntax is the same as for other disk types -drive file=/home/berrange/output.img,if=none,id=usb-1,index=1 -device usb-storage,drive=usb-1 Again, the index= arg is wrong here, and will be removed in a later merge --- src/qemu/qemu_conf.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 464f582..7582319 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1447,7 +1447,9 @@ qemuBuildDriveDevStr(virConnectPtr conn, case VIR_DOMAIN_DISK_BUS_VIRTIO: virBufferAddLit(&opt, "virtio-blk-pci"); break; - + case VIR_DOMAIN_DISK_BUS_USB: + virBufferAddLit(&opt, "usb-storage"); + break; default: qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("unsupported disk bus '%s' with device setup"), bus); @@ -2264,7 +2266,10 @@ int qemudBuildCommandLine(virConnectPtr conn, virDomainDiskDefPtr disk = def->disks[i]; int withDeviceArg = 0; - if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { + /* Unless we have -device, then USB disks need special + handling */ + if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && + !(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { ADD_USBDISK(disk->src); } else { -- 1.6.5.2

On Tue, Dec 15, 2009 at 03:14:47PM +0000, Daniel P. Berrange wrote:
The previous syntax was severely limited in its options
-usbdevice disk:/home/berrange/output.img
The new syntax is the same as for other disk types
-drive file=/home/berrange/output.img,if=none,id=usb-1,index=1 -device usb-storage,drive=usb-1
Again, the index= arg is wrong here, and will be removed in a later merge --- src/qemu/qemu_conf.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 464f582..7582319 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1447,7 +1447,9 @@ qemuBuildDriveDevStr(virConnectPtr conn, case VIR_DOMAIN_DISK_BUS_VIRTIO: virBufferAddLit(&opt, "virtio-blk-pci"); break; - + case VIR_DOMAIN_DISK_BUS_USB: + virBufferAddLit(&opt, "usb-storage"); + break; default: qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("unsupported disk bus '%s' with device setup"), bus); @@ -2264,7 +2266,10 @@ int qemudBuildCommandLine(virConnectPtr conn, virDomainDiskDefPtr disk = def->disks[i]; int withDeviceArg = 0;
- if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { + /* Unless we have -device, then USB disks need special + handling */ + if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && + !(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { ADD_USBDISK(disk->src); } else {
ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

The old syntax was -usbdevice host:PRODUCT:VENDOR Or -usbdevice host:BUS.DEV The new syntax is -device usb-host,product=PRODUCT,vendor=VENDOR Or -device usb-host,hostbus=BUS,hostaddr=DEV --- src/qemu/qemu_conf.c | 39 ++++++++++++++++++++++++++++----------- 1 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 7582319..ac322a7 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2596,8 +2596,13 @@ int qemudBuildCommandLine(virConnectPtr conn, virDomainInputDefPtr input = def->inputs[i]; if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { - ADD_ARG_LIT("-usbdevice"); - ADD_ARG_LIT(input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet"); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ADD_ARG_LIT("-device"); + ADD_ARG_LIT(input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "usb-mouse" : "usb-tablet"); + } else { + ADD_ARG_LIT("-usbdevice"); + ADD_ARG_LIT(input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet"); + } } } @@ -2814,22 +2819,34 @@ int qemudBuildCommandLine(virConnectPtr conn, /* USB */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { - if(hostdev->source.subsys.u.usb.vendor) { - ret = virAsprintf(&usbdev, "host:%.4x:%.4x", - hostdev->source.subsys.u.usb.vendor, - hostdev->source.subsys.u.usb.product); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ADD_ARG_LIT("-device"); + if (hostdev->source.subsys.u.usb.vendor) { + ret = virAsprintf(&usbdev, "usb-host,vendor=%.4x,product=%.4x", + hostdev->source.subsys.u.usb.vendor, + hostdev->source.subsys.u.usb.product); + } else { + ret = virAsprintf(&usbdev, "usb-host,hostbus=%.3d,hostaddr=%.3d", + hostdev->source.subsys.u.usb.bus, + hostdev->source.subsys.u.usb.device); + } } else { + ADD_ARG_LIT("-usbdevice"); + if (hostdev->source.subsys.u.usb.vendor) { + ret = virAsprintf(&usbdev, "host:%.4x:%.4x", + hostdev->source.subsys.u.usb.vendor, + hostdev->source.subsys.u.usb.product); + } else { ret = virAsprintf(&usbdev, "host:%.3d.%.3d", - hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device); + hostdev->source.subsys.u.usb.bus, + hostdev->source.subsys.u.usb.device); + } } if (ret < 0) goto error; - ADD_ARG_LIT("-usbdevice"); - ADD_ARG_LIT(usbdev); - VIR_FREE(usbdev); + ADD_ARG(usbdev); } /* PCI */ -- 1.6.5.2

On Tue, Dec 15, 2009 at 03:14:48PM +0000, Daniel P. Berrange wrote:
The old syntax was
-usbdevice host:PRODUCT:VENDOR
Or
-usbdevice host:BUS.DEV
The new syntax is
-device usb-host,product=PRODUCT,vendor=VENDOR
Or
-device usb-host,hostbus=BUS,hostaddr=DEV --- src/qemu/qemu_conf.c | 39 ++++++++++++++++++++++++++++----------- 1 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 7582319..ac322a7 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2596,8 +2596,13 @@ int qemudBuildCommandLine(virConnectPtr conn, virDomainInputDefPtr input = def->inputs[i];
if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { - ADD_ARG_LIT("-usbdevice"); - ADD_ARG_LIT(input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet"); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ADD_ARG_LIT("-device"); + ADD_ARG_LIT(input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "usb-mouse" : "usb-tablet"); + } else { + ADD_ARG_LIT("-usbdevice"); + ADD_ARG_LIT(input->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ? "mouse" : "tablet"); + } } }
@@ -2814,22 +2819,34 @@ int qemudBuildCommandLine(virConnectPtr conn, /* USB */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { - if(hostdev->source.subsys.u.usb.vendor) { - ret = virAsprintf(&usbdev, "host:%.4x:%.4x", - hostdev->source.subsys.u.usb.vendor, - hostdev->source.subsys.u.usb.product);
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ADD_ARG_LIT("-device"); + if (hostdev->source.subsys.u.usb.vendor) { + ret = virAsprintf(&usbdev, "usb-host,vendor=%.4x,product=%.4x", + hostdev->source.subsys.u.usb.vendor, + hostdev->source.subsys.u.usb.product); + } else { + ret = virAsprintf(&usbdev, "usb-host,hostbus=%.3d,hostaddr=%.3d", + hostdev->source.subsys.u.usb.bus, + hostdev->source.subsys.u.usb.device); + } } else { + ADD_ARG_LIT("-usbdevice"); + if (hostdev->source.subsys.u.usb.vendor) { + ret = virAsprintf(&usbdev, "host:%.4x:%.4x", + hostdev->source.subsys.u.usb.vendor, + hostdev->source.subsys.u.usb.product); + } else { ret = virAsprintf(&usbdev, "host:%.3d.%.3d", - hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device); + hostdev->source.subsys.u.usb.bus, + hostdev->source.subsys.u.usb.device); + } } if (ret < 0) goto error;
- ADD_ARG_LIT("-usbdevice"); - ADD_ARG_LIT(usbdev); - VIR_FREE(usbdev); + ADD_ARG(usbdev); }
/* PCI */
ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

The old syntax is -pcidevice host=BUS:SLOT:FUNCTION The new syntax is -device pci-assign,host=BUS:SLOT:FUNCTION --- src/qemu/qemu_conf.c | 26 +++++++++++++++----------- 1 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ac322a7..f8c6ae6 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2852,22 +2852,26 @@ int qemudBuildCommandLine(virConnectPtr conn, /* PCI */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { - if (!(qemuCmdFlags & QEMUD_CMD_FLAG_PCIDEVICE)) { + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ADD_ARG_LIT("-device"); + ret = virAsprintf(&pcidev, "pci-assign,host=%.2x:%.2x.%.1x", + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function); + } else if (qemuCmdFlags & QEMUD_CMD_FLAG_PCIDEVICE) { + ADD_ARG_LIT("-pcidevice"); + ret = virAsprintf(&pcidev, "host=%.2x:%.2x.%.1x", + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function); + } else { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", _("PCI device assignment is not supported by this version of qemu")); goto error; } - ret = virAsprintf(&pcidev, "host=%.2x:%.2x.%.1x", - hostdev->source.subsys.u.pci.bus, - hostdev->source.subsys.u.pci.slot, - hostdev->source.subsys.u.pci.function); - if (ret < 0) { - pcidev = NULL; + if (ret < 0) goto no_memory; - } - ADD_ARG_LIT("-pcidevice"); - ADD_ARG_LIT(pcidev); - VIR_FREE(pcidev); + ADD_ARG(pcidev); } } -- 1.6.5.2

On Tue, Dec 15, 2009 at 03:14:49PM +0000, Daniel P. Berrange wrote:
The old syntax is
-pcidevice host=BUS:SLOT:FUNCTION
The new syntax is
-device pci-assign,host=BUS:SLOT:FUNCTION --- src/qemu/qemu_conf.c | 26 +++++++++++++++----------- 1 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index ac322a7..f8c6ae6 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2852,22 +2852,26 @@ int qemudBuildCommandLine(virConnectPtr conn, /* PCI */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { - if (!(qemuCmdFlags & QEMUD_CMD_FLAG_PCIDEVICE)) { + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ADD_ARG_LIT("-device"); + ret = virAsprintf(&pcidev, "pci-assign,host=%.2x:%.2x.%.1x", + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function); + } else if (qemuCmdFlags & QEMUD_CMD_FLAG_PCIDEVICE) { + ADD_ARG_LIT("-pcidevice"); + ret = virAsprintf(&pcidev, "host=%.2x:%.2x.%.1x", + hostdev->source.subsys.u.pci.bus, + hostdev->source.subsys.u.pci.slot, + hostdev->source.subsys.u.pci.function); + } else { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", _("PCI device assignment is not supported by this version of qemu")); goto error; } - ret = virAsprintf(&pcidev, "host=%.2x:%.2x.%.1x", - hostdev->source.subsys.u.pci.bus, - hostdev->source.subsys.u.pci.slot, - hostdev->source.subsys.u.pci.function); - if (ret < 0) { - pcidev = NULL; + if (ret < 0) goto no_memory; - } - ADD_ARG_LIT("-pcidevice"); - ADD_ARG_LIT(pcidev); - VIR_FREE(pcidev); + ADD_ARG(pcidev); } }
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

The old syntax was -chardev SOMECONFIG -nic user,guestfwd=tcp:IP:PORT-chardev:CHARDEV The new syntax is -chardev SOMECONFIG -netdev user,guestfwd=tcp:IP:PORT,chardev=CHARDEV --- src/qemu/qemu_conf.c | 41 ++++++++++++++++++++++++++++++++--------- src/qemu/qemu_conf.h | 1 + 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f8c6ae6..3d1cafa 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -911,6 +911,8 @@ static unsigned int qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_CHARDEV; if (strstr(help, "-device")) flags |= QEMUD_CMD_FLAG_DEVICE; + if (strstr(help, "-sdl")) + flags |= QEMUD_CMD_FLAG_SDL; if (version >= 9000) flags |= QEMUD_CMD_FLAG_VNC_COLON; @@ -2576,18 +2578,33 @@ int qemudBuildCommandLine(virConnectPtr conn, const char *addr = virSocketFormatAddr(channel->target.addr); int port = virSocketGetPort(channel->target.addr); - virBufferVSprintf(&buf, "user,guestfwd=tcp:%s:%i-chardev:%s", - addr, port, id); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ADD_ARG_LIT("-netdev"); + virBufferVSprintf(&buf, "user,guestfwd=tcp:%s:%i,chardev=%s", + addr, port, id); - VIR_FREE(addr); + VIR_FREE(addr); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto no_memory; + } - ADD_ARG_LIT("-net"); - ADD_ARG(virBufferContentAndReset(&buf)); + ADD_ARG(virBufferContentAndReset(&buf)); + } else { + ADD_ARG_LIT("-net"); + virBufferVSprintf(&buf, "user,guestfwd=tcp:%s:%i-chardev:%s", + addr, port, id); + + VIR_FREE(addr); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto no_memory; + } + + ADD_ARG(virBufferContentAndReset(&buf)); + } } } @@ -2696,6 +2713,12 @@ int qemudBuildCommandLine(virConnectPtr conn, */ ADD_ENV_COPY("QEMU_AUDIO_DRV"); ADD_ENV_COPY("SDL_AUDIODRIVER"); + + /* New QEMU has this flag to let us explicitly ask for + * SDL graphics. This is better than relying on the + * default, since the default changes :-( */ + if (qemuCmdFlags & QEMUD_CMD_FLAG_SDL) + ADD_ARG_LIT("-sdl"); } if (def->nvideos) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 840b749..62f62e5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -77,6 +77,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_0_12 = (1 << 24), QEMUD_CMD_FLAG_MONITOR_JSON = QEMUD_CMD_FLAG_0_12, /* JSON mode for monitor */ QEMUD_CMD_FLAG_DEVICE = (1 << 25), /* Is the new -chardev arg available */ + QEMUD_CMD_FLAG_SDL = (1 << 26), /* Is the new -sdl arg available */ }; /* Main driver state */ -- 1.6.5.2

On Tue, Dec 15, 2009 at 03:14:50PM +0000, Daniel P. Berrange wrote:
The old syntax was
-chardev SOMECONFIG -nic user,guestfwd=tcp:IP:PORT-chardev:CHARDEV
The new syntax is
-chardev SOMECONFIG -netdev user,guestfwd=tcp:IP:PORT,chardev=CHARDEV --- src/qemu/qemu_conf.c | 41 ++++++++++++++++++++++++++++++++--------- src/qemu/qemu_conf.h | 1 + 2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f8c6ae6..3d1cafa 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -911,6 +911,8 @@ static unsigned int qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_CHARDEV; if (strstr(help, "-device")) flags |= QEMUD_CMD_FLAG_DEVICE; + if (strstr(help, "-sdl")) + flags |= QEMUD_CMD_FLAG_SDL;
if (version >= 9000) flags |= QEMUD_CMD_FLAG_VNC_COLON; @@ -2576,18 +2578,33 @@ int qemudBuildCommandLine(virConnectPtr conn, const char *addr = virSocketFormatAddr(channel->target.addr); int port = virSocketGetPort(channel->target.addr);
- virBufferVSprintf(&buf, "user,guestfwd=tcp:%s:%i-chardev:%s", - addr, port, id); + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + ADD_ARG_LIT("-netdev"); + virBufferVSprintf(&buf, "user,guestfwd=tcp:%s:%i,chardev=%s", + addr, port, id);
- VIR_FREE(addr); + VIR_FREE(addr);
- if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - goto no_memory; - } + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto no_memory; + }
- ADD_ARG_LIT("-net"); - ADD_ARG(virBufferContentAndReset(&buf)); + ADD_ARG(virBufferContentAndReset(&buf)); + } else { + ADD_ARG_LIT("-net"); + virBufferVSprintf(&buf, "user,guestfwd=tcp:%s:%i-chardev:%s", + addr, port, id); + + VIR_FREE(addr); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + goto no_memory; + } + + ADD_ARG(virBufferContentAndReset(&buf)); + } } }
@@ -2696,6 +2713,12 @@ int qemudBuildCommandLine(virConnectPtr conn, */ ADD_ENV_COPY("QEMU_AUDIO_DRV"); ADD_ENV_COPY("SDL_AUDIODRIVER"); + + /* New QEMU has this flag to let us explicitly ask for + * SDL graphics. This is better than relying on the + * default, since the default changes :-( */ + if (qemuCmdFlags & QEMUD_CMD_FLAG_SDL) + ADD_ARG_LIT("-sdl"); }
if (def->nvideos) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 840b749..62f62e5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -77,6 +77,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_0_12 = (1 << 24), QEMUD_CMD_FLAG_MONITOR_JSON = QEMUD_CMD_FLAG_0_12, /* JSON mode for monitor */ QEMUD_CMD_FLAG_DEVICE = (1 << 25), /* Is the new -chardev arg available */ + QEMUD_CMD_FLAG_SDL = (1 << 26), /* Is the new -sdl arg available */ };
/* Main driver state */
ACK, the -sdl option handling feels like a separate change but fine Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Tue, Dec 15, 2009 at 03:14:40PM +0000, Daniel P. Berrange wrote:
QEMU has introduced a new command line arg called -device for specifying the guest device config. This allows separation from the host device config. libvirt needs to use this new syntax so that we can specify static PCI addresses for devices (the old syntax did not allow this).
This patch series is the quick conversion I've done so far. The most obvious thing still lacking here is new test cases. I want to wait until this series merges with the device addressing series before adding those, since one set of tests can cover both patch series.
In addition the handling of disk drivers is not quite correct. We need the disk controller patches myself/Wolfgang have worked on in order to specify the disk bus/unit instead of index.
ACK, let's push the two sets Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel P. Berrange
-
Daniel Veillard