11:35 a.m.
We have such wrapper in other drivers too: qemu, lxc, network, ...
Michal Privoznik (4):
struct _parallelsConn: Mark @domains as immutable pointer
parallels: Introduce parallelsDomObjFromDomain()
parallels_driver: Utilize parallelsDomObjFromDomain()
parallels_sdk: Utilize parallelsDomObjFromDomain()
src/parallels/parallels_driver.c | 93 ++++++----------------------------------
src/parallels/parallels_sdk.c | 5 +--
src/parallels/parallels_utils.c | 32 ++++++++++++++
src/parallels/parallels_utils.h | 5 +++
4 files changed, 51 insertions(+), 84 deletions(-)
--
2.0.5
11:35 a.m.
New subject: [libvirt] [PATCH 1/4] struct _parallelsConn: Mark @domains as immutable pointer
The pointer does not change throughout the while life of a
parallels connection. Mark it as such.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/parallels/parallels_utils.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h
index 5731381..4797794 100644
--- a/src/parallels/parallels_utils.h
+++ b/src/parallels/parallels_utils.h
@@ -57,7 +57,10 @@
struct _parallelsConn {
virMutex lock;
+
+ /* Immutable pointer, self-locking APIs */
virDomainObjListPtr domains;
+
PRL_HANDLE server;
PRL_UINT32 jobTimeout;
virStoragePoolObjList pools;
--
2.0.5
11:35 a.m.
New subject: [libvirt] [PATCH 2/4] parallels: Introduce parallelsDomObjFromDomain()
This function is practically copied over from qemu driver. Its
only purpose in life is to lookup a domain object and print an
error if no object is found.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/parallels/parallels_utils.c | 32 ++++++++++++++++++++++++++++++++
src/parallels/parallels_utils.h | 2 ++
2 files changed, 34 insertions(+)
diff --git a/src/parallels/parallels_utils.c b/src/parallels/parallels_utils.c
index 8a3caa4..ff9d47d 100644
--- a/src/parallels/parallels_utils.c
+++ b/src/parallels/parallels_utils.c
@@ -30,9 +30,41 @@
#include "virjson.h"
#include "parallels_utils.h"
#include "virstring.h"
+#include "datatypes.h"
#define VIR_FROM_THIS VIR_FROM_PARALLELS
+/**
+ * parallelsDomObjFromDomain:
+ * @domain: Domain pointer that has to be looked up
+ *
+ * This function looks up @domain and returns the appropriate virDomainObjPtr
+ * that has to be unlocked by virObjectUnlock().
+ *
+ * Returns the domain object without incremented reference counter which is locked
+ * on success, NULL otherwise.
+ */
+virDomainObjPtr
+parallelsDomObjFromDomain(virDomainPtr domain)
+{
+ virDomainObjPtr vm;
+ parallelsConnPtr privconn = domain->conn->privateData;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
+ if (!vm) {
+ virUUIDFormat(domain->uuid, uuidstr);
+ virReportError(VIR_ERR_NO_DOMAIN,
+ _("no domain with matching uuid '%s' (%s)"),
+ uuidstr, domain->name);
+ return NULL;
+ }
+
+ return vm;
+
+}
+
+
static int
parallelsDoCmdRun(char **outbuf, const char *binary, va_list list)
{
diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h
index 4797794..8bbfe85 100644
--- a/src/parallels/parallels_utils.h
+++ b/src/parallels/parallels_utils.h
@@ -91,6 +91,8 @@ virDrvOpenStatus parallelsNetworkOpen(virConnectPtr conn, unsigned int
flags);
int parallelsNetworkClose(virConnectPtr conn);
extern virNetworkDriver parallelsNetworkDriver;
+virDomainObjPtr parallelsDomObjFromDomain(virDomainPtr domain);
+
virJSONValuePtr parallelsParseOutput(const char *binary, ...)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL;
char * parallelsGetOutput(const char *binary, ...)
--
2.0.5
11:35 a.m.
New subject: [libvirt] [PATCH 3/4] parallels_driver: Utilize parallelsDomObjFromDomain()
Instead of each API copying the same lines of code, lets use the
generic function designed just for that purpose. At the same time,
drop useless connection object locking in some functions.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/parallels/parallels_driver.c | 93 ++++++----------------------------------
1 file changed, 13 insertions(+), 80 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 662bb38..07f1311 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -534,18 +534,11 @@ parallelsDomainLookupByName(virConnectPtr conn, const char *name)
static int
parallelsDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
{
- parallelsConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
int ret = -1;
- parallelsDriverLock(privconn);
- privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- parallelsDriverUnlock(privconn);
-
- if (privdom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(privdom = parallelsDomObjFromDomain(domain)))
goto cleanup;
- }
info->state = virDomainObjGetState(privdom, NULL);
info->memory = privdom->def->mem.cur_balloon;
@@ -563,47 +556,35 @@ parallelsDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
static char *
parallelsDomainGetOSType(virDomainPtr domain)
{
- parallelsConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
char *ret = NULL;
- parallelsDriverLock(privconn);
- privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- if (privdom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(privdom = parallelsDomObjFromDomain(domain)))
goto cleanup;
- }
ignore_value(VIR_STRDUP(ret, virDomainOSTypeToString(privdom->def->os.type)));
cleanup:
if (privdom)
virObjectUnlock(privdom);
- parallelsDriverUnlock(privconn);
return ret;
}
static int
parallelsDomainIsPersistent(virDomainPtr domain)
{
- parallelsConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
int ret = -1;
- parallelsDriverLock(privconn);
- privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- if (privdom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(privdom = parallelsDomObjFromDomain(domain)))
goto cleanup;
- }
ret = 1;
cleanup:
if (privdom)
virObjectUnlock(privdom);
- parallelsDriverUnlock(privconn);
return ret;
}
@@ -611,19 +592,12 @@ static int
parallelsDomainGetState(virDomainPtr domain,
int *state, int *reason, unsigned int flags)
{
- parallelsConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
int ret = -1;
virCheckFlags(0, -1);
- parallelsDriverLock(privconn);
- privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- parallelsDriverUnlock(privconn);
-
- if (privdom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(privdom = parallelsDomObjFromDomain(domain)))
goto cleanup;
- }
*state = virDomainObjGetState(privdom, reason);
ret = 0;
@@ -637,21 +611,14 @@ parallelsDomainGetState(virDomainPtr domain,
static char *
parallelsDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
{
- parallelsConnPtr privconn = domain->conn->privateData;
virDomainDefPtr def;
virDomainObjPtr privdom;
char *ret = NULL;
/* Flags checked by virDomainDefFormat */
- parallelsDriverLock(privconn);
- privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- parallelsDriverUnlock(privconn);
-
- if (privdom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(privdom = parallelsDomObjFromDomain(domain)))
goto cleanup;
- }
def = (flags & VIR_DOMAIN_XML_INACTIVE) &&
privdom->newDef ? privdom->newDef : privdom->def;
@@ -667,18 +634,11 @@ parallelsDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
static int
parallelsDomainGetAutostart(virDomainPtr domain, int *autostart)
{
- parallelsConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom;
int ret = -1;
- parallelsDriverLock(privconn);
- privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- parallelsDriverUnlock(privconn);
-
- if (privdom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(privdom = parallelsDomObjFromDomain(domain)))
goto cleanup;
- }
*autostart = privdom->autostart;
ret = 0;
@@ -822,20 +782,13 @@ parallelsDomainGetVcpus(virDomainPtr domain,
unsigned char *cpumaps,
int maplen)
{
- parallelsConnPtr privconn = domain->conn->privateData;
virDomainObjPtr privdom = NULL;
size_t i;
int v, maxcpu, hostcpus;
int ret = -1;
- parallelsDriverLock(privconn);
- privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- parallelsDriverUnlock(privconn);
-
- if (privdom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(privdom = parallelsDomObjFromDomain(domain)))
goto cleanup;
- }
if (!virDomainObjIsActive(privdom)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -956,15 +909,11 @@ static int parallelsDomainShutdown(virDomainPtr domain)
static int parallelsDomainIsActive(virDomainPtr domain)
{
- parallelsConnPtr privconn = domain->conn->privateData;
virDomainObjPtr dom = NULL;
int ret = -1;
- dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- if (dom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(dom = parallelsDomObjFromDomain(domain)))
return -1;
- }
ret = virDomainObjIsActive(dom);
virObjectUnlock(dom);
@@ -991,11 +940,8 @@ parallelsDomainUndefineFlags(virDomainPtr domain,
virCheckFlags(0, -1);
- dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- if (dom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(dom = parallelsDomObjFromDomain(domain)))
return -1;
- }
ret = prlsdkUnregisterDomain(privconn, dom);
if (ret)
@@ -1013,18 +959,14 @@ parallelsDomainUndefine(virDomainPtr domain)
static int
parallelsDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags)
{
- parallelsConnPtr privconn = domain->conn->privateData;
virDomainObjPtr dom = NULL;
int state, reason;
int ret = 0;
virCheckFlags(0, -1);
- dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- if (dom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(dom = parallelsDomObjFromDomain(domain)))
return -1;
- }
state = virDomainObjGetState(dom, &reason);
if (state == VIR_DOMAIN_SHUTOFF && reason == VIR_DOMAIN_SHUTOFF_SAVED)
@@ -1045,11 +987,8 @@ parallelsDomainManagedSave(virDomainPtr domain, unsigned int flags)
virCheckFlags(VIR_DOMAIN_SAVE_RUNNING |
VIR_DOMAIN_SAVE_PAUSED, -1);
- dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- if (dom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(dom = parallelsDomObjFromDomain(domain)))
return -1;
- }
state = virDomainObjGetState(dom, &reason);
@@ -1076,11 +1015,8 @@ parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int
flags)
virCheckFlags(0, -1);
- dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- if (dom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(dom = parallelsDomObjFromDomain(domain)))
return -1;
- }
state = virDomainObjGetState(dom, &reason);
@@ -1106,11 +1042,8 @@ static int parallelsDomainAttachDeviceFlags(virDomainPtr dom, const
char *xml,
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
- privdom = virDomainObjListFindByUUID(privconn->domains, dom->uuid);
- if (privdom == NULL) {
- parallelsDomNotFoundError(dom);
+ if (!(privdom = parallelsDomObjFromDomain(dom)))
return -1;
- }
if (!(flags & VIR_DOMAIN_AFFECT_CONFIG)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
--
2.0.5
11:35 a.m.
New subject: [libvirt] [PATCH 4/4] parallels_sdk: Utilize parallelsDomObjFromDomain()
Instead of each API copying the same lines of code, lets use the
generic function designed just for that purpose.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/parallels/parallels_sdk.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 59ca62e..d54f894 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1838,11 +1838,8 @@ prlsdkDomainChangeState(virDomainPtr domain,
virDomainObjPtr dom;
int ret = -1;
- dom = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
- if (dom == NULL) {
- parallelsDomNotFoundError(domain);
+ if (!(dom = parallelsDomObjFromDomain(domain)))
return -1;
- }
ret = prlsdkDomainChangeStateLocked(privconn, dom, chstate);
virObjectUnlock(dom);
--
2.0.5
4:33 a.m.
22.04.2015 19:35, Michal Privoznik пишет:
We have such wrapper in other drivers too: qemu, lxc, network, ...
Michal Privoznik (4):
struct _parallelsConn: Mark @domains as immutable pointer
parallels: Introduce parallelsDomObjFromDomain()
parallels_driver: Utilize parallelsDomObjFromDomain()
parallels_sdk: Utilize parallelsDomObjFromDomain()
src/parallels/parallels_driver.c | 93 ++++++----------------------------------
src/parallels/parallels_sdk.c | 5 +--
src/parallels/parallels_utils.c | 32 ++++++++++++++
src/parallels/parallels_utils.h | 5 +++
4 files changed, 51 insertions(+), 84 deletions(-)
The whole series looks OK. Also I checked these patches on my working
environment - it works OK.
7:48 a.m.
On 23.04.2015 11:33, Maxim Nestratov wrote:
22.04.2015 19:35, Michal Privoznik пишет:
> We have such wrapper in other drivers too: qemu, lxc, network, ...
>
> Michal Privoznik (4):
> struct _parallelsConn: Mark @domains as immutable pointer
> parallels: Introduce parallelsDomObjFromDomain()
> parallels_driver: Utilize parallelsDomObjFromDomain()
> parallels_sdk: Utilize parallelsDomObjFromDomain()
>
> src/parallels/parallels_driver.c | 93
> ++++++----------------------------------
> src/parallels/parallels_sdk.c | 5 +--
> src/parallels/parallels_utils.c | 32 ++++++++++++++
> src/parallels/parallels_utils.h | 5 +++
> 4 files changed, 51 insertions(+), 84 deletions(-)
>
The whole series looks OK. Also I checked these patches on my working
environment - it works OK.
Okay, I take it as ACK :)
I've pushed these. Thanks!
Michal
7:48 a.m.
On 04/22/2015 07:35 PM, Michal Privoznik wrote:
We have such wrapper in other drivers too: qemu, lxc, network, ...
Michal Privoznik (4):
struct _parallelsConn: Mark @domains as immutable pointer
parallels: Introduce parallelsDomObjFromDomain()
parallels_driver: Utilize parallelsDomObjFromDomain()
parallels_sdk: Utilize parallelsDomObjFromDomain()
src/parallels/parallels_driver.c | 93 ++++++----------------------------------
src/parallels/parallels_sdk.c | 5 +--
src/parallels/parallels_utils.c | 32 ++++++++++++++
src/parallels/parallels_utils.h | 5 +++
4 files changed, 51 insertions(+), 84 deletions(-)
ACK, but somebody has already pushed this series :)
--
Dmitry Guryanov
3501
days inactive
3502
days old
7 comments
3 participants
participants (3)
-
Dmitry Guryanov -
Maxim Nestratov -
Michal Privoznik