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

[Donald Douwsma <ddouwsma@redhat.com>]

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
>>
>