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

Re: [Libvir] PATCH: 10/16: general purpose helper APIs



On Tue, Feb 12, 2008 at 04:37:08AM +0000, Daniel P. Berrange wrote:
> This patch provides a couple of helper APIs used by the forthcoming
> driver backends.
> 
>    - virStorageBackendUpdateVolInfo - take a filename and virStorageVolPtr
>      and update the capacity/allocation information based on the file
>      stat() results. Also update the permissions data on owner/mode
>      and SELinux label.
> 
>    - virStorageBackendUpdateVolInfoFD - same as above but with a pre-opened
>      filehandle
> 
>    - virStorageBackendStablePath - given a /dev/XXX node, and a directory
>      of symlinks (eg /dev/disk/by-path), attempts to find a symlink pointing
>      to the desired file.
> 
>    - virStorageBackendRunProgRegex - given one or more regexes and a command
>      line argv[], execute the program and attempt to match its STDOUT against
>      the regex. When complete matches are found invoke a callback.
> 
>    - virStorageBackendRunProgNul - given a command line argv[] and an expected
>      number of fields per record, tokenize the STDOUT into records splitting
>      on NULL, and invoke the callback per record.
> 
> The SELinux code is optional, and can be replaced with calls to any other
> library which can provide MAC file labels, or just disabled completely..
[...]
> +SELINUX_CFLAGS=
> +SELINUX_LIBS=
> +if test "$with_selinux" != "no"; then
> +  old_cflags="$CFLAGS"
> +  old_libs="$LIBS"
> +  if test "$with_selinux" = "check"; then
> +    AC_CHECK_HEADER([selinux/selinux.h],[],[with_selinux=no])
> +    AC_CHECK_LIB(selinux, fgetfilecon,[],[with_selinux=no])
> +    if test "$with_selinux" != "no"; then
> +      with_selinux="yes"
> +    fi
> +  else
> +    AC_CHECK_HEADER([selinux/selinux.h],[],
> +       [AC_MSG_ERROR([You must install the SELinux development package in order to compile libvirt])])
> +    AC_CHECK_LIB(selinux, fgetfilecon,[],
> +       [AC_MSG_ERROR([You must install the SELinux development package in order to compile and run libvirt])])

   Hum, does that mean that withough any configure flags and if SELinux is
not found then configure would fail ? It may be the right thing to do since
it's a security option (and hence deactivation as an opt-in looks reasonable)
but this may be viewed as a bit too Fedora centric :-)

[...]
> +    if (S_ISREG(sb.st_mode)) {
> +        vol->allocation = (unsigned long long)sb.st_blocks * (unsigned long long)512;

  Hum, I think this should be 

     sb.st_blocks * sb.st_blksize

since you can have 1k or 4k kind of filesystems too or at least to ensure
portability.

> +        /* Regular files may be sparse, so logical size (capacity) is not same
> +         * as actual allocation above
> +         */
> +        if (withCapacity)
> +            vol->capacity = sb.st_size;
> +    } else {
> +        off_t end;
> +        /* XXX this is POSIX compliant, but doesn't work for for CHAR files,
> +         * only BLOCK. There is a Linux specific ioctl() for getting
> +         * size of both CHAR / BLOCK devices we should check for in
> +         * configure
> +         */
> +        end = lseek(fd, 0, SEEK_END);
> +        if (end == (off_t)-1) {
> +            virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
> +                                  "cannot seek to end of file '%s': %d (%s)",
> +                                  vol->target.path, errno, strerror(errno));
> +            return -1;
> +        }
> +        vol->allocation = end;
> +        if (withCapacity) vol->capacity = end;
> +    }

  It would be good to see what this gives on Solaris, Windows ...

[...]
> +/*
> + * Run an external program.
> + *
> + * Read its output and apply a series of regexes to each line
> + * When the entire set of regexes has matched consequetively
> + * then run a callback passing in all the matches
> + */
> +int virStorageBackendRunProgRegex(virConnectPtr conn,
> +                                  virStoragePoolObjPtr pool,
> +                                  const char **prog,
> +                                  int nregex,
> +                                  const char **regex,
> +                                  int *nvars,
> +                                  virStorageBackendListVolRegexFunc func,
> +                                  void *data)

  Ouch, that's very very specialized ... but if needed okay

> +
> +/*
> + * Run an external program and read from its standard output
> + * a stream of tokens from IN_STREAM, applying FUNC to
> + * each successive sequence of N_COLUMNS tokens.
> + * If FUNC returns < 0, stop processing input and return -1.
> + * Return -1 if N_COLUMNS == 0.
> + * Return -1 upon memory allocation error.
> + * If the number of input tokens is not a multiple of N_COLUMNS,
> + * then the final FUNC call will specify a number smaller than N_COLUMNS.
> + * If there are no input tokens (empty input), call FUNC with N_COLUMNS == 0.
> + */
> +int virStorageBackendRunProgNul(virConnectPtr conn,
> +                                virStoragePoolObjPtr pool,
> +                                const char **prog,
> +                                size_t n_columns,
> +                                virStorageBackendListVolNulFunc func,
> +                                void *data)

  yeah we are reaching an impressive level of specialization there...

  okay, +1

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/


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