Re: [PATCH v2 3/3] replay: do not build if TCG is not available

[Claudio Fontana <cfontana@suse.de>]

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
>>
>