Mon 25 Apr 2011 05:40:30 PM UTC, comment #19:
You don't have partitions.
|
Mon 25 Apr 2011 04:12:38 PM UTC, comment #18:
What I was checked:
# truncate -s 100M qwe
# mkdir qwed
# mkfs.ext4 qwe
# mount -o loop qwe qwed
# cp -a /boot/grub qwed/
# echo '(hd0) qwe' > qwe.map
# ./grub-setup -m qwe.map qwe -r '(hd0)' -s -f -d qwed/grub
# umount -f qwed
in both cases (patched and unpatched) operation is successfull.
What I do wrong ?
|
Mon 25 Apr 2011 12:09:41 PM UTC, comment #17:
>> Calling grub_util_biosdisk_flush in grub-setup makes the difference between failed and successfull install
>What did you mean ?
>grub_util_biosdisk_flush() is completely "read-only" i.e. does not change anything, return code does not checked anywhere.
You can try it yourself. Comment all the calls to grub_util_biosdisk_flush and try to install in blocklist mode (e.g. onto partition using --force).
|
Mon 25 Apr 2011 11:44:08 AM UTC, comment #16:
> Calling grub_util_biosdisk_flush in grub-setup makes the difference between failed and successfull install
What did you mean ?
grub_util_biosdisk_flush() is completely "read-only" i.e. does not change anything, return code does not checked anywhere.
|
Mon 25 Apr 2011 11:08:47 AM UTC, comment #15:
> grub_util_biosdisk_flush() tries to open device as RDONLY and flush() after that. This will not work.
Calling grub_util_biosdisk_flush in grub-setup makes the difference between failed and successfull install
|
Mon 25 Apr 2011 09:31:13 AM UTC, comment #14:
I have found some errors in code and provide a patch:
grub_util_biosdisk_flush():
grub_util_biosdisk_flush() tries to open device as RDONLY and flush() after that. This will not work. As I think, it should not try to open closed device, and second, it should not try to flush devices opened read-only. Also it should check return values of fsync() and ioctl()
open_device():
In case of error during open, disk structure will be corrupted (data->dev points to free()d location. data->fd close()d, but still > 0.)
(The same issue in second place of that function)
grub_util_biosdisk_close():
simplifiction for use new grub_util_biosdisk_flush().
(file #23298)
|
Fri 22 Apr 2011 06:01:26 PM UTC, comment #13:
Okay, just what I want to see :) Thanks. Really.
But I have some notes about mingw.
According to http://msdn.microsoft.com/en-us/library/17618685 fsync may be implemented as:
int fsync(int fd)
{
return _commit(fd);
}
/* Please do not implement as '#define fsync(x) _commit(x)' or '#define fsync _commit'. */
Googling says, that there is implmentation via FlushFileBuffers(), but http://support.microsoft.com/kb/99456 says, that
_commit is equivalent of FlushFileBuffers, so _commit() is preffered, as it automatically set errno in case of error.
2. Unused sync() stub for mingw should be removed, as I think.
|
Fri 22 Apr 2011 01:15:05 PM UTC, comment #12:
I went ahead and fixed this before rc2 since it created a real problem
|
Fri 15 Apr 2011 07:11:17 AM UTC, comment #11:
Remaining part is postponed after 1.99
|
Fri 15 Apr 2011 04:56:38 AM UTC, comment #10:
bump. Please reopen bug.
|
Sun 10 Apr 2011 04:27:31 PM UTC, comment #9:
open_device():
If we use fsync() before close(),
O_SYNC, O_FSYNC should not be used, as it give performance penalty.
When O_SYNC used, every write is blocked until it goes to disk, and fsync() does nothing.
When O_SYNC is not used, every write is not blocked, but all modified blocks are written inside fsync(). During fsync(), blocks may be merged, so not using O_SYNC may significantly improve performance.
So, see attached patch
(file #23191)
|
Sun 10 Apr 2011 04:14:04 PM UTC, comment #8:
Also, i'm not
"Mark Korenberger" :)
i'm
"Mark Korenberg"
|
Sun 10 Apr 2011 03:41:10 PM UTC, comment #7:
> Sync is still needed
No, it's not true.
sync() will sync all data on all (including innocent) filesystems.
So, if someone writes to another disk (suppose dropbox, email server, other disk-heavy apps), sync() never complete, and grub hang (for the time)!
fsync() is always sufficient, even without BLKFLSBUF, but BLKFLSBUF guarantee that data is flushed from disk hardware buffers to disk.
Never use sync() in any program. Also, sync() force kernel to allocate ALL blocks marked for delayed allocation....
|
Sun 10 Apr 2011 02:08:16 PM UTC, comment #6:
Thanks for reporting (2), fixed. Sync is still needed though because we do FS writes as well (e.g. creating/modifying core.img, copying modules)
|
Sat 09 Apr 2011 02:31:02 PM UTC, comment #5:
> If user wants to install to block device (normal usage) we should call ioctl()
I mean:
"If user wants to install to block device (normal usage) we should call fsync() + ioctl()"
Also, it will be nice if errors (return values of thess functions) will be checked...
|
Sat 09 Apr 2011 02:28:02 PM UTC, comment #4:
Okay, you just replace close() with fsync() + ioctl.
1. The original request was to replace sync() with appropriate call(s) (see util/grub-setup.c)
2. ioctl is appropriate only if opened file was "block device". issuing BLKFLSBUF on generic file may lead to unexpected behaviour. So, if user wants to install grub into generic file (disk image), we should call fsync() only. If user wants to install to block device (normal usage) we should call ioctl()
So, please reopen bug. bug is not closed. You accidentally closed another bug.
|
Sat 09 Apr 2011 02:14:17 PM UTC, comment #3:
It's in bzr. r3178
|
Sat 09 Apr 2011 02:03:19 PM UTC, comment #2:
Did not found changes in bzr.... how I can look to your changes?
|
Fri 08 Apr 2011 12:01:22 PM UTC, comment #1:
fixed.
|
Mon 14 Feb 2011 09:31:27 AM UTC, original submission:
It is safer to call fsync() on FD connected to device [flush kernel cache]. After which, ioctl(BLKFLSBUF) should follow [flush hardware cache]. This sequence guaranteed data flushing.
Now, GRUB only call sync() which does nothing.
In Linux, sync() has influence only to data written to file on a filesystem. Embedding GRUB into first sectors is not such case, so sync() will not work as expected, redundant, and also have performance issues (this call will blocked until all current and ongoing data on another drive will be written, so installing GRUB on massively used server may take much time)
|