Re: [Qemu-devel] [PATCH v3 19/23] blockdev: Drop DriveInfo member enable_auto_del

[Max Reitz <mreitz@redhat.com>]

On 16.09.2014 20:12, Markus Armbruster wrote:
> Commit 2d246f0 introduced DriveInfo member enable_auto_del to
> distinguish DriveInfo created via drive_new() from DriveInfo created
> via qmp_blockdev_add().  The latter no longer exist.  Drop
> enable_auto_del.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   blockdev.c                | 11 +++--------
>   include/sysemu/blockdev.h |  1 -
>   2 files changed, 3 insertions(+), 9 deletions(-)

I would've liked some comment somewhere about DriveInfo's presence 
corresponding with the drive having been created through drive_new(), 
but I can live without, too.

> diff --git a/blockdev.c b/blockdev.c
> index 0d99be0..e218c59 100644
> --- a/blockdev.c
> +++ b/blockdev.c

[snip]

> @@ -1727,8 +1723,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
>       }
>       bs = blk_bs(blk);
>   
> -    dinfo = blk_legacy_dinfo(blk);
> -    if (dinfo && !dinfo->enable_auto_del) {
> +    if (!blk_legacy_dinfo(blk)) {
>           error_report("Deleting device added with blockdev-add"
>                        " is not supported");
>           return -1;

This doesn't look like a 1-to-1 correspondence. Before this patch, if 
DriveInfo was not present, the condition was false (actually, it was 
always false, which is the reason for this patch). Now it's true. It 
seems like the behavior is now correct but wasn't before... I guess this 
means patch 18 should be fixed?

However, for this patch:

Reviewed-by: Max Reitz <mreitz@redhat.com>