Bug 162690 - [geom] [patch] gpart label changes only take effect after a reboot
Summary: [geom] [patch] gpart label changes only take effect after a reboot
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: Normal Affects Only Me
Assignee: Marcelo Araujo
URL:
Keywords: patch
: 177080 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-11-20 13:30 UTC by sub.mesa
Modified: 2017-05-17 05:21 UTC (History)
7 users (show)

See Also:


Attachments
patch to add refresh method to glabel (1.48 KB, patch)
2016-09-13 12:13 UTC, Ben RUBSON
no flags Details | Diff
patch to handle gpart add/modify automatically (thx to Andrey) (626 bytes, patch)
2016-09-18 14:45 UTC, Ben RUBSON
no flags Details | Diff
patch to add refresh method to glabel (1.94 KB, patch)
2016-11-25 16:22 UTC, Ben RUBSON
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description sub.mesa 2011-11-20 13:30:16 UTC
gpart has functionality to change the label of a (GPT) partition. This functionality works like it should, however, after a label change the /dev/gpt/ entries remain unchanged. glabel status output remains unchanged. The change only takes effect after a reboot.

The device was not in use or otherwise 'locked', gpart list shows the new label, but otherwise it appears that the gpart modify -l command does not actually change the device entries, as is expected and desired.

How-To-Repeat: [root@zfsguru ~]# gpart show ada0
=>      34  12582845  ada0  GPT  (6.0G)
        34       512     1  freebsd-boot  (256k)
       546      1502        - free -  (751k)
      2048  12576768     2  freebsd-zfs  (6G)
  12578816      4063        - free -  (2M)

[root@zfsguru ~]# glabel status
                  Name  Status  Components
iso9660/ZFSGURU-LIVECD     N/A  cd0
         gpt/testdisk1     N/A  ada0p2

[root@zfsguru ~]# gpart list ada0 | grep label
   label: (null)
   label: testdisk1

[root@zfsguru ~]# gpart modify -i 1 -l "bootlabel" ada0
ada0p1 modified

[root@zfsguru ~]# gpart list ada0 | grep label
   label: bootlabel
   label: testdisk1

[root@zfsguru ~]# glabel status
                  Name  Status  Components
iso9660/ZFSGURU-LIVECD     N/A  cd0
         gpt/testdisk1     N/A  ada0p2

[root@zfsguru ~]# ls -l /dev/gpt/
total 0
crw-r-----  1 root  operator    0,  76 Nov 20 13:19 testdisk1
Comment 1 Andrey V. Elsukov freebsd_committer freebsd_triage 2012-08-06 05:59:45 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-geom

Reassign.
Comment 2 deeptech71@gmail.com 2013-03-29 07:52:32 UTC
There is a (critical-severity (!)) duplicate of this bug report:

bin/177080: gpart(8): /dev/gpt labels are not updated on ``gpart modify''



I'll note an additional discovery: mounting and unmounting the relabeled partition also causes /dev/gpt/* to be updated.
Comment 3 Geoff Garside 2013-08-22 10:31:49 UTC
This still appears to be alive and kicking in 9.1p6, I've admittedly not =
tested 9.2 or 10-CURRENT
Comment 4 Xenomorph 2015-05-12 19:11:00 UTC
This seems to still be an issue as of 10.1-RELEASE-p9 in May of 2015. Has this really been open since 2011?

I have partition 1 named "test1" on da1. /dev/da1p1 is not mounted or in use in any way.

# gpart list | grep label

label: test1

# ll /dev/gpt

crw-r-----  1 root  operator   0x72 May 12 13:44 test1

I want to rename it to "disk1"

# gpart modify -i 1 -l "disk1" da1

da1 modified

# gpart list | grep label

label: disk1

# ll /dev/gpt

crw-r-----  1 root  operator   0x72 May 12 13:44 test1


The partition label has been changed, but the contents of /dev/gpt doesn't update unto I reboot the system.

I've found that if I *delete* the partition, then remake it with the new label, then /dev/gpt will show the correct labels.

Even if the update of /dev/gpt isn't automatic, should there a command or something to manually trigger its update?

On servers with a lot of drive bays, the slot order can be changed on startup. I haven't found a way to make "mfisyspdX" remain consistent (even with /boot/device.hints), to being able to easily manage labels can be a big deal.
Comment 5 Andrey V. Elsukov freebsd_committer freebsd_triage 2015-05-14 19:16:56 UTC
The problem is that GEOM_LABEL class was appointed for handling GPT labels, but it isn't compatible to do this task. By design it handles labels from the same GEOM provider. GPT labels are stored in the partition table metadada on the different GEOM provider. And when you are changing the label, GEOM_LABEL doesn't know about this, because its provider wasn't spoiled. There are several solutions are possible:
1. Don't use GPT labels.
2. Assign GEOM_PART class for handling GPT labels and implement such functional.
3. Probably some dirty hack can notify GEOM_LABEL about changed GPT labels.
Comment 6 Ben RUBSON 2016-08-17 15:58:03 UTC
Hello,

Same sort of issue with glabel (labels in /dev/label).

// let's create a label.
# glabel label /dev/gpt/mydisk mylabel
# ls -l /dev/label/mylabel
crw-r-----  1 root  operator  0xa5 17 Aug 15:32 /dev/label/mylabel

// let's put /dev/gpt/mydisk into /etc/ctl.conf to export it as an iSCSI target.
# service ctld onestart
Starting ctld.

# ls -l /dev/label/mylabel
ls: /dev/label/mylabel: No such file or directory
// expected, because at this time the label is part of the exported iSCSI disk.

# service ctld onestop
Stopping ctld.
# ls -l /dev/label/mylabel
ls: /dev/label/mylabel: No such file or directory
// not expected, label should be available at this time !

Issue is present in both 10.3 and 11-RC1.

Is there any way to notify / update labels ?

Many thanks !

Ben
Comment 7 Ben RUBSON 2016-08-17 17:05:15 UTC
I found a workaround for glabel.

At the end of sbin/geom/class/label/geom_label.c, in label_dump, right after label_metadata_dump(&md);

I added the following :
int fd = g_open(name, 1);
if (fd != -1)
  (void)g_close(fd);

Then, 'glabel dump /dev/da*' will refresh labels.

Trick also works with GPT, if you g_open(device, 1), labels will be refreshed.

Perhaps we could add a glabel refresh option which would simply open/close devices in order to refresh their labels ?
Comment 8 Ben RUBSON 2016-08-18 06:11:59 UTC
https://github.com/freebsd/freebsd/pull/84
Comment 9 Ben RUBSON 2016-08-25 05:37:59 UTC
Andrey, do you think we could have this committed to stable/11 so that it will be in 11.0-RELEASE ?

Many thanks !

Ben
Comment 10 Ben RUBSON 2016-09-13 12:13:46 UTC
Created attachment 174723 [details]
patch to add refresh method to glabel
Comment 11 Ben RUBSON 2016-09-13 12:15:24 UTC
I just have been told that GitHub PR is not the right method to submit a patch, so I followed the "How to Contribute" section of the FreeBSD Manual, and here is the patch.
Comment 12 Kurt Jaeger freebsd_committer freebsd_triage 2016-09-18 11:51:45 UTC
*** Bug 177080 has been marked as a duplicate of this bug. ***
Comment 13 Ben RUBSON 2016-09-18 14:45:34 UTC
Created attachment 174918 [details]
patch to handle gpart add/modify automatically (thx to Andrey)
Comment 14 Ben RUBSON 2016-09-18 14:48:22 UTC
So, 2 patches attached.

The gpart label can be refreshed automatically (when they are added or modified), thanks to a patch by Andrey (many thanks Andrey !).

There is however a case where glabel refresh is needed.

Let's take this example (which is my use case) :

# gpart create -s gpt da0
# gpart add -t freebsd-zfs -s 2048G -a 1m -l mygptlabel da0
# glabel label myglabellabel da0p1

# cat /etc/ctl.conf
target iqn.2012-06.srv:t1 {
lun 0 {
  path /dev/gpt/mygptlabel
}
}

So as you can see /dev/gpt/mygptlabel is an iscsi target.
And this target contains a generic label "myglabellabel".

/dev/label/myglabellabel will show up on initiator side, perfect, this is what we want.
However, when ctld is stopped, /dev/label/myglabellabel will not show up again on target.
We then need to "glabel refresh" to make this label visible again.

Same issue if for example we add /dev/gpt/mygptlabel to as zpool.
/dev/label/myglabellabel will disappear because ZFS holds the disk, which is expected.
However, when we remove the disks from the zpool, /dev/label/myglabellabel will not show up again.
Here again we need to "glabel refresh" to make this label visible again.

I then think that both patches may be required.
The "glabel refresh" can handle all cases in a manual fashion.
The gpart patch can only handle gpart add/modify automatically.
Comment 15 Ben RUBSON 2016-11-08 07:36:24 UTC
Any news regarding these 2 patches ?
Could we think about merging them ?
Thank you very much !
Comment 16 Marcelo Araujo freebsd_committer freebsd_triage 2016-11-11 02:58:24 UTC
I'm taking it.
Comment 17 Andrey V. Elsukov freebsd_committer freebsd_triage 2016-11-11 05:26:25 UTC
(In reply to Marcelo Araujo from comment #16)
> I'm taking it.

Please note, that the patch I proposed should be carefully tested before committing and merging. It might have some side effects that are hard to find.
Comment 18 Ben RUBSON 2016-11-25 16:22:20 UTC
Created attachment 177389 [details]
patch to add refresh method to glabel

Also update glabel man page.
Comment 19 Ben RUBSON 2017-01-25 10:30:26 UTC
Hi,

Do you have some news regarding this please ? :)
I would have been glad to see these 2 patches (at least the "patch to add refresh method to glabel") in 11.1.

Many thanks again,

Ben
Comment 20 Marcelo Araujo freebsd_committer freebsd_triage 2017-03-09 06:59:28 UTC
(In reply to Ben RUBSON from comment #19)

OK, sorry my delay with it.
I'm making some tests with the patch and I got the following situation, I create and changed a label name:

1) Using glabel refresh and glabel list I get the correct new label.
root@pipoca:/tmp # glabel list md1p1
Geom name: md1p1
Providers:
1. Name: label/lalala

