diff mbox series

[3/4] xfsdump: fix on-media inventory stream packing

Message ID 20221013031518.1815861-4-ddouwsma@redhat.com (mailing list archive)
State Superseded, archived
Headers show
Series xfsrestore: fix inventory unpacking | expand

Commit Message

Donald Douwsma Oct. 13, 2022, 3:15 a.m. UTC
With the on-media inventory now being restored for multiple streams we
can see that the restored streams both claim to be for /dev/nst0.

[root@rhel8 xfsdump-dev]# xfsdump -L "Test2" -f /dev/nst0 -M "tape1" -f /dev/nst1 -M "tape2" /boot
...
[root@rhel8 xfsdump-dev]# rm -rf /var/lib/xfsdump/inventory /tmp/test1/*
[root@rhel8 xfsdump-dev]# restore/xfsrestore -L Test2 -f /dev/nst0 -f /dev/nst1 /tmp/test2
restore/xfsrestore: using scsi tape (drive_scsitape) strategy
restore/xfsrestore: using scsi tape (drive_scsitape) strategy
restore/xfsrestore: version 3.1.10 (dump format 3.0) - type ^C for status and control
...
restore/xfsrestore: Restore Summary:
restore/xfsrestore:   stream 0 /dev/nst0 OK (success)
restore/xfsrestore:   stream 1 /dev/nst1 ALREADY_DONE (another stream completed the operation)
restore/xfsrestore: Restore Status: SUCCESS
[root@rhel8 xfsdump-dev]# xfsdump -I
file system 0:
        fs id:          26dd5aa0-b901-4cf5-9b68-0c5753cb3ab8
        session 0:
                mount point:    rhel8:/boot
                device:         rhel8:/dev/sda1
                time:           Wed Oct 12 15:36:55 2022
                session label:  "Test2"
                session id:     50be3b17-d9f9-414d-885b-ababf660e189
                level:          0
                resumed:        NO
                subtree:        NO
                streams:        2
                stream 0:
                        pathname:       /dev/nst0
                        start:          ino 133 offset 0
                        end:            ino 28839 offset 0
                        interrupted:    YES
                        media files:    1
                        media file 0:
                                mfile index:    2
                                mfile type:     data
                                mfile size:     165675008
                                mfile start:    ino 133 offset 0
                                mfile end:      ino 28839 offset 0
                                media label:    "test"
                                media id:       e2e6978d-5546-4f1f-8c9e-307168071889
                stream 1:
                        pathname:       /dev/nst0
                        start:          ino 133 offset 0
                        end:            ino 28839 offset 0
                        interrupted:    YES
                        media files:    1
                        media file 0:
                                mfile index:    0
                                mfile type:     data
                                mfile size:     166723584
                                mfile start:    ino 28839 offset 0
                                mfile end:      ino 1572997 offset 0
                                media label:    "tape2"
                                media id:       1ad6d919-7159-42fb-a20f-5a2c4e3e24b1
xfsdump: Dump Status: SUCCESS
[root@rhel8 xfsdump-dev]#

Fix this by indexing the stream being packed for the on-media inventory.

Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
Suggested-by: Darrick J. Wong <djwong@kernel.org>
---
 inventory/inv_stobj.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Darrick J. Wong Oct. 13, 2022, 7:10 p.m. UTC | #1
On Thu, Oct 13, 2022 at 02:15:17PM +1100, Donald Douwsma wrote:
> With the on-media inventory now being restored for multiple streams we
> can see that the restored streams both claim to be for /dev/nst0.
> 
> [root@rhel8 xfsdump-dev]# xfsdump -L "Test2" -f /dev/nst0 -M "tape1" -f /dev/nst1 -M "tape2" /boot
> ...
> [root@rhel8 xfsdump-dev]# rm -rf /var/lib/xfsdump/inventory /tmp/test1/*
> [root@rhel8 xfsdump-dev]# restore/xfsrestore -L Test2 -f /dev/nst0 -f /dev/nst1 /tmp/test2
> restore/xfsrestore: using scsi tape (drive_scsitape) strategy
> restore/xfsrestore: using scsi tape (drive_scsitape) strategy
> restore/xfsrestore: version 3.1.10 (dump format 3.0) - type ^C for status and control
> ...
> restore/xfsrestore: Restore Summary:
> restore/xfsrestore:   stream 0 /dev/nst0 OK (success)
> restore/xfsrestore:   stream 1 /dev/nst1 ALREADY_DONE (another stream completed the operation)
> restore/xfsrestore: Restore Status: SUCCESS
> [root@rhel8 xfsdump-dev]# xfsdump -I
> file system 0:
>         fs id:          26dd5aa0-b901-4cf5-9b68-0c5753cb3ab8
>         session 0:
>                 mount point:    rhel8:/boot
>                 device:         rhel8:/dev/sda1
>                 time:           Wed Oct 12 15:36:55 2022
>                 session label:  "Test2"
>                 session id:     50be3b17-d9f9-414d-885b-ababf660e189
>                 level:          0
>                 resumed:        NO
>                 subtree:        NO
>                 streams:        2
>                 stream 0:
>                         pathname:       /dev/nst0
>                         start:          ino 133 offset 0
>                         end:            ino 28839 offset 0
>                         interrupted:    YES
>                         media files:    1
>                         media file 0:
>                                 mfile index:    2
>                                 mfile type:     data
>                                 mfile size:     165675008
>                                 mfile start:    ino 133 offset 0
>                                 mfile end:      ino 28839 offset 0
>                                 media label:    "test"

It's odd that you have -M tape1 above but this ends up labelled "test"?
If that isn't just a munged patch message, then that might need fixing
(separate patch) as well.

The code change looks correct though.  Thanks for fixing dump.

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

--D

