From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57903) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zpd4L-0005mQ-V9 for qemu-devel@nongnu.org; Fri, 23 Oct 2015 10:11:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zpd4K-0006Dy-MY for qemu-devel@nongnu.org; Fri, 23 Oct 2015 10:11:45 -0400 Date: Fri, 23 Oct 2015 16:11:31 +0200 From: Kevin Wolf Message-ID: <20151023141131.GI3797@noname.redhat.com> References: <1445270025-22999-1-git-send-email-mreitz@redhat.com> <1445270025-22999-34-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1445270025-22999-34-git-send-email-mreitz@redhat.com> 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: Max Reitz Cc: Alberto Garcia , qemu-block@nongnu.org, John Snow , qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi 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: > } > } > > - > -static void eject_device(BlockBackend *blk, int force, Error **errp) > -{ > - BlockDriverState *bs = blk_bs(blk); > - AioContext *aio_context; > - > - if (!bs) { > - /* No medium inserted, so there is nothing to do */ > - return; > - } > - > - aio_context = 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 = NULL; > @@ -1976,78 +1938,6 @@ void qmp_block_passwd(bool has_device, const char *device, > aio_context_release(aio_context); > } > > -/* Assumes AioContext is held */ > -static void qmp_bdrv_open_encrypted(BlockDriverState **pbs, > - const char *filename, > - int bdrv_flags, const char *format, > - const char *password, Error **errp) > -{ > - Error *local_err = NULL; > - QDict *options = NULL; > - int ret; > - > - if (format) { > - options = qdict_new(); > - qdict_put(options, "driver", qstring_from_str(format)); > - } > - > - ret = bdrv_open(pbs, filename, NULL, options, bdrv_flags, &local_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 = NULL; > - > - blk = blk_by_name(device); > - if (!blk) { > - error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > - "Device '%s' not found", device); > - return; > - } > - bs = blk_bs(blk); > - new_bs = !bs; > - > - aio_context = 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 = blk_is_read_only(blk) ? 0 : BDRV_O_RDWR; > - bdrv_flags |= blk_get_root_state(blk)->open_flags & ~BDRV_O_RDWR; > - > - 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 *device, const char *node_name, > qmp_blockdev_insert_anon_medium(device, bs, errp); > } > > +void qmp_change_blockdev(const char *device, const char *filename, > + const char *format, Error **errp) > +{ > + BlockBackend *blk; > + BlockBackendRootState *blk_rs; > + BlockDriverState *medium_bs = NULL; > + int bdrv_flags, ret; > + QDict *options = NULL; > + Error *err = NULL; > + > + blk = 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 = blk_get_root_state(blk); > + bdrv_flags = blk_rs->read_only ? 0 : BDRV_O_RDWR; > + bdrv_flags |= blk_rs->open_flags & ~BDRV_O_RDWR; > + > + if (format) { > + options = qdict_new(); > + qdict_put(options, "driver", qstring_from_str(format)); > + } > + > + assert(!medium_bs); > + ret = bdrv_open(&medium_bs, filename, NULL, options, bdrv_flags, errp); > + if (ret < 0) { > + goto fail; > + } > + > + medium_bs->detect_zeroes = blk_rs->detect_zeroes; > + if (blk_rs->throttle_group) { > + bdrv_io_limits_enable(medium_bs, blk_rs->throttle_group); > + } 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.) Kevin