Hi,

I'm working on propagating errors out to do_device_add(). I've come
accross qbus_find_recursive().

Re commit 1395af6f ("qdev: add a maximum device allowed field for the
bus."), could someone please explain:

(1) why the early termination due to max_index vs. max_dev merits an
error message -- both non-recursive call-sites print an error of their
own anyway,

(2) why the

  max_dev != 0 &&
  max_dev <= max_index &&
  name != NULL

condition justifies traversal termination -- in general, a bus called
"foo", albeit full of devices, could allow some of those devices to be a
child bus. Let's call one such (direct) child bus "bar".

If we're looking for "bar", or for one of its descendants, the current
logic seems to stop the search early, at "foo".

I think "bus_class->max_dev" should be checked only when we're actually
trying to add the device to the bus. (In qdev_set_parent_bus() -->
bus_add_child().)

- bus_add_child() should check the limit and set an Error,
- qdev_set_parent_bus_nofail() would abort() if there's an Error,
- qdev_set_parent_bus() would change signature and propagate error,
- all callers would be converted to either the new _nofail() or the old
function with the new signature.

I can try to write the patch if there's consensus about what should happen.

Thoughts?

Thanks!
Laszlo

Reply via email to