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

[Anthony Liguori]

Glauber Costa wrote:
> On Wed, Nov 19, 2008 at 1:18 PM, Anthony Liguori <address@hidden> 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