vm(qemu): support saving external drive bookmark before starting QEMU

This requires us to save two bookmarks: one for the main process and one for
the helper process. This also simplifies the logic for changing images and we
no longer need to pass around references to the UTMQemuConfigurationDrive.

Configuration/UTMQemuConfigurationDrive.swift

@@ -218,14 +218,3 @@ extension UTMQemuConfigurationDrive {
         self.interface = defaultInterfaceForImageType!(imageType)
     }
 }
-
-// MARK: - Drive label for display
-
-extension UTMQemuConfigurationDrive {
-    var label: String {
-        String.localizedStringWithFormat(NSLocalizedString("%@ (%@): %@", comment: "UTMQemuConfigurationDrive"),
-                                         imageType.prettyValue,
-                                         interface.prettyValue,
-                                         imageURL?.lastPathComponent ?? NSLocalizedString("none", comment: "UTMQemuConfigurationDrive"))
-    }
-}

Managers/UTMQemuVirtualMachine.swift

@@ -22,34 +22,33 @@ extension UTMQemuVirtualMachine {
         config.qemuConfig!
     }
     
-    func eject(_ drive: inout UTMQemuConfigurationDrive, isForced: Bool = false) throws {
-        guard let oldURL = drive.imageURL else {
-            return // nothing to eject
-        }
+    func eject(_ drive: UTMQemuConfigurationDrive, isForced: Bool = false) throws {
         guard drive.isExternal else {
             return
         }
-        drive.imageURL = nil
-        registryEntry?.externalDrives.removeValue(forKey: drive.id)
-        system?.stopAccessingPath(oldURL.path)
+        if let oldPath = registryEntry.externalDrives[drive.id]?.path {
+            system?.stopAccessingPath(oldPath)
+        }
+        registryEntry.externalDrives.removeValue(forKey: drive.id)
         guard let qemu = qemu, qemu.isConnected else {
             return
         }
         try qemu.ejectDrive("drive\(drive.id)", force: isForced)
     }
     
-    func changeMedium(_ drive: inout UTMQemuConfigurationDrive, with url: URL) async throws {
+    func changeMedium(_ drive: UTMQemuConfigurationDrive, with url: URL) async throws {
         _ = url.startAccessingSecurityScopedResource()
         defer {
             url.stopAccessingSecurityScopedResource()
         }
         let tempBookmark = try url.bookmarkData()
-        try eject(&drive, isForced: true)
-        try await changeMedium(&drive, with: tempBookmark, isSecurityScoped: false)
-        drive.imageURL = url
+        try eject(drive, isForced: true)
+        let file = try UTMRegistryEntry.File(url: url, isReadOnly: drive.isReadOnly)
+        registryEntry.externalDrives[drive.id] = file
+        try await changeMedium(drive, with: tempBookmark, isSecurityScoped: false)
     }
     
-    private func changeMedium(_ drive: inout UTMQemuConfigurationDrive, with bookmark: Data, isSecurityScoped: Bool) async throws {
+    private func changeMedium(_ drive: UTMQemuConfigurationDrive, with bookmark: Data, isSecurityScoped: Bool) async throws {
         guard let system = system else {
             return
         }
@@ -57,8 +56,9 @@ extension UTMQemuVirtualMachine {
         guard let bookmark = bookmark, let path = path, success else {
             throw UTMQemuVirtualMachineError.accessDriveImageFailed
         }
-        let file = UTMRegistryEntry.File(path: path, bookmark: bookmark, isReadOnly: drive.isReadOnly)
-        registryEntry?.externalDrives[drive.id] = file
+        if registryEntry.externalDrives[drive.id] != nil {
+            registryEntry.externalDrives[drive.id]!.remoteBookmark = bookmark
+        }
         if let qemu = qemu, qemu.isConnected {
             try qemu.changeMedium(forDrive: "drive\(drive.id)", path: path)
         }
@@ -68,18 +68,18 @@ extension UTMQemuVirtualMachine {
         guard system != nil && qemu != nil && qemu!.isConnected else {
             throw UTMQemuVirtualMachineError.invalidVmState
         }
-        let qemuConfig = config.qemuConfig!
-        for i in qemuConfig.drives.indices {
-            if !qemuConfig.drives[i].isExternal {
+        for drive in qemuConfig.drives {
+            if !drive.isExternal {
                 continue
             }
-            let id = qemuConfig.drives[i].id
-            if let url = qemuConfig.drives[i].imageURL {
-                // an image was selected while the VM was stopped
-                try await changeMedium(&qemuConfig.drives[i], with: url)
-            } else if let bookmark = registryEntry?.externalDrives[id]?.bookmark {
-                // an image bookmark was saved
-                try await changeMedium(&qemuConfig.drives[i], with: bookmark, isSecurityScoped: true)
+            let id = drive.id
+            if let bookmark = registryEntry.externalDrives[id]?.remoteBookmark {
+                // an image bookmark was saved while QEMU was running
+                try await changeMedium(drive, with: bookmark, isSecurityScoped: true)
+            } else if let localBookmark = registryEntry.externalDrives[id]?.bookmark {
+                // an image bookmark was saved while QEMU was NOT running
+                let url = try URL(resolvingPersistentBookmarkData: localBookmark)
+                try await changeMedium(drive, with: url)
             }
         }
     }
@@ -94,6 +94,10 @@ extension UTMQemuVirtualMachine {
             }
         }
     }
+    
+    func externalImageURL(for drive: UTMQemuConfigurationDrive) -> URL? {
+        registryEntry.externalDrives[drive.id]?.url
+    }
 }
 
 enum UTMQemuVirtualMachineError: Error {

Managers/UTMRegistryEntry.swift

@@ -44,7 +44,10 @@ import Foundation
         }
         let path = vm.path.path
         name = vm.detailsTitleLabel
-        package = File(path: path, bookmark: bookmark, isReadOnly: false)
+        guard let package = try? File(path: path, bookmark: bookmark, isReadOnly: false) else {
+            return nil
+        }
+        self.package = package;
         uuid = vm.config.uuid.uuidString
         externalDrives = [:]
         sharedDirectories = []
@@ -74,22 +77,35 @@ import Foundation
 
 extension UTMRegistryEntry {
     struct File: Codable {
+        var url: URL
+        
         var path: String
         
         var bookmark: Data
         
+        var remoteBookmark: Data?
+        
         var isReadOnly: Bool
         
         private enum CodingKeys: String, CodingKey {
             case path = "Path"
             case bookmark = "Bookmark"
+            case remoteBookmark = "BookmarkRemote"
             case isReadOnly = "ReadOnly"
         }
         
-        init(path: String, bookmark: Data, isReadOnly: Bool = false) {
+        init(path: String, bookmark: Data, isReadOnly: Bool = false) throws {
             self.path = path
             self.bookmark = bookmark
             self.isReadOnly = isReadOnly
+            self.url = try URL(resolvingPersistentBookmarkData: bookmark)
+        }
+        
+        init(url: URL, isReadOnly: Bool = false) throws {
+            self.path = url.path
+            self.bookmark = try url.persistentBookmarkData(isReadyOnly: isReadOnly)
+            self.isReadOnly = isReadOnly
+            self.url = url
         }
         
         init(from decoder: Decoder) throws {
@@ -97,6 +113,8 @@ extension UTMRegistryEntry {
             path = try container.decode(String.self, forKey: .path)
             bookmark = try container.decode(Data.self, forKey: .bookmark)
             isReadOnly = try container.decode(Bool.self, forKey: .isReadOnly)
+            remoteBookmark = try container.decodeIfPresent(Data.self, forKey: .remoteBookmark)
+            url = try URL(resolvingPersistentBookmarkData: bookmark)
         }
         
         func encode(to encoder: Encoder) throws {
@@ -104,6 +122,7 @@ extension UTMRegistryEntry {
             try container.encode(path, forKey: .path)
             try container.encode(bookmark, forKey: .bookmark)
             try container.encode(isReadOnly, forKey: .isReadOnly)
+            try container.encodeIfPresent(remoteBookmark, forKey: .remoteBookmark)
         }
     }
     

Managers/UTMVirtualMachine.h

@@ -54,7 +54,7 @@ NS_ASSUME_NONNULL_BEGIN
 /// This includes display size, bookmarks to removable drives, etc.
 /// This property is observable and must only be accessed on the main thread.
 @property (nonatomic, readonly) UTMViewState *viewState;
-@property (nonatomic, nullable) UTMRegistryEntry *registryEntry;
+@property (nonatomic, readonly) UTMRegistryEntry *registryEntry;
 
 /// Current VM state, can observe this property for state changes or use the delegate
 ///

Managers/UTMVirtualMachine.m

@@ -49,6 +49,7 @@ const dispatch_time_t kScreenshotPeriodSeconds = 60 * NSEC_PER_SEC;
 @property (nonatomic, readonly) BOOL isScreenshotSaveEnabled;
 @property (nonatomic, nullable) void (^screenshotTimerHandler)(void);
 @property (nonatomic) BOOL isScopedAccess;
+@property (nonatomic, readwrite) UTMRegistryEntry *registryEntry;
 
 @end
 

Managers/UTMVirtualMachineExtension.swift

@@ -210,6 +210,44 @@ public extension UTMQemuVirtualMachine {
     }
 }
 
+// MARK: - Bookmark handling
+extension URL {
+    private static var defaultCreationOptions: BookmarkCreationOptions {
+        #if os(iOS)
+        return .minimalBookmark
+        #else
+        return .withSecurityScope
+        #endif
+    }
+    
+    private static var defaultResolutionOptions: BookmarkResolutionOptions {
+        #if os(iOS)
+        return []
+        #else
+        return .withSecurityScope
+        #endif
+    }
+    
+    func persistentBookmarkData(isReadyOnly: Bool = false) throws -> Data {
+        var options = Self.defaultCreationOptions
+        #if os(macOS)
+        if isReadyOnly {
+            options.insert(.securityScopeAllowOnlyReadAccess)
+        }
+        #endif
+        return try self.bookmarkData(options: options,
+                                     includingResourceValuesForKeys: nil,
+                                     relativeTo: nil)
+    }
+    
+    init(resolvingPersistentBookmarkData bookmark: Data) throws {
+        var stale: Bool = false
+        try self.init(resolvingBookmarkData: bookmark,
+                      options: Self.defaultResolutionOptions,
+                      bookmarkDataIsStale: &stale)
+    }
+}
+
 extension UTMDrive: Identifiable {
     public var id: Int {
         self.index

Platform/Shared/VMRemovableDrivesView.swift

@@ -24,7 +24,7 @@ struct VMRemovableDrivesView: View {
     @State private var diskImageFileImportPresented: Bool = false
     /// Explanation see "SwiftUI FileImporter modal bug" in the `body`
     @State private var workaroundFileImporterBug: Bool = false
-    @State private var currentDriveBinding: Binding<UTMQemuConfigurationDrive>?
+    @State private var currentDrive: UTMQemuConfigurationDrive?
     
     var fileManager: FileManager {
         FileManager.default
@@ -70,60 +70,58 @@ struct VMRemovableDrivesView: View {
                     }
                 }.fileImporter(isPresented: $shareDirectoryFileImportPresented, allowedContentTypes: [.folder], onCompletion: selectShareDirectory)
             }
-            ForEach($config.drives) { $drive in
-                if drive.isExternal {
-                    HStack {
-                        // Drive menu
-                        Menu {
-                            // Browse button
-                            Button(action: {
-                                currentDriveBinding = $drive
-                                // MARK: SwiftUI FileImporter modal bug
-                                /// At this point in the execution, `diskImageFileImportPresented` must be `false`.
-                                /// However there is a SwiftUI FileImporter modal bug:
-                                /// if the user taps outside the import modal to cancel instead of tapping the actual cancel button,
-                                /// the `.fileImporter` doesn't actually set the isPresented Binding to `false`.
-                                if (diskImageFileImportPresented) {
-                                    /// bug! Let's set the bool to false ourselves.
-                                    diskImageFileImportPresented = false
-                                    /// One more thing: we can't immediately set it to `true` again because then the state won't have changed.
-                                    /// So we have to use the workaround, which is caught in the `.onChange` below.
-                                    workaroundFileImporterBug = true
-                                } else {
+            ForEach(config.drives.filter { $0.isExternal }) { drive in
+                HStack {
+                    // Drive menu
+                    Menu {
+                        // Browse button
+                        Button(action: {
+                            currentDrive = drive
+                            // MARK: SwiftUI FileImporter modal bug
+                            /// At this point in the execution, `diskImageFileImportPresented` must be `false`.
+                            /// However there is a SwiftUI FileImporter modal bug:
+                            /// if the user taps outside the import modal to cancel instead of tapping the actual cancel button,
+                            /// the `.fileImporter` doesn't actually set the isPresented Binding to `false`.
+                            if (diskImageFileImportPresented) {
+                                /// bug! Let's set the bool to false ourselves.
+                                diskImageFileImportPresented = false
+                                /// One more thing: we can't immediately set it to `true` again because then the state won't have changed.
+                                /// So we have to use the workaround, which is caught in the `.onChange` below.
+                                workaroundFileImporterBug = true
+                            } else {
+                                diskImageFileImportPresented = true
+                            }
+                        }, label: {
+                            Label("Browse…", systemImage: "doc.badge.plus")
+                        })
+                        .onChange(of: workaroundFileImporterBug) { doWorkaround in
+                            /// Explanation see "SwiftUI FileImporter modal bug" above
+                            if doWorkaround {
+                                DispatchQueue.main.async {
+                                    workaroundFileImporterBug = false
                                     diskImageFileImportPresented = true
                                 }
-                            }, label: {
-                                Label("Browse…", systemImage: "doc.badge.plus")
-                            })
-                            .onChange(of: workaroundFileImporterBug) { doWorkaround in
-                                /// Explanation see "SwiftUI FileImporter modal bug" above
-                                if doWorkaround {
-                                    DispatchQueue.main.async {
-                                        workaroundFileImporterBug = false
-                                        diskImageFileImportPresented = true
-                                    }
-                                }
-                            }
-                            // Eject button
-                            if drive.imageURL != nil {
-                                Button(action: { clearRemovableImage(forDrive: $drive) }, label: {
-                                    Label("Clear", systemImage: "eject")
-                                })
                             }
-                        } label: {
-                            DriveLabel(drive: drive)
-                        }.disabled(vm.viewState.hasSaveState)
-                        Spacer()
-                        // Disk image path, or (empty)
-                        Text(pathFor(drive))
-                            .lineLimit(1)
-                            .truncationMode(.tail)
-                            .foregroundColor(.secondary)
-                    }.fileImporter(isPresented: $diskImageFileImportPresented, allowedContentTypes: [.data]) { result in
-                        if let currentDrive = self.currentDriveBinding {
-                            selectRemovableImage(forDrive: currentDrive, result: result)
-                            self.currentDriveBinding = nil
                         }
+                        // Eject button
+                        if vm.externalImageURL(for: drive) != nil {
+                            Button(action: { clearRemovableImage(forDrive: drive) }, label: {
+                                Label("Clear", systemImage: "eject")
+                            })
+                        }
+                    } label: {
+                        DriveLabel(drive: drive, isInserted: vm.externalImageURL(for: drive) != nil)
+                    }.disabled(vm.viewState.hasSaveState)
+                    Spacer()
+                    // Disk image path, or (empty)
+                    Text(pathFor(drive))
+                        .lineLimit(1)
+                        .truncationMode(.tail)
+                        .foregroundColor(.secondary)
+                }.fileImporter(isPresented: $diskImageFileImportPresented, allowedContentTypes: [.data]) { result in
+                    if let currentDrive = self.currentDrive {
+                        selectRemovableImage(forDrive: currentDrive, result: result)
+                        self.currentDrive = nil
                     }
                 }
             }
@@ -152,7 +150,7 @@ struct VMRemovableDrivesView: View {
     }
     
     private func pathFor(_ drive: UTMQemuConfigurationDrive) -> String {
-        if let url = drive.imageURL {
+        if let url = vm.externalImageURL(for: drive) {
             return url.lastPathComponent
         } else {
             return NSLocalizedString("(empty)", comment: "A removable drive that has no image file inserted.")
@@ -161,10 +159,11 @@ struct VMRemovableDrivesView: View {
     
     private struct DriveLabel: View {
         let drive: UTMQemuConfigurationDrive
+        let isInserted: Bool
 
         var body: some View {
             if drive.imageType == .cd {
-                return Label("CD/DVD", systemImage: drive.imageURL == nil ? "opticaldiscdrive" : "opticaldiscdrive.fill")
+                return Label("CD/DVD", systemImage: !isInserted ? "opticaldiscdrive" : "opticaldiscdrive.fill")
             } else {
                 return Label("Removable", systemImage: "externaldrive")
             }
@@ -187,11 +186,11 @@ struct VMRemovableDrivesView: View {
         vm.clearSharedDirectory()
     }
     
-    private func selectRemovableImage(forDrive drive: Binding<UTMQemuConfigurationDrive>, result: Result<URL, Error>) {
+    private func selectRemovableImage(forDrive drive: UTMQemuConfigurationDrive, result: Result<URL, Error>) {
         data.busyWorkAsync {
             switch result {
             case .success(let url):
-                try await vm.changeMedium(&drive.wrappedValue, with: url)
+                try await vm.changeMedium(drive, with: url)
                 break
             case .failure(let err):
                 throw err
@@ -199,9 +198,9 @@ struct VMRemovableDrivesView: View {
         }
     }
     
-    private func clearRemovableImage(forDrive drive: Binding<UTMQemuConfigurationDrive>) {
+    private func clearRemovableImage(forDrive drive: UTMQemuConfigurationDrive) {
         data.busyWorkAsync {
-            try await vm.eject(&drive.wrappedValue)
+            try await vm.eject(drive)
         }
     }
 }

Platform/iOS/VMToolbarDriveMenuView.swift

@@ -20,32 +20,32 @@ struct VMToolbarDriveMenuView: View {
     @State var config: UTMQemuConfiguration
     @EnvironmentObject private var session: VMSessionState
     @State private var isFileImporterShown: Bool = false
-    @State private var selectedDrive: Binding<UTMQemuConfigurationDrive>?
+    @State private var selectedDrive: UTMQemuConfigurationDrive?
     @State private var isRefreshRequired: Bool = false
     
     var body: some View {
         Menu {
-            ForEach($config.drives) { $drive in
+            ForEach(config.drives) { drive in
                 if drive.isExternal {
                     Menu {
                         Button {
-                            selectedDrive = $drive
+                            selectedDrive = drive
                             isFileImporterShown.toggle()
                         } label: {
                             MenuLabel("Change…", systemImage: "opticaldisc")
                         }
                         Button {
-                            ejectDriveImage(for: $drive)
+                            ejectDriveImage(for: drive)
                         } label: {
                             MenuLabel("Eject…", systemImage: "eject")
                         }
                     } label: {
-                        MenuLabel(drive.label, systemImage: drive.imageURL == nil ? "opticaldiscdrive" : "opticaldiscdrive.fill")
+                        MenuLabel(label(for: drive), systemImage: session.vm.externalImageURL(for: drive) == nil ? "opticaldiscdrive" : "opticaldiscdrive.fill")
                     }
                 } else if drive.imageType == .disk || drive.imageType == .cd {
                     Button {
                     } label: {
-                        MenuLabel(drive.label, systemImage: "internaldrive")
+                        MenuLabel(label(for: drive), systemImage: "internaldrive")
                     }.disabled(true)
                 }
             }
@@ -65,10 +65,10 @@ struct VMToolbarDriveMenuView: View {
         }
     }
     
-    private func changeDriveImage(for driveBinding: Binding<UTMQemuConfigurationDrive>, with imageURL: URL) {
+    private func changeDriveImage(for drive: UTMQemuConfigurationDrive, with imageURL: URL) {
         Task.detached(priority: .background) {
             do {
-                try await session.vm.changeMedium(&driveBinding.wrappedValue, with: imageURL)
+                try await session.vm.changeMedium(drive, with: imageURL)
                 Task { @MainActor in
                     isRefreshRequired.toggle()
                 }
@@ -80,10 +80,10 @@ struct VMToolbarDriveMenuView: View {
         }
     }
     
-    private func ejectDriveImage(for driveBinding: Binding<UTMQemuConfigurationDrive>) {
+    private func ejectDriveImage(for drive: UTMQemuConfigurationDrive) {
         Task.detached(priority: .background) {
             do {
-                try await session.vm.eject(&driveBinding.wrappedValue)
+                try await session.vm.eject(drive)
                 Task { @MainActor in
                     isRefreshRequired.toggle()
                 }
@@ -94,6 +94,14 @@ struct VMToolbarDriveMenuView: View {
             }
         }
     }
+    
+    private func label(for drive: UTMQemuConfigurationDrive) -> String {
+        let imageURL = session.vm.externalImageURL(for: drive) ?? drive.imageURL
+        return String.localizedStringWithFormat(NSLocalizedString("%@ (%@): %@", comment: "VMToolbarDriveMenuView"),
+                                                drive.imageType.prettyValue,
+                                                drive.interface.prettyValue,
+                                                imageURL?.lastPathComponent ?? NSLocalizedString("none", comment: "VMToolbarDriveMenuView"))
+    }
 }
 
 struct VMToolbarDriveMenuView_Previews: PreviewProvider {

Platform/macOS/Display/VMDisplayQemuDisplayController.swift

@@ -109,7 +109,7 @@ class VMDisplayQemuWindowController: VMDisplayWindowController {
                 continue // skip non-disks
             }
             let item = NSMenuItem()
-            item.title = drive.label
+            item.title = label(for: drive)
             if !drive.isExternal {
                 item.isEnabled = false
             } else {
@@ -120,7 +120,7 @@ class VMDisplayQemuWindowController: VMDisplayWindowController {
                                        keyEquivalent: "")
                 eject.target = self
                 eject.tag = i
-                eject.isEnabled = drive.imageURL != nil
+                eject.isEnabled = qemuVM.externalImageURL(for: drive) != nil
                 submenu.addItem(eject)
                 let change = NSMenuItem(title: NSLocalizedString("Change", comment: "VMDisplayWindowController"),
                                         action: #selector(changeDriveImage),
@@ -141,10 +141,10 @@ class VMDisplayQemuWindowController: VMDisplayWindowController {
             logger.error("wrong sender for ejectDrive")
             return
         }
-        let config = vmQemuConfig!
+        let drive = vmQemuConfig.drives[menu.tag]
         Task.detached(priority: .background) { [self] in
             do {
-                try await qemuVM.eject(&config.drives[menu.tag])
+                try await qemuVM.eject(drive)
             } catch {
                 Task { @MainActor in
                     showErrorAlert(error.localizedDescription)
@@ -154,6 +154,7 @@ class VMDisplayQemuWindowController: VMDisplayWindowController {
     }
     
     func openDriveImage(forDriveIndex index: Int) {
+        let drive = vmQemuConfig.drives[index]
         let openPanel = NSOpenPanel()
         openPanel.title = NSLocalizedString("Select Drive Image", comment: "VMDisplayWindowController")
         openPanel.allowedContentTypes = [.data]
@@ -165,10 +166,9 @@ class VMDisplayQemuWindowController: VMDisplayWindowController {
                 logger.debug("no file selected")
                 return
             }
-            let config = self.vmQemuConfig!
             Task.detached(priority: .background) { [self] in
                 do {
-                    try await qemuVM.changeMedium(&config.drives[index], with: url)
+                    try await qemuVM.changeMedium(drive, with: url)
                 } catch {
                     Task { @MainActor in
                         showErrorAlert(error.localizedDescription)
@@ -185,6 +185,14 @@ class VMDisplayQemuWindowController: VMDisplayWindowController {
         }
         openDriveImage(forDriveIndex: menu.tag)
     }
+    
+    @nonobjc private func label(for drive: UTMQemuConfigurationDrive) -> String {
+        let imageURL = qemuVM.externalImageURL(for: drive) ?? drive.imageURL
+        return String.localizedStringWithFormat(NSLocalizedString("%@ (%@): %@", comment: "VMDisplayQemuDisplayController"),
+                                                drive.imageType.prettyValue,
+                                                drive.interface.prettyValue,
+                                                imageURL?.lastPathComponent ?? NSLocalizedString("none", comment: "VMDisplayQemuDisplayController"))
+    }
 }
 
 // MARK: - Shared folders