Re: [Qemu-devel] [PATCH 1/3] blockdev: Rename drive_init(), drive_uninit() to drive_new(), drive_del()

[Benoît Canet]

The Friday 06 Jun 2014 à 14:50:58 (+0200), Markus Armbruster wrote :
> "Init" and "uninit" suggest the functions don't allocate / free
> storage.  But they do.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  blockdev.c                | 14 +++++++-------
>  device-hotplug.c          |  2 +-
>  hw/usb/dev-storage.c      |  2 +-
>  include/sysemu/blockdev.h |  4 ++--
>  vl.c                      |  4 ++--
>  5 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 9b5261b..1256013 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -213,7 +213,7 @@ static void bdrv_format_print(void *opaque, const char 
> *name)
>      error_printf(" %s", name);
>  }
>  
> -static void drive_uninit(DriveInfo *dinfo)
> +static void drive_del(DriveInfo *dinfo)
>  {
>      if (dinfo->opts) {
>          qemu_opts_del(dinfo->opts);
> @@ -230,7 +230,7 @@ void drive_put_ref(DriveInfo *dinfo)
>  {
>      assert(dinfo->refcount);
>      if (--dinfo->refcount == 0) {
> -        drive_uninit(dinfo);
> +        drive_del(dinfo);
>      }
>  }
>  
> @@ -658,7 +658,7 @@ QemuOptsList qemu_legacy_drive_opts = {
>      },
>  };
>  
> -DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType 
> block_default_type)
> +DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType 
> block_default_type)
>  {
>      const char *value;
>      DriveInfo *dinfo = NULL;
> @@ -1791,7 +1791,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, 
> QObject **ret_data)
>          bdrv_set_on_error(bs, BLOCKDEV_ON_ERROR_REPORT,
>                            BLOCKDEV_ON_ERROR_REPORT);
>      } else {
> -        drive_uninit(drive_get_by_blockdev(bs));
> +        drive_del(drive_get_by_blockdev(bs));
>      }
>  
>      return 0;
> @@ -2334,9 +2334,9 @@ void qmp_blockdev_add(BlockdevOptions *options, Error 
> **errp)
>          goto fail;
>      }
>  
> -    /* TODO Sort it out in raw-posix and drive_init: Reject aio=native with
> +    /* TODO Sort it out in raw-posix and drive_new(): Reject aio=native with
>       * cache.direct=false instead of silently switching to aio=threads, 
> except
> -     * if called from drive_init.
> +     * when called from drive_new().
>       *
>       * For now, simply forbidding the combination for all drivers will do. */
>      if (options->has_aio && options->aio == BLOCKDEV_AIO_OPTIONS_NATIVE) {
> @@ -2368,7 +2368,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error 
> **errp)
>      }
>  
>      if (bdrv_key_required(dinfo->bdrv)) {
> -        drive_uninit(dinfo);
> +        drive_del(dinfo);
>          error_setg(errp, "blockdev-add doesn't support encrypted devices");
>          goto fail;
>      }
> diff --git a/device-hotplug.c b/device-hotplug.c
> index eecb08e..fc09d10 100644
> --- a/device-hotplug.c
> +++ b/device-hotplug.c
> @@ -40,7 +40,7 @@ DriveInfo *add_init_drive(const char *optstr)
>          return NULL;
>  
>      mc = MACHINE_GET_CLASS(current_machine);
> -    dinfo = drive_init(opts, mc->block_default_type);
> +    dinfo = drive_new(opts, mc->block_default_type);
>      if (!dinfo) {
>          qemu_opts_del(opts);
>          return NULL;
> diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
> index e919100..ae4efcb 100644
> --- a/hw/usb/dev-storage.c
> +++ b/hw/usb/dev-storage.c
> @@ -691,7 +691,7 @@ static USBDevice *usb_msd_init(USBBus *bus, const char 
> *filename)
>      qemu_opt_set(opts, "if", "none");
>  
>      /* create host drive */
> -    dinfo = drive_init(opts, 0);
> +    dinfo = drive_new(opts, 0);
>      if (!dinfo) {
>          qemu_opts_del(opts);
>          return NULL;
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 134712b..0fdbd68 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -37,7 +37,7 @@ struct DriveInfo {
>      int bus;
>      int unit;
>      int auto_del;               /* see blockdev_mark_auto_del() */
> -    bool enable_auto_del; /* Only for legacy drive_init() */
> +    bool enable_auto_del;       /* Only for legacy drive_new() */
>      int media_cd;
>      int cyls, heads, secs, trans;
>      QemuOpts *opts;
> @@ -57,7 +57,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
>  QemuOpts *drive_def(const char *optstr);
>  QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
>                      const char *optstr);
> -DriveInfo *drive_init(QemuOpts *arg, BlockInterfaceType block_default_type);
> +DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
>  
>  /* device-hotplug */
>  
> diff --git a/vl.c b/vl.c
gG> index a3def97..f4b4841 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1074,7 +1074,7 @@ static int drive_init_func(QemuOpts *opts, void *opaque)
>  {
>      BlockInterfaceType *block_default_type = opaque;
>  
> -    return drive_init(opts, *block_default_type) == NULL;
> +    return drive_new(opts, *block_default_type) == NULL;
>  }
>  
>  static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
> @@ -1098,7 +1098,7 @@ static void default_drive(int enable, int snapshot, 
> BlockInterfaceType type,
>      if (snapshot) {
>          drive_enable_snapshot(opts, NULL);
>      }
> -    if (!drive_init(opts, type)) {
> +    if (!drive_new(opts, type)) {

Unrelated but it seems QEMU is leaking this one on exit.

>          exit(1);
>      }
>  }
> -- 
> 1.9.3
> 
> 

Reviewed-by: Benoit Canet <address@hidden>