Skip to content

Commit 30674fc

Browse files
committed
storage: dup() newVZDiskBlockDeviceStorageDeviceAttachment file descriptor
In order to solve #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 <cfergeau@redhat.com>
1 parent e1687e3 commit 30674fc

4 files changed

Lines changed: 42 additions & 8 deletions

File tree

storage_test.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"os"
66
"os/exec"
77
"path/filepath"
8-
"runtime"
98
"strings"
109
"testing"
1110

@@ -107,6 +106,22 @@ func TestBlockDeviceWithCacheAndSyncMode(t *testing.T) {
107106
}
108107
}
109108

109+
func TestBlockDeviceStorageDeviceAttachmentError(t *testing.T) {
110+
if vz.Available(14) {
111+
t.Skip("vz.NewDiskBlockDeviceStorageDeviceAttachment is supported from macOS 14")
112+
}
113+
114+
f, err := os.Create(filepath.Join(t.TempDir(), "empty"))
115+
if err != nil {
116+
t.Fatal(err)
117+
}
118+
f.Close()
119+
_, err = vz.NewDiskBlockDeviceStorageDeviceAttachment(f, false, vz.DiskSynchronizationModeNone)
120+
if err == nil {
121+
t.Fatal("did not get an error with invalid file descriptor")
122+
}
123+
}
124+
110125
func TestBlockDeviceWithDeviceAttachment(t *testing.T) {
111126
if vz.Available(12) {
112127
t.Skip("vz.NewDiskImageStorageDeviceAttachmentWithCacheAndSync is supported from macOS 12")
@@ -147,11 +162,11 @@ func TestBlockDeviceWithDeviceAttachment(t *testing.T) {
147162
if err != nil {
148163
t.Fatal(err)
149164
}
165+
if err := dev.Close(); err != nil {
166+
log.Printf("failed to close %s: %s\n", devPath, err)
167+
}
168+
150169
}
151-
// `dev` from the block above will be garbage collected and the underlying file descriptor will be closed.
152-
// This will trigger an internal virtualization error in the subsequent code.
153-
// https://github.com/Code-Hex/vz/issues/201
154-
runtime.GC()
155170

156171
config, err := vz.NewVirtioBlockDeviceConfiguration(attachment)
157172
if err != nil {

virtualization_14.m

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
@param error If not nil, assigned with the error if the initialization failed.
3131
@return An initialized `VZDiskBlockDeviceStorageDeviceAttachment` or nil if there was an error.
3232
@discussion
33-
The file handle is retained by the disk attachment.
33+
The file handle is dup()’ed by this function.
3434
The handle must be open when the virtual machine starts.
3535
3636
The `readOnly` parameter affects how the disk is exposed to the guest operating system
@@ -41,7 +41,10 @@
4141
{
4242
#ifdef INCLUDE_TARGET_OSX_14
4343
if (@available(macOS 14, *)) {
44-
NSFileHandle *fileHandle = [[NSFileHandle alloc] initWithFileDescriptor:fileDescriptor];
44+
NSFileHandle *fileHandle = newFileHandleDupFd(fileDescriptor, error);
45+
if (fileHandle == nil) {
46+
return nil;
47+
}
4548
return [[VZDiskBlockDeviceStorageDeviceAttachment alloc]
4649
initWithFileHandle:fileHandle
4750
readOnly:(BOOL)readOnly

virtualization_helper.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#import <Foundation/Foundation.h>
55

66
NSDictionary *dumpProcessinfo();
7+
NSFileHandle *newFileHandleDupFd(int fileDescriptor, void **error);
78

89
#define RAISE_REASON_MESSAGE \
910
"This may possibly be a bug due to library handling errors.\n" \

virtualization_helper.m

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,19 @@
2727
@"Min Required OS Version" : @__MAC_OS_X_VERSION_MIN_REQUIRED,
2828
#endif
2929
};
30-
}
30+
}
31+
32+
NSFileHandle *newFileHandleDupFd(int fileDescriptor, void **error)
33+
{
34+
int dupedFd = dup(fileDescriptor);
35+
if (dupedFd < 0) {
36+
if (error != nil) {
37+
*error = [NSError errorWithDomain:NSPOSIXErrorDomain
38+
code:errno
39+
userInfo:nil];
40+
}
41+
return nil;
42+
}
43+
44+
return [[NSFileHandle alloc] initWithFileDescriptor:dupedFd closeOnDealloc:true];
45+
}

0 commit comments

Comments
 (0)