[Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen

[Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen

[Supriya Kannery]

 For changing host pagecache setting of a running VM, it is
important to have a safe way of reopening its image file.

Following patchset introduces:
 * a generic way to reopen image files safely. 
        In this approach, before reopening an image, for each
    block driver, its state will be stashed. Incase preparation,
    (bdrv_reopen_prepare) for reopening returns success, the stashed 
    state will be cleared (bdrv_reopen_commit) and reopened state will 
    be used further. Incase preparation of reopening returns failure, 
    the state of the driver will be rolled back (bdrv_reopen_abort) 
    to the stashed state. This approach is implemented for raw-posix, 
    raw-win32, vmdk, qcow, qcow2 and qed block drivers.
  
 * qmp and hmp command 'block_set_hostcache' using which host 
   pagecache setting for a block device can be changed 
   when the VM is running.

 * BDRVReopenState, a generic structure which can be 
   extended by each of the block drivers to reopen 
   respective image files.

ToDo:
* Currently driver state is stashed by field-by-field copy.
  Optimise this by stashing only the required fields.
* Find out other drivers needing bdrv_reopen implementation.
* Do some more extensive testing, especially with drivers like
  floppy, cdrom etc..

Earlier discussions on RFC for dynamic change of host pagecache can be
 found at: http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg04139.html

New block command added:
"block_set_hostcache"
    -- Sets hostcache parameter for block device  while guest is running.

Usage:
 block_set_hostcache  <device> <option>
   <device> = block device
   <option>  = on/off

 qemu/block.c           |  127 ++++++++++++++++++++++++++++++++--
 qemu/block.h           |    5 +
 qemu/block/qcow.c      |  108 +++++++++++++++++++++++++++++
 qemu/block/qcow2.c     |  177 +++++++++++++++++++++++++++++++++++++++++++++++++
 qemu/block/qed.c       |  103 ++++++++++++++++++++++++++++
 qemu/block/raw-posix.c |  123 ++++++++++++++++++++++++++++++++++
 qemu/block/raw-win32.c |   96 ++++++++++++++++++++++++++
 qemu/block/raw.c       |   20 +++++
 qemu/block/vmdk.c      |  103 ++++++++++++++++++++++++++++
 qemu/block_int.h       |   11 +++
 qemu/blockdev.c        |   24 ++++++
 qemu/hmp-commands.hx   |   16 ++++
 qemu/hmp.c             |   11 ++
 qemu/hmp.h             |    1
 qemu/qapi-schema.json  |   20 ++++-
 qemu/qemu-common.h     |    1
 qemu/qerror.c          |    8 ++
 qemu/qerror.h          |    6 +
 qemu/qmp-commands.hx   |   24 ++++++
 19 files changed, 971 insertions(+), 13 deletions(-)

[Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting

[Supriya Kannery]

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(-)

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]);

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

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?

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

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

[Kevin Wolf]

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

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

[Luiz Capitulino]

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.

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

[Supriya Kannery]

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

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

[Jeff Cody]

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]);
> 
> 

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

[Supriya Kannery]

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

[Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures

[Supriya Kannery]

New error classes defined for hostcache setting and data 
sync error

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

---
 qerror.c |    8 ++++++++
 qerror.h |    6 ++++++
 2 files changed, 14 insertions(+)

Index: qemu/qerror.c
===================================================================
--- qemu.orig/qerror.c
+++ qemu/qerror.c
@@ -80,6 +80,10 @@ static const QErrorStringTable qerror_ta
         .desc      = "The command %(name) has not been found",
     },
     {
+        .error_fmt = QERR_DATA_SYNC_FAILED,
+        .desc      = "Syncing of data failed for device '%(device)'",
+    },
+    {
         .error_fmt = QERR_DEVICE_ENCRYPTED,
         .desc      = "Device '%(device)' is encrypted",
     },
@@ -152,6 +156,10 @@ static const QErrorStringTable qerror_ta
         .desc      = "The feature '%(name)' is not enabled",
     },
     {
+        .error_fmt = QERR_HOSTCACHE_NOT_CHANGED,
+        .desc      = "Could not change hostcache setting for '%(device)'",
+    },
+    {
         .error_fmt = QERR_INVALID_BLOCK_FORMAT,
         .desc      = "Invalid block format '%(name)'",
     },
Index: qemu/qerror.h
===================================================================
--- qemu.orig/qerror.h
+++ qemu/qerror.h
@@ -82,6 +82,9 @@ QError *qobject_to_qerror(const QObject 
 #define QERR_COMMAND_NOT_FOUND \
     "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
 
+#define QERR_DATA_SYNC_FAILED \
+    "{ 'class': 'DataSyncFailed', 'data': { 'device': %s } }"
+
 #define QERR_DEVICE_ENCRYPTED \
     "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
 
@@ -136,6 +139,9 @@ QError *qobject_to_qerror(const QObject 
 #define QERR_FEATURE_DISABLED \
     "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
 
+#define QERR_HOSTCACHE_NOT_CHANGED \
+    "{ 'class': 'HostcacheNotChanged', 'data': { 'device': %s } }"
+
 #define QERR_INVALID_BLOCK_FORMAT \
     "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
 

Re: [Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures

[Kevin Wolf]

Am 15.06.2012 22:47, schrieb Supriya Kannery:
> New error classes defined for hostcache setting and data 
> sync error
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 
> ---
>  qerror.c |    8 ++++++++
>  qerror.h |    6 ++++++
>  2 files changed, 14 insertions(+)
> 
> Index: qemu/qerror.c
> ===================================================================
> --- qemu.orig/qerror.c
> +++ qemu/qerror.c
> @@ -80,6 +80,10 @@ static const QErrorStringTable qerror_ta
>          .desc      = "The command %(name) has not been found",
>      },
>      {
> +        .error_fmt = QERR_DATA_SYNC_FAILED,
> +        .desc      = "Syncing of data failed for device '%(device)'",
> +    },
> +    {
>          .error_fmt = QERR_DEVICE_ENCRYPTED,
>          .desc      = "Device '%(device)' is encrypted",
>      },
> @@ -152,6 +156,10 @@ static const QErrorStringTable qerror_ta
>          .desc      = "The feature '%(name)' is not enabled",
>      },
>      {
> +        .error_fmt = QERR_HOSTCACHE_NOT_CHANGED,
> +        .desc      = "Could not change hostcache setting for '%(device)'",
> +    },
> +    {
>          .error_fmt = QERR_INVALID_BLOCK_FORMAT,
>          .desc      = "Invalid block format '%(name)'",
>      },
> Index: qemu/qerror.h
> ===================================================================
> --- qemu.orig/qerror.h
> +++ qemu/qerror.h
> @@ -82,6 +82,9 @@ QError *qobject_to_qerror(const QObject 
>  #define QERR_COMMAND_NOT_FOUND \
>      "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
>  
> +#define QERR_DATA_SYNC_FAILED \
> +    "{ 'class': 'DataSyncFailed', 'data': { 'device': %s } }"
> +
>  #define QERR_DEVICE_ENCRYPTED \
>      "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
>  
> @@ -136,6 +139,9 @@ QError *qobject_to_qerror(const QObject 
>  #define QERR_FEATURE_DISABLED \
>      "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
>  
> +#define QERR_HOSTCACHE_NOT_CHANGED \
> +    "{ 'class': 'HostcacheNotChanged', 'data': { 'device': %s } }"
> +
>  #define QERR_INVALID_BLOCK_FORMAT \
>      "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"

In the light of the recent error handling discussion: Do we really need
two separate errors? Can we just reuse an existing one? Just
QERR_IO_ERROR could be good enough.

Kevin

Re: [Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures

[Supriya Kannery]

On 07/09/2012 08:17 PM, Kevin Wolf wrote:
> Am 15.06.2012 22:47, schrieb Supriya Kannery:
>> New error classes defined for hostcache setting and data
>> sync error
>>
>> Signed-off-by: Supriya Kannery<supriyak@linux.vnet.ibm.com>
>>
>> ---
>>   qerror.c |    8 ++++++++
>>   qerror.h |    6 ++++++
>>   2 files changed, 14 insertions(+)
>>
>> Index: qemu/qerror.c
>> ===================================================================
>> --- qemu.orig/qerror.c
>> +++ qemu/qerror.c
>> @@ -80,6 +80,10 @@ static const QErrorStringTable qerror_ta
>>           .desc      = "The command %(name) has not been found",
>>       },
>>       {
>> +        .error_fmt = QERR_DATA_SYNC_FAILED,
>> +        .desc      = "Syncing of data failed for device '%(device)'",
>> +    },
>> +    {
>>           .error_fmt = QERR_DEVICE_ENCRYPTED,
>>           .desc      = "Device '%(device)' is encrypted",
>>       },
>> @@ -152,6 +156,10 @@ static const QErrorStringTable qerror_ta
>>           .desc      = "The feature '%(name)' is not enabled",
>>       },
>>       {
>> +        .error_fmt = QERR_HOSTCACHE_NOT_CHANGED,
>> +        .desc      = "Could not change hostcache setting for '%(device)'",
>> +    },
>> +    {
>>           .error_fmt = QERR_INVALID_BLOCK_FORMAT,
>>           .desc      = "Invalid block format '%(name)'",
>>       },
>> Index: qemu/qerror.h
>> ===================================================================
>> --- qemu.orig/qerror.h
>> +++ qemu/qerror.h
>> @@ -82,6 +82,9 @@ QError *qobject_to_qerror(const QObject
>>   #define QERR_COMMAND_NOT_FOUND \
>>       "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
>>
>> +#define QERR_DATA_SYNC_FAILED \
>> +    "{ 'class': 'DataSyncFailed', 'data': { 'device': %s } }"
>> +
>>   #define QERR_DEVICE_ENCRYPTED \
>>       "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
>>
>> @@ -136,6 +139,9 @@ QError *qobject_to_qerror(const QObject
>>   #define QERR_FEATURE_DISABLED \
>>       "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
>>
>> +#define QERR_HOSTCACHE_NOT_CHANGED \
>> +    "{ 'class': 'HostcacheNotChanged', 'data': { 'device': %s } }"
>> +
>>   #define QERR_INVALID_BLOCK_FORMAT \
>>       "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
>
> In the light of the recent error handling discussion: Do we really need
> two separate errors? Can we just reuse an existing one? Just
> QERR_IO_ERROR could be good enough.
>
> Kevin
>

Yes, will use QERR_IO_ERROR instead of these two new errors.

-thanks, Supriya

[Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change

[Supriya Kannery]

New command "block_set_hostcache" added for dynamically changing 
host pagecache setting of a block device.

Usage: 
 block_set_hostcache  <device> <option>
   <device> = block device
   <option> = on/off

Example:
 (qemu) block_set_hostcache ide0-hd0 off

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

---
 block.c         |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 block.h         |    2 ++
 blockdev.c      |   26 ++++++++++++++++++++++++++
 blockdev.h      |    2 ++
 hmp-commands.hx |   14 ++++++++++++++
 qmp-commands.hx |   27 +++++++++++++++++++++++++++
 6 files changed, 125 insertions(+)

Index: qemu/block.c
===================================================================
--- qemu.orig/block.c
+++ qemu/block.c
@@ -858,6 +858,35 @@ unlink_and_fail:
     return ret;
 }
 
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
+{
+    BlockDriver *drv = bs->drv;
+    int ret = 0, open_flags;
+
+    /* Quiesce IO for the given block device */
+    qemu_aio_flush();
+    ret = bdrv_flush(bs);
+    if (ret != 0) {
+        qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
+        return ret;
+    }
+    open_flags = bs->open_flags;
+    bdrv_close(bs);
+
+    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
+    if (ret < 0) {
+        /* Reopen failed. Try to open with original flags */
+        qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
+        ret = bdrv_open(bs, bs->filename, open_flags, drv);
+        if (ret < 0) {
+            /* Reopen failed with orig and modified flags */
+            abort();
+        }
+    }
+
+    return ret;
+}
+
 void bdrv_close(BlockDriverState *bs)
 {
     bdrv_flush(bs);
@@ -953,6 +982,34 @@ void bdrv_drain_all(void)
     }
 }
 
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable)
+{
+    int bdrv_flags = bs->open_flags;
+
+    /* set hostcache flags (without changing WCE/flush bits) */
+    if (enable) {
+        bdrv_flags &= ~BDRV_O_NOCACHE;
+    } else {
+        bdrv_flags |= BDRV_O_NOCACHE;
+    }
+
+    /* If no change in flags, no need to reopen */
+    if (bdrv_flags == bs->open_flags) {
+        printf("no need to change\n");
+        return 0;
+    }
+
+    if (bdrv_is_inserted(bs)) {
+        /* Reopen file with changed set of flags */
+        bdrv_flags &= ~BDRV_O_CACHE_WB;
+        return bdrv_reopen(bs, bdrv_flags);
+    } else {
+        /* Save hostcache change for future use */
+        bs->open_flags = bdrv_flags;
+        return 0;
+    }
+}
+
 /* make a BlockDriverState anonymous by removing from bdrv_state list.
    Also, NULL terminate the device_name to prevent double remove */
 void bdrv_make_anon(BlockDriverState *bs)
Index: qemu/block.h
===================================================================
--- qemu.orig/block.h
+++ qemu/block.h
@@ -128,6 +128,7 @@ int bdrv_parse_cache_flags(const char *m
 int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
 int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
               BlockDriver *drv);
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags);
 void bdrv_close(BlockDriverState *bs);
 int bdrv_attach_dev(BlockDriverState *bs, void *dev);
 void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
@@ -177,6 +178,7 @@ int bdrv_commit_all(void);
 int bdrv_change_backing_file(BlockDriverState *bs,
     const char *backing_file, const char *backing_fmt);
 void bdrv_register(BlockDriver *bdrv);
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache);
 
 
 typedef struct BdrvCheckResult {
Index: qemu/blockdev.c
===================================================================
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -1198,3 +1198,27 @@ BlockJobInfoList *qmp_query_block_jobs(E
     bdrv_iterate(do_qmp_query_block_jobs_one, &prev);
     return dummy.next;
 }
+
+
+/*
+ * Change host page cache setting while guest is running.
+*/
+void qmp_block_set_hostcache(const char *device, bool enable,
+                             Error **errp)
+{
+    BlockDriverState *bs = NULL;
+
+    /* Validate device */
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    if (bdrv_change_hostcache(bs, enable)) {
+        error_set(errp, QERR_HOSTCACHE_NOT_CHANGED, device);
+    }
+
+    return;
+}
+
Index: qemu/hmp-commands.hx
===================================================================
--- qemu.orig/hmp-commands.hx
+++ qemu/hmp-commands.hx
@@ -808,7 +808,6 @@ ETEXI
         .mhandler.cmd = hmp_migrate,
     },
 
-
 STEXI
 @item migrate [-d] [-b] [-i] @var{uri}
 @findex migrate
@@ -1314,6 +1313,21 @@ client.  @var{keep} changes the password
 ETEXI
 
     {
+        .name       = "block_set_hostcache",
+        .args_type  = "device:B,option:b",
+        .params     = "device on|off",
+        .help       = "Change setting of host pagecache",
+        .mhandler.cmd = hmp_block_set_hostcache,
+    },
+
+STEXI
+@item block_set_hostcache @var{device} @var{option}
+@findex block_set_hostcache
+Change host pagecache setting of a block device while guest is running.
+ETEXI
+
+
+    {
         .name       = "expire_password",
         .args_type  = "protocol:s,time:s",
         .params     = "protocol time",
Index: qemu/qmp-commands.hx
===================================================================
--- qemu.orig/qmp-commands.hx
+++ qemu/qmp-commands.hx
@@ -821,7 +821,31 @@ Example:
 
 EQMP
 
+
     {
+        .name       = "block_set_hostcache",
+        .args_type  = "device:B,option:b",
+        .mhandler.cmd_new = qmp_block_set_hostcache,
+    },
+
+SQMP
+block_set_hostcache
+-------------------
+
+Change host pagecache setting of a block device
+
+Arguments:
+
+- "device": the device's ID, must be unique (json-string)
+- "option": hostcache setting (json-bool)
+
+Example:
+-> { "execute": "block_set_hostcache", "arguments": { "device": "ide0-hd0", "option": false } }
+<- { "return": {} }
+
+EQMP
+
+	{
         .name       = "balloon",
         .args_type  = "value:M",
         .mhandler.cmd_new = qmp_marshal_input_balloon,
Index: qemu/qapi-schema.json
===================================================================
--- qemu.orig/qapi-schema.json
+++ qemu/qapi-schema.json
@@ -1598,6 +1598,22 @@
 { 'command': 'block_set_io_throttle',
   'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
             'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int' } }
+##
+# @block_set_hostcache:
+#
+# Change host pagecache setting of a block device
+#
+# @device: name of the block device
+#
+# @option: hostcache setting (true/false)
+#
+# Returns: Nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#
+# Since: 1.2
+##
+{ 'command': 'block_set_hostcache',
+  'data': { 'device': 'str', 'option': 'bool' } }
 
 ##
 # @block-stream:
Index: qemu/hmp.c
===================================================================
--- qemu.orig/hmp.c
+++ qemu/hmp.c
@@ -836,6 +836,15 @@ void hmp_block_set_io_throttle(Monitor *
     hmp_handle_error(mon, &err);
 }
 
+void hmp_block_set_hostcache(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+
+    qmp_block_set_hostcache(qdict_get_str(qdict, "device"),
+                              qdict_get_bool(qdict, "option"), &err);
+    hmp_handle_error(mon, &err);
+}
+
 void hmp_block_stream(Monitor *mon, const QDict *qdict)
 {
     Error *error = NULL;
Index: qemu/hmp.h
===================================================================
--- qemu.orig/hmp.h
+++ qemu/hmp.h
@@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const 
 void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
 void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_block_set_hostcache(Monitor *mon, const QDict *qdict);
 
 #endif

Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3548 bytes --]

On 06/15/2012 02:47 PM, Supriya Kannery wrote:
> New command "block_set_hostcache" added for dynamically changing 
> host pagecache setting of a block device.
> 
> Usage: 
>  block_set_hostcache  <device> <option>
>    <device> = block device
>    <option> = on/off
> 
> Example:
>  (qemu) block_set_hostcache ide0-hd0 off
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 
> ---
>  block.c         |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block.h         |    2 ++
>  blockdev.c      |   26 ++++++++++++++++++++++++++
>  blockdev.h      |    2 ++
>  hmp-commands.hx |   14 ++++++++++++++
>  qmp-commands.hx |   27 +++++++++++++++++++++++++++
>  6 files changed, 125 insertions(+)

Doesn't this also need to touch qapi-schema.json?
[/me reads full patch]
Oh, you did - but your diffstat is stale.  It might be worth figuring
out what in your workflow leads to stale diffstats.

> 
> Index: qemu/block.c
> ===================================================================
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -858,6 +858,35 @@ unlink_and_fail:
>      return ret;
>  }
>  
> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
> +{
> +    BlockDriver *drv = bs->drv;
> +    int ret = 0, open_flags;
> +
> +    /* Quiesce IO for the given block device */
> +    qemu_aio_flush();
> +    ret = bdrv_flush(bs);
> +    if (ret != 0) {
> +        qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
> +        return ret;
> +    }
> +    open_flags = bs->open_flags;
> +    bdrv_close(bs);
> +
> +    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);

Yuck.  This is bad, and why 'transaction' was invented.  Any time you
close() before open() you risk completely losing the file...

> +    if (ret < 0) {
> +        /* Reopen failed. Try to open with original flags */
> +        qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> +        ret = bdrv_open(bs, bs->filename, open_flags, drv);
> +        if (ret < 0) {
> +            /* Reopen failed with orig and modified flags */
> +            abort();

and an abort() is not a nice reaction to that.

I think we should rebase the series to do the safe reopen prior to
adding this command (at least, just judging by the title of 4/10), to
avoid intermediate bad code.


> @@ -808,7 +808,6 @@ ETEXI
>          .mhandler.cmd = hmp_migrate,
>      },
>  
> -
>  STEXI

Spurious whitespace change.


> +
> +SQMP
> +block_set_hostcache

QMP commands favor '-' over '_'; this should be 'block-set-hostcache'.


> Index: qemu/qapi-schema.json
> ===================================================================
> --- qemu.orig/qapi-schema.json
> +++ qemu/qapi-schema.json
> @@ -1598,6 +1598,22 @@
>  { 'command': 'block_set_io_throttle',
>    'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
>              'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int' } }
> +##
> +# @block_set_hostcache:
> +#
> +# Change host pagecache setting of a block device
> +#
> +# @device: name of the block device
> +#
> +# @option: hostcache setting (true/false)
> +#
> +# Returns: Nothing on success
> +#          If @device is not a valid block device, DeviceNotFound

What happens if the device is valid, but the setting cannot be changed
(perhaps because reopen has not been implemented for that driver)?

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change

[Supriya Kannery]

