[libvirt] [rust PATCH] Add list_all_volumes method for storage_pool::StoragePool
From: Sage Imel <sage@sagenite.net> Always returns the full list of volumes, can't just ask it how many volumes are in the pool Signed-off-by: Sage Imel <sage@cat.pdx.edu> --- src/storage_pool.rs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/storage_pool.rs b/src/storage_pool.rs index 38676c2..e8ed21c 100644 --- a/src/storage_pool.rs +++ b/src/storage_pool.rs @@ -18,7 +18,7 @@ extern crate libc; -use std::str; +use std::{str, ptr}; use connect::sys::virConnectPtr; use storage_vol::sys::virStorageVolPtr; @@ -57,6 +57,10 @@ extern "C" { xml: *const libc::c_char, flags: libc::c_uint) -> sys::virStoragePoolPtr; + fn virStoragePoolListAllVolumes(ptr: sys::virStoragePoolPtr, + vols: *mut *mut virStorageVolPtr, + flags:libc::c_uint) + -> libc::c_int; fn virStoragePoolLookupByID(c: virConnectPtr, id: libc::c_int) -> sys::virStoragePoolPtr; fn virStoragePoolLookupByName(c: virConnectPtr, id: *const libc::c_char) @@ -103,6 +107,8 @@ pub const STORAGE_POOL_CREATE_WITH_BUILD: StoragePoolCreateFlags = 1 << 0; pub const STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE: StoragePoolCreateFlags = 1 << 1; pub const STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE: StoragePoolCreateFlags = 1 << 2; +pub type VirStoragePoolListAllVolumesFlags = self::libc::c_uint; + pub type StoragePoolState = self::libc::c_uint; pub const VIR_STORAGE_POOL_INACTIVE: StoragePoolState = 0; pub const VIR_STORAGE_POOL_BUILDING: StoragePoolState = 1; @@ -201,6 +207,27 @@ impl StoragePool { } } + pub fn list_all_volumes(&self, + flags: VirStoragePoolListAllVolumesFlags) + -> 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()); + } + + let mut array: Vec<StorageVol> = Vec::new(); + 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 lookup_by_id(conn: &Connect, id: u32) -> Result<StoragePool, Error> { unsafe { let ptr = virStoragePoolLookupByID(conn.as_ptr(), id as libc::c_int); -- 2.17.1
This was copied and barely modified from the implementation of connect::Connect::list_all_storage_pools -- Sage Imel <sage@cat.pdx.edu> Maseeh College of Engineering and Computer Science Computer Action Team <www.cat.pdx.edu> On Thu, Aug 29, 2019 at 2:05 AM Sage Imel <sage@cat.pdx.edu> wrote:
From: Sage Imel <sage@sagenite.net>
Always returns the full list of volumes, can't just ask it how many volumes are in the pool
Signed-off-by: Sage Imel <sage@cat.pdx.edu> --- src/storage_pool.rs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/src/storage_pool.rs b/src/storage_pool.rs index 38676c2..e8ed21c 100644 --- a/src/storage_pool.rs +++ b/src/storage_pool.rs @@ -18,7 +18,7 @@
extern crate libc;
-use std::str; +use std::{str, ptr};
use connect::sys::virConnectPtr; use storage_vol::sys::virStorageVolPtr; @@ -57,6 +57,10 @@ extern "C" { xml: *const libc::c_char, flags: libc::c_uint) -> sys::virStoragePoolPtr; + fn virStoragePoolListAllVolumes(ptr: sys::virStoragePoolPtr, + vols: *mut *mut virStorageVolPtr, + flags:libc::c_uint) + -> libc::c_int; fn virStoragePoolLookupByID(c: virConnectPtr, id: libc::c_int) -> sys::virStoragePoolPtr; fn virStoragePoolLookupByName(c: virConnectPtr, id: *const libc::c_char) @@ -103,6 +107,8 @@ pub const STORAGE_POOL_CREATE_WITH_BUILD: StoragePoolCreateFlags = 1 << 0; pub const STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE: StoragePoolCreateFlags = 1 << 1; pub const STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE: StoragePoolCreateFlags = 1 << 2;
+pub type VirStoragePoolListAllVolumesFlags = self::libc::c_uint; + pub type StoragePoolState = self::libc::c_uint; pub const VIR_STORAGE_POOL_INACTIVE: StoragePoolState = 0; pub const VIR_STORAGE_POOL_BUILDING: StoragePoolState = 1; @@ -201,6 +207,27 @@ impl StoragePool { } }
+ pub fn list_all_volumes(&self, + flags: VirStoragePoolListAllVolumesFlags) + -> 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()); + } + + let mut array: Vec<StorageVol> = Vec::new(); + 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 lookup_by_id(conn: &Connect, id: u32) -> Result<StoragePool, Error> { unsafe { let ptr = virStoragePoolLookupByID(conn.as_ptr(), id as libc::c_int); -- 2.17.1
On Thu, Aug 29, 2019 at 02:05:12AM -0700, Sage Imel wrote:
From: Sage Imel <sage@sagenite.net>
Always returns the full list of volumes, can't just ask it how many volumes are in the pool
Signed-off-by: Sage Imel <sage@cat.pdx.edu> ---
That looks to be a nice add, thank you for this contribution, can you just consider the comments that i have added?
src/storage_pool.rs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/src/storage_pool.rs b/src/storage_pool.rs index 38676c2..e8ed21c 100644 --- a/src/storage_pool.rs +++ b/src/storage_pool.rs @@ -18,7 +18,7 @@
extern crate libc;
-use std::str; +use std::{str, ptr};
use connect::sys::virConnectPtr; use storage_vol::sys::virStorageVolPtr; @@ -57,6 +57,10 @@ extern "C" { xml: *const libc::c_char, flags: libc::c_uint) -> sys::virStoragePoolPtr; + fn virStoragePoolListAllVolumes(ptr: sys::virStoragePoolPtr, + vols: *mut *mut virStorageVolPtr, + flags:libc::c_uint) + -> libc::c_int; fn virStoragePoolLookupByID(c: virConnectPtr, id: libc::c_int) -> sys::virStoragePoolPtr; fn virStoragePoolLookupByName(c: virConnectPtr, id: *const libc::c_char) @@ -103,6 +107,8 @@ pub const STORAGE_POOL_CREATE_WITH_BUILD: StoragePoolCreateFlags = 1 << 0; pub const STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE: StoragePoolCreateFlags = 1 << 1; pub const STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE: StoragePoolCreateFlags = 1 << 2;
+pub type VirStoragePoolListAllVolumesFlags = self::libc::c_uint; +
You well added the new type but as you can see in the other types we prefer to remove the "vir" prefix. So that should be: pub type StoragePoolListAllVolumesFlags = self::libc::c_uint; Also that, you have to declare the related flags, see: include/libvirt-storage.h
pub type StoragePoolState = self::libc::c_uint; pub const VIR_STORAGE_POOL_INACTIVE: StoragePoolState = 0; pub const VIR_STORAGE_POOL_BUILDING: StoragePoolState = 1; @@ -201,6 +207,27 @@ impl StoragePool { } }
+ pub fn list_all_volumes(&self, + flags: VirStoragePoolListAllVolumesFlags) + -> Result<Vec<StorageVol>, Error> {
Please consider to well align the code.
+ unsafe {
I know that you will find it everywhere but now we prefer to avoid using a global "unsafe". It was not right. You should just use it when it's necessary and also you should add a comment to explain why it is safe. You can find an example in connect::open_auth().
+ 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()); + } + + let mut array: Vec<StorageVol> = Vec::new(); + 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 lookup_by_id(conn: &Connect, id: u32) -> Result<StoragePool, Error> { unsafe { let ptr = virStoragePoolLookupByID(conn.as_ptr(), id as libc::c_int);
It would be great if you can provide a test case or integration test with it. Thanks, s.
participants (2)
-
Sage Imel -
Sahid Orentino Ferdjaoui