Re: [Qemu-devel] [PATCH v5 01/11] notify: add NotiferWithReturn so notifier list can abort

[Eric Blake]

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 <address@hidden>
> ---
>  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).

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?

At any rate, adding a new naming scheme minimizes churn, and the code
itself looks okay;

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature