Re: [PATCH v2 1/3] blkid: implement zone-aware probing

From: Naohiro Aota <naohiro.aota@wdc.com>
To: Karel Zak <kzak@redhat.com>
Cc: util-linux@vger.kernel.org, linux-btrfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	Damien Le Moal <damien.lemoal@wdc.com>,
	Johannes Thumshirn <johannes.thumshirn@wdc.com>
Subject: Re: [PATCH v2 1/3] blkid: implement zone-aware probing
Date: Thu, 15 Apr 2021 00:03:50 +0900	[thread overview]
Message-ID: <20210414150350.srqsmzyoscec6phy@naota-xeon> (raw)
In-Reply-To: <20210414133101.p5amev6tkfroiyw5@ws.net.home>

On Wed, Apr 14, 2021 at 03:31:01PM +0200, Karel Zak wrote:
> On Wed, Apr 14, 2021 at 10:33:37AM +0900, Naohiro Aota wrote:
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -302,6 +302,7 @@ AC_CHECK_HEADERS([ \
> >  	lastlog.h \
> >  	libutil.h \
> >  	linux/btrfs.h \
> > +	linux/blkzoned.h \
> 
> unnecessary, there is already AC_CHECK_HEADERS([linux/blkzoned.h]) on
> another place.

Ah, I missed that. I will drop it.

> >  	linux/capability.h \
> >  	linux/cdrom.h \
> >  	linux/falloc.h \
> > diff --git a/libblkid/src/blkidP.h b/libblkid/src/blkidP.h
> > index a3fe6748a969..e3a160aa97c0 100644
> > --- a/libblkid/src/blkidP.h
> > +++ b/libblkid/src/blkidP.h
> > @@ -150,6 +150,10 @@ struct blkid_idmag
> >  	const char	*hoff;		/* hint which contains byte offset to kboff */
> >  	long		kboff;		/* kilobyte offset of superblock */
> >  	unsigned int	sboff;		/* byte offset within superblock */
> > +
> > +	int		is_zoned;	/* indicate magic location is calcluated based on zone position  */
> > +	long		zonenum;	/* zone number which has superblock */
> > +	long		kboff_inzone;	/* kilobyte offset of superblock in a zone */
> 
> It would be better to use 'flags' struct field and
> 
>   #define BLKID_FL_ZONED_DEV (1 << 6)
> 
> like we use for another stuff.

BLKID_FL_* flags looks like to indicate a device's property. Instead,
this one indicates a magic is placed relative to a zone. I do not see
blkid_idmag is currently using "flags" filed. Should we really add it
and follow the flag style? I thought, we can do it later when other
use case exists.

> > diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
> > index a47a8720d4ac..9d180aab5242 100644
> > --- a/libblkid/src/probe.c
> > +++ b/libblkid/src/probe.c
> > @@ -94,6 +94,9 @@
> >  #ifdef HAVE_LINUX_CDROM_H
> >  #include <linux/cdrom.h>
> >  #endif
> > +#ifdef HAVE_LINUX_BLKZONED_H
> > +#include <linux/blkzoned.h>
> > +#endif
> >  #ifdef HAVE_SYS_STAT_H
> >  #include <sys/stat.h>
> >  #endif
> > @@ -897,6 +900,7 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
> >  	pr->wipe_off = 0;
> >  	pr->wipe_size = 0;
> >  	pr->wipe_chain = NULL;
> > +	pr->zone_size = 0;
> 
> you also need to update blkid_clone_probe() function

Will do. I completely missed that. Thanks.

>   Karel
> 
> -- 
>  Karel Zak  <kzak@redhat.com>
>  http://karelzak.blogspot.com
> 