3:38 p.m.
Pretty easy to paste together compared to existing functions.
* src/esx/esx_driver.c (esxDomainSnapshotGetParent): New function.
---
I can only compile-test this; I'm relying on someone with an actual
esx setup to actually test it. Also, I didn't see anything in
existing code that would efficiently implement
virDomainSnapshotNumChildren; there may an API that I'm not aware of,
but someone else will have to implement that API (Matthias?)
src/esx/esx_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index c15c0d6..ab93efd 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4484,6 +4484,46 @@ esxDomainHasCurrentSnapshot(virDomainPtr domain, unsigned int
flags)
static virDomainSnapshotPtr
+esxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, unsigned int flags)
+{
+ esxPrivate *priv = snapshot->domain->conn->privateData;
+ esxVI_VirtualMachineSnapshotTree *rootSnapshotList = NULL;
+ esxVI_VirtualMachineSnapshotTree *snapshotTree = NULL;
+ esxVI_VirtualMachineSnapshotTree *snapshotTreeParent = NULL;
+ virDomainSnapshotPtr parent = NULL;
+
+ virCheckFlags(0, NULL);
+
+ if (esxVI_EnsureSession(priv->primary) < 0) {
+ return NULL;
+ }
+
+ if (esxVI_LookupRootSnapshotTreeList(priv->primary, snapshot->domain->uuid,
+ &rootSnapshotList) < 0 ||
+ esxVI_GetSnapshotTreeByName(rootSnapshotList, snapshot->name,
+ &snapshotTree, &snapshotTreeParent,
+ esxVI_Occurrence_RequiredItem) < 0) {
+ goto cleanup;
+ }
+
+ if (!snapshotTreeParent) {
+ ESX_ERROR(VIR_ERR_NO_DOMAIN_SNAPSHOT,
+ _("snapshot '%s' does not have a parent"),
+ snapshotTree->name);
+ goto cleanup;
+ }
+
+ parent = virGetDomainSnapshot(snapshot->domain, snapshotTreeParent->name);
+
+cleanup:
+ esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList);
+
+ return parent;
+}
+
+
+
+static virDomainSnapshotPtr
esxDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags)
{
esxPrivate *priv = domain->conn->privateData;
@@ -4831,6 +4871,7 @@ static virDriver esxDriver = {
.domainSnapshotListNames = esxDomainSnapshotListNames, /* 0.8.0 */
.domainSnapshotLookupByName = esxDomainSnapshotLookupByName, /* 0.8.0 */
.domainHasCurrentSnapshot = esxDomainHasCurrentSnapshot, /* 0.8.0 */
+ .domainSnapshotGetParent = esxDomainSnapshotGetParent, /* 0.9.7 */
.domainSnapshotCurrent = esxDomainSnapshotCurrent, /* 0.8.0 */
.domainRevertToSnapshot = esxDomainRevertToSnapshot, /* 0.8.0 */
.domainSnapshotDelete = esxDomainSnapshotDelete, /* 0.8.0 */
--
1.7.4.4
3:38 p.m.
New subject: [libvirt] [PATCH 2/2] snapshot: implement getparent for vbox
Again, built by copying from existing functions.
* src/vbox/vbox_tmpl.c (vboxDomainSnapshotGetParent): New function.
---
I can only compile-test this; I'm relying on someone with an actual
vbox setup to actually test it. Also, I didn't see anything in
existing code that would efficiently implement
virDomainSnapshotNumChildren; there may an API that I'm not aware of,
but someone else will have to implement that API (Matthias?)
src/vbox/vbox_tmpl.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index b483d0f..3d5f4ae 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6046,6 +6046,76 @@ cleanup:
}
static virDomainSnapshotPtr
+vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
+ unsigned int flags)
+{
+ virDomainPtr dom = snapshot->domain;
+ VBOX_OBJECT_CHECK(dom->conn, virDomainSnapshotPtr, NULL);
+ vboxIID iid = VBOX_IID_INITIALIZER;
+ IMachine *machine = NULL;
+ ISnapshot *snap = NULL;
+ ISnapshot *parent = NULL;
+ PRUnichar *nameUtf16 = NULL;
+ char *name = NULL;
+ nsresult rc;
+
+ virCheckFlags(0, NULL);
+
+ vboxIIDFromUUID(&iid, dom->uuid);
+ rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_NO_DOMAIN, "%s",
+ _("no domain with matching UUID"));
+ goto cleanup;
+ }
+
+ if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name)))
+ goto cleanup;
+
+ rc = machine->vtbl->GetCurrentSnapshot(machine, &snap);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("could not get current snapshot"));
+ goto cleanup;
+ }
+
+ rc = snap->vtbl->GetParent(snap, &parent);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not get parent of snapshot %s"),
+ snapshot->name);
+ goto cleanup;
+ }
+ if (!parent) {
+ vboxError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
+ _("snapshot '%s' does not have a parent"),
+ snapshot->name);
+ goto cleanup;
+ }
+
+ rc = parent->vtbl->GetName(parent, &nameUtf16);
+ if (NS_FAILED(rc) || !nameUtf16) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not get name of parent of snapshot %s"),
+ snapshot->name);
+ goto cleanup;
+ }
+ VBOX_UTF16_TO_UTF8(nameUtf16, &name);
+ VBOX_UTF16_FREE(nameUtf16);
+
+ ret = virGetDomainSnapshot(dom, name);
+
+cleanup:
+ VBOX_UTF8_FREE(name);
+ VBOX_UTF16_FREE(nameUtf16);
+ VBOX_RELEASE(snap);
+ VBOX_RELEASE(parent);
+ VBOX_RELEASE(machine);
+ vboxIIDUnalloc(&iid);
+ return ret;
+}
+
+static virDomainSnapshotPtr
vboxDomainSnapshotCurrent(virDomainPtr dom,
unsigned int flags)
{
@@ -8879,6 +8949,7 @@ virDriver NAME(Driver) = {
.domainSnapshotListNames = vboxDomainSnapshotListNames, /* 0.8.0 */
.domainSnapshotLookupByName = vboxDomainSnapshotLookupByName, /* 0.8.0 */
.domainHasCurrentSnapshot = vboxDomainHasCurrentSnapshot, /* 0.8.0 */
+ .domainSnapshotGetParent = vboxDomainSnapshotGetParent, /* 0.9.7 */
.domainSnapshotCurrent = vboxDomainSnapshotCurrent, /* 0.8.0 */
.domainRevertToSnapshot = vboxDomainRevertToSnapshot, /* 0.8.0 */
.domainSnapshotDelete = vboxDomainSnapshotDelete, /* 0.8.0 */
--
1.7.4.4
3:50 a.m.
New subject: [libvirt] [PATCH 2/2] snapshot: implement getparent for vbox
2011/9/29 Eric Blake <eblake(a)redhat.com>:
Again, built by copying from existing functions.
* src/vbox/vbox_tmpl.c (vboxDomainSnapshotGetParent): New function.
---
I can only compile-test this; I'm relying on someone with an actual
vbox setup to actually test it.
This patch needs some fixes, see below.
Also, I didn't see anything in
existing code that would efficiently implement
virDomainSnapshotNumChildren; there may an API that I'm not aware of,
but someone else will have to implement that API (Matthias?)
Is virDomainSnapshotNumChildren supposed to only return the number of
direct children, or all children (direct children, grandchildren, etc)
of a snapshot? In the later case they'll have to be counted
recursively, unfortunately.
src/vbox/vbox_tmpl.c | 71
++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index b483d0f..3d5f4ae 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6046,6 +6046,76 @@ cleanup:
}
static virDomainSnapshotPtr
+vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
+ unsigned int flags)
+{
+ virDomainPtr dom = snapshot->domain;
+ VBOX_OBJECT_CHECK(dom->conn, virDomainSnapshotPtr, NULL);
+ vboxIID iid = VBOX_IID_INITIALIZER;
+ IMachine *machine = NULL;
+ ISnapshot *snap = NULL;
+ ISnapshot *parent = NULL;
+ PRUnichar *nameUtf16 = NULL;
+ char *name = NULL;
+ nsresult rc;
+
+ virCheckFlags(0, NULL);
+
+ vboxIIDFromUUID(&iid, dom->uuid);
+ rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_NO_DOMAIN, "%s",
+ _("no domain with matching UUID"));
+ goto cleanup;
+ }
+
+ if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name)))
+ goto cleanup;
At this point you already have the snapshot you want.
+ rc = machine->vtbl->GetCurrentSnapshot(machine,
&snap);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("could not get current snapshot"));
+ goto cleanup;
+ }
This block here gives you the current snapshot, that's not what you want.
+ rc = snap->vtbl->GetParent(snap, &parent);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not get parent of snapshot %s"),
+ snapshot->name);
+ goto cleanup;
+ }
+ if (!parent) {
+ vboxError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
+ _("snapshot '%s' does not have a parent"),
+ snapshot->name);
+ goto cleanup;
+ }
+
+ rc = parent->vtbl->GetName(parent, &nameUtf16);
+ if (NS_FAILED(rc) || !nameUtf16) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not get name of parent of snapshot %s"),
+ snapshot->name);
+ goto cleanup;
+ }
+ VBOX_UTF16_TO_UTF8(nameUtf16, &name);
+ VBOX_UTF16_FREE(nameUtf16);
Because VBOX_UTF16_FREE doesn't set the pointer to NULL, calling it
twice on the same pointer isn't safe.
+
+ ret = virGetDomainSnapshot(dom, name);
+
+cleanup:
+ VBOX_UTF8_FREE(name);
+ VBOX_UTF16_FREE(nameUtf16);
And this second call segfaults in the success path.
+ VBOX_RELEASE(snap);
+ VBOX_RELEASE(parent);
+ VBOX_RELEASE(machine);
+ vboxIIDUnalloc(&iid);
+ return ret;
+}
ACK with this diff applied:
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 3d5f4ae..2eb23fb 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6072,13 +6072,6 @@
vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name)))
goto cleanup;
- rc = machine->vtbl->GetCurrentSnapshot(machine, &snap);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("could not get current snapshot"));
- goto cleanup;
- }
-
rc = snap->vtbl->GetParent(snap, &parent);
if (NS_FAILED(rc)) {
vboxError(VIR_ERR_INTERNAL_ERROR,
@@ -6101,7 +6094,10 @@
vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
goto cleanup;
}
VBOX_UTF16_TO_UTF8(nameUtf16, &name);
- VBOX_UTF16_FREE(nameUtf16);
+ if (!name) {
+ virReportOOMError();
+ goto cleanup;
+ }
ret = virGetDomainSnapshot(dom, name);
--
Matthias Bolte
http://photron.blogspot.com
9:27 a.m.
New subject: [libvirt] [PATCH 2/2] snapshot: implement getparent for vbox
On 10/02/2011 02:50 AM, Matthias Bolte wrote:
2011/9/29 Eric Blake<eblake(a)redhat.com>:
> Again, built by copying from existing functions.
>
> * src/vbox/vbox_tmpl.c (vboxDomainSnapshotGetParent): New function.
> ---
>
> I can only compile-test this; I'm relying on someone with an actual
> vbox setup to actually test it.
This patch needs some fixes, see below.
> Also, I didn't see anything in
> existing code that would efficiently implement
> virDomainSnapshotNumChildren; there may an API that I'm not aware of,
> but someone else will have to implement that API (Matthias?)
Is virDomainSnapshotNumChildren supposed to only return the number of
direct children, or all children (direct children, grandchildren, etc)
of a snapshot? In the later case they'll have to be counted
recursively, unfortunately.
Both, depending on the value of flags.
> +
> + if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name)))
> + goto cleanup;
At this point you already have the snapshot you want.
> + rc = machine->vtbl->GetCurrentSnapshot(machine,&snap);
> + if (NS_FAILED(rc)) {
> + vboxError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("could not get current snapshot"));
> + goto cleanup;
> + }
This block here gives you the current snapshot, that's not what you want.
That's why we do reviews :)
ACK with this diff applied:
Done.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
2:14 a.m.
On Thu, Sep 29, 2011 at 02:38:24PM -0600, Eric Blake wrote:
Pretty easy to paste together compared to existing functions.
* src/esx/esx_driver.c (esxDomainSnapshotGetParent): New function.
---
I can only compile-test this; I'm relying on someone with an actual
esx setup to actually test it. Also, I didn't see anything in
existing code that would efficiently implement
virDomainSnapshotNumChildren; there may an API that I'm not aware of,
but someone else will have to implement that API (Matthias?)
The 2 patches looks just fine to me but agreed someone with the
setup should check those and give the final ACK :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
3:17 a.m.
2011/9/30 Daniel Veillard <veillard(a)redhat.com>:
On Thu, Sep 29, 2011 at 02:38:24PM -0600, Eric Blake wrote:
> Pretty easy to paste together compared to existing functions.
>
> * src/esx/esx_driver.c (esxDomainSnapshotGetParent): New function.
> ---
>
> I can only compile-test this; I'm relying on someone with an actual
> esx setup to actually test it. Also, I didn't see anything in
> existing code that would efficiently implement
> virDomainSnapshotNumChildren; there may an API that I'm not aware of,
> but someone else will have to implement that API (Matthias?)
The 2 patches looks just fine to me but agreed someone with the
setup should check those and give the final ACK :-)
Daniel
I'll probably have some time this weekend to have a look at it.
--
Matthias Bolte
http://photron.blogspot.com
4:32 a.m.
2011/9/29 Eric Blake <eblake(a)redhat.com>:
Pretty easy to paste together compared to existing functions.
* src/esx/esx_driver.c (esxDomainSnapshotGetParent): New function.
---
I can only compile-test this; I'm relying on someone with an actual
esx setup to actually test it. Also, I didn't see anything in
existing code that would efficiently implement
virDomainSnapshotNumChildren; there may an API that I'm not aware of,
but someone else will have to implement that API (Matthias?)
This will have to be done by esxVI_GetNumberOfSnapshotTrees that just
counts the snapshots in the driver. This is one of things where
vSphere API doesn't translate efficiently to libvirt API.
src/esx/esx_driver.c | 41
+++++++++++++++++++++++++++++++++++++++++
1 files changed, 41 insertions(+), 0 deletions(-)
Looks good, tested, works, ACK :)
--
Matthias Bolte
http://photron.blogspot.com
9 a.m.
On 10/02/2011 03:32 AM, Matthias Bolte wrote:
2011/9/29 Eric Blake<eblake(a)redhat.com>:
> Pretty easy to paste together compared to existing functions.
>
> * src/esx/esx_driver.c (esxDomainSnapshotGetParent): New function.
> ---
>
> I can only compile-test this; I'm relying on someone with an actual
> esx setup to actually test it. Also, I didn't see anything in
> existing code that would efficiently implement
> virDomainSnapshotNumChildren; there may an API that I'm not aware of,
> but someone else will have to implement that API (Matthias?)
This will have to be done by esxVI_GetNumberOfSnapshotTrees that just
counts the snapshots in the driver. This is one of things where
vSphere API doesn't translate efficiently to libvirt API.
Actually, virDomainSnapshotNumChildren has two modes (well, it will once
my API addition series gets reviewed...):
Direct children only, when flags==0 (from looking at
esxVI_GetNumberOfSnapshotTrees, this looks like it is just a count of
the for loop through snapshotTreeList)
All descendants, when flags==VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS, which
is a count of the for loop plus recursion on each child.
Thanks; you pointed me to enough of a hint that I can try and implement it.
Speaking of flags support for various APIs, I'm wondering if
http://libvirt.org/hvsupport.html should be improved to list the version
at which each hypervisor added support for each flag, since flags are
often introduced after the original API, and not all hypervisors support
all flags. I'll try and take a look into that.
> src/esx/esx_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 41 insertions(+), 0 deletions(-)
>
Looks good, tested, works, ACK :)
All right, ESX getParent is now pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
4798
days inactive
4802
days old
7 comments
3 participants
participants (3)
-
Daniel Veillard -
Eric Blake -
Matthias Bolte