Re: [Xen-devel] [PATCH] Don't track all memory when enabling log dirty to track vram

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

On 02/19/2014 11:13 AM, Jan Beulich wrote:
> > > > On 19.02.14 at 12:03, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> > On 02/19/2014 08:55 AM, Jan Beulich wrote:
> > > > > > On 19.02.14 at 02:28, "Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx> wrote:
> > > > George Dunlap wrote on 2014-02-18:
> > > > > On 02/18/2014 03:14 AM, Zhang, Yang Z wrote:
> > > > > perhaps my original patch is better which will check
> > > > > paging_mode_log_dirty(d) && log_global:
> > > > >
> > > > > It turns out that the reason I couldn't get a crash was because libxc
> > > > > was actually paying attention to the -EINVAL return value, and
> > > > > disabling and then re-enabling logdirty.  That's what would happen
> > > > > before your dirty vram patch, and that's what happens after.  And
> > > > > arguably, that's the correct behavior for any toolstack, given that the
> > > > interface returns an error.
> > > >
> > > > Agree.
> > > >
> > > > > This patch would actually change the interface; if we check this in,
> > > > > then if you enable logdirty when dirty vram tracking is enabled, you
> > > > > *won't* get an error, and thus *won't* disable and re-enable logdirty mode.
> > > > > So actually, this patch would be more disruptive.
> > > > Jan, do you have any comment?
> > > This simplistic variant is just calling for problems. As was already
> > > said elsewhere on this thread, we should simply do the mode change
> > > properly: Track that a partial log-dirty mode is in use, and allow
> > > switching to global log-dirty mode (converting all entries to R/O).
> > I think Yang was asking you for your opinion on my suggestion that
> > nothing actually needed to be done.  Enabling full logdirty mode for
> > migration when dirty vram tracking was enabled has *always* returned an
> > error (or at least for a long time now), and *always* resulted in the
> > toolstack disabling and re-enabling logdirty mode; Yang's patch doesn't
> > change that at all.
> >
> > If you think that's an interface we need to improve in the future, we
> > can put it on the list of improvements.  But at this point it seems to
> > me more like a nice-to-have.
> I agree - for 4.4.0 we shouldn't need any further adjustments. And
> I hoped to imply that I don't see a need for this incremental change
> to go in by having said "This simplistic variant is just calling for
> problems".
No, but "we should simply do the mode change properly" could be 
interpreted as saying, "this needs to be done as a follow-up to the 
dirty vram tracking patch"; someone might even interpret it as, "you 
need to do this as a follow-up".  That's what I was trying to clarify / 
express an opinion on. :-)
 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel