Re: [PATCH] RFC: alias rework

[Izik Eidus <ieidus@redhat.com>]

On Mon, 25 Jan 2010 18:49:25 -0200
Marcelo Tosatti <mtosatti@redhat.com> wrote:

> On Mon, Jan 25, 2010 at 10:40:32PM +0200, Izik Eidus wrote:
> > On Mon, 25 Jan 2010 18:20:39 -0200
> > Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > 
> > > With current code, if a memslot is deleted, access through any aliases
> > > that use it will fail (BTW it looks this is not properly handled, but
> > > thats a separate problem).
> > 
> > 
> > Yea I had some still open concerns about this code (this why I sent it on RFC)
> > 
> > > 
> > > So AFAICS there is no requirement for an alias to continue "operable" 
> > > if its parent memslot is deleted.
> > 
> > 
> > With this patch alias will stop to opearte when the parent is deleted
> > just like the behivor with the current code...
> > 
> > base_gfn will be set to 0 and npages will be set to 0 as well
> > (the true values wil be hide in real_base_gfn...), so gfn_to_memslot
> > and gfn_to_page will fail....
> 
> But you adjust the alias (and keep it valid) if dirty logging is
> enabled?

I am sorry, but probaby you got confused beacuse the code is wrong
the adjust of aliasing should happen in every case of:
 if(slot->rmap -> valid (!NULL)):
     this mean we got NEW parent slot that mapped into the gfn
     that the alias is mapped to, and we want the userspace address
     of the alias slot to intersect with the new parent slot.

and the latter adjustmant of the dirty_bitmap should happen only in case
of - if(slot->dirty_bitmap -> valid (!NULL)):
     the alias slot need to mark_page_dirty the bitmap of the new parent slot

I hope this will make things more clear
(I think there is another small issue there, but I will send it when it wont be RFC)

> 
> > > 
> > > Or is this a feature you need?
> > 
> > 
> > I dont need it (I asked Avi to do something), So he said he want to nuke the aliasing
> > from kvm and keep supporting the old userspace`s
> 
> With feature i meant keeping the alias around when parent slot is
> deleted.


The code doesnt try to do this, infact:
	} else if (!slot->rmap) {
		alias_memslot->base_gfn = 0;
		alias_memslot->npages = 0;
	}
came to invalidate the alias slot.

Sorry if I made to much mess :).

> 
> > Do you have any other way to achive this?
> 
> No.
> 
> > Btw I do realize it might be better not to push this patch and just keep the old
> > way of treating aliasing as we have now, I really don`t mind.
> > 
> > > 
> > > Motivation is that nukeing aliases is simpler than adjusting them.
> > > 
> > 
> > Agree.
>