From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:46045)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1Ztwmt-0006W9-I3
	for qemu-devel@nongnu.org; Wed, 04 Nov 2015 07:03:36 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1Ztwmq-0005Ef-CO
	for qemu-devel@nongnu.org; Wed, 04 Nov 2015 07:03:35 -0500
Received: from mx1.redhat.com ([209.132.183.28]:41176)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1Ztwmq-0005EY-8H
	for qemu-devel@nongnu.org; Wed, 04 Nov 2015 07:03:32 -0500
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <5639E812.5030303@parallels.com> (Denis V. Lunev's message of
	"Wed, 4 Nov 2015 14:12:18 +0300")
References: <87bnbbxhk4.fsf@emacs.mitica>
	<1446622348-2479-1-git-send-email-den@openvz.org>
	<87ziyuksbw.fsf@emacs.mitica> <5639E812.5030303@parallels.com>
Date: Wed, 04 Nov 2015 13:03:29 +0100
Message-ID: <87mvuuc6pa.fsf@emacs.mitica>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [RFC PATCH 1/1] dataplane: alternative approach to
	locking
Reply-To: quintela@redhat.com
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Denis V. Lunev" <den-lists@parallels.com>
Cc: "Denis V. Lunev" <den@openvz.org>, qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@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.