Apply another patch from Hamish Morrison to improve the VirtualMemory preflet.

Pretty much implements everything needed to set a swap volume once that is
implemented on the kernel side. This also improves the behavior of the Default
button.

A few changes I made:

- When the swap volume feature is turned on in the code, default to the boot
  volume if the settings file does not have one set.
- Don't show the volumes menu field in the GUI if the swap volume feature is
  not on.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@40101 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/preferences/virtualmemory/Settings.cpp

@@ -1,6 +1,7 @@
 /*
- * Copyright 2005, Axel Dörfler, axeld@pinc-software.de. All rights reserved.
+ * Copyright 2010-2011, Hamish Morrison, hamish@lavabit.com
- * Distributed under the terms of the MIT License.
+ * Copyright 2005, Axel Dörfler, axeld@pinc-software.de
+ * All rights reserved. Distributed under the terms of the MIT License.
  */
 
 
@@ -21,12 +22,12 @@
 
 static const char* kWindowSettingsFile = "VM_data";
 static const char* kVirtualMemorySettings = "virtual_memory";
+static const int64 kMegaByte = 1048576;
 
 
 Settings::Settings()
 	:
-	fPositionUpdated(false),
+	fPositionUpdated(false)
-	fSwapUpdated(false)
 {
 	ReadWindowSettings();
 	ReadSwapSettings();
@@ -50,7 +51,6 @@ Settings::ReadWindowSettings()
 		path.Append(kWindowSettingsFile);
 		BFile file;
 		if (file.SetTo(path.Path(), B_READ_ONLY) == B_OK)
-			// Now read in the data
 			if (file.Read(&fWindowPosition, sizeof(BPoint)) == sizeof(BPoint))
 				success = true;
 	}
@@ -92,38 +92,42 @@ Settings::SetWindowPosition(BPoint position)
 void
 Settings::ReadSwapSettings()
 {
-	// read current swap settings from disk
+	void* settings = load_driver_settings(kVirtualMemorySettings);
-	void* settings = load_driver_settings("virtual_memory");
 	if (settings != NULL) {
-		fSwapEnabled = get_driver_boolean_parameter(settings, "vm", false, false);
+		SetSwapEnabled(get_driver_boolean_parameter(settings, "vm", false, false));
+		const char* swapSize = get_driver_parameter(settings, "swap_size", NULL, NULL);
+		SetSwapSize(swapSize ? atoll(swapSize) : 0);
 		
-		const char* string = get_driver_parameter(settings, "swap_size", NULL, NULL);
+#ifdef SWAP_VOLUME_IMPLEMENTED
-		fSwapSize = string ? atoll(string) : 0;
+		// we need to hang onto this one
+		fBadVolName = strdup(get_driver_parameter(settings, "swap_volume", NULL, NULL));
 		
-		if (fSwapSize <= 0) {
+		BVolumeRoster volumeRoster;
-			fSwapEnabled = false;
+		BVolume temporaryVolume;
-			fSwapSize = 0;
+		
+		if (fBadVolName != NULL) {
+			status_t result = volumeRoster.GetNextVolume(&temporaryVolume);
+			char volumeName[B_FILE_NAME_LENGTH];
+			while (result != B_BAD_VALUE) {
+				temporaryVolume.GetName(volumeName);
+				if (strcmp(volumeName, fBadVolName) == 0 
+					&& temporaryVolume.IsPersistent() && volumeName[0]) {
+					SetSwapVolume(temporaryVolume);
+					break;
 				}
+				result = volumeRoster.GetNextVolume(&temporaryVolume);
+			}
+		} else
+			volumeRoster.GetBootVolume(&fSwapVolume);
+#endif
 		unload_driver_settings(settings);
-	} else {
+	} else
-		// settings are not available, try to find out what the kernel is up to
+		SetSwapNull();
-		// ToDo: introduce a kernel call for this!
-		fSwapSize = 0;
 
-		BPath path;
+#ifndef SWAP_VOLUME_IMPLEMENTED
-		if (find_directory(B_COMMON_VAR_DIRECTORY, &path) == B_OK) {
-			path.Append("swap");
-			BEntry swap(path.Path());
-			if (swap.GetSize(&fSwapSize) != B_OK)
-				fSwapSize = 0;
-		}
-
-		fSwapEnabled = fSwapSize != 0;
-	}
-
-	// ToDo: read those as well
 	BVolumeRoster volumeRoster;
 	volumeRoster.GetBootVolume(&fSwapVolume);
+#endif
 
 	fInitialSwapEnabled = fSwapEnabled;
 	fInitialSwapSize = fSwapSize;
@@ -134,7 +138,7 @@ Settings::ReadSwapSettings()
 void
 Settings::WriteSwapSettings()
 {	
-	if (!SwapChanged())
+	if (!IsRevertible())
 		return;
 
 	BPath path;
@@ -145,12 +149,24 @@ Settings::WriteSwapSettings()
 	path.Append(kVirtualMemorySettings);
 
 	BFile file;
-	if (file.SetTo(path.Path(), B_WRITE_ONLY | B_CREATE_FILE | B_ERASE_FILE) != B_OK)
+	if (file.SetTo(path.Path(), B_WRITE_ONLY | B_CREATE_FILE | B_ERASE_FILE)
+		!= B_OK)
 		return;
 	
 	char buffer[256];
+#ifdef SWAP_VOLUME_IMPLEMENTED
+	char volumeName[B_FILE_NAME_LENGTH] = {0};
+	if (SwapVolume().InitCheck() != B_NO_INIT)
+		SwapVolume().GetName(volumeName);
+	else if (fBadVolName)
+		strcpy(volumeName, fBadVolName);
+	snprintf(buffer, sizeof(buffer), "vm %s\nswap_size %Ld\nswap_volume %s\n",
+		SwapEnabled() ? "on" : "off", SwapSize(), 
+		volumeName[0] ? volumeName : NULL);
+#else
 	snprintf(buffer, sizeof(buffer), "vm %s\nswap_size %Ld\n",
 		fSwapEnabled ? "on" : "off", fSwapSize);
+#endif
 
 	file.Write(buffer, strlen(buffer));
 }
@@ -173,7 +189,7 @@ Settings::SetSwapSize(off_t size)
 void 
 Settings::SetSwapVolume(BVolume &volume)
 {
-	if (volume.Device() == fSwapVolume.Device()
+	if (volume.Device() == SwapVolume().Device()
 		|| volume.InitCheck() != B_OK)
 		return;
 
@@ -182,16 +198,14 @@ Settings::SetSwapVolume(BVolume &volume)
 
 
 void
-Settings::SetSwapDefaults()
+Settings::SetSwapNull()
 {
-	fSwapEnabled = true;
+	SetSwapEnabled(false);
-
 	BVolumeRoster volumeRoster;
-	volumeRoster.GetBootVolume(&fSwapVolume);
+	BVolume temporaryVolume;
-
+	volumeRoster.GetBootVolume(&temporaryVolume);
-	system_info info;
+	SetSwapVolume(temporaryVolume);
-	get_system_info(&info);
+	SetSwapSize(0);
-	fSwapSize = (off_t)info.max_pages * B_PAGE_SIZE;
 }
 
 
@@ -205,18 +219,9 @@ Settings::RevertSwapChanges()
 
 
 bool
-Settings::IsDefaultable()
+Settings::IsRevertible()
-{
-	return fSwapEnabled != fInitialSwapEnabled
-		|| fSwapSize != fInitialSwapSize;
-}
-
-
-bool
-Settings::SwapChanged()
 {
 	return fSwapEnabled != fInitialSwapEnabled
 		|| fSwapSize != fInitialSwapSize
 		|| fSwapVolume.Device() != fInitialSwapVolume;
 }
-

src/preferences/virtualmemory/Settings.h

@@ -25,12 +25,11 @@ class Settings {
 		void SetSwapSize(off_t size);
 		void SetSwapVolume(BVolume& volume);
 
-		void SetSwapDefaults();
 		void RevertSwapChanges();
-		bool IsDefaultable();
+		bool IsRevertible();
-		bool SwapChanged();
 
 	private:
+		void SetSwapNull();
 		void ReadWindowSettings();
 		void WriteWindowSettings();
 
@@ -47,7 +46,8 @@ class Settings {
 		off_t		fInitialSwapSize;
 		dev_t		fInitialSwapVolume;
 
-		bool		fPositionUpdated, fSwapUpdated;
+		bool		fPositionUpdated;
+		const char*	fBadVolName;
 };
 
 #endif	/* SETTINGS_H */

src/preferences/virtualmemory/SettingsWindow.cpp

@@ -1,6 +1,7 @@
 /*
- * Copyright 2005-2009, Axel Dörfler, axeld@pinc-software.de. All rights reserved.
+ * Copyright 2010-2011, Hamish Morrison, hamish@lavabit.com 
- * Distributed under the terms of the MIT License.
+ * Copyright 2005-2009, Axel Dörfler, axeld@pinc-software.de
+ * All rights reserved. Distributed under the terms of the MIT License.
  */
 
 
@@ -30,7 +31,6 @@
 
 #include <stdio.h>
 
-
 #undef B_TRANSLATE_CONTEXT
 #define B_TRANSLATE_CONTEXT "SettingsWindow"
 
@@ -39,6 +39,8 @@ static const uint32 kMsgDefaults = 'dflt';
 static const uint32 kMsgRevert = 'rvrt';
 static const uint32 kMsgSliderUpdate = 'slup';
 static const uint32 kMsgSwapEnabledUpdate = 'swen';
+static const uint32 kMsgVolumeSelected = 'vlsl';
+static const int64 kMegaByte = 1048576;
 
 
 class SizeSlider : public BSlider {
@@ -54,10 +56,21 @@ class SizeSlider : public BSlider {
 };
 
 
-static const int64 kMegaByte = 1048576;
+SizeSlider::SizeSlider(const char* name, const char* label,
+	BMessage* message, int32 min, int32 max, uint32 flags)
+	: BSlider(name, label, message, min, max, B_HORIZONTAL, B_BLOCK_THUMB, flags)
+{
+	rgb_color color = ui_color(B_CONTROL_HIGHLIGHT_COLOR);
+	UseFillColor(true, &color);
+}
 
 
-const char *
+SizeSlider::~SizeSlider()
+{
+}
+
+
+const char*
 byte_string(int64 size)
 {
 	double value = 1. * size;
@@ -80,7 +93,7 @@ byte_string(int64 size)
 		} while (value >= 1024 && units[i + 1]);
 
 		off_t rounded = off_t(value * 100LL);
-		sprintf(string, "%g %s", rounded / 100.0,
+		snprintf(string, sizeof(string), "%g %s", rounded / 100.0,
 			B_TRANSLATE_NOCOLLECT(units[i]));
 	}
 
@@ -88,39 +101,42 @@ byte_string(int64 size)
 }
 
 
-//	#pragma mark -
+const char*
-
-
-SizeSlider::SizeSlider(const char* name, const char* label,
-	BMessage* message, int32 min, int32 max, uint32 flags)
-	: BSlider(name, label, message, min, max, B_HORIZONTAL, B_BLOCK_THUMB, flags)
-{
-	rgb_color color = ui_color(B_CONTROL_HIGHLIGHT_COLOR);
-	UseFillColor(true, &color);
-}
-
-
-SizeSlider::~SizeSlider()
-{
-}
-
-
-const char *
 SizeSlider::UpdateText() const
 {
 	fText = byte_string(Value() * kMegaByte);
-
 	return fText.String();
 }
 
 
-//	#pragma mark -
+class VolumeMenuItem : public BMenuItem {
+	public:
+		VolumeMenuItem(const char* label, BMessage* message, BVolume* volume);
+		virtual ~VolumeMenuItem();
+		BVolume* fVolume;
+};
+
+
+VolumeMenuItem::VolumeMenuItem(const char* label, BMessage* message,
+	BVolume* volume)
+	: BMenuItem(label, message)
+{
+	fVolume = volume;
+}
+
+
+VolumeMenuItem::~VolumeMenuItem()
+{
+}
 
 
 SettingsWindow::SettingsWindow()
-	: BWindow(BRect(0, 0, 269, 172), B_TRANSLATE("VirtualMemory"),
+	:
+	BWindow(BRect(0, 0, 269, 172), B_TRANSLATE("VirtualMemory"),
 		B_TITLED_WINDOW, B_NOT_RESIZABLE | B_ASYNCHRONOUS_CONTROLS
-			| B_NOT_ZOOMABLE | B_AUTO_UPDATE_SIZE_LIMITS)
+		| B_NOT_ZOOMABLE | B_AUTO_UPDATE_SIZE_LIMITS),
+	fLocked(false)
+	
 {
 	BView* view = new BGroupView();
 
@@ -142,34 +158,29 @@ SettingsWindow::SettingsWindow()
 	string << byte_string(fSettings.SwapSize());
 	BStringView* swapfileView = new BStringView("current swap size", string.String());
 
-	BPopUpMenu* menu = new BPopUpMenu("volumes");
+	BPopUpMenu* menu = new BPopUpMenu("invalid");
 
 	// collect volumes
 	// TODO: listen to volume changes!
 	// TODO: accept dropped volumes
 
 	BVolumeRoster volumeRoster;
-	BVolume volume;
+	BVolume* volume = new BVolume();
-	while (volumeRoster.GetNextVolume(&volume) == B_OK) {
 	char name[B_FILE_NAME_LENGTH];
-		if (!volume.IsPersistent() || volume.GetName(name) != B_OK || !name[0])
+	while (volumeRoster.GetNextVolume(volume) == B_OK) {	
+		if (!volume->IsPersistent() || volume->GetName(name) != B_OK || !name[0])
 			continue;
-
+		VolumeMenuItem* item = new VolumeMenuItem(name, 
-		BMenuItem* item = new BMenuItem(name, NULL);
+			new BMessage(kMsgVolumeSelected), volume);
 		menu->AddItem(item);
-
+		volume = new BVolume();
-		if (volume.Device() == fSettings.SwapVolume().Device())
-			item->SetMarked(true);
 	}
 
-	fVolumeMenuField = new BMenuField("devices", B_TRANSLATE("Use volume:"), menu);
+	fVolumeMenuField = new BMenuField("volumes", B_TRANSLATE("Use volume:"), menu);
-	
-	// When swap volume changing support is implemeneted, remove me:
-	fVolumeMenuField->SetEnabled(false);
 	
 	fSizeSlider = new SizeSlider("size slider", 
 		B_TRANSLATE("Requested swap file size:"), new BMessage(kMsgSliderUpdate),
-		0, 0, B_WILL_DRAW | B_FRAME_EVENTS);
+		1, 1, B_WILL_DRAW | B_FRAME_EVENTS);
 	fSizeSlider->SetViewColor(255, 0, 255);
 
 	fWarningStringView = new BStringView("", "");
@@ -185,21 +196,22 @@ SettingsWindow::SettingsWindow()
 			.Add(swapfileView)
 			.AddGlue()
 		.End()
+#ifdef SWAP_VOLUME_IMPLEMENTED
 		.AddGroup(B_HORIZONTAL)
 			.Add(fVolumeMenuField)
 			.AddGlue()
 		.End()
+#else
+		.AddGlue()
+#endif
 		.Add(fSizeSlider)
 		.Add(fWarningStringView)
 		.SetInsets(10, 10, 10, 10)
 	);
 	box->AddChild(view);
 
-	// Add "Defaults" and "Revert" buttons
-
 	fDefaultsButton = new BButton("defaults", B_TRANSLATE("Defaults"),
 		new BMessage(kMsgDefaults));
-	fDefaultsButton->SetEnabled(fSettings.IsDefaultable());
 
 	fRevertButton = new BButton("revert", B_TRANSLATE("Revert"),
 		new BMessage(kMsgRevert));
@@ -216,15 +228,34 @@ SettingsWindow::SettingsWindow()
 		.SetInsets(10, 10, 10, 10)
 	);
 
-	_Update();
-
 	BScreen screen;
 	BRect screenFrame = screen.Frame();
-
 	if (!screenFrame.Contains(fSettings.WindowPosition()))
 		CenterOnScreen();
 	else
 		MoveTo(fSettings.WindowPosition());
+
+#ifdef SWAP_VOLUME_IMPLEMENTED
+	// Validate the volume specified in settings file
+	status_t result = fSettings.SwapVolume().InitCheck();
+
+	if (result == B_NO_INIT) {
+		int32 choice = (new BAlert("VirtualMemory",	B_TRANSLATE(
+			"The swap volume specified in the settings file is invalid.\n"
+			"You can keep the current setting or switch to the "
+			"default swap volume."),
+			B_TRANSLATE("Keep"), B_TRANSLATE("Switch"), NULL,
+			B_WIDTH_AS_USUAL, B_WARNING_ALERT))->Go();
+		if (choice == 1) {
+			BVolumeRoster volumeRoster;
+			BVolume bootVolume;
+			volumeRoster.GetBootVolume(&bootVolume);
+			fSettings.SetSwapVolume(bootVolume);
+		}
+	}
+#endif
+
+	_Update();
 }
 
 
@@ -239,55 +270,67 @@ SettingsWindow::_Update()
 	if ((fSwapEnabledCheckBox->Value() != 0) != fSettings.SwapEnabled())
 		fSwapEnabledCheckBox->SetValue(fSettings.SwapEnabled());
 
-	if (fSizeSlider->IsEnabled() != fSettings.SwapEnabled())
-		fSizeSlider->SetEnabled(fSettings.SwapEnabled());
-	
 #ifdef SWAP_VOLUME_IMPLEMENTED	
 	if (fVolumeMenuField->IsEnabled() != fSettings.SwapEnabled())
 		fVolumeMenuField->SetEnabled(fSettings.SwapEnabled());
+	VolumeMenuItem* selectedVolumeItem =
+		(VolumeMenuItem*)fVolumeMenuField->Menu()->FindMarked();
+	if (selectedVolumeItem == NULL) {
+		VolumeMenuItem* currentVolumeItem;
+		int32 items = fVolumeMenuField->Menu()->CountItems();
+		for (int32 index = 0; index < items; ++index) {
+			currentVolumeItem = ((VolumeMenuItem*)fVolumeMenuField->Menu()->ItemAt(index));
+			if (*(currentVolumeItem->fVolume) == fSettings.SwapVolume()) {
+				currentVolumeItem->SetMarked(true);
+				break;
+			}
+		}
+	} else if (*selectedVolumeItem->fVolume != fSettings.SwapVolume()) {
+		VolumeMenuItem* currentVolumeItem;
+		int32 items = fVolumeMenuField->Menu()->CountItems();
+		for (int32 index = 0; index < items; ++index) {
+			currentVolumeItem = ((VolumeMenuItem*)fVolumeMenuField->Menu()->ItemAt(index));
+			if (*(currentVolumeItem->fVolume) == fSettings.SwapVolume()) {
+				currentVolumeItem->SetMarked(true);
+				break;
+			}
+		}
+	}
 #endif
 
+	fWarningStringView->SetText("");
+	fLocked = false;
+	
+	if (fSettings.IsRevertible())
+			fWarningStringView->SetText(
+				B_TRANSLATE("Changes will take effect on restart!"));
+	if (fRevertButton->IsEnabled() != fSettings.IsRevertible())
+		fRevertButton->SetEnabled(fSettings.IsRevertible());
+	
 	off_t minSize, maxSize;
 	if (_GetSwapFileLimits(minSize, maxSize) == B_OK) {
+		// round to nearest MB -- slider steps in whole MBs
+		(minSize >>= 20) <<= 20;
+		(maxSize >>= 20) <<= 20;
 		BString minLabel, maxLabel;
 		minLabel << byte_string(minSize);
 		maxLabel << byte_string(maxSize);
 		if (minLabel != fSizeSlider->MinLimitLabel()
 			|| maxLabel != fSizeSlider->MaxLimitLabel()) {
 			fSizeSlider->SetLimitLabels(minLabel.String(), maxLabel.String());
-#ifdef __HAIKU__
 			fSizeSlider->SetLimits(minSize / kMegaByte, maxSize / kMegaByte);
-#endif
 		}
-
+	} else if (fSettings.SwapEnabled()) {
-		if (fSizeSlider->Value() != fSettings.SwapSize() / kMegaByte)
-			fSizeSlider->SetValue(fSettings.SwapSize() / kMegaByte);
-	} else {
-		// Not enough space on volume for a swap file. Make UI inoperable.
 		fWarningStringView->SetText(
 			B_TRANSLATE("Insufficient space for a swap file."));
-		fSwapEnabledCheckBox->SetEnabled(false);
+		fLocked = true;
-		fSizeSlider->SetEnabled(false);
-		// When swap volume is implemented, we'll want to keep the volume
-		// menu field enabled so the user can get around the space issue
-		// by selecting a different volume.
-#ifdef SWAP_VOLUME_IMPLEMENTED
-		fVolumeMenuField->SetEnabled(true);
-#endif
 	}
-
+	if (fSizeSlider->Value() != fSettings.SwapSize() / kMegaByte)
-	// ToDo: set volume
+		fSizeSlider->SetValue(fSettings.SwapSize() / kMegaByte);
-
+	if (fSizeSlider->IsEnabled() != fSettings.SwapEnabled() || fLocked)
-	fDefaultsButton->SetEnabled(fSettings.IsDefaultable());
+	{
-
+		fSizeSlider->SetEnabled(fSettings.SwapEnabled() && !fLocked);
-	bool changed = fSettings.SwapChanged();
+		fSettings.SetSwapSize((off_t)fSizeSlider->Value() * kMegaByte);
-	if (fRevertButton->IsEnabled() != changed) {
-		fRevertButton->SetEnabled(changed);
-		if (changed)
-			fWarningStringView->SetText(
-				B_TRANSLATE("Changes will take effect on restart!"));
-		else
-			fWarningStringView->SetText("");
 	}
 }
 
@@ -332,6 +375,30 @@ SettingsWindow::_GetSwapFileLimits(off_t& minSize, off_t& maxSize)
 }
 
 
+void
+SettingsWindow::SetSwapDefaults()
+{
+	fSettings.SetSwapEnabled(true);
+
+	BVolumeRoster volumeRoster;
+	BVolume temporaryVolume;
+	volumeRoster.GetBootVolume(&temporaryVolume);
+	fSettings.SetSwapVolume(temporaryVolume);
+
+	system_info info;
+	get_system_info(&info);
+	
+	off_t defaultSize = (off_t)info.max_pages * B_PAGE_SIZE;
+	off_t minSize, maxSize;
+	_GetSwapFileLimits(minSize, maxSize);
+	
+	if (defaultSize > maxSize / 2)
+		defaultSize = maxSize / 2;
+		
+	fSettings.SetSwapSize(defaultSize);
+}
+
+
 void
 SettingsWindow::MessageReceived(BMessage* message)
 {
@@ -341,13 +408,18 @@ SettingsWindow::MessageReceived(BMessage* message)
 			_Update();
 			break;
 		case kMsgDefaults:
-			fSettings.SetSwapDefaults();
+			SetSwapDefaults();
 			_Update();
 			break;
 		case kMsgSliderUpdate:
 			fSettings.SetSwapSize((off_t)fSizeSlider->Value() * kMegaByte);
 			_Update();
 			break;
+		case kMsgVolumeSelected:
+			fSettings.SetSwapVolume(*((VolumeMenuItem*)fVolumeMenuField->Menu()
+				->FindMarked())->fVolume);
+			_Update();
+			break;
 		case kMsgSwapEnabledUpdate:
 		{
 			int32 value;
@@ -374,14 +446,7 @@ SettingsWindow::MessageReceived(BMessage* message)
 				}
 			}
 
-			bool enabled = value != 0;
+			fSettings.SetSwapEnabled(value != 0);
-			fSettings.SetSwapEnabled(enabled);
-			if (enabled && fSettings.SwapSize() == 0) {
-				off_t min;
-				off_t max;
-				_GetSwapFileLimits(min, max);
-				fSettings.SetSwapSize(min);
-			}
 			_Update();
 			break;
 		}

src/preferences/virtualmemory/SettingsWindow.h

@@ -27,6 +27,7 @@ class SettingsWindow : public BWindow {
 	private:
 		void _Update();
 		status_t _GetSwapFileLimits(off_t& minSize, off_t& maxSize);
+		void SetSwapDefaults();
 
 		BCheckBox*		fSwapEnabledCheckBox;
 		BSlider*		fSizeSlider;
@@ -35,6 +36,8 @@ class SettingsWindow : public BWindow {
 		BStringView*	fWarningStringView;
 		BMenuField*		fVolumeMenuField;
 		Settings		fSettings;
+		
+		bool fLocked;
 };
 
 #endif	/* SETTINGS_WINDOW_H */