stackhpc / ansible-role-libvirt-vm

This role configures and creates VMs on a KVM hypervisor.
128 stars 67 forks source link

Always undefine a VM before defining it #14

Closed jovial closed 4 years ago

jovial commented 5 years ago

It seems that the virt module does not take into account changes to the templated vm.xml.j2 file. As such, changes to variables used within this template are not propagated to libvirt i.e the output of virsh dumpxml remains the same before and after the application of define.

jovial commented 5 years ago

Good point, I believe this will delete the VM. it would probably be better to replicate the behaviour of virsh edit, from the man page:

edit domain
Edit the XML configuration file for a domain, which will affect the next boot of the guest.
This is equivalent to:

virsh dumpxml --inactive --security-info domain > domain.xml
vi domain.xml (or make changes with your other text editor)
virsh define domain.xml
except that it does some error checking.
The editor used can be supplied by the $VISUAL or $EDITOR environment variables, and defaults to "vi".

It is tricky to detect whether or not it will change though as the XML passed into define often differs from the output of dumpxml immediately after. Ideally this would be an option on the virt module, otherwise we would have to use virsh define via the command module.

markgoddard commented 5 years ago

Good point, I believe this will delete the VM. it would probably be better to replicate the behaviour of virsh edit, from the man page:

edit domain
Edit the XML configuration file for a domain, which will affect the next boot of the guest.
This is equivalent to:

virsh dumpxml --inactive --security-info domain > domain.xml
vi domain.xml (or make changes with your other text editor)
virsh define domain.xml
except that it does some error checking.
The editor used can be supplied by the $VISUAL or $EDITOR environment variables, and defaults to "vi".

It is tricky to detect whether or not it will change though as the XML passed into define often differs from the output of dumpxml immediately after. Ideally this would be an option on the virt module, otherwise we would have to use virsh define via the command module.

Looks like undefine doesn't delete the active instance, but I don't know whether a subsequent define would be associated with the existing transient instance.

https://stackoverflow.com/questions/51098622/whats-the-difference-between-virsh-destroy-and-virsh-undefine

JPvRiel commented 5 years ago

I'd argue upstream virt module should be patched to support this and the role shouldn't wrap the behaviour in a different way.

As I commented on a PR that tangentially touched this issue:

The only way I foresee the upstream virt module managing to gracefully handle this idempotency problem is to run a careful sub-match of XML sub-elements of the current define given vs what the API returns, ignore the irrelevant XML from runtime, and then use the equivalent mechanism behind virsh edit to change VM settings only when something intentional has changed.

[virt] Overhaul and complete the virt_ modules #27905 raises doubts about the quality of the virt* modules. ovirt seems to maturing faster, but this is an extra layer of things over libvirt and probably too many dependencies we wouldn't want dragged in.

torvitas commented 5 years ago

If you undefine a running machine you will end up having a transient domain which will just disappear on next shutdown. And most importantly the configuration still wont change if u redefine it.

jovial commented 4 years ago

Agreed, this seems a bad idea and needs to be solved in a different way.