From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NF4HY-00088N-1j for qemu-devel@nongnu.org; Mon, 30 Nov 2009 06:19:04 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NF4HT-00084E-28 for qemu-devel@nongnu.org; Mon, 30 Nov 2009 06:19:03 -0500 Received: from [199.232.76.173] (port=51053 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NF4HS-000846-TB for qemu-devel@nongnu.org; Mon, 30 Nov 2009 06:18:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:9636) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NF4HS-0000l1-Ji for qemu-devel@nongnu.org; Mon, 30 Nov 2009 06:18:58 -0500 Date: Mon, 30 Nov 2009 13:16:15 +0200 From: "Michael S. Tsirkin" Message-ID: <20091130111615.GA12924@redhat.com> References: <20091124194502.GA4250@redhat.com> <4B0C3901.5010100@linux.vnet.ibm.com> <20091124195458.GA4290@redhat.com> <4B0C4A6F.3000305@linux.vnet.ibm.com> <20091124213000.GA4614@redhat.com> <4B0C56DC.4010608@linux.vnet.ibm.com> <20091124222005.GA4724@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091124222005.GA4724@redhat.com> Subject: [Qemu-devel] Re: [PATCH] qemu/virtio-net: remove wrong s/g layout assumptions List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Shirley Ma , Rusty Russell , qemu-devel@nongnu.org On Wed, Nov 25, 2009 at 12:20:05AM +0200, Michael S. Tsirkin wrote: > On Tue, Nov 24, 2009 at 03:57:48PM -0600, Anthony Liguori wrote: > > Michael S. Tsirkin wrote: > >> It's useful because this way I won't have to maintain the fix, and it > >> will make it possible for guests to experiment with layouts, without > >> hacking qemu. If someone wants to make it a product, that's a different > >> thing. > >> > > > > Advertising a new feature is not hard. It's one line of code in qemu > > with Rusty's ACK. > > Okay. Rusty, ACK? > > >> Also, it might be a valid thing for a guest to say that host needs to be > >> fixed. Not everyone might care about running on any possible broken qemu > >> version. > >> > >> Finally - where do we draw the line? Does any bugfix need a feature bit? > >> > > > > This is a spec bug, not a qemu bug IMHO. > > > > The kernel drivers have always behaved this way and when this was all > > written originally, the semantics were never defined. All the userspace > > implementations relied on this. The fact that the spec claims a > > different behavior is a result of deciding that it should be different > > after the fact. > > > > Thinking about it more, your patch is broken. If you run it against a > > really old guest, it will break badly. > > From what I can tell, my patch will behave exactly as > existing code does unless guest puts more data than virtio net hdr > size in the first element. In that last case, qemu called exit(1) > and I handle it properly. > > > You assume that the guest header is always a fixed size. It's not, > > we've added fields as we've added feature bits. > > Hmm. How so? Look at old qemu code I am replacing. qemu > just exits if header size is different from sizeof virtio_net_hdr. > > > You actually have to > > look at the set of Ack'd features bits to determine how large the header > > is. > > I think you are speaking about the mergeable header thing. > If you look at the code, I think you will see that I handle this > case correctly. > > > Which is why I now remember why this has never changed. It's a PITA :-) > > I'm not even sure you can do it in a correct way. > > I think I got it right. If not, let me know which feature is handled > wrong please. Anthony could you comment please? I think the patch is correct, and I even tested on a relatively old guest. If you still think it is wrong, please tell me why. > > -- > > Regards, > > > > Anthony Liguori