14 Oct
2016
14 Oct
'16
2:59 p.m.
On 14/10/16 16:15, John Ferlan wrote: > [...] > >>> 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... 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. > > 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. I will split this series. > > > [...] -- Best regards, Olga