Re: [Qemu-devel] [PATCH v8 4/7] block: introduce backup-top filter driver

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3847 bytes --]

Am 17.06.2019 um 18:01 hat Max Reitz geschrieben:
> >>> Should new implicit/explicit
> >>> filters be created above or under them?
> >>
> >> That was always the most difficult question we had when we introduced
> >> filters.
> >>
> >> The problem is that we never answered it in our code base.
> >>
> >> One day, we just added filters (“let’s see what happens”), and in the
> >> beginning, they were all explicit, so we still didn’t have to answer
> >> this question (in code).  Then job filters were added, and we still
> >> didn’t have to, because they set blockers so the graph couldn’t be
> >> reorganized with them in place anyway.
> >>
> >> If we converted copy-on-read=on to a COR node, we would have to answer
> >> that question.
> > 
> > That's why we shouldn't do that, but just remove copy-on-read=on. :-)
> 
> And BB-level throttling, fair enough.
> 
> (Although the first step would be probably to make throttle groups
> non-experimental?  Like, drop the x- prefix for all their parameters.)

The first step would be making the block drivers full replacements of
the old things, which unfortunately isn't true today.

After your "deal with filters" series, we should be a lot closer for the
core infrastructure at least.

Not sure about copy-on-read, but I know that throttle still doesn't have
feature parity with -drive throttling. At least I'm pretty sure that
something about changing the group or group options at runtime (and not
just dropping the x-) was missing.

> >>>>> But should really filter do that job, or it is here only to do CBW? Maybe target
> >>>>> must have another parent which will care about flushing.
> >>>>>
> >>>>> Ok, I think I'll flush target here too for safety, and leave a comment, that something
> >>>>> smarter would be better.
> >>>>> (or, if we decide to flush all children by default in generic code, I'll drop this handler)
> >>>>
> >>>> [1] Thinking more about this, for normal backups the target file does
> >>>> not reflect a valid disk state until the backup is done; just like for
> >>>> qemu-img convert.  Just like qemu-img convert, there is therefore no
> >>>> reason to flush the target until the job is done.
> > 
> > Depends, the target could have the source as its backing file (like
> > fleecing, but without exporting it right now), and then you could always
> > have a consistent view on the target. Well, almost.
> > 
> > Almost because to guarantee this, we'd have to flush between each CBW
> > and the corresponding write to the same block, to make sure that the old
> > data is backed up before it is overwritten.
> 
> Yes, that’s what I meant by “enforce on the host that the target is
> always flushed before the source”.  Well, I meant to say there is no
> good way of doing that, and I kind of don’t consider this a good way.
> 
> > Of course, this would perform about as badly as internal COW in qcow2...
> > So probably not a guarantee we should be making by default. But it might
> > make sense as an option.
> 
> I don’t know.  “Use this option so your data isn’t lost in case of a
> power failure, but it makes everything pretty slow”?  Who is going to do
> explicitly enable that (before their first data loss)?

Maybe the backup job wasn't that clever after all. At least if you care
about keeping the point-in-time snapshot at the start of the backup job
instead of just retrying and getting a snapshot of a different point in
time that is just as good.

If you do care about the specific point in time, the safe way to do it
is to take a snapshot and copy that away, and then delete the snapshot
again.

The only problem is that merging external snapshots is slow and you end
up writing the new data twice. If only we had a COW image format... :-)

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]