Re: [libvirt] [PATCH V2] Do not drop kernel cmdline for xen pv domains
Eric Blake wrote: > On 07/07/2011 05:37 PM, Jim Fehlig wrote: > >> Jim Fehlig wrote: >> >>> Kernel cmdline args can be passed to xen pv domains even when a >>> bootloader is specified. The current config-to-sxpr mapping >>> ignores cmdline when bootloader is present. >>> >>> Since the xend sub-driver is used with many xen toolstack versions, >>> this patch takes conservative approach of adding an else block to >>> existing !def->os.bootloader, and only appends sxpr if def->os.cmdline >>> is non-NULL. >>> >>> V2: Fix existing testcase broken by this patch and add new testcases >>> >>> >> Hmm, now domainschematest is failing >> >> on these two xml files. If I'm reading domain.rng correctly, kernel must >> be specified?? Can it be optional like initrd and cmdline? >> > > Reading domain_conf.c agrees with that interpretation. I would be fine > with you squashing this in: > > diff --git i/docs/schemas/domain.rng w/docs/schemas/domain.rng > index c01801e..b659da9 100644 > --- i/docs/schemas/domain.rng > +++ w/docs/schemas/domain.rng > @@ -553,9 +553,11 @@ > > > > - > - > - > + > + > + > + > + > > > > > ACK, once you fix that and address Matthias' comment about spacing in > the .xml file: > Squashed in your change to domain.rng, removed spaces noted by Matthias, and pushed. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2] Do not drop kernel cmdline for xen pv domains
On 07/07/2011 05:37 PM, Jim Fehlig wrote: > Jim Fehlig wrote: >> Kernel cmdline args can be passed to xen pv domains even when a >> bootloader is specified. The current config-to-sxpr mapping >> ignores cmdline when bootloader is present. >> >> Since the xend sub-driver is used with many xen toolstack versions, >> this patch takes conservative approach of adding an else block to >> existing !def->os.bootloader, and only appends sxpr if def->os.cmdline >> is non-NULL. >> >> V2: Fix existing testcase broken by this patch and add new testcases >> > > Hmm, now domainschematest is failing > > on these two xml files. If I'm reading domain.rng correctly, kernel must > be specified?? Can it be optional like initrd and cmdline? Reading domain_conf.c agrees with that interpretation. I would be fine with you squashing this in: diff --git i/docs/schemas/domain.rng w/docs/schemas/domain.rng index c01801e..b659da9 100644 --- i/docs/schemas/domain.rng +++ w/docs/schemas/domain.rng @@ -553,9 +553,11 @@ - - - + + + + + ACK, once you fix that and address Matthias' comment about spacing in the .xml file: >> + xenfb.video=8,1280,1024 -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2] Do not drop kernel cmdline for xen pv domains
2011/7/8 Jim Fehlig : > Kernel cmdline args can be passed to xen pv domains even when a > bootloader is specified. The current config-to-sxpr mapping > ignores cmdline when bootloader is present. > > Since the xend sub-driver is used with many xen toolstack versions, > this patch takes conservative approach of adding an else block to > existing !def->os.bootloader, and only appends sxpr if def->os.cmdline > is non-NULL. > > V2: Fix existing testcase broken by this patch and add new testcases > --- > +++ b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml > @@ -0,0 +1,27 @@ > + > + pvtest > + 596a5d21-71f4-8fb2-e068-e2386a5c413e > + 430080 > + 430080 > + 2 > + /usr/bin/pygrub > + -q > + > + linux > + xenfb.video=8,1280,1024 Are the leading and trailing spaces essential? If not I'd suggest to remove them. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2] Do not drop kernel cmdline for xen pv domains
Jim Fehlig wrote:
> Kernel cmdline args can be passed to xen pv domains even when a
> bootloader is specified. The current config-to-sxpr mapping
> ignores cmdline when bootloader is present.
>
> Since the xend sub-driver is used with many xen toolstack versions,
> this patch takes conservative approach of adding an else block to
> existing !def->os.bootloader, and only appends sxpr if def->os.cmdline
> is non-NULL.
>
> V2: Fix existing testcase broken by this patch and add new testcases
>
Hmm, now domainschematest is failing
> ---
> src/xenxs/xen_sxpr.c |6
> .../sexpr2xml-pv-bootloader-cmdline.sexpr |5 +++
> .../sexpr2xml-pv-bootloader-cmdline.xml| 27
>
> tests/sexpr2xmltest.c |1 +
> .../xml2sexpr-disk-block-shareable.sexpr |1 +
> .../xml2sexpr-pv-bootloader-cmdline.sexpr |5 +++
> .../xml2sexpr-pv-bootloader-cmdline.xml| 22
> tests/xml2sexprtest.c |1 +
> 8 files changed, 68 insertions(+), 0 deletions(-)
> create mode 100644 tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.sexpr
> create mode 100644 tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml
> create mode 100644 tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.sexpr
> create mode 100644 tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.xml
>
on these two xml files. If I'm reading domain.rng correctly, kernel must
be specified?? Can it be optional like initrd and cmdline?
Regards,
Jim
> diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
> index 13ca015..bd770bc 100644
> --- a/src/xenxs/xen_sxpr.c
> +++ b/src/xenxs/xen_sxpr.c
> @@ -2261,6 +2261,12 @@ xenFormatSxpr(virConnectPtr conn,
> }
>
> virBufferAddLit(&buf, "))");
> +} else {
> +/* PV domains accept kernel cmdline args */
> +if (def->os.cmdline) {
> +virBufferEscapeSexpr(&buf, "(image (linux (args '%s')))",
> + def->os.cmdline);
> +}
> }
>
> for (i = 0 ; i < def->ndisks ; i++)
> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.sexpr
> b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.sexpr
> new file mode 100644
> index 000..dde37fc
> --- /dev/null
> +++ b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.sexpr
> @@ -0,0 +1,5 @@
> +(domain (domid 6)(name 'pvtest')(memory 420)(maxmem 420)(vcpus 2)\
> +(uuid '596a5d2171f48fb2e068e2386a5c413e')(bootloader '/usr/bin/pygrub')\
> +(bootloader_args '-q')(image (linux (args ' xenfb.video=8,1280,1024 ')))\
> +(on_poweroff 'destroy')(on_reboot 'destroy')(on_crash 'destroy')\
> +(device (vbd (dev 'xvda')(uname 'file:/root/some.img')(mode 'w'
> diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml
> b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml
> new file mode 100644
> index 000..7bc99d8
> --- /dev/null
> +++ b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml
> @@ -0,0 +1,27 @@
> +
> + pvtest
> + 596a5d21-71f4-8fb2-e068-e2386a5c413e
> + 430080
> + 430080
> + 2
> + /usr/bin/pygrub
> + -q
> +
> +linux
> + xenfb.video=8,1280,1024
> +
> +
> + destroy
> + destroy
> + destroy
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> diff --git a/tests/sexpr2xmltest.c b/tests/sexpr2xmltest.c
> index f393935..26a987d 100644
> --- a/tests/sexpr2xmltest.c
> +++ b/tests/sexpr2xmltest.c
> @@ -134,6 +134,7 @@ mymain(void)
> DO_TEST("pv-vfb-type-crash", "pv-vfb-type-crash", 3);
> DO_TEST("fv-autoport", "fv-autoport", 3);
> DO_TEST("pv-bootloader", "pv-bootloader", 1);
> +DO_TEST("pv-bootloader-cmdline", "pv-bootloader-cmdline", 1);
> DO_TEST("pv-vcpus", "pv-vcpus", 1);
>
> DO_TEST("disk-file", "disk-file", 2);
> diff --git a/tests/xml2sexprdata/xml2sexpr-disk-block-shareable.sexpr
> b/tests/xml2sexprdata/xml2sexpr-disk-block-shareable.sexpr
> index 8c0b1cd..b8387e5 100644
> --- a/tests/xml2sexprdata/xml2sexpr-disk-block-shareable.sexpr
> +++ b/tests/xml2sexprdata/xml2sexpr-disk-block-shareable.sexpr
> @@ -1,6 +1,7 @@
> (vm (name 'pvtest')(memory 384)(maxmem 512)(vcpus 1)\
> (uuid '49a0c6ff-c066-5392-6498-3632d093c2e7')(bootloader '/usr/bin/pygrub')\
> (on_poweroff 'destroy')(on_reboot 'restart')(on_crash 'restart')\
> +(image (linux (args 'ro root=/dev/VolGroup00/LogVol00')))\
> (device (tap (dev 'xvda')(uname 'tap:aio:/var/lib/xen/images/rhel5pv.img')\
> (mode 'w!')))(device (vif (mac '00:16:3e:23:9e:eb')(bridge 'xenbr0')\
> (script 'vif-bridge'\
> diff --git a/tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.sexpr
> b/tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.sexpr
> new file mode 100644
> index 000..c3ed727
> --- /dev/null
> +++ b/tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.sexpr
> @@ -0,0 +1,5 @@
> +(vm (name 'pvtest')(me
[libvirt] [PATCH V2] Do not drop kernel cmdline for xen pv domains
Kernel cmdline args can be passed to xen pv domains even when a
bootloader is specified. The current config-to-sxpr mapping
ignores cmdline when bootloader is present.
Since the xend sub-driver is used with many xen toolstack versions,
this patch takes conservative approach of adding an else block to
existing !def->os.bootloader, and only appends sxpr if def->os.cmdline
is non-NULL.
V2: Fix existing testcase broken by this patch and add new testcases
---
src/xenxs/xen_sxpr.c |6
.../sexpr2xml-pv-bootloader-cmdline.sexpr |5 +++
.../sexpr2xml-pv-bootloader-cmdline.xml| 27
tests/sexpr2xmltest.c |1 +
.../xml2sexpr-disk-block-shareable.sexpr |1 +
.../xml2sexpr-pv-bootloader-cmdline.sexpr |5 +++
.../xml2sexpr-pv-bootloader-cmdline.xml| 22
tests/xml2sexprtest.c |1 +
8 files changed, 68 insertions(+), 0 deletions(-)
create mode 100644 tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.sexpr
create mode 100644 tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml
create mode 100644 tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.sexpr
create mode 100644 tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.xml
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index 13ca015..bd770bc 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -2261,6 +2261,12 @@ xenFormatSxpr(virConnectPtr conn,
}
virBufferAddLit(&buf, "))");
+} else {
+/* PV domains accept kernel cmdline args */
+if (def->os.cmdline) {
+virBufferEscapeSexpr(&buf, "(image (linux (args '%s')))",
+ def->os.cmdline);
+}
}
for (i = 0 ; i < def->ndisks ; i++)
diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.sexpr
b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.sexpr
new file mode 100644
index 000..dde37fc
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.sexpr
@@ -0,0 +1,5 @@
+(domain (domid 6)(name 'pvtest')(memory 420)(maxmem 420)(vcpus 2)\
+(uuid '596a5d2171f48fb2e068e2386a5c413e')(bootloader '/usr/bin/pygrub')\
+(bootloader_args '-q')(image (linux (args ' xenfb.video=8,1280,1024 ')))\
+(on_poweroff 'destroy')(on_reboot 'destroy')(on_crash 'destroy')\
+(device (vbd (dev 'xvda')(uname 'file:/root/some.img')(mode 'w'
diff --git a/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml
b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml
new file mode 100644
index 000..7bc99d8
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-pv-bootloader-cmdline.xml
@@ -0,0 +1,27 @@
+
+ pvtest
+ 596a5d21-71f4-8fb2-e068-e2386a5c413e
+ 430080
+ 430080
+ 2
+ /usr/bin/pygrub
+ -q
+
+linux
+ xenfb.video=8,1280,1024
+
+
+ destroy
+ destroy
+ destroy
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/sexpr2xmltest.c b/tests/sexpr2xmltest.c
index f393935..26a987d 100644
--- a/tests/sexpr2xmltest.c
+++ b/tests/sexpr2xmltest.c
@@ -134,6 +134,7 @@ mymain(void)
DO_TEST("pv-vfb-type-crash", "pv-vfb-type-crash", 3);
DO_TEST("fv-autoport", "fv-autoport", 3);
DO_TEST("pv-bootloader", "pv-bootloader", 1);
+DO_TEST("pv-bootloader-cmdline", "pv-bootloader-cmdline", 1);
DO_TEST("pv-vcpus", "pv-vcpus", 1);
DO_TEST("disk-file", "disk-file", 2);
diff --git a/tests/xml2sexprdata/xml2sexpr-disk-block-shareable.sexpr
b/tests/xml2sexprdata/xml2sexpr-disk-block-shareable.sexpr
index 8c0b1cd..b8387e5 100644
--- a/tests/xml2sexprdata/xml2sexpr-disk-block-shareable.sexpr
+++ b/tests/xml2sexprdata/xml2sexpr-disk-block-shareable.sexpr
@@ -1,6 +1,7 @@
(vm (name 'pvtest')(memory 384)(maxmem 512)(vcpus 1)\
(uuid '49a0c6ff-c066-5392-6498-3632d093c2e7')(bootloader '/usr/bin/pygrub')\
(on_poweroff 'destroy')(on_reboot 'restart')(on_crash 'restart')\
+(image (linux (args 'ro root=/dev/VolGroup00/LogVol00')))\
(device (tap (dev 'xvda')(uname 'tap:aio:/var/lib/xen/images/rhel5pv.img')\
(mode 'w!')))(device (vif (mac '00:16:3e:23:9e:eb')(bridge 'xenbr0')\
(script 'vif-bridge'\
diff --git a/tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.sexpr
b/tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.sexpr
new file mode 100644
index 000..c3ed727
--- /dev/null
+++ b/tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.sexpr
@@ -0,0 +1,5 @@
+(vm (name 'pvtest')(memory 420)(maxmem 420)(vcpus 2)\
+(uuid '596a5d21-71f4-8fb2-e068-e2386a5c413e')(bootloader '/usr/bin/pygrub')\
+(bootloader_args '-q')(on_poweroff 'destroy')(on_reboot 'destroy')\
+(on_crash 'destroy')(image (linux (args ' xenfb.video=8,1280,1024 ')))\
+(device (vbd (dev 'xvda')(uname 'file:/root/some.img')(mode 'w'\
diff --git a/tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.xml
b/tests/xml2sexprdata/xml2sexpr-pv-bootloader-cmdline.xml
new f
