[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium |
Date: |
Thu, 10 Sep 2015 21:10:10 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 10.09.2015 05:22, Wen Congyang wrote:
> On 09/09/2015 08:59 PM, Max Reitz wrote:
>> On 09.09.2015 12:01, Wen Congyang wrote:
>>> 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 <address@hidden>
>>>>>> ---
>>>>>> 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.
>>
>> Yes, and that would be op blockers.
>>
>> As I said, using blockdev-backup you can write to a BB that can be
>> written to by the guest as well. I think this is a bug, but it is a bug
>> that needs to be fixed by having better op blockers in place, which Jeff
>> Cody is working on.
>
> I don't find such patches in the maillist.
That's because Jeff is still working on designing and writing them.
Max
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, (continued)
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, Eric Blake, 2015/09/08
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, Wen Congyang, 2015/09/08
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, Max Reitz, 2015/09/08
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, Wen Congyang, 2015/09/09
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, Max Reitz, 2015/09/09
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, Wen Congyang, 2015/09/09
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, Max Reitz, 2015/09/10
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, Wen Congyang, 2015/09/11
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, Max Reitz, 2015/09/11
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium, Wen Congyang, 2015/09/09
- Re: [Qemu-devel] [PATCH v4 29/38] blockdev: Add blockdev-insert-medium,
Max Reitz <=