Re: [Qemu-devel] block-commit & dropping privs

[Kevin Wolf <kwolf@redhat.com>]

Am 06.04.2015 um 17:37 hat Michael Tokarev geschrieben:
> 02.04.2015 16:19, Kevin Wolf wrote:
> > Am 02.04.2015 um 14:04 hat Michael Tokarev geschrieben:
> >> 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?
> >>
> >> 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.
> >>
> >> 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.
> > 
> > For the record, we discussed this on IRC:
> > 
> > Yes, we did talk past each other because HMP commit isn't supposed to
> > touch the backing file name at all, so I didn't expect such behaviour,
> > yet Michael saw it.
> > 
> > The reason is a bug in qcow2_update_header(). Instead of rewriting the
> > same value as is already in the image, it writes bs->backing_file to the
> > image. This was always the same as long as you couldn't override the
> > backing file name on the command line or in blockdev-add, but now it's
> > obviously wrong.
> > 
> > It would also rewrite the backing file name on other occasions such as
> > marking the image dirty with lazy_refcounts=on (i.e. on the first
> > write request).
> 
> Kevin, did you have a chance to fix this prob (for 2.3)?
> 2.1 version does the right thing here.

I started working on it on Thursday, but as both Good Friday and Easter
Monday are public holidays in Germany, it's not ready yet. I hope to get
it ready in time for -rc3.

2.1 does the "right" thing because it doesn't support bdrv_make_empty()
for qcow2 yet, which is the function that triggers the buggy header
rewrite. But without it, your image can't shrink.

> >>>> 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?
> > 
> > This would be a bug as well, but Michael double-checked and it does
> > shrink in fact.
> 
> Actually it WAS a bug.  Initially I tested with 2.1 version,
> and there, neither `commit' HMP command nor `qemu-img commit'
> command shrinks the overlay image, so each new commit re-writes
> both the new data AND the old data, and never shrinks.  With
> 2.2 version it works as expected, except of the above problem
> with rewriting the base file name.

I would call it a new feature rather than a bug fix, but yes, that's new
in 2.2 (see above).

Kevin