[Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test

[Juan Quintela]

"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.