On 10.07.19 20:20, John Snow wrote: > > > On 7/10/19 12:36 PM, Max Reitz wrote: >> On 10.07.19 03:05, John Snow wrote: >>> The way bitmap backups work is by starting at 75% if it needs >>> to copy just 25% of the disk. >> >> Although there is this comment: >> >>> /* TODO job_progress_set_remaining() would make more sense */ >> >> So... >> >>> The way sync=top currently works, however, is to start at 0% and then >>> never update the progress if it doesn't copy a region. If it needs to >>> copy 25% of the disk, we'll finish at 25%. >>> >>> Update the progress when we skip regions. >> >> Wouldn’t it be more correct to decrease the job length? >> >> Max >> > > Admittedly I have precisely no idea. Maybe? As far as I understand it, > we guarantee only: > > (1) Progress monotonically increases > (2) Upon completion, progress will equal the total work estimate. > [Trying to fix that to be true here.] > > This means we can do stuff like: > > - Total work estimate can increase or decrease arbitrarily > - Neither value has to mean anything in particular > > > Bitmap sync works by artificially increasing progress for NOP regions, > seen in init_copy_bitmap.
Yes, and it has a TODO comment that says it should be done differently. > Full sync also tends to increase progress regardless of it actually did > a copy or not; offload support also counts as progress here. So if you > full sync an empty image, you'll see it increasing the progress as it > doesn't actually do anything. > > Top sync is the odd one out, which just omits progress for regions it skips. > > My only motivation here was to make them consistent. Can I do it the > other way? Yeah, probably. Is one way better than the other? I > legitimately have no idea. I guess whoever wrote the last comment felt > that it should all be the other way instead. Why'd they not do that? If you look at the commit (05df8a6a2b4), I suppose it was because that commit simply did not intend to change behavior. It just touched that piece of code and noted that maybe there should be a follow-up commit to change it. But yeah, whatever. Reviewed-by: Max Reitz <mre...@redhat.com>
signature.asc
Description: OpenPGP digital signature