Re: [rust PATCH 1/3] Implement virStoragePoolListAllVolumes and virStoragePoolListVolumes for StoragePool

2020-02-13 Thread Sahid Orentino Ferdjaoui
On Mon, Feb 10, 2020 at 01:38:04PM +, math...@pius.io wrote:
> From: Mathias Pius 
> 
> ---
>  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(
> +,
> +flags: StoragePoolListAllVolumesFlags
> +) -> Result, Error> {
> +unsafe {
> +let mut volumes: *mut virStorageVolPtr = ptr::null_mut();
> +
> +let size = virStoragePoolListAllVolumes(self.as_ptr(),  
> volumes, flags as libc::c_uint);
> +if size == -1 {
> +return Err(Error::new());
> +}
> +
> +mem::forget(volumes);
> +
> +let mut array: Vec = 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() -> Result, 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 = 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(, flags: u32) -> Result {
>  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(, [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(, [0]) {
> +Ok(storage_pool) => {
> +storage_pool.list_all_volumes(0).unwrap();
> +}
> +Err(e) => panic!("failed with code 

Re: [libvirt-rust PATCH v3 0/4] Map more functions in stream module

2020-01-30 Thread Sahid Orentino Ferdjaoui
On Wed, Jan 29, 2020 at 08:24:10PM -0700, Zixing Liu wrote:
> This set of patches will add more functions to the Rust bindings.
> Newly mapped functions from C library: virStreamNew 
> virStreamEventUpdateCallback virStreamEventRemoveCallback 
> virStreamEventAddCallback.
> 
> virStreamEventAddCallback can accept normal fn functions or closures (can 
> capture variables outside)
> 
> The changes are not very thoroughly tested since event module is not 
> implemented at all so the virStreamEventAddCallback will always return 
> "unsupported by the connection driver".
> 
> Version 2: Addressed comments
> Version 3: Undo format changes and rebased against latest branch

The version 3 of your patches don't pass CI. Please consider to use
`cargo fmt -v -- --check` to validate the coding style before to
submit any patches. Also you can use `cargo fmt` to help you for the
coding style.

> Zixing Liu (4):
>   libvirt-rust: stream: add more functions in stream
>   libvirt-rust: stream: add more functions in stream

I would imagine both of these patches can be merged together, `git
rebase -i master` and usage of `squash` is really helpful of that.

For the title you should find something a bit more explicit like:

  stream: add better coverage for the stream API

>   libvirt-rust: use reference instead of moving
>   libvirt-rust: stream: addressed comments

The best is to have the comments addressed in the patch itself. This
can easily be achieved using `git rebase -i master` and by editing the
right patch.

Besides that your patches are OK. If you don't mind I could take care
of addressing my comments before to merge them. You don't have to be
worry, you still keep the credits for them.

Sounds good for you?

>  src/domain.rs   |  2 +-
>  src/stream.rs   | 94 ++---
>  tests/stream.rs | 40 +
>  3 files changed, 130 insertions(+), 6 deletions(-)
>  create mode 100644 tests/stream.rs
> 
> -- 
> 2.25.0




[PATCH] docs: update Rust releases and resources links

2020-01-30 Thread Sahid Orentino Ferdjaoui
This is updating the releases and resources links so they point now to
crates.io for the releases and docs.rs for the api ref.

Signed-off-by: Sahid Orentino Ferdjaoui 
---
 docs/downloads.html.in | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/downloads.html.in b/docs/downloads.html.in
index e84455ca04..50e0c54046 100644
--- a/docs/downloads.html.in
+++ b/docs/downloads.html.in
@@ -165,7 +165,7 @@
 
   Rust
   
-https://libvirt.org/sources/rust/;>libvirt
+https://crates.io/crates/virt;>crates.io
   
   
 https://libvirt.org/git/?p=libvirt-rust.git;a=summary;>libvirt
@@ -174,7 +174,9 @@
 https://gitlab.com/libvirt/libvirt-rust;>gitlab
 https://github.com/libvirt/libvirt-rust;>github
   
-  
+  
+https://docs.rs/virt;>api ref
+  
 
 
   Integration modules
-- 
2.20.1




[libvirt-rust] change in coding style to follow rustfmt

2020-01-29 Thread Sahid Orentino Ferdjaoui
I'm proposing to merge a change [0] so the CI would check whether the
source code is well-formatted using rustfmt [1].

The aim is to add an easy convention to follow regarding the coding
style.

For contributors the natural thing to do before to submit any patches
is to verify the format using:

`cargo fmt -v -- --check`

The support of rustfmt via cargo can be added using:

`rustup component add rustfmt`

I did not want to pollute the mailing list with such big change which
doesn't bring anything.

Please let me know if you have any objections.


commit 851296786626d61ebec985a9caeb515514955dd8
Author: Sahid Orentino Ferdjaoui 
Date:   Tue Jan 28 15:05:44 2020 +0100

ensure that the code is well formatted using rustfmt.

In this commit we add CI rule so travis would check whether the code
is well formatted using rustfmt.

Signed-off-by: Sahid Orentino Ferdjaoui 

 .travis.yml   |4 +
 README.md |6 +
 examples/auth.rs  |   19 +-
 examples/hello.rs |   76 +--
 examples/migrate.rs   |   35 +-
 examples/suspend.rs   |   31 +-
 src/connect.rs|  534 ++
 src/domain.rs | 1323 +
 src/domain_snapshot.rs|   88 +--
 src/error.rs  |   13 +-
 src/interface.rs  |   38 +-
 src/lib.rs|   30 +-
 src/network.rs|   75 +--
 src/nodedev.rs|  122 +++--
 src/nwfilter.rs   |   14 +-
 src/secret.rs |   79 +--
 src/storage_pool.rs   |   99 ++--
 src/storage_vol.rs|  218 
 src/stream.rs |   27 +-
 tests/common/mod.rs   |  115 ++--
 tests/connect.rs  |   74 +--
 tests/domain.rs   |   14 +-
 tests/integration_qemu.rs |   39 +-
 tests/interface.rs|2 -
 tests/network.rs  |1 -
 tests/storage_pool.rs |   18 +-
 26 files changed, 1717 insertions(+), 1377 deletions(-)


Thanks.
s.

[0] 
https://github.com/sahid/libvirt-rust/commit/851296786626d61ebec985a9caeb515514955dd8
[1] https://travis-ci.org/sahid/libvirt-rust/builds/642931039




Re: [rust PATCH v2 3/5] libvirt-rust: stream: automated lint

2020-01-17 Thread Sahid Orentino Ferdjaoui
On Fri, Jan 17, 2020 at 12:23:46AM -0700, Zixing Liu wrote:
> On 1/13/20 3:53 AM, Sahid Orentino Ferdjaoui wrote:
> 
> > On Fri, Jan 10, 2020 at 11:35:32AM -0700, Zixing Liu wrote:
> >> On 2020-01-10 04:48, Sahid Orentino Ferdjaoui wrote:
> >>
> >>> On Tue, Dec 24, 2019 at 12:12:53AM -0700, Zixing Liu wrote:
> >>>> * used rustfmt to clean up the code
> >>>>
> >>>> Signed-off-by: Zixing Liu 
> >>>> ---
> >>>>  src/stream.rs | 97 +--
> >>>>  1 file changed, 56 insertions(+), 41 deletions(-)
> >>> NAK, we don't have specific rule for that. Even if I'm OK with you to
> >>> use rustfmt we should consider having CI to also check that.
> >> Well, it's also one part of my habit since formatted code is more
> >> readable and looks neat.  Speaking of CI, I think you could
> >> integrate the format check or even commit format changes in CI using
> >> `cargo-fmt` which comes with every Rust installation.
> > Do you think you can add with this patch a change in CI so that format
> > of src/stream.rs will be verified? So basically we will avoid
> > regression and we will be able to clean the code incrementally.
> 
> Well, I guess I can add the patch to check the format in the changed
> files from the last commit in the CI system. However,  I am not quite
> sure after merging a multi-commit patch, what changes would the
> `HEAD^...HEAD`  include.
> 
> 
> Also, in this patch series, I changed more than one file. The formatter
> will check the entire file in which the changes had occurred instead of
> the diff since the format check is contextual in Rust.
> 
> 
> And are there any other problems with this patch-set that I haven't
> addressed?

I suggest you to rebase your change and re-order the patches so the
"3/5 libvirt-rust: stream: automated lint" become the last one + you
add to that patch the CI update so that will ensure no regression
regarding the formatting of src/stream.rs.

About the other patches I commented to some of your patches, they are
just nits. if you can address them that would be great.

Thank you Zixing Liu for your work I will wait for your change to be
merged before to release a new version.

s.

> >>>> diff --git a/src/stream.rs b/src/stream.rs
> >>>> index 1ffd186..0d84fd7 100644
> >>>> --- a/src/stream.rs
> >>>> +++ b/src/stream.rs
> >>>> @@ -18,8 +18,8 @@
> >>>>  
> >>>>  extern crate libc;
> >>>>  
> >>>> -use connect::Connect;
> >>>>  use connect::sys::virConnectPtr;
> >>>> +use connect::Connect;
> >>>>  use std::convert::TryFrom;
> >>>>  
> >>>>  use error::Error;
> >>>> @@ -33,28 +33,28 @@ pub mod sys {
> >>>>  
> >>>>  #[link(name = "virt")]
> >>>>  extern "C" {
> >>>> -fn virStreamNew(c: virConnectPtr,
> >>>> -flags: libc::c_uint)
> >>>> - -> sys::virStreamPtr;
> >>>> -fn virStreamSend(c: sys::virStreamPtr,
> >>>> - data: *const libc::c_char,
> >>>> - nbytes: libc::size_t)
> >>>> - -> libc::c_int;
> >>>> -fn virStreamRecv(c: sys::virStreamPtr,
> >>>> - data: *mut libc::c_char,
> >>>> - nbytes: libc::size_t)
> >>>> - -> libc::c_int;
> >>>> +fn virStreamNew(c: virConnectPtr, flags: libc::c_uint) -> 
> >>>> sys::virStreamPtr;
> >>>> +fn virStreamSend(
> >>>> +c: sys::virStreamPtr,
> >>>> +data: *const libc::c_char,
> >>>> +nbytes: libc::size_t,
> >>>> +) -> libc::c_int;
> >>>> +fn virStreamRecv(
> >>>> +c: sys::virStreamPtr,
> >>>> +data: *mut libc::c_char,
> >>>> +nbytes: libc::size_t,
> >>>> +) -> libc::c_int;
> >>>>  fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int;
> >>>>  fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int;
> >>>>  fn virStreamFinish(c: sys::virStreamPtr) -> libc::c_int;
> >>>> -fn virStreamEventAddCallback(c: sys::virStreamPtr,
> >>>> - event: libc::c_int

Re: [libvirt] [rust PATCH v2 3/5] libvirt-rust: stream: automated lint

2020-01-13 Thread Sahid Orentino Ferdjaoui
On Fri, Jan 10, 2020 at 11:35:32AM -0700, Zixing Liu wrote:
> On 2020-01-10 04:48, Sahid Orentino Ferdjaoui wrote:
> 
> > On Tue, Dec 24, 2019 at 12:12:53AM -0700, Zixing Liu wrote:
> >> * used rustfmt to clean up the code
> >>
> >> Signed-off-by: Zixing Liu 
> >> ---
> >>  src/stream.rs | 97 +--
> >>  1 file changed, 56 insertions(+), 41 deletions(-)
> > NAK, we don't have specific rule for that. Even if I'm OK with you to
> > use rustfmt we should consider having CI to also check that.
> 

> Well, it's also one part of my habit since formatted code is more
> readable and looks neat.  Speaking of CI, I think you could
> integrate the format check or even commit format changes in CI using
> `cargo-fmt` which comes with every Rust installation.

Do you think you can add with this patch a change in CI so that format
of src/stream.rs will be verified? So basically we will avoid
regression and we will be able to clean the code incrementally.

> 
> >> diff --git a/src/stream.rs b/src/stream.rs
> >> index 1ffd186..0d84fd7 100644
> >> --- a/src/stream.rs
> >> +++ b/src/stream.rs
> >> @@ -18,8 +18,8 @@
> >>  
> >>  extern crate libc;
> >>  
> >> -use connect::Connect;
> >>  use connect::sys::virConnectPtr;
> >> +use connect::Connect;
> >>  use std::convert::TryFrom;
> >>  
> >>  use error::Error;
> >> @@ -33,28 +33,28 @@ pub mod sys {
> >>  
> >>  #[link(name = "virt")]
> >>  extern "C" {
> >> -fn virStreamNew(c: virConnectPtr,
> >> -flags: libc::c_uint)
> >> - -> sys::virStreamPtr;
> >> -fn virStreamSend(c: sys::virStreamPtr,
> >> - data: *const libc::c_char,
> >> - nbytes: libc::size_t)
> >> - -> libc::c_int;
> >> -fn virStreamRecv(c: sys::virStreamPtr,
> >> - data: *mut libc::c_char,
> >> - nbytes: libc::size_t)
> >> - -> libc::c_int;
> >> +fn virStreamNew(c: virConnectPtr, flags: libc::c_uint) -> 
> >> sys::virStreamPtr;
> >> +fn virStreamSend(
> >> +c: sys::virStreamPtr,
> >> +data: *const libc::c_char,
> >> +nbytes: libc::size_t,
> >> +) -> libc::c_int;
> >> +fn virStreamRecv(
> >> +c: sys::virStreamPtr,
> >> +data: *mut libc::c_char,
> >> +nbytes: libc::size_t,
> >> +) -> libc::c_int;
> >>  fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int;
> >>  fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int;
> >>  fn virStreamFinish(c: sys::virStreamPtr) -> libc::c_int;
> >> -fn virStreamEventAddCallback(c: sys::virStreamPtr,
> >> - event: libc::c_int,
> >> - callback: StreamEventCallback,
> >> - opaque: *const libc::c_void,
> >> - ff: FreeCallback)
> >> - -> libc::c_int;
> >> -fn virStreamEventUpdateCallback(c: sys::virStreamPtr,
> >> -events: libc::c_int) -> libc::c_int;
> >> +fn virStreamEventAddCallback(
> >> +c: sys::virStreamPtr,
> >> +event: libc::c_int,
> >> +callback: StreamEventCallback,
> >> +opaque: *const libc::c_void,
> >> +ff: FreeCallback,
> >> +) -> libc::c_int;
> >> +fn virStreamEventUpdateCallback(c: sys::virStreamPtr, events: 
> >> libc::c_int) -> libc::c_int;
> >>  fn virStreamEventRemoveCallback(c: sys::virStreamPtr) -> libc::c_int;
> >>  }
> >>  
> >> @@ -71,14 +71,16 @@ pub type StreamEventCallback = extern "C" 
> >> fn(sys::virStreamPtr, libc::c_int, *co
> >>  pub type FreeCallback = extern "C" fn(*mut libc::c_void);
> >>  
> >>  // wrapper for callbacks
> >> -extern "C" fn event_callback(c: sys::virStreamPtr, flags: libc::c_int, 
> >> opaque: *const libc::c_void) {
> >> -let flags = flags as StreamFlags;
> >> -let shadow_self = unsafe {
> >> -*(opaque as *mut Stream)
> >> -};
> >> -if let Some(callback) =  shadow_self.callback {
> >> -callback(::from_ptr

Re: [libvirt] [rust PATCH v2 5/5] libvirt-rust: stream: addressed comments

2020-01-10 Thread Sahid Orentino Ferdjaoui
On Tue, Dec 24, 2019 at 12:12:55AM -0700, Zixing Liu wrote:
> * minimized unsafe scope
> * removed pub from `from_ptr` function
> 
> Signed-off-by: Zixing Liu 
> ---
>  src/stream.rs | 16 +++-
>  1 file changed, 7 insertions(+), 9 deletions(-)

Reviewed-by: Sahid Orentino Ferdjaoui 

> diff --git a/src/stream.rs b/src/stream.rs
> index 0d84fd7..ea623f6 100644
> --- a/src/stream.rs
> +++ b/src/stream.rs
> @@ -114,16 +114,14 @@ impl Drop for Stream {
>  
>  impl Stream {
>  pub fn new(conn: , flags: StreamFlags) -> Result {
> -unsafe {
> -let ptr = virStreamNew(conn.as_ptr(), flags as libc::c_uint);
> -if ptr.is_null() {
> -return Err(Error::new());
> -}
> -return Ok(Stream::from_ptr(ptr));
> +let ptr = unsafe { virStreamNew(conn.as_ptr(), flags as 
> libc::c_uint) };
> +if ptr.is_null() {
> +return Err(Error::new());
>  }
> +return Ok(Stream::from_ptr(ptr));
>  }
>  
> -pub fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
> +fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
>  Stream {
>  ptr: Some(ptr),
>  callback: None,
> @@ -139,9 +137,9 @@ impl Stream {
>  if virStreamFree(self.as_ptr()) == -1 {
>  return Err(Error::new());
>  }
> -self.ptr = None;
> -return Ok(());
>  }
> +self.ptr = None;
> +return Ok(());
>  }
>  
>  pub fn finish(self) -> Result<(), Error> {
> -- 
> 2.24.1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [rust PATCH v2 4/5] libvirt-rust: use reference instead of moving

2020-01-10 Thread Sahid Orentino Ferdjaoui
On Tue, Dec 24, 2019 at 12:12:54AM -0700, Zixing Liu wrote:
> Signed-off-by: Zixing Liu 
> ---
>  src/domain.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Sahid Orentino Ferdjaoui 

> diff --git a/src/domain.rs b/src/domain.rs
> index 40a18d8..61ca411 100644
> --- a/src/domain.rs
> +++ b/src/domain.rs
> @@ -1546,7 +1546,7 @@ impl Domain {
>  }
>  }
>  
> -pub fn open_console(, name: , stream: Stream, flags: u32) -> 
> Result {
> +pub fn open_console(, name: , stream: , flags: u32) -> 
> Result {
>  unsafe {
>  let ret = virDomainOpenConsole(self.as_ptr(),
> string_to_c_chars!(name),
> -- 
> 2.24.1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [rust PATCH v2 3/5] libvirt-rust: stream: automated lint

2020-01-10 Thread Sahid Orentino Ferdjaoui
On Tue, Dec 24, 2019 at 12:12:53AM -0700, Zixing Liu wrote:
> * used rustfmt to clean up the code
> 
> Signed-off-by: Zixing Liu 
> ---
>  src/stream.rs | 97 +--
>  1 file changed, 56 insertions(+), 41 deletions(-)

NAK, we don't have specific rule for that. Even if I'm OK with you to
use rustfmt we should consider having CI to also check that.

> diff --git a/src/stream.rs b/src/stream.rs
> index 1ffd186..0d84fd7 100644
> --- a/src/stream.rs
> +++ b/src/stream.rs
> @@ -18,8 +18,8 @@
>  
>  extern crate libc;
>  
> -use connect::Connect;
>  use connect::sys::virConnectPtr;
> +use connect::Connect;
>  use std::convert::TryFrom;
>  
>  use error::Error;
> @@ -33,28 +33,28 @@ pub mod sys {
>  
>  #[link(name = "virt")]
>  extern "C" {
> -fn virStreamNew(c: virConnectPtr,
> -flags: libc::c_uint)
> - -> sys::virStreamPtr;
> -fn virStreamSend(c: sys::virStreamPtr,
> - data: *const libc::c_char,
> - nbytes: libc::size_t)
> - -> libc::c_int;
> -fn virStreamRecv(c: sys::virStreamPtr,
> - data: *mut libc::c_char,
> - nbytes: libc::size_t)
> - -> libc::c_int;
> +fn virStreamNew(c: virConnectPtr, flags: libc::c_uint) -> 
> sys::virStreamPtr;
> +fn virStreamSend(
> +c: sys::virStreamPtr,
> +data: *const libc::c_char,
> +nbytes: libc::size_t,
> +) -> libc::c_int;
> +fn virStreamRecv(
> +c: sys::virStreamPtr,
> +data: *mut libc::c_char,
> +nbytes: libc::size_t,
> +) -> libc::c_int;
>  fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int;
>  fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int;
>  fn virStreamFinish(c: sys::virStreamPtr) -> libc::c_int;
> -fn virStreamEventAddCallback(c: sys::virStreamPtr,
> - event: libc::c_int,
> - callback: StreamEventCallback,
> - opaque: *const libc::c_void,
> - ff: FreeCallback)
> - -> libc::c_int;
> -fn virStreamEventUpdateCallback(c: sys::virStreamPtr,
> -events: libc::c_int) -> libc::c_int;
> +fn virStreamEventAddCallback(
> +c: sys::virStreamPtr,
> +event: libc::c_int,
> +callback: StreamEventCallback,
> +opaque: *const libc::c_void,
> +ff: FreeCallback,
> +) -> libc::c_int;
> +fn virStreamEventUpdateCallback(c: sys::virStreamPtr, events: 
> libc::c_int) -> libc::c_int;
>  fn virStreamEventRemoveCallback(c: sys::virStreamPtr) -> libc::c_int;
>  }
>  
> @@ -71,14 +71,16 @@ pub type StreamEventCallback = extern "C" 
> fn(sys::virStreamPtr, libc::c_int, *co
>  pub type FreeCallback = extern "C" fn(*mut libc::c_void);
>  
>  // wrapper for callbacks
> -extern "C" fn event_callback(c: sys::virStreamPtr, flags: libc::c_int, 
> opaque: *const libc::c_void) {
> -let flags = flags as StreamFlags;
> -let shadow_self = unsafe {
> -*(opaque as *mut Stream)
> -};
> -if let Some(callback) =  shadow_self.callback {
> -callback(::from_ptr(c), flags);
> -}
> +extern "C" fn event_callback(
> +c: sys::virStreamPtr,
> +flags: libc::c_int,
> +opaque: *const libc::c_void,
> +) {
> +let flags = flags as StreamFlags;
> +let shadow_self = unsafe {  *(opaque as *mut Stream) };
> +if let Some(callback) =  shadow_self.callback {
> +callback(::from_ptr(c), flags);
> +}
>  }
>  
>  extern "C" fn event_free(_opaque: *mut libc::c_void) {}
> @@ -93,16 +95,18 @@ impl Drop for Stream {
>  fn drop( self) {
>  if self.ptr.is_some() {
>  if let Err(e) = self.free() {
> -panic!("Unable to drop memory for Stream, code {}, message: 
> {}",
> -   e.code,
> -   e.message)
> +panic!(
> +"Unable to drop memory for Stream, code {}, message: {}",
> +e.code, e.message
> +)
>  }
>  }
>  if self.callback.is_some() {
>  if let Err(e) = self.event_remove_callback() {
> -panic!("Unable to remove event callback for Stream, code {}, 
> message: {}",
> -   e.code,
> -   e.message)
> +panic!(
> +"Unable to remove event callback for Stream, code {}, 
> message: {}",
> +e.code, e.message
> +)
>  }
>  }
>  }
> @@ -120,7 +124,10 @@ impl Stream {
>  }
>  
>  pub fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
> -Stream { ptr: Some(ptr), callback: None }
> +Stream {
> +ptr: Some(ptr),
> +callback: None,
> +}
>  }
>  
>  pub fn 

Re: [libvirt] [rust PATCH v2 2/5] libvirt-rust: stream: add more functions in stream

2020-01-10 Thread Sahid Orentino Ferdjaoui
On Tue, Dec 24, 2019 at 12:12:52AM -0700, Zixing Liu wrote:
> * added virStreamEventAddCallback function
> * added new types: StreamEventCallback and FreeCallback
> * added new field: callback for storing event callback
> * drop: will drop the Box if any
> * added wrapper event_callback for easier callback authoring for the
>   user (so that closures with Fn or FnMut references could be used)
> * added padding function event_free to just makes it compile (Rust
>   should not need this, because Rust sticks to RAII)
> 
> Signed-off-by: Zixing Liu 
> ---
>  src/stream.rs | 46 --
>  1 file changed, 44 insertions(+), 2 deletions(-)

Reviewed-by: Sahid Orentino Ferdjaoui 

> diff --git a/src/stream.rs b/src/stream.rs
> index de272ab..1ffd186 100644
> --- a/src/stream.rs
> +++ b/src/stream.rs
> @@ -47,6 +47,12 @@ extern "C" {
>  fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int;
>  fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int;
>  fn virStreamFinish(c: sys::virStreamPtr) -> libc::c_int;
> +fn virStreamEventAddCallback(c: sys::virStreamPtr,
> + event: libc::c_int,
> + callback: StreamEventCallback,
> + opaque: *const libc::c_void,
> + ff: FreeCallback)
> + -> libc::c_int;
>  fn virStreamEventUpdateCallback(c: sys::virStreamPtr,
>  events: libc::c_int) -> libc::c_int;
>  fn virStreamEventRemoveCallback(c: sys::virStreamPtr) -> libc::c_int;
> @@ -61,9 +67,26 @@ pub const VIR_STREAM_EVENT_HANGUP: StreamEventType = (1 << 
> 3);
>  pub type StreamFlags = self::libc::c_uint;
>  pub const VIR_STREAM_NONBLOCK: StreamFlags = (1 << 0);
>  
> -#[derive(Debug)]
> +pub type StreamEventCallback = extern "C" fn(sys::virStreamPtr, libc::c_int, 
> *const libc::c_void);
> +pub type FreeCallback = extern "C" fn(*mut libc::c_void);
> +
> +// wrapper for callbacks
> +extern "C" fn event_callback(c: sys::virStreamPtr, flags: libc::c_int, 
> opaque: *const libc::c_void) {
> +let flags = flags as StreamFlags;
> +let shadow_self = unsafe {
> +*(opaque as *mut Stream)
> +};
> +if let Some(callback) =  shadow_self.callback {
> +callback(::from_ptr(c), flags);
> +}
> +}
> +
> +extern "C" fn event_free(_opaque: *mut libc::c_void) {}
> +
> +// #[derive(Debug)]

This line can be removed.

>  pub struct Stream {
>  ptr: Option,
> +callback: Option>,
>  }
>  
>  impl Drop for Stream {
> @@ -75,6 +98,13 @@ impl Drop for Stream {
> e.message)
>  }
>  }
> +if self.callback.is_some() {
> +if let Err(e) = self.event_remove_callback() {
> +panic!("Unable to remove event callback for Stream, code {}, 
> message: {}",
> +   e.code,
> +   e.message)
> +}
> +}
>  }
>  }
>  
> @@ -90,7 +120,7 @@ impl Stream {
>  }
>  
>  pub fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
> -Stream { ptr: Some(ptr) }
> +Stream { ptr: Some(ptr), callback: None }
>  }
>  
>  pub fn as_ptr() -> sys::virStreamPtr {
> @@ -147,6 +177,18 @@ impl Stream {
>  usize::try_from(ret).map_err(|_| Error::new())
>  }
>  
> +pub fn event_add_callback StreamEventType)>( self, events: StreamEventType, cb: F) -> Result<(), 
> Error> {
> +let ret = unsafe {
> +let ptr = &*self as *const _ as *const _;
> +virStreamEventAddCallback(self.as_ptr(), events as libc::c_int, 
> event_callback, ptr, event_free)
> +};
> +if ret == -1 {
> +return Err(Error::new());
> +}
> +self.callback = Some(Box::new(cb));
> +return Ok(());
> +}
> +
>  pub fn event_update_callback(, events: StreamEventType) -> 
> Result<(), Error> {
>  let ret = unsafe {
>  virStreamEventUpdateCallback(self.as_ptr(), events as 
> libc::c_int)
> -- 
> 2.24.1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [rust PATCH v2 1/5] libvirt-rust: stream: add more functions in stream

2020-01-10 Thread Sahid Orentino Ferdjaoui
On Tue, Dec 24, 2019 at 12:12:51AM -0700, Zixing Liu wrote:
> * added new functions: virStreamNew virStreamEventUpdateCallback
>   virStreamEventRemoveCallback
> * added new constants: VIR_STREAM_NONBLOCK
> * added new types/aliases: StreamFlags
> * changed the previous `new` associate function to `from_ptr` to avoid
>   naming conflicts
> * added basic tests to test the creation of Stream struct (can not test
>   the correctness of virStreamEventRemoveCallback and
>   virStreamEventUpdateCallback due to "unsupported by the connection
>   driver")
> 
> Signed-off-by: Zixing Liu 
> ---
>  src/stream.rs   | 42 +-
>  tests/stream.rs | 40 
>  2 files changed, 81 insertions(+), 1 deletion(-)
>  create mode 100644 tests/stream.rs

Reviewed-by: Sahid Orentino Ferdjaoui 

> diff --git a/src/stream.rs b/src/stream.rs
> index 8bcdf4b..de272ab 100644
> --- a/src/stream.rs
> +++ b/src/stream.rs
> @@ -18,6 +18,8 @@
>  
>  extern crate libc;
>  
> +use connect::Connect;
> +use connect::sys::virConnectPtr;
>  use std::convert::TryFrom;
>  
>  use error::Error;
> @@ -31,6 +33,9 @@ pub mod sys {
>  
>  #[link(name = "virt")]
>  extern "C" {
> +fn virStreamNew(c: virConnectPtr,
> +flags: libc::c_uint)
> + -> sys::virStreamPtr;
>  fn virStreamSend(c: sys::virStreamPtr,
>   data: *const libc::c_char,
>   nbytes: libc::size_t)
> @@ -42,6 +47,9 @@ extern "C" {
>  fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int;
>  fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int;
>  fn virStreamFinish(c: sys::virStreamPtr) -> libc::c_int;
> +fn virStreamEventUpdateCallback(c: sys::virStreamPtr,
> +events: libc::c_int) -> libc::c_int;
> +fn virStreamEventRemoveCallback(c: sys::virStreamPtr) -> libc::c_int;
>  }
>  
>  pub type StreamEventType = self::libc::c_uint;
> @@ -50,6 +58,9 @@ pub const VIR_STREAM_EVENT_WRITABLE: StreamEventType = (1 
> << 1);
>  pub const VIR_STREAM_EVENT_ERROR: StreamEventType = (1 << 2);
>  pub const VIR_STREAM_EVENT_HANGUP: StreamEventType = (1 << 3);
>  
> +pub type StreamFlags = self::libc::c_uint;
> +pub const VIR_STREAM_NONBLOCK: StreamFlags = (1 << 0);
> +
>  #[derive(Debug)]
>  pub struct Stream {
>  ptr: Option,
> @@ -68,7 +79,17 @@ impl Drop for Stream {
>  }
>  
>  impl Stream {
> -pub fn new(ptr: sys::virStreamPtr) -> Stream {
> +pub fn new(conn: , flags: StreamFlags) -> Result {
> +unsafe {
> +let ptr = virStreamNew(conn.as_ptr(), flags as libc::c_uint);
> +if ptr.is_null() {
> +return Err(Error::new());
> +}

Is this block can be outside of the unsafe ? +1 if you add comment
explaining why it is safe to use conn.as_ptr().

> +return Ok(Stream::from_ptr(ptr));
> +}
> +}
> +
> +pub fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
>  Stream { ptr: Some(ptr) }
>  }
>  
> @@ -125,4 +146,23 @@ impl Stream {
>  };
>  usize::try_from(ret).map_err(|_| Error::new())
>  }
> +
> +pub fn event_update_callback(, events: StreamEventType) -> 
> Result<(), Error> {
> +let ret = unsafe {
> +virStreamEventUpdateCallback(self.as_ptr(), events as 
> libc::c_int)

nit: Could be great to have a comment explaining why self.as_ptr() is safe to 
use.

> +};
> +if ret == -1 {
> +return Err(Error::new());
> +}
> +return Ok(());
> +}
> +
> +pub fn event_remove_callback() -> Result<(), Error> {
> +unsafe {
> +if virStreamEventRemoveCallback(self.as_ptr()) == -1 {

Same comment here.

> +return Err(Error::new());
> +}
> +return Ok(());
> +}
> +}
>  }
> diff --git a/tests/stream.rs b/tests/stream.rs
> new file mode 100644
> index 000..16531b3
> --- /dev/null
> +++ b/tests/stream.rs
> @@ -0,0 +1,40 @@
> +/*
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTI

Re: [libvirt] [rust PATCH] Domain: Implement scheduler information related APIs

2019-12-16 Thread Sahid Orentino Ferdjaoui
On Sat, Dec 14, 2019 at 01:38:56PM +, Vineeth Remanan Pillai wrote:
> Implement the following:
> - virDomainGetSchedulerType
> - virDomainSetSchedulerParameters
> - virDomainSetSchedulerParametersFlags
> - virDomainGetSchedulerParameters
> - virDomainGetSchedulerParametersFlags
> 
> Signed-off-by: Vineeth Remanan Pillai 
> ---
>  examples/hello.rs |  38 +++
>  src/domain.rs | 260 +++---
>  2 files changed, 283 insertions(+), 15 deletions(-)

Reviewed-by: Sahid Orentino Ferdjaoui 

I will merge it and push a new release as soon as I can. Thank you for
your contribution.

s.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH Rust 4/4] libvirt-rust: use reference instead of moving

2019-11-14 Thread Sahid Orentino Ferdjaoui
On Tue, Nov 12, 2019 at 02:44:45PM -0700, Zixing Liu wrote:
> Signed-off-by: Zixing Liu 
> ---
>  src/domain.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Sahid Orentino Ferdjaoui 

> 
> diff --git a/src/domain.rs b/src/domain.rs
> index acb9e6e..4d4f593 100644
> --- a/src/domain.rs
> +++ b/src/domain.rs
> @@ -1397,7 +1397,7 @@ impl Domain {
>  }
>  }
>  
> -pub fn open_console(, name: , stream: Stream, flags: u32) -> 
> Result {
> +pub fn open_console(, name: , stream: , flags: u32) -> 
> Result {
>  unsafe {
>  let ret = virDomainOpenConsole(self.as_ptr(),
> string_to_c_chars!(name),
> -- 
> 2.24.0


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH Rust 3/4] libvirt-rust: stream: automated lint

2019-11-14 Thread Sahid Orentino Ferdjaoui
On Tue, Nov 12, 2019 at 02:44:44PM -0700, Zixing Liu wrote:
> * used rustfmt to clean up the code
> 
> Signed-off-by: Zixing Liu 
> ---
>  src/stream.rs | 97 +--
>  1 file changed, 56 insertions(+), 41 deletions(-)

NACK, We don't have any rules for that so far. I think that would make
sense at some point but we will probably have to do that in one
iteration and document somewhere our desires.

Thanks,
s.

> diff --git a/src/stream.rs b/src/stream.rs
> index 1ffd186..0d84fd7 100644
> --- a/src/stream.rs
> +++ b/src/stream.rs
> @@ -18,8 +18,8 @@
>  
>  extern crate libc;
>  
> -use connect::Connect;
>  use connect::sys::virConnectPtr;
> +use connect::Connect;
>  use std::convert::TryFrom;
>  
>  use error::Error;
> @@ -33,28 +33,28 @@ pub mod sys {
>  
>  #[link(name = "virt")]
>  extern "C" {
> -fn virStreamNew(c: virConnectPtr,
> -flags: libc::c_uint)
> - -> sys::virStreamPtr;
> -fn virStreamSend(c: sys::virStreamPtr,
> - data: *const libc::c_char,
> - nbytes: libc::size_t)
> - -> libc::c_int;
> -fn virStreamRecv(c: sys::virStreamPtr,
> - data: *mut libc::c_char,
> - nbytes: libc::size_t)
> - -> libc::c_int;
> +fn virStreamNew(c: virConnectPtr, flags: libc::c_uint) -> 
> sys::virStreamPtr;
> +fn virStreamSend(
> +c: sys::virStreamPtr,
> +data: *const libc::c_char,
> +nbytes: libc::size_t,
> +) -> libc::c_int;
> +fn virStreamRecv(
> +c: sys::virStreamPtr,
> +data: *mut libc::c_char,
> +nbytes: libc::size_t,
> +) -> libc::c_int;
>  fn virStreamFree(c: sys::virStreamPtr) -> libc::c_int;
>  fn virStreamAbort(c: sys::virStreamPtr) -> libc::c_int;
>  fn virStreamFinish(c: sys::virStreamPtr) -> libc::c_int;
> -fn virStreamEventAddCallback(c: sys::virStreamPtr,
> - event: libc::c_int,
> - callback: StreamEventCallback,
> - opaque: *const libc::c_void,
> - ff: FreeCallback)
> - -> libc::c_int;
> -fn virStreamEventUpdateCallback(c: sys::virStreamPtr,
> -events: libc::c_int) -> libc::c_int;
> +fn virStreamEventAddCallback(
> +c: sys::virStreamPtr,
> +event: libc::c_int,
> +callback: StreamEventCallback,
> +opaque: *const libc::c_void,
> +ff: FreeCallback,
> +) -> libc::c_int;
> +fn virStreamEventUpdateCallback(c: sys::virStreamPtr, events: 
> libc::c_int) -> libc::c_int;
>  fn virStreamEventRemoveCallback(c: sys::virStreamPtr) -> libc::c_int;
>  }
>  
> @@ -71,14 +71,16 @@ pub type StreamEventCallback = extern "C" 
> fn(sys::virStreamPtr, libc::c_int, *co
>  pub type FreeCallback = extern "C" fn(*mut libc::c_void);
>  
>  // wrapper for callbacks
> -extern "C" fn event_callback(c: sys::virStreamPtr, flags: libc::c_int, 
> opaque: *const libc::c_void) {
> -let flags = flags as StreamFlags;
> -let shadow_self = unsafe {
> -*(opaque as *mut Stream)
> -};
> -if let Some(callback) =  shadow_self.callback {
> -callback(::from_ptr(c), flags);
> -}
> +extern "C" fn event_callback(
> +c: sys::virStreamPtr,
> +flags: libc::c_int,
> +opaque: *const libc::c_void,
> +) {
> +let flags = flags as StreamFlags;
> +let shadow_self = unsafe {  *(opaque as *mut Stream) };
> +if let Some(callback) =  shadow_self.callback {
> +callback(::from_ptr(c), flags);
> +}
>  }
>  
>  extern "C" fn event_free(_opaque: *mut libc::c_void) {}
> @@ -93,16 +95,18 @@ impl Drop for Stream {
>  fn drop( self) {
>  if self.ptr.is_some() {
>  if let Err(e) = self.free() {
> -panic!("Unable to drop memory for Stream, code {}, message: 
> {}",
> -   e.code,
> -   e.message)
> +panic!(
> +"Unable to drop memory for Stream, code {}, message: {}",
> +e.code, e.message
> +)
>  }
>  }
>  if self.callback.is_some() {
>  if let Err(e) = self.event_remove_callback() {
> -panic!("Unable to remove event callback for Stream, code {}, 
> message: {}",
> -   e.code,
> -   e.message)
> +panic!(
> +"Unable to remove event callback for Stream, code {}, 
> message: {}",
> +e.code, e.message
> +)
>  }
>  }
>  }
> @@ -120,7 +124,10 @@ impl Stream {
>  }
>  
>  pub fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
> -Stream { ptr: Some(ptr), callback: None }
> +Stream {
> +ptr: Some(ptr),
> +

Re: [libvirt] [PATCH Rust 1/4] libvirt-rust: stream: add more functions in stream

2019-11-13 Thread Sahid Orentino Ferdjaoui
Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Sahid Orentino Ferdjaoui 
> + */
> +
> +extern crate virt;
> +
> +mod common;
> +
> +use virt::stream::Stream;
> +use virt::stream::VIR_STREAM_NONBLOCK;
> +
> +#[test]
> +fn test_create_blocking() {
> +let c = common::conn();
> +let s = Stream::new(, 0).unwrap();
> +drop(s);
> +common::close(c);
> +}
> +
> +#[test]
> +fn test_create_non_blocking() {
> +let c = common::conn();
> +let s = Stream::new(, VIR_STREAM_NONBLOCK).unwrap();
> +drop(s);
> +common::close(c);
> +}
> -- 
> 2.24.0


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [rust PATCH] gitpublish: Fix subject prefix

2019-11-13 Thread Sahid Orentino Ferdjaoui
On Wed, Nov 13, 2019 at 10:24:29AM +0100, Andrea Bolognani wrote:
> All libvirt-related projects, including language bindings,
> follow a certain convention for subject prefix, and there's no
> reason libvirt-rust should be any different.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  .gitpublish | 2 +-
>  README.md   | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Sahid Orentino Ferdjaoui 

Thanks,
s.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH Rust] api_tests.py: update to use Python 3

2019-11-12 Thread Sahid Orentino Ferdjaoui
On Tue, Nov 12, 2019 at 11:36:49AM +0100, Andrea Bolognani wrote:
> On Fri, 2019-11-01 at 14:23 -0600, liushuyu wrote:
> > From: liushuyu 
> 
> Please include a Signed-off-by: tag with your full legal name and
> your email address to certify that you're in compliance with the DCO,
> as explained in [1]. git authorship should also match.
> 
> We cannot merge the patch until you've signed off the DCO.
> 
> > +++ b/tools/api_tests.py
> > @@ -47,10 +47,10 @@ def main():
> >  else:
> >  missing.add(el)
> >  
> > -print "missing: %s, implemented: %s" % (len(missing), len(implemented))
> > -print "missing:"
> > +print("missing: %s, implemented: %s" % (len(missing), 
> > len(implemented)))
> > +print("missing:")
> 
> Interesting: it looks like
> 
>   from __future__ import print_function
> 
> is no longer necessary in Python 2.7, which is the only version of
> Python 2 we still care about (though not for much longer!).
> 
> 
> The patch looks good! Please take care of the formalities mentioned
> above and repost, then I'll happily merge it :)

Hum... yes, thank you Andrea to handle this patch and thanks Liushuyu
for your contribution.

s.

> 
> [1] https://libvirt.org/hacking.html#patches item 6
> -- 
> Andrea Bolognani / Red Hat / Virtualization


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH 2/2] libvirt-rust Fix Stream::send

2019-10-16 Thread Sahid Orentino Ferdjaoui
On Fri, Sep 20, 2019 at 10:02:47AM +0200, Martin Kletzander wrote:
> On Thu, Sep 19, 2019 at 05:48:49AM +, Linus Färnstrand wrote:
> > * Handle the -2 error case
> > * Allow sending arbitrary byte array, not just UTF-8 strings
> > * Fix FFI declaration, takes size_t, not c_uint
> > * Return usize to be more idiomatic (type used for slice indexing)
> > 
> > Signed-off-by: Linus Färnstrand 
> > ---
> > src/stream.rs | 21 ++---
> > 1 file changed, 10 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/stream.rs b/src/stream.rs
> > index af6c8ec..2ca5d0b 100644
> > --- a/src/stream.rs
> > +++ b/src/stream.rs
> > @@ -34,7 +34,7 @@ pub mod sys {
> > extern "C" {
> > fn virStreamSend(c: sys::virStreamPtr,
> >  data: *const libc::c_char,
> > - nbytes: libc::c_uint)
> > + nbytes: libc::size_t)
> >  -> libc::c_int;
> > fn virStreamRecv(c: sys::virStreamPtr,
> >  data: *mut libc::c_char,
> > @@ -105,16 +105,15 @@ impl Stream {
> > }
> > }
> > 
> > -pub fn send(, data: ) -> Result {
> > -unsafe {
> > -let ret = virStreamSend(self.as_ptr(),
> > -string_to_c_chars!(data),
> > -data.len() as libc::c_uint);
> > -if ret == -1 {
> > -return Err(Error::new());
> > -}
> > -return Ok(ret as u32);
> > -}
> > +pub fn send(, data: &[u8]) -> Result {
> > +let ret = unsafe {
> > +virStreamSend(
> > +self.as_ptr(),
> > +data.as_ptr() as *mut libc::c_char,
> > +data.len()
> > +)
> > +};
> > +usize::try_from(ret).map_err(|_| Error::new())
> 
> I guess same as the comment form the first patch should be applied here, but 
> we
> might need to even change the return value so that it is properly 
> distinguished
> as here it has yet another meaning.  Are you basing this on some of your work 
> on
> top of libvirt-rust?  Is this the easy to use (semi-)properly?  If yes, we 
> might
> just as well keep it this way as it is still way better than what we have now.

I think the patch is good as it is now. Of-course distinguish between
-1 and -2 would be the ideal but even previously that was not the
case. We could still improve it in the future.

Thank you Linus for your patch.

I will push it Martin if you don't have objection.

Thanks,
s.

> > }
> > 
> > pub fn recv(, buf:  [u8]) -> Result {
> > --
> > 2.21.0
> > 
> > 
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list



> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [libvirt-rust PATCH 1/1] Make creating safe wrapper from raw pointer unsafe

2019-10-16 Thread Sahid Orentino Ferdjaoui
On Mon, Sep 30, 2019 at 12:10:49PM +0200, Martin Kletzander wrote:
> On Wed, Sep 25, 2019 at 04:36:16PM +, Linus Färnstrand wrote:
> > Giving an invalid pointer to the safe wrapper types causes
> > undefined behavior when methods are later called on said wrapper
> > 
> > Properly document safety contract of using unsafe constructor
> > ---
> 

> Ideally it should not be exposed at all because there already is a
> method for getting the proper structure.  And that method (or those
> methods) already use ::new() so with this patch it just fails.
> Tests, examples, etc.  Instead of fixing that I would, probably,
> just not expose them.  I don't see a reason for them being public.

Yes I'm agree, they should just not be public.

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

2019-09-02 Thread Sahid Orentino Ferdjaoui
On Thu, Aug 29, 2019 at 02:05:12AM -0700, Sage Imel wrote:
> From: Sage Imel 
> 
> Always returns the full list of volumes,
> can't just ask it how many volumes are in the pool
> 
> Signed-off-by: Sage Imel 
> ---

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(,
> +  flags: VirStoragePoolListAllVolumesFlags)
> +  -> Result, 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(),  volumes, 
> flags as libc::c_uint);
> +if size == -1 {
> +return Err(Error::new());
> +}
> +
> +let mut array: Vec = 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: , id: u32) -> Result 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


[libvirt] Rust, new release v0.2.11

2019-08-29 Thread Sahid Orentino Ferdjaoui
  Just A small note to indicate that a new release v0.2.11 has been
published.

  https://docs.rs/virt/0.2.11/virt/

Thanks to Sage Imel for its contribution to a bug fix:
  e737a93 virDomainGetAutostart takes a pointer that it writes the output value 
to

s.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH Rust 1/4] connect: cleanup around Connect::open_auth()'s method

2019-07-17 Thread Sahid Orentino Ferdjaoui
In this refactor we avoid to enclose all the code with unsafe tags and
just use it when necessary. Also we add annotations to explain why
it's safe to use.

The integrations tests related are also been reviewed to avoid using
panic.

Signed-off-by: Sahid Orentino Ferdjaoui 
---
 src/connect.rs| 91 ++-
 tests/integration_qemu.rs | 16 +++
 2 files changed, 56 insertions(+), 51 deletions(-)

diff --git a/src/connect.rs b/src/connect.rs
index 0fa8551..108224d 100644
--- a/src/connect.rs
+++ b/src/connect.rs
@@ -240,6 +240,41 @@ extern "C" {
 -> *mut libc::c_char;
 }
 
+extern "C" fn connectCallback(ccreds: sys::virConnectCredentialPtr,
+  ncred: libc::c_uint,
+  cbdata: *mut libc::c_void)
+  -> libc::c_int {
+let callback: ConnectAuthCallback = unsafe {
+// Safe because connectCallback is private and only used by
+// Connect::open_auth(). In open_auth() we transmute the
+// callback allocate in *void.
+mem::transmute(cbdata)
+};
+let mut rcreds: Vec = Vec::new();
+for i in 0..ncred as isize {
+unsafe {
+// Safe because ccreds is allocated.
+let c = ConnectCredential::from_ptr(ccreds.offset(i));
+rcreds.push(c);
+}
+}
+callback( rcreds);
+for i in 0..ncred as isize {
+if rcreds[i as usize].result.is_some() {
+if let Some(ref result) = rcreds[i as usize].result {
+unsafe {
+// Safe because ccreds is allocated and the result
+// is comming from Rust calls.
+(*ccreds.offset(i)).resultlen = result.len() as 
libc::c_uint;
+(*ccreds.offset(i)).result = 
string_to_mut_c_chars!(result.clone());
+}
+}
+}
+}
+0
+}
+
+
 pub type ConnectFlags = self::libc::c_uint;
 pub const VIR_CONNECT_RO: ConnectFlags = 1 << 0;
 pub const VIR_CONNECT_NO_ALIASES: ConnectFlags = 1 << 1;
@@ -412,39 +447,6 @@ impl ConnectAuth {
 callback: callback,
 }
 }
-
-fn to_cstruct( self) -> sys::virConnectAuth {
-unsafe extern "C" fn wrapper(ccreds: sys::virConnectCredentialPtr,
- ncred: libc::c_uint,
- cbdata: *mut libc::c_void)
- -> libc::c_int {
-let callback: ConnectAuthCallback = mem::transmute(cbdata);
-let mut rcreds: Vec = Vec::new();
-for i in 0..ncred as isize {
-let c = ConnectCredential::from_ptr(ccreds.offset(i));
-rcreds.push(c);
-}
-callback( rcreds);
-for i in 0..ncred as isize {
-if rcreds[i as usize].result.is_some() {
-if let Some(ref result) = rcreds[i as usize].result {
-(*ccreds.offset(i)).resultlen = result.len() as 
libc::c_uint;
-(*ccreds.offset(i)).result = 
string_to_mut_c_chars!(result.clone());
-}
-}
-}
-0
-}
-
-unsafe {
-sys::virConnectAuth {
-credtype:  self.creds[0],
-ncredtype: self.creds.len() as u32,
-cb: wrapper,
-cbdata: mem::transmute(self.callback),
-}
-}
-}
 }
 
 /// Provides APIs for the management of hosts.
@@ -554,14 +556,23 @@ impl Connect {
  auth:  ConnectAuth,
  flags: ConnectFlags)
  -> Result {
-unsafe {
-let mut cauth = auth.to_cstruct();
-let c = virConnectOpenAuth(string_to_c_chars!(uri),  cauth, 
flags as libc::c_uint);
-if c.is_null() {
-return Err(Error::new());
-}
-return Ok(Connect::new(c));
+let mut cauth = unsafe {
+// Safe because Rust forces to allocate all attributes of
+// the struct ConnectAuth.
+sys::virConnectAuth {
+credtype:  auth.creds[0],
+ncredtype: auth.creds.len() as libc::c_uint,
+cb: connectCallback,
+cbdata: mem::transmute(auth.callback),
+}
+};
+let c = unsafe {
+virConnectOpenAuth(string_to_c_chars!(uri),  cauth, flags as 
libc::c_uint)
+};
+if c.is_null() {
+return Err(Error::new());
 }
+return Ok(Connect::new(c));
 }
 
 
diff --git a/tests/integration_qemu.rs b/tests/integration_qemu.rs
index 49e07c4..79aa2bd 100644
--- a/tests/integration_qemu.rs
+++ b/tests/integration_qemu.rs
@@ -108,14 +108,9 @@ fn test_connection_with_auth() {

[libvirt] [PATCH Rust 2/4] remove duplicated macro

2019-07-17 Thread Sahid Orentino Ferdjaoui
Signed-off-by: Sahid Orentino Ferdjaoui 
---
 src/lib.rs | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index f9ede21..77bf4a9 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -107,10 +107,6 @@ macro_rules! string_to_mut_c_chars {
 ($x:expr) => (::std::ffi::CString::new($x).unwrap().into_raw())
 }
 
-macro_rules! string_to_mut_c_chars {
-($x:expr) => (::std::ffi::CString::new($x).unwrap().into_raw())
-}
-
 macro_rules! impl_from {
 // Largely inspired by impl_from! in rust core/num/mod.rs
 ($Small: ty, $Large: ty) => {
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH Rust 3/4] fix pointer type of macro used to convert rs string to c string

2019-07-17 Thread Sahid Orentino Ferdjaoui
In this commit we also add todo and warning to avoid using them +
remove them in future.

Signed-off-by: Sahid Orentino Ferdjaoui 
---
 src/lib.rs | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index 77bf4a9..64d49cd 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -99,12 +99,31 @@ macro_rules! c_chars_to_string {
 
 }
 
+// Those two macros are not completely safe and we should probably
+// stop using them to avoid possibility of pointers dangling. The
+// memory may be freed too early.
+//
+// To avoid that, the right pattern would be:
+//
+// let cstring = CString::new(rs_string).unwrap();
+// unsafe {
+//   some_c_function(cstring.as_ptr() as *const libc::c_char);
+// }
+//
+// So we ensure the pointer passed to 'some_c_function()' will live
+// until 'cstring' exists.
+//
+// TODO(sahid): fix code + remove macros.
+
 macro_rules! string_to_c_chars {
-($x:expr) => (::std::ffi::CString::new($x).unwrap().as_ptr())
+($x:expr) => (
+::std::ffi::CString::new($x).unwrap().as_ptr() as *const libc::c_char)
 }
 
 macro_rules! string_to_mut_c_chars {
-($x:expr) => (::std::ffi::CString::new($x).unwrap().into_raw())
+($x:expr) => (
+// Usage of this should ensure deallocation.
+::std::ffi::CString::new($x).unwrap().into_raw() as *mut libc::c_char)
 }
 
 macro_rules! impl_from {
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH Rust 4/4] fix email address in Cargo.toml + update README

2019-07-17 Thread Sahid Orentino Ferdjaoui
This is fixing the invalid email address in Cargo.toml + update the
command examples to send patches so they CC the current maintainer.

Signed-off-by: Sahid Orentino Ferdjaoui 
---
 .gitpublish |  3 ++-
 Cargo.toml  |  2 +-
 README.md   | 12 +++-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/.gitpublish b/.gitpublish
index f7a938f..b42ee56 100644
--- a/.gitpublish
+++ b/.gitpublish
@@ -1,4 +1,5 @@
 [gitpublishprofile "default"]
 base = master
 to = libvir-list@redhat.com
-prefix = rust PATCH
+cc = sahid.ferdja...@canonical.com
+prefix = PATCH Rust
diff --git a/Cargo.toml b/Cargo.toml
index 25538f0..2136aaf 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "virt"
 version = "0.2.10"
-authors = ["Sahid Orentino Ferdjaoui ",]
+authors = ["Sahid Orentino Ferdjaoui ",]
 license = "LGPL-2.1"
 readme = "README.md"
 description = "Rust bindings to the libvirt C library"
diff --git a/README.md b/README.md
index 6e1ad64..93cd461 100644
--- a/README.md
+++ b/README.md
@@ -77,18 +77,20 @@ to submit patches to the libvir-list@redhat.com mailing 
list. eg. to
 send a single patch
 
 ```
-   git send-email --to libvir-list@redhat.com --subject-prefix "PATCH rust" \
-   --smtp-server=$HOSTNAME -1
+   git send-email --to libvir-list@redhat.com --cc 
sahid.ferdja...@canonical.com \
+   --subject-prefix "PATCH Rust" --smtp-server=$HOSTNAME -1
 ```
 
 Or to send all patches on the current branch, against master
 
 ```
-   git send-email --to libvir-list@redhat.com --subject-prefix "PATCH rust" \
-   --smtp-server=$HOSTNAME --no-chain-reply-to --cover-letter --annotate \
-   master..
+   git send-email --to libvir-list@redhat.com --cc 
sahid.ferdja...@canonical.com \
+   --subject-prefix "PATCH Rust" --smtp-server=$HOSTNAME 
--no-chain-reply-to \
+   --cover-letter --annotate master..
 ```
 
+It is also possible to use git-publish.
+
 Note the master GIT repository is at
 
 * https://libvirt.org/git/?p=libvirt-rust.git;a=summary
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH Rust v2 5/6] fix code formating in README

2019-07-09 Thread Sahid Orentino Ferdjaoui
Signed-off-by: Sahid Orentino Ferdjaoui 
---
 README.md | 4 
 1 file changed, 4 insertions(+)

diff --git a/README.md b/README.md
index a359574..6e1ad64 100644
--- a/README.md
+++ b/README.md
@@ -76,14 +76,18 @@ at any time. The preferred submission method is to use git 
send-email
 to submit patches to the libvir-list@redhat.com mailing list. eg. to
 send a single patch
 
+```
git send-email --to libvir-list@redhat.com --subject-prefix "PATCH rust" \
--smtp-server=$HOSTNAME -1
+```
 
 Or to send all patches on the current branch, against master
 
+```
git send-email --to libvir-list@redhat.com --subject-prefix "PATCH rust" \
--smtp-server=$HOSTNAME --no-chain-reply-to --cover-letter --annotate \
master..
+```
 
 Note the master GIT repository is at
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH Rust v2 1/6] ci workaround when running integration tests

2019-07-09 Thread Sahid Orentino Ferdjaoui
Due to an issue fixed in libvirt master we should not consider running
integration tests in parallel.

https://www.redhat.com/archives/libvir-list/2019-July/msg00287.html

Signed-off-by: Sahid Orentino Ferdjaoui 
---
 .travis.yml | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index c52f745..54f3c24 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -41,4 +41,7 @@ install:
 
 script:
   - cargo test --verbose
-  - cargo test --verbose -- --ignored
+  # Due to an issue fixed in master we should not consider running
+  # integration tests in parallel.
+  # see: https://www.redhat.com/archives/libvir-list/2019-July/msg00287.html
+  - cargo test --verbose -- --ignored --test-threads=1
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH Rust v2 3/6] update tested versions from 2.5.0 to 5.5.0

2019-07-09 Thread Sahid Orentino Ferdjaoui
Signed-off-by: Sahid Orentino Ferdjaoui 
---
 .travis.yml | 7 +++
 README.md   | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 54f3c24..ebefca8 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -13,11 +13,10 @@ matrix:
 - rust: nightly
 
 env:
-  - LIBVIRT=1.2.0  EXT=gz
-  - LIBVIRT=1.2.10 EXT=gz
-  - LIBVIRT=1.2.20 EXT=gz
   - LIBVIRT=2.5.0  EXT=xz
-  - LIBVIRT=3.3.0  EXT=xz
+  - LIBVIRT=3.10.0  EXT=xz
+  - LIBVIRT=4.10.0  EXT=xz
+  - LIBVIRT=5.5.0  EXT=xz
 
 install:
   - sudo apt-get -qqy build-dep libvirt
diff --git a/README.md b/README.md
index 5643f74..a359574 100644
--- a/README.md
+++ b/README.md
@@ -21,7 +21,7 @@ The bindings use standard errors handling from Rust. Each 
method
 
 ## Tests/Exercises
 
-CI is executing tests automatically from libvirt 1.2.0 to 3.3.0. Using
+CI is executing tests automatically from libvirt 2.5.0 to 5.5.0. Using
 Rust from stable, beta to nightly.
 
 * https://travis-ci.org/libvirt/libvirt-rust
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH Rust v2 4/6] switch ci tests to ubuntu bionic

2019-07-09 Thread Sahid Orentino Ferdjaoui
Also configure default mech used by sasl to 'digest-md5' since
'scram-sha-1' requires an encryption layer.

Signed-off-by: Sahid Orentino Ferdjaoui 
---
 .travis.yml | 3 ++-
 tests/libvirtd.sasl | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)
 create mode 100644 tests/libvirtd.sasl

diff --git a/.travis.yml b/.travis.yml
index ebefca8..fc2c8f4 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,6 +1,6 @@
 language: rust
 os: linux
-dist: trusty
+dist: bionic
 sudo: require
 
 rust:
@@ -33,6 +33,7 @@ install:
   - make
   - sudo make install
   - popd
+  - sudo cp tests/libvirtd.sasl /etc/sasl2/libvirt.conf
   - sudo libvirtd -d -l -f tests/libvirtd.conf
   - sudo virtlogd -d || true
   - sudo chmod a+rwx /var/run/libvirt/libvirt-sock*
diff --git a/tests/libvirtd.sasl b/tests/libvirtd.sasl
new file mode 100644
index 000..ab609e0
--- /dev/null
+++ b/tests/libvirtd.sasl
@@ -0,0 +1,2 @@
+mech_list: digest-md5
+sasldb_path: /etc/libvirt/passwd.db
\ No newline at end of file
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH Rust v2 2/6] make lookup_by_id() test more robust

2019-07-09 Thread Sahid Orentino Ferdjaoui
Signed-off-by: Sahid Orentino Ferdjaoui 
---
 tests/domain.rs | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tests/domain.rs b/tests/domain.rs
index 5a64a75..dcf7dd9 100644
--- a/tests/domain.rs
+++ b/tests/domain.rs
@@ -91,14 +91,13 @@ fn test_get_vcpus_flags() {
 #[test]
 fn test_lookup_domain_by_id() {
 let c = common::conn();
-let v = c.list_domains().unwrap_or(vec![]);
-assert!(0 < v.len(), "At least one domain should exist");
-for domid in v {
-match Domain::lookup_by_id(, domid) {
-Ok(mut dom) => dom.free().unwrap_or(()),
-Err(e) => panic!("failed with code {}, message: {}", e.code, 
e.message),
-}
+let d = common::build_test_domain(, "by_id", true);
+let id = d.get_id().unwrap_or(0);
+match Domain::lookup_by_id(, id) {
+Ok(mut r) => r.free().unwrap_or(()),
+Err(e) => panic!("failed with code {}, message: {}", e.code, 
e.message),
 }
+common::clean(d);
 common::close(c);
 }
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH Rust v2 6/6] bump version to 0.2.10

2019-07-09 Thread Sahid Orentino Ferdjaoui
We passed the 0.2.9 since that one has been sent to crates.io but the
code was never merged. To avoid that in future, new version will be
sent to crates.io only when accepted by upstream.

Signed-off-by: Sahid Orentino Ferdjaoui 
---
 Cargo.toml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Cargo.toml b/Cargo.toml
index d75693f..25538f0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "virt"
-version = "0.2.8"
+version = "0.2.10"
 authors = ["Sahid Orentino Ferdjaoui ",]
 license = "LGPL-2.1"
 readme = "README.md"
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH] rpc: ensure thread safe initialization of SASL library

2019-07-08 Thread Sahid Orentino Ferdjaoui
On Mon, Jul 08, 2019 at 01:04:59PM +0100, Daniel P. Berrangé wrote:
> On Mon, Jul 08, 2019 at 01:12:06PM +0200, Michal Privoznik wrote:
> > On 7/8/19 12:39 PM, Daniel P. Berrangé wrote:
> > > Neither the sasl_client_init or sasl_server_init methods are even
> > > remotely threadsafe. They do a bunch of one-time initialization and
> > > merely use a simple integer counter to avoid repeated work, not even
> > > using atomic increment/reads on the counter. This can easily race in a
> > > threaded program. Protect the calls using a virOnce initializer function
> > > which is guaranteed threadsafe at least from libvirt's POV.
> > > 
> > > If the application using libvirt also uses another library that makes
> > > use of SASL then the race still exists. It is impossible to fix that
> > > fully except in SASL code itself.
> > > 
> > > Signed-off-by: Daniel P. Berrangé 
> > > ---
> > >   src/rpc/virnetsaslcontext.c | 50 -
> > >   1 file changed, 33 insertions(+), 17 deletions(-)
> > 
> > Reviewed-by: Michal Privoznik 
> > 
> > Thanks Sahid for the report!
> 
> FYI i wrote a simple demo program that can reliably reproduce the problem
> in isolation

Also tested in my side, the patch well fixed the issue.

Thanks Daniel and Michal.

s.

> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

> #include 
> #include 
> #include 
> #include 
> 
> 
> pthread_cond_t condReady;
> pthread_cond_t condRun;
> pthread_mutex_t lock;
> int running;
> 
> void *runme(void *arg)
> {
>   virConnectPtr conn;
> 
>   pthread_mutex_lock();
>   running++;
>   fprintf(stderr, "Notifying we are ready\n");
>   pthread_cond_signal();
>   pthread_cond_wait(, );
>   pthread_mutex_unlock();
>   fprintf(stderr, "Opening libvirt\n");
>   conn = virConnectOpenAuth("qemu:///system", virConnectAuthPtrDefault, 0);
>   fprintf(stderr, "Open %s\n", conn ? virConnectGetURI(conn) : "failed");
>   if (conn)
> virConnectClose(conn);
> }
> 
> int main(int argc, char **argv)
> {
>   int i;
>   pthread_t th[20];
> 
>   pthread_mutex_init(, NULL);
>   pthread_cond_init(, NULL);
>   pthread_cond_init(, NULL);
> 
>   for (i = 0; i < 20; i++) {
> pthread_create([i], NULL, runme, NULL);
>   }
> 
>   fprintf(stderr, "Waiting for threads to initialize\n");
>   pthread_mutex_lock();
>   while (running != 20)
> pthread_cond_wait(, );
>   fprintf(stderr, "Telling threads to proceed\n");
>   pthread_cond_broadcast();
>   pthread_mutex_unlock();
> 
>   for (i = 0; i < 20; i++) {
> pthread_join(th[i], NULL);
>   }
>   return 0;
> }

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH rust 4/5] switch to the last ubuntu lts bionic

2019-07-05 Thread Sahid Orentino Ferdjaoui
On Fri, Jul 05, 2019 at 09:55:37AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 04, 2019 at 12:15:28PM +0200, Sahid Orentino Ferdjaoui wrote:
> > Not sure what the problem is by using 'scram-sha-1' with ubuntu:
> > 
> > cannot list SASL mechanisms -4 (SASL(-4): no mechanism available:
> > Internal Error -4 in ../../lib/server.c near line 1762)
> 
> For some strange reason Debian decided to put the scram plugin
> in the libsasl2-modules-gssapi-mit  package so i expect you're
> missing that in the test env.

I tried but that does not work either. Also with
libsasl2-modules-gssapi-heimda.

> > So we currently switch the mech to digest-md5. Seems that libvirt-go
> > is doing same.
> 
> That's a historical accident, not good practice !

I reported the issue in ubuntu [0] and it looks like it's actually
libvirt which does not support it without TLS.

So I imagine digest-md5 is fine for the purpose of testing.

Perhaps you can comment.

Thanks,
s.

[0] https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1835427

> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH rust 2/5] make lookup_by_id() test more robust

2019-07-05 Thread Sahid Orentino Ferdjaoui
On Fri, Jul 05, 2019 at 09:49:02AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 04, 2019 at 12:15:26PM +0200, Sahid Orentino Ferdjaoui wrote:
> > Signed-off-by: Sahid Orentino Ferdjaoui 
> > ---
> >  tests/domain.rs | 19 +--
> >  1 file changed, 9 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tests/domain.rs b/tests/domain.rs
> > index 5a64a75..a70139e 100644
> > --- a/tests/domain.rs
> > +++ b/tests/domain.rs
> > @@ -89,26 +89,25 @@ fn test_get_vcpus_flags() {
> >  }
> >  
> >  #[test]
> > -fn test_lookup_domain_by_id() {
> > +fn test_lookup_domain_by_name() {
> >  let c = common::conn();
> > -let v = c.list_domains().unwrap_or(vec![]);
> > -assert!(0 < v.len(), "At least one domain should exist");
> > -for domid in v {
> > -match Domain::lookup_by_id(, domid) {
> > -Ok(mut dom) => dom.free().unwrap_or(()),
> > -Err(e) => panic!("failed with code {}, message: {}", e.code, 
> > e.message),
> > -}
> > +match Domain::lookup_by_name(, "test") {
> > +Ok(mut r) => r.free().unwrap_or(()),
> > +Err(e) => panic!("failed with code {}, message: {}", e.code, 
> > e.message),
> >  }
> >  common::close(c);
> >  }
> >  
> >  #[test]
> > -fn test_lookup_domain_by_name() {
> > +fn test_lookup_domain_by_id() {
> >  let c = common::conn();
> > -match Domain::lookup_by_name(, "test") {
> > +let d = common::build_test_domain(, "by_id", true);
> > +let id = d.get_id().unwrap_or(0);
> > +match Domain::lookup_by_id(, id) {
> >  Ok(mut r) => r.free().unwrap_or(()),
> >  Err(e) => panic!("failed with code {}, message: {}", e.code, 
> > e.message),
> >  }
> > +common::clean(d);
> >  common::close(c);
> >  }
> 
> Why did you change the order of the two test methods ? It has created a
> big diff that hides what is actually being changed. Can you put them back
> to the original order, so we just see the relevant change.

No ideas I will rearrange that.

> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH rust 1/5] fix bug integration test with rust multithreading

2019-07-05 Thread Sahid Orentino Ferdjaoui
On Fri, Jul 05, 2019 at 09:47:14AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 04, 2019 at 12:15:25PM +0200, Sahid Orentino Ferdjaoui wrote:
> > The open_auth test cases were randomly failling. It seems that because
> > tests are executed in parallel if a sasl connection is open when an
> > other happen in same time from the same libvirt connection an error
> > happens: "Failed to start SASL negotiation: -4 (SASL(-4): no mechanism
> > available: No worthy mechs found)".
> 
> This doesn't really make sense. Libvirt itself is fully multi-threaded
> so it should be fine to be opening connections in parallel.
> 
> If there's a non-deterministic failure happening this suggests a race
> condition bug somewhere, which shouldn't just be ignored by changing
> the tests to avoid hitting the race,

You are completely right I did lot of tests and I was not able to find
the root cause. I was only able to reproduce the problem when I'm
using the test framework and even with that if I'm using
test-threads=1, no problems, or if I'm executing only one of them
several times, no problems. Even, running 2 of example/auth.rs in
parallel lot of time does not cause any issues.

That is the leak which sometime happens:

==20734==
==20734== HEAP SUMMARY:
==20734== in use at exit: 165,843 bytes in 852 blocks
==20734==   total heap usage: 4,141 allocs, 3,289 frees, 1,923,649 bytes 
allocated
==20734==
==20734== 544 bytes in 1 blocks are definitely lost in loss record 103 of 121
==20734==at 0x4C2CE8E: realloc (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==20734==by 0xA780E5E: _plug_buf_alloc (in 
/usr/lib/x86_64-linux-gnu/sasl2/libdigestmd5.so.2.0.25)
==20734==by 0xA77A53D: ??? (in 
/usr/lib/x86_64-linux-gnu/sasl2/libdigestmd5.so.2.0.25)
==20734==by 0xA77F017: ??? (in 
/usr/lib/x86_64-linux-gnu/sasl2/libdigestmd5.so.2.0.25)
==20734==by 0xA77F8B0: ??? (in 
/usr/lib/x86_64-linux-gnu/sasl2/libdigestmd5.so.2.0.25)
==20734==by 0x7DF85EF: sasl_client_step (in 
/usr/lib/x86_64-linux-gnu/libsasl2.so.2.0.25)
==20734==by 0x7DF896D: sasl_client_start (in 
/usr/lib/x86_64-linux-gnu/libsasl2.so.2.0.25)
==20734==by 0x4FF0756: virNetSASLSessionClientStart (in 
/usr/lib/libvirt.so.0.1002.2)
==20734==by 0x4FD1AD4: ??? (in /usr/lib/libvirt.so.0.1002.2)
==20734==by 0x4FD27E5: ??? (in /usr/lib/libvirt.so.0.1002.2)
==20734==by 0x4F6C4E5: ??? (in /usr/lib/libvirt.so.0.1002.2)
==20734==by 0x4F6F00C: virConnectOpenAuth (in /usr/lib/libvirt.so.0.1002.2)
==20734==
==20734== LEAK SUMMARY:
==20734==definitely lost: 544 bytes in 1 blocks
==20734==indirectly lost: 0 bytes in 0 blocks
==20734==  possibly lost: 0 bytes in 0 blocks
==20734==still reachable: 165,299 bytes in 851 blocks
==20734== suppressed: 0 bytes in 0 blocks
==20734== Reachable blocks (those to which a pointer was found) are not shown.
==20734== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==20734==
==20734== For counts of detected and suppressed errors, rerun with: -v
==20734== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

I also have no clues why the heap is not completely freed. I will try
to investigate a bit more.

Thanks,
s.

> > 
> > Signed-off-by: Sahid Orentino Ferdjaoui 
> > ---
> >  tests/integration_qemu.rs | 15 +++
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/integration_qemu.rs b/tests/integration_qemu.rs
> > index 49e07c4..7db43d5 100644
> > --- a/tests/integration_qemu.rs
> > +++ b/tests/integration_qemu.rs
> > @@ -89,6 +89,16 @@ fn test_create_storage_pool_and_vols() {
> >  #[test]
> >  #[ignore]
> >  fn test_connection_with_auth() {
> > +// Rust is excecuting tests in parallel (threads), if a sasl
> > +// connection is open when an other happen in same time from the
> > +// same libvirt connection an error happens: "Failed to start SASL
> > +// negotiation: -4 (SASL(-4): no mechanism available: No worthy
> > +// mechs found)".
> > +connection_with_auth_ok();
> > +connection_with_auth_wrong();
> > +}
> > +
> > +fn connection_with_auth_ok() {
> >  fn callback(creds:  Vec) {
> >  for cred in creds {
> >  match cred.typed {
> > @@ -118,10 +128,7 @@ fn test_connection_with_auth() {
> >  }
> >  }
> >  
> > -
> > -#[test]
> > -#[ignore]
> > -fn test_connection_with_auth_wrong() {
> > +fn connection_with_auth_wrong() {
> >  fn callback(creds:  Vec) {
> >  for cred in creds {
> >  match cred.typed {
> > -- 
> > 2.20.1
> > 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH rust 5/5] fix code formating in README

2019-07-04 Thread Sahid Orentino Ferdjaoui
Signed-off-by: Sahid Orentino Ferdjaoui 
---
 README.md | 4 
 1 file changed, 4 insertions(+)

diff --git a/README.md b/README.md
index a359574..6e1ad64 100644
--- a/README.md
+++ b/README.md
@@ -76,14 +76,18 @@ at any time. The preferred submission method is to use git 
send-email
 to submit patches to the libvir-list@redhat.com mailing list. eg. to
 send a single patch
 
+```
git send-email --to libvir-list@redhat.com --subject-prefix "PATCH rust" \
--smtp-server=$HOSTNAME -1
+```
 
 Or to send all patches on the current branch, against master
 
+```
git send-email --to libvir-list@redhat.com --subject-prefix "PATCH rust" \
--smtp-server=$HOSTNAME --no-chain-reply-to --cover-letter --annotate \
master..
+```
 
 Note the master GIT repository is at
 
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH rust 1/5] fix bug integration test with rust multithreading

2019-07-04 Thread Sahid Orentino Ferdjaoui
The open_auth test cases were randomly failling. It seems that because
tests are executed in parallel if a sasl connection is open when an
other happen in same time from the same libvirt connection an error
happens: "Failed to start SASL negotiation: -4 (SASL(-4): no mechanism
available: No worthy mechs found)".

Signed-off-by: Sahid Orentino Ferdjaoui 
---
 tests/integration_qemu.rs | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tests/integration_qemu.rs b/tests/integration_qemu.rs
index 49e07c4..7db43d5 100644
--- a/tests/integration_qemu.rs
+++ b/tests/integration_qemu.rs
@@ -89,6 +89,16 @@ fn test_create_storage_pool_and_vols() {
 #[test]
 #[ignore]
 fn test_connection_with_auth() {
+// Rust is excecuting tests in parallel (threads), if a sasl
+// connection is open when an other happen in same time from the
+// same libvirt connection an error happens: "Failed to start SASL
+// negotiation: -4 (SASL(-4): no mechanism available: No worthy
+// mechs found)".
+connection_with_auth_ok();
+connection_with_auth_wrong();
+}
+
+fn connection_with_auth_ok() {
 fn callback(creds:  Vec) {
 for cred in creds {
 match cred.typed {
@@ -118,10 +128,7 @@ fn test_connection_with_auth() {
 }
 }
 
-
-#[test]
-#[ignore]
-fn test_connection_with_auth_wrong() {
+fn connection_with_auth_wrong() {
 fn callback(creds:  Vec) {
 for cred in creds {
 match cred.typed {
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH rust 4/5] switch to the last ubuntu lts bionic

2019-07-04 Thread Sahid Orentino Ferdjaoui
Not sure what the problem is by using 'scram-sha-1' with ubuntu:

cannot list SASL mechanisms -4 (SASL(-4): no mechanism available:
Internal Error -4 in ../../lib/server.c near line 1762)

So we currently switch the mech to digest-md5. Seems that libvirt-go
is doing same.

Signed-off-by: Sahid Orentino Ferdjaoui 
---
 .travis.yml | 3 ++-
 tests/libvirtd.sasl | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)
 create mode 100644 tests/libvirtd.sasl

diff --git a/.travis.yml b/.travis.yml
index 2267a8f..bea4496 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,6 +1,6 @@
 language: rust
 os: linux
-dist: trusty
+dist: bionic
 sudo: require
 
 rust:
@@ -33,6 +33,7 @@ install:
   - make
   - sudo make install
   - popd
+  - sudo cp tests/libvirtd.sasl /etc/sasl2/libvirt.conf
   - sudo libvirtd -d -l -f tests/libvirtd.conf
   - sudo virtlogd -d || true
   - sudo chmod a+rwx /var/run/libvirt/libvirt-sock*
diff --git a/tests/libvirtd.sasl b/tests/libvirtd.sasl
new file mode 100644
index 000..ab609e0
--- /dev/null
+++ b/tests/libvirtd.sasl
@@ -0,0 +1,2 @@
+mech_list: digest-md5
+sasldb_path: /etc/libvirt/passwd.db
\ No newline at end of file
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH rust 2/5] make lookup_by_id() test more robust

2019-07-04 Thread Sahid Orentino Ferdjaoui
Signed-off-by: Sahid Orentino Ferdjaoui 
---
 tests/domain.rs | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/tests/domain.rs b/tests/domain.rs
index 5a64a75..a70139e 100644
--- a/tests/domain.rs
+++ b/tests/domain.rs
@@ -89,26 +89,25 @@ fn test_get_vcpus_flags() {
 }
 
 #[test]
-fn test_lookup_domain_by_id() {
+fn test_lookup_domain_by_name() {
 let c = common::conn();
-let v = c.list_domains().unwrap_or(vec![]);
-assert!(0 < v.len(), "At least one domain should exist");
-for domid in v {
-match Domain::lookup_by_id(, domid) {
-Ok(mut dom) => dom.free().unwrap_or(()),
-Err(e) => panic!("failed with code {}, message: {}", e.code, 
e.message),
-}
+match Domain::lookup_by_name(, "test") {
+Ok(mut r) => r.free().unwrap_or(()),
+Err(e) => panic!("failed with code {}, message: {}", e.code, 
e.message),
 }
 common::close(c);
 }
 
 #[test]
-fn test_lookup_domain_by_name() {
+fn test_lookup_domain_by_id() {
 let c = common::conn();
-match Domain::lookup_by_name(, "test") {
+let d = common::build_test_domain(, "by_id", true);
+let id = d.get_id().unwrap_or(0);
+match Domain::lookup_by_id(, id) {
 Ok(mut r) => r.free().unwrap_or(()),
 Err(e) => panic!("failed with code {}, message: {}", e.code, 
e.message),
 }
+common::clean(d);
 common::close(c);
 }
 
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH rust 3/5] update tested versions from 2.5.0 to 5.5.0

2019-07-04 Thread Sahid Orentino Ferdjaoui
Signed-off-by: Sahid Orentino Ferdjaoui 
---
 .travis.yml | 7 +++
 README.md   | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index c52f745..2267a8f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -13,11 +13,10 @@ matrix:
 - rust: nightly
 
 env:
-  - LIBVIRT=1.2.0  EXT=gz
-  - LIBVIRT=1.2.10 EXT=gz
-  - LIBVIRT=1.2.20 EXT=gz
   - LIBVIRT=2.5.0  EXT=xz
-  - LIBVIRT=3.3.0  EXT=xz
+  - LIBVIRT=3.10.0  EXT=xz
+  - LIBVIRT=4.10.0  EXT=xz
+  - LIBVIRT=5.5.0  EXT=xz
 
 install:
   - sudo apt-get -qqy build-dep libvirt
diff --git a/README.md b/README.md
index 5643f74..a359574 100644
--- a/README.md
+++ b/README.md
@@ -21,7 +21,7 @@ The bindings use standard errors handling from Rust. Each 
method
 
 ## Tests/Exercises
 
-CI is executing tests automatically from libvirt 1.2.0 to 3.3.0. Using
+CI is executing tests automatically from libvirt 2.5.0 to 5.5.0. Using
 Rust from stable, beta to nightly.
 
 * https://travis-ci.org/libvirt/libvirt-rust
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH rust 0/5] ci related plus some small updates

2019-07-04 Thread Sahid Orentino Ferdjaoui
Hello Daniel,

When you have a moment, can you please merge this serie. It fixes CI
issue, switch to bionic, and updates the tested versions.

Results:
 https://travis-ci.org/sahid/libvirt-rust/builds/554170698

Thanks,
s.

Sahid Orentino Ferdjaoui (5):
  fix bug integration test with rust multithreading
  make lookup_by_id() test more robust
  update tested versions from 2.5.0 to 5.5.0
  switch to the last ubuntu lts bionic
  fix code formating in README

 .travis.yml   | 10 +-
 README.md |  6 +-
 tests/domain.rs   | 19 +--
 tests/integration_qemu.rs | 15 +++
 tests/libvirtd.sasl   |  2 ++
 5 files changed, 32 insertions(+), 20 deletions(-)
 create mode 100644 tests/libvirtd.sasl

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [rust PATCH] travis: fix syntax for script commands

2018-04-24 Thread Sahid Orentino Ferdjaoui
On Tue, Apr 24, 2018 at 01:33:20PM +0100, Daniel P. Berrangé wrote:
> The script: commands were listed without a leading '-' which caused
> travis to concatenate them into a single command. This meant the second
> command became a set of arguments to the first command. Historically
> cargo ignored these extra args so the mistake was not noticed, but it
> now generates a fatal error.
>

Thanks for this Daniel. Do you want me to merge it or are you going to
take care of that too?

Reviewed-by: Sahid Orentino Ferdjaoui <sahid.ferdja...@redhat.com>

> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> ---
>  .travis.yml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index f9844bb..c52f745 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -40,5 +40,5 @@ install:
>- echo "pass" | sudo saslpasswd2 -p -a libvirt user
>  
>  script:
> -  cargo test --verbose
> -  cargo test --verbose -- --ignored
> +  - cargo test --verbose
> +  - cargo test --verbose -- --ignored
> -- 
> 2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 3/3] qemu: add busy polling support

2017-07-17 Thread Sahid Orentino Ferdjaoui
On Sat, Jun 17, 2017 at 12:30:29PM -0400, Laine Stump wrote:
> On 06/15/2017 10:17 AM, sferd...@redhat.com wrote:
> > From: Sahid Orentino Ferdjaoui <sahid.ferdja...@redhat.com>
> > 
> > This patch finalizes support of 'poll_us' attribute as an option of
> > the NIC driver-specific element, the commit also takes care of
> > hotplug.
> > 
> > 
> >   
> > 
> > 
> > 
> >   
> > 
> > 
> > That option is supported for all networks which are using a tap
> > device.
> > 
> > To be used the backend should be specificly set to use vhost.
> 
> Actually, libvirt's qemu driver will use vhost-net automatically as long
> as the qemu binary supports it (unless 1) the domain is using TCG
> instead of KVM, or 2) the interface configuration specifically says
> " >  driver name='vhost' txmode='iothread' ioeventfd='on' 
> > event_idx='off' queues='5' rx_queue_size='256'
> >host csum='off' gso='off' tso4='off' tso6='off' ecn='off' 
> > ufo='off' mrg_rxbuf='off'/
> > -  guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off'/
> > +  guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off' 
> > poll_us='50'/
> >  /driver
> >  
> >/interface
> > @@ -5171,6 +5171,16 @@ qemu-kvm -net nic,model=? /dev/null
> >  In general you should leave this option alone, unless you
> >  are very certain you know what you are doing.
> >
> > +  
> > +The optional poll_us attribute configure the
> > +maximum number of microseconds that could be spent on busy
> > +polling. It improves the performance, but requires more
> > +CPU. This option is only available with vhost backend driver.
> > +Since 2.7.0 (QEMU and KVM 
> > only)
> > +
> > +In general you should leave this option alone, unless you
> > +are very certain you know what you are doing.
> > +  
> >virtio options
> >
> >  For virtio interfaces,
> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > index 4950ddc..f304385 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -2687,6 +2687,11 @@
> >
> >  
> >
> > +  
> > +
> > +  
> > +
> > +  
> >  
> >
> >
> 
> As mentioned in the previous patch, the RNG and docs changes should be
> in the previous patch. (In this patch you're going to want to add an
> entry to docs/news.xml)
> 
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 8c12b2b..412496e 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -3955,6 +3955,9 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
> >  }
> >  if (net->tune.sndbuf_specified)
> >  virBufferAsprintf(, "sndbuf=%lu,", net->tune.sndbuf);
> > +if (net->driver.virtio.pollus)
> > +virBufferAsprintf(, "poll-us=%u,",
> > +  net->driver.virtio.pollus);
> >  }
> >  
> >  virObjectUnref(cfg);
> > @@ -8451,6 +8454,25 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr 
> > driver,
> >  return -1;
> >  }
> >  
> > +if (net->driver.virtio.pollus > 0) {
> > +if (net->driver.virtio.name != VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) {
> 
> I haven't checked - does this get set automatically when the vhost-net
> FD's are successfully opened?
> 
> > +virReportError(
> > +VIR_ERR_CONFIG_UNSUPPORTED,
> > +_("Busy polling is only supported with vhost backend 
> > driver"));
> 
> I wonder if anyone would be confused by the lack of the exact option
> name "poll_us" in these error messages.
> 
> > +return -1;
> > +}
> > +/* Nothing besides TAP devices supports busy polling. */
> > +if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> > +  actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> > +  actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
> > +  actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +   _("Busy polli

Re: [libvirt] libvirt binding for Rust

2017-06-07 Thread Sahid Orentino Ferdjaoui
On Wed, Jun 07, 2017 at 11:14:11AM +0200, Martin Kletzander wrote:
> On Tue, Jun 06, 2017 at 12:19:21PM +0200, Sahid Orentino Ferdjaoui wrote:
> > Hello,
> > 
> > I started a work on a libvirt binding for Rust [0]. Not all of the API
> > is implemeted but I think it's now in a usable state.
> > 
> >  https://docs.rs/crate/virt
> >  https://github.com/sahid/libvirt-rs
> > 
> > The code is licensed under LGPL-2.1, it's tested by a CI running
> > libvirt 1.2.0, 2.5.0, 3.3.0. There are unit tests, integration tests
> > and some examples to exercise the code (some parts are still not
> > covered) I also have checked the code with valgrind to avoid any
> > memory leaks.
> > 
> >  https://travis-ci.org/sahid/libvirt-rs
> > 
> 
> This is nice, I am thinking of trying to add a module support for
> modules written in Rust, but that's just a fun project.
> 
> Anyway, if I may give you an advice, it would be way nicer if some of
> the bindings were automatically generated.  Libvirt provides all the
> infomartion necessary in libvirt{,-qemu-lxc}.xml files for you to be
> able to do this.  We already do this for the python bindings, for
> example.  That way they automatically support trivial function additions
> in newer libvirt versions.

It's certainly something we can do with a tool like [0], and you can
see an example of the API extracted here [1]. But the point of that
work is to provide for users a way to avoid to interact with unsafe
code.

[0] https://github.com/servo/rust-bindgen
[1] https://docs.rs/libvirt-sys/1.2.18/libvirt_sys/

> > Do you think that we can consider it ready to be hosted by libvirt.org
> > GIT server ? That could help to get new contributors also interested
> > by Rust and improve the code and coverage of the API.
> > 
> > Thanks,
> > s.
> > 
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] libvirt binding for Rust

2017-06-06 Thread Sahid Orentino Ferdjaoui
Hello,

I started a work on a libvirt binding for Rust [0]. Not all of the API
is implemeted but I think it's now in a usable state.

  https://docs.rs/crate/virt
  https://github.com/sahid/libvirt-rs

The code is licensed under LGPL-2.1, it's tested by a CI running
libvirt 1.2.0, 2.5.0, 3.3.0. There are unit tests, integration tests
and some examples to exercise the code (some parts are still not
covered) I also have checked the code with valgrind to avoid any
memory leaks.

  https://travis-ci.org/sahid/libvirt-rs

Do you think that we can consider it ready to be hosted by libvirt.org
GIT server ? That could help to get new contributors also interested
by Rust and improve the code and coverage of the API.

Thanks,
s.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 0/2] Add support of SASL authentication for QEMU migration

2014-04-14 Thread Sahid Orentino Ferdjaoui
This is a first contribution, I have tried to follow the most as possible rules
marked in HACKING. I hope this commit will be conform with the specifications.

  make check  OK
  make syntax-check   OK
  make -C tests valgrind  OK

Currently with peer to peer migration provided by virDomainMigrateToURI, QEMU
migration code uses virConnectOpen() which means that all authentication
callbacks are disabled. Since no auth callback is present, SASL doesn't find
any mechanisms and thus auth fails with the error:

  authentication failed: Failed to start SASL negotiation: -4 (SASL(-4):
   no mechanism available: No worthy mechs found)

The PATCH 1/2 adds a new example to illustrate how to use peer to peer
migration. This patch is not necessary to fix the problem and can be removed.
It is provided to help reviewers by avoiding the necessary to create code
that use this feature. Also as it demonstrates the performance of libvirt I have
thought it could be interesting to keep it for new users.

The PATCH 2/2 fixes the problem by configuring QEMU migration code to
use virConnectOpenAuth instead of virConnectOpen. Indeed this function will call
if necessary a callback responsible to fetching credentials.

Sahid Orentino Ferdjaoui (2):
  Add a new example to illustrate domain migration
  Add support for QEMU migration to use SASL authentication

 .gitignore   |  1 +
 Makefile.am  |  2 +-
 configure.ac |  1 +
 examples/dommigrate/Makefile.am  | 26 ++
 examples/dommigrate/dommigrate.c | 78 
 libvirt.spec.in  |  3 +-
 src/qemu/qemu_migration.c| 14 +++-
 7 files changed, 122 insertions(+), 3 deletions(-)
 create mode 100644 examples/dommigrate/Makefile.am
 create mode 100644 examples/dommigrate/dommigrate.c

-- 
1.9.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 1/2] Add a new example to illustrate domain migration

2014-04-14 Thread Sahid Orentino Ferdjaoui
This commit adds a new example to illustrate peer to
peer domain migration with virDomainMigrateToURI.

Signed-off-by: Sahid Orentino Ferdjaoui sahid.ferdja...@cloudwatt.com
---
 .gitignore   |  1 +
 Makefile.am  |  2 +-
 configure.ac |  1 +
 examples/dommigrate/Makefile.am  | 26 ++
 examples/dommigrate/dommigrate.c | 78 
 libvirt.spec.in  |  3 +-
 6 files changed, 109 insertions(+), 2 deletions(-)
 create mode 100644 examples/dommigrate/Makefile.am
 create mode 100644 examples/dommigrate/dommigrate.c

diff --git a/.gitignore b/.gitignore
index 0513a33..8c3b870 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@
 /examples/object-events/event-test
 /examples/dominfo/info1
 /examples/domsuspend/suspend
+/examples/dommigrate/dommigrate
 /examples/hellolibvirt/hellolibvirt
 /examples/openauth/openauth
 /gnulib/lib/*
diff --git a/Makefile.am b/Makefile.am
index 9847ff0..b961c0e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -23,7 +23,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs 
gnulib/tests \
   tests po examples/object-events examples/hellolibvirt \
   examples/dominfo examples/domsuspend examples/apparmor \
   examples/xml/nwfilter examples/openauth examples/systemtap \
-  tools/wireshark
+  tools/wireshark examples/dommigrate
 
 ACLOCAL_AMFLAGS = -I m4
 
diff --git a/configure.ac b/configure.ac
index ea85851..e461001 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2724,6 +2724,7 @@ AC_CONFIG_FILES([\
 examples/object-events/Makefile \
 examples/domsuspend/Makefile \
 examples/dominfo/Makefile \
+examples/dommigrate/Makefile \
 examples/openauth/Makefile \
 examples/hellolibvirt/Makefile \
 examples/systemtap/Makefile \
diff --git a/examples/dommigrate/Makefile.am b/examples/dommigrate/Makefile.am
new file mode 100644
index 000..43b55fc
--- /dev/null
+++ b/examples/dommigrate/Makefile.am
@@ -0,0 +1,26 @@
+## Copyright (C) 2014 Cloudwatt
+## Copyright (C) 2005-2013 Red Hat, Inc.
+##
+## This library is free software; you can redistribute it and/or
+## modify it under the terms of the GNU Lesser General Public
+## License as published by the Free Software Foundation; either
+## version 2.1 of the License, or (at your option) any later version.
+##
+## This library is distributed in the hope that it will be useful,
+## but WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+## Lesser General Public License for more details.
+##
+## You should have received a copy of the GNU Lesser General Public
+## License along with this library.  If not, see
+## http://www.gnu.org/licenses/.
+
+INCLUDES = \
+   -I$(top_builddir)/include -I$(top_srcdir)/include   \
+   -I$(top_builddir)/gnulib/lib -I$(top_srcdir)/gnulib/lib \
+   -I$(top_srcdir)/src -I$(top_srcdir)/src/util\
+   -I$(top_srcdir)
+noinst_PROGRAMS = dommigrate
+dommigrate_CFLAGS = $(WARN_CFLAGS)
+dommigrate_SOURCES = dommigrate.c
+dommigrate_LDADD = $(top_builddir)/src/libvirt.la
diff --git a/examples/dommigrate/dommigrate.c b/examples/dommigrate/dommigrate.c
new file mode 100644
index 000..a8f951e
--- /dev/null
+++ b/examples/dommigrate/dommigrate.c
@@ -0,0 +1,78 @@
+/* This file is largely inspired from hellolibvirt and contains a trivial
+   example that illustrate p2p domain migration with libvirt. */
+
+#include config.h
+
+#include stdio.h
+#include stdlib.h
+#include libvirt/libvirt.h
+#include libvirt/virterror.h
+
+#include virstring.h
+
+static int
+usage(char *prgn, int ret)
+{
+printf(Usage: %s src_uri dst_uri domain\n, prgn);
+return ret;
+}
+
+int
+main(int argc, char *argv[])
+{
+char *src_uri, *dst_uri;
+int ret = 0, id;
+virConnectPtr conn = NULL;
+virDomainPtr dom = NULL;
+
+if (argc  4) {
+ret = usage(argv[0], 1);
+goto out;
+}
+
+src_uri = argv[1];
+dst_uri = argv[2];
+virStrToLong_i(argv[3], NULL, 10, id);
+
+printf(Attempting to connect to the source hypervisor\n);
+conn = virConnectOpenAuth(src_uri, virConnectAuthPtrDefault, 0);
+if (!conn) {
+ret = 1;
+fprintf(stderr, No connection to the source hypervisor: %s\n,
+virGetLastErrorMessage());
+goto out;
+}
+src_uri = virConnectGetURI(conn);
+if (!src_uri) {
+ret = 1;
+fprintf(stderr, Failed to get uri for the source connection: %s\n,
+virGetLastErrorMessage());
+goto disconnect;
+}
+
+printf(Attempting to retrieve domain id: %d\n, id);
+dom = virDomainLookupByID(conn, id);
+if (!dom) {
+fprintf(stderr, Failed to find domain %d\n, id);
+goto disconnect;
+}
+
+printf(Attempting to migrate to: %s\n, dst_uri);
+if ((ret = virDomainMigrateToURI(dom, dst_uri

[libvirt] [PATCH 2/2] Add support for QEMU migration to use SASL authentication

2014-04-14 Thread Sahid Orentino Ferdjaoui
This commit provides the ability to virDomainMigrateToURI to
check for SASL credentials when attempts to migrate a domain
with the driver QEMU.

Signed-off-by: Sahid Orentino Ferdjaoui sahid.ferdja...@cloudwatt.com
---
 src/qemu/qemu_migration.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 593d2d3..e2010e0 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4020,6 +4020,18 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver,
 }
 
 
+static int virConnectCredType[] = {
+VIR_CRED_AUTHNAME,
+VIR_CRED_PASSPHRASE,
+};
+
+
+static virConnectAuth virConnectAuthConfig = {
+.credtype = virConnectCredType,
+.ncredtype = ARRAY_CARDINALITY(virConnectCredType),
+};
+
+
 static int doPeer2PeerMigrate(virQEMUDriverPtr driver,
   virConnectPtr sconn,
   virDomainObjPtr vm,
@@ -4053,7 +4065,7 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver,
  */
 
 qemuDomainObjEnterRemote(vm);
-dconn = virConnectOpen(dconnuri);
+dconn = virConnectOpenAuth(dconnuri, virConnectAuthConfig, 0);
 qemuDomainObjExitRemote(vm);
 if (dconn == NULL) {
 virReportError(VIR_ERR_OPERATION_FAILED,
-- 
1.9.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list