On Thu, May 30, 2013 at 04:27:48PM -0600, Eric Blake wrote: > On 05/30/2013 06:34 AM, Stefan Hajnoczi wrote: > > notifier_list_notify() has no return value. This is fine when we just > > want to invoke side-effects. > > > > Sometimes it's useful for notifiers to produce a return value. This > > allows notifiers to "veto" an operation and will be used by the block > > layer before-write notifier. > > > > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> > > --- > > include/qemu/notify.h | 25 +++++++++++++++++++++++++ > > util/notify.c | 30 ++++++++++++++++++++++++++++++ > > 2 files changed, 55 insertions(+) > > > > diff --git a/include/qemu/notify.h b/include/qemu/notify.h > > index 4e2e7f0..d3103e7 100644 > > --- a/include/qemu/notify.h > > +++ b/include/qemu/notify.h > > @@ -40,4 +40,29 @@ void notifier_remove(Notifier *notifier); > > > > void notifier_list_notify(NotifierList *list, void *data); > > > > +/* Same as Notifier but allows .notify() to return errors */ > > It's probably worth documenting that the callback must return 0 for > success, and that the first non-zero return is passed back without > further interpretation (thus allowing negative errno values if desired, > vs. a simpler -1).
You are right, let's improve the doc comments. > Isn't this really just syntax sugar since existing notifiers could > already use a member within the opaque *data argument as a way to > short-circuit later notifiers on earlier errors? If so, how hard would > it be to scrub the existing code base to always return 0 in existing > notifiers, rather than adding a parallel naming scheme? I considered both approaches and decided on NotifierWithReturn because it neither touches existing users nor adds a side-effect for aborting. BTW adding a side-effect in the BdrvTrackedRequest case is ugly, the struct is used for more than just the pre-write notifier and the field would be useless in the other cases. Stefan