Re: [PATCH v3] Btrfs-progs: check out if the swap device
On 2013/03/04 1:47, Brendan Hide wrote: On 2013/02/14 09:53 AM, Tsutomu Itoh wrote: +if (ret 0) { +fprintf(stderr, error checking %s status: %s\n, file, +strerror(-ret)); +exit(1); +} ... +/* check if the device is busy */ +fd = open(file, O_RDWR|O_EXCL); +if (fd 0) { +fprintf(stderr, unable to open %s: %s\n, file, +strerror(errno)); +exit(1); +} This is fine and works (as tested by David) - but I'm not sure if the below suggestions from Zach were taken into account. 1. If the check with open(file, O_RDWR|O_EXCL) shows that the device is available, there's no point in checking if it is mounted as a swap device. A preliminary check using this could precede all other checks which should be skipped if it shows success. 2. If there's an error checking the status (for example lets say /proc/swaps is deprecated), we should print the informational message but not error out. If open(file, O_RDWR|O_EXCL) failed, we should output an appropriate message why it failed. So, I'm testing is_swap_device() and check_mounted() first. But is_swap_device() is not perfect, so I'm trying to open O_EXCL after all other tests, as a last safety check. Thanks, Tsutomu On 2013/02/13 11:58 AM, Zach Brown wrote: - First always open with O_EXCL. If it succeeds then there's no reason to check /proc/swaps at all. (Maybe it doesn't need to try check_mounted() there either? Not sure if it's protecting against accidentally mounting mounted shared storage or not.) ... - At no point is failure of any of the /proc/swaps parsing fatal. It'd carry on ignoring errors until it doesnt have work to do. It'd only ever print the nice message when it finds a match. -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Btrfs-progs: check out if the swap device
On 2013/02/14 09:53 AM, Tsutomu Itoh wrote: + if (ret 0) { + fprintf(stderr, error checking %s status: %s\n, file, + strerror(-ret)); + exit(1); + } ... + /* check if the device is busy */ + fd = open(file, O_RDWR|O_EXCL); + if (fd 0) { + fprintf(stderr, unable to open %s: %s\n, file, + strerror(errno)); + exit(1); + } This is fine and works (as tested by David) - but I'm not sure if the below suggestions from Zach were taken into account. 1. If the check with open(file, O_RDWR|O_EXCL) shows that the device is available, there's no point in checking if it is mounted as a swap device. A preliminary check using this could precede all other checks which should be skipped if it shows success. 2. If there's an error checking the status (for example lets say /proc/swaps is deprecated), we should print the informational message but not error out. On 2013/02/13 11:58 AM, Zach Brown wrote: - First always open with O_EXCL. If it succeeds then there's no reason to check /proc/swaps at all. (Maybe it doesn't need to try check_mounted() there either? Not sure if it's protecting against accidentally mounting mounted shared storage or not.) ... - At no point is failure of any of the /proc/swaps parsing fatal. It'd carry on ignoring errors until it doesnt have work to do. It'd only ever print the nice message when it finds a match. -- __ Brendan Hide http://swiftspirit.co.za/ http://www.webafrica.co.za/?AFF1E97 -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Btrfs-progs: check out if the swap device
On Thu, Feb 14, 2013 at 04:53:04PM +0900, Tsutomu Itoh wrote: Signed-off-by: Tsutomu Itoh t-i...@jp.fujitsu.com Worked for me here in these situations (ie. mkfs failed, filenames specified given as simple path or with mixture of ./ and ../ leading to the device): * mounted /dev/sdx * /dev/sdx part of a multi-device mounted fs * /dev/sdx swap device * file as a swap device * swap files with funny names: \ \040 \134 \888 space Tested-by: David Sterba dste...@suse.cz -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html