δΊ 2013-4-10 23:19, Markus Armbruster ει:
> Wenchao Xia <xiaw...@linux.vnet.ibm.com> writes:
>
>> This patch adds a parameter to tell whether return valid snapshots
>> for whole VM only.
>>
>> Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com>
>> Reviewed-by: Eric Blake <ebl...@redhat.com>
>> Reviewed-by: Kevin Wolf <kw...@redhat.com>
>> ---
>> block/qapi.c | 39 +++++++++++++++++++++++++++++++++++++--
>> include/block/qapi.h | 1 +
>> qemu-img.c | 3 ++-
>> 3 files changed, 40 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/qapi.c b/block/qapi.c
>> index 03369a5..19d4d93 100644
>> --- a/block/qapi.c
>> +++ b/block/qapi.c
>> @@ -23,14 +23,47 @@
>> */
>>
>> #include "block/qapi.h"
>> +#include "block/snapshot.h"
>> #include "block/block_int.h"
>>
>> /*
>> + * check whether the snapshot is valid for whole vm.
>> + *
>> + * @sn: snapshot info to be checked.
>> + * @bs: where @sn was found.
>> + *
>> + * return true if the snapshot is consistent for the VM.
>> + */
>> +static bool snapshot_valid_for_vm(const QEMUSnapshotInfo *sn,
>> + BlockDriverState *bs)
>> +{
>> + BlockDriverState *bs1 = NULL;
>> + QEMUSnapshotInfo s, *sn_info = &s;
>> + int ret;
>> +
>> + /* Check logic is connected with load_vmstate():
>> + Only check the devices that can snapshot, other devices that can't
>> + take snapshot, for example, readonly ones, will be ignored in
>> + load_vmstate(). */
>> + while ((bs1 = bdrv_next(bs1))) {
>
> Unlike load_vmstate(), you don't do
>
> if (!bdrv_is_inserted(bs1) || bdrv_is_read_only(bs1)) {
> continue;
> }
>
> Why?
>
It is copied from do_info_snapshots() and forgot to check it in
load_vmstate(), I think that is not correct, will use the exactly same
logic in load_vmstate() in next version.
>> + if (bs1 != bs && bdrv_can_snapshot(bs1)) {
>> + ret = bdrv_snapshot_find(bs1, sn_info, sn->id_str, NULL);
>> + if (ret < 0) {
>> + return false;
>> + }
>> + }
>> + }
>> + return true;
>> +}
>> +
>> +/*
>> * return 0 on success, @p_list will be set only on success, and caller
>> need to
>> - * check *p_list on success.
>> + * check *p_list on success. If @vm_snapshot is true, limit the results to
>> + * snapshots valid for the whole VM.
>> */
>> int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>> SnapshotInfoList **p_list,
>> + bool vm_snapshot,
>> Error **errp)
>> {
>> int i, sn_count;
>> @@ -59,7 +92,9 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>> }
>>
>> for (i = 0; i < sn_count; i++) {
>> -
>> + if (vm_snapshot && !snapshot_valid_for_vm(&sn_tab[i], bs)) {
>> + continue;
>> + }
>> info = g_new0(SnapshotInfo, 1);
>> info->id = g_strdup(sn_tab[i].id_str);
>> info->name = g_strdup(sn_tab[i].name);
>> diff --git a/include/block/qapi.h b/include/block/qapi.h
>> index 91dc41b..fe66053 100644
>> --- a/include/block/qapi.h
>> +++ b/include/block/qapi.h
>> @@ -30,6 +30,7 @@
>>
>> int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>> SnapshotInfoList **p_list,
>> + bool vm_snapshot,
>> Error **errp);
>> void bdrv_collect_image_info(BlockDriverState *bs,
>> ImageInfo *info,
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 51043ef..261c277 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1735,7 +1735,8 @@ static ImageInfoList *collect_image_info_list(const
>> char *filename,
>>
>> info = g_new0(ImageInfo, 1);
>> bdrv_collect_image_info(bs, info, filename);
>> - if (!bdrv_query_snapshot_info_list(bs, &info->snapshots, NULL) &&
>> + if (!bdrv_query_snapshot_info_list(bs, &info->snapshots,
>> + false, NULL) &&
>> info->snapshots) {
>> info->has_snapshots = true;
>> }
>
--
Best Regards
Wenchao Xia
