qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Denis V. Lunev
Subject: Re: [Qemu-devel] [RFC PATCH 1/1] dataplane: alternative approach to locking
Date: Wed, 4 Nov 2015 15:07:04 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 11/04/2015 03:03 PM, Juan Quintela wrote:
"Denis V. Lunev" <address@hidden> 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.
yes. we should follow this way in both branches.
I like this and done similar thing in my RFC :)

Den



reply via email to

[Prev in Thread] Current Thread [Next in Thread]