| Message ID | 1601606494-1154-1-git-send-email-alejandro.j.jimenez@oracle.com |
|---|---|
| Headers | show |
| Series | Do not stop guest when panic event is received | expand |
On 02/10/20 04:41, Alejandro Jimenez wrote: > The fact that the behavior of hv-crash is also affected is why I chose to implement this change as an independent > option, as opposed to making it a property of the pvpanic device (e.g. -device pvpanic,no-panicstop). > > Please let me know if you have any comments or suggestions. Hi Alejandro, sorry for the delayed response. The concept is fine, and I agree this should not be a device property. On the other hand, we already have many similar options: -no-reboot, -no-shutdown, -watchdog-action and now --no-panicstop. I think it's time to group them into a single option: * -action reboot=pause|shutdown|none * -action shutdown=pause|poweroff|none * -action panic=pause|shutdown|none * -action watchdog=reset|shutdown|poweroff|pause|debug|none|inject-nmi where the existing options would translate to the new option, like: * -no-reboot "-action reboot=shutdown" * -no-shutdown "-action shutdown=pause" The implementation should be relatively easy too; there's already an enum WatchdogAction (that can be renamed to e.g. RunstateAction) and a parsing function select_watchdog_action that can be changed to just return the RunstateAction. Would you like to take a look at this? Thanks, Paolo
On 10/20/2020 1:14 PM, Paolo Bonzini wrote: > On 02/10/20 04:41, Alejandro Jimenez wrote: >> The fact that the behavior of hv-crash is also affected is why I chose to implement this change as an independent >> option, as opposed to making it a property of the pvpanic device (e.g. -device pvpanic,no-panicstop). >> >> Please let me know if you have any comments or suggestions. > Hi Alejandro, sorry for the delayed response. > > The concept is fine, and I agree this should not be a device property. > > On the other hand, we already have many similar options: -no-reboot, > -no-shutdown, -watchdog-action and now --no-panicstop. > > I think it's time to group them into a single option: > > * -action reboot=pause|shutdown|none > * -action shutdown=pause|poweroff|none > * -action panic=pause|shutdown|none > * -action watchdog=reset|shutdown|poweroff|pause|debug|none|inject-nmi > > where the existing options would translate to the new option, like: > > * -no-reboot "-action reboot=shutdown" > * -no-shutdown "-action shutdown=pause" > > The implementation should be relatively easy too; there's already an > enum WatchdogAction (that can be renamed to e.g. RunstateAction) and a > parsing function select_watchdog_action that can be changed to just > return the RunstateAction. > > Would you like to take a look at this? Hi Paolo, Thank you for your reply and the advice/hints above. I'll take a look and try to implement what you propose. Regards, Alejandro > > Thanks, > > Paolo >
On 21/10/20 15:26, Alejandro Jimenez wrote: > > > On 10/20/2020 1:14 PM, Paolo Bonzini wrote: >> On 02/10/20 04:41, Alejandro Jimenez wrote: >>> The fact that the behavior of hv-crash is also affected is why I >>> chose to implement this change as an independent >>> option, as opposed to making it a property of the pvpanic device >>> (e.g. -device pvpanic,no-panicstop). >>> >>> Please let me know if you have any comments or suggestions. >> Hi Alejandro, sorry for the delayed response. >> >> The concept is fine, and I agree this should not be a device property. >> >> On the other hand, we already have many similar options: -no-reboot, >> -no-shutdown, -watchdog-action and now --no-panicstop. >> >> I think it's time to group them into a single option: >> >> * -action reboot=pause|shutdown|none >> * -action shutdown=pause|poweroff|none >> * -action panic=pause|shutdown|none >> * -action watchdog=reset|shutdown|poweroff|pause|debug|none|inject-nmi >> >> where the existing options would translate to the new option, like: >> >> * -no-reboot "-action reboot=shutdown" >> * -no-shutdown "-action shutdown=pause" >> >> The implementation should be relatively easy too; there's already an >> enum WatchdogAction (that can be renamed to e.g. RunstateAction) and a >> parsing function select_watchdog_action that can be changed to just >> return the RunstateAction. >> >> Would you like to take a look at this? > Hi Paolo, > > Thank you for your reply and the advice/hints above. I'll take a look > and try to implement what you propose. Just one thing, for the parsing you can place it close to the existing qemu_opts_foreach(qemu_find_opts("name"), parse_name, NULL, &error_fatal); Paolo