* [Qemu-devel] [PULL 01/33] qcow2: Pass discard type to qcow2_discard_clusters()
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 02/33] qcow2: Discard VM state in active L1 after creating snapshot Kevin Wolf
` (31 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
The function will be used internally instead of only being called for
guest discard requests.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-cluster.c | 8 ++++----
block/qcow2.c | 2 +-
block/qcow2.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 2d5aa92..b0d688e 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1338,7 +1338,7 @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
* clusters.
*/
static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
- unsigned int nb_clusters)
+ unsigned int nb_clusters, enum qcow2_discard_type type)
{
BDRVQcowState *s = bs->opaque;
uint64_t *l2_table;
@@ -1367,7 +1367,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
l2_table[l2_index + i] = cpu_to_be64(0);
/* Then decrease the refcount */
- qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
+ qcow2_free_any_clusters(bs, old_offset, 1, type);
}
ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
@@ -1379,7 +1379,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
}
int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
- int nb_sectors)
+ int nb_sectors, enum qcow2_discard_type type)
{
BDRVQcowState *s = bs->opaque;
uint64_t end_offset;
@@ -1402,7 +1402,7 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
/* Each L2 table is handled by its own loop iteration */
while (nb_clusters > 0) {
- ret = discard_single_l2(bs, offset, nb_clusters);
+ ret = discard_single_l2(bs, offset, nb_clusters, type);
if (ret < 0) {
goto fail;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 578792f..147822e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1582,7 +1582,7 @@ static coroutine_fn int qcow2_co_discard(BlockDriverState *bs,
qemu_co_mutex_lock(&s->lock);
ret = qcow2_discard_clusters(bs, sector_num << BDRV_SECTOR_BITS,
- nb_sectors);
+ nb_sectors, QCOW2_DISCARD_REQUEST);
qemu_co_mutex_unlock(&s->lock);
return ret;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 1000239..9c33b98 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -450,7 +450,7 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
- int nb_sectors);
+ int nb_sectors, enum qcow2_discard_type type);
int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors);
/* qcow2-snapshot.c functions */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 02/33] qcow2: Discard VM state in active L1 after creating snapshot
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 01/33] qcow2: Pass discard type to qcow2_discard_clusters() Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 03/33] raw-win32.c: Fix incorrect handling behaviour of small block files Kevin Wolf
` (30 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
During savevm, the VM state is written to the active L1 of the image and
then a snapshot is taken. After that, the VM state isn't needed any more
in the active L1 and should be discarded. This is implemented by this
patch.
The impact of not discarding the VM state is that a snapshot can never
become smaller than any previous snapshot (because it would be padded
with old VM state), and more importantly that future savevm operations
cause unnecessary COWs (with associated flushes), which makes subsequent
snapshots much slower.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-snapshot.c | 7 +++++++
block/qcow2.c | 5 -----
block/qcow2.h | 5 +++++
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index e7e6013..ffead08 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -416,6 +416,13 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
g_free(old_snapshot_list);
+ /* The VM state isn't needed any more in the active L1 table; in fact, it
+ * hurts by causing expensive COW for the next snapshot. */
+ qcow2_discard_clusters(bs, qcow2_vm_state_offset(s),
+ align_offset(sn->vm_state_size, s->cluster_size)
+ >> BDRV_SECTOR_BITS,
+ QCOW2_DISCARD_NEVER);
+
#ifdef DEBUG_ALLOC
{
BdrvCheckResult result = {0};
diff --git a/block/qcow2.c b/block/qcow2.c
index 147822e..c9e266e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1757,11 +1757,6 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)
return 0;
}
-static int64_t qcow2_vm_state_offset(BDRVQcowState *s)
-{
- return (int64_t)s->l1_vm_state_index << (s->cluster_bits + s->l2_bits);
-}
-
static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
{
BDRVQcowState *s = bs->opaque;
diff --git a/block/qcow2.h b/block/qcow2.h
index 9c33b98..49eed82 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -361,6 +361,11 @@ static inline int64_t align_offset(int64_t offset, int n)
return offset;
}
+static inline int64_t qcow2_vm_state_offset(BDRVQcowState *s)
+{
+ return (int64_t)s->l1_vm_state_index << (s->cluster_bits + s->l2_bits);
+}
+
static inline int qcow2_get_cluster_type(uint64_t l2_entry)
{
if (l2_entry & QCOW_OFLAG_COMPRESSED) {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 03/33] raw-win32.c: Fix incorrect handling behaviour of small block files
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 01/33] qcow2: Pass discard type to qcow2_discard_clusters() Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 02/33] qcow2: Discard VM state in active L1 after creating snapshot Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 04/33] block: Image file option amendment Kevin Wolf
` (29 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Tal Kain <tal.kain@ravellosystems.com>
It is a valid case that the read data's size is smaller than the
requested size since there could be files that are smaller than
the minimum block size (For ex. when a VMDK disk descriptor file)
Signed-off-by: Tal Kain <tal.kain@ravellosystems.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/raw-win32.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/raw-win32.c b/block/raw-win32.c
index d2d2d9f..ff3c5ea 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -85,6 +85,7 @@ static size_t handle_aiocb_rw(RawWin32AIOData *aiocb)
ret_count = 0;
}
if (ret_count != len) {
+ offset += ret_count;
break;
}
offset += len;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 04/33] block: Image file option amendment
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (2 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 03/33] raw-win32.c: Fix incorrect handling behaviour of small block files Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 05/33] qcow2-cache: Empty cache Kevin Wolf
` (28 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
This patch adds the "amend" option to qemu-img which allows changing
image options on existing image files. It also adds the generic bdrv
implementation which is basically just a wrapper for the image format
specific function.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 8 +++++
include/block/block.h | 2 ++
include/block/block_int.h | 3 ++
qemu-img-cmds.hx | 6 ++++
qemu-img.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++
qemu-img.texi | 5 +++
6 files changed, 108 insertions(+)
diff --git a/block.c b/block.c
index a325efc..b81d1e2 100644
--- a/block.c
+++ b/block.c
@@ -4579,3 +4579,11 @@ void bdrv_add_before_write_notifier(BlockDriverState *bs,
{
notifier_with_return_list_add(&bs->before_write_notifiers, notifier);
}
+
+int bdrv_amend_options(BlockDriverState *bs, QEMUOptionParameter *options)
+{
+ if (bs->drv->bdrv_amend_options == NULL) {
+ return -ENOTSUP;
+ }
+ return bs->drv->bdrv_amend_options(bs, options);
+}
diff --git a/include/block/block.h b/include/block/block.h
index 728ec1a..1c5f939 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -241,6 +241,8 @@ typedef enum {
int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix);
+int bdrv_amend_options(BlockDriverState *bs_new, QEMUOptionParameter *options);
+
/* async block I/O */
typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,
int sector_num);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7c35198..1541777 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -188,6 +188,9 @@ struct BlockDriver {
int (*bdrv_check)(BlockDriverState* bs, BdrvCheckResult *result,
BdrvCheckMode fix);
+ int (*bdrv_amend_options)(BlockDriverState *bs,
+ QEMUOptionParameter *options);
+
void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event);
/* TODO Better pass a option string/QDict/QemuOpts to add any rule? */
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 0c36e59..da1d965 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -67,5 +67,11 @@ DEF("resize", img_resize,
"resize [-q] filename [+ | -]size")
STEXI
@item resize [-q] @var{filename} [+ | -]@var{size}
+ETEXI
+
+DEF("amend", img_amend,
+ "amend [-q] [-f fmt] -o options filename")
+STEXI
+@item amend [-q] [-f @var{fmt}] -o @var{options} @var{filename}
@end table
ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index 3e5e388..0cf6be2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2525,6 +2525,90 @@ out:
return 0;
}
+static int img_amend(int argc, char **argv)
+{
+ int c, ret = 0;
+ char *options = NULL;
+ QEMUOptionParameter *create_options = NULL, *options_param = NULL;
+ const char *fmt = NULL, *filename;
+ bool quiet = false;
+ BlockDriverState *bs = NULL;
+
+ for (;;) {
+ c = getopt(argc, argv, "hqf:o:");
+ if (c == -1) {
+ break;
+ }
+
+ switch (c) {
+ case 'h':
+ case '?':
+ help();
+ break;
+ case 'o':
+ options = optarg;
+ break;
+ case 'f':
+ fmt = optarg;
+ break;
+ case 'q':
+ quiet = true;
+ break;
+ }
+ }
+
+ if (optind != argc - 1) {
+ help();
+ }
+
+ if (!options) {
+ help();
+ }
+
+ filename = argv[argc - 1];
+
+ bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet);
+ if (!bs) {
+ error_report("Could not open image '%s'", filename);
+ ret = -1;
+ goto out;
+ }
+
+ fmt = bs->drv->format_name;
+
+ if (is_help_option(options)) {
+ ret = print_block_option_help(filename, fmt);
+ goto out;
+ }
+
+ create_options = append_option_parameters(create_options,
+ bs->drv->create_options);
+ options_param = parse_option_parameters(options, create_options,
+ options_param);
+ if (options_param == NULL) {
+ error_report("Invalid options for file format '%s'", fmt);
+ ret = -1;
+ goto out;
+ }
+
+ ret = bdrv_amend_options(bs, options_param);
+ if (ret < 0) {
+ error_report("Error while amending options: %s", strerror(-ret));
+ goto out;
+ }
+
+out:
+ if (bs) {
+ bdrv_unref(bs);
+ }
+ free_option_parameters(create_options);
+ free_option_parameters(options_param);
+ if (ret) {
+ return 1;
+ }
+ return 0;
+}
+
static const img_cmd_t img_cmds[] = {
#define DEF(option, callback, arg_string) \
{ option, callback },
diff --git a/qemu-img.texi b/qemu-img.texi
index 43ee4eb..768054e 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -350,6 +350,11 @@ sizes accordingly. Failure to do so will result in data loss!
After using this command to grow a disk image, you must use file system and
partitioning tools inside the VM to actually begin using the new space on the
device.
+
+@item amend [-f @var{fmt}] -o @var{options} @var{filename}
+
+Amends the image format specific @var{options} for the image file
+@var{filename}. Not all file formats support this operation.
@end table
@c man end
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 05/33] qcow2-cache: Empty cache
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (3 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 04/33] block: Image file option amendment Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 06/33] qcow2-cluster: Expand zero clusters Kevin Wolf
` (27 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Add a function for emptying a cache, i.e., flushing it and marking all
elements invalid.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2-cache.c | 18 ++++++++++++++++++
block/qcow2.h | 2 ++
2 files changed, 20 insertions(+)
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 7bcae09..40a5a3f 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -202,6 +202,24 @@ void qcow2_cache_depends_on_flush(Qcow2Cache *c)
c->depends_on_flush = true;
}
+int qcow2_cache_empty(BlockDriverState *bs, Qcow2Cache *c)
+{
+ int ret, i;
+
+ ret = qcow2_cache_flush(bs, c);
+ if (ret < 0) {
+ return ret;
+ }
+
+ for (i = 0; i < c->size; i++) {
+ assert(c->entries[i].ref == 0);
+ c->entries[i].offset = 0;
+ c->entries[i].cache_hits = 0;
+ }
+
+ return 0;
+}
+
static int qcow2_cache_find_entry_to_replace(Qcow2Cache *c)
{
int i;
diff --git a/block/qcow2.h b/block/qcow2.h
index 49eed82..35c822b 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -478,6 +478,8 @@ int qcow2_cache_set_dependency(BlockDriverState *bs, Qcow2Cache *c,
Qcow2Cache *dependency);
void qcow2_cache_depends_on_flush(Qcow2Cache *c);
+int qcow2_cache_empty(BlockDriverState *bs, Qcow2Cache *c);
+
int qcow2_cache_get(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,
void **table);
int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 06/33] qcow2-cluster: Expand zero clusters
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (4 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 05/33] qcow2-cache: Empty cache Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 07/33] qcow2: Save refcount order in BDRVQcowState Kevin Wolf
` (26 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Add functionality for expanding zero clusters. This is necessary for
downgrading the image version to one without zero cluster support.
For non-backed images, this function may also just discard zero clusters
instead of truly expanding them.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2-cluster.c | 233 +++++++++++++++++++++++++++++++++++++++++++++++++
block/qcow2-refcount.c | 29 +++---
block/qcow2.h | 5 ++
3 files changed, 253 insertions(+), 14 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index b0d688e..738ff73 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1497,3 +1497,236 @@ fail:
return ret;
}
+
+/*
+ * Expands all zero clusters in a specific L1 table (or deallocates them, for
+ * non-backed non-pre-allocated zero clusters).
+ *
+ * expanded_clusters is a bitmap where every bit corresponds to one cluster in
+ * the image file; a bit gets set if the corresponding cluster has been used for
+ * zero expansion (i.e., has been filled with zeroes and is referenced from an
+ * L2 table). nb_clusters contains the total cluster count of the image file,
+ * i.e., the number of bits in expanded_clusters.
+ */
+static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
+ int l1_size, uint8_t *expanded_clusters,
+ uint64_t nb_clusters)
+{
+ BDRVQcowState *s = bs->opaque;
+ bool is_active_l1 = (l1_table == s->l1_table);
+ uint64_t *l2_table = NULL;
+ int ret;
+ int i, j;
+
+ if (!is_active_l1) {
+ /* inactive L2 tables require a buffer to be stored in when loading
+ * them from disk */
+ l2_table = qemu_blockalign(bs, s->cluster_size);
+ }
+
+ for (i = 0; i < l1_size; i++) {
+ uint64_t l2_offset = l1_table[i] & L1E_OFFSET_MASK;
+ bool l2_dirty = false;
+
+ if (!l2_offset) {
+ /* unallocated */
+ continue;
+ }
+
+ if (is_active_l1) {
+ /* get active L2 tables from cache */
+ ret = qcow2_cache_get(bs, s->l2_table_cache, l2_offset,
+ (void **)&l2_table);
+ } else {
+ /* load inactive L2 tables from disk */
+ ret = bdrv_read(bs->file, l2_offset / BDRV_SECTOR_SIZE,
+ (void *)l2_table, s->cluster_sectors);
+ }
+ if (ret < 0) {
+ goto fail;
+ }
+
+ for (j = 0; j < s->l2_size; j++) {
+ uint64_t l2_entry = be64_to_cpu(l2_table[j]);
+ int64_t offset = l2_entry & L2E_OFFSET_MASK, cluster_index;
+ int cluster_type = qcow2_get_cluster_type(l2_entry);
+
+ if (cluster_type == QCOW2_CLUSTER_NORMAL) {
+ cluster_index = offset >> s->cluster_bits;
+ assert((cluster_index >= 0) && (cluster_index < nb_clusters));
+ if (expanded_clusters[cluster_index / 8] &
+ (1 << (cluster_index % 8))) {
+ /* Probably a shared L2 table; this cluster was a zero
+ * cluster which has been expanded, its refcount
+ * therefore most likely requires an update. */
+ ret = qcow2_update_cluster_refcount(bs, cluster_index, 1,
+ QCOW2_DISCARD_NEVER);
+ if (ret < 0) {
+ goto fail;
+ }
+ /* Since we just increased the refcount, the COPIED flag may
+ * no longer be set. */
+ l2_table[j] = cpu_to_be64(l2_entry & ~QCOW_OFLAG_COPIED);
+ l2_dirty = true;
+ }
+ continue;
+ }
+ else if (qcow2_get_cluster_type(l2_entry) != QCOW2_CLUSTER_ZERO) {
+ continue;
+ }
+
+ if (!offset) {
+ /* not preallocated */
+ if (!bs->backing_hd) {
+ /* not backed; therefore we can simply deallocate the
+ * cluster */
+ l2_table[j] = 0;
+ l2_dirty = true;
+ continue;
+ }
+
+ offset = qcow2_alloc_clusters(bs, s->cluster_size);
+ if (offset < 0) {
+ ret = offset;
+ goto fail;
+ }
+ }
+
+ ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
+ offset, s->cluster_size);
+ if (ret < 0) {
+ qcow2_free_clusters(bs, offset, s->cluster_size,
+ QCOW2_DISCARD_ALWAYS);
+ goto fail;
+ }
+
+ ret = bdrv_write_zeroes(bs->file, offset / BDRV_SECTOR_SIZE,
+ s->cluster_sectors);
+ if (ret < 0) {
+ qcow2_free_clusters(bs, offset, s->cluster_size,
+ QCOW2_DISCARD_ALWAYS);
+ goto fail;
+ }
+
+ l2_table[j] = cpu_to_be64(offset | QCOW_OFLAG_COPIED);
+ l2_dirty = true;
+
+ cluster_index = offset >> s->cluster_bits;
+ assert((cluster_index >= 0) && (cluster_index < nb_clusters));
+ expanded_clusters[cluster_index / 8] |= 1 << (cluster_index % 8);
+ }
+
+ if (is_active_l1) {
+ if (l2_dirty) {
+ qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+ qcow2_cache_depends_on_flush(s->l2_table_cache);
+ }
+ ret = qcow2_cache_put(bs, s->l2_table_cache, (void **)&l2_table);
+ if (ret < 0) {
+ l2_table = NULL;
+ goto fail;
+ }
+ } else {
+ if (l2_dirty) {
+ ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT &
+ ~(QCOW2_OL_INACTIVE_L2 | QCOW2_OL_ACTIVE_L2), l2_offset,
+ s->cluster_size);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ ret = bdrv_write(bs->file, l2_offset / BDRV_SECTOR_SIZE,
+ (void *)l2_table, s->cluster_sectors);
+ if (ret < 0) {
+ goto fail;
+ }
+ }
+ }
+ }
+
+ ret = 0;
+
+fail:
+ if (l2_table) {
+ if (!is_active_l1) {
+ qemu_vfree(l2_table);
+ } else {
+ if (ret < 0) {
+ qcow2_cache_put(bs, s->l2_table_cache, (void **)&l2_table);
+ } else {
+ ret = qcow2_cache_put(bs, s->l2_table_cache,
+ (void **)&l2_table);
+ }
+ }
+ }
+ return ret;
+}
+
+/*
+ * For backed images, expands all zero clusters on the image. For non-backed
+ * images, deallocates all non-pre-allocated zero clusters (and claims the
+ * allocation for pre-allocated ones). This is important for downgrading to a
+ * qcow2 version which doesn't yet support metadata zero clusters.
+ */
+int qcow2_expand_zero_clusters(BlockDriverState *bs)
+{
+ BDRVQcowState *s = bs->opaque;
+ uint64_t *l1_table = NULL;
+ int cluster_to_sector_bits = s->cluster_bits - BDRV_SECTOR_BITS;
+ uint64_t nb_clusters;
+ uint8_t *expanded_clusters;
+ int ret;
+ int i, j;
+
+ nb_clusters = (bs->total_sectors + (1 << cluster_to_sector_bits) - 1)
+ >> cluster_to_sector_bits;
+ expanded_clusters = g_malloc0((nb_clusters + 7) / 8);
+
+ ret = expand_zero_clusters_in_l1(bs, s->l1_table, s->l1_size,
+ expanded_clusters, nb_clusters);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ /* Inactive L1 tables may point to active L2 tables - therefore it is
+ * necessary to flush the L2 table cache before trying to access the L2
+ * tables pointed to by inactive L1 entries (else we might try to expand
+ * zero clusters that have already been expanded); furthermore, it is also
+ * necessary to empty the L2 table cache, since it may contain tables which
+ * are now going to be modified directly on disk, bypassing the cache.
+ * qcow2_cache_empty() does both for us. */
+ ret = qcow2_cache_empty(bs, s->l2_table_cache);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ for (i = 0; i < s->nb_snapshots; i++) {
+ int l1_sectors = (s->snapshots[i].l1_size * sizeof(uint64_t) +
+ BDRV_SECTOR_SIZE - 1) / BDRV_SECTOR_SIZE;
+
+ l1_table = g_realloc(l1_table, l1_sectors * BDRV_SECTOR_SIZE);
+
+ ret = bdrv_read(bs->file, s->snapshots[i].l1_table_offset /
+ BDRV_SECTOR_SIZE, (void *)l1_table, l1_sectors);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ for (j = 0; j < s->snapshots[i].l1_size; j++) {
+ be64_to_cpus(&l1_table[j]);
+ }
+
+ ret = expand_zero_clusters_in_l1(bs, l1_table, s->snapshots[i].l1_size,
+ expanded_clusters, nb_clusters);
+ if (ret < 0) {
+ goto fail;
+ }
+ }
+
+ ret = 0;
+
+fail:
+ g_free(expanded_clusters);
+ g_free(l1_table);
+ return ret;
+}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index ba129de..4264148 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -601,10 +601,10 @@ fail:
* If the return value is non-negative, it is the new refcount of the cluster.
* If it is negative, it is -errno and indicates an error.
*/
-static int update_cluster_refcount(BlockDriverState *bs,
- int64_t cluster_index,
- int addend,
- enum qcow2_discard_type type)
+int qcow2_update_cluster_refcount(BlockDriverState *bs,
+ int64_t cluster_index,
+ int addend,
+ enum qcow2_discard_type type)
{
BDRVQcowState *s = bs->opaque;
int ret;
@@ -733,8 +733,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
if (free_in_cluster == 0)
s->free_byte_offset = 0;
if ((offset & (s->cluster_size - 1)) != 0)
- update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
- QCOW2_DISCARD_NEVER);
+ qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
+ QCOW2_DISCARD_NEVER);
} else {
offset = qcow2_alloc_clusters(bs, s->cluster_size);
if (offset < 0) {
@@ -744,8 +744,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
if ((cluster_offset + s->cluster_size) == offset) {
/* we are lucky: contiguous data */
offset = s->free_byte_offset;
- update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
- QCOW2_DISCARD_NEVER);
+ qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
+ QCOW2_DISCARD_NEVER);
s->free_byte_offset += size;
} else {
s->free_byte_offset = offset;
@@ -754,8 +754,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
}
/* The cluster refcount was incremented, either by qcow2_alloc_clusters()
- * or explicitly by update_cluster_refcount(). Refcount blocks must be
- * flushed before the caller's L2 table updates.
+ * or explicitly by qcow2_update_cluster_refcount(). Refcount blocks must
+ * be flushed before the caller's L2 table updates.
*/
qcow2_cache_set_dependency(bs, s->l2_table_cache, s->refcount_block_cache);
return offset;
@@ -896,8 +896,9 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
break;
}
if (addend != 0) {
- refcount = update_cluster_refcount(bs, cluster_index, addend,
- QCOW2_DISCARD_SNAPSHOT);
+ refcount = qcow2_update_cluster_refcount(bs,
+ cluster_index, addend,
+ QCOW2_DISCARD_SNAPSHOT);
} else {
refcount = get_refcount(bs, cluster_index);
}
@@ -936,8 +937,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
if (addend != 0) {
- refcount = update_cluster_refcount(bs, l2_offset >> s->cluster_bits, addend,
- QCOW2_DISCARD_SNAPSHOT);
+ refcount = qcow2_update_cluster_refcount(bs, l2_offset >>
+ s->cluster_bits, addend, QCOW2_DISCARD_SNAPSHOT);
} else {
refcount = get_refcount(bs, l2_offset >> s->cluster_bits);
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 35c822b..48080fd 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -411,6 +411,9 @@ int qcow2_update_header(BlockDriverState *bs);
int qcow2_refcount_init(BlockDriverState *bs);
void qcow2_refcount_close(BlockDriverState *bs);
+int qcow2_update_cluster_refcount(BlockDriverState *bs, int64_t cluster_index,
+ int addend, enum qcow2_discard_type type);
+
int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t size);
int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
int nb_clusters);
@@ -458,6 +461,8 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
int nb_sectors, enum qcow2_discard_type type);
int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors);
+int qcow2_expand_zero_clusters(BlockDriverState *bs);
+
/* qcow2-snapshot.c functions */
int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 07/33] qcow2: Save refcount order in BDRVQcowState
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (5 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 06/33] qcow2-cluster: Expand zero clusters Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 08/33] qcow2: Implement bdrv_amend_options Kevin Wolf
` (25 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Save the image refcount order in BDRVQcowState. This will be relevant
for future code supporting different refcount orders than four and also
for code that needs to verify a certain refcount order for an opened
image.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2.c | 3 ++-
block/qcow2.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index c9e266e..27203f8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -455,6 +455,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
ret = -ENOTSUP;
goto fail;
}
+ s->refcount_order = header.refcount_order;
if (header.cluster_bits < MIN_CLUSTER_BITS ||
header.cluster_bits > MAX_CLUSTER_BITS) {
@@ -1143,7 +1144,7 @@ int qcow2_update_header(BlockDriverState *bs)
.incompatible_features = cpu_to_be64(s->incompatible_features),
.compatible_features = cpu_to_be64(s->compatible_features),
.autoclear_features = cpu_to_be64(s->autoclear_features),
- .refcount_order = cpu_to_be32(3 + REFCOUNT_SHIFT),
+ .refcount_order = cpu_to_be32(s->refcount_order),
.header_length = cpu_to_be32(header_length),
};
diff --git a/block/qcow2.h b/block/qcow2.h
index 48080fd..bea6ddb 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -199,6 +199,7 @@ typedef struct BDRVQcowState {
int flags;
int qcow_version;
bool use_lazy_refcounts;
+ int refcount_order;
bool discard_passthrough[QCOW2_DISCARD_MAX];
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 08/33] qcow2: Implement bdrv_amend_options
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (6 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 07/33] qcow2: Save refcount order in BDRVQcowState Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 09/33] qemu-iotest: qcow2 image option amendment Kevin Wolf
` (24 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Implement bdrv_amend_options for compat, size, backing_file, backing_fmt
and lazy_refcounts.
Downgrading images from compat=1.1 to compat=0.10 is achieved through
handling all incompatible flags accordingly, clearing all compatible and
autoclear flags and expanding all zero clusters.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2.c | 194 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 194 insertions(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index 27203f8..7c9354c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1820,6 +1820,199 @@ static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf,
return ret;
}
+/*
+ * Downgrades an image's version. To achieve this, any incompatible features
+ * have to be removed.
+ */
+static int qcow2_downgrade(BlockDriverState *bs, int target_version)
+{
+ BDRVQcowState *s = bs->opaque;
+ int current_version = s->qcow_version;
+ int ret;
+
+ if (target_version == current_version) {
+ return 0;
+ } else if (target_version > current_version) {
+ return -EINVAL;
+ } else if (target_version != 2) {
+ return -EINVAL;
+ }
+
+ if (s->refcount_order != 4) {
+ /* we would have to convert the image to a refcount_order == 4 image
+ * here; however, since qemu (at the time of writing this) does not
+ * support anything different than 4 anyway, there is no point in doing
+ * so right now; however, we should error out (if qemu supports this in
+ * the future and this code has not been adapted) */
+ error_report("qcow2_downgrade: Image refcount orders other than 4 are"
+ "currently not supported.");
+ return -ENOTSUP;
+ }
+
+ /* clear incompatible features */
+ if (s->incompatible_features & QCOW2_INCOMPAT_DIRTY) {
+ ret = qcow2_mark_clean(bs);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
+ /* with QCOW2_INCOMPAT_CORRUPT, it is pretty much impossible to get here in
+ * the first place; if that happens nonetheless, returning -ENOTSUP is the
+ * best thing to do anyway */
+
+ if (s->incompatible_features) {
+ return -ENOTSUP;
+ }
+
+ /* since we can ignore compatible features, we can set them to 0 as well */
+ s->compatible_features = 0;
+ /* if lazy refcounts have been used, they have already been fixed through
+ * clearing the dirty flag */
+
+ /* clearing autoclear features is trivial */
+ s->autoclear_features = 0;
+
+ ret = qcow2_expand_zero_clusters(bs);
+ if (ret < 0) {
+ return ret;
+ }
+
+ s->qcow_version = target_version;
+ ret = qcow2_update_header(bs);
+ if (ret < 0) {
+ s->qcow_version = current_version;
+ return ret;
+ }
+ return 0;
+}
+
+static int qcow2_amend_options(BlockDriverState *bs,
+ QEMUOptionParameter *options)
+{
+ BDRVQcowState *s = bs->opaque;
+ int old_version = s->qcow_version, new_version = old_version;
+ uint64_t new_size = 0;
+ const char *backing_file = NULL, *backing_format = NULL;
+ bool lazy_refcounts = s->use_lazy_refcounts;
+ int ret;
+ int i;
+
+ for (i = 0; options[i].name; i++)
+ {
+ if (!options[i].assigned) {
+ /* only change explicitly defined options */
+ continue;
+ }
+
+ if (!strcmp(options[i].name, "compat")) {
+ if (!options[i].value.s) {
+ /* preserve default */
+ } else if (!strcmp(options[i].value.s, "0.10")) {
+ new_version = 2;
+ } else if (!strcmp(options[i].value.s, "1.1")) {
+ new_version = 3;
+ } else {
+ fprintf(stderr, "Unknown compatibility level %s.\n",
+ options[i].value.s);
+ return -EINVAL;
+ }
+ } else if (!strcmp(options[i].name, "preallocation")) {
+ fprintf(stderr, "Cannot change preallocation mode.\n");
+ return -ENOTSUP;
+ } else if (!strcmp(options[i].name, "size")) {
+ new_size = options[i].value.n;
+ } else if (!strcmp(options[i].name, "backing_file")) {
+ backing_file = options[i].value.s;
+ } else if (!strcmp(options[i].name, "backing_fmt")) {
+ backing_format = options[i].value.s;
+ } else if (!strcmp(options[i].name, "encryption")) {
+ if ((options[i].value.n != !!s->crypt_method)) {
+ fprintf(stderr, "Changing the encryption flag is not "
+ "supported.\n");
+ return -ENOTSUP;
+ }
+ } else if (!strcmp(options[i].name, "cluster_size")) {
+ if (options[i].value.n != s->cluster_size) {
+ fprintf(stderr, "Changing the cluster size is not "
+ "supported.\n");
+ return -ENOTSUP;
+ }
+ } else if (!strcmp(options[i].name, "lazy_refcounts")) {
+ lazy_refcounts = options[i].value.n;
+ } else {
+ /* if this assertion fails, this probably means a new option was
+ * added without having it covered here */
+ assert(false);
+ }
+ }
+
+ if (new_version != old_version) {
+ if (new_version > old_version) {
+ /* Upgrade */
+ s->qcow_version = new_version;
+ ret = qcow2_update_header(bs);
+ if (ret < 0) {
+ s->qcow_version = old_version;
+ return ret;
+ }
+ } else {
+ ret = qcow2_downgrade(bs, new_version);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+ }
+
+ if (backing_file || backing_format) {
+ ret = qcow2_change_backing_file(bs, backing_file ?: bs->backing_file,
+ backing_format ?: bs->backing_format);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
+ if (s->use_lazy_refcounts != lazy_refcounts) {
+ if (lazy_refcounts) {
+ if (s->qcow_version < 3) {
+ fprintf(stderr, "Lazy refcounts only supported with compatibility "
+ "level 1.1 and above (use compat=1.1 or greater)\n");
+ return -EINVAL;
+ }
+ s->compatible_features |= QCOW2_COMPAT_LAZY_REFCOUNTS;
+ ret = qcow2_update_header(bs);
+ if (ret < 0) {
+ s->compatible_features &= ~QCOW2_COMPAT_LAZY_REFCOUNTS;
+ return ret;
+ }
+ s->use_lazy_refcounts = true;
+ } else {
+ /* make image clean first */
+ ret = qcow2_mark_clean(bs);
+ if (ret < 0) {
+ return ret;
+ }
+ /* now disallow lazy refcounts */
+ s->compatible_features &= ~QCOW2_COMPAT_LAZY_REFCOUNTS;
+ ret = qcow2_update_header(bs);
+ if (ret < 0) {
+ s->compatible_features |= QCOW2_COMPAT_LAZY_REFCOUNTS;
+ return ret;
+ }
+ s->use_lazy_refcounts = false;
+ }
+ }
+
+ if (new_size) {
+ ret = bdrv_truncate(bs, new_size);
+ if (ret < 0) {
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
static QEMUOptionParameter qcow2_create_options[] = {
{
.name = BLOCK_OPT_SIZE,
@@ -1903,6 +2096,7 @@ static BlockDriver bdrv_qcow2 = {
.create_options = qcow2_create_options,
.bdrv_check = qcow2_check,
+ .bdrv_amend_options = qcow2_amend_options,
};
static void bdrv_qcow2_init(void)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 09/33] qemu-iotest: qcow2 image option amendment
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (7 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 08/33] qcow2: Implement bdrv_amend_options Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 10/33] qemu-iotests: add unix socket help program Kevin Wolf
` (23 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Add tests for qemu-img amend on qcow2 image files.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/061 | 178 +++++++++++++++++++++++
tests/qemu-iotests/061.out | 349 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 528 insertions(+)
create mode 100755 tests/qemu-iotests/061
create mode 100644 tests/qemu-iotests/061.out
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
new file mode 100755
index 0000000..86404e6
--- /dev/null
+++ b/tests/qemu-iotests/061
@@ -0,0 +1,178 @@
+#!/bin/bash
+#
+# Test case for image option amendment in qcow2.
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This tests qocw2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+echo
+echo "=== Testing version downgrade with zero expansion ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
+$QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "=== Testing dirty version downgrade ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | _filter_qemu_io
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "=== Testing version downgrade with unknown compat/autoclear flags ==="
+echo
+IMGOPTS="compat=1.1" _make_test_img 64M
+./qcow2.py "$TEST_IMG" set-feature-bit compatible 42
+./qcow2.py "$TEST_IMG" set-feature-bit autoclear 42
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+./qcow2.py "$TEST_IMG" dump-header
+_check_test_img
+
+echo
+echo "=== Testing version upgrade and resize ==="
+echo
+IMGOPTS="compat=0.10" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IMG amend -o "compat=1.1,lazy_refcounts=on,size=128M" "$TEST_IMG"
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IO -c "read -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "=== Testing dirty lazy_refcounts=off ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | _filter_qemu_io
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IMG amend -o "lazy_refcounts=off" "$TEST_IMG"
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "=== Testing backing file ==="
+echo
+IMGOPTS="compat=1.1" _make_test_img 64M
+IMGOPTS="compat=1.1" TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG amend -o "backing_file=$TEST_IMG.base,backing_fmt=qcow2" "$TEST_IMG"
+$QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "=== Testing invalid configurations ==="
+echo
+IMGOPTS="compat=0.10" _make_test_img 64M
+$QEMU_IMG amend -o "lazy_refcounts=on" "$TEST_IMG"
+$QEMU_IMG amend -o "compat=1.1" "$TEST_IMG" # actually valid
+$QEMU_IMG amend -o "compat=0.10,lazy_refcounts=on" "$TEST_IMG"
+$QEMU_IMG amend -o "compat=0.42" "$TEST_IMG"
+$QEMU_IMG amend -o "foo=bar" "$TEST_IMG"
+$QEMU_IMG amend -o "cluster_size=1k" "$TEST_IMG"
+$QEMU_IMG amend -o "encryption=on" "$TEST_IMG"
+$QEMU_IMG amend -o "preallocation=on" "$TEST_IMG"
+
+echo
+echo "=== Testing correct handling of unset value ==="
+echo
+IMGOPTS="compat=1.1,cluster_size=1k" _make_test_img 64M
+echo "Should work:"
+$QEMU_IMG amend -o "lazy_refcounts=on" "$TEST_IMG"
+echo "Should not work:" # Just to know which of these tests actually fails
+$QEMU_IMG amend -o "cluster_size=64k" "$TEST_IMG"
+
+echo
+echo "=== Testing zero expansion on inactive clusters ==="
+echo
+IMGOPTS="compat=1.1" _make_test_img 64M
+$QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+$QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -a foo "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "=== Testing zero expansion on backed image ==="
+echo
+IMGOPTS="compat=1.1" TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG.base" | _filter_qemu_io
+IMGOPTS="compat=1.1,backing_file=$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "read -P 0x2a 0 128k" -c "write -z 0 64k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 64k" -c "read -P 0x2a 64k 64k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "=== Testing zero expansion on backed inactive clusters ==="
+echo
+IMGOPTS="compat=1.1" TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG.base" | _filter_qemu_io
+IMGOPTS="compat=1.1,backing_file=$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -z 0 64k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+$QEMU_IO -c "write -P 0x42 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0x42 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -a foo "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 64k" -c "read -P 0x2a 64k 64k" "$TEST_IMG" | _filter_qemu_io
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
new file mode 100644
index 0000000..05bd1d5
--- /dev/null
+++ b/tests/qemu-iotests/061.out
@@ -0,0 +1,349 @@
+QA output created by 061
+
+=== Testing version downgrade with zero expansion ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+magic 0x514649fb
+version 3
+backing_file_offset 0x0
+backing_file_size 0x0
+cluster_bits 16
+size 67108864
+crypt_method 0
+l1_size 1
+l1_table_offset 0x30000
+refcount_table_offset 0x10000
+refcount_table_clusters 1
+nb_snapshots 0
+snapshot_offset 0x0
+incompatible_features 0x0
+compatible_features 0x1
+autoclear_features 0x0
+refcount_order 4
+header_length 104
+
+magic 0x514649fb
+version 2
+backing_file_offset 0x0
+backing_file_size 0x0
+cluster_bits 16
+size 67108864
+crypt_method 0
+l1_size 1
+l1_table_offset 0x30000
+refcount_table_offset 0x10000
+refcount_table_clusters 1
+nb_snapshots 0
+snapshot_offset 0x0
+incompatible_features 0x0
+compatible_features 0x0
+autoclear_features 0x0
+refcount_order 4
+header_length 72
+
+Header extension:
+magic 0x6803f857
+length 144
+data <binary>
+
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== Testing dirty version downgrade ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+magic 0x514649fb
+version 3
+backing_file_offset 0x0
+backing_file_size 0x0
+cluster_bits 16
+size 67108864
+crypt_method 0
+l1_size 1
+l1_table_offset 0x30000
+refcount_table_offset 0x10000
+refcount_table_clusters 1
+nb_snapshots 0
+snapshot_offset 0x0
+incompatible_features 0x1
+compatible_features 0x1
+autoclear_features 0x0
+refcount_order 4
+header_length 104
+
+Repairing cluster 5 refcount=0 reference=1
+Repairing cluster 6 refcount=0 reference=1
+magic 0x514649fb
+version 2
+backing_file_offset 0x0
+backing_file_size 0x0
+cluster_bits 16
+size 67108864
+crypt_method 0
+l1_size 1
+l1_table_offset 0x30000
+refcount_table_offset 0x10000
+refcount_table_clusters 1
+nb_snapshots 0
+snapshot_offset 0x0
+incompatible_features 0x0
+compatible_features 0x0
+autoclear_features 0x0
+refcount_order 4
+header_length 72
+
+Header extension:
+magic 0x6803f857
+length 144
+data <binary>
+
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== Testing version downgrade with unknown compat/autoclear flags ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+magic 0x514649fb
+version 3
+backing_file_offset 0x0
+backing_file_size 0x0
+cluster_bits 16
+size 67108864
+crypt_method 0
+l1_size 1
+l1_table_offset 0x30000
+refcount_table_offset 0x10000
+refcount_table_clusters 1
+nb_snapshots 0
+snapshot_offset 0x0
+incompatible_features 0x0
+compatible_features 0x40000000000
+autoclear_features 0x40000000000
+refcount_order 4
+header_length 104
+
+magic 0x514649fb
+version 2
+backing_file_offset 0x0
+backing_file_size 0x0
+cluster_bits 16
+size 67108864
+crypt_method 0
+l1_size 1
+l1_table_offset 0x30000
+refcount_table_offset 0x10000
+refcount_table_clusters 1
+nb_snapshots 0
+snapshot_offset 0x0
+incompatible_features 0x0
+compatible_features 0x0
+autoclear_features 0x0
+refcount_order 4
+header_length 72
+
+Header extension:
+magic 0x6803f857
+length 144
+data <binary>
+
+No errors were found on the image.
+
+=== Testing version upgrade and resize ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 65536/65536 bytes at offset 44040192
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+magic 0x514649fb
+version 2
+backing_file_offset 0x0
+backing_file_size 0x0
+cluster_bits 16
+size 67108864
+crypt_method 0
+l1_size 1
+l1_table_offset 0x30000
+refcount_table_offset 0x10000
+refcount_table_clusters 1
+nb_snapshots 0
+snapshot_offset 0x0
+incompatible_features 0x0
+compatible_features 0x0
+autoclear_features 0x0
+refcount_order 4
+header_length 72
+
+magic 0x514649fb
+version 3
+backing_file_offset 0x0
+backing_file_size 0x0
+cluster_bits 16
+size 134217728
+crypt_method 0
+l1_size 1
+l1_table_offset 0x30000
+refcount_table_offset 0x10000
+refcount_table_clusters 1
+nb_snapshots 0
+snapshot_offset 0x0
+incompatible_features 0x0
+compatible_features 0x1
+autoclear_features 0x0
+refcount_order 4
+header_length 104
+
+Header extension:
+magic 0x6803f857
+length 144
+data <binary>
+
+read 65536/65536 bytes at offset 44040192
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== Testing dirty lazy_refcounts=off ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+magic 0x514649fb
+version 3
+backing_file_offset 0x0
+backing_file_size 0x0
+cluster_bits 16
+size 67108864
+crypt_method 0
+l1_size 1
+l1_table_offset 0x30000
+refcount_table_offset 0x10000
+refcount_table_clusters 1
+nb_snapshots 0
+snapshot_offset 0x0
+incompatible_features 0x1
+compatible_features 0x1
+autoclear_features 0x0
+refcount_order 4
+header_length 104
+
+Repairing cluster 5 refcount=0 reference=1
+Repairing cluster 6 refcount=0 reference=1
+magic 0x514649fb
+version 3
+backing_file_offset 0x0
+backing_file_size 0x0
+cluster_bits 16
+size 67108864
+crypt_method 0
+l1_size 1
+l1_table_offset 0x30000
+refcount_table_offset 0x10000
+refcount_table_clusters 1
+nb_snapshots 0
+snapshot_offset 0x0
+incompatible_features 0x0
+compatible_features 0x0
+autoclear_features 0x0
+refcount_order 4
+header_length 104
+
+Header extension:
+magic 0x6803f857
+length 144
+data <binary>
+
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== Testing backing file ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== Testing invalid configurations ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
+qemu-img: Error while amending options: Invalid argument
+Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
+qemu-img: Error while amending options: Invalid argument
+Unknown compatibility level 0.42.
+qemu-img: Error while amending options: Invalid argument
+Unknown option 'foo'
+qemu-img: Invalid options for file format 'qcow2'
+Changing the cluster size is not supported.
+qemu-img: Error while amending options: Operation not supported
+Changing the encryption flag is not supported.
+qemu-img: Error while amending options: Operation not supported
+Cannot change preallocation mode.
+qemu-img: Error while amending options: Operation not supported
+
+=== Testing correct handling of unset value ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Should work:
+Should not work:
+Changing the cluster size is not supported.
+qemu-img: Error while amending options: Operation not supported
+
+=== Testing zero expansion on inactive clusters ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing zero expansion on backed image ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.base'
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 65536
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing zero expansion on backed inactive clusters ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.base'
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 65536
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 316b1dd..8012828 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -65,5 +65,6 @@
056 rw auto backing
059 rw auto
060 rw auto
+061 rw auto
062 rw auto
063 rw auto
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 10/33] qemu-iotests: add unix socket help program
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (8 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 09/33] qemu-iotest: qcow2 image option amendment Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 11/33] qemu-iotests: add infrastructure of fd passing via SCM Kevin Wolf
` (22 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
This program can do a sendmsg call to transfer fd with unix
socket, which is not supported in python2.
The built binary will not be deleted in clean, but it is a
existing issue in ./tests, which should be solved in another
patch.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
configure | 2 +-
tests/Makefile | 3 +-
tests/qemu-iotests/socket_scm_helper.c | 135 +++++++++++++++++++++++++++++++++
3 files changed, 138 insertions(+), 2 deletions(-)
create mode 100644 tests/qemu-iotests/socket_scm_helper.c
diff --git a/configure b/configure
index 2b83936..b499028 100755
--- a/configure
+++ b/configure
@@ -4651,7 +4651,7 @@ if [ "$dtc_internal" = "yes" ]; then
fi
# build tree in object directory in case the source is not in the current directory
-DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa"
+DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
DIRS="$DIRS roms/seabios roms/vgabios"
DIRS="$DIRS qapi-generated"
diff --git a/tests/Makefile b/tests/Makefile
index c13fefc..994fef1 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -174,6 +174,7 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
+tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
# QTest rules
@@ -252,7 +253,7 @@ check-report.html: check-report.xml
# Other tests
.PHONY: check-tests/qemu-iotests-quick.sh
-check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF)
+check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) tests/qemu-iotests/socket_scm_helper$(EXESUF)
$<
.PHONY: check-tests/test-qapi.py
diff --git a/tests/qemu-iotests/socket_scm_helper.c b/tests/qemu-iotests/socket_scm_helper.c
new file mode 100644
index 0000000..0e2b285
--- /dev/null
+++ b/tests/qemu-iotests/socket_scm_helper.c
@@ -0,0 +1,135 @@
+/*
+ * SCM_RIGHTS with unix socket help program for test
+ *
+ * Copyright IBM, Inc. 2013
+ *
+ * Authors:
+ * Wenchao Xia <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include <stdio.h>
+#include <errno.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/* #define SOCKET_SCM_DEBUG */
+
+/*
+ * @fd and @fd_to_send will not be checked for validation in this function,
+ * a blank will be sent as iov data to notify qemu.
+ */
+static int send_fd(int fd, int fd_to_send)
+{
+ struct msghdr msg;
+ struct iovec iov[1];
+ int ret;
+ char control[CMSG_SPACE(sizeof(int))];
+ struct cmsghdr *cmsg;
+
+ memset(&msg, 0, sizeof(msg));
+ memset(control, 0, sizeof(control));
+
+ /* Send a blank to notify qemu */
+ iov[0].iov_base = (void *)" ";
+ iov[0].iov_len = 1;
+
+ msg.msg_iov = iov;
+ msg.msg_iovlen = 1;
+
+ msg.msg_control = control;
+ msg.msg_controllen = sizeof(control);
+
+ cmsg = CMSG_FIRSTHDR(&msg);
+
+ cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+ cmsg->cmsg_level = SOL_SOCKET;
+ cmsg->cmsg_type = SCM_RIGHTS;
+ memcpy(CMSG_DATA(cmsg), &fd, sizeof(int));
+
+ do {
+ ret = sendmsg(fd, &msg, 0);
+ } while (ret < 0 && errno == EINTR);
+
+ if (ret < 0) {
+ fprintf(stderr, "Failed to send msg, reason: %s\n", strerror(errno));
+ }
+
+ return ret;
+}
+
+/* Convert string to fd number. */
+static int get_fd_num(const char *fd_str)
+{
+ int sock;
+ char *err;
+
+ errno = 0;
+ sock = strtol(fd_str, &err, 10);
+ if (errno) {
+ fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n",
+ strerror(errno));
+ return -1;
+ }
+ if (!*fd_str || *err || sock < 0) {
+ fprintf(stderr, "bad numerical value for socket fd '%s'\n", fd_str);
+ return -1;
+ }
+
+ return sock;
+}
+
+/*
+ * To make things simple, the caller needs to specify:
+ * 1. socket fd.
+ * 2. path of the file to be sent.
+ */
+int main(int argc, char **argv, char **envp)
+{
+ int sock, fd, ret;
+
+#ifdef SOCKET_SCM_DEBUG
+ int i;
+ for (i = 0; i < argc; i++) {
+ fprintf(stderr, "Parameter %d: %s\n", i, argv[i]);
+ }
+#endif
+
+ if (argc != 3) {
+ fprintf(stderr,
+ "Usage: %s < socket-fd > < file-path >\n",
+ argv[0]);
+ return EXIT_FAILURE;
+ }
+
+
+ sock = get_fd_num(argv[1]);
+ if (sock < 0) {
+ return EXIT_FAILURE;
+ }
+
+ /* Now only open a file in readonly mode for test purpose. If more precise
+ control is needed, use python script in file operation, which is
+ supposed to fork and exec this program. */
+ fd = open(argv[2], O_RDONLY);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open file '%s'\n", argv[2]);
+ return EXIT_FAILURE;
+ }
+
+ ret = send_fd(sock, fd);
+ if (ret < 0) {
+ close(fd);
+ return EXIT_FAILURE;
+ }
+
+ close(fd);
+ return EXIT_SUCCESS;
+}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 11/33] qemu-iotests: add infrastructure of fd passing via SCM
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (9 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 10/33] qemu-iotests: add unix socket help program Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 12/33] qemu-iotests: add tests for runtime fd passing via SCM rights Kevin Wolf
` (21 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
This patch make use of the compiled scm helper program to transfer
fd via unix socket at runtime.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
QMP/qmp.py | 6 ++++++
tests/qemu-iotests/check | 1 +
tests/qemu-iotests/iotests.py | 23 +++++++++++++++++++++++
3 files changed, 30 insertions(+)
diff --git a/QMP/qmp.py b/QMP/qmp.py
index c551df1..074f09a 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -188,3 +188,9 @@ class QEMUMonitorProtocol:
def settimeout(self, timeout):
self.__sock.settimeout(timeout)
+
+ def get_sock_fd(self):
+ return self.__sock.fileno()
+
+ def is_scm_available(self):
+ return self.__sock.family == socket.AF_UNIX
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 4ecf497..f5f328f 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -164,6 +164,7 @@ QEMU_IO -- $QEMU_IO
IMGFMT -- $FULL_IMGFMT_DETAILS
IMGPROTO -- $FULL_IMGPROTO_DETAILS
PLATFORM -- $FULL_HOST_DETAILS
+SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
EOF
#MKFS_OPTIONS -- $FULL_MKFS_OPTIONS
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 33ad0ec..87b4a3a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -38,6 +38,8 @@ imgfmt = os.environ.get('IMGFMT', 'raw')
imgproto = os.environ.get('IMGPROTO', 'file')
test_dir = os.environ.get('TEST_DIR', '/var/tmp')
+socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
+
def qemu_img(*args):
'''Run qemu-img and return the exit code'''
devnull = open('/dev/null', 'r+')
@@ -80,6 +82,12 @@ class VM(object):
'-display', 'none', '-vga', 'none']
self._num_drives = 0
+ # This can be used to add an unused monitor instance.
+ def add_monitor_telnet(self, ip, port):
+ args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port)
+ self._args.append('-monitor')
+ self._args.append(args)
+
def add_drive(self, path, opts=''):
'''Add a virtio-blk drive to the VM'''
options = ['if=virtio',
@@ -112,6 +120,21 @@ class VM(object):
self._args.append(','.join(options))
return self
+ def send_fd_scm(self, fd_file_path):
+ # In iotest.py, the qmp should always use unix socket.
+ assert self._qmp.is_scm_available()
+ bin = socket_scm_helper
+ if os.path.exists(bin) == False:
+ print "Scm help program does not present, path '%s'." % bin
+ return -1
+ fd_param = ["%s" % bin,
+ "%d" % self._qmp.get_sock_fd(),
+ "%s" % fd_file_path]
+ devnull = open('/dev/null', 'rb')
+ p = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
+ stderr=sys.stderr)
+ return p.wait()
+
def launch(self):
'''Launch the VM and establish a QMP connection'''
devnull = open('/dev/null', 'rb')
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 12/33] qemu-iotests: add tests for runtime fd passing via SCM rights
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (10 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 11/33] qemu-iotests: add infrastructure of fd passing via SCM Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 18:30 ` Eric Blake
2013-09-13 11:50 ` [Qemu-devel] [PULL 13/33] qemu-iotests: New test case in 061 Kevin Wolf
` (20 subsequent siblings)
32 siblings, 1 reply; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
This case will test whether the monitor can receive fd at runtime.
To verify better, additional monitor is created to see if qemu
can handler two monitor instances correctly.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/045 | 51 +++++++++++++++++++++++++++++++++++++++++++++-
tests/qemu-iotests/045.out | 4 ++--
2 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/045 b/tests/qemu-iotests/045
index 2b6f1af..6be8fc4 100755
--- a/tests/qemu-iotests/045
+++ b/tests/qemu-iotests/045
@@ -1,6 +1,6 @@
#!/usr/bin/env python
#
-# Tests for fdsets.
+# Tests for fdsets and getfd.
#
# Copyright (C) 2012 IBM Corp.
#
@@ -125,5 +125,54 @@ class TestFdSets(iotests.QMPTestCase):
'No file descriptor supplied via SCM_RIGHTS')
self.vm.shutdown()
+# Add fd at runtime, there are two ways: monitor related or fdset related
+class TestSCMFd(iotests.QMPTestCase):
+ def setUp(self):
+ self.vm = iotests.VM()
+ qemu_img('create', '-f', iotests.imgfmt, image0, '128K')
+ # Add an unused monitor, to verify it works fine when two monitor
+ # instances present
+ self.vm.add_monitor_telnet("0",4445)
+ self.vm.launch()
+
+ def tearDown(self):
+ self.vm.shutdown()
+ os.remove(image0)
+
+ def _send_fd_by_SCM(self):
+ ret = self.vm.send_fd_scm(image0)
+ self.assertEqual(ret, 0, 'Failed to send fd with UNIX SCM')
+
+ def test_add_fd(self):
+ self._send_fd_by_SCM()
+ result = self.vm.qmp('add-fd', fdset_id=2, opaque='image0:r')
+ self.assert_qmp(result, 'return/fdset-id', 2)
+
+ def test_getfd(self):
+ self._send_fd_by_SCM()
+ result = self.vm.qmp('getfd', fdname='image0:r')
+ self.assert_qmp(result, 'return', {})
+
+ def test_getfd_invalid_fdname(self):
+ self._send_fd_by_SCM()
+ result = self.vm.qmp('getfd', fdname='0image0:r')
+ self.assert_qmp(result, 'error/class', 'GenericError')
+ self.assert_qmp(result, 'error/desc',
+ "Parameter 'fdname' expects a name not starting with a digit")
+
+ def test_closefd(self):
+ self._send_fd_by_SCM()
+ result = self.vm.qmp('getfd', fdname='image0:r')
+ self.assert_qmp(result, 'return', {})
+ result = self.vm.qmp('closefd', fdname='image0:r')
+ self.assert_qmp(result, 'return', {})
+
+ def test_closefd_fd_not_found(self):
+ fdname = 'image0:r'
+ result = self.vm.qmp('closefd', fdname=fdname)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+ self.assert_qmp(result, 'error/desc',
+ "File descriptor named '%s' not found" % fdname)
+
if __name__ == '__main__':
iotests.main(supported_fmts=['raw'])
diff --git a/tests/qemu-iotests/045.out b/tests/qemu-iotests/045.out
index 3f8a935..e56cae0 100644
--- a/tests/qemu-iotests/045.out
+++ b/tests/qemu-iotests/045.out
@@ -1,5 +1,5 @@
-......
+...........
----------------------------------------------------------------------
-Ran 6 tests
+Ran 11 tests
OK
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PULL 12/33] qemu-iotests: add tests for runtime fd passing via SCM rights
2013-09-13 11:50 ` [Qemu-devel] [PULL 12/33] qemu-iotests: add tests for runtime fd passing via SCM rights Kevin Wolf
@ 2013-09-13 18:30 ` Eric Blake
0 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2013-09-13 18:30 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, anthony
[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]
On 09/13/2013 05:50 AM, Kevin Wolf wrote:
> From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>
> This case will test whether the monitor can receive fd at runtime.
> To verify better, additional monitor is created to see if qemu
> can handler two monitor instances correctly.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/045 | 51 +++++++++++++++++++++++++++++++++++++++++++++-
> tests/qemu-iotests/045.out | 4 ++--
> 2 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/tests/qemu-iotests/045 b/tests/qemu-iotests/045
> index 2b6f1af..6be8fc4 100755
> --- a/tests/qemu-iotests/045
> +++ b/tests/qemu-iotests/045
> @@ -1,6 +1,6 @@
> #!/usr/bin/env python
> #
> -# Tests for fdsets.
> +# Tests for fdsets and getfd.
> #
> # Copyright (C) 2012 IBM Corp.
This is a non-trivial addition; did you want to expand the copyright to
include the current year?
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 13/33] qemu-iotests: New test case in 061
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (11 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 12/33] qemu-iotests: add tests for runtime fd passing via SCM rights Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 14/33] snapshot: new function bdrv_snapshot_find_by_id_and_name() Kevin Wolf
` (19 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Add one test case for zero cluster expansion on qcow2 version downgrade
in shared L2 tables (i.e., L2 tables with a refcount > 1) and one for
zero expansion on backed clusters in shared L2 tables.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/061 | 28 ++++++++++++++++++++++++++++
tests/qemu-iotests/061.out | 27 +++++++++++++++++++++++++++
2 files changed, 55 insertions(+)
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index 86404e6..5f04bfa 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -146,6 +146,19 @@ _check_test_img
$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
echo
+echo "=== Testing zero expansion on shared L2 table ==="
+echo
+IMGOPTS="compat=1.1" _make_test_img 64M
+$QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -a foo "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+
+echo
echo "=== Testing zero expansion on backed image ==="
echo
IMGOPTS="compat=1.1" TEST_IMG="$TEST_IMG.base" _make_test_img 64M
@@ -172,6 +185,21 @@ $QEMU_IMG snapshot -a foo "$TEST_IMG"
_check_test_img
$QEMU_IO -c "read -P 0 0 64k" -c "read -P 0x2a 64k 64k" "$TEST_IMG" | _filter_qemu_io
+echo
+echo "=== Testing zero expansion on backed image with shared L2 table ==="
+echo
+IMGOPTS="compat=1.1" TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG.base" | _filter_qemu_io
+IMGOPTS="compat=1.1,backing_file=$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -a foo "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 05bd1d5..d42127f 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -312,6 +312,18 @@ No errors were found on the image.
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+=== Testing zero expansion on shared L2 table ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
=== Testing zero expansion on backed image ===
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
@@ -346,4 +358,19 @@ read 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 65536/65536 bytes at offset 65536
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing zero expansion on backed image with shared L2 table ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.base'
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
*** done
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 14/33] snapshot: new function bdrv_snapshot_find_by_id_and_name()
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (12 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 13/33] qemu-iotests: New test case in 061 Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 15/33] snapshot: distinguish id and name in snapshot delete Kevin Wolf
` (18 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
To make it clear about id and name in searching, add this API
to distinguish them. Caller can choose to search by id or name,
*errp will be set only for exception.
Some code are modified based on Pavel's patch.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/snapshot.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
include/block/snapshot.h | 6 ++++
2 files changed, 79 insertions(+)
diff --git a/block/snapshot.c b/block/snapshot.c
index 8f61cc0..a923b38 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -48,6 +48,79 @@ int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
return ret;
}
+/**
+ * Look up an internal snapshot by @id and @name.
+ * @bs: block device to search
+ * @id: unique snapshot ID, or NULL
+ * @name: snapshot name, or NULL
+ * @sn_info: location to store information on the snapshot found
+ * @errp: location to store error, will be set only for exception
+ *
+ * This function will traverse snapshot list in @bs to search the matching
+ * one, @id and @name are the matching condition:
+ * If both @id and @name are specified, find the first one with id @id and
+ * name @name.
+ * If only @id is specified, find the first one with id @id.
+ * If only @name is specified, find the first one with name @name.
+ * if none is specified, abort().
+ *
+ * Returns: true when a snapshot is found and @sn_info will be filled, false
+ * when error or not found. If all operation succeed but no matching one is
+ * found, @errp will NOT be set.
+ */
+bool bdrv_snapshot_find_by_id_and_name(BlockDriverState *bs,
+ const char *id,
+ const char *name,
+ QEMUSnapshotInfo *sn_info,
+ Error **errp)
+{
+ QEMUSnapshotInfo *sn_tab, *sn;
+ int nb_sns, i;
+ bool ret = false;
+
+ assert(id || name);
+
+ nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+ if (nb_sns < 0) {
+ error_setg_errno(errp, -nb_sns, "Failed to get a snapshot list");
+ return false;
+ } else if (nb_sns == 0) {
+ return false;
+ }
+
+ if (id && name) {
+ for (i = 0; i < nb_sns; i++) {
+ sn = &sn_tab[i];
+ if (!strcmp(sn->id_str, id) && !strcmp(sn->name, name)) {
+ *sn_info = *sn;
+ ret = true;
+ break;
+ }
+ }
+ } else if (id) {
+ for (i = 0; i < nb_sns; i++) {
+ sn = &sn_tab[i];
+ if (!strcmp(sn->id_str, id)) {
+ *sn_info = *sn;
+ ret = true;
+ break;
+ }
+ }
+ } else if (name) {
+ for (i = 0; i < nb_sns; i++) {
+ sn = &sn_tab[i];
+ if (!strcmp(sn->name, name)) {
+ *sn_info = *sn;
+ ret = true;
+ break;
+ }
+ }
+ }
+
+ g_free(sn_tab);
+ return ret;
+}
+
int bdrv_can_snapshot(BlockDriverState *bs)
{
BlockDriver *drv = bs->drv;
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index eaf61f0..9d06dc1 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -26,6 +26,7 @@
#define SNAPSHOT_H
#include "qemu-common.h"
+#include "qapi/error.h"
typedef struct QEMUSnapshotInfo {
char id_str[128]; /* unique snapshot id */
@@ -40,6 +41,11 @@ typedef struct QEMUSnapshotInfo {
int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
const char *name);
+bool bdrv_snapshot_find_by_id_and_name(BlockDriverState *bs,
+ const char *id,
+ const char *name,
+ QEMUSnapshotInfo *sn_info,
+ Error **errp);
int bdrv_can_snapshot(BlockDriverState *bs);
int bdrv_snapshot_create(BlockDriverState *bs,
QEMUSnapshotInfo *sn_info);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 15/33] snapshot: distinguish id and name in snapshot delete
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (13 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 14/33] snapshot: new function bdrv_snapshot_find_by_id_and_name() Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 18:42 ` Eric Blake
2013-09-13 11:50 ` [Qemu-devel] [PULL 16/33] qmp: add internal snapshot support in qmp_transaction Kevin Wolf
` (17 subsequent siblings)
32 siblings, 1 reply; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Snapshot creation actually already distinguish id and name since it take
a structured parameter *sn, but delete can't. Later an accurate delete
is needed in qmp_transaction abort and blockdev-snapshot-delete-sync,
so change its prototype. Also *errp is added to tip error, but return
value is kepted to let caller check what kind of error happens. Existing
caller for it are savevm, delvm and qemu-img, they are not impacted by
introducing a new function bdrv_snapshot_delete_by_id_or_name(), which
check the return value and do the operation again.
Before this patch:
For qcow2, it search id first then name to find the one to delete.
For rbd, it search name.
For sheepdog, it does nothing.
After this patch:
For qcow2, logic is the same by call it twice in caller.
For rbd, it always fails in delete with id, but still search for name
in second try, no change to user.
Some code for *errp is based on Pavel's patch.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2-snapshot.c | 55 +++++++++++++++++++++++++++++++-------------
block/qcow2.h | 5 +++-
block/rbd.c | 21 ++++++++++++++++-
block/sheepdog.c | 5 +++-
block/snapshot.c | 58 ++++++++++++++++++++++++++++++++++++++++++++---
include/block/block_int.h | 5 +++-
include/block/snapshot.h | 8 ++++++-
qemu-img.c | 11 +++++----
savevm.c | 32 ++++++++++++++------------
9 files changed, 157 insertions(+), 43 deletions(-)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index ffead08..7d14420 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -297,31 +297,47 @@ static void find_new_snapshot_id(BlockDriverState *bs,
snprintf(id_str, id_str_size, "%d", id_max + 1);
}
-static int find_snapshot_by_id(BlockDriverState *bs, const char *id_str)
+static int find_snapshot_by_id_and_name(BlockDriverState *bs,
+ const char *id,
+ const char *name)
{
BDRVQcowState *s = bs->opaque;
int i;
- for(i = 0; i < s->nb_snapshots; i++) {
- if (!strcmp(s->snapshots[i].id_str, id_str))
- return i;
+ if (id && name) {
+ for (i = 0; i < s->nb_snapshots; i++) {
+ if (!strcmp(s->snapshots[i].id_str, id) &&
+ !strcmp(s->snapshots[i].name, name)) {
+ return i;
+ }
+ }
+ } else if (id) {
+ for (i = 0; i < s->nb_snapshots; i++) {
+ if (!strcmp(s->snapshots[i].id_str, id)) {
+ return i;
+ }
+ }
+ } else if (name) {
+ for (i = 0; i < s->nb_snapshots; i++) {
+ if (!strcmp(s->snapshots[i].name, name)) {
+ return i;
+ }
+ }
}
+
return -1;
}
-static int find_snapshot_by_id_or_name(BlockDriverState *bs, const char *name)
+static int find_snapshot_by_id_or_name(BlockDriverState *bs,
+ const char *id_or_name)
{
- BDRVQcowState *s = bs->opaque;
- int i, ret;
+ int ret;
- ret = find_snapshot_by_id(bs, name);
- if (ret >= 0)
+ ret = find_snapshot_by_id_and_name(bs, id_or_name, NULL);
+ if (ret >= 0) {
return ret;
- for(i = 0; i < s->nb_snapshots; i++) {
- if (!strcmp(s->snapshots[i].name, name))
- return i;
}
- return -1;
+ return find_snapshot_by_id_and_name(bs, NULL, id_or_name);
}
/* if no id is provided, a new one is constructed */
@@ -343,7 +359,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
}
/* Check that the ID is unique */
- if (find_snapshot_by_id(bs, sn_info->id_str) >= 0) {
+ if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) {
return -EEXIST;
}
@@ -560,15 +576,19 @@ fail:
return ret;
}
-int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+int qcow2_snapshot_delete(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp)
{
BDRVQcowState *s = bs->opaque;
QCowSnapshot sn;
int snapshot_index, ret;
/* Search the snapshot */
- snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id);
+ snapshot_index = find_snapshot_by_id_and_name(bs, snapshot_id, name);
if (snapshot_index < 0) {
+ error_setg(errp, "Can't find the snapshot");
return -ENOENT;
}
sn = s->snapshots[snapshot_index];
@@ -580,6 +600,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
s->nb_snapshots--;
ret = qcow2_write_snapshots(bs);
if (ret < 0) {
+ error_setg(errp, "Failed to remove snapshot from snapshot list");
return ret;
}
@@ -597,6 +618,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
ret = qcow2_update_snapshot_refcount(bs, sn.l1_table_offset,
sn.l1_size, -1);
if (ret < 0) {
+ error_setg(errp, "Failed to free the cluster and L1 table");
return ret;
}
qcow2_free_clusters(bs, sn.l1_table_offset, sn.l1_size * sizeof(uint64_t),
@@ -605,6 +627,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
/* must update the copied flag on the current cluster offsets */
ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 0);
if (ret < 0) {
+ error_setg(errp, "Failed to update snapshot status in disk");
return ret;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index bea6ddb..c90e5d6 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -467,7 +467,10 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs);
/* qcow2-snapshot.c functions */
int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id);
-int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id);
+int qcow2_snapshot_delete(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp);
int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab);
int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name);
diff --git a/block/rbd.c b/block/rbd.c
index e798e19..b1ab80c 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -891,12 +891,31 @@ static int qemu_rbd_snap_create(BlockDriverState *bs,
}
static int qemu_rbd_snap_remove(BlockDriverState *bs,
- const char *snapshot_name)
+ const char *snapshot_id,
+ const char *snapshot_name,
+ Error **errp)
{
BDRVRBDState *s = bs->opaque;
int r;
+ if (!snapshot_name) {
+ error_setg(errp, "rbd need a valid snapshot name");
+ return -EINVAL;
+ }
+
+ /* If snapshot_id is specified, it must be equal to name, see
+ qemu_rbd_snap_list() */
+ if (snapshot_id && strcmp(snapshot_id, snapshot_name)) {
+ error_setg(errp,
+ "rbd do not support snapshot id, it should be NULL or "
+ "equal to snapshot name");
+ return -EINVAL;
+ }
+
r = rbd_snap_remove(s->image, snapshot_name);
+ if (r < 0) {
+ error_setg_errno(errp, -r, "Failed to remove the snapshot");
+ }
return r;
}
diff --git a/block/sheepdog.c b/block/sheepdog.c
index f9988d3..fe438e0 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2072,7 +2072,10 @@ out:
return ret;
}
-static int sd_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+static int sd_snapshot_delete(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp)
{
/* FIXME: Delete specified snapshot id. */
return 0;
diff --git a/block/snapshot.c b/block/snapshot.c
index a923b38..82e602f 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -182,21 +182,73 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
return -ENOTSUP;
}
-int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+/**
+ * Delete an internal snapshot by @snapshot_id and @name.
+ * @bs: block device used in the operation
+ * @snapshot_id: unique snapshot ID, or NULL
+ * @name: snapshot name, or NULL
+ * @errp: location to store error
+ *
+ * If both @snapshot_id and @name are specified, delete the first one with
+ * id @snapshot_id and name @name.
+ * If only @snapshot_id is specified, delete the first one with id
+ * @snapshot_id.
+ * If only @name is specified, delete the first one with name @name.
+ * if none is specified, return -ENINVAL.
+ *
+ * Returns: 0 on success, -errno on failure. If @bs is not inserted, return
+ * -ENOMEDIUM. If @snapshot_id and @name are both NULL, return -EINVAL. If @bs
+ * does not support internal snapshot deletion, return -ENOTSUP. If @bs does
+ * not support parameter @snapshot_id or @name, or one of them is not correctly
+ * specified, return -EINVAL. If @bs can't find one matching @id and @name,
+ * return -ENOENT. If @errp != NULL, it will always be filled with error
+ * message on failure.
+ */
+int bdrv_snapshot_delete(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp)
{
BlockDriver *drv = bs->drv;
if (!drv) {
+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
return -ENOMEDIUM;
}
+ if (!snapshot_id && !name) {
+ error_setg(errp, "snapshot_id and name are both NULL");
+ return -EINVAL;
+ }
if (drv->bdrv_snapshot_delete) {
- return drv->bdrv_snapshot_delete(bs, snapshot_id);
+ return drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
}
if (bs->file) {
- return bdrv_snapshot_delete(bs->file, snapshot_id);
+ return bdrv_snapshot_delete(bs->file, snapshot_id, name, errp);
}
+ error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+ drv->format_name, bdrv_get_device_name(bs),
+ "internal snapshot deletion");
return -ENOTSUP;
}
+void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
+ const char *id_or_name,
+ Error **errp)
+{
+ int ret;
+ Error *local_err = NULL;
+
+ ret = bdrv_snapshot_delete(bs, id_or_name, NULL, &local_err);
+ if (ret == -ENOENT || ret == -EINVAL) {
+ error_free(local_err);
+ local_err = NULL;
+ ret = bdrv_snapshot_delete(bs, NULL, id_or_name, &local_err);
+ }
+
+ if (ret < 0) {
+ error_propagate(errp, local_err);
+ }
+}
+
int bdrv_snapshot_list(BlockDriverState *bs,
QEMUSnapshotInfo **psn_info)
{
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1541777..e7d1766 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -150,7 +150,10 @@ struct BlockDriver {
QEMUSnapshotInfo *sn_info);
int (*bdrv_snapshot_goto)(BlockDriverState *bs,
const char *snapshot_id);
- int (*bdrv_snapshot_delete)(BlockDriverState *bs, const char *snapshot_id);
+ int (*bdrv_snapshot_delete)(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp);
int (*bdrv_snapshot_list)(BlockDriverState *bs,
QEMUSnapshotInfo **psn_info);
int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs,
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 9d06dc1..012bf22 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -51,7 +51,13 @@ int bdrv_snapshot_create(BlockDriverState *bs,
QEMUSnapshotInfo *sn_info);
int bdrv_snapshot_goto(BlockDriverState *bs,
const char *snapshot_id);
-int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id);
+int bdrv_snapshot_delete(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp);
+void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
+ const char *id_or_name,
+ Error **errp);
int bdrv_snapshot_list(BlockDriverState *bs,
QEMUSnapshotInfo **psn_info);
int bdrv_snapshot_load_tmp(BlockDriverState *bs,
diff --git a/qemu-img.c b/qemu-img.c
index 0cf6be2..139526f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2006,6 +2006,7 @@ static int img_snapshot(int argc, char **argv)
int action = 0;
qemu_timeval tv;
bool quiet = false;
+ Error *err = NULL;
bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
/* Parse commandline parameters */
@@ -2098,10 +2099,12 @@ static int img_snapshot(int argc, char **argv)
break;
case SNAPSHOT_DELETE:
- ret = bdrv_snapshot_delete(bs, snapshot_name);
- if (ret) {
- error_report("Could not delete snapshot '%s': %d (%s)",
- snapshot_name, ret, strerror(-ret));
+ bdrv_snapshot_delete_by_id_or_name(bs, snapshot_name, &err);
+ if (error_is_set(&err)) {
+ error_report("Could not delete snapshot '%s': (%s)",
+ snapshot_name, error_get_pretty(err));
+ error_free(err);
+ ret = 1;
}
break;
}
diff --git a/savevm.c b/savevm.c
index c536aa4..4a3c819 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2325,18 +2325,21 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
{
BlockDriverState *bs;
QEMUSnapshotInfo sn1, *snapshot = &sn1;
- int ret;
+ Error *err = NULL;
bs = NULL;
while ((bs = bdrv_next(bs))) {
if (bdrv_can_snapshot(bs) &&
bdrv_snapshot_find(bs, snapshot, name) >= 0)
{
- ret = bdrv_snapshot_delete(bs, name);
- if (ret < 0) {
+ bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
+ if (error_is_set(&err)) {
monitor_printf(mon,
- "Error while deleting snapshot on '%s'\n",
- bdrv_get_device_name(bs));
+ "Error while deleting snapshot on device '%s':"
+ " %s\n",
+ bdrv_get_device_name(bs),
+ error_get_pretty(err));
+ error_free(err);
return -1;
}
}
@@ -2550,7 +2553,7 @@ int load_vmstate(const char *name)
void do_delvm(Monitor *mon, const QDict *qdict)
{
BlockDriverState *bs, *bs1;
- int ret;
+ Error *err = NULL;
const char *name = qdict_get_str(qdict, "name");
bs = find_vmstate_bs();
@@ -2562,15 +2565,14 @@ void do_delvm(Monitor *mon, const QDict *qdict)
bs1 = NULL;
while ((bs1 = bdrv_next(bs1))) {
if (bdrv_can_snapshot(bs1)) {
- ret = bdrv_snapshot_delete(bs1, name);
- if (ret < 0) {
- if (ret == -ENOTSUP)
- monitor_printf(mon,
- "Snapshots not supported on device '%s'\n",
- bdrv_get_device_name(bs1));
- else
- monitor_printf(mon, "Error %d while deleting snapshot on "
- "'%s'\n", ret, bdrv_get_device_name(bs1));
+ bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
+ if (error_is_set(&err)) {
+ monitor_printf(mon,
+ "Error while deleting snapshot on device '%s':"
+ " %s\n",
+ bdrv_get_device_name(bs),
+ error_get_pretty(err));
+ error_free(err);
}
}
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PULL 15/33] snapshot: distinguish id and name in snapshot delete
2013-09-13 11:50 ` [Qemu-devel] [PULL 15/33] snapshot: distinguish id and name in snapshot delete Kevin Wolf
@ 2013-09-13 18:42 ` Eric Blake
0 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2013-09-13 18:42 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, anthony
[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]
On 09/13/2013 05:50 AM, Kevin Wolf wrote:
> From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>
> Snapshot creation actually already distinguish id and name since it take
> a structured parameter *sn, but delete can't. Later an accurate delete
> is needed in qmp_transaction abort and blockdev-snapshot-delete-sync,
> so change its prototype. Also *errp is added to tip error, but return
> value is kepted to let caller check what kind of error happens. Existing
s/kepted/kept/
> caller for it are savevm, delvm and qemu-img, they are not impacted by
> introducing a new function bdrv_snapshot_delete_by_id_or_name(), which
> check the return value and do the operation again.
>
> Before this patch:
> For qcow2, it search id first then name to find the one to delete.
> For rbd, it search name.
> For sheepdog, it does nothing.
>
> After this patch:
> For qcow2, logic is the same by call it twice in caller.
> For rbd, it always fails in delete with id, but still search for name
> in second try, no change to user.
>
> Some code for *errp is based on Pavel's patch.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>
> static int qemu_rbd_snap_remove(BlockDriverState *bs,
> - const char *snapshot_name)
> + const char *snapshot_id,
> + const char *snapshot_name,
> + Error **errp)
> {
> BDRVRBDState *s = bs->opaque;
> int r;
>
> + if (!snapshot_name) {
> + error_setg(errp, "rbd need a valid snapshot name");
s/need/needs/
> + return -EINVAL;
> + }
> +
> + /* If snapshot_id is specified, it must be equal to name, see
> + qemu_rbd_snap_list() */
> + if (snapshot_id && strcmp(snapshot_id, snapshot_name)) {
> + error_setg(errp,
> + "rbd do not support snapshot id, it should be NULL or "
s/do/does/
> -int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
> +/**
> + * Delete an internal snapshot by @snapshot_id and @name.
> + * @bs: block device used in the operation
> + * @snapshot_id: unique snapshot ID, or NULL
> + * @name: snapshot name, or NULL
> + * @errp: location to store error
> + *
> + * If both @snapshot_id and @name are specified, delete the first one with
> + * id @snapshot_id and name @name.
> + * If only @snapshot_id is specified, delete the first one with id
> + * @snapshot_id.
> + * If only @name is specified, delete the first one with name @name.
> + * if none is specified, return -ENINVAL.
s/ENINVAL/EINVAL/
[Given that this is already in the pull request phase, I'm fine if the
typo in the commit message stays, and if the remaining typos are cleaned
up in a separate followup patch, rather than delaying this series any
longer - my fault for not reviewing sooner]
--
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: 621 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PULL 16/33] qmp: add internal snapshot support in qmp_transaction
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (14 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 15/33] snapshot: distinguish id and name in snapshot delete Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 17/33] qmp: add interface blockdev-snapshot-internal-sync Kevin Wolf
` (16 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Unlike savevm, the qmp_transaction interface will not generate
snapshot name automatically, saving trouble to return information
of the new created snapshot.
Although qcow2 support storing multiple snapshots with same name
but different ID, here it will fail when an snapshot with that name
already exist before the operation. Format such as rbd do not support
ID at all, and in most case, it means trouble to user when he faces
multiple snapshots with same name, so ban that case. Request with
empty name will be rejected.
Snapshot ID can't be specified in this interface.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qapi-schema.json | 19 ++++++++-
qmp-commands.hx | 34 ++++++++++++----
3 files changed, 160 insertions(+), 9 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 07dac05..0fd30e2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -889,6 +889,117 @@ struct BlkTransactionState {
QSIMPLEQ_ENTRY(BlkTransactionState) entry;
};
+/* internal snapshot private data */
+typedef struct InternalSnapshotState {
+ BlkTransactionState common;
+ BlockDriverState *bs;
+ QEMUSnapshotInfo sn;
+} InternalSnapshotState;
+
+static void internal_snapshot_prepare(BlkTransactionState *common,
+ Error **errp)
+{
+ const char *device;
+ const char *name;
+ BlockDriverState *bs;
+ QEMUSnapshotInfo old_sn, *sn;
+ bool ret;
+ qemu_timeval tv;
+ BlockdevSnapshotInternal *internal;
+ InternalSnapshotState *state;
+ int ret1;
+
+ g_assert(common->action->kind ==
+ TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC);
+ internal = common->action->blockdev_snapshot_internal_sync;
+ state = DO_UPCAST(InternalSnapshotState, common, common);
+
+ /* 1. parse input */
+ device = internal->device;
+ name = internal->name;
+
+ /* 2. check for validation */
+ bs = bdrv_find(device);
+ if (!bs) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+
+ if (!bdrv_is_inserted(bs)) {
+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+ return;
+ }
+
+ if (bdrv_is_read_only(bs)) {
+ error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
+ return;
+ }
+
+ if (!bdrv_can_snapshot(bs)) {
+ error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+ bs->drv->format_name, device, "internal snapshot");
+ return;
+ }
+
+ if (!strlen(name)) {
+ error_setg(errp, "Name is empty");
+ return;
+ }
+
+ /* check whether a snapshot with name exist */
+ ret = bdrv_snapshot_find_by_id_and_name(bs, NULL, name, &old_sn, errp);
+ if (error_is_set(errp)) {
+ return;
+ } else if (ret) {
+ error_setg(errp,
+ "Snapshot with name '%s' already exists on device '%s'",
+ name, device);
+ return;
+ }
+
+ /* 3. take the snapshot */
+ sn = &state->sn;
+ pstrcpy(sn->name, sizeof(sn->name), name);
+ qemu_gettimeofday(&tv);
+ sn->date_sec = tv.tv_sec;
+ sn->date_nsec = tv.tv_usec * 1000;
+ sn->vm_clock_nsec = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+
+ ret1 = bdrv_snapshot_create(bs, sn);
+ if (ret1 < 0) {
+ error_setg_errno(errp, -ret1,
+ "Failed to create snapshot '%s' on device '%s'",
+ name, device);
+ return;
+ }
+
+ /* 4. succeed, mark a snapshot is created */
+ state->bs = bs;
+}
+
+static void internal_snapshot_abort(BlkTransactionState *common)
+{
+ InternalSnapshotState *state =
+ DO_UPCAST(InternalSnapshotState, common, common);
+ BlockDriverState *bs = state->bs;
+ QEMUSnapshotInfo *sn = &state->sn;
+ Error *local_error = NULL;
+
+ if (!bs) {
+ return;
+ }
+
+ if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, &local_error) < 0) {
+ error_report("Failed to delete snapshot with id '%s' and name '%s' on "
+ "device '%s' in abort: %s",
+ sn->id_str,
+ sn->name,
+ bdrv_get_device_name(bs),
+ error_get_pretty(local_error));
+ error_free(local_error);
+ }
+}
+
/* external snapshot private data */
typedef struct ExternalSnapshotState {
BlkTransactionState common;
@@ -1072,6 +1183,11 @@ static const BdrvActionOps actions[] = {
.prepare = abort_prepare,
.commit = abort_commit,
},
+ [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC] = {
+ .instance_size = sizeof(InternalSnapshotState),
+ .prepare = internal_snapshot_prepare,
+ .abort = internal_snapshot_abort,
+ },
};
/*
diff --git a/qapi-schema.json b/qapi-schema.json
index 2b2c8bc..77bbbf5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1686,6 +1686,22 @@
'*mode': 'NewImageMode' } }
##
+# @BlockdevSnapshotInternal
+#
+# @device: the name of the device to generate the snapshot from
+#
+# @name: the name of the internal snapshot to be created
+#
+# Notes: In transaction, if @name is empty, or any snapshot matching @name
+# exists, the operation will fail. Only some image formats support it,
+# for example, qcow2, rbd, and sheepdog.
+#
+# Since: 1.7
+##
+{ 'type': 'BlockdevSnapshotInternal',
+ 'data': { 'device': 'str', 'name': 'str' } }
+
+##
# @DriveBackup
#
# @device: the name of the device which should be copied.
@@ -1747,7 +1763,8 @@
'data': {
'blockdev-snapshot-sync': 'BlockdevSnapshot',
'drive-backup': 'DriveBackup',
- 'abort': 'Abort'
+ 'abort': 'Abort',
+ 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
} }
##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 008cad9..6670192 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1001,14 +1001,15 @@ SQMP
transaction
-----------
-Atomically operate on one or more block devices. The only supported
-operation for now is snapshotting. If there is any failure performing
-any of the operations, all snapshots for the group are abandoned, and
-the original disks pre-snapshot attempt are used.
+Atomically operate on one or more block devices. The only supported operations
+for now are drive-backup, internal and external snapshotting. A list of
+dictionaries is accepted, that contains the actions to be performed.
+If there is any failure performing any of the operations, all operations
+for the group are abandoned.
-A list of dictionaries is accepted, that contains the actions to be performed.
-For snapshots this is the device, the file to use for the new snapshot,
-and the format. The default format, if not specified, is qcow2.
+For external snapshots, the dictionary contains the device, the file to use for
+the new snapshot, and the format. The default format, if not specified, is
+qcow2.
Each new snapshot defaults to being created by QEMU (wiping any
contents if the file already exists), but it is also possible to reuse
@@ -1017,6 +1018,17 @@ the new image file has the same contents as the current one; QEMU cannot
perform any meaningful check. Typically this is achieved by using the
current image file as the backing file for the new image.
+On failure, the original disks pre-snapshot attempt will be used.
+
+For internal snapshots, the dictionary contains the device and the snapshot's
+name. If an internal snapshot matching name already exists, the request will
+be rejected. Only some image formats support it, for example, qcow2, rbd,
+and sheepdog.
+
+On failure, qemu will try delete the newly created internal snapshot in the
+transaction. When an I/O error occurs during deletion, the user needs to fix
+it later with qemu-img or other command.
+
Arguments:
actions array:
@@ -1029,6 +1041,9 @@ actions array:
- "format": format of new image (json-string, optional)
- "mode": whether and how QEMU should create the snapshot file
(NewImageMode, optional, default "absolute-paths")
+ When "type" is "blockdev-snapshot-internal-sync":
+ - "device": device name to snapshot (json-string)
+ - "name": name of the new snapshot (json-string)
Example:
@@ -1040,7 +1055,10 @@ Example:
{ 'type': 'blockdev-snapshot-sync', 'data' : { "device": "ide-hd1",
"snapshot-file": "/some/place/my-image2",
"mode": "existing",
- "format": "qcow2" } } ] } }
+ "format": "qcow2" } },
+ { 'type': 'blockdev-snapshot-internal-sync', 'data' : {
+ "device": "ide-hd2",
+ "name": "snapshot0" } } ] } }
<- { "return": {} }
EQMP
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 17/33] qmp: add interface blockdev-snapshot-internal-sync
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (15 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 16/33] qmp: add internal snapshot support in qmp_transaction Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 18/33] qmp: add interface blockdev-snapshot-delete-internal-sync Kevin Wolf
` (15 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Snapshot ID can't be specified in this interface.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 13 +++++++++++++
qapi-schema.json | 20 ++++++++++++++++++++
qmp-commands.hx | 29 +++++++++++++++++++++++++++++
3 files changed, 62 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index 0fd30e2..0f4a7b5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -858,6 +858,19 @@ void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
&snapshot, errp);
}
+void qmp_blockdev_snapshot_internal_sync(const char *device,
+ const char *name,
+ Error **errp)
+{
+ BlockdevSnapshotInternal snapshot = {
+ .device = (char *) device,
+ .name = (char *) name
+ };
+
+ blockdev_do_action(TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC,
+ &snapshot, errp);
+}
+
/* New and old BlockDriverState structs for group snapshots */
diff --git a/qapi-schema.json b/qapi-schema.json
index 77bbbf5..43faf91 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1805,6 +1805,26 @@
'data': 'BlockdevSnapshot' }
##
+# @blockdev-snapshot-internal-sync
+#
+# Synchronously take an internal snapshot of a block device, when the format
+# of the image used supports it.
+#
+# For the arguments, see the documentation of BlockdevSnapshotInternal.
+#
+# Returns: nothing on success
+# If @device is not a valid block device, DeviceNotFound
+# If any snapshot matching @name exists, or @name is empty,
+# GenericError
+# If the format of the image used does not support it,
+# BlockFormatFeatureNotSupported
+#
+# Since 1.7
+##
+{ 'command': 'blockdev-snapshot-internal-sync',
+ 'data': 'BlockdevSnapshotInternal' }
+
+##
# @human-monitor-command:
#
# Execute a command on the human monitor and return the output.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6670192..5c9ddef 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1098,6 +1098,35 @@ Example:
EQMP
{
+ .name = "blockdev-snapshot-internal-sync",
+ .args_type = "device:B,name:s",
+ .mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_internal_sync,
+ },
+
+SQMP
+blockdev-snapshot-internal-sync
+-------------------------------
+
+Synchronously take an internal snapshot of a block device when the format of
+image used supports it. If the name is an empty string, or a snapshot with
+name already exists, the operation will fail.
+
+Arguments:
+
+- "device": device name to snapshot (json-string)
+- "name": name of the new snapshot (json-string)
+
+Example:
+
+-> { "execute": "blockdev-snapshot-internal-sync",
+ "arguments": { "device": "ide-hd0",
+ "name": "snapshot0" }
+ }
+<- { "return": {} }
+
+EQMP
+
+ {
.name = "drive-mirror",
.args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
"on-source-error:s?,on-target-error:s?,"
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 18/33] qmp: add interface blockdev-snapshot-delete-internal-sync
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (16 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 17/33] qmp: add interface blockdev-snapshot-internal-sync Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 19/33] hmp: add interface hmp_snapshot_blkdev_internal Kevin Wolf
` (14 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
This interface use id and name as optional parameters, to handle the
case that one image contain multiple snapshots with same name which
may be '', but with different id.
Adding parameter id is for historical compatiability reason, and
that case is not possible in qemu's new interface for internal
snapshot at block device level, but still possible in qemu-img.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
include/qemu-common.h | 3 +++
qapi-schema.json | 27 +++++++++++++++++++++++
qmp-commands.hx | 41 ++++++++++++++++++++++++++++++++++
4 files changed, 132 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index 0f4a7b5..84d55c1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -871,6 +871,67 @@ void qmp_blockdev_snapshot_internal_sync(const char *device,
&snapshot, errp);
}
+SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device,
+ bool has_id,
+ const char *id,
+ bool has_name,
+ const char *name,
+ Error **errp)
+{
+ BlockDriverState *bs = bdrv_find(device);
+ QEMUSnapshotInfo sn;
+ Error *local_err = NULL;
+ SnapshotInfo *info = NULL;
+ int ret;
+
+ if (!bs) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return NULL;
+ }
+
+ if (!has_id) {
+ id = NULL;
+ }
+
+ if (!has_name) {
+ name = NULL;
+ }
+
+ if (!id && !name) {
+ error_setg(errp, "Name or id must be provided");
+ return NULL;
+ }
+
+ ret = bdrv_snapshot_find_by_id_and_name(bs, id, name, &sn, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ return NULL;
+ }
+ if (!ret) {
+ error_setg(errp,
+ "Snapshot with id '%s' and name '%s' does not exist on "
+ "device '%s'",
+ STR_OR_NULL(id), STR_OR_NULL(name), device);
+ return NULL;
+ }
+
+ bdrv_snapshot_delete(bs, id, name, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ return NULL;
+ }
+
+ info = g_malloc0(sizeof(SnapshotInfo));
+ info->id = g_strdup(sn.id_str);
+ info->name = g_strdup(sn.name);
+ info->date_nsec = sn.date_nsec;
+ info->date_sec = sn.date_sec;
+ info->vm_state_size = sn.vm_state_size;
+ info->vm_clock_nsec = sn.vm_clock_nsec % 1000000000;
+ info->vm_clock_sec = sn.vm_clock_nsec / 1000000000;
+
+ return info;
+}
/* New and old BlockDriverState structs for group snapshots */
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 6948bb9..5054836 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -191,6 +191,9 @@ int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
int64_t strtosz_suffix_unit(const char *nptr, char **end,
const char default_suffix, int64_t unit);
+/* used to print char* safely */
+#define STR_OR_NULL(str) ((str) ? (str) : "null")
+
/* path.c */
void init_paths(const char *prefix);
const char *path(const char *pathname);
diff --git a/qapi-schema.json b/qapi-schema.json
index 43faf91..145eca8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1825,6 +1825,33 @@
'data': 'BlockdevSnapshotInternal' }
##
+# @blockdev-snapshot-delete-internal-sync
+#
+# Synchronously delete an internal snapshot of a block device, when the format
+# of the image used support it. The snapshot is identified by name or id or
+# both. One of the name or id is required. Return SnapshotInfo for the
+# successfully deleted snapshot.
+#
+# @device: the name of the device to delete the snapshot from
+#
+# @id: optional the snapshot's ID to be deleted
+#
+# @name: optional the snapshot's name to be deleted
+#
+# Returns: SnapshotInfo on success
+# If @device is not a valid block device, DeviceNotFound
+# If snapshot not found, GenericError
+# If the format of the image used does not support it,
+# BlockFormatFeatureNotSupported
+# If @id and @name are both not specified, GenericError
+#
+# Since 1.7
+##
+{ 'command': 'blockdev-snapshot-delete-internal-sync',
+ 'data': { 'device': 'str', '*id': 'str', '*name': 'str'},
+ 'returns': 'SnapshotInfo' }
+
+##
# @human-monitor-command:
#
# Execute a command on the human monitor and return the output.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5c9ddef..b17c46e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1127,6 +1127,47 @@ Example:
EQMP
{
+ .name = "blockdev-snapshot-delete-internal-sync",
+ .args_type = "device:B,id:s?,name:s?",
+ .mhandler.cmd_new =
+ qmp_marshal_input_blockdev_snapshot_delete_internal_sync,
+ },
+
+SQMP
+blockdev-snapshot-delete-internal-sync
+--------------------------------------
+
+Synchronously delete an internal snapshot of a block device when the format of
+image used supports it. The snapshot is identified by name or id or both. One
+of name or id is required. If the snapshot is not found, the operation will
+fail.
+
+Arguments:
+
+- "device": device name (json-string)
+- "id": ID of the snapshot (json-string, optional)
+- "name": name of the snapshot (json-string, optional)
+
+Example:
+
+-> { "execute": "blockdev-snapshot-delete-internal-sync",
+ "arguments": { "device": "ide-hd0",
+ "name": "snapshot0" }
+ }
+<- { "return": {
+ "id": "1",
+ "name": "snapshot0",
+ "vm-state-size": 0,
+ "date-sec": 1000012,
+ "date-nsec": 10,
+ "vm-clock-sec": 100,
+ "vm-clock-nsec": 20
+ }
+ }
+
+EQMP
+
+ {
.name = "drive-mirror",
.args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
"on-source-error:s?,on-target-error:s?,"
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 19/33] hmp: add interface hmp_snapshot_blkdev_internal
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (17 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 18/33] qmp: add interface blockdev-snapshot-delete-internal-sync Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 20/33] hmp: add interface hmp_snapshot_delete_blkdev_internal Kevin Wolf
` (13 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hmp-commands.hx | 19 +++++++++++++++++--
hmp.c | 10 ++++++++++
hmp.h | 1 +
3 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 65b7f60..f19a914 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1023,8 +1023,7 @@ ETEXI
"of device. If a new image file is specified, the\n\t\t\t"
"new image file will become the new root image.\n\t\t\t"
"If format is specified, the snapshot file will\n\t\t\t"
- "be created in that format. Otherwise the\n\t\t\t"
- "snapshot will be internal! (currently unsupported).\n\t\t\t"
+ "be created in that format.\n\t\t\t"
"The default format is qcow2. The -n flag requests QEMU\n\t\t\t"
"to reuse the image found in new-image-file, instead of\n\t\t\t"
"recreating it from scratch.",
@@ -1038,6 +1037,22 @@ Snapshot device, using snapshot file as target if provided
ETEXI
{
+ .name = "snapshot_blkdev_internal",
+ .args_type = "device:B,name:s",
+ .params = "device name",
+ .help = "take an internal snapshot of device.\n\t\t\t"
+ "The format of the image used by device must\n\t\t\t"
+ "support it, such as qcow2.\n\t\t\t",
+ .mhandler.cmd = hmp_snapshot_blkdev_internal,
+ },
+
+STEXI
+@item snapshot_blkdev_internal
+@findex snapshot_blkdev_internal
+Take an internal snapshot on device if it support
+ETEXI
+
+ {
.name = "drive_mirror",
.args_type = "reuse:-n,full:-f,device:B,target:s,format:s?",
.params = "[-n] [-f] device target [format]",
diff --git a/hmp.c b/hmp.c
index b4a6422..568bb91 100644
--- a/hmp.c
+++ b/hmp.c
@@ -978,6 +978,16 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &errp);
}
+void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict)
+{
+ const char *device = qdict_get_str(qdict, "device");
+ const char *name = qdict_get_str(qdict, "name");
+ Error *errp = NULL;
+
+ qmp_blockdev_snapshot_internal_sync(device, name, &errp);
+ hmp_handle_error(mon, &errp);
+}
+
void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
{
qmp_migrate_cancel(NULL);
diff --git a/hmp.h b/hmp.h
index 6c3bdcd..cc1ca58 100644
--- a/hmp.h
+++ b/hmp.h
@@ -54,6 +54,7 @@ void hmp_block_passwd(Monitor *mon, const QDict *qdict);
void hmp_balloon(Monitor *mon, const QDict *qdict);
void hmp_block_resize(Monitor *mon, const QDict *qdict);
void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
+void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict);
void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
void hmp_drive_backup(Monitor *mon, const QDict *qdict);
void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 20/33] hmp: add interface hmp_snapshot_delete_blkdev_internal
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (18 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 19/33] hmp: add interface hmp_snapshot_blkdev_internal Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 21/33] qemu-iotests: add 057 internal snapshot for block device test case Kevin Wolf
` (12 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
It is hard to make both id and name optional in hmp console as qmp
interface, so this interface require user to specify name.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hmp-commands.hx | 18 ++++++++++++++++++
hmp.c | 12 ++++++++++++
hmp.h | 1 +
3 files changed, 31 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index f19a914..caae5ad 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1053,6 +1053,24 @@ Take an internal snapshot on device if it support
ETEXI
{
+ .name = "snapshot_delete_blkdev_internal",
+ .args_type = "device:B,name:s,id:s?",
+ .params = "device name [id]",
+ .help = "delete an internal snapshot of device.\n\t\t\t"
+ "If id is specified, qemu will try delete\n\t\t\t"
+ "the snapshot matching both id and name.\n\t\t\t"
+ "The format of the image used by device must\n\t\t\t"
+ "support it, such as qcow2.\n\t\t\t",
+ .mhandler.cmd = hmp_snapshot_delete_blkdev_internal,
+ },
+
+STEXI
+@item snapshot_delete_blkdev_internal
+@findex snapshot_delete_blkdev_internal
+Delete an internal snapshot on device if it support
+ETEXI
+
+ {
.name = "drive_mirror",
.args_type = "reuse:-n,full:-f,device:B,target:s,format:s?",
.params = "[-n] [-f] device target [format]",
diff --git a/hmp.c b/hmp.c
index 568bb91..2a90295 100644
--- a/hmp.c
+++ b/hmp.c
@@ -988,6 +988,18 @@ void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &errp);
}
+void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict)
+{
+ const char *device = qdict_get_str(qdict, "device");
+ const char *name = qdict_get_str(qdict, "name");
+ const char *id = qdict_get_try_str(qdict, "id");
+ Error *errp = NULL;
+
+ qmp_blockdev_snapshot_delete_internal_sync(device, !!id, id,
+ true, name, &errp);
+ hmp_handle_error(mon, &errp);
+}
+
void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
{
qmp_migrate_cancel(NULL);
diff --git a/hmp.h b/hmp.h
index cc1ca58..54cf71f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -55,6 +55,7 @@ void hmp_balloon(Monitor *mon, const QDict *qdict);
void hmp_block_resize(Monitor *mon, const QDict *qdict);
void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict);
+void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict);
void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
void hmp_drive_backup(Monitor *mon, const QDict *qdict);
void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 21/33] qemu-iotests: add 057 internal snapshot for block device test case
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (19 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 20/33] hmp: add interface hmp_snapshot_delete_blkdev_internal Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 22/33] bdrv: Use "Error" for opening images Kevin Wolf
` (11 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Create in transaction and deletion in single command will be tested.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/057 | 259 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/057.out | 5 +
tests/qemu-iotests/group | 1 +
3 files changed, 265 insertions(+)
create mode 100755 tests/qemu-iotests/057
create mode 100644 tests/qemu-iotests/057.out
diff --git a/tests/qemu-iotests/057 b/tests/qemu-iotests/057
new file mode 100755
index 0000000..9cdd582
--- /dev/null
+++ b/tests/qemu-iotests/057
@@ -0,0 +1,259 @@
+#!/usr/bin/env python
+#
+# Tests for internal snapshot.
+#
+# Copyright (C) 2013 IBM, Inc.
+#
+# Based on 055.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+import time
+import os
+import iotests
+from iotests import qemu_img, qemu_io
+
+test_drv_base_name = 'drive'
+
+class ImageSnapshotTestCase(iotests.QMPTestCase):
+ image_len = 120 * 1024 * 1024 # MB
+
+ def __init__(self, *args):
+ self.expect = []
+ super(ImageSnapshotTestCase, self).__init__(*args)
+
+ def _setUp(self, test_img_base_name, image_num):
+ self.vm = iotests.VM()
+ for i in range(0, image_num):
+ filename = '%s%d' % (test_img_base_name, i)
+ img = os.path.join(iotests.test_dir, filename)
+ device = '%s%d' % (test_drv_base_name, i)
+ qemu_img('create', '-f', iotests.imgfmt, img, str(self.image_len))
+ self.vm.add_drive(img)
+ self.expect.append({'image': img, 'device': device,
+ 'snapshots': [],
+ 'snapshots_name_counter': 0})
+ self.vm.launch()
+
+ def tearDown(self):
+ self.vm.shutdown()
+ for dev_expect in self.expect:
+ os.remove(dev_expect['image'])
+
+ def createSnapshotInTransaction(self, snapshot_num, abort = False):
+ actions = []
+ for dev_expect in self.expect:
+ num = dev_expect['snapshots_name_counter']
+ for j in range(0, snapshot_num):
+ name = '%s_sn%d' % (dev_expect['device'], num)
+ num = num + 1
+ if abort == False:
+ dev_expect['snapshots'].append({'name': name})
+ dev_expect['snapshots_name_counter'] = num
+ actions.append({
+ 'type': 'blockdev-snapshot-internal-sync',
+ 'data': { 'device': dev_expect['device'],
+ 'name': name },
+ })
+
+ if abort == True:
+ actions.append({
+ 'type': 'abort',
+ 'data': {},
+ })
+
+ result = self.vm.qmp('transaction', actions = actions)
+
+ if abort == True:
+ self.assert_qmp(result, 'error/class', 'GenericError')
+ else:
+ self.assert_qmp(result, 'return', {})
+
+ def verifySnapshotInfo(self):
+ result = self.vm.qmp('query-block')
+
+ # Verify each expected result
+ for dev_expect in self.expect:
+ # 1. Find the returned image value and snapshot info
+ image_result = None
+ for device in result['return']:
+ if device['device'] == dev_expect['device']:
+ image_result = device['inserted']['image']
+ break
+ self.assertTrue(image_result != None)
+ # Do not consider zero snapshot case now
+ sn_list_result = image_result['snapshots']
+ sn_list_expect = dev_expect['snapshots']
+
+ # 2. Verify it with expect
+ self.assertTrue(len(sn_list_result) == len(sn_list_expect))
+
+ for sn_expect in sn_list_expect:
+ sn_result = None
+ for sn in sn_list_result:
+ if sn_expect['name'] == sn['name']:
+ sn_result = sn
+ break
+ self.assertTrue(sn_result != None)
+ # Fill in the detail info
+ sn_expect.update(sn_result)
+
+ def deleteSnapshot(self, device, id = None, name = None):
+ sn_list_expect = None
+ sn_expect = None
+
+ self.assertTrue(id != None or name != None)
+
+ # Fill in the detail info include ID
+ self.verifySnapshotInfo()
+
+ #find the expected snapshot list
+ for dev_expect in self.expect:
+ if dev_expect['device'] == device:
+ sn_list_expect = dev_expect['snapshots']
+ break
+ self.assertTrue(sn_list_expect != None)
+
+ if id != None and name != None:
+ for sn in sn_list_expect:
+ if sn['id'] == id and sn['name'] == name:
+ sn_expect = sn
+ result = \
+ self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+ device = device,
+ id = id,
+ name = name)
+ break
+ elif id != None:
+ for sn in sn_list_expect:
+ if sn['id'] == id:
+ sn_expect = sn
+ result = \
+ self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+ device = device,
+ id = id)
+ break
+ else:
+ for sn in sn_list_expect:
+ if sn['name'] == name:
+ sn_expect = sn
+ result = \
+ self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+ device = device,
+ name = name)
+ break
+
+ self.assertTrue(sn_expect != None)
+
+ self.assert_qmp(result, 'return', sn_expect)
+ sn_list_expect.remove(sn_expect)
+
+class TestSingleTransaction(ImageSnapshotTestCase):
+ def setUp(self):
+ self._setUp('test_a.img', 1)
+
+ def test_create(self):
+ self.createSnapshotInTransaction(1)
+ self.verifySnapshotInfo()
+
+ def test_error_name_empty(self):
+ actions = [{'type': 'blockdev-snapshot-internal-sync',
+ 'data': { 'device': self.expect[0]['device'],
+ 'name': '' },
+ }]
+ result = self.vm.qmp('transaction', actions = actions)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_error_device(self):
+ actions = [{'type': 'blockdev-snapshot-internal-sync',
+ 'data': { 'device': 'drive_error',
+ 'name': 'a' },
+ }]
+ result = self.vm.qmp('transaction', actions = actions)
+ self.assert_qmp(result, 'error/class', 'DeviceNotFound')
+
+ def test_error_exist(self):
+ self.createSnapshotInTransaction(1)
+ self.verifySnapshotInfo()
+ actions = [{'type': 'blockdev-snapshot-internal-sync',
+ 'data': { 'device': self.expect[0]['device'],
+ 'name': self.expect[0]['snapshots'][0] },
+ }]
+ result = self.vm.qmp('transaction', actions = actions)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+class TestMultipleTransaction(ImageSnapshotTestCase):
+ def setUp(self):
+ self._setUp('test_b.img', 2)
+
+ def test_create(self):
+ self.createSnapshotInTransaction(3)
+ self.verifySnapshotInfo()
+
+ def test_abort(self):
+ self.createSnapshotInTransaction(2)
+ self.verifySnapshotInfo()
+ self.createSnapshotInTransaction(3, abort = True)
+ self.verifySnapshotInfo()
+
+class TestSnapshotDelete(ImageSnapshotTestCase):
+ def setUp(self):
+ self._setUp('test_c.img', 1)
+
+ def test_delete_with_id(self):
+ self.createSnapshotInTransaction(2)
+ self.verifySnapshotInfo()
+ self.deleteSnapshot(self.expect[0]['device'],
+ id = self.expect[0]['snapshots'][0]['id'])
+ self.verifySnapshotInfo()
+
+ def test_delete_with_name(self):
+ self.createSnapshotInTransaction(3)
+ self.verifySnapshotInfo()
+ self.deleteSnapshot(self.expect[0]['device'],
+ name = self.expect[0]['snapshots'][1]['name'])
+ self.verifySnapshotInfo()
+
+ def test_delete_with_id_and_name(self):
+ self.createSnapshotInTransaction(4)
+ self.verifySnapshotInfo()
+ self.deleteSnapshot(self.expect[0]['device'],
+ id = self.expect[0]['snapshots'][2]['id'],
+ name = self.expect[0]['snapshots'][2]['name'])
+ self.verifySnapshotInfo()
+
+
+ def test_error_device(self):
+ result = self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+ device = 'drive_error',
+ id = '0')
+ self.assert_qmp(result, 'error/class', 'DeviceNotFound')
+
+ def test_error_no_id_and_name(self):
+ result = self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+ device = self.expect[0]['device'])
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_error_snapshot_not_exist(self):
+ self.createSnapshotInTransaction(2)
+ self.verifySnapshotInfo()
+ result = self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+ device = self.expect[0]['device'],
+ id = self.expect[0]['snapshots'][0]['id'],
+ name = self.expect[0]['snapshots'][1]['name'])
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+if __name__ == '__main__':
+ iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/057.out b/tests/qemu-iotests/057.out
new file mode 100644
index 0000000..281b69e
--- /dev/null
+++ b/tests/qemu-iotests/057.out
@@ -0,0 +1,5 @@
+............
+----------------------------------------------------------------------
+Ran 12 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 8012828..1ad02e5 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -63,6 +63,7 @@
054 rw auto
055 rw auto
056 rw auto backing
+057 rw auto
059 rw auto
060 rw auto
061 rw auto
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 22/33] bdrv: Use "Error" for opening images
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (20 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 21/33] qemu-iotests: add 057 internal snapshot for block device test case Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 23/33] bdrv: Use "Error" for creating images Kevin Wolf
` (10 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Add an Error ** parameter to BlockDriver.bdrv_open and
BlockDriver.bdrv_file_open to allow more specific error messages.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 4 ++--
block/blkdebug.c | 3 ++-
block/blkverify.c | 3 ++-
block/bochs.c | 3 ++-
block/cloop.c | 3 ++-
block/cow.c | 3 ++-
block/curl.c | 3 ++-
block/dmg.c | 3 ++-
block/gluster.c | 2 +-
block/iscsi.c | 5 +++--
block/nbd.c | 3 ++-
block/parallels.c | 3 ++-
block/qcow.c | 3 ++-
block/qcow2.c | 5 +++--
block/qed.c | 5 +++--
block/raw-posix.c | 12 ++++++++----
block/raw-win32.c | 6 ++++--
block/raw_bsd.c | 3 ++-
block/rbd.c | 3 ++-
block/sheepdog.c | 3 ++-
block/snapshot.c | 2 +-
block/ssh.c | 3 ++-
block/vdi.c | 3 ++-
block/vhdx.c | 3 ++-
block/vmdk.c | 3 ++-
block/vpc.c | 3 ++-
block/vvfat.c | 3 ++-
include/block/block_int.h | 6 ++++--
28 files changed, 67 insertions(+), 37 deletions(-)
diff --git a/block.c b/block.c
index b81d1e2..89098bc 100644
--- a/block.c
+++ b/block.c
@@ -761,7 +761,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
if (drv->bdrv_file_open) {
assert(file == NULL);
assert(drv->bdrv_parse_filename || filename != NULL);
- ret = drv->bdrv_file_open(bs, options, open_flags);
+ ret = drv->bdrv_file_open(bs, options, open_flags, NULL);
} else {
if (file == NULL) {
qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
@@ -771,7 +771,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
goto free_and_fail;
}
bs->file = file;
- ret = drv->bdrv_open(bs, options, open_flags);
+ ret = drv->bdrv_open(bs, options, open_flags, NULL);
}
if (ret < 0) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 5d33e03..52d65ff 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -350,7 +350,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags)
+static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVBlkdebugState *s = bs->opaque;
QemuOpts *opts;
diff --git a/block/blkverify.c b/block/blkverify.c
index c4e961e..2093391 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -116,7 +116,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int blkverify_open(BlockDriverState *bs, QDict *options, int flags)
+static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVBlkverifyState *s = bs->opaque;
QemuOpts *opts;
diff --git a/block/bochs.c b/block/bochs.c
index d7078c0..51d9a90 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -108,7 +108,8 @@ static int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int bochs_open(BlockDriverState *bs, QDict *options, int flags)
+static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVBochsState *s = bs->opaque;
int i;
diff --git a/block/cloop.c b/block/cloop.c
index 6ea7cf4..b907023 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -53,7 +53,8 @@ static int cloop_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int cloop_open(BlockDriverState *bs, QDict *options, int flags)
+static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVCloopState *s = bs->opaque;
uint32_t offsets_size, max_compressed_block_size = 1, i;
diff --git a/block/cow.c b/block/cow.c
index 764b93f..3ae1b5c 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -58,7 +58,8 @@ static int cow_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int cow_open(BlockDriverState *bs, QDict *options, int flags)
+static int cow_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVCowState *s = bs->opaque;
struct cow_header_v2 cow_header;
diff --git a/block/curl.c b/block/curl.c
index ca2cedc..5a46f97 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -395,7 +395,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int curl_open(BlockDriverState *bs, QDict *options, int flags)
+static int curl_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVCURLState *s = bs->opaque;
CURLState *state = NULL;
diff --git a/block/dmg.c b/block/dmg.c
index 3141cb5..d5e9b1f 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -92,7 +92,8 @@ static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result)
return 0;
}
-static int dmg_open(BlockDriverState *bs, QDict *options, int flags)
+static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVDMGState *s = bs->opaque;
uint64_t info_begin,info_end,last_in_offset,last_out_offset;
diff --git a/block/gluster.c b/block/gluster.c
index dbb03f4..6c7b000 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -288,7 +288,7 @@ static QemuOptsList runtime_opts = {
};
static int qemu_gluster_open(BlockDriverState *bs, QDict *options,
- int bdrv_flags)
+ int bdrv_flags, Error **errp)
{
BDRVGlusterState *s = bs->opaque;
int open_flags = O_BINARY;
diff --git a/block/iscsi.c b/block/iscsi.c
index 813abd8..5f26e73 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1046,7 +1046,8 @@ static QemuOptsList runtime_opts = {
* We support iscsi url's on the form
* iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
*/
-static int iscsi_open(BlockDriverState *bs, QDict *options, int flags)
+static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
IscsiLun *iscsilun = bs->opaque;
struct iscsi_context *iscsi = NULL;
@@ -1260,7 +1261,7 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options)
bs_options = qdict_new();
qdict_put(bs_options, "filename", qstring_from_str(filename));
- ret = iscsi_open(bs, bs_options, 0);
+ ret = iscsi_open(bs, bs_options, 0, NULL);
QDECREF(bs_options);
if (ret != 0) {
diff --git a/block/nbd.c b/block/nbd.c
index 691066f..c8deeee 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -453,7 +453,8 @@ static void nbd_teardown_connection(BlockDriverState *bs)
closesocket(s->sock);
}
-static int nbd_open(BlockDriverState *bs, QDict *options, int flags)
+static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVNBDState *s = bs->opaque;
int result;
diff --git a/block/parallels.c b/block/parallels.c
index 18b3ac0..2121e43 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -68,7 +68,8 @@ static int parallels_probe(const uint8_t *buf, int buf_size, const char *filenam
return 0;
}
-static int parallels_open(BlockDriverState *bs, QDict *options, int flags)
+static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVParallelsState *s = bs->opaque;
int i;
diff --git a/block/qcow.c b/block/qcow.c
index 93a993b..2a1c9c9 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -92,7 +92,8 @@ static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int qcow_open(BlockDriverState *bs, QDict *options, int flags)
+static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVQcowState *s = bs->opaque;
int len, i, shift, ret;
diff --git a/block/qcow2.c b/block/qcow2.c
index 7c9354c..eda085c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -350,7 +350,8 @@ static QemuOptsList qcow2_runtime_opts = {
},
};
-static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
+static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVQcowState *s = bs->opaque;
int len, i, ret = 0;
@@ -1060,7 +1061,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs)
qbool_from_int(s->use_lazy_refcounts));
memset(s, 0, sizeof(BDRVQcowState));
- qcow2_open(bs, options, flags);
+ qcow2_open(bs, options, flags, NULL);
QDECREF(options);
diff --git a/block/qed.c b/block/qed.c
index 49b3a37..69e1834 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -373,7 +373,8 @@ static void bdrv_qed_rebind(BlockDriverState *bs)
s->bs = bs;
}
-static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags)
+static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVQEDState *s = bs->opaque;
QEDHeader le_header;
@@ -1547,7 +1548,7 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs)
bdrv_qed_close(bs);
memset(s, 0, sizeof(BDRVQEDState));
- bdrv_qed_open(bs, NULL, bs->open_flags);
+ bdrv_qed_open(bs, NULL, bs->open_flags, NULL);
}
static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 1b41ea3..dcdf45c 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -335,7 +335,8 @@ fail:
return ret;
}
-static int raw_open(BlockDriverState *bs, QDict *options, int flags)
+static int raw_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
@@ -1331,7 +1332,8 @@ static int check_hdev_writable(BDRVRawState *s)
return 0;
}
-static int hdev_open(BlockDriverState *bs, QDict *options, int flags)
+static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
int ret;
@@ -1565,7 +1567,8 @@ static BlockDriver bdrv_host_device = {
};
#ifdef __linux__
-static int floppy_open(BlockDriverState *bs, QDict *options, int flags)
+static int floppy_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
int ret;
@@ -1686,7 +1689,8 @@ static BlockDriver bdrv_host_floppy = {
.bdrv_eject = floppy_eject,
};
-static int cdrom_open(BlockDriverState *bs, QDict *options, int flags)
+static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index ff3c5ea..80551b9 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -235,7 +235,8 @@ static QemuOptsList raw_runtime_opts = {
},
};
-static int raw_open(BlockDriverState *bs, QDict *options, int flags)
+static int raw_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
int access_flags;
@@ -532,7 +533,8 @@ static int hdev_probe_device(const char *filename)
return 0;
}
-static int hdev_open(BlockDriverState *bs, QDict *options, int flags)
+static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
int access_flags, create_flags;
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index a9060ca..2418f58 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -135,7 +135,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
return bdrv_create_file(filename, options);
}
-static int raw_open(BlockDriverState *bs, QDict *options, int flags)
+static int raw_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
bs->sg = bs->file->sg;
return 0;
diff --git a/block/rbd.c b/block/rbd.c
index b1ab80c..b77f73c 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -446,7 +446,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags)
+static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVRBDState *s = bs->opaque;
char pool[RBD_MAX_POOL_NAME_SIZE];
diff --git a/block/sheepdog.c b/block/sheepdog.c
index fe438e0..1cfc6ae 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1242,7 +1242,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int sd_open(BlockDriverState *bs, QDict *options, int flags)
+static int sd_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
int ret, fd;
uint32_t vid = 0;
diff --git a/block/snapshot.c b/block/snapshot.c
index 82e602f..a05c0c0 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -170,7 +170,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
if (bs->file) {
drv->bdrv_close(bs);
ret = bdrv_snapshot_goto(bs->file, snapshot_id);
- open_ret = drv->bdrv_open(bs, NULL, bs->open_flags);
+ open_ret = drv->bdrv_open(bs, NULL, bs->open_flags, NULL);
if (open_ret < 0) {
bdrv_unref(bs->file);
bs->drv = NULL;
diff --git a/block/ssh.c b/block/ssh.c
index 27691b4..23cd1f0 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -608,7 +608,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
return ret;
}
-static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags)
+static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
+ Error **errp)
{
BDRVSSHState *s = bs->opaque;
int ret;
diff --git a/block/vdi.c b/block/vdi.c
index 1bf7dc5..dedbafb 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -364,7 +364,8 @@ static int vdi_probe(const uint8_t *buf, int buf_size, const char *filename)
return result;
}
-static int vdi_open(BlockDriverState *bs, QDict *options, int flags)
+static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVVdiState *s = bs->opaque;
VdiHeader header;
diff --git a/block/vhdx.c b/block/vhdx.c
index e9704b1..b8aa49c 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -715,7 +715,8 @@ exit:
}
-static int vhdx_open(BlockDriverState *bs, QDict *options, int flags)
+static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVVHDXState *s = bs->opaque;
int ret = 0;
diff --git a/block/vmdk.c b/block/vmdk.c
index fb5b529..d60bf0c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -806,7 +806,8 @@ exit:
return ret;
}
-static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
+static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
int ret;
BDRVVmdkState *s = bs->opaque;
diff --git a/block/vpc.c b/block/vpc.c
index fe4f311..ed00370 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -155,7 +155,8 @@ static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
}
-static int vpc_open(BlockDriverState *bs, QDict *options, int flags)
+static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVVPCState *s = bs->opaque;
int i;
diff --git a/block/vvfat.c b/block/vvfat.c
index 0129195..2f8be7c 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1065,7 +1065,8 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
qdict_put(options, "rw", qbool_from_int(rw));
}
-static int vvfat_open(BlockDriverState *bs, QDict *options, int flags)
+static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
BDRVVVFATState *s = bs->opaque;
int cyls, heads, secs;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e7d1766..076b40a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -80,8 +80,10 @@ struct BlockDriver {
void (*bdrv_reopen_commit)(BDRVReopenState *reopen_state);
void (*bdrv_reopen_abort)(BDRVReopenState *reopen_state);
- int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags);
- int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags);
+ int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp);
+ int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp);
int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
int (*bdrv_write)(BlockDriverState *bs, int64_t sector_num,
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 23/33] bdrv: Use "Error" for creating images
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (21 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 22/33] bdrv: Use "Error" for opening images Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 24/33] block: Error parameter for open functions Kevin Wolf
` (9 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Add an Error ** parameter to BlockDriver.bdrv_create to allow more
specific error messages.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 2 +-
block/cow.c | 3 ++-
block/gluster.c | 2 +-
block/iscsi.c | 3 ++-
block/qcow.c | 3 ++-
block/qcow2.c | 3 ++-
block/qed.c | 3 ++-
block/raw-posix.c | 6 ++++--
block/raw-win32.c | 3 ++-
block/raw_bsd.c | 3 ++-
block/rbd.c | 3 ++-
block/sheepdog.c | 3 ++-
block/ssh.c | 3 ++-
block/vdi.c | 3 ++-
block/vmdk.c | 3 ++-
block/vpc.c | 3 ++-
include/block/block_int.h | 3 ++-
17 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/block.c b/block.c
index 89098bc..f9b7033 100644
--- a/block.c
+++ b/block.c
@@ -401,7 +401,7 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
CreateCo *cco = opaque;
assert(cco->drv);
- cco->ret = cco->drv->bdrv_create(cco->filename, cco->options);
+ cco->ret = cco->drv->bdrv_create(cco->filename, cco->options, NULL);
}
int bdrv_create(BlockDriver *drv, const char* filename,
diff --git a/block/cow.c b/block/cow.c
index 3ae1b5c..e252c95 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -295,7 +295,8 @@ static void cow_close(BlockDriverState *bs)
{
}
-static int cow_create(const char *filename, QEMUOptionParameter *options)
+static int cow_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
struct cow_header_v2 cow_header;
struct stat st;
diff --git a/block/gluster.c b/block/gluster.c
index 6c7b000..256de10 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -357,7 +357,7 @@ out:
}
static int qemu_gluster_create(const char *filename,
- QEMUOptionParameter *options)
+ QEMUOptionParameter *options, Error **errp)
{
struct glfs *glfs;
struct glfs_fd *fd;
diff --git a/block/iscsi.c b/block/iscsi.c
index 5f26e73..997b122 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1238,7 +1238,8 @@ static int iscsi_has_zero_init(BlockDriverState *bs)
return 0;
}
-static int iscsi_create(const char *filename, QEMUOptionParameter *options)
+static int iscsi_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int ret = 0;
int64_t total_size = 0;
diff --git a/block/qcow.c b/block/qcow.c
index 2a1c9c9..0911edf 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -659,7 +659,8 @@ static void qcow_close(BlockDriverState *bs)
error_free(s->migration_blocker);
}
-static int qcow_create(const char *filename, QEMUOptionParameter *options)
+static int qcow_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int header_size, backing_filename_len, l1_size, shift, i;
QCowHeader header;
diff --git a/block/qcow2.c b/block/qcow2.c
index eda085c..43edc5b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1468,7 +1468,8 @@ out:
return ret;
}
-static int qcow2_create(const char *filename, QEMUOptionParameter *options)
+static int qcow2_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
const char *backing_file = NULL;
const char *backing_fmt = NULL;
diff --git a/block/qed.c b/block/qed.c
index 69e1834..250fa89 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -604,7 +604,8 @@ out:
return ret;
}
-static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options)
+static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
uint64_t image_size = 0;
uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
diff --git a/block/raw-posix.c b/block/raw-posix.c
index dcdf45c..3ee5b62 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1041,7 +1041,8 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
return (int64_t)st.st_blocks * 512;
}
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int raw_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int fd;
int result = 0;
@@ -1506,7 +1507,8 @@ static coroutine_fn BlockDriverAIOCB *hdev_aio_discard(BlockDriverState *bs,
cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV);
}
-static int hdev_create(const char *filename, QEMUOptionParameter *options)
+static int hdev_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int fd;
int ret = 0;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 80551b9..1e7651b 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -422,7 +422,8 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
return st.st_size;
}
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int raw_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int fd;
int64_t total_size = 0;
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 2418f58..7d75ad2 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -130,7 +130,8 @@ static int raw_has_zero_init(BlockDriverState *bs)
return bdrv_has_zero_init(bs->file);
}
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int raw_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
return bdrv_create_file(filename, options);
}
diff --git a/block/rbd.c b/block/rbd.c
index b77f73c..11086c3 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -287,7 +287,8 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
return ret;
}
-static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options)
+static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int64_t bytes = 0;
int64_t objsize;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 1cfc6ae..a93a32a 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1438,7 +1438,8 @@ out:
return ret;
}
-static int sd_create(const char *filename, QEMUOptionParameter *options)
+static int sd_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int ret = 0;
uint32_t vid = 0, base_vid = 0;
diff --git a/block/ssh.c b/block/ssh.c
index 23cd1f0..aa63c9d 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -651,7 +651,8 @@ static QEMUOptionParameter ssh_create_options[] = {
{ NULL }
};
-static int ssh_create(const char *filename, QEMUOptionParameter *options)
+static int ssh_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int r, ret;
Error *local_err = NULL;
diff --git a/block/vdi.c b/block/vdi.c
index dedbafb..dcbc27c 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -645,7 +645,8 @@ static int vdi_co_write(BlockDriverState *bs,
return ret;
}
-static int vdi_create(const char *filename, QEMUOptionParameter *options)
+static int vdi_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int fd;
int result = 0;
diff --git a/block/vmdk.c b/block/vmdk.c
index d60bf0c..a98da08 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1552,7 +1552,8 @@ static int filename_decompose(const char *filename, char *path, char *prefix,
return VMDK_OK;
}
-static int vmdk_create(const char *filename, QEMUOptionParameter *options)
+static int vmdk_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
int fd, idx = 0;
char desc[BUF_SIZE];
diff --git a/block/vpc.c b/block/vpc.c
index ed00370..db61274 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -684,7 +684,8 @@ static int create_fixed_disk(int fd, uint8_t *buf, int64_t total_size)
return ret;
}
-static int vpc_create(const char *filename, QEMUOptionParameter *options)
+static int vpc_create(const char *filename, QEMUOptionParameter *options,
+ Error **errp)
{
uint8_t buf[1024];
struct vhd_footer *footer = (struct vhd_footer *) buf;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 076b40a..3eeb6fe 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -90,7 +90,8 @@ struct BlockDriver {
const uint8_t *buf, int nb_sectors);
void (*bdrv_close)(BlockDriverState *bs);
void (*bdrv_rebind)(BlockDriverState *bs);
- int (*bdrv_create)(const char *filename, QEMUOptionParameter *options);
+ int (*bdrv_create)(const char *filename, QEMUOptionParameter *options,
+ Error **errp);
int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
int (*bdrv_make_empty)(BlockDriverState *bs);
/* aio */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 24/33] block: Error parameter for open functions
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (22 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 23/33] bdrv: Use "Error" for creating images Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 25/33] block: Error parameter for create functions Kevin Wolf
` (8 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Add an Error ** parameter to bdrv_open, bdrv_file_open and associated
functions to allow more specific error messages.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 100 ++++++++++++++++++++++++++++++++------------------
block/blkdebug.c | 4 +-
block/blkverify.c | 8 +++-
block/cow.c | 5 ++-
block/mirror.c | 5 ++-
block/qcow.c | 5 ++-
block/qcow2.c | 4 +-
block/qed.c | 6 ++-
block/sheepdog.c | 10 ++++-
block/vmdk.c | 11 +++++-
block/vvfat.c | 6 ++-
blockdev.c | 30 +++++++--------
hw/block/xen_disk.c | 7 +++-
include/block/block.h | 6 +--
qemu-img.c | 21 +++++++----
qemu-io.c | 14 +++++--
qemu-nbd.c | 6 ++-
17 files changed, 163 insertions(+), 85 deletions(-)
diff --git a/block.c b/block.c
index f9b7033..ba0d876 100644
--- a/block.c
+++ b/block.c
@@ -552,7 +552,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
}
static int find_image_format(BlockDriverState *bs, const char *filename,
- BlockDriver **pdrv)
+ BlockDriver **pdrv, Error **errp)
{
int score, score_max;
BlockDriver *drv1, *drv;
@@ -563,6 +563,7 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
drv = bdrv_find_format("raw");
if (!drv) {
+ error_setg(errp, "Could not find raw image format");
ret = -ENOENT;
}
*pdrv = drv;
@@ -571,6 +572,8 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
ret = bdrv_pread(bs, 0, buf, sizeof(buf));
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read image for determining its "
+ "format");
*pdrv = NULL;
return ret;
}
@@ -587,6 +590,8 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
}
}
if (!drv) {
+ error_setg(errp, "Could not determine image format: No compatible "
+ "driver found");
ret = -ENOENT;
}
*pdrv = drv;
@@ -706,10 +711,11 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
* Removes all processed options from *options.
*/
static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
- QDict *options, int flags, BlockDriver *drv)
+ QDict *options, int flags, BlockDriver *drv, Error **errp)
{
int ret, open_flags;
const char *filename;
+ Error *local_err = NULL;
assert(drv != NULL);
assert(bs->file == NULL);
@@ -738,6 +744,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
bs->read_only = !(open_flags & BDRV_O_RDWR);
if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
+ error_setg(errp, "Driver '%s' is not whitelisted", drv->format_name);
return -ENOTSUP;
}
@@ -761,25 +768,32 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
if (drv->bdrv_file_open) {
assert(file == NULL);
assert(drv->bdrv_parse_filename || filename != NULL);
- ret = drv->bdrv_file_open(bs, options, open_flags, NULL);
+ ret = drv->bdrv_file_open(bs, options, open_flags, &local_err);
} else {
if (file == NULL) {
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
- "block driver for the protocol level",
- drv->format_name);
+ error_setg(errp, "Can't use '%s' as a block driver for the "
+ "protocol level", drv->format_name);
ret = -EINVAL;
goto free_and_fail;
}
bs->file = file;
- ret = drv->bdrv_open(bs, options, open_flags, NULL);
+ ret = drv->bdrv_open(bs, options, open_flags, &local_err);
}
if (ret < 0) {
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ } else if (filename) {
+ error_setg_errno(errp, -ret, "Could not open '%s'", filename);
+ } else {
+ error_setg_errno(errp, -ret, "Could not open image");
+ }
goto free_and_fail;
}
ret = refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not refresh total sector count");
goto free_and_fail;
}
@@ -808,12 +822,13 @@ free_and_fail:
* dictionary, it needs to use QINCREF() before calling bdrv_file_open.
*/
int bdrv_file_open(BlockDriverState **pbs, const char *filename,
- QDict *options, int flags)
+ QDict *options, int flags, Error **errp)
{
BlockDriverState *bs;
BlockDriver *drv;
const char *drvname;
bool allow_protocol_prefix = false;
+ Error *local_err = NULL;
int ret;
/* NULL means an empty set of options */
@@ -832,8 +847,8 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
qdict_put(options, "filename", qstring_from_str(filename));
allow_protocol_prefix = true;
} else {
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't specify 'file' and "
- "'filename' options at the same time");
+ error_setg(errp, "Can't specify 'file' and 'filename' options at the "
+ "same time");
ret = -EINVAL;
goto fail;
}
@@ -842,53 +857,53 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
drvname = qdict_get_try_str(options, "driver");
if (drvname) {
drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR));
+ if (!drv) {
+ error_setg(errp, "Unknown driver '%s'", drvname);
+ }
qdict_del(options, "driver");
} else if (filename) {
drv = bdrv_find_protocol(filename, allow_protocol_prefix);
if (!drv) {
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Unknown protocol");
+ error_setg(errp, "Unknown protocol");
}
} else {
- qerror_report(ERROR_CLASS_GENERIC_ERROR,
- "Must specify either driver or file");
+ error_setg(errp, "Must specify either driver or file");
drv = NULL;
}
if (!drv) {
+ /* errp has been set already */
ret = -ENOENT;
goto fail;
}
/* Parse the filename and open it */
if (drv->bdrv_parse_filename && filename) {
- Error *local_err = NULL;
drv->bdrv_parse_filename(filename, options, &local_err);
if (error_is_set(&local_err)) {
- qerror_report_err(local_err);
- error_free(local_err);
+ error_propagate(errp, local_err);
ret = -EINVAL;
goto fail;
}
qdict_del(options, "filename");
} else if (!drv->bdrv_parse_filename && !filename) {
- qerror_report(ERROR_CLASS_GENERIC_ERROR,
- "The '%s' block driver requires a file name",
- drv->format_name);
+ error_setg(errp, "The '%s' block driver requires a file name",
+ drv->format_name);
ret = -EINVAL;
goto fail;
}
- ret = bdrv_open_common(bs, NULL, options, flags, drv);
+ ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
if (ret < 0) {
+ error_propagate(errp, local_err);
goto fail;
}
/* Check if any unknown options were used */
if (qdict_size(options) != 0) {
const QDictEntry *entry = qdict_first(options);
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block protocol '%s' doesn't "
- "support the option '%s'",
- drv->format_name, entry->key);
+ error_setg(errp, "Block protocol '%s' doesn't support the option '%s'",
+ drv->format_name, entry->key);
ret = -EINVAL;
goto fail;
}
@@ -915,11 +930,12 @@ fail:
* function (even on failure), so if the caller intends to reuse the dictionary,
* it needs to use QINCREF() before calling bdrv_file_open.
*/
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
{
char backing_filename[PATH_MAX];
int back_flags, ret;
BlockDriver *back_drv = NULL;
+ Error *local_err = NULL;
if (bs->backing_hd != NULL) {
QDECREF(options);
@@ -952,11 +968,12 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
ret = bdrv_open(bs->backing_hd,
*backing_filename ? backing_filename : NULL, options,
- back_flags, back_drv);
+ back_flags, back_drv, &local_err);
if (ret < 0) {
bdrv_unref(bs->backing_hd);
bs->backing_hd = NULL;
bs->open_flags |= BDRV_O_NO_BACKING;
+ error_propagate(errp, local_err);
return ret;
}
return 0;
@@ -990,7 +1007,7 @@ static void extract_subqdict(QDict *src, QDict **dst, const char *start)
* dictionary, it needs to use QINCREF() before calling bdrv_open.
*/
int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
- int flags, BlockDriver *drv)
+ int flags, BlockDriver *drv, Error **errp)
{
int ret;
/* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
@@ -998,6 +1015,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
BlockDriverState *file = NULL;
QDict *file_options = NULL;
const char *drvname;
+ Error *local_err = NULL;
/* NULL means an empty set of options */
if (options == NULL) {
@@ -1016,7 +1034,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
char backing_filename[PATH_MAX];
if (qdict_size(options) != 0) {
- error_report("Can't use snapshot=on with driver-specific options");
+ error_setg(errp, "Can't use snapshot=on with driver-specific options");
ret = -EINVAL;
goto fail;
}
@@ -1027,7 +1045,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
/* if there is a backing file, use it */
bs1 = bdrv_new("");
- ret = bdrv_open(bs1, filename, NULL, 0, drv);
+ ret = bdrv_open(bs1, filename, NULL, 0, drv, &local_err);
if (ret < 0) {
bdrv_unref(bs1);
goto fail;
@@ -1038,6 +1056,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not get temporary filename");
goto fail;
}
@@ -1046,6 +1065,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
snprintf(backing_filename, sizeof(backing_filename),
"%s", filename);
} else if (!realpath(filename, backing_filename)) {
+ error_setg_errno(errp, errno, "Could not resolve path '%s'", filename);
ret = -errno;
goto fail;
}
@@ -1065,6 +1085,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
free_option_parameters(create_options);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not create temporary overlay "
+ "'%s'", tmp_filename);
goto fail;
}
@@ -1081,7 +1103,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
extract_subqdict(options, &file_options, "file.");
ret = bdrv_file_open(&file, filename, file_options,
- bdrv_open_flags(bs, flags | BDRV_O_UNMAP));
+ bdrv_open_flags(bs, flags | BDRV_O_UNMAP), &local_err);
if (ret < 0) {
goto fail;
}
@@ -1094,7 +1116,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
}
if (!drv) {
- ret = find_image_format(file, filename, &drv);
+ ret = find_image_format(file, filename, &drv, &local_err);
}
if (!drv) {
@@ -1102,7 +1124,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
}
/* Open the image */
- ret = bdrv_open_common(bs, file, options, flags, drv);
+ ret = bdrv_open_common(bs, file, options, flags, drv, &local_err);
if (ret < 0) {
goto unlink_and_fail;
}
@@ -1117,7 +1139,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
QDict *backing_options;
extract_subqdict(options, &backing_options, "backing.");
- ret = bdrv_open_backing_file(bs, backing_options);
+ ret = bdrv_open_backing_file(bs, backing_options, &local_err);
if (ret < 0) {
goto close_and_fail;
}
@@ -1126,9 +1148,9 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
/* Check if any unknown options were used */
if (qdict_size(options) != 0) {
const QDictEntry *entry = qdict_first(options);
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by "
- "device '%s' doesn't support the option '%s'",
- drv->format_name, bs->device_name, entry->key);
+ error_setg(errp, "Block format '%s' used by device '%s' doesn't "
+ "support the option '%s'", drv->format_name, bs->device_name,
+ entry->key);
ret = -EINVAL;
goto close_and_fail;
@@ -1152,11 +1174,17 @@ fail:
QDECREF(bs->options);
QDECREF(options);
bs->options = NULL;
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
return ret;
close_and_fail:
bdrv_close(bs);
QDECREF(options);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
return ret;
}
@@ -4519,7 +4547,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
bs = bdrv_new("");
ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
- backing_drv);
+ backing_drv, NULL);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not open '%s'",
backing_file->value.s);
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 52d65ff..be948b2 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -387,8 +387,10 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
- ret = bdrv_file_open(&bs->file, filename, NULL, flags);
+ ret = bdrv_file_open(&bs->file, filename, NULL, flags, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto fail;
}
diff --git a/block/blkverify.c b/block/blkverify.c
index 2093391..bff95d2 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -141,8 +141,10 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
goto fail;
}
- ret = bdrv_file_open(&bs->file, raw, NULL, flags);
+ ret = bdrv_file_open(&bs->file, raw, NULL, flags, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto fail;
}
@@ -154,8 +156,10 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
}
s->test_file = bdrv_new("");
- ret = bdrv_open(s->test_file, filename, NULL, flags, NULL);
+ ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
bdrv_unref(s->test_file);
s->test_file = NULL;
goto fail;
diff --git a/block/cow.c b/block/cow.c
index e252c95..3a93ed9 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -302,6 +302,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
struct stat st;
int64_t image_sectors = 0;
const char *image_filename = NULL;
+ Error *local_err = NULL;
int ret;
BlockDriverState *cow_bs;
@@ -320,8 +321,10 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
return ret;
}
- ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR);
+ ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
diff --git a/block/mirror.c b/block/mirror.c
index f61a779..6e7a274 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -505,14 +505,15 @@ static void mirror_iostatus_reset(BlockJob *job)
static void mirror_complete(BlockJob *job, Error **errp)
{
MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+ Error *local_err = NULL;
int ret;
- ret = bdrv_open_backing_file(s->target, NULL);
+ ret = bdrv_open_backing_file(s->target, NULL, &local_err);
if (ret < 0) {
char backing_filename[PATH_MAX];
bdrv_get_full_backing_filename(s->target, backing_filename,
sizeof(backing_filename));
- error_setg_file_open(errp, -ret, backing_filename);
+ error_propagate(errp, local_err);
return;
}
if (!s->synced) {
diff --git a/block/qcow.c b/block/qcow.c
index 0911edf..396636f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -668,6 +668,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
int64_t total_size = 0;
const char *backing_file = NULL;
int flags = 0;
+ Error *local_err = NULL;
int ret;
BlockDriverState *qcow_bs;
@@ -688,8 +689,10 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
return ret;
}
- ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR);
+ ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 43edc5b..dabfe8d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1370,7 +1370,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
return ret;
}
- ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR);
+ ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
if (ret < 0) {
return ret;
}
@@ -1423,7 +1423,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
BlockDriver* drv = bdrv_find_format("qcow2");
assert(drv != NULL);
ret = bdrv_open(bs, filename, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv);
+ BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, NULL);
if (ret < 0) {
goto out;
}
diff --git a/block/qed.c b/block/qed.c
index 250fa89..f17094c 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -551,6 +551,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
QEDHeader le_header;
uint8_t *l1_table = NULL;
size_t l1_size = header.cluster_size * header.table_size;
+ Error *local_err = NULL;
int ret = 0;
BlockDriverState *bs = NULL;
@@ -559,8 +560,11 @@ static int qed_create(const char *filename, uint32_t cluster_size,
return ret;
}
- ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB);
+ ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB,
+ &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
diff --git a/block/sheepdog.c b/block/sheepdog.c
index a93a32a..38fb629 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1401,10 +1401,13 @@ static int sd_prealloc(const char *filename)
uint32_t idx, max_idx;
int64_t vdi_size;
void *buf = g_malloc0(SD_DATA_OBJ_SIZE);
+ Error *local_err = NULL;
int ret;
- ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR);
+ ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto out;
}
@@ -1449,6 +1452,7 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN];
uint32_t snapid;
bool prealloc = false;
+ Error *local_err = NULL;
s = g_malloc0(sizeof(BDRVSheepdogState));
@@ -1502,8 +1506,10 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
goto out;
}
- ret = bdrv_file_open(&bs, backing_file, NULL, 0);
+ ret = bdrv_file_open(&bs, backing_file, NULL, 0, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto out;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index a98da08..96ef1b5 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -697,6 +697,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
int64_t flat_offset;
char extent_path[PATH_MAX];
BlockDriverState *extent_file;
+ Error *local_err = NULL;
while (*p) {
/* parse extent line:
@@ -726,8 +727,11 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
path_combine(extent_path, sizeof(extent_path),
desc_file_path, fname);
- ret = bdrv_file_open(&extent_file, extent_path, NULL, bs->open_flags);
+ ret = bdrv_file_open(&extent_file, extent_path, NULL, bs->open_flags,
+ &local_err);
if (ret) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
@@ -1591,6 +1595,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
"ddb.geometry.heads = \"%d\"\n"
"ddb.geometry.sectors = \"63\"\n"
"ddb.adapterType = \"%s\"\n";
+ Error *local_err = NULL;
if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
return -EINVAL;
@@ -1653,8 +1658,10 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
}
if (backing_file) {
BlockDriverState *bs = bdrv_new("");
- ret = bdrv_open(bs, backing_file, NULL, 0, NULL);
+ ret = bdrv_open(bs, backing_file, NULL, 0, NULL, &local_err);
if (ret != 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
bdrv_unref(bs);
return ret;
}
diff --git a/block/vvfat.c b/block/vvfat.c
index 2f8be7c..788d063 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2910,6 +2910,7 @@ static int enable_write_target(BDRVVVFATState *s)
{
BlockDriver *bdrv_qcow;
QEMUOptionParameter *options;
+ Error *local_err = NULL;
int ret;
int size = sector2cluster(s, s->sector_count);
s->used_clusters = calloc(size, 1);
@@ -2935,8 +2936,11 @@ static int enable_write_target(BDRVVVFATState *s)
s->qcow = bdrv_new("");
ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow);
+ BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
+ &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
bdrv_unref(s->qcow);
goto err;
}
diff --git a/blockdev.c b/blockdev.c
index 84d55c1..2ab236a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -710,17 +710,11 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
}
QINCREF(bs_opts);
- ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv);
+ ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
if (ret < 0) {
- if (ret == -EMEDIUMTYPE) {
- error_report("could not open disk image %s: not in %s format",
- file ?: dinfo->id, drv ? drv->format_name :
- qdict_get_str(bs_opts, "driver"));
- } else {
- error_report("could not open disk image %s: %s",
- file ?: dinfo->id, strerror(-ret));
- }
+ error_report("could not open disk image %s: %s",
+ file ?: dinfo->id, error_get_pretty(error));
goto err;
}
@@ -1156,9 +1150,9 @@ static void external_snapshot_prepare(BlkTransactionState *common,
/* TODO Inherit bs->options or only take explicit options with an
* extended QMP command? */
ret = bdrv_open(state->new_bs, new_image_file, NULL,
- flags | BDRV_O_NO_BACKING, drv);
+ flags | BDRV_O_NO_BACKING, drv, &local_err);
if (ret != 0) {
- error_setg_file_open(errp, -ret, new_image_file);
+ error_propagate(errp, local_err);
}
}
@@ -1393,11 +1387,12 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
int bdrv_flags, BlockDriver *drv,
const char *password, Error **errp)
{
+ Error *local_err = NULL;
int ret;
- ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv);
+ ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv, &local_err);
if (ret < 0) {
- error_setg_file_open(errp, -ret, filename);
+ error_propagate(errp, local_err);
return;
}
@@ -1817,10 +1812,10 @@ void qmp_drive_backup(const char *device, const char *target,
}
target_bs = bdrv_new("");
- ret = bdrv_open(target_bs, target, NULL, flags, drv);
+ ret = bdrv_open(target_bs, target, NULL, flags, drv, &local_err);
if (ret < 0) {
bdrv_unref(target_bs);
- error_setg_file_open(errp, -ret, target);
+ error_propagate(errp, local_err);
return;
}
@@ -1952,10 +1947,11 @@ void qmp_drive_mirror(const char *device, const char *target,
* file.
*/
target_bs = bdrv_new("");
- ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv);
+ ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
+ &local_err);
if (ret < 0) {
bdrv_unref(target_bs);
- error_setg_file_open(errp, -ret, target);
+ error_propagate(errp, local_err);
return;
}
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 668cc06..f35fc59 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -809,10 +809,15 @@ static int blk_connect(struct XenDevice *xendev)
xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
blkdev->bs = bdrv_new(blkdev->dev);
if (blkdev->bs) {
+ Error *local_err = NULL;
BlockDriver *drv = bdrv_find_whitelisted_format(blkdev->fileproto,
readonly);
if (bdrv_open(blkdev->bs,
- blkdev->filename, NULL, qflags, drv) != 0) {
+ blkdev->filename, NULL, qflags, drv, &local_err) != 0)
+ {
+ xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
+ error_get_pretty(local_err));
+ error_free(local_err);
bdrv_unref(blkdev->bs);
blkdev->bs = NULL;
}
diff --git a/include/block/block.h b/include/block/block.h
index 1c5f939..9dbc5ef 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -151,10 +151,10 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
int bdrv_parse_cache_flags(const char *mode, int *flags);
int bdrv_parse_discard_flags(const char *mode, int *flags);
int bdrv_file_open(BlockDriverState **pbs, const char *filename,
- QDict *options, int flags);
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
+ QDict *options, int flags, Error **errp);
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
- int flags, BlockDriver *drv);
+ int flags, BlockDriver *drv, Error **errp);
BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
BlockDriverState *bs, int flags);
int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
diff --git a/qemu-img.c b/qemu-img.c
index 139526f..49ff068 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -266,6 +266,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
BlockDriverState *bs;
BlockDriver *drv;
char password[256];
+ Error *local_err = NULL;
int ret;
bs = bdrv_new("image");
@@ -280,9 +281,11 @@ static BlockDriverState *bdrv_new_open(const char *filename,
drv = NULL;
}
- ret = bdrv_open(bs, filename, NULL, flags, drv);
+ ret = bdrv_open(bs, filename, NULL, flags, drv, &local_err);
if (ret < 0) {
- error_report("Could not open '%s': %s", filename, strerror(-ret));
+ error_report("Could not open '%s': %s", filename,
+ error_get_pretty(local_err));
+ error_free(local_err);
goto fail;
}
@@ -2127,6 +2130,7 @@ static int img_rebase(int argc, char **argv)
int unsafe = 0;
int progress = 0;
bool quiet = false;
+ Error *local_err = NULL;
/* Parse commandline parameters */
fmt = NULL;
@@ -2230,18 +2234,21 @@ static int img_rebase(int argc, char **argv)
bs_old_backing = bdrv_new("old_backing");
bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
ret = bdrv_open(bs_old_backing, backing_name, NULL, BDRV_O_FLAGS,
- old_backing_drv);
+ old_backing_drv, &local_err);
if (ret) {
- error_report("Could not open old backing file '%s'", backing_name);
+ error_report("Could not open old backing file '%s': %s",
+ backing_name, error_get_pretty(local_err));
+ error_free(local_err);
goto out;
}
if (out_baseimg[0]) {
bs_new_backing = bdrv_new("new_backing");
ret = bdrv_open(bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS,
- new_backing_drv);
+ new_backing_drv, &local_err);
if (ret) {
- error_report("Could not open new backing file '%s'",
- out_baseimg);
+ error_report("Could not open new backing file '%s': %s",
+ out_baseimg, error_get_pretty(local_err));
+ error_free(local_err);
goto out;
}
}
diff --git a/qemu-io.c b/qemu-io.c
index 71f4ff1..f4b8efc 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -46,21 +46,27 @@ static const cmdinfo_t close_cmd = {
static int openfile(char *name, int flags, int growable)
{
+ Error *local_err = NULL;
+
if (qemuio_bs) {
fprintf(stderr, "file open already, try 'help close'\n");
return 1;
}
if (growable) {
- if (bdrv_file_open(&qemuio_bs, name, NULL, flags)) {
- fprintf(stderr, "%s: can't open device %s\n", progname, name);
+ if (bdrv_file_open(&qemuio_bs, name, NULL, flags, &local_err)) {
+ fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
+ error_get_pretty(local_err));
+ error_free(local_err);
return 1;
}
} else {
qemuio_bs = bdrv_new("hda");
- if (bdrv_open(qemuio_bs, name, NULL, flags, NULL) < 0) {
- fprintf(stderr, "%s: can't open device %s\n", progname, name);
+ if (bdrv_open(qemuio_bs, name, NULL, flags, NULL, &local_err) < 0) {
+ fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
+ error_get_pretty(local_err));
+ error_free(local_err);
bdrv_unref(qemuio_bs);
qemuio_bs = NULL;
return 1;
diff --git a/qemu-nbd.c b/qemu-nbd.c
index f044546..c26c98e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -355,6 +355,7 @@ int main(int argc, char **argv)
#endif
pthread_t client_thread;
const char *fmt = NULL;
+ Error *local_err = NULL;
/* The client thread uses SIGTERM to interrupt the server. A signal
* handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -573,10 +574,11 @@ int main(int argc, char **argv)
bs = bdrv_new("hda");
srcpath = argv[optind];
- ret = bdrv_open(bs, srcpath, NULL, flags, drv);
+ ret = bdrv_open(bs, srcpath, NULL, flags, drv, &local_err);
if (ret < 0) {
errno = -ret;
- err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]);
+ err(EXIT_FAILURE, "Failed to bdrv_open '%s': %s", argv[optind],
+ error_get_pretty(local_err));
}
fd_size = bdrv_getlength(bs);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 25/33] block: Error parameter for create functions
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (23 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 24/33] block: Error parameter for open functions Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 26/33] qemu-img create: Emit filename on error Kevin Wolf
` (7 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Add an Error ** parameter to bdrv_create and its associated functions to
allow more specific error messages.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 80 +++++++++++++++++++++++++++++++++++----------------
block/cow.c | 4 ++-
block/qcow.c | 4 ++-
block/qcow2.c | 2 +-
block/qed.c | 4 ++-
block/raw_bsd.c | 10 ++++++-
block/vvfat.c | 4 ++-
include/block/block.h | 5 ++--
qemu-img.c | 16 ++++-------
9 files changed, 86 insertions(+), 43 deletions(-)
diff --git a/block.c b/block.c
index ba0d876..e176c6f 100644
--- a/block.c
+++ b/block.c
@@ -394,18 +394,26 @@ typedef struct CreateCo {
char *filename;
QEMUOptionParameter *options;
int ret;
+ Error *err;
} CreateCo;
static void coroutine_fn bdrv_create_co_entry(void *opaque)
{
+ Error *local_err = NULL;
+ int ret;
+
CreateCo *cco = opaque;
assert(cco->drv);
- cco->ret = cco->drv->bdrv_create(cco->filename, cco->options, NULL);
+ ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(&cco->err, local_err);
+ }
+ cco->ret = ret;
}
int bdrv_create(BlockDriver *drv, const char* filename,
- QEMUOptionParameter *options)
+ QEMUOptionParameter *options, Error **errp)
{
int ret;
@@ -415,9 +423,11 @@ int bdrv_create(BlockDriver *drv, const char* filename,
.filename = g_strdup(filename),
.options = options,
.ret = NOT_DONE,
+ .err = NULL,
};
if (!drv->bdrv_create) {
+ error_setg(errp, "Driver '%s' does not support image creation", drv->format_name);
ret = -ENOTSUP;
goto out;
}
@@ -434,22 +444,37 @@ int bdrv_create(BlockDriver *drv, const char* filename,
}
ret = cco.ret;
+ if (ret < 0) {
+ if (error_is_set(&cco.err)) {
+ error_propagate(errp, cco.err);
+ } else {
+ error_setg_errno(errp, -ret, "Could not create image");
+ }
+ }
out:
g_free(cco.filename);
return ret;
}
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
+int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
+ Error **errp)
{
BlockDriver *drv;
+ Error *local_err = NULL;
+ int ret;
drv = bdrv_find_protocol(filename, true);
if (drv == NULL) {
+ error_setg(errp, "Could not find protocol for file '%s'", filename);
return -ENOENT;
}
- return bdrv_create(drv, filename, options);
+ ret = bdrv_create(drv, filename, options, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
+ return ret;
}
/*
@@ -1082,11 +1107,14 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
drv->format_name);
}
- ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
+ ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
free_option_parameters(create_options);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not create temporary overlay "
- "'%s'", tmp_filename);
+ "'%s': %s", tmp_filename,
+ error_get_pretty(local_err));
+ error_free(local_err);
+ local_err = NULL;
goto fail;
}
@@ -4461,6 +4489,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
BlockDriverState *bs = NULL;
BlockDriver *drv, *proto_drv;
BlockDriver *backing_drv = NULL;
+ Error *local_err = NULL;
int ret = 0;
/* Find driver and parse its options */
@@ -4547,10 +4576,13 @@ void bdrv_img_create(const char *filename, const char *fmt,
bs = bdrv_new("");
ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
- backing_drv, NULL);
+ backing_drv, &local_err);
if (ret < 0) {
- error_setg_errno(errp, -ret, "Could not open '%s'",
- backing_file->value.s);
+ error_setg_errno(errp, -ret, "Could not open '%s': %s",
+ backing_file->value.s,
+ error_get_pretty(local_err));
+ error_free(local_err);
+ local_err = NULL;
goto out;
}
bdrv_get_geometry(bs, &size);
@@ -4569,22 +4601,19 @@ void bdrv_img_create(const char *filename, const char *fmt,
print_option_parameters(param);
puts("");
}
- ret = bdrv_create(drv, filename, param);
- if (ret < 0) {
- if (ret == -ENOTSUP) {
- error_setg(errp,"Formatting or formatting option not supported for "
- "file format '%s'", fmt);
- } else if (ret == -EFBIG) {
- const char *cluster_size_hint = "";
- if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE)) {
- cluster_size_hint = " (try using a larger cluster size)";
- }
- error_setg(errp, "The image size is too large for file format '%s'%s",
- fmt, cluster_size_hint);
- } else {
- error_setg(errp, "%s: error while creating %s: %s", filename, fmt,
- strerror(-ret));
+ ret = bdrv_create(drv, filename, param, &local_err);
+ if (ret == -EFBIG) {
+ /* This is generally a better message than whatever the driver would
+ * deliver (especially because of the cluster_size_hint), since that
+ * is most probably not much different from "image too large". */
+ const char *cluster_size_hint = "";
+ if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE)) {
+ cluster_size_hint = " (try using a larger cluster size)";
}
+ error_setg(errp, "The image size is too large for file format '%s'"
+ "%s", fmt, cluster_size_hint);
+ error_free(local_err);
+ local_err = NULL;
}
out:
@@ -4594,6 +4623,9 @@ out:
if (bs) {
bdrv_unref(bs);
}
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
}
AioContext *bdrv_get_aio_context(BlockDriverState *bs)
diff --git a/block/cow.c b/block/cow.c
index 3a93ed9..909c3e7 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -316,8 +316,10 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
options++;
}
- ret = bdrv_create_file(filename, options);
+ ret = bdrv_create_file(filename, options, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
diff --git a/block/qcow.c b/block/qcow.c
index 396636f..c470e05 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -684,8 +684,10 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
options++;
}
- ret = bdrv_create_file(filename, options);
+ ret = bdrv_create_file(filename, options, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index dabfe8d..b257347 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1365,7 +1365,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
uint8_t* refcount_table;
int ret;
- ret = bdrv_create_file(filename, options);
+ ret = bdrv_create_file(filename, options, NULL);
if (ret < 0) {
return ret;
}
diff --git a/block/qed.c b/block/qed.c
index f17094c..6c0cba0 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -555,8 +555,10 @@ static int qed_create(const char *filename, uint32_t cluster_size,
int ret = 0;
BlockDriverState *bs = NULL;
- ret = bdrv_create_file(filename, NULL);
+ ret = bdrv_create_file(filename, NULL, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
return ret;
}
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 7d75ad2..d4ace60 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -133,7 +133,15 @@ static int raw_has_zero_init(BlockDriverState *bs)
static int raw_create(const char *filename, QEMUOptionParameter *options,
Error **errp)
{
- return bdrv_create_file(filename, options);
+ Error *local_err = NULL;
+ int ret;
+
+ ret = bdrv_create_file(filename, options, &local_err);
+ if (error_is_set(&local_err)) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ }
+ return ret;
}
static int raw_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/block/vvfat.c b/block/vvfat.c
index 788d063..3ddaa0b 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2928,8 +2928,10 @@ static int enable_write_target(BDRVVVFATState *s)
set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count * 512);
set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:");
- ret = bdrv_create(bdrv_qcow, s->qcow_filename, options);
+ ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, &local_err);
if (ret < 0) {
+ qerror_report_err(local_err);
+ error_free(local_err);
goto err;
}
diff --git a/include/block/block.h b/include/block/block.h
index 9dbc5ef..f808550 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -142,8 +142,9 @@ BlockDriver *bdrv_find_format(const char *format_name);
BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
bool readonly);
int bdrv_create(BlockDriver *drv, const char* filename,
- QEMUOptionParameter *options);
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options);
+ QEMUOptionParameter *options, Error **errp);
+int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
+ Error **errp);
BlockDriverState *bdrv_new(const char *device_name);
void bdrv_make_anon(BlockDriverState *bs);
void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
diff --git a/qemu-img.c b/qemu-img.c
index 49ff068..ccb0468 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1139,6 +1139,7 @@ static int img_convert(int argc, char **argv)
float local_progress = 0;
int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
bool quiet = false;
+ Error *local_err = NULL;
fmt = NULL;
out_fmt = "raw";
@@ -1341,18 +1342,11 @@ static int img_convert(int argc, char **argv)
if (!skip_create) {
/* Create the new image */
- ret = bdrv_create(drv, out_filename, param);
+ ret = bdrv_create(drv, out_filename, param, &local_err);
if (ret < 0) {
- if (ret == -ENOTSUP) {
- error_report("Formatting not supported for file format '%s'",
- out_fmt);
- } else if (ret == -EFBIG) {
- error_report("The image size is too large for file format '%s'",
- out_fmt);
- } else {
- error_report("%s: error while converting %s: %s",
- out_filename, out_fmt, strerror(-ret));
- }
+ error_report("%s: error while converting %s: %s",
+ out_filename, out_fmt, error_get_pretty(local_err));
+ error_free(local_err);
goto out;
}
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 26/33] qemu-img create: Emit filename on error
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (24 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 25/33] block: Error parameter for create functions Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 27/33] qcow2: Use Error parameter Kevin Wolf
` (6 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
bdrv_img_create generally does not emit the target filename, although
this is pretty important information. Therefore, prepend its error
message with the output filename (if an error occurs).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index ccb0468..6a1ba69 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -412,7 +412,7 @@ static int img_create(int argc, char **argv)
bdrv_img_create(filename, fmt, base_filename, base_fmt,
options, img_size, BDRV_O_FLAGS, &local_err, quiet);
if (error_is_set(&local_err)) {
- error_report("%s", error_get_pretty(local_err));
+ error_report("%s: %s", filename, error_get_pretty(local_err));
error_free(local_err);
return 1;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 27/33] qcow2: Use Error parameter
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (25 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 26/33] qemu-img create: Emit filename on error Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 28/33] qemu-iotests: Adjustments due to error propagation Kevin Wolf
` (5 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Employ usage of the new Error ** parameter in qcow2_open, qcow2_create
and associated functions.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 134 ++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 88 insertions(+), 46 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index b257347..318d95d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -79,7 +79,8 @@ static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
* return 0 upon success, non-0 otherwise
*/
static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
- uint64_t end_offset, void **p_feature_table)
+ uint64_t end_offset, void **p_feature_table,
+ Error **errp)
{
BDRVQcowState *s = bs->opaque;
QCowExtension ext;
@@ -100,10 +101,10 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
printf("attempting to read extended header in offset %lu\n", offset);
#endif
- if (bdrv_pread(bs->file, offset, &ext, sizeof(ext)) != sizeof(ext)) {
- fprintf(stderr, "qcow2_read_extension: ERROR: "
- "pread fail from offset %" PRIu64 "\n",
- offset);
+ ret = bdrv_pread(bs->file, offset, &ext, sizeof(ext));
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "qcow2_read_extension: ERROR: "
+ "pread fail from offset %" PRIu64, offset);
return 1;
}
be32_to_cpus(&ext.magic);
@@ -113,7 +114,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
printf("ext.magic = 0x%x\n", ext.magic);
#endif
if (ext.len > end_offset - offset) {
- error_report("Header extension too large");
+ error_setg(errp, "Header extension too large");
return -EINVAL;
}
@@ -123,14 +124,16 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
case QCOW2_EXT_MAGIC_BACKING_FORMAT:
if (ext.len >= sizeof(bs->backing_format)) {
- fprintf(stderr, "ERROR: ext_backing_format: len=%u too large"
- " (>=%zu)\n",
- ext.len, sizeof(bs->backing_format));
+ error_setg(errp, "ERROR: ext_backing_format: len=%u too large"
+ " (>=%zu)", ext.len, sizeof(bs->backing_format));
return 2;
}
- if (bdrv_pread(bs->file, offset , bs->backing_format,
- ext.len) != ext.len)
+ ret = bdrv_pread(bs->file, offset, bs->backing_format, ext.len);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "ERROR: ext_backing_format: "
+ "Could not read format name");
return 3;
+ }
bs->backing_format[ext.len] = '\0';
#ifdef DEBUG_EXT
printf("Qcow2: Got format extension %s\n", bs->backing_format);
@@ -142,6 +145,8 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
void* feature_table = g_malloc0(ext.len + 2 * sizeof(Qcow2Feature));
ret = bdrv_pread(bs->file, offset , feature_table, ext.len);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "ERROR: ext_feature_table: "
+ "Could not read table");
return ret;
}
@@ -161,6 +166,8 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
ret = bdrv_pread(bs->file, offset , uext->data, uext->len);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "ERROR: unknown extension: "
+ "Could not read data");
return ret;
}
}
@@ -184,8 +191,8 @@ static void cleanup_unknown_header_ext(BlockDriverState *bs)
}
}
-static void GCC_FMT_ATTR(2, 3) report_unsupported(BlockDriverState *bs,
- const char *fmt, ...)
+static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs,
+ Error **errp, const char *fmt, ...)
{
char msg[64];
va_list ap;
@@ -194,17 +201,17 @@ static void GCC_FMT_ATTR(2, 3) report_unsupported(BlockDriverState *bs,
vsnprintf(msg, sizeof(msg), fmt, ap);
va_end(ap);
- qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bs->device_name, "qcow2", msg);
+ error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, bs->device_name, "qcow2",
+ msg);
}
static void report_unsupported_feature(BlockDriverState *bs,
- Qcow2Feature *table, uint64_t mask)
+ Error **errp, Qcow2Feature *table, uint64_t mask)
{
while (table && table->name[0] != '\0') {
if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
if (mask & (1 << table->bit)) {
- report_unsupported(bs, "%.46s",table->name);
+ report_unsupported(bs, errp, "%.46s", table->name);
mask &= ~(1 << table->bit);
}
}
@@ -212,7 +219,8 @@ static void report_unsupported_feature(BlockDriverState *bs,
}
if (mask) {
- report_unsupported(bs, "Unknown incompatible feature: %" PRIx64, mask);
+ report_unsupported(bs, errp, "Unknown incompatible feature: %" PRIx64,
+ mask);
}
}
@@ -363,6 +371,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read qcow2 header");
goto fail;
}
be32_to_cpus(&header.magic);
@@ -380,11 +389,12 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
be32_to_cpus(&header.nb_snapshots);
if (header.magic != QCOW_MAGIC) {
+ error_setg(errp, "Image is not in qcow2 format");
ret = -EMEDIUMTYPE;
goto fail;
}
if (header.version < 2 || header.version > 3) {
- report_unsupported(bs, "QCOW version %d", header.version);
+ report_unsupported(bs, errp, "QCOW version %d", header.version);
ret = -ENOTSUP;
goto fail;
}
@@ -412,6 +422,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = bdrv_pread(bs->file, sizeof(header), s->unknown_header_fields,
s->unknown_header_fields_size);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read unknown qcow2 header "
+ "fields");
goto fail;
}
}
@@ -430,8 +442,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
if (s->incompatible_features & ~QCOW2_INCOMPAT_MASK) {
void *feature_table = NULL;
qcow2_read_extensions(bs, header.header_length, ext_end,
- &feature_table);
- report_unsupported_feature(bs, feature_table,
+ &feature_table, NULL);
+ report_unsupported_feature(bs, errp, feature_table,
s->incompatible_features &
~QCOW2_INCOMPAT_MASK);
ret = -ENOTSUP;
@@ -442,8 +454,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
/* Corrupt images may not be written to unless they are being repaired
*/
if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_CHECK)) {
- error_report("qcow2: Image is corrupt; cannot be opened "
- "read/write.");
+ error_setg(errp, "qcow2: Image is corrupt; cannot be opened "
+ "read/write");
ret = -EACCES;
goto fail;
}
@@ -451,7 +463,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
/* Check support for various header values */
if (header.refcount_order != 4) {
- report_unsupported(bs, "%d bit reference counts",
+ report_unsupported(bs, errp, "%d bit reference counts",
1 << header.refcount_order);
ret = -ENOTSUP;
goto fail;
@@ -460,10 +472,13 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
if (header.cluster_bits < MIN_CLUSTER_BITS ||
header.cluster_bits > MAX_CLUSTER_BITS) {
+ error_setg(errp, "Unsupported cluster size: 2^%i", header.cluster_bits);
ret = -EINVAL;
goto fail;
}
if (header.crypt_method > QCOW_CRYPT_AES) {
+ error_setg(errp, "Unsupported encryption method: %i",
+ header.crypt_method);
ret = -EINVAL;
goto fail;
}
@@ -492,6 +507,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
l1_vm_state_index = size_to_l1(s, header.size);
if (l1_vm_state_index > INT_MAX) {
+ error_setg(errp, "Image is too big");
ret = -EFBIG;
goto fail;
}
@@ -500,6 +516,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
/* the L1 table must contain at least enough entries to put
header.size bytes */
if (s->l1_size < s->l1_vm_state_index) {
+ error_setg(errp, "L1 table is too small");
ret = -EINVAL;
goto fail;
}
@@ -510,6 +527,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = bdrv_pread(bs->file, s->l1_table_offset, s->l1_table,
s->l1_size * sizeof(uint64_t));
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read L1 table");
goto fail;
}
for(i = 0;i < s->l1_size; i++) {
@@ -530,6 +548,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = qcow2_refcount_init(bs);
if (ret != 0) {
+ error_setg_errno(errp, -ret, "Could not initialize refcount handling");
goto fail;
}
@@ -537,7 +556,9 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
QTAILQ_INIT(&s->discards);
/* read qcow2 extensions */
- if (qcow2_read_extensions(bs, header.header_length, ext_end, NULL)) {
+ if (qcow2_read_extensions(bs, header.header_length, ext_end, NULL,
+ &local_err)) {
+ error_propagate(errp, local_err);
ret = -EINVAL;
goto fail;
}
@@ -551,6 +572,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = bdrv_pread(bs->file, header.backing_file_offset,
bs->backing_file, len);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read backing file name");
goto fail;
}
bs->backing_file[len] = '\0';
@@ -558,6 +580,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = qcow2_read_snapshots(bs);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not read snapshots");
goto fail;
}
@@ -566,6 +589,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
s->autoclear_features = 0;
ret = qcow2_update_header(bs);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not update qcow2 header");
goto fail;
}
}
@@ -580,6 +604,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not repair dirty image");
goto fail;
}
}
@@ -588,8 +613,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
opts = qemu_opts_create_nofail(&qcow2_runtime_opts);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
- qerror_report_err(local_err);
- error_free(local_err);
+ error_propagate(errp, local_err);
ret = -EINVAL;
goto fail;
}
@@ -610,8 +634,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
qemu_opts_del(opts);
if (s->use_lazy_refcounts && s->qcow_version < 3) {
- qerror_report(ERROR_CLASS_GENERIC_ERROR, "Lazy refcounts require "
- "a qcow2 image with at least qemu 1.1 compatibility level");
+ error_setg(errp, "Lazy refcounts require a qcow2 image with at least "
+ "qemu 1.1 compatibility level");
ret = -EINVAL;
goto fail;
}
@@ -1334,7 +1358,8 @@ static int preallocate(BlockDriverState *bs)
static int qcow2_create2(const char *filename, int64_t total_size,
const char *backing_file, const char *backing_format,
int flags, size_t cluster_size, int prealloc,
- QEMUOptionParameter *options, int version)
+ QEMUOptionParameter *options, int version,
+ Error **errp)
{
/* Calculate cluster_bits */
int cluster_bits;
@@ -1342,9 +1367,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
if (cluster_bits < MIN_CLUSTER_BITS || cluster_bits > MAX_CLUSTER_BITS ||
(1 << cluster_bits) != cluster_size)
{
- error_report(
- "Cluster size must be a power of two between %d and %dk",
- 1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS - 10));
+ error_setg(errp, "Cluster size must be a power of two between %d and "
+ "%dk", 1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS - 10));
return -EINVAL;
}
@@ -1363,15 +1387,18 @@ static int qcow2_create2(const char *filename, int64_t total_size,
BlockDriverState* bs;
QCowHeader header;
uint8_t* refcount_table;
+ Error *local_err = NULL;
int ret;
- ret = bdrv_create_file(filename, options, NULL);
+ ret = bdrv_create_file(filename, options, &local_err);
if (ret < 0) {
+ error_propagate(errp, local_err);
return ret;
}
- ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
+ ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, &local_err);
if (ret < 0) {
+ error_propagate(errp, local_err);
return ret;
}
@@ -1401,6 +1428,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
ret = bdrv_pwrite(bs, 0, &header, sizeof(header));
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not write qcow2 header");
goto out;
}
@@ -1410,6 +1438,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
g_free(refcount_table);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not write refcount table");
goto out;
}
@@ -1423,13 +1452,16 @@ static int qcow2_create2(const char *filename, int64_t total_size,
BlockDriver* drv = bdrv_find_format("qcow2");
assert(drv != NULL);
ret = bdrv_open(bs, filename, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, NULL);
+ BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err);
if (ret < 0) {
+ error_propagate(errp, local_err);
goto out;
}
ret = qcow2_alloc_clusters(bs, 2 * cluster_size);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not allocate clusters for qcow2 "
+ "header and refcount table");
goto out;
} else if (ret != 0) {
@@ -1440,6 +1472,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
/* Okay, now that we have a valid image, let's give it the right size */
ret = bdrv_truncate(bs, total_size * BDRV_SECTOR_SIZE);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not resize image");
goto out;
}
@@ -1447,6 +1480,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
if (backing_file) {
ret = bdrv_change_backing_file(bs, backing_file, backing_format);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not assign backing file '%s' "
+ "with format '%s'", backing_file, backing_format);
goto out;
}
}
@@ -1458,6 +1493,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
ret = preallocate(bs);
qemu_co_mutex_unlock(&s->lock);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Could not preallocate metadata");
goto out;
}
}
@@ -1478,6 +1514,8 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
size_t cluster_size = DEFAULT_CLUSTER_SIZE;
int prealloc = 0;
int version = 3;
+ Error *local_err = NULL;
+ int ret;
/* Read out options */
while (options && options->name) {
@@ -1499,8 +1537,8 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
} else if (!strcmp(options->value.s, "metadata")) {
prealloc = 1;
} else {
- fprintf(stderr, "Invalid preallocation mode: '%s'\n",
- options->value.s);
+ error_setg(errp, "Invalid preallocation mode: '%s'",
+ options->value.s);
return -EINVAL;
}
} else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) {
@@ -1511,8 +1549,8 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
} else if (!strcmp(options->value.s, "1.1")) {
version = 3;
} else {
- fprintf(stderr, "Invalid compatibility level: '%s'\n",
- options->value.s);
+ error_setg(errp, "Invalid compatibility level: '%s'",
+ options->value.s);
return -EINVAL;
}
} else if (!strcmp(options->name, BLOCK_OPT_LAZY_REFCOUNTS)) {
@@ -1522,19 +1560,23 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
}
if (backing_file && prealloc) {
- fprintf(stderr, "Backing file and preallocation cannot be used at "
- "the same time\n");
+ error_setg(errp, "Backing file and preallocation cannot be used at "
+ "the same time");
return -EINVAL;
}
if (version < 3 && (flags & BLOCK_FLAG_LAZY_REFCOUNTS)) {
- fprintf(stderr, "Lazy refcounts only supported with compatibility "
- "level 1.1 and above (use compat=1.1 or greater)\n");
+ error_setg(errp, "Lazy refcounts only supported with compatibility "
+ "level 1.1 and above (use compat=1.1 or greater)");
return -EINVAL;
}
- return qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
- cluster_size, prealloc, options, version);
+ ret = qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
+ cluster_size, prealloc, options, version, &local_err);
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
+ return ret;
}
static int qcow2_make_empty(BlockDriverState *bs)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 28/33] qemu-iotests: Adjustments due to error propagation
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (26 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 27/33] qcow2: Use Error parameter Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:50 ` [Qemu-devel] [PULL 29/33] coroutine: add ./configure --disable-coroutine-pool Kevin Wolf
` (4 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
When opening/creating images, propagating errors instead of immediately
emitting them on occurrence results in errors generally being printed on
a single line rather than being split up into multiple ones. This in
turn requires adjustments to some test results.
Also, test 060 used a sed to filter out the test image directory and
format by removing everything from the affected line after a certain
keyword; this now also removes the error message itself, which can be
fixed by using _filter_testdir and _filter_imgfmt.
Finally, _make_test_img in common.rc did not filter out the test image
directory etc. from stderr. This has been fixed through a redirection of
stderr to stdout (which is already done in _check_test_img and
_img_info).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/049.out | 18 +++++++-----------
tests/qemu-iotests/051.out | 35 ++++++++++++-----------------------
tests/qemu-iotests/054.out | 4 ++--
tests/qemu-iotests/060 | 2 +-
tests/qemu-iotests/060.out | 3 +--
tests/qemu-iotests/common.rc | 2 +-
6 files changed, 24 insertions(+), 40 deletions(-)
diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index d2f0efe..ceb2328 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -96,7 +96,7 @@ qemu-img: Image size must be less than 8 EiB!
qemu-img create -f qcow2 -o size=-1024 TEST_DIR/t.qcow2
qemu-img: qcow2 doesn't support shrinking images yet
-qemu-img: Formatting or formatting option not supported for file format 'qcow2'
+qemu-img: TEST_DIR/t.qcow2: Could not resize image: Operation not supported
Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1k
@@ -104,7 +104,7 @@ qemu-img: Image size must be less than 8 EiB!
qemu-img create -f qcow2 -o size=-1k TEST_DIR/t.qcow2
qemu-img: qcow2 doesn't support shrinking images yet
-qemu-img: Formatting or formatting option not supported for file format 'qcow2'
+qemu-img: TEST_DIR/t.qcow2: Could not resize image: Operation not supported
Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off
qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- 1kilobyte
@@ -120,7 +120,7 @@ qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
qemu-img create -f qcow2 -o size=foobar TEST_DIR/t.qcow2
qemu-img: Parameter 'size' expects a size
-qemu-img: Invalid options for file format 'qcow2'.
+qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'.
== Check correct interpretation of suffixes for cluster size ==
@@ -163,13 +163,11 @@ qemu-img create -f qcow2 -o compat=1.1 TEST_DIR/t.qcow2 64M
Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='1.1' encryption=off cluster_size=65536 lazy_refcounts=off
qemu-img create -f qcow2 -o compat=0.42 TEST_DIR/t.qcow2 64M
-Invalid compatibility level: '0.42'
-qemu-img: TEST_DIR/t.qcow2: error while creating qcow2: Invalid argument
+qemu-img: TEST_DIR/t.qcow2: Invalid compatibility level: '0.42'
Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.42' encryption=off cluster_size=65536 lazy_refcounts=off
qemu-img create -f qcow2 -o compat=foobar TEST_DIR/t.qcow2 64M
-Invalid compatibility level: 'foobar'
-qemu-img: TEST_DIR/t.qcow2: error while creating qcow2: Invalid argument
+qemu-img: TEST_DIR/t.qcow2: Invalid compatibility level: 'foobar'
Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='foobar' encryption=off cluster_size=65536 lazy_refcounts=off
== Check preallocation option ==
@@ -181,8 +179,7 @@ qemu-img create -f qcow2 -o preallocation=metadata TEST_DIR/t.qcow2 64M
Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='metadata' lazy_refcounts=off
qemu-img create -f qcow2 -o preallocation=1234 TEST_DIR/t.qcow2 64M
-Invalid preallocation mode: '1234'
-qemu-img: TEST_DIR/t.qcow2: error while creating qcow2: Invalid argument
+qemu-img: TEST_DIR/t.qcow2: Invalid preallocation mode: '1234'
Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='1234' lazy_refcounts=off
== Check encryption option ==
@@ -205,8 +202,7 @@ qemu-img create -f qcow2 -o compat=0.10,lazy_refcounts=off TEST_DIR/t.qcow2 64M
Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.10' encryption=off cluster_size=65536 lazy_refcounts=off
qemu-img create -f qcow2 -o compat=0.10,lazy_refcounts=on TEST_DIR/t.qcow2 64M
-Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
-qemu-img: TEST_DIR/t.qcow2: error while creating qcow2: Invalid argument
+qemu-img: TEST_DIR/t.qcow2: Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.10' encryption=off cluster_size=65536 lazy_refcounts=on
*** done
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 86e989c..88e8fa7 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -4,20 +4,16 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
=== Unknown option ===
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
=== Enable and disable lazy refcounting on the command line, plus some invalid values ===
@@ -31,24 +27,20 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects 'on' or 'off'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: could not open disk image TEST_DIR/t.qcow2: Parameter 'lazy-refcounts' expects 'on' or 'off'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects 'on' or 'off'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: could not open disk image TEST_DIR/t.qcow2: Parameter 'lazy-refcounts' expects 'on' or 'off'
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects 'on' or 'off'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: could not open disk image TEST_DIR/t.qcow2: Parameter 'lazy-refcounts' expects 'on' or 'off'
=== With version 2 images enabling lazy refcounts must fail ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: Lazy refcounts require a qcow2 image with at least qemu 1.1 compatibility level
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: could not open disk image TEST_DIR/t.qcow2: Lazy refcounts require a qcow2 image with at least qemu 1.1 compatibility level
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=off
QEMU X.Y.Z monitor - type 'help' for more information
@@ -208,21 +200,18 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
Testing: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: Can't use 'qcow2' as a block driver for the protocol level
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: could not open disk image TEST_DIR/t.qcow2: Can't use 'qcow2' as a block driver for the protocol level
=== Parsing protocol from file name ===
Testing: -hda foo:bar
-QEMU_PROG: -hda foo:bar: Unknown protocol
-QEMU_PROG: -hda foo:bar: could not open disk image foo:bar: No such file or directory
+QEMU_PROG: -hda foo:bar: could not open disk image foo:bar: Unknown protocol
Testing: -drive file=foo:bar
-QEMU_PROG: -drive file=foo:bar: Unknown protocol
-QEMU_PROG: -drive file=foo:bar: could not open disk image foo:bar: No such file or directory
+QEMU_PROG: -drive file=foo:bar: could not open disk image foo:bar: Unknown protocol
Testing: -drive file.filename=foo:bar
-QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: No such file or directory
+QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: Could not open 'foo:bar': No such file or directory
*** done
diff --git a/tests/qemu-iotests/054.out b/tests/qemu-iotests/054.out
index 2f357c2..7161d6e 100644
--- a/tests/qemu-iotests/054.out
+++ b/tests/qemu-iotests/054.out
@@ -1,10 +1,10 @@
QA output created by 054
creating too large image (1 EB)
-qemu-img: The image size is too large for file format 'qcow2' (try using a larger cluster size)
+qemu-img: TEST_DIR/t.IMGFMT: The image size is too large for file format 'IMGFMT' (try using a larger cluster size)
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1152921504606846976
creating too large image (1 EB) using qcow2.py
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296
-qemu-img: Could not open 'TEST_DIR/t.qcow2': File too large
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Image is too big
*** done
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 65bb09f..9bbc43b 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -71,7 +71,7 @@ $QEMU_IO -c "write -P 0x2a 0 512" "$TEST_IMG" | _filter_qemu_io
./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
# Try to open the image R/W (which should fail)
-$QEMU_IO -c "read 0 512" "$TEST_IMG" 2>&1 | _filter_qemu_io | sed -e "s/can't open device .*$/can't open device/"
+$QEMU_IO -c "read 0 512" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir | _filter_imgfmt
# Try to open it RO (which should succeed)
$QEMU_IO -c "read 0 512" -r "$TEST_IMG" | _filter_qemu_io
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index ca4583a..648f743 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -11,8 +11,7 @@ incompatible_features 0x0
qcow2: Preventing invalid write on metadata (overlaps with active L1 table); image marked as corrupt.
write failed: Input/output error
incompatible_features 0x2
-qcow2: Image is corrupt; cannot be opened read/write.
-qemu-io: can't open device
+qemu-io: can't open device TEST_DIR/t.IMGFMT: IMGFMT: Image is corrupt; cannot be opened read/write
no file open, try 'help open'
read 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 88fecf7..28b39e4 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -123,7 +123,7 @@ _make_test_img()
fi
# XXX(hch): have global image options?
- $QEMU_IMG create -f $IMGFMT $extra_img_options $img_name $image_size | \
+ $QEMU_IMG create -f $IMGFMT $extra_img_options $img_name $image_size 2>&1 | \
sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
-e "s#$TEST_DIR#TEST_DIR#g" \
-e "s#$IMGFMT#IMGFMT#g" \
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 29/33] coroutine: add ./configure --disable-coroutine-pool
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (27 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 28/33] qemu-iotests: Adjustments due to error propagation Kevin Wolf
@ 2013-09-13 11:50 ` Kevin Wolf
2013-09-13 11:51 ` [Qemu-devel] [PULL 30/33] qemu-img: fix invalid JSON Kevin Wolf
` (3 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:50 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
The 'gthread' coroutine backend was written before the freelist (aka
pool) existed in qemu-coroutine.c.
This means that every thread is expected to exit when its coroutine
terminates. It is not possible to reuse threads from a pool.
This patch automatically disables the pool when 'gthread' is used. This
allows the 'gthread' backend to work again (for example,
tests/test-coroutine completes successfully instead of hanging).
I considered implementing thread reuse but I don't want quirks like CPU
affinity differences due to coroutine threads being recycled. The
'gthread' backend is a reference backend and it's therefore okay to skip
the pool optimization.
Note this patch also makes it easy to toggle the pool for benchmarking
purposes:
./configure --with-coroutine-backend=ucontext \
--disable-coroutine-pool
Reported-by: Gabriel Kerneis <gabriel@kerneis.info>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Gabriel Kerneis <gabriel@kerneis.info>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
configure | 24 ++++++++++++++++++++++++
qemu-coroutine.c | 34 +++++++++++++++++++---------------
2 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/configure b/configure
index b499028..1b6f68b 100755
--- a/configure
+++ b/configure
@@ -238,6 +238,7 @@ win_sdk="no"
want_tools="yes"
libiscsi=""
coroutine=""
+coroutine_pool=""
seccomp=""
glusterfs=""
glusterfs_discard="no"
@@ -888,6 +889,10 @@ for opt do
;;
--with-coroutine=*) coroutine="$optarg"
;;
+ --disable-coroutine-pool) coroutine_pool="no"
+ ;;
+ --enable-coroutine-pool) coroutine_pool="yes"
+ ;;
--disable-docs) docs="no"
;;
--enable-docs) docs="yes"
@@ -1189,6 +1194,8 @@ echo " --disable-seccomp disable seccomp support"
echo " --enable-seccomp enables seccomp support"
echo " --with-coroutine=BACKEND coroutine backend. Supported options:"
echo " gthread, ucontext, sigaltstack, windows"
+echo " --disable-coroutine-pool disable coroutine freelist (worse performance)"
+echo " --enable-coroutine-pool enable coroutine freelist (better performance)"
echo " --enable-glusterfs enable GlusterFS backend"
echo " --disable-glusterfs disable GlusterFS backend"
echo " --enable-gcov enable test coverage analysis with gcov"
@@ -3362,6 +3369,17 @@ else
esac
fi
+if test "$coroutine_pool" = ""; then
+ if test "$coroutine" = "gthread"; then
+ coroutine_pool=no
+ else
+ coroutine_pool=yes
+ fi
+fi
+if test "$coroutine" = "gthread" -a "$coroutine_pool" = "yes"; then
+ error_exit "'gthread' coroutine backend does not support pool (use --disable-coroutine-pool)"
+fi
+
##########################################
# check if we have open_by_handle_at
@@ -3733,6 +3751,7 @@ echo "build guest agent $guest_agent"
echo "QGA VSS support $guest_agent_with_vss"
echo "seccomp support $seccomp"
echo "coroutine backend $coroutine"
+echo "coroutine pool $coroutine_pool"
echo "GlusterFS support $glusterfs"
echo "virtio-blk-data-plane $virtio_blk_data_plane"
echo "gcov $gcov_tool"
@@ -4092,6 +4111,11 @@ if test "$rbd" = "yes" ; then
fi
echo "CONFIG_COROUTINE_BACKEND=$coroutine" >> $config_host_mak
+if test "$coroutine_pool" = "yes" ; then
+ echo "CONFIG_COROUTINE_POOL=1" >> $config_host_mak
+else
+ echo "CONFIG_COROUTINE_POOL=0" >> $config_host_mak
+fi
if test "$open_by_handle_at" = "yes" ; then
echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index 423430d..4708521 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -30,15 +30,17 @@ static unsigned int pool_size;
Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
{
- Coroutine *co;
-
- qemu_mutex_lock(&pool_lock);
- co = QSLIST_FIRST(&pool);
- if (co) {
- QSLIST_REMOVE_HEAD(&pool, pool_next);
- pool_size--;
+ Coroutine *co = NULL;
+
+ if (CONFIG_COROUTINE_POOL) {
+ qemu_mutex_lock(&pool_lock);
+ co = QSLIST_FIRST(&pool);
+ if (co) {
+ QSLIST_REMOVE_HEAD(&pool, pool_next);
+ pool_size--;
+ }
+ qemu_mutex_unlock(&pool_lock);
}
- qemu_mutex_unlock(&pool_lock);
if (!co) {
co = qemu_coroutine_new();
@@ -51,15 +53,17 @@ Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
static void coroutine_delete(Coroutine *co)
{
- qemu_mutex_lock(&pool_lock);
- if (pool_size < POOL_MAX_SIZE) {
- QSLIST_INSERT_HEAD(&pool, co, pool_next);
- co->caller = NULL;
- pool_size++;
+ if (CONFIG_COROUTINE_POOL) {
+ qemu_mutex_lock(&pool_lock);
+ if (pool_size < POOL_MAX_SIZE) {
+ QSLIST_INSERT_HEAD(&pool, co, pool_next);
+ co->caller = NULL;
+ pool_size++;
+ qemu_mutex_unlock(&pool_lock);
+ return;
+ }
qemu_mutex_unlock(&pool_lock);
- return;
}
- qemu_mutex_unlock(&pool_lock);
qemu_coroutine_delete(co);
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 30/33] qemu-img: fix invalid JSON
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (28 preceding siblings ...)
2013-09-13 11:50 ` [Qemu-devel] [PULL 29/33] coroutine: add ./configure --disable-coroutine-pool Kevin Wolf
@ 2013-09-13 11:51 ` Kevin Wolf
2013-09-13 11:51 ` [Qemu-devel] [PULL 31/33] qemu-iotests: Cleanup test image in test number 007 Kevin Wolf
` (2 subsequent siblings)
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:51 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
Single quotes for JSON are a QMP-ism, use real JSON in
qemu-img output.
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-img.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index 6a1ba69..926f0a0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1839,7 +1839,7 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e,
(e->flags & BDRV_BLOCK_ZERO) ? "true" : "false",
(e->flags & BDRV_BLOCK_DATA) ? "true" : "false");
if (e->flags & BDRV_BLOCK_OFFSET_VALID) {
- printf(", 'offset': %"PRId64"", e->offset);
+ printf(", \"offset\": %"PRId64"", e->offset);
}
putchar('}');
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 31/33] qemu-iotests: Cleanup test image in test number 007
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (29 preceding siblings ...)
2013-09-13 11:51 ` [Qemu-devel] [PULL 30/33] qemu-img: fix invalid JSON Kevin Wolf
@ 2013-09-13 11:51 ` Kevin Wolf
2013-09-13 11:51 ` [Qemu-devel] [PULL 32/33] block: Assert validity of BdrvActionOps Kevin Wolf
2013-09-13 11:51 ` [Qemu-devel] [PULL 33/33] qemu-iotests: Fix test 038 Kevin Wolf
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:51 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Bharata B Rao <bharata@linux.vnet.ibm.com>
qemu-iotests number 007 doesn't do test image cleanup. This will affect
those protocols that expect a clean state before every test. Hence
ensure that test image is cleaned up in this test.
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/007 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/007 b/tests/qemu-iotests/007
index c454f2c..6fa7603 100755
--- a/tests/qemu-iotests/007
+++ b/tests/qemu-iotests/007
@@ -30,7 +30,7 @@ status=1 # failure is the default!
_cleanup()
{
-# _cleanup_test_img
+ _cleanup_test_img
true
}
trap "_cleanup; exit \$status" 0 1 2 3 15
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 32/33] block: Assert validity of BdrvActionOps
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (30 preceding siblings ...)
2013-09-13 11:51 ` [Qemu-devel] [PULL 31/33] qemu-iotests: Cleanup test image in test number 007 Kevin Wolf
@ 2013-09-13 11:51 ` Kevin Wolf
2013-09-13 11:51 ` [Qemu-devel] [PULL 33/33] qemu-iotests: Fix test 038 Kevin Wolf
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:51 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
In qmp_transaction, assert that the BdrvActionOps to be used is actually
valid.
This assertion failing is very improbable, however, it might happen, if
a new TransactionActionKind is introduced "out of order" and the
actions[] array is not updated.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index 2ab236a..80605a2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1286,6 +1286,8 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp)
assert(dev_info->kind < ARRAY_SIZE(actions));
ops = &actions[dev_info->kind];
+ assert(ops->instance_size > 0);
+
state = g_malloc0(ops->instance_size);
state->ops = ops;
state->action = dev_info;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PULL 33/33] qemu-iotests: Fix test 038
2013-09-13 11:50 [Qemu-devel] [PULL 00/33] Block patches Kevin Wolf
` (31 preceding siblings ...)
2013-09-13 11:51 ` [Qemu-devel] [PULL 32/33] block: Assert validity of BdrvActionOps Kevin Wolf
@ 2013-09-13 11:51 ` Kevin Wolf
32 siblings, 0 replies; 36+ messages in thread
From: Kevin Wolf @ 2013-09-13 11:51 UTC (permalink / raw)
To: anthony; +Cc: kwolf, qemu-devel
From: Max Reitz <mreitz@redhat.com>
Test 038 uses asynchronous I/O, resulting (potentially) in a different
output for every run (regarding the order of the I/O accesses). This can
be fixed by simply sorting the I/O access messages, since their order is
irrelevant anyway (for this asynchonous I/O).
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/038 | 3 ++-
tests/qemu-iotests/038.out | 10 +++++-----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/tests/qemu-iotests/038 b/tests/qemu-iotests/038
index 36125ea..90de1a7 100755
--- a/tests/qemu-iotests/038
+++ b/tests/qemu-iotests/038
@@ -95,7 +95,8 @@ function overlay_io()
}
overlay_io | $QEMU_IO $TEST_IMG | _filter_qemu_io |\
- sed -e 's/bytes at offset [0-9]*/bytes at offset XXX/g'
+ sed -e 's/bytes at offset [0-9]*/bytes at offset XXX/g' \
+ -e 's/qemu-io> //g' | paste - - | sort | tr '\t' '\n'
echo
echo "== Verify image content =="
diff --git a/tests/qemu-iotests/038.out b/tests/qemu-iotests/038.out
index 9cd0cd8..96c2f84 100644
--- a/tests/qemu-iotests/038.out
+++ b/tests/qemu-iotests/038.out
@@ -517,7 +517,7 @@ qemu-io> wrote 65536/65536 bytes at offset 16711680
qemu-io> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=6442450944 backing_file='TEST_DIR/t.IMGFMT.base'
== Some concurrent requests touching the same cluster ==
-qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> wrote 65536/65536 bytes at offset XXX
+wrote 65536/65536 bytes at offset XXX
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset XXX
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -577,8 +577,6 @@ wrote 65536/65536 bytes at offset XXX
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset XXX
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 81920/81920 bytes at offset XXX
-80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset XXX
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset XXX
@@ -645,8 +643,6 @@ wrote 65536/65536 bytes at offset XXX
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset XXX
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 81920/81920 bytes at offset XXX
-80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset XXX
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset XXX
@@ -705,6 +701,10 @@ wrote 65536/65536 bytes at offset XXX
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset XXX
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset XXX
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset XXX
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== Verify image content ==
qemu-io> read 4096/4096 bytes at offset 2064384
--
1.8.1.4
^ permalink raw reply related [flat|nested] 36+ messages in thread