From c4daad43e59413e4f83426641c47a231232d4dc9 Mon Sep 17 00:00:00 2001 From: Matthew Arnold <5075485+mrnold@users.noreply.github.com> Date: Tue, 20 Aug 2024 16:26:24 -0400 Subject: [PATCH] VDDK: pass snapshot ID through to nbdkit. (#3385) * 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 * SonarCloud: Move VDDK plugin arguments to struct. Adding 'snapshot' exceeded the seven-parameter limit. Signed-off-by: Matthew Arnold * Change vddk-test expected snapshot argument count. Signed-off-by: Matthew Arnold * 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 --------- Signed-off-by: Matthew Arnold --- pkg/image/nbdkit.go | 37 +++++++++++++++++--------- pkg/importer/vddk-datasource_amd64.go | 14 +++++++--- pkg/importer/vddk-datasource_test.go | 38 ++++++++++++++++++++++++--- tools/vddk-test/vddk-test-plugin.c | 9 ++++--- 4 files changed, 77 insertions(+), 21 deletions(-) diff --git a/pkg/image/nbdkit.go b/pkg/image/nbdkit.go index 202b1f1b49..eebef0c369 100644 --- a/pkg/image/nbdkit.go +++ b/pkg/image/nbdkit.go @@ -122,26 +122,39 @@ func NewNbdkitCurl(nbdkitPidFile, user, password, certDir, socket string, extraH return n } -// NewNbdkitVddk creates a new Nbdkit instance with the vddk plugin -func NewNbdkitVddk(nbdkitPidFile, socket, server, username, password, thumbprint, moref string) (NbdkitOperation, error) { +// 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 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 != "" { - pluginArgs = append(pluginArgs, "password="+password) + if args.Password != "" { + pluginArgs = append(pluginArgs, "password="+args.Password) + } + if args.Thumbprint != "" { + pluginArgs = append(pluginArgs, "thumbprint="+args.Thumbprint) } - if thumbprint != "" { - pluginArgs = append(pluginArgs, "thumbprint="+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.controlpath=0") diff --git a/pkg/importer/vddk-datasource_amd64.go b/pkg/importer/vddk-datasource_amd64.go index d518144b6d..ea0cf29026 100644 --- 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 @@ -887,7 +895,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 index 4a16c5a0e5..d107e49d32 100644 --- a/pkg/importer/vddk-datasource_test.go +++ b/pkg/importer/vddk-datasource_test.go @@ -276,9 +276,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) { @@ -312,6 +312,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" @@ -629,7 +661,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 index ca605825b7..2d23ab9e30 100644 --- a/tools/vddk-test/vddk-test-plugin.c +++ b/tools/vddk-test/vddk-test-plugin.c @@ -20,8 +20,8 @@ #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)); @@ -29,15 +29,18 @@ void fakevddk_close(void *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; } }