[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/2] block: Allow x-blockdev-del on a BB with
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/2] block: Allow x-blockdev-del on a BB with a monitor-owned BDS |
Date: |
Wed, 24 Feb 2016 17:15:11 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 09.02.2016 16:57, Alberto Garcia wrote:
> When x-blockdev-del is performed on a BlockBackend that has inserted
> media it will only succeed if the BDS doesn't have any additional
> references.
>
> The only problem with this is that if the BDS was created separately
> using blockdev-add then the backend won't be able to be destroyed
> unless the BDS is ejected first. This is an unnecessary restriction.
Is it? In order to get into this situation, you need to execute:
blockdev-add (BB/BDS), blockdev-add (BDS/BB), x-blockdev-insert-medium
Now, in order to unravel it, you currently need:
x-blockdev-remove-medium (or eject), x-blockdev-del (BB/BDS),
x-blockdev-del (BDS/BB)
So you need to execute the x-blockdev-remove-medium because you did an
x-blockdev-insert-medium before. That seems reasonable to me, and not
very superfluous.
In fact, the behavior allowed by this patch appears a bit inconsistent
to me. Why is it OK for x-blockdev-del to automatically eject a BDS from
a BB if the BB is deleted, but not if the BDS is deleted? (which is what
your modifications to test 139 verify)
> Now that we have a list of monitor-owned BDSs we can allow
> x-blockdev-del to work in this scenario if the BDS has exactly one
> extra reference and that reference is from the monitor.
>
> This patch also updates iotest 139 to reflect this change. Both
> testAttachMedia() and testSnapshot() are split in two: one version
> keeps the previous behavior, and a second version checks that the new
> functionality works as expected.
>
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
> blockdev.c | 13 ++++++++++---
> tests/qemu-iotests/139 | 30 +++++++++++++++++++++++++-----
> tests/qemu-iotests/139.out | 4 ++--
> 3 files changed, 37 insertions(+), 10 deletions(-)
The patch itself looks fine to me, but I'm not so sure about the idea
behind it.
Max
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2 0/2] Allow x-blockdev-del on a BB with a monitor-owned BDS, Alberto Garcia, 2016/02/24