On 11/01/2012 08:11 AM, Michal Privoznik wrote:
qemu is sensitive to the order of arguments passed. Hence, if a device requires a controller, the controller cmd string must precede device cmd string. The same apply for controllers, when for instance ccid controller requires usb controller. So controllers create partial ordering in which they should be added to qemu cmd line. ---
The order is basically random for now, with one constraint: CCID must be preceded with USB.
src/qemu/qemu_command.c | 102 +++++++++++++++++++++++++--------------------- 1 files changed, 55 insertions(+), 47 deletions(-)
Shouldn't we have at least one test under tests/qemuxml2argvdata that is impacted by this new ordering? And if not, we should add such a test. The patch is a bit hard to read, because it reindents at the same time as adding only a couple lines of code. I find it easier to split these types of patches into two - one that adds the new code but doesn't reindent, and a followup that fixes indentation; or the reverse order of adding a {} group to reindent with no semantic change, followed by the patch that adds the actual feature. That said, the code looks right, but I'd still like to see a testcase proving it before giving ack. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org