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