Re: [libvirt] [PATCH v2 1/2] visrh dump compression support

On Mon, Oct 25, 2010 at 09:04:10AM +0900, KAMEZAWA Hiroyuki wrote: > Sorry, email was empty.. > > == > Add dump_image_format[] to qemu.conf and support compressed dump > at virsh dump. coredump compression is important for saving disk space > in an environment where multiple guest run. > (In general, "disk space for dump" is specially allocated and will be > a dead space in the system. It's used only at emergency. So, it's better > to have both of save_image_format and dump_image_format. "save" is done > in scheduled manner with enough calculated disk space for it.) > > This code reuses some of save_image_format[] and supports the same format > with virsh save. > > --- > src/qemu/qemu.conf | 4 ++++ > src/qemu/qemu_conf.c | 11 +++++++++++ > src/qemu/qemu_conf.h | 1 + > src/qemu/qemu_driver.c | 30 +++++++++++++++++++++++++----- > 4 files changed, 41 insertions(+), 5 deletions(-) This all looks good, but it is also neccessary to add the new option to the augeas files libvirtd_qemu.aug and test_libvirtd_qemu.aug

> > Index: libvirt-0.8.4/src/qemu/qemu_conf.c > =================================================================== > --- libvirt-0.8.4.orig/src/qemu/qemu_conf.c > +++ libvirt-0.8.4/src/qemu/qemu_conf.c > @@ -324,6 +324,17 @@ int qemudLoadDriverConfig(struct qemud_d > } > } > > + p = virConfGetValue (conf, "dump_image_format"); > + CHECK_TYPE ("dump_image_format", VIR_CONF_STRING); > + if (p && p->str) { > + VIR_FREE(driver->dumpImageFormat); > + if (!(driver->dumpImageFormat = strdup(p->str))) { > + virReportOOMError(); > + virConfFree(conf); > + return -1; > + } > + } > + > p = virConfGetValue (conf, "hugetlbfs_mount"); > CHECK_TYPE ("hugetlbfs_mount", VIR_CONF_STRING); > if (p && p->str) { > Index: libvirt-0.8.4/src/qemu/qemu_conf.h > =================================================================== > --- libvirt-0.8.4.orig/src/qemu/qemu_conf.h > +++ libvirt-0.8.4/src/qemu/qemu_conf.h > @@ -159,6 +159,7 @@ struct qemud_driver { > virSecurityDriverPtr securitySecondaryDriver; > > char *saveImageFormat; > + char *dumpImageFormat; > > pciDeviceList *activePciHostdevs; > > Index: libvirt-0.8.4/src/qemu/qemu.conf > =================================================================== > --- libvirt-0.8.4.orig/src/qemu/qemu.conf > +++ libvirt-0.8.4/src/qemu/qemu.conf > @@ -144,7 +144,11 @@ > # saving a domain in order to save disk space; the list above is in descending > # order by performance and ascending order by compression ratio. > # > +# save_image_format is used when you use 'virsh save' at scheduled saving. > +# dump_image_format is used when you use 'virsh dump' at emergency crashdump. > +# > # save_image_format = "raw" > +# dump_image_format = "raw" > > # If provided by the host and a hugetlbfs mount point is configured, > # a guest may request huge page backing. When this mount point is > 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 > @@ -5716,11 +5716,15 @@ static int qemudDomainCoreDump(virDomain > int resume = 0, paused = 0; > int ret = -1, fd = -1; > virDomainEventPtr event = NULL; > - const char *args[] = { > - "cat", > - NULL, > - }; > + int compress; > qemuDomainObjPrivatePtr priv; > + /* > + * We reuse "save" flag for "dump" here. Then, we can support the same > + * format in "save" and "dump". > + */ > + compress = QEMUD_SAVE_FORMAT_RAW; > + if (driver->dumpImageFormat) > + compress = qemudSaveCompressionTypeFromString(driver->dumpImageFormat); > > qemuDriverLock(driver); > vm = virDomainFindByUUID(&driver->domains, dom->uuid); > @@ -5787,9 +5791,25 @@ static int qemudDomainCoreDump(virDomain > } > > qemuDomainObjEnterMonitorWithDriver(driver, vm); > - ret = qemuMonitorMigrateToFile(priv->mon, > + if (compress == QEMUD_SAVE_FORMAT_RAW) { > + const char *args[] = { > + "cat", > + NULL, > + }; > + ret = qemuMonitorMigrateToFile(priv->mon, > QEMU_MONITOR_MIGRATE_BACKGROUND, > args, path, 0); > + } else { > + const char *prog = qemudSaveCompressionTypeToString(compress); > + const char *args[] = { > + prog, > + "-c", > + NULL, > + }; > + ret = qemuMonitorMigrateToFile(priv->mon, > + QEMU_MONITOR_MIGRATE_BACKGROUND, > + args, path, 0); > + } The whitespace indentation looks wrong here, seems to be using tabs instead of spaces. You can verify coding style by running 'make syntax-check'