From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NsFlm-0000qQ-KL for qemu-devel@nongnu.org; Thu, 18 Mar 2010 09:28:14 -0400 Received: from [199.232.76.173] (port=53831 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NsFll-0000p4-Ra for qemu-devel@nongnu.org; Thu, 18 Mar 2010 09:28:13 -0400 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NsFlk-0003Jx-CA for qemu-devel@nongnu.org; Thu, 18 Mar 2010 09:28:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53921) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NsFlj-0003Jt-VF for qemu-devel@nongnu.org; Thu, 18 Mar 2010 09:28:12 -0400 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o2IDSAjn022337 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 18 Mar 2010 09:28:10 -0400 Date: Thu, 18 Mar 2010 15:24:45 +0200 From: "Michael S. Tsirkin" Message-ID: <20100318132445.GA27478@redhat.com> References: <20100318072529.GB16973@redhat.com> <20100318084707.GA23649@redhat.com> <20100318091106.GC23649@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: [Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org On Thu, Mar 18, 2010 at 12:40:36PM +0100, Juan Quintela wrote: > "Michael S. Tsirkin" wrote: > > On Thu, Mar 18, 2010 at 09:59:03AM +0100, Juan Quintela wrote: > >> "Michael S. Tsirkin" wrote: > >> > On Thu, Mar 18, 2010 at 09:26:10AM +0100, Juan Quintela wrote: > >> >> "Michael S. Tsirkin" 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 > >> >> > > >> >> > 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