[libvirt] [PATCH 2/2] snapshot: implement getparent for vbox

[Eric Blake]

On 10/02/2011 02:50 AM, Matthias Bolte wrote:
> 2011/9/29 Eric Blake<eblake at 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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org