Re: [Qemu-block] [PATCH 14/27] gluster: Support .bdrv_co_create

[Max Reitz]

On 2018-02-08 20:23, Kevin Wolf wrote:
> This adds the .bdrv_co_create driver callback to gluster, which enables
> image creation over QMP.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  qapi/block-core.json |  18 ++++++-
>  block/gluster.c      | 149 
> +++++++++++++++++++++++++++++++++------------------
>  2 files changed, 115 insertions(+), 52 deletions(-)

[...]

> diff --git a/block/gluster.c b/block/gluster.c
> index 0f4265a3a4..b7e2b7fa2b 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c

[...]

> @@ -962,64 +976,33 @@ static coroutine_fn int 
> qemu_gluster_co_pwrite_zeroes(BlockDriverState *bs,
>  }
>  #endif
>  
> -static int qemu_gluster_create(const char *filename,
> -                               QemuOpts *opts, Error **errp)
> +static int qemu_gluster_co_create(BlockdevCreateOptions *options,
> +                                  Error **errp)
>  {
> -    BlockdevOptionsGluster *gconf;
> +    BlockdevCreateOptionsGluster *opts = &options->u.gluster;

In the other drivers so far you have asserted first that options->driver
is as expected and then retrieved the appropriate part of the union.  I
liked that a bit better, although of course it doesn't matter functionally.

Anyway:

Reviewed-by: Max Reitz <address@hidden>

>      struct glfs *glfs;
>      struct glfs_fd *fd;
>      int ret = 0;
> -    PreallocMode prealloc;
> -    int64_t total_size = 0;
> -    char *tmp = NULL;
> -    Error *local_err = NULL;
> -
> -    gconf = g_new0(BlockdevOptionsGluster, 1);
> -    gconf->debug = qemu_opt_get_number_del(opts, GLUSTER_OPT_DEBUG,
> -                                           GLUSTER_DEBUG_DEFAULT);
> -    if (gconf->debug < 0) {
> -        gconf->debug = 0;
> -    } else if (gconf->debug > GLUSTER_DEBUG_MAX) {
> -        gconf->debug = GLUSTER_DEBUG_MAX;
> -    }
> -    gconf->has_debug = true;
>  
> -    gconf->logfile = qemu_opt_get_del(opts, GLUSTER_OPT_LOGFILE);
> -    if (!gconf->logfile) {
> -        gconf->logfile = g_strdup(GLUSTER_LOGFILE_DEFAULT);
> -    }
> -    gconf->has_logfile = true;
> +    assert(options->driver == BLOCKDEV_DRIVER_GLUSTER);
>

signature.asc
Description: OpenPGP digital signature