On Fri, Feb 25, 2011 at 07:04:07PM +0100, Jiri Denemark wrote:
This is quite hacky since it involves falling back to HMP when savevm command is not found in QMP, which is something qemu monitor code was not designed to support. Hence, I'm providing 2 version of the first patch: 1.1/3 and 1.2/3.
- 1.1/3 version only touches JSON monitor code but involves copy&paste of the snapshot code from text monitor
- 1.2/3 touches more files but doesn't require duplicating the text monitor snapshot implementation into qemu_monitor_json.c. However, it results in somewhat funky call graphs:
-> qemuMonitorJSONCreateSnapshot -> qemuMonitorTextCreateSnapshot -> qemuMonitorCommand (a macro expanding to qemuMonitorCommandWithFd) -> qemuMonitorJSONHumanCommandWithFd -> qemuMonitorJSONCommandWithFd
The possibility to call qemuMonitorTextCreateSnapshot directly on JSON monitor is implemented by generalizing qemuMonitorCommandWithFd, which now either calls to qemuMonitorJSONHumanCommandWithFd or qemuMonitorTextCommandWithFd (former qemuMonitorCommandWithFd) depending on the monitor type.
This call path rather makes my head hurt but the lack of duplicated code is attractive.
I prefer version 2 since it reuses text monitor implementation, but other may prefer version 1, which is a bit more local...
I've suggested a possible 3rd alternative in my reply to v1 Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|