On 14/10/16 16:15, John Ferlan wrote:

I have written this lines as a part of  GPLv2+ boilerplate:
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...

Then, I guess, the first think that I should  do - src/util/vir*.c for
both drivers. I try to do it in the next version.

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.

Good point, thanks! I need some time to think it over.

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.

I agree.

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?

For directory fspool item object may seem meaningless, however
for zfs and virtuozzo storage, I think, it doesn't: pool can be mounted, can
have some restrictions and items is the object that is exposed to costumer.
I mean for zfs - we create pool, than create filesystems with different quotas, etc. For Virtuozzo storage - the entity that is exposed - is cluster. The administrator manipulations - is to mount cluster, and users are able to create items for their needs.


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.

I will split this series.


Best regards,

libvir-list mailing list

Reply via email to