5:48 p.m.
On 05/22/2012 11:50 PM, Beat.Joerg(a)ssatr.ch wrote:
Hi
As requested by Dave, I send this to the list.
Thank you.
I came across a bug that the command line generated for passtrough of the host parallel
port /dev/parport0 by libvirt for QEMU is incorrect.
s/passtrough/passthrough/
It currently produces:
-chardev tty,id=charparallel0,path=/dev/parport0
-device isa-parallel,chardev=charparallel0,id=parallel0
The first parameter is "tty". It sould be "parport".
If I launch qemu with -chardev parport,... it works as expected.
I have already filled a bug report ( https://bugzilla.redhat.com/show_bug.cgi?id=823879
), the topic was already on the list some months ago:
https://www.redhat.com/archives/libvirt-users/2011-September/msg00095.html
Not sure if this is fix is very clean, but for my case it works.
Perhaps it should also be checked if similar problems / patches are required for other
chardev backends.
Kind regards
Beat
diff -Naur libvirt-0.9.4.orig/src/qemu/qemu_command.c
libvirt-0.9.4/src/qemu/qemu_command.c
--- libvirt-0.9.4.orig/src/qemu/qemu_command.c 2012-05-22 13:40:16.788633656 +0200
+++ libvirt-0.9.4/src/qemu/qemu_command.c 2012-05-22 13:52:18.453608557 +0200
@@ -2357,8 +2357,14 @@
Unfortunately, your patch came through with severe whitespace mangling.
Thankfully, it was small enough, so I reconstructed it by hand.
break;
case VIR_DOMAIN_CHR_TYPE_DEV:
- virBufferAsprintf(&buf, "tty,id=char%s,path=%s", alias,
- dev->data.file.path);
+ if (STRPREFIX(dev->data.file.path, "/dev/parport")) {
Path-based parsing is wrong. But we _do_ know whether the user is
creating a <parallel> or <serial> device based on the alias name, so we
should key off of that instead.
+ virBufferAsprintf(&buf, "parport,id=char%s,path=%s", alias,
+ dev->data.file.path);
+ }
+ else {
Style - libvirt prefers '} else {' on one line.
+ virBufferAsprintf(&buf, "tty,id=char%s,path=%s", alias,
+ dev->data.file.path);
Rather than duplicating the entire virBufferAsprintf, you can use an
extra %s and the ?: operator to condense to one line.
It would also be nice to enhance the testsuite to cover this.
With that, I think this patch should have the same effect, but a bit
cleaner. I'll go ahead and push it in your name; I updated AUTHORS in
the process, so let me know if I need to adjust the spelling at all.
src/qemu/qemu_command.c | 5 ++--
.../qemuxml2argv-parallel-parport-chardev.args | 7 +++++
.../qemuxml2argv-parallel-parport-chardev.xml | 29
++++++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
5 files changed, 42 insertions(+), 2 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-parallel-parport-chardev.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-parallel-parport-chardev.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9ca7641..fb8d9a3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3365,8 +3365,9 @@ qemuBuildChrChardevStr(virDomainChrSourceDefPtr
dev, const char *alias,
break;
case VIR_DOMAIN_CHR_TYPE_DEV:
- virBufferAsprintf(&buf, "tty,id=char%s,path=%s", alias,
- dev->data.file.path);
+ virBufferAsprintf(&buf, "%s,id=char%s,path=%s",
+ STRPREFIX(alias, "parallel") ? "parport" :
"tty",
+ alias, dev->data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-parallel-parport-chardev.args
b/tests/qemuxml2argvdata/qemuxml2argv-parallel-parport-chardev.args
new file mode 100644
index 0000000..48f968a
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-parallel-parport-chardev.args
@@ -0,0 +1,7 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu
-S -M \
+pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,\
+id=charmonitor,path=/tmp/test-monitor,server,nowait -mon
chardev=charmonitor,\
+id=monitor,mode=readline -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 \
+-chardev parport,id=charparallel0,path=/dev/parport0 -device \
+isa-parallel,chardev=charparallel0,id=parallel0 -usb -device \
+virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-parallel-parport-chardev.xml
b/tests/qemuxml2argvdata/qemuxml2argv-parallel-parport-chardev.xml
new file mode 100644
index 0000000..b495cdc
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-parallel-parport-chardev.xml
@@ -0,0 +1,29 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='ide' index='0'/>
+ <parallel type='dev'>
+ <source path='/dev/parport0'/>
+ <target port='0'/>
+ </parallel>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 617b178..7b00ea2 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -604,6 +604,8 @@ mymain(void)
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
DO_TEST("parallel-tcp-chardev", false,
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
+ DO_TEST("parallel-parport-chardev", false,
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
DO_TEST("console-compat-chardev", false,
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
--
1.7.10.2
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org