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(...). Thanks, C.
