search

fog / fog-libvirt

libvirt provider for fog
MIT License
16 stars 42 forks source link

Undefine NVRAM not working if options without flags are passed to destroy #142

Open rwf14f opened 2 weeks ago

rwf14f commented 2 weeks ago

The code introduced with https://github.com/fog/fog-libvirt/pull/132 doesn't work when an options hash is passed to destroy that doesn't define :flags. For example, if { :destroy_volumes => true } is passed to destroy, then the if condition is false because it uses the default value of the options.fetch and then passes an undefined options[:flags] to the vm_action call in the else block. If the intention behind the patch is to use UNDEFINE_NVRAM as the default if no flags are passed to destroy then this should work:

flags = options.fetch(:flags, ::Libvirt::Domain::UNDEFINE_NVRAM)
if flags.zero?
  service.vm_action(uuid, :undefine)
else
  service.vm_action(uuid, :undefine, flags)
end
ekohl commented 2 weeks ago

I honestly would have preferred to make the method definition:

def destroy(destroy_volumes: false, flags: ::Libvirt::Domain::UNDEFINE_NVRAM)

But I don't know if I'd break something with that.

I think your suggestion is indeed fixing the problem I wanted to address. Could you submit a PR?