[Ovirt-devel] [PATCH server] Taskomatic Refactoring and Qpidification Take 4

[Chris Lalancette]

Ian Main wrote:
> This version incorporates feedback from Chris Lancette.
> 
> This patch reworks taskomatic quite a bit.  This mostly just shifts
> taskomatic to using the qpid interface in place of ruby-libvirt.  It
> also fixes a few bugs I discovered a long the way and adds new ones
> I'm sure.  The only other thing added was round-robin host selection
> for VMs.
> 
> Where ever possible the hosts are queried directly using qpid rather than
> relying on states from the database.
> 
> This patch loses about 150 lines from the original taskomatic and moves
> most of the task implementation into a central class.  This was done to
> provide access to the qpid session as well as providing for locking/task
> ordering in future versions.
> 
> This requires the latest libvirt-qpid as it fixes a number of bugs.
> It's in the ovirt repository now.
> 
> Issues remaining:
> 
> - libvirt-qpid migrate is broken.  Since the migrate takes place on the
>   node instead of from the ovirt-appliance, the source node doesn't have
>   the ability to authenticate against the destination node.  For this
>   reason I'm still using ruby-libvirt migrate.  I talked to Chris about
>   this and we have a plan worked out. :)
> 
> - LVM volume scanning is not in this version.  I intend to address this
>   asap.
> 
> Signed-off-by: Ian Main <imain at redhat.com>


This looks pretty good; you fixed up most of my suggestions, and we agreed to
leave the LVM stuff out for the moment, but to make it top priority.  I only
have one thing left to say.  The db_vm.reload methods sprinkled everywhere still
make me nervous, and I don't understand the reason they should be needed (you
alluded to another patch, but I don't remember/see where that is).  I guess we
can leave them in for now to get testing, but I would like to see that
investigated; as I mentioned, the last time I got these it was indeed a problem
with my code.

Other than that, this is great.

-- 
Chris Lalancette