9:24 p.m.
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
Zixing Liu (4):
libvirt-rust: stream: add more functions in stream
libvirt-rust: stream: add more functions in stream
libvirt-rust: use reference instead of moving
libvirt-rust: stream: addressed comments
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
9:24 p.m.
New subject: [libvirt-rust PATCH v3 1/4] libvirt-rust: stream: add more functions in stream
* 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 <liushuyu(a)aosc.io>
---
src/stream.rs | 43 ++++++++++++++++++++++++++++++++++++++++++-
tests/stream.rs | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 82 insertions(+), 1 deletion(-)
create mode 100644 tests/stream.rs
diff --git a/src/stream.rs b/src/stream.rs
index a110695..96e37cb 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,10 @@ 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,
@@ -44,6 +50,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;
@@ -52,6 +61,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<sys::virStreamPtr>,
@@ -71,7 +83,17 @@ impl Drop for Stream {
}
impl Stream {
- pub fn new(ptr: sys::virStreamPtr) -> Stream {
+ pub fn new(conn: &Connect, flags: StreamFlags) -> Result<Stream, Error>
{
+ 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));
+ }
+ }
+
+ pub fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
Stream { ptr: Some(ptr) }
}
@@ -128,4 +150,23 @@ impl Stream {
};
usize::try_from(ret).map_err(|_| Error::new())
}
+
+ pub fn event_update_callback(&self, events: StreamEventType) -> Result<(),
Error> {
+ let ret = unsafe {
+ virStreamEventUpdateCallback(self.as_ptr(), events as libc::c_int)
+ };
+ if ret == -1 {
+ return Err(Error::new());
+ }
+ return Ok(());
+ }
+
+ pub fn event_remove_callback(&self) -> Result<(), Error> {
+ unsafe {
+ if virStreamEventRemoveCallback(self.as_ptr()) == -1 {
+ return Err(Error::new());
+ }
+ return Ok(());
+ }
+ }
}
diff --git a/tests/stream.rs b/tests/stream.rs
new file mode 100644
index 0000000..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 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 <sahid.ferdjaoui(a)redhat.com>
+ */
+
+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(&c, 0).unwrap();
+ drop(s);
+ common::close(c);
+}
+
+#[test]
+fn test_create_non_blocking() {
+ let c = common::conn();
+ let s = Stream::new(&c, VIR_STREAM_NONBLOCK).unwrap();
+ drop(s);
+ common::close(c);
+}
--
2.25.0
9:24 p.m.
New subject: [libvirt-rust PATCH v3 2/4] libvirt-rust: stream: add more functions in stream
* added virStreamEventAddCallback function
* added new types: StreamEventCallback and FreeCallback
* added new field: callback for storing event callback
* drop: will drop the Box<callback> 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 <liushuyu(a)aosc.io>
---
src/stream.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/src/stream.rs b/src/stream.rs
index 96e37cb..3a83b34 100644
--- a/src/stream.rs
+++ b/src/stream.rs
@@ -50,6 +50,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;
@@ -64,9 +70,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 {
+ &mut*(opaque as *mut Stream)
+ };
+ if let Some(callback) = &mut shadow_self.callback {
+ callback(&Stream::from_ptr(c), flags);
+ }
+}
+
+extern "C" fn event_free(_opaque: *mut libc::c_void) {}
+
+// #[derive(Debug)]
pub struct Stream {
ptr: Option<sys::virStreamPtr>,
+ callback: Option<Box<dyn FnMut(&Stream, StreamEventType)>>,
}
impl Drop for Stream {
@@ -79,6 +102,13 @@ impl Drop for Stream {
)
}
}
+ 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)
+ }
+ }
}
}
@@ -94,7 +124,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(&self) -> sys::virStreamPtr {
@@ -151,6 +181,18 @@ impl Stream {
usize::try_from(ret).map_err(|_| Error::new())
}
+ pub fn event_add_callback<F: 'static + FnMut(&Stream,
StreamEventType)>(&mut 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(&self, events: StreamEventType) -> Result<(),
Error> {
let ret = unsafe {
virStreamEventUpdateCallback(self.as_ptr(), events as libc::c_int)
--
2.25.0
9:24 p.m.
New subject: [libvirt-rust PATCH v3 3/4] libvirt-rust: use reference instead of moving
Signed-off-by: Zixing Liu <liushuyu(a)aosc.io>
---
src/domain.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/domain.rs b/src/domain.rs
index 121d0f9..5de66c6 100644
--- a/src/domain.rs
+++ b/src/domain.rs
@@ -1644,7 +1644,7 @@ impl Domain {
}
}
- pub fn open_console(&self, name: &str, stream: Stream, flags: u32) ->
Result<u32, Error> {
+ pub fn open_console(&self, name: &str, stream: &Stream, flags: u32) ->
Result<u32, Error> {
unsafe {
let ret = virDomainOpenConsole(
self.as_ptr(),
--
2.25.0
9:24 p.m.
New subject: [libvirt-rust PATCH v3 4/4] libvirt-rust: stream: addressed comments
* minimized unsafe scope
* removed pub from `from_ptr` function
Signed-off-by: Zixing Liu <liushuyu(a)aosc.io>
---
src/stream.rs | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/stream.rs b/src/stream.rs
index 3a83b34..10145eb 100644
--- a/src/stream.rs
+++ b/src/stream.rs
@@ -114,17 +114,18 @@ impl Drop for Stream {
impl Stream {
pub fn new(conn: &Connect, flags: StreamFlags) -> Result<Stream, Error>
{
- 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 {
- Stream { ptr: Some(ptr), callback: None }
+ fn from_ptr(ptr: sys::virStreamPtr) -> Stream {
+ Stream {
+ ptr: Some(ptr),
+ callback: None,
+ }
}
pub fn as_ptr(&self) -> sys::virStreamPtr {
@@ -136,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.25.0
8:43 a.m.
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
1758
days inactive
1758
days old
5 comments
2 participants
participants (2)
-
Sahid Orentino Ferdjaoui -
Zixing Liu