[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Bug with creating snapshots in ESX in 0.8.0 (patch included)



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]