[pve-devel] Make LXC code independent of storage plugin names

Dmitry Petuhov mityapetuhov at gmail.com
Mon Sep 19 09:37:01 CEST 2016


19.09.2016 08:51, Fabian Grünbichler wrote:
> I sent you some feedback on patch #1 yesterday.. Since it breaks 
> stuff, it can't be merged (yet). Feel free to send an updated v2 (or 
> if you feel the feedback requires discussion, feel free to respond). 
> Thanks! 
Sorry, did not received it. Maybe Google's spam filters munched it.

> ------------------------------------------------------------------------
> general remark, rest of comments inline:
>
> the indentation is all messed up. I know our codebase is not very clean
> in this regard anyway, but for new / touched code we try to keep / make
> it clean. our indentation for perl code is a mix of tabs and spaces,
> with tabs being 8 spaces long.
>
> the first indentation level is indented with 4 spaces, the second with
> 1 tab, the third with 1 tab and 4 spaces, the fourth with 2 tabs, and
> so on. if you use vim, I recommend enabling listmode to easily
> differentiate between the two.
Ok. Configured my mcedit. Maybe it would be good idea to all coding 
style memo to https://pve.proxmox.com/wiki/Developer_Documentation ?

> you drop the $mounted_dev assignment in the else branch. after a quick
> search of the calling code, this will probably at least break quota
> support for such volumes?
Actually it's not. mountpoint_mount() is being called in list context 
only once (in PVE::API2::LXC) and only $use_loopdev is being used there. 
In other places it is being called in scalar context, or its returned 
values are ignored at all. So currently it brakes nothing. But OK, I can 
keep this assingment for future.

> subvols are not only for size == 0 , e.g. ZFS supports subvols with and
> without size (limit). ZFS is a bit tricky unfortunately, as for
> containers we always want subvols (regular ZFS filesystems), and for
> VMs we always want raw volumes ("zvols" in ZFS speak). so changing the
> default format does not really work, and we still need a special case
> for ZFS (and future storages with the same problem) here?
But why we are limiting zfspool to subvols only? Following this logic, 
we should forbid raw images for LXC on any other filesystem-based 
storage and use only subvols there, like it was with OpenVZ. Or we can 
spread generic behaviour to zfspool, like I did.
Maybe as compromise we could add configurable option to plugins to let 
user decide?

Also, why I can't set volume size to zero on volume creation via WEB UI 
and so use subvols? Is it bug?




More information about the pve-devel mailing list