[libvirt] [PATCH 0/4] conf: Invoke post-parse callbacks after parsing private XML
This will allow the qemu post-parse callbacks to access 'qemuCaps' which is stored in the status XML when we restore the status of VMs. It will become useful in cases when we need to decide whether storage authentication/encryption secret aliases will need to be regenerated. Peter Krempa (4): conf: domain: Remove code accessing 'bootHash' from the post-parse infrestructure conf: domain: Invoke post-parse callbacks after parsing private XML parts conf: domain: Allow passing in 'parseOpaque' for post-parse of status XML qemu: domain: Pass 'qemuCaps' to post parse callbacks when parsing status XML src/conf/domain_conf.c | 97 ++++++++++++++++++++++++++------------------------ src/conf/domain_conf.h | 5 +++ src/qemu/qemu_domain.c | 10 ++++++ 3 files changed, 65 insertions(+), 47 deletions(-) -- 2.16.2
There is only one block accessing it. Removing it is necessary so that post parse callbacks can later be invoked after the hypervisor private data callback so that e.g. qemuCaps are properly filled for the postparse callbacks. As the function signature of virDomainDefPostParseInternal does not differ from virDomainDefPostParse now, the wrapper can be dropped. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 62 +++++++++++++++++++------------------------------- 1 file changed, 24 insertions(+), 38 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d6ac47c629..eead28f5fb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4941,8 +4941,7 @@ virDomainDefPostParseCPU(virDomainDefPtr def) static int virDomainDefPostParseCommon(virDomainDefPtr def, - struct virDomainDefPostParseDeviceIteratorData *data, - virHashTablePtr bootHash) + struct virDomainDefPostParseDeviceIteratorData *data) { size_t i; @@ -4953,20 +4952,6 @@ virDomainDefPostParseCommon(virDomainDefPtr def, return -1; } - if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && bootHash) { - if (def->os.nBootDevs > 0 && virHashSize(bootHash) > 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("per-device boot elements cannot be used" - " together with os/boot elements")); - return -1; - } - - if (def->os.nBootDevs == 0 && virHashSize(bootHash) == 0) { - def->os.nBootDevs = 1; - def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK; - } - } - if (virDomainVcpuDefPostParse(def) < 0) return -1; @@ -5059,13 +5044,12 @@ virDomainDefPostParseCheckFailure(virDomainDefPtr def, } -static int -virDomainDefPostParseInternal(virDomainDefPtr def, - virCapsPtr caps, - unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt, - void *parseOpaque, - virHashTablePtr bootHash) +int +virDomainDefPostParse(virDomainDefPtr def, + virCapsPtr caps, + unsigned int parseFlags, + virDomainXMLOptionPtr xmlopt, + void *parseOpaque) { int ret = -1; bool localParseOpaque = false; @@ -5121,7 +5105,7 @@ virDomainDefPostParseInternal(virDomainDefPtr def, if (virDomainDefPostParseCheckFailure(def, parseFlags, ret) < 0) goto cleanup; - if ((ret = virDomainDefPostParseCommon(def, &data, bootHash)) < 0) + if ((ret = virDomainDefPostParseCommon(def, &data)) < 0) goto cleanup; if (xmlopt->config.assignAddressesCallback) { @@ -5148,18 +5132,6 @@ virDomainDefPostParseInternal(virDomainDefPtr def, } -int -virDomainDefPostParse(virDomainDefPtr def, - virCapsPtr caps, - unsigned int parseFlags, - virDomainXMLOptionPtr xmlopt, - void *parseOpaque) -{ - return virDomainDefPostParseInternal(def, caps, parseFlags, xmlopt, - parseOpaque, NULL); -} - - /** * virDomainDiskAddressDiskBusCompatibility: * @bus: disk bus type @@ -20502,9 +20474,23 @@ virDomainDefParseXML(xmlDocPtr xml, (def->ns.parse)(xml, root, ctxt, &def->namespaceData) < 0) goto error; + /* Fill in default boot device if none was present */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { + if (def->os.nBootDevs > 0 && virHashSize(bootHash) > 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("per-device boot elements cannot be used" + " together with os/boot elements")); + goto error; + } + + if (def->os.nBootDevs == 0 && virHashSize(bootHash) == 0) { + def->os.nBootDevs = 1; + def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK; + } + } + /* callback to fill driver specific domain aspects */ - if (virDomainDefPostParseInternal(def, caps, flags, xmlopt, parseOpaque, - bootHash) < 0) + if (virDomainDefPostParse(def, caps, flags, xmlopt, parseOpaque) < 0) goto error; /* valdiate configuration */ -- 2.16.2
On Mon, May 28, 2018 at 11:36:44AM +0200, Peter Krempa wrote:
There is only one block accessing it. Removing it is necessary so that post parse callbacks can later be invoked after the hypervisor private data callback so that e.g. qemuCaps are properly filled for the postparse callbacks.
As the function signature of virDomainDefPostParseInternal does not differ from virDomainDefPostParse now, the wrapper can be dropped.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 62 +++++++++++++++++++------------------------------- 1 file changed, 24 insertions(+), 38 deletions(-)
@@ -20502,9 +20474,23 @@ virDomainDefParseXML(xmlDocPtr xml, (def->ns.parse)(xml, root, ctxt, &def->namespaceData) < 0) goto error;
+ /* Fill in default boot device if none was present */ + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { + if (def->os.nBootDevs > 0 && virHashSize(bootHash) > 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("per-device boot elements cannot be used" + " together with os/boot elements")); + goto error; + } + + if (def->os.nBootDevs == 0 && virHashSize(bootHash) == 0) { + def->os.nBootDevs = 1; + def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK; + } + } +
I don't like moving this back into ParseXML. Can we remove it completely instead? https://www.redhat.com/archives/libvir-list/2018-May/msg02041.html cover.1527515594.git.jtomko@redhat.com Jano
When parsing status XML the post-parse callbacks can't access any private data present in the status XML as the private bits were parsed after invoking post-parse callbacks. Move the invocation so that everything is parsed first. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eead28f5fb..43ae5ad96a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18746,7 +18746,6 @@ virDomainDefParseXML(xmlDocPtr xml, xmlXPathContextPtr ctxt, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, - void *parseOpaque, unsigned int flags) { xmlNodePtr *nodes = NULL, node = NULL; @@ -20489,14 +20488,6 @@ virDomainDefParseXML(xmlDocPtr xml, } } - /* callback to fill driver specific domain aspects */ - if (virDomainDefPostParse(def, caps, flags, xmlopt, parseOpaque) < 0) - goto error; - - /* valdiate configuration */ - if (virDomainDefValidate(def, caps, flags, xmlopt) < 0) - goto error; - virHashFree(bootHash); return def; @@ -20539,7 +20530,7 @@ virDomainObjParseXML(xmlDocPtr xml, oldnode = ctxt->node; ctxt->node = config; - obj->def = virDomainDefParseXML(xml, config, ctxt, caps, xmlopt, NULL, flags); + obj->def = virDomainDefParseXML(xml, config, ctxt, caps, xmlopt, flags); ctxt->node = oldnode; if (!obj->def) goto error; @@ -20598,6 +20589,14 @@ virDomainObjParseXML(xmlDocPtr xml, xmlopt->privateData.parse(ctxt, obj, &xmlopt->config) < 0) goto error; + /* callback to fill driver specific domain aspects */ + if (virDomainDefPostParse(obj->def, caps, flags, xmlopt, NULL) < 0) + goto error; + + /* valdiate configuration */ + if (virDomainDefValidate(obj->def, caps, flags, xmlopt) < 0) + goto error; + return obj; error: @@ -20660,6 +20659,7 @@ virDomainDefParseNode(xmlDocPtr xml, { xmlXPathContextPtr ctxt = NULL; virDomainDefPtr def = NULL; + virDomainDefPtr ret = NULL; if (!virXMLNodeNameEqual(root, "domain")) { virReportError(VIR_ERR_XML_ERROR, @@ -20676,11 +20676,24 @@ virDomainDefParseNode(xmlDocPtr xml, } ctxt->node = root; - def = virDomainDefParseXML(xml, root, ctxt, caps, xmlopt, parseOpaque, flags); + + if (!(def = virDomainDefParseXML(xml, root, ctxt, caps, xmlopt, flags))) + goto cleanup; + + /* callback to fill driver specific domain aspects */ + if (virDomainDefPostParse(def, caps, flags, xmlopt, parseOpaque) < 0) + goto cleanup; + + /* valdiate configuration */ + if (virDomainDefValidate(def, caps, flags, xmlopt) < 0) + goto cleanup; + + VIR_STEAL_PTR(ret, def); cleanup: + virDomainDefFree(def); xmlXPathFreeContext(ctxt); - return def; + return ret; } -- 2.16.2
On Mon, May 28, 2018 at 11:36:45AM +0200, Peter Krempa wrote:
When parsing status XML the post-parse callbacks can't access any private data present in the status XML as the private bits were parsed after invoking post-parse callbacks.
Move the invocation so that everything is parsed first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The status XML parser function virDomainObjParseXML could not pass in parseOpaque into the post parse callbacks. Add a callback which will allow hypervisor drivers to fill it from the 'virDomainObj' data. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 6 +++++- src/conf/domain_conf.h | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 43ae5ad96a..cc38c441dc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20518,6 +20518,7 @@ virDomainObjParseXML(xmlDocPtr xml, int n; int state; int reason = 0; + void *parseOpaque = NULL; if (!(obj = virDomainObjNew(xmlopt))) return NULL; @@ -20589,8 +20590,11 @@ virDomainObjParseXML(xmlDocPtr xml, xmlopt->privateData.parse(ctxt, obj, &xmlopt->config) < 0) goto error; + if (xmlopt->privateData.getParseOpaque) + parseOpaque = xmlopt->privateData.getParseOpaque(obj); + /* callback to fill driver specific domain aspects */ - if (virDomainDefPostParse(obj->def, caps, flags, xmlopt, NULL) < 0) + if (virDomainDefPostParse(obj->def, caps, flags, xmlopt, parseOpaque) < 0) goto error; /* valdiate configuration */ diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b7e52a1e03..651dcc445f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2662,6 +2662,8 @@ typedef int (*virDomainXMLPrivateDataParseFunc)(xmlXPathContextPtr, virDomainObjPtr, virDomainDefParserConfigPtr); +typedef void *(*virDomainXMLPrivateDataGetParseOpaqueFunc)(virDomainObjPtr vm); + typedef int (*virDomainXMLPrivateDataStorageSourceParseFunc)(xmlXPathContextPtr ctxt, virStorageSourcePtr src); typedef int (*virDomainXMLPrivateDataStorageSourceFormatFunc)(virStorageSourcePtr src, @@ -2680,6 +2682,9 @@ struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataNewFunc chrSourceNew; virDomainXMLPrivateDataFormatFunc format; virDomainXMLPrivateDataParseFunc parse; + /* following function shall return a pointer which will be used as the + * 'parseOpaque' argument for virDomainDefPostParse */ + virDomainXMLPrivateDataGetParseOpaqueFunc getParseOpaque; virDomainXMLPrivateDataStorageSourceParseFunc storageParse; virDomainXMLPrivateDataStorageSourceFormatFunc storageFormat; }; -- 2.16.2
On Mon, May 28, 2018 at 11:36:46AM +0200, Peter Krempa wrote:
The status XML parser function virDomainObjParseXML could not pass in parseOpaque into the post parse callbacks. Add a callback which will allow hypervisor drivers to fill it from the 'virDomainObj' data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 6 +++++- src/conf/domain_conf.h | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
When status XML was parsed the post-parse callbacks could not access qemu caps and potentially upgrade the definition according to the present caps. Implement the callback to pass it in. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 47910acb83..0b99698046 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2801,6 +2801,15 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, } +static void * +qemuDomainObjPrivateXMLGetParseOpaque(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + return priv->qemuCaps; +} + + virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .alloc = qemuDomainObjPrivateAlloc, .free = qemuDomainObjPrivateFree, @@ -2809,6 +2818,7 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .chrSourceNew = qemuDomainChrSourcePrivateNew, .parse = qemuDomainObjPrivateXMLParse, .format = qemuDomainObjPrivateXMLFormat, + .getParseOpaque = qemuDomainObjPrivateXMLGetParseOpaque, .storageParse = qemuStorageSourcePrivateDataParse, .storageFormat = qemuStorageSourcePrivateDataFormat, }; -- 2.16.2
On Mon, May 28, 2018 at 11:36:47AM +0200, Peter Krempa wrote:
When status XML was parsed the post-parse callbacks could not access qemu caps and potentially upgrade the definition according to the present caps. Implement the callback to pass it in.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa