Re: [Qemu-devel] [v2 Patch 1/9]block: Framework for reopening image files safely
On 08/03/2012 01:49 AM, Luiz Capitulino wrote:
> On Tue, 31 Jul 2012 03:04:22 +0530
> Supriya Kannery wrote:
>
>> +
>> +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);
>> +}
>> +
>> +void bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp)
>> +{
>> +BlockDriver *drv = bs->drv;
>> +int ret = 0;
>> +BDRVReopenState *reopen_state = NULL;
>> +
>> +/* Quiesce IO for the given block device */
>> +bdrv_drain_all();
>> +ret = bdrv_flush(bs);
>> +if (ret != 0) {
>> +error_set(errp, QERR_IO_ERROR);
>> +return;
>> +}
>> +
>> +/* 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);
>
> Why do you have to call bdrv_reopen_abort()? I'd expect bdrv_reopen_prepare()
> (to be able) to undo anything it has done.
>
Having separate abort function avoids cluttering of reopen-
prepare(). We wanted to logically separate out preparation, commit and
abort. Same format is followed in implementations at block driver level
as well.
-thanks, Supriya
Re: [Qemu-devel] [v2 Patch 1/9]block: Framework for reopening image files safely
On 08/09/2012 02:43 AM, Jeff Cody wrote: On 07/30/2012 05:34 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. + +bdrv_reopen_commit(bs, reopen_state); +bs->open_flags = bdrv_flags; You also need to set bs->read_only here, like so: bs->read_only = !(bdrv_flags& BDRV_O_RDWR); Sure..will include in updated patch. - thanks, Supriya
Re: [Qemu-devel] [v2 Patch 1/9]block: Framework for reopening image files safely
On 08/09/2012 06:32 PM, Jeff Cody wrote:
> On 08/09/2012 05:20 AM, Kevin Wolf wrote:
>> Am 09.08.2012 06:26, schrieb Jeff Cody:
>>> On 07/30/2012 05:34 PM, Supriya Kannery wrote:
>>>> +
>>>> +void bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp)
>>>> +{
>>>> +BlockDriver *drv = bs->drv;
>>>> +int ret = 0;
>>>> +BDRVReopenState *reopen_state = NULL;
>>>> +
>>>
>>> We should assert if drv is NULL:
>>>
>>> assert(drv != NULL);
>>>
>>>
>>>> +/* Quiesce IO for the given block device */
>>>> +bdrv_drain_all();
>>>> +ret = bdrv_flush(bs);
>>>> +if (ret != 0) {
>>>> +error_set(errp, QERR_IO_ERROR);
>>>> +return;
>>>> +}
>>>> +
>>>
>>> We also need to reopen bs->file, so maybe something like this:
>>>
>>> /* open any file images */
>>> if (bs->file) {
>>> bdrv_reopen(bs->file, bdrv_flags, errp);
>>> if (errp&& *errp) {
>>> goto exit;
>>> }
>>> }
>>>
>>> This will necessitate making the handlers in the raw.c file just stubs
>>> (I'll respond to that patch as well).
>>
>> Doesn't this break the transactional semantics? I think you should only
>> prepare the bs->file reopen here and commit it when committing this one.
>>
>
> Yes, it would, so if everything stayed as it is now, that would be the
> right way to do it... but one thing that would be nice (I also mention
> this in my comments on patch 0/9) is that it become transactional for
> multiple BlockDriverStates to be reopened. That would obviously change
> the interface a bit, so that multiple BDS entries could be queued, but
> then it shouldn't be any different to queue the bs->file as well as the
> bs.
>
> All prepare() stages would happen on queued items, and only
> progress to commit() if all prepare() stages passed, as you mentioned.
Yes, will work on to get the transactional semantics applied to bs->file
reopen as well.
>
> One other thing, and perhaps this is nit-picking some - but the
> prepare() functions all modify the 'live' structures, after backing them
> up into stashes. So, on abort, the structures are restored from the
> stashes, and on commit the stashes are discarded. Conceptually, it
> seems cleaner to this the opposite way - prepare() does it work into
> temporary stashes, and the commit() then copies the data over from the
> stash to the live structure, and abort() would just discard the stashes.
>
>
prepare()/commit() and abort() are from the perspective of new changes
to be tried on respective block driver state. Once block driver is ready for the
state change, then commit() means we are commiting these new changes to driver
state. Similarly abort() means we are aborting these new changes half way
and getting back to old stashed state. This is the intended logic.
- thanks, Supriya
Re: [Qemu-devel] [v2 Patch 2/9]block: raw-posix image file reopen
On 07/31/2012 10:47 PM, Eric Blake wrote: > On 07/30/2012 03:34 PM, Supriya Kannery wrote: >> +s->fd = dup3(raw_rs->stash_s->fd, s->fd, O_CLOEXEC); > > You called it out in your cover letter, but just pointing out that this > is one of the locations that needs a conditional fallback to > dup2/qemu_set_cloexec if dup3 and O_CLOEXEC are missing. > > More importantly, though, you want this to be fcntl(F_DUP_CLOEXEC) and > NOT dup3, so that you are duplicating to the first available fd rather > than accidentally overwriting whatever s->fd happened to be on entry. > Also, where is your error checking that you didn't just assign s->fd to > -1? It looks like your goal here is to stash a copy of the fd, so that > on failure you can then pivot over to your copy. > Will use fcntl(F_DUP_CLOEXEC) in updated patch. >> + >> +*prs =&(raw_rs->reopen_state); >> + >> +/* Flags that can be set using fcntl */ >> +int fcntl_flags = BDRV_O_NOCACHE; > > This variable name is misleading; fcntl_flags in my mind implies O_* not > BDRV_O_*, as we are not guaranteed that they are the same values. Maybe > bdrv_flags is a better name. Or are you trying to list the subset of > BDRV_O flags that can be changed via mapping to the appropriate O_ flag > during fcntl? > From the list of flags in bdrv->openflags (BDRV_O*), only few of them can be changed using fcntl. So to identify those allowed subset, I am using fcntl_flags. Excerpt from man fcntl for F_SETFL: On Linux this command can only change the O_APPEND, O_ASYNC, O_DIRECT, O_NOATIME, and O_NONBLOCK flags. May be naming it fcntl_supported_flags would be better? - thanks, Supriya
Re: [Qemu-devel] [v2 Patch 2/9]block: raw-posix image file reopen
On 08/10/2012 07:15 PM, Corey Bryant wrote:
>
>
> On 07/30/2012 05:34 PM, Supriya Kannery wrote:
>> +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 */
>> + bs->drv->bdrv_close(bs);
>> + ret = bs->drv->bdrv_file_open(bs, bs->filename, flags);
>
> Will this allow the fdset refcount to get to zero? I was hoping your
> patches would prevent that from happening. Perhaps Kevin or Eric can
> weigh in. qemu_open() increments the refcount for an fdset when an fd
> from it is used, and qemu_close() decrements it. I think if you were
> able to perform the open before the close here that refcount wouldn't
> get to zero.
>
Since we are duping the file descriptor before reaching this bdrv_close(),
refcount for fd won't become zero.
- thanks, Supriya
[Qemu-devel] [RFC Patch 0/7]Qemu: Dynamic host pagecache change
For changing host pagecache setting of a running VM, it is important to have a safe way of reopening its image file. Following patchset introduces: * a generic way to reopen image files safely. In this approach, before reopening an image, for each block driver, its state will be stashed. Incase preparation (bdrv_reopen_prepare) for reopening returns success, the stashed state will be cleared (bdrv_reopen_commit) and reopened state will be used further. Incase preparation of reopening returns failure, the state of the driver will be rolled back (bdrv_reopen_abort) to the stashed state. This approach is extended to raw-posix, raw-win32 and vmdk block drivers in this patchset. Once this is reviewed and finalised, I will extend the implementation to other drivers like qcow2, qed etc.. * qmp and hmp command 'block_set_hostcache' using which host pagecache setting for a block device can be changed when the VM is running. * BDRVReopenState, a generic structure which can be extended by each of the block drivers to reopen respective image files. ToDo: * memcpy is used to save driver state. Replace this with copying individual fields of driver state (?) * Extend this implementation to other block drivers. * Build and verify raw-win32 driver changes in windows Earlier discussions related to dynamic change of host pagecache can be found at: http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01482.html New block command added: "block_set_hostcache" -- Sets hostcache parameter for block device while guest is running. Usage: block_set_hostcache = block device = on/off qemu/block.c | 112 + qemu/block.h |5 + qemu/block/raw-posix.c | 74 qemu/block/raw-win32.c | 95 + qemu/block/raw.c | 20 qemu/block/vmdk.c | 80 +-- qemu/block_int.h | 11 qemu/blockdev.c| 26 +++ qemu/blockdev.h|2 qemu/hmp-commands.hx | 14 ++ qemu/hmp.c |2 qemu/qapi-schema.json |4 + qemu/qemu-common.h |1 qemu/qerror.c |8 +++ qemu/qerror.h |6 ++ qemu/qmp-commands.hx | 27 +++ 18 files changed, 474 insertions(+), 13 deletions(-)
[Qemu-devel] [RFC Patch 2/7]Qemu: Error classes for file reopen and data sync failure
New error classes defined for file reopen failure and data
sync error
Signed-off-by: Supriya Kannery
---
qerror.c |8
qerror.h |6 ++
2 files changed, 14 insertions(+)
Index: qemu/qerror.c
===
--- qemu.orig/qerror.c
+++ qemu/qerror.c
@@ -108,6 +108,14 @@ static const QErrorStringTable qerror_ta
.desc = "Device '%(device)' has multiple child busses",
},
{
+.error_fmt = QERR_DATA_SYNC_FAILED,
+.desc = "Syncing of data failed for device '%(device)'",
+},
+{
+.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
@@ -117,6 +117,9 @@ QError *qobject_to_qerror(const QObject
#define QERR_DEVICE_NOT_FOUND \
"{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
+#define QERR_DATA_SYNC_FAILED \
+"{ 'class': 'DataSyncFailed', 'data': { 'device': %s } }"
+
#define QERR_DEVICE_NOT_REMOVABLE \
"{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
@@ -180,6 +183,9 @@ QError *qobject_to_qerror(const QObject
#define QERR_PERMISSION_DENIED \
"{ 'class': 'PermissionDenied', 'data': {} }"
+#define QERR_REOPEN_FILE_FAILED \
+"{ 'class': 'ReopenFileFailed', 'data': { 'filename': %s } }"
+
#define QERR_PROPERTY_NOT_FOUND \
"{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
[Qemu-devel] [RFC Patch 1/7]Qemu: Enhance "info block" to display host cache setting
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
---
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
@@ -423,6 +423,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)
#
@@ -436,7 +438,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
@@ -2285,6 +2285,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
@@ -209,6 +209,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]);
[Qemu-devel] [RFC Patch 3/7]Qemu: Cmd "block_set_hostcache" for dynamic cache change
New command "block_set_hostcache" added for dynamically changing
host pagecache setting of a block device.
Usage:
block_set_hostcache
= block device
= on/off
Example:
(qemu) block_set_hostcache ide0-hd0 off
Signed-off-by: Supriya Kannery
---
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
@@ -808,6 +808,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_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) {
@@ -870,6 +899,33 @@ void bdrv_drain_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 */
+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
@@ -119,6 +119,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);
@@ -160,6 +161,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
@@ -1080,3 +1080,29 @@ 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.
+*/
+int do_block_set_hostcache(Monitor *mon, const QDict *qdict,
+ QObject **ret_data)
+{
+BlockDriverState *bs = NULL;
+int enable;
+const char *device;
+
+/* Validate device */
+device = qdict_get_str(qdict, "device");
+bs = bdrv_find(device);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+/* Read hostcache setting */
+enable = qdict_get_bool(qdict, "option");
+return bdrv_change_hostcache(bs, enable);
+
+}
+
Index: qemu/blockdev.h
===
--- qemu.orig/blockdev.h
+++ qemu/blockdev.h
@@ -62,4 +62,6 @@ void qmp_change_blockdev(const char *dev
bool has_format, const char *format, Error **errp);
void do_commit(Monitor *mon, const QDict *qdict);
int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_block_set_hostcache(Monitor *mon, const QDict *qdict,
+
[Qemu-devel] [RFC Patch 4/7]Qemu: Framework for reopening image files safely
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
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -808,10 +808,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();
@@ -820,17 +842,32 @@ 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_REOPEN_FILE_FAILED, bs->filename);
-ret = bdrv_open(bs, bs->filename, open_flags, drv);
+/* 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_REOPEN_FILE_FAILED, bs->filename);
+return ret;
+}
+
+bdrv_reopen_commit(bs, reopen_state);
+bs->open_flags = bdrv_flags;
+
+} else {
+ open_flags = bs->open_flags;
+ bdrv_close(bs);
+
+ ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
if (ret < 0) {
-/* Reopen failed with orig and modified flags */
-abort();
+/* 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 */
+bs->drv = NULL;
+}
}
}
Index: qemu/block_int.h
===
--- qemu.orig/block_int.h
+++ qemu/block_int.h
@@ -105,6 +105,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);
@@ -299,6 +306,10 @@ struct BlockDriverState {
BlockJob *job;
};
+struct BDRVReopenState {
+BlockDriverState *bs;
+};
+
struct BlockDriverAIOCB {
AIOPool *pool;
BlockDriverState *bs;
Index: qemu/qemu-common.h
===
--- qemu.orig/qemu-common.h
+++ qemu/qemu-common.h
@@ -210,6 +210,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
@@ -120,6 +120,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 bd
[Qemu-devel] [RFC Patch 6/7]Qemu: raw-win32 image file reopen
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
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
+#include
#include
#define FTYPE_FILE 0
#define FTYPE_CD 1
#define FTYPE_HARDDISK 2
+#define WINDOWS_VISTA 6
typedef struct BDRVRawState {
HANDLE hfile;
int type;
char drive_path[16]; /* format: "d:\" */
+DWORD overlapped;
} BDRVRawState;
+typedef struct BDRVRawReopenState {
+BDRVReopenState reopen_state;
+HANDLE stash_hfile;
+DWORD stash_overlapped;
+} BDRVRawReopenState;
+
int qemu_ftruncate64(int fd, int64_t length)
{
LARGE_INTEGER li;
@@ -106,9 +115,96 @@ static int raw_open(BlockDriverState *bs
return -EACCES;
return -1;
}
+s->overlapped = overlapped;
return 0;
}
+static int raw_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+ int flags)
+{
+BDRVRawReopenState *raw_rs = g_malloc0(sizeof(BDRVRawReopenState));
+BDRVRawState *s = bs->opaque;
+int ret = 0;
+OSVERSIONINFO osvi;
+BOOL bIsWindowsVistaorLater;
+
+raw_rs->bs = bs;
+raw_rs->stash_hfile = s->hfile;
+raw_rs->stash_overlapped = s->overlapped;
+*prs = raw_rs;
+
+if (flags & BDRV_O_NOCACHE) {
+s->overlapped |= FILE_FLAG_NO_BUFFERING;
+} else {
+s->overlapped &= ~FILE_FLAG_NO_BUFFERING;
+}
+
+if (!(flags & BDRV_O_CACHE_WB)) {
+s->overlapped |= FILE_FLAG_WRITE_THROUGH;
+} else {
+s->overlapped &= ~FILE_FLAG_WRITE_THROUGH;
+}
+
+ZeroMemory(&osvi, sizeof(OSVERSIONINFO));
+osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
+
+GetVersionEx(&osvi);
+
+if (osvi.dwMajorVersion >= WINDOWS_VISTA) {
+s->hfile = ReOpenFile(raw_rs->stash_hfile, 0, FILE_SHARE_READ,
+ overlapped);
+if (s->hfile == INVALID_HANDLE_VALUE) {
+int err = GetLastError();
+if (err == ERROR_ACCESS_DENIED) {
+ret = -EACCES;
+} else {
+ret = -1;
+}
+}
+} else {
+
+DuplicateHandle(GetCurrentProcess(),
+raw_rs->stash_hfile,
+GetCurrentProcess(),
+&s->hfile,
+0,
+FALSE,
+DUPLICATE_SAME_ACCESS);
+bs->drv->bdrv_close(bs);
+ret = bs->drv->bdrv_open(bs, bs->filename, flags);
+}
+return ret;
+}
+
+
+static void raw_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+BDRVRawReopenState *raw_rs;
+
+raw_rs = container_of(rs, BDRVRawReopenState, reopen_state);
+
+/* clean up stashed handle */
+CloseHandle(raw_rs->stash_hfile);
+g_free(raw_rs);
+
+}
+
+static void raw_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+
+BDRVRawReopenState *raw_rs;
+BDRVRawState *s = bs->opaque;
+
+raw_rs = container_of(rs, BDRVRawReopenState, reopen_state);
+
+if (s->hfile && (s->hfile != INVALID_HANDLE_VALUE)) {
+CloseHandle(s->hfile);
+}
+s->hfile = raw_rs->stash_hfile;
+s->overlapped = raw_rs->stash_overlapped;
+g_free(raw_rs);
+}
+
static int raw_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors)
{
[Qemu-devel] [RFC Patch 7/7]Qemu: vmdk image file reopen
vmdk driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while
changing hostcache flag dynamically is handled here.
Signed-off-by: Supriya Kannery
Index: qemu/block/vmdk.c
===
--- qemu.orig/block/vmdk.c
+++ qemu/block/vmdk.c
@@ -115,6 +115,11 @@ typedef struct VmdkGrainMarker {
uint8_t data[0];
} VmdkGrainMarker;
+typedef struct BDRVVmdkReopenState {
+BDRVReopenState reopen_state;
+BDRVVmdkState *stash_s;
+} BDRVVmdkReopenState;
+
static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
{
uint32_t magic;
@@ -588,7 +593,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 +679,71 @@ 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));
+memcpy(vmdk_rs->stash_s, s, sizeof(BDRVVmdkState));
+s->num_extents = 0;
+s->extents = 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);
+memcpy(s, vmdk_rs->stash_s, sizeof(BDRVVmdkState));
+g_free(vmdk_rs->stash_s);
+g_free(vmdk_rs);
+}
+
static int get_whole_cluster(BlockDriverState *bs,
VmdkExtent *extent,
uint64_t cluster_offset,
@@ -1382,7 +1451,6 @@ static int vmdk_create(const char *filen
if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
return -EINVAL;
}
-printf("vmdk_create\n");
/* Read out options */
while (options && options->name) {
if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
@@ -1517,8 +1585,8 @@ exit:
static void vmdk_close(BlockDriverState *bs)
{
BDRVVmdkState *s = bs->opaque;
-
printf("vmdk_close\n");
+
vmdk_free_extents(bs);
migrate_del_blocker(s->migration_blocker);
@@ -1595,6 +1663,12 @@ static BlockDriver bdrv_vmdk = {
.instance_size = sizeof(BDRVVmdkState),
.bdrv_probe = vmdk_probe,
.bdrv_open = vmdk_open,
+.bdrv_reopen_prepare
+= vmdk_reopen_prepare,
+.bdrv_reopen_commit
+= vmdk_reopen_commit,
+.bdrv_reopen_abort
+= vmdk_reopen_abort,
.bdrv_read = vmdk_co_read,
.bdrv_write = vmdk_co_write,
.bdrv_close = vmdk_close,
[Qemu-devel] [RFC Patch 5/7]Qemu: raw-posix image file reopen
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
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)
{
@@ -109,6 +125,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,6 +136,11 @@ 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);
@@ -279,6 +284,71 @@ 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));
+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;
+}
+printf("O_DIRECT flag\n");
+ret = fcntl_setfl(s->fd, s->open_flags);
+} else {
+
+printf("close and open with new flags\n");
+/* 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));
+g_free(raw_rs->stash_s);
+g_free(raw_rs);
+}
+
/* XXX: use host sector size if necessary with:
#ifdef DIOCGSECTORSIZE
{
@@ -631,6 +701,9 @@ static BlockDriver bdrv_file = {
.instance_size = sizeof(BDRVRawState),
.bdrv_probe = NULL, /* no probe for protocols */
.bdrv_file_open = raw_open,
+.bdrv_reopen_prepare = raw_reopen_prepare,
+.bdrv_reopen_commit = raw_reopen_commit,
+.bdrv_reopen_abort = raw_reopen_abort,
.bdrv_close = raw_close,
.bdrv_create = raw_create,
.bdrv_co_discard = raw_co_discard,
Re: [Qemu-devel] [RFC Patch 5/7]Qemu: raw-posix image file reopen
On 02/02/2012 05:45 AM, Michael Roth wrote:
On 01/31/2012 09:07 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.
+
+ /* 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;
+ }
+ printf("O_DIRECT flag\n");
+ ret = fcntl_setfl(s->fd, s->open_flags);
raw-posix.c:raw_aio_submit() does some extra alignment work if
s->aligned_buf was set due to the image being opened O_DIRECT initially,
not sure what the impact is but probably want to clean that up here.
ok, will check on this
thanks! for reviewing
Supriya
Re: [Qemu-devel] [RFC Patch 2/7]Qemu: Error classes for file reopen and data sync failure
On 02/07/2012 01:26 PM, Stefan Hajnoczi wrote:
On Wed, Feb 01, 2012 at 08:36:28AM +0530, Supriya Kannery wrote:
Index: qemu/qerror.c
===
--- qemu.orig/qerror.c
+++ qemu/qerror.c
@@ -108,6 +108,14 @@ static const QErrorStringTable qerror_ta
.desc = "Device '%(device)' has multiple child busses",
},
{
+.error_fmt = QERR_DATA_SYNC_FAILED,
+.desc = "Syncing of data failed for device '%(device)'",
+},
+{
+.error_fmt = QERR_REOPEN_FILE_FAILED,
+.desc = "Could not reopen '%(filename)'",
+},
The comment in qerror.c says:
"Please keep the entries in alphabetical order.
Use scripts/check-qerror.sh to check."
ok
+{
.error_fmt = QERR_DEVICE_NO_BUS,
.desc = "Device '%(device)' has no child bus",
},
Index: qemu/qerror.h
===
--- qemu.orig/qerror.h
+++ qemu/qerror.h
@@ -117,6 +117,9 @@ QError *qobject_to_qerror(const QObject
#define QERR_DEVICE_NOT_FOUND \
"{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
+#define QERR_DATA_SYNC_FAILED \
+"{ 'class': 'DataSyncFailed', 'data': { 'device': %s } }"
+
Same here:
/*
* QError class list
* Please keep the definitions in alphabetical order.
* Use scripts/check-qerror.sh to check.
*/
ok
Re: [Qemu-devel] [RFC Patch 1/7]Qemu: Enhance "info block" to display host cache setting
On 02/08/2012 05:30 PM, Luiz Capitulino wrote: On Wed, 01 Feb 2012 08:36:14 +0530 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 The day we'll want to refactor 'info block' output is coming... ok :-)
Re: [Qemu-devel] [RFC Patch 3/7]Qemu: Cmd "block_set_hostcache" for dynamic cache change
On 02/08/2012 05:37 PM, Luiz Capitulino wrote:
On Wed, 01 Feb 2012 08:36:41 +0530
Supriya Kannery wrote:
+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);
OPEN_FILE_FAILED is fine.
ok
+/*
+ * Change host page cache setting while guest is running.
+*/
+int do_block_set_hostcache(Monitor *mon, const QDict *qdict,
+ QObject **ret_data)
This is not a QAPI command, please read docs/writing-qmp-commands.txt to know
how to write QMP commands using the QAPI.
fine, will check the doc
-thanks, Supriya
Re: [Qemu-devel] [RFC Patch 5/7]Qemu: raw-posix image file reopen
On 02/08/2012 08:24 PM, Kevin Wolf wrote:
Am 01.02.2012 04:07, schrieb Supriya Kannery:
raw-posix driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while
changing hostcache dynamically is handled here.
+typedef struct BDRVRawReopenState {
+BDRVReopenState reopen_state;
+BDRVRawState *stash_s;
+} BDRVRawReopenState;
See Stefan's comment. If it's possible to save only the fd and maybe one
or two other fields, then we should do that.
Yes, for V1 of this patchset, will look for stashing only those relevant
fields of a driver state wherever possible
+
+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;
+}
+printf("O_DIRECT flag\n");
Debugging leftover?
yes :-), didn't do a proper cleanup as this is RFC
for the stashing approach.
+ret = fcntl_setfl(s->fd, s->open_flags);
+} else {
+
+printf("close and open with new flags\n");
Same here.
V1 will be a clean one !
Kevin
Re: [Qemu-devel] [RFC Patch 6/7]Qemu: raw-win32 image file reopen
On 02/08/2012 08:32 PM, Kevin Wolf wrote:
Am 01.02.2012 04:07, schrieb Supriya Kannery:
win32 driver changes for bdrv_reopen_xx functions to
safely reopen image files. Reopening of image files while
changing hostcache dynamically is handled here.
+
+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;
Returning -1 where -errno is expected is bad (turns out as -EPERM on
Linux, which is misleading). Maybe -EIO here.
ok
-thanks, Supriya
Re: [Qemu-devel] [RFC Patch 4/7]Qemu: Framework for reopening image files safely
On 02/08/2012 08:37 PM, Kevin Wolf wrote:
Am 01.02.2012 04:06, 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.
+} else {
+ open_flags = bs->open_flags;
+ bdrv_close(bs);
+
+ ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
if (ret< 0) {
-/* Reopen failed with orig and modified flags */
-abort();
+/* 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 */
+bs->drv = NULL;
+}
}
Most image formats don't have a bdrv_reopen_* implementation after this
series, so usually you'll have something like qcow2 on top of file. This
code uses bdrv_close/open for the whole stack, even though the file
layer could actually make use of a bdrv_reopen_* implementation and the
qcow2 open isn't likely to fail if the image file could be opened.
I think we can use drv->bdrv_close/open to reopen only one layer and try
using bdrv_reopen_* for the lower layer again.
This is an improvement that can be done in a separate patch, though.
What I understood is, in the enhancement patch, we will have something
like (taking qcow2 as an example)
Implement bdrv_reopen_qcow2(image file) which reopens only the qcow2
image file
Then, drv->bdrv_open(qcow2 driver) will reopen qcow2 driver
=> calls bdrv_reopen_qcow2(qcow2 image file) if image file has
to be reopen
Can you please explain a bit more, it this is not what you meant.
-thanks, Supriya
Re: [Qemu-devel] [RFC Patch 4/7]Qemu: Framework for reopening image files safely
On 02/07/2012 03:38 PM, Stefan Hajnoczi wrote:
On Wed, Feb 01, 2012 at 08:36:58AM +0530, 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.
+int bdrv_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs, int flags)
+{
+ BlockDriver *drv = bs->drv;
+
+ return drv->bdrv_reopen_prepare(bs, prs, flags);
Indentation should be 4 spaces. I suggest configuring your editor so
this is always correct and done automatically.
ok
-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);
+/* Use driver specific reopen() if available */
+if (drv->bdrv_reopen_prepare) {
+ret = bdrv_reopen_prepare(bs,&reopen_state, bdrv_flags);
+ if (ret< 0) {
Indentation
+bdrv_reopen_abort(bs, reopen_state);
+qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+return ret;
+}
+
+bdrv_reopen_commit(bs, reopen_state);
+bs->open_flags = bdrv_flags;
Is bs->open_flags not assigned inside bdrv_reopen_*()?
No, Since it is generic for all the drivers, placed it here.
+
+} else {
+ open_flags = bs->open_flags;
+ bdrv_close(bs);
+
+ ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
if (ret< 0) {
-/* Reopen failed with orig and modified flags */
-abort();
+/* 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 */
+bs->drv = NULL;
This should be a post-condition of bdrv_open(). If you have found a
case where bs->drv != NULL after bdrv_open() fails, then please fix that
code path instead of assigning NULL here.
ok, will check on this
-thanks for reviewing, Supriya
Re: [Qemu-devel] [RFC Patch 5/7]Qemu: raw-posix image file reopen
On 02/07/2012 03:47 PM, Stefan Hajnoczi wrote: On Wed, Feb 01, 2012 at 08:37:12AM +0530, Supriya Kannery wrote: +/* stash state before reopen */ +raw_rs->stash_s = g_malloc0(sizeof(BDRVRawState)); +memcpy(raw_rs->stash_s, s, sizeof(BDRVRawState)); Copying a struct is fragile, Mike Roth pointed out the potential issue with aligned_buf. If raw-posix could open from a given file descriptor as an alternative to opening a filename, then it would be clean and natural to simply re-initialize from the dup'd file descriptor in the abort case. That's the approach I would try instead of stashing the whole struct. Stefan fine, will get V1 with stashing of only required fields wherever possible instead of stashing the full struct.
[Qemu-devel] [RFC Patch 0/3]Qemu: Enable dynamic cache change through qemu monitor
 Currently cache setting of a block device cannot be changed without restarting a running VM. Following patchset is for enabling dynamic change of cache setting for block devices through qemu monitor. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. Monitor command "info block" is extended to display cache setting of block devices. New monitor command "cache_set" is added using which cache setting can be dynamically changed Usage: "cache_set " = block device = "off"/"none", "on"/"writeback", "writethrough", "unsafe" 1/3 Enhance "info block" to display cache setting 2/3 New error classes for file reopen and device insertion 3/3 Add monitor command "cache_set" for dynamic cache change qemu/block.c | 71 ++-- qemu/block.h |2 + qemu/blockdev.c | 20 +++ qemu/blockdev.h |1 qemu/hmp-commands.hx | 14 + qemu/qerror.c|8 +++ qemu/qerror.h|6 + qemu/qmp-commands.hx | 28 ++ 8 files changed, 148 insertions(+), 2 deletions(-) ~
[Qemu-devel] [RFC Patch 1/3]Qemu: Enhance "info block" to display cache setting
Enhance "info block" to display cache setting
Example:
(qemu) info block
ide0-hd0: type=hd removable=0 file=../rhel6-32.qcow2 ro=0 drv=qcow2
encrypted=0
Enhanced to include "cache" setting:
(qemu) info block
ide0-hd0: type=hd removable=0 cache=none file=../rhel6-32.qcow2 ro=0
drv=qcow2 encrypted=0
Signed-off-by: Supriya Kannery
Signed-off-by: Prerna Saxena
---
block.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -1713,6 +1713,19 @@ 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, " cache=none");
+} else if (open_flags & BDRV_O_CACHE_WB) {
+if (open_flags & BDRV_O_NO_FLUSH)
+monitor_printf(mon, " cache=unsafe");
+else
+monitor_printf(mon, " cache=writeback");
+} else
+monitor_printf(mon, " cache=writethrough");
+}
+
if (qdict_haskey(bs_dict, "inserted")) {
QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted"));
@@ -1762,9 +1775,10 @@ void bdrv_info(Monitor *mon, QObject **r
}
bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': %s, "
-"'removable': %i, 'locked': %i }",
+"'removable': %i, 'locked': %i, "
+"'open_flags': %d }",
bs->device_name, type, bs->removable,
-bs->locked);
+bs->locked, bs->open_flags);
if (bs->drv) {
QObject *obj;
[Qemu-devel] [RFC Patch 2/3]Qemu: New error classes for file reopen and device insertion
New errors defined for device insertion and file reopen
Signed-off-by: Supriya Kannery
---
qerror.c |8
qerror.h |6 ++
2 files changed, 14 insertions(+)
Index: qemu/qerror.c
===
--- qemu.orig/qerror.c
+++ qemu/qerror.c
@@ -93,6 +93,10 @@ static const QErrorStringTable qerror_ta
.desc = "Device '%(device)' not found",
},
{
+.error_fmt = QERR_DEVICE_NOT_INSERTED,
+.desc = "Device '%(device)' has not been inserted",
+},
+{
.error_fmt = QERR_DEVICE_NOT_REMOVABLE,
.desc = "Device '%(device)' is not removable",
},
@@ -161,6 +165,10 @@ static const QErrorStringTable qerror_ta
.desc = "Could not open '%(filename)'",
},
{
+.error_fmt = QERR_REOPEN_FILE_FAILED,
+.desc = "Could not reopen '%(filename)'",
+},
+{
.error_fmt = QERR_PROPERTY_NOT_FOUND,
.desc = "Property '%(device).%(property)' not found",
},
Index: qemu/qerror.h
===
--- qemu.orig/qerror.h
+++ qemu/qerror.h
@@ -84,6 +84,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 } }"
@@ -135,6 +138,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 } }"
[Qemu-devel] [RFC Patch 3/3]Qemu: Add command "cache_set" for dynamic cache change
Add monitor command "cache_set" for dynamic cache change
Signed-off-by: Christoph Hellwig
Signed-off-by: Supriya Kannery
---
block.c | 53 +
block.h |2 ++
blockdev.c | 20
blockdev.h |1 +
hmp-commands.hx | 14 ++
qmp-commands.hx | 28
6 files changed, 118 insertions(+)
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 = "cache_set",
+.args_type = "device:B,cache:s",
+.params = "device cache",
+.help = "change cache setting for device",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_cache_set,
+},
+
+STEXI
+@item cache_set
+@findex cache_set
+Change cache options for a block device while guest is running.
+ETEXI
{
.name = "eject",
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -657,6 +657,34 @@ 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) {
@@ -3063,3 +3091,28 @@ out:
return ret;
}
+
+int bdrv_change_cache(BlockDriverState *bs, const char *cache)
+{
+int bdrv_flags = 0;
+
+/* Clear cache flags */
+bdrv_flags = bs->open_flags & ~BDRV_O_CACHE_MASK;
+
+/* Set flags for requested cache setting */
+if (strcmp(cache, "writethrough")) {
+ if (!strcmp(cache, "off") || !strcmp(cache, "none")) {
+bdrv_flags |= BDRV_O_NOCACHE;
+} else if (!strcmp(cache, "writeback") || !strcmp(cache, "on")) {
+bdrv_flags |= BDRV_O_CACHE_WB;
+} else if (!strcmp(cache, "unsafe")) {
+bdrv_flags |= BDRV_O_CACHE_WB;
+bdrv_flags |= BDRV_O_NO_FLUSH;
+} else {
+ error_report("invalid cache option");
+ return -1;
+}
+}
+
+return(bdrv_reopen(bs, bdrv_flags));
+}
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -796,3 +796,23 @@ int do_block_resize(Monitor *mon, const
return 0;
}
+
+int do_cache_set(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+const char *device = qdict_get_str(qdict, "device");
+const char *cache = qdict_get_str(qdict, "cache");
+BlockDriverState *bs;
+
+bs = bdrv_find(device);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+if(bdrv_is_inserted(bs)) {
+ return(bdrv_change_cache(bs, cache));
+} else {
+ qerror_report(QERR_DEVICE_NOT_INSERTED, device);
+ return -1;
+}
+}
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_cache(BlockDriverState *bs, const char *cache);
typedef struct BdrvCheckResult {
Index: qemu/blockdev.h
===
--- qemu.orig/blockdev.h
+++ qemu/blockdev.h
@@ -64,5 +64,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
Re: [Qemu-devel] [RFC Patch 1/3]Qemu: Enhance "info block" to display cache setting
Kevin Wolf wrote:
Am 16.05.2011 20:10, schrieb Supriya Kannery:
Enhance "info block" to display cache setting
Example:
(qemu) info block
ide0-hd0: type=hd removable=0 file=../rhel6-32.qcow2 ro=0 drv=qcow2
encrypted=0
Enhanced to include "cache" setting:
(qemu) info block
ide0-hd0: type=hd removable=0 cache=none file=../rhel6-32.qcow2 ro=0
drv=qcow2 encrypted=0
bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': %s, "
-"'removable': %i, 'locked': %i }",
+"'removable': %i, 'locked': %i, "
+"'open_flags': %d }",
bs->device_name, type, bs->removable,
-bs->locked);
+bs->locked, bs->open_flags);
if (bs->drv) {
QObject *obj;
bs->open_flags is a purely internal thing and its meaning is not
guaranteed to be stable. Exposing it to the user is wrong.
ok. Pls suggest what could a better approach to expose the cache setting.
Kevin
Re: [Qemu-devel] [RFC Patch 0/3]Qemu: Enable dynamic cache change through qemu monitor
Christoph Hellwig wrote: Why are you even trying this again? Enabling control of cache setting from qemu monitor will help users/admins to accomplish cache value change without depending on the guest. As explained very clearly last time you can't change from a writeback-style to a write-through style I/O from the monitor without creating massive data integrity problems. See my patchset that allows changing this from the guest for how it should be done - I just need to get back and revisit the virtio protocol support for it. ok, sure, I will go through your related patches and work further on this.
Re: [Qemu-devel] [RFC Patch 0/3]Qemu: Enable dynamic cache change through qemu monitor
Anthony Liguori wrote: On 05/16/2011 03:23 PM, Christoph Hellwig wrote: Why are you even trying this again? As explained very clearly last time you can't change from a writeback-style to a write-through style I/O from the monitor without creating massive data integrity problems. To further clarify: Today cache=none|writethrough|writeback does two things. It: 1) Changes the WCE flag that's visible to the guest 2) Determines whether the host page cache is used for doing guest I/O As Christoph is very correct in pointing out, we cannot change (1) at run time because this is guest visible. You will break a guest if you do this. ok But it's still desirable to be able to change (2) at run time. Before we can do this properly though, we need to separate out the logic for setting (1) vs. (2). Will go through the code in detail to understand handling of (1) and (2). And ideally, we would allow (1) to be changed by the guest itself at run time which allows for full dynamic control. This is what he's referring to below. Regards, Anthony Liguori See my patchset that allows changing this from the guest for how it should be done - I just need to get back and revisit the virtio protocol support for it.
[Qemu-devel] [V2 0/2]Qemu: Enable dynamic hostcache change through monitor
 Currently host page cache setting for a block device cannot be changed without restarting a running VM. Following patchset [V2] is for enabling dynamic change of host cache setting for devices through qemu monitor. Changes from patchset V1: 1. Support of dynamic cache change only for hostcache. 2. Monitor command "hostcache_get" added to display current cache setting 3. Backed off the changes for display of cache setting in "info block" Two new monitor commands added. 1. hostcache_get -- Displays hostcache setting of a block device. Usage: "hostcache_get = block device 2. hostcache_set -- Sets hostcache for the device to specified value. Usage: "hostcache_set " = block device = "on"/"off" 1/2 New error classes for file reopen and device insertion 2/2 Add commands "hostcache_set" and "hostcache_get" block.c | 48 block.h |2 ++ blockdev.c | 48 blockdev.h |2 ++ hmp-commands.hx | 29 + qerror.c|8 qerror.h|6 ++ qmp-commands.hx | 55 +++ 8 files changed, 198 insertions(+) ~ ~ ~
[Qemu-devel] [V2 2/2]Qemu: Add commands "hostcache_set" and "hostcache_get"
Monitor commands "hostcache_set" and "hostcache_get" added for dynamic
host cache change and display of host cache setting respectively.
Signed-off-by: Supriya Kannery
---
block.c | 48
block.h |2 ++
blockdev.c | 48
blockdev.h |2 ++
hmp-commands.hx | 29 +
qmp-commands.hx | 55 +++
6 files changed, 184 insertions(+)
Index: qemu/hmp-commands.hx
===
--- qemu.orig/hmp-commands.hx
+++ qemu/hmp-commands.hx
@@ -70,6 +70,35 @@ but should be used with extreme caution.
resizes image files, it can not resize block devices like LVM volumes.
ETEXI
+{
+.name = "hostcache_get",
+.args_type = "device:B",
+.params = "device",
+.help = "retrieve host cache settings for device",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_hostcache_get,
+},
+
+STEXI
+@item hostcache_get
+@findex hostcache_get
+Display host cache settings for a block device while guest is running.
+ETEXI
+
+{
+.name = "hostcache_set",
+.args_type = "device:B,hostcache:s",
+.params = "device hostcache",
+.help = "change host cache setting for device",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_hostcache_set,
+},
+
+STEXI
+@item hostcache_set
+@findex hostcache_set
+Change host cache options for a block device while guest is running.
+ETEXI
{
.name = "eject",
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -657,6 +657,34 @@ 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) {
@@ -3049,3 +3077,23 @@ out:
return ret;
}
+
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache)
+{
+int bdrv_flags = bs->open_flags;
+
+/* No change in existing hostcache setting */
+if(!enable_host_cache == (bdrv_flags & BDRV_O_NOCACHE)) {
+return -1;
+}
+
+/* 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));
+}
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -796,3 +796,51 @@ int do_block_resize(Monitor *mon, const
return 0;
}
+
+int do_hostcache_get(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+const char *device = qdict_get_str(qdict, "device");
+BlockDriverState *bs;
+
+bs = bdrv_find(device);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+monitor_printf(mon, "%s:", device);
+
+if (bs->open_flags & BDRV_O_NOCACHE) {
+monitor_printf(mon, " hostcache=off\n");
+} else {
+monitor_printf(mon, " hostcache=on\n");
+}
+
+return 0;
+}
+
+int do_hostcache_set(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+const char *device = qdict_get_str(qdict, "device");
+const char *hostcache = qdict_get_str(qdict, "hostcache");
+BlockDriverState *bs;
+bool enable_host_cache = 0;
+
+bs = bdrv_find(device);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+/* cache change applicable only if device inserted */
+if (bdrv_is_inserted(bs)) {
+if (!strcmp(hostcache,"on"))
+enable_host_cache = 1;
+else
+enable_host_cache = 0;
+return(bdrv_change_hostcache(bs, enable_host_cache));
+} else {
+ qerror_report(QERR_DEVICE_NOT_INSERTED, device);
+ return -1;
+}
+}
Index: qemu/block.h
==
[Qemu-devel] [V2 1/2]Qemu: New error classes for file reopen and device insertion
New errors defined for device insertion and file reopen
Signed-off-by: Supriya Kannery
---
qerror.c |8
qerror.h |6 ++
2 files changed, 14 insertions(+)
Index: qemu/qerror.c
===
--- qemu.orig/qerror.c
+++ qemu/qerror.c
@@ -93,6 +93,10 @@ static const QErrorStringTable qerror_ta
.desc = "Device '%(device)' not found",
},
{
+.error_fmt = QERR_DEVICE_NOT_INSERTED,
+.desc = "Device '%(device)' has not been inserted",
+},
+{
.error_fmt = QERR_DEVICE_NOT_REMOVABLE,
.desc = "Device '%(device)' is not removable",
},
@@ -161,6 +165,10 @@ static const QErrorStringTable qerror_ta
.desc = "Could not open '%(filename)'",
},
{
+.error_fmt = QERR_REOPEN_FILE_FAILED,
+.desc = "Could not reopen '%(filename)'",
+},
+{
.error_fmt = QERR_PROPERTY_NOT_FOUND,
.desc = "Property '%(device).%(property)' not found",
},
Index: qemu/qerror.h
===
--- qemu.orig/qerror.h
+++ qemu/qerror.h
@@ -84,6 +84,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 } }"
@@ -135,6 +138,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 } }"
Re: [Qemu-devel] [RFC Patch 0/3]Qemu: Enable dynamic cache change through qemu monitor
On 05/17/2011 09:11 PM, Christoph Hellwig wrote: On Mon, May 16, 2011 at 04:10:21PM -0500, Anthony Liguori wrote: To further clarify: Today cache=none|writethrough|writeback does two things. It: 1) Changes the WCE flag that's visible to the guest 2) Determines whether the host page cache is used for doing guest I/O As Christoph is very correct in pointing out, we cannot change (1) at run time because this is guest visible. You will break a guest if you do this. But it's still desirable to be able to change (2) at run time. Before we can do this properly though, we need to separate out the logic for setting (1) vs. (2). And ideally, we would allow (1) to be changed by the guest itself at run time which allows for full dynamic control. This is what he's referring to below. Exactly. Setting/clearing the BDRV_O_NO_FLUSH also seems useful, maybe in addition to also allowing an equivalent for the writethrough modes. Posted second version of the patchset (RFC) which supports only hostcache setting/clearing from qemu monitor. http://www.mail-archive.com/qemu-devel@nongnu.org/msg64658.html Please comment.
Re: [Qemu-devel] [V2 2/2]Qemu: Add commands "hostcache_set" and "hostcache_get"
On 05/20/2011 01:50 PM, Stefan Hajnoczi wrote:
On Thu, May 19, 2011 at 10:38:03PM +0530, Supriya Kannery wrote:
Monitor commands "hostcache_set" and "hostcache_get" added for dynamic
host cache change and display of host cache setting respectively.
A generic command for changing block device options would be nice,
althought I don't see other options where it makes sense to change them
at runtime.
The alternative would be:
block_set hostcache on
"block_set", {"device": "ide1-cd0", "name": "hostcache", "enable": true}
The hostcache_get information would be part of query-block output:
{
"device":"ide0-hd0",
"locked":false,
"removable":false,
"inserted":{
"ro":false,
"drv":"qcow2",
"encrypted":false,
"file":"disks/test.img"
"hostcache":true,
},
"type":"hd"
},
This approach is extensible if more options need to be exposed.
Sure, I will resubmit this patchset, after making this feature more
generic. Can you pls help finding atleast one or two options (other than
hostcache) which can be changed dynamically. This will help me evaluate
the generic approach.
+.params = "device",
+.help = "retrieve host cache settings for device",
Please make it clear these operations affect block devices:
"for block device"
ok
+/*
+ * A failed attempt to reopen the image file must lead to 'abort()'
+ */
+if (ret != 0) {
+qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+abort();
The error is never reported on a QMP monitor because qerror_report()
simply stashes away the qerror. The QMP client doesn't have a chance to
read the error before QEMU terminates.
Verified this from qemu command line and the error got displayed before
aborting (when the image file was renamed while VM was running).
+
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache)
Consistently using "hostcache" or "host_cache" would be nice.
ok
+return -1;
This shouldn't be a failure and please don't use -1 when a negative
errno indicates failure. -1 == -EPERM. The return value should be 0
here.
+}
Anyway, this whole check is unnecessary since bdrv_reopen() already
performs it.
will take this off
Please run scripts/checkpatch.pl before submitting patches.
ok..will do
+
+Arguments:
+
+- "device": the device's ID, must be unique (json-string)
+- "cache": host cache value "off" or "on" (json-string)
There is a boolean value that can be used instead of string on|off. See
the set_link command.
ok
Stefan
[Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting
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
---
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,
[Qemu-devel] [V3 0/3]block: Dynamically change hostcache setting using new "block_set" command
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 = block device = parameter (say, "hostcache") = 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(-) ~ ~ ~ ~
[Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change
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
---
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)
[Qemu-devel] [V3 2/3] Error classes for file reopen and device insertion
New error classes defined for cases where device not inserted
and file reopen failed.
Signed-off-by: Supriya Kannery
---
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 } }"
Re: [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change
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
---
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
Re: [Qemu-devel] [V4 Patch 3/4 - Updated]Qemu: Command "block_set" for dynamic block params change
On 07/25/2011 12:00 PM, Stefan Hajnoczi wrote:
On Wed, Jul 13, 2011 at 06:37:05PM +0530, Supriya Kannery wrote:
+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.
ok
+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.
Intention here is to use the changed hostcache setting during device
insertion and there after. To apply this to 'change' command (during
insertion of a device), will need to make the following code changes
in do_change_block.
+
+bdrv_flags = bs->open_flags;
+bdrv_flags |= bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
if (bdrv_open(bs, filename, bdrv_flags, drv) < 0) {
qerror_report(QERR_OPEN_FILE_FAILED, filename);
Need to track bs->open_flags a bit more to see what other changes
needed to ensure usage of changed hostcache setting until user
initiates next hostcache change explicitly for that drive.
Please suggest... should we be saving the hostcache change for
future use or just inhibit user from changing hostcache setting
for an empty drive?
+/* 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.
ok..will change error_report to qerror_report.
Will make further code changes into patchset version 5
Re: [Qemu-devel] [V4 Patch 3/4 - Updated]Qemu: Command "block_set" for dynamic block params change
On 07/25/2011 07:04 PM, Kevin Wolf wrote:
Am 25.07.2011 14:50, schrieb Stefan Hajnoczi:
On Mon, Jul 25, 2011 at 1:52 PM, Supriya Kannery wrote:
On 07/25/2011 12:00 PM, Stefan Hajnoczi wrote:
On Wed, Jul 13, 2011 at 06:37:05PM +0530, Supriya Kannery wrote:
+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.
Intention here is to use the changed hostcache setting during device
insertion and there after. To apply this to 'change' command (during
insertion of a device), will need to make the following code changes
in do_change_block.
+
+bdrv_flags = bs->open_flags;
+bdrv_flags |= bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
if (bdrv_open(bs, filename, bdrv_flags, drv)< 0) {
qerror_report(QERR_OPEN_FILE_FAILED, filename);
Need to track bs->open_flags a bit more to see what other changes
needed to ensure usage of changed hostcache setting until user
initiates next hostcache change explicitly for that drive.
Please suggest... should we be saving the hostcache change for
future use or just inhibit user from changing hostcache setting
for an empty drive?
The 'change' command does not support any cache= options today. It
always opens the new image with cache=writethrough semantics.
This seems like a bug in 'change' to me. We should preserve cache=
settings across change or at least provide a way to specify them as
arguments.
Perhaps your existing code is fine. When 'change' is fixed or
replaced then 'block_set' will work across 'change' too.
Kevin: Thoughts?
I'm not sure if I would say it's a bug. Probably preserving would be
more useful in most cases, but using the default cache mode doesn't
appear completely wrong either. If you like to change it, I'm not
opposed to it.
Kevin
ok, so for now, will retain the code that saves hostcache setting
for empty drives. When other commands (like 'change') are focused,
can look at using this saved setting.
Will post V5 with other error_report related code changes.
thanks, Supriya
[Qemu-devel] [V5 Patch 1/4]Qemu: Enhance "info block" to display host cache setting
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
---
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
@@ -1713,6 +1713,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=0");
+else
+monitor_printf(mon, " hostcache=1");
+}
+
if (qdict_haskey(bs_dict, "inserted")) {
QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted"));
@@ -1749,13 +1757,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
@@ -1131,6 +1131,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)
@@ -1152,6 +1153,7 @@ Example:
{
"device":"ide0-hd0",
"locked":false,
+"hostcache":false,
"removable":false,
"inserted":{
"ro":false,
[Qemu-devel] [V5 Patch 2/4]Qemu: qerrors for file reopen, data sync and cmd syntax
New error classes defined for file reopen failure, data
sync error and incorrect command syntax
Signed-off-by: Supriya Kannery
---
qerror.c | 12
qerror.h |8
2 files changed, 20 insertions(+)
Index: qemu/qerror.c
===
--- qemu.orig/qerror.c
+++ qemu/qerror.c
@@ -97,6 +97,14 @@ static const QErrorStringTable qerror_ta
.desc = "Device '%(device)' is not removable",
},
{
+.error_fmt = QERR_DATA_SYNC_FAILED,
+.desc = "Syncing of data failed for device '%(device)'",
+},
+{
+.error_fmt = QERR_REOPEN_FILE_FAILED,
+.desc = "Could not reopen '%(filename)'",
+},
+{
.error_fmt = QERR_DEVICE_NO_BUS,
.desc = "Device '%(device)' has no child bus",
},
@@ -230,6 +238,10 @@ static const QErrorStringTable qerror_ta
.error_fmt = QERR_QGA_COMMAND_FAILED,
.desc = "Guest agent command failed, error was '%(message)'",
},
+{
+.error_fmt = QERR_INCORRECT_COMMAND_SYNTAX,
+.desc = "Usage: %(syntax)",
+},
{}
};
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_DATA_SYNC_FAILED \
+"{ 'class': 'DataSyncFailed', 'data': { 'device': %s } }"
+
#define QERR_DEVICE_NOT_REMOVABLE \
"{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
@@ -142,6 +145,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 } }"
@@ -193,4 +199,6 @@ QError *qobject_to_qerror(const QObject
#define QERR_QGA_COMMAND_FAILED \
"{ 'class': 'QgaCommandFailed', 'data': { 'message': %s } }"
+#define QERR_INCORRECT_COMMAND_SYNTAX \
+"{ 'class': 'IncorrectCommandSyntax', 'data': { 'syntax': %s } }"
#endif /* QERROR_H */
[Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
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][,...]");
+q
[Qemu-devel] [V5 Patch 4/4]Qemu: Add commandline -drive option 'hostcache'
qemu command option 'hostcache' added to -drive for block devices.
While starting a VM from qemu commandline, this option can be used
for setting host cache usage for block data access. It is not
allowed to specify both 'hostcache' and 'cache' options in the same
commandline. User has to specify only one among these.
Signed-off-by: Supriya Kannery
---
blockdev.c | 13 +
qemu-config.c |4
qemu-options.hx |2 +-
3 files changed, 18 insertions(+), 1 deletion(-)
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -238,6 +238,7 @@ DriveInfo *drive_init(QemuOpts *opts, in
DriveInfo *dinfo;
int snapshot = 0;
int ret;
+int hostcache = 0;
translation = BIOS_ATA_TRANSLATION_AUTO;
media = MEDIA_DISK;
@@ -320,7 +321,19 @@ DriveInfo *drive_init(QemuOpts *opts, in
}
}
+if ((hostcache = qemu_opt_get_bool(opts, "hostcache", -1)) != -1) {
+if (!hostcache) {
+bdrv_flags |= BDRV_O_NOCACHE;
+} else {
+bdrv_flags &= ~BDRV_O_NOCACHE;
+}
+}
+
if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
+if (hostcache != -1) {
+error_report("'hostcache' and 'cache' cannot co-exist");
+return NULL;
+}
if (!strcmp(buf, "off") || !strcmp(buf, "none")) {
bdrv_flags |= BDRV_O_NOCACHE | BDRV_O_CACHE_WB;
} else if (!strcmp(buf, "writeback")) {
Index: qemu/qemu-options.hx
===
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -135,7 +135,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
" [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
" [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
" [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
-" [,readonly=on|off]\n"
+" [,readonly=on|off][,hostcache=on|off]\n"
"use 'file' as a drive image\n", QEMU_ARCH_ALL)
STEXI
@item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
Index: qemu/qemu-config.c
===
--- qemu.orig/qemu-config.c
+++ qemu/qemu-config.c
@@ -84,6 +84,10 @@ static QemuOptsList qemu_drive_opts = {
.name = "readonly",
.type = QEMU_OPT_BOOL,
.help = "open drive file as read-only",
+},{
+.name = "hostcache",
+.type = QEMU_OPT_BOOL,
+.help = "set or reset hostcache (on/off)"
},
{ /* end of list */ }
},
[Qemu-devel] [V5 Patch 0/4]Qemu: Set host cache from cmdline and monitor
Currently cache setting of a block device cannot be changed without restarting a running VM. Following patchset is for enabling dynamic change of host cache setting for block devices through qemu monitor. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. This patchset introduces monitor command 'block_set' using which various parameters for block devices can be changed dynamically. Dynamic changing of host cache setting is implemented for now. This patchset also introduces 'hostcache', a new option for setting host cache from qemu command line. 'Hostcache and 'cache' options cannot be used simultaneously from commandline. Changes from patchset V4: 1. Defined a new qerror class for incorrect command syntax. 2. Changed some of the error_report() calls to qerror_report() calls New block command added: "block_set" -- Sets block device parameters while guest is running. Usage: block_set = block device = parameter (say, "hostcache") = on/off New 'hostcache' option added to -drive: -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" " [,readonly=on|off][,hostcache=on|off]\n" 1/4 Enhance "info block" to display hostcache setting 2/4 New qerrors for file reopen, data sync and command syntax 3/4 Command "block_set" for dynamic params change for block device 4/4 'hostcache' option added to -drive in qemu commandline qemu/block.c | 75 + qemu/block.h |2 + qemu/blockdev.c | 74 +++ qemu/blockdev.h |1 qemu/hmp-commands.hx | 14 +++ qemu/qemu-config.c | 17 ++ qemu/qemu-option.c | 25 qemu/qemu-option.h |2 + qemu/qemu-options.hx |2 - qemu/qerror.c| 12 ++ qemu/qerror.h|8 ++ qemu/qmp-commands.hx | 30 +++ 12 files changed, 257 insertions(+), 2 deletions(-) ~
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On 07/27/2011 09:32 PM, Anthony Liguori wrote:
On 07/27/2011 09:31 AM, Stefan Hajnoczi wrote:
On Wed, Jul 27, 2011 at 1:58 PM, Anthony
Liguori wrote:
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.
The idea behind block_set was to have a unified interface for changing
block device parameters at runtime. This prevents us from reinventing
new commands from scratch. For example, block I/O throttling is
already queued up to add run-time parameters.
Without a unified command we have a bulkier QMP/HMP interface,
duplicated code, and possibly inconsistencies in syntax between the
commands. Isn't the best way to avoid these problems a unified
interface?
I understand the lack of type safety concern but in this case we
already have to manually pull parsed arguments (i.e. cast to specific
types and deal with invalid input). To me this is a reason *for*
using a unified interface like block_set.
Think about it from a client perspective. How do I determine which
properties are supported by this version of QEMU? I have no way to
identify programmatically what arguments are valid for block_set.
User can programmatically find out valid parameters for
block_set. Internally, validation of prop=value is done against -drive
parameter list and then, only the valid/implemented commands (for now,
hostcache) are accepted from that list. Please find execution output
attached:
(qemu) info block
ide0-hd0: removable=0 hostcache=1 file=../rhel6-32.qcow2 ro=0 drv=qcow2
encrypted=0
floppy0: removable=1 locked=0 hostcache=0 file=test.img ro=0 drv=raw
encrypted=0
ide1-cd0: removable=1 locked=0 hostcache=1 [not inserted]
sd0: removable=1 locked=0 hostcache=1 [not inserted]
(qemu) block
block_resize block_set block_passwd
(qemu) block_set
block_set: block device name expected
(qemu) block
block_resize block_set block_passwd
(qemu) help block_set
block_set device [prop=value][,...] -- Change block device parameters
[hostcache=on/off]
(qemu) block_set ide
Device 'ide' not found
(qemu) block_set ide0-hd0
Usage: block_set device [prop=value][,...]
(qemu) block_set ide0-hd0 cache
Invalid parameter 'cache'
(qemu) block_set ide0-hd0 cache=on
Parameter 'hostcache' expects on/off
(qemu) block_set ide0-hd0 hostcache=on
(qemu) block_set ide0-hd0 hostcache=off
(qemu) info block
ide0-hd0: removable=0 hostcache=0 file=../rhel6-32.qcow2 ro=0 drv=qcow2
encrypted=0
floppy0: removable=1 locked=0 hostcache=0 file=test.img ro=0 drv=raw
encrypted=0
ide1-cd0: removable=1 locked=0 hostcache=1 [not inserted]
sd0: removable=1 locked=0 hostcache=1 [not inserted]
When we add further parameters, "usage" string can be enhanced to
include those parameters for informing user.
- Thanks, Supriya
OTOH, if you have strong types like block_set_hostcache, query-commands
tells me exactly what's supported.
Regards,
Anthony Liguori
Stefan
Re: [Qemu-devel] [V5 Patch 1/4]Qemu: Enhance "info block" to display host cache setting
On 07/27/2011 07:49 PM, Stefan Hajnoczi wrote:
On Wed, Jul 27, 2011 at 12:30 PM, Supriya Kannery
wrote:
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
This description is outdated, should be hostcache=1 instead of hostcache=true.
@@ -1749,13 +1757,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");
Please use the same bool-from-int format specifier ('%i') as the other
fields for consistency. The value can be !(bs->open_flags&
BDRV_O_NOCACHE).
ok
+
+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 }",
My copy of the code has the following a few lines further down:
qdict_put_obj(bs_dict, "inserted", obj);
Removing bs_dict would break the build. Am I missing something?
qdict_put_obj() for "inserted" is still there further down in the code.
Here,
QDict *bs_dict = qobject_to_qdict(bs_obj);
is moved up to enable placing of "open_flags" into the dict. We need
"open_flags" in dict to calculate hostcache value for printing in
monitor. Code for placing "inserted" is not touched.
Index: qemu/qmp-commands.hx
===
--- qemu.orig/qmp-commands.hx
+++ qemu/qmp-commands.hx
@@ -1131,6 +1131,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)
Let's explain what "hostcache" means:
- "hostcache": true if host page cache is enabled, false otherwise (json-bool)
ok
-Thanks, Supriya
Stefan
[Qemu-devel] [Patch] virtio: security patch
For security purpose, convert 'int i' to 'unsigned int i' in
virtio functions, so that range of index is restricted
to positive value.
Signed-off-by: Supriya Kannery (supri...@linux.vnet.ibm.com)
---
hw/virtio.c | 27 +--
hw/virtio.h |3 ++-
2 files changed, 19 insertions(+), 11 deletions(-)
Index: qemu/hw/virtio.c
===
--- qemu.orig/hw/virtio.c
+++ qemu/hw/virtio.c
@@ -101,28 +101,32 @@ static void virtqueue_init(VirtQueue *vq
VIRTIO_PCI_VRING_ALIGN);
}
-static inline uint64_t vring_desc_addr(target_phys_addr_t desc_pa, int i)
+static inline uint64_t vring_desc_addr(target_phys_addr_t desc_pa,
+ unsigned int i)
{
target_phys_addr_t pa;
pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, addr);
return ldq_phys(pa);
}
-static inline uint32_t vring_desc_len(target_phys_addr_t desc_pa, int i)
+static inline uint32_t vring_desc_len(target_phys_addr_t desc_pa,
+ unsigned int i)
{
target_phys_addr_t pa;
pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, len);
return ldl_phys(pa);
}
-static inline uint16_t vring_desc_flags(target_phys_addr_t desc_pa, int i)
+static inline uint16_t vring_desc_flags(target_phys_addr_t desc_pa,
+unsigned int i)
{
target_phys_addr_t pa;
pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, flags);
return lduw_phys(pa);
}
-static inline uint16_t vring_desc_next(target_phys_addr_t desc_pa, int i)
+static inline uint16_t vring_desc_next(target_phys_addr_t desc_pa,
+ unsigned int i)
{
target_phys_addr_t pa;
pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, next);
@@ -143,7 +147,7 @@ static inline uint16_t vring_avail_idx(V
return lduw_phys(pa);
}
-static inline uint16_t vring_avail_ring(VirtQueue *vq, int i)
+static inline uint16_t vring_avail_ring(VirtQueue *vq, unsigned int i)
{
target_phys_addr_t pa;
pa = vq->vring.avail + offsetof(VRingAvail, ring[i]);
@@ -155,14 +159,16 @@ static inline uint16_t vring_used_event(
return vring_avail_ring(vq, vq->vring.num);
}
-static inline void vring_used_ring_id(VirtQueue *vq, int i, uint32_t val)
+static inline void vring_used_ring_id(VirtQueue *vq, unsigned int i,
+ uint32_t val)
{
target_phys_addr_t pa;
pa = vq->vring.used + offsetof(VRingUsed, ring[i].id);
stl_phys(pa, val);
}
-static inline void vring_used_ring_len(VirtQueue *vq, int i, uint32_t val)
+static inline void vring_used_ring_len(VirtQueue *vq, unsigned int i,
+ uint32_t val)
{
target_phys_addr_t pa;
pa = vq->vring.used + offsetof(VRingUsed, ring[i].len);
@@ -334,10 +340,11 @@ static unsigned virtqueue_next_desc(targ
return next;
}
-int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes)
+int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
+ unsigned int out_bytes)
{
unsigned int idx;
-int total_bufs, in_total, out_total;
+unsigned int total_bufs, in_total, out_total;
idx = vq->last_avail_idx;
@@ -345,7 +352,7 @@ int virtqueue_avail_bytes(VirtQueue *vq,
while (virtqueue_num_heads(vq, idx)) {
unsigned int max, num_bufs, indirect = 0;
target_phys_addr_t desc_pa;
-int i;
+unsigned int i;
max = vq->vring.num;
num_bufs = total_bufs;
Index: qemu/hw/virtio.h
===
--- qemu.orig/hw/virtio.h
+++ qemu/hw/virtio.h
@@ -156,7 +156,8 @@ void virtqueue_fill(VirtQueue *vq, const
void virtqueue_map_sg(struct iovec *sg, target_phys_addr_t *addr,
size_t num_sg, int is_write);
int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem);
-int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes);
+int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
+ unsigned int out_bytes);
void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On 08/01/2011 09:14 PM, Anthony Liguori wrote: On 08/01/2011 10:44 AM, Kevin Wolf wrote: Am 01.08.2011 17:28, schrieb Anthony Liguori: 2. Top-level command for each parameter (e.g. block_set_hostcache). Supported parameters are easily discoverable via query-commands. If individual block devices support different sets of parameters then they may have to return -ENOTSUPP. I like the block_set approach. Anthony, Kevin, Supriya: Any thoughts? For the sake of overall QMP sanity, I think block_set_hostcache is really our only option. Ideally we should have blockdev_add, and blockdev_set would just take the same arguments and update the given driver. Ideally we'd have a backend_add, backend_set, etc. But in the absence of that, we should provide the best interface we can with the current tools we have. For now, using high level commands is the best we can do. Will be modifying code to have 'block_set_hostcache' command implemented. Along with that, planning to implement 'query-block_set_hostcache', that returns current hostcache setting for all the applicable block devices. I am not able to find how "query-commands" is helping out to programmatically find out all the supported parameters of a specific command. When I tried out, "query-commands" is listing all the supported command names. "query-xx" is returning current settings related to command 'xx', but not any information related to supported parameters of 'xx'. Am I missing something? Regards, Anthony Liguori But we don't have blockdev_add today, so whatever works for your as a temporary solution... Kevin
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On 08/04/2011 02:02 PM, Supriya Kannery wrote: On 08/01/2011 09:14 PM, Anthony Liguori wrote: On 08/01/2011 10:44 AM, Kevin Wolf wrote: Am 01.08.2011 17:28, schrieb Anthony Liguori: 2. Top-level command for each parameter (e.g. block_set_hostcache). Supported parameters are easily discoverable via query-commands. If individual block devices support different sets of parameters then they may have to return -ENOTSUPP. I am not able to find how "query-commands" is helping out to programmatically find out all the supported parameters of a specific command. When I tried out, "query-commands" is listing all the supported command names. "query-xx" is returning current settings related to command 'xx', but not any information related to supported parameters of 'xx'. Am I missing something? ok, I got it. "query commands" is not supposed to list out supported params for each command. Each block device parameter setting, when implemented as separate high level command, will get listed through query-commands. And this helps user to identify supported block parameters that can be changed. Regards, Anthony Liguori But we don't have blockdev_add today, so whatever works for your as a temporary solution... Kevin
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On 08/04/2011 02:01 PM, Stefan Hajnoczi wrote: On Thu, Aug 4, 2011 at 9:32 AM, Supriya Kannery wrote: On 08/01/2011 09:14 PM, Anthony Liguori wrote: On 08/01/2011 10:44 AM, Kevin Wolf wrote: Am 01.08.2011 17:28, schrieb Anthony Liguori: 2. Top-level command for each parameter (e.g. block_set_hostcache). Supported parameters are easily discoverable via query-commands. If individual block devices support different sets of parameters then they may have to return -ENOTSUPP. For now, using high level commands is the best we can do. Will be modifying code to have 'block_set_hostcache' command implemented. Along with that, planning to implement 'query-block_set_hostcache', that returns current hostcache setting for all the applicable block devices. I don't think a query command is necessary since query-block already reports this information. ok Stefan
[Qemu-devel] [v6 Patch 1/4]Qemu: Enhance "info block" to display host cache setting
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=1 file=../rhel6-32.qcow2
ro=0 drv=qcow2 encrypted=0
Signed-off-by: Supriya Kannery
---
block.c | 20
qmp-commands.hx |2 ++
2 files changed, 18 insertions(+), 4 deletions(-)
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -1812,6 +1812,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=0");
+else
+monitor_printf(mon, " hostcache=1");
+}
+
if (qdict_haskey(bs_dict, "inserted")) {
QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted"));
@@ -1848,13 +1856,17 @@ 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': %i }",
+ bs->device_name, bs->removable,
+ bs->locked,
+ !(bs->open_flags & BDRV_O_NOCACHE));
+
+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
@@ -1131,6 +1131,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 host pagecache 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)
@@ -1152,6 +1153,7 @@ Example:
{
"device":"ide0-hd0",
"locked":false,
+"hostcache":false,
"removable":false,
"inserted":{
"ro":false,
[Qemu-devel] [v6 Patch 2/4]Qemu: qerrors for file reopen and data sync
New error classes defined for file reopen failure and data
sync error
Signed-off-by: Supriya Kannery
---
qerror.c |8
qerror.h |6 ++
2 files changed, 14 insertions(+)
Index: qemu/qerror.c
===
--- qemu.orig/qerror.c
+++ qemu/qerror.c
@@ -97,6 +97,14 @@ static const QErrorStringTable qerror_ta
.desc = "Device '%(device)' is not removable",
},
{
+.error_fmt = QERR_DATA_SYNC_FAILED,
+.desc = "Syncing of data failed for device '%(device)'",
+},
+{
+.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_DATA_SYNC_FAILED \
+"{ 'class': 'DataSyncFailed', 'data': { 'device': %s } }"
+
#define QERR_DEVICE_NOT_REMOVABLE \
"{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
@@ -142,6 +145,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 } }"
[Qemu-devel] [v6 Patch 3/4]Qemu: Command "block_set_hostcache" for dynamic hostcache change
New command "block_set_hostcache" added for dynamically changing
host pagecache setting of a block device.
Signed-off-by: Supriya Kannery
---
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
@@ -676,6 +676,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) {
@@ -716,6 +744,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
@@ -72,6 +72,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);
@@ -100,6 +101,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
@@ -790,3 +790,29 @@ int do_block_resize(Monitor *mon, const
return 0;
}
+
+
+/*
+ * Change host page cache setting while guest is running.
+*/
+int do_block_set_hostcache(Monitor *mon, const QDict *qdict,
+ QObject **ret_data)
+{
+BlockDriverState *bs = NULL;
+int enable;
+const char *device;
+
+/* Validate device */
+device = qdict_get_str(qdict, "device");
+bs = bdrv_find(device);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+/* Read hostcache setting */
+enable = qdict_get_bool(qdict, "option");
+return bdrv_change_hostcache(bs, enable);
+
+}
+
Index: qemu/blockdev.h
===
--- qemu.orig/blockdev.h
+++ qemu/blockdev.h
@@ -65,5 +65,7 @@ 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_hostcache(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
[Qemu-devel] [v6 Patch 4/4]Qemu: Add commandline -drive option 'hostcache'
qemu command option 'hostcache' added to -drive for block devices.
While starting a VM from qemu commandline, this option can be used
for setting host cache usage for block data access. It is not
allowed to specify both 'hostcache' and 'cache' options in the same
commandline.
Signed-off-by: Supriya Kannery
---
blockdev.c | 13 +
qemu-config.c |4
qemu-options.hx |2 +-
3 files changed, 18 insertions(+), 1 deletion(-)
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -238,6 +238,7 @@ DriveInfo *drive_init(QemuOpts *opts, in
DriveInfo *dinfo;
int snapshot = 0;
int ret;
+int hostcache = 0;
translation = BIOS_ATA_TRANSLATION_AUTO;
media = MEDIA_DISK;
@@ -320,7 +321,19 @@ DriveInfo *drive_init(QemuOpts *opts, in
}
}
+if ((hostcache = qemu_opt_get_bool(opts, "hostcache", -1)) != -1) {
+if (!hostcache) {
+bdrv_flags |= BDRV_O_NOCACHE;
+} else {
+bdrv_flags &= ~BDRV_O_NOCACHE;
+}
+}
+
if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
+if (hostcache != -1) {
+error_report("'hostcache' and 'cache' cannot co-exist");
+return NULL;
+}
if (!strcmp(buf, "off") || !strcmp(buf, "none")) {
bdrv_flags |= BDRV_O_NOCACHE | BDRV_O_CACHE_WB;
} else if (!strcmp(buf, "writeback")) {
Index: qemu/qemu-options.hx
===
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -135,7 +135,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
" [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
" [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
" [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
-" [,readonly=on|off]\n"
+" [,readonly=on|off][,hostcache=on|off]\n"
"use 'file' as a drive image\n", QEMU_ARCH_ALL)
STEXI
@item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
Index: qemu/qemu-config.c
===
--- qemu.orig/qemu-config.c
+++ qemu/qemu-config.c
@@ -84,6 +84,10 @@ static QemuOptsList qemu_drive_opts = {
.name = "readonly",
.type = QEMU_OPT_BOOL,
.help = "open drive file as read-only",
+},{
+.name = "hostcache",
+.type = QEMU_OPT_BOOL,
+.help = "set or reset hostcache (on/off)"
},
{ /* end of list */ }
},
[Qemu-devel] [v6 Patch 0/4]Qemu: Set host pagecache from cmdline and monitor
Currently cache setting of a block device cannot be changed without restarting a running VM. Following patchset is for enabling dynamic change of cache setting for block devices through qemu monitor. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. This patchset introduces monitor command 'block_set_hostcache' using which host pagecache setting for a block device can be changed dynamically. This patchset also introduces 'hostcache', a new option for setting host cache from qemu command line. Note: 'Hostcache and 'cache' options cannot be used simultaneously from commandline. v6: 1. "block_set_hostcache" to replace "block_set" command v5: 1. Defined qerror class for incorrect command syntax. 2. Changed error_report() calls to qerror_report() v4: Added 'hostcache' option to '-drive' commandline option. v3: 1. Command "block_set" for changing various block params 2. Enhanced info-block to display hostcache setting 3. Added qmp interfaces for setting and querying hostcache v2: 1. Support of dynamic cache change only for hostcache. 2. Monitor command "hostcache_get" added to display cache setting 3. Backed off the changes for display of cache setting in "info block" v1: Dynamic cache change through monitor New block command added: "block_set_hostcache" -- Sets hostcache parameter for block device while guest is running. Usage: block_set_hostcache = block device = on/off New 'hostcache' option added to -drive: -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" " [,readonly=on|off][,hostcache=on|off]\n" 1/4 Enhance "info block" to display hostcache setting 2/4 New qerrors for file reopen and data sync 3/4 Command "block_set_hostcache" for changing hostcache setting 4/4 'hostcache' option added to -drive in qemu commandline qemu/block.c | 75 +++ qemu/block.h |2 + qemu/blockdev.c | 39 qemu/blockdev.h |2 + qemu/hmp-commands.hx | 14 + qemu/qemu-config.c |4 +++ qemu/qemu-options.hx |2 - qemu/qerror.c|8 +++ qemu/qerror.h|6 + qemu/qmp-commands.hx | 29 + 10 files changed, 176 insertions(+), 5 deletions(-) ~
Re: [Qemu-devel] Safely reopening image files by stashing fds
On 08/05/2011 09:19 PM, Anthony Liguori wrote: On 08/05/2011 10:43 AM, Kevin Wolf wrote: Am 05.08.2011 17:24, schrieb Stefan Hajnoczi: On Fri, Aug 5, 2011 at 3:28 PM, Christoph Hellwig wrote: On Fri, Aug 05, 2011 at 02:12:48PM +0100, Daniel P. Berrange wrote: Because you cannot change O_DIRECT on an open fd :(. This is why we're going through this pain. Hmm, I remember hearing that before, but looking at the current fcntl() manpage, it claims you *can* change O_DIRECT using SET_FL. Perhaps this is a newish feature, but it'd be nicer to use it if possible ? It's been there since day 1 of O_DIRECT support. Sorry, my bad. So for Linux we could just use fcntl for block_set_hostcache and not bother with reopening. However, we will need to reopen should we wish to support changing O_DSYNC. We do wish to support that. Anthony thinks that allowing the guest to toggle WCE is a prerequisite for making cache=writeback the default. And this is something that I definitely want to do for 1.0. Indeed. We discussed the following so far... 1. How to safely reopen image files 2. Dynamic hostcache change 3. Support for dynamic change of O_DSYNC Since 2 is independent of 1, shall I go ahead implementing hostcache change using fcntl. Implementation for safely reopening image files using "BDRVReopenState" can be done separately as a pre-requisite before implementing 3 Thanks, Supriya Regards, Anthony Liguori Kevin
Re: [Qemu-devel] Safely reopening image files by stashing fds
Kevin Wolf wrote: Am 08.08.2011 09:02, schrieb Supriya Kannery: On 08/05/2011 09:19 PM, Anthony Liguori wrote: On 08/05/2011 10:43 AM, Kevin Wolf wrote: Am 05.08.2011 17:24, schrieb Stefan Hajnoczi: On Fri, Aug 5, 2011 at 3:28 PM, Christoph Hellwig wrote: On Fri, Aug 05, 2011 at 02:12:48PM +0100, Daniel P. Berrange wrote: Because you cannot change O_DIRECT on an open fd :(. This is why we're going through this pain. Hmm, I remember hearing that before, but looking at the current fcntl() manpage, it claims you *can* change O_DIRECT using SET_FL. Perhaps this is a newish feature, but it'd be nicer to use it if possible ? It's been there since day 1 of O_DIRECT support. Sorry, my bad. So for Linux we could just use fcntl for block_set_hostcache and not bother with reopening. However, we will need to reopen should we wish to support changing O_DSYNC. We do wish to support that. Anthony thinks that allowing the guest to toggle WCE is a prerequisite for making cache=writeback the default. And this is something that I definitely want to do for 1.0. Indeed. We discussed the following so far... 1. How to safely reopen image files 2. Dynamic hostcache change 3. Support for dynamic change of O_DSYNC Since 2 is independent of 1, shall I go ahead implementing hostcache change using fcntl. Implementation for safely reopening image files using "BDRVReopenState" can be done separately as a pre-requisite before implementing 3 Doing it separately means that we would introduce yet another callback that is used just to change O_DIRECT. In the end we want it to use bdrv_reopen(), too, so I'm not sure if there is a need for a temporary solution. Could you please explain "In the end we want it to use bdrv_reopen" at bit more. When fcntl() can change O_DIRECT on open fd , is there a need to "re-open" the image file? Considering the current way of having separate high level commands for changing block parameters (block_set_hostcache, and may be block_set_flush in furture), these dynamic requests will be sequential. So wouldn't it be better to avoid re-opening of image if possible for individual flag change request that comes in? Actually, once we know what we really want (I haven't seen many comments on the BDRVReopenState suggestion yet), it should be pretty easy to implement. Kevin Will work on to get an RFC patch with this proposed BDRVReopenState to get more inputs.
Re: [Qemu-devel] Safely reopening image files by stashing fds
Kevin Wolf wrote: Am 09.08.2011 11:22, schrieb supriya kannery: Kevin Wolf wrote: Am 08.08.2011 09:02, schrieb Supriya Kannery: On 08/05/2011 09:19 PM, Anthony Liguori wrote: On 08/05/2011 10:43 AM, Kevin Wolf wrote: Am 05.08.2011 17:24, schrieb Stefan Hajnoczi: On Fri, Aug 5, 2011 at 3:28 PM, Christoph Hellwig wrote: On Fri, Aug 05, 2011 at 02:12:48PM +0100, Daniel P. Berrange wrote: Because you cannot change O_DIRECT on an open fd :(. This is why we're going through this pain. Hmm, I remember hearing that before, but looking at the current fcntl() manpage, it claims you *can* change O_DIRECT using SET_FL. Perhaps this is a newish feature, but it'd be nicer to use it if possible ? It's been there since day 1 of O_DIRECT support. Sorry, my bad. So for Linux we could just use fcntl for block_set_hostcache and not bother with reopening. However, we will need to reopen should we wish to support changing O_DSYNC. We do wish to support that. Anthony thinks that allowing the guest to toggle WCE is a prerequisite for making cache=writeback the default. And this is something that I definitely want to do for 1.0. Indeed. We discussed the following so far... 1. How to safely reopen image files 2. Dynamic hostcache change 3. Support for dynamic change of O_DSYNC Since 2 is independent of 1, shall I go ahead implementing hostcache change using fcntl. Implementation for safely reopening image files using "BDRVReopenState" can be done separately as a pre-requisite before implementing 3 Doing it separately means that we would introduce yet another callback that is used just to change O_DIRECT. In the end we want it to use bdrv_reopen(), too, so I'm not sure if there is a need for a temporary solution. Could you please explain "In the end we want it to use bdrv_reopen" at bit more. When fcntl() can change O_DIRECT on open fd , is there a need to "re-open" the image file? What I meant is that in the end, with a generic bdrv_reopen(), we can have raw-posix only call dup() and fcntl() instead of doing a close()/open() sequence if it can satisfy the new flags this way. But this would be an implementation detail and not be visible in the interface. Kevin ok - thanks, Supriya
Re: [Qemu-devel] [RFC] Safely reopening image files by stashing fds
On 08/09/2011 03:02 PM, supriya kannery wrote:
> Kevin Wolf wrote:
>> Am 09.08.2011 11:22, schrieb supriya kannery:
>>> Kevin Wolf wrote:
>>
>> What I meant is that in the end, with a generic bdrv_reopen(), we can
>> have raw-posix only call dup() and fcntl() instead of doing a
>> close()/open() sequence if it can satisfy the new flags this way. But
>> this would be an implementation detail and not be visible in the
>> interface.
>>
>> Kevin
>
> ok
> - thanks, Supriya
>
Though I started the RFC patch with defining BDRVReopenState, ended up
in enhancing struct BlockDriver with .bdrv_reopen. bdrv_reopen
mplementation specific to respective driver is assigned to this
function pointer.
Please find the implementation of O_DIRECT flag change, done in
raw-posix.c. Similar implementation can be done for vmdk (with
bdrv_reopen_commit and bdrv_reopen_abort as internal functions in
vmdk.c for opening split files), sheepdog, nbd etc..
Request for comments on this approach.
Note: Following patch is for demonstrating the approach using
raw-posix as sample driver. This patch is not complete.
---
block.c | 33 +++--
block/raw-posix.c | 34 ++
block_int.h |1 +
3 files changed, 58 insertions(+), 10 deletions(-)
Index: qemu/block/raw-posix.c
===
--- qemu.orig/block/raw-posix.c
+++ qemu/block/raw-posix.c
@@ -276,6 +276,39 @@ static int raw_open(BlockDriverState *bs
return raw_open_common(bs, filename, flags, 0);
}
+static int raw_reopen(BlockDriverState *bs, int flags)
+{
+BDRVRawState *s = bs->opaque;
+int new_fd;
+int ret = 0;
+
+/* Handle request for toggling O_DIRECT */
+if ((bs->open_flags | BDRV_O_NOCACHE) ^
+(flags | BDRV_O_NOCACHE))
+{
+if ((new_fd = dup(s->fd)) <= 0) {
+return -1;
+}
+
+if ((ret = fcntl_setfl(new_fd, flags)) < 0) {
+close(new_fd);
+return ret;
+}
+
+/* Set new flags, so replace old fd with new one */
+close(s->fd);
+s->fd = new_fd;
+s->open_flags = flags;
+bs->open_flags = flags;
+}
+
+/*
+ * TBD: handle O_DSYNC and other flags for which image
+ * file has to be reopened
+ */
+return 0;
+}
+
/* XXX: use host sector size if necessary with:
#ifdef DIOCGSECTORSIZE
{
@@ -886,6 +919,7 @@ static BlockDriver bdrv_file = {
.instance_size = sizeof(BDRVRawState),
.bdrv_probe = NULL, /* no probe for protocols */
.bdrv_file_open = raw_open,
+.bdrv_reopen = raw_reopen,
.bdrv_read = raw_read,
.bdrv_write = raw_write,
.bdrv_close = raw_close,
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -687,20 +687,33 @@ 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_REOPEN_FILE_FAILED, bs->filename);
-ret = bdrv_open(bs, bs->filename, open_flags, drv);
+/* Use driver specific reopen() if available */
+if (drv->bdrv_reopen) {
+if ((ret = drv->bdrv_reopen(bs, bdrv_flags)) < 0) {
+qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+return ret;
+}
+} else {
+/* Use reopen procedure common for drivers */
+open_flags = bs->open_flags;
+bdrv_close(bs);
+
+ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
if (ret < 0) {
-/* Reopen failed with orig and modified flags */
-abort();
+/*
+ * 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 with orig and modified flags failed
+ */
+abort();
+}
}
}
-
return ret;
}
Index: qemu/block_int.h
===
--- qemu.orig/block_int.h
+++ qemu/block_int.h
@@ -54,6 +54,7 @@ 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);
+int (*bdrv_reopen)(BlockDriverState *bs, int flags);
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);
Re: [Qemu-devel] [RFC] Safely reopening image files by stashing fds
On 08/09/2011 03:02 PM, supriya kannery wrote:
> Kevin Wolf wrote:
>> Am 09.08.2011 11:22, schrieb supriya kannery:
>>> Kevin Wolf wrote:
>>
>> What I meant is that in the end, with a generic bdrv_reopen(), we can
>> have raw-posix only call dup() and fcntl() instead of doing a
>> close()/open() sequence if it can satisfy the new flags this way. But
>> this would be an implementation detail and not be visible in the
>> interface.
>>
>> Kevin
>
> ok
> - thanks, Supriya
>
Though I started the RFC patch with defining BDRVReopenState, ended up
in enhancing struct BlockDriver with .bdrv_reopen. bdrv_reopen
mplementation specific to respective driver is assigned to this
function pointer.
Please find the implementation of O_DIRECT flag change, done in
raw-posix.c. Similar implementation can be done for vmdk (with
bdrv_reopen_commit and bdrv_reopen_abort as internal functions in
vmdk.c for opening split files), sheepdog, nbd etc..
Request for comments on this approach.
Note: Following patch is for demonstrating the approach using
raw-posix as sample driver. This patch is not complete.
- thanks, Supriya
---
block.c | 33 +++--
block/raw-posix.c | 34 ++
block_int.h |1 +
3 files changed, 58 insertions(+), 10 deletions(-)
Index: qemu/block/raw-posix.c
===
--- qemu.orig/block/raw-posix.c
+++ qemu/block/raw-posix.c
@@ -276,6 +276,39 @@ static int raw_open(BlockDriverState *bs
return raw_open_common(bs, filename, flags, 0);
}
+static int raw_reopen(BlockDriverState *bs, int flags)
+{
+BDRVRawState *s = bs->opaque;
+int new_fd;
+int ret = 0;
+
+/* Handle request for toggling O_DIRECT */
+if ((bs->open_flags | BDRV_O_NOCACHE) ^
+(flags | BDRV_O_NOCACHE))
+{
+if ((new_fd = dup(s->fd)) <= 0) {
+return -1;
+}
+
+if ((ret = fcntl_setfl(new_fd, flags)) < 0) {
+close(new_fd);
+return ret;
+}
+
+/* Set new flags, so replace old fd with new one */
+close(s->fd);
+s->fd = new_fd;
+s->open_flags = flags;
+bs->open_flags = flags;
+}
+
+/*
+ * TBD: handle O_DSYNC and other flags for which image
+ * file has to be reopened
+ */
+return 0;
+}
+
/* XXX: use host sector size if necessary with:
#ifdef DIOCGSECTORSIZE
{
@@ -886,6 +919,7 @@ static BlockDriver bdrv_file = {
.instance_size = sizeof(BDRVRawState),
.bdrv_probe = NULL, /* no probe for protocols */
.bdrv_file_open = raw_open,
+.bdrv_reopen = raw_reopen,
.bdrv_read = raw_read,
.bdrv_write = raw_write,
.bdrv_close = raw_close,
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -687,20 +687,33 @@ 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_REOPEN_FILE_FAILED, bs->filename);
-ret = bdrv_open(bs, bs->filename, open_flags, drv);
+/* Use driver specific reopen() if available */
+if (drv->bdrv_reopen) {
+if ((ret = drv->bdrv_reopen(bs, bdrv_flags)) < 0) {
+qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+return ret;
+}
+} else {
+/* Use reopen procedure common for drivers */
+open_flags = bs->open_flags;
+bdrv_close(bs);
+
+ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
if (ret < 0) {
-/* Reopen failed with orig and modified flags */
-abort();
+/*
+ * 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 with orig and modified flags failed
+ */
+abort();
+}
}
}
-
return ret;
}
Index: qemu/block_int.h
===
--- qemu.orig/block_int.h
+++ qemu/block_int.h
@@ -54,6 +54,7 @@ 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);
+int (*bdrv_reopen)(BlockDriverState *bs, int flags);
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);
Re: [Qemu-devel] CoW image commit+shrink(= make_empty) support
On 06/12/2012 04:26 PM, Stefan Hajnoczi wrote: On Mon, Jun 11, 2012 at 4:37 PM, Jeff Cody wrote: On 06/11/2012 10:24 AM, Stefan Hajnoczi wrote: On Mon, Jun 11, 2012 at 1:50 PM, Kevin Wolf wrote: Am 11.06.2012 14:09, schrieb Stefan Hajnoczi: On Fri, Jun 8, 2012 at 6:46 PM, Jeff Cody wrote: On 06/08/2012 12:11 PM, Kevin Wolf wrote: Am 08.06.2012 16:32, schrieb Jeff Cody: On 06/08/2012 09:53 AM, Stefan Hajnoczi wrote: On Fri, Jun 8, 2012 at 2:19 PM, Jeff Cody wrote: On 06/08/2012 08:42 AM, Stefan Hajnoczi wrote: Let's figure out how to specify block-commit so we're all happy, that way we can avoid duplicating work. Any comments on my notes above? I think we are almost completely on the same page - devil is in the details, of course (for instance, on how to convert the destination base from r/o to r/w). Great. The atomic r/o -> r/w transition and the commit coroutine can be implemented on in parallel. Are you happy to implement the atomic r/o -> r/w transition since you wrote bdrv_append()? Then Zhi Hui can assume that part already works and focus on implementing the commit coroutine in the meantime. I'm just suggesting a way to split up the work, please let me know if you think this is good. I am happy to do it that way. I'll shift my focus to the atomic image reopen in r/w mode. I'll go ahead and post my diagrams and other info for block-commit on the wiki, because I don't think it conflicts with we discussed above (although I will modify my diagrams to not show commit from the top-level image). Of course, feel free to change it as necessary. I may have mentioned it before, but just in case: I think Supriya's bdrv_reopen() patches are a good starting point. I don't know why they were never completed, but I think we all agreed on the general design, so it should be possible to pick that up. Though if you have already started with your own work on it, Jeff, I expect that it won't be much different because it's basically the same transactional approach that you know and that we already used for group snapshots. I will definitely use parts of Supriya's as it makes sense - what I started work on is similar to bdrv_append() and the current transaction approach, so there will be plenty in common to reuse, even with some differences. I have CCed Supriya who has been working on the reopen patch series. She is close to posting a new version. It's just a bit disappointing that it takes several months between each two versions of the patch series. We'd like to have this in qemu 1.2, not only in qemu 1.14. I can understand if someone can't find the time, but then allow at least someone else to pick it up. Hey, don't shoot the messenger :). I just wanted add Supriya to CC so she can join the discussion and see how much overlap there is with what she's doing. We all contribute to QEMU from different angles and with different priorities. If there is a time critical dependency on the reopen code then discuss it here and the result will be that someone officially drives the feature on. I am more than happy to take the previous reopen() patches, and drive those forward, and also do whatever else is needed for live block commit. Supriya, Can you share with us whether you have enough time to complete the reopen() patches you've been working on? This functionality is a dependency for the new block-commit command. Jeff is willing to take on the reopen() work if you do not have time. Please let us know. Stefan All, Sorry! for not following up these discussions and hence the delay in responding. After few month's gap, I had resumed working on bdrv_reopen() patchset but was not aware of this dependency and urgency. Will be posting whatever I have by tomorrow so that Jeff can use that code as needed for block-commit. - Supriya
Re: [Qemu-devel] CoW image commit+shrink(= make_empty) support
On 06/14/2012 07:59 PM, Jeff Cody wrote: On 06/14/2012 10:23 AM, Zhi Hui Li wrote: On 2012年06月11日 23:37, Jeff Cody wrote: On 06/11/2012 10:24 AM, Stefan Hajnoczi wrote: On Mon, Jun 11, 2012 at 1:50 PM, Kevin Wolf wrote: Am 11.06.2012 14:09, schrieb Stefan Hajnoczi: On Fri, Jun 8, 2012 at 6:46 PM, Jeff Cody wrote: On 06/08/2012 12:11 PM, Kevin Wolf wrote: Am 08.06.2012 16:32, schrieb Jeff Cody: On 06/08/2012 09:53 AM, Stefan Hajnoczi wrote: On Fri, Jun 8, 2012 at 2:19 PM, Jeff Cody wrote: On 06/08/2012 08:42 AM, Stefan Hajnoczi wrote: Let's figure out how to specify block-commit so we're all happy, that way we can avoid duplicating work. Any comments on my notes above? I think we are almost completely on the same page - devil is in the details, of course (for instance, on how to convert the destination base from r/o to r/w). Great. The atomic r/o -> r/w transition and the commit coroutine can be implemented on in parallel. Are you happy to implement the atomic r/o -> r/w transition since you wrote bdrv_append()? Then Zhi Hui can assume that part already works and focus on implementing the commit coroutine in the meantime. I'm just suggesting a way to split up the work, please let me know if you think this is good. I am happy to do it that way. I'll shift my focus to the atomic image reopen in r/w mode. I'll go ahead and post my diagrams and other info for block-commit on the wiki, because I don't think it conflicts with we discussed above (although I will modify my diagrams to not show commit from the top-level image). Of course, feel free to change it as necessary. I may have mentioned it before, but just in case: I think Supriya's bdrv_reopen() patches are a good starting point. I don't know why they were never completed, but I think we all agreed on the general design, so it should be possible to pick that up. Though if you have already started with your own work on it, Jeff, I expect that it won't be much different because it's basically the same transactional approach that you know and that we already used for group snapshots. I will definitely use parts of Supriya's as it makes sense - what I started work on is similar to bdrv_append() and the current transaction approach, so there will be plenty in common to reuse, even with some differences. I have CCed Supriya who has been working on the reopen patch series. She is close to posting a new version. It's just a bit disappointing that it takes several months between each two versions of the patch series. We'd like to have this in qemu 1.2, not only in qemu 1.14. I can understand if someone can't find the time, but then allow at least someone else to pick it up. Hey, don't shoot the messenger :). I just wanted add Supriya to CC so she can join the discussion and see how much overlap there is with what she's doing. We all contribute to QEMU from different angles and with different priorities. If there is a time critical dependency on the reopen code then discuss it here and the result will be that someone officially drives the feature on. I am more than happy to take the previous reopen() patches, and drive those forward, and also do whatever else is needed for live block commit. It sounds like Zhi Hui is working on live block commit patches, and Supriya is working on the bdrv_reopen() portion - I don't want to duplicate any effort, but if there is anything I can do to help with either of those areas, just let me know. Thanks, Jeff Jeff please go ahead with block-commit, I am finishing up my fdc async conversion patch series first. I will reply when I'm ready to work on block-commit. Thank you very much! Hi Zhi, I will do that. Thanks! Jeff Jeff, Need another day's time for posting bdrv_reopen() patchset. Getting few hmp related build errors in hostcache toggling code when built on latest git. Will correct those and post the patchset soon. - Supriya
[Qemu-devel] [v1 Patch 0/10]Qemu: Dynamic host pagecache change and image file reopen
For changing host pagecache setting of a running VM, it is important to have a safe way of reopening its image file. Following patchset introduces: * a generic way to reopen image files safely. In this approach, before reopening an image, for each block driver, its state will be stashed. Incase preparation, (bdrv_reopen_prepare) for reopening returns success, the stashed state will be cleared (bdrv_reopen_commit) and reopened state will be used further. Incase preparation of reopening returns failure, the state of the driver will be rolled back (bdrv_reopen_abort) to the stashed state. This approach is implemented for raw-posix, raw-win32, vmdk, qcow, qcow2 and qed block drivers. * qmp and hmp command 'block_set_hostcache' using which host pagecache setting for a block device can be changed when the VM is running. * BDRVReopenState, a generic structure which can be extended by each of the block drivers to reopen respective image files. ToDo: * Currently driver state is stashed by field-by-field copy. Optimise this by stashing only the required fields. * Find out other drivers needing bdrv_reopen implementation. * Do some more extensive testing, especially with drivers like floppy, cdrom etc.. Earlier discussions on RFC for dynamic change of host pagecache can be found at: http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg04139.html New block command added: "block_set_hostcache" -- Sets hostcache parameter for block device while guest is running. Usage: block_set_hostcache = block device = on/off qemu/block.c | 127 -- qemu/block.h |5 + qemu/block/qcow.c | 108 + qemu/block/qcow2.c | 177 + qemu/block/qed.c | 103 qemu/block/raw-posix.c | 123 ++ qemu/block/raw-win32.c | 96 ++ qemu/block/raw.c | 20 + qemu/block/vmdk.c | 103 qemu/block_int.h | 11 +++ qemu/blockdev.c| 24 ++ qemu/hmp-commands.hx | 16 qemu/hmp.c | 11 ++ qemu/hmp.h |1 qemu/qapi-schema.json | 20 - qemu/qemu-common.h |1 qemu/qerror.c |8 ++ qemu/qerror.h |6 + qemu/qmp-commands.hx | 24 ++ 19 files changed, 971 insertions(+), 13 deletions(-)
[Qemu-devel] [v1 Patch 1/10]Qemu: Enhance "info block" to display host cache setting
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
---
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]);
[Qemu-devel] [v1 Patch 3/10]Qemu: Cmd "block_set_hostcache" for dynamic cache change
New command "block_set_hostcache" added for dynamically changing
host pagecache setting of a block device.
Usage:
block_set_hostcache
= block device
= on/off
Example:
(qemu) block_set_hostcache ide0-hd0 off
Signed-off-by: Supriya Kannery
---
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
[Qemu-devel] [v1 Patch 7/10]Qemu: vmdk image file reopen
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
Index: qemu/block/vmdk.c
===
--- qemu.orig/block/vmdk.c
+++ qemu/block/vmdk.c
@@ -115,6 +115,14 @@ typedef struct VmdkGrainMarker {
uint8_t data[0];
} VmdkGrainMarker;
+typedef struct BDRVVmdkReopenState {
+BDRVReopenState reopen_state;
+BDRVVmdkState *stash_s;
+} BDRVVmdkReopenState;
+
+static void vmdk_stash_state(BDRVVmdkState *stashed_state, BDRVVmdkState *s);
+static void vmdk_revert_state(BDRVVmdkState *s, BDRVVmdkState *stashed_state);
+
static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
{
uint32_t magic;
@@ -588,7 +596,6 @@ static int vmdk_parse_extents(const char
if (!strcmp(type, "FLAT")) {
/* FLAT extent */
VmdkExtent *extent;
-
extent = vmdk_add_extent(bs, extent_file, true, sectors,
0, 0, 0, 0, sectors);
extent->flat_start_offset = flat_offset << 9;
@@ -675,6 +682,94 @@ fail:
return ret;
}
+static int vmdk_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+ int flags)
+{
+BDRVVmdkReopenState *vmdk_rs = g_malloc0(sizeof(BDRVVmdkReopenState));
+int ret = 0;
+BDRVVmdkState *s = bs->opaque;
+
+vmdk_rs->reopen_state.bs = bs;
+
+/* save state before reopen */
+vmdk_rs->stash_s = g_malloc0(sizeof(BDRVVmdkState));
+vmdk_stash_state(vmdk_rs->stash_s, s);
+s->num_extents = 0;
+s->extents = NULL;
+s->migration_blocker = NULL;
+*prs = &(vmdk_rs->reopen_state);
+
+/* create extents afresh with new flags */
+ ret = vmdk_open(bs, flags);
+ return ret;
+}
+
+static void vmdk_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+BDRVVmdkReopenState *vmdk_rs;
+BDRVVmdkState *stashed_s;
+VmdkExtent *e;
+int i;
+
+vmdk_rs = container_of(rs, BDRVVmdkReopenState, reopen_state);
+stashed_s = vmdk_rs->stash_s;
+
+/* clean up stashed state */
+for (i = 0; i < stashed_s->num_extents; i++) {
+e = &stashed_s->extents[i];
+g_free(e->l1_table);
+g_free(e->l2_cache);
+g_free(e->l1_backup_table);
+}
+g_free(stashed_s->extents);
+g_free(vmdk_rs->stash_s);
+g_free(vmdk_rs);
+}
+
+static void vmdk_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+BDRVVmdkReopenState *vmdk_rs;
+BDRVVmdkState *s = bs->opaque;
+VmdkExtent *e;
+int i;
+
+vmdk_rs = container_of(rs, BDRVVmdkReopenState, reopen_state);
+
+/* revert to stashed state */
+for (i = 0; i < s->num_extents; i++) {
+e = &s->extents[i];
+g_free(e->l1_table);
+g_free(e->l2_cache);
+g_free(e->l1_backup_table);
+}
+g_free(s->extents);
+vmdk_revert_state(s, vmdk_rs->stash_s);
+g_free(vmdk_rs->stash_s);
+g_free(vmdk_rs);
+}
+
+static void vmdk_stash_state(BDRVVmdkState *stashed_state, BDRVVmdkState *s)
+{
+ stashed_state->lock = s->lock;
+ stashed_state->desc_offset = s->desc_offset;
+ stashed_state->cid_updated = s->cid_updated;
+ stashed_state->parent_cid = s->parent_cid;
+ stashed_state->num_extents = s->num_extents;
+ stashed_state->extents = s->extents;
+ stashed_state->migration_blocker = s->migration_blocker;
+}
+
+static void vmdk_revert_state(BDRVVmdkState *s, BDRVVmdkState *stashed_state)
+{
+ s->lock = stashed_state->lock;
+ s->desc_offset = stashed_state->desc_offset;
+ s->cid_updated = stashed_state->cid_updated;
+ s->parent_cid = stashed_state->parent_cid;
+ s->num_extents = stashed_state->num_extents;
+ s->extents = stashed_state->extents;
+ s->migration_blocker = stashed_state->migration_blocker;
+}
+
static int get_whole_cluster(BlockDriverState *bs,
VmdkExtent *extent,
uint64_t cluster_offset,
@@ -1593,6 +1688,12 @@ static BlockDriver bdrv_vmdk = {
.instance_size = sizeof(BDRVVmdkState),
.bdrv_probe = vmdk_probe,
.bdrv_open = vmdk_open,
+.bdrv_reopen_prepare
+= vmdk_reopen_prepare,
+.bdrv_reopen_commit
+= vmdk_reopen_commit,
+.bdrv_reopen_abort
+= vmdk_reopen_abort,
.bdrv_read = vmdk_co_read,
.bdrv_write = vmdk_co_write,
.bdrv_close = vmdk_close,
[Qemu-devel] [v1 Patch 10/10]Qemu: qed image file reopen
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
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,
[Qemu-devel] [v1 Patch 8/10]Qemu: qcow2 image file reopen
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
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;
+
+stashe
Re: [Qemu-devel] CoW image commit+shrink(= make_empty) support
On 06/14/2012 11:58 PM, Supriya Kannery wrote: On 06/14/2012 07:59 PM, Jeff Cody wrote: On 06/14/2012 10:23 AM, Zhi Hui Li wrote: On 2012年06月11日 23:37, Jeff Cody wrote: On 06/11/2012 10:24 AM, Stefan Hajnoczi wrote: On Mon, Jun 11, 2012 at 1:50 PM, Kevin Wolf wrote: Am 11.06.2012 14:09, schrieb Stefan Hajnoczi: On Fri, Jun 8, 2012 at 6:46 PM, Jeff Cody wrote: On 06/08/2012 12:11 PM, Kevin Wolf wrote: Am 08.06.2012 16:32, schrieb Jeff Cody: On 06/08/2012 09:53 AM, Stefan Hajnoczi wrote: On Fri, Jun 8, 2012 at 2:19 PM, Jeff Cody wrote: On 06/08/2012 08:42 AM, Stefan Hajnoczi wrote: Let's figure out how to specify block-commit so we're all happy, that way we can avoid duplicating work. Any comments on my notes above? I think we are almost completely on the same page - devil is in the details, of course (for instance, on how to convert the destination base from r/o to r/w). Great. The atomic r/o -> r/w transition and the commit coroutine can be implemented on in parallel. Are you happy to implement the atomic r/o -> r/w transition since you wrote bdrv_append()? Then Zhi Hui can assume that part already works and focus on implementing the commit coroutine in the meantime. I'm just suggesting a way to split up the work, please let me know if you think this is good. I am happy to do it that way. I'll shift my focus to the atomic image reopen in r/w mode. I'll go ahead and post my diagrams and other info for block-commit on the wiki, because I don't think it conflicts with we discussed above (although I will modify my diagrams to not show commit from the top-level image). Of course, feel free to change it as necessary. I may have mentioned it before, but just in case: I think Supriya's bdrv_reopen() patches are a good starting point. I don't know why they were never completed, but I think we all agreed on the general design, so it should be possible to pick that up. Though if you have already started with your own work on it, Jeff, I expect that it won't be much different because it's basically the same transactional approach that you know and that we already used for group snapshots. I will definitely use parts of Supriya's as it makes sense - what I started work on is similar to bdrv_append() and the current transaction approach, so there will be plenty in common to reuse, even with some differences. I have CCed Supriya who has been working on the reopen patch series. She is close to posting a new version. It's just a bit disappointing that it takes several months between each two versions of the patch series. We'd like to have this in qemu 1.2, not only in qemu 1.14. I can understand if someone can't find the time, but then allow at least someone else to pick it up. Hey, don't shoot the messenger :). I just wanted add Supriya to CC so she can join the discussion and see how much overlap there is with what she's doing. We all contribute to QEMU from different angles and with different priorities. If there is a time critical dependency on the reopen code then discuss it here and the result will be that someone officially drives the feature on. I am more than happy to take the previous reopen() patches, and drive those forward, and also do whatever else is needed for live block commit. It sounds like Zhi Hui is working on live block commit patches, and Supriya is working on the bdrv_reopen() portion - I don't want to duplicate any effort, but if there is anything I can do to help with either of those areas, just let me know. Thanks, Jeff Jeff please go ahead with block-commit, I am finishing up my fdc async conversion patch series first. I will reply when I'm ready to work on block-commit. Thank you very much! Hi Zhi, I will do that. Thanks! Jeff Jeff, Need another day's time for posting bdrv_reopen() patchset. Getting few hmp related build errors in hostcache toggling code when built on latest git. Will correct those and post the patchset soon. - Supriya Jeff, bdrv_reopen v1 patchset posted. Title: "Dynamic host pagecache change and image file reopen". Thanks, Supriya
[Qemu-devel] [v1 Patch 9/10]Qemu: qcow image file reopen
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
Index: qemu/block/qcow.c
===
--- qemu.orig/block/qcow.c
+++ qemu/block/qcow.c
@@ -78,7 +78,14 @@ typedef struct BDRVQcowState {
Error *migration_blocker;
} BDRVQcowState;
+typedef struct BDRVQcowReopenState {
+BDRVReopenState reopen_state;
+BDRVQcowState *stash_s;
+} BDRVQcowReopenState;
+
static int decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
+static void qcow_stash_state(BDRVQcowState *stash_s, BDRVQcowState *s);
+static void qcow_revert_state(BDRVQcowState *s, BDRVQcowState *stash_s);
static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
{
@@ -197,6 +204,103 @@ static int qcow_open(BlockDriverState *b
return ret;
}
+static int qcow_reopen_prepare(BlockDriverState *bs, BDRVReopenState **prs,
+ int flags)
+{
+BDRVQcowReopenState *qcow_rs = g_malloc0(sizeof(BDRVQcowReopenState));
+int ret = 0;
+BDRVQcowState *s = bs->opaque;
+
+qcow_rs->reopen_state.bs = bs;
+qcow_rs->stash_s = g_malloc0(sizeof(BDRVQcowState));
+qcow_stash_state(qcow_rs->stash_s, s);
+*prs = &(qcow_rs->reopen_state);
+
+ret = qcow_open(bs, flags);
+return ret;
+}
+
+static void qcow_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs)
+{
+BDRVQcowReopenState *qcow_rs;
+
+qcow_rs = container_of(rs, BDRVQcowReopenState, reopen_state);
+g_free(qcow_rs->stash_s);
+g_free(qcow_rs);
+}
+
+static void qcow_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+BDRVQcowReopenState *qcow_rs;
+BDRVQcowState *s = bs->opaque;
+
+qcow_rs = container_of(rs, BDRVQcowReopenState, reopen_state);
+
+/* revert to stashed state */
+qcow_revert_state(s, qcow_rs->stash_s);
+
+g_free(qcow_rs->stash_s);
+g_free(qcow_rs);
+}
+
+static void qcow_stash_state(BDRVQcowState *stash_s, BDRVQcowState *s)
+{
+int i;
+
+stash_s->cluster_bits = s->cluster_bits;
+stash_s->cluster_size = s->cluster_size;
+stash_s->cluster_sectors = s->cluster_sectors;
+stash_s->l2_bits = s->l2_bits;
+stash_s->l2_size = s->l2_size;
+stash_s->l1_size = s->l1_size;
+stash_s->cluster_offset_mask = s->cluster_offset_mask;
+stash_s->l1_table_offset = s->l1_table_offset;
+stash_s->l1_table = s->l1_table;
+stash_s->l2_cache = s->l2_cache;
+for (i = 0; i < L2_CACHE_SIZE; i++) {
+stash_s->l2_cache_offsets[i] = s->l2_cache_offsets[i];
+stash_s->l2_cache_counts[i] = s->l2_cache_counts[i];
+}
+stash_s->cluster_cache = s->cluster_cache;
+stash_s->cluster_data = s->cluster_data;
+stash_s->cluster_cache_offset = s->cluster_cache_offset;
+stash_s->crypt_method = s->crypt_method;
+stash_s->crypt_method_header = s->crypt_method_header;
+stash_s->aes_encrypt_key = s->aes_encrypt_key;
+stash_s->aes_decrypt_key = s->aes_decrypt_key;
+stash_s->lock = s->lock;
+stash_s->migration_blocker = s->migration_blocker;
+}
+
+static void qcow_revert_state(BDRVQcowState *s, BDRVQcowState *stash_s)
+{
+int i;
+
+s->cluster_bits = stash_s->cluster_bits;
+s->cluster_size = stash_s->cluster_size;
+s->cluster_sectors = stash_s->cluster_sectors;
+s->l2_bits = stash_s->l2_bits;
+s->l2_size = stash_s->l2_size;
+s->l1_size = stash_s->l1_size;
+s->cluster_offset_mask = stash_s->cluster_offset_mask;
+s->l1_table_offset = stash_s->l1_table_offset;
+s->l1_table = stash_s->l1_table;
+s->l2_cache = stash_s->l2_cache;
+for (i = 0; i < L2_CACHE_SIZE; i++) {
+s->l2_cache_offsets[i] = s->l2_cache_offsets[i];
+s->l2_cache_counts[i] = stash_s->l2_cache_counts[i];
+}
+s->cluster_cache = stash_s->cluster_cache;
+s->cluster_data = stash_s->cluster_data;
+s->cluster_cache_offset = stash_s->cluster_cache_offset;
+s->crypt_method = stash_s->crypt_method;
+s->crypt_method_header = stash_s->crypt_method_header;
+s->aes_encrypt_key = stash_s->aes_encrypt_key;
+s->aes_decrypt_key = stash_s->aes_decrypt_key;
+s->lock = stash_s->lock;
+s->migration_blocker = stash_s->migration_blocker;
+}
+
static int qcow_set_key(BlockDriverState *bs, const char *key)
{
BDRVQcowState *s = bs->opaque;
@@ -870,6 +974,10 @@ static BlockDriver bdrv_qcow = {
.bdrv_close= qcow_close,
.bdrv_create = qcow_create,
+.bdrv_reopen_prepare= qcow_reopen_prepare,
+.bdrv_reopen_commit = qcow_reopen_commit,
+.bdrv_reopen_abort = qcow_reopen_abort,
+
.bdrv_co_readv = qcow_co_readv,
.bdrv_co_writev = qcow_co_writev,
.bdrv_co_is_allocated = qcow_co_is_allocated,
[Qemu-devel] [v1 Patch 4/10]Qemu: Framework for reopening image files safely
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
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 DisplayChang
[Qemu-devel] [v1 Patch 2/10]Qemu: Error classes for hostcache setting and data sync failures
New error classes defined for hostcache setting and data
sync error
Signed-off-by: Supriya Kannery
---
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 } }"
[Qemu-devel] [v1 Patch 5/10]Qemu: raw-posix image file reopen
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
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 voi
[Qemu-devel] [v1 Patch 6/10]Qemu: raw-win32 image file reopen
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
Signed-off-by: Shrinidhi Joshi
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
+#include
#include
#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)
{
Re: [Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting
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.
Re: [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change
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
[Qemu-devel] [V4 Patch 1/4]Qemu: Enhance "info block" to display host cache setting
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
---
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,
[Qemu-devel] [V4 Patch 2/4]Qemu: Error classes for file reopen and data sync failure
New error classes defined for cases where device not inserted
and file reopen failed.
Signed-off-by: Supriya Kannery
---
qerror.c |8
qerror.h |6 ++
2 files changed, 14 insertions(+)
Index: qemu/qerror.c
===
--- qemu.orig/qerror.c
+++ qemu/qerror.c
@@ -97,6 +97,14 @@ static const QErrorStringTable qerror_ta
.desc = "Device '%(device)' is not removable",
},
{
+.error_fmt = QERR_DATA_SYNC_FAILED,
+.desc = "Syncing of data failed for device '%(device)'",
+},
+{
+.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_DATA_SYNC_FAILED \
+"{ 'class': 'DataSyncFailed', '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 } }"
[Qemu-devel] [V4 Patch 0/4]Qemu: Hostcache setting from cmdline and monitor
Currently cache setting of a block device cannot be changed without restarting a running VM. Following patchset is for enabling dynamic change of cache setting for block devices through qemu monitor. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. This patchset introduces monitor command 'block_set' using which various parameters for block devices can be changed dynamically. Dynamic changing of host cache setting is implemented for now. This patchset also introduces 'hostcache', a new option for setting host cache from qemu command line. 'Hostcache and 'cache' options cannot be used simultaneously from commandline. Changes from patchset V3: 1. Added 'hostcache' option to '-drive' commandline option. New block command added: "block_set" -- Sets block device parameters while guest is running. Usage: block_set = block device = parameter (say, "hostcache") = on/off New 'hostcache' option added to -drive: -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" " [,readonly=on|off][,hostcache=on|off]\n" 1/4 Enhance "info block" to display hostcache setting 2/4 New error classes for file reopen and device insertion 3/4 Command "block_set" for dynamic params change for block device 4/4 'hostcache' option added to -drive in qemu commandline qemu/block.c | 73 +++ qemu/block.h |2 + qemu/blockdev.c | 43 +++ qemu/blockdev.h |1 qemu/hmp-commands.hx | 15 ++ qemu/qemu-config.c |4 +++ qemu/qemu-options.hx |2 - qemu/qemu.pod|5 qemu/qerror.c|8 +++ qemu/qerror.h|6 + qemu/qmp-commands.hx | 30 +++ 11 files changed, 184 insertions(+), 5 deletions(-)
[Qemu-devel] [V4 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
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
---
block.c | 52
block.h |2 ++
blockdev.c | 26 ++
blockdev.h |1 +
hmp-commands.hx | 15 +++
qmp-commands.hx | 28
6 files changed, 124 insertions(+)
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -651,6 +651,32 @@ 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);
+
+/*
+ * 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 +717,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);
@@ -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.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -797,3 +797,29 @@ 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")) {
+return bdrv_change_hostcache(bs, enable);
+}
+
+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,21 @@ but should be used with extreme caution.
resizes image files, it can not resize block devices like LVM volumes.
ETEXI
+
[Qemu-devel] [V4 Patch 4/4]Qemu: Add commandline -drive option 'hostcache'
qemu command option 'hostcache' added to -drive for block devices.
While starting a VM from qemu commandline, this option can be used
for setting host cache usage for block data access. It is not
allowed to specify both 'hostcache' and 'cache' options in the same
commandline. User has to specify only one among these.
Signed-off-by: Supriya Kannery
---
blockdev.c | 17 +
qemu-config.c |4
qemu-options.hx |2 +-
qemu.pod|5 +
4 files changed, 27 insertions(+), 1 deletion(-)
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -238,6 +238,7 @@ DriveInfo *drive_init(QemuOpts *opts, in
DriveInfo *dinfo;
int snapshot = 0;
int ret;
+bool option_hostcache = false;
translation = BIOS_ATA_TRANSLATION_AUTO;
@@ -324,7 +325,23 @@ DriveInfo *drive_init(QemuOpts *opts, in
}
}
+if ((buf = qemu_opt_get(opts, "hostcache")) != NULL) {
+if (!strcmp(buf, "off")) {
+bdrv_flags |= BDRV_O_NOCACHE;
+} else if (!strcmp(buf, "on")) {
+bdrv_flags &= ~BDRV_O_NOCACHE;
+} else {
+error_report("invalid hostcache option");
+return NULL;
+}
+option_hostcache = true;
+}
+
if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
+if (option_hostcache) {
+error_report("'hostcache' and 'cache' cannot co-exist");
+return NULL;
+}
if (!strcmp(buf, "off") || !strcmp(buf, "none")) {
bdrv_flags |= BDRV_O_NOCACHE | BDRV_O_CACHE_WB;
} else if (!strcmp(buf, "writeback")) {
Index: qemu/qemu-options.hx
===
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -120,7 +120,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
" [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
" [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
" [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
-" [,readonly=on|off]\n"
+" [,readonly=on|off][,hostcache=on|off]\n"
"use 'file' as a drive image\n", QEMU_ARCH_ALL)
STEXI
@item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
Index: qemu/qemu-config.c
===
--- qemu.orig/qemu-config.c
+++ qemu/qemu-config.c
@@ -78,6 +78,10 @@ static QemuOptsList qemu_drive_opts = {
},{
.name = "readonly",
.type = QEMU_OPT_BOOL,
+},{
+.name = "hostcache",
+.type = QEMU_OPT_BOOL,
+.help = "set or reset hostcache (on/off)"
},
{ /* end of list */ }
},
Index: qemu/qemu.pod
===
--- qemu.orig/qemu.pod
+++ qemu/qemu.pod
@@ -226,6 +226,11 @@ I is "on" or "off" and allows
I is "none", "writeback", "unsafe", or "writethrough" and controls how
the host cache is used to access block data.
+=item BI
+
+I is "on" or "off" and allows to enable or disable hostcache while
accessing block data.
+Both 'cache=' and 'hostcache=' should not used in same command line. Use only
one among them.
+
=item BI
I is "threads", or "native" and selects between pthread based disk I/O
and native Linux AIO.
Re: [Qemu-devel] [V4 Patch 1/4 -Updated]Qemu: Enhance "info block" to display host cache setting
Updated patch to display hostcache = 1/0 instead of true/false
in monitor.
---
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
---
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=0");
+else
+monitor_printf(mon, " hostcache=1");
+}
+
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,
Re: [Qemu-devel] [V4 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On 07/04/2011 05:59 PM, Stefan Hajnoczi wrote:
On Mon, Jul 4, 2011 at 11:43 AM, Supriya Kannery
wrote:
{
+.name = "block_set",
+.args_type = "device:B,name:s,enable:b",
+.params = "device name enable",
Perhaps:
.args_type = "device:B,name:s,enable:b?",
.params = "device name [enable]",
But I am not sure what the best way to express this is.
+.help = "Enable/Disable block device params like hostcache",
Arguments (like "enable") should depend on the block parameter that is
being set:
.help = "Set block device parameter"
If there is no good way to support different optional arguments and
types then a json-string "value" argument would be best.
"device_add" is defined and implemented to handle multiple types of
optional arguments. Will work on to see whether that approach
can be adopted for block_set
{
.name = "device_add",
.args_type = "device:O",
.params = "driver[,prop=value][,...]",
.help = "add device, like -device on the command line",
.user_print = monitor_user_noop,
.mhandler.cmd_new = do_device_add,
},
Stefan
Re: [Qemu-devel] [V4 Patch 4/4 - Updated]Qemu: Add commandline -drive option 'hostcache'
Updated patch to use qemu_opt_get_bool() instead of qemu_opt_get()
to read 'hostcache'
---
qemu command option 'hostcache' added to -drive for block devices.
While starting a VM from qemu commandline, this option can be used
for setting host cache usage for block data access. It is not
allowed to specify both 'hostcache' and 'cache' options in the same
commandline. User has to specify only one among these.
Signed-off-by: Supriya Kannery
---
blockdev.c | 13 +
qemu-config.c |4
qemu-options.hx |2 +-
3 files changed, 18 insertions(+), 1 deletion(-)
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -238,6 +238,7 @@ DriveInfo *drive_init(QemuOpts *opts, in
DriveInfo *dinfo;
int snapshot = 0;
int ret;
+int hostcache = 0;
translation = BIOS_ATA_TRANSLATION_AUTO;
@@ -324,7 +325,19 @@ DriveInfo *drive_init(QemuOpts *opts, in
}
}
+if ((hostcache = qemu_opt_get_bool(opts, "hostcache", -1)) != -1) {
+if (!hostcache) {
+bdrv_flags |= BDRV_O_NOCACHE;
+} else {
+bdrv_flags &= ~BDRV_O_NOCACHE;
+}
+}
+
if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
+if (hostcache != -1) {
+error_report("'hostcache' and 'cache' cannot co-exist");
+return NULL;
+}
if (!strcmp(buf, "off") || !strcmp(buf, "none")) {
bdrv_flags |= BDRV_O_NOCACHE | BDRV_O_CACHE_WB;
} else if (!strcmp(buf, "writeback")) {
Index: qemu/qemu-options.hx
===
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -120,7 +120,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
" [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
" [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
" [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
-" [,readonly=on|off]\n"
+" [,readonly=on|off][,hostcache=on|off]\n"
"use 'file' as a drive image\n", QEMU_ARCH_ALL)
STEXI
@item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
Index: qemu/qemu-config.c
===
--- qemu.orig/qemu-config.c
+++ qemu/qemu-config.c
@@ -78,6 +78,10 @@ static QemuOptsList qemu_drive_opts = {
},{
.name = "readonly",
.type = QEMU_OPT_BOOL,
+},{
+.name = "hostcache",
+.type = QEMU_OPT_BOOL,
+.help = "set or reset hostcache (on/off)"
},
{ /* end of list */ }
},
Re: [Qemu-devel] [V4 Patch 3/4 - Updated]Qemu: Command "block_set" for dynamic block params change
Updated "block_set" command to accept multiple -drive parameters.
Also, added code for re-opening of device file with original flags,
incase opening file using changed hostcache setting fails.
--
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
---
block.c | 60
block.h |2 +
blockdev.c | 60
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, 205 insertions(+)
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);
+
+ret = bdrv_open(bs, bs->filename, bs->open_flags, drv);
+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;
+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);
@@ -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.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
Re: [Qemu-devel] [V4 Patch 3/4 - Updated]Qemu: Command "block_set" for dynamic block params change
On 07/13/2011 06:37 PM, Supriya Kannery wrote:
Updated "block_set" command to accept multiple -drive parameters.
Also, added code for re-opening of device file with original flags,
incase opening file using changed hostcache setting fails.
--
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
---
block.c | 60
block.h | 2 +
blockdev.c | 60
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, 205 insertions(+)
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);
+
+ ret = bdrv_open(bs, bs->filename, bs->open_flags, drv);
+ 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;
+ 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);
@@ -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.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);
+ return -1;
+ }
+
+ /* Before validating parameters, remove "device" option */
+ ret = qemu_opt_delete(opts, "device");
+ if (ret == 1) {
+ error_report("Specify pa
Re: [Qemu-devel] [V4 Patch 4/4 - Updated]Qemu: Add commandline -drive option 'hostcache'
On 07/05/2011 04:35 PM, Supriya Kannery wrote:
Updated patch to use qemu_opt_get_bool() instead of qemu_opt_get()
to read 'hostcache'
---
qemu command option 'hostcache' added to -drive for block devices.
While starting a VM from qemu commandline, this option can be used
for setting host cache usage for block data access. It is not
allowed to specify both 'hostcache' and 'cache' options in the same
commandline. User has to specify only one among these.
Signed-off-by: Supriya Kannery
---
blockdev.c | 13 +
qemu-config.c | 4
qemu-options.hx | 2 +-
3 files changed, 18 insertions(+), 1 deletion(-)
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -238,6 +238,7 @@ DriveInfo *drive_init(QemuOpts *opts, in
DriveInfo *dinfo;
int snapshot = 0;
int ret;
+ int hostcache = 0;
translation = BIOS_ATA_TRANSLATION_AUTO;
@@ -324,7 +325,19 @@ DriveInfo *drive_init(QemuOpts *opts, in
}
}
+ if ((hostcache = qemu_opt_get_bool(opts, "hostcache", -1)) != -1) {
+ if (!hostcache) {
+ bdrv_flags |= BDRV_O_NOCACHE;
+ } else {
+ bdrv_flags &= ~BDRV_O_NOCACHE;
+ }
+ }
+
if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
+ if (hostcache != -1) {
+ error_report("'hostcache' and 'cache' cannot co-exist");
+ return NULL;
+ }
if (!strcmp(buf, "off") || !strcmp(buf, "none")) {
bdrv_flags |= BDRV_O_NOCACHE | BDRV_O_CACHE_WB;
} else if (!strcmp(buf, "writeback")) {
Index: qemu/qemu-options.hx
===
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -120,7 +120,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
" [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
" [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
" [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
- " [,readonly=on|off]\n"
+ " [,readonly=on|off][,hostcache=on|off]\n"
" use 'file' as a drive image\n", QEMU_ARCH_ALL)
STEXI
@item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
Index: qemu/qemu-config.c
===
--- qemu.orig/qemu-config.c
+++ qemu/qemu-config.c
@@ -78,6 +78,10 @@ static QemuOptsList qemu_drive_opts = {
},{
.name = "readonly",
.type = QEMU_OPT_BOOL,
+ },{
+ .name = "hostcache",
+ .type = QEMU_OPT_BOOL,
+ .help = "set or reset hostcache (on/off)"
},
{ /* end of list */ }
},
Please review..
Re: [Qemu-devel] [V4 Patch 1/4 -Updated]Qemu: Enhance "info block" to display host cache setting
On 07/05/2011 04:19 PM, Supriya Kannery wrote:
Updated patch to display hostcache = 1/0 instead of true/false
in monitor.
---
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
---
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=0");
+ else
+ monitor_printf(mon, " hostcache=1");
+ }
+
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,
Pls review..
[Qemu-devel] [v8 Patch 0/6]Qemu: Host pagecache setting from cmdline and monitor
Currently cache setting of a block device cannot be changed without restarting a running VM. Following patchset is for enabling dynamic change of cache setting for block devices through qemu monitor. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. This patchset introduces a. monitor command 'block_set_hostcache' using which host pagecache setting for a block device can be changed dynamically. b. a new option for setting host cache from qemu commandline option -drive "hostcache=on/off". c. BDRVReopenState, a generic structure which can be extended by each of the block drivers to reopen respective image files. Extension of this structure for drivers raw-posix is done here. d. 'hostcache and 'cache' options when used together, cache=xx will override hostcache=yy. v8: 1. Mandate implementation of all three reopen related functions by block drivers. 2. If 'cache=xx' and 'hostcache=yy' specified in cmdline, 'cache=' overrides 'hostcache='. v7: 1. Added structure BDRVReopenState to support safe reopening of image files. 2. Implemented reopen functions for raw-posix driver v6: 1. "block_set_hostcache" to replace "block_set" command v5: 1. Defined qerror class for incorrect command syntax. 2. Changed error_report() calls to qerror_report() v4: Added 'hostcache' option to '-drive' commandline option. v3: 1. Command "block_set" for changing various block params 2. Enhanced info-block to display hostcache setting 3. Added qmp interfaces for setting and querying hostcache v2: 1. Support of dynamic cache change only for hostcache. 2. Monitor command "hostcache_get" added to display cache setting 3. Backed off the changes for display of cache setting in "info block" v1: Dynamic cache change through monitor New block command added: "block_set_hostcache" -- Sets hostcache parameter for block device while guest is running. Usage: block_set_hostcache = block device = on/off New 'hostcache' option added to -drive: -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" " [,readonly=on|off][,hostcache=on|off]\n" qemu/block.c | 79 +++ qemu/block.h |3 ++ qemu/block/raw-posix.c | 57 + qemu/block/raw.c | 23 +++- qemu/block_int.h | 16 +++ qemu/blockdev.c|7 + qemu/qemu-common.h |1 qemu/qemu-config.c |4 ++ qemu/qemu-options.hx |2 - qemu/qerror.c |8 + qemu/qerror.h |6 qemu/qmp-commands.hx |4 ++ 14 files changed, 194 insertions(+), 16 deletions(-) ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ "txt" 13L, 574C
[Qemu-devel] [v8 Patch 1/6]Qemu: Enhance "info block" to display host cache setting
Enhance "info block" to display hostcache setting for each
block device.
Example:
(qemu) info block
ide0-hd0: removable=0 file=../sles11-32.raw ro=0 drv=raw encrypted=0
Enhanced to display "hostcache" setting:
(qemu) info block
ide0-hd0: removable=0 hostcache=1 file=../sles11-32.raw ro=0 drv=raw encrypted=0
Signed-off-by: Supriya Kannery
---
block.c | 20
qmp-commands.hx |2 ++
2 files changed, 18 insertions(+), 4 deletions(-)
Index: qemu/qmp-commands.hx
===
--- qemu.orig/qmp-commands.hx
+++ qemu/qmp-commands.hx
@@ -1142,6 +1142,7 @@ Each json-object contain the following:
- "locked": true if the device is locked, false otherwise (json-bool)
- "tray-open": only present if removable, true if the device has a tray,
and it is open (json-bool)
+- "hostcache": true if host pagecache 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)
@@ -1168,6 +1169,7 @@ Example:
"io-status": "ok",
"device":"ide0-hd0",
"locked":false,
+"hostcache":false,
"removable":false,
"inserted":{
"ro":false,
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -1841,6 +1841,11 @@ static void bdrv_print_dict(QObject *obj
qdict_get_bool(bs_dict, "tray-open"));
}
+if (qdict_haskey(bs_dict, "hostcache")) {
+monitor_printf(mon, " hostcache=%d",
+ qdict_get_bool(bs_dict, "hostcache"));
+}
+
if (qdict_haskey(bs_dict, "io-status")) {
monitor_printf(mon, " io-status=%s", qdict_get_str(bs_dict,
"io-status"));
}
@@ -1888,10 +1893,12 @@ void bdrv_info(Monitor *mon, QObject **r
QDict *bs_dict;
bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', "
-"'removable': %i, 'locked': %i }",
+"'removable': %i, 'locked': %i, "
+"'hostcache': %i }",
bs->device_name,
bdrv_dev_has_removable_media(bs),
-bdrv_dev_is_medium_locked(bs));
+bdrv_dev_is_medium_locked(bs),
+!(bs->open_flags & BDRV_O_NOCACHE));
bs_dict = qobject_to_qdict(bs_obj);
if (bdrv_dev_has_removable_media(bs)) {
[Qemu-devel] [v8 Patch 2/6]Qemu: Error classes for file reopen and data sync failure
New error classes defined for file reopen failure and data
sync error
Signed-off-by: Supriya Kannery
---
qerror.c |8
qerror.h |6 ++
2 files changed, 14 insertions(+)
Index: qemu/qerror.c
===
--- qemu.orig/qerror.c
+++ qemu/qerror.c
@@ -97,6 +97,14 @@ static const QErrorStringTable qerror_ta
.desc = "Device '%(device)' is not removable",
},
{
+.error_fmt = QERR_DATA_SYNC_FAILED,
+.desc = "Syncing of data failed for device '%(device)'",
+},
+{
+.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
@@ -87,6 +87,9 @@ QError *qobject_to_qerror(const QObject
#define QERR_DEVICE_NOT_FOUND \
"{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
+#define QERR_DATA_SYNC_FAILED \
+"{ 'class': 'DataSyncFailed', 'data': { 'device': %s } }"
+
#define QERR_DEVICE_NOT_REMOVABLE \
"{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
@@ -144,6 +147,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 } }"
[Qemu-devel] [v8 Patch 4/6]Qemu: Add commandline -drive option 'hostcache'
qemu command option 'hostcache' added to -drive for block devices.
While starting a VM from qemu commandline, this option can be used
for setting host cache usage for block data access. 'cache=xx'
overrides 'hostcache=yy' when specified simultaneously.
Signed-off-by: Supriya Kannery
---
blockdev.c | 13 +
qemu-config.c |4
qemu-options.hx |2 +-
3 files changed, 18 insertions(+), 1 deletion(-)
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -237,6 +237,7 @@ DriveInfo *drive_init(QemuOpts *opts, in
DriveInfo *dinfo;
int snapshot = 0;
int ret;
+int hostcache = 0;
translation = BIOS_ATA_TRANSLATION_AUTO;
media = MEDIA_DISK;
@@ -324,6 +325,12 @@ DriveInfo *drive_init(QemuOpts *opts, in
error_report("invalid cache option");
return NULL;
}
+} else {
+if ((hostcache = qemu_opt_get_bool(opts, "hostcache", -1)) != -1) {
+if (!hostcache) {
+bdrv_flags |= BDRV_O_NOCACHE;
+}
+}
}
#ifdef CONFIG_LINUX_AIO
Index: qemu/qemu-options.hx
===
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -135,7 +135,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
" [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
"
[,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
" [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
-" [,readonly=on|off]\n"
+" [,readonly=on|off][,hostcache=on|off]\n"
"use 'file' as a drive image\n", QEMU_ARCH_ALL)
STEXI
@item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
Index: qemu/qemu-config.c
===
--- qemu.orig/qemu-config.c
+++ qemu/qemu-config.c
@@ -85,6 +85,10 @@ static QemuOptsList qemu_drive_opts = {
.name = "readonly",
.type = QEMU_OPT_BOOL,
.help = "open drive file as read-only",
+},{
+.name = "hostcache",
+.type = QEMU_OPT_BOOL,
+.help = "set or reset hostcache (on/off)"
},
{ /* end of list */ }
},
[Qemu-devel] [v8 Patch 6/6]Qemu: raw posix implementation of reopen functions
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
Index: qemu/block/raw.c
===
--- qemu.orig/block/raw.c
+++ qemu/block/raw.c
@@ -9,6 +9,24 @@ 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,
+ int flags)
+{
+bdrv_reopen_commit(bs->file, rs, flags);
+bs->open_flags = bs->file->open_flags;
+}
+
+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)
{
@@ -107,7 +125,10 @@ static BlockDriver bdrv_raw = {
/* It's really 0, but we need to make g_malloc() happy */
.instance_size = 1,
-
+.bdrv_reopen_prepare
+= raw_reopen_prepare,
+.bdrv_reopen_commit = raw_reopen_commit,
+.bdrv_reopen_abort = raw_reopen_abort,
.bdrv_open = raw_open,
.bdrv_close = raw_close,
Index: qemu/block/raw-posix.c
===
--- qemu.orig/block/raw-posix.c
+++ qemu/block/raw-posix.c
@@ -279,6 +279,60 @@ 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)
+{
+BDRVReopenState *raw_rs = g_malloc0(sizeof(BDRVReopenState));
+BDRVRawState *s = bs->opaque;
+
+raw_rs->bs = bs;
+raw_rs->reopen_flags = s->open_flags;
+raw_rs->reopen_fd = -1;
+*prs = raw_rs;
+int ret = 0;
+
+/* If only O_DIRECT to be toggled, use fcntl */
+if (!((bs->open_flags & ~BDRV_O_NOCACHE) ^
+(flags & ~BDRV_O_NOCACHE))) {
+raw_rs->reopen_fd = dup(s->fd);
+if (raw_rs->reopen_fd <= 0) {
+return -1;
+}
+if ((flags & BDRV_O_NOCACHE)) {
+raw_rs->reopen_flags |= O_DIRECT;
+} else {
+raw_rs->reopen_flags &= ~O_DIRECT;
+}
+ret = fcntl_setfl(raw_rs->reopen_fd, raw_rs->reopen_flags);
+}
+
+/* TBD: Handle O_DSYNC and other flags */
+
+return ret;
+}
+
+static void raw_reopen_commit(BlockDriverState *bs, BDRVReopenState *rs,
+ int flags)
+{
+BDRVRawState *s = bs->opaque;
+
+/* Set new flags, so replace old fd with new one */
+close(s->fd);
+s->fd = rs->reopen_fd;
+s->open_flags = rs->reopen_flags;
+bs->open_flags = flags;
+g_free(rs);
+
+}
+
+static void raw_reopen_abort(BlockDriverState *bs, BDRVReopenState *rs)
+{
+if (rs->reopen_fd != -1) {
+close(rs->reopen_fd);
+}
+g_free(rs);
+}
+
/* XXX: use host sector size if necessary with:
#ifdef DIOCGSECTORSIZE
{
@@ -631,6 +685,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,
[Qemu-devel] [v8 Patch 5/6]Qemu: Framework for reopening images safely
Struct BDRVReopenState along with three reopen related functions
introduced for handling reopen state of images safely. This can be
extended by each of the block drivers to reopen respective
image files.
Signed-off-by: Supriya Kannery
Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -693,10 +693,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, int flags)
+{
+BlockDriver *drv = bs->drv;
+
+drv->bdrv_reopen_commit(bs, rs, flags);
+}
+
+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 *rs = NULL;
/* Quiesce IO for the given block device */
qemu_aio_flush();
@@ -704,20 +726,35 @@ 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_REOPEN_FILE_FAILED, bs->filename);
-ret = bdrv_open(bs, bs->filename, open_flags, drv);
+/* Use driver specific reopen() if available */
+if (drv->bdrv_reopen_prepare) {
+ret = bdrv_reopen_prepare(bs, &rs, bdrv_flags);
if (ret < 0) {
-/* Reopen failed with orig and modified flags */
-abort();
+bdrv_reopen_abort(bs, rs);
+qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+return ret;
}
-}
+bdrv_reopen_commit(bs, rs, bdrv_flags);
+
+} else {
+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
+ * For next access to fail, set drv to NULL
+*/
+bs->drv = NULL;
+}
+}
+}
return ret;
}
Index: qemu/block_int.h
===
--- qemu.orig/block_int.h
+++ qemu/block_int.h
@@ -55,6 +55,14 @@ 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 **rs,
+ int flags);
+void (*bdrv_reopen_commit)(BlockDriverState *bs, BDRVReopenState *rs,
+ int flags);
+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);
@@ -211,6 +219,14 @@ struct BlockDriverState {
void *private;
};
+struct BDRVReopenState {
+BlockDriverState *bs;
+int reopen_flags;
+
+/* For raw-posix */
+int reopen_fd;
+};
+
struct BlockDriverAIOCB {
AIOPool *pool;
BlockDriverState *bs;
Index: qemu/qemu-common.h
===
--- qemu.orig/qemu-common.h
+++ qemu/qemu-common.h
@@ -202,6 +202,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
@@ -110,6 +110,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, int
[Qemu-devel] [v8 Patch 3/6]Qemu: Cmd "block_set_hostcache" for dynamic cache change
New command "block_set_hostcache" added for dynamically changing
host pagecache setting of a block device.
Usage:
block_set_hostcache
= block device
= on/off
Example:
(qemu) block_set_hostcache ide0-hd0 off
Signed-off-by: Supriya Kannery
---
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
@@ -693,6 +693,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) {
@@ -730,6 +758,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
@@ -109,6 +109,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);
@@ -143,6 +144,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
@@ -776,3 +776,29 @@ int do_block_resize(Monitor *mon, const
return 0;
}
+
+
+/*
+ * Change host page cache setting while guest is running.
+*/
+int do_block_set_hostcache(Monitor *mon, const QDict *qdict,
+ QObject **ret_data)
+{
+BlockDriverState *bs = NULL;
+int enable;
+const char *device;
+
+/* Validate device */
+device = qdict_get_str(qdict, "device");
+bs = bdrv_find(device);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+/* Read hostcache setting */
+enable = qdict_get_bool(qdict, "option");
+return bdrv_change_hostcache(bs, enable);
+
+}
+
Index: qemu/blockdev.h
===
--- qemu.orig/blockdev.h
+++ qemu/blockdev.h
@@ -65,5 +65,7 @@ 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_hostcache(Monitor *mon, const QDict *qdict,
+ QObject **ret_data);
#endif
Index: qemu/hmp-commands.hx
===
--- qemu.orig/hmp-commands.
Re: [Qemu-devel] [v8 Patch 1/6]Qemu: Enhance "info block" to display host cache setting
On 11/03/2011 07:25 PM, Luiz Capitulino wrote:
On Sun, 30 Oct 2011 16:03:54 +0530
Supriya Kannery wrote:
+if (qdict_haskey(bs_dict, "hostcache")) {
+monitor_printf(mon, " hostcache=%d",
+ qdict_get_bool(bs_dict, "hostcache"));
+}
This series needs to be rebased, as the info block command has been
converted to the QAPI. Please, see the following commit for details:
b202381800d81fbff9978abbdea95760dd363bb6.
Also note that if you're adding new commands (I haven't reviewed the
series) you should use the QAPI. A document on how to use it is coming soon.
yes, will rebase and use QAPI
if (qdict_haskey(bs_dict, "io-status")) {
monitor_printf(mon, " io-status=%s", qdict_get_str(bs_dict,
"io-status"));
}
Re: [Qemu-devel] [v8 Patch 3/6]Qemu: Cmd "block_set_hostcache" for dynamic cache change
On 11/04/2011 03:30 PM, Kevin Wolf wrote:
Am 30.10.2011 11:34, schrieb Supriya Kannery:
+
+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;
+}
Maybe the !bdrv_is_inserted() case should be handled in bdrv_reopen(). I
think it would be the same for changing other flags.
I am yet to look at the conditions specific to other flags. So I can
move this check to bdrv_reopen when implementing one more flag I guess.
Kevin
Re: [Qemu-devel] [v8 Patch 5/6]Qemu: Framework for reopening images safely
On 11/04/2011 03:35 PM, Kevin Wolf wrote:
Am 30.10.2011 11:35, schrieb Supriya Kannery:
+struct BDRVReopenState {
+BlockDriverState *bs;
+int reopen_flags;
+
+/* For raw-posix */
+int reopen_fd;
+};
I think I commented the same on the previous version: BDRVReopenState
shouldn't contain any format specific fields. raw-posix must extend the
struct like this and use container_of() to get it from a BDRVReopenState
pointer:
struct BDRVRawReopenState {
BDRVReopenState common;
int reopen_fd;
};
I don't recall this was suggested in prev version or may be I missed to
notice..
ok, will have raw extending common BDRVReopenState struct.
Kevin
