On Wed, Aug 31, 2011 at 10:34:47PM +0800, Osier Yang wrote:
> This patch adds the ability to make the filesystem for a filesystem
> pool during a pool build.
>
> The patch adds two new flags, no overwrite and overwrite, to control
> when mkfs gets executed. By default, the patch preserves the
> current behavior, i.e., if no flags are specified, pool build on a
> filesystem pool only makes the directory on which the filesystem
> will be mounted.
>
> If the no overwrite flag is specified, the target device is checked
> to determine if a filesystem of the type specified in the pool is
> present. If a filesystem of that type is already present, mkfs is
> not executed and the build call returns an error. Otherwise, mkfs
> is executed and any data present on the device is overwritten.
>
> If the overwrite flag is specified, mkfs is always executed, and any
> existing data on the target device is overwritten unconditionally.
> ---
> include/libvirt/libvirt.h.in | 6 +-
> include/libvirt/virterror.h | 2 +
> src/Makefile.am | 4 +
> src/libvirt.c | 4 +
> src/storage/storage_backend_fs.c | 188 +++++++++++++++++++++++++++++++++++++-
> src/storage/storage_backend_fs.h | 5 +
> src/util/virterror.c | 12 +++
> 7 files changed, 215 insertions(+), 6 deletions(-)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index c51a5b9..92e1d62 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1666,8 +1666,10 @@ typedef enum {
>
> typedef enum {
> VIR_STORAGE_POOL_BUILD_NEW = 0, /* Regular build from scratch */
> - VIR_STORAGE_POOL_BUILD_REPAIR = 1, /* Repair / reinitialize */
> - VIR_STORAGE_POOL_BUILD_RESIZE = 2 /* Extend existing pool */
> + VIR_STORAGE_POOL_BUILD_REPAIR = (1 << 0), /* Repair / reinitialize */
> + VIR_STORAGE_POOL_BUILD_RESIZE = (1 << 1), /* Extend existing pool */
> + VIR_STORAGE_POOL_BUILD_NO_OVERWRITE = (1 << 2), /* Do not overwrite existing pool */
> + VIR_STORAGE_POOL_BUILD_OVERWRITE = (1 << 3), /* Overwrite data */
> } virStoragePoolBuildFlags;
>
> typedef enum {
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index c270b54..0aae622 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -235,6 +235,8 @@ typedef enum {
> VIR_ERR_INVALID_STREAM = 73, /* stream pointer not valid */
> VIR_ERR_ARGUMENT_UNSUPPORTED = 74, /* valid API use but unsupported by
> the given driver */
> + VIR_ERR_STORAGE_PROBE_FAILED = 75, /* storage pool proble failed */
> + VIR_ERR_STORAGE_POOL_BUILT = 76, /* storage pool already built */
> } virErrorNumber;
>
> /**
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 322c900..14f09e4 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -954,6 +954,10 @@ endif
> if WITH_SECDRIVER_APPARMOR
> libvirt_driver_storage_la_LIBADD += $(APPARMOR_LIBS)
> endif
> +if HAVE_LIBBLKID
> +libvirt_driver_storage_la_CFLAGS += $(BLKID_CFLAGS)
> +libvirt_driver_storage_la_LIBADD += $(BLKID_LIBS)
> +endif
> if WITH_STORAGE_DIR
> if WITH_DRIVER_MODULES
> mod_LTLIBRARIES += libvirt_driver_storage.la
> diff --git a/src/libvirt.c b/src/libvirt.c
> index e4a21b6..b876b1c 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -10466,6 +10466,10 @@ error:
> * virStoragePoolBuild:
> * @pool: pointer to storage pool
> * @flags: future flags, use 0 for now
> + * @flags: flags to control pool build behaviour
> + *
> + * Currently only filesystem pool accepts flags VIR_STORAGE_POOL_BUILD_OVERWRITE
> + * and VIR_STORAGE_POOL_BUILD_NO_OVERWRITE.
> *
> * Build the underlying storage pool
> *
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index 4f53d3f..36f1c60 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -37,6 +37,10 @@
> #include <libxml/tree.h>
> #include <libxml/xpath.h>
>
> +#if HAVE_LIBBLKID
> +# include <blkid/blkid.h>
> +#endif
> +
> #include "virterror_internal.h"
> #include "storage_backend_fs.h"
> #include "storage_conf.h"
> @@ -45,6 +49,7 @@
> #include "memory.h"
> #include "xml.h"
> #include "virfile.h"
> +#include "logging.h"
>
> #define VIR_FROM_THIS VIR_FROM_STORAGE
>
> @@ -534,13 +539,172 @@ virStorageBackendFileSystemStart(virConnectPtr conn ATTRIBUTE_UNUSED,
> }
> #endif /* WITH_STORAGE_FS */
>
> +#if HAVE_LIBBLKID
> +static virStoragePoolProbeResult
> +virStorageBackendFileSystemProbe(const char *device,
> + const char *format) {
> +
> + virStoragePoolProbeResult ret = FILESYSTEM_PROBE_ERROR;
> + blkid_probe probe = NULL;
> + const char *fstype = NULL;
> + char *names[2], *libblkid_format = NULL;
> +
> + VIR_DEBUG("Probing for existing filesystem of type %s on device %s",
> + format, device);
> +
> + if (blkid_known_fstype(format) == 0) {
> + virStorageReportError(VIR_ERR_STORAGE_PROBE_FAILED,
> + _("Not capable of probing for "
> + "filesystem of type %s"),
> + format);
> + goto error;
> + }
> +
> + probe = blkid_new_probe_from_filename(device);
> + if (probe == NULL) {
> + virStorageReportError(VIR_ERR_STORAGE_PROBE_FAILED,
> + _("Failed to create filesystem probe "
> + "for device %s"),
> + device);
> + goto error;
> + }
> +
> + if ((libblkid_format = strdup(format)) == NULL) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + names[0] = libblkid_format;
> + names[1] = NULL;
> +
> + blkid_probe_filter_superblocks_type(probe,
> + BLKID_FLTR_ONLYIN,
> + names);
> +
> + if (blkid_do_probe(probe) != 0) {
> + VIR_INFO("No filesystem of type '%s' found on device '%s'",
> + format, device);
> + ret = FILESYSTEM_PROBE_NOT_FOUND;
> + } else if (blkid_probe_lookup_value(probe, "TYPE", &fstype, NULL) == 0) {
> + virStorageReportError(VIR_ERR_STORAGE_POOL_BUILT,
> + _("Existing filesystem of type '%s' found on "
> + "device '%s'"),
> + fstype, device);
> + ret = FILESYSTEM_PROBE_FOUND;
> + }
> +
> + if (blkid_do_probe(probe) != 1) {
> + virStorageReportError(VIR_ERR_STORAGE_PROBE_FAILED,
> + _("Found additional probes to run, "
> + "filesystem probing may be incorrect"));
> + ret = FILESYSTEM_PROBE_ERROR;
> + }
> +
> +error:
> + VIR_FREE(libblkid_format);
> +
> + if (probe != NULL) {
> + blkid_free_probe(probe);
> + }
> +
> + return ret;
> +}
> +
> +#else /* #if HAVE_LIBBLKID */
> +
> +static virStoragePoolProbeResult
> +virStorageBackendFileSystemProbe(const char *device ATTRIBUTE_UNUSED,
> + const char *format ATTRIBUTE_UNUSED)
> +{
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> + _("probing for filesystems is unsupported "
> + "by this build"));
> +
> + return FILESYSTEM_PROBE_ERROR;
> +}
> +
> +#endif /* #if HAVE_LIBBLKID */
> +
> +static int
> +virStorageBackendExecuteMKFS(const char *device,
> + const char *format)
> +{
> + int ret = 0;
> + virCommandPtr cmd = NULL;
> +
> + cmd = virCommandNewArgList(MKFS,
> + "-t",
> + format,
> + device,
> + NULL);
> +
> + if (virCommandRun(cmd, NULL) < 0) {
> + virReportSystemError(errno,
> + _("Failed to make filesystem of "
> + "type '%s' on device '%s'"),
> + format, device);
> + ret = -1;
> + }
> + return ret;
> +}
> +
> +static int
> +virStorageBackendMakeFileSystem(virStoragePoolObjPtr pool,
> + unsigned int flags)
> +{
> + const char *device = NULL, *format = NULL;
> + bool ok_to_mkfs = false;
> + int ret = -1;
> +
> + if (pool->def->source.devices == NULL) {
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> + _("No source device specified when formatting pool '%s'"),
> + pool->def->name);
> + goto error;
> + }
> +
> + device = pool->def->source.devices[0].path;
> + format = virStoragePoolFormatFileSystemTypeToString(pool->def->source.format);
> + VIR_DEBUG("source device: '%s' format: '%s'", device, format);
> +
> + if (!virFileExists(device)) {
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> + _("Source device does not exist when formatting pool '%s'"),
> + pool->def->name);
> + goto error;
> + }
> +
> + if (flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) {
> + ok_to_mkfs = true;
> + } else if (flags & VIR_STORAGE_POOL_BUILD_NO_OVERWRITE &&
> + virStorageBackendFileSystemProbe(device, format) ==
> + FILESYSTEM_PROBE_NOT_FOUND) {
> + ok_to_mkfs = true;
> + }
> +
> + if (ok_to_mkfs) {
> + ret = virStorageBackendExecuteMKFS(device, format);
> + }
> +
> +error:
> + return ret;
> +}
> +
>
> /**
> * @conn connection to report errors against
> * @pool storage pool to build
> + * @flags controls the pool formating behaviour
> *
> * Build a directory or FS based storage pool.
> *
> + * If no flag is set, it only makes the directory; If
> + * VIR_STORAGE_POOL_BUILD_NO_OVERWRITE set, it probes to determine if
> + * filesystem already exists on the target device, renurning an error
> + * if exists, or using mkfs to format the target device if not; If
> + * VIR_STORAGE_POOL_BUILD_OVERWRITE is set, mkfs is always executed,
> + * any existed data on the target device is overwritten unconditionally.
> + *
> * - If it is a FS based pool, mounts the unlying source device on the pool
> *
> * Returns 0 on success, -1 on error
> @@ -551,10 +715,20 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED,
> unsigned int flags)
> {
> int err, ret = -1;
> - char *parent;
> - char *p;
> + char *parent = NULL;
> + char *p = NULL;
>
> - virCheckFlags(0, -1);
> + virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE |
> + VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret);
> +
> + if (flags == (VIR_STORAGE_POOL_BUILD_OVERWRITE |
> + VIR_STORAGE_POOL_BUILD_NO_OVERWRITE)) {
> +
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> + _("Overwrite and no overwrite flags"
> + " are mutually exclusive"));
> + goto error;
> + }
>
> if ((parent = strdup(pool->def->target.path)) == NULL) {
> virReportOOMError();
> @@ -604,7 +778,13 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED,
> goto error;
> }
> }
> - ret = 0;
> +
> + if (flags != 0) {
> + ret = virStorageBackendMakeFileSystem(pool, flags);
> + } else {
> + ret = 0;
> + }
> +
> error:
> VIR_FREE(parent);
> return ret;
> diff --git a/src/storage/storage_backend_fs.h b/src/storage/storage_backend_fs.h
> index 7def53e..54c6739 100644
> --- a/src/storage/storage_backend_fs.h
> +++ b/src/storage/storage_backend_fs.h
> @@ -29,6 +29,11 @@
> # if WITH_STORAGE_FS
> extern virStorageBackend virStorageBackendFileSystem;
> extern virStorageBackend virStorageBackendNetFileSystem;
> +typedef enum {
> + FILESYSTEM_PROBE_FOUND,
> + FILESYSTEM_PROBE_NOT_FOUND,
> + FILESYSTEM_PROBE_ERROR,
> +} virStoragePoolProbeResult;
> # endif
> extern virStorageBackend virStorageBackendDirectory;
>
> diff --git a/src/util/virterror.c b/src/util/virterror.c
> index b50fbfd..26c4981 100644
> --- a/src/util/virterror.c
> +++ b/src/util/virterror.c
> @@ -1030,6 +1030,18 @@ virErrorMsg(virErrorNumber error, const char *info)
> else
> errmsg = _("Storage volume not found: %s");
> break;
> + case VIR_ERR_STORAGE_PROBE_FAILED:
> + if (info == NULL)
> + errmsg = _("Storage pool probe failed");
> + else
> + errmsg = _("Storage pool probe failed: %s");
> + break;
> + case VIR_ERR_STORAGE_POOL_BUILT:
> + if (info == NULL)
> + errmsg = _("Storage pool already built");
> + else
> + errmsg = _("Storage pool already built: %s");
> + break;
> case VIR_ERR_INVALID_STORAGE_POOL:
> if (info == NULL)
> errmsg = _("invalid storage pool pointer in");
ACK the logic looks right. I would love to see some ways to test
the various behaviours though,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list