[libvirt] [PATCH v4 1/2 RESEND] Add NVRAM device
Li Zhang
zhlcindy at gmail.com
Fri Apr 19 03:47:04 UTC 2013
On 2013年04月19日 11:19, Eric Blake wrote:
> On 04/17/2013 11:40 PM, Li Zhang wrote:
>> From: Li Zhang <zhlcindy at linux.vnet.ibm.com>
>>
>> For pSeries guest in QEMU, NVRAM is one kind of spapr-vio device.
>> Users are allowed to specify spapr-vio devices'address.
>> But NVRAM is not supported in libvirt. So this patch is to
>> add NVRAM device to allow users to specify its address.
>>
>> In QEMU, NVRAM device's address is specified by
>> "-global spapr-nvram.reg=xxxxx".
>>
>>
>> +static virDomainNVRAMDefPtr
>> +virDomainNVRAMDefParseXML(const xmlNodePtr node,
>> + unsigned int flags)
> Alignment. The 'c' of const and 'u' of unsigned should be in the same
> column:
>
> virDomainNVRAMDefParseXML(const xmlNodePtr node,
> unsigned int flags)
>
>> +{
>> + virDomainNVRAMDefPtr def;
>> +
>> + if (VIR_ALLOC(def) < 0) {
>> + virReportOOMError();
>> + return NULL;
>> + }
>> +
>> + if ( virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0 )
> Style: no spaces inside () and the expression it contains. This should be:
>
> if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
>
>> + return NULL;
> Memory leak. If the parse fails, you leaked def.
>
>> @@ -14159,6 +14216,21 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
>> }
>>
>> static int
>> +virDomainNVRAMDefFormat(virBufferPtr buf,
>> + virDomainNVRAMDefPtr def,
>> + unsigned int flags)
> Another case of incorrect indentation.
>
>> +{
>> + virBufferAsprintf(buf, " <nvram>\n");
> Use virBufferAddLit when there is nothing to be formatted (reserve
> virBufferAsprintf for use of "%" sequences).
>
>> + if (virDomainDeviceInfoIsSet(&def->info, flags))
>> + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
> You could combine these two into one:
>
> if (virDomainDeviceInfoIsSet(&def->info, flags) &&
> virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
>
> I know Dan acked this, but since you have a memory leak, and since Osier
> suggested improvements for 2/2, please send a v5.
>
Sure.
More information about the libvir-list
mailing list