Re: [libvirt] [PATCH v2] storage: vz storage pool support

On 12/16/2016 06:14 AM, Olga Krishtal wrote: > On 08/12/16 22:56, John Ferlan wrote: >>>> Compare that to NFS, which uses mount which is included in well every >>>> distro I can think of. That's a big difference. Also let's face it, NFS >>>> has been the essential de facto goto tool to access remote storage >>>> for a >>>> long time. Personally, I'd rather see the NFS code split out of the >>>> *_fs.c backend, but I don't have the desire/time to do it - so it stays >>>> as is. >>> To sum this up, you still think that copy and paste isn't a problem here >>> and will create more value than do any harm, right? >>> >> Not sure what you're inferring by copy and paste - other than perhaps >> having to decide for the vstorage backend which storage backend API's it >> needs or should support. >> >> The list of API's as I see from the path are: >> >> + >> + .startPool = virStorageBackendVzStart, >> + .checkPool = virStorageBackendFileSystemCheck, >> + .stopPool = virStorageBackendFileSystemStop, >> + .findPoolSources = virStorageBackendVzfindPoolSources, >> + .buildPool = virStorageBackendFileSystemBuild, >> + .deletePool = virStorageBackendFileSystemDelete, >> + .refreshPool = virStorageBackendFileSystemRefresh, >> + .buildVol = virStorageBackendFileSystemVolBuild, >> + .buildVolFrom = virStorageBackendFileSystemVolBuildFrom, >> + .createVol = virStorageBackendFileSystemVolCreate, >> + .refreshVol = virStorageBackendFileSystemVolRefresh, >> + .deleteVol = virStorageBackendFileSystemVolDelete, >> + .resizeVol = virStorageBackendFileSystemVolResize, >> + .uploadVol = virStorageBackendVolUploadLocal, >> + .downloadVol = virStorageBackendVolDownloadLocal, >> + .wipeVol = virStorageBackendVolWipeLocal, >> + >> >> Other than startPool and findPoolSources, the code will reuse/call the >> virStorageBackendFileSystem* API's - so the only copy/paste would be >> copyrights, some #include's that would be required only for vstorage, >> the VIR_FROM_THIS definition, VIR_LOG_INIT... Nothing any other backend >> wouldn't have to do. > By reusing you mean to export virStorageBackendFileSystem* and call > them from > staorage_backen_vstorage? > Otherwise, may be I could do vstorage stuff only under #ifdef without > touching any of > virStorageBackendFileSystem* ? I can see why the desire is to use storage_backend_fs because it already has a bunch of things you'll need, so I understand why it's desired to be included there. From the other POV though - the more #ifdef code in storage_backend_fs the 'harder' it is to separate things (conceptually).

Finally, I also have floating around is the fspool adjustments and wonder what kind of overlap exists.

Still seeing progress is good too and I guess unless I or someone else volunteers to split up storage_backend_fs.c into multiple backends (fs, nfs, dir, etc.) I guess we're stuck with what we have. John >> Although I do question "checkPool" - I would think for vstorage that >> should check if the environment is "available" somehow *if* you want >> pool autostart > Thanks, you are right. >> Also for stopPool the code will essentially call unmount. So is that >> "expected" for vstorage? > yes, just umount. >> Going through each API callout is how you'll be able to tell me/us that >> taking that path will work for the vstorage environment. >> >> John >