On 04/02/2015 06:04 AM, Michael Tokarev wrote:
> 02.04.2015 14:24, Kevin Wolf wrote:
> []
>>> But overall, I think qemu-system should not modify backing
>>> file name in this case.
>>
>> So you would leave the backing file with the data that you just
>> committed down one level in your backing file chain? Wouldn't that
>> defeat the whole purpose of committing?

No, because there are multiple reasons for committing.  And we recently
updated qemu-img to use QMP for multiple styles of commit:

1: merge data from the active layer to the immediate backing, but keep
the active layer intact (by freeing its sectors). Useful for saying "I
want to reset my point-in-time backup to now, but continue tracking the
delta since the point-in-time".
 starting with: A <- B <- C, with B created at point1, C created at
point2, and right now being point3. B contains point1..point2, C
contains point2..point3+
 qemu-img commit C
 ending with: A <- B <- C, B contains point1..point3, C contains
point3+; all three files still valid

2: merge data from the active layer to the immediate backing, and
discard the active layer (no need to waste time freeing its sectors).
Useful for saying "I no longer need my point-in-time backup, revert to
the smaller chain"
 starting with: A <- B <- C (as before)
 qemu-img commit -d C
 ending with: A <- B, B contains point1..point3+; C invalid the moment
something is written to B

3: merge data from the active layer to a specified backing (typically
more than one file) to really clean up a chain. Useful for saying "I
want to keep current state but discard all intermediate points in time".
 When crossing multiple backing files, you WILL cause corruption to the
intermediate files.  Therefore, this mode implies -d, and there is no
point wasting time in clearing out sectors in the discarded part of the
chain.
 starting with: A <- B <- C (as before)
 qemu-img commit -b A C
 ending with: A; A contains point3+, B is immediately invalid, C is
invalid the moment something else is written to A

4: offline-only: note that 'qemu-img commit' can ONLY commit the changes
of the filename argument, while QMP can commit any arbitrary point in a
chain (note that there is a difference between active commit and
intermediate commit). But qemu-img can be used to do intermediate
commit, by virtue of the fact that since the image is not in active use,
you can specify any intermediate file in the chain as the point to
commit, coupled with rewriting the backing file of the rest of the chain
to match. Requires two qemu-img commands, so might be worth adding sugar
to do it in one command the way QMP can do, if desired.
 starting with: A <- B <- C (as before)
 qemu-img commit -d B
 qemu-img rebase -u A C
 ending with: A <- C; A contains point2, C contains point2+

[Proof that committing to more than the immediate backing file was in
the thread on converting qemu-img to use QMP; although I'm somewhat
repeating that argument down below]

> 
> Um.  I don't think we understood each other.
> 
> I experimented with the "non-live" HMP commit command.  This
> one effectively empties everything in the overlay file,
> propagating it to the backing file, but keeps the (now
> empty) overlay.  So from the stacking perspective nothing
> has changed.  Yet, together with with propagation, it also
> modifies the overlay file headers and writes a new name
> of the backing file -- the one it currently uses, which,
> in my case, is virtual /dev/fdset/foo.  It should keep
> the original file name in there, such as win.raw, unless
> explicitly asked to write a different name there.

Bug #1 - HMP doesn't use QMP commit - and therefore it can only commit
the active layer. I argued above that qemu-img has the same limitation,
except that qemu-img can be used anywhere in the chain by using multiple
qemu-img commands

Bug #2 - HMP changes the backing file when it shouldn't (active commit
should never alter backing information if the active file doesn't change)

> 
> If the stack chain were to be modified by commit command,
> yes, the new name should be recorded ofcourse, such as
> after rebase.  But since stack chain is not modified,
> filename should not be modified either.
> 
>>> When performing commit, does qemu mark the areas in the
>>> overlay file as free after writing contents to the backing
>>> file, or will these areas be written again by a subsequent
>>> commit?  Somehow it smells like each next commit writes
>>> more and more data and completes in more and more time.
>>
>> With qcow2 and qcow, the committed data is discarded with HMP 'commit'.
>> Other image formats keep the copy.
> 
> Hm.  It is discarded, but the file isn't shrinked.  With "non-live"
> commit I don't see a reason why it can't be shrinked too?

Oh, you pointed out another difference.  I was going to say that QMP
active commit followed by aborting the transaction keeps the active
layer unchanged, but QMP does not yet expose a way to empty the active
layer.  That is, starting with these file contents (where - implies
deferring to the backing file):
B 1111
C --22

then qemu-img commit C leads to:
B 1122
C ----

while QMP active commit/abort leads to:
B 1122
C --22

at least both qemu-img commit -d C and QMP active commit/complete lead to:
B 1122
C useless

So we potentially have:

Bug #3 - QMP does not yet expose a way to empty the active layer

and if that is fixed, we need to be careful to not introduce:

Bug #4 - QMP must not allow emptying the active layer if the active
commit crosses more than one backing file but backing chain is not rewritten

To see why, you need merely understand why qemu-img -b implies -d:
     contents   guest would see
   A 1111       1111
A<-B -22-       1221
B<-C --33       1233

then commit C into A. If C is left untouched:
     contents   guest would see
   A 1233       1233
A<-B -22-       1223 # Oops, B is corrupted
B<-C --33       1233

and if C is emptied:
     contents   guest would see
   A 1233       1233
A<-B -22-       1223 # Oops, B is corrupted
B<-C ----       1223 # Oops, C is corrupted

Basically, once a commit crosses more than one file, all intermediate
files are useless and might as well be discarded. But you've pointed out
that by rewriting the backing file of C, we CAN make C still be
consistent and tracking the change since the commit:

     contents   guest would see
   A 1233       1233
A<-C ----       1233

Bug #5 - QMP commit doesn't offer a way to rewrite chain when committing
across multiple images


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org