Re: [Qemu-devel] [v7 Patch 1/5]Qemu: Enhance "info block" to display host cache setting

[Supriya Kannery <supriyak@linux.vnet.ibm.com>]

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