Re: [libvirt] [PATCH 6/7] Break out FS volume build routines to their own functions.

Improves readability, particularly wrt the pending CreateFromXML changes. This will also help implementing File->Block volume creation in the future, since some of this code will need to be moved to a backend agnostic file.

Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; --- src/storage_backend_fs.c | 327 +++++++++++++++++++++++++--------------------- 1 files changed, 179 insertions(+), 148 deletions(-) diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index 7a1bba8..241fb29 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -62,6 +62,8 @@ static int qcowXGetBackingStore(virConnectPtr, char **, static int vmdk4GetBackingStore(virConnectPtr, char **, const unsigned char *, size_t); +typedef int (*createFile)(virConnectPtr conn, virStorageVolDefPtr vol); + static int track_allocation_progress = 0; /* Either 'magic' or 'extension' *must* be provided */ @@ -1011,183 +1013,202 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn, return 0; } -/** - * Allocate a new file as a volume. This is either done directly - * for raw/sparse files, or by calling qemu-img/qcow-create for - * special kinds of files - */ -static int -virStorageBackendFileSystemVolBuild(virConnectPtr conn, - virStorageVolDefPtr vol) -{ +// XXX: Have these functions all use the same format? +static int createRaw(virConnectPtr conn, + virStorageVolDefPtr vol) { int fd; - if (vol->target.format == VIR_STORAGE_VOL_FILE_RAW) { - 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); - return -1; - } + 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); + return -1; + } - /* 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); - close(fd); - return -1; - } + /* 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); + close(fd); + return -1; + } - /* Pre-allocate any data if requested */ - /* XXX slooooooooooooooooow on non-extents-based file systems */ - /* FIXME: Add in progress bars & bg thread if progress bar requested */ - if (vol->allocation) { - if (track_allocation_progress) { - unsigned long long remain = vol->allocation; - - 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); - close(fd); - return -1; - } - remain -= bytes; - } - } else { /* No progress bars to be shown */ + /* Pre-allocate any data if requested */ + /* XXX slooooooooooooooooow on non-extents-based file systems */ + if (vol->allocation) { + if (track_allocation_progress) { + unsigned long long remain = vol->allocation; + + 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 ((r = safezero(fd, 0, 0, vol->allocation)) != 0) { + 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); close(fd); return -1; } + remain -= bytes; + } + } else { /* No progress bars to be shown */ + int r; + + if ((r = safezero(fd, 0, 0, vol->allocation)) != 0) { + virReportSystemError(conn, r, + _("cannot fill file '%s'"), + vol->target.path); + close(fd); + return -1; } } + } - } else if (vol->target.format == VIR_STORAGE_VOL_FILE_DIR) { - if (mkdir(vol->target.path, vol->target.perms.mode) < 0) { - virReportSystemError(conn, errno, - _("cannot create path '%s'"), - vol->target.path); - return -1; - } + if (close(fd) < 0) { + virReportSystemError(conn, errno, + _("cannot close file '%s'"), + vol->target.path); + return -1; + } - if ((fd = open(vol->target.path, O_RDWR)) < 0) { - virReportSystemError(conn, errno, - _("cannot read path '%s'"), - vol->target.path); - return -1; - } - } else { -#if HAVE_QEMU_IMG - const char *type = virStorageVolFormatFileSystemTypeToString(vol->target.format); - const char *backingType = vol->backingStore.path ? - virStorageVolFormatFileSystemTypeToString(vol->backingStore.format) : NULL; - char size[100]; - const char **imgargv; - const char *imgargvnormal[] = { - QEMU_IMG, "create", "-f", type, vol->target.path, size, NULL, - }; - /* XXX including "backingType" here too, once QEMU accepts - * the patches to specify it. It'll probably be -F backingType */ - const char *imgargvbacking[] = { - QEMU_IMG, "create", "-f", type, "-b", vol->backingStore.path, vol->target.path, size, NULL, - }; - - if (type == NULL) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("unknown storage vol type %d"), - vol->target.format); - return -1; - } - if (vol->backingStore.path == NULL) { - imgargv = imgargvnormal; - } else { - 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; - } + return 0; +} - imgargv = imgargvbacking; - } +static int createFileDir(virConnectPtr conn, + virStorageVolDefPtr vol) { + if (mkdir(vol->target.path, vol->target.perms.mode) < 0) { + virReportSystemError(conn, errno, + _("cannot create path '%s'"), + vol->target.path); + return -1; + } - /* Size in KB */ - snprintf(size, sizeof(size), "%llu", vol->capacity/1024); + return 0; +} - if (virRun(conn, imgargv, NULL) < 0) { - return -1; - } +#if HAVE_QEMU_IMG +static int createQemuImg(virConnectPtr conn, + virStorageVolDefPtr vol) { + const char *type = virStorageVolFormatFileSystemTypeToString(vol->target.format); + const char *backingType = vol->backingStore.path ? + virStorageVolFormatFileSystemTypeToString(vol->backingStore.format) : NULL; + char size[100]; + const char **imgargv; + const char *imgargvnormal[] = { + QEMU_IMG, "create", "-f", type, vol->target.path, size, NULL, + }; + /* XXX including "backingType" here too, once QEMU accepts + * the patches to specify it. It'll probably be -F backingType */ + const char *imgargvbacking[] = { + QEMU_IMG, "create", "-f", type, "-b", vol->backingStore.path, vol->target.path, size, NULL, + }; - if ((fd = open(vol->target.path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("cannot read path '%s'"), - vol->target.path); - return -1; - } -#elif HAVE_QCOW_CREATE - /* - * Xen removed the fully-functional qemu-img, and replaced it - * with a partially functional qcow-create. Go figure ??!? - */ - char size[100]; - const char *imgargv[4]; - - if (vol->target.format != VIR_STORAGE_VOL_FILE_QCOW2) { + if (type == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unknown storage vol type %d"), + vol->target.format); + return -1; + } + if (vol->backingStore.path == NULL) { + imgargv = imgargvnormal; + } else { + if (backingType == NULL) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("unsupported storage vol type %d"), - vol->target.format); + _("unknown storage vol backing store type %d"), + vol->backingStore.format); return -1; } - if (vol->backingStore.path != NULL) { - virStorageReportError(conn, VIR_ERR_NO_SUPPORT, - _("copy-on-write image not supported with " - "qcow-create")); + if (access(vol->backingStore.path, R_OK) != 0) { + virReportSystemError(conn, errno, + _("inaccessible backing store volume %s"), + vol->backingStore.path); return -1; } - /* Size in MB - yes different units to qemu-img :-( */ - snprintf(size, sizeof(size), "%llu", vol->capacity/1024/1024); + imgargv = imgargvbacking; + } - imgargv[0] = QCOW_CREATE; - imgargv[1] = size; - imgargv[2] = vol->target.path; - imgargv[3] = NULL; + /* Size in KB */ + snprintf(size, sizeof(size), "%llu", vol->capacity/1024); - if (virRun(conn, imgargv, NULL) < 0) { - return -1; - } + if (virRun(conn, imgargv, NULL) < 0) { + return -1; + } - if ((fd = open(vol->target.path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("cannot read path '%s'"), - vol->target.path); - return -1; - } + return 0; +} + +#elif HAVE_QCOW_CREATE +/* + * 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) { + char size[100]; + const char *imgargv[4]; + + 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] = QCOW_CREATE; + imgargv[1] = size; + imgargv[2] = vol->target.path; + imgargv[3] = NULL; + + if (virRun(conn, imgargv, NULL) < 0) { + return -1; + } + + return 0; +} +#endif /* HAVE_QEMU_IMG, elif HAVE_QCOW_CREATE */ + +/** + * Allocate a new file as a volume. This is either done directly + * for raw/sparse files, or by calling qemu-img/qcow-create for + * special kinds of files + */ +static int +virStorageBackendFileSystemVolBuild(virConnectPtr conn, + virStorageVolDefPtr vol) +{ + int fd; + createFile create_func; + + if (vol->target.format == VIR_STORAGE_VOL_FILE_RAW) { + create_func = createRaw; + } else if (vol->target.format == VIR_STORAGE_VOL_FILE_DIR) { + create_func = createFileDir; + } else { +#if HAVE_QEMU_IMG + create_func = createQemuImg; +#elif HAVE_QCOW_CREATE + create_func = createQemuCreate; #else virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("creation of non-raw images " @@ -1196,6 +1217,16 @@ virStorageBackendFileSystemVolBuild(virConnectPtr conn, #endif } + if (create_func(conn, vol) < 0) + return -1; + + if ((fd = open(vol->target.path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("cannot read path '%s'"), + vol->target.path); + return -1; + } + /* We can only chown/grp if root */ if (getuid() == 0) { if (fchown(fd, vol->target.perms.uid, vol->target.perms.gid) < 0) { -- 1.6.0.6 -- Libvir-list mailing list Libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list