[DO NOT MERGE, discussions only] refresh on delete backstore #170
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem: ------- [root@server1 ~]# targetcli [11/1366] targetcli shell version 2.1.51 Copyright 2011-2013 by Datera, Inc and others. For help on commands, type 'help'. /> backstores/fileio create name=disk file_or_dev=/tmp/iscsidisk size=1G /tmp/iscsidisk exists, using its size (1073741824 bytes) instead Created fileio disk with size 1073741824 /> iscsi/ create Created target iqn.2003-01.org.linux-iscsi.server1.x8664:sn.5679adc37869. Created TPG 1. /> iscsi/iqn.2003-01.org.linux-iscsi.server1.x8664:sn.5679adc37869/tpg1/luns create /backstores/fileio/disk Created LUN 0. /> ls o- / .............................................................. [...] o- backstores ................................................... [...] | o- block ...................................................... [Storage Objects: 0] | o- fileio ..................................................... [Storage Objects: 1] | | o- disk ..................................................... [/tmp/iscsidisk (1.0GiB) write-back activated] | | o- alua ................................................... [ALUA Groups: 1] | | o- default_tg_pt_gp ..................................... [ALUA state: Active/optimized] | o- pscsi ...................................................... [Storage Objects: 0] | o- ramdisk .................................................... [Storage Objects: 0] | o- user:glfs .................................................. [Storage Objects: 0] o- iscsi ........................................................ [Targets: 1] | o- iqn.2003-01.org.linux-iscsi.server1.x8664:sn.5679adc37869 .. [TPGs: 1] | o- tpg1 ..................................................... [disabled] | o- acls ................................................... [ACLs: 0] | o- luns ................................................... [LUNs: 1] | | o- lun0 ................................................. [fileio/disk (/tmp/iscsidisk) (default_tg_pt_gp)] | o- portals ................................................ [Portals: 0] o- loopback ..................................................... [Targets: 0] o- vhost ........................................................ [Targets: 0] o- xen-pvscsi ................................................... [Targets: 0] /> backstores/fileio/ delete disk Deleted storage object disk. /> ls This LUN does not exist in configFS /> Fix: --- Refresh at the end of ui_command_delete() Credits to: * Jing Yan [jiyan@redhat.com] who initially reported this issue. * Matt Coleman [@iammattcoleman] for bringing attention on this issue. * Maurizio Lombardi [@maurizio-lombardi] for reproducer. Fixes: BZ#1571728 Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
| rn._save_backups(default_save_file) | ||
|
|
||
| child.rtsnode.delete(save=save) | ||
| self.remove_child(child) | ||
| rn.ui_command_refresh() |
There was a problem hiding this comment.
Refreshing the entire root node works, but it can be a very heavy operation. I'll have a pull request that only refreshes the affected nodes in a couple hours.
There was a problem hiding this comment.
So Quick!!
I get you, you have a point.
Sure, if you are already working on it, I will wait for the patch then.
Thanks!
There was a problem hiding this comment.
@iammattcoleman I was also wondering if this is a valid use case of targetcli ?
Generally, the right order is to delete the target first and then delete the backstore.
We anyway have refresh command for targetcli:
[root@server1 ~]# targetcli [11/1366]
targetcli shell version 2.1.51
Copyright 2011-2013 by Datera, Inc and others.
For help on commands, type 'help'.
/> backstores/fileio create name=disk file_or_dev=/tmp/iscsidisk size=1G
/tmp/iscsidisk exists, using its size (1073741824 bytes) instead
Created fileio disk with size 1073741824
/> iscsi/ create
Created target iqn.2003-01.org.linux-iscsi.server1.x8664:sn.5679adc37869.
Created TPG 1.
/> iscsi/iqn.2003-01.org.linux-iscsi.server1.x8664:sn.5679adc37869/tpg1/luns create /backstores/fileio/disk
Created LUN 0.
/> ls
o- / ...................................................................... [...]
o- backstores ........................................................... [...]
| o- block .............................................................. [Storage Objects: 0]
| o- fileio ............................................................. [Storage Objects: 1]
| | o- disk ............................................................. [/tmp/iscsidisk (1.0GiB) write-back activated]
| | o- alua ........................................................... [ALUA Groups: 1]
| | o- default_tg_pt_gp ............................................. [ALUA state: Active/optimized]
| o- pscsi .............................................................. [Storage Objects: 0]
| o- ramdisk ............................................................ [Storage Objects: 0]
| o- user:glfs .......................................................... [Storage Objects: 0]
o- iscsi ................................................................ [Targets: 1]
| o- iqn.2003-01.org.linux-iscsi.server1.x8664:sn.5679adc37869 .......... [TPGs: 1]
| o- tpg1 ............................................................. [disabled]
| o- acls ........................................................... [ACLs: 0]
| o- luns ........................................................... [LUNs: 1]
| | o- lun0 ......................................................... [fileio/disk (/tmp/iscsidisk) (default_tg_pt_gp)]
| o- portals ........................................................ [Portals: 0]
o- loopback ............................................................. [Targets: 0]
o- vhost ................................................................ [Targets: 0]
o- xen-pvscsi ........................................................... [Targets: 0]
/> backstores/fileio/ delete disk
Deleted storage object disk.
/> pwd
/
/> get global auto_use_daemon
auto_use_daemon=false
/> version
targetcli version 2.1.51
/> ls
This LUN does not exist in configFS
/> refresh
/> ls
o- / ..................................................................... [...]
o- backstores .......................................................... [...]
| o- block ............................................................. [Storage Objects: 0]
| o- fileio ............................................................ [Storage Objects: 0]
| o- pscsi ............................................................. [Storage Objects: 0]
| o- ramdisk ........................................................... [Storage Objects: 0]
| o- user:glfs ......................................................... [Storage Objects: 0]
o- iscsi ............................................................... [Targets: 1]
| o- iqn.2003-01.org.linux-iscsi.server1.x8664:sn.5679adc37869 ......... [TPGs: 1]
| o- tpg1 ............................................................ [disabled]
| o- acls .......................................................... [ACLs: 0]
| o- luns .......................................................... [LUNs: 0]
| o- portals ....................................................... [Portals: 0]
o- loopback ............................................................ [Targets: 0]
o- vhost ............................................................... [Targets: 0]
o- xen-pvscsi .......................................................... [Targets: 0]
/>
@iammattcoleman @maurizio-lombardi thoughts?
There was a problem hiding this comment.
@iammattcoleman as you rightly pointed, refresh incurs delay, and having this on every delete might not be an acceptable solution.
There was a problem hiding this comment.
If you manually perform these operations directly in ConfigFS, the kernel removes the LUNs from targets that referenced the backstore. Since it's doing that automatically, I think targetcli should automatically refresh the data for the affected nodes. That way, its behavior is consistent with the kernel's.
|
@iammattcoleman excuse me if it feels like jumping the gun, our team is waiting on the new release having the taretclid fixes since a week now, but @maurizio-lombardi seems to be waiting for this one fix. |
pkalever
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
Fix:
Refresh at the end of ui_command_delete()
Credits to:
Fixes: BZ#1571728
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>