On 04/17/2014 01:29 AM, Kevin Wolf wrote:
Okay, now I have a better reason than "Meh, I don't like it" (which
is always a very bad reason, of course), being the following: As
mirror_run() is actually made for mirroring from an active block
device, some sectors may be marked dirty during the block job which
the initialization loop did not consider dirty. This will not occur
in the case of qemu-img commit, obviously, as the block device is
not really used. However, mirror_run() isn't made for use by
qemu-img only. If new sectors are marked dirty during the block
job's operation, we'd have to increase the length of the block job
live which seems very crude to me.
Of course, I'd love to be proven wrong, as I do see that the above
solution (taking the granularity into account) is pretty crude as
well.
I'm not sure if it really matters. The progress is only for the initial
bulk copy anyway. It's possible that you copied a cluster and then the
guest dirties it again, and the progress won't show that. So you already
have some inaccuracy of that kind today. In this light, I would consider
it reasonable enough to treat only the initially allocated clusters as
part of that first pass.
But you're right in that we need to be careful to cap the progress at
the maximum we're advertising.
Libvirt has documented for its job interfaces that the "maximum" number
is NOT fixed, and need not be the same as the size of the disk it is
tracking. An operation is entitled to raise the maximum appropriately
as it accounts for blocks dirtied by the guest after the host had
already passed the block on one iteration. For the sake of progress
reporting, raising the maximum value as we go DOES give better results,
as long as one thing holds true: when the operation is complete, the
amount remaining converges to 0 because the amount complete converges to
the maximum. Yes, this can mean that progress sometimes travels
backwards (the maximum increases by more than the completion across one
pass of the disk, if the guest is dirtying the disk faster than the job
was copying from the disk during that iteration) - but that's something
you WANT to see, as an indication that the job is not going to converge
if you don't throttle the guest.