5:27 p.m.
On 02/27/19 11:04, Michal Privoznik wrote:
This is going to extend virDomainLoader enum. The reason is that
once loader path is NULL its type makes no sense. However, since
value of zero corresponds to VIR_DOMAIN_LOADER_TYPE_ROM the
following XML would be produced:
<os>
<loader type='rom'/>
...
</os>
To solve this, introduce VIR_DOMAIN_LOADER_TYPE_NONE which would
correspond to value of zero and then use post parse callback to
set the default loader type to 'rom' if needed.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 23 +++++++++++++++++++++--
src/conf/domain_conf.h | 3 ++-
src/qemu/qemu_command.c | 1 +
src/qemu/qemu_domain.c | 1 +
tests/domaincapsschemadata/full.xml | 1 +
5 files changed, 26 insertions(+), 3 deletions(-)
Sounds pretty complex, but I guess it makes sense. If both @type and the
pathname content are missing, then @type will default to NONE. (This
used not to be possible, given that pathname used to be required.) If
@type is absent but the pathname is present, then we flip the type
manually to ROM.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f622a4dddf..b436b91c66 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1013,6 +1013,7 @@ VIR_ENUM_IMPL(virDomainMemoryAllocation,
VIR_DOMAIN_MEMORY_ALLOCATION_LAST,
VIR_ENUM_IMPL(virDomainLoader,
VIR_DOMAIN_LOADER_TYPE_LAST,
+ "none",
"rom",
"pflash",
);
@@ -4286,6 +4287,20 @@ virDomainDefPostParseMemory(virDomainDefPtr def,
}
+static void
+virDomainDefPostParseOs(virDomainDefPtr def)
+{
+ if (!def->os.loader)
+ return;
+
+ if (def->os.loader->path &&
+ def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_NONE) {
+ /* By default, loader is type of 'rom' */
+ def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_ROM;
+ }
+}
+
+
static void
virDomainDefPostParseMemtune(virDomainDefPtr def)
{
@@ -5465,6 +5480,8 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
if (virDomainDefPostParseMemory(def, data->parseFlags) < 0)
return -1;
+ virDomainDefPostParseOs(def);
+
virDomainDefPostParseMemtune(def);
if (virDomainDefRejectDuplicateControllers(def) < 0)
@@ -18706,7 +18723,7 @@ virDomainLoaderDefParseXML(xmlNodePtr node,
if (type_str) {
int type;
- if ((type = virDomainLoaderTypeFromString(type_str)) < 0) {
+ if ((type = virDomainLoaderTypeFromString(type_str)) <= 0) {
Why is this change necessary? Hm... I assume, due to the enum
auto-generation in libvirt, the introduction of "none" will
automatically cause virDomainLoaderTypeFromString() to recognize "none"
as value 0. But we want to act like "none" isn't a valid value (it's
internal use only). Hence the same error message as before.
I reckon this is OK.
Reviewed-by: Laszlo Ersek <lersek(a)redhat.com>
Thanks
Laszlo
virReportError(VIR_ERR_XML_DETAIL,
_("unknown type value: %s"), type_str);
goto cleanup;
@@ -27611,12 +27628,14 @@ virDomainLoaderDefFormat(virBufferPtr buf,
if (loader->secure)
virBufferAsprintf(buf, " secure='%s'", secure);
- virBufferAsprintf(buf, " type='%s'", type);
+ if (loader->type != VIR_DOMAIN_LOADER_TYPE_NONE)
+ virBufferAsprintf(buf, " type='%s'", type);
if (loader->path)
virBufferEscapeString(buf, ">%s</loader>\n",
loader->path);
else
virBufferAddLit(buf, "/>\n");
+
if (loader->nvram || loader->templt) {
virBufferAddLit(buf, "<nvram");
virBufferEscapeString(buf, " template='%s'",
loader->templt);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1f8454b38c..4e8c02b5e3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1899,7 +1899,8 @@ struct _virDomainBIOSDef {
};
typedef enum {
- VIR_DOMAIN_LOADER_TYPE_ROM = 0,
+ VIR_DOMAIN_LOADER_TYPE_NONE = 0,
+ VIR_DOMAIN_LOADER_TYPE_ROM,
VIR_DOMAIN_LOADER_TYPE_PFLASH,
VIR_DOMAIN_LOADER_TYPE_LAST
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 74f34af292..92729485ff 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9846,6 +9846,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
}
break;
+ case VIR_DOMAIN_LOADER_TYPE_NONE:
case VIR_DOMAIN_LOADER_TYPE_LAST:
/* nada */
break;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index cf7e650b34..cc3a01397c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -12201,6 +12201,7 @@ qemuDomainSetupLoader(virQEMUDriverConfigPtr cfg
ATTRIBUTE_UNUSED,
goto cleanup;
break;
+ case VIR_DOMAIN_LOADER_TYPE_NONE:
case VIR_DOMAIN_LOADER_TYPE_LAST:
break;
}
diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml
index eafba1ae5b..0a46e6bb78 100644
--- a/tests/domaincapsschemadata/full.xml
+++ b/tests/domaincapsschemadata/full.xml
@@ -10,6 +10,7 @@
<value>/foo/bar</value>
<value>/tmp/my_path</value>
<enum name='type'>
+ <value>none</value>
<value>rom</value>
<value>pflash</value>
</enum>