From mboxrd@z Thu Jan 1 00:00:00 1970 From: Uri Lublin Subject: Re: [PATCH] qemu: qemu_fopen_fd: differentiate between reader and writer user Date: Thu, 16 Oct 2008 02:13:58 +0200 Message-ID: <48F68746.8010904@il.qumranet.com> 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, Anthony Liguori To: Avi Kivity Return-path: Received: from il.qumranet.com ([212.179.150.194]:30745 "EHLO il.qumranet.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753775AbYJPAOA (ORCPT ); Wed, 15 Oct 2008 20:14:00 -0400 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 am not sure we are allowed to drop the buffer a on read-to-write switch, especially if there are unread bytes in it. We may need to pause the write that causes the switch, similar to pausing the read in order to flush the buffer on a write-to-read switch.