12:48 p.m.
This series improves helpers to retrieve domain objects from
virDomainPtrs and uses the improvements in the ManagedSave APIs.
This series also fixes a bug in error reporting of qemuDomainManagedSaveRemove
where if unlink() failed no error was reported but an error code was returned.
Although there's a slight net increase of number of lines by this patch, it's
mainly due to the docs added in 1/4
Peter Krempa (4):
qemu: Add a new domain lookup helper and improve the docs
qemu: Refactor managed save functions to use domain lookup helpers
qemu: Small code cleanups in the managedsave functions
qemu: Improve error reporting from qemuDomainManagedSaveRemove
src/qemu/qemu_driver.c | 115 +++++++++++++++++++++++++++----------------------
1 file changed, 63 insertions(+), 52 deletions(-)
--
1.8.0
12:48 p.m.
New subject: [libvirt] [PATCH 1/4] qemu: Add a new domain lookup helper and improve the docs
This patch adds a new domain lookup helper qemuDomObjFromDomainDriver
that lookups the domain and leaves the driver locked. The driver is
returned as the second argument of that function. If the lookup fails
the driver is unlocked to help avoid cleanup codepaths.
This patch also improves docs for the helpers.
---
src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 93d9b69..1f43386 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -187,11 +187,20 @@ struct qemuAutostartData {
virConnectPtr conn;
};
-
-/* Looks up the domain object and unlocks the driver. The returned domain
- * object is locked and the caller is responsible for unlocking it. */
+/**
+ * qemuDomObjFromDomainDriver:
+ * @domain: Domain pointer that has to be looked up
+ * @drv: Pointer to virQEMUDriverPtr to return the driver object
+ *
+ * This fucntion looks up @domain in the domain list and returns the
+ * appropriate virDomainObjPtr. On sucessful lookup, both driver and domain
+ * object are returned locked.
+ *
+ * Returns the domain object if it's found and the driver. Both are locked.
+ * In case of failure NULL is returned and the driver isn't locked.
+ */
static virDomainObjPtr
-qemuDomObjFromDomain(virDomainPtr domain)
+qemuDomObjFromDomainDriver(virDomainPtr domain, virQEMUDriverPtr *drv)
{
virQEMUDriverPtr driver = domain->conn->privateData;
virDomainObjPtr vm;
@@ -199,16 +208,42 @@ qemuDomObjFromDomain(virDomainPtr domain)
qemuDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, domain->uuid);
- qemuDriverUnlock(driver);
if (!vm) {
virUUIDFormat(domain->uuid, uuidstr);
virReportError(VIR_ERR_NO_DOMAIN,
_("no domain with matching uuid '%s'"),
uuidstr);
+ qemuDriverUnlock(driver);
+ *drv = NULL;
+ return NULL;
}
+ *drv = driver;
return vm;
}
+/**
+ * qemuDomObjFromDomain:
+ * @domain: Domain pointer that has to be looked up
+ *
+ * This function looks up @domain and returns the appropriate
+ * virDomainObjPtr. The driver is unlocked after the call.
+ *
+ * Returns the domain object which is locked on success, NULL
+ * otherwise. Te driver remains unlocked after the call.
+ */
+static virDomainObjPtr
+qemuDomObjFromDomain(virDomainPtr domain)
+{
+ virDomainObjPtr vm;
+ virQEMUDriverPtr driver;
+
+ if (!(vm = qemuDomObjFromDomainDriver(domain, &driver)))
+ return NULL;
+
+ qemuDriverUnlock(driver);
+
+ return vm;
+}
/* Looks up the domain object from snapshot and unlocks the driver. The
* returned domain object is locked and the caller is responsible for
--
1.8.0
3:46 p.m.
New subject: [libvirt] [PATCH 1/4] qemu: Add a new domain lookup helper and improve the docs
On 12/11/2012 11:48 AM, Peter Krempa wrote:
This patch adds a new domain lookup helper
qemuDomObjFromDomainDriver
that lookups the domain and leaves the driver locked. The driver is
returned as the second argument of that function. If the lookup fails
the driver is unlocked to help avoid cleanup codepaths.
This patch also improves docs for the helpers.
---
src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 5 deletions(-)
+/**
+ * qemuDomObjFromDomainDriver:
+ * @domain: Domain pointer that has to be looked up
+ * @drv: Pointer to virQEMUDriverPtr to return the driver object
+ *
+ * This fucntion looks up @domain in the domain list and returns the
s/fucntion/function/
+ * appropriate virDomainObjPtr. On sucessful lookup, both driver and
domain
s/sucessful/successful/
+ * object are returned locked.
+ *
+ * Returns the domain object if it's found and the driver. Both are locked.
+ * In case of failure NULL is returned and the driver isn't locked.
+ */
+/**
+ * qemuDomObjFromDomain:
+ * @domain: Domain pointer that has to be looked up
+ *
+ * This function looks up @domain and returns the appropriate
+ * virDomainObjPtr. The driver is unlocked after the call.
+ *
+ * Returns the domain object which is locked on success, NULL
+ * otherwise. Te driver remains unlocked after the call.
s/Te/The/
ACK with spelling fixes.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:48 p.m.
New subject: [libvirt] [PATCH 2/4] qemu: Refactor managed save functions to use domain lookup helpers
---
src/qemu/qemu_driver.c | 54 +++++++++++---------------------------------------
1 file changed, 12 insertions(+), 42 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1f43386..72da014 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3246,8 +3246,8 @@ qemuDomainManagedSavePath(virQEMUDriverPtr driver, virDomainObjPtr
vm) {
static int
qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
- virDomainObjPtr vm = NULL;
+ virQEMUDriverPtr driver;
+ virDomainObjPtr vm;
char *name = NULL;
int ret = -1;
int compressed;
@@ -3256,15 +3256,8 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
VIR_DOMAIN_SAVE_RUNNING |
VIR_DOMAIN_SAVE_PAUSED, -1);
- qemuDriverLock(driver);
- vm = virDomainFindByUUID(&driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuidstr);
- goto cleanup;
- }
+ if (!(vm = qemuDomObjFromDomainDriver(dom, &driver)))
+ return -1;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -3323,50 +3316,28 @@ 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);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuidstr);
- goto cleanup;
- }
-
- ret = vm->hasManagedSave;
+ if (!(vm = qemuDomObjFromDomain(dom)))
+ return -1;
-cleanup:
- if (vm)
- virDomainObjUnlock(vm);
- qemuDriverUnlock(driver);
- return ret;
+ return vm->hasManagedSave;
}
static int
qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
- virDomainObjPtr vm = NULL;
+ virQEMUDriverPtr driver;
+ virDomainObjPtr vm;
int ret = -1;
char *name = NULL;
virCheckFlags(0, -1);
- qemuDriverLock(driver);
- vm = virDomainFindByUUID(&driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuidstr);
- goto cleanup;
- }
+ if (!(vm = qemuDomObjFromDomainDriver(dom, &driver)))
+ return -1;
name = qemuDomainManagedSavePath(driver, vm);
if (name == NULL)
@@ -3377,8 +3348,7 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
cleanup:
VIR_FREE(name);
- if (vm)
- virDomainObjUnlock(vm);
+ virDomainObjUnlock(vm);
qemuDriverUnlock(driver);
return ret;
}
--
1.8.0
3:50 p.m.
New subject: [libvirt] [PATCH 2/4] qemu: Refactor managed save functions to use domain lookup helpers
On 12/11/2012 11:48 AM, Peter Krempa wrote:
---
src/qemu/qemu_driver.c | 54 +++++++++++---------------------------------------
1 file changed, 12 insertions(+), 42 deletions(-)
Nice reduction in size. However, you need a v2...
@@ -3323,50 +3316,28 @@ 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);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuidstr);
- goto cleanup;
- }
-
- ret = vm->hasManagedSave;
+ if (!(vm = qemuDomObjFromDomain(dom)))
+ return -1;
-cleanup:
- if (vm)
- virDomainObjUnlock(vm);
- qemuDriverUnlock(driver);
- return ret;
+ return vm->hasManagedSave;
}
Ouch. This leaves vm locked on exit. It needs to look more like:
if !(vm = qemuDomObjFromDomain(dom)))
return -1;
ret = vm->hasManagedSave;
virDomainObjUnlock(vm);
retun ret;
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:10 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: Refactor managed save functions to use domain lookup helpers
On 12/11/12 22:50, Eric Blake wrote:
On 12/11/2012 11:48 AM, Peter Krempa wrote:
> ---
> src/qemu/qemu_driver.c | 54 +++++++++++---------------------------------------
> 1 file changed, 12 insertions(+), 42 deletions(-)
Nice reduction in size. However, you need a v2...
[...]
>
> -cleanup:
> - if (vm)
> - virDomainObjUnlock(vm);
> - qemuDriverUnlock(driver);
> - return ret;
> + return vm->hasManagedSave;
> }
Ouch. This leaves vm locked on exit. It needs to look more like:
Removal of code is really addictive :) I got carried away and strived
for more lines :) V2 comming in soon.
if !(vm = qemuDomObjFromDomain(dom)))
return -1;
ret = vm->hasManagedSave;
virDomainObjUnlock(vm);
retun ret;
Peter
1:17 a.m.
New subject: [libvirt] [PATCHv2 2/5] qemu: Refactor managed save functions to use domain lookup helpers
---
Diff to v1:
-unlock the domain in qemuDomainHasManagedSaveImage()
---
src/qemu/qemu_driver.c | 53 +++++++++++++-------------------------------------
1 file changed, 13 insertions(+), 40 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9173413..4144e6f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3238,8 +3238,8 @@ qemuDomainManagedSavePath(virQEMUDriverPtr driver, virDomainObjPtr
vm) {
static int
qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
- virDomainObjPtr vm = NULL;
+ virQEMUDriverPtr driver;
+ virDomainObjPtr vm;
char *name = NULL;
int ret = -1;
int compressed;
@@ -3248,15 +3248,8 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
VIR_DOMAIN_SAVE_RUNNING |
VIR_DOMAIN_SAVE_PAUSED, -1);
- qemuDriverLock(driver);
- vm = virDomainFindByUUID(&driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuidstr);
- goto cleanup;
- }
+ if (!(vm = qemuDomObjFromDomainDriver(dom, &driver)))
+ return -1;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
@@ -3315,50 +3308,31 @@ cleanup:
static int
qemuDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
virDomainObjPtr vm = NULL;
- int ret = -1;
+ int ret;
virCheckFlags(0, -1);
- qemuDriverLock(driver);
- vm = virDomainFindByUUID(&driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuidstr);
- goto cleanup;
- }
+ if (!(vm = qemuDomObjFromDomain(dom)))
+ return -1;
ret = vm->hasManagedSave;
-
-cleanup:
- if (vm)
- virDomainObjUnlock(vm);
- qemuDriverUnlock(driver);
+ virDomainObjUnlock(vm);
return ret;
}
static int
qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
{
- virQEMUDriverPtr driver = dom->conn->privateData;
- virDomainObjPtr vm = NULL;
+ virQEMUDriverPtr driver;
+ virDomainObjPtr vm;
int ret = -1;
char *name = NULL;
virCheckFlags(0, -1);
- qemuDriverLock(driver);
- vm = virDomainFindByUUID(&driver->domains, dom->uuid);
- if (!vm) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(dom->uuid, uuidstr);
- virReportError(VIR_ERR_NO_DOMAIN,
- _("no domain with matching uuid '%s'"),
uuidstr);
- goto cleanup;
- }
+ if (!(vm = qemuDomObjFromDomainDriver(dom, &driver)))
+ return -1;
name = qemuDomainManagedSavePath(driver, vm);
if (name == NULL)
@@ -3369,8 +3343,7 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
cleanup:
VIR_FREE(name);
- if (vm)
- virDomainObjUnlock(vm);
+ virDomainObjUnlock(vm);
qemuDriverUnlock(driver);
return ret;
}
--
1.8.0
7:23 a.m.
New subject: [libvirt] [PATCHv2 2/5] qemu: Refactor managed save functions to use domain lookup helpers
On 12/12/2012 12:17 AM, Peter Krempa wrote:
---
Diff to v1:
-unlock the domain in qemuDomainHasManagedSaveImage()
---
src/qemu/qemu_driver.c | 53 +++++++++++++-------------------------------------
1 file changed, 13 insertions(+), 40 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:01 a.m.
New subject: [libvirt] [PATCHv2 2/5] qemu: Refactor managed save functions to use domain lookup helpers
On 12/12/12 14:23, Eric Blake wrote:
On 12/12/2012 12:17 AM, Peter Krempa wrote:
> ---
> Diff to v1:
> -unlock the domain in qemuDomainHasManagedSaveImage()
> ---
>
> src/qemu/qemu_driver.c | 53 +++++++++++++-------------------------------------
> 1 file changed, 13 insertions(+), 40 deletions(-)
ACK.
Thanks for the review. I pushed the series.
Peter
12:48 p.m.
New subject: [libvirt] [PATCH 3/4] qemu: Small code cleanups in the managedsave functions
Save a few lines moving assignments into conditions and fix braces
position.
---
src/qemu/qemu_driver.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 72da014..b1cb185 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3232,7 +3232,8 @@ qemuDomainSave(virDomainPtr dom, const char *path)
}
static char *
-qemuDomainManagedSavePath(virQEMUDriverPtr driver, virDomainObjPtr vm) {
+qemuDomainManagedSavePath(virQEMUDriverPtr driver, virDomainObjPtr vm)
+{
char *ret;
if (virAsprintf(&ret, "%s/%s.save", driver->saveDir,
vm->def->name) < 0) {
@@ -3270,8 +3271,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
goto cleanup;
}
- name = qemuDomainManagedSavePath(driver, vm);
- if (name == NULL)
+ if (!(name = qemuDomainManagedSavePath(driver, vm)))
goto cleanup;
VIR_INFO("Saving state to %s", name);
@@ -3339,8 +3339,7 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
if (!(vm = qemuDomObjFromDomainDriver(dom, &driver)))
return -1;
- name = qemuDomainManagedSavePath(driver, vm);
- if (name == NULL)
+ if (!(name = qemuDomainManagedSavePath(driver, vm)))
goto cleanup;
ret = unlink(name);
--
1.8.0
3:53 p.m.
New subject: [libvirt] [PATCH 3/4] qemu: Small code cleanups in the managedsave functions
On 12/11/2012 11:48 AM, Peter Krempa wrote:
Save a few lines moving assignments into conditions and fix braces
position.
---
src/qemu/qemu_driver.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
ACK. Cosmetic, but brings it more in line with our style.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:48 p.m.
New subject: [libvirt] [PATCH 4/4] qemu: Improve error reporting from qemuDomainManagedSaveRemove
Report an error if unlink of the managedsave file fails.
---
Hm, this patch changes semantics a little bit. If the unlink() fails now,
the domain isn't marked as not having a managed save file. If somebody does
not like this change I will return it to the previous semantics.
---
src/qemu/qemu_driver.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b1cb185..1941ae7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3342,8 +3342,15 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
if (!(name = qemuDomainManagedSavePath(driver, vm)))
goto cleanup;
- ret = unlink(name);
+ if (unlink(name) < 0) {
+ virReportSystemError(errno,
+ _("Failed to remove managed save file
'%s'"),
+ name);
+ goto cleanup;
+ }
+
vm->hasManagedSave = false;
+ ret = 0;
cleanup:
VIR_FREE(name);
--
1.8.0
4:02 p.m.
New subject: [libvirt] [PATCH 4/4] qemu: Improve error reporting from qemuDomainManagedSaveRemove
On 12/11/2012 11:48 AM, Peter Krempa wrote:
Report an error if unlink of the managedsave file fails.
---
Hm, this patch changes semantics a little bit. If the unlink() fails now,
the domain isn't marked as not having a managed save file. If somebody does
not like this change I will return it to the previous semantics.
The new semantics make sense - another libvirtd restart will see the
managedsave file still existing, and can attempt to reload/re-unlink it
again.
ACK as-is.
---
src/qemu/qemu_driver.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b1cb185..1941ae7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3342,8 +3342,15 @@ qemuDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
if (!(name = qemuDomainManagedSavePath(driver, vm)))
goto cleanup;
- ret = unlink(name);
+ if (unlink(name) < 0) {
+ virReportSystemError(errno,
+ _("Failed to remove managed save file
'%s'"),
+ name);
+ goto cleanup;
+ }
+
vm->hasManagedSave = false;
+ ret = 0;
cleanup:
VIR_FREE(name);
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4399
days inactive
4400
days old
12 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa