From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60685) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGBqf-00064R-1e for qemu-devel@nongnu.org; Tue, 27 Jan 2015 14:30:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGBqb-00061K-Mp for qemu-devel@nongnu.org; Tue, 27 Jan 2015 14:30:52 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53004) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGBqb-000619-Eq for qemu-devel@nongnu.org; Tue, 27 Jan 2015 14:30:49 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0RJUlc5002472 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 27 Jan 2015 14:30:48 -0500 Message-ID: <54C7E766.3080005@redhat.com> Date: Tue, 27 Jan 2015 12:30:46 -0700 From: Eric Blake MIME-Version: 1.0 References: <1422288204-29271-1-git-send-email-mreitz@redhat.com> In-Reply-To: <1422288204-29271-1-git-send-email-mreitz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="SH6lM8jPkLsqk9TlDhTS1PAAPfaXqIJ6Q" Subject: Re: [Qemu-devel] [PATCH 00/50] blockdev: BlockBackend and media List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-devel@nongnu.org Cc: Kevin Wolf , Fam Zheng , Jeff Cody , Markus Armbruster , Stefan Hajnoczi , john@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --SH6lM8jPkLsqk9TlDhTS1PAAPfaXqIJ6Q Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/26/2015 09:02 AM, Max Reitz wrote: > This series reworks a lot regarding BlockBackend and media. It is > essentially a v3 to the "blockdev: Add blockdev-change-medium with > read-only option" series (which is in fact a part of this series), but > of course does a lot more. >=20 I'm not done reviewing yet, but making some comments here as I go. >=20 > This series depends on v3 (or any later version) of my series > 'block: Remove "growable", add blk_new_open()'. So I reviewed that first. >=20 >=20 > - Patches 1 and 2 make it possible to use blockdev-add without creating= > a BlockBackend. This is necessary because the QMP command introduced > in patch 42 (blockdev-insert-medium) will insert a BDS tree (a medium= ) > into a BlockBackend (a drive). Creating a BlockBackend for such a BDS= > tree would both be a hassle and a waste, so this makes it possible to= > omit creating a BB. Basically, making blockdev-add be capable of creating a detached BDS tree for later attachment to some device or as a branch in an even larger BDS tree. Seems reasonable enough; I still haven't made up my mind if we need some form of introspection to know if this usage is supported (but since later in the series you are adding new commands that depend on this, that may be a good enough witness). > - Patch 6 makes blk_is_inserted() return true only if there is a BDS > tree; furthermore, blk_is_available() is added which additionally > checks whether the guest device tray is closed. >=20 > - Patches 7 and 8 are some kind of follow-up to patch 6; they make > bdrv_is_inserted() actually useful (unless I'm not mistaken; maybe I > am and they make bdrv_is_inserted() actually useless). Does it even make sense to have a quorum that is mixed between a CDROM passthrough drive (where medium can be removed) and on-disk/network locations (where medium is unchangeable)? But your idea makes sense - a tree is inserted if all of its parts are also inserted, and removing any one medium anywhere in the BDS tree makes it pointless to use all other parts of the tree that feed the same BB. Do we need some sort of operation blocker that prevents eject on a BDS that feeds a higher-level node of a BDS tree? Or if you want to read that paragraph differently: I reviewed the patches for coding bugs and found none, but I'm fuzzy enough on design details that I'd appreciate a second review from an interested party to make sure I'm not overlooking a design flaw in your change. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --SH6lM8jPkLsqk9TlDhTS1PAAPfaXqIJ6Q Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJUx+dmAAoJEKeha0olJ0NqY3cH/0cE1Rxa63XiklXJo2XZYs7A V5Y346x+Jtfq1Rd/6tq0OgMdw2/YuksZVa//h7zDOucqPah4v4Bc4EH9K9pSDq+m KEAGzYZbzz2CaTDlBLcxHg6pW0/HO43rkJMhIxvYvMzsAhb4bp2FTX3noEIbTdKG 7M9HnNo0cWHu3jyxmVj+/I0JiPXvCUYy/bPJfWEHtdo5NUJyO4xbR+UfNLXkMKJS sSPqpjzIS5RpcS0vBcHUNf6/qqvDR25GtmUibFkUwEY8QATCTSNJdj4KOModip4D iTgO7bO3Mu4j5YHESCHscWJCZQ6Ch9PIwVJv/5rU/gfnzMlwutg3MZeAE/xzmAo= =6qAL -----END PGP SIGNATURE----- --SH6lM8jPkLsqk9TlDhTS1PAAPfaXqIJ6Q--