[libvirt] [PATCH 00/12][v2] Introduce network-backed loader & nvram.
Libvirt domain XML today only allows local filepaths that can be used to specify a loader element or its matching NVRAM disk. Given that Vms may themselves move across hypervisor hosts, it should be possible to allocate loaders/NVRAM disks on network storage for uninterrupted access. This series extends the firmware loader & NVRAM disks to be hosted over network-backed disks, for high availability. It achieves this by embedding virStorageSource elements for loader & nvram into _virDomainLoaderDef, as discussed in https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html. Currently, the source type is annotated by introducing a new attribute "backing" for both 'loader' and 'nvram' elements. Hence, a sample XML with new annotation looks like this: <loader readonly='yes' type='pflash' backing='file'> <source file='/usr/share/OVMF/OVMF_CODE.fd'/> </loader> <nvram backing='network'> <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'> <host name='example.com' port='6000'/> </source> References: ---------- v0 / Proposal: https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html. v1 : https://www.redhat.com/archives/libvir-list/2018-April/msg02024.html Changelog: --------- Changes since v1: 1. Split up the patch into smaller units. 2. Added doc snippet & tests. 3. Changed the formatting code in patch 4 to format a domain's XML in the style that was read. I found that encryption seems to be a property of the storage volume. I didnt include that in this series since it does not use backing type as volume. Will include that later once the basic network support patches get done. Looking forward to feedback, Prerna Prerna Saxena (12): Schema: Introduce XML schema for network-backed loader and nvram elements. Loader: Add a more elaborate definition. Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef. Format the loader source appropriately. Plumb the loader source into generation of QEMU command line. Fix the domain def inference logic to correctly account for network-backed pflash devices. Bhyve: Fix command line generation to correctly pick up local loader path. virt-aa-helper: Adjust references to loader & nvram elements to correctly parse the virStorageSource types. Vbox: Adjust references to 'loader' and 'nvram' elements given that these are now represented by virStorageSourcePtr. Xen: Adjust all references to loader & nvram elements given that they are now backed by virStorageSourcePtr Test: Add a test snippet to evaluate command line generation for loader/nvram specified via virStorageSource Documentation: Add a blurb for the newly added XML snippets for loader and nvram. docs/formatdomain.html.in | 36 ++++- docs/schemas/domaincommon.rng | 108 ++++++++++--- src/bhyve/bhyve_command.c | 6 +- src/conf/domain_conf.c | 215 +++++++++++++++++++++++-- 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 | 42 +++-- 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 +- tests/qemuxml2argvdata/bios-nvram-network.args | 31 ++++ tests/qemuxml2argvdata/bios-nvram-network.xml | 42 +++++ tests/qemuxml2argvtest.c | 1 + 21 files changed, 562 insertions(+), 101 deletions(-) create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml -- 1.8.1.2
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@gmail.com> --- 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
$SUBJ: Is a bit long - goal is <= 70-ish characters On 05/14/2018 07:15 AM, Prerna Saxena wrote:
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@gmail.com> --- docs/schemas/domaincommon.rng | 108 +++++++++++++++++++++++++++++++++++------- 1 file changed, 90 insertions(+), 18 deletions(-)
First off: applying all the patches and running make w/ "check" and "syntax-check" fails during "check" w/ qemuxml2argvtest and qemuxml2xmltest failing. Before posting patches those must pass for each patch. When I get to patch 2, the build breaks. While one can appreciate having less to review in each patch, it's not possible to accept or review patches which cause a build break. Shouldn't be up to the reviewer to figure out how to make the series work. The rule is - each patch must be separately compileable and capable of passing check and syntax-check. If one ever needs to git bisect to determine where a problem lies, it'd be very awful if the build broke. As for this patch in particular, there are two things going on in this patch - #1 altering the <loader> and <nvram> schema and #2 extracting out part of the diskSourceFile to be used in #1. Ironically, Thing 2 is creating a referenced name to be included in part of Thing 1; however, Thing 1 is essentially a cut-n-paste of the same thing. All those elements that are cut-n-pasted are part of diskSourceNetwork, except you didn't include VxHS in your list. Still, a question in my mind is whether you really need to format the file using source. If the goal is to provide the ability to access networked storage, why provide the option to allow someone to change their XML from: <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> to <loader type='rom' backing='file'> <source file='/usr/lib/xen/boot/hvmloader'/> </loader> Yes, they are equivalent, but it seems the reason for it was because you wanted to format the former into the latter at one point in time. If you limit to network only, then perhaps your optional attribute changes to be network='yes' which means to parse a <source> which may clear up some of the "odd" pieces of the grammar below.
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>
This won't be equivalent to what you started with. Prior to this change absFilePath was required, now it would be optional. I would assume that if it's not optional here for absFilePath, then the <source> cannot be optional as well.
+ <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>
This is slightly different as absFilePath is optional...
+ <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>
RNG format not my area of expertise - I usually copy, paste, and pray it works. Still the above looks really strange with all those <group> .. </group>... After a bit of playing I came up with the following diffs from master - although I'm not quite sure if they work later on: For loader: - <ref name="absFilePath"/> + <optional> + <ref name="osLoaderNvramBacking"/> + </optional> + <ref name="osLoaderNvramSource"/> For nvram: - <ref name="absFilePath"/> + <ref name="osLoaderNvramBacking"/> + </optional> + <optional> + <ref name="osLoaderNvramSource"/> and just before <define name="ostypexen"> + + <define name="osLoaderNvramBacking"> + <attribute name="backing"> + <choice> + <value>file</value> + <value>network</value> + </choice> + </attribute> + </define> + + <define name="osLoaderNvramSource"> + <choice> + <group> + <ref name="absFilePath"/> + <empty/> + </group> + <group> + <ref name="diskSourceFileElement"/> + <ref name="diskSourceNetworkProtocolNBD"/> + <ref name="diskSourceNetworkProtocolGluster"/> + <ref name="diskSourceNetworkProtocolRBD"/> + <ref name="diskSourceNetworkProtocolISCSI"/> + <ref name="diskSourceNetworkProtocolHTTP"/> + <ref name="diskSourceNetworkProtocolSimple"/> + </group> + </choice> + </define> + But again - I'm not the expert... maybe someone else will have some ideas/help in this area. At the very least using the <define>'s in order to reduce the cut-copy-paste for loader and nvram is a must. How exactly the grammar has to look to make things work - that's TBD.
</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> +
I would have extracted this patch into it's own patch, but as noted above - I'm not convinced you need to have the <source> element using a file attribute. John
<define name="diskSourceBlock"> <attribute name="type"> <value>block</value>
Hi John, Thanks for the review. On Thu, May 17, 2018 at 3:45 AM, John Ferlan <jferlan@redhat.com> wrote:
$SUBJ:
Is a bit long - goal is <= 70-ish characters
Agree, I'll fix this.
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,
On 05/14/2018 07:15 AM, Prerna Saxena wrote: this patch
defines XML spec for serving loader & nvram disks via the network.
Signed-off-by: Prerna Saxena <saxenap.ltc@gmail.com> --- docs/schemas/domaincommon.rng | 108 ++++++++++++++++++++++++++++++ +++++------- 1 file changed, 90 insertions(+), 18 deletions(-)
First off: applying all the patches and running make w/ "check" and "syntax-check" fails during "check" w/ qemuxml2argvtest and qemuxml2xmltest failing.
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.
Before posting patches those must pass for each patch. When I get to patch 2, the build breaks. While one can appreciate having less to review in each patch, it's not possible to accept or review patches which cause a build break. Shouldn't be up to the reviewer to figure out how to make the series work. The rule is - each patch must be separately compileable and capable of passing check and syntax-check. If one ever needs to git bisect to determine where a problem lies, it'd be very awful if the build broke.
As for this patch in particular, there are two things going on in this patch - #1 altering the <loader> and <nvram> schema and #2 extracting out part of the diskSourceFile to be used in #1. Ironically, Thing 2 is creating a referenced name to be included in part of Thing 1; however, Thing 1 is essentially a cut-n-paste of the same thing. All those elements that are cut-n-pasted are part of diskSourceNetwork, except you didn't include VxHS in your list.
Did not include that since I was not sure if it is really useful.
Still, a question in my mind is whether you really need to format the file using source. If the goal is to provide the ability to access networked storage, why provide the option to allow someone to change their XML from:
<loader type='rom'>/usr/lib/xen/boot/hvmloader</loader>
to
<loader type='rom' backing='file'> <source file='/usr/lib/xen/boot/hvmloader'/> </loader>
Yes, they are equivalent, but it seems the reason for it was because you wanted to format the former into the latter at one point in time.
If you limit to network only, then perhaps your optional attribute
changes to be network='yes' which means to parse a <source> which may clear up some of the "odd" pieces of the grammar below.
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.
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>
This won't be equivalent to what you started with. Prior to this change absFilePath was required, now it would be optional.
absFilePath will become optional as there is now > 1 way to specify the storage. This is an intended side effect of broadening the spec.
I would assume that if it's not optional here for absFilePath, then the <source> cannot be optional as well.
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.
+ <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>
This is slightly different as absFilePath is optional...
+ <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>
RNG format not my area of expertise - I usually copy, paste, and pray it works. Still the above looks really strange with all those <group> .. </group>... After a bit of playing I came up with the following diffs from master - although I'm not quite sure if they work later on:
For loader:
- <ref name="absFilePath"/> + <optional> + <ref name="osLoaderNvramBacking"/> + </optional> + <ref name="osLoaderNvramSource"/>
For nvram:
- <ref name="absFilePath"/> + <ref name="osLoaderNvramBacking"/> + </optional> + <optional> + <ref name="osLoaderNvramSource"/>
and just before <define name="ostypexen">
+ + <define name="osLoaderNvramBacking"> + <attribute name="backing"> + <choice> + <value>file</value> + <value>network</value> + </choice> + </attribute> + </define> + + <define name="osLoaderNvramSource"> + <choice> + <group> + <ref name="absFilePath"/> + <empty/> + </group> + <group> + <ref name="diskSourceFileElement"/> + <ref name="diskSourceNetworkProtocolNBD"/> + <ref name="diskSourceNetworkProtocolGluster"/> + <ref name="diskSourceNetworkProtocolRBD"/> + <ref name="diskSourceNetworkProtocolISCSI"/> + <ref name="diskSourceNetworkProtocolHTTP"/> + <ref name="diskSourceNetworkProtocolSimple"/> + </group> + </choice> + </define> +
But again - I'm not the expert... maybe someone else will have some ideas/help in this area.
At the very least using the <define>'s in order to reduce the cut-copy-paste for loader and nvram is a must. How exactly the grammar has to look to make things work - that's TBD.
</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> +
I would have extracted this patch into it's own patch, but as noted above - I'm not convinced you need to have the <source> element using a file attribute.
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
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. Signed-off-by: Prerna Saxena <saxenap.ltc@gmail.com> --- src/conf/domain_conf.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 15d228b..bbd021b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1855,15 +1855,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); -- 1.8.1.2
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@gmail.com> --- 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 01:15 PM +0200, Prerna Saxena <saxenap.ltc@gmail.com> wrote:
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@gmail.com> --- 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)) { ^^ Usually it is checked for < 0.
+ 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) {
If virStorageTypeFromString fails there is a memory leak of @tmp.
+ 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:
I would replace: s/cleanup/error and s/exit/cleanup.
+ if (loader->src) + VIR_FREE(loader->src);
Shouldn’t it be a virStorageSourceFree() here? Not sure if it’s needed but it’s safer.
+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)) { ^^ Usually it is checked for < 0.
+ goto cleanup; + }
Unneeded braces.
+ + loader->nvram->type = VIR_STORAGE_TYPE_LAST; + loader->nvram->oldStyleNvram = false; + + if ((tmp = virXMLPropString(node, "backing")) && + (loader->nvram->type = virStorageTypeFromString(tmp)) <= 0) {
If virStorageTypeFromString fails there is a memory leak of @tmp.
+ 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);
Shouldn’t this be cleaned up in case of an error?
+ 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);
virStorageSourceFree…
+ 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;
Should be unneeded as VIR_ALLOC used calloc.
+ 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; } }
Note: I didn't take a closer look.
-- 1.8.1.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Mon, May 14, 2018 at 16:20:53 +0200, Marc Hartmayer wrote:
On Mon, May 14, 2018 at 01:15 PM +0200, Prerna Saxena <saxenap.ltc@gmail.com> wrote:
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@gmail.com> --- 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)) { ^^ Usually it is checked for < 0.
+ 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) {
If virStorageTypeFromString fails there is a memory leak of @tmp.
+ 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:
I would replace: s/cleanup/error and s/exit/cleanup.
+ if (loader->src) + VIR_FREE(loader->src);
Shouldn’t it be a virStorageSourceFree() here? Not sure if it’s needed but it’s safer.
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.
+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)) { ^^ Usually it is checked for < 0.
+ goto cleanup; + }
Unneeded braces.
Actually these would fail the syntax-check. Please make sure that syntax-check passes on every patch.
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. Signed-off-by: Prerna Saxena <saxenap.ltc@gmail.com> --- src/conf/domain_conf.c | 86 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 76 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index be43695..d59a579 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18094,7 +18094,7 @@ virDomainLoaderNvramDefParseXML(xmlNodePtr node, } loader->nvram->type = VIR_STORAGE_TYPE_LAST; - loader->nvram->oldStyleNvram = false; + loader->oldStyleNvram = false; if ((tmp = virXMLPropString(node, "backing")) && (loader->nvram->type = virStorageTypeFromString(tmp)) <= 0) { @@ -26212,11 +26212,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"); @@ -26226,17 +26234,75 @@ 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, + _("Cannot format loader source")); + goto cleanup; + } + + backing = virStorageTypeToString(loader->src->type); + virBufferAsprintf(buf, " backing='%s'>", backing); + + if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot format loader source")); + goto cleanup; + } + + } 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, + _("Cannot format NVRAM source")); + virBufferAddLit(buf, ">\n</nvram>\n"); + goto cleanup; + } + + virBufferEscapeString(buf, " backing='%s'>", backing); + if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot format NVRAM source")); + virBufferAddLit(buf, "</nvram>\n"); + goto cleanup; + } + } else { + /* old-style NVRAM declaration found */ + virBufferAsprintf(buf, ">%s", loader->nvram->path); + } + virBufferAddLit(buf, "\n</nvram>\n"); + } } +cleanup: + virBufferFreeAndReset(&attrBuf); + virBufferFreeAndReset(&childBuf); + return; } static void @@ -26899,7 +26965,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", -- 1.8.1.2
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@gmail.com> --- 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@gmail.com> --- src/qemu/qemu_parse_command.c | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) 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) -- 1.8.1.2
Signed-off-by: Prerna Saxena <saxenap.ltc@gmail.com> --- 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@gmail.com> --- src/security/virt-aa-helper.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) 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++) { -- 1.8.1.2
Signed-off-by: Prerna Saxena <saxenap.ltc@gmail.com> --- 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@gmail.com> --- src/xenapi/xenapi_driver.c | 4 +++- src/xenconfig/xen_sxpr.c | 19 +++++++++++-------- src/xenconfig/xen_xm.c | 9 ++++++--- 3 files changed, 20 insertions(+), 12 deletions(-) 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 8ef68bb..c6a1f03 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; @@ -481,9 +483,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
Signed-off-by: Prerna Saxena <saxenap.ltc@gmail.com> --- 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
Signed-off-by: Prerna Saxena <saxenap.ltc@gmail.com> --- docs/formatdomain.html.in | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index caeb14e..b8cb7ac 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -112,6 +112,20 @@ </os> ...</pre> + <p>Where loader/NVRAM can also be described as:</p> + +<pre> +... + <loader readonly='yes' secure='no' type='rom' backing='file'> + <source file='/usr/share/OVMF/OVMF_CODE.fd'/> + </loader> + <nvram backing='network' template='/usr/share/OVMF/OVMF_VARS.fd'> + <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/2'> + <host name='example.com' port='3260'/> + </source> + </nvram> +...</pre> + <dl> <dt><code>type</code></dt> <dd>The content of the <code>type</code> element specifies the @@ -143,7 +157,15 @@ <code>pflash</code>. Moreover, some firmwares may implement the Secure boot feature. Attribute <code>secure</code> can be used then to control it. - <span class="since">Since 2.1.0</span></dd> + <span class="since">Since 2.1.0</span><span class="since">Since 4.5.0</span> + Loader element can also be specified as a remote disk. The attribute + <code>backing</code> (accepted values are + <code>file</code> and <code>network</code>)can be used to represent + local absolute paths as well as network-backed disks. The details of + backing file may be specified as <a href="#elementsDiskSource">storage source</a> + Allowed backing type <code>file</code> replaces the old + specification and extends to all hypervisors that supported it, while + type <code>network</code> is only supported by QEMU.</dd> <dt><code>nvram</code></dt> <dd>Some UEFI firmwares may want to use a non-volatile memory to store some variables. In the host, this is represented as a file and the @@ -154,7 +176,15 @@ from the config file. Note, that for transient domains if the NVRAM file has been created by libvirt it is left behind and it is management application's responsibility to save and remove file (if needed to be - persistent). <span class="since">Since 1.2.8</span></dd> + persistent). <span class="since">Since 1.2.8</span> + <span class="since">Since 4.5.0</span>, attribute + <code>backing</code>(accepted values <code>file</code> + and <code>network</code>)can be used to specify a + <a href='#elementsDiskSource'>storage source</a> + of type file or network. The value <code>file</code>describes + absolute NVRAM paths and extends the present specification + for all hypervisors that support this, while + <code>network</code> applies only to QEMU.</dd> <dt><code>boot</code></dt> <dd>The <code>dev</code> attribute takes one of the values "fd", "hd", "cdrom" or "network" and is used to specify the next boot device @@ -2707,7 +2737,7 @@ </dd> </dl> </dd> - <dt><code>source</code></dt> + <h5><a id="elementsDiskSource">Disk source</a></h5> <dd>Representation of the disk <code>source</code> depends on the disk <code>type</code> attribute value as follows: <dl> -- 1.8.1.2
participants (5)
-
John Ferlan -
Marc Hartmayer -
Peter Krempa -
Prerna -
Prerna Saxena