| Message ID | 20230606092806.1604491-13-chandan.babu@oracle.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Metadump v2 | expand |
On Tue, Jun 06, 2023 at 02:57:55PM +0530, Chandan Babu R wrote: > This commit introduces a new function set_log_cur() allowing xfs_db to read > from an external log device. This is required by a future commit which will > add the ability to dump metadata from external log devices. > > Signed-off-by: Chandan Babu R <chandan.babu@oracle.com> > --- > db/io.c | 57 ++++++++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 44 insertions(+), 13 deletions(-) > > diff --git a/db/io.c b/db/io.c > index 3d257236..a6feaba3 100644 > --- a/db/io.c > +++ b/db/io.c > @@ -508,18 +508,19 @@ write_cur(void) > > } > > -void > -set_cur( > - const typ_t *type, > - xfs_daddr_t blknum, > - int len, > - int ring_flag, > - bbmap_t *bbmap) > +static void > +__set_cur( > + struct xfs_buftarg *btargp, > + const typ_t *type, > + xfs_daddr_t blknum, > + int len, > + int ring_flag, > + bbmap_t *bbmap) > { > - struct xfs_buf *bp; > - xfs_ino_t dirino; > - xfs_ino_t ino; > - uint16_t mode; > + struct xfs_buf *bp; > + xfs_ino_t dirino; > + xfs_ino_t ino; > + uint16_t mode; > const struct xfs_buf_ops *ops = type ? type->bops : NULL; > int error; > > @@ -548,11 +549,11 @@ set_cur( > if (!iocur_top->bbmap) > return; > memcpy(iocur_top->bbmap, bbmap, sizeof(struct bbmap)); > - error = -libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b, > + error = -libxfs_buf_read_map(btargp, bbmap->b, > bbmap->nmaps, LIBXFS_READBUF_SALVAGE, &bp, > ops); > } else { > - error = -libxfs_buf_read(mp->m_ddev_targp, blknum, len, > + error = -libxfs_buf_read(btargp, blknum, len, > LIBXFS_READBUF_SALVAGE, &bp, ops); > iocur_top->bbmap = NULL; > } > @@ -589,6 +590,36 @@ set_cur( > ring_add(); > } > > +void > +set_cur( > + const typ_t *type, > + xfs_daddr_t blknum, > + int len, > + int ring_flag, > + bbmap_t *bbmap) > +{ > + __set_cur(mp->m_ddev_targp, type, blknum, len, ring_flag, bbmap); > +} > + > +void > +set_log_cur( > + const typ_t *type, > + xfs_daddr_t blknum, > + int len, > + int ring_flag, > + bbmap_t *bbmap) > +{ > + struct xfs_buftarg *btargp = mp->m_ddev_targp; > + > + if (mp->m_logdev_targp->bt_bdev != mp->m_ddev_targp->bt_bdev) { > + ASSERT(mp->m_sb.sb_logstart == 0); > + btargp = mp->m_logdev_targp; > + } I think this should print an error message if there isn't an external log device and then leave iocur_top unchanged: if (mp->m_logdev_targp->bt_bdev == mp->m_ddev_targp->bt_bdev) { fprintf(stderr, "no external log specified\n"); exitcode = 1; return; } __set_cur(mp->m_logdev_targp, type, blknum, len, ring_flag, bbmap); because metadump shouldn't crash if there's an external log device but sb_logstart is zero. The superblock fields /could/ be corrupt, or other weird shenanigans are going on. (Also it's a layering violation for the io cursors to know anything at all about the filesystem stored above them.) --D > + > + __set_cur(btargp, type, blknum, len, ring_flag, bbmap); > +} > + > + > void > set_iocur_type( > const typ_t *type) > -- > 2.39.1 >
On Wed, Jul 12, 2023 at 10:35:29 AM -0700, Darrick J. Wong wrote: > On Tue, Jun 06, 2023 at 02:57:55PM +0530, Chandan Babu R wrote: >> This commit introduces a new function set_log_cur() allowing xfs_db to read >> from an external log device. This is required by a future commit which will >> add the ability to dump metadata from external log devices. >> >> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com> >> --- >> db/io.c | 57 ++++++++++++++++++++++++++++++++++++++++++++------------- >> 1 file changed, 44 insertions(+), 13 deletions(-) >> >> diff --git a/db/io.c b/db/io.c >> index 3d257236..a6feaba3 100644 >> --- a/db/io.c >> +++ b/db/io.c >> @@ -508,18 +508,19 @@ write_cur(void) >> >> } >> >> -void >> -set_cur( >> - const typ_t *type, >> - xfs_daddr_t blknum, >> - int len, >> - int ring_flag, >> - bbmap_t *bbmap) >> +static void >> +__set_cur( >> + struct xfs_buftarg *btargp, >> + const typ_t *type, >> + xfs_daddr_t blknum, >> + int len, >> + int ring_flag, >> + bbmap_t *bbmap) >> { >> - struct xfs_buf *bp; >> - xfs_ino_t dirino; >> - xfs_ino_t ino; >> - uint16_t mode; >> + struct xfs_buf *bp; >> + xfs_ino_t dirino; >> + xfs_ino_t ino; >> + uint16_t mode; >> const struct xfs_buf_ops *ops = type ? type->bops : NULL; >> int error; >> >> @@ -548,11 +549,11 @@ set_cur( >> if (!iocur_top->bbmap) >> return; >> memcpy(iocur_top->bbmap, bbmap, sizeof(struct bbmap)); >> - error = -libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b, >> + error = -libxfs_buf_read_map(btargp, bbmap->b, >> bbmap->nmaps, LIBXFS_READBUF_SALVAGE, &bp, >> ops); >> } else { >> - error = -libxfs_buf_read(mp->m_ddev_targp, blknum, len, >> + error = -libxfs_buf_read(btargp, blknum, len, >> LIBXFS_READBUF_SALVAGE, &bp, ops); >> iocur_top->bbmap = NULL; >> } >> @@ -589,6 +590,36 @@ set_cur( >> ring_add(); >> } >> >> +void >> +set_cur( >> + const typ_t *type, >> + xfs_daddr_t blknum, >> + int len, >> + int ring_flag, >> + bbmap_t *bbmap) >> +{ >> + __set_cur(mp->m_ddev_targp, type, blknum, len, ring_flag, bbmap); >> +} >> + >> +void >> +set_log_cur( >> + const typ_t *type, >> + xfs_daddr_t blknum, >> + int len, >> + int ring_flag, >> + bbmap_t *bbmap) >> +{ >> + struct xfs_buftarg *btargp = mp->m_ddev_targp; >> + >> + if (mp->m_logdev_targp->bt_bdev != mp->m_ddev_targp->bt_bdev) { >> + ASSERT(mp->m_sb.sb_logstart == 0); >> + btargp = mp->m_logdev_targp; >> + } > > I think this should print an error message if there isn't an external > log device and then leave iocur_top unchanged: > > if (mp->m_logdev_targp->bt_bdev == mp->m_ddev_targp->bt_bdev) { > fprintf(stderr, "no external log specified\n"); > exitcode = 1; > return; > } > > __set_cur(mp->m_logdev_targp, type, blknum, len, ring_flag, bbmap); > > because metadump shouldn't crash if there's an external log device but > sb_logstart is zero. The superblock fields /could/ be corrupt, or other > weird shenanigans are going on. > > (Also it's a layering violation for the io cursors to know anything at > all about the filesystem stored above them.) > I agree with your review comment. I will make the required changes.
diff --git a/db/io.c b/db/io.c index 3d257236..a6feaba3 100644 --- a/db/io.c +++ b/db/io.c @@ -508,18 +508,19 @@ write_cur(void) } -void -set_cur( - const typ_t *type, - xfs_daddr_t blknum, - int len, - int ring_flag, - bbmap_t *bbmap) +static void +__set_cur( + struct xfs_buftarg *btargp, + const typ_t *type, + xfs_daddr_t blknum, + int len, + int ring_flag, + bbmap_t *bbmap) { - struct xfs_buf *bp; - xfs_ino_t dirino; - xfs_ino_t ino; - uint16_t mode; + struct xfs_buf *bp; + xfs_ino_t dirino; + xfs_ino_t ino; + uint16_t mode; const struct xfs_buf_ops *ops = type ? type->bops : NULL; int error; @@ -548,11 +549,11 @@ set_cur( if (!iocur_top->bbmap) return; memcpy(iocur_top->bbmap, bbmap, sizeof(struct bbmap)); - error = -libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b, + error = -libxfs_buf_read_map(btargp, bbmap->b, bbmap->nmaps, LIBXFS_READBUF_SALVAGE, &bp, ops); } else { - error = -libxfs_buf_read(mp->m_ddev_targp, blknum, len, + error = -libxfs_buf_read(btargp, blknum, len, LIBXFS_READBUF_SALVAGE, &bp, ops); iocur_top->bbmap = NULL; } @@ -589,6 +590,36 @@ set_cur( ring_add(); } +void +set_cur( + const typ_t *type, + xfs_daddr_t blknum, + int len, + int ring_flag, + bbmap_t *bbmap) +{ + __set_cur(mp->m_ddev_targp, type, blknum, len, ring_flag, bbmap); +} + +void +set_log_cur( + const typ_t *type, + xfs_daddr_t blknum, + int len, + int ring_flag, + bbmap_t *bbmap) +{ + struct xfs_buftarg *btargp = mp->m_ddev_targp; + + if (mp->m_logdev_targp->bt_bdev != mp->m_ddev_targp->bt_bdev) { + ASSERT(mp->m_sb.sb_logstart == 0); + btargp = mp->m_logdev_targp; + } + + __set_cur(btargp, type, blknum, len, ring_flag, bbmap); +} + + void set_iocur_type( const typ_t *type)
This commit introduces a new function set_log_cur() allowing xfs_db to read from an external log device. This is required by a future commit which will add the ability to dump metadata from external log devices. Signed-off-by: Chandan Babu R <chandan.babu@oracle.com> --- db/io.c | 57 ++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 13 deletions(-)