On 17.02.2016 11:53, Kevin Wolf wrote: > Am 16.02.2016 um 19:08 hat Max Reitz geschrieben: >> The monitor does hold references to some BlockBackends so it should have > > s/does hold/holds/?
It was intentional, so I'd keep it unless you drop the question mark. >> a list of those BBs; blk_backends is a different list, as it contains >> references to all BBs (after a follow-up patch, that is), and that >> should not be changed because we do need such a list. >> >> monitor_remove_blk() is idempotent so that we can call it in >> blockdev_auto_del() without having to care whether it had been called in >> do_drive_del() before. monitor_add_blk() is idempotent for symmetry >> reasons (monitor_remove_blk() is, so it would be strange for >> monitor_add_blk() not to be). >> >> Signed-off-by: Max Reitz <mre...@redhat.com> > > I think hmp_drive_add() needs a monitor_remove_blk() in its error path. You're right, thanks. In addition, if we really do say that a BB having a name equals being referenced by the monitor, then maybe we don't need explicit calls to monitor_add_blk() because any BB that is created with a non-NULL name should be automatically added to the list of monitor BBs. But that would mean that qemu-img's, qemu-nbd's and qemu-io's BBs would have to be monitor-owned, too, and they'd all have to call monitor_remove_blk() all over the place... Unless we'd allow NULL BB names now and make them use it (I don't really see a reason why not; them calling their BBs "hda" seems weird anyway), or implicitly call monitor_remove_blk() in blk_delete(). Or maybe both, because the latter seems convenient anyway. Max
signature.asc
Description: OpenPGP digital signature