On Fri, Nov 20, 2015 at 03:57:17PM +0100, Markus Armbruster wrote: > Eduardo Habkost <ehabk...@redhat.com> writes: > > > On Fri, Nov 20, 2015 at 02:00:40PM +0100, Markus Armbruster wrote: > >> qmp_query_memdev() doesn't fail. Instead, it returns an empty list. > >> That's wrong. > >> > >> Two error paths: > >> > >> * When object_get_objects_root() returns null. It never does, so > >> simply drop the useless error handling. > >> > >> * When query_memdev() fails. This can happen, and the error to return > >> is the one that query_memdev() currently leaks. Passing the error > >> from query_memdev() to qmp_query_memdev() isn't so simple, because > >> object_child_foreach() is in the way. Fixable, but I'd rather not > >> try it in hard freeze. Plug the leak, make up an error, and add a > >> FIXME for the remaining work. > >> > >> Screwed up in commit 76b5d85 "qmp: add query-memdev". > >> > >> Signed-off-by: Markus Armbruster <arm...@redhat.com> > > > > Reviewed-by: Eduardo Habkost <ehabk...@redhat.com> > > > > Do you know how to trigger a query_memdev() error today, or is > > just theoretical? > > Theoretical; I tested by injecting an error with gdb. > > query_memdev() fails exactly when some object_property_get_FOO() fails. > If we decide such a failure would always be a programming error, we can > pass &error_abort and simplify things. Opinions?
The hostmem-backend property getters should never fail. Using &error_abort on query_memdev() would make everything simpler. (I would even use the HostMemoryBackend struct fields directly, instead of QOM properties. Is there a good reason to use QOM to fetch the data that's readily available in the C struct?) -- Eduardo