From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NN9J2-0002mI-DU for qemu-devel@nongnu.org; Tue, 22 Dec 2009 13:18:00 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NN9Ix-0002kH-QG for qemu-devel@nongnu.org; Tue, 22 Dec 2009 13:17:59 -0500 Received: from [199.232.76.173] (port=33800 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NN9Ix-0002kD-Gs for qemu-devel@nongnu.org; Tue, 22 Dec 2009 13:17:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58396) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NN9Iu-0005Bv-Ao for qemu-devel@nongnu.org; Tue, 22 Dec 2009 13:17:55 -0500 Date: Tue, 22 Dec 2009 23:46:41 +0530 From: Amit Shah Message-ID: <20091222181641.GA29396@amit-x200.redhat.com> References: <1261504146-26018-1-git-send-email-amit.shah@redhat.com> <1261504146-26018-2-git-send-email-amit.shah@redhat.com> <1261504146-26018-3-git-send-email-amit.shah@redhat.com> <4B310B28.9050503@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B310B28.9050503@suse.de> Subject: [Qemu-devel] Re: [PATCH 2/3] virtio-console: Add a virtio-serial bus, support for multiple devices and ports List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: armbru@redhat.com, qemu-devel@nongnu.org, kraxel@redhat.com On (Tue) Dec 22 2009 [19:08:40], Alexander Graf wrote: > > > > - /* 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"); > > - } > > - } > > - } > > - > > > > We have something pretty similar in s390-virtio.c. I suppose that needs > to be changed too? Yes, it'll have to be changed as well then; done in my tree. > > -static VirtIOS390DeviceInfo s390_virtio_console = { > > - .init = s390_virtio_console_init, > > - .qdev.name = "virtio-console-s390", > > +static VirtIOS390DeviceInfo s390_virtio_serial = { > > + .init = s390_virtio_serial_init, > > + .qdev.name = "virtio-serial-s390", > > > > Are you sure you changed all users of the old name too? There's only virtio-serial-pci and virtio-serial-s390. Is there something I missed? The new changes in vl.c, on the other hand, I've not yet fully studied so there might be something missing there. > > --- a/hw/virtio-console.c > > +++ b/hw/virtio-console.c > > @@ -1,143 +1,121 @@ > > /* > > - * Virtio Console Device > > + * Virtio Console and Generic Port Devices > > * > > - * Copyright IBM, Corp. 2008 > > + * Copyright Red Hat, Inc. 2009 > > * > > * Authors: > > - * Christian Ehrhardt > > + * Amit Shah > > > > Please don't remove copyrights. It seems that way due to file name changes. This is actually a new file that has nothing in common with the old one. Whatever is left of the old content is now in virtio-serial-bus.c. > > @@ -4823,6 +4826,13 @@ static int virtcon_parse(const char *devname) > > fprintf(stderr, "qemu: too many virtio consoles\n"); > > exit(1); > > } > > + > > + opts = qemu_opts_create(&qemu_device_opts, NULL, 0); > > + qemu_opt_set(opts, "driver", "virtio-serial-pci"); > > > > As you stated in your comment, this breaks. Maybe something as simple as > > #ifdef TARGET_S390X > qemu_opt_set(opts, "driver", "virtio-serial-pci"); > #else > qemu_opt_set(opts, "driver", "virtio-serial-s390"); > #endif > > is enough here? But it's ugly; Markus said we could do something better but I didn't fully understand it. On the lines of creating default devices or setting virtio-console's preferred bus type. Thanks for looking at this, Amit