[Ovirt-devel] [PATCH server] Use Logger class in taskomatic

Ian Main imain at redhat.com
Mon Feb 16 21:11:49 UTC 2009


This patch switches from using puts to using the Logger class to perform
logging.  Much more reliable to log to a file this way.

Signed-off-by: Ian Main <imain at redhat.com>
---
 src/task-omatic/task_host.rb  |   33 ------------------
 src/task-omatic/taskomatic.rb |   76 +++++++++++++++++++++-------------------
 2 files changed, 40 insertions(+), 69 deletions(-)
 delete mode 100644 src/task-omatic/task_host.rb

diff --git a/src/task-omatic/task_host.rb b/src/task-omatic/task_host.rb
deleted file mode 100644
index 3d039fb..0000000
--- a/src/task-omatic/task_host.rb
+++ /dev/null
@@ -1,33 +0,0 @@
-# Copyright (C) 2008 Red Hat, Inc.
-# Written by Chris Lalancette <clalance at redhat.com>
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; version 2 of the License.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
-# MA  02110-1301, USA.  A copy of the GNU General Public License is
-# also available at http://www.gnu.org/copyleft/gpl.html.
-
-require 'utils'
-
-# FIXME: a little ugly to be including all of task_vm here, but
-# utils really isn't the right place for the migrate() method
-require 'task_vm'
-
-def clear_vms_host(task)
-  puts "clear_vms_host"
-
-  src_host = task.host
-
-  src_host.vms.each do |vm|
-    migrate(vm)
-  end
-end
diff --git a/src/task-omatic/taskomatic.rb b/src/task-omatic/taskomatic.rb
index a5cf6ed..65aac32 100755
--- a/src/task-omatic/taskomatic.rb
+++ b/src/task-omatic/taskomatic.rb
@@ -28,6 +28,7 @@ require 'monitor'
 require 'dutils'
 require 'optparse'
 require 'daemons'
+require 'logger'
 include Daemonize
 
 require 'task_vm'
@@ -79,11 +80,12 @@ class TaskOmatic
       pwd = Dir.pwd
       daemonize
       Dir.chdir(pwd)
-      lf = open($logfile, 'a')
-      $stdout = lf
-      $stderr = lf
+      @logger = Logger.new($logfile)
+    else
+      @logger = Logger.new(STDERR)
     end
 
+
     # this has to be after daemonizing now because it could take a LONG time to
     # actually connect if qpidd isn't running yet etc.
     qpid_ensure_connected
@@ -104,9 +106,12 @@ class TaskOmatic
         @broker = @session.add_broker("amqp://#{server}:#{port}", :mechanism => 'GSSAPI')
 
         # Connection succeeded, go about our business.
+        @logger.info "Connected to amqp://#{server}:#{port}"
         return
+
       rescue Exception => msg
-        puts "Error connecting to qpidd: #{msg}"
+        @logger.error "Error connecting to qpidd: #{msg}"
+        @logger.error msg.backtrace
       end
       sleep(sleepy)
       sleepy *= 2
@@ -116,7 +121,7 @@ class TaskOmatic
         # Could also be a credentials problem?  Try to get them again..
         get_credentials('qpidd')
       rescue Exception => msg
-        puts "Error getting qpidd credentials: #{msg}"
+        @logger.error "Error getting qpidd credentials: #{msg}"
       end
     end
   end
@@ -182,7 +187,7 @@ class TaskOmatic
       if db_pool == nil
         # Hum.  Specified by the VM description, but not in the storage pool?
         # continue on and hope for the best
-        puts "Couldn't find pool for volume #{db_volume.path}; skipping"
+        @logger.error "Couldn't find pool for volume #{db_volume.path}; skipping"
         next
       end
 
@@ -254,7 +259,7 @@ class TaskOmatic
     db_vm = task.vm
     vm = @session.object(:class => 'domain', 'uuid' => db_vm.uuid)
     if !vm
-      puts "VM already shut down?"
+      @logger.error "VM already shut down?"
       return
     end
 
@@ -287,7 +292,7 @@ class TaskOmatic
     # we can tell if this domain was migrated; that way, we can tell the
     # difference between a real undefine failure and one because of migration
     result = vm.undefine
-    puts "Error undefining VM: #{result.text}" unless result.status == 0
+    @logger.info "Error undefining VM: #{result.text}" unless result.status == 0
 
     teardown_storage_pools(node)
 
@@ -415,7 +420,7 @@ class TaskOmatic
     raise "Unable to locate VM to save" unless dom
 
     filename = "/tmp/#{dom.uuid}.save"
-    puts "saving vm #{dom.name} to #{filename}"
+    @logger.info "saving vm #{dom.name} to #{filename}"
     result = dom.save(filename)
     raise "Error saving VM: #{result.text}" unless result.status == 0
 
@@ -431,7 +436,7 @@ class TaskOmatic
     raise "Unable to locate VM to restore" unless dom
 
     filename = "/tmp/#{dom.uuid}.save"
-    puts "restoring vm #{dom.name} from #{filename}"
+    @logger.info "restoring vm #{dom.name} from #{filename}"
     result = dom.restore("/tmp/" + dom.uuid + ".save")
     raise "Error restoring VM: #{result.text}" unless result.status == 0
 
@@ -445,7 +450,7 @@ class TaskOmatic
     src_node = @session.object(:object_id => vm.node)
     raise "Unable to find node that VM is on??" unless src_node
 
-    puts "Migrating domain lookup complete, domain is #{vm}"
+    @logger.info "Migrating domain lookup complete, domain is #{vm}"
 
     vm_orig_state = db_vm.state
     set_vm_state(db_vm, Vm::STATE_MIGRATING)
@@ -485,13 +490,12 @@ class TaskOmatic
       # difference between a real undefine failure and one because of migration
       result = vm.undefine
 
-      # Note this is just a puts!  Not a raise! :)
-      puts "Error undefining old vm after migrate: #{result.text}" unless result.status == 0
+      @logger.info "Error undefining old vm after migrate: #{result.text}" unless result.status == 0
 
       # See if we can take down storage pools on the src host.
       teardown_storage_pools(src_node)
     rescue => ex
-      puts "Error: #{ex}"
+      @logger.error "Error: #{ex}"
       set_vm_state(db_vm, vm_orig_state)
       raise ex
     end
@@ -503,7 +507,7 @@ class TaskOmatic
   end
 
   def task_migrate_vm(task)
-    puts "migrate_vm"
+    @logger.info "migrate_vm"
 
     # here, we are given an id for a VM to migrate; we have to lookup which
     # physical host it is running on
@@ -514,10 +518,10 @@ class TaskOmatic
   def storage_find_suitable_host(hardware_pool)
     # find all of the hosts in the same pool as the storage
     hardware_pool.hosts.each do |host|
-      puts "storage_find_suitable_host: host #{host.hostname} uuid #{host.uuid}"
-      puts "host.is_disabled is #{host.is_disabled}"
+      @logger.info "storage_find_suitable_host: host #{host.hostname} uuid #{host.uuid}"
+      @logger.info "host.is_disabled is #{host.is_disabled}"
       if host.is_disabled.to_i != 0
-        puts "host #{host.hostname} is disabled"
+        @logger.info "host #{host.hostname} is disabled"
         next
       end
       node = @session.object(:class => 'node', 'hostname' => host.hostname)
@@ -537,7 +541,7 @@ class TaskOmatic
     storage_volume.lv_group_perms = group
     storage_volume.lv_mode_perms = mode
     storage_volume.state = StorageVolume::STATE_AVAILABLE
-    puts "saving storage volume to db."
+    @logger.info "saving storage volume to db."
     storage_volume.save!
   end
 
@@ -553,7 +557,7 @@ class TaskOmatic
   #                   represented in the database
 
   def task_refresh_pool(task)
-    puts "refresh_pool"
+    @logger.info "refresh_pool"
 
     db_pool_phys = task.storage_pool
     raise "Could not find storage pool" unless db_pool_phys
@@ -589,14 +593,14 @@ class TaskOmatic
           if not existing_vol
             add_volume_to_db(db_pool_phys, volume);
           else
-            puts "volume #{volume.name} already exists in db.."
+            @logger.error "volume #{volume.name} already exists in db.."
           end
 
           # Now check for an LVM pool carving up this volume.
           lvm_name = volume.childLVMName
           next if lvm_name == ''
 
