Re: [Qemu-devel] [PATCH v3 04/23] block: Connect BlockBackend and DriveInfo

[Max Reitz]

On 16.09.2014 20:12, Markus Armbruster wrote:
> Make the BlockBackend own the DriveInfo.  Change blockdev_init() to
> return the BlockBackend instead of the DriveInfo.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>   block.c                   |  2 --
>   block/block-backend.c     | 38 ++++++++++++++++++++++++
>   blockdev.c                | 73 ++++++++++++++++++++++++-----------------------
>   include/sysemu/blockdev.h |  4 +++
>   4 files changed, 79 insertions(+), 38 deletions(-)

[snip]

> diff --git a/blockdev.c b/blockdev.c
> index 5da6028..722d083 100644
> --- a/blockdev.c
> +++ b/blockdev.c

[snip]

> @@ -920,19 +922,18 @@ DriveInfo *drive_new(QemuOpts *all_opts, 
> BlockInterfaceType block_default_type)
>       }
>   
>       /* Actual block device init: Functionality shared with blockdev-add */
> -    dinfo = blockdev_init(filename, bs_opts, &local_err);
> +    blk = blockdev_init(filename, bs_opts, &local_err);
>       bs_opts = NULL;
> -    if (dinfo == NULL) {
> -        if (local_err) {
> -            error_report("%s", error_get_pretty(local_err));
> -            error_free(local_err);
> -        }
> +    if (!blk) {
> +        error_report("%s", error_get_pretty(local_err));
> +        error_free(local_err);
>           goto fail;

Not all of blockdev_init() sets errp on failure. Try "qemu-system-x86_64 
-drive format=help" and you'll see a segfault with this patch applied. 
So either you can make blockdev_init() do things right, which is, set 
errp for format=help instead of dumping the list to stdout (but I'm not 
even sure whether that's really correct, because it's not really an 
error), or you keep the "if" around error_report() and error_free() here.

Looks good otherwise, though.

Max