[libvirt] [PATCH v2 0/3] QEMU: Specific order for controllers in cmd line
The order is basically random for now, with one constraint: CCID must be preceded with USB. diff to v1: -split into 2 patches -add a test case Michal Privoznik (3): qemu: Wrap controllers code into dummy loop qemu: Add controllers in specified order tests: Add test for controller order src/qemu/qemu_command.c | 101 +++++++++++--------- .../qemuxml2argv-controller-order.args | 28 ++++++ .../qemuxml2argv-controller-order.xml | 89 +++++++++++++++++ tests/qemuxml2argvtest.c | 6 + 4 files changed, 177 insertions(+), 47 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-order.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml -- 1.7.8.6
which just re-indent code and prepare it for next patch. --- src/qemu/qemu_command.c | 95 ++++++++++++++++++++++++----------------------- 1 files changed, 48 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 389c480..566565a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4429,7 +4429,7 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainSnapshotObjPtr snapshot, enum virNetDevVPortProfileOp vmop) { - int i; + int i, j; struct utsname ut; int disableKQEMU = 0; int enableKQEMU = 0; @@ -5024,61 +5024,63 @@ qemuBuildCommandLine(virConnectPtr conn, } if (qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - for (i = 0 ; i < def->ncontrollers ; i++) { - virDomainControllerDefPtr cont = def->controllers[i]; + for (j = 0; j < 1; j++) { + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; - /* We don't add an explicit IDE or FD controller because the - * provided PIIX4 device already includes one. It isn't possible to - * remove the PIIX4. */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE || - cont->type == VIR_DOMAIN_CONTROLLER_TYPE_FDC) - continue; + /* We don't add an explicit IDE or FD controller because the + * provided PIIX4 device already includes one. It isn't possible to + * remove the PIIX4. */ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE || + cont->type == VIR_DOMAIN_CONTROLLER_TYPE_FDC) + continue; - /* Also, skip USB controllers with type none.*/ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && - cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) { - usbcontroller = -1; /* mark we don't want a controller */ - continue; - } + /* Also, skip USB controllers with type none.*/ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) { + usbcontroller = -1; /* mark we don't want a controller */ + continue; + } - /* Only recent QEMU implements a SATA (AHCI) controller */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) { - if (!qemuCapsGet(caps, QEMU_CAPS_ICH9_AHCI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("SATA is not supported with this " - "QEMU binary")); - goto error; + /* Only recent QEMU implements a SATA (AHCI) controller */ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) { + if (!qemuCapsGet(caps, QEMU_CAPS_ICH9_AHCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("SATA is not supported with this " + "QEMU binary")); + goto error; + } else { + char *devstr; + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildControllerDevStr(def, cont, + caps, NULL))) + goto error; + + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && + cont->model == -1 && + !qemuCapsGet(caps, QEMU_CAPS_PIIX3_USB_UHCI)) { + if (usblegacy) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Multiple legacy USB controllers are " + "not supported")); + goto error; + } + usblegacy = true; } else { + virCommandAddArg(cmd, "-device"); + char *devstr; - - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildControllerDevStr(def, cont, - caps, NULL))) + if (!(devstr = qemuBuildControllerDevStr(def, cont, caps, + &usbcontroller))) goto error; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); } - } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && - cont->model == -1 && - !qemuCapsGet(caps, QEMU_CAPS_PIIX3_USB_UHCI)) { - if (usblegacy) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Multiple legacy USB controllers are " - "not supported")); - goto error; - } - usblegacy = true; - } else { - virCommandAddArg(cmd, "-device"); - - char *devstr; - if (!(devstr = qemuBuildControllerDevStr(def, cont, caps, - &usbcontroller))) - goto error; - - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); } } } @@ -5553,7 +5555,6 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainSmartcardDefPtr smartcard = def->smartcards[0]; char *devstr; virBuffer opt = VIR_BUFFER_INITIALIZER; - int j; const char *database; if (def->nsmartcards > 1 || -- 1.7.8.6
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. --- src/qemu/qemu_command.c | 18 ++++++++++++------ 1 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 566565a..1e96982 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4446,6 +4446,16 @@ qemuBuildCommandLine(virConnectPtr conn, int usbcontroller = 0; bool usblegacy = false; uname_normalize(&ut); + int contOrder[] = { + /* We don't add an explicit IDE or FD controller because the + * provided PIIX4 device already includes one. It isn't possible to + * remove the PIIX4. */ + VIR_DOMAIN_CONTROLLER_TYPE_USB, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI, + VIR_DOMAIN_CONTROLLER_TYPE_SATA, + VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, + VIR_DOMAIN_CONTROLLER_TYPE_CCID, + }; VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " "caps=%p migrateFrom=%s migrateFD=%d " @@ -5024,15 +5034,11 @@ qemuBuildCommandLine(virConnectPtr conn, } if (qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - for (j = 0; j < 1; j++) { + for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) { for (i = 0; i < def->ncontrollers; i++) { virDomainControllerDefPtr cont = def->controllers[i]; - /* We don't add an explicit IDE or FD controller because the - * provided PIIX4 device already includes one. It isn't possible to - * remove the PIIX4. */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE || - cont->type == VIR_DOMAIN_CONTROLLER_TYPE_FDC) + if (cont->type != contOrder[j]) continue; /* Also, skip USB controllers with type none.*/ -- 1.7.8.6
--- .../qemuxml2argv-controller-order.args | 28 ++++++ .../qemuxml2argv-controller-order.xml | 89 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 ++ 3 files changed, 123 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-order.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args new file mode 100644 index 0000000..ec70c87 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.args @@ -0,0 +1,28 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +QEMU_AUDIO_DRV=spice /usr/libexec/qemu-kvm -S -M rhel6.1.0 -enable-kvm -m \ +4096 -smp 4 -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -boot order=cna,menu=off \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device \ +virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x7 -device \ +usb-ccid,id=ccid0 -drive \ +file=/tmp/fdr.img,if=none,id=drive-virtio-disk0,cache=off,aio=native \ +-device \ +virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0 \ +-drive \ +file=/tmp/Fedora-17-x86_64-Live-Desktop.iso,if=none,media=cdrom,id=drive-ide0-1-0 \ +-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -device \ +virtio-net-pci,vlan=0,id=net0,mac=52:54:00:4d:4b:19,bus=pci.0,addr=0x3 -net \ +user,vlan=0,name=hostnet0 -chardev \ +spicevmc,id=charsmartcard0,name=smartcard -device \ +ccid-card-passthru,chardev=charsmartcard0,id=smartcard0,bus=ccid0.0 \ +-chardev pty,id=charserial0 -device \ +isa-serial,chardev=charserial0,id=serial0 -chardev \ +spicevmc,id=charchannel0,name=vdagent -device \ +virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 \ +-device usb-tablet,id=input0 -spice \ +port=0,addr=0.0.0.0,x509-dir=/etc/pki/libvirt-spice -device \ +intel-hda,id=sound0,bus=pci.0,addr=0x4 -device \ +hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device \ +usb-host,hostbus=14,hostaddr=6,id=hostdev0 -device \ +virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml new file mode 100644 index 0000000..6a98eaa --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml @@ -0,0 +1,89 @@ +<domain type='kvm'> + <name>fdr</name> + <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='x86_64' machine='rhel6.1.0'>hvm</type> + <boot dev='hd'/> + <boot dev='network'/> + <boot dev='fd'/> + <bootmenu enable='no'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw' cache='none' io='native'/> + <source file='/tmp/fdr.img'/> + <target dev='vda' bus='virtio'/> + <shareable/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/tmp/Fedora-17-x86_64-Live-Desktop.iso'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </controller> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ccid' index='0'/> + <interface type='user'> + <mac address='52:54:00:4d:4b:19'/> + <model type='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <smartcard mode='passthrough' type='spicevmc'> + <address type='ccid' controller='0' slot='0'/> + </smartcard> + <serial type='pty'> + <target port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + <address type='virtio-serial' controller='0' bus='0' port='1'/> + </channel> + <input type='tablet' bus='usb'/> + <input type='mouse' bus='ps2'/> + <graphics type='spice' autoport='yes' listen='0.0.0.0'> + <listen type='address' address='0.0.0.0'/> + </graphics> + <sound model='ich6'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </sound> + <video> + <model type='cirrus' vram='9216' heads='1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <hostdev mode='subsystem' type='usb' managed='yes'> + <source> + <address bus='14' device='6'/> + </source> + </hostdev> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </memballoon> + </devices> + <seclabel type='dynamic' model='selinux' relabel='yes'/> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 20b0b35..48e09ab 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -408,6 +408,12 @@ mymain(void) DO_TEST("cpu-eoi-disabled", QEMU_CAPS_ENABLE_KVM); DO_TEST("cpu-eoi-enabled", QEMU_CAPS_ENABLE_KVM); + DO_TEST("controller-order", QEMU_CAPS_DRIVE, QEMU_CAPS_PCIDEVICE, + QEMU_CAPS_KVM, QEMU_CAPS_DEVICE, QEMU_CAPS_ENABLE_KVM, + QEMU_CAPS_BOOT_MENU, QEMU_CAPS_PIIX3_USB_UHCI, + QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_DRIVE_AIO, + QEMU_CAPS_CCID_PASSTHRU, QEMU_CAPS_CHARDEV, + QEMU_CAPS_CHARDEV_SPICEVMC, QEMU_CAPS_SPICE, QEMU_CAPS_HDA_DUPLEX); DO_TEST("eoi-disabled", NONE); DO_TEST("eoi-enabled", NONE); DO_TEST("kvmclock+eoi-disabled", QEMU_CAPS_ENABLE_KVM); -- 1.7.8.6
On 11/05/2012 06:37 AM, Michal Privoznik wrote:
The order is basically random for now, with one constraint: CCID must be preceded with USB.
diff to v1: -split into 2 patches -add a test case
Thanks. ACK series.
Michal Privoznik (3): qemu: Wrap controllers code into dummy loop qemu: Add controllers in specified order tests: Add test for controller order
src/qemu/qemu_command.c | 101 +++++++++++--------- .../qemuxml2argv-controller-order.args | 28 ++++++ .../qemuxml2argv-controller-order.xml | 89 +++++++++++++++++ tests/qemuxml2argvtest.c | 6 + 4 files changed, 177 insertions(+), 47 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-order.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05.11.2012 17:25, Eric Blake wrote:
On 11/05/2012 06:37 AM, Michal Privoznik wrote:
The order is basically random for now, with one constraint: CCID must be preceded with USB.
diff to v1: -split into 2 patches -add a test case
Thanks. ACK series.
Pushed. Thanks. Michal
participants (2)
-
Eric Blake -
Michal Privoznik