Re: [PATCH 1/3] xfsrestore: fix inventory unpacking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 29/09/2022 01:12, Darrick J. Wong wrote:
On Wed, Sep 28, 2022 at 03:53:05PM +1000, Donald Douwsma wrote:
When xfsrestore reads the inventory from tape media it fails to convert
media file records from bigendian. If the xfsdump inventory is not
available xfsrestore will write this invalid record to the on-line
inventory.

[root@rhel8 ~]# xfsdump -L Test1 -M "" -f /dev/st0 /boot > /dev/null
[root@rhel8 ~]# xfsdump -L Test2 -M "" -f /dev/st0 /boot > /dev/null
[root@rhel8 ~]# rm -rf /var/lib/xfsdump/inventory/
[root@rhel8 ~]# mt -f /dev/nst0 asf 2
[root@rhel8 ~]# xfsrestore -f /dev/nst0 -L Test2 /tmp/test2
xfsrestore: using scsi tape (drive_scsitape) strategy
xfsrestore: version 3.1.8 (dump format 3.0) - type ^C for status and control
xfsrestore: searching media for dump
xfsrestore: preparing drive
xfsrestore: examining media file 3
xfsrestore: found dump matching specified label:
xfsrestore: hostname: rhel8
xfsrestore: mount point: /boot
xfsrestore: volume: /dev/sda1
xfsrestore: session time: Tue Sep 27 16:05:28 2022
xfsrestore: level: 0
xfsrestore: session label: "Test2"
xfsrestore: media label: ""
xfsrestore: file system id: 26dd5aa0-b901-4cf5-9b68-0c5753cb3ab8
xfsrestore: session id: 62402423-7ae0-49ed-8ecb-9e5bc7642b91
xfsrestore: media id: 47ba45ca-3417-4006-ab10-3dc6419b83e2
xfsrestore: incorporating on-media session inventory into online inventory
xfsrestore: /var/lib/xfsdump/inventory created
xfsrestore: using on-media session inventory
xfsrestore: searching media for directory dump
xfsrestore: rewinding
xfsrestore: examining media file 0
xfsrestore: inventory session uuid (62402423-7ae0-49ed-8ecb-9e5bc7642b91) does not match the media header's session uuid (1771d9e8-a1ba-4e87-a61e-f6be97e41b45)
xfsrestore: examining media file 1
xfsrestore: inventory session uuid (62402423-7ae0-49ed-8ecb-9e5bc7642b91) does not match the media header's session uuid (1771d9e8-a1ba-4e87-a61e-f6be97e41b45)
xfsrestore: examining media file 2
xfsrestore: reading directories
xfsrestore: 9 directories and 320 entries processed
xfsrestore: directory post-processing
xfsrestore: restore complete: 0 seconds elapsed
xfsrestore: Restore Summary:
xfsrestore:   stream 0 /dev/nst0 OK (success)
xfsrestore: Restore Status: SUCCESS
[root@rhel8 ~]# xfsdump -I
file system 0:
         fs id:          26dd5aa0-b901-4cf5-9b68-0c5753cb3ab8
         session 0:
                 mount point:    rhel8:/boot
                 device:         rhel8:/dev/sda1
                 time:           Tue Sep 27 16:05:28 2022
                 session label:  "Test2"
                 session id:     62402423-7ae0-49ed-8ecb-9e5bc7642b91
                 level:          0
                 resumed:        NO
                 subtree:        NO
                 streams:        1
                 stream 0:
                         pathname:       /dev/st0
                         start:          ino 133 offset 0
                         end:            ino 1572997 offset 0
                         interrupted:    YES
                         media files:    1
                         media file 0:
                                 mfile index:    33554432
                                 mfile type:     data
                                 mfile size:     211187836911616
                                 mfile start:    ino 9583660007044415488 offset 0
                                 mfile end:      ino 9583686395323482112 offset 0
                                 media label:    ""
                                 media id:       47ba45ca-3417-4006-ab10-3dc6419b83e2
