search

open-iscsi / targetcli-fb

Command shell for managing Linux LIO kernel target
Apache License 2.0
103 stars 69 forks source link

Improve user experience if configuration is empty #85

Open agrover opened 7 years ago

agrover commented 7 years ago

See https://bugzilla.redhat.com/show_bug.cgi?id=1442018. We should not auto save configurations if they are empty. Once this change is made, this also means that if the user wants to remove the configuration, they'll need to explicitly call saveconfig within targetcli, or remove saveconfig.json manually.

rickysarraf commented 7 years ago

"set global auto_save_on_exit=false"

I never knew there was an auto save feature, and enabled. Shouldn't this feature (auto save) be disabled by default ?

agrover commented 7 years ago

Well, we print this when exiting targetcli:

Global pref auto_save_on_exit=true
Last 10 configs saved in /etc/target/backup.
Configuration saved to /etc/target/saveconfig.json

in the hopes that people understand it is doing this, and also how to turn it off.

I also would be reluctant because targetcli makes modifications to the running config as they are input, rather than being a configuration editor that allows changes to be applied (or thown away) at the end. The fear was that this way of working wouldn't be clear to users, they'd configure everything, exit targetcli, and be mystified and annoyed when everything vanished on reboot. (We already make users explicitly enable target.service, which I also don't like, but at least their configuration is saved, it just needs to be loaded.)

In addition to fixing the empty-config case, what if we added an exit --nosave option, or similar separate command? This would make it possible to keep autosave on but avoid it in cases where it is not what you want.

rickysarraf commented 7 years ago

I never knew (or noticed) the auto_save_on_exit=true when configuring targetcli. But then, I hadn't had to configure it that often, so possibly I may have missed that notification.

My comment was in the context of server software. Most systems and admins prefer behavior of the tools to be defined manually. For example, enabling apache modules still require a manual reload/restart.

10 is a sane enough backup number for cases where humans are manually triggering targetcli. I am not sure how others use LIO, like Openstack and other automation tools. In the latter cases, if automation invokes it too often, auto_save could eventually lead to unexpected config.

bluecmd commented 5 years ago

Fwiw, on Debian targetcli does not restore configuration automatically on reboot, and while debugging why my configuration was not coming up I accidentally saved an empty configuration, exactly like the issue discussed here.

mikebounds commented 4 years ago

I have had an issue twice where some LVM volumes are open after a reboot and then targetcli can't open these volumes and so then config partially loads, so then when I exit it saves the partial config. I need to find out why I get issue of open volumes, but in terms of saving, I agree with agrover that there should be a "exit --nosave" option and also if auto_save_on_exit=true, then if changes are made and not saved, then warn config is not saved when using exit and say "use exit --nosave to discard changes" . Also you could have a 3rd option to auto_save_on_exit of "prompt".