Max Reitz <mre...@redhat.com> writes:
> 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 <arm...@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.
Fair request. I could squash the following into PATCH 18:
diff --git a/block/block-backend.c b/block/block-backend.c
index 5646628..5e4ecd7 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -22,7 +22,7 @@ struct BlockBackend {
char *name;
int refcnt;
BlockDriverState *bs;
- DriveInfo *legacy_dinfo;
+ DriveInfo *legacy_dinfo; /* null unless created by drive_new() */
QTAILQ_ENTRY(BlockBackend) link; /* for blk_backends */
void *dev; /* attached device model, if any */
>> 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?
PATCH 18 is broken. See also my reply to Fam on v2 of this patch.
> However, for this patch:
>
> Reviewed-by: Max Reitz <mre...@redhat.com>
Thanks!
