On Wed, Sep 09, 2009 at 10:49:46AM +0200, Daniel Veillard wrote:
> On Wed, Sep 09, 2009 at 09:24:17AM +0200, Jim Meyering wrote:
> > >>> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> > >>> index f64d70b..7b64712 100644
> > >>> --- a/src/qemu_driver.c
> > >>> +++ b/src/qemu_driver.c
> > >>> @@ -3622,7 +3622,8 @@ enum qemud_save_formats {
> > >>> QEMUD_SAVE_FORMAT_RAW,
> > >>> QEMUD_SAVE_FORMAT_GZIP,
> > >>> QEMUD_SAVE_FORMAT_BZIP2,
> > >>> - QEMUD_SAVE_FORMAT_LZMA,
> > >>> + QEMUD_SAVE_FORMAT_LZMA, /* deprecated, in favor of xz */
> > >>> + QEMUD_SAVE_FORMAT_XZ,
> > >>> QEMUD_SAVE_FORMAT_LZOP,
> > >>> };
> > >>
> > >> You'll need to add QEMUD_SAVE_FORMAT_XZ to the end of the enum, to maintain
> > >> on-disk compatibility. Otherwise it looks good.
[...]
> > qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
> > _("Invalid compress format %d"),
> > @@ -4385,6 +4394,8 @@ static int qemudDomainRestore(virConnectPtr conn,
> > intermediate_argv[0] = "lzma";
> > else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP)
> > intermediate_argv[0] = "lzop";
> > + else if (header.compressed == QEMUD_SAVE_FORMAT_XZ)
> > + intermediate_argv[0] = "xz";
> > else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) {
> > qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> > _("Unknown compressed save format %d"),
>
> Hum ... in practice I think it adds a new dependancy to libvirt. We
> didn't raise that for the lzop patch but if we start to exec binaries
> we should check for their presence at runtime somehow.
> I'm not against the patch, actually I have xz on my machine but not
> lzop, but the whole code right now looks quite fragile, we don't offer
> the capacity to detect what compressors are available on the Node and
> just leave the API user to a game of try/failure checking. That doesn't
> sounds too good to me. One way is to add the dependancy at the packaging
> level (at least libvirt.spec.in for now) but right now requesting both
> lzop and xz is just bad IMHO, so some kind of runtime detection should
> be done. For gzip and bzip2 this wasn't really a probem because they are
> really ubiquitous, but now we are suggesting 2 options which are very
> likely to not be both present.
>
> I dislike the fact that we are lowering the API reliability for an
> optimization in disk space usage.
Hum, I realize that support of LZOP was added after 0.7.0, so we never
made a release with it (well except for git snapshot which may have been
pushed).
I wonder if the best is not to just drop the lzop option altogether
and stick xz as a package dependancy until we have found a way to
provide at the API level which compression options are actually
available.
Opinions ?
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list