[libvirt] [PATCH 0/2] Use more of qemuDomObjFromDomain
I know we are in the freeze so I will push this after upstream is thaw. Michal Privoznik (2): qemu: Relax locking in DomainHasManagedSaveImage and DomainMonitorCommand qemu: Convert some APIs to use qemuDomObjFromDomain src/qemu/qemu_driver.c | 478 +++++------------------------------------------ 1 files changed, 51 insertions(+), 427 deletions(-) -- 1.7.8.6
There is no need to hold qemu lock during the whole execution of these two APIs. --- src/qemu/qemu_driver.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2fda44e..74e442b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3288,6 +3288,7 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); @@ -3301,7 +3302,6 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; } @@ -12676,6 +12676,7 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(domain->uuid, uuidstr); @@ -12688,9 +12689,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); goto cleanup; - } + } - if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -12705,9 +12706,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, hmp = !!(flags & VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP); - qemuDomainObjEnterMonitorWithDriver(driver, vm); + qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp); - qemuDomainObjExitMonitorWithDriver(driver, vm); + qemuDomainObjExitMonitor(driver, vm); endjob: if (qemuDomainObjEndJob(driver, vm) == 0) { @@ -12717,7 +12718,6 @@ endjob: cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; } -- 1.7.8.6
On 12/13/12 12:40, Michal Privoznik wrote:
There is no need to hold qemu lock during the whole execution of these two APIs. --- src/qemu/qemu_driver.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2fda44e..74e442b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3288,6 +3288,7 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); @@ -3301,7 +3302,6 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; }
This is already taken care of upstream with: commit 2745177b346a0ebbd3629e298178c60a54af242a Author: Peter Krempa <pkrempa@redhat.com> Date: Tue Dec 11 19:28:17 2012 +0100 qemu: Refactor managed save functions to use domain lookup helpers
@@ -12676,6 +12676,7 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(domain->uuid, uuidstr); @@ -12688,9 +12689,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); goto cleanup; - } + }
- if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup;
if (!virDomainObjIsActive(vm)) { @@ -12705,9 +12706,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
hmp = !!(flags & VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP);
- qemuDomainObjEnterMonitorWithDriver(driver, vm); + qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp); - qemuDomainObjExitMonitorWithDriver(driver, vm); + qemuDomainObjExitMonitor(driver, vm);
endjob: if (qemuDomainObjEndJob(driver, vm) == 0) { @@ -12717,7 +12718,6 @@ endjob: cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; }
This cleanup makes sense, but it'd be great to take advantage of the lookup helpers I introduced lately to decrease the complexity even more.
Peter
On 12/13/12 13:50, Peter Krempa wrote:
On 12/13/12 12:40, Michal Privoznik wrote:
There is no need to hold qemu lock during the whole execution of these two APIs. --- src/qemu/qemu_driver.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2fda44e..74e442b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3288,6 +3288,7 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); @@ -3301,7 +3302,6 @@ qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; }
This is already taken care of upstream with: commit 2745177b346a0ebbd3629e298178c60a54af242a Author: Peter Krempa <pkrempa@redhat.com> Date: Tue Dec 11 19:28:17 2012 +0100
qemu: Refactor managed save functions to use domain lookup helpers
@@ -12676,6 +12676,7 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, domain->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(domain->uuid, uuidstr); @@ -12688,9 +12689,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); goto cleanup; - } + }
- if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup;
if (!virDomainObjIsActive(vm)) { @@ -12705,9 +12706,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
hmp = !!(flags & VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP);
- qemuDomainObjEnterMonitorWithDriver(driver, vm); + qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp); - qemuDomainObjExitMonitorWithDriver(driver, vm); + qemuDomainObjExitMonitor(driver, vm);
endjob: if (qemuDomainObjEndJob(driver, vm) == 0) { @@ -12717,7 +12718,6 @@ endjob: cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; }
This cleanup makes sense, but it'd be great to take advantage of the lookup helpers I introduced lately to decrease the complexity even more.
Ah yeah, you do that in 2/2. ACK in this case if you rebase this patch on the current HEAD (and thus get rid of the first two hunks.
Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Many internal qemu APIs must find domain object from passed virDomainPtr. And with function Peter's introduced, we can use it instead of copying multiple lines among code. --- src/qemu/qemu_driver.c | 470 +++++------------------------------------------- 1 files changed, 47 insertions(+), 423 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 74e442b..8ace5c2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1481,20 +1481,12 @@ cleanup: static int qemuDomainIsActive(virDomainPtr dom) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr obj; int ret = -1; - qemuDriverLock(driver); - obj = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!obj) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(obj = qemuDomObjFromDomain(dom))) goto cleanup; - } + ret = virDomainObjIsActive(obj); cleanup: @@ -1505,20 +1497,12 @@ cleanup: static int qemuDomainIsPersistent(virDomainPtr dom) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr obj; int ret = -1; - qemuDriverLock(driver); - obj = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!obj) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(obj = qemuDomObjFromDomain(dom))) goto cleanup; - } + ret = obj->persistent; cleanup: @@ -1529,20 +1513,12 @@ cleanup: static int qemuDomainIsUpdated(virDomainPtr dom) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr obj; int ret = -1; - qemuDriverLock(driver); - obj = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!obj) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(obj = qemuDomObjFromDomain(dom))) goto cleanup; - } + ret = obj->updated; cleanup: @@ -1874,17 +1850,8 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { return -1; } - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } priv = vm->privateData; @@ -1964,17 +1931,8 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) return -1; } - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } priv = vm->privateData; @@ -2056,17 +2014,8 @@ qemuDomainReset(virDomainPtr dom, unsigned int flags) virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -2197,20 +2146,11 @@ qemuDomainDestroy(virDomainPtr dom) } static char *qemuDomainGetOSType(virDomainPtr dom) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; char *type = NULL; - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (!(type = strdup(vm->def->os.type))) virReportOOMError(); @@ -2225,21 +2165,11 @@ cleanup: static unsigned long long qemuDomainGetMaxMemory(virDomainPtr dom) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; unsigned long long ret = 0; - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } ret = vm->def->mem.max_balloon; @@ -2261,16 +2191,8 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_MEM_MAXIMUM, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -2487,16 +2409,8 @@ static int qemuDomainGetInfo(virDomainPtr dom, int err; unsigned long long balloon; - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } info->state = virDomainObjGetState(vm, NULL); @@ -2569,23 +2483,13 @@ qemuDomainGetState(virDomainPtr dom, int *reason, unsigned int flags) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } *state = virDomainObjGetState(vm, reason); ret = 0; @@ -2601,24 +2505,14 @@ qemuDomainGetControlInfo(virDomainPtr dom, virDomainControlInfoPtr info, unsigned int flags) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; int ret = -1; virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -3280,22 +3174,13 @@ cleanup: static int qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; int ret = -1; virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } ret = vm->hasManagedSave; @@ -3588,17 +3473,8 @@ qemuDomainScreenshot(virDomainPtr dom, virCheckFlags(0, NULL); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } priv = vm->privateData; @@ -3954,17 +3830,8 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, return -1; } - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -4066,17 +3933,8 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) @@ -4249,17 +4107,8 @@ qemuDomainGetVcpuPinInfo(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &targetDef) < 0) @@ -4339,17 +4188,8 @@ qemuDomainPinEmulator(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -4496,17 +4336,8 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &targetDef) < 0) @@ -4561,23 +4392,13 @@ qemuDomainGetVcpus(virDomainPtr dom, int maxinfo, unsigned char *cpumaps, int maplen) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int i, v, maxcpu, hostcpus; int ret = -1; qemuDomainObjPrivatePtr priv; - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -4667,17 +4488,8 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) VIR_DOMAIN_AFFECT_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &def) < 0) goto cleanup; @@ -6746,21 +6558,11 @@ static int qemuDomainDetachDevice(virDomainPtr dom, const char *xml) static int qemuDomainGetAutostart(virDomainPtr dom, int *autostart) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } *autostart = vm->autostart; ret = 0; @@ -8521,17 +8323,8 @@ qemuDomainBlockResize(virDomainPtr dom, size *= 1024; } - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } priv = vm->privateData; @@ -8592,16 +8385,8 @@ qemuDomainBlockStats(virDomainPtr dom, virDomainDiskDefPtr disk = NULL; qemuDomainObjPrivatePtr priv; - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -8677,16 +8462,8 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, /* We don't return strings, and thus trivially support this flag. */ flags &= ~VIR_TYPED_PARAM_STRING_OKAY; - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -8840,22 +8617,12 @@ qemuDomainInterfaceStats(virDomainPtr dom, const char *path, struct _virDomainInterfaceStats *stats) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int i; int ret = -1; - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -9203,17 +8970,8 @@ qemuDomainMemoryStats(virDomainPtr dom, virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; @@ -9257,24 +9015,14 @@ qemuDomainBlockPeek(virDomainPtr dom, void *buffer, unsigned int flags) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int fd = -1, ret = -1; const char *actual; virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (!path || path[0] == '\0') { virReportError(VIR_ERR_INVALID_ARG, @@ -9332,17 +9080,8 @@ qemuDomainMemoryPeek(virDomainPtr dom, virCheckFlags(VIR_MEMORY_VIRTUAL | VIR_MEMORY_PHYSICAL, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (flags != VIR_MEMORY_VIRTUAL && flags != VIR_MEMORY_PHYSICAL) { virReportError(VIR_ERR_INVALID_ARG, @@ -9430,16 +9169,8 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (!path || path[0] == '\0') { virReportError(VIR_ERR_INVALID_ARG, @@ -10370,21 +10101,12 @@ qemuCPUBaseline(virConnectPtr conn ATTRIBUTE_UNUSED, static int qemuDomainGetJobInfo(virDomainPtr dom, virDomainJobInfoPtr info) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; qemuDomainObjPrivatePtr priv; - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } priv = vm->privateData; @@ -10425,16 +10147,8 @@ static int qemuDomainAbortJob(virDomainPtr dom) { int ret = -1; qemuDomainObjPrivatePtr priv; - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_ABORT) < 0) goto cleanup; @@ -10487,17 +10201,8 @@ qemuDomainMigrateSetMaxDowntime(virDomainPtr dom, virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); - return -1; - } + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) goto cleanup; @@ -10543,17 +10248,8 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); - return -1; - } + if (!(vm = qemuDomObjFromDomain(dom))) + goto cleanup; priv = vm->privateData; if (virDomainObjIsActive(vm)) { @@ -10593,24 +10289,14 @@ qemuDomainMigrateGetMaxSpeed(virDomainPtr dom, unsigned long *bandwidth, unsigned int flags) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; int ret = -1; virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } priv = vm->privateData; *bandwidth = priv->migMaxBandwidth; @@ -12674,16 +12360,8 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, virCheckFlags(VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, domain->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; - } if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -14030,7 +13708,6 @@ qemuDomainGetDiskErrors(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; qemuDomainObjPrivatePtr priv; - char uuidstr[VIR_UUID_STRING_BUFLEN]; virHashTablePtr table = NULL; int ret = -1; int i; @@ -14038,16 +13715,8 @@ qemuDomainGetDiskErrors(virDomainPtr dom, virCheckFlags(0, -1); - qemuDriverLock(driver); - virUUIDFormat(dom->uuid, uuidstr); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } priv = vm->privateData; @@ -14122,17 +13791,8 @@ qemuDomainSetMetadata(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &persistentDef) < 0) @@ -14222,17 +13882,8 @@ qemuDomainGetMetadata(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, NULL); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, &def) < 0) goto cleanup; @@ -14578,17 +14229,8 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom, return -1; } - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } priv = vm->privateData; @@ -14671,17 +14313,8 @@ qemuDomainPMWakeup(virDomainPtr dom, virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -14746,17 +14379,8 @@ qemuDomainAgentCommand(virDomainPtr domain, virCheckFlags(0, NULL); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, domain->uuid); - qemuDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; - } priv = vm->privateData; -- 1.7.8.6
On 12/13/12 12:40, Michal Privoznik wrote:
Many internal qemu APIs must find domain object from passed virDomainPtr. And with function Peter's introduced, we can use it instead of copying multiple lines among code. --- src/qemu/qemu_driver.c | 470 +++++------------------------------------------- 1 files changed, 47 insertions(+), 423 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 74e442b..8ace5c2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
[...]
@@ -3280,22 +3174,13 @@ cleanup: static int qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; int ret = -1;
virCheckFlags(0, -1);
- qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - }
ret = vm->hasManagedSave;
Again, this hunk is already changed upstream.
ACK if you rebase this on the current upstream head and thus get rid of the hunk above. Peter
On 13.12.2012 14:23, Peter Krempa wrote:
On 12/13/12 12:40, Michal Privoznik wrote:
Many internal qemu APIs must find domain object from passed virDomainPtr. And with function Peter's introduced, we can use it instead of copying multiple lines among code. --- src/qemu/qemu_driver.c | 470 +++++------------------------------------------- 1 files changed, 47 insertions(+), 423 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 74e442b..8ace5c2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
[...]
@@ -3280,22 +3174,13 @@ cleanup: static int qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; int ret = -1;
virCheckFlags(0, -1);
- qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - }
ret = vm->hasManagedSave;
Again, this hunk is already changed upstream.
ACK if you rebase this on the current upstream head and thus get rid of the hunk above.
Peter
Thanks, I've rebased both patches. Seems like our git is changing so fast that I need to pull every day not every two days :). Morover, I'll hold pushing this till 24th - it's gonna be a small Christmas gift to libvirt from me :) Michal
On 13.12.2012 14:23, Peter Krempa wrote:
On 12/13/12 12:40, Michal Privoznik wrote:
Many internal qemu APIs must find domain object from passed virDomainPtr. And with function Peter's introduced, we can use it instead of copying multiple lines among code. --- src/qemu/qemu_driver.c | 470 +++++------------------------------------------- 1 files changed, 47 insertions(+), 423 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 74e442b..8ace5c2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
[...]
@@ -3280,22 +3174,13 @@ cleanup: static int qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) { - virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; int ret = -1;
virCheckFlags(0, -1);
- qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - }
ret = vm->hasManagedSave;
Again, this hunk is already changed upstream.
ACK if you rebase this on the current upstream head and thus get rid of the hunk above.
Peter
Thanks, pushed now! Michal
participants (2)
-
Michal Privoznik -
Peter Krempa