On Mon, May 04, 2009 at 01:43:01PM -0400, Cole Robinson wrote:
> 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.
This is a nice idea - it was getting rather unwieldly as it grew.
ACK.
Daniel
>
> Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
|: 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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list