[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] block: Allow x-blockdev-del on a BB with a
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] block: Allow x-blockdev-del on a BB with a monitor-owned BDS |
Date: |
Tue, 09 Feb 2016 16:39:50 +0100 |
User-agent: |
Notmuch/0.13.2 (http://notmuchmail.org) Emacs/23.2.1 (i486-pc-linux-gnu) |
On Tue 09 Feb 2016 04:34:09 PM CET, Eric Blake wrote:
> On 02/08/2016 07:14 AM, Alberto Garcia wrote:
>
> When sending a multi-patch series, you should always include a 0/3 cover
> letter. The cover letter is optional only for a lone patch.
Sorry, I didn't know. The description of the first patch already
contains everything that would go into the cover letter, that's why I
decided to do it like this. I'll include the cover letter in the future.
>> if (bs->refcnt > 1) {
>> - error_setg(errp, "Block device %s is in use",
>> - bdrv_get_device_or_node_name(bs));
>> - goto out;
>> + /* We allow deleting a BlockBackend that has a BDS with an
>> + * extra reference if that extra reference is from the
>> + * monitor. */
>> + bool bs_has_only_monitor_ref =
>> + blk && bs->monitor_list.tqe_prev && bs->refcnt == 2;
>> + if (!bs_has_only_monitor_ref) {
>
> I don't think the temporary bool or nested 'if' are necessary; but at
> the same time, I don't think the following is any more legible:
>
> /* Prohibit deleting a BlockBackend whose BDS is in use by any more than
> a single monitor */
> if (bs->refcnt > 1 + (blk && bs->monitor_list.tqe_prev)) {
> error_setg(...
Exactly, I considered several options and I thought the one I finally
chose would be the easiest to read.
Thanks!
Berto