All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fox Chen <foxhlchen@gmail.com>
To: wangyugui@e16-tech.com, dsterba@suse.com
Cc: linux-btrfs@vger.kernel.org, Greg KH <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] btrfs-progs: utils: fix btrfs_wipe_existing_sb probe bug
Date: Thu, 15 Apr 2021 14:19:17 +0800	[thread overview]
Message-ID: <CAC2o3D+TkW7js=C+XYdnxs8wxruQ3kT3rutVBeuaozz2N9FDMA@mail.gmail.com> (raw)
In-Reply-To: <20210409123251.86BE.409509F4@e16-tech.com>

On Fri, Apr 9, 2021 at 12:32 PM Wang Yugui <wangyugui@e16-tech.com> wrote:
>
> Hi,
>
> > btrfs_wipe_existing_sb() misses calling blkid_do_fullprobe() to do
> > the real probe. After calling blkid_new_probe() &
> > blkid_probe_set_device() to setup blkid_probe context, it directly
> > calls blkid_probe_lookup_value(). This results in
> > blkid_probe_lookup_value returning -1, because pr->values is empty.
> >
> > Signed-off-by: Fox Chen <foxhlchen@gmail.com>
> > ---
> >  common/device-utils.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/device-utils.c b/common/device-utils.c
> > index c860b946..f8e2e776 100644
> > --- a/common/device-utils.c
> > +++ b/common/device-utils.c
> > @@ -114,7 +114,7 @@ static int btrfs_wipe_existing_sb(int fd)
> >       if (!pr)
> >               return -1;
> >
> > -     if (blkid_probe_set_device(pr, fd, 0, 0)) {
> > +     if (blkid_probe_set_device(pr, fd, 0, 0) || blkid_do_fullprobe(pr)) {
> >               ret = -1;
> >               goto out;
> >       }
> > --
> > 2.31.0
>
>
> With this patch,  'mkfs.btrfs -f /dev/nvme0n1 /dev/sdb' output some
> error when /dev/nvme0n1 have 2 partitions.
>         ERROR: cannot wipe superblocks on /dev/nvme0n1

I am not sure this error is caused by my improper fix, or bugs lie
somewhere else on mkfs.btrfs path.

But without this patch, I think, btrfs_wipe_existing_sb will not do
what is supposed to do -- wiping out existing superblocks.
Instead, it does nothing and always returns 1. Since it has no effect
and nobody complaints about it, that brings a question, do we really
need to call it in btrfs_prepare_device?

> # blkid
> /dev/nvme0n1: PTUUID="93a54ce8-04b2-470b-8c05-31bfcef02f28" PTTYPE="gpt"
> /dev/sda1: LABEL="OS_USB" UUID="2b7f4cb9-3dac-443f-8c96-a907b9276942" BLOCK_SIZE="512" TYPE="xfs" PARTUUID="ee58e9d3-01"
> /dev/nvme0n1p1: UUID="1d94dc2b-abd1-47df-bf39-ab31cf579d29" UUID_SUB="577542b7-91f5-48d4-a54c-98cbd4525c00" BLOCK_SIZE="4096" TYPE="btrfs" PARTLABEL="primary" PARTUUID="efed009f-8ae6-4567-9cd3-80a57cdcf225"
> /dev/nvme0n1p2: PARTLABEL="primary" PARTUUID="fcab66cd-daad-457f-a53c-110592d8941f"
> ...
>
>
> Without this patch,  'mkfs.btrfs -f /dev/nvme0n1 /dev/sdb' have some
> issue too when /dev/nvme0n1 and /dev/sdb  have some partitions.
>
> some blkid of partition is still left in the output of blkid.
> 'blockdev --rereadpt' will let them disappear.
>
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2021/04/09
>
>

thanks,
fox

      reply	other threads:[~2021-04-15  6:19 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-25 13:10 [PATCH] btrfs-progs: utils: fix btrfs_wipe_existing_sb probe bug Fox Chen
2021-04-09  4:32 ` Wang Yugui
2021-04-15  6:19   ` Fox Chen [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='CAC2o3D+TkW7js=C+XYdnxs8wxruQ3kT3rutVBeuaozz2N9FDMA@mail.gmail.com' \
    --to=foxhlchen@gmail.com \
    --cc=dsterba@suse.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=wangyugui@e16-tech.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.