gadget, osutil/disks: fix some bugs from prior PR's

Merge pull request #10905 from anonymouse64/feature/uc20-multi-volume-gadget-asset-updates-10.5-whoopsies Fix a couple mistakes from previous PR's that were only evident on an end-to-end test I did again with multi-volume stuff. It remains to be seen if the udev properties change to get partitions will actually pose a problem for us, I think it is only a problem in the uc20-create-partitions test when we start actually using disks.DiskFromDevicePath from gadget/install.Run(), but I think we can cross that bridge when we get there. The other problem is that we actually need to write the mapping file from install mode to boot.InstallHostWritableDir, which we can't import from gadget package, so instead pass in the dir. Example usage now from the gadget/install package eventually:	if err := gadget.SaveDiskVolumesDeviceTraits(dirs.SnapDeviceDirUnder(boot.InstallHostWritableDir), allVols); err != nil {	return nil, fmt.Errorf("cannot save disk to volume device traits: %v", err)	}

4 files changed, 77 insertions, 26 deletions

diff --git a/gadget/gadget.go b/gadget/gadget.go
index bb550ba517..a0da7a8830 100644
--- a/gadget/gadget.go
+++ b/gadget/gadget.go
@@ -35,7 +35,6 @@ import (
	"gopkg.in/yaml.v2"
 
	"github.com/snapcore/snapd/asserts"
-	"github.com/snapcore/snapd/dirs"
	"github.com/snapcore/snapd/gadget/edition"
	"github.com/snapcore/snapd/gadget/quantity"
	"github.com/snapcore/snapd/metautil"
@@ -272,16 +271,16 @@ type DiskStructureDeviceTraits struct {
	Size quantity.Size `json:"size"`
 }
 
-// SaveDiskVolumesDeviceTraits saves the mapping of volume names to volume / device
-// traits to a file on disk for later loading and verification.
-func SaveDiskVolumesDeviceTraits(mapping map[string]DiskVolumeDeviceTraits) error {
+// SaveDiskVolumesDeviceTraits saves the mapping of volume names to volume /
+// device traits to a file inside the provided directory on disk for
+// later loading and verification.
+func SaveDiskVolumesDeviceTraits(dir string, mapping map[string]DiskVolumeDeviceTraits) error {
	b, err := json.Marshal(mapping)
	if err != nil {
	return err
	}
 
-	// TODO: should this live in dirs?
-	filename := filepath.Join(dirs.SnapDeviceDir, "disk-mapping.json")
+	filename := filepath.Join(dir, "disk-mapping.json")
 
	if err := os.MkdirAll(filepath.Dir(filename), 0755); err != nil {
	return err
@@ -292,10 +291,10 @@ func SaveDiskVolumesDeviceTraits(mapping map[string]DiskVolumeDeviceTraits) erro
 // LoadDiskVolumesDeviceTraits loads the mapping of volumes to disk traits if
 // there is any. If there is no file with the mapping available, nil is
 // returned.
-func LoadDiskVolumesDeviceTraits() (map[string]DiskVolumeDeviceTraits, error) {
+func LoadDiskVolumesDeviceTraits(dir string) (map[string]DiskVolumeDeviceTraits, error) {
	var mapping map[string]DiskVolumeDeviceTraits
 
-	filename := filepath.Join(dirs.SnapDeviceDir, "disk-mapping.json")
+	filename := filepath.Join(dir, "disk-mapping.json")
	if !osutil.FileExists(filename) {
	return nil, nil
	}
diff --git a/gadget/gadget_test.go b/gadget/gadget_test.go
index c54f0c730f..a8817b34da 100644
--- a/gadget/gadget_test.go
+++ b/gadget/gadget_test.go
@@ -3034,14 +3034,22 @@ func (s *gadgetYamlTestSuite) TestSaveLoadDiskVolumeDeviceTraits(c *C) {
 
	// when there is no mapping file, it is not an error, the map returned is
	// just nil/has no items in it
-	mAbsent, err := gadget.LoadDiskVolumesDeviceTraits()
+	mAbsent, err := gadget.LoadDiskVolumesDeviceTraits(dirs.SnapDeviceDir)
	c.Assert(err, IsNil)
	c.Assert(mAbsent, HasLen, 0)
 
-	err = gadget.SaveDiskVolumesDeviceTraits(m)
+	// load looks in SnapDeviceDir since it is meant to be used during run mode
+	// when /var/lib/snapd/device/disk-mapping.json is the real version from
+	// ubuntu-data, but during install mode, we will need to save to the host
+	// ubuntu-data which is not located at /run/mnt/data or
+	// /var/lib/snapd/device, but rather
+	// /run/mnt/ubuntu-data/system-data/var/lib/snapd/device so this takes a
+	// directory argument when we save it
+	err = gadget.SaveDiskVolumesDeviceTraits(dirs.SnapDeviceDir, m)
	c.Assert(err, IsNil)
 
-	m2, err := gadget.LoadDiskVolumesDeviceTraits()
+	// now that it was saved to dirs.SnapDeviceDir, we can load it correctly
+	m2, err := gadget.LoadDiskVolumesDeviceTraits(dirs.SnapDeviceDir)
	c.Assert(err, IsNil)
 
	c.Assert(m, DeepEquals, m2)
diff --git a/osutil/disks/disks_linux.go b/osutil/disks/disks_linux.go
index 31f6f68e9a..09142bddf4 100644
--- a/osutil/disks/disks_linux.go
+++ b/osutil/disks/disks_linux.go
@@ -144,11 +144,20 @@ func diskFromUdevProps(deviceIdentifier string, deviceIDType string, props map[s
	// create the full path by pre-pending /sys, since udev doesn't include /sys
	devpath = filepath.Join(dirs.SysfsDir, devpath)
 
+	// check if the device has partitions by attempting to actually search for
+	// them in /sys with the DEVPATH and DEVNAME
+
+	paths, err := filepath.Glob(filepath.Join(devpath, filepath.Base(devname)+"*"))
+	if err != nil {
+	return nil, fmt.Errorf("internal error with glob pattern: %v", err)
+	}
+
	return &disk{
-	major: major,
-	minor: minor,
-	devname: devname,
-	devpath: devpath,
+	major: major,
+	minor: minor,
+	devname: devname,
+	devpath: devpath,
+	hasPartitions: len(paths) != 0,
	}, nil
 }
 
diff --git a/osutil/disks/disks_linux_test.go b/osutil/disks/disks_linux_test.go
index f12a5dcd6c..fb8f5f8170 100644
--- a/osutil/disks/disks_linux_test.go
+++ b/osutil/disks/disks_linux_test.go
@@ -89,8 +89,11 @@ var (
	}
 )
 
-func createVirtioDevicesInSysfs(c *C, devsToPartition map[string]bool) {
-	diskDir := filepath.Join(dirs.SysfsDir, virtioDiskDevPath)
+func createVirtioDevicesInSysfs(c *C, path string, devsToPartition map[string]bool) {
+	if path == "" {
+	path = virtioDiskDevPath
+	}
+	diskDir := filepath.Join(dirs.SysfsDir, path)
	for dev, isPartition := range devsToPartition {
	err := os.MkdirAll(filepath.Join(diskDir, dev), 0755)
	c.Assert(err, IsNil)
@@ -111,7 +114,7 @@ func (s *diskSuite) SetUpTest(c *C) {
	dirs.SetRootDir(c.MkDir())
 }
 
-func (s *diskSuite) TestDiskFromNameHappy(c *C) {
+func (s *diskSuite) TestDiskFromDeviceNameHappy(c *C) {
	const sdaSysfsPath = "/devices/pci0000:00/0000:00:01.1/0000:01:00.1/ata1/host0/target0:0:0/0:0:0:0/block/sda"
	restore := disks.MockUdevPropertiesForDevice(func(typeOpt, dev string) (map[string]string, error) {
	c.Assert(typeOpt, Equals, "--name")
@@ -131,13 +134,29 @@ func (s *diskSuite) TestDiskFromNameHappy(c *C) {
	c.Assert(d.Dev(), Equals, "1:2")
	c.Assert(d.KernelDeviceNode(), Equals, "/dev/sda")
	c.Assert(d.KernelDevicePath(), Equals, filepath.Join(dirs.SysfsDir, sdaSysfsPath))
+	// it doesn't have any partitions since we didn't mock any in sysfs
+	c.Assert(d.HasPartitions(), Equals, false)
+
+	// if we mock some sysfs partitions then it has partitions when we it has
+	// some partitions on it it
+	createVirtioDevicesInSysfs(c, sdaSysfsPath, map[string]bool{
+	"sda1": true,
+	"sda2": true,
+	})
+
+	d, err = disks.DiskFromDeviceName("sda")
+	c.Assert(err, IsNil)
+	c.Assert(d.Dev(), Equals, "1:2")
+	c.Assert(d.KernelDeviceNode(), Equals, "/dev/sda")
+	c.Assert(d.HasPartitions(), Equals, true)
 }
 
-func (s *diskSuite) TestDiskFromPathHappy(c *C) {
+func (s *diskSuite) TestDiskFromDevicePathHappy(c *C) {
	const vdaSysfsPath = "/devices/pci0000:00/0000:00:04.0/virtio2/block/vdb"
+	fullSysPath := filepath.Join("/sys", vdaSysfsPath)
	restore := disks.MockUdevPropertiesForDevice(func(typeOpt, dev string) (map[string]string, error) {
	c.Assert(typeOpt, Equals, "--path")
-	c.Assert(dev, Equals, filepath.Join("/sys", vdaSysfsPath))
+	c.Assert(dev, Equals, fullSysPath)
	return map[string]string{
	"MAJOR": "1",
	"MINOR": "2",
@@ -148,15 +167,31 @@ func (s *diskSuite) TestDiskFromPathHappy(c *C) {
	})
	defer restore()
 
-	d, err := disks.DiskFromDevicePath(filepath.Join("/sys", vdaSysfsPath))
+	d, err := disks.DiskFromDevicePath(fullSysPath)
	c.Assert(err, IsNil)
	c.Assert(d.Dev(), Equals, "1:2")
	c.Assert(d.KernelDeviceNode(), Equals, "/dev/vdb")
	// note that we don't always prepend exactly /sys, we use dirs.SysfsDir
	c.Assert(d.KernelDevicePath(), Equals, filepath.Join(dirs.SysfsDir, vdaSysfsPath))
+
+	// it doesn't have any partitions since we didn't mock any in sysfs
+	c.Assert(d.HasPartitions(), Equals, false)
+
+	// if we mock some sysfs partitions then it has partitions when we it has
+	// some partitions on it it
+	createVirtioDevicesInSysfs(c, vdaSysfsPath, map[string]bool{
+	"vdb1": true,
+	"vdb2": true,
+	})
+
+	d, err = disks.DiskFromDevicePath(fullSysPath)
+	c.Assert(err, IsNil)
+	c.Assert(d.Dev(), Equals, "1:2")
+	c.Assert(d.KernelDeviceNode(), Equals, "/dev/vdb")
+	c.Assert(d.HasPartitions(), Equals, true)
 }
 
-func (s *diskSuite) TestDiskFromNameUnhappyPartition(c *C) {
+func (s *diskSuite) TestDiskFromDeviceNameUnhappyPartition(c *C) {
	restore := disks.MockUdevPropertiesForDevice(func(typeOpt, dev string) (map[string]string, error) {
	c.Assert(typeOpt, Equals, "--name")
	c.Assert(dev, Equals, "sda1")
@@ -172,7 +207,7 @@ func (s *diskSuite) TestDiskFromNameUnhappyPartition(c *C) {
	c.Assert(err, ErrorMatches, "device \"sda1\" is not a disk, it has DEVTYPE of \"partition\"")
 }
 
-func (s *diskSuite) TestDiskFromNameUnhappyBadUdevOutput(c *C) {
+func (s *diskSuite) TestDiskFromDeviceNameUnhappyBadUdevOutput(c *C) {
	restore := disks.MockUdevPropertiesForDevice(func(typeOpt, dev string) (map[string]string, error) {
	c.Assert(typeOpt, Equals, "--name")
	c.Assert(dev, Equals, "sda")
@@ -346,7 +381,7 @@ func (s *diskSuite) TestDiskFromMountPointHappySinglePartitionIgnoresNonPartitio
 
	// create just the single valid partition in sysfs, and an invalid
	// non-partition device that we should ignore
-	createVirtioDevicesInSysfs(c, map[string]bool{
+	createVirtioDevicesInSysfs(c, "", map[string]bool{
	"vda4": true,
	"vda5": false,
	})
@@ -601,7 +636,7 @@ func (s *diskSuite) TestDiskFromMountPointPartitionsHappy(c *C) {
	defer restore()
 
	// create all 4 partitions as device nodes in sysfs
-	createVirtioDevicesInSysfs(c, map[string]bool{
+	createVirtioDevicesInSysfs(c, "", map[string]bool{
	"vda1": true,
	"vda2": true,
	"vda3": true,
@@ -833,7 +868,7 @@ func (s *diskSuite) TestDiskFromMountPointDecryptedDevicePartitionsHappy(c *C) {
	c.Assert(err, IsNil)
 
	// mock the dev nodes in sysfs for the partitions
-	createVirtioDevicesInSysfs(c, map[string]bool{
+	createVirtioDevicesInSysfs(c, "", map[string]bool{
	"vda1": true,
	"vda2": true,
	"vda3": true,