12.04.2016 17:04, Nikolay Shirokovskiy пишет:

On 07.04.2016 23:09, Maxim Nestratov wrote:

...

--- src/vz/vz_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 9de88cd..f2bbf1e 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1491,7 +1491,7 @@ vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb) return ret; }

-static virHypervisorDriver vzDriver = { +static virHypervisorDriver vzHypervisorDriver = { .name = "vz", .connectOpen = vzConnectOpen, /* 0.10.0 */ .connectClose = vzConnectClose, /* 0.10.0 */ @@ -1558,11 +1558,11 @@ static virHypervisorDriver vzDriver = { };

static virConnectDriver vzConnectDriver = { - .hypervisorDriver = &vzDriver, + .hypervisorDriver = &vzHypervisorDriver, };

/* Parallels domain type backward compatibility*/ -static virHypervisorDriver parallelsDriver; +static virHypervisorDriver parallelsHypervisorDriver; static virConnectDriver parallelsConnectDriver;

/** @@ -1584,10 +1584,10 @@ vzRegister(void) VIR_FREE(prlctl_path);

/* Backward compatibility with Parallels domain type */ - parallelsDriver = vzDriver; - parallelsDriver.name = "Parallels"; + parallelsHypervisorDriver = vzHypervisorDriver; + parallelsHypervisorDriver.name = "Parallels"; parallelsConnectDriver = vzConnectDriver; - parallelsConnectDriver.hypervisorDriver = ¶llelsDriver; + parallelsConnectDriver.hypervisorDriver = ¶llelsHypervisorDriver; if (virRegisterConnectDriver(¶llelsConnectDriver, false) < 0) return -1;

ACK

Thanks. Pushed 1-4 of the series.

12.04.2016 14:11, Nikolay Shirokovskiy пишет:

On 07.04.2016 23:09, Maxim Nestratov wrote:

...

This patch introduces a new 'vzDriver' lockable object and provides helper functions to allocate/destroy it and we pass it to prlsdkXxx functions instead of virConnectPtr. Now we store domain related objects such as domain list, capabitilies etc. within a single vz_driver vzDriver structure, which is shared by all driver connections. It is allocated during daemon initialization or in a lazy manner when a new connection to 'vz' driver is established. When a connection to vz daemon drops, vzDestroyConnection is called, which in turn relays disconnect event to all connection to 'vz' driver.

Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 339 +++++++++++++++++++++++++++++++++++------------------ src/vz/vz_sdk.c | 211 ++++++++++++++++----------------- src/vz/vz_sdk.h | 30 ++--- src/vz/vz_utils.c | 27 +++-- src/vz/vz_utils.h | 29 ++++- 5 files changed, 380 insertions(+), 256 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index f2bbf1e..e9fe89f 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -63,18 +63,25 @@ VIR_LOG_INIT("parallels.parallels_driver"); #define PRLCTL "prlctl"

static int vzConnectClose(virConnectPtr conn); +static virClassPtr vzDriverConnClass; why 'conn' suffix? i suggest just vzDriverClass

fixed

...

void -vzDriverLock(vzConnPtr driver) +vzDriverLock(vzConnPtr privconn) { - virMutexLock(&driver->lock); + virObjectLock(privconn->driver); }

void -vzDriverUnlock(vzConnPtr driver) +vzDriverUnlock(vzConnPtr privconn) { - virMutexUnlock(&driver->lock); + virObjectUnlock(privconn->driver); }

empty line here will be good

agree

...

+static virMutex vz_driver_lock; +static vzDriverPtr vz_driver; +static vzConnPtr vz_conn_list; i would move this list into driver, i even think if we could write version of close callback object that can take multiple callbacks, thus we can get rid of vz_conn_list and vzConn altogether.

not sure about moving as for a new version of close callback - certainly not in this patch series

...

+ +static vzDriverPtr +vzDriverObjNew(void);

static int vzCapsAddGuestDomain(virCapsPtr caps, @@ -158,6 +165,69 @@ vzBuildCapabilities(void) goto cleanup; }

+static void vzDriverDispose(void * obj) +{ + vzDriverPtr conn = obj; looks like 'driver' will be better here

agree

...

+ + if (conn->server) { + prlsdkUnsubscribeFromPCSEvents(conn); + prlsdkDisconnect(conn); + } + + virObjectUnref(conn->domains); + virObjectUnref(conn->caps); + virObjectUnref(conn->xmlopt); + virObjectEventStateFree(conn->domainEventState); +} + +static int vzDriverOnceInit(void) +{ + if (!(vzDriverConnClass = virClassNew(virClassForObjectLockable(), + "vzDriver", + sizeof(vzDriver), + vzDriverDispose))) indentation

ahh, sure

...

+ return -1; + + return 0; +} +VIR_ONCE_GLOBAL_INIT(vzDriver) + +vzDriverPtr +vzGetDriverConnection(void) +{ + virMutexLock(&vz_driver_lock); + if (!vz_driver) + vz_driver = vzDriverObjNew(); + virObjectRef(vz_driver); + virMutexUnlock(&vz_driver_lock); + return vz_driver; +} i would put more empty lines here,

not sure, but if you insist...

and since this func is pretty simple i suggest open code it in that new function

didn't get you, sorry

...

+ +void +vzDestroyDriverConnection(void) +{ + + vzDriverPtr driver; + vzConnPtr privconn_list; + + virMutexLock(&vz_driver_lock); + driver = vz_driver; + vz_driver = NULL; + + privconn_list = vz_conn_list; + vz_conn_list = NULL; + + virMutexUnlock(&vz_driver_lock); + + while (privconn_list) { + vzConnPtr privconn = privconn_list; + privconn_list = privconn->next; + virConnectCloseCallbackDataCall(privconn->closeCallback, + VIR_CONNECT_CLOSE_REASON_EOF); + } + virObjectUnref(driver); +} + static char * vzConnectGetCapabilities(virConnectPtr conn) { @@ -165,7 +235,7 @@ vzConnectGetCapabilities(virConnectPtr conn) char *xml;

vzDriverLock(privconn); - xml = virCapabilitiesFormatXML(privconn->caps); + xml = virCapabilitiesFormatXML(privconn->driver->caps); vzDriverUnlock(privconn); return xml; } @@ -214,70 +284,34 @@ virDomainDefParserConfig vzDomainDefParserConfig = { .domainPostParseCallback = vzDomainDefPostParse, };

- -static int -vzOpenDefault(virConnectPtr conn) +static vzDriverPtr +vzDriverObjNew(void) { - vzConnPtr privconn; - - if (VIR_ALLOC(privconn) < 0) - return VIR_DRV_OPEN_ERROR; - if (virMutexInit(&privconn->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize mutex")); - goto err_free; - } + vzDriverPtr conn; i think 'driver' will be more comprehensible

sure, just forgot to change it since last version

...

- if (prlsdkInit()) { - VIR_DEBUG("%s", _("Can't initialize Parallels SDK")); - goto err_free; - } - - if (prlsdkConnect(privconn) < 0) - goto err_free; - - if (vzInitVersion(privconn) < 0) - goto error; - - if (!(privconn->caps = vzBuildCapabilities())) - goto error; - - vzDomainDefParserConfig.priv = &privconn->vzCaps; - if (!(privconn->xmlopt = virDomainXMLOptionNew(&vzDomainDefParserConfig, - NULL, NULL))) - goto error; - - if (!(privconn->domains = virDomainObjListNew())) - goto error; - - if (!(privconn->domainEventState = virObjectEventStateNew())) - goto error; - - if (prlsdkSubscribeToPCSEvents(privconn)) - goto error; - - if (!(privconn->closeCallback = virNewConnectCloseCallbackData())) - goto error; - - conn->privateData = privconn; + if (vzDriverInitialize() < 0) + return NULL;

- if (prlsdkLoadDomains(privconn)) - goto error; + if (!(conn = virObjectLockableNew(vzDriverConnClass))) + return NULL;

- return VIR_DRV_OPEN_SUCCESS; + vzDomainDefParserConfig.priv = &conn->vzCaps; + + if (!(conn->caps = vzBuildCapabilities()) || + !(conn->xmlopt = virDomainXMLOptionNew(&vzDomainDefParserConfig, + NULL, NULL)) || + !(conn->domains = virDomainObjListNew()) || + !(conn->domainEventState = virObjectEventStateNew()) || + (vzInitVersion(conn) < 0) || + (prlsdkConnect(conn) < 0) || + (prlsdkSubscribeToPCSEvents(conn) < 0) + ) { + virObjectUnref(conn); + return NULL; + }

- error: - virObjectUnref(privconn->closeCallback); - privconn->closeCallback = NULL; - virObjectUnref(privconn->domains); - virObjectUnref(privconn->caps); - virObjectEventStateFree(privconn->domainEventState); - prlsdkDisconnect(privconn); - prlsdkDeinit(); - err_free: - conn->privateData = NULL; - VIR_FREE(privconn); - return VIR_DRV_OPEN_ERROR; + prlsdkLoadDomains(conn);

failure should be checked

or just ignored since I intentionally did this but forgot to put it in ignore_value()

...

+ return conn; }

static virDrvOpenStatus @@ -285,7 +319,8 @@ vzConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { - int ret; + vzDriverPtr driver = NULL; + vzConnPtr privconn = NULL;

virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);

@@ -317,36 +352,60 @@ vzConnectOpen(virConnectPtr conn, return VIR_DRV_OPEN_ERROR; }

- if ((ret = vzOpenDefault(conn)) != VIR_DRV_OPEN_SUCCESS) - return ret; + if (!(driver = vzGetDriverConnection())) + return VIR_DRV_OPEN_ERROR; + + if (VIR_ALLOC(privconn) < 0) { + + virObjectUnref(driver); + return VIR_DRV_OPEN_ERROR; + } you can just goto error here

ok, seems to be shorter...

...

+ + conn->privateData = privconn; + privconn->driver = driver; + + if (!(privconn->closeCallback = virNewConnectCloseCallbackData())) + goto error; + + virMutexLock(&vz_driver_lock); + privconn->next = vz_conn_list; + vz_conn_list = privconn; + virMutexUnlock(&vz_driver_lock);

return VIR_DRV_OPEN_SUCCESS; + + error: + + conn->privateData = NULL; + virObjectUnref(driver); + VIR_FREE(privconn); + return VIR_DRV_OPEN_ERROR; }

static int vzConnectClose(virConnectPtr conn) { + vzConnPtr curr, prev = NULL; vzConnPtr privconn = conn->privateData;

if (!privconn) return 0;

- vzDriverLock(privconn); - prlsdkUnsubscribeFromPCSEvents(privconn); - virObjectUnref(privconn->caps); - virObjectUnref(privconn->xmlopt); - virObjectUnref(privconn->domains); - virObjectUnref(privconn->closeCallback); - privconn->closeCallback = NULL; - virObjectEventStateFree(privconn->domainEventState); - prlsdkDisconnect(privconn); - conn->privateData = NULL; - prlsdkDeinit(); - - vzDriverUnlock(privconn); - virMutexDestroy(&privconn->lock); + virMutexLock(&vz_driver_lock); + for (curr = vz_conn_list; curr; prev = curr, curr = curr->next) + if (curr == privconn) { + if (prev) + prev->next = curr->next; + else + vz_conn_list = curr->next; + break; + } i'd suggest use braces here for 'for' too, BTW you can reduce nesting

ok, makes sense

...

+ virMutexUnlock(&vz_driver_lock);

+ virObjectUnref(privconn->closeCallback); + virObjectUnref(privconn->driver); VIR_FREE(privconn); + conn->privateData = NULL; return 0; }

@@ -354,7 +413,7 @@ static int vzConnectGetVersion(virConnectPtr conn, unsigned long *hvVer) { vzConnPtr privconn = conn->privateData; - *hvVer = privconn->vzVersion; + *hvVer = privconn->driver->vzVersion; return 0; }

@@ -372,7 +431,7 @@ vzConnectListDomains(virConnectPtr conn, int *ids, int maxids) int n;

vzDriverLock(privconn); - n = virDomainObjListGetActiveIDs(privconn->domains, ids, maxids, + n = virDomainObjListGetActiveIDs(privconn->driver->domains, ids, maxids, NULL, NULL); vzDriverUnlock(privconn);

@@ -386,7 +445,7 @@ vzConnectNumOfDomains(virConnectPtr conn) int count;

vzDriverLock(privconn); - count = virDomainObjListNumOfDomains(privconn->domains, true, + count = virDomainObjListNumOfDomains(privconn->driver->domains, true, NULL, NULL); vzDriverUnlock(privconn);

@@ -394,14 +453,16 @@ vzConnectNumOfDomains(virConnectPtr conn) }

static int -vzConnectListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) +vzConnectListDefinedDomains(virConnectPtr conn, + char **const names, + int maxnames) unrelated

ok

...

{ vzConnPtr privconn = conn->privateData; int n;

vzDriverLock(privconn); memset(names, 0, sizeof(*names) * maxnames); - n = virDomainObjListGetInactiveNames(privconn->domains, names, + n = virDomainObjListGetInactiveNames(privconn->driver->domains, names, maxnames, NULL, NULL); vzDriverUnlock(privconn);

@@ -415,7 +476,7 @@ vzConnectNumOfDefinedDomains(virConnectPtr conn) int count;

vzDriverLock(privconn); - count = virDomainObjListNumOfDomains(privconn->domains, false, + count = virDomainObjListNumOfDomains(privconn->driver->domains, false, NULL, NULL); vzDriverUnlock(privconn);

@@ -432,7 +493,7 @@ vzConnectListAllDomains(virConnectPtr conn,

virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1); vzDriverLock(privconn); - ret = virDomainObjListExport(privconn->domains, conn, domains, + ret = virDomainObjListExport(privconn->driver->domains, conn, domains, NULL, flags); vzDriverUnlock(privconn);

@@ -447,7 +508,7 @@ vzDomainLookupByID(virConnectPtr conn, int id) virDomainObjPtr dom;

vzDriverLock(privconn); - dom = virDomainObjListFindByID(privconn->domains, id); + dom = virDomainObjListFindByID(privconn->driver->domains, id); vzDriverUnlock(privconn);

if (dom == NULL) { @@ -473,7 +534,8 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) virDomainObjPtr dom;

vzDriverLock(privconn); - dom = virDomainObjListFindByUUID(privconn->domains, uuid); + + dom = virDomainObjListFindByUUID(privconn->driver->domains, uuid); vzDriverUnlock(privconn);

if (dom == NULL) { @@ -502,7 +564,7 @@ vzDomainLookupByName(virConnectPtr conn, const char *name) virDomainObjPtr dom;

vzDriverLock(privconn); - dom = virDomainObjListFindByName(privconn->domains, name); + dom = virDomainObjListFindByName(privconn->driver->domains, name); vzDriverUnlock(privconn);

if (dom == NULL) { @@ -626,7 +688,7 @@ vzDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) def = (flags & VIR_DOMAIN_XML_INACTIVE) && privdom->newDef ? privdom->newDef : privdom->def;

- ret = virDomainDefFormat(def, privconn->caps, flags); + ret = virDomainDefFormat(def, privconn->driver->caps, flags);

cleanup: if (privdom) @@ -661,28 +723,29 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virDomainObjPtr olddom = NULL; virDomainObjPtr newdom = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + vzDriverPtr driver = privconn->driver;

virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL);

if (flags & VIR_DOMAIN_DEFINE_VALIDATE) parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE;

- vzDriverLock(privconn); - if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, + virObjectLock(driver); + if ((def = virDomainDefParseString(xml, driver->caps, driver->xmlopt, parse_flags)) == NULL) goto cleanup;

- olddom = virDomainObjListFindByUUID(privconn->domains, def->uuid); + olddom = virDomainObjListFindByUUID(driver->domains, def->uuid); if (olddom == NULL) { virResetLastError(); - newdom = vzNewDomain(privconn, def->name, def->uuid); + newdom = vzNewDomain(driver, def->name, def->uuid); if (!newdom) goto cleanup; if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { - if (prlsdkCreateVm(conn, def)) + if (prlsdkCreateVm(driver, def)) goto cleanup; } else if (def->os.type == VIR_DOMAIN_OSTYPE_EXE) { - if (prlsdkCreateCt(conn, def)) + if (prlsdkCreateCt(driver, def)) goto cleanup; } else { virReportError(VIR_ERR_INVALID_ARG, @@ -691,7 +754,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; }

- if (prlsdkLoadDomain(privconn, newdom)) + if (prlsdkLoadDomain(driver, newdom)) goto cleanup; } else { int state, reason; @@ -717,10 +780,10 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; } } else { - if (prlsdkApplyConfig(conn, olddom, def)) + if (prlsdkApplyConfig(driver, olddom, def)) goto cleanup;

- if (prlsdkUpdateDomain(privconn, olddom)) + if (prlsdkUpdateDomain(driver, olddom)) goto cleanup; } } @@ -734,12 +797,12 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virObjectUnlock(olddom); if (newdom) { if (!retdom) - virDomainObjListRemove(privconn->domains, newdom); + virDomainObjListRemove(driver->domains, newdom); else virObjectUnlock(newdom); } virDomainDefFree(def); - vzDriverUnlock(privconn); + virObjectUnlock(driver); return retdom; }

@@ -855,7 +918,7 @@ vzConnectDomainEventRegisterAny(virConnectPtr conn, int ret = -1; vzConnPtr privconn = conn->privateData; if (virDomainEventStateRegisterID(conn, - privconn->domainEventState, + privconn->driver->domainEventState, domain, eventID, callback, opaque, freecb, &ret) < 0) ret = -1; @@ -870,7 +933,7 @@ vzConnectDomainEventDeregisterAny(virConnectPtr conn, int ret = -1;

if (virObjectEventStateDeregisterID(conn, - privconn->domainEventState, + privconn->driver->domainEventState, callbackID) < 0) goto cleanup;

@@ -949,7 +1012,7 @@ vzDomainUndefineFlags(virDomainPtr domain, if (!(dom = vzDomObjFromDomain(domain))) return -1;

- ret = prlsdkUnregisterDomain(privconn, dom, flags); + ret = prlsdkUnregisterDomain(privconn->driver, dom, flags); if (ret) virObjectUnlock(dom);

@@ -999,12 +1062,12 @@ vzDomainManagedSave(virDomainPtr domain, unsigned int flags) state = virDomainObjGetState(dom, &reason);

if (state == VIR_DOMAIN_RUNNING && (flags & VIR_DOMAIN_SAVE_PAUSED)) { - ret = prlsdkDomainChangeStateLocked(privconn, dom, prlsdkPause); + ret = prlsdkDomainChangeStateLocked(privconn->driver, dom, prlsdkPause); if (ret) goto cleanup; }

- ret = prlsdkDomainChangeStateLocked(privconn, dom, prlsdkSuspend); + ret = prlsdkDomainChangeStateLocked(privconn->driver, dom, prlsdkSuspend);

cleanup: virObjectUnlock(dom); @@ -1070,14 +1133,14 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, "VIR_DOMAIN_AFFECT_LIVE flag")); }

- dev = virDomainDeviceDefParse(xml, privdom->def, privconn->caps, - privconn->xmlopt, VIR_DOMAIN_XML_INACTIVE); + dev = virDomainDeviceDefParse(xml, privdom->def, privconn->driver->caps, + privconn->driver->xmlopt, VIR_DOMAIN_XML_INACTIVE); if (dev == NULL) goto cleanup;

switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: - ret = prlsdkAttachVolume(privconn, privdom, dev->data.disk); + ret = prlsdkAttachVolume(privconn->driver, privdom, dev->data.disk); if (ret) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("disk attach failed")); @@ -1085,7 +1148,7 @@ static int vzDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, } break; case VIR_DOMAIN_DEVICE_NET: - ret = prlsdkAttachNet(privconn, privdom, dev->data.net); + ret = prlsdkAttachNet(privconn->driver, privdom, dev->data.net); if (ret) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("network attach failed")); @@ -1147,8 +1210,8 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, "VIR_DOMAIN_AFFECT_LIVE flag")); }

