Merge pull request #213 from cfergeau/fddup

dup() golang FDs in obj-C code

Author: Code-Hex

network.go

@@ -192,14 +192,20 @@ func NewFileHandleNetworkDeviceAttachment(file *os.File) (*FileHandleNetworkDevi
 		return nil, err
 	}
 
+	nserrPtr := newNSErrorAsNil()
+
 	attachment := &FileHandleNetworkDeviceAttachment{
 		pointer: objc.NewPointer(
 			C.newVZFileHandleNetworkDeviceAttachment(
 				C.int(file.Fd()),
+				&nserrPtr,
 			),
 		),
 		mtu: 1500, // The default MTU is 1500.
 	}
+	if err := newNSError(nserrPtr); err != nil {
+		return nil, err
+	}
 	objc.SetFinalizer(attachment, func(self *FileHandleNetworkDeviceAttachment) {
 		objc.Release(self)
 	})

serial_console.go

@@ -49,14 +49,19 @@ func NewFileHandleSerialPortAttachment(read, write *os.File) (*FileHandleSerialP
 		return nil, err
 	}
 
+	nserrPtr := newNSErrorAsNil()
 	attachment := &FileHandleSerialPortAttachment{
 		pointer: objc.NewPointer(
 			C.newVZFileHandleSerialPortAttachment(
 				C.int(read.Fd()),
 				C.int(write.Fd()),
+				&nserrPtr,
 			),
 		),
 	}
+	if err := newNSError(nserrPtr); err != nil {
+		return nil, err
+	}
 	objc.SetFinalizer(attachment, func(self *FileHandleSerialPortAttachment) {
 		objc.Release(self)
 	})

storage_test.go

@@ -2,6 +2,8 @@ package vz_test
 
 import (
 	"log"
+	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -103,3 +105,92 @@ func TestBlockDeviceWithCacheAndSyncMode(t *testing.T) {
 		t.Fatalf("want state %v but got %v", vz.VirtualMachineStateRunning, got)
 	}
 }
