From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35600) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aYc5x-0004So-00 for qemu-devel@nongnu.org; Wed, 24 Feb 2016 11:15:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aYc5w-0000S7-5D for qemu-devel@nongnu.org; Wed, 24 Feb 2016 11:15:20 -0500 References: <87083486787f4d4b9752f4b29209e6d0d1d31626.1455033112.git.berto@igalia.com> From: Max Reitz Message-ID: <56CDD70F.8000304@redhat.com> Date: Wed, 24 Feb 2016 17:15:11 +0100 MIME-Version: 1.0 In-Reply-To: <87083486787f4d4b9752f4b29209e6d0d1d31626.1455033112.git.berto@igalia.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="rEPtxp70QKAWTXNQU8jTolJPfHIhCwXj1" Subject: Re: [Qemu-devel] [PATCH v2 1/2] block: Allow x-blockdev-del on a BB with a monitor-owned BDS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia , qemu-devel@nongnu.org Cc: Kevin Wolf , qemu-block@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --rEPtxp70QKAWTXNQU8jTolJPfHIhCwXj1 Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable 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. >=20 > 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. >=20 > 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. >=20 > Signed-off-by: Alberto Garcia > --- > 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 --rEPtxp70QKAWTXNQU8jTolJPfHIhCwXj1 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWzdcPAAoJEDuxQgLoOKyt67gH/0Q/k7h1x7h7bjzELOBQabsT P3hEYRnKPNxHgjNXuI0DIPnz+JbN1Yho/JmPOB16TKaKVESRhv/hk5pVnRcq2SOL C9DfmKaiM2LUwpOToRBj8dnHy3XWAWZmxkbZPKY64azoeBIHloaOwZOaPKtGRkKJ KfEx39QZSsFxhOTY8wGzyzK2CAO6y1VSI2+dAGobZpzD7Rky+aWzTWqaLJ/+lKNu rVyOXgTM5/JcqgBGZjE0+f+xOHQP1FDLdvZBYtdwCcQ5BrwmPRI5Ga6xeUaYAEEL dPwDqn6JXdzjSHJhEX8bV+DcPUxPRdMOfI29C50mR/dEKWc6UYLeDeNw36A23L0= =V1gT -----END PGP SIGNATURE----- --rEPtxp70QKAWTXNQU8jTolJPfHIhCwXj1--