* [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool
@ 2024-03-20 14:15 Filip Schauer
2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 01/10] Initial commit Filip Schauer
` (11 more replies)
0 siblings, 12 replies; 23+ messages in thread
From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw)
To: pbs-devel
Implement a tool to import VMA files into a Proxmox Backup Server
Example usage:
zstd -d --stdout vzdump.vma.zst | vma-to-pbs \
--repository <auth_id@host:port:datastore> \
--vmid 123 \
--password_file pbs_password
Commit 07/10 requires
https://lists.proxmox.com/pipermail/pve-devel/2024-March/062182.html
to be applied first.
Changes since v4:
* Bump proxmox-backup-qemu
* Remove unnecessary "extern crate" declarations
* Refactor error handling with anyhow
* vma.rs: Improve code readability by adding constants and using more
descriptive variable/type names.
* vma.rs: Move duplicate code into read_string function
* Print elapsed time in minutes, seconds and ms
* Refactor block device id and size retrieval logic
* vma: Document break statement when reaching end of file
* Use selected imports instead of glob imports
* Split up vma2pbs logic into seperate functions
* Makefile: remove reference to unused submodule
Changes since v3:
* Add the ability to provide credentials via files
* Add support for streaming the VMA file via stdin
* Add a fallback for the --fingerprint argument
Changes since v2:
* Use the deb packages from the proxmox-io and proxmox-sys dependencies
instead of the proxmox submodule
* Remove the proxmox submodule
* Update the proxmox-backup-qemu submodule to make it buildable with
the newest librust dependencies
Changes since v1:
* Remove unused crates and uses
* Format the code
* Use anyhow for error handling
* Use clap for parsing arguments instead of getopts
* Fix blocks being reindexed on every read
* Make sure ProxmoxBackupHandle is dropped properly on error
* Move image_chunk_buffer from stack to heap
* Move the block_index in VmaReader to the heap completely
* Initialize vectors with `Vec::with_capacity` and `resize` instead of
the `vec!` macro, to potentially improve performance on debug builds.
* Add comments to code filling the MD5 sum field with zeros
* Change device_id arguments to usize
* Handle devices that have a size that is not aligned to 4096 properly
in read_device_contents, when the caller provides a buffer that would
exceed the device size.
* Avoid unnecessary loop iterations in read_device_contents when the
buffer size is not aligned to 65536
Filip Schauer (8):
Initial commit
Add the ability to provide credentials via files
bump proxmox-backup-qemu
remove unnecessary "extern crate" declarations
add support for streaming the VMA file via stdin
add a fallback for the --fingerprint argument
refactor error handling with anyhow
Makefile: remove reference to unused submodule
Wolfgang Bumiller (2):
cargo fmt
bump proxmox-backup-qemu
--
2.39.2
^ permalink raw reply [flat|nested] 23+ messages in thread* [pbs-devel] [PATCH vma-to-pbs 01/10] Initial commit 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer @ 2024-03-20 14:15 ` Filip Schauer 2024-03-25 9:42 ` Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 02/10] cargo fmt Filip Schauer ` (10 subsequent siblings) 11 siblings, 1 reply; 23+ messages in thread From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw) To: pbs-devel Implement a tool to import VMA files into a Proxmox Backup Server Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- .cargo/config | 5 + .gitmodules | 3 + Cargo.toml | 19 ++ Makefile | 70 +++++++ src/main.rs | 311 ++++++++++++++++++++++++++++++ src/vma.rs | 340 +++++++++++++++++++++++++++++++++ submodules/proxmox-backup-qemu | 1 + 7 files changed, 749 insertions(+) create mode 100644 .cargo/config create mode 100644 .gitmodules create mode 100644 Cargo.toml create mode 100644 Makefile create mode 100644 src/main.rs create mode 100644 src/vma.rs create mode 160000 submodules/proxmox-backup-qemu diff --git a/.cargo/config b/.cargo/config new file mode 100644 index 0000000..3b5b6e4 --- /dev/null +++ b/.cargo/config @@ -0,0 +1,5 @@ +[source] +[source.debian-packages] +directory = "/usr/share/cargo/registry" +[source.crates-io] +replace-with = "debian-packages" diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000..6d21e06 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "submodules/proxmox-backup-qemu"] + path = submodules/proxmox-backup-qemu + url = git://git.proxmox.com/git/proxmox-backup-qemu.git diff --git a/Cargo.toml b/Cargo.toml new file mode 100644 index 0000000..9711690 --- /dev/null +++ b/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "vma-to-pbs" +version = "0.0.1" +authors = ["Filip Schauer <f.schauer@proxmox.com>"] +edition = "2021" + +[dependencies] +anyhow = "1.0" +bincode = "1.3" +clap = { version = "4.0.32", features = ["cargo"] } +md5 = "0.7.0" +scopeguard = "1.1.0" +serde = "1.0" +serde-big-array = "0.4.1" + +proxmox-io = "1.0.1" +proxmox-sys = "0.5.0" + +proxmox-backup-qemu = { path = "submodules/proxmox-backup-qemu" } diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..a0c841d --- /dev/null +++ b/Makefile @@ -0,0 +1,70 @@ +include /usr/share/dpkg/default.mk + +PACKAGE = proxmox-vma-to-pbs +BUILDDIR = $(PACKAGE)-$(DEB_VERSION_UPSTREAM) + +ARCH := $(DEB_BUILD_ARCH) + +DSC=$(DEB_SOURCE)_$(DEB_VERSION).dsc +MAIN_DEB=$(PACKAGE)_$(DEB_VERSION)_$(ARCH).deb +OTHER_DEBS = \ + $(PACKAGE)-dev_$(DEB_VERSION)_$(ARCH).deb \ + $(PACKAGE)-dbgsym_$(DEB_VERSION)_$(ARCH).deb +DEBS=$(MAIN_DEB) $(OTHER_DEBS) + +DESTDIR= + +TARGET_DIR := target/debug + +ifeq ($(BUILD_MODE), release) +CARGO_BUILD_ARGS += --release +TARGETDIR := target/release +endif + +.PHONY: all build +all: build + +build: $(TARGETDIR)/vma-to-pbs +$(TARGETDIR)/vma-to-pbs: Cargo.toml src/ + cargo build $(CARGO_BUILD_ARGS) + +.PHONY: install +install: $(TARGETDIR)/vma-to-pbs + install -D -m 0755 $(TARGETDIR)/vma-to-pbs $(DESTDIR)/usr/bin/vma-to-pbs + +$(BUILDDIR): submodule + rm -rf $@ $@.tmp && mkdir $@.tmp + cp -a submodules debian Makefile .cargo Cargo.toml build.rs src $@.tmp/ + mv $@.tmp $@ + +submodule: + [ -e submodules/proxmox-backup-qemu/Cargo.toml ] || [ -e submodules/proxmox/proxmox-sys/Cargo.toml ] || git submodule update --init --recursive + +dsc: + rm -rf $(BUILDDIR) $(DSC) + $(MAKE) $(DSC) + lintian $(DSC) + +$(DSC): $(BUILDDIR) + cd $(BUILDDIR); dpkg-buildpackage -S -us -uc -d + +sbuild: $(DSC) + sbuild $< + +.PHONY: deb dsc +deb: $(OTHER_DEBS) +$(OTHER_DEBS): $(MAIN_DEB) +$(MAIN_DEB): $(BUILDDIR) + cd $(BUILDDIR); dpkg-buildpackage -b -us -uc + lintian $(DEBS) + +distclean: clean +clean: + cargo clean + rm -rf $(PACKAGE)-[0-9]*/ + rm -r *.deb *.dsc $(DEB_SOURCE)*.tar* *.build *.buildinfo *.changes Cargo.lock + +.PHONY: dinstall +dinstall: $(DEBS) + dpkg -i $(DEBS) + diff --git a/src/main.rs b/src/main.rs new file mode 100644 index 0000000..1aefd29 --- /dev/null +++ b/src/main.rs @@ -0,0 +1,311 @@ +extern crate anyhow; +extern crate clap; +extern crate proxmox_backup_qemu; +extern crate proxmox_io; +extern crate proxmox_sys; +extern crate scopeguard; + +use std::env; +use std::ffi::{c_char, CStr, CString}; +use std::ptr; +use std::time::{SystemTime, UNIX_EPOCH}; + +use anyhow::{anyhow, Context, Result}; +use clap::{command, Arg, ArgAction}; +use proxmox_backup_qemu::*; +use proxmox_sys::linux::tty; +use scopeguard::defer; + +mod vma; +use vma::*; + +fn backup_vma_to_pbs( + vma_file_path: String, + pbs_repository: String, + backup_id: String, + pbs_password: String, + keyfile: Option<String>, + key_password: Option<String>, + master_keyfile: Option<String>, + fingerprint: String, + compress: bool, + encrypt: bool, +) -> Result<()> { + println!("VMA input file: {}", vma_file_path); + println!("PBS repository: {}", pbs_repository); + println!("PBS fingerprint: {}", fingerprint); + println!("compress: {}", compress); + println!("encrypt: {}", encrypt); + + let backup_time = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_secs(); + println!("backup time: {}", backup_time); + + let mut pbs_err: *mut c_char = ptr::null_mut(); + + let pbs_repository_cstr = CString::new(pbs_repository).unwrap(); + let backup_id_cstr = CString::new(backup_id).unwrap(); + let pbs_password_cstr = CString::new(pbs_password).unwrap(); + let fingerprint_cstr = CString::new(fingerprint).unwrap(); + let keyfile_cstr = keyfile.map(|v| CString::new(v).unwrap()); + let keyfile_ptr = keyfile_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); + let key_password_cstr = key_password.map(|v| CString::new(v).unwrap()); + let key_password_ptr = key_password_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); + let master_keyfile_cstr = master_keyfile.map(|v| CString::new(v).unwrap()); + let master_keyfile_ptr = master_keyfile_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); + + let pbs = proxmox_backup_new_ns( + pbs_repository_cstr.as_ptr(), + ptr::null(), + backup_id_cstr.as_ptr(), + backup_time, + PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE, + pbs_password_cstr.as_ptr(), + keyfile_ptr, + key_password_ptr, + master_keyfile_ptr, + true, + false, + fingerprint_cstr.as_ptr(), + &mut pbs_err, + ); + + defer! { + proxmox_backup_disconnect(pbs); + } + + if pbs == ptr::null_mut() { + unsafe { + let pbs_err_cstr = CStr::from_ptr(pbs_err); + return Err(anyhow!("proxmox_backup_new_ns failed: {pbs_err_cstr:?}")); + } + } + + let connect_result = proxmox_backup_connect(pbs, &mut pbs_err); + + if connect_result < 0 { + unsafe { + let pbs_err_cstr = CStr::from_ptr(pbs_err); + return Err(anyhow!("proxmox_backup_connect failed: {pbs_err_cstr:?}")); + } + } + + let mut vma_reader = VmaReader::new(&vma_file_path)?; + + // Handle configs + let configs = vma_reader.get_configs(); + for (config_name, config_data) in configs { + println!("CFG: size: {} name: {}", config_data.len(), config_name); + + let config_name_cstr = CString::new(config_name).unwrap(); + + if proxmox_backup_add_config( + pbs, + config_name_cstr.as_ptr(), + config_data.as_ptr(), + config_data.len() as u64, + &mut pbs_err, + ) < 0 + { + unsafe { + let pbs_err_cstr = CStr::from_ptr(pbs_err); + return Err(anyhow!( + "proxmox_backup_add_config failed: {pbs_err_cstr:?}" + )); + } + } + } + + // Handle block devices + for device_id in 0..255 { + let device_name = match vma_reader.get_device_name(device_id) { + Some(x) => x, + None => { + continue; + } + }; + + let device_size = match vma_reader.get_device_size(device_id) { + Some(x) => x, + None => { + continue; + } + }; + + println!( + "DEV: dev_id={} size: {} devname: {}", + device_id, device_size, device_name + ); + + let device_name_cstr = CString::new(device_name).unwrap(); + let pbs_device_id = proxmox_backup_register_image( + pbs, + device_name_cstr.as_ptr(), + device_size, + false, + &mut pbs_err, + ); + + if pbs_device_id < 0 { + unsafe { + let pbs_err_cstr = CStr::from_ptr(pbs_err); + return Err(anyhow!( + "proxmox_backup_register_image failed: {pbs_err_cstr:?}" + )); + } + } + + let mut image_chunk_buffer = proxmox_io::boxed::zeroed(PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE as usize); + let mut bytes_transferred = 0; + + while bytes_transferred < device_size { + let bytes_left = device_size - bytes_transferred; + let chunk_size = bytes_left.min(PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE); + println!( + "Uploading dev_id: {} offset: {:#0X} - {:#0X}", + device_id, + bytes_transferred, + bytes_transferred + chunk_size + ); + + let is_zero_chunk = vma_reader + .read_device_contents( + device_id, + &mut image_chunk_buffer[0..chunk_size as usize], + bytes_transferred, + ) + .with_context(|| { + format!( + "read {} bytes at offset {} from disk {} from VMA file", + chunk_size, bytes_transferred, device_id + ) + })?; + + let write_data_result = proxmox_backup_write_data( + pbs, + pbs_device_id as u8, + if is_zero_chunk { + ptr::null() + } else { + image_chunk_buffer.as_ptr() + }, + bytes_transferred, + chunk_size, + &mut pbs_err, + ); + + if write_data_result < 0 { + unsafe { + let pbs_err_cstr = CStr::from_ptr(pbs_err); + return Err(anyhow!( + "proxmox_backup_write_data failed: {pbs_err_cstr:?}" + )); + } + } + + bytes_transferred += chunk_size; + } + + if proxmox_backup_close_image(pbs, pbs_device_id as u8, &mut pbs_err) < 0 { + unsafe { + let pbs_err_cstr = CStr::from_ptr(pbs_err); + return Err(anyhow!( + "proxmox_backup_close_image failed: {pbs_err_cstr:?}" + )); + } + } + } + + if proxmox_backup_finish(pbs, &mut pbs_err) < 0 { + unsafe { + let pbs_err_cstr = CStr::from_ptr(pbs_err); + return Err(anyhow!("proxmox_backup_finish failed: {pbs_err_cstr:?}")); + } + } + + Ok(()) +} + +fn main() -> Result<()> { + let matches = command!() + .arg( + Arg::new("repository") + .long("repository") + .value_name("auth_id@host:port:datastore") + .help("Repository URL") + .required(true), + ) + .arg( + Arg::new("vmid") + .long("vmid") + .value_name("VMID") + .help("Backup ID") + .required(true), + ) + .arg( + Arg::new("fingerprint") + .long("fingerprint") + .value_name("FINGERPRINT") + .help("Proxmox Backup Server Fingerprint") + .required(true), + ) + .arg( + Arg::new("keyfile") + .long("keyfile") + .value_name("KEYFILE") + .help("Key file"), + ) + .arg( + Arg::new("master_keyfile") + .long("master_keyfile") + .value_name("MASTER_KEYFILE") + .help("Master key file"), + ) + .arg( + Arg::new("compress") + .long("compress") + .short('c') + .help("Compress the Backup") + .action(ArgAction::SetTrue), + ) + .arg( + Arg::new("encrypt") + .long("encrypt") + .short('e') + .help("Encrypt the Backup") + .action(ArgAction::SetTrue), + ) + .arg(Arg::new("vma_file")) + .get_matches(); + + let pbs_repository = matches.get_one::<String>("repository").unwrap().to_string(); + let vmid = matches.get_one::<String>("vmid").unwrap().to_string(); + let fingerprint = matches.get_one::<String>("fingerprint").unwrap().to_string(); + + let keyfile = matches.get_one::<String>("keyfile"); + let master_keyfile = matches.get_one::<String>("master_keyfile"); + let compress = matches.get_flag("compress"); + let encrypt = matches.get_flag("encrypt"); + + let vma_file_path = matches.get_one::<String>("vma_file").unwrap().to_string(); + + let pbs_password = String::from_utf8(tty::read_password(&"Password: ").unwrap()).unwrap(); + let key_password = match keyfile { + Some(_) => Some(String::from_utf8(tty::read_password(&"Key Password: ").unwrap()).unwrap()), + None => None, + }; + + backup_vma_to_pbs( + vma_file_path, + pbs_repository, + vmid, + pbs_password, + keyfile.cloned(), + key_password, + master_keyfile.cloned(), + fingerprint, + compress, + encrypt, + )?; + + Ok(()) +} diff --git a/src/vma.rs b/src/vma.rs new file mode 100644 index 0000000..e2c3475 --- /dev/null +++ b/src/vma.rs @@ -0,0 +1,340 @@ +extern crate anyhow; +extern crate md5; + +use std::collections::HashMap; +use std::fs::File; +use std::io::{Read, Seek, SeekFrom}; +use std::mem::size_of; +use std::{cmp, str}; + +use anyhow::{anyhow, Result}; +use bincode::Options; +use serde::{Deserialize, Serialize}; +use serde_big_array::BigArray; + +const VMA_BLOCKS_PER_EXTENT: usize = 59; +const VMA_MAX_CONFIGS: usize = 256; +const VMA_MAX_DEVICES: usize = 256; + +#[repr(C)] +#[derive(Serialize, Deserialize)] +struct VmaDeviceInfoHeader { + pub device_name_offset: u32, + reserved: [u8; 4], + pub device_size: u64, + reserved1: [u8; 16], +} + +#[repr(C)] +#[derive(Serialize, Deserialize)] +struct VmaHeader { + pub magic: [u8; 4], + pub version: u32, + pub uuid: [u8; 16], + pub ctime: u64, + pub md5sum: [u8; 16], + pub blob_buffer_offset: u32, + pub blob_buffer_size: u32, + pub header_size: u32, + #[serde(with = "BigArray")] + reserved: [u8; 1984], + #[serde(with = "BigArray")] + pub config_names: [u32; VMA_MAX_CONFIGS], + #[serde(with = "BigArray")] + pub config_data: [u32; VMA_MAX_CONFIGS], + reserved1: [u8; 4], + #[serde(with = "BigArray")] + pub dev_info: [VmaDeviceInfoHeader; VMA_MAX_DEVICES], +} + +#[repr(C)] +#[derive(Serialize, Deserialize)] +struct VmaBlockInfo { + pub mask: u16, + reserved: u8, + pub dev_id: u8, + pub cluster_num: u32, +} + +#[repr(C)] +#[derive(Serialize, Deserialize)] +struct VmaExtentHeader { + pub magic: [u8; 4], + reserved: [u8; 2], + pub block_count: u16, + pub uuid: [u8; 16], + pub md5sum: [u8; 16], + #[serde(with = "BigArray")] + pub blockinfo: [VmaBlockInfo; VMA_BLOCKS_PER_EXTENT], +} + +#[derive(Clone)] +struct VmaBlockIndexEntry { + pub cluster_file_offset: u64, + pub mask: u16, +} + +pub struct VmaReader { + vma_file: File, + vma_header: VmaHeader, + configs: HashMap<String, String>, + block_index: Vec<Vec<VmaBlockIndexEntry>>, + blocks_are_indexed: bool, +} + +impl VmaReader { + pub fn new(vma_file_path: &str) -> Result<Self> { + let mut vma_file = match File::open(vma_file_path) { + Err(why) => return Err(anyhow!("couldn't open {}: {}", vma_file_path, why)), + Ok(file) => file, + }; + + let vma_header = Self::read_header(&mut vma_file).unwrap(); + let configs = Self::read_blob_buffer(&mut vma_file, &vma_header).unwrap(); + let block_index: Vec<Vec<VmaBlockIndexEntry>> = (0..256).map(|_| Vec::new()).collect(); + + let instance = Self { + vma_file, + vma_header, + configs, + block_index, + blocks_are_indexed: false, + }; + + Ok(instance) + } + + fn read_header(vma_file: &mut File) -> Result<VmaHeader> { + let mut buffer = Vec::with_capacity(size_of::<VmaHeader>()); + buffer.resize(size_of::<VmaHeader>(), 0); + vma_file.read_exact(&mut buffer)?; + + let bincode_options = bincode::DefaultOptions::new() + .with_fixint_encoding() + .with_big_endian(); + + let vma_header: VmaHeader = bincode_options.deserialize(&buffer)?; + + if vma_header.magic != [b'V', b'M', b'A', 0] { + return Err(anyhow!("Invalid magic number")); + } + + if vma_header.version != 1 { + return Err(anyhow!("Invalid VMA version {}", vma_header.version)); + } + + buffer.resize(vma_header.header_size as usize, 0); + vma_file.read_exact(&mut buffer[size_of::<VmaHeader>()..])?; + + // Fill the MD5 sum field with zeros to compute the MD5 sum + buffer[32..48].fill(0); + let computed_md5sum: [u8; 16] = md5::compute(&buffer).into(); + + if vma_header.md5sum != computed_md5sum { + return Err(anyhow!("Wrong VMA header checksum")); + } + + return Ok(vma_header); + } + + fn read_string_from_file(vma_file: &mut File, file_offset: u64) -> Result<String> { + let mut size_bytes = [0u8; 2]; + vma_file.seek(SeekFrom::Start(file_offset))?; + vma_file.read_exact(&mut size_bytes)?; + let size = u16::from_le_bytes(size_bytes) as usize; + let mut string_bytes = Vec::with_capacity(size - 1); + string_bytes.resize(size - 1, 0); + vma_file.read_exact(&mut string_bytes)?; + let string = str::from_utf8(&string_bytes)?; + + return Ok(string.to_string()); + } + + fn read_blob_buffer( + vma_file: &mut File, + vma_header: &VmaHeader, + ) -> Result<HashMap<String, String>> { + let mut configs = HashMap::new(); + + for i in 0..VMA_MAX_CONFIGS { + let config_name_offset = vma_header.config_names[i]; + let config_data_offset = vma_header.config_data[i]; + + if config_name_offset == 0 || config_data_offset == 0 { + continue; + } + + let config_name_file_offset = (vma_header.blob_buffer_offset + config_name_offset) as u64; + let config_data_file_offset = (vma_header.blob_buffer_offset + config_data_offset) as u64; + let config_name = Self::read_string_from_file(vma_file, config_name_file_offset)?; + let config_data = Self::read_string_from_file(vma_file, config_data_file_offset)?; + + configs.insert(String::from(config_name), String::from(config_data)); + } + + return Ok(configs); + } + + pub fn get_configs(&self) -> HashMap<String, String> { + return self.configs.clone(); + } + + pub fn get_device_name(&mut self, device_id: usize) -> Option<String> { + if device_id >= VMA_MAX_DEVICES { + return None; + } + + let device_name_offset = self.vma_header.dev_info[device_id].device_name_offset; + + if device_name_offset == 0 { + return None; + } + + let device_name_file_offset = (self.vma_header.blob_buffer_offset + device_name_offset) as u64; + let device_name = Self::read_string_from_file(&mut self.vma_file, device_name_file_offset).unwrap(); + + return Some(device_name.to_string()); + } + + pub fn get_device_size(&self, device_id: usize) -> Option<u64> { + if device_id >= VMA_MAX_DEVICES { + return None; + } + + let dev_info = &self.vma_header.dev_info[device_id]; + + if dev_info.device_name_offset == 0 { + return None; + } + + return Some(dev_info.device_size); + } + + fn read_extent_header(vma_file: &mut File) -> Result<VmaExtentHeader> { + let mut buffer = Vec::with_capacity(size_of::<VmaExtentHeader>()); + buffer.resize(size_of::<VmaExtentHeader>(), 0); + vma_file.read_exact(&mut buffer)?; + + let bincode_options = bincode::DefaultOptions::new() + .with_fixint_encoding() + .with_big_endian(); + + let vma_extent_header: VmaExtentHeader = bincode_options.deserialize(&buffer)?; + + if vma_extent_header.magic != [b'V', b'M', b'A', b'E'] { + return Err(anyhow!("Invalid magic number")); + } + + // Fill the MD5 sum field with zeros to compute the MD5 sum + buffer[24..40].fill(0); + let computed_md5sum: [u8; 16] = md5::compute(&buffer).into(); + + if vma_extent_header.md5sum != computed_md5sum { + return Err(anyhow!("Wrong VMA extent header checksum")); + } + + return Ok(vma_extent_header); + } + + fn index_device_clusters(&mut self) -> Result<()> { + for device_id in 0..255 { + let device_size = match self.get_device_size(device_id) { + Some(x) => x, + None => { + continue; + } + }; + + let device_cluster_count = (device_size + 4096 * 16 - 1) / (4096 * 16); + + let block_index_entry_placeholder = VmaBlockIndexEntry { + cluster_file_offset: 0, + mask: 0, + }; + + self.block_index[device_id].resize(device_cluster_count as usize, block_index_entry_placeholder); + } + + let mut file_offset = self.vma_header.header_size as u64; + let vma_file_size = self.vma_file.metadata()?.len(); + + while file_offset < vma_file_size { + self.vma_file.seek(SeekFrom::Start(file_offset))?; + let vma_extent_header = Self::read_extent_header(&mut self.vma_file)?; + file_offset += size_of::<VmaExtentHeader>() as u64; + + for i in 0..VMA_BLOCKS_PER_EXTENT { + let blockinfo = &vma_extent_header.blockinfo[i]; + + if blockinfo.dev_id == 0 { + continue; + } + + let block_index_entry = VmaBlockIndexEntry { + cluster_file_offset: file_offset, + mask: blockinfo.mask, + }; + + self.block_index[blockinfo.dev_id as usize][blockinfo.cluster_num as usize] = block_index_entry; + file_offset += blockinfo.mask.count_ones() as u64 * 4096; + } + } + + self.blocks_are_indexed = true; + + return Ok(()); + } + + pub fn read_device_contents( + &mut self, + device_id: usize, + buffer: &mut [u8], + offset: u64, + ) -> Result<bool> { + if device_id >= VMA_MAX_DEVICES { + return Err(anyhow!("invalid device id {}", device_id)); + } + + if offset % (4096 * 16) != 0 { + return Err(anyhow!("offset is not aligned to 65536")); + } + + // Make sure that the device clusters are already indexed + if !self.blocks_are_indexed { + self.index_device_clusters()?; + } + + let this_device_block_index = &self.block_index[device_id]; + let length = cmp::min( + buffer.len(), + this_device_block_index.len() * 4096 * 16 - offset as usize, + ); + let mut buffer_offset = 0; + let mut buffer_is_zero = true; + + while buffer_offset < length { + let block_index_entry = &this_device_block_index[(offset as usize + buffer_offset) / (4096 * 16)]; + self.vma_file.seek(SeekFrom::Start(block_index_entry.cluster_file_offset))?; + + for i in 0..16 { + if buffer_offset >= length { + break; + } + + let block_buffer_end = buffer_offset + cmp::min(length - buffer_offset, 4096); + let block_mask = ((block_index_entry.mask >> i) & 1) == 1; + + if block_mask { + self.vma_file.read_exact(&mut buffer[buffer_offset..block_buffer_end])?; + buffer_is_zero = false; + } else { + buffer[buffer_offset..block_buffer_end].fill(0); + } + + buffer_offset += 4096; + } + } + + return Ok(buffer_is_zero); + } +} diff --git a/submodules/proxmox-backup-qemu b/submodules/proxmox-backup-qemu new file mode 160000 index 0000000..8af623b --- /dev/null +++ b/submodules/proxmox-backup-qemu @@ -0,0 +1 @@ +Subproject commit 8af623b2100bcda171074addbcb27d828bed2e99 -- 2.39.2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs 01/10] Initial commit 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 01/10] Initial commit Filip Schauer @ 2024-03-25 9:42 ` Filip Schauer 0 siblings, 0 replies; 23+ messages in thread From: Filip Schauer @ 2024-03-25 9:42 UTC (permalink / raw) To: pbs-devel Ignore this commit, as it is already applied. On 20/03/2024 15:15, Filip Schauer wrote: > Implement a tool to import VMA files into a Proxmox Backup Server > Signed-off-by: Filip Schauer <f.schauer@proxmox.com>--- .cargo/config > | 5 + .gitmodules | 3 + Cargo.toml | 19 ++ Makefile | 70 +++++++ > src/main.rs | 311 ++++++++++++++++++++++++++++++ src/vma.rs | 340 > +++++++++++++++++++++++++++++++++ submodules/proxmox-backup-qemu | 1 + > 7 files changed, 749 insertions(+) create mode 100644 .cargo/config > create mode 100644 .gitmodules create mode 100644 Cargo.toml create > mode 100644 Makefile create mode 100644 src/main.rs create mode 100644 > src/vma.rs create mode 160000 submodules/proxmox-backup-qemu ^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs 02/10] cargo fmt 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 01/10] Initial commit Filip Schauer @ 2024-03-20 14:15 ` Filip Schauer 2024-03-25 9:43 ` Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 03/10] bump proxmox-backup-qemu Filip Schauer ` (9 subsequent siblings) 11 siblings, 1 reply; 23+ messages in thread From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw) To: pbs-devel; +Cc: Wolfgang Bumiller From: Wolfgang Bumiller <w.bumiller@proxmox.com> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> --- src/main.rs | 17 +++++++++++++---- src/vma.rs | 27 ++++++++++++++++++--------- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/main.rs b/src/main.rs index 1aefd29..8d95b11 100644 --- a/src/main.rs +++ b/src/main.rs @@ -37,7 +37,10 @@ fn backup_vma_to_pbs( println!("compress: {}", compress); println!("encrypt: {}", encrypt); - let backup_time = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_secs(); + let backup_time = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs(); println!("backup time: {}", backup_time); let mut pbs_err: *mut c_char = ptr::null_mut(); @@ -51,7 +54,9 @@ fn backup_vma_to_pbs( let key_password_cstr = key_password.map(|v| CString::new(v).unwrap()); let key_password_ptr = key_password_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); let master_keyfile_cstr = master_keyfile.map(|v| CString::new(v).unwrap()); - let master_keyfile_ptr = master_keyfile_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); + let master_keyfile_ptr = master_keyfile_cstr + .map(|v| v.as_ptr()) + .unwrap_or(ptr::null()); let pbs = proxmox_backup_new_ns( pbs_repository_cstr.as_ptr(), @@ -154,7 +159,8 @@ fn backup_vma_to_pbs( } } - let mut image_chunk_buffer = proxmox_io::boxed::zeroed(PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE as usize); + let mut image_chunk_buffer = + proxmox_io::boxed::zeroed(PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE as usize); let mut bytes_transferred = 0; while bytes_transferred < device_size { @@ -279,7 +285,10 @@ fn main() -> Result<()> { let pbs_repository = matches.get_one::<String>("repository").unwrap().to_string(); let vmid = matches.get_one::<String>("vmid").unwrap().to_string(); - let fingerprint = matches.get_one::<String>("fingerprint").unwrap().to_string(); + let fingerprint = matches + .get_one::<String>("fingerprint") + .unwrap() + .to_string(); let keyfile = matches.get_one::<String>("keyfile"); let master_keyfile = matches.get_one::<String>("master_keyfile"); diff --git a/src/vma.rs b/src/vma.rs index e2c3475..5ec3822 100644 --- a/src/vma.rs +++ b/src/vma.rs @@ -164,8 +164,10 @@ impl VmaReader { continue; } - let config_name_file_offset = (vma_header.blob_buffer_offset + config_name_offset) as u64; - let config_data_file_offset = (vma_header.blob_buffer_offset + config_data_offset) as u64; + let config_name_file_offset = + (vma_header.blob_buffer_offset + config_name_offset) as u64; + let config_data_file_offset = + (vma_header.blob_buffer_offset + config_data_offset) as u64; let config_name = Self::read_string_from_file(vma_file, config_name_file_offset)?; let config_data = Self::read_string_from_file(vma_file, config_data_file_offset)?; @@ -190,8 +192,10 @@ impl VmaReader { return None; } - let device_name_file_offset = (self.vma_header.blob_buffer_offset + device_name_offset) as u64; - let device_name = Self::read_string_from_file(&mut self.vma_file, device_name_file_offset).unwrap(); + let device_name_file_offset = + (self.vma_header.blob_buffer_offset + device_name_offset) as u64; + let device_name = + Self::read_string_from_file(&mut self.vma_file, device_name_file_offset).unwrap(); return Some(device_name.to_string()); } @@ -252,7 +256,8 @@ impl VmaReader { mask: 0, }; - self.block_index[device_id].resize(device_cluster_count as usize, block_index_entry_placeholder); + self.block_index[device_id] + .resize(device_cluster_count as usize, block_index_entry_placeholder); } let mut file_offset = self.vma_header.header_size as u64; @@ -275,7 +280,8 @@ impl VmaReader { mask: blockinfo.mask, }; - self.block_index[blockinfo.dev_id as usize][blockinfo.cluster_num as usize] = block_index_entry; + self.block_index[blockinfo.dev_id as usize][blockinfo.cluster_num as usize] = + block_index_entry; file_offset += blockinfo.mask.count_ones() as u64 * 4096; } } @@ -313,8 +319,10 @@ impl VmaReader { let mut buffer_is_zero = true; while buffer_offset < length { - let block_index_entry = &this_device_block_index[(offset as usize + buffer_offset) / (4096 * 16)]; - self.vma_file.seek(SeekFrom::Start(block_index_entry.cluster_file_offset))?; + let block_index_entry = + &this_device_block_index[(offset as usize + buffer_offset) / (4096 * 16)]; + self.vma_file + .seek(SeekFrom::Start(block_index_entry.cluster_file_offset))?; for i in 0..16 { if buffer_offset >= length { @@ -325,7 +333,8 @@ impl VmaReader { let block_mask = ((block_index_entry.mask >> i) & 1) == 1; if block_mask { - self.vma_file.read_exact(&mut buffer[buffer_offset..block_buffer_end])?; + self.vma_file + .read_exact(&mut buffer[buffer_offset..block_buffer_end])?; buffer_is_zero = false; } else { buffer[buffer_offset..block_buffer_end].fill(0); -- 2.39.2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs 02/10] cargo fmt 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 02/10] cargo fmt Filip Schauer @ 2024-03-25 9:43 ` Filip Schauer 0 siblings, 0 replies; 23+ messages in thread From: Filip Schauer @ 2024-03-25 9:43 UTC (permalink / raw) To: pbs-devel Ignore this commit, as it is already applied. On 20/03/2024 15:15, Filip Schauer wrote: > From: Wolfgang Bumiller <w.bumiller@proxmox.com>Signed-off-by: > Wolfgang Bumiller <w.bumiller@proxmox.com>--- src/main.rs | 17 > +++++++++++++---- src/vma.rs | 27 ++++++++++++++++++--------- 2 files > changed, 31 insertions(+), 13 deletions(-) ^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs 03/10] bump proxmox-backup-qemu 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 01/10] Initial commit Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 02/10] cargo fmt Filip Schauer @ 2024-03-20 14:15 ` Filip Schauer 2024-03-25 9:43 ` Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 04/10] Add the ability to provide credentials via files Filip Schauer ` (8 subsequent siblings) 11 siblings, 1 reply; 23+ messages in thread From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw) To: pbs-devel; +Cc: Wolfgang Bumiller From: Wolfgang Bumiller <w.bumiller@proxmox.com> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> --- submodules/proxmox-backup-qemu | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submodules/proxmox-backup-qemu b/submodules/proxmox-backup-qemu index 8af623b..3adc6a1 160000 --- a/submodules/proxmox-backup-qemu +++ b/submodules/proxmox-backup-qemu @@ -1 +1 @@ -Subproject commit 8af623b2100bcda171074addbcb27d828bed2e99 +Subproject commit 3adc6a17877a6bf19a2d04d5fe1dfe6eb62e1eb7 -- 2.39.2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs 03/10] bump proxmox-backup-qemu 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 03/10] bump proxmox-backup-qemu Filip Schauer @ 2024-03-25 9:43 ` Filip Schauer 0 siblings, 0 replies; 23+ messages in thread From: Filip Schauer @ 2024-03-25 9:43 UTC (permalink / raw) To: pbs-devel Ignore this commit, as it is already applied. On 20/03/2024 15:15, Filip Schauer wrote: > From: Wolfgang Bumiller <w.bumiller@proxmox.com>Signed-off-by: > Wolfgang Bumiller <w.bumiller@proxmox.com> ^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs 04/10] Add the ability to provide credentials via files 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer ` (2 preceding siblings ...) 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 03/10] bump proxmox-backup-qemu Filip Schauer @ 2024-03-20 14:15 ` Filip Schauer 2024-03-25 12:33 ` Max Carrara 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 05/10] bump proxmox-backup-qemu Filip Schauer ` (7 subsequent siblings) 11 siblings, 1 reply; 23+ messages in thread From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw) To: pbs-devel Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- src/main.rs | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index 8d95b11..b58387b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -280,6 +280,18 @@ fn main() -> Result<()> { .help("Encrypt the Backup") .action(ArgAction::SetTrue), ) + .arg( + Arg::new("password_file") + .long("password_file") + .value_name("PASSWORD_FILE") + .help("Password file"), + ) + .arg( + Arg::new("key_password_file") + .long("key_password_file") + .value_name("KEY_PASSWORD_FILE") + .help("Key password file"), + ) .arg(Arg::new("vma_file")) .get_matches(); @@ -296,10 +308,25 @@ fn main() -> Result<()> { let encrypt = matches.get_flag("encrypt"); let vma_file_path = matches.get_one::<String>("vma_file").unwrap().to_string(); + let password_file = matches.get_one::<String>("password_file"); + + let pbs_password = match password_file { + Some(password_file) => { + std::fs::read_to_string(password_file).expect("Could not read password file") + } + None => String::from_utf8(tty::read_password("Password: ")?)?, + }; - let pbs_password = String::from_utf8(tty::read_password(&"Password: ").unwrap()).unwrap(); let key_password = match keyfile { - Some(_) => Some(String::from_utf8(tty::read_password(&"Key Password: ").unwrap()).unwrap()), + Some(_) => { + let key_password_file = matches.get_one::<String>("key_password_file"); + + Some(match key_password_file { + Some(key_password_file) => std::fs::read_to_string(key_password_file) + .expect("Could not read key password file"), + None => String::from_utf8(tty::read_password("Key Password: ")?)?, + }) + } None => None, }; -- 2.39.2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs 04/10] Add the ability to provide credentials via files 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 04/10] Add the ability to provide credentials via files Filip Schauer @ 2024-03-25 12:33 ` Max Carrara 2024-03-25 15:26 ` Thomas Lamprecht 0 siblings, 1 reply; 23+ messages in thread From: Max Carrara @ 2024-03-25 12:33 UTC (permalink / raw) To: Proxmox Backup Server development discussion; +Cc: w.bumiller On Wed Mar 20, 2024 at 3:15 PM CET, Filip Schauer wrote: > Signed-off-by: Filip Schauer <f.schauer@proxmox.com> > --- > src/main.rs | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > > diff --git a/src/main.rs b/src/main.rs > index 8d95b11..b58387b 100644 > --- a/src/main.rs > +++ b/src/main.rs > @@ -280,6 +280,18 @@ fn main() -> Result<()> { > .help("Encrypt the Backup") > .action(ArgAction::SetTrue), > ) > + .arg( > + Arg::new("password_file") > + .long("password_file") > + .value_name("PASSWORD_FILE") > + .help("Password file"), > + ) > + .arg( > + Arg::new("key_password_file") > + .long("key_password_file") > + .value_name("KEY_PASSWORD_FILE") > + .help("Key password file"), > + ) > .arg(Arg::new("vma_file")) > .get_matches(); One thing I had noticed when using the CLI: Would it maaaybe be nicer to use e.g. --password-file instead of --password_file? AFAIR @Wolfgang mentioned something regarding this off list. > > @@ -296,10 +308,25 @@ fn main() -> Result<()> { > let encrypt = matches.get_flag("encrypt"); > > let vma_file_path = matches.get_one::<String>("vma_file").unwrap().to_string(); > + let password_file = matches.get_one::<String>("password_file"); > + > + let pbs_password = match password_file { > + Some(password_file) => { > + std::fs::read_to_string(password_file).expect("Could not read password file") You changed this later and got rid of the undying, but that fix could've perhaps been in here already. > + } > + None => String::from_utf8(tty::read_password("Password: ")?)?, > + }; > > - let pbs_password = String::from_utf8(tty::read_password(&"Password: ").unwrap()).unwrap(); > let key_password = match keyfile { > - Some(_) => Some(String::from_utf8(tty::read_password(&"Key Password: ").unwrap()).unwrap()), > + Some(_) => { > + let key_password_file = matches.get_one::<String>("key_password_file"); > + > + Some(match key_password_file { > + Some(key_password_file) => std::fs::read_to_string(key_password_file) > + .expect("Could not read key password file"), Same as above. > + None => String::from_utf8(tty::read_password("Key Password: ")?)?, > + }) > + } > None => None, > }; > [0]: https://docs.rs/anyhow/1.0.79/anyhow/trait.Context.html ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs 04/10] Add the ability to provide credentials via files 2024-03-25 12:33 ` Max Carrara @ 2024-03-25 15:26 ` Thomas Lamprecht 0 siblings, 0 replies; 23+ messages in thread From: Thomas Lamprecht @ 2024-03-25 15:26 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Max Carrara; +Cc: w.bumiller Am 25/03/2024 um 13:33 schrieb Max Carrara: > On Wed Mar 20, 2024 at 3:15 PM CET, Filip Schauer wrote: >> Signed-off-by: Filip Schauer <f.schauer@proxmox.com> >> --- >> src/main.rs | 31 +++++++++++++++++++++++++++++-- >> 1 file changed, 29 insertions(+), 2 deletions(-) >> >> diff --git a/src/main.rs b/src/main.rs >> index 8d95b11..b58387b 100644 >> --- a/src/main.rs >> +++ b/src/main.rs >> @@ -280,6 +280,18 @@ fn main() -> Result<()> { >> .help("Encrypt the Backup") >> .action(ArgAction::SetTrue), >> ) >> + .arg( >> + Arg::new("password_file") >> + .long("password_file") >> + .value_name("PASSWORD_FILE") >> + .help("Password file"), >> + ) >> + .arg( >> + Arg::new("key_password_file") >> + .long("key_password_file") >> + .value_name("KEY_PASSWORD_FILE") >> + .help("Key password file"), >> + ) >> .arg(Arg::new("vma_file")) >> .get_matches(); > > One thing I had noticed when using the CLI: Would it maaaybe be nicer to > use e.g. --password-file instead of --password_file? AFAIR @Wolfgang > mentioned something regarding this off list. yes, we use kebab-case for all new (CLI & API) options if anyhow possible, please! btw. is clap really required here? Our usage in the past has shown that it's rather a huge maintenance burden, and so I'd like to prefer using pico-args. You can use the clap usage output as source for a manual implemented usage though (extending that then in the future isn't that much work). For a conversion example and some more background see: https://git.proxmox.com/?p=pve-xtermjs.git;a=commitdiff;h=24d707d0506b120a085b06b5f2b6000696879a1e;hp=749ebb0907293a9f1cf0f5074e0a240f39f94f6f Note though that I would not block clap usage for now, but there's a significant possibility that I'll rip it out on the next clap major bump in anyway. > >> >> @@ -296,10 +308,25 @@ fn main() -> Result<()> { >> let encrypt = matches.get_flag("encrypt"); >> >> let vma_file_path = matches.get_one::<String>("vma_file").unwrap().to_string(); >> + let password_file = matches.get_one::<String>("password_file"); >> + >> + let pbs_password = match password_file { >> + Some(password_file) => { >> + std::fs::read_to_string(password_file).expect("Could not read password file") > > You changed this later and got rid of the undying, but that fix could've > perhaps been in here already. yeah, not that big of an issue on such initial additions, but if we somehow can avoid intermediate breakage or the like it's always better. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs 05/10] bump proxmox-backup-qemu 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer ` (3 preceding siblings ...) 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 04/10] Add the ability to provide credentials via files Filip Schauer @ 2024-03-20 14:15 ` Filip Schauer 2024-03-25 12:33 ` Max Carrara 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 06/10] remove unnecessary "extern crate" declarations Filip Schauer ` (6 subsequent siblings) 11 siblings, 1 reply; 23+ messages in thread From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw) To: pbs-devel Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- submodules/proxmox-backup-qemu | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submodules/proxmox-backup-qemu b/submodules/proxmox-backup-qemu index 3adc6a1..c35034c 160000 --- a/submodules/proxmox-backup-qemu +++ b/submodules/proxmox-backup-qemu @@ -1 +1 @@ -Subproject commit 3adc6a17877a6bf19a2d04d5fe1dfe6eb62e1eb7 +Subproject commit c35034c98f12b40c097631fb34541c489eb4fea7 -- 2.39.2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs 05/10] bump proxmox-backup-qemu 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 05/10] bump proxmox-backup-qemu Filip Schauer @ 2024-03-25 12:33 ` Max Carrara 0 siblings, 0 replies; 23+ messages in thread From: Max Carrara @ 2024-03-25 12:33 UTC (permalink / raw) To: Proxmox Backup Server development discussion On Wed Mar 20, 2024 at 3:15 PM CET, Filip Schauer wrote: > Signed-off-by: Filip Schauer <f.schauer@proxmox.com> > --- > submodules/proxmox-backup-qemu | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/submodules/proxmox-backup-qemu b/submodules/proxmox-backup-qemu > index 3adc6a1..c35034c 160000 > --- a/submodules/proxmox-backup-qemu > +++ b/submodules/proxmox-backup-qemu > @@ -1 +1 @@ > -Subproject commit 3adc6a17877a6bf19a2d04d5fe1dfe6eb62e1eb7 > +Subproject commit c35034c98f12b40c097631fb34541c489eb4fea7 As of this writing, this can now instead be bumped to 9ee3e88d129f68c41c9e418049801deacc5d09da. proxmox-backup-qemu was very recently updated. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs 06/10] remove unnecessary "extern crate" declarations 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer ` (4 preceding siblings ...) 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 05/10] bump proxmox-backup-qemu Filip Schauer @ 2024-03-20 14:15 ` Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 07/10] add support for streaming the VMA file via stdin Filip Schauer ` (5 subsequent siblings) 11 siblings, 0 replies; 23+ messages in thread From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw) To: pbs-devel Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- src/main.rs | 7 ------- src/vma.rs | 3 --- 2 files changed, 10 deletions(-) diff --git a/src/main.rs b/src/main.rs index b58387b..f619b3e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,10 +1,3 @@ -extern crate anyhow; -extern crate clap; -extern crate proxmox_backup_qemu; -extern crate proxmox_io; -extern crate proxmox_sys; -extern crate scopeguard; - use std::env; use std::ffi::{c_char, CStr, CString}; use std::ptr; diff --git a/src/vma.rs b/src/vma.rs index 5ec3822..d30cb09 100644 --- a/src/vma.rs +++ b/src/vma.rs @@ -1,6 +1,3 @@ -extern crate anyhow; -extern crate md5; - use std::collections::HashMap; use std::fs::File; use std::io::{Read, Seek, SeekFrom}; -- 2.39.2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs 07/10] add support for streaming the VMA file via stdin 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer ` (5 preceding siblings ...) 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 06/10] remove unnecessary "extern crate" declarations Filip Schauer @ 2024-03-20 14:15 ` Filip Schauer 2024-03-25 12:34 ` Max Carrara 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 08/10] add a fallback for the --fingerprint argument Filip Schauer ` (4 subsequent siblings) 11 siblings, 1 reply; 23+ messages in thread From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw) To: pbs-devel This allows the user to stream a compressed VMA file directly to a PBS without the need to extract it to an intermediate .vma file. Example usage: zstd -d --stdout vzdump.vma.zst | vma-to-pbs \ --repository <auth_id@host:port:datastore> \ --vmid 123 \ --password_file pbs_password Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- This requires https://lists.proxmox.com/pipermail/pve-devel/2024-March/062182.html to be applied first. src/main.rs | 241 ++------------------------------ src/vma.rs | 326 ++++++++++++++++++++----------------------- src/vma2pbs.rs | 368 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 530 insertions(+), 405 deletions(-) create mode 100644 src/vma2pbs.rs diff --git a/src/main.rs b/src/main.rs index f619b3e..e0e1076 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,228 +1,10 @@ -use std::env; -use std::ffi::{c_char, CStr, CString}; -use std::ptr; -use std::time::{SystemTime, UNIX_EPOCH}; - -use anyhow::{anyhow, Context, Result}; +use anyhow::Result; use clap::{command, Arg, ArgAction}; -use proxmox_backup_qemu::*; use proxmox_sys::linux::tty; -use scopeguard::defer; mod vma; -use vma::*; - -fn backup_vma_to_pbs( - vma_file_path: String, - pbs_repository: String, - backup_id: String, - pbs_password: String, - keyfile: Option<String>, - key_password: Option<String>, - master_keyfile: Option<String>, - fingerprint: String, - compress: bool, - encrypt: bool, -) -> Result<()> { - println!("VMA input file: {}", vma_file_path); - println!("PBS repository: {}", pbs_repository); - println!("PBS fingerprint: {}", fingerprint); - println!("compress: {}", compress); - println!("encrypt: {}", encrypt); - - let backup_time = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_secs(); - println!("backup time: {}", backup_time); - - let mut pbs_err: *mut c_char = ptr::null_mut(); - - let pbs_repository_cstr = CString::new(pbs_repository).unwrap(); - let backup_id_cstr = CString::new(backup_id).unwrap(); - let pbs_password_cstr = CString::new(pbs_password).unwrap(); - let fingerprint_cstr = CString::new(fingerprint).unwrap(); - let keyfile_cstr = keyfile.map(|v| CString::new(v).unwrap()); - let keyfile_ptr = keyfile_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); - let key_password_cstr = key_password.map(|v| CString::new(v).unwrap()); - let key_password_ptr = key_password_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); - let master_keyfile_cstr = master_keyfile.map(|v| CString::new(v).unwrap()); - let master_keyfile_ptr = master_keyfile_cstr - .map(|v| v.as_ptr()) - .unwrap_or(ptr::null()); - - let pbs = proxmox_backup_new_ns( - pbs_repository_cstr.as_ptr(), - ptr::null(), - backup_id_cstr.as_ptr(), - backup_time, - PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE, - pbs_password_cstr.as_ptr(), - keyfile_ptr, - key_password_ptr, - master_keyfile_ptr, - true, - false, - fingerprint_cstr.as_ptr(), - &mut pbs_err, - ); - - defer! { - proxmox_backup_disconnect(pbs); - } - - if pbs == ptr::null_mut() { - unsafe { - let pbs_err_cstr = CStr::from_ptr(pbs_err); - return Err(anyhow!("proxmox_backup_new_ns failed: {pbs_err_cstr:?}")); - } - } - - let connect_result = proxmox_backup_connect(pbs, &mut pbs_err); - - if connect_result < 0 { - unsafe { - let pbs_err_cstr = CStr::from_ptr(pbs_err); - return Err(anyhow!("proxmox_backup_connect failed: {pbs_err_cstr:?}")); - } - } - - let mut vma_reader = VmaReader::new(&vma_file_path)?; - - // Handle configs - let configs = vma_reader.get_configs(); - for (config_name, config_data) in configs { - println!("CFG: size: {} name: {}", config_data.len(), config_name); - - let config_name_cstr = CString::new(config_name).unwrap(); - - if proxmox_backup_add_config( - pbs, - config_name_cstr.as_ptr(), - config_data.as_ptr(), - config_data.len() as u64, - &mut pbs_err, - ) < 0 - { - unsafe { - let pbs_err_cstr = CStr::from_ptr(pbs_err); - return Err(anyhow!( - "proxmox_backup_add_config failed: {pbs_err_cstr:?}" - )); - } - } - } - - // Handle block devices - for device_id in 0..255 { - let device_name = match vma_reader.get_device_name(device_id) { - Some(x) => x, - None => { - continue; - } - }; - - let device_size = match vma_reader.get_device_size(device_id) { - Some(x) => x, - None => { - continue; - } - }; - - println!( - "DEV: dev_id={} size: {} devname: {}", - device_id, device_size, device_name - ); - - let device_name_cstr = CString::new(device_name).unwrap(); - let pbs_device_id = proxmox_backup_register_image( - pbs, - device_name_cstr.as_ptr(), - device_size, - false, - &mut pbs_err, - ); - - if pbs_device_id < 0 { - unsafe { - let pbs_err_cstr = CStr::from_ptr(pbs_err); - return Err(anyhow!( - "proxmox_backup_register_image failed: {pbs_err_cstr:?}" - )); - } - } - - let mut image_chunk_buffer = - proxmox_io::boxed::zeroed(PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE as usize); - let mut bytes_transferred = 0; - - while bytes_transferred < device_size { - let bytes_left = device_size - bytes_transferred; - let chunk_size = bytes_left.min(PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE); - println!( - "Uploading dev_id: {} offset: {:#0X} - {:#0X}", - device_id, - bytes_transferred, - bytes_transferred + chunk_size - ); - - let is_zero_chunk = vma_reader - .read_device_contents( - device_id, - &mut image_chunk_buffer[0..chunk_size as usize], - bytes_transferred, - ) - .with_context(|| { - format!( - "read {} bytes at offset {} from disk {} from VMA file", - chunk_size, bytes_transferred, device_id - ) - })?; - - let write_data_result = proxmox_backup_write_data( - pbs, - pbs_device_id as u8, - if is_zero_chunk { - ptr::null() - } else { - image_chunk_buffer.as_ptr() - }, - bytes_transferred, - chunk_size, - &mut pbs_err, - ); - - if write_data_result < 0 { - unsafe { - let pbs_err_cstr = CStr::from_ptr(pbs_err); - return Err(anyhow!( - "proxmox_backup_write_data failed: {pbs_err_cstr:?}" - )); - } - } - - bytes_transferred += chunk_size; - } - - if proxmox_backup_close_image(pbs, pbs_device_id as u8, &mut pbs_err) < 0 { - unsafe { - let pbs_err_cstr = CStr::from_ptr(pbs_err); - return Err(anyhow!( - "proxmox_backup_close_image failed: {pbs_err_cstr:?}" - )); - } - } - } - - if proxmox_backup_finish(pbs, &mut pbs_err) < 0 { - unsafe { - let pbs_err_cstr = CStr::from_ptr(pbs_err); - return Err(anyhow!("proxmox_backup_finish failed: {pbs_err_cstr:?}")); - } - } - - Ok(()) -} +mod vma2pbs; +use vma2pbs::{backup_vma_to_pbs, BackupVmaToPbsArgs}; fn main() -> Result<()> { let matches = command!() @@ -300,7 +82,8 @@ fn main() -> Result<()> { let compress = matches.get_flag("compress"); let encrypt = matches.get_flag("encrypt"); - let vma_file_path = matches.get_one::<String>("vma_file").unwrap().to_string(); + let vma_file_path = matches.get_one::<String>("vma_file"); + let password_file = matches.get_one::<String>("password_file"); let pbs_password = match password_file { @@ -323,18 +106,20 @@ fn main() -> Result<()> { None => None, }; - backup_vma_to_pbs( - vma_file_path, + let args = BackupVmaToPbsArgs { + vma_file_path: vma_file_path.cloned(), pbs_repository, - vmid, + backup_id: vmid, pbs_password, - keyfile.cloned(), + keyfile: keyfile.cloned(), key_password, - master_keyfile.cloned(), + master_keyfile: master_keyfile.cloned(), fingerprint, compress, encrypt, - )?; + }; + + backup_vma_to_pbs(args)?; Ok(()) } diff --git a/src/vma.rs b/src/vma.rs index d30cb09..d369b36 100644 --- a/src/vma.rs +++ b/src/vma.rs @@ -1,24 +1,55 @@ -use std::collections::HashMap; -use std::fs::File; -use std::io::{Read, Seek, SeekFrom}; +use std::collections::HashSet; +use std::io::Read; use std::mem::size_of; -use std::{cmp, str}; +use std::str; use anyhow::{anyhow, Result}; use bincode::Options; use serde::{Deserialize, Serialize}; use serde_big_array::BigArray; -const VMA_BLOCKS_PER_EXTENT: usize = 59; +/// Maximum number of clusters in an extent +/// See Image Data Streams in pve-qemu.git/vma_spec.txt +const VMA_CLUSTERS_PER_EXTENT: usize = 59; + +/// Number of 4k blocks per cluster +/// See pve-qemu.git/vma_spec.txt +const VMA_BLOCKS_PER_CLUSTER: usize = 16; + +/// Maximum number of config files +/// See VMA Header in pve-qemu.git/vma_spec.txt const VMA_MAX_CONFIGS: usize = 256; + +/// Maximum number of block devices +/// See VMA Header in pve-qemu.git/vma_spec.txt const VMA_MAX_DEVICES: usize = 256; +/// VMA magic string +/// See VMA Header in pve-qemu.git/vma_spec.txt +const VMA_HEADER_MAGIC: [u8; 4] = [b'V', b'M', b'A', 0]; + +/// VMA extent magic string +/// See VMA Extent Header in pve-qemu.git/vma_spec.txt +const VMA_EXTENT_HEADER_MAGIC: [u8; 4] = [b'V', b'M', b'A', b'E']; + +/// Size of a block +/// See pve-qemu.git/vma_spec.txt +const BLOCK_SIZE: usize = 4096; + +/// Size of the VMA header without the blob buffer appended at the end +/// See VMA Header in pve-qemu.git/vma_spec.txt +const VMA_HEADER_SIZE_NO_BLOB_BUFFER: usize = 12288; + +type VmaDeviceId = u8; +type VmaDeviceOffset = u64; +type VmaDeviceSize = u64; + #[repr(C)] #[derive(Serialize, Deserialize)] struct VmaDeviceInfoHeader { pub device_name_offset: u32, reserved: [u8; 4], - pub device_size: u64, + pub device_size: VmaDeviceSize, reserved1: [u8; 16], } @@ -42,6 +73,8 @@ struct VmaHeader { reserved1: [u8; 4], #[serde(with = "BigArray")] pub dev_info: [VmaDeviceInfoHeader; VMA_MAX_DEVICES], + #[serde(skip_deserializing, skip_serializing)] + blob_buffer: Vec<u8>, } #[repr(C)] @@ -62,57 +95,46 @@ struct VmaExtentHeader { pub uuid: [u8; 16], pub md5sum: [u8; 16], #[serde(with = "BigArray")] - pub blockinfo: [VmaBlockInfo; VMA_BLOCKS_PER_EXTENT], + pub blockinfo: [VmaBlockInfo; VMA_CLUSTERS_PER_EXTENT], } -#[derive(Clone)] -struct VmaBlockIndexEntry { - pub cluster_file_offset: u64, - pub mask: u16, +#[derive(Clone, Eq, Hash, PartialEq)] +pub struct VmaConfig { + pub name: String, + pub content: String, } pub struct VmaReader { - vma_file: File, + vma_file: Box<dyn Read>, vma_header: VmaHeader, - configs: HashMap<String, String>, - block_index: Vec<Vec<VmaBlockIndexEntry>>, - blocks_are_indexed: bool, + configs: HashSet<VmaConfig>, } impl VmaReader { - pub fn new(vma_file_path: &str) -> Result<Self> { - let mut vma_file = match File::open(vma_file_path) { - Err(why) => return Err(anyhow!("couldn't open {}: {}", vma_file_path, why)), - Ok(file) => file, - }; - - let vma_header = Self::read_header(&mut vma_file).unwrap(); - let configs = Self::read_blob_buffer(&mut vma_file, &vma_header).unwrap(); - let block_index: Vec<Vec<VmaBlockIndexEntry>> = (0..256).map(|_| Vec::new()).collect(); + pub fn new(mut vma_file: impl Read + 'static) -> Result<Self> { + let vma_header = Self::read_header(&mut vma_file)?; + let configs = Self::read_configs(&vma_header)?; let instance = Self { - vma_file, + vma_file: Box::new(vma_file), vma_header, configs, - block_index, - blocks_are_indexed: false, }; Ok(instance) } - fn read_header(vma_file: &mut File) -> Result<VmaHeader> { - let mut buffer = Vec::with_capacity(size_of::<VmaHeader>()); - buffer.resize(size_of::<VmaHeader>(), 0); + fn read_header(vma_file: &mut impl Read) -> Result<VmaHeader> { + let mut buffer = vec![0; VMA_HEADER_SIZE_NO_BLOB_BUFFER]; vma_file.read_exact(&mut buffer)?; let bincode_options = bincode::DefaultOptions::new() .with_fixint_encoding() .with_big_endian(); - let vma_header: VmaHeader = bincode_options.deserialize(&buffer)?; + let mut vma_header: VmaHeader = bincode_options.deserialize(&buffer)?; - if vma_header.magic != [b'V', b'M', b'A', 0] { + if vma_header.magic != VMA_HEADER_MAGIC { return Err(anyhow!("Invalid magic number")); } @@ -121,7 +143,7 @@ impl VmaReader { } buffer.resize(vma_header.header_size as usize, 0); - vma_file.read_exact(&mut buffer[size_of::<VmaHeader>()..])?; + vma_file.read_exact(&mut buffer[VMA_HEADER_SIZE_NO_BLOB_BUFFER..])?; // Fill the MD5 sum field with zeros to compute the MD5 sum buffer[32..48].fill(0); @@ -131,89 +153,77 @@ impl VmaReader { return Err(anyhow!("Wrong VMA header checksum")); } - return Ok(vma_header); + let blob_buffer = &buffer[VMA_HEADER_SIZE_NO_BLOB_BUFFER..vma_header.header_size as usize]; + vma_header.blob_buffer = blob_buffer.to_vec(); + + Ok(vma_header) } - fn read_string_from_file(vma_file: &mut File, file_offset: u64) -> Result<String> { - let mut size_bytes = [0u8; 2]; - vma_file.seek(SeekFrom::Start(file_offset))?; - vma_file.read_exact(&mut size_bytes)?; + fn read_string(buffer: &[u8]) -> Result<String> { + let size_bytes: [u8; 2] = buffer[0..2].try_into()?; let size = u16::from_le_bytes(size_bytes) as usize; - let mut string_bytes = Vec::with_capacity(size - 1); - string_bytes.resize(size - 1, 0); - vma_file.read_exact(&mut string_bytes)?; - let string = str::from_utf8(&string_bytes)?; + let string_bytes: &[u8] = &buffer[2..1 + size]; + let string = str::from_utf8(string_bytes)?; - return Ok(string.to_string()); + Ok(String::from(string)) } - fn read_blob_buffer( - vma_file: &mut File, - vma_header: &VmaHeader, - ) -> Result<HashMap<String, String>> { - let mut configs = HashMap::new(); + fn read_configs(vma_header: &VmaHeader) -> Result<HashSet<VmaConfig>> { + let mut configs = HashSet::new(); for i in 0..VMA_MAX_CONFIGS { - let config_name_offset = vma_header.config_names[i]; - let config_data_offset = vma_header.config_data[i]; + let config_name_offset = vma_header.config_names[i] as usize; + let config_data_offset = vma_header.config_data[i] as usize; if config_name_offset == 0 || config_data_offset == 0 { continue; } - let config_name_file_offset = - (vma_header.blob_buffer_offset + config_name_offset) as u64; - let config_data_file_offset = - (vma_header.blob_buffer_offset + config_data_offset) as u64; - let config_name = Self::read_string_from_file(vma_file, config_name_file_offset)?; - let config_data = Self::read_string_from_file(vma_file, config_data_file_offset)?; - - configs.insert(String::from(config_name), String::from(config_data)); + let config_name = Self::read_string(&vma_header.blob_buffer[config_name_offset..])?; + let config_data = Self::read_string(&vma_header.blob_buffer[config_data_offset..])?; + let vma_config = VmaConfig { + name: config_name, + content: config_data, + }; + configs.insert(vma_config); } - return Ok(configs); + Ok(configs) } - pub fn get_configs(&self) -> HashMap<String, String> { - return self.configs.clone(); + pub fn get_configs(&self) -> HashSet<VmaConfig> { + self.configs.clone() } - pub fn get_device_name(&mut self, device_id: usize) -> Option<String> { - if device_id >= VMA_MAX_DEVICES { - return None; - } + pub fn contains_device(&self, device_id: VmaDeviceId) -> bool { + self.vma_header.dev_info[device_id as usize].device_name_offset != 0 + } - let device_name_offset = self.vma_header.dev_info[device_id].device_name_offset; + pub fn get_device_name(&self, device_id: VmaDeviceId) -> Result<String> { + let device_name_offset = + self.vma_header.dev_info[device_id as usize].device_name_offset as usize; if device_name_offset == 0 { - return None; + anyhow::bail!("device_name_offset cannot be 0"); } - let device_name_file_offset = - (self.vma_header.blob_buffer_offset + device_name_offset) as u64; - let device_name = - Self::read_string_from_file(&mut self.vma_file, device_name_file_offset).unwrap(); + let device_name = Self::read_string(&self.vma_header.blob_buffer[device_name_offset..])?; - return Some(device_name.to_string()); + Ok(device_name) } - pub fn get_device_size(&self, device_id: usize) -> Option<u64> { - if device_id >= VMA_MAX_DEVICES { - return None; - } - - let dev_info = &self.vma_header.dev_info[device_id]; + pub fn get_device_size(&self, device_id: VmaDeviceId) -> Result<VmaDeviceSize> { + let dev_info = &self.vma_header.dev_info[device_id as usize]; if dev_info.device_name_offset == 0 { - return None; + anyhow::bail!("device_name_offset cannot be 0"); } - return Some(dev_info.device_size); + Ok(dev_info.device_size) } - fn read_extent_header(vma_file: &mut File) -> Result<VmaExtentHeader> { - let mut buffer = Vec::with_capacity(size_of::<VmaExtentHeader>()); - buffer.resize(size_of::<VmaExtentHeader>(), 0); + fn read_extent_header(mut vma_file: impl Read) -> Result<VmaExtentHeader> { + let mut buffer = vec![0; size_of::<VmaExtentHeader>()]; vma_file.read_exact(&mut buffer)?; let bincode_options = bincode::DefaultOptions::new() @@ -222,7 +232,7 @@ impl VmaReader { let vma_extent_header: VmaExtentHeader = bincode_options.deserialize(&buffer)?; - if vma_extent_header.magic != [b'V', b'M', b'A', b'E'] { + if vma_extent_header.magic != VMA_EXTENT_HEADER_MAGIC { return Err(anyhow!("Invalid magic number")); } @@ -234,113 +244,75 @@ impl VmaReader { return Err(anyhow!("Wrong VMA extent header checksum")); } - return Ok(vma_extent_header); + Ok(vma_extent_header) } - fn index_device_clusters(&mut self) -> Result<()> { - for device_id in 0..255 { - let device_size = match self.get_device_size(device_id) { - Some(x) => x, - None => { - continue; - } - }; - - let device_cluster_count = (device_size + 4096 * 16 - 1) / (4096 * 16); + fn restore_extent<F>(&mut self, callback: F) -> Result<()> + where + F: Fn(VmaDeviceId, VmaDeviceOffset, Option<Vec<u8>>) -> Result<()>, + { + let vma_extent_header = Self::read_extent_header(&mut self.vma_file)?; - let block_index_entry_placeholder = VmaBlockIndexEntry { - cluster_file_offset: 0, - mask: 0, - }; - - self.block_index[device_id] - .resize(device_cluster_count as usize, block_index_entry_placeholder); - } + for cluster_index in 0..VMA_CLUSTERS_PER_EXTENT { + let blockinfo = &vma_extent_header.blockinfo[cluster_index]; - let mut file_offset = self.vma_header.header_size as u64; - let vma_file_size = self.vma_file.metadata()?.len(); - - while file_offset < vma_file_size { - self.vma_file.seek(SeekFrom::Start(file_offset))?; - let vma_extent_header = Self::read_extent_header(&mut self.vma_file)?; - file_offset += size_of::<VmaExtentHeader>() as u64; - - for i in 0..VMA_BLOCKS_PER_EXTENT { - let blockinfo = &vma_extent_header.blockinfo[i]; + if blockinfo.dev_id == 0 { + continue; + } - if blockinfo.dev_id == 0 { - continue; + let image_offset = + (BLOCK_SIZE * VMA_BLOCKS_PER_CLUSTER * blockinfo.cluster_num as usize) as u64; + let cluster_is_zero = blockinfo.mask == 0; + + let image_chunk_buffer = if cluster_is_zero { + None + } else { + let mut image_chunk_buffer = vec![0; BLOCK_SIZE * VMA_BLOCKS_PER_CLUSTER]; + + for block_index in 0..VMA_BLOCKS_PER_CLUSTER { + let block_is_zero = ((blockinfo.mask >> block_index) & 1) == 0; + let block_start = BLOCK_SIZE * block_index; + let block_end = block_start + BLOCK_SIZE; + + if block_is_zero { + image_chunk_buffer[block_start..block_end].fill(0); + } else { + self.vma_file + .read_exact(&mut image_chunk_buffer[block_start..block_end])?; + } } - let block_index_entry = VmaBlockIndexEntry { - cluster_file_offset: file_offset, - mask: blockinfo.mask, - }; + Some(image_chunk_buffer) + }; - self.block_index[blockinfo.dev_id as usize][blockinfo.cluster_num as usize] = - block_index_entry; - file_offset += blockinfo.mask.count_ones() as u64 * 4096; - } + callback(blockinfo.dev_id, image_offset, image_chunk_buffer)?; } - self.blocks_are_indexed = true; - - return Ok(()); + Ok(()) } - pub fn read_device_contents( - &mut self, - device_id: usize, - buffer: &mut [u8], - offset: u64, - ) -> Result<bool> { - if device_id >= VMA_MAX_DEVICES { - return Err(anyhow!("invalid device id {}", device_id)); - } - - if offset % (4096 * 16) != 0 { - return Err(anyhow!("offset is not aligned to 65536")); - } - - // Make sure that the device clusters are already indexed - if !self.blocks_are_indexed { - self.index_device_clusters()?; - } - - let this_device_block_index = &self.block_index[device_id]; - let length = cmp::min( - buffer.len(), - this_device_block_index.len() * 4096 * 16 - offset as usize, - ); - let mut buffer_offset = 0; - let mut buffer_is_zero = true; - - while buffer_offset < length { - let block_index_entry = - &this_device_block_index[(offset as usize + buffer_offset) / (4096 * 16)]; - self.vma_file - .seek(SeekFrom::Start(block_index_entry.cluster_file_offset))?; - - for i in 0..16 { - if buffer_offset >= length { - break; - } - - let block_buffer_end = buffer_offset + cmp::min(length - buffer_offset, 4096); - let block_mask = ((block_index_entry.mask >> i) & 1) == 1; - - if block_mask { - self.vma_file - .read_exact(&mut buffer[buffer_offset..block_buffer_end])?; - buffer_is_zero = false; - } else { - buffer[buffer_offset..block_buffer_end].fill(0); - } - - buffer_offset += 4096; + pub fn restore<F>(&mut self, callback: F) -> Result<()> + where + F: Fn(VmaDeviceId, VmaDeviceOffset, Option<Vec<u8>>) -> Result<()>, + { + loop { + match self.restore_extent(&callback) { + Ok(()) => {} + Err(e) => match e.downcast_ref::<std::io::Error>() { + Some(ioerr) => { + if ioerr.kind() == std::io::ErrorKind::UnexpectedEof { + break; // Break out of the loop since the end of the file was reached. + } else { + return Err(anyhow!("Failed to read VMA file: {}", ioerr)); + } + } + _ => { + return Err(anyhow::format_err!(e)); + } + }, } } - return Ok(buffer_is_zero); + Ok(()) } } diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs new file mode 100644 index 0000000..cef7b60 --- /dev/null +++ b/src/vma2pbs.rs @@ -0,0 +1,368 @@ +use std::cell::RefCell; +use std::collections::HashMap; +use std::ffi::{c_char, CStr, CString}; +use std::fs::File; +use std::io::{stdin, BufRead, BufReader}; +use std::ptr; +use std::time::{SystemTime, UNIX_EPOCH}; + +use anyhow::{anyhow, Result}; +use proxmox_backup_qemu::{ + capi_types::ProxmoxBackupHandle, proxmox_backup_add_config, proxmox_backup_close_image, + proxmox_backup_connect, proxmox_backup_disconnect, proxmox_backup_finish, + proxmox_backup_new_ns, proxmox_backup_register_image, proxmox_backup_write_data, + PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE, +}; +use scopeguard::defer; + +use crate::vma::VmaReader; + +const VMA_CLUSTER_SIZE: usize = 65536; + +pub struct BackupVmaToPbsArgs { + pub vma_file_path: Option<String>, + pub pbs_repository: String, + pub backup_id: String, + pub pbs_password: String, + pub keyfile: Option<String>, + pub key_password: Option<String>, + pub master_keyfile: Option<String>, + pub fingerprint: String, + pub compress: bool, + pub encrypt: bool, +} + +#[derive(Copy, Clone)] +struct BlockDeviceInfo { + pub pbs_device_id: u8, + pub device_size: u64, +} + +fn create_pbs_backup_task(args: BackupVmaToPbsArgs) -> Result<*mut ProxmoxBackupHandle> { + println!("PBS repository: {}", args.pbs_repository); + println!("PBS fingerprint: {}", args.fingerprint); + println!("compress: {}", args.compress); + println!("encrypt: {}", args.encrypt); + + let backup_time = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(); + println!("backup time: {}", backup_time); + + let mut pbs_err: *mut c_char = ptr::null_mut(); + + let pbs_repository_cstr = CString::new(args.pbs_repository)?; + let backup_id_cstr = CString::new(args.backup_id)?; + let pbs_password_cstr = CString::new(args.pbs_password)?; + let fingerprint_cstr = CString::new(args.fingerprint)?; + let keyfile_cstr = args.keyfile.map(|v| CString::new(v).unwrap()); + let keyfile_ptr = keyfile_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); + let key_password_cstr = args.key_password.map(|v| CString::new(v).unwrap()); + let key_password_ptr = key_password_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); + let master_keyfile_cstr = args.master_keyfile.map(|v| CString::new(v).unwrap()); + let master_keyfile_ptr = master_keyfile_cstr + .map(|v| v.as_ptr()) + .unwrap_or(ptr::null()); + + let pbs = proxmox_backup_new_ns( + pbs_repository_cstr.as_ptr(), + ptr::null(), + backup_id_cstr.as_ptr(), + backup_time, + PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE, + pbs_password_cstr.as_ptr(), + keyfile_ptr, + key_password_ptr, + master_keyfile_ptr, + true, + false, + fingerprint_cstr.as_ptr(), + &mut pbs_err, + ); + + if pbs.is_null() { + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; + anyhow::bail!("proxmox_backup_new_ns failed: {pbs_err_cstr:?}"); + } + + Ok(pbs) +} + +fn upload_configs(vma_reader: &VmaReader, pbs: *mut ProxmoxBackupHandle) -> Result<()> { + let mut pbs_err: *mut c_char = ptr::null_mut(); + let configs = vma_reader.get_configs(); + for config in configs { + let config_name = config.name; + let config_data = config.content; + + println!("CFG: size: {} name: {}", config_data.len(), config_name); + + let config_name_cstr = CString::new(config_name)?; + + if proxmox_backup_add_config( + pbs, + config_name_cstr.as_ptr(), + config_data.as_ptr(), + config_data.len() as u64, + &mut pbs_err, + ) < 0 + { + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; + anyhow::bail!("proxmox_backup_add_config failed: {pbs_err_cstr:?}"); + } + } + + Ok(()) +} + +fn register_block_devices( + vma_reader: &VmaReader, + pbs: *mut ProxmoxBackupHandle, +) -> Result<[Option<BlockDeviceInfo>; 256]> { + let mut block_device_infos: [Option<BlockDeviceInfo>; 256] = [None; 256]; + let mut pbs_err: *mut c_char = ptr::null_mut(); + + for device_id in 0..255 { + if !vma_reader.contains_device(device_id) { + continue; + } + + let device_name = vma_reader.get_device_name(device_id)?; + let device_size = vma_reader.get_device_size(device_id)?; + + println!( + "DEV: dev_id={} size: {} devname: {}", + device_id, device_size, device_name + ); + + let device_name_cstr = CString::new(device_name)?; + let pbs_device_id = proxmox_backup_register_image( + pbs, + device_name_cstr.as_ptr(), + device_size, + false, + &mut pbs_err, + ); + + if pbs_device_id < 0 { + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; + anyhow::bail!("proxmox_backup_register_image failed: {pbs_err_cstr:?}"); + } + + let block_device_info = BlockDeviceInfo { + pbs_device_id: pbs_device_id as u8, + device_size, + }; + + block_device_infos[device_id as usize] = Some(block_device_info); + } + + Ok(block_device_infos) +} + +fn upload_block_devices(mut vma_reader: VmaReader, pbs: *mut ProxmoxBackupHandle) -> Result<()> { + let block_device_infos = register_block_devices(&vma_reader, pbs)?; + + struct ImageChunk { + sub_chunks: HashMap<u8, Option<Vec<u8>>>, + mask: u64, + non_zero_mask: u64, + } + + let images_chunks: RefCell<HashMap<u8, HashMap<u64, ImageChunk>>> = + RefCell::new(HashMap::new()); + + vma_reader.restore(|dev_id, offset, buffer| { + let block_device_info = match block_device_infos[dev_id as usize] { + Some(block_device_info) => block_device_info, + None => anyhow::bail!("Reference to unknown device id {} in VMA file", dev_id), + }; + + let pbs_device_id = block_device_info.pbs_device_id; + let device_size = block_device_info.device_size; + + let mut images_chunks = images_chunks.borrow_mut(); + let image_chunks = images_chunks.get_mut(&dev_id); + let pbs_chunk_offset = + PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE * (offset / PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE); + let sub_chunk_index = + ((offset % PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE) / VMA_CLUSTER_SIZE as u64) as u8; + + let pbs_chunk_size = PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE.min(device_size - pbs_chunk_offset); + + let pbs_upload_chunk = |pbs_chunk_buffer: Option<&[u8]>| { + println!( + "Uploading dev_id: {} offset: {:#0X} - {:#0X}", + dev_id, + pbs_chunk_offset, + pbs_chunk_offset + pbs_chunk_size, + ); + + let mut pbs_err: *mut c_char = ptr::null_mut(); + + let write_data_result = proxmox_backup_write_data( + pbs, + pbs_device_id, + match pbs_chunk_buffer { + Some(pbs_chunk_buffer) => pbs_chunk_buffer.as_ptr(), + None => ptr::null(), + }, + pbs_chunk_offset, + pbs_chunk_size, + &mut pbs_err, + ); + + if write_data_result < 0 { + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; + anyhow::bail!("proxmox_backup_write_data failed: {pbs_err_cstr:?}"); + } + + Ok(()) + }; + + let insert_image_chunk = |image_chunks: &mut HashMap<u64, ImageChunk>, + buffer: Option<Vec<u8>>| { + let mut sub_chunks: HashMap<u8, Option<Vec<u8>>> = HashMap::new(); + let mask = 1 << sub_chunk_index; + let non_zero_mask = buffer.is_some() as u64; + sub_chunks.insert(sub_chunk_index, buffer); + + let image_chunk = ImageChunk { + sub_chunks, + mask, + non_zero_mask, + }; + + image_chunks.insert(pbs_chunk_offset, image_chunk); + }; + + match image_chunks { + Some(image_chunks) => { + let image_chunk = image_chunks.get_mut(&pbs_chunk_offset); + + match image_chunk { + Some(image_chunk) => { + image_chunk.mask |= 1 << sub_chunk_index; + image_chunk.non_zero_mask |= (buffer.is_some() as u64) << sub_chunk_index; + image_chunk.sub_chunks.insert(sub_chunk_index, buffer); + + let sub_chunk_count = + ((pbs_chunk_size + 65535) / VMA_CLUSTER_SIZE as u64) as u8; + let pbs_chunk_mask = 1_u64 + .checked_shl(sub_chunk_count.into()) + .unwrap_or(0) + .wrapping_sub(1); + + if image_chunk.mask == pbs_chunk_mask { + if image_chunk.non_zero_mask == 0 { + pbs_upload_chunk(None)?; + } else { + let mut pbs_chunk_buffer = + proxmox_io::boxed::zeroed(pbs_chunk_size as usize); + + for i in 0..sub_chunk_count { + let sub_chunk = &image_chunk.sub_chunks[&i]; + let start = i as usize * VMA_CLUSTER_SIZE; + let end = + (start + VMA_CLUSTER_SIZE).min(pbs_chunk_size as usize); + + match sub_chunk { + Some(sub_chunk) => { + pbs_chunk_buffer[start..end] + .copy_from_slice(&sub_chunk[0..end - start]); + } + None => pbs_chunk_buffer[start..end].fill(0), + } + } + + pbs_upload_chunk(Some(&*pbs_chunk_buffer))?; + } + + image_chunks.remove(&pbs_chunk_offset); + } + } + None => { + if pbs_chunk_size <= VMA_CLUSTER_SIZE as u64 { + pbs_upload_chunk(buffer.as_deref())?; + } else { + insert_image_chunk(image_chunks, buffer); + } + } + } + } + None => { + if pbs_chunk_size <= VMA_CLUSTER_SIZE as u64 { + pbs_upload_chunk(buffer.as_deref())?; + } else { + let mut image_chunks: HashMap<u64, ImageChunk> = HashMap::new(); + insert_image_chunk(&mut image_chunks, buffer); + images_chunks.insert(dev_id, image_chunks); + } + } + } + + Ok(()) + })?; + + let mut pbs_err: *mut c_char = ptr::null_mut(); + + for block_device_info in block_device_infos.iter().take(255) { + let block_device_info = match block_device_info { + Some(block_device_info) => block_device_info, + None => continue, + }; + + let pbs_device_id = block_device_info.pbs_device_id; + + if proxmox_backup_close_image(pbs, pbs_device_id, &mut pbs_err) < 0 { + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; + anyhow::bail!("proxmox_backup_close_image failed: {pbs_err_cstr:?}"); + } + } + + Ok(()) +} + +pub fn backup_vma_to_pbs(args: BackupVmaToPbsArgs) -> Result<()> { + let vma_file: Box<dyn BufRead> = match &args.vma_file_path { + Some(vma_file_path) => match File::open(vma_file_path) { + Err(why) => return Err(anyhow!("Couldn't open file: {}", why)), + Ok(file) => Box::new(BufReader::new(file)), + }, + None => Box::new(BufReader::new(stdin())), + }; + let vma_reader = VmaReader::new(vma_file)?; + + let pbs = create_pbs_backup_task(args)?; + + defer! { + proxmox_backup_disconnect(pbs); + } + + let mut pbs_err: *mut c_char = ptr::null_mut(); + let connect_result = proxmox_backup_connect(pbs, &mut pbs_err); + + if connect_result < 0 { + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; + anyhow::bail!("proxmox_backup_connect failed: {pbs_err_cstr:?}"); + } + + println!("Connected to Proxmox Backup Server"); + + let start_transfer_time = SystemTime::now(); + + upload_configs(&vma_reader, pbs)?; + upload_block_devices(vma_reader, pbs)?; + + if proxmox_backup_finish(pbs, &mut pbs_err) < 0 { + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; + anyhow::bail!("proxmox_backup_finish failed: {pbs_err_cstr:?}"); + } + + let elapsed_time = SystemTime::now().duration_since(start_transfer_time)?; + let total_ms = elapsed_time.as_millis(); + let ms = total_ms % 1000; + let seconds = (total_ms / 1000) % 60; + let minutes = total_ms / 1000000; + println!("Backup finished within {minutes} minutes, {seconds} seconds and {ms} ms"); + + Ok(()) +} -- 2.39.2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs 07/10] add support for streaming the VMA file via stdin 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 07/10] add support for streaming the VMA file via stdin Filip Schauer @ 2024-03-25 12:34 ` Max Carrara 0 siblings, 0 replies; 23+ messages in thread From: Max Carrara @ 2024-03-25 12:34 UTC (permalink / raw) To: Proxmox Backup Server development discussion On Wed Mar 20, 2024 at 3:15 PM CET, Filip Schauer wrote: > This allows the user to stream a compressed VMA file directly to a PBS > without the need to extract it to an intermediate .vma file. > > Example usage: > > zstd -d --stdout vzdump.vma.zst | vma-to-pbs \ > --repository <auth_id@host:port:datastore> \ > --vmid 123 \ > --password_file pbs_password > > Signed-off-by: Filip Schauer <f.schauer@proxmox.com> > --- > This requires > https://lists.proxmox.com/pipermail/pve-devel/2024-March/062182.html > to be applied first. > > src/main.rs | 241 ++------------------------------ > src/vma.rs | 326 ++++++++++++++++++++----------------------- > src/vma2pbs.rs | 368 +++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 530 insertions(+), 405 deletions(-) > create mode 100644 src/vma2pbs.rs > > diff --git a/src/main.rs b/src/main.rs > index f619b3e..e0e1076 100644 > --- a/src/main.rs > +++ b/src/main.rs > @@ -1,228 +1,10 @@ > -use std::env; > -use std::ffi::{c_char, CStr, CString}; > -use std::ptr; > -use std::time::{SystemTime, UNIX_EPOCH}; > - > -use anyhow::{anyhow, Context, Result}; > +use anyhow::Result; > use clap::{command, Arg, ArgAction}; > -use proxmox_backup_qemu::*; > use proxmox_sys::linux::tty; > -use scopeguard::defer; > > mod vma; > -use vma::*; > - > -fn backup_vma_to_pbs( > - vma_file_path: String, > - pbs_repository: String, > - backup_id: String, > - pbs_password: String, > - keyfile: Option<String>, > - key_password: Option<String>, > - master_keyfile: Option<String>, > - fingerprint: String, > - compress: bool, > - encrypt: bool, > -) -> Result<()> { > - println!("VMA input file: {}", vma_file_path); > - println!("PBS repository: {}", pbs_repository); > - println!("PBS fingerprint: {}", fingerprint); > - println!("compress: {}", compress); > - println!("encrypt: {}", encrypt); > - > - let backup_time = SystemTime::now() > - .duration_since(UNIX_EPOCH) > - .unwrap() > - .as_secs(); > - println!("backup time: {}", backup_time); > - > - let mut pbs_err: *mut c_char = ptr::null_mut(); > - > - let pbs_repository_cstr = CString::new(pbs_repository).unwrap(); > - let backup_id_cstr = CString::new(backup_id).unwrap(); > - let pbs_password_cstr = CString::new(pbs_password).unwrap(); > - let fingerprint_cstr = CString::new(fingerprint).unwrap(); > - let keyfile_cstr = keyfile.map(|v| CString::new(v).unwrap()); > - let keyfile_ptr = keyfile_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); > - let key_password_cstr = key_password.map(|v| CString::new(v).unwrap()); > - let key_password_ptr = key_password_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); > - let master_keyfile_cstr = master_keyfile.map(|v| CString::new(v).unwrap()); > - let master_keyfile_ptr = master_keyfile_cstr > - .map(|v| v.as_ptr()) > - .unwrap_or(ptr::null()); > - > - let pbs = proxmox_backup_new_ns( > - pbs_repository_cstr.as_ptr(), > - ptr::null(), > - backup_id_cstr.as_ptr(), > - backup_time, > - PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE, > - pbs_password_cstr.as_ptr(), > - keyfile_ptr, > - key_password_ptr, > - master_keyfile_ptr, > - true, > - false, > - fingerprint_cstr.as_ptr(), > - &mut pbs_err, > - ); > - > - defer! { > - proxmox_backup_disconnect(pbs); > - } > - > - if pbs == ptr::null_mut() { > - unsafe { > - let pbs_err_cstr = CStr::from_ptr(pbs_err); > - return Err(anyhow!("proxmox_backup_new_ns failed: {pbs_err_cstr:?}")); > - } > - } > - > - let connect_result = proxmox_backup_connect(pbs, &mut pbs_err); > - > - if connect_result < 0 { > - unsafe { > - let pbs_err_cstr = CStr::from_ptr(pbs_err); > - return Err(anyhow!("proxmox_backup_connect failed: {pbs_err_cstr:?}")); > - } > - } > - > - let mut vma_reader = VmaReader::new(&vma_file_path)?; > - > - // Handle configs > - let configs = vma_reader.get_configs(); > - for (config_name, config_data) in configs { > - println!("CFG: size: {} name: {}", config_data.len(), config_name); > - > - let config_name_cstr = CString::new(config_name).unwrap(); > - > - if proxmox_backup_add_config( > - pbs, > - config_name_cstr.as_ptr(), > - config_data.as_ptr(), > - config_data.len() as u64, > - &mut pbs_err, > - ) < 0 > - { > - unsafe { > - let pbs_err_cstr = CStr::from_ptr(pbs_err); > - return Err(anyhow!( > - "proxmox_backup_add_config failed: {pbs_err_cstr:?}" > - )); > - } > - } > - } > - > - // Handle block devices > - for device_id in 0..255 { > - let device_name = match vma_reader.get_device_name(device_id) { > - Some(x) => x, > - None => { > - continue; > - } > - }; > - > - let device_size = match vma_reader.get_device_size(device_id) { > - Some(x) => x, > - None => { > - continue; > - } > - }; > - > - println!( > - "DEV: dev_id={} size: {} devname: {}", > - device_id, device_size, device_name > - ); > - > - let device_name_cstr = CString::new(device_name).unwrap(); > - let pbs_device_id = proxmox_backup_register_image( > - pbs, > - device_name_cstr.as_ptr(), > - device_size, > - false, > - &mut pbs_err, > - ); > - > - if pbs_device_id < 0 { > - unsafe { > - let pbs_err_cstr = CStr::from_ptr(pbs_err); > - return Err(anyhow!( > - "proxmox_backup_register_image failed: {pbs_err_cstr:?}" > - )); > - } > - } > - > - let mut image_chunk_buffer = > - proxmox_io::boxed::zeroed(PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE as usize); > - let mut bytes_transferred = 0; > - > - while bytes_transferred < device_size { > - let bytes_left = device_size - bytes_transferred; > - let chunk_size = bytes_left.min(PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE); > - println!( > - "Uploading dev_id: {} offset: {:#0X} - {:#0X}", > - device_id, > - bytes_transferred, > - bytes_transferred + chunk_size > - ); > - > - let is_zero_chunk = vma_reader > - .read_device_contents( > - device_id, > - &mut image_chunk_buffer[0..chunk_size as usize], > - bytes_transferred, > - ) > - .with_context(|| { > - format!( > - "read {} bytes at offset {} from disk {} from VMA file", > - chunk_size, bytes_transferred, device_id > - ) > - })?; > - > - let write_data_result = proxmox_backup_write_data( > - pbs, > - pbs_device_id as u8, > - if is_zero_chunk { > - ptr::null() > - } else { > - image_chunk_buffer.as_ptr() > - }, > - bytes_transferred, > - chunk_size, > - &mut pbs_err, > - ); > - > - if write_data_result < 0 { > - unsafe { > - let pbs_err_cstr = CStr::from_ptr(pbs_err); > - return Err(anyhow!( > - "proxmox_backup_write_data failed: {pbs_err_cstr:?}" > - )); > - } > - } > - > - bytes_transferred += chunk_size; > - } > - > - if proxmox_backup_close_image(pbs, pbs_device_id as u8, &mut pbs_err) < 0 { > - unsafe { > - let pbs_err_cstr = CStr::from_ptr(pbs_err); > - return Err(anyhow!( > - "proxmox_backup_close_image failed: {pbs_err_cstr:?}" > - )); > - } > - } > - } > - > - if proxmox_backup_finish(pbs, &mut pbs_err) < 0 { > - unsafe { > - let pbs_err_cstr = CStr::from_ptr(pbs_err); > - return Err(anyhow!("proxmox_backup_finish failed: {pbs_err_cstr:?}")); > - } > - } > - > - Ok(()) > -} > +mod vma2pbs; > +use vma2pbs::{backup_vma_to_pbs, BackupVmaToPbsArgs}; > > fn main() -> Result<()> { > let matches = command!() > @@ -300,7 +82,8 @@ fn main() -> Result<()> { > let compress = matches.get_flag("compress"); > let encrypt = matches.get_flag("encrypt"); > > - let vma_file_path = matches.get_one::<String>("vma_file").unwrap().to_string(); > + let vma_file_path = matches.get_one::<String>("vma_file"); > + > let password_file = matches.get_one::<String>("password_file"); > > let pbs_password = match password_file { > @@ -323,18 +106,20 @@ fn main() -> Result<()> { > None => None, > }; > > - backup_vma_to_pbs( > - vma_file_path, > + let args = BackupVmaToPbsArgs { > + vma_file_path: vma_file_path.cloned(), > pbs_repository, > - vmid, > + backup_id: vmid, > pbs_password, > - keyfile.cloned(), > + keyfile: keyfile.cloned(), > key_password, > - master_keyfile.cloned(), > + master_keyfile: master_keyfile.cloned(), > fingerprint, > compress, > encrypt, > - )?; > + }; > + > + backup_vma_to_pbs(args)?; Very glad to see an arg struct being used here now! :) > > Ok(()) > } > diff --git a/src/vma.rs b/src/vma.rs > index d30cb09..d369b36 100644 > --- a/src/vma.rs > +++ b/src/vma.rs > @@ -1,24 +1,55 @@ > -use std::collections::HashMap; > -use std::fs::File; > -use std::io::{Read, Seek, SeekFrom}; > +use std::collections::HashSet; > +use std::io::Read; > use std::mem::size_of; > -use std::{cmp, str}; > +use std::str; > > use anyhow::{anyhow, Result}; > use bincode::Options; > use serde::{Deserialize, Serialize}; > use serde_big_array::BigArray; > > -const VMA_BLOCKS_PER_EXTENT: usize = 59; > +/// Maximum number of clusters in an extent > +/// See Image Data Streams in pve-qemu.git/vma_spec.txt > +const VMA_CLUSTERS_PER_EXTENT: usize = 59; > + > +/// Number of 4k blocks per cluster > +/// See pve-qemu.git/vma_spec.txt > +const VMA_BLOCKS_PER_CLUSTER: usize = 16; > + > +/// Maximum number of config files > +/// See VMA Header in pve-qemu.git/vma_spec.txt > const VMA_MAX_CONFIGS: usize = 256; > + > +/// Maximum number of block devices > +/// See VMA Header in pve-qemu.git/vma_spec.txt > const VMA_MAX_DEVICES: usize = 256; > > +/// VMA magic string > +/// See VMA Header in pve-qemu.git/vma_spec.txt > +const VMA_HEADER_MAGIC: [u8; 4] = [b'V', b'M', b'A', 0]; > + > +/// VMA extent magic string > +/// See VMA Extent Header in pve-qemu.git/vma_spec.txt > +const VMA_EXTENT_HEADER_MAGIC: [u8; 4] = [b'V', b'M', b'A', b'E']; > + > +/// Size of a block > +/// See pve-qemu.git/vma_spec.txt > +const BLOCK_SIZE: usize = 4096; > + > +/// Size of the VMA header without the blob buffer appended at the end > +/// See VMA Header in pve-qemu.git/vma_spec.txt > +const VMA_HEADER_SIZE_NO_BLOB_BUFFER: usize = 12288; > + > +type VmaDeviceId = u8; > +type VmaDeviceOffset = u64; > +type VmaDeviceSize = u64; > + > #[repr(C)] > #[derive(Serialize, Deserialize)] > struct VmaDeviceInfoHeader { > pub device_name_offset: u32, > reserved: [u8; 4], > - pub device_size: u64, > + pub device_size: VmaDeviceSize, > reserved1: [u8; 16], > } > > @@ -42,6 +73,8 @@ struct VmaHeader { > reserved1: [u8; 4], > #[serde(with = "BigArray")] > pub dev_info: [VmaDeviceInfoHeader; VMA_MAX_DEVICES], > + #[serde(skip_deserializing, skip_serializing)] > + blob_buffer: Vec<u8>, > } > > #[repr(C)] > @@ -62,57 +95,46 @@ struct VmaExtentHeader { > pub uuid: [u8; 16], > pub md5sum: [u8; 16], > #[serde(with = "BigArray")] > - pub blockinfo: [VmaBlockInfo; VMA_BLOCKS_PER_EXTENT], > + pub blockinfo: [VmaBlockInfo; VMA_CLUSTERS_PER_EXTENT], > } > > -#[derive(Clone)] > -struct VmaBlockIndexEntry { > - pub cluster_file_offset: u64, > - pub mask: u16, > +#[derive(Clone, Eq, Hash, PartialEq)] > +pub struct VmaConfig { > + pub name: String, > + pub content: String, > } > > pub struct VmaReader { > - vma_file: File, > + vma_file: Box<dyn Read>, > vma_header: VmaHeader, > - configs: HashMap<String, String>, > - block_index: Vec<Vec<VmaBlockIndexEntry>>, > - blocks_are_indexed: bool, > + configs: HashSet<VmaConfig>, > } > > impl VmaReader { > - pub fn new(vma_file_path: &str) -> Result<Self> { > - let mut vma_file = match File::open(vma_file_path) { > - Err(why) => return Err(anyhow!("couldn't open {}: {}", vma_file_path, why)), > - Ok(file) => file, > - }; > - > - let vma_header = Self::read_header(&mut vma_file).unwrap(); > - let configs = Self::read_blob_buffer(&mut vma_file, &vma_header).unwrap(); > - let block_index: Vec<Vec<VmaBlockIndexEntry>> = (0..256).map(|_| Vec::new()).collect(); > + pub fn new(mut vma_file: impl Read + 'static) -> Result<Self> { Still not too sure about this here - I guess it's *fine* to stay with `impl Read` here and `Box<dyn Read>` in the struct above, but IMO if you're already generic here, the `VmaReader` might as well become a `VmaReader<T>`. The relevant `impl` blocks where `Read` is necessary could then just be constrained. Otherwise, the stuff given to `new()` here is being monomorphized and _then_ made into a trait object and boxed. Sure, it's a minor thing, but still some unnecessary work. > + let vma_header = Self::read_header(&mut vma_file)?; > + let configs = Self::read_configs(&vma_header)?; > > let instance = Self { > - vma_file, > + vma_file: Box::new(vma_file), > vma_header, > configs, > - block_index, > - blocks_are_indexed: false, > }; > > Ok(instance) > } > > - fn read_header(vma_file: &mut File) -> Result<VmaHeader> { > - let mut buffer = Vec::with_capacity(size_of::<VmaHeader>()); > - buffer.resize(size_of::<VmaHeader>(), 0); > + fn read_header(vma_file: &mut impl Read) -> Result<VmaHeader> { > + let mut buffer = vec![0; VMA_HEADER_SIZE_NO_BLOB_BUFFER]; > vma_file.read_exact(&mut buffer)?; > > let bincode_options = bincode::DefaultOptions::new() > .with_fixint_encoding() > .with_big_endian(); > > - let vma_header: VmaHeader = bincode_options.deserialize(&buffer)?; > + let mut vma_header: VmaHeader = bincode_options.deserialize(&buffer)?; > > - if vma_header.magic != [b'V', b'M', b'A', 0] { > + if vma_header.magic != VMA_HEADER_MAGIC { > return Err(anyhow!("Invalid magic number")); > } > > @@ -121,7 +143,7 @@ impl VmaReader { > } > > buffer.resize(vma_header.header_size as usize, 0); > - vma_file.read_exact(&mut buffer[size_of::<VmaHeader>()..])?; > + vma_file.read_exact(&mut buffer[VMA_HEADER_SIZE_NO_BLOB_BUFFER..])?; > > // Fill the MD5 sum field with zeros to compute the MD5 sum > buffer[32..48].fill(0); > @@ -131,89 +153,77 @@ impl VmaReader { > return Err(anyhow!("Wrong VMA header checksum")); > } > > - return Ok(vma_header); > + let blob_buffer = &buffer[VMA_HEADER_SIZE_NO_BLOB_BUFFER..vma_header.header_size as usize]; > + vma_header.blob_buffer = blob_buffer.to_vec(); > + > + Ok(vma_header) > } > > - fn read_string_from_file(vma_file: &mut File, file_offset: u64) -> Result<String> { > - let mut size_bytes = [0u8; 2]; > - vma_file.seek(SeekFrom::Start(file_offset))?; > - vma_file.read_exact(&mut size_bytes)?; > + fn read_string(buffer: &[u8]) -> Result<String> { > + let size_bytes: [u8; 2] = buffer[0..2].try_into()?; > let size = u16::from_le_bytes(size_bytes) as usize; > - let mut string_bytes = Vec::with_capacity(size - 1); > - string_bytes.resize(size - 1, 0); > - vma_file.read_exact(&mut string_bytes)?; > - let string = str::from_utf8(&string_bytes)?; > + let string_bytes: &[u8] = &buffer[2..1 + size]; > + let string = str::from_utf8(string_bytes)?; > > - return Ok(string.to_string()); > + Ok(String::from(string)) > } > > - fn read_blob_buffer( > - vma_file: &mut File, > - vma_header: &VmaHeader, > - ) -> Result<HashMap<String, String>> { > - let mut configs = HashMap::new(); > + fn read_configs(vma_header: &VmaHeader) -> Result<HashSet<VmaConfig>> { > + let mut configs = HashSet::new(); > > for i in 0..VMA_MAX_CONFIGS { > - let config_name_offset = vma_header.config_names[i]; > - let config_data_offset = vma_header.config_data[i]; > + let config_name_offset = vma_header.config_names[i] as usize; > + let config_data_offset = vma_header.config_data[i] as usize; > > if config_name_offset == 0 || config_data_offset == 0 { > continue; > } > > - let config_name_file_offset = > - (vma_header.blob_buffer_offset + config_name_offset) as u64; > - let config_data_file_offset = > - (vma_header.blob_buffer_offset + config_data_offset) as u64; > - let config_name = Self::read_string_from_file(vma_file, config_name_file_offset)?; > - let config_data = Self::read_string_from_file(vma_file, config_data_file_offset)?; > - > - configs.insert(String::from(config_name), String::from(config_data)); > + let config_name = Self::read_string(&vma_header.blob_buffer[config_name_offset..])?; > + let config_data = Self::read_string(&vma_header.blob_buffer[config_data_offset..])?; > + let vma_config = VmaConfig { > + name: config_name, > + content: config_data, > + }; > + configs.insert(vma_config); > } > > - return Ok(configs); > + Ok(configs) > } > > - pub fn get_configs(&self) -> HashMap<String, String> { > - return self.configs.clone(); > + pub fn get_configs(&self) -> HashSet<VmaConfig> { > + self.configs.clone() > } > > - pub fn get_device_name(&mut self, device_id: usize) -> Option<String> { > - if device_id >= VMA_MAX_DEVICES { > - return None; > - } > + pub fn contains_device(&self, device_id: VmaDeviceId) -> bool { > + self.vma_header.dev_info[device_id as usize].device_name_offset != 0 > + } > > - let device_name_offset = self.vma_header.dev_info[device_id].device_name_offset; > + pub fn get_device_name(&self, device_id: VmaDeviceId) -> Result<String> { > + let device_name_offset = > + self.vma_header.dev_info[device_id as usize].device_name_offset as usize; > > if device_name_offset == 0 { > - return None; > + anyhow::bail!("device_name_offset cannot be 0"); > } > > - let device_name_file_offset = > - (self.vma_header.blob_buffer_offset + device_name_offset) as u64; > - let device_name = > - Self::read_string_from_file(&mut self.vma_file, device_name_file_offset).unwrap(); > + let device_name = Self::read_string(&self.vma_header.blob_buffer[device_name_offset..])?; > > - return Some(device_name.to_string()); > + Ok(device_name) > } > > - pub fn get_device_size(&self, device_id: usize) -> Option<u64> { > - if device_id >= VMA_MAX_DEVICES { > - return None; > - } > - > - let dev_info = &self.vma_header.dev_info[device_id]; > + pub fn get_device_size(&self, device_id: VmaDeviceId) -> Result<VmaDeviceSize> { > + let dev_info = &self.vma_header.dev_info[device_id as usize]; > > if dev_info.device_name_offset == 0 { > - return None; > + anyhow::bail!("device_name_offset cannot be 0"); It's okay if you `use anyhow::bail` above, no need to fully qualify it ;) > } > > - return Some(dev_info.device_size); > + Ok(dev_info.device_size) > } > > - fn read_extent_header(vma_file: &mut File) -> Result<VmaExtentHeader> { > - let mut buffer = Vec::with_capacity(size_of::<VmaExtentHeader>()); > - buffer.resize(size_of::<VmaExtentHeader>(), 0); > + fn read_extent_header(mut vma_file: impl Read) -> Result<VmaExtentHeader> { > + let mut buffer = vec![0; size_of::<VmaExtentHeader>()]; > vma_file.read_exact(&mut buffer)?; > > let bincode_options = bincode::DefaultOptions::new() > @@ -222,7 +232,7 @@ impl VmaReader { > > let vma_extent_header: VmaExtentHeader = bincode_options.deserialize(&buffer)?; > > - if vma_extent_header.magic != [b'V', b'M', b'A', b'E'] { > + if vma_extent_header.magic != VMA_EXTENT_HEADER_MAGIC { > return Err(anyhow!("Invalid magic number")); > } > > @@ -234,113 +244,75 @@ impl VmaReader { > return Err(anyhow!("Wrong VMA extent header checksum")); > } > > - return Ok(vma_extent_header); > + Ok(vma_extent_header) > } > > - fn index_device_clusters(&mut self) -> Result<()> { > - for device_id in 0..255 { > - let device_size = match self.get_device_size(device_id) { > - Some(x) => x, > - None => { > - continue; > - } > - }; > - > - let device_cluster_count = (device_size + 4096 * 16 - 1) / (4096 * 16); > + fn restore_extent<F>(&mut self, callback: F) -> Result<()> > + where > + F: Fn(VmaDeviceId, VmaDeviceOffset, Option<Vec<u8>>) -> Result<()>, > + { > + let vma_extent_header = Self::read_extent_header(&mut self.vma_file)?; > > - let block_index_entry_placeholder = VmaBlockIndexEntry { > - cluster_file_offset: 0, > - mask: 0, > - }; > - > - self.block_index[device_id] > - .resize(device_cluster_count as usize, block_index_entry_placeholder); > - } > + for cluster_index in 0..VMA_CLUSTERS_PER_EXTENT { > + let blockinfo = &vma_extent_header.blockinfo[cluster_index]; > > - let mut file_offset = self.vma_header.header_size as u64; > - let vma_file_size = self.vma_file.metadata()?.len(); > - > - while file_offset < vma_file_size { > - self.vma_file.seek(SeekFrom::Start(file_offset))?; > - let vma_extent_header = Self::read_extent_header(&mut self.vma_file)?; > - file_offset += size_of::<VmaExtentHeader>() as u64; > - > - for i in 0..VMA_BLOCKS_PER_EXTENT { > - let blockinfo = &vma_extent_header.blockinfo[i]; > + if blockinfo.dev_id == 0 { > + continue; > + } > > - if blockinfo.dev_id == 0 { > - continue; > + let image_offset = > + (BLOCK_SIZE * VMA_BLOCKS_PER_CLUSTER * blockinfo.cluster_num as usize) as u64; > + let cluster_is_zero = blockinfo.mask == 0; > + > + let image_chunk_buffer = if cluster_is_zero { > + None > + } else { > + let mut image_chunk_buffer = vec![0; BLOCK_SIZE * VMA_BLOCKS_PER_CLUSTER]; > + > + for block_index in 0..VMA_BLOCKS_PER_CLUSTER { > + let block_is_zero = ((blockinfo.mask >> block_index) & 1) == 0; > + let block_start = BLOCK_SIZE * block_index; > + let block_end = block_start + BLOCK_SIZE; > + > + if block_is_zero { > + image_chunk_buffer[block_start..block_end].fill(0); > + } else { > + self.vma_file > + .read_exact(&mut image_chunk_buffer[block_start..block_end])?; > + } > } > > - let block_index_entry = VmaBlockIndexEntry { > - cluster_file_offset: file_offset, > - mask: blockinfo.mask, > - }; > + Some(image_chunk_buffer) > + }; > > - self.block_index[blockinfo.dev_id as usize][blockinfo.cluster_num as usize] = > - block_index_entry; > - file_offset += blockinfo.mask.count_ones() as u64 * 4096; > - } > + callback(blockinfo.dev_id, image_offset, image_chunk_buffer)?; > } > > - self.blocks_are_indexed = true; > - > - return Ok(()); > + Ok(()) > } > > - pub fn read_device_contents( > - &mut self, > - device_id: usize, > - buffer: &mut [u8], > - offset: u64, > - ) -> Result<bool> { > - if device_id >= VMA_MAX_DEVICES { > - return Err(anyhow!("invalid device id {}", device_id)); > - } > - > - if offset % (4096 * 16) != 0 { > - return Err(anyhow!("offset is not aligned to 65536")); > - } > - > - // Make sure that the device clusters are already indexed > - if !self.blocks_are_indexed { > - self.index_device_clusters()?; > - } > - > - let this_device_block_index = &self.block_index[device_id]; > - let length = cmp::min( > - buffer.len(), > - this_device_block_index.len() * 4096 * 16 - offset as usize, > - ); > - let mut buffer_offset = 0; > - let mut buffer_is_zero = true; > - > - while buffer_offset < length { > - let block_index_entry = > - &this_device_block_index[(offset as usize + buffer_offset) / (4096 * 16)]; > - self.vma_file > - .seek(SeekFrom::Start(block_index_entry.cluster_file_offset))?; > - > - for i in 0..16 { > - if buffer_offset >= length { > - break; > - } > - > - let block_buffer_end = buffer_offset + cmp::min(length - buffer_offset, 4096); > - let block_mask = ((block_index_entry.mask >> i) & 1) == 1; > - > - if block_mask { > - self.vma_file > - .read_exact(&mut buffer[buffer_offset..block_buffer_end])?; > - buffer_is_zero = false; > - } else { > - buffer[buffer_offset..block_buffer_end].fill(0); > - } > - > - buffer_offset += 4096; > + pub fn restore<F>(&mut self, callback: F) -> Result<()> > + where > + F: Fn(VmaDeviceId, VmaDeviceOffset, Option<Vec<u8>>) -> Result<()>, > + { > + loop { > + match self.restore_extent(&callback) { > + Ok(()) => {} > + Err(e) => match e.downcast_ref::<std::io::Error>() { > + Some(ioerr) => { > + if ioerr.kind() == std::io::ErrorKind::UnexpectedEof { > + break; // Break out of the loop since the end of the file was reached. > + } else { > + return Err(anyhow!("Failed to read VMA file: {}", ioerr)); > + } > + } > + _ => { > + return Err(anyhow::format_err!(e)); > + } > + }, > } > } > > - return Ok(buffer_is_zero); > + Ok(()) > } > } > diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs > new file mode 100644 > index 0000000..cef7b60 > --- /dev/null > +++ b/src/vma2pbs.rs > @@ -0,0 +1,368 @@ > +use std::cell::RefCell; > +use std::collections::HashMap; > +use std::ffi::{c_char, CStr, CString}; > +use std::fs::File; > +use std::io::{stdin, BufRead, BufReader}; > +use std::ptr; > +use std::time::{SystemTime, UNIX_EPOCH}; > + > +use anyhow::{anyhow, Result}; > +use proxmox_backup_qemu::{ > + capi_types::ProxmoxBackupHandle, proxmox_backup_add_config, proxmox_backup_close_image, > + proxmox_backup_connect, proxmox_backup_disconnect, proxmox_backup_finish, > + proxmox_backup_new_ns, proxmox_backup_register_image, proxmox_backup_write_data, > + PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE, > +}; > +use scopeguard::defer; > + > +use crate::vma::VmaReader; > + > +const VMA_CLUSTER_SIZE: usize = 65536; > + > +pub struct BackupVmaToPbsArgs { > + pub vma_file_path: Option<String>, > + pub pbs_repository: String, > + pub backup_id: String, > + pub pbs_password: String, > + pub keyfile: Option<String>, > + pub key_password: Option<String>, > + pub master_keyfile: Option<String>, > + pub fingerprint: String, > + pub compress: bool, > + pub encrypt: bool, > +} > + > +#[derive(Copy, Clone)] > +struct BlockDeviceInfo { > + pub pbs_device_id: u8, > + pub device_size: u64, > +} > + > +fn create_pbs_backup_task(args: BackupVmaToPbsArgs) -> Result<*mut ProxmoxBackupHandle> { Ugh, I know all of the `proxmox-backup-qemu`-related things take a `*mut` - so I guess passing that around here is fine, as it makes things easier to read... (In an ideal world that API would take a `NonNull<ProxmoxBackupHandle>` <.<) > + println!("PBS repository: {}", args.pbs_repository); > + println!("PBS fingerprint: {}", args.fingerprint); > + println!("compress: {}", args.compress); > + println!("encrypt: {}", args.encrypt); > + > + let backup_time = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(); > + println!("backup time: {}", backup_time); Should maybe format the time a little different; I e.g. get: [...] backup time: 1711359868 [...] > + > + let mut pbs_err: *mut c_char = ptr::null_mut(); > + > + let pbs_repository_cstr = CString::new(args.pbs_repository)?; > + let backup_id_cstr = CString::new(args.backup_id)?; > + let pbs_password_cstr = CString::new(args.pbs_password)?; > + let fingerprint_cstr = CString::new(args.fingerprint)?; > + let keyfile_cstr = args.keyfile.map(|v| CString::new(v).unwrap()); > + let keyfile_ptr = keyfile_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); > + let key_password_cstr = args.key_password.map(|v| CString::new(v).unwrap()); > + let key_password_ptr = key_password_cstr.map(|v| v.as_ptr()).unwrap_or(ptr::null()); > + let master_keyfile_cstr = args.master_keyfile.map(|v| CString::new(v).unwrap()); > + let master_keyfile_ptr = master_keyfile_cstr > + .map(|v| v.as_ptr()) > + .unwrap_or(ptr::null()); > + > + let pbs = proxmox_backup_new_ns( > + pbs_repository_cstr.as_ptr(), > + ptr::null(), > + backup_id_cstr.as_ptr(), > + backup_time, > + PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE, > + pbs_password_cstr.as_ptr(), > + keyfile_ptr, > + key_password_ptr, > + master_keyfile_ptr, > + true, > + false, > + fingerprint_cstr.as_ptr(), > + &mut pbs_err, > + ); > + > + if pbs.is_null() { > + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; > + anyhow::bail!("proxmox_backup_new_ns failed: {pbs_err_cstr:?}"); > + } As mentioned in my previous review, you really should put this kind of error handling / propagation into some kind of helper function. You want to make sure that all necessary invariants for `CStr::from_ptr` [0] are upheld. I know, this is our API and it's very unlikely that we'll do anything funky with those pointers, but we should nevertheless never rely on C FFI stuff to always remain sound IMO, as mistakes / accidents can happen. All it takes is one faulty commit in `proxmox-backup-qemu`. > + > + Ok(pbs) > +} > + > +fn upload_configs(vma_reader: &VmaReader, pbs: *mut ProxmoxBackupHandle) -> Result<()> { > + let mut pbs_err: *mut c_char = ptr::null_mut(); > + let configs = vma_reader.get_configs(); > + for config in configs { > + let config_name = config.name; > + let config_data = config.content; > + > + println!("CFG: size: {} name: {}", config_data.len(), config_name); > + > + let config_name_cstr = CString::new(config_name)?; > + > + if proxmox_backup_add_config( > + pbs, > + config_name_cstr.as_ptr(), > + config_data.as_ptr(), > + config_data.len() as u64, > + &mut pbs_err, > + ) < 0 > + { > + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; > + anyhow::bail!("proxmox_backup_add_config failed: {pbs_err_cstr:?}"); > + } Same as above (error handling). > + } > + > + Ok(()) > +} > + > +fn register_block_devices( > + vma_reader: &VmaReader, > + pbs: *mut ProxmoxBackupHandle, > +) -> Result<[Option<BlockDeviceInfo>; 256]> { > + let mut block_device_infos: [Option<BlockDeviceInfo>; 256] = [None; 256]; > + let mut pbs_err: *mut c_char = ptr::null_mut(); > + > + for device_id in 0..255 { > + if !vma_reader.contains_device(device_id) { > + continue; > + } > + > + let device_name = vma_reader.get_device_name(device_id)?; > + let device_size = vma_reader.get_device_size(device_id)?; > + > + println!( > + "DEV: dev_id={} size: {} devname: {}", > + device_id, device_size, device_name > + ); > + > + let device_name_cstr = CString::new(device_name)?; > + let pbs_device_id = proxmox_backup_register_image( > + pbs, > + device_name_cstr.as_ptr(), > + device_size, > + false, > + &mut pbs_err, > + ); > + > + if pbs_device_id < 0 { > + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; > + anyhow::bail!("proxmox_backup_register_image failed: {pbs_err_cstr:?}"); > + } Same as above (error handling). > + > + let block_device_info = BlockDeviceInfo { > + pbs_device_id: pbs_device_id as u8, > + device_size, > + }; > + > + block_device_infos[device_id as usize] = Some(block_device_info); > + } > + > + Ok(block_device_infos) > +} > + > +fn upload_block_devices(mut vma_reader: VmaReader, pbs: *mut ProxmoxBackupHandle) -> Result<()> { > + let block_device_infos = register_block_devices(&vma_reader, pbs)?; > + > + struct ImageChunk { > + sub_chunks: HashMap<u8, Option<Vec<u8>>>, > + mask: u64, > + non_zero_mask: u64, > + } > + > + let images_chunks: RefCell<HashMap<u8, HashMap<u64, ImageChunk>>> = > + RefCell::new(HashMap::new()); > + All of this, starting from here ... > + vma_reader.restore(|dev_id, offset, buffer| { > + let block_device_info = match block_device_infos[dev_id as usize] { > + Some(block_device_info) => block_device_info, > + None => anyhow::bail!("Reference to unknown device id {} in VMA file", dev_id), > + }; > + > + let pbs_device_id = block_device_info.pbs_device_id; > + let device_size = block_device_info.device_size; > + > + let mut images_chunks = images_chunks.borrow_mut(); > + let image_chunks = images_chunks.get_mut(&dev_id); > + let pbs_chunk_offset = > + PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE * (offset / PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE); > + let sub_chunk_index = > + ((offset % PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE) / VMA_CLUSTER_SIZE as u64) as u8; > + > + let pbs_chunk_size = PROXMOX_BACKUP_DEFAULT_CHUNK_SIZE.min(device_size - pbs_chunk_offset); > + > + let pbs_upload_chunk = |pbs_chunk_buffer: Option<&[u8]>| { > + println!( > + "Uploading dev_id: {} offset: {:#0X} - {:#0X}", > + dev_id, > + pbs_chunk_offset, > + pbs_chunk_offset + pbs_chunk_size, > + ); > + > + let mut pbs_err: *mut c_char = ptr::null_mut(); > + > + let write_data_result = proxmox_backup_write_data( > + pbs, > + pbs_device_id, > + match pbs_chunk_buffer { > + Some(pbs_chunk_buffer) => pbs_chunk_buffer.as_ptr(), > + None => ptr::null(), > + }, > + pbs_chunk_offset, > + pbs_chunk_size, > + &mut pbs_err, > + ); > + > + if write_data_result < 0 { > + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; > + anyhow::bail!("proxmox_backup_write_data failed: {pbs_err_cstr:?}"); > + } > + > + Ok(()) > + }; > + > + let insert_image_chunk = |image_chunks: &mut HashMap<u64, ImageChunk>, > + buffer: Option<Vec<u8>>| { > + let mut sub_chunks: HashMap<u8, Option<Vec<u8>>> = HashMap::new(); > + let mask = 1 << sub_chunk_index; > + let non_zero_mask = buffer.is_some() as u64; > + sub_chunks.insert(sub_chunk_index, buffer); > + > + let image_chunk = ImageChunk { > + sub_chunks, > + mask, > + non_zero_mask, > + }; > + > + image_chunks.insert(pbs_chunk_offset, image_chunk); > + }; > + > + match image_chunks { > + Some(image_chunks) => { > + let image_chunk = image_chunks.get_mut(&pbs_chunk_offset); > + > + match image_chunk { > + Some(image_chunk) => { > + image_chunk.mask |= 1 << sub_chunk_index; > + image_chunk.non_zero_mask |= (buffer.is_some() as u64) << sub_chunk_index; > + image_chunk.sub_chunks.insert(sub_chunk_index, buffer); > + > + let sub_chunk_count = > + ((pbs_chunk_size + 65535) / VMA_CLUSTER_SIZE as u64) as u8; > + let pbs_chunk_mask = 1_u64 > + .checked_shl(sub_chunk_count.into()) > + .unwrap_or(0) > + .wrapping_sub(1); > + > + if image_chunk.mask == pbs_chunk_mask { > + if image_chunk.non_zero_mask == 0 { > + pbs_upload_chunk(None)?; > + } else { > + let mut pbs_chunk_buffer = > + proxmox_io::boxed::zeroed(pbs_chunk_size as usize); > + > + for i in 0..sub_chunk_count { > + let sub_chunk = &image_chunk.sub_chunks[&i]; > + let start = i as usize * VMA_CLUSTER_SIZE; > + let end = > + (start + VMA_CLUSTER_SIZE).min(pbs_chunk_size as usize); > + > + match sub_chunk { > + Some(sub_chunk) => { > + pbs_chunk_buffer[start..end] > + .copy_from_slice(&sub_chunk[0..end - start]); > + } > + None => pbs_chunk_buffer[start..end].fill(0), > + } > + } > + > + pbs_upload_chunk(Some(&*pbs_chunk_buffer))?; > + } > + > + image_chunks.remove(&pbs_chunk_offset); > + } > + } > + None => { > + if pbs_chunk_size <= VMA_CLUSTER_SIZE as u64 { > + pbs_upload_chunk(buffer.as_deref())?; > + } else { > + insert_image_chunk(image_chunks, buffer); > + } > + } > + } > + } > + None => { > + if pbs_chunk_size <= VMA_CLUSTER_SIZE as u64 { > + pbs_upload_chunk(buffer.as_deref())?; > + } else { > + let mut image_chunks: HashMap<u64, ImageChunk> = HashMap::new(); > + insert_image_chunk(&mut image_chunks, buffer); > + images_chunks.insert(dev_id, image_chunks); > + } > + } > + } > + > + Ok(()) > + })?; ... and ending here should still be taken apart and put into a bunch of smaller functions. If it's hard to keep track of all the things going on here while you're disentangling this, encapsulate the state as struct. > + > + let mut pbs_err: *mut c_char = ptr::null_mut(); > + > + for block_device_info in block_device_infos.iter().take(255) { > + let block_device_info = match block_device_info { > + Some(block_device_info) => block_device_info, > + None => continue, > + }; > + > + let pbs_device_id = block_device_info.pbs_device_id; > + > + if proxmox_backup_close_image(pbs, pbs_device_id, &mut pbs_err) < 0 { > + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; > + anyhow::bail!("proxmox_backup_close_image failed: {pbs_err_cstr:?}"); > + } Same as above (error handling). > + } > + > + Ok(()) > +} > + > +pub fn backup_vma_to_pbs(args: BackupVmaToPbsArgs) -> Result<()> { > + let vma_file: Box<dyn BufRead> = match &args.vma_file_path { > + Some(vma_file_path) => match File::open(vma_file_path) { > + Err(why) => return Err(anyhow!("Couldn't open file: {}", why)), > + Ok(file) => Box::new(BufReader::new(file)), > + }, > + None => Box::new(BufReader::new(stdin())), IMO we should consider using a separate argument that makes the CLI read from stin, or e.g. if the file is named "-" (without quotes). Otherwise, weird things could happen if one forgets to add an argument: zstd -d --stdout vzdump-qemu-300-2024_03_25-10_21_10.vma.zst | vma-to-pbs --repository root@pam@my.pbs.tld:8007:default --vmid 300 Error: stream did not contain valid UTF-8 > + }; > + let vma_reader = VmaReader::new(vma_file)?; > + > + let pbs = create_pbs_backup_task(args)?; > + > + defer! { > + proxmox_backup_disconnect(pbs); > + } > + > + let mut pbs_err: *mut c_char = ptr::null_mut(); > + let connect_result = proxmox_backup_connect(pbs, &mut pbs_err); > + > + if connect_result < 0 { > + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; > + anyhow::bail!("proxmox_backup_connect failed: {pbs_err_cstr:?}"); > + } > + > + println!("Connected to Proxmox Backup Server"); > + > + let start_transfer_time = SystemTime::now(); > + > + upload_configs(&vma_reader, pbs)?; > + upload_block_devices(vma_reader, pbs)?; > + > + if proxmox_backup_finish(pbs, &mut pbs_err) < 0 { > + let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) }; > + anyhow::bail!("proxmox_backup_finish failed: {pbs_err_cstr:?}"); > + } Same as above (error handling). > + > + let elapsed_time = SystemTime::now().duration_since(start_transfer_time)?; > + let total_ms = elapsed_time.as_millis(); > + let ms = total_ms % 1000; > + let seconds = (total_ms / 1000) % 60; > + let minutes = total_ms / 1000000; > + println!("Backup finished within {minutes} minutes, {seconds} seconds and {ms} ms"); > + > + Ok(()) > +} [0]: https://doc.rust-lang.org/std/ffi/struct.CStr.html#method.from_ptr ^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs 08/10] add a fallback for the --fingerprint argument 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer ` (6 preceding siblings ...) 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 07/10] add support for streaming the VMA file via stdin Filip Schauer @ 2024-03-20 14:15 ` Filip Schauer 2024-03-25 12:35 ` Max Carrara 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 09/10] refactor error handling with anyhow Filip Schauer ` (3 subsequent siblings) 11 siblings, 1 reply; 23+ messages in thread From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw) To: pbs-devel Fallback to the PBS_FINGERPRINT environment variable if the --fingerprint argument is not specified. Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- Cargo.toml | 2 +- src/main.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9711690..f56e351 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" [dependencies] anyhow = "1.0" bincode = "1.3" -clap = { version = "4.0.32", features = ["cargo"] } +clap = { version = "4.0.32", features = ["cargo", "env"] } md5 = "0.7.0" scopeguard = "1.1.0" serde = "1.0" diff --git a/src/main.rs b/src/main.rs index e0e1076..149c1a6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -27,7 +27,7 @@ fn main() -> Result<()> { .long("fingerprint") .value_name("FINGERPRINT") .help("Proxmox Backup Server Fingerprint") - .required(true), + .env("PBS_FINGERPRINT"), ) .arg( Arg::new("keyfile") @@ -72,9 +72,10 @@ fn main() -> Result<()> { let pbs_repository = matches.get_one::<String>("repository").unwrap().to_string(); let vmid = matches.get_one::<String>("vmid").unwrap().to_string(); + let fingerprint = matches .get_one::<String>("fingerprint") - .unwrap() + .expect("Fingerprint not set. Use $PBS_FINGERPRINT or --fingerprint") .to_string(); let keyfile = matches.get_one::<String>("keyfile"); -- 2.39.2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs 08/10] add a fallback for the --fingerprint argument 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 08/10] add a fallback for the --fingerprint argument Filip Schauer @ 2024-03-25 12:35 ` Max Carrara 0 siblings, 0 replies; 23+ messages in thread From: Max Carrara @ 2024-03-25 12:35 UTC (permalink / raw) To: Proxmox Backup Server development discussion On Wed Mar 20, 2024 at 3:15 PM CET, Filip Schauer wrote: > Fallback to the PBS_FINGERPRINT environment variable if the > --fingerprint argument is not specified. > > Signed-off-by: Filip Schauer <f.schauer@proxmox.com> > --- > Cargo.toml | 2 +- > src/main.rs | 5 +++-- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/Cargo.toml b/Cargo.toml > index 9711690..f56e351 100644 > --- a/Cargo.toml > +++ b/Cargo.toml > @@ -7,7 +7,7 @@ edition = "2021" > [dependencies] > anyhow = "1.0" > bincode = "1.3" > -clap = { version = "4.0.32", features = ["cargo"] } > +clap = { version = "4.0.32", features = ["cargo", "env"] } > md5 = "0.7.0" > scopeguard = "1.1.0" > serde = "1.0" > diff --git a/src/main.rs b/src/main.rs > index e0e1076..149c1a6 100644 > --- a/src/main.rs > +++ b/src/main.rs > @@ -27,7 +27,7 @@ fn main() -> Result<()> { > .long("fingerprint") > .value_name("FINGERPRINT") > .help("Proxmox Backup Server Fingerprint") > - .required(true), > + .env("PBS_FINGERPRINT"), > ) > .arg( > Arg::new("keyfile") > @@ -72,9 +72,10 @@ fn main() -> Result<()> { > > let pbs_repository = matches.get_one::<String>("repository").unwrap().to_string(); > let vmid = matches.get_one::<String>("vmid").unwrap().to_string(); > + > let fingerprint = matches > .get_one::<String>("fingerprint") > - .unwrap() > + .expect("Fingerprint not set. Use $PBS_FINGERPRINT or --fingerprint") > .to_string(); Something I hadn't mentioned in my last review for this patch in particular (but for other parts in main.rs): Please avoid using `unwrap()` and `expect()` in cases like these - there's no need to `panic!` here. Specifically, if the user forgets to provide the fingerprint, the error message looks like this: thread 'main' panicked at 'Fingerprint not set. Use $PBS_FINGERPRINT or --fingerprint', src/main.rs:78:10 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ... which ought not happen - you should only really `panic!` when an invariant isn't being upheld. Instead, use `anyhow::Context` [0] here with a `?`. [0]: https://docs.rs/anyhow/1.0.79/anyhow/trait.Context.html > > let keyfile = matches.get_one::<String>("keyfile"); ^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs 09/10] refactor error handling with anyhow 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer ` (7 preceding siblings ...) 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 08/10] add a fallback for the --fingerprint argument Filip Schauer @ 2024-03-20 14:15 ` Filip Schauer 2024-03-25 12:35 ` Max Carrara 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 10/10] Makefile: remove reference to unused submodule Filip Schauer ` (2 subsequent siblings) 11 siblings, 1 reply; 23+ messages in thread From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw) To: pbs-devel Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- src/main.rs | 6 +++--- src/vma.rs | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main.rs b/src/main.rs index 149c1a6..e44257f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{Context, Result}; use clap::{command, Arg, ArgAction}; use proxmox_sys::linux::tty; @@ -89,7 +89,7 @@ fn main() -> Result<()> { let pbs_password = match password_file { Some(password_file) => { - std::fs::read_to_string(password_file).expect("Could not read password file") + std::fs::read_to_string(password_file).context("Could not read password file")? } None => String::from_utf8(tty::read_password("Password: ")?)?, }; @@ -100,7 +100,7 @@ fn main() -> Result<()> { Some(match key_password_file { Some(key_password_file) => std::fs::read_to_string(key_password_file) - .expect("Could not read key password file"), + .context("Could not read key password file")?, None => String::from_utf8(tty::read_password("Key Password: ")?)?, }) } diff --git a/src/vma.rs b/src/vma.rs index d369b36..fca6586 100644 --- a/src/vma.rs +++ b/src/vma.rs @@ -3,7 +3,7 @@ use std::io::Read; use std::mem::size_of; use std::str; -use anyhow::{anyhow, Result}; +use anyhow::{Context, Result}; use bincode::Options; use serde::{Deserialize, Serialize}; use serde_big_array::BigArray; @@ -135,11 +135,11 @@ impl VmaReader { let mut vma_header: VmaHeader = bincode_options.deserialize(&buffer)?; if vma_header.magic != VMA_HEADER_MAGIC { - return Err(anyhow!("Invalid magic number")); + anyhow::bail!("Invalid magic number"); } if vma_header.version != 1 { - return Err(anyhow!("Invalid VMA version {}", vma_header.version)); + anyhow::bail!("Invalid VMA version {}", vma_header.version); } buffer.resize(vma_header.header_size as usize, 0); @@ -150,7 +150,7 @@ impl VmaReader { let computed_md5sum: [u8; 16] = md5::compute(&buffer).into(); if vma_header.md5sum != computed_md5sum { - return Err(anyhow!("Wrong VMA header checksum")); + anyhow::bail!("Wrong VMA header checksum"); } let blob_buffer = &buffer[VMA_HEADER_SIZE_NO_BLOB_BUFFER..vma_header.header_size as usize]; @@ -233,7 +233,7 @@ impl VmaReader { let vma_extent_header: VmaExtentHeader = bincode_options.deserialize(&buffer)?; if vma_extent_header.magic != VMA_EXTENT_HEADER_MAGIC { - return Err(anyhow!("Invalid magic number")); + anyhow::bail!("Invalid magic number"); } // Fill the MD5 sum field with zeros to compute the MD5 sum @@ -241,7 +241,7 @@ impl VmaReader { let computed_md5sum: [u8; 16] = md5::compute(&buffer).into(); if vma_extent_header.md5sum != computed_md5sum { - return Err(anyhow!("Wrong VMA extent header checksum")); + anyhow::bail!("Wrong VMA extent header checksum"); } Ok(vma_extent_header) @@ -303,7 +303,7 @@ impl VmaReader { if ioerr.kind() == std::io::ErrorKind::UnexpectedEof { break; // Break out of the loop since the end of the file was reached. } else { - return Err(anyhow!("Failed to read VMA file: {}", ioerr)); + return Err(anyhow::format_err!(e)).context("Failed to read VMA file"); } } _ => { -- 2.39.2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH vma-to-pbs 09/10] refactor error handling with anyhow 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 09/10] refactor error handling with anyhow Filip Schauer @ 2024-03-25 12:35 ` Max Carrara 0 siblings, 0 replies; 23+ messages in thread From: Max Carrara @ 2024-03-25 12:35 UTC (permalink / raw) To: Proxmox Backup Server development discussion On Wed Mar 20, 2024 at 3:15 PM CET, Filip Schauer wrote: > Signed-off-by: Filip Schauer <f.schauer@proxmox.com> > --- > src/main.rs | 6 +++--- > src/vma.rs | 14 +++++++------- > 2 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/src/main.rs b/src/main.rs > index 149c1a6..e44257f 100644 > --- a/src/main.rs > +++ b/src/main.rs > @@ -1,4 +1,4 @@ > -use anyhow::Result; > +use anyhow::{Context, Result}; > use clap::{command, Arg, ArgAction}; > use proxmox_sys::linux::tty; > > @@ -89,7 +89,7 @@ fn main() -> Result<()> { > > let pbs_password = match password_file { > Some(password_file) => { > - std::fs::read_to_string(password_file).expect("Could not read password file") > + std::fs::read_to_string(password_file).context("Could not read password file")? > } > None => String::from_utf8(tty::read_password("Password: ")?)?, > }; > @@ -100,7 +100,7 @@ fn main() -> Result<()> { > > Some(match key_password_file { > Some(key_password_file) => std::fs::read_to_string(key_password_file) > - .expect("Could not read key password file"), > + .context("Could not read key password file")?, > None => String::from_utf8(tty::read_password("Key Password: ")?)?, > }) > } > diff --git a/src/vma.rs b/src/vma.rs > index d369b36..fca6586 100644 > --- a/src/vma.rs > +++ b/src/vma.rs > @@ -3,7 +3,7 @@ use std::io::Read; > use std::mem::size_of; > use std::str; > > -use anyhow::{anyhow, Result}; > +use anyhow::{Context, Result}; > use bincode::Options; > use serde::{Deserialize, Serialize}; > use serde_big_array::BigArray; > @@ -135,11 +135,11 @@ impl VmaReader { > let mut vma_header: VmaHeader = bincode_options.deserialize(&buffer)?; > > if vma_header.magic != VMA_HEADER_MAGIC { > - return Err(anyhow!("Invalid magic number")); > + anyhow::bail!("Invalid magic number"); You could've just imported `bail` above. ;) > } > > if vma_header.version != 1 { > - return Err(anyhow!("Invalid VMA version {}", vma_header.version)); > + anyhow::bail!("Invalid VMA version {}", vma_header.version); > } > > buffer.resize(vma_header.header_size as usize, 0); > @@ -150,7 +150,7 @@ impl VmaReader { > let computed_md5sum: [u8; 16] = md5::compute(&buffer).into(); > > if vma_header.md5sum != computed_md5sum { > - return Err(anyhow!("Wrong VMA header checksum")); > + anyhow::bail!("Wrong VMA header checksum"); > } > > let blob_buffer = &buffer[VMA_HEADER_SIZE_NO_BLOB_BUFFER..vma_header.header_size as usize]; > @@ -233,7 +233,7 @@ impl VmaReader { > let vma_extent_header: VmaExtentHeader = bincode_options.deserialize(&buffer)?; > > if vma_extent_header.magic != VMA_EXTENT_HEADER_MAGIC { > - return Err(anyhow!("Invalid magic number")); > + anyhow::bail!("Invalid magic number"); > } > > // Fill the MD5 sum field with zeros to compute the MD5 sum > @@ -241,7 +241,7 @@ impl VmaReader { > let computed_md5sum: [u8; 16] = md5::compute(&buffer).into(); > > if vma_extent_header.md5sum != computed_md5sum { > - return Err(anyhow!("Wrong VMA extent header checksum")); > + anyhow::bail!("Wrong VMA extent header checksum"); > } > > Ok(vma_extent_header) > @@ -303,7 +303,7 @@ impl VmaReader { > if ioerr.kind() == std::io::ErrorKind::UnexpectedEof { > break; // Break out of the loop since the end of the file was reached. > } else { > - return Err(anyhow!("Failed to read VMA file: {}", ioerr)); > + return Err(anyhow::format_err!(e)).context("Failed to read VMA file"); > } > } > _ => { ^ permalink raw reply [flat|nested] 23+ messages in thread
* [pbs-devel] [PATCH vma-to-pbs 10/10] Makefile: remove reference to unused submodule 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer ` (8 preceding siblings ...) 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 09/10] refactor error handling with anyhow Filip Schauer @ 2024-03-20 14:15 ` Filip Schauer 2024-03-25 12:38 ` [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Max Carrara 2024-04-03 9:56 ` Filip Schauer 11 siblings, 0 replies; 23+ messages in thread From: Filip Schauer @ 2024-03-20 14:15 UTC (permalink / raw) To: pbs-devel Signed-off-by: Filip Schauer <f.schauer@proxmox.com> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index a0c841d..75b1bad 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ $(BUILDDIR): submodule mv $@.tmp $@ submodule: - [ -e submodules/proxmox-backup-qemu/Cargo.toml ] || [ -e submodules/proxmox/proxmox-sys/Cargo.toml ] || git submodule update --init --recursive + [ -e submodules/proxmox-backup-qemu/Cargo.toml ] || git submodule update --init --recursive dsc: rm -rf $(BUILDDIR) $(DSC) -- 2.39.2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer ` (9 preceding siblings ...) 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 10/10] Makefile: remove reference to unused submodule Filip Schauer @ 2024-03-25 12:38 ` Max Carrara 2024-03-25 12:52 ` Wolfgang Bumiller 2024-04-03 9:56 ` Filip Schauer 11 siblings, 1 reply; 23+ messages in thread From: Max Carrara @ 2024-03-25 12:38 UTC (permalink / raw) To: Proxmox Backup Server development discussion; +Cc: w.bumiller On Wed Mar 20, 2024 at 3:15 PM CET, Filip Schauer wrote: > Implement a tool to import VMA files into a Proxmox Backup Server > > Example usage: > > zstd -d --stdout vzdump.vma.zst | vma-to-pbs \ > --repository <auth_id@host:port:datastore> \ > --vmid 123 \ > --password_file pbs_password > > Commit 07/10 requires > https://lists.proxmox.com/pipermail/pve-devel/2024-March/062182.html > to be applied first. > > Changes since v4: > * Bump proxmox-backup-qemu > * Remove unnecessary "extern crate" declarations > * Refactor error handling with anyhow > * vma.rs: Improve code readability by adding constants and using more > descriptive variable/type names. > * vma.rs: Move duplicate code into read_string function > * Print elapsed time in minutes, seconds and ms > * Refactor block device id and size retrieval logic > * vma: Document break statement when reaching end of file > * Use selected imports instead of glob imports > * Split up vma2pbs logic into seperate functions > * Makefile: remove reference to unused submodule To preface all of my following comments: You really did incorporate a lot of my feedback, hats off to you! That sure must've been a lot of work and it certainly does not go unnoticed nor unappreciated. I do have some more comments, but the vast majority of what I had mentioned in v4 has been resolved. See more below and in my replies to individual patches. First and foremost, the patches 1 - 3 can just be dropped - as they're already applied, there's no need to keep including them in this series. (Nevermind, ignore this - you just mentioned it yourself as I was writing this.) Furthermore, there are still a handful of things that could use some untangling and some minor changes otherwise, but it's nowhere near as much as I had mentioned last time. For more details see my comments inline on individual patches. That being said, I was able to successfully build the whole thing and tried to test it against some locally stored backups - unfortunately without success. Here's the (adapted) output: zstd -d --stdout vzdump-qemu-300-2024_03_25-10_21_10.vma.zst | vma-to-pbs --repository root@pam@my.pbs.tld:8007:default --vmid 300 --password_file ~/pbs-pw.txt PBS repository: root@pam@my.pbs.tld:8007:default PBS fingerprint: [...] compress: false encrypt: false backup time: 1711360054 Error: proxmox_backup_connect failed: "command error: parameter verification failed - \'password\': value does not match the regex pattern" (Fingerprint was set via env var.) I'm 100% certain that my password is correct - it seems to be related to proxmox-schema. I haven't really looked into that otherwise yet; maybe @Wolfgang could weigh in here? > > Changes since v3: > * Add the ability to provide credentials via files > * Add support for streaming the VMA file via stdin > * Add a fallback for the --fingerprint argument > > Changes since v2: > * Use the deb packages from the proxmox-io and proxmox-sys dependencies > instead of the proxmox submodule > * Remove the proxmox submodule > * Update the proxmox-backup-qemu submodule to make it buildable with > the newest librust dependencies > > Changes since v1: > * Remove unused crates and uses > * Format the code > * Use anyhow for error handling > * Use clap for parsing arguments instead of getopts > * Fix blocks being reindexed on every read > * Make sure ProxmoxBackupHandle is dropped properly on error > * Move image_chunk_buffer from stack to heap > * Move the block_index in VmaReader to the heap completely > * Initialize vectors with `Vec::with_capacity` and `resize` instead of > the `vec!` macro, to potentially improve performance on debug builds. > * Add comments to code filling the MD5 sum field with zeros > * Change device_id arguments to usize > * Handle devices that have a size that is not aligned to 4096 properly > in read_device_contents, when the caller provides a buffer that would > exceed the device size. > * Avoid unnecessary loop iterations in read_device_contents when the > buffer size is not aligned to 65536 > > Filip Schauer (8): > Initial commit > Add the ability to provide credentials via files > bump proxmox-backup-qemu > remove unnecessary "extern crate" declarations > add support for streaming the VMA file via stdin > add a fallback for the --fingerprint argument > refactor error handling with anyhow > Makefile: remove reference to unused submodule > > Wolfgang Bumiller (2): > cargo fmt > bump proxmox-backup-qemu ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool 2024-03-25 12:38 ` [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Max Carrara @ 2024-03-25 12:52 ` Wolfgang Bumiller 0 siblings, 0 replies; 23+ messages in thread From: Wolfgang Bumiller @ 2024-03-25 12:52 UTC (permalink / raw) To: Max Carrara; +Cc: Proxmox Backup Server development discussion On Mon, Mar 25, 2024 at 01:38:12PM +0100, Max Carrara wrote: > On Wed Mar 20, 2024 at 3:15 PM CET, Filip Schauer wrote: > > Implement a tool to import VMA files into a Proxmox Backup Server > > > > Example usage: > > > > zstd -d --stdout vzdump.vma.zst | vma-to-pbs \ > > --repository <auth_id@host:port:datastore> \ > > --vmid 123 \ > > --password_file pbs_password > > > > Commit 07/10 requires > > https://lists.proxmox.com/pipermail/pve-devel/2024-March/062182.html > > to be applied first. > > > > Changes since v4: > > * Bump proxmox-backup-qemu > > * Remove unnecessary "extern crate" declarations > > * Refactor error handling with anyhow > > * vma.rs: Improve code readability by adding constants and using more > > descriptive variable/type names. > > * vma.rs: Move duplicate code into read_string function > > * Print elapsed time in minutes, seconds and ms > > * Refactor block device id and size retrieval logic > > * vma: Document break statement when reaching end of file > > * Use selected imports instead of glob imports > > * Split up vma2pbs logic into seperate functions > > * Makefile: remove reference to unused submodule > > To preface all of my following comments: You really did incorporate a > lot of my feedback, hats off to you! That sure must've been a lot of > work and it certainly does not go unnoticed nor unappreciated. > > I do have some more comments, but the vast majority of what I had > mentioned in v4 has been resolved. See more below and in my replies to > individual patches. > > First and foremost, the patches 1 - 3 can just be dropped - as they're > already applied, there's no need to keep including them in this series. > (Nevermind, ignore this - you just mentioned it yourself as I was > writing this.) > > Furthermore, there are still a handful of things that could use some > untangling and some minor changes otherwise, but it's nowhere near as > much as I had mentioned last time. For more details see my comments > inline on individual patches. > > That being said, I was able to successfully build the whole thing and > tried to test it against some locally stored backups - unfortunately > without success. Here's the (adapted) output: > > > zstd -d --stdout vzdump-qemu-300-2024_03_25-10_21_10.vma.zst | vma-to-pbs --repository root@pam@my.pbs.tld:8007:default --vmid 300 --password_file ~/pbs-pw.txt > PBS repository: root@pam@my.pbs.tld:8007:default > PBS fingerprint: [...] > compress: false > encrypt: false > backup time: 1711360054 > Error: proxmox_backup_connect failed: "command error: parameter verification failed - \'password\': value does not match the regex pattern" > > (Fingerprint was set via env var.) > > I'm 100% certain that my password is correct - it seems to be related to > proxmox-schema. I haven't really looked into that otherwise yet; maybe > @Wolfgang could weigh in here? I'm assuming it's what I mentioned in your listvms.py list. There's a `std::fs::read_to_string()` for the password, but it should cut off the trailing newline ;-) ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer ` (10 preceding siblings ...) 2024-03-25 12:38 ` [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Max Carrara @ 2024-04-03 9:56 ` Filip Schauer 11 siblings, 0 replies; 23+ messages in thread From: Filip Schauer @ 2024-04-03 9:56 UTC (permalink / raw) To: pbs-devel Patch v6 is available: https://lists.proxmox.com/pipermail/pbs-devel/2024-April/008291.html On 20/03/2024 15:15, Filip Schauer wrote: > Implement a tool to import VMA files into a Proxmox Backup Server > > Example usage: > > zstd -d --stdout vzdump.vma.zst | vma-to-pbs \ > --repository <auth_id@host:port:datastore> \ > --vmid 123 \ > --password_file pbs_password > > Commit 07/10 requires > https://lists.proxmox.com/pipermail/pve-devel/2024-March/062182.html > to be applied first. > > Changes since v4: > * Bump proxmox-backup-qemu > * Remove unnecessary "extern crate" declarations > * Refactor error handling with anyhow > * vma.rs: Improve code readability by adding constants and using more > descriptive variable/type names. > * vma.rs: Move duplicate code into read_string function > * Print elapsed time in minutes, seconds and ms > * Refactor block device id and size retrieval logic > * vma: Document break statement when reaching end of file > * Use selected imports instead of glob imports > * Split up vma2pbs logic into seperate functions > * Makefile: remove reference to unused submodule > > Changes since v3: > * Add the ability to provide credentials via files > * Add support for streaming the VMA file via stdin > * Add a fallback for the --fingerprint argument > > Changes since v2: > * Use the deb packages from the proxmox-io and proxmox-sys dependencies > instead of the proxmox submodule > * Remove the proxmox submodule > * Update the proxmox-backup-qemu submodule to make it buildable with > the newest librust dependencies > > Changes since v1: > * Remove unused crates and uses > * Format the code > * Use anyhow for error handling > * Use clap for parsing arguments instead of getopts > * Fix blocks being reindexed on every read > * Make sure ProxmoxBackupHandle is dropped properly on error > * Move image_chunk_buffer from stack to heap > * Move the block_index in VmaReader to the heap completely > * Initialize vectors with `Vec::with_capacity` and `resize` instead of > the `vec!` macro, to potentially improve performance on debug builds. > * Add comments to code filling the MD5 sum field with zeros > * Change device_id arguments to usize > * Handle devices that have a size that is not aligned to 4096 properly > in read_device_contents, when the caller provides a buffer that would > exceed the device size. > * Avoid unnecessary loop iterations in read_device_contents when the > buffer size is not aligned to 65536 > > Filip Schauer (8): > Initial commit > Add the ability to provide credentials via files > bump proxmox-backup-qemu > remove unnecessary "extern crate" declarations > add support for streaming the VMA file via stdin > add a fallback for the --fingerprint argument > refactor error handling with anyhow > Makefile: remove reference to unused submodule > > Wolfgang Bumiller (2): > cargo fmt > bump proxmox-backup-qemu > ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2024-04-03 9:56 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-20 14:15 [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 01/10] Initial commit Filip Schauer 2024-03-25 9:42 ` Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 02/10] cargo fmt Filip Schauer 2024-03-25 9:43 ` Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 03/10] bump proxmox-backup-qemu Filip Schauer 2024-03-25 9:43 ` Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 04/10] Add the ability to provide credentials via files Filip Schauer 2024-03-25 12:33 ` Max Carrara 2024-03-25 15:26 ` Thomas Lamprecht 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 05/10] bump proxmox-backup-qemu Filip Schauer 2024-03-25 12:33 ` Max Carrara 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 06/10] remove unnecessary "extern crate" declarations Filip Schauer 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 07/10] add support for streaming the VMA file via stdin Filip Schauer 2024-03-25 12:34 ` Max Carrara 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 08/10] add a fallback for the --fingerprint argument Filip Schauer 2024-03-25 12:35 ` Max Carrara 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 09/10] refactor error handling with anyhow Filip Schauer 2024-03-25 12:35 ` Max Carrara 2024-03-20 14:15 ` [pbs-devel] [PATCH vma-to-pbs 10/10] Makefile: remove reference to unused submodule Filip Schauer 2024-03-25 12:38 ` [pbs-devel] [PATCH v5 vma-to-pbs 00/10] Implement vma-to-pbs tool Max Carrara 2024-03-25 12:52 ` Wolfgang Bumiller 2024-04-03 9:56 ` Filip Schauer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox