17.06.2019 19:25, Kevin Wolf wrote: > 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.
OK, thanks, I understand now that implicit filters are not just a feature but compatibility mechanism. So, can we at some point deprecate "optionality" of filter-node-name parameters of jobs, in addition to deprecation of things like old copy-on-read option? And actually deprecate implicit filters by this? > >>>>>>> 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... :-) > So, I still don't understand the reason of flushing backup target in a meantime. Backup target remains invalid until the successful end of the job, at which point we definitely flush target, but only once. If node crashes during backup, backup would be invalid independently of were there flushes after each write (or better just enable O_DIRECT) or not. -- Best regards, Vladimir
