[libvirt] [PATCH 0/3] qemu: improve device capability detection

As promised in https://www.redhat.com/archives/libvir-list/2011-January/msg00562.html Patch 1 is complete, patch 2 can be pushed as-is, but if anyone has qemu-0.12.1 or qemu-kvm-0.12.3 handy (Fedora12 has 0.11.x, and Fedora13 has 0.12.5), then we can enhance it by filling in the contents of a couple more files. Patch 3 does not need to be pushed until the rest of my smartcard series is complete, but I'm including it now to show how to rewrite things in order to parse more device characteristics from a single qemu run, so it can be reused by the boot order patch series (add "-device","virtio-blk-pci,?" to extract; add strstr(str,"virtio-blk-pci.bootindex") to parse; and possibly update the qemuhelpdata/*-device files to include appropriate new lines to trigger the new flag detection). Eric Blake (3): qemu: convert capabilities to use virCommand qemu: improve device flag parsing smartcard: check for qemu capability src/qemu/qemu_capabilities.c | 229 +++++++------------- src/qemu/qemu_capabilities.h | 5 +- tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device | 57 +++++ tests/qemuhelpdata/qemu-kvm-0.13.0-device | 70 ++++++ tests/qemuhelptest.c | 48 +++- 5 files changed, 244 insertions(+), 165 deletions(-) create mode 100644 tests/qemuhelpdata/qemu-0.12.1-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.3-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.13.0-device -- 1.7.3.4

* src/qemu/qemu_capabilities.c (qemuCapsProbeMachineTypes) (qemuCapsProbeCPUModels, qemuCapsParsePCIDeviceStrs) (qemuCapsExtractVersionInfo): Use virCommand rather than virExec. --- src/qemu/qemu_capabilities.c | 184 ++++++++++-------------------------------- 1 files changed, 42 insertions(+), 142 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3d10b42..9bab317 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1,7 +1,7 @@ /* * qemu_capabilities.c: QEMU capabilities generation * - * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc. + * Copyright (C) 2006-2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -33,6 +33,7 @@ #include "cpu/cpu.h" #include "domain_conf.h" #include "qemu_conf.h" +#include "command.h" #include <sys/stat.h> #include <unistd.h> @@ -167,52 +168,26 @@ qemuCapsProbeMachineTypes(const char *binary, virCapsGuestMachinePtr **machines, int *nmachines) { - const char *const qemuarg[] = { binary, "-M", "?", NULL }; - const char *const qemuenv[] = { "LC_ALL=C", NULL }; char *output; - enum { MAX_MACHINES_OUTPUT_SIZE = 1024*4 }; - pid_t child; - int newstdout = -1, len; - int ret = -1, status; + int ret = -1; + virCommandPtr cmd; - if (virExec(qemuarg, qemuenv, NULL, - &child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0) - return -1; + cmd = virCommandNewArgList(binary, "-M", "?", NULL); + virCommandAddEnvPassCommon(cmd); + virCommandSetOutputBuffer(cmd, &output); + virCommandClearCaps(cmd); - len = virFileReadLimFD(newstdout, MAX_MACHINES_OUTPUT_SIZE, &output); - if (len < 0) { - virReportSystemError(errno, "%s", - _("Unable to read 'qemu -M ?' output")); + if (virCommandRun(cmd, NULL) < 0) goto cleanup; - } if (qemuCapsParseMachineTypesStr(output, machines, nmachines) < 0) - goto cleanup2; + goto cleanup; ret = 0; -cleanup2: - VIR_FREE(output); cleanup: - if (VIR_CLOSE(newstdout) < 0) - ret = -1; - -rewait: - if (waitpid(child, &status, 0) != child) { - if (errno == EINTR) - goto rewait; - - VIR_ERROR(_("Unexpected exit status from qemu %d pid %lu"), - WEXITSTATUS(status), (unsigned long)child); - ret = -1; - } - /* Check & log unexpected exit status, but don't fail, - * as there's really no need to throw an error if we did - * actually read a valid version number above */ - if (WEXITSTATUS(status) != 0) { - VIR_WARN("Unexpected exit status '%d', qemu probably failed", - WEXITSTATUS(status)); - } + VIR_FREE(output); + virCommandFree(cmd); return ret; } @@ -396,21 +371,10 @@ qemuCapsProbeCPUModels(const char *qemu, unsigned int *count, const char ***cpus) { - const char *const qemuarg[] = { - qemu, - "-cpu", "?", - (qemuCmdFlags & QEMUD_CMD_FLAG_NODEFCONFIG) ? "-nodefconfig" : NULL, - NULL - }; - const char *const qemuenv[] = { "LC_ALL=C", NULL }; - enum { MAX_MACHINES_OUTPUT_SIZE = 1024*4 }; char *output = NULL; - int newstdout = -1; int ret = -1; - pid_t child; - int status; - int len; qemuCapsParseCPUModels parse; + virCommandPtr cmd; if (count) *count = 0; @@ -424,16 +388,15 @@ qemuCapsProbeCPUModels(const char *qemu, return 0; } - if (virExec(qemuarg, qemuenv, NULL, - &child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0) - return -1; + cmd = virCommandNewArgList(qemu, "-cpu", "?", NULL); + if (qemuCmdFlags & QEMUD_CMD_FLAG_NODEFCONFIG) + virCommandAddArg(cmd, "-nodefconfig"); + virCommandAddEnvPassCommon(cmd); + virCommandSetOutputBuffer(cmd, &output); + virCommandClearCaps(cmd); - len = virFileReadLimFD(newstdout, MAX_MACHINES_OUTPUT_SIZE, &output); - if (len < 0) { - virReportSystemError(errno, "%s", - _("Unable to read QEMU supported CPU models")); + if (virCommandRun(cmd, NULL) < 0) goto cleanup; - } if (parse(output, count, cpus) < 0) { virReportOOMError(); @@ -444,25 +407,7 @@ qemuCapsProbeCPUModels(const char *qemu, cleanup: VIR_FREE(output); - if (VIR_CLOSE(newstdout) < 0) - ret = -1; - -rewait: - if (waitpid(child, &status, 0) != child) { - if (errno == EINTR) - goto rewait; - - VIR_ERROR(_("Unexpected exit status from qemu %d pid %lu"), - WEXITSTATUS(status), (unsigned long)child); - ret = -1; - } - /* Check & log unexpected exit status, but don't fail, - * as there's really no need to throw an error if we did - * actually read a valid version number above */ - if (WEXITSTATUS(status) != 0) { - VIR_WARN("Unexpected exit status '%d', qemu probably failed", - WEXITSTATUS(status)); - } + virCommandFree(cmd); return ret; } @@ -1091,57 +1036,35 @@ static void qemuCapsParsePCIDeviceStrs(const char *qemu, unsigned long long *flags) { - const char *const qemuarg[] = { qemu, "-device", "pci-assign,?", NULL }; - const char *const qemuenv[] = { "LC_ALL=C", NULL }; - pid_t child; - int status; - int newstderr = -1; + char *pciassign = NULL; + virCommandPtr cmd; - if (virExec(qemuarg, qemuenv, NULL, - &child, -1, NULL, &newstderr, VIR_EXEC_CLEAR_CAPS) < 0) - return; + cmd = virCommandNewArgList(qemu, "-device", "pci-assign,?", NULL); + virCommandAddEnvPassCommon(cmd); + /* qemu -help goes to stdout, but qemu -device ? goes to stderr. */ + virCommandSetErrorBuffer(cmd, &pciassign); + virCommandClearCaps(cmd); - char *pciassign = NULL; - enum { MAX_PCI_OUTPUT_SIZE = 1024*4 }; - int len = virFileReadLimFD(newstderr, MAX_PCI_OUTPUT_SIZE, &pciassign); - if (len < 0) { - virReportSystemError(errno, - _("Unable to read %s pci-assign device output"), - qemu); + if (virCommandRun(cmd, NULL) < 0) goto cleanup; - } if (strstr(pciassign, "pci-assign.configfd")) *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD; cleanup: VIR_FREE(pciassign); - VIR_FORCE_CLOSE(newstderr); -rewait: - if (waitpid(child, &status, 0) != child) { - if (errno == EINTR) - goto rewait; - - VIR_ERROR(_("Unexpected exit status from qemu %d pid %lu"), - WEXITSTATUS(status), (unsigned long)child); - } - if (WEXITSTATUS(status) != 0) { - VIR_WARN("Unexpected exit status '%d', qemu probably failed", - WEXITSTATUS(status)); - } + virCommandFree(cmd); } int qemuCapsExtractVersionInfo(const char *qemu, unsigned int *retversion, unsigned long long *retflags) { - const char *const qemuarg[] = { qemu, "-help", NULL }; - const char *const qemuenv[] = { "LC_ALL=C", NULL }; - pid_t child; - int newstdout = -1; - int ret = -1, status; + int ret = -1; unsigned int version, is_kvm, kvm_version; unsigned long long flags = 0; + char *help = NULL; + virCommandPtr cmd; if (retflags) *retflags = 0; @@ -1157,22 +1080,17 @@ int qemuCapsExtractVersionInfo(const char *qemu, return -1; } - if (virExec(qemuarg, qemuenv, NULL, - &child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0) - return -1; + cmd = virCommandNewArgList(qemu, "-help", NULL); + virCommandAddEnvPassCommon(cmd); + virCommandSetOutputBuffer(cmd, &help); + virCommandClearCaps(cmd); - char *help = NULL; - enum { MAX_HELP_OUTPUT_SIZE = 1024*64 }; - int len = virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, &help); - if (len < 0) { - virReportSystemError(errno, - _("Unable to read %s help output"), qemu); - goto cleanup2; - } + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; if (qemuCapsParseHelpStr(qemu, help, &flags, &version, &is_kvm, &kvm_version) == -1) - goto cleanup2; + goto cleanup; if (flags & QEMUD_CMD_FLAG_DEVICE) qemuCapsParsePCIDeviceStrs(qemu, &flags); @@ -1184,27 +1102,9 @@ int qemuCapsExtractVersionInfo(const char *qemu, ret = 0; -cleanup2: +cleanup: VIR_FREE(help); - if (VIR_CLOSE(newstdout) < 0) - ret = -1; - -rewait: - if (waitpid(child, &status, 0) != child) { - if (errno == EINTR) - goto rewait; - - VIR_ERROR(_("Unexpected exit status from qemu %d pid %lu"), - WEXITSTATUS(status), (unsigned long)child); - ret = -1; - } - /* Check & log unexpected exit status, but don't fail, - * as there's really no need to throw an error if we did - * actually read a valid version number above */ - if (WEXITSTATUS(status) != 0) { - VIR_WARN("Unexpected exit status '%d', qemu probably failed", - WEXITSTATUS(status)); - } + virCommandFree(cmd); return ret; } -- 1.7.3.4

On 01/13/2011 02:10 PM, Eric Blake wrote:
* src/qemu/qemu_capabilities.c (qemuCapsProbeMachineTypes) (qemuCapsProbeCPUModels, qemuCapsParsePCIDeviceStrs) (qemuCapsExtractVersionInfo): Use virCommand rather than virExec. --- src/qemu/qemu_capabilities.c | 184 ++++++++++-------------------------------- 1 files changed, 42 insertions(+), 142 deletions(-)
ACK. That all looks pretty straightforward.

On 01/13/2011 12:49 PM, Laine Stump wrote:
On 01/13/2011 02:10 PM, Eric Blake wrote:
* src/qemu/qemu_capabilities.c (qemuCapsProbeMachineTypes) (qemuCapsProbeCPUModels, qemuCapsParsePCIDeviceStrs) (qemuCapsExtractVersionInfo): Use virCommand rather than virExec. --- src/qemu/qemu_capabilities.c | 184 ++++++++++-------------------------------- 1 files changed, 42 insertions(+), 142 deletions(-)
ACK. That all looks pretty straightforward.
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

* src/qemu/qemu_capabilities.h (qemuCapsParseDeviceStr): New prototype. * src/qemu/qemu_capabilities.c (qemuCapsParsePCIDeviceStrs) Rename and split... (qemuCapsExtractDeviceStr, qemuCapsParseDeviceStr): ...to make it easier to add and test device-specific checks. (qemuCapsExtractVersionInfo): Update caller. * tests/qemuhelptest.c (testHelpStrParsing): Also test parsing of device-related flags. (mymain): Update expected flags. * tests/qemuhelpdata/qemu-0.12.1-device: New file. * tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device: New file. * tests/qemuhelpdata/qemu-kvm-0.12.3-device: New file. * tests/qemuhelpdata/qemu-kvm-0.13.0-device: New file. --- src/qemu/qemu_capabilities.c | 41 ++++++++--- src/qemu/qemu_capabilities.h | 2 + tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device | 57 ++++++++++++++++ tests/qemuhelpdata/qemu-kvm-0.13.0-device | 70 ++++++++++++++++++++ tests/qemuhelptest.c | 48 ++++++++++---- 5 files changed, 195 insertions(+), 23 deletions(-) create mode 100644 tests/qemuhelpdata/qemu-0.12.1-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.3-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.13.0-device diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9bab317..f967255 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1032,28 +1032,46 @@ fail: return -1; } -static void -qemuCapsParsePCIDeviceStrs(const char *qemu, - unsigned long long *flags) +static int +qemuCapsExtractDeviceStr(const char *qemu, + unsigned long long *flags) { - char *pciassign = NULL; + char *output = NULL; virCommandPtr cmd; + int ret = -1; - cmd = virCommandNewArgList(qemu, "-device", "pci-assign,?", NULL); + /* Cram together all device-related queries into one invocation; + * the output format makes it possible to distinguish what we + * need. Unrecognized '-device bogus,?' cause an error in + * isolation, but are silently ignored in combination with + * '-device ?'. */ + cmd = virCommandNewArgList(qemu, + "-device", "pci-assign,?", + NULL); virCommandAddEnvPassCommon(cmd); /* qemu -help goes to stdout, but qemu -device ? goes to stderr. */ - virCommandSetErrorBuffer(cmd, &pciassign); + virCommandSetErrorBuffer(cmd, &output); virCommandClearCaps(cmd); if (virCommandRun(cmd, NULL) < 0) goto cleanup; - if (strstr(pciassign, "pci-assign.configfd")) - *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD; + ret = qemuCapsParseDeviceStr(output, flags); cleanup: - VIR_FREE(pciassign); + VIR_FREE(output); virCommandFree(cmd); + return ret; +} + + +int +qemuCapsParseDeviceStr(const char *str, unsigned long long *flags) +{ + if (strstr(str, "pci-assign.configfd")) + *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD; + + return 0; } int qemuCapsExtractVersionInfo(const char *qemu, @@ -1092,8 +1110,9 @@ int qemuCapsExtractVersionInfo(const char *qemu, &version, &is_kvm, &kvm_version) == -1) goto cleanup; - if (flags & QEMUD_CMD_FLAG_DEVICE) - qemuCapsParsePCIDeviceStrs(qemu, &flags); + if ((flags & QEMUD_CMD_FLAG_DEVICE) && + qemuCapsExtractDeviceStr(qemu, &flags) < 0) + goto cleanup; if (retversion) *retversion = version; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index ee648f0..8057479 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -109,6 +109,8 @@ int qemuCapsParseHelpStr(const char *qemu, unsigned int *version, unsigned int *is_kvm, unsigned int *kvm_version); +int qemuCapsParseDeviceStr(const char *str, + unsigned long long *qemuCmdFlags); #endif /* __QEMU_CAPABILITIES_H__*/ diff --git a/tests/qemuhelpdata/qemu-0.12.1-device b/tests/qemuhelpdata/qemu-0.12.1-device new file mode 100644 index 0000000..e69de29 diff --git a/tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device b/tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device new file mode 100644 index 0000000..d20fb7d --- /dev/null +++ b/tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device @@ -0,0 +1,57 @@ +name "pci-bridge", bus PCI +name "virtio-balloon-pci", bus PCI +name "virtio-serial-pci", bus PCI, alias "virtio-serial" +name "virtio-net-pci", bus PCI +name "virtio-blk-pci", bus PCI +name "i82562", bus PCI +name "i82559er", bus PCI +name "i82559c", bus PCI +name "i82559b", bus PCI +name "i82559a", bus PCI +name "i82558b", bus PCI +name "i82558a", bus PCI +name "i82557c", bus PCI +name "i82557b", bus PCI +name "i82557a", bus PCI +name "i82551", bus PCI +name "i82550", bus PCI +name "pcnet", bus PCI +name "rtl8139", bus PCI +name "e1000", bus PCI, desc "Intel Gigabit Ethernet" +name "ide-drive", bus IDE +name "isa-ide", bus ISA +name "ES1370", bus PCI, desc "ENSONIQ AudioPCI ES1370" +name "AC97", bus PCI, desc "Intel 82801AA AC97 Audio" +name "VGA", bus PCI +name "SUNW,fdtwo", bus System +name "sysbus-fdc", bus System +name "isa-serial", bus ISA +name "cirrus-vga", bus PCI, desc "Cirrus CLGD 54xx VGA" +name "isa-parallel", bus ISA +name "piix4-usb-uhci", bus PCI +name "piix3-usb-uhci", bus PCI +name "vmware-svga", bus PCI +name "ib700", bus ISA +name "ne2k_isa", bus ISA +name "testdev", bus ISA +name "pci-assign", bus PCI, desc "pass through host pci devices to the guest" +name "qxl", bus PCI, desc "Spice QXL GPU" +name "spicevmc", bus virtio-serial-bus +name "smbus-eeprom", bus I2C +name "usb-hub", bus USB +name "usb-host", bus USB +name "usb-kbd", bus USB +name "usb-mouse", bus USB +name "usb-tablet", bus USB +name "usb-wacom-tablet", bus USB, desc "QEMU PenPartner Tablet" +name "usb-braille", bus USB +name "usb-serial", bus USB +name "usb-net", bus USB +name "usb-bt-dongle", bus USB +name "virtserialport", bus virtio-serial-bus +name "virtconsole", bus virtio-serial-bus +name "i6300esb", bus PCI +name "ne2k_pci", bus PCI +pci-assign.host=pci-hostaddr +pci-assign.iommu=uint32 +pci-assign.configfd=string diff --git a/tests/qemuhelpdata/qemu-kvm-0.12.3-device b/tests/qemuhelpdata/qemu-kvm-0.12.3-device new file mode 100644 index 0000000..e69de29 diff --git a/tests/qemuhelpdata/qemu-kvm-0.13.0-device b/tests/qemuhelpdata/qemu-kvm-0.13.0-device new file mode 100644 index 0000000..b121257 --- /dev/null +++ b/tests/qemuhelpdata/qemu-kvm-0.13.0-device @@ -0,0 +1,70 @@ +name "pci-bridge", bus PCI +name "virtio-balloon-pci", bus PCI +name "virtio-serial-pci", bus PCI, alias "virtio-serial" +name "virtio-net-pci", bus PCI +name "virtio-blk-pci", bus PCI +name "sysbus-ohci", bus System, desc "OHCI USB Controller" +name "pci-ohci", bus PCI, desc "Apple USB Controller" +name "rtl8139", bus PCI +name "e1000", bus PCI, desc "Intel Gigabit Ethernet" +name "ivshmem", bus PCI +name "smbus-eeprom", bus I2C +name "scsi-disk", bus SCSI, desc "virtual scsi disk or cdrom" +name "scsi-generic", bus SCSI, desc "pass through generic scsi device (/dev/sg*)" +name "usb-hub", bus USB +name "usb-host", bus USB +name "usb-kbd", bus USB +name "usb-mouse", bus USB +name "usb-tablet", bus USB +name "usb-storage", bus USB +name "usb-wacom-tablet", bus USB, desc "QEMU PenPartner Tablet" +name "usb-braille", bus USB +name "usb-serial", bus USB +name "usb-net", bus USB +name "usb-bt-dongle", bus USB +name "virtconsole", bus virtio-serial-bus +name "virtserialport", bus virtio-serial-bus +name "isa-serial", bus ISA +name "isa-parallel", bus ISA +name "vt82c686b-usb-uhci", bus PCI +name "piix4-usb-uhci", bus PCI +name "piix3-usb-uhci", bus PCI +name "SUNW,fdtwo", bus System +name "sysbus-fdc", bus System +name "i6300esb", bus PCI +name "ne2k_pci", bus PCI +name "i82801", bus PCI, desc "Intel i82801 Ethernet" +name "i82562", bus PCI, desc "Intel i82562 Ethernet" +name "i82559er", bus PCI, desc "Intel i82559ER Ethernet" +name "i82559c", bus PCI, desc "Intel i82559C Ethernet" +name "i82559b", bus PCI, desc "Intel i82559B Ethernet" +name "i82559a", bus PCI, desc "Intel i82559A Ethernet" +name "i82558b", bus PCI, desc "Intel i82558B Ethernet" +name "i82558a", bus PCI, desc "Intel i82558A Ethernet" +name "i82557c", bus PCI, desc "Intel i82557C Ethernet" +name "i82557b", bus PCI, desc "Intel i82557B Ethernet" +name "i82557a", bus PCI, desc "Intel i82557A Ethernet" +name "i82551", bus PCI, desc "Intel i82551 Ethernet" +name "i82550", bus PCI, desc "Intel i82550 Ethernet" +name "pcnet", bus PCI +name "ne2k_isa", bus ISA +name "ide-drive", bus IDE +name "isa-ide", bus ISA +name "lsi53c895a", bus PCI, alias "lsi" +name "VGA", bus PCI +name "vmware-svga", bus PCI +name "sb16", bus ISA, desc "Creative Sound Blaster 16" +name "ES1370", bus PCI, desc "ENSONIQ AudioPCI ES1370" +name "AC97", bus PCI, desc "Intel 82801AA AC97 Audio" +name "cirrus-vga", bus PCI, desc "Cirrus CLGD 54xx VGA" +name "isa-applesmc", bus ISA +name "ib700", bus ISA +name "isa-debugcon", bus ISA +name "testdev", bus ISA +name "PIIX4_PM", bus PCI, desc "PM" +name "qxl", bus PCI, desc "Spice QXL GPU" +name "spicevmc", bus virtio-serial-bus +name "pci-assign", bus PCI, desc "pass through host pci devices to the guest" +pci-assign.host=pci-hostaddr +pci-assign.iommu=uint32 +pci-assign.configfd=string diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 18a71fa..5d78e2d 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -7,6 +7,7 @@ # include "testutils.h" # include "qemu/qemu_capabilities.h" +# include "memory.h" # define MAX_HELP_OUTPUT_SIZE 1024*64 @@ -39,50 +40,71 @@ static void printMismatchedFlags(unsigned long long got, static int testHelpStrParsing(const void *data) { const struct testInfo *info = data; - char path[PATH_MAX]; + char *path = NULL; char helpStr[MAX_HELP_OUTPUT_SIZE]; char *help = &(helpStr[0]); unsigned int version, is_kvm, kvm_version; unsigned long long flags; + int ret = -1; - snprintf(path, PATH_MAX, "%s/qemuhelpdata/%s", abs_srcdir, info->name); + if (virAsprintf(&path, "%s/qemuhelpdata/%s", abs_srcdir, info->name) < 0) + return -1; if (virtTestLoadFile(path, &help, MAX_HELP_OUTPUT_SIZE) < 0) - return -1; + goto cleanup; if (qemuCapsParseHelpStr("QEMU", help, &flags, &version, &is_kvm, &kvm_version) == -1) - return -1; + goto cleanup; + + if (info->flags & QEMUD_CMD_FLAG_DEVICE) { + VIR_FREE(path); + if (virAsprintf(&path, "%s/qemuhelpdata/%s-device", abs_srcdir, + info->name) < 0) + goto cleanup; + + if (virtTestLoadFile(path, &help, MAX_HELP_OUTPUT_SIZE) < 0) + goto cleanup; + + if (qemuCapsParseDeviceStr(help, &flags) < 0) + goto cleanup; + } if (flags != info->flags) { - fprintf(stderr, "Computed flags do not match: got 0x%llx, expected 0x%llx\n", + fprintf(stderr, + "Computed flags do not match: got 0x%llx, expected 0x%llx\n", flags, info->flags); if (getenv("VIR_TEST_DEBUG")) printMismatchedFlags(flags, info->flags); - return -1; + goto cleanup; } if (version != info->version) { fprintf(stderr, "Parsed versions do not match: got %u, expected %u\n", version, info->version); - return -1; + goto cleanup; } if (is_kvm != info->is_kvm) { - fprintf(stderr, "Parsed is_kvm flag does not match: got %u, expected %u\n", + fprintf(stderr, + "Parsed is_kvm flag does not match: got %u, expected %u\n", is_kvm, info->is_kvm); - return -1; + goto cleanup; } if (kvm_version != info->kvm_version) { - fprintf(stderr, "Parsed KVM versions do not match: got %u, expected %u\n", + fprintf(stderr, + "Parsed KVM versions do not match: got %u, expected %u\n", kvm_version, info->kvm_version); - return -1; + goto cleanup; } - return 0; + ret = 0; +cleanup: + VIR_FREE(path); + return ret; } static int @@ -318,6 +340,7 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_VNET_HOST | QEMUD_CMD_FLAG_NO_KVM_PIT | QEMUD_CMD_FLAG_TDF | + QEMUD_CMD_FLAG_PCI_CONFIGFD | QEMUD_CMD_FLAG_NODEFCONFIG | QEMUD_CMD_FLAG_BOOT_MENU | QEMUD_CMD_FLAG_NESTING | @@ -399,6 +422,7 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_NO_HPET | QEMUD_CMD_FLAG_NO_KVM_PIT | QEMUD_CMD_FLAG_TDF | + QEMUD_CMD_FLAG_PCI_CONFIGFD | QEMUD_CMD_FLAG_NODEFCONFIG | QEMUD_CMD_FLAG_BOOT_MENU | QEMUD_CMD_FLAG_FSDEV | -- 1.7.3.4

* src/qemu/qemu_capabilities.h (qemuCapsParseDeviceStr): New prototype. * src/qemu/qemu_capabilities.c (qemuCapsParsePCIDeviceStrs) Rename and split... (qemuCapsExtractDeviceStr, qemuCapsParseDeviceStr): ...to make it easier to add and test device-specific checks. (qemuCapsExtractVersionInfo): Update caller. * tests/qemuhelptest.c (testHelpStrParsing): Also test parsing of device-related flags. (mymain): Update expected flags. * tests/qemuhelpdata/qemu-0.12.1-device: New file. * tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device: New file. * tests/qemuhelpdata/qemu-kvm-0.12.3-device: New file. * tests/qemuhelpdata/qemu-kvm-0.13.0-device: New file. --- src/qemu/qemu_capabilities.c | 41 ++++++++--- src/qemu/qemu_capabilities.h | 2 + tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device | 57 ++++++++++++++++ tests/qemuhelpdata/qemu-kvm-0.13.0-device | 70 ++++++++++++++++++++ tests/qemuhelptest.c | 48 ++++++++++---- 5 files changed, 195 insertions(+), 23 deletions(-) create mode 100644 tests/qemuhelpdata/qemu-0.12.1-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.3-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.13.0-device
Nice and clean, thanks. I think this can be pushed without test data for qemu-0.12.1 and qemu-kvm-0.12.3. The -device files for them can be filled with data later. ACK Jirka

On 01/14/2011 08:03 AM, Jiri Denemark wrote:
* src/qemu/qemu_capabilities.h (qemuCapsParseDeviceStr): New prototype. * src/qemu/qemu_capabilities.c (qemuCapsParsePCIDeviceStrs) Rename and split... (qemuCapsExtractDeviceStr, qemuCapsParseDeviceStr): ...to make it easier to add and test device-specific checks.
Nice and clean, thanks.
I think this can be pushed without test data for qemu-0.12.1 and qemu-kvm-0.12.3. The -device files for them can be filled with data later.
ACK
I've gone ahead and pushed it as-is. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

At 01/14/2011 03:10 AM, Eric Blake Write:
* src/qemu/qemu_capabilities.h (qemuCapsParseDeviceStr): New prototype. * src/qemu/qemu_capabilities.c (qemuCapsParsePCIDeviceStrs) Rename and split... (qemuCapsExtractDeviceStr, qemuCapsParseDeviceStr): ...to make it easier to add and test device-specific checks. (qemuCapsExtractVersionInfo): Update caller. * tests/qemuhelptest.c (testHelpStrParsing): Also test parsing of device-related flags. (mymain): Update expected flags. * tests/qemuhelpdata/qemu-0.12.1-device: New file. * tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device: New file. * tests/qemuhelpdata/qemu-kvm-0.12.3-device: New file. * tests/qemuhelpdata/qemu-kvm-0.13.0-device: New file. --- src/qemu/qemu_capabilities.c | 41 ++++++++--- src/qemu/qemu_capabilities.h | 2 + tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device | 57 ++++++++++++++++ tests/qemuhelpdata/qemu-kvm-0.13.0-device | 70 ++++++++++++++++++++ tests/qemuhelptest.c | 48 ++++++++++---- 5 files changed, 195 insertions(+), 23 deletions(-) create mode 100644 tests/qemuhelpdata/qemu-0.12.1-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.3-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.13.0-device
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9bab317..f967255 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1032,28 +1032,46 @@ fail: return -1; }
-static void -qemuCapsParsePCIDeviceStrs(const char *qemu, - unsigned long long *flags) +static int +qemuCapsExtractDeviceStr(const char *qemu, + unsigned long long *flags) { - char *pciassign = NULL; + char *output = NULL; virCommandPtr cmd; + int ret = -1;
- cmd = virCommandNewArgList(qemu, "-device", "pci-assign,?", NULL); + /* Cram together all device-related queries into one invocation; + * the output format makes it possible to distinguish what we + * need. Unrecognized '-device bogus,?' cause an error in + * isolation, but are silently ignored in combination with + * '-device ?'. */ + cmd = virCommandNewArgList(qemu, + "-device", "pci-assign,?", + NULL);
The qemu that I used does not support '-device pci-assign,?'... So, I can not start the guest with this patch. The qemu is cloned from here: http://git.qemu.org/git/qemu.git
virCommandAddEnvPassCommon(cmd); /* qemu -help goes to stdout, but qemu -device ? goes to stderr. */ - virCommandSetErrorBuffer(cmd, &pciassign); + virCommandSetErrorBuffer(cmd, &output); virCommandClearCaps(cmd);
if (virCommandRun(cmd, NULL) < 0) goto cleanup;
- if (strstr(pciassign, "pci-assign.configfd")) - *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD; + ret = qemuCapsParseDeviceStr(output, flags);
cleanup: - VIR_FREE(pciassign); + VIR_FREE(output); virCommandFree(cmd); + return ret; +} + +

On 01/17/2011 10:39 PM, Wen Congyang wrote:
+ cmd = virCommandNewArgList(qemu, + "-device", "pci-assign,?", + NULL);
The qemu that I used does not support '-device pci-assign,?'... So, I can not start the guest with this patch.
The qemu is cloned from here: http://git.qemu.org/git/qemu.git
What do the following commands produce (assuming that both qemu and qemu-kvm on your PATH are the binaries that you are testing with)? qemu -version qemu-kvm -version qemu -device '?' echo $? qemu -device 'pci-assign,?' echo $? qemu -device '?' -device 'pci-assign,?' echo $? qemu-kvm -device '?' echo $? qemu-kvm -device 'pci-assign,?' echo $? qemu-kvm -device '?' -device 'pci-assign,?' echo $? Depending on those results, I will probably be able to come up with a followup patch that helps you run again (it may just be a matter of ignoring non-zero exit status, or of adding the extra "-device","?", arguments in order to ensure that qemu doesn't error out on unknown devices). For example, in my case, I know that with the qemu 0.13.0 package in Fedora 14, 'qemu -device pci-assign,?' fails but 'qemu-kvm -device pci-assign,?' succeeds; and for both binaries, '$binary -device ? -device pci-assign,?' succeeds but has more output for qemu-kvm than for qemu. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

[replying to myself] On 01/18/2011 08:16 AM, Eric Blake wrote:
On 01/17/2011 10:39 PM, Wen Congyang wrote:
+ cmd = virCommandNewArgList(qemu, + "-device", "pci-assign,?", + NULL);
The qemu that I used does not support '-device pci-assign,?'... So, I can not start the guest with this patch.
The qemu is cloned from here: http://git.qemu.org/git/qemu.git
What do the following commands produce (assuming that both qemu and qemu-kvm on your PATH are the binaries that you are testing with)?
qemu -version qemu-kvm -version qemu -device '?' echo $? qemu -device 'pci-assign,?' echo $? qemu -device '?' -device 'pci-assign,?' echo $? qemu-kvm -device '?' echo $? qemu-kvm -device 'pci-assign,?' echo $? qemu-kvm -device '?' -device 'pci-assign,?' echo $?
I may have reproduced your problem. With a Fedora 13 VM (no nested kvm support), I see: $ qemu -version QEMU PC emulator version 0.12.5 (qemu-kvm-0.12.5), Copyright (c) 2003-2008 Fabrice Bellard $ qemu-kvm -device pci-assign,? open /dev/kvm: No such file or directory Could not initialize KVM, will disable KVM support property "pci-assign.?" not found can't set property "?" to "on" for "pci-assign" $ echo $? 1 $ qemu -device pci-assign,? The pci-assign device has not been found $ echo $? 1 $ qemu -device ? name "pci-bridge", bus PCI ... name "isabus-bridge", bus System, no-user $ echo ? 1 $ qemu -device pci-bridge,? property "pci-bridge.?" not found can't set property "?" to "on" for "pci-bridge" $ echo ? 1 Looks like -device was added in qemu 0.12.x, but device-specific property parsing via -device name,? wasn't added until 0.13.x. Furthermore, -device ? works in both versions to list available devices, but in 0.12.x it always makes qemu fail with non-zero status (which we can probably safely ignore). Thankfully, this is reflected in -help output: 0.12.5: -device driver[,options] add device -name ... 0.13.0: -device driver[,prop[=value][,...]] add device (based on driver) prop=value,... sets driver properties use -device ? to print all possible drivers use -device driver,? to print all possible properties File ... In fact, rather than keying off of QEMUD_CMD_FLAG_DEVICE (which is whether -device is present at all) to call qemuCapsExtractDeviceStr, it may be as simple as keying off of "-device driver,?" in the -help output. Patch coming up shortly... -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Fixes regression introduced in commit 2211518, where all qemu 0.12.x fails to start, as does qemu 0.13.x lacking the pci-assign device. * src/qemu/qemu_capabilities.c (qemuCapsExtractVersionInfo): Check for -device driver,? support. (qemuCapsExtractDeviceStr): Avoid failure if all probed devices are unsupported. Reported by Ken Congyang. --- src/qemu/qemu_capabilities.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1f0a3c2..faf7d44 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1048,10 +1048,14 @@ qemuCapsExtractDeviceStr(const char *qemu, /* Cram together all device-related queries into one invocation; * the output format makes it possible to distinguish what we - * need. Unrecognized '-device bogus,?' cause an error in - * isolation, but are silently ignored in combination with - * '-device ?'. */ + * need. With qemu 0.13.0 and later, unrecognized '-device + * bogus,?' cause an error in isolation, but are silently ignored + * in combination with '-device ?'. Qemu 0.12.x doesn't + * understand '-device name,?', and always exits with status 1 for + * the simpler '-device ?', so this function is really only useful + * for parsing out features added in 0.13.0 or later. */ cmd = virCommandNewArgList(qemu, + "-device", "?", "-device", "pci-assign,?", "-device", "virtio-blk-pci,?", NULL); @@ -1119,7 +1123,10 @@ int qemuCapsExtractVersionInfo(const char *qemu, &version, &is_kvm, &kvm_version) == -1) goto cleanup; + /* Only call qemuCapsExtractDeviceStr for qemu 0.13.0+, since it + * won't set any additional flags for qemu 0.12.x. */ if ((flags & QEMUD_CMD_FLAG_DEVICE) && + strstr(help, "-device driver,?") && qemuCapsExtractDeviceStr(qemu, &flags) < 0) goto cleanup; -- 1.7.3.4

2011/1/18 Eric Blake <eblake@redhat.com>:
Fixes regression introduced in commit 2211518, where all qemu 0.12.x fails to start, as does qemu 0.13.x lacking the pci-assign device.
* src/qemu/qemu_capabilities.c (qemuCapsExtractVersionInfo): Check for -device driver,? support. (qemuCapsExtractDeviceStr): Avoid failure if all probed devices are unsupported. Reported by Ken Congyang. ---
ACK. Matthias

On 01/18/2011 02:00 PM, Matthias Bolte wrote:
2011/1/18 Eric Blake <eblake@redhat.com>:
Fixes regression introduced in commit 2211518, where all qemu 0.12.x fails to start, as does qemu 0.13.x lacking the pci-assign device.
* src/qemu/qemu_capabilities.c (qemuCapsExtractVersionInfo): Check for -device driver,? support. (qemuCapsExtractDeviceStr): Avoid failure if all probed devices are unsupported. Reported by Ken Congyang. ---
ACK.
Thanks; pushed. I expanded the commit message a bit for clarity: Fixes regression introduced in commit 2211518, where all qemu 0.12.x fails to start, as does qemu 0.13.x lacking the pci-assign device. Prior to 2211518, the code was just ignoring a non-zero exit status from the qemu child, but the virCommand code checked this to avoid masking any other issues, which means the real bug of provoking non-zero exit status has been latent for a longer time. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

At 01/19/2011 12:51 AM, Eric Blake Write:
Fixes regression introduced in commit 2211518, where all qemu 0.12.x fails to start, as does qemu 0.13.x lacking the pci-assign device.
* src/qemu/qemu_capabilities.c (qemuCapsExtractVersionInfo): Check for -device driver,? support. (qemuCapsExtractDeviceStr): Avoid failure if all probed devices are unsupported. Reported by Ken Congyang. s/Ken/Wen/
This patch seems fine to me.
---
src/qemu/qemu_capabilities.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1f0a3c2..faf7d44 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1048,10 +1048,14 @@ qemuCapsExtractDeviceStr(const char *qemu,
/* Cram together all device-related queries into one invocation; * the output format makes it possible to distinguish what we - * need. Unrecognized '-device bogus,?' cause an error in - * isolation, but are silently ignored in combination with - * '-device ?'. */ + * need. With qemu 0.13.0 and later, unrecognized '-device + * bogus,?' cause an error in isolation, but are silently ignored + * in combination with '-device ?'. Qemu 0.12.x doesn't + * understand '-device name,?', and always exits with status 1 for + * the simpler '-device ?', so this function is really only useful + * for parsing out features added in 0.13.0 or later. */ cmd = virCommandNewArgList(qemu, + "-device", "?", "-device", "pci-assign,?", "-device", "virtio-blk-pci,?", NULL); @@ -1119,7 +1123,10 @@ int qemuCapsExtractVersionInfo(const char *qemu, &version, &is_kvm, &kvm_version) == -1) goto cleanup;
+ /* Only call qemuCapsExtractDeviceStr for qemu 0.13.0+, since it + * won't set any additional flags for qemu 0.12.x. */ if ((flags & QEMUD_CMD_FLAG_DEVICE) && + strstr(help, "-device driver,?") && qemuCapsExtractDeviceStr(qemu, &flags) < 0) goto cleanup;

At 01/18/2011 11:16 PM, Eric Blake Write:
On 01/17/2011 10:39 PM, Wen Congyang wrote:
+ cmd = virCommandNewArgList(qemu, + "-device", "pci-assign,?", + NULL);
The qemu that I used does not support '-device pci-assign,?'... So, I can not start the guest with this patch.
The qemu is cloned from here: http://git.qemu.org/git/qemu.git
What do the following commands produce (assuming that both qemu and qemu-kvm on your PATH are the binaries that you are testing with)?
After builing qemu, I only have the qemy-system-x86_64 command # qemu-system-x86_64 --version QEMU emulator version 0.13.50, Copyright (c) 2003-2008 Fabrice Bellard # qemu-system-x86_64 -device ? -device pci-assign,? -device virtio-blk-pci,? name "virtio-9p-pci", bus PCI name "virtio-balloon-pci", bus PCI name "virtio-serial-pci", bus PCI, alias "virtio-serial" name "virtio-net-pci", bus PCI name "virtio-blk-pci", bus PCI, alias "virtio-blk" name "ivshmem", bus PCI name "smbus-eeprom", bus I2C name "scsi-disk", bus SCSI, desc "virtual scsi disk or cdrom" name "scsi-generic", bus SCSI, desc "pass through generic scsi device (/dev/sg*)" name "usb-hub", bus USB name "usb-host", bus USB name "usb-kbd", bus USB name "usb-mouse", bus USB name "usb-tablet", bus USB name "usb-storage", bus USB name "usb-wacom-tablet", bus USB, desc "QEMU PenPartner Tablet" name "usb-braille", bus USB name "usb-serial", bus USB name "usb-net", bus USB name "usb-bt-dongle", bus USB name "virtserialport", bus virtio-serial-bus name "virtconsole", bus virtio-serial-bus name "ioh3420", bus PCI, desc "Intel IOH device id 3420 PCIE Root Port" name "x3130-upstream", bus PCI, desc "TI X3130 Upstream Port of PCI Express Switch" name "xio3130-downstream", bus PCI, desc "TI X3130 Downstream Port of PCI Express Switch" name "isa-serial", bus ISA name "isa-parallel", bus ISA name "vt82c686b-usb-uhci", bus PCI name "piix4-usb-uhci", bus PCI name "piix3-usb-uhci", bus PCI name "sysbus-ohci", bus System, desc "OHCI USB Controller" name "pci-ohci", bus PCI, desc "Apple USB Controller" name "SUNW,fdtwo", bus System name "sysbus-fdc", bus System name "PIIX4_PM", bus PCI, desc "PM" name "i6300esb", bus PCI name "ne2k_pci", bus PCI name "i82801", bus PCI, desc "Intel i82801 Ethernet" name "i82562", bus PCI, desc "Intel i82562 Ethernet" name "i82559er", bus PCI, desc "Intel i82559ER Ethernet" name "i82559c", bus PCI, desc "Intel i82559C Ethernet" name "i82559b", bus PCI, desc "Intel i82559B Ethernet" name "i82559a", bus PCI, desc "Intel i82559A Ethernet" name "i82558b", bus PCI, desc "Intel i82558B Ethernet" name "i82558a", bus PCI, desc "Intel i82558A Ethernet" name "i82557c", bus PCI, desc "Intel i82557C Ethernet" name "i82557b", bus PCI, desc "Intel i82557B Ethernet" name "i82557a", bus PCI, desc "Intel i82557A Ethernet" name "i82551", bus PCI, desc "Intel i82551 Ethernet" name "i82550", bus PCI, desc "Intel i82550 Ethernet" name "pcnet", bus PCI name "e1000", bus PCI, desc "Intel Gigabit Ethernet" name "rtl8139", bus PCI name "ne2k_isa", bus ISA name "ide-drive", bus IDE name "isa-ide", bus ISA name "ahci", bus PCI name "lsi53c895a", bus PCI, alias "lsi" name "VGA", bus PCI name "vmware-svga", bus PCI name "sb16", bus ISA, desc "Creative Sound Blaster 16" name "ES1370", bus PCI, desc "ENSONIQ AudioPCI ES1370" name "AC97", bus PCI, desc "Intel 82801AA AC97 Audio" name "intel-hda", bus PCI, desc "Intel HD Audio Controller" name "hda-duplex", bus HDA, desc "HDA Audio Codec, duplex" name "hda-output", bus HDA, desc "HDA Audio Codec, output-only" name "cirrus-vga", bus PCI, desc "Cirrus CLGD 54xx VGA" name "isa-applesmc", bus ISA name "ib700", bus ISA name "isa-debugcon", bus ISA virtio-blk-pci.class=hex32 virtio-blk-pci.drive=drive virtio-blk-pci.logical_block_size=uint16 virtio-blk-pci.physical_block_size=uint16 virtio-blk-pci.min_io_size=uint16 virtio-blk-pci.opt_io_size=uint32 virtio-blk-pci.bootindex=int32 virtio-blk-pci.discard_granularity=uint32 virtio-blk-pci.vectors=uint32 virtio-blk-pci.indirect_desc=on/off virtio-blk-pci.scsi=on/off
qemu -version qemu-kvm -version qemu -device '?' echo $? qemu -device 'pci-assign,?' echo $? qemu -device '?' -device 'pci-assign,?' echo $? qemu-kvm -device '?' echo $? qemu-kvm -device 'pci-assign,?' echo $? qemu-kvm -device '?' -device 'pci-assign,?' echo $?
Depending on those results, I will probably be able to come up with a followup patch that helps you run again (it may just be a matter of ignoring non-zero exit status, or of adding the extra "-device","?", arguments in order to ensure that qemu doesn't error out on unknown devices).
For example, in my case, I know that with the qemu 0.13.0 package in Fedora 14, 'qemu -device pci-assign,?' fails but 'qemu-kvm -device pci-assign,?' succeeds; and for both binaries, '$binary -device ? -device pci-assign,?' succeeds but has more output for qemu-kvm than for qemu.

On 01/18/2011 06:22 PM, Wen Congyang wrote:
At 01/18/2011 11:16 PM, Eric Blake Write:
On 01/17/2011 10:39 PM, Wen Congyang wrote:
+ cmd = virCommandNewArgList(qemu, + "-device", "pci-assign,?", + NULL);
The qemu that I used does not support '-device pci-assign,?'... So, I can not start the guest with this patch.
The qemu is cloned from here: http://git.qemu.org/git/qemu.git
What do the following commands produce (assuming that both qemu and qemu-kvm on your PATH are the binaries that you are testing with)?
After builing qemu, I only have the qemy-system-x86_64 command
That explains it. git.qemu.org/git/qemu.git is the upstream qemu repository, and lacks kvm integration. As such, it lacks -device pci-assign. To build qemu-kvm, you also need to track git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git (which is heavily based on the former qemu.git). Without kvm support, qemu will still be able to emulate x86_64 platforms, but it's noticeably slower.
# qemu-system-x86_64 --version QEMU emulator version 0.13.50, Copyright (c) 2003-2008 Fabrice Bellard
Good - my reproduction of your issue (and my patch) was correct.
# qemu-system-x86_64 -device ? -device pci-assign,? -device virtio-blk-pci,? name "virtio-9p-pci", bus PCI ... name "isa-debugcon", bus ISA virtio-blk-pci.class=hex32 virtio-blk-pci.drive=drive virtio-blk-pci.logical_block_size=uint16 virtio-blk-pci.physical_block_size=uint16 virtio-blk-pci.min_io_size=uint16 virtio-blk-pci.opt_io_size=uint32 virtio-blk-pci.bootindex=int32 virtio-blk-pci.discard_granularity=uint32 virtio-blk-pci.vectors=uint32 virtio-blk-pci.indirect_desc=on/off virtio-blk-pci.scsi=on/off
And this, as desired, is a successful feature probe even though it lacks pci-assign.* information. Sorry for getting your name wrong in my commit message. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

At 01/19/2011 09:54 AM, Eric Blake Write:
On 01/18/2011 06:22 PM, Wen Congyang wrote:
At 01/18/2011 11:16 PM, Eric Blake Write:
On 01/17/2011 10:39 PM, Wen Congyang wrote:
+ cmd = virCommandNewArgList(qemu, + "-device", "pci-assign,?", + NULL);
The qemu that I used does not support '-device pci-assign,?'... So, I can not start the guest with this patch.
The qemu is cloned from here: http://git.qemu.org/git/qemu.git
What do the following commands produce (assuming that both qemu and qemu-kvm on your PATH are the binaries that you are testing with)?
After builing qemu, I only have the qemy-system-x86_64 command
That explains it. git.qemu.org/git/qemu.git is the upstream qemu repository, and lacks kvm integration. As such, it lacks -device pci-assign.
To build qemu-kvm, you also need to track git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git (which is heavily based on the former qemu.git). Without kvm support, qemu will still be able to emulate x86_64 platforms, but it's noticeably slower.
The qemu in git.qemu.org/git/qemu.git has supported kvm.
# qemu-system-x86_64 --version QEMU emulator version 0.13.50, Copyright (c) 2003-2008 Fabrice Bellard
Good - my reproduction of your issue (and my patch) was correct.
# qemu-system-x86_64 -device ? -device pci-assign,? -device virtio-blk-pci,? name "virtio-9p-pci", bus PCI ... name "isa-debugcon", bus ISA virtio-blk-pci.class=hex32 virtio-blk-pci.drive=drive virtio-blk-pci.logical_block_size=uint16 virtio-blk-pci.physical_block_size=uint16 virtio-blk-pci.min_io_size=uint16 virtio-blk-pci.opt_io_size=uint32 virtio-blk-pci.bootindex=int32 virtio-blk-pci.discard_granularity=uint32 virtio-blk-pci.vectors=uint32 virtio-blk-pci.indirect_desc=on/off virtio-blk-pci.scsi=on/off
And this, as desired, is a successful feature probe even though it lacks pci-assign.* information.
Sorry for getting your name wrong in my commit message.
It does not matter.

于 2011年01月14日 03:10, Eric Blake 写道:
* src/qemu/qemu_capabilities.h (qemuCapsParseDeviceStr): New prototype. * src/qemu/qemu_capabilities.c (qemuCapsParsePCIDeviceStrs) Rename and split... (qemuCapsExtractDeviceStr, qemuCapsParseDeviceStr): ...to make it easier to add and test device-specific checks. (qemuCapsExtractVersionInfo): Update caller. * tests/qemuhelptest.c (testHelpStrParsing): Also test parsing of device-related flags. (mymain): Update expected flags. * tests/qemuhelpdata/qemu-0.12.1-device: New file. * tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device: New file. * tests/qemuhelpdata/qemu-kvm-0.12.3-device: New file. * tests/qemuhelpdata/qemu-kvm-0.13.0-device: New file. --- src/qemu/qemu_capabilities.c | 41 ++++++++--- src/qemu/qemu_capabilities.h | 2 + tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device | 57 ++++++++++++++++ tests/qemuhelpdata/qemu-kvm-0.13.0-device | 70 ++++++++++++++++++++ tests/qemuhelptest.c | 48 ++++++++++---- 5 files changed, 195 insertions(+), 23 deletions(-) create mode 100644 tests/qemuhelpdata/qemu-0.12.1-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.3-device create mode 100644 tests/qemuhelpdata/qemu-kvm-0.13.0-device
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9bab317..f967255 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1032,28 +1032,46 @@ fail: return -1; }
-static void -qemuCapsParsePCIDeviceStrs(const char *qemu, - unsigned long long *flags) +static int +qemuCapsExtractDeviceStr(const char *qemu, + unsigned long long *flags) { - char *pciassign = NULL; + char *output = NULL; virCommandPtr cmd; + int ret = -1;
- cmd = virCommandNewArgList(qemu, "-device", "pci-assign,?", NULL); + /* Cram together all device-related queries into one invocation; + * the output format makes it possible to distinguish what we + * need. Unrecognized '-device bogus,?' cause an error in + * isolation, but are silently ignored in combination with + * '-device ?'. */ + cmd = virCommandNewArgList(qemu, + "-device", "pci-assign,?", + NULL); virCommandAddEnvPassCommon(cmd); /* qemu -help goes to stdout, but qemu -device ? goes to stderr. */ - virCommandSetErrorBuffer(cmd,&pciassign); + virCommandSetErrorBuffer(cmd,&output); virCommandClearCaps(cmd);
if (virCommandRun(cmd, NULL)< 0) goto cleanup;
- if (strstr(pciassign, "pci-assign.configfd")) - *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD; + ret = qemuCapsParseDeviceStr(output, flags);
cleanup: - VIR_FREE(pciassign); + VIR_FREE(output); virCommandFree(cmd); + return ret; +} + + +int +qemuCapsParseDeviceStr(const char *str, unsigned long long *flags) +{ + if (strstr(str, "pci-assign.configfd")) + *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD; + + return 0; }
int qemuCapsExtractVersionInfo(const char *qemu, @@ -1092,8 +1110,9 @@ int qemuCapsExtractVersionInfo(const char *qemu, &version,&is_kvm,&kvm_version) == -1) goto cleanup;
- if (flags& QEMUD_CMD_FLAG_DEVICE) - qemuCapsParsePCIDeviceStrs(qemu,&flags); + if ((flags& QEMUD_CMD_FLAG_DEVICE)&& + qemuCapsExtractDeviceStr(qemu,&flags)< 0) + goto cleanup;
This causes problem? older qemu which doesn't support "-device pci-assign,?" or "-device virtio-blk-pci,?" won't work anymore. (raised by nikunj in #virt). Regards Osier

On 01/19/2011 06:12 AM, Osier Yang wrote:
This causes problem? older qemu which doesn't support "-device pci-assign,?" or "-device virtio-blk-pci,?" won't work anymore. (raised by nikunj in #virt).
Those problems have already been fixed by commit 93681a3683 -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Qemu smartcard support exists on branches (such as http://cgit.freedesktop.org/~alon/qemu/commit/?h=usb_ccid.v15&id=024a37b) but is not yet upstream. Once an upstream version exists, then we can add new -help and -device ? output files to tests/qemuhelptest to prove that the new flag works. * src/qemu/qemu_capabilities.h (QEMUD_CMD_FLAG_USB_CCID): New flag. * src/qemu/qemu_capabilities.c (qemuCapsExtractDeviceStr) (qemuCapsParseDeviceStr): Check for smartcard device support. --- src/qemu/qemu_capabilities.c | 6 ++++++ src/qemu/qemu_capabilities.h | 3 ++- 2 files changed, 8 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f967255..c131124 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1046,6 +1046,7 @@ qemuCapsExtractDeviceStr(const char *qemu, * isolation, but are silently ignored in combination with * '-device ?'. */ cmd = virCommandNewArgList(qemu, + "-device", "?", "-device", "pci-assign,?", NULL); virCommandAddEnvPassCommon(cmd); @@ -1068,6 +1069,11 @@ cleanup: int qemuCapsParseDeviceStr(const char *str, unsigned long long *flags) { + /* Which devices exist. */ + if (strstr(str, "name \"usb-ccid\"")) + *flags |= QEMUD_CMD_FLAG_USB_CCID; + + /* Features of given devices. */ if (strstr(str, "pci-assign.configfd")) *flags |= QEMUD_CMD_FLAG_PCI_CONFIGFD; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 8057479..484c112 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -1,7 +1,7 @@ /* * qemu_capabilities.h: QEMU capabilities generation * - * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -83,6 +83,7 @@ enum qemuCapsFlags { QEMUD_CMD_FLAG_SPICE = (1LL << 46), /* Is -spice avail */ QEMUD_CMD_FLAG_VGA_NONE = (1LL << 47), /* The 'none' arg for '-vga' */ QEMUD_CMD_FLAG_MIGRATE_QEMU_FD = (1LL << 48), /* -incoming fd:n */ + QEMUD_CMD_FLAG_USB_CCID = (1LL << 49), /* -device usb-ccid */ }; virCapsPtr qemuCapsInit(virCapsPtr old_caps); -- 1.7.3.4
participants (6)
-
Eric Blake
-
Jiri Denemark
-
Laine Stump
-
Matthias Bolte
-
Osier Yang
-
Wen Congyang