2010/4/20 Chris Wong <wongc-redhat@hoku.net>:
I was testing out the new snapshot on functionality with GSX (VMWare Server 2.0.2) and noticed that it would fail to create a snapshot on a VM with no snapshots. I tracked it down to the esxDomainSnapshotCreateXML call, which would prematurely fail if the Root Snapshot Tree was empty -- which it would be.
Seems like I didn't test this with a VM without snapshots and did notice this problem. Sorry for that.
Verified that both ESXi 4.0 and GSX return an empty snapshot tree. I don't feel this should be an error, and it prevents snapshots from being created.
Yes, an empty root snapshot list is a valid response.
I don't think I saw a fix for this in the master git branch. I'm not too sure if this is the proper path to go down, but at the very least it fixes my specific problem. If there is another fix or if I completely missed this point, let me know.
The problem has not been fixed yet, because I wasn't aware of it until now.
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index fea887a..70bfc2c 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3330,12 +3330,16 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, (priv->host, domain->uuid, NULL, &virtualMachine, priv->autoAnswer) < 0 || esxVI_LookupRootSnapshotTreeList(priv->host, domain->uuid, - &rootSnapshotList) < 0 || - esxVI_GetSnapshotTreeByName(rootSnapshotList, def->name, - &snapshotTree, &snapshotTreeParent, - esxVI_Occurrence_OptionalItem) < 0) { + &rootSnapshotList) < 0) { goto failure; } + if (rootSnapshotList) { + if (esxVI_GetSnapshotTreeByName(rootSnapshotList, def->name, + &snapshotTree, &snapshotTreeParent, + esxVI_Occurrence_OptionalItem) < 0) { + goto failure; + } + } if (snapshotTree != NULL) { ESX_ERROR(VIR_ERR_OPERATION_INVALID, diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 89ef2dd..93ef36b 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2561,12 +2561,6 @@ esxVI_LookupRootSnapshotTreeList } } - if (*rootSnapshotTreeList == NULL) { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not lookup root snapshot list")); - goto failure; - } - cleanup: esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&virtualMachine);
Your patch looks good, but I'll have to validate that removing the NULL check from esxVI_LookupRootSnapshotTreeList doesn't break any other caller of this function. Thanks for reporting this issue. Matthias