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.
[...]