* [Qemu-devel] [PATCH V5 1/5] snapshot: distinguish id and name in load_tmp
2013-11-10 22:03 [Qemu-devel] [PATCH V5 0/5] export internal snapshot by qemu-nbd Wenchao Xia
@ 2013-11-10 22:03 ` Wenchao Xia
2013-11-19 11:20 ` Kevin Wolf
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 2/5] qemu-nbd: support internal snapshot export Wenchao Xia
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Wenchao Xia @ 2013-11-10 22:03 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia
Since later this function will be used so improve it. The only caller of it
now is qemu-img, and it is not impacted by introduce function
bdrv_snapshot_load_tmp_by_id_or_name() that call bdrv_snapshot_load_tmp()
twice to keep old search logic. bdrv_snapshot_load_tmp_by_id_or_name() return
int to let caller know the errno, and errno will be used later.
Also fix a typo in comments of bdrv_snapshot_delete().
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
block/qcow2-snapshot.c | 16 ++++++++++-
block/qcow2.h | 5 +++-
block/snapshot.c | 60 ++++++++++++++++++++++++++++++++++++++++++--
include/block/block_int.h | 4 ++-
include/block/snapshot.h | 7 ++++-
qemu-img.c | 8 ++++-
6 files changed, 90 insertions(+), 10 deletions(-)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 3529c68..aeb5efd 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -675,7 +675,10 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
return s->nb_snapshots;
}
-int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
+int qcow2_snapshot_load_tmp(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp)
{
int i, snapshot_index;
BDRVQcowState *s = bs->opaque;
@@ -683,12 +686,17 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
uint64_t *new_l1_table;
int new_l1_bytes;
int ret;
+ const char *device = bdrv_get_device_name(bs);
assert(bs->read_only);
/* Search the snapshot */
- snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_name);
+ snapshot_index = find_snapshot_by_id_and_name(bs, snapshot_id, name);
if (snapshot_index < 0) {
+ error_setg(errp,
+ "Can't find a snapshot with ID '%s' and name '%s' "
+ "on device '%s'",
+ STR_OR_NULL(snapshot_id), STR_OR_NULL(name), device);
return -ENOENT;
}
sn = &s->snapshots[snapshot_index];
@@ -699,6 +707,10 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
ret = bdrv_pread(bs->file, sn->l1_table_offset, new_l1_table, new_l1_bytes);
if (ret < 0) {
+ error_setg(errp,
+ "Failed to read l1 table for snapshot with ID '%s' and name "
+ "'%s' on device '%s'",
+ sn->id_str, sn->name, device);
g_free(new_l1_table);
return ret;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index 922e190..303eb26 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -488,7 +488,10 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
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);
+int qcow2_snapshot_load_tmp(BlockDriverState *bs,
+ const char *snapshot_id,
+ const char *name,
+ Error **errp);
void qcow2_free_snapshots(BlockDriverState *bs);
int qcow2_read_snapshots(BlockDriverState *bs);
diff --git a/block/snapshot.c b/block/snapshot.c
index a05c0c0..e51a7db 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -194,7 +194,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
* 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.
+ * if none is specified, return -EINVAL.
*
* 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
@@ -265,18 +265,72 @@ int bdrv_snapshot_list(BlockDriverState *bs,
return -ENOTSUP;
}
+/**
+ * Temporarily load 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, load the first one with
+ * id @snapshot_id and name @name.
+ * If only @snapshot_id is specified, load the first one with id
+ * @snapshot_id.
+ * If only @name is specified, load the first one with name @name.
+ * if none is specified, return -EINVAL.
+ *
+ * Returns: 0 on success, -errno on fail. If @bs is not inserted, return
+ * -ENOMEDIUM. If @bs is not readonly, return -EINVAL. If @bs did not support
+ * internal snapshot, return -ENOTSUP. If qemu can't find one matching @id and
+ * @name, return -ENOENT. If @bs does not support parameter @snapshot_id or
+ * @name, return -EINVAL. If @errp != NULL, it will always be filled on
+ * failure.
+ */
int bdrv_snapshot_load_tmp(BlockDriverState *bs,
- const char *snapshot_name)
+ const char *snapshot_id,
+ const char *name,
+ Error **errp)
{
BlockDriver *drv = bs->drv;
+ const char *device = bdrv_get_device_name(bs);
if (!drv) {
+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
return -ENOMEDIUM;
}
+ if (!snapshot_id && !name) {
+ error_setg(errp, "snapshot_id and name are both NULL");
+ return -EINVAL;
+ }
if (!bs->read_only) {
+ error_setg(errp, "Device '%s' is not readonly", device);
return -EINVAL;
}
if (drv->bdrv_snapshot_load_tmp) {
- return drv->bdrv_snapshot_load_tmp(bs, snapshot_name);
+ return drv->bdrv_snapshot_load_tmp(bs, snapshot_id, name, errp);
}
+ error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+ drv->format_name, device,
+ "temporarily load internal snapshot");
return -ENOTSUP;
}
+
+int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
+ const char *id_or_name,
+ Error **errp)
+{
+ int ret;
+ Error *local_err = NULL;
+
+ ret = bdrv_snapshot_load_tmp(bs, id_or_name, NULL, &local_err);
+ if (ret == -ENOENT || ret == -EINVAL) {
+ error_free(local_err);
+ local_err = NULL;
+ ret = bdrv_snapshot_load_tmp(bs, NULL, id_or_name, &local_err);
+ }
+
+ if (error_is_set(&local_err)) {
+ error_propagate(errp, local_err);
+ }
+
+ return ret;
+}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1666066..8bbfb09 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -175,7 +175,9 @@ struct BlockDriver {
int (*bdrv_snapshot_list)(BlockDriverState *bs,
QEMUSnapshotInfo **psn_info);
int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs,
- const char *snapshot_name);
+ const char *snapshot_id,
+ const char *name,
+ Error **errp);
int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs);
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 012bf22..d05bea7 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -61,5 +61,10 @@ void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
int bdrv_snapshot_list(BlockDriverState *bs,
QEMUSnapshotInfo **psn_info);
int bdrv_snapshot_load_tmp(BlockDriverState *bs,
- const char *snapshot_name);
+ const char *snapshot_id,
+ const char *name,
+ Error **errp);
+int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
+ const char *id_or_name,
+ Error **errp);
#endif
diff --git a/qemu-img.c b/qemu-img.c
index bf3fb4f..fe28119 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1260,8 +1260,12 @@ static int img_convert(int argc, char **argv)
ret = -1;
goto out;
}
- if (bdrv_snapshot_load_tmp(bs[0], snapshot_name) < 0) {
- error_report("Failed to load snapshot");
+
+ bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err);
+ if (error_is_set(&local_err)) {
+ error_report("Failed to load snapshot: %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
ret = -1;
goto out;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH V5 1/5] snapshot: distinguish id and name in load_tmp
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 1/5] snapshot: distinguish id and name in load_tmp Wenchao Xia
@ 2013-11-19 11:20 ` Kevin Wolf
2013-11-22 1:52 ` Wenchao Xia
0 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2013-11-19 11:20 UTC (permalink / raw)
To: Wenchao Xia; +Cc: pbonzini, jcody, qemu-devel, stefanha
Am 10.11.2013 um 23:03 hat Wenchao Xia geschrieben:
> Since later this function will be used so improve it. The only caller of it
> now is qemu-img, and it is not impacted by introduce function
> bdrv_snapshot_load_tmp_by_id_or_name() that call bdrv_snapshot_load_tmp()
> twice to keep old search logic. bdrv_snapshot_load_tmp_by_id_or_name() return
> int to let caller know the errno, and errno will be used later.
> Also fix a typo in comments of bdrv_snapshot_delete().
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> block/qcow2-snapshot.c | 16 ++++++++++-
> block/qcow2.h | 5 +++-
> block/snapshot.c | 60 ++++++++++++++++++++++++++++++++++++++++++--
> include/block/block_int.h | 4 ++-
> include/block/snapshot.h | 7 ++++-
> qemu-img.c | 8 ++++-
> 6 files changed, 90 insertions(+), 10 deletions(-)
>
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 3529c68..aeb5efd 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -675,7 +675,10 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
> return s->nb_snapshots;
> }
>
> -int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
> +int qcow2_snapshot_load_tmp(BlockDriverState *bs,
> + const char *snapshot_id,
> + const char *name,
> + Error **errp)
> {
> int i, snapshot_index;
> BDRVQcowState *s = bs->opaque;
> @@ -683,12 +686,17 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
> uint64_t *new_l1_table;
> int new_l1_bytes;
> int ret;
> + const char *device = bdrv_get_device_name(bs);
This is wrong, low-level functions shouldn't need the device name for
anything.
> assert(bs->read_only);
>
> /* Search the snapshot */
> - snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_name);
> + snapshot_index = find_snapshot_by_id_and_name(bs, snapshot_id, name);
> if (snapshot_index < 0) {
> + error_setg(errp,
> + "Can't find a snapshot with ID '%s' and name '%s' "
> + "on device '%s'",
> + STR_OR_NULL(snapshot_id), STR_OR_NULL(name), device);
> return -ENOENT;
> }
> sn = &s->snapshots[snapshot_index];
I think we already discussed the same thing in the context of a
different series: The caller knows which device and which snapshot name
or ID he used. The only information he really needs is:
error_setg(errp, "Can't find snapshot");
If in the context of the caller's caller this isn't enough, the caller
can add this information. I doubt that it's even necessary in this case.
> @@ -699,6 +707,10 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
>
> ret = bdrv_pread(bs->file, sn->l1_table_offset, new_l1_table, new_l1_bytes);
> if (ret < 0) {
> + error_setg(errp,
> + "Failed to read l1 table for snapshot with ID '%s' and name "
> + "'%s' on device '%s'",
> + sn->id_str, sn->name, device);
> g_free(new_l1_table);
> return ret;
> }
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 922e190..303eb26 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -488,7 +488,10 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
> 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);
> +int qcow2_snapshot_load_tmp(BlockDriverState *bs,
> + const char *snapshot_id,
> + const char *name,
> + Error **errp);
>
> void qcow2_free_snapshots(BlockDriverState *bs);
> int qcow2_read_snapshots(BlockDriverState *bs);
> diff --git a/block/snapshot.c b/block/snapshot.c
> index a05c0c0..e51a7db 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -194,7 +194,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
> * 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.
> + * if none is specified, return -EINVAL.
> *
> * 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
> @@ -265,18 +265,72 @@ int bdrv_snapshot_list(BlockDriverState *bs,
> return -ENOTSUP;
> }
>
> +/**
> + * Temporarily load 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, load the first one with
> + * id @snapshot_id and name @name.
> + * If only @snapshot_id is specified, load the first one with id
> + * @snapshot_id.
> + * If only @name is specified, load the first one with name @name.
> + * if none is specified, return -EINVAL.
> + *
> + * Returns: 0 on success, -errno on fail. If @bs is not inserted, return
> + * -ENOMEDIUM. If @bs is not readonly, return -EINVAL. If @bs did not support
> + * internal snapshot, return -ENOTSUP. If qemu can't find one matching @id and
s/one/a/
> + * @name, return -ENOENT. If @bs does not support parameter @snapshot_id or
> + * @name, return -EINVAL.
What do you mean by "bs does not support parameter"? Is this when you
specify a name, but the block driver doesn't use names, but only IDs?
> If @errp != NULL, it will always be filled on
> + * failure.
> + */
The rest looks good.
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH V5 1/5] snapshot: distinguish id and name in load_tmp
2013-11-19 11:20 ` Kevin Wolf
@ 2013-11-22 1:52 ` Wenchao Xia
0 siblings, 0 replies; 13+ messages in thread
From: Wenchao Xia @ 2013-11-22 1:52 UTC (permalink / raw)
To: Kevin Wolf; +Cc: pbonzini, jcody, qemu-devel, stefanha
于 2013/11/19 19:20, Kevin Wolf 写道:
> Am 10.11.2013 um 23:03 hat Wenchao Xia geschrieben:
>> Since later this function will be used so improve it. The only caller of it
>> now is qemu-img, and it is not impacted by introduce function
>> bdrv_snapshot_load_tmp_by_id_or_name() that call bdrv_snapshot_load_tmp()
>> twice to keep old search logic. bdrv_snapshot_load_tmp_by_id_or_name() return
>> int to let caller know the errno, and errno will be used later.
>> Also fix a typo in comments of bdrv_snapshot_delete().
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>> block/qcow2-snapshot.c | 16 ++++++++++-
>> block/qcow2.h | 5 +++-
>> block/snapshot.c | 60 ++++++++++++++++++++++++++++++++++++++++++--
>> include/block/block_int.h | 4 ++-
>> include/block/snapshot.h | 7 ++++-
>> qemu-img.c | 8 ++++-
>> 6 files changed, 90 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
>> index 3529c68..aeb5efd 100644
>> --- a/block/qcow2-snapshot.c
>> +++ b/block/qcow2-snapshot.c
>> @@ -675,7 +675,10 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
>> return s->nb_snapshots;
>> }
>>
>> -int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
>> +int qcow2_snapshot_load_tmp(BlockDriverState *bs,
>> + const char *snapshot_id,
>> + const char *name,
>> + Error **errp)
>> {
>> int i, snapshot_index;
>> BDRVQcowState *s = bs->opaque;
>> @@ -683,12 +686,17 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
>> uint64_t *new_l1_table;
>> int new_l1_bytes;
>> int ret;
>> + const char *device = bdrv_get_device_name(bs);
>
> This is wrong, low-level functions shouldn't need the device name for
> anything.
>
>> assert(bs->read_only);
>>
>> /* Search the snapshot */
>> - snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_name);
>> + snapshot_index = find_snapshot_by_id_and_name(bs, snapshot_id, name);
>> if (snapshot_index < 0) {
>> + error_setg(errp,
>> + "Can't find a snapshot with ID '%s' and name '%s' "
>> + "on device '%s'",
>> + STR_OR_NULL(snapshot_id), STR_OR_NULL(name), device);
>> return -ENOENT;
>> }
>> sn = &s->snapshots[snapshot_index];
>
> I think we already discussed the same thing in the context of a
> different series: The caller knows which device and which snapshot name
> or ID he used. The only information he really needs is:
>
> error_setg(errp, "Can't find snapshot");
>
> If in the context of the caller's caller this isn't enough, the caller
> can add this information. I doubt that it's even necessary in this case.
>
I remember that, will follow this rule.
>> @@ -699,6 +707,10 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
>>
>> ret = bdrv_pread(bs->file, sn->l1_table_offset, new_l1_table, new_l1_bytes);
>> if (ret < 0) {
>> + error_setg(errp,
>> + "Failed to read l1 table for snapshot with ID '%s' and name "
>> + "'%s' on device '%s'",
>> + sn->id_str, sn->name, device);
>> g_free(new_l1_table);
>> return ret;
>> }
>> diff --git a/block/qcow2.h b/block/qcow2.h
>> index 922e190..303eb26 100644
>> --- a/block/qcow2.h
>> +++ b/block/qcow2.h
>> @@ -488,7 +488,10 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
>> 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);
>> +int qcow2_snapshot_load_tmp(BlockDriverState *bs,
>> + const char *snapshot_id,
>> + const char *name,
>> + Error **errp);
>>
>> void qcow2_free_snapshots(BlockDriverState *bs);
>> int qcow2_read_snapshots(BlockDriverState *bs);
>> diff --git a/block/snapshot.c b/block/snapshot.c
>> index a05c0c0..e51a7db 100644
>> --- a/block/snapshot.c
>> +++ b/block/snapshot.c
>> @@ -194,7 +194,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
>> * 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.
>> + * if none is specified, return -EINVAL.
>> *
>> * 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
>> @@ -265,18 +265,72 @@ int bdrv_snapshot_list(BlockDriverState *bs,
>> return -ENOTSUP;
>> }
>>
>> +/**
>> + * Temporarily load 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, load the first one with
>> + * id @snapshot_id and name @name.
>> + * If only @snapshot_id is specified, load the first one with id
>> + * @snapshot_id.
>> + * If only @name is specified, load the first one with name @name.
>> + * if none is specified, return -EINVAL.
>> + *
>> + * Returns: 0 on success, -errno on fail. If @bs is not inserted, return
>> + * -ENOMEDIUM. If @bs is not readonly, return -EINVAL. If @bs did not support
>> + * internal snapshot, return -ENOTSUP. If qemu can't find one matching @id and
>
> s/one/a/
>
OK.
>> + * @name, return -ENOENT. If @bs does not support parameter @snapshot_id or
>> + * @name, return -EINVAL.
>
> What do you mean by "bs does not support parameter"? Is this when you
> specify a name, but the block driver doesn't use names, but only IDs?
>
likely, I mean rbd doesn't support ID. But rbd and snapshot doesn't
implement load_tmp, so will remove this comments.
>> If @errp != NULL, it will always be filled on
>> + * failure.
>> + */
>
> The rest looks good.
>
> Kevin
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH V5 2/5] qemu-nbd: support internal snapshot export
2013-11-10 22:03 [Qemu-devel] [PATCH V5 0/5] export internal snapshot by qemu-nbd Wenchao Xia
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 1/5] snapshot: distinguish id and name in load_tmp Wenchao Xia
@ 2013-11-10 22:03 ` Wenchao Xia
2013-11-19 11:26 ` Kevin Wolf
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 3/5] qemu-iotests: add 058 internal snapshot export with qemu-nbd case Wenchao Xia
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Wenchao Xia @ 2013-11-10 22:03 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia
Now it is possible to directly export an internal snapshot, which
can be used to probe the snapshot's contents without qemu-img
convert.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
block/snapshot.c | 18 +++++++++++++++++
include/block/snapshot.h | 8 +++++++
qemu-nbd.c | 47 ++++++++++++++++++++++++++++++++++++++++++++-
qemu-nbd.texi | 8 ++++++-
4 files changed, 78 insertions(+), 3 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index e51a7db..7cc45fa 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -25,6 +25,24 @@
#include "block/snapshot.h"
#include "block/block_int.h"
+QemuOptsList internal_snapshot_opts = {
+ .name = "snapshot",
+ .head = QTAILQ_HEAD_INITIALIZER(internal_snapshot_opts.head),
+ .desc = {
+ {
+ .name = SNAPSHOT_OPT_ID,
+ .type = QEMU_OPT_STRING,
+ .help = "snapshot id"
+ },{
+ .name = SNAPSHOT_OPT_NAME,
+ .type = QEMU_OPT_STRING,
+ .help = "snapshot name"
+ },{
+ /* end of list */
+ }
+ },
+};
+
int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
const char *name)
{
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index d05bea7..770d9bb 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -27,6 +27,14 @@
#include "qemu-common.h"
#include "qapi/error.h"
+#include "qemu/option.h"
+
+
+#define SNAPSHOT_OPT_BASE "snapshot."
+#define SNAPSHOT_OPT_ID "snapshot.id"
+#define SNAPSHOT_OPT_NAME "snapshot.name"
+
+extern QemuOptsList internal_snapshot_opts;
typedef struct QEMUSnapshotInfo {
char id_str[128]; /* unique snapshot id */
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c26c98e..f934eaa 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -20,6 +20,7 @@
#include "block/block.h"
#include "block/nbd.h"
#include "qemu/main-loop.h"
+#include "block/snapshot.h"
#include <stdarg.h>
#include <stdio.h>
@@ -79,7 +80,14 @@ static void usage(const char *name)
"\n"
"Block device options:\n"
" -r, --read-only export read-only\n"
-" -s, --snapshot use snapshot file\n"
+" -s, --snapshot use FILE as an external snapshot, create a temporary\n"
+" file with backing_file=FILE, redirect the write to\n"
+" the temporary one\n"
+" -l, --load-snapshot=SNAPSHOT_PARAM\n"
+" load an internal snapshot inside FILE and export it\n"
+" as an read-only device, SNAPSHOT_PARAM format is\n"
+" 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
+" '[ID_OR_NAME]'\n"
" -n, --nocache disable host cache\n"
" --cache=MODE set cache mode (none, writeback, ...)\n"
#ifdef CONFIG_LINUX_AIO
@@ -315,7 +323,9 @@ int main(int argc, char **argv)
char *device = NULL;
int port = NBD_DEFAULT_PORT;
off_t fd_size;
- const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
+ QemuOpts *sn_opts = NULL;
+ const char *sn_id_or_name = NULL;
+ const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
struct option lopt[] = {
{ "help", 0, NULL, 'h' },
{ "version", 0, NULL, 'V' },
@@ -328,6 +338,7 @@ int main(int argc, char **argv)
{ "connect", 1, NULL, 'c' },
{ "disconnect", 0, NULL, 'd' },
{ "snapshot", 0, NULL, 's' },
+ { "load-snapshot", 1, NULL, 'l' },
{ "nocache", 0, NULL, 'n' },
{ "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
#ifdef CONFIG_LINUX_AIO
@@ -428,6 +439,18 @@ int main(int argc, char **argv)
errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
}
break;
+ case 'l':
+ if (strncmp(optarg, SNAPSHOT_OPT_BASE, strlen(SNAPSHOT_OPT_BASE))
+ == 0) {
+ sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
+ if (!sn_opts) {
+ errx(EXIT_FAILURE, "Failed in parsing snapshot param `%s'",
+ optarg);
+ }
+ } else {
+ sn_id_or_name = optarg;
+ }
+ /* fall through */
case 'r':
nbdflags |= NBD_FLAG_READ_ONLY;
flags &= ~BDRV_O_RDWR;
@@ -581,6 +604,22 @@ int main(int argc, char **argv)
error_get_pretty(local_err));
}
+ if (sn_opts) {
+ ret = bdrv_snapshot_load_tmp(bs,
+ qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
+ qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME),
+ &local_err);
+ } else if (sn_id_or_name) {
+ ret = bdrv_snapshot_load_tmp_by_id_or_name(bs, sn_id_or_name,
+ &local_err);
+ }
+ if (ret < 0) {
+ errno = -ret;
+ err(EXIT_FAILURE,
+ "Failed to load snapshot: %s",
+ error_get_pretty(local_err));
+ }
+
fd_size = bdrv_getlength(bs);
if (partition != -1) {
@@ -641,6 +680,10 @@ int main(int argc, char **argv)
unlink(sockpath);
}
+ if (sn_opts) {
+ qemu_opts_del(sn_opts);
+ }
+
if (device) {
void *ret;
pthread_join(client_thread, &ret);
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 6055ec6..5b55f76 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -27,7 +27,13 @@ Export QEMU disk image using NBD protocol.
@item -P, --partition=@var{num}
only expose partition @var{num}
@item -s, --snapshot
- use snapshot file
+ use @var{filename} as an external snapshot, create a temporary
+ file with backing_file=@var{filename}, redirect the write to
+ the temporary one
+@item -l, --load-snapshot=@var{snapshot_param}
+ load an internal snapshot inside @var{filename} and export it
+ as an read-only device, @var{snapshot_param} format is
+ 'snapshot.id=[ID],snapshot.name=[NAME]' or '[ID_OR_NAME]'
@item -n, --nocache
@itemx --cache=@var{cache}
set cache mode to be used with the file. See the documentation of
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH V5 2/5] qemu-nbd: support internal snapshot export
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 2/5] qemu-nbd: support internal snapshot export Wenchao Xia
@ 2013-11-19 11:26 ` Kevin Wolf
2013-11-22 1:53 ` Wenchao Xia
0 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2013-11-19 11:26 UTC (permalink / raw)
To: Wenchao Xia; +Cc: pbonzini, jcody, qemu-devel, stefanha
Am 10.11.2013 um 23:03 hat Wenchao Xia geschrieben:
> Now it is possible to directly export an internal snapshot, which
> can be used to probe the snapshot's contents without qemu-img
> convert.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> block/snapshot.c | 18 +++++++++++++++++
> include/block/snapshot.h | 8 +++++++
> qemu-nbd.c | 47 ++++++++++++++++++++++++++++++++++++++++++++-
> qemu-nbd.texi | 8 ++++++-
> 4 files changed, 78 insertions(+), 3 deletions(-)
>
> diff --git a/block/snapshot.c b/block/snapshot.c
> index e51a7db..7cc45fa 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -25,6 +25,24 @@
> #include "block/snapshot.h"
> #include "block/block_int.h"
>
> +QemuOptsList internal_snapshot_opts = {
> + .name = "snapshot",
> + .head = QTAILQ_HEAD_INITIALIZER(internal_snapshot_opts.head),
> + .desc = {
> + {
> + .name = SNAPSHOT_OPT_ID,
> + .type = QEMU_OPT_STRING,
> + .help = "snapshot id"
> + },{
> + .name = SNAPSHOT_OPT_NAME,
> + .type = QEMU_OPT_STRING,
> + .help = "snapshot name"
> + },{
> + /* end of list */
> + }
> + },
> +};
> +
> int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
> const char *name)
> {
> diff --git a/include/block/snapshot.h b/include/block/snapshot.h
> index d05bea7..770d9bb 100644
> --- a/include/block/snapshot.h
> +++ b/include/block/snapshot.h
> @@ -27,6 +27,14 @@
>
> #include "qemu-common.h"
> #include "qapi/error.h"
> +#include "qemu/option.h"
> +
> +
> +#define SNAPSHOT_OPT_BASE "snapshot."
> +#define SNAPSHOT_OPT_ID "snapshot.id"
> +#define SNAPSHOT_OPT_NAME "snapshot.name"
> +
> +extern QemuOptsList internal_snapshot_opts;
>
> typedef struct QEMUSnapshotInfo {
> char id_str[128]; /* unique snapshot id */
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index c26c98e..f934eaa 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -20,6 +20,7 @@
> #include "block/block.h"
> #include "block/nbd.h"
> #include "qemu/main-loop.h"
> +#include "block/snapshot.h"
>
> #include <stdarg.h>
> #include <stdio.h>
> @@ -79,7 +80,14 @@ static void usage(const char *name)
> "\n"
> "Block device options:\n"
> " -r, --read-only export read-only\n"
> -" -s, --snapshot use snapshot file\n"
> +" -s, --snapshot use FILE as an external snapshot, create a temporary\n"
> +" file with backing_file=FILE, redirect the write to\n"
> +" the temporary one\n"
> +" -l, --load-snapshot=SNAPSHOT_PARAM\n"
> +" load an internal snapshot inside FILE and export it\n"
> +" as an read-only device, SNAPSHOT_PARAM format is\n"
> +" 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
> +" '[ID_OR_NAME]'\n"
> " -n, --nocache disable host cache\n"
> " --cache=MODE set cache mode (none, writeback, ...)\n"
> #ifdef CONFIG_LINUX_AIO
> @@ -315,7 +323,9 @@ int main(int argc, char **argv)
> char *device = NULL;
> int port = NBD_DEFAULT_PORT;
> off_t fd_size;
> - const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
> + QemuOpts *sn_opts = NULL;
> + const char *sn_id_or_name = NULL;
> + const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
> struct option lopt[] = {
> { "help", 0, NULL, 'h' },
> { "version", 0, NULL, 'V' },
> @@ -328,6 +338,7 @@ int main(int argc, char **argv)
> { "connect", 1, NULL, 'c' },
> { "disconnect", 0, NULL, 'd' },
> { "snapshot", 0, NULL, 's' },
> + { "load-snapshot", 1, NULL, 'l' },
> { "nocache", 0, NULL, 'n' },
> { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
> #ifdef CONFIG_LINUX_AIO
> @@ -428,6 +439,18 @@ int main(int argc, char **argv)
> errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
> }
> break;
> + case 'l':
> + if (strncmp(optarg, SNAPSHOT_OPT_BASE, strlen(SNAPSHOT_OPT_BASE))
> + == 0) {
You can avoid this ugly line break by using strstart():
if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
...
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH V5 2/5] qemu-nbd: support internal snapshot export
2013-11-19 11:26 ` Kevin Wolf
@ 2013-11-22 1:53 ` Wenchao Xia
0 siblings, 0 replies; 13+ messages in thread
From: Wenchao Xia @ 2013-11-22 1:53 UTC (permalink / raw)
To: Kevin Wolf; +Cc: pbonzini, jcody, qemu-devel, stefanha
于 2013/11/19 19:26, Kevin Wolf 写道:
> Am 10.11.2013 um 23:03 hat Wenchao Xia geschrieben:
>> Now it is possible to directly export an internal snapshot, which
>> can be used to probe the snapshot's contents without qemu-img
>> convert.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>> block/snapshot.c | 18 +++++++++++++++++
>> include/block/snapshot.h | 8 +++++++
>> qemu-nbd.c | 47 ++++++++++++++++++++++++++++++++++++++++++++-
>> qemu-nbd.texi | 8 ++++++-
>> 4 files changed, 78 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/snapshot.c b/block/snapshot.c
>> index e51a7db..7cc45fa 100644
>> --- a/block/snapshot.c
>> +++ b/block/snapshot.c
>> @@ -25,6 +25,24 @@
>> #include "block/snapshot.h"
>> #include "block/block_int.h"
>>
>> +QemuOptsList internal_snapshot_opts = {
>> + .name = "snapshot",
>> + .head = QTAILQ_HEAD_INITIALIZER(internal_snapshot_opts.head),
>> + .desc = {
>> + {
>> + .name = SNAPSHOT_OPT_ID,
>> + .type = QEMU_OPT_STRING,
>> + .help = "snapshot id"
>> + },{
>> + .name = SNAPSHOT_OPT_NAME,
>> + .type = QEMU_OPT_STRING,
>> + .help = "snapshot name"
>> + },{
>> + /* end of list */
>> + }
>> + },
>> +};
>> +
>> int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
>> const char *name)
>> {
>> diff --git a/include/block/snapshot.h b/include/block/snapshot.h
>> index d05bea7..770d9bb 100644
>> --- a/include/block/snapshot.h
>> +++ b/include/block/snapshot.h
>> @@ -27,6 +27,14 @@
>>
>> #include "qemu-common.h"
>> #include "qapi/error.h"
>> +#include "qemu/option.h"
>> +
>> +
>> +#define SNAPSHOT_OPT_BASE "snapshot."
>> +#define SNAPSHOT_OPT_ID "snapshot.id"
>> +#define SNAPSHOT_OPT_NAME "snapshot.name"
>> +
>> +extern QemuOptsList internal_snapshot_opts;
>>
>> typedef struct QEMUSnapshotInfo {
>> char id_str[128]; /* unique snapshot id */
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index c26c98e..f934eaa 100644
>> --- a/qemu-nbd.c
>> +++ b/qemu-nbd.c
>> @@ -20,6 +20,7 @@
>> #include "block/block.h"
>> #include "block/nbd.h"
>> #include "qemu/main-loop.h"
>> +#include "block/snapshot.h"
>>
>> #include <stdarg.h>
>> #include <stdio.h>
>> @@ -79,7 +80,14 @@ static void usage(const char *name)
>> "\n"
>> "Block device options:\n"
>> " -r, --read-only export read-only\n"
>> -" -s, --snapshot use snapshot file\n"
>> +" -s, --snapshot use FILE as an external snapshot, create a temporary\n"
>> +" file with backing_file=FILE, redirect the write to\n"
>> +" the temporary one\n"
>> +" -l, --load-snapshot=SNAPSHOT_PARAM\n"
>> +" load an internal snapshot inside FILE and export it\n"
>> +" as an read-only device, SNAPSHOT_PARAM format is\n"
>> +" 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
>> +" '[ID_OR_NAME]'\n"
>> " -n, --nocache disable host cache\n"
>> " --cache=MODE set cache mode (none, writeback, ...)\n"
>> #ifdef CONFIG_LINUX_AIO
>> @@ -315,7 +323,9 @@ int main(int argc, char **argv)
>> char *device = NULL;
>> int port = NBD_DEFAULT_PORT;
>> off_t fd_size;
>> - const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
>> + QemuOpts *sn_opts = NULL;
>> + const char *sn_id_or_name = NULL;
>> + const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
>> struct option lopt[] = {
>> { "help", 0, NULL, 'h' },
>> { "version", 0, NULL, 'V' },
>> @@ -328,6 +338,7 @@ int main(int argc, char **argv)
>> { "connect", 1, NULL, 'c' },
>> { "disconnect", 0, NULL, 'd' },
>> { "snapshot", 0, NULL, 's' },
>> + { "load-snapshot", 1, NULL, 'l' },
>> { "nocache", 0, NULL, 'n' },
>> { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
>> #ifdef CONFIG_LINUX_AIO
>> @@ -428,6 +439,18 @@ int main(int argc, char **argv)
>> errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
>> }
>> break;
>> + case 'l':
>> + if (strncmp(optarg, SNAPSHOT_OPT_BASE, strlen(SNAPSHOT_OPT_BASE))
>> + == 0) {
>
> You can avoid this ugly line break by using strstart():
>
> if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
> ...
>
> Kevin
>
Will use strstart(), thanks for tipping.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH V5 3/5] qemu-iotests: add 058 internal snapshot export with qemu-nbd case
2013-11-10 22:03 [Qemu-devel] [PATCH V5 0/5] export internal snapshot by qemu-nbd Wenchao Xia
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 1/5] snapshot: distinguish id and name in load_tmp Wenchao Xia
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 2/5] qemu-nbd: support internal snapshot export Wenchao Xia
@ 2013-11-10 22:03 ` Wenchao Xia
2013-11-19 11:29 ` Kevin Wolf
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 4/5] qemu-img: add -l for snapshot in convert Wenchao Xia
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Wenchao Xia @ 2013-11-10 22:03 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
tests/qemu-iotests/058 | 102 ++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/058.out | 32 ++++++++++++++
tests/qemu-iotests/check | 1 +
tests/qemu-iotests/group | 1 +
4 files changed, 136 insertions(+), 0 deletions(-)
create mode 100755 tests/qemu-iotests/058
create mode 100644 tests/qemu-iotests/058.out
diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
new file mode 100755
index 0000000..2d50d97
--- /dev/null
+++ b/tests/qemu-iotests/058
@@ -0,0 +1,102 @@
+#!/bin/bash
+#
+# Test export internal snapshot by qemu-nbd.
+#
+# Copyright (C) 2013 IBM, Inc.
+#
+# Based on 029.
+#
+# 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=xiawenc@linux.vnet.ibm.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+nbd_snapshot_port=10850
+nbd_snapshot_img="nbd:127.0.0.1:$nbd_snapshot_port"
+
+_export_nbd_snapshot()
+{
+ $QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l $1 &
+ NBD_SNAPSHOT_PID=$!
+ sleep 1
+}
+
+_export_nbd_snapshot1()
+{
+ $QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l snapshot.name=$1 &
+ NBD_SNAPSHOT_PID=$!
+ sleep 1
+}
+
+_cleanup()
+{
+ if [ -n "$NBD_SNAPSHOT_PID" ]; then
+ kill $NBD_SNAPSHOT_PID
+ fi
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+_supported_fmt qcow2
+_supported_proto generic
+
+echo
+echo "== preparing image =="
+_make_test_img 64M
+$QEMU_IO -c 'write -P 0xa 0x1000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'write -P 0xb 0x2000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c sn1 "$TEST_IMG"
+$QEMU_IO -c 'write -P 0xc 0x1000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'write -P 0xd 0x2000 0x1000' "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "== verifying the image file with patterns =="
+$QEMU_IO -c 'read -P 0xc 0x1000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xd 0x2000 0x1000' "$TEST_IMG" | _filter_qemu_io
+
+_export_nbd_snapshot sn1
+
+echo
+echo "== verifying the exported snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+
+kill $NBD_SNAPSHOT_PID
+sleep 1
+
+_export_nbd_snapshot1 sn1
+
+echo
+echo "== verifying the exported snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/058.out b/tests/qemu-iotests/058.out
new file mode 100644
index 0000000..cc4b8ca
--- /dev/null
+++ b/tests/qemu-iotests/058.out
@@ -0,0 +1,32 @@
+QA output created by 058
+
+== preparing image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+== verifying the image file with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the exported snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the exported snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index f5f328f..f879474 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -161,6 +161,7 @@ cat <<EOF
QEMU -- $QEMU
QEMU_IMG -- $QEMU_IMG
QEMU_IO -- $QEMU_IO
+QEMU_NBD -- $QEMU_NBD
IMGFMT -- $FULL_IMGFMT_DETAILS
IMGPROTO -- $FULL_IMGPROTO_DETAILS
PLATFORM -- $FULL_HOST_DETAILS
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index c57ff35..d0534ed 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -64,6 +64,7 @@
055 rw auto
056 rw auto backing
057 rw auto
+058 rw auto
059 rw auto
060 rw auto
061 rw auto
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH V5 3/5] qemu-iotests: add 058 internal snapshot export with qemu-nbd case
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 3/5] qemu-iotests: add 058 internal snapshot export with qemu-nbd case Wenchao Xia
@ 2013-11-19 11:29 ` Kevin Wolf
2013-11-22 6:49 ` Wenchao Xia
0 siblings, 1 reply; 13+ messages in thread
From: Kevin Wolf @ 2013-11-19 11:29 UTC (permalink / raw)
To: Wenchao Xia; +Cc: pbonzini, jcody, qemu-devel, stefanha
Am 10.11.2013 um 23:03 hat Wenchao Xia geschrieben:
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> tests/qemu-iotests/058 | 102 ++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/058.out | 32 ++++++++++++++
> tests/qemu-iotests/check | 1 +
> tests/qemu-iotests/group | 1 +
> 4 files changed, 136 insertions(+), 0 deletions(-)
> create mode 100755 tests/qemu-iotests/058
> create mode 100644 tests/qemu-iotests/058.out
I think this should have:
_supported_proto nbd
Or otherwise the check in common needs to be changed. At least for me
the test failed without an error message because I didn't have a
qemu-nbd symlink in place.
Kevin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH V5 3/5] qemu-iotests: add 058 internal snapshot export with qemu-nbd case
2013-11-19 11:29 ` Kevin Wolf
@ 2013-11-22 6:49 ` Wenchao Xia
0 siblings, 0 replies; 13+ messages in thread
From: Wenchao Xia @ 2013-11-22 6:49 UTC (permalink / raw)
To: Kevin Wolf; +Cc: pbonzini, jcody, qemu-devel, stefanha
于 2013/11/19 19:29, Kevin Wolf 写道:
> Am 10.11.2013 um 23:03 hat Wenchao Xia geschrieben:
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>> tests/qemu-iotests/058 | 102 ++++++++++++++++++++++++++++++++++++++++++++
>> tests/qemu-iotests/058.out | 32 ++++++++++++++
>> tests/qemu-iotests/check | 1 +
>> tests/qemu-iotests/group | 1 +
>> 4 files changed, 136 insertions(+), 0 deletions(-)
>> create mode 100755 tests/qemu-iotests/058
>> create mode 100644 tests/qemu-iotests/058.out
>
> I think this should have:
>
> _supported_proto nbd
>
> Or otherwise the check in common needs to be changed. At least for me
> the test failed without an error message because I didn't have a
> qemu-nbd symlink in place.
>
> Kevin
>
I found a little problem:
$QEMU_IMG snapshot -c sn1 "$TEST_IMG"
can't work when IMGPROTO=nbd, since in common.rc the image was not
exported as raw:
if [ $IMGPROTO = "nbd" ]; then
eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 $TEST_IMG_FILE &"
QEMU_NBD_PID=$!
sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
fi
So later the test case think $TEST_IMG as a raw image. All case used
$QEMU_IMG snapshot will fail, such as 015? Do you think this is a bug?
Instead if change common.rc to exported as raw(I am not sure whether
it is done on purpose), how about add a check called:
_required_util
in common.rc?
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH V5 4/5] qemu-img: add -l for snapshot in convert
2013-11-10 22:03 [Qemu-devel] [PATCH V5 0/5] export internal snapshot by qemu-nbd Wenchao Xia
` (2 preceding siblings ...)
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 3/5] qemu-iotests: add 058 internal snapshot export with qemu-nbd case Wenchao Xia
@ 2013-11-10 22:03 ` Wenchao Xia
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 5/5] qemu-iotests: add test for snapshot in qemu-img convert Wenchao Xia
2013-11-14 2:42 ` [Qemu-devel] [PATCH V5 0/5] export internal snapshot by qemu-nbd Wenchao Xia
5 siblings, 0 replies; 13+ messages in thread
From: Wenchao Xia @ 2013-11-10 22:03 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia
Now qemu-img convert have similar options as qemu-nbd for internal
snapshot.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
qemu-img-cmds.hx | 2 +-
qemu-img.c | 45 ++++++++++++++++++++++++++++++++++++---------
qemu-img.texi | 12 ++++++++----
3 files changed, 45 insertions(+), 14 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index da1d965..9a8153b 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,7 +34,7 @@ STEXI
ETEXI
DEF("convert", img_convert,
- "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
+ "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
STEXI
@item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index fe28119..e9f5e3a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -93,6 +93,11 @@ static void help(void)
" 'options' is a comma separated list of format specific options in a\n"
" name=value format. Use -o ? for an overview of the options supported by the\n"
" used format\n"
+ " 'snapshot_param' is param used for internal snapshot, format\n"
+ " is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
+ " '[ID_OR_NAME]'\n"
+ " 'snapshot_id_or_name' is deprecated, use 'snapshot_param'\n"
+ " instead\n"
" '-c' indicates that target image must be compressed (qcow format only)\n"
" '-u' enables unsafe rebasing. It is assumed that old and new backing file\n"
" match exactly. The image doesn't need a working backing file before\n"
@@ -1140,6 +1145,7 @@ static int img_convert(int argc, char **argv)
int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
bool quiet = false;
Error *local_err = NULL;
+ QemuOpts *sn_opts = NULL;
fmt = NULL;
out_fmt = "raw";
@@ -1148,7 +1154,7 @@ static int img_convert(int argc, char **argv)
compress = 0;
skip_create = 0;
for(;;) {
- c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qn");
+ c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qnl:");
if (c == -1) {
break;
}
@@ -1183,6 +1189,19 @@ static int img_convert(int argc, char **argv)
case 's':
snapshot_name = optarg;
break;
+ case 'l':
+ if (strncmp(optarg, SNAPSHOT_OPT_BASE, strlen(SNAPSHOT_OPT_BASE))
+ == 0) {
+ sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
+ if (!sn_opts) {
+ error_report("Failed in parsing snapshot param '%s'",
+ optarg);
+ return 1;
+ }
+ } else {
+ snapshot_name = optarg;
+ }
+ break;
case 'S':
{
int64_t sval;
@@ -1254,7 +1273,12 @@ static int img_convert(int argc, char **argv)
total_sectors += bs_sectors;
}
- if (snapshot_name != NULL) {
+ if (sn_opts) {
+ ret = bdrv_snapshot_load_tmp(bs[0],
+ qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
+ qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME),
+ &local_err);
+ } else if (snapshot_name != NULL) {
if (bs_n > 1) {
error_report("No support for concatenating multiple snapshot");
ret = -1;
@@ -1262,13 +1286,13 @@ static int img_convert(int argc, char **argv)
}
bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err);
- if (error_is_set(&local_err)) {
- error_report("Failed to load snapshot: %s",
- error_get_pretty(local_err));
- error_free(local_err);
- ret = -1;
- goto out;
- }
+ }
+ if (error_is_set(&local_err)) {
+ error_report("Failed to load snapshot: %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
+ ret = -1;
+ goto out;
}
/* Find driver and parse its options */
@@ -1559,6 +1583,9 @@ out:
free_option_parameters(create_options);
free_option_parameters(param);
qemu_vfree(buf);
+ if (sn_opts) {
+ qemu_opts_del(sn_opts);
+ }
if (out_bs) {
bdrv_unref(out_bs);
}
diff --git a/qemu-img.texi b/qemu-img.texi
index 768054e..fbdbfa7 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -46,7 +46,11 @@ is the destination disk image filename
is a comma separated list of format specific options in a
name=value format. Use @code{-o ?} for an overview of the options supported
by the used format or see the format descriptions below for details.
-
+@item snapshot_param
+is param used for internal snapshot, format is
+'snapshot.id=[ID],snapshot.name=[NAME]' or '[ID_OR_NAME]'
+@item snapshot_id_or_name
+is deprecated, use snapshot_param instead
@item -c
indicates that target image must be compressed (qcow format only)
@@ -179,10 +183,10 @@ Error on reading data
@end table
-@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
-Convert the disk image @var{filename} or a snapshot @var{snapshot_name} to disk image @var{output_filename}
-using format @var{output_fmt}. It can be optionally compressed (@code{-c}
+Convert the disk image @var{filename} or a snapshot @var{snapshot_param}(@var{snapshot_id_or_name} is deprecated)
+to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c}
option) or use any format specific options like encryption (@code{-o} option).
Only the formats @code{qcow} and @code{qcow2} support compression. The
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH V5 5/5] qemu-iotests: add test for snapshot in qemu-img convert
2013-11-10 22:03 [Qemu-devel] [PATCH V5 0/5] export internal snapshot by qemu-nbd Wenchao Xia
` (3 preceding siblings ...)
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 4/5] qemu-img: add -l for snapshot in convert Wenchao Xia
@ 2013-11-10 22:03 ` Wenchao Xia
2013-11-14 2:42 ` [Qemu-devel] [PATCH V5 0/5] export internal snapshot by qemu-nbd Wenchao Xia
5 siblings, 0 replies; 13+ messages in thread
From: Wenchao Xia @ 2013-11-10 22:03 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
tests/qemu-iotests/058 | 19 ++++++++++++++++++-
tests/qemu-iotests/058.out | 12 ++++++++++++
2 files changed, 30 insertions(+), 1 deletions(-)
diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
index 2d50d97..9e28132 100755
--- a/tests/qemu-iotests/058
+++ b/tests/qemu-iotests/058
@@ -1,6 +1,6 @@
#!/bin/bash
#
-# Test export internal snapshot by qemu-nbd.
+# Test export internal snapshot by qemu-nbd, convert it by qemu-img.
#
# Copyright (C) 2013 IBM, Inc.
#
@@ -33,6 +33,8 @@ status=1 # failure is the default!
nbd_snapshot_port=10850
nbd_snapshot_img="nbd:127.0.0.1:$nbd_snapshot_port"
+converted_image=$TEST_IMG.converted
+
_export_nbd_snapshot()
{
$QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l $1 &
@@ -53,6 +55,7 @@ _cleanup()
kill $NBD_SNAPSHOT_PID
fi
_cleanup_test_img
+ rm -f "$converted_image"
}
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -96,6 +99,20 @@ echo "== verifying the exported snapshot with patterns =="
$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IMG convert "$TEST_IMG" -l sn1 -O qcow2 "$converted_image"
+
+echo
+echo "== verifying the converted snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$converted_image" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$converted_image" | _filter_qemu_io
+
+$QEMU_IMG convert "$TEST_IMG" -l snapshot.name=sn1 -O qcow2 "$converted_image"
+
+echo
+echo "== verifying the converted snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$converted_image" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$converted_image" | _filter_qemu_io
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/058.out b/tests/qemu-iotests/058.out
index cc4b8ca..a8381b9 100644
--- a/tests/qemu-iotests/058.out
+++ b/tests/qemu-iotests/058.out
@@ -29,4 +29,16 @@ read 4096/4096 bytes at offset 4096
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 4096/4096 bytes at offset 8192
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the converted snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the converted snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
*** done
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCH V5 0/5] export internal snapshot by qemu-nbd
2013-11-10 22:03 [Qemu-devel] [PATCH V5 0/5] export internal snapshot by qemu-nbd Wenchao Xia
` (4 preceding siblings ...)
2013-11-10 22:03 ` [Qemu-devel] [PATCH V5 5/5] qemu-iotests: add test for snapshot in qemu-img convert Wenchao Xia
@ 2013-11-14 2:42 ` Wenchao Xia
5 siblings, 0 replies; 13+ messages in thread
From: Wenchao Xia @ 2013-11-14 2:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia
Hello, any more comments?
^ permalink raw reply [flat|nested] 13+ messages in thread