[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 6/8] block/backup: issue progress updates for sk
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH 6/8] block/backup: issue progress updates for skipped regions |
Date: |
Wed, 10 Jul 2019 22:30:23 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 |
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 <address@hidden>
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH 1/8] iotests/257: add Pattern class, (continued)
[Qemu-block] [PATCH 4/8] block/backup: hoist bitmap check into QMP interface, John Snow, 2019/07/09
[Qemu-block] [PATCH 5/8] iotests/257: test API failures, John Snow, 2019/07/09
[Qemu-block] [PATCH 7/8] block/backup: support bitmap sync modes for non-bitmap backups, John Snow, 2019/07/09