From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:59539)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <supriyak@linux.vnet.ibm.com>) id 1REfIv-00061g-OO
	for qemu-devel@nongnu.org; Fri, 14 Oct 2011 06:47:54 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <supriyak@linux.vnet.ibm.com>) id 1REfIu-0003Oc-A3
	for qemu-devel@nongnu.org; Fri, 14 Oct 2011 06:47:53 -0400
Received: from e28smtp05.in.ibm.com ([122.248.162.5]:41421)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <supriyak@linux.vnet.ibm.com>) id 1REfIt-0003Mk-GY
	for qemu-devel@nongnu.org; Fri, 14 Oct 2011 06:47:52 -0400
Received: from d28relay05.in.ibm.com (d28relay05.in.ibm.com [9.184.220.62])
	by e28smtp05.in.ibm.com (8.14.4/8.13.1) with ESMTP id p9EAliDD013994
	for <qemu-devel@nongnu.org>; Fri, 14 Oct 2011 16:17:44 +0530
Received: from d28av04.in.ibm.com (d28av04.in.ibm.com [9.184.220.66])
	by d28relay05.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id
	p9EAlgRU2232426
	for <qemu-devel@nongnu.org>; Fri, 14 Oct 2011 16:17:43 +0530
Received: from d28av04.in.ibm.com (loopback [127.0.0.1])
	by d28av04.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id
	p9EAlgvm021730
	for <qemu-devel@nongnu.org>; Fri, 14 Oct 2011 21:47:42 +1100
Message-ID: <4E98134D.3060200@linux.vnet.ibm.com>
Date: Fri, 14 Oct 2011 16:17:41 +0530
From: Supriya Kannery <supriyak@linux.vnet.ibm.com>
MIME-Version: 1.0
References: <20111011031046.9587.44474.sendpatchset@skannery.in.ibm.com>
	<20111011031059.9587.54786.sendpatchset@skannery.in.ibm.com>
	<4E95A17A.1050105@redhat.com>
In-Reply-To: <4E95A17A.1050105@redhat.com>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [v7 Patch 1/5]Qemu: Enhance "info block" to
 display host cache setting
Reply-To: supriyak@linux.vnet.ibm.com
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: Stefan Hajnoczi <stefanha@gmail.com>, qemu-devel@nongnu.org, Christoph Hellwig <hch@lst.de>

On 10/12/2011 07:47 PM, Kevin Wolf wrote:
> Am 11.10.2011 05:10, schrieb Supriya Kannery:
>> Enhance "info block" to display hostcache setting for each
>> block device.

>> +    if (qdict_haskey(bs_dict, "open_flags")) {
>> +        int open_flags = qdict_get_int(bs_dict, "open_flags");
>> +        if (open_flags&  BDRV_O_NOCACHE)
>> +            monitor_printf(mon, " hostcache=0");
>> +        else
>> +            monitor_printf(mon, " hostcache=1");
>
> Coding style requires braces.
>

ok..will add. checkpatch.pl didn't catch this!

>>
>>           bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', "
>> -                                    "'removable': %i, 'locked': %i }",
>> +                                    "'removable': %i, 'locked': %i, "
>> +                                    "'hostcache': %i }",
>>                                       bs->device_name,
>>                                       bdrv_dev_has_removable_media(bs),
>> -                                    bdrv_dev_is_medium_locked(bs));
>> +                                    bdrv_dev_is_medium_locked(bs),
>> +                                    !(bs->open_flags&  BDRV_O_NOCACHE));
>>           bs_dict = qobject_to_qdict(bs_obj);
>> +        qdict_put(bs_dict, "open_flags", qint_from_int(bs->open_flags));
>
> No. This adds a open_flags field to the QMP structure that is
> transferred to clients. This is wrong, open_flags is an internal thing
> that should never be visible on an interface.
>
> In bdrv_print_dict, access the hostcache field that you introduced, it
> provides the same information.
>

Will replace "open_flags" with "hostcache" field.

thanks, Supriya