[Libguestfs] [PATCH 1/2] New API: get_min_size

[Richard W.M. Jones]

On Fri, Oct 16, 2015 at 07:44:20PM +0300, Maxim Perevedentsev wrote:
> This call provides the way to get minimum size of filesystem.
> This is needed for shrinking. The return units are bytes.

Shall we call it 'vfs_min_size'?  There are already other
'vfs_*' functions like 'vfs_type' and 'vfs_label'.

> +/* libguestfs - the guestfsd daemon
> + * Copyright (C) 2012 Red Hat Inc.

You probably want to change at least the date, and possibly the
copyright attribution too.  Distributing the code ownership helps to
protect the freedom of the code, by making it difficult to make
proprietary in future.

> diff --git a/daemon/ntfs.c b/daemon/ntfs.c
> index 1ead159..fd77716 100644
> --- a/daemon/ntfs.c
> +++ b/daemon/ntfs.c
> @@ -153,6 +153,74 @@ do_ntfsresize_size (const char *device, int64_t size)
>    return do_ntfsresize (device, size, 0);
>  }
> 
> +int64_t
> +ntfs_get_min_size (const char *device)
> +{
...

You must call reply_with_error or reply_with_perror, exactly once, on
every error path out of the do_* function.  The reply_with_* functions
are a part of the protocol -- they send data back to the library -- so
if they are not called or called too often, then the library and
daemon lose synch.

> +  lines = split_lines (out);
> +  if (lines == NULL)
> +    return -1;

Here, you *don't* need to call reply_with_*, because split_lines does
it already.  But ...

> +      else if ((p = strstr (lines[i], cluster_size_pattern))) {
> +        if (sscanf (p + strlen(cluster_size_pattern),
> +                    "%*[ ]:%" SCNd32, &cluster_size) != 1)
> +          return -1;

... here and several other places in this function you must call
reply_with_*.

[...]
> +        reply_with_error("%s", "Bad cluster size");

This should be: reply_with_error ("Bad cluster size");

> +    longdesc = "\
> +Get the minimum size of filesystem in bytes. This is the minimum possible size for filesystem shrinking.

You can break these long lines.  Empty line = paragraph break.

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