[rust PATCH 0/3] Implement virStoragePoolListVolumes and virStoragePoolListAllVolumes

From: Mathias Pius <mathias@pius.io> This patch implements virStoragePoolListAllVolumes and virStoragePoolListVolumes for the StoragePool object. I'm not too familiar with the libvirt codebase, so I've used similar functions from connect.rs and domain.rs for so the implementations should (hopefully) be as correct as those. I've taken the liberty of using Vec::with_capacity when allocating vectors to store the result of these operations in, to prevent reallocations while converting the object types from the internal type to StorageVol and Strings. Feedback is very welcome! Mathias Pius (3): Implement virStoragePoolListAllVolumes and virStoragePoolListVolumes for StoragePool cargo fmt Newline at end of file src/storage_pool.rs | 55 ++++++++++++++++++++++++++++++++++++++++++- tests/storage_pool.rs | 34 ++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) -- 2.25.0

From: Mathias Pius <mathias@pius.io> --- src/storage_pool.rs | 54 ++++++++++++++++++++++++++++++++++++++++++- tests/storage_pool.rs | 34 +++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/storage_pool.rs b/src/storage_pool.rs index 96258f0..02395bf 100644 --- a/src/storage_pool.rs +++ b/src/storage_pool.rs @@ -18,7 +18,7 @@ extern crate libc; -use std::str; +use std::{mem, ptr, str}; use connect::sys::virConnectPtr; use storage_vol::sys::virStorageVolPtr; @@ -78,6 +78,16 @@ extern "C" { fn virStoragePoolFree(ptr: sys::virStoragePoolPtr) -> libc::c_int; fn virStoragePoolIsActive(ptr: sys::virStoragePoolPtr) -> libc::c_int; fn virStoragePoolIsPersistent(ptr: sys::virStoragePoolPtr) -> libc::c_int; + fn virStoragePoolListAllVolumes( + ptr: sys::virStoragePoolPtr, + vols: *mut *mut virStorageVolPtr, + flags: libc::c_uint, + ) -> libc::c_int; + fn virStoragePoolListVolumes( + ptr: sys::virStoragePoolPtr, + names: *mut *mut libc::c_char, + maxnames: libc::c_int, + ) -> libc::c_int; fn virStoragePoolGetName(ptr: sys::virStoragePoolPtr) -> *const libc::c_char; fn virStoragePoolGetXMLDesc( ptr: sys::virStoragePoolPtr, @@ -119,6 +129,8 @@ pub const VIR_STORAGE_POOL_RUNNING: StoragePoolState = 2; pub const VIR_STORAGE_POOL_DEGRADED: StoragePoolState = 3; pub const VIR_STORAGE_POOL_INACCESSIBLE: StoragePoolState = 4; +pub type StoragePoolListAllVolumesFlags = self::libc::c_uint; + #[derive(Clone, Debug)] pub struct StoragePoolInfo { /// A `StoragePoolState` flags @@ -373,6 +385,46 @@ impl StoragePool { } } + pub fn list_all_volumes( + &self, + flags: StoragePoolListAllVolumesFlags + ) -> Result<Vec<StorageVol>, Error> { + unsafe { + let mut volumes: *mut virStorageVolPtr = ptr::null_mut(); + + let size = virStoragePoolListAllVolumes(self.as_ptr(), &mut volumes, flags as libc::c_uint); + if size == -1 { + return Err(Error::new()); + } + + mem::forget(volumes); + + let mut array: Vec<StorageVol> = Vec::with_capacity(size as usize); + for x in 0..size as isize { + array.push(StorageVol::new(*volumes.offset(x))); + } + libc::free(volumes as *mut libc::c_void); + + return Ok(array) + } + } + + pub fn list_volumes(&self) -> Result<Vec<String>, Error> { + unsafe { + let mut names: [*mut libc::c_char; 1024] = [ptr::null_mut(); 1024]; + let size = virStoragePoolListVolumes(self.as_ptr(), names.as_mut_ptr(), 1024); + if size == -1 { + return Err(Error::new()); + } + + let mut array: Vec<String> = Vec::with_capacity(size as usize); + for x in 0..size as usize { + array.push(c_chars_to_string!(names[x])); + } + return Ok(array); + } + } + pub fn refresh(&self, flags: u32) -> Result<u32, Error> { unsafe { let ret = virStoragePoolRefresh(self.as_ptr(), flags as libc::c_uint); diff --git a/tests/storage_pool.rs b/tests/storage_pool.rs index 4bfa71d..303867b 100644 --- a/tests/storage_pool.rs +++ b/tests/storage_pool.rs @@ -58,3 +58,37 @@ fn test_lookup_storage_pool_by_name() { } common::close(c); } + +#[test] +fn test_list_volumes() { + match Connect::open("test:///default") { + Ok(mut conn) => { + let sp = conn.list_storage_pools().unwrap_or(vec![]); + match StoragePool::lookup_by_name(&conn, &sp[0]) { + Ok(storage_pool) => { + storage_pool.list_volumes().unwrap(); + } + Err(e) => panic!("failed with code {}, message: {}", e.code, e.message), + } + assert_eq!(0, conn.close().unwrap_or(-1)); + } + Err(e) => panic!("failed with code {}, message: {}", e.code, e.message), + } +} + +#[test] +fn test_list_all_volumes() { + match Connect::open("test:///default") { + Ok(mut conn) => { + let sp = conn.list_storage_pools().unwrap_or(vec![]); + match StoragePool::lookup_by_name(&conn, &sp[0]) { + Ok(storage_pool) => { + storage_pool.list_all_volumes(0).unwrap(); + } + Err(e) => panic!("failed with code {}, message: {}", e.code, e.message), + } + assert_eq!(0, conn.close().unwrap_or(-1)); + } + Err(e) => panic!("failed with code {}, message: {}", e.code, e.message), + } +} \ No newline at end of file -- 2.25.0

On Mon, Feb 10, 2020 at 01:38:04PM +0000, mathias@pius.io wrote:
From: Mathias Pius <mathias@pius.io>
--- src/storage_pool.rs | 54 ++++++++++++++++++++++++++++++++++++++++++- tests/storage_pool.rs | 34 +++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-)
Sounds good thank you Mathias. Two requests - Can you add the "Signed-off-by" tag I can't accept your patch without it [0]. - Can you merge patch 2/3 and 3/3 with 1/3, each individual patch should pass CI. Thanks, s. [0] https://libvirt.org/hacking.html#patches
diff --git a/src/storage_pool.rs b/src/storage_pool.rs index 96258f0..02395bf 100644 --- a/src/storage_pool.rs +++ b/src/storage_pool.rs @@ -18,7 +18,7 @@
extern crate libc;
-use std::str; +use std::{mem, ptr, str};
use connect::sys::virConnectPtr; use storage_vol::sys::virStorageVolPtr; @@ -78,6 +78,16 @@ extern "C" { fn virStoragePoolFree(ptr: sys::virStoragePoolPtr) -> libc::c_int; fn virStoragePoolIsActive(ptr: sys::virStoragePoolPtr) -> libc::c_int; fn virStoragePoolIsPersistent(ptr: sys::virStoragePoolPtr) -> libc::c_int; + fn virStoragePoolListAllVolumes( + ptr: sys::virStoragePoolPtr, + vols: *mut *mut virStorageVolPtr, + flags: libc::c_uint, + ) -> libc::c_int; + fn virStoragePoolListVolumes( + ptr: sys::virStoragePoolPtr, + names: *mut *mut libc::c_char, + maxnames: libc::c_int, + ) -> libc::c_int; fn virStoragePoolGetName(ptr: sys::virStoragePoolPtr) -> *const libc::c_char; fn virStoragePoolGetXMLDesc( ptr: sys::virStoragePoolPtr, @@ -119,6 +129,8 @@ pub const VIR_STORAGE_POOL_RUNNING: StoragePoolState = 2; pub const VIR_STORAGE_POOL_DEGRADED: StoragePoolState = 3; pub const VIR_STORAGE_POOL_INACCESSIBLE: StoragePoolState = 4;
+pub type StoragePoolListAllVolumesFlags = self::libc::c_uint; + #[derive(Clone, Debug)] pub struct StoragePoolInfo { /// A `StoragePoolState` flags @@ -373,6 +385,46 @@ impl StoragePool { } }
+ pub fn list_all_volumes( + &self, + flags: StoragePoolListAllVolumesFlags + ) -> Result<Vec<StorageVol>, Error> { + unsafe { + let mut volumes: *mut virStorageVolPtr = ptr::null_mut(); + + let size = virStoragePoolListAllVolumes(self.as_ptr(), &mut volumes, flags as libc::c_uint); + if size == -1 { + return Err(Error::new()); + } + + mem::forget(volumes); + + let mut array: Vec<StorageVol> = Vec::with_capacity(size as usize); + for x in 0..size as isize { + array.push(StorageVol::new(*volumes.offset(x))); + } + libc::free(volumes as *mut libc::c_void); + + return Ok(array) + } + } + + pub fn list_volumes(&self) -> Result<Vec<String>, Error> { + unsafe { + let mut names: [*mut libc::c_char; 1024] = [ptr::null_mut(); 1024]; + let size = virStoragePoolListVolumes(self.as_ptr(), names.as_mut_ptr(), 1024); + if size == -1 { + return Err(Error::new()); + } + + let mut array: Vec<String> = Vec::with_capacity(size as usize); + for x in 0..size as usize { + array.push(c_chars_to_string!(names[x])); + } + return Ok(array); + } + } + pub fn refresh(&self, flags: u32) -> Result<u32, Error> { unsafe { let ret = virStoragePoolRefresh(self.as_ptr(), flags as libc::c_uint); diff --git a/tests/storage_pool.rs b/tests/storage_pool.rs index 4bfa71d..303867b 100644 --- a/tests/storage_pool.rs +++ b/tests/storage_pool.rs @@ -58,3 +58,37 @@ fn test_lookup_storage_pool_by_name() { } common::close(c); } + +#[test] +fn test_list_volumes() { + match Connect::open("test:///default") { + Ok(mut conn) => { + let sp = conn.list_storage_pools().unwrap_or(vec![]); + match StoragePool::lookup_by_name(&conn, &sp[0]) { + Ok(storage_pool) => { + storage_pool.list_volumes().unwrap(); + } + Err(e) => panic!("failed with code {}, message: {}", e.code, e.message), + } + assert_eq!(0, conn.close().unwrap_or(-1)); + } + Err(e) => panic!("failed with code {}, message: {}", e.code, e.message), + } +} + +#[test] +fn test_list_all_volumes() { + match Connect::open("test:///default") { + Ok(mut conn) => { + let sp = conn.list_storage_pools().unwrap_or(vec![]); + match StoragePool::lookup_by_name(&conn, &sp[0]) { + Ok(storage_pool) => { + storage_pool.list_all_volumes(0).unwrap(); + } + Err(e) => panic!("failed with code {}, message: {}", e.code, e.message), + } + assert_eq!(0, conn.close().unwrap_or(-1)); + } + Err(e) => panic!("failed with code {}, message: {}", e.code, e.message), + } +} \ No newline at end of file -- 2.25.0

From: Mathias Pius <mathias@pius.io> --- src/storage_pool.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/storage_pool.rs b/src/storage_pool.rs index 02395bf..571fabd 100644 --- a/src/storage_pool.rs +++ b/src/storage_pool.rs @@ -387,12 +387,13 @@ impl StoragePool { pub fn list_all_volumes( &self, - flags: StoragePoolListAllVolumesFlags + flags: StoragePoolListAllVolumesFlags, ) -> Result<Vec<StorageVol>, Error> { unsafe { let mut volumes: *mut virStorageVolPtr = ptr::null_mut(); - let size = virStoragePoolListAllVolumes(self.as_ptr(), &mut volumes, flags as libc::c_uint); + let size = + virStoragePoolListAllVolumes(self.as_ptr(), &mut volumes, flags as libc::c_uint); if size == -1 { return Err(Error::new()); } @@ -405,7 +406,7 @@ impl StoragePool { } libc::free(volumes as *mut libc::c_void); - return Ok(array) + Ok(array) } } -- 2.25.0

From: Mathias Pius <mathias@pius.io> --- tests/storage_pool.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/storage_pool.rs b/tests/storage_pool.rs index 303867b..21c1139 100644 --- a/tests/storage_pool.rs +++ b/tests/storage_pool.rs @@ -91,4 +91,4 @@ fn test_list_all_volumes() { } Err(e) => panic!("failed with code {}, message: {}", e.code, e.message), } -} \ No newline at end of file +} -- 2.25.0
participants (2)
-
mathias@pius.io
-
Sahid Orentino Ferdjaoui