[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH] Use mount-options instead of mount to avoid implicit -o sync.



On 09/02/10 18:38, Richard W.M. Jones wrote:
> 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";

Can you make that:

const char *options = read_only ? "ro" : "";

Extra negations hurt my feeble mind ;)

> +    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";

And again.

> +    r = guestfs_mount_options (g, options, mp->device, mp->mountpoint);
>      if (r == -1)
>        exit (EXIT_FAILURE);
>    }

ACK, regardless of above.

Matt

-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]