Re: [Qemu-devel] [PATCH v15 08/25] block: introduce auto-loading bitmaps

[Kevin Wolf]

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().
> >
> > Kevin
> I 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