Re: [PATCH 15/19] mkfs: don't treat files as though they are block devices

Jan Tulak
2016-04-20T13:17:51Z

[-- Attachment #1.1: Type: text/plain, Size: 3876 bytes --]

On Wed, Apr 20, 2016 at 11:51 AM, Jan Tulak <jtulak@redhat.com> wrote:

> On Thu, Apr 14, 2016 at 11:49 AM, Jan Tulak <jtulak@redhat.com> wrote:
>
>> On Fri, Apr 8, 2016 at 4:58 PM, Jan Tulak <jtulak@redhat.com> wrote:
>>
>>> On Fri, Apr 8, 2016 at 2:25 AM, Eric Sandeen <sandeen@sandeen.net>
>>> wrote:
>>>
>> [
>>>> ​snip]​
>>>>
>>>> I still think this patch might need a reset though :)
>>>>
>>>> Thanks,
>>>> -Eric
>>>>
>>>>
>>> ​On few places, the original patch looked as if files had direct IO
>>> still enabled​ (using platform_findsize...), and I think it was causing
>>> some failures - fixing issues is why I did most of the changes.
>>>
>>> I will revert to the original version and see what exactly fails. But
>>> with being Friday late afternoon, the results will be available the next
>>> week (Wednesday and further, all my courses at university are stuffed in
>>> Mon/Tue).
>>>
>>>
>> ​All right. So I run the tests on the set with both this and the original
>> version of this patch and there is no change in results. Here and there
>> some tests failed on one run, but on other runs they are ok and I can't get
>> them to fail again (namely, xfs/033, 073 and 085 did this).
>>
>> It looks like whatever the issues were to start the changes, they are
>> already resolved. Thus, I will send the original patch in the next batch.
>>
>>
>> ​I'm taking it back, one tests fails, most likely it happened when I
> removed patch 17 - xfsprogs: disable truncating of files.
> If I do "mkfs.xfs $file", it doesn't know the old size to recreate it. I'm
> working on a change for this patch to fix this issue, but I'm not entirely
> certain how to proceed. The right place to get the size seems to be
> get_topology(). I could save it into xi.dsize, but then libxfs_init()
> zeroes it.
>
> So I think that I might rather convert the measured size do char *dsize
> and pretend that it is an user-given value. But this looks to me more like
> a hack. On the other side, any other way would require further changes in
> the code to convert the char* to int as soon as possible - which does not
> belong to this patch...
>
> Diff of the change:
>
> @@ -999,7 +999,7 @@ static void get_topology(
>
> fd = open(dfile, flags, 0666);
> if (fd >= 0) {
> - platform_findsizes(dfile, fd, &dummy,
> &ft->lsectorsize);
> + platform_findsizes(dfile, fd, &xi->dsize,
> &ft->lsectorsize);
> close(fd);
> ft->psectorsize = ft->lsectorsize;
> } else
> @@ -2273,6 +2273,18 @@ _("Minimum block size for CRC enabled filesystems
> is %d bytes.\n"),
> memset(&ft, 0, sizeof(ft));
> get_topology(&xi, &ft, force_overwrite);
>
> + /*
> + * If it is an existing file and we have no info about it
> + * like in case of "mkfs.xfs some-file", we need to save the size
> + * of the file to somewhere where it can be read later.
> + */
> + if (xi.disfile && !xi.dcreat && xi.dsize) {
> + long long num = xi.dsize * ft.lsectorsize;
> + int length = snprintf(NULL, 0,"%lld", num);
> + dsize = calloc( length + 1 ,sizeof(char));
> + sprintf(dsize, "%lld", num);
> + }
> +
> if (!ssflag) {
> /*
> * Unless specified manually on the command line use the
>
> So, what do you think about this?
>
> ​
And before anyone will reply, I found that putting a platform_findsizes​
into libxfs_init will fix it too. Which is nice, especially because for
block devices, the findsizes is already used there. So ignore the previous
hack. :-)

Cheers,
Jan

--
Jan Tulak
jtulak@redhat.com / jan@tulak.me

[-- Attachment #1.2: Type: text/html, Size: 10358 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]