> From: Kevin Wolf [mailto:kw...@redhat.com]
> Am 09.02.2016 um 06:55 hat Pavel Dovgalyuk geschrieben:
> > This patch introduces a set of functions that implement recording
> > and replaying of block devices' operations. These functions form a thin
> > layer between blk_aio_ functions and replay subsystem.
> > All asynchronous block requests are added to the queue to be processed
> > at deterministically invoked record/replay checkpoints.
> > Queue is flushed at checkpoints and information about processed requests
> > is recorded to the log. In replay phase the queue is matched with
> > events read from the log. Therefore block devices requests are processed
> > deterministically.
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovga...@ispras.ru>
>
> This series doesn't seem to apply to current master, so it's somewhat
> hard to look at the end result. I can see just from patches, though,
> that this will need some more discussion.
Thank you for the response.
I forgot to rebase these patches, but there are minor problems with applying
them.
>
> Just picking one example of how you convert blk_* functions:
>
> > -BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t sector_num,
> > - int nb_sectors, BdrvRequestFlags flags,
> > - BlockCompletionFunc *cb, void *opaque)
> > +BlockAIOCB *blk_aio_write_zeroes_impl(BlockBackend *blk, int64_t
> > sector_num,
> > + int nb_sectors, BdrvRequestFlags
> > flags,
> > + BlockCompletionFunc *cb, void
> > *opaque)
> > {
> > int ret = blk_check_request(blk, sector_num, nb_sectors);
> > if (ret < 0) {
> > @@ -673,6 +674,13 @@ BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk,
> > int64_t sector_num,
> > cb, opaque);
> > }
> >
> > +BlockAIOCB *blk_aio_write_zeroes(BlockBackend *blk, int64_t sector_num,
> > + int nb_sectors, BdrvRequestFlags flags,
> > + BlockCompletionFunc *cb, void *opaque)
> > +{
> > + return replay_aio_write_zeroes(blk, sector_num, nb_sectors, flags, cb,
> > opaque);
> > +}
> > +
>
> > +BlockAIOCB *replay_aio_write_zeroes(BlockBackend *blk, int64_t sector_num,
> > + int nb_sectors, BdrvRequestFlags flags,
> > + BlockCompletionFunc *cb, void *opaque)
> > +{
> > + if (replay_mode == REPLAY_MODE_NONE) {
> > + return blk_aio_write_zeroes_impl(blk, sector_num, nb_sectors,
> > flags, cb, opaque);
> > + } else {
> > + ReplayAIOCB *acb =
> > replay_aio_create(REPLAY_ASYNC_EVENT_BLOCK_WRITE_ZEROES,
> > + blk, cb, opaque);
> > + acb->req.sector = sector_num;
> > + acb->req.nb_sectors = nb_sectors;
> > + acb->req.flags = flags;
> > + replay_add_event(REPLAY_ASYNC_EVENT_BLOCK_WRITE_ZEROES, acb, NULL,
> > 0);
> > +
> > + return &acb->common;
> > + }
> > +}
>
> I think it's obvious that adding two functions to the call chain which
> do nothing in the common case is a bit ugly. If we did this for every
> feature that could possibly be enabled, we'd end up with two-kilometer
> stack traces.
>
> So definitely don't call into replay.c, which just calls back in 99.9%
> of the cases, but if anything, do the check in block-backends.c.
This way seems to be better.
> But even this doesn't feel completely right, because block drivers are
> already layered and there is no need to hardcode something optional (and
> rarely used) in the hot code path that could just be another layer.
>
> I assume that you know beforehand if you want to replay something, so
> requiring you to configure your block devices with a replay driver on
> top of the stack seems reasonable enough.
I cannot use block drivers for this. When driver functions are used, QEMU
is already used coroutines (and probably started bottom halves).
Coroutines make execution non-deterministic.
That's why we have to intercept blk_aio_ functions, that are called
deterministically.
Pavel Dovgalyuk
