On Sat 18-01-20 01:11:07, Pali Rohár wrote: > On Friday 17 January 2020 13:05:11 Jan Kara wrote: > > On Mon 13-01-20 13:08:51, Jan Kara wrote: > > > Hello, > > > > > > On Sun 12-01-20 17:23:11, Pali Rohár wrote: > > > > I looked at udf_statfs() implementation and I see there two things which > > > > are probably incorrect: > > > > > > > > First one: > > > > > > > > buf->f_blocks = sbi->s_partmaps[sbi->s_partition].s_partition_len; > > > > > > > > If sbi->s_partition points to Metadata partition then reported number > > > > of blocks seems to be incorrect. Similar like in udf_count_free(). > > > > > > Oh, right. This needs similar treatment like udf_count_free(). I'll fix it. > > > Thanks for spotting. > > > > Patch for this is attached. > > I was wrong. After reading UDF specification and kernel implementation > again I realized that there is a complete mess what "partition" means. > > Sometimes it is Partition Map, sometimes it is Partition Descriptor, > sometimes it is index of Partition Map, sometimes index of Partition > Descriptor, sometimes it refers to data referenced by Partition > Descriptor and sometimes it refers to data referenced by Partition Map. > > And "length" means length of any of above structure (either of map > structure, either of data pointed by map structure, either of descriptor > or either of data pointed by descriptor). > > So to make it clear, member "s_partition_len" refers to length of data > pointed by Partition Descriptor, therefore length of physical partition. Yes, now that you say it I remember :) > As kernel probably does not support UDF fs with more then one Partition > Descriptor, whatever Partition Map we choose (s_partmaps[i] member) we > would always get same value in "s_partition_len" as it does not refer to > data of Partition Map, but rather to data of Partition Descriptor. > > As both Metadata Partition Map and Type 1 Partition Map (or Sparable > Partition Map) shares same Partition Descriptor, this patch does not > change value of "f_blocks" member and therefore is not needed at all. > So current code should be correct. > > But please, double check that I'm correct as "partition" naming in > probably every one variable is misleading. > > Just to note, free space is calculated from Partition Map index > (not from Partition Descriptor index), therefore previous patch for > udf_count_free() is really needed and should be correct. Right, I've checked the code and I completely agree with your analysis so I've dropped the patch from my tree. Thanks for checking! Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