On 06/16/2012 03:26 AM, Eric Blake wrote:
> On 06/15/2012 02:47 PM, Supriya Kannery wrote:
>> New command "block_set_hostcache" added for dynamically changing
>> host pagecache setting of a block device.
>>
>> Usage:
>>   block_set_hostcache<device>  <option>
>>     <device>  = block device
>>     <option>  = on/off
>>
>> Example:
>>   (qemu) block_set_hostcache ide0-hd0 off
>>
>> Signed-off-by: Supriya Kannery<supriyak@linux.vnet.ibm.com>
>>
>> ---
>>   block.c         |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   block.h         |    2 ++
>>   blockdev.c      |   26 ++++++++++++++++++++++++++
>>   blockdev.h      |    2 ++
>>   hmp-commands.hx |   14 ++++++++++++++
>>   qmp-commands.hx |   27 +++++++++++++++++++++++++++
>>   6 files changed, 125 insertions(+)
>
> Doesn't this also need to touch qapi-schema.json?
> [/me reads full patch]
> Oh, you did - but your diffstat is stale.  It might be worth figuring
> out what in your workflow leads to stale diffstats.
>
>>
>> Index: qemu/block.c
>> ===================================================================
>> --- qemu.orig/block.c
>> +++ qemu/block.c
>> @@ -858,6 +858,35 @@ unlink_and_fail:
>>       return ret;
>>   }
>>
>> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
>> +{
>> +    BlockDriver *drv = bs->drv;
>> +    int ret = 0, open_flags;
>> +
>> +    /* Quiesce IO for the given block device */
>> +    qemu_aio_flush();
>> +    ret = bdrv_flush(bs);
>> +    if (ret != 0) {
>> +        qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
>> +        return ret;
>> +    }
>> +    open_flags = bs->open_flags;
>> +    bdrv_close(bs);
>> +
>> +    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
>
> Yuck.  This is bad, and why 'transaction' was invented.  Any time you
> close() before open() you risk completely losing the file...
>
>> +    if (ret<  0) {
>> +        /* Reopen failed. Try to open with original flags */
>> +        qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
>> +        ret = bdrv_open(bs, bs->filename, open_flags, drv);
>> +        if (ret<  0) {
>> +            /* Reopen failed with orig and modified flags */
>> +            abort();
>
> and an abort() is not a nice reaction to that.
>
> I think we should rebase the series to do the safe reopen prior to
> adding this command (at least, just judging by the title of 4/10), to
> avoid intermediate bad code.
>
>

  Yes, will reorder the patches to have safe reopen done first and
then block_set_hostcache use it.

-thanks, Supriya

Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change

[Jeff Cody]

On 06/15/2012 04:47 PM, Supriya Kannery wrote:
> New command "block_set_hostcache" added for dynamically changing 
> host pagecache setting of a block device.
> 
> Usage: 
>  block_set_hostcache  <device> <option>
>    <device> = block device
>    <option> = on/off
> 
> Example:
>  (qemu) block_set_hostcache ide0-hd0 off
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 
> ---
>  block.c         |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block.h         |    2 ++
>  blockdev.c      |   26 ++++++++++++++++++++++++++
>  blockdev.h      |    2 ++
>  hmp-commands.hx |   14 ++++++++++++++
>  qmp-commands.hx |   27 +++++++++++++++++++++++++++
>  6 files changed, 125 insertions(+)
> 
> Index: qemu/block.c
> ===================================================================
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -858,6 +858,35 @@ unlink_and_fail:
>      return ret;
>  }
>  
> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
> +{
> +    BlockDriver *drv = bs->drv;
> +    int ret = 0, open_flags;
> +
> +    /* Quiesce IO for the given block device */
> +    qemu_aio_flush();
> +    ret = bdrv_flush(bs);
> +    if (ret != 0) {
> +        qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
> +        return ret;
> +    }
> +    open_flags = bs->open_flags;
> +    bdrv_close(bs);
> +
> +    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
> +    if (ret < 0) {
> +        /* Reopen failed. Try to open with original flags */
> +        qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> +        ret = bdrv_open(bs, bs->filename, open_flags, drv);
> +        if (ret < 0) {
> +            /* Reopen failed with orig and modified flags */
> +            abort();
> +        }
> +    }
> +
> +    return ret;
> +}
> +
>  void bdrv_close(BlockDriverState *bs)
>  {
>      bdrv_flush(bs);
> @@ -953,6 +982,34 @@ void bdrv_drain_all(void)
>      }
>  }
>  
> +int bdrv_change_hostcache(BlockDriverState *bs, bool enable)
> +{
> +    int bdrv_flags = bs->open_flags;
> +
> +    /* set hostcache flags (without changing WCE/flush bits) */
> +    if (enable) {
> +        bdrv_flags &= ~BDRV_O_NOCACHE;
> +    } else {
> +        bdrv_flags |= BDRV_O_NOCACHE;
> +    }
> +
> +    /* If no change in flags, no need to reopen */
> +    if (bdrv_flags == bs->open_flags) {
> +        printf("no need to change\n");
> +        return 0;
> +    }
> +
> +    if (bdrv_is_inserted(bs)) {
> +        /* Reopen file with changed set of flags */
> +        bdrv_flags &= ~BDRV_O_CACHE_WB;
> +        return bdrv_reopen(bs, bdrv_flags);
> +    } else {
> +        /* Save hostcache change for future use */
> +        bs->open_flags = bdrv_flags;
> +        return 0;
> +    }
> +}
> +
>  /* make a BlockDriverState anonymous by removing from bdrv_state list.
>     Also, NULL terminate the device_name to prevent double remove */
>  void bdrv_make_anon(BlockDriverState *bs)
> Index: qemu/block.h
> ===================================================================
> --- qemu.orig/block.h
> +++ qemu/block.h
> @@ -128,6 +128,7 @@ int bdrv_parse_cache_flags(const char *m
>  int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
>  int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
>                BlockDriver *drv);
> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags);
>  void bdrv_close(BlockDriverState *bs);
>  int bdrv_attach_dev(BlockDriverState *bs, void *dev);
>  void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
> @@ -177,6 +178,7 @@ int bdrv_commit_all(void);
>  int bdrv_change_backing_file(BlockDriverState *bs,
>      const char *backing_file, const char *backing_fmt);
>  void bdrv_register(BlockDriver *bdrv);
> +int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache);
>  
>  
>  typedef struct BdrvCheckResult {
> Index: qemu/blockdev.c
> ===================================================================
> --- qemu.orig/blockdev.c
> +++ qemu/blockdev.c
> @@ -1198,3 +1198,27 @@ BlockJobInfoList *qmp_query_block_jobs(E
>      bdrv_iterate(do_qmp_query_block_jobs_one, &prev);
>      return dummy.next;
>  }
> +
> +
> +/*
> + * Change host page cache setting while guest is running.
> +*/
> +void qmp_block_set_hostcache(const char *device, bool enable,
> +                             Error **errp)
> +{
> +    BlockDriverState *bs = NULL;
> +
> +    /* Validate device */
> +    bs = bdrv_find(device);
> +    if (!bs) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +        return;
> +    }
> +
> +    if (bdrv_change_hostcache(bs, enable)) {
> +        error_set(errp, QERR_HOSTCACHE_NOT_CHANGED, device);
> +    }
> +
> +    return;
> +}
> +
> Index: qemu/hmp-commands.hx
> ===================================================================
> --- qemu.orig/hmp-commands.hx
> +++ qemu/hmp-commands.hx
> @@ -808,7 +808,6 @@ ETEXI
>          .mhandler.cmd = hmp_migrate,
>      },
>  
> -
>  STEXI
>  @item migrate [-d] [-b] [-i] @var{uri}
>  @findex migrate
> @@ -1314,6 +1313,21 @@ client.  @var{keep} changes the password
>  ETEXI
>  
>      {
> +        .name       = "block_set_hostcache",
> +        .args_type  = "device:B,option:b",
> +        .params     = "device on|off",
> +        .help       = "Change setting of host pagecache",
> +        .mhandler.cmd = hmp_block_set_hostcache,
> +    },
> +
> +STEXI
> +@item block_set_hostcache @var{device} @var{option}
> +@findex block_set_hostcache
> +Change host pagecache setting of a block device while guest is running.
> +ETEXI
> +
> +
> +    {
>          .name       = "expire_password",
>          .args_type  = "protocol:s,time:s",
>          .params     = "protocol time",
> Index: qemu/qmp-commands.hx
> ===================================================================
> --- qemu.orig/qmp-commands.hx
> +++ qemu/qmp-commands.hx
> @@ -821,7 +821,31 @@ Example:
>  
>  EQMP
>  
> +
>      {
> +        .name       = "block_set_hostcache",
> +        .args_type  = "device:B,option:b",
> +        .mhandler.cmd_new = qmp_block_set_hostcache,

The QAPI commands need to go through the marshaller - this needs to be:
           .mhandler.cmd_new = qmp_marshal_input_block_set_hostcache,

(qmp_marshal_input_block_set_hostcache is automatically generated, and
it will call qmp_block_set_hostcache)

> +    },
> +
> +SQMP
> +block_set_hostcache
> +-------------------
> +
> +Change host pagecache setting of a block device
> +
> +Arguments:
> +
> +- "device": the device's ID, must be unique (json-string)
> +- "option": hostcache setting (json-bool)
> +
> +Example:
> +-> { "execute": "block_set_hostcache", "arguments": { "device": "ide0-hd0", "option": false } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +	{
>          .name       = "balloon",
>          .args_type  = "value:M",
>          .mhandler.cmd_new = qmp_marshal_input_balloon,
> Index: qemu/qapi-schema.json
> ===================================================================
> --- qemu.orig/qapi-schema.json
> +++ qemu/qapi-schema.json
> @@ -1598,6 +1598,22 @@
>  { 'command': 'block_set_io_throttle',
>    'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
>              'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int' } }
> +##
> +# @block_set_hostcache:
> +#
> +# Change host pagecache setting of a block device
> +#
> +# @device: name of the block device
> +#
> +# @option: hostcache setting (true/false)
> +#
> +# Returns: Nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#
> +# Since: 1.2
> +##
> +{ 'command': 'block_set_hostcache',
> +  'data': { 'device': 'str', 'option': 'bool' } }
>  
>  ##
>  # @block-stream:
> Index: qemu/hmp.c
> ===================================================================
> --- qemu.orig/hmp.c
> +++ qemu/hmp.c
> @@ -836,6 +836,15 @@ void hmp_block_set_io_throttle(Monitor *
>      hmp_handle_error(mon, &err);
>  }
>  
> +void hmp_block_set_hostcache(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +
> +    qmp_block_set_hostcache(qdict_get_str(qdict, "device"),
> +                              qdict_get_bool(qdict, "option"), &err);
> +    hmp_handle_error(mon, &err);
> +}
> +
>  void hmp_block_stream(Monitor *mon, const QDict *qdict)
>  {
>      Error *error = NULL;
> Index: qemu/hmp.h
> ===================================================================
> --- qemu.orig/hmp.h
> +++ qemu/hmp.h
> @@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const 
>  void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> +void hmp_block_set_hostcache(Monitor *mon, const QDict *qdict);
>  
>  #endif
> 
> 

Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change

[Shrinidhi Joshi]

[-- Attachment #1: Type: text/plain, Size: 17166 bytes --]

On Wednesday 20 June 2012 11:48 PM, Jeff Cody wrote:
>  On 06/15/2012 04:47 PM, Supriya Kannery wrote:
> > New command "block_set_hostcache" added for dynamically changing
> > host pagecache setting of a block device.
> >
> > Usage:
> > block_set_hostcache <device> <option>
> > <device> = block device
> > <option> = on/off
> >
> > Example:
> > (qemu) block_set_hostcache ide0-hd0 off
> >
> > Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> >
> > ---
> > block.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > block.h | 2 ++
> > blockdev.c | 26 ++++++++++++++++++++++++++
> > blockdev.h | 2 ++
> > hmp-commands.hx | 14 ++++++++++++++
> > qmp-commands.hx | 27 +++++++++++++++++++++++++++
> > 6 files changed, 125 insertions(+)
> >
> > Index: qemu/block.c
> > ===================================================================
> > --- qemu.orig/block.c
> > +++ qemu/block.c
> > @@ -858,6 +858,35 @@ unlink_and_fail:
> > return ret;
> > }
> >
> > +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
> > +{
> > + BlockDriver *drv = bs->drv;
> > + int ret = 0, open_flags;
> > +
> > + /* Quiesce IO for the given block device */
> > + qemu_aio_flush();
> > + ret = bdrv_flush(bs);
> > + if (ret != 0) {
> > + qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
> > + return ret;
> > + }
> > + open_flags = bs->open_flags;
> > + bdrv_close(bs);
> > +
> > + ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
> > + if (ret < 0) {
> > + /* Reopen failed. Try to open with original flags */
> > + qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> > + ret = bdrv_open(bs, bs->filename, open_flags, drv);
> > + if (ret < 0) {
> > + /* Reopen failed with orig and modified flags */
> > + abort();
> > + }
> > + }
> > +
> > + return ret;
> > +}
> > +
> > void bdrv_close(BlockDriverState *bs)
> > {
> > bdrv_flush(bs);
> > @@ -953,6 +982,34 @@ void bdrv_drain_all(void)
> > }
> > }
> >
> > +int bdrv_change_hostcache(BlockDriverState *bs, bool enable)
> > +{
> > + int bdrv_flags = bs->open_flags;
> > +
> > + /* set hostcache flags (without changing WCE/flush bits) */
> > + if (enable) {
> > + bdrv_flags &= ~BDRV_O_NOCACHE;
> > + } else {
> > + bdrv_flags |= BDRV_O_NOCACHE;
> > + }
> > +
> > + /* If no change in flags, no need to reopen */
> > + if (bdrv_flags == bs->open_flags) {
> > + printf("no need to change\n");
> > + return 0;
> > + }
> > +
> > + if (bdrv_is_inserted(bs)) {
> > + /* Reopen file with changed set of flags */
> > + bdrv_flags &= ~BDRV_O_CACHE_WB;
> > + return bdrv_reopen(bs, bdrv_flags);
> > + } else {
> > + /* Save hostcache change for future use */
> > + bs->open_flags = bdrv_flags;
> > + return 0;
> > + }
> > +}
> > +
> > /* make a BlockDriverState anonymous by removing from bdrv_state list.
> > Also, NULL terminate the device_name to prevent double remove */
> > void bdrv_make_anon(BlockDriverState *bs)
> > Index: qemu/block.h
> > ===================================================================
> > --- qemu.orig/block.h
> > +++ qemu/block.h
> > @@ -128,6 +128,7 @@ int bdrv_parse_cache_flags(const char *m
> > int bdrv_file_open(BlockDriverState **pbs, const char *filename, int 
flags);
> > int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
> > BlockDriver *drv);
> > +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags);
> > void bdrv_close(BlockDriverState *bs);
> > int bdrv_attach_dev(BlockDriverState *bs, void *dev);
> > void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
> > @@ -177,6 +178,7 @@ int bdrv_commit_all(void);
> > int bdrv_change_backing_file(BlockDriverState *bs,
> > const char *backing_file, const char *backing_fmt);
> > void bdrv_register(BlockDriver *bdrv);
> > +int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache);
> >
> >
> > typedef struct BdrvCheckResult {
> > Index: qemu/blockdev.c
> > ===================================================================
> > --- qemu.orig/blockdev.c
> > +++ qemu/blockdev.c
> > @@ -1198,3 +1198,27 @@ BlockJobInfoList *qmp_query_block_jobs(E
> > bdrv_iterate(do_qmp_query_block_jobs_one, &prev);
> > return dummy.next;
> > }
> > +
> > +
> > +/*
> > + * Change host page cache setting while guest is running.
> > +*/
> > +void qmp_block_set_hostcache(const char *device, bool enable,
> > + Error **errp)
> > +{
> > + BlockDriverState *bs = NULL;
> > +
> > + /* Validate device */
> > + bs = bdrv_find(device);
> > + if (!bs) {
> > + error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> > + return;
> > + }
> > +
> > + if (bdrv_change_hostcache(bs, enable)) {
> > + error_set(errp, QERR_HOSTCACHE_NOT_CHANGED, device);
> > + }
> > +
> > + return;
> > +}
> > +
> > Index: qemu/hmp-commands.hx
> > ===================================================================
> > --- qemu.orig/hmp-commands.hx
> > +++ qemu/hmp-commands.hx
> > @@ -808,7 +808,6 @@ ETEXI
> > .mhandler.cmd = hmp_migrate,
> > },
> >
> > -
> > STEXI
> > @item migrate [-d] [-b] [-i] @var{uri}
> > @findex migrate
> > @@ -1314,6 +1313,21 @@ client. @var{keep} changes the password
> > ETEXI
> >
> > {
> > + .name = "block_set_hostcache",
> > + .args_type = "device:B,option:b",
> > + .params = "device on|off",
> > + .help = "Change setting of host pagecache",
> > + .mhandler.cmd = hmp_block_set_hostcache,
> > + },
> > +
> > +STEXI
> > +@item block_set_hostcache @var{device} @var{option}
> > +@findex block_set_hostcache
> > +Change host pagecache setting of a block device while guest is running.
> > +ETEXI
> > +
> > +
> > + {
> > .name = "expire_password",
> > .args_type = "protocol:s,time:s",
> > .params = "protocol time",
> > Index: qemu/qmp-commands.hx
> > ===================================================================
> > --- qemu.orig/qmp-commands.hx
> > +++ qemu/qmp-commands.hx
> > @@ -821,7 +821,31 @@ Example:
> >
> > EQMP
> >
> > +
> > {
> > + .name = "block_set_hostcache",
> > + .args_type = "device:B,option:b",
> > + .mhandler.cmd_new = qmp_block_set_hostcache,
>
>  The QAPI commands need to go through the marshaller - this needs to be:
>  .mhandler.cmd_new = qmp_marshal_input_block_set_hostcache,
>
>  (qmp_marshal_input_block_set_hostcache is automatically generated, and
>  it will call qmp_block_set_hostcache)
>
> > + },
> > +
> > +SQMP
> > +block_set_hostcache
> > +-------------------
> > +
> > +Change host pagecache setting of a block device
> > +
> > +Arguments:
> > +
> > +- "device": the device's ID, must be unique (json-string)
> > +- "option": hostcache setting (json-bool)
> > +
> > +Example:
> > +-> { "execute": "block_set_hostcache", "arguments": { "device": 
"ide0-hd0", "option": false } }
> > +<- { "return": {} }
> > +
> > +EQMP
> > +
> > + {
> > .name = "balloon",
> > .args_type = "value:M",
> > .mhandler.cmd_new = qmp_marshal_input_balloon,
> > Index: qemu/qapi-schema.json
> > ===================================================================
> > --- qemu.orig/qapi-schema.json
> > +++ qemu/qapi-schema.json
> > @@ -1598,6 +1598,22 @@
> > { 'command': 'block_set_io_throttle',
> > 'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 
'int',
> > 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int' } }
> > +##
> > +# @block_set_hostcache:
> > +#
> > +# Change host pagecache setting of a block device
> > +#
> > +# @device: name of the block device
> > +#
> > +# @option: hostcache setting (true/false)
> > +#
> > +# Returns: Nothing on success
> > +# If @device is not a valid block device, DeviceNotFound
> > +#
> > +# Since: 1.2
> > +##
> > +{ 'command': 'block_set_hostcache',
> > + 'data': { 'device': 'str', 'option': 'bool' } }
> >
> > ##
> > # @block-stream:
> > Index: qemu/hmp.c
> > ===================================================================
> > --- qemu.orig/hmp.c
> > +++ qemu/hmp.c
> > @@ -836,6 +836,15 @@ void hmp_block_set_io_throttle(Monitor *
> > hmp_handle_error(mon, &err);
> > }
> >
> > +void hmp_block_set_hostcache(Monitor *mon, const QDict *qdict)
> > +{
> > + Error *err = NULL;
> > +
> > + qmp_block_set_hostcache(qdict_get_str(qdict, "device"),
> > + qdict_get_bool(qdict, "option"), &err);
> > + hmp_handle_error(mon, &err);
> > +}
> > +
> > void hmp_block_stream(Monitor *mon, const QDict *qdict)
> > {
> > Error *error = NULL;
> > Index: qemu/hmp.h
> > ===================================================================
> > --- qemu.orig/hmp.h
> > +++ qemu/hmp.h
> > @@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const
> > void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
> > void hmp_netdev_add(Monitor *mon, const QDict *qdict);
> > void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> > +void hmp_block_set_hostcache(Monitor *mon, const QDict *qdict);
> >
> > #endif
> >
> >
>
Updated patch to auto generate qmp_marshal_input_block_set_hostcache

--------------------------------------------------------------------
New command "block_set_hostcache" added for dynamically changing
host pagecache setting of a block device.

Usage:
  block_set_hostcache <device> <option>
<device> = block device
<option> = on/off

Example:
  (qemu) block_set_hostcache ide0-hd0 off

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

Signed-off-by: Shrinidhi Joshi <spjoshi31@gmail.com>
---
  block.c         |   54 
++++++++++++++++++++++++++++++++++++++++++++++++++++++
  block.h         |    2 ++
  blockdev.c      |   26 ++++++++++++++++++++++++++
  blockdev.h      |    2 ++
  hmp-commands.hx |   14 ++++++++++++++
  qmp-commands.hx |   27 +++++++++++++++++++++++++++
  6 files changed, 125 insertions(+)

Index: qemu/block.c
===================================================================
--- qemu.orig/block.c    2012-06-19 17:24:34.335927717 +0530
+++ qemu/block.c    2012-06-25 09:18:06.940040103 +0530
@@ -859,6 +859,35 @@
      return ret;
  }

+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
+{
+    BlockDriver *drv = bs->drv;
+    int ret = 0, open_flags;
+
+    /* Quiesce IO for the given block device */
+    qemu_aio_flush();
+    ret = bdrv_flush(bs);
+    if (ret != 0) {
+        qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
+        return ret;
+    }
+    open_flags = bs->open_flags;
+    bdrv_close(bs);
+
+    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
+    if (ret < 0) {
+        /* Reopen failed. Try to open with original flags */
+        qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
+        ret = bdrv_open(bs, bs->filename, open_flags, drv);
+        if (ret < 0) {
+            /* Reopen failed with orig and modified flags */
+            abort();
+        }
+    }
+
+    return ret;
+}
+
  void bdrv_close(BlockDriverState *bs)
  {
      bdrv_flush(bs);
@@ -954,6 +983,34 @@
      }
  }

+int bdrv_change_hostcache(BlockDriverState *bs, bool enable)
+{
+    int bdrv_flags = bs->open_flags;
+
+    /* set hostcache flags (without changing WCE/flush bits) */
+    if (enable) {
+        bdrv_flags &= ~BDRV_O_NOCACHE;
+    } else {
+        bdrv_flags |= BDRV_O_NOCACHE;
+    }
+
+    /* If no change in flags, no need to reopen */
+    if (bdrv_flags == bs->open_flags) {
+        printf("no need to change\n");
+        return 0;
+    }
+
+    if (bdrv_is_inserted(bs)) {
+        /* Reopen file with changed set of flags */
+        bdrv_flags &= ~BDRV_O_CACHE_WB;
+        return bdrv_reopen(bs, bdrv_flags);
+    } else {
+        /* Save hostcache change for future use */
+        bs->open_flags = bdrv_flags;
+        return 0;
+    }
+}
+
  /* make a BlockDriverState anonymous by removing from bdrv_state list.
     Also, NULL terminate the device_name to prevent double remove */
  void bdrv_make_anon(BlockDriverState *bs)
Index: qemu/block.h
===================================================================
--- qemu.orig/block.h    2012-06-19 15:52:43.516601040 +0530
+++ qemu/block.h    2012-06-25 09:18:06.912039970 +0530
@@ -128,6 +128,7 @@
  int bdrv_file_open(BlockDriverState **pbs, const char *filename, int 
flags);
  int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
                BlockDriver *drv);
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags);
  void bdrv_close(BlockDriverState *bs);
  int bdrv_attach_dev(BlockDriverState *bs, void *dev);
  void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
@@ -181,6 +182,7 @@
  int bdrv_change_backing_file(BlockDriverState *bs,
      const char *backing_file, const char *backing_fmt);
  void bdrv_register(BlockDriver *bdrv);
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache);


  typedef struct BdrvCheckResult {
Index: qemu/blockdev.c
===================================================================
--- qemu.orig/blockdev.c    2012-06-19 15:52:43.532601120 +0530
+++ qemu/blockdev.c    2012-06-19 17:24:54.124025837 +0530
@@ -1195,3 +1195,27 @@
      bdrv_iterate(do_qmp_query_block_jobs_one, &prev);
      return dummy.next;
  }
+
+
+/*
+ * Change host page cache setting while guest is running.
+*/
+void qmp_block_set_hostcache(const char *device, bool enable,
+                             Error **errp)
+{
+    BlockDriverState *bs = NULL;
+
+    /* Validate device */
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    if (bdrv_change_hostcache(bs, enable)) {
+        error_set(errp, QERR_HOSTCACHE_NOT_CHANGED, device);
+    }
+
+    return;
+}
+
Index: qemu/hmp-commands.hx
===================================================================
--- qemu.orig/hmp-commands.hx    2012-06-19 15:52:43.536601135 +0530
+++ qemu/hmp-commands.hx    2012-06-19 17:24:54.124025837 +0530
@@ -808,7 +808,6 @@
          .mhandler.cmd = hmp_migrate,
      },

