Re: [qemu-upstream-4.2-testing test] 16779: regressions - FAIL

[Alex Bligh <alex@alex.org.uk>]

Stefano,

> It could be any of the last 6 commits: I would start by bisecting the
> problem further until you manage to pinpoint the precise backport that
> caused it.

My problem is trying to replicate it...

> But like you say, it is probably the last one.
> Consider that the changes introduced by the last commit are only useful
> for live-migration, you can try to remove one by one to figure out
> exactly what's breaking Windows install.
> My guess would be this chunk:

Me too. This was the bit that I couldn't actually work out what it was 
doing and why.

In the original patch (below) it called memory_region_set_dirty, which
doesn't exist in 4.2, with the parameter 'framebuffer', which also doesn't
exist in 4.2. This only gets called if xc_hvm_track_dirty_vram returns
an error other than ENODATA. I had presumed the intent was to mark the
whole framebuffer as dirty if xc_hvm_track_dirty_vram failed this way.

Does the loop start (and end) need to start from vram_offset (i.e.
physmap->phys_offset), rather than start_addr? I suspect my confusion
is that both start_addr and vram_offset are target_phys_addr_t but
one is one side of a mapping and one the other.

>
> @@ -470,7 +470,21 @@ static int xen_sync_dirty_bitmap(XenIOState *state,
>      rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid,
>                                   start_addr >> TARGET_PAGE_BITS, npages,
>                                   bitmap);
> -    if (rc) {
> +    if (rc < 0) {
> +        if (rc != -ENODATA) {
> +            ram_addr_t addr, end;
> +
> +            xen_modified_memory(start_addr, size);
> +
> +            end = TARGET_PAGE_ALIGN(start_addr + size);
> +            for (addr = start_addr & TARGET_PAGE_MASK; addr < end; addr
> += TARGET_PAGE_SIZE) { +
> cpu_physical_memory_set_dirty(addr);
> +            }
> +
> +            DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx
> +                    ", 0x" TARGET_FMT_plx "): %s\n",
> +                    start_addr, start_addr + size, strerror(-rc));
> +        }
>          return rc;
>      }
>
> in particular the loop around cpu_physical_memory_set_dirty
>
>


Original patch:

diff --git a/xen-all.c b/xen-all.c
index b11542c..e6308be 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -507,7 +507,8 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
                                  bitmap);
     if (rc < 0) {
         if (rc != -ENODATA) {
-            fprintf(stderr, "xen: track_dirty_vram failed (0x" 
TARGET_FMT_plx
+            memory_region_set_dirty(framebuffer, 0, size);
+            DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx
                     ", 0x" TARGET_FMT_plx "): %s\n",
                     start_addr, start_addr + size, strerror(-rc));
         }


-- 
Alex Bligh