xfsdump: Dump Status: SUCCESS
[root@rhel8 ~]#
[root@rhel8 ~]# ls /tmp/test2
efi  grub2  loader

The invalid start and end inode information cause xfsrestore to consider
that non-directory files do not reside in the current media and will
fail to restore them.

The behaviour of an initial restore may succeed if the position of the
tape is such that the data file is encountered before the inventory
file, or if there is only one dump session on tape, xfsrestore is
somewhat inconsistent in this regard. Subsequent restores will use the
invalid on-line inventory and fail to restore files.

Fix this by correctly unpacking the inventory data.

Signed-off-by: Donald Douwsma <ddouwsma@xxxxxxxxxx>
---
  inventory/inv_stobj.c | 27 +++++++--------------------
  1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/inventory/inv_stobj.c b/inventory/inv_stobj.c
index c20e71c..5075ee4 100644
--- a/inventory/inv_stobj.c
+++ b/inventory/inv_stobj.c
@@ -1008,7 +1008,7 @@ stobj_unpack_sessinfo(
          size_t             bufsz,
  	invt_sessinfo_t   *s)
  {
-	uint 		 i;
+	uint 		 i, j;
  	char	         *tmpbuf;
  	char 		 *p = (char *)bufp;
@@ -1087,26 +1087,13 @@ stobj_unpack_sessinfo( /* all the media files */
  	s->mfiles = (invt_mediafile_t *)p;
-
-#ifdef INVT_DELETION
-	{
-		int tmpfd = open("moids", O_RDWR | O_CREAT, S_IRUSR|S_IWUSR);

I wonder, do you need to preserve this behavior (writing the inventory
records to "moids")?  testmain.c seems to want to read the file, but
OTOH that looks like some sort of test program; arbitrarily overwriting
a file in $PWD seems ... risky?  And I guess this chunk has never run.

Also testmain.c has such gems as:

"/dana/hates/me"

"/the/krays"

and mentions that -I supersedes most of its functionality.  So maybe
that's why you deleted moids?  Aside from the name just sounding gross?

:)


I think they were trying to mirror what's done in stobj_pack_sessinfo(),
which takes a file handle as a parameter, possibly its meant to provide
locking while packing for the inventory. Calling put_invtrecord without
having first called get_invtrecord seems unbalanced, and the unneeded
file "mods" was just left around. Either way the target for
stobj_unpack_sessinfo is a buffer not a file.

AFIKT this was a work in progress by someone who never got to finish it.

-		uint j;
-		invt_mediafile_t *mmf = s->mfiles;
-		for (i=0; i< s->ses->s_cur_nstreams; i++) {
-			for (j=0; j< s->strms[i].st_nmediafiles;
-			     j++, mmf++)
-				xlate_invt_mediafile((invt_mediafile_t *)mmf, (invt_mediafile_t *)tmpbuf, 1);
-				bcopy(tmpbuf, mmf, sizeof(invt_mediafile_t));
-				put_invtrecord(tmpfd, &mmf->mf_moid,
-					 sizeof(uuid_t), 0, SEEK_END, 0);
+	for (i=0; i< s->ses->s_cur_nstreams; i++) {
+		for (j=0; j < s->strms[i].st_nmediafiles; j++) {
+			xlate_invt_mediafile((invt_mediafile_t *)p,

Nit: trailing whitespace                                      here ^

Urg, sorry I thought I'd fixed all the white-space problems.


If the the answer to the above question is "Yeah, nobody wants the moids
file" then:

Yes, I think moids was a hack that shouldn't have been there.

Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>

--D

+					     (invt_mediafile_t *)tmpbuf, 1);
+			bcopy(tmpbuf, p, sizeof(invt_mediafile_t));
+			p +=  sizeof(invt_mediafile_t);
  		}
-		close(tmpfd);
-	}
-#endif
-	for (i = 0; i < s->ses->s_cur_nstreams; i++) {
-		p += (size_t) (s->strms[i].st_nmediafiles)
-			* sizeof(invt_mediafile_t);
  	}
/* sanity check the size of the buffer given to us vs. the size it
--
2.31.1






[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux