Re: [Qemu-devel] [PATCH v2 1/2] qga: flush explicitly when needed

[Eric Blake]

On 11/24/2015 03:19 PM, Michael Roth wrote:

>> Hmm.  This always attempts fseek() on the first write() to a file, even
>> if the file is not also open for read.  While guest-file-open is most
>> likely used on regular files (and therefore seekable), I'm worried that
>> we might have a client that is attempting to use it on terminal files or
>> other non-seekable file names.  Since the fseek() on first write is
>> unconditional, that means we would now fail to let a user write to such
>> a file, even if they could previously do so.  Should we add more logic
>> to only do the fseek() after a previous write (as in a tri-state
>> variable of untouched, last written, last read), so that we aren't
>> breaking one-pass usage of non-seekable files?
> 
> I think that would be prudent. !gfh->writing doesn't imply gfh->reading,
> and failed calls to qmp_guest_file_read() should probably not set
> gfh->writing to true either.
> 
> Maybe something like:
> 
> gfh->rw_state = RW_STATE_NEW | RW_STATE_READING | RW_STATE_WRITING, skip the
> fseek()/fflush() on RW_NEW, and only move to RW_STATE_READING/WRITING when
> fread()/fwrite(), respectively, actually succeed?

Additionally, if guest-file-seek succeeds, reset state to RW_STATE_NEW
(any user-triggered seek satisfies the requirement so that we don't need
our implicit seek).

> 
> I guess that still leaves the possibility of writes failing after reads
> on non-seekable files. Are such files always directional? Otherwise that
> means there are cases where it's impossible to implement the
> requirements of the spec.

Remember, /dev/tty is commonly opened for both reading and writing - but
via separate fds (you read from fd 0, write to fd 1) - and it is the
console or terminal emulator that sets up the initial things (you seldom
see scripts directly attempting a bi-directional 'exec <> /dev/tty'.  I
think we'll be okay on that front.  Or if we're paranoid, then have
guest-file-seek inspect errno - if seek failed due to ESPIPE, then the
file is non-seekable, stdio shouldn't be buffering anyways, and we can
just blindly force RW_STATE_NEW in that case (the real reason for
fflush/fseek between read/write transitions is for seekable files).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature