From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mm8oX-0002pw-Hg for qemu-devel@nongnu.org; Fri, 11 Sep 2009 12:17:33 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mm8oS-0002jD-Gc for qemu-devel@nongnu.org; Fri, 11 Sep 2009 12:17:32 -0400 Received: from [199.232.76.173] (port=52944 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mm8oS-0002iH-AX for qemu-devel@nongnu.org; Fri, 11 Sep 2009 12:17:28 -0400 Received: from mail-yw0-f203.google.com ([209.85.211.203]:62384) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mm8oR-0000jN-Me for qemu-devel@nongnu.org; Fri, 11 Sep 2009 12:17:27 -0400 Received: by ywh41 with SMTP id 41so1666029ywh.19 for ; Fri, 11 Sep 2009 09:17:26 -0700 (PDT) Message-ID: <4AAA7813.1030904@codemonkey.ws> Date: Fri, 11 Sep 2009 11:17:23 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1252684353-25067-1-git-send-email-amit.shah@redhat.com> <1252684353-25067-2-git-send-email-amit.shah@redhat.com> <1252684353-25067-3-git-send-email-amit.shah@redhat.com> In-Reply-To: <1252684353-25067-3-git-send-email-amit.shah@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 2/5] virtio-console: Add support for multiple ports for generic guest-host communication List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: qemu-devel@nongnu.org Amit Shah wrote: > This interface extends the virtio-console device to handle > multiple ports that present 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. > > I thought the consensus was that we wanted strings to identify ports such that a scheme like reverse fqdn could be used? > diff --git a/hw/pc.c b/hw/pc.c > index d96d756..d5d2542 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -1422,11 +1422,17 @@ static void pc_init1(ram_addr_t ram_size, > } > > /* Add virtio console devices */ > - if (pci_enabled) { > - for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) { > - if (virtcon_hds[i]) { > - pci_create_simple(pci_bus, -1, "virtio-console-pci"); > - } > + if (pci_enabled && virtcon_nr_ports) { > + void *dev; > + > + dev = pci_create_simple(pci_bus, -1, "virtio-console-pci"); > + if (!dev) { > + fprintf(stderr, "qemu: could not create virtio console pci device\n"); > + exit(1); > + } > + > + for (i = 0; i < virtcon_nr_ports; i++) { > + virtio_console_new_port(dev, virtcon_idx[i]); > } > } > } > If a user does: qemu -M pc-0.11.0 -virtiocon vc -virtiocon vc This patch will break that guest. I think the best solution to this is to properly integrate with qdev and the new chardev infrastructure. virtio-console-pci-0.11 should have the old semantics, virtio-console-pci-0.12 should be a bus. > #include "hw.h" > +#include "monitor.h" > +#include "pci.h" > We don't want to add PCI dependency to virtio console. It isn't always used on platforms with PCI. > +static VirtIOConsole *virtio_console; > +static struct virtio_console_config virtcon_config; > +static VirtIOConsolePort virtcon_ports[MAX_VIRTIO_CONSOLE_PORTS] > Introducing statics like this indicates something isn't right. Again, I think a proper qdev bus conversion would take care of that. > +/* This function gets called from vl.c during initial options > + * parsing as well as from the monitor to parse the options. > + * So it's a good idea to not print out anything and just > + * return values which can become meaningful. > + */ > +int init_virtio_console_port(int port, const char *opts) > +{ > + char dev[256]; > + const char *prot; > + const char *idx; > + uint32_t port_nr; > + int j, k; > + > + memset(dev, 0, sizeof(dev)); > + prot = strstr(opts, ",protocol="); > + idx = strstr(opts, ",port="); > Need to integrate with QemuOpts. > + > + register_savevm("virtio-console", -1, 2, virtio_console_save, virtio_console_load, s); > Should integrate with VMState. Regards, Anthony Liguori