Re: [Qemu-devel] [PATCH V7 2/8] snapshot: distinguish id and name in snapshot delete

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

于 2013/9/10 20:36, Kevin Wolf 写道:
> Am 07.08.2013 um 05:00 hat Wenchao Xia geschrieben:
>> Snapshot creation actually already distinguish id and name since it take
>> a structured parameter *sn, but delete can't. Later an accurate delete
>> is needed in qmp_transaction abort and blockdev-snapshot-delete-sync,
>> so change its prototype. Also *errp is added to tip error, but return
>> value is kepted to let caller check what kind of error happens. Existing
>> caller for it are savevm, delvm and qemu-img, they are not impacted by
>> introducing a new function bdrv_snapshot_delete_by_id_or_name(), which
>> check the return value and do the operation again.
>>
>> Before this patch:
>>    For qcow2, it search id first then name to find the one to delete.
>>    For rbd, it search name.
>>    For sheepdog, it does nothing.
>>
>> After this patch:
>>    For qcow2, logic is the same by call it twice in caller.
>>    For rbd, it always fails in delete with id, but still search for name
>> in second try, no change to user.
>>
>> Some code for *errp is based on Pavel's patch.
>>
>> Signed-off-by: Wenchao Xia<xiawenc@linux.vnet.ibm.com>
>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>> diff --git a/savevm.c b/savevm.c
>> index 03fc4d9..0808414 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -2325,18 +2325,21 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
>>   {
>>       BlockDriverState *bs;
>>       QEMUSnapshotInfo sn1, *snapshot =&sn1;
>> -    int ret;
>> +    Error *err = NULL;
>>
>>       bs = NULL;
>>       while ((bs = bdrv_next(bs))) {
>>           if (bdrv_can_snapshot(bs)&&
>>               bdrv_snapshot_find(bs, snapshot, name)>= 0)
>>           {
>> -            ret = bdrv_snapshot_delete(bs, name);
>> -            if (ret<  0) {
>> +            bdrv_snapshot_delete_by_id_or_name(bs, name,&err);
>> +            if (error_is_set(&err)) {
>>                   monitor_printf(mon,
>> -                               "Error while deleting snapshot on '%s'\n",
>> -                               bdrv_get_device_name(bs));
>> +                               "Error while deleting snapshot on device '%s', "
>> +                               "reason: %s\n",
> More commonly, error messages just use a colon before the detailed
> error code instead of saying ", reason:"
>
>> +                               bdrv_get_device_name(bs),
>> +                               error_get_pretty(err));
>> +                error_free(err);
>>                   return -1;
>>               }
>>           }
> Kevin
>
Thanks for reviewing, will fix and rebase.