Add extra parameters for qemu script

Add extra parameters for qemu script

[Ke, Liping]

[-- Attachment #1: Type: text/plain, Size: 805 bytes --]

Hi, Scott

The patch is in the attachment for your review. Below is some notes:

1) Basically I wouldn't like to change any logic of the original code.
2) -serial stdio and -nographic options are removed since they're be covered by the extra parameters.
3) User input would be $poky-qemu qemux86 "<-nographic -m 300>"
4) -m input will be checked still. If it exceeds 128 for arm, it will be changed back to
   128M, same logic as before. And after parsing, -m option will be removed and replaced by
    Kernel options mem=128M for avoiding some instability issue.

Generally I modified very few, just add an extra parameters with least intrusion of
Current logic.

Any comments are welcomed. 
I will conduct more test with latest code in parallel.

Thanks a lot for your help!
 criping

[-- Attachment #2: qemu_extra_options.patch --]
[-- Type: application/octet-stream, Size: 3652 bytes --]

diff --git a/scripts/poky-qemu b/scripts/poky-qemu
index bc312e0..dcaf6a3 100755
--- a/scripts/poky-qemu
+++ b/scripts/poky-qemu
@@ -27,13 +27,13 @@ usage() {
     echo "  ROOTFS - the rootfs image file or nfsroot directory to use"
     echo "  MACHINE=xyz - the machine name (optional, autodetected from KERNEL filename if unspecified)"
     echo "  Additional QEMU command-line options can be passed with:"
-    echo "    nographic - disables video console"
-    echo "    serial - enables a serial console on /dev/ttyS0"
+    echo "    \"<extra-qemu-options>\" - enables extra qemu options"
     echo ""
     echo "Examples:"
     echo "  $0 qemuarm"
     echo "  $0 qemux86-64 poky-image-sato ext3"
     echo "  $0 path/to/bzImage-qemux86.bin path/to/nfsrootdir/ serial"
+    echo "  $0 qemux86 \"<-nographic -m 256 -serial stdio>\""
     exit 1
 }
 
@@ -46,7 +46,7 @@ KERNEL=""
 FSTYPE=""
 ROOTFS=""
 LAZY_ROOTFS=""
-SCRIPT_QEMU_OPT=""
+SCRIPT_QEMU_EXTRA_OPT=""
 SCRIPT_KERNEL_OPT=""
 
 TMPDIR=""
@@ -117,12 +117,13 @@ while [ $i -le $# ]; do
                 usage
             fi
             ;;
-        "nographic")
-            SCRIPT_QEMU_OPT="$SCRIPT_QEMU_OPT -nographic"
-            ;;
-        "serial")
-            SCRIPT_QEMU_OPT="$SCRIPT_QEMU_OPT -serial stdio"
-            SCRIPT_KERNEL_OPT="$SCRIPT_KERNEL_OPT console=ttyS0"
+        \<*\>)
+            SCRIPT_QEMU_EXTRA_OPT=$arg
+            serial_option=`expr "$SCRIPT_QEMU_EXTRA_OPT" : '.*\(-serial[[:space:]] *stdio\)'`
+#if -serial stdio option is added, we need to add an extra option in $SCRIPT_KERNEL_OPT
+            if [ ! -z "$serial_option" ]; then
+              SCRIPT_KERNEL_OPT="$SCRIPT_KERNEL_OPT console=ttyS0"
+            fi
             ;;
         *)
             # A directory name is an nfs rootfs
diff --git a/scripts/poky-qemu-internal b/scripts/poky-qemu-internal
index 62c1040..ea39786 100755
--- a/scripts/poky-qemu-internal
+++ b/scripts/poky-qemu-internal
@@ -29,6 +29,23 @@
 #   ROOTFS - the disk image file to use
 #
 
+mem_size=-1
+
+#Get rid of <> and get the contents of extra qemu running params
+SCRIPT_QEMU_EXTRA_OPT=`echo $SCRIPT_QEMU_EXTRA_OPT | sed -e 's/<//' -e 's/>//'`
+#if user set qemu memory, eg: -m 256 in qemu extra params, we need to do some 
+# validation check
+mem_set=`expr "$SCRIPT_QEMU_EXTRA_OPT" : '.*\(-m[[:space:]] *[0-9]*\)'`
+if [ ! -z "$mem_set" ] ; then
+#Get memory setting size from user input
+  mem_size=`echo $mem_set | sed 's/-m[[:space:]] *//'`
+  echo "$SCRIPT_QEMU_EXTRA_OPT"
+fi
+
+if [ $mem_size -gt 0 ]; then
+  QEMU_MEMORY="$mem_size"M
+fi
+
 if [ -z "$QEMU_MEMORY" ]; then
     case "$MACHINE" in
         "qemux86")
@@ -60,6 +77,7 @@ if [ "$MACHINE" = "qemuarm" ]; then
         echo "WARNING: qemuarm does not support > 128M of RAM."
         echo "*** Changing QEMU_MEMORY to default of 128M ***"
         QEMU_MEMORY="128M"
+        SCRIPT_QEMU_EXTRA_OPT=`echo $SCRIPT_QEMU_EXTRA_OPT | sed -e "s/$mem_set/-m 128/" `
     fi
 fi
 
@@ -441,8 +459,8 @@ fi
 
 echo "Running $QEMU..."
 # -no-reboot is a mandatory option - see bug #100
-echo $QEMUBIN -kernel $KERNEL $QEMUOPTIONS $SERIALOPTS -no-reboot $SCRIPT_QEMU_CMDLINE_OPT --append '"'$KERNCMDLINE $SCRIPT_KERNEL_OPT'"'
-$QEMUBIN -kernel $KERNEL $QEMUOPTIONS $SERIALOPTS -no-reboot $SCRIPT_QEMU_OPT --append "$KERNCMDLINE $SCRIPT_KERNEL_OPT" || /bin/true
+echo $QEMUBIN -kernel $KERNEL $QEMUOPTIONS $SERIALOPTS -no-reboot $SCRIPT_QEMU_EXTRA_OPT --append '"'$KERNCMDLINE $SCRIPT_KERNEL_OPT'"'
+$QEMUBIN -kernel $KERNEL $QEMUOPTIONS $SERIALOPTS -no-reboot $SCRIPT_QEMU_EXTRA_OPT --append "$KERNCMDLINE $SCRIPT_KERNEL_OPT" || /bin/true
 
 cleanup
 

Re: Add extra parameters for qemu script

[Scott Garman]

On 12/09/2010 12:44 AM, Ke, Liping wrote:
> Hi, Scott
>
> The patch is in the attachment for your review. Below is some notes:
>
> 1) Basically I wouldn't like to change any logic of the original code.
> 2) -serial stdio and -nographic options are removed since they're be covered by the extra parameters.
> 3) User input would be $poky-qemu qemux86 "<-nographic -m 300>"
> 4) -m input will be checked still. If it exceeds 128 for arm, it will be changed back to
>     128M, same logic as before. And after parsing, -m option will be removed and replaced by
>      Kernel options mem=128M for avoiding some instability issue.
>
> Generally I modified very few, just add an extra parameters with least intrusion of
> Current logic.
>
> Any comments are welcomed.
> I will conduct more test with latest code in parallel.

Hi Criping,

Thanks for the patch. Overall I think this looks good.

However, one thing I feel the need to run by Richard, as he expressed 
some preferences with how the poky-qemu script would work with regard to 
options.

Richard: Criping's patch would remove the standalone options we had to 
the poky-qemu script (e.g, nographic, serial) and instead requires the 
user to specify them in one command option which can take any qemu 
command switch.

So for example:

poky-qemu qemux86 nographic

would become:

poky-qemu qemux86 "<-nographic>"

The benefit of this is that this allows the user to specify any qemu 
option they wish. Previously they were limited by the options we allowed 
them to specify (which were quite limited).

I tend to feel that this approach is more flexible, and scales better 
than having to support each and every qemu option with our own script 
syntax. Is this acceptable, or should we continue to support our own 
custom options in addition to Criping's new approach?

Scott

-- 
Scott Garman
Embedded Linux Distro Engineer - Yocto Project

Re: Add extra parameters for qemu script

[Zhang, Jessica]

[-- Attachment #1: Type: text/plain, Size: 2316 bytes --]

Garman, Scott A wrote:
> On 12/09/2010 12:44 AM, Ke, Liping wrote:
>> Hi, Scott
>> 
>> The patch is in the attachment for your review. Below is some notes:
>> 
>> 1) Basically I wouldn't like to change any logic of the original
>> code. 2) -serial stdio and -nographic options are removed since
>> they're be covered by the extra parameters. 3) User input would be
>> $poky-qemu qemux86 "<-nographic -m 300>" 4) -m input will be checked
>>     still. If it exceeds 128 for arm, it will be changed back to
>>      128M, same logic as before. And after parsing, -m option will
>> be removed and replaced by Kernel options mem=128M for avoiding some
>> instability issue.  
>> 
>> Generally I modified very few, just add an extra parameters with
>> least intrusion of 
>> Current logic.
>> 
>> Any comments are welcomed.
>> I will conduct more test with latest code in parallel.
> 
> Hi Criping,
> 
> Thanks for the patch. Overall I think this looks good.
> 
> However, one thing I feel the need to run by Richard, as he expressed
> some preferences with how the poky-qemu script would work with regard
> to options.
> 
> Richard: Criping's patch would remove the standalone options we had to
> the poky-qemu script (e.g, nographic, serial) and instead requires the
> user to specify them in one command option which can take any qemu
> command switch.
> 
> So for example:
> 
> poky-qemu qemux86 nographic
> 
> would become:
> 
> poky-qemu qemux86 "<-nographic>"
> 
> The benefit of this is that this allows the user to specify any qemu
> option they wish. Previously they were limited by the options we
> allowed them to specify (which were quite limited).
> 
> I tend to feel that this approach is more flexible, and scales better
> than having to support each and every qemu option with our own script
> syntax. Is this acceptable, or should we continue to support our own
> custom options in addition to Criping's new approach?

Liping and I had the discussion regarding this before she went on taking the
approach.  Since we now provide this capability to allow user to provide
their qemu params, then why we still in our qemu scripts isolate certain
qemu params out, which may make things complicated unnecessarily and be more
error prone...
> 
> Scott

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 8455 bytes --]

Re: Add extra parameters for qemu script

[Richard Purdie]

On Thu, 2010-12-09 at 12:44 -0800, Garman, Scott A wrote:
> However, one thing I feel the need to run by Richard, as he expressed 
> some preferences with how the poky-qemu script would work with regard to 
> options.
> 
> Richard: Criping's patch would remove the standalone options we had to 
> the poky-qemu script (e.g, nographic, serial) and instead requires the 
> user to specify them in one command option which can take any qemu 
> command switch.
> 
> So for example:
> 
> poky-qemu qemux86 nographic
> 
> would become:
> 
> poky-qemu qemux86 "<-nographic>"
> 
> The benefit of this is that this allows the user to specify any qemu 
> option they wish. Previously they were limited by the options we allowed 
> them to specify (which were quite limited).
> 
> I tend to feel that this approach is more flexible, and scales better 
> than having to support each and every qemu option with our own script 
> syntax. Is this acceptable, or should we continue to support our own 
> custom options in addition to Criping's new approach?

My gut feeling is that having some simplified way to trigger possibly
complex option combinations is still desirable but adding a way to pass
additional custom commandline is equally good. This gives us the maximum
flexibility moving forwards but keeps the script easy to use?

Cheers,

Richard

Re: Add extra parameters for qemu script

[Ke, Liping]

> I tend to feel that this approach is more flexible, and scales better
> > than having to support each and every qemu option with our own script
> > syntax. Is this acceptable, or should we continue to support our own
> > custom options in addition to Criping's new approach?
> 
> My gut feeling is that having some simplified way to trigger possibly
> complex option combinations is still desirable but adding a way to pass
> additional custom commandline is equally good. This gives us the
> maximum
> flexibility moving forwards but keeps the script easy to use?
> 

Hi, Scott

So the conclusion is that I should keep the old (serial nographic) option while
adding the new "<-XXX -XXX -XXX>" option?
OK. I will send out the modified patch to you for review later.

Thanks& Regards,
criping


> Cheers,
> 
> Richard

Re: Add extra parameters for qemu script

[Scott Garman]

On 12/12/2010 05:43 PM, Ke, Liping wrote:
>> I tend to feel that this approach is more flexible, and scales better
>>> than having to support each and every qemu option with our own script
>>> syntax. Is this acceptable, or should we continue to support our own
>>> custom options in addition to Criping's new approach?
>>
>> My gut feeling is that having some simplified way to trigger possibly
>> complex option combinations is still desirable but adding a way to pass
>> additional custom commandline is equally good. This gives us the
>> maximum
>> flexibility moving forwards but keeps the script easy to use?
>>
>
> Hi, Scott
>
> So the conclusion is that I should keep the old (serial nographic) option while
> adding the new "<-XXX -XXX -XXX>" option?
> OK. I will send out the modified patch to you for review later.

Yes, please respin the patch with those changes.

Thanks!

Scott

-- 
Scott Garman
Embedded Linux Distro Engineer - Yocto Project

Re: Add extra parameters for qemu script

[Ke, Liping]

[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]

Hi, Scott

I have updated the patch and tested with poky-tree mode (arm, x86).
Since kvm and serial needs special processing, so for avoiding repeating the code, I will exclude serial and kvm in permitted extra-option, user need to use (serial, kvm) it they want to use it. 

For "<-m XXX>" options, I will keep the original logic. If it's arm, the > 128M memory will be forced back to 128M.

It's the high-level user's responsibility to make sure other params are valid.

Any problem, just let me know.

Thanks a lot for your help!

criping

> -----Original Message-----
> From: Garman, Scott A
> Sent: Monday, December 13, 2010 10:16 AM
> To: Ke, Liping
> Cc: Richard Purdie; Zhang, Jessica; Lu, Lianhao; Cui, Dexuan;
> yocto@yoctoproject.org
> Subject: Re: Add extra parameters for qemu script
> 
> On 12/12/2010 05:43 PM, Ke, Liping wrote:
> >> I tend to feel that this approach is more flexible, and scales
> better
> >>> than having to support each and every qemu option with our own
> script
> >>> syntax. Is this acceptable, or should we continue to support our
> own
> >>> custom options in addition to Criping's new approach?
> >>
> >> My gut feeling is that having some simplified way to trigger
> possibly
> >> complex option combinations is still desirable but adding a way to
> pass
> >> additional custom commandline is equally good. This gives us the
> >> maximum
> >> flexibility moving forwards but keeps the script easy to use?
> >>
> >
> > Hi, Scott
> >
> > So the conclusion is that I should keep the old (serial nographic)
> option while
> > adding the new "<-XXX -XXX -XXX>" option?
> > OK. I will send out the modified patch to you for review later.
> 
> Yes, please respin the patch with those changes.
> 
> Thanks!
> 
> Scott
> 
> --
> Scott Garman
> Embedded Linux Distro Engineer - Yocto Project

[-- Attachment #2: qemu_extra_option.patch --]
[-- Type: application/octet-stream, Size: 3917 bytes --]

diff --git a/meta/recipes-graphics/xorg-lib/libxfixes_4.0.5.bb b/meta/recipes-graphics/xorg-lib/libxfixes_4.0.5.bb
index 8ecf1f8..24da69d 100644
--- a/meta/recipes-graphics/xorg-lib/libxfixes_4.0.5.bb
+++ b/meta/recipes-graphics/xorg-lib/libxfixes_4.0.5.bb
@@ -16,3 +16,7 @@ BBCLASSEXTEND = "nativesdk"
 
 SRC_URI[md5sum] = "1b4b8386bd5d1751b2c7177223ad4629"
 SRC_URI[sha256sum] = "2e6cd020460e4ef5d5a1d9b5d21143e9f5e580036a79c7de26ae539d7bcb8d74"
+
+do_configure_prepend() {
+	echo "${PKG_CONFIG_PATH}"
+}
diff --git a/scripts/poky-qemu b/scripts/poky-qemu
index bfeefc7..da56532 100755
--- a/scripts/poky-qemu
+++ b/scripts/poky-qemu
@@ -29,11 +29,14 @@ usage() {
     echo "  Additional QEMU command-line options can be passed with:"
     echo "    nographic - disables video console"
     echo "    serial - enables a serial console on /dev/ttyS0"
+    echo "    kvm - enables kvm"
+    echo "    \"<extra-qemu-options>\" - enables extra qemu options, excluding serial and kvm"
     echo ""
     echo "Examples:"
     echo "  $MYNAME qemuarm"
     echo "  $MYNAME qemux86-64 poky-image-sato ext3"
     echo "  $MYNAME path/to/bzImage-qemux86.bin path/to/nfsrootdir/ serial"
+    echo "  $0 qemux86 \"<-m 256>\""
     exit 1
 }
 
@@ -47,6 +50,7 @@ FSTYPE=""
 ROOTFS=""
 LAZY_ROOTFS=""
 SCRIPT_QEMU_OPT=""
+SCRIPT_QEMU_EXTRA_OPT=""
 SCRIPT_KERNEL_OPT=""
 
 TMPDIR=""
@@ -129,6 +133,16 @@ while [ $i -le $# ]; do
             KVM_ENABLED="yes"
             KVM_CAPABLE=`grep 'vmx\|smx' /proc/cpuinfo`
             ;;
+        \<*\>)
+            SCRIPT_QEMU_EXTRA_OPT=$arg 
+            serial_option=`expr "$SCRIPT_QEMU_EXTRA_OPT" : '.*\(-serial\)'`
+            kvm_option=`expr "$SCRIPT_QEMU_EXTRA_OPT" : '.*\(-enable-kvm\)'`
+            echo "$kvm_option"
+            if [[ ! -z "$serial_option" || ! -z "$kvm_option" ]]; then
+                echo "Error: Please use serial or kvm params instead!"
+                usage
+            fi
+            ;;
         *)
             # A directory name is an nfs rootfs
             if [ -d "$arg" ]; then
diff --git a/scripts/poky-qemu-internal b/scripts/poky-qemu-internal
index ca2511a..5e74169 100755
--- a/scripts/poky-qemu-internal
+++ b/scripts/poky-qemu-internal
@@ -29,6 +29,23 @@
 #   ROOTFS - the disk image file to use
 #
 
+
+mem_size=-1
+
+#Get rid of <> and get the contents of extra qemu running params
+SCRIPT_QEMU_EXTRA_OPT=`echo $SCRIPT_QEMU_EXTRA_OPT | sed -e 's/<//' -e 's/>//'`
+#if user set qemu memory, eg: -m 256 in qemu extra params, we need to do some 
+# validation check
+mem_set=`expr "$SCRIPT_QEMU_EXTRA_OPT" : '.*\(-m[[:space:]] *[0-9]*\)'`
+if [ ! -z "$mem_set" ] ; then
+#Get memory setting size from user input
+  mem_size=`echo $mem_set | sed 's/-m[[:space:]] *//'`
+fi
+
+if [ $mem_size -gt 0 ]; then
+  QEMU_MEMORY="$mem_size"M
+fi
+
 if [ -z "$QEMU_MEMORY" ]; then
     case "$MACHINE" in
         "qemux86")
@@ -60,6 +77,7 @@ if [ "$MACHINE" = "qemuarm" ]; then
         echo "WARNING: qemuarm does not support > 128M of RAM."
         echo "*** Changing QEMU_MEMORY to default of 128M ***"
         QEMU_MEMORY="128M"
+        SCRIPT_QEMU_EXTRA_OPT=`echo $SCRIPT_QEMU_EXTRA_OPT | sed -e "s/$mem_set/-m 128/" `
     fi
 fi
 
@@ -427,8 +445,8 @@ fi
 
 echo "Running $QEMU..."
 # -no-reboot is a mandatory option - see bug #100
-echo $QEMUBIN -kernel $KERNEL $QEMUOPTIONS $SERIALOPTS -no-reboot $SCRIPT_QEMU_CMDLINE_OPT --append '"'$KERNCMDLINE $SCRIPT_KERNEL_OPT'"'
-$QEMUBIN -kernel $KERNEL $QEMUOPTIONS $SERIALOPTS -no-reboot $SCRIPT_QEMU_OPT --append "$KERNCMDLINE $SCRIPT_KERNEL_OPT" || /bin/true
+echo $QEMUBIN -kernel $KERNEL $QEMUOPTIONS $SERIALOPTS -no-reboot $SCRIPT_QEMU_OPT $SCRIPT_QEMU_EXTRA_OPT --append '"'$KERNCMDLINE $SCRIPT_KERNEL_OPT'"'
+$QEMUBIN -kernel $KERNEL $QEMUOPTIONS $SERIALOPTS -no-reboot $SCRIPT_QEMU_OPT $SCRIPT_QEMU_EXTRA_OPT --append "$KERNCMDLINE $SCRIPT_KERNEL_OPT" || /bin/true
 
 cleanup
 

Re: Add extra parameters for qemu script

[Scott Garman]

On 12/13/2010 07:41 PM, Ke, Liping wrote:
> Hi, Scott
>
> I have updated the patch and tested with poky-tree mode (arm, x86).
> Since kvm and serial needs special processing, so for avoiding repeating the code, I will exclude serial and kvm in permitted extra-option, user need to use (serial, kvm) it they want to use it.
>
> For "<-m XXX>" options, I will keep the original logic. If it's arm, the>  128M memory will be forced back to 128M.
>
> It's the high-level user's responsibility to make sure other params are valid.
>
> Any problem, just let me know.
>
> Thanks a lot for your help!

Thanks for the patch, Criping.

This patch includes the addition of a do_configure_prepend step for the 
libxfixes recipe? That looks like some debugging info crept in that may 
not have been intended.

In the usage() function, please use $MYNAME instead of $0 for 
consistency - see the other help echo lines.

The rest of the patch looks ok. Please respin one last time with the 
above minor changes and I'll accept it.

Thanks,

Scott
-- 
Scott Garman
Embedded Linux Distro Engineer - Yocto Project

Re: Add extra parameters for qemu script

[Scott Garman]

On 12/14/2010 02:27 PM, Scott Garman wrote:
> On 12/13/2010 07:41 PM, Ke, Liping wrote:
>> Hi, Scott
>>
>> I have updated the patch and tested with poky-tree mode (arm, x86).
>> Since kvm and serial needs special processing, so for avoiding repeating the code, I will exclude serial and kvm in permitted extra-option, user need to use (serial, kvm) it they want to use it.
>>
>> For "<-m XXX>" options, I will keep the original logic. If it's arm, the>   128M memory will be forced back to 128M.
>>
>> It's the high-level user's responsibility to make sure other params are valid.
>>
>> Any problem, just let me know.
>>
>> Thanks a lot for your help!
>
> Thanks for the patch, Criping.
>
> This patch includes the addition of a do_configure_prepend step for the
> libxfixes recipe? That looks like some debugging info crept in that may
> not have been intended.
>
> In the usage() function, please use $MYNAME instead of $0 for
> consistency - see the other help echo lines.
>
> The rest of the patch looks ok. Please respin one last time with the
> above minor changes and I'll accept it.

One final thing - when you resubmit your patch this time, could you use 
the create-pull-request/send-pull-request scripts? That way the patch 
can be pulled in by Richard or Saul with minimal effort, and it will 
help ensure you get proper credit in the commit log.

Scott

-- 
Scott Garman
Embedded Linux Distro Engineer - Yocto Project

Re: Add extra parameters for qemu script

[Ke, Liping]

Hi, Scott

Thanks for the review! Sure, I will use the script following distro-team conventions.

And also, $MYNAME will be used in the final patch. And the debug line for building will be
removed when I send the final patch.

Thanks & Regards,
criping

> One final thing - when you resubmit your patch this time, could you use
> the create-pull-request/send-pull-request scripts? That way the patch
> can be pulled in by Richard or Saul with minimal effort, and it will
> help ensure you get proper credit in the commit log.
> 
> Scott
> 
> --
> Scott Garman
> Embedded Linux Distro Engineer - Yocto Project