From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [PATCH] qemu: qemu_fopen_fd: differentiate between reader and writer user Date: Sun, 12 Oct 2008 13:09:17 -0500 Message-ID: <48F23D4D.2050709@codemonkey.ws> References: <1223829030-14962-1-git-send-email-uril@qumranet.com> <48F22BF1.3000608@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Uri Lublin , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from yx-out-2324.google.com ([74.125.44.28]:6823 "EHLO yx-out-2324.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754561AbYJLSJW (ORCPT ); Sun, 12 Oct 2008 14:09:22 -0400 Received: by yx-out-2324.google.com with SMTP id 8so343561yxm.1 for ; Sun, 12 Oct 2008 11:09:20 -0700 (PDT) In-Reply-To: <48F22BF1.3000608@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Avi Kivity wrote: > Uri Lublin wrote: >> Currently qemu_fopen_ops accepts both get_buffer and put_buffer, but >> if both are given (non NULL) we encounter problems: >> 1. There is only one buffer and index, which may mean data corruption. >> 2. qemu_flush (which is also called by qemu_fclose) is writing >> ("flushing") >> some of the data that was read (for the reader part). >> >> Currently qemu_fopen_fd registers both get_buffer and put_buffer >> functions. >> >> This breaks migration for tcp and ssh migration protocols. >> >> The following patch fix the above by: >> 1. It makes sure that at most one of get_buffer and put_buffer is >> given to qemu_fopen_ops. >> 2. It changes qemu_fopen_fd to register only get_buffer for a reader >> and only put_buffer for a writer (adding a 'reader' parameter). >> 3. The incoming fd migration code calls qemu_fopen_fd as a reader >> only. >> >> > > Anthony, this is a problem with qemu-upstream so I'd like to solve it > in a way that's acceptable for upstream. > > The proposed patch is less that ideal IMO as it introduces limitations > on what you can do with a file. An alternative implementation would > add a read/write mode to the buffer, based on the last access type. > When switching from read to write, we drop the buffer, and when > switching from write to read, we flush it and then drop it. This is > more complex but results in a cleaner API. I would think a better solution would introduce two buffers, one for read and one for write. That way, you can have a proper bidirectional stream. Regards, Anthony Liguori