Re: [libvirt] chardev aliases [was: [RFC PATCHv2 5/5] WIP: smartcard: turn on qemu support]

On 01/14/2011 06:46 AM, Daniel P. Berrange wrote: >> The 'id' setting for the chardev should really have a prefix >> on it, because we won't want it to be the same as the 'id' >> setting used for the -device option. > > This is in fact a pre-existing flaw in our chardev handling > which we need to fix. The 'qemuBuildChrChardevStr' API & its > callers are buggy. They takes the assigned alias from the XML > and use it directly for the chardev. So we end up with > > -chardev pty,id=$alias > -device isa-serial,chardev=$alias > > The serial/parallel/channels -device args are all missing > any 'id' property entirely which is bad, because it means > we won't be able to support hotplug. What we should be > generating is > > -chardev pty,id=cdev$alias > -device isa-serial,id=$alias,chardev=cdev$alias > > So that both parts have a unique identifier assigned, > and the alias from the XML is associated with the > '-device' arg, and the -chardev is a derivative. Done with the attached patch. git send-email won't let me send it directly, since it has a line longer than SMTP limits for inline mail: fatal: /tmp/canS3cO9Y0/0001-qemu-use-separate-alias-for-chardev-and-associated-d.patch: 163: patch contains a line longer than 998 characters warning: no patches were sent -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

From d4a3f317dbd6b5b513b505d0eee21f9ec1a2d362 Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake(a)redhat.com&gt; Date: Tue, 25 Jan 2011 14:59:50 -0700 Subject: [PATCH] qemu: use separate alias for chardev and associated device * src/qemu/qemu_command.c (qemuBuildChrChardevStr): Alter the chardev alias. (qemuBuildCommandLine): Output an id for the chardev counterpart. * tests/qemuxml2argvdata/*: Update tests to match. Reported by Daniel P. Berrange. --- src/qemu/qemu_command.c | 36 +++++++++++--------- .../qemuxml2argv-channel-guestfwd.args | 2 +- .../qemuxml2argv-channel-virtio-auto.args | 2 +- .../qemuxml2argv-channel-virtio.args | 2 +- .../qemuxml2argv-console-compat-chardev.args | 2 +- .../qemuxml2argv-console-virtio.args | 2 +- .../qemuxml2argv-parallel-tcp-chardev.args | 2 +- .../qemuxml2argv-serial-dev-chardev.args | 2 +- .../qemuxml2argv-serial-file-chardev.args | 2 +- .../qemuxml2argv-serial-many-chardev.args | 2 +- .../qemuxml2argv-serial-pty-chardev.args | 2 +- .../qemuxml2argv-serial-tcp-chardev.args | 2 +- .../qemuxml2argv-serial-tcp-telnet-chardev.args | 2 +- .../qemuxml2argv-serial-udp-chardev.args | 2 +- .../qemuxml2argv-serial-unix-chardev.args | 2 +- .../qemuxml2argv-serial-vc-chardev.args | 2 +- 16 files changed, 35 insertions(+), 31 deletions(-)