[release-v1.60] VDDK: pass snapshot ID through to nbdkit. (#3400)

* VDDK: pass snapshot ID through to nbdkit. Opening VMware snapshots can sometimes fail with "Error 13 (You do not have access rights to this file)" if the snapshot MOref is not provided. This specifically seems to affect Windows VMs with VMware guest tools installed. Signed-off-by: Matthew Arnold <[email protected]> * SonarCloud: Move VDDK plugin arguments to struct. Adding 'snapshot' exceeded the seven-parameter limit. Signed-off-by: Matthew Arnold <[email protected]> * Change vddk-test expected snapshot argument count. Signed-off-by: Matthew Arnold <[email protected]> * Remove "san" from VDDK transports list. Some versions of the VDDK crash when determining system support for its "san" transport mode, if there are no SCSI disks present. This transport mode is active by default, and is not used by CDI, so avoid the crash by setting the list of allowable transports to not include "san". Signed-off-by: Matthew Arnold <[email protected]> --------- Signed-off-by: Matthew Arnold <[email protected]> Co-authored-by: Matthew Arnold <[email protected]>
kubevirt · Aug 20, 2024 · fc6f48e · fc6f48e
1 parent c186264
commit fc6f48e
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 20 deletions.
diff --git a/pkg/image/nbdkit.go b/pkg/image/nbdkit.go
@@ -126,29 +126,43 @@ func NewNbdkitCurl(nbdkitPidFile, user, password, certDir, socket string, extraH
 	return n, nil
 }
 
+// Keep these in a struct to keep NewNbdkitVddk from going over the argument limit
+type NbdKitVddkPluginArgs struct {
+	Server     string
+	Username   string
+	Password   string
+	Thumbprint string
+	Moref      string
+	Snapshot   string
+}
+
 // NewNbdkitVddk creates a new Nbdkit instance with the vddk plugin
-func NewNbdkitVddk(nbdkitPidFile, socket, server, username, password, thumbprint, moref string) (NbdkitOperation, error) {
+func NewNbdkitVddk(nbdkitPidFile, socket string, args NbdKitVddkPluginArgs) (NbdkitOperation, error) {
 	pluginArgs := []string{
 		"libdir=" + nbdVddkLibraryPath,
 	}
-	if server != "" {
-		pluginArgs = append(pluginArgs, "server="+server)
+	if args.Server != "" {
+		pluginArgs = append(pluginArgs, "server="+args.Server)
 	}
-	if username != "" {
-		pluginArgs = append(pluginArgs, "user="+username)
+	if args.Username != "" {
+		pluginArgs = append(pluginArgs, "user="+args.Username)
 	}
-	if password != "" {
-		passwordfile, err := writePasswordFile(password)
+	if args.Password != "" {
+		passwordfile, err := writePasswordFile(args.Password)
 		if err != nil {
 			return nil, err
 		}
 		pluginArgs = append(pluginArgs, "password=+"+passwordfile)
 	}
-	if thumbprint != "" {
-		pluginArgs = append(pluginArgs, "thumbprint="+thumbprint)
+	if args.Thumbprint != "" {
+		pluginArgs = append(pluginArgs, "thumbprint="+args.Thumbprint)
+	}
+	if args.Moref != "" {
+		pluginArgs = append(pluginArgs, "vm=moref="+args.Moref)
 	}
-	if moref != "" {
-		pluginArgs = append(pluginArgs, "vm=moref="+moref)
+	if args.Snapshot != "" {
+		pluginArgs = append(pluginArgs, "snapshot="+args.Snapshot)
+		pluginArgs = append(pluginArgs, "transports=file:nbdssl:nbd")
 	}
 	pluginArgs = append(pluginArgs, "--verbose")
 	pluginArgs = append(pluginArgs, "-D", "nbdkit.backend.datapath=0")

diff --git a/pkg/importer/vddk-datasource_amd64.go b/pkg/importer/vddk-datasource_amd64.go
@@ -85,8 +85,16 @@ type NbdKitLogWatcherVddk struct {
 }
 
 // createNbdKitWrapper starts nbdkit and returns a process handle for further management
-func createNbdKitWrapper(vmware *VMwareClient, diskFileName string) (*NbdKitWrapper, error) {
-	n, err := image.NewNbdkitVddk(nbdPidFile, nbdUnixSocket, vmware.url.Host, vmware.username, vmware.password, vmware.thumbprint, vmware.moref)
+func createNbdKitWrapper(vmware *VMwareClient, diskFileName, snapshot string) (*NbdKitWrapper, error) {
+	args := image.NbdKitVddkPluginArgs{
+		Server:     vmware.url.Host,
+		Username:   vmware.username,
+		Password:   vmware.password,
+		Thumbprint: vmware.thumbprint,
+		Moref:      vmware.moref,
+		Snapshot:   snapshot,
+	}
+	n, err := image.NewNbdkitVddk(nbdPidFile, nbdUnixSocket, args)
 	if err != nil {
 		klog.Errorf("Error validating nbdkit plugins: %v", err)
 		return nil, err
@@ -891,7 +899,7 @@ func createVddkDataSource(endpoint string, accessKey string, secKey string, thum
 		}
 		klog.Infof("Set disk file name from current snapshot: %s", diskFileName)
 	}
-	nbdkit, err := newNbdKitWrapper(vmware, diskFileName)
+	nbdkit, err := newNbdKitWrapper(vmware, diskFileName, currentCheckpoint)
 	if err != nil {
 		klog.Errorf("Unable to start nbdkit: %v", err)
 		return nil, err

diff --git a/pkg/importer/vddk-datasource_test.go b/pkg/importer/vddk-datasource_test.go
@@ -279,9 +279,9 @@ var _ = Describe("VDDK data source", func() {
 		var returnedDiskName string
 
 		newVddkDataSource = createVddkDataSource
-		newNbdKitWrapper = func(vmware *VMwareClient, fileName string) (*NbdKitWrapper, error) {
+		newNbdKitWrapper = func(vmware *VMwareClient, fileName, snapshot string) (*NbdKitWrapper, error) {
 			returnedDiskName = fileName
-			return createMockNbdKitWrapper(vmware, fileName)
+			return createMockNbdKitWrapper(vmware, fileName, snapshot)
 		}
 		currentVMwareFunctions.Properties = func(ctx context.Context, ref types.ManagedObjectReference, property []string, result interface{}) error {
 			switch out := result.(type) {
@@ -315,6 +315,38 @@ var _ = Describe("VDDK data source", func() {
 		Entry("should fail if backing file is not found in snapshot tree", "[teststore] testvm/testfile.vmdk", "wrong disk 1.vmdk", "wrong disk 2.vmdk", "wrong disk 1.vmdk", false),
 	)
 
+	It("should pass snapshot reference through to nbdkit", func() {
+		expectedSnapshot := "snapshot-10"
+
+		var receivedSnapshotRef string
+		newVddkDataSource = createVddkDataSource
+		newNbdKitWrapper = func(vmware *VMwareClient, fileName, snapshot string) (*NbdKitWrapper, error) {
+			receivedSnapshotRef = snapshot
+			return createMockNbdKitWrapper(vmware, fileName, snapshot)
+		}
+
+		currentVMwareFunctions.Properties = func(ctx context.Context, ref types.ManagedObjectReference, property []string, result interface{}) error {
+			switch out := result.(type) {
+			case *mo.VirtualMachine:
+				if property[0] == "config.hardware.device" {
+					out.Config = createVirtualDiskConfig("disk1", 12345)
+				} else if property[0] == "snapshot" {
+					out.Snapshot = createSnapshots(expectedSnapshot, "snapshot-11")
+				}
+			case *mo.VirtualMachineSnapshot:
+				out.Config = *createVirtualDiskConfig("snapshot-disk", 123456)
+				disk := out.Config.Hardware.Device[0].(*types.VirtualDisk)
+				parent := disk.Backing.(*types.VirtualDiskFlatVer1BackingInfo).Parent
+				parent.FileName = "snapshot-root"
+			}
+			return nil
+		}
+
+		_, err := NewVDDKDataSource("http://vcenter.test", "user", "pass", "aa:bb:cc:dd", "1-2-3-4", "disk1", expectedSnapshot, "", "", v1.PersistentVolumeFilesystem)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(receivedSnapshotRef).To(Equal(expectedSnapshot))
+	})
+
 	It("should find two snapshots and get a list of changed blocks", func() {
 		newVddkDataSource = createVddkDataSource
 		diskName := "testdisk.vmdk"
@@ -880,7 +912,7 @@ func createMockVMwareClient(endpoint string, accessKey string, secKey string, th
 	}, nil
 }
 
-func createMockNbdKitWrapper(vmware *VMwareClient, diskFileName string) (*NbdKitWrapper, error) {
+func createMockNbdKitWrapper(vmware *VMwareClient, diskFileName, snapshot string) (*NbdKitWrapper, error) {
 	u, _ := url.Parse("http://vcenter.test")
 	return &NbdKitWrapper{
 		n:      &image.Nbdkit{},

diff --git a/tools/vddk-test/vddk-test-plugin.c b/tools/vddk-test/vddk-test-plugin.c
@@ -20,24 +20,27 @@
 
 #define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS
 
-#define EXPECTED_ARG_COUNT 7
 int arg_count = 0;
+int expected_arg_count = 7;
 
 void fakevddk_close(void *handle) {
     close(*((int *) handle));
 }
 
 int fakevddk_config(const char *key, const char *value) {
     arg_count++;
+    if (strcmp(key, "snapshot") == 0) {
+        expected_arg_count = 9; // Expect one for 'snapshot' and one for 'transports'
+    }
     return 0;
 }
 
 int fakevddk_config_complete(void) {
     nbdkit_debug("VMware VixDiskLib (1.2.3) Release build-12345");
-    if (arg_count == EXPECTED_ARG_COUNT) {
+    if (arg_count == expected_arg_count) {
         return 0;
     } else {
-        nbdkit_error("Expected %d arguments to fake VDDK test plugin, but got %d!\n", EXPECTED_ARG_COUNT, arg_count);
+        nbdkit_error("Expected %d arguments to fake VDDK test plugin, but got %d!\n", expected_arg_count, arg_count);
         return -1;
     }
 }