New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Importing an unlabeled btrfs RAID5 pool fails #1342

Closed
AngeleToR opened this Issue Jun 9, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@AngeleToR

AngeleToR commented Jun 9, 2016

Yesterday I found a minor bug when trying to import a btrfs RAID5 5 disk array into my new system. Another similar array with a label in it worked flawlessly, but the unlabeled one got succesfully imported, mounted and even displayed for a seconds, then dissapeared from the GUI but remained mounted under /mnt2/none (none is how it got temporaly labeled).

Just going to a shell, manually unmounting it, setting a label and re-importing from the GUI did the trick and made it work. As I said, minor annoyance and we all should label our filesystems after all, don't we??? 😆

It happened on latest stable, 3.8.13 fresh install

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Jun 10, 2016

Member

@AngeleToR This is a great find; thanks for reporting.
Linking to your forum thread to keep the two tied together.

Please update the following forum thread (by the same author) with any significant development on this issue:- https://forum.rockstor.com/t/importing-an-unlabeled-btrfs-raid5-volume-pool-fails/1590

This issue is caused by the import mechanism doing it's job as expected, the momentary appearance of the pool as evidence, but because the background periodic _update_disk_state() mechanism depends on labels to do it's thing it then fails to deal with the 'None' label condition.

My thoughts on this are that we have some pending changes in this area so I think it would be best to get these out of the way first before looking to this issue, especially now you have provided the work around (Thanks) but maybe after the commits associated with #1320 have been finalised, reviewed, and committed (if appropriate), we could fix this by enhancing the import mechanism to either auto label a pool if it is found to be un-labelled or prompt the use for a label when this circumstance is found. Just my quick thoughts on the matter.

Member

phillxnet commented Jun 10, 2016

@AngeleToR This is a great find; thanks for reporting.
Linking to your forum thread to keep the two tied together.

Please update the following forum thread (by the same author) with any significant development on this issue:- https://forum.rockstor.com/t/importing-an-unlabeled-btrfs-raid5-volume-pool-fails/1590

This issue is caused by the import mechanism doing it's job as expected, the momentary appearance of the pool as evidence, but because the background periodic _update_disk_state() mechanism depends on labels to do it's thing it then fails to deal with the 'None' label condition.

My thoughts on this are that we have some pending changes in this area so I think it would be best to get these out of the way first before looking to this issue, especially now you have provided the work around (Thanks) but maybe after the commits associated with #1320 have been finalised, reviewed, and committed (if appropriate), we could fix this by enhancing the import mechanism to either auto label a pool if it is found to be un-labelled or prompt the use for a label when this circumstance is found. Just my quick thoughts on the matter.

@schakrava schakrava added the bug label Jun 13, 2016

@schakrava schakrava added this to the Pinnacles milestone Jun 13, 2016

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Oct 28, 2016

Member

Please also update the following forum thread with significant progress on this issue:
https://forum.rockstor.com/t/importing-a-btrfs-array-from-a-different-server/2265

Member

phillxnet commented Oct 28, 2016

Please also update the following forum thread with significant progress on this issue:
https://forum.rockstor.com/t/importing-a-btrfs-array-from-a-different-server/2265

@AngeleToR

This comment has been minimized.

Show comment
Hide comment
@AngeleToR

AngeleToR Oct 28, 2016

Sorry for not updating the thread, I though I already did... Anyway, as Phillxnet just pointed out, naming the pool just fixed the issue. Tha fail came from a previous Ubuntu setup, where I could create and use the array unnamed and so I tried to import it that way... triggering the error...

AngeleToR commented Oct 28, 2016

Sorry for not updating the thread, I though I already did... Anyway, as Phillxnet just pointed out, naming the pool just fixed the issue. Tha fail came from a previous Ubuntu setup, where I could create and use the array unnamed and so I tried to import it that way... triggering the error...

@AngeleToR AngeleToR closed this Oct 28, 2016

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Oct 28, 2016

Member

@AngeleToR Thanks for the update, but I think this issue could be re-opened as it still exists, I was simply referencing your fine find on the work around you presented here. Not strictly an issue with intra-Rockstor pools and a niggle as you say but still it's not elegant behaviour and imports of whole disk btrfs should fall within our remit. Your call.

Member

phillxnet commented Oct 28, 2016

@AngeleToR Thanks for the update, but I think this issue could be re-opened as it still exists, I was simply referencing your fine find on the work around you presented here. Not strictly an issue with intra-Rockstor pools and a niggle as you say but still it's not elegant behaviour and imports of whole disk btrfs should fall within our remit. Your call.

