[libvirt] [PATCH] qemu: command: don't overwrite watchdog dump action
The watchdog cli refactoring in 4666b762 dropped the temporary variable we use to convert to action=dump to action=pause for the qemu cli, and stored the converted value in the domain structure. Our other watchdog handling code then treated it as though the user requested action=pause, which broke action=dump handling. Revive the temporary variable to fix things. --- src/qemu/qemu_command.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 21f505b..434c731 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3350,6 +3350,7 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd, virDomainWatchdogDefPtr watchdog = def->watchdog; char *optstr; const char *action; + int action_id; if (!def->watchdog) return 0; @@ -3376,10 +3377,12 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, optstr); VIR_FREE(optstr); + /* 'dump' action tells qemu to pause, then we handle the dump ourselves */ + action_id = watchdog->action; if (watchdog->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) - watchdog->action = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE; + action_id = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE; - action = virDomainWatchdogActionTypeToString(watchdog->action); + action = virDomainWatchdogActionTypeToString(action_id); if (!action) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid watchdog action")); -- 2.7.3
On Wed, 2016-04-13 at 11:33 -0400, Cole Robinson wrote:
The watchdog cli refactoring in 4666b762 dropped the temporary variable we use to convert to action=dump to action=pause for the qemu cli, and stored the converted value in the domain structure. Our other watchdog handling code then treated it as though the user requested action=pause, which broke action=dump handling.
Revive the temporary variable to fix things. --- src/qemu/qemu_command.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 21f505b..434c731 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3350,6 +3350,7 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd, virDomainWatchdogDefPtr watchdog = def->watchdog; char *optstr; const char *action; + int action_id;
Maybe use something like 'actualAction'...
if (!def->watchdog) return 0; @@ -3376,10 +3377,12 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, optstr); VIR_FREE(optstr); + /* 'dump' action tells qemu to pause, then we handle the dump ourselves */
... and be a bit more verbose in the comment, so we don't risk it being deleted again?
+ action_id = watchdog->action; if (watchdog->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) - watchdog->action = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE; + action_id = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE; - action = virDomainWatchdogActionTypeToString(watchdog->action); + action = virDomainWatchdogActionTypeToString(action_id); if (!action) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid watchdog action"));
ACK in any case. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
On 04/14/2016 11:58 AM, Andrea Bolognani wrote:
On Wed, 2016-04-13 at 11:33 -0400, Cole Robinson wrote:
The watchdog cli refactoring in 4666b762 dropped the temporary variable we use to convert to action=dump to action=pause for the qemu cli, and stored the converted value in the domain structure. Our other watchdog handling code then treated it as though the user requested action=pause, which broke action=dump handling.
Revive the temporary variable to fix things. --- src/qemu/qemu_command.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 21f505b..434c731 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3350,6 +3350,7 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd, virDomainWatchdogDefPtr watchdog = def->watchdog; char *optstr; const char *action; + int action_id;
Maybe use something like 'actualAction'...
if (!def->watchdog) return 0; @@ -3376,10 +3377,12 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, optstr); VIR_FREE(optstr);
+ /* 'dump' action tells qemu to pause, then we handle the dump ourselves */
... and be a bit more verbose in the comment, so we don't risk it being deleted again?
+ action_id = watchdog->action; if (watchdog->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) - watchdog->action = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE; + action_id = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE;
- action = virDomainWatchdogActionTypeToString(watchdog->action); + action = virDomainWatchdogActionTypeToString(action_id); if (!action) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid watchdog action"));
Made the actualAction change, and changed the comment to /* qemu doesn't have a 'dump' action; we tell qemu to 'pause', then libvirt listens for the watchdog event, and we perform the dump ourselves. so convert 'dump' to 'pause' for the qemu cli */ Thanks, Cole
participants (2)
-
Andrea Bolognani -
Cole Robinson