[libvirt] [PATCHv2 11/33] storage: Move virStorageFileGetMetadata to the storage driver

[Eric Blake]

On 05/22/2014 07:47 AM, Peter Krempa wrote:
> My future work will modify the metadata crawler function to use the
> storage driver file APIs to access the files instead of accessing them
> directly so that we will be able to request the metadata for remote
> files too. To avoid linking the storage driver to every helper file
> using the utils code, the backing chain traversal function needs to be
> moved to the storage driver source.
> 
> Additionally the virt-aa-helper and virstoragetest programs need to be
> linked with the storage driver as a result of this change.
> ---
>  cfg.mk                        |   2 +-
>  src/Makefile.am               |   2 +
>  src/libvirt_private.syms      |   2 +-
>  src/qemu/qemu_domain.c        |   2 +
>  src/security/virt-aa-helper.c |   2 +
>  src/storage/storage_driver.c  | 233 ++++++++++++++++++++++++++++++++++++++++++
>  src/storage/storage_driver.h  |   5 +
>  src/util/virstoragefile.c     | 233 +-----------------------------------------
>  src/util/virstoragefile.h     |   7 +-
>  tests/Makefile.am             |   7 +-
>  tests/virstoragetest.c        |   2 +
>  11 files changed, 258 insertions(+), 239 deletions(-)
> 
> diff --git a/cfg.mk b/cfg.mk
> index 5ff2721..9e8fcec 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -774,7 +774,7 @@ sc_prohibit_cross_inclusion:
>  	    access/ | conf/) safe="($$dir|conf|util)";;			\
>  	    locking/) safe="($$dir|util|conf|rpc)";;			\
>  	    cpu/| network/| node_device/| rpc/| security/| storage/)	\
> -	      safe="($$dir|util|conf)";;				\
> +	      safe="($$dir|util|conf|storage)";;			\

Fair enough - this lets security/ use storage/, but as they are on the
same level, it is not a layering violation.


> 
>  /* Internal version that also supports a containing directory name.  */
> -static int
> +int
>  virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta,
>                                          int fd,
>                                          int *backingFormat)

It's a bit confusing that we now have virStorageFile* functions spread
across two different files; maybe a later patch should rename the
storage_driver.h functions to have a different prefix?

>  	virstoragetest.c testutils.h testutils.c
> -virstoragetest_LDADD = $(LDADDS)
> +virstoragetest_LDADD = $(LDADDS) \
> +					   ../src/libvirt.la \
> +					   ../src/libvirt_conf.la \
> +					   ../src/libvirt_util.la \
> +					   ../src/libvirt_driver_storage_impl.la \
> +					   ../gnulib/lib/libgnu.la

That's a lot of whitespace (5 tab + 3 space); 1 or 2 tabs would have
been sufficient.  We aren't very consistent on whether to end a list
with $(NULL), so that you can insert a new line anywhere later (even
last) without having to touch multiple lines.

But overall, this looks like straightforward code motion.  ACK with
whitespace nit fixed.

-- 
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/20140523/09a68680/attachment-0001.sig>