Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

Commit 8b74066

Browse files
egernstegernst
committed
volumes: cleanup, minimal refactoring
Update some headers, very minor refactoring Signed-off-by: Eric Ernst <[email protected]>
1 parent cf32518 commit 8b74066

File tree

5 files changed

+103
-54
lines changed

5 files changed

+103
-54
lines changed

‎virtcontainers/container.go‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, hostMountDir, gu
499499
}
500500

501501
// mountSharedDirMounts handles bind-mounts by bindmounting to the host shared
502-
// directory which is mounted through 9pfs in the VM.
502+
// directory which is mounted through virtiofs/9pfs in the VM.
503503
// It also updates the container mount list with the HostPath info, and store
504504
// container mounts to the storage. This way, we will have the HostPath info
505505
// available when we will need to unmount those mounts.
@@ -522,6 +522,18 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, hostMountDir, guestShare
522522
continue
523523
}
524524

525+
// Check if mount is a block device file. If it is, the block device will be attached to the host
526+
// instead of passing this as a shared mount:
527+
if len(m.BlockDeviceID) > 0 {
528+
// Attach this block device, all other devices passed in the config have been attached at this point
529+
if err = c.sandbox.devManager.AttachDevice(m.BlockDeviceID, c.sandbox); err != nil {
530+
return nil, nil, err
531+
}
532+
devicesToDetach = append(devicesToDetach, m.BlockDeviceID)
533+
continue
534+
}
535+
536+
// For non-block based mounts, we are only interested in bind mounts
525537
if m.Type != "bind" {
526538
continue
527539
}
@@ -533,17 +545,6 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, hostMountDir, guestShare
533545
continue
534546
}
535547

536-
// Check if mount is a block device file. If it is, the block device will be attached to the host
537-
// instead of passing this as a shared mount.
538-
if len(m.BlockDeviceID) > 0 {
539-
// Attach this block device, all other devices passed in the config have been attached at this point
540-
if err = c.sandbox.devManager.AttachDevice(m.BlockDeviceID, c.sandbox); err != nil {
541-
return nil, nil, err
542-
}
543-
devicesToDetach = append(devicesToDetach, m.BlockDeviceID)
544-
continue
545-
}
546-
547548
// Ignore /dev, directories and all other device files. We handle
548549
// only regular files in /dev. It does not make sense to pass the host
549550
// device nodes to the guest.
@@ -639,6 +640,9 @@ func filterDevices(c *Container, devices []ContainerDevice) (ret []ContainerDevi
639640
return
640641
}
641642

