Today, 'loader' and 'nvram' elements are supposed to be backed by a local disk. Given that NVRAM data could be network-backed for high availability, this patch defines XML spec for serving loader & nvram disks via the network. Signed-off-by: Prerna Saxena <saxenap.ltc(a)gmail.com&gt; --- docs/schemas/domaincommon.rng | 108 +++++++++++++++++++++++++++++++++++------- 1 file changed, 90 insertions(+), 18 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0a6b29b..a6207db 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -276,7 +276,42 @@ </choice> </attribute> </optional> - <ref name="absFilePath"/> + <optional> + <attribute name="backing"> + <choice> + <value>file</value> + <value>network</value> + </choice> + </attribute> + </optional> + <optional> + <choice> + <group> + <ref name="absFilePath"/> + </group> + <group> + <ref name="diskSourceFileElement"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolNBD"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolGluster"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolRBD"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolISCSI"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolHTTP"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolSimple"/> + </group> + </choice> + </optional> </element> </optional> <optional> @@ -287,7 +322,40 @@ </attribute> </optional> <optional> - <ref name="absFilePath"/> + <attribute name="backing"> + <choice> + <value>file</value> + <value>network</value> + </choice> + </attribute> + </optional> + <optional> + <choice> + <group> + <ref name="absFilePath"/> + </group> + <group> + <ref name="diskSourceFileElement"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolNBD"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolGluster"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolRBD"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolISCSI"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolHTTP"/> + </group> + <group> + <ref name="diskSourceNetworkProtocolSimple"/> + </group> + </choice> </optional> </element> </optional> @@ -1494,25 +1562,29 @@ </attribute> </optional> <optional> - <element name="source"> - <optional> - <attribute name="file"> - <ref name="absFilePath"/> - </attribute> - </optional> - <optional> - <ref name="storageStartupPolicy"/> - </optional> - <optional> - <ref name="encryption"/> - </optional> - <zeroOrMore> - <ref name='devSeclabel'/> - </zeroOrMore> - </element> + <ref name='diskSourceFileElement'/> </optional> </define> + <define name="diskSourceFileElement"> + <element name="source"> + <optional> + <attribute name="file"> + <ref name="absFilePath"/> + </attribute> + </optional> + <optional> + <ref name="storageStartupPolicy"/> + </optional> + <optional> + <ref name="encryption"/> + </optional> + <zeroOrMore> + <ref name='devSeclabel'/> + </zeroOrMore> + </element> + </define> + <define name="diskSourceBlock"> <attribute name="type"> <value>block</value> -- 1.8.1.2

