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). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature