Skip to content
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

feat(lib): Use lsblk directly, parse json output #255

Merged
merged 3 commits into from
Jul 9, 2018

Conversation

jhermsmeier
Copy link
Contributor

@jhermsmeier jhermsmeier commented Mar 9, 2018

This switches from using a bash script on linux to shelling out
to lsblk directly, using its --json option to obtain & parse
machine-readable output – removing the need for external script
files entirely.

Change-Type: minor
Connects To: #253, #231, #165, balena-io/etcher#2122

@jhermsmeier
Copy link
Contributor Author

Updated to parse the key / value pairs that can be obtained through the --pairs option, should the --json option result in an error, or not be supported.

Copy link
Contributor

@jviotti jviotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments, but overall looks good, and it works fine on macOS

exec('diskutil', [ 'info', '-plist', devicePath ], (error, stdout) => {
let data = null;
try {
data = plist.parse(stdout);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if exec yields back an error? We will still attempt to parse stdout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as diskutil may output error descriptions in plist format to stdout

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raw: device.kname,
description: getDescription(device),
error: null,
size: Number(device.size),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if these evaluate to NaN? Should we handle this somehow and make them undefined or something like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defaulted to null now, although that shouldn't happen.

logicalBlockSize: Number(device['log-sec']),
mountpoints: device.children ? getMountpoints(device.children) : [],
isReadOnly: isReadOnly,
isSystem: !isRemovable && !isVirtual,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can a system drive be virtual?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure; there might be contraptions where a system device could be virtual (apfs containers, encrypted volumes), not sure if they'd be labeled as virtual though; need to test.

@resin-io-modules-versionbot
Copy link
Contributor

@jhermsmeier, status checks have failed for this PR. Please make appropriate changes and recommit.

@lurch
Copy link
Contributor

lurch commented Jun 18, 2018

Just as a heads-up, this appears to be non-functional on Ubuntu 14.04 :-(

With the master branch of this repo, node example/index.js gives me:

[ { enumerator: 'lsblk',
    busType: 'UNKNOWN',
    busVersion: '0.0',
    device: '/dev/sda',
    devicePath: null,
    raw: '/dev/sda',
    description: 'SanDisk SD8SN8U1',
    error: null,
    size: 1024209543168,
    blockSize: null,
    logicalBlockSize: null,
    mountpoints: 
     [ { path: '/' },
       { path: '/boot/efi' },
       { path: '/home' },
       { path: '/var/lib/docker/aufs' } ],
    isReadOnly: false,
    isSystem: true,
    isVirtual: null,
    isRemovable: null,
    isCard: null,
    isSCSI: null,
    isUSB: null,
    isUAS: null } ]

but on the lsblk-json branch of this repo, node example/index.js gives me:

[]

If it helps, this is the output of lsblk --bytes --pairs --all:

NAME="sda" MAJ:MIN="8:0" RM="0" SIZE="1024209543168" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="sda1" MAJ:MIN="8:1" RM="0" SIZE="524288000" RO="0" TYPE="part" MOUNTPOINT="/boot/efi"
NAME="sda2" MAJ:MIN="8:2" RM="0" SIZE="41943040" RO="0" TYPE="part" MOUNTPOINT=""
NAME="sda3" MAJ:MIN="8:3" RM="0" SIZE="3221225472" RO="0" TYPE="part" MOUNTPOINT=""
NAME="sda4" MAJ:MIN="8:4" RM="0" SIZE="235291017216" RO="0" TYPE="part" MOUNTPOINT="/"
NAME="sda5" MAJ:MIN="8:5" RM="0" SIZE="16980639744" RO="0" TYPE="part" MOUNTPOINT="[SWAP]"
NAME="sda6" MAJ:MIN="8:6" RM="0" SIZE="549755813888" RO="0" TYPE="part" MOUNTPOINT="/home"
NAME="sda7" MAJ:MIN="8:7" RM="0" SIZE="218392166400" RO="0" TYPE="part" MOUNTPOINT=""
NAME="ram0" MAJ:MIN="1:0" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram1" MAJ:MIN="1:1" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram2" MAJ:MIN="1:2" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram3" MAJ:MIN="1:3" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram4" MAJ:MIN="1:4" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram5" MAJ:MIN="1:5" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram6" MAJ:MIN="1:6" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram7" MAJ:MIN="1:7" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram8" MAJ:MIN="1:8" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram9" MAJ:MIN="1:9" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="loop0" MAJ:MIN="7:0" RM="0" SIZE="" RO="0" TYPE="loop" MOUNTPOINT=""
NAME="loop1" MAJ:MIN="7:1" RM="0" SIZE="" RO="0" TYPE="loop" MOUNTPOINT=""
NAME="loop2" MAJ:MIN="7:2" RM="0" SIZE="" RO="0" TYPE="loop" MOUNTPOINT=""
NAME="loop3" MAJ:MIN="7:3" RM="0" SIZE="" RO="0" TYPE="loop" MOUNTPOINT=""
NAME="loop4" MAJ:MIN="7:4" RM="0" SIZE="" RO="0" TYPE="loop" MOUNTPOINT=""
NAME="loop5" MAJ:MIN="7:5" RM="0" SIZE="" RO="0" TYPE="loop" MOUNTPOINT=""
NAME="loop6" MAJ:MIN="7:6" RM="0" SIZE="" RO="0" TYPE="loop" MOUNTPOINT=""
NAME="loop7" MAJ:MIN="7:7" RM="0" SIZE="" RO="0" TYPE="loop" MOUNTPOINT=""
NAME="ram10" MAJ:MIN="1:10" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram11" MAJ:MIN="1:11" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram12" MAJ:MIN="1:12" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram13" MAJ:MIN="1:13" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram14" MAJ:MIN="1:14" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""
NAME="ram15" MAJ:MIN="1:15" RM="0" SIZE="67108864" RO="0" TYPE="disk" MOUNTPOINT=""

and this is the output of lsblk --help:

Usage:
 lsblk [options] [<device> ...]

Options:
 -a, --all            print all devices
 -b, --bytes          print SIZE in bytes rather than in human readable format
 -d, --nodeps         don't print slaves or holders
 -D, --discard        print discard capabilities
 -e, --exclude <list> exclude devices by major number (default: RAM disks)
 -f, --fs             output info about filesystems
 -h, --help           usage information (this)
 -i, --ascii          use ascii characters only
 -m, --perms          output info about permissions
 -l, --list           use list format ouput
 -n, --noheadings     don't print headings
 -o, --output <list>  output columns
 -P, --pairs          use key="value" output format
 -r, --raw            use raw output format
 -t, --topology       output info about topology

Available columns:
       NAME  device name
      KNAME  internal kernel device name
    MAJ:MIN  major:minor device number
     FSTYPE  filesystem type
 MOUNTPOINT  where the device is mounted
      LABEL  filesystem LABEL
       UUID  filesystem UUID
         RO  read-only device
         RM  removable device
      MODEL  device identifier
       SIZE  size of the device
      STATE  state of the device
      OWNER  user name
      GROUP  group name
       MODE  device node permissions
  ALIGNMENT  alignment offset
     MIN-IO  minimum I/O size
     OPT-IO  optimal I/O size
    PHY-SEC  physical sector size
    LOG-SEC  logical sector size
       ROTA  rotational device
      SCHED  I/O scheduler name
    RQ-SIZE  request queue size
       TYPE  device type
   DISC-ALN  discard alignment offset
  DISC-GRAN  discard granularity
   DISC-MAX  discard max bytes
  DISC-ZERO  discard zeroes data

For more information see lsblk(8).

@jhermsmeier
Copy link
Contributor Author

@lurch what does lsblk --version output for you?

@lurch
Copy link
Contributor

lurch commented Jun 20, 2018

what does lsblk --version output for you?

lsblk: unrecognised option '--version'

😉

However doing man lsblk I see it says "The lsblk command is part of the util-linux package" and doing dpkg -l util-linux it tells me:

Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name           Version      Architecture Description
+++-==============-============-============-=================================
ii  util-linux     2.20.1-5.1ub amd64        Miscellaneous system utilities

See also https://packages.ubuntu.com/trusty-updates/util-linux

@jhermsmeier jhermsmeier force-pushed the lsblk-json branch 2 times, most recently from 4bb4d3f to 7397ee1 Compare July 5, 2018 23:38
This switches from using a bash script on linux to shelling out
to `lsblk` directly, using its `--json` option to obtain & parse
machine-readable output – removing the need for external script
files entirely.

Change-Type: minor
@jhermsmeier jhermsmeier removed the request for review from Shou July 6, 2018 20:33
@jhermsmeier jhermsmeier requested review from zvin, jviotti and curcuz July 6, 2018 20:33
@jhermsmeier
Copy link
Contributor Author

Thanks @lurch – that's fixed now; was a bug in how disks & their child partitions were mapped to each other.

@resin-io-modules-versionbot
Copy link
Contributor

@jhermsmeier, status checks have failed for this PR. Please make appropriate changes and recommit.

@jhermsmeier jhermsmeier force-pushed the lsblk-json branch 3 times, most recently from 6aa13a8 to cb93b23 Compare July 6, 2018 21:17
@resin-io-modules-versionbot
Copy link
Contributor

@jhermsmeier, status checks have failed for this PR. Please make appropriate changes and recommit.

This fixes consolidation & mapping of children on systems
with an old version of `lsblk` (like Ubuntu 14.04), where only
the `--pairs` option is available and no `pkname` or `kname`
fields are exposed in the output.

Change-Type: patch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants