On Thu, Mar 18, 2010 at 12:40:36PM +0100, Juan Quintela wrote: > "Michael S. Tsirkin" <m...@redhat.com> wrote: > > On Thu, Mar 18, 2010 at 09:59:03AM +0100, Juan Quintela wrote: > >> "Michael S. Tsirkin" <m...@redhat.com> wrote: > >> > On Thu, Mar 18, 2010 at 09:26:10AM +0100, Juan Quintela wrote: > >> >> "Michael S. Tsirkin" <m...@redhat.com> 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 <quint...@redhat.com> > >> >> > > >> >> > 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