[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 01/11] notify: add NotiferWithReturn so notif
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v5 01/11] notify: add NotiferWithReturn so notifier list can abort |
Date: |
Mon, 3 Jun 2013 11:11:43 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <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).
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
- Re: [Qemu-devel] [PATCH v5 01/11] notify: add NotiferWithReturn so notifier list can abort,
Stefan Hajnoczi <=