- dev = virDomainDeviceDefParse(xml, privdom->def, privconn->caps, - privconn->xmlopt, VIR_DOMAIN_XML_INACTIVE); + dev = virDomainDeviceDefParse(xml, privdom->def, privconn->driver->caps, + privconn->driver->xmlopt, VIR_DOMAIN_XML_INACTIVE); if (dev == NULL) goto cleanup; looks like it's worth introducing driver var here

...

@@ -1162,7 +1225,7 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, } break; case VIR_DOMAIN_DEVICE_NET: - ret = prlsdkDetachNet(privconn, privdom, dev->data.net); + ret = prlsdkDetachNet(privconn->driver, privdom, dev->data.net); if (ret) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("network detach failed")); @@ -1451,7 +1514,6 @@ vzConnectRegisterCloseCallback(virConnectPtr conn, int ret = -1;

vzDriverLock(privconn); -

unrelated

ok

...

if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != NULL) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("A close callback is already registered")); @@ -1561,6 +1623,48 @@ static virConnectDriver vzConnectDriver = { .hypervisorDriver = &vzHypervisorDriver, };

+static int +vzStateCleanup(void) +{ + prlsdkDeinit();

i think this should go after unreferencing driver which in turn destroy driver object and call some sdk cleanup logic

indeed

...

+ virObjectUnref(vz_driver); + vz_driver = NULL; + virMutexDestroy(&vz_driver_lock); + return 0; +} + +static int +vzStateInitialize(bool privileged ATTRIBUTE_UNUSED, + virStateInhibitCallback callback ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (!privileged) { + VIR_INFO("Not running privileged, disabling driver"); + return 0; + } why is this success?

just not to prevent daemon from starting

...

+ + if (prlsdkInit() < 0) { + VIR_DEBUG("%s", _("Can't initialize Parallels SDK")); look like you need to report error here

I just followed the rule other state drivers stick to

...

+ return -1; + } + + if (virMutexInit(&vz_driver_lock) < 0) and here

the same

...

+ goto error; + + vz_driver = vzDriverObjNew(); looks like it is not a problem if we get NULL as we are lazy, i think this should be commented

ok

...

+ return 0; + + error: + vzStateCleanup(); + return -1; +} + +static virStateDriver vzStateDriver = { + .name = "vz", + .stateInitialize = vzStateInitialize, + .stateCleanup = vzStateCleanup, +}; + /* Parallels domain type backward compatibility*/ static virHypervisorDriver parallelsHypervisorDriver; static virConnectDriver parallelsConnectDriver; @@ -1594,5 +1698,8 @@ vzRegister(void) if (virRegisterConnectDriver(&vzConnectDriver, false) < 0) return -1;

