public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Anand Jain <anand.jain@oracle.com>
Cc: systemd-devel@lists.freedesktop.org,
	"linux-btrfs@vger.kernel.org >> linux-btrfs"
	<linux-btrfs@vger.kernel.org>,
	lennart@poettering.net
Subject: Re: [survey]  BTRFS_IOC_DEVICES_READY return status
Date: Mon, 15 Jun 2015 17:01:57 +0200	[thread overview]
Message-ID: <20150615150156.GO6761@suse.cz> (raw)
In-Reply-To: <557ADBAE.9040407@oracle.com>

On Fri, Jun 12, 2015 at 09:16:30PM +0800, Anand Jain wrote:
> BTRFS_IOC_DEVICES_READY is to check if all the required devices
> are known by the btrfs kernel, so that admin/system-application
> could mount the FS. It is checked against a device in the argument.
> 
> However the actual implementation is bit more than just that,
> in the way that it would also scan and register the device
> provided in the argument (same as btrfs device scan subcommand
> or BTRFS_IOC_SCAN_DEV ioctl).
> 
> So BTRFS_IOC_DEVICES_READY ioctl isn't a read/view only ioctl,
> but its a write command as well.

The implemented DEVICES_READY behaviour is intentional, but not a good
example of ioctl interface design. I asked for a more generic interface
to querying devices when this patch was submitted but to no outcome.

> Next, since in the kernel we only check if total_devices
> (read from SB)  is equal to num_devices (counted in the list)
> to state the status as 0 (ready) or 1 (not ready). But this
> does not work in rest of the device pool state like missing,
> seeding, replacing since total_devices is actually not equal
> to num_devices in these state but device pool is ready for
> the mount and its a bug which is not part of this discussions.

That's an example why the single-shot ioctl is bad - it relies on some
internal state that's otherwise nontrivial to get.

> Questions:
> 
>   - Do we want BTRFS_IOC_DEVICES_READY ioctl to also scan and
>     register the device provided (same as btrfs device scan
>     command or the BTRFS_IOC_SCAN_DEV ioctl)
>     OR can BTRFS_IOC_DEVICES_READY be read-only ioctl interface
>     to check the state of the device pool. ?

This has been mentioned in the thread, we cannot change the ioctl that
way. Extensions are possible as far as they stay backward compatible
without changes to the existing users.

>   - If the the device in the argument is already mounted,
>     can it straightaway return 0 (ready) ? (as of now it would
>     again independently read the SB determine total_devices
>     and check against num_devices.

We can do that, looks like a safe optimization.

>   - What should be the expected return when the FS is mounted
>     and there is a missing device.

I think the current ioctl cannot give a good answer to that, similar to
the seeding or dev-replace case. We'd need an improved ioctl or do it
via sysfs which is my preference at the moment.

      parent reply	other threads:[~2015-06-15 15:01 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-12 13:16 [survey] BTRFS_IOC_DEVICES_READY return status Anand Jain
2015-06-12 18:04 ` [systemd-devel] " Andrei Borzenkov
2015-06-12 20:08   ` Goffredo Baroncelli
2015-06-13  9:35     ` Anand Jain
2015-06-13 15:09       ` Goffredo Baroncelli
     [not found]         ` <pan$63061$a3cdf5f6$a390adbd$e6097ad9@cox.net>
2015-06-14 19:44           ` Goffredo Baroncelli
2015-06-15 10:46         ` Lennart Poettering
2015-06-15 17:23           ` Goffredo Baroncelli
2015-06-15 17:38             ` Lennart Poettering
2015-06-17 19:10               ` Goffredo Baroncelli
2015-06-17 21:02                 ` Lennart Poettering
2015-06-18  2:40                   ` Andrei Borzenkov
2015-06-14  5:48       ` Andrei Borzenkov
2015-06-15 10:41       ` Lennart Poettering
2015-06-13  7:20 ` btrfs filesystem show confused when label is same as mountpoint Sjoerd
2015-06-13  9:51   ` Duncan
2015-06-25 16:37     ` David Sterba
2015-06-15 10:27 ` [survey] BTRFS_IOC_DEVICES_READY return status Lennart Poettering
2015-06-15 15:01 ` David Sterba [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150615150156.GO6761@suse.cz \
    --to=dsterba@suse.cz \
    --cc=anand.jain@oracle.com \
    --cc=lennart@poettering.net \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=systemd-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox