* [Qemu-devel] [V3 0/3]block: Dynamically change hostcache setting using new "block_set" command @ 2011-06-17 16:37 Supriya Kannery 2011-06-17 16:37 ` [Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting Supriya Kannery ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Supriya Kannery @ 2011-06-17 16:37 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Supriya Kannery, Christoph Hellwig Currently host page cache setting for a block device cannot be changed without restarting a running VM. Following patchset [V3] is for enabling dynamic change of hostcache setting for block devices through qemu monitor and QMP. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. Changes from patchset V2: 1. Command "block_set" added for changing block device params dynamically 2. Enhanced info-block to display hostcache setting of block device 3. Added qmp interfaces for setting and querying hostcache New block command added: "block_set" -- Sets block device parameters while guest is running. Usage: block_set <device> <param> <value> <device> = block device <param> = parameter (say, "hostcache") <value> = on/off 1/3 Enhance "info block" to display hostcache setting 2/3 New error classes for file reopen and device insertion 3/3 Command "block_set" for dynamic params change for block device qemu/block.c | 62 +++++++++++++++++++++++++++++++++++++++++ +++++++++---- qemu/block.h | 2 ++ qemu/blockdev.c | 32 ++++++++++++++++++++++++++++++++ qemu/blockdev.h | 1 + qemu/hmp-commands.hx | 15 +++++++++++++++ qemu/qerror.c | 8 ++++++++ qemu/qerror.h | 6 ++++++ qemu/qmp-commands.hx | 2 ++ qmp-commands.hx | 30 +++++++++++++++++++++++++++++- 9 files changed, 153 insertions(+), 5 deletions(-) ~ ~ ~ ~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting 2011-06-17 16:37 [Qemu-devel] [V3 0/3]block: Dynamically change hostcache setting using new "block_set" command Supriya Kannery @ 2011-06-17 16:37 ` Supriya Kannery 2011-06-20 14:23 ` Kevin Wolf 2011-06-17 16:37 ` [Qemu-devel] [V3 2/3] Error classes for file reopen and device insertion Supriya Kannery 2011-06-17 16:38 ` [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change Supriya Kannery 2 siblings, 1 reply; 11+ messages in thread From: Supriya Kannery @ 2011-06-17 16:37 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Supriya Kannery, Christoph Hellwig Enhance "info block" to display hostcache setting for each block device. Example: (qemu) info block ide0-hd0: type=hd removable=0 file=../rhel6-32.qcow2 ro=0 drv=qcow2 encrypted=0 Enhanced to display "hostcache" setting: (qemu) info block ide0-hd0: type=hd removable=0 hostcache=true file=../rhel6-32.qcow2 ro=0 drv=qcow2 encrypted=0 Signed-off-by: Supriya Kannery <supriyak@in.ibm.com> --- block.c | 21 +++++++++++++++++---- qmp-commands.hx | 2 ++ 2 files changed, 19 insertions(+), 4 deletions(-) Index: qemu/block.c =================================================================== --- qemu.orig/block.c +++ qemu/block.c @@ -1694,6 +1694,14 @@ static void bdrv_print_dict(QObject *obj monitor_printf(mon, " locked=%d", qdict_get_bool(bs_dict, "locked")); } + if (qdict_haskey(bs_dict, "open_flags")) { + int open_flags = qdict_get_int(bs_dict, "open_flags"); + if (open_flags & BDRV_O_NOCACHE) + monitor_printf(mon, " hostcache=false"); + else + monitor_printf(mon, " hostcache=true"); + } + if (qdict_haskey(bs_dict, "inserted")) { QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted")); @@ -1730,13 +1738,18 @@ void bdrv_info(Monitor *mon, QObject **r QObject *bs_obj; bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', " - "'removable': %i, 'locked': %i }", - bs->device_name, bs->removable, - bs->locked); + "'removable': %i, 'locked': %i, " + "'hostcache': %s }", + bs->device_name, bs->removable, + bs->locked, + (bs->open_flags & BDRV_O_NOCACHE) ? + "false" : "true"); + + QDict *bs_dict = qobject_to_qdict(bs_obj); + qdict_put(bs_dict, "open_flags", qint_from_int(bs->open_flags)); if (bs->drv) { QObject *obj; - QDict *bs_dict = qobject_to_qdict(bs_obj); obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, " "'encrypted': %i }", Index: qemu/qmp-commands.hx =================================================================== --- qemu.orig/qmp-commands.hx +++ qemu/qmp-commands.hx @@ -1070,6 +1070,7 @@ Each json-object contain the following: - Possible values: "unknown" - "removable": true if the device is removable, false otherwise (json-bool) - "locked": true if the device is locked, false otherwise (json-bool) +- "hostcache": true if hostcache enabled, false otherwise (json-bool) - "inserted": only present if the device is inserted, it is a json-object containing the following: - "file": device file name (json-string) @@ -1091,6 +1092,7 @@ Example: { "device":"ide0-hd0", "locked":false, + "hostcache":false, "removable":false, "inserted":{ "ro":false, ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting 2011-06-17 16:37 ` [Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting Supriya Kannery @ 2011-06-20 14:23 ` Kevin Wolf 2011-06-22 14:59 ` Supriya Kannery 0 siblings, 1 reply; 11+ messages in thread From: Kevin Wolf @ 2011-06-20 14:23 UTC (permalink / raw) To: Supriya Kannery; +Cc: qemu-devel, Christoph Hellwig Am 17.06.2011 18:37, schrieb Supriya Kannery: > Enhance "info block" to display hostcache setting for each > block device. > > Example: > (qemu) info block > ide0-hd0: type=hd removable=0 file=../rhel6-32.qcow2 ro=0 drv=qcow2 > encrypted=0 > > Enhanced to display "hostcache" setting: > (qemu) info block > ide0-hd0: type=hd removable=0 hostcache=true file=../rhel6-32.qcow2 > ro=0 drv=qcow2 encrypted=0 > > Signed-off-by: Supriya Kannery <supriyak@in.ibm.com> > > --- > block.c | 21 +++++++++++++++++---- > qmp-commands.hx | 2 ++ > 2 files changed, 19 insertions(+), 4 deletions(-) > > Index: qemu/block.c > =================================================================== > --- qemu.orig/block.c > +++ qemu/block.c > @@ -1694,6 +1694,14 @@ static void bdrv_print_dict(QObject *obj > monitor_printf(mon, " locked=%d", qdict_get_bool(bs_dict, "locked")); > } > > + if (qdict_haskey(bs_dict, "open_flags")) { > + int open_flags = qdict_get_int(bs_dict, "open_flags"); > + if (open_flags & BDRV_O_NOCACHE) > + monitor_printf(mon, " hostcache=false"); > + else > + monitor_printf(mon, " hostcache=true"); Coding style requires braces. > + } > + > if (qdict_haskey(bs_dict, "inserted")) { > QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted")); > > @@ -1730,13 +1738,18 @@ void bdrv_info(Monitor *mon, QObject **r > QObject *bs_obj; > > bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', " > - "'removable': %i, 'locked': %i }", > - bs->device_name, bs->removable, > - bs->locked); > + "'removable': %i, 'locked': %i, " > + "'hostcache': %s }", > + bs->device_name, bs->removable, > + bs->locked, > + (bs->open_flags & BDRV_O_NOCACHE) ? > + "false" : "true"); Don't use tabs. Kevin ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting 2011-06-20 14:23 ` Kevin Wolf @ 2011-06-22 14:59 ` Supriya Kannery 0 siblings, 0 replies; 11+ messages in thread From: Supriya Kannery @ 2011-06-22 14:59 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel, Christoph Hellwig On 06/20/2011 07:53 PM, Kevin Wolf wrote: > Am 17.06.2011 18:37, schrieb Supriya Kannery: >> >> + int open_flags = qdict_get_int(bs_dict, "open_flags"); >> + if (open_flags& BDRV_O_NOCACHE) >> + monitor_printf(mon, " hostcache=false"); >> + else >> + monitor_printf(mon, " hostcache=true"); > > Coding style requires braces. > Now realising that, by mistake I used checkpatch.pl in linux kernel tree instead of the one in qemu tree. Will resubmit this patchset after checking using checkpatch.pl in qemu tree. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [V3 2/3] Error classes for file reopen and device insertion 2011-06-17 16:37 [Qemu-devel] [V3 0/3]block: Dynamically change hostcache setting using new "block_set" command Supriya Kannery 2011-06-17 16:37 ` [Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting Supriya Kannery @ 2011-06-17 16:37 ` Supriya Kannery 2011-06-20 14:23 ` Kevin Wolf 2011-06-17 16:38 ` [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change Supriya Kannery 2 siblings, 1 reply; 11+ messages in thread From: Supriya Kannery @ 2011-06-17 16:37 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Supriya Kannery, Christoph Hellwig New error classes defined for cases where device not inserted and file reopen failed. Signed-off-by: Supriya Kannery <supriyak@in.ibm.com> --- qerror.c | 8 ++++++++ qerror.h | 6 ++++++ 2 files changed, 14 insertions(+) Index: qemu/qerror.c =================================================================== --- qemu.orig/qerror.c +++ qemu/qerror.c @@ -96,6 +96,14 @@ static const QErrorStringTable qerror_ta .error_fmt = QERR_DEVICE_NOT_REMOVABLE, .desc = "Device '%(device)' is not removable", }, + { + .error_fmt = QERR_DEVICE_NOT_INSERTED, + .desc = "Device '%(device)' has not been inserted", + }, + { + .error_fmt = QERR_REOPEN_FILE_FAILED, + .desc = "Could not reopen '%(filename)'", + }, { .error_fmt = QERR_DEVICE_NO_BUS, .desc = "Device '%(device)' has no child bus", Index: qemu/qerror.h =================================================================== --- qemu.orig/qerror.h +++ qemu/qerror.h @@ -85,6 +85,9 @@ QError *qobject_to_qerror(const QObject #define QERR_DEVICE_NOT_FOUND \ "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }" +#define QERR_DEVICE_NOT_INSERTED \ + "{ 'class': 'DeviceNotInserted', 'data': { 'device': %s } }" + #define QERR_DEVICE_NOT_REMOVABLE \ "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }" @@ -139,6 +142,9 @@ QError *qobject_to_qerror(const QObject #define QERR_OPEN_FILE_FAILED \ "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }" +#define QERR_REOPEN_FILE_FAILED \ + "{ 'class': 'ReopenFileFailed', 'data': { 'filename': %s } }" + #define QERR_PROPERTY_NOT_FOUND \ "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }" ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [V3 2/3] Error classes for file reopen and device insertion 2011-06-17 16:37 ` [Qemu-devel] [V3 2/3] Error classes for file reopen and device insertion Supriya Kannery @ 2011-06-20 14:23 ` Kevin Wolf 0 siblings, 0 replies; 11+ messages in thread From: Kevin Wolf @ 2011-06-20 14:23 UTC (permalink / raw) To: Supriya Kannery; +Cc: qemu-devel, Christoph Hellwig Am 17.06.2011 18:37, schrieb Supriya Kannery: > New error classes defined for cases where device not inserted > and file reopen failed. > > Signed-off-by: Supriya Kannery <supriyak@in.ibm.com> This one has tabs, too. Kevin ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change 2011-06-17 16:37 [Qemu-devel] [V3 0/3]block: Dynamically change hostcache setting using new "block_set" command Supriya Kannery 2011-06-17 16:37 ` [Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting Supriya Kannery 2011-06-17 16:37 ` [Qemu-devel] [V3 2/3] Error classes for file reopen and device insertion Supriya Kannery @ 2011-06-17 16:38 ` Supriya Kannery 2011-06-17 17:23 ` [Qemu-devel] [V2 3/3] <Resend> " Supriya Kannery 2011-06-20 14:34 ` [Qemu-devel] [V2 3/3] " Kevin Wolf 2 siblings, 2 replies; 11+ messages in thread From: Supriya Kannery @ 2011-06-17 16:38 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Supriya Kannery, Christoph Hellwig New command "block_set" added for dynamically changing any of the block device parameters. For now, dynamic setting of hostcache params using this command is implemented. Other block device parameters, can be integrated in similar lines. Signed-off-by: Supriya Kannery <supriyak@in.ibm.com> --- block.c | 41 +++++++++++++++++++++++++++++++++++++++++ block.h | 2 ++ blockdev.c | 32 ++++++++++++++++++++++++++++++++ blockdev.h | 1 + hmp-commands.hx | 15 +++++++++++++++ qmp-commands.hx | 30 +++++++++++++++++++++++++++++- 6 files changed, 120 insertions(+), 1 deletion(-) Index: qemu/hmp-commands.hx =================================================================== --- qemu.orig/hmp-commands.hx +++ qemu/hmp-commands.hx @@ -70,6 +70,21 @@ but should be used with extreme caution. resizes image files, it can not resize block devices like LVM volumes. ETEXI + { + .name = "block_set", + .args_type = "device:B,paramname:s,enable:b", + .params = "device paramname enable", + .help = "On/Off block device parameters like hostcache", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_block_set, + }, + +STEXI +@item block_set +@findex block_set +Change block device params (eg:"hostcache"=on/off) while guest is running. +ETEXI + { .name = "eject", Index: qemu/qmp-commands.hx =================================================================== --- qemu.orig/qmp-commands.hx +++ qemu/qmp-commands.hx @@ -693,7 +693,35 @@ Example: EQMP - { + { + .name = "block_set", + .args_type = "device:B,name:s,enable:b", + .params = "device name enable", + .help = "Enable/Disable block device params like hostcache", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_block_set, + }, + +SQMP +block_set +--------- + +Change various block device parameters like hostcache. + +Arguments: + +- "device": the device's ID, must be unique (json-string) +- "name": name of the parameter to be changed" (json-string) +- "enable": value to be set for the parameter, 'true' or 'false' (json-bool) + +Example: + +-> { "execute": "block_set", "arguments": { "device": "ide0-hd0", "name": "hostcache", "enable": true } } +<- { "return": {} } + +EQMP + + { .name = "balloon", .args_type = "value:M", .params = "target", Index: qemu/blockdev.c =================================================================== --- qemu.orig/blockdev.c +++ qemu/blockdev.c @@ -797,3 +797,35 @@ int do_block_resize(Monitor *mon, const return 0; } + + +/* + * Handle changes to block device settings, like hostcache, + * while guest is running. +*/ +int do_block_set(Monitor *mon, const QDict *qdict, QObject **ret_data) +{ + const char *device = qdict_get_str(qdict, "device"); + const char *name = qdict_get_str(qdict, "name"); + int enable = qdict_get_bool(qdict, "enable"); + BlockDriverState *bs; + + bs = bdrv_find(device); + if (!bs) { + qerror_report(QERR_DEVICE_NOT_FOUND, device); + return -1; + } + + if (!(strcmp(name, "hostcache"))) { + if (bdrv_is_inserted(bs)) { + /* cache change applicable only if device inserted */ + return bdrv_change_hostcache(bs, enable); + } else { + qerror_report(QERR_DEVICE_NOT_INSERTED, device); + return -1; + } + } + + return 0; +} + Index: qemu/block.c =================================================================== --- qemu.orig/block.c +++ qemu/block.c @@ -651,6 +651,33 @@ unlink_and_fail: return ret; } +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags) +{ + BlockDriver *drv = bs->drv; + int ret = 0; + + /* No need to reopen as no change in flags */ + if (bdrv_flags == bs->open_flags) + return 0; + + /* Quiesce IO for the given block device */ + qemu_aio_flush(); + bdrv_flush(bs); + + bdrv_close(bs); + ret = bdrv_open(bs, bs->filename, bdrv_flags, drv); + + /* + * A failed attempt to reopen the image file must lead to 'abort()' + */ + if (ret != 0) { + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename); + abort(); + } + + return ret; +} + void bdrv_close(BlockDriverState *bs) { if (bs->drv) { @@ -691,6 +718,20 @@ void bdrv_close_all(void) } } +int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache) +{ + int bdrv_flags = bs->open_flags; + + /* set hostcache flags (without changing WCE/flush bits) */ + if (enable_host_cache) + bdrv_flags &= ~BDRV_O_NOCACHE; + else + bdrv_flags |= BDRV_O_NOCACHE; + + /* Reopen file with changed set of flags */ + return bdrv_reopen(bs, bdrv_flags); +} + /* 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 @@ -71,6 +71,7 @@ void bdrv_delete(BlockDriverState *bs); 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(BlockDriverState *bs, DeviceState *qdev); void bdrv_detach(BlockDriverState *bs, DeviceState *qdev); @@ -96,6 +97,7 @@ void 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.h =================================================================== --- qemu.orig/blockdev.h +++ qemu/blockdev.h @@ -65,5 +65,6 @@ int do_change_block(Monitor *mon, const int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data); int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data); int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data); +int do_block_set(Monitor *mon, const QDict *qdict, QObject **ret_data); #endif ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [V2 3/3] <Resend> Command "block_set" for dynamic block params change 2011-06-17 16:38 ` [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change Supriya Kannery @ 2011-06-17 17:23 ` Supriya Kannery 2011-06-20 14:34 ` [Qemu-devel] [V2 3/3] " Kevin Wolf 1 sibling, 0 replies; 11+ messages in thread From: Supriya Kannery @ 2011-06-17 17:23 UTC (permalink / raw) To: Supriya Kannery; +Cc: Kevin Wolf, qemu-devel, Christoph Hellwig Resending block_set command patch to avoid mismatch between "paramname" defined for human monitor and "name" used in do_block_set for reading name of block device parameter. --- New command "block_set" added for dynamically changing any of the block device parameters. For now, dynamic setting of hostcache params using block_set is implemented. Other block device parameter changes can be integrated in similar lines. Signed-off-by: Supriya Kannery <supriyak@in.ibm.com> --- block.c | 41 +++++++++++++++++++++++++++++++++++++++++ block.h | 2 ++ blockdev.c | 32 ++++++++++++++++++++++++++++++++ blockdev.h | 1 + hmp-commands.hx | 15 +++++++++++++++ qmp-commands.hx | 30 +++++++++++++++++++++++++++++- 6 files changed, 120 insertions(+), 1 deletion(-) Index: qemu/hmp-commands.hx =================================================================== --- qemu.orig/hmp-commands.hx +++ qemu/hmp-commands.hx @@ -70,6 +70,21 @@ but should be used with extreme caution. resizes image files, it can not resize block devices like LVM volumes. ETEXI + { + .name = "block_set", + .args_type = "device:B,name:s,enable:b", + .params = "device name enable", + .help = "On/Off block device parameters like hostcache", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_block_set, + }, + +STEXI +@item block_set +@findex block_set +Change block device params (eg:"hostcache"=on/off) while guest is running. +ETEXI + { .name = "eject", Index: qemu/qmp-commands.hx =================================================================== --- qemu.orig/qmp-commands.hx +++ qemu/qmp-commands.hx @@ -693,7 +693,35 @@ Example: EQMP - { + { + .name = "block_set", + .args_type = "device:B,name:s,enable:b", + .params = "device name enable", + .help = "Enable/Disable block device params like hostcache", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_block_set, + }, + +SQMP +block_set +--------- + +Change various block device parameters like hostcache. + +Arguments: + +- "device": the device's ID, must be unique (json-string) +- "name": name of the parameter to be changed" (json-string) +- "enable": value to be set for the parameter, 'true' or 'false' (json-bool) + +Example: + +-> { "execute": "block_set", "arguments": { "device": "ide0-hd0", "name": "hostcache", "enable": true } } +<- { "return": {} } + +EQMP + + { .name = "balloon", .args_type = "value:M", .params = "target", Index: qemu/blockdev.c =================================================================== --- qemu.orig/blockdev.c +++ qemu/blockdev.c @@ -797,3 +797,35 @@ int do_block_resize(Monitor *mon, const return 0; } + + +/* + * Handle changes to block device settings, like hostcache, + * while guest is running. +*/ +int do_block_set(Monitor *mon, const QDict *qdict, QObject **ret_data) +{ + const char *device = qdict_get_str(qdict, "device"); + const char *name = qdict_get_str(qdict, "name"); + int enable = qdict_get_bool(qdict, "enable"); + BlockDriverState *bs; + + bs = bdrv_find(device); + if (!bs) { + qerror_report(QERR_DEVICE_NOT_FOUND, device); + return -1; + } + + if (!(strcmp(name, "hostcache"))) { + if (bdrv_is_inserted(bs)) { + /* cache change applicable only if device inserted */ + return bdrv_change_hostcache(bs, enable); + } else { + qerror_report(QERR_DEVICE_NOT_INSERTED, device); + return -1; + } + } + + return 0; +} + Index: qemu/block.c =================================================================== --- qemu.orig/block.c +++ qemu/block.c @@ -651,6 +651,33 @@ unlink_and_fail: return ret; } +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags) +{ + BlockDriver *drv = bs->drv; + int ret = 0; + + /* No need to reopen as no change in flags */ + if (bdrv_flags == bs->open_flags) + return 0; + + /* Quiesce IO for the given block device */ + qemu_aio_flush(); + bdrv_flush(bs); + + bdrv_close(bs); + ret = bdrv_open(bs, bs->filename, bdrv_flags, drv); + + /* + * A failed attempt to reopen the image file must lead to 'abort()' + */ + if (ret != 0) { + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename); + abort(); + } + + return ret; +} + void bdrv_close(BlockDriverState *bs) { if (bs->drv) { @@ -691,6 +718,20 @@ void bdrv_close_all(void) } } +int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache) +{ + int bdrv_flags = bs->open_flags; + + /* set hostcache flags (without changing WCE/flush bits) */ + if (enable_host_cache) + bdrv_flags &= ~BDRV_O_NOCACHE; + else + bdrv_flags |= BDRV_O_NOCACHE; + + /* Reopen file with changed set of flags */ + return bdrv_reopen(bs, bdrv_flags); +} + /* 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 @@ -71,6 +71,7 @@ void bdrv_delete(BlockDriverState *bs); 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(BlockDriverState *bs, DeviceState *qdev); void bdrv_detach(BlockDriverState *bs, DeviceState *qdev); @@ -96,6 +97,7 @@ void 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.h =================================================================== --- qemu.orig/blockdev.h +++ qemu/blockdev.h @@ -65,5 +65,6 @@ int do_change_block(Monitor *mon, const int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data); int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data); int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data); +int do_block_set(Monitor *mon, const QDict *qdict, QObject **ret_data); #endif ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change 2011-06-17 16:38 ` [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change Supriya Kannery 2011-06-17 17:23 ` [Qemu-devel] [V2 3/3] <Resend> " Supriya Kannery @ 2011-06-20 14:34 ` Kevin Wolf 2011-06-22 16:09 ` Supriya Kannery 1 sibling, 1 reply; 11+ messages in thread From: Kevin Wolf @ 2011-06-20 14:34 UTC (permalink / raw) To: Supriya Kannery; +Cc: qemu-devel, Christoph Hellwig Am 17.06.2011 18:38, schrieb Supriya Kannery: > New command "block_set" added for dynamically changing any of the block > device parameters. For now, dynamic setting of hostcache params using this > command is implemented. Other block device parameters, can be integrated > in similar lines. > > Signed-off-by: Supriya Kannery <supriyak@in.ibm.com> Coding style is off in this one as well. > Index: qemu/blockdev.c > =================================================================== > --- qemu.orig/blockdev.c > +++ qemu/blockdev.c > @@ -797,3 +797,35 @@ int do_block_resize(Monitor *mon, const > > return 0; > } > + > + > +/* > + * Handle changes to block device settings, like hostcache, > + * while guest is running. > +*/ > +int do_block_set(Monitor *mon, const QDict *qdict, QObject **ret_data) > +{ > + const char *device = qdict_get_str(qdict, "device"); > + const char *name = qdict_get_str(qdict, "name"); > + int enable = qdict_get_bool(qdict, "enable"); > + BlockDriverState *bs; > + > + bs = bdrv_find(device); > + if (!bs) { > + qerror_report(QERR_DEVICE_NOT_FOUND, device); > + return -1; > + } > + > + if (!(strcmp(name, "hostcache"))) { The bracket after ! isn't necessary. > + if (bdrv_is_inserted(bs)) { > + /* cache change applicable only if device inserted */ > + return bdrv_change_hostcache(bs, enable); > + } else { > + qerror_report(QERR_DEVICE_NOT_INSERTED, device); > + return -1; > + } I'm not so sure about this one. Why shouldn't I change the cache mode for a device which is currently? The next thing I want to do could be inserting a medium and using it with the new cache mode. > + } > + > + return 0; > +} > + > Index: qemu/block.c > =================================================================== > --- qemu.orig/block.c > +++ qemu/block.c > @@ -651,6 +651,33 @@ unlink_and_fail: > return ret; > } > > +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags) > +{ > + BlockDriver *drv = bs->drv; > + int ret = 0; > + > + /* No need to reopen as no change in flags */ > + if (bdrv_flags == bs->open_flags) > + return 0; There could be other reasons for reopening besides changing flags, e.g. invalidating cached metadata. > + > + /* Quiesce IO for the given block device */ > + qemu_aio_flush(); > + bdrv_flush(bs); Missing error handling. > + > + bdrv_close(bs); Here, too. > + ret = bdrv_open(bs, bs->filename, bdrv_flags, drv); > + > + /* > + * A failed attempt to reopen the image file must lead to 'abort()' > + */ > + if (ret != 0) { > + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename); > + abort(); > + } Maybe we can retry with the old flags at least before aborting? Also I would like to see a (Linux specific) version that uses the old fd for the reopen, so that we can handle files that aren't accessible with their old name any more. This would mean adding a .bdrv_reopen callback in raw-posix. > + > + return ret; > +} > + > void bdrv_close(BlockDriverState *bs) > { > if (bs->drv) { > @@ -691,6 +718,20 @@ void bdrv_close_all(void) > } > } > > +int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache) > +{ > + int bdrv_flags = bs->open_flags; > + > + /* set hostcache flags (without changing WCE/flush bits) */ > + if (enable_host_cache) > + bdrv_flags &= ~BDRV_O_NOCACHE; > + else > + bdrv_flags |= BDRV_O_NOCACHE; > + > + /* Reopen file with changed set of flags */ > + return bdrv_reopen(bs, bdrv_flags); > +} Hm, interesting. Now we can get a O_DIRECT | O_SYNC mode with the monitor. We should probably expose the same functionality for the command line, too. Kevin ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change 2011-06-20 14:34 ` [Qemu-devel] [V2 3/3] " Kevin Wolf @ 2011-06-22 16:09 ` Supriya Kannery 2011-06-27 8:18 ` Kevin Wolf 0 siblings, 1 reply; 11+ messages in thread From: Supriya Kannery @ 2011-06-22 16:09 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel, Christoph Hellwig On 06/20/2011 08:04 PM, Kevin Wolf wrote: > Am 17.06.2011 18:38, schrieb Supriya Kannery: >> + if (bdrv_is_inserted(bs)) { >> + /* cache change applicable only if device inserted */ >> + return bdrv_change_hostcache(bs, enable); >> + } else { >> + qerror_report(QERR_DEVICE_NOT_INSERTED, device); >> + return -1; >> + } > > I'm not so sure about this one. Why shouldn't I change the cache mode > for a device which is currently? The next thing I want to do could be > inserting a medium and using it with the new cache mode. Uninserted device has no file to be re-opened. So such a check was used to avoid calling bdrv_reopen without a filename. I agree with your point. Hostcache value change is applicable for devices that are not inserted as well. If device is inserted, request for hostcache setting change can lead to change in open_flags and then reopen of file. Otherwise, the request can result in just open_flags updation, which can be used for opening a file when a device gets inserted. Will make the above modification in V3 > >> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags) >> +{ >> + BlockDriver *drv = bs->drv; >> + int ret = 0; >> + >> + /* No need to reopen as no change in flags */ >> + if (bdrv_flags == bs->open_flags) >> + return 0; > > There could be other reasons for reopening besides changing flags, e.g. > invalidating cached metadata. > >> + >> + /* Quiesce IO for the given block device */ >> + qemu_aio_flush(); >> + bdrv_flush(bs); > > Missing error handling. > will add in V3 >> + ret = bdrv_open(bs, bs->filename, bdrv_flags, drv); >> + >> + /* >> + * A failed attempt to reopen the image file must lead to 'abort()' >> + */ >> + if (ret != 0) { >> + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename); >> + abort(); >> + } > > Maybe we can retry with the old flags at least before aborting? > > Also I would like to see a (Linux specific) version that uses the old fd > for the reopen, so that we can handle files that aren't accessible with > their old name any more. This would mean adding a .bdrv_reopen callback > in raw-posix. > Will work on to implement version of re-open using fd. Since for block_set command processing, re-open using filename already works, is it ok to have the 're-open using fd' done as a separate patchset ? >> + /* Reopen file with changed set of flags */ >> + return bdrv_reopen(bs, bdrv_flags); >> +} > > Hm, interesting. Now we can get a O_DIRECT | O_SYNC mode with the > monitor. We should probably expose the same functionality for the > command line, too. > hostcache is indirectly set/reset from qemu commandline, using "cache=" (none/writeback/writethrough/unsafe) option. So should we be having "hostcache=xx" option added to -drive in addition to "cache=" ? > Kevin ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change 2011-06-22 16:09 ` Supriya Kannery @ 2011-06-27 8:18 ` Kevin Wolf 0 siblings, 0 replies; 11+ messages in thread From: Kevin Wolf @ 2011-06-27 8:18 UTC (permalink / raw) To: Supriya Kannery; +Cc: qemu-devel, Christoph Hellwig Am 22.06.2011 18:09, schrieb Supriya Kannery: > On 06/20/2011 08:04 PM, Kevin Wolf wrote: >> Am 17.06.2011 18:38, schrieb Supriya Kannery: > >>> + if (bdrv_is_inserted(bs)) { >>> + /* cache change applicable only if device inserted */ >>> + return bdrv_change_hostcache(bs, enable); >>> + } else { >>> + qerror_report(QERR_DEVICE_NOT_INSERTED, device); >>> + return -1; >>> + } >> >> I'm not so sure about this one. Why shouldn't I change the cache mode >> for a device which is currently? The next thing I want to do could be >> inserting a medium and using it with the new cache mode. > > Uninserted device has no file to be re-opened. So such a check was used > to avoid calling bdrv_reopen without a filename. > > I agree with your point. Hostcache value change is applicable > for devices that are not inserted as well. If device is inserted, > request for hostcache setting change can lead to change in open_flags > and then reopen of file. Otherwise, the request can result in just > open_flags updation, which can be used for opening a file when > a device gets inserted. > > Will make the above modification in V3 > >> >>> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags) >>> +{ >>> + BlockDriver *drv = bs->drv; >>> + int ret = 0; >>> + >>> + /* No need to reopen as no change in flags */ >>> + if (bdrv_flags == bs->open_flags) >>> + return 0; >> >> There could be other reasons for reopening besides changing flags, e.g. >> invalidating cached metadata. >> >>> + >>> + /* Quiesce IO for the given block device */ >>> + qemu_aio_flush(); >>> + bdrv_flush(bs); >> >> Missing error handling. >> > > will add in V3 > > >>> + ret = bdrv_open(bs, bs->filename, bdrv_flags, drv); >>> + >>> + /* >>> + * A failed attempt to reopen the image file must lead to 'abort()' >>> + */ >>> + if (ret != 0) { >>> + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename); >>> + abort(); >>> + } >> >> Maybe we can retry with the old flags at least before aborting? >> >> Also I would like to see a (Linux specific) version that uses the old fd >> for the reopen, so that we can handle files that aren't accessible with >> their old name any more. This would mean adding a .bdrv_reopen callback >> in raw-posix. >> > > Will work on to implement version of re-open using fd. Since for > block_set command processing, re-open using filename already works, is > it ok to have the 're-open using fd' done as a separate patchset ? Sure, it's just an obvious improvement. >>> + /* Reopen file with changed set of flags */ >>> + return bdrv_reopen(bs, bdrv_flags); >>> +} >> >> Hm, interesting. Now we can get a O_DIRECT | O_SYNC mode with the >> monitor. We should probably expose the same functionality for the >> command line, too. >> > > hostcache is indirectly set/reset from qemu commandline, using "cache=" > (none/writeback/writethrough/unsafe) option. So should we be having > "hostcache=xx" option added to -drive in addition to "cache=" ? I think that's the right thing to do in the long run. Christoph? Kevin ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-06-27 8:16 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-06-17 16:37 [Qemu-devel] [V3 0/3]block: Dynamically change hostcache setting using new "block_set" command Supriya Kannery 2011-06-17 16:37 ` [Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting Supriya Kannery 2011-06-20 14:23 ` Kevin Wolf 2011-06-22 14:59 ` Supriya Kannery 2011-06-17 16:37 ` [Qemu-devel] [V3 2/3] Error classes for file reopen and device insertion Supriya Kannery 2011-06-20 14:23 ` Kevin Wolf 2011-06-17 16:38 ` [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change Supriya Kannery 2011-06-17 17:23 ` [Qemu-devel] [V2 3/3] <Resend> " Supriya Kannery 2011-06-20 14:34 ` [Qemu-devel] [V2 3/3] " Kevin Wolf 2011-06-22 16:09 ` Supriya Kannery 2011-06-27 8:18 ` Kevin Wolf
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.