From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42705) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1KdQ-0000zO-Mb for qemu-devel@nongnu.org; Tue, 24 Nov 2015 15:56:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1KdN-0002Vw-CR for qemu-devel@nongnu.org; Tue, 24 Nov 2015 15:56:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38131) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1KdN-0002Vo-5H for qemu-devel@nongnu.org; Tue, 24 Nov 2015 15:56:17 -0500 References: <1448388281-18691-1-git-send-email-marcandre.lureau@redhat.com> <1448388281-18691-2-git-send-email-marcandre.lureau@redhat.com> From: Eric Blake Message-ID: <5654CEEF.1040102@redhat.com> Date: Tue, 24 Nov 2015 13:56:15 -0700 MIME-Version: 1.0 In-Reply-To: <1448388281-18691-2-git-send-email-marcandre.lureau@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="e4tQO8fs0B8uLhewJlGq6SUPhJoJJRr32" Subject: Re: [Qemu-devel] [PATCH v2 1/2] qga: flush explicitly when needed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com, qemu-devel@nongnu.org Cc: mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --e4tQO8fs0B8uLhewJlGq6SUPhJoJJRr32 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/24/2015 11:04 AM, marcandre.lureau@redhat.com wrote: > From: Marc-Andr=C3=A9 Lureau >=20 > According to the specification: > http://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html >=20 > "the application shall ensure that output is not directly followed by > input without an intervening call to fflush() or to a file positioning > function (fseek(), fsetpos(), or rewind()), and input is not directly > followed by output without an intervening call to a file positioning > function, unless the input operation encounters end-of-file." >=20 > Without this change, a write() followed by a read() may lose the > previously written content, as shown in the following test. >=20 > Fixes: > https://bugzilla.redhat.com/show_bug.cgi?id=3D1210246 >=20 > Signed-off-by: Marc-Andr=C3=A9 Lureau > --- > qga/commands-posix.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) >=20 > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 0ebd473..d0228ce 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -219,6 +219,7 @@ void qmp_guest_set_time(bool has_time, int64_t time= _ns, Error **errp) > typedef struct GuestFileHandle { > uint64_t id; > FILE *fh; > + bool writing; > QTAILQ_ENTRY(GuestFileHandle) next; > } GuestFileHandle; > =20 > @@ -460,6 +461,17 @@ struct GuestFileRead *qmp_guest_file_read(int64_t = handle, bool has_count, > } > =20 > fh =3D gfh->fh; > + > + /* explicitly flush when switching from writing to reading */ > + if (gfh->writing) { > + int ret =3D fflush(fh); > + if (ret =3D=3D EOF) { > + error_setg_errno(errp, errno, "failed to flush file"); > + return NULL; > + } > + gfh->writing =3D false; > + } > + > buf =3D g_malloc0(count+1); > read_count =3D fread(buf, 1, count, fh); > if (ferror(fh)) { > @@ -496,6 +508,16 @@ GuestFileWrite *qmp_guest_file_write(int64_t handl= e, const char *buf_b64, > } > =20 > fh =3D gfh->fh; > + > + if (!gfh->writing) { > + int ret =3D fseek(fh, 0, SEEK_CUR); > + if (ret =3D=3D -1) { > + error_setg_errno(errp, errno, "failed to seek file"); > + return NULL; > + } > + gfh->writing =3D true; > + } 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? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --e4tQO8fs0B8uLhewJlGq6SUPhJoJJRr32 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWVM7vAAoJEKeha0olJ0NqKcYH/jNFHO+5rOVZEBUOkRJTOKXk ou1Mahi5UcTsZft8rMXwGxa0oQQdP8VN9OOUFZsJUMx6c32KJLTpAOhS5WDJvj42 E0qfJ8hS73f7FBOvVX4t2IkiyvKHOv12BzCwYvBc0FSmFNvonldqyapAH+xXJc9W oF4EZUvhnT6yXWFoyEtoom+Ej6QqrXSBJ+NxHZHmljOQzhw8iN6w0hNDIJ3VJ3s4 jsu7lMTInhnUeZqWhzKZzVeYiVhIygduA96fnqmPbwBveeVwRwRI0722TENglNth zzp41w001quU45fhEa2tqfyITqU1nTUTLcvleXABo+ZrX/sbySzFrMfNuUbQxjg= =a+OL -----END PGP SIGNATURE----- --e4tQO8fs0B8uLhewJlGq6SUPhJoJJRr32--