[Qemu-devel] [PATCH 0/4] Adding LZFSE compression support for DMG block driver.

[Qemu-devel] [PATCH 0/4] Adding LZFSE compression support for DMG block driver.

[Julio Faracco]

Since Mac OS X El Capitain (v10.11), Apple uses LZFSE compression to 
generate compressed DMGs as an alternative to BZIP2. Possible, Apple
want to keep this algorithm as default in long term. Some years ago, 
Apple opened the LZFSE algorithm to opensource and the main source (or 
the most active repo), can be found at: https://github.com/lzfse/lzfse

Julio Faracco (4):
  block: adding lzfse decompressing support as a module.
  configure: adding support to lzfse library.
  dmg: including dmg-lzfse module inside dmg block driver.
  dmg: exchanging hardcoded dmg UDIF block types to enum.

 block/Makefile.objs |  2 ++
 block/dmg-lzfse.c   | 54 +++++++++++++++++++++++++++++++++++++
 block/dmg.c         | 65 ++++++++++++++++++++++++++++++++++++---------
 block/dmg.h         |  3 +++
 configure           | 32 ++++++++++++++++++++++
 5 files changed, 144 insertions(+), 12 deletions(-)
 create mode 100644 block/dmg-lzfse.c

-- 
2.17.1

[Qemu-devel] [PATCH 1/4] block: adding lzfse decompressing support as a module.

[Julio Faracco]

QEMU dmg support includes zlib and bzip2, but it does not contains lzfse
support. This commit adds the source file to extend compression support
for new DMGs.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 block/dmg-lzfse.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 block/dmg-lzfse.c

diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c
new file mode 100644
index 0000000000..d09b544a0a
--- /dev/null
+++ b/block/dmg-lzfse.c
@@ -0,0 +1,54 @@
+/*
+ * DMG lzfse uncompression
+ *
+ * Copyright (c) 2018 Julio Cesar Faracco
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "dmg.h"
+#include <lzfse.h>
+
+static int dmg_uncompress_lzfse_do(char *next_in, unsigned int avail_in,
+                                   char *next_out, unsigned int avail_out)
+{
+    void *aux;
+    size_t aux_allocated;
+    size_t out_size;
+
+    aux_allocated = lzfse_decode_scratch_size();
+    aux = g_malloc(aux_allocated);
+
+    if (aux_allocated != 0 && aux == 0) {
+        return -1;
+    }
+
+    out_size = lzfse_decode_buffer((uint8_t *) next_out, avail_out,
+                                   (uint8_t *) next_in, avail_in, aux);
+
+    return out_size;
+}
+
+__attribute__((constructor))
+static void dmg_lzfse_init(void)
+{
+    assert(!dmg_uncompress_lzfse);
+    dmg_uncompress_lzfse = dmg_uncompress_lzfse_do;
+}
-- 
2.17.1

[Qemu-devel] [PATCH 2/4] configure: adding support to lzfse library.

[Julio Faracco]

This commit includes the support to lzfse opensource library. With this
library dmg block driver can decompress images with this type of
compression inside.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 block/Makefile.objs |  2 ++
 configure           | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index c8337bf186..f4ddbb9c7b 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -47,6 +47,8 @@ ssh.o-cflags       := $(LIBSSH2_CFLAGS)
 ssh.o-libs         := $(LIBSSH2_LIBS)
 block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
 dmg-bz2.o-libs     := $(BZIP2_LIBS)
+block-obj-$(if $(CONFIG_LZFSE),m,n) += dmg-lzfse.o
+dmg-lzfse.o-libs   := $(LZFSE_LIBS)
 qcow.o-libs        := -lz
 linux-aio.o-libs   := -laio
 parallels.o-cflags := $(LIBXML2_CFLAGS)
diff --git a/configure b/configure
index 2a7796ea80..b12a16f2bf 100755
--- a/configure
+++ b/configure
@@ -432,6 +432,7 @@ capstone=""
 lzo=""
 snappy=""
 bzip2=""
+lzfse=""
 guest_agent=""
 guest_agent_with_vss="no"
 guest_agent_ntddscsi="no"
@@ -1300,6 +1301,10 @@ for opt do
   ;;
   --enable-bzip2) bzip2="yes"
   ;;
+  --enable-lzfse) lzfse="yes"
+  ;;
+  --disable-lzfse) lzfse="no"
+  ;;
   --enable-guest-agent) guest_agent="yes"
   ;;
   --disable-guest-agent) guest_agent="no"
@@ -1689,6 +1694,8 @@ disabled with --disable-FEATURE, default is enabled if available:
   snappy          support of snappy compression library
   bzip2           support of bzip2 compression library
                   (for reading bzip2-compressed dmg images)
+  lzfse           support of lzfse compression library
+                  (for reading lzfse-compressed dmg images)
   seccomp         seccomp support
   coroutine-pool  coroutine freelist (better performance)
   glusterfs       GlusterFS backend
@@ -2213,6 +2220,25 @@ EOF
     fi
 fi
 
+##########################################
+# lzfse check
+
+if test "$lzfse" != "no" ; then
+    cat > $TMPC << EOF
+#include <lzfse.h>
+int main(void) { lzfse_decode_scratch_size(); return 0; }
+EOF
+    if compile_prog "" "-llzfse" ; then
+        libs_softmmu="$libs_softmmu -llzfse"
+        lzfse="yes"
+    else
+        if test "$lzfse" = "yes"; then
+            feature_not_found "lzfse" "Install lzfse devel"
+        fi
+        lzfse="no"
+    fi
+fi
+
 ##########################################
 # libseccomp check
 
@@ -6001,6 +6027,7 @@ echo "Live block migration $live_block_migration"
 echo "lzo support       $lzo"
 echo "snappy support    $snappy"
 echo "bzip2 support     $bzip2"
+echo "lzfse support     $lzfse"
 echo "NUMA host support $numa"
 echo "libxml2           $libxml2"
 echo "tcmalloc support  $tcmalloc"
@@ -6525,6 +6552,11 @@ if test "$bzip2" = "yes" ; then
   echo "BZIP2_LIBS=-lbz2" >> $config_host_mak
 fi
 
+if test "$lzfse" = "yes" ; then
+  echo "CONFIG_LZFSE=y" >> $config_host_mak
+  echo "LZFSE_LIBS=-llzfse" >> $config_host_mak
+fi
+
 if test "$libiscsi" = "yes" ; then
   echo "CONFIG_LIBISCSI=m" >> $config_host_mak
   echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak
-- 
2.17.1

[Qemu-devel] [PATCH 3/4] dmg: including dmg-lzfse module inside dmg block driver.

[Julio Faracco]

This commit includes the support to new module dmg-lzfse into dmg block
driver. It includes the support for block type ULFO (0x80000007).

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 block/dmg.c | 28 ++++++++++++++++++++++++++++
 block/dmg.h |  3 +++
 2 files changed, 31 insertions(+)

diff --git a/block/dmg.c b/block/dmg.c
index c9b3c519c4..390ab67e53 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -33,6 +33,9 @@
 int (*dmg_uncompress_bz2)(char *next_in, unsigned int avail_in,
                           char *next_out, unsigned int avail_out);
 
+int (*dmg_uncompress_lzfse)(char *next_in, unsigned int avail_in,
+                            char *next_out, unsigned int avail_out);
+
 enum {
     /* Limit chunk sizes to prevent unreasonable amounts of memory being used
      * or truncating when converting to 32-bit types
@@ -107,6 +110,7 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
     switch (s->types[chunk]) {
     case 0x80000005: /* zlib compressed */
     case 0x80000006: /* bzip2 compressed */
+    case 0x80000007: /* lzfse compressed */
         compressed_size = s->lengths[chunk];
         uncompressed_sectors = s->sectorcounts[chunk];
         break;
@@ -188,6 +192,8 @@ static bool dmg_is_known_block_type(uint32_t entry_type)
         return true;
     case 0x80000006:    /* bzip2 */
         return !!dmg_uncompress_bz2;
+    case 0x80000007:    /* lzfse */
+	return !!dmg_uncompress_lzfse;
     default:
         return false;
     }
@@ -431,6 +437,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     block_module_load_one("dmg-bz2");
+    block_module_load_one("dmg-lzfse");
 
     s->n_chunks = 0;
     s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
@@ -629,6 +636,27 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
                 return ret;
             }
             break;
+	case 0x80000007:
+	    if (!dmg_uncompress_lzfse) {
+                break;
+            }
+            /* we need to buffer, because only the chunk as whole can be
+             * inflated. */
+            ret = bdrv_pread(bs->file, s->offsets[chunk],
+                             s->compressed_chunk, s->lengths[chunk]);
+            if (ret != s->lengths[chunk]) {
+                return -1;
+            }
+
+            ret = dmg_uncompress_lzfse((char *)s->compressed_chunk,
+                                       (unsigned int) s->lengths[chunk],
+                                       (char *)s->uncompressed_chunk,
+                                       (unsigned int)
+                                           (512 * s->sectorcounts[chunk]));
+            if (ret < 0) {
+                return ret;
+            }
+            break;
         case 1: /* copy */
             ret = bdrv_pread(bs->file, s->offsets[chunk],
                              s->uncompressed_chunk, s->lengths[chunk]);