+ if (virRegisterStateDriver(&vzStateDriver) < 0) + return -1; + return 0; } diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index c006517..9e369ce 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -234,22 +234,22 @@ prlsdkDeinit(void) };

int -prlsdkConnect(vzConnPtr privconn) +prlsdkConnect(vzDriverPtr driver) { PRL_RESULT ret; PRL_HANDLE job = PRL_INVALID_HANDLE;

- ret = PrlSrv_Create(&privconn->server); + ret = PrlSrv_Create(&driver->server); if (PRL_FAILED(ret)) { logPrlError(ret); return -1; }

- job = PrlSrv_LoginLocalEx(privconn->server, NULL, 0, + job = PrlSrv_LoginLocalEx(driver->server, NULL, 0, PSL_HIGH_SECURITY, PACF_NON_INTERACTIVE_MODE);

if (waitJob(job)) { - PrlHandle_Free(privconn->server); + PrlHandle_Free(driver->server); return -1; }

@@ -257,18 +257,18 @@ prlsdkConnect(vzConnPtr privconn) }

void -prlsdkDisconnect(vzConnPtr privconn) +prlsdkDisconnect(vzDriverPtr driver) { PRL_HANDLE job;

- job = PrlSrv_Logoff(privconn->server); + job = PrlSrv_Logoff(driver->server); waitJob(job);

- PrlHandle_Free(privconn->server); + PrlHandle_Free(driver->server); }

static int -prlsdkSdkDomainLookup(vzConnPtr privconn, +prlsdkSdkDomainLookup(vzDriverPtr driver, const char *id, unsigned int flags, PRL_HANDLE *sdkdom) @@ -278,7 +278,7 @@ prlsdkSdkDomainLookup(vzConnPtr privconn, PRL_RESULT pret = PRL_ERR_UNINITIALIZED; int ret = -1;

- job = PrlSrv_GetVmConfig(privconn->server, id, flags); + job = PrlSrv_GetVmConfig(driver->server, id, flags); if (PRL_FAILED(getJobResult(job, &result))) goto cleanup;

@@ -303,14 +303,14 @@ prlsdkUUIDFormat(const unsigned char *uuid, char *uuidstr) }

static PRL_HANDLE -prlsdkSdkDomainLookupByUUID(vzConnPtr privconn, const unsigned char *uuid) +prlsdkSdkDomainLookupByUUID(vzDriverPtr driver, const unsigned char *uuid) { char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;

prlsdkUUIDFormat(uuid, uuidstr);

- if (prlsdkSdkDomainLookup(privconn, uuidstr, + if (prlsdkSdkDomainLookup(driver, uuidstr, PGVC_SEARCH_BY_UUID, &sdkdom) < 0) { virUUIDFormat(uuid, uuidstr); virReportError(VIR_ERR_NO_DOMAIN, @@ -466,7 +466,7 @@ prlsdkAddDomainVideoInfo(PRL_HANDLE sdkdom, virDomainDefPtr def) }

static int -prlsdkGetDiskInfo(vzConnPtr privconn, +prlsdkGetDiskInfo(vzDriverPtr driver, PRL_HANDLE prldisk, virDomainDiskDefPtr disk, bool isCdrom, @@ -489,9 +489,9 @@ prlsdkGetDiskInfo(vzConnPtr privconn, virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); } else { if (isCt) - virDomainDiskSetFormat(disk, privconn->vzCaps.ctDiskFormat); + virDomainDiskSetFormat(disk, driver->vzCaps.ctDiskFormat); else - virDomainDiskSetFormat(disk, privconn->vzCaps.vmDiskFormat); + virDomainDiskSetFormat(disk, driver->vzCaps.vmDiskFormat); } } else { virDomainDiskSetType(disk, VIR_STORAGE_TYPE_BLOCK); @@ -620,7 +620,7 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk, }

static int -prlsdkAddDomainHardDisksInfo(vzConnPtr privconn, PRL_HANDLE sdkdom, virDomainDefPtr def) +prlsdkAddDomainHardDisksInfo(vzDriverPtr driver, PRL_HANDLE sdkdom, virDomainDefPtr def) { PRL_RESULT pret; PRL_UINT32 hddCount; @@ -660,7 +660,7 @@ prlsdkAddDomainHardDisksInfo(vzConnPtr privconn, PRL_HANDLE sdkdom, virDomainDef if (!(disk = virDomainDiskDefNew(NULL))) goto error;

- if (prlsdkGetDiskInfo(privconn, hdd, disk, false, IS_CT(def)) < 0) + if (prlsdkGetDiskInfo(driver, hdd, disk, false, IS_CT(def)) < 0) goto error;

if (virDomainDiskInsert(def, disk) < 0) @@ -682,7 +682,7 @@ prlsdkAddDomainHardDisksInfo(vzConnPtr privconn, PRL_HANDLE sdkdom, virDomainDef }

static int -prlsdkAddDomainOpticalDisksInfo(vzConnPtr privconn, PRL_HANDLE sdkdom, virDomainDefPtr def) +prlsdkAddDomainOpticalDisksInfo(vzDriverPtr driver, PRL_HANDLE sdkdom, virDomainDefPtr def) { PRL_RESULT pret; PRL_UINT32 cdromsCount; @@ -700,7 +700,7 @@ prlsdkAddDomainOpticalDisksInfo(vzConnPtr privconn, PRL_HANDLE sdkdom, virDomain if (!(disk = virDomainDiskDefNew(NULL))) goto error;

- if (prlsdkGetDiskInfo(privconn, cdrom, disk, true, IS_CT(def)) < 0) + if (prlsdkGetDiskInfo(driver, cdrom, disk, true, IS_CT(def)) < 0) goto error;

PrlHandle_Free(cdrom); @@ -963,16 +963,16 @@ prlsdkAddSerialInfo(PRL_HANDLE sdkdom,

static int -prlsdkAddDomainHardware(vzConnPtr privconn, PRL_HANDLE sdkdom, virDomainDefPtr def) +prlsdkAddDomainHardware(vzDriverPtr driver, PRL_HANDLE sdkdom, virDomainDefPtr def) { if (!IS_CT(def)) if (prlsdkAddDomainVideoInfo(sdkdom, def) < 0) goto error;

- if (prlsdkAddDomainHardDisksInfo(privconn, sdkdom, def) < 0) + if (prlsdkAddDomainHardDisksInfo(driver, sdkdom, def) < 0) goto error;

- if (prlsdkAddDomainOpticalDisksInfo(privconn, sdkdom, def) < 0) + if (prlsdkAddDomainOpticalDisksInfo(driver, sdkdom, def) < 0) goto error;

if (prlsdkAddDomainNetInfo(sdkdom, def) < 0) @@ -1250,7 +1250,7 @@ prlsdkConvertCpuMode(PRL_HANDLE sdkdom, virDomainDefPtr def) }

static virDomainObjPtr -prlsdkNewDomainByHandle(vzConnPtr privconn, PRL_HANDLE sdkdom) +prlsdkNewDomainByHandle(vzDriverPtr driver, PRL_HANDLE sdkdom) { virDomainObjPtr dom = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; @@ -1259,16 +1259,18 @@ prlsdkNewDomainByHandle(vzConnPtr privconn, PRL_HANDLE sdkdom) if (prlsdkGetDomainIds(sdkdom, &name, uuid) < 0) goto cleanup;

- if (!(dom = vzNewDomain(privconn, name, uuid))) + virObjectLock(driver); if you need to introduce lock here i think you need extra patch with explanations

ok

...

+ if (!(dom = vzNewDomain(driver, name, uuid))) goto cleanup;

- if (prlsdkLoadDomain(privconn, dom) < 0) { - virDomainObjListRemove(privconn->domains, dom); + if (prlsdkLoadDomain(driver, dom) < 0) { + virDomainObjListRemove(driver->domains, dom); dom = NULL; goto cleanup; }

cleanup: + virObjectUnlock(driver); VIR_FREE(name); return dom; } @@ -1509,7 +1511,7 @@ prlsdkConvertBootOrder(PRL_HANDLE sdkdom, virDomainDefPtr def) }

int -prlsdkLoadDomain(vzConnPtr privconn, virDomainObjPtr dom) +prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom) { virDomainDefPtr def = NULL; vzDomObjPtr pdom = NULL; @@ -1523,11 +1525,10 @@ prlsdkLoadDomain(vzConnPtr privconn, virDomainObjPtr dom) PRL_VM_AUTOSTART_OPTION autostart; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;

- virCheckNonNullArgGoto(privconn, error); virCheckNonNullArgGoto(dom, error);

pdom = dom->privateData; - sdkdom = prlsdkSdkDomainLookupByUUID(privconn, dom->def->uuid); + sdkdom = prlsdkSdkDomainLookupByUUID(driver, dom->def->uuid); if (sdkdom == PRL_INVALID_HANDLE) return -1;

@@ -1560,7 +1561,7 @@ prlsdkLoadDomain(vzConnPtr privconn, virDomainObjPtr dom) if (prlsdkConvertDomainType(sdkdom, def) < 0) goto error;

- if (prlsdkAddDomainHardware(privconn, sdkdom, def) < 0) + if (prlsdkAddDomainHardware(driver, sdkdom, def) < 0) goto error;

/* depends on prlsdkAddDomainHardware */ @@ -1652,7 +1653,7 @@ prlsdkLoadDomain(vzConnPtr privconn, virDomainObjPtr dom) }

int -prlsdkLoadDomains(vzConnPtr privconn) +prlsdkLoadDomains(vzDriverPtr driver) { PRL_HANDLE job = PRL_INVALID_HANDLE; PRL_HANDLE result; @@ -1662,7 +1663,7 @@ prlsdkLoadDomains(vzConnPtr privconn) size_t i = 0; virDomainObjPtr dom;

- job = PrlSrv_GetVmListEx(privconn->server, PVTF_VM | PVTF_CT); + job = PrlSrv_GetVmListEx(driver->server, PVTF_VM | PVTF_CT);

if (PRL_FAILED(getJobResult(job, &result))) return -1; @@ -1674,8 +1675,8 @@ prlsdkLoadDomains(vzConnPtr privconn) pret = PrlResult_GetParamByIndex(result, i, &sdkdom); prlsdkCheckRetGoto(pret, error);

- if (!(dom = prlsdkNewDomainByHandle(privconn, sdkdom))) - goto error; + if (!(dom = prlsdkNewDomainByHandle(driver, sdkdom))) + continue;

virObjectUnlock(dom); PrlHandle_Free(sdkdom); @@ -1692,7 +1693,7 @@ prlsdkLoadDomains(vzConnPtr privconn) }

int -prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom) +prlsdkUpdateDomain(vzDriverPtr driver, virDomainObjPtr dom) { PRL_HANDLE job; vzDomObjPtr pdom = dom->privateData; @@ -1701,10 +1702,10 @@ prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom) if (waitJob(job)) return -1;

- return prlsdkLoadDomain(privconn, dom); + return prlsdkLoadDomain(driver, dom); }

-static int prlsdkSendEvent(vzConnPtr privconn, +static int prlsdkSendEvent(vzDriverPtr driver, virDomainObjPtr dom, virDomainEventType lvEventType, int lvEventTypeDetails) @@ -1717,7 +1718,7 @@ static int prlsdkSendEvent(vzConnPtr privconn, if (!event) return -1;

- virObjectEventStateQueue(privconn->domainEventState, event); + virObjectEventStateQueue(driver->domainEventState, event); return 0; }

@@ -1755,7 +1756,7 @@ prlsdkNewStateToEvent(VIRTUAL_MACHINE_STATE domainState, }

static void -prlsdkHandleVmStateEvent(vzConnPtr privconn, +prlsdkHandleVmStateEvent(vzDriverPtr driver, PRL_HANDLE prlEvent, unsigned char *uuid) { @@ -1767,7 +1768,7 @@ prlsdkHandleVmStateEvent(vzConnPtr privconn, virDomainEventType lvEventType = 0; int lvEventTypeDetails = 0;

- dom = virDomainObjListFindByUUID(privconn->domains, uuid); + dom = virDomainObjListFindByUUID(driver->domains, uuid); if (dom == NULL) return;

@@ -1785,7 +1786,7 @@ prlsdkHandleVmStateEvent(vzConnPtr privconn, &lvEventType, &lvEventTypeDetails);

- prlsdkSendEvent(privconn, dom, lvEventType, lvEventTypeDetails); + prlsdkSendEvent(driver, dom, lvEventType, lvEventTypeDetails);

cleanup: virObjectUnlock(dom); @@ -1793,19 +1794,19 @@ prlsdkHandleVmStateEvent(vzConnPtr privconn, }

static void -prlsdkHandleVmConfigEvent(vzConnPtr privconn, +prlsdkHandleVmConfigEvent(vzDriverPtr driver, unsigned char *uuid) { virDomainObjPtr dom = NULL;

- dom = virDomainObjListFindByUUID(privconn->domains, uuid); + dom = virDomainObjListFindByUUID(driver->domains, uuid); if (dom == NULL) return;

- if (prlsdkUpdateDomain(privconn, dom) < 0) + if (prlsdkUpdateDomain(driver, dom) < 0) goto cleanup;

- prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_DEFINED, + prlsdkSendEvent(driver, dom, VIR_DOMAIN_EVENT_DEFINED, VIR_DOMAIN_EVENT_DEFINED_UPDATED);

cleanup: @@ -1814,23 +1815,23 @@ prlsdkHandleVmConfigEvent(vzConnPtr privconn, }

static void -prlsdkHandleVmAddedEvent(vzConnPtr privconn, +prlsdkHandleVmAddedEvent(vzDriverPtr driver, unsigned char *uuid) { virDomainObjPtr dom = NULL; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;

- dom = virDomainObjListFindByUUID(privconn->domains, uuid); + dom = virDomainObjListFindByUUID(driver->domains, uuid); if (!dom) { - sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid); + sdkdom = prlsdkSdkDomainLookupByUUID(driver, uuid); if (sdkdom == PRL_INVALID_HANDLE) goto cleanup;

- if (!(dom = prlsdkNewDomainByHandle(privconn, sdkdom))) + if (!(dom = prlsdkNewDomainByHandle(driver, sdkdom))) goto cleanup; }

- prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_DEFINED, + prlsdkSendEvent(driver, dom, VIR_DOMAIN_EVENT_DEFINED, VIR_DOMAIN_EVENT_DEFINED_ADDED);

cleanup: @@ -1841,28 +1842,28 @@ prlsdkHandleVmAddedEvent(vzConnPtr privconn, }

static void -prlsdkHandleVmRemovedEvent(vzConnPtr privconn, +prlsdkHandleVmRemovedEvent(vzDriverPtr driver, unsigned char *uuid) { virDomainObjPtr dom = NULL;

- dom = virDomainObjListFindByUUID(privconn->domains, uuid); + dom = virDomainObjListFindByUUID(driver->domains, uuid); /* domain was removed from the list from the libvirt * API function in current connection */ if (dom == NULL) return;

- prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_UNDEFINED, + prlsdkSendEvent(driver, dom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);

- virDomainObjListRemove(privconn->domains, dom); + virDomainObjListRemove(driver->domains, dom); return; }

#define PARALLELS_STATISTICS_DROP_COUNT 3

static PRL_RESULT -prlsdkHandlePerfEvent(vzConnPtr privconn, +prlsdkHandlePerfEvent(vzDriverPtr driver, PRL_HANDLE event, unsigned char *uuid) { @@ -1870,7 +1871,7 @@ prlsdkHandlePerfEvent(vzConnPtr privconn, vzDomObjPtr privdom = NULL; PRL_HANDLE job = PRL_INVALID_HANDLE;

- dom = virDomainObjListFindByUUID(privconn->domains, uuid); + dom = virDomainObjListFindByUUID(driver->domains, uuid); if (dom == NULL) goto cleanup; privdom = dom->privateData; @@ -1907,7 +1908,7 @@ prlsdkHandlePerfEvent(vzConnPtr privconn, static PRL_RESULT prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) { - vzConnPtr privconn = opaque; + vzDriverPtr driver = opaque; PRL_RESULT pret = PRL_ERR_FAILURE; PRL_HANDLE_TYPE handleType; char uuidstr[VIR_UUID_STRING_BUFLEN + 2]; @@ -1922,7 +1923,7 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) if (handleType != PHT_EVENT) goto cleanup;

- if (privconn == NULL) + if (driver == NULL) goto cleanup;

pret = PrlEvent_GetIssuerId(prlEvent, uuidstr, &bufsize); @@ -1938,27 +1939,26 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque)

switch (prlEventType) { case PET_DSP_EVT_VM_STATE_CHANGED: - prlsdkHandleVmStateEvent(privconn, prlEvent, uuid); + prlsdkHandleVmStateEvent(driver, prlEvent, uuid); break; case PET_DSP_EVT_VM_CONFIG_CHANGED: - prlsdkHandleVmConfigEvent(privconn, uuid); + prlsdkHandleVmConfigEvent(driver, uuid); break; case PET_DSP_EVT_VM_CREATED: case PET_DSP_EVT_VM_ADDED: - prlsdkHandleVmAddedEvent(privconn, uuid); + prlsdkHandleVmAddedEvent(driver, uuid); break; case PET_DSP_EVT_VM_DELETED: case PET_DSP_EVT_VM_UNREGISTERED: - prlsdkHandleVmRemovedEvent(privconn, uuid); + prlsdkHandleVmRemovedEvent(driver, uuid); break; case PET_DSP_EVT_VM_PERFSTATS: - prlsdkHandlePerfEvent(privconn, prlEvent, uuid); + prlsdkHandlePerfEvent(driver, prlEvent, uuid); /* above function takes own of event */ prlEvent = PRL_INVALID_HANDLE; break; case PET_DSP_EVT_DISP_CONNECTION_CLOSED: - virConnectCloseCallbackDataCall(privconn->closeCallback, - VIR_CONNECT_CLOSE_REASON_EOF); + vzDestroyDriverConnection(); break; default: VIR_DEBUG("Skipping event of type %d", prlEventType); @@ -1969,13 +1969,13 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque) return PRL_ERR_SUCCESS; }

-int prlsdkSubscribeToPCSEvents(vzConnPtr privconn) +int prlsdkSubscribeToPCSEvents(vzDriverPtr driver) { PRL_RESULT pret = PRL_ERR_UNINITIALIZED;

- pret = PrlSrv_RegEventHandler(privconn->server, + pret = PrlSrv_RegEventHandler(driver->server, prlsdkEventsHandler, - privconn); + driver); prlsdkCheckRetGoto(pret, error); return 0;

@@ -1983,12 +1983,12 @@ int prlsdkSubscribeToPCSEvents(vzConnPtr privconn) return -1; }

-void prlsdkUnsubscribeFromPCSEvents(vzConnPtr privconn) +void prlsdkUnsubscribeFromPCSEvents(vzDriverPtr driver) { PRL_RESULT ret = PRL_ERR_UNINITIALIZED; - ret = PrlSrv_UnregEventHandler(privconn->server, + ret = PrlSrv_UnregEventHandler(driver->server, prlsdkEventsHandler, - privconn); + driver); if (PRL_FAILED(ret)) logPrlError(ret); } @@ -2052,7 +2052,7 @@ PRL_RESULT prlsdkRestart(PRL_HANDLE sdkdom) }

int -prlsdkDomainChangeStateLocked(vzConnPtr privconn, +prlsdkDomainChangeStateLocked(vzDriverPtr driver, virDomainObjPtr dom, prlsdkChangeStateFunc chstate) { @@ -2078,7 +2078,7 @@ prlsdkDomainChangeStateLocked(vzConnPtr privconn, return -1; }

- return prlsdkUpdateDomain(privconn, dom); + return prlsdkUpdateDomain(driver, dom); }

int @@ -2092,7 +2092,7 @@ prlsdkDomainChangeState(virDomainPtr domain, if (!(dom = vzDomObjFromDomain(domain))) return -1;

- ret = prlsdkDomainChangeStateLocked(privconn, dom, chstate); + ret = prlsdkDomainChangeStateLocked(privconn->driver, dom, chstate); virObjectUnlock(dom); return ret; } @@ -2850,7 +2850,7 @@ static const char * prlsdkFormatMac(virMacAddrPtr mac, char *macstr) return macstr; }

-static int prlsdkAddNet(vzConnPtr privconn, +static int prlsdkAddNet(vzDriverPtr driver, PRL_HANDLE sdkdom, virDomainNetDefPtr net, bool isCt) @@ -3055,7 +3055,7 @@ static int prlsdkAddNet(vzConnPtr privconn, pret = PrlVirtNet_SetNetworkType(vnet, PVN_BRIDGED_ETHERNET); prlsdkCheckRetGoto(pret, cleanup);

- job = PrlSrv_AddVirtualNetwork(privconn->server, + job = PrlSrv_AddVirtualNetwork(driver->server, vnet, PRL_USE_VNET_NAME_FOR_BRIDGE_NAME); if (PRL_FAILED(pret = waitJob(job))) @@ -3084,7 +3084,7 @@ static int prlsdkAddNet(vzConnPtr privconn, }

static void -prlsdkCleanupBridgedNet(vzConnPtr privconn, virDomainNetDefPtr net) +prlsdkCleanupBridgedNet(vzDriverPtr driver, virDomainNetDefPtr net) { PRL_RESULT pret; PRL_HANDLE vnet = PRL_INVALID_HANDLE; @@ -3099,7 +3099,7 @@ prlsdkCleanupBridgedNet(vzConnPtr privconn, virDomainNetDefPtr net) pret = PrlVirtNet_SetNetworkId(vnet, net->data.network.name); prlsdkCheckRetGoto(pret, cleanup);

- job = PrlSrv_DeleteVirtualNetwork(privconn->server, vnet, 0); + job = PrlSrv_DeleteVirtualNetwork(driver->server, vnet, 0); if (PRL_FAILED(pret = waitJob(job))) goto cleanup;

@@ -3107,7 +3107,7 @@ prlsdkCleanupBridgedNet(vzConnPtr privconn, virDomainNetDefPtr net) PrlHandle_Free(vnet); }

-int prlsdkAttachNet(vzConnPtr privconn, +int prlsdkAttachNet(vzDriverPtr driver, virDomainObjPtr dom, virDomainNetDefPtr net) { @@ -3125,7 +3125,7 @@ int prlsdkAttachNet(vzConnPtr privconn, if (PRL_FAILED(waitJob(job))) return ret;

- ret = prlsdkAddNet(privconn, privdom->sdkdom, net, IS_CT(dom->def)); + ret = prlsdkAddNet(driver, privdom->sdkdom, net, IS_CT(dom->def)); if (ret == 0) { job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE); if (PRL_FAILED(waitJob(job))) @@ -3172,7 +3172,7 @@ prlsdkFindNetByMAC(PRL_HANDLE sdkdom, virMacAddrPtr mac) return adapter; }

-int prlsdkDetachNet(vzConnPtr privconn, +int prlsdkDetachNet(vzDriverPtr driver, virDomainObjPtr dom, virDomainNetDefPtr net) { @@ -3196,7 +3196,7 @@ int prlsdkDetachNet(vzConnPtr privconn, if (sdknet == PRL_INVALID_HANDLE) goto cleanup;

- prlsdkCleanupBridgedNet(privconn, net); + prlsdkCleanupBridgedNet(driver, net);

pret = PrlVmDev_Remove(sdknet); prlsdkCheckRetGoto(pret, cleanup); @@ -3231,7 +3231,7 @@ static int prlsdkDelDisk(PRL_HANDLE sdkdom, int idx) return ret; }

-static int prlsdkAddDisk(vzConnPtr privconn, +static int prlsdkAddDisk(vzDriverPtr driver, PRL_HANDLE sdkdom, virDomainDiskDefPtr disk) { @@ -3342,7 +3342,7 @@ static int prlsdkAddDisk(vzConnPtr privconn, }

if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { - if (vzGetDefaultSCSIModel(privconn, &scsiModel) < 0) + if (vzGetDefaultSCSIModel(driver, &scsiModel) < 0) goto cleanup; pret = PrlVmDev_SetSubType(sdkdisk, scsiModel); prlsdkCheckRetGoto(pret, cleanup); @@ -3380,7 +3380,7 @@ static int prlsdkAddDisk(vzConnPtr privconn, }

int -prlsdkAttachVolume(vzConnPtr privconn, +prlsdkAttachVolume(vzDriverPtr driver, virDomainObjPtr dom, virDomainDiskDefPtr disk) { @@ -3392,7 +3392,7 @@ prlsdkAttachVolume(vzConnPtr privconn, if (PRL_FAILED(waitJob(job))) goto cleanup;

- ret = prlsdkAddDisk(privconn, privdom->sdkdom, disk); + ret = prlsdkAddDisk(driver, privdom->sdkdom, disk); if (ret == 0) { job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE); if (PRL_FAILED(waitJob(job))) { @@ -3594,7 +3594,7 @@ prlsdkSetBootOrderVm(PRL_HANDLE sdkdom, virDomainDefPtr def) }

static int -prlsdkDoApplyConfig(virConnectPtr conn, +prlsdkDoApplyConfig(vzDriverPtr driver, PRL_HANDLE sdkdom, virDomainDefPtr def, virDomainDefPtr olddef) @@ -3660,11 +3660,11 @@ prlsdkDoApplyConfig(virConnectPtr conn,

if (olddef) { for (i = 0; i < olddef->nnets; i++) - prlsdkCleanupBridgedNet(conn->privateData, olddef->nets[i]); + prlsdkCleanupBridgedNet(driver, olddef->nets[i]); }

for (i = 0; i < def->nnets; i++) { - if (prlsdkAddNet(conn->privateData, sdkdom, def->nets[i], IS_CT(def)) < 0) + if (prlsdkAddNet(driver, sdkdom, def->nets[i], IS_CT(def)) < 0) goto error; }

@@ -3685,7 +3685,7 @@ prlsdkDoApplyConfig(virConnectPtr conn, }

for (i = 0; i < def->ndisks; i++) { - if (prlsdkAddDisk(conn->privateData, sdkdom, def->disks[i]) < 0) + if (prlsdkAddDisk(driver, sdkdom, def->disks[i]) < 0) goto error; }

@@ -3703,22 +3703,21 @@ prlsdkDoApplyConfig(virConnectPtr conn, VIR_FREE(mask);

for (i = 0; i < def->nnets; i++) - prlsdkCleanupBridgedNet(conn->privateData, def->nets[i]); + prlsdkCleanupBridgedNet(driver, def->nets[i]);

return -1; }

int -prlsdkApplyConfig(virConnectPtr conn, +prlsdkApplyConfig(vzDriverPtr driver, virDomainObjPtr dom, virDomainDefPtr new) { - vzConnPtr privconn = conn->privateData; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; PRL_HANDLE job = PRL_INVALID_HANDLE; int ret;

- sdkdom = prlsdkSdkDomainLookupByUUID(privconn, dom->def->uuid); + sdkdom = prlsdkSdkDomainLookupByUUID(driver, dom->def->uuid); if (sdkdom == PRL_INVALID_HANDLE) return -1;

@@ -3726,7 +3725,7 @@ prlsdkApplyConfig(virConnectPtr conn, if (PRL_FAILED(waitJob(job))) return -1;

- ret = prlsdkDoApplyConfig(conn, sdkdom, new, dom->def); + ret = prlsdkDoApplyConfig(driver, sdkdom, new, dom->def);

if (ret == 0) { job = PrlVm_CommitEx(sdkdom, PVCF_DETACH_HDD_BUNDLE); @@ -3740,9 +3739,8 @@ prlsdkApplyConfig(virConnectPtr conn, }

int -prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) +prlsdkCreateVm(vzDriverPtr driver, virDomainDefPtr def) { - vzConnPtr privconn = conn->privateData; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; PRL_HANDLE job = PRL_INVALID_HANDLE; PRL_HANDLE result = PRL_INVALID_HANDLE; @@ -3750,10 +3748,10 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) PRL_RESULT pret; int ret = -1;

- pret = PrlSrv_CreateVm(privconn->server, &sdkdom); + pret = PrlSrv_CreateVm(driver->server, &sdkdom); prlsdkCheckRetGoto(pret, cleanup);

- job = PrlSrv_GetSrvConfig(privconn->server); + job = PrlSrv_GetSrvConfig(driver->server); if (PRL_FAILED(getJobResult(job, &result))) goto cleanup;

@@ -3766,7 +3764,7 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) pret = PrlVmCfg_SetOfflineManagementEnabled(sdkdom, 0); prlsdkCheckRetGoto(pret, cleanup);

- ret = prlsdkDoApplyConfig(conn, sdkdom, def, NULL); + ret = prlsdkDoApplyConfig(driver, sdkdom, def, NULL); if (ret) goto cleanup;

@@ -3780,9 +3778,8 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) }

int -prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) +prlsdkCreateCt(vzDriverPtr driver, virDomainDefPtr def) { - vzConnPtr privconn = conn->privateData; PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; PRL_GET_VM_CONFIG_PARAM_DATA confParam; PRL_HANDLE job = PRL_INVALID_HANDLE; @@ -3815,7 +3812,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) confParam.sConfigSample = "vswap.1024MB"; confParam.nOsVersion = 0;

- job = PrlSrv_GetDefaultVmConfig(privconn->server, &confParam, 0); + job = PrlSrv_GetDefaultVmConfig(driver->server, &confParam, 0); if (PRL_FAILED(getJobResult(job, &result))) goto cleanup;

@@ -3828,7 +3825,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def)

}

- ret = prlsdkDoApplyConfig(conn, sdkdom, def, NULL); + ret = prlsdkDoApplyConfig(driver, sdkdom, def, NULL); if (ret) goto cleanup;

@@ -3947,7 +3944,7 @@ prlsdkDomainHasSnapshots(PRL_HANDLE sdkdom, int* found) }

int -prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom, unsigned int flags) +prlsdkUnregisterDomain(vzDriverPtr driver, virDomainObjPtr dom, unsigned int flags) { vzDomObjPtr privdom = dom->privateData; PRL_HANDLE job; @@ -3984,13 +3981,13 @@ prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom, unsigned int fla return -1;

for (i = 0; i < dom->def->nnets; i++) - prlsdkCleanupBridgedNet(privconn, dom->def->nets[i]); + prlsdkCleanupBridgedNet(driver, dom->def->nets[i]);

- if (prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_UNDEFINED, + if (prlsdkSendEvent(driver, dom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) < 0) return -1;

- virDomainObjListRemove(privconn->domains, dom); + virDomainObjListRemove(driver->domains, dom); return 0; }

diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index 2f11d4f..f197832 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -26,16 +26,16 @@

int prlsdkInit(void); void prlsdkDeinit(void); -int prlsdkConnect(vzConnPtr privconn); -void prlsdkDisconnect(vzConnPtr privconn); +int prlsdkConnect(vzDriverPtr driver); +void prlsdkDisconnect(vzDriverPtr driver); int -prlsdkLoadDomains(vzConnPtr privconn); -int prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom); +prlsdkLoadDomains(vzDriverPtr driver); +int prlsdkUpdateDomain(vzDriverPtr driver, virDomainObjPtr dom); int -prlsdkLoadDomain(vzConnPtr privconn, +prlsdkLoadDomain(vzDriverPtr driver, virDomainObjPtr dom); -int prlsdkSubscribeToPCSEvents(vzConnPtr privconn); -void prlsdkUnsubscribeFromPCSEvents(vzConnPtr privconn); +int prlsdkSubscribeToPCSEvents(vzDriverPtr driver); +void prlsdkUnsubscribeFromPCSEvents(vzDriverPtr driver); PRL_RESULT prlsdkStart(PRL_HANDLE sdkdom); PRL_RESULT prlsdkKill(PRL_HANDLE sdkdom); PRL_RESULT prlsdkStop(PRL_HANDLE sdkdom); @@ -49,29 +49,29 @@ int prlsdkDomainChangeState(virDomainPtr domain, prlsdkChangeStateFunc chstate); int -prlsdkDomainChangeStateLocked(vzConnPtr privconn, +prlsdkDomainChangeStateLocked(vzDriverPtr driver, virDomainObjPtr dom, prlsdkChangeStateFunc chstate); int -prlsdkApplyConfig(virConnectPtr conn, +prlsdkApplyConfig(vzDriverPtr driver, virDomainObjPtr dom, virDomainDefPtr new); -int prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def); -int prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def); +int prlsdkCreateVm(vzDriverPtr driver, virDomainDefPtr def); +int prlsdkCreateCt(vzDriverPtr driver, virDomainDefPtr def); int -prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom, unsigned int flags); +prlsdkUnregisterDomain(vzDriverPtr driver, virDomainObjPtr dom, unsigned int flags); int prlsdkDomainManagedSaveRemove(virDomainObjPtr dom); int -prlsdkAttachVolume(vzConnPtr privconn, virDomainObjPtr dom, virDomainDiskDefPtr disk); +prlsdkAttachVolume(vzDriverPtr driver, virDomainObjPtr dom, virDomainDiskDefPtr disk); int prlsdkDetachVolume(virDomainObjPtr dom, virDomainDiskDefPtr disk); int prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBlockStatsPtr stats); int -prlsdkAttachNet(vzConnPtr privconn, virDomainObjPtr dom, virDomainNetDefPtr net); +prlsdkAttachNet(vzDriverPtr driver, virDomainObjPtr dom, virDomainNetDefPtr net); int -prlsdkDetachNet(vzConnPtr privconn, virDomainObjPtr dom, virDomainNetDefPtr net); +prlsdkDetachNet(vzDriverPtr driver, virDomainObjPtr dom, virDomainNetDefPtr net); int prlsdkGetNetStats(virDomainObjPtr dom, const char *path, virDomainInterfaceStatsPtr stats); int diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index 64e469c..6e28f20 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -73,8 +73,9 @@ vzDomObjFromDomain(virDomainPtr domain) virDomainObjPtr vm; vzConnPtr privconn = domain->conn->privateData; char uuidstr[VIR_UUID_STRING_BUFLEN]; + vzDriverPtr driver = privconn->driver;

- vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid); + vm = virDomainObjListFindByUUID(driver->domains, domain->uuid); if (!vm) { virUUIDFormat(domain->uuid, uuidstr); virReportError(VIR_ERR_NO_DOMAIN, @@ -84,7 +85,6 @@ vzDomObjFromDomain(virDomainPtr domain) }

return vm; - }

/** @@ -103,8 +103,9 @@ vzDomObjFromDomainRef(virDomainPtr domain) virDomainObjPtr vm; vzConnPtr privconn = domain->conn->privateData; char uuidstr[VIR_UUID_STRING_BUFLEN]; + vzDriverPtr driver = privconn->driver;

- vm = virDomainObjListFindByUUIDRef(privconn->domains, domain->uuid); + vm = virDomainObjListFindByUUIDRef(driver->domains, domain->uuid); if (!vm) { virUUIDFormat(domain->uuid, uuidstr); virReportError(VIR_ERR_NO_DOMAIN, @@ -159,7 +160,7 @@ vzGetOutput(const char *binary, ...) }

virDomainObjPtr -vzNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid) +vzNewDomain(vzDriverPtr driver, char *name, const unsigned char *uuid) { virDomainDefPtr def = NULL; virDomainObjPtr dom = NULL; @@ -180,8 +181,8 @@ vzNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid)

def->virtType = VIR_DOMAIN_VIRT_VZ;

- if (!(dom = virDomainObjListAdd(privconn->domains, def, - privconn->xmlopt, + if (!(dom = virDomainObjListAdd(driver->domains, def, + driver->xmlopt, 0, NULL))) goto error;

@@ -199,7 +200,7 @@ vzNewDomain(vzConnPtr privconn, char *name, const unsigned char *uuid) }

static void -vzInitCaps(unsigned long vzVersion, vzCapabilities *vzCaps) +vzInitCaps(unsigned long vzVersion, vzCapabilitiesPtr vzCaps) { if (vzVersion < VIRTUOZZO_VER_7) { vzCaps->ctDiskFormat = VIR_STORAGE_FILE_PLOOP; @@ -217,7 +218,7 @@ vzInitCaps(unsigned long vzVersion, vzCapabilities *vzCaps) }

int -vzInitVersion(vzConnPtr privconn) +vzInitVersion(vzDriverPtr driver) { char *output, *sVer, *tmp; const char *searchStr = "prlsrvctl version "; @@ -250,12 +251,12 @@ vzInitVersion(vzConnPtr privconn) }

tmp[0] = '\0'; - if (virParseVersionString(sVer, &(privconn->vzVersion), true) < 0) { + if (virParseVersionString(sVer, &(driver->vzVersion), true) < 0) { vzParseError(); goto cleanup; }

- vzInitCaps(privconn->vzVersion, &privconn->vzCaps); + vzInitCaps(driver->vzVersion, &driver->vzCaps); ret = 0;

cleanup: @@ -473,10 +474,10 @@ vzCheckUnsupportedControllers(virDomainDefPtr def, vzCapabilitiesPtr vzCaps) return 0; }

-int vzGetDefaultSCSIModel(vzConnPtr privconn, +int vzGetDefaultSCSIModel(vzDriverPtr driver, PRL_CLUSTERED_DEVICE_SUBTYPE *scsiModel) { - switch (privconn->vzCaps.scsiControllerModel) { + switch (driver->vzCaps.scsiControllerModel) { case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI: *scsiModel = PCD_VIRTIO_SCSI; break; @@ -487,7 +488,7 @@ int vzGetDefaultSCSIModel(vzConnPtr privconn, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown SCSI controller model %s"), virDomainControllerModelSCSITypeToString( - privconn->vzCaps.scsiControllerModel)); + driver->vzCaps.scsiControllerModel)); return -1; } return 0; diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index b415b0f..ce2fd92 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -60,8 +60,8 @@ struct _vzCapabilities { typedef struct _vzCapabilities vzCapabilities; typedef struct _vzCapabilities *vzCapabilitiesPtr;

-struct _vzConn { - virMutex lock; +struct _vzDriver { + virObjectLockable parent;

/* Immutable pointer, self-locking APIs */ virDomainObjListPtr domains; @@ -76,9 +76,21 @@ struct _vzConn { vzCapabilities vzCaps; }; you need to remove virConnectCloseCallbackDataPtr field from driver

ok

...

+typedef struct _vzDriver vzDriver; +typedef struct _vzDriver *vzDriverPtr; + +struct _vzConn { + struct _vzConn* next; + + vzDriverPtr driver; + /* Immutable pointer, self-locking APIs */ + virConnectCloseCallbackDataPtr closeCallback; +}; + typedef struct _vzConn vzConn; typedef struct _vzConn *vzConnPtr;

+ struct _vzCountersCache { PRL_HANDLE stats; virCond cond; @@ -105,12 +117,19 @@ char * vzGetOutput(const char *binary, ...) ATTRIBUTE_NONNULL(1) ATTRIBUTE_SENTINEL; void vzDriverLock(vzConnPtr driver); void vzDriverUnlock(vzConnPtr driver); + +vzDriverPtr +vzGetDriverConnection(void); + +void +vzDestroyDriverConnection(void); + virDomainObjPtr -vzNewDomain(vzConnPtr privconn, +vzNewDomain(vzDriverPtr driver, char *name, const unsigned char *uuid); int -vzInitVersion(vzConnPtr privconn); +vzInitVersion(vzDriverPtr driver); int vzCheckUnsupportedDisks(virDomainDefPtr def, vzCapabilitiesPtr vzCaps); @@ -118,7 +137,7 @@ int vzCheckUnsupportedControllers(virDomainDefPtr def, vzCapabilitiesPtr vzCaps); int -vzGetDefaultSCSIModel(vzConnPtr privconn, +vzGetDefaultSCSIModel(vzDriverPtr driver, PRL_CLUSTERED_DEVICE_SUBTYPE *scsiModel);

# define PARALLELS_BLOCK_STATS_FOREACH(OP) \

On 07.04.2016 23:10, Maxim Nestratov wrote:

We don't need them anymore as all pointers within vzDriver structure are not changed during the time it exists. Where we still need to synchronize we use virObjectLock/Unlock as far as vzDriver is lockable object.

Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 35 ----------------------------------- src/vz/vz_utils.h | 2 -- 2 files changed, 37 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index dce7a87..50da2fe 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c

...

@@ -1537,7 +1506,6 @@ vzConnectRegisterCloseCallback(virConnectPtr conn, vzConnPtr privconn = conn->privateData; int ret = -1;

- vzDriverLock(privconn); if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != NULL) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("A close callback is already registered")); @@ -1549,7 +1517,6 @@ vzConnectRegisterCloseCallback(virConnectPtr conn, ret = 0;

cleanup: - vzDriverUnlock(privconn);

return ret; } @@ -1560,7 +1527,6 @@ vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb) vzConnPtr privconn = conn->privateData; int ret = -1;

- vzDriverLock(privconn);

if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != cb) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -1572,7 +1538,6 @@ vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb) ret = 0;

cleanup: - vzDriverUnlock(privconn);

return ret; }

In this two particular functions we still need locking. The problem is that close connect object API is not complete self-locking. We need to use higher level lock because the check that register is possible and register itself are not atomic. This was made intentionally to make register function result type be void so that remote driver could be written transactionally. On ther other hand we can not be client side driver anymore and our only user is remote driver/daemon pair which is considered correct. So it's up to you.

On 07.04.2016 23:10, Maxim Nestratov wrote:

remove unnecessary vzConnectClose prototype and make local structure vzDomainDefParserConfig be static

Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 50da2fe..b3ce404 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -62,7 +62,6 @@ VIR_LOG_INIT("parallels.parallels_driver");

#define PRLCTL "prlctl"

-static int vzConnectClose(virConnectPtr conn); static virClassPtr vzDriverConnClass;

static virMutex vz_driver_lock; @@ -266,7 +265,7 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, }

-virDomainDefParserConfig vzDomainDefParserConfig = { +static virDomainDefParserConfig vzDomainDefParserConfig = { .macPrefix = {0x42, 0x1C, 0x00}, .devicesPostParseCallback = vzDomainDeviceDefPostParse, .domainPostParseCallback = vzDomainDefPostParse,

ACK

On 07.04.2016 23:10, Maxim Nestratov wrote:

Now it detects if a connection to vz dispatcher is up or not by comparing current driver to connection stored pointer.

Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index b3ce404..7d7cb17 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -824,8 +824,12 @@ static int vzConnectIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) return 1; }

-static int vzConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) +static int vzConnectIsAlive(virConnectPtr conn) { + vzConnPtr privconn = conn->privateData; + if (privconn->driver != vz_driver) + return 0; + return 1; }

Looks like we never get here because now we live only in daemon and remote driver will never ask us for this function. Remote driver only checks connection to daemon.