On 2018-04-25 14:58, Alberto Garcia wrote: > On Fri 20 Apr 2018 03:13:49 PM CEST, Max Reitz wrote: >>>>>>> One other way is to have a more generic replace-node command >>>>>>> which would call bdrv_replace_node(), but I don't know if we want >>>>>>> to expose that and I don't have any other use case for it at the >>>>>>> moment. >>>>>> >>>>>> I think we do want to expose that. As far as I'm informed, for >>>>>> graph manipulation we want a command that can replace nodes >>>>>> (including replacing nothing by a new node or replacing an >>>>>> existing node by nothing, if the parent supports that). >>>>> >>>>> Was there any discussion about this in the mailing list? (proposed >>>>> name for this function, features, etc.) >>>> >>>> Well, there is x-blockdev-change. But we probably want to expose >>>> bdrv_reopen() in the long run. >>> >>> Exposing bdrv_reopen() itself shouldn't be too much work (it takes >>> just two node names, or am I missing something?). > > I just realized that I meant "Exposing bdrv_replace_node()" here. > >> So from my perspective... If you think it's easy, why don't you try >> it and then we'll see? *cough* > > I'm doing it :-) > >>> There's still the question of how to update the backing file string >>> (on the overlay). stream and commit do it automatically, but if we do >>> bdrv_reopen() or the aforementioned modification to blockdev-mirror > > bdrv_replace_node() again
But the question stands whether we need simple node replacement when we want bdrv_reopen() anyway. In addition, we don't need just replacement, we also need addition and removal (e.g. for backing files or quorum children) -- and especially in the case of quorum, that is going to be a pain (mostly naming the children). With bdrv_reopen(), we can just require the user to respecify all children, so we don't run into the issue of how to name things at least. (Other things we we'd want bdrv_reopen() are listed in the meeting notes.) >>> we'd need to do it explicitly with change-backing-file, or extend the >>> API to allow for that. >> >> Well, the December notes do say that we probably want an option to >> specify the target's filename which is what will be written into the >> overlays of @to_replace as their backing file string, so I think >> extending the API should be fine. > > Yes, but how do you find out what the overlays are (without an > additional parameter, that is)? I thought we didn't want to support > walking the backing chain in reverse direction. We already have an .update_filename() BdrvChildRole callback, though. Max
signature.asc
Description: OpenPGP digital signature