From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:43129)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1QzX0G-0003Zn-Bc
	for qemu-devel@nongnu.org; Fri, 02 Sep 2011 12:54:05 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1QzX0F-0002Vw-FD
	for qemu-devel@nongnu.org; Fri, 02 Sep 2011 12:54:04 -0400
Received: from mx1.redhat.com ([209.132.183.28]:16275)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1QzX0F-0002Vj-8C
	for qemu-devel@nongnu.org; Fri, 02 Sep 2011 12:54:03 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1312376904-16115-1-git-send-email-armbru@redhat.com>
	<1312376904-16115-42-git-send-email-armbru@redhat.com>
	<4E60D692.40105@redhat.com> <m3obz3ezbr.fsf@blackfin.pond.sub.org>
	<4E610377.7050708@redhat.com>
Date: Fri, 02 Sep 2011 18:53:57 +0200
In-Reply-To: <4E610377.7050708@redhat.com> (Kevin Wolf's message of "Fri, 02
	Sep 2011 18:25:27 +0200")
Message-ID: <m3d3fic2ca.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: [Qemu-devel] [PATCH v2 41/45] block: New
	bdrv_set_buffer_alignment()
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin Wolf <kwolf@redhat.com>
Cc: quintela@redhat.com, stefano.stabellini@eu.citrix.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, hare@suse.de, amit.shah@redhat.com, hch@lst.de

Kevin Wolf <kwolf@redhat.com> writes:

> Am 02.09.2011 17:30, schrieb Markus Armbruster:
>> Kevin Wolf <kwolf@redhat.com> writes:
>> 
>>> Am 03.08.2011 15:08, schrieb Markus Armbruster:
>>>> Device models should be able to set it without an unclean include of
>>>> block_int.h.
>>>>
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>> ---
>>>>  block.c         |    6 ++++--
>>>>  block.h         |    1 +
>>>>  hw/ide/core.c   |    2 +-
>>>>  hw/scsi-disk.c  |    2 +-
>>>>  hw/virtio-blk.c |    3 +--
>>>>  5 files changed, 8 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/block.c b/block.c
>>>> index fed0c16..67d9429 100644
>>>> --- a/block.c
>>>> +++ b/block.c
>>>> @@ -453,7 +453,6 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename,
>>>>      bs->encrypted = 0;
>>>>      bs->valid_key = 0;
>>>>      bs->open_flags = flags;
>>>> -    /* buffer_alignment defaulted to 512, drivers can change this value */
>>>>      bs->buffer_alignment = 512;
>>>
>>> This comment is still right.
>> 
>> It's also next to useless.  I hate comments paraphrasing the code :)
>> 
>> I'll keep it if you insist.
>
> The information that the comment gives isn't that we set it to 512, but
> that this is only a default and "drivers" (should be "devices", I think)
> are expected to change it.

Devices can and do change any number of default values set up here, and
documenting that just for buffer_alignment is useless at best,
misleading at worst.

Nevertheless, I'll keep it if you insist.