[Ovirt-devel] Re: [PATCH] added ability to reprovision vm via edit vm form

[Scott Seago]

Mohammed Morsi wrote:
> Scott Seago wrote:
>   
>> Mohammed Morsi wrote:
>>     
>>> ---
>>>  wui/src/app/controllers/vm_controller.rb |   73
>>> +++++++++++++++++++++++++-----
>>>  wui/src/app/views/vm/_form.rhtml         |    6 ++-
>>>  2 files changed, 65 insertions(+), 14 deletions(-)
>>>
>>>
>>>   
>>>       
>> This all looks good so far, but I think there's still something
>> missing. From what I see here, I don't think the cobbler system is 
>> being created in the reprovisioning case.
>>     
> In the patch, the very first line added (to VmController.update) is a
> call to _setup_vm_provision(params), a method I added below to update
> the cobbler system if found, else create it. If I am misunderstanding
> something, please clarify.
>
>   
Ugh. not enough coffee today -- somehow I glossed right over that new bit.
>   
>> Right the cobbler system is created in taskomatic in the create_vm
>> action, but this won't be run in the reprovisioning case.
>>     
> You bring up a good point here about an inconsistency, in the create vm
> case, taskomatic handles creating the vm, where as in the edit case
> which I added with this patch, the vm controller takes care of it. Will
> probably work, but as mentioned it is not consistent and probably should
> be taken care of by one component. Right now we could probably easily
> add a call to my new _setup_vm_provision(params) method in
> VmController.create
>
>   
Yeah -- we need to do them in the same place. As it is with this patch, 
there's a bit of a race condition if you edit a vm right after creating 
(but before taskomatic executes create_vm). Originally I'd made the 
decision to put the system create in taskomatic, since that's where all 
the other action-oriented bits that call out to external servers are 
handled. It's probably not that big a deal either way though, as we're 
already making cobbler calls from the WUI to get the profile list.

But yes, we need to either put both of these calls in taskomatic or gut 
create_vm and put both in the WUI. My original preference here was for 
taskomatic to handle it, but the reprovision case makes that much more 
complex (since we'd need a new task type, etc), so maybe it makes more 
sense to do it all in the WUI.

Any other opinions on this?
>> happen when we're not reprovisioning.
>>     
> Is there a definitive reason for making taskomatic handle the cobbler
> update as opposed to the rails application?
>
>
>   
The original reason was to pull anything that's not instantaneous into 
taskomatic -- although the cobbler calls should be fairly quick, so it 
may be no big deal either way -- as long as we do both new and 
reprovisioning bits the same way.

Scott
>   -Mo
>