From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202410 header.b=cRbEc41J; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 9900E5A004C for ; Tue, 29 Oct 2024 10:34:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202410; t=1730194434; bh=UbgCPLs3AIN3V4d++ntb3B/9g7WMZqv+Wwauy5lmrzo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=cRbEc41JOE07CKgavC8SRj24sLI6u0Q7y/Nak2fcm3VsFpHtLYtmJtDKCMbOeguWD xQIi++68Xt/v1d1aGa6o3+D3fj+ge+FDLKDbD722sRN42PtHI3O2WzRBFUI0eziDuI LaPWcecJn1QZFQW0/d69NotPpEjlw5fP8IE9Soc5dEog0v3MsDSXFjtHaFiMU56OwW RmsMLpTk3/tBsViZVblc8ynVBImXfPU0BgVbprecUMwNW6DQVl5n1vEKGA1aOoBg18 3ZBrg3HdIFC3LtXMJcmd4rjcdZ5M+UnOeDKk0ffh8bJY2T9X1S2y9J2Cm32eXN5R5O CBPMdTY2zLVNg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Xd4nt4pfxz4x7D; Tue, 29 Oct 2024 20:33:54 +1100 (AEDT) Date: Tue, 29 Oct 2024 20:32:40 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 5/9] log: Don't use O_APPEND at all Message-ID: References: <20241028100044.939714-1-sbrivio@redhat.com> <20241028100044.939714-6-sbrivio@redhat.com> <20241029094850.206c06bc@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Opa/CM9VO55S+w1E" Content-Disposition: inline In-Reply-To: <20241029094850.206c06bc@elisabeth> Message-ID-Hash: VUJ4MC33YOOA3P5XVIESV33JSBTGKMKZ X-Message-ID-Hash: VUJ4MC33YOOA3P5XVIESV33JSBTGKMKZ X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --Opa/CM9VO55S+w1E Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 29, 2024 at 09:48:50AM +0100, Stefano Brivio wrote: > On Tue, 29 Oct 2024 15:20:56 +1100 > David Gibson wrote: >=20 > > On Mon, Oct 28, 2024 at 11:00:40AM +0100, Stefano Brivio wrote: > > > We open the log file with O_APPEND, but switch it off before seeking, > > > and turn it back on afterwards. > > >=20 > > > We never seek when O_APPEND is on, so we don't actually need it, as > > > its only function is to override the offset for writes so that they > > > are always performed at the end regardless of the current offset > > > (which is at the end anyway, for us). =20 > >=20 > > Sorry, this sounded fishy to me on the call, but I figured I was just > > missing something. But looking at this the reasoning doesn't make > > sense to me. > >=20 > > We don't seek with O_APPEND, but we do write(), which is exactly where > > it matters. AIUI the point of O_APPEND is that if you have multiple > > processes writing to the same file, they won't clobber each others > > writes because of a stale file pointer. >=20 > That's not the reason why I originally added it though: it was there > because I thought I would lseek() to do the rotation and possibly end > up with the cursor somewhere before the end. Then restart writing, and > the write would happen in the middle of the file: I don't entirely follow. I see why you disable O_APPEND across the rotation, but I'm not clear on why it's opened with O_APPEND in the first place, if it's not for the typical logging reason. > $ cat append.c > #include > #include > #include > #include >=20 > int main(int argc, char **argv) > { > int flags =3D O_CREAT | O_TRUNC | O_WRONLY | ((argc =3D=3D 3) ? O_APPEND= : 0); > int fd =3D open(argv[1], flags, S_IRUSR | S_IWUSR); > char buf[BUFSIZ]; >=20 > memset(buf, 'a', BUFSIZ); > write(fd, buf, 10); > lseek(fd, 1, SEEK_SET); > memset(buf, 'b', BUFSIZ); > write(fd, buf, 10); > write(fd, (char *){ "\n" }, 1); >=20 > return 0; > } > $ gcc -o append{,.c} > $ ./append test append > $ cat test > aaaaaaaaaabbbbbbbbbb > $ ./append test > $ cat test > abbbbbbbbbb >=20 > > That's usually not > > _necessary_ for us as such, but it's perhaps valuable since it reduces > > the likelihood of data loss if somehow you do get two instances > > logging to the same file. >=20 > The result will be completely unreadable anyway, so I don't think it > matters for us. Not necessarily. It certainly can get garbled, but individual writes of reasonable size - such as a single log line will generally complete atomically. With a text logging format, that's not ideal but often pretty decipherable. Particularly if each writer includes a prefix identifying itself. > > Of course the rotation process *can* clobber things (which is exactly > > why I was always a bit sceptical of this "in place" rotation, not that > > we really have other options). >=20 > Why would it clobber things? logfile_rotate_fallocate() and > logfile_rotate_move() take care of cutting cleanly at a line boundary, > and tests check that. I mean that in the case that there are multiple writers, the rotation breaks that "no data loss, and probably readable-ish" property of O_APPEND. --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --Opa/CM9VO55S+w1E Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmcgq7gACgkQzQJF27ox 2GfLng/+IFHAEPqbTTcnte530I7GgKC5q7lSx280gzUN+jAyOVCL5IIBc5B9jGDa hq0R0bvPpsFAgvnsgFo7BSBvYsPBvNV76Nrr9b+s63NzkFMsxd9Du19iTGsaBlon XXRxdC/0Vz48KUT+a31ADD01rvA9dDdINdT/uimBkmvTNBbtQaHrr6aDNouRQJkH a0RvQ9LPLGxbcrhEDqALPlPYTUYVfUZh8WrT4gr6+BL3SiO6YmZsfwaRYuiDfhXC qzA7YuSiDJ0nkK1E2EKjc+r7CvdBdUUBLPE0aDtDOhjnxLPJa+At3hlMYEWWI6AF ME4FUau2R5QpxFVr9fnmq/6aeHhX8RFC5ieSW4oxmVZ7EFe2givXjmGDwadz9oZM tXTIRSw0CIG6NEawj2NsTwBn6/RosP4h4FHh4NvOuEUXk3PIKy5Li5/ZguH1oiOW Ox5i4HHZNuABM0vPs+IUP8hO/qIbsq/2rXKNu16EMnKyR6VfzZjoWzfXUrrQlORI rpfk2JdNWB6orgM6Ki0WAronJkLZmx+APqI2VkE7nbK2ONZcRNXCSOZ9t+VHDOXj 6cILW/UTEls/yBF+YWl4ZsH7uW9jsIO+8ts+fLCYfm1phClj3hyxUHPcRDT58+kD AEpfeF1Y1MhfHawWz75vn95NrwPkyFoROqiQwb+N6ZwV1fZn9tA= =v24p -----END PGP SIGNATURE----- --Opa/CM9VO55S+w1E--