From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37208) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGZ4I-0005SX-0o for qemu-devel@nongnu.org; Wed, 28 Jan 2015 15:18:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGZ4D-0001RI-Vs for qemu-devel@nongnu.org; Wed, 28 Jan 2015 15:18:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54823) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGZ4D-0001Qo-Fq for qemu-devel@nongnu.org; Wed, 28 Jan 2015 15:18:25 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0SKINDw017339 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 28 Jan 2015 15:18:24 -0500 Message-ID: <54C9440D.8020702@redhat.com> Date: Wed, 28 Jan 2015 13:18:21 -0700 From: Eric Blake MIME-Version: 1.0 References: <1422387983-32153-1-git-send-email-mreitz@redhat.com> <1422387983-32153-42-git-send-email-mreitz@redhat.com> In-Reply-To: <1422387983-32153-42-git-send-email-mreitz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="iRMinxcJNjiaslU8bINj5NlHfDUNJbbTL" Subject: Re: [Qemu-devel] [PATCH RESEND 41/50] blockdev: Add blockdev-insert-medium 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 Snow This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --iRMinxcJNjiaslU8bINj5NlHfDUNJbbTL Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 01/27/2015 12:46 PM, Max Reitz wrote: > And a helper function for that which directly takes a pointer to the BD= S > to be inserted instead of its node-name (which will be used for > implementing 'change' using blockdev-insert-medium). >=20 > Signed-off-by: Max Reitz > --- > blockdev.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > qapi/block-core.json | 17 +++++++++++++++++ > qmp-commands.hx | 38 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 98 insertions(+) >=20 > +static void qmp_blockdev_insert_anon_medium(const char *device, > + BlockDriverState *bs, Erro= r **errp) > +{ > + BlockBackend *blk; > + > + blk =3D blk_by_name(device); > + if (!blk) { > + error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + return; > + } > + > + if (!blk_dev_has_removable_media(blk)) { > + error_setg(errp, "Device '%s' is not removable", device); > + return; > + } > + > + if (!blk_dev_is_tray_open(blk)) { > + error_setg(errp, "Tray of device '%s' is not open", device); > + return; > + } > + > + if (blk_bs(blk)) { > + error_setg(errp, "There already is a medium in device '%s'", d= evice); > + return; > + } Good, you didn't implement hot-swap semantics of replacing an existing medium (that gets too confusing; so I _like_ that you force the user to consider all the steps through multiple low-level commands). > + > +Example (1): I'll quit pointing it out; but if you clean up the useless (1) in one patch, do it across the series. > + > +-> { "execute": "blockdev-add", > + "arguments": { "options": { "id": "backend0", > + "node-name": "node0", Why is 'id' needed? Isn't the point of this command sequence to create a BDS tree that is NOT tied to a BB, and then use insert-medium to make the association after the fact? We don't need to create a BB named 'backend0' if we are immediately going to reuse 'node0' in a different BB (true, we have somewhat anticipated the idea of sharing BDS tree among multiple BB, but haven't quite turned that on before now). > + "driver": "raw", > + "file": { "driver": "file", > + "filename": "fedora.iso" } = } } } > + > +<- { "return": {} } > + > +-> { "execute": "blockdev-insert-medium", > + "arguments": { "device": "ide1-cd0", > + "node-name": "node0" } } > + > +<- { "return": {} } If you can either explain why you used 'id' in the example, or remove that parameter, you can add: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --iRMinxcJNjiaslU8bINj5NlHfDUNJbbTL 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/ iQEcBAEBCAAGBQJUyUQNAAoJEKeha0olJ0Nq/QgH/j6/id0Xe2j/74g0crRRUSOK OeqTReOXkB0BoQzdIXQ4mizPqVAhcFNnFeTlfYTRm7VT1XnIzgnVDg1xRFD8JqQp +IPryyyOYaZeFka4QTdt4h2531leqypcUT19MuQzRlb970UEJEAUL9Y+9dVZztya f9lNq/ufGAJ9hmQsh64SIy9aaIF9Z59l8xodje3Kf+O3CXii1Dfcm1fYSJsIJrAo 0CXg+6TUDGUJGRkRh9sdVDgCOWd8hD0FHhRukx/OG5O+bN5tEXYTxzCRaLNUKXGA YABRngJOsQqJSuCId1fWr36jFuvwyHSImyowB6WiAzM2j3avjmVPjipCtBtruRs= =cxCj -----END PGP SIGNATURE----- --iRMinxcJNjiaslU8bINj5NlHfDUNJbbTL--