Re: [libvirt] [PATCH rfc v2 0/8] fspool: backend directory

[...] >> I have written this lines as a part of GPLv2+ boilerplate: >> https://www.redhat.com/archives/libvir-list/2016-August/msg01160.html, >> which I took from >> other libvirt parts. And I guess it was naturally to change name and >> company, don't you? >> And again, if you insist I can leave out the author/copyright as it >> wasn't the aim of this series. > Indeed, storage pool is very similar to FS pool but their items are not > - volumes (block devices) > versus filesystems (directory trees). And intention here was to > introduce a *new API*, which is > also very different from storage pool one, effectivly introducing a new > driver. As driver > boundaries crossing isn't favored, the code was simply borrowed, > following earlier practice used > by libvirt to get new drivers implemented. > > John, keeping all said above in mind, do you think it's worth trying to > reuse common code while > introducing a new API? It won't allow us to leave existing code > untouched and it will increase the > series even more. > Sorry - just haven't been able to keep up with my work and all the activity on this list lately. Here's my point - if you find yourself copying a function from one driver to another for the express purpose that you cannot cross driver boundaries, then perhaps your first thought should be - can the copied code can go in an existing or a new src/util/vir*.c file and be used in both places. I think there are synergies in the existing code...

Another thought that occurs to me - I cannot recall if it's been mentioned in this context or not (short term memory isn't what it used to be!). IIUC the model is to use these trees more for containers, then I would hope the security is "built in" very tightly rather than being an added on after thought. One particularly thorny area is NFS storage pools (and well directory trees) especially w/r/t root_squash or not. In any case, I can only imagine that for container consumers - being "held" to certain security rules will be very important.

One final thought, if the existing 'storage driver' is for BLOCK storage and this new driver is a 'File System Storage Driver', then rather than using 'fspool' (which is really confusing IMO) maybe the base driver is "fs_storage". So that means we create a "src/fs_storage" and start from there. The 'fs' was just not descriptive enough.

Since it's a File System Storage Driver that's essentially exposing entire directories, is there really a need for a "pool" concept? Isn't the driver providing that alone? IOW: The implementation is a pool. What else other than a directory would something like this expose? Is there something else that could be envisioned that would add to the (from patch 1) virFSItemType?

John Oh and please let's not drop 12K lines of new code into one series - please! Rome wasn't built in a day and certainly it's very hard to commit the time to review 12K lines of code in one series. A logical progression to the finish line is fine.

[...]