>From 3ec1380eb6425b4f73024200817bd6b192d3b0b0 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Tue, 9 Feb 2010 18:00:24 +0000 Subject: [PATCH] Use mount-options instead of mount to avoid implicit -o sync. guestfs_mount adds -o sync implicitly. This causes a very large performance problem for write-intensive programs (eg. virt-v2v). Document this as a "gotcha". Change the tests, guestfish, Sys::Guestfs::Lib, guestmount to use mount-options instead. (Note that this gotcha does not affect mount-ro). The source of the performance problem was first identified by Matthew Booth. --- fish/fish.c | 11 ++++-- fuse/guestmount.c | 11 ++++-- perl/lib/Sys/Guestfs/Lib.pm | 2 +- perl/t/060-readdir.t | 2 +- regressions/rhbz503169c10.sh | 2 +- regressions/rhbz503169c13.sh | 2 +- .../test-cancellation-upload-daemoncancels.sh | 2 +- regressions/test-remote.sh | 2 +- src/generator.ml | 34 ++++++++++---------- src/guestfs.pod | 9 +++++ test-tool/test-tool.c | 2 +- 11 files changed, 47 insertions(+), 32 deletions(-) diff --git a/fish/fish.c b/fish/fish.c index dd73af7..bc518da 100644 --- a/fish/fish.c +++ b/fish/fish.c @@ -474,10 +474,13 @@ mount_mps (struct mp *mp) if (mp) { mount_mps (mp->next); - if (!read_only) - r = guestfs_mount (g, mp->device, mp->mountpoint); - else - r = guestfs_mount_ro (g, mp->device, mp->mountpoint); + + /* Don't use guestfs_mount here because that will default to mount + * options -o sync,noatime. For more information, see guestfs(3) + * section "LIBGUESTFS GOTCHAS". + */ + const char *options = !read_only ? "" : "ro"; + r = guestfs_mount_options (g, options, mp->device, mp->mountpoint); if (r == -1) exit (EXIT_FAILURE); } diff --git a/fuse/guestmount.c b/fuse/guestmount.c index c935493..cbd39f2 100644 --- a/fuse/guestmount.c +++ b/fuse/guestmount.c @@ -1160,10 +1160,13 @@ mount_mps (struct mp *mp) if (mp) { mount_mps (mp->next); - if (!read_only) - r = guestfs_mount (g, mp->device, mp->mountpoint); - else - r = guestfs_mount_ro (g, mp->device, mp->mountpoint); + + /* Don't use guestfs_mount here because that will default to mount + * options -o sync,noatime. For more information, see guestfs(3) + * section "LIBGUESTFS GOTCHAS". + */ + const char *options = !read_only ? "" : "ro"; + r = guestfs_mount_options (g, options, mp->device, mp->mountpoint); if (r == -1) exit (EXIT_FAILURE); } diff --git a/perl/lib/Sys/Guestfs/Lib.pm b/perl/lib/Sys/Guestfs/Lib.pm index 49c08b3..21c9a64 100644 --- a/perl/lib/Sys/Guestfs/Lib.pm +++ b/perl/lib/Sys/Guestfs/Lib.pm @@ -1282,7 +1282,7 @@ sub mount_operating_system if($ro) { $g->mount_ro ($mounts->{$_}, $_) } else { - $g->mount ($mounts->{$_}, $_) + $g->mount_options ("", $mounts->{$_}, $_) } } } diff --git a/perl/t/060-readdir.t b/perl/t/060-readdir.t index 5ed5b7a..dc8278b 100644 --- a/perl/t/060-readdir.t +++ b/perl/t/060-readdir.t @@ -38,7 +38,7 @@ $h->part_disk ("/dev/sda", "mbr"); ok (1); $h->mkfs ("ext2", "/dev/sda1"); ok (1); -$h->mount ("/dev/sda1", "/"); +$h->mount_options ("", "/dev/sda1", "/"); ok (1); $h->mkdir ("/p"); ok (1); diff --git a/regressions/rhbz503169c10.sh b/regressions/rhbz503169c10.sh index 5cf7069..0a32749 100755 --- a/regressions/rhbz503169c10.sh +++ b/regressions/rhbz503169c10.sh @@ -28,7 +28,7 @@ dd if=/dev/zero of=test1.img bs=1024k count=10 launch part-disk /dev/sda mbr mkfs ext2 /dev/sda1 -mount /dev/sda1 / +mount-options "" /dev/sda1 / ll /../dev/console ll /../dev/full ll /../dev/mapper/ diff --git a/regressions/rhbz503169c13.sh b/regressions/rhbz503169c13.sh index d360d5c..f7ad9e4 100755 --- a/regressions/rhbz503169c13.sh +++ b/regressions/rhbz503169c13.sh @@ -33,7 +33,7 @@ dd if=/dev/zero of=test1.img bs=1024k count=10 run part-disk /dev/sda mbr mkfs ext2 /dev/sda1 -mount /dev/sda1 / +mount-options "" /dev/sda1 / mkdir /dev -command /ignore-this-error unmount-all diff --git a/regressions/test-cancellation-upload-daemoncancels.sh b/regressions/test-cancellation-upload-daemoncancels.sh index 296d368..4962c25 100755 --- a/regressions/test-cancellation-upload-daemoncancels.sh +++ b/regressions/test-cancellation-upload-daemoncancels.sh @@ -30,7 +30,7 @@ run part-disk /dev/sda mbr mkfs ext2 /dev/sda1 -mount /dev/sda1 / +mount-options "" /dev/sda1 / # Upload image, daemon should cancel because the image is too large # to upload into itself. diff --git a/regressions/test-remote.sh b/regressions/test-remote.sh index d778a07..40c2ee9 100755 --- a/regressions/test-remote.sh +++ b/regressions/test-remote.sh @@ -28,7 +28,7 @@ eval `../fish/guestfish --listen` ../fish/guestfish --remote run ../fish/guestfish --remote part-disk /dev/sda mbr ../fish/guestfish --remote mkfs ext2 /dev/sda1 -../fish/guestfish --remote mount /dev/sda1 / +../fish/guestfish --remote mount-options "" /dev/sda1 / # Failure of the above commands will cause the guestfish listener to exit. # Incorrect return from echo_daemon will not, so need to ensure the listener diff --git a/src/generator.ml b/src/generator.ml index b4bbf1e..44dc8a5 100755 --- a/src/generator.ml +++ b/src/generator.ml @@ -1432,7 +1432,7 @@ on the volume group C, with C megabytes."); [InitEmpty, Always, TestOutput ( [["part_disk"; "/dev/sda"; "mbr"]; ["mkfs"; "ext2"; "/dev/sda1"]; - ["mount"; "/dev/sda1"; "/"]; + ["mount_options"; ""; "/dev/sda1"; "/"]; ["write_file"; "/new"; "new file contents"; "0"]; ["cat"; "/new"]], "new file contents")], "make a filesystem", @@ -1508,12 +1508,12 @@ use C."); [InitEmpty, Always, TestOutputListOfDevices ( [["part_disk"; "/dev/sda"; "mbr"]; ["mkfs"; "ext2"; "/dev/sda1"]; - ["mount"; "/dev/sda1"; "/"]; + ["mount_options"; ""; "/dev/sda1"; "/"]; ["mounts"]], ["/dev/sda1"]); InitEmpty, Always, TestOutputList ( [["part_disk"; "/dev/sda"; "mbr"]; ["mkfs"; "ext2"; "/dev/sda1"]; - ["mount"; "/dev/sda1"; "/"]; + ["mount_options"; ""; "/dev/sda1"; "/"]; ["umount"; "/"]; ["mounts"]], [])], "unmount a filesystem", @@ -1544,11 +1544,11 @@ See also: C"); ["mkfs"; "ext2"; "/dev/sda1"]; ["mkfs"; "ext2"; "/dev/sda2"]; ["mkfs"; "ext2"; "/dev/sda3"]; - ["mount"; "/dev/sda1"; "/"]; + ["mount_options"; ""; "/dev/sda1"; "/"]; ["mkdir"; "/mp1"]; - ["mount"; "/dev/sda2"; "/mp1"]; + ["mount_options"; ""; "/dev/sda2"; "/mp1"]; ["mkdir"; "/mp1/mp2"]; - ["mount"; "/dev/sda3"; "/mp1/mp2"]; + ["mount_options"; ""; "/dev/sda3"; "/mp1/mp2"]; ["mkdir"; "/mp1/mp2/mp3"]; ["umount_all"]; ["mounts"]], [])], @@ -2405,11 +2405,11 @@ the human-readable, canonical hex dump of the file."); [InitNone, Always, TestOutput ( [["part_disk"; "/dev/sda"; "mbr"]; ["mkfs"; "ext3"; "/dev/sda1"]; - ["mount"; "/dev/sda1"; "/"]; + ["mount_options"; ""; "/dev/sda1"; "/"]; ["write_file"; "/new"; "test file"; "0"]; ["umount"; "/dev/sda1"]; ["zerofree"; "/dev/sda1"]; - ["mount"; "/dev/sda1"; "/"]; + ["mount_options"; ""; "/dev/sda1"; "/"]; ["cat"; "/new"]], "test file")], "zero unused inodes and disk blocks on ext2/3 filesystem", "\ @@ -2510,13 +2510,13 @@ are activated or deactivated."); ["vgcreate"; "VG"; "/dev/sda1"]; ["lvcreate"; "LV"; "VG"; "10"]; ["mkfs"; "ext2"; "/dev/VG/LV"]; - ["mount"; "/dev/VG/LV"; "/"]; + ["mount_options"; ""; "/dev/VG/LV"; "/"]; ["write_file"; "/new"; "test content"; "0"]; ["umount"; "/"]; ["lvresize"; "/dev/VG/LV"; "20"]; ["e2fsck_f"; "/dev/VG/LV"]; ["resize2fs"; "/dev/VG/LV"]; - ["mount"; "/dev/VG/LV"; "/"]; + ["mount_options"; ""; "/dev/VG/LV"; "/"]; ["cat"; "/new"]], "test content")], "resize an LVM logical volume", "\ @@ -3560,7 +3560,7 @@ and C"); [InitEmpty, Always, TestOutput ( [["part_disk"; "/dev/sda"; "mbr"]; ["mkfs_b"; "ext2"; "4096"; "/dev/sda1"]; - ["mount"; "/dev/sda1"; "/"]; + ["mount_options"; ""; "/dev/sda1"; "/"]; ["write_file"; "/new"; "new file contents"; "0"]; ["cat"; "/new"]], "new file contents")], "make a filesystem with block size", @@ -3575,7 +3575,7 @@ are C<1024>, C<2048> or C<4096> only."); [["sfdiskM"; "/dev/sda"; ",100 ,"]; ["mke2journal"; "4096"; "/dev/sda1"]; ["mke2fs_J"; "ext2"; "4096"; "/dev/sda2"; "/dev/sda1"]; - ["mount"; "/dev/sda2"; "/"]; + ["mount_options"; ""; "/dev/sda2"; "/"]; ["write_file"; "/new"; "new file contents"; "0"]; ["cat"; "/new"]], "new file contents")], "make ext2/3/4 external journal", @@ -3590,7 +3590,7 @@ to the command: [["sfdiskM"; "/dev/sda"; ",100 ,"]; ["mke2journal_L"; "4096"; "JOURNAL"; "/dev/sda1"]; ["mke2fs_JL"; "ext2"; "4096"; "/dev/sda2"; "JOURNAL"]; - ["mount"; "/dev/sda2"; "/"]; + ["mount_options"; ""; "/dev/sda2"; "/"]; ["write_file"; "/new"; "new file contents"; "0"]; ["cat"; "/new"]], "new file contents")], "make ext2/3/4 external journal with label", @@ -3603,7 +3603,7 @@ This creates an ext2 external journal on C with label C