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

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

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 <dplotnikov@virtuozzo.com>
>>> 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...

> 
> 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