|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [Qemu-devel] [PATCH v15 08/25] block: introduce auto-loading bitmaps |
Date: | Mon, 20 Feb 2017 13:09:53 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 |
17.02.2017 17:24, Kevin Wolf wrote:
Bitmaps are not just qcow2 metadata. They belongs to generic block layer. And qcow2's ability to store bitmap is used to realize the common interface. After bitmap is loaded and turned into BdrvDirtyBitmap it not belongs to qcow2.. And qcow2 layer should not touch it on intermediate reopens..Am 17.02.2017 um 14:48 hat Denis V. Lunev geschrieben:On 02/17/2017 04:34 PM, Kevin Wolf wrote:Am 17.02.2017 um 14:22 hat Denis V. Lunev geschrieben:But for sure this is bad from the downtime point of view. On migrate you will have to write to the image and re-read it again on the target. This would be very slow. This will not help for the migration with non-shared disk too. That is why we have specifically worked in a migration, which for a good does not influence downtime at all now. With a write we are issuing several write requests + sync. Our measurements shows that bdrv_drain could take around a second on an averagely loaded conventional system, which seems unacceptable addition to me.I'm not arguing against optimising migration, I fully agree with you. I just think that we should start with a correct if slow base version and then add optimisation to that, instead of starting with a broken base version and adding to that. Look, whether you do the expensive I/O on open/close and make that a slow operation or whether you do it on invalidate_cache/inactivate doesn't really make a difference in term of slowness because in general both operations are called exactly once. But it does make a difference in terms of correctness. Once you do the optimisation, of course, you'll skip writing those bitmaps that you transfer using a different channel, no matter whether you skip it in bdrv_close() or in bdrv_inactivate(). KevinI do not understand this point as in order to optimize this we will have to create specific code path or option from the migration code and keep this as an ugly kludge forever.The point that I don't understand is why it makes any difference for the follow-up migration series whether the writeout is in bdrv_close() or bdrv_inactivate(). I don't really see the difference between the two from a migration POV; both need to be skipped if we transfer the bitmap using a different channel. Maybe I would see the reason if I could find the time to look at the migration patches first, but unfortunately I don't have this time at the moment. My point is just that generally we want to have a correctly working qemu after every single patch, and even more importantly after every series. As the migration series is separate from this, I don't think it's a good excuse for doing worse than we could easily do here. Kevin
We can introduce additional flags to control loading of autoloading bitmaps. Isn't it better to handle them in common place for all formats? (yes, for now only qcow2 can store bitmaps, but parallels format defines this ability too, in future we may have some way to load bitmaps for raw format.. Also, there is bitmap related extension for NBD protocol).
bdrv_load_dirty_bitmap, defined in my series about NBD BLOCK_STATUS is definetly generic, as it should be called from qmp command.. (just note, not concrete argument)
-- Best regards, Vladimir
[Prev in Thread] | Current Thread | [Next in Thread] |