On 03/08/2011 04:06 AM, Minoru Usui wrote:
> This patch changes zerobuf variable from array to VIR_ALLOC_N().
>
> Signed-off-by: Minoru Usui <usui@xxxxxxxxxxxxxxxxx>
Thanks for contributing! This looks like your first patch in, so I
updated AUTHORS; let me know if I need to adjust any spelling.
> ---
> src/storage/storage_backend.c | 38 +++++++++++++++++++++++++++++---------
> 1 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 2eede74..3a22da4 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -36,6 +36,10 @@
> #include <sys/stat.h>
> #include <sys/param.h>
> #include <dirent.h>
> +#ifdef __linux__
> +#include <sys/ioctl.h>
> +#include <linux/fs.h>
> +#endif
This needs indentation tweaking to keep 'make syntax-check' happy when
cppi is installed.
> +#define READ_BLOCK_SIZE_DEFAULT (1024 * 1024)
> +#define WRITE_BLOCK_SIZE_DEFAULT (512)
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?
> +#ifdef __linux__
> + if (ioctl(fd, BLKBSZGET, &wbytes) < 0) {
> + wbytes = WRITE_BLOCK_SIZE_DEFAULT;
> + }
> +#endif
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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list