On Thu, Apr 14, 2016 at 10:45:03AM +0300, Marius Vollmer wrote: > Vivek Goyal <vgo...@redhat.com> writes: > > > On Wed, Apr 13, 2016 at 10:36:40AM +0300, Marius Vollmer wrote: > > > >> [..] So any user-prompting from docker-storage-setup is a bug? I get > >> a prompt from pvcreate sometimes, for example when the random data on > >> the new partition has a filesystem signature at the beginning. With > >> what I know now, this is probably unintended. Should I file an > >> issue? Lvm likes to add confirmation prompts like this over > >> time... :) > > > > I think if it runs as service, then no terminal is attached and it > > assumes a default answer. I guess yes. Give it a try. > > Here what happens with stdin from /dev/null: > > # docker-storage-setup </dev/null > Checking that no-one is using this disk right now ... OK > > Disk /dev/sda: 2 GiB, 2147483648 bytes, 4194304 sectors > Units: sectors of 1 * 512 = 512 bytes > Sector size (logical/physical): 512 bytes / 512 bytes > I/O size (minimum/optimal): 512 bytes / 512 bytes > > >>> Script header accepted. > >>> Created a new DOS disklabel with disk identifier 0x79a24362. > Created a new partition 1 of type 'Linux LVM' and of size 2 GiB. > /dev/sda2: > New situation: > > Device Boot Start End Sectors Size Id Type > /dev/sda1 2048 4194303 4192256 2G 8e Linux LVM > > The partition table has been altered. > Calling ioctl() to re-read partition table. > Syncing disks. > WARNING: xfs signature detected on /dev/sda1 at offset 0. Wipe it? [y/n]: > n > Aborted wiping of xfs. > 1 existing signature left on the device. > Aborting pvcreate on /dev/sda1. > > In your opinion, is it a bug or a feature that pvcreate might prompt > like this?
I think this is default behavior of pvcreate. By default it will not create lvm signature on disk if it finds other signatures on disk. This is not a bug and it is trying to be safe. > > >> Why does d-s-s run during boot, actually? Shouldn't people run it > >> interactively after changing /e/s/d-s-s? Or does it only run on > >> first boot? > > > > [ - Get things right automatically during first boot. > > - Wait for thin pool to come up > > - Automatically grow pvs backing volume group > > ] > > Thanks for the explanation! > > >> If you think that a --force-wipe option is not appropriate for d-s-s, > >> Cockpit can wipe the devices itself. But I think the option is nicer. > > > > I don't know, wiping partitions is always risky by default. > > It's an option, it would not be the default behavior. Sure. Even if you want to introduce option, it might be better to add a config option in /usr/lib/docker-storage-setup/docker-storage-setup Docker storage setup reads all the knobs from there and tweaks its behavior. > > > User might lose important data. If cockpit can do it, that would be > > great. If there is a strong demand for this, we can add it, but nobody > > has asked for it yet. > > I have, and I think there was some demand earlier in the thread. I > can contribute the code and am happy to look after bugs with it. If you think this is important to add, open a PR and add an option say FORCE_WIPE_DISKS=false > > > [d-s-s --status] > > > > So all this current config detection can be easily done outside d-s-s > > as well. I think it probably would be better if you run various lvm > > commands to figure out volumes, thin-pools, pvs and display whatever > > information is useful? > > I am not sure about "easy". :-) I was hoping that d-s-s can provide a > higher-level abstraction that would make it unnecessary to agree on a > (ever growing) set of low-level conventions. > > I'd say we need to have some code somewhere for inspecting the current > state of the docker storage pool. Having it in d-s-s itself would be > the natural place, IMO, and would make it accessible for others and give > it a fighting chance to keep working. Having it in Cockpit would create > a pretty high risk that things get out of sync, and nobody else would > benefit from it. dss is more of a service. So tyring to use it as command line with various options might not be best fit. But if you have concrete user stories, I am open to changes as long as it does not make the existing code messy. > > (The same is true for --force-wipe.) > > >> I think a reliable "other" flag will be interesting for a potential > >> "docker-storage-setup reset" operation. This would remove the > >> current pool and create a new one, so that the user can meaningfully > >> remove devices from DEVS again. > > > > If user removes the thin pool, a new one will be created. "lvremove > > docker-vg/docker-pool". > > (You need to remove /etc/sysconfig/docker-storage as well before d-s-s > will make a new pool. Also stopping docker and rm /var/lib/docker are > probably good ideas.) Lot of people have complained about being able to reset the state of storage. So it is a good idea to provide something there. But I think we need something outside of docker-storage-setup, say atomic. Reason being that resetting the state of storage also requires stopping docker and cleaning up /var/lib/docker and docker-storage-setup should not be the one touching /var/lib/docker/. Once Dan had suggested an atomic command say "reset-docker" or "reinit-docker" or something else. That might be a good place to cleanup docker state as well as docker-storage-setup state. To make thins simple I don't mind providing a command in dss which can do some cleanup for storage state. So say a dss command "reset" which will remove thin pool and remove /etc/sysconfig/docker-storage should be reasonable. But one will have to make sure docker has been stopped and /var/lib/docker/ has been removed in advance other bad things will happen. And that's where a command in atomic to deal with it will help. > > > I really don't see a need for a new command or option. > > But maybe others do. I hope that counts for something. :-) Sure, if you have convincing story, I am open to it. > > > Don't want to make a simple bash script too complicated until and > > unless it is really really needed. > > If we would follow your arguments until the end, there would be no > reason to have d-s-s at all. :-) Users can 'easily' create the necessary > thin pools with a few lvm command line calls and write the right > incantation to /e/s/docker-storage. Nope, we needed a service which could do it automatically on every boot and it was hard to do with lvm commands. And it was a concrete need. I am against adding too much of functionality and code without lot of thinking otherwise it becomes maintenance burden soon. > > But we give them a tool since doing all this is in fact non-trivial and > a common use case. Let's improve the tool to be more robust and cover > more use cases. That is the fate of all useful software, it will grow. > I would be happy to help. If we have good concrete use cases and stories, I am open to improve things incrementally. > > "d-s-s reset" would allow people to switch storage drivers, switch from > loopback to thinpool, and remove disks from the pool again in a simple > and restricted, but robust and supported way. Lets implement "atomic reset-docker" and that in turn can call dss reset. Thanks Vivek
