Libguestfs - Oct 2017 - Re: [PATCH v2 2/2] builder: Choose better weights in the planner.

On Tuesday, 3 October 2017 14:24:14 CEST Richard W.M. Jones
wrote:
> ---
>  builder/builder.ml            | 84
+++++++++++++++++++++++++++++++++----------
Cannot comment on the actual logic in the planner, since I admit I did
not have an in-depth look at it.
> diff --git a/common/mlutils/unix_utils-c.c b/common/mlutils/unix_utils-c.c
> index 657852ef2..340fdec44 100644
> --- a/common/mlutils/unix_utils-c.c
> +++ b/common/mlutils/unix_utils-c.c
> @@ -28,10 +28,18 @@
>  #include <errno.h>
>  #include <sys/types.h>
>  
> +#ifdef HAVE_SYS_STATFS_H
> +#include <sys/statfs.h>
> +#endif
> +
>  #ifdef HAVE_SYS_STATVFS_H
>  #include <sys/statvfs.h>
>  #endif
>  
> +#ifdef HAVE_SYS_VFS_H
> +#include <sys/vfs.h>
> +#endif
> +
>  #if MAJOR_IN_MKDEV
>  #include <sys/mkdev.h>
>  #elif MAJOR_IN_SYSMACROS
> @@ -62,6 +70,7 @@ extern value guestfs_int_mllib_fsync_file (value
filenamev);
>  extern value guestfs_int_mllib_mkdtemp (value val_pattern);
>  extern value guestfs_int_mllib_realpath (value pathv);
>  extern value guestfs_int_mllib_statvfs_statvfs (value pathv);
> +extern value guestfs_int_mllib_statvfs_filesystem_is_remote (value pathv);
>  
>  /* NB: This is a "noalloc" call. */
>  value
> @@ -342,3 +351,21 @@ guestfs_int_mllib_statvfs_statvfs (value pathv)
>  
>    CAMLreturn (rv);
>  }
> +
> +/* NB: This is a "noalloc" call. */
> +value
> +guestfs_int_mllib_statvfs_filesystem_is_remote (value pathv)
> +{
> +#ifdef HAVE_STATFS
> +  struct statfs buf;
> +
> +  if (statfs (String_val (pathv), &buf) == -1)
> +    unix_error (errno, (char *) "statvfs", pathv);
> +
> +  return Val_bool (buf.f_type == 0xff534d42 /* CIFS_MAGIC_NUMBER */ ||
> +                   buf.f_type == 0x6969 /* NFS_SUPER_MAGIC */ ||
> +                   buf.f_type == 0x517b /* SMB_SUPER_MAGIC */);
Using linux/magic.h + the constants from it should avoid the hardcoded
numbers, and possibly allow more remote filesystems.  Not a big issue
otherwise, anyway.

-- 
Pino Toscano

On Wed, Oct 04, 2017 at 03:20:53PM +0200, Pino Toscano
wrote:
> > +  return Val_bool (buf.f_type == 0xff534d42 /* CIFS_MAGIC_NUMBER */
||
> > +                   buf.f_type == 0x6969 /* NFS_SUPER_MAGIC */ ||
> > +                   buf.f_type == 0x517b /* SMB_SUPER_MAGIC */);
> 
> Using linux/magic.h + the constants from it should avoid the hardcoded
> numbers, and possibly allow more remote filesystems.  Not a big issue
> otherwise, anyway.
I didn't see this email until after I'd sent v3, but I can add this.
Just a note that CIFS_MAGIC_NUMBER is not present in <linux/magic.h> ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On Wednesday, 4 October 2017 15:59:07 CEST Richard W.M. Jones
wrote:
> On Wed, Oct 04, 2017 at 03:20:53PM +0200, Pino Toscano wrote:
> > > +  return Val_bool (buf.f_type == 0xff534d42 /* CIFS_MAGIC_NUMBER
*/ ||
> > > +                   buf.f_type == 0x6969 /* NFS_SUPER_MAGIC */ ||
> > > +                   buf.f_type == 0x517b /* SMB_SUPER_MAGIC */);
> > 
> > Using linux/magic.h + the constants from it should avoid the hardcoded
> > numbers, and possibly allow more remote filesystems.  Not a big issue
> > otherwise, anyway.
> 
> I didn't see this email until after I'd sent v3, but I can add
this.
> Just a note that CIFS_MAGIC_NUMBER is not present in <linux/magic.h>
...
Ah OK, so I guess it is fine to use the local value for
CIFS_MAGIC_NUMBER (maybe as local constant/define).

Thanks,
-- 
Pino Toscano