[pve-devel] [PATCH v6 pve-storage 0/1] FreeNAS storage plugin

Fabian Grünbichler f.gruenbichler at proxmox.com
Mon Jun 19 08:24:36 CEST 2017


On Sat, Jun 17, 2017 at 11:52:41PM +0200, mir at datanom.net wrote:
> From: Michael Rasmussen <mir at datanom.net>
> 
> This is a patch to be added to v5 of the patch series.
> 

sorry - but like I said this is not how patches are supposed to be sent.

please split up your patch series into sensible patches based on what
they do, not when you have written them. then add/update/remove patches
between versions as makes sense. reviewing 1000 line diffs for every
iteration when the whole code is just a little over that size is crazy.

the bigger the changes, the more important it is to keep the whole thing
managable ;)

> fixes
> # if ! to unless
> # all helper methods now private
> # remove unused includes
> # remove unused code
> # more descriptive variable names
> # change api timeout to be in sync with the PVE API
> # add loop over limit until empty resultset is returned
> # fix API version check
> # fix handling 409 code in create_target and create_target_group
> # Remove unnecessary error handling
> # Replace hardcoded max luns number with variable
> # Fix error handling in freenas_create_lun method
> # Write directly to file instead of using a shell to echo to file
> # Fix error handling in deactivate_lun and get_active_luns
> # Improve code readability and error handling in rescan_session
> # Declaring variables where used in create_base
> # Check if base already exists when creating base
> # Improve error handling in create_base
> # Fix clone_image error handling and remove unnecessary check for running base
> # Remove unused variable in alloc_image
> # More robust error handling in free_image
> # Remove HTML code from volume_resize
> # Fix error handling in shapshot rollback
> # Improve code readability and error handling in activate_lun
> # Improve code readability and error handling in deactivate_lun
> # Replace sleep 1 with a combination of udevadm trigger udevadm settle
> # Remove check of running VM and CT. relaying on parsed option running for
>   Qemu and handle running LXC in PVE::API2::LXC now sends running status
>   as part of call to volume_resize when storage id is freenas.
> 
> TODO
> # Make target prefix configurable in gui
> # Configurable in gui whether to validate SSL certificate or not
> 
> 
> Michael Rasmussen (1):
>   A lot of bug fixes and clean-ups
> 
>  PVE/Storage/FreeNASPlugin.pm | 1011 ++++++++++++++++++++++--------------------
>  1 file changed, 533 insertions(+), 478 deletions(-)




More information about the pve-devel mailing list