Darryl L. Pierce
2009-Mar-31 15:26 UTC
[Ovirt-devel] [PATCH server] Fixed a few bugs in o-identify-node processing.
Fixed the processing of NICs to handle properly deleting a NIC that was
not identified by the node. All unidentified NICs are now deleted from
the database after o-i-node runs.
Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
---
src/host-browser/host-browser.rb | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/host-browser/host-browser.rb b/src/host-browser/host-browser.rb
index 1c5cf83..b40a73f 100755
--- a/src/host-browser/host-browser.rb
+++ b/src/host-browser/host-browser.rb
@@ -271,6 +271,7 @@ class HostBrowser
puts "Updating NIC records for the node"
nics = Array.new
+ nics_to_delete = Array.new
host.nics.collect do |nic|
found = false
@@ -278,7 +279,8 @@ class HostBrowser
nic_info.collect do |detail|
# if we have a match, then update the database and remove
# the received data to avoid creating a dupe later
- if detail['MAC'] == nic.mac
+ if detail['MAC'].upcase == nic.mac
+ puts "Updating details for: #{nic.interface_name}
[#{nic.mac}]}"
nic_info.delete(detail)
updated_nic = Nic.find_by_id(nic.id)
@@ -286,7 +288,6 @@ class HostBrowser
updated_nic.bandwidth = detail['BANDWIDTH'].to_i
updated_nic.interface_name = detail['IFACE_NAME']
- updated_nic.save!
found=true
nic_info.delete detail
end
@@ -294,14 +295,16 @@ class HostBrowser
# if the record wasn't found, then remove it from the database
unless found
- host.nics.delete(nic)
- nic.destroy
+ puts "Marking NIC for removal: #{nic.interface_name}
[#{nic.mac}]"
+ nics_to_delete << nic
end
end
+ nics_to_delete.each { |nic| puts "Removing NIC:
#{nic.interface_name} []#{nic.mac}]"; host.nics.delete(nic) }
+
# iterate over any nics left and create new records for them.
nic_info.collect do |nic|
- puts "Creating a new nic..."
+ puts "Creating a new nic: #{nic.interface_name}
[#{nic.mac}]"
detail = Nic.new(
'mac' => nic['MAC'].upcase,
'bandwidth' => nic['BANDWIDTH'].to_i,
--
1.6.0.6
Mohammed Morsi
2009-Mar-31 22:43 UTC
[Ovirt-devel] [PATCH server] Fixed a few bugs in o-identify-node processing.
Applied this patch and haven't run it yet, but noticed a few things Darryl L. Pierce wrote:> Fixed the processing of NICs to handle properly deleting a NIC that was > not identified by the node. All unidentified NICs are now deleted from > the database after o-i-node runs. > > Signed-off-by: Darryl L. Pierce <dpierce at redhat.com> > --- > src/host-browser/host-browser.rb | 13 ++++++++----- > 1 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/src/host-browser/host-browser.rb b/src/host-browser/host-browser.rb > index 1c5cf83..b40a73f 100755 > --- a/src/host-browser/host-browser.rb > +++ b/src/host-browser/host-browser.rb > @@ -271,6 +271,7 @@ class HostBrowser > > puts "Updating NIC records for the node" > nics = Array.new > + nics_to_delete = Array.new > > host.nics.collect do |nic| >This 'collect' should more optimally be 'each' as we're not using the array returned by collect.> found = false > @@ -278,7 +279,8 @@ class HostBrowser > nic_info.collect do |detail| >Same with this collect here> # if we have a match, then update the database and remove > # the received data to avoid creating a dupe later > - if detail['MAC'] == nic.mac > + if detail['MAC'].upcase == nic.mac > + puts "Updating details for: #{nic.interface_name} [#{nic.mac}]}" > nic_info.delete(detail) >The 'detail' nic is being removed from the nic_info array here, and at the end of this if block as well (see below)> > updated_nic = Nic.find_by_id(nic.id) > @@ -286,7 +288,6 @@ class HostBrowser > updated_nic.bandwidth = detail['BANDWIDTH'].to_i > updated_nic.interface_name = detail['IFACE_NAME'] > > - updated_nic.save! >Will this work if you remove this? If I understand what your doing by reading this, you are relying on the host.save! a little further down to save the updated nics, but the manipulation of the nic object (eg updating bandwidth and interface name) occurs on a nic object you explicitly retrieve via find_by_id here. Unless I'm mistaken you will either need this updated_nic.save! or instead of updating the nic via the "updated_nic" object, you can simply use the "nic" object (eg the iterator as set via the host.nics.collect{ |nic| above).> found=true > nic_info.delete detail >Here is the other place the 'detail' nic is being removed from nic_info> end > @@ -294,14 +295,16 @@ class HostBrowser > > # if the record wasn't found, then remove it from the database > unless found > - host.nics.delete(nic) > - nic.destroy > + puts "Marking NIC for removal: #{nic.interface_name} [#{nic.mac}]" > + nics_to_delete << nic > end > end > > + nics_to_delete.each { |nic| puts "Removing NIC: #{nic.interface_name} []#{nic.mac}]"; host.nics.delete(nic) } > + > # iterate over any nics left and create new records for them. > nic_info.collect do |nic| > - puts "Creating a new nic..." > + puts "Creating a new nic: #{nic.interface_name} [#{nic.mac}]" > detail = Nic.new( > 'mac' => nic['MAC'].upcase, > 'bandwidth' => nic['BANDWIDTH'].to_i, >-Mo
Darryl L. Pierce
2009-Apr-01 12:32 UTC
[Ovirt-devel] [PATCH server] Fixed a few bugs in o-identify-node processing.
On Tue, Mar 31, 2009 at 06:43:30PM -0400, Mohammed Morsi wrote:> This 'collect' should more optimally be 'each' as we're not using the > array returned by collect.Fixed those.> > - if detail['MAC'] == nic.mac > > + if detail['MAC'].upcase == nic.mac > > + puts "Updating details for: #{nic.interface_name} [#{nic.mac}]}" > > nic_info.delete(detail) > > > The 'detail' nic is being removed from the nic_info array here, and at > the end of this if block as well (see below)D'oh! Fixed that.> > > > updated_nic = Nic.find_by_id(nic.id) > > @@ -286,7 +288,6 @@ class HostBrowser > > updated_nic.bandwidth = detail['BANDWIDTH'].to_i > > updated_nic.interface_name = detail['IFACE_NAME'] > > > > - updated_nic.save! > > > Will this work if you remove this? If I understand what your doing by > reading this, you are relying on the host.save! a little further down to > save the updated nics, but the manipulation of the nic object (eg > updating bandwidth and interface name) occurs on a nic object you > explicitly retrieve via find_by_id here. Unless I'm mistaken you will > either need this updated_nic.save! or instead of updating the nic via > the "updated_nic" object, you can simply use the "nic" object (eg the > iterator as set via the host.nics.collect{ |nic| above).This was some old code. I refactored that whole chunk so that it would use the nic object it already had. Now it'll update that and save it inline. Updated patch is in-flight for this. -- Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc. Virtual Machine Management - http://www.ovirt.org/ Is fearr Gaeilge bhriste n? B?arla cliste. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 197 bytes Desc: not available URL: <http://listman.redhat.com/archives/ovirt-devel/attachments/20090401/30f0a9b4/attachment.sig>
Darryl L. Pierce
2009-Apr-01 12:34 UTC
[Ovirt-devel] [PATCH server] Fixed a few bugs in o-identify-node processing.
NOTE: This version includes fixes based on feedback from mmorsi and some
refactorings to the way nics are updated.
Fixed the processing of NICs to handle properly deleting a NIC that was
not identified by the node. All unidentified NICs are now deleted from
the database after o-i-node runs.
Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
---
src/host-browser/host-browser.rb | 36 ++++++++++++++++++------------------
1 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/host-browser/host-browser.rb b/src/host-browser/host-browser.rb
index 1c5cf83..b927d3c 100755
--- a/src/host-browser/host-browser.rb
+++ b/src/host-browser/host-browser.rb
@@ -248,7 +248,7 @@ class HostBrowser
Cpu.delete_all(['host_id = ?', host.id])
puts "Saving new CPU records"
- cpu_info.collect do |cpu|
+ cpu_info.each do |cpu|
detail = Cpu.new(
"cpu_number" => cpu['CPUNUM'].to_i,
"core_number" => cpu['CORENUM]'].to_i,
@@ -271,37 +271,37 @@ class HostBrowser
puts "Updating NIC records for the node"
nics = Array.new
+ nics_to_delete = Array.new
- host.nics.collect do |nic|
+ host.nics.each do |nic|
found = false
- nic_info.collect do |detail|
+ nic_info.each do |detail|
# if we have a match, then update the database and remove
# the received data to avoid creating a dupe later
- if detail['MAC'] == nic.mac
- nic_info.delete(detail)
-
- updated_nic = Nic.find_by_id(nic.id)
-
- updated_nic.bandwidth = detail['BANDWIDTH'].to_i
- updated_nic.interface_name = detail['IFACE_NAME']
-
- updated_nic.save!
- found=true
- nic_info.delete detail
+ puts "Searching for existing record for:
#{detail['MAC'].upcase}"
+ if detail['MAC'].upcase == nic.mac
+ puts "Updating details for:
#{detail['IFACE_NAME']} [#{nic.mac}]}"
+ nic.bandwidth = detail['BANDWIDTH'].to_i
+ nic.interface_name = detail['IFACE_NAME']
+ nic.save!
+ found = true
+ nic_info.delete(detail)
end
end
# if the record wasn't found, then remove it from the database
unless found
- host.nics.delete(nic)
- nic.destroy
+ puts "Marking NIC for removal: #{nic.interface_name}
[#{nic.mac}]"
+ nics_to_delete << nic
end
end
+ nics_to_delete.each { |nic| puts "Removing NIC:
#{nic.interface_name} []#{nic.mac}]"; host.nics.delete(nic) }
+
# iterate over any nics left and create new records for them.
- nic_info.collect do |nic|
- puts "Creating a new nic..."
+ nic_info.each do |nic|
+ puts "Creating a new nic: #{nic.interface_name}
[#{nic.mac}]"
detail = Nic.new(
'mac' => nic['MAC'].upcase,
'bandwidth' => nic['BANDWIDTH'].to_i,
--
1.6.0.6