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

2018-06-03 Thread Roman Bogorodskiy 
  Fabian Freyer wrote:

> On 26 Apr 2018, at 18:38, John Ferlan wrote:
> 
> > On 04/13/2018 03:27 PM, Fabian Freyer wrote:
> >> This commit adds the  node to the domain definition,
> >> with the following semantics:
> >>
> >> To pass standard input verbatim to the bootloader, set
> >>
> >> some stdin
> >>
> >> Multiline standard input can be set using a CDATA tag:
> >>
> >> 
> >>
> >> Standard input can be read from a file as follows:
> >>
> >> 
> >
> > 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.

If there are just a line or two to be passed to loader's stdin, being
able to specify that right in the domain xml feels more convenient than
creating a 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 
> >> ---
> >>  docs/formatdomain.html.in  | 19 ++

Would be good to add some example to drvbhyve.html.in as well.

> >>  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 
> >>

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

2018-04-26 Thread Fabian Freyer 


On 26 Apr 2018, at 18:38, John Ferlan wrote:

> On 04/13/2018 03:27 PM, Fabian Freyer wrote:
>> This commit adds the  node to the domain definition,
>> with the following semantics:
>>
>> To pass standard input verbatim to the bootloader, set
>>
>> some stdin
>>
>> Multiline standard input can be set using a CDATA tag:
>>
>> 
>>
>> Standard input can be read from a file as follows:
>>
>> 
>
> 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 
>> ---
>>  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

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

2018-04-26 Thread John Ferlan 


On 04/13/2018 03:27 PM, Fabian Freyer wrote:
> This commit adds the  node to the domain definition,
> with the following semantics:
> 
> To pass standard input verbatim to the bootloader, set
> 
> some stdin
> 
> Multiline standard input can be set using a CDATA tag:
> 
> 
> 
> Standard input can be read from a file as follows:
> 
> 

Not my area of expertise, but some feedback to hopefully help a bit
seeing as there have been no other takes for 2 weeks.

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?
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.

> 
> Signed-off-by: Fabian Freyer 
> ---
>  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 
>

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

2018-04-15 Thread Fabian Freyer 
This commit adds the  node to the domain definition,
with the following semantics:

To pass standard input verbatim to the bootloader, set

some stdin

Multiline standard input can be set using a CDATA tag:



Standard input can be read from a file as follows:



Signed-off-by: Fabian Freyer 
---
 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

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 @@
 ...
 bootloader/usr/bin/pygrub/bootloader
 bootloader_args--append single/bootloader_args
+bootloader_stdin![CDATA[
+kernel (hd)/path/to/kernel
+initrd (host)/path/to/initrd
+boot
+]]
 ...
 
 
@@ -259,6 +264,20 @@
 command line arguments to be passed to the bootloader.
 Since 0.2.3
 
+  bootloader_stdin
+  The optional bootloader_stdin 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
+file