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

storaged: support creating btrfs snapshots #20562

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions pkg/storaged/btrfs/get-path-uuid.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import os
import os.path
import subprocess
import sys


def main(path):
# If the path does not exist, we will create but need to verify it lives on the same volume
if not os.path.exists(path):
if path.endswith('/'):
path = path.rstrip('/')
path = os.path.dirname(path)

# bail out if the parent path is not found
if not os.path.exists(path):
sys.exit(2)

try:
sys.stdout.write(subprocess.check_output(["findmnt", "--output", "UUID,fstype", "--json", "--target", path]).decode().strip())
except subprocess.SubprocessError as exc:
print(exc, file=sys.stderr)
sys.exit(3)


if __name__ == "__main__":
if len(sys.argv) != 2:
sys.stderr.write("Path not provided\n")
sys.exit(1)

main(sys.argv[1])
154 changes: 152 additions & 2 deletions pkg/storaged/btrfs/subvolume.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@ import {
get_fstab_config_with_client, reload_systemd, extract_option, parse_options,
flatten, teardown_active_usage,
} from "../utils.js";
import { btrfs_usage, validate_subvolume_name, parse_subvol_from_options } from "./utils.jsx";
import { btrfs_usage, validate_subvolume_name, parse_subvol_from_options, validate_snapshots_location } from "./utils.jsx";
import { at_boot_input, update_at_boot_input, mounting_dialog, mount_options } from "../filesystem/mounting-dialog.jsx";
import {
dialog_open, TextInput,
dialog_open, TextInput, CheckBoxes,
TeardownMessage, init_teardown_usage,
SelectOneRadioVerticalTextInput,
} from "../dialog.jsx";
import { check_mismounted_fsys, MismountAlert } from "../filesystem/mismounting.jsx";
import {
Expand Down Expand Up @@ -180,6 +181,149 @@ function subvolume_create(volume, subvol, parent_dir) {
});
}

async function snapshot_create(volume, subvol, subvolume_path) {
const localstorage_key = "storage:snapshot-locations";
console.log(volume, subvol, subvolume_path);
const action_variants = [
{ tag: null, Title: _("Create snapshot") },
];

const get_local_storage_snapshots_locs = () => {
const localstorage_snapshot_data = localStorage.getItem(localstorage_key);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using the location of the most recently made snapshot? By looking at the list of snapshots of this subvolume. That makes this independent from browser storage.

if (localstorage_snapshot_data === null)
return null;

try {
return JSON.parse(localstorage_snapshot_data);
} catch (err) {
console.warn("localstorage btrfs snapshot locations data malformed", localstorage_snapshot_data);
return null;
}
};

const get_localstorage_snapshot_location = subvol => {
const snapshot_locations = get_local_storage_snapshots_locs();
if (snapshot_locations != null)
return snapshot_locations[subvol.id] || null;
return snapshot_locations;
};

const get_current_date = async () => {
const out = await cockpit.spawn(["date", "+%s"]);
const now = parseInt(out.trim()) * 1000;
const d = new Date(now);
d.setSeconds(0);
d.setMilliseconds(0);
return d;
};

const folder_exists = async (path) => {
// Check if path exist and can be created
try {
await cockpit.spawn(["test", "-d", path]);
return true;
} catch {
return false;
}
};

const date = await get_current_date();
// Convert dates to ISO-8601
const current_date = date.toISOString().split("T")[0];
const current_date_time = date.toISOString().replace(":00.000Z", "");
const choices = [
{
value: "current_date",
title: cockpit.format(_("Current date $0"), current_date),
},
{
value: "current_date_time",
title: cockpit.format(_("Current date and time $0"), current_date_time),
},
{
value: "custom_name",
title: _("Custom name"),
type: "radioWithInput",
},
];

const get_snapshot_name = (vals) => {
let snapshot_name = "";
if (vals.snapshot_name.checked == "current_date") {
snapshot_name = current_date;
} else if (vals.snapshot_name.checked === "current_date_time") {
snapshot_name = current_date_time;
} else if (vals.snapshot_name.checked === "custom_name") {
snapshot_name = vals.snapshot_name.inputs.custom_name;
}
return snapshot_name;
};

dialog_open({
Title: _("Create snapshot"),
Fields: [
TextInput("subvolume", _("Subvolume"),
{
value: subvol.pathname,
disabled: true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The is confusing, imo. Usually we put the subject of a dialog into the title, like "Permanently delete subvolume foo/bar?" so I would expect "Create snapshot of foo/bar" as the title instead of this disabled text field.

My immediate thought when seeing it in the dialog was "why is it disabled? what can I do to enable it?" and there are no good answers to those questions, I guess.

The mockups have a static text instead of a disabled textinput, which avoids those questions. I think that's much better.

I like this new pattern of putting the subject of a dialog into the list of input fields, as a static text. However, it's still a new pattern and it feels random to introduce it here, and make this dialog different from all others in this regard.

}),
TextInput("snapshots_location", _("Snapshots location"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be a autocomplete thing, offering the mount points of all mounted subvolumes.

{
value: get_localstorage_snapshot_location(subvol),
placeholder: cockpit.format(_("Example, $0"), "/.snapshots"),
explanation: (<>
<p>{_("Snapshots must reside within the same btrfs volume.")}</p>
<p>{_("When the snapshot location does not exist, it will be created as btrfs subvolume automatically.")}</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, why not as a directory? Also, non existing parents should be created as well, no?

</>),
validate: path => validate_snapshots_location(path, volume),
}),
SelectOneRadioVerticalTextInput("snapshot_name", _("Snapshot name"),
{
value: { checked: "current_date", inputs: { } },
choices,
validate: (val, _values, _variant) => {
if (val.checked === "custom_name")
return validate_subvolume_name(val.inputs.custom_name);
}
}),

CheckBoxes("readonly", _("Option"),
{
fields: [
{ tag: "on", title: _("Read-only") }
],
}),
],
Action: {
Variants: action_variants,
action: async function (vals) {
// Create snapshot location if it does not exists
console.log("values", vals);
const exists = await folder_exists(vals.snapshots_location);
if (!exists) {
await cockpit.spawn(["btrfs", "subvolume", "create", vals.snapshots_location], { superuser: "require", err: "message" });
}

// HACK: cannot use block_btrfs.CreateSnapshot as it always creates a subvolume relative to MountPoints[0] which
// makes it impossible to handle a situation where we have multiple subvolumes mounted.
// https://github.com/storaged-project/udisks/issues/1242
const cmd = ["btrfs", "subvolume", "snapshot"];
if (vals.readonly?.on)
cmd.push("-r");

const snapshot_name = get_snapshot_name(vals);
console.log([...cmd, subvolume_path, `${vals.snapshots_location}/${snapshot_name}`]);
const snapshot_location = `${vals.snapshots_location}/${snapshot_name}`;
await cockpit.spawn([...cmd, subvolume_path, snapshot_location], { superuser: "require", err: "message" });
localStorage.setItem(localstorage_key, JSON.stringify({ ...get_local_storage_snapshots_locs(), [subvol.id]: vals.snapshots_location }));

// Re-trigger btrfs poll so the users sees the created snapshot in the overview or subvolume detail page
await btrfs_poll();
}
}
});
}

function subvolume_delete(volume, subvol, mount_point_in_parent, card) {
const block = client.blocks[volume.path];
const subvols = client.uuids_btrfs_subvols[volume.data.uuid];
Expand Down Expand Up @@ -364,6 +508,12 @@ function make_btrfs_subvolume_page(parent, volume, subvol, path_prefix, subvols)
action: () => subvolume_create(volume, subvol, (mounted && !opt_ro) ? mount_point : mount_point_in_parent),
});

actions.push({
title: _("Create snapshot"),
excuse: create_excuse,
action: () => snapshot_create(volume, subvol, (mounted && !opt_ro) ? mount_point : mount_point_in_parent),
});

let delete_excuse = "";
if (!mount_point_in_parent) {
delete_excuse = _("At least one parent needs to be mounted writable");
Expand Down
45 changes: 45 additions & 0 deletions pkg/storaged/btrfs/utils.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import cockpit from "cockpit";

import { decode_filename } from "../utils.js";
import * as python from "python.js";
import get_path_uuid from "./get-path-uuid.py";

const _ = cockpit.gettext;

Expand Down Expand Up @@ -88,3 +90,46 @@ export function validate_subvolume_name(name) {
if (name.includes('/'))
return cockpit.format(_("Name cannot contain the character '/'."));
}

export async function validate_snapshots_location(path, volume) {
if (path === "")
return _("Location cannot be empty.");

try {
const output = await python.spawn([get_path_uuid], [path],
{ environ: ["LANGUAGE=" + (cockpit.language || "en")] });
console.log(output);
const path_info = JSON.parse(output);
if (path_info.filesystems.length !== 1)
return _("Unable to detect filesystem for given path");

const fs = path_info.filesystems[0];
if (fs.fstype !== "btrfs")
return _("Provided path is not btrfs");

if (fs.uuid !== volume.data.uuid)
return _("Snapshot location needs to be on the same btrfs volume");
} catch (err) {
if (err.exit_status == 2)
return _("Parent of snapshot location does not exist");
console.warn("Unable to detect UUID of snapshot location", err);
}
// const path_exists = await folder_exists(path);
// // Verify that the parent is in the same btrfs volume
// if (!path_exists) {
// }
//
// try {
// const output = await cockpit.spawn(["findmnt", "-o", "UUID", "-n", "-T", path], { err: "message" });
// const uuid = output.trim();
// if (uuid !== volume.data.uuid) {
// return _("Snapshot location needs to be on the same btrfs volume");
// }
// } catch (err) {
// console.log(err);
// if (err?.message === "") {
// return _("Given path does not exist");
// }
// console.warn("Unable to detect UUID of snapshot location", err);
// }
}
30 changes: 30 additions & 0 deletions pkg/storaged/btrfs/verify-btrfs-snapshot-location.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unused.

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import os
import os.path
import subprocess
import sys


def main(path):
# If the path does not exist, we will create but need to verify it lives on the same volume
if not os.path.exists(path):
if path.endswith('/'):
path = path.rstrip('/')
path = os.path.dirname(path)

# bail out if the parent path is not found
if not os.path.exists(path):
sys.exit(2)

try:
print(subprocess.check_output(["findmnt", "--output", "UUID", "--no-heading", "--target", path]))
except subprocess.SubprocessError as exc:
print(exc, file=sys.stderr)
sys.exit(3)


if __name__ == "__main__":
if len(sys.argv) != 2:
sys.stdout.write("Path not provided\n")
sys.exit(1)

main(sys.argv[1])
46 changes: 45 additions & 1 deletion pkg/storaged/dialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ const Row = ({ field, values, errors, onChange }) => {
);
} else if (!field.bare) {
return (
<FormGroup validated={validated} hasNoPaddingTop={field.hasNoPaddingTop}>
<FormGroup isInline={field.isInline || false} validated={validated} hasNoPaddingTop={field.hasNoPaddingTop}>
{ field_elts }
{ nested_elts }
<FormHelper helperText={explanation} helperTextInvalid={validated && error} />
Expand Down Expand Up @@ -601,13 +601,15 @@ export const TextInput = (tag, title, options) => {
title,
options,
initial_value: options.value || "",
isInline: options.isInline || false,

render: (val, change, validated) =>
<TextInputPF4 data-field={tag} data-field-type="text-input"
validated={validated}
aria-label={title}
value={val}
isDisabled={options.disabled}
placeholder={options.placeholder}
onChange={(_event, value) => change(value)} />
};
};
Expand Down Expand Up @@ -738,6 +740,48 @@ export const SelectOneRadio = (tag, title, options) => {
};
};

export const SelectOneRadioVerticalTextInput = (tag, title, options) => {
return {
tag,
title,
options,
initial_value: options.value || { checked: {}, inputs: {} },
hasNoPaddingTop: true,

render: (val, change) => {
const fieldset = options.choices.map(c => {
const ftag = tag + "." + c.value;
const fval = val.checked === c.value;
const tval = val.inputs[c.value] || '';
function fchange(newval) {
val.checked = newval;
change(val);
}

function tchange(newval) {
val.inputs[c.value] = newval;
change(val);
}

return (
<React.Fragment key={c.value}>
<Radio isChecked={fval} data-data={fval}
id={ftag}
onChange={() => fchange(c.value)} label={c.title} />
{fval !== false && c?.type === "radioWithInput" && <TextInputPF4 id={ftag + "-input"} value={tval} onChange={(_event, value) => tchange(value)} />}
</React.Fragment>
);
});

return (
<div data-field={tag} data-field-type="select-radio">
{fieldset}
</div>
);
}
};
};

export const SelectRow = (tag, headers, options) => {
return {
tag,
Expand Down