[libvirt] [PATCH 0/8] lxc: add job support
This patch series adds job support to the lxc driver, using techiques from the libxl driver. One benefit is no longer blocking get operations during long running modify operations. E.g. with these patches 'vish dominfo dom' will work while 'virsh save dom ...' is in progress. The first patch adds the job support machinery, followed by several patches that make use of it. Although this might look not needed that much right now, it is preparing environment for future work. This patch series has been reviewed by Michal Privoznik who is my mentor for GSoC 2016. Katerina Koukiou (8): lxc: Add job support to lxc driver lxc: use job functions in lxcDomain{CreateXMLWithFiles, CreateWithFiles} lxc: use job functions in lxcDomainSetMemoryFlags lxc: use job functions in lxcDomain{Suspend, Resume} lxc: use job functions in lxcDomain{AttachDeviceFlags, DetachDeviceFlags, UpdateDeviceFlags} lxc: add job functions in lxcDomainSetAutostart lxc: use job functions in lxcDomain* functions that do query operations. lxc: use job functions in lxcDomain* functions that perform modify actions. src/lxc/lxc_domain.c | 154 ++++++++++++++++++++-- src/lxc/lxc_domain.h | 37 ++++++ src/lxc/lxc_driver.c | 351 ++++++++++++++++++++++++++++++++++++--------------- 3 files changed, 428 insertions(+), 114 deletions(-) -- 2.7.4
Follows the pattern used in the libxl driver for managing multiple, simultaneous jobs within the driver. Signed-off-by: Katerina Koukiou <k.koukiou@gmail.com> --- src/lxc/lxc_domain.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++---- src/lxc/lxc_domain.h | 37 +++++++++++++ 2 files changed, 180 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 3177a62..bca2bb2 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -30,22 +30,164 @@ #include "virstring.h" #include "virutil.h" #include "virfile.h" +#include "virtime.h" #define VIR_FROM_THIS VIR_FROM_LXC #define LXC_NAMESPACE_HREF "http://libvirt.org/schemas/domain/lxc/1.0" +VIR_ENUM_IMPL(virLXCDomainJob, LXC_JOB_LAST, + "none", + "query", + "destroy", + "modify", +); + VIR_LOG_INIT("lxc.lxc_domain"); -static void *virLXCDomainObjPrivateAlloc(void) +static int +virLXCDomainObjInitJob(virLXCDomainObjPrivatePtr priv) +{ + memset(&priv->job, 0, sizeof(priv->job)); + + if (virCondInit(&priv->job.cond) < 0) + return -1; + + return 0; +} + +static void +virLXCDomainObjResetJob(virLXCDomainObjPrivatePtr priv) +{ + struct virLXCDomainJobObj *job = &priv->job; + + job->active = LXC_JOB_NONE; + job->owner = 0; +} + +static void +virLXCDomainObjFreeJob(virLXCDomainObjPrivatePtr priv) +{ + ignore_value(virCondDestroy(&priv->job.cond)); +} + +/* Give up waiting for mutex after 30 seconds */ +#define LXC_JOB_WAIT_TIME (1000ull * 30) + +/* + * obj must be locked before calling, virLXCDriverPtr must NOT be locked + * + * This must be called by anything that will change the VM state + * in any way + * + * Upon successful return, the object will have its ref count increased, + * successful calls must be followed by EndJob eventually + */ +int +virLXCDomainObjBeginJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr obj, + enum virLXCDomainJob job) +{ + virLXCDomainObjPrivatePtr priv = obj->privateData; + unsigned long long now; + unsigned long long then; + + if (virTimeMillisNow(&now) < 0) + return -1; + then = now + LXC_JOB_WAIT_TIME; + + virObjectRef(obj); + + while (priv->job.active) { + VIR_DEBUG("Wait normal job condition for starting job: %s", + virLXCDomainJobTypeToString(job)); + if (virCondWaitUntil(&priv->job.cond, &obj->parent.lock, then) < 0) + goto error; + } + + virLXCDomainObjResetJob(priv); + + VIR_DEBUG("Starting job: %s", virLXCDomainJobTypeToString(job)); + priv->job.active = job; + priv->job.owner = virThreadSelfID(); + + return 0; + + error: + VIR_WARN("Cannot start job (%s) for domain %s;" + " current job is (%s) owned by (%d)", + virLXCDomainJobTypeToString(job), + obj->def->name, + virLXCDomainJobTypeToString(priv->job.active), + priv->job.owner); + + if (errno == ETIMEDOUT) + virReportError(VIR_ERR_OPERATION_TIMEOUT, + "%s", _("cannot acquire state change lock")); + else + virReportSystemError(errno, + "%s", _("cannot acquire job mutex")); + + virObjectUnref(obj); + return -1; +} + + +/* + * obj must be locked before calling + * + * To be called after completing the work associated with the + * earlier virLXCDomainBeginJob() call + * + * Returns true if the remaining reference count on obj is + * non-zero, false if the reference count has dropped to zero + * and obj is disposed. + */ +bool +virLXCDomainObjEndJob(virLXCDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr obj) +{ + virLXCDomainObjPrivatePtr priv = obj->privateData; + enum virLXCDomainJob job = priv->job.active; + + VIR_DEBUG("Stopping job: %s", + virLXCDomainJobTypeToString(job)); + + virLXCDomainObjResetJob(priv); + virCondSignal(&priv->job.cond); + + return virObjectUnref(obj); +} + + +static void * +virLXCDomainObjPrivateAlloc(void) { virLXCDomainObjPrivatePtr priv; if (VIR_ALLOC(priv) < 0) return NULL; + if (virLXCDomainObjInitJob(priv) < 0) { + VIR_FREE(priv); + return NULL; + } + return priv; } + +static void +virLXCDomainObjPrivateFree(void *data) +{ + virLXCDomainObjPrivatePtr priv = data; + + virCgroupFree(&priv->cgroup); + virLXCDomainObjFreeJob(priv); + VIR_FREE(priv); +} + + + VIR_ENUM_IMPL(virLXCDomainNamespace, VIR_LXC_DOMAIN_NAMESPACE_LAST, "sharenet", @@ -190,16 +332,6 @@ virDomainXMLNamespace virLXCDriverDomainXMLNamespace = { }; -static void virLXCDomainObjPrivateFree(void *data) -{ - virLXCDomainObjPrivatePtr priv = data; - - virCgroupFree(&priv->cgroup); - - VIR_FREE(priv); -} - - static int virLXCDomainObjPrivateXMLFormat(virBufferPtr buf, virDomainObjPtr vm) diff --git a/src/lxc/lxc_domain.h b/src/lxc/lxc_domain.h index 39c6e7d..e82c719 100644 --- a/src/lxc/lxc_domain.h +++ b/src/lxc/lxc_domain.h @@ -27,6 +27,7 @@ # include "lxc_conf.h" # include "lxc_monitor.h" + typedef enum { VIR_LXC_DOMAIN_NAMESPACE_SHARENET = 0, VIR_LXC_DOMAIN_NAMESPACE_SHAREIPC, @@ -53,6 +54,28 @@ struct _lxcDomainDef { char *ns_val[VIR_LXC_DOMAIN_NAMESPACE_LAST]; }; + +/* Only 1 job is allowed at any time + * A job includes *all* lxc.so api, even those just querying + * information, not merely actions */ + +enum virLXCDomainJob { + LXC_JOB_NONE = 0, /* Always set to 0 for easy if (jobActive) conditions */ + LXC_JOB_QUERY, /* Doesn't change any state */ + LXC_JOB_DESTROY, /* Destroys the domain (cannot be masked out) */ + LXC_JOB_MODIFY, /* May change state */ + LXC_JOB_LAST +}; +VIR_ENUM_DECL(virLXCDomainJob) + + +struct virLXCDomainJobObj { + virCond cond; /* Use to coordinate jobs */ + enum virLXCDomainJob active; /* Currently running job */ + int owner; /* Thread which set current job */ +}; + + typedef struct _virLXCDomainObjPrivate virLXCDomainObjPrivate; typedef virLXCDomainObjPrivate *virLXCDomainObjPrivatePtr; struct _virLXCDomainObjPrivate { @@ -65,10 +88,24 @@ struct _virLXCDomainObjPrivate { virCgroupPtr cgroup; char *machineName; + + struct virLXCDomainJobObj job; }; extern virDomainXMLNamespace virLXCDriverDomainXMLNamespace; extern virDomainXMLPrivateDataCallbacks virLXCDriverPrivateDataCallbacks; extern virDomainDefParserConfig virLXCDriverDomainDefParserConfig; +int +virLXCDomainObjBeginJob(virLXCDriverPtr driver, + virDomainObjPtr obj, + enum virLXCDomainJob job) + ATTRIBUTE_RETURN_CHECK; + +bool +virLXCDomainObjEndJob(virLXCDriverPtr driver, + virDomainObjPtr obj) + ATTRIBUTE_RETURN_CHECK; + + #endif /* __LXC_DOMAIN_H__ */ -- 2.7.4
Creating a large domain could potentially be time consuming. Use the recently added job functions and unlock the virDomainObj while the create operation is in progress. Signed-off-by: Katerina Koukiou <k.koukiou@gmail.com> --- src/lxc/lxc_driver.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index a226850..1e21ee4 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1127,10 +1127,13 @@ static int lxcDomainCreateWithFiles(virDomainPtr dom, goto cleanup; } + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + if (virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is already running")); - goto cleanup; + goto endjob; } ret = virLXCProcessStart(dom->conn, driver, vm, @@ -1147,6 +1150,10 @@ static int lxcDomainCreateWithFiles(virDomainPtr dom, virDomainAuditStart(vm, "booted", false); } + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -1249,6 +1256,14 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, goto cleanup; def = NULL; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) { + if (!vm->persistent) { + virDomainObjListRemove(driver->domains, vm); + vm = NULL; + } + goto cleanup; + } + if (virLXCProcessStart(conn, driver, vm, nfiles, files, (flags & VIR_DOMAIN_START_AUTODESTROY), @@ -1258,7 +1273,7 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, virDomainObjListRemove(driver->domains, vm); vm = NULL; } - goto cleanup; + goto endjob; } event = virDomainEventLifecycleNewFromObj(vm, @@ -1270,6 +1285,10 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, if (dom) dom->id = vm->def->id; + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: virDomainDefFree(def); if (vm) -- 2.7.4
Large balloon operation can be time consuming. Use the recently added job functions and unlock the virDomainObj while ballooning. Signed-off-by: Katerina Koukiou <k.koukiou@gmail.com> --- src/lxc/lxc_driver.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1e21ee4..863c799 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -725,19 +725,22 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (virDomainSetMemoryFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; - if (!(caps = virLXCDriverGetCapabilities(driver, false))) + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) goto cleanup; + if (!(caps = virLXCDriverGetCapabilities(driver, false))) + goto endjob; + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, &persistentDef) < 0) - goto cleanup; + goto endjob; if (flags & VIR_DOMAIN_MEM_MAXIMUM) { if (flags & VIR_DOMAIN_AFFECT_LIVE) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Cannot resize the max memory " "on an active domain")); - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { @@ -746,7 +749,7 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, persistentDef->mem.cur_balloon = newmem; if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0) - goto cleanup; + goto endjob; } } else { unsigned long oldmax = 0; @@ -761,31 +764,35 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (newmem > oldmax) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Cannot set memory higher than max memory")); - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (virCgroupSetMemory(priv->cgroup, newmem) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Failed to set memory for domain")); - goto cleanup; + goto endjob; } vm->def->mem.cur_balloon = newmem; if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { persistentDef->mem.cur_balloon = newmem; if (virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0) - goto cleanup; + goto endjob; } } ret = 0; + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); -- 2.7.4
These operations aren't necessarily time consuming, but need to wait in the queue of modify jobs. Signed-off-by: Katerina Koukiou <k.koukiou@gmail.com> --- src/lxc/lxc_driver.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 863c799..179dc35 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3416,17 +3416,20 @@ static int lxcDomainSuspend(virDomainPtr dom) if (virDomainSuspendEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { if (lxcFreezeContainer(vm) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Suspend operation failed")); - goto cleanup; + goto endjob; } virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER); @@ -3436,9 +3439,12 @@ static int lxcDomainSuspend(virDomainPtr dom) } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - goto cleanup; + goto endjob; ret = 0; + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; cleanup: if (event) virObjectEventStateQueue(driver->domainEventState, event); @@ -3465,17 +3471,20 @@ static int lxcDomainResume(virDomainPtr dom) if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { if (virCgroupSetFreezerState(priv->cgroup, "THAWED") < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Resume operation failed")); - goto cleanup; + goto endjob; } virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNPAUSED); @@ -3486,9 +3495,12 @@ static int lxcDomainResume(virDomainPtr dom) } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - goto cleanup; + goto endjob; ret = 0; + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; cleanup: if (event) virObjectEventStateQueue(driver->domainEventState, event); -- 2.7.4
These operations aren't necessarily time consuming, but need to wait in the queue of modify jobs. Signed-off-by: Katerina Koukiou <k.koukiou@gmail.com> --- src/lxc/lxc_driver.c | 77 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 30 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 179dc35..565c192 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5055,17 +5055,20 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom, if (virDomainAttachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; - if (!(caps = virLXCDriverGetCapabilities(driver, false))) + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) goto cleanup; + if (!(caps = virLXCDriverGetCapabilities(driver, false))) + goto endjob; + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) - goto cleanup; + goto endjob; dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE); if (dev == NULL) - goto cleanup; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_CONFIG && flags & VIR_DOMAIN_AFFECT_LIVE) { @@ -5076,30 +5079,30 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom, dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt); if (!dev_copy) - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt); if (!vmdef) - goto cleanup; + goto endjob; if (virDomainDefCompatibleDevice(vmdef, dev, VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - goto cleanup; + goto endjob; if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev)) < 0) - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (virDomainDefCompatibleDevice(vm->def, dev_copy, VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0) - goto cleanup; + goto endjob; if ((ret = lxcDomainAttachDeviceLive(dom->conn, driver, vm, dev_copy)) < 0) - goto cleanup; + goto endjob; /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, @@ -5107,7 +5110,7 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom, */ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) { ret = -1; - goto cleanup; + goto endjob; } } @@ -5120,6 +5123,9 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom, } } + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; cleanup: virDomainDefFree(vmdef); if (dev != dev_copy) @@ -5163,17 +5169,20 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom, if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; - if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) goto cleanup; + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) + goto endjob; + if (!(caps = virLXCDriverGetCapabilities(driver, false))) - goto cleanup; + goto endjob; dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE); if (dev == NULL) - goto cleanup; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_CONFIG && flags & VIR_DOMAIN_AFFECT_LIVE) { @@ -5184,32 +5193,32 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom, dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt); if (!dev_copy) - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt); if (!vmdef) - goto cleanup; + goto endjob; if (virDomainDefCompatibleDevice(vmdef, dev, VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - goto cleanup; + goto endjob; if ((ret = lxcDomainUpdateDeviceConfig(vmdef, dev)) < 0) - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (virDomainDefCompatibleDevice(vm->def, dev_copy, VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0) - goto cleanup; + goto endjob; virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Unable to modify live devices")); - goto cleanup; + goto endjob; } /* Finally, if no error until here, we can save config. */ @@ -5220,7 +5229,9 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom, vmdef = NULL; } } - + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; cleanup: virDomainDefFree(vmdef); if (dev != dev_copy) @@ -5255,17 +5266,20 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom, if (virDomainDetachDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; - if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) goto cleanup; + if (virDomainObjUpdateModificationImpact(vm, &flags) < 0) + goto endjob; + if (!(caps = virLXCDriverGetCapabilities(driver, false))) - goto cleanup; + goto endjob; dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, caps, driver->xmlopt, VIR_DOMAIN_DEF_PARSE_INACTIVE); if (dev == NULL) - goto cleanup; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_CONFIG && flags & VIR_DOMAIN_AFFECT_LIVE) { @@ -5276,30 +5290,30 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom, dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt); if (!dev_copy) - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt); if (!vmdef) - goto cleanup; + goto endjob; if (virDomainDefCompatibleDevice(vmdef, dev, VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - goto cleanup; + goto endjob; if ((ret = lxcDomainDetachDeviceConfig(vmdef, dev)) < 0) - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (virDomainDefCompatibleDevice(vm->def, dev_copy, VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0) - goto cleanup; + goto endjob; if ((ret = lxcDomainDetachDeviceLive(driver, vm, dev_copy)) < 0) - goto cleanup; + goto endjob; /* * update domain status forcibly because the domain status may be * changed even if we failed to attach the device. For example, @@ -5307,7 +5321,7 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom, */ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) { ret = -1; - goto cleanup; + goto endjob; } } @@ -5320,6 +5334,9 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom, } } + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; cleanup: virDomainDefFree(vmdef); if (dev != dev_copy) -- 2.7.4
This operation isn't necessarily time consuming, but need to wait in the queue of modify jobs. Signed-off-by: Katerina Koukiou <k.koukiou@gmail.com> --- src/lxc/lxc_driver.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 565c192..af6603c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3259,54 +3259,60 @@ static int lxcDomainSetAutostart(virDomainPtr dom, if (virDomainSetAutostartEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + if (!vm->persistent) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Cannot set autostart for transient domain")); - goto cleanup; + goto endjob; } autostart = (autostart != 0); if (vm->autostart == autostart) { ret = 0; - goto cleanup; + goto endjob; } configFile = virDomainConfigFile(cfg->configDir, vm->def->name); if (configFile == NULL) - goto cleanup; + goto endjob; autostartLink = virDomainConfigFile(cfg->autostartDir, vm->def->name); if (autostartLink == NULL) - goto cleanup; + goto endjob; if (autostart) { if (virFileMakePath(cfg->autostartDir) < 0) { virReportSystemError(errno, _("Cannot create autostart directory %s"), cfg->autostartDir); - goto cleanup; + goto endjob; } if (symlink(configFile, autostartLink) < 0) { virReportSystemError(errno, _("Failed to create symlink '%s to '%s'"), autostartLink, configFile); - goto cleanup; + goto endjob; } } else { if (unlink(autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { virReportSystemError(errno, _("Failed to delete symlink '%s'"), autostartLink); - goto cleanup; + goto endjob; } } vm->autostart = autostart; ret = 0; + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; cleanup: VIR_FREE(configFile); VIR_FREE(autostartLink); -- 2.7.4
This commit affects lxcDomain{InterfaceStats, MemoryStats, BlockStats, BlockStatsFlags} Signed-off-by: Katerina Koukiou <k.koukiou@gmail.com> --- src/lxc/lxc_driver.c | 70 ++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index af6603c..2d610f4 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2380,6 +2380,7 @@ lxcDomainBlockStats(virDomainPtr dom, const char *path, virDomainBlockStatsPtr stats) { + virLXCDriverPtr driver = dom->conn->privateData; int ret = -1; virDomainObjPtr vm; virDomainDiskDefPtr disk = NULL; @@ -2393,16 +2394,19 @@ lxcDomainBlockStats(virDomainPtr dom, if (virDomainBlockStatsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); - goto cleanup; + goto endjob; } if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("blkio cgroup isn't mounted")); - goto cleanup; + goto endjob; } if (!*path) { @@ -2412,19 +2416,19 @@ lxcDomainBlockStats(virDomainPtr dom, &stats->wr_bytes, &stats->rd_req, &stats->wr_req); - goto cleanup; + goto endjob; } if (!(disk = virDomainDiskByName(vm->def, path, false))) { virReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); - goto cleanup; + goto endjob; } if (!disk->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, _("missing disk device alias name for %s"), disk->dst); - goto cleanup; + goto endjob; } ret = virCgroupGetBlkioIoDeviceServiced(priv->cgroup, @@ -2433,6 +2437,11 @@ lxcDomainBlockStats(virDomainPtr dom, &stats->wr_bytes, &stats->rd_req, &stats->wr_req); + + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -2447,6 +2456,7 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, int * nparams, unsigned int flags) { + virLXCDriverPtr driver = dom->conn->privateData; int tmp, ret = -1; virDomainObjPtr vm; virDomainDiskDefPtr disk = NULL; @@ -2472,16 +2482,19 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, if (virDomainBlockStatsFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); - goto cleanup; + goto endjob; } if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("blkio cgroup isn't mounted")); - goto cleanup; + goto endjob; } if (!*path) { @@ -2493,19 +2506,19 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, &wr_req) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain stats query failed")); - goto cleanup; + goto endjob; } } else { if (!(disk = virDomainDiskByName(vm->def, path, false))) { virReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); - goto cleanup; + goto endjob; } if (!disk->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, _("missing disk device alias name for %s"), disk->dst); - goto cleanup; + goto endjob; } if (virCgroupGetBlkioIoDeviceServiced(priv->cgroup, @@ -2516,7 +2529,7 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, &wr_req) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain stats query failed")); - goto cleanup; + goto endjob; } } @@ -2527,7 +2540,7 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, param = ¶ms[tmp]; if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES, VIR_TYPED_PARAM_LLONG, wr_bytes) < 0) - goto cleanup; + goto endjob; tmp++; } @@ -2535,7 +2548,7 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, param = ¶ms[tmp]; if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ, VIR_TYPED_PARAM_LLONG, wr_req) < 0) - goto cleanup; + goto endjob; tmp++; } @@ -2543,7 +2556,7 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, param = ¶ms[tmp]; if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_BYTES, VIR_TYPED_PARAM_LLONG, rd_bytes) < 0) - goto cleanup; + goto endjob; tmp++; } @@ -2551,13 +2564,17 @@ lxcDomainBlockStatsFlags(virDomainPtr dom, param = ¶ms[tmp]; if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_REQ, VIR_TYPED_PARAM_LLONG, rd_req) < 0) - goto cleanup; + goto endjob; tmp++; } ret = 0; *nparams = tmp; + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -3179,6 +3196,7 @@ lxcDomainInterfaceStats(virDomainPtr dom, virDomainObjPtr vm; size_t i; int ret = -1; + virLXCDriverPtr driver = dom->conn->privateData; if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; @@ -3186,10 +3204,13 @@ lxcDomainInterfaceStats(virDomainPtr dom, if (virDomainInterfaceStatsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } /* Check the path is one of the domain's network interfaces. */ @@ -3207,6 +3228,10 @@ lxcDomainInterfaceStats(virDomainPtr dom, virReportError(VIR_ERR_INVALID_ARG, _("Invalid path, '%s' is not a known interface"), path); + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -5454,6 +5479,7 @@ lxcDomainMemoryStats(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; unsigned long long swap_usage; unsigned long mem_usage; + virLXCDriverPtr driver = dom->conn->privateData; virCheckFlags(0, -1); @@ -5465,17 +5491,20 @@ lxcDomainMemoryStats(virDomainPtr dom, if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not active")); - goto cleanup; + goto endjob; } if (virCgroupGetMemSwapUsage(priv->cgroup, &swap_usage) < 0) - goto cleanup; + goto endjob; if (virCgroupGetMemoryUsage(priv->cgroup, &mem_usage) < 0) - goto cleanup; + goto endjob; ret = 0; if (ret < nr_stats) { @@ -5494,6 +5523,9 @@ lxcDomainMemoryStats(virDomainPtr dom, ret++; } + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; cleanup: if (vm) virObjectUnlock(vm); -- 2.7.4
Use the recently added job functions and unlock the virDomainObj while performing the respective modify operation. This commit affects lxcDomain{DestroyFlags, Reboot, SetBlkioParameters, SetMemoryParameters, SetMetadata, SetSchedulerParameterFlags, ShutdownFlags} Signed-off-by: Katerina Koukiou <k.koukiou@gmail.com> --- src/lxc/lxc_driver.c | 114 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 83 insertions(+), 31 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 2d610f4..7cdea2c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -857,16 +857,19 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, vm, &flags, &vmdef) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + if (flags & VIR_DOMAIN_AFFECT_LIVE && !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup memory controller is not mounted")); - goto cleanup; + goto endjob; } #define VIR_GET_LIMIT_PARAMETER(PARAM, VALUE) \ if ((rc = virTypedParamsGetULLong(params, nparams, PARAM, &VALUE)) < 0) \ - goto cleanup; \ + goto endjob; \ \ if (rc == 1) \ set_ ## VALUE = true; @@ -893,24 +896,24 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, virReportError(VIR_ERR_INVALID_ARG, "%s", _("memory hard_limit tunable value must be lower " "than or equal to swap_hard_limit")); - goto cleanup; + goto endjob; } } -#define LXC_SET_MEM_PARAMETER(FUNC, VALUE) \ +#define LXC_SET_MEM_PARAMETER(FUNC, VALUE) \ if (set_ ## VALUE) { \ if (flags & VIR_DOMAIN_AFFECT_LIVE) { \ if ((rc = FUNC(priv->cgroup, VALUE)) < 0) { \ virReportSystemError(-rc, _("unable to set memory %s tunable"), \ #VALUE); \ \ - goto cleanup; \ + goto endjob; \ } \ vm->def->mem.VALUE = VALUE; \ } \ \ if (flags & VIR_DOMAIN_AFFECT_CONFIG) \ - vmdef->mem.VALUE = VALUE; \ + vmdef->mem.VALUE = VALUE; \ } /* Soft limit doesn't clash with the others */ @@ -932,9 +935,14 @@ lxcDomainSetMemoryParameters(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG && virDomainSaveConfig(cfg->configDir, driver->caps, vmdef) < 0) - goto cleanup; + goto endjob; ret = 0; + + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -1538,10 +1546,13 @@ lxcDomainDestroyFlags(virDomainPtr dom, if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } priv = vm->privateData; @@ -1556,6 +1567,10 @@ lxcDomainDestroyFlags(virDomainPtr dom, vm = NULL; } + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -1989,22 +2004,25 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom, if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, &vmdef) < 0) - goto cleanup; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt); if (!vmdef) - goto cleanup; + goto endjob; } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); - goto cleanup; + goto endjob; } } @@ -2015,10 +2033,10 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_LIVE) { unsigned long long val; if (virCgroupSetCpuShares(priv->cgroup, params[i].value.ul) < 0) - goto cleanup; + goto endjob; if (virCgroupGetCpuShares(priv->cgroup, &val) < 0) - goto cleanup; + goto endjob; vm->def->cputune.shares = val; vm->def->cputune.sharesSpecified = true; @@ -2032,7 +2050,7 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = lxcSetVcpuBWLive(priv->cgroup, params[i].value.ul, 0); if (rc != 0) - goto cleanup; + goto endjob; if (params[i].value.ul) vm->def->cputune.period = params[i].value.ul; @@ -2044,7 +2062,7 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = lxcSetVcpuBWLive(priv->cgroup, 0, params[i].value.l); if (rc != 0) - goto cleanup; + goto endjob; if (params[i].value.l) vm->def->cputune.quota = params[i].value.l; @@ -2056,13 +2074,13 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom, } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - goto cleanup; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_CONFIG) { rc = virDomainSaveConfig(cfg->configDir, driver->caps, vmdef); if (rc < 0) - goto cleanup; + goto endjob; virDomainObjAssignDef(vm, vmdef, false, NULL); vmdef = NULL; @@ -2070,6 +2088,10 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom, ret = 0; + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: virDomainDefFree(vmdef); if (vm) @@ -2627,15 +2649,18 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, &persistentDef) < 0) - goto cleanup; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("blkio cgroup isn't mounted")); - goto cleanup; + goto endjob; } } @@ -2745,7 +2770,7 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, } } if (ret < 0) - goto cleanup; + goto endjob; if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Clang can't see that if we get here, persistentDef was set. */ sa_assert(persistentDef); @@ -2783,6 +2808,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, ret = -1; } + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -3712,6 +3741,7 @@ static int lxcDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { + virLXCDriverPtr driver = dom->conn->privateData; virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; int ret = -1; @@ -3728,16 +3758,19 @@ lxcDomainShutdownFlags(virDomainPtr dom, if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } if (priv->initpid == 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Init process ID is not yet known")); - goto cleanup; + goto endjob; } if (flags == 0 || @@ -3747,12 +3780,12 @@ lxcDomainShutdownFlags(virDomainPtr dom, if ((rc = virProcessRunInMountNamespace(priv->initpid, lxcDomainInitctlCallback, &command)) < 0) - goto cleanup; + goto endjob; if (rc == 0 && flags != 0 && ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Container does not provide an initctl pipe")); - goto cleanup; + goto endjob; } } else { rc = 0; @@ -3766,12 +3799,16 @@ lxcDomainShutdownFlags(virDomainPtr dom, virReportSystemError(errno, _("Unable to send SIGTERM to init pid %llu"), (unsigned long long)priv->initpid); - goto cleanup; + goto endjob; } } ret = 0; + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -3789,6 +3826,7 @@ static int lxcDomainReboot(virDomainPtr dom, unsigned int flags) { + virLXCDriverPtr driver = dom->conn->privateData; virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; int ret = -1; @@ -3805,16 +3843,19 @@ lxcDomainReboot(virDomainPtr dom, if (virDomainRebootEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not running")); - goto cleanup; + goto endjob; } if (priv->initpid == 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Init process ID is not yet known")); - goto cleanup; + goto endjob; } if (flags == 0 || @@ -3824,12 +3865,12 @@ lxcDomainReboot(virDomainPtr dom, if ((rc = virProcessRunInMountNamespace(priv->initpid, lxcDomainInitctlCallback, &command)) < 0) - goto cleanup; + goto endjob; if (rc == 0 && flags != 0 && ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Container does not provide an initctl pipe")); - goto cleanup; + goto endjob; } } else { rc = 0; @@ -3843,12 +3884,16 @@ lxcDomainReboot(virDomainPtr dom, virReportSystemError(errno, _("Unable to send SIGTERM to init pid %llu"), (unsigned long long)priv->initpid); - goto cleanup; + goto endjob; } } ret = 0; + endjob: + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: if (vm) virObjectUnlock(vm); @@ -5669,12 +5714,19 @@ lxcDomainSetMetadata(virDomainPtr dom, if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto cleanup; + if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup; + ret = virDomainObjSetMetadata(vm, type, metadata, key, uri, caps, driver->xmlopt, cfg->stateDir, cfg->configDir, flags); + if (!virLXCDomainObjEndJob(driver, vm)) + vm = NULL; + cleanup: - virObjectUnlock(vm); + if (vm) + virObjectUnlock(vm); virObjectUnref(caps); virObjectUnref(cfg); return ret; -- 2.7.4
On Tue, May 17, 2016 at 12:36:54AM +0300, Katerina Koukiou wrote:
Use the recently added job functions and unlock the virDomainObj while performing the respective modify operation. This commit affects lxcDomain{DestroyFlags, Reboot, SetBlkioParameters, SetMemoryParameters, SetMetadata, SetSchedulerParameterFlags, ShutdownFlags}
Signed-off-by: Katerina Koukiou <k.koukiou@gmail.com> --- src/lxc/lxc_driver.c | 114 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 83 insertions(+), 31 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 2d610f4..7cdea2c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -857,16 +857,19 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
Adding extra context: if (virDomainSetMemoryParametersEnsureACL(dom->conn, vm->def, flags) < 0 || !(caps = virLXCDriverGetCapabilities(driver, false)) || virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
vm, &flags, &vmdef) < 0) goto cleanup;
+ if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) + goto cleanup;
This should be called between the virDomainSetMemoryParametersEnsureACL and virDomainLiveConfigHelperMethod calls above. In virDomainLiveConfigHelperMethod, there is a check to see if the domain is active when AFFECT_LIVE is set. Since virLXCDomainObjBeginJob unlocks the virDomainObjPtr lock, the domain could possibly be destroyed while we wait for the job and the check results would no longer be valid. (Found while trying to remove the remaining uses of virDomainLiveConfigHelperMethod) Jan
+ if (flags & VIR_DOMAIN_AFFECT_LIVE && !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup memory controller is not mounted"));
On 16.05.2016 23:36, Katerina Koukiou wrote:
This patch series adds job support to the lxc driver, using techiques from the libxl driver. One benefit is no longer blocking get operations during long running modify operations. E.g. with these patches 'vish dominfo dom' will work while 'virsh save dom ...' is in progress.
The first patch adds the job support machinery, followed by several patches that make use of it. Although this might look not needed that much right now, it is preparing environment for future work.
This patch series has been reviewed by Michal Privoznik who is my mentor for GSoC 2016.
Katerina Koukiou (8): lxc: Add job support to lxc driver lxc: use job functions in lxcDomain{CreateXMLWithFiles, CreateWithFiles} lxc: use job functions in lxcDomainSetMemoryFlags lxc: use job functions in lxcDomain{Suspend, Resume} lxc: use job functions in lxcDomain{AttachDeviceFlags, DetachDeviceFlags, UpdateDeviceFlags} lxc: add job functions in lxcDomainSetAutostart lxc: use job functions in lxcDomain* functions that do query operations. lxc: use job functions in lxcDomain* functions that perform modify actions.
src/lxc/lxc_domain.c | 154 ++++++++++++++++++++-- src/lxc/lxc_domain.h | 37 ++++++ src/lxc/lxc_driver.c | 351 ++++++++++++++++++++++++++++++++++++--------------- 3 files changed, 428 insertions(+), 114 deletions(-)
ACK series, although, I'll leave others time to chime in before pushing. Michal
On 18.05.2016 16:53, Michal Privoznik wrote:
On 16.05.2016 23:36, Katerina Koukiou wrote:
This patch series adds job support to the lxc driver, using techiques from the libxl driver. One benefit is no longer blocking get operations during long running modify operations. E.g. with these patches 'vish dominfo dom' will work while 'virsh save dom ...' is in progress.
The first patch adds the job support machinery, followed by several patches that make use of it. Although this might look not needed that much right now, it is preparing environment for future work.
This patch series has been reviewed by Michal Privoznik who is my mentor for GSoC 2016.
Katerina Koukiou (8): lxc: Add job support to lxc driver lxc: use job functions in lxcDomain{CreateXMLWithFiles, CreateWithFiles} lxc: use job functions in lxcDomainSetMemoryFlags lxc: use job functions in lxcDomain{Suspend, Resume} lxc: use job functions in lxcDomain{AttachDeviceFlags, DetachDeviceFlags, UpdateDeviceFlags} lxc: add job functions in lxcDomainSetAutostart lxc: use job functions in lxcDomain* functions that do query operations. lxc: use job functions in lxcDomain* functions that perform modify actions.
src/lxc/lxc_domain.c | 154 ++++++++++++++++++++-- src/lxc/lxc_domain.h | 37 ++++++ src/lxc/lxc_driver.c | 351 ++++++++++++++++++++++++++++++++++++--------------- 3 files changed, 428 insertions(+), 114 deletions(-)
ACK series, although, I'll leave others time to chime in before pushing.
Okay, nobody objected, so I've pushed these. Congratulations on your first libvirt contribution! Michal
On 05/19/2016 12:48 PM, Michal Privoznik wrote:
On 18.05.2016 16:53, Michal Privoznik wrote:
On 16.05.2016 23:36, Katerina Koukiou wrote:
This patch series adds job support to the lxc driver, using techiques from the libxl driver. One benefit is no longer blocking get operations during long running modify operations. E.g. with these patches 'vish dominfo dom' will work while 'virsh save dom ...' is in progress.
The first patch adds the job support machinery, followed by several patches that make use of it. Although this might look not needed that much right now, it is preparing environment for future work.
This patch series has been reviewed by Michal Privoznik who is my mentor for GSoC 2016.
Katerina Koukiou (8): lxc: Add job support to lxc driver lxc: use job functions in lxcDomain{CreateXMLWithFiles, CreateWithFiles} lxc: use job functions in lxcDomainSetMemoryFlags lxc: use job functions in lxcDomain{Suspend, Resume} lxc: use job functions in lxcDomain{AttachDeviceFlags, DetachDeviceFlags, UpdateDeviceFlags} lxc: add job functions in lxcDomainSetAutostart lxc: use job functions in lxcDomain* functions that do query operations. lxc: use job functions in lxcDomain* functions that perform modify actions.
src/lxc/lxc_domain.c | 154 ++++++++++++++++++++-- src/lxc/lxc_domain.h | 37 ++++++ src/lxc/lxc_driver.c | 351 ++++++++++++++++++++++++++++++++++++--------------- 3 files changed, 428 insertions(+), 114 deletions(-)
ACK series, although, I'll leave others time to chime in before pushing.
Okay, nobody objected, so I've pushed these. Congratulations on your first libvirt contribution!
I had glanced over them and they looked quite good to me. Nice work Katerina! - Cole
participants (4)
-
Cole Robinson -
Ján Tomko -
Katerina Koukiou -
Michal Privoznik