Re: [Qemu-devel] [PATCH v2 1/2] block: Allow x-blockdev-del on a BB with a monitor-owned BDS

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]

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 <berto@igalia.com>
> ---
>  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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]