Re: [libvirt] [RFC PATCH 2/5] conf: Parse and format ivshmem device XML

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)

> + /* 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.