From nobody Wed Apr 24 00:15:48 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VPKJy050rz5JMwj for ; Wed, 24 Apr 2024 00:16:02 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com [209.85.221.48]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VPKJx5QZLz3xNc for ; Wed, 24 Apr 2024 00:16:01 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-wr1-f48.google.com with SMTP id ffacd0b85a97d-3455ff1339dso4839254f8f.0 for ; Tue, 23 Apr 2024 17:16:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713917760; x=1714522560; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=nsc8o3gozv+Iz3FoGcfQRos0OPAlQoiRR0fzOIZ6ubg=; b=JE6kuHaSTmGen5qWEi90EbdolFe0KyTjwaSjNCxrh+5aRzweAte/HmScIguCragSFE mU39xOKa9pzTv8stX3A09B6oEkN/cVSXSQurBgPuT/ASY5C+fYghxKD0arPeMMd0Fgl7 hzl9ShtCIBIn82c3Hj5KFUJtEhWnnAR5+y0fXGtRNlt0frZ7+75D47LIFTb4WNWfhAlT c95WRF/Eg0bI85qKrZCuTYu6ylS1g2sMfkSjX96yJFWzSJjkTefmYoo3Kt7N9b60d6Wu l5Sxpu1h/BekGwczbQjbnC+qA6e4z1yx21RF5l4Bb+jJ3HLeUsqJ7lZc+mNgq0dbH/un yjoQ== X-Forwarded-Encrypted: i=1; AJvYcCVuTRCmyi3km0GbObkk78ZJlKZ3xJwBlOuGOofZYdMtb6rAgLDSTKRtBZot1N6OQJSA4wCNt0jaLL7t7761SJxI+ekfzwWxhcupphQmKIj1 X-Gm-Message-State: AOJu0Yzns0DYozOpstUzdVRp+5ANGXh3WkDEJ2m5rY0e2X/EMvX9xiVw bCyS+2ZvPEl9xEjBVglTgMPYJZTrwuD+f/zBhanhMsYkKF5Hf0o40rswViB7GuQ= X-Google-Smtp-Source: AGHT+IFw39ATGw8OmoD+slq1tR/eEa8+q6KYigqxrRV/uqiaYdPD74xM3yDGFcp2NU8oJx9PIyxZNg== X-Received: by 2002:a5d:4b84:0:b0:34b:88e:3712 with SMTP id b4-20020a5d4b84000000b0034b088e3712mr618494wrt.60.1713917759648; Tue, 23 Apr 2024 17:15:59 -0700 (PDT) Received: from smtpclient.apple ([131.111.5.246]) by smtp.gmail.com with ESMTPSA id e18-20020a056000195200b003462fec9f5asm15627382wry.62.2024.04.23.17.15.58 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 23 Apr 2024 17:15:58 -0700 (PDT) Content-Type: text/plain; charset=utf-8 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.200.91.1.1\)) Subject: Re: git: cf04a7775a4e - main - swapon: Do not overwrite Linux swap header From: Jessica Clarke In-Reply-To: <202404232150.43NLoEsI087796@gitrepo.freebsd.org> Date: Wed, 24 Apr 2024 01:15:48 +0100 Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: References: <202404232150.43NLoEsI087796@gitrepo.freebsd.org> To: Warner Losh X-Mailer: Apple Mail (2.3774.200.91.1.1) X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US] X-Rspamd-Queue-Id: 4VPKJx5QZLz3xNc On 23 Apr 2024, at 22:50, Warner Losh wrote: > The branch main has been updated by imp: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3Dcf04a7775a4e8ff6fd28c768be9daa3d= 83dd382e >=20 > commit cf04a7775a4e8ff6fd28c768be9daa3d83dd382e > Author: Ricardo Branco > AuthorDate: 2024-04-23 21:47:56 +0000 > Commit: Warner Losh > CommitDate: 2024-04-23 21:48:01 +0000 >=20 > swapon: Do not overwrite Linux swap header I don=E2=80=99t think this is a sensible default. swapon should, by = default, do exactly what you asked for, rather than magically do something more convoluted and different, with no way to opt out of it. The fact that it looks like some other random OS happened to touch the partition in the past shouldn=E2=80=99t be relevant; we=E2=80=99re not Linux, we=E2=80=99= re FreeBSD. Especially in the age of GPT where the OSes have different GUIDs for their swap partitions. Having some user-friendly way to the gnop dance seems fine, but it should be optional and off by default, not forced upon you. As it stands the only way to avoid this silent behaviour change is to zero out the first 4K of the partition, which is not user-friendly (and starts to sound like going back to the old days when that was a common thing to do when partitioning). I also don=E2=80=99t believe the patch to be uncontested as you state in = your comment on the PR; kib@ had last commented on 1083 that it should be documented and left up to the user to configure. Jess > Reviewed by: imp, jhb > Pull Request: https://github.com/freebsd/freebsd-src/pull/1084 > --- > sbin/swapon/Makefile | 2 ++ > sbin/swapon/extern.h | 5 ++++ > sbin/swapon/swaplinux.c | 66 = +++++++++++++++++++++++++++++++++++++++++++++++++ > sbin/swapon/swapon.c | 54 +++++++++++++++++++++++++++++++++++++++- > 4 files changed, 126 insertions(+), 1 deletion(-) >=20 > diff --git a/sbin/swapon/Makefile b/sbin/swapon/Makefile > index 27808aed5857..930e938e5b05 100644 > --- a/sbin/swapon/Makefile > +++ b/sbin/swapon/Makefile > @@ -6,6 +6,8 @@ LINKS=3D ${BINDIR}/swapon ${BINDIR}/swapoff > LINKS+=3D ${BINDIR}/swapon ${BINDIR}/swapctl > MLINKS=3D swapon.8 swapoff.8 > MLINKS+=3Dswapon.8 swapctl.8 > +SRCS=3D swapon.c swaplinux.c > +HDRS+=3D extern.h >=20 > LIBADD=3D util >=20 > diff --git a/sbin/swapon/extern.h b/sbin/swapon/extern.h > new file mode 100644 > index 000000000000..cb33dbbee953 > --- /dev/null > +++ b/sbin/swapon/extern.h > @@ -0,0 +1,5 @@ > +/*- > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +int is_linux_swap(const char *); > diff --git a/sbin/swapon/swaplinux.c b/sbin/swapon/swaplinux.c > new file mode 100644 > index 000000000000..d01a94345e68 > --- /dev/null > +++ b/sbin/swapon/swaplinux.c > @@ -0,0 +1,66 @@ > +/*- > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +#include "extern.h" > + > +/* > + * Definitions and structure taken from > + * = https://github.com/util-linux/util-linux/blob/master/include/swapheader.h > + */ > + > +#define SWAP_VERSION 1 > +#define SWAP_UUID_LENGTH 16 > +#define SWAP_LABEL_LENGTH 16 > +#define SWAP_SIGNATURE "SWAPSPACE2" > +#define SWAP_SIGNATURE_SZ (sizeof(SWAP_SIGNATURE) - 1) > + > +struct swap_header_v1_2 { > + char bootbits[1024]; /* Space for disklabel etc. */ > + uint32_t version; > + uint32_t last_page; > + uint32_t nr_badpages; > + unsigned char uuid[SWAP_UUID_LENGTH]; > + char volume_name[SWAP_LABEL_LENGTH]; > + uint32_t padding[117]; > + uint32_t badpages[1]; > +}; > + > +typedef union { > + struct swap_header_v1_2 header; > + struct { > + uint8_t reserved[4096 - SWAP_SIGNATURE_SZ]; > + char signature[SWAP_SIGNATURE_SZ]; > + } tail; > +} swhdr_t; > + > +#define sw_version header.version > +#define sw_volume_name header.volume_name > +#define sw_signature tail.signature > + > +int > +is_linux_swap(const char *name) > +{ > + uint8_t buf[4096]; > + swhdr_t *hdr =3D (swhdr_t *) buf; > + int fd; > + > + fd =3D open(name, O_RDONLY); > + if (fd =3D=3D -1) > + return (-1); > + > + if (read(fd, buf, 4096) !=3D 4096) { > + close(fd); > + return (-1); > + } > + close(fd); > + > + return (hdr->sw_version =3D=3D SWAP_VERSION && > + !memcmp(hdr->sw_signature, SWAP_SIGNATURE, SWAP_SIGNATURE_SZ)); > +} > diff --git a/sbin/swapon/swapon.c b/sbin/swapon/swapon.c > index 26a7dc22654a..0f2ad1f5e213 100644 > --- a/sbin/swapon/swapon.c > +++ b/sbin/swapon/swapon.c > @@ -54,10 +54,15 @@ > #include > #include >=20 > +#include "extern.h" > + > +#define _PATH_GNOP "/sbin/gnop" > + > static void usage(void) __dead2; > static const char *swap_on_off(const char *, int, char *); > static const char *swap_on_off_gbde(const char *, int); > static const char *swap_on_off_geli(const char *, char *, int); > +static const char *swap_on_off_linux(const char *, int); > static const char *swap_on_off_md(const char *, char *, int); > static const char *swap_on_off_sfile(const char *, int); > static void swaplist(int, int, int); > @@ -250,8 +255,13 @@ swap_on_off(const char *name, int doingall, char = *mntops) > return (swap_on_off_geli(name, mntops, doingall)); > } >=20 > - /* Swap on special file. */ > free(basebuf); > + > + /* Linux swap */ > + if (is_linux_swap(name)) > + return (swap_on_off_linux(name, doingall)); > + > + /* Swap on special file. */ > return (swap_on_off_sfile(name, doingall)); > } >=20 > @@ -466,6 +476,48 @@ swap_on_off_geli(const char *name, char *mntops, = int doingall) > return (swap_on_off_sfile(name, doingall)); > } >=20 > +static const char * > +swap_on_off_linux(const char *name, int doingall) > +{ > + const char *ret; > + char *nopname; > + size_t nopnamelen; > + int error; > + > + if (which_prog =3D=3D SWAPON) { > + /* Skip the header for Linux swap partitions */ > + error =3D run_cmd(NULL, "%s create -o 4096 %s", _PATH_GNOP, > + name); > + if (error) { > + warnx("gnop (create) error: %s", name); > + return (NULL); > + } > + } > + > + /* Append ".nop" to name */ > + nopnamelen =3D strlen(name) + sizeof(".nop"); > + nopname =3D (char *) malloc(nopnamelen); > + if (nopname =3D=3D NULL) > + err(1, "malloc()"); > + (void)strlcpy(nopname, name, nopnamelen); > + (void)strlcat(nopname, ".nop", nopnamelen); > + > + ret =3D swap_on_off_sfile(nopname, doingall); > + > + if (which_prog =3D=3D SWAPOFF) { > + error =3D run_cmd(NULL, "%s destroy %s", _PATH_GNOP, nopname); > + if (error) { > + warnx("gnop (destroy) error: %s", name); > + free(nopname); > + return (NULL); > + } > + } > + > + free(nopname); > + > + return (ret); > +} > + > static const char * > swap_on_off_md(const char *name, char *mntops, int doingall) > {