>                                 media id:       e2e6978d-5546-4f1f-8c9e-307168071889
>                 stream 1:
>                         pathname:       /dev/nst0
>                         start:          ino 133 offset 0
>                         end:            ino 28839 offset 0
>                         interrupted:    YES
>                         media files:    1
>                         media file 0:
>                                 mfile index:    0
>                                 mfile type:     data
>                                 mfile size:     166723584
>                                 mfile start:    ino 28839 offset 0
>                                 mfile end:      ino 1572997 offset 0
>                                 media label:    "tape2"
>                                 media id:       1ad6d919-7159-42fb-a20f-5a2c4e3e24b1
> xfsdump: Dump Status: SUCCESS
> [root@rhel8 xfsdump-dev]#
> 
> Fix this by indexing the stream being packed for the on-media inventory.
> 
> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  inventory/inv_stobj.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/inventory/inv_stobj.c b/inventory/inv_stobj.c
> index 025d431..fb4d93a 100644
> --- a/inventory/inv_stobj.c
> +++ b/inventory/inv_stobj.c
> @@ -798,7 +798,7 @@ stobj_pack_sessinfo(int fd, invt_session_t *ses, invt_seshdr_t *hdr,
>  	sesbuf += sizeof(invt_session_t);
>  
>  	for (i = 0; i < ses->s_cur_nstreams; i++) {
> -		xlate_invt_stream(strms, (invt_stream_t *)sesbuf, 1);
> +		xlate_invt_stream(&strms[i], (invt_stream_t *)sesbuf, 1);
>  		sesbuf += sizeof(invt_stream_t);
>  	}
>  
> -- 
> 2.31.1
>
Donald Douwsma Oct. 14, 2022, 1:15 a.m. UTC | #2
On 14/10/2022 06:10, Darrick J. Wong wrote:
> On Thu, Oct 13, 2022 at 02:15:17PM +1100, Donald Douwsma wrote:
>> With the on-media inventory now being restored for multiple streams we
>> can see that the restored streams both claim to be for /dev/nst0.
>>
>> [root@rhel8 xfsdump-dev]# xfsdump -L "Test2" -f /dev/nst0 -M "tape1" -f /dev/nst1 -M "tape2" /boot
>> ...
>> [root@rhel8 xfsdump-dev]# rm -rf /var/lib/xfsdump/inventory /tmp/test1/*
>> [root@rhel8 xfsdump-dev]# restore/xfsrestore -L Test2 -f /dev/nst0 -f /dev/nst1 /tmp/test2
>> restore/xfsrestore: using scsi tape (drive_scsitape) strategy
>> restore/xfsrestore: using scsi tape (drive_scsitape) strategy
>> restore/xfsrestore: version 3.1.10 (dump format 3.0) - type ^C for status and control
>> ...
>> restore/xfsrestore: Restore Summary:
>> restore/xfsrestore:   stream 0 /dev/nst0 OK (success)
>> restore/xfsrestore:   stream 1 /dev/nst1 ALREADY_DONE (another stream completed the operation)
>> restore/xfsrestore: Restore Status: SUCCESS
>> [root@rhel8 xfsdump-dev]# xfsdump -I
>> file system 0:
>>          fs id:          26dd5aa0-b901-4cf5-9b68-0c5753cb3ab8
>>          session 0:
>>                  mount point:    rhel8:/boot
>>                  device:         rhel8:/dev/sda1
>>                  time:           Wed Oct 12 15:36:55 2022
>>                  session label:  "Test2"
>>                  session id:     50be3b17-d9f9-414d-885b-ababf660e189
>>                  level:          0
>>                  resumed:        NO
>>                  subtree:        NO
>>                  streams:        2
>>                  stream 0:
>>                          pathname:       /dev/nst0
>>                          start:          ino 133 offset 0
>>                          end:            ino 28839 offset 0
>>                          interrupted:    YES
>>                          media files:    1
>>                          media file 0:
>>                                  mfile index:    2
>>                                  mfile type:     data
>>                                  mfile size:     165675008
>>                                  mfile start:    ino 133 offset 0
>>                                  mfile end:      ino 28839 offset 0
>>                                  media label:    "test"
> 
> It's odd that you have -M tape1 above but this ends up labelled "test"?
> If that isn't just a munged patch message, then that might need fixing
> (separate patch) as well.

I looked back over the terminals where I tested this and they're all
tape1, I think this was an earlier cut n pasta mistake when I prepared
the commit message. Its probably worth fixing the message because not
everyone will want to setup a virtual tape library to look at this.

Thanks for the reviews,
Don

> The code change looks correct though.  Thanks for fixing dump.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> 
> --D
> 
>>                                  media id:       e2e6978d-5546-4f1f-8c9e-307168071889
>>                  stream 1:
>>                          pathname:       /dev/nst0
>>                          start:          ino 133 offset 0
>>                          end:            ino 28839 offset 0
>>                          interrupted:    YES
>>                          media files:    1
>>                          media file 0:
>>                                  mfile index:    0
>>                                  mfile type:     data
>>                                  mfile size:     166723584
>>                                  mfile start:    ino 28839 offset 0
>>                                  mfile end:      ino 1572997 offset 0
>>                                  media label:    "tape2"
>>                                  media id:       1ad6d919-7159-42fb-a20f-5a2c4e3e24b1
>> xfsdump: Dump Status: SUCCESS
>> [root@rhel8 xfsdump-dev]#
>>
>> Fix this by indexing the stream being packed for the on-media inventory.
>>
>> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
>> Suggested-by: Darrick J. Wong <djwong@kernel.org>
>> ---
>>   inventory/inv_stobj.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/inventory/inv_stobj.c b/inventory/inv_stobj.c
>> index 025d431..fb4d93a 100644
>> --- a/inventory/inv_stobj.c
>> +++ b/inventory/inv_stobj.c
>> @@ -798,7 +798,7 @@ stobj_pack_sessinfo(int fd, invt_session_t *ses, invt_seshdr_t *hdr,
>>   	sesbuf += sizeof(invt_session_t);
>>   
>>   	for (i = 0; i < ses->s_cur_nstreams; i++) {
>> -		xlate_invt_stream(strms, (invt_stream_t *)sesbuf, 1);
>> +		xlate_invt_stream(&strms[i], (invt_stream_t *)sesbuf, 1);
>>   		sesbuf += sizeof(invt_stream_t);
>>   	}
>>   
>> -- 
>> 2.31.1
>>
>
diff mbox series

Patch

diff --git a/inventory/inv_stobj.c b/inventory/inv_stobj.c
index 025d431..fb4d93a 100644
--- a/inventory/inv_stobj.c
+++ b/inventory/inv_stobj.c
@@ -798,7 +798,7 @@  stobj_pack_sessinfo(int fd, invt_session_t *ses, invt_seshdr_t *hdr,
 	sesbuf += sizeof(invt_session_t);
 
 	for (i = 0; i < ses->s_cur_nstreams; i++) {
-		xlate_invt_stream(strms, (invt_stream_t *)sesbuf, 1);
+		xlate_invt_stream(&strms[i], (invt_stream_t *)sesbuf, 1);
 		sesbuf += sizeof(invt_stream_t);
 	}