[libvirt] [PATCH 18/18] util: storage: Invert the way recursive metadata retrieval works

[Eric Blake]

On 04/20/2014 04:13 PM, Peter Krempa wrote:
> To avoid having the root of a backing chain present twice in the list we
> need to invert the working of virStorageFileGetMetadataRecurse.
> 
> Until now the recursive worker created a new backing chain element from
> the name and other information passed as arguments. This required us to
> pass the data of the parend in a deconstructed way and the worker

s/parend/parent/

> created a new entry for the parent.
> 
> This patch converts this function so that it just fills in metadata
> about the parent and creates a backing chain element from those. This
> removes the duplication of the first element.
> 
> To avoid breaking the test suite, virstoragetest now calls a wrapper
> that creates the parent structure explicitly and pre-fills it with the
> test data with same function signature as previously used.
> ---
>  src/conf/domain_conf.c        |   5 +-
>  src/qemu/qemu_domain.c        |  12 ++-
>  src/qemu/qemu_driver.c        |   6 +-
>  src/security/virt-aa-helper.c |   7 +-
>  src/util/virstoragefile.c     | 193 ++++++++++++++++++++++--------------------
>  src/util/virstoragefile.h     |   7 +-
>  tests/virstoragetest.c        |  47 +++++++++-
>  7 files changed, 158 insertions(+), 119 deletions(-)
> 


> -        }
> +    /* check wether we need to go deeper */

s/wether/whether/


>          }
> +    } else {
> +        /* TODO: To satisfy the test case, copy the network URI as path. T
> +         * his will be removed later */

s/T.*his/This/


> +
> +    if (virStorageFileGetMetadataRecurse(backingStore,
> +                                         backingStore->path,
> +                                         uid, gid, allow_probe,
> +                                         cycle) < 0) {
> +        /* if we fail somewhere midway, just accept the and return a
> +         * broken chain */

s/the and/and/ ?


> @@ -1220,51 +1227,51 @@ virStorageFileGetMetadataRecurse(const char *path, const char *canonPath,
>   *
>   * Caller MUST free result after use via virStorageSourceFree.
>   */
> -virStorageSourcePtr
> -virStorageFileGetMetadata(const char *path, int format,
> +int
> +virStorageFileGetMetadata(virStorageSourcePtr src,
>                            uid_t uid, gid_t gid,
>                            bool allow_probe)

Yep, quite an inversion; but looks reasonable.


> +
> +static virStorageSourcePtr
> +testStorageFileGetMetadata(const char *path,
> +                           int format,
> +                           uid_t uid, gid_t gid,
> +                           bool allow_probe)

Nice wrapper.


> 
> -    meta = virStorageFileGetMetadata(data->start, data->format, -1, -1,
> +    meta = testStorageFileGetMetadata(data->start, data->format, -1, -1,
>                                       (data->flags & ALLOW_PROBE) != 0);

Indentation of second line is now off; here and in many other hunks this
file.

It's late for me, and this one's big enough that I'd like to revisit it
in my morning to make sure I'm not overlooking anything else.  It will
probably be ack with nits fixed, but can't hurt to be careful...


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140422/ede0367a/attachment-0001.sig>