Hi, On Wednesday January 21 2009 at 09:15:07 PM +0100, Bruno De Fraine wrote: > On 21-jan-09, at 19:20, Loïc Le Loarer wrote: > >> The problem is that the cp command which "update" the database from >> /var/cache/sanesecurity/phish.ndb to >> /var/lib/clamav/sanesecurity-phish.ndb doesn't preverve the time. So the >> destination takes the date where the update happens. >> >> If the mirror is long to take the new file, the new file can have a date >> which is older than the time at which you launch the update script. And >> as the two files dates are compared before the the final copy, the problem >> happens. > > This is well-spotted. It is not sane that the file in $cache_dir retains > the original timestamp, while the file in $data_dir receives the > timestamp of updating, and the two timestamps are compared. > >> Here is a patch which use rsync instead of cp for the final copy, it >> preverves the time and do the copy atomically (because rsync do), which >> avoid any problem if clamav reload the database during the cp. > > To preserve the timestamp of a local copy, the simplest solution would > be "cp -a". True, or better cp --preserve=timestamps (we don't need -d and -R and --preserve=mode,ownership). This is enough to fix the upper bug. > What makes you conclude rsync copies atomically? This is the way it works (unless you use the --inplace option). rsync first create a temporary file for the new version and then do a rename to the new name. On the contrary, cp is overwritting the existing file, so if you read the file at the same time cp is running, you see a file which is a mix of the previous and the new one (or a truncated version of the new, it depends if cp first truncates, I don't remember). So when multiple application are using the same files at the same time, cp should be avoided and an atomic rename should be done. Instead of using rsync, you can also do : cp orig tmp mv tmp final It has the same effect. See this extract from the rename system call man page: -------- int rename(const char *oldpath, const char *newpath); If newpath already exists it will be atomically replaced (subject to a few conditions; see ERRORS below), so that there is no point at which another process attempting to access newpath will find it missing. -------- See also this from rsync man page: -------- --inplace: WARNING: The file's data will be in an inconsistent state during the transfer (and possibly afterward if the transfer gets interrupted), so you should not use this option to update files that are in use. Also note that rsync will be unable to update a file in-place that is not writable by the receiving user. ---------- > Also, your > patch changes the current directory, but I don't think that is necessary > ($db includes a full path). I don't know why yet but the command 'rsync --perms --times "$db" "$clamd_dbdir/sanesecurity-$db_name"' fails if rsync doesn't have write access to the current working dirrectory (this is what I tryed to explain in the comment of the 'cd' command). It is strange and I haven't find out why yet. Anyway, for a script which is launch from cron, it isn't a bad idea to change current working directory to a known location... Best regards. -- Loïc