Re: [RFC V10 4/4] domain snapshot design: libxl

[Ian Campbell <Ian.Campbell@citrix.com>]

On Mon, 2015-01-26 at 11:25 +0800, Chunyan Liu wrote:
> Changes to V9:
>   * update libxl_disk_snapshot structures as Ian suggests
>   * add more descriptions for libxl_disk_snapshot_delete
>   * add two functions for deleting external disk snapshot:
>     libxl_domain_block_rebase and libxl_domain_block_commit
> 
> ===========================================================================
> 
> Libxl Design
> 
> 1. New Structures
> 
> libxl_disk_snapshot_type = Struct("disk_snapshot_type", [
>     (0, "default"),

How is default handled wrt the keyed union? In particualr if default
turns out to mean external, then external_path would need to be non-null
-- how is that fabricated?

Or did you mean s/default/unknown/ suggesting this would be an illegal
input?

>     (1, "internal"),
>     (2, "external"),
>     ])
> 
> libxl_disk_snapshot = Struct("disk_snapshot",[
>     # target disk
>     ("disk",            libxl_device_disk),
> 
>     # disk snapshot name
>     ("name",            string),
> 
>     ("u", KeyedUnion(None, libxl_disk_snapshot_type, "type",
>          [("external", Struct(None, [
> 
>             # disk format for external files. Since external disk snapshot is
>             # implemented with backing file mechanism, the external file disk
>             # format must support backing file. This field can be NULL, then
>             # a proper disk format will be used by default according to the
>             # orignal disk format.
>             ("external_format", libxl_disk_format),
> 
>             # external file path. This field should be non-NULL and a new path.
>             ("external_path",   string),
>             ]))
>          ]))
>     ])
> 
> 
> 2. New Functions
[...]

> /**
>  * libxl_disk_to_snapshot:
>  * @snapshot: array of disk snapshot configuration. Has "nb" members.
>  * @nb: number of disks.
>  * @ctx: libxl context
>  * @domid: domain id
>  * @snapshot (OUTPUT): array of disk snapshot configuration.
>  * @num (OUTPUT): number of disks.

@nb must equal @num, perhaps you actually meant for this to be an IN/OUT
parameter along with @snapshot instead of passing an IN array and an OUT
array?

>  *
>  * Helper function to produce an array of libxl_disk_snapshot.
>  *
>  * Could be used when user doesn't specify disk snapshot information
>  * in creating a domain snapshot, it will by default fill disk snapshot
>  * information according to domain disks:
>  *   By default it will take internal disk snapshot to each domain
>  *   disk that could take internal disk snapshot, otherwise, take
>  *   external disk snapshot.

How does this function determine the external path?