From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38452) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpdYq-0007Cp-En for qemu-devel@nongnu.org; Fri, 23 Oct 2015 10:43:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZpdYp-0008RO-3t for qemu-devel@nongnu.org; Fri, 23 Oct 2015 10:43:16 -0400 References: <1445270025-22999-1-git-send-email-mreitz@redhat.com> <1445270025-22999-34-git-send-email-mreitz@redhat.com> <20151023141131.GI3797@noname.redhat.com> From: Max Reitz Message-ID: <562A4779.2040708@redhat.com> Date: Fri, 23 Oct 2015 16:43:05 +0200 MIME-Version: 1.0 In-Reply-To: <20151023141131.GI3797@noname.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="vliGq7EmxuPNw2WdsCP7GJtlQd1FCGJHr" Subject: Re: [Qemu-devel] [PATCH v7 33/39] blockdev: Implement change with basic operations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Alberto Garcia , qemu-block@nongnu.org, John Snow , qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --vliGq7EmxuPNw2WdsCP7GJtlQd1FCGJHr Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 23.10.2015 16:11, Kevin Wolf wrote: > Am 19.10.2015 um 17:53 hat Max Reitz geschrieben: >> Implement 'change' on block devices by calling blockdev-open-tray, >> blockdev-remove-medium, blockdev-insert-medium (a variation of that >> which does not need a node-name) and blockdev-close-tray. >> >> Signed-off-by: Max Reitz >> --- >> blockdev.c | 184 +++++++++++++++++++++++++---------------------------= --------- >> 1 file changed, 74 insertions(+), 110 deletions(-) >> >> diff --git a/blockdev.c b/blockdev.c >> index 0481686..50e5e74 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -1901,44 +1901,6 @@ exit: >> } >> } >> =20 >> - >> -static void eject_device(BlockBackend *blk, int force, Error **errp) >> -{ >> - BlockDriverState *bs =3D blk_bs(blk); >> - AioContext *aio_context; >> - >> - if (!bs) { >> - /* No medium inserted, so there is nothing to do */ >> - return; >> - } >> - >> - aio_context =3D bdrv_get_aio_context(bs); >> - aio_context_acquire(aio_context); >> - >> - if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) { >> - goto out; >> - } >> - if (!blk_dev_has_removable_media(blk)) { >> - error_setg(errp, "Device '%s' is not removable", >> - bdrv_get_device_name(bs)); >> - goto out; >> - } >> - >> - if (blk_dev_is_medium_locked(blk) && !blk_dev_is_tray_open(blk)) = { >> - blk_dev_eject_request(blk, force); >> - if (!force) { >> - error_setg(errp, "Device '%s' is locked", >> - bdrv_get_device_name(bs)); >> - goto out; >> - } >> - } >> - >> - bdrv_close(bs); >> - >> -out: >> - aio_context_release(aio_context); >> -} >> - >> void qmp_eject(const char *device, bool has_force, bool force, Error = **errp) >> { >> Error *local_err =3D NULL; >> @@ -1976,78 +1938,6 @@ void qmp_block_passwd(bool has_device, const ch= ar *device, >> aio_context_release(aio_context); >> } >> =20 >> -/* Assumes AioContext is held */ >> -static void qmp_bdrv_open_encrypted(BlockDriverState **pbs, >> - const char *filename, >> - int bdrv_flags, const char *forma= t, >> - const char *password, Error **err= p) >> -{ >> - Error *local_err =3D NULL; >> - QDict *options =3D NULL; >> - int ret; >> - >> - if (format) { >> - options =3D qdict_new(); >> - qdict_put(options, "driver", qstring_from_str(format)); >> - } >> - >> - ret =3D bdrv_open(pbs, filename, NULL, options, bdrv_flags, &loca= l_err); >> - if (ret < 0) { >> - error_propagate(errp, local_err); >> - return; >> - } >> - >> - bdrv_add_key(*pbs, password, errp); >> -} >> - >> -void qmp_change_blockdev(const char *device, const char *filename, >> - const char *format, Error **errp) >> -{ >> - BlockBackend *blk; >> - BlockDriverState *bs; >> - AioContext *aio_context; >> - int bdrv_flags; >> - bool new_bs; >> - Error *err =3D NULL; >> - >> - blk =3D blk_by_name(device); >> - if (!blk) { >> - error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, >> - "Device '%s' not found", device); >> - return; >> - } >> - bs =3D blk_bs(blk); >> - new_bs =3D !bs; >> - >> - aio_context =3D blk_get_aio_context(blk); >> - aio_context_acquire(aio_context); >> - >> - eject_device(blk, 0, &err); >> - if (err) { >> - error_propagate(errp, err); >> - goto out; >> - } >> - >> - bdrv_flags =3D blk_is_read_only(blk) ? 0 : BDRV_O_RDWR; >> - bdrv_flags |=3D blk_get_root_state(blk)->open_flags & ~BDRV_O_RDW= R; >> - >> - qmp_bdrv_open_encrypted(&bs, filename, bdrv_flags, format, NULL, = &err); >> - if (err) { >> - error_propagate(errp, err); >> - goto out; >> - } >> - >> - if (new_bs) { >> - blk_insert_bs(blk, bs); >> - /* Has been sent automatically by bdrv_open() if blk_bs(blk) = was not >> - * NULL */ >> - blk_dev_change_media_cb(blk, true); >> - } >> - >> -out: >> - aio_context_release(aio_context); >> -} >> - >> void qmp_blockdev_open_tray(const char *device, bool has_force, bool = force, >> Error **errp) >> { >> @@ -2204,6 +2094,80 @@ void qmp_blockdev_insert_medium(const char *dev= ice, const char *node_name, >> qmp_blockdev_insert_anon_medium(device, bs, errp); >> } >> =20 >> +void qmp_change_blockdev(const char *device, const char *filename, >> + const char *format, Error **errp) >> +{ >> + BlockBackend *blk; >> + BlockBackendRootState *blk_rs; >> + BlockDriverState *medium_bs =3D NULL; >> + int bdrv_flags, ret; >> + QDict *options =3D NULL; >> + Error *err =3D NULL; >> + >> + blk =3D blk_by_name(device); >> + if (!blk) { >> + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, >> + "Device '%s' not found", device); >> + goto fail; >> + } >> + >> + if (blk_bs(blk)) { >> + blk_update_root_state(blk); >> + } >> + >> + blk_rs =3D blk_get_root_state(blk); >> + bdrv_flags =3D blk_rs->read_only ? 0 : BDRV_O_RDWR; >> + bdrv_flags |=3D blk_rs->open_flags & ~BDRV_O_RDWR; >> + >> + if (format) { >> + options =3D qdict_new(); >> + qdict_put(options, "driver", qstring_from_str(format)); >> + } >> + >> + assert(!medium_bs); >> + ret =3D bdrv_open(&medium_bs, filename, NULL, options, bdrv_flags= , errp); >> + if (ret < 0) { >> + goto fail; >> + } >> + >> + medium_bs->detect_zeroes =3D blk_rs->detect_zeroes; >> + if (blk_rs->throttle_group) { >> + bdrv_io_limits_enable(medium_bs, blk_rs->throttle_group); >> + } >=20 > Would it make sense to have a blk_apply_root_state() that does these > updates? Otherwise you have to keep this place up to date if the struct= > is ever extended. (Which would mean that we forgot something, so > hopefully not.) Makes sense. I'll probably write one with an annotation that it's only to be used by this function. Max --vliGq7EmxuPNw2WdsCP7GJtlQd1FCGJHr 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 iQEcBAEBCAAGBQJWKkd5AAoJEDuxQgLoOKyt78YIAJYGiV+7J4GFrCfovhnjh+3t kuul0iQNHdV28eNDVxt7BcoHX+vkCQ7oDZU6qah2mlFnmmOSVMlTXl7S9dMHbw8s cygrt/8ZxQDrNNAlSpUG7U/2PxH2lT/QHLeQl5cgsgcJ7mfOVIOi65jkYjQSFgFJ rXn6Dpt4j1Qjir6/+tijM63gHjZyBEFx4ACfHUoWMEDN2++daoLyHVwLF+yri+ef moG9o9xzoeahl5qLhcw9kruCwULMyuHDVBGbFVRT4OkC4F2r9Gv0Rc7FHEQODtbp t02+8IFVHvtOX6+xka/GWDRcCEjrKs+0N2h/jYJndf2Pc7Jjk7jXaccn1bptL6I= =ja/t -----END PGP SIGNATURE----- --vliGq7EmxuPNw2WdsCP7GJtlQd1FCGJHr--