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. On Sun, Sep 11, 2016 at 09:07:36AM +0300, Dmitry Petuhov wrote: > Instead, rely on plugin-defined supported formats to decide if it > supports subvols. > > Still workaround 'rbd' plugin, that cannot be used for LXC without > krbd. Maybe better way is to pass $scfg into plugin's plugindata() > in storage code? So plugin could show instance's capabilities > instead of whole plugin's capabilities, that may or may not be available > in instance. the second paragraph does not belong into the commit message ;) either include it as a comment below the --- , or in the cover letter, or as a separate mail altogether. > > Signed-off-by: Dmitry Petuhov <mityapetu...@gmail.com> > --- > src/PVE/LXC.pm | 45 +++++++++++++++------------------------------ > 1 file changed, 15 insertions(+), 30 deletions(-) > > diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm > index 35ce796..70d13e4 100644 > --- a/src/PVE/LXC.pm > +++ b/src/PVE/LXC.pm > @@ -1238,12 +1238,8 @@ sub mountpoint_mount { > if ($scfg->{path}) { > $mounted_dev = run_with_loopdev($domount, $path); > $use_loopdev = 1; > - } elsif ($scfg->{type} eq 'drbd' || $scfg->{type} eq 'lvm' || > - $scfg->{type} eq 'rbd' || $scfg->{type} eq 'lvmthin') { > - $mounted_dev = $path; > - &$domount($path); > } else { > - die "unsupported storage type '$scfg->{type}'\n"; > + &$domount($path); > } > return wantarray ? ($path, $use_loopdev, $mounted_dev) : $path; > } else { 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? > @@ -1330,37 +1326,26 @@ sub create_disks { > > my $size_kb = int(${size_gb}*1024) * 1024; > > - my $scfg = PVE::Storage::storage_config($storecfg, $storage); > # fixme: use better naming ct-$vmid-disk-X.raw? > > - if ($scfg->{type} eq 'dir' || $scfg->{type} eq 'nfs') { > - if ($size_kb > 0) { > - $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, > $vmid, 'raw', > - undef, $size_kb); > - format_disk($storecfg, $volid, $rootuid, $rootgid); > - } else { > + if ($size_kb > 0) { > + my $scfg = PVE::Storage::storage_config($storecfg, > $storage); > + die "krbd option must be enabled on storage type > 'rbd'\n" if ($scfg->{type} eq 'rbd') && !$scfg->{krbd}; > + > + $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, > $vmid, 'raw', > + undef, $size_kb); > + format_disk($storecfg, $volid, $rootuid, $rootgid); > + } else { > + my ($defFormat, $validFormats) = > PVE::Storage::storage_default_format($storecfg, $storage); this seems to be copied verbatim from QemuServer.pm, but the camelCase is still wrong (here and there). def_format and valid_formats please ;) > + if (grep { $_ eq 'subvol' } @$validFormats) { > $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, > $vmid, 'subvol', > undef, 0); > push @$chown_vollist, $volid; > - } > - } elsif ($scfg->{type} eq 'zfspool') { > + } else { > + die "Selected storage does not support subvols. > Please, specify image size or select another storage"; > + } > + } > > - $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, > $vmid, 'subvol', > - undef, $size_kb); > - push @$chown_vollist, $volid; > - } elsif ($scfg->{type} eq 'drbd' || $scfg->{type} eq 'lvm' || > $scfg->{type} eq 'lvmthin') { > - > - $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, > $vmid, 'raw', undef, $size_kb); > - format_disk($storecfg, $volid, $rootuid, $rootgid); > - > - } elsif ($scfg->{type} eq 'rbd') { > - > - die "krbd option must be enabled on storage type > '$scfg->{type}'\n" if !$scfg->{krbd}; > - $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, > $vmid, 'raw', undef, $size_kb); > - format_disk($storecfg, $volid, $rootuid, $rootgid); > - } else { > - die "unable to create containers on storage type > '$scfg->{type}'\n"; > - } > push @$vollist, $volid; > $mountpoint->{volume} = $volid; > $mountpoint->{size} = $size_kb * 1024; 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? _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel