Patchwork [v1,1/10] Qemu: Enhance "info block" to display host cache setting

login
register
mail settings
Submitter Supriya Kannery
Date June 15, 2012, 8:47 p.m.
Message ID <20120615204701.9853.3126.sendpatchset@skannery.in.ibm.com>
Download mbox | patch
Permalink /patch/165219/
State New
Headers show

Comments

Supriya Kannery - June 15, 2012, 8:47 p.m.
Enhance "info block" to display hostcache setting for each
block device.

Example:
(qemu) info block
ide0-hd0: removable=0 file=../rhel6-32.raw ro=0 drv=raw encrypted=0

Enhanced to display "hostcache" setting:
(qemu) info block
ide0-hd0: removable=0 hostcache=1 file=../rhel6-32.raw ro=0 drv=raw encrypted=0

Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>

---
 block.c         |   20 ++++++++++++++++----
 qmp-commands.hx |    2 ++
 2 files changed, 18 insertions(+), 4 deletions(-)
Eric Blake - June 15, 2012, 9:07 p.m.
On 06/15/2012 02:47 PM, Supriya Kannery wrote:
> Enhance "info block" to display hostcache setting for each
> block device.
> 

>  ##
>  { 'type': 'BlockInfo',
>    'data': {'device': 'str', 'type': 'str', 'removable': 'bool',
> -           'locked': 'bool', '*inserted': 'BlockDeviceInfo',
> +           'locked': 'bool','hostcache': 'bool', '*inserted': 'BlockDeviceInfo',

space after comma

Since 'hostcache' was not present when talking to older qemu, should we
mark it optional?
Jeff Cody - July 5, 2012, 4:38 p.m.
On 06/15/2012 04:47 PM, Supriya Kannery wrote:
> Enhance "info block" to display hostcache setting for each
> block device.
> 
> Example:
> (qemu) info block
> ide0-hd0: removable=0 file=../rhel6-32.raw ro=0 drv=raw encrypted=0
> 
> Enhanced to display "hostcache" setting:
> (qemu) info block
> ide0-hd0: removable=0 hostcache=1 file=../rhel6-32.raw ro=0 drv=raw encrypted=0
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 

This email is not about any changes per se, but just noting some
conflicts (see below) with Paolo's blkmirror series (from his branch
blkmirror-job-1.2 in git://github.com/bonzini/qemu.git).  This is just
for future reference, I don't know which will go in first.


> ---
>  block.c         |   20 ++++++++++++++++----
>  qmp-commands.hx |    2 ++
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> Index: qemu/qapi-schema.json
> ===================================================================
> --- qemu.orig/qapi-schema.json
> +++ qemu/qapi-schema.json
> @@ -447,6 +447,8 @@
>  # @locked: True if the guest has locked this device from having its media
>  #          removed
>  #
> +# @hostcache: True if host pagecache is enabled.
> +#
>  # @tray_open: #optional True if the device has a tray and it is open
>  #             (only present if removable is true)
>  #
> @@ -460,7 +462,7 @@
>  ##
>  { 'type': 'BlockInfo',
>    'data': {'device': 'str', 'type': 'str', 'removable': 'bool',
> -           'locked': 'bool', '*inserted': 'BlockDeviceInfo',
> +           'locked': 'bool','hostcache': 'bool', '*inserted': 'BlockDeviceInfo',
>             '*tray_open': 'bool', '*io-status': 'BlockDeviceIoStatus'} }
>  

Conflicts with 'block: make device optional in BlockInfo' (f6c1f133a8),
but just in the contextual info (not in the actual change).

>  ##
> Index: qemu/block.c
> ===================================================================
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -2581,6 +2581,7 @@ BlockInfoList *qmp_query_block(Error **e
>          info->value->device = g_strdup(bs->device_name);
>          info->value->type = g_strdup("unknown");
>          info->value->locked = bdrv_dev_is_medium_locked(bs);
> +        info->value->hostcache = !(bs->open_flags & BDRV_O_NOCACHE);
>          info->value->removable = bdrv_dev_has_removable_media(bs);
>  
>          if (bdrv_dev_has_removable_media(bs)) {


Conflicts with 'block: add bdrv_query_info' (804ce1520d)
This would probably go in his new function 'bdrv_query_info()'.


> Index: qemu/hmp.c
> ===================================================================
> --- qemu.orig/hmp.c
> +++ qemu/hmp.c
> @@ -212,6 +212,8 @@ void hmp_info_block(Monitor *mon)
>              monitor_printf(mon, " tray-open=%d", info->value->tray_open);
>          }
>  
> +        monitor_printf(mon, " hostcache=%d", info->value->hostcache);
> +
>          if (info->value->has_io_status) {
>              monitor_printf(mon, " io-status=%s",
>                             BlockDeviceIoStatus_lookup[info->value->io_status]);
> 
>
Kevin Wolf - July 9, 2012, 2:43 p.m.
Am 15.06.2012 23:07, schrieb Eric Blake:
> On 06/15/2012 02:47 PM, Supriya Kannery wrote:
>> Enhance "info block" to display hostcache setting for each
>> block device.
>>
> 
>>  ##
>>  { 'type': 'BlockInfo',
>>    'data': {'device': 'str', 'type': 'str', 'removable': 'bool',
>> -           'locked': 'bool', '*inserted': 'BlockDeviceInfo',
>> +           'locked': 'bool','hostcache': 'bool', '*inserted': 'BlockDeviceInfo',
> 
> space after comma
> 
> Since 'hostcache' was not present when talking to older qemu, should we
> mark it optional?

What does "optional" really mean? I always understood that it means that
whether the field exists or not depends on some runtime condition, not
on the qemu version. I would specify something like this, that always
exists in new qemu versions, in the "Since" section. Or maybe a separate
"Since" specification like in SpiceInfo for mouse-mode.

Kevin
Luiz Capitulino - July 11, 2012, 2:03 p.m.
On Mon, 09 Jul 2012 16:43:40 +0200
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 15.06.2012 23:07, schrieb Eric Blake:
> > On 06/15/2012 02:47 PM, Supriya Kannery wrote:
> >> Enhance "info block" to display hostcache setting for each
> >> block device.
> >>
> > 
> >>  ##
> >>  { 'type': 'BlockInfo',
> >>    'data': {'device': 'str', 'type': 'str', 'removable': 'bool',
> >> -           'locked': 'bool', '*inserted': 'BlockDeviceInfo',
> >> +           'locked': 'bool','hostcache': 'bool', '*inserted': 'BlockDeviceInfo',
> > 
> > space after comma
> > 
> > Since 'hostcache' was not present when talking to older qemu, should we
> > mark it optional?
> 
> What does "optional" really mean? I always understood that it means that
> whether the field exists or not depends on some runtime condition, not
> on the qemu version. I would specify something like this, that always
> exists in new qemu versions, in the "Since" section. Or maybe a separate
> "Since" specification like in SpiceInfo for mouse-mode.

Yes, Kevin is right.
Supriya Kannery - July 29, 2012, 6:21 a.m.
On 07/11/2012 07:33 PM, Luiz Capitulino wrote:
> On Mon, 09 Jul 2012 16:43:40 +0200
> Kevin Wolf<kwolf@redhat.com>  wrote:
>
>> Am 15.06.2012 23:07, schrieb Eric Blake:
>>> On 06/15/2012 02:47 PM, Supriya Kannery wrote:
>>>> Enhance "info block" to display hostcache setting for each
>>>> block device.
>>>>
>>>
>>>>   ##
>>>>   { 'type': 'BlockInfo',
>>>>     'data': {'device': 'str', 'type': 'str', 'removable': 'bool',
>>>> -           'locked': 'bool', '*inserted': 'BlockDeviceInfo',
>>>> +           'locked': 'bool','hostcache': 'bool', '*inserted': 'BlockDeviceInfo',
>>>
>>> space after comma
>>>
>>> Since 'hostcache' was not present when talking to older qemu, should we
>>> mark it optional?
>>
>> What does "optional" really mean? I always understood that it means that
>> whether the field exists or not depends on some runtime condition, not
>> on the qemu version. I would specify something like this, that always
>> exists in new qemu versions, in the "Since" section. Or maybe a separate
>> "Since" specification like in SpiceInfo for mouse-mode.
>
> Yes, Kevin is right.
>

Will add a comment " Since: 1.x"

-thanks, Supriya
Supriya Kannery - July 29, 2012, 6:54 a.m.
On 07/05/2012 10:08 PM, Jeff Cody wrote:
> On 06/15/2012 04:47 PM, Supriya Kannery wrote:
>> Enhance "info block" to display hostcache setting for each
>> block device.
>>
>> Example:
>> (qemu) info block
>> ide0-hd0: removable=0 file=../rhel6-32.raw ro=0 drv=raw encrypted=0
>>
>> Enhanced to display "hostcache" setting:
>> (qemu) info block
>> ide0-hd0: removable=0 hostcache=1 file=../rhel6-32.raw ro=0 drv=raw encrypted=0
>>
>> Signed-off-by: Supriya Kannery<supriyak@linux.vnet.ibm.com>
>>
>
> This email is not about any changes per se, but just noting some
> conflicts (see below) with Paolo's blkmirror series (from his branch
> blkmirror-job-1.2 in git://github.com/bonzini/qemu.git).  This is just
> for future reference, I don't know which will go in first.
>
>

I have been using code from git://git.qemu.org/qemu.git for posting
patches. Will use the same for next version of this patchset as well, so 
that patches are prepared over the latest code changes,
including Paolo's patchset if they are upstream.
-thanks, Supriya

Patch

Index: qemu/qapi-schema.json
===================================================================
--- qemu.orig/qapi-schema.json
+++ qemu/qapi-schema.json
@@ -447,6 +447,8 @@ 
 # @locked: True if the guest has locked this device from having its media
 #          removed
 #
+# @hostcache: True if host pagecache is enabled.
+#
 # @tray_open: #optional True if the device has a tray and it is open
 #             (only present if removable is true)
 #
@@ -460,7 +462,7 @@ 
 ##
 { 'type': 'BlockInfo',
   'data': {'device': 'str', 'type': 'str', 'removable': 'bool',
-           'locked': 'bool', '*inserted': 'BlockDeviceInfo',
+           'locked': 'bool','hostcache': 'bool', '*inserted': 'BlockDeviceInfo',
            '*tray_open': 'bool', '*io-status': 'BlockDeviceIoStatus'} }
 
 ##
Index: qemu/block.c
===================================================================
--- qemu.orig/block.c
+++ qemu/block.c
@@ -2581,6 +2581,7 @@  BlockInfoList *qmp_query_block(Error **e
         info->value->device = g_strdup(bs->device_name);
         info->value->type = g_strdup("unknown");
         info->value->locked = bdrv_dev_is_medium_locked(bs);
+        info->value->hostcache = !(bs->open_flags & BDRV_O_NOCACHE);
         info->value->removable = bdrv_dev_has_removable_media(bs);
 
         if (bdrv_dev_has_removable_media(bs)) {
Index: qemu/hmp.c
===================================================================
--- qemu.orig/hmp.c
+++ qemu/hmp.c
@@ -212,6 +212,8 @@  void hmp_info_block(Monitor *mon)
             monitor_printf(mon, " tray-open=%d", info->value->tray_open);
         }
 
+        monitor_printf(mon, " hostcache=%d", info->value->hostcache);
+
         if (info->value->has_io_status) {
             monitor_printf(mon, " io-status=%s",
                            BlockDeviceIoStatus_lookup[info->value->io_status]);