From e1687e38276f4ca455f1d465a79fcfbec0f2b671 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Fri, 9 Jan 2026 09:53:26 +0100 Subject: [PATCH 1/4] storage: Add reproducer for https://github.com/Code-Hex/vz/issues/201 At the moment, when using Code-Hex/vz like this: ``` f, err := os.OpenFile(devPath, open_flags, 0) if err != nil { return nil, fmt.Errorf("error opening file: %v", err) } attachment, err := vz.NewDiskBlockDeviceStorageDeviceAttachment(f, conf.ReadOnly, vz.DiskSynchronizationModeFull) if err != nil { _ = f.Close() return nil, fmt.Errorf("error creating disk attachment: %v", err) } ``` the developer has to know that `f` must be kept alive at least as long as the disk attachment is in use. If not, `f` will be garbage collected, its file descriptor will be closed, and the attachment becomes invalid. In this situation, the virtualization framework would return an error. This commit simply adds a test case exhibiting this problem. This will be improved in the next commits. Signed-off-by: Christophe Fergeau --- storage_test.go | 76 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/storage_test.go b/storage_test.go index 8c18e48..85c5e7b 100644 --- a/storage_test.go +++ b/storage_test.go @@ -2,7 +2,10 @@ package vz_test import ( "log" + "os" + "os/exec" "path/filepath" + "runtime" "strings" "testing" @@ -103,3 +106,76 @@ func TestBlockDeviceWithCacheAndSyncMode(t *testing.T) { t.Fatalf("want state %v but got %v", vz.VirtualMachineStateRunning, got) } } + +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) + } + } + // `dev` from the block above will be garbage collected and the underlying file descriptor will be closed. + // This will trigger an internal virtualization error in the subsequent code. + // https://github.com/Code-Hex/vz/issues/201 + runtime.GC() + + 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) + } +} From 30674fceee78aade4a639c514e40c01c203e4a4e Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Tue, 6 Jan 2026 11:57:21 +0100 Subject: [PATCH 2/4] storage: dup() newVZDiskBlockDeviceStorageDeviceAttachment file descriptor In order to solve https://github.com/Code-Hex/vz/issues/201, this commit adds a `newFileHandleDupFd()` objective-C helper. This helper calls `dup()` on its file descriptor argument, and wraps it in a `NSFileHandle` with `closeOnDealloc:true`. This new file handle is fully independent from its golang counterpart, if the golang `os.File` holding the file descriptor is closed, the `NSFileHandle` will still be valid. This new helper is then used with vz.NewDiskBlockDeviceStorageDeviceAttachment. Signed-off-by: Christophe Fergeau --- storage_test.go | 25 ++++++++++++++++++++----- virtualization_14.m | 7 +++++-- virtualization_helper.h | 1 + virtualization_helper.m | 17 ++++++++++++++++- 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/storage_test.go b/storage_test.go index 85c5e7b..26bcd53 100644 --- a/storage_test.go +++ b/storage_test.go @@ -5,7 +5,6 @@ import ( "os" "os/exec" "path/filepath" - "runtime" "strings" "testing" @@ -107,6 +106,22 @@ func TestBlockDeviceWithCacheAndSyncMode(t *testing.T) { } } +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") @@ -147,11 +162,11 @@ func TestBlockDeviceWithDeviceAttachment(t *testing.T) { if err != nil { t.Fatal(err) } + if err := dev.Close(); err != nil { + log.Printf("failed to close %s: %s\n", devPath, err) + } + } - // `dev` from the block above will be garbage collected and the underlying file descriptor will be closed. - // This will trigger an internal virtualization error in the subsequent code. - // https://github.com/Code-Hex/vz/issues/201 - runtime.GC() config, err := vz.NewVirtioBlockDeviceConfiguration(attachment) if err != nil { diff --git a/virtualization_14.m b/virtualization_14.m index b6970f0..ab2a0b4 100644 --- a/virtualization_14.m +++ b/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 diff --git a/virtualization_helper.h b/virtualization_helper.h index 7914efd..c1b5207 100644 --- a/virtualization_helper.h +++ b/virtualization_helper.h @@ -4,6 +4,7 @@ #import NSDictionary *dumpProcessinfo(); +NSFileHandle *newFileHandleDupFd(int fileDescriptor, void **error); #define RAISE_REASON_MESSAGE \ "This may possibly be a bug due to library handling errors.\n" \ diff --git a/virtualization_helper.m b/virtualization_helper.m index 9d65b34..0cb2039 100644 --- a/virtualization_helper.m +++ b/virtualization_helper.m @@ -27,4 +27,19 @@ @"Min Required OS Version" : @__MAC_OS_X_VERSION_MIN_REQUIRED, #endif }; -} \ No newline at end of file +} + +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]; +} From b0149a04d277b8ba27708ea9ed72bafa243973a6 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Tue, 6 Jan 2026 15:42:58 +0100 Subject: [PATCH 3/4] network: Use newFileHandleDupFd in newVZFileHandleNetworkDeviceAttachment NewFileHandleNetworkDeviceAttachment has similar limitations as NewDiskBlockDeviceStorageDeviceAttachment. This commit fixes this by using newFileHandleDupFd to implement it. Signed-off-by: Christophe Fergeau --- network.go | 6 ++++++ virtualization_11.h | 2 +- virtualization_11.m | 7 +++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/network.go b/network.go index c3ecddc..fdcdccd 100644 --- a/network.go +++ b/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) }) diff --git a/virtualization_11.h b/virtualization_11.h index f9f9fdc..8736bfb 100644 --- a/virtualization_11.h +++ b/virtualization_11.h @@ -94,7 +94,7 @@ 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); diff --git a/virtualization_11.m b/virtualization_11.m index bab7e29..742da81 100644 --- a/virtualization_11.m +++ b/virtualization_11.m @@ -602,12 +602,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; From 84a9a9431408971fdbcd7d7012a6fc2da3d1b482 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Tue, 6 Jan 2026 15:42:58 +0100 Subject: [PATCH 4/4] serial: Use newFileHandleDupFd in newVZFileHandleSerialPortAttachment NewFileHandleSerialPortAttachment has similar limitations as NewDiskBlockDeviceStorageDeviceAttachment. This commit fixes this by using newFileHandleDupFd to implement it. Signed-off-by: Christophe Fergeau --- serial_console.go | 5 +++++ virtualization_11.h | 2 +- virtualization_11.m | 13 ++++++++++--- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/serial_console.go b/serial_console.go index 92309cb..61107db 100644 --- a/serial_console.go +++ b/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) }) diff --git a/virtualization_11.h b/virtualization_11.h index 8736bfb..464c69d 100644 --- a/virtualization_11.h +++ b/virtualization_11.h @@ -86,7 +86,7 @@ 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); diff --git a/virtualization_11.m b/virtualization_11.m index 742da81..28060e0 100644 --- a/virtualization_11.m +++ b/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];