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
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
