[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 19/23] blockdev: Drop DriveInfo member enable
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 19/23] blockdev: Drop DriveInfo member enable_auto_del |
Date: |
Mon, 15 Sep 2014 10:40:56 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Fam Zheng <address@hidden> writes:
> On Sat, 09/13 17:00, 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 <address@hidden>
>> ---
>> blockdev.c | 11 +++--------
>> include/sysemu/blockdev.h | 1 -
>> 2 files changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 0d99be0..e218c59 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -90,16 +90,14 @@ void blockdev_mark_auto_del(BlockBackend *blk)
>> DriveInfo *dinfo = blk_legacy_dinfo(blk);
>> BlockDriverState *bs = blk_bs(blk);
>>
>> - if (dinfo && !dinfo->enable_auto_del) {
>> + if (!dinfo) {
>> return;
>> }
>>
>> if (bs->job) {
>> block_job_cancel(bs->job);
>> }
>> - if (dinfo) {
>> - dinfo->auto_del = 1;
>> - }
>> + dinfo->auto_del = 1;
>> }
>
> Looks good.
>
> Based on the explanation in the commit message of patch 18, previouly,
> dinfo is always non-NULL, so enable_auto_del distinguishes whether the
> device is created by drive_new() or qmp_blockdev_add().
Correct.
> Now dinfo is NULL iff created by qmp_blockdev_add(), so
> enable_auto_del is not necessary any more.
Correct again.
> If I'm reading correctly, the dropped two "if (dinfo.."'s are also redundant.
Yes. Let me explain why.
The two cases to distinguish are "created by drive_new()" and "created
by qmp_blockdev_add()".
Commit 2d246f0 introduced DriveInfo member enable_auto_del to exactly
that.
The code tests for "created by qmp_blockdev_add()" like this:
dinfo && !dinfo->enable_auto_del
Before PATCH 18, this was actually unnecessarily careful, because dinfo
could never be null, but that's okay.
PATCH 18 changes the "created by qmp_blockdev_add() case: now dinfo is
null there. The test remains correct, only now its second rather than
first part is unnecessary: dinfo now implies dinfo->enable_auto_del.
Before PATCH 18 dinfo != NULL !dinfo->enable_auto_del &&
by drive_new() true false false
by qmp_blockdev_add() true true true
After PATCH 18 dinfo != NULL !dinfo->enable_auto_del &&
by drive_new() true false false
by qmp_blockdev_add() false N/A false
Oops.
After PATCH 19 dinfo !dinfo
by drive_new() true false
by qmp_blockdev_add() false true
Looks like I need to squash PATCH 19 into PATCH 18... Thanks for making
me think!
[...]
- Re: [Qemu-devel] [PATCH v2 05/23] block: Code motion to get rid of stubs/blockdev.c, (continued)
[Qemu-devel] [PATCH v2 16/23] pc87312: Drop unused members of PC87312State, Markus Armbruster, 2014/09/13
[Qemu-devel] [PATCH v2 15/23] ide: Complete conversion from BlockDriverState to BlockBackend, Markus Armbruster, 2014/09/13
[Qemu-devel] [PATCH v2 17/23] blockdev: Drop superfluous DriveInfo member id, Markus Armbruster, 2014/09/13
[Qemu-devel] [PATCH v2 21/23] blockdev: Convert qmp_eject(), qmp_change_blockdev() to BlockBackend, Markus Armbruster, 2014/09/13
[Qemu-devel] [PATCH v2 20/23] block/qapi: Convert qmp_query_block() to BlockBackend, Markus Armbruster, 2014/09/13
[Qemu-devel] [PATCH v2 23/23] block: Make device model's references to BlockBackend strong, Markus Armbruster, 2014/09/13
[Qemu-devel] [PATCH v2 19/23] blockdev: Drop DriveInfo member enable_auto_del, Markus Armbruster, 2014/09/13
[Qemu-devel] [PATCH v2 11/23] block: Rename BlockDriverAIOCB* to BlockAIOCB*, Markus Armbruster, 2014/09/13
[Qemu-devel] [PATCH v2 18/23] blockdev: Fix blockdev-add not to create IDE drive (0, 0), Markus Armbruster, 2014/09/13
[Qemu-devel] [PATCH v2 22/23] block: Lift device model API into BlockBackend, Markus Armbruster, 2014/09/13
[Qemu-devel] [PATCH v2 14/23] hw: Convert from BlockDriverState to BlockBackend, mostly, Markus Armbruster, 2014/09/13
Re: [Qemu-devel] [PATCH v2 00/23] Split BlockBackend off BDS with an axe, Markus Armbruster, 2014/09/16