2) Using gpart list I still get the old label.
root@pipoca:/tmp # gpart list md1
Geom name: md1
modified: false
state: OK
fwheads: 32
fwsectors: 1
last: 20439
first: 40
entries: 152
scheme: GPT
Providers:
1. Name: md1p1
   Mediasize: 8388608 (8.0M)
   Sectorsize: 512
   Stripesize: 0
   Stripeoffset: 1048576
   Mode: r0w0e0
   rawuuid: 3c03eac6-0495-11e7-a52c-68f7285fdb52
   rawtype: 516e7cba-6ecf-11d6-8ff8-00022d09712b
   label: test

As we can see, with gpart list it shows the label named as 'test' and with glabel it shows as 'lalala'.

Any thoughts?
Comment 21 Marcelo Araujo freebsd_committer freebsd_triage 2017-03-09 07:03:15 UTC
(In reply to Marcelo Araujo from comment #20)


OK, I guess I have missed Andrey's patch, I'm rebuilding my kernel. :)
Comment 22 Ben RUBSON 2017-03-09 07:24:58 UTC
Many thanks Marcelo for looking at this ! :)

glabel refresh should work without Andrey patch.
Andrey patch adds an automatic way to refresh gpart(-only) labels.

Was your device opened while you were testing ?

Here is an example :

# gpart list da2
(...)
1. Name: da2p1
   label: ben

# gpart modify -i 1 -l newben da2
da2p1 modified

# gpart list da2
(...)
1. Name: da2p1
   label: newben

# ll /dev/gpt/
total 0
crw-r-----  1 root  operator  0xab  9 Mar 08:19 ben

# glabel refresh /dev/da2
Metadata from /dev/da2 refreshed.

# ll /dev/gpt/
total 0
crw-r-----  1 root  operator  0xdb  9 Mar 08:22 newben
Comment 23 Marcelo Araujo freebsd_committer freebsd_triage 2017-03-10 02:31:15 UTC
(In reply to Ben RUBSON from comment #22)

OK, I did open a review for the 'refresh' patch, and I will take a second look on Andrey's patch later today.

Both looks ok for me.
Comment 24 Ben RUBSON 2017-03-10 06:53:01 UTC
Perfect, many thx Marcelo !

Note that issue is exactly the same with labels in /dev/label.
glabel refresh then allows to reveal the news labels.
Comment 25 commit-hook freebsd_committer freebsd_triage 2017-03-12 04:16:53 UTC
A commit references this bug:

Author: araujo
Date: Sun Mar 12 04:15:56 UTC 2017
New revision: 315112
URL: https://svnweb.freebsd.org/changeset/base/315112

Log:
  Add the capability to refresh the gpart(8) label without need a reboot.

  gpart(8) has functionality to change the label of an GPT partition.
  This functionality works like it should, however, after a label change
  the /dev/gpt/ entries remain unchanged. glabel(8) status output remains
  unchanged. The change only takes effect after a reboot.

  PR:		162690
  Submitted by:	sub.mesa@gmail, Ben RUBSON <ben.rubson@gmail.com>, ae
  Reviewed by:	allanjude, bapt, bcr
  MFC after:	6 weeks.
  Differential Revision:	https://reviews.freebsd.org/D9935

Changes:
  head/sbin/geom/class/label/geom_label.c
  head/sbin/geom/class/label/glabel.8
  head/sys/geom/part/g_part.c
Comment 26 Marcelo Araujo freebsd_committer freebsd_triage 2017-03-12 04:17:45 UTC
Thank you all!
Comment 27 Ben RUBSON 2017-03-12 07:35:29 UTC
Thank you very much Marcelo for your work on this !
Comment 28 commit-hook freebsd_committer freebsd_triage 2017-05-17 05:21:28 UTC
A commit references this bug:

Author: araujo
Date: Wed May 17 05:21:04 UTC 2017
New revision: 318394
URL: https://svnweb.freebsd.org/changeset/base/318394

Log:
  MFC r315112, r315196

  r315112:
  Add the capability to refresh the gpart(8) label without need a reboot.

  gpart(8) has functionality to change the label of an GPT partition.
  This functionality works like it should, however, after a label change
  the /dev/gpt/ entries remain unchanged. glabel(8) status output remains
  unchanged. The change only takes effect after a reboot.

  PR:		162690
  Submitted by:	sub.mesa@gmail, Ben RUBSON <ben.rubson@gmail.com>, ae
  Reviewed by:	allanjude, bapt, bcr
  Differential Revision:	https://reviews.freebsd.org/D9935

  r315196:
  After r315112 I broke the tests with eli, instead to pass 0, I should pass
  M_NOWAIT to g_media_changed() that will call g_post_event() with this flag.

  Reported by:	lwhsu, ngie and ae

Changes:
_U  stable/11/
  stable/11/sbin/geom/class/label/geom_label.c
  stable/11/sbin/geom/class/label/glabel.8
  stable/11/sys/geom/part/g_part.c