Re: [pve-devel] [PATCH container 0/2] Linux LIO/targetcli support

2018-06-28 Thread Udo Rader
On Fri, 2018-06-15 at 13:45 +0200, Dominik Csapak wrote:
> > > 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?
> > 
> > One of the things I found too about SETTINGS is that the other
> > providers somehow seem to use it as a "cache", in order to reduce
> > the
> > amount of calls between the PVE and the target.
> 
> this cache is in general a good idea, because if we have an
> api call that needs many infos in short time it makes it faster
> e.g. an clone call of a vm with 10 disks:
> 
> * get infos about 10 disks
> * create 10 disks
> * copy 10 disks
> 
> in such a case, a cache can be useful
> 
> i do not know how costly the connections to a target really is, so i
> cannot really say if this caching makes sense and we have to fix it,
> or to drop it altogether
> 
> > 
> > This has the dangerous drawback, that if some external process
> > modifies
> > the target (ie adds or removes another LUN), PVE will probably
> > never
> > pick up that change, because it relies on the assumption that the
> > target is only controlled by the PVE. I have not fully investigated
> > this though.
> 
> that is exactly the problem, but also happens within pve
> (due to the worker mechanic)
> 
> > 
> > Originally I always repopulated the SETTINGS variable before
> > running
> > any "LUN command", but after looking at the other implementations,
> > I
> > decided to do it as they do it.
> 
> if the time to get the information is not too high, i would in
> general
> prefer your initial approach. but since the other implementations
> also do this, i originally asked @Michael (or mir; the initial author
> of this plugin) this
> 
> > 
> > So the "easy" fix for this would probably be to revert that and to
> > repopulate the SETTINGS hash for each call.
> 
> another option would be to have a 'clear cache' sub and
> if we executed a command *not* in a worker we call it after
> the 'lun_command' call in ZFSPlugin.pm
> 
> or a 'cache' parameter which indicates the use of cache,
> which we can the use in a worker
> 
> any other opinions?

I've just submitted a new version of the patch that expires the
$SETTINGS "cache" after 15 seconds, so if I'm right, that should solve
your issues ("[PATCH V2 storage 0/3] updated Linux targetcli/LIO
support")

As per how expensive it is to query the portal too often, I can't
really say.

What I see is that quite a few operations are done against the portal
for only one action in the user interface, so completely dropping the
cache would certainly be "expensive".

The old iet implementation has a maximum number of LUNs, which is set
to 16K.

Reprocessing 16K of LUNs over and over again, both on the portal an on
the PVE side of life is certainly no good idea, so I hope that 15
seconds are a good compromise.

Regards

Udo

-- 
Udo Rader, GF/CEO
BestSolution.at EDV Systemhaus GmbH
Eduard-Bodem-Gasse 5-7, A-6020 Innsbruck
http://www.bestsolution.at/
Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck

signature.asc
Description: This is a digitally signed message part
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH container 0/2] Linux LIO/targetcli support

2018-06-15 Thread Dominik Csapak

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


can you please elaborate what a "worker" is in PVE speech?


our daemons (pvedaemon, pveproxy) have a hard timeout for (synchronous) 
api calls (afair its 30 seconds), so for long running tasks

we do a 'fork_worker' which forks the process and generates a
task log entry

such a 'worker' copies the state of the forked process, including
the content of variables




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?


One of the things I found too about SETTINGS is that the other
providers somehow seem to use it as a "cache", in order to reduce the
amount of calls between the PVE and the target.


this cache is in general a good idea, because if we have an
api call that needs many infos in short time it makes it faster
e.g. an clone call of a vm with 10 disks:

* get infos about 10 disks
* create 10 disks
* copy 10 disks

in such a case, a cache can be useful

i do not know how costly the connections to a target really is, so i
cannot really say if this caching makes sense and we have to fix it,
or to drop it altogether



This has the dangerous drawback, that if some external process modifies
the target (ie adds or removes another LUN), PVE will probably never
pick up that change, because it relies on the assumption that the
target is only controlled by the PVE. I have not fully investigated
this though.


that is exactly the problem, but also happens within pve
(due to the worker mechanic)



Originally I always repopulated the SETTINGS variable before running
any "LUN command", but after looking at the other implementations, I
decided to do it as they do it.


if the time to get the information is not too high, i would in general
prefer your initial approach. but since the other implementations
also do this, i originally asked @Michael (or mir; the initial author
of this plugin) this



So the "easy" fix for this would probably be to revert that and to
repopulate the SETTINGS hash for each call.


another option would be to have a 'clear cache' sub and
if we executed a command *not* in a worker we call it after
the 'lun_command' call in ZFSPlugin.pm

or a 'cache' parameter which indicates the use of cache,
which we can the use in a worker

any other opinions?

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


Re: [pve-devel] [PATCH container 0/2] Linux LIO/targetcli support

2018-06-15 Thread Udo Rader
On Wed, 2018-06-13 at 09:31 +0200, Dominik Csapak wrote:
> 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)

yes, sorry, only noticed that once I had sent them to the list.

> 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 I see it right, the commit messages were "adding linux LIO support",
but next time I'll try to add more info there. The cover letter should
however give more detailled information about the patch.

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

ok

> 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

can you please elaborate what a "worker" is in PVE speech? 

> 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?

One of the things I found too about SETTINGS is that the other
providers somehow seem to use it as a "cache", in order to reduce the
amount of calls between the PVE and the target.

This has the dangerous drawback, that if some external process modifies
the target (ie adds or removes another LUN), PVE will probably never
pick up that change, because it relies on the assumption that the
target is only controlled by the PVE. I have not fully investigated
this though.

Originally I always repopulated the SETTINGS variable before running
any "LUN command", but after looking at the other implementations, I
decided to do it as they do it.

So the "easy" fix for this would probably be to revert that and to
repopulate the SETTINGS hash for each call.

Regards

Udo

-- 
Udo Rader, GF/CEO
BestSolution.at EDV Systemhaus GmbH
Eduard-Bodem-Gasse 5-7, A-6020 Innsbruck
http://www.bestsolution.at/
Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck

signature.asc
Description: This is a digitally signed message part
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH container 0/2] Linux LIO/targetcli support

2018-06-13 Thread Dominik Csapak

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


[pve-devel] [PATCH container 0/2] Linux LIO/targetcli support

2018-06-08 Thread Udo Rader
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

-- 
2.17.1

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