Re: [PATCH v4 8/9] tools/libxc: x86 hvm save implementation

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 07/05/14 15:14, Ian Campbell wrote:
> On Wed, 2014-04-30 at 19:36 +0100, Andrew Cooper wrote:
>> +static int write_hvm_params(struct context *ctx)
>> +{
>> +    static const unsigned int params[] = {
>> +        HVM_PARAM_STORE_PFN,
>> +        HVM_PARAM_IOREQ_PFN,
>> +        HVM_PARAM_BUFIOREQ_PFN,
>> +        HVM_PARAM_PAGING_RING_PFN,
>> +        HVM_PARAM_ACCESS_RING_PFN,
>> +        HVM_PARAM_SHARING_RING_PFN,
>> +        HVM_PARAM_VM86_TSS,
>> +        HVM_PARAM_CONSOLE_PFN,
>> +        HVM_PARAM_ACPI_IOPORTS_LOCATION,
>> +        HVM_PARAM_VIRIDIAN,
>> +        HVM_PARAM_IDENT_PT,
>> +        HVM_PARAM_PAE_ENABLED,
>> +    };
>> +    static const unsigned int num_params = ARRAY_SIZE(params);
> Blank line between statics and regular please.
>
>> +    xc_interface *xch = ctx->xch;
>> +    struct rec_hvm_params_entry *entries;
>> +    struct rec_hvm_params hdr = {
>> +        .count = 0,
>> +    };
>> +    struct record rec = {
>> +        .type   = REC_TYPE_hvm_params,
>> +        .length = sizeof(hdr),
>> +        .data   = &hdr,
>> +    };
>> +    unsigned int i;
>> +    int rc;
>> +
>> +    entries = malloc(num_params * sizeof(*entries));
> It's small enough that I think you could get away with
> 	struct rec_hvm_params_entry entries[num_params];

I would agree.  (Not my code originally).  I shall fix this up without
malloc.

>
>
>> +    if ( !entries )
>> +    {
>> +        PERROR("HVM params record");
> "Allocating..." (or goes away)
>
>> +    /* Write an END record */
>> +    rc = write_record(ctx, &end);
> Didn't I see a helper for this earlier on? Could it not be in common
> code in any case?
>
> Ian.
>

Probably should be.

~Andrew