[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V5 04/13] block: add snapshot info query functio
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH V5 04/13] block: add snapshot info query function bdrv_query_snapshot_infolist() |
Date: |
Tue, 29 Jan 2013 13:48:17 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 |
Am 24.01.2013 03:57, schrieb Wenchao Xia:
> This patch add function bdrv_query_snapshot_infolist(), which will
> return snapshot info of an image in qmp object format. The implementation
> code are mostly copied from qemu-img.c with modification to fit more
> for qmp based block layer API.
> To help filter out snapshot info not needed, a call back function is
> added in bdrv_query_snapshot_infolist().
> bdrv_can_read_snapshot() should be called before call this function,
> to avoid got *errp set unexpectedly.
>
> Signed-off-by: Wenchao Xia <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
> block.c | 60
> +++++++++++++++++++++++++++++++++++++++++++++++++
> include/block/block.h | 7 +++++
> 2 files changed, 67 insertions(+), 0 deletions(-)
>
> diff --git a/block.c b/block.c
> index 934bb3f..7cdb6c6 100644
> --- a/block.c
> +++ b/block.c
> @@ -2842,6 +2842,66 @@ int coroutine_fn
> bdrv_co_is_allocated_above(BlockDriverState *top,
> return 0;
> }
>
> +SnapshotInfoList *bdrv_query_snapshot_infolist(BlockDriverState *bs,
> + SnapshotFilterFunc filter,
This filter things looks overengineered to me, and no patch in this
series really uses it. Do we really need it?
> + void *opaque,
> + Error **errp)
> +{
> + int i, sn_count;
> + QEMUSnapshotInfo *sn_tab = NULL;
> + SnapshotInfoList *info_list, *cur_item = NULL, *head = NULL;
> +
> + sn_count = bdrv_snapshot_list(bs, &sn_tab);
> + if (sn_count < 0) {
> + /* Now bdrv_snapshot_list() use negative value to tip error, so a
> check
> + * of it was done here. In future errp can be set by that function
> + * itself, by changing the call back functions in c files in ./block.
> + */
> + const char *dev = bdrv_get_device_name(bs);
> + if (sn_count == -ENOMEDIUM) {
> + error_setg(errp, "Device '%s' is not inserted.", dev);
> + } else if (sn_count == -ENOTSUP) {
> + error_setg(errp,
> + "Device '%s' does not support internal snapshot.",
> + dev);
> + } else {
> + error_setg(errp,
> + "Device '%s' got %d for bdrv_snapshot_list().",
> + dev, sn_count);
strerror(-sn_count) would make the error message more useful.
You could also consider using switch instead of an if/else if chain.
> + }
> + return NULL;
> + }
> +
> + for (i = 0; i < sn_count; i++) {
> + if (filter && filter(&sn_tab[i], opaque) != 0) {
> + continue;
> + }
> +
> + info_list = g_new0(SnapshotInfoList, 1);
> +
> + info_list->value = g_new0(SnapshotInfo, 1);
> + info_list->value->id = g_strdup(sn_tab[i].id_str);
> + info_list->value->name = g_strdup(sn_tab[i].name);
> + info_list->value->vm_state_size = sn_tab[i].vm_state_size;
> + info_list->value->date_sec = sn_tab[i].date_sec;
> + info_list->value->date_nsec = sn_tab[i].date_nsec;
> + info_list->value->vm_clock_sec = sn_tab[i].vm_clock_nsec /
> 1000000000;
> + info_list->value->vm_clock_nsec = sn_tab[i].vm_clock_nsec %
> 1000000000;
Did I already mention somewhere that I'm a fan of compound literals?
Looks so much nicer than repeating info_list->value-> in each line. ;-)
Kevin
- [Qemu-devel] [PATCH V5 01/13] qemu-img: remove unused parameter in collect_image_info(), (continued)
- [Qemu-devel] [PATCH V5 04/13] block: add snapshot info query function bdrv_query_snapshot_infolist(), Wenchao Xia, 2013/01/23
- Re: [Qemu-devel] [PATCH V5 04/13] block: add snapshot info query function bdrv_query_snapshot_infolist(),
Kevin Wolf <=
- [Qemu-devel] [PATCH V5 05/13] block: add image info query function bdrv_query_image_info(), Wenchao Xia, 2013/01/23
- [Qemu-devel] [PATCH V5 06/13] qemu-img: switch image retrieving function, Wenchao Xia, 2013/01/23
- [Qemu-devel] [PATCH V5 07/13] block: rename bdrv_query_info to bdrv_query_block_info, Wenchao Xia, 2013/01/23