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

[Tsutomu Itoh <t-itoh@jp.fujitsu.com>]

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.
>
>