Re: [PATCH] fat/vfat: optionally ignore system timezone offset when reading/writing timestamps

2007-03-27 Thread OGAWA Hirofumi
Paul Collins <[EMAIL PROTECTED]> writes:

> OGAWA Hirofumi <[EMAIL PROTECTED]> writes:
>
>> Thank you. I see. So we need "timezone" option to specify adjusted
>> time?  If so, I feel we can add it as "timezone=utc", then it'll can
>> be improved later...
>
> I am happy to change the patch as needed.  However, since the kernel
> does not have access to a timezone database and therefore cannot
> convert timezone names to offsets, perhaps the mount option should be
> something like "tzoffset=0", "tzoffset=1200" etc., with "tzoffset=0"
> being equivalent to the "posixtime" option implemented by my patch.

Looks sane. The mount command would be able to convert the string
timezone to it if needed.
-- 
OGAWA Hirofumi <[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fat/vfat: optionally ignore system timezone offset when reading/writing timestamps

2007-03-27 Thread Paul Collins
OGAWA Hirofumi <[EMAIL PROTECTED]> writes:

> Hiroyuki Machida <[EMAIL PROTECTED]> writes:
>
>> I'm not famillar with recent fat code, but code itself looks good for
>> just turn on/off time adjusting. On the other hand, I feel we need more 
>> consideration on use cases/requirements. I feel that turning off
>> time adjustment is a just ad-hoc solution to issues like Paul san 
>> brought up.
>
> Thank you. I see. So we need "timezone" option to specify adjusted
> time?  If so, I feel we can add it as "timezone=utc", then it'll can
> be improved later...

I am happy to change the patch as needed.  However, since the kernel
does not have access to a timezone database and therefore cannot
convert timezone names to offsets, perhaps the mount option should be
something like "tzoffset=0", "tzoffset=1200" etc., with "tzoffset=0"
being equivalent to the "posixtime" option implemented by my patch.

-- 
Paul Collins
Wellington, New Zealand

Dag vijandelijk luchtschip de huismeester is dood
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fat/vfat: optionally ignore system timezone offset when reading/writing timestamps

2007-03-26 Thread OGAWA Hirofumi
Hiroyuki Machida <[EMAIL PROTECTED]> writes:

> I'm not famillar with recent fat code, but code itself looks good for
> just turn on/off time adjusting. On the other hand, I feel we need more 
> consideration on use cases/requirements. I feel that turning off
> time adjustment is a just ad-hoc solution to issues like Paul san 
> brought up.

Thank you. I see. So we need "timezone" option to specify adjusted
time?  If so, I feel we can add it as "timezone=utc", then it'll can
be improved later...
-- 
OGAWA Hirofumi <[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] fat/vfat: optionally ignore system timezone offset when reading/writing timestamps

2007-03-25 Thread Hiroyuki Machida
HI OGAWA-san and Paul-san,

Sorry late response,

I'm not famillar with recent fat code, but code itself looks good for
just turn on/off time adjusting. On the other hand, I feel we need more 
consideration on use cases/requirements. I feel that turning off
time adjustment is a just ad-hoc solution to issues like Paul san 
brought up.

Thanks,
Hiroyuki
 

OGAWA Hirofumi <[EMAIL PROTECTED]> wrote on 2007/03/19 23:31:06:
> Paul Collins <[EMAIL PROTECTED]> writes:
> 
> > Hello,
> 
> Hello,
> 
> > Here is a patch that adds a mount option named "posixtime" that, when
> > enabled, causes the fat/vfat code to not adjust timestamps as they are
> > read/written to/from disk.  The intent of the adjustment as performed
> > by the existing code appears to be to present correct timestamps to
> > Windows and friends, which treat the timestamp values as local time.
> >
> > However, the systems that I use to update my FAT32 filesystems are all
> > running Linux, and as such will process POSIX timestamps correctly.
> > (The filesystems are on disks in digital audio players, which do not
> > process timestamps except to check if they have changed.)  Due to the
> > aforementioned adjustment on read/write, after a Daylight Savings
> > transition I must update the file timestamps on the filesystems so
> > that rsync does not needlessly re-copy files.
> >
> > Please review and consider applying this patch.
> 
> Thanks. Looks good to me as start. IIRC, Machida-san did this few
> years ago. Machida-san, do you have any comment?
> 
> > diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> > index c16af24..d6c0a7f 100644
> > --- a/fs/fat/dir.c
> > +++ b/fs/fat/dir.c
> > @@ -1064,7 +1064,7 @@ int fat_alloc_new_dir(struct inode *dir, struct 
> timespec *ts)
> >goto error_free;
> >   }
> >
> > - fat_date_unix2dos(ts->tv_sec, &time, &date);
> > + fat_date_unix2dos(ts->tv_sec, &time, &date, sbi->options.adjust);
> >
> >   de = (struct msdos_dir_entry *)bhs[0]->b_data;
> >   /* filling the new directory slots ("." and ".." entries) */
> > diff --git a/fs/fat/inode.c b/fs/fat/inode.c
> > index a9e4688..02d9225 100644
> > --- a/fs/fat/inode.c
> > +++ b/fs/fat/inode.c
> > @@ -374,17 +374,20 @@ static int fat_fill_inode(struct inode *inode, 
> struct msdos_dir_entry *de)
> >   inode->i_blocks = ((inode->i_size + (sbi->cluster_size - 1))
> >& ~((loff_t)sbi->cluster_size - 1)) >> 9;
> >   inode->i_mtime.tv_sec =
> > -  date_dos2unix(le16_to_cpu(de->time), le16_to_cpu(de->date));
> > +  date_dos2unix(le16_to_cpu(de->time), le16_to_cpu(de->date),
> > + sbi->options.adjust);
> >   inode->i_mtime.tv_nsec = 0;
> >   if (sbi->options.isvfat) {
> >int secs = de->ctime_cs / 100;
> >int csecs = de->ctime_cs % 100;
> >inode->i_ctime.tv_sec  =
> > date_dos2unix(le16_to_cpu(de->ctime),
> > -  le16_to_cpu(de->cdate)) + secs;
> > +  le16_to_cpu(de->cdate),
> > +  sbi->options.adjust) + secs;
> >inode->i_ctime.tv_nsec = csecs * 1000;
> >inode->i_atime.tv_sec =
> > -   date_dos2unix(0, le16_to_cpu(de->adate));
> > +   date_dos2unix(0, le16_to_cpu(de->adate),
> > +  sbi->options.adjust);
> >inode->i_atime.tv_nsec = 0;
> >   } else
> >inode->i_ctime = inode->i_atime = inode->i_mtime;
> > @@ -592,11 +595,14 @@ retry:
> >   raw_entry->attr = fat_attr(inode);
> >   raw_entry->start = cpu_to_le16(MSDOS_I(inode)->i_logstart);
> >   raw_entry->starthi = cpu_to_le16(MSDOS_I(inode)->i_logstart >> 16);
> > - fat_date_unix2dos(inode->i_mtime.tv_sec, &raw_entry->time, 
> &raw_entry->date);
> > + fat_date_unix2dos(inode->i_mtime.tv_sec, &raw_entry->time, 
> &raw_entry->date,
> > + sbi->options.adjust);
> >   if (sbi->options.isvfat) {
> >__le16 atime;
> > - 
> fat_date_unix2dos(inode->i_ctime.tv_sec,&raw_entry->ctime,&raw_entry->cd
> ate);
> > -  fat_date_unix2dos(inode->i_atime.tv_sec,&atime,&raw_entry->adate);
> > + 
> fat_date_unix2dos(inode->i_ctime.tv_sec,&raw_entry->ctime,&raw_entry->cd
> ate,
> > +  sbi->options.adjust);
> > +  fat_date_unix2dos(inode->i_atime.tv_sec,&atime,&raw_entry->adate,
> > +  sbi->options.adjust);
> >raw_entry->ctime_cs = (inode->i_ctime.tv_sec & 1) * 100 +
> > inode->i_ctime.tv_nsec / 1000;
> >   }
> > @@ -854,7 +860,7 @@ enum {
> >   Opt_charset, Opt_shortname_lower, Opt_shortname_win95,
> >   Opt_shortname_winnt, Opt_shortname_mixed, Opt_utf8_no, Opt_utf8_yes,
> >   Opt_uni_xl_no, Opt_uni_xl_yes, Opt_nonumtail_no, Opt_nonumtail_yes,
> > - Opt_obsolate, Opt_flush, Opt_err,
> > + Opt_obsolate, Opt_flush, Opt_posixtime, Opt_err,
> >  };
> >
> >  static match_table_t fat_tokens = {
> > @@ -887,6 +893,7 @@ static match_table_t fat_tokens = {
> >   {Opt_obsolate, "cvf_options=%100s"},
> >   {Opt_obsolate, "posix"},
> >   {Opt_flush, "flush"},
> > + {Opt_posixtime, "posixtime"},
> >   {Opt_err, NULL},
> >  };
> >  static match_table_t msdos_tokens = {
> > @@ -950,6 +957,7 @@ static int parse_options(char *options, int 
> is_vfat, int silent, i

Re: [PATCH] fat/vfat: optionally ignore system timezone offset when reading/writing timestamps

2007-03-19 Thread OGAWA Hirofumi
Paul Collins <[EMAIL PROTECTED]> writes:

> Hello,

Hello,

> Here is a patch that adds a mount option named "posixtime" that, when
> enabled, causes the fat/vfat code to not adjust timestamps as they are
> read/written to/from disk.  The intent of the adjustment as performed
> by the existing code appears to be to present correct timestamps to
> Windows and friends, which treat the timestamp values as local time.
>
> However, the systems that I use to update my FAT32 filesystems are all
> running Linux, and as such will process POSIX timestamps correctly.
> (The filesystems are on disks in digital audio players, which do not
> process timestamps except to check if they have changed.)  Due to the
> aforementioned adjustment on read/write, after a Daylight Savings
> transition I must update the file timestamps on the filesystems so
> that rsync does not needlessly re-copy files.
>
> Please review and consider applying this patch.

Thanks. Looks good to me as start. IIRC, Machida-san did this few
years ago. Machida-san, do you have any comment?

> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index c16af24..d6c0a7f 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -1064,7 +1064,7 @@ int fat_alloc_new_dir(struct inode *dir, struct 
> timespec *ts)
>   goto error_free;
>   }
>  
> - fat_date_unix2dos(ts->tv_sec, &time, &date);
> + fat_date_unix2dos(ts->tv_sec, &time, &date, sbi->options.adjust);
>  
>   de = (struct msdos_dir_entry *)bhs[0]->b_data;
>   /* filling the new directory slots ("." and ".." entries) */
> diff --git a/fs/fat/inode.c b/fs/fat/inode.c
> index a9e4688..02d9225 100644
> --- a/fs/fat/inode.c
> +++ b/fs/fat/inode.c
> @@ -374,17 +374,20 @@ static int fat_fill_inode(struct inode *inode, struct 
> msdos_dir_entry *de)
>   inode->i_blocks = ((inode->i_size + (sbi->cluster_size - 1))
>  & ~((loff_t)sbi->cluster_size - 1)) >> 9;
>   inode->i_mtime.tv_sec =
> - date_dos2unix(le16_to_cpu(de->time), le16_to_cpu(de->date));
> + date_dos2unix(le16_to_cpu(de->time), le16_to_cpu(de->date),
> +   sbi->options.adjust);
>   inode->i_mtime.tv_nsec = 0;
>   if (sbi->options.isvfat) {
>   int secs = de->ctime_cs / 100;
>   int csecs = de->ctime_cs % 100;
>   inode->i_ctime.tv_sec  =
>   date_dos2unix(le16_to_cpu(de->ctime),
> -   le16_to_cpu(de->cdate)) + secs;
> +   le16_to_cpu(de->cdate),
> +   sbi->options.adjust) + secs;
>   inode->i_ctime.tv_nsec = csecs * 1000;
>   inode->i_atime.tv_sec =
> - date_dos2unix(0, le16_to_cpu(de->adate));
> + date_dos2unix(0, le16_to_cpu(de->adate),
> +   sbi->options.adjust);
>   inode->i_atime.tv_nsec = 0;
>   } else
>   inode->i_ctime = inode->i_atime = inode->i_mtime;
> @@ -592,11 +595,14 @@ retry:
>   raw_entry->attr = fat_attr(inode);
>   raw_entry->start = cpu_to_le16(MSDOS_I(inode)->i_logstart);
>   raw_entry->starthi = cpu_to_le16(MSDOS_I(inode)->i_logstart >> 16);
> - fat_date_unix2dos(inode->i_mtime.tv_sec, &raw_entry->time, 
> &raw_entry->date);
> + fat_date_unix2dos(inode->i_mtime.tv_sec, &raw_entry->time, 
> &raw_entry->date,
> +   sbi->options.adjust);
>   if (sbi->options.isvfat) {
>   __le16 atime;
> - 
> fat_date_unix2dos(inode->i_ctime.tv_sec,&raw_entry->ctime,&raw_entry->cdate);
> - 
> fat_date_unix2dos(inode->i_atime.tv_sec,&atime,&raw_entry->adate);
> + 
> fat_date_unix2dos(inode->i_ctime.tv_sec,&raw_entry->ctime,&raw_entry->cdate,
> +   sbi->options.adjust);
> + 
> fat_date_unix2dos(inode->i_atime.tv_sec,&atime,&raw_entry->adate,
> +   sbi->options.adjust);
>   raw_entry->ctime_cs = (inode->i_ctime.tv_sec & 1) * 100 +
>   inode->i_ctime.tv_nsec / 1000;
>   }
> @@ -854,7 +860,7 @@ enum {
>   Opt_charset, Opt_shortname_lower, Opt_shortname_win95,
>   Opt_shortname_winnt, Opt_shortname_mixed, Opt_utf8_no, Opt_utf8_yes,
>   Opt_uni_xl_no, Opt_uni_xl_yes, Opt_nonumtail_no, Opt_nonumtail_yes,
> - Opt_obsolate, Opt_flush, Opt_err,
> + Opt_obsolate, Opt_flush, Opt_posixtime, Opt_err,
>  };
>  
>  static match_table_t fat_tokens = {
> @@ -887,6 +893,7 @@ static match_table_t fat_tokens = {
>   {Opt_obsolate, "cvf_options=%100s"},
>   {Opt_obsolate, "posix"},
>   {Opt_flush, "flush"},
> + {Opt_posixtime, "posixtime"},
>   {Opt_err, NULL},
>  };
>  static match_table_t msdos_tokens = {
> @@ -950,6 +957,7 @@ static int parse_options(char *options, int is_vfat, int 
> silent, int *debug,
>   opts->utf8 = opts->unicode_xlate = 0;
> 

[PATCH] fat/vfat: optionally ignore system timezone offset when reading/writing timestamps

2007-03-19 Thread Paul Collins
Hello,

Here is a patch that adds a mount option named "posixtime" that, when
enabled, causes the fat/vfat code to not adjust timestamps as they are
read/written to/from disk.  The intent of the adjustment as performed
by the existing code appears to be to present correct timestamps to
Windows and friends, which treat the timestamp values as local time.

However, the systems that I use to update my FAT32 filesystems are all
running Linux, and as such will process POSIX timestamps correctly.
(The filesystems are on disks in digital audio players, which do not
process timestamps except to check if they have changed.)  Due to the
aforementioned adjustment on read/write, after a Daylight Savings
transition I must update the file timestamps on the filesystems so
that rsync does not needlessly re-copy files.

Please review and consider applying this patch.

Signed-off-by: Paul Collins <[EMAIL PROTECTED]>


diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index c16af24..d6c0a7f 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1064,7 +1064,7 @@ int fat_alloc_new_dir(struct inode *dir, struct timespec 
*ts)
goto error_free;
}
 
-   fat_date_unix2dos(ts->tv_sec, &time, &date);
+   fat_date_unix2dos(ts->tv_sec, &time, &date, sbi->options.adjust);
 
de = (struct msdos_dir_entry *)bhs[0]->b_data;
/* filling the new directory slots ("." and ".." entries) */
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index a9e4688..02d9225 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -374,17 +374,20 @@ static int fat_fill_inode(struct inode *inode, struct 
msdos_dir_entry *de)
inode->i_blocks = ((inode->i_size + (sbi->cluster_size - 1))
   & ~((loff_t)sbi->cluster_size - 1)) >> 9;
inode->i_mtime.tv_sec =
-   date_dos2unix(le16_to_cpu(de->time), le16_to_cpu(de->date));
+   date_dos2unix(le16_to_cpu(de->time), le16_to_cpu(de->date),
+ sbi->options.adjust);
inode->i_mtime.tv_nsec = 0;
if (sbi->options.isvfat) {
int secs = de->ctime_cs / 100;
int csecs = de->ctime_cs % 100;
inode->i_ctime.tv_sec  =
date_dos2unix(le16_to_cpu(de->ctime),
- le16_to_cpu(de->cdate)) + secs;
+ le16_to_cpu(de->cdate),
+ sbi->options.adjust) + secs;
inode->i_ctime.tv_nsec = csecs * 1000;
inode->i_atime.tv_sec =
-   date_dos2unix(0, le16_to_cpu(de->adate));
+   date_dos2unix(0, le16_to_cpu(de->adate),
+ sbi->options.adjust);
inode->i_atime.tv_nsec = 0;
} else
inode->i_ctime = inode->i_atime = inode->i_mtime;
@@ -592,11 +595,14 @@ retry:
raw_entry->attr = fat_attr(inode);
raw_entry->start = cpu_to_le16(MSDOS_I(inode)->i_logstart);
raw_entry->starthi = cpu_to_le16(MSDOS_I(inode)->i_logstart >> 16);
-   fat_date_unix2dos(inode->i_mtime.tv_sec, &raw_entry->time, 
&raw_entry->date);
+   fat_date_unix2dos(inode->i_mtime.tv_sec, &raw_entry->time, 
&raw_entry->date,
+ sbi->options.adjust);
if (sbi->options.isvfat) {
__le16 atime;
-   
fat_date_unix2dos(inode->i_ctime.tv_sec,&raw_entry->ctime,&raw_entry->cdate);
-   
fat_date_unix2dos(inode->i_atime.tv_sec,&atime,&raw_entry->adate);
+   
fat_date_unix2dos(inode->i_ctime.tv_sec,&raw_entry->ctime,&raw_entry->cdate,
+ sbi->options.adjust);
+   
fat_date_unix2dos(inode->i_atime.tv_sec,&atime,&raw_entry->adate,
+ sbi->options.adjust);
raw_entry->ctime_cs = (inode->i_ctime.tv_sec & 1) * 100 +
inode->i_ctime.tv_nsec / 1000;
}
@@ -854,7 +860,7 @@ enum {
Opt_charset, Opt_shortname_lower, Opt_shortname_win95,
Opt_shortname_winnt, Opt_shortname_mixed, Opt_utf8_no, Opt_utf8_yes,
Opt_uni_xl_no, Opt_uni_xl_yes, Opt_nonumtail_no, Opt_nonumtail_yes,
-   Opt_obsolate, Opt_flush, Opt_err,
+   Opt_obsolate, Opt_flush, Opt_posixtime, Opt_err,
 };
 
 static match_table_t fat_tokens = {
@@ -887,6 +893,7 @@ static match_table_t fat_tokens = {
{Opt_obsolate, "cvf_options=%100s"},
{Opt_obsolate, "posix"},
{Opt_flush, "flush"},
+   {Opt_posixtime, "posixtime"},
{Opt_err, NULL},
 };
 static match_table_t msdos_tokens = {
@@ -950,6 +957,7 @@ static int parse_options(char *options, int is_vfat, int 
silent, int *debug,
opts->utf8 = opts->unicode_xlate = 0;
opts->numtail = 1;
opts->nocase = 0;
+   opts->adjust = 1;
*debug = 0;
 
if (!options)
@@ -1032,6 +1040,10 @@ static int parse_options(char *options, int is_vfat, int 
sile