search

coreos / torus

Torus Distributed Storage
https://coreos.com/blog/torus-distributed-storage-by-coreos.html
Apache License 2.0
1.77k stars 172 forks source link

aoe: flush aoe device when finished serving #412

Open nak3 opened 7 years ago

nak3 commented 7 years ago

fixes https://github.com/coreos/torus/issues/389

nak3 commented 7 years ago

Sure, I updated and added the comment.

xiang90 commented 7 years ago

@nak3 Did you verify this that this patch fixes #389?

nak3 commented 7 years ago

Yes, I did verify it.

(test record)

1. Start and stop torusblk with aoe block device

[vagrant@torus1 torus]$ sudo ./bin/torusblk aoe test1 --debug lo 1 1 ... Attached to AoE device (lo, Major: 1, Minor: 1). Server loop begins ... ... 2016-12-13 12:28:08.869225 D | block: Syncing block volume: test1 ^C Received an interrupt, stopping services... 2016-12-13 12:28:11.547470 E | aoe: failed to handle frame: bad file descriptor 2016-12-13 12:28:11.547612 E | aoe: ReadFrom failed: bad file descriptor 2016-12-13 12:28:11.556207 D | aoe: canceling background goroutines 2016-12-13 12:28:11.556368 D | aoe: waiting for background goroutines to stop 2016-12-13 12:28:11.564921 D | aoe: stopping device sync goroutine 2016-12-13 12:28:11.565137 D | block: not syncing

2. Confirm /dev/etherd/e1.1 has been removed

[vagrant@torus1 torus]$ ls /dev/etherd/p discover err flush interfaces revalidate

3. fdisk works

[vagrant@torus1 torus]$ sudo fdisk -l > /dev/null [vagrant@torus1 torus]$ echo $? 0

xiang90 commented 7 years ago

lgtm. defer to @lpabon

mischief commented 7 years ago

would this be better as a defer, or possibly in the aoe server's Close()?

nak3 commented 7 years ago

I got a review https://github.com/coreos/torus/pull/412#discussion_r91745216 before. And I agreed with reviewer that there are no reason to use defer.

For the idea of aoe server's Close(), as.Close() here should be called after NewServer() but before as.Serve(). So, since flush() should be surely called after the device was created, it should not be called with defer as.Close().