* [Qemu-devel] [PATCH 1/2] block: copy over job and dirty bitmap fields in bdrv_append
2012-06-14 14:55 [Qemu-devel] [PATCH 0/2] introduce bdrv_swap, implement bdrv_append on top Paolo Bonzini
@ 2012-06-14 14:55 ` Paolo Bonzini
2012-06-14 14:55 ` [Qemu-devel] [PATCH 2/2] block: introduce bdrv_swap, implement bdrv_append on top of it Paolo Bonzini
2012-06-15 9:38 ` [Qemu-devel] [PATCH 0/2] introduce bdrv_swap, implement bdrv_append on top Kevin Wolf
2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2012-06-14 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody
While these should not be in use at the time a transaction is started,
a command in the prepare phase of a transaction might have added them,
so they need to be brought over.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/block.c b/block.c
index 0acdcac..702821d 100644
--- a/block.c
+++ b/block.c
@@ -1027,6 +1027,16 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
tmp.iostatus_enabled = bs_top->iostatus_enabled;
tmp.iostatus = bs_top->iostatus;
+ /* dirty bitmap */
+ tmp.dirty_count = bs_top->dirty_count;
+ tmp.dirty_bitmap = bs_top->dirty_bitmap;
+ assert(bs_new->dirty_bitmap == NULL);
+
+ /* job */
+ tmp.in_use = bs_top->in_use;
+ tmp.job = bs_top->job;
+ assert(bs_new->job == NULL);
+
/* keep the same entry in bdrv_states */
pstrcpy(tmp.device_name, sizeof(tmp.device_name), bs_top->device_name);
tmp.list = bs_top->list;
@@ -1051,6 +1061,11 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
/* clear the copied fields in the new backing file */
bdrv_detach_dev(bs_new, bs_new->dev);
+ bs_new->job = NULL;
+ bs_new->in_use = 0;
+ bs_new->dirty_bitmap = NULL;
+ bs_new->dirty_count = 0;
+
qemu_co_queue_init(&bs_new->throttled_reqs);
memset(&bs_new->io_base, 0, sizeof(bs_new->io_base));
memset(&bs_new->io_limits, 0, sizeof(bs_new->io_limits));
--
1.7.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH 2/2] block: introduce bdrv_swap, implement bdrv_append on top of it
2012-06-14 14:55 [Qemu-devel] [PATCH 0/2] introduce bdrv_swap, implement bdrv_append on top Paolo Bonzini
2012-06-14 14:55 ` [Qemu-devel] [PATCH 1/2] block: copy over job and dirty bitmap fields in bdrv_append Paolo Bonzini
@ 2012-06-14 14:55 ` Paolo Bonzini
2012-07-04 10:30 ` Kevin Wolf
2012-06-15 9:38 ` [Qemu-devel] [PATCH 0/2] introduce bdrv_swap, implement bdrv_append on top Kevin Wolf
2 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2012-06-14 14:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody
The new function can be made a bit nicer than bdrv_append. It swaps the
whole contents, and then swaps back (using the usual t=a;a=b;b=t idiom)
the fields that need to stay on top. Thus, it does not need explicit
bdrv_detach_dev, bdrv_iostatus_disable, etc.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block.c | 184 ++++++++++++++++++++++++++++++++++-----------------------------
block.h | 1 +
2 files changed, 100 insertions(+), 85 deletions(-)
diff --git a/block.c b/block.c
index 702821d..0991ded 100644
--- a/block.c
+++ b/block.c
@@ -971,116 +971,130 @@ static void bdrv_rebind(BlockDriverState *bs)
}
}
-/*
- * Add new bs contents at the top of an image chain while the chain is
- * live, while keeping required fields on the top layer.
- *
- * This will modify the BlockDriverState fields, and swap contents
- * between bs_new and bs_top. Both bs_new and bs_top are modified.
- *
- * bs_new is required to be anonymous.
- *
- * This function does not create any image files.
- */
-void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
+static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
+ BlockDriverState *bs_src)
{
- BlockDriverState tmp;
-
- /* bs_new must be anonymous */
- assert(bs_new->device_name[0] == '\0');
-
- tmp = *bs_new;
-
- /* there are some fields that need to stay on the top layer: */
- tmp.open_flags = bs_top->open_flags;
+ /* move some fields that need to stay attached to the device */
+ bs_dest->open_flags = bs_src->open_flags;
/* dev info */
- tmp.dev_ops = bs_top->dev_ops;
- tmp.dev_opaque = bs_top->dev_opaque;
- tmp.dev = bs_top->dev;
- tmp.buffer_alignment = bs_top->buffer_alignment;
- tmp.copy_on_read = bs_top->copy_on_read;
+ bs_dest->dev_ops = bs_src->dev_ops;
+ bs_dest->dev_opaque = bs_src->dev_opaque;
+ bs_dest->dev = bs_src->dev;
+ bs_dest->buffer_alignment = bs_src->buffer_alignment;
+ bs_dest->copy_on_read = bs_src->copy_on_read;
- tmp.enable_write_cache = bs_top->enable_write_cache;
+ bs_dest->enable_write_cache = bs_src->enable_write_cache;
/* i/o timing parameters */
- tmp.slice_time = bs_top->slice_time;
- tmp.slice_start = bs_top->slice_start;
- tmp.slice_end = bs_top->slice_end;
- tmp.io_limits = bs_top->io_limits;
- tmp.io_base = bs_top->io_base;
- tmp.throttled_reqs = bs_top->throttled_reqs;
- tmp.block_timer = bs_top->block_timer;
- tmp.io_limits_enabled = bs_top->io_limits_enabled;
+ bs_dest->slice_time = bs_src->slice_time;
+ bs_dest->slice_start = bs_src->slice_start;
+ bs_dest->slice_end = bs_src->slice_end;
+ bs_dest->io_limits = bs_src->io_limits;
+ bs_dest->io_base = bs_src->io_base;
+ bs_dest->throttled_reqs = bs_src->throttled_reqs;
+ bs_dest->block_timer = bs_src->block_timer;
+ bs_dest->io_limits_enabled = bs_src->io_limits_enabled;
/* geometry */
- tmp.cyls = bs_top->cyls;
- tmp.heads = bs_top->heads;
- tmp.secs = bs_top->secs;
- tmp.translation = bs_top->translation;
+ bs_dest->cyls = bs_src->cyls;
+ bs_dest->heads = bs_src->heads;
+ bs_dest->secs = bs_src->secs;
+ bs_dest->translation = bs_src->translation;
/* r/w error */
- tmp.on_read_error = bs_top->on_read_error;
- tmp.on_write_error = bs_top->on_write_error;
+ bs_dest->on_read_error = bs_src->on_read_error;
+ bs_dest->on_write_error = bs_src->on_write_error;
/* i/o status */
- tmp.iostatus_enabled = bs_top->iostatus_enabled;
- tmp.iostatus = bs_top->iostatus;
+ bs_dest->iostatus_enabled = bs_src->iostatus_enabled;
+ bs_dest->iostatus = bs_src->iostatus;
/* dirty bitmap */
- tmp.dirty_count = bs_top->dirty_count;
- tmp.dirty_bitmap = bs_top->dirty_bitmap;
- assert(bs_new->dirty_bitmap == NULL);
+ bs_dest->dirty_count = bs_src->dirty_count;
+ bs_dest->dirty_bitmap = bs_src->dirty_bitmap;
/* job */
- tmp.in_use = bs_top->in_use;
- tmp.job = bs_top->job;
- assert(bs_new->job == NULL);
+ bs_dest->in_use = bs_src->in_use;
+ bs_dest->job = bs_src->job;
/* keep the same entry in bdrv_states */
- pstrcpy(tmp.device_name, sizeof(tmp.device_name), bs_top->device_name);
- tmp.list = bs_top->list;
+ pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name),
+ bs_src->device_name);
+ bs_dest->list = bs_src->list;
+}
- /* The contents of 'tmp' will become bs_top, as we are
- * swapping bs_new and bs_top contents. */
- tmp.backing_hd = bs_new;
- pstrcpy(tmp.backing_file, sizeof(tmp.backing_file), bs_top->filename);
- pstrcpy(tmp.backing_format, sizeof(tmp.backing_format),
- bs_top->drv ? bs_top->drv->format_name : "");
-
- /* swap contents of the fixed new bs and the current top */
- *bs_new = *bs_top;
- *bs_top = tmp;
-
- /* device_name[] was carried over from the old bs_top. bs_new
- * shouldn't be in bdrv_states, so we need to make device_name[]
- * reflect the anonymity of bs_new
- */
- bs_new->device_name[0] = '\0';
+/*
+ * Swap bs contents for two image chains while they are live,
+ * while keeping required fields on the BlockDriverState that is
+ * actually attached to a device.
+ *
+ * This will modify the BlockDriverState fields, and swap contents
+ * between bs_new and bs_old. Both bs_new and bs_old are modified.
+ *
+ * bs_new is required to be anonymous.
+ *
+ * This function does not create any image files.
+ */
+void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)
+{
+ BlockDriverState tmp;
- /* clear the copied fields in the new backing file */
- bdrv_detach_dev(bs_new, bs_new->dev);
+ /* bs_new must be anonymous and shouldn't have anything fancy enabled */
+ assert(bs_new->device_name[0] == '\0');
+ assert(bs_new->dirty_bitmap == NULL);
+ assert(bs_new->job == NULL);
+ assert(bs_new->dev == NULL);
+ assert(bs_new->in_use == 0);
+ assert(bs_new->io_limits_enabled == false);
+ assert(bs_new->block_timer == NULL);
- bs_new->job = NULL;
- bs_new->in_use = 0;
- bs_new->dirty_bitmap = NULL;
- bs_new->dirty_count = 0;
+ tmp = *bs_new;
+ *bs_new = *bs_old;
+ *bs_old = tmp;
- qemu_co_queue_init(&bs_new->throttled_reqs);
- memset(&bs_new->io_base, 0, sizeof(bs_new->io_base));
- memset(&bs_new->io_limits, 0, sizeof(bs_new->io_limits));
- bdrv_iostatus_disable(bs_new);
+ /* there are some fields that should not be swapped, move them back */
+ bdrv_move_feature_fields(&tmp, bs_old);
+ bdrv_move_feature_fields(bs_old, bs_new);
+ bdrv_move_feature_fields(bs_new, &tmp);
- /* we don't use bdrv_io_limits_disable() for this, because we don't want
- * to affect or delete the block_timer, as it has been moved to bs_top */
- bs_new->io_limits_enabled = false;
- bs_new->block_timer = NULL;
- bs_new->slice_time = 0;
- bs_new->slice_start = 0;
- bs_new->slice_end = 0;
+ /* bs_new shouldn't be in bdrv_states even after the swap! */
+ assert(bs_new->device_name[0] == '\0');
+
+ /* Check a few fields that should remain attached to the device */
+ assert(bs_new->dev == NULL);
+ assert(bs_new->job == NULL);
+ assert(bs_new->in_use == 0);
+ assert(bs_new->io_limits_enabled == false);
+ assert(bs_new->block_timer == NULL);
bdrv_rebind(bs_new);
- bdrv_rebind(bs_top);
+ bdrv_rebind(bs_old);
+}
+
+/*
+ * Add new bs contents at the top of an image chain while the chain is
+ * live, while keeping required fields on the top layer.
+ *
+ * This will modify the BlockDriverState fields, and swap contents
+ * between bs_new and bs_top. Both bs_new and bs_top are modified.
+ *
+ * bs_new is required to be anonymous.
+ *
+ * This function does not create any image files.
+ */
+void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
+{
+ bdrv_swap(bs_new, bs_top);
+
+ /* The contents of 'tmp' will become bs_top, as we are
+ * swapping bs_new and bs_top contents. */
+ bs_top->backing_hd = bs_new;
+ bs_top->open_flags &= ~BDRV_O_NO_BACKING;
+ pstrcpy(bs_top->backing_file, sizeof(bs_top->backing_file),
+ bs_new->filename);
+ pstrcpy(bs_top->backing_format, sizeof(bs_top->backing_format),
+ bs_new->drv ? bs_new->drv->format_name : "");
}
void bdrv_delete(BlockDriverState *bs)
diff --git a/block.h b/block.h
index 42e30d6..3af93c6 100644
--- a/block.h
+++ b/block.h
@@ -122,6 +122,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
int bdrv_create_file(const char* filename, QEMUOptionParameter *options);
BlockDriverState *bdrv_new(const char *device_name);
void bdrv_make_anon(BlockDriverState *bs);
+void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
void bdrv_delete(BlockDriverState *bs);
int bdrv_parse_cache_flags(const char *mode, int *flags);
--
1.7.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread