qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH for-2.8] dmg: Move the driver to block-obj-y


From: Paolo Bonzini
Subject: Re: [Qemu-block] [PATCH for-2.8] dmg: Move the driver to block-obj-y
Date: Wed, 3 Aug 2016 13:41:48 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1


On 03/08/2016 10:01, Fam Zheng wrote:
> dmg.o was moved to block-obj-m in 5505e8b76 to become a separate module,
> so that its reference to libbz2, since 6b383c08c, doesn't add an extra
> library to the main executable.
> 
> We are working on on-demand loading of block drivers which will be
> easier if all format drivers are always built-in (so that the format
> probing is not a egg-and-chicken problem).
> 
> dmg is the only modularized format driver for now. For above reason, we
> want to move it back to block-obj-y, as is done in this patch.
> 
> The bz2 uncompressing code that requires bzip2 library is kept in a
> separate function in the added dmg-bz2.o, which will be loaded at
> program start in bdrv_init() as usual. It fills in the function pointer
> inside dmg.o, so that bz2 uncompress can be done in reading path.
> 
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  block/Makefile.objs |  6 ++---
>  block/dmg-bz2.c     | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
>  block/dmg.c         | 68 
> +++++++++++++----------------------------------------
>  block/dmg.h         | 59 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 140 insertions(+), 55 deletions(-)
>  create mode 100644 block/dmg-bz2.c
>  create mode 100644 block/dmg.h
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 2593a2f..8a8a48e 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -1,4 +1,4 @@
> -block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o
> +block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o 
> dmg.o
>  block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o 
> qcow2-cache.o
>  block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>  block-obj-y += qed-check.o
> @@ -39,7 +39,7 @@ gluster.o-libs     := $(GLUSTERFS_LIBS)
>  ssh.o-cflags       := $(LIBSSH2_CFLAGS)
>  ssh.o-libs         := $(LIBSSH2_LIBS)
>  archipelago.o-libs := $(ARCHIPELAGO_LIBS)
> -block-obj-m        += dmg.o
> -dmg.o-libs         := $(BZIP2_LIBS)
> +block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o

You can make CONFIG_BZIP2=m in configure and simplify this line.

Otherwise,

Reviewed-by: Paolo Bonzini <address@hidden>

> +dmg-bz2.o-libs     := $(BZIP2_LIBS)
>  qcow.o-libs        := -lz
>  linux-aio.o-libs   := -laio
> diff --git a/block/dmg-bz2.c b/block/dmg-bz2.c
> new file mode 100644
> index 0000000..a60e398
> --- /dev/null
> +++ b/block/dmg-bz2.c
> @@ -0,0 +1,62 @@
> +/*
> + * DMG bzip2 uncompression
> + *
> + * Copyright (c) 2004 Johannes E. Schindelin
> + * Copyright (c) 2016 Red Hat, Int.
> + *
> + * 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 <bzlib.h>
> +
> +static int dmg_uncompress_bz2_do(char *next_in, unsigned int avail_in,
> +                                 char *next_out, unsigned int avail_out)
> +{
> +    int ret;
> +    uint64_t total_out;
> +    bz_stream bzstream = {};
> +
> +    ret = BZ2_bzDecompressInit(&bzstream, 0, 0);
> +    if (ret != BZ_OK) {
> +        return -1;
> +    }
> +    bzstream.next_in = next_in;
> +    bzstream.avail_in = avail_in;
> +    bzstream.next_out = next_out;
> +    bzstream.avail_out = avail_out;
> +    ret = BZ2_bzDecompress(&bzstream);
> +    total_out = ((uint64_t)bzstream.total_out_hi32 << 32) +
> +                bzstream.total_out_lo32;
> +    BZ2_bzDecompressEnd(&bzstream);
> +    if (ret != BZ_STREAM_END ||
> +        total_out != avail_out) {
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +__attribute__((constructor))
> +static void dmg_bz2_init(void)
> +{
> +    assert(!dmg_uncompress_bz2);
> +    dmg_uncompress_bz2 = dmg_uncompress_bz2_do;
> +}
> +
> diff --git a/block/dmg.c b/block/dmg.c
> index b0ed89b..861d3aa 100644
> --- a/block/dmg.c
> +++ b/block/dmg.c
> @@ -28,10 +28,10 @@
>  #include "qemu/bswap.h"
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
> -#include <zlib.h>
> -#ifdef CONFIG_BZIP2
> -#include <bzlib.h>
> -#endif
> +#include "dmg.h"
> +
> +int (*dmg_uncompress_bz2)(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
> @@ -41,31 +41,6 @@ enum {
>      DMG_SECTORCOUNTS_MAX = DMG_LENGTHS_MAX / 512,
>  };
>  
> -typedef struct BDRVDMGState {
> -    CoMutex lock;
> -    /* each chunk contains a certain number of sectors,
> -     * offsets[i] is the offset in the .dmg file,
> -     * lengths[i] is the length of the compressed chunk,
> -     * sectors[i] is the sector beginning at offsets[i],
> -     * sectorcounts[i] is the number of sectors in that chunk,
> -     * the sectors array is ordered
> -     * 0<=i<n_chunks */
> -
> -    uint32_t n_chunks;
> -    uint32_t* types;
> -    uint64_t* offsets;
> -    uint64_t* lengths;
> -    uint64_t* sectors;
> -    uint64_t* sectorcounts;
> -    uint32_t current_chunk;
> -    uint8_t *compressed_chunk;
> -    uint8_t *uncompressed_chunk;
> -    z_stream zstream;
> -#ifdef CONFIG_BZIP2
> -    bz_stream bzstream;
> -#endif
> -} BDRVDMGState;
> -
>  static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
>  {
>      int len;
> @@ -210,10 +185,9 @@ static bool dmg_is_known_block_type(uint32_t entry_type)
>      case 0x00000001:    /* uncompressed */
>      case 0x00000002:    /* zeroes */
>      case 0x80000005:    /* zlib */
> -#ifdef CONFIG_BZIP2
> -    case 0x80000006:    /* bzip2 */
> -#endif
>          return true;
> +    case 0x80000006:    /* bzip2 */
> +        return !!dmg_uncompress_bz2;
>      default:
>          return false;
>      }
> @@ -587,9 +561,6 @@ static inline int dmg_read_chunk(BlockDriverState *bs, 
> uint64_t sector_num)
>      if (!is_sector_in_chunk(s, s->current_chunk, sector_num)) {
>          int ret;
>          uint32_t chunk = search_chunk(s, sector_num);
> -#ifdef CONFIG_BZIP2
> -        uint64_t total_out;
> -#endif
>  
>          if (chunk >= s->n_chunks) {
>              return -1;
> @@ -620,8 +591,10 @@ static inline int dmg_read_chunk(BlockDriverState *bs, 
> uint64_t sector_num)
>                  return -1;
>              }
>              break; }
> -#ifdef CONFIG_BZIP2
>          case 0x80000006: /* bzip2 compressed */
> +            if (!dmg_uncompress_bz2) {
> +                break;
> +            }
>              /* we need to buffer, because only the chunk as whole can be
>               * inflated. */
>              ret = bdrv_pread(bs->file, s->offsets[chunk],
> @@ -630,24 +603,15 @@ static inline int dmg_read_chunk(BlockDriverState *bs, 
> uint64_t sector_num)
>                  return -1;
>              }
>  
> -            ret = BZ2_bzDecompressInit(&s->bzstream, 0, 0);
> -            if (ret != BZ_OK) {
> -                return -1;
> -            }
> -            s->bzstream.next_in = (char *)s->compressed_chunk;
> -            s->bzstream.avail_in = (unsigned int) s->lengths[chunk];
> -            s->bzstream.next_out = (char *)s->uncompressed_chunk;
> -            s->bzstream.avail_out = (unsigned int) 512 * 
> s->sectorcounts[chunk];
> -            ret = BZ2_bzDecompress(&s->bzstream);
> -            total_out = ((uint64_t)s->bzstream.total_out_hi32 << 32) +
> -                        s->bzstream.total_out_lo32;
> -            BZ2_bzDecompressEnd(&s->bzstream);
> -            if (ret != BZ_STREAM_END ||
> -                total_out != 512 * s->sectorcounts[chunk]) {
> -                return -1;
> +            ret = dmg_uncompress_bz2((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;
> -#endif /* CONFIG_BZIP2 */
>          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
> new file mode 100644
> index 0000000..b592d6f
> --- /dev/null
> +++ b/block/dmg.h
> @@ -0,0 +1,59 @@
> +/*
> + * Header for DMG driver
> + *
> + * Copyright (c) 2004-2006 Fabrice Bellard
> + * Copyright (c) 2016 Red hat, Inc.
> + *
> + * 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.
> + */
> +
> +#ifndef BLOCK_DMG_H
> +#define BLOCK_DMG_H
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "block/block_int.h"
> +#include <zlib.h>
> +
> +typedef struct BDRVDMGState {
> +    CoMutex lock;
> +    /* each chunk contains a certain number of sectors,
> +     * offsets[i] is the offset in the .dmg file,
> +     * lengths[i] is the length of the compressed chunk,
> +     * sectors[i] is the sector beginning at offsets[i],
> +     * sectorcounts[i] is the number of sectors in that chunk,
> +     * the sectors array is ordered
> +     * 0<=i<n_chunks */
> +
> +    uint32_t n_chunks;
> +    uint32_t *types;
> +    uint64_t *offsets;
> +    uint64_t *lengths;
> +    uint64_t *sectors;
> +    uint64_t *sectorcounts;
> +    uint32_t current_chunk;
> +    uint8_t *compressed_chunk;
> +    uint8_t *uncompressed_chunk;
> +    z_stream zstream;
> +} BDRVDMGState;
> +
> +extern int (*dmg_uncompress_bz2)(char *next_in, unsigned int avail_in,
> +                                 char *next_out, unsigned int avail_out);
> +
> +#endif
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]