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

[George Dunlap <george.dunlap@eu.citrix.com>]

On 02/19/2014 08:55 AM, Jan Beulich wrote:
>>>> On 19.02.14 at 02:28, "Zhang, Yang Z" <yang.z.zhang@intel.com> 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.

  -George