On Mon, Dec 09, 2024 at 09:47:02PM -0800, Christoph Hellwig wrote:
> > + /* The realtime device only contains metadata if rtgroups is enabled. */
> > + if (mp->m_rtdev_targp->bt_bdev && xfs_has_rtgroups(mp))
>
> As you pointed out when looking at my follow on series, this should be
> xfs_has_rtsb instead of xfs_has_rtgroups (for cosmetic reasons).
>
> > + metadump.realtime_data = true;
> > +
> > + if (metadump.realtime_data && !version_opt_set)
> > + metadump.version = 2;
> > +
> > + if (metadump.version == 2 && xfs_has_realtime(mp) &&
> > + xfs_has_rtgroups(mp) &&
> > + !metadump.realtime_data) {
> > + print_warning("realtime device not loaded, use -R");
> > + return 1;
> > + }
>
> Also this whole flow looks a bit weird. This is what I ended up with
> removing my later changes:
>
> /*
> * The realtime device only contains metadata if the RT superblock is
> * enabled.
> */
> if (xfs_has_realtime(mp) && xfs_has_rtsb(mp)) {
> if (mp->m_rtdev_targp->bt_bdev) {
> metadump.realtime_data = true;
> if (!version_opt_set)
> metadump.version = 2;
> } else if (metadump.version == 2 && !metadump.realtime_data) {
> print_warning("realtime device not loaded, use -R")
> return 1;
> }
> }
Oh, yeah, that is a lot more straightforward. Thank you for correcting
that.
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
<nod>
--D
