Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DO NOT MERGE, discussions only] refresh on delete backstore #170

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[DO NOT MERGE, discussions only] refresh on delete backstore #170

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion targetcli/ui_backstore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,12 +302,13 @@ def ui_command_delete(self, name, save=None):
raise ExecutionError("No storage object named %s." % name)

save = self.ui_eval_param(save, 'bool', False)
rn = self.get_root()
if save:
rn = self.get_root()
rn._save_backups(default_save_file)

child.rtsnode.delete(save=save)
self.remove_child(child)
rn.ui_command_refresh()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iammattcoleman as you rightly pointed, refresh incurs delay, and having this on every delete might not be an acceptable solution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

self.shell.log.info("Deleted storage object %s." % name)

def ui_complete_delete(self, parameters, text, current_param):
Expand Down