Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sparse image file cause file system corrputed #5869

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
4 changes: 4 additions & 0 deletions Configuration/Legacy/UTMLegacyAppleConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -313,13 +313,15 @@ struct DiskImage: Codable, Hashable, Identifiable {

var sizeMib: Int
var isReadOnly: Bool
var isSparse: Bool
var isExternal: Bool
var imageURL: URL?
private var uuid = UUID() // for identifiable

private enum CodingKeys: String, CodingKey {
case sizeMib
case isReadOnly
case isSparse
case isExternal
case imagePath
case imageBookmark
Expand All @@ -344,6 +346,7 @@ struct DiskImage: Codable, Hashable, Identifiable {
let container = try decoder.container(keyedBy: CodingKeys.self)
sizeMib = try container.decode(Int.self, forKey: .sizeMib)
isReadOnly = try container.decode(Bool.self, forKey: .isReadOnly)
isSparse = try container.decode(Bool.self, forKey: .isSparse)
isExternal = try container.decode(Bool.self, forKey: .isExternal)
if !isExternal, let imagePath = try container.decodeIfPresent(String.self, forKey: .imagePath) {
imageURL = dataURL.appendingPathComponent(imagePath)
Expand All @@ -357,6 +360,7 @@ struct DiskImage: Codable, Hashable, Identifiable {
var container = encoder.container(keyedBy: CodingKeys.self)
try container.encode(sizeMib, forKey: .sizeMib)
try container.encode(isReadOnly, forKey: .isReadOnly)
try container.encode(isSparse, forKey: .isSparse)
try container.encode(isExternal, forKey: .isExternal)
if !isExternal {
try container.encodeIfPresent(imageURL?.lastPathComponent, forKey: .imagePath)
Expand Down
26 changes: 25 additions & 1 deletion Configuration/UTMAppleConfigurationDrive.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ struct UTMAppleConfigurationDrive: UTMConfigurationDrive {

var sizeMib: Int = 0
var isReadOnly: Bool
var isSparse: Bool
var isExternal: Bool
var imageURL: URL?
var imageName: String?
Expand All @@ -36,6 +37,7 @@ struct UTMAppleConfigurationDrive: UTMConfigurationDrive {

private enum CodingKeys: String, CodingKey {
case isReadOnly = "ReadOnly"
case isSparse = "Sparse"
case imageName = "ImageName"
case bookmark = "Bookmark" // legacy only
case identifier = "Identifier"
Expand All @@ -54,13 +56,22 @@ struct UTMAppleConfigurationDrive: UTMConfigurationDrive {
init(newSize: Int) {
sizeMib = newSize
isReadOnly = false
isSparse = true
isExternal = false
}

init(newSize: Int, isSparse: Bool = true) {
sizeMib = newSize
isReadOnly = false
isExternal = false
self.isSparse = isSparse
}

init(existingURL url: URL?, isExternal: Bool = false) {
self.imageURL = url
self.isReadOnly = isExternal
self.isExternal = isExternal
self.isSparse = true
}

init(from decoder: Decoder) throws {
Expand All @@ -83,6 +94,7 @@ struct UTMAppleConfigurationDrive: UTMConfigurationDrive {
isExternal = true
}
isReadOnly = try container.decodeIfPresent(Bool.self, forKey: .isReadOnly) ?? isExternal
isSparse = try container.decodeIfPresent(Bool.self, forKey: .isSparse) ?? true
id = try container.decode(String.self, forKey: .identifier)
}

Expand All @@ -92,12 +104,22 @@ struct UTMAppleConfigurationDrive: UTMConfigurationDrive {
try container.encodeIfPresent(imageName, forKey: .imageName)
}
try container.encode(isReadOnly, forKey: .isReadOnly)
try container.encode(isSparse, forKey: .isSparse)
try container.encode(id, forKey: .identifier)
}

func vzDiskImage() throws -> VZDiskImageStorageDeviceAttachment? {
if let imageURL = imageURL {
return try VZDiskImageStorageDeviceAttachment(url: imageURL, readOnly: isReadOnly)
if #available(macOS 12, *) {
/*
* virtual disk cache mode have bugs,
* when it is disabled or set to auto (default value)
* may cause linux file system corrputed, especially in the case of heavy IO loads
*/
return try VZDiskImageStorageDeviceAttachment(url: imageURL, readOnly: isReadOnly, cachingMode:VZDiskImageCachingMode.cached, synchronizationMode: VZDiskImageSynchronizationMode.full)
} else {
return try VZDiskImageStorageDeviceAttachment(url: imageURL, readOnly: isReadOnly)
}
} else {
return nil
}
Expand All @@ -107,6 +129,7 @@ struct UTMAppleConfigurationDrive: UTMConfigurationDrive {
imageName?.hash(into: &hasher)
sizeMib.hash(into: &hasher)
isReadOnly.hash(into: &hasher)
isSparse.hash(into: &hasher)
isExternal.hash(into: &hasher)
id.hash(into: &hasher)
}
Expand All @@ -126,6 +149,7 @@ extension UTMAppleConfigurationDrive {
init(migrating oldDrive: DiskImage) {
sizeMib = oldDrive.sizeMib
isReadOnly = oldDrive.isReadOnly
isSparse = oldDrive.isSparse
isExternal = oldDrive.isExternal
imageURL = oldDrive.imageURL
}
Expand Down
31 changes: 28 additions & 3 deletions Configuration/UTMConfigurationDrive.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ protocol UTMConfigurationDrive: Codable, Hashable, Identifiable {
/// If true, the drive image will be mounted as read-only.
var isReadOnly: Bool { get }

/// If true, the drive image is sparse file.
var isSparse: Bool { get }

/// If true, a bookmark is stored in the package.
var isExternal: Bool { get }

Expand Down Expand Up @@ -77,7 +80,7 @@ extension UTMConfigurationDrive {
throw UTMConfigurationError.driveAlreadyExists(newURL)
}
if isRawImage {
try await createRawImage(at: newURL, size: sizeMib)
try await createRawImage(at: newURL, size: sizeMib, sparse: isSparse)
} else {
try await createQcow2Image(at: newURL, size: sizeMib)
}
Expand All @@ -90,14 +93,36 @@ extension UTMConfigurationDrive {
}
}

private func createRawImage(at newURL: URL, size sizeMib: Int) async throws {
private func createRawImage(at newURL: URL, size sizeMib: Int, sparse isSparse: Bool) async throws {
let size = UInt64(sizeMib) * bytesInMib
try await Task.detached {
guard FileManager.default.createFile(atPath: newURL.path, contents: nil, attributes: nil) else {
throw UTMConfigurationError.cannotCreateDiskImage
}
let handle = try FileHandle(forWritingTo: newURL)
try handle.truncate(atOffset: size)
if(isSparse) {
/* truncate command make a sparse file, the space will not alloc before really used
* this should be better at most time.
* but maybe not suitable for virtual machines, especially in the case of heavy IO loads
* this may give extra time delay and operational interruptions when system do really space alloc
* this behavior may cause later write completed before previous write
* the incorrect write order may cause file system corrputed
*/
try handle.truncate(atOffset: size)
} else {
var val = 0
let scale = 100; // write large block will be faster, but too large may cause OOM
let data = NSMutableData(length: NSNumber(value: bytesInMib).intValue * scale) // 100MB
while val < (sizeMib / scale) {
try handle.write(contentsOf: data!)
val += 1;
}
val = sizeMib % scale
if(val > 0) {
val = val * NSNumber(value: bytesInMib).intValue
try handle.write(contentsOf: data!.subdata(with: NSRange(location: 0, length: val)))
}
}
try handle.close()
}.value
}
Expand Down
7 changes: 7 additions & 0 deletions Configuration/UTMQemuConfigurationDrive.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ struct UTMQemuConfigurationDrive: UTMConfigurationDrive {
/// If true, the drive image will be mounted as read-only.
var isReadOnly: Bool = false

/// If true, the drive image is sparse file.
var isSparse: Bool = true

/// If true, a bookmark is stored in the package.
var isExternal: Bool = false

Expand Down Expand Up @@ -60,6 +63,7 @@ struct UTMQemuConfigurationDrive: UTMConfigurationDrive {
case interfaceVersion = "InterfaceVersion"
case identifier = "Identifier"
case isReadOnly = "ReadOnly"
case isSparse = "Sparse"
}

init() {
Expand All @@ -78,6 +82,7 @@ struct UTMQemuConfigurationDrive: UTMConfigurationDrive {
isExternal = true
}
isReadOnly = try values.decodeIfPresent(Bool.self, forKey: .isReadOnly) ?? isExternal
isSparse = try values.decodeIfPresent(Bool.self, forKey: .isSparse) ?? true
imageType = try values.decode(QEMUDriveImageType.self, forKey: .imageType)
interface = try values.decode(QEMUDriveInterface.self, forKey: .interface)
interfaceVersion = try values.decodeIfPresent(Int.self, forKey: .interfaceVersion) ?? 0
Expand All @@ -90,6 +95,7 @@ struct UTMQemuConfigurationDrive: UTMConfigurationDrive {
try container.encodeIfPresent(imageURL?.lastPathComponent, forKey: .imageName)
}
try container.encode(isReadOnly, forKey: .isReadOnly)
try container.encode(isSparse, forKey: .isSparse)
try container.encode(imageType, forKey: .imageType)
if imageType == .cd || imageType == .disk {
try container.encode(interface, forKey: .interface)
Expand All @@ -104,6 +110,7 @@ struct UTMQemuConfigurationDrive: UTMConfigurationDrive {
imageName?.hash(into: &hasher)
sizeMib.hash(into: &hasher)
isReadOnly.hash(into: &hasher)
isSparse.hash(into: &hasher)
isExternal.hash(into: &hasher)
id.hash(into: &hasher)
imageType.hash(into: &hasher)
Expand Down
12 changes: 11 additions & 1 deletion Platform/Shared/VMWizardDrivesView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,14 @@ import SwiftUI

struct VMWizardDrivesView: View {
@ObservedObject var wizardState: VMWizardState

var allocNow: Binding<Bool> {
return Binding(get: {
return wizardState.allocateAllDiskSpaceNow
}, set: { newValue in
wizardState.allocateAllDiskSpaceNow = newValue
})
}

var body: some View {
VMWizardContent("Storage") {
Section {
Expand All @@ -34,6 +41,9 @@ struct VMWizardDrivesView: View {
.frame(maxWidth: 50)
Text("GB")
}
Toggle(isOn: allocNow, label: {
Text("Allocate all disk space now")
}).help("If checked, allocate all disk space immediately rather than allow the disk space to gradually grow to the maximum amount.")
} header: {
Text("Size")
}
Expand Down
4 changes: 3 additions & 1 deletion Platform/Shared/VMWizardState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ enum VMWizardOS: String, Identifiable {
@Published var systemMemoryMib: Int = 512
@Published var storageSizeGib: Int = 8
#endif
@Published var allocateAllDiskSpaceNow = false

@Published var systemCpuCount: Int = 0
@Published var isGLEnabled: Bool = false
@Published var sharingDirectoryURL: URL?
Expand Down Expand Up @@ -324,7 +326,7 @@ enum VMWizardOS: String, Identifiable {
}
}
if !isSkipDiskCreate {
config.drives.append(UTMAppleConfigurationDrive(newSize: storageSizeGib * bytesInGib / bytesInMib))
config.drives.append(UTMAppleConfigurationDrive(newSize: storageSizeGib * bytesInGib / bytesInMib, isSparse: !allocateAllDiskSpaceNow))
}
if #available(macOS 12, *), let sharingDirectoryURL = sharingDirectoryURL {
config.sharedDirectories = [UTMAppleConfigurationSharedDirectory(directoryURL: sharingDirectoryURL, isReadOnly: sharingReadOnly)]
Expand Down
11 changes: 11 additions & 0 deletions Platform/macOS/VMConfigAppleDriveCreateView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,24 @@ struct VMConfigAppleDriveCreateView: View {

@Binding var config: UTMAppleConfigurationDrive
@State private var isGiB: Bool = true
var allocNow: Binding<Bool> {
return Binding(get: {
return !config.isSparse
}, set: { newValue in
config.isSparse = !newValue
})
}

var body: some View {

Form {
VStack {
Toggle(isOn: $config.isExternal.animation(), label: {
Text("Removable")
}).help("If checked, the drive image will be stored with the VM.")
Toggle(isOn: allocNow, label: {
Text("Allocate all disk space now")
}).help("If checked, allocate all disk space immediately rather than allow the disk space to gradually grow to the maximum amount.")
.onChange(of: config.isExternal) { newValue in
if newValue {
config.sizeMib = 0
Expand Down
2 changes: 2 additions & 0 deletions Platform/zh-Hans.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,8 @@

/* VMRemovableDrivesView */
"Removable" = "可移除";
"Allocate all disk space now" = "立即分配所有磁盘空间";
"If checked, allocate all disk space immediately rather than allow the disk space to gradually grow to the maximum amount." = "立即分配所有磁盘空间,而不是允许磁盘空间逐渐增长到最大。";

/* No comment provided by engineer. */
"Removable Drive" = "可移除驱动器";
Expand Down