[Ovirt-devel] [PATCH server] Revised attempt at refactoring the vm_controller using the service layer.

[Ian Main]

On Wed, 22 Apr 2009 14:19:11 -0400
Scott Seago <sseago at redhat.com> wrote:

> Ian Main wrote:
> > On Wed, 22 Apr 2009 12:35:55 -0400
> > Scott Seago <sseago at redhat.com> wrote:
> >
> >   
> >> Ian Main wrote:
> >>     
> >>> On Tue, 21 Apr 2009 21:49:15 +0000
> >>> David Lutterkort <lutter at redhat.com> wrote:
> >>>
> >>>   
> >>>       
> >>>> On Tue, 2009-04-21 at 13:14 -0700, Ian Main wrote:
> >>>>     
> >>>>         
> >>>>> So with this service layer.. with it being included and setting local
> >>>>> properties of the object, will that be thread safe?  Or will we have
> >>>>> to do locking around each call?
> >>>>>
> >>>>> I'm not sure yet if the agent will be threaded.. but it certainly
> >>>>> could have a thread per method invocation..
> >>>>>       
> >>>>>           
> >>>> To echo what Scott said: I would prefer if we followed the rails model
> >>>> fairly closely, with some sort of notion of a 'QMF controller', and
> >>>> instantiate one for every method call from the QMF agent.
> >>>>
> >>>> Given that we want to minimize the amount of things we keep in memory
> >>>> (mostly to avoid cache coherency issues between memory and DB), we want
> >>>> the QMF API to be as statefree as possible.
> >>>>     
> >>>>         
> >>> Yeah that makes sense.. the only place where it might be a little odd is
> >>> in setting properties.  When properties are set it seems the way to do it
> >>> is to load the vm properties hash from an active record and then call
> >>> update() to have everything verified the same way as in rails?
> >>>
> >>> I'm not sure if there's any other way to verify the contents of various
> >>> fields using the active record directly?  Either way it seems we need
> >>> direct acccess to it as well.. is that a problem?
> >>>
> >>>     Ian
> >>>
> >>>   
> >>>       
> >> Yes, to make persistent changes to VM properties here you would need to 
> >> use the svc_update call. Or are you saying you also want to make changes 
> >> to activerecord objects outside the service API layer? This would 
> >> certainly be possible -- and if you call save! then any validation 
> >> errors will raise exceptions just like they do in the svc method, 
> >> although you'd lose the permission checks and any other business logic 
> >> that is in the service layer.
> >>     
> >
> > I think in many instances it would be easier to simply change the active record directly.  If you think about something like..
> >
> > vm = ovirt.create_vm_def(...)
> > vm.description = 'changed description'
> >
> > in the backend there will be a call to the 'description' method of a
> > VmDef object.  So in ruby we'll have eg:
> >
> > class VmDef
> >
> >   def description(new_desc)
> >     vm = vm_ar.reload
> >     vm.description = new_desc
> >     vm.save!
> >   end
> >
> > ...
> >
> > Otherwise you have to load the VM active record and make a hash from it
> > (?? maybe that's easy?) and then call svc_update() with the new hash..
> > maybe it would be possible to just have the permission check callable
> > directly?  Not sure what is best..
> >
> >     Ian
> >
> >   
> For edits you only need to include changed attributes in the hash. So 
> you could call svc_obj.svc_update(vm_id, {:description => 'changed 
> description'}).

OK that's cool then.  That will work fine.

> I'm still slightly confused on how VmDef relates to the service classes. 
> Is VmDef the class that includes the VmService module? Or is VmDef 
> something longer-lived and more related to the activerecord stuff directly?

Well, I'm still getting my head around this new agent stuff :).
Traditionally in QMF there would be a long-lived VmDef object for every
VM active record. However, we're working out the callback system right
now and it looks like it will essentially become a lot like a web
request in that we will receive an event something like "what are the
properties of object with id 12341234?" and we'll look that up via AR
and return it.  Same for methods.. "call method create for object with
id 12435125" and we'll have to look that up via AR and then call
into the service API.

So it sounds like your concern regarding long lived objects should be 
taken care of.  Somewhere in there though it would be nice to have a
class that looks something like the schema we're implementing.. even if
it gets instantiated for every request.

Also, somehow we're going to need a way to map IDs to records in the
database on a per-agent-instance basis (eg they can change across
restarts of the agent).  I could create some kind of mapping in memory
or we could add fields to the active records.. It may be possible to
make them persistent too, not sure on that one.

    Ian