Re: [libvirt] [PATCH 4/5] conf: Refactor virDomainDiskDefParseXML to pass vmdef
On 07/16/2015 08:03 AM, Ján Tomko wrote:
On Mon, Jun 22, 2015 at 05:05:06PM -0400, John Ferlan wrote:
> Rather than passing the def->seclabels and def->nseclabels, refactor
> the API to pass the entire domain definition. This will be used in a
> future patch as well.
I think it would be nicer to separate XML parsing (which would just
record what was in the XML) and auto-generating missing configuration
(like generating drive addresses or checking for conflicts).
Jan
Are you advocating removing virDomainDiskDefAssignAddress() from
virDomainDiskDefParseXML() and relying on qemuParseCommandLineDisk() and
qemuParseCommandLine()?
Following around all the existing "assumptions" especially as they
relate to hotplug and even perhaps blockcopy could be an adventure. Not
to say that I don't disagree with the concept separating parse from
autofill; however, that seems outside the scope of this particular issue
John
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/conf/domain_conf.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e02cd49..6259d4a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6390,8 +6390,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
> xmlNodePtr node,
> xmlXPathContextPtr ctxt,
> virHashTablePtr bootHash,
> - virSecurityLabelDefPtr* vmSeclabels,
> - int nvmSeclabels,
> + const virDomainDef *vmdef,
> unsigned int flags)
> {
> virDomainDiskDefPtr def;
> @@ -6930,8 +6929,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
> ctxt->node = sourceNode;
> if (virSecurityDeviceLabelDefParseXML(&def->src->seclabels,
> &def->src->nseclabels,
> - vmSeclabels,
> - nvmSeclabels,
> + vmdef->seclabels,
> + vmdef->nseclabels,
> ctxt,
> flags) < 0)
> goto error;
> @@ -12256,9 +12255,7 @@ virDomainDeviceDefParse(const char *xmlStr,
> switch ((virDomainDeviceType) dev->type) {
> case VIR_DOMAIN_DEVICE_DISK:
> if (!(dev->data.disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
> - NULL, def->seclabels,
> - def->nseclabels,
> - flags)))
> + NULL, def, flags)))
> goto error;
> break;
> case VIR_DOMAIN_DEVICE_LEASE:
> @@ -12400,9 +12397,7 @@ virDomainDiskDefSourceParse(const char *xmlStr,
>
> flags |= VIR_DOMAIN_DEF_PARSE_DISK_SOURCE;
> if (!(disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
> - NULL, def->seclabels,
> - def->nseclabels,
> - flags)))
> + NULL, def, flags)))
> goto cleanup;
>
> ret = disk->src;
> @@ -15418,8 +15413,7 @@ virDomainDefParseXML(xmlDocPtr xml,
> nodes[i],
> ctxt,
> bootHash,
> - def->seclabels,
> - def->nseclabels,
> + def,
> flags);
> if (!disk)
> goto error;
> --
> 2.1.0
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list