[libvirt] [PATCH] storage_backend: Fix issue with allocation of 0 length volume

[Daniel P. Berrange]

On Wed, Oct 09, 2013 at 10:17:19AM -0400, John Ferlan wrote:
> Commit id '532fef36' added a call to fallocate() and some error
> handling based on whether or not the function existed. This new
> call resulted in libvirt-cim/cimtest failures when attempting to
> create a volume with "0" (zero) allocation value. The failure is
> logged as:
> 
> Oct  9 07:51:33 localhost libvirtd[8030]: cannot allocate 0 bytes in
> file '/var/lib/libvirt/images/cimtest-vol.img': Invalid argument
> 
> This can also be seen with virsh vol-create-as:
> 
> error: Failed to create vol test
> error: cannot allocate 0 bytes in file '/home/vm-images/test': Invalid
> argument
> 
> error: Failed to create vol test
> error: cannot allocate 0 bytes in file '/home/vm-images/test': Invalid
> argument
> 
> It turns out fallocate() will return EINVAL when the incoming 'len'
> (or allocation) value is 0 (or less).
> ---
>  src/storage/storage_backend.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 662af32..d0580f9 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -332,19 +332,22 @@ createRawFile(int fd, virStorageVolDefPtr vol,
>  /* Avoid issues with older kernel's <linux/fs.h> namespace pollution. */
>  #if HAVE_FALLOCATE - 0
>      /* Try to preallocate all requested disk space, but fall back to
> -     * other methods if this fails with ENOSYS or EOPNOTSUPP.
> +     * other methods if this fails with ENOSYS or EOPNOTSUPP. If allocation
> +     * is 0 (or less than 0), then fallocate will fail with EINVAL.
>       * NOTE: do not use posix_fallocate; posix_fallocate falls back
>       * to writing zeroes block by block in case fallocate isn't
>       * available, and since we're going to copy data from another
>       * file it doesn't make sense to write the file twice. */
> -    if (fallocate(fd, 0, 0, vol->allocation) == 0) {
> -        need_alloc = 0;
> -    } else if (errno != ENOSYS && errno != EOPNOTSUPP) {
> -        ret = -errno;
> -        virReportSystemError(errno,
> -                             _("cannot allocate %llu bytes in file '%s'"),
> -                             vol->allocation, vol->target.path);
> -        goto cleanup;
> +    if (vol->allocation) {
> +        if (fallocate(fd, 0, 0, vol->allocation) == 0) {
> +            need_alloc = 0;
> +        } else if (errno != ENOSYS && errno != EOPNOTSUPP) {
> +            ret = -errno;
> +            virReportSystemError(errno,
> +                                 _("cannot allocate %llu bytes in file '%s'"),
> +                                 vol->allocation, vol->target.path);
> +            goto cleanup;
> +        }
>      }
>  #endif

ACK


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|