Re: [Qemu-devel] [PATCH 00/23] block migration: Fixes, cleanups and speedups

[Jan Kiszka]

Pierre Riteau wrote:
> On 30 nov. 2009, at 19:34, Anthony Liguori wrote:
> 
>> Jan Kiszka wrote:
>>> This series is a larger rework of the block migration support qemu
>>> recently gained. Besides lots of code refactorings the major changes
>>> are:
>>> - Faster restore due to larger block sizes (even if the target disk is
>>>   unallocated)
>>> - Off-by-one fixes in the block dirty tracking code
>>> - Allow for multiple migrations (after cancellation or if migrating
>>>   into a backup image)
>>> - Proper error handling
>>> - Progress reporting fixes: report to monitor instead of stdout, report
>>>   sum of multiple disks
>>> - Report disk migration progress via 'info migrate'
>>> - Progress report during restore
>>>
>>> One patch is directly taken from Pierre Riteau queue [1] who happend to
>>> work on the some topic the last days, two more are derived from his
>>> commits.
>>>
>>> These patches make block migration usable for us. Still, there are two
>>> more major improvements on my wish/todo list:
>>> - Respect specified maximum migration downtime (will require tracking
>>>   of the number of dirty blocks + some coordination with ram migration)
>>> - Do not transfere unallocated disk space (also for raw images, ie. add
>>>   bdrv_is_allocated support for the latter)
>>>
>>> In an off-list chat, Liran additionally brought up the topic that RAM
>>> migration should not start too early so that we avoid re-transmitting
>>> dirty pages over and over again while the disk image is slowly beamed
>>> over.
>>>
>>> I hope we can join our efforts to resolve the open topics quickly, the
>>> critical ones ideally before the merge window closes.
>>>  
>> That really needs to happen no later than the end of this week.
>>
>> So Pierre/Liran, what do you think about Jan's series?
>>
>> Regards,
>>
>> Anthony Liguori
> 
> 
> I'm currently testing these patches. Here are a few issues I noticed, before 
> I forget about them.
> 
> - "migrate -d -b tcp:dest:port" works, but "migrate -b -d tcp:dest:port" 
> doesn't, although "help migrate" doesn't really specify ordering as 
> important. But anyway I think Liran is working on a new version of the 
> command.

Saw that too. I think the monitor commands simply do very primitive
option parsing so far. Should be addressed if the final format comes
with this issue as well.

> - We use bdrv_aio_readv() to read blocks from the disk. This function 
> increments rd_bytes and rd_ops, which are reported by "info blockstats". I 
> don't think this read operations should appear in VM activity, especially if 
> this interface is used by libvirt to report VM stats (and draw graphs in 
> virt-manager, etc.). Same for write stats.

Ack.

> - We may need to call bdrv_reset_dirty() _before_ sending the data, to be 
> sure the block is not rewritten in the meantime (maybe it's an issue only 
> with kvm?)

Can you elaborate? Even in case of multi-threaded qemu, the iomutex
should protect us here.

> - I seem to remember that disk images with 0 size are now possible. I'm 
> afraid we will hit a divide by zero in this case: "progress = 
> completed_sector_sum * 100 / block_mig_state.total_sector_sum;"

Although I don't see their use, it should be handled gracefully, likely
by skipping such disks.

> 
> Apart from that, it works quite fine. Still a few things to cleanup (e.g. 
> unused constants) but much better than before.
> However, I haven't tested the incremental transfer support at all yet. It's 
> on my todo list.
> 

Jan

signature.asc
Description: OpenPGP digital signature