* [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
@ 2012-06-15 20:47 ` Supriya Kannery
2012-06-15 21:07 ` Eric Blake
2012-07-05 16:38 ` Jeff Cody
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures Supriya Kannery
` (9 subsequent siblings)
10 siblings, 2 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-06-15 20:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
Luiz Capitulino, Christoph Hellwig
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]);
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting Supriya Kannery
@ 2012-06-15 21:07 ` Eric Blake
2012-07-09 14:43 ` Kevin Wolf
2012-07-05 16:38 ` Jeff Cody
1 sibling, 1 reply; 33+ messages in thread
From: Eric Blake @ 2012-06-15 21:07 UTC (permalink / raw)
To: Supriya Kannery
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
qemu-devel, Luiz Capitulino, Christoph Hellwig
[-- 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 --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting
2012-06-15 21:07 ` Eric Blake
@ 2012-07-09 14:43 ` Kevin Wolf
2012-07-11 14:03 ` Luiz Capitulino
0 siblings, 1 reply; 33+ messages in thread
From: Kevin Wolf @ 2012-07-09 14:43 UTC (permalink / raw)
To: Eric Blake
Cc: Supriya Kannery, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
qemu-devel, Luiz Capitulino, Christoph Hellwig
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting
2012-07-09 14:43 ` Kevin Wolf
@ 2012-07-11 14:03 ` Luiz Capitulino
2012-07-29 6:21 ` Supriya Kannery
0 siblings, 1 reply; 33+ messages in thread
From: Luiz Capitulino @ 2012-07-11 14:03 UTC (permalink / raw)
To: Kevin Wolf
Cc: Supriya Kannery, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
qemu-devel, Eric Blake, Christoph Hellwig
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.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting
2012-07-11 14:03 ` Luiz Capitulino
@ 2012-07-29 6:21 ` Supriya Kannery
0 siblings, 0 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-07-29 6:21 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting Supriya Kannery
2012-06-15 21:07 ` Eric Blake
@ 2012-07-05 16:38 ` Jeff Cody
2012-07-29 6:54 ` Supriya Kannery
1 sibling, 1 reply; 33+ messages in thread
From: Jeff Cody @ 2012-07-05 16:38 UTC (permalink / raw)
To: Supriya Kannery
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, qemu-devel,
Luiz Capitulino, Paolo Bonzini, Christoph Hellwig
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]);
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting
2012-07-05 16:38 ` Jeff Cody
@ 2012-07-29 6:54 ` Supriya Kannery
0 siblings, 0 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-07-29 6:54 UTC (permalink / raw)
To: jcody
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, qemu-devel,
Luiz Capitulino, Paolo Bonzini, Christoph Hellwig
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting Supriya Kannery
@ 2012-06-15 20:47 ` Supriya Kannery
2012-07-09 14:47 ` Kevin Wolf
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change Supriya Kannery
` (8 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Supriya Kannery @ 2012-06-15 20:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
Luiz Capitulino, Christoph Hellwig
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 } }"
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures Supriya Kannery
@ 2012-07-09 14:47 ` Kevin Wolf
2012-07-29 6:58 ` Supriya Kannery
0 siblings, 1 reply; 33+ messages in thread
From: Kevin Wolf @ 2012-07-09 14:47 UTC (permalink / raw)
To: Supriya Kannery
Cc: Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody, qemu-devel,
Luiz Capitulino, Christoph Hellwig
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures
2012-07-09 14:47 ` Kevin Wolf
@ 2012-07-29 6:58 ` Supriya Kannery
0 siblings, 0 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-07-29 6:58 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting Supriya Kannery
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures Supriya Kannery
@ 2012-06-15 20:47 ` Supriya Kannery
2012-06-15 21:56 ` Eric Blake
` (3 more replies)
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely Supriya Kannery
` (7 subsequent siblings)
10 siblings, 4 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-06-15 20:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
Luiz Capitulino, Christoph Hellwig
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change Supriya Kannery
@ 2012-06-15 21:56 ` Eric Blake
2012-07-29 7:33 ` Supriya Kannery
2012-06-20 18:18 ` Jeff Cody
` (2 subsequent siblings)
3 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2012-06-15 21:56 UTC (permalink / raw)
To: Supriya Kannery
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
qemu-devel, Luiz Capitulino, Christoph Hellwig
[-- 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 --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change
2012-06-15 21:56 ` Eric Blake
@ 2012-07-29 7:33 ` Supriya Kannery
0 siblings, 0 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-07-29 7:33 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
qemu-devel, Luiz Capitulino, Christoph Hellwig
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change Supriya Kannery
2012-06-15 21:56 ` Eric Blake
@ 2012-06-20 18:18 ` Jeff Cody
2012-07-04 5:10 ` Shrinidhi Joshi
2012-07-09 14:52 ` Kevin Wolf
2012-07-11 14:16 ` Luiz Capitulino
3 siblings, 1 reply; 33+ messages in thread
From: Jeff Cody @ 2012-06-20 18:18 UTC (permalink / raw)
To: Supriya Kannery
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, qemu-devel,
Luiz Capitulino, Christoph Hellwig
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
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change
2012-06-20 18:18 ` Jeff Cody
@ 2012-07-04 5:10 ` Shrinidhi Joshi
2012-07-04 6:30 ` Kevin Wolf
0 siblings, 1 reply; 33+ messages in thread
From: Shrinidhi Joshi @ 2012-07-04 5:10 UTC (permalink / raw)
To: jcody
Cc: Kevin Wolf, Stefan Hajnoczi, Christoph Hellwig, qemu-devel,
Luiz Capitulino
[-- 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 --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change
2012-07-04 5:10 ` Shrinidhi Joshi
@ 2012-07-04 6:30 ` Kevin Wolf
0 siblings, 0 replies; 33+ messages in thread
From: Kevin Wolf @ 2012-07-04 6:30 UTC (permalink / raw)
To: Shrinidhi Joshi
Cc: Stefan Hajnoczi, jcody, Christoph Hellwig, qemu-devel, Luiz Capitulino
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change Supriya Kannery
2012-06-15 21:56 ` Eric Blake
2012-06-20 18:18 ` Jeff Cody
@ 2012-07-09 14:52 ` Kevin Wolf
2012-07-11 14:16 ` Luiz Capitulino
3 siblings, 0 replies; 33+ messages in thread
From: Kevin Wolf @ 2012-07-09 14:52 UTC (permalink / raw)
To: Supriya Kannery
Cc: Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody, qemu-devel,
Luiz Capitulino, Christoph Hellwig
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change Supriya Kannery
` (2 preceding siblings ...)
2012-07-09 14:52 ` Kevin Wolf
@ 2012-07-11 14:16 ` Luiz Capitulino
2012-07-29 7:56 ` Supriya Kannery
3 siblings, 1 reply; 33+ messages in thread
From: Luiz Capitulino @ 2012-07-11 14:16 UTC (permalink / raw)
To: Supriya Kannery
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
qemu-devel, Christoph Hellwig
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
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change
2012-07-11 14:16 ` Luiz Capitulino
@ 2012-07-29 7:56 ` Supriya Kannery
0 siblings, 0 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-07-29 7:56 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
` (2 preceding siblings ...)
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change Supriya Kannery
@ 2012-06-15 20:47 ` Supriya Kannery
2012-06-15 22:02 ` Eric Blake
2012-07-09 15:06 ` Kevin Wolf
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen Supriya Kannery
` (6 subsequent siblings)
10 siblings, 2 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-06-15 20:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
Luiz Capitulino, Christoph Hellwig
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);
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely Supriya Kannery
@ 2012-06-15 22:02 ` Eric Blake
2012-07-09 15:06 ` Kevin Wolf
1 sibling, 0 replies; 33+ messages in thread
From: Eric Blake @ 2012-06-15 22:02 UTC (permalink / raw)
To: Supriya Kannery
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
qemu-devel, Luiz Capitulino, Christoph Hellwig
[-- 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 --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely Supriya Kannery
2012-06-15 22:02 ` Eric Blake
@ 2012-07-09 15:06 ` Kevin Wolf
1 sibling, 0 replies; 33+ messages in thread
From: Kevin Wolf @ 2012-07-09 15:06 UTC (permalink / raw)
To: Supriya Kannery
Cc: Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody, qemu-devel,
Luiz Capitulino, Christoph Hellwig
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
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
` (3 preceding siblings ...)
2012-06-15 20:47 ` [Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely Supriya Kannery
@ 2012-06-15 20:48 ` Supriya Kannery
2012-06-15 22:11 ` Eric Blake
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 6/10]Qemu: raw-win32 " Supriya Kannery
` (5 subsequent siblings)
10 siblings, 1 reply; 33+ messages in thread
From: Supriya Kannery @ 2012-06-15 20:48 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
Luiz Capitulino, Christoph Hellwig
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,
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen Supriya Kannery
@ 2012-06-15 22:11 ` Eric Blake
2012-07-04 5:15 ` Shrinidhi Joshi
0 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2012-06-15 22:11 UTC (permalink / raw)
To: Supriya Kannery
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
qemu-devel, Luiz Capitulino, Christoph Hellwig
[-- 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 --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen
2012-06-15 22:11 ` Eric Blake
@ 2012-07-04 5:15 ` Shrinidhi Joshi
2012-07-04 11:32 ` Eric Blake
0 siblings, 1 reply; 33+ messages in thread
From: Shrinidhi Joshi @ 2012-07-04 5:15 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, Stefan Hajnoczi, jcody, qemu-devel, Luiz Capitulino,
Christoph Hellwig
[-- 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 --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen
2012-07-04 5:15 ` Shrinidhi Joshi
@ 2012-07-04 11:32 ` Eric Blake
0 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2012-07-04 11:32 UTC (permalink / raw)
To: Shrinidhi Joshi
Cc: Kevin Wolf, Stefan Hajnoczi, jcody, qemu-devel, Luiz Capitulino,
Christoph Hellwig
[-- 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 --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [v1 Patch 6/10]Qemu: raw-win32 image file reopen
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
` (4 preceding siblings ...)
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen Supriya Kannery
@ 2012-06-15 20:48 ` Supriya Kannery
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 7/10]Qemu: vmdk " Supriya Kannery
` (4 subsequent siblings)
10 siblings, 0 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-06-15 20:48 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
Luiz Capitulino, Christoph Hellwig
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)
{
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [v1 Patch 7/10]Qemu: vmdk image file reopen
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
` (5 preceding siblings ...)
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 6/10]Qemu: raw-win32 " Supriya Kannery
@ 2012-06-15 20:48 ` Supriya Kannery
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 8/10]Qemu: qcow2 " Supriya Kannery
` (3 subsequent siblings)
10 siblings, 0 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-06-15 20:48 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
Luiz Capitulino, Christoph Hellwig
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,
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [v1 Patch 8/10]Qemu: qcow2 image file reopen
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
` (6 preceding siblings ...)
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 7/10]Qemu: vmdk " Supriya Kannery
@ 2012-06-15 20:48 ` Supriya Kannery
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 9/10]Qemu: qcow " Supriya Kannery
` (2 subsequent siblings)
10 siblings, 0 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-06-15 20:48 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
Luiz Capitulino, Christoph Hellwig
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,
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [v1 Patch 9/10]Qemu: qcow image file reopen
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
` (7 preceding siblings ...)
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 8/10]Qemu: qcow2 " Supriya Kannery
@ 2012-06-15 20:48 ` Supriya Kannery
2012-06-15 20:49 ` [Qemu-devel] [v1 Patch 10/10]Qemu: qed " Supriya Kannery
2012-07-09 17:51 ` [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and " Stefan Weil
10 siblings, 0 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-06-15 20:48 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
Luiz Capitulino, Christoph Hellwig
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,
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [v1 Patch 10/10]Qemu: qed image file reopen
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
` (8 preceding siblings ...)
2012-06-15 20:48 ` [Qemu-devel] [v1 Patch 9/10]Qemu: qcow " Supriya Kannery
@ 2012-06-15 20:49 ` Supriya Kannery
2012-07-09 17:51 ` [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and " Stefan Weil
10 siblings, 0 replies; 33+ messages in thread
From: Supriya Kannery @ 2012-06-15 20:49 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
Luiz Capitulino, Christoph Hellwig
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,
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen
2012-06-15 20:46 [Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen Supriya Kannery
` (9 preceding siblings ...)
2012-06-15 20:49 ` [Qemu-devel] [v1 Patch 10/10]Qemu: qed " Supriya Kannery
@ 2012-07-09 17:51 ` Stefan Weil
10 siblings, 0 replies; 33+ messages in thread
From: Stefan Weil @ 2012-07-09 17:51 UTC (permalink / raw)
To: Supriya Kannery
Cc: Kevin Wolf, Shrinidhi Joshi, Stefan Hajnoczi, Jeff Cody,
qemu-devel, Luiz Capitulino, Christoph Hellwig
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
^ permalink raw reply [flat|nested] 33+ messages in thread