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

[David Lutterkort]

On Mon, 2009-04-20 at 15:13 -0700, Ian Main wrote:
> > +  def svc_create(vm_hash, start_now)
> > +    # from before_filter
> > +    vm_hash[:state] = Vm::STATE_PENDING
> > +    @vm = Vm.new(params[:vm])
> > +    @perm_obj = @vm.vm_resource_pool
> > +    @current_pool_id=@perm_obj.id
> > +    authorized!(Privilege::MODIFY)
> > +
> > +    alert = "VM was successfully created."
> > +    Vm.transaction do
> > +      @vm.save!
> > +      vm_provision
> > +      @task = VmTask.new({ :user        => @user,
> > +                           :task_target => @vm,
> > +                           :action      => VmTask::ACTION_CREATE_VM,
> > +                           :state       => Task::STATE_QUEUED})
> > +      @task.save!
> > +      if start_now
> > +        if @vm.get_action_list.include?(VmTask::ACTION_START_VM)
> > +          @task = VmTask.new({ :user        => @user,
> > +                               :task_target => @vm,
> > +                               :action      => VmTask::ACTION_START_VM,
> > +                               :state       => Task::STATE_QUEUED})
> > +          @task.save!
> 
> I can see this is a bit odd.. if we're calling this from QMF then
> there's going to be issues here..  In the current API I don't have
> create VM returning a task, which is kind of beside the point, but if
> it did then we would have to have a QMF object of this task figured
> out when the command returned (there are other APIs that do return
> tasks like start_vm).  I'm not entirely sure how that would work using
> this API right now.. we'd have to somehow figure out which task it was
> that was put in the DB.. I wonder if there is a better way to approach
> this?

It depends on which task the QMF API would return, but in general the
last task created by this method would be available in the QMF agent as
@task.

If you needed something else, we could change the above code to remember
it somewhere else - these are all internal API's, and we can change
things around as we go along. I expect that once we start implementing
the QMF agent, we'll discover lots of places where things need to be
changed to be more convenient for QMF purposes.

> I'm not going to argue this hard here because I know this is a first cut
> and just a refactoring but I do think there should be no tasks
> associated with creating a VM definition for the API.  I think we've
> talked about this before.. not a big deal, just wanted to bring it up
> for future reference :).

Yeah, I think we should see if we can kill the CREATE_VM task
altogether, but as you said, it's a little besides the point, since it's
orthogonal to the refactoring happening right now.

David