Hi,

I finally got around to give your patches a spin, and can report that
they work for my preliminary tests.

One thing, which I think is not a good idea is the reuse of the
comstar_tg property for the tpg of LIO (from my understanding they do
serve somewhat different purposes). Should we introduce it like that,
we will be stuck with supporting this syntax for backwards
compatibility. We should probably introduce a different optional
property "lio_tpg" and initially just present it along with the others
in the GUI dialog. What would your opinion be on that?

Most possible places for some refactoring we saw apply to the iet 
implementation you based your patches on as well, but if you feel like
it, it would be great to use the new implementation for an initial
cleanup.

One thing which would probably be nice is to remove the created zvol,
if the subsequent creation of the lun fails (I stumbled upon that by
having a typo in the iqn of the target).

I'll post some comments to the individual patches.

Thanks again for your work!


On Fri, 29 Jun 2018 10:00:51 +0200
Udo Rader <udo.ra...@bestsolution.at> wrote:

> fixed a typo that prevented LIO.pm from loading
> 
> Udo Rader (4):
>   adding linux LIO support
>   adding linux LIO support
>   expiring cached iSCSI configuration after 15 seconds, after which it
>     is re-read from the portal
>   fixed syntax error, preventing LIO.pm from loading
> 
>  PVE/Storage/LunCmd/LIO.pm   | 407
> ++++++++++++++++++++++++++++++++++++ PVE/Storage/LunCmd/Makefile |
> 2 +- PVE/Storage/ZFSPlugin.pm    |   7 +-
>  3 files changed, 414 insertions(+), 2 deletions(-)
>  create mode 100644 PVE/Storage/LunCmd/LIO.pm
> 


_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to