On Thu, 21 Oct 2010 10:35:10 +0200 Daniel Veillard <veillard@xxxxxxxxxx> wrote: > On Thu, Oct 21, 2010 at 12:02:11PM +0900, KAMEZAWA Hiroyuki wrote: > > > > Now, virsh dump doesn't support compresses dump. > > This patch adds GZIP and LZOP option to virsh dump and support > > it at qemu coredump. (AFAIK, LZOP is available on RHEL6.) > > > > When I did 4G guest dump, > > (Raw) 3844669750 > > (Gzip) 1029846577 > > (LZOP) 1416263880 (faster than gzip in general) > > > > This will be a help for a host where crash-dump is used > > and several guests works on it. > > > > help message is modified as this. > > NAME > > dump - dump the core of a domain to a file for analysis > > > > SYNOPSIS > > dump [--live] [--crash] [--gzip] [--lzop] <domain> <file> > > > > DESCRIPTION > > Core dump a domain. > > > > OPTIONS > > --live perform a live core dump if supported > > --crash crash the domain after core dump > > --gzip gzip dump(only one compression allowed > > --lzop lzop dump(only one compression allowed > > [--domain] <string> domain name, id or uuid > > [--file] <string> where to dump the core > > > > Tested on Fedora-13+x86-64. > > > > Note: for better compression, we may have to skip pages filled by > > zero or freed pages. But it seems it's qemu's works. > Thank you for review. > The patch looks relatively simple and clean, I still have one comemnt > below though. It also seems to me that any use of the dump need a follow > up decompression before it can be fed to debug tools (gdb ...) as I > don't think they handle compressed dumps natively. > Yes. But at support-job, coredump must be transfered to support-team enviroment in many case. In that case, dump image tend to be compressed. > The other comment on principle about this patch is that for saving > compression we use a qemu.conf option, maybe we should instead use > a new option there for core compression, or simply reuse the same > option for both (core being just a special kind of save). You will > also note that save_image_format takes more options than lzop or gzip. > If doing so one doesn't need to change virsh too. > Ah, okay, this one ? http://www.mail-archive.com/libvir-list@xxxxxxxxxx/msg15564.html Doesn't this affect behaviors other than dump ? > It's a trade off. If using the configuration option you need to plan > in advance the fact you will be dumping compressed core, if you expose > it at the API level itself, you can activate it anytime, and that may be > more useful for example when interacting with a customer facing an > issue needing debug. So both ways are acceptable to me. > Sure. > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> > > --- > > include/libvirt/libvirt.h.in | 2 ++ > > src/qemu/qemu_driver.c | 23 +++++++++++++++++++---- > > tools/virsh.c | 10 +++++++++- > > 3 files changed, 30 insertions(+), 5 deletions(-) > > > > Index: libvirt-0.8.4/src/qemu/qemu_driver.c > > =================================================================== > > --- libvirt-0.8.4.orig/src/qemu/qemu_driver.c > > +++ libvirt-0.8.4/src/qemu/qemu_driver.c > > @@ -5710,7 +5710,7 @@ cleanup: > > > > static int qemudDomainCoreDump(virDomainPtr dom, > > const char *path, > > - int flags ATTRIBUTE_UNUSED) { > > + int flags) { > > struct qemud_driver *driver = dom->conn->privateData; > > virDomainObjPtr vm; > > int resume = 0, paused = 0; > > @@ -5720,6 +5720,14 @@ static int qemudDomainCoreDump(virDomain > > "cat", > > NULL, > > }; > > + const char *zargs[] = { > > + "gzip", > > + NULL, > > + }; > > + const char *lzargs[] = { > > + "lzop", > > + NULL, > > + }; > > qemuDomainObjPrivatePtr priv; > > > > qemuDriverLock(driver); > > @@ -5787,9 +5795,16 @@ static int qemudDomainCoreDump(virDomain > > } > > > > qemuDomainObjEnterMonitorWithDriver(driver, vm); > > - ret = qemuMonitorMigrateToFile(priv->mon, > > - QEMU_MONITOR_MIGRATE_BACKGROUND, > > - args, path, 0); > > + if (flags & VIR_DUMP_GZIP) > > + ret = qemuMonitorMigrateToFile(priv->mon, > > + QEMU_MONITOR_MIGRATE_BACKGROUND, zargs, path, 0); > > + else if (flags & VIR_DUMP_LZOP) > > + ret = qemuMonitorMigrateToFile(priv->mon, > > + QEMU_MONITOR_MIGRATE_BACKGROUND, lzargs, path, 0); > > + else > > + ret = qemuMonitorMigrateToFile(priv->mon, > > + QEMU_MONITOR_MIGRATE_BACKGROUND, args, path, 0); > > + > > I'm wondering what happens if the compression command is not present, > if the current error message we get back from qemu is clear enough then > fine, but otherwise we may have to check for the presence of the > compressor program. > Ok, I will add check code. But hmm, calling system() for checking command exists is ok ? Or some good function do we have ? > > qemuDomainObjExitMonitorWithDriver(driver, vm); > > if (ret < 0) > > goto endjob; > > Index: libvirt-0.8.4/tools/virsh.c > > =================================================================== > > --- libvirt-0.8.4.orig/tools/virsh.c > > +++ libvirt-0.8.4/tools/virsh.c > > @@ -1751,6 +1751,8 @@ static const vshCmdInfo info_dump[] = { > > static const vshCmdOptDef opts_dump[] = { > > {"live", VSH_OT_BOOL, 0, N_("perform a live core dump if supported")}, > > {"crash", VSH_OT_BOOL, 0, N_("crash the domain after core dump")}, > > + {"gzip", VSH_OT_BOOL, 0, N_("gzip dump(only one compression allowed")}, > > + {"lzop", VSH_OT_BOOL, 0, N_("lzop dump(only one compression allowed")}, > > what about bzip2 and xz, the first one is probably way too slow, but > we accept it for saves. > will add bzip2 and xz. (but maybe very slow.) > > {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, > > {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("where to dump the core")}, > > {NULL, 0, 0, NULL} > > @@ -1778,7 +1780,13 @@ cmdDump(vshControl *ctl, const vshCmd *c > > flags |= VIR_DUMP_LIVE; > > if (vshCommandOptBool (cmd, "crash")) > > flags |= VIR_DUMP_CRASH; > > - > > + if (vshCommandOptBool (cmd, "gzip")) > > + flags |= VIR_DUMP_GZIP; > > + if (vshCommandOptBool (cmd, "lzop")) > > + flags |= VIR_DUMP_LZOP; > > + if ((flags & (VIR_DUMP_GZIP | VIR_DUMP_LZOP)) > > + == (VIR_DUMP_GZIP | VIR_DUMP_LZOP)) > > + return FALSE; > > I think an error message need to be provided stating that both options > are mutually exclusive. > I'll add one. Thank you. -Kame -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list