On 08/21/2014 11:53 AM, Ján Tomko wrote:
> On 08/11/2014 10:30 PM, John Ferlan wrote:
>> Currently virStorageFileResize() function uses build conditionals to
>> choose either the posix_fallocate() or mmap() with no fallback in order
>> to preallocate the space in the newly resized file.
>>
>> This patch will modify the logic in order to allow fallbacks in the event
>> that posix_fallocate() or the sys_fallocate syscall() doesn't work properly.
>> The fallback will be to use the slow safewrite of zero filled buffers to
>> the file.
>>
>> Use the virFileFdPosixFallocate() rather than a local function.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/util/virstoragefile.c | 52 +++++++++++++++++++++++++++++++----------------
>> 1 file changed, 34 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
>> index 5b6b2f5..4d37de1 100644
>> --- a/src/util/virstoragefile.c
>> +++ b/src/util/virstoragefile.c
>> @@ -1086,6 +1086,39 @@ virStorageFileChainGetBroken(virStorageSourcePtr chain,
>> return 0;
>> }
>>
>> +static int
>> +resizeSysFallocate(const char *path,
>> + int fd,
>> + off_t offset,
>> + off_t len)
>> +{
>> + int rc = -1;
>> +#if HAVE_SYS_SYSCALL_H && defined(SYS_fallocate)
>> + if ((rc = syscall(SYS_fallocate, fd, 0, offset, len)) != 0) {
>> + virReportSystemError(errno,
>> + _("Failed to pre-allocate space for "
>> + "file '%s'"), path);
>
> I think this should not log an error, since we have a fallback.
> VIR_DEBUG maybe?
>
Yep - right. Although to match the other path, VIR_WARN...
>> + }
>> +#endif
>> + return rc;
>> +}
>> +
>> +static int
>> +doResize(const char *path,
>> + int fd,
>> + off_t offset,
>> + off_t len)
>> +{
>> + if (virFileFdPosixFallocate(fd, offset, len) == 0 ||
>> + resizeSysFallocate(path, fd, offset, len) == 0 ||
>> + safezero(fd, offset, len) == 0)
>> + return 0;
>> +
>> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>> + _("preallocate is not supported on this platform"));
>> + return -1;
>
> safezero is always supported. And this function should do:
> return safezero(fd, offset, len);
>
>
Hmm.. Perhaps a better way to do this would be to modify safezero() to
add a 4th boolean parameter "resize" and make the "safezero_mmap()" be
the false side and the check/call to SYS_fallocate() be the true side.
That way all the logic resides in virfile.c
Thus doResize() goes away and resizeSysFallocate() moves to virfile.c.
All virStorageFileResize() does for the "if (preAllocate)" condition "if
(safezero(...,true) < 0)"
See the attached patch file that "should" be able to be "git am"'d on
top of the existing patches (although I did rebase to top to tree this
morning).
Does that seem better?
John
>> +}
>> +
>>
>> /**
>> * virStorageFileResize:
>> @@ -1113,25 +1146,8 @@ virStorageFileResize(const char *path,
>> }
>>
>> if (pre_allocate) {
>> -#if HAVE_POSIX_FALLOCATE
>> - if ((rc = posix_fallocate(fd, offset, len)) != 0) {
>> - virReportSystemError(rc,
>> - _("Failed to pre-allocate space for "
>> - "file '%s'"), path);
>> - goto cleanup;
>> - }
>> -#elif HAVE_SYS_SYSCALL_H && defined(SYS_fallocate)
>> - if (syscall(SYS_fallocate, fd, 0, offset, len) != 0) {
>> - virReportSystemError(errno,
>> - _("Failed to pre-allocate space for "
>> - "file '%s'"), path);
>> + if (doResize(path, fd, offset, len) < 0)
>> goto cleanup;
>> - }
>> -#else
>> - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>> - _("preallocate is not supported on this platform"));
>> - goto cleanup;
>> -#endif
>> } else {
>> if (ftruncate(fd, capacity) < 0) {
>> virReportSystemError(errno,
>>
>
>
>From 94fce5c3b3ba6575998d6be7203b8e3a657c2bdf Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@xxxxxxxxxx>
Date: Thu, 21 Aug 2014 12:43:52 -0400
Subject: [PATCH] Rework virStorageFileResize
Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
src/locking/lock_driver_sanlock.c | 4 ++--
src/storage/storage_backend.c | 2 +-
src/util/virfile.c | 29 ++++++++++++++++++++++++++---
src/util/virfile.h | 2 +-
src/util/virstoragefile.c | 39 +--------------------------------------
5 files changed, 31 insertions(+), 45 deletions(-)
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index ea43051..b368f17 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -281,7 +281,7 @@ static int virLockManagerSanlockSetupLockspace(void)
/*
* Pre allocate enough data for 1 block of leases at preferred alignment
*/
- if (safezero(fd, 0, rv) < 0) {
+ if (safezero(fd, 0, rv, false) < 0) {
virReportSystemError(errno,
_("Unable to allocate lockspace %s"),
path);
@@ -690,7 +690,7 @@ static int virLockManagerSanlockCreateLease(struct sanlk_resource *res)
/*
* Pre allocate enough data for 1 block of leases at preferred alignment
*/
- if (safezero(fd, 0, rv) < 0) {
+ if (safezero(fd, 0, rv, false) < 0) {
virReportSystemError(errno,
_("Unable to allocate lease %s"),
res->disks[0].path);
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 9c775c9..a6be9d6 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -401,7 +401,7 @@ createRawFile(int fd, virStorageVolDefPtr vol,
}
if (remain && need_alloc) {
- if (safezero(fd, vol->target.allocation - remain, remain) < 0) {
+ if (safezero(fd, vol->target.allocation - remain, remain, false) < 0) {
ret = -errno;
virReportSystemError(errno, _("cannot fill file '%s'"),
vol->target.path);
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 6139810..a642e5c 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -42,6 +42,9 @@
#if HAVE_MMAP
# include <sys/mman.h>
#endif
+#if HAVE_SYS_SYSCALL_H
+# include <sys/syscall.h>
+#endif
#ifdef __linux__
# if HAVE_LINUX_MAGIC_H
@@ -1104,6 +1107,21 @@ safezero_mmap(int fd, off_t offset, off_t len)
}
static int
+safezero_sys_fallocate(int fd,
+ off_t offset,
+ off_t len)
+{
+ int rc = -1;
+#if HAVE_SYS_SYSCALL_H && defined(SYS_fallocate)
+ if ((rc = syscall(SYS_fallocate, fd, 0, offset, len)) != 0) {
+ VIR_WARN("Failed to SYS_fallocate '%lu' bytes with errno=%d",
+ len, errno);
+ }
+#endif
+ return rc;
+}
+
+static int
safezero_slow(int fd, off_t offset, off_t len)
{
int r;
@@ -1141,12 +1159,17 @@ safezero_slow(int fd, off_t offset, off_t len)
}
int
-safezero(int fd, off_t offset, off_t len)
+safezero(int fd, off_t offset, off_t len, bool resize)
{
if (virFileFdPosixFallocate(fd, offset, len) == 0)
return 0;
- if (safezero_mmap(fd, offset, len) == 0)
- return 0;
+ if (resize) {
+ if (safezero_sys_fallocate(fd, offset, len) == 0)
+ return 0;
+ } else {
+ if (safezero_mmap(fd, offset, len) == 0)
+ return 0;
+ }
return safezero_slow(fd, offset, len);
}
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 29a1776..546f674 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -43,7 +43,7 @@ ssize_t safewrite(int fd, const void *buf, size_t count)
ATTRIBUTE_RETURN_CHECK;
int virFileFdPosixFallocate(int fd, off_t offset, off_t len)
ATTRIBUTE_RETURN_CHECK;
-int safezero(int fd, off_t offset, off_t len)
+int safezero(int fd, off_t offset, off_t len, bool resize)
ATTRIBUTE_RETURN_CHECK;
/* Don't call these directly - use the macros below */
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 4d37de1..77a39ac 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -43,9 +43,6 @@
#include "viruri.h"
#include "dirname.h"
#include "virbuffer.h"
-#if HAVE_SYS_SYSCALL_H
-# include <sys/syscall.h>
-#endif
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -1086,40 +1083,6 @@ virStorageFileChainGetBroken(virStorageSourcePtr chain,
return 0;
}
-static int
-resizeSysFallocate(const char *path,
- int fd,
- off_t offset,
- off_t len)
-{
- int rc = -1;
-#if HAVE_SYS_SYSCALL_H && defined(SYS_fallocate)
- if ((rc = syscall(SYS_fallocate, fd, 0, offset, len)) != 0) {
- virReportSystemError(errno,
- _("Failed to pre-allocate space for "
- "file '%s'"), path);
- }
-#endif
- return rc;
-}
-
-static int
-doResize(const char *path,
- int fd,
- off_t offset,
- off_t len)
-{
- if (virFileFdPosixFallocate(fd, offset, len) == 0 ||
- resizeSysFallocate(path, fd, offset, len) == 0 ||
- safezero(fd, offset, len) == 0)
- return 0;
-
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("preallocate is not supported on this platform"));
- return -1;
-}
-
-
/**
* virStorageFileResize:
*
@@ -1146,7 +1109,7 @@ virStorageFileResize(const char *path,
}
if (pre_allocate) {
- if (doResize(path, fd, offset, len) < 0)
+ if (safezero(fd, offset, len, true) < 0)
goto cleanup;
} else {
if (ftruncate(fd, capacity) < 0) {
--
1.9.3
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list