On 10/13/20 10:05 AM, Paolo Bonzini wrote: > On 13/10/20 09:56, Claudio Fontana wrote: >> Hi Paolo, >> >> On 10/13/20 12:29 AM, Paolo Bonzini wrote: >>> On 12/10/20 23:45, Claudio Fontana wrote: >>>> + ctx = blk_get_aio_context(blk); >>>> + if (!replay_bh_schedule_oneshot_event(ctx, error_callback_bh, acb)) { >>>> + /* regular case without replay */ >>>> + aio_bh_schedule_oneshot(ctx, error_callback_bh, acb); >>>> + } >>> >>> Why can't the stub just call aio_bh_schedule_oneshot? >> >> Absolutely, it can, I just considered the option and dropped it in the end. >> >>> This makes the API even more complicated. >> >> In my view not really, the API just returns a boolean that tells you if the >> event was consumed or not. > > The question to ask is, is there _any_ other way to use > replay_bh_schedule_oneshot_event other than > > if (!replay_bh_schedule_oneshot_event(ctx, error_callback_bh, acb)) { > aio_bh_schedule_oneshot(ctx, error_callback_bh, acb); > } > > and I think there isn't. Your point of avoiding functional code in the stubs > is also valid though. > > Perhaps you could have replay_bh_schedule_oneshot_event as you have it now, > but > also add a wrapper (called for example replay_bh_schedule_oneshot) that takes > care of calling aio_bh_schedule_oneshot too. But in my opinion the "if" has
Hi Paolo, hmm, in my view the wrapper should not be "replay_" though, what about block_bh_schedule_oneshot_event, for example, where we would move the "if replay is built-in and enabled" logic from the replay framework? Also interesting note, replay code hooks all aio_bg_schedule_oneshot in block, with the exception of: nbd.c export/export.c Is this wanted? Ciao, Claudio > no place in block/io.c. > > Paolo > >> If people feel strongly that this is a wrong step, we can do the alternative >> and put production code inside the stubs, but it just seems wrong. >> >> Thanks! >> >> Ciao, >> >> Claudio >> >