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 <[email protected]> > --- > 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
