qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCHv2] qemu-kvm/vhost: fix up irqfd support


From: Michael S. Tsirkin
Subject: [Qemu-devel] Re: [PATCHv2] qemu-kvm/vhost: fix up irqfd support
Date: Wed, 6 Oct 2010 19:29:47 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

On Wed, Oct 06, 2010 at 11:24:24AM -0600, Alex Williamson wrote:
> On Wed, 2010-10-06 at 19:02 +0200, Michael S. Tsirkin wrote:
> > On Wed, Oct 06, 2010 at 10:48:44AM -0600, Alex Williamson wrote:
> > > > -int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector)
> > > > +static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, 
> > > > unsigned vector)
> > > >  {
> > > >      int r = 0;
> > > > -    void *opaque;
> > > >      if (vector >= dev->msix_entries_nr || 
> > > > !dev->msix_entry_used[vector])
> > > >          return 0;
> > > >  
> > > > -    opaque = dev->msix_mask_notifier_opaque[vector];
> > > > -
> > > >      assert(dev->msix_mask_notifier);
> > > > -    assert(opaque);
> > > >  
> > > >      /* Mask the old notifier unless it is already masked. */
> > > >      if (!msix_is_masked(dev, vector)) {
> > > > -        r = dev->msix_mask_notifier(dev, vector, opaque, true);
> > > > +        r = dev->msix_mask_notifier(dev, vector, true);
> > > >          if (r < 0) {
> > > >              return r;
> > > >          }
> > > >      }
> > > > -    dev->msix_mask_notifier_opaque[vector] = NULL;
> > > > +    return r;
> > > > +}
> > > 
> > > The above need to be combined to a single function now since the only
> > > difference is s/true/false.
> > > 
> > > Alex
> > 
> > This is the way it was in the past, and it turned out to be very
> > confusing to read since both variables: mask and assign are bool but
> > polarity is reversed.
> > 
> > Unrolled it seems easier to grok.
> 
> You could always keep the functions as separate wrapper callers of the
> common function so you only need to keep true = unset, false = set
> straight in one place.  Thanks,
> 
> Alex

wrappers still make this confusing.
we had so many bugs here, I feel minor duplication
is worth it.

-- 
MST



reply via email to

[Prev in Thread] Current Thread [Next in Thread]