Ovirt devel - May 2008 - [PATCH] support optimistic locking with lock_version

Hi Ovirt,

Edited the migration files to add the :lock_version column, which will
allow us to use optimistic locking by default.

We can override this with more restrictive locking when needed, but
this should help catch stuff that might slip through the cracks.

Thanks,
Steve
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: lock_version.txt
URL:
<http://listman.redhat.com/archives/ovirt-devel/attachments/20080521/711c6e21/attachment.txt>

On Wed, May 21, 2008 at 08:23:07PM -0500, steve linabery
wrote:
> Hi Ovirt,
> 
> Edited the migration files to add the :lock_version column, which will
> allow us to use optimistic locking by default.
> 
> We can override this with more restrictive locking when needed, but
> this should help catch stuff that might slip through the cracks.
> 
Thanks Steve. I have a couple of questions in-line below, please
indulge my ignorance if you would...
> From 65af40d990b6d881f927609da7b63cbcbabe9e57 Mon Sep 17 00:00:00 2001
> From: Steve Linabery <slinabery at gmail.com>
> Date: Wed, 21 May 2008 20:16:39 -0500
> Subject: [PATCH] Add lock_version attribute to all objects to enable
optimistic locking.
>  Edit migration files to use the 't.type :fieldname' syntax.
> 
> 
> Signed-off-by: Steve Linabery <slinabery at gmail.com>
> ---
>  wui/src/db/migrate/001_create_pools.rb           |    3 +-
>  wui/src/db/migrate/002_create_hosts.rb           |   21 ++++++------
>  wui/src/db/migrate/003_create_nics.rb            |   15 +++++----
>  wui/src/db/migrate/004_create_storage_volumes.rb |   28 +++++++++--------
>  wui/src/db/migrate/005_create_quotas.rb          |   15 +++++----
>  wui/src/db/migrate/006_create_vms.rb             |   37
++++++++++++----------
>  wui/src/db/migrate/007_create_tasks.rb           |   30 ++++++++++--------
>  wui/src/db/migrate/008_create_permissions.rb     |    7 ++--
>  8 files changed, 85 insertions(+), 71 deletions(-)
> 
> diff --git a/wui/src/db/migrate/001_create_pools.rb
b/wui/src/db/migrate/001_create_pools.rb
> index 2348013..34c01d3 100644
> --- a/wui/src/db/migrate/001_create_pools.rb
> +++ b/wui/src/db/migrate/001_create_pools.rb
> @@ -1,4 +1,4 @@
> -# 
> +#
>  # Copyright (C) 2008 Red Hat, Inc.
>  # Written by Scott Seago <sseago at redhat.com>
>  #
> @@ -25,6 +25,7 @@ class CreatePools < ActiveRecord::Migration
>        t.integer :parent_id
>        t.integer :lft
>        t.integer :rgt
> +      t.integer :lock_version, :default => 0
>        t.timestamps
>      end
>  
> diff --git a/wui/src/db/migrate/002_create_hosts.rb
b/wui/src/db/migrate/002_create_hosts.rb
> index 62d9edd..e8b26f2 100644
> --- a/wui/src/db/migrate/002_create_hosts.rb
> +++ b/wui/src/db/migrate/002_create_hosts.rb
> @@ -1,4 +1,4 @@
> -# 
> +#
>  # Copyright (C) 2008 Red Hat, Inc.
>  # Written by Scott Seago <sseago at redhat.com>
>  #
> @@ -20,15 +20,16 @@
>  class CreateHosts < ActiveRecord::Migration
>    def self.up
>      create_table :hosts do |t|
> -      t.column :uuid,                       :string
> -      t.column :hypervisor_type,            :string
> -      t.column :hostname,                   :string
> -      t.column :num_cpus,                   :integer
> -      t.column :cpu_speed,                  :integer
> -      t.column :arch,                       :string
> -      t.column :memory,                     :integer
> -      t.column :is_disabled,                :integer
> -      t.column :hardware_pool_id,           :integer, :null => false
> +      t.string  :uuid
> +      t.string  :hypervisor_type
> +      t.string  :hostname
> +      t.integer :num_cpus
> +      t.integer :cpu_speed
> +      t.string  :arch
> +      t.integer :memory
> +      t.integer :is_disabled
> +      t.integer :hardware_pool_id, :null => false
> +      t.integer :lock_version,     :default => 0
>      end
What's the reason for this change? Just tidying, or is there some
advantage to using e.g. "t.string" rather than "t.column :foo
:string"?

[snip]
> diff --git a/wui/src/db/migrate/008_create_permissions.rb
b/wui/src/db/migrate/008_create_permissions.rb
> index 0cc361c..69a0aa7 100644
> --- a/wui/src/db/migrate/008_create_permissions.rb
> +++ b/wui/src/db/migrate/008_create_permissions.rb
> @@ -1,4 +1,4 @@
> -# 
> +#
>  # Copyright (C) 2008 Red Hat, Inc.
>  # Written by Scott Seago <sseago at redhat.com>
>  #
> @@ -20,9 +20,10 @@
>  class CreatePermissions < ActiveRecord::Migration
>    def self.up
>      create_table :permissions do |t|
> -      t.string :user_role
> -      t.string :user
> +      t.string  :user_role
> +      t.string  :user
>        t.integer :pool_id
> +      t.integer :lock_version, :default => 0
>      end
>    end
>  
Other than that, looks good... I will ACK, but Scott can you take a
look before I commit?

Thanks,
--Hugh

On Wed, May 21, 2008 at 08:23:07PM -0500, steve linabery
wrote:
> Hi Ovirt,
> 
> Edited the migration files to add the :lock_version column, which will
> allow us to use optimistic locking by default.
> 
> We can override this with more restrictive locking when needed, but
> this should help catch stuff that might slip through the cracks.
> 
> Thanks,
> Steve
I have committed this and pushed to the main repo. Thanks!

--Hugh