On Wed, Jul 13, 2011 at 06:37:05PM +0530, Supriya Kannery wrote:
> Index: qemu/block.c
> ===================================================================
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -651,6 +651,40 @@ unlink_and_fail:
> return ret;
> }
>
> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
> +{
> + BlockDriver *drv = bs->drv;
> + int ret = 0;
> +
> + /* 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;
> + }
> + bdrv_close(bs);
> +
> +
> + ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
> + if (ret < 0) {
> + /* Reopen failed. Try to open with original flags */
> + error_report("Opening file with changed flags...");
> + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
If the next open fails too then there will be two qerror_report() calls.
This causes a warning and the new qerror is dropped. Please consolidate
the error reporting so there is only one error before returning from
this function.
> +
> + ret = bdrv_open(bs, bs->filename, bs->open_flags, drv);
bs->open_flags has been clobbered by the previous bdrv_open(). It would
be best to take a copy of bs->open_flags before bdrv_close(bs) above.
> + if (ret < 0) {
> + /*
> + * Reopen failed with orig and modified flags
> + */
> + error_report("Opening file with original flags...");
> + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
> + abort();
> + }
> + }
> +
> + return ret;
> +}
> +
> void bdrv_close(BlockDriverState *bs)
> {
> if (bs->drv) {
> @@ -691,6 +725,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;
Can you explain the scenario where this works?
Looking at do_change_block() the flags will be clobbered so saving them
away does not help.
> Index: qemu/blockdev.c
> ===================================================================
> --- qemu.orig/blockdev.c
> +++ qemu/blockdev.c
> @@ -793,3 +793,63 @@ 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;
> +
> + /* 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) {
> + error_report("Invalid parameter %s", driver);
error_report() only works for HMP. Please use qerror_report() so both
HMP and QMP see the error. Same issue further down.
Stefan
