Re: [Qemu-devel] [PATCH 02/23] block: New BlockBackend

["Benoît Canet" <benoit.canet@irqsave.net>]

The Wednesday 10 Sep 2014 à 14:40:42 (+0200), Benoît Canet wrote :
> The Wednesday 10 Sep 2014 à 10:13:31 (+0200), Markus Armbruster wrote :
> > A block device consists of a frontend device model and a backend.
> > 
> > A block backend has a tree of block drivers doing the actual work.
> > The tree is managed by the block layer.
> > 
> > We currently use a single abstraction BlockDriverState both for tree
> > nodes and the backend as a whole.  Drawbacks:
> > 
> > * Its API includes both stuff that makes sense only at the block
> >   backend level (root of the tree) and stuff that's only for use
> >   within the block layer.  This makes the API bigger and more complex
> >   than necessary.  Moreover, it's not obvious which interfaces are
> >   meant for device models, and which really aren't.
> > 
> > * Since device models keep a reference to their backend, the backend
> >   object can't just be destroyed.  But for media change, we need to
> >   replace the tree.  Our solution is to make the BlockDriverState
> >   generic, with actual driver state in a separate object, pointed to
> >   by member opaque.  That lets us replace the tree by deinitializing
> >   and reinitializing its root.  This special need of the root makes
> >   the data structure awkward everywhere in the tree.
> > 
> > The general plan is to separate the APIs into "block backend", for use
> > by device models, monitor and whatever other code dealing with block
> > backends, and "block driver", for use by the block layer and whatever
> > other code (if any) dealing with trees and tree nodes.
> > 
> > Code dealing with block backends, device models in particular, should
> > become completely oblivious of BlockDriverState.  This should let us
> > clean up both APIs, and the tree data structures.
> > 
> > This commit is a first step.  It creates a minimal "block backend"
> > API: type BlockBackend and functions to create, destroy and find them.
> > BlockBackend objects are created and destroyed, but not yet used for
> > anything; that'll come shortly.
> > 
> > BlockBackend is reference-counted.  Its reference count never exceeds
> > one so far, but that's going to change.
> > 
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > ---
> >  block/Makefile.objs            |   2 +-
> >  block/block-backend.c          | 110 +++++++++++++++++++++++++++++++++++++++++
> >  blockdev.c                     |  10 +++-
> >  hw/block/xen_disk.c            |  11 +++++
> >  include/qemu/typedefs.h        |   1 +
> >  include/sysemu/block-backend.h |  26 ++++++++++
> >  qemu-img.c                     |  46 +++++++++++++++++
> >  qemu-io.c                      |   8 +++
> >  qemu-nbd.c                     |   3 +-
> >  9 files changed, 214 insertions(+), 3 deletions(-)
> >  create mode 100644 block/block-backend.c
> >  create mode 100644 include/sysemu/block-backend.h
> > 
> > diff --git a/block/Makefile.objs b/block/Makefile.objs
> > index f45f939..a70140b 100644
> > --- a/block/Makefile.objs
> > +++ b/block/Makefile.objs
> > @@ -5,7 +5,7 @@ block-obj-y += qed-check.o
> >  block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o
> >  block-obj-$(CONFIG_QUORUM) += quorum.o
> >  block-obj-y += parallels.o blkdebug.o blkverify.o
> > -block-obj-y += snapshot.o qapi.o
> > +block-obj-y += block-backend.o snapshot.o qapi.o
> >  block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
> >  block-obj-$(CONFIG_POSIX) += raw-posix.o
> >  block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> > diff --git a/block/block-backend.c b/block/block-backend.c
> > new file mode 100644
> > index 0000000..833f7d9
> > --- /dev/null
> > +++ b/block/block-backend.c
> > @@ -0,0 +1,110 @@
> > +/*
> > + * QEMU Block backends
> > + *
> > + * Copyright (C) 2014 Red Hat, Inc.
> > + *
> > + * Authors:
> > + *  Markus Armbruster <armbru@redhat.com>,
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > + * later.  See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "sysemu/block-backend.h"
> > +#include "block/block_int.h"
> > +
> > +struct BlockBackend {
> > +    char *name;
> > +    int refcnt;
> > +    QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */
> > +};
> > +
> > +static QTAILQ_HEAD(, BlockBackend) blk_backends =
> > +    QTAILQ_HEAD_INITIALIZER(blk_backends);
> > +
> > +/**
> > + * blk_new:
> > + * @name: name, must not be %NULL or empty
> > + * @errp: return location for an error to be set on failure, or %NULL
> > + *
> > + * Create a new BlockBackend, with a reference count of one.  Fail if
> > + * @name already exists.
> > + *
> > + * Returns: the BlockBackend on success, %NULL on failure
> > + */
> > +BlockBackend *blk_new(const char *name, Error **errp)
> > +{
> > +    BlockBackend *blk = g_new0(BlockBackend, 1);
> > +
> > +    assert(name && name[0]);
> > +    if (blk_by_name(name)) {
> > +        error_setg(errp, "Device with id '%s' already exists", name);
> > +        return NULL;
> > +    }
> > +    blk->name = g_strdup(name);
> > +    blk->refcnt = 1;
> > +    QTAILQ_INSERT_TAIL(&blk_backends, blk, link);
> > +    return blk;
> > +}
> > +
> > +static void blk_delete(BlockBackend *blk)
> > +{
> > +    assert(!blk->refcnt);
> > +    QTAILQ_REMOVE(&blk_backends, blk, link);
> > +    g_free(blk->name);
> > +    g_free(blk);
> > +}
> > +
> > +/**
> > + * blk_ref:
> > + *
> > + * Increment @blk's reference count.
> > + */
> > +void blk_ref(BlockBackend *blk)
> > +{
> > +    blk->refcnt++;
> > +}
> > +
> > +/**
> > + * blk_unref:
> > + *
> > + * Decrement @blk's reference count.  If this drops it to zero,
> > + * destroy @blk.
> > + */
> > +void blk_unref(BlockBackend *blk)
> > +{
> > +    if (blk) {
> > +        g_assert(blk->refcnt > 0);
> > +        if (!--blk->refcnt) {
> > +            blk_delete(blk);
> > +        }
> > +    }
> > +}
> > +
> > +const char *blk_name(BlockBackend *blk)
> > +{
> > +    return blk->name;
> > +}
> > +
> > +BlockBackend *blk_by_name(const char *name)
> > +{
> > +    BlockBackend *blk;
> > +
> > +    QTAILQ_FOREACH(blk, &blk_backends, link) {
> > +        if (!strcmp(name, blk->name)) {
> > +            return blk;
> > +        }
> > +    }
> > +    return NULL;
> > +}
> > +
> > +/**
> > + * blk_next:
> > + *
> > + * Returns: the first BlockBackend if @blk is null, else @blk's next
> > + * sibling, which is %NULL for the last BlockBackend
> > + */
> > +BlockBackend *blk_next(BlockBackend *blk)
> > +{
> > +    return blk ? QTAILQ_NEXT(blk, link) : QTAILQ_FIRST(&blk_backends);
> > +}
> > diff --git a/blockdev.c b/blockdev.c
> > index 9fbd888..86596bc 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -30,6 +30,7 @@
> >   * THE SOFTWARE.
> >   */
> >  
> > +#include "sysemu/block-backend.h"
> >  #include "sysemu/blockdev.h"
> >  #include "hw/block/block.h"
> >  #include "block/blockjob.h"
> > @@ -221,6 +222,7 @@ void drive_del(DriveInfo *dinfo)
> >      }
> >  
> >      bdrv_unref(dinfo->bdrv);
> > +    blk_unref(blk_by_name(dinfo->id));
> >      g_free(dinfo->id);
> >      QTAILQ_REMOVE(&drives, dinfo, next);
> >      g_free(dinfo->serial);
> > @@ -301,6 +303,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
> >      int ro = 0;
> >      int bdrv_flags = 0;
> >      int on_read_error, on_write_error;
> > +    BlockBackend *blk;
> >      DriveInfo *dinfo;
> >      ThrottleConfig cfg;
> >      int snapshot = 0;
> > @@ -456,6 +459,10 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts,
> >      }
> >  
> >      /* init */
> > +    blk = blk_new(qemu_opts_id(opts), errp);
> > +    if (!blk) {
> > +        goto early_err;
> > +    }
> 
> Here you create a new block backend.
> And you don't attach it to anything in any way yet.
> 
> So down in the code the following test will leak it:
>     if (!file || !*file) {                                                      
>         if (has_driver_specific_opts) {                                         
>             file = NULL;                                                        
>         } else {                                                                
>             QDECREF(bs_opts);                                                   
>             qemu_opts_del(opts);                                                
>             return dinfo;                                                       
>         }                                                                       
>     } 
> 
> I am sure one of your next patchs fixes this but for this
> precise commit this do look like a leak.
> 
> >      dinfo = g_malloc0(sizeof(*dinfo));
> >      dinfo->id = g_strdup(qemu_opts_id(opts));
> >      dinfo->bdrv = bdrv_new_named(dinfo->id, &error);
> > @@ -525,6 +532,7 @@ err:
> >  bdrv_new_err:
> >      g_free(dinfo->id);
> >      g_free(dinfo);
> > +    blk_unref(blk);
> >  early_err:
> >      qemu_opts_del(opts);
> >  err_no_opts:
> > @@ -1770,7 +1778,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >       */
> >      if (bdrv_get_attached_dev(bs)) {
> >          bdrv_make_anon(bs);
> > -
> > +        blk_unref(blk_by_name(id));
> >          /* Further I/O must not pause the guest */
> >          bdrv_set_on_error(bs, BLOCKDEV_ON_ERROR_REPORT,
> >                            BLOCKDEV_ON_ERROR_REPORT);
> > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> > index 8bac7ff..730a021 100644
> > --- a/hw/block/xen_disk.c
> > +++ b/hw/block/xen_disk.c
> > @@ -39,6 +39,7 @@
> >  #include "hw/xen/xen_backend.h"
> >  #include "xen_blkif.h"
> >  #include "sysemu/blockdev.h"
> > +#include "sysemu/block-backend.h"
> >  
> >  /* ------------------------------------------------------------- */
> >  
> > @@ -852,12 +853,18 @@ static int blk_connect(struct XenDevice *xendev)
> >      blkdev->dinfo = drive_get(IF_XEN, 0, index);
> >      if (!blkdev->dinfo) {
> >          Error *local_err = NULL;
> > +        BlockBackend *blk;
> >          BlockDriver *drv;
> >  
> >          /* setup via xenbus -> create new block driver instance */
> >          xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
> > +        blk = blk_new(blkdev->dev, NULL);
> > +        if (!blk) {
> > +            return -1;
> > +        }
> >          blkdev->bs = bdrv_new_named(blkdev->dev, NULL);
> >          if (!blkdev->bs) {
> > +            blk_unref(blk);
> >              return -1;
> >          }
> >  
> > @@ -868,6 +875,7 @@ static int blk_connect(struct XenDevice *xendev)
> >                            error_get_pretty(local_err));
> >              error_free(local_err);
> >              bdrv_unref(blkdev->bs);
> > +            blk_unref(blk);
> >              blkdev->bs = NULL;
> >              return -1;
> >          }
> > @@ -983,6 +991,9 @@ static void blk_disconnect(struct XenDevice *xendev)
> >      if (blkdev->bs) {
> >          bdrv_detach_dev(blkdev->bs, blkdev);
> >          bdrv_unref(blkdev->bs);
> > +        if (!blkdev->dinfo) {
> > +            blk_unref(blk_by_name(blkdev->dev));
> > +        }
> >          blkdev->bs = NULL;
> >      }
> >      xen_be_unbind_evtchn(&blkdev->xendev);
> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> > index 5f20b0e..198da2e 100644
> > --- a/include/qemu/typedefs.h
> > +++ b/include/qemu/typedefs.h
> > @@ -35,6 +35,7 @@ typedef struct MachineClass MachineClass;
> >  typedef struct NICInfo NICInfo;
> >  typedef struct HCIInfo HCIInfo;
> >  typedef struct AudioState AudioState;
> > +typedef struct BlockBackend BlockBackend;
> >  typedef struct BlockDriverState BlockDriverState;
> >  typedef struct DriveInfo DriveInfo;
> >  typedef struct DisplayState DisplayState;
> > diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> > new file mode 100644
> > index 0000000..3f8371c
> > --- /dev/null
> > +++ b/include/sysemu/block-backend.h
> > @@ -0,0 +1,26 @@
> > +/*
> > + * QEMU Block backends
> > + *
> > + * Copyright (C) 2014 Red Hat, Inc.
> > + *
> > + * Authors:
> > + *  Markus Armbruster <armbru@redhat.com>,
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > + * later.  See the COPYING file in the top-level directory.
> > + */
> > +
> > +#ifndef BLOCK_BACKEND_H
> > +#define BLOCK_BACKEND_H
> > +
> > +#include "qemu/typedefs.h"
> > +#include "qapi/error.h"
> > +
> > +BlockBackend *blk_new(const char *name, Error **errp);
> > +void blk_ref(BlockBackend *blk);
> > +void blk_unref(BlockBackend *blk);
> > +const char *blk_name(BlockBackend *blk);
> > +BlockBackend *blk_by_name(const char *name);
> > +BlockBackend *blk_next(BlockBackend *blk);
> > +
> > +#endif
> > diff --git a/qemu-img.c b/qemu-img.c
> > index 4490a22..bad3f64 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -29,6 +29,7 @@
> >  #include "qemu/error-report.h"
> >  #include "qemu/osdep.h"
> >  #include "sysemu/sysemu.h"
> > +#include "sysemu/block-backend.h"
> >  #include "block/block_int.h"
> >  #include "block/qapi.h"
> >  #include <getopt.h>
> > @@ -575,6 +576,7 @@ static int img_check(int argc, char **argv)
> >      int c, ret;
> >      OutputFormat output_format = OFORMAT_HUMAN;
> >      const char *filename, *fmt, *output, *cache;
> > +    BlockBackend *blk;
> >      BlockDriverState *bs;
> >      int fix = 0;
> >      int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
> > @@ -649,6 +651,7 @@ static int img_check(int argc, char **argv)
> >          return 1;
> >      }
> >  
> 
> > +    blk = blk_new("image", &error_abort);
> Hmm we are so sure this will work that we don't do if (!block) ?

Ok I understood we are sure because we control the id and won't use twice the same.

> 
> >      bs = bdrv_new_open("image", filename, fmt, flags, true, quiet);
> >      if (!bs) {
> >          return 1;
> > @@ -710,6 +713,7 @@ static int img_check(int argc, char **argv)
> >  fail:
> >      qapi_free_ImageCheck(check);
> >      bdrv_unref(bs);
> > +    blk_unref(blk);
> >  
> >      return ret;
> >  }
> > @@ -718,6 +722,7 @@ static int img_commit(int argc, char **argv)
> >  {
> >      int c, ret, flags;
> >      const char *filename, *fmt, *cache;
> > +    BlockBackend *blk;
> >      BlockDriverState *bs;
> >      bool quiet = false;
> >  
> > @@ -756,6 +761,7 @@ static int img_commit(int argc, char **argv)
> >          return 1;
> >      }
> >  
> > +    blk = blk_new("image", &error_abort);
> >      bs = bdrv_new_open("image", filename, fmt, flags, true, quiet);
> >      if (!bs) {
> >          return 1;
> > @@ -780,6 +786,7 @@ static int img_commit(int argc, char **argv)
> >      }
> >  
> >      bdrv_unref(bs);
> > +    blk_unref(blk);
> >      if (ret) {
> >          return 1;
> >      }
> > @@ -942,6 +949,7 @@ static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num,
> >  static int img_compare(int argc, char **argv)
> >  {
> >      const char *fmt1 = NULL, *fmt2 = NULL, *cache, *filename1, *filename2;
> > +    BlockBackend *blk1, *blk2;
> >      BlockDriverState *bs1, *bs2;
> >      int64_t total_sectors1, total_sectors2;
> >      uint8_t *buf1 = NULL, *buf2 = NULL;
> > @@ -1011,6 +1019,7 @@ static int img_compare(int argc, char **argv)
> >          goto out3;
> >      }
> >  
> > +    blk1 = blk_new("image 1", &error_abort);
> >      bs1 = bdrv_new_open("image 1", filename1, fmt1, flags, true, quiet);
> >      if (!bs1) {
> >          error_report("Can't open file %s", filename1);
> > @@ -1018,6 +1027,7 @@ static int img_compare(int argc, char **argv)
> >          goto out3;
> >      }
> >  
> > +    blk2 = blk_new("image 2", &error_abort);
> >      bs2 = bdrv_new_open("image 2", filename2, fmt2, flags, true, quiet);
> >      if (!bs2) {
> >          error_report("Can't open file %s", filename2);
> > @@ -1184,10 +1194,12 @@ static int img_compare(int argc, char **argv)
> >  
> >  out:
> >      bdrv_unref(bs2);
> > +    blk_unref(blk2);
> >      qemu_vfree(buf1);
> >      qemu_vfree(buf2);
> >  out2:
> >      bdrv_unref(bs1);
> > +    blk_unref(blk1);
> >  out3:
> >      qemu_progress_end();
> >      return ret;
> > @@ -1200,6 +1212,7 @@ static int img_convert(int argc, char **argv)
> >      int progress = 0, flags, src_flags;
> >      const char *fmt, *out_fmt, *cache, *src_cache, *out_baseimg, *out_filename;
> >      BlockDriver *drv, *proto_drv;
> > +    BlockBackend **blk = NULL, *out_blk = NULL;
> >      BlockDriverState **bs = NULL, *out_bs = NULL;
> >      int64_t total_sectors, nb_sectors, sector_num, bs_offset;
> >      int64_t *bs_sectors = NULL;
> > @@ -1354,6 +1367,7 @@ static int img_convert(int argc, char **argv)
> >  
> >      qemu_progress_print(0, 100);
> >  
> > +    blk = g_new0(BlockBackend *, bs_n);
> >      bs = g_new0(BlockDriverState *, bs_n);
> >      bs_sectors = g_new(int64_t, bs_n);
> >  
> > @@ -1361,6 +1375,7 @@ static int img_convert(int argc, char **argv)
> >      for (bs_i = 0; bs_i < bs_n; bs_i++) {
> >          char *id = bs_n > 1 ? g_strdup_printf("source %d", bs_i)
> >                              : g_strdup("source");
> > +        blk[bs_i] = blk_new(id, &error_abort);
> >          bs[bs_i] = bdrv_new_open(id, argv[optind + bs_i], fmt, src_flags,
> >                                   true, quiet);
> >          g_free(id);
> > @@ -1486,6 +1501,7 @@ static int img_convert(int argc, char **argv)
> >          goto out;
> >      }
> >  
> > +    out_blk = blk_new("target", &error_abort);
> >      out_bs = bdrv_new_open("target", out_filename, out_fmt, flags, true, quiet);
> >      if (!out_bs) {
> >          ret = -1;
> > @@ -1742,6 +1758,7 @@ out:
> >      if (out_bs) {
> >          bdrv_unref(out_bs);
> >      }
> > +    blk_unref(out_blk);
> >      if (bs) {
> >          for (bs_i = 0; bs_i < bs_n; bs_i++) {
> >              if (bs[bs_i]) {
> > @@ -1750,6 +1767,12 @@ out:
> >          }
> >          g_free(bs);
> >      }
> > +    if (blk) {
> > +        for (bs_i = 0; bs_i < bs_n; bs_i++) {
> > +            blk_unref(blk[bs_i]);
> > +        }
> > +        g_free(blk);
> > +    }
> >      g_free(bs_sectors);
> >  fail_getopt:
> >      g_free(options);
> > @@ -1858,6 +1881,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
> >      filenames = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL);
> >  
> >      while (filename) {
> > +        BlockBackend *blk;
> >          BlockDriverState *bs;
> >          ImageInfo *info;
> >          ImageInfoList *elem;
> > @@ -1869,6 +1893,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
> >          }
> >          g_hash_table_insert(filenames, (gpointer)filename, NULL);
> >  
> > +        blk = blk_new("image", &error_abort);
> >          bs = bdrv_new_open("image", filename, fmt,
> >                             BDRV_O_FLAGS | BDRV_O_NO_BACKING, false, false);
> >          if (!bs) {
> 
> I think it misses an 
> > +            blk_unref(blk);
> in if(!bs) branch.
> 
> > @@ -1880,6 +1905,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
> >              error_report("%s", error_get_pretty(err));
> >              error_free(err);
> >              bdrv_unref(bs);
> > +            blk_unref(blk);
> >              goto err;
> >          }
> >  
> > @@ -1889,6 +1915,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
> >          last = &elem->next;
> >  
> >          bdrv_unref(bs);
> > +        blk_unref(blk);
> >  
> >          filename = fmt = NULL;
> >          if (chain) {
> > @@ -2082,6 +2109,7 @@ static int img_map(int argc, char **argv)
> >  {
> >      int c;
> >      OutputFormat output_format = OFORMAT_HUMAN;
> > +    BlockBackend *blk;
> >      BlockDriverState *bs;
> >      const char *filename, *fmt, *output;
> >      int64_t length;
> > @@ -2130,6 +2158,7 @@ static int img_map(int argc, char **argv)
> >          return 1;
> >      }
> >  
> > +    blk = blk_new("image", &error_abort);
> >      bs = bdrv_new_open("image", filename, fmt, BDRV_O_FLAGS, true, false);
> >      if (!bs) {
> >          return 1;
> > @@ -2175,6 +2204,7 @@ static int img_map(int argc, char **argv)
> >  
> >  out:
> >      bdrv_unref(bs);
> > +    blk_unref(blk);
> >      return ret < 0;
> >  }
> >  
> > @@ -2185,6 +2215,7 @@ out:
> >  
> >  static int img_snapshot(int argc, char **argv)
> >  {
> > +    BlockBackend *blk;
> >      BlockDriverState *bs;
> >      QEMUSnapshotInfo sn;
> >      char *filename, *snapshot_name = NULL;
> > @@ -2250,6 +2281,7 @@ static int img_snapshot(int argc, char **argv)
> >      filename = argv[optind++];
> >  
> >      /* Open the image */
> > +    blk = blk_new("image", &error_abort);
> >      bs = bdrv_new_open("image", filename, NULL, bdrv_oflags, true, quiet);
> >      if (!bs) {
> >          return 1;
> > @@ -2297,6 +2329,7 @@ static int img_snapshot(int argc, char **argv)
> >  
> >      /* Cleanup */
> >      bdrv_unref(bs);
> > +    blk_unref(blk);
> >      if (ret) {
> >          return 1;
> >      }
> > @@ -2305,6 +2338,7 @@ static int img_snapshot(int argc, char **argv)
> >  
> >  static int img_rebase(int argc, char **argv)
> >  {
> > +    BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL;
> >      BlockDriverState *bs = NULL, *bs_old_backing = NULL, *bs_new_backing = NULL;
> >      BlockDriver *old_backing_drv, *new_backing_drv;
> >      char *filename;
> > @@ -2393,6 +2427,7 @@ static int img_rebase(int argc, char **argv)
> >       * Ignore the old backing file for unsafe rebase in case we want to correct
> >       * the reference to a renamed or moved backing file.
> >       */
> > +    blk = blk_new("image", &error_abort);
> >      bs = bdrv_new_open("image", filename, fmt, flags, true, quiet);
> >      if (!bs) {
> >          ret = -1;
> > @@ -2425,6 +2460,7 @@ static int img_rebase(int argc, char **argv)
> >      if (!unsafe) {
> >          char backing_name[1024];
> >  
> > +        blk_old_backing = blk_new("old_backing", &error_abort);
> >          bs_old_backing = bdrv_new_named("old_backing", &error_abort);
> >          bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
> >          ret = bdrv_open(&bs_old_backing, backing_name, NULL, NULL, src_flags,
> > @@ -2436,6 +2472,7 @@ static int img_rebase(int argc, char **argv)
> >              goto out;
> >          }
> >          if (out_baseimg[0]) {
> > +            blk_new_backing = blk_new("new_backing", &error_abort);
> >              bs_new_backing = bdrv_new_named("new_backing", &error_abort);
> >              ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL, src_flags,
> >                              new_backing_drv, &local_err);
> > @@ -2614,12 +2651,15 @@ out:
> >          if (bs_old_backing != NULL) {
> >              bdrv_unref(bs_old_backing);
> >          }
> > +        blk_unref(blk_old_backing);
> >          if (bs_new_backing != NULL) {
> >              bdrv_unref(bs_new_backing);
> >          }
> > +        blk_unref(blk_new_backing);
> >      }
> >  
> >      bdrv_unref(bs);
> > +    blk_unref(blk);
> >      if (ret) {
> >          return 1;
> >      }
> > @@ -2632,6 +2672,7 @@ static int img_resize(int argc, char **argv)
> >      const char *filename, *fmt, *size;
> >      int64_t n, total_size;
> >      bool quiet = false;
> > +    BlockBackend *blk = NULL;
> >      BlockDriverState *bs = NULL;
> >      QemuOpts *param;
> >      static QemuOptsList resize_options = {
> > @@ -2708,6 +2749,7 @@ static int img_resize(int argc, char **argv)
> >      n = qemu_opt_get_size(param, BLOCK_OPT_SIZE, 0);
> >      qemu_opts_del(param);
> >  
> > +    blk = blk_new("image", &error_abort);
> >      bs = bdrv_new_open("image", filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR,
> >                         true, quiet);
> >      if (!bs) {
> > @@ -2745,6 +2787,7 @@ out:
> >      if (bs) {
> >          bdrv_unref(bs);
> >      }
> > +    blk_unref(blk);
> >      if (ret) {
> >          return 1;
> >      }
> > @@ -2760,6 +2803,7 @@ static int img_amend(int argc, char **argv)
> >      const char *fmt = NULL, *filename, *cache;
> >      int flags;
> >      bool quiet = false;
> > +    BlockBackend *blk = NULL;
> >      BlockDriverState *bs = NULL;
> >  
> >      cache = BDRV_DEFAULT_CACHE;
> > @@ -2823,6 +2867,7 @@ static int img_amend(int argc, char **argv)
> >          goto out;
> >      }
> >  
> > +    blk = blk_new("image", &error_abort);
> >      bs = bdrv_new_open("image", filename, fmt, flags, true, quiet);
> >      if (!bs) {
> >          error_report("Could not open image '%s'", filename);
> > @@ -2856,6 +2901,7 @@ out:
> >      if (bs) {
> >          bdrv_unref(bs);
> >      }
> > +    blk_unref(blk);
> >      qemu_opts_del(opts);
> >      qemu_opts_free(create_opts);
> >      g_free(options);
> > diff --git a/qemu-io.c b/qemu-io.c
> > index 44c2e1c..45e5494 100644
> > --- a/qemu-io.c
> > +++ b/qemu-io.c
> > @@ -19,6 +19,7 @@
> >  #include "qemu/option.h"
> >  #include "qemu/config-file.h"
> >  #include "qemu/readline.h"
> > +#include "sysemu/block-backend.h"
> >  #include "block/block_int.h"
> >  #include "trace/control.h"
> >  
> > @@ -26,6 +27,7 @@
> >  
> >  static char *progname;
> >  
> > +static BlockBackend *qemuio_blk;
> >  static BlockDriverState *qemuio_bs;
> >  
> >  /* qemu-io commands passed using -c */
> > @@ -37,7 +39,9 @@ static ReadLineState *readline_state;
> >  static int close_f(BlockDriverState *bs, int argc, char **argv)
> >  {
> >      bdrv_unref(bs);
> > +    blk_unref(qemuio_blk);
> >      qemuio_bs = NULL;
> > +    qemuio_blk = NULL;
> >      return 0;
> >  }
> >  
> > @@ -58,6 +62,7 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
> >          return 1;
> >      }
> >  
> > +    qemuio_blk = blk_new("hda", &error_abort);
> 
> >      qemuio_bs = bdrv_new_named("hda", &error_abort);
> I see accepting that an allocation _will_ work is the qemu tools style.
> 
> >  
> >      if (growable) {
> > @@ -70,7 +75,9 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
> >                  error_get_pretty(local_err));
> >          error_free(local_err);
> >          bdrv_unref(qemuio_bs);
> > +        blk_unref(qemuio_blk);
> >          qemuio_bs = NULL;
> > +        qemuio_blk = NULL;
> >          return 1;
> >      }
> >  
> > @@ -479,6 +486,7 @@ int main(int argc, char **argv)
> >      if (qemuio_bs) {
> >          bdrv_unref(qemuio_bs);
> >      }
> > +    blk_unref(qemuio_blk);
> >      g_free(readline_state);
> >      return 0;
> >  }
> > diff --git a/qemu-nbd.c b/qemu-nbd.c
> > index a56ebfc..94b9b49 100644
> > --- a/qemu-nbd.c
> > +++ b/qemu-nbd.c
> > @@ -17,7 +17,7 @@
> >   */
> >  
> >  #include "qemu-common.h"
> > -#include "block/block.h"
> > +#include "sysemu/block-backend.h"
> >  #include "block/block_int.h"
> >  #include "block/nbd.h"
> >  #include "qemu/main-loop.h"
> > @@ -687,6 +687,7 @@ int main(int argc, char **argv)
> >          drv = NULL;
> >      }
> >  
> > +    blk_new("hda", &error_abort);
> >      bs = bdrv_new_named("hda", &error_abort);
> >  
> >      srcpath = argv[optind];
> > -- 
> > 1.9.3
> > 
> >