* [Qemu-devel] [PATCH V2 1/4] snapshot: distinguish id and name in load_tmp
2013-09-22 9:39 [Qemu-devel] [PATCH V2 0/4] export internal snapshot by qemu-nbd Wenchao Xia
@ 2013-09-22 9:39 ` Wenchao Xia
2013-09-22 9:39 ` [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export Wenchao Xia
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Wenchao Xia @ 2013-09-22 9:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, Wenchao Xia, stefanha
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 call the function twice to keep
old search logic.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
block/qcow2-snapshot.c | 16 ++++++++++++++--
block/qcow2.h | 5 ++++-
block/snapshot.c | 37 +++++++++++++++++++++++++++++++++++--
include/block/block_int.h | 4 +++-
include/block/snapshot.h | 4 +++-
qemu-img.c | 14 ++++++++++++--
6 files changed, 71 insertions(+), 9 deletions(-)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 7d14420..fe8f0eb 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -669,7 +669,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;
@@ -677,12 +680,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];
@@ -693,6 +701,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 c90e5d6..12cfeaf 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -472,7 +472,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..0dab05d 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -265,18 +265,51 @@ 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 -ENINVAL.
+ *
+ * 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 do 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;
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3eeb6fe..554cadd 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -160,7 +160,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);
int (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov,
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 012bf22..e50c3bf 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -61,5 +61,7 @@ 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);
#endif
diff --git a/qemu-img.c b/qemu-img.c
index 926f0a0..d1f0e11 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1260,8 +1260,18 @@ 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");
+
+ ret = bdrv_snapshot_load_tmp(bs[0], snapshot_name, NULL, &local_err);
+ if (ret == -ENOENT || ret == -EINVAL) {
+ error_free(local_err);
+ local_err = NULL;
+ ret = bdrv_snapshot_load_tmp(bs[0], NULL, snapshot_name,
+ &local_err);
+ }
+ if (ret < 0) {
+ error_report("Failed to load snapshot, reason: %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
ret = -1;
goto out;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export
2013-09-22 9:39 [Qemu-devel] [PATCH V2 0/4] export internal snapshot by qemu-nbd Wenchao Xia
2013-09-22 9:39 ` [Qemu-devel] [PATCH V2 1/4] snapshot: distinguish id and name in load_tmp Wenchao Xia
@ 2013-09-22 9:39 ` Wenchao Xia
2013-09-23 10:25 ` Paolo Bonzini
2013-09-22 9:39 ` [Qemu-devel] [PATCH V2 3/4] qemu-nbd: add doc for " Wenchao Xia
2013-09-22 9:39 ` [Qemu-devel] [PATCH V2 4/4] qemu-iotests: add 058 internal snapshot export with qemu-nbd case Wenchao Xia
3 siblings, 1 reply; 8+ messages in thread
From: Wenchao Xia @ 2013-09-22 9:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, Wenchao Xia, stefanha
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>
---
qemu-nbd.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 53 insertions(+), 1 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c26c98e..e450d04 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>
@@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
}
}
+#define SNAPSHOT_OPT_ID "id"
+#define SNAPSHOT_OPT_NAME "name"
+
+static QEMUOptionParameter snapshot_options[] = {
+ {
+ .name = SNAPSHOT_OPT_ID,
+ .type = OPT_STRING,
+ .help = "snapshot id"
+ },
+ {
+ .name = SNAPSHOT_OPT_NAME,
+ .type = OPT_STRING,
+ .help = "snapshot name"
+ },
+ { NULL }
+};
+
int main(int argc, char **argv)
{
BlockDriverState *bs;
@@ -315,7 +333,10 @@ 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";
+ QEMUOptionParameter *sn_param = NULL;
+ const QEMUOptionParameter *sn_param_id, *sn_param_name;
+ const char *sn_id = NULL, *sn_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 +349,7 @@ int main(int argc, char **argv)
{ "connect", 1, NULL, 'c' },
{ "disconnect", 0, NULL, 'd' },
{ "snapshot", 0, NULL, 's' },
+ { "snapshot-load", 1, NULL, 'l' },
{ "nocache", 0, NULL, 'n' },
{ "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
#ifdef CONFIG_LINUX_AIO
@@ -428,6 +450,14 @@ int main(int argc, char **argv)
errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
}
break;
+ case 'l':
+ sn_param = parse_option_parameters(optarg,
+ snapshot_options, sn_param);
+ if (!sn_param) {
+ errx(EXIT_FAILURE,
+ "Invalid snapshot-load options '%s'", optarg);
+ }
+ /* fall through */
case 'r':
nbdflags |= NBD_FLAG_READ_ONLY;
flags &= ~BDRV_O_RDWR;
@@ -581,6 +611,24 @@ int main(int argc, char **argv)
error_get_pretty(local_err));
}
+ if (sn_param) {
+ sn_param_id = get_option_parameter(sn_param, SNAPSHOT_OPT_ID);
+ sn_param_name = get_option_parameter(sn_param, SNAPSHOT_OPT_NAME);
+ if (sn_param_id) {
+ sn_id = sn_param_id->value.s;
+ }
+ if (sn_param_name) {
+ sn_name = sn_param_name->value.s;
+ }
+ ret = bdrv_snapshot_load_tmp(bs, sn_id, sn_name, &local_err);
+ if (ret < 0) {
+ errno = -ret;
+ err(EXIT_FAILURE,
+ "Failed to load snapshot, reason:\n%s",
+ error_get_pretty(local_err));
+ }
+ }
+
fd_size = bdrv_getlength(bs);
if (partition != -1) {
@@ -641,6 +689,10 @@ int main(int argc, char **argv)
unlink(sockpath);
}
+ if (sn_param) {
+ free_option_parameters(sn_param);
+ }
+
if (device) {
void *ret;
pthread_join(client_thread, &ret);
--
1.7.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export
2013-09-22 9:39 ` [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export Wenchao Xia
@ 2013-09-23 10:25 ` Paolo Bonzini
2013-09-24 2:56 ` Wenchao Xia
0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2013-09-23 10:25 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, stefanha, qemu-devel
Il 22/09/2013 11:39, Wenchao Xia ha scritto:
> 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>
> ---
> qemu-nbd.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 53 insertions(+), 1 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index c26c98e..e450d04 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>
> @@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
> }
> }
>
> +#define SNAPSHOT_OPT_ID "id"
> +#define SNAPSHOT_OPT_NAME "name"
> +
> +static QEMUOptionParameter snapshot_options[] = {
> + {
> + .name = SNAPSHOT_OPT_ID,
> + .type = OPT_STRING,
> + .help = "snapshot id"
> + },
> + {
> + .name = SNAPSHOT_OPT_NAME,
> + .type = OPT_STRING,
> + .help = "snapshot name"
> + },
> + { NULL }
> +};
I think whatever mechanism you use here to pick a snapshot id or name
should be implemented in qemu-img too.
Also, I think QEMUOptionParameter is being phased out.
> int main(int argc, char **argv)
> {
> BlockDriverState *bs;
> @@ -315,7 +333,10 @@ 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";
> + QEMUOptionParameter *sn_param = NULL;
> + const QEMUOptionParameter *sn_param_id, *sn_param_name;
> + const char *sn_id = NULL, *sn_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 +349,7 @@ int main(int argc, char **argv)
> { "connect", 1, NULL, 'c' },
> { "disconnect", 0, NULL, 'd' },
> { "snapshot", 0, NULL, 's' },
> + { "snapshot-load", 1, NULL, 'l' },
Please call this option "load-snapshot".
Paolo
> { "nocache", 0, NULL, 'n' },
> { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
> #ifdef CONFIG_LINUX_AIO
> @@ -428,6 +450,14 @@ int main(int argc, char **argv)
> errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
> }
> break;
> + case 'l':
> + sn_param = parse_option_parameters(optarg,
> + snapshot_options, sn_param);
> + if (!sn_param) {
> + errx(EXIT_FAILURE,
> + "Invalid snapshot-load options '%s'", optarg);
> + }
> + /* fall through */
> case 'r':
> nbdflags |= NBD_FLAG_READ_ONLY;
> flags &= ~BDRV_O_RDWR;
> @@ -581,6 +611,24 @@ int main(int argc, char **argv)
> error_get_pretty(local_err));
> }
>
> + if (sn_param) {
> + sn_param_id = get_option_parameter(sn_param, SNAPSHOT_OPT_ID);
> + sn_param_name = get_option_parameter(sn_param, SNAPSHOT_OPT_NAME);
> + if (sn_param_id) {
> + sn_id = sn_param_id->value.s;
> + }
> + if (sn_param_name) {
> + sn_name = sn_param_name->value.s;
> + }
> + ret = bdrv_snapshot_load_tmp(bs, sn_id, sn_name, &local_err);
> + if (ret < 0) {
> + errno = -ret;
> + err(EXIT_FAILURE,
> + "Failed to load snapshot, reason:\n%s",
> + error_get_pretty(local_err));
> + }
> + }
> +
> fd_size = bdrv_getlength(bs);
>
> if (partition != -1) {
> @@ -641,6 +689,10 @@ int main(int argc, char **argv)
> unlink(sockpath);
> }
>
> + if (sn_param) {
> + free_option_parameters(sn_param);
> + }
> +
> if (device) {
> void *ret;
> pthread_join(client_thread, &ret);
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export
2013-09-23 10:25 ` Paolo Bonzini
@ 2013-09-24 2:56 ` Wenchao Xia
2013-09-24 8:11 ` Paolo Bonzini
0 siblings, 1 reply; 8+ messages in thread
From: Wenchao Xia @ 2013-09-24 2:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kwolf, stefanha, qemu-devel
于 2013/9/23 18:25, Paolo Bonzini 写道:
> Il 22/09/2013 11:39, Wenchao Xia ha scritto:
>> 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>
>> ---
>> qemu-nbd.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 files changed, 53 insertions(+), 1 deletions(-)
>>
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index c26c98e..e450d04 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>
>> @@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
>> }
>> }
>>
>> +#define SNAPSHOT_OPT_ID "id"
>> +#define SNAPSHOT_OPT_NAME "name"
>> +
>> +static QEMUOptionParameter snapshot_options[] = {
>> + {
>> + .name = SNAPSHOT_OPT_ID,
>> + .type = OPT_STRING,
>> + .help = "snapshot id"
>> + },
>> + {
>> + .name = SNAPSHOT_OPT_NAME,
>> + .type = OPT_STRING,
>> + .help = "snapshot name"
>> + },
>> + { NULL }
>> +};
> I think whatever mechanism you use here to pick a snapshot id or name
> should be implemented in qemu-img too.
qemu-img already pick up snapshot by mixed id and name, do you like to
add a new
interface like the above(Keep old interface untouched for compatiablity)?
> Also, I think QEMUOptionParameter is being phased out.
>
Is QemuOptsList the recommanded method?
>> int main(int argc, char **argv)
>> {
>> BlockDriverState *bs;
>> @@ -315,7 +333,10 @@ 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";
>> + QEMUOptionParameter *sn_param = NULL;
>> + const QEMUOptionParameter *sn_param_id, *sn_param_name;
>> + const char *sn_id = NULL, *sn_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 +349,7 @@ int main(int argc, char **argv)
>> { "connect", 1, NULL, 'c' },
>> { "disconnect", 0, NULL, 'd' },
>> { "snapshot", 0, NULL, 's' },
>> + { "snapshot-load", 1, NULL, 'l' },
> Please call this option "load-snapshot".
>
OK.
> Paolo
>
>> { "nocache", 0, NULL, 'n' },
>> { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
>> #ifdef CONFIG_LINUX_AIO
>> @@ -428,6 +450,14 @@ int main(int argc, char **argv)
>> errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
>> }
>> break;
>> + case 'l':
>> + sn_param = parse_option_parameters(optarg,
>> + snapshot_options, sn_param);
>> + if (!sn_param) {
>> + errx(EXIT_FAILURE,
>> + "Invalid snapshot-load options '%s'", optarg);
>> + }
>> + /* fall through */
>> case 'r':
>> nbdflags |= NBD_FLAG_READ_ONLY;
>> flags&= ~BDRV_O_RDWR;
>> @@ -581,6 +611,24 @@ int main(int argc, char **argv)
>> error_get_pretty(local_err));
>> }
>>
>> + if (sn_param) {
>> + sn_param_id = get_option_parameter(sn_param, SNAPSHOT_OPT_ID);
>> + sn_param_name = get_option_parameter(sn_param, SNAPSHOT_OPT_NAME);
>> + if (sn_param_id) {
>> + sn_id = sn_param_id->value.s;
>> + }
>> + if (sn_param_name) {
>> + sn_name = sn_param_name->value.s;
>> + }
>> + ret = bdrv_snapshot_load_tmp(bs, sn_id, sn_name,&local_err);
>> + if (ret< 0) {
>> + errno = -ret;
>> + err(EXIT_FAILURE,
>> + "Failed to load snapshot, reason:\n%s",
>> + error_get_pretty(local_err));
>> + }
>> + }
>> +
>> fd_size = bdrv_getlength(bs);
>>
>> if (partition != -1) {
>> @@ -641,6 +689,10 @@ int main(int argc, char **argv)
>> unlink(sockpath);
>> }
>>
>> + if (sn_param) {
>> + free_option_parameters(sn_param);
>> + }
>> +
>> if (device) {
>> void *ret;
>> pthread_join(client_thread,&ret);
>>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export
2013-09-24 2:56 ` Wenchao Xia
@ 2013-09-24 8:11 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2013-09-24 8:11 UTC (permalink / raw)
To: Wenchao Xia; +Cc: kwolf, stefanha, qemu-devel
Il 24/09/2013 04:56, Wenchao Xia ha scritto:
> 于 2013/9/23 18:25, Paolo Bonzini 写道:
>> Il 22/09/2013 11:39, Wenchao Xia ha scritto:
>>> 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>
>>> ---
>>> qemu-nbd.c | 54
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>> 1 files changed, 53 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>>> index c26c98e..e450d04 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>
>>> @@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
>>> }
>>> }
>>>
>>> +#define SNAPSHOT_OPT_ID "id"
>>> +#define SNAPSHOT_OPT_NAME "name"
>>> +
>>> +static QEMUOptionParameter snapshot_options[] = {
>>> + {
>>> + .name = SNAPSHOT_OPT_ID,
>>> + .type = OPT_STRING,
>>> + .help = "snapshot id"
>>> + },
>>> + {
>>> + .name = SNAPSHOT_OPT_NAME,
>>> + .type = OPT_STRING,
>>> + .help = "snapshot name"
>>> + },
>>> + { NULL }
>>> +};
>> I think whatever mechanism you use here to pick a snapshot id or name
>> should be implemented in qemu-img too.
> qemu-img already pick up snapshot by mixed id and name, do you like to
> add a new
> interface like the above(Keep old interface untouched for compatiablity)?
Yes, please. And also implement the "mixed" method here.
>> Also, I think QEMUOptionParameter is being phased out.
>>
> Is QemuOptsList the recommanded method?
Yes.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH V2 3/4] qemu-nbd: add doc for internal snapshot export
2013-09-22 9:39 [Qemu-devel] [PATCH V2 0/4] export internal snapshot by qemu-nbd Wenchao Xia
2013-09-22 9:39 ` [Qemu-devel] [PATCH V2 1/4] snapshot: distinguish id and name in load_tmp Wenchao Xia
2013-09-22 9:39 ` [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export Wenchao Xia
@ 2013-09-22 9:39 ` Wenchao Xia
2013-09-22 9:39 ` [Qemu-devel] [PATCH V2 4/4] qemu-iotests: add 058 internal snapshot export with qemu-nbd case Wenchao Xia
3 siblings, 0 replies; 8+ messages in thread
From: Wenchao Xia @ 2013-09-22 9:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, Wenchao Xia, stefanha
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
qemu-nbd.c | 8 +++++++-
qemu-nbd.texi | 8 +++++++-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index e450d04..8cb4bf1 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -80,7 +80,13 @@ 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, --snapshot-load=PARAM\n"
+" load an internal snapshot inside FILE and export it\n"
+" as an read-only device, PARAM format is\n"
+" 'id=[ID],name=[NAME]'\n"
" -n, --nocache disable host cache\n"
" --cache=MODE set cache mode (none, writeback, ...)\n"
#ifdef CONFIG_LINUX_AIO
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 6055ec6..69e9e9a 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, --snapshot-load=@var{param}
+ load an internal snapshot inside @var{filename} and export it
+ as an read-only device, @var{param} format is
+ 'id=[ID],name=[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] 8+ messages in thread* [Qemu-devel] [PATCH V2 4/4] qemu-iotests: add 058 internal snapshot export with qemu-nbd case
2013-09-22 9:39 [Qemu-devel] [PATCH V2 0/4] export internal snapshot by qemu-nbd Wenchao Xia
` (2 preceding siblings ...)
2013-09-22 9:39 ` [Qemu-devel] [PATCH V2 3/4] qemu-nbd: add doc for " Wenchao Xia
@ 2013-09-22 9:39 ` Wenchao Xia
3 siblings, 0 replies; 8+ messages in thread
From: Wenchao Xia @ 2013-09-22 9:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, pbonzini, Wenchao Xia, stefanha
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
tests/qemu-iotests/058 | 87 ++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/058.out | 26 +++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 114 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..301ef1f
--- /dev/null
+++ b/tests/qemu-iotests/058
@@ -0,0 +1,87 @@
+#!/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()
+{
+ eval "$QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port $TEST_IMG -l 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
+
+# Any format supporting intenal snapshots
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+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 foo $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 foo
+
+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..b174f3b
--- /dev/null
+++ b/tests/qemu-iotests/058.out
@@ -0,0 +1,26 @@
+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)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1ad02e5..2793e1d 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] 8+ messages in thread