Re: [pve-devel] Make LXC code independent of storage plugin names
> Also, why I can't set volume size to zero on volume creation via WEB UI > and so use subvols? Is it bug? Using simply directories is a hack, because there is no disk quota in that case. That is why I disabled it on the GUI. Or is it a feature? ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Make LXC code independent of storage plugin names
On Mon, Sep 19, 2016 at 10:37:01AM +0300, Dmitry Petuhov wrote:
> 19.09.2016 08:51, Fabian Grünbichler wrote:
> >
> > 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 ?
>
yes, that would probably be a good idea :)
> > 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.
our LXC prestart hook uses it to setup the devices for quota support,
see src/lxc-pve-prestart-hook, lines 79-108:
my $devlist_file = "/var/lib/lxc/$vmid/devices";
unlink $devlist_file;
my $devices = [];
my $setup_mountpoint = sub {
my ($ms, $mountpoint) = @_;
#return if $ms eq 'rootfs';
my (undef, undef, $dev) = PVE::LXC::mountpoint_mount($mountpoint,
$rootdir, $storage_cfg);
push @$devices, $dev if $dev && $mountpoint->{quota};
};
PVE::LXC::Config->foreach_mountpoint($conf, $setup_mountpoint);
my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir);
$lxc_setup->pre_start_hook();
if (@$devices) {
my $devlist = '';
foreach my $dev (@$devices) {
my ($mode, $rdev) = (stat($dev))[2,6];
next if !$mode || !S_ISBLK($mode) || !$rdev;
my $major = int($rdev / 0x100);
my $minor = $rdev % 0x100;
$devlist .= "b:$major:$minor:$dev\n";
}
PVE::Tools::file_set_contents($devlist_file, $devlist);
}
return undef;
}});
> > 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?
It's actually the opposite, we are limited for KVM/Qemu, where we can
only use zvols. ZFS datasets are vastly superior to zvols for container
usage, so that is the default (and only option) for them. In theory we
could allow zvols as well, but there is no reason to do so (they only
have disadvantages, and checking all the places were subvols are
implicitly assumed right now is tedious work for basically no gain).
All of this is orthogonal to the issue at hand though, which is that
your patch would break a currently existing feature: support for ZFS
subvols for containers WITH and without size (limit). This is a feature
which we definitely don't want to drop ;)
> Also, why I can't set volume size to zero on volume creation via WEB UI and
> so use subvols? Is it bug?
It's intentional. Some advanced features are not available in the GUI,
either to keep the GUI simple, or because they are only available to the
root user for security reasons. This one belongs in the first category
(IIRC, it's only available to allow setups similar to legacy OpenVZ
ones, but I may be wrong about that).
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Make LXC code independent of storage plugin names
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?
___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Make LXC code independent of storage plugin names
On Sat, Sep 17, 2016 at 10:25:08PM +0300, Dmitry Petuhov wrote: > Please, consider applying this patchset. Last week began to use it with my > Netapp > https://github.com/mityarzn/pve-storage-custom-mpnetapp and Dell PS-series > https://github.com/mityarzn/pve-storage-custom-dellps SAN plugins, as well as > with 'local' standard storage (both raw and subvol formats). All seems to > work. > 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! ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Make LXC code independent of storage plugin names
Please, consider applying this patchset. Last week began to use it with my Netapp https://github.com/mityarzn/pve-storage-custom-mpnetapp and Dell PS-series https://github.com/mityarzn/pve-storage-custom-dellps SAN plugins, as well as with 'local' standard storage (both raw and subvol formats). All seems to work. 11.09.2016 9:07, Dmitry Petuhov wrote: > This patch series simplifies LXC volume creation and makes it independent > of storage plugin names. It will allow to use LXC with custom plugins. > > We assume here that all storages (except rbd) can create raw volumes, > that we can format and mount, which is'nt acually true. But this case > is being checked (isn't it?) by 'rootdir' property of plugin conterns. ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] Make LXC code independent of storage plugin names
This patch series simplifies LXC volume creation and makes it independent of storage plugin names. It will allow to use LXC with custom plugins. We assume here that all storages (except rbd) can create raw volumes, that we can format and mount, which is'nt acually true. But this case is being checked (isn't it?) by 'rootdir' property of plugin conterns. ___ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