-
  STEXI
  @item migrate [-d] [-b] [-i] @var{uri}
  @findex migrate
@@ -1314,6 +1313,21 @@
  ETEXI

      {
+        .name       = "block_set_hostcache",
+        .args_type  = "device:B,option:b",
+        .params     = "device on|off",
+        .help       = "Change setting of host pagecache",
+        .mhandler.cmd = hmp_block_set_hostcache,
+    },
+
+STEXI
+@item block_set_hostcache @var{device} @var{option}
+@findex block_set_hostcache
+Change host pagecache setting of a block device while guest is running.
+ETEXI
+
+
+    {
          .name       = "expire_password",
          .args_type  = "protocol:s,time:s",
          .params     = "protocol time",
Index: qemu/qmp-commands.hx
===================================================================
--- qemu.orig/qmp-commands.hx    2012-06-19 15:52:43.552601213 +0530
+++ qemu/qmp-commands.hx    2012-06-19 18:37:32.557638142 +0530
@@ -821,7 +821,31 @@

  EQMP

+
      {
+        .name       = "block_set_hostcache",
+        .args_type  = "device:B,option:b",
+        .mhandler.cmd_new = qmp_marshal_input_block_set_hostcache,
+    },
+
+SQMP
+block_set_hostcache
+-------------------
+
+Change host pagecache setting of a block device
+
+Arguments:
+
+- "device": the device's ID, must be unique (json-string)
+- "option": hostcache setting (json-bool)
+
+Example:
+-> { "execute": "block_set_hostcache", "arguments": { "device": 
"ide0-hd0", "option": false } }
+<- { "return": {} }
+
+EQMP
+
+    {
          .name       = "balloon",
          .args_type  = "value:M",
          .mhandler.cmd_new = qmp_marshal_input_balloon,
Index: qemu/qapi-schema.json
===================================================================
--- qemu.orig/qapi-schema.json    2012-06-19 17:24:34.335927717 +0530
+++ qemu/qapi-schema.json    2012-06-19 17:24:54.128025861 +0530
@@ -1598,6 +1598,22 @@
  { 'command': 'block_set_io_throttle',
    'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 
'int',
              'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int' } }
+##
+# @block_set_hostcache:
+#
+# Change host pagecache setting of a block device
+#
+# @device: name of the block device
+#
+# @option: hostcache setting (true/false)
+#
+# Returns: Nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#
+# Since: 1.2
+##
+{ 'command': 'block_set_hostcache',
+  'data': { 'device': 'str', 'option': 'bool' } }

  ##
  # @block-stream:
Index: qemu/hmp.c
===================================================================
--- qemu.orig/hmp.c    2012-06-19 17:24:34.335927717 +0530
+++ qemu/hmp.c    2012-06-19 17:24:54.128025861 +0530
@@ -837,6 +837,15 @@
      hmp_handle_error(mon, &err);
  }

