Add libvirt domain XML option

jrich523 · May 23, 2018, 6:39pm

We’re having an issue with PXE not getting the image fast enough (known issue apparently), causing some systems to hang (no boot device found typical bios msg)

What we found is that libvirt has an option to help force a reboot if that happens.
https://libvirt.org/formatdomain.html#elementsOS
OS/Bios has the RebootTimeout

We’ve verified this works as we expect with Virsh

I’ve tried to find a way to configure this some place but it doesnt seem like there is any ability to do that.

After digging and reading lots of code, i think maybe the easiest solution would be to fork the fog-libvirt and inject my settings there (hard coded or pulled from a file, but, just something simple)

Im sure it would be awesome to really fix this the right way, but with my ruby skill i doubt you’d accept a PR

Also working up the stack to expose it to the end user is likely a LOT more work than hijacking the fog-libvirt module.

So perhaps you’re wondering what the question is?!

Does this approach make sense (with the idea that speed/ease are key)
Did i over look anything that would allow us to solve this problem easily without having to modify someone else’s source.

The other method that came to mind was a plugin that hooked in to the provisioning process, but that seemed like it would be more effort with some higher odds of complication. I’ve dug enough that it seems possible but i dont know enough to judge the effort involved in this method.

Thanks,
Justin

Gwmngilfen · May 23, 2018, 7:24pm

I agree fog-libvirt is probably the best place - sadly we don’t maintain Fog, although we do have a couple of developers in common. I’m sure they’d help you to refine a PR though

For short term hacks, we do override Fog in a variety of places, such as here, but that kinda relies on there being the right Fog primitives in place, and it’s been ages since I messed with that.

For short-term hackery that would break on upgrade (and I’m certain I couldn’t recommend such a course of action), the XML that Fog uses will be on disk. On my Debian host it’s something like /usr/share/foreman/vendor/ruby/<ruby version>/gems/fog-libvirt-<gem version>/lib/fog/libvirt/requests/compute/mock_files/domain.xml although you’ll have to go digging. Hacking into that to match the XML in your virsh trial would at least confirm Foreman can generate the right config

jrich523 · May 23, 2018, 11:45pm

Oh man, I had looked for something like that but was unable to find any reference as to where that might be. In digging through the libvirt stuff it sounded like maybe there was a file some place i was just never able to find it. I’ll see if i can find that because in this case a default of “always have that option” was really what we were going for anyways and if we need to update that upon upgrade, im sure we can get Puppet to handle that mess

And as far as code, thats where i was looking

github.com

theforeman/foreman/blob/develop/app/models/compute_resources/foreman/model/libvirt.rb#L129-L148


def new_vm(attr = { })
  test_connection
  return unless errors.empty?
  opts = vm_instance_defaults.merge(attr.to_h).deep_symbolize_keys


  # convert rails nested_attributes into a plain hash
  [:nics, :volumes].each do |collection|
    nested_attrs = opts.delete("#{collection}_attributes".to_sym)
    opts[collection] = nested_attributes_for(collection, nested_attrs) if nested_attrs
  end


  opts.reject! { |k, v| v.nil? }


  opts[:boot_order] = %w[hd]
  opts[:boot_order].unshift 'network' unless attr[:image_id]


  vm = client.servers.new opts
  vm.memory = opts[:memory] if opts[:memory]
  vm
end

line 145,
vm = client.servers.new opts

I was under the impression that was the call to Fog’s services?

Which is why i figured that injecting the option in to the fog-libvirt module would just sort of magically work when used with Foreman. With how foreman is setup, I can just modify the gem setup in bundle.d to use our version of fog-libvirt (something else we can monitor with puppet to deal with upgrades)

The fog-libvirt module looks mostly like it screws with the xml and then hands that over, so just ball parking that i could edit this

github.com

fog/fog-libvirt/blob/master/lib/fog/libvirt/requests/compute/create_domain.rb#L6


module Fog
module Compute
  class Libvirt
    class Real
      def create_domain(xml)
        client.create_domain_xml(xml)
      end
    end


    class Mock
      def create_domain(xml)
        ::Libvirt::Domain.new()
      end
    end
  end
end

rather than return the XML directly, edit it to toss in the params I need and then hand it back. The idea being that even if there are upgrades the chances of that upgrade involving fog-libvirt are probably pretty minor, and also with how little code is really in it, it shouldnt be hard if we need to merge updates. It might need to be in two places, since im not actually sure what the difference is between “define_domain” and “create_domain” is, they both make the same call and basically do the same thing (probably just edit both)

However, thats still a horrible bandaid, and if we’re going to do that, its probably better that we just modify that template xml like you mentioned (provided we can find it)

lzap · May 24, 2018, 3:36pm

I think you might be more lucky at fog bugtracker, but for the record we want to solve the long-standing concurrency TFTP bug.

jrich523 · May 24, 2018, 4:33pm

@Gwmngilfen
Found it on centos here
/opt/theforeman/tfm/root/usr/share/gems/gems/fog-libvirt-0.4.1/lib/fog/libvirt/requests/compute/mock_files

I didnt realize, until i start to dig for the path, that its listed under the mock_files… which… seems a tad odd to store the production template in a mock folder?

Will run some tests in a bit to see if this cuts it for us. really appreciate the help/insight!

@lzap
Well, do you guys have any notes/directions/plans I could look at to see if its something i could attempt to do the right way? sort of doubtful I can help, but doesnt hurt to look/ask i guess

lzap · May 25, 2018, 6:25am

Currently there are no known workarounds.

jrich523 · May 25, 2018, 2:32pm

@lzap I guess i wasnt looking for a workaround to the TFTP problem (I have one, found the server.xml.erb in fog-libvirt and it works!)

Was more so curious if there was a purposed solution (correct one) that just needed to be implimented or if it was that its jsut a knoew problem that needs to be resolved and thats the extent of the information on it.

lzap · May 25, 2018, 7:32pm

The correct solution to this problem is to file PR into fog-libvirt with the flag (assuming it is not there yet I haven’t tried) and once merged, wait until they release a new version, then pull the version into our deps, we can make RPM/DEB packages and then you can file PR to use the new flag in Foreman in UI/API/CLI. This would be perhaps new fields bootmenu and timeout in the form.