"Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
> * Paolo Bonzini (pbonz...@redhat.com) wrote:
>> Dividing integer expressions transferred_bytes and time_spent, and then
>> converting
>> the integer quotient to type double. Any remainder, or fractional part of the
>> quotient, is ignored. Fix this.
>>
>> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
>> ---
>> migration/migration.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index b3adbc6..6db75b8 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -646,7 +646,7 @@ static void *migration_thread(void *opaque)
>> if (current_time >= initial_time + BUFFER_DELAY) {
>> uint64_t transferred_bytes = qemu_ftell(s->file) -
>> initial_bytes;
>> uint64_t time_spent = current_time - initial_time;
>> - double bandwidth = transferred_bytes / time_spent;
>> + double bandwidth = (double)transferred_bytes / time_spent;
>> max_size = bandwidth * migrate_max_downtime() / 1000000;
>>
>> s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
Reviewed-by: Juan Quintela <quint...@redhat.com>
we had that fix on RHEL, but I forgot to push it upstream (it was not
needed as it optimized the calculations at the time).
>
> This feels like it would be better to fix this by merging it into
> the s->mbps calculation just off the bottom; we currently have:
>
> uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
> uint64_t time_spent = current_time - initial_time;
> double bandwidth = transferred_bytes / time_spent;
> max_size = bandwidth * migrate_max_downtime() / 1000000;
>
> s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
> ((double) time_spent / 1000.0)) / 1000.0 / 1000.0 : -1;
>
>
> Note that the mbps has a check for time_spent being 0 - if that can ever
> happen,
> how come 'bandwidth' has never triggered it?
>
> transferred_bytes - bytes
> time_spent - ms
> bandwidth - bytes/ms
> migrate_max_downtime - in ns
> s->mbps - mbit/s
>
> giving
>
> max_size = bytes/ms * time-in-ns / 1000000
> = bytes/ms * time-in-ms
> = bytes
>
> so how about something like:
> uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
> uint64_t time_spent = current_time - initial_time;
> double bytes_s = (double) transferred_bytes / ((double)
> time_spent / 1000.0));
> s->mbps = (bytes_s * 8.0) / 1000000.0;
> max_size = bytes_s * (migrate_max_downtime() / 1000000000.0);
I think we can do something like this on the normal tree, or just go for
sane units left and right?
> that also needs the trace fixing and the line a few lines below, I *think* we
> have
> dirty_bytes_rate is in bytes/second ? (arch_init.c)
> expected_downtime in ms ?
> s->expected_downtime = s->dirty_bytes_rate / bandwidth;
> so, bytes/s / bytes/ms erm that's supposed to come out as time in ms
>
> s->expected_downtime = (int64_t)(1000 *
> (double)s->dirty_bytes_rate / bytes_s);
>
> Yeuch.
>
> Dave
>
>> --
>> 1.8.3.1
>>
>>
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