+void hmp_block_set_hostcache(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+
+    qmp_block_set_hostcache(qdict_get_str(qdict, "device"),
+                              qdict_get_bool(qdict, "option"), &err);
+    hmp_handle_error(mon, &err);
+}
+
  void hmp_block_stream(Monitor *mon, const QDict *qdict)
  {
      Error *error = NULL;
Index: qemu/hmp.h
===================================================================
--- qemu.orig/hmp.h    2012-06-19 15:52:43.500600959 +0530
+++ qemu/hmp.h    2012-06-19 17:24:54.128025861 +0530
@@ -64,5 +64,6 @@
  void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
  void hmp_netdev_add(Monitor *mon, const QDict *qdict);
  void hmp_netdev_del(Monitor *mon, const QDict *qdict);
+void hmp_block_set_hostcache(Monitor *mon, const QDict *qdict);

  #endif




[-- Attachment #2: Type: text/html, Size: 28188 bytes --]

Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change

[Kevin Wolf]

Am 04.07.2012 07:10, schrieb Shrinidhi Joshi:
> Updated patch to auto generate qmp_marshal_input_block_set_hostcache
> 
> --------------------------------------------------------------------
> New command "block_set_hostcache" added for dynamically changing
> host pagecache setting of a block device.
> 
> Usage:
>  block_set_hostcache  <device> <option>
>    <device> = block device
>    <option> = on/off
> 
> Example:
>  (qemu) block_set_hostcache ide0-hd0 off
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 
> Signed-off-by: Shrinidhi Joshi <spjoshi31@gmail.com>

Please use git-send-email for sending patches. A line-wrapped
HTML-formatted patch like this cannot be applied and is useless.

Kevin

Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change

[Kevin Wolf]

Am 15.06.2012 22:47, schrieb Supriya Kannery:
> New command "block_set_hostcache" added for dynamically changing 
> host pagecache setting of a block device.
> 
> Usage: 
>  block_set_hostcache  <device> <option>
>    <device> = block device
>    <option> = on/off
> 
> Example:
>  (qemu) block_set_hostcache ide0-hd0 off
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 
> ---
>  block.c         |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block.h         |    2 ++
>  blockdev.c      |   26 ++++++++++++++++++++++++++
>  blockdev.h      |    2 ++
>  hmp-commands.hx |   14 ++++++++++++++
>  qmp-commands.hx |   27 +++++++++++++++++++++++++++
>  6 files changed, 125 insertions(+)
> 
> Index: qemu/block.c
> ===================================================================
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -858,6 +858,35 @@ unlink_and_fail:
>      return ret;
>  }
>  
> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
> +{
> +    BlockDriver *drv = bs->drv;
> +    int ret = 0, open_flags;
> +
> +    /* Quiesce IO for the given block device */
> +    qemu_aio_flush();

bdrv_drain_all() should be used instead.

> +    ret = bdrv_flush(bs);
> +    if (ret != 0) {
> +        qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
> +        return ret;
> +    }
> +    open_flags = bs->open_flags;
> +    bdrv_close(bs);
> +
> +    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
> +    if (ret < 0) {
> +        /* Reopen failed. Try to open with original flags */
> +        qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> +        ret = bdrv_open(bs, bs->filename, open_flags, drv);
> +        if (ret < 0) {
> +            /* Reopen failed with orig and modified flags */
> +            abort();

Like Eric said, committing a broken version and then fixing it later in
the series isn't really nice. At least bs->drv = NULL; instead of
abort() is required. Starting with the real thing is probably even better.

Kevin

Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change

[Luiz Capitulino]

On Sat, 16 Jun 2012 02:17:30 +0530
Supriya Kannery <supriyak@linux.vnet.ibm.com> wrote:

> New command "block_set_hostcache" added for dynamically changing 
> host pagecache setting of a block device.
> 
> Usage: 
>  block_set_hostcache  <device> <option>
>    <device> = block device
>    <option> = on/off
> 
> Example:
>  (qemu) block_set_hostcache ide0-hd0 off
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 
> ---
>  block.c         |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  block.h         |    2 ++
>  blockdev.c      |   26 ++++++++++++++++++++++++++
>  blockdev.h      |    2 ++
>  hmp-commands.hx |   14 ++++++++++++++
>  qmp-commands.hx |   27 +++++++++++++++++++++++++++
>  6 files changed, 125 insertions(+)
> 
> Index: qemu/block.c
> ===================================================================
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -858,6 +858,35 @@ unlink_and_fail:
>      return ret;
>  }
>  
> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
> +{
> +    BlockDriver *drv = bs->drv;
> +    int ret = 0, open_flags;
> +
> +    /* Quiesce IO for the given block device */
> +    qemu_aio_flush();
> +    ret = bdrv_flush(bs);
> +    if (ret != 0) {
> +        qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);

Please, use error_set() instead of qerror_report() and as Kevin said IOError
seems enough here.

> +        return ret;
> +    }
> +    open_flags = bs->open_flags;
> +    bdrv_close(bs);
> +
> +    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
> +    if (ret < 0) {
> +        /* Reopen failed. Try to open with original flags */
> +        qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> +        ret = bdrv_open(bs, bs->filename, open_flags, drv);
> +        if (ret < 0) {
> +            /* Reopen failed with orig and modified flags */
> +            abort();
> +        }
> +    }
> +
> +    return ret;
> +}
> +
>  void bdrv_close(BlockDriverState *bs)
>  {
>      bdrv_flush(bs);
> @@ -953,6 +982,34 @@ void bdrv_drain_all(void)
>      }
>  }
>  
> +int bdrv_change_hostcache(BlockDriverState *bs, bool enable)
> +{
> +    int bdrv_flags = bs->open_flags;
> +
> +    /* set hostcache flags (without changing WCE/flush bits) */
> +    if (enable) {
> +        bdrv_flags &= ~BDRV_O_NOCACHE;
> +    } else {
> +        bdrv_flags |= BDRV_O_NOCACHE;
> +    }
> +
> +    /* If no change in flags, no need to reopen */
> +    if (bdrv_flags == bs->open_flags) {
> +        printf("no need to change\n");
> +        return 0;
> +    }
> +
> +    if (bdrv_is_inserted(bs)) {
> +        /* Reopen file with changed set of flags */
> +        bdrv_flags &= ~BDRV_O_CACHE_WB;
> +        return bdrv_reopen(bs, bdrv_flags);
> +    } else {
> +        /* Save hostcache change for future use */
> +        bs->open_flags = bdrv_flags;
> +        return 0;
> +    }
> +}
> +
>  /* make a BlockDriverState anonymous by removing from bdrv_state list.
>     Also, NULL terminate the device_name to prevent double remove */
>  void bdrv_make_anon(BlockDriverState *bs)
> Index: qemu/block.h
> ===================================================================
> --- qemu.orig/block.h
> +++ qemu/block.h
> @@ -128,6 +128,7 @@ int bdrv_parse_cache_flags(const char *m
>  int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
>  int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
>                BlockDriver *drv);
> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags);
>  void bdrv_close(BlockDriverState *bs);
>  int bdrv_attach_dev(BlockDriverState *bs, void *dev);
>  void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
> @@ -177,6 +178,7 @@ int bdrv_commit_all(void);
>  int bdrv_change_backing_file(BlockDriverState *bs,
>      const char *backing_file, const char *backing_fmt);
>  void bdrv_register(BlockDriver *bdrv);
> +int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache);
>  
>  
>  typedef struct BdrvCheckResult {
> Index: qemu/blockdev.c
> ===================================================================
> --- qemu.orig/blockdev.c
> +++ qemu/blockdev.c
> @@ -1198,3 +1198,27 @@ BlockJobInfoList *qmp_query_block_jobs(E
>      bdrv_iterate(do_qmp_query_block_jobs_one, &prev);
>      return dummy.next;
>  }
> +
> +
> +/*
> + * Change host page cache setting while guest is running.
> +*/
> +void qmp_block_set_hostcache(const char *device, bool enable,
> +                             Error **errp)
> +{
> +    BlockDriverState *bs = NULL;
> +
> +    /* Validate device */
> +    bs = bdrv_find(device);
> +    if (!bs) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +        return;
> +    }
> +
> +    if (bdrv_change_hostcache(bs, enable)) {
> +        error_set(errp, QERR_HOSTCACHE_NOT_CHANGED, device);
> +    }
> +
> +    return;
> +}
> +
> Index: qemu/hmp-commands.hx
> ===================================================================
> --- qemu.orig/hmp-commands.hx
> +++ qemu/hmp-commands.hx
> @@ -808,7 +808,6 @@ ETEXI
>          .mhandler.cmd = hmp_migrate,
>      },
>  
> -
>  STEXI
>  @item migrate [-d] [-b] [-i] @var{uri}
>  @findex migrate
> @@ -1314,6 +1313,21 @@ client.  @var{keep} changes the password
>  ETEXI
>  
>      {
> +        .name       = "block_set_hostcache",
> +        .args_type  = "device:B,option:b",
> +        .params     = "device on|off",
> +        .help       = "Change setting of host pagecache",
> +        .mhandler.cmd = hmp_block_set_hostcache,
> +    },
> +
> +STEXI
> +@item block_set_hostcache @var{device} @var{option}
> +@findex block_set_hostcache
> +Change host pagecache setting of a block device while guest is running.
> +ETEXI
> +
> +
> +    {
>          .name       = "expire_password",
>          .args_type  = "protocol:s,time:s",
>          .params     = "protocol time",
> Index: qemu/qmp-commands.hx
> ===================================================================
> --- qemu.orig/qmp-commands.hx
> +++ qemu/qmp-commands.hx
> @@ -821,7 +821,31 @@ Example:
>  
>  EQMP
>  
> +
>      {
> +        .name       = "block_set_hostcache",
> +        .args_type  = "device:B,option:b",
> +        .mhandler.cmd_new = qmp_block_set_hostcache,
> +    },
> +
> +SQMP
> +block_set_hostcache
> +-------------------
> +
> +Change host pagecache setting of a block device
> +
> +Arguments:
> +
> +- "device": the device's ID, must be unique (json-string)
> +- "option": hostcache setting (json-bool)
> +
> +Example:
> +-> { "execute": "block_set_hostcache", "arguments": { "device": "ide0-hd0", "option": false } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +	{
>          .name       = "balloon",
>          .args_type  = "value:M",
>          .mhandler.cmd_new = qmp_marshal_input_balloon,
> Index: qemu/qapi-schema.json
> ===================================================================
> --- qemu.orig/qapi-schema.json
> +++ qemu/qapi-schema.json
> @@ -1598,6 +1598,22 @@
>  { 'command': 'block_set_io_throttle',
>    'data': { 'device': 'str', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
>              'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int' } }
> +##
> +# @block_set_hostcache:
> +#
> +# Change host pagecache setting of a block device
> +#
> +# @device: name of the block device
> +#
> +# @option: hostcache setting (true/false)
> +#
> +# Returns: Nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#
> +# Since: 1.2
> +##
> +{ 'command': 'block_set_hostcache',
> +  'data': { 'device': 'str', 'option': 'bool' } }
>  
>  ##
>  # @block-stream:
> Index: qemu/hmp.c
> ===================================================================
> --- qemu.orig/hmp.c
> +++ qemu/hmp.c
> @@ -836,6 +836,15 @@ void hmp_block_set_io_throttle(Monitor *
>      hmp_handle_error(mon, &err);
>  }
>  
> +void hmp_block_set_hostcache(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +
> +    qmp_block_set_hostcache(qdict_get_str(qdict, "device"),
> +                              qdict_get_bool(qdict, "option"), &err);
> +    hmp_handle_error(mon, &err);
> +}
> +
>  void hmp_block_stream(Monitor *mon, const QDict *qdict)
>  {
>      Error *error = NULL;
> Index: qemu/hmp.h
> ===================================================================
> --- qemu.orig/hmp.h
> +++ qemu/hmp.h
> @@ -64,5 +64,6 @@ void hmp_device_del(Monitor *mon, const 
>  void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>  void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> +void hmp_block_set_hostcache(Monitor *mon, const QDict *qdict);
>  
>  #endif
> 

Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change

[Supriya Kannery]

On 07/11/2012 07:46 PM, Luiz Capitulino wrote:
> On Sat, 16 Jun 2012 02:17:30 +0530
> Supriya Kannery<supriyak@linux.vnet.ibm.com>  wrote:
>
>> New command "block_set_hostcache" added for dynamically changing
>> host pagecache setting of a block device.
>>
>> Usage:
>>   block_set_hostcache<device>  <option>
>>     <device>  = block device
>>     <option>  = on/off
>>
>> Example:
>>   (qemu) block_set_hostcache ide0-hd0 off
>>
>> Signed-off-by: Supriya Kannery<supriyak@linux.vnet.ibm.com>
>>
>> ---
>>   block.c         |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   block.h         |    2 ++
>>   blockdev.c      |   26 ++++++++++++++++++++++++++
>>   blockdev.h      |    2 ++
>>   hmp-commands.hx |   14 ++++++++++++++
>>   qmp-commands.hx |   27 +++++++++++++++++++++++++++
>>   6 files changed, 125 insertions(+)
>>
>> Index: qemu/block.c
>> ===================================================================
>> --- qemu.orig/block.c
>> +++ qemu/block.c
>> @@ -858,6 +858,35 @@ unlink_and_fail:
>>       return ret;
>>   }
>>
>> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
>> +{
>> +    BlockDriver *drv = bs->drv;
>> +    int ret = 0, open_flags;
>> +
>> +    /* Quiesce IO for the given block device */
>> +    qemu_aio_flush();
>> +    ret = bdrv_flush(bs);
>> +    if (ret != 0) {
>> +        qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
>
> Please, use error_set() instead of qerror_report() and as Kevin said IOError
> seems enough here.
>

Sure, will replace qemu_aio_flush() with bdrv_drain_all()
and qerror_report() with error_set().

Combination of bdrv_drain_all() and bdrv_flush(), should
help in flushing out the specific disk.

-thanks, Supriya

[Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely

[Supriya Kannery]

Struct BDRVReopenState along with three reopen related functions
introduced for handling reopening of images safely. This can be
extended by each of the block drivers to reopen respective
image files.

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

Index: qemu/block.c
===================================================================
--- qemu.orig/block.c
+++ qemu/block.c
@@ -858,10 +858,32 @@ unlink_and_fail:
     return ret;
 }
 
+int bdrv_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs, int flags)
+{
+     BlockDriver *drv = bs->drv;
+
+     return drv->bdrv_reopen_prepare(bs, prs, flags);
+}
+
+void bdrv_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BlockDriver *drv = bs->drv;
+
+    drv->bdrv_reopen_commit(bs, rs);
+}
+
+void bdrv_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BlockDriver *drv = bs->drv;
+
+    drv->bdrv_reopen_abort(bs, rs);
+}
+
 int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
 {
     BlockDriver *drv = bs->drv;
     int ret = 0, open_flags;
+    BDRVReopenState *reopen_state = NULL;
 
     /* Quiesce IO for the given block device */
     qemu_aio_flush();
@@ -870,17 +892,44 @@ int bdrv_reopen(BlockDriverState *bs, in
         qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
         return ret;
     }
-    open_flags = bs->open_flags;
-    bdrv_close(bs);
 
-    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
-    if (ret < 0) {
-        /* Reopen failed. Try to open with original flags */
-        qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
-        ret = bdrv_open(bs, bs->filename, open_flags, drv);
-        if (ret < 0) {
-            /* Reopen failed with orig and modified flags */
-            abort();
+    /* Use driver specific reopen() if available */
+    if (drv->bdrv_reopen_prepare) {
+        ret = bdrv_reopen_prepare(bs, &reopen_state, bdrv_flags);
+         if (ret < 0) {
+            bdrv_reopen_abort(bs, reopen_state);
+            qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
+            return ret;
+        }
+
+        bdrv_reopen_commit(bs, reopen_state);
+        bs->open_flags = bdrv_flags;
+
+    } else {
+
+        /* Try reopening the image using protocol or directly */
+        if (bs->file) {
+            open_flags = bs->open_flags;
+            drv->bdrv_close(bs);
+            if (drv->bdrv_file_open) {
+                ret = drv->bdrv_file_open(bs, bs->filename, bdrv_flags);
+            } else {
+                ret = bdrv_file_open(&bs->file, bs->filename, bdrv_flags);
+            }
+            if (ret < 0) {
+                /* Reopen failed. Try to open with original flags */
+                qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
+                if (drv->bdrv_file_open) {
+                    ret = drv->bdrv_file_open(bs, bs->filename, open_flags);
+                } else {
+                    ret = bdrv_file_open(&bs->file, bs->filename, open_flags);
+                }
+                if (ret < 0) {
+                    /* Reopen failed with orig and modified flags */
+                    bdrv_delete(bs->file);
+                    bs->drv = NULL;
+                }
+            }
         }
     }
 
Index: qemu/block_int.h
===================================================================
--- qemu.orig/block_int.h
+++ qemu/block_int.h
@@ -137,6 +137,13 @@ struct BlockDriver {
     int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
     int (*bdrv_probe_device)(const char *filename);
     int (*bdrv_open)(BlockDriverState *bs, int flags);
+
+    /* For handling image reopen for split or non-split files */
+    int (*bdrv_reopen_prepare)(BlockDriverState *bs,
+                               BDRVReopenState **prs,
+                               int flags);
+    void (*bdrv_reopen_commit)(BlockDriverState *bs, BDRVReopenState *rs);
+    void (*bdrv_reopen_abort)(BlockDriverState *bs, BDRVReopenState *rs);
     int (*bdrv_file_open)(BlockDriverState *bs, const char *filename, int flags);
     int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
                      uint8_t *buf, int nb_sectors);
@@ -335,6 +342,10 @@ struct BlockDriverState {
     BlockJob *job;
 };
 
+struct BDRVReopenState {
+    BlockDriverState *bs;
+};
+
 int get_tmp_filename(char *filename, int size);
 
 void bdrv_set_io_limits(BlockDriverState *bs,
Index: qemu/qemu-common.h
===================================================================
--- qemu.orig/qemu-common.h
+++ qemu/qemu-common.h
@@ -225,6 +225,7 @@ typedef struct NICInfo NICInfo;
 typedef struct HCIInfo HCIInfo;
 typedef struct AudioState AudioState;
 typedef struct BlockDriverState BlockDriverState;
+typedef struct BDRVReopenState BDRVReopenState;
 typedef struct DriveInfo DriveInfo;
 typedef struct DisplayState DisplayState;
 typedef struct DisplayChangeListener DisplayChangeListener;
Index: qemu/block.h
===================================================================
--- qemu.orig/block.h
+++ qemu/block.h
@@ -129,6 +129,9 @@ int bdrv_file_open(BlockDriverState **pb
 int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
               BlockDriver *drv);
 int bdrv_reopen(BlockDriverState *bs, int bdrv_flags);
+int bdrv_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs, int flags);
+void bdrv_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs);
+void bdrv_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs);
 void bdrv_close(BlockDriverState *bs);
 int bdrv_attach_dev(BlockDriverState *bs, void *dev);
 void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);

Re: [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2361 bytes --]

On 06/15/2012 02:47 PM, Supriya Kannery wrote:
> Struct BDRVReopenState along with three reopen related functions
> introduced for handling reopening of images safely. This can be
> extended by each of the block drivers to reopen respective
> image files.
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 

> +    /* Use driver specific reopen() if available */
> +    if (drv->bdrv_reopen_prepare) {
> +        ret = bdrv_reopen_prepare(bs, &reopen_state, bdrv_flags);
> +         if (ret < 0) {
> +            bdrv_reopen_abort(bs, reopen_state);
> +            qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> +            return ret;
> +        }
> +
> +        bdrv_reopen_commit(bs, reopen_state);
> +        bs->open_flags = bdrv_flags;

Good.

> +
> +    } else {
> +
> +        /* Try reopening the image using protocol or directly */
> +        if (bs->file) {
> +            open_flags = bs->open_flags;
> +            drv->bdrv_close(bs);
> +            if (drv->bdrv_file_open) {
> +                ret = drv->bdrv_file_open(bs, bs->filename, bdrv_flags);

Not so good.  Are we sure we can't flush all data, so that we can then
attempt the open before the close, and avoid losing the original file on
error?

> +            } else {
> +                ret = bdrv_file_open(&bs->file, bs->filename, bdrv_flags);
> +            }
> +            if (ret < 0) {
> +                /* Reopen failed. Try to open with original flags */
> +                qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> +                if (drv->bdrv_file_open) {
> +                    ret = drv->bdrv_file_open(bs, bs->filename, open_flags);
> +                } else {
> +                    ret = bdrv_file_open(&bs->file, bs->filename, open_flags);
> +                }
> +                if (ret < 0) {
> +                    /* Reopen failed with orig and modified flags */
> +                    bdrv_delete(bs->file);
> +                    bs->drv = NULL;

You dropped the abort() here, but now the device is silently gone.  But
since the error of QERR_OPEN_FILE_FAILED is the same as in the safe
case, how does the management app know that things were corrupted?

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely

[Kevin Wolf]

Am 15.06.2012 22:47, schrieb Supriya Kannery:
> Struct BDRVReopenState along with three reopen related functions
> introduced for handling reopening of images safely. This can be
> extended by each of the block drivers to reopen respective
> image files.
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 
> Index: qemu/block.c
> ===================================================================
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -858,10 +858,32 @@ unlink_and_fail:
>      return ret;
>  }
>  
> +int bdrv_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs, int flags)
> +{
> +     BlockDriver *drv = bs->drv;
> +
> +     return drv->bdrv_reopen_prepare(bs, prs, flags);
> +}
> +
> +void bdrv_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
> +{
> +    BlockDriver *drv = bs->drv;
> +
> +    drv->bdrv_reopen_commit(bs, rs);
> +}
> +
> +void bdrv_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
> +{
> +    BlockDriver *drv = bs->drv;
> +
> +    drv->bdrv_reopen_abort(bs, rs);
> +}
> +
>  int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
>  {
>      BlockDriver *drv = bs->drv;
>      int ret = 0, open_flags;
> +    BDRVReopenState *reopen_state = NULL;
>  
>      /* Quiesce IO for the given block device */
>      qemu_aio_flush();
> @@ -870,17 +892,44 @@ int bdrv_reopen(BlockDriverState *bs, in
>          qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
>          return ret;
>      }
> -    open_flags = bs->open_flags;
> -    bdrv_close(bs);
>  
> -    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
> -    if (ret < 0) {
> -        /* Reopen failed. Try to open with original flags */
> -        qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> -        ret = bdrv_open(bs, bs->filename, open_flags, drv);
> -        if (ret < 0) {
> -            /* Reopen failed with orig and modified flags */
> -            abort();
> +    /* Use driver specific reopen() if available */
> +    if (drv->bdrv_reopen_prepare) {
> +        ret = bdrv_reopen_prepare(bs, &reopen_state, bdrv_flags);
> +         if (ret < 0) {
> +            bdrv_reopen_abort(bs, reopen_state);
> +            qerror_report(QERR_OPEN_FILE_FAILED, bs->filename);
> +            return ret;
> +        }
> +
> +        bdrv_reopen_commit(bs, reopen_state);
> +        bs->open_flags = bdrv_flags;
> +
> +    } else {
> +
> +        /* Try reopening the image using protocol or directly */
> +        if (bs->file) {
> +            open_flags = bs->open_flags;
> +            drv->bdrv_close(bs);
> +            if (drv->bdrv_file_open) {
> +                ret = drv->bdrv_file_open(bs, bs->filename, bdrv_flags);
> +            } else {
> +                ret = bdrv_file_open(&bs->file, bs->filename, bdrv_flags);

Doesn't this forget to reopen bs itself? And bs->file wasn't even
closed. If think we need something more along the lines of:

if (bs->file) {
    bdrv_reopen(bs->file)
}

if (drv->bdrv_file_open) {
    drv->bdrv_file_open(bs)
} else {
   drv->bdrv_open(bs)
}

In fact we would really want to be able to commit/abort the bdrv_reopen
of bs->file only after we know if bdrv_open(bs) has succeeded, but with
the current design we can't because bdrv_open wants to read from the new fd.

Maybe it would make sense to require bdrv_reopen_prepare() to do the
switch without throwing the old state away yet, but it sounds as if it
would make implementations for image formats quite a bit harder.

Maybe we should completely avoid this default implementation and just
fail bdrv_reopen if a driver doesn't support the explicit,
transactionable reopen functions.

Kevin

[Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen

[Supriya Kannery]

raw-posix driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while 
changing hostcache dynamically is handled here.

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

Index: qemu/block/raw.c
===================================================================
--- qemu.orig/block/raw.c
+++ qemu/block/raw.c
@@ -9,6 +9,22 @@ static int raw_open(BlockDriverState *bs
     return 0;
 }
 
+static int raw_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+                              int flags)
+{
+    return bdrv_reopen_prepare(bs->file, prs, flags);
+}
+
+static void raw_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    bdrv_reopen_commit(bs->file, rs);
+}
+
+static void raw_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    bdrv_reopen_abort(bs->file, rs);
+}
+
 static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
                                      int nb_sectors, QEMUIOVector *qiov)
 {
@@ -104,6 +120,10 @@ static BlockDriver bdrv_raw = {
     .instance_size      = 1,
 
     .bdrv_open          = raw_open,
+    .bdrv_reopen_prepare
+                        = raw_reopen_prepare,
+    .bdrv_reopen_commit = raw_reopen_commit,
+    .bdrv_reopen_abort  = raw_reopen_abort,
     .bdrv_close         = raw_close,
 
     .bdrv_co_readv          = raw_co_readv,
Index: qemu/block/raw-posix.c
===================================================================
--- qemu.orig/block/raw-posix.c
+++ qemu/block/raw-posix.c
@@ -136,8 +136,15 @@ typedef struct BDRVRawState {
 #endif
 } BDRVRawState;
 
+typedef struct BDRVRawReopenState {
+    BDRVReopenState reopen_state;
+    BDRVRawState *stash_s;
+} BDRVRawReopenState;
+
 static int fd_open(BlockDriverState *bs);
 static int64_t raw_getlength(BlockDriverState *bs);
+static void raw_stash_state(BDRVRawState *stashed_state, BDRVRawState *s);
+static void raw_revert_state(BDRVRawState *s, BDRVRawState *stashed_state);
 
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
 static int cdrom_reopen(BlockDriverState *bs);
@@ -279,6 +286,119 @@ static int raw_open(BlockDriverState *bs
     return raw_open_common(bs, filename, flags, 0);
 }
 
+static int raw_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+                              int flags)
+{
+    BDRVRawReopenState *raw_rs = g_malloc0(sizeof(BDRVRawReopenState));
+    BDRVRawState *s = bs->opaque;
+    int ret = 0;
+
+    raw_rs->reopen_state.bs = bs;
+
+    /* stash state before reopen */
+    raw_rs->stash_s = g_malloc0(sizeof(BDRVRawState));
+/*    memcpy(raw_rs->stash_s, s, sizeof(BDRVRawState)); */
+    raw_stash_state(raw_rs->stash_s, s);
+    s->fd = dup(raw_rs->stash_s->fd);
+
+    *prs = &(raw_rs->reopen_state);
+
+    /* Flags that can be set using fcntl */
+    int fcntl_flags = BDRV_O_NOCACHE;
+
+    if ((bs->open_flags & ~fcntl_flags) == (flags & ~fcntl_flags)) {
+        if ((flags & BDRV_O_NOCACHE)) {
+            s->open_flags |= O_DIRECT;
+        } else {
+            s->open_flags &= ~O_DIRECT;
+        }
+        ret = fcntl_setfl(s->fd, s->open_flags);
+    } else {
+
+        /* close and reopen using new flags */
+        bs->drv->bdrv_close(bs);
+        ret = bs->drv->bdrv_file_open(bs, bs->filename, flags);
+    }
+    return ret;
+}
+
+static void raw_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVRawReopenState *raw_rs;
+
+    raw_rs = container_of(rs, BDRVRawReopenState, reopen_state);
+
+    /* clean up stashed state */
+    close(raw_rs->stash_s->fd);
+    g_free(raw_rs->stash_s);
+    g_free(raw_rs);
+}
+
+static void raw_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVRawReopenState *raw_rs;
+    BDRVRawState *s = bs->opaque;
+
+    raw_rs = container_of(rs, BDRVRawReopenState, reopen_state);
+
+    /* revert to stashed state */
+    if (s->fd != -1) {
+        close(s->fd);
+    }
+/*   memcpy(s, raw_rs->stash_s, sizeof(BDRVRawState)); */
+    raw_revert_state(s, raw_rs->stash_s);
+    g_free(raw_rs->stash_s);
+    g_free(raw_rs);
+}
+
+static void raw_stash_state(BDRVRawState *stashed_s, BDRVRawState *s)
+{
+    stashed_s->fd = -1;
+    stashed_s->type = s->type;
+    stashed_s->open_flags = s->open_flags;
+#if defined(__linux__)
+    /* linux floppy specific */
+    stashed_s->fd_open_time = s->fd_open_time;
+    stashed_s->fd_error_time = s->fd_error_time;
+    stashed_s->fd_got_error = s->fd_got_error;
+    stashed_s->fd_media_changed = s->fd_media_changed;
+#endif
+#ifdef CONFIG_LINUX_AIO
+    stashed_s->use_aio = s->use_aio;
+    stashed_s->aio_ctx = s->aio_ctx;
+#endif
+    stashed_s->aligned_buf = s->aligned_buf;
+    stashed_s->aligned_buf_size = s->aligned_buf_size;
+#ifdef CONFIG_XFS
+    stashed_s->is_xfs = s->is_xfs;
+#endif
+
+}
+
+static void raw_revert_state(BDRVRawState *s, BDRVRawState *stashed_s)
+{
+
+    s->fd = stashed_s->fd;
+    s->type = stashed_s->type;
+    s->open_flags = stashed_s->open_flags;
+#if defined(__linux__)
+    /* linux floppy specific */
+    s->fd_open_time = stashed_s->fd_open_time;
+    s->fd_error_time = stashed_s->fd_error_time;
+    s->fd_got_error = stashed_s->fd_got_error;
+    s->fd_media_changed = stashed_s->fd_media_changed;
+#endif
+#ifdef CONFIG_LINUX_AIO
+    s->use_aio = stashed_s->use_aio;
+    s->aio_ctx = stashed_s->aio_ctx;
+#endif
+    s->aligned_buf = stashed_s->aligned_buf;
+    s->aligned_buf_size = stashed_s->aligned_buf_size;
+#ifdef CONFIG_XFS
+    s->is_xfs = stashed_s->is_xfs;
+#endif
+}
+
 /* XXX: use host sector size if necessary with:
 #ifdef DIOCGSECTORSIZE
         {
@@ -631,6 +751,9 @@ static BlockDriver bdrv_file = {
     .instance_size = sizeof(BDRVRawState),
     .bdrv_probe = NULL, /* no probe for protocols */
     .bdrv_file_open = raw_open,
+    .bdrv_reopen_prepare = raw_reopen_prepare,
+    .bdrv_reopen_commit = raw_reopen_commit,
+    .bdrv_reopen_abort = raw_reopen_abort,
     .bdrv_close = raw_close,
     .bdrv_create = raw_create,
     .bdrv_co_discard = raw_co_discard,

Re: [Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2040 bytes --]

On 06/15/2012 02:48 PM, Supriya Kannery wrote:
> raw-posix driver changes for bdrv_reopen_xx functions to
> safely reopen image files. Reopening of image files while 
> changing hostcache dynamically is handled here.
> 
> Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> 

>  
> +static int raw_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
> +                              int flags)
> +{
> +    BDRVRawReopenState *raw_rs = g_malloc0(sizeof(BDRVRawReopenState));
> +    BDRVRawState *s = bs->opaque;
> +    int ret = 0;
> +
> +    raw_rs->reopen_state.bs = bs;
> +
> +    /* stash state before reopen */
> +    raw_rs->stash_s = g_malloc0(sizeof(BDRVRawState));
> +/*    memcpy(raw_rs->stash_s, s, sizeof(BDRVRawState)); */

Why the comment?

> +    raw_stash_state(raw_rs->stash_s, s);
> +    s->fd = dup(raw_rs->stash_s->fd);

Needs to handle O_CLOEXEC open flag, which means using
fcntl(F_DUPFD_CLOEXEC) when available for atomic support, and a fallback
to fcntl(F_GETFD/F_SETFD) if not.

> +
> +    *prs = &(raw_rs->reopen_state);
> +
> +    /* Flags that can be set using fcntl */
> +    int fcntl_flags = BDRV_O_NOCACHE;
> +
> +    if ((bs->open_flags & ~fcntl_flags) == (flags & ~fcntl_flags)) {
> +        if ((flags & BDRV_O_NOCACHE)) {
> +            s->open_flags |= O_DIRECT;
> +        } else {
> +            s->open_flags &= ~O_DIRECT;
> +        }
> +        ret = fcntl_setfl(s->fd, s->open_flags);
> +    } else {
> +
> +        /* close and reopen using new flags */
> +        bs->drv->bdrv_close(bs);
> +        ret = bs->drv->bdrv_file_open(bs, bs->filename, flags);

Is this a case where Paolo's proposed bdrv_swap command would be useful?

> +    /* revert to stashed state */
> +    if (s->fd != -1) {
> +        close(s->fd);
> +    }
> +/*   memcpy(s, raw_rs->stash_s, sizeof(BDRVRawState)); */

Why the comment?

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen

[Shrinidhi Joshi]

[-- Attachment #1: Type: text/plain, Size: 7949 bytes --]

On Saturday 16 June 2012 03:41 AM, Eric Blake wrote:
>  On 06/15/2012 02:48 PM, Supriya Kannery wrote:
> > raw-posix driver changes for bdrv_reopen_xx functions to
> > safely reopen image files. Reopening of image files while
> > changing hostcache dynamically is handled here.
> >
> > Signed-off-by: Supriya Kannery <supriyak@linux.vnet.ibm.com>
> >
>
> >
> > +static int raw_reopen_prepare(BlockDriverState *bs, BDRVReopenState 
**prs,
> > + int flags)
> > +{
> > + BDRVRawReopenState *raw_rs = g_malloc0(sizeof(BDRVRawReopenState));
> > + BDRVRawState *s = bs->opaque;
> > + int ret = 0;
> > +
> > + raw_rs->reopen_state.bs = bs;
> > +
> > + /* stash state before reopen */
> > + raw_rs->stash_s = g_malloc0(sizeof(BDRVRawState));
> > +/* memcpy(raw_rs->stash_s, s, sizeof(BDRVRawState)); */
>
>  Why the comment?
>
> > + raw_stash_state(raw_rs->stash_s, s);
> > + s->fd = dup(raw_rs->stash_s->fd);
>
>  Needs to handle O_CLOEXEC open flag, which means using
>  fcntl(F_DUPFD_CLOEXEC) when available for atomic support, and a fallback
>  to fcntl(F_GETFD/F_SETFD) if not.
>
> > +
> > + *prs = &(raw_rs->reopen_state);
> > +
> > + /* Flags that can be set using fcntl */
> > + int fcntl_flags = BDRV_O_NOCACHE;
> > +
> > + if ((bs->open_flags & ~fcntl_flags) == (flags & ~fcntl_flags)) {
> > + if ((flags & BDRV_O_NOCACHE)) {
> > + s->open_flags |= O_DIRECT;
> > + } else {
> > + s->open_flags &= ~O_DIRECT;
> > + }
> > + ret = fcntl_setfl(s->fd, s->open_flags);
> > + } else {
> > +
> > + /* close and reopen using new flags */
> > + bs->drv->bdrv_close(bs);
> > + ret = bs->drv->bdrv_file_open(bs, bs->filename, flags);
>
>  Is this a case where Paolo's proposed bdrv_swap command would be useful?
>
> > + /* revert to stashed state */
> > + if (s->fd != -1) {
> > + close(s->fd);
> > + }
> > +/* memcpy(s, raw_rs->stash_s, sizeof(BDRVRawState)); */
>
>  Why the comment?
>
raw-posix driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while
changing hostcache dynamically is handled here.

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

Changed the dup function to dup3 to handle the O_CLOEXEC flag.

Signed-off-by: Shrinidhi Joshi <spjoshi31@gmail.com>

Index: qemu/block/raw.c
===================================================================
--- qemu.orig/block/raw.c    2012-06-19 15:52:40.512586150 +0530
+++ qemu/block/raw.c    2012-06-19 17:39:33.572386768 +0530
@@ -9,6 +9,22 @@
      return 0;
  }

+static int raw_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+                              int flags)
+{
+    return bdrv_reopen_prepare(bs->file, prs, flags);
+}
+
+static void raw_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    bdrv_reopen_commit(bs->file, rs);
+}
+
+static void raw_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    bdrv_reopen_abort(bs->file, rs);
+}
+
  static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t 
sector_num,
                                       int nb_sectors, QEMUIOVector *qiov)
  {
@@ -111,6 +127,10 @@
      .instance_size      = 1,

      .bdrv_open          = raw_open,
+    .bdrv_reopen_prepare
+                        = raw_reopen_prepare,
+    .bdrv_reopen_commit = raw_reopen_commit,
+    .bdrv_reopen_abort  = raw_reopen_abort,
      .bdrv_close         = raw_close,

      .bdrv_co_readv          = raw_co_readv,
Index: qemu/block/raw-posix.c
===================================================================
--- qemu.orig/block/raw-posix.c    2012-06-19 15:52:40.520586178 +0530
+++ qemu/block/raw-posix.c    2012-06-20 15:59:09.036535061 +0530
@@ -140,8 +140,15 @@
  #endif
  } BDRVRawState;

+typedef struct BDRVRawReopenState {
+    BDRVReopenState reopen_state;
+    BDRVRawState *stash_s;
+} BDRVRawReopenState;
+
  static int fd_open(BlockDriverState *bs);
  static int64_t raw_getlength(BlockDriverState *bs);
+static void raw_stash_state(BDRVRawState *stashed_state, BDRVRawState *s);
+static void raw_revert_state(BDRVRawState *s, BDRVRawState *stashed_state);

  #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
  static int cdrom_reopen(BlockDriverState *bs);
@@ -283,6 +290,119 @@
      return raw_open_common(bs, filename, flags, 0);
  }

+static int raw_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+                              int flags)
+{
+    BDRVRawReopenState *raw_rs = g_malloc0(sizeof(BDRVRawReopenState));
+    BDRVRawState *s = bs->opaque;
+    int ret = 0;
+
+    raw_rs->reopen_state.bs = bs;
+
+    /* stash state before reopen */
+    raw_rs->stash_s = g_malloc0(sizeof(BDRVRawState));
+
+    raw_stash_state(raw_rs->stash_s, s);
+    s->fd = dup3(raw_rs->stash_s->fd,s->fd,O_CLOEXEC);
+
+    *prs = &(raw_rs->reopen_state);
+
+    /* Flags that can be set using fcntl */
+    int fcntl_flags = BDRV_O_NOCACHE;
+
+    if ((bs->open_flags & ~fcntl_flags) == (flags & ~fcntl_flags)) {
+        if ((flags & BDRV_O_NOCACHE)) {
+            s->open_flags |= O_DIRECT;
+        } else {
+            s->open_flags &= ~O_DIRECT;
+        }
+        ret = fcntl_setfl(s->fd, s->open_flags);
+    } else {
+
+        /* close and reopen using new flags */
+      bdrv_flush(bs);
+        ret = bs->drv->bdrv_file_open(bs, bs->filename, flags);
+    }
+    return ret;
+}
+
+static void raw_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVRawReopenState *raw_rs;
+
+    raw_rs = container_of(rs, BDRVRawReopenState, reopen_state);
+
+    /* clean up stashed state */
+    close(raw_rs->stash_s->fd);
+    g_free(raw_rs->stash_s);
+    g_free(raw_rs);
+}
+
+static void raw_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVRawReopenState *raw_rs;
+    BDRVRawState *s = bs->opaque;
+
+    raw_rs = container_of(rs, BDRVRawReopenState, reopen_state);
+
+    /* revert to stashed state */
+    if (s->fd != -1) {
+        close(s->fd);
+    }
+
+    raw_revert_state(s, raw_rs->stash_s);
+    g_free(raw_rs->stash_s);
+    g_free(raw_rs);
+}
+
+static void raw_stash_state(BDRVRawState *stashed_s, BDRVRawState *s)
+{
+    stashed_s->fd = -1;
+    stashed_s->type = s->type;
+    stashed_s->open_flags = s->open_flags;
+#if defined(__linux__)
+    /* linux floppy specific */
+    stashed_s->fd_open_time = s->fd_open_time;
+    stashed_s->fd_error_time = s->fd_error_time;
+    stashed_s->fd_got_error = s->fd_got_error;
+    stashed_s->fd_media_changed = s->fd_media_changed;
+#endif
+#ifdef CONFIG_LINUX_AIO
+    stashed_s->use_aio = s->use_aio;
+    stashed_s->aio_ctx = s->aio_ctx;
+#endif
+    stashed_s->aligned_buf = s->aligned_buf;
+    stashed_s->aligned_buf_size = s->aligned_buf_size;
+#ifdef CONFIG_XFS
+    stashed_s->is_xfs = s->is_xfs;
+#endif
+
+}
+
+static void raw_revert_state(BDRVRawState *s, BDRVRawState *stashed_s)
+{
+
+    s->fd = stashed_s->fd;
+    s->type = stashed_s->type;
+    s->open_flags = stashed_s->open_flags;
+#if defined(__linux__)
+    /* linux floppy specific */
+    s->fd_open_time = stashed_s->fd_open_time;
+    s->fd_error_time = stashed_s->fd_error_time;
+    s->fd_got_error = stashed_s->fd_got_error;
+    s->fd_media_changed = stashed_s->fd_media_changed;
+#endif
+#ifdef CONFIG_LINUX_AIO
+    s->use_aio = stashed_s->use_aio;
+    s->aio_ctx = stashed_s->aio_ctx;
+#endif
+    s->aligned_buf = stashed_s->aligned_buf;
+    s->aligned_buf_size = stashed_s->aligned_buf_size;
+#ifdef CONFIG_XFS
+    s->is_xfs = stashed_s->is_xfs;
+#endif
+}
+
  /* XXX: use host sector size if necessary with:
  #ifdef DIOCGSECTORSIZE
          {
@@ -728,6 +848,9 @@
      .instance_size = sizeof(BDRVRawState),
      .bdrv_probe = NULL, /* no probe for protocols */
      .bdrv_file_open = raw_open,
+    .bdrv_reopen_prepare = raw_reopen_prepare,
+    .bdrv_reopen_commit = raw_reopen_commit,
+    .bdrv_reopen_abort = raw_reopen_abort,
      .bdrv_close = raw_close,
      .bdrv_create = raw_create,
      .bdrv_co_discard = raw_co_discard,


[-- Attachment #2: Type: text/html, Size: 11807 bytes --]

Re: [Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1084 bytes --]

On 07/03/2012 11:15 PM, Shrinidhi Joshi wrote:
> On Saturday 16 June 2012 03:41 AM, Eric Blake wrote:
>>  On 06/15/2012 02:48 PM, Supriya Kannery wrote:
>> > raw-posix driver changes for bdrv_reopen_xx functions to
>> > safely reopen image files. Reopening of image files while
>> > changing hostcache dynamically is handled here.
>> >

> raw-posix driver changes for bdrv_reopen_xx functions to
> safely reopen image files. Reopening of image files while
> changing hostcache dynamically is handled here.

A resubmission of a fixed patch is better handled as a new thread with a
v2 in the subject line, rather than embedded as a reply to the v1 comments.


> +    raw_stash_state(raw_rs->stash_s, s);
> +    s->fd = dup3(raw_rs->stash_s->fd,s->fd,O_CLOEXEC);
> +

dup3() is not universally available (it is not in POSIX, yet).  You need
configure checks to determine if it exists, and a graceful fallback
using dup2()/fcntl(F_GETFD/F_SETFD) if not.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

[Qemu-devel] [v1 Patch 6/10]Qemu: raw-win32 image file reopen

[Supriya Kannery]

raw-win32 driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while
changing hostcache dynamically is handled here.

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

Index: qemu/block/raw-win32.c
===================================================================
--- qemu.orig/block/raw-win32.c
+++ qemu/block/raw-win32.c
@@ -26,18 +26,27 @@
 #include "block_int.h"
 #include "module.h"
 #include <windows.h>
+#include <winbase.h>
 #include <winioctl.h>
 
 #define FTYPE_FILE 0
 #define FTYPE_CD     1
 #define FTYPE_HARDDISK 2
+#define WINDOWS_VISTA 6
 
 typedef struct BDRVRawState {
     HANDLE hfile;
     int type;
     char drive_path[16]; /* format: "d:\" */
+    DWORD overlapped;
 } BDRVRawState;
 
+typedef struct BDRVRawReopenState {
+    BDRVReopenState reopen_state;
+    HANDLE stash_hfile;
+    DWORD  stash_overlapped;
+} BDRVRawReopenState;
+
 int qemu_ftruncate64(int fd, int64_t length)
 {
     LARGE_INTEGER li;
@@ -106,9 +115,96 @@ static int raw_open(BlockDriverState *bs
             return -EACCES;
         return -1;
     }
+    s->overlapped = overlapped;
     return 0;
 }
 
+static int raw_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+                              int flags)
+{
+    BDRVRawReopenState *raw_rs = g_malloc0(sizeof(BDRVRawReopenState));
+    BDRVRawState *s = bs->opaque;
+    int ret = 0;
+    OSVERSIONINFO osvi;
+    BOOL bIsWindowsVistaorLater;
+
+    raw_rs->bs = bs;
+    raw_rs->stash_hfile = s->hfile;
+    raw_rs->stash_overlapped = s->overlapped;
+    *prs = raw_rs;
+
+    if (flags & BDRV_O_NOCACHE) {
+        s->overlapped |= FILE_FLAG_NO_BUFFERING;
+    } else {
+        s->overlapped &= ~FILE_FLAG_NO_BUFFERING;
+    }
+
+    if (!(flags & BDRV_O_CACHE_WB)) {
+        s->overlapped |= FILE_FLAG_WRITE_THROUGH;
+    } else {
+        s->overlapped &= ~FILE_FLAG_WRITE_THROUGH;
+    }
+
+    ZeroMemory(&osvi, sizeof(OSVERSIONINFO));
+    osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
+
+    GetVersionEx(&osvi);
+
+    if (osvi.dwMajorVersion >= WINDOWS_VISTA) {
+        s->hfile = ReOpenFile(raw_rs->stash_hfile, 0, FILE_SHARE_READ,
+                              overlapped);
+        if (s->hfile == INVALID_HANDLE_VALUE) {
+            int err = GetLastError();
+            if (err == ERROR_ACCESS_DENIED) {
+                ret = -EACCES;
+            } else {
+                ret = -1;
+            }
+        }
+    } else {
+
+        DuplicateHandle(GetCurrentProcess(),
+                    raw_rs->stash_hfile,
+                    GetCurrentProcess(),
+                    &s->hfile,
+                    0,
+                    FALSE,
+                    DUPLICATE_SAME_ACCESS);
+        bs->drv->bdrv_close(bs);
+        ret = bs->drv->bdrv_open(bs, bs->filename, flags);
+    }
+    return ret;
+}
+
+
+static void raw_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVRawReopenState *raw_rs;
+
+    raw_rs = container_of(rs, BDRVRawReopenState, reopen_state);
+
+    /* clean up stashed handle */
+    CloseHandle(raw_rs->stash_hfile);
+    g_free(raw_rs);
+
+}
+
+static void raw_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+
+    BDRVRawReopenState *raw_rs;
+    BDRVRawState *s = bs->opaque;
+
+    raw_rs = container_of(rs, BDRVRawReopenState, reopen_state);
+
+    if (s->hfile && (s->hfile != INVALID_HANDLE_VALUE)) {
+        CloseHandle(s->hfile);
+    }
+    s->hfile = raw_rs->stash_hfile;
+    s->overlapped = raw_rs->stash_overlapped;
+    g_free(raw_rs);
+}
+
 static int raw_read(BlockDriverState *bs, int64_t sector_num,
                     uint8_t *buf, int nb_sectors)
 {

[Qemu-devel] [v1 Patch 7/10]Qemu: vmdk image file reopen

[Supriya Kannery]

vmdk driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while
changing hostcache dynamically is handled here.

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

Index: qemu/block/vmdk.c
===================================================================
--- qemu.orig/block/vmdk.c
+++ qemu/block/vmdk.c
@@ -115,6 +115,14 @@ typedef struct VmdkGrainMarker {
     uint8_t  data[0];
 } VmdkGrainMarker;
 
+typedef struct BDRVVmdkReopenState {
+    BDRVReopenState reopen_state;
+    BDRVVmdkState *stash_s;
+} BDRVVmdkReopenState;
+
+static void vmdk_stash_state(BDRVVmdkState *stashed_state, BDRVVmdkState *s);
+static void vmdk_revert_state(BDRVVmdkState *s, BDRVVmdkState *stashed_state);
+
 static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
     uint32_t magic;
@@ -588,7 +596,6 @@ static int vmdk_parse_extents(const char
         if (!strcmp(type, "FLAT")) {
             /* FLAT extent */
             VmdkExtent *extent;
-
             extent = vmdk_add_extent(bs, extent_file, true, sectors,
                             0, 0, 0, 0, sectors);
             extent->flat_start_offset = flat_offset << 9;
@@ -675,6 +682,94 @@ fail:
     return ret;
 }
 
+static int vmdk_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+                               int flags)
+{
+    BDRVVmdkReopenState *vmdk_rs = g_malloc0(sizeof(BDRVVmdkReopenState));
+    int ret = 0;
+    BDRVVmdkState *s = bs->opaque;
+
+    vmdk_rs->reopen_state.bs = bs;
+
+    /* save state before reopen */
+    vmdk_rs->stash_s = g_malloc0(sizeof(BDRVVmdkState));
+    vmdk_stash_state(vmdk_rs->stash_s, s);
+    s->num_extents = 0;
+    s->extents = NULL;
+    s->migration_blocker = NULL;
+    *prs = &(vmdk_rs->reopen_state);
+
+    /* create extents afresh with new flags */
+     ret = vmdk_open(bs, flags);
+     return ret;
+}
+
+static void vmdk_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVVmdkReopenState *vmdk_rs;
+    BDRVVmdkState *stashed_s;
+    VmdkExtent *e;
+    int i;
+
+    vmdk_rs = container_of(rs, BDRVVmdkReopenState, reopen_state);
+    stashed_s = vmdk_rs->stash_s;
+
+    /* clean up stashed state */
+    for (i = 0; i < stashed_s->num_extents; i++) {
+        e = &stashed_s->extents[i];
+        g_free(e->l1_table);
+        g_free(e->l2_cache);
+        g_free(e->l1_backup_table);
+    }
+    g_free(stashed_s->extents);
+    g_free(vmdk_rs->stash_s);
+    g_free(vmdk_rs);
+}
+
+static void vmdk_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVVmdkReopenState *vmdk_rs;
+    BDRVVmdkState *s = bs->opaque;
+    VmdkExtent *e;
+    int i;
+
+    vmdk_rs = container_of(rs, BDRVVmdkReopenState, reopen_state);
+
+    /* revert to stashed state */
+    for (i = 0; i < s->num_extents; i++) {
+        e = &s->extents[i];
+        g_free(e->l1_table);
+        g_free(e->l2_cache);
+        g_free(e->l1_backup_table);
+    }
+    g_free(s->extents);
+    vmdk_revert_state(s, vmdk_rs->stash_s);
+    g_free(vmdk_rs->stash_s);
+    g_free(vmdk_rs);
+}
+
+static void vmdk_stash_state(BDRVVmdkState *stashed_state, BDRVVmdkState *s)
+{
+   stashed_state->lock = s->lock;
+   stashed_state->desc_offset = s->desc_offset;
+   stashed_state->cid_updated = s->cid_updated;
+   stashed_state->parent_cid = s->parent_cid;
+   stashed_state->num_extents = s->num_extents;
+   stashed_state->extents = s->extents;
+   stashed_state->migration_blocker = s->migration_blocker;
+}
+
+static void vmdk_revert_state(BDRVVmdkState *s, BDRVVmdkState *stashed_state)
+{
+   s->lock = stashed_state->lock;
+   s->desc_offset = stashed_state->desc_offset;
+   s->cid_updated = stashed_state->cid_updated;
+   s->parent_cid = stashed_state->parent_cid;
+   s->num_extents = stashed_state->num_extents;
+   s->extents = stashed_state->extents;
+   s->migration_blocker = stashed_state->migration_blocker;
+}
+
 static int get_whole_cluster(BlockDriverState *bs,
                 VmdkExtent *extent,
                 uint64_t cluster_offset,
@@ -1593,6 +1688,12 @@ static BlockDriver bdrv_vmdk = {
     .instance_size  = sizeof(BDRVVmdkState),
     .bdrv_probe     = vmdk_probe,
     .bdrv_open      = vmdk_open,
+    .bdrv_reopen_prepare
+                    = vmdk_reopen_prepare,
+    .bdrv_reopen_commit
+                    = vmdk_reopen_commit,
+    .bdrv_reopen_abort
+                    = vmdk_reopen_abort,
     .bdrv_read      = vmdk_co_read,
     .bdrv_write     = vmdk_co_write,
     .bdrv_close     = vmdk_close,

[Qemu-devel] [v1 Patch 8/10]Qemu: qcow2 image file reopen

[Supriya Kannery]

qcow2 driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while
changing hostcache dynamically is handled here.

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

Index: qemu/block/qcow2.c
===================================================================
--- qemu.orig/block/qcow2.c
+++ qemu/block/qcow2.c
@@ -52,10 +52,19 @@ typedef struct {
     uint32_t magic;
     uint32_t len;
 } QCowExtension;
+
+typedef struct BDRVQcowReopenState {
+    BDRVReopenState reopen_state;
+    BDRVQcowState *stash_s;
+} BDRVQcowReopenState;
+
 #define  QCOW2_EXT_MAGIC_END 0
 #define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
 #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
 
+static void qcow2_stash_state(BDRVQcowState *stashed_state, BDRVQcowState *s);
+static void qcow2_revert_state(BDRVQcowState *s, BDRVQcowState *stashed_state);
+
 static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
     const QCowHeader *cow_header = (const void *)buf;
@@ -436,6 +445,171 @@ static int qcow2_open(BlockDriverState *
     return ret;
 }
 
+static int qcow2_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+                               int flags)
+{
+    BDRVQcowReopenState *qcow2_rs = g_malloc0(sizeof(BDRVQcowReopenState));
+    int ret = 0;
+    BDRVQcowState *s = bs->opaque;
+
+    qcow2_rs->reopen_state.bs = bs;
+
+    /* save state before reopen */
+    qcow2_rs->stash_s = g_malloc0(sizeof(BDRVQcowState));
+    qcow2_stash_state(qcow2_rs->stash_s, s);
+    *prs = &(qcow2_rs->reopen_state);
+
+    /* Reopen file with new flags */
+     ret = qcow2_open(bs, flags);
+     return ret;
+}
+
+static void qcow2_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVQcowReopenState *qcow2_rs;
+    BDRVQcowState *stashed_s;
+
+    qcow2_rs = container_of(rs, BDRVQcowReopenState, reopen_state);
+    stashed_s = qcow2_rs->stash_s;
+
+    g_free(stashed_s->l1_table);
+
+    qcow2_cache_flush(bs, stashed_s->l2_table_cache);
+    qcow2_cache_flush(bs, stashed_s->refcount_block_cache);
+
+    qcow2_cache_destroy(bs, stashed_s->l2_table_cache);
+    qcow2_cache_destroy(bs, stashed_s->refcount_block_cache);
+
+    g_free(stashed_s->unknown_header_fields);
+    cleanup_unknown_header_ext(bs);
+
+    g_free(stashed_s->cluster_cache);
+    qemu_vfree(stashed_s->cluster_data);
+    qcow2_refcount_close(bs);
+    qcow2_free_snapshots(bs);
+
+    g_free(stashed_s);
+    g_free(qcow2_rs);
+}
+
+static void qcow2_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVQcowReopenState *qcow2_rs;
+    BDRVQcowState *s = bs->opaque;
+    BDRVQcowState *stashed_s;
+
+    qcow2_rs = container_of(rs, BDRVQcowReopenState, reopen_state);
+
+    /* Revert to stashed state */
+    qcow2_revert_state(s, qcow2_rs->stash_s);
+    stashed_s = qcow2_rs->stash_s;
+
+    g_free(stashed_s);
+    g_free(qcow2_rs);
+}
+
+static void qcow2_stash_state(BDRVQcowState *stashed_s, BDRVQcowState *s)
+{
+    stashed_s->cluster_bits = s->cluster_bits;
+    stashed_s->cluster_size = s->cluster_size;
+    stashed_s->cluster_sectors = s->cluster_sectors;
+    stashed_s->l2_bits = s->l2_bits;
+    stashed_s->l2_size = s->l2_size;
+    stashed_s->l1_size = s->l1_size;
+    stashed_s->l1_vm_state_index = s->l1_vm_state_index;
+    stashed_s->csize_shift = s->csize_shift;
+    stashed_s->csize_mask = s->csize_mask;
+    stashed_s->cluster_offset_mask = s->cluster_offset_mask;
+    stashed_s->l1_table_offset = s->l1_table_offset;
+    stashed_s->l1_table = s->l1_table;
+
+    stashed_s->l2_table_cache = s->l2_table_cache;
+    stashed_s->refcount_block_cache = s->refcount_block_cache;
+
+    stashed_s->cluster_cache = s->cluster_cache;
+    stashed_s->cluster_data = s->cluster_data;
+    stashed_s->cluster_cache_offset = s->cluster_cache_offset;
+    stashed_s->cluster_allocs = s->cluster_allocs;
+
+    stashed_s->refcount_table = s->refcount_table;
+    stashed_s->refcount_table_offset = s->refcount_table_offset;
+    stashed_s->refcount_table_size = s->refcount_table_size;
+    stashed_s->free_cluster_index = s->free_cluster_index;
+    stashed_s->free_byte_offset = s->free_byte_offset;
+
+    stashed_s->lock = s->lock;
+
+    stashed_s->crypt_method = s->crypt_method;
+    stashed_s->crypt_method_header = s->crypt_method_header;
+    stashed_s->aes_encrypt_key = s->aes_encrypt_key;
+    stashed_s->aes_decrypt_key = s->aes_decrypt_key;
+    stashed_s->snapshots_offset = s->snapshots_offset;
+    stashed_s->snapshots_size = s->snapshots_size;
+    stashed_s->nb_snapshots = s->nb_snapshots;
+    stashed_s->snapshots = s->snapshots;
+
+    stashed_s->flags = s->flags;
+    stashed_s->qcow_version = s->qcow_version;
+
+    stashed_s->incompatible_features = s->incompatible_features;
+    stashed_s->compatible_features = s->compatible_features;
+
+    stashed_s->unknown_header_fields_size = s->unknown_header_fields_size;
+    stashed_s->unknown_header_fields = s->unknown_header_fields;
+    stashed_s->unknown_header_ext = s->unknown_header_ext;
+
+}
+
+static void qcow2_revert_state(BDRVQcowState *s, BDRVQcowState *stashed_s)
+{
+   s->cluster_bits = stashed_s->cluster_bits;
+   s->cluster_size = stashed_s->cluster_size;
+   s->cluster_sectors = stashed_s->cluster_sectors;
+   s->l2_bits = stashed_s->l2_bits;
+   s->l2_size = stashed_s->l2_size;
+   s->l1_size = stashed_s->l1_size;
+   s->l1_vm_state_index = stashed_s->l1_vm_state_index;
+   s->csize_shift = stashed_s->csize_shift;
+   s->csize_mask = stashed_s->csize_mask;
+   s->cluster_offset_mask = stashed_s->cluster_offset_mask;
+   s->l1_table_offset = stashed_s->l1_table_offset;
+   s->l1_table = stashed_s->l1_table;
+   s->l2_table_cache = stashed_s->l2_table_cache;
+   s->refcount_block_cache = stashed_s->refcount_block_cache;
+
+   s->cluster_cache = stashed_s->cluster_cache;
+   s->cluster_data = stashed_s->cluster_data;
+   s->cluster_cache_offset = stashed_s->cluster_cache_offset;
+   s->cluster_allocs = stashed_s->cluster_allocs;
+
+   s->refcount_table = stashed_s->refcount_table;
+   s->refcount_table_offset = stashed_s->refcount_table_offset;
+   s->refcount_table_size = stashed_s->refcount_table_size;
+   s->free_cluster_index = stashed_s->free_cluster_index;
+   s->free_byte_offset = stashed_s->free_byte_offset;
+
+   s->lock = stashed_s->lock;
+
+   s->crypt_method = stashed_s->crypt_method;
+   s->crypt_method_header = stashed_s->crypt_method_header;
+   s->aes_encrypt_key = stashed_s->aes_encrypt_key;
+   s->aes_decrypt_key = stashed_s->aes_decrypt_key;
+   s->snapshots_offset = stashed_s->snapshots_offset;
+   s->snapshots_size = stashed_s->snapshots_size;
+   s->nb_snapshots = stashed_s->nb_snapshots;
+   s->snapshots = stashed_s->snapshots;
+
+   s->flags = stashed_s->flags;
+   s->qcow_version = stashed_s->qcow_version;
+
+   s->incompatible_features = stashed_s->incompatible_features;
+   s->compatible_features = stashed_s->compatible_features;
+
+   s->unknown_header_fields_size = stashed_s->unknown_header_fields_size;
+   s->unknown_header_fields = stashed_s->unknown_header_fields;
+   s->unknown_header_ext = stashed_s->unknown_header_ext;
+}
+
 static int qcow2_set_key(BlockDriverState *bs, const char *key)
 {
     BDRVQcowState *s = bs->opaque;
@@ -1571,6 +1745,9 @@ static BlockDriver bdrv_qcow2 = {
     .instance_size      = sizeof(BDRVQcowState),
     .bdrv_probe         = qcow2_probe,
     .bdrv_open          = qcow2_open,
+    .bdrv_reopen_prepare = qcow2_reopen_prepare,
+    .bdrv_reopen_commit = qcow2_reopen_commit,
+    .bdrv_reopen_abort  = qcow2_reopen_abort,
     .bdrv_close         = qcow2_close,
     .bdrv_create        = qcow2_create,
     .bdrv_co_is_allocated = qcow2_co_is_allocated,

[Qemu-devel] [v1 Patch 9/10]Qemu: qcow image file reopen

[Supriya Kannery]

qcow driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while
changing hostcache dynamically is handled here.

Signed-off-by: Shrinidhi Joshi <spjoshi31@gmail.com>

Index: qemu/block/qcow.c
===================================================================
--- qemu.orig/block/qcow.c
+++ qemu/block/qcow.c
@@ -78,7 +78,14 @@ typedef struct BDRVQcowState {
     Error *migration_blocker;
 } BDRVQcowState;
 
+typedef struct BDRVQcowReopenState {
+    BDRVReopenState reopen_state;
+    BDRVQcowState *stash_s;
+} BDRVQcowReopenState;
+
 static int decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
+static void qcow_stash_state(BDRVQcowState *stash_s, BDRVQcowState *s);
+static void qcow_revert_state(BDRVQcowState *s, BDRVQcowState *stash_s);
 
 static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
@@ -197,6 +204,103 @@ static int qcow_open(BlockDriverState *b
     return ret;
 }
 
+static int qcow_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+                                         int flags)
+{
+    BDRVQcowReopenState *qcow_rs = g_malloc0(sizeof(BDRVQcowReopenState));
+    int ret = 0;
+    BDRVQcowState *s = bs->opaque;
+
+    qcow_rs->reopen_state.bs = bs;
+    qcow_rs->stash_s = g_malloc0(sizeof(BDRVQcowState));
+    qcow_stash_state(qcow_rs->stash_s, s);
+    *prs = &(qcow_rs->reopen_state);
+
+    ret = qcow_open(bs, flags);
+    return ret;
+}
+
+static void qcow_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVQcowReopenState *qcow_rs;
+
+    qcow_rs = container_of(rs, BDRVQcowReopenState, reopen_state);
+    g_free(qcow_rs->stash_s);
+    g_free(qcow_rs);
+}
+
+static void qcow_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVQcowReopenState *qcow_rs;
+    BDRVQcowState *s = bs->opaque;
+
+    qcow_rs = container_of(rs, BDRVQcowReopenState, reopen_state);
+
+    /* revert to stashed state */
+    qcow_revert_state(s, qcow_rs->stash_s);
+
+    g_free(qcow_rs->stash_s);
+    g_free(qcow_rs);
+}
+
+static void qcow_stash_state(BDRVQcowState *stash_s, BDRVQcowState *s)
+{
+    int i;
+
+    stash_s->cluster_bits = s->cluster_bits;
+    stash_s->cluster_size = s->cluster_size;
+    stash_s->cluster_sectors = s->cluster_sectors;
+    stash_s->l2_bits = s->l2_bits;
+    stash_s->l2_size = s->l2_size;
+    stash_s->l1_size = s->l1_size;
+    stash_s->cluster_offset_mask = s->cluster_offset_mask;
+    stash_s->l1_table_offset = s->l1_table_offset;
+    stash_s->l1_table = s->l1_table;
+    stash_s->l2_cache = s->l2_cache;
+    for (i = 0; i < L2_CACHE_SIZE; i++) {
+        stash_s->l2_cache_offsets[i] = s->l2_cache_offsets[i];
+        stash_s->l2_cache_counts[i] = s->l2_cache_counts[i];
+    }
+    stash_s->cluster_cache = s->cluster_cache;
+    stash_s->cluster_data = s->cluster_data;
+    stash_s->cluster_cache_offset = s->cluster_cache_offset;
+    stash_s->crypt_method = s->crypt_method;
+    stash_s->crypt_method_header = s->crypt_method_header;
+    stash_s->aes_encrypt_key = s->aes_encrypt_key;
+    stash_s->aes_decrypt_key = s->aes_decrypt_key;
+    stash_s->lock = s->lock;
+    stash_s->migration_blocker = s->migration_blocker;
+}
+
+static void qcow_revert_state(BDRVQcowState *s, BDRVQcowState *stash_s)
+{
+    int i;
+
+    s->cluster_bits = stash_s->cluster_bits;
+    s->cluster_size = stash_s->cluster_size;
+    s->cluster_sectors = stash_s->cluster_sectors;
+    s->l2_bits = stash_s->l2_bits;
+    s->l2_size = stash_s->l2_size;
+    s->l1_size = stash_s->l1_size;
+    s->cluster_offset_mask = stash_s->cluster_offset_mask;
+    s->l1_table_offset = stash_s->l1_table_offset;
+    s->l1_table = stash_s->l1_table;
+    s->l2_cache = stash_s->l2_cache;
+    for (i = 0; i < L2_CACHE_SIZE; i++) {
+        s->l2_cache_offsets[i] = s->l2_cache_offsets[i];
+        s->l2_cache_counts[i] = stash_s->l2_cache_counts[i];
+    }
+    s->cluster_cache = stash_s->cluster_cache;
+    s->cluster_data = stash_s->cluster_data;
+    s->cluster_cache_offset = stash_s->cluster_cache_offset;
+    s->crypt_method = stash_s->crypt_method;
+    s->crypt_method_header = stash_s->crypt_method_header;
+    s->aes_encrypt_key = stash_s->aes_encrypt_key;
+    s->aes_decrypt_key = stash_s->aes_decrypt_key;
+    s->lock = stash_s->lock;
+    s->migration_blocker = stash_s->migration_blocker;
+}
+
 static int qcow_set_key(BlockDriverState *bs, const char *key)
 {
     BDRVQcowState *s = bs->opaque;
@@ -870,6 +974,10 @@ static BlockDriver bdrv_qcow = {
     .bdrv_close		= qcow_close,
     .bdrv_create	= qcow_create,
 
+    .bdrv_reopen_prepare    = qcow_reopen_prepare,
+    .bdrv_reopen_commit     = qcow_reopen_commit,
+    .bdrv_reopen_abort      = qcow_reopen_abort,
+
     .bdrv_co_readv          = qcow_co_readv,
     .bdrv_co_writev         = qcow_co_writev,
     .bdrv_co_is_allocated   = qcow_co_is_allocated,

[Qemu-devel] [v1 Patch 10/10]Qemu: qed image file reopen

[Supriya Kannery]

qed driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while
changing hostcache dynamically is handled here.

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

Index: qemu/block/qed.c
===================================================================
--- qemu.orig/block/qed.c
+++ qemu/block/qed.c
@@ -18,6 +18,14 @@
 #include "qerror.h"
 #include "migration.h"
 
+typedef struct BDRVQEDReopenState {
+    BDRVReopenState reopen_state;
+    BDRVQEDState *stash_s;
+} BDRVQEDReopenState;
+
+static void qed_stash_state(BDRVQEDState *stashed_state, BDRVQEDState *s);
+static void qed_revert_state(BDRVQEDState *s, BDRVQEDState *stashed_state);
+
 static void qed_aio_cancel(BlockDriverAIOCB *blockacb)
 {
     QEDAIOCB *acb = (QEDAIOCB *)blockacb;
@@ -512,6 +520,98 @@ out:
     return ret;
 }
 
+static int bdrv_qed_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+                               int flags)
+{
+    BDRVQEDReopenState *qed_rs = g_malloc0(sizeof(BDRVQEDReopenState));
+    int ret = 0;
+    BDRVQEDState *s = bs->opaque;
+
+    qed_rs->reopen_state.bs = bs;
+
+    /* save state before reopen */
+    qed_rs->stash_s = g_malloc0(sizeof(BDRVQEDState));
+    qed_stash_state(qed_rs->stash_s, s);
+    *prs = &(qed_rs->reopen_state);
+
+    /* Reopen file with new flags */
+     ret = bdrv_qed_open(bs, flags);
+     return ret;
+}
+
+static void bdrv_qed_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVQEDReopenState *qed_rs;
+    BDRVQEDState *stashed_s;
+
+    qed_rs = container_of(rs, BDRVQEDReopenState, reopen_state);
+    stashed_s = qed_rs->stash_s;
+
+    qed_cancel_need_check_timer(stashed_s);
+    qemu_free_timer(stashed_s->need_check_timer);
+
+    qed_free_l2_cache(&stashed_s->l2_cache);
+    qemu_vfree(stashed_s->l1_table);
+
+    g_free(stashed_s);
+    g_free(qed_rs);
+}
+
+static void bdrv_qed_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+    BDRVQEDReopenState *qed_rs;
+    BDRVQEDState *s = bs->opaque;
+    BDRVQEDState *stashed_s;
+
+    qed_rs = container_of(rs, BDRVQEDReopenState, reopen_state);
+
+    /* Revert to stashed state */
+    qed_revert_state(s, qed_rs->stash_s);
+    stashed_s = qed_rs->stash_s;
+
+    g_free(stashed_s);
+    g_free(qed_rs);
+}
+
+static void qed_stash_state(BDRVQEDState *stashed_state, BDRVQEDState *s)
+{
+    stashed_state->bs = s->bs;
+    stashed_state->file_size = s->file_size;
+
+    stashed_state->header = s->header;
+    stashed_state->l1_table = s->l1_table;
+    stashed_state->l2_cache = s->l2_cache;
+    stashed_state->table_nelems = s->table_nelems;
+    stashed_state->l1_shift = s->l1_shift;
+    stashed_state->l2_shift = s->l2_shift;
+    stashed_state->l2_mask = s->l2_mask;
+
+    stashed_state->allocating_write_reqs = s->allocating_write_reqs;
+    stashed_state->allocating_write_reqs_plugged =
+                                         s->allocating_write_reqs_plugged;
+
+    stashed_state->need_check_timer = s->need_check_timer;
+}
+
+static void qed_revert_state(BDRVQEDState *s, BDRVQEDState *stashed_state)
+{
+    s->bs = stashed_state->bs;
+    s->file_size = stashed_state->file_size;
+
+    s->header = stashed_state->header;
+    s->l1_table = stashed_state->l1_table;
+    s->l2_cache = stashed_state->l2_cache;
+    s->table_nelems = stashed_state->table_nelems;
+    s->l1_shift = stashed_state->l1_shift;
+    s->l2_shift = stashed_state->l2_shift;
+    s->l2_mask = stashed_state->l2_mask;
+
+    s->allocating_write_reqs = s->allocating_write_reqs;
+    s->allocating_write_reqs_plugged = s->allocating_write_reqs_plugged;
+
+    s->need_check_timer = s->need_check_timer;
+}
+
 static void bdrv_qed_close(BlockDriverState *bs)
 {
     BDRVQEDState *s = bs->opaque;
@@ -1559,6 +1659,9 @@ static BlockDriver bdrv_qed = {
     .bdrv_rebind              = bdrv_qed_rebind,
     .bdrv_open                = bdrv_qed_open,
     .bdrv_close               = bdrv_qed_close,
+    .bdrv_reopen_prepare      = bdrv_qed_reopen_prepare,
+    .bdrv_reopen_commit       = bdrv_qed_reopen_commit,
+    .bdrv_reopen_abort        = bdrv_qed_reopen_abort,
     .bdrv_create              = bdrv_qed_create,
     .bdrv_co_is_allocated     = bdrv_qed_co_is_allocated,
     .bdrv_make_empty          = bdrv_qed_make_empty,

Re: [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen

[Stefan Weil]

Am 15.06.2012 22:46, schrieb Supriya Kannery:
>   For changing host pagecache setting of a running VM, it is
> important to have a safe way of reopening its image file.
>


Hello,

please use 'QEMU' instead of 'Qemu' where needed.

I assume that everybody here expects that any patch
on this mailing list is for QEMU, so there is no need
to start the summaries of indiviual patches with
"QEMU:". You could start them with "block:" or take
a look in the files' history to see what other people
used.

Regards

Stefan Weil