[libvirt] [PATCH] qemu_driver.c: factor out more duplication

[Daniel Veillard]

On Thu, Sep 10, 2009 at 11:34:37AM +0200, Jim Meyering wrote:
> Charles Duffy wrote:
> >>From 3c4f6568623ed420a9e71da33b9ce74abda289a8 Mon Sep 17 00:00:00 2001
> > From: Charles Duffy <Charles_Duffy at dell.com>
> > Date: Wed, 9 Sep 2009 15:53:25 -0500
> > Subject: [PATCH] Reintroduce support for lzop compression
> >
> > lzop was removed due to some confusion over whether it provided functional
> > advantages distinct from xz. This has been addressed in the mailing list post
> > archived at http://permalink.gmane.org/gmane.comp.emulators.libvirt/16487, and
> > support for lzop is re-added here.
> >
> > lzop is added to the enum after xz, leaving xz at the position lzma was
> > originally in (which is appropriate, as it should handle decompression of
> > lzma-format files) and giving lzop back its prior position.
> >
> > Documentation in qemu.conf is amended to remove references to lzma and add
> > suggestions regarding the tradeoffs made by various compressors.
> >
> > Signed-off-by: Charles Duffy <charles at dyfis.net>
> > ---
> >  libvirt.spec.in   |    1 +
> >  src/qemu.conf     |    7 ++++---
> >  src/qemu_driver.c |    9 ++++++---
> >  3 files changed, 11 insertions(+), 6 deletions(-)
> ...
> >  struct qemud_save_header {
> >      char magic[sizeof(QEMUD_SAVE_MAGIC)-1];
> > @@ -4384,6 +4385,8 @@ static int qemudDomainRestore(virConnectPtr conn,
> >              intermediate_argv[0] = "bzip2";
> >          else if (header.compressed == QEMUD_SAVE_FORMAT_XZ)
> >              intermediate_argv[0] = "xz";
> > +        else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP)
> > +            intermediate_argv[0] = "lzop";
> >          else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) {
> >              qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> >                               _("Unknown compressed save format %d"),
> 
> Thanks for that patch.
> I'm glad I read it, since it made me realize my clean-up
> from yesterday was incomplete.  You should not have had to add
> the literal, "lzop", in two places.
> 
> This completes the job:
> 
> >From 05b3f087b7ac29cebd22dfdf099f8fb36a31c86a Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Thu, 10 Sep 2009 11:26:00 +0200
> Subject: [PATCH] qemu_driver.c: factor out more duplication
> 
> * src/qemu_driver.c (qemudDomainRestore): Use the new ...TypeToString
> function here, too.
> 
> (qemudSaveCompressionTypeToString): Declare.
> ---
>  src/qemu_driver.c |   18 +++++++-----------
>  1 files changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index 5c2a8ec..256e8e7 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -4379,21 +4379,17 @@ static int qemudDomainRestore(virConnectPtr conn,
> 
>      if (header.version == 2) {
>          const char *intermediate_argv[3] = { NULL, "-dc", NULL };
> -        if (header.compressed == QEMUD_SAVE_FORMAT_GZIP)
> -            intermediate_argv[0] = "gzip";
> -        else if (header.compressed == QEMUD_SAVE_FORMAT_BZIP2)
> -            intermediate_argv[0] = "bzip2";
> -        else if (header.compressed == QEMUD_SAVE_FORMAT_XZ)
> -            intermediate_argv[0] = "xz";
> -        else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP)
> -            intermediate_argv[0] = "lzop";
> -        else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) {
> +        const char *prog = qemudSaveCompressionTypeToString(header.compressed);
> +        if (prog == NULL) {
>              qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> -                             _("Unknown compressed save format %d"),
> +                             _("Invalid compressed save format %d"),
>                               header.compressed);
>              goto cleanup;
>          }
> -        if (intermediate_argv[0] != NULL) {
> +
> +        if (header.compressed != QEMUD_SAVE_FORMAT_RAW)
> +            intermediate_argv[0] = prog;
> +        else {
>              intermediatefd = fd;
>              fd = -1;
>              if (virExec(conn, intermediate_argv, NULL, NULL,

  Ah, right :-) ACK !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/