* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.Service provided by Proxmox Server Solutions GmbH | Privacy | Legal