[Issue 276] QEMU Agent configuration settings #287
Conversation
mpywell
changed the title
- Extend QEMU agent handling by introducing qemu_quest_agent block, deprecating qemu_agent bool - Enable setting the agent type - Enable toggling freeze/thaw filesystems on backup - Enable toggling fstrim after disk move or VM migration - Each added setting defaults to Proxmox's backend defaults when not configured.
mpywell
force-pushed
the
feat/276_agent_settings
branch
from
b3848f4 to
0b738ef
Compare
There was a problem hiding this comment.
Thanks for this one @mpywell!
Overall LGTM, I left a couple of suggestions, feel free to address them and I'll come back for another round of review after that.
builder/proxmox/common/config.go
Outdated
| // Sets the Agent Type. Must be `isa` or `virtio`. Defaults to `virtio` | ||
| Type string `mapstructure:"type"` | ||
| // Enable freeze/thaw of guest filesystem on backup. Defaults to `true` | ||
| Freeze config.Trilean `mapstructure:"freeze"` |
There was a problem hiding this comment.
If this is an option we're introducing here, and the default is true, in general I would suggest maybe inverting the option to something like disable_freeze, so it's clear that we freeze by default, and only don't do it if requested.
There was a problem hiding this comment.
Makes sense, have rerolled with disable_freeze as a bool
| // Default qemu_agent to true | ||
| if c.Agent != config.TriFalse { | ||
| c.Agent = config.TriTrue | ||
| warnings = append(warnings, "qemu_agent is deprecated and will be removed in a future release. define QEMU agent settings in a qemu_guest_agent block instead") |
There was a problem hiding this comment.
I would suggest we only print that warning if qemu_agent is unset here, not if it's not explicitly false, otherwise we'll see it all the time unless we add qemu_agent = false to the template.
There was a problem hiding this comment.
It occurred to me that we don't need to set a default value for c.Agent given the replacement c.GuestAgent.Enabled has a default set further down, so I've changed the logic to warn and convert the value only if the c.Agent is not Unset.
- fix qemu_agent deprecation warnings and conversion - invert freeze config option for qemu_guest_agent - simplify test logic for TestBasicExampleFromDocsIsValid
|
Hi @lbajolet-hashicorp have rerolled this one and added comments in the review |
|
This would be really nice to have merged :) |
3 participants
Extend QEMU agent handling by introducing the
qemu_quest_agentblockqemu_guest_agentadds the ability to set each of the QEMU Guest Agent's available settings:qemu_agentbool)Each added setting defaults to Proxmox's backend defaults when not configured.
Closes #276