On 09/01/2015 11:30 PM, Eric Blake wrote: > On 08/31/2015 06:44 PM, Wen Congyang wrote: > >>> >>>> + * Hot add/remove a BDS's child. So the user can take a child offline when >>>> + * it is broken and take a new child online >>>> + */ >>>> +void bdrv_add_child(BlockDriverState *bs, QDict *options, Error **errp) >>>> +{ >>>> + >>>> + if (!bs->drv || !bs->drv->bdrv_add_child) { >>>> + error_setg(errp, "The BDS %s doesn't support adding a child", >>>> + bdrv_get_device_or_node_name(bs)); >>>> + return; >>>> + } >>>> + >>>> + bs->drv->bdrv_add_child(bs, options, errp); >>> >>> Should this also check that bs is not already a child of something? Or >>> a bit looser, we may want to allow a BDS to be a child of multiple trees >>> (a common shared backing file), but we still definitely don't want to >>> allow nonsensical loops such as trying to make a BDS be hot-added as its >>> own child. >>> >> >> hot-added BDS is a new BDS, but it is OK to check it here. I will update it >> in the next version. > > Design-wise, I think we really want to have the add-child operation be > handed a pre-opened BDS, rather than the options dictionary to open the > BDS itself. That is, we should use the existing blockdev-add (and > enhance it to support everything) to open the BDS, and then this command > should just attach that BDS as the new child (which is why it IS > important that we validate that the new BDS being added doesn't create > an invalid loop). >
How to check it? The parent BDS can get all children. But the child doesn't know if it is some BDS's child. Thanks Wen Congyang