diff --git a/block/dmg.h b/block/dmg.h
index 2ecf239ba5..f28929998f 100644
--- a/block/dmg.h
+++ b/block/dmg.h
@@ -55,4 +55,7 @@ typedef struct BDRVDMGState {
 extern int (*dmg_uncompress_bz2)(char *next_in, unsigned int avail_in,
                                  char *next_out, unsigned int avail_out);
 
+extern int (*dmg_uncompress_lzfse)(char *next_in, unsigned int avail_in,
+                                   char *next_out, unsigned int avail_out);
+
 #endif
-- 
2.17.1

[Qemu-devel] [PATCH 4/4] dmg: exchanging hardcoded dmg UDIF block types to enum.

[Julio Faracco]

This change is better to understand what kind of block type is being
handled by the code. Using a syntax similar to the DMG documentation is
easier than tracking all hex values assigned to a block type.

Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
---
 block/dmg.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index 390ab67e53..ae379adb20 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -44,6 +44,19 @@ enum {
     DMG_SECTORCOUNTS_MAX = DMG_LENGTHS_MAX / 512,
 };
 
+enum {
+    /* DMG Block Type */
+    UDZE=0, /* Zeroes */
+    UDRW,   /* RAW type */
+    UDIG,   /* Ignore */
+    UDCO=0x80000004,
+    UDZO,
+    UDBZ,
+    ULFO,
+    UDCM=0x7ffffffe, /* Comments */
+    UDLE             /* Last Entry */
+};
+
 static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
     int len;
@@ -108,16 +121,16 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
     uint32_t uncompressed_sectors = 0;
 
     switch (s->types[chunk]) {
-    case 0x80000005: /* zlib compressed */
-    case 0x80000006: /* bzip2 compressed */
-    case 0x80000007: /* lzfse compressed */
+    case UDCO: /* zlib compressed */
+    case UDBZ: /* bzip2 compressed */
+    case ULFO: /* lzfse compressed */
         compressed_size = s->lengths[chunk];
         uncompressed_sectors = s->sectorcounts[chunk];
         break;
-    case 1: /* copy */
+    case UDRW: /* copy */
         uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
         break;
-    case 2: /* zero */
+    case UDIG: /* zero */
         /* as the all-zeroes block may be large, it is treated specially: the
          * sector is not copied from a large buffer, a simple memset is used
          * instead. Therefore uncompressed_sectors does not need to be set. */
@@ -186,13 +199,13 @@ typedef struct DmgHeaderState {
 static bool dmg_is_known_block_type(uint32_t entry_type)
 {
     switch (entry_type) {
-    case 0x00000001:    /* uncompressed */
-    case 0x00000002:    /* zeroes */
-    case 0x80000005:    /* zlib */
+    case UDRW:    /* uncompressed */
+    case UDIG:    /* zeroes */
+    case UDCO:    /* zlib */
         return true;
-    case 0x80000006:    /* bzip2 */
+    case UDBZ:    /* bzip2 */
         return !!dmg_uncompress_bz2;
-    case 0x80000007:    /* lzfse */
+    case ULFO:    /* lzfse */
 	return !!dmg_uncompress_lzfse;
     default:
         return false;
@@ -592,7 +605,7 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
 
         s->current_chunk = s->n_chunks;
         switch (s->types[chunk]) { /* block entry type */
-        case 0x80000005: { /* zlib compressed */
+        case UDZO: { /* zlib compressed */
             /* we need to buffer, because only the chunk as whole can be
              * inflated. */
             ret = bdrv_pread(bs->file, s->offsets[chunk],
@@ -615,7 +628,7 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
                 return -1;
             }
             break; }
-        case 0x80000006: /* bzip2 compressed */
+        case UDBZ: /* bzip2 compressed */
             if (!dmg_uncompress_bz2) {
                 break;
             }
@@ -636,7 +649,7 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
                 return ret;
             }
             break;
-	case 0x80000007:
+	case ULFO:
 	    if (!dmg_uncompress_lzfse) {
                 break;
             }
@@ -657,14 +670,14 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
                 return ret;
             }
             break;
-        case 1: /* copy */
+        case UDRW: /* copy */
             ret = bdrv_pread(bs->file, s->offsets[chunk],
                              s->uncompressed_chunk, s->lengths[chunk]);
             if (ret != s->lengths[chunk]) {
                 return -1;
             }
             break;
-        case 2: /* zero */
+        case UDIG: /* zero */
             /* see dmg_read, it is treated specially. No buffer needs to be
              * pre-filled, the zeroes can be set directly. */
             break;
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/4] block: adding lzfse decompressing support as a module.

[Kevin Wolf]

Am 10.08.2018 um 06:07 hat Julio Faracco geschrieben:
> QEMU dmg support includes zlib and bzip2, but it does not contains lzfse
> support. This commit adds the source file to extend compression support
> for new DMGs.
> 
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
>  block/dmg-lzfse.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 block/dmg-lzfse.c
> 
> diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c
> new file mode 100644
> index 0000000000..d09b544a0a
> --- /dev/null
> +++ b/block/dmg-lzfse.c
> @@ -0,0 +1,54 @@
> +/*
> + * DMG lzfse uncompression
> + *
> + * Copyright (c) 2018 Julio Cesar Faracco
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "dmg.h"
> +#include <lzfse.h>
> +
> +static int dmg_uncompress_lzfse_do(char *next_in, unsigned int avail_in,
> +                                   char *next_out, unsigned int avail_out)
> +{
> +    void *aux;
> +    size_t aux_allocated;
> +    size_t out_size;
> +
> +    aux_allocated = lzfse_decode_scratch_size();
> +    aux = g_malloc(aux_allocated);
> +
> +    if (aux_allocated != 0 && aux == 0) {
> +        return -1;
> +    }
> +
> +    out_size = lzfse_decode_buffer((uint8_t *) next_out, avail_out,
> +                                   (uint8_t *) next_in, avail_in, aux);
> +
> +    return out_size;
> +}

aux is leaked in this function.

Also, I don't think we actually need it. lzfse_decode_buffer() is
documented to just do the necessary allocation internally if NULL is
passed. Unless we do something more sophisticated, doing the memory
allocation ourselves is unnecessary complexity.

Kevin

Re: [Qemu-devel] [PATCH 2/4] configure: adding support to lzfse library.

[Kevin Wolf]

Am 10.08.2018 um 06:07 hat Julio Faracco geschrieben:
> This commit includes the support to lzfse opensource library. With this
> library dmg block driver can decompress images with this type of
> compression inside.
> 
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
>  block/Makefile.objs |  2 ++
>  configure           | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index c8337bf186..f4ddbb9c7b 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -47,6 +47,8 @@ ssh.o-cflags       := $(LIBSSH2_CFLAGS)
>  ssh.o-libs         := $(LIBSSH2_LIBS)
>  block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
>  dmg-bz2.o-libs     := $(BZIP2_LIBS)
> +block-obj-$(if $(CONFIG_LZFSE),m,n) += dmg-lzfse.o
> +dmg-lzfse.o-libs   := $(LZFSE_LIBS)
>  qcow.o-libs        := -lz
>  linux-aio.o-libs   := -laio
>  parallels.o-cflags := $(LIBXML2_CFLAGS)
> diff --git a/configure b/configure
> index 2a7796ea80..b12a16f2bf 100755
> --- a/configure
> +++ b/configure
> @@ -432,6 +432,7 @@ capstone=""
>  lzo=""
>  snappy=""
>  bzip2=""
> +lzfse=""
>  guest_agent=""
>  guest_agent_with_vss="no"
>  guest_agent_ntddscsi="no"
> @@ -1300,6 +1301,10 @@ for opt do
>    ;;
>    --enable-bzip2) bzip2="yes"
>    ;;
> +  --enable-lzfse) lzfse="yes"
> +  ;;
> +  --disable-lzfse) lzfse="no"
> +  ;;
>    --enable-guest-agent) guest_agent="yes"
>    ;;
>    --disable-guest-agent) guest_agent="no"
> @@ -1689,6 +1694,8 @@ disabled with --disable-FEATURE, default is enabled if available:
>    snappy          support of snappy compression library
>    bzip2           support of bzip2 compression library
>                    (for reading bzip2-compressed dmg images)
> +  lzfse           support of lzfse compression library
> +                  (for reading lzfse-compressed dmg images)
>    seccomp         seccomp support
>    coroutine-pool  coroutine freelist (better performance)
>    glusterfs       GlusterFS backend
> @@ -2213,6 +2220,25 @@ EOF
>      fi
>  fi
>  
> +##########################################
> +# lzfse check
> +
> +if test "$lzfse" != "no" ; then
> +    cat > $TMPC << EOF
> +#include <lzfse.h>
> +int main(void) { lzfse_decode_scratch_size(); return 0; }
> +EOF
> +    if compile_prog "" "-llzfse" ; then
> +        libs_softmmu="$libs_softmmu -llzfse"

Are you sure about libs_softmmu? I think this is only for QEMU proper,
but not for tools like qemu-img or qemu-io, so if this were relevant,
we'd be missing lzfse support in some tools.

> +        lzfse="yes"
> +    else
> +        if test "$lzfse" = "yes"; then
> +            feature_not_found "lzfse" "Install lzfse devel"
> +        fi
> +        lzfse="no"
> +    fi
> +fi
> +
>  ##########################################
>  # libseccomp check
>  
> @@ -6001,6 +6027,7 @@ echo "Live block migration $live_block_migration"
>  echo "lzo support       $lzo"
>  echo "snappy support    $snappy"
>  echo "bzip2 support     $bzip2"
> +echo "lzfse support     $lzfse"
>  echo "NUMA host support $numa"
>  echo "libxml2           $libxml2"
>  echo "tcmalloc support  $tcmalloc"
> @@ -6525,6 +6552,11 @@ if test "$bzip2" = "yes" ; then
>    echo "BZIP2_LIBS=-lbz2" >> $config_host_mak
>  fi
>  
> +if test "$lzfse" = "yes" ; then
> +  echo "CONFIG_LZFSE=y" >> $config_host_mak
> +  echo "LZFSE_LIBS=-llzfse" >> $config_host_mak

But since we have LZFSE_LIBS here and this is referenced in
block/Makefile.objs, I suspect that the libs_softmmu addition is
actually redundant and could just go away above.

> +fi
> +
>  if test "$libiscsi" = "yes" ; then
>    echo "CONFIG_LIBISCSI=m" >> $config_host_mak
>    echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak

Kevin

Re: [Qemu-devel] [PATCH 3/4] dmg: including dmg-lzfse module inside dmg block driver.

[Kevin Wolf]

Am 10.08.2018 um 06:07 hat Julio Faracco geschrieben:
> This commit includes the support to new module dmg-lzfse into dmg block
> driver. It includes the support for block type ULFO (0x80000007).
> 
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
>  block/dmg.c | 28 ++++++++++++++++++++++++++++
>  block/dmg.h |  3 +++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/block/dmg.c b/block/dmg.c
> index c9b3c519c4..390ab67e53 100644
> --- a/block/dmg.c
> +++ b/block/dmg.c
> @@ -33,6 +33,9 @@
>  int (*dmg_uncompress_bz2)(char *next_in, unsigned int avail_in,
>                            char *next_out, unsigned int avail_out);
>  
> +int (*dmg_uncompress_lzfse)(char *next_in, unsigned int avail_in,
> +                            char *next_out, unsigned int avail_out);
> +
>  enum {
>      /* Limit chunk sizes to prevent unreasonable amounts of memory being used
>       * or truncating when converting to 32-bit types
> @@ -107,6 +110,7 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
>      switch (s->types[chunk]) {
>      case 0x80000005: /* zlib compressed */
>      case 0x80000006: /* bzip2 compressed */
> +    case 0x80000007: /* lzfse compressed */
>          compressed_size = s->lengths[chunk];
>          uncompressed_sectors = s->sectorcounts[chunk];
>          break;
> @@ -188,6 +192,8 @@ static bool dmg_is_known_block_type(uint32_t entry_type)
>          return true;
>      case 0x80000006:    /* bzip2 */
>          return !!dmg_uncompress_bz2;
> +    case 0x80000007:    /* lzfse */
> +	return !!dmg_uncompress_lzfse;

Indentation is off. I think you got a tab instead of spaces here.

>      default:
>          return false;
>      }
> @@ -431,6 +437,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
>      }
>  
>      block_module_load_one("dmg-bz2");
> +    block_module_load_one("dmg-lzfse");
>  
>      s->n_chunks = 0;
>      s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
> @@ -629,6 +636,27 @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
>                  return ret;
>              }
>              break;
> +	case 0x80000007:
> +	    if (!dmg_uncompress_lzfse) {

More tab damage in these two lines.

> +                break;
> +            }
> +            /* we need to buffer, because only the chunk as whole can be
> +             * inflated. */
> +            ret = bdrv_pread(bs->file, s->offsets[chunk],
> +                             s->compressed_chunk, s->lengths[chunk]);
> +            if (ret != s->lengths[chunk]) {
> +                return -1;
> +            }
> +
> +            ret = dmg_uncompress_lzfse((char *)s->compressed_chunk,
> +                                       (unsigned int) s->lengths[chunk],
> +                                       (char *)s->uncompressed_chunk,
> +                                       (unsigned int)
> +                                           (512 * s->sectorcounts[chunk]));
> +            if (ret < 0) {

lzfse_decode_buffer() returns 0 in error cases, and possibly a short
number of bytes. Should the condition compare ret with the expected full
byte count?

> +                return ret;
> +            }
> +            break;
>          case 1: /* copy */
>              ret = bdrv_pread(bs->file, s->offsets[chunk],
>                               s->uncompressed_chunk, s->lengths[chunk]);

Kevin

Re: [Qemu-devel] [PATCH 4/4] dmg: exchanging hardcoded dmg UDIF block types to enum.

[Kevin Wolf]

Am 10.08.2018 um 06:07 hat Julio Faracco geschrieben:
> This change is better to understand what kind of block type is being
> handled by the code. Using a syntax similar to the DMG documentation is
> easier than tracking all hex values assigned to a block type.
> 
> Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> ---
>  block/dmg.c | 43 ++++++++++++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/block/dmg.c b/block/dmg.c
> index 390ab67e53..ae379adb20 100644
> --- a/block/dmg.c
> +++ b/block/dmg.c
> @@ -44,6 +44,19 @@ enum {
>      DMG_SECTORCOUNTS_MAX = DMG_LENGTHS_MAX / 512,
>  };
>  
> +enum {
> +    /* DMG Block Type */
> +    UDZE=0, /* Zeroes */
> +    UDRW,   /* RAW type */
> +    UDIG,   /* Ignore */
> +    UDCO=0x80000004,

Note that 0x80000004 didn't exist in the code previously, so UDCO
shouldn't occur anywhere else in this patch.

> +    UDZO,
> +    UDBZ,
> +    ULFO,
> +    UDCM=0x7ffffffe, /* Comments */
> +    UDLE             /* Last Entry */
> +};
> +
>  static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
>  {
>      int len;
> @@ -108,16 +121,16 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
>      uint32_t uncompressed_sectors = 0;
>  
>      switch (s->types[chunk]) {
> -    case 0x80000005: /* zlib compressed */
> -    case 0x80000006: /* bzip2 compressed */
> -    case 0x80000007: /* lzfse compressed */
> +    case UDCO: /* zlib compressed */

Oops, should this be UDZO?

> +    case UDBZ: /* bzip2 compressed */
> +    case ULFO: /* lzfse compressed */
>          compressed_size = s->lengths[chunk];
>          uncompressed_sectors = s->sectorcounts[chunk];
>          break;
> -    case 1: /* copy */
> +    case UDRW: /* copy */
>          uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
>          break;
> -    case 2: /* zero */
> +    case UDIG: /* zero */
>          /* as the all-zeroes block may be large, it is treated specially: the
>           * sector is not copied from a large buffer, a simple memset is used
>           * instead. Therefore uncompressed_sectors does not need to be set. */
> @@ -186,13 +199,13 @@ typedef struct DmgHeaderState {
>  static bool dmg_is_known_block_type(uint32_t entry_type)
>  {
>      switch (entry_type) {
> -    case 0x00000001:    /* uncompressed */
> -    case 0x00000002:    /* zeroes */
> -    case 0x80000005:    /* zlib */
> +    case UDRW:    /* uncompressed */
> +    case UDIG:    /* zeroes */
> +    case UDCO:    /* zlib */

And this one, too?

>          return true;
> -    case 0x80000006:    /* bzip2 */
> +    case UDBZ:    /* bzip2 */
>          return !!dmg_uncompress_bz2;
> -    case 0x80000007:    /* lzfse */
> +    case ULFO:    /* lzfse */
>  	return !!dmg_uncompress_lzfse;
>      default:
>          return false;

Kevin

Re: [Qemu-devel] [PATCH 2/4] configure: adding support to lzfse library.

[Julio Faracco]

Em seg, 13 de ago de 2018 às 11:10, Kevin Wolf <kwolf@redhat.com> escreveu:
>
> Am 10.08.2018 um 06:07 hat Julio Faracco geschrieben:
> > This commit includes the support to lzfse opensource library. With this
> > library dmg block driver can decompress images with this type of
> > compression inside.
> >
> > Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
> > ---
> >  block/Makefile.objs |  2 ++
> >  configure           | 32 ++++++++++++++++++++++++++++++++
> >  2 files changed, 34 insertions(+)
> >
> > diff --git a/block/Makefile.objs b/block/Makefile.objs
> > index c8337bf186..f4ddbb9c7b 100644
> > --- a/block/Makefile.objs
> > +++ b/block/Makefile.objs
> > @@ -47,6 +47,8 @@ ssh.o-cflags       := $(LIBSSH2_CFLAGS)
> >  ssh.o-libs         := $(LIBSSH2_LIBS)
> >  block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
> >  dmg-bz2.o-libs     := $(BZIP2_LIBS)
> > +block-obj-$(if $(CONFIG_LZFSE),m,n) += dmg-lzfse.o
> > +dmg-lzfse.o-libs   := $(LZFSE_LIBS)
> >  qcow.o-libs        := -lz
> >  linux-aio.o-libs   := -laio
> >  parallels.o-cflags := $(LIBXML2_CFLAGS)
> > diff --git a/configure b/configure
> > index 2a7796ea80..b12a16f2bf 100755
> > --- a/configure
> > +++ b/configure
> > @@ -432,6 +432,7 @@ capstone=""
> >  lzo=""
> >  snappy=""
> >  bzip2=""
> > +lzfse=""
> >  guest_agent=""
> >  guest_agent_with_vss="no"
> >  guest_agent_ntddscsi="no"
> > @@ -1300,6 +1301,10 @@ for opt do
> >    ;;
> >    --enable-bzip2) bzip2="yes"
> >    ;;
> > +  --enable-lzfse) lzfse="yes"
> > +  ;;
> > +  --disable-lzfse) lzfse="no"
> > +  ;;
> >    --enable-guest-agent) guest_agent="yes"
> >    ;;
> >    --disable-guest-agent) guest_agent="no"
> > @@ -1689,6 +1694,8 @@ disabled with --disable-FEATURE, default is enabled if available:
> >    snappy          support of snappy compression library
> >    bzip2           support of bzip2 compression library
> >                    (for reading bzip2-compressed dmg images)
> > +  lzfse           support of lzfse compression library
> > +                  (for reading lzfse-compressed dmg images)
> >    seccomp         seccomp support
> >    coroutine-pool  coroutine freelist (better performance)
> >    glusterfs       GlusterFS backend
> > @@ -2213,6 +2220,25 @@ EOF
> >      fi
> >  fi
> >
> > +##########################################
> > +# lzfse check
> > +
> > +if test "$lzfse" != "no" ; then
> > +    cat > $TMPC << EOF
> > +#include <lzfse.h>
> > +int main(void) { lzfse_decode_scratch_size(); return 0; }
> > +EOF
> > +    if compile_prog "" "-llzfse" ; then
> > +        libs_softmmu="$libs_softmmu -llzfse"
>
> Are you sure about libs_softmmu? I think this is only for QEMU proper,
> but not for tools like qemu-img or qemu-io, so if this were relevant,
> we'd be missing lzfse support in some tools.

It is relevant for qemu-img because it can be able to convert dmg file
into qcow2 or any other format.
Right now, I don't think we really need to implement something
specific for those tools.

Never mind, I really don't know why I included softmmu here. My final
local commit does not have it.
I probably sent the wrong file patch file. Well, I need to send a V2 anyway.

>
> > +        lzfse="yes"
> > +    else
> > +        if test "$lzfse" = "yes"; then
> > +            feature_not_found "lzfse" "Install lzfse devel"
> > +        fi
> > +        lzfse="no"
> > +    fi
> > +fi
> > +
> >  ##########################################
> >  # libseccomp check
> >
> > @@ -6001,6 +6027,7 @@ echo "Live block migration $live_block_migration"
> >  echo "lzo support       $lzo"
> >  echo "snappy support    $snappy"
> >  echo "bzip2 support     $bzip2"
> > +echo "lzfse support     $lzfse"
> >  echo "NUMA host support $numa"
> >  echo "libxml2           $libxml2"
> >  echo "tcmalloc support  $tcmalloc"
> > @@ -6525,6 +6552,11 @@ if test "$bzip2" = "yes" ; then
> >    echo "BZIP2_LIBS=-lbz2" >> $config_host_mak
> >  fi
> >
> > +if test "$lzfse" = "yes" ; then
> > +  echo "CONFIG_LZFSE=y" >> $config_host_mak
> > +  echo "LZFSE_LIBS=-llzfse" >> $config_host_mak
>
> But since we have LZFSE_LIBS here and this is referenced in
> block/Makefile.objs, I suspect that the libs_softmmu addition is
> actually redundant and could just go away above.
>
> > +fi
> > +
> >  if test "$libiscsi" = "yes" ; then
> >    echo "CONFIG_LIBISCSI=m" >> $config_host_mak
> >    echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak
>
> Kevin