On Fri, Jul 10, 2009 at 04:46:59PM -0400, Cole Robinson wrote: Do later patches have a dependancy on this ? The allocation progress tracking you mention is actually WRT to a 2nd thread calling virStorageVolGetInfo while the first thread is doing the work. AFAIK, that would not be impacted by either of these 2 code paths. The 'track_allocation_progress' variable was a placeholder for the case where we do proper async non-blocking vol creation from a single thread. By switching this code to use the iterative allocation, it will increase the number of ext4 extents required for large files which is not so desirable. So I think its better to just leave this code in as is, or remove the other code path. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

These will be used by other pool cloning implementations. --- src/storage_backend.c | 397 ++++++++++++++++++++++++++++++++++++++++++++ src/storage_backend.h | 17 ++ src/storage_backend_fs.c | 408 +--------------------------------------------- 3 files changed, 421 insertions(+), 401 deletions(-) diff --git a/src/storage_backend.c b/src/storage_backend.c index 953928e..59a24e1 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -95,6 +95,403 @@ static virStorageBackendPtr backends[] = { NULL }; +enum { + TOOL_QEMU_IMG, + TOOL_KVM_IMG, + TOOL_QCOW_CREATE, +}; + +int +virStorageBackendCreateRaw(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol) +{ + int fd = -1; + int inputfd = -1; + int ret = -1; + unsigned long long remain; + char *buf = NULL; + + if ((fd = open(vol->target.path, O_RDWR | O_CREAT | O_EXCL, + vol->target.perms.mode)) < 0) { + virReportSystemError(conn, errno, + _("cannot create path '%s'"), + vol->target.path); + goto cleanup; + } + + if (inputvol) { + if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("could not open input path '%s'"), + inputvol->target.path); + goto cleanup; + } + } + + /* Seek to the final size, so the capacity is available upfront + * for progress reporting */ + if (ftruncate(fd, vol->capacity) < 0) { + virReportSystemError(conn, errno, + _("cannot extend file '%s'"), + vol->target.path); + goto cleanup; + } + + remain = vol->allocation; + + if (inputfd != -1) { + int amtread = -1; + size_t bytes = 1024 * 1024; + char zerobuf[512]; + + bzero(&zerobuf, sizeof(zerobuf)); + + if (VIR_ALLOC_N(buf, bytes) < 0) { + virReportOOMError(conn); + goto cleanup; + } + + while (amtread != 0) { + int amtleft; + + if (remain < bytes) + bytes = remain; + + if ((amtread = saferead(inputfd, buf, bytes)) < 0) { + virReportSystemError(conn, errno, + _("failed reading from file '%s'"), + inputvol->target.path); + goto cleanup; + } + remain -= amtread; + + /* Loop over amt read in 512 byte increments, looking for sparse + * blocks */ + amtleft = amtread; + do { + int interval = ((512 > amtleft) ? amtleft : 512); + int offset = amtread - amtleft; + + if (memcmp(buf+offset, zerobuf, interval) == 0) { + if (lseek(fd, interval, SEEK_CUR) < 0) { + virReportSystemError(conn, errno, + _("cannot extend file '%s'"), + vol->target.path); + goto cleanup; + } + } else if (safewrite(fd, buf+offset, interval) < 0) { + virReportSystemError(conn, errno, + _("failed writing to file '%s'"), + vol->target.path); + goto cleanup; + + } + } while ((amtleft -= 512) > 0); + } + } + + /* Pre-allocate any data if requested */ + /* XXX slooooooooooooooooow on non-extents-based file systems */ + while (remain) { + /* Allocate in chunks of 512MiB: big-enough chunk + * size and takes approx. 9s on ext3. A progress + * update every 9s is a fair-enough trade-off + */ + unsigned long long bytes = 512 * 1024 * 1024; + int r; + + if (bytes > remain) + bytes = remain; + if ((r = safezero(fd, 0, vol->allocation - remain, bytes)) != 0) { + virReportSystemError(conn, r, _("cannot fill file '%s'"), + vol->target.path); + goto cleanup; + } + remain -= bytes; + } + + if (close(fd) < 0) { + virReportSystemError(conn, errno, + _("cannot close file '%s'"), + vol->target.path); + goto cleanup; + } + fd = -1; + + if (inputfd != -1 && close(inputfd) < 0) { + virReportSystemError(conn, errno, + _("cannot close file '%s'"), + inputvol->target.path); + goto cleanup; + } + inputfd = -1; + + ret = 0; +cleanup: + if (fd != -1) + close(fd); + if (inputfd != -1) + close(inputfd); + VIR_FREE(buf); + + return ret; +} + +static int +virStorageBackendCreateQemuImg(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol) +{ + char size[100]; + char *create_tool; + short use_kvmimg; + + const char *type = virStorageVolFormatFileSystemTypeToString(vol->target.format); + const char *backingType = vol->backingStore.path ? + virStorageVolFormatFileSystemTypeToString(vol->backingStore.format) : NULL; + + const char *inputBackingPath = (inputvol ? inputvol->backingStore.path + : NULL); + const char *inputPath = inputvol ? inputvol->target.path : NULL; + /* Treat input block devices as 'raw' format */ + const char *inputType = inputPath ? + virStorageVolFormatFileSystemTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ? VIR_STORAGE_VOL_FILE_RAW : inputvol->target.format) : + NULL; + + const char **imgargv; + const char *imgargvnormal[] = { + NULL, "create", + "-f", type, + vol->target.path, + size, + NULL, + }; + /* Extra NULL fields are for including "backingType" when using + * kvm-img. It's -F backingType + */ + const char *imgargvbacking[] = { + NULL, "create", + "-f", type, + "-b", vol->backingStore.path, + vol->target.path, + size, + NULL, + NULL, + NULL + }; + const char *convargv[] = { + NULL, "convert", + "-f", inputType, + "-O", type, + inputPath, + vol->target.path, + NULL, + }; + + if (type == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unknown storage vol type %d"), + vol->target.format); + return -1; + } + if (inputvol && inputType == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unknown storage vol type %d"), + inputvol->target.format); + return -1; + } + + if (vol->backingStore.path) { + + /* XXX: Not strictly required: qemu-img has an option a different + * backing store, not really sure what use it serves though, and it + * may cause issues with lvm. Untested essentially. + */ + if (inputvol && + (!inputBackingPath || + STRNEQ(inputBackingPath, vol->backingStore.path))) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("a different backing store can not " + "be specified.")); + return -1; + } + + if (backingType == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unknown storage vol backing store type %d"), + vol->backingStore.format); + return -1; + } + if (access(vol->backingStore.path, R_OK) != 0) { + virReportSystemError(conn, errno, + _("inaccessible backing store volume %s"), + vol->backingStore.path); + return -1; + } + } + + if ((create_tool = virFindFileInPath("kvm-img")) != NULL) + use_kvmimg = 1; + else if ((create_tool = virFindFileInPath("qemu-img")) != NULL) + use_kvmimg = 0; + else { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unable to find kvm-img or qemu-img")); + return -1; + } + + if (inputvol) { + convargv[0] = create_tool; + imgargv = convargv; + } else if (vol->backingStore.path) { + imgargvbacking[0] = create_tool; + if (use_kvmimg) { + imgargvbacking[6] = "-F"; + imgargvbacking[7] = backingType; + imgargvbacking[8] = vol->target.path; + imgargvbacking[9] = size; + } + imgargv = imgargvbacking; + } else { + imgargvnormal[0] = create_tool; + imgargv = imgargvnormal; + } + + + /* Size in KB */ + snprintf(size, sizeof(size), "%llu", vol->capacity/1024); + + if (virRun(conn, imgargv, NULL) < 0) { + VIR_FREE(imgargv[0]); + return -1; + } + + VIR_FREE(imgargv[0]); + + return 0; +} + +/* + * Xen removed the fully-functional qemu-img, and replaced it + * with a partially functional qcow-create. Go figure ??!? + */ +static int +virStorageBackendCreateQcowCreate(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol) +{ + char size[100]; + const char *imgargv[4]; + + if (inputvol) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", + _("cannot copy from volume with qcow-create")); + return -1; + } + + if (vol->target.format != VIR_STORAGE_VOL_FILE_QCOW2) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unsupported storage vol type %d"), + vol->target.format); + return -1; + } + if (vol->backingStore.path != NULL) { + virStorageReportError(conn, VIR_ERR_NO_SUPPORT, + _("copy-on-write image not supported with " + "qcow-create")); + return -1; + } + + /* Size in MB - yes different units to qemu-img :-( */ + snprintf(size, sizeof(size), "%llu", vol->capacity/1024/1024); + + imgargv[0] = virFindFileInPath("qcow-create"); + imgargv[1] = size; + imgargv[2] = vol->target.path; + imgargv[3] = NULL; + + if (virRun(conn, imgargv, NULL) < 0) { + VIR_FREE(imgargv[0]); + return -1; + } + + VIR_FREE(imgargv[0]); + + return 0; +} + +createFile +virStorageBackendFSImageToolTypeToFunc(virConnectPtr conn, int tool_type) +{ + switch (tool_type) { + case TOOL_KVM_IMG: + case TOOL_QEMU_IMG: + return virStorageBackendCreateQemuImg; + case TOOL_QCOW_CREATE: + return virStorageBackendCreateQcowCreate; + default: + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Unknown file create tool type '%d'."), + tool_type); + } + + return NULL; +} + +int +virStorageBackendFindFSImageTool(char **tool) +{ + int tool_type = -1; + char *tmp = NULL; + + if ((tmp = virFindFileInPath("kvm-img")) != NULL) { + tool_type = TOOL_KVM_IMG; + } else if ((tmp = virFindFileInPath("qemu-img")) != NULL) { + tool_type = TOOL_QEMU_IMG; + } else if ((tmp = virFindFileInPath("qcow-create")) != NULL) { + tool_type = TOOL_QCOW_CREATE; + } + + if (tool) + *tool = tmp; + else + VIR_FREE(tmp); + + return tool_type; +} + +createFile +virStorageBackendGetBuildVolFromFunction(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol) +{ + int tool_type; + + if (!inputvol) + return NULL; + + /* If either volume is a non-raw file vol, we need to use an external + * tool for converting + */ + if ((vol->type == VIR_STORAGE_VOL_FILE && + vol->target.format != VIR_STORAGE_VOL_FILE_RAW) || + (inputvol->type == VIR_STORAGE_VOL_FILE && + inputvol->target.format != VIR_STORAGE_VOL_FILE_RAW)) { + + if ((tool_type = virStorageBackendFindFSImageTool(NULL)) != -1) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("creation of non-raw file images is " + "not supported without qemu-img.")); + return NULL; + } + + return virStorageBackendFSImageToolTypeToFunc(conn, tool_type); + } + + return virStorageBackendCreateRaw; +} #if defined(UDEVADM) || defined(UDEVSETTLE) void virWaitForDevices(virConnectPtr conn) diff --git a/src/storage_backend.h b/src/storage_backend.h index a3f441c..ec75f41 100644 --- a/src/storage_backend.h +++ b/src/storage_backend.h @@ -40,6 +40,23 @@ typedef int (*virStorageBackendRefreshVol)(virConnectPtr conn, virStoragePoolObj typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, virStorageVolDefPtr origvol, virStorageVolDefPtr newvol, unsigned int flags); +typedef int (*createFile)(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol); + +/* File creation/cloning functions used for cloning between backends */ +int virStorageBackendCreateRaw(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol); +createFile +virStorageBackendGetBuildVolFromFunction(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol); +int virStorageBackendFindFSImageTool(char **tool); +createFile virStorageBackendFSImageToolTypeToFunc(virConnectPtr conn, + int tool_type); + + typedef struct _virStorageBackend virStorageBackend; typedef virStorageBackend *virStorageBackendPtr; diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index c7c1c9c..3b2af59 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -55,12 +55,6 @@ enum { BACKING_STORE_ERROR, }; -enum { - TOOL_QEMU_IMG, - TOOL_KVM_IMG, - TOOL_QCOW_CREATE, -}; - static int cowGetBackingStore(virConnectPtr, char **, const unsigned char *, size_t); static int qcowXGetBackingStore(virConnectPtr, char **, @@ -68,10 +62,6 @@ static int qcowXGetBackingStore(virConnectPtr, char **, static int vmdk4GetBackingStore(virConnectPtr, char **, const unsigned char *, size_t); -typedef int (*createFile)(virConnectPtr conn, - virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol); - /* Either 'magic' or 'extension' *must* be provided */ struct FileTypeInfo { int type; /* One of the constants above */ @@ -1016,142 +1006,6 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn, return 0; } -static int createRaw(virConnectPtr conn, - virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) { - int fd = -1; - int inputfd = -1; - int ret = -1; - unsigned long long remain; - char *buf = NULL; - - if ((fd = open(vol->target.path, O_RDWR | O_CREAT | O_EXCL, - vol->target.perms.mode)) < 0) { - virReportSystemError(conn, errno, - _("cannot create path '%s'"), - vol->target.path); - goto cleanup; - } - - if (inputvol) { - if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("could not open input path '%s'"), - inputvol->target.path); - goto cleanup; - } - } - - /* Seek to the final size, so the capacity is available upfront - * for progress reporting */ - if (ftruncate(fd, vol->capacity) < 0) { - virReportSystemError(conn, errno, - _("cannot extend file '%s'"), - vol->target.path); - goto cleanup; - } - - remain = vol->allocation; - - if (inputfd != -1) { - int amtread = -1; - size_t bytes = 1024 * 1024; - char zerobuf[512]; - - bzero(&zerobuf, sizeof(zerobuf)); - - if (VIR_ALLOC_N(buf, bytes) < 0) { - virReportOOMError(conn); - goto cleanup; - } - - while (amtread != 0) { - int amtleft; - - if (remain < bytes) - bytes = remain; - - if ((amtread = saferead(inputfd, buf, bytes)) < 0) { - virReportSystemError(conn, errno, - _("failed reading from file '%s'"), - inputvol->target.path); - goto cleanup; - } - remain -= amtread; - - /* Loop over amt read in 512 byte increments, looking for sparse - * blocks */ - amtleft = amtread; - do { - int interval = ((512 > amtleft) ? amtleft : 512); - int offset = amtread - amtleft; - - if (memcmp(buf+offset, zerobuf, interval) == 0) { - if (lseek(fd, interval, SEEK_CUR) < 0) { - virReportSystemError(conn, errno, - _("cannot extend file '%s'"), - vol->target.path); - goto cleanup; - } - } else if (safewrite(fd, buf+offset, interval) < 0) { - virReportSystemError(conn, errno, - _("failed writing to file '%s'"), - vol->target.path); - goto cleanup; - - } - } while ((amtleft -= 512) > 0); - } - } - - /* Pre-allocate any data if requested */ - /* XXX slooooooooooooooooow on non-extents-based file systems */ - while (remain) { - /* Allocate in chunks of 512MiB: big-enough chunk - * size and takes approx. 9s on ext3. A progress - * update every 9s is a fair-enough trade-off - */ - unsigned long long bytes = 512 * 1024 * 1024; - int r; - - if (bytes > remain) - bytes = remain; - if ((r = safezero(fd, 0, vol->allocation - remain, bytes)) != 0) { - virReportSystemError(conn, r, - _("cannot fill file '%s'"), - vol->target.path); - goto cleanup; - } - remain -= bytes; - } - - if (close(fd) < 0) { - virReportSystemError(conn, errno, - _("cannot close file '%s'"), - vol->target.path); - goto cleanup; - } - fd = -1; - - if (inputfd != -1 && close(inputfd) < 0) { - virReportSystemError(conn, errno, - _("cannot close file '%s'"), - inputvol->target.path); - goto cleanup; - } - inputfd = -1; - - ret = 0; -cleanup: - if (fd != -1) - close(fd); - if (inputfd != -1) - close(inputfd); - VIR_FREE(buf); - - return ret; -} - static int createFileDir(virConnectPtr conn, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol) { @@ -1172,257 +1026,6 @@ static int createFileDir(virConnectPtr conn, return 0; } -static int createQemuImg(virConnectPtr conn, - virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) { - char size[100]; - char *create_tool; - short use_kvmimg; - - const char *type = virStorageVolFormatFileSystemTypeToString(vol->target.format); - const char *backingType = vol->backingStore.path ? - virStorageVolFormatFileSystemTypeToString(vol->backingStore.format) : NULL; - - const char *inputBackingPath = (inputvol ? inputvol->backingStore.path - : NULL); - const char *inputPath = inputvol ? inputvol->target.path : NULL; - /* Treat input block devices as 'raw' format */ - const char *inputType = inputPath ? - virStorageVolFormatFileSystemTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ? VIR_STORAGE_VOL_FILE_RAW : inputvol->target.format) : - NULL; - - const char **imgargv; - const char *imgargvnormal[] = { - NULL, "create", - "-f", type, - vol->target.path, - size, - NULL, - }; - /* Extra NULL fields are for including "backingType" when using - * kvm-img. It's -F backingType - */ - const char *imgargvbacking[] = { - NULL, "create", - "-f", type, - "-b", vol->backingStore.path, - vol->target.path, - size, - NULL, - NULL, - NULL - }; - const char *convargv[] = { - NULL, "convert", - "-f", inputType, - "-O", type, - inputPath, - vol->target.path, - NULL, - }; - - if (type == NULL) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("unknown storage vol type %d"), - vol->target.format); - return -1; - } - if (inputvol && inputType == NULL) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("unknown storage vol type %d"), - inputvol->target.format); - return -1; - } - - if (vol->backingStore.path) { - - /* XXX: Not strictly required: qemu-img has an option a different - * backing store, not really sure what use it serves though, and it - * may cause issues with lvm. Untested essentially. - */ - if (inputvol && - (!inputBackingPath || - STRNEQ(inputBackingPath, vol->backingStore.path))) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("a different backing store can not " - "be specified.")); - return -1; - } - - if (backingType == NULL) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("unknown storage vol backing store type %d"), - vol->backingStore.format); - return -1; - } - if (access(vol->backingStore.path, R_OK) != 0) { - virReportSystemError(conn, errno, - _("inaccessible backing store volume %s"), - vol->backingStore.path); - return -1; - } - } - - if ((create_tool = virFindFileInPath("kvm-img")) != NULL) - use_kvmimg = 1; - else if ((create_tool = virFindFileInPath("qemu-img")) != NULL) - use_kvmimg = 0; - else { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("unable to find kvm-img or qemu-img")); - return -1; - } - - if (inputvol) { - convargv[0] = create_tool; - imgargv = convargv; - } else if (vol->backingStore.path) { - imgargvbacking[0] = create_tool; - if (use_kvmimg) { - imgargvbacking[6] = "-F"; - imgargvbacking[7] = backingType; - imgargvbacking[8] = vol->target.path; - imgargvbacking[9] = size; - } - imgargv = imgargvbacking; - } else { - imgargvnormal[0] = create_tool; - imgargv = imgargvnormal; - } - - - /* Size in KB */ - snprintf(size, sizeof(size), "%llu", vol->capacity/1024); - - if (virRun(conn, imgargv, NULL) < 0) { - VIR_FREE(imgargv[0]); - return -1; - } - - VIR_FREE(imgargv[0]); - - return 0; -} - -/* - * Xen removed the fully-functional qemu-img, and replaced it - * with a partially functional qcow-create. Go figure ??!? - */ -static int createQemuCreate(virConnectPtr conn, - virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) { - char size[100]; - const char *imgargv[4]; - - if (inputvol) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", - _("cannot copy from volume with qcow-create")); - return -1; - } - - if (vol->target.format != VIR_STORAGE_VOL_FILE_QCOW2) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("unsupported storage vol type %d"), - vol->target.format); - return -1; - } - if (vol->backingStore.path != NULL) { - virStorageReportError(conn, VIR_ERR_NO_SUPPORT, - _("copy-on-write image not supported with " - "qcow-create")); - return -1; - } - - /* Size in MB - yes different units to qemu-img :-( */ - snprintf(size, sizeof(size), "%llu", vol->capacity/1024/1024); - - imgargv[0] = virFindFileInPath("qcow-create"); - imgargv[1] = size; - imgargv[2] = vol->target.path; - imgargv[3] = NULL; - - if (virRun(conn, imgargv, NULL) < 0) { - VIR_FREE(imgargv[0]); - return -1; - } - - VIR_FREE(imgargv[0]); - - return 0; -} - -static createFile -toolTypeToFunction(virConnectPtr conn, int tool_type) -{ - switch (tool_type) { - case TOOL_KVM_IMG: - case TOOL_QEMU_IMG: - return createQemuImg; - case TOOL_QCOW_CREATE: - return createQemuCreate; - default: - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Unknown file create tool type '%d'."), - tool_type); - } - - return NULL; -} - -static int -findImageTool(char **tool) -{ - int tool_type = -1; - char *tmp = NULL; - - if ((tmp = virFindFileInPath("kvm-img")) != NULL) { - tool_type = TOOL_KVM_IMG; - } else if ((tmp = virFindFileInPath("qemu-img")) != NULL) { - tool_type = TOOL_QEMU_IMG; - } else if ((tmp = virFindFileInPath("qcow-create")) != NULL) { - tool_type = TOOL_QCOW_CREATE; - } - - if (tool) - *tool = tmp; - else - VIR_FREE(tmp); - - return tool_type; -} - -static createFile -buildVolFromFunction(virConnectPtr conn, - virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) -{ - int tool_type; - - if (!inputvol) - return NULL; - - /* If either volume is a non-raw file vol, we need to use an external - * tool for converting - */ - if ((vol->type == VIR_STORAGE_VOL_FILE && - vol->target.format != VIR_STORAGE_VOL_FILE_RAW) || - (inputvol->type == VIR_STORAGE_VOL_FILE && - inputvol->target.format != VIR_STORAGE_VOL_FILE_RAW)) { - - if ((tool_type = findImageTool(NULL)) != -1) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("creation of non-raw file images is " - "not supported without qemu-img.")); - return NULL; - } - - return toolTypeToFunction(conn, tool_type); - } - - return createRaw; -} - static int _virStorageBackendFileSystemVolBuild(virConnectPtr conn, virStorageVolDefPtr vol, @@ -1433,15 +1036,18 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, int tool_type; if (inputvol) { - create_func = buildVolFromFunction(conn, vol, inputvol); + create_func = virStorageBackendGetBuildVolFromFunction(conn, vol, + inputvol); if (!create_func) return -1; } else if (vol->target.format == VIR_STORAGE_VOL_FILE_RAW) { - create_func = createRaw; + create_func = virStorageBackendCreateRaw; } else if (vol->target.format == VIR_STORAGE_VOL_FILE_DIR) { create_func = createFileDir; - } else if ((tool_type = findImageTool(NULL)) != -1) { - if ((create_func = toolTypeToFunction(conn, tool_type)) == NULL) + } else if ((tool_type = virStorageBackendFindFSImageTool(NULL)) != -1) { + create_func = virStorageBackendFSImageToolTypeToFunc(conn, tool_type); + + if (!create_func) return -1; } else { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, -- 1.6.0.6

Have storage building functions be definitions of virStorageBackendBuildVolFrom: we will need to do this in the future anyways if we ever support the flags attribute. --- src/storage_backend.c | 13 ++++++++----- src/storage_backend.h | 14 ++++++-------- src/storage_backend_fs.c | 7 ++++--- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/storage_backend.c b/src/storage_backend.c index 59a24e1..aea6105 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -104,7 +104,8 @@ enum { int virStorageBackendCreateRaw(virConnectPtr conn, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) + virStorageVolDefPtr inputvol, + unsigned int flags ATTRIBUTE_UNUSED) { int fd = -1; int inputfd = -1; @@ -241,7 +242,8 @@ cleanup: static int virStorageBackendCreateQemuImg(virConnectPtr conn, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) + virStorageVolDefPtr inputvol, + unsigned int flags ATTRIBUTE_UNUSED) { char size[100]; char *create_tool; @@ -379,7 +381,8 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, static int virStorageBackendCreateQcowCreate(virConnectPtr conn, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) + virStorageVolDefPtr inputvol, + unsigned int flags ATTRIBUTE_UNUSED) { char size[100]; const char *imgargv[4]; @@ -422,7 +425,7 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn, return 0; } -createFile +virStorageBackendBuildVolFrom virStorageBackendFSImageToolTypeToFunc(virConnectPtr conn, int tool_type) { switch (tool_type) { @@ -462,7 +465,7 @@ virStorageBackendFindFSImageTool(char **tool) return tool_type; } -createFile +virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virConnectPtr conn, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol) diff --git a/src/storage_backend.h b/src/storage_backend.h index ec75f41..a948c01 100644 --- a/src/storage_backend.h +++ b/src/storage_backend.h @@ -40,21 +40,19 @@ typedef int (*virStorageBackendRefreshVol)(virConnectPtr conn, virStoragePoolObj typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags); typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, virStorageVolDefPtr origvol, virStorageVolDefPtr newvol, unsigned int flags); -typedef int (*createFile)(virConnectPtr conn, - virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol); - /* File creation/cloning functions used for cloning between backends */ int virStorageBackendCreateRaw(virConnectPtr conn, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol); -createFile + virStorageVolDefPtr inputvol, + unsigned int flags); +virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virConnectPtr conn, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol); int virStorageBackendFindFSImageTool(char **tool); -createFile virStorageBackendFSImageToolTypeToFunc(virConnectPtr conn, - int tool_type); +virStorageBackendBuildVolFrom +virStorageBackendFSImageToolTypeToFunc(virConnectPtr conn, + int tool_type); diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index 3b2af59..8cfc462 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -1008,7 +1008,8 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn, static int createFileDir(virConnectPtr conn, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol) { + virStorageVolDefPtr inputvol, + unsigned int flags ATTRIBUTE_UNUSED) { if (inputvol) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", @@ -1032,7 +1033,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, virStorageVolDefPtr inputvol) { int fd; - createFile create_func; + virStorageBackendBuildVolFrom create_func; int tool_type; if (inputvol) { @@ -1056,7 +1057,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, return -1; } - if (create_func(conn, vol, inputvol) < 0) + if (create_func(conn, vol, inputvol, 0) < 0) return -1; if ((fd = open(vol->target.path, O_RDONLY)) < 0) { -- 1.6.0.6

The CreateRaw function has some 'file' only assumptions, so break the agnostic cloning bits to a separate function. --- src/storage_backend.c | 160 ++++++++++++++++++++++++++++--------------------- 1 files changed, 92 insertions(+), 68 deletions(-) diff --git a/src/storage_backend.c b/src/storage_backend.c index aea6105..868c297 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -101,6 +101,95 @@ enum { TOOL_QCOW_CREATE, }; +static int +virStorageBackendCopyToFD(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + int fd, + unsigned long long *total) +{ + int inputfd = -1; + int amtread = -1; + int ret = -1; + unsigned long long remain; + size_t bytes = 1024 * 1024; + char zerobuf[512]; + char *buf = NULL; + + if (inputvol) { + if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("could not open input path '%s'"), + inputvol->target.path); + goto cleanup; + } + } + + bzero(&zerobuf, sizeof(zerobuf)); + + if (VIR_ALLOC_N(buf, bytes) < 0) { + virReportOOMError(conn); + goto cleanup; + } + + remain = *total; + + while (amtread != 0) { + int amtleft; + + if (remain < bytes) + bytes = remain; + + if ((amtread = saferead(inputfd, buf, bytes)) < 0) { + virReportSystemError(conn, errno, + _("failed reading from file '%s'"), + inputvol->target.path); + goto cleanup; + } + remain -= amtread; + + /* Loop over amt read in 512 byte increments, looking for sparse + * blocks */ + amtleft = amtread; + do { + int interval = ((512 > amtleft) ? amtleft : 512); + int offset = amtread - amtleft; + + if (memcmp(buf+offset, zerobuf, interval) == 0) { + if (lseek(fd, interval, SEEK_CUR) < 0) { + virReportSystemError(conn, errno, + _("cannot extend file '%s'"), + vol->target.path); + goto cleanup; + } + } else if (safewrite(fd, buf+offset, interval) < 0) { + virReportSystemError(conn, errno, + _("failed writing to file '%s'"), + vol->target.path); + goto cleanup; + + } + } while ((amtleft -= 512) > 0); + } + + if (inputfd != -1 && close(inputfd) < 0) { + virReportSystemError(conn, errno, + _("cannot close file '%s'"), + inputvol->target.path); + goto cleanup; + } + inputfd = -1; + + *total -= remain; + ret = 0; + +cleanup: + if (inputfd != -1) + close(inputfd); + + return ret; +} + int virStorageBackendCreateRaw(virConnectPtr conn, virStorageVolDefPtr vol, @@ -108,7 +197,6 @@ virStorageBackendCreateRaw(virConnectPtr conn, unsigned int flags ATTRIBUTE_UNUSED) { int fd = -1; - int inputfd = -1; int ret = -1; unsigned long long remain; char *buf = NULL; @@ -121,15 +209,6 @@ virStorageBackendCreateRaw(virConnectPtr conn, goto cleanup; } - if (inputvol) { - if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("could not open input path '%s'"), - inputvol->target.path); - goto cleanup; - } - } - /* Seek to the final size, so the capacity is available upfront * for progress reporting */ if (ftruncate(fd, vol->capacity) < 0) { @@ -141,55 +220,10 @@ virStorageBackendCreateRaw(virConnectPtr conn, remain = vol->allocation; - if (inputfd != -1) { - int amtread = -1; - size_t bytes = 1024 * 1024; - char zerobuf[512]; - - bzero(&zerobuf, sizeof(zerobuf)); - - if (VIR_ALLOC_N(buf, bytes) < 0) { - virReportOOMError(conn); + if (inputvol) { + int res = virStorageBackendCopyToFD(conn, vol, inputvol, fd, &remain); + if (res < 0) goto cleanup; - } - - while (amtread != 0) { - int amtleft; - - if (remain < bytes) - bytes = remain; - - if ((amtread = saferead(inputfd, buf, bytes)) < 0) { - virReportSystemError(conn, errno, - _("failed reading from file '%s'"), - inputvol->target.path); - goto cleanup; - } - remain -= amtread; - - /* Loop over amt read in 512 byte increments, looking for sparse - * blocks */ - amtleft = amtread; - do { - int interval = ((512 > amtleft) ? amtleft : 512); - int offset = amtread - amtleft; - - if (memcmp(buf+offset, zerobuf, interval) == 0) { - if (lseek(fd, interval, SEEK_CUR) < 0) { - virReportSystemError(conn, errno, - _("cannot extend file '%s'"), - vol->target.path); - goto cleanup; - } - } else if (safewrite(fd, buf+offset, interval) < 0) { - virReportSystemError(conn, errno, - _("failed writing to file '%s'"), - vol->target.path); - goto cleanup; - - } - } while ((amtleft -= 512) > 0); - } } /* Pre-allocate any data if requested */ @@ -220,20 +254,10 @@ virStorageBackendCreateRaw(virConnectPtr conn, } fd = -1; - if (inputfd != -1 && close(inputfd) < 0) { - virReportSystemError(conn, errno, - _("cannot close file '%s'"), - inputvol->target.path); - goto cleanup; - } - inputfd = -1; - ret = 0; cleanup: if (fd != -1) close(fd); - if (inputfd != -1) - close(inputfd); VIR_FREE(buf); return ret; -- 1.6.0.6

Add a 'CreateBlockFrom' in the global storage_backend, which sets up the destination block device: CopyFromFD does the rest of the cloning. --- src/storage_backend.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 45 insertions(+), 1 deletions(-) diff --git a/src/storage_backend.c b/src/storage_backend.c index 868c297..37c227f 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -190,6 +190,47 @@ cleanup: return ret; } +static int +virStorageBackendCreateBlockFrom(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags ATTRIBUTE_UNUSED) +{ + int fd = -1; + int ret = -1; + unsigned long long remain; + + if ((fd = open(vol->target.path, O_RDWR)) < 0) { + virReportSystemError(conn, errno, + _("cannot create path '%s'"), + vol->target.path); + goto cleanup; + } + + remain = vol->allocation; + + if (inputvol) { + int res = virStorageBackendCopyToFD(conn, vol, inputvol, fd, &remain); + if (res < 0) + goto cleanup; + } + + if (close(fd) < 0) { + virReportSystemError(conn, errno, + _("cannot close file '%s'"), + vol->target.path); + goto cleanup; + } + fd = -1; + + ret = 0; +cleanup: + if (fd != -1) + close(fd); + + return ret; +} + int virStorageBackendCreateRaw(virConnectPtr conn, virStorageVolDefPtr vol, @@ -517,7 +558,10 @@ virStorageBackendGetBuildVolFromFunction(virConnectPtr conn, return virStorageBackendFSImageToolTypeToFunc(conn, tool_type); } - return virStorageBackendCreateRaw; + if (vol->type == VIR_STORAGE_VOL_BLOCK) + return virStorageBackendCreateBlockFrom; + else + return virStorageBackendCreateRaw; } #if defined(UDEVADM) || defined(UDEVSETTLE) -- 1.6.0.6

We don't gain any space savings, so skip the detection to speed up the cloning operation. --- src/storage_backend.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/storage_backend.c b/src/storage_backend.c index 37c227f..b5e4ac7 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -106,7 +106,8 @@ virStorageBackendCopyToFD(virConnectPtr conn, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, int fd, - unsigned long long *total) + unsigned long long *total, + int is_dest_file) { int inputfd = -1; int amtread = -1; @@ -155,7 +156,7 @@ virStorageBackendCopyToFD(virConnectPtr conn, int interval = ((512 > amtleft) ? amtleft : 512); int offset = amtread - amtleft; - if (memcmp(buf+offset, zerobuf, interval) == 0) { + if (is_dest_file && memcmp(buf+offset, zerobuf, interval) == 0) { if (lseek(fd, interval, SEEK_CUR) < 0) { virReportSystemError(conn, errno, _("cannot extend file '%s'"), @@ -210,7 +211,8 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn, remain = vol->allocation; if (inputvol) { - int res = virStorageBackendCopyToFD(conn, vol, inputvol, fd, &remain); + int res = virStorageBackendCopyToFD(conn, vol, inputvol, + fd, &remain, 0); if (res < 0) goto cleanup; } @@ -262,7 +264,8 @@ virStorageBackendCreateRaw(virConnectPtr conn, remain = vol->allocation; if (inputvol) { - int res = virStorageBackendCopyToFD(conn, vol, inputvol, fd, &remain); + int res = virStorageBackendCopyToFD(conn, vol, inputvol, + fd, &remain, 1); if (res < 0) goto cleanup; } -- 1.6.0.6

With the previous refactoring, this is a simple process, since the global 'CreateBlockFrom' in storage_backend does all the work. --- src/storage_backend_disk.c | 15 +++++++++++++++ src/storage_backend_logical.c | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 0 deletions(-) diff --git a/src/storage_backend_disk.c b/src/storage_backend_disk.c index eb694a7..1eb3eac 100644 --- a/src/storage_backend_disk.c +++ b/src/storage_backend_disk.c @@ -587,6 +587,20 @@ virStorageBackendDiskCreateVol(virConnectPtr conn, return 0; } +static int +virStorageBackendDiskBuildVolFrom(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags) +{ + virStorageBackendBuildVolFrom build_func; + + build_func = virStorageBackendGetBuildVolFromFunction(conn, vol, inputvol); + if (!build_func) + return -1; + + return build_func(conn, vol, inputvol, flags); +} static int virStorageBackendDiskDeleteVol(virConnectPtr conn, @@ -655,4 +669,5 @@ virStorageBackend virStorageBackendDisk = { .createVol = virStorageBackendDiskCreateVol, .deleteVol = virStorageBackendDiskDeleteVol, + .buildVolFrom = virStorageBackendDiskBuildVolFrom, }; diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c index 1bd00d4..6c123ae 100644 --- a/src/storage_backend_logical.c +++ b/src/storage_backend_logical.c @@ -654,6 +654,21 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, } static int +virStorageBackendLogicalBuildVolFrom(virConnectPtr conn, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags) +{ + virStorageBackendBuildVolFrom build_func; + + build_func = virStorageBackendGetBuildVolFromFunction(conn, vol, inputvol); + if (!build_func) + return -1; + + return build_func(conn, vol, inputvol, flags); +} + +static int virStorageBackendLogicalDeleteVol(virConnectPtr conn, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, @@ -679,6 +694,8 @@ virStorageBackend virStorageBackendLogical = { .refreshPool = virStorageBackendLogicalRefreshPool, .stopPool = virStorageBackendLogicalStopPool, .deletePool = virStorageBackendLogicalDeletePool, + .buildVol = NULL, + .buildVolFrom = virStorageBackendLogicalBuildVolFrom, .createVol = virStorageBackendLogicalCreateVol, .deleteVol = virStorageBackendLogicalDeleteVol, }; -- 1.6.0.6

We need to unlock the first pool before looking up the second, since the search locks every pool it checks. --- src/storage_driver.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/storage_driver.c b/src/storage_driver.c index c27534c..e9ecb20 100644 --- a/src/storage_driver.c +++ b/src/storage_driver.c @@ -1327,9 +1327,11 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, storageDriverLock(driver); pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - if (diffpool) + if (diffpool) { + virStoragePoolObjUnlock(pool); origpool = virStoragePoolObjFindByName(&driver->pools, vobj->pool); - else + virStoragePoolObjLock(pool); + } else origpool = pool; storageDriverUnlock(driver); -- 1.6.0.6