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

[Ian Main]

On Fri, 23 Jan 2009 13:43:49 +0100
Chris Lalancette <clalance at redhat.com> wrote:

> Chris Lalancette wrote:
> > 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.
> > 
> 
> Duh, I'm a moron.  Sorry, I forgot I had commented on that other thread.  I see
> now where the stale objects are coming from.  This still worries me, though; if
> the database can change between the first time you grab the db_vm object and
> later, why can't it change between the "reload" and the write?  I.e. it looks
> like there is still a bad race here.  Incidentally, this exact race is the one I
> was preventing with making database updates go through taskomatic, instead of
> directly manipulating the database.

OK, as per our conversation, we're going to switch to using real locking instead
of these optimistic locks.  I'm going to push this taskomatic now and then fix
LVM and database locking.

	Ian