On 05/08/2015 07:14 AM, Stefan Hajnoczi wrote: > No it doesn't. Actions have to appear atomic to the qmp_transaction > caller. Both approaches achieve that so they are both correct in > isolation. > > The ambiguity is whether "commit the changes" for .commit() means > "changes take effect" or "discard stashed state, making undo > impossible". > > I think the "discard stashed state, making undo impossible" > interpretation is good because .commit() is not allowed to fail. That > function should only do things that never fail. > >> That's going to get hard to maintain as we add more transactions. > > Yes, we need to be consistent and stick to one of the interpretations in > order to guarantee ordering. > > Unfortunately, there is already an inconsistency: > > 1. internal_snapshot - snapshot taken in .prepare() > 2. external_snapshot - BDS node appended in .commit() > 3. drive_backup - block job started in .prepare() > 4. blockdev_backup - block job started in .prepare() > > external_snapshot followed by internal_snapshot acts like the reverse > ordering!
Is that fatal, though? Let's see if I'm understanding the problem correctly: if you start with a.qcow2, then external_snapshot followed by internal_snapshot should create b.qcow2 then the internal snapshot inside b.qcow2, while internal_snapshot followed by external_snapshot should create the internal snapshot inside a.qcow2, then create b.qcow2 But since we create the BDS node later than the internal snapshot is taken, both sequences currently cause the internal snapshot to live in a.qcow2. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
