[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Max Reitz
Subject: Re: [Qemu-block] [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.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]