7:06 p.m.
We currently have a restriction that virConnectPtr objects
must only be used by a single thread. At the same time we
have a number of operations that can take a very long time.
This means that applications are pretty much forced to use
multi-threading if they want to continue to respond to user
input, and thus are also forced to use multiple virConnectPtr
objects.
This is sub-optimal because opening multiple virConnectPtr
objects may well result in the user being prompted for auth
credentials multiple times. It also doesn't provide any way
of providing progess information on the long running jobs,
nor a way to cancel them.
Thus the following series of patches introduces the concept of
an 'asynchronous background job', represented by a virJobPtr
object in the libvirt public API.
A job has an elapsed time, and may optionally include an
estimated time of completion. If it is a job with a bounded
amount of work, it will also have a percentage completion
counter. There is the ability to request cancellation of an
in progress job, and fetch any error associated with a
completed job.
The primary reason I implemented this support, is that the
storage APIs will make the current situation untennable.
Allocating storage can take a seriously long time (many
many minutes if cloning multi-GB files). This absolutely
requires progress information and an ability to cancel
an operation.
There are a number of existing APIs which can benefit from
this, hence I decided to work on this separately from the
main storage APIs. The APIs which can make use of this are
virDomainCreateLinux, virDomainCreate, virNetworkCreate,
virNetworkCreateXML, virDomainSave, virDomainRestore, and
virDomainDumpCore. For all of these we add a second variant
postfixed with 'Job' in the name, returning a virJobPtr object
This work isn't finished, but I wanted to put these patches
out for comment before going further. In particular I need
to figure out how to hook this up to the remote driver and
daemon, and deal with a few lifecycle issues. eg what todo
if a virConnectPtr object is released while a background job
is active.
With all 5 patches applied you should be able to run the
following test case:
virsh --connect test:///default save --verbose test foo
No other drivers / commands are hooked up aside from 'save'
in the 'test' driver.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
7:08 p.m.
New subject: [Libvir] RFC: PATCH 1/5: cleanup connection ref counting
The referencing counting code for Connect/Domain/Network
objects has many repeated codepaths, not all of which are
correct. eg, the virFreeDomain method forgets to release
networks when garbage collecting a virConnectPtr, and the
virFreeNetwork method forgets to release domains.
So I've moved the code for garbage collecting a virConnectPtr
object into a new virUnrefConnect() method which can be called
from virFreeConnect, virFreeDomain and virFreeNetwork.
I have also switched from xmlMutex to the POSIX standard
pthread_mutex_t object. In theory the former may give more
portability, but the patches which follow are also going
to require actual pthread_t objects, for which libxml has
no wrappers. Thus it is pointless using the xmlMutex
abstraction. The added plus, is that pthread_mutex_t objects
do not require any memory allocation, merely initialization
of their pre-allocated contents so it simplifies a little
code.
Finally, the code in hash.c for dealing with ref counting
seriously abused the 'goto' statement with multiple jumps
overlapping each other in a single method. This made it
very hard to follow. So I removed the use of goto in most
places so its only used in error cleanup paths, and not
for regular flow control.
Oh, and in cleaning up internal.h I found an odd declaration
of some functions from xs_internal.h which could have been
made static. So I made them static.
hash.c | 325 ++++++++++++++++++++++++++--------------------------------
internal.h | 55 ++++-----
xs_internal.c | 28 ----
3 files changed, 180 insertions(+), 228 deletions(-)
Dan.
diff -r 3a46c145fac4 src/hash.c
--- a/src/hash.c Thu Jan 03 21:48:19 2008 -0500
+++ b/src/hash.c Fri Jan 04 17:29:33 2008 -0500
@@ -25,6 +25,7 @@
#include <libxml/threads.h>
#include "internal.h"
#include "hash.h"
+#include <pthread.h>
#define MAX_HASH_LEN 8
@@ -677,60 +678,70 @@ virGetConnect(void) {
ret->networks = virHashCreate(20);
if (ret->networks == NULL)
goto failed;
- ret->hashes_mux = xmlNewMutex();
- if (ret->hashes_mux == NULL)
- goto failed;
-
- ret->uses = 1;
+
+ pthread_mutex_init(&ret->lock, NULL);
+
+ ret->refs = 1;
return(ret);
failed:
if (ret != NULL) {
- if (ret->domains != NULL)
- virHashFree(ret->domains, (virHashDeallocator) virDomainFreeName);
- if (ret->networks != NULL)
- virHashFree(ret->networks, (virHashDeallocator) virNetworkFreeName);
- if (ret->hashes_mux != NULL)
- xmlFreeMutex(ret->hashes_mux);
+ if (ret->domains != NULL)
+ virHashFree(ret->domains, (virHashDeallocator) virDomainFreeName);
+ if (ret->networks != NULL)
+ virHashFree(ret->networks, (virHashDeallocator) virNetworkFreeName);
+
+ pthread_mutex_destroy(&ret->lock);
free(ret);
}
return(NULL);
}
/**
- * virFreeConnect:
- * @conn: the hypervisor connection
- *
- * Release the connection. if the use count drops to zero, the structure is
- * actually freed.
- *
- * Returns the reference count or -1 in case of failure.
- */
-int
-virFreeConnect(virConnectPtr conn) {
- int ret;
-
- if ((!VIR_IS_CONNECT(conn)) || (conn->hashes_mux == NULL)) {
- virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
- return(-1);
- }
- xmlMutexLock(conn->hashes_mux);
- conn->uses--;
- ret = conn->uses;
- if (ret > 0) {
- xmlMutexUnlock(conn->hashes_mux);
- return(ret);
- }
+ * Release all memory associated with a connection.
+ * The conn.lock mutex must be held prior to calling this
+ */
+int
+virUnrefConnect(virConnectPtr conn) {
+ conn->refs--;
+ if (conn->refs > 0)
+ return conn->refs;
if (conn->domains != NULL)
virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName);
if (conn->networks != NULL)
virHashFree(conn->networks, (virHashDeallocator) virNetworkFreeName);
- if (conn->hashes_mux != NULL)
- xmlFreeMutex(conn->hashes_mux);
virResetError(&conn->err);
+
+ pthread_mutex_unlock(&conn->lock);
+ pthread_mutex_destroy(&conn->lock);
free(conn);
- return(0);
+
+ return 0;
+}
+
+/**
+ * virFreeConnect:
+ * @conn: the hypervisor connection
+ *
+ * Release the connection. if the use count drops to zero, the structure is
+ * actually freed.
+ *
+ * Returns the reference count or -1 in case of failure.
+ */
+int
+virFreeConnect(virConnectPtr conn) {
+ int refs;
+
+ if ((!VIR_IS_CONNECT(conn))) {
+ virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ return(-1);
+ }
+ pthread_mutex_lock(&conn->lock);
+ refs = virUnrefConnect(conn);
+ if (refs > 0)
+ pthread_mutex_unlock(&conn->lock);
+ return (refs);
}
/**
@@ -750,57 +761,50 @@ __virGetDomain(virConnectPtr conn, const
__virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) {
virDomainPtr ret = NULL;
- if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL) ||
- (conn->hashes_mux == NULL)) {
+ if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) {
virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(NULL);
}
- xmlMutexLock(conn->hashes_mux);
+ pthread_mutex_lock(&conn->lock);
/* TODO search by UUID first as they are better differenciators */
ret = (virDomainPtr) virHashLookup(conn->domains, name);
+ /* TODO check the UUID */
+ if (ret == NULL) {
+ ret = (virDomainPtr) calloc(1, sizeof(*ret));
+ if (ret == NULL) {
+ virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating domain"));
+ goto error;
+ }
+ ret->name = strdup(name);
+ if (ret->name == NULL) {
+ virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating domain"));
+ goto error;
+ }
+ ret->magic = VIR_DOMAIN_MAGIC;
+ ret->conn = conn;
+ ret->id = -1;
+ if (uuid != NULL)
+ memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
+
+ if (virHashAddEntry(conn->domains, name, ret) < 0) {
+ virHashError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("failed to add domain to connection hash table"));
+ goto error;
+ }
+ conn->refs++;
+ }
+ ret->refs++;
+ pthread_mutex_unlock(&conn->lock);
+ return(ret);
+
+error:
+ pthread_mutex_unlock(&conn->lock);
if (ret != NULL) {
- /* TODO check the UUID */
- goto done;
- }
-
- /*
- * not found, allocate a new one
- */
- ret = (virDomainPtr) calloc(1, sizeof(virDomain));
- if (ret == NULL) {
- virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating domain"));
- goto error;
- }
- ret->name = strdup(name);
- if (ret->name == NULL) {
- virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating domain"));
- goto error;
- }
- ret->magic = VIR_DOMAIN_MAGIC;
- ret->conn = conn;
- ret->id = -1;
- if (uuid != NULL)
- memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
-
- if (virHashAddEntry(conn->domains, name, ret) < 0) {
- virHashError(conn, VIR_ERR_INTERNAL_ERROR,
- _("failed to add domain to connection hash table"));
- goto error;
- }
- conn->uses++;
-done:
- ret->uses++;
- xmlMutexUnlock(conn->hashes_mux);
- return(ret);
-
-error:
- xmlMutexUnlock(conn->hashes_mux);
- if (ret != NULL) {
- if (ret->name != NULL)
- free(ret->name );
- free(ret);
+ if (ret->name != NULL)
+ free(ret->name );
+ free(ret);
}
return(NULL);
}
@@ -817,29 +821,31 @@ error:
*/
int
virFreeDomain(virConnectPtr conn, virDomainPtr domain) {
- int ret = 0;
+ int refs;
if ((!VIR_IS_CONNECT(conn)) || (!VIR_IS_CONNECTED_DOMAIN(domain)) ||
- (domain->conn != conn) || (conn->hashes_mux == NULL)) {
+ (domain->conn != conn)) {
virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(-1);
}
- xmlMutexLock(conn->hashes_mux);
+ pthread_mutex_lock(&conn->lock);
/*
* decrement the count for the domain
*/
- domain->uses--;
- ret = domain->uses;
- if (ret > 0)
- goto done;
+ domain->refs--;
+ refs = domain->refs;
+ if (refs > 0) {
+ pthread_mutex_unlock(&conn->lock);
+ return (refs);
+ }
/* TODO search by UUID first as they are better differenciators */
-
if (virHashRemoveEntry(conn->domains, domain->name, NULL) < 0) {
virHashError(conn, VIR_ERR_INTERNAL_ERROR,
- _("domain missing from connection hash table"));
- goto done;
+ _("domain missing from connection hash table"));
+ pthread_mutex_unlock(&conn->lock);
+ return (refs);
}
domain->magic = -1;
domain->id = -1;
@@ -847,23 +853,11 @@ virFreeDomain(virConnectPtr conn, virDom
free(domain->name);
free(domain);
- /*
- * decrement the count for the connection
- */
- conn->uses--;
- if (conn->uses > 0)
- goto done;
-
- if (conn->domains != NULL)
- virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName);
- if (conn->hashes_mux != NULL)
- xmlFreeMutex(conn->hashes_mux);
- free(conn);
+
+ refs = virUnrefConnect(conn);
+ if (refs > 0)
+ pthread_mutex_unlock(&conn->lock);
return(0);
-
-done:
- xmlMutexUnlock(conn->hashes_mux);
- return(ret);
}
/**
@@ -883,56 +877,48 @@ __virGetNetwork(virConnectPtr conn, cons
__virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) {
virNetworkPtr ret = NULL;
- if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL) ||
- (conn->hashes_mux == NULL)) {
+ if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) {
virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(NULL);
}
- xmlMutexLock(conn->hashes_mux);
+ pthread_mutex_lock(&conn->lock);
/* TODO search by UUID first as they are better differenciators */
ret = (virNetworkPtr) virHashLookup(conn->networks, name);
+ if (ret == NULL) {
+ ret = (virNetworkPtr) calloc(1, sizeof(*ret));
+ if (ret == NULL) {
+ virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating network"));
+ goto error;
+ }
+ ret->name = strdup(name);
+ if (ret->name == NULL) {
+ virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating network"));
+ goto error;
+ }
+ ret->magic = VIR_NETWORK_MAGIC;
+ ret->conn = conn;
+ if (uuid != NULL)
+ memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
+
+ if (virHashAddEntry(conn->networks, name, ret) < 0) {
+ virHashError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("failed to add network to connection hash table"));
+ goto error;
+ }
+ conn->refs++;
+ }
+ ret->refs++;
+ pthread_mutex_unlock(&conn->lock);
+ return(ret);
+
+error:
+ pthread_mutex_unlock(&conn->lock);
if (ret != NULL) {
- /* TODO check the UUID */
- goto done;
- }
-
- /*
- * not found, allocate a new one
- */
- ret = (virNetworkPtr) calloc(1, sizeof(virNetwork));
- if (ret == NULL) {
- virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating network"));
- goto error;
- }
- ret->name = strdup(name);
- if (ret->name == NULL) {
- virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating network"));
- goto error;
- }
- ret->magic = VIR_NETWORK_MAGIC;
- ret->conn = conn;
- if (uuid != NULL)
- memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
-
- if (virHashAddEntry(conn->networks, name, ret) < 0) {
- virHashError(conn, VIR_ERR_INTERNAL_ERROR,
- _("failed to add network to connection hash table"));
- goto error;
- }
- conn->uses++;
-done:
- ret->uses++;
- xmlMutexUnlock(conn->hashes_mux);
- return(ret);
-
-error:
- xmlMutexUnlock(conn->hashes_mux);
- if (ret != NULL) {
- if (ret->name != NULL)
- free(ret->name );
- free(ret);
+ if (ret->name != NULL)
+ free(ret->name );
+ free(ret);
}
return(NULL);
}
@@ -949,53 +935,44 @@ error:
*/
int
virFreeNetwork(virConnectPtr conn, virNetworkPtr network) {
- int ret = 0;
+ int refs;
if ((!VIR_IS_CONNECT(conn)) || (!VIR_IS_CONNECTED_NETWORK(network)) ||
- (network->conn != conn) || (conn->hashes_mux == NULL)) {
+ (network->conn != conn)) {
virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(-1);
}
- xmlMutexLock(conn->hashes_mux);
+ pthread_mutex_lock(&conn->lock);
/*
* decrement the count for the network
*/
- network->uses--;
- ret = network->uses;
- if (ret > 0)
- goto done;
+ network->refs--;
+ refs = network->refs;
+ if (refs > 0) {
+ pthread_mutex_unlock(&conn->lock);
+ return (refs);
+ }
/* TODO search by UUID first as they are better differenciators */
-
if (virHashRemoveEntry(conn->networks, network->name, NULL) < 0) {
virHashError(conn, VIR_ERR_INTERNAL_ERROR,
_("network missing from connection hash table"));
- goto done;
+ pthread_mutex_unlock(&conn->lock);
+ return (refs);
}
network->magic = -1;
if (network->name)
free(network->name);
free(network);
- /*
- * decrement the count for the connection
- */
- conn->uses--;
- if (conn->uses > 0)
- goto done;
-
- if (conn->networks != NULL)
- virHashFree(conn->networks, (virHashDeallocator) virNetworkFreeName);
- if (conn->hashes_mux != NULL)
- xmlFreeMutex(conn->hashes_mux);
- free(conn);
+ refs = virUnrefConnect(conn);
+ if (refs > 0)
+ pthread_mutex_unlock(&conn->lock);
return(0);
-
-done:
- xmlMutexUnlock(conn->hashes_mux);
- return(ret);
-}
+}
+
+
/*
* Local variables:
diff -r 3a46c145fac4 src/internal.h
--- a/src/internal.h Thu Jan 03 21:48:19 2008 -0500
+++ b/src/internal.h Fri Jan 04 17:29:33 2008 -0500
@@ -132,8 +132,8 @@ extern "C" {
*/
struct _virConnect {
unsigned int magic; /* specific value to check */
-
- int uses; /* reference count */
+ int flags; /* a set of connection flags */
+ char *name; /* connection URI */
/* The underlying hypervisor driver and network driver. */
virDriverPtr driver;
@@ -151,12 +151,16 @@ struct _virConnect {
virErrorFunc handler; /* associated handlet */
void *userData; /* the user data */
- /* misc */
- xmlMutexPtr hashes_mux;/* a mutex to protect the domain and networks hash tables */
- virHashTablePtr domains;/* hash table for known domains */
- virHashTablePtr networks;/* hash table for known domains */
- int flags; /* a set of connection flags */
- char *name; /* connection URI */
+ /*
+ * The lock mutex must be acquired before accessing/changing
+ * any of members following this point, or changing the ref
+ * count of any virDomain/virNetwork object associated with
+ * this connection
+ */
+ pthread_mutex_t lock;
+ virHashTablePtr domains; /* hash table for known domains */
+ virHashTablePtr networks; /* hash table for known domains */
+ int refs; /* reference count */
};
/**
@@ -166,7 +170,7 @@ struct _virConnect {
*/
struct _virDomain {
unsigned int magic; /* specific value to check */
- int uses; /* reference count */
+ int refs; /* reference count */
virConnectPtr conn; /* pointer back to the connection */
char *name; /* the domain external name */
int id; /* the domain ID */
@@ -180,18 +184,12 @@ struct _virDomain {
*/
struct _virNetwork {
unsigned int magic; /* specific value to check */
- int uses; /* reference count */
+ int refs; /* reference count */
virConnectPtr conn; /* pointer back to the connection */
char *name; /* the network external name */
unsigned char uuid[VIR_UUID_BUFLEN]; /* the network unique identifier */
};
-/*
-* Internal routines
-*/
-char *virDomainGetVM(virDomainPtr domain);
-char *virDomainGetVMInfo(virDomainPtr domain,
- const char *vm, const char *name);
/************************************************************************
* *
@@ -217,18 +215,19 @@ const char *__virErrorMsg(virErrorNumber
* *
************************************************************************/
-virConnectPtr virGetConnect (void);
-int virFreeConnect (virConnectPtr conn);
-virDomainPtr __virGetDomain (virConnectPtr conn,
- const char *name,
- const unsigned char *uuid);
-int virFreeDomain (virConnectPtr conn,
- virDomainPtr domain);
-virNetworkPtr __virGetNetwork (virConnectPtr conn,
- const char *name,
- const unsigned char *uuid);
-int virFreeNetwork (virConnectPtr conn,
- virNetworkPtr domain);
+virConnectPtr virGetConnect(void);
+int virUnrefConnect(virConnectPtr conn);
+int virFreeConnect(virConnectPtr conn);
+virDomainPtr __virGetDomain(virConnectPtr conn,
+ const char *name,
+ const unsigned char *uuid);
+int virFreeDomain(virConnectPtr conn,
+ virDomainPtr domain);
+virNetworkPtr __virGetNetwork(virConnectPtr conn,
+ const char *name,
+ const unsigned char *uuid);
+int virFreeNetwork(virConnectPtr conn,
+ virNetworkPtr domain);
#define virGetDomain(c,n,u) __virGetDomain((c),(n),(u))
#define virGetNetwork(c,n,u) __virGetNetwork((c),(n),(u))
diff -r 3a46c145fac4 src/xs_internal.c
--- a/src/xs_internal.c Thu Jan 03 21:48:19 2008 -0500
+++ b/src/xs_internal.c Fri Jan 04 17:29:33 2008 -0500
@@ -227,7 +227,7 @@ virDomainDoStoreWrite(virDomainPtr domai
*
* Returns the new string or NULL in case of error
*/
-char *
+static char *
virDomainGetVM(virDomainPtr domain)
{
char *vm;
@@ -261,7 +261,7 @@ virDomainGetVM(virDomainPtr domain)
*
* Returns the new string or NULL in case of error
*/
-char *
+static char *
virDomainGetVMInfo(virDomainPtr domain, const char *vm, const char *name)
{
char s[256];
@@ -284,30 +284,6 @@ virDomainGetVMInfo(virDomainPtr domain,
return (ret);
}
-#if 0
-/**
- * virConnectCheckStoreID:
- * @conn: pointer to the hypervisor connection
- * @id: the id number as returned from Xenstore
- *
- * the xenstore sometimes list non-running domains, double check
- * from the hypervisor if we have direct access
- *
- * Returns -1 if the check failed, 0 if successful or not possible to check
- */
-static int
-virConnectCheckStoreID(virConnectPtr conn, int id)
-{
- if (conn->id >= 0) {
- int tmp;
-
- tmp = xenHypervisorCheckID(conn, id);
- if (tmp < 0)
- return (-1);
- }
- return (0);
-}
-#endif
#endif /* ! PROXY */
/************************************************************************
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
4:35 a.m.
New subject: [Libvir] RFC: PATCH 1/5: cleanup connection ref counting
Daniel P. Berrange wrote:
The referencing counting code for Connect/Domain/Network
objects has many repeated codepaths, not all of which are
correct. eg, the virFreeDomain method forgets to release
networks when garbage collecting a virConnectPtr, and the
virFreeNetwork method forgets to release domains.
The reference counting in libvirt is not just ugly when interfacing with
a language with real GC, but also broken at the moment.
The most serious example are the connect/domain/network handles included
in a virterror. These are not reference counted so that the caller
doesn't have to free the virterror or these handles. But on the other
hand it means that the handles have an indeterminate lifetime, so cannot
be used safely.
So I've moved the code for garbage collecting a virConnectPtr
object into a new virUnrefConnect() method which can be called
from virFreeConnect, virFreeDomain and virFreeNetwork.
This probably has negative implications in the language bindings. At
this moment I don't care much because network objects are in practice
used only very rarely by real code. This could change when we have
storage objects which, I guess, will be used frequently like domains. A
bit too early in the morning for me to be thinking about GC and its
interaction with reference counting :-)
The rest of the patch looks good. We should probably integrate this
patch in the code right now since (a) it's isolated, (b) it implements
much-needed cleanups, and (c) it needs serious testing.
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
7:30 a.m.
New subject: [Libvir] RFC: PATCH 1/5: cleanup connection ref counting
On Mon, Jan 14, 2008 at 10:35:10AM +0000, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
>The referencing counting code for Connect/Domain/Network
>objects has many repeated codepaths, not all of which are
>correct. eg, the virFreeDomain method forgets to release
>networks when garbage collecting a virConnectPtr, and the
>virFreeNetwork method forgets to release domains.
The reference counting in libvirt is not just ugly when interfacing with
a language with real GC, but also broken at the moment.
The most serious example are the connect/domain/network handles included
in a virterror. These are not reference counted so that the caller
doesn't have to free the virterror or these handles. But on the other
hand it means that the handles have an indeterminate lifetime, so cannot
be used safely.
Ahhh you noticed that too then :-) That's on my list of things to fix
somehow I've thought of a couple of options:
- Increment ref count when assign a domain/network to a virError
Or
- When ref count of domain/network drops to zero check for any virError
a NULL-ify the domain/network
Or
- Or tell people not to use the domain/network objects in errors and
don't set them at all
With the first option we might consider a special case in virConnectClose
to automatically decrement the ref count held by the error object, otherwie
we'd be in situation where a virConnectPtr would potentially never be
free'd unless user calls virErrorReset which they probably don't.
My preference is the last. IMHO its a flawed idea because its not extensible
to other objects we're adding. eg I've no way to add a Job or StoragePool
or StorageVol object to the virErrorPtr without breaking ABI each time.
IMHO we should just make use of 'str1' and 'str2' and 'int1' to
store the
name, uuid and id of the associated objects, since we have all these spare
generic fields never used ..
>So I've moved the code for garbage collecting a
virConnectPtr
>object into a new virUnrefConnect() method which can be called
>from virFreeConnect, virFreeDomain and virFreeNetwork.
This probably has negative implications in the language bindings. At
this moment I don't care much because network objects are in practice
used only very rarely by real code. This could change when we have
storage objects which, I guess, will be used frequently like domains. A
bit too early in the morning for me to be thinking about GC and its
interaction with reference counting :-)
Actually it shouldn't hurt the language bindings. When I say I moved
the gargage collection code, this is merely a re-factoring of the
internal cleanup code. So instead of having the same broken duplicted
in virFreeConnect, virFreeDomain and virFreeNetwork it is centralized
in virUnrefConnect. The public API is unchanged, just the impl.
Dan
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
7:08 p.m.
New subject: [Libvir] RFC: PATCH 2/5: public & driver API for async jobs
This patch adds the public API for the virJobPtr object, and the
internal driver support code. The new public APIs are:
- int virJobGetInfo(virJobPtr job, virJobInfoPtr info);
This gets elapsed time, estimate completion time, completion
progress percentage, and completion status - success, failed,
cancelled.
- virDomainPtr virJobGetDomain(virJobPtr job);
If the job was a domain creation attempt, this gets the
resulting domain object
- virNetworkPtr virJobGetNetwork(virJobPtr job);
If the job was a network creation attempt, this gets the
resulting network object
- virErrorPtr virJobGetNetwork(virJobPtr job);
If the job failed, this gets the associated error
- int virJobCancel(virJobPtr job);
Request that the job be cancelled. This is not an immediate
operation. It merely sets a flag, which background jobs will
check periodically. So a job may still complete even if it
was cancelled. It should be responded to within a few seconds
though.
- int virJobDestroy(virJobPtr job);
Once a job has finished running (success, failed, or cancelled)
this may be called to release all resources associated with
the job.
As mentioned before, the following variants on existing APis are
added:
- virDomainCreateLinuxJob
- virDomainCreateJob
- virNetworkCreateJob
- virNetworkCreateXMLJob
- virDomainSaveJob
- virDomainRestoreJob
- virDomainDumpCoreJob
All are no-ops by default, unless the underlying driver supports
async jobs.
Finally, the job.c and job.h files provides a set of internal
routines to authors for the purpose of managing virJobPtr objects.
There is a default impl of the virJobDriver APIs which should be
suitable for all existing the remote driver. The async jobs are
basically run in a background pthread.
This psuedo code illustrates how the 'test' driver might use
these to implement an asynchronous variant of the 'save' method.
It basically stores the parameters in a struct, and then calls
the virJobCreate() method.
privdata = malloc(sizeof(*privdata));
if (privdata == NULL)
return (NULL);
memset(privdata, 0, sizeof(*privdata));
privdata->type = TEST_JOB_DOMAIN_SAVE;
privdata->data.save.domidx = domidx;
privdata->data.save.path = strdup(path);
job = virJobCreate(domain->conn,
domain->conn->driver->jobDriver,
testDomainSaveWorker,
NULL,
privdata,
VIR_JOB_BOUNDED);
The actual work is done in the 'testDomainSaveWorker' method
which is passed in to virJobCreate. The job.h file contains
APIs for it to call to update progress information and detect
cancellation:
static void testDomainSaveWorker(virJobPtr job, void *data)
{
testJobPtr privdata = data;
/* A 50 second ETA for saving */
virJobInitialize(job, 50);
do {
... do a unit of work...
... Check for cancellation ...
if (virJobFinishCancel(job))
return;
... update progress, indicating 1 second
of work complete, remove 1 second from
the ETA, and add 2% of completion state...
virJobUpdateRel(job, 1, -1, 2);
} while (...not done...);
... Inform world we're all done ...
virJobFinishSuccess(job);
}
The virJobInitialize, virJobFinishCancel, virJobFinishSuccess
and virJobUpdateRel methods all take care to lock the virJobPtr
object to ensure thread safety.
b/src/job.c | 304 ++++++++++++++++++++++++++++
b/src/job.h | 102 +++++++++
include/libvirt/libvirt.h | 127 ++++++++++-
include/libvirt/libvirt.h.in | 127 ++++++++++-
include/libvirt/virterror.h | 6
src/Makefile.am | 1
src/driver.h | 61 +++++
src/internal.h | 77 ++++++-
src/libvirt.c | 459 ++++++++++++++++++++++++++++++++++++++++++-
src/libvirt_sym.version | 16 +
src/openvz_driver.c | 50 ----
src/qemu_driver.c | 9
src/test.c | 9
src/virterror.c | 104 ++++-----
14 files changed, 1304 insertions(+), 148 deletions(-)
Dan
diff -r 9b65842892de include/libvirt/libvirt.h
--- a/include/libvirt/libvirt.h Fri Jan 04 17:29:33 2008 -0500
+++ b/include/libvirt/libvirt.h Fri Jan 04 18:00:06 2008 -0500
@@ -51,6 +51,21 @@ typedef struct _virDomain virDomain;
* type used to reference a Xen domain in the API.
*/
typedef virDomain *virDomainPtr;
+
+/**
+ * virNetwork:
+ *
+ * a virNetwork is a private structure representing a virtual network.
+ */
+typedef struct _virNetwork virNetwork;
+
+/**
+ * virNetworkPtr:
+ *
+ * a virNetworkPtr is pointer to a virNetwork private structure, this is the
+ * type used to reference a virtual network in the API.
+ */
+typedef virNetwork *virNetworkPtr;
/**
* virDomainState:
@@ -408,6 +423,88 @@ char * virConnectGetCap
unsigned long long virNodeGetFreeMemory (virConnectPtr conn);
+
+
+/*
+ * Asynchronous background jobs
+ */
+
+
+/**
+ * virJob:
+ *
+ * a virJob is a private structure representing a background job.
+ */
+typedef struct _virJob virJob;
+
+/**
+ * virJobPtr:
+ *
+ * a virJobPtr is pointer to a virJob private structure, this is the
+ * type used to reference a background job in the API.
+ */
+typedef virJob *virJobPtr;
+
+/**
+ * virJobType;
+ *
+ * A job may have a finite bounded progress, or may be
+ * unbounded.
+ */
+typedef enum {
+ VIR_JOB_BOUNDED = 0, /* finite, 0-> 100 completion */
+ VIR_JOB_UNBOUNDED = 1, /* unknown completion percent */
+} virJobType;
+
+
+/**
+ * virJobState;
+ *
+ * A job may be in one of several states
+ */
+typedef enum {
+ VIR_JOB_RUNNING = 0, /* Still active */
+ VIR_JOB_COMPLETE = 1, /* Completed successfully */
+ VIR_JOB_FAILED = 2, /* Failed to complete see virJobGetError */
+ VIR_JOB_CANCELLED = 3, /* User requested cancellation */
+} virJobState;
+
+/**
+ * virJobInfoPtr:
+ *
+ * a virJobInfo is a structure filled by virJobGetInfo() and extracting
+ * runtime informations for a given active Job
+ */
+
+typedef struct _virJobInfo virJobInfo;
+
+struct _virJobInfo {
+ int type; /* One of virJobType constants */
+ int state; /* One of virJobState constants */
+ unsigned int runningTime; /* Actual running time in seconds */
+ unsigned int remainingTime;/* Estimated remaining time in seconds */
+ int percentComplete; /* Completion progress 0 -> 100, if VIR_JOB_BOUNDED */
+};
+
+/**
+ * virJobInfoPtr:
+ *
+ * a virJobInfoPtr is a pointer to a virJobInfo structure.
+ */
+
+typedef virJobInfo *virJobInfoPtr;
+
+int virJobGetInfo (virJobPtr job,
+ virJobInfoPtr info);
+
+
+virDomainPtr virJobGetDomain (virJobPtr job);
+virNetworkPtr virJobGetNetwork (virJobPtr job);
+
+int virJobCancel (virJobPtr job);
+int virJobDestroy (virJobPtr job);
+
+
/*
* Gather list of running domains
*/
@@ -432,6 +529,10 @@ virDomainPtr virDomainCreateLinux (virC
virDomainPtr virDomainCreateLinux (virConnectPtr conn,
const char *xmlDesc,
unsigned int flags);
+virJobPtr virDomainCreateLinuxJob (virConnectPtr conn,
+ const char *xmlDesc,
+ unsigned int flags);
+
virDomainPtr virDomainLookupByName (virConnectPtr conn,
const char *name);
virDomainPtr virDomainLookupByID (virConnectPtr conn,
@@ -458,13 +559,20 @@ int virDomainResume (virDomainPtr dom
*/
int virDomainSave (virDomainPtr domain,
const char *to);
+virJobPtr virDomainSaveJob (virDomainPtr domain,
+ const char *to);
int virDomainRestore (virConnectPtr conn,
const char *from);
+virJobPtr virDomainRestoreJob (virConnectPtr conn,
+ const char *from);
/*
* Domain core dump
*/
int virDomainCoreDump (virDomainPtr domain,
+ const char *to,
+ int flags);
+virJobPtr virDomainCoreDumpJob (virDomainPtr domain,
const char *to,
int flags);
@@ -535,6 +643,8 @@ int virConnectListDefinedDomains (virC
char **const names,
int maxnames);
int virDomainCreate (virDomainPtr domain);
+virJobPtr virDomainCreateJob (virDomainPtr domain,
+ unsigned int flags);
int virDomainGetAutostart (virDomainPtr domain,
int *autostart);
@@ -669,20 +779,6 @@ int virNodeGetCellsFreeMemory(virConn
* Virtual Networks API
*/
-/**
- * virNetwork:
- *
- * a virNetwork is a private structure representing a virtual network.
- */
-typedef struct _virNetwork virNetwork;
-
-/**
- * virNetworkPtr:
- *
- * a virNetworkPtr is pointer to a virNetwork private structure, this is the
- * type used to reference a virtual network in the API.
- */
-typedef virNetwork *virNetworkPtr;
/*
* Get connection from network.
@@ -720,6 +816,8 @@ virNetworkPtr virNetworkLookupByUUIDStr
*/
virNetworkPtr virNetworkCreateXML (virConnectPtr conn,
const char *xmlDesc);
+virJobPtr virNetworkCreateXMLJob (virConnectPtr conn,
+ const char *xmlDesc);
/*
* Define inactive persistent network
@@ -736,6 +834,7 @@ int virNetworkUndefine (virNetworkPtr
* Activate persistent network
*/
int virNetworkCreate (virNetworkPtr network);
+virJobPtr virNetworkCreateJob (virNetworkPtr network);
/*
* Network destroy/free
diff -r 9b65842892de include/libvirt/libvirt.h.in
--- a/include/libvirt/libvirt.h.in Fri Jan 04 17:29:33 2008 -0500
+++ b/include/libvirt/libvirt.h.in Fri Jan 04 18:00:06 2008 -0500
@@ -51,6 +51,21 @@ typedef struct _virDomain virDomain;
* type used to reference a Xen domain in the API.
*/
typedef virDomain *virDomainPtr;
+
+/**
+ * virNetwork:
+ *
+ * a virNetwork is a private structure representing a virtual network.
+ */
+typedef struct _virNetwork virNetwork;
+
+/**
+ * virNetworkPtr:
+ *
+ * a virNetworkPtr is pointer to a virNetwork private structure, this is the
+ * type used to reference a virtual network in the API.
+ */
+typedef virNetwork *virNetworkPtr;
/**
* virDomainState:
@@ -408,6 +423,88 @@ char * virConnectGetCap
unsigned long long virNodeGetFreeMemory (virConnectPtr conn);
+
+
+/*
+ * Asynchronous background jobs
+ */
+
+
+/**
+ * virJob:
+ *
+ * a virJob is a private structure representing a background job.
+ */
+typedef struct _virJob virJob;
+
+/**
+ * virJobPtr:
+ *
+ * a virJobPtr is pointer to a virJob private structure, this is the
+ * type used to reference a background job in the API.
+ */
+typedef virJob *virJobPtr;
+
+/**
+ * virJobType;
+ *
+ * A job may have a finite bounded progress, or may be
+ * unbounded.
+ */
+typedef enum {
+ VIR_JOB_BOUNDED = 0, /* finite, 0-> 100 completion */
+ VIR_JOB_UNBOUNDED = 1, /* unknown completion percent */
+} virJobType;
+
+
+/**
+ * virJobState;
+ *
+ * A job may be in one of several states
+ */
+typedef enum {
+ VIR_JOB_RUNNING = 0, /* Still active */
+ VIR_JOB_COMPLETE = 1, /* Completed successfully */
+ VIR_JOB_FAILED = 2, /* Failed to complete see virJobGetError */
+ VIR_JOB_CANCELLED = 3, /* User requested cancellation */
+} virJobState;
+
+/**
+ * virJobInfoPtr:
+ *
+ * a virJobInfo is a structure filled by virJobGetInfo() and extracting
+ * runtime informations for a given active Job
+ */
+
+typedef struct _virJobInfo virJobInfo;
+
+struct _virJobInfo {
+ int type; /* One of virJobType constants */
+ int state; /* One of virJobState constants */
+ unsigned int runningTime; /* Actual running time in seconds */
+ unsigned int remainingTime;/* Estimated remaining time in seconds */
+ int percentComplete; /* Completion progress 0 -> 100, if VIR_JOB_BOUNDED */
+};
+
+/**
+ * virJobInfoPtr:
+ *
+ * a virJobInfoPtr is a pointer to a virJobInfo structure.
+ */
+
+typedef virJobInfo *virJobInfoPtr;
+
+int virJobGetInfo (virJobPtr job,
+ virJobInfoPtr info);
+
+
+virDomainPtr virJobGetDomain (virJobPtr job);
+virNetworkPtr virJobGetNetwork (virJobPtr job);
+
+int virJobCancel (virJobPtr job);
+int virJobDestroy (virJobPtr job);
+
+
/*
* Gather list of running domains
*/
@@ -432,6 +529,10 @@ virDomainPtr virDomainCreateLinux (virC
virDomainPtr virDomainCreateLinux (virConnectPtr conn,
const char *xmlDesc,
unsigned int flags);
+virJobPtr virDomainCreateLinuxJob (virConnectPtr conn,
+ const char *xmlDesc,
+ unsigned int flags);
+
virDomainPtr virDomainLookupByName (virConnectPtr conn,
const char *name);
virDomainPtr virDomainLookupByID (virConnectPtr conn,
@@ -458,13 +559,20 @@ int virDomainResume (virDomainPtr dom
*/
int virDomainSave (virDomainPtr domain,
const char *to);
+virJobPtr virDomainSaveJob (virDomainPtr domain,
+ const char *to);
int virDomainRestore (virConnectPtr conn,
const char *from);
+virJobPtr virDomainRestoreJob (virConnectPtr conn,
+ const char *from);
/*
* Domain core dump
*/
int virDomainCoreDump (virDomainPtr domain,
+ const char *to,
+ int flags);
+virJobPtr virDomainCoreDumpJob (virDomainPtr domain,
const char *to,
int flags);
@@ -535,6 +643,8 @@ int virConnectListDefinedDomains (virC
char **const names,
int maxnames);
int virDomainCreate (virDomainPtr domain);
+virJobPtr virDomainCreateJob (virDomainPtr domain,
+ unsigned int flags);
int virDomainGetAutostart (virDomainPtr domain,
int *autostart);
@@ -669,20 +779,6 @@ int virNodeGetCellsFreeMemory(virConn
* Virtual Networks API
*/
-/**
- * virNetwork:
- *
- * a virNetwork is a private structure representing a virtual network.
- */
-typedef struct _virNetwork virNetwork;
-
-/**
- * virNetworkPtr:
- *
- * a virNetworkPtr is pointer to a virNetwork private structure, this is the
- * type used to reference a virtual network in the API.
- */
-typedef virNetwork *virNetworkPtr;
/*
* Get connection from network.
@@ -720,6 +816,8 @@ virNetworkPtr virNetworkLookupByUUIDStr
*/
virNetworkPtr virNetworkCreateXML (virConnectPtr conn,
const char *xmlDesc);
+virJobPtr virNetworkCreateXMLJob (virConnectPtr conn,
+ const char *xmlDesc);
/*
* Define inactive persistent network
@@ -736,6 +834,7 @@ int virNetworkUndefine (virNetworkPtr
* Activate persistent network
*/
int virNetworkCreate (virNetworkPtr network);
+virJobPtr virNetworkCreateJob (virNetworkPtr network);
/*
* Network destroy/free
diff -r 9b65842892de include/libvirt/virterror.h
--- a/include/libvirt/virterror.h Fri Jan 04 17:29:33 2008 -0500
+++ b/include/libvirt/virterror.h Fri Jan 04 18:00:06 2008 -0500
@@ -165,6 +165,12 @@ void virConnSetErrorFunc (virConnectPt
virErrorFunc handler);
int virConnCopyLastError (virConnectPtr conn,
virErrorPtr to);
+
+virErrorPtr virJobGetError (virJobPtr job);
+int virJobCopyLastError (virJobPtr job,
+ virErrorPtr to);
+
+
#ifdef __cplusplus
}
#endif
diff -r 9b65842892de src/Makefile.am
--- a/src/Makefile.am Fri Jan 04 17:29:33 2008 -0500
+++ b/src/Makefile.am Fri Jan 04 18:00:06 2008 -0500
@@ -32,6 +32,7 @@ CLIENT_SOURCES = \
libvirt.c internal.h \
gnutls_1_0_compat.h \
hash.c hash.h \
+ job.c job.h \
test.c test.h \
buf.c buf.h \
xml.c xml.h \
diff -r 9b65842892de src/driver.h
--- a/src/driver.h Fri Jan 04 17:29:33 2008 -0500
+++ b/src/driver.h Fri Jan 04 18:00:06 2008 -0500
@@ -95,6 +95,23 @@ typedef int
virNodeInfoPtr info);
typedef char *
(*virDrvGetCapabilities) (virConnectPtr conn);
+
+typedef int
+ (*virDrvJobGetInfo) (virJobPtr job,
+ virJobInfoPtr info);
+typedef virDomainPtr
+ (*virDrvJobGetDomain) (virJobPtr job);
+typedef virNetworkPtr
+ (*virDrvJobGetNetwork) (virJobPtr job);
+typedef virErrorPtr
+ (*virDrvJobGetError) (virJobPtr job);
+
+typedef int
+ (*virDrvJobCancel) (virJobPtr job);
+typedef int
+ (*virDrvJobDestroy) (virJobPtr job);
+
+
typedef int
(*virDrvListDomains) (virConnectPtr conn,
int *ids,
@@ -105,6 +122,10 @@ typedef virDomainPtr
(*virDrvDomainCreateLinux) (virConnectPtr conn,
const char *xmlDesc,
unsigned int flags);
+typedef virJobPtr
+ (*virDrvDomainCreateLinuxJob) (virConnectPtr conn,
+ const char *xmlDesc,
+ unsigned int flags);
typedef virDomainPtr
(*virDrvDomainLookupByID) (virConnectPtr conn,
int id);
@@ -141,13 +162,23 @@ typedef int
typedef int
(*virDrvDomainSave) (virDomainPtr domain,
const char *to);
+typedef virJobPtr
+ (*virDrvDomainSaveJob) (virDomainPtr domain,
+ const char *to);
typedef int
(*virDrvDomainRestore) (virConnectPtr conn,
+ const char *from);
+typedef virJobPtr
+ (*virDrvDomainRestoreJob) (virConnectPtr conn,
const char *from);
typedef int
(*virDrvDomainCoreDump) (virDomainPtr domain,
const char *to,
int flags);
+typedef virJobPtr
+ (*virDrvDomainCoreDumpJob) (virDomainPtr domain,
+ const char *to,
+ int flags);
typedef char *
(*virDrvDomainDumpXML) (virDomainPtr dom,
int flags);
@@ -159,6 +190,9 @@ typedef int
(*virDrvNumOfDefinedDomains) (virConnectPtr conn);
typedef int
(*virDrvDomainCreate) (virDomainPtr dom);
+typedef virJobPtr
+ (*virDrvDomainCreateJob) (virDomainPtr dom,
+ unsigned int flags);
typedef virDomainPtr
(*virDrvDomainDefineXML) (virConnectPtr conn,
const char *xml);
@@ -264,6 +298,19 @@ typedef unsigned long long
(*virDrvNodeGetFreeMemory)
(virConnectPtr conn);
+
+typedef struct _virJobDriver virJobDriver;
+typedef virJobDriver *virJobDriverPtr;
+
+struct _virJobDriver {
+ virDrvJobGetInfo getInfo;
+ virDrvJobGetDomain getDomain;
+ virDrvJobGetNetwork getNetwork;
+ virDrvJobGetError getError;
+ virDrvJobCancel cancel;
+ virDrvJobDestroy destroy;
+};
+
/**
* _virDriver:
*
@@ -280,6 +327,7 @@ struct _virDriver {
int no; /* the number virDrvNo */
const char * name; /* the name of the driver */
unsigned long ver; /* the version of the backend */
+ virJobDriverPtr jobDriver;
virDrvOpen open;
virDrvClose close;
virDrvSupportsFeature supports_feature;
@@ -293,6 +341,7 @@ struct _virDriver {
virDrvListDomains listDomains;
virDrvNumOfDomains numOfDomains;
virDrvDomainCreateLinux domainCreateLinux;
+ virDrvDomainCreateLinuxJob domainCreateLinuxJob;
virDrvDomainLookupByID domainLookupByID;
virDrvDomainLookupByUUID domainLookupByUUID;
virDrvDomainLookupByName domainLookupByName;
@@ -307,8 +356,11 @@ struct _virDriver {
virDrvDomainSetMemory domainSetMemory;
virDrvDomainGetInfo domainGetInfo;
virDrvDomainSave domainSave;
+ virDrvDomainSaveJob domainSaveJob;
virDrvDomainRestore domainRestore;
+ virDrvDomainRestoreJob domainRestoreJob;
virDrvDomainCoreDump domainCoreDump;
+ virDrvDomainCoreDumpJob domainCoreDumpJob;
virDrvDomainSetVcpus domainSetVcpus;
virDrvDomainPinVcpu domainPinVcpu;
virDrvDomainGetVcpus domainGetVcpus;
@@ -317,6 +369,7 @@ struct _virDriver {
virDrvListDefinedDomains listDefinedDomains;
virDrvNumOfDefinedDomains numOfDefinedDomains;
virDrvDomainCreate domainCreate;
+ virDrvDomainCreateJob domainCreateJob;
virDrvDomainDefineXML domainDefineXML;
virDrvDomainUndefine domainUndefine;
virDrvDomainAttachDevice domainAttachDevice;
@@ -356,6 +409,9 @@ typedef virNetworkPtr
typedef virNetworkPtr
(*virDrvNetworkCreateXML) (virConnectPtr conn,
const char *xmlDesc);
+typedef virJobPtr
+ (*virDrvNetworkCreateXMLJob) (virConnectPtr conn,
+ const char *xmlDesc);
typedef virNetworkPtr
(*virDrvNetworkDefineXML) (virConnectPtr conn,
const char *xml);
@@ -363,6 +419,8 @@ typedef int
(*virDrvNetworkUndefine) (virNetworkPtr network);
typedef int
(*virDrvNetworkCreate) (virNetworkPtr network);
+typedef virJobPtr
+ (*virDrvNetworkCreateJob) (virNetworkPtr network);
typedef int
(*virDrvNetworkDestroy) (virNetworkPtr network);
typedef char *
@@ -393,6 +451,7 @@ typedef virNetworkDriver *virNetworkDriv
*/
struct _virNetworkDriver {
const char * name; /* the name of the driver */
+ virJobDriverPtr jobDriver;
virDrvOpen open;
virDrvClose close;
virDrvNumOfNetworks numOfNetworks;
@@ -402,9 +461,11 @@ struct _virNetworkDriver {
virDrvNetworkLookupByUUID networkLookupByUUID;
virDrvNetworkLookupByName networkLookupByName;
virDrvNetworkCreateXML networkCreateXML;
+ virDrvNetworkCreateXMLJob networkCreateXMLJob;
virDrvNetworkDefineXML networkDefineXML;
virDrvNetworkUndefine networkUndefine;
virDrvNetworkCreate networkCreate;
+ virDrvNetworkCreateJob networkCreateJob;
virDrvNetworkDestroy networkDestroy;
virDrvNetworkDumpXML networkDumpXML;
virDrvNetworkGetBridgeName networkGetBridgeName;
diff -r 9b65842892de src/internal.h
--- a/src/internal.h Fri Jan 04 17:29:33 2008 -0500
+++ b/src/internal.h Fri Jan 04 18:00:06 2008 -0500
@@ -100,6 +100,16 @@ extern "C" {
/**
+ * VIR_JOB_MAGIC:
+ *
+ * magic value used to protect the API when pointers to job structures
+ * are passed down by the users.
+ */
+#define VIR_JOB_MAGIC 0x99DEAD11
+#define VIR_IS_JOB(obj) ((obj) && (obj)->magic==VIR_JOB_MAGIC)
+#define VIR_IS_CONNECTED_JOB(obj) (VIR_IS_JOB(obj) &&
VIR_IS_CONNECT((obj)->conn))
+
+/**
* VIR_DOMAIN_MAGIC:
*
* magic value used to protect the API when pointers to domain structures
@@ -160,6 +170,7 @@ struct _virConnect {
pthread_mutex_t lock;
virHashTablePtr domains; /* hash table for known domains */
virHashTablePtr networks; /* hash table for known domains */
+ virJobPtr jobs; /* list of active background jobs */
int refs; /* reference count */
};
@@ -196,17 +207,28 @@ struct _virNetwork {
* API for error handling *
* *
************************************************************************/
+void __virSetError(virErrorPtr err,
+ virConnectPtr conn,
+ virDomainPtr dom,
+ virNetworkPtr net,
+ int domain,
+ int code,
+ virErrorLevel level,
+ const char *str1,
+ const char *str2,
+ const char *str3,
+ int int1, int int2, const char *message);
void __virRaiseError(virConnectPtr conn,
- virDomainPtr dom,
- virNetworkPtr net,
- int domain,
- int code,
- virErrorLevel level,
- const char *str1,
- const char *str2,
- const char *str3,
- int int1, int int2, const char *msg, ...)
- ATTRIBUTE_FORMAT(printf, 12, 13);
+ virDomainPtr dom,
+ virNetworkPtr net,
+ int domain,
+ int code,
+ virErrorLevel level,
+ const char *str1,
+ const char *str2,
+ const char *str3,
+ int int1, int int2, const char *msg, ...)
+ ATTRIBUTE_FORMAT(printf, 12, 13);
const char *__virErrorMsg(virErrorNumber error, const char *info);
/************************************************************************
@@ -290,6 +312,41 @@ xstrtol_ui(char const *s, char **end_ptr
return 0;
}
+/*
+ * Macro used to format the message as a string in __virRaiseError
+ * and borrowed from libxml2.
+ */
+#define VIR_GET_VAR_STR(msg, str) { \
+ int size = 150, prev_size = -1; \
+ int chars; \
+ char *larger; \
+ va_list ap; \
+ \
+ str = (char *) malloc(size); \
+ if (str != NULL) { \
+ while (1) { \
+ va_start(ap, msg); \
+ chars = vsnprintf(str, size, msg, ap); \
+ va_end(ap); \
+ if ((chars > -1) && (chars < size)) { \
+ if (prev_size == chars) { \
+ break; \
+ } else { \
+ prev_size = chars; \
+ } \
+ } \
+ if (chars > -1) \
+ size += chars + 1; \
+ else \
+ size += 100; \
+ if ((larger = (char *) realloc(str, size)) == NULL) { \
+ break; \
+ } \
+ str = larger; \
+ } \
+ } \
+ }
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
diff -r 9b65842892de src/job.c
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/job.c Fri Jan 04 18:00:06 2008 -0500
@@ -0,0 +1,304 @@
+/*
+ * job.h: asynchronous background jobs
+ *
+ * Copyright (C) 2008 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Daniel Berrange <berrange(a)redhat.com>
+ */
+
+#include "job.h"
+
+#include <stdlib.h>
+#include <string.h>
+#include <pthread.h>
+
+
+/*
+ * Threading rules
+ * 1. Thou shalt lock the virConnectPtr before virJobPtr
+ * 2. Thou shalt not delete the virJobPtr while the thread is running
+ *
+ */
+
+static void *virJobStartFunc(void *data)
+{
+ virJobPtr job = data;
+
+ (*job->worker)(job, job->funcData);
+
+ return NULL;
+}
+
+virJobPtr virJobCreate(virConnectPtr conn,
+ virJobDriverPtr driver,
+ virJobWorkerFunc worker,
+ virJobCleanupFunc cleanup,
+ void *funcData,
+ int type)
+{
+ virJobPtr job = malloc(sizeof(*job));
+ if (!job) {
+ if (cleanup)
+ (*cleanup)(funcData);
+ return NULL;
+ }
+ memset(job, 0, sizeof(*job));
+
+ pthread_mutex_lock(&conn->lock);
+
+ pthread_mutex_init(&job->lock, NULL);
+ job->magic = VIR_JOB_MAGIC;
+ job->conn = conn;
+ job->driver = driver;
+ job->info.type = type;
+ job->worker = worker;
+ job->cleanup = cleanup;
+ job->funcData = funcData;
+
+ job->next = conn->jobs;
+ conn->jobs = job;
+ conn->refs++;
+
+ pthread_mutex_lock(&job->lock);
+ job->info.state = VIR_JOB_RUNNING;
+ if (pthread_create(&job->thread, NULL,
+ virJobStartFunc, job) != 0) {
+ job->info.state = VIR_JOB_FAILED;
+ pthread_mutex_unlock(&job->lock);
+ pthread_mutex_unlock(&conn->lock);
+ virJobFree(conn, job);
+ return NULL;
+ }
+
+ pthread_mutex_unlock(&job->lock);
+ pthread_mutex_unlock(&conn->lock);
+
+ return job;
+}
+
+
+int virJobFree(virConnectPtr conn,
+ virJobPtr job) {
+ virJobPtr tmp, prev = NULL;
+
+ pthread_mutex_lock(&conn->lock);
+ pthread_mutex_lock(&job->lock);
+ if (job->info.state == VIR_JOB_RUNNING) {
+ pthread_mutex_unlock(&job->lock);
+ pthread_mutex_unlock(&conn->lock);
+ return -1;
+ }
+ pthread_mutex_unlock(&job->lock);
+
+ pthread_join(job->thread, NULL);
+
+ tmp = conn->jobs;
+ while (tmp) {
+ if (tmp == job) {
+ if (prev)
+ prev->next = job->next;
+ else
+ conn->jobs = job->next;
+ break;
+ }
+ prev = tmp;
+ tmp = tmp->next;
+ }
+ if (job->cleanup)
+ (*job->cleanup)(job->funcData);
+ if (virUnrefConnect(conn) > 0)
+ pthread_mutex_unlock(&conn->lock);
+ free(job);
+ return 0;
+}
+
+
+void virJobInitialize(virJobPtr job,
+ int remaining) {
+ pthread_mutex_lock(&job->conn->lock);
+ pthread_mutex_lock(&job->lock);
+ pthread_mutex_unlock(&job->conn->lock);
+
+ job->info.remainingTime = remaining;
+
+ pthread_mutex_unlock(&job->lock);
+}
+
+void virJobUpdateAbs(virJobPtr job,
+ int running,
+ int remaining,
+ int complete) {
+ pthread_mutex_lock(&job->conn->lock);
+ pthread_mutex_lock(&job->lock);
+ pthread_mutex_unlock(&job->conn->lock);
+
+ job->info.remainingTime = remaining;
+ job->info.runningTime = running;
+ job->info.percentComplete = complete;
+
+ pthread_mutex_unlock(&job->lock);
+}
+
+void virJobUpdateRel(virJobPtr job,
+ int running,
+ int remaining,
+ int complete) {
+ pthread_mutex_lock(&job->conn->lock);
+ pthread_mutex_lock(&job->lock);
+ pthread_mutex_unlock(&job->conn->lock);
+
+ job->info.remainingTime += remaining;
+ job->info.runningTime += running;
+ job->info.percentComplete += complete;
+
+ pthread_mutex_unlock(&job->lock);
+}
+
+void virJobFinishSuccess(virJobPtr job) {
+ pthread_mutex_lock(&job->conn->lock);
+ pthread_mutex_lock(&job->lock);
+ pthread_mutex_unlock(&job->conn->lock);
+
+ job->info.remainingTime = 0;
+ job->info.state = VIR_JOB_COMPLETE;
+
+ pthread_mutex_unlock(&job->lock);
+}
+
+int virJobFinishCancel(virJobPtr job) {
+ int ret = 0;
+
+ pthread_mutex_lock(&job->conn->lock);
+ pthread_mutex_lock(&job->lock);
+ pthread_mutex_unlock(&job->conn->lock);
+
+ if (job->cancelled) {
+ ret = 1;
+ job->info.remainingTime = 0;
+ job->info.state = VIR_JOB_CANCELLED;
+ }
+
+ pthread_mutex_unlock(&job->lock);
+
+ return ret;
+}
+
+void virJobFinishFailed(virJobPtr job,
+ int domain, int code, int level,
+ const char *str1, const char *str2, const char *str3,
+ int int1, int int2,
+ char *msg, ...) {
+ char *str;
+ pthread_mutex_lock(&job->conn->lock);
+ pthread_mutex_lock(&job->lock);
+ pthread_mutex_unlock(&job->conn->lock);
+
+ job->info.remainingTime = 0;
+ job->info.state = VIR_JOB_FAILED;
+
+ if (msg == NULL) {
+ str = strdup(_("No error message provided"));
+ } else {
+ VIR_GET_VAR_STR(msg, str);
+ }
+
+ __virSetError(&job->err, job->conn, NULL, NULL,
+ domain, code, level,
+ str1, str2, str3, int1, int2,
+ str);
+
+ free(str);
+
+ pthread_mutex_unlock(&job->lock);
+}
+
+static int virJobDefaultGetInfo(virJobPtr job,
+ virJobInfoPtr info) {
+ pthread_mutex_lock(&job->conn->lock);
+ pthread_mutex_lock(&job->lock);
+ pthread_mutex_unlock(&job->conn->lock);
+
+ memcpy(info, &job->info, sizeof(*info));
+
+ pthread_mutex_unlock(&job->lock);
+
+ return 0;
+}
+
+static virErrorPtr virJobDefaultGetError(virJobPtr job) {
+ virErrorPtr err;
+
+ pthread_mutex_lock(&job->conn->lock);
+ pthread_mutex_lock(&job->lock);
+ pthread_mutex_unlock(&job->conn->lock);
+
+ if (job->info.state == VIR_JOB_RUNNING ||
+ job->err.code == VIR_ERR_OK) {
+ pthread_mutex_unlock(&job->lock);
+ return NULL;
+ }
+
+ err = &job->err;
+ pthread_mutex_unlock(&job->lock);
+
+ return err;
+}
+
+
+static int virJobDefaultCancel(virJobPtr job) {
+ pthread_mutex_lock(&job->conn->lock);
+ pthread_mutex_lock(&job->lock);
+ pthread_mutex_unlock(&job->conn->lock);
+
+ if (job->info.state != VIR_JOB_RUNNING) {
+ pthread_mutex_unlock(&job->lock);
+ return -1;
+ }
+
+ job->cancelled = 1;
+ pthread_mutex_unlock(&job->lock);
+ return 0;
+}
+
+static int virJobDefaultDestroy(virJobPtr job) {
+ return virJobFree(job->conn, job);
+}
+
+
+virJobDriver defaultJobDriver = {
+ virJobDefaultGetInfo,
+ NULL,
+ NULL,
+ virJobDefaultGetError,
+ virJobDefaultCancel,
+ virJobDefaultDestroy,
+};
+
+
+/*
+ * vim: set tabstop=4:
+ * vim: set shiftwidth=4:
+ * vim: set expandtab:
+ */
+/*
+ * Local variables:
+ * indent-tabs-mode: nil
+ * c-indent-level: 4
+ * c-basic-offset: 4
+ * tab-width: 4
+ * End:
+ */
diff -r 9b65842892de src/job.h
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/job.h Fri Jan 04 18:00:06 2008 -0500
@@ -0,0 +1,102 @@
+/*
+ * job.h: asynchronous background jobs
+ *
+ * Copyright (C) 2008 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Daniel Berrange <berrange(a)redhat.com>
+ */
+
+#include "config.h"
+
+#include "internal.h"
+#include "driver.h"
+
+extern virJobDriver defaultJobDriver;
+
+typedef void (*virJobWorkerFunc)(virJobPtr job, void *data);
+typedef void (*virJobCleanupFunc)(void *data);
+
+struct _virJob {
+ int magic;
+ virJobDriverPtr driver;
+ virConnectPtr conn;
+
+ virJobInfo info;
+ int cancelled;
+ virError err;
+
+ pthread_mutex_t lock;
+ pthread_t thread;
+ virJobWorkerFunc worker;
+ virJobCleanupFunc cleanup;
+ void *funcData;
+
+ virJobPtr next;
+};
+
+
+virJobPtr virJobCreate(virConnectPtr conn,
+ virJobDriverPtr driver,
+ virJobWorkerFunc worker,
+ virJobCleanupFunc cleanup,
+ void *funcData,
+ int type);
+
+int virJobFree(virConnectPtr conn,
+ virJobPtr job);
+
+
+
+/* Following methods can be called from the worker
+ thread to update the job status. They automatically
+ lock the job */
+
+void virJobInitialize(virJobPtr job,
+ int remaining);
+
+void virJobUpdateAbs(virJobPtr job,
+ int running,
+ int remaining,
+ int complete);
+
+void virJobUpdateRel(virJobPtr job,
+ int running,
+ int remaining,
+ int complete);
+
+void virJobFinishSuccess(virJobPtr job);
+int virJobFinishCancel(virJobPtr job);
+void virJobFinishFailed(virJobPtr job,
+ int domain, int code, int level,
+ const char *str1, const char *str2, const char *str3,
+ int int1, int int2,
+ char *msg, ...)
+ ATTRIBUTE_FORMAT(printf, 10, 11);
+
+/*
+ * vim: set tabstop=4:
+ * vim: set shiftwidth=4:
+ * vim: set expandtab:
+ */
+/*
+ * Local variables:
+ * indent-tabs-mode: nil
+ * c-indent-level: 4
+ * c-basic-offset: 4
+ * tab-width: 4
+ * End:
+ */
diff -r 9b65842892de src/libvirt.c
--- a/src/libvirt.c Fri Jan 04 17:29:33 2008 -0500
+++ b/src/libvirt.c Fri Jan 04 18:00:06 2008 -0500
@@ -26,6 +26,7 @@
#include "internal.h"
#include "driver.h"
+#include "job.h"
#include "uuid.h"
#include "test.h"
@@ -865,6 +866,141 @@ virConnectGetMaxVcpus(virConnectPtr conn
return -1;
}
+
+int
+virJobGetInfo(virJobPtr job,
+ virJobInfoPtr info)
+{
+ DEBUG("job=%p, info=%p", job, info);
+
+ if (!VIR_IS_JOB(job)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return (-1);
+ }
+
+ if (info == NULL) {
+ virLibConnError(job->conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ return (-1);
+ }
+
+ if (job->driver->getInfo)
+ return job->driver->getInfo (job, info);
+
+ virLibConnError (job->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return -1;
+}
+
+
+virDomainPtr
+virJobGetDomain(virJobPtr job)
+{
+ DEBUG("job=%p", job);
+
+ if (!VIR_IS_JOB(job)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return (NULL);
+ }
+
+ if (job->driver->getDomain)
+ return job->driver->getDomain (job);
+
+ virLibConnError (job->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return NULL;
+}
+
+virNetworkPtr
+virJobGetNetwork(virJobPtr job)
+{
+ DEBUG("job=%p", job);
+
+ if (!VIR_IS_JOB(job)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return (NULL);
+ }
+
+ if (job->driver->getNetwork)
+ return job->driver->getNetwork (job);
+
+ virLibConnError (job->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return NULL;
+}
+
+virErrorPtr
+virJobGetError(virJobPtr job)
+{
+ DEBUG("job=%p", job);
+
+ if (!VIR_IS_JOB(job)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return (NULL);
+ }
+
+ if (job->driver->getError)
+ return job->driver->getError (job);
+
+ virLibConnError (job->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return NULL;
+}
+
+int
+virJobCopyLastError (virJobPtr job,
+ virErrorPtr to)
+{
+ virErrorPtr err;
+ DEBUG("job=%p", job);
+
+ if (!VIR_IS_JOB(job)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return (-1);
+ }
+ if (to == NULL) {
+ virLibConnError(job->conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ return (-1);
+ }
+
+ err = virJobGetError(job);
+ if (err == NULL)
+ return (0);
+ if (err->code == VIR_ERR_OK)
+ return (0);
+ memcpy(to, err, sizeof(virError));
+ return (err->code);
+}
+
+int
+virJobCancel(virJobPtr job)
+{
+ DEBUG("job=%p", job);
+
+ if (!VIR_IS_JOB(job)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return (-1);
+ }
+
+ if (job->driver->cancel)
+ return job->driver->cancel (job);
+
+ virLibConnError (job->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return -1;
+}
+
+int
+virJobDestroy(virJobPtr job)
+{
+ DEBUG("job=%p", job);
+
+ if (!VIR_IS_JOB(job)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return (-1);
+ }
+
+ if (job->driver->destroy)
+ return job->driver->destroy (job);
+
+ virLibConnError (job->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return -1;
+}
+
/**
* virConnectListDomains:
* @conn: pointer to the hypervisor connection
@@ -981,6 +1117,45 @@ virDomainCreateLinux(virConnectPtr conn,
if (conn->driver->domainCreateLinux)
return conn->driver->domainCreateLinux (conn, xmlDesc, flags);
+
+ virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return NULL;
+}
+
+/**
+ * virDomainCreateLinuxJob:
+ * @conn: pointer to the hypervisor connection
+ * @xmlDesc: an XML description of the domain
+ * @flags: an optional set of virDomainFlags
+ *
+ * Launch a new Linux guest domain, based on an XML description similar
+ * to the one returned by virDomainGetXMLDesc()
+ * This function may requires priviledged access to the hypervisor.
+ *
+ * Returns a new job object for monitoring progress of the creation
+ * attempt or NULL in case of failure
+ */
+virJobPtr
+virDomainCreateLinuxJob(virConnectPtr conn, const char *xmlDesc,
+ unsigned int flags)
+{
+ DEBUG("conn=%p, xmlDesc=%s, flags=%d", conn, xmlDesc, flags);
+
+ if (!VIR_IS_CONNECT(conn)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return (NULL);
+ }
+ if (xmlDesc == NULL) {
+ virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ return (NULL);
+ }
+ if (conn->flags & VIR_CONNECT_RO) {
+ virLibConnError(conn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (NULL);
+ }
+
+ if (conn->driver->domainCreateLinuxJob)
+ return conn->driver->domainCreateLinuxJob (conn, xmlDesc, flags);
virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
return NULL;
@@ -1323,6 +1498,66 @@ virDomainSave(virDomainPtr domain, const
}
/**
+ * virDomainSaveJob:
+ * @domain: a domain object
+ * @to: path for the output file
+ *
+ * This method will suspend a domain and save its memory contents to
+ * a file on disk. After the call, if successful, the domain is not
+ * listed as running anymore (this may be a problem).
+ * Use virDomainRestore() to restore a domain after saving.
+ *
+ * Returns a new job for monitoring save progress, and NULL in case of failure.
+ */
+virJobPtr
+virDomainSaveJob(virDomainPtr domain, const char *to)
+{
+ char filepath[4096];
+ virConnectPtr conn;
+ DEBUG("domain=%p, to=%s", domain, to);
+
+ if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
+ virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+ return (NULL);
+ }
+ if (domain->conn->flags & VIR_CONNECT_RO) {
+ virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (NULL);
+ }
+ conn = domain->conn;
+ if (to == NULL) {
+ virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ return (NULL);
+ }
+
+ /*
+ * We must absolutize the file path as the save is done out of process
+ * TODO: check for URI when libxml2 is linked in.
+ */
+ if (to[0] != '/') {
+ unsigned int len, t;
+
+ t = strlen(to);
+ if (getcwd(filepath, sizeof(filepath) - (t + 3)) == NULL)
+ return (NULL);
+ len = strlen(filepath);
+ /* that should be covered by getcwd() semantic, but be 100% sure */
+ if (len > sizeof(filepath) - (t + 3))
+ return (NULL);
+ filepath[len] = '/';
+ strcpy(&filepath[len + 1], to);
+ to = &filepath[0];
+
+ }
+
+ if (conn->driver->domainSaveJob)
+ return conn->driver->domainSaveJob (domain, to);
+
+ virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return (NULL);
+}
+
+/**
* virDomainRestore:
* @conn: pointer to the hypervisor connection
* @from: path to the
@@ -1377,6 +1612,60 @@ virDomainRestore(virConnectPtr conn, con
}
/**
+ * virDomainRestoreJob:
+ * @conn: pointer to the hypervisor connection
+ * @from: path to the
+ *
+ * This method will restore a domain saved to disk by virDomainSave().
+ *
+ * Returns a new job for monitoring restore progress and NULL in case of failure.
+ */
+virJobPtr
+virDomainRestoreJob(virConnectPtr conn, const char *from)
+{
+ char filepath[4096];
+ DEBUG("conn=%p, from=%s", conn, from);
+
+ if (!VIR_IS_CONNECT(conn)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return (NULL);
+ }
+ if (conn->flags & VIR_CONNECT_RO) {
+ virLibConnError(conn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (NULL);
+ }
+ if (from == NULL) {
+ virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ return (NULL);
+ }
+
+ /*
+ * We must absolutize the file path as the restore is done out of process
+ * TODO: check for URI when libxml2 is linked in.
+ */
+ if (from[0] != '/') {
+ unsigned int len, t;
+
+ t = strlen(from);
+ if (getcwd(filepath, sizeof(filepath) - (t + 3)) == NULL)
+ return (NULL);
+ len = strlen(filepath);
+ /* that should be covered by getcwd() semantic, but be 100% sure */
+ if (len > sizeof(filepath) - (t + 3))
+ return (NULL);
+ filepath[len] = '/';
+ strcpy(&filepath[len + 1], from);
+ from = &filepath[0];
+ }
+
+ if (conn->driver->domainRestoreJob)
+ return conn->driver->domainRestoreJob (conn, from);
+
+ virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return (NULL);
+}
+
+/**
* virDomainCoreDump:
* @domain: a domain object
* @to: path for the core file
@@ -1434,6 +1723,66 @@ virDomainCoreDump(virDomainPtr domain, c
virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
+}
+
+/**
+ * virDomainCoreDumpJob:
+ * @domain: a domain object
+ * @to: path for the core file
+ * @flags: extra flags, currently unused
+ *
+ * This method will dump the core of a domain on a given file for analysis.
+ * Note that for remote Xen Daemon the file path will be interpreted in
+ * the remote host.
+ *
+ * Returns a new job for monitoring dump progress and NULL in case of failure.
+ */
+virJobPtr
+virDomainCoreDumpJob(virDomainPtr domain, const char *to, int flags)
+{
+ char filepath[4096];
+ virConnectPtr conn;
+ DEBUG("domain=%p, to=%s, flags=%d", domain, to, flags);
+
+ if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
+ virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+ return (NULL);
+ }
+ if (domain->conn->flags & VIR_CONNECT_RO) {
+ virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (NULL);
+ }
+ conn = domain->conn;
+ if (to == NULL) {
+ virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ return (NULL);
+ }
+
+ /*
+ * We must absolutize the file path as the save is done out of process
+ * TODO: check for URI when libxml2 is linked in.
+ */
+ if (to[0] != '/') {
+ unsigned int len, t;
+
+ t = strlen(to);
+ if (getcwd(filepath, sizeof(filepath) - (t + 3)) == NULL)
+ return (NULL);
+ len = strlen(filepath);
+ /* that should be covered by getcwd() semantic, but be 100% sure */
+ if (len > sizeof(filepath) - (t + 3))
+ return (NULL);
+ filepath[len] = '/';
+ strcpy(&filepath[len + 1], to);
+ to = &filepath[0];
+
+ }
+
+ if (conn->driver->domainCoreDumpJob)
+ return conn->driver->domainCoreDumpJob (domain, to, flags);
+
+ virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return (NULL);
}
/**
@@ -2477,10 +2826,6 @@ virDomainCreate(virDomainPtr domain) {
virConnectPtr conn;
DEBUG("domain=%p", domain);
- if (domain == NULL) {
- TODO
- return (-1);
- }
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
return (-1);
@@ -2495,7 +2840,40 @@ virDomainCreate(virDomainPtr domain) {
return conn->driver->domainCreate (domain);
virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
- return -1;
+ return (-1);
+}
+
+/**
+ * virDomainCreateJob:
+ * @domain: pointer to a defined domain
+ *
+ * launch a defined domain. If the call succeed the domain moves from the
+ * defined to the running domains pools.
+ *
+ * Returns a new job object for monitoring progress of
+ * creation attempt, NULL in case of error
+ */
+virJobPtr
+virDomainCreateJob(virDomainPtr domain,
+ unsigned int flags) {
+ virConnectPtr conn;
+ DEBUG("domain=%p", domain);
+
+ if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
+ virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+ return (NULL);
+ }
+ conn = domain->conn;
+ if (conn->flags & VIR_CONNECT_RO) {
+ virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (NULL);
+ }
+
+ if (conn->driver->domainCreateJob)
+ return conn->driver->domainCreateJob (domain, flags);
+
+ virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return (NULL);
}
/**
@@ -3147,6 +3525,41 @@ virNetworkCreateXML(virConnectPtr conn,
}
/**
+ * virNetworkCreateXMLJob:
+ * @conn: pointer to the hypervisor connection
+ * @xmlDesc: an XML description of the network
+ *
+ * Create and start a new virtual network, based on an XML description
+ * similar to the one returned by virNetworkGetXMLDesc()
+ *
+ * Returns a new job object or NULL in case of failure
+ */
+virJobPtr
+virNetworkCreateXMLJob(virConnectPtr conn, const char *xmlDesc)
+{
+ DEBUG("conn=%p, xmlDesc=%s", conn, xmlDesc);
+
+ if (!VIR_IS_CONNECT(conn)) {
+ virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+ return (NULL);
+ }
+ if (xmlDesc == NULL) {
+ virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ return (NULL);
+ }
+ if (conn->flags & VIR_CONNECT_RO) {
+ virLibConnError(conn, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (NULL);
+ }
+
+ if (conn->networkDriver &&
conn->networkDriver->networkCreateXMLJob)
+ return conn->networkDriver->networkCreateXMLJob (conn, xmlDesc);
+
+ virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return NULL;
+}
+
+/**
* virNetworkDefineXML:
* @conn: pointer to the hypervisor connection
* @xml: the XML description for the network, preferably in UTF-8
@@ -3225,10 +3638,6 @@ virNetworkCreate(virNetworkPtr network)
virConnectPtr conn;
DEBUG("network=%p", network);
- if (network == NULL) {
- TODO
- return (-1);
- }
if (!VIR_IS_CONNECTED_NETWORK(network)) {
virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
return (-1);
@@ -3244,6 +3653,38 @@ virNetworkCreate(virNetworkPtr network)
virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
+}
+
+/**
+ * virNetworkCreateJob:
+ * @network: pointer to a defined network
+ *
+ * Create and start a defined network. If the call succeed the network
+ * moves from the defined to the running networks pools.
+ *
+ * Returns a job object for monitoring progress, NULL in case of error
+ */
+virJobPtr
+virNetworkCreateJob(virNetworkPtr network)
+{
+ virConnectPtr conn;
+ DEBUG("network=%p", network);
+
+ if (!VIR_IS_CONNECTED_NETWORK(network)) {
+ virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
+ return (NULL);
+ }
+ conn = network->conn;
+ if (conn->flags & VIR_CONNECT_RO) {
+ virLibNetworkError(network, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+ return (NULL);
+ }
+
+ if (conn->networkDriver && conn->networkDriver->networkCreateJob)
+ return conn->networkDriver->networkCreateJob (network);
+
+ virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ return (NULL);
}
/**
diff -r 9b65842892de src/libvirt_sym.version
--- a/src/libvirt_sym.version Fri Jan 04 17:29:33 2008 -0500
+++ b/src/libvirt_sym.version Fri Jan 04 18:00:06 2008 -0500
@@ -11,11 +11,22 @@
virConnectGetVersion;
virConnectGetHostname;
virConnectGetURI;
+
+ virJobGetInfo;
+ virJobGetDomain;
+ virJobGetNetwork;
+ virJobGetError;
+ virJobCopyError;
+ virJobCancel;
+ virJobDestroy;
+
virDomainGetConnect;
virConnectListDomains;
virConnectNumOfDomains;
virDomainCreate;
+ virDomainCreateJob;
virDomainCreateLinux;
+ virDomainCreateLinuxJob;
virDomainDefineXML;
virDomainDestroy;
virDomainFree;
@@ -33,9 +44,12 @@
virDomainLookupByUUID;
virDomainLookupByUUIDString;
virDomainRestore;
+ virDomainRestoreJob;
virDomainResume;
virDomainSave;
+ virDomainSaveJob;
virDomainCoreDump;
+ virDomainCoreDumpJob;
virDomainSetMemory;
virDomainSetMaxMemory;
virDomainShutdown;
@@ -87,9 +101,11 @@
virNetworkLookupByUUID;
virNetworkLookupByUUIDString;
virNetworkCreateXML;
+ virNetworkCreateXMLJob;
virNetworkDefineXML;
virNetworkUndefine;
virNetworkCreate;
+ virNetworkCreateJob;
virNetworkDestroy;
virNetworkFree;
virNetworkGetName;
diff -r 9b65842892de src/openvz_driver.c
--- a/src/openvz_driver.c Fri Jan 04 17:29:33 2008 -0500
+++ b/src/openvz_driver.c Fri Jan 04 18:00:06 2008 -0500
@@ -85,14 +85,6 @@ static int openvzShutdown(void);
static int openvzShutdown(void);
static int openvzReload(void);
static int openvzActive(void);
-static int openvzCloseNetwork(virConnectPtr conn);
-static virDrvOpenStatus openvzOpenNetwork(virConnectPtr conn,
- const char *name ATTRIBUTE_UNUSED,
- int *credtype ATTRIBUTE_UNUSED,
- int ncredtype ATTRIBUTE_UNUSED,
- virConnectAuthCallbackPtr cb ATTRIBUTE_UNUSED,
- void *cbdata ATTRIBUTE_UNUSED,
- int flags ATTRIBUTE_UNUSED);
static virDomainPtr openvzDomainDefineXML(virConnectPtr conn, const char *xml);
static virDomainPtr openvzDomainCreateLinux(virConnectPtr conn, const char *xml,
@@ -695,24 +687,11 @@ static int openvzActive(void) {
return 1;
}
-static int openvzCloseNetwork(virConnectPtr conn ATTRIBUTE_UNUSED) {
- return 0;
-}
-
-static virDrvOpenStatus openvzOpenNetwork(virConnectPtr conn ATTRIBUTE_UNUSED,
- const char *name ATTRIBUTE_UNUSED,
- int *credtype ATTRIBUTE_UNUSED,
- int ncredtype ATTRIBUTE_UNUSED,
- virConnectAuthCallbackPtr cb ATTRIBUTE_UNUSED,
- void *cbdata ATTRIBUTE_UNUSED,
- int flags ATTRIBUTE_UNUSED) {
- return VIR_DRV_OPEN_SUCCESS;
-}
-
static virDriver openvzDriver = {
VIR_DRV_OPENVZ,
"OPENVZ",
LIBVIR_VERSION_NUMBER,
+ NULL, /* jobDriver */
openvzOpen, /* open */
openvzClose, /* close */
NULL, /* supports_feature */
@@ -726,6 +705,7 @@ static virDriver openvzDriver = {
openvzListDomains, /* listDomains */
openvzNumDomains, /* numOfDomains */
openvzDomainCreateLinux, /* domainCreateLinux */
+ NULL, /* domainCreateLinuxJob */
openvzDomainLookupByID, /* domainLookupByID */
openvzDomainLookupByUUID, /* domainLookupByUUID */
openvzDomainLookupByName, /* domainLookupByName */
@@ -740,8 +720,11 @@ static virDriver openvzDriver = {
NULL, /* domainSetMemory */
openvzDomainGetInfo, /* domainGetInfo */
NULL, /* domainSave */
+ NULL, /* domainSaveJob */
NULL, /* domainRestore */
+ NULL, /* domainRestoreJob */
NULL, /* domainCoreDump */
+ NULL, /* domainCoreDumpJob */
NULL, /* domainSetVcpus */
NULL, /* domainPinVcpu */
NULL, /* domainGetVcpus */
@@ -750,6 +733,7 @@ static virDriver openvzDriver = {
openvzListDefinedDomains, /* listDomains */
openvzNumDefinedDomains, /* numOfDomains */
openvzDomainCreate, /* domainCreate */
+ NULL, /* domainCreateJob */
openvzDomainDefineXML, /* domainDefineXML */
openvzDomainUndefine, /* domainUndefine */
NULL, /* domainAttachDevice */
@@ -768,27 +752,6 @@ static virDriver openvzDriver = {
NULL, /* nodeGetFreeMemory */
};
-static virNetworkDriver openvzNetworkDriver = {
- NULL, /* name */
- openvzOpenNetwork, /* open */
- openvzCloseNetwork, /* close */
- NULL, /* numOfNetworks */
- NULL, /* listNetworks */
- NULL, /* numOfDefinedNetworks */
- NULL, /* listDefinedNetworks */
- NULL, /* networkLookupByUUID */
- NULL, /* networkLookupByName */
- NULL, /* networkCreateXML */
- NULL, /* networkDefineXML */
- NULL, /* networkUndefine */
- NULL, /* networkCreate */
- NULL, /* networkDestroy */
- NULL, /* networkDumpXML */
- NULL, /* networkGetBridgeName */
- NULL, /* networkGetAutostart */
- NULL, /* networkSetAutostart */
-};
-
static virStateDriver openvzStateDriver = {
openvzStartup,
openvzShutdown,
@@ -798,7 +761,6 @@ static virStateDriver openvzStateDriver
int openvzRegister(void) {
virRegisterDriver(&openvzDriver);
- virRegisterNetworkDriver(&openvzNetworkDriver);
virRegisterStateDriver(&openvzStateDriver);
return 0;
}
diff -r 9b65842892de src/qemu_driver.c
--- a/src/qemu_driver.c Fri Jan 04 17:29:33 2008 -0500
+++ b/src/qemu_driver.c Fri Jan 04 18:00:06 2008 -0500
@@ -2837,6 +2837,7 @@ static virDriver qemuDriver = {
VIR_DRV_QEMU,
"QEMU",
LIBVIR_VERSION_NUMBER,
+ NULL, /* jobDriver */
qemudOpen, /* open */
qemudClose, /* close */
NULL, /* supports_feature */
@@ -2850,6 +2851,7 @@ static virDriver qemuDriver = {
qemudListDomains, /* listDomains */
qemudNumDomains, /* numOfDomains */
qemudDomainCreate, /* domainCreateLinux */
+ NULL, /* domainCreateLinuxJob */
qemudDomainLookupByID, /* domainLookupByID */
qemudDomainLookupByUUID, /* domainLookupByUUID */
qemudDomainLookupByName, /* domainLookupByName */
@@ -2864,8 +2866,11 @@ static virDriver qemuDriver = {
NULL, /* domainSetMemory */
qemudDomainGetInfo, /* domainGetInfo */
qemudDomainSave, /* domainSave */
+ NULL, /* domainSaveJob */
qemudDomainRestore, /* domainRestore */
+ NULL, /* domainRestoreJob */
NULL, /* domainCoreDump */
+ NULL, /* domainCoreDumpJob */
NULL, /* domainSetVcpus */
NULL, /* domainPinVcpu */
NULL, /* domainGetVcpus */
@@ -2874,6 +2879,7 @@ static virDriver qemuDriver = {
qemudListDefinedDomains, /* listDomains */
qemudNumDefinedDomains, /* numOfDomains */
qemudDomainStart, /* domainCreate */
+ NULL, /* domainCreateJob */
qemudDomainDefine, /* domainDefineXML */
qemudDomainUndefine, /* domainUndefine */
qemudDomainAttachDevice, /* domainAttachDevice */
@@ -2894,6 +2900,7 @@ static virDriver qemuDriver = {
static virNetworkDriver qemuNetworkDriver = {
"QEMU",
+ NULL, /* jobDriver */
qemudOpenNetwork, /* open */
qemudCloseNetwork, /* close */
qemudNumNetworks, /* numOfNetworks */
@@ -2903,9 +2910,11 @@ static virNetworkDriver qemuNetworkDrive
qemudNetworkLookupByUUID, /* networkLookupByUUID */
qemudNetworkLookupByName, /* networkLookupByName */
qemudNetworkCreate, /* networkCreateXML */
+ NULL, /* networkCreateXMLJob */
qemudNetworkDefine, /* networkDefineXML */
qemudNetworkUndefine, /* networkUndefine */
qemudNetworkStart, /* networkCreate */
+ NULL, /* networkCreateJob */
qemudNetworkDestroy, /* networkDestroy */
qemudNetworkDumpXML, /* networkDumpXML */
qemudNetworkGetBridgeName, /* networkGetBridgeName */
diff -r 9b65842892de src/test.c
--- a/src/test.c Fri Jan 04 17:29:33 2008 -0500
+++ b/src/test.c Fri Jan 04 18:00:06 2008 -0500
@@ -1915,6 +1915,7 @@ static virDriver testDriver = {
VIR_DRV_TEST,
"Test",
LIBVIR_VERSION_NUMBER,
+ NULL, /* jobDriver */
testOpen, /* open */
testClose, /* close */
NULL, /* supports_feature */
@@ -1928,6 +1929,7 @@ static virDriver testDriver = {
testListDomains, /* listDomains */
testNumOfDomains, /* numOfDomains */
testDomainCreateLinux, /* domainCreateLinux */
+ NULL, /* domainCreateLinuxJob */
testLookupDomainByID, /* domainLookupByID */
testLookupDomainByUUID, /* domainLookupByUUID */
testLookupDomainByName, /* domainLookupByName */
@@ -1942,8 +1944,11 @@ static virDriver testDriver = {
testSetMemory, /* domainSetMemory */
testGetDomainInfo, /* domainGetInfo */
testDomainSave, /* domainSave */
+ NULL, /* domainSaveJob */
testDomainRestore, /* domainRestore */
+ NULL, /* domainRestoreJob */
testDomainCoreDump, /* domainCoreDump */
+ NULL, /* domainCoreDumpJob */
testSetVcpus, /* domainSetVcpus */
NULL, /* domainPinVcpu */
NULL, /* domainGetVcpus */
@@ -1952,6 +1957,7 @@ static virDriver testDriver = {
testListDefinedDomains, /* listDefinedDomains */
testNumOfDefinedDomains, /* numOfDefinedDomains */
testDomainCreate, /* domainCreate */
+ NULL, /* domainCreateJob */
testDomainDefineXML, /* domainDefineXML */
testDomainUndefine, /* domainUndefine */
NULL, /* domainAttachDevice */
@@ -1972,6 +1978,7 @@ static virDriver testDriver = {
static virNetworkDriver testNetworkDriver = {
"Test",
+ NULL, /* jobDriver */
testOpenNetwork, /* open */
testCloseNetwork, /* close */
testNumNetworks, /* numOfNetworks */
@@ -1981,9 +1988,11 @@ static virNetworkDriver testNetworkDrive
testLookupNetworkByUUID, /* networkLookupByUUID */
testLookupNetworkByName, /* networkLookupByName */
testNetworkCreate, /* networkCreateXML */
+ NULL, /* networkCreateXMLJob */
testNetworkDefine, /* networkDefineXML */
testNetworkUndefine, /* networkUndefine */
testNetworkStart, /* networkCreate */
+ NULL, /* networkCreateJob */
testNetworkDestroy, /* networkDestroy */
testNetworkDumpXML, /* networkDumpXML */
testNetworkGetBridgeName, /* networkGetBridgeName */
diff -r 9b65842892de src/virterror.c
--- a/src/virterror.c Fri Jan 04 17:29:33 2008 -0500
+++ b/src/virterror.c Fri Jan 04 18:00:06 2008 -0500
@@ -22,43 +22,6 @@ static virError lastErr = /* the l
{ 0, 0, NULL, VIR_ERR_NONE, NULL, NULL, NULL, NULL, NULL, 0, 0, NULL };
static virErrorFunc virErrorHandler = NULL; /* global error handlet */
static void *virUserData = NULL; /* associated data */
-
-/*
- * Macro used to format the message as a string in __virRaiseError
- * and borrowed from libxml2.
- */
-#define VIR_GET_VAR_STR(msg, str) { \
- int size, prev_size = -1; \
- int chars; \
- char *larger; \
- va_list ap; \
- \
- str = (char *) malloc(150); \
- if (str != NULL) { \
- \
- size = 150; \
- \
- while (1) { \
- va_start(ap, msg); \
- chars = vsnprintf(str, size, msg, ap); \
- va_end(ap); \
- if ((chars > -1) && (chars < size)) { \
- if (prev_size == chars) { \
- break; \
- } else { \
- prev_size = chars; \
- } \
- } \
- if (chars > -1) \
- size += chars + 1; \
- else \
- size += 100; \
- if ((larger = (char *) realloc(str, size)) == NULL) { \
- break; \
- } \
- str = larger; \
- }} \
-}
/*
* virGetLastError:
@@ -318,6 +281,49 @@ virDefaultErrorFunc(virErrorPtr err)
else
fprintf(stderr, "libvir: %s%s %s%s: %s",
dom, lvl, domain, network, err->message);
+}
+
+/**
+ * __virSetError:
+ * @err: the error object to set
+ * @conn: the connection to the hypervisor if available
+ * @dom: the domain if available
+ * @net: the network if available
+ * @domain: the virErrorDomain indicating where it's coming from
+ * @code: the virErrorNumber code for the error
+ * @level: the virErrorLevel for the error
+ * @str1: extra string info
+ * @str2: extra string info
+ * @str3: extra string info
+ * @int1: extra int info
+ * @int2: extra int info
+ * @msg: the message to display/transmit
+ *
+ * Internal routine called when an error is detected.
+ */
+void
+__virSetError(virErrorPtr err,
+ virConnectPtr conn, virDomainPtr dom, virNetworkPtr net,
+ int domain, int code, virErrorLevel level,
+ const char *str1, const char *str2, const char *str3,
+ int int1, int int2, const char *message)
+{
+ virResetError(err);
+ err->conn = conn;
+ err->dom = dom;
+ err->net = net;
+ err->domain = domain;
+ err->code = code;
+ err->message = strdup(message);
+ err->level = level;
+ if (str1 != NULL)
+ err->str1 = strdup(str1);
+ if (str2 != NULL)
+ err->str2 = strdup(str2);
+ if (str3 != NULL)
+ err->str3 = strdup(str3);
+ err->int1 = int1;
+ err->int2 = int2;
}
/**
@@ -373,26 +379,10 @@ __virRaiseError(virConnectPtr conn, virD
VIR_GET_VAR_STR(msg, str);
}
- /*
- * Save the information about the error
- */
- virResetError(to);
- to->conn = conn;
- to->dom = dom;
- to->net = net;
- to->domain = domain;
- to->code = code;
- to->message = str;
- to->level = level;
- if (str1 != NULL)
- to->str1 = strdup(str1);
- if (str2 != NULL)
- to->str2 = strdup(str2);
- if (str3 != NULL)
- to->str3 = strdup(str3);
- to->int1 = int1;
- to->int2 = int2;
-
+ __virSetError(to, conn, dom, net, domain, code, level,
+ str1, str2, str3, int1, int2, str);
+
+ free(str);
/*
* now, report it
*/
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
7:27 p.m.
New subject: [Libvir] RFC: PATCH 2/5: public & driver API for async jobs
On Sat, Jan 05, 2008 at 01:08:44AM +0000, Daniel P. Berrange wrote:
static void testDomainSaveWorker(virJobPtr job, void *data)
{
testJobPtr privdata = data;
/* A 50 second ETA for saving */
virJobInitialize(job, 50);
do {
... do a unit of work...
... Check for cancellation ...
if (virJobFinishCancel(job))
return;
... update progress, indicating 1 second
of work complete, remove 1 second from
the ETA, and add 2% of completion state...
virJobUpdateRel(job, 1, -1, 2);
} while (...not done...);
... Inform world we're all done ...
virJobFinishSuccess(job);
}
Considering the Xen driver for a minute. The 'domain create',
'domain save', 'domain restore' APIs we call in XenD via the
HTTP RPC service don't have any progress information so we can't
use these APIs to give any incremental progress or ETA for
completion. This is still useful to to give cancellation of the
API calls. Baically Xen driver does an HTTP post, and then blocks
waiting for the HTTP reply. Instead of blocking on read() we
could select() with a timeout of 500 ms, and on each timeout
check the cancellation flag. if cancelled we could drop the HTTP
connection prior to getting the response. So this could be an
incremental benefit even for existing Xen driver.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
5:09 a.m.
New subject: [Libvir] RFC: PATCH 2/5: public & driver API for async jobs
Daniel P. Berrange wrote:
This patch adds the public API for the virJobPtr object, and the
internal driver support code. The new public APIs are:
- int virJobGetInfo(virJobPtr job, virJobInfoPtr info);
This gets elapsed time, estimate completion time, completion
progress percentage, and completion status - success, failed,
cancelled.
- virDomainPtr virJobGetDomain(virJobPtr job);
If the job was a domain creation attempt, this gets the
resulting domain object
- virNetworkPtr virJobGetNetwork(virJobPtr job);
If the job was a network creation attempt, this gets the
resulting network object
- virErrorPtr virJobGetNetwork(virJobPtr job);
If the job failed, this gets the associated error
- int virJobCancel(virJobPtr job);
Request that the job be cancelled. This is not an immediate
operation. It merely sets a flag, which background jobs will
check periodically. So a job may still complete even if it
was cancelled. It should be responded to within a few seconds
though.
- int virJobDestroy(virJobPtr job);
Once a job has finished running (success, failed, or cancelled)
this may be called to release all resources associated with
the job.
As mentioned before, the following variants on existing APis are
added:
- virDomainCreateLinuxJob
- virDomainCreateJob
- virNetworkCreateJob
- virNetworkCreateXMLJob
- virDomainSaveJob
- virDomainRestoreJob
- virDomainDumpCoreJob
All are no-ops by default, unless the underlying driver supports
async jobs.
Finally, the job.c and job.h files provides a set of internal
routines to authors for the purpose of managing virJobPtr objects.
There is a default impl of the virJobDriver APIs which should be
suitable for all existing the remote driver. The async jobs are
basically run in a background pthread.
This psuedo code illustrates how the 'test' driver might use
these to implement an asynchronous variant of the 'save' method.
It basically stores the parameters in a struct, and then calls
the virJobCreate() method.
privdata = malloc(sizeof(*privdata));
if (privdata == NULL)
return (NULL);
memset(privdata, 0, sizeof(*privdata));
privdata->type = TEST_JOB_DOMAIN_SAVE;
privdata->data.save.domidx = domidx;
privdata->data.save.path = strdup(path);
job = virJobCreate(domain->conn,
domain->conn->driver->jobDriver,
testDomainSaveWorker,
NULL,
privdata,
VIR_JOB_BOUNDED);
The actual work is done in the 'testDomainSaveWorker' method
which is passed in to virJobCreate. The job.h file contains
APIs for it to call to update progress information and detect
cancellation:
static void testDomainSaveWorker(virJobPtr job, void *data)
{
testJobPtr privdata = data;
/* A 50 second ETA for saving */
virJobInitialize(job, 50);
do {
... do a unit of work...
... Check for cancellation ...
if (virJobFinishCancel(job))
return;
... update progress, indicating 1 second
of work complete, remove 1 second from
the ETA, and add 2% of completion state...
virJobUpdateRel(job, 1, -1, 2);
} while (...not done...);
... Inform world we're all done ...
virJobFinishSuccess(job);
}
The virJobInitialize, virJobFinishCancel, virJobFinishSuccess
and virJobUpdateRel methods all take care to lock the virJobPtr
object to ensure thread safety.
This API looks sensible.
I haven't checked the code in detail - I'll follow your Storage API
repository instead.
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
7:09 p.m.
New subject: [Libvir] RFC: PATCH 3/5: verbose job progress in virsh
This patch updates virsh to make use of any APIs with
new *Job variants. This means the 'create', 'start',
'save', 'restore', 'dump', 'net-create' and
'net-start'
methods all now take a '--verbose' option. If this
option is given, it will invoke the *Job variant of
the API and print out increment progress information.
eg, As an example of a job which has a bounded time,
$ ./virsh --connect test:///default save --verbose test foo
save [===== 18% ] Duration: 9 s, ETA: 41 s
error: Cancelled domain save operation
eg, As an example which is unbounded
$ ./virsh --connect test:///default save --verbose test foo
save [ = ] Duration: 9 s
error: Cancelled domain save operation
In the latter case, the '=' will bounce back & forth while
the job is running.
Both cases illustrate how the 'Ctrl-C' / SIGINT handler is
hooked up such that 'virJobCancel' is run. Pressing Ctrl-C
twice in quick succession will still immediately exit the
program - useful if cancellation fails for some reason.
virsh.c | 405 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 356 insertions(+), 49 deletions(-)
Dan.
diff -r ba58ad7f9763 src/virsh.c
--- a/src/virsh.c Fri Jan 04 18:00:06 2008 -0500
+++ b/src/virsh.c Fri Jan 04 18:33:32 2008 -0500
@@ -36,6 +36,7 @@
#include <sys/stat.h>
#include <inttypes.h>
#include <test.h>
+#include <signal.h>
#include <libxml/parser.h>
#include <libxml/tree.h>
@@ -50,6 +51,8 @@
#include "console.h"
static char *progname;
+
+static int interrupted = 0;
#ifndef TRUE
#define TRUE 1
@@ -302,6 +305,84 @@ static int namesorter(const void *a, con
}
+static void sigint_handler(int sig ATTRIBUTE_UNUSED) {
+ if (interrupted) {
+ _exit(255);
+ }
+ interrupted = 1;
+}
+
+/*
+ * Helper for commands running with an async job
+ */
+static int
+cmdMonitorProgress(vshControl *ctl, vshCmd *cmd, virJobPtr job, virJobInfoPtr info)
+{
+ int tick = 0;
+ int tickStep = 1;
+ /* Save cursor position */
+ fprintf(stdout, "\x1b[s");
+ fflush(stdout);
+
+ do {
+ if (virJobGetInfo(job, info) < 0) {
+ vshError(ctl, FALSE, _("Failed to get job status"));
+ return -1;
+ }
+
+ if (info->state == VIR_JOB_RUNNING) {
+ /* Restore cursor position and clear current line */
+ fprintf(stdout, "\x1b[u\x1b[K");
+
+ if (info->type == VIR_JOB_UNBOUNDED) {
+ int i;
+ char progress[26];
+ for (i = 0 ; i < 25 ; i++) {
+ if (i == tick)
+ progress[i] = '=';
+ else
+ progress[i] = ' ';
+ }
+ progress[25] = '\0';
+
+ fprintf(stdout, "%s [%s] Duration: %d s",
+ cmd->def->name, progress, info->runningTime);
+ } else {
+ int i;
+ char progress[26];
+ for (i = 0 ; i < 25 ; i++) {
+ if (i <= (info->percentComplete/4))
+ progress[i] = '=';
+ else
+ progress[i] = ' ';
+ }
+ if (info->percentComplete > 99)
+ progress[11] = '0' + (info->percentComplete / 100);
+ if (info->percentComplete > 9)
+ progress[12] = '0' + ((info->percentComplete % 100) /
10);
+ progress[13] = '0' + (info->percentComplete % 10);
+ progress[14] = '%';
+ progress[25] = '\0';
+
+ fprintf(stdout, "%s [%s] Duration: %d s, ETA: %d s",
+ cmd->def->name, progress, info->runningTime,
info->remainingTime);
+ }
+ fflush(stdout);
+ }
+ usleep(100 * 1000);
+ tick += tickStep;
+ if (tick == 24 || tick == 0)
+ tickStep *= -1;
+
+ if (interrupted) {
+ virJobCancel(job);
+ }
+ } while (info->state == VIR_JOB_RUNNING);
+ interrupted = 0;
+ fprintf(stdout, "\n");
+ return 0;
+}
+
/* ---------------
* Commands
* ---------------
@@ -840,6 +921,7 @@ static vshCmdInfo info_create[] = {
static vshCmdOptDef opts_create[] = {
{"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("file containing an
XML domain description")},
+ {"verbose", VSH_OT_BOOL, 0, gettext_noop("show progress
information")},
{NULL, 0, 0, NULL}
};
@@ -906,11 +988,11 @@ static int
static int
cmdCreate(vshControl * ctl, vshCmd * cmd)
{
- virDomainPtr dom;
char *from;
int found;
int ret = TRUE;
char *buffer;
+ int verbose = vshCommandOptBool(cmd, "verbose");
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
return FALSE;
@@ -922,17 +1004,48 @@ cmdCreate(vshControl * ctl, vshCmd * cmd
buffer = readFile (ctl, from);
if (buffer == NULL) return FALSE;
- dom = virDomainCreateLinux(ctl->conn, buffer, 0);
- free (buffer);
-
- if (dom != NULL) {
- vshPrint(ctl, _("Domain %s created from %s\n"),
- virDomainGetName(dom), from);
- virDomainFree(dom);
+ if (verbose) {
+ virJobPtr job;
+ virJobInfo info;
+
+ job = virDomainCreateLinuxJob(ctl->conn, buffer, 0);
+ free(buffer);
+ if (job == NULL) {
+ vshError(ctl, FALSE, _("Failed to create domain from %s"), from);
+ return FALSE;
+ }
+
+ if (cmdMonitorProgress(ctl,cmd, job, &info) < 0) {
+ virJobDestroy(job);
+ return FALSE;
+ }
+
+ if (info.state == VIR_JOB_COMPLETE) {
+ virDomainPtr dom = virJobGetDomain(job);
+ vshPrint(ctl, _("Domain %s created from %s\n"),
+ virDomainGetName(dom), from);
+ virDomainFree(dom);
+ } else if (info.state == VIR_JOB_CANCELLED) {
+ vshError(ctl, FALSE, _("Cancelled domain create operation"));
+ ret = FALSE;
+ } else {
+ vshError(ctl, FALSE, _("Failed to create domain from %s"), from);
+ ret = FALSE;
+ }
+ virJobDestroy(job);
} else {
- vshError(ctl, FALSE, _("Failed to create domain from %s"), from);
- ret = FALSE;
- }
+ virDomainPtr dom = virDomainCreateLinux(ctl->conn, buffer, 0);
+ free(buffer);
+ if (dom != NULL) {
+ vshPrint(ctl, _("Domain %s created from %s\n"),
+ virDomainGetName(dom), from);
+ virDomainFree(dom);
+ } else {
+ vshError(ctl, FALSE, _("Failed to create domain from %s"), from);
+ ret = FALSE;
+ }
+ }
+
return ret;
}
@@ -1036,6 +1149,7 @@ static vshCmdInfo info_start[] = {
static vshCmdOptDef opts_start[] = {
{"name", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("name of the
inactive domain")},
+ {"verbose", VSH_OT_BOOL, 0, gettext_noop("show progress
information")},
{NULL, 0, 0, NULL}
};
@@ -1044,6 +1158,7 @@ cmdStart(vshControl * ctl, vshCmd * cmd)
{
virDomainPtr dom;
int ret = TRUE;
+ int verbose = vshCommandOptBool(cmd, "verbose");
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
return FALSE;
@@ -1053,17 +1168,49 @@ cmdStart(vshControl * ctl, vshCmd * cmd)
if (virDomainGetID(dom) != (unsigned int)-1) {
vshError(ctl, FALSE, _("Domain is already active"));
- virDomainFree(dom);
- return FALSE;
- }
-
- if (virDomainCreate(dom) == 0) {
- vshPrint(ctl, _("Domain %s started\n"),
- virDomainGetName(dom));
+ virDomainFree(dom);
+ return FALSE;
+ }
+
+ if (verbose) {
+ virJobPtr job;
+ virJobInfo info;
+
+ job = virDomainCreateJob(dom, 0);
+ if (job == NULL) {
+ vshError(ctl, FALSE, _("Failed to start domain %s"),
+ virDomainGetName(dom));
+ virDomainFree(dom);
+ return FALSE;
+ }
+
+ if (cmdMonitorProgress(ctl,cmd, job, &info) < 0) {
+ virJobDestroy(job);
+ virDomainFree(dom);
+ return FALSE;
+ }
+
+ if (info.state == VIR_JOB_COMPLETE) {
+ vshPrint(ctl, _("Domain %s started\n"),
+ virDomainGetName(dom));
+ } else if (info.state == VIR_JOB_CANCELLED) {
+ vshError(ctl, FALSE, _("Cancelled domain start operation"));
+ ret = FALSE;
+ } else {
+ vshError(ctl, FALSE, _("Failed to start domain %s"),
+ virDomainGetName(dom));
+ ret = FALSE;
+ }
+ virJobDestroy(job);
} else {
- vshError(ctl, FALSE, _("Failed to start domain %s"),
- virDomainGetName(dom));
- ret = FALSE;
+ if (virDomainCreate(dom) == 0) {
+ vshPrint(ctl, _("Domain %s started\n"),
+ virDomainGetName(dom));
+ } else {
+ vshError(ctl, FALSE, _("Failed to start domain %s"),
+ virDomainGetName(dom));
+ ret = FALSE;
+ }
}
virDomainFree(dom);
return ret;
@@ -1082,6 +1229,7 @@ static vshCmdOptDef opts_save[] = {
static vshCmdOptDef opts_save[] = {
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id
or uuid")},
{"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("where to save the
data")},
+ {"verbose", VSH_OT_BOOL, 0, gettext_noop("show progress
information")},
{NULL, 0, 0, NULL}
};
@@ -1092,6 +1240,7 @@ cmdSave(vshControl * ctl, vshCmd * cmd)
char *name;
char *to;
int ret = TRUE;
+ int verbose = vshCommandOptBool(cmd, "verbose");
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
return FALSE;
@@ -1102,11 +1251,39 @@ cmdSave(vshControl * ctl, vshCmd * cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, "domain", &name)))
return FALSE;
- if (virDomainSave(dom, to) == 0) {
- vshPrint(ctl, _("Domain %s saved to %s\n"), name, to);
+ if (verbose) {
+ virJobPtr job;
+ virJobInfo info;
+ job = virDomainSaveJob(dom, to);
+ if (job == NULL) {
+ vshError(ctl, FALSE, _("Failed to save domain %s to %s"), name,
to);
+ virDomainFree(dom);
+ return FALSE;
+ }
+
+ if (cmdMonitorProgress(ctl,cmd, job, &info) < 0) {
+ virDomainFree(dom);
+ virJobDestroy(job);
+ return FALSE;
+ }
+
+ if (info.state == VIR_JOB_COMPLETE) {
+ vshPrint(ctl, _("Domain %s saved to %s\n"), name, to);
+ } else if (info.state == VIR_JOB_CANCELLED) {
+ vshError(ctl, FALSE, _("Cancelled domain save operation"));
+ ret = FALSE;
+ } else {
+ vshError(ctl, FALSE, _("Failed to save domain %s to %s"), name,
to);
+ ret = FALSE;
+ }
+ virJobDestroy(job);
} else {
- vshError(ctl, FALSE, _("Failed to save domain %s to %s"), name, to);
- ret = FALSE;
+ if (virDomainSave(dom, to) == 0) {
+ vshPrint(ctl, _("Domain %s saved to %s\n"), name, to);
+ } else {
+ vshError(ctl, FALSE, _("Failed to save domain %s to %s"), name,
to);
+ ret = FALSE;
+ }
}
virDomainFree(dom);
@@ -1277,6 +1454,7 @@ static vshCmdInfo info_restore[] = {
static vshCmdOptDef opts_restore[] = {
{"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("the state to
restore")},
+ {"verbose", VSH_OT_BOOL, 0, gettext_noop("show progress
information")},
{NULL, 0, 0, NULL}
};
@@ -1286,6 +1464,7 @@ cmdRestore(vshControl * ctl, vshCmd * cm
char *from;
int found;
int ret = TRUE;
+ int verbose = vshCommandOptBool(cmd, "verbose");
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
return FALSE;
@@ -1294,11 +1473,37 @@ cmdRestore(vshControl * ctl, vshCmd * cm
if (!found)
return FALSE;
- if (virDomainRestore(ctl->conn, from) == 0) {
- vshPrint(ctl, _("Domain restored from %s\n"), from);
+ if (verbose) {
+ virJobPtr job;
+ virJobInfo info;
+ job = virDomainRestoreJob(ctl->conn, from);
+ if (job == NULL) {
+ vshError(ctl, FALSE, _("Failed to restore domain from %s"), from);
+ return FALSE;
+ }
+
+ if (cmdMonitorProgress(ctl,cmd, job, &info) < 0) {
+ virJobDestroy(job);
+ return FALSE;
+ }
+
+ if (info.state == VIR_JOB_COMPLETE) {
+ vshPrint(ctl, _("Domain restored from %s\n"),from);
+ } else if (info.state == VIR_JOB_CANCELLED) {
+ vshError(ctl, FALSE, _("Cancelled domain restore operation"));
+ ret = FALSE;
+ } else {
+ vshError(ctl, FALSE, _("Failed to restore domain from %s"), from);
+ ret = FALSE;
+ }
+ virJobDestroy(job);
} else {
- vshError(ctl, FALSE, _("Failed to restore domain from %s"), from);
- ret = FALSE;
+ if (virDomainRestore(ctl->conn, from) == 0) {
+ vshPrint(ctl, _("Domain restored from %s\n"), from);
+ } else {
+ vshError(ctl, FALSE, _("Failed to restore domain from %s"), from);
+ ret = FALSE;
+ }
}
return ret;
}
@@ -1316,6 +1521,7 @@ static vshCmdOptDef opts_dump[] = {
static vshCmdOptDef opts_dump[] = {
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id
or uuid")},
{"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("where to dump the
core")},
+ {"verbose", VSH_OT_BOOL, 0, gettext_noop("show progress
information")},
{NULL, 0, 0, NULL}
};
@@ -1326,6 +1532,7 @@ cmdDump(vshControl * ctl, vshCmd * cmd)
char *name;
char *to;
int ret = TRUE;
+ int verbose = vshCommandOptBool(cmd, "verbose");
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
return FALSE;
@@ -1336,12 +1543,42 @@ cmdDump(vshControl * ctl, vshCmd * cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, "domain", &name)))
return FALSE;
- if (virDomainCoreDump(dom, to, 0) == 0) {
- vshPrint(ctl, _("Domain %s dumpd to %s\n"), name, to);
+ if (verbose) {
+ virJobPtr job;
+ virJobInfo info;
+ job = virDomainCoreDumpJob(dom, to, 0);
+ if (job == NULL) {
+ vshError(ctl, FALSE, _("Failed to core dump domain %s to %s"),
+ name, to);
+ virDomainFree(dom);
+ return FALSE;
+ }
+
+ if (cmdMonitorProgress(ctl,cmd, job, &info) < 0) {
+ virDomainFree(dom);
+ virJobDestroy(job);
+ return FALSE;
+ }
+
+ if (info.state == VIR_JOB_COMPLETE) {
+ vshPrint(ctl, _("Domain %s dumped to %s\n"), name, to);
+ } else if (info.state == VIR_JOB_CANCELLED) {
+ vshError(ctl, FALSE, _("Cancelled domain dump operation"));
+ ret = FALSE;
+ } else {
+ vshError(ctl, FALSE, _("Failed to core dump domain %s to %s"),
+ name, to);
+ ret = FALSE;
+ }
+ virJobDestroy(job);
} else {
- vshError(ctl, FALSE, _("Failed to core dump domain %s to %s"),
- name, to);
- ret = FALSE;
+ if (virDomainCoreDump(dom, to, 0) == 0) {
+ vshPrint(ctl, _("Domain %s dumped to %s\n"), name, to);
+ } else {
+ vshError(ctl, FALSE, _("Failed to core dump domain %s to %s"),
+ name, to);
+ ret = FALSE;
+ }
}
virDomainFree(dom);
@@ -2338,6 +2575,7 @@ static vshCmdInfo info_network_create[]
static vshCmdOptDef opts_network_create[] = {
{"file", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("file containing an
XML network description")},
+ {"verbose", VSH_OT_BOOL, 0, gettext_noop("show progress
information")},
{NULL, 0, 0, NULL}
};
@@ -2349,6 +2587,7 @@ cmdNetworkCreate(vshControl * ctl, vshCm
int found;
int ret = TRUE;
char *buffer;
+ int verbose = vshCommandOptBool(cmd, "verbose");
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
return FALSE;
@@ -2360,15 +2599,46 @@ cmdNetworkCreate(vshControl * ctl, vshCm
buffer = readFile (ctl, from);
if (buffer == NULL) return FALSE;
- network = virNetworkCreateXML(ctl->conn, buffer);
- free (buffer);
-
- if (network != NULL) {
- vshPrint(ctl, _("Network %s created from %s\n"),
- virNetworkGetName(network), from);
+ if (verbose) {
+ virJobPtr job;
+ virJobInfo info;
+
+ job = virNetworkCreateXMLJob(ctl->conn, buffer);
+ free(buffer);
+ if (job == NULL) {
+ vshError(ctl, FALSE, _("Failed to create network from %s"), from);
+ return FALSE;
+ }
+
+ if (cmdMonitorProgress(ctl,cmd, job, &info) < 0) {
+ virJobDestroy(job);
+ return FALSE;
+ }
+
+ if (info.state == VIR_JOB_COMPLETE) {
+ virNetworkPtr dom = virJobGetNetwork(job);
+ vshPrint(ctl, _("Network %s created from %s\n"),
+ virNetworkGetName(dom), from);
+ virNetworkFree(dom);
+ } else if (info.state == VIR_JOB_CANCELLED) {
+ vshError(ctl, FALSE, _("Cancelled network create operation"));
+ ret = FALSE;
+ } else {
+ vshError(ctl, FALSE, _("Failed to create network from %s"), from);
+ ret = FALSE;
+ }
+ virJobDestroy(job);
} else {
- vshError(ctl, FALSE, _("Failed to create network from %s"), from);
- ret = FALSE;
+ network = virNetworkCreateXML(ctl->conn, buffer);
+ free (buffer);
+
+ if (network != NULL) {
+ vshPrint(ctl, _("Network %s created from %s\n"),
+ virNetworkGetName(network), from);
+ } else {
+ vshError(ctl, FALSE, _("Failed to create network from %s"), from);
+ ret = FALSE;
+ }
}
return ret;
}
@@ -2674,6 +2944,7 @@ static vshCmdInfo info_network_start[] =
static vshCmdOptDef opts_network_start[] = {
{"name", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("name of the
inactive network")},
+ {"verbose", VSH_OT_BOOL, 0, gettext_noop("show progress
information")},
{NULL, 0, 0, NULL}
};
@@ -2682,6 +2953,7 @@ cmdNetworkStart(vshControl * ctl, vshCmd
{
virNetworkPtr network;
int ret = TRUE;
+ int verbose = vshCommandOptBool(cmd, "verbose");
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
return FALSE;
@@ -2689,14 +2961,47 @@ cmdNetworkStart(vshControl * ctl, vshCmd
if (!(network = vshCommandOptNetworkBy(ctl, cmd, "name", NULL,
VSH_BYNAME)))
return FALSE;
- if (virNetworkCreate(network) == 0) {
- vshPrint(ctl, _("Network %s started\n"),
- virNetworkGetName(network));
+ if (verbose) {
+ virJobPtr job;
+ virJobInfo info;
+
+ job = virNetworkCreateJob(network);
+ if (job == NULL) {
+ vshError(ctl, FALSE, _("Failed to start network %s"),
+ virNetworkGetName(network));
+ virNetworkFree(network);
+ return FALSE;
+ }
+
+ if (cmdMonitorProgress(ctl,cmd, job, &info) < 0) {
+ virJobDestroy(job);
+ virNetworkFree(network);
+ return FALSE;
+ }
+
+ if (info.state == VIR_JOB_COMPLETE) {
+ vshPrint(ctl, _("Network %s started\n"),
+ virNetworkGetName(network));
+ } else if (info.state == VIR_JOB_CANCELLED) {
+ vshError(ctl, FALSE, _("Cancelled network start operation"));
+ ret = FALSE;
+ } else {
+ vshError(ctl, FALSE, _("Failed to start network %s"),
+ virNetworkGetName(network));
+ ret = FALSE;
+ }
+ virJobDestroy(job);
} else {
- vshError(ctl, FALSE, _("Failed to start network %s"),
- virNetworkGetName(network));
- ret = FALSE;
- }
+ if (virNetworkCreate(network) == 0) {
+ vshPrint(ctl, _("Network %s started\n"),
+ virNetworkGetName(network));
+ } else {
+ vshError(ctl, FALSE, _("Failed to start network %s"),
+ virNetworkGetName(network));
+ ret = FALSE;
+ }
+ }
+ virNetworkFree(network);
return ret;
}
@@ -5017,6 +5322,8 @@ main(int argc, char **argv)
return -1;
}
+ signal(SIGINT, sigint_handler);
+
if (!(progname = strrchr(argv[0], '/')))
progname = argv[0];
else
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
5:09 a.m.
New subject: [Libvir] RFC: PATCH 3/5: verbose job progress in virsh
Daniel P. Berrange wrote:
This patch updates virsh to make use of any APIs with
new *Job variants. This means the 'create', 'start',
'save', 'restore', 'dump', 'net-create' and
'net-start'
methods all now take a '--verbose' option. If this
option is given, it will invoke the *Job variant of
the API and print out increment progress information.
eg, As an example of a job which has a bounded time,
$ ./virsh --connect test:///default save --verbose test foo
save [===== 18% ] Duration: 9 s, ETA: 41 s
error: Cancelled domain save operation
eg, As an example which is unbounded
$ ./virsh --connect test:///default save --verbose test foo
save [ = ] Duration: 9 s
error: Cancelled domain save operation
In the latter case, the '=' will bounce back & forth while
the job is running.
Both cases illustrate how the 'Ctrl-C' / SIGINT handler is
hooked up such that 'virJobCancel' is run. Pressing Ctrl-C
twice in quick succession will still immediately exit the
program - useful if cancellation fails for some reason.
Looks sensible.
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
7:10 p.m.
New subject: [Libvir] RFC: PATCH 4/5: make test driver thread safe
The current test driver keeps all its state in memory and is
not thread safe. Since background jobs will need to access
state from a thread periodically we need to make the test
driver thread safe. This is done with 2 levels of locking
- A global driver lock.
- A per-connection lock
Before taking the per-connection lock, the global driver lock
must be acquired. The driver lock can be released the moment
the connection lock is acquired. The connection lock must be
held for the duration of all driver methods which are accessing
the 'struct testConn' or 'struct testDom' or 'struct testNet'
data.
To keep this reasonably unobtrusive, the GET_CONNECT, and
GET_DOMAIN / GET_NETWORK macros have been altered to do the
neccessary lock acquisition. The RELEASE_DOMAIN and RELEASE_NETWORL
macros are new, and must be used prior to returning from any
driver method so that mutex are released.
This is a fairly coarse level of locking, but effective enough
for the test driver which is not performance critical - merely
safety critical.
This model should apply reasonably safely to the QEMU driver
too, though we may wish to make it finer grained.
test.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 214 insertions(+), 31 deletions(-)
Dan
diff -r d56f1d6ccb4a src/test.c
--- a/src/test.c Fri Jan 04 17:48:15 2008 -0500
+++ b/src/test.c Fri Jan 04 17:52:09 2008 -0500
@@ -37,12 +37,31 @@
#include <unistd.h>
#include <net/if.h>
#include <netinet/in.h>
+#include <pthread.h>
#include "internal.h"
#include "test.h"
#include "xml.h"
#include "buf.h"
#include "uuid.h"
+#include "job.h"
+
+
+/**
+ * Description of thread locking requirements:
+ *
+ * Locks on objects:
+ *
+ * - Global driver lock
+ * - Per connection lock
+ *
+ * If we wanted better granularity we could lock per-domain but
+ * that's overkill for now
+ *
+ * The driver lock must be held before acquiring a connection lock
+ *
+ * The GET / RELEASE macros should be used for this
+ */
/* Flags that determine the action to take on a shutdown or crash of a domain
*/
@@ -106,6 +125,7 @@ typedef struct _testNet *testNetPtr;
#define MAX_NETWORKS 20
struct _testConn {
+ pthread_mutex_t lock;
char path[PATH_MAX];
int nextDomID;
virNodeInfo nodeInfo;
@@ -117,6 +137,8 @@ typedef struct _testConn testConn;
typedef struct _testConn testConn;
typedef struct _testConn *testConnPtr;
+static pthread_mutex_t driverLock = PTHREAD_MUTEX_INITIALIZER;
+
#define TEST_MODEL "i686"
#define TEST_MODEL_WORDSIZE "32"
@@ -131,36 +153,62 @@ static const virNodeInfo defaultNodeInfo
2,
};
+/* Must call RELEASE_DOMAIN after this before returning */
#define GET_DOMAIN(dom, ret) \
int domidx; \
testConnPtr privconn; \
testDomPtr privdom; \
\
+ pthread_mutex_lock(&driverLock); \
privconn = (testConnPtr)dom->conn->privateData; \
- if ((domidx = getDomainIndex(dom)) < 0) { \
+ pthread_mutex_lock(&privconn->lock); \
+ pthread_mutex_unlock(&driverLock); \
+ if ((domidx = getDomainIndex(privconn, dom)) < 0) { \
testError((dom)->conn, (dom), NULL, VIR_ERR_INVALID_ARG, \
__FUNCTION__); \
+ pthread_mutex_unlock(&privconn->lock); \
return (ret); \
} \
privdom = &privconn->domains[domidx];
+/* Must call RELEASE_NETWORK after this before returning */
#define GET_NETWORK(net, ret) \
int netidx; \
testConnPtr privconn; \
testNetPtr privnet; \
\
+ pthread_mutex_lock(&driverLock); \
privconn = (testConnPtr)net->conn->privateData; \
- if ((netidx = getNetworkIndex(net)) < 0) { \
+ pthread_mutex_lock(&privconn->lock); \
+ pthread_mutex_unlock(&driverLock); \
+ if ((netidx = getNetworkIndex(privconn, net)) < 0) { \
testError((net)->conn, NULL, (net), VIR_ERR_INVALID_ARG, \
__FUNCTION__); \
+ pthread_mutex_unlock(&privconn->lock); \
return (ret); \
} \
privnet = &privconn->networks[netidx];
+/* Must call RELEASE_CONNECTION after this before returning */
#define GET_CONNECTION(conn, ret) \
testConnPtr privconn; \
\
- privconn = (testConnPtr)conn->privateData;
+ pthread_mutex_lock(&driverLock); \
+ privconn = (testConnPtr)conn->privateData; \
+ pthread_mutex_lock(&privconn->lock); \
+ pthread_mutex_unlock(&driverLock);
+
+/* To be called after GET_CONNECTIOn */
+#define RELEASE_CONNECTION() \
+ pthread_mutex_unlock(&privconn->lock)
+
+/* To be called after GET_NETWORK */
+#define RELEASE_NETWORK() \
+ pthread_mutex_unlock(&privconn->lock)
+
+/* To be called after GET_DOMAIN */
+#define RELEASE_DOMAIN() \
+ pthread_mutex_unlock(&privconn->lock)
static void
@@ -230,6 +278,7 @@ static int testLoadDomain(virConnectPtr
if (gettimeofday(&tv, NULL) < 0) {
testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("getting time of
day"));
+ RELEASE_CONNECTION();
return (-1);
}
@@ -327,7 +376,7 @@ static int testLoadDomain(virConnectPtr
}
}
if (handle < 0)
- return (-1);
+ goto error;
privconn->domains[handle].active = 1;
privconn->domains[handle].id = domid;
@@ -351,11 +400,13 @@ static int testLoadDomain(virConnectPtr
privconn->domains[handle].onPoweroff = onPoweroff;
privconn->domains[handle].onCrash = onCrash;
+ RELEASE_CONNECTION();
return (handle);
error:
if (name)
free(name);
+ RELEASE_CONNECTION();
return (-1);
}
@@ -488,7 +539,7 @@ static int testLoadNetwork(virConnectPtr
}
}
if (handle < 0)
- return (-1);
+ goto error;
privconn->networks[handle].active = 1;
privconn->networks[handle].running = 1;
@@ -522,6 +573,8 @@ static int testLoadNetwork(virConnectPtr
strncpy(privconn->networks[handle].dhcpEnd, dhcpend,
sizeof(privconn->networks[handle].dhcpEnd)-1);
privconn->networks[handle].dhcpEnd[sizeof(privconn->networks[handle].dhcpEnd)-1] =
'\0';
free(dhcpend);
+
+ RELEASE_CONNECTION();
return (handle);
error:
@@ -535,6 +588,7 @@ static int testLoadNetwork(virConnectPtr
free(dhcpend);
if (name)
free(name);
+ RELEASE_CONNECTION();
return (-1);
}
@@ -669,7 +723,7 @@ static int testOpenFromFile(virConnectPt
char *str;
xmlDocPtr xml = NULL;
xmlNodePtr root = NULL;
- xmlNodePtr *domains, *networks = NULL;
+ xmlNodePtr *domains = NULL, *networks = NULL;
xmlXPathContextPtr ctxt = NULL;
virNodeInfoPtr nodeInfo;
testConnPtr privconn = malloc(sizeof(testConn));
@@ -851,9 +905,8 @@ static int testOpenFromFile(virConnectPt
return VIR_DRV_OPEN_ERROR;
}
-static int getDomainIndex(virDomainPtr domain) {
+static int getDomainIndex(testConnPtr privconn, virDomainPtr domain) {
int i;
- GET_CONNECTION(domain->conn, -1);
for (i = 0 ; i < MAX_DOMAINS ; i++) {
if (domain->id >= 0) {
@@ -867,9 +920,8 @@ static int getDomainIndex(virDomainPtr d
return (-1);
}
-static int getNetworkIndex(virNetworkPtr network) {
+static int getNetworkIndex(testConnPtr privconn, virNetworkPtr network) {
int i;
- GET_CONNECTION(network->conn, -1);
for (i = 0 ; i < MAX_NETWORKS ; i++) {
if (STREQ(network->name, privconn->networks[i].name))
@@ -956,8 +1008,10 @@ static char * testGetURI (virConnectPtr
if (asprintf (&uri, "test://%s", privconn->path) == -1) {
testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, strerror (errno));
+ RELEASE_CONNECTION();
return NULL;
}
+ RELEASE_CONNECTION();
return uri;
}
@@ -972,6 +1026,7 @@ static int testNodeGetInfo(virConnectPtr
{
GET_CONNECTION(conn, -1);
memcpy(info, &privconn->nodeInfo, sizeof(virNodeInfo));
+ RELEASE_CONNECTION();
return (0);
}
@@ -1022,6 +1077,7 @@ static int testNumOfDomains(virConnectPt
continue;
numActive++;
}
+ RELEASE_CONNECTION();
return (numActive);
}
@@ -1035,22 +1091,30 @@ testDomainCreateLinux(virConnectPtr conn
if (xmlDesc == NULL) {
testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ RELEASE_CONNECTION();
return (NULL);
}
if (privconn->numDomains == MAX_DOMAINS) {
testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many
domains"));
+ RELEASE_CONNECTION();
return (NULL);
}
domid = privconn->nextDomID++;
- if ((handle = testLoadDomainFromDoc(conn, domid, xmlDesc)) < 0)
+ if ((handle = testLoadDomainFromDoc(conn, domid, xmlDesc)) < 0) {
+ RELEASE_CONNECTION();
return (NULL);
+ }
privconn->domains[handle].config = 0;
dom = virGetDomain(conn, privconn->domains[handle].name,
privconn->domains[handle].uuid);
- if (dom == NULL) return NULL;
+ if (dom == NULL) {
+ RELEASE_CONNECTION();
+ return NULL;
+ }
privconn->numDomains++;
+ RELEASE_CONNECTION();
return (dom);
}
@@ -1072,12 +1136,17 @@ static virDomainPtr testLookupDomainByID
if (idx < 0) {
testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+ RELEASE_CONNECTION();
return(NULL);
}
dom = virGetDomain(conn, privconn->domains[idx].name,
privconn->domains[idx].uuid);
- if (dom == NULL) return NULL;
+ if (dom == NULL) {
+ RELEASE_CONNECTION();
+ return NULL;
+ }
dom->id = id;
+ RELEASE_CONNECTION();
return (dom);
}
@@ -1098,13 +1167,18 @@ static virDomainPtr testLookupDomainByUU
if (idx < 0) {
testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+ RELEASE_CONNECTION();
return NULL;
}
dom = virGetDomain(conn, privconn->domains[idx].name,
privconn->domains[idx].uuid);
- if (dom == NULL) return NULL;
+ if (dom == NULL) {
+ RELEASE_CONNECTION();
+ return NULL;
+ }
dom->id = privconn->domains[idx].id;
+ RELEASE_CONNECTION();
return dom;
}
@@ -1125,13 +1199,18 @@ static virDomainPtr testLookupDomainByNa
if (idx < 0) {
testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+ RELEASE_CONNECTION();
return NULL;
}
dom = virGetDomain(conn, privconn->domains[idx].name,
privconn->domains[idx].uuid);
- if (dom == NULL) return NULL;
+ if (dom == NULL) {
+ RELEASE_CONNECTION();
+ return NULL;
+ }
dom->id = privconn->domains[idx].id;
+ RELEASE_CONNECTION();
return dom;
}
@@ -1148,6 +1227,7 @@ static int testListDomains (virConnectPt
ids[n++] = privconn->domains[i].id;
}
}
+ RELEASE_CONNECTION();
return (n);
}
@@ -1162,6 +1242,7 @@ static int testDestroyDomain (virDomainP
} else {
privdom->active = 0;
}
+ RELEASE_DOMAIN();
return (0);
}
@@ -1171,10 +1252,12 @@ static int testResumeDomain (virDomainPt
if (privdom->info.state != VIR_DOMAIN_PAUSED) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not
paused");
+ RELEASE_DOMAIN();
return -1;
}
privdom->info.state = VIR_DOMAIN_RUNNING;
+ RELEASE_DOMAIN();
return (0);
}
@@ -1185,10 +1268,12 @@ static int testPauseDomain (virDomainPtr
if (privdom->info.state == VIR_DOMAIN_SHUTOFF ||
privdom->info.state == VIR_DOMAIN_PAUSED) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not
running");
+ RELEASE_DOMAIN();
return -1;
}
privdom->info.state = VIR_DOMAIN_PAUSED;
+ RELEASE_DOMAIN();
return (0);
}
@@ -1198,13 +1283,14 @@ static int testShutdownDomain (virDomain
if (privdom->info.state == VIR_DOMAIN_SHUTOFF) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not
running");
+ RELEASE_DOMAIN();
return -1;
}
privdom->info.state = VIR_DOMAIN_SHUTOFF;
domain->id = -1;
privdom->id = -1;
-
+ RELEASE_DOMAIN();
return (0);
}
@@ -1245,6 +1331,7 @@ static int testRebootDomain (virDomainPt
break;
}
+ RELEASE_DOMAIN();
return (0);
}
@@ -1256,6 +1343,7 @@ static int testGetDomainInfo (virDomainP
if (gettimeofday(&tv, NULL) < 0) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, _("getting
time of day"));
+ RELEASE_DOMAIN();
return (-1);
}
@@ -1266,10 +1354,12 @@ static int testGetDomainInfo (virDomainP
privdom->info.cpuTime = ((tv.tv_sec * 1000ll * 1000ll * 1000ll) + (tv.tv_usec
* 1000ll));
}
memcpy(info, &privdom->info, sizeof(virDomainInfo));
+
+ RELEASE_DOMAIN();
return (0);
}
-static char *testDomainDumpXML(virDomainPtr domain, int flags);
+static char *testDomainDumpXMLLocked(virDomainPtr domain, testDomPtr privdom, int
flags);
#define TEST_SAVE_MAGIC "TestGuestMagic"
@@ -1280,11 +1370,12 @@ static int testDomainSave(virDomainPtr d
int fd, len;
GET_DOMAIN(domain, -1);
- xml = testDomainDumpXML(domain, 0);
+ xml = testDomainDumpXMLLocked(domain, privdom, 0);
if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
"cannot save domain");
+ RELEASE_DOMAIN();
return (-1);
}
len = strlen(xml);
@@ -1292,24 +1383,28 @@ static int testDomainSave(virDomainPtr d
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
"cannot write header");
close(fd);
+ RELEASE_DOMAIN();
return (-1);
}
if (write(fd, (char*)&len, sizeof(len)) != sizeof(len)) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
"cannot write metadata length");
close(fd);
+ RELEASE_DOMAIN();
return (-1);
}
if (write(fd, xml, len) != len) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
"cannot write metadata");
close(fd);
+ RELEASE_DOMAIN();
return (-1);
}
if (close(fd) < 0) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
"cannot save domain data");
close(fd);
+ RELEASE_DOMAIN();
return (-1);
}
if (privdom->config) {
@@ -1319,8 +1414,10 @@ static int testDomainSave(virDomainPtr d
} else {
privdom->active = 0;
}
+ RELEASE_DOMAIN();
return 0;
}
+
static int testDomainRestore(virConnectPtr conn,
const char *path)
@@ -1333,42 +1430,49 @@ static int testDomainRestore(virConnectP
if ((fd = open(path, O_RDONLY)) < 0) {
testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"cannot read domain image");
+ RELEASE_CONNECTION();
return (-1);
}
if (read(fd, magic, sizeof(magic)) != sizeof(magic)) {
testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"incomplete save header");
close(fd);
+ RELEASE_CONNECTION();
return (-1);
}
if (memcmp(magic, TEST_SAVE_MAGIC, sizeof(magic))) {
testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"mismatched header magic");
close(fd);
+ RELEASE_CONNECTION();
return (-1);
}
if (read(fd, (char*)&len, sizeof(len)) != sizeof(len)) {
testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"failed to read metadata length");
close(fd);
+ RELEASE_CONNECTION();
return (-1);
}
if (len < 1 || len > 8192) {
testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"length of metadata out of range");
close(fd);
+ RELEASE_CONNECTION();
return (-1);
}
xml = malloc(len+1);
if (!xml) {
testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, "xml");
close(fd);
+ RELEASE_CONNECTION();
return (-1);
}
if (read(fd, xml, len) != len) {
testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"incomplete metdata");
close(fd);
+ RELEASE_CONNECTION();
return (-1);
}
xml[len] = '\0';
@@ -1376,6 +1480,7 @@ static int testDomainRestore(virConnectP
domid = privconn->nextDomID++;
ret = testLoadDomainFromDoc(conn, domid, xml);
free(xml);
+ RELEASE_CONNECTION();
return ret < 0 ? -1 : 0;
}
@@ -1389,18 +1494,21 @@ static int testDomainCoreDump(virDomainP
if ((fd = open(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
"cannot save domain core");
+ RELEASE_DOMAIN();
return (-1);
}
if (write(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) != sizeof(TEST_SAVE_MAGIC))
{
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
"cannot write header");
close(fd);
+ RELEASE_DOMAIN();
return (-1);
}
if (close(fd) < 0) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
"cannot save domain data");
close(fd);
+ RELEASE_DOMAIN();
return (-1);
}
if (privdom->config) {
@@ -1410,6 +1518,7 @@ static int testDomainCoreDump(virDomainP
} else {
privdom->active = 0;
}
+ RELEASE_DOMAIN();
return 0;
}
@@ -1418,9 +1527,12 @@ static char *testGetOSType(virDomainPtr
}
static unsigned long testGetMaxMemory(virDomainPtr domain) {
+ unsigned long ret;
GET_DOMAIN(domain, -1);
- return privdom->info.maxMem;
+ ret = privdom->info.maxMem;
+ RELEASE_DOMAIN();
+ return ret;
}
static int testSetMaxMemory(virDomainPtr domain,
@@ -1430,6 +1542,7 @@ static int testSetMaxMemory(virDomainPtr
/* XXX validate not over host memory wrt to other domains */
privdom->info.maxMem = memory;
+ RELEASE_DOMAIN();
return (0);
}
@@ -1440,10 +1553,12 @@ static int testSetMemory(virDomainPtr do
if (memory > privdom->info.maxMem) {
testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ RELEASE_DOMAIN();
return (-1);
}
privdom->info.memory = memory;
+ RELEASE_DOMAIN();
return (0);
}
@@ -1454,20 +1569,21 @@ static int testSetVcpus(virDomainPtr dom
/* We allow more cpus in guest than host */
if (nrCpus > 32) {
testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ RELEASE_DOMAIN();
return (-1);
}
privdom->info.nrVirtCpu = nrCpus;
+ RELEASE_DOMAIN();
return (0);
}
-static char *testDomainDumpXML(virDomainPtr domain, int flags ATTRIBUTE_UNUSED)
+static char *testDomainDumpXMLLocked(virDomainPtr domain, testDomPtr privdom, int flags
ATTRIBUTE_UNUSED)
{
virBufferPtr buf;
char *xml;
unsigned char *uuid;
char uuidstr[VIR_UUID_STRING_BUFLEN];
- GET_DOMAIN(domain, NULL);
if (!(buf = virBufferNew(4000))) {
testError(domain->conn, domain, NULL, VIR_ERR_NO_MEMORY, __FUNCTION__);
@@ -1489,8 +1605,17 @@ static char *testDomainDumpXML(virDomain
xml = buf->content;
free(buf);
-
return (xml);
+}
+
+static char *testDomainDumpXML(virDomainPtr domain, int flags)
+{
+ char *xml;
+ GET_DOMAIN(domain, NULL);
+
+ xml = testDomainDumpXMLLocked(domain, privdom, flags);
+ RELEASE_DOMAIN();
+ return xml;
}
static int testNumOfDefinedDomains(virConnectPtr conn) {
@@ -1503,6 +1628,7 @@ static int testNumOfDefinedDomains(virCo
continue;
numInactive++;
}
+ RELEASE_CONNECTION();
return (numInactive);
}
@@ -1518,6 +1644,7 @@ static int testListDefinedDomains(virCon
names[n++] = strdup(privconn->domains[i].name);
}
}
+ RELEASE_CONNECTION();
return (n);
}
@@ -1525,12 +1652,14 @@ static virDomainPtr testDomainDefineXML(
const char *doc) {
int handle;
xmlDocPtr xml;
+ virDomainPtr ret;
GET_CONNECTION(conn, NULL);
if (!(xml = xmlReadDoc(BAD_CAST doc, "domain.xml", NULL,
XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain"));
+ RELEASE_CONNECTION();
return (NULL);
}
@@ -1539,10 +1668,14 @@ static virDomainPtr testDomainDefineXML(
xmlFreeDoc(xml);
- if (handle < 0)
+ if (handle < 0) {
+ RELEASE_CONNECTION();
return (NULL);
-
- return virGetDomain(conn, privconn->domains[handle].name,
privconn->domains[handle].uuid);
+ }
+
+ ret = virGetDomain(conn, privconn->domains[handle].name,
privconn->domains[handle].uuid);
+ RELEASE_CONNECTION();
+ return ret;
}
static int testDomainCreate(virDomainPtr domain) {
@@ -1551,12 +1684,14 @@ static int testDomainCreate(virDomainPtr
if (privdom->info.state != VIR_DOMAIN_SHUTOFF) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
_("Domain is already running"));
+ RELEASE_DOMAIN();
return (-1);
}
domain->id = privdom->id = privconn->nextDomID++;
privdom->info.state = VIR_DOMAIN_RUNNING;
+ RELEASE_DOMAIN();
return (0);
}
@@ -1566,11 +1701,13 @@ static int testDomainUndefine(virDomainP
if (privdom->info.state != VIR_DOMAIN_SHUTOFF) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
_("Domain is still running"));
+ RELEASE_DOMAIN();
return (-1);
}
privdom->active = 0;
+ RELEASE_DOMAIN();
return (0);
}
@@ -1579,6 +1716,7 @@ static int testDomainGetAutostart(virDom
{
GET_DOMAIN(domain, -1);
*autostart = privdom->autostart;
+ RELEASE_DOMAIN();
return (0);
}
@@ -1588,6 +1726,7 @@ static int testDomainSetAutostart(virDom
{
GET_DOMAIN(domain, -1);
privdom->autostart = autostart ? 1 : 0;
+ RELEASE_DOMAIN();
return (0);
}
@@ -1611,11 +1750,13 @@ static int testDomainGetSchedulerParams(
GET_DOMAIN(domain, -1);
if (*nparams != 1) {
testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG,
"nparams");
+ RELEASE_DOMAIN();
return (-1);
}
strcpy(params[0].field, "weight");
params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT;
params[0].value.ui = privdom->weight;
+ RELEASE_DOMAIN();
return 0;
}
@@ -1627,17 +1768,21 @@ static int testDomainSetSchedulerParams(
GET_DOMAIN(domain, -1);
if (nparams != 1) {
testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG,
"nparams");
+ RELEASE_DOMAIN();
return (-1);
}
if (STRNEQ(params[0].field, "weight")) {
testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG,
"field");
+ RELEASE_DOMAIN();
return (-1);
}
if (params[0].type != VIR_DOMAIN_SCHED_FIELD_UINT) {
testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG, "type");
+ RELEASE_DOMAIN();
return (-1);
}
privdom->weight = params[0].value.ui;
+ RELEASE_DOMAIN();
return 0;
}
@@ -1662,6 +1807,7 @@ static virNetworkPtr testLookupNetworkBy
const unsigned char *uuid)
{
int i, idx = -1;
+ virNetworkPtr ret;
GET_CONNECTION(conn, NULL);
for (i = 0 ; i < MAX_NETWORKS ; i++) {
@@ -1677,13 +1823,16 @@ static virNetworkPtr testLookupNetworkBy
return NULL;
}
- return virGetNetwork(conn, privconn->networks[idx].name,
privconn->networks[idx].uuid);
+ ret = virGetNetwork(conn, privconn->networks[idx].name,
privconn->networks[idx].uuid);
+ RELEASE_CONNECTION();
+ return ret;
}
static virNetworkPtr testLookupNetworkByName(virConnectPtr conn,
const char *name)
{
int i, idx = -1;
+ virNetworkPtr ret;
GET_CONNECTION(conn, NULL);
for (i = 0 ; i < MAX_NETWORKS ; i++) {
@@ -1696,10 +1845,13 @@ static virNetworkPtr testLookupNetworkBy
if (idx < 0) {
testError (conn, NULL, NULL, VIR_ERR_NO_NETWORK, NULL);
+ RELEASE_CONNECTION();
return NULL;
}
- return virGetNetwork(conn, privconn->networks[idx].name,
privconn->networks[idx].uuid);
+ ret = virGetNetwork(conn, privconn->networks[idx].name,
privconn->networks[idx].uuid);
+ RELEASE_CONNECTION();
+ return ret;
}
@@ -1713,6 +1865,7 @@ static int testNumNetworks(virConnectPtr
continue;
numInactive++;
}
+ RELEASE_CONNECTION();
return (numInactive);
}
@@ -1726,6 +1879,7 @@ static int testListNetworks(virConnectPt
names[n++] = strdup(privconn->networks[i].name);
}
}
+ RELEASE_CONNECTION();
return (n);
}
@@ -1739,6 +1893,7 @@ static int testNumDefinedNetworks(virCon
continue;
numInactive++;
}
+ RELEASE_CONNECTION();
return (numInactive);
}
@@ -1752,6 +1907,7 @@ static int testListDefinedNetworks(virCo
names[n++] = strdup(privconn->networks[i].name);
}
}
+ RELEASE_CONNECTION();
return (n);
}
@@ -1762,21 +1918,29 @@ static virNetworkPtr testNetworkCreate(v
if (xml == NULL) {
testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ RELEASE_CONNECTION();
return (NULL);
}
if (privconn->numNetworks == MAX_NETWORKS) {
testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many
networks"));
+ RELEASE_CONNECTION();
return (NULL);
}
- if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0)
+ if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0) {
+ RELEASE_CONNECTION();
return (NULL);
+ }
privconn->networks[handle].config = 0;
net = virGetNetwork(conn, privconn->networks[handle].name,
privconn->networks[handle].uuid);
- if (net == NULL) return NULL;
+ if (net == NULL) {
+ RELEASE_CONNECTION();
+ return NULL;
+ }
privconn->numNetworks++;
+ RELEASE_CONNECTION();
return (net);
}
@@ -1787,21 +1951,29 @@ static virNetworkPtr testNetworkDefine(v
if (xml == NULL) {
testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ RELEASE_CONNECTION();
return (NULL);
}
if (privconn->numNetworks == MAX_NETWORKS) {
testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many
networks"));
+ RELEASE_CONNECTION();
return (NULL);
}
- if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0)
+ if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0) {
+ RELEASE_CONNECTION();
return (NULL);
+ }
net = virGetNetwork(conn, privconn->networks[handle].name,
privconn->networks[handle].uuid);
privconn->networks[handle].config = 1;
- if (net == NULL) return NULL;
+ if (net == NULL) {
+ RELEASE_CONNECTION();
+ return NULL;
+ }
privconn->numNetworks++;
+ RELEASE_CONNECTION();
return (net);
}
@@ -1811,11 +1983,13 @@ static int testNetworkUndefine(virNetwor
if (privnet->running) {
testError(network->conn, NULL, network, VIR_ERR_INTERNAL_ERROR,
_("Network is still running"));
+ RELEASE_NETWORK();
return (-1);
}
privnet->active = 0;
+ RELEASE_NETWORK();
return (0);
}
@@ -1825,11 +1999,13 @@ static int testNetworkStart(virNetworkPt
if (privnet->running) {
testError(network->conn, NULL, network, VIR_ERR_INTERNAL_ERROR,
_("Network is already running"));
+ RELEASE_NETWORK();
return (-1);
}
privnet->running = 1;
+ RELEASE_NETWORK();
return (0);
}
@@ -1841,6 +2017,7 @@ static int testNetworkDestroy(virNetwork
} else {
privnet->active = 0;
}
+ RELEASE_NETWORK();
return (0);
}
@@ -1853,6 +2030,7 @@ static char *testNetworkDumpXML(virNetwo
if (!(buf = virBufferNew(4000))) {
testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY, __FUNCTION__);
+ RELEASE_NETWORK();
return (NULL);
}
@@ -1882,6 +2060,7 @@ static char *testNetworkDumpXML(virNetwo
xml = buf->content;
free(buf);
+ RELEASE_NETWORK();
return (xml);
}
@@ -1891,8 +2070,10 @@ static char *testNetworkGetBridgeName(vi
bridge = strdup(privnet->bridge);
if (!bridge) {
testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY,
"network");
+ RELEASE_NETWORK();
return NULL;
}
+ RELEASE_NETWORK();
return bridge;
}
@@ -1900,6 +2081,7 @@ static int testNetworkGetAutostart(virNe
int *autostart) {
GET_NETWORK(network, -1);
*autostart = privnet->autostart;
+ RELEASE_NETWORK();
return (0);
}
@@ -1907,6 +2089,7 @@ static int testNetworkSetAutostart(virNe
int autostart) {
GET_NETWORK(network, -1);
privnet->autostart = autostart ? 1 : 0;
+ RELEASE_NETWORK();
return (0);
}
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
5:11 a.m.
New subject: [Libvir] RFC: PATCH 4/5: make test driver thread safe
Daniel P. Berrange wrote:
The current test driver keeps all its state in memory and is
not thread safe. Since background jobs will need to access
state from a thread periodically we need to make the test
driver thread safe. This is done with 2 levels of locking
- A global driver lock.
- A per-connection lock
Before taking the per-connection lock, the global driver lock
must be acquired. The driver lock can be released the moment
the connection lock is acquired. The connection lock must be
held for the duration of all driver methods which are accessing
the 'struct testConn' or 'struct testDom' or 'struct testNet'
data.
To keep this reasonably unobtrusive, the GET_CONNECT, and
GET_DOMAIN / GET_NETWORK macros have been altered to do the
neccessary lock acquisition. The RELEASE_DOMAIN and RELEASE_NETWORL
macros are new, and must be used prior to returning from any
driver method so that mutex are released.
This is a fairly coarse level of locking, but effective enough
for the test driver which is not performance critical - merely
safety critical.
This model should apply reasonably safely to the QEMU driver
too, though we may wish to make it finer grained.
Sensible. Maybe add some artifically slow functions to the test driver
as well so that we can test job control?
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
7:11 p.m.
New subject: [Libvir] RFC: PATCH 5/5: async job for test driver save API
This patch implements the virDomainSaveJob method for the
test driver as a proof of concept. First of all it adds a
inJob flag to the testDom and testNet objects. This is to
make sure that operations which may change an objects run
state are rejected, if a background job is active.
Then it merely implements the virDomainSaveJob method as
per the previous description. Actually it doesn't really
do any save operation, this impl just illustrates the way
progress updates propagate back to the caller, and the use
of cancellation.
test.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 116 insertions(+), 3 deletions(-)
Dan.
diff -r 356d3624723c src/test.c
--- a/src/test.c Fri Jan 04 18:22:17 2008 -0500
+++ b/src/test.c Fri Jan 04 18:24:56 2008 -0500
@@ -72,6 +72,28 @@ typedef enum {
VIR_DOMAIN_RENAME_RESTART= 4/* restart under an new unique name */
} virDomainRestart;
+typedef enum {
+ TEST_JOB_INACTIVE = 0,
+ TEST_JOB_DOMAIN_START = 1,
+ TEST_JOB_DOMAIN_SAVE = 2,
+ TEST_JOB_DOMAIN_RESTORE = 3,
+ TEST_JOB_DOMAIN_CORE = 4,
+ TEST_JOB_NETWORK_START = 5,
+} testJobType;
+
+typedef struct _testJob testJob;
+typedef struct _testJob *testJobPtr;
+
+struct _testJob {
+ int type;
+ union {
+ struct {
+ int domidx;
+ char *path;
+ } save;
+ } data;
+};
+
struct _testDev {
char name[20];
int mode;
@@ -82,6 +104,7 @@ typedef struct _testDev *testDevPtr;
#define MAX_DEVICES 10
struct _testDom {
+ int inJob;
int active;
int config;
int id;
@@ -102,6 +125,7 @@ typedef struct _testDom *testDomPtr;
typedef struct _testDom *testDomPtr;
struct _testNet {
+ int inJob;
int active;
int config;
int running;
@@ -209,6 +233,20 @@ static const virNodeInfo defaultNodeInfo
/* To be called after GET_DOMAIN */
#define RELEASE_DOMAIN() \
pthread_mutex_unlock(&privconn->lock)
+
+#define CHECK_DOMAIN_JOB_INACTIVE(dom, ret) \
+ if (privdom->inJob) { \
+ testError((dom)->conn, (dom), NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); \
+ RELEASE_DOMAIN(); \
+ return (ret); \
+ }
+
+#define CHECK_NETWORK_JOB_INACTIVE(net, ret) \
+ if (privnet->inJob) { \
+ testError((net)->conn, NULL, (net), VIR_ERR_INVALID_ARG, __FUNCTION__); \
+ RELEASE_NETWORK(); \
+ return (ret); \
+ }
static void
@@ -1234,6 +1272,7 @@ static int testDestroyDomain (virDomainP
static int testDestroyDomain (virDomainPtr domain)
{
GET_DOMAIN(domain, -1);
+ CHECK_DOMAIN_JOB_INACTIVE(domain, -1);
if (privdom->config) {
privdom->info.state = VIR_DOMAIN_SHUTOFF;
@@ -1249,6 +1288,7 @@ static int testResumeDomain (virDomainPt
static int testResumeDomain (virDomainPtr domain)
{
GET_DOMAIN(domain, -1);
+ CHECK_DOMAIN_JOB_INACTIVE(domain, -1);
if (privdom->info.state != VIR_DOMAIN_PAUSED) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not
paused");
@@ -1264,6 +1304,7 @@ static int testPauseDomain (virDomainPtr
static int testPauseDomain (virDomainPtr domain)
{
GET_DOMAIN(domain, -1);
+ CHECK_DOMAIN_JOB_INACTIVE(domain, -1);
if (privdom->info.state == VIR_DOMAIN_SHUTOFF ||
privdom->info.state == VIR_DOMAIN_PAUSED) {
@@ -1280,6 +1321,7 @@ static int testShutdownDomain (virDomain
static int testShutdownDomain (virDomainPtr domain)
{
GET_DOMAIN(domain, -1);
+ CHECK_DOMAIN_JOB_INACTIVE(domain, -1);
if (privdom->info.state == VIR_DOMAIN_SHUTOFF) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not
running");
@@ -1298,6 +1340,7 @@ static int testRebootDomain (virDomainPt
static int testRebootDomain (virDomainPtr domain, virDomainRestart action)
{
GET_DOMAIN(domain, -1);
+ CHECK_DOMAIN_JOB_INACTIVE(domain, -1);
if (!action)
action = VIR_DOMAIN_RESTART;
@@ -1369,6 +1412,7 @@ static int testDomainSave(virDomainPtr d
char *xml;
int fd, len;
GET_DOMAIN(domain, -1);
+ CHECK_DOMAIN_JOB_INACTIVE(domain, -1);
xml = testDomainDumpXMLLocked(domain, privdom, 0);
@@ -1418,6 +1462,69 @@ static int testDomainSave(virDomainPtr d
return 0;
}
+static void testDomainSaveWorker(virJobPtr job, void *data)
+{
+ testJobPtr jobInfo = data;
+ int countdown = 50;
+ virJobInitialize(job, countdown);
+
+ do {
+ sleep(1);
+
+ if (virJobFinishCancel(job))
+ return;
+
+ virJobUpdateRel(job, 1, -1, 2);
+ } while (countdown--);
+
+ if (jobInfo)
+ jobInfo = jobInfo;
+
+ virJobFinishSuccess(job);
+}
+
+static virJobPtr testDomainSaveJob(virDomainPtr domain,
+ const char *path)
+{
+ virJobPtr job;
+ testJobPtr privjob;
+ GET_DOMAIN(domain, NULL);
+ CHECK_DOMAIN_JOB_INACTIVE(domain, NULL);
+
+ privjob = malloc(sizeof(*privjob));
+ if (privjob == NULL) {
+ testError(domain->conn, domain, NULL, VIR_ERR_NO_MEMORY, "job");
+ RELEASE_DOMAIN();
+ return (NULL);
+ }
+ memset(privjob, 0, sizeof(*privjob));
+
+ privjob->type = TEST_JOB_DOMAIN_SAVE;
+ privjob->data.save.domidx = domidx;
+ privjob->data.save.path = strdup(path);
+ if (privjob->data.save.path == NULL) {
+ testError(domain->conn, domain, NULL, VIR_ERR_NO_MEMORY, "job");
+ goto cleanup;
+ }
+
+ job = virJobCreate(domain->conn,
+ domain->conn->driver->jobDriver,
+ testDomainSaveWorker,
+ NULL,
+ privjob,
+ VIR_JOB_BOUNDED);
+
+ RELEASE_DOMAIN();
+
+ return job;
+
+ cleanup:
+ if (privjob->data.save.path) free(privjob->data.save.path);
+ free(privjob);
+ RELEASE_DOMAIN();
+ return (NULL);
+}
+
static int testDomainRestore(virConnectPtr conn,
const char *path)
@@ -1490,6 +1597,7 @@ static int testDomainCoreDump(virDomainP
{
int fd;
GET_DOMAIN(domain, -1);
+ CHECK_DOMAIN_JOB_INACTIVE(domain, -1);
if ((fd = open(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -1680,6 +1788,7 @@ static virDomainPtr testDomainDefineXML(
static int testDomainCreate(virDomainPtr domain) {
GET_DOMAIN(domain, -1);
+ CHECK_DOMAIN_JOB_INACTIVE(domain, -1);
if (privdom->info.state != VIR_DOMAIN_SHUTOFF) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -1697,6 +1806,7 @@ static int testDomainCreate(virDomainPtr
static int testDomainUndefine(virDomainPtr domain) {
GET_DOMAIN(domain, -1);
+ CHECK_DOMAIN_JOB_INACTIVE(domain, -1);
if (privdom->info.state != VIR_DOMAIN_SHUTOFF) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -1979,6 +2089,7 @@ static virNetworkPtr testNetworkDefine(v
static int testNetworkUndefine(virNetworkPtr network) {
GET_NETWORK(network, -1);
+ CHECK_NETWORK_JOB_INACTIVE(network, -1);
if (privnet->running) {
testError(network->conn, NULL, network, VIR_ERR_INTERNAL_ERROR,
@@ -1995,6 +2106,7 @@ static int testNetworkUndefine(virNetwor
static int testNetworkStart(virNetworkPtr network) {
GET_NETWORK(network, -1);
+ CHECK_NETWORK_JOB_INACTIVE(network, -1);
if (privnet->running) {
testError(network->conn, NULL, network, VIR_ERR_INTERNAL_ERROR,
@@ -2011,6 +2123,7 @@ static int testNetworkStart(virNetworkPt
static int testNetworkDestroy(virNetworkPtr network) {
GET_NETWORK(network, -1);
+ CHECK_NETWORK_JOB_INACTIVE(network, -1);
if (privnet->config) {
privnet->running = 0;
@@ -2098,7 +2211,7 @@ static virDriver testDriver = {
VIR_DRV_TEST,
"Test",
LIBVIR_VERSION_NUMBER,
- NULL, /* jobDriver */
+ &defaultJobDriver, /* jobDriver */
testOpen, /* open */
testClose, /* close */
NULL, /* supports_feature */
@@ -2127,7 +2240,7 @@ static virDriver testDriver = {
testSetMemory, /* domainSetMemory */
testGetDomainInfo, /* domainGetInfo */
testDomainSave, /* domainSave */
- NULL, /* domainSaveJob */
+ testDomainSaveJob, /* domainSaveJob */
testDomainRestore, /* domainRestore */
NULL, /* domainRestoreJob */
testDomainCoreDump, /* domainCoreDump */
@@ -2161,7 +2274,7 @@ static virDriver testDriver = {
static virNetworkDriver testNetworkDriver = {
"Test",
- NULL, /* jobDriver */
+ &defaultJobDriver, /* jobDriver */
testOpenNetwork, /* open */
testCloseNetwork, /* close */
testNumNetworks, /* numOfNetworks */
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
5:11 a.m.
New subject: [Libvir] RFC: PATCH 5/5: async job for test driver save API
Daniel P. Berrange wrote:
This patch implements the virDomainSaveJob method for the
test driver as a proof of concept. First of all it adds a
inJob flag to the testDom and testNet objects. This is to
make sure that operations which may change an objects run
state are rejected, if a background job is active.
Then it merely implements the virDomainSaveJob method as
per the previous description. Actually it doesn't really
do any save operation, this impl just illustrates the way
progress updates propagate back to the caller, and the use
of cancellation.
Right, well there we go :-)
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
9:53 p.m.
On Sat, Jan 05, 2008 at 01:06:52AM +0000, Daniel P. Berrange wrote:
We currently have a restriction that virConnectPtr objects
must only be used by a single thread. At the same time we
have a number of operations that can take a very long time.
This means that applications are pretty much forced to use
multi-threading if they want to continue to respond to user
input, and thus are also forced to use multiple virConnectPtr
objects.
This is sub-optimal because opening multiple virConnectPtr
objects may well result in the user being prompted for auth
credentials multiple times. It also doesn't provide any way
of providing progess information on the long running jobs,
nor a way to cancel them.
Thus the following series of patches introduces the concept of
an 'asynchronous background job', represented by a virJobPtr
object in the libvirt public API.
A job has an elapsed time, and may optionally include an
estimated time of completion. If it is a job with a bounded
amount of work, it will also have a percentage completion
counter. There is the ability to request cancellation of an
in progress job, and fetch any error associated with a
completed job.
The primary reason I implemented this support, is that the
storage APIs will make the current situation untennable.
Allocating storage can take a seriously long time (many
many minutes if cloning multi-GB files). This absolutely
requires progress information and an ability to cancel
an operation.
Okay, I understand the need for asynch operations. And I
also agree that an API which forces to use threads in practice
is a really annoying thing and should be avoided. I didn't
really though about solutions for this until now, the 'job'
approach is interesting, it sounds close to what OSes provides
for some asynchronous I/O. I like the idea, the question is
if we will provide any synchronization primitive or if the
time estimate will be sufficient to handle to an application
select/poll main loop.
There are a number of existing APIs which can benefit from
this, hence I decided to work on this separately from the
main storage APIs. The APIs which can make use of this are
virDomainCreateLinux, virDomainCreate, virNetworkCreate,
virNetworkCreateXML, virDomainSave, virDomainRestore, and
virDomainDumpCore. For all of these we add a second variant
postfixed with 'Job' in the name, returning a virJobPtr object
This work isn't finished, but I wanted to put these patches
out for comment before going further. In particular I need
to figure out how to hook this up to the remote driver and
daemon, and deal with a few lifecycle issues. eg what todo
if a virConnectPtr object is released while a background job
is active.
Sounds to me that the creation of a Job should increate a reference
counter for conn, which will be decreased when the Job object is destroyed.
This shouldmap well with Python bindings too.
With all 5 patches applied you should be able to run the
following test case:
virsh --connect test:///default save --verbose test foo
No other drivers / commands are hooked up aside from 'save'
in the 'test' driver.
I will try to provide a finer grained review of your patches
for Tuesday, not really urgent as this is a work in progress, right ?
thanks a lot for looking at it, sounds a good idea,
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
5:12 a.m.
Daniel Veillard wrote:
Sounds to me that the creation of a Job should increate a
reference
counter for conn, which will be decreased when the Job object is destroyed.
This shouldmap well with Python bindings too.
Good point - I'm sure it should do.
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
4:39 a.m.
Daniel P. Berrange wrote:
There are a number of existing APIs which can benefit from
this, hence I decided to work on this separately from the
main storage APIs. The APIs which can make use of this are
virDomainCreateLinux, virDomainCreate, virNetworkCreate,
virNetworkCreateXML, virDomainSave, virDomainRestore, and
virDomainDumpCore. For all of these we add a second variant
postfixed with 'Job' in the name, returning a virJobPtr object
Another way to do this is to overload the domain etc. objects and make
them into suspensions. The advantages being (a) no new API is needed
and (b) some code benefits from overlapping computation even without
being changed. See also my reply here:
http://www.redhat.com/archives/libvir-list/2007-July/msg00257.html
OTOH, suspensions require "interesting" code changes inside libvirt,
although not necessarily requiring threads - there are several possible
implementations.
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
7:59 a.m.
On Mon, Jan 14, 2008 at 10:39:25AM +0000, Richard W.M. Jones wrote:
Daniel P. Berrange wrote:
>There are a number of existing APIs which can benefit from
>this, hence I decided to work on this separately from the
>main storage APIs. The APIs which can make use of this are
>virDomainCreateLinux, virDomainCreate, virNetworkCreate,
>virNetworkCreateXML, virDomainSave, virDomainRestore, and
>virDomainDumpCore. For all of these we add a second variant
>postfixed with 'Job' in the name, returning a virJobPtr object
Another way to do this is to overload the domain etc. objects and make
them into suspensions. The advantages being (a) no new API is needed
and (b) some code benefits from overlapping computation even without
being changed. See also my reply here:
http://www.redhat.com/archives/libvir-list/2007-July/msg00257.html
OTOH, suspensions require "interesting" code changes inside libvirt,
although not necessarily requiring threads - there are several possible
implementations.
That's an interesting idea though i think we'd still need a fair number
of new APIs. Only a handful of existing APIs have a 'flags' param where
we could request async operation, so even if we could keep the same
return type by using a suspension we still need extra args to indicate
sync vs async.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
6147
days inactive
6156
days old
16 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones