1. Patchew
  2. Libvirt
  3. View series

[libvirt] [rust PATCH] Add list_all_volumes method for storage_pool::StoragePool

Sage Imel posted 1 patch 4 years, 7 months ago
Failed in applying to current master (apply log)
src/storage_pool.rs | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
[libvirt] [rust PATCH] Add list_all_volumes method for storage_pool::StoragePool
Posted by Sage Imel 4 years, 7 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [rust PATCH] Add list_all_volumes method for storage_pool::StoragePool
Posted by Sahid Orentino Ferdjaoui 4 years, 7 months ago 
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.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [rust PATCH] Add list_all_volumes method for storage_pool::StoragePool
Posted by Sage Imel 4 years, 7 months ago 
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
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.