slam command don't slam #77

Closed
davinerd opened this Issue Jan 13, 2012 · 21 comments

Projects

None yet

3 participants

@davinerd
Member

slam_tomb() should be revised: it don't slam tomb.
I'm unsure about fuser syntax; also, we don't implement an -f option, but the code above mentions it.

Anyone got this issue? Please check it out.

@boyska
Member
boyska commented Jan 13, 2012

Works for me. But you're right, the "-f" option is not recognized by slam subcommand, we should add it.
I tried with vim being the bad process, and it exited on USR1.
then I tried with dd, which does NOT exit on USR1 (instead, it displays stats), and it exited on HUP (after 3 seconds)

@davinerd
Member

On 13/01/2012 21:55, BoySka wrote:

Works for me. But you're right, the "-f" option is not recognized by slam subcommand, we should add it.
I tried with vim being the bad process, and it exited on USR1.
then I tried with dd, which does NOT exit on USR1 (instead, it displays stats), and it exited on HUP (after 3 seconds)
Yeah, I'had to be more specific.
The tomb won't slam if it isn't busy.
I mean: open a tomb, and immediatly slam the tomb, without holding it
with processes.

The close command works, slam don't.

Anathema

+--------------------------------------------------------------------+
|GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ |
|Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C |
| |
|http://www.msack.org |
|https://tboxes.tracciabi.li/anathema
+--------------------------------------------------------------------+

@boyska
Member
boyska commented Jan 14, 2012

again, works for me.

Password: [_] Commanded to open tomb testing.tomb
 .  mountpoint not specified, using default: /media/testing.tomb
[_] mounting testing.tomb on mountpoint /media/testing.tomb
 .  check for a valid LUKS encrypted device
[_] Password is required for key testing
 .  encrypted storage filesystem check
fsck da util-linux 2.20.1
testing: clean, 13/4608 files, 1768/18432 blocks
[_] encrypted storage testing.tomb succesfully mounted on /media/testing.tomb
davide ~/Desktop % tomb slam
[_] Slamming tomb testing mounted on /media/testing.tomb
 .  Kill all processes busy inside the tomb
[_] Tomb testing closed: your bones will rest in peace.
davide ~/Desktop % tomb list
[!] I can't see any open tomb, may they all rest in peace.

@davinerd
Member

Not working for me, either with an holding proc.
I'm doing some debug, and I would to ask somethings:

slam_tomb() {
# $1 = tomb mount point
for s in INT TERM HUP KILL; do
fuser -s -m "$1" || return 0
fuser -s -m "$1" -k -M -$s && { option_is_set -f || sleep 3 }
done
return 1
}

To successfully slam the tomb, that function must return 0. Ok. Than:

fuser -s -m "$1" || return 0

this should be true, but fuser -s -m "$1" always return 0. Than that
code will never be executed.

Another point: if the for loop kills my pids on KILL (last for loop
run), then the last command executed will be

fuser -s -m "$1" -k -M -$s && { option_is_set -f || sleep 3 }

the for loop returns and then

return 1

will be executed, meaning the failure.
I don't know how this can be works on your distro...I'm on gentoo.

Anathema

+--------------------------------------------------------------------+
|GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ |
|Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C |
| |
|http://www.msack.org |
|https://tboxes.tracciabi.li/anathema
+--------------------------------------------------------------------+

@boyska
Member
boyska commented Jan 16, 2012

You seem to be right. Indeed, fuser -s -m $1 always returns 0. BUT fuser -m $1 &> /dev/null has the behaviour we need.
Try if this change make it work fine.

@jaromil
Member
jaromil commented Jan 17, 2012

nope, it still returns 0

I've done some tries, but fuser seems to me a non-well formed commandline command.

why we changed to it? not for reliability, just for it being elegant?

I vote for reliability and revert to lsof parsing unless someone comes with a reliable use of fuser. I couldn't.

@davinerd davinerd was assigned Jan 17, 2012
@jaromil
Member
jaromil commented Jan 17, 2012

just for reference, incriminated commit:
d53c028

@davinerd
Member

On 16/01/2012 11:08, BoySka wrote:

You seem to be right. Indeed, fuser -s -m $1 always returns 0. BUT fuser -m $1 &> /dev/null has the behaviour we need.
Try if this change make it work fine.
This works for me too.
Changing the two lines makes slam_tomb() works as expected.

But we need to test the

fuser -m "$1" -k -M -$s &>/dev/null && { option_is_set -f || sleep 3 }

that needs the -f flag. Do we need it?

Anathema

+--------------------------------------------------------------------+
|GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ |
|Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C |
| |
|http://www.msack.org |
|https://tboxes.tracciabi.li/anathema
+--------------------------------------------------------------------+

@jaromil
Member
jaromil commented Jan 21, 2012

no the -f flag is not needed. actually I'm wondering now if we need the 'slam' command at all. we can start activating slam also when 'close -f' is called.

@davinerd
Member

On 21/01/2012 16:09, Jaromil wrote:

no the -f flag is not needed. actually I'm wondering now if we need the 'slam' command at all. we can start activating slam also when 'close -f' is called.
yeah: we can recycle slam_tomb(), calling it when using 'close -f'.
BUT: the -f flag? Why we havn't it yet?

It seems to be a great thing, maybe 'limited' for now to 'close' and
some others.

Anathema

+--------------------------------------------------------------------+
|GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ |
|Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C |
| |
|http://www.msack.org |
|https://tboxes.tracciabi.li/anathema
+--------------------------------------------------------------------+

@boyska
Member
boyska commented Jan 22, 2012

mh, there is a problem. Atm, slam use the -f flag to control whether or not to sleep between the kills.
We need to find another name, for this, like:
tomb close #normal
tomb close -f # same as tomb slam
tomb close -f --brutal # same as tomb slam -f

@davinerd
Member

On 22/01/2012 13:33, BoySka wrote:

mh, there is a problem. Atm, slam use the -f flag to control whether or not to sleep between the kills.
We need to find another name, for this, like:
tomb close #normal
tomb close -f # same as tomb slam
tomb close -f --brutal # same as tomb slam -f
Maybe

tomb close --brutal # without -f

can be very nice.
Who do this?

I'm busy working on tomb filesystem choose, and I can be ready for that
after 3-4 days.

Anathema

+--------------------------------------------------------------------+
|GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ |
|Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C |
| |
|http://www.msack.org |
|https://tboxes.tracciabi.li/anathema
+--------------------------------------------------------------------+

@jaromil
Member
jaromil commented Jan 23, 2012

mmm, wait a sec. we need to have LESS flags, not more. they complicate things, give users more documentation to read and often introduce ambiguity. I personally dream of a world without flags! (voluntary double-sense)

said that, there is no problem because:

close -f -> slam (wait)
slam -f -> slam (nowait)

@davinerd
Member

On 23/01/2012 10:48, Jaromil wrote:

mmm, wait a sec. we need to have LESS flags, not more. they complicate things, give users more documentation to read and often introduce ambiguity. I personally dream of a world without flags! (voluntary double-sense)

said that, there is no problem because:

close -f -> slam (wait)
slam -f -> slam (nowait)

Well, we can do:
close -f -> slam (wait)
slam -> slam (nowait)

agreed?

Anathema

+--------------------------------------------------------------------+
|GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ |
|Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C |
| |
|http://www.msack.org |
|https://tboxes.tracciabi.li/anathema
+--------------------------------------------------------------------+

@boyska
Member
boyska commented Jan 30, 2012

davinerd,jaromil: is fuser -m $1 &> /dev/null a valid solution for the problem? shall we commit it? please test it

@jaromil
Member
jaromil commented Jan 30, 2012

I've tested it again and same result: it does return zero all the time (I've tested on common Ubuntu and Debian systems)

We need something else there to return non-zero if the directory is not being used....

@boyska
Member
boyska commented Jan 31, 2012

uff.
we could do something like fuser|wc -w (that checks if there is any result), and checking it being zero/non-zero.
It do what expected, here, and it should work flawlessy everywhere. Can you test it, please? I want to get rid of this issue quickly.

@jaromil
Member
jaromil commented Feb 1, 2012

yes we need to solve this issue quick.

on the return code I get a different behaviour on debian and ubuntu (!?)

instead, your approach seems to work. i'm trying only on debian now
assuming '/' is in use and '/mnt' is not in use on my comp now, I tried

% caz=fuser /mnt 2> /dev/null ; { test "$caz" = "" } && { echo vuoto }
vuoto
% caz=fuser / 2> /dev/null ; { test "$caz" = "" } && { echo vuoto }
(return 1)

seems to be viable

@boyska
Member
boyska commented Feb 1, 2012

your simplification is fine: so the code is

if [[ -n `fuser -m /mount/point 2> /dev/null` ]]; then #there are processes
@boyska boyska added a commit that referenced this issue Feb 1, 2012
@boyska boyska FIX (tries) #77: slam wasn't slamming
That's because fuser behaves differently on debian.
(Why, debian, why???)
d57994f
@boyska
Member
boyska commented Feb 1, 2012

I tried to fix it with this new commit. Hope it works, please everyone test it.

tomb-git package for archlinux has been updated. arch testers, no excuses for you :)

@jaromil
Member
jaromil commented Feb 16, 2013

It works for me, I've been using it for a year now. Thanks :^)

@jaromil jaromil closed this Feb 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment