[libvirt] [PATCH 00/15] qemu: Add possibility to control qemu capabilities for debug (blockdev-add saga spin-off)
While this is not strictly required for blockdev to work it was created as a part of the testing (and possibly regression mitigation) effort. These patches introduce the option to add or remove qemu capabilities via the qemu XML namespace custom elements and remove features via qemu.conf. This series depends (very slightly) on the series deleting qemu parsing. To make applying it easy I'd sugges you fetrch everything needed by: git fetch https://github.com/pipo/libvirt.git features-filter Peter Krempa (15): qemu: Rename qemuDomainCmdlineDefPtr to qemuDomainXmlNsDefPtr qemu: Move qemuDomainXmlNsDef(Free) from qemu_conf.(ch) qemu: domain: Use virStringListFreeCount in qemuDomainXmlNsDefFree qemu: Extract parsing of qemu namespace arguments into separate function qemu: Extract parsing of qemu namespace env vars into separate function qemu: Refactor qemuDomainDefNamespaceParse qemu: domain: Split out commandline namespace data formatting conf: Add taint flag for custom hypervisor features qemu: Add support for controling qemu capabilities via the qemu XML namespace qemu: domain: Add support for modifying qemu capability list via qemu namespace tests: qemuxml2argv: Modernize and fork 'qemu-ns' test tests: qemuxml2argv: Test qemu namespace capability tweaking qemu: Remove unused var 'corestr' from virQEMUDriverConfigLoadFile qemu: conf: Add debug option to allow disabling qemu capabilities qemu: Implement support for 'capability_filters' config option docs/drvqemu.html.in | 30 ++ docs/schemas/domaincommon.rng | 19 ++ src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/libvirtd_qemu.aug | 3 + src/qemu/qemu.conf | 7 + src/qemu/qemu_command.c | 16 +- src/qemu/qemu_conf.c | 39 ++- src/qemu/qemu_conf.h | 17 +- src/qemu/qemu_domain.c | 285 +++++++++++++----- src/qemu/qemu_domain.h | 19 ++ src/qemu/qemu_process.c | 55 ++++ src/qemu/test_libvirtd_qemu.aug.in | 3 + tests/qemuxml2argvdata/qemu-ns.args | 33 -- .../qemu-ns.x86_64-4.0.0.args | 41 +++ .../qemu-ns.x86_64-latest.args | 41 +++ tests/qemuxml2argvdata/qemu-ns.xml | 5 + tests/qemuxml2argvtest.c | 3 +- 18 files changed, 458 insertions(+), 160 deletions(-) delete mode 100644 tests/qemuxml2argvdata/qemu-ns.args create mode 100644 tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args create mode 100644 tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args -- 2.21.0
The data injected via the namespace may contain also other things than commandline passthrough definitions. Rename it to make it more universal. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 16 ++++++++-------- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_conf.h | 8 ++++---- src/qemu/qemu_domain.c | 12 ++++++------ 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 59dc134785..637de74e90 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10881,14 +10881,14 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); if (def->namespaceData) { - qemuDomainCmdlineDefPtr qemucmd; - - qemucmd = def->namespaceData; - for (i = 0; i < qemucmd->num_args; i++) - virCommandAddArg(cmd, qemucmd->args[i]); - for (i = 0; i < qemucmd->num_env; i++) - virCommandAddEnvPair(cmd, qemucmd->env_name[i], - NULLSTR_EMPTY(qemucmd->env_value[i])); + qemuDomainXmlNsDefPtr qemuxmlns; + + qemuxmlns = def->namespaceData; + for (i = 0; i < qemuxmlns->num_args; i++) + virCommandAddArg(cmd, qemuxmlns->args[i]); + for (i = 0; i < qemuxmlns->num_env; i++) + virCommandAddEnvPair(cmd, qemuxmlns->env_name[i], + NULLSTR_EMPTY(qemuxmlns->env_value[i])); } if (qemuBuildSeccompSandboxCommandLine(cmd, cfg, qemuCaps) < 0) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index daea11dacb..afff77b1f3 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -94,7 +94,7 @@ qemuDriverUnlock(virQEMUDriverPtr driver) virMutexUnlock(&driver->lock); } -void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def) +void qemuDomainXmlNsDefFree(qemuDomainXmlNsDefPtr def) { size_t i; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 983e74a3cf..26aa96f194 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -297,9 +297,9 @@ struct _virQEMUDriver { virHashAtomicPtr migrationErrors; }; -typedef struct _qemuDomainCmdlineDef qemuDomainCmdlineDef; -typedef qemuDomainCmdlineDef *qemuDomainCmdlineDefPtr; -struct _qemuDomainCmdlineDef { +typedef struct _qemuDomainXmlNsDef qemuDomainXmlNsDef; +typedef qemuDomainXmlNsDef *qemuDomainXmlNsDefPtr; +struct _qemuDomainXmlNsDef { size_t num_args; char **args; @@ -310,7 +310,7 @@ struct _qemuDomainCmdlineDef { -void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def); +void qemuDomainXmlNsDefFree(qemuDomainXmlNsDefPtr def); virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8262107311..10215ffa47 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3094,9 +3094,9 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { static void qemuDomainDefNamespaceFree(void *nsdata) { - qemuDomainCmdlineDefPtr cmd = nsdata; + qemuDomainXmlNsDefPtr cmd = nsdata; - qemuDomainCmdlineDefFree(cmd); + qemuDomainXmlNsDefFree(cmd); } static int @@ -3105,7 +3105,7 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, xmlXPathContextPtr ctxt, void **data) { - qemuDomainCmdlineDefPtr cmd = NULL; + qemuDomainXmlNsDefPtr cmd = NULL; bool uses_qemu_ns = false; xmlNodePtr *nodes = NULL; int n; @@ -3205,7 +3205,7 @@ static int qemuDomainDefNamespaceFormatXML(virBufferPtr buf, void *nsdata) { - qemuDomainCmdlineDefPtr cmd = nsdata; + qemuDomainXmlNsDefPtr cmd = nsdata; size_t i; if (!cmd->num_args && !cmd->num_env) @@ -8182,8 +8182,8 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HOOK, logCtxt); if (obj->def->namespaceData) { - qemuDomainCmdlineDefPtr qemucmd = obj->def->namespaceData; - if (qemucmd->num_args || qemucmd->num_env) + qemuDomainXmlNsDefPtr qemuxmlns = obj->def->namespaceData; + if (qemuxmlns->num_args || qemuxmlns->num_env) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CUSTOM_ARGV, logCtxt); } -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:10PM +0200, Peter Krempa wrote:
The data injected via the namespace may contain also other things than commandline passthrough definitions. Rename it to make it more universal.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 16 ++++++++-------- src/qemu/qemu_conf.c | 2 +- src/qemu/qemu_conf.h | 8 ++++---- src/qemu/qemu_domain.c | 12 ++++++------ 4 files changed, 19 insertions(+), 19 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
qemu_conf.c deals with the configuration file. Better fit for the structure and freeing function will be qemu_domain.c where the rest of the namespace parsing/formatting stuff resides. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_conf.c | 20 -------------------- src/qemu/qemu_conf.h | 15 --------------- src/qemu/qemu_domain.c | 21 +++++++++++++++++++++ src/qemu/qemu_domain.h | 13 +++++++++++++ 4 files changed, 34 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index afff77b1f3..d981001600 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -94,26 +94,6 @@ qemuDriverUnlock(virQEMUDriverPtr driver) virMutexUnlock(&driver->lock); } -void qemuDomainXmlNsDefFree(qemuDomainXmlNsDefPtr def) -{ - size_t i; - - if (!def) - return; - - for (i = 0; i < def->num_args; i++) - VIR_FREE(def->args[i]); - for (i = 0; i < def->num_env; i++) { - VIR_FREE(def->env_name[i]); - VIR_FREE(def->env_value[i]); - } - VIR_FREE(def->args); - VIR_FREE(def->env_name); - VIR_FREE(def->env_value); - VIR_FREE(def); -} - - #ifndef DEFAULT_LOADER_NVRAM # define DEFAULT_LOADER_NVRAM \ "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd:" \ diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 26aa96f194..386e774653 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -297,21 +297,6 @@ struct _virQEMUDriver { virHashAtomicPtr migrationErrors; }; -typedef struct _qemuDomainXmlNsDef qemuDomainXmlNsDef; -typedef qemuDomainXmlNsDef *qemuDomainXmlNsDefPtr; -struct _qemuDomainXmlNsDef { - size_t num_args; - char **args; - - unsigned int num_env; - char **env_name; - char **env_value; -}; - - - -void qemuDomainXmlNsDefFree(qemuDomainXmlNsDefPtr def); - virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged); int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 10215ffa47..74d5186bc7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3091,6 +3091,27 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { }; +static void +qemuDomainXmlNsDefFree(qemuDomainXmlNsDefPtr def) +{ + size_t i; + + if (!def) + return; + + for (i = 0; i < def->num_args; i++) + VIR_FREE(def->args[i]); + for (i = 0; i < def->num_env; i++) { + VIR_FREE(def->env_name[i]); + VIR_FREE(def->env_value[i]); + } + VIR_FREE(def->args); + VIR_FREE(def->env_name); + VIR_FREE(def->env_value); + VIR_FREE(def); +} + + static void qemuDomainDefNamespaceFree(void *nsdata) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f92f0dbc27..474c7b613a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -531,6 +531,19 @@ struct _qemuDomainSaveCookie { virCPUDefPtr cpu; }; + +typedef struct _qemuDomainXmlNsDef qemuDomainXmlNsDef; +typedef qemuDomainXmlNsDef *qemuDomainXmlNsDefPtr; +struct _qemuDomainXmlNsDef { + size_t num_args; + char **args; + + unsigned int num_env; + char **env_name; + char **env_value; +}; + + qemuDomainSaveCookiePtr qemuDomainSaveCookieNew(virDomainObjPtr vm); const char *qemuDomainAsyncJobPhaseToString(qemuDomainAsyncJob job, -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:11PM +0200, Peter Krempa wrote:
qemu_conf.c deals with the configuration file. Better fit for the structure and freeing function will be qemu_domain.c where the rest of the namespace parsing/formatting stuff resides.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_conf.c | 20 -------------------- src/qemu/qemu_conf.h | 15 --------------- src/qemu/qemu_domain.c | 21 +++++++++++++++++++++ src/qemu/qemu_domain.h | 13 +++++++++++++ 4 files changed, 34 insertions(+), 35 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 74d5186bc7..362b860c73 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3094,20 +3094,13 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { static void qemuDomainXmlNsDefFree(qemuDomainXmlNsDefPtr def) { - size_t i; - if (!def) return; - for (i = 0; i < def->num_args; i++) - VIR_FREE(def->args[i]); - for (i = 0; i < def->num_env; i++) { - VIR_FREE(def->env_name[i]); - VIR_FREE(def->env_value[i]); - } - VIR_FREE(def->args); - VIR_FREE(def->env_name); - VIR_FREE(def->env_value); + virStringListFreeCount(def->args, def->num_args); + virStringListFreeCount(def->env_name, def->num_env); + virStringListFreeCount(def->env_value, def->num_env); + VIR_FREE(def); } -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:12PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Simplify the main function by splitting out how we parse the extra passthrough commandline arguments. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 51 +++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 362b860c73..3dfe4fc544 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3113,6 +3113,36 @@ qemuDomainDefNamespaceFree(void *nsdata) qemuDomainXmlNsDefFree(cmd); } + +static int +qemuDomainDefNamespaceParseCommandlineArgs(qemuDomainXmlNsDefPtr nsdef, + xmlXPathContextPtr ctxt) +{ + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; + ssize_t nnodes; + size_t i; + + if ((nnodes = virXPathNodeSet("./qemu:commandline/qemu:arg", ctxt, &nodes)) < 0) + return -1; + + if (nnodes == 0) + return 0; + + if (VIR_ALLOC_N(nsdef->args, nnodes) < 0) + return -1; + + for (i = 0; i < nnodes; i++) { + if (!(nsdef->args[nsdef->num_args++] = virXMLPropString(nodes[i], "value"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No qemu command-line argument specified")); + return -1; + } + } + + return 0; +} + + static int qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, xmlNodePtr root ATTRIBUTE_UNUSED, @@ -3135,26 +3165,11 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, if (VIR_ALLOC(cmd) < 0) return -1; - /* first handle the extra command-line arguments */ - n = virXPathNodeSet("./qemu:commandline/qemu:arg", ctxt, &nodes); - if (n < 0) - goto error; - uses_qemu_ns |= n > 0; - - if (n && VIR_ALLOC_N(cmd->args, n) < 0) + if (qemuDomainDefNamespaceParseCommandlineArgs(cmd, ctxt) < 0) goto error; - for (i = 0; i < n; i++) { - cmd->args[cmd->num_args] = virXMLPropString(nodes[i], "value"); - if (cmd->args[cmd->num_args] == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("No qemu command-line argument specified")); - goto error; - } - cmd->num_args++; - } - - VIR_FREE(nodes); + if (cmd->num_args > 0) + uses_qemu_ns = true; /* now handle the extra environment variables */ n = virXPathNodeSet("./qemu:commandline/qemu:env", ctxt, &nodes); -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:13PM +0200, Peter Krempa wrote:
Simplify the main function by splitting out how we parse the extra passthrough commandline arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 51 +++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 18 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Simplify the main function by splitting out how we parse the extra passthrough environment variables. Note that the validation function checks that the first letter must be a character or underscore which makes the check whether the name is redundant. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 111 ++++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 52 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3dfe4fc544..5c4bca1c6a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3143,6 +3143,62 @@ qemuDomainDefNamespaceParseCommandlineArgs(qemuDomainXmlNsDefPtr nsdef, } +static int +qemuDomainDefNamespaceParseCommandlineEnvNameValidate(const char *envname) +{ + if (!c_isalpha(envname[0]) && envname[0] != '_') { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Invalid environment name, it must begin with a letter or underscore")); + return -1; + } + + if (strspn(envname, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_") != strlen(envname)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Invalid environment name, it must contain only alphanumerics and underscore")); + return -1; + } + + return 0; +} + + +static int +qemuDomainDefNamespaceParseCommandlineEnv(qemuDomainXmlNsDefPtr nsdef, + xmlXPathContextPtr ctxt) +{ + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; + ssize_t nnodes; + size_t i; + + if ((nnodes = virXPathNodeSet("./qemu:commandline/qemu:env", ctxt, &nodes)) < 0) + return -1; + + if (nnodes == 0) + return 0; + + if (VIR_ALLOC_N(nsdef->env_name, nnodes) < 0 || + VIR_ALLOC_N(nsdef->env_value, nnodes) < 0) + return -1; + + for (i = 0; i < nnodes; i++) { + if (!(nsdef->env_name[nsdef->num_env] = virXMLPropString(nodes[i], "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No qemu environment name specified")); + return -1; + } + + if (qemuDomainDefNamespaceParseCommandlineEnvNameValidate(nsdef->env_name[nsdef->num_env]) < 0) + return -1; + + nsdef->env_value[nsdef->num_env] = virXMLPropString(nodes[i], "value"); + /* a NULL value for command is allowed, since it might be empty */ + nsdef->num_env++; + } + + return 0; +} + + static int qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, xmlNodePtr root ATTRIBUTE_UNUSED, @@ -3151,9 +3207,6 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, { qemuDomainXmlNsDefPtr cmd = NULL; bool uses_qemu_ns = false; - xmlNodePtr *nodes = NULL; - int n; - size_t i; if (xmlXPathRegisterNs(ctxt, BAD_CAST "qemu", BAD_CAST QEMU_NAMESPACE_HREF) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3165,58 +3218,13 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, if (VIR_ALLOC(cmd) < 0) return -1; - if (qemuDomainDefNamespaceParseCommandlineArgs(cmd, ctxt) < 0) + if (qemuDomainDefNamespaceParseCommandlineArgs(cmd, ctxt) < 0 || + qemuDomainDefNamespaceParseCommandlineEnv(cmd, ctxt) < 0) goto error; - if (cmd->num_args > 0) + if (cmd->num_args > 0 || cmd->num_env > 0) uses_qemu_ns = true; - /* now handle the extra environment variables */ - n = virXPathNodeSet("./qemu:commandline/qemu:env", ctxt, &nodes); - if (n < 0) - goto error; - uses_qemu_ns |= n > 0; - - if (n && VIR_ALLOC_N(cmd->env_name, n) < 0) - goto error; - - if (n && VIR_ALLOC_N(cmd->env_value, n) < 0) - goto error; - - for (i = 0; i < n; i++) { - char *tmp; - - tmp = virXMLPropString(nodes[i], "name"); - if (tmp == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("No qemu environment name specified")); - goto error; - } - if (tmp[0] == '\0') { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Empty qemu environment name specified")); - goto error; - } - if (!c_isalpha(tmp[0]) && tmp[0] != '_') { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Invalid environment name, it must begin with a letter or underscore")); - goto error; - } - if (strspn(tmp, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789_") != strlen(tmp)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Invalid environment name, it must contain only alphanumerics and underscore")); - goto error; - } - - cmd->env_name[cmd->num_env] = tmp; - - cmd->env_value[cmd->num_env] = virXMLPropString(nodes[i], "value"); - /* a NULL value for command is allowed, since it might be empty */ - cmd->num_env++; - } - - VIR_FREE(nodes); - if (uses_qemu_ns) *data = cmd; else @@ -3225,7 +3233,6 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, return 0; error: - VIR_FREE(nodes); qemuDomainDefNamespaceFree(cmd); return -1; } -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:14PM +0200, Peter Krempa wrote:
Simplify the main function by splitting out how we parse the extra passthrough environment variables.
Note that the validation function checks that the first letter must be a character or underscore which makes the check whether the name is redundant.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 111 ++++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 52 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Rename 'cmd' to 'nsdef' and improve the control flow. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5c4bca1c6a..ee8c9397e0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3205,8 +3205,8 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, xmlXPathContextPtr ctxt, void **data) { - qemuDomainXmlNsDefPtr cmd = NULL; - bool uses_qemu_ns = false; + qemuDomainXmlNsDefPtr nsdata = NULL; + int ret = -1; if (xmlXPathRegisterNs(ctxt, BAD_CAST "qemu", BAD_CAST QEMU_NAMESPACE_HREF) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3215,26 +3215,21 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, return -1; } - if (VIR_ALLOC(cmd) < 0) + if (VIR_ALLOC(nsdata) < 0) return -1; - if (qemuDomainDefNamespaceParseCommandlineArgs(cmd, ctxt) < 0 || - qemuDomainDefNamespaceParseCommandlineEnv(cmd, ctxt) < 0) - goto error; - - if (cmd->num_args > 0 || cmd->num_env > 0) - uses_qemu_ns = true; + if (qemuDomainDefNamespaceParseCommandlineArgs(nsdata, ctxt) < 0 || + qemuDomainDefNamespaceParseCommandlineEnv(nsdata, ctxt) < 0) + goto cleanup; - if (uses_qemu_ns) - *data = cmd; - else - VIR_FREE(cmd); + if (nsdata->num_args > 0 || nsdata->num_env > 0) + VIR_STEAL_PTR(*data, nsdata); - return 0; + ret = 0; - error: - qemuDomainDefNamespaceFree(cmd); - return -1; + cleanup: + qemuDomainDefNamespaceFree(nsdata); + return ret; } static int -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:15PM +0200, Peter Krempa wrote:
Rename 'cmd' to 'nsdef' and improve the control flow.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Separate it from qemuDomainDefNamespaceFormatXML. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee8c9397e0..864071220b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3232,15 +3232,15 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, return ret; } -static int -qemuDomainDefNamespaceFormatXML(virBufferPtr buf, - void *nsdata) + +static void +qemuDomainDefNamespaceFormatXMLCommandline(virBufferPtr buf, + qemuDomainXmlNsDefPtr cmd) { - qemuDomainXmlNsDefPtr cmd = nsdata; size_t i; if (!cmd->num_args && !cmd->num_env) - return 0; + return; virBufferAddLit(buf, "<qemu:commandline>\n"); virBufferAdjustIndent(buf, 2); @@ -3257,6 +3257,17 @@ qemuDomainDefNamespaceFormatXML(virBufferPtr buf, virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</qemu:commandline>\n"); +} + + +static int +qemuDomainDefNamespaceFormatXML(virBufferPtr buf, + void *nsdata) +{ + qemuDomainXmlNsDefPtr cmd = nsdata; + + qemuDomainDefNamespaceFormatXMLCommandline(buf, cmd); + return 0; } -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:16PM +0200, Peter Krempa wrote:
Separate it from qemuDomainDefNamespaceFormatXML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Upcomming patches will allow enabling/disabling custom hypervisor features for debugging/testing purposes via the qemu namespace. Add a taint flag where we will flag such a domain so it's obvious what's happening. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ee651e7742..5503f081f1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -107,6 +107,7 @@ VIR_ENUM_IMPL(virDomainTaint, "cdrom-passthrough", "custom-dtb", "custom-ga-command", + "custom-hypervisor-feature", ); VIR_ENUM_IMPL(virDomainVirt, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2878050672..c1b5fc1337 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2530,6 +2530,7 @@ typedef enum { VIR_DOMAIN_TAINT_CDROM_PASSTHROUGH,/* CDROM passthrough */ VIR_DOMAIN_TAINT_CUSTOM_DTB, /* Custom device tree blob was specified */ VIR_DOMAIN_TAINT_CUSTOM_GA_COMMAND, /* Custom guest agent command */ + VIR_DOMAIN_TAINT_CUSTOM_HYPERVISOR_FEATURE, /* custom hypervisor feature control */ VIR_DOMAIN_TAINT_LAST } virDomainTaintFlags; -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:17PM +0200, Peter Krempa wrote:
Upcomming patches will allow enabling/disabling custom hypervisor
Upcoming
features for debugging/testing purposes via the qemu namespace.
Add a taint flag where we will flag such a domain so it's obvious what's happening.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + 2 files changed, 2 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Similarly how we allow adding arbitrary command line arguments and environment variables this patch introduces the ability to control libvirt's perception of the qemu process by tweaking the capability bits for testing purposes. The idea is to allow developers and users either test a new feature by enabling it early or disabling it to see whether it introduced regressions. This feature is not meant for production use though, so users should handle it with care. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/drvqemu.html.in | 30 ++++++++++++ docs/schemas/domaincommon.rng | 19 ++++++++ src/qemu/qemu_domain.c | 76 +++++++++++++++++++++++++++++- src/qemu/qemu_domain.h | 6 +++ tests/qemuxml2argvdata/qemu-ns.xml | 5 ++ 5 files changed, 134 insertions(+), 2 deletions(-) diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index cf36f6cc7b..aa61ee5ced 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -519,6 +519,36 @@ mount -t cgroup none /dev/cgroup -o devices <qemu:env name='QEMU_ENV' value='VAL'/> </qemu:commandline> </domain> +</pre> + + <h2><a id="xmlnsfeatures">QEMU feature configuration for testing</a></h2> + + <p> + In some cases e.g. when developing a new feature or for testing it may + be required to control a given qemu feature (or qemu capability) to test + it before it's complete or disable it for debugging purposes. + <span class="since">Since 5.5.0</span> it's possible to use the same + special qemu namespace as above + (<code>http://libvirt.org/schemas/domain/qemu/1.0</code>) and use + <code><qemu:capabilities></code> element to add + (<code><qemu:add capability="capname"/></code>) or remove + (<code><qemu:del capability="capname"/></code>) capability bits. + The naming of the feature bits is the same libvirt uses in the status + XML. Note that this feature is meant for experiments only and should + _not_ be used in production. + </p> + + <p>Example:</p><pre> +<domain type='qemu' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>; + <name>testvm</name> + + [...] + + <qemu:capabilities> + <qemu:add capability='blockdev'/> + <qemu:del capability='drive'/> + </qemu:capabilities> +</domain> </pre> <h2><a id="xmlconfig">Example domain XML config</a></h2> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3661f0a556..ad4c32d480 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -72,6 +72,9 @@ <optional> <ref name='qemucmdline'/> </optional> + <optional> + <ref name='qemucapabilities'/> + </optional> <optional> <ref name='lxcsharens'/> </optional> @@ -6197,6 +6200,22 @@ </element> </define> + <define name="qemucapabilities"> + <element name="capabilities" ns="http://libvirt.org/schemas/domain/qemu/1.0"> + <zeroOrMore> + <element name="add"> + <attribute name="capability"/> + </element> + </zeroOrMore> + <zeroOrMore> + <element name="del"> + <attribute name="capability"/> + </element> + </zeroOrMore> + </element> + </define> + + <!-- Optional hypervisor extensions in their own namespace: LXC diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 864071220b..5caa31d3bc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3100,6 +3100,8 @@ qemuDomainXmlNsDefFree(qemuDomainXmlNsDefPtr def) virStringListFreeCount(def->args, def->num_args); virStringListFreeCount(def->env_name, def->num_env); virStringListFreeCount(def->env_value, def->num_env); + virStringListFreeCount(def->capsadd, def->ncapsadd); + virStringListFreeCount(def->capsdel, def->ncapsdel); VIR_FREE(def); } @@ -3199,6 +3201,50 @@ qemuDomainDefNamespaceParseCommandlineEnv(qemuDomainXmlNsDefPtr nsdef, } +static int +qemuDomainDefNamespaceParseCaps(qemuDomainXmlNsDefPtr nsdef, + xmlXPathContextPtr ctxt) +{ + VIR_AUTOFREE(xmlNodePtr *) nodesadd = NULL; + ssize_t nnodesadd; + VIR_AUTOFREE(xmlNodePtr *) nodesdel = NULL; + ssize_t nnodesdel; + size_t i; + + if ((nnodesadd = virXPathNodeSet("./qemu:capabilities/qemu:add", ctxt, &nodesadd)) < 0 || + (nnodesdel = virXPathNodeSet("./qemu:capabilities/qemu:del", ctxt, &nodesdel)) < 0) + return -1; + + if (nnodesadd > 0) { + if (VIR_ALLOC_N(nsdef->capsadd, nnodesadd) < 0) + return -1; + + for (i = 0; i < nnodesadd; i++) { + if (!(nsdef->capsadd[nsdef->ncapsadd++] = virXMLPropString(nodesadd[i], "capability"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing capability name")); + return -1; + } + } + } + + if (nnodesdel > 0) { + if (VIR_ALLOC_N(nsdef->capsdel, nnodesdel) < 0) + return -1; + + for (i = 0; i < nnodesdel; i++) { + if (!(nsdef->capsdel[nsdef->ncapsdel++] = virXMLPropString(nodesdel[i], "capability"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing capability name")); + return -1; + } + } + } + + return 0; +} + + static int qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, xmlNodePtr root ATTRIBUTE_UNUSED, @@ -3219,10 +3265,12 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, return -1; if (qemuDomainDefNamespaceParseCommandlineArgs(nsdata, ctxt) < 0 || - qemuDomainDefNamespaceParseCommandlineEnv(nsdata, ctxt) < 0) + qemuDomainDefNamespaceParseCommandlineEnv(nsdata, ctxt) < 0 || + qemuDomainDefNamespaceParseCaps(nsdata, ctxt) < 0) goto cleanup; - if (nsdata->num_args > 0 || nsdata->num_env > 0) + if (nsdata->num_args > 0 || nsdata->num_env > 0 || + nsdata->ncapsadd > 0 || nsdata->ncapsdel > 0) VIR_STEAL_PTR(*data, nsdata); ret = 0; @@ -3260,6 +3308,29 @@ qemuDomainDefNamespaceFormatXMLCommandline(virBufferPtr buf, } +static void +qemuDomainDefNamespaceFormatXMLCaps(virBufferPtr buf, + qemuDomainXmlNsDefPtr xmlns) +{ + size_t i; + + if (!xmlns->ncapsadd && !xmlns->ncapsdel) + return; + + virBufferAddLit(buf, "<qemu:capabilities>\n"); + virBufferAdjustIndent(buf, 2); + + for (i = 0; i < xmlns->ncapsadd; i++) + virBufferEscapeString(buf, "<qemu:add capability='%s'/>\n", xmlns->capsadd[i]); + + for (i = 0; i < xmlns->ncapsdel; i++) + virBufferEscapeString(buf, "<qemu:del capability='%s'/>\n", xmlns->capsdel[i]); + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</qemu:capabilities>\n"); +} + + static int qemuDomainDefNamespaceFormatXML(virBufferPtr buf, void *nsdata) @@ -3267,6 +3338,7 @@ qemuDomainDefNamespaceFormatXML(virBufferPtr buf, qemuDomainXmlNsDefPtr cmd = nsdata; qemuDomainDefNamespaceFormatXMLCommandline(buf, cmd); + qemuDomainDefNamespaceFormatXMLCaps(buf, cmd); return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 474c7b613a..fd52696807 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -541,6 +541,12 @@ struct _qemuDomainXmlNsDef { unsigned int num_env; char **env_name; char **env_value; + + size_t ncapsadd; + char **capsadd; + + size_t ncapsdel; + char **capsdel; }; diff --git a/tests/qemuxml2argvdata/qemu-ns.xml b/tests/qemuxml2argvdata/qemu-ns.xml index 62860a683c..c1095b3e81 100644 --- a/tests/qemuxml2argvdata/qemu-ns.xml +++ b/tests/qemuxml2argvdata/qemu-ns.xml @@ -27,4 +27,9 @@ <qemu:env name='NS' value='ns'/> <qemu:env name='BAR'/> </qemu:commandline> + <qemu:capabilities> + <qemu:add capability="vnc-colon"/> + <qemu:add capability="drive"/> + <qemu:del capability="name"/> + </qemu:capabilities> </domain> -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:18PM +0200, Peter Krempa wrote:
Similarly how we allow adding arbitrary command line arguments and environment variables this patch introduces the ability to control libvirt's perception of the qemu process by tweaking the capability bits for testing purposes.
The idea is to allow developers and users either test a new feature by enabling it early or disabling it to see whether it introduced regressions.
This feature is not meant for production use though, so users should handle it with care.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/drvqemu.html.in | 30 ++++++++++++ docs/schemas/domaincommon.rng | 19 ++++++++ src/qemu/qemu_domain.c | 76 +++++++++++++++++++++++++++++- src/qemu/qemu_domain.h | 6 +++ tests/qemuxml2argvdata/qemu-ns.xml | 5 ++ 5 files changed, 134 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
For testing purposes it's sometimes desired to be able to control the presence of capabilities of qemu. This adds the possibility to do this via the qemu namespace. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 8 ++++++++ src/qemu/qemu_process.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5caa31d3bc..2e0a0ca88b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8285,6 +8285,7 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, size_t i; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = obj->privateData; + bool custom_hypervisor_feat = false; if (virQEMUDriverIsPrivileged(driver) && (!cfg->clearEmulatorCapabilities || @@ -8299,6 +8300,13 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, qemuDomainXmlNsDefPtr qemuxmlns = obj->def->namespaceData; if (qemuxmlns->num_args || qemuxmlns->num_env) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CUSTOM_ARGV, logCtxt); + if (qemuxmlns->ncapsadd > 0 || qemuxmlns->ncapsdel > 0) + custom_hypervisor_feat = true; + } + + if (custom_hypervisor_feat) { + qemuDomainObjTaint(driver, obj, + VIR_DOMAIN_TAINT_CUSTOM_HYPERVISOR_FEATURE, logCtxt); } if (obj->def->cpu && obj->def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1626796c19..8fafd067ce 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5406,6 +5406,42 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, } +static int +qemuProcessStartUpdateCustomCaps(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainXmlNsDefPtr nsdef = vm->def->namespaceData; + int tmp; + size_t i; + + if (nsdef) { + for (i = 0; i < nsdef->ncapsadd; i++) { + if ((tmp = virQEMUCapsTypeFromString(nsdef->capsadd[i])) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid qemu namespace capability '%s'"), + nsdef->capsadd[i]); + return -1; + } + + virQEMUCapsSet(priv->qemuCaps, tmp); + } + + for (i = 0; i < nsdef->ncapsdel; i++) { + if ((tmp = virQEMUCapsTypeFromString(nsdef->capsdel[i])) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid qemu namespace capability '%s'"), + nsdef->capsdel[i]); + return -1; + } + + virQEMUCapsClear(priv->qemuCaps, tmp); + } + } + + return 0; +} + + /** * qemuProcessInit: * @@ -5476,6 +5512,10 @@ qemuProcessInit(virQEMUDriverPtr driver, if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm) < 0) goto cleanup; + /* Update qemu capabilities according to lists passed in via namespace */ + if (qemuProcessStartUpdateCustomCaps(vm) < 0) + goto cleanup; + if (flags & VIR_QEMU_PROCESS_START_PRETEND) { if (qemuDomainSetPrivatePaths(driver, vm) < 0) { virDomainObjRemoveTransientDef(vm); -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:19PM +0200, Peter Krempa wrote:
For testing purposes it's sometimes desired to be able to control the presence of capabilities of qemu. This adds the possibility to do this via the qemu namespace.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 8 ++++++++ src/qemu/qemu_process.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use the DO_TEST_CAPS_LATEST/VER infrastructure to run a more modern version of this and also fork it to a pre-blockdev version so that we can check the qemu namespace capability tweaking. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../qemu-ns.x86_64-4.0.0.args | 38 +++++++++++++++++++ ...emu-ns.args => qemu-ns.x86_64-latest.args} | 19 ++++++---- tests/qemuxml2argvtest.c | 3 +- 3 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args rename tests/qemuxml2argvdata/{qemu-ns.args => qemu-ns.x86_64-latest.args} (61%) diff --git a/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args b/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args new file mode 100644 index 0000000000..3aadfc8955 --- /dev/null +++ b/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args @@ -0,0 +1,38 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +NS=ns \ +BAR='' \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc-i440fx-4.0,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ +-unknown parameter \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/qemu-ns.args b/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args similarity index 61% rename from tests/qemuxml2argvdata/qemu-ns.args rename to tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args index ad858b3da1..526c5f1770 100644 --- a/tests/qemuxml2argvdata/qemu-ns.args +++ b/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args @@ -10,24 +10,29 @@ QEMU_AUDIO_DRV=none \ NS=ns \ BAR='' \ /usr/bin/qemu-system-i686 \ --name QEMUGuest1 \ +-name guest=QEMUGuest1,debug-threads=on \ -S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -machine pc,accel=tcg,usb=off,dump-guest-core=off \ -m 214 \ --realtime mlock=off \ +-overcommit mem-lock=off \ -smp 1,sockets=1,cores=1,threads=1 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ -display none \ -no-user-config \ -nodefaults \ --chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ -server,nowait \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ -mon chardev=charmonitor,id=monitor,mode=control \ -rtc base=utc \ -no-shutdown \ -no-acpi \ --usb \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ --unknown parameter +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ +-unknown parameter \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 49220733ae..a4e14a4a5e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1622,7 +1622,8 @@ mymain(void) QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM); - DO_TEST("qemu-ns", NONE); + DO_TEST_CAPS_VER("qemu-ns", "4.0.0"); + DO_TEST_CAPS_LATEST("qemu-ns"); DO_TEST("qemu-ns-no-env", NONE); DO_TEST("qemu-ns-alt", NONE); -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:20PM +0200, Peter Krempa wrote:
Use the DO_TEST_CAPS_LATEST/VER infrastructure to run a more modern version of this and also fork it to a pre-blockdev version so that we can check the qemu namespace capability tweaking.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../qemu-ns.x86_64-4.0.0.args | 38 +++++++++++++++++++ ...emu-ns.args => qemu-ns.x86_64-latest.args} | 19 ++++++---- tests/qemuxml2argvtest.c | 3 +- 3 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args rename tests/qemuxml2argvdata/{qemu-ns.args => qemu-ns.x86_64-latest.args} (61%)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Show that the capability tweaking stuff works by enabling blockdev in the 'qemu-ns' test even in versions where it's not yet fully supported. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args | 7 +++++-- tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args | 7 +++++-- tests/qemuxml2argvdata/qemu-ns.xml | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args b/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args index 3aadfc8955..6e059d336a 100644 --- a/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args +++ b/tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args @@ -29,8 +29,11 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ --device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1",\ +"node-name":"libvirt-1-storage","read-only":false,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw",\ +"file":"libvirt-1-storage"}' \ +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ -unknown parameter \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ diff --git a/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args b/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args index 526c5f1770..ebb7b0da37 100644 --- a/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args +++ b/tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args @@ -29,8 +29,11 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ --device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1",\ +"node-name":"libvirt-1-storage","read-only":false,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw",\ +"file":"libvirt-1-storage"}' \ +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ -unknown parameter \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ diff --git a/tests/qemuxml2argvdata/qemu-ns.xml b/tests/qemuxml2argvdata/qemu-ns.xml index c1095b3e81..02cbf4bbd9 100644 --- a/tests/qemuxml2argvdata/qemu-ns.xml +++ b/tests/qemuxml2argvdata/qemu-ns.xml @@ -29,7 +29,7 @@ </qemu:commandline> <qemu:capabilities> <qemu:add capability="vnc-colon"/> - <qemu:add capability="drive"/> + <qemu:add capability="blockdev"/> <qemu:del capability="name"/> </qemu:capabilities> </domain> -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:21PM +0200, Peter Krempa wrote:
Show that the capability tweaking stuff works by enabling blockdev in the 'qemu-ns' test even in versions where it's not yet fully supported.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvdata/qemu-ns.x86_64-4.0.0.args | 7 +++++-- tests/qemuxml2argvdata/qemu-ns.x86_64-latest.args | 7 +++++-- tests/qemuxml2argvdata/qemu-ns.xml | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_conf.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index d981001600..745e0f21bd 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -990,7 +990,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, { virConfPtr conf = NULL; int ret = -1; - char *corestr = NULL; /* Just check the file is readable before opening it, otherwise * libvirt emits an error. @@ -1057,7 +1056,6 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, ret = 0; cleanup: - VIR_FREE(corestr); virConfFree(conf); return ret; } -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:22PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_conf.c | 2 -- 1 file changed, 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
In cases when e.g. a new feature breaks upstream behaviour it's useful to allow users to disable the new feature to verify the regression and possibly use it as a workaround until a fix is available. The new qemu.conf option named "capability_filters" allows to remove qemu capabilities from the detected bitmap. This patch introduces the configuration infrastructure to parse the option and pass it around. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf | 7 +++++++ src/qemu/qemu_conf.c | 17 +++++++++++++++++ src/qemu/qemu_conf.h | 2 ++ src/qemu/test_libvirtd_qemu.aug.in | 3 +++ 5 files changed, 32 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index b311f02da6..6821cc4a29 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -126,6 +126,8 @@ module Libvirtd_qemu = let swtpm_entry = str_entry "swtpm_user" | str_entry "swtpm_group" + let capability_filters_entry = str_array_entry "capability_filters" + (* Each entry in the config is one of the following ... *) let entry = default_tls_entry | vnc_entry @@ -147,6 +149,7 @@ module Libvirtd_qemu = | vxhs_entry | nbd_entry | swtpm_entry + | capability_filters_entry let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 5a85789d81..1969b3f0a1 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -825,3 +825,10 @@ # #swtpm_user = "tss" #swtpm_group = "tss" + +# For debugging and testing purposes it's sometimes useful to be able to disable +# libvirt behaviour based on the capabilities of the qemu process. This option +# allows to do so. DO _NOT_ use in production and beaware that the behaviour +# may change across versions. +# +#capability_filters = [ "capname" ] diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 745e0f21bd..8312f99f80 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -381,6 +381,8 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->memoryBackingDir); VIR_FREE(cfg->swtpmStorageDir); + + virStringListFree(cfg->capabilityfilters); } @@ -984,6 +986,18 @@ virQEMUDriverConfigLoadSWTPMEntry(virQEMUDriverConfigPtr cfg, } +static int +virQEMUDriverConfigLoadCapsFiltersEntry(virQEMUDriverConfigPtr cfg, + virConfPtr conf) +{ + if (virConfGetValueStringList(conf, "capability_filters", false, + &cfg->capabilityfilters) < 0) + return -1; + + return 0; +} + + int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, const char *filename, bool privileged) @@ -1053,6 +1067,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, if (virQEMUDriverConfigLoadSWTPMEntry(cfg, conf) < 0) goto cleanup; + if (virQEMUDriverConfigLoadCapsFiltersEntry(cfg, conf) < 0) + goto cleanup; + ret = 0; cleanup: diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 386e774653..8bef009bc9 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -218,6 +218,8 @@ struct _virQEMUDriverConfig { uid_t swtpm_user; gid_t swtpm_group; + + char **capabilityfilters; }; /* Main driver state */ diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index fea1d308b7..50b728ad22 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -104,3 +104,6 @@ module Test_libvirtd_qemu = { "pr_helper" = "/usr/bin/qemu-pr-helper" } { "swtpm_user" = "tss" } { "swtpm_group" = "tss" } +{ "capability_filters" + { "1" = "capname" } +} -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:23PM +0200, Peter Krempa wrote:
In cases when e.g. a new feature breaks upstream behaviour it's useful to allow users to disable the new feature to verify the regression and possibly use it as a workaround until a fix is available.
The new qemu.conf option named "capability_filters" allows to remove qemu capabilities from the detected bitmap.
This patch introduces the configuration infrastructure to parse the option and pass it around.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/qemu.conf | 7 +++++++ src/qemu/qemu_conf.c | 17 +++++++++++++++++ src/qemu/qemu_conf.h | 2 ++ src/qemu/test_libvirtd_qemu.aug.in | 3 +++ 5 files changed, 32 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Filter out the given capabilities and set domain taint if we done so. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 3 ++- src/qemu/qemu_process.c | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2e0a0ca88b..9685d13ade 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8304,7 +8304,8 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver, custom_hypervisor_feat = true; } - if (custom_hypervisor_feat) { + if (custom_hypervisor_feat || + (cfg->capabilityfilters && *cfg->capabilityfilters)) { qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CUSTOM_HYPERVISOR_FEATURE, logCtxt); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8fafd067ce..a5cc4daa1b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5410,10 +5410,25 @@ static int qemuProcessStartUpdateCustomCaps(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(priv->driver); qemuDomainXmlNsDefPtr nsdef = vm->def->namespaceData; + char **next; int tmp; size_t i; + if (cfg->capabilityfilters) { + for (next = cfg->capabilityfilters; *next; next++) { + if ((tmp = virQEMUCapsTypeFromString(*next)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid capability_filters capability '%s'"), + *next); + return -1; + } + + virQEMUCapsClear(priv->qemuCaps, tmp); + } + } + if (nsdef) { for (i = 0; i < nsdef->ncapsadd; i++) { if ((tmp = virQEMUCapsTypeFromString(nsdef->capsadd[i])) < 0) { -- 2.21.0
On Tue, Jun 18, 2019 at 03:10:24PM +0200, Peter Krempa wrote:
Filter out the given capabilities and set domain taint if we done so.
we've
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 3 ++- src/qemu/qemu_process.c | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa