Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update existing VMs in virt.running #48736

Merged
merged 6 commits into from
Jul 27, 2018
Merged

Conversation

cbosdo
Copy link
Contributor

@cbosdo cbosdo commented Jul 24, 2018

What does this PR do?

This PR makes virt.running able to update existing VMs, running or not. If the VM is already running, the new definition will be applied for the next start and as many live updates as possible will be attempted.

A new virt.update function has been introduced in the virt execution module.

The PR also removes the use of minidom in the virt execution module for more consistency... and ElementTree helps removing some XML helper functions.

What issues does this PR fix or reference?

None

Previous Behavior

  • virt.running on a running VM did nothing
  • virt.running on a defined, but stopped VM started it

New Behavior

  • virt.running on a running VM tries hard to update it without restarting it
  • virt.running on a stopped VM changes the definition and starts it.

Tests written?

Yes

Commits signed with GPG?

Yes

@cbosdo
Copy link
Contributor Author

cbosdo commented Jul 24, 2018

@rallytime @gtmanfred yet another feature (+ cleanup) PR for the virt modules that you surely want to look into.

Copy link
Contributor

@gtmanfred gtmanfred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbosdo
Copy link
Contributor Author

cbosdo commented Jul 25, 2018

@gtmanfred oops... Two of the failures are indeed mine and I'll fix them, but the 3 others ones are completely unrelated to this PR.

@cbosdo
Copy link
Contributor Author

cbosdo commented Jul 25, 2018

The two unit tests are now fixed, thanks for the heads up

@rallytime rallytime requested a review from gtmanfred July 26, 2018 20:48
@cbosdo
Copy link
Contributor Author

cbosdo commented Jul 27, 2018

fixed a remaining lint error and rebased on the develop branch

The target element is not mandatory, introduce a test that we properly
handle this situation
ElementTree provides a more convenient API than minidom. Switching to
ElementTree will reduce the number of dependencies for the virt module
and simplify the code a little.
Graphics type 'none' will help to distinguish between no graphics data
and removal of graphics device in a future virt.update function.
User need to be able to update an existing virtual machine definition.
This function changes the definition for the next start of the VM and
tries hard to live update the virtual machine.
So far virt.running does nothing if the corresponding domain is already
defined. Use the new virt.update function to change the domain
configuration.
Modules test files are named after the module itself, the state virt
module needs to comply with that rule too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants