[PATCH v3 00/14] move checks from parse to validate callbacks
This is the v3 of https://www.redhat.com/archives/libvir-list/2020-December/msg00419.html Michal mentioned in his v2 review that we should move these validations out of domain_conf.c to a new file. Let's create a new file called domain_validate.c and start move the validations done in v2 into it, instead of pushing the already reviewed v2 series just to have more stuff to be moved later on. A follow up series will push more validations from domain_conf.c to domain_validate.c. Michal's R-bs were kept in all patches but patch 01. Daniel Henrique Barboza (14): domain_conf: move boot timeouts check to domain_validate.c domain_conf.c: move primary video check to validate callback domain_conf.c: move virDomainVideoDefValidate() to domain_validate.c domain_conf.c: move QXL attributes check to virDomainVideoDefValidate() domain_conf: move virDomainDiskDefValidate() to domain_validate.c domain_conf: move vendor, product and tray checks to domain_validate.c domain_validate.c: rename virSecurityDeviceLabelDefValidateXML() domain_conf: move all ChrSource checks to domain_validate.c domain_conf.c: move smartcard address check to domain_validate.c domain_conf.c: move blkio path check to domain_validate.c domain_conf.c: move virDomainControllerDefValidate() to domain_validate.c domain_conf: move virDomainPCIControllerOpts checks to domain_validate.c domain_conf: move pci-root/pcie-root address check to domain_validate.c domain_conf.c: move idmapEntry checks to domain_validate.c po/POTFILES.in | 1 + src/conf/domain_conf.c | 562 +-------------- src/conf/domain_validate.c | 637 ++++++++++++++++++ src/conf/domain_validate.h | 47 ++ src/conf/meson.build | 1 + tests/qemuxml2argvdata/pci-root-address.err | 2 +- .../pseries-default-phb-numa-node.err | 2 +- .../video-multiple-primaries.err | 1 + .../video-multiple-primaries.xml | 32 + tests/qemuxml2argvtest.c | 14 +- 10 files changed, 756 insertions(+), 543 deletions(-) create mode 100644 src/conf/domain_validate.c create mode 100644 src/conf/domain_validate.h create mode 100644 tests/qemuxml2argvdata/video-multiple-primaries.err create mode 100644 tests/qemuxml2argvdata/video-multiple-primaries.xml -- 2.26.2
This patch creates a new function, virDomainDefBootValidate(), to host the validation of boot menu timeout and rebootTimeout outside of parse time. The checks in virDomainDefParseBootXML() were changed to throw VIR_ERR_XML_ERROR in case of parse error of those values. In an attempt to alleviate the amount of code being stacked inside domain_conf.c, let's put this new function in a new domain_validate.c file that will be used to place these validations. Suggested-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- po/POTFILES.in | 1 + src/conf/domain_conf.c | 20 +++++++-------- src/conf/domain_validate.c | 51 ++++++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 27 ++++++++++++++++++++ src/conf/meson.build | 1 + tests/qemuxml2argvtest.c | 3 ++- 6 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 src/conf/domain_validate.c create mode 100644 src/conf/domain_validate.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 9782b2beb4..14636d4b93 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -26,6 +26,7 @@ @SRCDIR@src/conf/domain_capabilities.c @SRCDIR@src/conf/domain_conf.c @SRCDIR@src/conf/domain_event.c +@SRCDIR@src/conf/domain_validate.c @SRCDIR@src/conf/interface_conf.c @SRCDIR@src/conf/netdev_bandwidth_conf.c @SRCDIR@src/conf/netdev_vlan_conf.c diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66ee658e7b..4a45c76cf1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -33,6 +33,7 @@ #include "datatypes.h" #include "domain_addr.h" #include "domain_conf.h" +#include "domain_validate.h" #include "snapshot_conf.h" #include "viralloc.h" #include "virxml.h" @@ -7344,6 +7345,9 @@ virDomainDefValidateInternal(const virDomainDef *def, if (virDomainDefCputuneValidate(def) < 0) return -1; + if (virDomainDefBootValidate(def) < 0) + return -1; + if (virDomainNumaDefValidate(def->numa) < 0) return -1; @@ -18867,11 +18871,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, tmp = virXMLPropString(node, "timeout"); if (tmp && def->os.bootmenu == VIR_TRISTATE_BOOL_YES) { - if (virStrToLong_uip(tmp, NULL, 0, &def->os.bm_timeout) < 0 || - def->os.bm_timeout > 65535) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("invalid value for boot menu timeout, " - "must be in range [0,65535]")); + if (virStrToLong_uip(tmp, NULL, 0, &def->os.bm_timeout) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid value for boot menu timeout")); return -1; } def->os.bm_timeout_set = true; @@ -18892,11 +18894,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, if (tmp) { /* that was really just for the check if it is there */ - if (virStrToLong_i(tmp, NULL, 0, &def->os.bios.rt_delay) < 0 || - def->os.bios.rt_delay < -1 || def->os.bios.rt_delay > 65535) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("invalid value for rebootTimeout, " - "must be in range [-1,65535]")); + if (virStrToLong_i(tmp, NULL, 0, &def->os.bios.rt_delay) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid value for rebootTimeout")); return -1; } def->os.bios.rt_set = true; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c new file mode 100644 index 0000000000..e4d947c553 --- /dev/null +++ b/src/conf/domain_validate.c @@ -0,0 +1,51 @@ +/* + * domain_validate.c: domain general validation functions + * + * Copyright IBM Corp, 2020 + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "domain_validate.h" +#include "domain_conf.h" +#include "virlog.h" +#include "virutil.h" + +#define VIR_FROM_THIS VIR_FROM_DOMAIN + +VIR_LOG_INIT("conf.domain_validate"); + +int +virDomainDefBootValidate(const virDomainDef *def) +{ + if (def->os.bm_timeout_set && def->os.bm_timeout > 65535) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("invalid value for boot menu timeout, " + "must be in range [0,65535]")); + return -1; + } + + if (def->os.bios.rt_set && + (def->os.bios.rt_delay < -1 || def->os.bios.rt_delay > 65535)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("invalid value for rebootTimeout, " + "must be in range [-1,65535]")); + return -1; + } + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h new file mode 100644 index 0000000000..2b558668a9 --- /dev/null +++ b/src/conf/domain_validate.h @@ -0,0 +1,27 @@ +/* + * domain_validate.h: domain general validation functions + * + * Copyright IBM Corp, 2020 + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +#include <glib-object.h> + +#include "domain_conf.h" + +int virDomainDefBootValidate(const virDomainDef *def); diff --git a/src/conf/meson.build b/src/conf/meson.build index 03b90aa6f6..8ddcc9968d 100644 --- a/src/conf/meson.build +++ b/src/conf/meson.build @@ -14,6 +14,7 @@ domain_conf_sources = [ 'domain_capabilities.c', 'domain_conf.c', 'domain_nwfilter.c', + 'domain_validate.c', 'moment_conf.c', 'numa_conf.c', 'snapshot_conf.c', diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 7c9dc0e641..202fc83ccf 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1006,7 +1006,8 @@ mymain(void) DO_TEST("boot-menu-enable-with-timeout", QEMU_CAPS_SPLASH_TIMEOUT); DO_TEST_PARSE_ERROR("boot-menu-enable-with-timeout", NONE); - DO_TEST_PARSE_ERROR("boot-menu-enable-with-timeout-invalid", NONE); + DO_TEST_PARSE_ERROR("boot-menu-enable-with-timeout-invalid", + QEMU_CAPS_SPLASH_TIMEOUT); DO_TEST("boot-menu-disable", NONE); DO_TEST("boot-menu-disable-drive", NONE); DO_TEST_PARSE_ERROR("boot-dev+order", -- 2.26.2
On 12/8/20 11:20 PM, Daniel Henrique Barboza wrote:
This patch creates a new function, virDomainDefBootValidate(), to host the validation of boot menu timeout and rebootTimeout outside of parse time. The checks in virDomainDefParseBootXML() were changed to throw VIR_ERR_XML_ERROR in case of parse error of those values.
In an attempt to alleviate the amount of code being stacked inside domain_conf.c, let's put this new function in a new domain_validate.c file that will be used to place these validations.
Suggested-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- po/POTFILES.in | 1 + src/conf/domain_conf.c | 20 +++++++-------- src/conf/domain_validate.c | 51 ++++++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 27 ++++++++++++++++++++ src/conf/meson.build | 1 + tests/qemuxml2argvtest.c | 3 ++- 6 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 src/conf/domain_validate.c create mode 100644 src/conf/domain_validate.h
diff --git a/po/POTFILES.in b/po/POTFILES.in index 9782b2beb4..14636d4b93 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -26,6 +26,7 @@ @SRCDIR@src/conf/domain_capabilities.c @SRCDIR@src/conf/domain_conf.c @SRCDIR@src/conf/domain_event.c +@SRCDIR@src/conf/domain_validate.c @SRCDIR@src/conf/interface_conf.c @SRCDIR@src/conf/netdev_bandwidth_conf.c @SRCDIR@src/conf/netdev_vlan_conf.c diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66ee658e7b..4a45c76cf1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -33,6 +33,7 @@ #include "datatypes.h" #include "domain_addr.h" #include "domain_conf.h" +#include "domain_validate.h" #include "snapshot_conf.h" #include "viralloc.h" #include "virxml.h" @@ -7344,6 +7345,9 @@ virDomainDefValidateInternal(const virDomainDef *def, if (virDomainDefCputuneValidate(def) < 0) return -1;
+ if (virDomainDefBootValidate(def) < 0) + return -1; + if (virDomainNumaDefValidate(def->numa) < 0) return -1;
@@ -18867,11 +18871,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
tmp = virXMLPropString(node, "timeout"); if (tmp && def->os.bootmenu == VIR_TRISTATE_BOOL_YES) { - if (virStrToLong_uip(tmp, NULL, 0, &def->os.bm_timeout) < 0 || - def->os.bm_timeout > 65535) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("invalid value for boot menu timeout, " - "must be in range [0,65535]")); + if (virStrToLong_uip(tmp, NULL, 0, &def->os.bm_timeout) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid value for boot menu timeout")); return -1; } def->os.bm_timeout_set = true; @@ -18892,11 +18894,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, if (tmp) { /* that was really just for the check if it is there */
- if (virStrToLong_i(tmp, NULL, 0, &def->os.bios.rt_delay) < 0 || - def->os.bios.rt_delay < -1 || def->os.bios.rt_delay > 65535) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("invalid value for rebootTimeout, " - "must be in range [-1,65535]")); + if (virStrToLong_i(tmp, NULL, 0, &def->os.bios.rt_delay) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid value for rebootTimeout")); return -1; } def->os.bios.rt_set = true; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c new file mode 100644 index 0000000000..e4d947c553 --- /dev/null +++ b/src/conf/domain_validate.c @@ -0,0 +1,51 @@ +/* + * domain_validate.c: domain general validation functions + * + * Copyright IBM Corp, 2020
Honestly, I have only vague idea how these Copyright lines work, but shouldn't they also include (at least subset) of the lines from the original file? I mean, my common sense tells me that if I have a file written by person X, and later the file is split into two the person X is still the original author. Extending that, if a company holds a copyright on a file then moving bits out to a different file should keep the copyright. But I admit that law has completely different model of "common sense". And also there is a disconnection between files and these Copyright lines. If a copyright holder Y changed a tiny bit that is not moved - should their Copyright line also appear in the new file? Michal
On 12/9/20 5:13 AM, Michal Privoznik wrote:
On 12/8/20 11:20 PM, Daniel Henrique Barboza wrote:
This patch creates a new function, virDomainDefBootValidate(), to host the validation of boot menu timeout and rebootTimeout outside of parse time. The checks in virDomainDefParseBootXML() were changed to throw VIR_ERR_XML_ERROR in case of parse error of those values.
In an attempt to alleviate the amount of code being stacked inside domain_conf.c, let's put this new function in a new domain_validate.c file that will be used to place these validations.
Suggested-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
[...]
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c new file mode 100644 index 0000000000..e4d947c553 --- /dev/null +++ b/src/conf/domain_validate.c @@ -0,0 +1,51 @@ +/* + * domain_validate.c: domain general validation functions + * + * Copyright IBM Corp, 2020
Honestly, I have only vague idea how these Copyright lines work, but shouldn't they also include (at least subset) of the lines from the original file? I mean, my common sense tells me that if I have a file written by person X, and later the file is split into two the person X is still the original author. Extending that, if a company holds a copyright on a file then moving bits out to a different file should keep the copyright. But I admit that law has completely different model of "common sense". And also there is a disconnection between files and these Copyright lines. If a copyright holder Y changed a tiny bit that is not moved - should their Copyright line also appear in the new file?
TBH I have no idea what's the best practice here. What I did was simply copy the Copyright header from qemu_validate.c. I believe I can add a "This file was based on src/qemu/qemu_validate.c", since the inspiration is quite obvious in this case, right after the legal text. I see some people doing this in QEMU. I see people putting copyright nominal in their own name as well. I guess this means that the original author wasn't bind to a company contract by the time the file was created or something like that. I am not sure about the implications of having a copyright in your own name, aside from people emailing you to ask for a license change (Linus and the GPLv3 versus LGPLv2 comes to mind). Anyway, I'm happy to hear suggestions about how to handle this copyright text :) Thanks, DHB
Michal
On Wed, Dec 09, 2020 at 07:35:09AM -0300, Daniel Henrique Barboza wrote:
On 12/9/20 5:13 AM, Michal Privoznik wrote:
On 12/8/20 11:20 PM, Daniel Henrique Barboza wrote:
This patch creates a new function, virDomainDefBootValidate(), to host the validation of boot menu timeout and rebootTimeout outside of parse time. The checks in virDomainDefParseBootXML() were changed to throw VIR_ERR_XML_ERROR in case of parse error of those values.
In an attempt to alleviate the amount of code being stacked inside domain_conf.c, let's put this new function in a new domain_validate.c file that will be used to place these validations.
Suggested-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
[...]
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c new file mode 100644 index 0000000000..e4d947c553 --- /dev/null +++ b/src/conf/domain_validate.c @@ -0,0 +1,51 @@ +/* + * domain_validate.c: domain general validation functions + * + * Copyright IBM Corp, 2020
Honestly, I have only vague idea how these Copyright lines work, but shouldn't they also include (at least subset) of the lines from the original file? I mean, my common sense tells me that if I have a file written by person X, and later the file is split into two the person X is still the original author. Extending that, if a company holds a copyright on a file then moving bits out to a different file should keep the copyright. But I admit that law has completely different model of "common sense". And also there is a disconnection between files and these Copyright lines. If a copyright holder Y changed a tiny bit that is not moved - should their Copyright line also appear in the new file?
TBH I have no idea what's the best practice here. What I did was simply copy the Copyright header from qemu_validate.c. I believe I can add a "This file was based on src/qemu/qemu_validate.c", since the inspiration is quite obvious in this case, right after the legal text. I see some people doing this in QEMU.
If you're copying code from one file to a new file, then the simplest way is to copy the existing copyright headers unchanged. Not copying a copyright header would require you to audit the history of the code to determine whether it is valid to exclude them. This is usually a waste of time, so preserving the full existing copyright headers is normal practice for sake of speed. You can then optionally also add new copyright lines if you're also making code changes after the copy.
I see people putting copyright nominal in their own name as well. I guess this means that the original author wasn't bind to a company contract by the time the file was created or something like that. I am not sure about the implications of having a copyright in your own name, aside from people emailing you to ask for a license change (Linus and the GPLv3 versus LGPLv2 comes to mind).
A combination of your country's laws, and your employer's rules generally determine whether copyright statements have to be in your name or your employer's name. Most commonly it is your employer's name, as most companies require their employees to assign copyright on all their work as part of terms of employment. There are exceptions though, so I can't explicitly say what is right for you personally. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 12/9/20 7:52 AM, Daniel P. Berrangé wrote:
On Wed, Dec 09, 2020 at 07:35:09AM -0300, Daniel Henrique Barboza wrote:
On 12/9/20 5:13 AM, Michal Privoznik wrote:
On 12/8/20 11:20 PM, Daniel Henrique Barboza wrote:
This patch creates a new function, virDomainDefBootValidate(), to host the validation of boot menu timeout and rebootTimeout outside of parse time. The checks in virDomainDefParseBootXML() were changed to throw VIR_ERR_XML_ERROR in case of parse error of those values.
In an attempt to alleviate the amount of code being stacked inside domain_conf.c, let's put this new function in a new domain_validate.c file that will be used to place these validations.
Suggested-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
[...]
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c new file mode 100644 index 0000000000..e4d947c553 --- /dev/null +++ b/src/conf/domain_validate.c @@ -0,0 +1,51 @@ +/* + * domain_validate.c: domain general validation functions + * + * Copyright IBM Corp, 2020
Honestly, I have only vague idea how these Copyright lines work, but shouldn't they also include (at least subset) of the lines from the original file? I mean, my common sense tells me that if I have a file written by person X, and later the file is split into two the person X is still the original author. Extending that, if a company holds a copyright on a file then moving bits out to a different file should keep the copyright. But I admit that law has completely different model of "common sense". And also there is a disconnection between files and these Copyright lines. If a copyright holder Y changed a tiny bit that is not moved - should their Copyright line also appear in the new file?
TBH I have no idea what's the best practice here. What I did was simply copy the Copyright header from qemu_validate.c. I believe I can add a "This file was based on src/qemu/qemu_validate.c", since the inspiration is quite obvious in this case, right after the legal text. I see some people doing this in QEMU.
If you're copying code from one file to a new file, then the simplest way is to copy the existing copyright headers unchanged.
Not copying a copyright header would require you to audit the history of the code to determine whether it is valid to exclude them. This is usually a waste of time, so preserving the full existing copyright headers is normal practice for sake of speed.
You can then optionally also add new copyright lines if you're also making code changes after the copy.
I see people putting copyright nominal in their own name as well. I guess this means that the original author wasn't bind to a company contract by the time the file was created or something like that. I am not sure about the implications of having a copyright in your own name, aside from people emailing you to ask for a license change (Linus and the GPLv3 versus LGPLv2 comes to mind).
A combination of your country's laws, and your employer's rules generally determine whether copyright statements have to be in your name or your employer's name. Most commonly it is your employer's name, as most companies require their employees to assign copyright on all their work as part of terms of employment. There are exceptions though, so I can't explicitly say what is right for you personally.
Thanks for the info. Just double checked internally and my case is the most most common scenario (copyright is in the employer's name). So it's all good. DHB
Regards, Daniel
This check isn't exclusive to XML parsing. Let's move it to virDomainDefVideoValidate() in domain_validate.c We don't have a failure test for this scenario, so a new test called 'video-multiple-primaries' was added to test this failure case. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 10 +++--- src/conf/domain_validate.c | 24 ++++++++++++++ src/conf/domain_validate.h | 1 + .../video-multiple-primaries.err | 1 + .../video-multiple-primaries.xml | 32 +++++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++ 6 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/video-multiple-primaries.err create mode 100644 tests/qemuxml2argvdata/video-multiple-primaries.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4a45c76cf1..a8bd54a368 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7348,6 +7348,9 @@ virDomainDefValidateInternal(const virDomainDef *def, if (virDomainDefBootValidate(def) < 0) return -1; + if (virDomainDefVideoValidate(def) < 0) + return -1; + if (virDomainNumaDefValidate(def->numa) < 0) return -1; @@ -22141,14 +22144,9 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; if (video->primary) { - if (def->nvideos != 0 && def->videos[0]->primary) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only one primary video device is supported")); - goto error; - } - insertAt = 0; } + if (VIR_INSERT_ELEMENT_INPLACE(def->videos, insertAt, def->nvideos, diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index e4d947c553..eb2ef6c7fb 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -49,3 +49,27 @@ virDomainDefBootValidate(const virDomainDef *def) return 0; } + + +int +virDomainDefVideoValidate(const virDomainDef *def) +{ + size_t i; + + if (def->nvideos == 0) + return 0; + + /* Any video marked as primary will be put in index 0 by the + * parser. Ensure that we have only one primary set by the user. */ + if (def->videos[0]->primary) { + for (i = 1; i < def->nvideos; i++) { + if (def->videos[i]->primary) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only one primary video device is supported")); + return -1; + } + } + } + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index 2b558668a9..df4f35c1dd 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -25,3 +25,4 @@ #include "domain_conf.h" int virDomainDefBootValidate(const virDomainDef *def); +int virDomainDefVideoValidate(const virDomainDef *def); diff --git a/tests/qemuxml2argvdata/video-multiple-primaries.err b/tests/qemuxml2argvdata/video-multiple-primaries.err new file mode 100644 index 0000000000..f81b7275e4 --- /dev/null +++ b/tests/qemuxml2argvdata/video-multiple-primaries.err @@ -0,0 +1 @@ +unsupported configuration: Only one primary video device is supported diff --git a/tests/qemuxml2argvdata/video-multiple-primaries.xml b/tests/qemuxml2argvdata/video-multiple-primaries.xml new file mode 100644 index 0000000000..977227c5ff --- /dev/null +++ b/tests/qemuxml2argvdata/video-multiple-primaries.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i386</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/var/lib/libvirt/images/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <video> + <model type='vga' vram='16384' heads='1' primary='yes'/> + </video> + <video> + <model type='qxl' heads='1' primary='yes'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 202fc83ccf..0e7d8d5ba3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2379,6 +2379,11 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("default-video-type-riscv64", "riscv64"); DO_TEST_CAPS_ARCH_LATEST("default-video-type-s390x", "s390x"); + DO_TEST_PARSE_ERROR("video-multiple-primaries", + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_DEVICE_VGA, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + DO_TEST("virtio-rng-default", QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM); -- 2.26.2
We'll add more video validations into the function in the next patch. Let's move it beforehand to domain_validate.c. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 57 -------------------------------------- src/conf/domain_validate.c | 57 ++++++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 2 ++ 3 files changed, 59 insertions(+), 57 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a8bd54a368..d80bc8495e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6612,63 +6612,6 @@ virDomainHostdevDefValidate(const virDomainHostdevDef *hostdev) } -static int -virDomainVideoDefValidate(const virDomainVideoDef *video, - const virDomainDef *def) -{ - size_t i; - - if (video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing video model and cannot determine default")); - return -1; - } - - /* it doesn't make sense to pair video device type 'none' with any other - * types, there can be only a single video device in such case - */ - for (i = 0; i < def->nvideos; i++) { - if (def->videos[i]->type == VIR_DOMAIN_VIDEO_TYPE_NONE && - def->nvideos > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("a 'none' video type must be the only video device " - "defined for the domain")); - return -1; - } - } - - switch (video->backend) { - case VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER: - if (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("'vhostuser' driver is only supported with 'virtio' device")); - return -1; - } - break; - case VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT: - case VIR_DOMAIN_VIDEO_BACKEND_TYPE_QEMU: - if (video->accel && video->accel->rendernode) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("unsupported rendernode accel attribute without 'vhostuser'")); - return -1; - } - break; - case VIR_DOMAIN_VIDEO_BACKEND_TYPE_LAST: - default: - virReportEnumRangeError(virDomainInputType, video->backend); - return -1; - } - - if (video->res && (video->res->x == 0 || video->res->y == 0)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("video resolution values must be greater than 0")); - return -1; - } - - return 0; -} - - static int virDomainMemoryDefValidate(const virDomainMemoryDef *mem, const virDomainDef *def) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index eb2ef6c7fb..c81fd296b9 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -73,3 +73,60 @@ virDomainDefVideoValidate(const virDomainDef *def) return 0; } + + +int +virDomainVideoDefValidate(const virDomainVideoDef *video, + const virDomainDef *def) +{ + size_t i; + + if (video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing video model and cannot determine default")); + return -1; + } + + /* it doesn't make sense to pair video device type 'none' with any other + * types, there can be only a single video device in such case + */ + for (i = 0; i < def->nvideos; i++) { + if (def->videos[i]->type == VIR_DOMAIN_VIDEO_TYPE_NONE && + def->nvideos > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("a 'none' video type must be the only video device " + "defined for the domain")); + return -1; + } + } + + switch (video->backend) { + case VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER: + if (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'vhostuser' driver is only supported with 'virtio' device")); + return -1; + } + break; + case VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT: + case VIR_DOMAIN_VIDEO_BACKEND_TYPE_QEMU: + if (video->accel && video->accel->rendernode) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported rendernode accel attribute without 'vhostuser'")); + return -1; + } + break; + case VIR_DOMAIN_VIDEO_BACKEND_TYPE_LAST: + default: + virReportEnumRangeError(virDomainInputType, video->backend); + return -1; + } + + if (video->res && (video->res->x == 0 || video->res->y == 0)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("video resolution values must be greater than 0")); + return -1; + } + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index df4f35c1dd..ed170391f8 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -26,3 +26,5 @@ int virDomainDefBootValidate(const virDomainDef *def); int virDomainDefVideoValidate(const virDomainDef *def); +int virDomainVideoDefValidate(const virDomainVideoDef *video, + const virDomainDef *def); -- 2.26.2
These checks are not related to XML parsing and can be moved to the validate callback. Errors were changed from VIR_ERR_XML_ERROR to VIR_ERR_CONFIG_UNSUPPORTED. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 15 --------------- src/conf/domain_validate.c | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d80bc8495e..db0ca975fe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16132,11 +16132,6 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, } if (ram) { - if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("ram attribute only supported for type of qxl")); - return NULL; - } if (virStrToLong_uip(ram, NULL, 10, &def->ram) < 0) { virReportError(VIR_ERR_XML_ERROR, _("cannot parse video ram '%s'"), ram); @@ -16153,11 +16148,6 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, } if (vram64) { - if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("vram64 attribute only supported for type of qxl")); - return NULL; - } if (virStrToLong_uip(vram64, NULL, 10, &def->vram64) < 0) { virReportError(VIR_ERR_XML_ERROR, _("cannot parse video vram64 '%s'"), vram64); @@ -16166,11 +16156,6 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, } if (vgamem) { - if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("vgamem attribute only supported for type of qxl")); - return NULL; - } if (virStrToLong_uip(vgamem, NULL, 10, &def->vgamem) < 0) { virReportError(VIR_ERR_XML_ERROR, _("cannot parse video vgamem '%s'"), vgamem); diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index c81fd296b9..234eb72f11 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -128,5 +128,25 @@ virDomainVideoDefValidate(const virDomainVideoDef *video, return -1; } + if (video->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->ram != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ram attribute only supported for video type qxl")); + return -1; + } + + if (video->vram64 != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vram64 attribute only supported for video type qxl")); + return -1; + } + + if (video->vgamem != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vgamem attribute only supported for video type qxl")); + return -1; + } + } + return 0; } -- 2.26.2
Next patch will add more validations to the function. Let's move it beforehand to domain_validate.c. virSecurityDeviceLabelDefValidateXML() is still used inside domain_conf.c, so make it public for now until its current caller (virDomainChrSourceDefValidate()) is also moved to domain_validate.c. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 152 ------------------------------------ src/conf/domain_validate.c | 153 +++++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 6 ++ 3 files changed, 159 insertions(+), 152 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index db0ca975fe..5aeb75ce59 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6043,158 +6043,6 @@ virDomainDefPostParse(virDomainDefPtr def, } -/** - * virDomainDiskAddressDiskBusCompatibility: - * @bus: disk bus type - * @addressType: disk address type - * - * Check if the specified disk address type @addressType is compatible - * with the specified disk bus type @bus. This function checks - * compatibility with the bus types SATA, SCSI, FDC, and IDE only, - * because only these are handled in common code. - * - * Returns true if compatible or can't be decided in common code, - * false if known to be not compatible. - */ -static bool -virDomainDiskAddressDiskBusCompatibility(virDomainDiskBus bus, - virDomainDeviceAddressType addressType) -{ - if (addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) - return true; - - switch (bus) { - case VIR_DOMAIN_DISK_BUS_IDE: - case VIR_DOMAIN_DISK_BUS_FDC: - case VIR_DOMAIN_DISK_BUS_SCSI: - case VIR_DOMAIN_DISK_BUS_SATA: - return addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; - case VIR_DOMAIN_DISK_BUS_VIRTIO: - case VIR_DOMAIN_DISK_BUS_XEN: - case VIR_DOMAIN_DISK_BUS_USB: - case VIR_DOMAIN_DISK_BUS_UML: - case VIR_DOMAIN_DISK_BUS_SD: - case VIR_DOMAIN_DISK_BUS_LAST: - return true; - } - - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unexpected bus type '%d'"), - bus); - return true; -} - - -static int -virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels, - size_t nseclabels, - virSecurityLabelDefPtr *vmSeclabels, - size_t nvmSeclabels) -{ - virSecurityDeviceLabelDefPtr seclabel; - size_t i; - size_t j; - - for (i = 0; i < nseclabels; i++) { - seclabel = seclabels[i]; - - /* find the security label that it's being overridden */ - for (j = 0; j < nvmSeclabels; j++) { - if (STRNEQ_NULLABLE(vmSeclabels[j]->model, seclabel->model)) - continue; - - if (!vmSeclabels[j]->relabel) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("label overrides require relabeling to be " - "enabled at the domain level")); - return -1; - } - } - } - - return 0; -} - - -static int -virDomainDiskDefValidate(const virDomainDef *def, - const virDomainDiskDef *disk) -{ - virStorageSourcePtr next; - - /* Validate LUN configuration */ - if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - /* volumes haven't been translated at this point, so accept them */ - if (!(disk->src->type == VIR_STORAGE_TYPE_BLOCK || - disk->src->type == VIR_STORAGE_TYPE_VOLUME || - (disk->src->type == VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk '%s' improperly configured for a " - "device='lun'"), disk->dst); - return -1; - } - } - - if (disk->src->pr && - disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("<reservations/> allowed only for lun devices")); - return -1; - } - - /* Reject disks with a bus type that is not compatible with the - * given address type. The function considers only buses that are - * handled in common code. For other bus types it's not possible - * to decide compatibility in common code. - */ - if (!virDomainDiskAddressDiskBusCompatibility(disk->bus, disk->info.type)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Invalid address type '%s' for the disk '%s' with the bus type '%s'"), - virDomainDeviceAddressTypeToString(disk->info.type), - disk->dst, - virDomainDiskBusTypeToString(disk->bus)); - return -1; - } - - if (disk->queues && disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("queues attribute in disk driver element is only " - "supported by virtio-blk")); - return -1; - } - - if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && - (disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO || - disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL || - disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk model '%s' not supported for bus '%s'"), - virDomainDiskModelTypeToString(disk->model), - virDomainDiskBusTypeToString(disk->bus)); - return -1; - } - - if (disk->src->type == VIR_STORAGE_TYPE_NVME) { - /* NVMe namespaces start from 1 */ - if (disk->src->nvme->namespc == 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("NVMe namespace can't be zero")); - return -1; - } - } - - for (next = disk->src; next; next = next->backingStore) { - if (virSecurityDeviceLabelDefValidateXML(next->seclabels, - next->nseclabels, - def->seclabels, - def->nseclabels) < 0) - return -1; - } - - return 0; -} - bool virDomainDefHasUSB(const virDomainDef *def) { diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 234eb72f11..da36bef31a 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -150,3 +150,156 @@ virDomainVideoDefValidate(const virDomainVideoDef *video, return 0; } + + +/** + * virDomainDiskAddressDiskBusCompatibility: + * @bus: disk bus type + * @addressType: disk address type + * + * Check if the specified disk address type @addressType is compatible + * with the specified disk bus type @bus. This function checks + * compatibility with the bus types SATA, SCSI, FDC, and IDE only, + * because only these are handled in common code. + * + * Returns true if compatible or can't be decided in common code, + * false if known to be not compatible. + */ +static bool +virDomainDiskAddressDiskBusCompatibility(virDomainDiskBus bus, + virDomainDeviceAddressType addressType) +{ + if (addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + return true; + + switch (bus) { + case VIR_DOMAIN_DISK_BUS_IDE: + case VIR_DOMAIN_DISK_BUS_FDC: + case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_SATA: + return addressType == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE; + case VIR_DOMAIN_DISK_BUS_VIRTIO: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: + return true; + } + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unexpected bus type '%d'"), + bus); + return true; +} + + +int +virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels, + size_t nseclabels, + virSecurityLabelDefPtr *vmSeclabels, + size_t nvmSeclabels) +{ + virSecurityDeviceLabelDefPtr seclabel; + size_t i; + size_t j; + + for (i = 0; i < nseclabels; i++) { + seclabel = seclabels[i]; + + /* find the security label that it's being overridden */ + for (j = 0; j < nvmSeclabels; j++) { + if (STRNEQ_NULLABLE(vmSeclabels[j]->model, seclabel->model)) + continue; + + if (!vmSeclabels[j]->relabel) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("label overrides require relabeling to be " + "enabled at the domain level")); + return -1; + } + } + } + + return 0; +} + + +int +virDomainDiskDefValidate(const virDomainDef *def, + const virDomainDiskDef *disk) +{ + virStorageSourcePtr next; + + /* Validate LUN configuration */ + if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + /* volumes haven't been translated at this point, so accept them */ + if (!(disk->src->type == VIR_STORAGE_TYPE_BLOCK || + disk->src->type == VIR_STORAGE_TYPE_VOLUME || + (disk->src->type == VIR_STORAGE_TYPE_NETWORK && + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' improperly configured for a " + "device='lun'"), disk->dst); + return -1; + } + } + + if (disk->src->pr && + disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("<reservations/> allowed only for lun devices")); + return -1; + } + + /* Reject disks with a bus type that is not compatible with the + * given address type. The function considers only buses that are + * handled in common code. For other bus types it's not possible + * to decide compatibility in common code. + */ + if (!virDomainDiskAddressDiskBusCompatibility(disk->bus, disk->info.type)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Invalid address type '%s' for the disk '%s' with the bus type '%s'"), + virDomainDeviceAddressTypeToString(disk->info.type), + disk->dst, + virDomainDiskBusTypeToString(disk->bus)); + return -1; + } + + if (disk->queues && disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("queues attribute in disk driver element is only " + "supported by virtio-blk")); + return -1; + } + + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && + (disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO || + disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL || + disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk model '%s' not supported for bus '%s'"), + virDomainDiskModelTypeToString(disk->model), + virDomainDiskBusTypeToString(disk->bus)); + return -1; + } + + if (disk->src->type == VIR_STORAGE_TYPE_NVME) { + /* NVMe namespaces start from 1 */ + if (disk->src->nvme->namespc == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("NVMe namespace can't be zero")); + return -1; + } + } + + for (next = disk->src; next; next = next->backingStore) { + if (virSecurityDeviceLabelDefValidateXML(next->seclabels, + next->nseclabels, + def->seclabels, + def->nseclabels) < 0) + return -1; + } + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index ed170391f8..fe7c752e8c 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -28,3 +28,9 @@ int virDomainDefBootValidate(const virDomainDef *def); int virDomainDefVideoValidate(const virDomainDef *def); int virDomainVideoDefValidate(const virDomainVideoDef *video, const virDomainDef *def); +int virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels, + size_t nseclabels, + virSecurityLabelDefPtr *vmSeclabels, + size_t nvmSeclabels); +int virDomainDiskDefValidate(const virDomainDef *def, + const virDomainDiskDef *disk); -- 2.26.2
The 'tray' check isn't a XML parse specific code and can be pushed to the validate callback, in virDomainDiskDefValidate(). 'vendor' and 'product' string sizes are already checked by the domaincommon.rng schema, but can be of use in the validate callback since not all scenarios will go through the XML parsing. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 22 ---------------------- src/conf/domain_validate.c | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5aeb75ce59..220bb8cdcf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10222,9 +10222,6 @@ virDomainDiskDefParsePrivateData(xmlXPathContextPtr ctxt, } -#define VENDOR_LEN 8 -#define PRODUCT_LEN 16 - static virDomainDiskDefPtr virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, xmlNodePtr node, @@ -10399,12 +10396,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, if (!(vendor = virXMLNodeContentString(cur))) return NULL; - if (strlen(vendor) > VENDOR_LEN) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("disk vendor is more than 8 characters")); - return NULL; - } - if (!virStringIsPrintable(vendor)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("disk vendor is not printable string")); @@ -10415,12 +10406,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, if (!(product = virXMLNodeContentString(cur))) return NULL; - if (strlen(product) > PRODUCT_LEN) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("disk product is more than 16 characters")); - return NULL; - } - if (!virStringIsPrintable(product)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("disk product is not printable string")); @@ -10551,13 +10536,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, _("unknown disk tray status '%s'"), tray); return NULL; } - - if (def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY && - def->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("tray is only valid for cdrom and floppy")); - return NULL; - } } if (removable) { diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index da36bef31a..bc5ca3f668 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -225,6 +225,9 @@ virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels, } +#define VENDOR_LEN 8 +#define PRODUCT_LEN 16 + int virDomainDiskDefValidate(const virDomainDef *def, const virDomainDiskDef *disk) @@ -301,5 +304,27 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } + if (disk->tray_status && + disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY && + disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("tray is only valid for cdrom and floppy")); + return -1; + } + + if (disk->vendor && strlen(disk->vendor) > VENDOR_LEN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk vendor is more than %d characters"), + VENDOR_LEN); + return -1; + } + + if (disk->product && strlen(disk->product) > PRODUCT_LEN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk product is more than %d characters"), + PRODUCT_LEN); + return -1; + } + return 0; } -- 2.26.2
The function isn't doing XML validation of any sort. Rename it to be compatible with its actual use. While we're at it, change the VIR_ERR_XML_ERROR error being thrown in the function to VIR_ERR_CONFIG_UNSUPPORTED. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/domain_validate.c | 18 +++++++++--------- src/conf/domain_validate.h | 8 ++++---- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 220bb8cdcf..6578055119 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6164,10 +6164,10 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *src_def, break; } - if (virSecurityDeviceLabelDefValidateXML(src_def->seclabels, - src_def->nseclabels, - def->seclabels, - def->nseclabels) < 0) + if (virSecurityDeviceLabelDefValidate(src_def->seclabels, + src_def->nseclabels, + def->seclabels, + def->nseclabels) < 0) return -1; return 0; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index bc5ca3f668..e9427cffa8 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -195,10 +195,10 @@ virDomainDiskAddressDiskBusCompatibility(virDomainDiskBus bus, int -virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels, - size_t nseclabels, - virSecurityLabelDefPtr *vmSeclabels, - size_t nvmSeclabels) +virSecurityDeviceLabelDefValidate(virSecurityDeviceLabelDefPtr *seclabels, + size_t nseclabels, + virSecurityLabelDefPtr *vmSeclabels, + size_t nvmSeclabels) { virSecurityDeviceLabelDefPtr seclabel; size_t i; @@ -213,7 +213,7 @@ virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels, continue; if (!vmSeclabels[j]->relabel) { - virReportError(VIR_ERR_XML_ERROR, "%s", + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("label overrides require relabeling to be " "enabled at the domain level")); return -1; @@ -297,10 +297,10 @@ virDomainDiskDefValidate(const virDomainDef *def, } for (next = disk->src; next; next = next->backingStore) { - if (virSecurityDeviceLabelDefValidateXML(next->seclabels, - next->nseclabels, - def->seclabels, - def->nseclabels) < 0) + if (virSecurityDeviceLabelDefValidate(next->seclabels, + next->nseclabels, + def->seclabels, + def->nseclabels) < 0) return -1; } diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index fe7c752e8c..aef169a4c9 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -28,9 +28,9 @@ int virDomainDefBootValidate(const virDomainDef *def); int virDomainDefVideoValidate(const virDomainDef *def); int virDomainVideoDefValidate(const virDomainVideoDef *video, const virDomainDef *def); -int virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels, - size_t nseclabels, - virSecurityLabelDefPtr *vmSeclabels, - size_t nvmSeclabels); +int virSecurityDeviceLabelDefValidate(virSecurityDeviceLabelDefPtr *seclabels, + size_t nseclabels, + virSecurityLabelDefPtr *vmSeclabels, + size_t nvmSeclabels); int virDomainDiskDefValidate(const virDomainDef *def, const virDomainDiskDef *disk); -- 2.26.2
Next patch will move a validation to virDomainSmartcardDefValidate(), but this function can't be moved alone to domain_validate.c without making virDomainChrSourceDefValidate(), from domain_conf.c, public. Given that the idea is to eventually move all validations to domain_validate.c anyways, let's move all ChrSource related validations in a single punch. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 161 ------------------------------------- src/conf/domain_validate.c | 161 +++++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 8 ++ 3 files changed, 169 insertions(+), 161 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6578055119..a301ab5c74 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6058,137 +6058,6 @@ virDomainDefHasUSB(const virDomainDef *def) } -#define SERIAL_CHANNEL_NAME_CHARS \ - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-." - - -static int -virDomainChrSourceDefValidate(const virDomainChrSourceDef *src_def, - const virDomainChrDef *chr_def, - const virDomainDef *def) -{ - switch ((virDomainChrType) src_def->type) { - case VIR_DOMAIN_CHR_TYPE_NULL: - case VIR_DOMAIN_CHR_TYPE_PTY: - case VIR_DOMAIN_CHR_TYPE_VC: - case VIR_DOMAIN_CHR_TYPE_STDIO: - case VIR_DOMAIN_CHR_TYPE_SPICEVMC: - case VIR_DOMAIN_CHR_TYPE_LAST: - break; - - case VIR_DOMAIN_CHR_TYPE_FILE: - case VIR_DOMAIN_CHR_TYPE_DEV: - case VIR_DOMAIN_CHR_TYPE_PIPE: - if (!src_def->data.file.path) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source path attribute for char device")); - return -1; - } - break; - - case VIR_DOMAIN_CHR_TYPE_NMDM: - if (!src_def->data.nmdm.master) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing master path attribute for nmdm device")); - return -1; - } - - if (!src_def->data.nmdm.slave) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing slave path attribute for nmdm device")); - return -1; - } - break; - - case VIR_DOMAIN_CHR_TYPE_TCP: - if (!src_def->data.tcp.host) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source host attribute for char device")); - return -1; - } - - if (!src_def->data.tcp.service) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source service attribute for char device")); - return -1; - } - - if (src_def->data.tcp.listen && src_def->data.tcp.reconnect.enabled) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("chardev reconnect is possible only for connect mode")); - return -1; - } - break; - - case VIR_DOMAIN_CHR_TYPE_UDP: - if (!src_def->data.udp.connectService) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source service attribute for char device")); - return -1; - } - break; - - case VIR_DOMAIN_CHR_TYPE_UNIX: - /* The source path can be auto generated for certain specific - * types of channels, but in most cases we should report an - * error if the user didn't provide it */ - if (!src_def->data.nix.path && - !(chr_def && - chr_def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && - (chr_def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN || - chr_def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing source path attribute for char device")); - return -1; - } - - if (src_def->data.nix.listen && src_def->data.nix.reconnect.enabled) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("chardev reconnect is possible only for connect mode")); - return -1; - } - break; - - case VIR_DOMAIN_CHR_TYPE_SPICEPORT: - if (!src_def->data.spiceport.channel) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Missing source channel attribute for char device")); - return -1; - } - if (strspn(src_def->data.spiceport.channel, - SERIAL_CHANNEL_NAME_CHARS) < strlen(src_def->data.spiceport.channel)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("Invalid character in source channel for char device")); - return -1; - } - break; - } - - if (virSecurityDeviceLabelDefValidate(src_def->seclabels, - src_def->nseclabels, - def->seclabels, - def->nseclabels) < 0) - return -1; - - return 0; -} - - -static int -virDomainRedirdevDefValidate(const virDomainDef *def, - const virDomainRedirdevDef *redirdev) -{ - if (redirdev->bus == VIR_DOMAIN_REDIRDEV_BUS_USB && - !virDomainDefHasUSB(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("cannot add redirected USB device: " - "USB is disabled for this domain")); - return -1; - } - - return virDomainChrSourceDefValidate(redirdev->source, NULL, def); -} - static int virDomainNetDefValidatePortOptions(const char *macstr, @@ -6379,36 +6248,6 @@ virDomainControllerDefValidate(const virDomainControllerDef *controller) } -static int -virDomainChrDefValidate(const virDomainChrDef *chr, - const virDomainDef *def) -{ - return virDomainChrSourceDefValidate(chr->source, chr, def); -} - - -static int -virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard, - const virDomainDef *def) -{ - if (smartcard->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH) - return virDomainChrSourceDefValidate(smartcard->data.passthru, NULL, def); - - return 0; -} - - -static int -virDomainRNGDefValidate(const virDomainRNGDef *rng, - const virDomainDef *def) -{ - if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) - return virDomainChrSourceDefValidate(rng->source.chardev, NULL, def); - - return 0; -} - - static int virDomainHostdevDefValidate(const virDomainHostdevDef *hostdev) { diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index e9427cffa8..882fbdac57 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -328,3 +328,164 @@ virDomainDiskDefValidate(const virDomainDef *def, return 0; } + + +#define SERIAL_CHANNEL_NAME_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-." + +static int +virDomainChrSourceDefValidate(const virDomainChrSourceDef *src_def, + const virDomainChrDef *chr_def, + const virDomainDef *def) +{ + switch ((virDomainChrType) src_def->type) { + case VIR_DOMAIN_CHR_TYPE_NULL: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_VC: + case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + case VIR_DOMAIN_CHR_TYPE_LAST: + break; + + case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_PIPE: + if (!src_def->data.file.path) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source path attribute for char device")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_NMDM: + if (!src_def->data.nmdm.master) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing master path attribute for nmdm device")); + return -1; + } + + if (!src_def->data.nmdm.slave) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing slave path attribute for nmdm device")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_TCP: + if (!src_def->data.tcp.host) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source host attribute for char device")); + return -1; + } + + if (!src_def->data.tcp.service) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source service attribute for char device")); + return -1; + } + + if (src_def->data.tcp.listen && src_def->data.tcp.reconnect.enabled) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("chardev reconnect is possible only for connect mode")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_UDP: + if (!src_def->data.udp.connectService) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source service attribute for char device")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_UNIX: + /* The source path can be auto generated for certain specific + * types of channels, but in most cases we should report an + * error if the user didn't provide it */ + if (!src_def->data.nix.path && + !(chr_def && + chr_def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && + (chr_def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN || + chr_def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing source path attribute for char device")); + return -1; + } + + if (src_def->data.nix.listen && src_def->data.nix.reconnect.enabled) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("chardev reconnect is possible only for connect mode")); + return -1; + } + break; + + case VIR_DOMAIN_CHR_TYPE_SPICEPORT: + if (!src_def->data.spiceport.channel) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing source channel attribute for char device")); + return -1; + } + if (strspn(src_def->data.spiceport.channel, + SERIAL_CHANNEL_NAME_CHARS) < strlen(src_def->data.spiceport.channel)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Invalid character in source channel for char device")); + return -1; + } + break; + } + + if (virSecurityDeviceLabelDefValidate(src_def->seclabels, + src_def->nseclabels, + def->seclabels, + def->nseclabels) < 0) + return -1; + + return 0; +} + + +int +virDomainRedirdevDefValidate(const virDomainDef *def, + const virDomainRedirdevDef *redirdev) +{ + if (redirdev->bus == VIR_DOMAIN_REDIRDEV_BUS_USB && + !virDomainDefHasUSB(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cannot add redirected USB device: " + "USB is disabled for this domain")); + return -1; + } + + return virDomainChrSourceDefValidate(redirdev->source, NULL, def); +} + + +int +virDomainChrDefValidate(const virDomainChrDef *chr, + const virDomainDef *def) +{ + return virDomainChrSourceDefValidate(chr->source, chr, def); +} + + +int +virDomainRNGDefValidate(const virDomainRNGDef *rng, + const virDomainDef *def) +{ + if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) + return virDomainChrSourceDefValidate(rng->source.chardev, NULL, def); + + return 0; +} + + +int +virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard, + const virDomainDef *def) +{ + if (smartcard->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH) + return virDomainChrSourceDefValidate(smartcard->data.passthru, NULL, def); + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index aef169a4c9..d65de50422 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -34,3 +34,11 @@ int virSecurityDeviceLabelDefValidate(virSecurityDeviceLabelDefPtr *seclabels, size_t nvmSeclabels); int virDomainDiskDefValidate(const virDomainDef *def, const virDomainDiskDef *disk); +int virDomainRedirdevDefValidate(const virDomainDef *def, + const virDomainRedirdevDef *redirdev); +int virDomainChrDefValidate(const virDomainChrDef *chr, + const virDomainDef *def); +int virDomainRNGDefValidate(const virDomainRNGDef *rng, + const virDomainDef *def); +int virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard, + const virDomainDef *def); -- 2.26.2
This check is not tied to XML parsing and can be moved to virDomainSmartcardDefValidate(). Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 7 ------- src/conf/domain_validate.c | 7 +++++++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a301ab5c74..a5dcb0bbce 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13222,13 +13222,6 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt, if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) return NULL; - if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && - def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Controllers must use the 'ccid' address type")); - return NULL; - } - return g_steal_pointer(&def); } diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 882fbdac57..6fca604d17 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -484,6 +484,13 @@ int virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard, const virDomainDef *def) { + if (smartcard->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + smartcard->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Controllers must use the 'ccid' address type")); + return -1; + } + if (smartcard->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH) return virDomainChrSourceDefValidate(smartcard->data.passthru, NULL, def); -- 2.26.2
Move this check to a new virDomainDefTunablesValidate(), which is called by virDomainDefValidateInternal(). Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 14 ++++---------- src/conf/domain_validate.c | 21 +++++++++++++++++++++ src/conf/domain_validate.h | 1 + 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a5dcb0bbce..064d77d933 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6981,6 +6981,9 @@ virDomainDefValidateInternal(const virDomainDef *def, if (virDomainDefVideoValidate(def) < 0) return -1; + if (virDomainDefTunablesValidate(def) < 0) + return -1; + if (virDomainNumaDefValidate(def->numa) < 0) return -1; @@ -20875,7 +20878,7 @@ virDomainDefTunablesParse(virDomainDefPtr def, unsigned int flags) { g_autofree xmlNodePtr *nodes = NULL; - size_t i, j; + size_t i; int n; /* Extract blkio cgroup tunables */ @@ -20896,15 +20899,6 @@ virDomainDefTunablesParse(virDomainDefPtr def, &def->blkio.devices[i]) < 0) return -1; def->blkio.ndevices++; - for (j = 0; j < i; j++) { - if (STREQ(def->blkio.devices[j].path, - def->blkio.devices[i].path)) { - virReportError(VIR_ERR_XML_ERROR, - _("duplicate blkio device path '%s'"), - def->blkio.devices[i].path); - return -1; - } - } } VIR_FREE(nodes); diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 6fca604d17..09ab908ea3 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -496,3 +496,24 @@ virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard, return 0; } + + +int +virDomainDefTunablesValidate(const virDomainDef *def) +{ + size_t i, j; + + for (i = 0; i < def->blkio.ndevices; i++) { + for (j = 0; j < i; j++) { + if (STREQ(def->blkio.devices[j].path, + def->blkio.devices[i].path)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("duplicate blkio device path '%s'"), + def->blkio.devices[i].path); + return -1; + } + } + } + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index d65de50422..2bd9e71073 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -42,3 +42,4 @@ int virDomainRNGDefValidate(const virDomainRNGDef *rng, const virDomainDef *def); int virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard, const virDomainDef *def); +int virDomainDefTunablesValidate(const virDomainDef *def); -- 2.26.2
Next patch will add more validations to this function. Let's move it to domain_validate.c beforehand. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 37 ------------------------------------- src/conf/domain_validate.c | 37 +++++++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 1 + 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 064d77d933..453dc6cf6a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6211,43 +6211,6 @@ virDomainNetDefValidate(const virDomainNetDef *net) } -static int -virDomainControllerDefValidate(const virDomainControllerDef *controller) -{ - if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { - const virDomainPCIControllerOpts *opts = &controller->opts.pciopts; - - if (controller->idx > 255) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("PCI controller index %d too high, maximum is 255"), - controller->idx); - return -1; - } - - /* Only validate the target index if it's been set */ - if (opts->targetIndex != -1) { - - if (opts->targetIndex < 0 || opts->targetIndex > 30) { - virReportError(VIR_ERR_XML_ERROR, - _("PCI controller target index '%d' out of " - "range - must be 0-30"), - opts->targetIndex); - return -1; - } - - if ((controller->idx == 0 && opts->targetIndex != 0) || - (controller->idx != 0 && opts->targetIndex == 0)) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Only the PCI controller with index 0 can " - "have target index 0, and vice versa")); - return -1; - } - } - } - return 0; -} - - static int virDomainHostdevDefValidate(const virDomainHostdevDef *hostdev) { diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 09ab908ea3..416c24f97b 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -517,3 +517,40 @@ virDomainDefTunablesValidate(const virDomainDef *def) return 0; } + + +int +virDomainControllerDefValidate(const virDomainControllerDef *controller) +{ + if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + const virDomainPCIControllerOpts *opts = &controller->opts.pciopts; + + if (controller->idx > 255) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller index %d too high, maximum is 255"), + controller->idx); + return -1; + } + + /* Only validate the target index if it's been set */ + if (opts->targetIndex != -1) { + + if (opts->targetIndex < 0 || opts->targetIndex > 30) { + virReportError(VIR_ERR_XML_ERROR, + _("PCI controller target index '%d' out of " + "range - must be 0-30"), + opts->targetIndex); + return -1; + } + + if ((controller->idx == 0 && opts->targetIndex != 0) || + (controller->idx != 0 && opts->targetIndex == 0)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Only the PCI controller with index 0 can " + "have target index 0, and vice versa")); + return -1; + } + } + } + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index 2bd9e71073..e8004e358d 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -43,3 +43,4 @@ int virDomainRNGDefValidate(const virDomainRNGDef *rng, int virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard, const virDomainDef *def); int virDomainDefTunablesValidate(const virDomainDef *def); +int virDomainControllerDefValidate(const virDomainControllerDef *controller); -- 2.26.2
virDomainControllerDefParseXML() does a lot of checks with virDomainPCIControllerOpts parameters that can be moved to virDomainControllerDefValidate, sharing the logic with other use cases that does not rely on XML parsing. 'pseries-default-phb-numa-node' parse error was changed to reflect the error that is being thrown by qemuValidateDomainDeviceDefController() via deviceValidateCallback, that is executed before virDomainControllerDefValidate(). Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 42 +--------------- src/conf/domain_validate.c | 48 +++++++++++++++++++ .../pseries-default-phb-numa-node.err | 2 +- tests/qemuxml2argvtest.c | 6 ++- 4 files changed, 56 insertions(+), 42 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 453dc6cf6a..7adf2700ae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10864,14 +10864,6 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt, chassisNr); return NULL; } - if (def->opts.pciopts.chassisNr < 1 || - def->opts.pciopts.chassisNr > 255) { - virReportError(VIR_ERR_XML_ERROR, - _("PCI controller chassisNr '%s' out of range " - "- must be 1-255"), - chassisNr); - return NULL; - } } if (chassis) { if (virStrToLong_i(chassis, NULL, 0, @@ -10881,14 +10873,6 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt, chassis); return NULL; } - if (def->opts.pciopts.chassis < 0 || - def->opts.pciopts.chassis > 255) { - virReportError(VIR_ERR_XML_ERROR, - _("PCI controller chassis '%s' out of range " - "- must be 0-255"), - chassis); - return NULL; - } } if (port) { if (virStrToLong_i(port, NULL, 0, @@ -10898,14 +10882,6 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt, port); return NULL; } - if (def->opts.pciopts.port < 0 || - def->opts.pciopts.port > 255) { - virReportError(VIR_ERR_XML_ERROR, - _("PCI controller port '%s' out of range " - "- must be 0-255"), - port); - return NULL; - } } if (busNr) { if (virStrToLong_i(busNr, NULL, 0, @@ -10915,14 +10891,6 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt, busNr); return NULL; } - if (def->opts.pciopts.busNr < 1 || - def->opts.pciopts.busNr > 254) { - virReportError(VIR_ERR_XML_ERROR, - _("PCI controller busNr '%s' out of range " - "- must be 1-254"), - busNr); - return NULL; - } } if (targetIndex) { if (virStrToLong_i(targetIndex, NULL, 0, @@ -10934,15 +10902,9 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt, return NULL; } } - if (numaNode >= 0) { - if (def->idx == 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("The PCI controller with index=0 can't " - "be associated with a NUMA node")); - return NULL; - } + if (numaNode >= 0) def->opts.pciopts.numaNode = numaNode; - } + if (hotplug) { int val = virTristateSwitchTypeFromString(hotplug); diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 416c24f97b..f47e80ca38 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -551,6 +551,54 @@ virDomainControllerDefValidate(const virDomainControllerDef *controller) return -1; } } + + if (opts->chassisNr != -1) { + if (opts->chassisNr < 1 || opts->chassisNr > 255) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller chassisNr '%d' out of range " + "- must be 1-255"), + opts->chassisNr); + return -1; + } + } + + if (opts->chassis != -1) { + if (opts->chassis < 0 || opts->chassis > 255) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller chassis '%d' out of range " + "- must be 0-255"), + opts->chassis); + return -1; + } + } + + if (opts->port != -1) { + if (opts->port < 0 || opts->port > 255) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller port '%d' out of range " + "- must be 0-255"), + opts->port); + return -1; + } + } + + if (opts->busNr != -1) { + if (opts->busNr < 1 || opts->busNr > 254) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller busNr '%d' out of range " + "- must be 1-254"), + opts->busNr); + return -1; + } + } + + if (opts->numaNode >= 0 && controller->idx == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("The PCI controller with index=0 can't " + "be associated with a NUMA node")); + return -1; + } } + return 0; } diff --git a/tests/qemuxml2argvdata/pseries-default-phb-numa-node.err b/tests/qemuxml2argvdata/pseries-default-phb-numa-node.err index 5d11109317..e46b710330 100644 --- a/tests/qemuxml2argvdata/pseries-default-phb-numa-node.err +++ b/tests/qemuxml2argvdata/pseries-default-phb-numa-node.err @@ -1 +1 @@ -XML error: The PCI controller with index=0 can't be associated with a NUMA node +unsupported configuration: Option 'numaNode' is not valid for PCI controller with index '0', model 'pci-root' and modelName 'spapr-pci-host-bridge' diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0e7d8d5ba3..9b853c6d59 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2115,7 +2115,11 @@ mymain(void) QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE); - DO_TEST_PARSE_ERROR("pseries-default-phb-numa-node", NONE); + DO_TEST_PARSE_ERROR("pseries-default-phb-numa-node", + QEMU_CAPS_NUMA, + QEMU_CAPS_OBJECT_MEMORY_RAM, + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, + QEMU_CAPS_SPAPR_PCI_HOST_BRIDGE_NUMA_NODE); DO_TEST_PARSE_ERROR("pseries-phb-invalid-target-index-1", NONE); DO_TEST_PARSE_ERROR("pseries-phb-invalid-target-index-2", NONE); DO_TEST_PARSE_ERROR("pseries-phb-invalid-target-index-3", NONE); -- 2.26.2
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 6 ------ src/conf/domain_validate.c | 10 ++++++++++ tests/qemuxml2argvdata/pci-root-address.err | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7adf2700ae..7a7c9ec85c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10820,12 +10820,6 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt, case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: { unsigned long long bytes; - if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("pci-root and pcie-root controllers should not " - "have an address")); - return NULL; - } if ((rc = virParseScaledValue("./pcihole64", NULL, ctxt, &bytes, 1024, 1024ULL * ULONG_MAX, false)) < 0) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index f47e80ca38..35deb9f017 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -525,6 +525,16 @@ virDomainControllerDefValidate(const virDomainControllerDef *controller) if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { const virDomainPCIControllerOpts *opts = &controller->opts.pciopts; + if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { + if (controller->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("pci-root and pcie-root controllers " + "should not have an address")); + return -1; + } + } + if (controller->idx > 255) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller index %d too high, maximum is 255"), diff --git a/tests/qemuxml2argvdata/pci-root-address.err b/tests/qemuxml2argvdata/pci-root-address.err index 53dad81985..ffe5438224 100644 --- a/tests/qemuxml2argvdata/pci-root-address.err +++ b/tests/qemuxml2argvdata/pci-root-address.err @@ -1 +1 @@ -XML error: pci-root and pcie-root controllers should not have an address +unsupported configuration: pci-root and pcie-root controllers should not have an address -- 2.26.2
Create a new function called virDomainDefIdMapValidate() and use it to move these checks out of virDomainIdmapDefParseXML() and virDomainDefParseXML(). Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 19 +++---------------- src/conf/domain_validate.c | 23 +++++++++++++++++++++++ src/conf/domain_validate.h | 1 + 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7a7c9ec85c..44f136f530 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6947,6 +6947,9 @@ virDomainDefValidateInternal(const virDomainDef *def, if (virDomainDefTunablesValidate(def) < 0) return -1; + if (virDomainDefIdMapValidate(def) < 0) + return -1; + if (virDomainNumaDefValidate(def->numa) < 0) return -1; @@ -18463,15 +18466,6 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, qsort(idmap, num, sizeof(idmap[0]), virDomainIdMapEntrySort); - if (idmap[0].start != 0) { - /* Root user of container hasn't been mapped to any user of host, - * return error. */ - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("You must map the root user of container")); - VIR_FREE(idmap); - return NULL; - } - return idmap; } @@ -21961,13 +21955,6 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes); - if ((def->idmap.uidmap && !def->idmap.gidmap) || - (!def->idmap.uidmap && def->idmap.gidmap)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("uid and gid should be mapped both")); - goto error; - } - if ((n = virXPathNodeSet("./sysinfo", ctxt, &nodes)) < 0) goto error; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 35deb9f017..0eed1ba982 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -612,3 +612,26 @@ virDomainControllerDefValidate(const virDomainControllerDef *controller) return 0; } + + +int +virDomainDefIdMapValidate(const virDomainDef *def) +{ + if ((def->idmap.uidmap && !def->idmap.gidmap) || + (!def->idmap.uidmap && def->idmap.gidmap)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("uid and gid should be mapped both")); + return -1; + } + + if ((def->idmap.uidmap && def->idmap.uidmap[0].start != 0) || + (def->idmap.gidmap && def->idmap.gidmap[0].start != 0)) { + /* Root user of container hasn't been mapped to any user of host, + * return error. */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("You must map the root user of container")); + return -1; + } + + return 0; +} diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index e8004e358d..497a02b9b3 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -44,3 +44,4 @@ int virDomainSmartcardDefValidate(const virDomainSmartcardDef *smartcard, const virDomainDef *def); int virDomainDefTunablesValidate(const virDomainDef *def); int virDomainControllerDefValidate(const virDomainControllerDef *controller); +int virDomainDefIdMapValidate(const virDomainDef *def); -- 2.26.2
On 12/8/20 11:20 PM, Daniel Henrique Barboza wrote:
This is the v3 of https://www.redhat.com/archives/libvir-list/2020-December/msg00419.html
Michal mentioned in his v2 review that we should move these validations out of domain_conf.c to a new file. Let's create a new file called domain_validate.c and start move the validations done in v2 into it, instead of pushing the already reviewed v2 series just to have more stuff to be moved later on.
A follow up series will push more validations from domain_conf.c to domain_validate.c.
Michal's R-bs were kept in all patches but patch 01.
Daniel Henrique Barboza (14): domain_conf: move boot timeouts check to domain_validate.c domain_conf.c: move primary video check to validate callback domain_conf.c: move virDomainVideoDefValidate() to domain_validate.c domain_conf.c: move QXL attributes check to virDomainVideoDefValidate() domain_conf: move virDomainDiskDefValidate() to domain_validate.c domain_conf: move vendor, product and tray checks to domain_validate.c domain_validate.c: rename virSecurityDeviceLabelDefValidateXML() domain_conf: move all ChrSource checks to domain_validate.c domain_conf.c: move smartcard address check to domain_validate.c domain_conf.c: move blkio path check to domain_validate.c domain_conf.c: move virDomainControllerDefValidate() to domain_validate.c domain_conf: move virDomainPCIControllerOpts checks to domain_validate.c domain_conf: move pci-root/pcie-root address check to domain_validate.c domain_conf.c: move idmapEntry checks to domain_validate.c
po/POTFILES.in | 1 + src/conf/domain_conf.c | 562 +-------------- src/conf/domain_validate.c | 637 ++++++++++++++++++ src/conf/domain_validate.h | 47 ++ src/conf/meson.build | 1 + tests/qemuxml2argvdata/pci-root-address.err | 2 +- .../pseries-default-phb-numa-node.err | 2 +- .../video-multiple-primaries.err | 1 + .../video-multiple-primaries.xml | 32 + tests/qemuxml2argvtest.c | 14 +- 10 files changed, 756 insertions(+), 543 deletions(-) create mode 100644 src/conf/domain_validate.c create mode 100644 src/conf/domain_validate.h create mode 100644 tests/qemuxml2argvdata/video-multiple-primaries.err create mode 100644 tests/qemuxml2argvdata/video-multiple-primaries.xml
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On 12/9/20 5:13 AM, Michal Privoznik wrote:
On 12/8/20 11:20 PM, Daniel Henrique Barboza wrote:
This is the v3 of https://www.redhat.com/archives/libvir-list/2020-December/msg00419.html
Michal mentioned in his v2 review that we should move these validations out of domain_conf.c to a new file. Let's create a new file called domain_validate.c and start move the validations done in v2 into it, instead of pushing the already reviewed v2 series just to have more stuff to be moved later on.
A follow up series will push more validations from domain_conf.c to domain_validate.c.
Michal's R-bs were kept in all patches but patch 01.
Daniel Henrique Barboza (14): domain_conf: move boot timeouts check to domain_validate.c domain_conf.c: move primary video check to validate callback domain_conf.c: move virDomainVideoDefValidate() to domain_validate.c domain_conf.c: move QXL attributes check to virDomainVideoDefValidate() domain_conf: move virDomainDiskDefValidate() to domain_validate.c domain_conf: move vendor, product and tray checks to domain_validate.c domain_validate.c: rename virSecurityDeviceLabelDefValidateXML() domain_conf: move all ChrSource checks to domain_validate.c domain_conf.c: move smartcard address check to domain_validate.c domain_conf.c: move blkio path check to domain_validate.c domain_conf.c: move virDomainControllerDefValidate() to domain_validate.c domain_conf: move virDomainPCIControllerOpts checks to domain_validate.c domain_conf: move pci-root/pcie-root address check to domain_validate.c domain_conf.c: move idmapEntry checks to domain_validate.c
po/POTFILES.in | 1 + src/conf/domain_conf.c | 562 +-------------- src/conf/domain_validate.c | 637 ++++++++++++++++++ src/conf/domain_validate.h | 47 ++ src/conf/meson.build | 1 + tests/qemuxml2argvdata/pci-root-address.err | 2 +- .../pseries-default-phb-numa-node.err | 2 +- .../video-multiple-primaries.err | 1 + .../video-multiple-primaries.xml | 32 + tests/qemuxml2argvtest.c | 14 +- 10 files changed, 756 insertions(+), 543 deletions(-) create mode 100644 src/conf/domain_validate.c create mode 100644 src/conf/domain_validate.h create mode 100644 tests/qemuxml2argvdata/video-multiple-primaries.err create mode 100644 tests/qemuxml2argvdata/video-multiple-primaries.xml
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thanks! I'll wait a bit to see if people has any opinions about the copyright text before pushing the series. DHB
Michal
participants (3)
-
Daniel Henrique Barboza -
Daniel P. Berrangé -
Michal Privoznik