@AngeleToR

This comment has been minimized.

Show comment
Hide comment
@AngeleToR

AngeleToR Oct 28, 2016

Oh well, I misunderstood it then... Pools should be named, as we all know, to avoid mistakes like deleting the one you don't want to and so. So it's not a bug in the strict sense, more like a 'nice to have' feature or, as you said, a non elegant behaviour of the interface, but it's OK for me to reopen it so it can be nicely closed once you fix it in the GUI.
Regards

AngeleToR commented Oct 28, 2016

Oh well, I misunderstood it then... Pools should be named, as we all know, to avoid mistakes like deleting the one you don't want to and so. So it's not a bug in the strict sense, more like a 'nice to have' feature or, as you said, a non elegant behaviour of the interface, but it's OK for me to reopen it so it can be nicely closed once you fix it in the GUI.
Regards

@AngeleToR AngeleToR reopened this Oct 28, 2016

@schakrava schakrava closed this in 0d17722 Mar 29, 2017

schakrava added a commit that referenced this issue Mar 29, 2017

Merge pull request #1683 from schakrava/1342_unlabeled
pool name should default to uuid if label is none. Fixes #1342

@schakrava schakrava self-assigned this Jul 2, 2017

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Aug 5, 2018

add system pool auto label and enhance associated db mechanisms #1947
Normalise use of 'btrfs fi show' for label info and when no label is
found, initiate an auto label; given the existence of a by-id (device
serial). If no serial and no label on the system drive / pool then skip
system pool / vol db counterpart creation. Also adds better logging and
earlier fail points concerning no by-id (no serial) by avoiding
subsequent mount attempts of associated vols and subvols.

Summary:

- Make ‘btrfs fi show’ via get_pool_info() canonical for volume label
retrieval, retaining a fail over to disk label for robustness.

- Extend existing auto volume (pool) labeling, within get_pool_info(),
to account specifically for the system pool: see previous issue #1342,
pr #1683, for non system disk.

- Improve system volume database management code to be more robust to no
system volume label scenarios; including in concert with no by-id name
being available (ie no serial on system disk).

- Add debug logging re system pool database counterpart creation or
otherwise (ie no by-id in combination with no serial).

- Dynamically retrieve and assign / update system volume (pool) raid;
where a by-id name is available.

- Move previous multiple hardcoded ‘system’ label references to
settings.conf.in / test-settings.conf.in, see caveats section of
pr #1935.

- Avoid all pool mounts when the referenced disk has no by-id name, and
error log accordingly: avoiding later complex fail messages where
downstream code requires by-id naming.

- Avoid share import / status update when the associated pool is not
mounted, error log accordingly.

- Avoid mount attempts of prior known shares whose associated pool is
not mounted, error log accordingly.

- Unrelated conditional around postgresql-setup to aid in building on
non legacy systems.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Aug 5, 2018

add system pool auto label and enhance associated db mechanisms #1947
Normalise use of 'btrfs fi show' for label info and when no label is
found, initiate an auto label; given the existence of a by-id (device
serial). If no serial and no label on the system drive / pool then skip
system pool / vol db counterpart creation. Also adds better logging and
earlier fail points concerning no by-id (no serial) by avoiding
subsequent mount attempts of associated vols and subvols.

Summary:

- Make ‘btrfs fi show’ via get_pool_info() canonical for volume label
retrieval, retaining a fail over to disk label for robustness.

- Extend existing auto volume (pool) labeling, within get_pool_info(),
to account specifically for the system pool: see previous issue #1342,
pr #1683, for non system disk.

- Improve system volume database management code to be more robust to no
system volume label scenarios; including in concert with no by-id name
being available (ie no serial on system disk).

- Add debug logging re system pool database counterpart creation or
otherwise (ie no by-id in combination with no serial).

- Dynamically retrieve and assign / update system volume (pool) raid;
where a by-id name is available.

- Move previous multiple hardcoded ‘system’ label references to
settings.conf.in / test-settings.conf.in, see caveats section of
pr #1935.

- Avoid all pool mounts when the referenced disk has no by-id name, and
error log accordingly: avoiding later complex fail messages where
downstream code requires by-id naming.

- Avoid share import / status update when the associated pool is not
mounted, error log accordingly.

- Avoid mount attempts of prior known shares whose associated pool is
not mounted, error log accordingly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment