OndrejHome / ansible.targetcli-modules

Modules for managing targetcli via Ansible
GNU General Public License v3.0
10 stars 12 forks source link

Added 'attributes' support for targetcli_iscsi and targetcli_iscsi_lun modules #1

Closed danielerez closed 7 years ago

danielerez commented 7 years ago

@OndrejHome

OndrejHome commented 7 years ago

Thanks @danielerez for submission! I will have a better look at this over weekend. So far I think it would be good to include the examples and documentation about 'attributes' to 'DOCUMENTATION' and 'EXAMPLES' variables in module codes so 'ansible-doc' can show that properly.

danielerez commented 7 years ago

@OndrejHome Sure :) Updated examples and docs.

OndrejHome commented 7 years ago

@danielerez could you please push those changes to support_attributes branch so it is in one pull request?

danielerez commented 7 years ago

It's pushed to the same branch. I've overridden the previous commits.

OndrejHome commented 7 years ago

HI @danielerez . I have checked the changes and noticed that it makes more sense to me to have attributes for luns in 'targetcli_backstore' module. As the 'targetcli_iscsi_lun' module only create luns in iSCSI targets where you cannot set any attributes. So the real attributes are applied only to backstore objects in '/backstore/../backstore_object'. I have created commit a3fc152 that reflects this idea. Otherwise this looks good to me. If you have any additions to recent change please let me know. If no then I will merge changes on this Thursday.

danielerez commented 7 years ago

I think it's indeed a good idea to have attributes in 'targetcli_backstore' module. However, from my tests, emulate_tpu attribute must be set on the lun itself to have the required effect. Why not keep the support also in 'targetcli_iscsi_lun' module just in case..?

OndrejHome commented 7 years ago

@danielerez how did you managed to configure attribute for LUN? In example below I have added /dev/sda3 as block device backstore object and then added it as lun0 to iscsi target. While I can get/set attribute from/on backstore object I cannot get/set any attribute when I'm trying to reach it via lun0 in iscsi target. Do I understand correctly that you want to set the attribute for lun0 in example below? Could you please give me example which commands you use to set attribute for LUN?

# targetcli /backstores/block/ ls
o- block ............................................................. [Storage Objects: 1]
  o- test_block ................................. [/dev/sda3 (3.9GiB) write-thru activated]

# targetcli /iscsi/iqn.1994-05.com.redhat:fastvm/tpg1/luns ls
o- luns ......................................................................... [LUNs: 1]
  o- lun0 .................................................. [block/test_block (/dev/sda3)]

# targetcli /backstores/block/test_block get attribute emulate_tpu
emulate_tpu=1

# targetcli /iscsi/iqn.1994-05.com.redhat:fastvm/tpg1/luns/lun0 get attribute emulate_tpu
Unknown configuration group: attribute

# yum list targetcli
targetcli.noarch          2.1.fb41-3.el7    @base
danielerez commented 7 years ago

Oh sorry, my mistake. It should be fine in 'targetcli_backstore' module indeed. You can merge it :)

OndrejHome commented 7 years ago

Thank you @danielerez for confirmation and contributions. Merged to master, closing this PR.