[libvirt] [PATCH v2 0/5] vz: make vz driver more responsive
diff from original: ======================== 1. include patch to check domain status on acquiring job condition 2. add patch to fix domain objects naming (update device patch series which was created before convention was enstablished sneak in) 3. squash in all quickfixes 4. rename vzDomainObjIsExist to vzEnsureDomainExists Nikolay Shirokovskiy (5): vz: use state variable sdkdom in prlsdkApplyConfig vz: remove redundant variable in prlsdkHandleVmAddedEvent vz: keep naming convention for domain objects vz: make vz driver more responsive vz: handle gracefully races on undefining domain src/vz/vz_driver.c | 257 +++++++++++++++++++++++++++++++++++++++-------------- src/vz/vz_sdk.c | 207 ++++++++++++++++++++++++++++-------------- src/vz/vz_utils.c | 50 +++++++++++ src/vz/vz_utils.h | 7 ++ 4 files changed, 387 insertions(+), 134 deletions(-) -- 1.8.3.1
sdk domain handle is unique per connection so there is no sense to query it again if we have it in vzDomObjPtr. Side effect of prlsdkSdkDomainLookupByUUID is refreshing domain config is of no use too as PrlVm_BeginEdit do it too. --- src/vz/vz_sdk.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 5e36a8b..6b5adbf 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3906,28 +3906,22 @@ prlsdkApplyConfig(vzDriverPtr driver, virDomainObjPtr dom, virDomainDefPtr new) { - PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; + vzDomObjPtr privdom = dom->privateData; PRL_HANDLE job = PRL_INVALID_HANDLE; int ret; - sdkdom = prlsdkSdkDomainLookupByUUID(driver, dom->def->uuid); - if (sdkdom == PRL_INVALID_HANDLE) - return -1; - - job = PrlVm_BeginEdit(sdkdom); + job = PrlVm_BeginEdit(privdom->sdkdom); if (PRL_FAILED(waitJob(job))) return -1; - ret = prlsdkDoApplyConfig(driver, sdkdom, new, dom->def); + ret = prlsdkDoApplyConfig(driver, privdom->sdkdom, new, dom->def); if (ret == 0) { - job = PrlVm_CommitEx(sdkdom, PVCF_DETACH_HDD_BUNDLE); + job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE); if (PRL_FAILED(waitJob(job))) ret = -1; } - PrlHandle_Free(sdkdom); - return ret; } -- 1.8.3.1
--- src/vz/vz_sdk.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 6b5adbf..5390ead 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2036,7 +2036,6 @@ prlsdkHandleVmAddedEvent(vzDriverPtr driver, unsigned char *uuid) { virDomainObjPtr dom = NULL; - PRL_HANDLE sdkdom = PRL_INVALID_HANDLE; if (!(dom = virDomainObjListFindByUUID(driver->domains, uuid)) && !(dom = prlsdkAddDomainByUUID(driver, uuid))) @@ -2048,7 +2047,6 @@ prlsdkHandleVmAddedEvent(vzDriverPtr driver, cleanup: if (dom) virObjectUnlock(dom); - PrlHandle_Free(sdkdom); return; } -- 1.8.3.1
see 4385b868 --- src/vz/vz_driver.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index c12832c..2ad3d94 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1208,38 +1208,38 @@ vzDomainSetUserPassword(virDomainPtr domain, return ret; } -static int vzDomainUpdateDeviceFlags(virDomainPtr dom, +static int vzDomainUpdateDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { int ret = -1; - vzConnPtr privconn = dom->conn->privateData; - virDomainObjPtr privdom = NULL; + vzConnPtr privconn = domain->conn->privateData; + virDomainObjPtr dom = NULL; virDomainDeviceDefPtr dev = NULL; vzDriverPtr driver = privconn->driver; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (!(privdom = vzDomObjFromDomain(dom))) + if (!(dom = vzDomObjFromDomain(domain))) return -1; - if (vzCheckConfigUpdateFlags(privdom, &flags) < 0) + if (vzCheckConfigUpdateFlags(dom, &flags) < 0) goto cleanup; - if (!(dev = virDomainDeviceDefParse(xml, privdom->def, driver->caps, + if (!(dev = virDomainDeviceDefParse(xml, dom->def, driver->caps, driver->xmlopt, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; - if (prlsdkUpdateDevice(driver, privdom, dev) < 0) + if (prlsdkUpdateDevice(driver, dom, dev) < 0) goto cleanup; ret = 0; cleanup: virDomainDeviceDefFree(dev); - virObjectUnlock(privdom); + virObjectUnlock(dom); return ret; } -- 1.8.3.1
Current vz driver implementation is not usable when it comes to long runnig operations. Migration or saving a domain blocks all other operations even query ones which are expecteted to be available. This patch address this problem. All vz driver API calls fall into next 3 groups: 1. only query domain cache (virDomainObj, vz cache statistic) examples are vzDomainGetState, vzDomainGetXMLDesc etc. 2. use thread shared sdkdom object examples are vzDomainSetMemoryFlags, vzDomainAttachDevice etc. 3. use no thread shared sdkdom object nor domain cache examples are vzDomainSnapshotListNames, vzDomainSnapshotGetXMLDesc etc API calls from group 1 don't need to be changed as they hold domain lock only for short period of time. This calls [1] are easily distinguished. They query domain object thru libvirt common code or query vz sdk statistics handle thru vz sdk sync operations. vzDomainInterfaceStats is the only exception. It uses sdkdom object to convert interface name to its vz sdk stack index which could not be saved in domain cache. Interface statistic is available thru this stack index as a key rather than name. As a result we can have accidental 'not known interface' errors on quering intrerface stats. The reason is that in the process of updating domain configuration we drop all devices and then recreate them again in sdkdom object and domain lock can be dropped meanwhile (to remove networks for existing bridged interfaces and(or) (re)create new ones). We can fix this by changing the way we support bridged interfaces or by reordering operations and changing bridged networks beforehand. Anyway this is better then moving this API call into 2 group and making it an exclusive job. As to API calls from group 2 first thread shared sdkdom object need to be explained. vz sdk has only one handle for given domain thus threads needs exclusive access to operate on it. These calls are fixed to drop and reacquire domain lock on any lengthy operations - namely waiting the result of async vz sdk operation. As lock is dropped we need to take extra reference to domain object if it is not taken already as domain object can be deleted from list while lock is dropped. As this operations use thread shared sdkdom object the simpliest way to make calls from group 2 consistent to each other is to make them mutually exclusive. This is done by taking/releasing job condition thru calling correspondent job routine. This approach makes group 1 and group 2 calls consistent to each other too. Not all calls of group 2 change the domain cache but those that do update it thru prlsdkUpdateDomain which hold the lock thoughout the update. API calls from group [2] are easily distinguished too. They use beginEdit/commit to change domain configuration (vzDomainSetMemoryFlags) or/and update domain cache from sdkdom at the end of operation (vzDomainSuspend). There is a known issue however. Frankly speaking it is introduced by ealier patch '[PATCH 6/9] vz: cleanup loading domain code' from a different series. The patch significantly reduces amount of time when driver lock is hold when creating domain from API call or as a result of domain added event from vz sdk. The problem is these two paths race on using thread shared sdkdom as we don't have libvirt domain object and can not lock on it. However this don't invalidates the patch as we can't use the former approach of preadding domain into the list as we need name at least and name is not given by event. Anyway i'm against adding half baked object into the list. Eventually this race can be fixed by extra measures. As to current situation races with different configurations are unlikely and race when adding domain thru vz driver and simultaneous event from vz sdk is not dangerous as configuration is the same. The last group [3] is API calls that need only sdkdom object to make vz sdk call which don't change thread shared sdkdom object or domain cache in any way. For now these are mostly domain snapshot API calls. The changes are similar to those of group 2 - add extra reference and drop/reacquire the lock on waiting vz async call result. One can simply take the immutable sdkdom object from the cache and drop the lock for the rest of operations but the chosen approach makes implementation of these API calls somewhat similar to those of from group 2 and thus a bit futureproof. As calls of group 3 don't need vz driver domain/vz sdk cache in any way they consistent with respect to API calls from groups 1 and 3. There is another exception. Calls to make-snapshot/revert-to-snapshot/migrate are moved to group 2. That is they are made mutually exclusive. The reason is that libvirt API supports control/query only for one job per domain and these are jobs that are likely to be queried/aborted. Appendix. [1] API calls that only query domain cache. (marked [*] are included for a different reason) .domainLookupByID = vzDomainLookupByID, /* 0.10.0 */ .domainLookupByUUID = vzDomainLookupByUUID, /* 0.10.0 */ .domainLookupByName = vzDomainLookupByName, /* 0.10.0 */ .domainGetOSType = vzDomainGetOSType, /* 0.10.0 */ .domainGetInfo = vzDomainGetInfo, /* 0.10.0 */ .domainGetState = vzDomainGetState, /* 0.10.0 */ .domainGetXMLDesc = vzDomainGetXMLDesc, /* 0.10.0 */ .domainIsPersistent = vzDomainIsPersistent, /* 0.10.0 */ .domainGetAutostart = vzDomainGetAutostart, /* 0.10.0 */ .domainGetVcpus = vzDomainGetVcpus, /* 1.2.6 */ .domainIsActive = vzDomainIsActive, /* 1.2.10 */ .domainIsUpdated = vzDomainIsUpdated, /* 1.2.21 */ .domainGetVcpusFlags = vzDomainGetVcpusFlags, /* 1.2.21 */ .domainGetMaxVcpus = vzDomainGetMaxVcpus, /* 1.2.21 */ .domainHasManagedSaveImage = vzDomainHasManagedSaveImage, /* 1.2.13 */ .domainGetMaxMemory = vzDomainGetMaxMemory, /* 1.2.15 */ .domainBlockStats = vzDomainBlockStats, /* 1.2.17 */ .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.2.17 */ .domainInterfaceStats = vzDomainInterfaceStats, /* 1.2.17 */ [*] .domainMemoryStats = vzDomainMemoryStats, /* 1.2.17 */ .domainMigrateBegin3Params = vzDomainMigrateBegin3Params, /* 1.3.5 */ .domainMigrateConfirm3Params = vzDomainMigrateConfirm3Params, /* 1.3.5 */ [2] API calls that use thread shared sdkdom object (marked [*] are included for a different reason) .domainSuspend = vzDomainSuspend, /* 0.10.0 */ .domainResume = vzDomainResume, /* 0.10.0 */ .domainDestroy = vzDomainDestroy, /* 0.10.0 */ .domainShutdown = vzDomainShutdown, /* 0.10.0 */ .domainCreate = vzDomainCreate, /* 0.10.0 */ .domainCreateWithFlags = vzDomainCreateWithFlags, /* 1.2.10 */ .domainReboot = vzDomainReboot, /* 1.3.0 */ .domainDefineXML = vzDomainDefineXML, /* 0.10.0 */ .domainDefineXMLFlags = vzDomainDefineXMLFlags, /* 1.2.12 */ (update part) .domainUndefine = vzDomainUndefine, /* 1.2.10 */ .domainAttachDevice = vzDomainAttachDevice, /* 1.2.15 */ .domainAttachDeviceFlags = vzDomainAttachDeviceFlags, /* 1.2.15 */ .domainDetachDevice = vzDomainDetachDevice, /* 1.2.15 */ .domainDetachDeviceFlags = vzDomainDetachDeviceFlags, /* 1.2.15 */ .domainSetUserPassword = vzDomainSetUserPassword, /* 1.3.6 */ .domainManagedSave = vzDomainManagedSave, /* 1.2.14 */ .domainSetMemoryFlags = vzDomainSetMemoryFlags, /* 1.3.4 */ .domainSetMemory = vzDomainSetMemory, /* 1.3.4 */ .domainRevertToSnapshot = vzDomainRevertToSnapshot, /* 1.3.5 */ [*] .domainSnapshotCreateXML = vzDomainSnapshotCreateXML, /* 1.3.5 */ [*] .domainMigratePerform3Params = vzDomainMigratePerform3Params, /* 1.3.5 */ [*] .domainUpdateDeviceFlags = vzDomainUpdateDeviceFlags, /* 2.0.0 */ prlsdkHandleVmConfigEvent [3] API calls that do not use thread shared sdkdom object .domainManagedSaveRemove = vzDomainManagedSaveRemove, /* 1.2.14 */ .domainSnapshotNum = vzDomainSnapshotNum, /* 1.3.5 */ .domainSnapshotListNames = vzDomainSnapshotListNames, /* 1.3.5 */ .domainListAllSnapshots = vzDomainListAllSnapshots, /* 1.3.5 */ .domainSnapshotGetXMLDesc = vzDomainSnapshotGetXMLDesc, /* 1.3.5 */ .domainSnapshotNumChildren = vzDomainSnapshotNumChildren, /* 1.3.5 */ .domainSnapshotListChildrenNames = vzDomainSnapshotListChildrenNames, /* 1.3.5 */ .domainSnapshotListAllChildren = vzDomainSnapshotListAllChildren, /* 1.3.5 */ .domainSnapshotLookupByName = vzDomainSnapshotLookupByName, /* 1.3.5 */ .domainHasCurrentSnapshot = vzDomainHasCurrentSnapshot, /* 1.3.5 */ .domainSnapshotGetParent = vzDomainSnapshotGetParent, /* 1.3.5 */ .domainSnapshotCurrent = vzDomainSnapshotCurrent, /* 1.3.5 */ .domainSnapshotIsCurrent = vzDomainSnapshotIsCurrent, /* 1.3.5 */ .domainSnapshotHasMetadata = vzDomainSnapshotHasMetadata, /* 1.3.5 */ .domainSnapshotDelete = vzDomainSnapshotDelete, /* 1.3.5 */ [4] Known issues. 1. accidental errors on getting network statistics 2. race with simultaneous use of thread shared domain object on paths of adding domain thru API and adding domain on vz sdk domain added event. --- src/vz/vz_driver.c | 196 +++++++++++++++++++++++++++++++++++++---------------- src/vz/vz_sdk.c | 180 ++++++++++++++++++++++++++++++++---------------- src/vz/vz_utils.c | 50 ++++++++++++++ src/vz/vz_utils.h | 7 ++ 4 files changed, 316 insertions(+), 117 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 2ad3d94..ce43d1b 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -723,6 +723,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virDomainObjPtr dom = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; vzDriverPtr driver = privconn->driver; + bool job = false; virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); @@ -733,7 +734,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) parse_flags)) == NULL) goto cleanup; - dom = virDomainObjListFindByUUID(driver->domains, def->uuid); + dom = virDomainObjListFindByUUIDRef(driver->domains, def->uuid); if (dom == NULL) { virResetLastError(); if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { @@ -775,6 +776,10 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; } } else { + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + if (prlsdkApplyConfig(driver, dom, def)) goto cleanup; @@ -788,8 +793,9 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) retdom->id = def->id; cleanup: - if (dom) - virObjectUnlock(dom); + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); virDomainDefFree(def); return retdom; } @@ -991,17 +997,26 @@ vzDomainUndefineFlags(virDomainPtr domain, { vzConnPtr privconn = domain->conn->privateData; virDomainObjPtr dom = NULL; - int ret; + int ret = -1; + bool job = false; virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + ret = prlsdkUnregisterDomain(privconn->driver, dom, flags); - if (ret) - virObjectUnlock(dom); + + cleanup: + + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -1039,13 +1054,18 @@ vzDomainManagedSave(virDomainPtr domain, unsigned int flags) virDomainObjPtr dom = NULL; int state, reason; int ret = -1; + bool job = false; virCheckFlags(VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + state = virDomainObjGetState(dom, &reason); if (state == VIR_DOMAIN_RUNNING && (flags & VIR_DOMAIN_SAVE_PAUSED)) { @@ -1057,7 +1077,9 @@ vzDomainManagedSave(virDomainPtr domain, unsigned int flags) ret = prlsdkDomainChangeStateLocked(privconn->driver, dom, prlsdkSuspend); cleanup: - virObjectUnlock(dom); + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -1070,7 +1092,7 @@ vzDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) virCheckFlags(0, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; state = virDomainObjGetState(dom, &reason); @@ -1081,7 +1103,7 @@ vzDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags) ret = prlsdkDomainManagedSaveRemove(dom); cleanup: - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -1115,11 +1137,12 @@ static int vzDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, virDomainDeviceDefPtr dev = NULL; virDomainObjPtr dom = NULL; vzDriverPtr driver = privconn->driver; + bool job = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; if (vzCheckConfigUpdateFlags(dom, &flags) < 0) @@ -1130,13 +1153,19 @@ static int vzDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, if (dev == NULL) goto cleanup; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + if (prlsdkAttachDevice(driver, dom, dev) < 0) goto cleanup; ret = 0; cleanup: virDomainDeviceDefFree(dev); - virObjectUnlock(dom); + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -1154,11 +1183,12 @@ static int vzDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, virDomainDeviceDefPtr dev = NULL; virDomainObjPtr dom = NULL; vzDriverPtr driver = privconn->driver; + bool job = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - dom = vzDomObjFromDomain(domain); + dom = vzDomObjFromDomainRef(domain); if (dom == NULL) return -1; @@ -1172,13 +1202,19 @@ static int vzDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, if (dev == NULL) goto cleanup; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + if (prlsdkDetachDevice(driver, dom, dev) < 0) goto cleanup; ret = 0; cleanup: virDomainDeviceDefFree(dev); - virObjectUnlock(dom); + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -1197,14 +1233,22 @@ vzDomainSetUserPassword(virDomainPtr domain, { virDomainObjPtr dom = NULL; int ret = -1; + bool job = false; virCheckFlags(0, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + ret = prlsdkDomainSetUserPassword(dom, user, password); - virObjectUnlock(dom); + cleanup: + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -1217,11 +1261,12 @@ static int vzDomainUpdateDeviceFlags(virDomainPtr domain, virDomainObjPtr dom = NULL; virDomainDeviceDefPtr dev = NULL; vzDriverPtr driver = privconn->driver; + bool job = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; if (vzCheckConfigUpdateFlags(dom, &flags) < 0) @@ -1232,6 +1277,10 @@ static int vzDomainUpdateDeviceFlags(virDomainPtr domain, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + if (prlsdkUpdateDevice(driver, dom, dev) < 0) goto cleanup; @@ -1239,7 +1288,9 @@ static int vzDomainUpdateDeviceFlags(virDomainPtr domain, cleanup: virDomainDeviceDefFree(dev); - virObjectUnlock(dom); + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -1553,21 +1604,28 @@ static int vzDomainSetMemoryFlagsImpl(virDomainPtr domain, unsigned long memory, { virDomainObjPtr dom = NULL; int ret = -1; + bool job = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; if (useflags && vzCheckConfigUpdateFlags(dom, &flags) < 0) goto cleanup; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + ret = prlsdkSetMemsize(dom, memory >> 10); cleanup: - virObjectUnlock(dom); + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -1634,7 +1692,7 @@ vzDomainSnapshotNum(virDomainPtr domain, unsigned int flags) virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1644,7 +1702,7 @@ vzDomainSnapshotNum(virDomainPtr domain, unsigned int flags) cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return n; } @@ -1662,7 +1720,7 @@ vzDomainSnapshotListNames(virDomainPtr domain, virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1672,7 +1730,7 @@ vzDomainSnapshotListNames(virDomainPtr domain, cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return n; } @@ -1689,7 +1747,7 @@ vzDomainListAllSnapshots(virDomainPtr domain, virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1699,7 +1757,7 @@ vzDomainListAllSnapshots(virDomainPtr domain, cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return n; } @@ -1716,7 +1774,7 @@ vzDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, unsigned int flags) virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); - if (!(dom = vzDomObjFromDomain(snapshot->domain))) + if (!(dom = vzDomObjFromDomainRef(snapshot->domain))) return NULL; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1733,7 +1791,7 @@ vzDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, unsigned int flags) cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return xml; } @@ -1749,7 +1807,7 @@ vzDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, unsigned int flags) virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - if (!(dom = vzDomObjFromDomain(snapshot->domain))) + if (!(dom = vzDomObjFromDomainRef(snapshot->domain))) return -1; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1762,7 +1820,7 @@ vzDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, unsigned int flags) cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return n; } @@ -1781,7 +1839,7 @@ vzDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - if (!(dom = vzDomObjFromDomain(snapshot->domain))) + if (!(dom = vzDomObjFromDomainRef(snapshot->domain))) return -1; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1794,7 +1852,7 @@ vzDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return n; } @@ -1812,7 +1870,7 @@ vzDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - if (!(dom = vzDomObjFromDomain(snapshot->domain))) + if (!(dom = vzDomObjFromDomainRef(snapshot->domain))) return -1; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1825,7 +1883,7 @@ vzDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return n; } @@ -1842,7 +1900,7 @@ vzDomainSnapshotLookupByName(virDomainPtr domain, virCheckFlags(0, NULL); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return NULL; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1854,7 +1912,7 @@ vzDomainSnapshotLookupByName(virDomainPtr domain, snapshot = virGetDomainSnapshot(domain, snap->def->name); cleanup: - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); virDomainSnapshotObjListFree(snapshots); return snapshot; @@ -1869,7 +1927,7 @@ vzDomainHasCurrentSnapshot(virDomainPtr domain, unsigned int flags) virCheckFlags(0, -1); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1879,7 +1937,7 @@ vzDomainHasCurrentSnapshot(virDomainPtr domain, unsigned int flags) cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -1894,7 +1952,7 @@ vzDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, unsigned int flags) virCheckFlags(0, NULL); - if (!(dom = vzDomObjFromDomain(snapshot->domain))) + if (!(dom = vzDomObjFromDomainRef(snapshot->domain))) return NULL; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1914,7 +1972,7 @@ vzDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, unsigned int flags) cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return parent; } @@ -1929,7 +1987,7 @@ vzDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags) virCheckFlags(0, NULL); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return NULL; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1945,7 +2003,7 @@ vzDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags) cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return snapshot; } @@ -1960,7 +2018,7 @@ vzDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, unsigned int flags) virCheckFlags(0, -1); - if (!(dom = vzDomObjFromDomain(snapshot->domain))) + if (!(dom = vzDomObjFromDomainRef(snapshot->domain))) return -1; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -1971,7 +2029,7 @@ vzDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, unsigned int flags) cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -1987,7 +2045,7 @@ vzDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, virCheckFlags(0, -1); - if (!(dom = vzDomObjFromDomain(snapshot->domain))) + if (!(dom = vzDomObjFromDomainRef(snapshot->domain))) return -1; if (!(snapshots = prlsdkLoadSnapshots(dom))) @@ -2000,7 +2058,8 @@ vzDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); + return ret; } @@ -2017,10 +2076,11 @@ vzDomainSnapshotCreateXML(virDomainPtr domain, unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; virDomainSnapshotObjListPtr snapshots = NULL; virDomainSnapshotObjPtr current; + bool job = false; virCheckFlags(0, NULL); - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return NULL; if (!(def = virDomainSnapshotDefParseString(xmlDesc, driver->caps, @@ -2039,6 +2099,10 @@ vzDomainSnapshotCreateXML(virDomainPtr domain, goto cleanup; } + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + /* snaphot name is ignored, it will be set to auto generated by sdk uuid */ if (prlsdkCreateSnapshot(dom, def->description) < 0) goto cleanup; @@ -2057,8 +2121,10 @@ vzDomainSnapshotCreateXML(virDomainPtr domain, cleanup: virDomainSnapshotObjListFree(snapshots); - virObjectUnlock(dom); virDomainSnapshotDefFree(def); + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return snapshot; } @@ -2071,13 +2137,13 @@ vzDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); - if (!(dom = vzDomObjFromDomain(snapshot->domain))) + if (!(dom = vzDomObjFromDomainRef(snapshot->domain))) return -1; ret = prlsdkDeleteSnapshot(dom, snapshot->name, flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN); - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -2087,16 +2153,23 @@ vzDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) { virDomainObjPtr dom; int ret = -1; + bool job = false; virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED, -1); if (!(dom = vzDomObjFromDomain(snapshot->domain))) return -1; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + ret = prlsdkSwitchToSnapshot(dom, snapshot->name, flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED); - - virObjectUnlock(dom); + cleanup: + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -2431,6 +2504,7 @@ vzDomainMigratePerformStep(virDomainPtr domain, const char *miguri = NULL; const char *dname = NULL; vzMigrationCookiePtr mig = NULL; + bool job = false; virCheckFlags(VZ_MIGRATION_FLAGS, -1); @@ -2453,9 +2527,13 @@ vzDomainMigratePerformStep(virDomainPtr domain, VZ_MIGRATION_COOKIE_SESSION_UUID))) goto cleanup; - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) goto cleanup; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + if (!(vzuri = vzParseVzURI(miguri))) goto cleanup; @@ -2463,13 +2541,14 @@ vzDomainMigratePerformStep(virDomainPtr domain, goto cleanup; virDomainObjListRemove(privconn->driver->domains, dom); - dom = NULL; + virObjectLock(dom); ret = 0; cleanup: - if (dom) - virObjectUnlock(dom); + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); virURIFree(vzuri); vzMigrationCookieFree(mig); @@ -2635,8 +2714,7 @@ vzDomainMigrateFinish3Params(virConnectPtr dconn, * is already finished. */ if (!domain) VIR_WARN("Can't provide domain '%s' after successfull migration.", name); - if (dom) - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return domain; } diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 5390ead..5c2a6c5 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -198,6 +198,27 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result, result, __FILE__, __FUNCTION__, __LINE__) static PRL_RESULT +getDomainJobResultHelper(PRL_HANDLE job, virDomainObjPtr dom, + unsigned int timeout, PRL_HANDLE *result, + const char *filename, const char *funcname, + size_t linenr) +{ + PRL_RESULT pret; + + if (dom) + virObjectUnlock(dom); + pret = getJobResultHelper(job, timeout, result, filename, funcname, linenr); + if (dom) + virObjectLock(dom); + + return pret; +} + +#define getDomainJobResult(job, dom, result) \ + getDomainJobResultHelper(job, dom, JOB_INFINIT_WAIT_TIMEOUT, \ + result, __FILE__, __FUNCTION__, __LINE__) + +static PRL_RESULT waitJobHelper(PRL_HANDLE job, unsigned int timeout, const char *filename, const char *funcname, size_t linenr) @@ -215,6 +236,26 @@ waitJobHelper(PRL_HANDLE job, unsigned int timeout, waitJobHelper(job, JOB_INFINIT_WAIT_TIMEOUT, __FILE__, \ __FUNCTION__, __LINE__) +static PRL_RESULT +waitDomainJobHelper(PRL_HANDLE job, virDomainObjPtr dom, unsigned int timeout, + const char *filename, const char *funcname, + size_t linenr) +{ + PRL_RESULT ret; + + if (dom) + virObjectUnlock(dom); + ret = waitJobHelper(job, timeout, filename, funcname, linenr); + if (dom) + virObjectLock(dom); + + return ret; +} + +#define waitDomainJob(job, dom) \ + waitDomainJobHelper(job, dom, JOB_INFINIT_WAIT_TIMEOUT, __FILE__, \ + __FUNCTION__, __LINE__) + typedef PRL_RESULT (*prlsdkParamGetterType)(PRL_HANDLE, char*, PRL_UINT32*); static char* @@ -418,7 +459,7 @@ prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid) } static int -prlsdkGetDomainState(PRL_HANDLE sdkdom, VIRTUAL_MACHINE_STATE_PTR vmState) +prlsdkGetDomainState(virDomainObjPtr dom, PRL_HANDLE sdkdom, VIRTUAL_MACHINE_STATE_PTR vmState) { PRL_HANDLE job = PRL_INVALID_HANDLE; PRL_HANDLE result = PRL_INVALID_HANDLE; @@ -428,7 +469,7 @@ prlsdkGetDomainState(PRL_HANDLE sdkdom, VIRTUAL_MACHINE_STATE_PTR vmState) job = PrlVm_GetState(sdkdom); - if (PRL_FAILED(getJobResult(job, &result))) + if (PRL_FAILED(getDomainJobResult(job, dom, &result))) goto cleanup; pret = PrlResult_GetParamByIndex(result, 0, &vmInfo); @@ -1691,7 +1732,7 @@ prlsdkConvertBootOrder(PRL_HANDLE sdkdom, virDomainDefPtr def) /* if dom is NULL adds new domain into domain list * if dom not NULL updates given locked dom object. * - * Returned object is locked. + * Returned object is locked and referenced. */ static virDomainObjPtr @@ -1771,7 +1812,7 @@ prlsdkLoadDomain(vzDriverPtr driver, goto error; } - if (prlsdkGetDomainState(sdkdom, &domainState) < 0) + if (prlsdkGetDomainState(dom, sdkdom, &domainState) < 0) goto error; if (virDomainDefAddImplicitDevices(def) < 0) @@ -1800,7 +1841,7 @@ prlsdkLoadDomain(vzDriverPtr driver, goto error; virObjectLock(driver); - if (!(olddom = virDomainObjListFindByUUID(driver->domains, def->uuid))) + if (!(olddom = virDomainObjListFindByUUIDRef(driver->domains, def->uuid))) dom = virDomainObjListAdd(driver->domains, def, driver->xmlopt, 0, NULL); virObjectUnlock(driver); @@ -1811,6 +1852,7 @@ prlsdkLoadDomain(vzDriverPtr driver, goto error; } + virObjectRef(dom); pdom = dom->privateData; pdom->sdkdom = sdkdom; PrlHandle_AddRef(sdkdom); @@ -1863,8 +1905,8 @@ prlsdkLoadDomains(vzDriverPtr driver) pret = PrlResult_GetParamByIndex(result, i, &sdkdom); prlsdkCheckRetGoto(pret, error); - if ((dom = prlsdkLoadDomain(driver, sdkdom, NULL))) - virObjectUnlock(dom); + dom = prlsdkLoadDomain(driver, sdkdom, NULL); + virDomainObjEndAPI(&dom); PrlHandle_Free(sdkdom); sdkdom = PRL_INVALID_HANDLE; @@ -1918,7 +1960,7 @@ prlsdkUpdateDomain(vzDriverPtr driver, virDomainObjPtr dom) vzDomObjPtr pdom = dom->privateData; job = PrlVm_RefreshConfig(pdom->sdkdom); - if (waitJob(job)) + if (waitDomainJob(job, dom)) return -1; return prlsdkLoadDomain(driver, pdom->sdkdom, dom) ? 0 : -1; @@ -2015,11 +2057,16 @@ prlsdkHandleVmConfigEvent(vzDriverPtr driver, unsigned char *uuid) { virDomainObjPtr dom = NULL; + bool job = false; - dom = virDomainObjListFindByUUID(driver->domains, uuid); + dom = virDomainObjListFindByUUIDRef(driver->domains, uuid); if (dom == NULL) return; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + if (prlsdkUpdateDomain(driver, dom) < 0) goto cleanup; @@ -2027,7 +2074,9 @@ prlsdkHandleVmConfigEvent(vzDriverPtr driver, VIR_DOMAIN_EVENT_DEFINED_UPDATED); cleanup: - virObjectUnlock(dom); + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return; } @@ -2037,7 +2086,7 @@ prlsdkHandleVmAddedEvent(vzDriverPtr driver, { virDomainObjPtr dom = NULL; - if (!(dom = virDomainObjListFindByUUID(driver->domains, uuid)) && + if (!(dom = virDomainObjListFindByUUIDRef(driver->domains, uuid)) && !(dom = prlsdkAddDomainByUUID(driver, uuid))) goto cleanup; @@ -2045,8 +2094,7 @@ prlsdkHandleVmAddedEvent(vzDriverPtr driver, VIR_DOMAIN_EVENT_DEFINED_ADDED); cleanup: - if (dom) - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); return; } @@ -2245,7 +2293,9 @@ prlsdkDomainChangeStateLocked(vzDriverPtr driver, virErrorNumber virerr; pdom = dom->privateData; + virObjectUnlock(dom); pret = chstate(pdom->sdkdom); + virObjectLock(dom); if (PRL_FAILED(pret)) { virResetLastError(); @@ -2274,12 +2324,21 @@ prlsdkDomainChangeState(virDomainPtr domain, vzConnPtr privconn = domain->conn->privateData; virDomainObjPtr dom; int ret = -1; + bool job = false; - if (!(dom = vzDomObjFromDomain(domain))) + if (!(dom = vzDomObjFromDomainRef(domain))) return -1; + if (vzDomainObjBeginJob(dom) < 0) + goto cleanup; + job = true; + ret = prlsdkDomainChangeStateLocked(privconn->driver, dom, chstate); - virObjectUnlock(dom); + + cleanup: + if (job) + vzDomainObjEndJob(dom); + virDomainObjEndAPI(&dom); return ret; } @@ -3076,6 +3135,7 @@ static int prlsdkConfigureGateways(PRL_HANDLE sdknet, virDomainNetDefPtr net) } static int prlsdkConfigureNet(vzDriverPtr driver, + virDomainObjPtr dom, PRL_HANDLE sdkdom, virDomainNetDefPtr net, bool isCt, bool create) @@ -3215,7 +3275,7 @@ static int prlsdkConfigureNet(vzDriverPtr driver, job = PrlSrv_AddVirtualNetwork(driver->server, vnet, PRL_USE_VNET_NAME_FOR_BRIDGE_NAME); - if (PRL_FAILED(pret = waitJob(job))) + if (PRL_FAILED(pret = waitDomainJob(job, dom))) goto cleanup; pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET); @@ -3239,7 +3299,9 @@ static int prlsdkConfigureNet(vzDriverPtr driver, } static void -prlsdkCleanupBridgedNet(vzDriverPtr driver, virDomainNetDefPtr net) +prlsdkCleanupBridgedNet(vzDriverPtr driver, + virDomainObjPtr dom, + virDomainNetDefPtr net) { PRL_RESULT pret; PRL_HANDLE vnet = PRL_INVALID_HANDLE; @@ -3255,8 +3317,7 @@ prlsdkCleanupBridgedNet(vzDriverPtr driver, virDomainNetDefPtr net) prlsdkCheckRetGoto(pret, cleanup); job = PrlSrv_DeleteVirtualNetwork(driver->server, vnet, 0); - if (PRL_FAILED(pret = waitJob(job))) - goto cleanup; + ignore_value(waitDomainJob(job, dom)); cleanup: PrlHandle_Free(vnet); @@ -3449,7 +3510,7 @@ prlsdkAttachDevice(vzDriverPtr driver, PRL_HANDLE job = PRL_INVALID_HANDLE; job = PrlVm_BeginEdit(privdom->sdkdom); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) return -1; switch (dev->type) { @@ -3466,7 +3527,7 @@ prlsdkAttachDevice(vzDriverPtr driver, return -1; } - if (prlsdkConfigureNet(driver, privdom->sdkdom, dev->data.net, + if (prlsdkConfigureNet(driver, dom, privdom->sdkdom, dev->data.net, IS_CT(dom->def), true) < 0) return -1; @@ -3490,7 +3551,7 @@ prlsdkAttachDevice(vzDriverPtr driver, } job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) return -1; return 0; @@ -3508,7 +3569,7 @@ prlsdkDetachDevice(vzDriverPtr driver, PRL_RESULT pret; job = PrlVm_BeginEdit(privdom->sdkdom); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto cleanup; switch (dev->type) { @@ -3532,7 +3593,7 @@ prlsdkDetachDevice(vzDriverPtr driver, if (sdkdev == PRL_INVALID_HANDLE) goto cleanup; - prlsdkCleanupBridgedNet(driver, dev->data.net); + prlsdkCleanupBridgedNet(driver, dom, dev->data.net); pret = PrlVmDev_Remove(sdkdev); prlsdkCheckRetGoto(pret, cleanup); @@ -3557,7 +3618,7 @@ prlsdkDetachDevice(vzDriverPtr driver, } job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto cleanup; ret = 0; @@ -3577,7 +3638,7 @@ prlsdkUpdateDevice(vzDriverPtr driver, PRL_HANDLE job = PRL_INVALID_HANDLE; job = PrlVm_BeginEdit(privdom->sdkdom); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) return -1; switch (dev->type) { @@ -3588,7 +3649,7 @@ prlsdkUpdateDevice(vzDriverPtr driver, break; case VIR_DOMAIN_DEVICE_NET: - if (prlsdkConfigureNet(driver, privdom->sdkdom, dev->data.net, + if (prlsdkConfigureNet(driver, dom, privdom->sdkdom, dev->data.net, IS_CT(dom->def), false) < 0) return -1; @@ -3612,7 +3673,7 @@ prlsdkUpdateDevice(vzDriverPtr driver, } job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) return -1; return 0; @@ -3754,7 +3815,7 @@ prlsdkDomainSetUserPassword(virDomainObjPtr dom, PRL_HANDLE job = PRL_INVALID_HANDLE; job = PrlVm_BeginEdit(privdom->sdkdom); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto cleanup; job = PrlVm_SetUserPasswd(privdom->sdkdom, @@ -3762,11 +3823,11 @@ prlsdkDomainSetUserPassword(virDomainObjPtr dom, password, 0); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto cleanup; job = PrlVm_CommitEx(privdom->sdkdom, 0); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto cleanup; ret = 0; @@ -3777,9 +3838,9 @@ prlsdkDomainSetUserPassword(virDomainObjPtr dom, static int prlsdkDoApplyConfig(vzDriverPtr driver, + virDomainObjPtr dom, PRL_HANDLE sdkdom, - virDomainDefPtr def, - virDomainDefPtr olddef) + virDomainDefPtr def) { PRL_RESULT pret; size_t i; @@ -3840,13 +3901,13 @@ prlsdkDoApplyConfig(vzDriverPtr driver, if (prlsdkRemoveBootDevices(sdkdom) < 0) goto error; - if (olddef) { - for (i = 0; i < olddef->nnets; i++) - prlsdkCleanupBridgedNet(driver, olddef->nets[i]); + if (dom) { + for (i = 0; i < dom->def->nnets; i++) + prlsdkCleanupBridgedNet(driver, dom, dom->def->nets[i]); } for (i = 0; i < def->nnets; i++) { - if (prlsdkConfigureNet(driver, sdkdom, def->nets[i], + if (prlsdkConfigureNet(driver, dom, sdkdom, def->nets[i], IS_CT(def), true) < 0) goto error; } @@ -3894,7 +3955,7 @@ prlsdkDoApplyConfig(vzDriverPtr driver, VIR_FREE(mask); for (i = 0; i < def->nnets; i++) - prlsdkCleanupBridgedNet(driver, def->nets[i]); + prlsdkCleanupBridgedNet(driver, dom, def->nets[i]); return -1; } @@ -3909,14 +3970,14 @@ prlsdkApplyConfig(vzDriverPtr driver, int ret; job = PrlVm_BeginEdit(privdom->sdkdom); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) return -1; - ret = prlsdkDoApplyConfig(driver, privdom->sdkdom, new, dom->def); + ret = prlsdkDoApplyConfig(driver, dom, privdom->sdkdom, new); if (ret == 0) { job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) ret = -1; } @@ -3949,7 +4010,7 @@ prlsdkCreateVm(vzDriverPtr driver, virDomainDefPtr def) pret = PrlVmCfg_SetOfflineManagementEnabled(sdkdom, 0); prlsdkCheckRetGoto(pret, cleanup); - if (prlsdkDoApplyConfig(driver, sdkdom, def, NULL) < 0) + if (prlsdkDoApplyConfig(driver, NULL, sdkdom, def) < 0) goto cleanup; job = PrlVm_Reg(sdkdom, "", 1); @@ -4063,7 +4124,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) } - if (prlsdkDoApplyConfig(driver, sdkdom, def, NULL) < 0) + if (prlsdkDoApplyConfig(driver, NULL, sdkdom, def) < 0) goto cleanup; flags = PACF_NON_INTERACTIVE_MODE; @@ -4089,7 +4150,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) * Returns 0 if hard disks were successfully detached or not detected. */ static int -prlsdkDetachDomainHardDisks(PRL_HANDLE sdkdom) +prlsdkDetachDomainHardDisks(virDomainObjPtr dom) { int ret = -1; PRL_RESULT pret; @@ -4097,9 +4158,11 @@ prlsdkDetachDomainHardDisks(PRL_HANDLE sdkdom) PRL_UINT32 i; PRL_HANDLE job; PRL_HANDLE sdkdisk = PRL_INVALID_HANDLE; + vzDomObjPtr pdom = dom->privateData; + PRL_HANDLE sdkdom = pdom->sdkdom; job = PrlVm_BeginEdit(sdkdom); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto cleanup; pret = PrlVmCfg_GetHardDisksCount(sdkdom, &hddCount); @@ -4117,7 +4180,7 @@ prlsdkDetachDomainHardDisks(PRL_HANDLE sdkdom) } job = PrlVm_CommitEx(sdkdom, PVCF_DETACH_HDD_BUNDLE); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto cleanup; ret = 0; @@ -4138,7 +4201,7 @@ prlsdkUnregisterDomain(vzDriverPtr driver, virDomainObjPtr dom, unsigned int fla int ret = -1; int num; - if (prlsdkGetDomainState(privdom->sdkdom, &domainState) < 0) + if (prlsdkGetDomainState(dom, privdom->sdkdom, &domainState) < 0) return -1; if (VMS_SUSPENDED == domainState && @@ -4162,20 +4225,21 @@ prlsdkUnregisterDomain(vzDriverPtr driver, virDomainObjPtr dom, unsigned int fla goto cleanup; } - if (prlsdkDetachDomainHardDisks(privdom->sdkdom)) + if (prlsdkDetachDomainHardDisks(dom)) goto cleanup; job = PrlVm_Delete(privdom->sdkdom, PRL_INVALID_HANDLE); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto cleanup; for (i = 0; i < dom->def->nnets; i++) - prlsdkCleanupBridgedNet(driver, dom->def->nets[i]); + prlsdkCleanupBridgedNet(driver, dom, dom->def->nets[i]); prlsdkSendEvent(driver, dom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); virDomainObjListRemove(driver->domains, dom); + virObjectLock(dom); ret = 0; cleanup: @@ -4191,7 +4255,7 @@ prlsdkDomainManagedSaveRemove(virDomainObjPtr dom) PRL_HANDLE job; job = PrlVm_DropSuspendedState(privdom->sdkdom); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) return -1; return 0; @@ -4449,14 +4513,14 @@ int prlsdkSetMemsize(virDomainObjPtr dom, unsigned int memsize) PRL_RESULT pret; job = PrlVm_BeginEdit(privdom->sdkdom); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto error; pret = PrlVmCfg_SetRamSize(privdom->sdkdom, memsize); prlsdkCheckRetGoto(pret, error); job = PrlVm_CommitEx(privdom->sdkdom, 0); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto error; return 0; @@ -4626,7 +4690,7 @@ prlsdkLoadSnapshots(virDomainObjPtr dom) char *treexml = NULL; job = PrlVm_GetSnapshotsTreeEx(privdom->sdkdom, PGST_WITHOUT_SCREENSHOTS); - if (PRL_FAILED(getJobResult(job, &result))) + if (PRL_FAILED(getDomainJobResult(job, dom, &result))) goto cleanup; if (!(treexml = prlsdkGetStringParamVar(PrlResult_GetParamAsString, result))) @@ -4647,7 +4711,7 @@ int prlsdkCreateSnapshot(virDomainObjPtr dom, const char *description) job = PrlVm_CreateSnapshot(privdom->sdkdom, "", description ? : ""); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) return -1; return 0; @@ -4659,7 +4723,7 @@ int prlsdkDeleteSnapshot(virDomainObjPtr dom, const char *uuid, bool children) PRL_HANDLE job; job = PrlVm_DeleteSnapshot(privdom->sdkdom, uuid, children); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) return -1; return 0; @@ -4675,7 +4739,7 @@ int prlsdkSwitchToSnapshot(virDomainObjPtr dom, const char *uuid, bool paused) flags |= PSSF_SKIP_RESUME; job = PrlVm_SwitchToSnapshotEx(privdom->sdkdom, uuid, flags); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) return -1; return 0; @@ -4714,7 +4778,7 @@ int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri, PRL_TRUE ); - if (PRL_FAILED(waitJob(job))) + if (PRL_FAILED(waitDomainJob(job, dom))) goto cleanup; ret = 0; diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index d3427ca..3aea7a5 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -33,6 +33,7 @@ #include "virstring.h" #include "datatypes.h" #include "virlog.h" +#include "virtime.h" #define VIR_FROM_THIS VIR_FROM_PARALLELS #define PRLSRVCTL "prlsrvctl" @@ -587,9 +588,17 @@ vzDomObjAlloc(void) if (VIR_ALLOC(pdom) < 0) return NULL; + if (virCondInit(&pdom->jobCond) < 0) + goto error; + pdom->stats = PRL_INVALID_HANDLE; return pdom; + + error: + VIR_FREE(pdom); + + return NULL; } void @@ -602,5 +611,46 @@ vzDomObjFree(void* p) PrlHandle_Free(pdom->sdkdom); PrlHandle_Free(pdom->stats); + virCondDestroy(&pdom->jobCond); VIR_FREE(pdom); }; + +#define VZ_JOB_WAIT_TIME (1000 * 30) + +int +vzDomainObjBeginJob(virDomainObjPtr dom) +{ + vzDomObjPtr pdom = dom->privateData; + unsigned long long now; + unsigned long long then; + + if (virTimeMillisNow(&now) < 0) + return -1; + then = now + VZ_JOB_WAIT_TIME; + + while (pdom->job) { + if (virCondWaitUntil(&pdom->jobCond, &dom->parent.lock, then) < 0) + goto error; + } + + pdom->job = true; + return 0; + + error: + if (errno == ETIMEDOUT) + virReportError(VIR_ERR_OPERATION_TIMEOUT, + "%s", _("cannot acquire state change lock")); + else + virReportSystemError(errno, + "%s", _("cannot acquire job mutex")); + return -1; +} + +void +vzDomainObjEndJob(virDomainObjPtr dom) +{ + vzDomObjPtr pdom = dom->privateData; + + pdom->job = false; + virCondSignal(&pdom->jobCond); +} diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index 92503a7..3f6b620 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -98,6 +98,8 @@ struct vzDomObj { int id; PRL_HANDLE sdkdom; PRL_HANDLE stats; + bool job; + virCond jobCond; }; typedef struct vzDomObj *vzDomObjPtr; @@ -139,3 +141,8 @@ int vzCheckUnsupportedGraphics(virDomainGraphicsDefPtr gr); OP(wr_bytes, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES, "write_total") #endif + +int +vzDomainObjBeginJob(virDomainObjPtr dom); +void +vzDomainObjEndJob(virDomainObjPtr dom); -- 1.8.3.1
This patch is not critical but nice to have. The original motivation was error message in logs on undefining domain thru vz driver. Undefine procedure drops domain lock while waiting for detaching disks vz sdk call. Meanwhile vz sdk event domain-config-changed arrives, its handler finds domain and is blocked waiting for job condition. After undefine API call finishes event processing procedes and tries to refreshes domain config thru existing vz sdk domain handle. Domain does not exists anymore and event processing fails. Everything is fine we just don't want to see error message in log for this particular case. Fortunately domain has flag that domain is removed from list. This also imply that vz sdk domain is also undefined. Thus if we check for this flag right after domain is locked again on accuiring job condition we gracefully handle this situation. Actually the race can happen in other situations too. Any time we wait for job condition in mutualy exclusive job in time when we acquire it vz sdk domain can cease to exist. So instead of general internal error we can return domain not found which is easier to handle. We don't need to patch other places in mutually exclusive jobs where domain lock is dropped as if job is started domain can't be undefine by mutually exclusive undefine job. The code of this patch is quite similar to qemu driver checks for is domain is active after acquiring a job. The difference only while qemu domain is operational while process is active vz domain is operational while domain exists. --- src/vz/vz_driver.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ src/vz/vz_sdk.c | 13 +++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index ce43d1b..3431455 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -714,6 +714,22 @@ vzDomainGetAutostart(virDomainPtr domain, int *autostart) return 0; } +static int +vzEnsureDomainExists(virDomainObjPtr dom) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (!dom->removing) + return 0; + + virUUIDFormat(dom->def->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s' (%s)"), + uuidstr, dom->def->name); + + return -1; +} + static virDomainPtr vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) { @@ -780,6 +796,9 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + if (prlsdkApplyConfig(driver, dom, def)) goto cleanup; @@ -1010,6 +1029,9 @@ vzDomainUndefineFlags(virDomainPtr domain, goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + ret = prlsdkUnregisterDomain(privconn->driver, dom, flags); cleanup: @@ -1066,6 +1088,9 @@ vzDomainManagedSave(virDomainPtr domain, unsigned int flags) goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + state = virDomainObjGetState(dom, &reason); if (state == VIR_DOMAIN_RUNNING && (flags & VIR_DOMAIN_SAVE_PAUSED)) { @@ -1157,6 +1182,9 @@ static int vzDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + if (prlsdkAttachDevice(driver, dom, dev) < 0) goto cleanup; @@ -1206,6 +1234,9 @@ static int vzDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + if (prlsdkDetachDevice(driver, dom, dev) < 0) goto cleanup; @@ -1243,6 +1274,9 @@ vzDomainSetUserPassword(virDomainPtr domain, goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + ret = prlsdkDomainSetUserPassword(dom, user, password); cleanup: @@ -1281,6 +1315,9 @@ static int vzDomainUpdateDeviceFlags(virDomainPtr domain, goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + if (prlsdkUpdateDevice(driver, dom, dev) < 0) goto cleanup; @@ -1619,6 +1656,9 @@ static int vzDomainSetMemoryFlagsImpl(virDomainPtr domain, unsigned long memory, goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + ret = prlsdkSetMemsize(dom, memory >> 10); cleanup: @@ -2103,6 +2143,9 @@ vzDomainSnapshotCreateXML(virDomainPtr domain, goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + /* snaphot name is ignored, it will be set to auto generated by sdk uuid */ if (prlsdkCreateSnapshot(dom, def->description) < 0) goto cleanup; @@ -2164,6 +2207,9 @@ vzDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + ret = prlsdkSwitchToSnapshot(dom, snapshot->name, flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED); cleanup: @@ -2534,6 +2580,9 @@ vzDomainMigratePerformStep(virDomainPtr domain, goto cleanup; job = true; + if (vzEnsureDomainExists(dom) < 0) + goto cleanup; + if (!(vzuri = vzParseVzURI(miguri))) goto cleanup; diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 5c2a6c5..38254c0 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -2067,6 +2067,9 @@ prlsdkHandleVmConfigEvent(vzDriverPtr driver, goto cleanup; job = true; + if (dom->removing) + goto cleanup; + if (prlsdkUpdateDomain(driver, dom) < 0) goto cleanup; @@ -2333,6 +2336,16 @@ prlsdkDomainChangeState(virDomainPtr domain, goto cleanup; job = true; + if (dom->removing) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(dom->def->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s' (%s)"), + uuidstr, dom->def->name); + goto cleanup; + } + ret = prlsdkDomainChangeStateLocked(privconn->driver, dom, chstate); cleanup: -- 1.8.3.1
20-Jul-16 18:00, Nikolay Shirokovskiy пишет:
diff from original: ========================
1. include patch to check domain status on acquiring job condition 2. add patch to fix domain objects naming (update device patch series which was created before convention was enstablished sneak in) 3. squash in all quickfixes 4. rename vzDomainObjIsExist to vzEnsureDomainExists
Nikolay Shirokovskiy (5): vz: use state variable sdkdom in prlsdkApplyConfig vz: remove redundant variable in prlsdkHandleVmAddedEvent vz: keep naming convention for domain objects vz: make vz driver more responsive vz: handle gracefully races on undefining domain
src/vz/vz_driver.c | 257 +++++++++++++++++++++++++++++++++++++++-------------- src/vz/vz_sdk.c | 207 ++++++++++++++++++++++++++++-------------- src/vz/vz_utils.c | 50 +++++++++++ src/vz/vz_utils.h | 7 ++ 4 files changed, 387 insertions(+), 134 deletions(-)
ACK the series. Thanks a lot. It really makes things much more better and user friendly. Pushed now. Maxim
participants (2)
-
Maxim Nestratov -
Nikolay Shirokovskiy