From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=49706 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pq43R-0007Q8-Ck for qemu-devel@nongnu.org; Thu, 17 Feb 2011 08:37:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pq43Q-0003Io-93 for qemu-devel@nongnu.org; Thu, 17 Feb 2011 08:37:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:2272) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pq43Q-0003IY-2U for qemu-devel@nongnu.org; Thu, 17 Feb 2011 08:37:56 -0500 Date: Thu, 17 Feb 2011 19:07:45 +0530 From: Amit Shah Subject: Re: [Qemu-devel] [PATCH REBASE/RESEND 1/4] qdev: Add a description field for qdev properties for documentation Message-ID: <20110217133745.GA28478@amit-x200.redhat.com> References: <4D5AAD3E.6010201@codemonkey.ws> <20110217130657.GD27487@amit-x200.redhat.com> <4D5D1ECA.1060901@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4D5D1ECA.1060901@codemonkey.ws> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu list , Markus Armbruster On (Thu) 17 Feb 2011 [07:12:42], Anthony Liguori wrote: > On 02/17/2011 07:06 AM, Amit Shah wrote: > >On (Tue) 15 Feb 2011 [10:43:42], Anthony Liguori wrote: > > > >>> #define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ > >>>- DEFINE_PROP_DRIVE("drive", _state, _conf.bs), \ > >>>+ DEFINE_PROP_DRIVE("drive", _state, _conf.bs, ""), \ > >>> DEFINE_PROP_UINT16("logical_block_size", _state, \ > >>>- _conf.logical_block_size, 512), \ > >>>+ _conf.logical_block_size, 512, ""), \ > >>> DEFINE_PROP_UINT16("physical_block_size", _state, \ > >>>- _conf.physical_block_size, 512), \ > >>>- DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0), \ > >>>- DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ > >>>- DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \ > >>>+ _conf.physical_block_size, 512, ""), \ > >>>+ DEFINE_PROP_UINT16("min_io_size", _state, _conf.min_io_size, 0, ""), \ > >>>+ DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0, ""), \ > >>>+ DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1, ""), \ > >>> DEFINE_PROP_UINT32("discard_granularity", _state, \ > >>>- _conf.discard_granularity, 0) > >>>+ _conf.discard_granularity, 0, "") > >>This is pretty horribly ugly. If we were going this, we should at > >>least introduce new defines that include a documentation field and > >>not just add empty documentation fields. > >We've discussed this in the past. Once this patch series gets in, > >I'll work to fill in the documentation here along with the > >maintainers. > > It means you're touching everything twice instead of touching > everything once. That's unnecessary churn and blame breakage. Well no one can claim to be good enough to document each and every parameter for each device that qemu has. The infrastructure has to be laid out, then the documentation can be filled in by the subsystem maintainers. I'm willing to co-ordinate that activity, as discussed earlier. Amit