-          puts "Child LVM exists for this volume - #{lvm_name}"
+          @logger.info "Child LVM exists for this volume - #{lvm_name}"
           lvm_db_pool = LvmStoragePool.find(:first, :conditions =>
                                           [ "vg_name = ?", lvm_name ])
           if lvm_db_pool == nil
@@ -635,7 +639,7 @@ class TaskOmatic
             if not existing_vol
               add_volume_to_db(lvm_db_pool, lvm_volume, "0744", "0744", "0744");
             else
-              puts "volume #{lvm_volume.name} already exists in db.."
+              @logger.error "volume #{lvm_volume.name} already exists in db.."
             end
           end
         end
@@ -646,7 +650,7 @@ class TaskOmatic
   end
 
   def task_create_volume(task)
-    puts "create_volume"
+    @logger.info "create_volume"
 
     db_volume = task.storage_volume
     raise "Could not find storage volume to create" unless db_volume
@@ -671,9 +675,9 @@ class TaskOmatic
           volume = @session.object(:object_id => volume_id)
           raise "Unable to find newly created volume" unless volume
 
-          puts "  volume:"
+          @logger.info "  volume:"
           for (key, val) in volume.properties
-            puts "    property: #{key}, #{val}"
+            @logger.info "    property: #{key}, #{val}"
           end
 
           # FIXME: Should have this too I think..
@@ -698,7 +702,7 @@ class TaskOmatic
   end
 
   def task_delete_volume(task)
-    puts "delete_volume"
+    @logger.info "delete_volume"
 
     db_volume = task.storage_volume
     raise "Could not find storage volume to create" unless db_volume
@@ -712,7 +716,7 @@ class TaskOmatic
       if db_volume[:type] == "LvmStorageVolume"
         phys_libvirt_pool = get_libvirt_lvm_pool_from_volume(db_volume)
         phys_libvirt_pool.connect(@session, node)
-        puts "connected to lvm pool.."
+        @logger.info "connected to lvm pool.."
       end
 
       begin
@@ -723,7 +727,7 @@ class TaskOmatic
           volume = @session.object(:class => 'volume',
                                    'storagePool' => libvirt_pool.remote_pool.object_id,
                                    'path' => db_volume.path)
-          puts "Unable to find volume to delete" unless volume
+          @logger.error "Unable to find volume to delete" unless volume
 
           # FIXME: we actually probably want to zero out the whole volume
           # here, so we aren't potentially leaking data from one user
@@ -771,18 +775,18 @@ class TaskOmatic
         tasks = Task.find(:all, :conditions =>
                           [ "state = ?", Task::STATE_QUEUED ])
       rescue => ex
-        puts "1 #{ex.class}: #{ex.message}"
+        @logger.error "1 #{ex.class}: #{ex.message}"
         if Task.connected?
           begin
             ActiveRecord::Base.connection.reconnect!
           rescue => norecon
-            puts "2 #{norecon.class}: #{norecon.message}"
+            @logger.error "2 #{norecon.class}: #{norecon.message}"
           end
         else
           begin
             database_connect
           rescue => ex
-            puts "3 #{ex.class}: #{ex.message}"
+            @logger.error "3 #{ex.class}: #{ex.message}"
           end
         end
       end
@@ -822,13 +826,13 @@ class TaskOmatic
               task_delete_volume(task)
             when HostTask::ACTION_CLEAR_VMS: task_clear_vms_host(task)
           else
-            puts "unknown task " + task.action
+            @logger.error "unknown task " + task.action
             state = Task::STATE_FAILED
             task.message = "Unknown task type"
           end
         rescue => ex
-          puts "Task action processing failed: #{ex.class}: #{ex.message}"
-          puts ex.backtrace
+          @logger.error "Task action processing failed: #{ex.class}: #{ex.message}"
+          @logger.error ex.backtrace
           state = Task::STATE_FAILED
           task.message = ex.message
         end
@@ -836,7 +840,7 @@ class TaskOmatic
         task.state = state
         task.time_ended = Time.now
         task.save!
-        puts "done"
+        @logger.info "done"
       end
       # FIXME: here, we clean up "orphaned" tasks.  These are tasks
       # that we had to orphan (set task_target to nil) because we were
-- 
1.6.0.6




More information about the ovirt-devel mailing list