Re: [PATCH] Implement the sync libnetfs stubs.

[olafBuddenhagen]

Hi,

On Wed, Jul 08, 2009 at 10:30:41PM +0300, Sergiu Ivanov wrote:

> diff --git a/netfs.c b/netfs.c
> index 89d1bf6..d8211e0 100644
> --- a/netfs.c
> +++ b/netfs.c
> @@ -1,7 +1,10 @@
>  /* Hurd unionfs
> -   Copyright (C) 2001, 2002, 2003, 2005 Free Software Foundation, Inc.
> +   Copyright (C) 2001, 2002, 2003, 2005, 2009 Free Software Foundation, Inc.
> +
>     Written by Moritz Schulte <moritz@duesseldorf.ccc.de>.
>  
> +   Modified by Sergiu Ivanov <unlimitedscolobb@gmail.com>.
> +

You can do this if it's important to you -- but so far it's not common
in the Hurd to include author information for every change made to a
file... In fact, many files do not even have the original author
mentioned.

>     This program is free software; you can redistribute it and/or
>     modify it under the terms of the GNU General Public License as
>     published by the Free Software Foundation; either version 2 of the
> @@ -282,7 +285,36 @@ error_t
>  netfs_attempt_sync (struct iouser *cred, struct node *np,
>                   int wait)
>  {
> -  return EOPNOTSUPP;
> +  error_t err = 0;
> +
> +  /* The index of the currently analyzed filesystem.  */
> +  int i = 0;
> +
> +  /* The information about the currently analyzed filesystem.  */
> +  ulfs_t * ulfs;
> +
> +  mutex_lock (&ulfs_lock);
> +
> +  /* Sync every writable directory associated with `np`.   */
> +  node_ulfs_iterate_unlocked (np)
> +  {
> +    /* Get the information about the current filesystem.  */
> +    err = ulfs_get_num (i, &ulfs);

I don't think you really got the idea of the iterator... No need for
"i".

> +    if (err)
> +      break;

I wonder whether it wouldn't perhaps be better to continue in spite of
errors?...

> +
> +    if (ulfs->flags & FLAG_ULFS_WRITABLE)
> +      {
> +     err = file_sync (node_ulfs->port, wait, 0);
> +     if (err)
> +       break;
> +      }
> +
> +    ++i;
> +  }
> +
> +  mutex_unlock (&ulfs_lock);
> +  return err;
>  }
>  
>  /* This should sync the entire remote filesystem.  If WAIT is set,
> @@ -290,7 +322,10 @@ netfs_attempt_sync (struct iouser *cred, struct node *np,
>  error_t
>  netfs_attempt_syncfs (struct iouser *cred, int wait)
>  {
> -  return 0;
> +  /* The complete list of ports to the merged filesystems is
> +     maintained in the root node of unionfs, so if we sync it, we sync
> +     every single merged directory.  */
> +  return netfs_attempt_sync (cred, netfs_root_node, wait);

I'm don't think this is really the right approach. Why not forward the
syncfs to all unioned filesystems?

-antrik-