From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MabZK-0006Xj-Ki for qemu-devel@nongnu.org; Mon, 10 Aug 2009 16:34:10 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MabZF-0006XX-5R for qemu-devel@nongnu.org; Mon, 10 Aug 2009 16:34:09 -0400 Received: from [199.232.76.173] (port=33905 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MabZF-0006XU-12 for qemu-devel@nongnu.org; Mon, 10 Aug 2009 16:34:05 -0400 Received: from qw-out-1920.google.com ([74.125.92.144]:50249) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MabZE-0007PU-KJ for qemu-devel@nongnu.org; Mon, 10 Aug 2009 16:34:04 -0400 Received: by qw-out-1920.google.com with SMTP id 5so1144469qwc.4 for ; Mon, 10 Aug 2009 13:34:04 -0700 (PDT) Message-ID: <4A808437.8040307@codemonkey.ws> Date: Mon, 10 Aug 2009 15:33:59 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH] qemu/virtio: move features to an inline function References: <20090810192809.GA16800@redhat.com> <4A8076F0.5050700@codemonkey.ws> <20090810194753.GA16803@redhat.com> In-Reply-To: <20090810194753.GA16803@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: rusty@rustcorp.com.au, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org Michael S. Tsirkin wrote: >> Normally, the common features are transport features and the devices >> should have absolutely no knowledge of transport feature (since they're >> transport dependent). >> > > Good point. But > > 1. note that with my patch they don't. They call > virtio_get_common_features and that's all. > > 2. some features may not make sense for some devices. For example, it is > quite possible that indirect ring entries feature improves performance > on block but hurts on net, as net has a similar merged buffers feature. > Someone should try benchmarking with it disabled, and it becomes > possible with my patch. > I don't necessarily disagree but I think your patch goes about it the wrong way. There ought to be a way to layer qdev properties that achieves this goal so that when you create a virtio-pci-block device, you have the ability to turn off indirect sg without virtio-block having to know what that is. For your use-case, I wonder if you're integrating at the wrong level. If you implement a ring in-kernel, maybe the thing to do is introduce more layering in qemu like we have in the kernel so that you can easily add a new ring backend type. At any rate, see if you can achieve the same goal with qdev properties. If you could, you should be able to hack something up easily to disable this for vhost without completely overhauling qemu's virtio implementation. Regards, Anthony Liguori