Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 5 Oct 2021 17:45:00 +0200
From:   David Sterba <dsterba@suse.cz>
To:     Hamza Mahfooz <someguy@effective-light.com>
Cc:     linux-kernel@vger.kernel.org, Chris Mason <clm@fb.com>,
        Josef Bacik <josef@toxicpanda.com>,
        David Sterba <dsterba@suse.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: send: add otime support to send_utimes()
Message-ID: <20211005154500.GG9286@twin.jikos.cz>
Reply-To: dsterba@suse.cz
Mail-Followup-To: dsterba@suse.cz,
        Hamza Mahfooz <someguy@effective-light.com>,
        linux-kernel@vger.kernel.org, Chris Mason <clm@fb.com>,
        Josef Bacik <josef@toxicpanda.com>, David Sterba <dsterba@suse.com>,
        linux-btrfs@vger.kernel.org
References: <20211005153514.4281-1-someguy@effective-light.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211005153514.4281-1-someguy@effective-light.com>
User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12)
Precedence: bulk

On Tue, Oct 05, 2021 at 11:35:14AM -0400, Hamza Mahfooz wrote:
> Commit 766702ef49b8 ("Btrfs: add/fix comments/documentation for
> send/receive") suggests that, otime support should be added to
> send_utimes() after btrfs gets otime support. So, since btrfs has had otime
> support for many years, we should add otime support to send_utimes().
> 
> Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
> ---
>  fs/btrfs/send.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 72f9b865e847..0bee9f7a45da 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -2544,7 +2544,7 @@ static int send_utimes(struct send_ctx *sctx, u64 ino, u64 gen)
>  	TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_ATIME, eb, &ii->atime);
>  	TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_MTIME, eb, &ii->mtime);
>  	TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_CTIME, eb, &ii->ctime);
> -	/* TODO Add otime support when the otime patches get into upstream */

Are you going after TODOs in the btrfs code? Some of them are stale or
don't reveal the real scope of the required changes.

> +	TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_OTIME, eb, &ii->otime);

For example this adds a new data to 'utimes' in the protocol v1, which
would require a major update. One such update is in progress and would
probably gather all pending updates though I'm not sure is this one in
particular is there.

So the reason why it can't be done by that is that the send steam would
be invalid and would break the receive. Also, testing changes is good.