Re: [libvirt] [PATCH] bhyve: add support for passing stdin to loader

On 26 Apr 2018, at 18:38, John Ferlan wrote: > On 04/13/2018 03:27 PM, Fabian Freyer wrote: >> This commit adds the <bootloader_stdin> node to the domain definition, >> with the following semantics: >> >> To pass standard input verbatim to the bootloader, set >> >> <bootloader_stdin>some stdin</bootloader_stdin> >> >> Multiline standard input can be set using a CDATA tag: >> >> <bootloader_stdin><![CDATA[ >> this standard input >> will be passed in with >> newlines and indentation. >> ]]></bootloader_stdin> >> >> Standard input can be read from a file as follows: >> >> <bootloader_stdin file="/path/to/some/file"/> > > Not my area of expertise, but some feedback to hopefully help a bit > seeing as there have been no other takes for 2 weeks. Thanks! > Personally this format over the other format would seem to be better > although the attribute name would be "path" not "file". IOW: That whole > CDATA thing - not sure it passes by all the censors^W reviewers > scrutiny. Is there a particular reason to have this CDATA tag syntax? The idea here is to be able to specify several lines of standard input, especially for line-oriented command line interfaces, while not needing to add a separate file.

> Parsing the line and making sure consumers provide a specific format is > probably more hassle than it's worth and I would think presents an > opportunity to insert things that may cause interesting errors. Any time > you accept something from stdin like that you open up buffer overflows > and buffer handling problems. Not that the same thing cannot be in the > file, but at least you're then passing that off to something else to > manage whether what's in a file is correctly formatted as you're not > validating the contents of the file, just that it exists. I’m not quite sure I comprehend the difference to a file here. >> >> Signed-off-by: Fabian Freyer <fabian.freyer(a)physik.tu-berlin.de&gt; >> --- >> docs/formatdomain.html.in | 19 ++++++

>> docs/schemas/domaincommon.rng | 10 ++++ >> src/bhyve/bhyve_driver.c | 10 ++++ >> src/bhyve/bhyve_parse_command.c | 70 ++++++++++++++++++++++ >> src/bhyve/bhyve_process.c | 22 +++++++ >> src/conf/domain_conf.c | 41 +++++++++++++ >> src/conf/domain_conf.h | 11 ++++ >> .../bhyveargv2xml-loader-stdin-file.args | 9 +++ >> .../bhyveargv2xml-loader-stdin-file.xml | 19 ++++++ >> .../bhyveargv2xml-loader-stdin-multiline.args | 13 ++++ >> .../bhyveargv2xml-loader-stdin-multiline.xml | 21 +++++++ >> .../bhyveargv2xml-loader-stdin-oneline.args | 11 ++++ >> .../bhyveargv2xml-loader-stdin-oneline.xml | 19 ++++++ >> tests/bhyveargv2xmltest.c | 3 + >> .../bhyvexml2argv-grub-stdin-file.args | 9 +++ >> .../bhyvexml2argv-grub-stdin-file.devmap | 1 + >> .../bhyvexml2argv-grub-stdin-file.ldargs | 4 ++ >> .../bhyvexml2argv-grub-stdin-file.xml | 25 ++++++++ >> .../bhyvexml2argv-grub-stdin-multiline.args | 9 +++ >> .../bhyvexml2argv-grub-stdin-multiline.devmap | 1 + >> .../bhyvexml2argv-grub-stdin-multiline.ldargs | 4 ++ >> .../bhyvexml2argv-grub-stdin-multiline.xml | 30 ++++++++++ >> .../bhyvexml2argv-grub-stdin-oneline.args | 9 +++ >> .../bhyvexml2argv-grub-stdin-oneline.devmap | 1 + >> .../bhyvexml2argv-grub-stdin-oneline.ldargs | 4 ++ >> .../bhyvexml2argv-grub-stdin-oneline.xml | 25 ++++++++ >> tests/bhyvexml2argvtest.c | 3 + >> .../bhyvexml2xmlout-grub-stdin-file.xml | 34 +++++++++++ >> .../bhyvexml2xmlout-grub-stdin-multiline.xml | 39 ++++++++++++ >> .../bhyvexml2xmlout-grub-stdin-oneline.xml | 34 +++++++++++ >> tests/bhyvexml2xmltest.c | 3 + >> 31 files changed, 513 insertions(+) >> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-file.args >> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-file.xml >> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-multiline.args >> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-multiline.xml >> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-oneline.args >> create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-oneline.xml >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.args >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.devmap >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.ldargs >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.xml >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.args >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.devmap >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.ldargs >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.xml >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.args >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.devmap >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.ldargs >> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.xml >> create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-file.xml >> create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-multiline.xml >> create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-oneline.xml >> > > Couple of quick hitter general comments > > 1. Separate the domain_conf, docs, and xml2xml changes into one patch > > 2. Place the driver, argv2xml and xml2argv changes in another patch or > even another 2 patches (argv2xml being separated out)... > > 3. Be sure to run syntax-check before posting - although I don't have > the necessary parts for bhyve on my host, I can compile parts of this > and syntax-check will run on it all. > > 4. CC Roman Bogorodskiy <bogorodskiy(a)gmail.com&gt; on your submit since > that's who generally handles bhyve related things and perhaps doesn't > follow libvir-list every day... He’s CC’d via a different alias. > Splitting patches means the concepts can be reviewed separately. If > using stdin args for loader has uses for other hypervisors, then we need > to make sure the domain_conf, docs/schemas, etc are "good enough" for > others. Could allow some success too... I’ll split out the patches in a followup series. >> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in >> index 5e99884dc..cea024235 100644 >> --- a/docs/formatdomain.html.in >> +++ b/docs/formatdomain.html.in >> @@ -245,6 +245,11 @@ >> ... >> &lt;bootloader&gt;/usr/bin/pygrub&lt;/bootloader&gt; >> &lt;bootloader_args&gt;--append single&lt;/bootloader_args&gt; >> +&lt;bootloader_stdin&gt;&lt;![CDATA[ >> +kernel (hd)/path/to/kernel >> +initrd (host)/path/to/initrd >> +boot >> +]]&gt; >> ...</pre> >> >> <dl> >> @@ -259,6 +264,20 @@ >> command line arguments to be passed to the bootloader. >> <span class="since">Since 0.2.3</span> >> </dd> >> + <dt><code>bootloader_stdin</code></dt> >> + <dd>The optional <code>bootloader_stdin</code> element specifies >> + standard input to be passed to the bootloader. To pass multiple >> + lines of standard input to the bootloader, wrap the content in >> + a CDATA tag. Instead of specifying the standard input in the >> + domain XML, the path to a file to be read may be given using the >> + <code>file</code> attribute: >> +<pre> >> +... >> +&lt;bootloader_stdin file="/path/to/some/file"/&gt; >> +... >> +</pre> >> + <span class="since">Since 4.3.0 (bhyve only)</span> > > Won't be 4.3.0.... this wouldn't seem at face value to be a bhyve thing > only... Ok. I just wasn’t sure what to put in here, I’d basically update the version in future reviews. >> + </dd> >> >> </dl> >> >> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng >> index 4cab55f05..a44d88ef3 100644 >> --- a/docs/schemas/domaincommon.rng >> +++ b/docs/schemas/domaincommon.rng >> @@ -1211,6 +1211,16 @@ >> <text/> >> </element> >> </optional> >> + <optional> >> + <choice> >> + <element name="bootloader_stdin"> >> + <text/> >> + </element> >> + <element name="bootloader_stdin"> >> + <attribute name="file"/> > > Use "path" and "absFilePath" (see other "path" attributes) > >> + </element> >> + </choice> >> + </optional> >> </interleave> >> </define> >> <define name="osbootkernel"> >> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c >> index 24c4a9c80..7ac3ad3f0 100644 >> --- a/src/bhyve/bhyve_driver.c >> +++ b/src/bhyve/bhyve_driver.c >> @@ -743,6 +743,16 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn, >> goto cleanup; >> >> virBufferAdd(&buf, virCommandToString(loadcmd), -1); >> + > > Should you check if the file exists and can be accessed? Probably, thanks for catching that. >> + if (def->os.bootloaderStdinSource == VIR_DOMAIN_BOOTLOADER_STDIN_FILE) >> + virBufferEscapeString(&buf, " < %s", def->os.bootloaderStdin); >> + else if (def->os.bootloaderStdinSource >> + == VIR_DOMAIN_BOOTLOADER_STDIN_LITERAL) { >> + virBufferEscapeString(&buf, " << END_LOADER_STDIN\n" >> + "%s\nEND_LOADER_STDIN", >> + def->os.bootloaderStdin); >> + } >> + >> virBufferAddChar(&buf, '\n'); >> } >> >> diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c >> index fcaaed275..ef51a75f1 100644 >> --- a/src/bhyve/bhyve_parse_command.c >> +++ b/src/bhyve/bhyve_parse_command.c >> @@ -124,6 +124,8 @@ static int >> bhyveCommandLineToArgv(const char *nativeConfig, >> int *loader_argc, >> char ***loader_argv, >> + char **loader_stdin_buffer, >> + char **loader_stdin_file, >> int *bhyve_argc, >> char ***bhyve_argv) >> { >> @@ -139,6 +141,10 @@ bhyveCommandLineToArgv(const char *nativeConfig, >> char **_bhyve_argv = NULL; >> char **_loader_argv = NULL; >> >> + virBuffer heredoc = VIR_BUFFER_INITIALIZER; >> + int in_heredoc = 0; >> + char *heredoc_delim = NULL; >> + >> nativeConfig_unescaped = bhyveParseCommandLineUnescape(nativeConfig); >> if (nativeConfig_unescaped == NULL) { >> virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> @@ -178,6 +184,52 @@ bhyveCommandLineToArgv(const char *nativeConfig, >> char **arglist = NULL; >> size_t args_count = 0; >> size_t args_alloc = 0; >> + char *stdin_redir = NULL; >> + >> + /* are we in a heredoc? */ >> + if ( in_heredoc ) { > > ^^ syntax-check failure due to extra spaces > >> + if (STRPREFIX(curr, heredoc_delim)) { >> + in_heredoc = 0; >> + *loader_stdin_buffer = virBufferContentAndReset(&heredoc); >> + continue; >> + } >> + >> + if (in_heredoc++ == 1) >> + virBufferAsprintf(&heredoc, "%s", curr); >> + else >> + virBufferAsprintf(&heredoc, "\n%s", curr); >> + >> + continue; >> + } >> + >> + /* check if this line contains standard input redirection. */ >> + if ( (stdin_redir = strchr(curr, '<')) ) { > > syntax-check failure again > >> + if (STREQLEN(stdin_redir, "<<", 2)) { >> + *stdin_redir = '\0'; >> + in_heredoc = 1; >> + heredoc_delim = stdin_redir + 2; >> + >> + /* skip non-alphanumeric chars */ >> + while (*heredoc_delim && !c_isalnum(*heredoc_delim)) >> + heredoc_delim ++; >> + >> + if (!*heredoc_delim) >> + goto error; >> + >> + virBufferFreeAndReset(&heredoc); >> + } else { >> + /* file redirection */ >> + *stdin_redir = '\0'; >> + stdin_redir ++; >> + >> + /* skip non-alphanumeric chars */ >> + while (*stdin_redir && !c_isalnum(*stdin_redir)) >> + stdin_redir ++; >> + >> + if (VIR_STRDUP(*loader_stdin_file, stdin_redir) != 1) >> + goto error; >> + } >> + } >> >> /* iterate over each line, splitting on sequences of ' '. This code is >> * adapted from qemu/qemu_parse_command.c. */ >> @@ -254,12 +306,16 @@ bhyveCommandLineToArgv(const char *nativeConfig, >> if (!(*bhyve_argv = _bhyve_argv)) >> goto error; >> >> + if (in_heredoc) >> + goto error; >> + >> virStringListFree(lines); >> return 0; >> >> error: >> VIR_FREE(_loader_argv); >> VIR_FREE(_bhyve_argv); >> + virBufferFreeAndReset(&heredoc); >> virStringListFree(lines); >> return -1; >> } >> @@ -869,6 +925,8 @@ bhyveParseCommandLineString(const char* nativeConfig, >> char **bhyve_argv = NULL; >> int loader_argc = 0; >> char **loader_argv = NULL; >> + char *loader_stdin_file = NULL; >> + char *loader_stdin_buffer = NULL; >> >> if (!(def = virDomainDefNew())) >> goto cleanup; >> @@ -887,12 +945,21 @@ bhyveParseCommandLineString(const char* nativeConfig, >> >> if (bhyveCommandLineToArgv(nativeConfig, >> &loader_argc, &loader_argv, >> + &loader_stdin_buffer, &loader_stdin_file, >> &bhyve_argc, &bhyve_argv)) { >> virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> _("Failed to convert the command string to argv-lists")); >> goto error; >> } >> >> + if (loader_stdin_file && !loader_stdin_buffer) { >> + def->os.bootloaderStdinSource = VIR_DOMAIN_BOOTLOADER_STDIN_FILE; >> + def->os.bootloaderStdin = loader_stdin_file; >> + } else if (loader_stdin_buffer && !loader_stdin_file) { >> + def->os.bootloaderStdinSource = VIR_DOMAIN_BOOTLOADER_STDIN_LITERAL, >> + def->os.bootloaderStdin = loader_stdin_buffer; >> + } >> + >> if (bhyveParseBhyveCommandLine(def, xmlopt, caps, bhyve_argc, bhyve_argv)) >> goto error; >> if (loader_argv && STREQ(loader_argv[0], "/usr/sbin/bhyveload")) { >> @@ -906,9 +973,12 @@ bhyveParseCommandLineString(const char* nativeConfig, >> cleanup: >> virStringListFree(loader_argv); >> virStringListFree(bhyve_argv); >> + >> return def; >> error: >> virDomainDefFree(def); >> + VIR_FREE(loader_stdin_buffer); >> + VIR_FREE(loader_stdin_file); >> def = NULL; >> goto cleanup; >> } >> diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c >> index 9276d7d36..1a6f783d7 100644 >> --- a/src/bhyve/bhyve_process.c >> +++ b/src/bhyve/bhyve_process.c >> @@ -113,6 +113,7 @@ virBhyveProcessStart(virConnectPtr conn, >> bhyveDomainObjPrivatePtr priv = vm->privateData; >> int ret = -1, rc; >> virCapsPtr caps = NULL; >> + int stdinfd = -1; >> >> if (virAsprintf(&logfile, "%s/%s.log", >> BHYVE_LOG_DIR, vm->def->name) < 0) >> @@ -173,6 +174,26 @@ virBhyveProcessStart(virConnectPtr conn, >> if (!(load_cmd = virBhyveProcessBuildLoadCmd(conn, vm->def, devmap_file, >> &devicemap))) >> goto cleanup; >> + >> + switch (vm->def->os.bootloaderStdinSource) { >> + case VIR_DOMAIN_BOOTLOADER_STDIN_NONE: >> + break; >> + case VIR_DOMAIN_BOOTLOADER_STDIN_FILE: >> + if ((stdinfd = open(vm->def->os.bootloaderStdin, O_RDONLY)) < 0) { > > Does using virFileOpenAs make sense here? Probably doesn't much matter.? Other things in the driver seem to use open(...), but probably virFileOpenAs might be better here. I can change this in a future revision, if it would make more sense. Maybe Roman has some insights here?