Hi John, Thanks for the review. On Thu, May 17, 2018 at 3:45 AM, John Ferlan <jferlan(a)redhat.com&gt; wrote: Agree, I'll fix this. I had thought make rpm ran both, but later found that it did not. Have the next series ready which : (1) combines all the related patches into ones that do not break the build. (2) Pass make check and syntax-check. Did not include that since I was not sure if it is really useful. If you limit to network only, then perhaps your optional attribute Just accounting for network source alone using <source> and directly specifying absolute paths does not look like a clean design to me. When the <source> tag can describe all storage sources, we can unify the parsing and formatting of data using the helpers for virStorageSource*. If not, redundant code needs to be maintained for treating the two types separately. Please note that I am maintaining helpers to format-as-you-read, because that seems to be a requirement. But the underlying implementation should be able to unify code treating these two formats as mere rendering choices. absFilePath will become optional as there is now > 1 way to specify the storage. This is an intended side effect of broadening the spec. A user can specify either absFilePath, or the description of the backing source using the various diskSource* elements. Individually, neither of them are mandatory, but at least one of those needs to be provided. The code takes care of this case but unfortunately I could not find a way to specify "exactly one of.. " relation in the schema. I believe we need that for consistency's sake. I do not see any benefit of using "source" for just network elements alone, and directly specifying file paths the old way. I agree it seems like a less invasive way, but it makes both the code and documentation very counter-intuitive. My next set of patches which pass unit tests and syntax-check are ready, and I think I will post them to the list. Maybe it is easier to discuss the XML semantics over a saner patchqueue. Thanks, Prerna

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). Signed-off-by: Prerna Saxena <saxenap.ltc(a)gmail.com&gt; --- src/conf/domain_conf.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 124 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f678e26..be43695 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); } @@ -17986,17 +17986,62 @@ 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; + xmlNodePtr cur; + xmlXPathContextPtr cur_ctxt = ctxt; + + if (VIR_ALLOC(loader->src)) { + goto cleanup; + } + 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); + goto cleanup; + } + 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, + _("Error parsing Loader source element")); + goto cleanup; + } + break; + } + } + + /* Old-style absolute path found ? */ + if (loader->src->type == VIR_STORAGE_TYPE_LAST) { + if (!(loader->src->path = (char *) xmlNodeGetContent(node))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing loader source")); + goto cleanup; + } else { + loader->src->type = VIR_STORAGE_TYPE_FILE; + loader->oldStyleLoader = true; + } + } if (readonly_str && (loader->readonly = virTristateBoolTypeFromString(readonly_str)) <= 0) { @@ -18023,13 +18068,81 @@ virDomainLoaderDefParseXML(xmlNodePtr node, } ret = 0; - cleanup: + goto exit; +cleanup: + if (loader->src) + VIR_FREE(loader->src); +exit: VIR_FREE(readonly_str); VIR_FREE(secure_str); VIR_FREE(type_str); + return ret; } +static int +virDomainLoaderNvramDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virDomainLoaderDefPtr loader) +{ + int ret = -1; + char *tmp = NULL; + xmlNodePtr cur; + + if (VIR_ALLOC(loader->nvram)) { + goto cleanup; + } + + loader->nvram->type = VIR_STORAGE_TYPE_LAST; + loader->nvram->oldStyleNvram = false; + + if ((tmp = virXMLPropString(node, "backing")) && + (loader->nvram->type = virStorageTypeFromString(tmp)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown nvram type '%s'"), tmp); + goto cleanup; + } + VIR_FREE(tmp); + + for (cur = node->children; cur != NULL; cur = cur->next) { + if (cur->type != XML_ELEMENT_NODE) { + continue; + } + + if (virXMLNodeNameEqual(cur, "template")) { + loader->templt = virXPathString("string(./os/nvram[1]/@template)", ctxt); + continue; + } + + if (virXMLNodeNameEqual(cur, "source")) { + if (virDomainStorageSourceParse(cur, ctxt, loader->nvram, 0) < 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("Error parsing nvram source element")); + goto cleanup; + } + ret = 0; + } + } + + /* Old-style declaration found */ + if (loader->nvram->type == VIR_STORAGE_TYPE_LAST) { + if (!(loader->nvram->path = (char *) xmlNodeGetContent(node))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing nvram source")); + goto cleanup; + } else { + loader->nvram->type = VIR_STORAGE_TYPE_FILE; + loader->oldStyleNvram = true; + ret = 0; + } + } + return ret; + +cleanup: + if (loader->nvram) + VIR_FREE(loader->nvram); + return ret; +} static virBitmapPtr virDomainSchedulerParse(xmlNodePtr node, @@ -18422,11 +18535,15 @@ virDomainDefParseBootOptions(virDomainDefPtr def, if (VIR_ALLOC(def->os.loader) < 0) goto error; - if (virDomainLoaderDefParseXML(loader_node, def->os.loader) < 0) + def->os.loader->src = NULL; + def->os.loader->nvram = NULL; + 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; } } -- 1.8.1.2

On Mon, May 14, 2018 at 16:20:53 +0200, Marc Hartmayer wrote: Additionally the preferred way is to use a second virStorageSourcePtr to hold the data while it's being parsed and then use VIR_STEAL_PTR to put it into the structure. The cleanup section then can call virStorageSourceFree on the temp ptr unconditionally and it also avoids this messy labels. Actually these would fail the syntax-check. Please make sure that syntax-check passes on every patch.

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. Signed-off-by: Prerna Saxena <saxenap.ltc(a)gmail.com&gt; --- 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_process.c | 42 ++++++++++++++++++++++++++++------------- src/security/security_dac.c | 6 ++++-- src/security/security_selinux.c | 6 ++++-- 7 files changed, 88 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index d88eb78..2068eb0 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -580,16 +580,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 08f67a4..e9d6e4b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9260,6 +9260,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainLoaderDefPtr loader = def->os.loader; virBuffer buf = VIR_BUFFER_INITIALIZER; int unit = 0; + char *source = NULL; if (!loader) return; @@ -9267,7 +9268,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: @@ -9279,9 +9280,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); + free(source); + virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", + unit); unit++; if (loader->readonly) { @@ -9294,9 +9300,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 9bb6d8a..2d4e299 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3318,6 +3318,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) { @@ -3332,13 +3335,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) @@ -10494,19 +10504,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 c129321..9c491b2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7540,12 +7540,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_process.c b/src/qemu/qemu_process.c index 37876b8..e4c05e2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3994,25 +3994,41 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, const char *master_nvram_path; ssize_t r; - if (!loader || !loader->nvram || virFileExists(loader->nvram)) + /* return early if either loader is network-backed + * or NVRAM is already specified. + */ + 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, @@ -4022,13 +4038,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; @@ -4046,7 +4062,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); @@ -4060,7 +4076,7 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg, if (VIR_CLOSE(dstFD) < 0) { virReportSystemError(errno, _("Unable to close file '%s'"), - loader->nvram); + loader->nvram->path); goto cleanup; } @@ -4070,7 +4086,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; -- 1.8.1.2

Signed-off-by: Prerna Saxena <saxenap.ltc(a)gmail.com&gt; --- src/bhyve/bhyve_command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 9413ae5..2b67014 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -518,10 +518,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", -- 1.8.1.2

Signed-off-by: Prerna Saxena <saxenap.ltc(a)gmail.com&gt; --- src/vbox/vbox_common.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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); -- 1.8.1.2

Signed-off-by: Prerna Saxena <saxenap.ltc(a)gmail.com&gt; --- tests/qemuxml2argvdata/bios-nvram-network.args | 31 +++++++++++++++++++ tests/qemuxml2argvdata/bios-nvram-network.xml | 42 ++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 74 insertions(+) create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml diff --git a/tests/qemuxml2argvdata/bios-nvram-network.args b/tests/qemuxml2argvdata/bios-nvram-network.args new file mode 100644 index 0000000..3fc68ef --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name test-bios \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,\ +readonly=on \ +-drive file=iscsi://example.org:6000/iqn.1992-01.com.example/0,\ +if=pflash,format=raw,unit=1 \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-test-bios/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot order=c,menu=on \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/bios-nvram-network.xml b/tests/qemuxml2argvdata/bios-nvram-network.xml new file mode 100644 index 0000000..bad114c --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network.xml @@ -0,0 +1,42 @@ +<domain type='qemu'> + <name>test-bios</name> + <uuid>362d1fc1-df7d-193e-5c18-49a71bd1da66</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <loader readonly='yes' type='pflash' backing='file'> + <source file='/usr/share/OVMF/OVMF_CODE.fd'/> + </loader> + <nvram backing='network'> + <source protocol='iscsi' name='iqn.1992-01.com.example'> + <host name='example.org' port='6000'/> + </source> + </nvram> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='tablet' bus='usb'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ddf567b..7338dba 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -853,6 +853,7 @@ mymain(void) QEMU_CAPS_DEVICE_ISA_SERIAL, QEMU_CAPS_SGA); DO_TEST("bios-nvram", NONE); + DO_TEST("bios-nvram-network", NONE); DO_TEST("bios-nvram-secure", QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_PCI_BRIDGE, -- 1.8.1.2