On 09/09/2015 05:20 AM, Max Reitz wrote:
> 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.
A BDS can be written by its parent, its block backend, a block job..
So I think we should have some way to avoid more than two sources writing
to it, otherwise the data may be corrupted.
Thanks
Wen Congyang
>
> 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,
>>>
>>
>
>
