Am 15.05.2013 um 10:25 hat Paolo Bonzini geschrieben: > Il 15/05/2013 09:59, Kevin Wolf ha scritto: > >>> Do you mean you'd model the 'active' mode after 'block-backup,' or > >>> actually > >>> call functions provided by 'block-backup'? > >> > >> No, I'll just reuse the same hooks within block/mirror.c (almost... it > >> looks like I need after_write too, not just before_write :( that's a > >> pity). > > > > Makes me wonder if using a real BlockDriver for the filter from the > > beginning wouldn't be better than accumulating more and more hooks and > > having to find ways to pass data from 'before' to 'after' hooks... > > We don't need a way to pass data from before to after hooks, a simple > scan of a linked list will do.
So in this case the linked list is the way. > >> Basically: > >> > >> 1) before the write, if there is space in the job's buffers, allocate a > >> MirrorOp and a data buffer for the write. Also record whether the block > >> was dirty before; > >> > >> 2) after the write, do nothing if there was no room to allocate the data > >> buffer. Else clear the block from the dirty bitmap. If the block was > >> dirty, read the whole cluster from the source as in passive mirroring. > >> If it wasn't, copy the data from guest memory to the preallocated buffer > >> and write it to the destination; > > > > Does the "if there was no room" part mean that the mirror is active only > > sometimes? > > Yes, otherwise the guest can allocate arbitrary amounts of memory in the > host just by starting a few very large I/O operations. I think I would rather throttle I/O in this case, i.e. requests wait until they can get the space. At least for a synchronous mirror we have to do something like this. > > And why even bother with a dirty bitmap for an active mirror? The > > background job that sequentially processes the whole image only needs a > > counter, no bitmap. > > That's not enough for the case when the host crashes and you have to > restart the mirroring or complete it offline. You're thinking of a persistent bitmap here? Makes sense then, I didn't think about that. Kevin
