Augment definition to include virStorageSourcePtr that
more comprehensively describes the nature of backing element.
Also include flags for annotating if input XML definition is
old-style or new-style.
2) Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef.
This patch is used to interpret domain XML and store the Loader &
Nvram's backing definitions as virStorageSource.
It also identifies if input XML used old or new-style declaration.
(This will later be used by the formatter).
3) Format the loader source appropriately.
If the initial XML used the old-style declaration as follows:
<loader type='pflash'>/path/to/file</loader>
we format it as was read.
However, if it used new-style declaration:
<loader type='pflash' backing='file'>
<source file='path/to/file'/>
</loader>
The formatter identifies that this is a new-style format
and renders it likewise.
4) Plumb the loader source into generation of QEMU command line.
Given that nvram & loader elements can now be backed by a non-local
source too, adjust all steps leading to generation of
QEMU command line.
5) Fix the domain def inference logic to correctly account for network-backed
pflash devices.
6) Bhyve: Fix command line generation to correctly pick up local loader path.
7) virt-aa-helper: Adjust references to loader & nvram elements to correctly
parse the virStorageSource types.
8) Vbox: Adjust references to 'loader' and 'nvram' elements given that
these are now represented by virStorageSourcePtr.
9)Xen: Adjust all references to loader & nvram elements given that they are now backed by virStorageSourcePtr
---
src/bhyve/bhyve_command.c | 6 +-
src/conf/domain_conf.c | 250 +++++++++++++++++++++++++++++++++++++---
src/conf/domain_conf.h | 11 +-
src/qemu/qemu_cgroup.c | 13 ++-
src/qemu/qemu_command.c | 21 +++-
src/qemu/qemu_domain.c | 31 +++--
src/qemu/qemu_driver.c | 7 +-
src/qemu/qemu_parse_command.c | 30 ++++-
src/qemu/qemu_process.c | 54 ++++++---
src/security/security_dac.c | 6 +-
src/security/security_selinux.c | 6 +-
src/security/virt-aa-helper.c | 14 ++-
src/vbox/vbox_common.c | 11 +-
src/xenapi/xenapi_driver.c | 4 +-
src/xenconfig/xen_sxpr.c | 19 +--
src/xenconfig/xen_xm.c | 9 +-
16 files changed, 409 insertions(+), 83 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index e3f7ded..9e53f40 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -521,10 +521,12 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn,
virCommandAddArgList(cmd, "-s", "0:0,hostbridge", NULL);
if (def->os.bootloader == NULL &&
- def->os.loader) {
+ def->os.loader &&
+ def->os.loader->src &&
+ def->os.loader->src->type == VIR_STORAGE_TYPE_FILE) {
if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) {
virCommandAddArg(cmd, "-l");
- virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path);
+ virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->src->path);
add_lpc = true;
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3689ac0..df2ed3a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2884,8 +2884,8 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader)
if (!loader)
return;
- VIR_FREE(loader->path);
- VIR_FREE(loader->nvram);
+ virStorageSourceFree(loader->src);
+ virStorageSourceFree(loader->nvram);
VIR_FREE(loader->templt);
VIR_FREE(loader);
}
@@ -18087,30 +18087,81 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)
static int
virDomainLoaderDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
virDomainLoaderDefPtr loader)
{
int ret = -1;
char *readonly_str = NULL;
char *secure_str = NULL;
char *type_str = NULL;
+ char *tmp = NULL;
+ char *lpath = NULL;
+ xmlNodePtr cur;
+ xmlXPathContextPtr cur_ctxt = ctxt;
+
+ if (VIR_ALLOC(loader->src) < 0)
+ goto error;
+
+ loader->src->type = VIR_STORAGE_TYPE_LAST;
+ loader->oldStyleLoader = false;
readonly_str = virXMLPropString(node, "readonly");
secure_str = virXMLPropString(node, "secure");
type_str = virXMLPropString(node, "type");
- loader->path = (char *) xmlNodeGetContent(node);
+
+ if ((tmp = virXMLPropString(node, "backing")) &&
+ (loader->src->type = virStorageTypeFromString(tmp)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown loader type '%s'"), tmp);
+ VIR_FREE(tmp);
+ goto error;
+ }
+ VIR_FREE(tmp);
+
+ for (cur = node->children; cur != NULL; cur = cur->next) {
+ if (cur->type != XML_ELEMENT_NODE)
+ continue;
+
+ if (virXMLNodeNameEqual(cur, "source")) {
+ /* new-style declaration found */
+ if (virDomainStorageSourceParse(cur, cur_ctxt, loader->src, 0) < 0) {
+ virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("Error parsing Loader source element"));
+ goto error;
+ }
+ break;
+ }
+ }
+
+ /* Old-style absolute path found ? */
+ if (loader->src->type == VIR_STORAGE_TYPE_LAST) {
+ lpath = (char *) xmlNodeGetContent(node);
+ if (virStringIsEmpty(lpath)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing loader source"));
+ VIR_FREE(lpath);
+ goto error;
+ }
+
+ loader->src->type = VIR_STORAGE_TYPE_FILE;
+ if (VIR_STRDUP(loader->src->path, lpath) < 0)
+ goto error;
+ VIR_FREE(lpath);
+ loader->oldStyleLoader = true;
+ }
if (readonly_str &&
(loader->readonly = virTristateBoolTypeFromString(readonly_str)) <= 0) {
virReportError(VIR_ERR_XML_DETAIL,
_("unknown readonly value: %s"), readonly_str);
- goto cleanup;
+ goto error;
}
if (secure_str &&
(loader->secure = virTristateBoolTypeFromString(secure_str)) <= 0) {
virReportError(VIR_ERR_XML_DETAIL,
_("unknown secure value: %s"), secure_str);
- goto cleanup;
+ goto error;
}
if (type_str) {
@@ -18118,19 +18169,97 @@ virDomainLoaderDefParseXML(xmlNodePtr node,
if ((type = virDomainLoaderTypeFromString(type_str)) < 0) {
virReportError(VIR_ERR_XML_DETAIL,
_("unknown type value: %s"), type_str);
- goto cleanup;
+ goto error;
}
loader->type = type;
}
ret = 0;
- cleanup:
+
+ exit:
VIR_FREE(readonly_str);
VIR_FREE(secure_str);
VIR_FREE(type_str);
+
return ret;
+ error:
+ virStorageSourceFree(loader->src);
+ VIR_FREE(lpath);
+ loader->src = NULL;
+ goto exit;
}
+static int
+virDomainLoaderNvramDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virDomainLoaderDefPtr loader)
+{
+ int ret = -1;
+ char *tmp = NULL;
+ char *npath = NULL;
+ xmlNodePtr cur;
+
+ if (VIR_ALLOC(loader->nvram) < 0)
+ goto error;
+
+ loader->nvram->type = VIR_STORAGE_TYPE_LAST;
+ loader->oldStyleNvram = false;
+
+ if ((tmp = virXMLPropString(node, "backing")) &&
+ (loader->nvram->type = virStorageTypeFromString(tmp)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown nvram type '%s'"), tmp);
+ VIR_FREE(tmp);
+ goto error;
+ }
+ VIR_FREE(tmp);
+
+ loader->templt = virXMLPropString(node, "template");
+
+ for (cur = node->children; cur != NULL; cur = cur->next) {
+ if (cur->type != XML_ELEMENT_NODE)
+ continue;
+
+ if (virXMLNodeNameEqual(cur, "source")) {
+ if (virDomainStorageSourceParse(cur, ctxt, loader->nvram, 0) < 0) {
+ virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("Error parsing nvram source element"));
+ goto error;
+ }
+ ret = 0;
+ break;
+ }
+ }
+
+ /* Old-style declaration found */
+ if (loader->nvram->type == VIR_STORAGE_TYPE_LAST) {
+ npath = (char *) xmlNodeGetContent(node);
+
+ /* Suppress failures from lack of NVRAM,
+ * it will eventually be generated from template*/
+ if (virStringIsEmpty(npath)) {
+ virStorageSourceFree(loader->nvram);
+ loader->nvram = NULL;
+ VIR_FREE(npath);
+ return 0;
+ }
+
+ if (VIR_STRDUP(loader->nvram->path, npath) < 0)
+ goto error;
+ VIR_FREE(npath);
+ loader->nvram->type = VIR_STORAGE_TYPE_FILE;
+ loader->oldStyleNvram = true;
+ ret = 0;
+ }
+ return ret;
+
+ error:
+ virStorageSourceFree(loader->nvram);
+ loader->nvram = NULL;
+ VIR_FREE(npath);
+ VIR_FREE(loader->templt);
+ return ret;
+}
static virBitmapPtr
virDomainSchedulerParse(xmlNodePtr node,
@@ -18523,11 +18652,13 @@ virDomainDefParseBootOptions(virDomainDefPtr def,
if (VIR_ALLOC(def->os.loader) < 0)
goto error;
- if (virDomainLoaderDefParseXML(loader_node, def->os.loader) < 0)
+ if (virDomainLoaderDefParseXML(loader_node, ctxt, def->os.loader) < 0)
goto error;
- def->os.loader->nvram = virXPathString("string(./os/nvram[1])", ctxt);
- def->os.loader->templt = virXPathString("string(./os/nvram[1]/@template)", ctxt);
+ if ((loader_node = virXPathNode("./os/nvram[1]", ctxt)) &&
+ (virDomainLoaderNvramDefParseXML(loader_node, ctxt,
+ def->os.loader) < 0))
+ goto error;
}
}
@@ -26217,11 +26348,19 @@ virDomainHugepagesFormat(virBufferPtr buf,
static void
virDomainLoaderDefFormat(virBufferPtr buf,
- virDomainLoaderDefPtr loader)
+ virDomainLoaderDefPtr loader,
+ unsigned int flags)
{
const char *readonly = virTristateBoolTypeToString(loader->readonly);
const char *secure = virTristateBoolTypeToString(loader->secure);
const char *type = virDomainLoaderTypeToString(loader->type);
+ const char *backing = NULL;
+
+ virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
+ virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+
+ virBufferSetChildIndent(&childBuf, buf);
+
virBufferAddLit(buf, "<loader");
@@ -26231,17 +26370,90 @@ virDomainLoaderDefFormat(virBufferPtr buf,
if (loader->secure)
virBufferAsprintf(buf, " secure='%s'", secure);
- virBufferAsprintf(buf, " type='%s'>", type);
+ virBufferAsprintf(buf, " type='%s'", type);
+ if (loader->src &&
+ loader->src->type < VIR_STORAGE_TYPE_LAST) {
+ if (!loader->oldStyleLoader) {
+ /* Format this in the new style, using the
+ * <source> sub-element */
+
+ if (virDomainStorageSourceFormat(&attrBuf, &childBuf, loader->src,
+ flags, 0) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot format loader source"));
+ goto error;
+ }
+
+ backing = virStorageTypeToString(loader->src->type);
+ virBufferAsprintf(buf, " backing='%s'>", backing);
+ virBufferAdjustIndent(buf, 2);
+
+ if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot format loader source"));
+ goto error;
+ }
+
+ virBufferAdjustIndent(buf, -2);
+
+ } else {
+ /* Format this in the old-style, using absolute paths directly. */
+ virBufferAsprintf(buf, ">%s", loader->src->path);
+ }
+ } else {
+ virBufferAddLit(buf, ">\n");
+ }
+
+ virBufferAddLit(buf, "</loader>\n");
- virBufferEscapeString(buf, "%s</loader>\n", loader->path);
if (loader->nvram || loader->templt) {
+ ignore_value(virBufferContentAndReset(&attrBuf));
+ ignore_value(virBufferContentAndReset(&childBuf));
+ virBufferSetChildIndent(&childBuf, buf);
+
virBufferAddLit(buf, "<nvram");
- virBufferEscapeString(buf, " template='%s'", loader->templt);
- if (loader->nvram)
- virBufferEscapeString(buf, ">%s</nvram>\n", loader->nvram);
- else
- virBufferAddLit(buf, "/>\n");
+
+ if (loader->templt)
+ virBufferEscapeString(buf, " template='%s'", loader->templt);
+
+ if (loader->nvram) {
+ backing = virStorageTypeToString(loader->nvram->type);
+ if (!loader->oldStyleNvram) {
+
+ if (virDomainStorageSourceFormat(&attrBuf, &childBuf,
+ loader->nvram, flags, 0) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot format NVRAM source"));
+ virBufferAddLit(buf, "></nvram>");
+ goto error;
+ }
+
+ virBufferEscapeString(buf, " backing='%s'>\n", backing);
+ virBufferAdjustIndent(buf, 2);
+
+ if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Cannot format NVRAM source"));
+ virBufferAddLit(buf, "</nvram>");
+ goto error;
+ }
+
+ virBufferAdjustIndent(buf, -2);
+
+ } else {
+ /* old-style NVRAM declaration found */
+ virBufferAsprintf(buf, ">%s", loader->nvram->path);
+ }
+ } else {
+ virBufferAddLit(buf, ">\n");
+ }
+
+ virBufferAddLit(buf, "</nvram>\n");
}
+ error:
+ virBufferFreeAndReset(&attrBuf);
+ virBufferFreeAndReset(&childBuf);
+ return;
}
static void
@@ -26918,7 +27130,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAsprintf(buf, "<initgroup>%s</initgroup>\n", def->os.initgroup);
if (def->os.loader)
- virDomainLoaderDefFormat(buf, def->os.loader);
+ virDomainLoaderDefFormat(buf, def->os.loader, flags);
virBufferEscapeString(buf, "<kernel>%s</kernel>\n",
def->os.kernel);
virBufferEscapeString(buf, "<initrd>%s</initrd>\n",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a78fdee..fb44a90 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1856,15 +1856,22 @@ typedef enum {
VIR_ENUM_DECL(virDomainLoader)
+struct _virStorageSource;
+typedef struct _virStorageSource *virStorageSourcePtr;
+
typedef struct _virDomainLoaderDef virDomainLoaderDef;
typedef virDomainLoaderDef *virDomainLoaderDefPtr;
struct _virDomainLoaderDef {
- char *path;
+ virStorageSourcePtr src;
int readonly; /* enum virTristateBool */
virDomainLoader type;
int secure; /* enum virTristateBool */
- char *nvram; /* path to non-volatile RAM */
+ virStorageSourcePtr nvram; /* path to non-voliatile RAM */
char *templt; /* user override of path to master nvram */
+ bool oldStyleLoader; /* is this an old-style XML formatting,
+ * ie, absolute path is directly specified? */
+ bool oldStyleNvram; /* is this an old-style XML formatting,
+ * ie, absolute path is directly specified? */
};
void virDomainLoaderDefFree(virDomainLoaderDefPtr loader);
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 546a4c8..5c16a48 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -607,16 +607,21 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm)
static int
qemuSetupFirmwareCgroup(virDomainObjPtr vm)
{
+ virStorageSourcePtr src = NULL;
+
if (!vm->def->os.loader)
return 0;
- if (vm->def->os.loader->path &&
- qemuSetupImagePathCgroup(vm, vm->def->os.loader->path,
- vm->def->os.loader->readonly == VIR_TRISTATE_BOOL_YES) < 0)
+ src = vm->def->os.loader->src;
+
+ if (src->type == VIR_STORAGE_TYPE_FILE &&
+ qemuSetupImagePathCgroup(vm, src->path,
+ src->readonly == VIR_TRISTATE_BOOL_YES) < 0)
return -1;
if (vm->def->os.loader->nvram &&
- qemuSetupImagePathCgroup(vm, vm->def->os.loader->nvram, false) < 0)
+ vm->def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE &&
+ qemuSetupImagePathCgroup(vm, vm->def->os.loader->nvram->path, false) < 0)
return -1;
return 0;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9da2d60..ba5283f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9320,6 +9320,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
virDomainLoaderDefPtr loader = def->os.loader;
virBuffer buf = VIR_BUFFER_INITIALIZER;
int unit = 0;
+ char *source = NULL;
if (!loader)
return;
@@ -9327,7 +9328,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
switch ((virDomainLoader) loader->type) {
case VIR_DOMAIN_LOADER_TYPE_ROM:
virCommandAddArg(cmd, "-bios");
- virCommandAddArg(cmd, loader->path);
+ virCommandAddArg(cmd, loader->src->path);
break;
case VIR_DOMAIN_LOADER_TYPE_PFLASH:
@@ -9339,9 +9340,14 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
NULL);
}
+ if (qemuGetDriveSourceString(loader->src, NULL, &source) < 0)
+ break;
+
virBufferAddLit(&buf, "file=");
- virQEMUBuildBufferEscapeComma(&buf, loader->path);
- virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", unit);
+ virQEMUBuildBufferEscapeComma(&buf, source);
+ VIR_FREE(source);
+ virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d",
+ unit);
unit++;
if (loader->readonly) {
@@ -9354,9 +9360,14 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
if (loader->nvram) {
virBufferFreeAndReset(&buf);
+ if (qemuGetDriveSourceString(loader->nvram, NULL, &source) < 0)
+ break;
+
virBufferAddLit(&buf, "file=");
- virQEMUBuildBufferEscapeComma(&buf, loader->nvram);
- virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", unit);
+ virQEMUBuildBufferEscapeComma(&buf, source);
+ virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d",
+ unit);
+ unit++;
virCommandAddArg(cmd, "-drive");
virCommandAddArgBuffer(cmd, &buf);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d3beee5..5b73a6e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3346,6 +3346,9 @@ qemuDomainDefPostParse(virDomainDefPtr def,
* function shall not fail in that case. It will be re-run on VM startup
* with the capabilities populated. */
virQEMUCapsPtr qemuCaps = parseOpaque;
+ virDomainLoaderDefPtr ldr = NULL;
+ char *nvramPath = NULL;
+
int ret = -1;
if (def->os.bootloader || def->os.bootloaderArgs) {
@@ -3360,13 +3363,20 @@ qemuDomainDefPostParse(virDomainDefPtr def,
goto cleanup;
}
- if (def->os.loader &&
- def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
- def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON &&
- !def->os.loader->nvram) {
- if (virAsprintf(&def->os.loader->nvram, "%s/%s_VARS.fd",
+ ldr = def->os.loader;
+ if (ldr &&
+ ldr->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
+ ldr->readonly == VIR_TRISTATE_SWITCH_ON &&
+ !ldr->nvram) {
+ if (virAsprintf(&nvramPath, "%s/%s_VARS.fd",
cfg->nvramDir, def->name) < 0)
goto cleanup;
+ ldr->nvram = virStorageSourceNewFromBackingAbsolute(nvramPath);
+ if (!ldr->nvram) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to add NVRAM drive %s"), nvramPath);
+ goto cleanup;
+ }
}
if (qemuDomainDefAddDefaultDevices(def, qemuCaps) < 0)
@@ -10547,19 +10557,22 @@ qemuDomainSetupLoader(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
VIR_DEBUG("Setting up loader");
- if (loader) {
+ if (loader && loader->src) {
switch ((virDomainLoader) loader->type) {
case VIR_DOMAIN_LOADER_TYPE_ROM:
- if (qemuDomainCreateDevice(loader->path, data, false) < 0)
+ if (loader->src->type == VIR_STORAGE_TYPE_FILE &&
+ qemuDomainCreateDevice(loader->src->path, data, false) < 0)
goto cleanup;
break;
case VIR_DOMAIN_LOADER_TYPE_PFLASH:
- if (qemuDomainCreateDevice(loader->path, data, false) < 0)
+ if (loader->src->type == VIR_STORAGE_TYPE_FILE &&
+ qemuDomainCreateDevice(loader->src->path, data, false) < 0)
goto cleanup;
if (loader->nvram &&
- qemuDomainCreateDevice(loader->nvram, data, false) < 0)
+ loader->nvram->type == VIR_STORAGE_TYPE_FILE &&
+ qemuDomainCreateDevice(loader->nvram->path, data, false) < 0)
goto cleanup;
break;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e61af23..a056bc4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7526,12 +7526,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
if (vm->def->os.loader &&
vm->def->os.loader->nvram &&
- virFileExists(vm->def->os.loader->nvram)) {
+ vm->def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE &&
+ virFileExists(vm->def->os.loader->nvram->path)) {
if ((flags & VIR_DOMAIN_UNDEFINE_NVRAM)) {
- if (unlink(vm->def->os.loader->nvram) < 0) {
+ if (unlink(vm->def->os.loader->nvram->path) < 0) {
virReportSystemError(errno,
_("failed to remove nvram: %s"),
- vm->def->os.loader->nvram);
+ vm->def->os.loader->nvram->path);
goto endjob;
}
} else if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)) {
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 351425f..9b1a86e 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -650,6 +650,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
int idx = -1;
int busid = -1;
int unitid = -1;
+ bool is_firmware = false;
if (qemuParseKeywords(val,
&keywords,
@@ -772,6 +773,9 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO;
} else if (STREQ(values[i], "xen")) {
def->bus = VIR_DOMAIN_DISK_BUS_XEN;
+ } else if (STREQ(values[i], "pflash")) {
+ def->bus = VIR_DOMAIN_DISK_BUS_LAST;
+ is_firmware = true;
} else if (STREQ(values[i], "sd")) {
def->bus = VIR_DOMAIN_DISK_BUS_SD;
}
@@ -943,8 +947,25 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
ignore_value(VIR_STRDUP(def->dst, "hda"));
}
- if (!def->dst)
- goto error;
+ if (!def->dst) {
+ if (is_firmware && def->bus == VIR_DOMAIN_DISK_BUS_LAST) {
+ if (!dom->os.loader && (VIR_ALLOC(dom->os.loader) < 0))
+ goto error;
+ if (def->src->readonly) {
+ /* Loader spec */
+ dom->os.loader->src = def->src;
+ dom->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
+ } else {
+ /* NVRAM Spec */
+ if (!dom->os.loader->nvram && (VIR_ALLOC(dom->os.loader->nvram) < 0))
+ goto error;
+ dom->os.loader->nvram = def->src;
+ }
+ } else {
+ goto error;
+ }
+ }
+
if (STREQ(def->dst, "xvda"))
def->dst[3] = 'a' + idx;
else
@@ -2215,8 +2236,11 @@ qemuParseCommandLine(virCapsPtr caps,
} else if (STREQ(arg, "-bios")) {
WANT_VALUE();
if (VIR_ALLOC(def->os.loader) < 0 ||
- VIR_STRDUP(def->os.loader->path, val) < 0)
+ VIR_ALLOC(def->os.loader->src) < 0 ||
+ VIR_STRDUP(def->os.loader->src->path, val) < 0)
goto error;
+ def->os.loader->src->type = VIR_STORAGE_TYPE_FILE;
+ def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_ROM;
} else if (STREQ(arg, "-initrd")) {
WANT_VALUE();
if (VIR_STRDUP(def->os.initrd, val) < 0)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5b73a61..5d406ef 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4187,25 +4187,53 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
const char *master_nvram_path;
ssize_t r;
- if (!loader || !loader->nvram || virFileExists(loader->nvram))
+ /* This function is used to prepare a pristine local
+ * NVRAM file based on an existing template.
+ * The template could be explicitly specified in domain XML
+ * or inferred from absolute firmware path.
+ *
+ * Return early in the following cases:
+ * (1) loader or NVRAM is not specified.
+ * (2) loader is not of type 'pflash'.
+ * (3) NVRAM is already described as 'network-backed'
+ * (4) NVRAM is file-backed but file already exists.
+ * (5) Loader is network-backed, and hence impossible to
+ * infer firmware.
+ * (6) NVRAM is network-backed,
+ * hence not easy to customize.
+ */
+ if (!loader || !loader->src || !loader->nvram ||
+ loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH ||
+ loader->src->type == VIR_STORAGE_TYPE_NETWORK ||
+ loader->nvram->type == VIR_STORAGE_TYPE_NETWORK)
+ return 0;
+
+ if (loader->nvram->type == VIR_STORAGE_TYPE_FILE &&
+ virFileExists(loader->nvram->path))
return 0;
master_nvram_path = loader->templt;
- if (!loader->templt) {
+ /* Even if a template is not specified, we associate "known" EFI firmware
+ * to their NVRAM templates.
+ * Ofcourse this only applies to local firmware paths, as it is diffcult
+ * for libvirt to parse all network paths.
+ */
+ if (!loader->templt && loader->src->type == VIR_STORAGE_TYPE_FILE) {
size_t i;
for (i = 0; i < cfg->nfirmwares; i++) {
- if (STREQ(cfg->firmwares[i]->name, loader->path)) {
+ if (STREQ(cfg->firmwares[i]->name, loader->src->path)) {
master_nvram_path = cfg->firmwares[i]->nvram;
break;
}
}
}
- if (!master_nvram_path) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("unable to find any master var store for "
- "loader: %s"), loader->path);
- goto cleanup;
+ if (!master_nvram_path && loader->nvram) {
+ /* There is no template description, but an NVRAM spec
+ * has already been provided.
+ * Trust the client to have generated the right spec here
+ */
+ return 0;
}
if ((srcFD = virFileOpenAs(master_nvram_path, O_RDONLY,
@@ -4215,13 +4243,13 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
master_nvram_path);
goto cleanup;
}
- if ((dstFD = virFileOpenAs(loader->nvram,
+ if ((dstFD = virFileOpenAs(loader->nvram->path,
O_WRONLY | O_CREAT | O_EXCL,
S_IRUSR | S_IWUSR,
cfg->user, cfg->group, 0)) < 0) {
virReportSystemError(-dstFD,
_("Failed to create file '%s'"),
- loader->nvram);
+ loader->nvram->path);
goto cleanup;
}
created = true;
@@ -4239,7 +4267,7 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
if (safewrite(dstFD, buf, r) < 0) {
virReportSystemError(errno,
_("Unable to write to file '%s'"),
- loader->nvram);
+ loader->nvram->path);
goto cleanup;
}
} while (r);
@@ -4253,7 +4281,7 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
if (VIR_CLOSE(dstFD) < 0) {
virReportSystemError(errno,
_("Unable to close file '%s'"),
- loader->nvram);
+ loader->nvram->path);
goto cleanup;
}
@@ -4263,7 +4291,7 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
* copy the file content. Roll back. */
if (ret < 0) {
if (created)
- unlink(loader->nvram);
+ unlink(loader->nvram->path);
}
VIR_FORCE_CLOSE(srcFD);
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 8938e2d..3febea6 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1604,7 +1604,8 @@ virSecurityDACRestoreAllLabel(virSecurityManagerPtr mgr,
}
if (def->os.loader && def->os.loader->nvram &&
- virSecurityDACRestoreFileLabel(priv, def->os.loader->nvram) < 0)
+ def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE &&
+ virSecurityDACRestoreFileLabel(priv, def->os.loader->nvram->path) < 0)
rc = -1;
return rc;
@@ -1732,8 +1733,9 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
return -1;
if (def->os.loader && def->os.loader->nvram &&
+ def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE &&
virSecurityDACSetOwnership(priv, NULL,
- def->os.loader->nvram, user, group) < 0)
+ def->os.loader->nvram->path, user, group) < 0)
return -1;
if (def->os.kernel &&
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 5f74ef7..bcda939 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2459,7 +2459,8 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManagerPtr mgr,
rc = -1;
if (def->os.loader && def->os.loader->nvram &&
- virSecuritySELinuxRestoreFileLabel(mgr, def->os.loader->nvram) < 0)
+ def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE &&
+ virSecuritySELinuxRestoreFileLabel(mgr, def->os.loader->nvram->path) < 0)
rc = -1;
return rc;
@@ -2851,8 +2852,9 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr,
/* This is different than kernel or initrd. The nvram store
* is really a disk, qemu can read and write to it. */
if (def->os.loader && def->os.loader->nvram &&
+ def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE &&
secdef && secdef->imagelabel &&
- virSecuritySELinuxSetFilecon(mgr, def->os.loader->nvram,
+ virSecuritySELinuxSetFilecon(mgr, def->os.loader->nvram->path,
secdef->imagelabel) < 0)
return -1;
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index d0f9876..8217d67 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1063,12 +1063,18 @@ get_files(vahControl * ctl)
if (vah_add_file(&buf, ctl->def->os.slic_table, "r") != 0)
goto cleanup;
- if (ctl->def->os.loader && ctl->def->os.loader->path)
- if (vah_add_file(&buf, ctl->def->os.loader->path, "rk") != 0)
+ if (ctl->def->os.loader &&
+ ctl->def->os.loader->src &&
+ ctl->def->os.loader->src->type == VIR_STORAGE_TYPE_FILE &&
+ ctl->def->os.loader->src->path)
+ if (vah_add_file(&buf, ctl->def->os.loader->src->path, "rk") != 0)
goto cleanup;
- if (ctl->def->os.loader && ctl->def->os.loader->nvram)
- if (vah_add_file(&buf, ctl->def->os.loader->nvram, "rwk") != 0)
+ if (ctl->def->os.loader &&
+ ctl->def->os.loader->nvram &&
+ ctl->def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE &&
+ ctl->def->os.loader->nvram->path)
+ if (vah_add_file(&buf, ctl->def->os.loader->nvram->path, "rwk") != 0)
goto cleanup;
for (i = 0; i < ctl->def->ngraphics; i++) {
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 72a24a3..60451a3 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -998,11 +998,16 @@ vboxSetBootDeviceOrder(virDomainDefPtr def, vboxDriverPtr data,
VIR_DEBUG("def->os.initrd %s", def->os.initrd);
VIR_DEBUG("def->os.cmdline %s", def->os.cmdline);
VIR_DEBUG("def->os.root %s", def->os.root);
- if (def->os.loader) {
- VIR_DEBUG("def->os.loader->path %s", def->os.loader->path);
+ if (def->os.loader &&
+ def->os.loader->src &&
+ def->os.loader->src->type == VIR_STORAGE_TYPE_FILE) {
+
+ VIR_DEBUG("def->os.loader->src->path %s", def->os.loader->src->path);
VIR_DEBUG("def->os.loader->readonly %d", def->os.loader->readonly);
VIR_DEBUG("def->os.loader->type %d", def->os.loader->type);
- VIR_DEBUG("def->os.loader->nvram %s", def->os.loader->nvram);
+ if (def->os.loader->nvram &&
+ def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE)
+ VIR_DEBUG("def->os.loader->nvram->path %s", def->os.loader->nvram->path);
}
VIR_DEBUG("def->os.bootloader %s", def->os.bootloader);
VIR_DEBUG("def->os.bootloaderArgs %s", def->os.bootloaderArgs);
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 42b305d..4070660 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -1444,10 +1444,12 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
char *value = NULL;
defPtr->os.type = VIR_DOMAIN_OSTYPE_XEN;
if (VIR_ALLOC(defPtr->os.loader) < 0 ||
- VIR_STRDUP(defPtr->os.loader->path, "pygrub") < 0) {
+ VIR_ALLOC(defPtr->os.loader->src) < 0 ||
+ VIR_STRDUP(defPtr->os.loader->src->path, "pygrub") < 0) {
VIR_FREE(boot_policy);
goto error;
}
+ defPtr->os.loader->src->type = VIR_STORAGE_TYPE_FILE;
xen_vm_get_pv_kernel(session, &value, vm);
if (STRNEQ(value, "")) {
if (VIR_STRDUP(defPtr->os.kernel, value) < 0) {
diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c
index e868c05..fd3165c 100644
--- a/src/xenconfig/xen_sxpr.c
+++ b/src/xenconfig/xen_sxpr.c
@@ -87,15 +87,17 @@ xenParseSxprOS(const struct sexpr *node,
int hvm)
{
if (hvm) {
- if (VIR_ALLOC(def->os.loader) < 0)
+ if ((VIR_ALLOC(def->os.loader) < 0) ||
+ (VIR_ALLOC(def->os.loader->src) < 0))
goto error;
- if (sexpr_node_copy(node, "domain/image/hvm/loader", &def->os.loader->path) < 0)
+ def->os.loader->src->type = VIR_STORAGE_TYPE_FILE;
+ if (sexpr_node_copy(node, "domain/image/hvm/loader", &def->os.loader->src->path) < 0)
goto error;
- if (def->os.loader->path == NULL) {
- if (sexpr_node_copy(node, "domain/image/hvm/kernel", &def->os.loader->path) < 0)
+ if (def->os.loader->src->path == NULL) {
+ if (sexpr_node_copy(node, "domain/image/hvm/kernel", &def->os.loader->src->path) < 0)
goto error;
- if (def->os.loader->path == NULL) {
+ if (def->os.loader->src->path == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("domain information incomplete, missing HVM loader"));
return -1;
@@ -124,7 +126,8 @@ xenParseSxprOS(const struct sexpr *node,
/* If HVM kenrel == loader, then old xend, so kill off kernel */
if (hvm &&
def->os.kernel &&
- STREQ(def->os.kernel, def->os.loader->path)) {
+ def->os.loader->src &&
+ STREQ(def->os.kernel, def->os.loader->src->path)) {
VIR_FREE(def->os.kernel);
}
/* Drop kernel argument that has no value */
@@ -2259,9 +2262,9 @@ xenFormatSxpr(virConnectPtr conn, virDomainDefPtr def)
if (hvm) {
char bootorder[VIR_DOMAIN_BOOT_LAST+1];
if (def->os.kernel)
- virBufferEscapeSexpr(&buf, "(loader '%s')", def->os.loader->path);
+ virBufferEscapeSexpr(&buf, "(loader '%s')", def->os.loader->src->path);
else
- virBufferEscapeSexpr(&buf, "(kernel '%s')", def->os.loader->path);
+ virBufferEscapeSexpr(&buf, "(kernel '%s')", def->os.loader->src->path);
virBufferAsprintf(&buf, "(vcpus %u)", virDomainDefGetVcpusMax(def));
if (virDomainDefHasVcpusOffline(def))
diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c
index 4becb40..408b7b8 100644
--- a/src/xenconfig/xen_xm.c
+++ b/src/xenconfig/xen_xm.c
@@ -47,8 +47,10 @@ xenParseXMOS(virConfPtr conf, virDomainDefPtr def)
const char *boot;
if (VIR_ALLOC(def->os.loader) < 0 ||
- xenConfigCopyString(conf, "kernel", &def->os.loader->path) < 0)
+ VIR_ALLOC(def->os.loader->src) < 0 ||
+ xenConfigCopyString(conf, "kernel", &def->os.loader->src->path) < 0)
return -1;
+ def->os.loader->src->type = VIR_STORAGE_TYPE_FILE;
if (xenConfigGetString(conf, "boot", &boot, "c") < 0)
return -1;
@@ -484,9 +486,10 @@ xenFormatXMOS(virConfPtr conf, virDomainDefPtr def)
if (xenConfigSetString(conf, "builder", "hvm") < 0)
return -1;
- if (def->os.loader && def->os.loader->path &&
- xenConfigSetString(conf, "kernel", def->os.loader->path) < 0)
+ if (def->os.loader && def->os.loader->src &&
+ xenConfigSetString(conf, "kernel", def->os.loader->src->path) < 0)
return -1;
+ def->os.loader->src->type = VIR_STORAGE_TYPE_FILE;
for (i = 0; i < def->os.nBootDevs; i++) {
switch (def->os.bootDevs[i]) {
--
1.8.1.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/21/2018 07:10 AM, Prerna Saxena wrote:
> Augment definition to include virStorageSourcePtr that
> more comprehensively describes the nature of backing element.
> Also include flags for annotating if input XML definition is
> old-style or new-style.
>
> 2) Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef.
>
> This patch is used to interpret domain XML and store the Loader &
> Nvram's backing definitions as virStorageSource.
> It also identifies if input XML used old or new-style declaration.
> (This will later be used by the formatter).
>
> 3) Format the loader source appropriately.
>
> If the initial XML used the old-style declaration as follows:
> <loader type='pflash'>/path/to/file</loader>
>
> we format it as was read.
>
> However, if it used new-style declaration:
> <loader type='pflash' backing='file'>
> <source file='path/to/file'/>
> </loader>
>
> The formatter identifies that this is a new-style format
> and renders it likewise.
>
> 4) Plumb the loader source into generation of QEMU command line.
>
> Given that nvram & loader elements can now be backed by a non-local
> source too, adjust all steps leading to generation of
> QEMU command line.
>
> 5) Fix the domain def inference logic to correctly account for network-backed
> pflash devices.
>
> 6) Bhyve: Fix command line generation to correctly pick up local loader path.
>
> 7) virt-aa-helper: Adjust references to loader & nvram elements to correctly
> parse the virStorageSource types.
>
> 8) Vbox: Adjust references to 'loader' and 'nvram' elements given that
> these are now represented by virStorageSourcePtr.
>
> 9)Xen: Adjust all references to loader & nvram elements given that they are now backed by virStorageSourcePtr
> ---
> src/bhyve/bhyve_command.c | 6 +-
> src/conf/domain_conf.c | 250 +++++++++++++++++++++++++++++++++++++---
> src/conf/domain_conf.h | 11 +-
> src/qemu/qemu_cgroup.c | 13 ++-
> src/qemu/qemu_command.c | 21 +++-
> src/qemu/qemu_domain.c | 31 +++--
> src/qemu/qemu_driver.c | 7 +-
> src/qemu/qemu_parse_command.c | 30 ++++-
> src/qemu/qemu_process.c | 54 ++++++---
> src/security/security_dac.c | 6 +-
> src/security/security_selinux.c | 6 +-
> src/security/virt-aa-helper.c | 14 ++-
> src/vbox/vbox_common.c | 11 +-
> src/xenapi/xenapi_driver.c | 4 +-
> src/xenconfig/xen_sxpr.c | 19 +--
> src/xenconfig/xen_xm.c | 9 +-
> 16 files changed, 409 insertions(+), 83 deletions(-)
>
The $SUBJ and commit message are just poorly formatted.
But it pretty much shows that everything just got thrown into this one
patch and you went from there.
This series needs to progress a bit more "reasonably" to the desired
goal. Consider this progression (with the following as patch 1 of the
entire series):
1. Change path of loader to union:
struct _virDomainLoaderDef {
union {
char *path;
} loader;
then compile/fix up references.
2. Create an accessor to loader.path - that way future consumers can
just fetch the source path, e.g.:
const char *
virDomainLoaderDefGetLoaderPath(virDomainLoaderDefPtr loader)
{
return loader->loader.path;
}
Anything that accesses loader.path should change to use this via
something like:
const char *loaderPath;
...
loaderPath = virDomainLoaderDefGetLoaderPath(xxx)
...
Eventually this code will return either loader.path or loader.src->path
since both FILE and NETWORK storage use src->path.
3. Add virStorageSourcePtr to the loader union and modify places in the
code to use/read it. Update the domaincommon.rng, update formatdomain to
describe usage of <source> for <loader>, add genericxml2xmltests for
both "loader-source-file" and "loader-source-network" type formats for
at least "type='rom'". You could add type='pflash' tests too...
...
union {
char *path;
virStorageSourcePtr src;
} loader;
bool useLoaderSrc;
...
The patch code to parse needs to be changed to follow more closely what
virDomainDisk{BackingStore|DefMirror}Parse does... Alter ctxt locally
to the passed @node (and restore at the end). It should also be able to
use the union to do the magic, consider:
loader->loader.path = (char *) xmlNodeGetContent(node);
/* When not present, could return '' */
if (virStringIsEmpty(loader->loader.path))
VIR_FREE(loader->loader.path);
/* See if we need to look for new style <source...> subelement */
if (!loader->loader.path) {
xmlNodePtr source;
if (!(source = virXPathNode("./source", ctxt))) {
virReportError(VIR_ERR_XML_ERROR, "%s"
_("missing os loader source"));
goto cleanup;
}
if (VIR_ALLOC(loader->loader.src) < 0)
goto cleanup;
if ((tmp = virXMLPropString(source, "file")))
loader->loader.src->type = VIR_STORAGE_TYPE_FILE;
else if ((tmp = virXMLPropString(source, "protocol")))
loader->loader.src->type = VIR_STORAGE_TYPE_NETWORK;
if (loader->loader.src->type == VIR_STORAGE_TYPE_NONE) {
virReportError(VIR_ERR_XML_ERROR,
_("unknown loader source type: '%s"), tmp);
goto cleanup;
}
if (virDomainStorageSourceParse(source, ctxt,
loader->loader.src, 0) < 0)
goto cleanup;
loader->useLoaderSrc = true;
}
Then do the similar set of changes for nvram... Although for this one -
it's slightly trickier since <nvram> is optional... You'll probably
also need to modify qemuDomainDefPostParse to not automagically generate
an nvram.path if you're using <source>.
Once the xml2xml portion is done, the next patch alters qemu_command,
adds more tests, etc. Since you have generic xml2xml files, you probably
could just create a link from the qemuxml2argvdata directory or
create/use new files. Eventually it'd be nice for hte qemuxml2* code to
be able to use the generic xml files, but that's outside this scope.
BTW: I wouldn't bother with too many qemu_parse_command.c changes. Just
do the minimum. That code is so far out of date it's going to take a
solid effort to bring it back to today's options.
In any case, there's a lot of changes to be made so it's really not
worth going through each of the files in depth... I'll just point out
the domain_conf.h changes.
John
[...]
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index a78fdee..fb44a90 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1856,15 +1856,22 @@ typedef enum {
>
> VIR_ENUM_DECL(virDomainLoader)
>
> +struct _virStorageSource;
> +typedef struct _virStorageSource *virStorageSourcePtr;
> +
The above is not necessary.
> typedef struct _virDomainLoaderDef virDomainLoaderDef;
> typedef virDomainLoaderDef *virDomainLoaderDefPtr;
> struct _virDomainLoaderDef {
> - char *path;
> + virStorageSourcePtr src;
Eventually:
union {
char *path;
virStorageSourcePtr src;
} loaderSource;
bool useLoaderSrc;
> int readonly; /* enum virTristateBool */
> virDomainLoader type;
> int secure; /* enum virTristateBool */
> - char *nvram; /* path to non-volatile RAM */
> + virStorageSourcePtr nvram; /* path to non-voliatile RAM */
Eventually:
union {
char *nvram; /* path to non-volatile RAM */
virStorageSourcePtr src;
} nvramSource;
bool useNvramSrc;
> char *templt; /* user override of path to master nvram */
> + bool oldStyleLoader; /* is this an old-style XML formatting,
> + * ie, absolute path is directly specified? */
> + bool oldStyleNvram; /* is this an old-style XML formatting,
> + * ie, absolute path is directly specified? */
The two are replaced by the use{Loader|Nvram}Src
> };
>
You'll need to add API's for the fetch of the loader and nvram paths
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jun 4, 2018 at 6:24 PM, John Ferlan <jferlan@redhat.com> wrote:
>
> On 05/21/2018 07:10 AM, Prerna Saxena wrote:
> > Augment definition to include virStorageSourcePtr that
> > more comprehensively describes the nature of backing element.
> > Also include flags for annotating if input XML definition is
> > old-style or new-style.
> >
> > 2) Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef.
> >
> > This patch is used to interpret domain XML and store the Loader &
> > Nvram's backing definitions as virStorageSource.
> > It also identifies if input XML used old or new-style declaration.
> > (This will later be used by the formatter).
> >
> > 3) Format the loader source appropriately.
> >
> > If the initial XML used the old-style declaration as follows:
> > <loader type='pflash'>/path/to/file</loader>
> >
> > we format it as was read.
> >
> > However, if it used new-style declaration:
> > <loader type='pflash' backing='file'>
> > <source file='path/to/file'/>
> > </loader>
> >
> > The formatter identifies that this is a new-style format
> > and renders it likewise.
> >
> > 4) Plumb the loader source into generation of QEMU command line.
> >
> > Given that nvram & loader elements can now be backed by a non-local
> > source too, adjust all steps leading to generation of
> > QEMU command line.
> >
> > 5) Fix the domain def inference logic to correctly account for
> network-backed
> > pflash devices.
> >
> > 6) Bhyve: Fix command line generation to correctly pick up local loader
> path.
> >
> > 7) virt-aa-helper: Adjust references to loader & nvram elements to
> correctly
> > parse the virStorageSource types.
> >
> > 8) Vbox: Adjust references to 'loader' and 'nvram' elements given that
> > these are now represented by virStorageSourcePtr.
> >
> > 9)Xen: Adjust all references to loader & nvram elements given that they
> are now backed by virStorageSourcePtr
> > ---
> > src/bhyve/bhyve_command.c | 6 +-
> > src/conf/domain_conf.c | 250 ++++++++++++++++++++++++++++++
> +++++++---
> > src/conf/domain_conf.h | 11 +-
> > src/qemu/qemu_cgroup.c | 13 ++-
> > src/qemu/qemu_command.c | 21 +++-
> > src/qemu/qemu_domain.c | 31 +++--
> > src/qemu/qemu_driver.c | 7 +-
> > src/qemu/qemu_parse_command.c | 30 ++++-
> > src/qemu/qemu_process.c | 54 ++++++---
> > src/security/security_dac.c | 6 +-
> > src/security/security_selinux.c | 6 +-
> > src/security/virt-aa-helper.c | 14 ++-
> > src/vbox/vbox_common.c | 11 +-
> > src/xenapi/xenapi_driver.c | 4 +-
> > src/xenconfig/xen_sxpr.c | 19 +--
> > src/xenconfig/xen_xm.c | 9 +-
> > 16 files changed, 409 insertions(+), 83 deletions(-)
> >
>
> The $SUBJ and commit message are just poorly formatted.
>
> But it pretty much shows that everything just got thrown into this one
> patch and you went from there.
>
> This series needs to progress a bit more "reasonably" to the desired
> goal. Consider this progression (with the following as patch 1 of the
> entire series):
>
> 1. Change path of loader to union:
>
> struct _virDomainLoaderDef {
> union {
> char *path;
> } loader;
>
> then compile/fix up references.
>
> 2. Create an accessor to loader.path - that way future consumers can
> just fetch the source path, e.g.:
>
> const char *
> virDomainLoaderDefGetLoaderPath(virDomainLoaderDefPtr loader)
> {
> return loader->loader.path;
> }
>
> Anything that accesses loader.path should change to use this via
> something like:
>
> const char *loaderPath;
> ...
> loaderPath = virDomainLoaderDefGetLoaderPath(xxx)
> ...
>
> Eventually this code will return either loader.path or loader.src->path
> since both FILE and NETWORK storage use src->path.
>
> 3. Add virStorageSourcePtr to the loader union and modify places in the
> code to use/read it. Update the domaincommon.rng, update formatdomain to
> describe usage of <source> for <loader>, add genericxml2xmltests for
> both "loader-source-file" and "loader-source-network" type formats for
> at least "type='rom'". You could add type='pflash' tests too...
>
> ...
> union {
> char *path;
> virStorageSourcePtr src;
> } loader;
> bool useLoaderSrc;
> ...
>
> The patch code to parse needs to be changed to follow more closely what
> virDomainDisk{BackingStore|DefMirror}Parse does... Alter ctxt locally
> to the passed @node (and restore at the end). It should also be able to
> use the union to do the magic, consider:
>
> loader->loader.path = (char *) xmlNodeGetContent(node);
>
> /* When not present, could return '' */
> if (virStringIsEmpty(loader->loader.path))
> VIR_FREE(loader->loader.path);
>
> /* See if we need to look for new style <source...> subelement */
> if (!loader->loader.path) {
> xmlNodePtr source;
>
> if (!(source = virXPathNode("./source", ctxt))) {
> virReportError(VIR_ERR_XML_ERROR, "%s"
> _("missing os loader source"));
> goto cleanup;
> }
>
> if (VIR_ALLOC(loader->loader.src) < 0)
> goto cleanup;
>
> if ((tmp = virXMLPropString(source, "file")))
> loader->loader.src->type = VIR_STORAGE_TYPE_FILE;
> else if ((tmp = virXMLPropString(source, "protocol")))
> loader->loader.src->type = VIR_STORAGE_TYPE_NETWORK;
>
> if (loader->loader.src->type == VIR_STORAGE_TYPE_NONE) {
> virReportError(VIR_ERR_XML_ERROR,
> _("unknown loader source type: '%s"), tmp);
> goto cleanup;
> }
>
> if (virDomainStorageSourceParse(source, ctxt,
> loader->loader.src, 0) < 0)
> goto cleanup;
>
> loader->useLoaderSrc = true;
> }
>
>
> Then do the similar set of changes for nvram... Although for this one -
> it's slightly trickier since <nvram> is optional... You'll probably
> also need to modify qemuDomainDefPostParse to not automagically generate
> an nvram.path if you're using <source>.
>
> Once the xml2xml portion is done, the next patch alters qemu_command,
> adds more tests, etc. Since you have generic xml2xml files, you probably
> could just create a link from the qemuxml2argvdata directory or
> create/use new files. Eventually it'd be nice for hte qemuxml2* code to
> be able to use the generic xml files, but that's outside this scope.
>
> BTW: I wouldn't bother with too many qemu_parse_command.c changes. Just
> do the minimum. That code is so far out of date it's going to take a
> solid effort to bring it back to today's options.
>
> In any case, there's a lot of changes to be made so it's really not
> worth going through each of the files in depth... I'll just point out
> the domain_conf.h changes.
>
Thanks John for the elaborate review. I will start in this direction and
post the next series asap.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.