[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 12/15] blockdev: Keep track of monitor-owned
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v6 12/15] blockdev: Keep track of monitor-owned BDS |
Date: |
Mon, 9 Nov 2015 17:28:18 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 09.11.2015 16:05, Alberto Garcia wrote:
> On Wed 04 Nov 2015 07:57:44 PM CET, Max Reitz wrote:
>> @@ -3519,11 +3537,18 @@ void qmp_x_blockdev_del(bool has_id, const char *id,
>> bdrv_get_device_or_node_name(bs));
>> goto out;
>> }
>> +
>> + if (!blk && !bs->monitor_list.tqe_prev) {
>> + error_setg(errp, "Node %s is not owned by the monitor",
>> + bs->node_name);
>> + goto out;
>> + }
>> }
>>
>> if (blk) {
>> blk_unref(blk);
>> } else {
>> + QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list);
>> bdrv_unref(bs);
>> }
>
> blk_unref(blk) will also unref the BDS (if there's any), so you also
> need to update monitor_bdrv_states in that case, don't you?
If we get to blk_unref(blk), that means there is a BB attached to that
BDS (if there is any). If that BDS is monitor-owned, that means its
refcount is at least 2 (one from the BB, one from the monitor).
Therefore, blockdev-del will fail before that point.
> Anyway, wouldn't it make more sense to do this in bdrv_delete() ?
No, I don't think so. The monitor_bdrv_states is not a plain list of
BDSs, but actually contains strong references. Every time a BDS is
entered there, that reference should be counted (which we don't actually
have to call bdrv_ref() for since we're just keeping the initial
refcount of 1 in qmp_blockdev_add()), and every time a BDS is removed,
its reference should be dropped, just like is done here.
That's the issue I had with this implementation of blockdev-del. It's
just dropping references without actually having those references. Now
we at least have the reference through monitor_bdrv_states, and in the
future, we'll have such a list of monitor references for BBs, too.
Max
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [PATCH v6 07/15] block: Move BDS close notifiers into BB, (continued)
- [Qemu-block] [PATCH v6 08/15] block: Use blk_remove_bs() in blk_delete(), Max Reitz, 2015/11/04
- [Qemu-block] [PATCH v6 09/15] blockdev: Use blk_remove_bs() in do_drive_del(), Max Reitz, 2015/11/04
- [Qemu-block] [PATCH v6 10/15] block: Make bdrv_close() static, Max Reitz, 2015/11/04
- [Qemu-block] [PATCH v6 11/15] block: Add list of all BlockDriverStates, Max Reitz, 2015/11/04
- [Qemu-block] [PATCH v6 12/15] blockdev: Keep track of monitor-owned BDS, Max Reitz, 2015/11/04
- [Qemu-block] [PATCH v6 13/15] block: Add blk_remove_all_bs(), Max Reitz, 2015/11/04
- [Qemu-block] [PATCH v6 14/15] block: Rewrite bdrv_close_all(), Max Reitz, 2015/11/04
[Qemu-block] [PATCH v6 15/15] iotests: Add test for multiple BB on BDS tree, Max Reitz, 2015/11/04
Re: [Qemu-block] [PATCH v6 for-2.6 00/15] block: Rework bdrv_close_all(), Kevin Wolf, 2015/11/09