On Thu, Feb 13, 2014 at 11:08:50AM +0800, Qiao Nuohan wrote:

On 01/29/2014 10:04 AM, qiaonuohan wrote:

...

--memory-only option is introduced without compression supported. Therefore, this is a freature regression of virsh dump. This patchset is used to add compression support in libvirt side and please refer the following address to see the qemu side, the lastest version of qemu side v7(ready for comment now).

http://lists.gnu.org/archive/html/qemu-devel/2014-01/msg03669.html

qiaonuohan (2): support compression when --memory-only option is specified support configuring the format of dumping memory in qemu.conf

include/libvirt/libvirt.h.in | 18 +++++++++---- src/libvirt.c | 15 +++++++++++ src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 6 +++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 52 +++++++++++++++++++++++++++++++++++--- src/qemu/qemu_monitor.c | 6 ++--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 ++- src/qemu/qemu_monitor_json.h | 3 ++- src/qemu/test_libvirtd_qemu.aug.in | 1 + tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 29 +++++++++++++++++++++ 14 files changed, 127 insertions(+), 16 deletions(-)

Hello

Do you have some comments on my patches?

Hi. Having an option to set the compression type is good to have, but I'd go with a different approach. I don't know if by the "regression" in 1/2 you mean that if someone specifies a compression it is not used with '--memory-only' then yes, but that's the only one I see. And you are mixing two different compressions together, one what qemu will use and one that libvirt uses. Can we first address just the fact that we ignore the dump image compression (which I believe should be used for memory-only dumps too) and make it consistent and then (possibly) deal with selecting the compression and the fact who will do the compression? You are adding too many flags which seem to overlap or intersect with each other. How about adding a flag "nocompress" that would disable any compression set in qemu.conf in current API as a second patch and then work your way to the result? One question is whether specifying the format or compression is useful on every call. Also, are these "qemu-compressions" available only on the dump-guest-memory call? I guess yes as it can't be very well used for migrations, there's only xbzrle IIRC. If we just add an option to select the default format qemu should use for memory dumps it should be enough. It's probably not expressed, so let me re-capitulate what I'd suggest: 1) Do a patch that properly propagates the settings from qemu.conf into the dump call even when memory-only is selected. This makes it possible to compress memory-only dumps even with old qemu. 2) (optional) Add a NOCOMPRESS flag that disables any compression set in qemu.conf for current API call that works for both normal and memory-only dump. 3) Add a configuration option (basically what you did) to select the default format qemu will use. Have a nice day, Martin