Re: [Qemu-devel] [PATCH V3 07/11] block: export function bdrv_find_snapshot()

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013-1-15 7:39, Eric Blake 写道:
> On 01/14/2013 12:09 AM, Wenchao Xia wrote:
>>    This patch move it from savevm.c to block.c and export it.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   block.c               |   23 +++++++++++++++++++++++
>>   include/block/block.h |    2 ++
>>   savevm.c              |   22 ----------------------
>>   3 files changed, 25 insertions(+), 22 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 8192d8e..b7d2f03 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3351,6 +3351,29 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
>>       return -ENOTSUP;
>>   }
>>
>> +int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
>> +                       const char *name)
>> +{
>> +    QEMUSnapshotInfo *sn_tab, *sn;
>> +    int nb_sns, i, ret;
>> +
>> +    ret = -ENOENT;
>> +    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
>> +    if (nb_sns < 0) {
>> +        return ret;
>> +    }
>> +    for (i = 0; i < nb_sns; i++) {
>> +        sn = &sn_tab[i];
>> +        if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) {
>
> It is possible (albeit probably stupid) to create a qcow2 file where
> snapshot names are merely numeric strings.  In fact, just to see what
> would happen, I once[1] created a file where:
>
> snapshot id '1' was named '2'
> snapshot id '2' was named 'foo'
>
> This code comparison favors ids over names; so if I request to delvm 2,
> I end up removing the second snapshot, not the first.  This is okay, but
> probably worth documenting, and probably worth making sure that all code
> that looks up a snapshot by name or id goes through this function so
> that we get the same behavior everywhere.  My experiment was done
> several months ago, but my recollection was that at the time, there was
> an inconsistency where 'qemu-img snapshot' picked a different snapshot
> for the request of '2' than the online 'delvm' monitor command of qemu;
> making it unsafe to rely on either behavior in that version of qemu
> source code.
>
> [1]https://bugzilla.redhat.com/show_bug.cgi?id=733143
>

how about:
/*  if id is not NULL, try find it with id, if not exist, return NULL
  *  if id is NULL and name is not NULL, try find it with name.
  */ if id and name is NULL, direct return fail.
int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
                        const char *id, const char *name)

-- 
Best Regards

Wenchao Xia