Re: [libvirt] [PATCH v2] Fix performance problem of virStorageVolCreateXMLFrom()

On 03/08/2011 04:06 AM, Minoru Usui wrote: Thanks for contributing! This looks like your first patch in, so I updated AUTHORS; let me know if I need to adjust any spelling. This needs indentation tweaking to keep 'make syntax-check' happy when cppi is installed. Technically, you don't need () around a single integer constant, but it didn't hurt to leave them in. Why isn't this 4k, though? What's wrong with fstat(fd) and using st.st_blksize on all other platforms where the ioctl doesn't exist? That may still be too small on some platforms, but it's more likely to be write than hard-coding things at 512 (for example, on Cygwin, st.st_blksize is 64k, which is indeed the optimal size for NTFS drives). Also, I don't know of any file system that supports sparse files with holes smaller than st_blksize. Here's what I squashed in before pushing: diff --git i/AUTHORS w/AUTHORS index c0c1780..0e56468 100644 --- i/AUTHORS +++ w/AUTHORS diff --git i/src/storage/storage_backend.c w/src/storage/storage_backend.c index b3eba10..fc08c68 100644 --- i/src/storage/storage_backend.c +++ w/src/storage/storage_backend.c @@ -1,7 +1,7 @@ /* * storage_backend.c: internal storage driver backend contract * - * Copyright (C) 2007-2010 Red Hat, Inc. + * Copyright (C) 2007-2011 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -37,8 +37,8 @@ #include <sys/param.h> #include <dirent.h> #ifdef __linux__ -#include <sys/ioctl.h> -#include <linux/fs.h> +# include <sys/ioctl.h> +# include <linux/fs.h> #endif #if HAVE_SELINUX @@ -113,7 +113,7 @@ enum { }; #define READ_BLOCK_SIZE_DEFAULT (1024 * 1024) -#define WRITE_BLOCK_SIZE_DEFAULT (512) +#define WRITE_BLOCK_SIZE_DEFAULT (4 * 1024) static int ATTRIBUTE_NONNULL (2) virStorageBackendCopyToFD(virStorageVolDefPtr vol, @@ -127,10 +127,11 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, int ret = 0; unsigned long long remain; size_t rbytes = READ_BLOCK_SIZE_DEFAULT; - size_t wbytes = WRITE_BLOCK_SIZE_DEFAULT; + size_t wbytes = 0; int interval; char *zerobuf; char *buf = NULL; + struct stat st; if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { ret = -errno; @@ -142,9 +143,13 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, #ifdef __linux__ if (ioctl(fd, BLKBSZGET, &wbytes) < 0) { - wbytes = WRITE_BLOCK_SIZE_DEFAULT; + wbytes = 0; } #endif + if ((wbytes == 0) && fstat(fd, &st) == 0) + wbytes = st.st_blksize; + if (wbytes < WRITE_BLOCK_SIZE_DEFAULT) + wbytes = WRITE_BLOCK_SIZE_DEFAULT; if (VIR_ALLOC_N(zerobuf, wbytes) < 0) { ret = -errno; -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org