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 <[email protected]> >> --- >> 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
