Re: [Qemu-devel] [PATCH 5/6] Introduce kvm logging interface.

[Anthony Liguori <anthony@codemonkey.ws>]

Glauber Costa wrote:
> On Wed, Nov 19, 2008 at 1:18 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
>   
>
> Are you okay with all dprintfs having these info? This is very
> valuable info, so I'd
> like to have it. I  can send a separate patch.
>   

Yes.

>> Note that start and stop are identical except for a different printf().  The
>> call a common helper function.  Why not fold everything into
>> kvm_dirty_pages_log_change() and make that the public interface
>> (kvm_set_log).
>>     
>
> I personally believe kvm_set_log is a very bad interface. It's nicer
> to read "start"
> and "stop" instead, but I can definitely do this internally.
>   

It's not that important to me.  I'm more interested in removing the 
duplicated code.

>> This interface is weird and broken.  If we wanted to use this for live
>> migration, we would end up passing phys_offset=0 but that has a special
>> meaning here.
>>
>> But why are we passing phys_offset at all?  Why can't we do the lookup here?
>>     
> Because, if you remember, the last time I sent a patch _without_ it,
> you complained
> that we can't really trust any translation based on userspace_addr.
>   

But now that we have phys_offset, we know what the phys_offset is of a 
given slot, right?  So can't we just use that?

Regards,

Anthony Liguori