Re: [libvirt] [PATCH V2] Do not drop kernel cmdline for xen pv domains

2011-07-11 Thread Jim Fehlig
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 @@
/define
define name=osbootkernel
  interleave
 -  element name=kernel
 -ref name=absFilePath/
 -  /element
 +  optional
 +element name=kernel
 +  ref name=absFilePath/
 +/element
 +  /optional
optional
  element name=initrd
ref name=absFilePath/

 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

2011-07-08 Thread Matthias Bolte
2011/7/8 Jim Fehlig jfeh...@novell.com:
 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 @@
 +domain type='xen' id='6'
 +  namepvtest/name
 +  uuid596a5d21-71f4-8fb2-e068-e2386a5c413e/uuid
 +  memory430080/memory
 +  currentMemory430080/currentMemory
 +  vcpu2/vcpu
 +  bootloader/usr/bin/pygrub/bootloader
 +  bootloader_args-q/bootloader_args
 +  os
 +    typelinux/type
 +    cmdline xenfb.video=8,1280,1024 /cmdline

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

2011-07-07 Thread Jim Fehlig
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 @@
 +domain type='xen' id='6'
 +  namepvtest/name
 +  uuid596a5d21-71f4-8fb2-e068-e2386a5c413e/uuid
 +  memory430080/memory
 +  currentMemory430080/currentMemory
 +  vcpu2/vcpu
 +  bootloader/usr/bin/pygrub/bootloader
 +  bootloader_args-q/bootloader_args
 +  os
 +typelinux/type
 +cmdline xenfb.video=8,1280,1024 /cmdline
 +  /os
 +  clock offset='utc'/
 +  on_poweroffdestroy/on_poweroff
 +  on_rebootdestroy/on_reboot
 +  on_crashdestroy/on_crash
 +  devices
 +disk type='file' device='disk'
 +  driver name='file'/
 +  source file='/root/some.img'/
 +  target dev='xvda' bus='xen'/
 +/disk
 +console type='pty'
 +  target type='xen' port='0'/
 +/console
 +  /devices
 +/domain
 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