hi, thanks for the patches

i looked briefly over them (and will dive deeper into the code in the following days) and played a little around


here an initial review of what i saw/found

nitpicks:

you used the wrong git repository in the subject (container vs storage), not really important, but you might want to correct this if you plan to send patches in the future (makes it less confusing)

both patches have the same commit subject and no commit message
it would be very good if the patches would describe more detailed what they do

if they really need to have the same commit subject, it would probably be better to combine them into one commit

during my initial tests, it worked (mostly) but i found some strange behaviours:

when we execute a zfs request not in a worker (e.g. a content listing)
and then create a lun in a worker, only that worker, and no future worker knows of it (because when we fork, we copy all data currently in variables)

this seems due to the $SETTINGS variable which get filled whenever we
get info from the target

it seems this is a general problem also for the other zfs over iscsi providers, has anyone who uses them (@mir?) the same problems?
how do you prevent/handle this?

a possible solution i guess would be to prevent filling the
$SETTINGS variable when not in a worker, any other idea?

On 06/08/2018 12:27 PM, Udo Rader wrote:
introducing LIO/targetcli support allowing to use recent linux
distributions as iSCSI targets for ZFS volumes.

In order for this to work, two preconditions have to be met:

#1 the portal has to be set up correctly using targetcli
#2 the initiator has to be authorized to connect to the target
    based on the initiator's InitiatorName

When adding a LIO iSCSI target, the "Target group" field has to be
populated with the fitting "Target Portal Group" name (typically something
like 'tpg1').

Udo Rader (2):
   adding linux LIO support
   adding linux LIO support

  PVE/Storage/LunCmd/LIO.pm   | 398 ++++++++++++++++++++++++++++++++++++
  PVE/Storage/LunCmd/Makefile |   2 +-
  PVE/Storage/ZFSPlugin.pm    |   7 +-
  3 files changed, 405 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