[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test |
Date: |
Thu, 18 Mar 2010 15:24:45 +0200 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
On Thu, Mar 18, 2010 at 12:40:36PM +0100, Juan Quintela wrote:
> "Michael S. Tsirkin" <address@hidden> wrote:
> > On Thu, Mar 18, 2010 at 09:59:03AM +0100, Juan Quintela wrote:
> >> "Michael S. Tsirkin" <address@hidden> wrote:
> >> > On Thu, Mar 18, 2010 at 09:26:10AM +0100, Juan Quintela wrote:
> >> >> "Michael S. Tsirkin" <address@hidden> wrote:
> >> >> > On Tue, Mar 16, 2010 at 07:51:22PM +0100, Juan Quintela wrote:
> >> >> >> We already do the test for msix on the caller, just use that test
> >> >> >>
> >> >> >> Signed-off-by: Juan Quintela <address@hidden>
> >> >> >
> >> >> > NAK
> >> >> >
> >> >> > I think we are better off not making assumptions
> >> >> > about caller behaviour in msix.c, virtio
> >> >> > will not be the only user forever.
> >> >>
> >> >> That makes migration testing more difficult. Basically we are testing
> >> >> if we are using msix in two places. Obvious thing is:
> >> >> - we don't test in msix_save() if msix is used.
> >> >> - we don't test it in virtio_pci_save_config()
> >> >>
> >> >> I don't care if it is one way or another, but requiring to check it in
> >> >> the caller and the callee is a bit too much for me.
> >> >>
> >> >> Later, Juan.
> >> >
> >> > msix does not require the check in the caller, by design it is
> >> > safe to call msix_save when msix is not present.
> >>
> >> look at it, it requires to test msix support for other things, which
> >> amount to the same thing :(
> >>
> >> Later, Juan.
> >
> > Yes, but it does not have to be the only user. We'll have at least pci
> > assignment use it, at some point. IOW, msix tries to present an API
> > that is safe to use, and checks capability so we do not crash if it's
> > not there. virtio happens to need the same test for its own reasons
> > (vmsave format backwards compatibility).
> >
> > We could optimize the test if we cared about speed here, but we don't
> > ...
>
> It makes my vmstate work more complex, because this is the only user
> that calls a vvnmstat* and uses _nothing_ of it.
>
> rest of users are something like:
>
> VMSTATE(...., test_function);
>
> on the caller side. As virtio already needs to know msix use for other
> reasons, I really expect that anything else that uses msix would need
> the same test for the same reason.
>
> Later, Juan.
Well, ok. If you add assumptions about callers please add a comment
before function thought,
--
MST
- [Qemu-devel] [PATCH 5/9] virtio: Use DO_UPCAST instead of a cast, (continued)
- [Qemu-devel] [PATCH 5/9] virtio: Use DO_UPCAST instead of a cast, Juan Quintela, 2010/03/16
- [Qemu-devel] [PATCH 7/9] QLIST: Introduce QLIST_COPY_HEAD, Juan Quintela, 2010/03/16
- [Qemu-devel] [PATCH 6/9] virtio-pci: Remove duplicate test, Juan Quintela, 2010/03/16
- [Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test, Michael S. Tsirkin, 2010/03/18
- [Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test, Juan Quintela, 2010/03/18
- [Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test, Michael S. Tsirkin, 2010/03/18
- [Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test, Juan Quintela, 2010/03/18
- [Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test, Michael S. Tsirkin, 2010/03/18
- [Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test, Juan Quintela, 2010/03/18
- [Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test,
Michael S. Tsirkin <=
- [Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test, Juan Quintela, 2010/03/18
[Qemu-devel] [PATCH 4/9] virtio: Teach virtio-net about DO_UPCAST, Juan Quintela, 2010/03/16
[Qemu-devel] [PATCH 8/9] virtio-blk: change rq type to VirtIOBlockReq, Juan Quintela, 2010/03/16
[Qemu-devel] [PATCH 9/9] virtio-blk: use QLIST for the list of requests, Juan Quintela, 2010/03/16
[Qemu-devel] Re: [PATCH 0/9] Virtio cleanups, Michael S. Tsirkin, 2010/03/18