* [Qemu-devel] [PATCH v3 01/19] block: correctly set the keep_read_only flag
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 02/19] block: make bdrv_set_enable_write_cache() modify open_flags Jeff Cody
` (17 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
I believe the bs->keep_read_only flag is supposed to reflect
the initial open state of the device. If the device is initially
opened R/O, then commit operations, or reopen operations changing
to R/W, are prohibited.
Currently, the keep_read_only flag is only accurate for the active
layer, and its backing file. Subsequent images end up always having
the keep_read_only flag set.
For instance, what happens now:
[ base ] kro = 1, ro = 1
|
v
[ snap-1 ] kro = 1, ro = 1
|
v
[ snap-2 ] kro = 0, ro = 1
|
v
[ active ] kro = 0, ro = 0
What we want:
[ base ] kro = 0, ro = 1
|
v
[ snap-1 ] kro = 0, ro = 1
|
v
[ snap-2 ] kro = 0, ro = 1
|
v
[ active ] kro = 0, ro = 0
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 14 +++++++-------
block.h | 1 +
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index 470bdcc..c3eb585 100644
--- a/block.c
+++ b/block.c
@@ -664,7 +664,7 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename,
open_flags |= BDRV_O_RDWR;
}
- bs->keep_read_only = bs->read_only = !(open_flags & BDRV_O_RDWR);
+ bs->read_only = !(open_flags & BDRV_O_RDWR);
/* Open the image, either directly or using a protocol */
if (drv->bdrv_file_open) {
@@ -804,6 +804,12 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
goto unlink_and_fail;
}
+ if (flags & BDRV_O_RDWR) {
+ flags |= BDRV_O_ALLOW_RDWR;
+ }
+
+ bs->keep_read_only = !(flags & BDRV_O_ALLOW_RDWR);
+
/* Open the image */
ret = bdrv_open_common(bs, filename, flags, drv);
if (ret < 0) {
@@ -833,12 +839,6 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
bdrv_close(bs);
return ret;
}
- if (bs->is_temporary) {
- bs->backing_hd->keep_read_only = !(flags & BDRV_O_RDWR);
- } else {
- /* base image inherits from "parent" */
- bs->backing_hd->keep_read_only = bs->keep_read_only;
- }
}
if (!bdrv_key_required(bs)) {
diff --git a/block.h b/block.h
index 2e2be11..4d919c2 100644
--- a/block.h
+++ b/block.h
@@ -80,6 +80,7 @@ typedef struct BlockDevOps {
#define BDRV_O_COPY_ON_READ 0x0400 /* copy read backing sectors into image */
#define BDRV_O_INCOMING 0x0800 /* consistency hint for incoming migration */
#define BDRV_O_CHECK 0x1000 /* open solely for consistency check */
+#define BDRV_O_ALLOW_RDWR 0x2000 /* allow reopen to change from r/o to r/w */
#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 02/19] block: make bdrv_set_enable_write_cache() modify open_flags
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 01/19] block: correctly set the keep_read_only flag Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 03/19] block: Framework for reopening files safely Jeff Cody
` (16 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
bdrv_set_enable_write_cache() sets the bs->enable_write_cache flag,
but without the flag recorded in bs->open_flags, then next time
a reopen() is performed the enable_write_cache setting may be
inadvertently lost.
This will set the flag in open_flags, so it is preserved across
reopens.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/block.c b/block.c
index c3eb585..2b96c8e 100644
--- a/block.c
+++ b/block.c
@@ -2164,6 +2164,13 @@ int bdrv_enable_write_cache(BlockDriverState *bs)
void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce)
{
bs->enable_write_cache = wce;
+
+ /* so a reopen() will preserve wce */
+ if (wce) {
+ bs->open_flags |= BDRV_O_CACHE_WB;
+ } else {
+ bs->open_flags &= ~BDRV_O_CACHE_WB;
+ }
}
int bdrv_is_encrypted(BlockDriverState *bs)
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 03/19] block: Framework for reopening files safely
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 01/19] block: correctly set the keep_read_only flag Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 02/19] block: make bdrv_set_enable_write_cache() modify open_flags Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-20 11:24 ` Kevin Wolf
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 04/19] block: move aio initialization into a helper function Jeff Cody
` (15 subsequent siblings)
18 siblings, 1 reply; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
This is based on Supriya Kannery's bdrv_reopen() patch series.
This provides a transactional method to reopen multiple
images files safely.
Image files are queue for reopen via bdrv_reopen_queue(), and the
reopen occurs when bdrv_reopen_multiple() is called. Changes are
staged in bdrv_reopen_prepare() and in the equivalent driver level
functions. If any of the staged images fails a prepare, then all
of the images left untouched, and the staged changes for each image
abandoned.
Block drivers are passed a reopen state structure, that contains:
* BDS to reopen
* flags for the reopen
* opaque pointer for any driver-specific data that needs to be
persistent from _prepare to _commit/_abort
* reopen queue pointer, if the driver needs to queue additional
BDS for a reopen
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 232 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
block.h | 17 +++++
block_int.h | 8 ++
qemu-common.h | 1 +
4 files changed, 258 insertions(+)
diff --git a/block.c b/block.c
index 2b96c8e..d625703 100644
--- a/block.c
+++ b/block.c
@@ -859,6 +859,238 @@ unlink_and_fail:
return ret;
}
+typedef struct BlockReopenQueueEntry {
+ bool prepared;
+ BDRVReopenState state;
+ QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;
+} BlockReopenQueueEntry;
+
+/*
+ * Adds a BlockDriverState to a simple queue for an atomic, transactional
+ * reopen of multiple devices.
+ *
+ * bs_queue can either be an existing BlockReopenQueue that has had QSIMPLE_INIT
+ * already performed, or alternatively may be NULL a new BlockReopenQueue will
+ * be created and initialized. This newly created BlockReopenQueue should be
+ * passed back in for subsequent calls that are intended to be of the same
+ * atomic 'set'.
+ *
+ * bs is the BlockDriverState to add to the reopen queue.
+ *
+ * flags contains the open flags for the associated bs
+ *
+ * returns a pointer to bs_queue, which is either the newly allocated
+ * bs_queue, or the existing bs_queue being used.
+ *
+ */
+BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
+ BlockDriverState *bs, int flags)
+{
+ assert(bs != NULL);
+
+ BlockReopenQueueEntry *bs_entry;
+ if (bs_queue == NULL) {
+ bs_queue = g_new0(BlockReopenQueue, 1);
+ QSIMPLEQ_INIT(bs_queue);
+ }
+
+ if (bs->file) {
+ bdrv_reopen_queue(bs_queue, bs->file, flags);
+ }
+
+ bs_entry = g_new0(BlockReopenQueueEntry, 1);
+ QSIMPLEQ_INSERT_TAIL(bs_queue, bs_entry, entry);
+
+ bs_entry->state.bs = bs;
+ bs_entry->state.flags = flags;
+
+ return bs_queue;
+}
+
+/*
+ * Reopen multiple BlockDriverStates atomically & transactionally.
+ *
+ * The queue passed in (bs_queue) must have been built up previous
+ * via bdrv_reopen_queue().
+ *
+ * Reopens all BDS specified in the queue, with the appropriate
+ * flags. All devices are prepared for reopen, and failure of any
+ * device will cause all device changes to be abandonded, and intermediate
+ * data cleaned up.
+ *
+ * If all devices prepare successfully, then the changes are committed
+ * to all devices.
+ *
+ */
+int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
+{
+ int ret = -1;
+ BlockReopenQueueEntry *bs_entry, *next;
+ Error *local_err = NULL;
+
+ assert(bs_queue != NULL);
+
+ bdrv_drain_all();
+
+ QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+ if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, &local_err)) {
+ error_propagate(errp, local_err);
+ goto cleanup;
+ }
+ bs_entry->prepared = true;
+ }
+
+ /* If we reach this point, we have success and just need to apply the
+ * changes
+ */
+ QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+ bdrv_reopen_commit(&bs_entry->state);
+ }
+
+ ret = 0;
+
+cleanup:
+ QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
+ if (ret && bs_entry->prepared) {
+ bdrv_reopen_abort(&bs_entry->state);
+ }
+ g_free(bs_entry);
+ }
+ g_free(bs_queue);
+ return ret;
+}
+
+
+/* Reopen a single BlockDriverState with the specified flags. */
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp)
+{
+ int ret = -1;
+ Error *local_err = NULL;
+ BlockReopenQueue *queue = bdrv_reopen_queue(NULL, bs, bdrv_flags);
+
+ ret = bdrv_reopen_multiple(queue, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ }
+ return ret;
+}
+
+
+/*
+ * Prepares a BlockDriverState for reopen. All changes are staged in the
+ * 'opaque' field of the BDRVReopenState, which is used and allocated by
+ * the block driver layer .bdrv_reopen_prepare()
+ *
+ * bs is the BlockDriverState to reopen
+ * flags are the new open flags
+ * queue is the reopen queue
+ *
+ * Returns 0 on success, non-zero on error. On error errp will be set
+ * as well.
+ *
+ * On failure, bdrv_reopen_abort() will be called to clean up any data.
+ * It is the responsibility of the caller to then call the abort() or
+ * commit() for any other BDS that have been left in a prepare() state
+ *
+ */
+int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
+ Error **errp)
+{
+ int ret = -1;
+ Error *local_err = NULL;
+ BlockDriver *drv;
+
+ assert(reopen_state != NULL);
+ assert(reopen_state->bs->drv != NULL);
+ drv = reopen_state->bs->drv;
+
+ /* if we are to stay read-only, do not allow permission change
+ * to r/w */
+ if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
+ reopen_state->flags & BDRV_O_RDWR) {
+ error_set(errp, QERR_DEVICE_IS_READ_ONLY,
+ reopen_state->bs->device_name);
+ goto error;
+ }
+
+
+ ret = bdrv_flush(reopen_state->bs);
+ if (ret) {
+ error_set(errp, ERROR_CLASS_GENERIC_ERROR, "Error (%s) flushing drive",
+ strerror(-ret));
+ goto error;
+ }
+
+ if (drv->bdrv_reopen_prepare) {
+ ret = drv->bdrv_reopen_prepare(reopen_state, queue, &local_err);
+ if (ret) {
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ } else {
+ error_set(errp, QERR_OPEN_FILE_FAILED,
+ reopen_state->bs->filename);
+ }
+ goto error;
+ }
+ } else {
+ /* It is currently mandatory to have a bdrv_reopen_prepare()
+ * handler for each supported drv. */
+ error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+ drv->format_name, reopen_state->bs->device_name,
+ "reopening of file");
+ ret = -1;
+ goto error;
+ }
+
+ ret = 0;
+
+error:
+ return ret;
+}
+
+/*
+ * Takes the staged changes for the reopen from bdrv_reopen_prepare(), and
+ * makes them final by swapping the staging BlockDriverState contents into
+ * the active BlockDriverState contents.
+ */
+void bdrv_reopen_commit(BDRVReopenState *reopen_state)
+{
+ BlockDriver *drv;
+
+ assert(reopen_state != NULL);
+ drv = reopen_state->bs->drv;
+ assert(drv != NULL);
+
+ /* If there are any driver level actions to take */
+ if (drv->bdrv_reopen_commit) {
+ drv->bdrv_reopen_commit(reopen_state);
+ }
+
+ /* set BDS specific flags now */
+ reopen_state->bs->open_flags = reopen_state->flags;
+ reopen_state->bs->enable_write_cache = !!(reopen_state->flags &
+ BDRV_O_CACHE_WB);
+ reopen_state->bs->read_only = !(reopen_state->flags & BDRV_O_RDWR);
+}
+
+/*
+ * Abort the reopen, and delete and free the staged changes in
+ * reopen_state
+ */
+void bdrv_reopen_abort(BDRVReopenState *reopen_state)
+{
+ BlockDriver *drv;
+
+ assert(reopen_state != NULL);
+ drv = reopen_state->bs->drv;
+ assert(drv != NULL);
+
+ if (drv->bdrv_reopen_abort) {
+ drv->bdrv_reopen_abort(reopen_state);
+ }
+}
+
+
void bdrv_close(BlockDriverState *bs)
{
bdrv_flush(bs);
diff --git a/block.h b/block.h
index 4d919c2..b1095d8 100644
--- a/block.h
+++ b/block.h
@@ -97,6 +97,15 @@ typedef enum {
BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
} BlockQMPEventAction;
+typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue;
+
+typedef struct BDRVReopenState {
+ BlockDriverState *bs;
+ int flags;
+ void *opaque;
+} BDRVReopenState;
+
+
void bdrv_iostatus_enable(BlockDriverState *bs);
void bdrv_iostatus_reset(BlockDriverState *bs);
void bdrv_iostatus_disable(BlockDriverState *bs);
@@ -131,6 +140,14 @@ int bdrv_parse_cache_flags(const char *mode, int *flags);
int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
BlockDriver *drv);
+BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
+ BlockDriverState *bs, int flags);
+int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp);
+int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
+ BlockReopenQueue *queue, Error **errp);
+void bdrv_reopen_commit(BDRVReopenState *reopen_state);
+void bdrv_reopen_abort(BDRVReopenState *reopen_state);
void bdrv_close(BlockDriverState *bs);
int bdrv_attach_dev(BlockDriverState *bs, void *dev);
void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
diff --git a/block_int.h b/block_int.h
index 4452f6f..22b3d93 100644
--- a/block_int.h
+++ b/block_int.h
@@ -139,6 +139,13 @@ struct BlockDriver {
int instance_size;
int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
int (*bdrv_probe_device)(const char *filename);
+
+ /* For handling image reopen for split or non-split files */
+ int (*bdrv_reopen_prepare)(BDRVReopenState *reopen_state,
+ BlockReopenQueue *queue, Error **errp);
+ void (*bdrv_reopen_commit)(BDRVReopenState *reopen_state);
+ void (*bdrv_reopen_abort)(BDRVReopenState *reopen_state);
+
int (*bdrv_open)(BlockDriverState *bs, int flags);
int (*bdrv_file_open)(BlockDriverState *bs, const char *filename, int flags);
int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
@@ -336,6 +343,7 @@ struct BlockDriverState {
/* long-running background operation */
BlockJob *job;
+
};
int get_tmp_filename(char *filename, int size);
diff --git a/qemu-common.h b/qemu-common.h
index e5c2bcd..6a6181c 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -245,6 +245,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;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 03/19] block: Framework for reopening files safely
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 03/19] block: Framework for reopening files safely Jeff Cody
@ 2012-09-20 11:24 ` Kevin Wolf
2012-09-20 12:55 ` Jeff Cody
0 siblings, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2012-09-20 11:24 UTC (permalink / raw)
To: Jeff Cody; +Cc: stefanha, pbonzini, eblake, qemu-devel, supriyak
Am 18.09.2012 20:53, schrieb Jeff Cody:
> This is based on Supriya Kannery's bdrv_reopen() patch series.
>
> This provides a transactional method to reopen multiple
> images files safely.
>
> Image files are queue for reopen via bdrv_reopen_queue(), and the
> reopen occurs when bdrv_reopen_multiple() is called. Changes are
> staged in bdrv_reopen_prepare() and in the equivalent driver level
> functions. If any of the staged images fails a prepare, then all
> of the images left untouched, and the staged changes for each image
> abandoned.
>
> Block drivers are passed a reopen state structure, that contains:
> * BDS to reopen
> * flags for the reopen
> * opaque pointer for any driver-specific data that needs to be
> persistent from _prepare to _commit/_abort
> * reopen queue pointer, if the driver needs to queue additional
> BDS for a reopen
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> diff --git a/block.h b/block.h
> index 4d919c2..b1095d8 100644
> --- a/block.h
> +++ b/block.h
> @@ -97,6 +97,15 @@ typedef enum {
> BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
> } BlockQMPEventAction;
>
> +typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue;
> +
> +typedef struct BDRVReopenState {
> + BlockDriverState *bs;
> + int flags;
> + void *opaque;
> +} BDRVReopenState;
Doesn't compile, it should be typedefed only once:
In file included from monitor.h:8,
from qemu-timer.c:27:
block.h:106: Fehler: Redefinition des typedef »BDRVReopenState«
qemu-common.h:248: Anmerkung: Vorherige Deklaration von
»BDRVReopenState« war hier
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -245,6 +245,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;
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 03/19] block: Framework for reopening files safely
2012-09-20 11:24 ` Kevin Wolf
@ 2012-09-20 12:55 ` Jeff Cody
0 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-20 12:55 UTC (permalink / raw)
To: Kevin Wolf; +Cc: stefanha, pbonzini, eblake, qemu-devel, supriyak
On 09/20/2012 07:24 AM, Kevin Wolf wrote:
> Am 18.09.2012 20:53, schrieb Jeff Cody:
>> This is based on Supriya Kannery's bdrv_reopen() patch series.
>>
>> This provides a transactional method to reopen multiple
>> images files safely.
>>
>> Image files are queue for reopen via bdrv_reopen_queue(), and the
>> reopen occurs when bdrv_reopen_multiple() is called. Changes are
>> staged in bdrv_reopen_prepare() and in the equivalent driver level
>> functions. If any of the staged images fails a prepare, then all
>> of the images left untouched, and the staged changes for each image
>> abandoned.
>>
>> Block drivers are passed a reopen state structure, that contains:
>> * BDS to reopen
>> * flags for the reopen
>> * opaque pointer for any driver-specific data that needs to be
>> persistent from _prepare to _commit/_abort
>> * reopen queue pointer, if the driver needs to queue additional
>> BDS for a reopen
>>
>> Signed-off-by: Jeff Cody <jcody@redhat.com>
>
>> diff --git a/block.h b/block.h
>> index 4d919c2..b1095d8 100644
>> --- a/block.h
>> +++ b/block.h
>> @@ -97,6 +97,15 @@ typedef enum {
>> BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
>> } BlockQMPEventAction;
>>
>> +typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue;
>> +
>> +typedef struct BDRVReopenState {
>> + BlockDriverState *bs;
>> + int flags;
>> + void *opaque;
>> +} BDRVReopenState;
>
> Doesn't compile, it should be typedefed only once:
>
> In file included from monitor.h:8,
> from qemu-timer.c:27:
> block.h:106: Fehler: Redefinition des typedef »BDRVReopenState«
> qemu-common.h:248: Anmerkung: Vorherige Deklaration von
> »BDRVReopenState« war hier
>
Looks like this has been relaxed in later versions of gcc - in 4.7.0 (what I am using under F17), it
compiled fine. I'll change this though, so that it compiles under previous versions of gcc (and check
for any other such errors), and verify that under an older version of gcc.
>> --- a/qemu-common.h
>> +++ b/qemu-common.h
>> @@ -245,6 +245,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;
>
> Kevin
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v3 04/19] block: move aio initialization into a helper function
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (2 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 03/19] block: Framework for reopening files safely Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-20 13:14 ` Kevin Wolf
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 05/19] block: move open flag parsing in raw block drivers to helper functions Jeff Cody
` (14 subsequent siblings)
18 siblings, 1 reply; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
Move AIO initialization for raw-posix block driver into a helper function.
In addition to just code motion, the aio_ctx pointer is checked for NULL,
prior to calling laio_init(), to make sure laio_init() is only run once.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/raw-posix.c | 55 +++++++++++++++++++++++++++++++++++--------------------
1 file changed, 35 insertions(+), 20 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6be20b1..ee55f79 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -185,6 +185,40 @@ static int raw_normalize_devicepath(const char **filename)
}
#endif
+static int raw_set_aio(void **aio_ctx, int *use_aio, int bdrv_flags)
+{
+#ifdef CONFIG_LINUX_AIO
+ int ret = -1;
+ assert(aio_ctx != NULL);
+ assert(use_aio != NULL);
+ /*
+ * Currently Linux do AIO only for files opened with O_DIRECT
+ * specified so check NOCACHE flag too
+ */
+ if ((bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
+ (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) {
+
+ /* if non-NULL, laio_init() has already been run */
+ if (*aio_ctx == NULL) {
+ *aio_ctx = laio_init();
+ if (!*aio_ctx) {
+ goto error;
+ }
+ }
+ *use_aio = 1;
+ } else {
+ *use_aio = 0;
+ }
+
+ ret = 0;
+
+error:
+ return ret;
+#else
+ return 0;
+#endif
+}
+
static int raw_open_common(BlockDriverState *bs, const char *filename,
int bdrv_flags, int open_flags)
{
@@ -239,26 +273,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
goto out_free_buf;
}
-#ifdef CONFIG_LINUX_AIO
- /*
- * Currently Linux do AIO only for files opened with O_DIRECT
- * specified so check NOCACHE flag too
- */
- if ((bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
- (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) {
-
- s->aio_ctx = laio_init();
- if (!s->aio_ctx) {
- goto out_free_buf;
- }
- s->use_aio = 1;
- } else
-#endif
- {
-#ifdef CONFIG_LINUX_AIO
- s->use_aio = 0;
-#endif
- }
+ raw_set_aio(&s->aio_ctx, &s->use_aio, bdrv_flags);
#ifdef CONFIG_XFS
if (platform_test_xfs_fd(s->fd)) {
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/19] block: move aio initialization into a helper function
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 04/19] block: move aio initialization into a helper function Jeff Cody
@ 2012-09-20 13:14 ` Kevin Wolf
2012-09-20 14:49 ` Jeff Cody
0 siblings, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2012-09-20 13:14 UTC (permalink / raw)
To: Jeff Cody; +Cc: supriyak, pbonzini, eblake, qemu-devel, stefanha
Am 18.09.2012 20:53, schrieb Jeff Cody:
> Move AIO initialization for raw-posix block driver into a helper function.
>
> In addition to just code motion, the aio_ctx pointer is checked for NULL,
> prior to calling laio_init(), to make sure laio_init() is only run once.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/raw-posix.c | 55 +++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 35 insertions(+), 20 deletions(-)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 6be20b1..ee55f79 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -185,6 +185,40 @@ static int raw_normalize_devicepath(const char **filename)
> }
> #endif
>
> +static int raw_set_aio(void **aio_ctx, int *use_aio, int bdrv_flags)
> +{
> +#ifdef CONFIG_LINUX_AIO
> + int ret = -1;
> + assert(aio_ctx != NULL);
> + assert(use_aio != NULL);
> + /*
> + * Currently Linux do AIO only for files opened with O_DIRECT
> + * specified so check NOCACHE flag too
> + */
> + if ((bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
> + (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) {
> +
> + /* if non-NULL, laio_init() has already been run */
> + if (*aio_ctx == NULL) {
> + *aio_ctx = laio_init();
> + if (!*aio_ctx) {
> + goto error;
> + }
> + }
> + *use_aio = 1;
> + } else {
> + *use_aio = 0;
> + }
> +
> + ret = 0;
> +
> +error:
> + return ret;
> +#else
> + return 0;
> +#endif
> +}
> +
> static int raw_open_common(BlockDriverState *bs, const char *filename,
> int bdrv_flags, int open_flags)
> {
> @@ -239,26 +273,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
> goto out_free_buf;
> }
>
> -#ifdef CONFIG_LINUX_AIO
> - /*
> - * Currently Linux do AIO only for files opened with O_DIRECT
> - * specified so check NOCACHE flag too
> - */
> - if ((bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
> - (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) {
> -
> - s->aio_ctx = laio_init();
> - if (!s->aio_ctx) {
> - goto out_free_buf;
> - }
> - s->use_aio = 1;
> - } else
> -#endif
> - {
> -#ifdef CONFIG_LINUX_AIO
> - s->use_aio = 0;
> -#endif
> - }
> + raw_set_aio(&s->aio_ctx, &s->use_aio, bdrv_flags);
This should check the return value to handle laio_init() failure properly.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 04/19] block: move aio initialization into a helper function
2012-09-20 13:14 ` Kevin Wolf
@ 2012-09-20 14:49 ` Jeff Cody
0 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-20 14:49 UTC (permalink / raw)
To: Kevin Wolf; +Cc: supriyak, pbonzini, eblake, qemu-devel, stefanha
On 09/20/2012 09:14 AM, Kevin Wolf wrote:
> Am 18.09.2012 20:53, schrieb Jeff Cody:
>> Move AIO initialization for raw-posix block driver into a helper function.
>>
>> In addition to just code motion, the aio_ctx pointer is checked for NULL,
>> prior to calling laio_init(), to make sure laio_init() is only run once.
>>
>> Signed-off-by: Jeff Cody <jcody@redhat.com>
>> ---
>> block/raw-posix.c | 55 +++++++++++++++++++++++++++++++++++--------------------
>> 1 file changed, 35 insertions(+), 20 deletions(-)
>>
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index 6be20b1..ee55f79 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -185,6 +185,40 @@ static int raw_normalize_devicepath(const char **filename)
>> }
>> #endif
>>
>> +static int raw_set_aio(void **aio_ctx, int *use_aio, int bdrv_flags)
>> +{
>> +#ifdef CONFIG_LINUX_AIO
>> + int ret = -1;
>> + assert(aio_ctx != NULL);
>> + assert(use_aio != NULL);
>> + /*
>> + * Currently Linux do AIO only for files opened with O_DIRECT
>> + * specified so check NOCACHE flag too
>> + */
>> + if ((bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
>> + (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) {
>> +
>> + /* if non-NULL, laio_init() has already been run */
>> + if (*aio_ctx == NULL) {
>> + *aio_ctx = laio_init();
>> + if (!*aio_ctx) {
>> + goto error;
>> + }
>> + }
>> + *use_aio = 1;
>> + } else {
>> + *use_aio = 0;
>> + }
>> +
>> + ret = 0;
>> +
>> +error:
>> + return ret;
>> +#else
>> + return 0;
>> +#endif
>> +}
>> +
>> static int raw_open_common(BlockDriverState *bs, const char *filename,
>> int bdrv_flags, int open_flags)
>> {
>> @@ -239,26 +273,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
>> goto out_free_buf;
>> }
>>
>> -#ifdef CONFIG_LINUX_AIO
>> - /*
>> - * Currently Linux do AIO only for files opened with O_DIRECT
>> - * specified so check NOCACHE flag too
>> - */
>> - if ((bdrv_flags & (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) ==
>> - (BDRV_O_NOCACHE|BDRV_O_NATIVE_AIO)) {
>> -
>> - s->aio_ctx = laio_init();
>> - if (!s->aio_ctx) {
>> - goto out_free_buf;
>> - }
>> - s->use_aio = 1;
>> - } else
>> -#endif
>> - {
>> -#ifdef CONFIG_LINUX_AIO
>> - s->use_aio = 0;
>> -#endif
>> - }
>> + raw_set_aio(&s->aio_ctx, &s->use_aio, bdrv_flags);
>
> This should check the return value to handle laio_init() failure properly.
>
> Kevin
>
Thanks, fixed for v4.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v3 05/19] block: move open flag parsing in raw block drivers to helper functions
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (3 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 04/19] block: move aio initialization into a helper function Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 06/19] block: do not parse BDRV_O_CACHE_WB in block drivers Jeff Cody
` (13 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
Code motion, to move parsing of open flags into a helper function.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/raw-posix.c | 38 ++++++++++++++++++++++++--------------
block/raw-win32.c | 43 +++++++++++++++++++++++--------------------
2 files changed, 47 insertions(+), 34 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index ee55f79..7d3ac9d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -185,6 +185,28 @@ static int raw_normalize_devicepath(const char **filename)
}
#endif
+static void raw_parse_flags(int bdrv_flags, int *open_flags)
+{
+ assert(open_flags != NULL);
+
+ *open_flags |= O_BINARY;
+ *open_flags &= ~O_ACCMODE;
+ if (bdrv_flags & BDRV_O_RDWR) {
+ *open_flags |= O_RDWR;
+ } else {
+ *open_flags |= O_RDONLY;
+ }
+
+ /* Use O_DSYNC for write-through caching, no flags for write-back caching,
+ * and O_DIRECT for no caching. */
+ if ((bdrv_flags & BDRV_O_NOCACHE)) {
+ *open_flags |= O_DIRECT;
+ }
+ if (!(bdrv_flags & BDRV_O_CACHE_WB)) {
+ *open_flags |= O_DSYNC;
+ }
+}
+
static int raw_set_aio(void **aio_ctx, int *use_aio, int bdrv_flags)
{
#ifdef CONFIG_LINUX_AIO
@@ -230,20 +252,8 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
return ret;
}
- s->open_flags = open_flags | O_BINARY;
- s->open_flags &= ~O_ACCMODE;
- if (bdrv_flags & BDRV_O_RDWR) {
- s->open_flags |= O_RDWR;
- } else {
- s->open_flags |= O_RDONLY;
- }
-
- /* Use O_DSYNC for write-through caching, no flags for write-back caching,
- * and O_DIRECT for no caching. */
- if ((bdrv_flags & BDRV_O_NOCACHE))
- s->open_flags |= O_DIRECT;
- if (!(bdrv_flags & BDRV_O_CACHE_WB))
- s->open_flags |= O_DSYNC;
+ s->open_flags = open_flags;
+ raw_parse_flags(bdrv_flags, &s->open_flags);
s->fd = -1;
fd = qemu_open(filename, s->open_flags, 0644);
diff --git a/block/raw-win32.c b/block/raw-win32.c
index c56bf83..335c06a 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -77,6 +77,26 @@ static int set_sparse(int fd)
NULL, 0, NULL, 0, &returned, NULL);
}
+static void raw_parse_flags(int flags, int *access_flags, DWORD *overlapped)
+{
+ assert(access_flags != NULL);
+ assert(overlapped != NULL);
+
+ if (flags & BDRV_O_RDWR) {
+ *access_flags = GENERIC_READ | GENERIC_WRITE;
+ } else {
+ *access_flags = GENERIC_READ;
+ }
+
+ *overlapped = FILE_ATTRIBUTE_NORMAL;
+ if (flags & BDRV_O_NOCACHE) {
+ *overlapped |= FILE_FLAG_NO_BUFFERING;
+ }
+ if (!(flags & BDRV_O_CACHE_WB)) {
+ *overlapped |= FILE_FLAG_WRITE_THROUGH;
+ }
+}
+
static int raw_open(BlockDriverState *bs, const char *filename, int flags)
{
BDRVRawState *s = bs->opaque;
@@ -85,17 +105,8 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags)
s->type = FTYPE_FILE;
- if (flags & BDRV_O_RDWR) {
- access_flags = GENERIC_READ | GENERIC_WRITE;
- } else {
- access_flags = GENERIC_READ;
- }
+ raw_parse_flags(flags, &access_flags, &overlapped);
- overlapped = FILE_ATTRIBUTE_NORMAL;
- if (flags & BDRV_O_NOCACHE)
- overlapped |= FILE_FLAG_NO_BUFFERING;
- if (!(flags & BDRV_O_CACHE_WB))
- overlapped |= FILE_FLAG_WRITE_THROUGH;
s->hfile = CreateFile(filename, access_flags,
FILE_SHARE_READ, NULL,
OPEN_EXISTING, overlapped, NULL);
@@ -374,18 +385,10 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
}
s->type = find_device_type(bs, filename);
- if (flags & BDRV_O_RDWR) {
- access_flags = GENERIC_READ | GENERIC_WRITE;
- } else {
- access_flags = GENERIC_READ;
- }
+ raw_parse_flags(flags, &access_flags, &overlapped);
+
create_flags = OPEN_EXISTING;
- overlapped = FILE_ATTRIBUTE_NORMAL;
- if (flags & BDRV_O_NOCACHE)
- overlapped |= FILE_FLAG_NO_BUFFERING;
- if (!(flags & BDRV_O_CACHE_WB))
- overlapped |= FILE_FLAG_WRITE_THROUGH;
s->hfile = CreateFile(filename, access_flags,
FILE_SHARE_READ, NULL,
create_flags, overlapped, NULL);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 06/19] block: do not parse BDRV_O_CACHE_WB in block drivers
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (4 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 05/19] block: move open flag parsing in raw block drivers to helper functions Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 07/19] block: use BDRV_O_NOCACHE instead of s->aligned_buf in raw-posix.c Jeff Cody
` (12 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
Block drivers should ignore BDRV_O_CACHE_WB in .bdrv_open flags,
and in the bs->open_flags.
This patch removes the code, leaving the behaviour behind as if
BDRV_O_CACHE_WB was set.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/iscsi.c | 4 ----
block/raw-posix.c | 3 ---
block/raw-win32.c | 3 ---
block/rbd.c | 6 ------
block/sheepdog.c | 14 ++++++--------
5 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 0b96165..70cf700 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -268,10 +268,6 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
acb->task->xfer_dir = SCSI_XFER_WRITE;
acb->task->cdb_size = 16;
acb->task->cdb[0] = 0x8a;
- if (!(bs->open_flags & BDRV_O_CACHE_WB)) {
- /* set FUA on writes when cache mode is write through */
- acb->task->cdb[1] |= 0x04;
- }
lba = sector_qemu2lun(sector_num, iscsilun);
*(uint32_t *)&acb->task->cdb[2] = htonl(lba >> 32);
*(uint32_t *)&acb->task->cdb[6] = htonl(lba & 0xffffffff);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 7d3ac9d..4a1047c 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -202,9 +202,6 @@ static void raw_parse_flags(int bdrv_flags, int *open_flags)
if ((bdrv_flags & BDRV_O_NOCACHE)) {
*open_flags |= O_DIRECT;
}
- if (!(bdrv_flags & BDRV_O_CACHE_WB)) {
- *open_flags |= O_DSYNC;
- }
}
static int raw_set_aio(void **aio_ctx, int *use_aio, int bdrv_flags)
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 335c06a..78c8306 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -92,9 +92,6 @@ static void raw_parse_flags(int flags, int *access_flags, DWORD *overlapped)
if (flags & BDRV_O_NOCACHE) {
*overlapped |= FILE_FLAG_NO_BUFFERING;
}
- if (!(flags & BDRV_O_CACHE_WB)) {
- *overlapped |= FILE_FLAG_WRITE_THROUGH;
- }
}
static int raw_open(BlockDriverState *bs, const char *filename, int flags)
diff --git a/block/rbd.c b/block/rbd.c
index 5a0f79f..015a9db 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -487,12 +487,6 @@ static int qemu_rbd_open(BlockDriverState *bs, const char *filename, int flags)
rados_conf_set(s->cluster, "rbd_cache", "false");
} else {
rados_conf_set(s->cluster, "rbd_cache", "true");
- if (!(flags & BDRV_O_CACHE_WB)) {
- r = rados_conf_set(s->cluster, "rbd_cache_max_dirty", "0");
- if (r < 0) {
- rados_conf_set(s->cluster, "rbd_cache", "false");
- }
- }
}
if (strstr(conf, "conf=") == NULL) {
diff --git a/block/sheepdog.c b/block/sheepdog.c
index df4f441..5b553db 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1114,14 +1114,12 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags)
goto out;
}
- if (flags & BDRV_O_CACHE_WB) {
- s->cache_enabled = 1;
- s->flush_fd = connect_to_sdog(s->addr, s->port);
- if (s->flush_fd < 0) {
- error_report("failed to connect");
- ret = s->flush_fd;
- goto out;
- }
+ s->cache_enabled = 1;
+ s->flush_fd = connect_to_sdog(s->addr, s->port);
+ if (s->flush_fd < 0) {
+ error_report("failed to connect");
+ ret = s->flush_fd;
+ goto out;
}
if (snapid || tag[0] != '\0') {
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 07/19] block: use BDRV_O_NOCACHE instead of s->aligned_buf in raw-posix.c
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (5 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 06/19] block: do not parse BDRV_O_CACHE_WB in block drivers Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 08/19] block: purge s->aligned_buf and s->aligned_buf_size from raw-posix.c Jeff Cody
` (11 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
Rather than check for a non-NULL aligned_buf to determine if
raw_aio_submit needs to check for alignment, check for the presence
of BDRV_O_NOCACHE in the bs->open_flags.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/raw-posix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 4a1047c..1ea09c0 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -352,7 +352,7 @@ static BlockDriverAIOCB *raw_aio_submit(BlockDriverState *bs,
* boundary. Check if this is the case or tell the low-level
* driver that it needs to copy the buffer.
*/
- if (s->aligned_buf) {
+ if ((bs->open_flags & BDRV_O_NOCACHE)) {
if (!qiov_is_aligned(bs, qiov)) {
type |= QEMU_AIO_MISALIGNED;
#ifdef CONFIG_LINUX_AIO
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 08/19] block: purge s->aligned_buf and s->aligned_buf_size from raw-posix.c
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (6 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 07/19] block: use BDRV_O_NOCACHE instead of s->aligned_buf in raw-posix.c Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 09/19] block: raw-posix image file reopen Jeff Cody
` (10 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
The aligned_buf pointer and aligned_buf size are no longer used in
raw_posix.c, so remove all references to them.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/raw-posix.c | 21 +--------------------
1 file changed, 1 insertion(+), 20 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 1ea09c0..6bf5480 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -133,8 +133,6 @@ typedef struct BDRVRawState {
int use_aio;
void *aio_ctx;
#endif
- uint8_t *aligned_buf;
- unsigned aligned_buf_size;
#ifdef CONFIG_XFS
bool is_xfs : 1;
#endif
@@ -261,23 +259,10 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
return ret;
}
s->fd = fd;
- s->aligned_buf = NULL;
-
- if ((bdrv_flags & BDRV_O_NOCACHE)) {
- /*
- * Allocate a buffer for read/modify/write cycles. Chose the size
- * pessimistically as we don't know the block size yet.
- */
- s->aligned_buf_size = 32 * MAX_BLOCKSIZE;
- s->aligned_buf = qemu_memalign(MAX_BLOCKSIZE, s->aligned_buf_size);
- if (s->aligned_buf == NULL) {
- goto out_close;
- }
- }
/* We're falling back to POSIX AIO in some cases so init always */
if (paio_init() < 0) {
- goto out_free_buf;
+ goto out_close;
}
raw_set_aio(&s->aio_ctx, &s->use_aio, bdrv_flags);
@@ -290,8 +275,6 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
return 0;
-out_free_buf:
- qemu_vfree(s->aligned_buf);
out_close:
qemu_close(fd);
return -errno;
@@ -400,8 +383,6 @@ static void raw_close(BlockDriverState *bs)
if (s->fd >= 0) {
qemu_close(s->fd);
s->fd = -1;
- if (s->aligned_buf != NULL)
- qemu_vfree(s->aligned_buf);
}
}
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 09/19] block: raw-posix image file reopen
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (7 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 08/19] block: purge s->aligned_buf and s->aligned_buf_size from raw-posix.c Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 21:20 ` Eric Blake
2012-09-20 14:10 ` Kevin Wolf
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 10/19] block: raw " Jeff Cody
` (9 subsequent siblings)
18 siblings, 2 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
This is derived from the Supriya Kannery's reopen patches.
This contains the raw-posix driver changes for the bdrv_reopen_*
functions. All changes are staged into a temporary scratch buffer
during the prepare() stage, and copied over to the live structure
during commit(). Upon abort(), all changes are abandoned, and the
live structures are unmodified.
The _prepare() will create an extra fd - either by means of a dup,
if possible, or opening a new fd if not (for instance, access
control changes). Upon _commit(), the original fd is closed and
the new fd is used. Upon _abort(), the duplicate/new fd is closed.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/raw-posix.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6bf5480..edd1eca 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -138,6 +138,15 @@ typedef struct BDRVRawState {
#endif
} BDRVRawState;
+typedef struct BDRVRawReopenState {
+ int fd;
+ int open_flags;
+#ifdef CONFIG_LINUX_AIO
+ int use_aio;
+ void *aio_ctx;
+#endif
+} BDRVRawReopenState;
+
static int fd_open(BlockDriverState *bs);
static int64_t raw_getlength(BlockDriverState *bs);
@@ -288,6 +297,93 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags)
return raw_open_common(bs, filename, flags, 0);
}
+static int raw_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+ BDRVRawState *s;
+ BDRVRawReopenState *raw_s;
+ int ret = 0;
+
+ assert(state != NULL);
+ assert(state->bs != NULL);
+
+ s = state->bs->opaque;
+
+ state->opaque = g_malloc0(sizeof(BDRVRawReopenState));
+ raw_s = state->opaque;
+ raw_s->use_aio = s->use_aio;
+ raw_s->aio_ctx = s->aio_ctx;
+
+ raw_parse_flags(state->flags, &raw_s->open_flags);
+ raw_set_aio(&raw_s->aio_ctx, &raw_s->use_aio, state->flags);
+
+ raw_s->fd = -1;
+
+ int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
+#ifdef O_NOATIME
+ fcntl_flags |= O_NOATIME;
+#endif
+ if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
+ /* dup the original fd */
+ /* TODO: use qemu fcntl wrapper */
+ raw_s->fd = fcntl(s->fd, F_DUPFD_CLOEXEC, 0);
+ if (raw_s->fd >= 0) {
+ ret = fcntl_setfl(raw_s->fd, raw_s->open_flags);
+ if (ret) {
+ qemu_close(raw_s->fd);
+ raw_s->fd = -1;
+ }
+ }
+ }
+
+ /* If we cannot use fctnl, or fcntl failed, fall back to qemu_open() */
+ if (raw_s->fd == -1) {
+ assert(!(raw_s->open_flags & O_CREAT));
+ raw_s->fd = qemu_open(state->bs->filename, raw_s->open_flags);
+ if (raw_s->fd == -1) {
+ ret = -1;
+ }
+ }
+
+ return ret;
+}
+
+
+static void raw_reopen_commit(BDRVReopenState *state)
+{
+ BDRVRawReopenState *raw_s = state->opaque;
+ BDRVRawState *s = state->bs->opaque;
+
+ s->open_flags = raw_s->open_flags;
+
+ qemu_close(s->fd);
+ s->fd = raw_s->fd;
+ s->use_aio = raw_s->use_aio;
+ s->aio_ctx = raw_s->aio_ctx;
+
+ g_free(state->opaque);
+ state->opaque = NULL;
+}
+
+
+static void raw_reopen_abort(BDRVReopenState *state)
+{
+ BDRVRawReopenState *raw_s = state->opaque;
+
+ /* nothing to do if NULL, we didn't get far enough */
+ if (raw_s == NULL) {
+ return;
+ }
+
+ if (raw_s->fd >= 0) {
+ qemu_close(raw_s->fd);
+ raw_s->fd = -1;
+ }
+ g_free(state->opaque);
+ state->opaque = NULL;
+}
+
+
/* XXX: use host sector size if necessary with:
#ifdef DIOCGSECTORSIZE
{
@@ -738,6 +834,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,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 09/19] block: raw-posix image file reopen
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 09/19] block: raw-posix image file reopen Jeff Cody
@ 2012-09-18 21:20 ` Eric Blake
2012-09-18 22:20 ` Jeff Cody
2012-09-20 14:10 ` Kevin Wolf
1 sibling, 1 reply; 30+ messages in thread
From: Eric Blake @ 2012-09-18 21:20 UTC (permalink / raw)
To: Jeff Cody; +Cc: kwolf, pbonzini, stefanha, qemu-devel, supriyak
[-- Attachment #1: Type: text/plain, Size: 2857 bytes --]
On 09/18/2012 12:53 PM, Jeff Cody wrote:
> This is derived from the Supriya Kannery's reopen patches.
>
> This contains the raw-posix driver changes for the bdrv_reopen_*
> functions. All changes are staged into a temporary scratch buffer
> during the prepare() stage, and copied over to the live structure
> during commit(). Upon abort(), all changes are abandoned, and the
> live structures are unmodified.
>
> The _prepare() will create an extra fd - either by means of a dup,
> if possible, or opening a new fd if not (for instance, access
> control changes). Upon _commit(), the original fd is closed and
> the new fd is used. Upon _abort(), the duplicate/new fd is closed.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/raw-posix.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 99 insertions(+)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 6bf5480..edd1eca 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -138,6 +138,15 @@ typedef struct BDRVRawState {
> #endif
> } BDRVRawState;
>
> +typedef struct BDRVRawReopenState {
> + int fd;
> + int open_flags;
> +#ifdef CONFIG_LINUX_AIO
> + int use_aio;
> + void *aio_ctx;
> +#endif
These members are conditional...
> +static int raw_reopen_prepare(BDRVReopenState *state,
> + BlockReopenQueue *queue, Error **errp)
> +{
> + BDRVRawState *s;
> + BDRVRawReopenState *raw_s;
> + int ret = 0;
> +
> + assert(state != NULL);
> + assert(state->bs != NULL);
> +
> + s = state->bs->opaque;
> +
> + state->opaque = g_malloc0(sizeof(BDRVRawReopenState));
> + raw_s = state->opaque;
> + raw_s->use_aio = s->use_aio;
> + raw_s->aio_ctx = s->aio_ctx;
...but you are unconditionally assigning into them. This will introduce
compile failures on other platforms.
> + if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
> + /* dup the original fd */
> + /* TODO: use qemu fcntl wrapper */
> + raw_s->fd = fcntl(s->fd, F_DUPFD_CLOEXEC, 0);
F_DUPFD_CLOEXEC is not defined everywhere yet; you still need to address
your TODO.
> +
> + /* If we cannot use fctnl, or fcntl failed, fall back to qemu_open() */
s/fctnl/fcntl/
> +static void raw_reopen_commit(BDRVReopenState *state)
> +{
> + BDRVRawReopenState *raw_s = state->opaque;
> + BDRVRawState *s = state->bs->opaque;
> +
> + s->open_flags = raw_s->open_flags;
> +
> + qemu_close(s->fd);
> + s->fd = raw_s->fd;
> + s->use_aio = raw_s->use_aio;
> + s->aio_ctx = raw_s->aio_ctx;
Again, more unconditional use of conditional members.
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 09/19] block: raw-posix image file reopen
2012-09-18 21:20 ` Eric Blake
@ 2012-09-18 22:20 ` Jeff Cody
0 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 22:20 UTC (permalink / raw)
To: Eric Blake; +Cc: kwolf, pbonzini, stefanha, qemu-devel, supriyak
On 09/18/2012 05:20 PM, Eric Blake wrote:
> On 09/18/2012 12:53 PM, Jeff Cody wrote:
>> This is derived from the Supriya Kannery's reopen patches.
>>
>> This contains the raw-posix driver changes for the bdrv_reopen_*
>> functions. All changes are staged into a temporary scratch buffer
>> during the prepare() stage, and copied over to the live structure
>> during commit(). Upon abort(), all changes are abandoned, and the
>> live structures are unmodified.
>>
>> The _prepare() will create an extra fd - either by means of a dup,
>> if possible, or opening a new fd if not (for instance, access
>> control changes). Upon _commit(), the original fd is closed and
>> the new fd is used. Upon _abort(), the duplicate/new fd is closed.
>>
>> Signed-off-by: Jeff Cody <jcody@redhat.com>
>> ---
>> block/raw-posix.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 99 insertions(+)
>>
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index 6bf5480..edd1eca 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -138,6 +138,15 @@ typedef struct BDRVRawState {
>> #endif
>> } BDRVRawState;
>>
>> +typedef struct BDRVRawReopenState {
>> + int fd;
>> + int open_flags;
>> +#ifdef CONFIG_LINUX_AIO
>> + int use_aio;
>> + void *aio_ctx;
>> +#endif
>
> These members are conditional...
>
>> +static int raw_reopen_prepare(BDRVReopenState *state,
>> + BlockReopenQueue *queue, Error **errp)
>> +{
>> + BDRVRawState *s;
>> + BDRVRawReopenState *raw_s;
>> + int ret = 0;
>> +
>> + assert(state != NULL);
>> + assert(state->bs != NULL);
>> +
>> + s = state->bs->opaque;
>> +
>> + state->opaque = g_malloc0(sizeof(BDRVRawReopenState));
>> + raw_s = state->opaque;
>> + raw_s->use_aio = s->use_aio;
>> + raw_s->aio_ctx = s->aio_ctx;
>
> ...but you are unconditionally assigning into them. This will introduce
> compile failures on other platforms.
Argh. Thanks.
>
>> + if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
>> + /* dup the original fd */
>> + /* TODO: use qemu fcntl wrapper */
>> + raw_s->fd = fcntl(s->fd, F_DUPFD_CLOEXEC, 0);
>
> F_DUPFD_CLOEXEC is not defined everywhere yet; you still need to address
> your TODO.
>
>> +
>> + /* If we cannot use fctnl, or fcntl failed, fall back to qemu_open() */
>
> s/fctnl/fcntl/
>
>> +static void raw_reopen_commit(BDRVReopenState *state)
>> +{
>> + BDRVRawReopenState *raw_s = state->opaque;
>> + BDRVRawState *s = state->bs->opaque;
>> +
>> + s->open_flags = raw_s->open_flags;
>> +
>> + qemu_close(s->fd);
>> + s->fd = raw_s->fd;
>> + s->use_aio = raw_s->use_aio;
>> + s->aio_ctx = raw_s->aio_ctx;
>
> Again, more unconditional use of conditional members.
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/19] block: raw-posix image file reopen
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 09/19] block: raw-posix image file reopen Jeff Cody
2012-09-18 21:20 ` Eric Blake
@ 2012-09-20 14:10 ` Kevin Wolf
2012-09-20 14:45 ` Jeff Cody
1 sibling, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2012-09-20 14:10 UTC (permalink / raw)
To: Jeff Cody; +Cc: stefanha, pbonzini, eblake, qemu-devel, supriyak
Am 18.09.2012 20:53, schrieb Jeff Cody:
> This is derived from the Supriya Kannery's reopen patches.
>
> This contains the raw-posix driver changes for the bdrv_reopen_*
> functions. All changes are staged into a temporary scratch buffer
> during the prepare() stage, and copied over to the live structure
> during commit(). Upon abort(), all changes are abandoned, and the
> live structures are unmodified.
>
> The _prepare() will create an extra fd - either by means of a dup,
> if possible, or opening a new fd if not (for instance, access
> control changes). Upon _commit(), the original fd is closed and
> the new fd is used. Upon _abort(), the duplicate/new fd is closed.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> +static int raw_reopen_prepare(BDRVReopenState *state,
> + BlockReopenQueue *queue, Error **errp)
> +{
> + BDRVRawState *s;
> + BDRVRawReopenState *raw_s;
> + int ret = 0;
> +
> + assert(state != NULL);
> + assert(state->bs != NULL);
> +
> + s = state->bs->opaque;
> +
> + state->opaque = g_malloc0(sizeof(BDRVRawReopenState));
> + raw_s = state->opaque;
> + raw_s->use_aio = s->use_aio;
> + raw_s->aio_ctx = s->aio_ctx;
You can immediately set s->aio_ctx instead of going through
BDRVRawReopenState with it. It seems to be valid to have it present
while use_aio = 0. If it wasn't valid, you'd have to free the context
when reopening without Linux AIO.
> +
> + raw_parse_flags(state->flags, &raw_s->open_flags);
> + raw_set_aio(&raw_s->aio_ctx, &raw_s->use_aio, state->flags);
At least you're consistently omitting the error check. :-)
> +
> + raw_s->fd = -1;
> +
> + int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
> +#ifdef O_NOATIME
> + fcntl_flags |= O_NOATIME;
> +#endif
> + if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
> + /* dup the original fd */
> + /* TODO: use qemu fcntl wrapper */
Hm, still not addressed?
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 09/19] block: raw-posix image file reopen
2012-09-20 14:10 ` Kevin Wolf
@ 2012-09-20 14:45 ` Jeff Cody
0 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-20 14:45 UTC (permalink / raw)
To: Kevin Wolf; +Cc: stefanha, pbonzini, eblake, qemu-devel, supriyak
On 09/20/2012 10:10 AM, Kevin Wolf wrote:
> Am 18.09.2012 20:53, schrieb Jeff Cody:
>> This is derived from the Supriya Kannery's reopen patches.
>>
>> This contains the raw-posix driver changes for the bdrv_reopen_*
>> functions. All changes are staged into a temporary scratch buffer
>> during the prepare() stage, and copied over to the live structure
>> during commit(). Upon abort(), all changes are abandoned, and the
>> live structures are unmodified.
>>
>> The _prepare() will create an extra fd - either by means of a dup,
>> if possible, or opening a new fd if not (for instance, access
>> control changes). Upon _commit(), the original fd is closed and
>> the new fd is used. Upon _abort(), the duplicate/new fd is closed.
>>
>> Signed-off-by: Jeff Cody <jcody@redhat.com>
>
>> +static int raw_reopen_prepare(BDRVReopenState *state,
>> + BlockReopenQueue *queue, Error **errp)
>> +{
>> + BDRVRawState *s;
>> + BDRVRawReopenState *raw_s;
>> + int ret = 0;
>> +
>> + assert(state != NULL);
>> + assert(state->bs != NULL);
>> +
>> + s = state->bs->opaque;
>> +
>> + state->opaque = g_malloc0(sizeof(BDRVRawReopenState));
>> + raw_s = state->opaque;
>> + raw_s->use_aio = s->use_aio;
>> + raw_s->aio_ctx = s->aio_ctx;
>
> You can immediately set s->aio_ctx instead of going through
> BDRVRawReopenState with it. It seems to be valid to have it present
> while use_aio = 0. If it wasn't valid, you'd have to free the context
> when reopening without Linux AIO.
>
Good catch, thanks.
>> +
>> + raw_parse_flags(state->flags, &raw_s->open_flags);
>> + raw_set_aio(&raw_s->aio_ctx, &raw_s->use_aio, state->flags);
>
> At least you're consistently omitting the error check. :-)
>
Thanks, fixed.
I guess I am an optimist at heart :)
>> +
>> + raw_s->fd = -1;
>> +
>> + int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
>> +#ifdef O_NOATIME
>> + fcntl_flags |= O_NOATIME;
>> +#endif
>> + if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
>> + /* dup the original fd */
>> + /* TODO: use qemu fcntl wrapper */
>
> Hm, still not addressed?
>
No. I mentioned this in the cover letter for v2. I'd rather see the
qemu fcntl wrapper happen as a separate series, and then come back and
update this, if that is OK. I'm afraid changes to qemu_open or
qemu_dup_flags would delay getting this series in.
Although Eric is right, I do need to ifdef the F_DUPFD_CLOEXEC.
> Kevin
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v3 10/19] block: raw image file reopen
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (8 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 09/19] block: raw-posix image file reopen Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 11/19] block: qed " Jeff Cody
` (8 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
These are the stubs for the file reopen drivers for the raw format.
There is currently nothing that needs to be done by the raw driver
in reopen.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/raw.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/block/raw.c b/block/raw.c
index ff34ea4..253e949 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -9,6 +9,14 @@ static int raw_open(BlockDriverState *bs, int flags)
return 0;
}
+/* We have nothing to do for raw reopen, stubs just return
+ * success */
+static int raw_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+ return 0;
+}
+
static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
@@ -115,6 +123,8 @@ static BlockDriver bdrv_raw = {
.bdrv_open = raw_open,
.bdrv_close = raw_close,
+ .bdrv_reopen_prepare = raw_reopen_prepare,
+
.bdrv_co_readv = raw_co_readv,
.bdrv_co_writev = raw_co_writev,
.bdrv_co_is_allocated = raw_co_is_allocated,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 11/19] block: qed image file reopen
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (9 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 10/19] block: raw " Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 12/19] block: qcow2 " Jeff Cody
` (7 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
These are the stubs for the file reopen drivers for the qed format.
There is currently nothing that needs to be done by the qed driver
in reopen.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/qed.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/block/qed.c b/block/qed.c
index 21cb239..6c182ca 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -505,6 +505,14 @@ out:
return ret;
}
+/* We have nothing to do for QED reopen, stubs just return
+ * success */
+static int bdrv_qed_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+ return 0;
+}
+
static void bdrv_qed_close(BlockDriverState *bs)
{
BDRVQEDState *s = bs->opaque;
@@ -1564,6 +1572,7 @@ 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_create = bdrv_qed_create,
.bdrv_co_is_allocated = bdrv_qed_co_is_allocated,
.bdrv_make_empty = bdrv_qed_make_empty,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 12/19] block: qcow2 image file reopen
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (10 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 11/19] block: qed " Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 13/19] block: qcow " Jeff Cody
` (6 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
These are the stubs for the file reopen drivers for the qcow2 format.
There is currently nothing that needs to be done by the qcow2 driver
in reopen.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/qcow2.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index 8f183f1..aa5e603 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -52,6 +52,7 @@ typedef struct {
uint32_t magic;
uint32_t len;
} QCowExtension;
+
#define QCOW2_EXT_MAGIC_END 0
#define QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
#define QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
@@ -558,6 +559,14 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key)
return 0;
}
+/* We have nothing to do for QCOW2 reopen, stubs just return
+ * success */
+static int qcow2_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+ return 0;
+}
+
static int coroutine_fn qcow2_co_is_allocated(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int *pnum)
{
@@ -1679,6 +1688,7 @@ static BlockDriver bdrv_qcow2 = {
.bdrv_probe = qcow2_probe,
.bdrv_open = qcow2_open,
.bdrv_close = qcow2_close,
+ .bdrv_reopen_prepare = qcow2_reopen_prepare,
.bdrv_create = qcow2_create,
.bdrv_co_is_allocated = qcow2_co_is_allocated,
.bdrv_set_key = qcow2_set_key,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 13/19] block: qcow image file reopen
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (11 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 12/19] block: qcow2 " Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 14/19] block: vmdk " Jeff Cody
` (5 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
These are the stubs for the file reopen drivers for the qcow format.
There is currently nothing that needs to be done by the qcow driver
in reopen.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/qcow.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/block/qcow.c b/block/qcow.c
index 7b5ab87..b239c82 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -197,6 +197,15 @@ static int qcow_open(BlockDriverState *bs, int flags)
return ret;
}
+
+/* We have nothing to do for QCOW reopen, stubs just return
+ * success */
+static int qcow_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+ return 0;
+}
+
static int qcow_set_key(BlockDriverState *bs, const char *key)
{
BDRVQcowState *s = bs->opaque;
@@ -868,6 +877,7 @@ static BlockDriver bdrv_qcow = {
.bdrv_probe = qcow_probe,
.bdrv_open = qcow_open,
.bdrv_close = qcow_close,
+ .bdrv_reopen_prepare = qcow_reopen_prepare,
.bdrv_create = qcow_create,
.bdrv_co_readv = qcow_co_readv,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 14/19] block: vmdk image file reopen
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (12 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 13/19] block: qcow " Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-20 14:12 ` Kevin Wolf
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 15/19] block: raw-win32 driver reopen support Jeff Cody
` (4 subsequent siblings)
18 siblings, 1 reply; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
This patch supports reopen for VMDK image files. VMDK extents are added
to the existing reopen queue, so that the transactional model of reopen
is maintained with multiple image files.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/vmdk.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/block/vmdk.c b/block/vmdk.c
index bba4c61..f2e861b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -300,6 +300,40 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
return 1;
}
+/* Queue extents, if any, for reopen() */
+static int vmdk_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+ BDRVVmdkState *s;
+ int ret = -1;
+ int i;
+ VmdkExtent *e;
+
+ assert(state != NULL);
+ assert(state->bs != NULL);
+
+ if (queue == NULL) {
+ error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+ "No reopen queue for VMDK extents");
+ goto exit;
+ }
+
+ s = state->bs->opaque;
+
+ assert(s != NULL);
+
+ for (i = 0; i < s->num_extents; i++) {
+ e = &s->extents[i];
+ if (e->file != state->bs->file) {
+ bdrv_reopen_queue(queue, e->file, state->flags);
+ }
+ }
+ ret = 0;
+
+exit:
+ return ret;
+}
+
static int vmdk_parent_open(BlockDriverState *bs)
{
char *p_name;
@@ -1646,6 +1680,7 @@ static BlockDriver bdrv_vmdk = {
.instance_size = sizeof(BDRVVmdkState),
.bdrv_probe = vmdk_probe,
.bdrv_open = vmdk_open,
+ .bdrv_reopen_prepare = vmdk_reopen_prepare,
.bdrv_read = vmdk_co_read,
.bdrv_write = vmdk_co_write,
.bdrv_close = vmdk_close,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 14/19] block: vmdk image file reopen
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 14/19] block: vmdk " Jeff Cody
@ 2012-09-20 14:12 ` Kevin Wolf
2012-09-20 14:49 ` Jeff Cody
0 siblings, 1 reply; 30+ messages in thread
From: Kevin Wolf @ 2012-09-20 14:12 UTC (permalink / raw)
To: Jeff Cody; +Cc: stefanha, pbonzini, eblake, qemu-devel, supriyak
Am 18.09.2012 20:53, schrieb Jeff Cody:
> This patch supports reopen for VMDK image files. VMDK extents are added
> to the existing reopen queue, so that the transactional model of reopen
> is maintained with multiple image files.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
> block/vmdk.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index bba4c61..f2e861b 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -300,6 +300,40 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
> return 1;
> }
>
> +/* Queue extents, if any, for reopen() */
> +static int vmdk_reopen_prepare(BDRVReopenState *state,
> + BlockReopenQueue *queue, Error **errp)
> +{
> + BDRVVmdkState *s;
> + int ret = -1;
> + int i;
> + VmdkExtent *e;
> +
> + assert(state != NULL);
> + assert(state->bs != NULL);
> +
> + if (queue == NULL) {
> + error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> + "No reopen queue for VMDK extents");
> + goto exit;
> + }
> +
> + s = state->bs->opaque;
> +
> + assert(s != NULL);
> +
> + for (i = 0; i < s->num_extents; i++) {
> + e = &s->extents[i];
> + if (e->file != state->bs->file) {
> + bdrv_reopen_queue(queue, e->file, state->flags);
This is wrong, you can't abort the transaction if you do it like this.
VMDK needs to separately pass through each of prepare/commit/abort to
all extents, it can't use the all-in-one function.
Kevin
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [Qemu-devel] [PATCH v3 14/19] block: vmdk image file reopen
2012-09-20 14:12 ` Kevin Wolf
@ 2012-09-20 14:49 ` Jeff Cody
0 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-20 14:49 UTC (permalink / raw)
To: Kevin Wolf; +Cc: stefanha, pbonzini, eblake, qemu-devel, supriyak
On 09/20/2012 10:12 AM, Kevin Wolf wrote:
> Am 18.09.2012 20:53, schrieb Jeff Cody:
>> This patch supports reopen for VMDK image files. VMDK extents are added
>> to the existing reopen queue, so that the transactional model of reopen
>> is maintained with multiple image files.
>>
>> Signed-off-by: Jeff Cody <jcody@redhat.com>
>> ---
>> block/vmdk.c | 35 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index bba4c61..f2e861b 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -300,6 +300,40 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
>> return 1;
>> }
>>
>> +/* Queue extents, if any, for reopen() */
>> +static int vmdk_reopen_prepare(BDRVReopenState *state,
>> + BlockReopenQueue *queue, Error **errp)
>> +{
>> + BDRVVmdkState *s;
>> + int ret = -1;
>> + int i;
>> + VmdkExtent *e;
>> +
>> + assert(state != NULL);
>> + assert(state->bs != NULL);
>> +
>> + if (queue == NULL) {
>> + error_set(errp, ERROR_CLASS_GENERIC_ERROR,
>> + "No reopen queue for VMDK extents");
>> + goto exit;
>> + }
>> +
>> + s = state->bs->opaque;
>> +
>> + assert(s != NULL);
>> +
>> + for (i = 0; i < s->num_extents; i++) {
>> + e = &s->extents[i];
>> + if (e->file != state->bs->file) {
>> + bdrv_reopen_queue(queue, e->file, state->flags);
>
> This is wrong, you can't abort the transaction if you do it like this.
>
> VMDK needs to separately pass through each of prepare/commit/abort to
> all extents, it can't use the all-in-one function.
>
> Kevin
>
(Follow up from conversation on IRC)
This is actually correct, because it is adding the extents to the queue
in the prepare(). This will cause the extents to go through the same
transactional prepare/commit/abort cycle as the rest of the images in
the queue.
Jeff
^ permalink raw reply [flat|nested] 30+ messages in thread
* [Qemu-devel] [PATCH v3 15/19] block: raw-win32 driver reopen support
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (13 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 14/19] block: vmdk " Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 16/19] block: vdi image file reopen Jeff Cody
` (3 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
This is the support for reopen, for win32. Note that there is a key
difference in the win32 reopen, namely that it is not guaranteed safe
like all the other drivers. Attempts are made to reopen the file, or
open the file in a new handle prior to closing the old handle. However,
this is not always feasible, and there are times when we must close the
existing file and then open the new file, breaking the transactional nature
of the reopen.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/raw-win32.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 78c8306..8a698d3 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -31,6 +31,7 @@
#define FTYPE_FILE 0
#define FTYPE_CD 1
#define FTYPE_HARDDISK 2
+#define WINDOWS_VISTA 6
typedef struct BDRVRawState {
HANDLE hfile;
@@ -38,6 +39,10 @@ typedef struct BDRVRawState {
char drive_path[16]; /* format: "d:\" */
} BDRVRawState;
+typedef struct BDRVRawReopenState {
+ HANDLE hfile;
+} BDRVRawReopenState;
+
int qemu_ftruncate64(int fd, int64_t length)
{
LARGE_INTEGER li;
@@ -117,6 +122,103 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags)
return 0;
}
+static int raw_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+ BDRVRawState *s;
+ BDRVRawReopenState *raw_s;
+ int ret = 0;
+ int access_flags;
+ DWORD overlapped;
+ OSVERSIONINFO osvi;
+
+ assert(state != NULL);
+ assert(state->bs != NULL);
+
+ s = state->bs->opaque;
+
+ state->opaque = g_malloc0(sizeof(BDRVRawReopenState));
+ raw_s = state->opaque;
+
+ raw_parse_flags(state->flags, &access_flags, &overlapped);
+
+ ZeroMemory(&osvi, sizeof(OSVERSIONINFO));
+ osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);
+
+ GetVersionEx(&osvi);
+ raw_s->hfile = INVALID_HANDLE_VALUE;
+
+ if (osvi.dwMajorVersion >= WINDOWS_VISTA) {
+ raw_s->hfile = ReOpenFile(s->hfile, access_flags, FILE_SHARE_READ,
+ overlapped);
+ }
+
+ /* could not reopen the file handle, so fall back to opening
+ * new file (CreateFile) */
+ if (raw_s->hfile == INVALID_HANDLE_VALUE) {
+ raw_s->hfile = CreateFile(state->bs->filename, access_flags,
+ FILE_SHARE_READ, NULL, OPEN_EXISTING,
+ overlapped, NULL);
+ if (raw_s->hfile == INVALID_HANDLE_VALUE) {
+ /* this could happen because the access_flags requested are
+ * incompatible with the existing share mode of s->hfile,
+ * so our only option now is to close s->hfile, and try again.
+ * This could end badly */
+ CloseHandle(s->hfile);
+ s->hfile = INVALID_HANDLE_VALUE;
+ raw_s->hfile = CreateFile(state->bs->filename, access_flags,
+ FILE_SHARE_READ, NULL, OPEN_EXISTING,
+ overlapped, NULL);
+ if (raw_s->hfile == INVALID_HANDLE_VALUE) {
+ /* Unrecoverable error */
+ error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+ "Fatal error reopening %s file; file closed and cannot be opened\n",
+ state->bs->filename);
+ ret = -1;
+ } else{
+ /* since we had to close the original, go ahead and
+ * re-assign here */
+ s->hfile = raw_s->hfile;
+ raw_s->hfile = INVALID_HANDLE_VALUE;
+ }
+ }
+ }
+
+ return ret;
+}
+
+
+static void raw_reopen_commit(BDRVReopenState *state)
+{
+ BDRVRawReopenState *raw_s = state->opaque;
+ BDRVRawState *s = state->bs->opaque;
+
+ if (raw_s->hfile != INVALID_HANDLE_VALUE) {
+ CloseHandle(s->hfile);
+ s->hfile = raw_s->hfile;
+ }
+
+ g_free(state->opaque);
+ state->opaque = NULL;
+}
+
+
+static void raw_reopen_abort(BDRVReopenState *state)
+{
+ BDRVRawReopenState *raw_s = state->opaque;
+
+ /* nothing to do if NULL, we didn't get far enough */
+ if (raw_s == NULL) {
+ return;
+ }
+
+ if (raw_s->hfile != INVALID_HANDLE_VALUE) {
+ CloseHandle(raw_s->hfile);
+ }
+ g_free(state->opaque);
+ state->opaque = NULL;
+}
+
static int raw_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors)
{
@@ -287,6 +389,9 @@ static BlockDriver bdrv_file = {
.protocol_name = "file",
.instance_size = sizeof(BDRVRawState),
.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,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 16/19] block: vdi image file reopen
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (14 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 15/19] block: raw-win32 driver reopen support Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 17/19] block: vpc " Jeff Cody
` (2 subsequent siblings)
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
There is currently nothing that needs to be done for VDI reopen.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/vdi.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/block/vdi.c b/block/vdi.c
index c4f1529..ce38395 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -454,6 +454,12 @@ static int vdi_open(BlockDriverState *bs, int flags)
return -1;
}
+static int vdi_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+ return 0;
+}
+
static int coroutine_fn vdi_co_is_allocated(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, int *pnum)
{
@@ -762,6 +768,7 @@ static BlockDriver bdrv_vdi = {
.bdrv_probe = vdi_probe,
.bdrv_open = vdi_open,
.bdrv_close = vdi_close,
+ .bdrv_reopen_prepare = vdi_reopen_prepare,
.bdrv_create = vdi_create,
.bdrv_co_is_allocated = vdi_co_is_allocated,
.bdrv_make_empty = vdi_make_empty,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 17/19] block: vpc image file reopen
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (15 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 16/19] block: vdi image file reopen Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 18/19] block: convert bdrv_commit() to use bdrv_reopen() Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 19/19] block: remove keep_read_only flag from BlockDriverState struct Jeff Cody
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
There is currently nothing that needs to be done for VPC image
file reopen.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/vpc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/block/vpc.c b/block/vpc.c
index c0b82c4..b6bf52f 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -265,6 +265,12 @@ static int vpc_open(BlockDriverState *bs, int flags)
return err;
}
+static int vpc_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+ return 0;
+}
+
/*
* Returns the absolute byte offset of the given sector in the image file.
* If the sector is not allocated, -1 is returned instead.
@@ -783,6 +789,7 @@ static BlockDriver bdrv_vpc = {
.bdrv_probe = vpc_probe,
.bdrv_open = vpc_open,
.bdrv_close = vpc_close,
+ .bdrv_reopen_prepare = vpc_reopen_prepare,
.bdrv_create = vpc_create,
.bdrv_read = vpc_co_read,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 18/19] block: convert bdrv_commit() to use bdrv_reopen()
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (16 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 17/19] block: vpc " Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 19/19] block: remove keep_read_only flag from BlockDriverState struct Jeff Cody
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
Currently, bdrv_commit() reopens images r/w itself, via risky
_delete() and _open() calls. Use the new safe method for drive reopen.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 48 +++++-------------------------------------------
1 file changed, 5 insertions(+), 43 deletions(-)
diff --git a/block.c b/block.c
index d625703..ebdbf61 100644
--- a/block.c
+++ b/block.c
@@ -1497,13 +1497,11 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix)
int bdrv_commit(BlockDriverState *bs)
{
BlockDriver *drv = bs->drv;
- BlockDriver *backing_drv;
int64_t sector, total_sectors;
int n, ro, open_flags;
- int ret = 0, rw_ret = 0;
+ int ret = 0;
uint8_t *buf;
char filename[1024];
- BlockDriverState *bs_rw, *bs_ro;
if (!drv)
return -ENOMEDIUM;
@@ -1512,42 +1510,18 @@ int bdrv_commit(BlockDriverState *bs)
return -ENOTSUP;
}
- if (bs->backing_hd->keep_read_only) {
- return -EACCES;
- }
-
if (bdrv_in_use(bs) || bdrv_in_use(bs->backing_hd)) {
return -EBUSY;
}
- backing_drv = bs->backing_hd->drv;
ro = bs->backing_hd->read_only;
strncpy(filename, bs->backing_hd->filename, sizeof(filename));
open_flags = bs->backing_hd->open_flags;
if (ro) {
- /* re-open as RW */
- bdrv_delete(bs->backing_hd);
- bs->backing_hd = NULL;
- bs_rw = bdrv_new("");
- rw_ret = bdrv_open(bs_rw, filename, open_flags | BDRV_O_RDWR,
- backing_drv);
- if (rw_ret < 0) {
- bdrv_delete(bs_rw);
- /* try to re-open read-only */
- bs_ro = bdrv_new("");
- ret = bdrv_open(bs_ro, filename, open_flags & ~BDRV_O_RDWR,
- backing_drv);
- if (ret < 0) {
- bdrv_delete(bs_ro);
- /* drive not functional anymore */
- bs->drv = NULL;
- return ret;
- }
- bs->backing_hd = bs_ro;
- return rw_ret;
+ if (bdrv_reopen(bs->backing_hd, open_flags | BDRV_O_RDWR, NULL)) {
+ return -EACCES;
}
- bs->backing_hd = bs_rw;
}
total_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
@@ -1584,20 +1558,8 @@ ro_cleanup:
g_free(buf);
if (ro) {
- /* re-open as RO */
- bdrv_delete(bs->backing_hd);
- bs->backing_hd = NULL;
- bs_ro = bdrv_new("");
- ret = bdrv_open(bs_ro, filename, open_flags & ~BDRV_O_RDWR,
- backing_drv);
- if (ret < 0) {
- bdrv_delete(bs_ro);
- /* drive not functional anymore */
- bs->drv = NULL;
- return ret;
- }
- bs->backing_hd = bs_ro;
- bs->backing_hd->keep_read_only = 0;
+ /* ignoring error return here */
+ bdrv_reopen(bs->backing_hd, open_flags & ~BDRV_O_RDWR, NULL);
}
return ret;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread* [Qemu-devel] [PATCH v3 19/19] block: remove keep_read_only flag from BlockDriverState struct
2012-09-18 18:53 [Qemu-devel] [PATCH v3 00/19] block: bdrv_reopen() patches Jeff Cody
` (17 preceding siblings ...)
2012-09-18 18:53 ` [Qemu-devel] [PATCH v3 18/19] block: convert bdrv_commit() to use bdrv_reopen() Jeff Cody
@ 2012-09-18 18:53 ` Jeff Cody
18 siblings, 0 replies; 30+ messages in thread
From: Jeff Cody @ 2012-09-18 18:53 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, eblake, supriyak
The keep_read_only flag is no longer used, in favor of the bdrv
flag BDRV_O_ALLOW_RDWR.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 2 --
block_int.h | 1 -
2 files changed, 3 deletions(-)
diff --git a/block.c b/block.c
index ebdbf61..5db1df1 100644
--- a/block.c
+++ b/block.c
@@ -808,8 +808,6 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
flags |= BDRV_O_ALLOW_RDWR;
}
- bs->keep_read_only = !(flags & BDRV_O_ALLOW_RDWR);
-
/* Open the image */
ret = bdrv_open_common(bs, filename, flags, drv);
if (ret < 0) {
diff --git a/block_int.h b/block_int.h
index 22b3d93..ac4245c 100644
--- a/block_int.h
+++ b/block_int.h
@@ -275,7 +275,6 @@ struct BlockDriverState {
int64_t total_sectors; /* if we are reading a disk image, give its
size in sectors */
int read_only; /* if true, the media is read only */
- int keep_read_only; /* if true, the media was requested to stay read only */
int open_flags; /* flags used to open the file, re-used for re-open */
int encrypted; /* if true, the media is encrypted */
int valid_key; /* if true, a valid encryption key has been set */
--
1.7.11.4
^ permalink raw reply related [flat|nested] 30+ messages in thread