Re: [PATCH] blockdev: modify blockdev-change-medium to change non-removable device

[Vladimir Sementsov-Ogievskiy]

23.10.2019 16:56, Vladimir Sementsov-Ogievskiy wrote:
> 22.10.2019 14:05, Max Reitz wrote:
>> On 21.10.19 08:50, Denis Plotnikov wrote:
>>>
>>> On 18.10.2019 18:02, Max Reitz wrote:
>>>> On 18.10.19 14:09, Denis Plotnikov wrote:
>>>>> The modification is useful to workaround exclusive file access 
>>>>> restrictions,
>>>>> e.g. to implement VM migration with shared disk stored on a storage with
>>>>> the exclusive file opening model: a destination VM is started waiting for
>>>>> incomming migration with a fake image drive, and later, on the last 
>>>>> migration
>>>>> phase, the fake image file is replaced with the real one.
>>>>>
>>>>> Signed-off-by: Denis Plotnikov <address@hidden>
>>>> Isn’t this what we would want to use reopen for?
>>>>
>>>> Max
>>>
>>> Could you please explain what is "use reopen"?
>>
>> I was thinking of using (x-)blockdev-reopen to change the file that is
>> used by the format node (e.g. from a null-co node to a real file); or to
>> change the filename of the protocol node.
>>
>> Kevin has pointed out (on IRC) that this will not allow you to change
>> the node that is directly attached to the device.  While I don’t know
>> whether that’s really necessary in this case, if it were indeed
>> necessary, I’d prefer a method to change a guest device’s @drive option
>> because that seems more natural to me.
>>
>> In contrast, the approach taken in this patch seems not quite right to
>> me, because it overloads the whole blockdev-change-medium command with a
>> completely new and different implementation based on whether there’s a
>> removable medium or not.  If the implementation is so different (and the
>> interface is, too, because in one path you must give @medium whereas the
>> other doesn’t evaluate it at all), it should be a new command.
>>
>> I don’t know whether we need a new command at all, though.  On the node
>> level, we have (x-)blockdev-reopen.  So assuming we need something to
>> change the link between the guest device and the block layer, I wonder
>> whether there isn’t something similar; specifically, I’d prefer
>> something to simply change the device’s @drive option.
> 
> Ok, assume we can set @drive option with help of improved qom-set.
> But how to create this new blk? blockdev-add don't have 'id' parameter anymore
> and don't create blk...


Hmm, I see, we have command blockdev-insert-medium(id, node-name). Isn't it what
we want?

> 
>>
>> Kevin has pointed out (on IRC again) that there is indeed one such
>> command, and that’s qom-set.  Unfortunately, this is what happens if you
>> try to use it for @drive:
>>
>> {"error": {"class": "GenericError", "desc": "Attempt to set property
>> 'drive' on anonymous device (type 'virtio-blk-device') after it was
>> realized"}}
>>
>> However, Kevin has claimed it would be technically possible to make an
>> exception for @drive.  Maybe this is worth investigating?
>>
>>
>> (As for blockdev-change-medium, as I’ve said, I don’t really think this
>> fits there.  Furthermore, blockdev-change-medium is kind of a legacy
>> command because I think every command but blockdev-add that does a
>> bdrv_open() kind of is a legacy command.  So if anything, it should be a
>> new command that then takes a node-name.
>> But OTOH, it would be a bit strange to add a separate command for
>> something that in theory should be covered by qom-set @drive.)
>>
>> Max
>>
> 
> 


-- 
Best regards,
Vladimir