On Wed, Aug 6, 2014 at 11:30 AM, Wang Rui <moon.wangrui@xxxxxxxxxx> wrote:
> On 2014/8/6 0:48, Maxime Leroy wrote:
[..]
>> @@ -2805,6 +2841,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
>> case VIR_DOMAIN_DEVICE_NVRAM:
>> case VIR_DOMAIN_DEVICE_LAST:
>> case VIR_DOMAIN_DEVICE_RNG:
>> + case VIR_DOMAIN_DEVICE_IVSHMEM:
>> break;
>
> The function is good in logic. But I think it's better to keep VIR_DOMAIN_DEVICE_LAST
> the last case. (Also case VIR_DOMAIN_DEVICE_RNG should be moved ahead of
> case VIR_DOMAIN_DEVICE_LAST)
>
Good catch ;) I will fix it.
[...]
>> + /* analysis of the ivshmem devices */
>> + if ((n = virXPathNodeSet("./devices/ivshmem", ctxt, &nodes)) < 0) {
>> + goto error;
>> + }
>> + if (n && VIR_ALLOC_N(def->ivshmems, n) < 0)
>> + goto error;
>> +
>> + node = ctxt->node;
>> + for (i = 0; i < n; i++) {
>> + virDomainIvshmemDefPtr ivshmem;
>> + ctxt->node = nodes[i];
>> + ivshmem = virDomainIvshmemDefParseXML(nodes[i], ctxt, flags);
>> + if (!ivshmem)
>> + goto error;
>> +
>> + def->ivshmems[def->nivshmems++] = ivshmem;
>> + }
>> + ctxt->node = node;
>> + VIR_FREE(nodes);
>>
> Here actions: node = ctxt->node; ctxt->node = nodes[i]; ctxt->node = node;
> I see other devices' xml parsing function virDomainXXXParseXML (such as virDomainRNGDefParseXML).
> These actions are in the function virDomainXXXparesXML().
> So are the actions here are redundant? Or should be moved into virDomainIvshmemDefParseXML.
>
The action is not redundant. The virDomainDefParseXML can parse multi
ivshmem devices.
It's why we loop here on the different ivshmem nodes.
In virDomainIvshmemDefParseXML, we iterate on the different nodes of
one ivshmem device
(like msi node, size node, source node).
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list