+
+func TestBlockDeviceStorageDeviceAttachmentError(t *testing.T) {
+	if vz.Available(14) {
+		t.Skip("vz.NewDiskBlockDeviceStorageDeviceAttachment is supported from macOS 14")
+	}
+
+	f, err := os.Create(filepath.Join(t.TempDir(), "empty"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.Close()
+	_, err = vz.NewDiskBlockDeviceStorageDeviceAttachment(f, false, vz.DiskSynchronizationModeNone)
+	if err == nil {
+		t.Fatal("did not get an error with invalid file descriptor")
+	}
+}
+
+func TestBlockDeviceWithDeviceAttachment(t *testing.T) {
+	if vz.Available(12) {
+		t.Skip("vz.NewDiskImageStorageDeviceAttachmentWithCacheAndSync is supported from macOS 12")
+	}
+
+	devPath := ""
+	container := newVirtualizationMachine(t,
+		func(vmc *vz.VirtualMachineConfiguration) error {
+			dir := t.TempDir()
+			path := filepath.Join(dir, "disk.img")
+			if err := vz.CreateDiskImage(path, 512); err != nil {
+				t.Fatal(err)
+			}
+			cmd := exec.Command("hdiutil", "attach", "-imagekey", "diskimage-class=CRawDiskImage", "-nomount", path)
+			output, err := cmd.Output()
+			if err != nil {
+				t.Fatalf("failed to attach disk image: %v", err)
+			}
+
+			outputStr := string(output)
+			lines := strings.Split(outputStr, "\n")
+			if len(lines) == 0 || !strings.HasPrefix(lines[0], "/dev/") {
+				log.Printf("[%s]\n", lines)
+				t.Fatalf("unexpected output from `hdiutil attach`")
+			}
+			if len(lines) != 0 && strings.HasPrefix(lines[0], "/dev/") {
+				devPath = strings.TrimSpace(lines[0])
+			}
+
+			var attachment *vz.DiskBlockDeviceStorageDeviceAttachment
+			{
+				dev, err := os.Open(devPath)
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				attachment, err = vz.NewDiskBlockDeviceStorageDeviceAttachment(dev, false, vz.DiskSynchronizationModeNone)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if err := dev.Close(); err != nil {
+					log.Printf("failed to close %s: %s\n", devPath, err)
+				}
+
+			}
+
+			config, err := vz.NewVirtioBlockDeviceConfiguration(attachment)
+			if err != nil {
+				t.Fatal(err)
+			}
+			vmc.SetStorageDevicesVirtualMachineConfiguration([]vz.StorageDeviceConfiguration{
+				config,
+			})
+			return nil
+		},
+	)
+	t.Cleanup(func() {
+		if err := container.Shutdown(); err != nil {
+			log.Println(err)
+		}
+		cmd := exec.Command("hdiutil", "detach", devPath)
+		if err := cmd.Run(); err != nil {
+			log.Printf("hdiutil detach %s failed: %s\n", devPath, err)
+		}
+	})
+
+	vm := container.VirtualMachine
+
+	if got := vm.State(); vz.VirtualMachineStateRunning != got {
+		t.Fatalf("want state %v but got %v", vz.VirtualMachineStateRunning, got)
+	}
+}

virtualization_11.h

@@ -86,15 +86,15 @@ void setStorageDevicesVZVirtualMachineConfiguration(void *config,
 void *storageDevicesVZVirtualMachineConfiguration(void *config);
 
 /* Configurations */
-void *newVZFileHandleSerialPortAttachment(int readFileDescriptor, int writeFileDescriptor);
+void *newVZFileHandleSerialPortAttachment(int readFileDescriptor, int writeFileDescriptor, void **error);
 void *newVZFileSerialPortAttachment(const char *filePath, bool shouldAppend, void **error);
 void *newVZVirtioConsoleDeviceSerialPortConfiguration(void *attachment);
 void *VZBridgedNetworkInterface_networkInterfaces(void);
 const char *VZBridgedNetworkInterface_identifier(void *networkInterface);
 const char *VZBridgedNetworkInterface_localizedDisplayName(void *networkInterface);
 void *newVZBridgedNetworkDeviceAttachment(void *networkInterface);
 void *newVZNATNetworkDeviceAttachment(void);
-void *newVZFileHandleNetworkDeviceAttachment(int fileDescriptor);
+void *newVZFileHandleNetworkDeviceAttachment(int fileDescriptor, void **error);
 void *newVZVirtioNetworkDeviceConfiguration(void *attachment);
 void setNetworkDevicesVZMACAddress(void *config, void *macAddress);
 void *newVZVirtioEntropyDeviceConfiguration(void);

virtualization_11.m

@@ -448,13 +448,20 @@ void setStorageDevicesVZVirtualMachineConfiguration(void *config,
  @discussion
     Each file descriptor must a valid.
 */
-void *newVZFileHandleSerialPortAttachment(int readFileDescriptor, int writeFileDescriptor)
+void *newVZFileHandleSerialPortAttachment(int readFileDescriptor, int writeFileDescriptor, void **error)
 {
     if (@available(macOS 11, *)) {
         VZFileHandleSerialPortAttachment *ret;
         @autoreleasepool {
-            NSFileHandle *fileHandleForReading = [[NSFileHandle alloc] initWithFileDescriptor:readFileDescriptor];
-            NSFileHandle *fileHandleForWriting = [[NSFileHandle alloc] initWithFileDescriptor:writeFileDescriptor];
+            NSFileHandle *fileHandleForReading = newFileHandleDupFd(readFileDescriptor, error);
+            if (error != nil) {
+                return nil;
+            }
+
+            NSFileHandle *fileHandleForWriting = newFileHandleDupFd(writeFileDescriptor, error);
+            if (error != nil) {
+                return nil;
+            }
             ret = [[VZFileHandleSerialPortAttachment alloc]
                 initWithFileHandleForReading:fileHandleForReading
                         fileHandleForWriting:fileHandleForWriting];
@@ -602,12 +609,15 @@ void setStorageDevicesVZVirtualMachineConfiguration(void *config,
  @see VZNetworkDeviceConfiguration
  @see VZVirtioNetworkDeviceConfiguration
  */
-void *newVZFileHandleNetworkDeviceAttachment(int fileDescriptor)
+void *newVZFileHandleNetworkDeviceAttachment(int fileDescriptor, void **error)
 {
     if (@available(macOS 11, *)) {
         VZFileHandleNetworkDeviceAttachment *ret;
         @autoreleasepool {
-            NSFileHandle *fileHandle = [[NSFileHandle alloc] initWithFileDescriptor:fileDescriptor];
+            NSFileHandle *fileHandle = newFileHandleDupFd(fileDescriptor, error);
+            if (fileHandle == nil) {
+                return nil;
+            }
             ret = [[VZFileHandleNetworkDeviceAttachment alloc] initWithFileHandle:fileHandle];
         }
         return ret;

virtualization_14.m

@@ -30,7 +30,7 @@
  @param error If not nil, assigned with the error if the initialization failed.
  @return An initialized `VZDiskBlockDeviceStorageDeviceAttachment` or nil if there was an error.
  @discussion
-    The file handle is retained by the disk attachment.
+    The file handle is dup()’ed by this function.
     The handle must be open when the virtual machine starts.
 
     The `readOnly` parameter affects how the disk is exposed to the guest operating system
@@ -41,7 +41,10 @@
 {
 #ifdef INCLUDE_TARGET_OSX_14
     if (@available(macOS 14, *)) {
-        NSFileHandle *fileHandle = [[NSFileHandle alloc] initWithFileDescriptor:fileDescriptor];
+        NSFileHandle *fileHandle = newFileHandleDupFd(fileDescriptor, error);
+        if (fileHandle == nil) {
+            return nil;
+        }
         return [[VZDiskBlockDeviceStorageDeviceAttachment alloc]
              initWithFileHandle:fileHandle
                        readOnly:(BOOL)readOnly

virtualization_helper.h

@@ -4,6 +4,7 @@
 #import <Foundation/Foundation.h>
 
 NSDictionary *dumpProcessinfo();
+NSFileHandle *newFileHandleDupFd(int fileDescriptor, void **error);
 
 #define RAISE_REASON_MESSAGE                                                                               \
     "This may possibly be a bug due to library handling errors.\n"                                         \

virtualization_helper.m

@@ -27,4 +27,19 @@
         @"Min Required OS Version" : @__MAC_OS_X_VERSION_MIN_REQUIRED,
 #endif
     };
-}
+}
+
+NSFileHandle *newFileHandleDupFd(int fileDescriptor, void **error)
+{
+    int dupedFd = dup(fileDescriptor);
+    if (dupedFd < 0) {
+        if (error != nil) {
+            *error = [NSError errorWithDomain:NSPOSIXErrorDomain
+                                         code:errno
+                                     userInfo:nil];
+        }
+        return nil;
+    }
+
+    return [[NSFileHandle alloc] initWithFileDescriptor:dupedFd closeOnDealloc:true];
+}