[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/2] qga: flush explicitly when needed
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/2] qga: flush explicitly when needed |
Date: |
Tue, 24 Nov 2015 15:28:33 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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