On 08/21/2014 06:51 PM, John Ferlan wrote:
>
>
> 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
How about 'fallback_to_mmap' instead of resize?
Or even simpler, we don't really need a different set of fallbacks for resize
than the other uses. It seems the patch adding the preallocation to FileResize
should have used safezero instead of reimplementing it.
(I also wonder if there are systems without posix_fallocate, but having the
syscall...)
ACK to safezero implementation with all four methods:
int
safezero(int fd, off_t offset, off_t len)
{
if (virFileFdPosixFallocate(fd, offset, len) == 0)
return 0;
if (safezero_sys_fallocate(fd, offset, len) == 0)
return 0;
if (safezero_mmap(fd, offset, len) == 0)
return 0;
return safezero_slow(fd, offset, len);
}
Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list