login
Search
Hello all, This are the latest version of the patches. Lots of things have changed since the last submission. A few of which I remember: - VNC copy / paste works* (* conditions apply) - client vnc copies get propagated to guest port 3 (/dev/vmch3) - guest writes to port 3 (/dev/vmch3) go straight to client's clipboard - sysfs hooks to autodiscover ports - support for 64 ports in this version (MAX_VIRTIO_SERIAL_PORTS). More ports can be added by introducing a new feature flag to maintain backward compat. However, till this code gets accepted upstream, the value of that #define can change. I think 64 ports are enough for everyone. - remove support for control queue (though this queue could make a comeback for just one use-case that I can currently think of: to prevent rogue (host) userspace putting lots of data into a guest that goes unconsumed for a while, increasing the memory pressure. To prevent this a threshold level can be decided upon and a control message can be sent to host userspace to prevent any more writes to the port. - numerous fixes There still exist a few kmalloc/kfree-related debug logs that spew up in the guest but I haven't been able to track them down. As for the merge with virtio-console, Christian has voiced some oppostion to that idea. For the merge to happen, the kernel folks have to agree in merging the driver as well and I can proceed once we have a resolution on this. Other than that, a few more rebases have to be done to the qemu code to make it apply to qemu-upstream. Please give this a good review. Thanks, Amit. --
This helper is introduced to query the status of vnc.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
vnc.c | 10 +++++++++-
vnc.h | 2 +-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/vnc.c b/vnc.c
index de0ff87..e4e78dc 100644
--- a/vnc.c
+++ b/vnc.c
@@ -176,9 +176,17 @@ static void do_info_vnc_client(Monitor *mon, VncState *client)
#endif
}
-void do_info_vnc(Monitor *mon)
+int is_vnc_active(void)
{
if (vnc_display == NULL || vnc_display->display == NULL) {
+ return 0;
+ }
+ return 1;
+}
+
+void do_info_vnc(Monitor *mon)
+{
+ if (!is_vnc_active()) {
monitor_printf(mon, "Server: disabled\n");
} else {
char *serverAddr = vnc_socket_local_addr(" address: %s:%s\n",
diff --git a/vnc.h b/vnc.h
index 3ae95f3..9739c35 100644
--- a/vnc.h
+++ b/vnc.h
@@ -313,7 +313,7 @@ void buffer_append(Buffer *buffer, const void *data, size_t len);
/* Misc helpers */
+int is_vnc_active(void);
char *vnc_socket_local_addr(const char *format, int fd);
char *vnc_socket_remote_addr(const char *format, int fd);
-
#endif /* __QEMU_VNC_H */
--
1.6.2.5
--
- Send ServerCutText message over to the vnc client from qemu
on every write to the clipboard port via virtio-serial
- On receiving ClientCutText message send over the data to the
guest via virtio-serial.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
hw/virtio-serial.c | 15 +++++++++++++++
hw/virtio-serial.h | 4 ++++
vnc.c | 24 ++++++++++++++++++++++++
vnc.h | 2 ++
4 files changed, 45 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-serial.c b/hw/virtio-serial.c
index d77af9b..95c715f 100644
--- a/hw/virtio-serial.c
+++ b/hw/virtio-serial.c
@@ -20,6 +20,7 @@
#include "pci.h"
#include "monitor.h"
#include "qemu-char.h"
+#include "vnc.h"
#include "virtio.h"
#include "virtio-serial.h"
@@ -118,6 +119,10 @@ static size_t flush_buf(uint32_t id, const uint8_t *buf, size_t len)
VirtIOSerialPort *port;
size_t write_len = 0;
+ if (id == VIRTIO_SERIAL_CLIPBOARD_PORT && is_vnc_active()) {
+ vnc_clipboard_data_from_guest(buf, len);
+ return len;
+ }
port = get_port_from_id(id);
if (port && port->hd) {
write_len = qemu_chr_write(port->hd, buf, len);
@@ -211,6 +216,16 @@ static void write_to_port(VirtIOSerialPort *port,
virtio_notify(port->virtserial->vdev, vq);
}
+void virtio_serial_send_clipboard_to_guest(const uint8_t *buf, size_t len)
+{
+ VirtIOSerialPort *port = get_port_from_id(VIRTIO_SERIAL_CLIPBOARD_PORT);
+
+ if (!port) {
+ return;
+ }
+ write_to_port(port, buf, len);
+}
+
/* Readiness of the guest to accept data on a port */
static int cons_can_read(void *opaque)
{
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index 08c4b51..44238f6 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -18,6 +18,9 @@
#define VIRTIO_ID_SERIAL 7
#define VIRTIO_SERIAL_BAD_ID (~(uint32_t)0)
+/* Port number to function mapping */
+#define VIRTIO_SERIAL_CLIPBOARD_PORT 3
+
struct virtio_serial_config
{
...We expose multiple char devices ("ports") for simple communication
between the host userspace and guest.
Sample offline usages can be: poking around in a guest to find out
the file systems used, applications installed, etc. Online usages
can be sharing of clipboard data between the guest and the host,
sending information about logged-in users to the host, locking the
screen or session when a vnc session is closed, and so on.
The interface presented to guest userspace is of a simple char
device, so it can be used like this:
fd = open("/dev/vmch0", O_RDWR);
ret = read(fd, buf, 100);
ret = write(fd, string, strlen(string));
Each port is to be assigned a unique function, for example, the first
4 ports may be reserved for libvirt usage, the next 4 for generic
streaming data and so on. This port-function mapping isn't finalised
yet.
For requirements, use-cases and some history see
http://www.linux-kvm.org/page/VMchannel_Requirements
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/Kconfig | 6 +
drivers/char/Makefile | 1 +
drivers/char/virtio_serial.c | 690 +++++++++++++++++++++++++++++++++++++++++
include/linux/virtio_serial.h | 27 ++
4 files changed, 724 insertions(+), 0 deletions(-)
create mode 100644 drivers/char/virtio_serial.c
create mode 100644 include/linux/virtio_serial.h
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 6a06913..51adb48 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -679,6 +679,12 @@ config VIRTIO_CONSOLE
help
Virtio console for use with lguest and other hypervisors.
+config VIRTIO_SERIAL
+ tristate "Virtio serial"
+ select VIRTIO
+ select VIRTIO_RING
+ help
+ Virtio serial device driver for simple guest and host communication
config HVCS
tristate "IBM Hypervisor Virtual Console Server support"
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index 66f779a..5e1915b 100644
--- a/drivers/char/Makefile
+++ ...This interface presents a char device from which bits can be
sent and read.
Sample uses for such a device can be obtaining info from the
guest like the file systems used, apps installed, etc. for
offline usage and logged-in users, clipboard copy-paste, etc.
for online usage.
Each port is to be assigned a unique function, for example, the first
4 ports may be reserved for libvirt usage, the next 4 for generic
streaming data and so on. This port-function mapping isn't finalised
yet.
For requirements, use-cases and some history see
http://www.linux-kvm.org/page/VMchannel_Requirements
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
Makefile.target | 2 +-
hw/pc.c | 17 +++
hw/pci.h | 1 +
hw/qdev.c | 6 +-
hw/virtio-pci.c | 17 +++
hw/virtio-serial.c | 367 ++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/virtio-serial.h | 36 +++++
hw/virtio.h | 1 +
monitor.c | 7 +
qemu-monitor.hx | 10 ++
qemu-options.hx | 8 +
sysemu.h | 13 ++
vl.c | 90 +++++++++++++
13 files changed, 573 insertions(+), 2 deletions(-)
create mode 100644 hw/virtio-serial.c
create mode 100644 hw/virtio-serial.h
diff --git a/Makefile.target b/Makefile.target
index df1f32b..21e8ec3 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -538,7 +538,7 @@ obj-y = vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o \
gdbstub.o gdbstub-xml.o msix.o ioport.o
# virtio has to be here due to weird dependency between PCI and virtio-net.
# need to fix this properly
-obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o
+obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o virtio-serial.o
obj-$(CONFIG_KVM) += kvm.o kvm-all.o
LIBS+=-lz
diff --git a/hw/pc.c b/hw/pc.c
index ecc7046..6badad9 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -40,6 +40,8 @@
#include "qemu-kvm.h"
+void *virtio_serial_new_port(PCIDevice *dev, ...Hi Amit, depends on VIRTIO I don't think this code works. What protects from two simultaneous readers? Who resets the completion? It's more normal to use a waitqueue and This can return -EINTR; if you hadn't assigned ret = -EINTR unconditionally I thought -ENOTTY was normal for invalid ioctls? In which case, just don't This won't quite work. get_buf gets *used* buffers. You need to track ?? Cheers, Rusty. --
Hm, yes. Remnant from older code which used an array for storing port structures. So the id was found by iterating and returning the index where it was found. While converting the array to a linked list I just Yes, I'm going to switch this now that I have to use waitqueues for If there were more buffers queued up, we could easily go fetch data from them and return as much data as was requested. (BTW I've already Control would come here only if there was some data to be read. We check write(2) doesn't mention a return type of ENOMEM so I avoided it. But there's also a note saying other types of errors are possible. So I'll The ioctl was introduced for getting the 'name' of the port earlier. With that gone, there's no use for this currently. No harm removing. It is. I added it recently just because some other drivers were doing This is also done during probe and port hot-add case. And I guess if some host process is sending lots of data, we could defer the allocation to another workqueue and just allocate the empty space in one deferred Oh ok. Is there no way to get them from the vq? How about a new helper This is some unused IOCTL. Anyway, it should be gone. Thanks a lot for your review, Rusty! Amit --
Thanks for the review, Rusty. The result is:
drivers/char/Kconfig | 5 -
drivers/char/virtio_serial.c | 160
++++++++++++++++++------------------------
include/linux/virtio_serial.h | 3
3 files changed, 73 insertions(+), 95 deletions(-)
Major changes since the last version are:
- add support for poll()
- return as much data as possible for read()
- fix memleaks
- remove ioctl code
- other cleanups suggested by Rusty
Thanks,
Amit
From 04fa836c1d965ba57d6d662830e431e4c12637f5 Mon Sep 17 00:00:00 2001
From: Amit Shah <amit.shah@redhat.com>
Date: Wed, 5 Aug 2009 15:22:01 +0530
Subject: [PATCH] virtio_serial: A char device for simple guest <-> host communication
We expose multiple char devices ("ports") for simple communication
between the host userspace and guest.
Sample offline usages can be: poking around in a guest to find out
the file systems used, applications installed, etc. Online usages
can be sharing of clipboard data between the guest and the host,
sending information about logged-in users to the host, locking the
screen or session when a vnc session is closed, and so on.
The interface presented to guest userspace is of a simple char
device, so it can be used like this:
fd = open("/dev/vmch0", O_RDWR);
ret = read(fd, buf, 100);
ret = write(fd, string, strlen(string));
Each port is to be assigned a unique function, for example, the first
4 ports may be reserved for libvirt usage, the next 4 for generic
streaming data and so on. This port-function mapping isn't finalised
yet.
For requirements, use-cases and some history see
http://www.linux-kvm.org/page/VMchannel_Requirements
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/Kconfig | 7 +
drivers/char/Makefile | 1 +
drivers/char/virtio_serial.c | 688 +++++++++++++++++++++++++++++++++++++++++
include/linux/virtio_serial.h | 24 ++
4 files changed, 720 insertions(+), 0 deletions(-)
create mode 100644 ...Why 3? Where's the guest application that drives the copy/paste? I expect the first problem you'll run into is that copy/paste daemon has to run as an unprivileged user but /dev/vmch3 is going to be owned by root. You could set udev rules for /dev/vmch3 but that's pretty terrible IMHO. I think you'll find that you need a root daemon that talks to vmchannel and then allows unprivileged connections over a unix socket device. In that case, why even bother having multiple channels since your daemon can multiplex.. Regards, Anthony Liguori --
I don't think that's not too bad, for example, with fast-user-switching between multiple X servers and/or text consoles, there's already support code that deals with chown'ing things like /dev/snd/* devices to match the active console session. Doing the same with the /dev/vmch3 device so that it is only ever accessible to the current logged in user actually fits in to that scheme quite well. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| --
Yeah, I'm not sure how something like this would interact with f-u-s. If copy/paste daemon for user foo opens /dev/vmch3 directly, when you switch users, how do you forcefully disconnect user foo from /dev/vmch3 so that user bad can start using it? Regards, --
With multiple X servers, there can be more than one currently logged in user. Same with multiple text consoles - that's more familiar. Which one owns /dev/vmch3? -- Jamie --
For a VMM, copy/paste should work with whatever user has the active X session that's controlling the physical display. Yes, it could get complicated if we supported multiple video cards, but fortunately we don't :-) I really think you need to have a copy/paste daemon that allows multiple X sessions to connect to it and then that daemon can somehow determine who is the "active" session. This is part of the reason I've been pushing for a concrete example. All the signs here point to a privileged daemon that delegates to multiple users. I think just about any use-case will have a similar model. It really suggests that you need _one_ vmchannel that's exposed to userspace with a single userspace daemon that consumes it. You want the flexibility of a userspace daemon in determining how you multiplex and do security. I don't think it's something you want to bake into the userspace/kernel interface. And if you have a single daemon that serves vmchannel sessions, that daemon can make it transparent whether the session is going over /dev/ttyS0, a network device, /dev/hvc1, etc. Regards, Anthony Liguori --
Right; use virtio just as the transport and all the interesting activity happens in userspaces. That was the basis with which I started. I can imagine dbus doing the copy/paste, lock screen, etc. actions. However for libguestfs, dbus isn't an option and they already have some predefined agents for each port. So libguestfs is an example for a or /dev/vmch0. it doesn't matter. All minimal virtio devices will look the same. Pop buffers, populate them, push them, etc. Amit --
Or don't use dbus and use something that libguestfs is able to embed. The fact that libguestfs doesn't want dbus in the guest is not an argument for using a higher level kernel interface especially one that doesn't meet the requirements of the interface. Regards, Anthony Liguori --
But why do we want to limit the device to only one port? It's not too complex supporting additional ones. As I see it qemu and the kernel should provide the basic abstraction for the userspace to go do its job. Why create unnecessary barriers? Amit --
I agree. If userspace wants it may use only one channel and demultiplex messages by itself, but we shouldn't force it to. Also one of the requirements for virtio-serial is to have connect disconnect notifications. It is not possible with demultiplexing in the userspace. -- Gleb. --
I agree too, for all those reasons. However it would be useful if the devices provided a simpler way to be found by guest applications than /dev/vmch0, vmch1, vmch2... On Linux udev provides a sane way to find devices according to roles, subtypes, serial numbers, whatever you want, if the appropriate id codes are available from the devices and put into /sys/* by the kernel driver. That would make the devices much more useful to independent applications, imho. -- Jamie --
... or a more flexible API. I don't like having fixed /dev/vmch* devices either. A long time ago (on a mailing list not so far away) there was a much better userspace API proposed, which had a separate AF_VMCHANNEL address family. That API works much more like TCP sockets, except without requiring network devices: https://lists.linux-foundation.org/pipermail/virtualization/2008-December/012383.html Note: even better if it allows multiple channels with the same name to be created on demand from the guest, which the API would allow, although not the implementation above. That would allow the fast-user-switching / multi-X-server case to work well, and be useful if we parallelize libguestfs. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top --
Dave Miller nacked that approach with a sledgehammer instead preferring that we just use standard TCP/IP which is what led to the current implementation using slirp. A userspace daemon with unix domain sockets could give a similar solution. Regards, Anthony Liguori --
I'm aware of that - I just don't think it was a good choice. [BTW the qemu-devel mailing list seems to be bouncing messages] Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://et.redhat.com/~rjones/libguestfs/ See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html --
I know. I've reported it to the Savannah admins and am helping them Regards, Anthony Liguori --
Have you considered using a usb serial device? Something attractive about it is that a productid/vendorid can be specified which means that you can use that as a method of enumerating devices. Hot add/remove is supported automagically. Regards, Anthony Liguori --
The same applies to PCI: productid/vendorid (and subids);
PCI hotplug is possible though not as native as USB.
Here's another idea: Many devices these days have a serial number or
id string. E.g. USB storage, ATA drives, media cards, etc. Linux
these days creates alias device nodes which include the id string in
the device name. E.g. /dev/disks/by-id/ata-FUJITSU_MHV2100BH_NWAQT662615H
So in addition to (or instead of) /dev/vmch0, /dev/vmch1 etc.,
Linux guests could easily generate:
/dev/vmchannel/by-role/clipboard-0
/dev/vmchannel/by-role/gueststats-0
/dev/vmchannel/by-role/vmmanager-0
It's not necessary to do this at the beginning. All that is needed is
to provide enough id information that will appear in /sys/..., so that
that a udev policy for naming devices can be created at some later date.
-- Jamie
--
What's nice about USB is that HID specifies quite a few functional generic devices that can be extended to increase functionality. This means you can implement a more sophisticated usb device that satisfies the serial interface, provide a special more featureful driver for Linux, and just use normal serial for Windows. Well my thinking is that the "clipboard" device actually becomes a USB serial device. It's easy to enumerate and detect via the existing Linux infrastructure. Plus usb drivers can be implemented in userspace which is a nice plus (cross platform too via libusb). Regards, --
And the virtio code is pretty simple and self-contained. I don't see why Sure; but there's been no resistance from anyone from including the virtio-serial device driver so maybe we don't need to discuss that. Amit --
There certainly is from me. The userspace interface is not reasonable for guest applications to use. Regards, --
One example that would readily come to mind is dbus. A daemon running on the guest that reads data off the port and interacts with the desktop by appropriate dbus commands. All that's needed is a stream of bytes and virtio-serial provides just that. Any more complexity could easily be handled in userspace. Amit --
dbus runs as an unprivileged user, how does dbus know which virtio-serial port to open and who sets the permissions on that port? Regards, Anthony Liguori --
The permission part can be handled by package maintainers and sysadmins via udev policies. So all data destined for dbus consumption gets to a daemon and that daemon then sends it over to dbus. Amit --
virtio-serial is nice, easy, simple and versatile. We like that; it
should stay that way.
dbus isn't a good match for this.
dbus is not intended for communication between hosts, by design.
It depends on per-app configuration files in
/etc/dbus/{session,system}.d/, which are expected to match the
installed services.
For this, the guest's files in /etc/dbus/ would have to match the QEMU
host host services in detail. dbus doesn't have a good mechanism for
copying with version skew between both of them, because normally
everything resides on the same machine and the config and service are
updated at the same time. This is hard to guarantee with a VM.
Apart from dbus, hard-coded meanings of small N in /dev/vmchN are
asking for trouble. It is bound to break when widely deployed and
guest/host configs don't match. It also doesn't fit comfortably when
you have, say, bob and alice both logged in with desktops on separate
VTs. Clashes are inevitable, as third-party apps pick N values for
themselves then get distributed - unless N values can be large
(/dev/vmch44324 == kernelstats...).
Sysadmins shouldn't have to hand-configure each app, and shouldn't
have to repair clashes in defaults. Just Work is better.
virtio-serial is nice. The only ugly part is _finding_ the right
/dev/vmchN.
Fortunately, _any_ out-of-band id string or id number makes it perfect.
An option to specify PCI vendor/product ids in the QEMU host
configuration is good enough.
An option to specify one or more id strings is nicer.
Finally, Anthony hit on an interesting idea with USB. Emulating USB
sucks. But USB's _descriptors_ are quite effective, and the USB basic
protocol is quite reasonable too.
Descriptors are just a binary blob in a particular format, which
describe a device and also say what it supports, and what standard
interfaces can be used with it too. Bluetooth is similar; they might
even use the same byte format, I'm not sure.
All the code for parsing USB descriptors is ...Oh; I don't mean to say dbus on the host will communicate directly with dbus on the guest via virtio-serial. I'm just saying there'll be some daemon on the guest and if there's a request for, say, updating guest It's no different from the way major-minor numbering works on the Linux kernel: they uniquely identify a device. Or the way tcp port number works. In the same way, /dev/vmchN will uniquely identify some function over a port. You idea of introducing a symlink to /dev/clipboard is a Hm, so there can be one daemon on the guest handling all clipboard events. There's some work done already by the fast-user-switch support I wonder how that's any different or less complex that virtio-serial. Essentially the idea is the same. It's just the name that's different, Hm, making a very simple transport into something complicated involving a few more subsystems just to expose a usb device which functions as a char device in the end. I don't see a big benefit. People are deploying virtio anyway for block and net. I don't see why using the same transport for a char device would be an impediment. Amit --
I think the naming is very important. The guest needs to know who is listening on the other end of the line. I think a sysfs attribute (as suggested by Jamie IIRC) will work nicely here. So each device gets a property called 'class' or 'protocol' or something simliar named, therein a string which specifies the protocol it speaks. Host side would look like this: -virtioserial port=4,protocol=clipboard ... in case the protocol is implemented in qemu or like this: -virtioserial port=2,protocol=libguestfs,char=unix:something ... for stuff provided by external apps. Within the guest the two lines above would create vmch4 and vmch2, both having a protocol attribute, and udev then can create symlinks named by protocol, i.e. /dev/vmchannel/clipboard symlinked to /dev/vmch4 and /dev/vmchannel/libguestfs symlinked to /dev/vmch2 The port=<nr> attribute can be optional and dynamically auto-allocated reverse fqdn name space is a good idea. We don't need a central protocol name registry then. The examples above would then become something like this: protocol=orq.qemu.clipboard and protocol=org.libguestfs.fish ... and within the guest /dev/vmchannel/org/qemu/clipboard and I think virtio-serial is the better way to handle vmchannel. Unlike usb virtio is designed to work nicely in a virtual environment. But vmchannel-over-usbserial should be easy too though in case some guests lacks virtio backports or something. I think you can just stick a name like "vmchannel:orq.qemu.clipboard" into the usbserial product name, then have udev match that and create /dev/vmchannel/org/qemu/clipboard symlinking to /dev/ttyUSB<nr> Voila, you can switch transports and the apps don't even notice. cheers, Gerd --
I think you're missing my fundamental point. Don't use the kernel as the guest interface. Introduce a userspace daemon that exposes a domain socket. Then we can have a proper protocol that uses reverse fqdns for identification. We can do the backend over TCP/IP, usb, standard serial, etc. Regards, Anthony Liguori --
We need nothing but (a) bidirectional byte streams and (b) name tags for them. Do we really want design a daemon and a protocol for such a simple thing? Especially as requiring a daemon for that adds a few problems you don't have without them. Access control for example: For device nodes you can just use standard unix permissions and acls. You can easily do stuff like adding the logged in desktop user to the /dev/vmchannel/org/qemu/clipboard acl using existing solutions. With a daemon you have to hop through a number of loops to archive the same. Can't we simply have guest apps open "/dev/vmchannel/$protocol" ? cheers, Gerd --
Yes, because we also need (c) the ability to write cross platform software that targets vmchannel. So having a library interface is going to be extremely desirable. Also, see the previous discussion about security. How do you sanely /dev interfaces are only simple to kernel developers :-) Besides, why do something that can be clearly done in userspace within the kernel? It just increases the possibility of kernel bugs. You can have a /var/run/vmchannel/$protocol.sock unix domain socket and it has all the same properties that you describe. It also Just Works with standard tools like socat. You can transparently route it over the network, have it work over slirp, a serial device, or some custom virtio device if we so choose. It's the only sane way to support older guests too. If we really want vmchannel to be used by application developers, then we really need a libvmchannel. Regards, --
pam_console (I think that is the name of the beast). Or is it handled by hal these days? The piece of software which does the very same thing already for sound See above. There are other devices which need that too. There are Ok, lets rip out the in-kernel ioapic code then. It can (and has been) The daemon increases the possibility of userspace bugs. Seriously: Attaching a name tag to virtio-serial devices and have them exposed via sysfs is probably *much* less code than a vmchannel daemon. Also multiplexing over one device introduces a number of problems you have to take care of on both sides (qemu+daemon) of the connection. For example: When the communication stalls in one protocol the others should keep on going of course. With one device per protocol and thus We need a sane solution developed and merged and not a new idea each week. cheers, Gerd --
The justification is performance although that's not really true anymore post TPR optimization. But FWIW, I opposed both the in-kernel apic and the in-kernel pit when There is nothing sane about vmchannel. It's just an attempt to bypass QEMU which is going to introduce all sorts of complexities wrt migration, guest compatibility, etc. However, as I've mentioned repeatedly, the reason I won't merge virtio-serial is that it duplicates functionality with virtio-console. If the two are converged, I'm happy to merge it. I'm not opposed to having more functionality. I think it's the wrong solution for the use-case, and I always have, but that's independent of my willingness to merge it. Regards, Anthony Liguori --
NB: the userspace interface for these devices should be a tty, not a new character device. If you want to add a new bustype for these devices, and then have an entry in sysfs that had some sort of identification string, that's perfectly acceptable. Also note though that this is exactly what usb-serial is today. /sys/bus/usbserial contains all the usb serial devices and you can get a vendor id/device id to uniquely identify the device type. Using virtio vs. usb has it's advantage but the userspace interface model should be roughly equivalent. Regards, Anthony Liguori --
The guest code sort-of ends up looking like this after merging virtio_console into virtio_serial. Diff is against virtio_serial in my git tree, but that should be pretty close to the last submission I made at http://patchwork.kernel.org/patch/39335/ or the tree at git://git.kernel.org/pub/scm/linux/kernel/git/amit/vs-kernel.git I've merged bits from virtio_console.c into virtio_serial.c. If needed, virtio_serial can be renamed to virtio_console. The VIRITIO_ID also needs to change to that of virtio_console's. Similar changes are needed for userspace. Since there's support for only one console as of now, I've assigned port #2 as the console port so that we hook into hvc when a port is found at that location. One issue that crops up for put_chars: a copy of the buffer to be sent has to be made as we don't wait for host to ack the buffer before we move on. The biggest loss so far is Rusty's excellent comments: they will have to be reworked and added for the whole of the new file. Is this approach acceptable? diff --git a/drivers/char/Makefile b/drivers/char/Makefile index 5e1915b..ab9c914 100644 --- a/drivers/char/Makefile +++ b/drivers/char/Makefile @@ -53,7 +53,6 @@ obj-$(CONFIG_HVC_IRQ) += hvc_irq.o obj-$(CONFIG_HVC_XEN) += hvc_xen.o obj-$(CONFIG_HVC_IUCV) += hvc_iucv.o obj-$(CONFIG_HVC_UDBG) += hvc_udbg.o -obj-$(CONFIG_VIRTIO_CONSOLE) += virtio_console.o obj-$(CONFIG_VIRTIO_SERIAL) += virtio_serial.o obj-$(CONFIG_RAW_DRIVER) += raw.o obj-$(CONFIG_SGI_SNSC) += snsc.o snsc_event.o diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index c74dacf..f82c036 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -43,39 +43,6 @@ static struct virtio_device *vdev; static unsigned int in_len; static char *in, *inbuf; -/* The operations for our console. */ -static struct hv_ops virtio_cons; - -/* The hvc device */ -static struct hvc_struct *hvc; - -/*D:310 The put_chars() callback is ...
I think we want to keep virtio_console.c and definitely continue using the virtio_console id. It looks like you are still creating character devices as opposed to tty devices. Is this just an incremental step or are you choosing to not do tty devices for a reason (as if so, what's that reason)? Regards, Anthony Liguori --
Just an incremental step. In fact, I haven't yet thought about exposing a tty device and any problems that might come up. I'll get to doing that too, of course. Currently I plan to: - finish sending connect/disconnect notifications - finish merge of virtio-console and serial - look at tty Amit --
I've now seen some more code here and to me it looks like virtioconsole is not used on any of the guests that qemu supports. The virtio_console kernel module only works with lguest and s390 currently. There is one feature and some config values supported by the kernel module but not in qemu. So it looks as if we have virtio-console merged but no one uses it. Is this right? If that's the case, I'll send patches to replace virtio-console with virtio-serial, making sure we keep the command line arg, -virtioconsole <qemu char dev> which will be translated to something like -virtioserial <qemu char dev>,port=2 or -virtioserial <qemu char dev>,protocol=console depending on what we finalise on. Does anyone have objections to this or pointers for me to see where virtioconsole is actually used by qemu-supported guests? I'm also open to convert the guest kernel virtio_console driver over to support the new functionality, or just let that be and have a new virtio-serial module. Rusty, what's your take on this? Amit --
Oh; ok. So the console device is exposed only in the userspace; it's not used for the early boot messages and not registered early-on. That's Thanks, Amit --
which, by the way, doesn't change what I wrote above: since it's not used for earlyboot messages anyway it's not really used as a 'console' in its real sense and so we could just replace it with the newer version which makes it much easier for it to work with the kernel driver. Else supporting older qemu with newer kernel driver will be difficult (if not Amit --
Nope. Grab a Fedora 11 live CD, and boot with
# qemu-kvm -virtioconsole stdio -cdrom Fedora-11-i686-Live.iso -m 500
Once it completes booting & logs into gnome, open a terminal and run
as root
agetty /dev/hvc0 9600 vt100
You'll get a login prompt on the host machine now.
What appears to not be working, is early kernel boot messages. eg, I
ought to be able todo
# qemu-kvm -virtioconsole stdio -kernel vmlinuz -initrd initrd.img \
-append "console=hvc0" -m 500
and see the kernel boot messages, but this doesn't work with Fedora
kernels at least. Not tried upstream, or looked to see if this is just
an oversight in the Kconfig use for Fedora kernels.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
I think it should better go the other way around: add multichannel support to virtio-concole, probably guarded by a feature flag so old Doesn't sound like this is going to be backward compatible ... Also I still think passing a 'protocol' string for each port is a good idea, so you can stick that into a sysfs file for guests use. Note it is probably easy to make virtio-console support multiple hvc lines by having one protocol for that (named 'console' for example). cheers, Gerd --
Or drops ports altogether and just use protocol strings... Regards, Anthony Liguori --
Both is silly, yes. I guess strings + HAL magic can make the /dev names sane. I don't want to see userspace trolling through sysfs to figure out what device to open. Which is why I prefer assigned numbers, which get mapped to minors. Rusty. --
udev can create sane /dev names (or symlinks) by checking sysfs ports map trivially to minors. When using protocol strings minors can simply be dynamically auto-allocated by the guest and we don't need the port numbers in the host<->guest protocol any more. I think strings are better as numbers for identifying protocols as you can work without a central registry for the numbers then. cheers, Gerd --
There still will have to be some way in transferring all the strings from qemu to the guest. Could be done from the config space, but will I like the way assigned numbers work: it's simpler to code, needs a bitmap for all the ports that fits in nicely in the config space and udev rules / scripts can point /dev/vmch02 to /dev/console. Amit --
How would a third party go about assigning themselves a number? For the sake of example, imagine they develop a simple service like "guesttop" which let's the host get a listing of guest processes. They'll have to distributed app-specific udev rule patches for every guest distro, which sounds like a lot of work. The app itself is probably a very simple C program; the hardest part of making it portable across distros would be the udev rules, which is silly. Anyway, every other device has a name or uuid these days. You can still use /dev/sda1 to refer to your boot partition, but LABEL=boot is also available if you prefer. Isn't that the ethos these days? Why not both? /dev/vmch05 if you prefer, plus symlink /dev/vmch-guesttop -> /dev/vmch05 if name=guesttop was given to QEMU. If you do stay with numbers only, note that it's not like TCP/UDP port numbers because the number space is far smaller. Picking a random number that you hope nobody else uses is harder. ... Back to technical bits. If config space is tight, use a channel! Dedicate channel 0 to control, used to fetch the name (if there is one) for each number. -- Jamie --
Yep, all you need is a central registry for names :) Rusty. --
There are schemes to do without (reverse domain names, i.e. au.com.rustcorp.* is all yours and you don't have to register). Also with names the namespace is much bigger and clashes are much less likely, so chances that it works out with everybody simply picking a sane name are much higher. cheers, Gerd --
Yes, because we also need (c) the ability to write cross platform software that targets vmchannel. So having a library interface is going to be extremely desirable. Also, see the previous discussion about security. How do you sanely /dev interfaces are only simple to kernel developers :-) Besides, why do something that can be clearly done in userspace within the kernel? It just increases the possibility of kernel bugs. You can have a /var/run/vmchannel/$protocol.sock unix domain socket and it has all the same properties that you describe. It also Just Works with standard tools like socat. You can transparently route it over the network, have it work over slirp, a serial device, or some custom virtio device if we so choose. It's the only sane way to support older guests too. If we really want vmchannel to be used by application developers, then we really need a libvmchannel. Regards, --
I disagree. If you can hand out names, you can hand out numbers. Whether the guest chooses to put that number in sysfs or make it a minor, I don't care. Rusty. --
The problem with handing out names is that there has to be someone to "hand" things out. And even if you have a good hander-outer, development is difficult in a distributed environment because you may have folks using your code before you've gotten an official hand-out. A better discovery mechanism is based on something that piggy backs on another authority. For instance, reverse fully qualified domains work well. uuid's tend to work pretty well too although it's not perfect. In general, even just open strings can work out okay given that people are responsible in how they name things. Regards, Anthony Liguori --
Wrong. There are often good reasons to do stuff in kernel, even if you Ok, so the virtio-serial + usbserial combo should work well then I think. If you have guest drivers you'll go the virtio-serial route. If you don't have guest drivers you can go the usbserial route, either via /dev/ttyUSB or via libusb. We can also have a libvmchannel as abstraction layer on top of this. cheers, Gerd --
That's interesting; worth a thought. When we actually have all the parties together (libvirt, libguestfs, qemu) to decide which ports need True. Amit --
[ Meant to reply to this two days ago :-( ] We're using -net channel ^W guestfwd in libguestfs now. Apart from the problem with using the "new syntax", which I hope to get around to resolving some day, it performs quite well. The userspace API is somewhat annoying. Hot add/remove isn't a concern for us right now, nor is migration. Since we can throw up new qemu-based appliances in a few seconds. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw --
