[libvirt] [PATCH v2 12/12] qemu: show disks stats for nbd migration

[Nikolay Shirokovskiy]

On 17.02.2017 13:17, Jiri Denemark wrote:
> On Wed, Dec 28, 2016 at 17:39:21 +0300, Nikolay Shirokovskiy wrote:
>> There is no disks stats when migrating with VIR_MIGRATE_NON_SHARED_*
>> for qemu that supports nbd. The reason is that disks are copied via disk mirroring
>> and not in the scope of migration job itself. Below
>> a couble of issues of the implementation are described.
>>
>> 'total' field is set from 'end' field of block job info for the
>> sake of simplicity. This is true only when there is no guest disk
>> activity during migration. If there is an activity then 'end' will
>> grow
> 
> This is exactly how memory migration works too.
> 
>> while 'total' is an estimation that should stay constant.
> 
> Nope. That's the reason why migration is an unbounded job, we never know
> what the total is and it is designed to change anytime during migration
> as we realize more data needs to be transferred.
> 
>> I guess this can be fixed by setting 'total' to disk 'capacity'.
> 
> Nothing to fix really.
> 
>> There is also known possible corner case issue with this implementation.
>> There is a chance that client asking for stats at the process of the
>> mirroring stopping on successfull migration will see only part of mirroring disks
>> and thus will receive inconsisent partial info.
>> ---
>>  docs/news.html.in         |  4 ++++
>>  src/qemu/qemu_driver.c    |  3 ++-
>>  src/qemu/qemu_migration.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>>  src/qemu/qemu_migration.h |  5 +++++
>>  4 files changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/news.html.in b/docs/news.html.in
>> index 22611db..8036cf8 100644
>> --- a/docs/news.html.in
>> +++ b/docs/news.html.in
>> @@ -42,6 +42,10 @@
>>            cpu cycles, stalled backend cpu cycles, and ref cpu
>>            cycles by applications running on the platform
>>            </li>
>> +          <li>qemu: show disks stats for nbd disks migration<br/>
>> +          Show disks stats in migrations stats in disks copy phase
>> +          of migration with non-shared disks.
>> +          </li>
>>          </ul>
>>        </li>
>>        <li><strong>Bug fixes</strong>
> 
> Since it took me so long to review your patches (I apologize for that),
> we started using news.xml instead of news.html.in. And it should be
> modified in a separate patch to avoid conflicts during backports.
> 
> Otherwise the patch looks good and it's definitely cleaner than v1.
> However, I think you should also update disk migration statistics while
> stopping the mirror jobs so that the stats of a completed migration
> contain accurate data.
> 

Thanks for review! I need to refresh these patches in memory a bit
to get some of your comments and ask questions while the patches
are fresh in you memory too.

Nikolay