[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/2] Add fs pool formatting v3



On 06/02/2010 01:09 PM, David Allan wrote:
> * This patch adds the ability to make the filesystem for a filesystem
>   pool during a pool build.
> 
> ---
>  include/libvirt/libvirt.h.in     |    6 +-
>  include/libvirt/virterror.h      |    2 +
>  src/Makefile.am                  |    4 +
>  src/libvirt_private.syms         |    4 +
>  src/storage/storage_backend_fs.c |  180 +++++++++++++++++++++++++++++++++++++-
>  src/storage/storage_backend_fs.h |    9 ++-
>  src/util/virterror.c             |   12 +++
>  tools/virsh.c                    |   14 +++-
>  8 files changed, 224 insertions(+), 7 deletions(-)

Still no change to tools/virsh.pod?  We really need to get in the habit
of documenting our changes, so that someone doing 'man virsh' knows that
they exist.  And it wouldn't hurt if a separate patch changed
docs/api_extension/0008-* to better set that example.

> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 19d5205..075beee 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1126,8 +1126,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 */

We already assume C99 semantics of allowing a trailing comma; using it
here will make the next change to this enum touch one less line.

> +++ b/include/libvirt/virterror.h
> @@ -163,6 +163,8 @@ typedef enum {
>      VIR_WAR_NO_STORAGE, /* failed to start storage */
>      VIR_ERR_NO_STORAGE_POOL, /* storage pool not found */
>      VIR_ERR_NO_STORAGE_VOL, /* storage pool not found */
> +    VIR_ERR_STORAGE_PROBE_FAILED, /* storage pool probe failed */
> +    VIR_ERR_STORAGE_PROBE_BUILT, /* storage pool already built */

Typo in the name?  I would have though VIR_ERR_STORAGE_POOL_BUILT.

> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -624,6 +624,10 @@ virStorageFileGetMetadata;
>  virStorageFileGetMetadataFromFD;
>  virStorageFileIsSharedFS;
> 
> +# storage_backend_fs.h
> +virStorageBackendFileSystemProbeLibblkid;
> +virStorageBackendFileSystemProbeDummy;

You no longer need to export virStorageBackendFileSystemProbeDummy

> +#else /* #if HAVE_LIBBLKID */
> +
> +static virStoragePoolProbeResult
> +virStorageBackendFileSystemProbe(const char *device ATTRIBUTE_UNUSED,
> +                                     const char *format ATTRIBUTE_UNUSED)

Wonky spacing.

> +++ b/src/util/virterror.c
> @@ -1019,6 +1019,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;

Wonky spacing, but it is forgivable since it is copy-n-paste from the
previous line's wonky-ness (the whole file could use some TLC, so don't
sweat it for this patch).

Note to self: another merge conflict if this goes in before my pending
virterror cleanups.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]