8:54 a.m.
An alternative to Peter's:
[PATCH 1/4] conf: domain: Remove code accessing 'bootHash' from the post-parse
infrestructure
<8b4627fedd199117af1ab46f5562e3838679357e.1527500017.git.pkrempa(a)redhat.com>
Ján Tomko (3):
vmx: add VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER
conf: introduce virDomainDefCheckBootOrder
conf: remove 'bootHash' completely
Peter Krempa (1):
conf: remove 'bootHash' from the post-parse infrastructure
src/conf/domain_conf.c | 209 +++++++++++++++------------
src/conf/domain_conf.h | 1 +
src/vmx/vmx.c | 3 +-
tests/qemuargv2xmldata/nomachine-aarch64.xml | 1 +
tests/qemuargv2xmldata/nomachine-ppc64.xml | 1 +
tests/qemuargv2xmldata/nomachine-x86_64.xml | 1 +
tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml | 1 +
7 files changed, 120 insertions(+), 97 deletions(-)
--
2.16.1
8:54 a.m.
New subject: [libvirt] [PATCH 1/4] vmx: add VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER
Further patches will introduce validation and a default setting
of def->os.bootDevs in postParse.
Introduce a feature flag to opt out of this and set it in the vmx
driver.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/conf/domain_conf.h | 1 +
src/vmx/vmx.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index b7e52a1e03..e10206b358 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2557,6 +2557,7 @@ typedef enum {
VIR_DOMAIN_DEF_FEATURE_NAME_SLASH = (1 << 3),
VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS = (1 << 4),
VIR_DOMAIN_DEF_FEATURE_USER_ALIAS = (1 << 5),
+ VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER = (1 << 6),
} virDomainDefFeatures;
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index df6a58a474..bdc27b15b0 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -550,7 +550,8 @@ static virDomainDefParserConfig virVMXDomainDefParserConfig = {
.devicesPostParseCallback = virVMXDomainDevicesDefPostParse,
.domainPostParseCallback = virVMXDomainDefPostParse,
.features = (VIR_DOMAIN_DEF_FEATURE_WIDE_SCSI |
- VIR_DOMAIN_DEF_FEATURE_NAME_SLASH),
+ VIR_DOMAIN_DEF_FEATURE_NAME_SLASH |
+ VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER),
};
struct virVMXDomainDefNamespaceData {
--
2.16.1
2:30 a.m.
New subject: [libvirt] [PATCH 1/4] vmx: add VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER
On Mon, May 28, 2018 at 15:54:02 +0200, Ján Tomko wrote:
Further patches will introduce validation and a default setting
of def->os.bootDevs in postParse.
Introduce a feature flag to opt out of this and set it in the vmx
driver.
This does not clarify in any way why it is required.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/conf/domain_conf.h | 1 +
src/vmx/vmx.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
2:48 a.m.
New subject: [libvirt] [PATCH 1/4] vmx: add VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER
On Tue, May 29, 2018 at 09:30:05AM +0200, Peter Krempa wrote:
On Mon, May 28, 2018 at 15:54:02 +0200, Ján Tomko wrote:
> Further patches will introduce validation and a default setting
> of def->os.bootDevs in postParse.
>
> Introduce a feature flag to opt out of this and set it in the vmx
> driver.
This does not clarify in any way why it is required.
Introduce a feature flag to opt out of this and set it in the vmx
driver, otherwise we would be adding it <boot dev='hd'/> into every
vmx config despite having no way to change it.
(Alternatively, if booting from hard-drive is the default, we can just
leave it in because none of the vmx code even touches bootDevs, so
it will be safely ignored)
Jano
>
> Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> ---
> src/conf/domain_conf.h | 1 +
> src/vmx/vmx.c | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
2:54 a.m.
New subject: [libvirt] [PATCH 1/4] vmx: add VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER
On Tue, May 29, 2018 at 09:48:51 +0200, Ján Tomko wrote:
On Tue, May 29, 2018 at 09:30:05AM +0200, Peter Krempa wrote:
> On Mon, May 28, 2018 at 15:54:02 +0200, Ján Tomko wrote:
> > Further patches will introduce validation and a default setting
> > of def->os.bootDevs in postParse.
> >
> > Introduce a feature flag to opt out of this and set it in the vmx
> > driver.
>
> This does not clarify in any way why it is required.
>
Introduce a feature flag to opt out of this and set it in the vmx
driver, otherwise we would be adding it <boot dev='hd'/> into every
vmx config despite having no way to change it.
ACK to the patch if you add this wording.
(Alternatively, if booting from hard-drive is the default, we can just
leave it in because none of the vmx code even touches bootDevs, so
it will be safely ignored)
It very well might be the default. The capability can easily be deleted
later.
Jano
> >
> > Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> > ---
> > src/conf/domain_conf.h | 1 +
> > src/vmx/vmx.c | 3 ++-
> > 2 files changed, 3 insertions(+), 1 deletion(-)
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:54 a.m.
New subject: [libvirt] [PATCH 2/4] conf: introduce virDomainDefCheckBootOrder
Move the check for boot elements into a separate function
and remove its dependency on the parser-supplied bootHash table.
Reconstructing the hash table from the domain definition
effectively duplicates the check for duplicate boot order
values, also present in virDomainDeviceBootParseXML.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/conf/domain_conf.c | 89 +++++++++++++++++++++++-----
tests/qemuargv2xmldata/nomachine-aarch64.xml | 1 +
tests/qemuargv2xmldata/nomachine-ppc64.xml | 1 +
tests/qemuargv2xmldata/nomachine-x86_64.xml | 1 +
tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml | 1 +
5 files changed, 78 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d6ac47c629..f087a3680f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4939,10 +4939,79 @@ virDomainDefPostParseCPU(virDomainDefPtr def)
}
+static int
+virDomainDefCollectBootOrder(virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
+ virDomainDeviceInfoPtr info,
+ void *data)
+{
+ virHashTablePtr bootHash = data;
+ char *order = NULL;
+ int ret = -1;
+
+ if (info->bootIndex == 0)
+ return 0;
+
+ if (virAsprintf(&order, "%u", info->bootIndex) < 0)
+ goto cleanup;
+
+ if (virHashLookup(bootHash, order)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("boot order '%s' used for more than one
device"),
+ order);
+ goto cleanup;
+ }
+
+ if (virHashAddEntry(bootHash, order, (void *) 1) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(order);
+ return ret;
+}
+
+
+static int
+virDomainDefCheckBootOrder(virDomainDefPtr def)
+{
+ virHashTablePtr bootHash = NULL;
+ int ret = -1;
+
+ if (def->os.type != VIR_DOMAIN_OSTYPE_HVM)
+ return 0;
+
+ if (!(bootHash = virHashCreate(5, NULL)))
+ goto cleanup;
+
+ if (virDomainDeviceInfoIterate(def, virDomainDefCollectBootOrder, bootHash) < 0)
+ goto cleanup;
+
+ if (def->os.nBootDevs > 0 && virHashSize(bootHash) > 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("per-device boot elements cannot be used"
+ " together with os/boot elements"));
+ goto cleanup;
+ }
+
+ if (def->os.nBootDevs == 0 && virHashSize(bootHash) == 0) {
+ def->os.nBootDevs = 1;
+ def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK;
+ }
+
+ ret = 0;
+
+ cleanup:
+ virHashFree(bootHash);
+ return ret;
+}
+
+
static int
virDomainDefPostParseCommon(virDomainDefPtr def,
struct virDomainDefPostParseDeviceIteratorData *data,
- virHashTablePtr bootHash)
+ virHashTablePtr bootHash ATTRIBUTE_UNUSED)
{
size_t i;
@@ -4953,20 +5022,6 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
return -1;
}
- if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && bootHash) {
- if (def->os.nBootDevs > 0 && virHashSize(bootHash) > 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("per-device boot elements cannot be used"
- " together with os/boot elements"));
- return -1;
- }
-
- if (def->os.nBootDevs == 0 && virHashSize(bootHash) == 0) {
- def->os.nBootDevs = 1;
- def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK;
- }
- }
-
if (virDomainVcpuDefPostParse(def) < 0)
return -1;
@@ -4979,6 +5034,10 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
if (virDomainDefRejectDuplicatePanics(def) < 0)
return -1;
+ if (!(data->xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER)
&&
+ virDomainDefCheckBootOrder(def) < 0)
+ return -1;
+
if (virDomainDefPostParseTimer(def) < 0)
return -1;
diff --git a/tests/qemuargv2xmldata/nomachine-aarch64.xml
b/tests/qemuargv2xmldata/nomachine-aarch64.xml
index eb8f9db803..9492423389 100644
--- a/tests/qemuargv2xmldata/nomachine-aarch64.xml
+++ b/tests/qemuargv2xmldata/nomachine-aarch64.xml
@@ -6,6 +6,7 @@
<vcpu placement='static'>1</vcpu>
<os>
<type arch='aarch64' machine='virt'>hvm</type>
+ <boot dev='hd'/>
</os>
<features>
<gic version='2'/>
diff --git a/tests/qemuargv2xmldata/nomachine-ppc64.xml
b/tests/qemuargv2xmldata/nomachine-ppc64.xml
index 439f9e9ac6..1f15a950e3 100644
--- a/tests/qemuargv2xmldata/nomachine-ppc64.xml
+++ b/tests/qemuargv2xmldata/nomachine-ppc64.xml
@@ -6,6 +6,7 @@
<vcpu placement='static'>1</vcpu>
<os>
<type arch='ppc64' machine='pseries'>hvm</type>
+ <boot dev='hd'/>
</os>
<clock offset='utc'/>
<on_poweroff>destroy</on_poweroff>
diff --git a/tests/qemuargv2xmldata/nomachine-x86_64.xml
b/tests/qemuargv2xmldata/nomachine-x86_64.xml
index 71a36f0833..33cde4c55a 100644
--- a/tests/qemuargv2xmldata/nomachine-x86_64.xml
+++ b/tests/qemuargv2xmldata/nomachine-x86_64.xml
@@ -6,6 +6,7 @@
<vcpu placement='static'>1</vcpu>
<os>
<type arch='x86_64' machine='pc-0.11'>hvm</type>
+ <boot dev='hd'/>
</os>
<features>
<acpi/>
diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml
b/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml
index afb9030681..a3d54ae3c1 100644
--- a/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml
+++ b/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml
@@ -10,6 +10,7 @@
<kernel>/var/lib/xen/vmlinuz.2Dn2YT</kernel>
<initrd>/var/lib/xen/initrd.img.0u-Vhq</initrd>
<cmdline>
method=http://download.fedora.devel.redhat.com/pub/fedora/linux/core/test...
</cmdline>
+ <boot dev='hd'/>
</os>
<clock offset='variable' adjustment='0' basis='utc'/>
<on_poweroff>destroy</on_poweroff>
--
2.16.1
2:26 a.m.
New subject: [libvirt] [PATCH 2/4] conf: introduce virDomainDefCheckBootOrder
On Mon, May 28, 2018 at 15:54:03 +0200, Ján Tomko wrote:
Move the check for boot elements into a separate function
and remove its dependency on the parser-supplied bootHash table.
Reconstructing the hash table from the domain definition
effectively duplicates the check for duplicate boot order
values, also present in virDomainDeviceBootParseXML.
So the semantical difference is that places that call into the
post-parse infrastructure which construct the domain definition object
by other means that parsing XML will get the same treatment as when XML
is parsed.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/conf/domain_conf.c | 89 +++++++++++++++++++++++-----
tests/qemuargv2xmldata/nomachine-aarch64.xml | 1 +
tests/qemuargv2xmldata/nomachine-ppc64.xml | 1 +
tests/qemuargv2xmldata/nomachine-x86_64.xml | 1 +
tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml | 1 +
5 files changed, 78 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d6ac47c629..f087a3680f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4939,10 +4939,79 @@ virDomainDefPostParseCPU(virDomainDefPtr def)
[...]
+
+
+static int
+virDomainDefCheckBootOrder(virDomainDefPtr def)
[1]
+{
+ virHashTablePtr bootHash = NULL;
+ int ret = -1;
+
+ if (def->os.type != VIR_DOMAIN_OSTYPE_HVM)
+ return 0;
Please do this check outside of this function. If this function is
invoked it should o it's job, especially since you also have other
conditions when to invoke it outside.
+
+ if (!(bootHash = virHashCreate(5, NULL)))
+ goto cleanup;
+
+ if (virDomainDeviceInfoIterate(def, virDomainDefCollectBootOrder, bootHash) < 0)
+ goto cleanup;
+
+ if (def->os.nBootDevs > 0 && virHashSize(bootHash) > 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("per-device boot elements cannot be used"
+ " together with os/boot elements"));
+ goto cleanup;
+ }
+
+ if (def->os.nBootDevs == 0 && virHashSize(bootHash) == 0) {
+ def->os.nBootDevs = 1;
+ def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK;
[1] this in contrast to the name of the function modifies stuff ...
+ }
+
+ ret = 0;
+
+ cleanup:
+ virHashFree(bootHash);
+ return ret;
+}
+
+
[...]
@@ -4979,6 +5034,10 @@ virDomainDefPostParseCommon(virDomainDefPtr
def,
if (virDomainDefRejectDuplicatePanics(def) < 0)
return -1;
+ if (!(data->xmlopt->config.features &
VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER) &&
+ virDomainDefCheckBootOrder(def) < 0)
Add the condition for checking HVM here.
+ return -1;
+
if (virDomainDefPostParseTimer(def) < 0)
return -1;
[...]
diff --git a/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml
b/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml
index afb9030681..a3d54ae3c1 100644
--- a/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml
+++ b/tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml
@@ -10,6 +10,7 @@
<kernel>/var/lib/xen/vmlinuz.2Dn2YT</kernel>
<initrd>/var/lib/xen/initrd.img.0u-Vhq</initrd>
<cmdline>
method=http://download.fedora.devel.redhat.com/pub/fedora/linux/core/test...
</cmdline>
+ <boot dev='hd'/>
This looks wrong here since we do have the 'kernel' and 'initrd'
elements. Given that it's caused by the semantic difference I think it's
okay.
</os>
<clock offset='variable' adjustment='0' basis='utc'/>
<on_poweroff>destroy</on_poweroff>
ACK if you move out the condition and note the semantic impact in the
commit message.
2:55 a.m.
New subject: [libvirt] [PATCH 2/4] conf: introduce virDomainDefCheckBootOrder
On Tue, May 29, 2018 at 09:26:51AM +0200, Peter Krempa wrote:
On Mon, May 28, 2018 at 15:54:03 +0200, Ján Tomko wrote:
> Move the check for boot elements into a separate function
> and remove its dependency on the parser-supplied bootHash table.
>
> Reconstructing the hash table from the domain definition
> effectively duplicates the check for duplicate boot order
> values, also present in virDomainDeviceBootParseXML.
How about:
Now it will also be run on domains created by other means than XML
parsing, since it will be run even for code paths that did not supply
the bootHash table before.
So the semantical difference is that places that call into the
post-parse infrastructure which construct the domain definition object
by other means that parsing XML will get the same treatment as when XML
is parsed.
>
> Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> ---
> src/conf/domain_conf.c | 89 +++++++++++++++++++++++-----
> tests/qemuargv2xmldata/nomachine-aarch64.xml | 1 +
> tests/qemuargv2xmldata/nomachine-ppc64.xml | 1 +
> tests/qemuargv2xmldata/nomachine-x86_64.xml | 1 +
> tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml | 1 +
> 5 files changed, 78 insertions(+), 15 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d6ac47c629..f087a3680f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4939,10 +4939,79 @@ virDomainDefPostParseCPU(virDomainDefPtr def)
[...]
> +
> +
> +static int
> +virDomainDefCheckBootOrder(virDomainDefPtr def)
[1]
> +{
> + virHashTablePtr bootHash = NULL;
> + int ret = -1;
> +
> + if (def->os.type != VIR_DOMAIN_OSTYPE_HVM)
> + return 0;
Please do this check outside of this function. If this function is
invoked it should o it's job, especially since you also have other
conditions when to invoke it outside.
> +
> + if (!(bootHash = virHashCreate(5, NULL)))
> + goto cleanup;
> +
> + if (virDomainDeviceInfoIterate(def, virDomainDefCollectBootOrder, bootHash) <
0)
> + goto cleanup;
> +
> + if (def->os.nBootDevs > 0 && virHashSize(bootHash) > 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("per-device boot elements cannot be used"
> + " together with os/boot elements"));
> + goto cleanup;
> + }
> +
> + if (def->os.nBootDevs == 0 && virHashSize(bootHash) == 0) {
> + def->os.nBootDevs = 1;
> + def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK;
[1] this in contrast to the name of the function modifies stuff ...
I can rename it to virDomainDefBootOrderPostParse
Jano
3:27 a.m.
New subject: [libvirt] [PATCH 2/4] conf: introduce virDomainDefCheckBootOrder
On Tue, May 29, 2018 at 09:55:18 +0200, Ján Tomko wrote:
On Tue, May 29, 2018 at 09:26:51AM +0200, Peter Krempa wrote:
> On Mon, May 28, 2018 at 15:54:03 +0200, Ján Tomko wrote:
> > Move the check for boot elements into a separate function
> > and remove its dependency on the parser-supplied bootHash table.
> >
> > Reconstructing the hash table from the domain definition
> > effectively duplicates the check for duplicate boot order
> > values, also present in virDomainDeviceBootParseXML.
>
How about:
Now it will also be run on domains created by other means than XML
parsing, since it will be run even for code paths that did not supply
the bootHash table before.
Sounds goog to me.
> So the semantical difference is that places that call into the
> post-parse infrastructure which construct the domain definition object
> by other means that parsing XML will get the same treatment as when XML
> is parsed.
>
> >
> > Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> > ---
> > src/conf/domain_conf.c | 89
+++++++++++++++++++++++-----
> > tests/qemuargv2xmldata/nomachine-aarch64.xml | 1 +
> > tests/qemuargv2xmldata/nomachine-ppc64.xml | 1 +
> > tests/qemuargv2xmldata/nomachine-x86_64.xml | 1 +
> > tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml | 1 +
> > 5 files changed, 78 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index d6ac47c629..f087a3680f 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -4939,10 +4939,79 @@ virDomainDefPostParseCPU(virDomainDefPtr def)
>
> [...]
>
> > +
> > +
> > +static int
> > +virDomainDefCheckBootOrder(virDomainDefPtr def)
>
> [1]
>
> > +{
> > + virHashTablePtr bootHash = NULL;
> > + int ret = -1;
> > +
> > + if (def->os.type != VIR_DOMAIN_OSTYPE_HVM)
> > + return 0;
>
> Please do this check outside of this function. If this function is
> invoked it should o it's job, especially since you also have other
> conditions when to invoke it outside.
>
> > +
> > + if (!(bootHash = virHashCreate(5, NULL)))
> > + goto cleanup;
> > +
> > + if (virDomainDeviceInfoIterate(def, virDomainDefCollectBootOrder,
bootHash) < 0)
> > + goto cleanup;
> > +
> > + if (def->os.nBootDevs > 0 && virHashSize(bootHash) > 0)
{
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > + _("per-device boot elements cannot be used"
> > + " together with os/boot elements"));
> > + goto cleanup;
> > + }
> > +
> > + if (def->os.nBootDevs == 0 && virHashSize(bootHash) == 0) {
> > + def->os.nBootDevs = 1;
> > + def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK;
>
> [1] this in contrast to the name of the function modifies stuff ...
I can rename it to virDomainDefBootOrderPostParse
ACK to that
Jano
8:54 a.m.
New subject: [libvirt] [PATCH 3/4] conf: remove 'bootHash' from the post-parse infrastructure
From: Peter Krempa <pkrempa(a)redhat.com>
As the function signature of virDomainDefPostParseInternal does not
differ from virDomainDefPostParse now, the wrapper can be dropped.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/conf/domain_conf.c | 33 +++++++++------------------------
1 file changed, 9 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f087a3680f..6076424bc6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5010,8 +5010,7 @@ virDomainDefCheckBootOrder(virDomainDefPtr def)
static int
virDomainDefPostParseCommon(virDomainDefPtr def,
- struct virDomainDefPostParseDeviceIteratorData *data,
- virHashTablePtr bootHash ATTRIBUTE_UNUSED)
+ struct virDomainDefPostParseDeviceIteratorData *data)
{
size_t i;
@@ -5118,13 +5117,12 @@ virDomainDefPostParseCheckFailure(virDomainDefPtr def,
}
-static int
-virDomainDefPostParseInternal(virDomainDefPtr def,
- virCapsPtr caps,
- unsigned int parseFlags,
- virDomainXMLOptionPtr xmlopt,
- void *parseOpaque,
- virHashTablePtr bootHash)
+int
+virDomainDefPostParse(virDomainDefPtr def,
+ virCapsPtr caps,
+ unsigned int parseFlags,
+ virDomainXMLOptionPtr xmlopt,
+ void *parseOpaque)
{
int ret = -1;
bool localParseOpaque = false;
@@ -5180,7 +5178,7 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
if (virDomainDefPostParseCheckFailure(def, parseFlags, ret) < 0)
goto cleanup;
- if ((ret = virDomainDefPostParseCommon(def, &data, bootHash)) < 0)
+ if ((ret = virDomainDefPostParseCommon(def, &data)) < 0)
goto cleanup;
if (xmlopt->config.assignAddressesCallback) {
@@ -5207,18 +5205,6 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
}
-int
-virDomainDefPostParse(virDomainDefPtr def,
- virCapsPtr caps,
- unsigned int parseFlags,
- virDomainXMLOptionPtr xmlopt,
- void *parseOpaque)
-{
- return virDomainDefPostParseInternal(def, caps, parseFlags, xmlopt,
- parseOpaque, NULL);
-}
-
-
/**
* virDomainDiskAddressDiskBusCompatibility:
* @bus: disk bus type
@@ -20562,8 +20548,7 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
/* callback to fill driver specific domain aspects */
- if (virDomainDefPostParseInternal(def, caps, flags, xmlopt, parseOpaque,
- bootHash) < 0)
+ if (virDomainDefPostParse(def, caps, flags, xmlopt, parseOpaque) < 0)
goto error;
/* valdiate configuration */
--
2.16.1
2:27 a.m.
New subject: [libvirt] [PATCH 3/4] conf: remove 'bootHash' from the post-parse infrastructure
On Mon, May 28, 2018 at 15:54:04 +0200, Ján Tomko wrote:
From: Peter Krempa <pkrempa(a)redhat.com>
As the function signature of virDomainDefPostParseInternal does not
differ from virDomainDefPostParse now, the wrapper can be dropped.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/conf/domain_conf.c | 33 +++++++++------------------------
1 file changed, 9 insertions(+), 24 deletions(-)
Implicit ACK. Also please push the rest of the patches from my series
which depend on this when you'll be pushing this.
3:43 a.m.
New subject: [libvirt] [PATCH 3/4] conf: remove 'bootHash' from the post-parse infrastructure
On Tue, May 29, 2018 at 09:27:53AM +0200, Peter Krempa wrote:
On Mon, May 28, 2018 at 15:54:04 +0200, Ján Tomko wrote:
> From: Peter Krempa <pkrempa(a)redhat.com>
>
> As the function signature of virDomainDefPostParseInternal does not
> differ from virDomainDefPostParse now, the wrapper can be dropped.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
> ---
> src/conf/domain_conf.c | 33 +++++++++------------------------
> 1 file changed, 9 insertions(+), 24 deletions(-)
Implicit ACK. Also please push the rest of the patches from my series
which depend on this when you'll be pushing this.
Done.
Jano
8:54 a.m.
New subject: [libvirt] [PATCH 4/4] conf: remove 'bootHash' completely
Its only use is now to check for duplicate boot order values,
which is now also done in virDomainDefPostParseCommon.
Remove it completely.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/conf/domain_conf.c | 89 ++++++++++++++++++--------------------------------
1 file changed, 31 insertions(+), 58 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6076424bc6..c150b7d7d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6587,8 +6587,7 @@ virDomainDeviceUSBMasterParseXML(xmlNodePtr node,
static int
virDomainDeviceBootParseXML(xmlNodePtr node,
- virDomainDeviceInfoPtr info,
- virHashTablePtr bootHash)
+ virDomainDeviceInfoPtr info)
{
char *order;
char *loadparm = NULL;
@@ -6608,18 +6607,6 @@ virDomainDeviceBootParseXML(xmlNodePtr node,
goto cleanup;
}
- if (bootHash) {
- if (virHashLookup(bootHash, order)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("boot order '%s' used for more than one
device"),
- order);
- goto cleanup;
- }
-
- if (virHashAddEntry(bootHash, order, (void *) 1) < 0)
- goto cleanup;
- }
-
loadparm = virXMLPropString(node, "loadparm");
if (loadparm) {
if (virStringToUpper(&info->loadparm, loadparm) != 1) {
@@ -6817,7 +6804,6 @@ virDomainDeviceAliasIsUserAlias(const char *aliasStr)
static int
virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
xmlNodePtr node,
- virHashTablePtr bootHash,
virDomainDeviceInfoPtr info,
unsigned int flags)
{
@@ -6877,7 +6863,7 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt
ATTRIBUTE_UNUSED,
}
if (boot) {
- if (virDomainDeviceBootParseXML(boot, info, bootHash))
+ if (virDomainDeviceBootParseXML(boot, info))
goto cleanup;
}
@@ -9402,7 +9388,6 @@ static virDomainDiskDefPtr
virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlNodePtr node,
xmlXPathContextPtr ctxt,
- virHashTablePtr bootHash,
virSecurityLabelDefPtr* vmSeclabels,
int nvmSeclabels,
unsigned int flags)
@@ -9769,7 +9754,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
}
def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
} else {
- if (virDomainDeviceInfoParseXML(xmlopt, node, bootHash, &def->info,
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info,
flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT) < 0)
goto error;
}
@@ -10260,7 +10245,7 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt,
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
def->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
VIR_DEBUG("Ignoring device address for none model usb controller");
- } else if (virDomainDeviceInfoParseXML(xmlopt, node, NULL,
+ } else if (virDomainDeviceInfoParseXML(xmlopt, node,
&def->info, flags) < 0) {
goto error;
}
@@ -10648,7 +10633,7 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
def->dst = target;
target = NULL;
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
cleanup:
@@ -10938,7 +10923,6 @@ static virDomainNetDefPtr
virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlNodePtr node,
xmlXPathContextPtr ctxt,
- virHashTablePtr bootHash,
char *prefix,
unsigned int flags)
{
@@ -11226,7 +11210,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
} else {
- if (virDomainDeviceInfoParseXML(xmlopt, node, bootHash, &def->info,
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info,
flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT
| VIR_DOMAIN_DEF_PARSE_ALLOW_ROM) < 0)
goto error;
@@ -12525,7 +12509,7 @@ virDomainChrDefParseXML(virDomainXMLOptionPtr xmlopt,
if (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD) {
VIR_DEBUG("Ignoring device address for gustfwd channel");
- } else if (virDomainDeviceInfoParseXML(xmlopt, node, NULL,
+ } else if (virDomainDeviceInfoParseXML(xmlopt, node,
&def->info, flags) < 0) {
goto error;
}
@@ -12663,7 +12647,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID) {
@@ -12764,7 +12748,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
cleanup:
@@ -12793,7 +12777,7 @@ virDomainPanicDefParseXML(virDomainXMLOptionPtr xmlopt,
if (VIR_ALLOC(panic) < 0)
return NULL;
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL,
+ if (virDomainDeviceInfoParseXML(xmlopt, node,
&panic->info, flags) < 0)
goto error;
@@ -12929,7 +12913,7 @@ virDomainInputDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
if (def->bus == VIR_DOMAIN_INPUT_BUS_USB &&
@@ -12993,7 +12977,7 @@ virDomainHubDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
cleanup:
@@ -14150,7 +14134,7 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
cleanup:
@@ -14204,7 +14188,7 @@ virDomainWatchdogDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
cleanup:
@@ -14316,7 +14300,7 @@ virDomainRNGDefParseXML(virDomainXMLOptionPtr xmlopt,
break;
}
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
if (virDomainVirtioOptionsParseXML(virXPathNode("./driver", ctxt),
@@ -14386,7 +14370,7 @@ virDomainMemballoonDefParseXML(virDomainXMLOptionPtr xmlopt,
if (def->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)
VIR_DEBUG("Ignoring device address for none model Memballoon");
- else if (virDomainDeviceInfoParseXML(xmlopt, node, NULL,
+ else if (virDomainDeviceInfoParseXML(xmlopt, node,
&def->info, flags) < 0)
goto error;
@@ -14417,7 +14401,7 @@ virDomainNVRAMDefParseXML(virDomainXMLOptionPtr xmlopt,
if (VIR_ALLOC(def) < 0)
return NULL;
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
return def;
@@ -14513,7 +14497,7 @@ virDomainShmemDefParseXML(virDomainXMLOptionPtr xmlopt,
goto cleanup;
}
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto cleanup;
@@ -15167,7 +15151,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
- if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &def->info, flags) <
0)
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
def->driver = virDomainVideoDriverDefParseXML(node);
@@ -15194,7 +15178,6 @@ static virDomainHostdevDefPtr
virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlNodePtr node,
xmlXPathContextPtr ctxt,
- virHashTablePtr bootHash,
unsigned int flags)
{
virDomainHostdevDefPtr def;
@@ -15235,7 +15218,7 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
}
if (def->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
- if (virDomainDeviceInfoParseXML(xmlopt, node, bootHash, def->info,
+ if (virDomainDeviceInfoParseXML(xmlopt, node, def->info,
flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT
| VIR_DOMAIN_DEF_PARSE_ALLOW_ROM) < 0)
goto error;
@@ -15275,7 +15258,6 @@ static virDomainRedirdevDefPtr
virDomainRedirdevDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlNodePtr node,
xmlXPathContextPtr ctxt,
- virHashTablePtr bootHash,
unsigned int flags)
{
xmlNodePtr cur;
@@ -15322,7 +15304,7 @@ virDomainRedirdevDefParseXML(virDomainXMLOptionPtr xmlopt,
if (def->source->type == VIR_DOMAIN_CHR_TYPE_SPICEVMC)
def->source->data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_USBREDIR;
- if (virDomainDeviceInfoParseXML(xmlopt, node, bootHash, &def->info,
+ if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info,
flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT) < 0)
goto error;
@@ -15801,7 +15783,7 @@ virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
if (virDomainDeviceInfoParseXML(xmlopt, memdevNode,
- NULL, &def->info, flags) < 0)
+ &def->info, flags) < 0)
goto error;
ctxt->node = save;
@@ -15931,7 +15913,7 @@ virDomainDeviceDefParse(const char *xmlStr,
switch ((virDomainDeviceType) dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
if (!(dev->data.disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
- NULL, def->seclabels,
+ def->seclabels,
def->nseclabels,
flags)))
goto error;
@@ -15947,7 +15929,7 @@ virDomainDeviceDefParse(const char *xmlStr,
case VIR_DOMAIN_DEVICE_NET:
netprefix = caps->host.netprefix;
if (!(dev->data.net = virDomainNetDefParseXML(xmlopt, node, ctxt,
- NULL, netprefix, flags)))
+ netprefix, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_INPUT:
@@ -15972,7 +15954,7 @@ virDomainDeviceDefParse(const char *xmlStr,
break;
case VIR_DOMAIN_DEVICE_HOSTDEV:
if (!(dev->data.hostdev = virDomainHostdevDefParseXML(xmlopt, node,
- ctxt, NULL,
+ ctxt,
flags)))
goto error;
break;
@@ -15991,7 +15973,7 @@ virDomainDeviceDefParse(const char *xmlStr,
break;
case VIR_DOMAIN_DEVICE_REDIRDEV:
if (!(dev->data.redirdev = virDomainRedirdevDefParseXML(xmlopt, node,
- ctxt, NULL, flags)))
+ ctxt, flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_RNG:
@@ -16098,7 +16080,7 @@ virDomainDiskDefParse(const char *xmlStr,
}
disk = virDomainDiskDefParseXML(xmlopt, ctxt->node, ctxt,
- NULL, seclabels, nseclabels, flags);
+ seclabels, nseclabels, flags);
cleanup:
xmlFreeDoc(xml);
@@ -18478,8 +18460,7 @@ virDomainVcpuParse(virDomainDefPtr def,
static int
virDomainDefParseBootOptions(virDomainDefPtr def,
- xmlXPathContextPtr ctxt,
- virHashTablePtr *bootHash)
+ xmlXPathContextPtr ctxt)
{
xmlNodePtr *nodes = NULL;
char *tmp = NULL;
@@ -18611,8 +18592,6 @@ virDomainDefParseBootOptions(virDomainDefPtr def,
if (virDomainDefParseBootXML(ctxt, def) < 0)
goto error;
- if (!(*bootHash = virHashCreate(5, NULL)))
- goto error;
}
ret = 0;
@@ -18829,7 +18808,6 @@ virDomainDefParseXML(xmlDocPtr xml,
long id = -1;
virDomainDefPtr def;
bool uuid_generated = false;
- virHashTablePtr bootHash = NULL;
bool usb_none = false;
bool usb_other = false;
bool usb_master = false;
@@ -19874,7 +19852,7 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(nodes);
- if (virDomainDefParseBootOptions(def, ctxt, &bootHash) < 0)
+ if (virDomainDefParseBootOptions(def, ctxt) < 0)
goto error;
/* analysis of the disk devices */
@@ -19888,7 +19866,6 @@ virDomainDefParseXML(xmlDocPtr xml,
virDomainDiskDefPtr disk = virDomainDiskDefParseXML(xmlopt,
nodes[i],
ctxt,
- bootHash,
def->seclabels,
def->nseclabels,
flags);
@@ -19995,7 +19972,6 @@ virDomainDefParseXML(xmlDocPtr xml,
virDomainNetDefPtr net = virDomainNetDefParseXML(xmlopt,
nodes[i],
ctxt,
- bootHash,
netprefix,
flags);
if (!net)
@@ -20237,7 +20213,7 @@ virDomainDefParseXML(xmlDocPtr xml,
virDomainHostdevDefPtr hostdev;
hostdev = virDomainHostdevDefParseXML(xmlopt, nodes[i], ctxt,
- bootHash, flags);
+ flags);
if (!hostdev)
goto error;
@@ -20381,7 +20357,7 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
for (i = 0; i < n; i++) {
virDomainRedirdevDefPtr redirdev =
- virDomainRedirdevDefParseXML(xmlopt, nodes[i], ctxt, bootHash, flags);
+ virDomainRedirdevDefParseXML(xmlopt, nodes[i], ctxt, flags);
if (!redirdev)
goto error;
@@ -20555,14 +20531,11 @@ virDomainDefParseXML(xmlDocPtr xml,
if (virDomainDefValidate(def, caps, flags, xmlopt) < 0)
goto error;
- virHashFree(bootHash);
-
return def;
error:
VIR_FREE(tmp);
VIR_FREE(nodes);
- virHashFree(bootHash);
virDomainDefFree(def);
return NULL;
}
--
2.16.1
2:28 a.m.
New subject: [libvirt] [PATCH 4/4] conf: remove 'bootHash' completely
On Mon, May 28, 2018 at 15:54:05 +0200, Ján Tomko wrote:
Its only use is now to check for duplicate boot order values,
which is now also done in virDomainDefPostParseCommon.
Remove it completely.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/conf/domain_conf.c | 89 ++++++++++++++++++--------------------------------
1 file changed, 31 insertions(+), 58 deletions(-)
ACK
2370
days inactive
2371
days old
13 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa