Re: [libvirt] PATCH 0/5: connection cloning support (WIP)
Ok, with that fix, my asynchronous Java event implementation seems to be
working (though not fully tested) with these patches.
But I'm not using connection cloning (yet). It's your concurrency
control in the remote driver that makes this work.
On the Java side, domain event callbacks from another thread work
because of the (Java) locking of Connect objects. I don't think I
should remove that, since libvirt still prohibits concurrent use of the
same virConnect object (right?)
I suppose I could clone the virConnect object before delivering an event
to a Java Domain (so the Domain's Connect would wrap the clone). I
don't think this does much in the remote case (since the underlying RPC
pipe it still shared, albeit safely). But I suppose it might allow
greater concurrency in the non-remote cases. Is this what you had in
mind?
Dave
On Wed, 2008-12-17 at 11:21 -0500, David Lively wrote:
On Wed, 2008-12-17 at 13:58 +, Daniel P. Berrange wrote:
On Wed, Dec 17, 2008 at 07:44:15AM -0500, David Lively wrote:
Hi Daniel -
When I apply these patches, I'm seeing segfaults on event delivery
when just running the existing synchronous
examples/domain-events/events-c/event-test.c (using the remote driver).
I've not come across that specific problem, but there are a definitely
some locking bugs refcounting bugs inthe patches I've posted so far.
I'll post an updated series of patches which may address this.
Daniel
It turned out to be a double-free. Trivial fix below.
Dave
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 9245a2a..d6b94ff 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -6423,10 +6423,10 @@ remoteDomainQueueEvent(virConnectPtr conn, XDR
*xdr)
return;
if (virDomainEventQueuePush(priv-domainEvents,
-event) 0)
+event) 0) {
DEBUG0(Error adding event to queue);
-
-virDomainEventFree(event);
+virDomainEventFree(event);
+}
}
/** remoteDomainEventFired:
--
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] PATCH 0/5: connection cloning support (WIP)
On Wed, 2008-12-17 at 13:58 +, Daniel P. Berrange wrote:
On Wed, Dec 17, 2008 at 07:44:15AM -0500, David Lively wrote:
Hi Daniel -
When I apply these patches, I'm seeing segfaults on event delivery
when just running the existing synchronous
examples/domain-events/events-c/event-test.c (using the remote driver).
I've not come across that specific problem, but there are a definitely
some locking bugs refcounting bugs inthe patches I've posted so far.
I'll post an updated series of patches which may address this.
Daniel
It turned out to be a double-free. Trivial fix below.
Dave
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 9245a2a..d6b94ff 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -6423,10 +6423,10 @@ remoteDomainQueueEvent(virConnectPtr conn, XDR
*xdr)
return;
if (virDomainEventQueuePush(priv-domainEvents,
-event) 0)
+event) 0) {
DEBUG0(Error adding event to queue);
-
-virDomainEventFree(event);
+virDomainEventFree(event);
+}
}
/** remoteDomainEventFired:
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH 0/5: connection cloning support (WIP)
Hi Daniel - When I apply these patches, I'm seeing segfaults on event delivery when just running the existing synchronous examples/domain-events/events-c/event-test.c (using the remote driver). I've added a little debug. Apparently event-name is being NULLed out sometime after the event is put on the queue: DEBUG: remote_internal.c: remoteDomainEventFired (Event fired 0 3 1 1) DEBUG: remote_internal.c: processCallRecv (Do 4 0) DEBUG: remote_internal.c: processCallRecvLen (Got length, now need 64 total (60 more)) DEBUG: remote_internal.c: processCallRecv (Do 64 4) DEBUG: remote_internal.c: processCallAsyncEvent (Encountered an event while waiting for a response) DEBUG: remote_internal.c: get_nonnull_domain (domain.name: dsl) DEBUG: datatypes.c: virGetDomain (New hash entry 0x804c728) DEBUG: domain_event.c: virDomainEventNew (event: 0x804c770 -name: dsl) DEBUG: libvirt.c: virDomainFree (domain=0x804c728) DEBUG: datatypes.c: virUnrefDomain (unref domain 0x804c728 dsl 1) DEBUG: datatypes.c: virReleaseDomain (release domain 0x804c728 dsl) DEBUG: datatypes.c: virReleaseDomain (unref connection 0x804b040 4) DEBUG: domain_event.c: virDomainEventQueuePush (event: 0x804c770 -name: dsl) DEBUG: remote_internal.c: processCallRecv (Do 0 0) DEBUG: remote_internal.c: remoteDomainEventQueueFlush () DEBUG: domain_event.c: virDomainEventDispatchDefaultFunc (event: 0x804c770 -name: (null)) virGetDomain: name is NULL Segmentation fault I'll continue looking into it. But please let me know if you're familiar with the problem ... Thanks, Dave On Tue, 2008-12-16 at 10:24 -0500, David Lively wrote: Hi Daniels - Sorry for the delay. Life's been crazy. But now I'm finally looking into using this series of patches with my Java event implementation (which I've now redone via the java.nio.Select mechanism; ready to submit pending resolution of the RPC concurrency issues). I should have some feedback for you later today ... Thanks, Dave On Tue, 2008-12-09 at 12:08 +, Daniel P. Berrange wrote: This series is a work-in-progress set of patches implementing connection cloning. The idea is that if you want to use a connection form multiple threads, you could do virConnectPtr copy = virConnectClone(conn) and use 'copy' from the other thread. This avoids the problem of having to make all the virError handling stuff thread-local whic is the blocker for allowing real mutlti-thread access to a single virConnectPtr object. I believe this cloning support should be sufficient for the Java bindings need to use a thread for its event loop. The idea being that if you wanted to use an event loop in a background thread, you'd create a cloned object for use event loop triggered callbacks like domain events notifications. I'd still like to do some experiments later making the single virConnectPtr fully thread safe, but that'll take a little more time. I'm hoping this cloning will address the Java needs right now... Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Java bindings for domain events
Now that I'm actually wading around in the Java java.nio.channels Select swamp (getting this implemented), this is looking like it has exactly the same portability problems as my original code. This approach requires providing our own Selector (and SelectorProvider, corresponding handle/timeout Channels, and SelectionKeys), with our own native implementation of something that can provide select/poll functionality (because there's no way to get that functionality via Java for arbitrary unix fds; can't even construct java FileDescriptors from them in native code because FileDescriptors are declared final). And we also must implement Selector.wakeup() that can interrupt a select operation in another thread (requiring something like the pipe() in my original code - since the select operation will itself be in native code, so can't rely on Java thread interruption). So in the end this looks more like a java.nio.channels.Selector-style interface wrapped around a libvirt EventImpl (which might as well be the one copied from qemud). Certainly that exports a more standard interface for integration with a foreign event loop, but at this point, that looks like the only benefit. For now, I'll continue, assuming the more standard interface is worth the extra code. The Java side of this is basically done, with only the JNI code to write. Just wanted to set expectations ... Dave On Wed, 2008-11-19 at 08:34 +0100, Daniel Veillard wrote: On Tue, Nov 18, 2008 at 01:12:42PM -0500, David Lively wrote: On Tue, 2008-11-18 at 16:51 +, Daniel P. Berrange wrote: On Tue, Nov 18, 2008 at 11:06:10AM -0500, David Lively wrote: The attached patch (against libvirt-java) implements Java bindings for libvirt domain events. This version provides a libvirt EventImpl running in its own Java Thread, and provides per-Connect synchronization that makes using the bindings thread-safe. (Note the Domain, Network, StoragePool, and StorageVol methods also synchronize on their Connect object, as required by libvirt. I have similar changes for NodeDevice.java that need to be made when that code is checked in.) I don't particularly like the event loop code because it is adding a huge pile of non-portable JNI code that won't work on Windows, which lacks pipe() and poll(). Java already provides a portable pure Java API for building a poll() like event loop in form of NIO. http://www.xhaus.com/alan/python/jynio/select.html Yeah, Daniel V and I had briefly considered this, and rejected it on the basis of it's complicated and (more importantly) some negative feedback I hear from our Java folks on the java.nio Select mechanism. But I agree the java.nio Select mechanism should greatly decrease the amount of JNI code in the Java EventImpl. I need to look over the docs again, but I think it's just a matter of implementing a SelectableChannel on top of a fd. (That JNI code will presumably be very different in Win32 and Unix, but it should be a relatively small amount of JNI code in comparison to my current impl.) So I'll look over the java.nio Select documentation and start thinking about a more portable approach ... (and also talk more with our Java folks about their Select gripes). I guess it's better to invest a bit more time and come up with a solution relying as much as possible on Java threading, I/O and synchronization. After all we should capitalize as much as possible on the portability work done in the Java engine, and limit the C part of the bindings to the strict minimum JNI (as much as possible). On one hand we want the bindings to be the easiest possible to use and avoid threading limitation imposed to the client code, on the other hand limit the C part on those issue, of course that means growing the java side of the bindings, but it really should be easier to maintain and port than equivalent C code, even if NIO is not the nicest Java API :-\ Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix python events
On Mon, 2008-11-24 at 17:30 +0100, Jim Meyering wrote: David Lively [EMAIL PROTECTED] wrote: On Sat, 2008-11-22 at 08:21 +0100, Jim Meyering wrote: David Lively [EMAIL PROTECTED] wrote: On Fri, 2008-11-21 at 17:51 +0100, Jim Meyering wrote: I'm printing the (user-supplied) object names to help in debugging misbehaving python EventImpls (since there's no static type checking to catch these kinds of things). So instead of printing NULL when we can't alloc the name, I'm printing something a little more helpful (the appropriate generic name). That's better, indeed. I prefer your NAME macro, too. ACK, modulo syntax: ... Reversed diff? Better to split long lines, not to join. Ok. Fixed in attached patch. + Trailing white space ^^. .. along with this one ... and the trailing whitespace on line 1982. *This* time I ran syntax-check :-) ;-) Unqualified ACK, then, assuming those are the only changes. Those are the only changes. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix python events
On Sat, 2008-11-22 at 08:21 +0100, Jim Meyering wrote:
David Lively [EMAIL PROTECTED] wrote:
On Fri, 2008-11-21 at 17:51 +0100, Jim Meyering wrote:
I'm printing the (user-supplied) object names to help in debugging
misbehaving python EventImpls (since there's no static type checking to
catch these kinds of things). So instead of printing NULL when we
can't alloc the name, I'm printing something a little more helpful (the
appropriate generic name).
That's better, indeed.
I prefer your NAME macro, too.
ACK, modulo syntax:
...
Reversed diff? Better to split long lines,
not to join.
Ok. Fixed in attached patch.
+
Trailing white space ^^.
.. along with this one ... and the trailing whitespace on line 1982.
*This* time I ran syntax-check :-)
Dave
examples/domain-events/events-python/event-test.py | 26 +-
python/libvir.c| 203 +
python/libvir.py |4
python/libvirt_wrap.h |8
python/types.c |1
python/virConnect.py |4
6 files changed, 196 insertions(+), 50 deletions(-)
diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py
index 45aaa93..7bea606 100644
--- a/examples/domain-events/events-python/event-test.py
+++ b/examples/domain-events/events-python/event-test.py
@@ -72,22 +72,22 @@ def myAddHandle(fd, events, cb, opaque):
h_events = events
h_cb = cb
h_opaque = opaque
-
mypoll.register(fd, myEventHandleTypeToPollEvent(events))
+return 0
def myUpdateHandle(watch, event):
global h_fd, h_events
-#print Updating Handle %s %s % (str(fd), str(events))
-h_fd = fd
+#print Updating Handle %s %s % (str(h_fd), str(event))
h_events = event
-mypoll.unregister(watch)
-mypoll.register(watch, myEventHandleTypeToPollEvent(event))
+mypoll.unregister(h_fd)
+mypoll.register(h_fd, myEventHandleTypeToPollEvent(event))
def myRemoveHandle(watch):
global h_fd
-#print Removing Handle %s % str(fd)
+#print Removing Handle %s % str(h_fd)
+mypoll.unregister(h_fd)
h_fd = 0
-mypoll.unregister(watch)
+return h_opaque
def myAddTimeout(timeout, cb, opaque):
global t_active, t_timeout, t_cb, t_opaque
@@ -96,16 +96,18 @@ def myAddTimeout(timeout, cb, opaque):
t_timeout = timeout;
t_cb = cb;
t_opaque = opaque;
+return 0
def myUpdateTimeout(timer, timeout):
global t_timeout
-#print Updating Timeout %s % (str(timer), str(timeout))
+#print Updating Timeout %s %s % (str(timer), str(timeout))
t_timeout = timeout;
def myRemoveTimeout(timer):
global t_active
#print Removing Timeout %s % str(timer)
t_active = 0;
+return t_opaque
##
# Main
@@ -143,6 +145,14 @@ def main():
myRemoveTimeout );
vc = libvirt.open(uri)
+# Close connection on exit (to test cleanup paths)
+old_exitfunc = getattr(sys, 'exitfunc', None)
+def exit():
+print Closing + str(vc)
+vc.close()
+if (old_exitfunc): old_exitfunc()
+sys.exitfunc = exit
+
#Add 2 callbacks to prove this works with more than just one
vc.domainEventRegister(myDomainEventCallback1,None)
vc.domainEventRegister(myDomainEventCallback2,None)
diff --git a/python/libvir.c b/python/libvir.c
index 7d58442..ca1e890 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -35,6 +35,18 @@ extern void initcygvirtmod(void);
#define VIR_PY_INT_FAIL (libvirt_intWrap(-1))
#define VIR_PY_INT_SUCCESS (libvirt_intWrap(0))
+static char *py_str(PyObject *obj)
+{
+PyObject *str = PyObject_Str(obj);
+if (!str) {
+PyErr_Print();
+PyErr_Clear();
+return NULL;
+};
+return PyString_AsString(str);
+}
+
+
/
* *
* Statistics *
@@ -484,7 +496,8 @@ libvirt_virErrorFuncHandler(ATTRIBUTE_UNUSED void *ctx, virErrorPtr err)
PyObject *result;
#ifdef DEBUG_ERROR
-printf(libvirt_virErrorFuncHandler(%p, %s, ...) called\n, ctx, msg);
+printf(libvirt_virErrorFuncHandler(%p, %s, ...) called\n, ctx,
+ err-message);
#endif
if ((err == NULL) || (err-code == VIR_ERR_OK))
@@ -1780,12 +1793,19 @@ libvirt_virConnectDomainEventDeregister(ATTRIBUTE_UNUSED PyObject * self,
* Event Impl
***/
static PyObject *addHandleObj = NULL;
+static char *addHandleName= NULL;
static PyObject *updateHandleObj = NULL;
+static char *updateHandleName = NULL;
static PyObject *removeHandleObj = NULL;
+static char *removeHandleName = NULL;
static PyObject *addTimeoutObj= NULL;
+static char *addTimeoutName = NULL;
static PyObject
Re: [libvirt] [PATCH] fix python events
Doh! ... attached :-)
On Fri, 2008-11-21 at 10:30 +0100, Jim Meyering wrote:
David Lively [EMAIL PROTECTED] wrote:
This patch gets python events working again after upstream changes, and
make the test implementation properly clean up after itself and
implement the new EventImpl API properly.
Note that the Python RemoveHandle and RemoveTimeout implementations
should return the opaque object registered by the corresponding
AddHandle/Timeout calls, in lieu of calling the (C) freefunc. (The
binding code will then call freefunc if it's not NULL.) Ignoring this
means you'll leak memory in the same way that C RemoveHandle/Timeout
leak if they don't (now) call the freefunc.
I also moved around some of the error checking code to unclutter (and
speed up) the common code paths. For instance, we now check that the
virRegisterEventImpl arguments are callable just once (and return
failure if they're not), rather than checking them before every call and
blithely ignoring them if they're not callable.
Dave
examples/domain-events/events-python/event-test.py | 29 +--
python/libvir.c|
200+++
python/libvir.py |4
python/libvirt_wrap.h |8
python/types.c |1
python/virConnect.py |4
6 files changed, 194 insertions(+), 52 deletions(-)
Hi Dave,
It looks like this patch didn't make it to the list.
commit efd5098e9a834562cddbf1618e36eb43c272f8ea
Author: David Lively [EMAIL PROTECTED]
Date: Thu Nov 20 16:36:14 2008 -0500
vi-patch: fix-python-events
Get python events working again after upstream changes, and make the
test implementation properly clean up after itself and implement the
new EventImpl properly.
diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py
index 45aaa93..1ad436f 100644
--- a/examples/domain-events/events-python/event-test.py
+++ b/examples/domain-events/events-python/event-test.py
@@ -72,22 +72,22 @@ def myAddHandle(fd, events, cb, opaque):
h_events = events
h_cb = cb
h_opaque = opaque
-
mypoll.register(fd, myEventHandleTypeToPollEvent(events))
+return 0
def myUpdateHandle(watch, event):
global h_fd, h_events
-#print Updating Handle %s %s % (str(fd), str(events))
-h_fd = fd
+#print Updating Handle %s %s % (str(h_fd), str(event))
h_events = event
-mypoll.unregister(watch)
-mypoll.register(watch, myEventHandleTypeToPollEvent(event))
+mypoll.unregister(h_fd)
+mypoll.register(h_fd, myEventHandleTypeToPollEvent(event))
def myRemoveHandle(watch):
global h_fd
-#print Removing Handle %s % str(fd)
+#print Removing Handle %s % str(h_fd)
+mypoll.unregister(h_fd)
h_fd = 0
-mypoll.unregister(watch)
+return h_opaque
def myAddTimeout(timeout, cb, opaque):
global t_active, t_timeout, t_cb, t_opaque
@@ -96,16 +96,18 @@ def myAddTimeout(timeout, cb, opaque):
t_timeout = timeout;
t_cb = cb;
t_opaque = opaque;
+return 0
def myUpdateTimeout(timer, timeout):
global t_timeout
-#print Updating Timeout %s % (str(timer), str(timeout))
+#print Updating Timeout %s %s % (str(timer), str(timeout))
t_timeout = timeout;
def myRemoveTimeout(timer):
global t_active
#print Removing Timeout %s % str(timer)
t_active = 0;
+return t_opaque
##
# Main
@@ -143,6 +145,14 @@ def main():
myRemoveTimeout );
vc = libvirt.open(uri)
+# Close connection on exit (to test cleanup paths)
+old_exitfunc = getattr(sys, 'exitfunc', None)
+def exit():
+print Closing + str(vc)
+vc.close()
+if (old_exitfunc): old_exitfunc()
+sys.exitfunc = exit
+
#Add 2 callbacks to prove this works with more than just one
vc.domainEventRegister(myDomainEventCallback1,None)
vc.domainEventRegister(myDomainEventCallback2,None)
@@ -175,8 +185,7 @@ def main():
if h_cb != None:
#print Invoking Handle CB
-h_cb(0, h_fd, myPollEventToEventHandleType(revents h_events),
- h_opaque[0], h_opaque[1])
+h_cb(0, h_fd, myPollEventToEventHandleType(revents h_events), h_opaque[0], h_opaque[1])
#print DEBUG EXIT
#break
diff --git a/python/libvir.c b/python/libvir.c
index 07ed09e..6ae7cc1 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -35,6 +35,18 @@ extern void initcygvirtmod(void);
#define VIR_PY_INT_FAIL (libvirt_intWrap(-1))
#define VIR_PY_INT_SUCCESS (libvirt_intWrap(0))
+static char *py_str(PyObject *obj)
+{
+PyObject *str = PyObject_Str(obj);
+if (!str) {
+PyErr_Print();
+PyErr_Clear();
+return NULL;
+};
+return
Re: [libvirt] [PATCH] fix --without-xen build
Hold off on considering this quite yet. I don't think it's a complete fix. I'll followup with a complete one soon. Dave On Fri, 2008-11-21 at 11:04 -0500, David Lively wrote: The last change to tests/Makefile.am broke the build when --without-xen is specified. This patch fixes it ... Dave tests/Makefile.am |9 + 1 file changed, 9 insertions(+) -- 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] [PATCH] fix --without-xen build
Okay, *this* one seems to be a complete fix. Dave tests/Makefile.am | 29 ++--- 1 file changed, 22 insertions(+), 7 deletions(-) On Fri, 2008-11-21 at 12:35 -0500, David Lively wrote: Hold off on considering this quite yet. I don't think it's a complete fix. I'll followup with a complete one soon. diff --git a/tests/Makefile.am b/tests/Makefile.am index d2d1254..a17fdd4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -2,8 +2,11 @@ SHELL = $(PREFERABLY_POSIX_SHELL) -SUBDIRS = virshdata confdata sexpr2xmldata \ - xml2sexprdata xmconfigdata xencapsdata +SUBDIRS = virshdata confdata + +if ENABLE_XEN_TESTS +SUBDIRS += sexpr2xmldata xml2sexprdata xmconfigdata xencapsdata +endif INCLUDES = \ -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \ @@ -43,10 +46,14 @@ EXTRA_DIST = \ nodeinfodata \ domainschematest -noinst_PROGRAMS = xmlrpctest xml2sexprtest sexpr2xmltest virshtest conftest \ - reconnect xmconfigtest xencapstest \ +noinst_PROGRAMS = xmlrpctest virshtest conftest \ nodeinfotest statstest qparamtest +if ENABLE_XEN_TESTS +noinst_PROGRAMS += xml2sexprtest sexpr2xmltest reconnect xmconfigtest \ + xencapstest +endif + if WITH_QEMU noinst_PROGRAMS += qemuxml2argvtest qemuxml2xmltest endif @@ -66,11 +73,10 @@ endif EXTRA_DIST += $(test_scripts) -TESTS = xml2sexprtest sexpr2xmltest virshtest xmconfigtest \ -xencapstest nodeinfotest \ +TESTS = virshtest nodeinfotest \ statstest qparamtest $(test_scripts) if ENABLE_XEN_TESTS - TESTS += reconnect + TESTS += reconnect xml2sexprtest sexpr2xmltest xmconfigtest xencapstest endif if WITH_QEMU TESTS += qemuxml2argvtest qemuxml2xmltest @@ -103,6 +109,7 @@ xmlrpctest_SOURCES = \ xmlrpctest_LDADD = $(LDADDS) +if ENABLE_XEN_TESTS xml2sexprtest_SOURCES = \ xml2sexprtest.c testutilsxen.c testutilsxen.h \ testutils.c testutils.h @@ -117,6 +124,10 @@ xmconfigtest_SOURCES = \ xmconfigtest.c testutilsxen.c testutilsxen.h \ testutils.c testutils.h xmconfigtest_LDADD = ../src/libvirt_driver_xen.la $(LDADDS) +else +EXTRA_DIST += xml2sexprtest.c sexpr2xmltest.c xmconfigtest.c \ + testutilsxen.c testutilsxen.h +endif if WITH_QEMU qemuxml2argvtest_SOURCES = \ @@ -141,9 +152,13 @@ conftest_SOURCES = \ conftest.c conftest_LDADD = $(LDADDS) +if ENABLE_XEN_TESTS xencapstest_SOURCES = \ xencapstest.c testutils.h testutils.c xencapstest_LDADD = ../src/libvirt_driver_xen.la $(LDADDS) +else +EXTRA_DIST += xencapstest.c +endif nodeinfotest_SOURCES = \ nodeinfotest.c testutils.h testutils.c -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix python events
On Fri, 2008-11-21 at 17:51 +0100, Jim Meyering wrote:
No big deal, but there are several debug printf uses here that look
like they try to print NULL pointers upon memory allocation failure.
It's ok with glibc's printf of course, but not for others.
You're right. Attached patch fixes those issues. It also fixes some
cases in which I got some printf string operands switched around ...
I'm printing the (user-supplied) object names to help in debugging
misbehaving python EventImpls (since there's no static type checking to
catch these kinds of things). So instead of printing NULL when we
can't alloc the name, I'm printing something a little more helpful (the
appropriate generic name).
Dave
examples/domain-events/events-python/event-test.py | 29 +--
python/libvir.c| 203 +
python/libvir.py |4
python/libvirt_wrap.h |8
python/types.c |1
python/virConnect.py |4
6 files changed, 197 insertions(+), 52 deletions(-)
diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py
index 45aaa93..1ad436f 100644
--- a/examples/domain-events/events-python/event-test.py
+++ b/examples/domain-events/events-python/event-test.py
@@ -72,22 +72,22 @@ def myAddHandle(fd, events, cb, opaque):
h_events = events
h_cb = cb
h_opaque = opaque
-
mypoll.register(fd, myEventHandleTypeToPollEvent(events))
+return 0
def myUpdateHandle(watch, event):
global h_fd, h_events
-#print Updating Handle %s %s % (str(fd), str(events))
-h_fd = fd
+#print Updating Handle %s %s % (str(h_fd), str(event))
h_events = event
-mypoll.unregister(watch)
-mypoll.register(watch, myEventHandleTypeToPollEvent(event))
+mypoll.unregister(h_fd)
+mypoll.register(h_fd, myEventHandleTypeToPollEvent(event))
def myRemoveHandle(watch):
global h_fd
-#print Removing Handle %s % str(fd)
+#print Removing Handle %s % str(h_fd)
+mypoll.unregister(h_fd)
h_fd = 0
-mypoll.unregister(watch)
+return h_opaque
def myAddTimeout(timeout, cb, opaque):
global t_active, t_timeout, t_cb, t_opaque
@@ -96,16 +96,18 @@ def myAddTimeout(timeout, cb, opaque):
t_timeout = timeout;
t_cb = cb;
t_opaque = opaque;
+return 0
def myUpdateTimeout(timer, timeout):
global t_timeout
-#print Updating Timeout %s % (str(timer), str(timeout))
+#print Updating Timeout %s %s % (str(timer), str(timeout))
t_timeout = timeout;
def myRemoveTimeout(timer):
global t_active
#print Removing Timeout %s % str(timer)
t_active = 0;
+return t_opaque
##
# Main
@@ -143,6 +145,14 @@ def main():
myRemoveTimeout );
vc = libvirt.open(uri)
+# Close connection on exit (to test cleanup paths)
+old_exitfunc = getattr(sys, 'exitfunc', None)
+def exit():
+print Closing + str(vc)
+vc.close()
+if (old_exitfunc): old_exitfunc()
+sys.exitfunc = exit
+
#Add 2 callbacks to prove this works with more than just one
vc.domainEventRegister(myDomainEventCallback1,None)
vc.domainEventRegister(myDomainEventCallback2,None)
@@ -175,8 +185,7 @@ def main():
if h_cb != None:
#print Invoking Handle CB
-h_cb(0, h_fd, myPollEventToEventHandleType(revents h_events),
- h_opaque[0], h_opaque[1])
+h_cb(0, h_fd, myPollEventToEventHandleType(revents h_events), h_opaque[0], h_opaque[1])
#print DEBUG EXIT
#break
diff --git a/python/libvir.c b/python/libvir.c
index 7d58442..fed1031 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -35,6 +35,18 @@ extern void initcygvirtmod(void);
#define VIR_PY_INT_FAIL (libvirt_intWrap(-1))
#define VIR_PY_INT_SUCCESS (libvirt_intWrap(0))
+static char *py_str(PyObject *obj)
+{
+PyObject *str = PyObject_Str(obj);
+if (!str) {
+PyErr_Print();
+PyErr_Clear();
+return NULL;
+};
+return PyString_AsString(str);
+}
+
+
/
* *
* Statistics *
@@ -484,7 +496,8 @@ libvirt_virErrorFuncHandler(ATTRIBUTE_UNUSED void *ctx, virErrorPtr err)
PyObject *result;
#ifdef DEBUG_ERROR
-printf(libvirt_virErrorFuncHandler(%p, %s, ...) called\n, ctx, msg);
+printf(libvirt_virErrorFuncHandler(%p, %s, ...) called\n, ctx,
+ err-message);
#endif
if ((err == NULL) || (err-code == VIR_ERR_OK))
@@ -1780,12 +1793,19 @@ libvirt_virConnectDomainEventDeregister(ATTRIBUTE_UNUSED PyObject * self,
* Event Impl
***/
static PyObject *addHandleObj = NULL;
+static char *addHandleName=
Re: [libvirt] [PATCH 1/2] Java bindings for domain events
On Fri, 2008-11-21 at 14:49 +0100, Daniel Veillard wrote: On Wed, Nov 19, 2008 at 11:22:31AM -0500, David Lively wrote: On Wed, 2008-11-19 at 10:35 -0500, David Lively wrote: The patch already synchronizes operations using virConnect objects with each other. To avoid making illegal EventImpl callbacks from Java for the current libvirt, I have to lock every Connect object known to Java and hold off creating new connections (via open friends) around an EventImpl callback. This sounds rather appalling to me, but it's starting to sound like the only practical route in the short term (unless it turns out we can rely on pthreads in WIN32 ...). note that libxml2 that we rely on has a fully ported mutex basic API in libxml/threads.h in case you really want to do the exclusive locking at the C level while still being portable. Thanks for the pointer. This could be used. I'd assume we'd want to change the mutex usage in datatypes.c as well, for consistency. Still it's probably better to try to implement most of this at the Java level, at least IMHO, Just to be clear, I've been planning on keeping the per-Connect Java synchronization from the original patch. (And I think you guys have bought into that much, so far.) So here we're discussing the concurrency requirements imposed upon the callbacks made by an asynchronous external (non-libvirtd) EventImpl. Currently, we must make sure that no other connection is in use (and no other EventImpl callback is in progress). I'm arguing this is far too restrictive since it basically means locking ALL connections before an event can be recognized. So if one connection is performing some long-running operation (say something storage-related), no events can be delivered to *any* connection until the operation completes (and then we'd better hope another long-running operation hasn't been started on another connection in the mean time). Keep in mind I'm not proposing we make all libvirt C public interfaces threadsafe. We only need to make sure the paths reachable from external event impls (in the remote and xen-inotify drivers) are threadsafe. I believe the patch I submitted (2/2 in this series) does this for the remote driver, albeit via the PTHREADS_MUTEX macros that currently have no Win32 impl. (And I'll do the same for the xen-inotify driver once it's in. I haven't looked into it yet ...) I'll happily change the existing mutex usage in datatypes.c to the libxml one. And I'll use the same mutex impl in the remote and xen-inotify drivers. But I'd rather not bother until you guys agree this is desirable ... Thanks, Dave P.S. I realize this isn't really a practical problem right now. It's more of a future limit on scalability. But because it's part of a new interface (virEventRegisterImpl), it seems worth it to try and specify this nicely now (as opposed to relaxing the concurrency requirements in a later version, which is a kind of API change). -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] fix libvirtd crash in qemu driver
I noticed that the following sequence of events would crash libvirtd
when using the qemu driver:
(1) establish a connection that successfully registers for domain
events (either of the event-test programs will do, though the
python one is currently broken -- another patch on the way for
that)
(2) close this connection
(3) open another connection
(4) do something (like start a domain) that issues a domain event
The problem is that qemudClose() isn't removing registered domain event
callbacks when the connection closes. This patch does that, and fixes
the crash.
Dave
domain_event.c | 38 ++
domain_event.h |3 +++
qemu_driver.c |5 -
3 files changed, 45 insertions(+), 1 deletion(-)
commit d70494a2c2ebdf985943020cc84f22713904719a
Author: David Lively [EMAIL PROTECTED]
Date: Thu Nov 20 16:34:32 2008 -0500
vi-patch: qemu-driver-close-fix
Fix a bug in the QEMU driver causing libvirtd crashes. When closing
a connection, remove the DomainEvent callbacks associated with
the connection.
diff --git a/src/domain_event.c b/src/domain_event.c
index 85ca9b7..d5f5415 100644
--- a/src/domain_event.c
+++ b/src/domain_event.c
@@ -88,6 +88,44 @@ virDomainEventCallbackListRemove(virConnectPtr conn,
}
/**
+ * virDomainEventCallbackListRemoveConn:
+ * @conn: pointer to the connection
+ * @cbList: the list
+ *
+ * Internal function to remove all of a given connection's callback
+ * from a virDomainEventCallbackListPtr
+ */
+int
+virDomainEventCallbackListRemoveConn(virConnectPtr conn,
+ virDomainEventCallbackListPtr cbList)
+{
+int old_count = cbList-count;
+int i;
+for (i = 0 ; i cbList-count ; i++) {
+if(cbList-callbacks[i]-conn == conn) {
+virFreeCallback freecb = cbList-callbacks[i]-freecb;
+if (freecb)
+(*freecb)(cbList-callbacks[i]-opaque);
+virUnrefConnect(cbList-callbacks[i]-conn);
+VIR_FREE(cbList-callbacks[i]);
+
+if (i (cbList-count - 1))
+memmove(cbList-callbacks + i,
+cbList-callbacks + i + 1,
+sizeof(*(cbList-callbacks)) *
+(cbList-count - (i + 1)));
+cbList-count--;
+i--;
+}
+}
+if (cbList-count old_count
+VIR_REALLOC_N(cbList-callbacks, cbList-count) 0) {
+; /* Failure to reduce memory allocation isn't fatal */
+}
+return 0;
+}
+
+/**
* virDomainEventCallbackListAdd:
* @conn: pointer to the connection
* @cbList: the list
diff --git a/src/domain_event.h b/src/domain_event.h
index cfec1e1..454d084 100644
--- a/src/domain_event.h
+++ b/src/domain_event.h
@@ -55,6 +55,9 @@ int virDomainEventCallbackListRemove(virConnectPtr conn,
virDomainEventCallbackListPtr cbList,
virConnectDomainEventCallback callback);
+int virDomainEventCallbackListRemoveConn(virConnectPtr conn,
+ virDomainEventCallbackListPtr cbList);
+
/**
* Dispatching domain events that come in while
* in a call / response rpc
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 5ad60f1..7bec116 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1235,7 +1235,10 @@ static virDrvOpenStatus qemudOpen(virConnectPtr conn,
}
static int qemudClose(virConnectPtr conn) {
-/*struct qemud_driver *driver = (struct qemud_driver *)conn-privateData;*/
+struct qemud_driver *driver = (struct qemud_driver *)conn-privateData;
+
+/* Get rid of callbacks registered for this conn */
+virDomainEventCallbackListRemoveConn(conn, driver-domainEventCallbacks);
conn-privateData = NULL;
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] fix EventImpl-related error paths in remote driver
This patch makes the remote driver behave properly in the face of:
(a) no registered EventImpl, or
(b) an EventImpl that returns failure from AddHandle/Timeout
In both cases, we now cleanup properly (rather than always passing bogus
values to virEventRemoveHandle/Timeout) and fail attempts to register
for domain events (w/VIR_ERR_NO_SUPPORT rather than blissfully continue
when we can't possibly deliver events).
Dave
remote_internal.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
commit 02239f3e0fedf1c3860826f13efe356223b38e3c
Author: David Lively [EMAIL PROTECTED]
Date: Thu Nov 20 16:35:08 2008 -0500
vi-patch: remote-refuse-useless-event-registration
Made the remote driver's DomainEventRegister return an error if events
not supported (e.g., because there's no proper EventImpl registered).
Also, cleanup properly in this case, both in doRemoteOpen and
doRemoteClose.
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 7ca6ec1..7124d0a 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -773,6 +773,7 @@ doRemoteOpen (virConnectPtr conn,
conn, NULL)) 0) {
DEBUG0(virEventAddTimeout failed: No addTimeoutImpl defined.
continuing without events.);
+virEventRemoveHandle(priv-watch);
}
}
/* Successful. */
@@ -1209,10 +1210,12 @@ doRemoteClose (virConnectPtr conn, struct private_data *priv)
(xdrproc_t) xdr_void, (char *) NULL) == -1)
return -1;
-/* Remove handle for remote events */
-virEventRemoveHandle(priv-sock);
-/* Remove timout */
-virEventRemoveTimeout(priv-eventFlushTimer);
+if (priv-eventFlushTimer = 0) {
+/* Remove timeout */
+virEventRemoveTimeout(priv-eventFlushTimer);
+/* Remove handle for remote events */
+virEventRemoveHandle(priv-watch);
+}
/* Close socket. */
if (priv-uses_tls priv-session) {
@@ -4481,6 +4484,10 @@ static int remoteDomainEventRegister (virConnectPtr conn,
{
struct private_data *priv = conn-privateData;
+if (priv-eventFlushTimer 0) {
+ error (conn, VIR_ERR_NO_SUPPORT, _(no event support));
+ return -1;
+}
if (virDomainEventCallbackListAdd(conn, priv-callbackList,
callback, opaque, freecb) 0) {
error (conn, VIR_ERR_RPC, _(adding cb to list));
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Java bindings for domain events
On Wed, 2008-11-19 at 10:48 +, Daniel P. Berrange wrote: On Wed, Nov 19, 2008 at 08:34:41AM +0100, Daniel Veillard wrote: On Tue, Nov 18, 2008 at 01:12:42PM -0500, David Lively wrote: On Tue, 2008-11-18 at 16:51 +, Daniel P. Berrange wrote: On Tue, Nov 18, 2008 at 11:06:10AM -0500, David Lively wrote: The attached patch (against libvirt-java) implements Java bindings for libvirt domain events. This version provides a libvirt EventImpl running in its own Java Thread, and provides per-Connect synchronization that makes using the bindings thread-safe. (Note the Domain, Network, StoragePool, and StorageVol methods also synchronize on their Connect object, as required by libvirt. I have similar changes for NodeDevice.java that need to be made when that code is checked in.) I don't particularly like the event loop code because it is adding a huge pile of non-portable JNI code that won't work on Windows, which lacks pipe() and poll(). Java already provides a portable pure Java API for building a poll() like event loop in form of NIO. http://www.xhaus.com/alan/python/jynio/select.html Yeah, Daniel V and I had briefly considered this, and rejected it on the basis of it's complicated and (more importantly) some negative feedback I hear from our Java folks on the java.nio Select mechanism. But I agree the java.nio Select mechanism should greatly decrease the amount of JNI code in the Java EventImpl. I need to look over the docs again, but I think it's just a matter of implementing a SelectableChannel on top of a fd. (That JNI code will presumably be very different in Win32 and Unix, but it should be a relatively small amount of JNI code in comparison to my current impl.) So I'll look over the java.nio Select documentation and start thinking about a more portable approach ... (and also talk more with our Java folks about their Select gripes). I guess it's better to invest a bit more time and come up with a solution relying as much as possible on Java threading, I/O and synchronization. After all we should capitalize as much as possible on the portability work done in the Java engine, and limit the C part of the bindings to the strict minimum JNI (as much as possible). Agreed -- BUT if we don't make libvirt allow asynchronous EventImpl callbacks (the second patch in this series (plus requiring pthreads, which sounds like a Big Deal for WIN32)), we'll have to stop the EventImpl callbacks from happening: (a) at the same time *ANY* connection (in the same process) is in use (because EventImpl callbacks aren't necessarily associated with any particular connection, though they might be) (b) at the same time another EventImpl callback is happening The patch already synchronizes operations using virConnect objects with each other. To avoid making illegal EventImpl callbacks from Java for the current libvirt, I have to lock every Connect object known to Java and hold off creating new connections (via open friends) around an EventImpl callback. This sounds rather appalling to me, but it's starting to sound like the only practical route in the short term (unless it turns out we can rely on pthreads in WIN32 ...). Alternatively, I could just use one global lock for everything, but that essentially single-threads all libvirt operations, which seems *really* undesirable. On one hand we want the bindings to be the easiest possible to use and avoid threading limitation imposed to the client code, on the other hand limit the C part on those issue, of course that means growing the java side of the bindings, but it really should be easier to maintain and port than equivalent C code, even if NIO is not the nicest Java API :-\ Yeah, I suspect so. I'm starting to implement this path ... Also, while I remember any event loop implementation in Java should be an optional add-on class, not a mandatory part of the Java libvirt bindings. Applications may well already have an event loop they wish to use - for example a java desktop application will have an event loop provided by GTK or QT. So all that would be require is a Java binding to the libvirt-glib module, so you cn register libvirt with Glib from Java. Gotcha. I've just installed glib-java-devel, so I'll make sure this easily integrates with a glib event loop as exposed in glib-java. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Java bindings for domain events
On Wed, 2008-11-19 at 10:35 -0500, David Lively wrote: The patch already synchronizes operations using virConnect objects with each other. To avoid making illegal EventImpl callbacks from Java for the current libvirt, I have to lock every Connect object known to Java and hold off creating new connections (via open friends) around an EventImpl callback. This sounds rather appalling to me, but it's starting to sound like the only practical route in the short term (unless it turns out we can rely on pthreads in WIN32 ...). Hmmm ... maybe the less appalling :-) route is practical. Currently, we require only the Windows equivalent of a simple pthread mutex. We just need to support declaration, initialization, lock, unlock, and destruction, something like the following (thanks to Tom Hazel for pointing me to the Windows Mutex stuff): #if (defined _WIN32 || defined __WIN32__) #define PTHREAD_MUTEX_T(v) HANDLE v #define pthread_mutex_init(lk,p) ((*(lk)) = CreateMutex(0, FALSE, 0)) #define pthread_mutex_destroy(lk) CloseHandle(*(lk)) #define pthread_mutex_lock(lk) WaitForSingleObject(*(lk), INFINITE) #define pthread_mutex_unlock(lk) ReleaseMutex(*(lk)) #define pthread_sigmask(h, s, o) sigprocmask((h), (s), (o)) #endif I'm not a Windows guy, so maybe I'm missing something. But this doesn't seem like a Big Deal ... Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix WIN64 compatibility bug in external EventImpl API
On Wed, 2008-11-19 at 16:49 +, Daniel P. Berrange wrote: On Wed, Nov 19, 2008 at 11:41:43AM -0500, David Lively wrote: While starting to think about Windows compability, I realized the newly exposed API for registering an external EventImpl is not adequate. Currently it's assuming 32-bit unix fds. But Windows uses a pointer (HANDLE) here. So we need to generalize this interface so it can be implemented for 64-bit Windows. The attached patch does this. (I'm sure it conflicts with work Dan B is doing, so I'm hoping he'll just incorporate this into his changes.) I'm not sure whether this is actually required. We're using gnulib for socket stuff, and that wraps the Winsock socket() call so that it returns a real file descriptor rather than a socket handle. It does this calling _open_osfhandle which appears to be declared to accept a 'long' and return an 'int' - at least in MinGW headers. That means that the Windows application using libvirt must use gnulib as well. If the Windows version of libvirt actually exports the gnulib bindings and headers, then I guess that's not a problem. But does it export gnulib? -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix WIN64 compatibility bug in external EventImpl API
On Wed, 2008-11-19 at 18:33 +, Daniel P. Berrange wrote: On Wed, Nov 19, 2008 at 01:28:23PM -0500, David Lively wrote: On Wed, 2008-11-19 at 16:49 +, Daniel P. Berrange wrote: On Wed, Nov 19, 2008 at 11:41:43AM -0500, David Lively wrote: While starting to think about Windows compability, I realized the newly exposed API for registering an external EventImpl is not adequate. Currently it's assuming 32-bit unix fds. But Windows uses a pointer (HANDLE) here. So we need to generalize this interface so it can be implemented for 64-bit Windows. The attached patch does this. (I'm sure it conflicts with work Dan B is doing, so I'm hoping he'll just incorporate this into his changes.) I'm not sure whether this is actually required. We're using gnulib for socket stuff, and that wraps the Winsock socket() call so that it returns a real file descriptor rather than a socket handle. It does this calling _open_osfhandle which appears to be declared to accept a 'long' and return an 'int' - at least in MinGW headers. That means that the Windows application using libvirt must use gnulib as well. If the Windows version of libvirt actually exports the gnulib bindings and headers, then I guess that's not a problem. But does it export gnulib? No, the gnulib stuff is internal only - we don't force any apps to also use gnulib. It does however mean we should document that the 'fd' arg of the the virEventAddHandle callback is an file handle, and not a socket handle on Win32, so apps are clear on what to expect. Does Windows support integer file handles? Or are they a winsock concept - in which case we're assuming the app uses winsock, right? [Sorry - I'm not trying to be thick -- I'm not a Windows guy at all. But I have hazy memories of winsock not being standard WIN32 ...] Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Java bindings for domain events
This patch allows the remote driver to work with an asynchronous
EventImpl (it's the only one using an externally-supplied one), assuming
libvirt is compiled with pthread support. (Without pthreads, this code
is harmless in a single-threaded environment.)
Basically it uses a mutex to protect reads from the RPC socket in such a
way that message reads (in their entirety) are done atomically
(otherwise the remoteDomainEventFired() can race the call() code that
reads replies events).
In addition, I update the EventImpl handle to prevent
remoteDomainEventFired() from being called everytime a reply is sent.
(This helps us dispatch events in a timely manner, though it's not
strictly necessary. Without it, any events coming in during a call()
won't be dispatched until the call drops the socket lock (because
remoteDomainEventFired() will be stuck awaiting the lock).
Dave
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 2ca7930..59128f6 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -116,6 +116,7 @@ struct private_data {
virDomainEventQueuePtr domainEvents;
/* Timer for flushing domainEvents queue */
int eventFlushTimer;
+PTHREAD_MUTEX_T(lock); /* Serializes socket reads w/async EventImpl */
};
#define GET_PRIVATE(conn,retcode) \
@@ -700,6 +701,9 @@ doRemoteOpen (virConnectPtr conn,
} /* switch (transport) */
+/* This must precede the first call() */
+priv-eventFlushTimer = -1;
+
/* Try and authenticate with server */
if (remoteAuthenticate(conn, priv, 1, auth, authtype) == -1)
goto failed;
@@ -744,6 +748,8 @@ doRemoteOpen (virConnectPtr conn,
}
}
+pthread_mutex_init(priv-lock, NULL);
+
if(VIR_ALLOC(priv-callbackList)0) {
error(conn, VIR_ERR_INVALID_ARG, _(Error allocating callbacks list));
goto failed;
@@ -1250,6 +1256,8 @@ doRemoteClose (virConnectPtr conn, struct private_data *priv)
/* Free queued events */
virDomainEventQueueFree(priv-domainEvents);
+pthread_mutex_destroy(priv-lock);
+
return 0;
}
@@ -4536,11 +4544,11 @@ static int really_read (virConnectPtr conn, struct private_data *priv,
* else Bad Things will happen in the XDR code.
*/
static int
-call (virConnectPtr conn, struct private_data *priv,
- int flags /* if we are in virConnectOpen */,
- int proc_nr,
- xdrproc_t args_filter, char *args,
- xdrproc_t ret_filter, char *ret)
+really_call (virConnectPtr conn, struct private_data *priv,
+ int flags /* if we are in virConnectOpen */,
+ int proc_nr,
+ xdrproc_t args_filter, char *args,
+ xdrproc_t ret_filter, char *ret)
{
char buffer[REMOTE_MESSAGE_MAX];
char buffer2[4];
@@ -4596,16 +4604,18 @@ call (virConnectPtr conn, struct private_data *priv,
really_write (conn, priv, flags REMOTE_CALL_IN_OPEN, buffer, len-4) == -1)
return -1;
+pthread_mutex_lock(priv-lock);
+
retry_read:
/* Read and deserialise length word. */
if (really_read (conn, priv, flags REMOTE_CALL_IN_OPEN, buffer2, sizeof buffer2) == -1)
-return -1;
+goto unlock_return_err;
xdrmem_create (xdr, buffer2, sizeof buffer2, XDR_DECODE);
if (!xdr_int (xdr, len)) {
error (flags REMOTE_CALL_IN_OPEN ? NULL : conn,
VIR_ERR_RPC, _(xdr_int (length word, reply)));
-return -1;
+goto unlock_return_err;
}
xdr_destroy (xdr);
@@ -4615,12 +4625,14 @@ retry_read:
if (len 0 || len REMOTE_MESSAGE_MAX) {
error (flags REMOTE_CALL_IN_OPEN ? NULL : conn,
VIR_ERR_RPC, _(packet received from server too large));
-return -1;
+goto unlock_return_err;
}
/* Read reply header and what follows (either a ret or an error). */
if (really_read (conn, priv, flags REMOTE_CALL_IN_OPEN, buffer, len) == -1)
-return -1;
+goto unlock_return_err;
+
+pthread_mutex_unlock(priv-lock);
/* Deserialise reply header. */
xdrmem_create (xdr, buffer, len, XDR_DECODE);
@@ -4729,8 +4741,33 @@ retry_read:
xdr_destroy (xdr);
return -1;
}
+
+ unlock_return_err:
+pthread_mutex_unlock(priv-lock);
+return -1;
+}
+
+static int call (virConnectPtr conn, struct private_data *priv,
+ int flags /* if we are in virConnectOpen */,
+ int proc_nr,
+ xdrproc_t args_filter, char *args,
+ xdrproc_t ret_filter, char *ret)
+{
+int rv;
+if (priv-eventFlushTimer = 0)
+virEventUpdateHandle(priv-sock, 0);
+rv = really_call(conn, priv, flags, proc_nr,
+ args_filter, args,
+ ret_filter, ret);
+if (priv-eventFlushTimer = 0)
+virEventUpdateHandle(priv-sock,
+ VIR_EVENT_HANDLE_READABLE |
+
Re: [libvirt] [PATCH 2/2] Java bindings for domain events
On Tue, 2008-11-18 at 17:05 +, Daniel P. Berrange wrote: On Tue, Nov 18, 2008 at 11:18:38AM -0500, David Lively wrote: This patch allows the remote driver to work with an asynchronous EventImpl (it's the only one using an externally-supplied one), assuming libvirt is compiled with pthread support. (Without pthreads, this code is harmless in a single-threaded environment.) I don't like this patch, since it is only making one tiny part of the API thread-safe, for one tiny use case. eg, if someone uses events from the Xen driver in Java with threads, they'll crash burn, since only the remote driver is being protected. Currently ONLY the remote driver (and yes, soon - the xen driver, which would also need thread-safety changes) use an EventImpl supplied externally. All others use the libvirtd-provided synchronous impl. Why doesn't the Java code simply synchronize on the virConnect object before invoking the FD event callbacks. That will ensure another thread has finished whatever API call it was doing Note that the Java code uses (Java's builtin) Connect-level lock to avoid concurrent calls using the same virConnect. This even applies to domain event delivery - note that Connect.fireDomainEventCallbacks is (in the Java patch) a synchronized method. A FD event callback isn't associated with a particular virConnect object, and EventImpls aren't virConnect-specific, so I can't lock the virConnect (see below). When we exposed virEventRegisterImpl, we said that externally-supplied event impls must not make callbacks when a virConnect is being used. If the Java EventImpl were following this rule, we wouldn't need this patch. BUT because the EventImpl can't know which virConnect (if any) is involved in a particular callback, the only way to satisfy this rule is to lock ALL Connect objects before making an EventImpl callback. This is (IMO) both way too restrictive, and (I'm a little less sure of this) not restrictive enough. (I suspect we'd find that making two callbacks concurrently can break things, even if no virConnects are in use at the time.) I think we have to allow externally supplied EventImpls to make callbacks without regard to which virConnect, etc. objects are in use, since EventImpls don't know anything about virConnect, etc. This doesn't require all of libvirt to be made thread-safe. So I view this fix (and the one necessary for the xen driver once it starts using an externally-supplied EventImpl) as providing just enough concurrency control to allow an asynchronous EventImpl, while still making the libvirt user (the Java bindings, in my case) responsible for avoiding concurrent virConnect usage. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Java bindings for domain events
On Tue, 2008-11-18 at 16:51 +, Daniel P. Berrange wrote: On Tue, Nov 18, 2008 at 11:06:10AM -0500, David Lively wrote: The attached patch (against libvirt-java) implements Java bindings for libvirt domain events. This version provides a libvirt EventImpl running in its own Java Thread, and provides per-Connect synchronization that makes using the bindings thread-safe. (Note the Domain, Network, StoragePool, and StorageVol methods also synchronize on their Connect object, as required by libvirt. I have similar changes for NodeDevice.java that need to be made when that code is checked in.) I don't particularly like the event loop code because it is adding a huge pile of non-portable JNI code that won't work on Windows, which lacks pipe() and poll(). Java already provides a portable pure Java API for building a poll() like event loop in form of NIO. http://www.xhaus.com/alan/python/jynio/select.html Yeah, Daniel V and I had briefly considered this, and rejected it on the basis of it's complicated and (more importantly) some negative feedback I hear from our Java folks on the java.nio Select mechanism. But I agree the java.nio Select mechanism should greatly decrease the amount of JNI code in the Java EventImpl. I need to look over the docs again, but I think it's just a matter of implementing a SelectableChannel on top of a fd. (That JNI code will presumably be very different in Win32 and Unix, but it should be a relatively small amount of JNI code in comparison to my current impl.) So I'll look over the java.nio Select documentation and start thinking about a more portable approach ... (and also talk more with our Java folks about their Select gripes). Dave The addition of the 'synchronized' annotatioons is something we need regardless of the rest of the patch, since our API contract dictates that only a single thread is allowed to use a single virConnectPtr at once. This version of the patch also implements and uses an enum class (DomainEvent.Type), as suggested by Tóth István. IMPORTANT: THIS PATCH WILL BREAK THINGS UNLESS THE NEXT [PATCH 2/2] IS APPLIED TO libvirt FIRST. Also, libvirt must be compiled WITH_PTHREADS for Java events to work. Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Java bindings for domain events
On Fri, 2008-11-14 at 12:59 -0500, David Lively wrote:
On Fri, 2008-11-14 at 17:09 +, Daniel P. Berrange wrote:
Or have the virConnectDomainEventRegister method take an extra parameter
which is a callbackvoid (*freefunc)(void*). libvirt would just invoke
that to free the opaque data chunk.
Yeah, I like this better. The dbus(?) API allows an optional
destructor (freefunc) to be specified for callback userdata. So let's
allow it to be null (in which case we obviously don't call it at remove
time).
I think we need a similar thing with the event loops APIs for timers
and file handle watches, to make it easier to free the opaque data
blob they have.
Sounds good too.
I can make the DomainEvent changes today / this weekend while working on
the Java bindings (since I need them to plug the Java leak), and submit
them on Monday (or perhaps later today, if I don't get diverted).
The attached patch implements this change (adds a freefunc arg to
virConnectDomainEventRegister and calls it on Deregister (or Close)).
It also modifies the event-test.c example to register a freefunc and
deregister callbacks when interrupted or terminated (to verify the
freefuncs are properly called).
Dave
commit 1cacb0944958dbd39f002d99721112ec2b8df7f5
Author: David Lively [EMAIL PROTECTED]
Date: Mon Nov 17 15:48:50 2008 -0500
vi-patch: events
As discussed on libvir-list, added an extra arg:
void (*freefunc)(void *opaque)
to virConnectDomainEventRegister. If non-NULL, this
function is called by virConnectDomainEventDeregister()
and passed the void *opaque argument registered with
the callback being removed.
diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c
index 0a741ea..11d62c7 100644
--- a/examples/domain-events/events-c/event-test.c
+++ b/examples/domain-events/events-c/event-test.c
@@ -1,7 +1,9 @@
#include config.h
#include stdio.h
+#include stdlib.h
#include string.h
+#include signal.h
#if HAVE_SYS_POLL_H
#include sys/types.h
@@ -168,6 +170,13 @@ int myDomainEventCallback2 (virConnectPtr conn ATTRIBUTE_UNUSED,
return 0;
}
+static void myFreeFunc(void *opaque)
+{
+char *str = opaque;
+printf(%s: Freeing [%s]\n, __FUNCTION__, str);
+free(str);
+}
+
/* EventImpl Functions */
int myEventHandleTypeToPollEvent(virEventHandleType events)
@@ -254,15 +263,27 @@ void usage(const char *pname)
printf(%s uri\n, pname);
}
+int run = 1;
+
+static void stop(int sig)
+{
+printf(Exiting on signal %d\n, sig);
+run = 0;
+}
+
+
int main(int argc, char **argv)
{
-int run=1;
int sts;
+struct sigaction action_stop = {
+.sa_handler = stop
+};
if(argc 1 STREQ(argv[1],--help)) {
usage(argv[0]);
return -1;
}
+
virEventRegisterImpl( myEventAddHandleFunc,
myEventUpdateHandleFunc,
myEventRemoveHandleFunc,
@@ -277,11 +298,16 @@ int main(int argc, char **argv)
return -1;
}
+sigaction(SIGTERM, action_stop, NULL);
+sigaction(SIGINT, action_stop, NULL);
+
DEBUG0(Registering domain event cbs);
/* Add 2 callbacks to prove this works with more than just one */
-virConnectDomainEventRegister(dconn, myDomainEventCallback1, NULL);
-virConnectDomainEventRegister(dconn, myDomainEventCallback2, NULL);
+virConnectDomainEventRegister(dconn, myDomainEventCallback1,
+ strdup(callback 1), myFreeFunc);
+virConnectDomainEventRegister(dconn, myDomainEventCallback2,
+ strdup(callback 2), myFreeFunc);
while(run) {
struct pollfd pfd = { .fd = h_fd,
@@ -315,9 +341,15 @@ int main(int argc, char **argv)
}
+DEBUG0(Deregistering event handlers);
+virConnectDomainEventDeregister(dconn, myDomainEventCallback1);
+virConnectDomainEventDeregister(dconn, myDomainEventCallback2);
+
+DEBUG0(Closing connection);
if( dconn virConnectClose(dconn)0 ) {
printf(error closing\n);
}
+
printf(done\n);
return 0;
}
diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index d1bb154..c56d272 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -1095,7 +1095,8 @@ typedef int (*virConnectDomainEventCallback)(virConnectPtr conn,
int virConnectDomainEventRegister(virConnectPtr conn,
virConnectDomainEventCallback cb,
- void *opaque);
+ void *opaque,
+ void (*freefunc)(void *));
int virConnectDomainEventDeregister(virConnectPtr conn,
virConnectDomainEventCallback cb);
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 0ee657a..6a63ef4 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt
Re: [libvirt] [PATCH] Java bindings for domain events
On Fri, 2008-11-14 at 17:09 +, Daniel P. Berrange wrote:
On Fri, Nov 14, 2008 at 12:00:10PM -0500, David Lively wrote:
+JNIEXPORT void JNICALL Java_org_libvirt_Connect_registerForDomainEvents
+(JNIEnv *env, jobject obj, jlong VCP){
+// TODO: Need to DeleteGlobalRef(obj) when deregistering for
callbacks.
+// But then need to track global obj per Connect object.
Hum, that's a bit nasty. Can we make sure we can plug the leaks
without having to change the APIs, that would be a bummer...
Yeah. It's really not acceptable as is. The easiest solution (as you
hint) is changing the API so virConnectDomainEventDeregister returns the
void * registered with that callback. That would (of course) be my
preference. What do you think? That API hasn't been released quite
yet ...
Or have the virConnectDomainEventRegister method take an extra parameter
which is a callbackvoid (*freefunc)(void*). libvirt would just invoke
that to free the opaque data chunk.
Yeah, I like this better. The dbus(?) API allows an optional
destructor (freefunc) to be specified for callback userdata. So let's
allow it to be null (in which case we obviously don't call it at remove
time).
I think we need a similar thing with the event loops APIs for timers
and file handle watches, to make it easier to free the opaque data
blob they have.
Sounds good too.
I can make the DomainEvent changes today / this weekend while working on
the Java bindings (since I need them to plug the Java leak), and submit
them on Monday (or perhaps later today, if I don't get diverted).
Are you going to make the event impl changes?
Thanks,
Dave
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 8/12:Interal driver API for node devices
On Fri, 2008-11-14 at 13:20 +, Mark McLoughlin wrote:
On Thu, 2008-11-13 at 17:31 +, Daniel P. Berrange wrote:
+virBufferVSprintf(buf, capability type='%s'\n,
+ virNodeDevCapTypeToString(caps-type));
+switch (caps-type) {
...
+case VIR_NODE_DEV_CAP_NET:
+virBufferVSprintf(buf, interface%s/interface\n,
+ data-net.interface);
+if (data-net.address)
+virBufferVSprintf(buf, address%s/address\n,
+ data-net.address);
+if (data-net.subtype != VIR_NODE_DEV_CAP_NET_LAST) {
+const char *subtyp =
+virNodeDevNetCapTypeToString(data-net.subtype);
+virBufferVSprintf(buf, capability type='%s'\n,
subtyp);
+switch (data-net.subtype) {
+case VIR_NODE_DEV_CAP_NET_80203:
+virBufferVSprintf(buf,
+
mac_address%012llx/address\n,
+
data-net.data.ieee80203.mac_address);
+break;
+case VIR_NODE_DEV_CAP_NET_80211:
+virBufferVSprintf(buf,
+
mac_address%012llx/address\n,
+
data-net.data.ieee80211.mac_address);
+break;
+case VIR_NODE_DEV_CAP_NET_LAST:
+/* Keep dumb compiler happy */
+break;
+}
+virBufferAddLit(buf, /capability\n);
This all seems a bit odd.
We've listed the mac address once already, so why do it again in a hex format?
Agreed. It's basically just reflecting the HAL attributes. But I have
to admit I find a lot of HAL rather non-intuitive ... I have no
particular attachment to any of this node device description XML. I
really liked Daniel's suggestion that this be based on a subset of HAL,
and have tried to do that as much as possible. I don't really care what
it's based on, but it sure would be nice to avoid inventing yet another
XML device representation (hence the attraction to HAL).
But the more I've gotten to know HAL, the less I've liked the idea. I'm
not aware of an alternative that doesn't involve inventing our own.
Perhaps there's something suitable in CIM?? I'm a CIM-dummy, so I have
no idea ... This is related to the device naming issue as well. I
share all of Dan B's concerns here ...
And I wouldn't think of 802.3 vs 802.11 as a network device capability, but
more like it's type - they're mutually exclusive, right?
Also, we have:
device
...
capability type='net'
...
capability type='80203'
mac_address001320f74a06/address
/capability
/capability
/device
i.e. two different capability semantics?
Again, just reflecting HAL. But I agree it seems bizarre.
+}
+break;
+case VIR_NODE_DEV_CAP_BLOCK:
+virBufferVSprintf(buf, device%s/device\n,
+ data-block.device);
Two nested device nodes:
device
...
capability type='block'
device/dev/sda1/device
/capability
/device
How about path or device_path ?
This one doesn't bother me so much, though granted the two device tags
are talking about completely different things (clear to me from the
context, but perhaps violating some proper XML design rules?).
+if (data-storage.originating_device)
+virBufferVSprintf(buf,
+ originating_device%s
+ /originating_device\n,
+ data-storage.originating_device);
This originating_device thing sounds strange, and I don't think we
implement it yet. Leave it out for now?
Fine with me. I don't know what it is -- just sounded generally useful.
If it's not widely implemented, we probably shouldn't bother supporting
it.
+if (data-storage.flags VIR_NODE_DEV_CAP_STORAGE_REMOVABLE) {
+int avl = data-storage.flags
+VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE;
+virBufferAddLit(buf, capability
type='removable'\n);
+virBufferVSprintf(buf,
+media_available%d
+ /media_available\n, avl ? 1 : 0);
+virBufferVSprintf(buf,
media_size%llu/media_size\n,
+ data-storage.removable_media_size);
+virBufferAddLit(buf, /capability\n);
+} else {
+virBufferVSprintf(buf, size%llu/size\n,
+ data-storage.size);
+
Re: [libvirt] PATCH: 7/12: Public API for node devices
On Fri, 2008-11-14 at 13:28 +, Daniel P. Berrange wrote: On Fri, Nov 14, 2008 at 12:46:09PM +, Mark McLoughlin wrote: On Thu, 2008-11-13 at 17:30 +, Daniel P. Berrange wrote: This patch is the public API parts of the node device enumeration code. No changes since David's last submission of this, except for some Makefile tweaks + +int virNodeNumOfDevices (virConnectPtr conn, + unsigned int flags); + +int virNodeListDevices (virConnectPtr conn, + char **const names, + int maxnames, + unsigned int flags); + +int virNodeNumOfDevicesByCap (virConnectPtr conn, + const char *cap, + unsigned int flags); + +int virNodeListDevicesByCap (virConnectPtr conn, + const char *cap, + char **const names, + int maxnames, + unsigned int flags); How about combining these two sets of functions and if the capability type isn't supplied list all devices? Yes, we could just remove the ByCap APIs, and add the 'const char *cap' arg to the first two APIs, allowing NULL. I like this idea as well. + +virNodeDevicePtrvirNodeDeviceLookupByName (virConnectPtr conn, + const char *name); + +const char *virNodeDeviceGetName (virNodeDevicePtr dev); + How stable are these names? e.g. should we say that no-one should rely on the format of the name and that the name of a given device could change across node reboots? Even if HAL guarantees the name to be stable (does it?), if you switch between HAL and DevKit it could change, right? I don't think HAL explicitly guarentees it, it merely happens to have been stable AFAICT. The naming is definitely completely different between HAL and DevKit. This is probably my biggest worry with the impl so far - some app using it will need to have a stable identifier for a device and we won't be providing it. We could invent our own stable naming scheme for devices - the scheme would vary per capability - eg for PCI devices we can use the bus, function, slot identifiers. USB is hard to guarentee though - if a device is plugged in unpluged plugged in again it won't get the same address, and there's no real other identifier we can rely on for this. Yep. But I think HAL is already trying to specify stable names wherever possible. E.g., PCI devs aren't named by position on bus, but by vendor-id/product-id, so that a PCI impl supporting hotplug will give the same name for the card if it's taken out and reinserted (well, more or less ...). Perhaps we could just identify where we think HAL gets it wrong, and implement our own naming scheme for those devices (assuming we can come up with one we like better), and using the HAL names for the rest?? I see that all this naturally flows from the way hal does things, but the pci device isn't a parent of the network device in any real sense ... I guess I would expect to just see one device with both the 'net' and 'pci' capabilities. So that's basically removing all explicit tracking of 'logical' devices (NICs, disk, etc) and only ever representing 'physical' devices (PCI, USB devices). The problem I can see is that one physical device can result in many logical devices - even multiple instances of the same capability - particularly multi-function USB devices can result in a large number of sub-devices for audio, video, etc. Or a SCSI host adapter can have a single PCI device, but result in multiple host adapters in the OS view. Separating the physical from logical devices gives us the opportunity to define more stable names for devices with certain capabilities. eg, for a USB network card, its hard to invent a stable name at the level of the USB device, but for the logical NIC you can easily invent a name based off the MAC address. Agreed. I think this extra level of heirarchy is useful, though I originally found it confusing. +int virNodeDeviceNumOfCaps (virNodeDevicePtr dev); + +int virNodeDeviceListCaps(virNodeDevicePtr dev, + char **const names, + int maxnames); I think it would make more sense to me if there was a fixed set of capability types and for them to be an enum in the API
Re: [libvirt] PATCH: 9/12: HAL/DevitKit node device impl
On Fri, 2008-11-14 at 13:41 +, Mark McLoughlin wrote:
The DeviceKit implementation seems to be much more of a work-in-progress
than the HAL implementation - maybe disable it by default in configure?
(i.e. with_devkit=no instead of with_devkit=check)
That's a good idea. I don't think the devkit impl can go forward until
devkit itself starts moving forward. (Hmmm ... I (now) see the devkit
mailing list has started getting some traffic. Apparently they've just
released v2. Perhaps it's time for another look, though I can't sign up
for that right now.)
+interface = strrchr(sysfs_path, '/');
+if (!interface *interface *(++interface))
+return -1;
Was this meant to be:
if (!interface || !*interface || !*(++interface))
Yes. Or the equivalent (! (interface *interface *(++interface)).
+static void dev_create(void *_dkdev, void *_dkclient ATTRIBUTE_UNUSED)
+{
...
+
+dev-privateData = dkdev;
You need need a privateFree() hook here to do g_object_unref(), I think
Yes. Good catch, again.
+for (i = 0 ; i ARRAY_CARDINALITY(caps_tbl) ; i++) {
+const char *caps[] = { caps_tbl[i].cap_name, NULL };
+devs = devkit_client_enumerate_by_subsystem(devkit_client,
+caps,
+err);
+if (err) {
+DEBUG0(devkit_client_enumerate_by_subsystem failed);
+devs = NULL;
+goto failure;
+}
+g_list_foreach(devs, dev_create, devkit_client);
Need a g_list_free() here right?
Yes ...
+}
+
+driverState-privateData = devkit_client;
+
+// TODO: Register to get DeviceKit events on device changes and
+// coordinate updates with queries and other operations.
That's a pretty big missing feature - if both HAL and DevKit were
available on a system, we'd still want HAL until this is fixed, right?
Absolutely. I think this supports your request that this be disabled by
default.
+(void)get_int_prop(ctx, udi, pci.vendor_id, (int
*)d-pci_dev.vendor);
+if (get_str_prop(ctx, udi, pci.vendor, d-pci_dev.vendor_name) != 0)
+(void)get_str_prop(ctx, udi, info.vendor,
d-pci_dev.vendor_name);
+(void)get_int_prop(ctx, udi, pci.product_id, (int
*)d-pci_dev.product);
+if (get_str_prop(ctx, udi, pci.product, d-pci_dev.product_name) !=
0)
+(void)get_str_prop(ctx, udi, info.product,
d-pci_dev.product_name);
By the way - vendor and product IDs are normally quoted in hex, not
decimal - e.g. I'd know my NIC is 8086:10de, not 32902:4318
I guess most other integer values in libvirt XML are decimal, but might
be worth adding a hex format for this?
I'd prefer hex for vid/pid as well; I just stuck with decimal since the
rest of libvirt does. If this does get changed to output hex, I'd only
request that we prefix hex numbers with 0x so people don't have to
remember which attrs are dumped in hex and which in decimal.
+static void device_prop_modified(LibHalContext *ctx ATTRIBUTE_UNUSED,
+ const char *udi,
+ const char *key,
+ dbus_bool_t is_removed ATTRIBUTE_UNUSED,
+ dbus_bool_t is_added ATTRIBUTE_UNUSED)
+{
+const char *name = hal_name(udi);
+virNodeDeviceObjPtr dev =
virNodeDeviceFindByName(driverState-devs,name);
+DEBUG(%s %s, key, name);
+if (dev) {
+/* Simply rediscover device -- incrementally handling changes
+ * to properties (which are mapped into caps in very capability-
+ * specific ways) is nasty ... so avoid it.
+ */
+virNodeDeviceObjRemove(driverState-devs, dev);
+dev_create(strdup(udi));
+} else
+DEBUG(no device named %s, name);
+}
Could use the same callback for both of these (with a cast), or simplify
them to:
static void device_prop_modified(LibHalContext *ctx ATTRIBUTE_UNUSED,
const char *udi,
const char *key,
dbus_bool_t is_removed ATTRIBUTE_UNUSED,
dbus_bool_t is_added ATTRIBUTE_UNUSED)
{
DEBUG(%s %s, key, name);
/* Simply rediscover device -- incrementally handling changes
* to properties (which are mapped into caps in very capability-
* specific ways) is nasty ... so avoid it.
*/
device_removed(ctx, udi);
device_added(ctx, udi);
}
Yep, that's a good idea.
Thanks for the careful review,
Dave
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] 0/7 host (node) device enumeration - completed submission
P.S. Patch 7/7 (Java bindings for host dev enum) hasn't changed. On Thu, 2008-11-06 at 21:32 -0500, David Lively wrote: Daniel(s) - In the attached set of patches, I've merged in DanB's reworking of the headers, etc. and adjusted my code accordingly. I've also renamed NodeDriver to DeviceMonitor, as discussed with DanV, and added a --cap option to the virsh node-list-devices command. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Java bindings for domain events
The attached patch (against libvirt-java) contains Java bindings for the
new domain event code. It works (see EventTest.java), but there's a
certain amount of hokiness regarding the EventImpl stuff that I'd like
to discuss.
Unlike the C and Python interfaces, the Java interface does not
currently allow the client to supply an EventImpl. The problem is that
Java really has no way to interact with unix file descriptors so there's
no reasonable way to implement a fd-watching EventImpl in pure Java
(java.io.FileDescriptor doesn't do the trick since there's no way of
constructing one from raw (int) unix fd)[**].
So for now, I've had the Java bindings register an EventImpl when the
Connect class is loaded. This EventImpl is a Java class, with native
methods implementing it. In fact, I've simply stolen (verbatim) the
EventImpl from libvirt/qemud/events.c and made the native methods call
it. [If we stick with this solution, it would obviously be better to
somehow share this code with libvirtd rather than copy it.]
The other tricky subject is multi-threading. For now, I've avoided it
by exposing Connect.eventImpl.run_once() and forcing the client to call
it from their event loop. But the normal Java way of doing things
would simply run the EventImpl in a separate thread. In fact, this
EventImpl does implement Runnable, which makes it trivial to run in a
separate thread -- but I don't declare that it implements Runnable yet
because it's not safe to run in a thread while another thread might be
making libvirt calls.
It shouldn't be hard to make this thread-safe using Java synchronized
methods and statements, but I haven't done that yet. Should I??
** java.nio.Channel and friends seem to be the right interface for
exposing abstract selectable channels in Java. It's just complicated
enough that I've avoided it for now. But I can look into going this way
for allowing Java to provide an EventImpl in the future ..
Cheers,
Dave
diff --git a/EventTest.java b/EventTest.java
new file mode 100644
index 000..dc01f8b
--- /dev/null
+++ b/EventTest.java
@@ -0,0 +1,35 @@
+import org.libvirt.*;
+
+class TestDomainEventListener implements DomainEventListener {
+
+String name;
+
+TestDomainEventListener(String name) {
+ this.name = name;
+}
+
+public void handle(Domain dom, int event) {
+ try {
+ System.out.println(name + : dom + dom.getName() + got event + event);
+ } catch (LibvirtException e) {
+ System.out.println(e);
+ System.out.println(name + : unknown dom got event + event);
+ }
+}
+}
+
+class EventTest {
+
+public static void main(String args[]) throws LibvirtException {
+ String URI = qemu:///system;
+ if (args.length 0)
+ URI = args[0];
+ Connect conn = new Connect(URI);
+ conn.domainEventRegister(new TestDomainEventListener(Test 1));
+ conn.domainEventRegister(new TestDomainEventListener(Test 2));
+
+ while (true) {
+ conn.eventImpl.run_once();
+ }
+}
+}
diff --git a/src/Makefile.am b/src/Makefile.am
index 5200c1d..a265de9 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -7,9 +7,11 @@ java_libvirt_source_files = \
org/libvirt/ConnectAuthDefault.java \
org/libvirt/ConnectAuth.java \
org/libvirt/DomainBlockStats.java \
+ org/libvirt/DomainEventListener.java \
org/libvirt/DomainInfo.java \
org/libvirt/DomainInterfaceStats.java \
org/libvirt/Domain.java \
+ org/libvirt/EventImpl.java \
org/libvirt/ErrorException.java \
org/libvirt/Error.java \
org/libvirt/Network.java \
diff --git a/src/jni/Makefile.am b/src/jni/Makefile.am
index c894024..829298a 100644
--- a/src/jni/Makefile.am
+++ b/src/jni/Makefile.am
@@ -5,6 +5,7 @@ GENERATED = \
org_libvirt_Domain_CreateFlags.h \
org_libvirt_Domain_MigrateFlags.h \
org_libvirt_Domain_XMLFlags.h \
+ org_libvirt_EventImpl.h \
org_libvirt_StoragePool_BuildFlags.h \
org_libvirt_StoragePool_DeleteFlags.h \
org_libvirt_StoragePool.h \
@@ -25,6 +26,9 @@ org_libvirt_Network.h: $(JAVA_CLASS_ROOT)/org/libvirt/Network.class
org_libvirt_Domain.h org_libvirt_Domain_CreateFlags.h org_libvirt_Domain_MigrateFlags.h org_libvirt_Domain_XMLFlags.h : $(JAVA_CLASS_ROOT)/org/libvirt/Domain.class
$(JAVAH) -classpath $(JAVA_CLASS_ROOT) org.libvirt.Domain
+org_libvirt_EventImpl.h : $(JAVA_CLASS_ROOT)/org/libvirt/EventImpl.class
+ $(JAVAH) -classpath $(JAVA_CLASS_ROOT) org.libvirt.EventImpl
+
org_libvirt_StoragePool.h org_libvirt_StoragePool_BuildFlags.h org_libvirt_StoragePool_DeleteFlags.h : $(JAVA_CLASS_ROOT)/org/libvirt/StoragePool.class
$(JAVAH) -classpath $(JAVA_CLASS_ROOT) org.libvirt.StoragePool
@@ -36,6 +40,7 @@ libvirt_jni_la_SOURCES = \
org_libvirt_Network.c \
org_libvirt_Connect.c \
org_libvirt_Domain.c \
+ org_libvirt_EventImpl.c \
org_libvirt_StoragePool.c \
org_libvirt_StorageVol.c \
generic.h \
diff --git a/src/jni/org_libvirt_Connect.c b/src/jni/org_libvirt_Connect.c
index cbf437c..3107159 100644
--- a/src/jni/org_libvirt_Connect.c
+++
Re: [libvirt] [RFC] making (newly public) EventImpl interface moreconsistent
On Wed, 2008-11-05 at 11:51 +, Daniel P. Berrange wrote:
DevKit HAL are just APIs built ontop of DBus, so the key here
is integration with DBus watch APIs. AFAIK, those only require
that the event loop impl have one callback per unique FD.
Here's what I'm seeing when registering for dbus watch callbacks. In
halNodeDriverStartup (in node_device_hal.c in the submitted host dev
enum patch), I register for dbus watch callbacks:
/* Register dbus watch callbacks */
if (!dbus_connection_set_watch_functions(dbus_conn,
add_dbus_watch,
remove_dbus_watch,
toggle_dbus_watch,
NULL, NULL)) {
fprintf(stderr, %s: dbus_connection_set_watch_functions failed\n,
__FUNCTION__);
goto failure;
}
And then I instrumented add/remove/toggle_dbus_watch. add_dbus_watch is
called twice as soon as we register the watch functions:
add_dbus_watch 0xae4200 fd 6 flags 0x2 enabled 0
add_dbus_watch 0xaeb950 fd 6 flags 0x1 enabled 1
*** DUPLICATE HANDLE 6 at [0] ***
So here we have two different DBusWatches sharing the same unix fd. In
this case, the first one (POLLOUT flags) is disabled, and never toggled,
so things happen to work just fine. The current qemud AddHandleImpl
will in fact overwrite the first entry with the second, so it has
totally lost the first watch.
But this is just lucky. Because the behavior of adding a duplicate
handle is undefined, the implementation could just as well have ignored
the second entry, in which case DBus events would never be received.
I'll look into the glib handle-watching code (which I'm not familiar
with currently) and see how it behaves, but I can't imagine it doesn't
support this when DBus watches clearly require it.
Dave
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] making (newly public) EventImpl interface moreconsistent
Ugh. I see what you mean. It seems more complicated than that, though. For one thing, the AvahiWatch/AvahiPoll stuff is sufficiently abstract that it doesn't mention DBusConnection (or any other DBus type, AFAIK), so there's no indication that *any* DBusConnection is being used (though I trust that there is). Turning on AVAHI_DEBUG, I see both my (node_device_hal.c) callbacks and the avahi callbacks being called (but at different times, and with different fds and event sets). Makes me wonder whether the avahi library is using dbus_bus_get_private() to get its own DBusConnection that it doesn't have to worry about sharing with others. I tried changing the node_device_hal code to use a private dbus connection, and see exactly the same behavior (i.e., dbus immediately registers the same fd twice, with different event sets and enable state). Regardless, I like the idea of simply using this private connection (as you suggested but rejected), just for the sake of simplicity. (Also, what if Avahi's been configured out?) But ... back to the original problem. Immediately after calling dbus_set_watch_functions (and *before* initializing the Avahi stuff), I see two callbacks to watch_add with the same fd. Looking at the glib watch functions, I see watches on GIOChannels are added with g_io_add_watch(), which indeed doesn't specify what happens if the same GIOChannel is added twice. But GIOChannels aren't fds. Presumably, one could create two different GIOChannels with the same fd (via g_io_channel_unix_new) and then register watches on each of them. So I remain convinced that we need to support the registration of the same fd multiple times ... Dave [Caveat: Okay, so I've never actually *used* glib ... I'm going from the docs here ...] On Wed, 2008-11-05 at 16:49 +, Daniel P. Berrange wrote: On Wed, Nov 05, 2008 at 11:26:07AM -0500, David Lively wrote: On Wed, 2008-11-05 at 11:51 +, Daniel P. Berrange wrote: I think the problem here is that the existing Avahi mdns code in the libvirtd daemon is also already using DBus, and has already setup the DBus system bus instances to integrate with the libvirtd event loop. By default, there is a single DBus system bus connection in the app which is shared amongst all users. The DBus API spec for its watch functions mandates that the application only setup watches once per connection, so having both the Avahi and HAL/DevKit code in libvirt call dbus_connection_set_watch_functions() against the shared connection is in fact a bug. For added fun, PolicyKit code in the daemon also makes use of DBus, but doesn't (yet) need callbacks so hasn't done mainloop integration. So the flaw here is that the individual drivers should not all be attempting to setup integration with the shared dbus system bus connection. Either each driver should use a private dbus system bus conenction, or the main daemon startup code should take responsibility for integrating the shared connection instance into its main loop. I'd be inclined to go for the latter. For simplicitly, I think you ought to simply be able to remove the dbus_connection_set_watch_functions call from the driver, and rely on fact that Avahi code has already done this. Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] making (newly public) EventImpl interface more consistent
Hi Folks - Since virEventRegisterImpl is now public (in libvirt.h), a nagging concern of mine has become more urgent. Essentially this callback gives clients a way of registering their own handle (fd) watcher and timer functionality for use by libvirt. What bugs me is the inconsistency between the handle-watcher and timer interfaces: the timer add function returns a timer id, which is then used to identify the timer to the update and remove functions. But the handle-watcher add / update / remove functions identify the watcher by the handle (fd). The semantics of registering the same handle twice aren't specified (what happens when we pass that same fd in a subsequent update or remove?). Even worse, this doesn't allow one to manage multiple watches on the same handle reasonably. So why not make the handle add function return a watch id (analogous to the timer id returned by the timer add fn)? And then use this watch id to specify the handle-watcher in the update and remove functions. This conveniently handles multiple watches on the same handle, and also makes the handle-watching interface more consistent with the timer interface (which is registered at the same time). We'd pass both the watch id and the handle (fd) into the watch-handler callback. I'd like to implement and submit this (along with fixups to the event test code) if there are no objections. Thanks, Dave P.S. I'm currently working on Java bindings to the new event code ... -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] 0/7 host (node) device enumeration - completed submission
Since the version of this I posted last week still had some outstaning TODO items that I have since finished, I'm re-submitting the whole thing. I consider this a complete submission since I don't think it's necessary to implement any additional functionality to make this acceptable. (The two remaining unimplemented items are virNodeDeviceCreate / Destroy and a real Devkit impl, neither of which needs to be done immediately, for reasons discussed earlier.) These patches implement the node device enumeration functionality, as discussed here: https://www.redhat.com/archives/libvir-list/2008-September/msg00398.html I've broken it into the following pieces: 1-public-api additions to the public API 2-internal-apiadditions to the internal API 3-local-node-drivers the HAL DeviceKit implementations 4-remote-node-driver the remote driver 5-virsh-support virsh support 6-python-bindings python bindings 7-java-bindings Java bindings (for libvirt-java) Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [PATCH] 1/7 host (node) device enumeration - public API changes
This patch contains the public API changes for host device enumeration.
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 97aea7d..0b0a4bc 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -995,6 +995,74 @@ virDomainPtrvirDomainCreateLinux(virConnectPtr conn,
unsigned int flags);
/*
+ * Host device enumeration
+ */
+
+/**
+ * virNodeDevice:
+ *
+ * A virNodeDevice contains a node (host) device details.
+ */
+
+typedef struct _virNodeDevice virNodeDevice;
+
+/**
+ * virNodeDevicePtr:
+ *
+ * A virNodeDevicePtr is a pointer to a virNodeDevice structure. Get
+ * one via virNodeDeviceLookupByKey, virNodeDeviceLookupByName, or
+ * virNodeDeviceCreate. Be sure to Call virNodeDeviceFree when done
+ * using a virNodeDevicePtr obtained from any of the above functions to
+ * avoid leaking memory.
+ */
+
+typedef virNodeDevice *virNodeDevicePtr;
+
+
+int virNodeNumOfDevices (virConnectPtr conn,
+ unsigned int flags);
+
+int virNodeListDevices (virConnectPtr conn,
+ char **const names,
+ int maxnames,
+ unsigned int flags);
+
+int virNodeNumOfDevicesByCap (virConnectPtr conn,
+ const char *cap,
+ unsigned int flags);
+
+int virNodeListDevicesByCap (virConnectPtr conn,
+ const char *cap,
+ char **const names,
+ int maxnames,
+ unsigned int flags);
+
+virNodeDevicePtrvirNodeDeviceLookupByName (virConnectPtr conn,
+ const char *name);
+
+const char *virNodeDeviceGetName (virNodeDevicePtr dev);
+
+const char *virNodeDeviceGetParent (virNodeDevicePtr dev);
+
+int virNodeDeviceNumOfCaps (virNodeDevicePtr dev);
+
+int virNodeDeviceListCaps(virNodeDevicePtr dev,
+ char **const names,
+ int maxnames);
+
+char * virNodeDeviceGetXMLDesc (virNodeDevicePtr dev,
+ unsigned int flags);
+
+virNodeDevicePtrvirNodeDeviceCreate (virConnectPtr conn,
+ const char *xml,
+ unsigned int flags);
+
+int virNodeDeviceDestroy(virNodeDevicePtr dev,
+ unsigned int flags);
+
+int virNodeDeviceFree (virNodeDevicePtr dev);
+
+/*
* Domain Event Notification
*/
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 8e24708..020b8dc 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -58,6 +58,7 @@ typedef enum {
VIR_FROM_STORAGE, /* Error from storage driver */
VIR_FROM_NETWORK, /* Error from network config */
VIR_FROM_DOMAIN,/* Error from domain config */
+VIR_FROM_NODE, /* Error from node driver */
} virErrorDomain;
@@ -148,6 +149,9 @@ typedef enum {
VIR_WAR_NO_STORAGE, /* failed to start storage */
VIR_ERR_NO_STORAGE_POOL, /* storage pool not found */
VIR_ERR_NO_STORAGE_VOL, /* storage pool not found */
+VIR_WAR_NO_NODE, /* failed to start node driver */
+VIR_ERR_INVALID_NODE_DEVICE,/* invalid node device object */
+VIR_ERR_NO_NODE_DEVICE,/* node device not found */
} virErrorNumber;
/**
diff --git a/src/libvirt.c b/src/libvirt.c
index 8fd594b..8bf3b74 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -74,6 +74,8 @@ static virNetworkDriverPtr virNetworkDriverTab[MAX_DRIVERS];
static int virNetworkDriverTabCount = 0;
static virStorageDriverPtr virStorageDriverTab[MAX_DRIVERS];
static int virStorageDriverTabCount = 0;
+static virNodeDriverPtr virNodeDriverTab[MAX_DRIVERS];
+static int virNodeDriverTabCount = 0;
#ifdef WITH_LIBVIRTD
static virStateDriverPtr virStateDriverTab[MAX_DRIVERS];
static int virStateDriverTabCount = 0;
@@ -300,6 +302,19 @@ virInitialize(void)
#ifdef WITH_NETWORK
if (networkRegister() == -1) return -1;
#endif
+#if defined(HAVE_HAL) defined(HAVE_DEVKIT)
+/* Register only one of these two - they conflict */
+if (halNodeRegister() == -1)
+if (devkitNodeRegister() == -1)
+return -1;
+#else
+#ifdef HAVE_HAL
+if (halNodeRegister() == -1) return -1;
+#endif
+#ifdef HAVE_DEVKIT
+if
[libvirt] Re: [PATCH] 2/7 host (node) device enumeration - additions to internal API
This patch contains additions to the internal API for host device
enumeration.
diff --git a/src/hash.c b/src/hash.c
index 0a5bdcd..424c4a7 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -687,6 +687,9 @@ virGetConnect(void) {
ret-storageVols = virHashCreate(20);
if (ret-storageVols == NULL)
goto failed;
+ret-nodeDevices = virHashCreate(256);
+if (ret-nodeDevices == NULL)
+goto failed;
pthread_mutex_init(ret-lock, NULL);
@@ -703,6 +706,8 @@ failed:
virHashFree(ret-storagePools, (virHashDeallocator) virStoragePoolFreeName);
if (ret-storageVols != NULL)
virHashFree(ret-storageVols, (virHashDeallocator) virStorageVolFreeName);
+if (ret-nodeDevices != NULL)
+virHashFree(ret-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
pthread_mutex_destroy(ret-lock);
VIR_FREE(ret);
@@ -730,6 +735,8 @@ virReleaseConnect(virConnectPtr conn) {
virHashFree(conn-storagePools, (virHashDeallocator) virStoragePoolFreeName);
if (conn-storageVols != NULL)
virHashFree(conn-storageVols, (virHashDeallocator) virStorageVolFreeName);
+if (conn-nodeDevices != NULL)
+virHashFree(conn-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
virResetError(conn-err);
if (__lastErr.conn == conn)
@@ -1318,3 +1325,126 @@ virUnrefStorageVol(virStorageVolPtr vol) {
pthread_mutex_unlock(vol-conn-lock);
return (refs);
}
+
+
+/**
+ * virGetNodeDevice:
+ * @conn: the hypervisor connection
+ * @name: device name (unique on node)
+ *
+ * Lookup if the device is already registered for that connection,
+ * if yes return a new pointer to it, if no allocate a new structure,
+ * and register it in the table. In any case a corresponding call to
+ * virFreeNodeDevice() is needed to not leak data.
+ *
+ * Returns a pointer to the node device, or NULL in case of failure
+ */
+virNodeDevicePtr
+__virGetNodeDevice(virConnectPtr conn, const char *name)
+{
+virNodeDevicePtr ret = NULL;
+
+if ((!VIR_IS_CONNECT(conn)) || (name == NULL)) {
+virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+return(NULL);
+}
+pthread_mutex_lock(conn-lock);
+
+ret = (virNodeDevicePtr) virHashLookup(conn-nodeDevices, name);
+if (ret == NULL) {
+ if (VIR_ALLOC(ret) 0) {
+virHashError(conn, VIR_ERR_NO_MEMORY, _(allocating node dev));
+goto error;
+}
+ret-magic = VIR_NODE_DEVICE_MAGIC;
+ret-conn = conn;
+ret-name = strdup(name);
+if (ret-name == NULL) {
+virHashError(conn, VIR_ERR_NO_MEMORY, _(copying node dev name));
+goto error;
+}
+
+if (virHashAddEntry(conn-nodeDevices, name, ret) 0) {
+virHashError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(failed to add node dev to conn hash table));
+goto error;
+}
+conn-refs++;
+}
+ret-refs++;
+pthread_mutex_unlock(conn-lock);
+return(ret);
+
+error:
+pthread_mutex_unlock(conn-lock);
+if (ret != NULL) {
+VIR_FREE(ret-name);
+VIR_FREE(ret);
+}
+return(NULL);
+}
+
+
+/**
+ * virReleaseNodeDevice:
+ * @dev: the dev to release
+ *
+ * Unconditionally release all memory associated with a dev.
+ * The conn.lock mutex must be held prior to calling this, and will
+ * be released prior to this returning. The dev obj must not
+ * be used once this method returns.
+ *
+ * It will also unreference the associated connection object,
+ * which may also be released if its ref count hits zero.
+ */
+static void
+virReleaseNodeDevice(virNodeDevicePtr dev) {
+virConnectPtr conn = dev-conn;
+DEBUG(release dev %p %s, dev, dev-name);
+
+if (virHashRemoveEntry(conn-nodeDevices, dev-name, NULL) 0)
+virHashError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(dev missing from connection hash table));
+
+dev-magic = -1;
+VIR_FREE(dev-name);
+VIR_FREE(dev);
+
+DEBUG(unref connection %p %s %d, conn, conn-name, conn-refs);
+conn-refs--;
+if (conn-refs == 0) {
+virReleaseConnect(conn);
+/* Already unlocked mutex */
+return;
+}
+
+pthread_mutex_unlock(conn-lock);
+}
+
+
+/**
+ * virUnrefNodeDevice:
+ * @dev: the dev to unreference
+ *
+ * Unreference the dev. If the use count drops to zero, the structure is
+ * actually freed.
+ *
+ * Returns the reference count or -1 in case of failure.
+ */
+int
+virUnrefNodeDevice(virNodeDevicePtr dev) {
+int refs;
+
+pthread_mutex_lock(dev-conn-lock);
+DEBUG(unref dev %p %s %d, dev, dev-name, dev-refs);
+dev-refs--;
+refs = dev-refs;
+if (refs == 0) {
+virReleaseNodeDevice(dev);
+/* Already unlocked mutex */
+return (0);
+}
+
+pthread_mutex_unlock(dev-conn-lock);
+return (refs);
+}
diff --git a/src/internal.h b/src/internal.h
index 7ea1586..02aa343 100644
--- a/src/internal.h
[libvirt] Re: [PATCH] 3/7 host (node) device enumeration - HAL Devkit impls
This patch contains the HAL-based implementation, and a preliminary
DeviceKit-based implementation, of node device enumeration. The Devkit
impl is very incomplete, mostly because Devkit itself is very immature
at this stage.
diff --git a/configure.in b/configure.in
index 2d7fb14..3486a47 100644
--- a/configure.in
+++ b/configure.in
@@ -1048,6 +1048,93 @@ test $enable_shared = no lt_cv_objdir=.
LV_LIBTOOL_OBJDIR=${lt_cv_objdir-.}
AC_SUBST([LV_LIBTOOL_OBJDIR])
+dnl HAL or DeviceKit library for host device enumeration
+HAL_REQUIRED=0.0
+HAL_CFLAGS=
+HAL_LIBS=
+AC_ARG_WITH([hal],
+ [ --with-hal use HAL for host device enumeration],
+ [],
+ [with_hal=check])
+
+if test x$with_hal = xyes -o x$with_hal = xcheck; then
+ PKG_CHECK_MODULES(HAL, hal = $HAL_REQUIRED,
+[with_hal=yes], [
+if test x$with_hal = xcheck ; then
+ with_hal=no
+else
+ AC_MSG_ERROR(
+ [You must install hal-devel = $HAL_REQUIRED to compile libvirt])
+fi
+ ])
+ if test x$with_hal = xyes ; then
+AC_DEFINE_UNQUOTED([HAVE_HAL], 1,
+ [use HAL for host device enumeration])
+
+old_CFLAGS=$CFLAGS
+old_LDFLAGS=$LDFLAGS
+CFLAGS=$CFLAGS $HAL_CFLAGS
+LDFLAGS=$LDFLAGS $HAL_LIBS
+AC_CHECK_FUNCS([libhal_get_all_devices],,[with_hal=no])
+CFLAGS=$old_CFLAGS
+LDFLAGS=$old_LDFLAGS
+ fi
+fi
+AM_CONDITIONAL([HAVE_HAL], [test x$with_hal = xyes])
+AC_SUBST([HAL_CFLAGS])
+AC_SUBST([HAL_LIBS])
+
+DEVKIT_REQUIRED=0.0
+DEVKIT_CFLAGS=
+DEVKIT_LIBS=
+AC_ARG_WITH([devkit],
+ [ --with-devkit use DeviceKit for host device enumeration],
+ [],
+ [with_devkit=check])
+
+dnl Extra check needed while devkit pkg-config info missing glib2 dependency
+PKG_CHECK_MODULES(GLIB2, glib-2.0 = 0.0,,[
+ if test x$with_devkit = xcheck; then
+with_devkit=no
+ elif test x$with_devkit = xyes; then
+AC_MSG_ERROR([required package DeviceKit requires package glib-2.0])
+ fi])
+
+if test x$with_devkit = xyes -o x$with_devkit = xcheck; then
+ PKG_CHECK_MODULES(DEVKIT, devkit-gobject = $DEVKIT_REQUIRED,
+[with_devkit=yes], [
+if test x$with_devkit = xcheck ; then
+ with_devkit=no
+else
+ AC_MSG_ERROR(
+ [You must install DeviceKit-devel = $DEVKIT_REQUIRED to compile libvirt])
+fi
+ ])
+ if test x$with_devkit = xyes ; then
+AC_DEFINE_UNQUOTED([HAVE_DEVKIT], 1,
+ [use DeviceKit for host device enumeration])
+
+dnl Add glib2 flags explicitly while devkit pkg-config info missing glib2 dependency
+DEVKIT_CFLAGS=$GLIB2_CFLAGS $DEVKIT_CFLAGS
+DEVKIT_LIBS=$GLIB2_LIBS $DEVKIT_LIBS
+
+dnl Add more flags apparently required for devkit to work properly
+DEVKIT_CFLAGS=$DEVKIT_CFLAGS -D_POSIX_PTHREAD_SEMANTICS -D_REENTRANT
+
+old_CFLAGS=$CFLAGS
+old_LDFLAGS=$LDFLAGS
+CFLAGS=$CFLAGS $DEVKIT_CFLAGS
+LDFLAGS=$LDFLAGS $DEVKIT_LIBS
+AC_CHECK_FUNCS([devkit_client_connect],,[with_devkit=no])
+CFLAGS=$old_CFLAGS
+LDFLAGS=$old_LDFLAGS
+ fi
+fi
+AM_CONDITIONAL([HAVE_DEVKIT], [test x$with_devkit = xyes])
+AC_SUBST([DEVKIT_CFLAGS])
+AC_SUBST([DEVKIT_LIBS])
+
+
# very annoying
rm -f COPYING
cp COPYING.LIB COPYING
@@ -1124,6 +1211,16 @@ AC_MSG_NOTICE([ numactl: $NUMACTL_CFLAGS $NUMACTL_LIBS])
else
AC_MSG_NOTICE([ numactl: no])
fi
+if test $with_hal = yes ; then
+AC_MSG_NOTICE([ hal: $HAL_CFLAGS $HAL_LIBS])
+else
+AC_MSG_NOTICE([ hal: no])
+fi
+if test $with_devkit = yes ; then
+AC_MSG_NOTICE([ devkit: $DEVKIT_CFLAGS $DEVKIT_LIBS])
+else
+AC_MSG_NOTICE([ devkit: no])
+fi
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Test suite])
AC_MSG_NOTICE([])
diff --git a/po/POTFILES.in b/po/POTFILES.in
index f671155..684d300 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -12,6 +12,7 @@ src/lxc_controller.c
src/lxc_driver.c
src/network_conf.c
src/network_driver.c
+src/node_device.c
src/openvz_conf.c
src/openvz_driver.c
src/proxy_internal.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 5a769f8..963a307 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,5 +1,22 @@
## Process this file with automake to produce Makefile.in
+if WITH_LIBVIRTD
+NODE_DEVICE_SOURCES = node_device.c node_device.h \
+ node_device_conf.c node_device_conf.h
+NODE_DEVICE_CFLAGS =
+NODE_DEVICE_LIBS =
+if HAVE_HAL
+NODE_DEVICE_CFLAGS += $(HAL_CFLAGS)
+NODE_DEVICE_LIBS += $(HAL_LIBS)
+NODE_DEVICE_SOURCES += node_device_hal.c
+endif
+if HAVE_DEVKIT
+NODE_DEVICE_CFLAGS += $(DEVKIT_CFLAGS)
+NODE_DEVICE_LIBS += $(DEVKIT_LIBS)
+NODE_DEVICE_SOURCES += node_device_devkit.c
+endif
+endif
+
INCLUDES = \
-I$(top_srcdir)/gnulib/lib -I../gnulib/lib \
-I../include \
@@ -10,6 +27,7 @@ INCLUDES = \
$(SASL_CFLAGS) \
$(SELINUX_CFLAGS) \
$(NUMACTL_CFLAGS) \
+ $(NODE_DEVICE_CFLAGS) \
-DBINDIR=\$(libexecdir)\ \
-DSBINDIR=\$(sbindir)\ \
-DSYSCONF_DIR=\$(sysconfdir)\ \
@@ -146,6 +164,7 @@ libvirt_la_SOURCES = \
libvirt.c \
$(GENERIC_LIB_SOURCES)\
$(DOMAIN_CONF_SOURCES)\
+
[libvirt] Re: [PATCH] 4/7 host (node) device enumeration - remote driver
This patch contains the remote implementation of node device
enumeration.
diff --git a/qemud/remote.c b/qemud/remote.c
index a623494..56d88a7 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -66,6 +66,7 @@ static void make_nonnull_domain (remote_nonnull_domain *dom_dst, virDomainPtr do
static void make_nonnull_network (remote_nonnull_network *net_dst, virNetworkPtr net_src);
static void make_nonnull_storage_pool (remote_nonnull_storage_pool *pool_dst, virStoragePoolPtr pool_src);
static void make_nonnull_storage_vol (remote_nonnull_storage_vol *vol_dst, virStorageVolPtr vol_src);
+static void make_nonnull_node_device (remote_nonnull_node_device *dev_dst, virNodeDevicePtr dev_src);
#include remote_dispatch_prototypes.h
@@ -3640,6 +3641,303 @@ remoteDispatchStorageVolLookupByPath (struct qemud_server *server ATTRIBUTE_UNUS
}
+/***
+ * NODE INFO APIS
+ **/
+
+static int
+remoteDispatchNodeNumOfDevices (struct qemud_server *server ATTRIBUTE_UNUSED,
+struct qemud_client *client,
+remote_message_header *req,
+remote_node_num_of_devices_args *args,
+remote_node_num_of_devices_ret *ret)
+{
+CHECK_CONN(client);
+
+ret-num = virNodeNumOfDevices (client-conn, args-flags);
+if (ret-num == -1) return -1;
+
+return 0;
+}
+
+
+static int
+remoteDispatchNodeListDevices (struct qemud_server *server ATTRIBUTE_UNUSED,
+ struct qemud_client *client,
+ remote_message_header *req,
+ remote_node_list_devices_args *args,
+ remote_node_list_devices_ret *ret)
+{
+CHECK_CONN(client);
+
+if (args-maxnames REMOTE_NODE_DEVICE_NAME_LIST_MAX) {
+remoteDispatchError (client, req,
+ %s, _(maxnames REMOTE_NODE_DEVICE_NAME_LIST_MAX));
+return -2;
+}
+
+/* Allocate return buffer. */
+if (VIR_ALLOC_N(ret-names.names_val, args-maxnames) 0) {
+remoteDispatchSendError(client, req, VIR_ERR_NO_MEMORY, NULL);
+return -2;
+}
+
+ret-names.names_len =
+virNodeListDevices (client-conn,
+ret-names.names_val, args-maxnames, args-flags);
+if (ret-names.names_len == -1) {
+VIR_FREE(ret-names.names_val);
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+remoteDispatchNodeNumOfDevicesByCap (struct qemud_server *server ATTRIBUTE_UNUSED,
+ struct qemud_client *client,
+ remote_message_header *req,
+ remote_node_num_of_devices_by_cap_args *args,
+ remote_node_num_of_devices_by_cap_ret *ret)
+{
+CHECK_CONN(client);
+
+ret-num = virNodeNumOfDevicesByCap (client-conn, args-cap, args-flags);
+if (ret-num == -1) return -1;
+
+return 0;
+}
+
+
+static int
+remoteDispatchNodeListDevicesByCap (struct qemud_server *server ATTRIBUTE_UNUSED,
+struct qemud_client *client,
+remote_message_header *req,
+remote_node_list_devices_by_cap_args *args,
+remote_node_list_devices_by_cap_ret *ret)
+{
+CHECK_CONN(client);
+
+if (args-maxnames REMOTE_NODE_DEVICE_NAME_LIST_MAX) {
+remoteDispatchError (client, req,
+ %s, _(maxnames REMOTE_NODE_DEVICE_NAME_LIST_MAX));
+return -2;
+}
+
+/* Allocate return buffer. */
+if (VIR_ALLOC_N(ret-names.names_val, args-maxnames) 0) {
+remoteDispatchSendError(client, req, VIR_ERR_NO_MEMORY, NULL);
+return -2;
+}
+
+ret-names.names_len =
+virNodeListDevicesByCap (client-conn, args-cap,
+ ret-names.names_val, args-maxnames,
+ args-flags);
+if (ret-names.names_len == -1) {
+VIR_FREE(ret-names.names_val);
+return -1;
+}
+
+return 0;
+}
+
+
+static int
+remoteDispatchNodeDeviceLookupByName (struct qemud_server *server ATTRIBUTE_UNUSED,
+ struct qemud_client *client,
+ remote_message_header *req,
+ remote_node_device_lookup_by_name_args *args,
+ remote_node_device_lookup_by_name_ret *ret)
+{
+virNodeDevicePtr dev;
+
+CHECK_CONN(client);
+
+dev = virNodeDeviceLookupByName (client-conn, args-name);
+if (dev == NULL) return -1;
+
+make_nonnull_node_device (ret-dev, dev);
+virNodeDeviceFree(dev);
+return 0;
+}
+
+
+static int
[libvirt] Re: [PATCH] 5/7 host (node) device enumeration - virsh support
This patch contains virsh support for node device enumeration.
diff --git a/src/virsh.c b/src/virsh.c
index 89aa4fa..67963f0 100644
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -4415,6 +4415,83 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
}
/*
+ * node-list-devices command
+ */
+static const vshCmdInfo info_node_list_devices[] = {
+{syntax, node-list-devices},
+{help, gettext_noop(enumerate devices on this host)},
+{NULL, NULL}
+};
+
+static int
+cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
+{
+char **devices;
+int num_devices, i;
+
+if (!vshConnectionUsability(ctl, ctl-conn, TRUE))
+return FALSE;
+
+num_devices = virNodeNumOfDevices(ctl-conn, 0);
+if (num_devices 0) {
+vshError(ctl, FALSE, %s, _(Failed to count node devices));
+return FALSE;
+} else if (num_devices == 0) {
+return TRUE;
+}
+
+devices = vshMalloc(ctl, sizeof(char *) * num_devices);
+num_devices = virNodeListDevices(ctl-conn, devices, num_devices, 0);
+if (num_devices 0) {
+vshError(ctl, FALSE, %s, _(Failed to list node devices));
+free(devices);
+return FALSE;
+}
+for (i = 0; i num_devices; i++) {
+vshPrint(ctl, %s\n, devices[i]);
+free(devices[i]);
+}
+free(devices);
+return TRUE;
+}
+
+/*
+ * node-device-dumpxml command
+ */
+static const vshCmdInfo info_node_device_dumpxml[] = {
+{syntax, node-device-dumpxml device},
+{help, gettext_noop(node device details in XML)},
+{desc, gettext_noop(Output the node device details as an XML dump to stdout.)},
+{NULL, NULL}
+};
+
+
+static const vshCmdOptDef opts_node_device_dumpxml[] = {
+{device, VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop(device key)},
+{NULL, 0, 0, NULL}
+};
+
+static int
+cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd)
+{
+const char *name;
+virNodeDevicePtr device;
+
+if (!vshConnectionUsability(ctl, ctl-conn, TRUE))
+return FALSE;
+if (!(name = vshCommandOptString(cmd, device, NULL)))
+return FALSE;
+if (!(device = virNodeDeviceLookupByName(ctl-conn, name))) {
+vshError(ctl, FALSE, %s '%s', _(Could not find matching device), name);
+return FALSE;
+}
+
+vshPrint(ctl, %s\n, virNodeDeviceGetXMLDesc(device, 0));
+virNodeDeviceFree(device);
+return TRUE;
+}
+
+/*
* hostkey command
*/
static const vshCmdInfo info_hostname[] = {
@@ -5566,6 +5643,9 @@ static const vshCmdDef commands[] = {
{net-uuid, cmdNetworkUuid, opts_network_uuid, info_network_uuid},
{nodeinfo, cmdNodeinfo, NULL, info_nodeinfo},
+{node-list-devices, cmdNodeListDevices, NULL, info_node_list_devices},
+{node-device-dumpxml, cmdNodeDeviceDumpXML, opts_node_device_dumpxml, info_node_device_dumpxml},
+
{pool-autostart, cmdPoolAutostart, opts_pool_autostart, info_pool_autostart},
{pool-build, cmdPoolBuild, opts_pool_build, info_pool_build},
{pool-create, cmdPoolCreate, opts_pool_create, info_pool_create},
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [PATCH] 6/7 host (node) device enumeration - python bindings
This patch contains python bindings for node device enumeration.
diff --git a/python/generator.py b/python/generator.py
index ca83eaf..108dfc2 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -258,6 +258,11 @@ py_types = {
'const virConnectPtr': ('O', virConnect, virConnectPtr, virConnectPtr),
'virConnect *': ('O', virConnect, virConnectPtr, virConnectPtr),
'const virConnect *': ('O', virConnect, virConnectPtr, virConnectPtr),
+
+'virNodeDevicePtr': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'const virNodeDevicePtr': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'virNodeDevice *': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'const virNodeDevice *': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
}
py_return_types = {
@@ -315,6 +320,9 @@ skip_impl = (
'virStoragePoolListVolumes',
'virDomainBlockPeek',
'virDomainMemoryPeek',
+'virNodeListDevicesByCap',
+'virNodeListDevices',
+'virNodeDeviceListCaps',
)
@@ -599,6 +607,8 @@ classes_type = {
virStoragePool *: (._o, virStoragePool(self, _obj=%s), virStoragePool),
virStorageVolPtr: (._o, virStorageVol(self, _obj=%s), virStorageVol),
virStorageVol *: (._o, virStorageVol(self, _obj=%s), virStorageVol),
+virNodeDevicePtr: (._o, virNodeDevice(self, _obj=%s), virNodeDevice),
+virNodeDevice *: (._o, virNodeDevice(self, _obj=%s), virNodeDevice),
virConnectPtr: (._o, virConnect(_obj=%s), virConnect),
virConnect *: (._o, virConnect(_obj=%s), virConnect),
}
@@ -606,7 +616,8 @@ classes_type = {
converter_type = {
}
-primary_classes = [virDomain, virNetwork, virStoragePool, virStorageVol, virConnect]
+primary_classes = [virDomain, virNetwork, virStoragePool, virStorageVol,
+ virConnect, virNodeDevice ]
classes_ancestor = {
}
@@ -616,6 +627,7 @@ classes_destructors = {
virStoragePool: virStoragePoolFree,
virStorageVol: virStorageVolFree,
virConnect: virConnectClose,
+virNodeDevice : virNodeDeviceFree
}
functions_noexcept = {
@@ -625,6 +637,8 @@ functions_noexcept = {
'virStoragePoolGetName': True,
'virStorageVolGetName': True,
'virStorageVolGetkey': True,
+'virNodeDeviceGetName': True,
+'virNodeDeviceGetParent': True,
}
reference_keepers = {
@@ -705,6 +719,13 @@ def nameFixup(name, classe, type, file):
elif name[0:13] == virStorageVol:
func = name[13:]
func = string.lower(func[0:1]) + func[1:]
+elif name[0:13] == virNodeDevice:
+if name[13:16] == Get:
+func = string.lower(name[16]) + name[17:]
+elif name[13:19] == Lookup or name[13:] == Create:
+func = string.lower(name[3]) + name[4:]
+else:
+func = string.lower(name[13]) + name[14:]
elif name[0:7] == virNode:
func = name[7:]
func = string.lower(func[0:1]) + func[1:]
@@ -957,7 +978,7 @@ def buildWrappers():
else:
txt.write(Class %s()\n % (classname))
classes.write(class %s:\n % (classname))
-if classname in [ virDomain, virNetwork, virStoragePool, virStorageVol ]:
+if classname in [ virDomain, virNetwork, virStoragePool, virStorageVol, virNodeDevice ]:
classes.write(def __init__(self, conn, _obj=None):\n)
else:
classes.write(def __init__(self, _obj=None):\n)
@@ -965,7 +986,7 @@ def buildWrappers():
list = reference_keepers[classname]
for ref in list:
classes.write(self.%s = None\n % ref[1])
-if classname in [ virDomain, virNetwork ]:
+if classname in [ virDomain, virNetwork, virNodeDevice ]:
classes.write(self._conn = conn\n)
elif classname in [ virStorageVol, virStoragePool ]:
classes.write(self._conn = conn\n + \
diff --git a/python/libvir.c b/python/libvir.c
index 9cc0c81..7b2792e 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -1466,7 +1466,130 @@ libvirt_virStoragePoolLookupByUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *ar
return(py_retval);
}
+static PyObject *
+libvirt_virNodeListDevices(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args) {
+PyObject *py_retval;
+char **names = NULL;
+int c_retval, i;
+virConnectPtr conn;
+PyObject *pyobj_conn;
+unsigned int flags;
+
+
+if (!PyArg_ParseTuple(args, (char *)Oi:virNodeListDevices, pyobj_conn, flags))
+return(NULL);
+conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+c_retval = virNodeNumOfDevices(conn, flags);
+if (c_retval 0)
+return VIR_PY_NONE;
+
+if (c_retval) {
+names = malloc(sizeof(*names) * c_retval);
+if (!names)
+return VIR_PY_NONE;
+c_retval = virNodeListDevices(conn, names, c_retval, flags);
+if (c_retval 0)
[libvirt] Re: [PATCH] 7/7 host (node) device enumeration - java bindings
This patch (for libvirt-java) contains Java bindings for node device
enumeration.
diff --git a/src/Makefile.am b/src/Makefile.am
index 5200c1d..466a0eb 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -14,6 +14,7 @@ java_libvirt_source_files = \
org/libvirt/Error.java \
org/libvirt/Network.java \
org/libvirt/NodeInfo.java \
+ org/libvirt/NodeDevice.java \
org/libvirt/SchedBooleanParameter.java \
org/libvirt/SchedDoubleParameter.java \
org/libvirt/SchedIntParameter.java \
diff --git a/src/jni/Makefile.am b/src/jni/Makefile.am
index 6e19384..8feed24 100644
--- a/src/jni/Makefile.am
+++ b/src/jni/Makefile.am
@@ -10,7 +10,8 @@ GENERATED = \
org_libvirt_StoragePool.h \
org_libvirt_StorageVol_Type.h \
org_libvirt_StorageVol_DeleteFlags.h \
- org_libvirt_StorageVol.h
+ org_libvirt_StorageVol.h \
+ org_libvirt_NodeDevice.h
BUILT_SOURCES = $(GENERATED)
@@ -31,6 +32,9 @@ org_libvirt_StoragePool.h org_libvirt_StoragePool_BuildFlags.h org_libvirt_Stora
org_libvirt_StorageVol.h org_libvirt_StorageVol_Type.h org_libvirt_StorageVol_DeleteFlags.h : $(JAVA_CLASS_ROOT)/org/libvirt/StorageVol.class
$(JAVAH) -classpath $(JAVA_CLASS_ROOT) org.libvirt.StorageVol
+org_libvirt_NodeDevice.h : $(JAVA_CLASS_ROOT)/org/libvirt/NodeDevice.class
+ $(JAVAH) -classpath $(JAVA_CLASS_ROOT) org.libvirt.NodeDevice
+
lib_LTLIBRARIES = libvirt_jni.la
libvirt_jni_la_SOURCES = \
org_libvirt_Network.c \
@@ -38,6 +42,7 @@ libvirt_jni_la_SOURCES = \
org_libvirt_Domain.c \
org_libvirt_StoragePool.c \
org_libvirt_StorageVol.c \
+ org_libvirt_NodeDevice.c \
generic.h \
ErrorHandler.c \
ErrorHandler.h \
diff --git a/src/jni/generic.h b/src/jni/generic.h
index 347c076..e1da179 100644
--- a/src/jni/generic.h
+++ b/src/jni/generic.h
@@ -70,6 +70,16 @@
return retval;
/*
+ * Generic macro with a VIROBJ and a String and an int arguments
+ * returning an int
+ */
+#define GENERIC_VIROBJ_STRING_INT__INT(ENV, OBJ, VIROBJ, J_STR, ARG1, VIRFUNC1) \
+ const char *str=(*ENV)-GetStringUTFChars(ENV, J_STR, NULL); \
+ jint retval = (jlong)VIRFUNC1(VIROBJ, str, ARG1); \
+ (*ENV)-ReleaseStringUTFChars(ENV, J_STR, str); \
+ return retval;
+
+/*
* Generic macro with a VIROBJ and an String arguments returning a virObject
* (for functions like *CreateXML* that take no flags)
*/
@@ -81,15 +91,14 @@
/*
* Generic macro with a VIROBJ and String and int arguments returning a
- * virObject (for functions like *CreateXML* that take a flags)
+ * virObject
*/
-#define GENERIC_VIROBJ_STRING_INT__VIROBJ(ENV, OBJ, VIROBJ, J_XMLDESC, FLAGS, VIRFUNC1) \
- const char *xmlDesc=(*ENV)-GetStringUTFChars(ENV, J_XMLDESC, NULL); \
- jlong retval = (jlong)VIRFUNC1(VIROBJ, xmlDesc, FLAGS); \
- (*ENV)-ReleaseStringUTFChars(ENV, J_XMLDESC, xmlDesc); \
+#define GENERIC_VIROBJ_STRING_INT__VIROBJ(ENV, OBJ, VIROBJ, J_STR, ARG1, VIRFUNC1) \
+ const char *str=(*ENV)-GetStringUTFChars(ENV, J_STR, NULL); \
+ jlong retval = (jlong)VIRFUNC1(VIROBJ, str, ARG1); \
+ (*ENV)-ReleaseStringUTFChars(ENV, J_STR, str); \
return retval;
-
/*
* Generic macro for the *getAutoStart functions
*/
@@ -150,6 +159,61 @@
return j_names;
/*
+ * Generic macro for the *List* functions that return an array of strings
+ * and take a single int arg.
+ * VIRFUNC1 is the *List* function
+ * VIRFUNC2 is the corresponding *NumOf* function
+ */
+#define GENERIC_INT__LIST_STRINGARRAY(ENV, OBJ, VIROBJ, JINT, VF1, VF2) \
+ int maxnames; \
+ char **names; \
+ int c;\
+ jobjectArray j_names=NULL; \
+ if((maxnames = VF2(VIROBJ, JINT))0)\
+ return NULL; \
+ names= (char**)calloc(maxnames, sizeof(char*)); \
+ if(VF1(VIROBJ, names, maxnames, JINT)=0){ \
+ j_names= (jobjectArray)(*ENV)-NewObjectArray(ENV, maxnames, \
+ (*ENV)-FindClass(ENV,java/lang/String), \
+ (*ENV)-NewStringUTF(ENV,)); \
+ for(c=0; cmaxnames; c++){\
+ (*ENV)-SetObjectArrayElement(ENV, j_names, c, \
+ (*ENV)-NewStringUTF(ENV, names[c])); \
+ } \
+ }\
+ free(names); \
+ return j_names;
+
+/*
+ * Generic macro for the *List* functions that return an array of strings
+ * and take a single string arg and an int arg.
+ * VIRFUNC1 is the *List* function
+ * VIRFUNC2 is the corresponding *NumOf* function
+ */
+#define GENERIC_STRING_INT__LIST_STRINGARRAY(ENV, OBJ, VIROBJ, J_STR, JINT, VF1, VF2) \
+ int maxnames; \
+ char **names; \
+ int c;\
+ jobjectArray j_names=NULL; \
+ const char *str = (*ENV)-GetStringUTFChars(ENV, J_STR, NULL); \
+ if((maxnames = VF2(VIROBJ, str, JINT))0) \
+ goto cleanup; \
+ names= (char**)calloc(maxnames, sizeof(char*)); \
+ if(VF1(VIROBJ, str, names, maxnames, JINT)=0){ \
+ j_names= (jobjectArray)(*ENV)-NewObjectArray(ENV, maxnames, \
+ (*ENV)-FindClass(ENV,java/lang/String), \
+ (*ENV)-NewStringUTF(ENV,)); \
+ for(c=0; cmaxnames; c++){\
+ (*ENV)-SetObjectArrayElement(ENV, j_names, c, \
+ (*ENV)-NewStringUTF(ENV,
[libvirt] [PATCH] fix JNI flags in libvirt-java build
This tiny patch adds $LIBVIRT_CFLAGS to the flags used when compiling the Java Native Interface C code in libvirt-java. This allows one to build, for example, against a local (non-installed) libvirt tree. Dave commit dfd82305b4c871244dff2f9057e8b73a043c0213 Author: David Lively [EMAIL PROTECTED] Date: Fri Oct 3 09:28:38 2008 -0400 vi-patch: fix-jni-cflags Include LIBVIRT_CFLAGS in the CFLAGS used for the libvirt JNI sources. diff --git a/src/jni/Makefile.am b/src/jni/Makefile.am index 8feed24..ad1158a 100644 --- a/src/jni/Makefile.am +++ b/src/jni/Makefile.am @@ -52,7 +52,7 @@ libvirt_jni_la_SOURCES = \ nodist_libvirt_jni_la_SOURCES = $(GENERATED) libvirt_jni_la_LIBADD = $(LIBVIRT_LIBS) libvirt_jni_la_LDFLAGS = -version-info @JNI_VERSION_INFO@ -libvirt_jni_la_CFLAGS = @JNI_CFLAGS@ +libvirt_jni_la_CFLAGS = $(LIBVIRT_CFLAGS) @JNI_CFLAGS@ CLEANFILES = \ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] fix memory leak in __virStringListFree
Currently __virStringListFree is freeing only the list nodes, but not
the strings on the list (and neither is anyone else freeing these).
This small patch fixes that, and documents the two __virStringList
functions.
Dave
diff --git a/src/libvirt.c b/src/libvirt.c
index 8fd594b..bee98d2 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5308,8 +5308,18 @@ virStorageVolGetPath(virStorageVolPtr vol)
-/* Not for public use. Combines the elements of a virStringList
+/**
+ * __virStringListJoin:
+ * @list: the StringList
+ * @pre: string to prepend to the result string
+ * @post: string to append to the result string
+ * @sep: string to separate the list elements
+ *
+ * Internal function to combine the elements of a virStringList
* into a single string.
+ *
+ * Returns the resulting string, which the caller is responsible
+ * for freeing when they're done with it. Returns NULL on error.
*/
char *__virStringListJoin(const virStringList *list, const char *pre,
const char *post, const char *sep)
@@ -5338,10 +5348,17 @@ char *__virStringListJoin(const virStringList *list, const char *pre,
}
+/**
+ * __virStringListFree:
+ * @list: the StringList to free
+ *
+ * Internal function to free the memory used by a string list.
+ */
void __virStringListFree(virStringList *list)
{
while (list) {
virStringList *p = list-next;
+VIR_FREE(list-val);
VIR_FREE(list);
list = p;
}
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [PATCH 7/6] host (node) device enumeration - Java bindings
This patch (for libvirt-java) implements Java bindings for the node
device enumeration functions.
Dave
diff --git a/src/Makefile.am b/src/Makefile.am
index 5200c1d..466a0eb 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -14,6 +14,7 @@ java_libvirt_source_files = \
org/libvirt/Error.java \
org/libvirt/Network.java \
org/libvirt/NodeInfo.java \
+ org/libvirt/NodeDevice.java \
org/libvirt/SchedBooleanParameter.java \
org/libvirt/SchedDoubleParameter.java \
org/libvirt/SchedIntParameter.java \
diff --git a/src/jni/Makefile.am b/src/jni/Makefile.am
index 6e19384..8feed24 100644
--- a/src/jni/Makefile.am
+++ b/src/jni/Makefile.am
@@ -10,7 +10,8 @@ GENERATED = \
org_libvirt_StoragePool.h \
org_libvirt_StorageVol_Type.h \
org_libvirt_StorageVol_DeleteFlags.h \
- org_libvirt_StorageVol.h
+ org_libvirt_StorageVol.h \
+ org_libvirt_NodeDevice.h
BUILT_SOURCES = $(GENERATED)
@@ -31,6 +32,9 @@ org_libvirt_StoragePool.h org_libvirt_StoragePool_BuildFlags.h org_libvirt_Stora
org_libvirt_StorageVol.h org_libvirt_StorageVol_Type.h org_libvirt_StorageVol_DeleteFlags.h : $(JAVA_CLASS_ROOT)/org/libvirt/StorageVol.class
$(JAVAH) -classpath $(JAVA_CLASS_ROOT) org.libvirt.StorageVol
+org_libvirt_NodeDevice.h : $(JAVA_CLASS_ROOT)/org/libvirt/NodeDevice.class
+ $(JAVAH) -classpath $(JAVA_CLASS_ROOT) org.libvirt.NodeDevice
+
lib_LTLIBRARIES = libvirt_jni.la
libvirt_jni_la_SOURCES = \
org_libvirt_Network.c \
@@ -38,6 +42,7 @@ libvirt_jni_la_SOURCES = \
org_libvirt_Domain.c \
org_libvirt_StoragePool.c \
org_libvirt_StorageVol.c \
+ org_libvirt_NodeDevice.c \
generic.h \
ErrorHandler.c \
ErrorHandler.h \
diff --git a/src/jni/generic.h b/src/jni/generic.h
index 347c076..e1da179 100644
--- a/src/jni/generic.h
+++ b/src/jni/generic.h
@@ -70,6 +70,16 @@
return retval;
/*
+ * Generic macro with a VIROBJ and a String and an int arguments
+ * returning an int
+ */
+#define GENERIC_VIROBJ_STRING_INT__INT(ENV, OBJ, VIROBJ, J_STR, ARG1, VIRFUNC1) \
+ const char *str=(*ENV)-GetStringUTFChars(ENV, J_STR, NULL); \
+ jint retval = (jlong)VIRFUNC1(VIROBJ, str, ARG1); \
+ (*ENV)-ReleaseStringUTFChars(ENV, J_STR, str); \
+ return retval;
+
+/*
* Generic macro with a VIROBJ and an String arguments returning a virObject
* (for functions like *CreateXML* that take no flags)
*/
@@ -81,15 +91,14 @@
/*
* Generic macro with a VIROBJ and String and int arguments returning a
- * virObject (for functions like *CreateXML* that take a flags)
+ * virObject
*/
-#define GENERIC_VIROBJ_STRING_INT__VIROBJ(ENV, OBJ, VIROBJ, J_XMLDESC, FLAGS, VIRFUNC1) \
- const char *xmlDesc=(*ENV)-GetStringUTFChars(ENV, J_XMLDESC, NULL); \
- jlong retval = (jlong)VIRFUNC1(VIROBJ, xmlDesc, FLAGS); \
- (*ENV)-ReleaseStringUTFChars(ENV, J_XMLDESC, xmlDesc); \
+#define GENERIC_VIROBJ_STRING_INT__VIROBJ(ENV, OBJ, VIROBJ, J_STR, ARG1, VIRFUNC1) \
+ const char *str=(*ENV)-GetStringUTFChars(ENV, J_STR, NULL); \
+ jlong retval = (jlong)VIRFUNC1(VIROBJ, str, ARG1); \
+ (*ENV)-ReleaseStringUTFChars(ENV, J_STR, str); \
return retval;
-
/*
* Generic macro for the *getAutoStart functions
*/
@@ -150,6 +159,61 @@
return j_names;
/*
+ * Generic macro for the *List* functions that return an array of strings
+ * and take a single int arg.
+ * VIRFUNC1 is the *List* function
+ * VIRFUNC2 is the corresponding *NumOf* function
+ */
+#define GENERIC_INT__LIST_STRINGARRAY(ENV, OBJ, VIROBJ, JINT, VF1, VF2) \
+ int maxnames; \
+ char **names; \
+ int c;\
+ jobjectArray j_names=NULL; \
+ if((maxnames = VF2(VIROBJ, JINT))0)\
+ return NULL; \
+ names= (char**)calloc(maxnames, sizeof(char*)); \
+ if(VF1(VIROBJ, names, maxnames, JINT)=0){ \
+ j_names= (jobjectArray)(*ENV)-NewObjectArray(ENV, maxnames, \
+ (*ENV)-FindClass(ENV,java/lang/String), \
+ (*ENV)-NewStringUTF(ENV,)); \
+ for(c=0; cmaxnames; c++){\
+ (*ENV)-SetObjectArrayElement(ENV, j_names, c, \
+ (*ENV)-NewStringUTF(ENV, names[c])); \
+ } \
+ }\
+ free(names); \
+ return j_names;
+
+/*
+ * Generic macro for the *List* functions that return an array of strings
+ * and take a single string arg and an int arg.
+ * VIRFUNC1 is the *List* function
+ * VIRFUNC2 is the corresponding *NumOf* function
+ */
+#define GENERIC_STRING_INT__LIST_STRINGARRAY(ENV, OBJ, VIROBJ, J_STR, JINT, VF1, VF2) \
+ int maxnames; \
+ char **names; \
+ int c;\
+ jobjectArray j_names=NULL; \
+ const char *str = (*ENV)-GetStringUTFChars(ENV, J_STR, NULL); \
+ if((maxnames = VF2(VIROBJ, str, JINT))0) \
+ goto cleanup; \
+ names= (char**)calloc(maxnames, sizeof(char*)); \
+ if(VF1(VIROBJ, str, names, maxnames, JINT)=0){ \
+ j_names= (jobjectArray)(*ENV)-NewObjectArray(ENV, maxnames, \
+ (*ENV)-FindClass(ENV,java/lang/String), \
+ (*ENV)-NewStringUTF(ENV,)); \
+ for(c=0; cmaxnames; c++){\
+ (*ENV)-SetObjectArrayElement(ENV, j_names, c, \
+
[libvirt] Re: [PATCH 0/6] host (node) device enumeration, take two
On Thu, 2008-10-23 at 13:53 +0100, Daniel P. Berrange wrote:
On Tue, Oct 21, 2008 at 01:45:47PM -0400, David Lively wrote:
* using newfangled array-based lists for NodeDeviceObj's
The individual capabilities within a device are still using a linked list,
though that's not a critical problem from the thread safety point of view.
Right. This was intentional, since there are no per-capability
operations (and hence no need for per-capability locking granularity).
* subscribe to HAL events update dev state from them (next for me?)
BTW, I'm working on this now. Should have something to submit in
another day or two.
* implement pci_bus/usb_bus capabilities (for PCI/USB controllers)
While on the subject of USB, I've just noticed that HAL seems to
expose 2 devices for every USB device - 'usb' and 'usb_device',
which we're mapping into just a 'usb' capability. Unless there's
compelling reason to have both we might consider just filtering
out one of the two.
Yeah, I meant to bring this up. HAL uses usb_device for the USB
devices, and usb for the USB interface(s) provided by the device. The
usb_device is the parent of the usb interface(s). The HAL usb
namespace mirrors the attributes of the parent usb_device, and adds a
few of its own, specific to the interface. Right now I'm reporting both
mostly so the device hierarchy (as defined by the parent relation) is
consistent.
I see two choices:
(a) Just report the interface (usb) objects and fixup their parent to
be the parent of their owning USB device. Each interface object (as in
HAL) will reflect the details of the parent USB device, as well as the
interface-specific bits.
(b) Split libvirt's usb capability into usb_device and
usb_interface. A usb_interface will always have a usb_device as it's
parent (and I would *not* replicate the usb_device details in the
usb_interface - that's the whole reason for keeping it as a separate
device).
I'm leaning fairly strongly towards (b). What do you all think?
* DeviceKit implementation is barely a proof of concept
Noticed one problem - on my build it refuses to enumerate devices
if you pass in a NULL for subsystem name list. I made a really
quick dirty hack to just try it out
@@ -300,13 +301,18 @@ static int devkitNodeDriverStartup(void)
}
/* Populate with known devices */
-devs = devkit_client_enumerate_by_subsystem(devkit_client, NULL, err);
-if (err) {
-DEBUG0(devkit_client_enumerate_by_subsystem failed);
-devs = NULL;
-goto failure;
+for (i = 0 ; i ARRAY_CARDINALITY(caps_tbl) ; i++) {
+const char *caps[] = { caps_tbl[i].cap_name, NULL };
+devs = devkit_client_enumerate_by_subsystem(devkit_client,
+caps,
+err);
+if (err) {
+DEBUG0(devkit_client_enumerate_by_subsystem failed);
+devs = NULL;
+goto failure;
+}
+g_list_foreach(devs, dev_create, devkit_client);
Oh yeah. I forgot I fixed devkit_client_enumerate_by_subsystem to work
as advertised with a NULL subsystem. I submitted the fix a couple
months ago to [EMAIL PROTECTED], but never got any
acknowledgment, and haven't seen any activity on the list whatsoever, so
I'm not surprised it hasn't made it in. Any idea where I should submit
devkit fixes?
In any case, I'll put your workaround in for now. It won't pick up
devices in subsystems not listed in caps_tbl, but those are fairly
useless devices anyway (since we won't recognize any device caps if we
don't explicitly handle that subsystem in caps_tbl).
}
-g_list_foreach(devs, dev_create, devkit_client);
driverState-privateData = devkit_client;
* need to resolve naming other issues (see
https://www.redhat.com/archives/libvir-list/2008-September/msg00430.html
* ... and then fill in impl (no hurry; devkit immature now)
I'm still wondering if it is worth us santizing the device names ourselves
somehow, though it might end up being rather a large job. Will have to
think some more about it.
Yeah, it's a nasty issue. In a lot of ways, the HAL names are pretty
nice. For example, I really like NICs named by MAC address (so knowing
that a particular NIC is eth0 is *not* encoded in the device name, but
rather in a capability). This is also much nicer than naming by (e.g.)
sysfs path, which encodes too much info about where the card is plugged
in (that's what parent info is for).
But I don't see any sign that Devkit is exposing HALish names, though
I'd imagine (if only to make HAL-Devkit transition easier) that they
will need to expose a device property like HAL_UDI. With no activity
on the devkit-devel list, I'm not sure where they're going.
Thanks for the comments. I'll incorporate your devkit workaround, and
wait on further comment before submitting the next take.
Dave
Re: [libvirt] Re: [PATCH 6/6] host (node) device enumeration - python bindings
Ok, here are proper python bindings to the node device enumeration
functions and objects.
diff --git a/python/generator.py b/python/generator.py
index c706b19..0186e6e 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -258,6 +258,11 @@ py_types = {
'const virConnectPtr': ('O', virConnect, virConnectPtr, virConnectPtr),
'virConnect *': ('O', virConnect, virConnectPtr, virConnectPtr),
'const virConnect *': ('O', virConnect, virConnectPtr, virConnectPtr),
+
+'virNodeDevicePtr': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'const virNodeDevicePtr': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'virNodeDevice *': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'const virNodeDevice *': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
}
py_return_types = {
@@ -315,6 +320,9 @@ skip_impl = (
'virStoragePoolListVolumes',
'virDomainBlockPeek',
'virDomainMemoryPeek',
+'virNodeListDevicesByCap',
+'virNodeListDevices',
+'virNodeDeviceListCaps',
)
@@ -596,6 +604,8 @@ classes_type = {
virStoragePool *: (._o, virStoragePool(self, _obj=%s), virStoragePool),
virStorageVolPtr: (._o, virStorageVol(self, _obj=%s), virStorageVol),
virStorageVol *: (._o, virStorageVol(self, _obj=%s), virStorageVol),
+virNodeDevicePtr: (._o, virNodeDevice(self, _obj=%s), virNodeDevice),
+virNodeDevice *: (._o, virNodeDevice(self, _obj=%s), virNodeDevice),
virConnectPtr: (._o, virConnect(_obj=%s), virConnect),
virConnect *: (._o, virConnect(_obj=%s), virConnect),
}
@@ -603,7 +613,8 @@ classes_type = {
converter_type = {
}
-primary_classes = [virDomain, virNetwork, virStoragePool, virStorageVol, virConnect]
+primary_classes = [virDomain, virNetwork, virStoragePool, virStorageVol,
+ virConnect, virNodeDevice ]
classes_ancestor = {
}
@@ -613,6 +624,7 @@ classes_destructors = {
virStoragePool: virStoragePoolFree,
virStorageVol: virStorageVolFree,
virConnect: virConnectClose,
+virNodeDevice : virNodeDeviceFree
}
functions_noexcept = {
@@ -622,6 +634,8 @@ functions_noexcept = {
'virStoragePoolGetName': True,
'virStorageVolGetName': True,
'virStorageVolGetkey': True,
+'virNodeDeviceGetName': True,
+'virNodeDeviceGetParent': True,
}
reference_keepers = {
@@ -702,6 +716,13 @@ def nameFixup(name, classe, type, file):
elif name[0:13] == virStorageVol:
func = name[13:]
func = string.lower(func[0:1]) + func[1:]
+elif name[0:13] == virNodeDevice:
+if name[13:16] == Get:
+func = string.lower(name[16]) + name[17:]
+elif name[13:19] == Lookup or name[13:] == Create:
+func = string.lower(name[3]) + name[4:]
+else:
+func = string.lower(name[13]) + name[14:]
elif name[0:7] == virNode:
func = name[7:]
func = string.lower(func[0:1]) + func[1:]
@@ -954,7 +975,7 @@ def buildWrappers():
else:
txt.write(Class %s()\n % (classname))
classes.write(class %s:\n % (classname))
-if classname in [ virDomain, virNetwork, virStoragePool, virStorageVol ]:
+if classname in [ virDomain, virNetwork, virStoragePool, virStorageVol, virNodeDevice ]:
classes.write(def __init__(self, conn, _obj=None):\n)
else:
classes.write(def __init__(self, _obj=None):\n)
@@ -962,7 +983,7 @@ def buildWrappers():
list = reference_keepers[classname]
for ref in list:
classes.write(self.%s = None\n % ref[1])
-if classname in [ virDomain, virNetwork ]:
+if classname in [ virDomain, virNetwork, virNodeDevice ]:
classes.write(self._conn = conn\n)
elif classname in [ virStorageVol, virStoragePool ]:
classes.write(self._conn = conn\n + \
diff --git a/python/libvir.c b/python/libvir.c
index 9cc0c81..7b2792e 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -1466,7 +1466,130 @@ libvirt_virStoragePoolLookupByUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *ar
return(py_retval);
}
+static PyObject *
+libvirt_virNodeListDevices(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args) {
+PyObject *py_retval;
+char **names = NULL;
+int c_retval, i;
+virConnectPtr conn;
+PyObject *pyobj_conn;
+unsigned int flags;
+
+
+if (!PyArg_ParseTuple(args, (char *)Oi:virNodeListDevices, pyobj_conn, flags))
+return(NULL);
+conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+c_retval = virNodeNumOfDevices(conn, flags);
+if (c_retval 0)
+return VIR_PY_NONE;
+
+if (c_retval) {
+names = malloc(sizeof(*names) * c_retval);
+if (!names)
+return VIR_PY_NONE;
+c_retval = virNodeListDevices(conn, names, c_retval, flags);
[libvirt] Re: [PATCH 1/6] host (node) device enumeration - public API additions
This patch contains the public API additions.
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 3624367..c888943 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -993,6 +993,76 @@ char * virStorageVolGetPath(virStorageVolPtr vol);
virDomainPtrvirDomainCreateLinux(virConnectPtr conn,
const char *xmlDesc,
unsigned int flags);
+
+/*
+ * Host device enumeration
+ */
+
+/**
+ * virNodeDevice:
+ *
+ * A virNodeDevice contains a node (host) device details.
+ */
+
+typedef struct _virNodeDevice virNodeDevice;
+
+/**
+ * virNodeDevicePtr:
+ *
+ * A virNodeDevicePtr is a pointer to a virNodeDevice structure. Get
+ * one via virNodeDeviceLookupByKey, virNodeDeviceLookupByName, or
+ * virNodeDeviceCreate. Be sure to Call virNodeDeviceFree when done
+ * using a virNodeDevicePtr obtained from any of the above functions to
+ * avoid leaking memory.
+ */
+
+typedef virNodeDevice *virNodeDevicePtr;
+
+
+int virNodeNumOfDevices (virConnectPtr conn,
+ unsigned int flags);
+
+int virNodeListDevices (virConnectPtr conn,
+ char **const names,
+ int maxnames,
+ unsigned int flags);
+
+int virNodeNumOfDevicesByCap (virConnectPtr conn,
+ const char *cap,
+ unsigned int flags);
+
+int virNodeListDevicesByCap (virConnectPtr conn,
+ const char *cap,
+ char **const names,
+ int maxnames,
+ unsigned int flags);
+
+virNodeDevicePtrvirNodeDeviceLookupByName (virConnectPtr conn,
+ const char *name);
+
+const char *virNodeDeviceGetName (virNodeDevicePtr dev);
+
+const char *virNodeDeviceGetParent (virNodeDevicePtr dev);
+
+int virNodeDeviceNumOfCaps (virNodeDevicePtr dev);
+
+int virNodeDeviceListCaps(virNodeDevicePtr dev,
+ char **const names,
+ int maxnames);
+
+char * virNodeDeviceGetXMLDesc (virNodeDevicePtr dev,
+ unsigned int flags);
+
+virNodeDevicePtrvirNodeDeviceCreate (virConnectPtr conn,
+ const char *xml,
+ unsigned int flags);
+
+int virNodeDeviceDestroy(virNodeDevicePtr dev,
+ unsigned int flags);
+
+int virNodeDeviceFree (virNodeDevicePtr dev);
+
+
#ifdef __cplusplus
}
#endif
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 8e24708..020b8dc 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -58,6 +58,7 @@ typedef enum {
VIR_FROM_STORAGE, /* Error from storage driver */
VIR_FROM_NETWORK, /* Error from network config */
VIR_FROM_DOMAIN,/* Error from domain config */
+VIR_FROM_NODE, /* Error from node driver */
} virErrorDomain;
@@ -148,6 +149,9 @@ typedef enum {
VIR_WAR_NO_STORAGE, /* failed to start storage */
VIR_ERR_NO_STORAGE_POOL, /* storage pool not found */
VIR_ERR_NO_STORAGE_VOL, /* storage pool not found */
+VIR_WAR_NO_NODE, /* failed to start node driver */
+VIR_ERR_INVALID_NODE_DEVICE,/* invalid node device object */
+VIR_ERR_NO_NODE_DEVICE,/* node device not found */
} virErrorNumber;
/**
diff --git a/src/libvirt.c b/src/libvirt.c
index ca2675a..5cef752 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -73,6 +73,8 @@ static virNetworkDriverPtr virNetworkDriverTab[MAX_DRIVERS];
static int virNetworkDriverTabCount = 0;
static virStorageDriverPtr virStorageDriverTab[MAX_DRIVERS];
static int virStorageDriverTabCount = 0;
+static virNodeDriverPtr virNodeDriverTab[MAX_DRIVERS];
+static int virNodeDriverTabCount = 0;
#ifdef WITH_LIBVIRTD
static virStateDriverPtr virStateDriverTab[MAX_DRIVERS];
static int virStateDriverTabCount = 0;
@@ -299,6 +301,12 @@ virInitialize(void)
#ifdef WITH_NETWORK
if (networkRegister() == -1) return -1;
#endif
+#ifdef HAVE_HAL
+if (halNodeRegister() == -1) return -1;
+#endif
+#ifdef HAVE_DEVKIT
+if (devkitNodeRegister() == -1) return -1;
+#endif
if (storageRegister() == -1) return -1;
[libvirt] Re: [PATCH 2/6] host (node) device enumeration - internal API additions
This patch contains the internal API additions.
diff --git a/src/hash.c b/src/hash.c
index 0a5bdcd..424c4a7 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -687,6 +687,9 @@ virGetConnect(void) {
ret-storageVols = virHashCreate(20);
if (ret-storageVols == NULL)
goto failed;
+ret-nodeDevices = virHashCreate(256);
+if (ret-nodeDevices == NULL)
+goto failed;
pthread_mutex_init(ret-lock, NULL);
@@ -703,6 +706,8 @@ failed:
virHashFree(ret-storagePools, (virHashDeallocator) virStoragePoolFreeName);
if (ret-storageVols != NULL)
virHashFree(ret-storageVols, (virHashDeallocator) virStorageVolFreeName);
+if (ret-nodeDevices != NULL)
+virHashFree(ret-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
pthread_mutex_destroy(ret-lock);
VIR_FREE(ret);
@@ -730,6 +735,8 @@ virReleaseConnect(virConnectPtr conn) {
virHashFree(conn-storagePools, (virHashDeallocator) virStoragePoolFreeName);
if (conn-storageVols != NULL)
virHashFree(conn-storageVols, (virHashDeallocator) virStorageVolFreeName);
+if (conn-nodeDevices != NULL)
+virHashFree(conn-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
virResetError(conn-err);
if (__lastErr.conn == conn)
@@ -1318,3 +1325,126 @@ virUnrefStorageVol(virStorageVolPtr vol) {
pthread_mutex_unlock(vol-conn-lock);
return (refs);
}
+
+
+/**
+ * virGetNodeDevice:
+ * @conn: the hypervisor connection
+ * @name: device name (unique on node)
+ *
+ * Lookup if the device is already registered for that connection,
+ * if yes return a new pointer to it, if no allocate a new structure,
+ * and register it in the table. In any case a corresponding call to
+ * virFreeNodeDevice() is needed to not leak data.
+ *
+ * Returns a pointer to the node device, or NULL in case of failure
+ */
+virNodeDevicePtr
+__virGetNodeDevice(virConnectPtr conn, const char *name)
+{
+virNodeDevicePtr ret = NULL;
+
+if ((!VIR_IS_CONNECT(conn)) || (name == NULL)) {
+virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+return(NULL);
+}
+pthread_mutex_lock(conn-lock);
+
+ret = (virNodeDevicePtr) virHashLookup(conn-nodeDevices, name);
+if (ret == NULL) {
+ if (VIR_ALLOC(ret) 0) {
+virHashError(conn, VIR_ERR_NO_MEMORY, _(allocating node dev));
+goto error;
+}
+ret-magic = VIR_NODE_DEVICE_MAGIC;
+ret-conn = conn;
+ret-name = strdup(name);
+if (ret-name == NULL) {
+virHashError(conn, VIR_ERR_NO_MEMORY, _(copying node dev name));
+goto error;
+}
+
+if (virHashAddEntry(conn-nodeDevices, name, ret) 0) {
+virHashError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(failed to add node dev to conn hash table));
+goto error;
+}
+conn-refs++;
+}
+ret-refs++;
+pthread_mutex_unlock(conn-lock);
+return(ret);
+
+error:
+pthread_mutex_unlock(conn-lock);
+if (ret != NULL) {
+VIR_FREE(ret-name);
+VIR_FREE(ret);
+}
+return(NULL);
+}
+
+
+/**
+ * virReleaseNodeDevice:
+ * @dev: the dev to release
+ *
+ * Unconditionally release all memory associated with a dev.
+ * The conn.lock mutex must be held prior to calling this, and will
+ * be released prior to this returning. The dev obj must not
+ * be used once this method returns.
+ *
+ * It will also unreference the associated connection object,
+ * which may also be released if its ref count hits zero.
+ */
+static void
+virReleaseNodeDevice(virNodeDevicePtr dev) {
+virConnectPtr conn = dev-conn;
+DEBUG(release dev %p %s, dev, dev-name);
+
+if (virHashRemoveEntry(conn-nodeDevices, dev-name, NULL) 0)
+virHashError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(dev missing from connection hash table));
+
+dev-magic = -1;
+VIR_FREE(dev-name);
+VIR_FREE(dev);
+
+DEBUG(unref connection %p %s %d, conn, conn-name, conn-refs);
+conn-refs--;
+if (conn-refs == 0) {
+virReleaseConnect(conn);
+/* Already unlocked mutex */
+return;
+}
+
+pthread_mutex_unlock(conn-lock);
+}
+
+
+/**
+ * virUnrefNodeDevice:
+ * @dev: the dev to unreference
+ *
+ * Unreference the dev. If the use count drops to zero, the structure is
+ * actually freed.
+ *
+ * Returns the reference count or -1 in case of failure.
+ */
+int
+virUnrefNodeDevice(virNodeDevicePtr dev) {
+int refs;
+
+pthread_mutex_lock(dev-conn-lock);
+DEBUG(unref dev %p %s %d, dev, dev-name, dev-refs);
+dev-refs--;
+refs = dev-refs;
+if (refs == 0) {
+virReleaseNodeDevice(dev);
+/* Already unlocked mutex */
+return (0);
+}
+
+pthread_mutex_unlock(dev-conn-lock);
+return (refs);
+}
diff --git a/src/internal.h b/src/internal.h
index 2ae764d..76b57ad 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -37,6
[libvirt] [PATCH 0/6] host (node) device enumeration, take two
Ok, here's my substantially-reworked node device enumeration patch, this time done with a proper understanding of the public-obj/Obj/Def model :-) as last discussed here: https://www.redhat.com/archives/libvir-list/2008-September/msg00398.html I've broken it into the following pieces: 1-public-api additions to the public API 2-internal-apiadditions to the internal API 3-local-node-drivers the HAL DeviceKit implementations 4-remote-node-driver the remote driver 5-virsh-support virsh support 6-python-bindings python bindings Big differences from the last submission: * public-obj/Obj/Def object model finally understood :-) * capabilities structure now struct-ified, handled like other Def bits * using newfangled array-based lists for NodeDeviceObj's * added flags args to various public APIs (as suggested by Dan V) * bus folded into capabilities (as discussed w/Dan B) * device key no longer exists - name is unique on node Some pieces are still incomplete, but I thought it would be better to post what I have since it's useful as is. Here's what I know of that's left to do: * finish Python bindings (will get to Real Soon Now) * submit Java bindings (I have them, and have been using them) * implement virNodeDeviceCreate/Destroy (I have no plans for these) * subscribe to HAL events update dev state from them (next for me?) * implement pci_bus/usb_bus capabilities (for PCI/USB controllers) * DeviceKit implementation is barely a proof of concept * need to resolve naming other issues (see https://www.redhat.com/archives/libvir-list/2008-September/msg00430.html * ... and then fill in impl (no hurry; devkit immature now) Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [PATCH 0/6] host (node) device enumeration - local node drivers
This patch contains the actual HAL- and DeviceKit-based local node
drivers.
diff --git a/configure.in b/configure.in
index 66d271a..3441742 100644
--- a/configure.in
+++ b/configure.in
@@ -1048,6 +1048,93 @@ test $enable_shared = no lt_cv_objdir=.
LV_LIBTOOL_OBJDIR=${lt_cv_objdir-.}
AC_SUBST([LV_LIBTOOL_OBJDIR])
+dnl HAL or DeviceKit library for host device enumeration
+HAL_REQUIRED=0.0
+HAL_CFLAGS=
+HAL_LIBS=
+AC_ARG_WITH([hal],
+ [ --with-hal use HAL for host device enumeration],
+ [],
+ [with_hal=check])
+
+if test x$with_hal = xyes -o x$with_hal = xcheck; then
+ PKG_CHECK_MODULES(HAL, hal = $HAL_REQUIRED,
+[with_hal=yes], [
+if test x$with_hal = xcheck ; then
+ with_hal=no
+else
+ AC_MSG_ERROR(
+ [You must install hal-devel = $HAL_REQUIRED to compile libvirt])
+fi
+ ])
+ if test x$with_hal = xyes ; then
+AC_DEFINE_UNQUOTED([HAVE_HAL], 1,
+ [use HAL for host device enumeration])
+
+old_CFLAGS=$CFLAGS
+old_LDFLAGS=$LDFLAGS
+CFLAGS=$CFLAGS $HAL_CFLAGS
+LDFLAGS=$LDFLAGS $HAL_LIBS
+AC_CHECK_FUNCS([libhal_get_all_devices],,[with_hal=no])
+CFLAGS=$old_CFLAGS
+LDFLAGS=$old_LDFLAGS
+ fi
+fi
+AM_CONDITIONAL([HAVE_HAL], [test x$with_hal = xyes])
+AC_SUBST([HAL_CFLAGS])
+AC_SUBST([HAL_LIBS])
+
+DEVKIT_REQUIRED=0.0
+DEVKIT_CFLAGS=
+DEVKIT_LIBS=
+AC_ARG_WITH([devkit],
+ [ --with-devkit use DeviceKit for host device enumeration],
+ [],
+ [with_devkit=check])
+
+dnl Extra check needed while devkit pkg-config info missing glib2 dependency
+PKG_CHECK_MODULES(GLIB2, glib-2.0 = 0.0,,[
+ if test x$with_devkit = xcheck; then
+with_devkit=no
+ elif test x$with_devkit = xyes; then
+AC_MSG_ERROR([required package DeviceKit requires package glib-2.0])
+ fi])
+
+if test x$with_devkit = xyes -o x$with_devkit = xcheck; then
+ PKG_CHECK_MODULES(DEVKIT, devkit-gobject = $DEVKIT_REQUIRED,
+[with_devkit=yes], [
+if test x$with_devkit = xcheck ; then
+ with_devkit=no
+else
+ AC_MSG_ERROR(
+ [You must install DeviceKit-devel = $DEVKIT_REQUIRED to compile libvirt])
+fi
+ ])
+ if test x$with_devkit = xyes ; then
+AC_DEFINE_UNQUOTED([HAVE_DEVKIT], 1,
+ [use DeviceKit for host device enumeration])
+
+dnl Add glib2 flags explicitly while devkit pkg-config info missing glib2 dependency
+DEVKIT_CFLAGS=$GLIB2_CFLAGS $DEVKIT_CFLAGS
+DEVKIT_LIBS=$GLIB2_LIBS $DEVKIT_LIBS
+
+dnl Add more flags apparently required for devkit to work properly
+DEVKIT_CFLAGS=$DEVKIT_CFLAGS -D_POSIX_PTHREAD_SEMANTICS -D_REENTRANT
+
+old_CFLAGS=$CFLAGS
+old_LDFLAGS=$LDFLAGS
+CFLAGS=$CFLAGS $DEVKIT_CFLAGS
+LDFLAGS=$LDFLAGS $DEVKIT_LIBS
+AC_CHECK_FUNCS([devkit_client_connect],,[with_devkit=no])
+CFLAGS=$old_CFLAGS
+LDFLAGS=$old_LDFLAGS
+ fi
+fi
+AM_CONDITIONAL([HAVE_DEVKIT], [test x$with_devkit = xyes])
+AC_SUBST([DEVKIT_CFLAGS])
+AC_SUBST([DEVKIT_LIBS])
+
+
# very annoying
rm -f COPYING
cp COPYING.LIB COPYING
@@ -1123,6 +1210,16 @@ AC_MSG_NOTICE([ numactl: $NUMACTL_CFLAGS $NUMACTL_LIBS])
else
AC_MSG_NOTICE([ numactl: no])
fi
+if test $with_hal = yes ; then
+AC_MSG_NOTICE([ hal: $HAL_CFLAGS $HAL_LIBS])
+else
+AC_MSG_NOTICE([ hal: no])
+fi
+if test $with_devkit = yes ; then
+AC_MSG_NOTICE([ devkit: $DEVKIT_CFLAGS $DEVKIT_LIBS])
+else
+AC_MSG_NOTICE([ devkit: no])
+fi
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Test suite])
AC_MSG_NOTICE([])
diff --git a/po/POTFILES.in b/po/POTFILES.in
index f671155..684d300 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -12,6 +12,7 @@ src/lxc_controller.c
src/lxc_driver.c
src/network_conf.c
src/network_driver.c
+src/node_device.c
src/openvz_conf.c
src/openvz_driver.c
src/proxy_internal.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 5a769f8..963a307 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,5 +1,22 @@
## Process this file with automake to produce Makefile.in
+if WITH_LIBVIRTD
+NODE_DEVICE_SOURCES = node_device.c node_device.h \
+ node_device_conf.c node_device_conf.h
+NODE_DEVICE_CFLAGS =
+NODE_DEVICE_LIBS =
+if HAVE_HAL
+NODE_DEVICE_CFLAGS += $(HAL_CFLAGS)
+NODE_DEVICE_LIBS += $(HAL_LIBS)
+NODE_DEVICE_SOURCES += node_device_hal.c
+endif
+if HAVE_DEVKIT
+NODE_DEVICE_CFLAGS += $(DEVKIT_CFLAGS)
+NODE_DEVICE_LIBS += $(DEVKIT_LIBS)
+NODE_DEVICE_SOURCES += node_device_devkit.c
+endif
+endif
+
INCLUDES = \
-I$(top_srcdir)/gnulib/lib -I../gnulib/lib \
-I../include \
@@ -10,6 +27,7 @@ INCLUDES = \
$(SASL_CFLAGS) \
$(SELINUX_CFLAGS) \
$(NUMACTL_CFLAGS) \
+ $(NODE_DEVICE_CFLAGS) \
-DBINDIR=\$(libexecdir)\ \
-DSBINDIR=\$(sbindir)\ \
-DSYSCONF_DIR=\$(sysconfdir)\ \
@@ -146,6 +164,7 @@ libvirt_la_SOURCES = \
libvirt.c \
$(GENERIC_LIB_SOURCES)\
$(DOMAIN_CONF_SOURCES)\
+ $(NODE_DEVICE_SOURCES)\
$(NETWORK_CONF_SOURCES)\
$(STORAGE_CONF_SOURCES)
@@ -212,7 +231,7 @@ EXTRA_DIST += \
libvirt_la_LIBADD =
[libvirt] Re: [PATCH 6/6] host (node) device enumeration - python bindings
This patch implements the python bindings, rather lamely. I'll submit a
new version with a proper NodeDevice object soon ...
diff --git a/python/generator.py b/python/generator.py
index c706b19..a8fd969 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -258,6 +258,11 @@ py_types = {
'const virConnectPtr': ('O', virConnect, virConnectPtr, virConnectPtr),
'virConnect *': ('O', virConnect, virConnectPtr, virConnectPtr),
'const virConnect *': ('O', virConnect, virConnectPtr, virConnectPtr),
+
+'virNodeDevicePtr': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'const virNodeDevicePtr': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'virNodeDevice *': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
+'const virNodeDevice *': ('O', virNodeDevice, virNodeDevicePtr, virNodeDevicePtr),
}
py_return_types = {
@@ -315,6 +320,8 @@ skip_impl = (
'virStoragePoolListVolumes',
'virDomainBlockPeek',
'virDomainMemoryPeek',
+'virNodeListDevicesByCap',
+'virNodeListDevices',
)
@@ -332,6 +339,10 @@ skip_function = (
'virCopyLastError', # Python API is called virGetLastError instead
'virConnectOpenAuth', # Python C code is manually written
'virDefaultErrorFunc', # Python virErrorFuncHandler impl calls this from C
+'virNodeDeviceGetName',
+'virNodeDeviceGetParent',
+'virNodeDeviceNumOfCaps',
+'virNodeDeviceListCaps',
)
@@ -613,6 +624,7 @@ classes_destructors = {
virStoragePool: virStoragePoolFree,
virStorageVol: virStorageVolFree,
virConnect: virConnectClose,
+virNodeDevice : virNodeDeviceFree
}
functions_noexcept = {
diff --git a/python/libvir.c b/python/libvir.c
index 9cc0c81..e03101d 100644
--- a/python/libvir.c
+++ b/python/libvir.c
@@ -1466,7 +1466,90 @@ libvirt_virStoragePoolLookupByUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *ar
return(py_retval);
}
+static PyObject *
+libvirt_virNodeListDevices(PyObject *self ATTRIBUTE_UNUSED,
+ PyObject *args) {
+PyObject *py_retval;
+char **names = NULL;
+int c_retval, i;
+virConnectPtr conn;
+PyObject *pyobj_conn;
+unsigned int flags;
+
+
+if (!PyArg_ParseTuple(args, (char *)Oi:virNodeListDevices, pyobj_conn, flags))
+return(NULL);
+conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+c_retval = virNodeNumOfDevices(conn, flags);
+if (c_retval 0)
+return VIR_PY_NONE;
+
+if (c_retval) {
+names = malloc(sizeof(*names) * c_retval);
+if (!names)
+return VIR_PY_NONE;
+c_retval = virNodeListDevices(conn, names, c_retval, flags);
+if (c_retval 0) {
+free(names);
+return VIR_PY_NONE;
+}
+}
+py_retval = PyList_New(c_retval);
+
+if (names) {
+for (i = 0;i c_retval;i++) {
+PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i]));
+free(names[i]);
+}
+free(names);
+}
+
+return(py_retval);
+}
+static PyObject *
+libvirt_virNodeListDevicesByCap(PyObject *self ATTRIBUTE_UNUSED,
+PyObject *args) {
+PyObject *py_retval;
+char **names = NULL;
+int c_retval, i;
+virConnectPtr conn;
+PyObject *pyobj_conn;
+char *cap;
+unsigned int flags;
+
+if (!PyArg_ParseTuple(args, (char *)Ozi:virNodeListDevicesByCap,
+ pyobj_conn, cap, flags))
+return(NULL);
+conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn);
+
+c_retval = virNodeNumOfDevicesByCap(conn, cap, flags);
+if (c_retval 0)
+return VIR_PY_NONE;
+
+if (c_retval) {
+names = malloc(sizeof(*names) * c_retval);
+if (!names)
+return VIR_PY_NONE;
+c_retval = virNodeListDevicesByCap(conn, cap, names, c_retval, flags);
+if (c_retval 0) {
+free(names);
+return VIR_PY_NONE;
+}
+}
+py_retval = PyList_New(c_retval);
+
+if (names) {
+for (i = 0;i c_retval;i++) {
+PyList_SetItem(py_retval, i, libvirt_constcharPtrWrap(names[i]));
+free(names[i]);
+}
+free(names);
+}
+
+return(py_retval);
+}
/
* *
@@ -1511,6 +1594,8 @@ static PyMethodDef libvirtMethods[] = {
{(char *) virStoragePoolGetUUID, libvirt_virStoragePoolGetUUID, METH_VARARGS, NULL},
{(char *) virStoragePoolGetUUIDString, libvirt_virStoragePoolGetUUIDString, METH_VARARGS, NULL},
{(char *) virStoragePoolLookupByUUID, libvirt_virStoragePoolLookupByUUID, METH_VARARGS, NULL},
+{(char *) virNodeListDevices, libvirt_virNodeListDevices, METH_VARARGS, NULL},
+{(char *) virNodeListDevicesByCap, libvirt_virNodeListDevicesByCap, METH_VARARGS, NULL},
{NULL, NULL, 0, NULL}
};
diff --git a/python/libvirt-python-api.xml
[libvirt] Re: [PATCH 5/6] host (node) device enumeration - virsh support
This patch implements virsh support. It's pretty sparse right now, and
I'm not wild about the command names I chose. I welcome suggestions
(i.e., command names / args) for completing (or changing) this.
diff --git a/src/virsh.c b/src/virsh.c
index 89aa4fa..67963f0 100644
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -4415,6 +4415,83 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
}
/*
+ * node-list-devices command
+ */
+static const vshCmdInfo info_node_list_devices[] = {
+{syntax, node-list-devices},
+{help, gettext_noop(enumerate devices on this host)},
+{NULL, NULL}
+};
+
+static int
+cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
+{
+char **devices;
+int num_devices, i;
+
+if (!vshConnectionUsability(ctl, ctl-conn, TRUE))
+return FALSE;
+
+num_devices = virNodeNumOfDevices(ctl-conn, 0);
+if (num_devices 0) {
+vshError(ctl, FALSE, %s, _(Failed to count node devices));
+return FALSE;
+} else if (num_devices == 0) {
+return TRUE;
+}
+
+devices = vshMalloc(ctl, sizeof(char *) * num_devices);
+num_devices = virNodeListDevices(ctl-conn, devices, num_devices, 0);
+if (num_devices 0) {
+vshError(ctl, FALSE, %s, _(Failed to list node devices));
+free(devices);
+return FALSE;
+}
+for (i = 0; i num_devices; i++) {
+vshPrint(ctl, %s\n, devices[i]);
+free(devices[i]);
+}
+free(devices);
+return TRUE;
+}
+
+/*
+ * node-device-dumpxml command
+ */
+static const vshCmdInfo info_node_device_dumpxml[] = {
+{syntax, node-device-dumpxml device},
+{help, gettext_noop(node device details in XML)},
+{desc, gettext_noop(Output the node device details as an XML dump to stdout.)},
+{NULL, NULL}
+};
+
+
+static const vshCmdOptDef opts_node_device_dumpxml[] = {
+{device, VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop(device key)},
+{NULL, 0, 0, NULL}
+};
+
+static int
+cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd)
+{
+const char *name;
+virNodeDevicePtr device;
+
+if (!vshConnectionUsability(ctl, ctl-conn, TRUE))
+return FALSE;
+if (!(name = vshCommandOptString(cmd, device, NULL)))
+return FALSE;
+if (!(device = virNodeDeviceLookupByName(ctl-conn, name))) {
+vshError(ctl, FALSE, %s '%s', _(Could not find matching device), name);
+return FALSE;
+}
+
+vshPrint(ctl, %s\n, virNodeDeviceGetXMLDesc(device, 0));
+virNodeDeviceFree(device);
+return TRUE;
+}
+
+/*
* hostkey command
*/
static const vshCmdInfo info_hostname[] = {
@@ -5566,6 +5643,9 @@ static const vshCmdDef commands[] = {
{net-uuid, cmdNetworkUuid, opts_network_uuid, info_network_uuid},
{nodeinfo, cmdNodeinfo, NULL, info_nodeinfo},
+{node-list-devices, cmdNodeListDevices, NULL, info_node_list_devices},
+{node-device-dumpxml, cmdNodeDeviceDumpXML, opts_node_device_dumpxml, info_node_device_dumpxml},
+
{pool-autostart, cmdPoolAutostart, opts_pool_autostart, info_pool_autostart},
{pool-build, cmdPoolBuild, opts_pool_build, info_pool_build},
{pool-create, cmdPoolCreate, opts_pool_create, info_pool_create},
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [PATCH 04/12] Domain Events - rpc changes
On Sun, 2008-10-19 at 20:22 +0100, Daniel P. Berrange wrote:
On Fri, Oct 17, 2008 at 11:58:15AM -0400, Ben Guthro wrote:
Changes to the RPC protocol
+struct remote_domain_event_ret {
+remote_nonnull_domain dom;
+int event;
+unsigned long int callback;
+unsigned long int user_data;
+};
Using a 'unsigned long int' field to transmit the raw pointer feels a little
wrong to me. Could we have the client side pass a simple integer 'token' when
registering / unregistering, and have that 'token' passed back by the server
in the actual event. The client could use this token to lookup the callback
and user_data.
Hold on. We can (and IMO should) quite easily avoid both this lookup
and the passing of the callback pointer to the server:
Suppose we have the same client registered for two different domain
event callbacks. In the current patch, the server will send two RPCs
per event, one for each callback (which the client then unmarshals,
casts, and calls).
But what if we sent just one RPC per event ( per client) and had the
client walk its list of callbacks (which we'll need to track on the
client side anyway, if we're not sending the data over the wire)? We
*always* make *all* the callbacks on the list, so there's no point in
making individual RPCs to fire off each callback individually. This
gets rid of the need to send callback/user_data over the wire, and also
doesn't require tokenization (which is all just extra overhead in this
case).
remote_domain_events_register/deregister_args structs will go away.
remoteDispatchhDomainEventsRegister/Deregister will simply inc/dec a
value, sending events only when the value is 0.
While I'm not sure I've described this very well, I feel pretty strongly
that it's the right way to go. If my explanation isn't clear, please
get back to me ...
Dave
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
Hi Daniel -
I'm implementing virNodeDeviceDef, as suggested, but I just noticed
something that really bothers me. In a nutshell, it's that the current
implementation of this scheme unnecessarily increases the (time)
complexity of O(1) operations to O(n).
For example, take virDomainGetXMLDesc(). One would think dumping the
XML descriptor for a domain object we have in hand would be O(1), but in
fact, it's O(n) (where n is the number of domains with the same driver),
because qemudDomainDumpXML must find the DomainObjPtr (from which it can
get the def). I suppose this lookup could be made O(log n) if the
domains were sorted by UUID on the list, but the fact remains that
lookup could be eliminated entirely.
Perhaps it's just a matter of allowing the public objects
(virDomain, virNetwork, etc.) to hold a pointer to their optional (never
used by remote driver, for example) private state objects. So I guess
I'm suggesting adding a void * to each of the public objects, which
drivers can use for this purpose. I'll go ahead and do this for
NodeDevices in the next incarnation of this patch to show you what I
mean. (It won't be hard go to the conventional lookup impl if this
turns out to be a bad idea.)
Dave
On Fri, 2008-10-03 at 16:23 +0100, Daniel P. Berrange wrote:
On Tue, Sep 30, 2008 at 12:17:27AM -0400, David Lively wrote:
diff --git a/src/internal.h b/src/internal.h
index d96504d..9a94f6d 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -292,6 +307,25 @@ struct _virStorageVol {
};
+/**
+ * _virNodeDevice:
+ *
+ * Internal structure associated with a node device
+ */
+struct _virNodeDevice {
+unsigned int magic; /* specific value to check */
+int refs; /* reference count */
+virConnectPtr conn; /* pointer back to the connection
*/
+char *key; /* device key */
+char *name; /* device name */
+char *parent_key; /* parent device */
+char *bus_name; /* (optional) bus name */
+xmlNodePtr bus; /* (optional) bus descriptor */
+char **cap_names; /* capability names (null-term) */
+xmlNodePtr *caps; /* capability descs (null-term) */
+};
This is not the right way to maintain the internal representation of
devices.
The model we follow is to have 2, sometimes 3 structs to represent
an object. I'll summarize in terms of domain objects
- virDomainPtr - the public opaque struct representing a domain
the internal struct is defined in internal.h
and merely contains reference counting, connection
pointer, and any unique keys (ID, name, UUID).
- virDomainObjPtr - the internal struct representing the state of
a domain object. Not all drivers use us - only
stateful drivers do. It is used to track state
such as guest PID, logfile, running state,
and a pointer to the live and inactive configs
This is done in domain_conf.h/.c
- virDomainDefPtr - the internal struct representing the canonical
configuration of a domain. This is a direct
mapping of the XML into a series of structs.
This is done in domain_conf.h/.c
So, in the context for host devices we need a few changes. In the
internal.h file, you should only store the key/name attributes.
There should then be a separate file handling configuration parsing
and formatting, node_device_conf.h/.c. There is probably no need
for a state object, so skip virNodeDeviceObjPtr, and just create a
virNodeDeviceDefPtr representing the XML format as a series of
structs/unions, etc. See virDomainDefPtr for a good example. This
should not store any xmlNodePtr instances - it should be all done
as explicit struct/enum fields
The node_device_conf.c file should at mimium have 2 methods, one
for converting an XML document into a virNodeDeviceDefPtr object,
and one for the converting a virNodeDeviceObjPtr back into an XML
document. Follow the existing API naming / method signatures that
are seen in domain_conf.h / network_conf.h for current 'best practice'
Daniel
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Thu, 2008-10-09 at 18:01 +0100, Daniel P. Berrange wrote: On Thu, Oct 09, 2008 at 12:25:18PM -0400, David Lively wrote: Hi Daniel - I'm implementing virNodeDeviceDef, as suggested, but I just noticed something that really bothers me. In a nutshell, it's that the current implementation of this scheme unnecessarily increases the (time) complexity of O(1) operations to O(n). For example, take virDomainGetXMLDesc(). One would think dumping the XML descriptor for a domain object we have in hand would be O(1), but in fact, it's O(n) (where n is the number of domains with the same driver), because qemudDomainDumpXML must find the DomainObjPtr (from which it can get the def). I suppose this lookup could be made O(log n) if the domains were sorted by UUID on the list, but the fact remains that lookup could be eliminated entirely. Sure the lookup algorithm is O(n), but have you actually got real world benchmarks showing this is the serious bottleneck? In the XML Well ... uhh ... no. :-o dump example, the time to lookup the object is likely dwarfed by the time to generate the XML doc, or to talk to the QEMU monitor. IMHO, optimizing this is overkill. Worst case we can just hash on the UUID if it ever provdes a problem. Okay, I'll buy that. And (now) I see what you mean about the differing object lifetime. ... In fact (perhaps I shouldn't be admitting this publicly), your whole public-obj/Obj scheme now finally makes sense to me! The public objs are really just a kind of handle to the private Objs, and they exist independently of one another. public objs belong to the apps, private Objs to the appropriate driver. And (now that I look at the Xen driver), I finally understand that there are stateless drivers. In fact, both the devkit and HAL node-dev drivers need to be stateful (since the device name isn't a reasonable handle for either devkit or HAL devices), so I'm creating a virNodeDeviceObj as well as a virNodeDeviceDef. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Tue, 2008-09-30 at 19:31 +0200, Daniel Veillard wrote: Good to see that you seems to have the python bindings ready too ! Many python bindings are not really ready (in this patch). But I should have them in the next patch (Monday/Tuesday next week). I have complete java bindings for the current API, and will submit them once we agree upon it. I wonder how many of them should be future-proofed by adding them a final flags argument too ... For example it may be useful to only lookup devices which are local to the machine, or the opposite only remote devices. We don't have to specify now flags values, but having the APIs ready is sufficient. The virNodeNum/virNodeListDevices devices can probably all share the same flags definitions when needed. I agree. I'll add a flags arg to virNodeNum*/virNodeList*. The LookupByName/LookupByKey may be able to use the same set. I wonder a bit about the key argument though, I assume it's something actually defined by the lower APIs (hal/devkit). As long as the lookup keys are unique, I don't see why we'd want a flags arg for these functions. Currently the key is implementation- (HAL- or Devkit-) dependent, but I have questions about that (and for that matter, the name arg -- if it's unique, why have a separate key??). Dan B brings up some similar points, so I'll address this in my response to his post rather than here. For virNodeDeviceCreate maybe a flags may be needed too, though the flexibility of the API is provided by the XML. I think the XML should provide sufficient flexibility here. But let me know if you really want me to add a flag arg. +int virNodeDeviceDestroy(virNodeDevicePtr dev); + +int virNodeDeviceFree (virNodeDevicePtr dev); Maybe Destroy need flags too, for example to force (or not) destruction of devices which may be in use. Ok, I'll add a flags arg to Destroy as well. FWIW, I'm not wild about the names virNodeDeviceCreate/Destroy. Don't Attach and Detach make more sense? If there are no objections, I'll change those in my next iteration (though I'll still leave them unimplemented). @@ -332,6 +340,12 @@ skip_function = ( 'virCopyLastError', # Python API is called virGetLastError instead 'virConnectOpenAuth', # Python C code is manually written 'virDefaultErrorFunc', # Python virErrorFuncHandler impl calls this from C +'virNodeDeviceGetKey', +'virNodeDeviceGetName', +'virNodeDeviceGetParentKey', +'virNodeDeviceGetBus', +'virNodeDeviceNumOfCaps', +'virNodeDeviceListCaps', ) Strange how are the accessors supposed to work from python then ? They don't. They're just here so you don't get errors building the Python bindings. I'll implement real bindings for these soon. Hum, the libvirt_lxc really need to link to the device discovery libs ? I was making host device enumeration work for ALL hypervisors (though of course the remote driver has a remote impl), so I think this is necessary. But that said, I'm still digesting Dan B's point about licensing issues, and I really have no clue about how lxc and openvz work with libvirt (do they have separate control daemons, like Xen, or are they more like QEMU, or ???). virLibConnError(virConnectPtr conn, virErrorNumber error, const char *info) virLibConnWarning(virConnectPtr conn, virErrorNumber error, const char *info) you really need to export them ? Oh, uhhh, apparently not :-) (I did at one point, but must have removed that code.) I'll un-export them ... Hum ... we need the comment on the accessors, so they get extracted as part of the API when doing 'make rebuild' in docs/ added to the resulting XML, which then will provide the python accessors automatically I think. Will do. Thanks for the response. I'll implement the things discussed here (plus in Dan B's comments - another response coming) and submit a new patch on Monday or early Tuesday next week. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Tue, 2008-09-30 at 19:02 +0100, Daniel P. Berrange wrote:
On Tue, Sep 30, 2008 at 12:17:27AM -0400, David Lively wrote:
- Split stateful drivers out of libvirt.so, so they're only used
by the daemon, and not apps linking to libvirt.so.
This solves the licensing problem that HAL/DeviceKit introduce
libvirt.so needs to be LGPL to allow closed-source apps to link
to it. HAL/DeviceKit are both GPL or AFL licensed, by virtue
of using DBus. Since LGPL is not AFL compatible, if we link to
HAL/DeviceKit/DBus we do so under the GPL, and thus would
prevent closed source apps using libvirt.so We don't want this,
so we ned to make sure only the libvirtd daemon links to the
GPL bits.
Ok. I see. Will do.
* figure out how devkit and HAL correlate, so we report device info
consistently
This is looking like it'll be much harder than I had anticipated.
Yeah, I struggled for a while trying to reconcile them. But devkit's
not even close to complete yet, so I gave up. I'm assuming that, since
there are a lot of HAL - DevKit conversions to be done, some kind of
(probably domain-specific) method(s) of correlating the two will
eventually be described.
So I'm thinking perhaps we need to simplify our modelling a little
so its not so closely replicating HAL, getting rid of the distinct
elements for 'class' and 'bus' and having a device simply have a
'subsystem'. And instead of having a complete tree hierarchy, have
a specialized hierarchy. eg if we can identify a 'usb' or 'pci'
device parent for a device, then include its name, but don't
claim to provide a full hierarchy.
Yes! I find the distinction between bus and capability to be pretty
arbitrary anyway (being on a bus is a type of capability, IMO).
So I'll drop the bus stuff, and combine bus capabilities with the
other capabilities.
The other interesting question, is what policy we should define
for compatability. Do we absolutely need to have compatible
keys names for devices, whether using HAL vs DeviceKit, or
can be just say that the format of a name is opaque and liable
to change ? This has upgrade implications, for example, if we
ship libvirt with a HAL backend in Fedora 10, and then switched
it to the DeviceKit backend in Fedora 11, do we need to ensure
consistent naming across the upgrade path. I don't know...
Right. I was hoping to find a consistent set of keys/names to use, but
gave up. (sysfs-paths seemed the most common denominator, but not all
HAL devices have sysfs-paths, and I think the HAL names look more
appropriate anyway.) I'm hoping that devkit will eventually add a
property that either specifies or is convertible to a HAL name.
Also, if names are really unique (as specified), then why have separate
keys? I'd prefer dropping the Key functions and using Name as the key
(as we do for storage pools, etc.).
+struct _stringv {
+int len;
+char **const base;
+const int maxlen;
+};
+
+typedef struct _stringv stringv;
Perhaps I'm mis-undersatnding what this does, but isn't this
similar to the virStringList class in internal.h ?
Well, it's a vector instead of a singly-linked list. I use it when I
know the max length ahead of time (while the list is used in pool source
discovery code that knows of no limit on the number of sources it will
find). I could use a list for the former case, but that would require
an extra copy of each name (plus unnecessary alloc/dealloc code for the
list).
+static char *nodeDeviceDumpXML(virNodeDevicePtr dev,
+ unsigned int flags ATTRIBUTE_UNUSED)
+{
+xmlBufferPtr buf = xmlBufferCreate();
...
+xmlBufferCCat(buf, /key\n);
This should use the libvirt buffer routines from buf.h which
ensure you don't ignore return values indicating OOM like
you have here :-) See buf.h, or HACKING file for some examples.
Note I call xmlNodeDump(buf, ...). I suppose the right thing to do is
to add virBufNodeDump(...) to use libxml to dump into a virBuf ???
+/* TODO: Can we avoid this copy? */
+xml = strdup((const char *)xmlBufferContent(buf));
Yes, you can with the libvirt buf.h APIs :-)
But if I have to have to call xmlNodeDump, I can't avoid it ...
Thanks for the comments!
Dave
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
I definitely wasn't planning on covering all of HAL :-) I assume you weren't planning on exposing these capability-specific representations in the public API. Right? (If not, ignore the rest of this ...) So I guess I don't see the value of having these cap-specific internal representations. I keep a string array of the cap names for ListDevicesByCap, but other than that, the capability data is used only by virNodeDeviceGetXMLDesc(). So it seems natural to represent it in a form that can easily be converted to XML, and that can represent all the XML we'll need to output (like xmlNode). Otherwise, we are forced to write more capability-specific code, with no particular advantage (no stronger typing guarantees if we don't expose specific cap types). Dave P.S. I do think it would be useful to have access to capability details other than GetXMLDesc. Perhaps: const char *virNodeDeviceCapabilityProperty(virNodeDevicePtr dev, const char *cap, const char *prop) but note this exposes them only in a general (property / value) way, and is quite easily implemented with a xmlNode representation. On Fri, 2008-10-03 at 18:41 +0100, Daniel P. Berrange wrote: On Fri, Oct 03, 2008 at 01:20:35PM -0400, David Lively wrote: Okay, I see what you mean. I'll create a virNodeDeviceDefPtr and follow the established pattern. I'm really trying to avoid creating a struct/union that must be extended every time we support a new capability. I struggled quite a bit with the right representation for capabilities and bus details.. At one point, I had my own (general-purpose; i.e., not specific to any type of capability) virNodeDeviceCapabilities type, but it looked so much like a DOM tree that just using a xmlNode seemed like the best choice. Are you suggesting creating a struct for each kind of currently-supported capability, or are you suggesting creating a single struct that's general enough to represent all capabilities? I'm suggesting a something that is specific for each capability. Now if we were to support all metadata that HAL exposes this would be a huge job. But I don't believe we should be exposing all the metadata that HAL has, because in the future this XML format has to work with DeviceKit which is basically exposing UDev metadata, and VMWare's APIs which expose hardware info in their own way. Thus, IMHO, we should expose specific capabilities we know we care about, for specific sub-substems, and not try to handle the entire of HAL. A good starting point would be - PCI, domain, bus, slot, function, and vendor/product - USB, bus, device and vendor/product Basically same info required for attaching the device to a domain, thus matching the struct virDomainHostdevDefPtr in domain_conf.h - NIC, name mac address - Block, name and host adapter - Host adapter, name For these also have a way to link to the parent device associated with them (ie the PCI/USB device providing them). That would basically be enough for use of the existing domain/storage and network APIs which is what we need this data for, and this minimal info should be satisifiable with VMWare's APIs, and DeviceKit. Regards, Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
Hi Daniel -
I have an implementation underway right now, with both HAL- and
Devkit-based drivers. We have some folks who need this *next week*,
so I'm trying to get the most functionality finished up today and this
weekend.
The generic (HAL/Devkit-agnostic) framework is essentially done and
tested (except for a few odds and ends like some missing virsh fns).
The HAL-based driver is ~80% done. The Devkit-based driver isn't very
capable yet, mostly because of the limited Devkit functionality
available (also, there are some serious naming issues to discuss).
I haven't yet implemented virNodeDevice{Create,Destroy}, nor am I
registering for HAL/Devkit events. We don't need this functionality
right away, so I may wait 'til next week to implement them. The
underlying virNodeDevice infrastructure is ref-counted and accessed via
a conn-lock-protected virHashTable (like connections, domains, etc.),
so concurrent access shouldn't be a problem.
I'll post a patch (with some TODOs ...) on Sunday or Monday morning.
Dave
On Fri, 2008-09-26 at 14:15 +0100, Daniel P. Berrange wrote:
On Thu, Aug 21, 2008 at 03:18:57PM -0400, David Lively wrote:
Hi -
I'm about to start working on host device enumeration, along the
(HAL-ish) lines of what was discussed back in April:
https://www.redhat.com/archives/libvir-list/2008-April/msg5.html
I know the xml details haven't been fully fleshed out, but there seems
to be agreement that it will be a fairly direct mapping from (a subset
of the) HAL info to the details that we need in the xml. Doubtless it
will take a while to figure out exactly what subset suffices (and, for
that matter, if everything needed is available via HAL ...), but I think
the work is well-defined for some of the obvious details (discussed in
the above thread) on which there's broad agreement.
Is anyone working on such an implementation?
Did you ever start any work on this project ? The oVirt guys really want
this done asap, so if you've not started on it, or have a partial start
to work from, I plan to make time to look at it next week
Regards,
Daniel
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Events API
Okay - I'm looking at glib docs. You guys are suggesting I create something that's easy to plug into a glib main loop, right? So ... something that's easy to wrap with a GSource via g_source_new()? (Stop me now if I'm on the wrong track!) So ... something that plugs into GSourceFuncs elements (in particular prepare/check/dispatch/finalize) easily?? int virEventsNeedDelivering(virConnectionPtr conn) int virEventsDispatch(virConnectionPtr conn) int virEventsCleanup(virConnectionPtr conn) and perhaps: int virEventsRunOnce(virConnectionPtr conn) to wrap them all for non-glib-ish implementations? Sorry if I'm being dense. I've looked at the qemud events interface, but I still feel like I'm missing something here. Dave On Mon, 2008-09-22 at 07:23 +0100, Richard W.M. Jones wrote: On Fri, Sep 19, 2008 at 11:08:37AM -0400, David Lively wrote: int virDeliverEvents(int timeout) This isn't really much use because it doesn't integrate with common main loops. I suggest you look at glib to see how it handles main loops and how to integrate with it. Rich. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Events API
Okay, at long last, I see what you mean (I think). Apps using libvirt events must register an EventImpl via virRegisterEventImpl. Internally, suppose we implement events via an anonymous pipe. libvirt would call EventImpl.addHandle(pipe_read_fd, POLLIN ..., __virHandleEvents, conn) so the app's main loop would monitor fd (in whatever manner it chooses), then call __virHandleEvents(fd, conn) when it detected activity. __virHandleEvents would pull the event from the pipe and dispatch handlers as appropriate. We'd call eventImpl.removeHandle(pipe_read_fd) when the domain goes away. Am I finally on the right track?? If so, are you proposing that we simply make the existing src/events.h interface public? At this point, I don't see the need for anything but addHandle and removeHandle (but I can see how the others might be useful to libvirt eventually). Dave On Mon, 2008-09-22 at 16:22 +0100, Daniel P. Berrange wrote: No, this the wrong approach. This is defining an event loop impl - we don't want todo that. We need to define an API to let an application provide a set of callback for libvirt to talk to an existing event loop impl. ie a way for libvirt to register FD watches and timeouts, not a way for apps to manually process libvirt events in this way this example shows. The public API for this is along the lines of that currently defined in the src/events.h file. There are a set of functions libvirt needs in order to register actions for FDs, and/or timeouts. So the public API should consist of a way to register impls of these APIs typedef int (*virEventAddHandleFunc)(int, int, virEventHandleCallback, void *); typedef void (*virEventUpdateHandleFunc)(int, int); typedef int (*virEventRemoveHandleFunc)(int); typedef int (*virEventAddTimeoutFunc)(int, virEventTimeoutCallback, void *); typedef void (*virEventUpdateTimeoutFunc)(int, int); typedef int (*virEventRemoveTimeoutFunc)(int); void virEventRegisterImpl(virEventAddHandleFunc addHandle, virEventUpdateHandleFunc updateHandle, virEventRemoveHandleFunc removeHandle, virEventAddTimeoutFunc addTimeout, virEventUpdateTimeoutFunc updateTimeout, virEventRemoveTimeoutFunc removeTimeout); A separate libvirt-glib.so, would provide a API call virEventRegisterGLib() which calls virEventRegisterImpl() with a suitable implementation for glib. An application like virt-manager which uses glib and wants events would then calll virEventRegisterGLib(). If it had a custom event loop of its own, then it could call virEventRegisterImpl() directly with its special impl. It may be worth making our public API even more closely aligned with dbus - see dbus-connection.h and dbus-server.h - so people writing glue functions for it could just reuse what they've already written for dbus. typedef dbus_bool_t (* DBusAddWatchFunction) (DBusWatch *watch, void *data); typedef void(* DBusWatchToggledFunction) (DBusWatch *watch, void *data); typedef void(* DBusRemoveWatchFunction)(DBusWatch *watch, void *data); typedef dbus_bool_t (* DBusAddTimeoutFunction) (DBusTimeout*timeout, void *data); typedef void(* DBusTimeoutToggledFunction) (DBusTimeout*timeout, void *data); typedef void(* DBusRemoveTimeoutFunction) (DBusTimeout*timeout, void *data); dbus_bool_t dbus_server_set_watch_functions (DBusServer *server, DBusAddWatchFunction add_function, DBusRemoveWatchFunction remove_function, DBusWatchToggledFunction toggled_function, void *data, DBusFreeFunction free_data_function); dbus_bool_t dbus_server_set_timeout_functions (DBusServer *server, DBusAddTimeoutFunction add_function, DBusRemoveTimeoutFunction remove_function, DBusTimeoutToggledFunction toggled_function, void *data,
Re: [libvirt] [RFC] Events API
On Fri, 2008-09-19 at 11:14 +0100, Richard W.M. Jones wrote: On Thu, Sep 18, 2008 at 12:45:07PM -0400, David Lively wrote: I'm a little concerned that a vector of event type names isn't really adequate for specifying a filter. Does this need to be more general (XPathString exprs??) I think I'm with Dan on this one. I would start small -- just domains coming going (unless VirtualIron needs other events). Since there is no limit to the number of API calls we can have in libvirt, add an API call just for registering for these domain events. Instead of trying to overload untyped strings with complicated meanings. Okay. I'm fine with a more strongly-typed event protocol. While it's more work to add new classes of events (as compared with extending event XML), that's probably a Good Thing :-) But my larger concern is that an asynchronous callback mechanism (as proposed above) assumes the presence of some thread / process from which to make the callbacks. This works fine in the libvirtd context, but not outside of it. For instance, we build a client only version of libvirt with ONLY the remote driver, which currently doesn't require pthreads at all. Introducing asynchronous callbacks into the API means pthreads is now required for this. I'm not quite sure I follow this -- you mean it introduces pthreads into libvirt or into the caller? As far as I can see, nothing about this would require threads in either. I meant that if we expected the callbacks to just happen, libvirt (at least, the libvirtd-less version) would need to spawn a thread to make the callbacks. But this is quite easily avoided by providing a hook that clients are responsible for calling for event delivery, as suggested. I had considered this as an alternative to the file-descriptor interface (but thought the fd interface was convenient with to use w/select() poll()). After considering the problems with fds and power-saving and windows compatibility, I agree an event-delivery hook sounds like the best idea, perhaps: int virDeliverEvents(int timeout) where timeout is interpreted as in poll() (i.e., max millisecs to block, 0 means don't block, negative means block forever). The remote protocol allows event messages to be passed back asynchronously, although the current remote driver wouldn't expect to receive them which might be a problem for backwards/forwards compatibility. (Therefore the remote client must tell the remote server that it can handle asynchronous events, and the remote client must be prepared to talk to a server which cannot understand asynchronous events -- there is an internal feature API you can use for this). Notwithstanding that, you would have to add a client call to poll for events or (better) to expose the file descriptor so that callers can use it in select(2)/poll(2). Yeah, I expect the remote implementation will be the worst part! Thanks for the pointers. Thanks, Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] Events API
Hi - We have some folks looking into the implementation of events (just VM state transition events, for now) in libvirtd. I've been assuming that events will be XML strings, something like: event type=domain-state-transition timestamp=x domain-id22/domain-id old-statenostate/old-state new-staterunning/new-state /event where the contents of the event element are determined by the event type, and the attributes type and timestamp are required. Originally, I was thinking one would listen for events by registering a callback, something like: int virConnectAddEventHandler(virConnPtr conn, char **filter, virEventHandler handler, void *arg) where filter is either NULL, indicating interest in all events, or else a NULL-terminated vector of event type names, allowing filtering by event type. void handler(conn, const char *eventXML, arg) would be called for each matching event. I'm a little concerned that a vector of event type names isn't really adequate for specifying a filter. Does this need to be more general (XPathString exprs??) But my larger concern is that an asynchronous callback mechanism (as proposed above) assumes the presence of some thread / process from which to make the callbacks. This works fine in the libvirtd context, but not outside of it. For instance, we build a client only version of libvirt with ONLY the remote driver, which currently doesn't require pthreads at all. Introducing asynchronous callbacks into the API means pthreads is now required for this. I'm not sure how much requiring this extra thread matters. If it does, we could always define a synchronous delivery mechanism instead. For instance, we could have a virDeliverEvents(conn) call to make the callbacks for any outstanding events. Or we could just dispense with callbacks altogether, and return a readable (pipe) fd from which the client can read events. Comments?? Dave P.S. We'll also need an internal API for sending events (most likely, an extension of the existing virEvent stuff), but let's get the basic shape of the external interface agreed upon first. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] introducing source name (for logical storage pools)
Thanks Daniel. I just merged in your changes. You seem to be missing a
small incremental change (checking the strdup return value for NULL),
attached.
Dave
On Tue, 2008-09-02 at 16:17 +0200, Daniel Veillard wrote:
On Fri, Aug 29, 2008 at 03:49:27PM -0400, David Lively wrote:
Hi Jim -
I've attached a (very) small incremental patch (i.e., to be applied
after the one you've already merged) that addresses a couple things I
noticed missing:
(a) documents the new source name element in formatstorage.html.in
(b) adds --source-name to the (optional) args for virsh pool-define-as
I've also attached a new version of the full patch containing this
change, in case that's easier.
Okidoc, I finally added this in CVS, i just had to do a bit of porting
since the XPath lookup function have an extra argument, but nothing
hard. I also changed some of the error message to provide more context
because as Jim pointed out they were a bit too generic.
thanks a lot !
Daniel
diff --git a/src/storage_conf.c b/src/storage_conf.c
index 2f6093b..37a2040 100644
--- a/src/storage_conf.c
+++ b/src/storage_conf.c
@@ -331,6 +331,8 @@ virStoragePoolDefParseDoc(virConnectPtr conn,
if (ret-source.name == NULL) {
/* source name defaults to pool name */
ret-source.name = strdup(ret-name);
+if (ret-source.name == NULL)
+virStorageReportError(conn, VIR_ERR_NO_MEMORY, %s, _(pool name));
}
}
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] A laundry list of TODO items for libvirt
Does host power support belong on the TODO?
(i.e. virConnect{Poweroff,Suspend,Hibernate,Reboot})
If libvirt is going to be the only external interface used to manage a
virtualized host, it must be able to poweroff, suspend, (hibernate?),
and reboot the host. (For hosts that support IPMI/iLO or other power
control interfaces, this isn't necessary, but there are plenty of hosts
that don't support any of this.)
Dave
On Thu, 2008-08-28 at 15:45 +0100, Daniel P. Berrange wrote:
A bunch of us at Red Hat had a bit of a brainstorming session to make a
list of things we'd like to see added to libvirt going forward, to better
support our needs for virtualization in Fedora / oVirt. We've put details
of everything up on the libvirt wiki:
http://wiki.libvirt.org/page/Todo
This list isn't a firm commitment to actually implement these features.
We may later decide that some of them don't belong in libvirt (eg, the
question of allowing multiu-thread access to virConnectPtr is open to
debate). Nor have we got people actually working on these items - unless
explicitly noted against a todo item. If anyone is looking to do work
on libvirt and isn't sure of what to work on, this list may be of interest.
Also, this is a wiki, so feel free to add more ideas - or add expanded
details to existing ideas. Or reply to this mail, and one of us will
add new ideas to the wiki for you
The only request is that you don't actually start implementing things from
the list without first discussing proposals on this list, because most of
these ideas need more design / analysis before its reasonable to write
code
Regards,
Daniel
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
On Fri, 2008-08-22 at 09:50 +0100, Daniel P. Berrange wrote: +const char *start_tag = SourceList\n; +const char *end_tag = /SourceList\n; I'd prefer that to be sources - we avoid capitals in the XML element names everywhere else, and in the few cases of joining words use an underscore, but I think plural form is OK for this. I prefer sources as well, so I'll change this. And I'll put those defines in storage_backend.h as well. As long as we're on the subject of naming (and before it's too late), it's been bothering me that we keep calling this storage pool discovery. To me, storage source discovery seems more accurate (because they're not pools until we define libvirt pools based on the sources). So I'd prefer renaming the various *Discover[Storage]Pools* functions (and support structs) introduced in this patch to *Discover[Storage]Sources*. I was just sticking with the originally-proposed names to avoid confusion. What do you all think? Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
Here's the patch with those issues addressed (also merged with latest
upstream - avoids internal.h merge conflict) ...
Dave
On Fri, 2008-08-22 at 09:50 +0100, Daniel P. Berrange wrote:
On Thu, Aug 21, 2008 at 10:29:43AM -0400, David Lively wrote:
Hi Folks -
Here's my second pass at storage pool discovery. I've taken Daniel's
suggestion and made it return a single XML doc containing source
elements rather than an array of pool docs (and also incorporated
suggestions from Daniel V and Jim M).
Note that the storage source name patch is closely related
(without it, the source docs returned for logical pools aren't
correct).
Excellant, I think this patch looks more or less good to commit now.
+static char *
+virStorageBackendFileSystemNetDiscoverPools(virConnectPtr conn,
+const char *srcSpec,
+unsigned int flags
ATTRIBUTE_UNUSED)
...
+const char *prog[] = { SHOWMOUNT, --no-headers, --exports, NULL,
NULL };
+const char *start_tag = SourceList\n;
+const char *end_tag = /SourceList\n;
I'd prefer that to be sources - we avoid capitals in the
XML element names everywhere else, and in the few cases of
joining words use an underscore, but I think plural form is
OK for this.
+static char *
+virStorageBackendLogicalDiscoverPools(virConnectPtr conn,
+ const char *srcSpec ATTRIBUTE_UNUSED,
+ unsigned int flags ATTRIBUTE_UNUSED)
+const char *prog[] = { VGS, --noheadings, -o, vg_name, NULL };
+const char *start_tag = SourceList\n;
+const char *end_tag = /SourceList\n;
We should #define these two tags in storage_backend.h so each driver doesn't
need to dup them
Daniel
diff --git a/configure.in b/configure.in
index 9479f1c..430a097 100644
--- a/configure.in
+++ b/configure.in
@@ -671,6 +671,11 @@ if test $with_storage_fs = yes -o $with_storage_fs = check; then
fi
fi
AM_CONDITIONAL([WITH_STORAGE_FS], [test $with_storage_fs = yes])
+if test $with_storage_fs = yes; then
+ AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin])
+ AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT],
+[Location or name of the showmount program])
+fi
AC_PATH_PROG([QEMU_IMG], [qemu-img], [], [$PATH:/sbin:/usr/sbin:/bin:/usr/bin])
if test -n $QEMU_IMG ; then
diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index 9c3e1c2..5308fa9 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -890,6 +890,14 @@ int virConnectListDefinedStoragePools(virConnectPtr conn,
int maxnames);
/*
+ * Query a host for storage pools of a particular type
+ */
+char * virConnectDiscoverStoragePools(virConnectPtr conn,
+ const char *type,
+ const char *srcSpec,
+ unsigned int flags);
+
+/*
* Lookup pool by name or uuid
*/
virStoragePoolPtr virStoragePoolLookupByName (virConnectPtr conn,
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index f077a26..3d06d76 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -890,6 +890,14 @@ int virConnectListDefinedStoragePools(virConnectPtr conn,
int maxnames);
/*
+ * Query a host for storage pools of a particular type
+ */
+char * virConnectDiscoverStoragePools(virConnectPtr conn,
+ const char *type,
+ const char *srcSpec,
+ unsigned int flags);
+
+/*
* Lookup pool by name or uuid
*/
virStoragePoolPtr virStoragePoolLookupByName (virConnectPtr conn,
diff --git a/qemud/remote.c b/qemud/remote.c
index b5a6ec9..43b3a56 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -2958,6 +2958,27 @@ remoteDispatchListStoragePools (struct qemud_server *server ATTRIBUTE_UNUSED,
}
static int
+remoteDispatchDiscoverStoragePools (struct qemud_server *server ATTRIBUTE_UNUSED,
+struct qemud_client *client,
+remote_message_header *req,
+remote_discover_storage_pools_args *args,
+remote_discover_storage_pools_ret *ret)
+{
+CHECK_CONN(client);
+
+ret-xml =
+virConnectDiscoverStoragePools (client-conn,
+args-type,
+args-srcSpec ? *args-srcSpec : NULL,
+args-flags
Re: [libvirt] [PATCH] storage pool discovery
Hi Jim - Thanks for your comments. I really appreciate the detailed look. I'll implement your suggestions (including the refactoring of the code to turn a virStringList into a single XML string), though I have a question about one of them: On Fri, 2008-08-22 at 19:16 +0200, Jim Meyering wrote: diff --git a/configure.in b/configure.in index 9479f1c..430a097 100644 --- a/configure.in +++ b/configure.in @@ -671,6 +671,11 @@ if test $with_storage_fs = yes -o $with_storage_fs = check; then fi fi AM_CONDITIONAL([WITH_STORAGE_FS], [test $with_storage_fs = yes]) +if test $with_storage_fs = yes; then + AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin]) + AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT], +[Location or name of the showmount program]) +fi Do we want to require the showmount package via the spec file? I was a little worried about this too. It's probably better to handle this more like iscsiadmin/--with-storage-iscsi: defaults to enabling iscsi backend if and only if iscsiadmin is present. Fails with --with-storage-iscsi=yes explicitly specified and iscsiadmin not found. But I'm not sure we want to disable the storage_fs backend just because we can't find showmount. So maybe there should be another config option --with-storage-netfs-discovery to cover the only code that actually uses showmount? Either of these sounds reasonable to me. Do you have a preference? Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
On Fri, 2008-08-22 at 19:16 +0200, Jim Meyering wrote:
+const char *name, *path;
path can be const, too.
It is, at least according to my gcc (4.3.0-8, x86_64, Fedora9).
Compile the following and without FORCE_WARNING to check:
void check()
{
#ifdef FORCE_WARNING
const char *foo;
char *bar;
#else
const char *foo, *bar;
#endif
foo = I'm a const char *;
bar = foo; // warns iff bar not const char *
}
Dave
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] introducing source name (for logical storage pools)
Oops - that was against an old base. Sorry. Here's the new one.
Also fixed a few other issues ...
Dave
On Mon, 2008-08-18 at 12:12 -0400, David Lively wrote:
Same patch, resubmitted after fixing allocation issue you pointed out.
Looking more closely, I notice it was leaking when pool/source/name was
specified. Just added a strdup for the other case (when
pool/source/name defaults to pool/name) and a VIR_FREE in the
destructor.
Dave
On Tue, 2008-08-12 at 05:21 -0400, Daniel Veillard wrote:
On Fri, Aug 08, 2008 at 03:17:52PM -0400, David Lively wrote:
Hi Folks -
This small patch is a proposed prerequisite for the storage pool
discovery patch I submitted last week.
Daniel B proposed having storage pool discovery return a bunch of XML
storage source elements, rather than full pool elements (which
contain target-dependent details like the local pool name and device
or mount path -- things which don't need to be decided unless/until the
pool is defined).
I like his suggestion a lot, and I think it addresses the final
API-definition problem keeping storage pool discovery out of libvirt.
But ... it doesn't quite work for logical storage pools because those
are created from the pool name element (which is the same as the
volume group name).
I will let Daniel B comment on the pure storage aspects of the patch.
The patch looks fine to me except for one thing:
[...]
+if (options-flags VIR_STORAGE_BACKEND_POOL_SOURCE_NAME) {
+ret-source.name = virXPathString(conn,
string(/pool/source/name), ctxt);
+if (ret-source.name == NULL) {
+/* source name defaults to pool name */
+ret-source.name = ret-name;
+}
+}
I'm vary of allocation issues there.
Basically the patch adds a new string field to the record. But I could not
see
any deallocation addition in the patch, and the field seems to only be
set by copying another value. Maybe this is fine now, but if we ever update
the field in a different way (which I would expect at some point in the
code
evolution) then we will have a leak.
So instead of copying the string pointer directly, I suggest to do a
string
dup and in the freeing part of the record , do the VIR_FREE call for it.
Otherwise this looks fine to me
Daniel
This patch introduces a new source element name, which is distinct
from the pool name. name is used only by the logical storage
backend, and represents the volume group name. For convenience and
back-compatibility, source name defaults to the pool name if not
specified when the pool is defined, and pool name defaults to the
source name if not specified. There is no requirement that pool and
source name be the same, though.
Signed-off-by: David Lively [EMAIL PROTECTED]
diff --git a/src/storage_backend.h b/src/storage_backend.h
index 797ca01..422f26c 100644
--- a/src/storage_backend.h
+++ b/src/storage_backend.h
@@ -53,6 +53,7 @@ enum {
VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE = (11),
VIR_STORAGE_BACKEND_POOL_SOURCE_DIR = (12),
VIR_STORAGE_BACKEND_POOL_SOURCE_ADAPTER = (13),
+VIR_STORAGE_BACKEND_POOL_SOURCE_NAME= (14),
};
typedef struct _virStorageBackendPoolOptions virStorageBackendPoolOptions;
diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
index 0c4f6a5..382078b 100644
--- a/src/storage_backend_logical.c
+++ b/src/storage_backend_logical.c
@@ -80,7 +80,7 @@ virStorageBackendLogicalSetActive(virConnectPtr conn,
cmdargv[0] = VGCHANGE;
cmdargv[1] = on ? -ay : -an;
-cmdargv[2] = pool-def-name;
+cmdargv[2] = pool-def-source.name;
cmdargv[3] = NULL;
if (virRun(conn, cmdargv, NULL) 0)
@@ -213,7 +213,7 @@ virStorageBackendLogicalFindLVs(virConnectPtr conn,
LVS, --separator, :, --noheadings, --units, b,
--unbuffered, --nosuffix, --options,
lv_name,uuid,devices,seg_size,vg_extent_size,
-pool-def-name, NULL
+pool-def-source.name, NULL
};
int exitstatus;
@@ -288,7 +288,7 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn,
}
vgargv[n++] = VGCREATE;
-vgargv[n++] = pool-def-name;
+vgargv[n++] = pool-def-source.name;
pvargv[0] = PVCREATE;
pvargv[2] = NULL;
@@ -365,7 +365,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn,
const char *prog[] = {
VGS, --separator, :, --noheadings, --units, b, --unbuffered,
--nosuffix, --options, vg_size,vg_free,
-pool-def-name, NULL
+pool-def-source.name, NULL
};
int exitstatus;
@@ -419,7 +419,7 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn,
unsigned int flags ATTRIBUTE_UNUSED)
{
const char *cmdargv[] = {
-VGREMOVE, -f, pool-def-name, NULL
+VGREMOVE, -f, pool-def-source.name, NULL
};
if (virRun(conn
[libvirt] [PATCH] storage pool discovery
Hi Folks -
Here's my second pass at storage pool discovery. I've taken Daniel's
suggestion and made it return a single XML doc containing source
elements rather than an array of pool docs (and also incorporated
suggestions from Daniel V and Jim M).
Note that the storage source name patch is closely related
(without it, the source docs returned for logical pools aren't
correct).
Dave
diff --git a/configure.in b/configure.in
index 0513a72..f29a0fa 100644
--- a/configure.in
+++ b/configure.in
@@ -660,6 +660,11 @@ if test $with_storage_fs = yes -o $with_storage_fs = check; then
fi
fi
AM_CONDITIONAL([WITH_STORAGE_FS], [test $with_storage_fs = yes])
+if test $with_storage_fs = yes; then
+ AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin])
+ AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT],
+[Location or name of the showmount program])
+fi
AC_PATH_PROG([QEMU_IMG], [qemu-img], [], [$PATH:/sbin:/usr/sbin:/bin:/usr/bin])
if test -n $QEMU_IMG ; then
diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index 9c3e1c2..5308fa9 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -890,6 +890,14 @@ int virConnectListDefinedStoragePools(virConnectPtr conn,
int maxnames);
/*
+ * Query a host for storage pools of a particular type
+ */
+char * virConnectDiscoverStoragePools(virConnectPtr conn,
+ const char *type,
+ const char *srcSpec,
+ unsigned int flags);
+
+/*
* Lookup pool by name or uuid
*/
virStoragePoolPtr virStoragePoolLookupByName (virConnectPtr conn,
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index f077a26..3d06d76 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -890,6 +890,14 @@ int virConnectListDefinedStoragePools(virConnectPtr conn,
int maxnames);
/*
+ * Query a host for storage pools of a particular type
+ */
+char * virConnectDiscoverStoragePools(virConnectPtr conn,
+ const char *type,
+ const char *srcSpec,
+ unsigned int flags);
+
+/*
* Lookup pool by name or uuid
*/
virStoragePoolPtr virStoragePoolLookupByName (virConnectPtr conn,
diff --git a/qemud/remote.c b/qemud/remote.c
index b5a6ec9..43b3a56 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -2958,6 +2958,27 @@ remoteDispatchListStoragePools (struct qemud_server *server ATTRIBUTE_UNUSED,
}
static int
+remoteDispatchDiscoverStoragePools (struct qemud_server *server ATTRIBUTE_UNUSED,
+struct qemud_client *client,
+remote_message_header *req,
+remote_discover_storage_pools_args *args,
+remote_discover_storage_pools_ret *ret)
+{
+CHECK_CONN(client);
+
+ret-xml =
+virConnectDiscoverStoragePools (client-conn,
+args-type,
+args-srcSpec ? *args-srcSpec : NULL,
+args-flags);
+if (ret-xml == NULL)
+return -1;
+
+return 0;
+}
+
+
+static int
remoteDispatchStoragePoolCreate (struct qemud_server *server ATTRIBUTE_UNUSED,
struct qemud_client *client,
remote_message_header *req,
diff --git a/qemud/remote_dispatch_localvars.h b/qemud/remote_dispatch_localvars.h
index d889c8a..6915d5a 100644
--- a/qemud/remote_dispatch_localvars.h
+++ b/qemud/remote_dispatch_localvars.h
@@ -137,6 +137,8 @@ remote_domain_attach_device_args lv_remote_domain_attach_device_args;
remote_num_of_networks_ret lv_remote_num_of_networks_ret;
remote_storage_pool_get_info_args lv_remote_storage_pool_get_info_args;
remote_storage_pool_get_info_ret lv_remote_storage_pool_get_info_ret;
+remote_discover_storage_pools_args lv_remote_discover_storage_pools_args;
+remote_discover_storage_pools_ret lv_remote_discover_storage_pools_ret;
remote_list_storage_pools_args lv_remote_list_storage_pools_args;
remote_list_storage_pools_ret lv_remote_list_storage_pools_ret;
remote_domain_restore_args lv_remote_domain_restore_args;
diff --git a/qemud/remote_dispatch_proc_switch.h b/qemud/remote_dispatch_proc_switch.h
index ebb2433..1850b92 100644
--- a/qemud/remote_dispatch_proc_switch.h
+++ b/qemud/remote_dispatch_proc_switch.h
@@ -41,6 +41,15 @@ case REMOTE_PROC_AUTH_SASL_STEP:
case REMOTE_PROC_CLOSE:
fn = (dispatch_fn) remoteDispatchClose;
break;
[libvirt] anyone implementing host device enumeration?
Hi - I'm about to start working on host device enumeration, along the (HAL-ish) lines of what was discussed back in April: https://www.redhat.com/archives/libvir-list/2008-April/msg5.html I know the xml details haven't been fully fleshed out, but there seems to be agreement that it will be a fairly direct mapping from (a subset of the) HAL info to the details that we need in the xml. Doubtless it will take a while to figure out exactly what subset suffices (and, for that matter, if everything needed is available via HAL ...), but I think the work is well-defined for some of the obvious details (discussed in the above thread) on which there's broad agreement. Is anyone working on such an implementation? Thanks, Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] introducing source name (for logical storage pools)
Hi Folks -
This small patch is a proposed prerequisite for the storage pool
discovery patch I submitted last week.
Daniel B proposed having storage pool discovery return a bunch of XML
storage source elements, rather than full pool elements (which
contain target-dependent details like the local pool name and device
or mount path -- things which don't need to be decided unless/until the
pool is defined).
I like his suggestion a lot, and I think it addresses the final
API-definition problem keeping storage pool discovery out of libvirt.
But ... it doesn't quite work for logical storage pools because those
are created from the pool name element (which is the same as the
volume group name).
This patch introduces a new source element name, which is
distinct
from the pool name. name is used only by the logical storage
backend, and represents the volume group name. For convenience and
back-compatibility, source name defaults to the pool name if not
specified when the pool is defined, and pool name defaults to the
source name if not specified. There is no requirement that pool and
source name be the same, though.
While admittedly it seems a little weird, it does allow more
flexibility (pool name not tied to vol group name), and allows source
to fully specify a logical pool source, as it does for the other pool
types[*]. Defaulting the source name from the pool name means all
existing pool XML definitions continue to work. Defaulting the pool
name from the source name is simply a matter of convenience (but perhaps
a step too far?)
If this is accepted, logical pool discovery can then return a bunch of
sources like:
sourcenameVolGroup00/name/source
sourcenameVolGroup01/name/source
Please note I'll be on vacation next week (Aug 11-15), so I may not be
responding until the following week.
Thanks,
Dave
[*] Well ... almost. Note that directory pools have a similar issue --
the source of the pool is given by the target path -- there's no
source. I suppose implementing directory pool discovery (for the sake
of completeness) means addressing this as well, maybe via some similar
mechanism ...
This patch introduces a new source element name, which is distinct
from the pool name. name is used only by the logical storage
backend, and represents the volume group name. For convenience and
back-compatibility, source name defaults to the pool name if not
specified when the pool is defined, and pool name defaults to the
source name if not specified. There is no requirement that pool and
source name be the same, though.
Signed-off-by: David Lively [EMAIL PROTECTED]
diff --git a/src/storage_backend.h b/src/storage_backend.h
index 797ca01..422f26c 100644
--- a/src/storage_backend.h
+++ b/src/storage_backend.h
@@ -53,6 +53,7 @@ enum {
VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE = (11),
VIR_STORAGE_BACKEND_POOL_SOURCE_DIR = (12),
VIR_STORAGE_BACKEND_POOL_SOURCE_ADAPTER = (13),
+VIR_STORAGE_BACKEND_POOL_SOURCE_NAME= (14),
};
typedef struct _virStorageBackendPoolOptions virStorageBackendPoolOptions;
diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
index 0c4f6a5..382078b 100644
--- a/src/storage_backend_logical.c
+++ b/src/storage_backend_logical.c
@@ -80,7 +80,7 @@ virStorageBackendLogicalSetActive(virConnectPtr conn,
cmdargv[0] = VGCHANGE;
cmdargv[1] = on ? -ay : -an;
-cmdargv[2] = pool-def-name;
+cmdargv[2] = pool-def-source.name;
cmdargv[3] = NULL;
if (virRun(conn, cmdargv, NULL) 0)
@@ -213,7 +213,7 @@ virStorageBackendLogicalFindLVs(virConnectPtr conn,
LVS, --separator, :, --noheadings, --units, b,
--unbuffered, --nosuffix, --options,
lv_name,uuid,devices,seg_size,vg_extent_size,
-pool-def-name, NULL
+pool-def-source.name, NULL
};
int exitstatus;
@@ -288,7 +288,7 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn,
}
vgargv[n++] = VGCREATE;
-vgargv[n++] = pool-def-name;
+vgargv[n++] = pool-def-source.name;
pvargv[0] = PVCREATE;
pvargv[2] = NULL;
@@ -365,7 +365,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn,
const char *prog[] = {
VGS, --separator, :, --noheadings, --units, b, --unbuffered,
--nosuffix, --options, vg_size,vg_free,
-pool-def-name, NULL
+pool-def-source.name, NULL
};
int exitstatus;
@@ -419,7 +419,7 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn,
unsigned int flags ATTRIBUTE_UNUSED)
{
const char *cmdargv[] = {
-VGREMOVE, -f, pool-def-name, NULL
+VGREMOVE, -f, pool-def-source.name, NULL
};
if (virRun(conn, cmdargv, NULL) 0)
@@ -535,6 +535,7 @@ virStorageBackend virStorageBackendLogical = {
.deleteVol = virStorageBackendLogicalDeleteVol,
.poolOptions = {
+.flags = VIR_STORAGE_BACKEND_POOL_SOURCE_NAME,
.formatFromString
Re: [libvirt] [PATCH] storage pool discovery
On Fri, 2008-08-01 at 10:40 +0100, Daniel P. Berrange wrote: On Wed, Jul 30, 2008 at 04:30:01PM -0400, David Lively wrote: Finally, there's an underspecification issue. The XML pool descriptions returned are supposed to be usable as valid input to virStoragePoolDefineXML. But these descriptions include some data (pool name, target path) that isn't specified by the discover input or the discovered resources. For now, I'm making up a somewhat arbitrary pool name (logical: VolGroup name, netfs: last component of export path). And I don't even specify targetpath in the netfs discovery output (which means it's not valid input to virStoragePoolDefineXML until a target path is added). Yep, my original intention was that you could send the XML straight back into the PoolDefineXML api. One alternative I've thought of though, is that it perhaps it might be nicer to return all the discovered pools as one XML doc poolList type='netfs' source host name='someserver'/ dir path='/exports/home'/ /source source host name='someserver'/ dir path='/exports/web'/ /source source host name='someserver'/ dir path='/exports/ftp'/ /source /poolList And just let the caller decide how they want to use this - they may not even want to define pools from them immediately - instead they might just want to display list of NFS exports to the user. It'd be easier than having to make up data for the name and target elements which is really something the client app wants to decide upon. This is really two suggestions rolled into one: (1) just return source xml (not whole pool xml), and (2) instead of returning an array of strings, return a single (xml) string wrapped with a poolList element Either (1) or (2) could be done independently of the other, and I think they're both worth considering. I like the fact that (2) lets us auto-generate the python binding, but I don't have strong feelings either way. I like (1) a lot, but it doesn't really work correctly for logical storage pools in their current incarnation. The source element for a logical pool currently consists only of the device paths to the physical volumes backing the volume group. In particular, it doesn't specify the volume group name (which seems to be assumed to be the same as the libvirt pool name??). This could be fixed by allowing the source element for an existing volume group to specify (only) the volume group name (which might be different from the pool name). In this way, logical pool discovery can return source elements that fully specify existing pools. And the client app can decide on pool names and target paths as you propose. [Note I'm proposing *extending* logical pool creation, so existing XML should continue to work. Specifically I'm proposing we extend source with an optional name element which the logical storage backend will interpret as volume group name. For back-compatibility, the backend will default the source name to the pool name when the source name isn't specified. The source name is used to instantiate existing volume groups as well as to name new volume groups (whose source must also specify the backing physical devices).] Does this sound reasonable? There's some emacs rules in the HACKING file which will make sure it does correct indentation for you. BTW, anyone fancy adding VIM equivalents... Sorry, I'm an emacs guy :-) [What we *really* need is an emacs mode (indent-assimilation-mode) that will figure out the indentation settings for each file from the code that's already there ... (assuming it's consistent ...). I suppose it wouldn't be that hard to at least get c-basic-offset, c-indent-level, and indent-tabs-mode right. (I know you can embed emacs directives in source comments, but that's kind of ugly ...)] In any case, thanks much for your comments. I'm finally getting back to working on this, so I should be able to submit my next take soon. Thanks, Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage pool discovery
Thanks much for your comments, Jim. They all look reasonable (though I
may be intentionally trimming a NL in one of these cases -- I'm
checking).
And I'll start following the HACKING file recommendations before
submitting my next attempt :-)
(Though note I'm not yet submitting tests since we haven't really
finished hashing out the API - at least some crucial XML details ...)
Dave
On Mon, 2008-08-04 at 08:25 +0200, Jim Meyering wrote:
Hi David,
I spotted a few things that would be good to change:
David Lively [EMAIL PROTECTED] wrote:
diff --git a/configure.in b/configure.in
index 8e04f14..5ef0384 100644
--- a/configure.in
+++ b/configure.in
@@ -660,6 +660,10 @@ if test $with_storage_fs = yes -o
$with_storage_fs = check; then
fi
fi
AM_CONDITIONAL([WITH_STORAGE_FS], [test $with_storage_fs = yes])
+if test $with_storage_fs = yes; then
+ AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin])
+ AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT], [Location or name of the
showmount program])
Please split long lines:
AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT],
[Location or name of the showmount program])
...
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
...
+static int
+virStorageBackendFileSystemNetDiscoverPoolsFunc(virConnectPtr conn
ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool
ATTRIBUTE_UNUSED,
+ char **const groups,
+ void *data)
+{
+virNetfsDiscoverState *state = data;
+virStringList *newItem;
+const char *name, *path;
+int len;
+
+path = groups[0];
+
+name = rindex(path, '/');
rindex is deprecated. Using it causes portability problems.
Use strrchr instead.
+if (name == NULL) {
+ virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, %s, _(no / in
path?));
If you include the offending string in the diagnostic
and add a word of description, it'll be more useful:
virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
_(invalid netfs path (no slash): %s), path);
+ return -1;
+}
+name += 1;
+
+/* Append new XML desc to list */
+
+if (VIR_ALLOC(newItem) != 0) {
+virStorageReportError(conn, VIR_ERR_NO_MEMORY, %s, _(new xml
desc));
+return -1;
+}
+
+
+/* regexp pattern is too greedy, so we may have trailing spaces to
trim.
+ * NB showmount output ambiguous for (evil) exports with names
w/trailing whitespace
Too long.
+ */
+len = 0;
+while (name[len])
+len++;
This is equivalent to the three lines above:
len = strlen (name);
+while (c_isspace(name[len-1]))
+len--;
It is customary to trim spaces and TABs (but less so CR, FF, NL),
so c_isblank might be better than c_isspace here.
...
+static int
+virStorageBackendFileSystemNetDiscoverPools(virConnectPtr conn,
...
+ cleanup:
+if (doc)
+ xmlFreeDoc(doc);
+if (xpath_ctxt)
You can remove this if test. It's unnecessary.
BTW, make syntax-check should spot this and fail because of it.
+ xmlXPathFreeContext(xpath_ctxt);
+VIR_FREE(state.host);
+while (state.list) {
+ p = state.list-next;
+ VIR_FREE(state.list);
+ state.list = p;
+}
+
+return n_descs;
+}
...
diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
index 9a0c27f..3c16d20 100644
--- a/src/storage_backend_logical.c
+++ b/src/storage_backend_logical.c
...
@@ -257,6 +258,90 @@ virStorageBackendLogicalRefreshPoolFunc(virConnectPtr
conn ATTRIBUTE_UNUSED,
static int
+virStorageBackendLogicalDiscoverPoolsFunc(virConnectPtr conn
ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool
ATTRIBUTE_UNUSED,
+ size_t n_tokens,
+ char **const tokens,
+ void *data)
+{
+virStringList **rest = data;
+virStringList *newItem;
+const char *name;
+int len;
+
+/* Append new XML desc to list */
+
+if (VIR_ALLOC(newItem) != 0) {
+virStorageReportError(conn, VIR_ERR_NO_MEMORY, %s, _(new xml
desc));
+return -1;
+}
+
+if (n_tokens != 1) {
+ virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, %s, _(n_tokens
should be 1));
+ return -1;
+}
+
+
+name = tokens[0];
+virSkipSpaces(name);
Is is necessary to skip leading backslashes and carriage returns here?
+len = 0;
+while (name[len])
+len++;
+while (c_isspace(name[len-1]))
+len--;
A zero-length or all-space name would lead to referencing name[-1].
You can use this instead:
while (len c_isspace
[libvirt] [PATCH] storage pool discovery
Hi -
The attached patch implements virConnectDiscoverStoragePools mostly as
specified by Daniel Berrange in:
http://www.redhat.com/archives/libvir-list/2008-February/msg00107.html
Daniel wasn't happy with the interface for this function because it
wasn't sufficiently general, so it currently doesn't exist. I've
attempted to generalize his proposed interface by changing the
hostname argument to a srcSpec argument, which is intended to be a
XML storage pool source element. Note that srcSpec is not necessary
for some (local) pool types.
For example, to discover existing LVM volume groups, there's no need
to specify a source:
virsh # pool-discover logical
While discovering nfs shares requires a source document to specify
the host to query:
virsh # pool-discover netfs sourcehost name='share' //source
Currently (i.e., in this patch) I've implemented discovery support
only for the logical and netfs pools. I plan on covering the other
types once we've agreed upon the API. While this patch is rather large,
the vast majority of it is generic (not specific to any particular
type of storage pool) code, which I mostly took from Daniel's
submission. I could easily break the storage-pool-type-specific pieces
into separate patches, but those changes are already segregated into the
appropriate storage_backend_ files.
* Known Issues *
I made the virsh interface take a XML string rather than a filename.
I found it convenient, but AFAIK the rest of the commands that take XML
take it from a file, so perhaps that should be changed (just let me know
what you'd prefer).
I realize the description of srcSpec is kind of fuzzy. For instance,
for netfs discovery, the source must have a host element (to specify
the host to query), but shouldn't have a dir element since the
exported dirs are exactly what we're trying to discover in this case.
So this really needs to be documented accurately for each storage pool
type. (Where?)
Finally, there's an underspecification issue. The XML pool
descriptions returned are supposed to be usable as valid input to
virStoragePoolDefineXML. But these descriptions include some data (pool
name, target path) that isn't specified by the discover input or the
discovered resources. For now, I'm making up a somewhat arbitrary pool
name (logical: VolGroup name, netfs: last component of export path).
And I don't even specify targetpath in the netfs discovery output
(which means it's not valid input to virStoragePoolDefineXML until a
target path is added).
Dave
diff --git a/configure.in b/configure.in
index 8e04f14..5ef0384 100644
--- a/configure.in
+++ b/configure.in
@@ -660,6 +660,10 @@ if test $with_storage_fs = yes -o $with_storage_fs = check; then
fi
fi
AM_CONDITIONAL([WITH_STORAGE_FS], [test $with_storage_fs = yes])
+if test $with_storage_fs = yes; then
+ AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin])
+ AC_DEFINE_UNQUOTED([SHOWMOUNT], [$SHOWMOUNT], [Location or name of the showmount program])
+fi
AC_PATH_PROG([QEMU_IMG], [qemu-img], [], [$PATH:/sbin:/usr/sbin:/bin:/usr/bin])
if test -n $QEMU_IMG ; then
diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index 8980bc2..463cf2b 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -890,6 +890,15 @@ int virConnectListDefinedStoragePools(virConnectPtr conn,
int maxnames);
/*
+ * Query a host for storage pools of a particular type
+ */
+int virConnectDiscoverStoragePools(virConnectPtr conn,
+ const char *type,
+ const char *srcSpec,
+ unsigned int flags,
+ char ***xmlDesc);
+
+/*
* Lookup pool by name or uuid
*/
virStoragePoolPtr virStoragePoolLookupByName (virConnectPtr conn,
@@ -979,6 +988,13 @@ char * virStorageVolGetXMLDesc (virStorageVolPtr pool,
char * virStorageVolGetPath(virStorageVolPtr vol);
+typedef struct _virStringList virStringList;
+
+struct _virStringList {
+char *val;
+struct _virStringList *next;
+};
+
#ifdef __cplusplus
}
#endif
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index bd5195c..dc4f049 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -890,6 +890,15 @@ int virConnectListDefinedStoragePools(virConnectPtr conn,
int maxnames);
/*
+ * Query a host for storage pools of a particular type
+ */
+int virConnectDiscoverStoragePools(virConnectPtr conn,
+ const char *type,
+ const char *srcSpec,
+ unsigned int flags,
+ char ***xmlDesc);
+
+/*
* Lookup pool by name or uuid
*/
virStoragePoolPtr virStoragePoolLookupByName (virConnectPtr conn,
@@ -979,6 +988,13 @@ char *
Re: [libvirt] [PATCH] Fix logical storage pool operation on SLES10-SP2
On Tue, 2008-07-29 at 09:44 +0100, Daniel P. Berrange wrote: BTW, what version of the LVM tools is SLES using - its probably useful to note that in the comment you added, in case the same problem is particular to a version, rather than just SLES lvm2 2.02.17 (-7.19, x86_64) But it's hard to know (without reading the ChangeLog in detail) how much backporting is hidden in the distro-specific part of the version (-7.19) ... Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix logical storage pool operation on SLES10-SP2
The attached patch adjusts for a difference in behavior in the LVM
utilities 'lvs' and 'vgs'. The SLES10-SP2 versions of these (and
presumably others) append a trailing separator. This patch simply
adjusts the regexps to allow (but not require) this.
I thought just adding the :? to the regexps would do this, but this
was leaving the trailing separator in the last group match, so I ended
up tweaking the preceding group pattern as well.
Dave
commit 021470f5cfa0e770f09c73cf8a2fc270121378f6
Author: David Lively [EMAIL PROTECTED]
Date: Mon Jul 28 15:05:30 2008 -0400
Some distros' (e.g. SLES10-SP2) lvm utilities (lvs, vgs) put the specified
separator at the end of each input line. Adjusted the regexes used by the
logical storage backend to allow this.
Signed-off-by: David Lively [EMAIL PROTECTED]
diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
index 674a74f..52e645b 100644
--- a/src/storage_backend_logical.c
+++ b/src/storage_backend_logical.c
@@ -201,9 +201,11 @@ virStorageBackendLogicalFindLVs(virConnectPtr conn,
* Pull out name uuid, device, device extent start #, segment size, extent size.
*
* NB can be multiple rows per volume if they have many extents
+ *
+ * NB lvs from some distros (e.g. SLES10 SP2) outputs trailing : on each line
*/
const char *regexes[] = {
-^\\s*(\\S+):(\\S+):(\\S+)\\((\\S+)\\):(\\S+):(\\S+)\\s*$
+^\\s*(\\S+):(\\S+):(\\S+)\\((\\S+)\\):(\\S+):([0-9]+):?\\s*$
};
int vars[] = {
6
@@ -442,9 +444,11 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn,
*10603200512:4328521728
*
* Pull out size free
+ *
+ * NB vgs from some distros (e.g. SLES10 SP2) outputs trailing : on each line
*/
const char *regexes[] = {
-^\\s*(\\S+):(\\S+)\\s*$
+^\\s*(\\S+):([0-9]+):?\\s*$
};
int vars[] = {
2
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC? finding potential storage pool resources
On Thu, 2008-07-17 at 22:42 +0100, Daniel P. Berrange wrote: You're not missing anything - this is a TODO item. When I wrote the original storage APIs, I had a prototype http://www.redhat.com/archives/libvir-list/2008-February/msg00107.html http://www.redhat.com/archives/libvir-list/2008-February/msg00108.html int virConnectDiscoverStoragePools(virConnectPtr conn, const char *hostname, const char *type, unsigned int flags, char ***xmlDesc); Which was intended to probe for available storage of the requested type (eg, LVM, vs disks, vs iSCSI targets, etc, etc), and return a list of XML documents describing each discovered object. This could be fed into the API virStoragePoolDefineXML. I didn't include this in the end, because I wasn't happy with the API contract. For example, it only allows a hostname to be specified as metadata, but it may be desirable to include a port number as well for network based storage. Thanks for the pointers. I like your proposal, but I agree the API contract isn't general enough (what about possible future driver types??) Perhaps your hostname parameter could be replaced with a source_spec parameter which is an XML document consisting of a storage pool source element (with entries appropriate to the given storage pool type)? So for network storage, you'd pass a source_spec (a string) like: source host name=foo.bar.com port=123 / /source source_spec would be optional for some storage pool types, like disk and logical. (And if specified, it could restrict the discovery to those sources listed?? There are scalability issues as SANs proliferate ... even on hosts with a single HBA ) Using the storage pool source element here should essentially guarantee this is general enough to support all storage drivers. (If a future storage driver requires the source XML to be extended, the discovery API is extended in the same way.) (I like your later hardware device enumeration API proposal too. I'm ignoring it for now for the sake of simplicity ...) Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] configure.in: make --with-xen-distdir work for 64bit xen too
Currently running autogen.sh with --with-xen-distdir=something fails to find libxenstore if there's only a 64-bit version, and subsequently fails to enable xen support (i.e., ends up with WITH_XEN=0). This one-line patch fixes that by telling it to search both lib and lib64. I guess it would be better to search only the appropriate directory (lib OR lib64) rather than both, but I'm not sure (being an autoconf-dummy) how to ask about the target architecture. Dave configure.in |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) [diffs: repository libvirt] diff --git a/configure.in b/configure.in index 4b9669f..8e04f14 100644 --- a/configure.in +++ b/configure.in @@ -235,7 +235,7 @@ AC_ARG_WITH([xen-distdir], [AC_HELP_STRING([--with-xen-distdir=path], if test x$with_xen_distdir != x then CPPFLAGS=$CPPFLAGS -I$withval/install/usr/include -LDFLAGS=$LDFLAGS -L$withval/install/usr/lib +LDFLAGS=$LDFLAGS -L$withval/install/usr/lib -L$withval/install/usr/lib64 fi LIBVIRT_FEATURES= -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] RFC? finding potential storage pool resources
Hi -
I'm looking into using (which I think means extending) libvirt to
enumerate potential storage pool resources, in particular:
* existing physical disk device names (for creating disk pools)
* existing logical volume group names (for creating logical pools)
Note that List{Defined,Active}StorageGroups don't do the trick. Suppose
this is a new host and I'm trying to start defining the storage pools
(and I want to be able to use existing volume groups, for example). I
don't see how to do that within the current libvirt framework. If I'm
missing something, please let me know (and ignore the rest of this
message ...).
This could be done by adding some new calls like:
int virConnectListPhysDisks(virConnectPtr conn, char ** const name, int
maxnames)
int virConnectListLogicalVolGroups(virConnectPtr conn, char ** const name,
int maxnames)
... plus a pair of NumOf functions ...
But these are each storage-driver specific. For example, if I'm not
using the logical storage driver, I have no need (or means) of listing
volume groups. So maybe it's cleaner to fold these two functions into
one, now parameterized by storage driver type:
int virConnectListStorageSources(virConnectPtr conn, const char *type, char
** const name, int maxnames)
... plus a NumOf function ...
where type is one of the supported storage pool types.
So, if type is disk, ListStorageSources acts like ListPhysDisks,
and if type is logical, ListStorageSources acts like
ListLogicalVolumeGroups,
(and we return empty lists or some sort of unsupported error for any
other types ... can't list all possible network servers, for instance).
What do you all think?
Thanks,
Dave
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
