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

Misc virt fixes #50897

Merged
merged 3 commits into from
Dec 18, 2018
Merged

Misc virt fixes #50897

merged 3 commits into from
Dec 18, 2018

Conversation

cbosdo
Copy link
Contributor

@cbosdo cbosdo commented Dec 18, 2018

What does this PR do?

Fixes two unrelated bugs:

  • libvirt-events engine didn't map the defined/updated domain lifecycle events properly (only with the real libvirt, our unit test had a flaw)
  • set/update the current memory as well as the max one on VMs.

What issues does this PR fix or reference?

None

Tests written?

Yes

Commits signed with GPG?

Yes

Libvirt events algorigthm converting the libvirt enums into string has a
flaw that unit tests couldn't see. Libvirt python binding defines the
following constants:

VIR_DOMAIN_EVENT_CRASHED_PANICKED = 0
VIR_DOMAIN_EVENT_DEFINED = 0
VIR_DOMAIN_EVENT_CRASHED = 8

However VIR_DOMAIN_EVENT_CRASHED_PANICKED is the only value in this enum
and thus wasn't not considered a sub-enum element. So the value 0 in
enum 'VIR_DOMAIN_EVENT_' was wrongly mapped to "crashed panicked"
instead of "defined".

In order to safely rule this case out, check if we have an item that
just ends with the subprefix without the '_'.
@cbosdo
Copy link
Contributor Author

cbosdo commented Dec 18, 2018

@rallytime this PR is ready, just fixes two minor bugs.

Setting the <memory> element in the libvirt definition just changes the
maximum amount of memory. Also set the <currentMemory> amount to the
same value.
Fix unused variable in _domain_event_graphics_cb.
@garethgreenaway garethgreenaway merged commit f2b9e7f into saltstack:fluorine Dec 18, 2018
@cbosdo cbosdo deleted the misc-virt-fixes branch March 13, 2020 15:25
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.

None yet

2 participants