From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36441) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qm3hS-0001f3-RX for qemu-devel@nongnu.org; Wed, 27 Jul 2011 08:58:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qm3hR-0006Ll-5G for qemu-devel@nongnu.org; Wed, 27 Jul 2011 08:58:58 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:64153) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qm3hR-0006LY-1c for qemu-devel@nongnu.org; Wed, 27 Jul 2011 08:58:57 -0400 Received: by yxt3 with SMTP id 3so878943yxt.4 for ; Wed, 27 Jul 2011 05:58:56 -0700 (PDT) Message-ID: <4E300B8E.2020509@codemonkey.ws> Date: Wed, 27 Jul 2011 07:58:54 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <20110727113000.25109.16204.sendpatchset@skannery> <20110727113045.25109.54866.sendpatchset@skannery> In-Reply-To: <20110727113045.25109.54866.sendpatchset@skannery> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Supriya Kannery Cc: Kevin Wolf , Stefan Hajnoczi , qemu-devel@nongnu.org, Christoph Hellwig On 07/27/2011 06:30 AM, Supriya Kannery wrote: > 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 parameter changes, can be > integrated in similar lines. > > Signed-off-by: Supriya Kannery > > --- > block.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ > block.h | 2 + > blockdev.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > blockdev.h | 1 > hmp-commands.hx | 14 ++++++++++++ > qemu-config.c | 13 +++++++++++ > qemu-option.c | 25 ++++++++++++++++++++++ > qemu-option.h | 2 + > qmp-commands.hx | 28 +++++++++++++++++++++++++ > 9 files changed, 200 insertions(+) > > Index: qemu/block.c > =================================================================== > --- qemu.orig/block.c > +++ qemu/block.c > @@ -651,6 +651,34 @@ 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(); > + if (bdrv_flush(bs)) { > + 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_REOPEN_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) > { > if (bs->drv) { > @@ -691,6 +719,32 @@ 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; > + } > + > + /* If no change in flags, no need to reopen */ > + if (bdrv_flags == bs->open_flags) { > + return 0; > + } > + > + if (bdrv_is_inserted(bs)) { > + /* Reopen file with changed set of flags */ > + 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 > @@ -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); > @@ -97,6 +98,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.c > =================================================================== > --- qemu.orig/blockdev.c > +++ qemu/blockdev.c > @@ -793,3 +793,64 @@ 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) > +{ > + BlockDriverState *bs = NULL; > + QemuOpts *opts; > + int enable; > + const char *device, *driver; > + int ret; > + char usage[50]; > + > + /* Validate device */ > + device = qdict_get_str(qdict, "device"); > + bs = bdrv_find(device); > + if (!bs) { > + qerror_report(QERR_DEVICE_NOT_FOUND, device); > + return -1; > + } > + > + opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict); > + if (opts == NULL) { > + return -1; > + } > + > + /* If input not in "param=value" format, display error */ > + driver = qemu_opt_get(opts, "driver"); > + if (driver != NULL) { > + qerror_report(QERR_INVALID_PARAMETER, driver); > + return -1; > + } > + > + /* Before validating parameters, remove "device" option */ > + ret = qemu_opt_delete(opts, "device"); > + if (ret == 1) { > + strcpy(usage,"block_set device [prop=value][,...]"); > + qerror_report(QERR_INCORRECT_COMMAND_SYNTAX, usage); > + return 0; > + } > + > + /* Validate parameters with "-drive" parameter list */ > + ret = qemu_validate_opts(opts, "drive"); > + if (ret == -1) { > + return -1; > + } > + > + /* Check for 'hostcache' parameter */ > + enable = qemu_opt_get_bool(opts, "hostcache", -1); > + if (enable != -1) { > + return bdrv_change_hostcache(bs, enable); > + } else { > + qerror_report(QERR_INVALID_PARAMETER_VALUE, "hostcache","on/off"); > + } > + > + return 0; > + > +} > + > 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 > Index: qemu/hmp-commands.hx > =================================================================== > --- qemu.orig/hmp-commands.hx > +++ qemu/hmp-commands.hx > @@ -70,6 +70,20 @@ 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,device:O", > + .params = "device [prop=value][,...]", > + .help = "Change block device parameters [hostcache=on/off]", > + .user_print = monitor_user_noop, > + .mhandler.cmd_new = do_block_set, > + }, > +STEXI > +@item block_set @var{config} > +@findex block_set > +Change block device parameters (eg: hostcache=on/off) while guest is running. > +ETEXI > + block_set_hostcache() please. Multiplexing commands is generally a bad idea. It weakens typing. In the absence of a generic way to set block device properties, implementing properties as generic in the QMP layer seems like a bad idea to me. Regards, Anthony Liguori