Skip to content

Commit

Permalink
memory: Add Error** argument to the global_dirty_log routines
Browse files Browse the repository at this point in the history
Now that the log_global*() handlers take an Error** parameter and
return a bool, do the same for memory_global_dirty_log_start() and
memory_global_dirty_log_stop(). The error is reported in the callers
for now and it will be propagated in the call stack in the next
changes.

To be noted a functional change in ram_init_bitmaps(), if the dirty
pages logger fails to start, there is no need to synchronize the dirty
pages bitmaps. colo_incoming_start_dirty_log() could be modified in a
similar way.

Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Paul Durrant <paul@xen.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Hyman Huang <yong.huang@smartx.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Acked-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-12-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
  • Loading branch information
legoater authored and xzpeter committed Apr 23, 2024
1 parent 92c20b2 commit 639ec3f
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 12 deletions.
2 changes: 1 addition & 1 deletion hw/i386/xen/xen-hvm.c
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
{
if (enable) {
memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION, errp);
} else {
memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
}
Expand Down
5 changes: 4 additions & 1 deletion include/exec/memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -2570,8 +2570,11 @@ void memory_listener_unregister(MemoryListener *listener);
* memory_global_dirty_log_start: begin dirty logging for all regions
*
* @flags: purpose of starting dirty log, migration or dirty rate
* @errp: pointer to Error*, to store an error if it happens.
*
* Return: true on success, else false setting @errp with error.
*/
void memory_global_dirty_log_start(unsigned int flags);
bool memory_global_dirty_log_start(unsigned int flags, Error **errp);

/**
* memory_global_dirty_log_stop: end dirty logging for all regions
Expand Down
13 changes: 11 additions & 2 deletions migration/dirtyrate.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,15 @@ static int64_t do_calculate_dirtyrate(DirtyPageRecord dirty_pages,

void global_dirty_log_change(unsigned int flag, bool start)
{
Error *local_err = NULL;
bool ret;

bql_lock();
if (start) {
memory_global_dirty_log_start(flag);
ret = memory_global_dirty_log_start(flag, &local_err);
if (!ret) {
error_report_err(local_err);
}
} else {
memory_global_dirty_log_stop(flag);
}
Expand Down Expand Up @@ -608,9 +614,12 @@ static void calculate_dirtyrate_dirty_bitmap(struct DirtyRateConfig config)
{
int64_t start_time;
DirtyPageRecord dirty_pages;
Error *local_err = NULL;

bql_lock();
memory_global_dirty_log_start(GLOBAL_DIRTY_DIRTY_RATE);
if (!memory_global_dirty_log_start(GLOBAL_DIRTY_DIRTY_RATE, &local_err)) {
error_report_err(local_err);
}

/*
* 1'round of log sync may return all 1 bits with
Expand Down
23 changes: 21 additions & 2 deletions migration/ram.c
Original file line number Diff line number Diff line change
Expand Up @@ -2862,18 +2862,32 @@ static void migration_bitmap_clear_discarded_pages(RAMState *rs)

static void ram_init_bitmaps(RAMState *rs)
{
Error *local_err = NULL;
bool ret = true;

qemu_mutex_lock_ramlist();

WITH_RCU_READ_LOCK_GUARD() {
ram_list_init_bitmaps();
/* We don't use dirty log with background snapshots */
if (!migrate_background_snapshot()) {
memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
ret = memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION,
&local_err);
if (!ret) {
error_report_err(local_err);
goto out_unlock;
}
migration_bitmap_sync_precopy(rs, false);
}
}
out_unlock:
qemu_mutex_unlock_ramlist();

if (!ret) {
ram_bitmaps_destroy();
return;
}

/*
* After an eventual first bitmap sync, fixup the initial bitmap
* containing all 1s to exclude any discarded pages from migration.
Expand Down Expand Up @@ -3665,6 +3679,8 @@ int colo_init_ram_cache(void)
void colo_incoming_start_dirty_log(void)
{
RAMBlock *block = NULL;
Error *local_err = NULL;

/* For memory_global_dirty_log_start below. */
bql_lock();
qemu_mutex_lock_ramlist();
Expand All @@ -3676,7 +3692,10 @@ void colo_incoming_start_dirty_log(void)
/* Discard this dirty bitmap record */
bitmap_zero(block->bmap, block->max_length >> TARGET_PAGE_BITS);
}
memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
if (!memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION,
&local_err)) {
error_report_err(local_err);
}
}
ram_state->migration_dirty_pages = 0;
qemu_mutex_unlock_ramlist();
Expand Down
11 changes: 5 additions & 6 deletions system/memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -2937,10 +2937,9 @@ static bool memory_global_dirty_log_do_start(Error **errp)
return false;
}

void memory_global_dirty_log_start(unsigned int flags)
bool memory_global_dirty_log_start(unsigned int flags, Error **errp)
{
unsigned int old_flags;
Error *local_err = NULL;

assert(flags && !(flags & (~GLOBAL_DIRTY_MASK)));

Expand All @@ -2952,25 +2951,25 @@ void memory_global_dirty_log_start(unsigned int flags)

flags &= ~global_dirty_tracking;
if (!flags) {
return;
return true;
}

old_flags = global_dirty_tracking;
global_dirty_tracking |= flags;
trace_global_dirty_changed(global_dirty_tracking);

if (!old_flags) {
if (!memory_global_dirty_log_do_start(&local_err)) {
if (!memory_global_dirty_log_do_start(errp)) {
global_dirty_tracking &= ~flags;
trace_global_dirty_changed(global_dirty_tracking);
error_report_err(local_err);
return;
return false;
}

memory_region_transaction_begin();
memory_region_update_pending = true;
memory_region_transaction_commit();
}
return true;
}

static void memory_global_dirty_log_do_stop(unsigned int flags)
Expand Down

0 comments on commit 639ec3f

Please sign in to comment.