Merge pull request #283 from kzys/read-only

The rootfs of a microVM should be read-only

Author: kzys

go.mod

@@ -18,7 +18,7 @@ require (
 	github.com/docker/go-events v0.0.0-20170721190031-9461782956ad // indirect
 	github.com/docker/go-metrics v0.0.0-20181218153428-b84716841b82 // indirect
 	github.com/docker/go-units v0.3.3
-	github.com/firecracker-microvm/firecracker-go-sdk v0.17.1-0.20190920221449-6afccc1d121f
+	github.com/firecracker-microvm/firecracker-go-sdk v0.17.1-0.20191014182425-56995a05946a
 	github.com/go-ole/go-ole v1.2.4 // indirect
 	github.com/godbus/dbus v0.0.0-20181025153459-66d97aec3384 // indirect
 	github.com/gofrs/uuid v3.2.0+incompatible

go.sum

@@ -59,8 +59,8 @@ github.com/docker/go-metrics v0.0.0-20181218153428-b84716841b82 h1:X0fj836zx99zF
 github.com/docker/go-metrics v0.0.0-20181218153428-b84716841b82/go.mod h1:/u0gXw0Gay3ceNrsHubL3BtdOL2fHf93USgMTe0W5dI=
 github.com/docker/go-units v0.3.3 h1:Xk8S3Xj5sLGlG5g67hJmYMmUgXv5N4PhkjJHHqrwnTk=
 github.com/docker/go-units v0.3.3/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
-github.com/firecracker-microvm/firecracker-go-sdk v0.17.1-0.20190920221449-6afccc1d121f h1:G81Ey0lQDWCqwdIRq6aLN5RyYbnSDx2O7FKFl433shc=
-github.com/firecracker-microvm/firecracker-go-sdk v0.17.1-0.20190920221449-6afccc1d121f/go.mod h1:tVXziw7GjioCKVjI5/agymYxUaqJM6q7cp9e6kwjo8Q=
+github.com/firecracker-microvm/firecracker-go-sdk v0.17.1-0.20191014182425-56995a05946a h1:WK1TpnYdiXA8Kb5UMMkybaZtlo+F1a2QeBmqCJHWeTE=
+github.com/firecracker-microvm/firecracker-go-sdk v0.17.1-0.20191014182425-56995a05946a/go.mod h1:tVXziw7GjioCKVjI5/agymYxUaqJM6q7cp9e6kwjo8Q=
 github.com/globalsign/mgo v0.0.0-20180905125535-1ca0a4f7cbcb h1:D4uzjWwKYQ5XnAvUbuvHW93esHg7F8N/OYeBBcJoTr0=
 github.com/globalsign/mgo v0.0.0-20180905125535-1ca0a4f7cbcb/go.mod h1:xkRDCp4j0OGD1HRkm4kmhM+pmpv3AKq5SU7GMg4oO/Q=
 github.com/go-ole/go-ole v1.2.4 h1:nNBDSCOigTSiarFpYE9J/KtEA1IOW4CNeqT9TQDqCxI=

proto/types.pb.go

@@ -108,7 +108,7 @@ message FirecrackerMachineConfiguration {
 
 // Message to specify the block device config for a Firecracker VM
 message FirecrackerDrive {
-	bool IsReadOnly = 1; // Specifies if the drive is read only
+	bool IsWritable = 1; // Specifies if the drive is writable from a guest
 	bool IsRootDevice = 2; // Specifies if the drive is the root device
 	string Partuuid = 3; // Specifies the unique id of the boot partition on this device
 	string PathOnHost = 4; // Specifies the host level path for the guest drive

proto/types.proto

@@ -94,7 +94,6 @@ func TestCNISupport_Isolated(t *testing.T) {
 				},
 				RootDrive: &proto.FirecrackerDrive{
 					PathOnHost:   defaultVMRootfsPath,
-					IsReadOnly:   false,
 					IsRootDevice: true,
 				},
 				NetworkInterfaces: []*proto.FirecrackerNetworkInterface{{
@@ -269,7 +268,6 @@ func TestCNIPlugin_Performance(t *testing.T) {
 				},
 				RootDrive: &proto.FirecrackerDrive{
 					PathOnHost:   defaultVMRootfsPath,
-					IsReadOnly:   false,
 					IsRootDevice: true,
 				},
 				NetworkInterfaces: []*proto.FirecrackerNetworkInterface{{

runtime/cni_integ_test.go

@@ -122,7 +122,7 @@ func addDriveFromProto(builder firecracker.DrivesBuilder, drive *proto.Firecrack
 		}
 	}
 
-	return builder.AddDrive(drive.PathOnHost, drive.IsReadOnly, opt)
+	return builder.AddDrive(drive.PathOnHost, !drive.IsWritable, opt)
 }
 
 // rateLimiterFromProto creates a firecracker RateLimiter object from the
@@ -140,6 +140,15 @@ func rateLimiterFromProto(rl *proto.FirecrackerRateLimiter) *models.RateLimiter
 	return &result
 }
 
+func withRateLimiterFromProto(rl *proto.FirecrackerRateLimiter) firecracker.DriveOpt {
+	if rl == nil {
+		return func(d *models.Drive) {
+			// no-op
+		}
+	}
+	return firecracker.WithRateLimiter(*rateLimiterFromProto(rl))
+}
+
 // tokenBucketFromProto creates a firecracker TokenBucket object from the
 // protobuf message.
 func tokenBucketFromProto(bucket *proto.FirecrackerTokenBucket) *models.TokenBucket {

runtime/helpers.go

@@ -210,7 +210,7 @@ func TestTokenBucketFromProto(t *testing.T) {
 
 func TestAddDriveFromProto(t *testing.T) {
 	list := addDriveFromProto(firecracker.DrivesBuilder{}, &proto.FirecrackerDrive{
-		IsReadOnly: true,
+		IsWritable: false,
 		PathOnHost: "/a",
 		Partuuid:   "xy",
 	}).Build()

runtime/helpers_test.go

@@ -40,6 +40,7 @@ import (
 	"github.com/containerd/fifo"
 	"github.com/containerd/ttrpc"
 	"github.com/firecracker-microvm/firecracker-go-sdk"
+	"github.com/firecracker-microvm/firecracker-go-sdk/client/models"
 	"github.com/gofrs/uuid"
 	ptypes "github.com/gogo/protobuf/types"
 	"github.com/golang/protobuf/ptypes/empty"
@@ -638,27 +639,13 @@ func (s *service) buildVMConfiguration(req *proto.CreateVMRequest) (*firecracker
 		containerCount = 1
 	}
 
-	// Create stub drives first and let stub driver handler manage the drives
-	handler, err := newStubDriveHandler(s.shimDir.RootPath(), s.logger, containerCount)
+	stubDriveHandler, err := newStubDriveHandler(s.shimDir.RootPath(), s.logger, containerCount)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create stub drives")
 	}
-	s.stubDriveHandler = handler
+	s.stubDriveHandler = stubDriveHandler
 
-	var driveBuilder firecracker.DrivesBuilder
-	// Create non-stub drives
-	if root := req.RootDrive; root != nil {
-		driveBuilder = firecracker.NewDrivesBuilder(root.PathOnHost)
-	} else {
-		driveBuilder = firecracker.NewDrivesBuilder(s.config.RootDrive)
-	}
-
-	for _, drive := range req.AdditionalDrives {
-		driveBuilder = addDriveFromProto(driveBuilder, drive)
-	}
-
-	// a micro VM must know all drives
-	cfg.Drives = append(handler.GetDrives(), driveBuilder.Build()...)
+	cfg.Drives = append(stubDriveHandler.GetDrives(), s.buildNonStubDrives(req)...)
 
 	// If no value for NetworkInterfaces was specified (not even an empty but non-nil list) and
 	// the runtime config specifies a default list, use those defaults
@@ -681,6 +668,25 @@ func (s *service) buildVMConfiguration(req *proto.CreateVMRequest) (*firecracker
 	return &cfg, nil
 }
 
+func (s *service) buildNonStubDrives(req *proto.CreateVMRequest) []models.Drive {
+	var builder firecracker.DrivesBuilder
+
+	if input := req.RootDrive; input != nil {
+		builder = builder.WithRootDrive(input.PathOnHost,
+			firecracker.WithReadOnly(!input.IsWritable),
+			firecracker.WithPartuuid(input.Partuuid),
+			withRateLimiterFromProto(input.RateLimiter))
+	} else {
+		builder = builder.WithRootDrive(s.config.RootDrive, firecracker.WithReadOnly(true))
+	}
+
+	for _, drive := range req.AdditionalDrives {
+		builder = addDriveFromProto(builder, drive)
+	}
+
+	return builder.Build()
+}
+
 func (s *service) Create(requestCtx context.Context, request *taskAPI.CreateTaskRequest) (*taskAPI.CreateTaskResponse, error) {
 	logger := s.logger.WithField("task_id", request.ID)
 	defer logPanicAndDie(logger)

runtime/service.go

@@ -268,7 +268,6 @@ func TestMultipleVMs_Isolated(t *testing.T) {
 				},
 				RootDrive: &proto.FirecrackerDrive{
 					PathOnHost:   rootfsPath,
-					IsReadOnly:   false,
 					IsRootDevice: true,
 				},
 				NetworkInterfaces: []*proto.FirecrackerNetworkInterface{
@@ -478,7 +477,6 @@ func TestLongUnixSocketPath_Isolated(t *testing.T) {
 		VMID: vmID,
 		RootDrive: &proto.FirecrackerDrive{
 			PathOnHost:   defaultVMRootfsPath,
-			IsReadOnly:   false,
 			IsRootDevice: true,
 		},
 		NetworkInterfaces: []*proto.FirecrackerNetworkInterface{},
@@ -533,7 +531,6 @@ func TestStubBlockDevices_Isolated(t *testing.T) {
 		VMID: strconv.Itoa(vmID),
 		RootDrive: &proto.FirecrackerDrive{
 			PathOnHost:   rootfsPath,
-			IsReadOnly:   false,
 			IsRootDevice: true,
 		},
 		NetworkInterfaces: []*proto.FirecrackerNetworkInterface{
@@ -670,7 +667,6 @@ func testCreateContainerWithSameName(t *testing.T, vmID string) {
 			VMID: vmID,
 			RootDrive: &proto.FirecrackerDrive{
 				PathOnHost:   defaultRootfsPath,
-				IsReadOnly:   true,
 				IsRootDevice: true,
 			},
 			ContainerCount: 2,