From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NAMCu-0001x2-MW for qemu-devel@nongnu.org; Tue, 17 Nov 2009 06:26:48 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NAMCp-0001uq-HQ for qemu-devel@nongnu.org; Tue, 17 Nov 2009 06:26:48 -0500 Received: from [199.232.76.173] (port=58810 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NAMCp-0001um-Cq for qemu-devel@nongnu.org; Tue, 17 Nov 2009 06:26:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:18060) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NAMCo-0001QF-T1 for qemu-devel@nongnu.org; Tue, 17 Nov 2009 06:26:43 -0500 Date: Tue, 17 Nov 2009 16:55:54 +0530 From: Amit Shah Subject: Re: [Qemu-devel] virtio-rng Message-ID: <20091117112554.GC11493@amit-x200.redhat.com> References: <4B011F38.9070500@redhat.com> <4B014584.6000001@collabora.co.uk> <4B014F2D.3040205@redhat.com> <4B0192BE.4010105@collabora.co.uk> <20091117092053.GA10556@amit-x200.redhat.com> <4B02705A.5060400@collabora.co.uk> <20091117095456.GA11125@amit-x200.redhat.com> <4B0278B0.1080505@collabora.co.uk> <20091117102837.GA11493@amit-x200.redhat.com> <4B0284A8.5050800@collabora.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B0284A8.5050800@collabora.co.uk> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ian Molton Cc: qemu-devel@nongnu.org On (Tue) Nov 17 2009 [11:10:32], Ian Molton wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Amit Shah wrote: > > (Any reason to take this off-list?) > > None other than hitting reply rather than reply-all. CCing list once > more :-) > > >> Either way, you still need to specify the properties - they've just > >> moved into the console driver in your patch by the look of it. > > > > Yeah; there are two ways of setting properties -- from the command line > > or from code. > > > > What you're doing is something like > > > > -virtiorng,a=foo,b=bar > > Ah, I see why we're at cross purposes here - I thought we'd moved to > discussing virtio-console. I've already moved to qdev based init for > virtio-rng - 'fixing' virtio-console was where I learnt about the 'new > way' (qdev). I actually meant to talk about virtio-console but had the virtio-rng example ready in mind. I thought you did the same thing for console too? Sorry I've not really looked at the patch in detail so you can disregard that comment. > >> I'd prefer to do the same for virtio-rng, > > Here I refer to you having (and I didnt apply the patch so I may have > misread) dropped the virtio-pci proxy from virtio-console. Once I've got > a tree I can apply your patch to I'll take another look :-) It slightly works differently now: virtio is supposed to be pci-agnostic. So what now happens is: virtio-console hooks on to the virtio-serial-bus via -device virtconsole virtio-serial-bus hooks on to the virtio-pci-proxy via -device virtio-serial-pci > >> Yes, I saw that. Would it not be better to generate the device / chardev > >> pair dynamically though, rather than preserve the icky old array? > > > > I don't understand what you mean by 'dynamically'. > > Rather than have that array virtcon_hds at all, create the pairs of host > side data and qdev properties during parsing the commandline - no need > to store them up and iterate through the later, or to set an arbitrary > limit on how many can be specified in that way. qdev won't understand the old-style commandline syntax; so once -virtioconsole is encounters, all parsing of the arguments for that param are to be done by the code that handles -virtioconsole. The array is maintained because multiple virtio-consoles could be spawned, upto a max. of MAX_VIRTIO_CONSOLES. But, as it stands, MAX_VIRTIO_CONSOLES is 1 and so the array can be dropped, but a local var. would still be needed to fetch the chardev and then init it properly using qemu_opt. > Might be an idea for qemu to warn people that this syntax will be > deprecated, too... Yes, that should be done (maybe for 1-2 releases). Amit