>> + virReportSystemError(errno, _("Failed to open '%s'"), >> + vm->def->os.bootloaderStdin); >> + goto cleanup; >> + } >> + virCommandSetInputFD(load_cmd, stdinfd); >> + break; >> + case VIR_DOMAIN_BOOTLOADER_STDIN_LITERAL: >> + virCommandSetInputBuffer(load_cmd, vm->def->os.bootloaderStdin); >> + break; >> + /* coverity[dead_error_begin] */ >> + case VIR_DOMAIN_BOOTLOADER_STDIN_LAST: >> + break; >> + } >> + >> virCommandSetOutputFD(load_cmd, &logfd); >> virCommandSetErrorFD(load_cmd, &logfd); >> >> @@ -252,6 +273,7 @@ virBhyveProcessStart(virConnectPtr conn, >> virCommandFree(load_cmd); >> virCommandFree(cmd); >> VIR_FREE(logfile); >> + VIR_FORCE_CLOSE(stdinfd); >> VIR_FORCE_CLOSE(logfd); >> return ret; >> } >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index d23182f18..d99ecf9f7 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -3037,6 +3037,8 @@ void virDomainDefFree(virDomainDefPtr def) >> VIR_FREE(def->os.bootloader); >> VIR_FREE(def->os.bootloaderArgs); >> >> + VIR_FREE(def->os.bootloaderStdin); >> + >> virDomainClockDefClear(&def->clock); >> >> VIR_FREE(def->name); >> @@ -18700,6 +18702,16 @@ virDomainDefParseXML(xmlDocPtr xml, >> def->os.bootloader = virXPathString("string(./bootloader)", ctxt); >> def->os.bootloaderArgs = virXPathString("string(./bootloader_args)", ctxt); >> >> + if ((def->os.bootloaderStdin = virXPathString("string(./bootloader_stdin/" >> + "@file)", ctxt))) >> + def->os.bootloaderStdinSource = VIR_DOMAIN_BOOTLOADER_STDIN_FILE; >> + else if ((def->os.bootloaderStdin = virXPathString("string(" >> + "./bootloader_stdin)", >> + ctxt))) >> + def->os.bootloaderStdinSource = VIR_DOMAIN_BOOTLOADER_STDIN_LITERAL; >> + else >> + def->os.bootloaderStdinSource = VIR_DOMAIN_BOOTLOADER_STDIN_NONE; >> + >> tmp = virXPathString("string(./os/type[1])", ctxt); >> if (!tmp) { >> if (def->os.bootloader) { >> @@ -26717,6 +26729,35 @@ virDomainDefFormatInternal(virDomainDefPtr def, >> virBufferEscapeString(buf, >> "<bootloader_args>%s</bootloader_args>\n", >> def->os.bootloaderArgs); >> + >> + switch (def->os.bootloaderStdinSource) { >> + case VIR_DOMAIN_BOOTLOADER_STDIN_NONE: >> + break; >> + case VIR_DOMAIN_BOOTLOADER_STDIN_FILE: >> + virBufferEscapeString(buf, "<bootloader_stdin file=\"%s\"/>\n", >> + def->os.bootloaderStdin); >> + break; >> + case VIR_DOMAIN_BOOTLOADER_STDIN_LITERAL: >> + if (strchr(def->os.bootloaderStdin, '\n') >> + || strchr(def->os.bootloaderStdin, '<') >> + || strchr(def->os.bootloaderStdin, '>') >> + || strchr(def->os.bootloaderStdin, '&')) > > Move the || to the end of the previous lines. Although I think a moot > point. > >> + { >> + virBufferEscapeString(buf, >> + "<bootloader_stdin><![CDATA[%s]]>" >> + "</bootloader_stdin>\n", >> + def->os.bootloaderStdin); >> + } else { >> + virBufferEscapeString(buf, >> + "<bootloader_stdin>%s" >> + "</bootloader_stdin>\n", >> + def->os.bootloaderStdin); >> + } >> + break; >> + /* coverity[dead_error_begin] */ >> + case VIR_DOMAIN_BOOTLOADER_STDIN_LAST: >> + break; >> + } >> } >> >> virBufferAddLit(buf, "<os>\n"); >> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h >> index bbaa24137..41af6cc8a 100644 >> --- a/src/conf/domain_conf.h >> +++ b/src/conf/domain_conf.h >> @@ -1897,6 +1897,15 @@ struct _virDomainOSEnv { >> char *value; >> }; >> >> +/* Bootloader standard input source */ >> +typedef enum { >> + VIR_DOMAIN_BOOTLOADER_STDIN_NONE = 0, >> + VIR_DOMAIN_BOOTLOADER_STDIN_FILE, >> + VIR_DOMAIN_BOOTLOADER_STDIN_LITERAL, >> + >> + VIR_DOMAIN_BOOTLOADER_STDIN_LAST >> +} virDomainBootloaderStdinSource; >> + >> typedef struct _virDomainOSDef virDomainOSDef; >> typedef virDomainOSDef *virDomainOSDefPtr; >> struct _virDomainOSDef { >> @@ -1923,6 +1932,8 @@ struct _virDomainOSDef { >> virDomainLoaderDefPtr loader; >> char *bootloader; >> char *bootloaderArgs; >> + virDomainBootloaderStdinSource bootloaderStdinSource; >> + char *bootloaderStdin; >> int smbios_mode; >> >> virDomainBIOSDef bios; >> diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-file.args b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-file.args >> new file mode 100644 >> index 000000000..ca51f2f04 >> --- /dev/null >> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-file.args >> @@ -0,0 +1,9 @@ >> +/usr/bin/custom-loader \ >> +-s ome \ >> +--args < path/to/some/file > > abs/rel file? > >> +/usr/sbin/bhyve \ >> +-c 1 \ >> +-m 214 \ >> +-H \ >> +-P \ >> +-s 0:0,hostbridge bhyve >> diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-file.xml b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-file.xml >> new file mode 100644 >> index 000000000..a56a4c451 >> --- /dev/null >> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-file.xml >> @@ -0,0 +1,19 @@ >> +<domain type='bhyve'> >> + <name>bhyve</name> >> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> >> + <memory unit='KiB'>219136</memory> >> + <currentMemory unit='KiB'>219136</currentMemory> >> + <vcpu placement='static'>1</vcpu> >> + <bootloader>/usr/bin/custom-loader</bootloader> >> + <bootloader_args>-s ome --args</bootloader_args> >> + <bootloader_stdin file="path/to/some/file"/> > > You want absFile or relFile? > > That's why I noted it above in rng schema... Not really sure what would make more sense here. I guess that’s open for discussion?

>> + <os> >> + <type>hvm</type> >> + </os> >> + <clock offset='localtime'/> >> + <on_poweroff>destroy</on_poweroff> >> + <on_reboot>destroy</on_reboot> >> + <on_crash>destroy</on_crash> >> + <devices> >> + </devices> >> +</domain> >> diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-multiline.args b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-multiline.args >> new file mode 100644 >> index 000000000..050ddf442 >> --- /dev/null >> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-multiline.args >> @@ -0,0 +1,13 @@ >> +/usr/bin/custom-loader \ >> +-s ome \ >> +--args << END_OF_THIS_HEREDOC > > This doesn't seem right - "END_OF_THIS_HEREDOC" This is basically an arbitrary identifier that ends ... >> +some >> +standard input >> +here >> +END_OF_THIS_HEREDOC here >> +/usr/sbin/bhyve \ >> +-c 1 \ >> +-m 214 \ >> +-H \ >> +-P \ >> +-s 0:0,hostbridge bhyve >> diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-multiline.xml b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-multiline.xml >> new file mode 100644 >> index 000000000..496b5ea87 >> --- /dev/null >> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-multiline.xml >> @@ -0,0 +1,21 @@ >> +<domain type='bhyve'> >> + <name>bhyve</name> >> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> >> + <memory unit='KiB'>219136</memory> >> + <currentMemory unit='KiB'>219136</currentMemory> >> + <vcpu placement='static'>1</vcpu> >> + <bootloader>/usr/bin/custom-loader</bootloader> >> + <bootloader_args>-s ome --args</bootloader_args> >> + <bootloader_stdin><![CDATA[some >> +standard input >> +here]]></bootloader_stdin> >> + <os> >> + <type>hvm</type> >> + </os> >> + <clock offset='localtime'/> >> + <on_poweroff>destroy</on_poweroff> >> + <on_reboot>destroy</on_reboot> >> + <on_crash>destroy</on_crash> >> + <devices> >> + </devices> >> +</domain> >> diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-oneline.args b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-oneline.args >> new file mode 100644 >> index 000000000..f8bcdcddd >> --- /dev/null >> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-oneline.args >> @@ -0,0 +1,11 @@ >> +/usr/bin/custom-loader \ >> +-s ome \ >> +--args << END_OF_THIS_HEREDOC> +some standard input here >> +END_OF_THIS_HEREDOC >> +/usr/sbin/bhyve \ >> +-c 1 \ >> +-m 214 \ >> +-H \ >> +-P \ >> +-s 0:0,hostbridge bhyve >> diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-oneline.xml b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-oneline.xml >> new file mode 100644 >> index 000000000..17c9da664 >> --- /dev/null >> +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-loader-stdin-oneline.xml >> @@ -0,0 +1,19 @@ >> +<domain type='bhyve'> >> + <name>bhyve</name> >> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> >> + <memory unit='KiB'>219136</memory> >> + <currentMemory unit='KiB'>219136</currentMemory> >> + <vcpu placement='static'>1</vcpu> >> + <bootloader>/usr/bin/custom-loader</bootloader> >> + <bootloader_args>-s ome --args</bootloader_args> >> + <bootloader_stdin>some standard input here</bootloader_stdin> >> + <os> >> + <type>hvm</type> >> + </os> >> + <clock offset='localtime'/> >> + <on_poweroff>destroy</on_poweroff> >> + <on_reboot>destroy</on_reboot> >> + <on_crash>destroy</on_crash> >> + <devices> >> + </devices> >> +</domain> >> diff --git a/tests/bhyveargv2xmltest.c b/tests/bhyveargv2xmltest.c >> index e5d78530c..fef01d7da 100644 >> --- a/tests/bhyveargv2xmltest.c >> +++ b/tests/bhyveargv2xmltest.c >> @@ -187,6 +187,9 @@ mymain(void) >> DO_TEST("memsize-human"); >> DO_TEST_FAIL("memsize-fail"); >> DO_TEST("custom-loader"); >> + DO_TEST("loader-stdin-file"); >> + DO_TEST("loader-stdin-oneline"); >> + DO_TEST("loader-stdin-multiline"); >> DO_TEST("bhyveload-custom"); >> DO_TEST("bhyveload-vda"); >> DO_TEST_FAIL("bhyveload-name-mismatch"); >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.args b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.args >> new file mode 100644 >> index 000000000..3ba5c1160 >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.args >> @@ -0,0 +1,9 @@ >> +/usr/sbin/bhyve \ >> +-c 1 \ >> +-m 214 \ >> +-u \ >> +-H \ >> +-P \ >> +-s 0:0,hostbridge \ >> +-s 2:0,ahci,hd:/tmp/freebsd.img \ >> +-s 3:0,virtio-net,faketapdev,mac=52:54:00:ee:f5:79 bhyve >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.devmap b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.devmap >> new file mode 100644 >> index 000000000..b312bfdaf >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.devmap >> @@ -0,0 +1 @@ >> +(hd0) /tmp/freebsd.img >> \ No newline at end of file >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.ldargs >> new file mode 100644 >> index 000000000..7d9a5155a >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.ldargs >> @@ -0,0 +1,4 @@ >> +/usr/local/sbin/grub-bhyve \ >> +--root hd0,msdos1 \ >> +--device-map '<device.map>' \ >> +--memory 214 bhyve >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.xml >> new file mode 100644 >> index 000000000..f804da0db >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-file.xml >> @@ -0,0 +1,25 @@ >> +<domain type='bhyve'> >> + <name>bhyve</name> >> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> >> + <memory>219136</memory> >> + <vcpu>1</vcpu> >> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> >> + <bootloader_stdin file="/path/to/some/file"/> >> + <os> >> + <type>hvm</type> >> + </os> >> + <devices> >> + <disk type='file'> >> + <driver name='file' type='raw'/> >> + <source file='/tmp/freebsd.img'/> >> + <target dev='hda' bus='sata'/> >> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> >> + </disk> >> + <interface type='bridge'> >> + <mac address='52:54:00:ee:f5:79'/> >> + <model type='virtio'/> >> + <source bridge="virbr0"/> >> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> >> + </interface> >> + </devices> >> +</domain> >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.args b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.args >> new file mode 100644 >> index 000000000..3ba5c1160 >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.args >> @@ -0,0 +1,9 @@ >> +/usr/sbin/bhyve \ >> +-c 1 \ >> +-m 214 \ >> +-u \ >> +-H \ >> +-P \ >> +-s 0:0,hostbridge \ >> +-s 2:0,ahci,hd:/tmp/freebsd.img \ >> +-s 3:0,virtio-net,faketapdev,mac=52:54:00:ee:f5:79 bhyve >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.devmap b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.devmap >> new file mode 100644 >> index 000000000..b312bfdaf >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.devmap >> @@ -0,0 +1 @@ >> +(hd0) /tmp/freebsd.img >> \ No newline at end of file >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.ldargs >> new file mode 100644 >> index 000000000..7d9a5155a >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.ldargs >> @@ -0,0 +1,4 @@ >> +/usr/local/sbin/grub-bhyve \ >> +--root hd0,msdos1 \ >> +--device-map '<device.map>' \ >> +--memory 214 bhyve >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.xml >> new file mode 100644 >> index 000000000..456ab0443 >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-multiline.xml >> @@ -0,0 +1,30 @@ >> +<domain type='bhyve'> >> + <name>bhyve</name> >> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> >> + <memory>219136</memory> >> + <vcpu>1</vcpu> >> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> >> + <bootloader_stdin><![CDATA[ >> +multiple >> +boot >> +loader >> +commands >> +]]></bootloader_stdin> >> + <os> >> + <type>hvm</type> >> + </os> >> + <devices> >> + <disk type='file'> >> + <driver name='file' type='raw'/> >> + <source file='/tmp/freebsd.img'/> >> + <target dev='hda' bus='sata'/> >> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> >> + </disk> >> + <interface type='bridge'> >> + <mac address='52:54:00:ee:f5:79'/> >> + <model type='virtio'/> >> + <source bridge="virbr0"/> >> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> >> + </interface> >> + </devices> >> +</domain> >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.args b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.args >> new file mode 100644 >> index 000000000..3ba5c1160 >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.args >> @@ -0,0 +1,9 @@ >> +/usr/sbin/bhyve \ >> +-c 1 \ >> +-m 214 \ >> +-u \ >> +-H \ >> +-P \ >> +-s 0:0,hostbridge \ >> +-s 2:0,ahci,hd:/tmp/freebsd.img \ >> +-s 3:0,virtio-net,faketapdev,mac=52:54:00:ee:f5:79 bhyve >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.devmap b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.devmap >> new file mode 100644 >> index 000000000..b312bfdaf >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.devmap >> @@ -0,0 +1 @@ >> +(hd0) /tmp/freebsd.img >> \ No newline at end of file >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.ldargs >> new file mode 100644 >> index 000000000..7d9a5155a >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.ldargs >> @@ -0,0 +1,4 @@ >> +/usr/local/sbin/grub-bhyve \ >> +--root hd0,msdos1 \ >> +--device-map '<device.map>' \ >> +--memory 214 bhyve >> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.xml >> new file mode 100644 >> index 000000000..03b6987fd >> --- /dev/null >> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-stdin-oneline.xml >> @@ -0,0 +1,25 @@ >> +<domain type='bhyve'> >> + <name>bhyve</name> >> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> >> + <memory>219136</memory> >> + <vcpu>1</vcpu> >> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> >> + <bootloader_stdin>some input commands</bootloader_stdin> >> + <os> >> + <type>hvm</type> >> + </os> >> + <devices> >> + <disk type='file'> >> + <driver name='file' type='raw'/> >> + <source file='/tmp/freebsd.img'/> >> + <target dev='hda' bus='sata'/> >> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> >> + </disk> >> + <interface type='bridge'> >> + <mac address='52:54:00:ee:f5:79'/> >> + <model type='virtio'/> >> + <source bridge="virbr0"/> >> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> >> + </interface> >> + </devices> >> +</domain> >> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c >> index 6f3b0c2eb..e4cb0592e 100644 >> --- a/tests/bhyvexml2argvtest.c >> +++ b/tests/bhyvexml2argvtest.c >> @@ -188,6 +188,9 @@ mymain(void) >> DO_TEST("grub-defaults"); >> DO_TEST("grub-bootorder"); >> DO_TEST("grub-bootorder2"); >> + DO_TEST("grub-stdin-file"); >> + DO_TEST("grub-stdin-oneline"); >> + DO_TEST("grub-stdin-multiline"); >> DO_TEST("bhyveload-bootorder"); >> DO_TEST("bhyveload-bootorder1"); >> DO_TEST_FAILURE("bhyveload-bootorder2"); >> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-file.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-file.xml >> new file mode 100644 >> index 000000000..f07368d01 >> --- /dev/null >> +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-file.xml >> @@ -0,0 +1,34 @@ >> +<domain type='bhyve'> >> + <name>bhyve</name> >> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> >> + <memory unit='KiB'>219136</memory> >> + <currentMemory unit='KiB'>219136</currentMemory> >> + <vcpu placement='static'>1</vcpu> >> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> >> + <bootloader_stdin file="/path/to/some/file"/> >> + <os> >> + <type arch='x86_64'>hvm</type> >> + </os> >> + <clock offset='utc'/> >> + <on_poweroff>destroy</on_poweroff> >> + <on_reboot>restart</on_reboot> >> + <on_crash>destroy</on_crash> >> + <devices> >> + <disk type='file' device='disk'> >> + <driver name='file' type='raw'/> >> + <source file='/tmp/freebsd.img'/> >> + <target dev='hda' bus='sata'/> >> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> >> + </disk> >> + <controller type='pci' index='0' model='pci-root'/> >> + <controller type='sata' index='0'> >> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> >> + </controller> >> + <interface type='bridge'> >> + <mac address='52:54:00:ee:f5:79'/> >> + <source bridge='virbr0'/> >> + <model type='virtio'/> >> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> >> + </interface> >> + </devices> >> +</domain> >> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-multiline.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-multiline.xml >> new file mode 100644 >> index 000000000..eae6df4b4 >> --- /dev/null >> +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-multiline.xml >> @@ -0,0 +1,39 @@ >> +<domain type='bhyve'> >> + <name>bhyve</name> >> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> >> + <memory unit='KiB'>219136</memory> >> + <currentMemory unit='KiB'>219136</currentMemory> >> + <vcpu placement='static'>1</vcpu> >> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> >> + <bootloader_stdin><![CDATA[ >> +multiple >> +boot >> +loader >> +commands >> +]]></bootloader_stdin> >> + <os> >> + <type arch='x86_64'>hvm</type> >> + </os> >> + <clock offset='utc'/> >> + <on_poweroff>destroy</on_poweroff> >> + <on_reboot>restart</on_reboot> >> + <on_crash>destroy</on_crash> >> + <devices> >> + <disk type='file' device='disk'> >> + <driver name='file' type='raw'/> >> + <source file='/tmp/freebsd.img'/> >> + <target dev='hda' bus='sata'/> >> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> >> + </disk> >> + <controller type='pci' index='0' model='pci-root'/> >> + <controller type='sata' index='0'> >> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> >> + </controller> >> + <interface type='bridge'> >> + <mac address='52:54:00:ee:f5:79'/> >> + <source bridge='virbr0'/> >> + <model type='virtio'/> >> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> >> + </interface> >> + </devices> >> +</domain> >> diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-oneline.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-oneline.xml >> new file mode 100644 >> index 000000000..b038a9065 >> --- /dev/null >> +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-grub-stdin-oneline.xml >> @@ -0,0 +1,34 @@ >> +<domain type='bhyve'> >> + <name>bhyve</name> >> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> >> + <memory unit='KiB'>219136</memory> >> + <currentMemory unit='KiB'>219136</currentMemory> >> + <vcpu placement='static'>1</vcpu> >> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> >> + <bootloader_stdin>some input commands</bootloader_stdin> >> + <os> >> + <type arch='x86_64'>hvm</type> >> + </os> >> + <clock offset='utc'/> >> + <on_poweroff>destroy</on_poweroff> >> + <on_reboot>restart</on_reboot> >> + <on_crash>destroy</on_crash> >> + <devices> >> + <disk type='file' device='disk'> >> + <driver name='file' type='raw'/> >> + <source file='/tmp/freebsd.img'/> >> + <target dev='hda' bus='sata'/> >> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> >> + </disk> >> + <controller type='pci' index='0' model='pci-root'/> >> + <controller type='sata' index='0'> >> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> >> + </controller> >> + <interface type='bridge'> >> + <mac address='52:54:00:ee:f5:79'/> >> + <source bridge='virbr0'/> >> + <model type='virtio'/> >> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> >> + </interface> >> + </devices> >> +</domain> >> diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c >> index 4d9c1681d..fd386b504 100644 >> --- a/tests/bhyvexml2xmltest.c >> +++ b/tests/bhyvexml2xmltest.c >> @@ -98,6 +98,9 @@ mymain(void) >> DO_TEST_DIFFERENT("grub-bootorder"); >> DO_TEST_DIFFERENT("grub-bootorder2"); >> DO_TEST_DIFFERENT("grub-defaults"); >> + DO_TEST_DIFFERENT("grub-stdin-file"); >> + DO_TEST_DIFFERENT("grub-stdin-oneline"); >> + DO_TEST_DIFFERENT("grub-stdin-multiline"); >> DO_TEST_DIFFERENT("localtime"); >> DO_TEST_DIFFERENT("macaddr"); >> DO_TEST_DIFFERENT("metadata"); >>