* [RFC] BTRFS_DEV_REPLACE_ITEM_STATE_* doesn't match with on disk @ 2018-11-12 4:58 Anand Jain 2018-11-12 7:50 ` Nikolay Borisov 0 siblings, 1 reply; 6+ messages in thread From: Anand Jain @ 2018-11-12 4:58 UTC (permalink / raw) To: linux-btrfs The dev_replace_state defines are miss matched between the BTRFS_IOCTL_DEV_REPLACE_STATE_* and BTRFS_DEV_REPLACE_ITEM_STATE_* [1]. [1] ----------------------------- btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED 2 btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED 3 btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED 4 btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 ----------------------------- The BTRFS_DEV_REPLACE_ITEM_STATE_* series is unused in both btrfs.ko and btrfs-progs, the on-disk also follows BTRFS_IOCTL_DEV_REPLACE_STATE_* (we set dev_replace->replace_state using the BTRFS_IOCTL_DEV_REPLACE_STATE_* defines and write to the on-disk). 359 btrfs_set_dev_replace_replace_state(eb, ptr, 360 dev_replace->replace_state); IMO it should be ok to delete the BTRFS_DEV_REPLACE_ITEM_STATE_* altogether? But how about the userland progs other than btrfs-progs? If not at least fix the miss match as in [2], any comments? [2] -------------------------------------- diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h index aff1356c2bb8..9ffa7534cadf 100644 --- a/include/uapi/linux/btrfs_tree.h +++ b/include/uapi/linux/btrfs_tree.h @@ -805,9 +805,9 @@ struct btrfs_dev_stats_item { #define BTRFS_DEV_REPLACE_ITEM_CONT_READING_FROM_SRCDEV_MODE_AVOID 1 #define BTRFS_DEV_REPLACE_ITEM_STATE_NEVER_STARTED 0 #define BTRFS_DEV_REPLACE_ITEM_STATE_STARTED 1 -#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 -#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 -#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 +#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 2 +#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 3 +#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 4 struct btrfs_dev_replace_item { /* -------------------------------------- Thanks, Anand ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC] BTRFS_DEV_REPLACE_ITEM_STATE_* doesn't match with on disk 2018-11-12 4:58 [RFC] BTRFS_DEV_REPLACE_ITEM_STATE_* doesn't match with on disk Anand Jain @ 2018-11-12 7:50 ` Nikolay Borisov 2018-11-13 10:32 ` Anand Jain 2019-08-02 9:46 ` David Sterba 0 siblings, 2 replies; 6+ messages in thread From: Nikolay Borisov @ 2018-11-12 7:50 UTC (permalink / raw) To: Anand Jain, linux-btrfs On 12.11.18 г. 6:58 ч., Anand Jain wrote: > > The dev_replace_state defines are miss matched between the > BTRFS_IOCTL_DEV_REPLACE_STATE_* and BTRFS_DEV_REPLACE_ITEM_STATE_* [1]. > > [1] > ----------------------------- > btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED 2 > btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED 3 > btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED 4 > > btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 > btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 > btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 > ----------------------------- > > The BTRFS_DEV_REPLACE_ITEM_STATE_* series is unused in both btrfs.ko and > btrfs-progs, the on-disk also follows BTRFS_IOCTL_DEV_REPLACE_STATE_* > (we set dev_replace->replace_state using the > BTRFS_IOCTL_DEV_REPLACE_STATE_* defines and write to the on-disk). > > 359 btrfs_set_dev_replace_replace_state(eb, ptr, > 360 dev_replace->replace_state); > > IMO it should be ok to delete the BTRFS_DEV_REPLACE_ITEM_STATE_* > altogether? But how about the userland progs other than btrfs-progs? > If not at least fix the miss match as in [2], any comments? Unfortunately you are right. This seems to stem from sloppy job back in the days of initial dev-replace support. BTRFS_DEV_REPLACE_ITEM_STATE_* were added in e922e087a35c ("Btrfs: enhance btrfs structures for device replace support"), yet they were never used. And the IOCTL_DEV_REPLACE_STATE* were added in e93c89c1aaaa ("Btrfs: add new sources for device replace code"). It looks like the ITEM_STATE* definitions were stillborn so to speak and personally I'm in favor of removing them. They shouldn't have been merged in the first place and indeed the patch doesn't even have a Reviewed-by tag. So it originated from the, I'd say, spartan days of btrfs development... David, any code which is using BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED is inherently broken, so how about we remove those definitions, then when it's compilation is broken in the future the author will actually have a chance to fix it, though it's highly unlikely anyone is relying on those definitions. > > [2] > -------------------------------------- > diff --git a/include/uapi/linux/btrfs_tree.h > b/include/uapi/linux/btrfs_tree.h > index aff1356c2bb8..9ffa7534cadf 100644 > --- a/include/uapi/linux/btrfs_tree.h > +++ b/include/uapi/linux/btrfs_tree.h > @@ -805,9 +805,9 @@ struct btrfs_dev_stats_item { > #define BTRFS_DEV_REPLACE_ITEM_CONT_READING_FROM_SRCDEV_MODE_AVOID 1 > #define BTRFS_DEV_REPLACE_ITEM_STATE_NEVER_STARTED 0 > #define BTRFS_DEV_REPLACE_ITEM_STATE_STARTED 1 > -#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 > -#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 > -#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 > +#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 2 > +#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 3 > +#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 4 > > struct btrfs_dev_replace_item { > /* > -------------------------------------- > > > Thanks, Anand > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] BTRFS_DEV_REPLACE_ITEM_STATE_* doesn't match with on disk 2018-11-12 7:50 ` Nikolay Borisov @ 2018-11-13 10:32 ` Anand Jain 2018-11-21 7:31 ` Anand Jain 2019-08-02 9:46 ` David Sterba 1 sibling, 1 reply; 6+ messages in thread From: Anand Jain @ 2018-11-13 10:32 UTC (permalink / raw) To: David Sterba; +Cc: Nikolay Borisov, linux-btrfs David, Gentle ping. Thanks, Anand On 11/12/2018 03:50 PM, Nikolay Borisov wrote: > > > On 12.11.18 г. 6:58 ч., Anand Jain wrote: >> >> The dev_replace_state defines are miss matched between the >> BTRFS_IOCTL_DEV_REPLACE_STATE_* and BTRFS_DEV_REPLACE_ITEM_STATE_* [1]. >> >> [1] >> ----------------------------- >> btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED 2 >> btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED 3 >> btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED 4 >> >> btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 >> btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 >> btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 >> ----------------------------- >> >> The BTRFS_DEV_REPLACE_ITEM_STATE_* series is unused in both btrfs.ko and >> btrfs-progs, the on-disk also follows BTRFS_IOCTL_DEV_REPLACE_STATE_* >> (we set dev_replace->replace_state using the >> BTRFS_IOCTL_DEV_REPLACE_STATE_* defines and write to the on-disk). >> >> 359 btrfs_set_dev_replace_replace_state(eb, ptr, >> 360 dev_replace->replace_state); >> >> IMO it should be ok to delete the BTRFS_DEV_REPLACE_ITEM_STATE_* >> altogether? But how about the userland progs other than btrfs-progs? >> If not at least fix the miss match as in [2], any comments? > > Unfortunately you are right. This seems to stem from sloppy job back in > the days of initial dev-replace support. BTRFS_DEV_REPLACE_ITEM_STATE_* > were added in e922e087a35c ("Btrfs: enhance btrfs structures for device > replace support"), yet they were never used. And the > IOCTL_DEV_REPLACE_STATE* were added in e93c89c1aaaa ("Btrfs: add new > sources for device replace code"). > > It looks like the ITEM_STATE* definitions were stillborn so to speak and > personally I'm in favor of removing them. They shouldn't have been > merged in the first place and indeed the patch doesn't even have a > Reviewed-by tag. So it originated from the, I'd say, spartan days of > btrfs development... > > David, any code which is using BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED > is inherently broken, so how about we remove those definitions, then > when it's compilation is broken in the future the author will actually > have a chance to fix it, though it's highly unlikely anyone is relying > on those definitions. > > >> >> [2] >> -------------------------------------- >> diff --git a/include/uapi/linux/btrfs_tree.h >> b/include/uapi/linux/btrfs_tree.h >> index aff1356c2bb8..9ffa7534cadf 100644 >> --- a/include/uapi/linux/btrfs_tree.h >> +++ b/include/uapi/linux/btrfs_tree.h >> @@ -805,9 +805,9 @@ struct btrfs_dev_stats_item { >> #define BTRFS_DEV_REPLACE_ITEM_CONT_READING_FROM_SRCDEV_MODE_AVOID 1 >> #define BTRFS_DEV_REPLACE_ITEM_STATE_NEVER_STARTED 0 >> #define BTRFS_DEV_REPLACE_ITEM_STATE_STARTED 1 >> -#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 >> -#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 >> -#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 >> +#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 2 >> +#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 3 >> +#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 4 >> >> struct btrfs_dev_replace_item { >> /* >> -------------------------------------- >> >> >> Thanks, Anand >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] BTRFS_DEV_REPLACE_ITEM_STATE_* doesn't match with on disk 2018-11-13 10:32 ` Anand Jain @ 2018-11-21 7:31 ` Anand Jain 2019-08-02 4:07 ` Anand Jain 0 siblings, 1 reply; 6+ messages in thread From: Anand Jain @ 2018-11-21 7:31 UTC (permalink / raw) To: David Sterba, dsterba; +Cc: Nikolay Borisov, linux-btrfs David, any comments on this please. Thanks, Anand On 11/13/2018 06:32 PM, Anand Jain wrote: > > David, Gentle ping. > > Thanks, Anand > > On 11/12/2018 03:50 PM, Nikolay Borisov wrote: >> >> >> On 12.11.18 г. 6:58 ч., Anand Jain wrote: >>> >>> The dev_replace_state defines are miss matched between the >>> BTRFS_IOCTL_DEV_REPLACE_STATE_* and BTRFS_DEV_REPLACE_ITEM_STATE_* [1]. >>> >>> [1] >>> ----------------------------- >>> btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED 2 >>> btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED 3 >>> btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED 4 >>> >>> btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 >>> btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 >>> btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 >>> ----------------------------- >>> >>> The BTRFS_DEV_REPLACE_ITEM_STATE_* series is unused in both btrfs.ko and >>> btrfs-progs, the on-disk also follows BTRFS_IOCTL_DEV_REPLACE_STATE_* >>> (we set dev_replace->replace_state using the >>> BTRFS_IOCTL_DEV_REPLACE_STATE_* defines and write to the on-disk). >>> >>> 359 btrfs_set_dev_replace_replace_state(eb, ptr, >>> 360 dev_replace->replace_state); >>> >>> IMO it should be ok to delete the BTRFS_DEV_REPLACE_ITEM_STATE_* >>> altogether? But how about the userland progs other than btrfs-progs? >>> If not at least fix the miss match as in [2], any comments? >> >> Unfortunately you are right. This seems to stem from sloppy job back in >> the days of initial dev-replace support. BTRFS_DEV_REPLACE_ITEM_STATE_* >> were added in e922e087a35c ("Btrfs: enhance btrfs structures for device >> replace support"), yet they were never used. And the >> IOCTL_DEV_REPLACE_STATE* were added in e93c89c1aaaa ("Btrfs: add new >> sources for device replace code"). >> >> It looks like the ITEM_STATE* definitions were stillborn so to speak and >> personally I'm in favor of removing them. They shouldn't have been >> merged in the first place and indeed the patch doesn't even have a >> Reviewed-by tag. So it originated from the, I'd say, spartan days of >> btrfs development... >> >> David, any code which is using BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED >> is inherently broken, so how about we remove those definitions, then >> when it's compilation is broken in the future the author will actually >> have a chance to fix it, though it's highly unlikely anyone is relying >> on those definitions. >> >> >>> >>> [2] >>> -------------------------------------- >>> diff --git a/include/uapi/linux/btrfs_tree.h >>> b/include/uapi/linux/btrfs_tree.h >>> index aff1356c2bb8..9ffa7534cadf 100644 >>> --- a/include/uapi/linux/btrfs_tree.h >>> +++ b/include/uapi/linux/btrfs_tree.h >>> @@ -805,9 +805,9 @@ struct btrfs_dev_stats_item { >>> #define >>> BTRFS_DEV_REPLACE_ITEM_CONT_READING_FROM_SRCDEV_MODE_AVOID 1 >>> #define BTRFS_DEV_REPLACE_ITEM_STATE_NEVER_STARTED 0 >>> #define BTRFS_DEV_REPLACE_ITEM_STATE_STARTED 1 >>> -#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 >>> -#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 >>> -#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 >>> +#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 2 >>> +#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 3 >>> +#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 4 >>> >>> struct btrfs_dev_replace_item { >>> /* >>> -------------------------------------- >>> >>> >>> Thanks, Anand >>> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] BTRFS_DEV_REPLACE_ITEM_STATE_* doesn't match with on disk 2018-11-21 7:31 ` Anand Jain @ 2019-08-02 4:07 ` Anand Jain 0 siblings, 0 replies; 6+ messages in thread From: Anand Jain @ 2019-08-02 4:07 UTC (permalink / raw) To: David Sterba, dsterba; +Cc: Nikolay Borisov, linux-btrfs Ping. On 11/21/18 3:31 PM, Anand Jain wrote: > > David, any comments on this please. > > Thanks, Anand > > > On 11/13/2018 06:32 PM, Anand Jain wrote: >> >> David, Gentle ping. >> >> Thanks, Anand >> >> On 11/12/2018 03:50 PM, Nikolay Borisov wrote: >>> >>> >>> On 12.11.18 г. 6:58 ч., Anand Jain wrote: >>>> >>>> The dev_replace_state defines are miss matched between the >>>> BTRFS_IOCTL_DEV_REPLACE_STATE_* and BTRFS_DEV_REPLACE_ITEM_STATE_* [1]. >>>> >>>> [1] >>>> ----------------------------- >>>> btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED 2 >>>> btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED 3 >>>> btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED 4 >>>> >>>> btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 >>>> btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 >>>> btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 >>>> ----------------------------- >>>> >>>> The BTRFS_DEV_REPLACE_ITEM_STATE_* series is unused in both btrfs.ko >>>> and >>>> btrfs-progs, the on-disk also follows BTRFS_IOCTL_DEV_REPLACE_STATE_* >>>> (we set dev_replace->replace_state using the >>>> BTRFS_IOCTL_DEV_REPLACE_STATE_* defines and write to the on-disk). >>>> >>>> 359 btrfs_set_dev_replace_replace_state(eb, ptr, >>>> 360 dev_replace->replace_state); >>>> >>>> IMO it should be ok to delete the BTRFS_DEV_REPLACE_ITEM_STATE_* >>>> altogether? But how about the userland progs other than btrfs-progs? >>>> If not at least fix the miss match as in [2], any comments? >>> >>> Unfortunately you are right. This seems to stem from sloppy job back in >>> the days of initial dev-replace support. BTRFS_DEV_REPLACE_ITEM_STATE_* >>> were added in e922e087a35c ("Btrfs: enhance btrfs structures for device >>> replace support"), yet they were never used. And the >>> IOCTL_DEV_REPLACE_STATE* were added in e93c89c1aaaa ("Btrfs: add new >>> sources for device replace code"). >>> >>> It looks like the ITEM_STATE* definitions were stillborn so to speak and >>> personally I'm in favor of removing them. They shouldn't have been >>> merged in the first place and indeed the patch doesn't even have a >>> Reviewed-by tag. So it originated from the, I'd say, spartan days of >>> btrfs development... >>> >>> David, any code which is using BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED >>> is inherently broken, so how about we remove those definitions, then >>> when it's compilation is broken in the future the author will actually >>> have a chance to fix it, though it's highly unlikely anyone is relying >>> on those definitions. >>> >>> >>>> >>>> [2] >>>> -------------------------------------- >>>> diff --git a/include/uapi/linux/btrfs_tree.h >>>> b/include/uapi/linux/btrfs_tree.h >>>> index aff1356c2bb8..9ffa7534cadf 100644 >>>> --- a/include/uapi/linux/btrfs_tree.h >>>> +++ b/include/uapi/linux/btrfs_tree.h >>>> @@ -805,9 +805,9 @@ struct btrfs_dev_stats_item { >>>> #define >>>> BTRFS_DEV_REPLACE_ITEM_CONT_READING_FROM_SRCDEV_MODE_AVOID 1 >>>> #define BTRFS_DEV_REPLACE_ITEM_STATE_NEVER_STARTED 0 >>>> #define BTRFS_DEV_REPLACE_ITEM_STATE_STARTED 1 >>>> -#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 >>>> -#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 >>>> -#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 >>>> +#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 2 >>>> +#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 3 >>>> +#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 4 >>>> >>>> struct btrfs_dev_replace_item { >>>> /* >>>> -------------------------------------- >>>> >>>> >>>> Thanks, Anand >>>> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] BTRFS_DEV_REPLACE_ITEM_STATE_* doesn't match with on disk 2018-11-12 7:50 ` Nikolay Borisov 2018-11-13 10:32 ` Anand Jain @ 2019-08-02 9:46 ` David Sterba 1 sibling, 0 replies; 6+ messages in thread From: David Sterba @ 2019-08-02 9:46 UTC (permalink / raw) To: Nikolay Borisov; +Cc: Anand Jain, linux-btrfs On Mon, Nov 12, 2018 at 09:50:36AM +0200, Nikolay Borisov wrote: > On 12.11.18 г. 6:58 ч., Anand Jain wrote: > > The dev_replace_state defines are miss matched between the > > BTRFS_IOCTL_DEV_REPLACE_STATE_* and BTRFS_DEV_REPLACE_ITEM_STATE_* [1]. > > > > [1] > > ----------------------------- > > btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_FINISHED 2 > > btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_CANCELED 3 > > btrfs.h:#define BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED 4 > > > > btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED 2 > > btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_FINISHED 3 > > btrfs_tree.h:#define BTRFS_DEV_REPLACE_ITEM_STATE_CANCELED 4 > > ----------------------------- > > > > The BTRFS_DEV_REPLACE_ITEM_STATE_* series is unused in both btrfs.ko and > > btrfs-progs, the on-disk also follows BTRFS_IOCTL_DEV_REPLACE_STATE_* > > (we set dev_replace->replace_state using the > > BTRFS_IOCTL_DEV_REPLACE_STATE_* defines and write to the on-disk). > > > > 359 btrfs_set_dev_replace_replace_state(eb, ptr, > > 360 dev_replace->replace_state); > > > > IMO it should be ok to delete the BTRFS_DEV_REPLACE_ITEM_STATE_* > > altogether? But how about the userland progs other than btrfs-progs? > > If not at least fix the miss match as in [2], any comments? > > Unfortunately you are right. This seems to stem from sloppy job back in > the days of initial dev-replace support. BTRFS_DEV_REPLACE_ITEM_STATE_* > were added in e922e087a35c ("Btrfs: enhance btrfs structures for device > replace support"), yet they were never used. And the > IOCTL_DEV_REPLACE_STATE* were added in e93c89c1aaaa ("Btrfs: add new > sources for device replace code"). > > It looks like the ITEM_STATE* definitions were stillborn so to speak and > personally I'm in favor of removing them. They shouldn't have been > merged in the first place and indeed the patch doesn't even have a > Reviewed-by tag. So it originated from the, I'd say, spartan days of > btrfs development... > > David, any code which is using BTRFS_DEV_REPLACE_ITEM_STATE_SUSPENDED > is inherently broken, so how about we remove those definitions, then > when it's compilation is broken in the future the author will actually > have a chance to fix it, though it's highly unlikely anyone is relying > on those definitions. I agree it's better to remove them, progs use the define but there's own defintion in ioctl.h so it's not affected by the kernel header. It's not like we're removing something in wide use so even if there are some code that uses it it can be fixed trivially. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-02 9:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-12 4:58 [RFC] BTRFS_DEV_REPLACE_ITEM_STATE_* doesn't match with on disk Anand Jain 2018-11-12 7:50 ` Nikolay Borisov 2018-11-13 10:32 ` Anand Jain 2018-11-21 7:31 ` Anand Jain 2019-08-02 4:07 ` Anand Jain 2019-08-02 9:46 ` David Sterba
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).