643+
// Add any mount based block devices to the device manager and save the
644+
// device ID for the particular mount. This'll occur when the mountpoint source
645+
// is a block device.
642646
func (c *Container) createBlockDevices() error {
643647
if !c.checkBlockDeviceSupport() {
644648
c.Logger().Warn("Block device not supported")
@@ -647,13 +651,18 @@ func (c *Container) createBlockDevices() error {
647651

648652
// iterate all mounts and create block device if it's block based.
649653
for i, m := range c.mounts {
650-
if len(m.BlockDeviceID) > 0 || m.Type != "bind" {
654+
if len(m.BlockDeviceID) > 0 {
651655
// Non-empty m.BlockDeviceID indicates there's already one device
652656
// associated with the mount,so no need to create a new device for it
653657
// and we only create block device for bind mount
654658
continue
655659
}
656660

661+
if m.Type != "bind" {
662+
// We only handle for bind-mounts
663+
continue
664+
}
665+
657666
var stat unix.Stat_t
658667
if err := unix.Stat(m.Source, &stat); err != nil {
659668
return fmt.Errorf("stat %q failed: %v", m.Source, err)
@@ -681,7 +690,6 @@ func (c *Container) createBlockDevices() error {
681690

682691
if err == nil && di != nil {
683692
b, err := c.sandbox.devManager.NewDevice(*di)
684-
685693
if err != nil {
686694
// Do not return an error, try to create
687695
// devices for other mounts
@@ -750,11 +758,12 @@ func newContainer(sandbox *Sandbox, contConfig *ContainerConfig) (*Container, er
750758
}
751759
}
752760

753-
// Go to next step for first created container
761+
// If mounts are block devices, add to devmanager
754762
if err := c.createMounts(); err != nil {
755763
return nil, err
756764
}
757765

766+
// Add container's devices to sandbox's device-manager
758767
if err := c.createDevices(contConfig); err != nil {
759768
return nil, err
760769
}
@@ -792,11 +801,7 @@ func (c *Container) createMounts() error {
792801
}
793802

794803
// Create block devices for newly created container
795-
if err := c.createBlockDevices(); err != nil {
796-
return err
797-
}
798-
799-
return nil
804+
return c.createBlockDevices()
800805
}
801806

802807
func (c *Container) createDevices(contConfig *ContainerConfig) error {
@@ -878,6 +883,7 @@ func (c *Container) create() (err error) {
878883
}()
879884

880885
if c.checkBlockDeviceSupport() {
886+
// If the rootfs is backed by a block device, go ahead and hotplug it to the guest
881887
if err = c.hotplugDrive(); err != nil {
882888
return
883889
}
@@ -1315,11 +1321,14 @@ func (c *Container) resume() error {
13151321
return c.setContainerState(types.StateRunning)
13161322
}
13171323

1324+
// hotplugDrive will attempt to hotplug the container rootfs if it is backed by a
1325+
// block device
13181326
func (c *Container) hotplugDrive() error {
13191327
var dev device
13201328
var err error
13211329

1322-
// container rootfs is blockdevice backed and isn't mounted
1330+
// Check to see if the rootfs is an umounted block device (source) or if the
1331+
// mount (target) is backed by a block device:
13231332
if !c.rootFs.Mounted {
13241333
dev, err = getDeviceForPath(c.rootFs.Source)
13251334
// there is no "rootfs" dir on block device backed rootfs
@@ -1381,6 +1390,7 @@ func (c *Container) hotplugDrive() error {
13811390
return c.setStateFstype(fsType)
13821391
}
13831392

1393+
// plugDevice will attach the rootfs if blockdevice is supported (this is rootfs specific)
13841394
func (c *Container) plugDevice(devicePath string) error {
13851395
var stat unix.Stat_t
13861396
if err := unix.Stat(devicePath, &stat); err != nil {

‎virtcontainers/kata_agent.go‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,7 +1333,6 @@ func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPat
13331333
}
13341334

13351335
case sandbox.config.HypervisorConfig.BlockDeviceDriver == config.VirtioSCSI:
1336-
13371336
rootfs.Driver = kataSCSIDevType
13381337
rootfs.Source = blockDrive.SCSIAddr
13391338
default:
@@ -1348,22 +1347,19 @@ func (k *kataAgent) buildContainerRootfs(sandbox *Sandbox, c *Container, rootPat
13481347
}
13491348

13501349
// Ensure container mount destination exists
1351-
// TODO: remove dependency on shared fs path. shared fs is just one kind of storage sources.
1352-
// we should not always use shared fs path for all kinds of storage. Stead, all storage
1350+
// TODO: remove dependency on shared fs path. shared fs is just one kind of storage source.
1351+
// we should not always use shared fs path for all kinds of storage. Instead, all storage
13531352
// should be bind mounted to a tmpfs path for containers to use.
13541353
if err := os.MkdirAll(filepath.Join(getMountPath(c.sandbox.id), c.id, c.rootfsSuffix), DirMode); err != nil {
13551354
return nil, err
13561355
}
13571356
return rootfs, nil
13581357
}
13591358

1360-
// This is not a block based device rootfs.
1361-
// We are going to bind mount it into the 9pfs
1362-
// shared drive between the host and the guest.
1363-
// With 9pfs we don't need to ask the agent to
1364-
// mount the rootfs as the shared directory
1365-
// (kataGuestSharedDir) is already mounted in the
1366-
// guest. We only need to mount the rootfs from
1359+
// This is not a block based device rootfs. We are going to bind mount it into the shared drive
1360+
// between the host and the guest.
1361+
// With virtiofs/9pfs we don't need to ask the agent to mount the rootfs as the shared directory
1362+
// (kataGuestSharedDir) is already mounted in the guest. We only need to mount the rootfs from
13671363
// the host and it will show up in the guest.
13681364
if err := bindMountContainerRootfs(k.ctx, getMountPath(sandbox.id), c.id, c.rootFs.Target, false); err != nil {
13691365
return nil, err
@@ -1403,9 +1399,14 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,
14031399
}
14041400
}()
14051401

1402+
// setup rootfs -- if its block based, we'll receive a non-nil storage object representing
1403+
// the block device for the rootfs, which us utilized for mounting in the guest. This'll be handled
1404+
// already for non-block based rootfs
14061405
if rootfs, err = k.buildContainerRootfs(sandbox, c, rootPathParent); err != nil {
14071406
return nil, err
1408-
} else if rootfs != nil {
1407+
}
1408+
1409+
if rootfs != nil {
14091410
// Add rootfs to the list of container storage.
14101411
// We only need to do this for block based rootfs, as we
14111412
// want the agent to mount it into the right location
@@ -1454,6 +1455,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,
14541455
if err != nil {
14551456
return nil, err
14561457
}
1458+
14571459
if err := k.replaceOCIMountsForStorages(ociSpec, volumeStorages); err != nil {
14581460
return nil, err
14591461
}
@@ -1580,7 +1582,7 @@ func (k *kataAgent) handleLocalStorage(mounts []specs.Mount, sandboxID string, r
15801582

15811583
// handleDeviceBlockVolume handles volume that is block device file
15821584
// and DeviceBlock type.
1583-
func (k *kataAgent) handleDeviceBlockVolume(c *Container, device api.Device) (*grpc.Storage, error) {
1585+
func (k *kataAgent) handleDeviceBlockVolume(c *Container, m Mount, device api.Device) (*grpc.Storage, error) {
15841586
vol := &grpc.Storage{}
15851587

15861588
blockDrive, ok := device.GetDeviceInfo().(*config.BlockDrive)
@@ -1615,12 +1617,22 @@ func (k *kataAgent) handleDeviceBlockVolume(c *Container, device api.Device) (*g
16151617
return nil, fmt.Errorf("Unknown block device driver: %s", c.sandbox.config.HypervisorConfig.BlockDeviceDriver)
16161618
}
16171619

1620+
vol.MountPoint = m.Destination
1621+
1622+
// If no explicit FS Type or Options are being set, then let's use what is provided for the particular mount:
1623+
if vol.Fstype == "" {
1624+
vol.Fstype = m.Type
1625+
}
1626+
if len(vol.Options) == 0 {
1627+
vol.Options = m.Options
1628+
}
1629+
16181630
return vol, nil
16191631
}
16201632

16211633
// handleVhostUserBlkVolume handles volume that is block device file
16221634
// and VhostUserBlk type.
1623-
func (k *kataAgent) handleVhostUserBlkVolume(c *Container, device api.Device) (*grpc.Storage, error) {
1635+
func (k *kataAgent) handleVhostUserBlkVolume(c *Container, m Mount, device api.Device) (*grpc.Storage, error) {
16241636
vol := &grpc.Storage{}
16251637

16261638
d, ok := device.GetDeviceInfo().(*config.VhostUserDeviceAttrs)
@@ -1631,6 +1643,9 @@ func (k *kataAgent) handleVhostUserBlkVolume(c *Container, device api.Device) (*
16311643

16321644
vol.Driver = kataBlkDevType
16331645
vol.Source = d.PCIPath.String()
1646+
vol.Fstype = "bind"
1647+
vol.Options = []string{"bind"}
1648+
vol.MountPoint = m.Destination
16341649

16351650
return vol, nil
16361651
}
@@ -1663,9 +1678,9 @@ func (k *kataAgent) handleBlockVolumes(c *Container) ([]*grpc.Storage, error) {
16631678
var err error
16641679
switch device.DeviceType() {
16651680
case config.DeviceBlock:
1666-
vol, err = k.handleDeviceBlockVolume(c, device)
1681+
vol, err = k.handleDeviceBlockVolume(c, m, device)
16671682
case config.VhostUserBlk:
1668-
vol, err = k.handleVhostUserBlkVolume(c, device)
1683+
vol, err = k.handleVhostUserBlkVolume(c, m, device)
16691684
default:
16701685
k.Logger().Error("Unknown device type")
16711686
continue
@@ -1675,14 +1690,6 @@ func (k *kataAgent) handleBlockVolumes(c *Container) ([]*grpc.Storage, error) {
16751690
return nil, err
16761691
}
16771692

1678-
vol.MountPoint = m.Destination
1679-
if vol.Fstype == "" {
1680-
vol.Fstype = "bind"
1681-
}
1682-
if len(vol.Options) == 0 {
1683-
vol.Options = []string{"bind"}
1684-
}
1685-
16861693
volumeStorages = append(volumeStorages, vol)
16871694
}
16881695

‎virtcontainers/kata_agent_test.go‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ func TestHandleDeviceBlockVolume(t *testing.T) {
413413

414414
tests := []struct {
415415
BlockDeviceDriver string
416+
inputMount Mount
416417
inputDev *drivers.BlockDevice
417418
resultVol *pb.Storage
418419
}{
@@ -424,6 +425,7 @@ func TestHandleDeviceBlockVolume(t *testing.T) {
424425
Format: testBlkDriveFormat,
425426
},
426427
},
428+
inputMount: Mount{},
427429
resultVol: &pb.Storage{
428430
Driver: kataNvdimmDevType,
429431
Source: fmt.Sprintf("/dev/pmem%s", testNvdimmID),
@@ -433,18 +435,25 @@ func TestHandleDeviceBlockVolume(t *testing.T) {
433435
},
434436
{
435437
BlockDeviceDriver: config.VirtioBlockCCW,
438+
inputMount: Mount{
439+
Type: "bind",
440+
Options: []string{"ro"},
441+
},
436442
inputDev: &drivers.BlockDevice{
437443
BlockDrive: &config.BlockDrive{
438444
DevNo: testDevNo,
439445
},
440446
},
441447
resultVol: &pb.Storage{
442-
Driver: kataBlkCCWDevType,
443-
Source: testDevNo,
448+
Driver: kataBlkCCWDevType,
449+
Source: testDevNo,
450+
Fstype: "bind",
451+
Options: []string{"ro"},
444452
},
445453
},
446454
{
447455
BlockDeviceDriver: config.VirtioBlock,
456+
inputMount: Mount{},
448457
inputDev: &drivers.BlockDevice{
449458
BlockDrive: &config.BlockDrive{
450459
PCIPath: testPCIPath,
@@ -505,7 +514,7 @@ func TestHandleDeviceBlockVolume(t *testing.T) {
505514
},
506515
}
507516

508-
vol, _ := k.handleDeviceBlockVolume(c, test.inputDev)
517+
vol, _ := k.handleDeviceBlockVolume(c, test.inputMount, test.inputDev)
509518
assert.True(t, reflect.DeepEqual(vol, test.resultVol),
510519
"Volume didn't match: got %+v, expecting %+v",
511520
vol, test.resultVol)
@@ -521,24 +530,30 @@ func TestHandleBlockVolume(t *testing.T) {
521530
containers := map[string]*Container{}
522531
containers[c.id] = c
523532

524-
// Create a VhostUserBlk device and a DeviceBlock device
533+
// Create a devices for VhostUserBlk, standard DeviceBlock and direct assigned Block device
525534
vDevID := "MockVhostUserBlk"
526535
bDevID := "MockDeviceBlock"
536+
dDevID := "MockDeviceBlockDirect"
527537
vDestination := "/VhostUserBlk/destination"
528538
bDestination := "/DeviceBlock/destination"
539+
dDestination := "/DeviceDirectBlock/destination"
529540
vPCIPath, err := vcTypes.PciPathFromString("01/02")
530541
assert.NoError(t, err)
531542
bPCIPath, err := vcTypes.PciPathFromString("03/04")
532543
assert.NoError(t, err)
544+
dPCIPath, err := vcTypes.PciPathFromString("05/06")
545+
assert.NoError(t, err)
533546

534547
vDev := drivers.NewVhostUserBlkDevice(&config.DeviceInfo{ID: vDevID})
535548
bDev := drivers.NewBlockDevice(&config.DeviceInfo{ID: bDevID})
549+
dDev := drivers.NewBlockDevice(&config.DeviceInfo{ID: dDevID})
536550

537551
vDev.VhostUserDeviceAttrs = &config.VhostUserDeviceAttrs{PCIPath: vPCIPath}
538552
bDev.BlockDrive = &config.BlockDrive{PCIPath: bPCIPath}
553+
dDev.BlockDrive = &config.BlockDrive{PCIPath: dPCIPath}
539554

540555
var devices []api.Device
541-
devices = append(devices, vDev, bDev)
556+
devices = append(devices, vDev, bDev, dDev)
542557

543558
// Create a VhostUserBlk mount and a DeviceBlock mount
544559
var mounts []Mount
@@ -549,8 +564,16 @@ func TestHandleBlockVolume(t *testing.T) {
549564
bMount := Mount{
550565
BlockDeviceID: bDevID,
551566
Destination: bDestination,
567+
Type: "bind",
568+
Options: []string{"bind"},
569+
}
570+
dMount := Mount{
571+
BlockDeviceID: dDevID,
572+
Destination: dDestination,
573+
Type: "ext4",
574+
Options: []string{"ro"},
552575
}
553-
mounts = append(mounts, vMount, bMount)
576+
mounts = append(mounts, vMount, bMount, dMount)
554577

555578
tmpDir := "/vhost/user/dir"
556579
dm := manager.NewDeviceManager(manager.VirtioBlock, true, tmpDir, devices)
@@ -585,9 +608,17 @@ func TestHandleBlockVolume(t *testing.T) {
585608
Driver: kataBlkDevType,
586609
Source: bPCIPath.String(),
587610
}
611+
dStorage := &pb.Storage{
612+
MountPoint: dDestination,
613+
Fstype: "ext4",
614+
Options: []string{"ro"},
615+
Driver: kataBlkDevType,
616+
Source: dPCIPath.String(),
617+
}
588618

589619
assert.Equal(t, vStorage, volumeStorages[0], "Error while handle VhostUserBlk type block volume")
590620
assert.Equal(t, bStorage, volumeStorages[1], "Error while handle BlockDevice type block volume")
621+
assert.Equal(t, dStorage, volumeStorages[2], "Error while handle direct BlockDevice type block volume")
591622
}
592623

593624
func TestAppendDevicesEmptyContainerDeviceList(t *testing.T) {

0 commit comments

Comments
 (0)