| Message ID | 20210413221051.2600455-4-naohiro.aota@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | implement zone-aware probing/wiping for zoned btrfs | expand |
On 2021/04/14 7:11, Naohiro Aota wrote: > We cannot overwrite superblock magic in a sequential required zone. So, > wipefs cannot work as it is. Instead, this commit implements the wiping by > zone resetting. > > Zone resetting must be done only for a sequential write zone. This is > checked by is_conventional(). > > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> > --- > libblkid/src/probe.c | 65 ++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 59 insertions(+), 6 deletions(-) > > diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c > index 102766e57aa0..7454a14bdfe6 100644 > --- a/libblkid/src/probe.c > +++ b/libblkid/src/probe.c > @@ -107,6 +107,7 @@ > #include <stdint.h> > #include <stdarg.h> > #include <limits.h> > +#include <stdbool.h> > > #include "blkidP.h" > #include "all-io.h" > @@ -1225,6 +1226,40 @@ int blkid_do_probe(blkid_probe pr) > return rc; > } > > +static int is_conventional(blkid_probe pr, uint64_t offset) > +{ > + struct blk_zone_report *rep = NULL; > + size_t rep_size; > + bool conventional; > + int ret; > + > + if (!pr->zone_size) > + return 1; > + > + rep_size = sizeof(struct blk_zone_report) + sizeof(struct blk_zone); > + rep = malloc(rep_size); > + if (!rep) > + return -1; > + > + memset(rep, 0, rep_size); Use calloc() to get the zeroing done at the same time as allocation, may be ? > + rep->sector = (offset & pr->zone_size) >> 9; Why the "& pr->zone_size" ? This seems wrong. To get the zone info of the zone containing offset, you can just have: rep->sector = offset >> 9; And if you want to align to the zone start, then: rep->sector = (offset & (~(pr->zone_size - 1))) >> 9; > + rep->nr_zones = 1; > + ret = ioctl(blkid_probe_get_fd(pr), BLKREPORTZONE, rep); > + if (ret) { > + free(rep); > + return -1; > + } > + > + if (rep->zones[0].type == BLK_ZONE_TYPE_CONVENTIONAL) > + ret = 1; > + else > + ret = 0; > + > + free(rep); > + > + return ret; > +} > + > /** > * blkid_do_wipe: > * @pr: prober > @@ -1264,6 +1299,7 @@ int blkid_do_wipe(blkid_probe pr, int dryrun) > const char *off = NULL; > size_t len = 0; > uint64_t offset, magoff; > + bool conventional; > char buf[BUFSIZ]; > int fd, rc = 0; > struct blkid_chain *chn; > @@ -1299,6 +1335,11 @@ int blkid_do_wipe(blkid_probe pr, int dryrun) > if (len > sizeof(buf)) > len = sizeof(buf); > > + rc = is_conventional(pr, offset); > + if (rc < 0) > + return rc; > + conventional = rc == 1; > + > DBG(LOWPROBE, ul_debug( > "do_wipe [offset=0x%"PRIx64" (%"PRIu64"), len=%zu, chain=%s, idx=%d, dryrun=%s]\n", > offset, offset, len, chn->driver->name, chn->idx, dryrun ? "yes" : "not")); > @@ -1306,13 +1347,25 @@ int blkid_do_wipe(blkid_probe pr, int dryrun) > if (lseek(fd, offset, SEEK_SET) == (off_t) -1) > return -1; > > - memset(buf, 0, len); > - > if (!dryrun && len) { > - /* wipen on device */ > - if (write_all(fd, buf, len)) > - return -1; > - fsync(fd); > + if (conventional) { > + memset(buf, 0, len); > + > + /* wipen on device */ > + if (write_all(fd, buf, len)) > + return -1; > + fsync(fd); > + } else { > + struct blk_zone_range range = { > + (offset & pr->zone_size) >> 9, > + pr->zone_size >> 9, > + }; Please add the field names for clarity and fix the sector position (it looks very wrong): struct blk_zone_range range = { .sector = (offset & (~(pr->zone_size - 1))) >> 9, .nr_sectors = pr->zone_size >> 9, }; > + > + rc = ioctl(fd, BLKRESETZONE, &range); > + if (rc < 0) > + return -1; > + } > + > pr->flags &= ~BLKID_FL_MODIF_BUFF; /* be paranoid */ > > return blkid_probe_step_back(pr); >
On Tue, Apr 13, 2021 at 10:47:00PM +0000, Damien Le Moal wrote: > On 2021/04/14 7:11, Naohiro Aota wrote: > > We cannot overwrite superblock magic in a sequential required zone. So, > > wipefs cannot work as it is. Instead, this commit implements the wiping by > > zone resetting. > > > > Zone resetting must be done only for a sequential write zone. This is > > checked by is_conventional(). > > > > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> > > --- > > libblkid/src/probe.c | 65 ++++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 59 insertions(+), 6 deletions(-) > > > > diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c > > index 102766e57aa0..7454a14bdfe6 100644 > > --- a/libblkid/src/probe.c > > +++ b/libblkid/src/probe.c > > @@ -107,6 +107,7 @@ > > #include <stdint.h> > > #include <stdarg.h> > > #include <limits.h> > > +#include <stdbool.h> > > > > #include "blkidP.h" > > #include "all-io.h" > > @@ -1225,6 +1226,40 @@ int blkid_do_probe(blkid_probe pr) > > return rc; > > } > > > > +static int is_conventional(blkid_probe pr, uint64_t offset) > > +{ > > + struct blk_zone_report *rep = NULL; > > + size_t rep_size; > > + bool conventional; > > + int ret; > > + > > + if (!pr->zone_size) > > + return 1; > > + > > + rep_size = sizeof(struct blk_zone_report) + sizeof(struct blk_zone); > > + rep = malloc(rep_size); > > + if (!rep) > > + return -1; > > + > > + memset(rep, 0, rep_size); > > Use calloc() to get the zeroing done at the same time as allocation, may be ? Will do. > > + rep->sector = (offset & pr->zone_size) >> 9; > > Why the "& pr->zone_size" ? This seems wrong. To get the zone info of the zone > containing offset, you can just have: > > rep->sector = offset >> 9; > > And if you want to align to the zone start, then: > > rep->sector = (offset & (~(pr->zone_size - 1))) >> 9; Ouch. I was silly and missed this bug because it was only doing reset on zone #0 and #1. I'll introduce "zone_mask" and do the correct calculation. > > + rep->nr_zones = 1; > > + ret = ioctl(blkid_probe_get_fd(pr), BLKREPORTZONE, rep); > > + if (ret) { > > + free(rep); > > + return -1; > > + } > > + > > + if (rep->zones[0].type == BLK_ZONE_TYPE_CONVENTIONAL) > > + ret = 1; > > + else > > + ret = 0; > > + > > + free(rep); > > + > > + return ret; > > +} > > + > > /** > > * blkid_do_wipe: > > * @pr: prober > > @@ -1264,6 +1299,7 @@ int blkid_do_wipe(blkid_probe pr, int dryrun) > > const char *off = NULL; > > size_t len = 0; > > uint64_t offset, magoff; > > + bool conventional; > > char buf[BUFSIZ]; > > int fd, rc = 0; > > struct blkid_chain *chn; > > @@ -1299,6 +1335,11 @@ int blkid_do_wipe(blkid_probe pr, int dryrun) > > if (len > sizeof(buf)) > > len = sizeof(buf); > > > > + rc = is_conventional(pr, offset); > > + if (rc < 0) > > + return rc; > > + conventional = rc == 1; > > + > > DBG(LOWPROBE, ul_debug( > > "do_wipe [offset=0x%"PRIx64" (%"PRIu64"), len=%zu, chain=%s, idx=%d, dryrun=%s]\n", > > offset, offset, len, chn->driver->name, chn->idx, dryrun ? "yes" : "not")); > > @@ -1306,13 +1347,25 @@ int blkid_do_wipe(blkid_probe pr, int dryrun) > > if (lseek(fd, offset, SEEK_SET) == (off_t) -1) > > return -1; > > > > - memset(buf, 0, len); > > - > > if (!dryrun && len) { > > - /* wipen on device */ > > - if (write_all(fd, buf, len)) > > - return -1; > > - fsync(fd); > > + if (conventional) { > > + memset(buf, 0, len); > > + > > + /* wipen on device */ > > + if (write_all(fd, buf, len)) > > + return -1; > > + fsync(fd); > > + } else { > > + struct blk_zone_range range = { > > + (offset & pr->zone_size) >> 9, > > + pr->zone_size >> 9, > > + }; > > Please add the field names for clarity and fix the sector position (it looks > very wrong): > > struct blk_zone_range range = { > .sector = (offset & (~(pr->zone_size - 1))) >> 9, > .nr_sectors = pr->zone_size >> 9, > }; Will do the same. > > + > > + rc = ioctl(fd, BLKRESETZONE, &range); > > + if (rc < 0) > > + return -1; > > + } > > + > > pr->flags &= ~BLKID_FL_MODIF_BUFF; /* be paranoid */ > > > > return blkid_probe_step_back(pr); > > > > > -- > Damien Le Moal > Western Digital Research
diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c index 102766e57aa0..7454a14bdfe6 100644 --- a/libblkid/src/probe.c +++ b/libblkid/src/probe.c @@ -107,6 +107,7 @@ #include <stdint.h> #include <stdarg.h> #include <limits.h> +#include <stdbool.h> #include "blkidP.h" #include "all-io.h" @@ -1225,6 +1226,40 @@ int blkid_do_probe(blkid_probe pr) return rc; } +static int is_conventional(blkid_probe pr, uint64_t offset) +{ + struct blk_zone_report *rep = NULL; + size_t rep_size; + bool conventional; + int ret; + + if (!pr->zone_size) + return 1; + + rep_size = sizeof(struct blk_zone_report) + sizeof(struct blk_zone); + rep = malloc(rep_size); + if (!rep) + return -1; + + memset(rep, 0, rep_size); + rep->sector = (offset & pr->zone_size) >> 9; + rep->nr_zones = 1; + ret = ioctl(blkid_probe_get_fd(pr), BLKREPORTZONE, rep); + if (ret) { + free(rep); + return -1; + } + + if (rep->zones[0].type == BLK_ZONE_TYPE_CONVENTIONAL) + ret = 1; + else + ret = 0; + + free(rep); + + return ret; +} + /** * blkid_do_wipe: * @pr: prober @@ -1264,6 +1299,7 @@ int blkid_do_wipe(blkid_probe pr, int dryrun) const char *off = NULL; size_t len = 0; uint64_t offset, magoff; + bool conventional; char buf[BUFSIZ]; int fd, rc = 0; struct blkid_chain *chn; @@ -1299,6 +1335,11 @@ int blkid_do_wipe(blkid_probe pr, int dryrun) if (len > sizeof(buf)) len = sizeof(buf); + rc = is_conventional(pr, offset); + if (rc < 0) + return rc; + conventional = rc == 1; + DBG(LOWPROBE, ul_debug( "do_wipe [offset=0x%"PRIx64" (%"PRIu64"), len=%zu, chain=%s, idx=%d, dryrun=%s]\n", offset, offset, len, chn->driver->name, chn->idx, dryrun ? "yes" : "not")); @@ -1306,13 +1347,25 @@ int blkid_do_wipe(blkid_probe pr, int dryrun) if (lseek(fd, offset, SEEK_SET) == (off_t) -1) return -1; - memset(buf, 0, len); - if (!dryrun && len) { - /* wipen on device */ - if (write_all(fd, buf, len)) - return -1; - fsync(fd); + if (conventional) { + memset(buf, 0, len); + + /* wipen on device */ + if (write_all(fd, buf, len)) + return -1; + fsync(fd); + } else { + struct blk_zone_range range = { + (offset & pr->zone_size) >> 9, + pr->zone_size >> 9, + }; + + rc = ioctl(fd, BLKRESETZONE, &range); + if (rc < 0) + return -1; + } + pr->flags &= ~BLKID_FL_MODIF_BUFF; /* be paranoid */ return blkid_probe_step_back(pr);
We cannot overwrite superblock magic in a sequential required zone. So, wipefs cannot work as it is. Instead, this commit implements the wiping by zone resetting. Zone resetting must be done only for a sequential write zone. This is checked by is_conventional(). Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> --- libblkid/src/probe.c | 65 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 6 deletions(-)