On 08.09.2015 11:13, Wen Congyang wrote: > On 07/21/2015 01:45 AM, Max Reitz wrote: >> And a helper function for that, which directly takes a pointer to the >> BDS to be inserted instead of its node-name (which will be used for >> implementing 'change' using blockdev-insert-medium). >> >> Signed-off-by: Max Reitz <mre...@redhat.com> >> --- >> blockdev.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ >> qapi/block-core.json | 17 +++++++++++++++++ >> qmp-commands.hx | 37 +++++++++++++++++++++++++++++++++++++ >> 3 files changed, 102 insertions(+) >> >> diff --git a/blockdev.c b/blockdev.c >> index 481760a..a80d0e2 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -2164,6 +2164,54 @@ void qmp_blockdev_remove_medium(const char *device, >> Error **errp) >> } >> } >> >> +static void qmp_blockdev_insert_anon_medium(const char *device, >> + BlockDriverState *bs, Error >> **errp) >> +{ >> + BlockBackend *blk; >> + bool has_device; >> + >> + blk = blk_by_name(device); >> + if (!blk) { >> + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, >> + "Device '%s' not found", device); >> + return; >> + } >> + >> + /* For BBs without a device, we can exchange the BDS tree at will */ >> + has_device = blk_get_attached_dev(blk); >> + >> + if (has_device && !blk_dev_has_removable_media(blk)) { >> + error_setg(errp, "Device '%s' is not removable", device); >> + return; >> + } >> + >> + if (has_device && !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'", >> device); >> + return; >> + } >> + >> + blk_insert_bs(blk, bs); >> +} >> + >> +void qmp_blockdev_insert_medium(const char *device, const char *node_name, >> + Error **errp) >> +{ >> + BlockDriverState *bs; >> + >> + bs = bdrv_find_node(node_name); >> + if (!bs) { >> + error_setg(errp, "Node '%s' not found", node_name); >> + return; >> + } > > Hmm, it is OK if the bs is not top BDS?
I think so, yes. Generally, there's probably no reason to do that, but I don't know why we should not allow that case. For instance, you might want to make a backing file available read-only somewhere. It should be impossible to make it available writable, and it should not be allowed to start a block-commit operation while the backing file can be accessed by the guest, but this should be achieved using op blockers. What we need for this to work are fine-grained op blockers, I think. But working around that for now by only allowing to insert top BDS won't work, since you can still start block jobs which target top BDS, too (e.g. blockdev-backup can write to a BDS/BB that is visible to the guest). All in all, I think it's fine to insert non-top BDS, but we should definitely worry about which exact BDS one can insert once we have fine-grained op blockers. Max > Thanks > Wen Congyang > >> + >> + qmp_blockdev_insert_anon_medium(device, bs, errp); >> +} >> + >> /* throttling disk I/O limits */ >> void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t >> bps_rd, >> int64_t bps_wr, >> diff --git a/qapi/block-core.json b/qapi/block-core.json >> index 63a83e4..84c9b23 100644 >> --- a/qapi/block-core.json >> +++ b/qapi/block-core.json >> @@ -1925,6 +1925,23 @@ >> { 'command': 'blockdev-remove-medium', >> 'data': { 'device': 'str' } } >> >> +## >> +# @blockdev-insert-medium: >> +# >> +# Inserts a medium (a block driver state tree) into a block device. That >> block >> +# device's tray must currently be open and there must be no medium inserted >> +# already. >> +# >> +# @device: block device name >> +# >> +# @node-name: name of a node in the block driver state graph >> +# >> +# Since: 2.5 >> +## >> +{ 'command': 'blockdev-insert-medium', >> + 'data': { 'device': 'str', >> + 'node-name': 'str'} } >> + >> >> ## >> # @BlockErrorAction >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index ff6c572..b4c34fe 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -3991,6 +3991,43 @@ Example: >> EQMP >> >> { >> + .name = "blockdev-insert-medium", >> + .args_type = "device:s,node-name:s", >> + .mhandler.cmd_new = qmp_marshal_input_blockdev_insert_medium, >> + }, >> + >> +SQMP >> +blockdev-insert-medium >> +---------------------- >> + >> +Inserts a medium (a block driver state tree) into a block device. That block >> +device's tray must currently be open and there must be no medium inserted >> +already. >> + >> +Arguments: >> + >> +- "device": block device name (json-string) >> +- "node-name": root node of the BDS tree to insert into the block device >> + >> +Example: >> + >> +-> { "execute": "blockdev-add", >> + "arguments": { "options": { "node-name": "node0", >> + "driver": "raw", >> + "file": { "driver": "file", >> + "filename": "fedora.iso" } } } } >> + >> +<- { "return": {} } >> + >> +-> { "execute": "blockdev-insert-medium", >> + "arguments": { "device": "ide1-cd0", >> + "node-name": "node0" } } >> + >> +<- { "return": {} } >> + >> +EQMP >> + >> + { >> .name = "query-named-block-nodes", >> .args_type = "", >> .mhandler.cmd_new = qmp_marshal_input_query_named_block_nodes, >> >
signature.asc
Description: OpenPGP digital signature