[Ovirt-devel] Re: [PATCH server] allow admin to setup iptables port forwarding on server for a vm's vnc port

[David Lutterkort]

On Mon, 2009-02-02 at 17:48 -0500, Mohammed Morsi wrote:
> David Lutterkort wrote:
> > On Wed, 2009-01-28 at 20:16 -0500, Mohammed Morsi wrote:
> >
> > To address the race condition, the simplest solution is to take an
> > exclusive table lock on the vms table (before listing the assigned vnc
> > ports !) and hold that until the vm is saved in the DB, i.e. the end of
> > the current transaction.
> >
> >   
> This is not done as I'm a little concerned about locking the table (what
> if the user doesn't submit the form and walks away, will it stay
> locked?).

I must have misunderstood the logic: I thought the port-assignment logic
was happening as part of saving the VM, not when the form is presented
to the user.

>  I changed the additions to the server such that the
> autogenerated port isn't displayed in the form, eg the user is prompted
> to set the port or leave it at '0' after which the server will
> autogenerate it immediately before saving.

Yes, that's the flow that makes sense.

> Because I'm not employing
> locks, I believe the race condition you described is still possible, but
> the rails validation / errors framework should take care of that, as
> when the user submits the form, the uniqueness constraint will be
> checked and fail, resulting in the save operation not completing and an
> error message being presented in the wui indicating that the port is
> already in use and to select another (or autoassign it).

Kicking out that error when the user wants 'autoassign' is pretty
confusing.

As long as you take the lock inside a transaction, it will be released
at the end of the transaction, i.e. something like this _should_ do the
right thing

        Vm.transaction do
          # Whatever Rails magic is needed to do 'LOCK TABLE vms EXCLUSIVE;'
          ... determine vnc port ...
          ... save VM ...
        end

David