On 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 <[email protected]> >>> 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?
Is there any guess how complex it might be? In the case if it's quite complex may be it's worth to make the separate command? > > > (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. Out of curiosity, could you please explain why it's decided to be so? > 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 >
