Without having looked at the code, the 'saveconfig' command sure looks like it's not concurrency-safe. One thing that drove me absolutely crazy about itadm/stmfadm on Illumos was that it's not concurrency-safe. Commands would fail mysteriously once you did that, and had to resort to a reboot. It's very hard to readily build a program that shares the same CLI that administrators use, if it can't be used concurrently. Basically, the administrator can't know whether when he tries to run a command, if the program is doing the command at the same time. You would have to build some lock around targetcli to protect it. And you can't know that other tools don't use targetcli periodically. On Illumos, because of these limitations, we built a web server that would only allow one request to be processed at a time, serializing all commands; but it didn't stop the adminsitrator from using the command. Also, a web administration portal that we didn't build, napp-it, also used those same commands. So all of our engineering was ultimately flawed, because we didn't address the underlying issue of command concurrency. I apologize if saveconfig is concurrency-safe, but because it appears that it saves *everything*, I'm pretty safe in making this assumption. On Fri, Oct 11, 2013 at 7:50 PM, Tregaron Bayly <tbayly@xxxxxxxxxxxx> wrote: > On Fri, 2013-10-11 at 16:17 -0700, Andy Grover wrote: >> On 10/11/2013 04:10 PM, Tregaron Bayly wrote: >> > On Fri, 2013-10-11 at 15:41 -0700, Nicholas A. Bellinger wrote: >> >> >>> 3) reading/modifying/writing /etc/target/saveconfig.json then reloading >> >>> config >> >> >> >> Ugh.. >> > >> > Seconded, but we considered it. >> >> Oh no? I thought this would be more popular, assuming most languages had >> good tools for manipulating json. What was the sticking point? Just too >> kludgey? Did it not work? > > It's kind of the worst of all worlds. It's overkill to clear your > configuration and replace it with a savefile configuration if you only > want to add or remove a lun. You also have no guarantee that someone > hasn't messed with the savefile and you'll lose some of your running > config when you apply it. You could try to limit this by having > targetcli write a savefile first but that's time consuming and you can't > check a return code to make sure it worked before proceeding. In fact > that's the heart of the conundrum - the only guarantee that you have is > that you've modified the json, but the entire solution requires relying > on targetcli which we've already established can't be checked for > success or failure. After you modify the file you have to clear your > entire configuration and try to apply the savefile...with targetcli. > You can't know whether it worked or not and if there's something wrong > in the parsing of the file you don't get any configuration at all. In > essence you have as much chance of having no running config or a broken > config as a successful modification of whatever's running. That's a > horrible risk to take. > -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
