Add libxo support to `gpart show` and document libxo flags in geom and gpart man pages

[guest-jsollvander]

Summary

Follow up on D53313 and https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=290629.

Added libxo support to gpart show, as I thought that could be somewhat handy for some people. I also updated the man pages for geom and gpart to show where you can expect libxo formatted output.

I initially considered adding libxo support to all subcommands for every class, but after going through each and every one of them, I changed my mind. As most classes' subcommands doesn't print anything unless there is an error, with the exception of the dump subcommand that some of them have. But I personally never use that one and thought the output wasn't something most people ever need to look at, so I decided against putting in the time and effort.

Example output:

root@johan_test:/usr/src # gpart show
=>      40  41942960  vtbd0  GPT  (20G)
        40    532480      1  efi  (260M)
    532520      2008         - free -  (1.0M)
    534528   1048576      2  freebsd-swap  (512M)
   1583104  40357888      3  freebsd-zfs  (19G)
  41940992      2008         - free -  (1.0M)
root@johan_test:/usr/src # gpart --libxo:JP show
{
  "__version": "1",
  "PART": [
    {
      "logical_starting_block": 40,
      "size_in_blocks": 41942960,
      "name": "vtbd0",
      "scheme": "GPT",
      "size": "20G",
      "state": "",
      "partitions": [
        {
          "logical_starting_block": 40,
          "size_in_blocks": 532480,
          "index": 1,
          "provider_name": "vtbd0p1",
          "type": "efi",
          "attribute": "",
          "size": "260M"
        },
        {
          "logical_starting_block": 532520,
          "size_in_blocks": 2008,
          "free": "true",
          "size": "1.0M"
        },
        {
          "logical_starting_block": 534528,
          "size_in_blocks": 1048576,
          "index": 2,
          "provider_name": "vtbd0p2",
          "type": "freebsd-swap",
          "attribute": "",
          "size": "512M"
        },
        {
          "logical_starting_block": 1583104,
          "size_in_blocks": 40357888,
          "index": 3,
          "provider_name": "vtbd0p3",
          "type": "freebsd-zfs",
          "attribute": "",
          "size": "19G"
        }
      ],
      "unallocated": {
        "logical_starting_block": 41940992,
        "size_in_blocks": 2008,
        "size": "1.0M"
      }
    }
  ]
}

kyua tests:

root@johan_test:/usr/tests/sys/geom/class/part # kyua report
===> Summary
Results read from /root/.kyua/store/results.usr_tests_sys_geom_class_part.20251127-163431-514005.db
Test cases: 5 total, 0 skipped, 0 expected failures, 0 broken, 0 failed
Total time: 0.350s

Just like everyone else, I think naming things are hard. So if you have name suggestions for the fields, please let me know.

Test Plan

Save output from gpart show <dev> before applying my changes to a file and compare the output after applying my changes.
Tests in sys/geom/class/part must pass.

[brd]

LGTM!

[asomers]

Nice job! I suggest a few changes, though.

• I suggest using the same field names that gpart list does, where possible.
  • logical_starting_block => start
  • size_in_blocks => sectors
  • provider_name => name
  • attribute => attrib (or better yet, change "gpart list" to show "attribute" instead of "attrib")
• When you do "gpart show -l --libxo=json", it will wrongly print something like "type": "swap0". That must be fixed. It should either show "label": "swap0", or just always have separate fields for "label" and "type".
• There's an asymmetry in the way that "gpart show --libxo=json" displays gaps in the middle of the partition table vs at the end. It only prints "free" if they're in the middle, and they're only in the "unallocated" section if they're at the end. I like the first way better. I think you should get rid of the "unallocated" section.
• Also, the json formatting for "attribute" isn't very machine-parseable. Right now it shows " [bootonce,bootme] ". I'm not sure what format would be best.
• Don't forget to bump the .Dd dates in the man pages

lib/geom/part/geom_part.c

673

Booleans should be unquoted in json. I think you need to add the "n" modifier to remove the quotes around "true".

682–684

Here and elsewhere, I think it would be good if the JSON-formatted output printed the size in bytes, not as a humanized string. libxo can do that, like this.

[ziaee]

lib/geom/part/geom_part.c

49

Don't we sort this too?

sbin/geom/core/geom.8

234

These need to be sorted lexographically by section, so 233m217.

You can check it in the linter with mandoc -Tlint /here/is/the/manual

I recommend removing libxo(3) here. xo_options(7) is what's relevant to geom, and xo_options(3) links to libxo(3) already. The SEE ALSO section gets bloated very easily.

[imp]

lib/geom/part/geom_part.c

49

No. Usually these go after the standard headers

[ziaee]

lib/geom/part/geom_part.c

49

Okay, thanks!

[guest-jsollvander]

In D53950#1235136, @asomers wrote:

Nice job! I suggest a few changes, though.

• I suggest using the same field names that gpart list does, where possible.
  • logical_starting_block => start
  • size_in_blocks => sectors
  • provider_name => name
  • attribute => attrib (or better yet, change "gpart list" to show "attribute" instead of "attrib")
• When you do "gpart show -l --libxo=json", it will wrongly print something like "type": "swap0". That must be fixed. It should either show "label": "swap0", or just always have separate fields for "label" and "type".
• There's an asymmetry in the way that "gpart show --libxo=json" displays gaps in the middle of the partition table vs at the end. It only prints "free" if they're in the middle, and they're only in the "unallocated" section if they're at the end. I like the first way better. I think you should get rid of the "unallocated" section.
• Also, the json formatting for "attribute" isn't very machine-parseable. Right now it shows " [bootonce,bootme] ". I'm not sure what format would be best.
• Don't forget to bump the .Dd dates in the man pages

Thanks Alan!

Could you explain why "type" is wrong here? According to the manpage for gpart show this field describes the "... the partition type, ...", so I thought "type" would be suitable field name for "element"? I'll still change it, I'm just curious.

Easy enough to fix.

Let me see what I can do about the attribute field.

[asomers]

In D53950#1235311, @guest-jsollvander wrote:

In D53950#1235136, @asomers wrote:

Nice job! I suggest a few changes, though.

• I suggest using the same field names that gpart list does, where possible.
  • logical_starting_block => start
  • size_in_blocks => sectors
  • provider_name => name
  • attribute => attrib (or better yet, change "gpart list" to show "attribute" instead of "attrib")
• When you do "gpart show -l --libxo=json", it will wrongly print something like "type": "swap0". That must be fixed. It should either show "label": "swap0", or just always have separate fields for "label" and "type".
• There's an asymmetry in the way that "gpart show --libxo=json" displays gaps in the middle of the partition table vs at the end. It only prints "free" if they're in the middle, and they're only in the "unallocated" section if they're at the end. I like the first way better. I think you should get rid of the "unallocated" section.
• Also, the json formatting for "attribute" isn't very machine-parseable. Right now it shows " [bootonce,bootme] ". I'm not sure what format would be best.
• Don't forget to bump the .Dd dates in the man pages

Thanks Alan!

Could you explain why "type" is wrong here? According to the manpage for gpart show this field describes the "... the partition type, ...", so I thought "type" would be suitable field name for "element"? I'll still change it, I'm just curious.

Easy enough to fix.

Let me see what I can do about the attribute field.

"type" is wrong because "swap0" isn't the partition type; it's the partition label. So the JSON should show some subset of "label": "swap0", "type": "freebsd-swap".

[guest-jsollvander]

This should address all the things. Here is the new sample output:

root@johan_test:/usr/src # gpart show
=>      40  41942960  vtbd0  GPT  (20G)
        40    532480      1  efi  [bootonce,bootme]  (260M)
    532520      2008         - free -  (1004K)
    534528   1048576      2  freebsd-swap  (512M)
   1583104  40357888      3  freebsd-zfs  (19G)
  41940992      2008         - free -  (1004K)

root@johan_test:/usr/src # gpart --libxo:JP,warn show
{
  "__version": "1",
  "PART": [
    {
      "start": 40,
      "sectors": 41942960,
      "name": "vtbd0",
      "scheme": "GPT",
      "size": 21474836480,
      "state": "",
      "partitions": [
        {
          "start": 40,
          "sectors": 532480,
          "index": 1,
          "name": "vtbd0p1",
          "type": "efi",
          "label": "efiboot0",
          "rawtype": "c12a7328-f81f-11d2-ba4b-00a0c93ec93b",
          "attribute": [
            "bootonce",
            "bootme"
          ],
          "size": 272629760
        },
        {
          "start": 532520,
          "sectors": 2008,
          "free": true,
          "size": 1028096
        },
        {
          "start": 534528,
          "sectors": 1048576,
          "index": 2,
          "name": "vtbd0p2",
          "type": "freebsd-swap",
          "label": "swap0",
          "rawtype": "516e7cb5-6ecf-11d6-8ff8-00022d09712b",
          "size": 536870912
        },
        {
          "start": 1583104,
          "sectors": 40357888,
          "index": 3,
          "name": "vtbd0p3",
          "type": "freebsd-zfs",
          "label": "zfs0",
          "rawtype": "516e7cba-6ecf-11d6-8ff8-00022d09712b",
          "size": 20663238656
        },
        {
          "start": 41940992,
          "sectors": 2008,
          "free": true,
          "size": 1028096
        }
      ]
    }
  ]
}

[asomers]

Nice! It almost LGTM , but I agree with @ziaee that you should remove the man page link to libxo(3).

lib/geom/part/Makefile

5

Did you mean to remove xo from LIBADD? Remember, when using the arc command line tool, you must include all files in every update to the review, even if the update didn't touch all files. Otherwise, arc thinks that you want to revert a change to any omitted file.

[guest-jsollvander]

In D53950#1235417, @asomers wrote:

Nice! It almost LGTM , but I agree with @ziaee that you should remove the man page link to libxo(3).

Hmm, I thought I did? Let me fix that.

lib/geom/part/Makefile

5

I didn't remove it? And I'm not using arc.

[guest-jsollvander]

Re-uploaded all my changes.

[ziaee]

lib/geom/part/gpart.8

1529–1530

[guest-jsollvander]

lib/geom/part/gpart.8

1529–1530

Did I miss something you wanted me to fix here?

[ziaee]

lib/geom/part/gpart.8

1529–1530

Yes I suggested adding xo_options(7) to the see also section of gpart(8).

[guest-jsollvander]

Added xo_options(7) to gpart manpage.

Also, I saw this that should be fixed:

root@johan_test:/usr/src # mandoc -Tlint lib/geom/part/gpart.8
mandoc: lib/geom/part/gpart.8:1524:6: STYLE: referenced manual not found: Xr zfsprops 8

I can do it in this PR now if we want

[mckusick]

This LGTM. I will commit it Dec 9th if no objections are raised.

[phil]

Looks good. Thanks....

lib/geom/part/geom_part.c

724

"ne:free" uses the "e" modifier, which means "only emit this field for encoding style", meaning xml, json, etc. The original string had this is static text, so that's probably not what you want. "d" would mean "display only" (text, html) but I'm not sure that's needed. Just "{n:free}" should be fine.

[guest-jsollvander]

lib/geom/part/geom_part.c

724

"ne" here is intentional. For encoding styles I want it to say "free": true and the normal text output can use the original static text.