Re: [Qemu-devel] [RFC PATCH 1/1] dataplane: alternative approach to locking

[Juan Quintela <quintela@redhat.com>]

"Denis V. Lunev" <den-lists@parallels.com> wrote:
> On 11/04/2015 12:49 PM, Juan Quintela wrote:
>> void hmp_delvm(Monitor *mon, const QDict *qdict)
>> {
>>      const char *name = qdict_get_str(qdict, "name");
>>
>>      if (!bdrv_find_snapshot_bs()) {
>>          monitor_printf(mon, "No block device supports snapshots\n");
>>          return;
>>      }
>>
>>      del_existing_snapshots(mon, name);
>> }
>>
>> Yes, we have changed the semantics "slightly".  Pervious version of
>> hmp_delvm() will try to remove all the snapshots from any device with
>> that name.  This one would remove them until it finds one error.  I
>> think that the code reuse and the consistence trumps the change in
>> semantics (really the change is only on error cases).
>
> I think you are wrong here. You can not abort operation if one
> disk does not have a snapshot assuming the following situation
> - VM has one disk
> - snapshot XXX is made
> - 2nd disk is added
> - remove XXX snapshot

I think that my *completely* untested suggestion handled that well.

char *name bdrv_remove_snapshots(const char *name, Error *err)
{
    BlockDriverState *bs;
    QEMUSnapshotInfo sn1, *snapshot = &sn1;

    bs = NULL;
    while ((bs = bdrv_next(bs))) {
        if (bdrv_can_snapshot(bs) &&
            bdrv_snapshot_find(bs, snapshot, name) >= 0) {
            bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
            if (err) {
                return bdrv_get_device_name(bs);
            }
        }
    }
    return NULL;
}

It only stops without removing an snapshot if there is one error
deleting one snapshot.  Current code just tells that there is one error
and continues in the rest of the disks.

Notice that we are going to have problems on this operation, we have
found a disk with one snapshot with the name that we want to remove and
we have failed.


>
> Your position is understood. I'll send yet another proof of concept
> in an hour.

Thanks, Juan.