Vincent, Thanks for raising this on the reflector. This is a timely discussion and your concerns are noted. The following list summarizes the changes that had to be made: 1 - don't let ipmi stuff run on vms - one minor issue here in the rebase which I will back out (good catch by Vincent) 2 - don't allow admin install to run unbmc.sh on vms 3 - ensure admin install on h/w or vms works without throwing fatal errors w.r.t ipmi modules (this will happen only on redhat / centos 6.4 versions since the module is supported only on more recent kernels) 4 - ensure admin install is not broken and doesn't mess up ipmi stuff (on physical h/w) during ubuntu installs - it needs to detect that it's an ubuntu install and load the ipmi_si kernel module 5 - remove all checks in the code for an ipmi_si related file before running ipmi commands - changed to a newer module (ipmi_devintf) which is present on both OSs 6- validated that running ipmitool commands (which depends on these ipmi modules) is not broken in sledgehammer during hardware-installing phase (by changing bmc password and validating that we could reset it on C series systems)
Does that help you? - John T. -----Original Message----- From: crowbar-bounces On Behalf Of Vincent Untz Sent: Friday, September 27, 2013 4:35 AM To: crowbar Subject: Re: [Crowbar] [barclamp-ipmi] Cleanup of errors related to ipmi_si [1/2] (#56) Hi, The pull request got merged last week without review about SUSE support (without smoketesting, and again without much time between the pull request being opened and the merge). I don't know enough about IPMI, to know if it's right or not, but on the other there's not much explanation on the change either... I've commented on the pull request about some weird stuff in the patch. But more importantly, I'm concerned about the fact that this seems to care about what is right for Ubuntu and RHEL, but lacks any comment about SUSE support. As far as I can tell, the old code was working fine for SUSE. And now, I'm left wondering if we want to use this or not... Cheers, Vincent Le vendredi 20 septembre 2013, à 13:09 -0700, Raajeev a écrit : > Prevent running ipmi barclamp on VMs > Prevent running unbmc.sh on VMs during admin install Fix cosmetic > issues with modprobe of ipmi_si failures on centos/redhat > > > chef/cookbooks/ipmi/providers/bmc_command.rb | 2 +- > chef/cookbooks/ipmi/providers/lan_set.rb | 2 +- > chef/cookbooks/ipmi/providers/load.rb | 10 +--------- > chef/cookbooks/ipmi/providers/unload.rb | 10 +--------- > chef/cookbooks/ipmi/providers/user_set.rb | 5 +---- > chef/cookbooks/ipmi/recipes/ipmi-discover.rb | 20 ++++++++++++++------ > 6 files changed, 19 insertions(+), 30 deletions(-) > > Crowbar-Pull-ID: 6d14733eb8ebf8e528649f2c05512e135ae91f0b > > Crowbar-Release: pebbles > You can merge this Pull Request by running: > > git pull https://github.com/Raajeev/barclamp-ipmi > pull-req-release-pebbles-master-6d14733eb8ebf8e528649f2c05512e135ae91f > 0b > > Or you can view, comment on it, or merge it online at: > > https://github.com/crowbar/barclamp-ipmi/pull/56 > > -- Commit Summary -- > > * Need to remove the ipmi_si module dependency - CES 21 > * Need to remove the ipmi_si module dependency - CES 21 > > -- File Changes -- > > M chef/cookbooks/ipmi/providers/bmc_command.rb (2) > M chef/cookbooks/ipmi/providers/lan_set.rb (2) > M chef/cookbooks/ipmi/providers/load.rb (10) > M chef/cookbooks/ipmi/providers/unload.rb (10) > M chef/cookbooks/ipmi/providers/user_set.rb (5) > M chef/cookbooks/ipmi/recipes/ipmi-discover.rb (20) > > -- Patch Links -- > > https://github.com/crowbar/barclamp-ipmi/pull/56.patch > https://github.com/crowbar/barclamp-ipmi/pull/56.diff -- Les gens heureux ne sont pas pressés. _______________________________________________ Crowbar mailing list [email protected] https://lists.us.dell.com/mailman/listinfo/crowbar For more information: http://crowbar.github.com/
_______________________________________________ Crowbar mailing list [email protected] https://lists.us.dell.com/mailman/listinfo/crowbar For more information: http://crowbar.github.com/
