Re: [PATCH v3] Btrfs-progs: check out if the swap device

2013-03-04 Thread Tsutomu Itoh

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

2013-03-03 Thread Brendan Hide

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

2013-02-14 Thread David Sterba
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