On 17.4.2013 12:19, Wenchao Xia wrote:
> δΊ 2013-4-17 15:52, Pavel Hrdina ει:
>> Hi Wenchao,
>>
>> unfortunately no. According to new design of savevm, loadvm and delvm I
>> need also search for snapshots that have the specified name and id.
>>
> It seems the logic in your function, is same with mine...
It is not the same.
Your logic:
if id is set:
if there is snapshot with that id:
end searching
if name set (search also if id is set but nothing found):
if there is snapshot with that name:
end searching
My logic:
if name is set and id is set:
if there is snapshot with than name and with that id:
end searching
else if name is set (means that only name is set):
if there is snapshot with that name:
end searching
else if id is set (means that only id is set):
if there is snapshot with that id:
end searching
>
>> I'm also touching bdrv_snapshot_list where I'm adding an Error parameter
> I looked it before, but it needs all call back in ./block support it,
> so is it really necessary?
I think it is better if this function internally set appropriate error
message based on used disk image format (qcow2, sheepdog, rbd).
Adding to CC Eric for his opinion.
>
>> and changing the return value to be used only for getting a number of
>> snapshots. So in case that there is some error, the return value will be 0.
>>
>> Pavel
>>
>> On 17.4.2013 04:53, Wenchao Xia wrote:
>>> Hi, Pavel
>>> I have implemented it at
>>> http://lists.nongnu.org/archive/html/qemu-devel/2013-04/msg02533.html
>>> in patch 1,2. Could u check if it satisfy your requirement, if yes maybe
>>> you can directly use them.
>>>
>>>> Signed-off-by: Pavel Hrdina <phrd...@redhat.com>
>>>> ---
>>>> savevm.c | 33 +++++++++++----------------------
>>>> 1 file changed, 11 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/savevm.c b/savevm.c
>>>> index 66753da..bc829a5 100644
>>>> --- a/savevm.c
>>>> +++ b/savevm.c
>>>> @@ -2195,7 +2195,7 @@ out:
>>>> }
>>>>
>>>> static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo
>>>> *sn_info,
>>>> - const char *name, const char *id, bool
>>>> old_match)
>>>> + const char *name, const char *id)
>>>> {
>>>> QEMUSnapshotInfo *sn_tab, *sn;
>>>> int nb_sns, i, found = 0;
>>>> @@ -2218,20 +2218,10 @@ static int bdrv_snapshot_find(BlockDriverState
>>>> *bs, QEMUSnapshotInfo *sn_info,
>>>> break;
>>>> }
>>>> } else if (name) {
>>>> - /* for compatibility for old bdrv_snapshot_find call
>>>> - * will be removed */
>>>> - if (old_match) {
>>>> - if (!strcmp(sn->id_str, id) || !strcmp(sn->name, name)) {
>>>> - *sn_info = *sn;
>>>> - found = 1;
>>>> - break;
>>>> - }
>>>> - } else {
>>>> - if (!strcmp(sn->name, name)) {
>>>> - *sn_info = *sn;
>>>> - found = 1;
>>>> - break;
>>>> - }
>>>> + if (!strcmp(sn->name, name)) {
>>>> + *sn_info = *sn;
>>>> + found = 1;
>>>> + break;
>>>> }
>>>> } else if (id) {
>>>> if (!strcmp(sn->id_str, id)) {
>>>> @@ -2290,7 +2280,7 @@ SnapshotInfo *qmp_vm_snapshot_save(const char *name,
>>>> Error **errp)
>>>> sn->date_nsec = tv.tv_usec * 1000;
>>>> sn->vm_clock_nsec = qemu_get_clock_ns(vm_clock);
>>>>
>>>> - if (bdrv_snapshot_find(bs, old_sn, name, NULL, false)) {
>>>> + if (bdrv_snapshot_find(bs, old_sn, name, NULL)) {
>>>> error_setg(errp, "snapshot '%s' exists", name);
>>>> goto the_end;
>>>> } else {
>>>> @@ -2388,7 +2378,7 @@ SnapshotInfo *qmp_vm_snapshot_load(bool has_name,
>>>> const char *name,
>>>> }
>>>>
>>>> /* Don't even try to load empty VM states */
>>>> - if (!bdrv_snapshot_find(bs_vm_state, &sn, name, id, false)) {
>>>> + if (!bdrv_snapshot_find(bs_vm_state, &sn, name, id)) {
>>>> return NULL;
>>>> }
>>>>
>>>> @@ -2413,7 +2403,7 @@ SnapshotInfo *qmp_vm_snapshot_load(bool has_name,
>>>> const char *name,
>>>> return NULL;
>>>> }
>>>>
>>>> - if (!bdrv_snapshot_find(bs, &sn, name, id, false)) {
>>>> + if (!bdrv_snapshot_find(bs, &sn, name, id)) {
>>>> return NULL;
>>>> }
>>>> }
>>>> @@ -2484,7 +2474,7 @@ SnapshotInfo *qmp_vm_snapshot_delete(const bool
>>>> has_name, const char *name,
>>>> return NULL;
>>>> }
>>>>
>>>> - if (!bdrv_snapshot_find(bs, &sn, name, id, false)) {
>>>> + if (!bdrv_snapshot_find(bs, &sn, name, id)) {
>>>> /* no need to set an error if snapshot doesn't exist */
>>>> return NULL;
>>>> }
>>>> @@ -2501,7 +2491,7 @@ SnapshotInfo *qmp_vm_snapshot_delete(const bool
>>>> has_name, const char *name,
>>>> bs = NULL;
>>>> while ((bs = bdrv_next(bs))) {
>>>> if (bdrv_can_snapshot(bs)
>>>> - && bdrv_snapshot_find(bs, &sn, name, id, false)) {
>>>> + && bdrv_snapshot_find(bs, &sn, name, id)) {
>>>> bdrv_snapshot_delete(bs, sn.name, errp);
>>>> if (error_is_set(errp)) {
>>>> return NULL;
>>>> @@ -2549,8 +2539,7 @@ void do_info_snapshots(Monitor *mon, const QDict
>>>> *qdict)
>>>>
>>>> while ((bs1 = bdrv_next(bs1))) {
>>>> if (bdrv_can_snapshot(bs1) && bs1 != bs) {
>>>> - if (!bdrv_snapshot_find(bs1, sn_info, sn->name,
>>>> sn->id_str,
>>>> - true)) {
>>>> + if (!bdrv_snapshot_find(bs1, sn_info, sn->name,
>>>> sn->id_str)) {
>>>> available = 0;
>>>> break;
>>>> }
>>>>
>>>
>>>
>>
>>
>
>
