Re: [libvirt] [PATCH] qemu: Add controllers in specified order
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 617 bytes)