| Message ID | 1342531805-29894-5-git-send-email-anthony.perard@citrix.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 17 Jul 2012, Anthony PERARD wrote: > Because the call to track the dirty bit in the video ram during migration won't > work (it returns -1), we set dirtybit on the all video ram. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > xen-all.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/xen-all.c b/xen-all.c > index 498883b..00bdb50 100644 > --- a/xen-all.c > +++ b/xen-all.c > @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state, > return; > } > > + if (unlikely(xen_in_migration)) { > + /* track_dirty_vram does not work during migration */ > + memory_region_set_dirty(framebuffer, 0, size); > + return; > + } > rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid, > start_addr >> TARGET_PAGE_BITS, npages, > bitmap); Why are you setting the entire framebuffer dirty? We should set dirty only the actualy region that is supposed to be dirty.
On Tue, 17 Jul 2012, Stefano Stabellini wrote: > On Tue, 17 Jul 2012, Anthony PERARD wrote: > > Because the call to track the dirty bit in the video ram during migration won't > > work (it returns -1), we set dirtybit on the all video ram. > > > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > --- > > xen-all.c | 5 +++++ > > 1 files changed, 5 insertions(+), 0 deletions(-) > > > > diff --git a/xen-all.c b/xen-all.c > > index 498883b..00bdb50 100644 > > --- a/xen-all.c > > +++ b/xen-all.c > > @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state, > > return; > > } > > > > + if (unlikely(xen_in_migration)) { > > + /* track_dirty_vram does not work during migration */ > > + memory_region_set_dirty(framebuffer, 0, size); > > + return; > > + } > > rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid, > > start_addr >> TARGET_PAGE_BITS, npages, > > bitmap); > > Why are you setting the entire framebuffer dirty? > We should set dirty only the actualy region that is supposed to be > dirty. > Also memory_region_set_dirty calls cpu_physical_memory_set_dirty_range, in which you didn't add a call to xen_modified_memory.
On 17/07/12 19:30, Stefano Stabellini wrote: > On Tue, 17 Jul 2012, Stefano Stabellini wrote: >> On Tue, 17 Jul 2012, Anthony PERARD wrote: >>> Because the call to track the dirty bit in the video ram during migration won't >>> work (it returns -1), we set dirtybit on the all video ram. >>> >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >>> --- >>> xen-all.c | 5 +++++ >>> 1 files changed, 5 insertions(+), 0 deletions(-) >>> >>> diff --git a/xen-all.c b/xen-all.c >>> index 498883b..00bdb50 100644 >>> --- a/xen-all.c >>> +++ b/xen-all.c >>> @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state, >>> return; >>> } >>> >>> + if (unlikely(xen_in_migration)) { >>> + /* track_dirty_vram does not work during migration */ >>> + memory_region_set_dirty(framebuffer, 0, size); >>> + return; >>> + } >>> rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid, >>> start_addr >> TARGET_PAGE_BITS, npages, >>> bitmap); >> >> Why are you setting the entire framebuffer dirty? >> We should set dirty only the actualy region that is supposed to be >> dirty. I set the dirty bit on the all framebuffer because the track dirty call fail, so I don't which bits are dirty. This one is for QEMU to know which part of the screen need to be updated. > Also memory_region_set_dirty calls cpu_physical_memory_set_dirty_range, > in which you didn't add a call to xen_modified_memory.
On Fri, 20 Jul 2012, Anthony PERARD wrote: > On 17/07/12 19:30, Stefano Stabellini wrote: > > On Tue, 17 Jul 2012, Stefano Stabellini wrote: > >> On Tue, 17 Jul 2012, Anthony PERARD wrote: > >>> Because the call to track the dirty bit in the video ram during migration won't > >>> work (it returns -1), we set dirtybit on the all video ram. > >>> > >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > >>> --- > >>> xen-all.c | 5 +++++ > >>> 1 files changed, 5 insertions(+), 0 deletions(-) > >>> > >>> diff --git a/xen-all.c b/xen-all.c > >>> index 498883b..00bdb50 100644 > >>> --- a/xen-all.c > >>> +++ b/xen-all.c > >>> @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state, > >>> return; > >>> } > >>> > >>> + if (unlikely(xen_in_migration)) { > >>> + /* track_dirty_vram does not work during migration */ > >>> + memory_region_set_dirty(framebuffer, 0, size); > >>> + return; > >>> + } > >>> rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid, > >>> start_addr >> TARGET_PAGE_BITS, npages, > >>> bitmap); > >> > >> Why are you setting the entire framebuffer dirty? > >> We should set dirty only the actualy region that is supposed to be > >> dirty. > > I set the dirty bit on the all framebuffer because the track dirty call > fail, so I don't which bits are dirty. This one is for QEMU to know > which part of the screen need to be updated. In that case it would be better to set the entire bitmap to 0xff in case xc_hvm_track_dirty_vram fails, right? So that we don't need to special case live migration.
diff --git a/xen-all.c b/xen-all.c index 498883b..00bdb50 100644 --- a/xen-all.c +++ b/xen-all.c @@ -502,6 +502,11 @@ static void xen_sync_dirty_bitmap(XenIOState *state, return; } + if (unlikely(xen_in_migration)) { + /* track_dirty_vram does not work during migration */ + memory_region_set_dirty(framebuffer, 0, size); + return; + } rc = xc_hvm_track_dirty_vram(xen_xc, xen_domid, start_addr >> TARGET_PAGE_BITS, npages, bitmap);
Because the call to track the dirty bit in the video ram during migration won't work (it returns -1), we set dirtybit on the all video ram. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- xen-all.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)