[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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.

Then I don't understand next passage from virDomainJobInfo definition.

     * For VIR_DOMAIN_JOB_UNBOUNDED, dataTotal may be less
     * than the final sum of dataProcessed + dataRemaining
     * in the event that the hypervisor has to repeat some
     * data, such as due to dirtied pages during migration.

I thought total is initial estimation which never changed 
and processed and remaining are updated as we go. IIRC this is the case
for memory migration.

>> 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.

You mean after migration is completed there can be some disks transfers yet like
buffered data? Ok.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]