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

[Max Reitz]

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.

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

signature.asc
Description: OpenPGP digital signature