Hi Cedric,
On 09/02/2024 12:14, Cédric Le Goater wrote:
External email: Use caution opening links or attachments
On 2/8/24 06:48, Peter Xu wrote:
On Wed, Feb 07, 2024 at 02:33:36PM +0100, Cédric Le Goater wrote:
@@ -2936,14 +2940,14 @@ void memory_global_dirty_log_start(unsigned
int flags)
trace_global_dirty_changed(global_dirty_tracking);
if (!old_flags) {
- MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward);
+ MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward, errp);
memory_region_transaction_begin();
memory_region_update_pending = true;
memory_region_transaction_commit();
}
}
-static void memory_global_dirty_log_do_stop(unsigned int flags)
+static void memory_global_dirty_log_do_stop(unsigned int flags,
Error **errp)
{
assert(flags && !(flags & (~GLOBAL_DIRTY_MASK)));
assert((global_dirty_tracking & flags) == flags);
@@ -2955,7 +2959,7 @@ static void
memory_global_dirty_log_do_stop(unsigned int flags)
memory_region_transaction_begin();
memory_region_update_pending = true;
memory_region_transaction_commit();
- MEMORY_LISTENER_CALL_GLOBAL(log_global_stop, Reverse);
+ MEMORY_LISTENER_CALL_GLOBAL(log_global_stop, Reverse, errp);
}
}
I'm a little bit surprised to see that MEMORY_LISTENER_CALL_GLOBAL()
already allows >2 args, with the ability to conditionally pass over errp
with such oneliner change; even if all callers were only using 2 args
before this patch..
yes. The proposal takes the easy path.
Should we change all memory listener global handlers :
begin
commit
log_global_after_sync
log_global_start
log_global_stop
to take an extra Error **errp argument ?
I think we should distinguish begin + commit handlers from the
log_global_*
with a new macro. In which case, we could also change the handler to
return
a bool and fail at the first error in MEMORY_LISTENER_CALL_GLOBAL(...).
I think we must fail at first error in any case. Otherwise, if two
handlers error and call error_setg() with errp, the second handler will
assert IIUC.
