On Wed, 4 Jan 2012, Anthony PERARD wrote:
> A recent patch set from Avi break the dirty bitmap support of Xen. But this is
> because the internal function will return an error for an unhandled memory
> region (a0000 - bffff).
s/the internal function/xen_sync_dirty_bitmap
> But this is not an error. So the patch clarify the
> function from this point of view.
Please change the two lines above into
"However this is not a fatal error, so we should just continue instead
of aborting."
> There is now an error print when the Xen call failed.
printed
> Signed-off-by: Anthony PERARD <anthony.per...@citrix.com>
please send again with a fixed commit message
> ---
> xen-all.c | 38 ++++++++++++++++++--------------------
> 1 files changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/xen-all.c b/xen-all.c
> index dc23265..2aa8c03 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -403,9 +403,9 @@ static void xen_region_del(MemoryListener *listener,
> xen_set_memory(listener, section, false);
> }
>
> -static int xen_sync_dirty_bitmap(XenIOState *state,
> - target_phys_addr_t start_addr,
> - ram_addr_t size)
> +static void xen_sync_dirty_bitmap(XenIOState *state,
> + target_phys_addr_t start_addr,
> + ram_addr_t size)
> {
> target_phys_addr_t npages = size >> TARGET_PAGE_BITS;
> target_phys_addr_t vram_offset = 0;
> @@ -417,21 +417,27 @@ static int xen_sync_dirty_bitmap(XenIOState *state,
> physmap = get_physmapping(state, start_addr, size);
> if (physmap == NULL) {
> /* not handled */
> - return -1;
> + return;
> }
>
> if (state->log_for_dirtybit == NULL) {
> state->log_for_dirtybit = physmap;
> } else if (state->log_for_dirtybit != physmap) {
> - return -1;
> + /* Only one range for dirty bitmap can be tracked. */
> + return;
> }
> vram_offset = physmap->phys_offset;
>
> rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
> start_addr >> TARGET_PAGE_BITS, npages,
> bitmap);
> - if (rc) {
> - return rc;
> + if (rc < 0) {
> + if (rc != -ENODATA) {
> + fprintf(stderr, "xen: track_dirty_vram failed (0x" TARGET_FMT_plx
> + ", 0x" TARGET_FMT_plx "): %s\n",
> + start_addr, start_addr + size, strerror(-rc));
> + }
> + return;
> }
>
> for (i = 0; i < ARRAY_SIZE(bitmap); i++) {
> @@ -442,40 +448,32 @@ static int xen_sync_dirty_bitmap(XenIOState *state,
> cpu_physical_memory_set_dirty(vram_offset + (i * width + j) *
> TARGET_PAGE_SIZE);
> };
> }
> -
> - return 0;
> }
>
> static void xen_log_start(MemoryListener *listener,
> MemoryRegionSection *section)
> {
> XenIOState *state = container_of(listener, XenIOState, memory_listener);
> - int r;
>
> - r = xen_sync_dirty_bitmap(state, section->offset_within_address_space,
> - section->size);
> - assert(r >= 0);
> + xen_sync_dirty_bitmap(state, section->offset_within_address_space,
> + section->size);
> }
>
> static void xen_log_stop(MemoryListener *listener, MemoryRegionSection
> *section)
> {
> XenIOState *state = container_of(listener, XenIOState, memory_listener);
> - int r;
>
> state->log_for_dirtybit = NULL;
> /* Disable dirty bit tracking */
> - r = xc_hvm_track_dirty_vram(xen_xc, xen_domid, 0, 0, NULL);
> - assert(r >= 0);
> + xc_hvm_track_dirty_vram(xen_xc, xen_domid, 0, 0, NULL);
> }
>
> static void xen_log_sync(MemoryListener *listener, MemoryRegionSection
> *section)
> {
> XenIOState *state = container_of(listener, XenIOState, memory_listener);
> - int r;
>
> - r = xen_sync_dirty_bitmap(state, section->offset_within_address_space,
> - section->size);
> - assert(r >= 0);
> + xen_sync_dirty_bitmap(state, section->offset_within_address_space,
> + section->size);
> }
>
> static void xen_log_global_start(MemoryListener *listener)
> --
> tg: (61ec004..) fix/sync-dirty-bitmap (depends on: master)
>
