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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
