[Qemu-devel] [PATCH 1/5] substitute all uses of which by type
Except in one case, we are only interested in knowing if a command exist, not is path. Just create prog_exists() function that does this check. Signed-off-by: Juan Quintela --- configure | 25 ++--- 1 files changed, 14 insertions(+), 11 deletions(-) diff --git a/configure b/configure index 5631bbb..71edaf5 100755 --- a/configure +++ b/configure @@ -27,6 +27,11 @@ compile_prog() { $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null } +prog_exist() { +prog="$1" +type $prog > /dev/null 2> /dev/null +} + # default parameters cpu="" prefix="" @@ -763,21 +768,19 @@ fi # Solaris specific configure tool chain decisions # if test "$solaris" = "yes" ; then - solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"` - if test -z "$solinst" ; then + if ! prog_exist "$install" ; then echo "Solaris install program not found. Use --install=/usr/ucb/install or" echo "install fileutils from www.blastwave.org using pkg-get -i fileutils" echo "to get ginstall which is used by default (which lives in /opt/csw/bin)" exit 1 fi - if test "$solinst" = "/usr/sbin/install" ; then + if type "$install" 2> /dev/null | grep /usr/bin/install >/dev/null ; then echo "Error: Solaris /usr/sbin/install is not an appropriate install program." echo "try ginstall from the GNU fileutils available from www.blastwave.org" echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install" exit 1 fi - sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"` - if test -z "$sol_ar" ; then + if ! prog_exist "ar" ; then echo "Error: No path includes ar" if test -f /usr/ccs/bin/ar ; then echo "Add /usr/ccs/bin to your path and rerun configure" @@ -969,7 +972,7 @@ fi # pkgconfig probe pkgconfig="${cross_prefix}pkg-config" -if ! test -x "$(which $pkgconfig 2>/dev/null)"; then +if ! prog_exist "$pkgconfig" ; then # likely not cross compiling, or hope for the best pkgconfig=pkg-config fi @@ -977,7 +980,7 @@ fi ## # Sparse probe if test "$sparse" != "no" ; then - if test -x "$(which cgcc 2>/dev/null)"; then + if prog_exist "cgcc" ; then sparse=yes else if test "$sparse" = "yes" ; then @@ -1419,8 +1422,8 @@ EOF fi else if test "$kvm" = "yes" ; then - if [ -x "`which awk 2>/dev/null`" ] && \ - [ -x "`which grep 2>/dev/null`" ]; then + if prog_exist "awk" && \ + prog_exist "grep"; then kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \ | grep "error: " \ | awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'` @@ -1689,8 +1692,8 @@ fi # Check if tools are available to build documentation. if test "$docs" != "no" ; then - if test -x "`which texi2html 2>/dev/null`" -a \ - -x "`which pod2man 2>/dev/null`" ; then + if prog_exist "texi2html" && \ + prog_exist "pod2man" ; then docs=yes else if test "$docs" = "yes" ; then -- 1.6.6
[Qemu-devel] [PATCH 0/5] Remove which use
This series substitute "which" use by "type" one. It is similar to Loïc patch, just that I didn't use path_of() because it was used just once. This addresses commets about using && instead of -a. It also remove the search for "grep" in kvm code. grep is assumed in lots of other places in configure already. Riku: are you going to pick the missing symbols patch (its is for linux-user) or should Anthony pick it? Later, Juan. Juan Quintela (3): substitute all uses of which by type Check for sdl-config before calling it Remove check for grep Loïc Minier (2): Add -static earlier to LDFLAGS for compile_prog() Fix missing symbols in .rel/.rela.plt sections cc: riku.voi...@nokia.com configure | 37 +++-- i386.ld | 16 ++-- x86_64.ld | 16 ++-- 3 files changed, 51 insertions(+), 18 deletions(-)
[Qemu-devel] [PATCH 4/5] Remove check for grep
The rest of configure assumes that grep is available already Signed-off-by: Juan Quintela --- configure |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 83f8b26..f31be87 100755 --- a/configure +++ b/configure @@ -1430,8 +1430,7 @@ EOF fi else if test "$kvm" = "yes" ; then - if prog_exist "awk" && \ - prog_exist "grep"; then + if prog_exist "awk" ; then kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \ | grep "error: " \ | awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'` -- 1.6.6
[Qemu-devel] [PATCH 3/5] Add -static earlier to LDFLAGS for compile_prog()
From: Loïc Minier Add -static to LDFLAGS earlier as to run the compile_prog() tests with this flags, this will avoid turning on features for which a shared library is available but not a static one. Signed-off-by: Loïc Minier Signed-off-by: Juan Quintela --- configure |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/configure b/configure index c671918..83f8b26 100755 --- a/configure +++ b/configure @@ -456,7 +456,9 @@ for opt do ;; --enable-gprof) gprof="yes" ;; - --static) static="yes" + --static) +static="yes" +LDFLAGS="-static $LDFLAGS" ;; --disable-sdl) sdl="no" ;; @@ -1977,7 +1979,6 @@ if test "$solaris" = "yes" ; then fi if test "$static" = "yes" ; then echo "CONFIG_STATIC=y" >> $config_host_mak - LDFLAGS="-static $LDFLAGS" fi if test $profiler = "yes" ; then echo "CONFIG_PROFILER=y" >> $config_host_mak -- 1.6.6
[Qemu-devel] [PATCH 2/5] Check for sdl-config before calling it
Adapted Loïc Minier version to not use which. cc: Loïc Minier Signed-off-by: Juan Quintela --- configure |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/configure b/configure index 71edaf5..c671918 100755 --- a/configure +++ b/configure @@ -996,9 +996,15 @@ fi if $pkgconfig sdl --modversion >/dev/null 2>&1; then sdlconfig="$pkgconfig sdl" _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'` -else +else if prog_exist "sdl-config"; then sdlconfig='sdl-config' _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'` +else + if test "$sdl" = "yes" ; then +feature_not_found "sdl" + fi + sdl=no +fi fi sdl_too_old=no -- 1.6.6
[Qemu-devel] [PATCH 5/5] Fix missing symbols in .rel/.rela.plt sections
From: Loïc Minier Fix .rel.plt sections in the output to not only include .rel.plt sections from the input but also the .rel.iplt sections and to define the hidden symbols __rel_iplt_start and __rel_iplt_end around .rel.iplt as otherwise we get undefined references to these when linking statically to a multilib libc.a. This fixes the static build under i386. Apply similar logic to rela.plt/.iplt and __rela_iplt/_plt_start/_end to fix the static build under amd64. Signed-off-by: Loïc Minier Signed-off-by: Juan Quintela --- i386.ld | 16 ++-- x86_64.ld | 16 ++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/i386.ld b/i386.ld index f2dafec..f8df7bf 100644 --- a/i386.ld +++ b/i386.ld @@ -39,8 +39,20 @@ SECTIONS .rela.fini : { *(.rela.fini) } .rel.bss : { *(.rel.bss) } .rela.bss : { *(.rela.bss) } - .rel.plt : { *(.rel.plt) } - .rela.plt : { *(.rela.plt) } + .rel.plt : + { +*(.rel.plt) +PROVIDE_HIDDEN (__rel_iplt_start = .); +*(.rel.iplt) +PROVIDE_HIDDEN (__rel_iplt_end = .); + } + .rela.plt : + { +*(.rela.plt) +PROVIDE_HIDDEN (__rela_iplt_start = .); +*(.rela.iplt) +PROVIDE_HIDDEN (__rela_iplt_end = .); + } .init : { *(.init) } =0x47ff041f .text : { diff --git a/x86_64.ld b/x86_64.ld index 24ea77d..46d8d4d 100644 --- a/x86_64.ld +++ b/x86_64.ld @@ -35,8 +35,20 @@ SECTIONS .rela.got : { *(.rela.got) } .rel.bss: { *(.rel.bss .rel.bss.* .rel.gnu.linkonce.b.*) } .rela.bss : { *(.rela.bss .rela.bss.* .rela.gnu.linkonce.b.*) } - .rel.plt: { *(.rel.plt) } - .rela.plt : { *(.rela.plt) } + .rel.plt : + { +*(.rel.plt) +PROVIDE_HIDDEN (__rel_iplt_start = .); +*(.rel.iplt) +PROVIDE_HIDDEN (__rel_iplt_end = .); + } + .rela.plt : + { +*(.rela.plt) +PROVIDE_HIDDEN (__rela_iplt_start = .); +*(.rela.iplt) +PROVIDE_HIDDEN (__rela_iplt_end = .); + } .init : { KEEP (*(.init)) -- 1.6.6
Re: [Qemu-devel] [PATCH] make: qemu-img depends on config-host.h
Am 20.01.2010 um 17:12 schrieb Anthony Liguori: Fixes mingw32 build out of tree. Signed-off-by: Anthony Liguori --- Makefile |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index b1bbe6d..60d5c66 100644 --- a/Makefile +++ b/Makefile @@ -126,7 +126,7 @@ bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS) ## -qemu-img.o: qemu-img-cmds.h +qemu-img.o: qemu-img-cmds.h config-host.h Maybe use $(GENERATED_HEADERS) instead? Okay otherwise. Andreas
[Qemu-devel] [PATCH 0/7] *** SUBJECT HERE ***
*** BLURB HERE *** Juan Quintela (7): Rename --enable-uname-release Add PATH argument to options that need it Sort together all options that take arguments Add help to --cpu and --with-pkgversion Change --static to --enable-static Sort boolean options Add documentation for --enable/disable-docs configure | 362 - 1 files changed, 188 insertions(+), 174 deletions(-)
[Qemu-devel] [PATCH 1/7] Rename --enable-uname-release
It really sets uname string. It don't make sense a --disable option Signed-off-by: Juan Quintela --- configure |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 192338f..08ff6a2 100755 --- a/configure +++ b/configure @@ -578,7 +578,7 @@ for opt do ;; --disable-user-pie) user_pie="no" ;; - --enable-uname-release=*) uname_release="$optarg" + --set-uname-release=*) uname_release="$optarg" ;; --sparc_cpu=*) ;; @@ -782,7 +782,7 @@ echo " --disable-user-pie do not build usermode emulation targets as PIE" echo " --fmod-lib path to FMOD library" echo " --fmod-inc path to FMOD includes" echo " --oss-libpath to OSS library" -echo " --enable-uname-release=R Return R for uname -r in usermode emulation" +echo " --set-uname-release=R Return R for uname -r in usermode emulation" echo " --sparc_cpu=VBuild qemu for Sparc architecture v7, v8, v8plus, v8plusa, v9" echo " --disable-uuid disable uuid support" echo " --enable-uuidenable uuid support" -- 1.6.6
[Qemu-devel] [PATCH 2/7] Add PATH argument to options that need it
Signed-off-by: Juan Quintela --- configure |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 08ff6a2..6175b75 100755 --- a/configure +++ b/configure @@ -779,9 +779,9 @@ echo " emulation targets" echo " --disable-guest-base disable GUEST_BASE support" echo " --enable-user-piebuild usermode emulation targets as PIE" echo " --disable-user-pie do not build usermode emulation targets as PIE" -echo " --fmod-lib path to FMOD library" -echo " --fmod-inc path to FMOD includes" -echo " --oss-libpath to OSS library" +echo " --fmod-lib=PATH path to FMOD library" +echo " --fmod-inc=PATH path to FMOD includes" +echo " --oss-lib=PATH path to OSS library" echo " --set-uname-release=R Return R for uname -r in usermode emulation" echo " --sparc_cpu=VBuild qemu for Sparc architecture v7, v8, v8plus, v8plusa, v9" echo " --disable-uuid disable uuid support" -- 1.6.6
[Qemu-devel] [PATCH 3/7] Sort together all options that take arguments
Signed-off-by: Juan Quintela --- configure | 103 +++- 1 files changed, 53 insertions(+), 50 deletions(-) diff --git a/configure b/configure index 6175b75..1e6088c 100755 --- a/configure +++ b/configure @@ -60,12 +60,12 @@ audio_win_int="" for opt do optarg=`expr "x$opt" : 'x[^=]*=\(.*\)'` case "$opt" in - --cross-prefix=*) cross_prefix="$optarg" - ;; --cc=*) cc="$optarg" ;; --cpu=*) cpu="$optarg" ;; + --cross-prefix=*) cross_prefix="$optarg" + ;; --extra-cflags=*) QEMU_CFLAGS="$optarg $QEMU_CFLAGS" ;; --extra-ldflags=*) LDFLAGS="$optarg $LDFLAGS" @@ -427,56 +427,67 @@ werror="" for opt do optarg=`expr "x$opt" : 'x[^=]*=\(.*\)'` case "$opt" in +# Standard options --help|-h) show_help=yes ;; + --interp-prefix=*) interp_prefix="$optarg" + ;; --prefix=*) prefix="$optarg" ;; - --interp-prefix=*) interp_prefix="$optarg" + --target-list=*) target_list="$optarg" ;; - --source-path=*) source_path="$optarg" - source_path_used="yes" +# Advanced options + --audio-card-list=*) audio_card_list=`echo "$optarg" | sed -e 's/,/ /g'` ;; - --cross-prefix=*) + --audio-drv-list=*) audio_drv_list="$optarg" ;; - --cc=*) + --block-drv-whitelist=*) block_drv_whitelist=`echo "$optarg" | sed -e 's/,/ /g'` ;; - --host-cc=*) host_cc="$optarg" + --cc=*) ;; - --make=*) make="$optarg" + --cpu=*) ;; - --install=*) install="$optarg" + --cross-prefix=*) ;; --extra-cflags=*) ;; --extra-ldflags=*) ;; - --cpu=*) + --fmod-inc=*) fmod_inc="$optarg" ;; - --target-list=*) target_list="$optarg" + --fmod-lib=*) fmod_lib="$optarg" ;; - --enable-gprof) gprof="yes" + --host-cc=*) host_cc="$optarg" ;; - --disable-gprof) gprof="no" + --install=*) install="$optarg" ;; - --static) -static="yes" -LDFLAGS="-static $LDFLAGS" + --kerneldir=*) kerneldir="$optarg" ;; - --disable-sdl) sdl="no" + --make=*) make="$optarg" ;; - --enable-sdl) sdl="yes" + --oss-lib=*) oss_lib="$optarg" ;; - --fmod-lib=*) fmod_lib="$optarg" + --set-uname-release=*) uname_release="$optarg" ;; - --fmod-inc=*) fmod_inc="$optarg" + --source-path=*) +source_path="$optarg" +source_path_used="yes" ;; - --oss-lib=*) oss_lib="$optarg" + --sparc_cpu=*) ;; - --audio-card-list=*) audio_card_list=`echo "$optarg" | sed -e 's/,/ /g'` + --with-pkgversion=*) pkgversion=" ($optarg)" ;; - --audio-drv-list=*) audio_drv_list="$optarg" + --static) +static="yes" +LDFLAGS="-static $LDFLAGS" ;; - --block-drv-whitelist=*) block_drv_whitelist=`echo "$optarg" | sed -e 's/,/ /g'` + --enable-gprof) gprof="yes" + ;; + --disable-gprof) gprof="no" + ;; + --disable-sdl) sdl="no" + ;; + --enable-sdl) sdl="yes" ;; --enable-debug-tcg) debug_tcg="yes" ;; @@ -578,10 +589,6 @@ for opt do ;; --disable-user-pie) user_pie="no" ;; - --set-uname-release=*) uname_release="$optarg" - ;; - --sparc_cpu=*) - ;; --enable-werror) werror="yes" ;; --disable-werror) werror="no" @@ -622,10 +629,6 @@ for opt do ;; --enable-blobs) blobs="yes" ;; - --kerneldir=*) kerneldir="$optarg" - ;; - --with-pkgversion=*) pkgversion=" ($optarg)" - ;; --disable-docs) docs="no" ;; --enable-docs) docs="yes" @@ -699,20 +702,32 @@ Options: [defaults in brackets after descriptions] EOF echo "Standard options:" echo " --help print this message" -echo " --prefix=PREFIX install in PREFIX [$prefix]" echo " --interp-prefix=PREFIX where to find shared libraries, etc." echo " use %M for cpu name [$interp_prefix]" +echo " --prefix=PREFIX install in PREFIX [$prefix]" echo " --target-list=LIST set target list [$target_list]" echo "" echo "Advanced options (experts only):" -echo " --source-path=PATH path of source code [$source_path]" -echo " --cross-prefix=PREFIXuse PREFIX for compile tools [$cross_prefix]" +echo " --audio-drv-list=LISTset audio drivers list:" +echo " Available drivers: $audio_possible_drivers" +echo " --audio-card-list=LIST set list of emulated audio cards [$audio_card_list]" +echo " Available cards: $audio_possible_cards" +echo " --block-drv-whitelist=L set block driver whitelist" +echo " (affects only QEMU, not qemu-img)" echo " --cc=CC use C compiler CC [$cc]" -echo " --host-cc=CC use C compiler CC [$host_cc] for dyngen etc." +echo " --cross-prefix=PREFIXuse PREFIX for compile tools [$cross_prefix]" echo " --extra-cflags=CFLAGSappend extra C compiler flags QEMU_CFLAGS" echo " --extra-ldflags=LDFLAGS append extra linker flags LDFLAGS" -echo " --make=MAKE use specified make [$make]" +echo " --fmod-inc=PATH path to FMOD includes" +echo " --fmod-lib=PATH path to FMOD library" +echo " --host-
[Qemu-devel] [PATCH 4/7] Add help to --cpu and --with-pkgversion
Signed-off-by: Juan Quintela --- configure |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/configure b/configure index 1e6088c..c9061e1 100755 --- a/configure +++ b/configure @@ -715,6 +715,7 @@ echo " Available cards: $audio_possible_cards" echo " --block-drv-whitelist=L set block driver whitelist" echo " (affects only QEMU, not qemu-img)" echo " --cc=CC use C compiler CC [$cc]" +echo " --cpu=CPUuse CPU for configuration" echo " --cross-prefix=PREFIXuse PREFIX for compile tools [$cross_prefix]" echo " --extra-cflags=CFLAGSappend extra C compiler flags QEMU_CFLAGS" echo " --extra-ldflags=LDFLAGS append extra linker flags LDFLAGS" @@ -728,6 +729,7 @@ echo " --oss-lib=PATH path to OSS library" echo " --set-uname-release=R Return R for uname -r in usermode emulation" echo " --source-path=PATH path of source code [$source_path]" echo " --sparc_cpu=VBuild qemu for Sparc architecture v7, v8, v8plus, v8plusa, v9" +echo " --with-pkgversion=PKGUse PKG string as packageversion" echo " --static enable static build [$static]" echo " --disable-gprof disable gprof profiling" echo " --enable-gprof enable gprof profiling" -- 1.6.6
[Qemu-devel] [PATCH 5/7] Change --static to --enable-static
This way it is consistent with everything else. Leave --static as compatible option, but don't document it Signed-off-by: Juan Quintela --- configure |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/configure b/configure index c9061e1..f86fef3 100755 --- a/configure +++ b/configure @@ -477,7 +477,10 @@ for opt do ;; --with-pkgversion=*) pkgversion=" ($optarg)" ;; - --static) + --disable-static) +static="no" + ;; + --enable-static|--static) #--static was the old name static="yes" LDFLAGS="-static $LDFLAGS" ;; @@ -730,7 +733,8 @@ echo " --set-uname-release=R Return R for uname -r in usermode emulation" echo " --source-path=PATH path of source code [$source_path]" echo " --sparc_cpu=VBuild qemu for Sparc architecture v7, v8, v8plus, v8plusa, v9" echo " --with-pkgversion=PKGUse PKG string as packageversion" -echo " --static enable static build [$static]" +echo " --disable-static disable static build" +echo " --enable-static enable static build" echo " --disable-gprof disable gprof profiling" echo " --enable-gprof enable gprof profiling" echo " --enable-debug-tcg enable TCG debugging" -- 1.6.6
[Qemu-devel] [PATCH 6/7] Sort boolean options
No code changes Signed-off-by: Juan Quintela --- configure | 285 +++-- 1 files changed, 144 insertions(+), 141 deletions(-) diff --git a/configure b/configure index f86fef3..046b0fb 100755 --- a/configure +++ b/configure @@ -477,164 +477,166 @@ for opt do ;; --with-pkgversion=*) pkgversion=" ($optarg)" ;; - --disable-static) -static="no" +# Boolean options + --disable-blobs) blobs="no" ;; - --enable-static|--static) #--static was the old name -static="yes" -LDFLAGS="-static $LDFLAGS" + --enable-blobs) blobs="yes" ;; - --enable-gprof) gprof="yes" + --disable-bluez) bluez="no" ;; - --disable-gprof) gprof="no" + --enable-bluez) bluez="yes" ;; - --disable-sdl) sdl="no" + --disable-brlapi) brlapi="no" ;; - --enable-sdl) sdl="yes" + --enable-brlapi) brlapi="yes" ;; - --enable-debug-tcg) debug_tcg="yes" + --disable-bsd-user) bsd_user="no" ;; - --disable-debug-tcg) debug_tcg="no" + --enable-bsd-user) bsd_user="yes" ;; - --disable-debug) - debug_tcg="no" - debug="no" + --disable-check-utests) check_utests="no" ;; - --enable-debug) - # Enable debugging options that aren't excessively noisy - debug_tcg="yes" - debug="yes" - strip_opt="no" + --enable-check-utests) check_utests="yes" ;; - --enable-sparse) sparse="yes" + --disable-cocoa) + cocoa="no" ;; - --disable-sparse) sparse="no" + --enable-cocoa) +cocoa="yes" ; +sdl="no" ; +audio_drv_list="coreaudio `echo $audio_drv_list | sed s,coreaudio,,g`" ;; - --disable-strip) strip_opt="no" + --disable-curl) curl="no" ;; - --enable-strip) strip_opt="yes" + --enable-curl) curl="yes" ;; - --disable-vnc-tls) vnc_tls="no" + --disable-curses) curses="no" ;; - --enable-vnc-tls) vnc_tls="yes" + --enable-curses) curses="yes" ;; - --disable-vnc-sasl) vnc_sasl="no" + --disable-darwin-user) darwin_user="no" ;; - --enable-vnc-sasl) vnc_sasl="yes" + --enable-darwin-user) darwin_user="yes" ;; - --disable-slirp) slirp="no" + --disable-debug) +debug_tcg="no" +debug="no" ;; - --enable-slirp) slirp="yes" + --enable-debug) +# Enable debugging options that aren't excessively noisy +debug_tcg="yes" +debug="yes" +strip_opt="no" ;; - --disable-uuid) uuid="no" + --disable-debug-tcg) debug_tcg="no" ;; - --enable-uuid) uuid="yes" + --enable-debug-tcg) debug_tcg="yes" ;; - --disable-vde) vde="no" + --disable-docs) docs="no" ;; - --enable-vde) vde="yes" + --enable-docs) docs="yes" ;; - --disable-xen) xen="no" + --disable-fdt) fdt="no" ;; - --enable-xen) xen="yes" + --enable-fdt) fdt="yes" ;; - --disable-brlapi) brlapi="no" + --enable-gprof) gprof="yes" ;; - --enable-brlapi) brlapi="yes" + --disable-gprof) gprof="no" ;; - --disable-bluez) bluez="no" + --disable-guest-base) guest_base="no" ;; - --enable-bluez) bluez="yes" + --enable-guest-base) guest_base="yes" + ;; + --disable-io-thread) io_thread="no" + ;; + --enable-io-thread) io_thread="yes" ;; --disable-kvm) kvm="no" ;; --enable-kvm) kvm="yes" ;; - --disable-profiler) profiler="no" - ;; - --enable-profiler) profiler="yes" + --disable-linux-aio) linux_aio="no" ;; - --enable-cocoa) - cocoa="yes" ; - sdl="no" ; - audio_drv_list="coreaudio `echo $audio_drv_list | sed s,coreaudio,,g`" + --enable-linux-aio) linux_aio="yes" ;; - --disable-cocoa) - cocoa="no" + --disable-linux-user) linux_user="no" ;; - --disable-system) softmmu="no" + --enable-linux-user) linux_user="yes" ;; - --enable-system) softmmu="yes" + --disable-mixemu) mixemu="no" ;; - --disable-user) - linux_user="no" ; - bsd_user="no" ; - darwin_user="no" + --enable-mixemu) mixemu="yes" ;; - --enable-user) ;; - --disable-linux-user) linux_user="no" + --disable-nptl) nptl="no" ;; - --enable-linux-user) linux_user="yes" + --enable-nptl) nptl="yes" ;; - --disable-darwin-user) darwin_user="no" + --disable-profiler) profiler="no" ;; - --enable-darwin-user) darwin_user="yes" + --enable-profiler) profiler="yes" ;; - --disable-bsd-user) bsd_user="no" + --disable-sdl) sdl="no" ;; - --enable-bsd-user) bsd_user="yes" + --enable-sdl) sdl="yes" ;; - --enable-guest-base) guest_base="yes" + --disable-slirp) slirp="no" ;; - --disable-guest-base) guest_base="no" + --enable-slirp) slirp="yes" ;; - --enable-user-pie) user_pie="yes" + --enable-sparse) sparse="yes" ;; - --disable-user-pie) user_pie="no" + --disable-sparse) sparse="no" ;; - --enable-werror) werror="yes" + --disable-static) +static="no" ;; - --disable-werror) werror="no" + --enable-static|--static) #--static was the old name +static="yes" +LDFLAGS="-static $LDFLAGS" ;; - --disable-curses) curses="no" + --disable-strip) strip_opt="no" ;; - --enable-curses) curses="yes" + --enable-strip) strip_op
[Qemu-devel] [PATCH 7/7] Add documentation for --enable/disable-docs
Signed-off-by: Juan Quintela --- configure |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/configure b/configure index 046b0fb..9d315f4 100755 --- a/configure +++ b/configure @@ -758,6 +758,8 @@ echo " --disable-debug disable common debug build options" echo " --enable-debug enable common debug build options" echo " --disable-debug-tcg disable TCG debugging (default)" echo " --enable-debug-tcg enable TCG debugging" +echo " --disable-docs disable documentation generation" +echo " --enable-docsenable documentation generation" echo " --disable-fdtdisable fdt device tree" echo " --enable-fdt enable fdt device tree" echo " --disable-gprof disable gprof profiling" -- 1.6.6
Re: [Qemu-devel] [PATCH v2 0/5]: Convert pci_info() to QObject
Luiz Capitulino writes: > Hi, > > This new version addresses Markus's comments. > > changelog > - > > V1 -> V2 > > - Make class_info's key 'desc' optional > - Better indentation > - Doc fixes > > V0 -> V1 > > - Coding style fixes > - Make 'BAR' and 'IRQ' keys lowercase > - Add 'irq' key to the documentation > > Thanks. Looks good, although one comment still applies: PATCH 3/5 regresses info pci, 4/5 and 5/5 fix it. Do we care? They're separate because they're untested.
Re: [Qemu-devel] [PATCH v2 0/5]: Convert pci_info() to QObject
On Wed, 20 Jan 2010 18:57:56 +0100 Markus Armbruster wrote: > Luiz Capitulino writes: > > > Hi, > > > > This new version addresses Markus's comments. > > > > changelog > > - > > > > V1 -> V2 > > > > - Make class_info's key 'desc' optional > > - Better indentation > > - Doc fixes > > > > V0 -> V1 > > > > - Coding style fixes > > - Make 'BAR' and 'IRQ' keys lowercase > > - Add 'irq' key to the documentation > > > > Thanks. > > Looks good, although one comment still applies: PATCH 3/5 regresses info > pci, 4/5 and 5/5 fix it. Do we care? They're separate because they're > untested. There are two problems here, which apply for those whom emulate the hardware: 1. 'info pci' output will brake with git bisect 2. As the code is untested, it might be broken Only two 2 seems serious. Michael, does the sparc image on qemu.org have the hardware in question (pci bridge)?
[Qemu-devel] Re: Stop using "which" in ./configure
Loïc Minier writes: > On Wed, Jan 20, 2010, Paolo Bonzini wrote: >> > Are you saying that I can't backup/restore IFS without setting it >> > first? >> Yes, it affects the behavior of read for example: >> $ echo a b c | (read a b c; echo $a; echo $b; echo $c) >> a >> b >> c >> $ IFS= >> $ echo a b c | (read a b c; echo $a; echo $b; echo $c) >> a b c >> >> $ >> (It's not used by QEMU's configure, but it's better to be defensive). > > I *do* understand that changing IFS will affect the program, but the > patch I sent will backup IFS and then restore it; perhaps you missed > the backup/restore bits: It might not be set at all to begin with, in which case you'd set it the empty string when restoring, and that would change the behaviour. The POSIX spec says this: If IFS is not set, the shell shall behave as if the value of IFS is , , and > +local_ifs="$IFS" > [...] > +IFS=: > [...] > +IFS="$local_ifs" > +return 0 > [...] > +IFS="$local_ifs" > +return 1 If you make that IFS=${local_ifs:-$(printf ' \t\n')} it should be safe. Likewise if you set the value first. -- Måns Rullgård m...@mansr.com
[Qemu-devel] Re: sparc32 do_unassigned_access overhaul
On Tue, Jan 19, 2010 at 9:44 PM, Artyom Tarasenko wrote: > 2010/1/19 Blue Swirl : >> On Tue, Jan 19, 2010 at 5:30 PM, Artyom Tarasenko >> wrote: >>> 2010/1/15 Artyom Tarasenko : 2010/1/15 Blue Swirl : > On Fri, Jan 15, 2010 at 9:11 PM, Artyom Tarasenko > wrote: >> 2010/1/15 Blue Swirl : >>> On Fri, Jan 15, 2010 at 6:46 PM, Artyom Tarasenko >>> wrote: According to pages 9-31 - 9-34 of "SuperSPARC & MultiCache Controller User's Manual": 1. "A lower priority fault may not overwrite the MFSR status of a higher priority fault." 2. The MFAR is overwritten according to the policy defined for the MFSR 3. The overwrite bit is asserted if the fault status register (MFSR) has been written more than once by faults of the same class 4. SuperSPARC will never place instruction fault addresses in the MFAR. Implementation of points 1-3 allows booting Solaris 2.6 and 2.5.1. >>> >>> Nice work! This also passes my tests. >> >> I'm afraid we still are not there yet though: Solaris 7 fails >> potentially due to >> another bug in the MMU emulation, and the initial [missing-] RAM >> detection in OBP fails >> very probably due to a bug in in the MMU emulation. > > Some guesses: > - Access to unassigned RAM area may be handled by the memory > controller differently (no faults, different faults etc.) than > unassigned access to SBus or other area. >>> >>> You are right! It seems to be true for the area larger than max RAM though. >>> On a real SS-5 with 32M in the first bank, no fault is produced at >>> least for the areas >>> 0-0x2fff, 0x7000-0xafff (ha, this would solve problems >>> with SS-20 OBP >>> too) and 0xf000-0xf6ff. >> >> The fault may still be recorded somewhere else (MXCC, RAM/ECC >> controller or IOMMU). > > sfar and sfsr were empty, so it's definitely not MXCC. Don't know > where to look for the other two. For SS-5 there is MFAR and MFSR in IOMMU (0x10001050, 0x10001054), see chapters 6.3.7 and 6.3.8 in "TurboSPARC Microprocessor User's Guide", Fujitsu, October 1996, version 1.0. SS-20 has M-S AFAR/AFSR (5.2.1 in Sun4m System Architecture Manual) and ECC EFSR and EFAR (5.5.2/5.5.3). With Viking there is also B.III.5.4.6. MXCC Error Register. > But how the fault would be generated? Don't know about Sun simms, but > PC ones don't have any handshake. IMHO the ECC can be the only > possibility. > >> OBP may have disabled the fault, or it has not >> enabled fault generation. > > NF bit is not set. Also, you can see the other faults. I meant memory parity etc. On TurboSparc, parity is enabled in MMU config register. The fault is registerd in MFSR/MFAR. >> On SS-5, the physical address space should be only 31 bits, so you >> should see RAM aliased at 0x8000. > > No. The RAM can be aliased only within one bank or completely outside > the RAM area. Otherwise different banks would have interfered. > >>> Would you like to implement it? >> >> For RAM, there could be a new device which implements generic address >> space wrapping (base, length, AND mask, OR mask), it should be useful >> for embedded boards. Shouldn't be too difficult, want to try? :-) > > Minutes for you, days for me. :) > >> Dummy MMIO could be registered for the other areas in sun4m.c. I'm not >> sure this is the correct approach, if the fault is still handled >> somewhere else. >> >>> That's how I tested it: >>> >>> ok 800 map? >>> Virtual : 0800. >>> Context : @ 0.01ff.f000 001f.eec1 # 0 >>> Region : @ 0.01fe.ec20 . Invalid >>> ok 800 obmem 800 map-page >>> ok 800 map? >>> Virtual : 0800. >>> Context : @ 0.01ff.f000 001f.eec1 # 0 >>> Region : @ 0.01fe.ec20 001f.b231 >>> Segment : @ 0.01fb.2300 001f.b221 >>> Page : @ 0.01fb.2200 0080.001e Access : rwx--- >>> Physical : 0.0800. >>> ok 800 20 dump >>> \/ 1 2 3 4 5 6 7 8 9 a b c d e f v123456789abcdef >>> 800 00 d1 e1 44 ff d1 e2 18 08 d1 e1 4e ff d1 e2 18 .QaV.Qb..QaV.Qb. >>> 810 00 d1 e1 44 ff d1 e2 18 08 d1 e1 4e ff d1 e2 18 .QaV.Qb..QaV.Qb. >> >> RAM? > > Looks like a white noise to me. The first byte is frequently > different. Also the mapped RAM is filled with 0's. The pattern can not > be found anywhere in the mapped RAM (0x1000-0x9f): > > ok create pattern hex 00 c, d1 c, e1 c, 44 c, ff c, d1 c, e2 c, 18 c, > ok pattern 8 1000 9e sindex . > > ok > > Hold on... I tested only the reading. Should have tested writing too: > > ok aa55aa55 800 l! > ok sfar@ . sfsr@ . > 0 0 > > no fault, no interrupt, but > > ok 800 10 dump > \/ 1 2 3 4 5 6 7 8 9 a b c d e f v123456789abcdef > 800 00 d1 e1 44 ff d1 e2 18 08 d1 e1 4e ff d1 e2 18 .QaV.Qb..QaV.Qb. > ok > > no change either. And if I read it differently I get other contents: > ok 800 l@ . > f
Re: [Qemu-devel] [PATCH 1/5] substitute all uses of which by type
Juan Quintela schrieb: > Except in one case, we are only interested in knowing if a command > exist, not is path. Just create prog_exists() function that does this > check. > > Signed-off-by: Juan Quintela See comments below. The changes proposed are a suggestion, no real need. Regards, Stefan > --- > configure | 25 ++--- > 1 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/configure b/configure > index 5631bbb..71edaf5 100755 > --- a/configure > +++ b/configure > @@ -27,6 +27,11 @@ compile_prog() { > $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > > /dev/null 2> /dev/null > } > > +prog_exist() { > + prog="$1" > + type $prog > /dev/null 2> /dev/null I personally prefer + type $prog >/dev/null 2>&1 because it is shorter (no whitespace after >, no duplicate /dev/null). Currently, all variants to write this pattern are used. Maybe we can reduce this. > +} > + > # default parameters > cpu="" > prefix="" > @@ -763,21 +768,19 @@ fi > # Solaris specific configure tool chain decisions > # > if test "$solaris" = "yes" ; then > - solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install > in"` 2>/dev/null > - if test -z "$solinst" ; then > + if ! prog_exist "$install" ; then > echo "Solaris install program not found. Use > --install=/usr/ucb/install or" > echo "install fileutils from www.blastwave.org using pkg-get -i fileutils" > echo "to get ginstall which is used by default (which lives in > /opt/csw/bin)" > exit 1 > fi > - if test "$solinst" = "/usr/sbin/install" ; then > + if type "$install" 2> /dev/null | grep /usr/bin/install >/dev/null ; > then 2>/dev/null > echo "Error: Solaris /usr/sbin/install is not an appropriate install > program." > echo "try ginstall from the GNU fileutils available from > www.blastwave.org" > echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install" > exit 1 > fi > - sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"` > - if test -z "$sol_ar" ; then > + if ! prog_exist "ar" ; then > echo "Error: No path includes ar" > if test -f /usr/ccs/bin/ar ; then > echo "Add /usr/ccs/bin to your path and rerun configure" > @@ -969,7 +972,7 @@ fi > # pkgconfig probe > > pkgconfig="${cross_prefix}pkg-config" > -if ! test -x "$(which $pkgconfig 2>/dev/null)"; then > +if ! prog_exist "$pkgconfig" ; then > # likely not cross compiling, or hope for the best > pkgconfig=pkg-config > fi > @@ -977,7 +980,7 @@ fi > ## > # Sparse probe > if test "$sparse" != "no" ; then > - if test -x "$(which cgcc 2>/dev/null)"; then > + if prog_exist "cgcc" ; then > sparse=yes > else > if test "$sparse" = "yes" ; then > @@ -1419,8 +1422,8 @@ EOF > fi > else > if test "$kvm" = "yes" ; then > - if [ -x "`which awk 2>/dev/null`" ] && \ > - [ -x "`which grep 2>/dev/null`" ]; then > + if prog_exist "awk" && \ > + prog_exist "grep"; then > kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \ > | grep "error: " \ > | awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'` > @@ -1689,8 +1692,8 @@ fi > > # Check if tools are available to build documentation. > if test "$docs" != "no" ; then > - if test -x "`which texi2html 2>/dev/null`" -a \ > - -x "`which pod2man 2>/dev/null`" ; then > + if prog_exist "texi2html" && \ > + prog_exist "pod2man" ; then > docs=yes > else > if test "$docs" = "yes" ; then
Re: [Qemu-devel] [PATCH] sparc64: reimplement tick timers v2
On Tue, Jan 19, 2010 at 10:24 PM, Igor Kovalenko wrote: > On Tue, Jan 19, 2010 at 9:44 PM, Blue Swirl wrote: >> On Mon, Jan 18, 2010 at 10:28 PM, Igor V. Kovalenko >> wrote: >>> From: Igor V. Kovalenko >>> >>> sparc64 timer has tick counter which can be set and read, >>> and tick compare value used as deadline to fire timer interrupt. >>> The timer is not used as periodic timer, instead deadline >>> is set each time new timer interrupt is needed. >> >> Does not compile: >> >> CC sparc64-softmmu/sun4u.o >> cc1: warnings being treated as errors >> /src/qemu/hw/sun4u.c: In function 'cpu_tick_set_count': >> /src/qemu/hw/sun4u.c:467: error: implicit declaration of function >> 'TIMER_DPRINTF' >> make[1]: *** [sun4u.o] Error 1 > > Sorry forgot the split out debugging piece. New v3 has all these > comments addressed. > >> If I add the missing TIMER_DPRINTF, Linux still crashes: >> >> Memory: 117376k available (2136k kernel code, 664k data, 184k init) >> [f800,07e8] >> SLUB: Genslabs=14, HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1 >> Hierarchical RCU implementation. >> NR_IRQS:255 >> clocksource: mult[a] shift[16] >> clockevent: mult[1999] shift[32] >> Console: colour dummy device 80x25 >> Unable to handle kernel NULL pointer dereference >> tsk->{mm,active_mm}->context = >> tsk->{mm,active_mm}->pgd = f86fdaa4 >> \|/ \|/ >> "@'/ .. \`@" >> /_| \__/ |_\ >> \__U_/ >> swapper(0): Oops [#1] >> TSTATE: 004480001607 TPC: 006e32f4 TNPC: 006e32f8 >> Y: Not tainted >> TPC: > > Easy to reproduce here, and I still fail to find the reason for this crash. > It looks like linux kernel loads zero values from memory while returning > from timer softirq handler. Note this does not always happen at first timer > interrupt inside calibrate_delay loop. Maybe a bug with AG/MG/IG handling? > I really want to see debug trace of all processed instructions, but > qemu command line switch -singlestep does not really work for sparc64-softmmu > and stepping through calibrate_delay does not seem to help at the moment. > Any ideas on how to force all instructions after approx. 500 cpu > cycles to be > single stepped, so I get the trace in qemu.log ? Fixing -singlestep should not be too difficult. :-) For a quick hack, you could add a call to tb_flush() near the end of cpu_exec() and hack gen_intermediate_code_internal() so that max_insns is 1 (or 2 for delay slots?).
[Qemu-devel] [PATCH 01/14] arm host: Fix compiler warning
Compilation for arm (native or cross) results in this warning: fpu/softfloat-native.c: In function ‘float64_round_to_int’: fpu/softfloat-native.c:387: error: control reaches end of non-void function float64_round_to_int uses special assembler code for arm and has no explicit return value. As there is no obvious reason why arm should need special code, all fpu related conditionals were removed. The remaining code is standard (C99) and compiles for arm, too. Signed-off-by: Stefan Weil --- fpu/softfloat-native.c | 20 fpu/softfloat-native.h |7 --- 2 files changed, 0 insertions(+), 27 deletions(-) diff --git a/fpu/softfloat-native.c b/fpu/softfloat-native.c index 8d64f4e..049c830 100644 --- a/fpu/softfloat-native.c +++ b/fpu/softfloat-native.c @@ -12,8 +12,6 @@ void set_float_rounding_mode(int val STATUS_PARAM) #if (defined(CONFIG_BSD) && !defined(__APPLE__) && !defined(__GLIBC__)) || \ (defined(CONFIG_SOLARIS) && CONFIG_SOLARIS_VERSION < 10) fpsetround(val); -#elif defined(__arm__) -/* nothing to do */ #else fesetround(val); #endif @@ -365,25 +363,7 @@ float64 float64_trunc_to_int( float64 a STATUS_PARAM ) float64 float64_round_to_int( float64 a STATUS_PARAM ) { -#if defined(__arm__) -switch(STATUS(float_rounding_mode)) { -default: -case float_round_nearest_even: -asm("rndd %0, %1" : "=f" (a) : "f"(a)); -break; -case float_round_down: -asm("rnddm %0, %1" : "=f" (a) : "f"(a)); -break; -case float_round_up: -asm("rnddp %0, %1" : "=f" (a) : "f"(a)); -break; -case float_round_to_zero: -asm("rnddz %0, %1" : "=f" (a) : "f"(a)); -break; -} -#else return rint(a); -#endif } float64 float64_rem( float64 a, float64 b STATUS_PARAM) diff --git a/fpu/softfloat-native.h b/fpu/softfloat-native.h index fe737b3..6da0bcb 100644 --- a/fpu/softfloat-native.h +++ b/fpu/softfloat-native.h @@ -126,13 +126,6 @@ enum { float_round_up = FP_RP, float_round_to_zero = FP_RZ }; -#elif defined(__arm__) -enum { -float_round_nearest_even = 0, -float_round_down = 1, -float_round_up = 2, -float_round_to_zero = 3 -}; #else enum { float_round_nearest_even = FE_TONEAREST, -- 1.6.5
[Qemu-devel] Re: [PATCH 1/5] substitute all uses of which by type
Stefan Weil wrote: > Juan Quintela schrieb: >> Except in one case, we are only interested in knowing if a command >> exist, not is path. Just create prog_exists() function that does this >> check. >> >> Signed-off-by: Juan Quintela > > See comments below. The changes proposed are a suggestion, no real need. > > Regards, > Stefan > >> --- >> configure | 25 ++--- >> 1 files changed, 14 insertions(+), 11 deletions(-) >> >> diff --git a/configure b/configure >> index 5631bbb..71edaf5 100755 >> --- a/configure >> +++ b/configure >> @@ -27,6 +27,11 @@ compile_prog() { >> $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> > /dev/null 2> /dev/null >> } >> >> +prog_exist() { >> + prog="$1" >> + type $prog > /dev/null 2> /dev/null > > I personally prefer > + type $prog >/dev/null 2>&1 > because it is shorter (no whitespace after >, no duplicate /dev/null). > Currently, all variants to write this pattern are used. > Maybe we can reduce this. Agreed. For next series if there is not needed a rebase. Later, Juan.
Re: [Qemu-devel] [PATCH v2 0/5]: Convert pci_info() to QObject
On Wed, Jan 20, 2010 at 6:11 PM, Luiz Capitulino wrote: > On Wed, 20 Jan 2010 18:57:56 +0100 > Markus Armbruster wrote: > >> Luiz Capitulino writes: >> >> > Hi, >> > >> > This new version addresses Markus's comments. >> > >> > changelog >> > - >> > >> > V1 -> V2 >> > >> > - Make class_info's key 'desc' optional >> > - Better indentation >> > - Doc fixes >> > >> > V0 -> V1 >> > >> > - Coding style fixes >> > - Make 'BAR' and 'IRQ' keys lowercase >> > - Add 'irq' key to the documentation >> > >> > Thanks. >> >> Looks good, although one comment still applies: PATCH 3/5 regresses info >> pci, 4/5 and 5/5 fix it. Do we care? They're separate because they're >> untested. > > There are two problems here, which apply for those whom emulate > the hardware: > > 1. 'info pci' output will brake with git bisect > > 2. As the code is untested, it might be broken > > Only two 2 seems serious. > > Michael, does the sparc image on qemu.org have the hardware > in question (pci bridge)? Sparc64 has two Simba bridges, but currently they are broken so there are no devices behind them. In addition there should be a DEC 21154 bridge. There is no Sparc64 test image yet (very few Sparc32 machines did have any PCI and we don't emulate them), but you can test the output without any images: qemu-system-sparc64 -L pc-bios -S -monitor stdio QEMU 0.12.50 monitor - type 'help' for more information (qemu) info pci Bus 0, device 0, function 0: Host bridge: PCI device 108e:a000 id "" Bus 0, device 1, function 0: PCI bridge: PCI device 108e:5000 BUS 0. secondary bus 0. subordinate bus 0. IO range [0x, 0x0fff] memory range [0x, 0x000f] prefetchable memory range [0x, 0x000f] id "" Bus 0, device 1, function 1: PCI bridge: PCI device 108e:5000 BUS 0. secondary bus 0. subordinate bus 0. IO range [0x, 0x0fff] memory range [0x, 0x000f] prefetchable memory range [0x, 0x000f] id "" Bus 0, device 2, function 0: VGA controller: PCI device 1234: BAR0: 32 bit prefetchable memory at 0x [0x007e]. id "" Bus 0, device 3, function 0: Bridge: PCI device 108e:1000 BAR0: 32 bit memory at 0x [0x00fe]. BAR1: 32 bit memory at 0x [0x007e]. id "" Bus 0, device 4, function 0: Ethernet controller: PCI device 10ec:8029 IRQ 0. BAR0: I/O at 0x [0x00fe]. id "" Bus 0, device 5, function 0: IDE controller: PCI device 1095:0646 IRQ 0. BAR0: I/O at 0x [0x0006]. BAR1: I/O at 0x [0x0002]. BAR2: I/O at 0x [0x0006]. BAR3: I/O at 0x [0x0002]. BAR4: I/O at 0x [0x000e]. id "" (qemu) c (qemu) info pci Bus 0, device 0, function 0: Host bridge: PCI device 108e:a000 id "" Bus 0, device 1, function 0: PCI bridge: PCI device 108e:5000 BUS 0. secondary bus 0. subordinate bus 0. IO range [0x, 0x0fff] memory range [0x, 0x000f] prefetchable memory range [0x, 0x000f] id "" Bus 0, device 1, function 1: PCI bridge: PCI device 108e:5000 BUS 0. secondary bus 0. subordinate bus 0. IO range [0x, 0x0fff] memory range [0x, 0x000f] prefetchable memory range [0x, 0x000f] id "" Bus 0, device 2, function 0: VGA controller: PCI device 1234: BAR0: 32 bit prefetchable memory at 0x0080 [0x00ff]. id "" Bus 0, device 3, function 0: Bridge: PCI device 108e:1000 BAR0: 32 bit memory at 0x0100 [0x01ff]. BAR1: 32 bit memory at 0x0200 [0x027f]. id "" Bus 0, device 4, function 0: Ethernet controller: PCI device 10ec:8029 IRQ 0. BAR0: I/O at 0x0400 [0x04ff]. id "" Bus 0, device 5, function 0: IDE controller: PCI device 1095:0646 IRQ 1. BAR0: I/O at 0x0500 [0x0507]. BAR1: I/O at 0x0580 [0x0583]. BAR2: I/O at 0x0600 [0x0607]. BAR3: I/O at 0x0680 [0x0683]. BAR4: I/O at 0x0700 [0x070f]. id ""
[Qemu-devel] [PATCH] Fix generation of config-host.h
This patch improves Anthony patch a6a853c86275efd89996ce59612a000c5873db5d Once there, it improves handling of object files for qemu tools cc: Andreas Färber Signed-off-by: Juan Quintela --- Makefile | 17 + 1 files changed, 5 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 60d5c66..3848627 100644 --- a/Makefile +++ b/Makefile @@ -126,21 +126,14 @@ bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS) ## -qemu-img.o: qemu-img-cmds.h config-host.h +qemu-img.o: qemu-img-cmds.h +qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o: $(GENERATED_HEADERS) -obj-y = qemu-img.o qemu-tool.o $(block-obj-y) $(qobject-obj-y) +qemu-img$(EXESUF): qemu-img.o qemu-tool.o $(block-obj-y) $(qobject-obj-y) -qemu-img$(EXESUF): $(obj-y) +qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o $(block-obj-y) $(qobject-obj-y) -obj-y = qemu-nbd.o qemu-tool.o $(block-obj-y) $(qobject-obj-y) -$(obj-y): $(GENERATED_HEADERS) - -qemu-nbd$(EXESUF): $(obj-y) - -obj-y = qemu-io.o qemu-tool.o cmd.o $(block-obj-y) $(qobject-obj-y) -$(obj-y): $(GENERATED_HEADERS) - -qemu-io$(EXESUF): $(obj-y) +qemu-io$(EXESUF): qemu-io.o qemu-tool.o cmd.o $(block-obj-y) $(qobject-obj-y) qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx $(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@," GEN $@") -- 1.6.6
Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..
Jamie Lokier wrote: > Anthony Liguori wrote: >> On 01/18/2010 10:45 AM, john cooper wrote: >>> x86 Conroe Intel Celeron_4x0 (Conroe/Merom Class Core 2) >>> x86 Penryn Intel Core 2 Duo P9xxx (Penryn Class Core 2) >>> x86 Nehalem Intel Core i7 9xx (Nehalem Class Core i7) >>> x86 Opteron_G1 AMD Opteron 240 (Gen 1 Class Opteron) >>> x86 Opteron_G2 AMD Opteron 22xx (Gen 2 Class Opteron) >>> x86 Opteron_G3 AMD Opteron 23xx (Gen 3 Class Opteron) >> I'm very much against having -cpu Nehalem. The whole point of this is >> to make things easier for a user and for most of the users I've >> encountered, -cpu Nehalem is just as obscure as -cpu qemu64,-sse3,+vmx,... > > When I saw that table just now, I had no idea whether Nehalem is newer > and more advanced than Penryn, or the other way around. I also have > no idea if "Core i7" is newer than "Core 2 Duo" or not. I can appreciate the argument above, however the goal was choosing names with some basis in reality. These were recommended by our contacts within Intel, are used by VmWare to describe their similar cpu models, and arguably have fallen to defacto usage as evidenced by such sources as: http://en.wikipedia.org/wiki/Conroe_(microprocessor) http://en.wikipedia.org/wiki/Penryn_(microprocessor) http://en.wikipedia.org/wiki/Nehalem_(microarchitecture) I suspect whatever we choose of reasonable length as a model tag for "-cpu" some further detail is going to be required. That was the motivation to augment the table as above with an instance of a LCD for that associated class. > I'm not a typical user: I know quite a lot about x86 architecture; > I just haven't kept up to date enough to know the code/model names. > Typical users will know less about them. Understood. One thought I had to further clarify what is going on under the hood was to dump the cpuid flags for each model as part of (or in addition to) the above table. But this seems a bit extreme and kvm itself can modify flags exported from qemu to a guest. -john -- john.coo...@redhat.com
Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..
Anthony Liguori wrote: > On 01/19/2010 02:03 PM, Chris Wright wrote: >> * Anthony Liguori (anth...@codemonkey.ws) wrote: >> >>> I'm very much against having -cpu Nehalem. The whole point of this is >>> to make things easier for a user and for most of the users I've >>> encountered, -cpu Nehalem is just as obscure as -cpu >>> qemu64,-sse3,+vmx,... >>> >> What name will these users know? FWIW, it makes sense to me as it is. >> > > Whatever is in /proc/cpuinfo. $ grep name /proc/cpuinfo model name : Intel(R) Core(TM)2 Duo CPU E8400 @ 3.00GHz Which is detailing that exact cpu vs. the class of which it is a member. So are you suggesting to map all instances of processors called out in /proc/cpuinfo into one of the three defined models? We can certainly do that however I was looking for a more terse and simplified solution at this level while deferring more ornate mapping schemes to management tools. Still at the user facing CLI this doesn't strike me as the most friendly encoding of a -cpu . -john -- john.coo...@redhat.com
[Qemu-devel] [PATCH v2 00/17] Fix compilation with _FORTIFY_SOURCE
changes in v2: - we retry the write in the signal handler when error is -EINTR (just use the qemu_write_full() function) (spoted by Jamie Lokier) - we don't do arith on void * anymore (spoted by malc) and there were no more pending commets. you can get it as a branch at: git://repo.or.cz/qemu/quintela.git fortify Later, Juan. v1: This series is a work on top of Kirill previous patches. Changes are: - I addressed all reviewers comments - Improved some error messages - Check that it is valid to return -errno (switched lots of places to just return -1). - check correctly system() result for errors. - -fstack-protector-all is only used if compiler accept it And new additions: - add WARN_UNUSED_RESULT - check pipe() use in xen code. - fix mmap_frag() returned -EINVAL when users only deal with -1 for errors. Kirill, could you coment on the series? Still not completely happy: - qemu_write_full() just loops if write() returns 0. it is only used for pipes and files, but pipes and files shouldn't give you short writes in the 1st place. Not sure what to do here. - check write() errors against -EINVAL/-EINTR/-EAGAIN series where not consistent on this regard. I didn't changed any. You can pull this series from: git://repo.or.cz/qemu/quintela.git fortify With this changes, I am able to compile qemu for all architectures in a linux host without a single warning. This is important for us (Fedora) because F12 compile all packages with -Wp,-D_FORTIFY_SOURCE=2, and we had -Werror disabled due to this. Juan Quintela (4): force to test result for qemu_write_full() check pipe() return value Check availavility of -fstack-protector-all mmap_frag() users only check for -1 error Kirill A. Shutemov (13): Introduce qemu_write_full() posix-aio-compat.c: fix warning with _FORTIFY_SOURCE block/cow.c: fix warnings with _FORTIFY_SOURCE block/qcow.c: fix warnings with _FORTIFY_SOURCE block/vmdk.o: fix warnings with _FORTIFY_SOURCE block/vvfat.c: fix warnings with _FORTIFY_SOURCE block/qcow2.c: fix warnings with _FORTIFY_SOURCE net/slirp.c: fix warning with _FORTIFY_SOURCE usb-linux.c: fix warning with _FORTIFY_SOURCE vl.c: fix warning with _FORTIFY_SOURCE monitor.c: fix warnings with _FORTIFY_SOURCE linux-user/mmap.c: fix warnings with _FORTIFY_SOURCE Enable _FORTIFY_SOURCE=2 block/cow.c | 19 ++-- block/qcow.c | 26 --- block/qcow2.c| 55 - block/vmdk.c | 50 - block/vvfat.c|9 ++- configure|4 +- hw/xen_domainbuild.c | 13 --- linux-user/mmap.c|8 -- monitor.c| 12 +- net/slirp.c |9 +++- osdep.c | 27 posix-aio-compat.c |5 +++- qemu-common.h|2 + usb-linux.c |3 +- vl.c | 22 --- 15 files changed, 216 insertions(+), 48 deletions(-)
[Qemu-devel] [PATCH 02/17] force to test result for qemu_write_full()
Signed-off-by: Juan Quintela --- qemu-common.h |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/qemu-common.h b/qemu-common.h index a2731e1..ae4f23b 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -161,7 +161,8 @@ void qemu_mutex_lock_iothread(void); void qemu_mutex_unlock_iothread(void); int qemu_open(const char *name, int flags, ...); -ssize_t qemu_write_full(int fd, const void *buf, size_t count); +ssize_t qemu_write_full(int fd, const void *buf, size_t count) +QEMU_WARN_UNUSED_RESULT; void qemu_set_cloexec(int fd); #ifndef _WIN32 -- 1.6.6
[Qemu-devel] [PATCH 01/17] Introduce qemu_write_full()
From: Kirill A. Shutemov A variant of write(2) which handles partial write. Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- osdep.c | 27 +++ qemu-common.h |1 + 2 files changed, 28 insertions(+), 0 deletions(-) diff --git a/osdep.c b/osdep.c index 1310684..c5d608e 100644 --- a/osdep.c +++ b/osdep.c @@ -243,6 +243,33 @@ int qemu_open(const char *name, int flags, ...) return ret; } +/* + * A variant of write(2) which handles partial write. + * + * Return the number of bytes transferred. + * Set errno if fewer than `count' bytes are written. + */ +ssize_t qemu_write_full(int fd, const void *buf, size_t count) +{ +ssize_t written = 0; +const char *mem = buf; + +while (count) { +ssize_t ret = write(fd, mem, count); +if (ret < 0) { +if (errno == EINTR || errno == EAGAIN) +continue; +break; +} + +count -= ret; +mem += ret; +written += ret; +} + +return written; +} + #ifndef _WIN32 /* * Creates a pipe with FD_CLOEXEC set on both file descriptors diff --git a/qemu-common.h b/qemu-common.h index d96060a..a2731e1 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -161,6 +161,7 @@ void qemu_mutex_lock_iothread(void); void qemu_mutex_unlock_iothread(void); int qemu_open(const char *name, int flags, ...); +ssize_t qemu_write_full(int fd, const void *buf, size_t count); void qemu_set_cloexec(int fd); #ifndef _WIN32 -- 1.6.6
[Qemu-devel] [PATCH 03/17] posix-aio-compat.c: fix warning with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCposix-aio-compat.o cc1: warnings being treated as errors posix-aio-compat.c: In function 'aio_signal_handler': posix-aio-compat.c:505: error: ignoring return value of 'write', declared with attribute warn_unused_result make: *** [posix-aio-compat.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- posix-aio-compat.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/posix-aio-compat.c b/posix-aio-compat.c index dc14f53..f5f3db6 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -501,8 +501,11 @@ static void aio_signal_handler(int signum) { if (posix_aio_state) { char byte = 0; +ssize_t ret; -write(posix_aio_state->wfd, &byte, sizeof(byte)); +ret = qemu_write_full(posix_aio_state->wfd, &byte, sizeof(byte)); +if (ret < 0) +die("write()"); } qemu_service_io(); -- 1.6.6
[Qemu-devel] [PATCH 05/17] block/qcow.c: fix warnings with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCblock/qcow.o cc1: warnings being treated as errors block/qcow.c: In function 'qcow_create': block/qcow.c:804: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow.c:806: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow.c:811: error: ignoring return value of 'write', declared with attribute warn_unused_result make: *** [block/qcow.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- block/qcow.c | 26 ++ 1 files changed, 22 insertions(+), 4 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index 1e3e59b..003db1e 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -750,6 +750,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) int64_t total_size = 0; const char *backing_file = NULL; int flags = 0; +int ret; /* Read out options */ while (options && options->name) { @@ -801,17 +802,34 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) } /* write all the data */ -write(fd, &header, sizeof(header)); +ret = qemu_write_full(fd, &header, sizeof(header)); +if (ret != sizeof(header)) { +ret = -1; +goto exit; +} + if (backing_file) { -write(fd, backing_file, backing_filename_len); +ret = qemu_write_full(fd, backing_file, backing_filename_len); +if (ret != backing_filename_len) { +ret = -1; +goto exit; +} + } lseek(fd, header_size, SEEK_SET); tmp = 0; for(i = 0;i < l1_size; i++) { -write(fd, &tmp, sizeof(tmp)); +ret = qemu_write_full(fd, &tmp, sizeof(tmp)); +if (ret != sizeof(tmp)) { +ret = -1; +goto exit; +} } + +ret = 0; +exit: close(fd); -return 0; +return ret; } static int qcow_make_empty(BlockDriverState *bs) -- 1.6.6
[Qemu-devel] [PATCH 04/17] block/cow.c: fix warnings with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCblock/cow.o cc1: warnings being treated as errors block/cow.c: In function 'cow_create': block/cow.c:251: error: ignoring return value of 'write', declared with attribute warn_unused_result block/cow.c:253: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result make: *** [block/cow.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- block/cow.c | 19 --- 1 files changed, 16 insertions(+), 3 deletions(-) diff --git a/block/cow.c b/block/cow.c index a70854e..3733385 100644 --- a/block/cow.c +++ b/block/cow.c @@ -209,6 +209,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) struct stat st; int64_t image_sectors = 0; const char *image_filename = NULL; +int ret; /* Read out options */ while (options && options->name) { @@ -248,11 +249,23 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) } cow_header.sectorsize = cpu_to_be32(512); cow_header.size = cpu_to_be64(image_sectors * 512); -write(cow_fd, &cow_header, sizeof(cow_header)); +ret = qemu_write_full(cow_fd, &cow_header, sizeof(cow_header)); +if (ret != sizeof(cow_header)) { +ret = -1; +goto exit; +} + /* resize to include at least all the bitmap */ -ftruncate(cow_fd, sizeof(cow_header) + ((image_sectors + 7) >> 3)); +ret = ftruncate(cow_fd, sizeof(cow_header) + ((image_sectors + 7) >> 3)); +if (ret) { +ret = -errno; +goto exit; +} + +ret = 0; +exit: close(cow_fd); -return 0; +return ret; } static void cow_flush(BlockDriverState *bs) -- 1.6.6
[Qemu-devel] [PATCH 06/17] block/vmdk.o: fix warnings with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCblock/vmdk.o cc1: warnings being treated as errors block/vmdk.c: In function 'vmdk_snapshot_create': block/vmdk.c:236: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result block/vmdk.c: In function 'vmdk_create': block/vmdk.c:775: error: ignoring return value of 'write', declared with attribute warn_unused_result block/vmdk.c:776: error: ignoring return value of 'write', declared with attribute warn_unused_result block/vmdk.c:778: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result block/vmdk.c:784: error: ignoring return value of 'write', declared with attribute warn_unused_result block/vmdk.c:790: error: ignoring return value of 'write', declared with attribute warn_unused_result block/vmdk.c:807: error: ignoring return value of 'write', declared with attribute warn_unused_result make: *** [block/vmdk.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- block/vmdk.c | 50 -- 1 files changed, 40 insertions(+), 10 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index ddc2fcb..56c28a0 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -233,7 +233,8 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file) memset(&header, 0, sizeof(header)); memcpy(&header,&hdr[4], sizeof(header)); // skip the VMDK4_MAGIC -ftruncate(snp_fd, header.grain_offset << 9); +if (ftruncate(snp_fd, header.grain_offset << 9)) +goto fail; /* the descriptor offset = 0x200 */ if (lseek(p_fd, 0x200, SEEK_SET) == -1) goto fail; @@ -717,6 +718,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) int64_t total_size = 0; const char *backing_file = NULL; int flags = 0; +int ret; // Read out options while (options && options->name) { @@ -773,22 +775,44 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) header.check_bytes[3] = 0xa; /* write all the data */ -write(fd, &magic, sizeof(magic)); -write(fd, &header, sizeof(header)); +ret = qemu_write_full(fd, &magic, sizeof(magic)); +if (ret != sizeof(magic)) { +ret = -1; +goto exit; +} +ret = qemu_write_full(fd, &header, sizeof(header)); +if (ret != sizeof(header)) { +ret = -1; +goto exit; +} -ftruncate(fd, header.grain_offset << 9); +ret = ftruncate(fd, header.grain_offset << 9); +if (ret < 0) { +ret = -1; +goto exit; +} /* write grain directory */ lseek(fd, le64_to_cpu(header.rgd_offset) << 9, SEEK_SET); for (i = 0, tmp = header.rgd_offset + gd_size; - i < gt_count; i++, tmp += gt_size) -write(fd, &tmp, sizeof(tmp)); + i < gt_count; i++, tmp += gt_size) { +ret = qemu_write_full(fd, &tmp, sizeof(tmp)); +if (ret != sizeof(tmp)) { +ret = -1; +goto exit; +} +} /* write backup grain directory */ lseek(fd, le64_to_cpu(header.gd_offset) << 9, SEEK_SET); for (i = 0, tmp = header.gd_offset + gd_size; - i < gt_count; i++, tmp += gt_size) -write(fd, &tmp, sizeof(tmp)); + i < gt_count; i++, tmp += gt_size) { +ret = qemu_write_full(fd, &tmp, sizeof(tmp)); +if (ret != sizeof(tmp)) { +ret = -1; +goto exit; +} +} /* compose the descriptor */ real_filename = filename; @@ -805,10 +829,16 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) /* write the descriptor */ lseek(fd, le64_to_cpu(header.desc_offset) << 9, SEEK_SET); -write(fd, desc, strlen(desc)); +ret = qemu_write_full(fd, desc, strlen(desc)); +if (ret != strlen(desc)) { +ret = -1; +goto exit; +} +ret = 0; +exit: close(fd); -return 0; +return ret; } static void vmdk_close(BlockDriverState *bs) -- 1.6.6
[Qemu-devel] [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCblock/vvfat.o cc1: warnings being treated as errors block/vvfat.c: In function 'commit_one_file': block/vvfat.c:2259: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result make: *** [block/vvfat.o] Error 1 CCblock/vvfat.o In file included from /usr/include/stdio.h:912, from ./qemu-common.h:19, from block/vvfat.c:27: In function 'snprintf', inlined from 'init_directories' at block/vvfat.c:871, inlined from 'vvfat_open' at block/vvfat.c:1068: /usr/include/bits/stdio2.h:65: error: call to __builtin___snprintf_chk will always overflow destination buffer make: *** [block/vvfat.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- block/vvfat.c |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index 063f731..df957e5 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s, { direntry_t* entry=array_get_next(&(s->directory)); entry->attributes=0x28; /* archive | volume label */ - snprintf((char*)entry->name,11,"QEMU VVFAT"); + memcpy(entry->name,"QEMU VVF",8); + memcpy(entry->extension,"AT ",3); } /* Now build FAT, and write back information into directory */ @@ -2256,7 +2257,11 @@ static int commit_one_file(BDRVVVFATState* s, c = c1; } -ftruncate(fd, size); +if (ftruncate(fd, size)) { +perror("ftruncate()"); +close(fd); +return -4; +} close(fd); return commit_mappings(s, first_cluster, dir_index); -- 1.6.6
[Qemu-devel] [PATCH 08/17] block/qcow2.c: fix warnings with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCblock/qcow2.o cc1: warnings being treated as errors block/qcow2.c: In function 'qcow_create2': block/qcow2.c:829: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:838: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:839: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:841: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:844: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:849: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:852: error: ignoring return value of 'write', declared with attribute warn_unused_result block/qcow2.c:855: error: ignoring return value of 'write', declared with attribute warn_unused_result make: *** [block/qcow2.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- block/qcow2.c | 55 +-- 1 files changed, 45 insertions(+), 10 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 6622eba..1bf94c5 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -842,7 +842,7 @@ static int qcow_create2(const char *filename, int64_t total_size, uint64_t tmp, offset; QCowCreateState s1, *s = &s1; QCowExtension ext_bf = {0, 0}; - +int ret; memset(s, 0, sizeof(*s)); @@ -925,7 +925,11 @@ static int qcow_create2(const char *filename, int64_t total_size, ref_clusters * s->cluster_size); /* write all the data */ -write(fd, &header, sizeof(header)); +ret = qemu_write_full(fd, &header, sizeof(header)); +if (ret != sizeof(header)) { +ret = -1; +goto exit; +} if (backing_file) { if (backing_format_len) { char zero[16]; @@ -934,25 +938,56 @@ static int qcow_create2(const char *filename, int64_t total_size, memset(zero, 0, sizeof(zero)); cpu_to_be32s(&ext_bf.magic); cpu_to_be32s(&ext_bf.len); -write(fd, &ext_bf, sizeof(ext_bf)); -write(fd, backing_format, backing_format_len); +ret = qemu_write_full(fd, &ext_bf, sizeof(ext_bf)); +if (ret != sizeof(ext_bf)) { +ret = -1; +goto exit; +} +ret = qemu_write_full(fd, backing_format, backing_format_len); +if (ret != backing_format_len) { +ret = -1; +goto exit; +} if (padding > 0) { -write(fd, zero, padding); +ret = qemu_write_full(fd, zero, padding); +if (ret != padding) { +ret = -1; +goto exit; +} } } -write(fd, backing_file, backing_filename_len); +ret = qemu_write_full(fd, backing_file, backing_filename_len); +if (ret != backing_filename_len) { +ret = -1; +goto exit; +} } lseek(fd, s->l1_table_offset, SEEK_SET); tmp = 0; for(i = 0;i < l1_size; i++) { -write(fd, &tmp, sizeof(tmp)); +ret = qemu_write_full(fd, &tmp, sizeof(tmp)); +if (ret != sizeof(tmp)) { +ret = -1; +goto exit; +} } lseek(fd, s->refcount_table_offset, SEEK_SET); -write(fd, s->refcount_table, s->cluster_size); +ret = qemu_write_full(fd, s->refcount_table, s->cluster_size); +if (ret != s->cluster_size) { +ret = -1; +goto exit; +} lseek(fd, s->refcount_block_offset, SEEK_SET); -write(fd, s->refcount_block, ref_clusters * s->cluster_size); +ret = qemu_write_full(fd, s->refcount_block, + ref_clusters * s->cluster_size); +if (ret != s->cluster_size) { +ret = -1; +goto exit; +} +ret = 0; +exit: qemu_free(s->refcount_table); qemu_free(s->refcount_block); close(fd); @@ -966,7 +1001,7 @@ static int qcow_create2(const char *filename, int64_t total_size, bdrv_close(bs); } -return 0; +return ret; } static int qcow_create(const char *filename, QEMUOptionParameter *options) -- 1.6.6
[Qemu-devel] [PATCH 09/17] net/slirp.c: fix warning with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCnet/slirp.o cc1: warnings being treated as errors net/slirp.c: In function 'slirp_smb_cleanup': net/slirp.c:470: error: ignoring return value of 'system', declared with attribute warn_unused_result make: *** [net/slirp.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- net/slirp.c |9 - 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/net/slirp.c b/net/slirp.c index 3f91c4b..b75ad16 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -464,10 +464,17 @@ int net_slirp_redir(const char *redir_str) static void slirp_smb_cleanup(SlirpState *s) { char cmd[128]; +int ret; if (s->smb_dir[0] != '\0') { snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir); -system(cmd); +ret = system(cmd); +if (!WIFEXITED(ret)) { +qemu_error("'%s' failed.\n", cmd); +} else if (WEXITSTATUS(ret)) { +qemu_error("'%s' failed. Error code: %d\n", +cmd, WEXITSTATUS(ret)); +} s->smb_dir[0] = '\0'; } } -- 1.6.6
[Qemu-devel] [PATCH 11/17] vl.c: fix warning with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCi386-softmmu/vl.o cc1: warnings being treated as errors /usr/src/RPM/BUILD/qemu-0.11.92/vl.c: In function 'qemu_event_increment': /usr/src/RPM/BUILD/qemu-0.11.92/vl.c:3404: error: ignoring return value of 'write', declared with attribute warn_unused_result /usr/src/RPM/BUILD/qemu-0.11.92/vl.c: In function 'main': /usr/src/RPM/BUILD/qemu-0.11.92/vl.c:5774: error: ignoring return value of 'write', declared with attribute warn_unused_result /usr/src/RPM/BUILD/qemu-0.11.92/vl.c:6064: error: ignoring return value of 'chdir', declared with attribute warn_unused_result /usr/src/RPM/BUILD/qemu-0.11.92/vl.c:6083: error: ignoring return value of 'chdir', declared with attribute warn_unused_result make[1]: *** [vl.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- vl.c | 22 ++ 1 files changed, 18 insertions(+), 4 deletions(-) diff --git a/vl.c b/vl.c index e070ec9..9c74038 100644 --- a/vl.c +++ b/vl.c @@ -3185,11 +3185,17 @@ static int io_thread_fd = -1; static void qemu_event_increment(void) { static const char byte = 0; +ssize_t ret; if (io_thread_fd == -1) return; -write(io_thread_fd, &byte, sizeof(byte)); +ret = write(io_thread_fd, &byte, sizeof(byte)); +if (ret < 0 && (errno != EINTR && errno != EAGAIN)) { +fprintf(stderr, "qemu_event_increment: write() filed: %s\n", +strerror(errno)); +exit (1); +} } static void qemu_event_read(void *opaque) @@ -5594,7 +5600,9 @@ int main(int argc, char **argv, char **envp) #ifndef _WIN32 if (daemonize) { uint8_t status = 1; -write(fds[1], &status, 1); +if (write(fds[1], &status, 1) != 1) { +perror("daemonize. Writing to pipe\n"); +} } else #endif fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno)); @@ -5893,7 +5901,10 @@ int main(int argc, char **argv, char **envp) if (len != 1) exit(1); - chdir("/"); +if (chdir("/")) { +perror("not able to chdir to /"); +exit(1); +} TFR(fd = qemu_open("/dev/null", O_RDWR)); if (fd == -1) exit(1); @@ -5912,7 +5923,10 @@ int main(int argc, char **argv, char **envp) fprintf(stderr, "chroot failed\n"); exit(1); } -chdir("/"); +if (chdir("/")) { +perror("not able to chdir to /"); +exit(1); +} } if (run_as) { -- 1.6.6
[Qemu-devel] [PATCH 10/17] usb-linux.c: fix warning with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCusb-linux.o cc1: warnings being treated as errors usb-linux.c: In function 'usb_host_read_file': usb-linux.c:1204: error: ignoring return value of 'fgets', declared with attribute warn_unused_result make: *** [usb-linux.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- usb-linux.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/usb-linux.c b/usb-linux.c index 5619b30..1aaa595 100644 --- a/usb-linux.c +++ b/usb-linux.c @@ -1201,9 +1201,8 @@ static int usb_host_read_file(char *line, size_t line_size, const char *device_f device_file); f = fopen(filename, "r"); if (f) { -fgets(line, line_size, f); +ret = fgets(line, line_size, f) != NULL; fclose(f); -ret = 1; #if 0 } else { if (mon) -- 1.6.6
[Qemu-devel] [PATCH 12/17] monitor.c: fix warnings with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCi386-softmmu/monitor.o cc1: warnings being treated as errors /usr/src/RPM/BUILD/qemu-0.11.92/monitor.c: In function 'do_memory_save': /usr/src/RPM/BUILD/qemu-0.11.92/monitor.c:1318: error: ignoring return value of 'fwrite', declared with attribute warn_unused_result /usr/src/RPM/BUILD/qemu-0.11.92/monitor.c: In function 'do_physical_memory_save': /usr/src/RPM/BUILD/qemu-0.11.92/monitor.c:1345: error: ignoring return value of 'fwrite', declared with attribute warn_unused_result make[1]: *** [monitor.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- monitor.c | 12 ++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/monitor.c b/monitor.c index cadf422..7fc9973 100644 --- a/monitor.c +++ b/monitor.c @@ -1330,10 +1330,14 @@ static void do_memory_save(Monitor *mon, const QDict *qdict, QObject **ret_data) if (l > size) l = size; cpu_memory_rw_debug(env, addr, buf, l, 0); -fwrite(buf, 1, l, f); +if (fwrite(buf, 1, l, f) != l) { +monitor_printf(mon, "fwrite() error in do_memory_save\n"); +goto exit; +} addr += l; size -= l; } +exit: fclose(f); } @@ -1357,11 +1361,15 @@ static void do_physical_memory_save(Monitor *mon, const QDict *qdict, if (l > size) l = size; cpu_physical_memory_rw(addr, buf, l, 0); -fwrite(buf, 1, l, f); +if (fwrite(buf, 1, l, f) != l) { +monitor_printf(mon, "fwrite() error in do_physical_memory_save\n"); +goto exit; +} fflush(f); addr += l; size -= l; } +exit: fclose(f); } -- 1.6.6
[Qemu-devel] [PATCH 13/17] linux-user/mmap.c: fix warnings with _FORTIFY_SOURCE
From: Kirill A. Shutemov CCi386-linux-user/mmap.o cc1: warnings being treated as errors /usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c: In function 'mmap_frag': /usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c:253: error: ignoring return value of 'pread', declared with attribute warn_unused_result /usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c: In function 'target_mmap': /usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c:477: error: ignoring return value of 'pread', declared with attribute warn_unused_result make[1]: *** [mmap.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- linux-user/mmap.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 144fb7c..c1c7e48 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -250,7 +250,8 @@ static int mmap_frag(abi_ulong real_start, mprotect(host_start, qemu_host_page_size, prot1 | PROT_WRITE); /* read the corresponding file data */ -pread(fd, g2h(start), end - start, offset); +if (pread(fd, g2h(start), end - start, offset) == -1) +return -1; /* put final protection */ if (prot_new != (prot1 | PROT_WRITE)) @@ -474,7 +475,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, -1, 0); if (retaddr == -1) goto fail; -pread(fd, g2h(start), len, offset); +if (pread(fd, g2h(start), len, offset) == -1) +goto fail; if (!(prot & PROT_WRITE)) { ret = target_mprotect(start, len, prot); if (ret != 0) { -- 1.6.6
[Qemu-devel] [PATCH 15/17] Enable _FORTIFY_SOURCE=2
From: Kirill A. Shutemov _FORTIFY_SOURCE is a Glibc feature which adds memory and string function protection. Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- configure |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure b/configure index 5631bbb..5556b9d 100755 --- a/configure +++ b/configure @@ -97,7 +97,7 @@ CFLAGS="-g $CFLAGS" QEMU_CFLAGS="-Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS" QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS" QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS" -QEMU_CFLAGS="-U_FORTIFY_SOURCE $QEMU_CFLAGS" +QEMU_CFLAGS="-D_FORTIFY_SOURCE=2 $QEMU_CFLAGS" QEMU_CFLAGS="-I. -I\$(SRC_PATH) $QEMU_CFLAGS" LDFLAGS="-g $LDFLAGS" -- 1.6.6
[Qemu-devel] [PATCH 14/17] check pipe() return value
Signed-off-by: Juan Quintela --- hw/xen_domainbuild.c | 13 + 1 files changed, 9 insertions(+), 4 deletions(-) diff --git a/hw/xen_domainbuild.c b/hw/xen_domainbuild.c index 20d731d..2f59856 100644 --- a/hw/xen_domainbuild.c +++ b/hw/xen_domainbuild.c @@ -156,15 +156,18 @@ quit: return; } -static void xen_domain_watcher(void) +static int xen_domain_watcher(void) { int qemu_running = 1; int fd[2], i, n, rc; char byte; -pipe(fd); +if (pipe(fd) != 0) { +qemu_log("%s: Huh? pipe error: %s\n", __FUNCTION__, strerror(errno)); +return -1; +} if (fork() != 0) -return; /* not child */ +return 0; /* not child */ /* close all file handles, except stdio/out/err, * our watch pipe and the xen interface handle */ @@ -238,7 +241,9 @@ int xen_domain_build_pv(const char *kernel, const char *ramdisk, } qemu_log("xen: created domain %d\n", xen_domid); atexit(xen_domain_cleanup); -xen_domain_watcher(); +if (xen_domain_watcher() == -1) { +goto err; +} xenstore_domain_init1(kernel, ramdisk, cmdline); -- 1.6.6
[Qemu-devel] [PATCH 16/17] Check availavility of -fstack-protector-all
Signed-off-by: Juan Quintela --- configure |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure b/configure index 5556b9d..d8af978 100755 --- a/configure +++ b/configure @@ -101,7 +101,7 @@ QEMU_CFLAGS="-D_FORTIFY_SOURCE=2 $QEMU_CFLAGS" QEMU_CFLAGS="-I. -I\$(SRC_PATH) $QEMU_CFLAGS" LDFLAGS="-g $LDFLAGS" -gcc_flags="-Wold-style-declaration -Wold-style-definition" +gcc_flags="-Wold-style-declaration -Wold-style-definition -fstack-protector-all" cat > $TMPC << EOF int main(void) { } EOF -- 1.6.6
[Qemu-devel] [PATCH 17/17] mmap_frag() users only check for -1 error
Signed-off-by: Juan Quintela --- linux-user/mmap.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/linux-user/mmap.c b/linux-user/mmap.c index c1c7e48..25fc0b2 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -243,7 +243,7 @@ static int mmap_frag(abi_ulong real_start, possible while it is a shared mapping */ if ((flags & MAP_TYPE) == MAP_SHARED && (prot & PROT_WRITE)) -return -EINVAL; +return -1; /* adjust protection to be able to read */ if (!(prot1 & PROT_WRITE)) -- 1.6.6
Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..
Jamie Lokier wrote: > john cooper wrote: >> As before a cpu feature 'check' option is added which warns when >> feature flags (either implicit in a cpu model or explicit on the >> command line) would have otherwise been quietly unavailable to a >> guest: >> >> # qemu-system-x86_64 ... -cpu Nehalem,check >> warning: host cpuid _0001 lacks requested flag 'sse4.2' [0x0010] >> warning: host cpuid _0001 lacks requested flag 'popcnt' [0x0080] > > That's a nice feature. Can we have a 'checkfail' option which refuses > to run if a requested capability isn't available? Thanks. Certainly, others have requested the same. Let's resolve the issue at hand first. > I foresee wanting to iterate over the models and pick the latest one > which a host supports - on the grounds that you have done the hard > work of ensuring it is a reasonably good performer, while "probably" > working on another host of similar capability when a new host is made > available. That's a fairly close use case to that of safe migration which was one of the primary motivations to identify the models being discussed. Although presentation and administration of such was considered the domain of management tools. -john -- john.coo...@redhat.com
Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..
On Wed, Jan 20, 2010 at 03:09:53PM -0500, john cooper wrote: > Anthony Liguori wrote: > > On 01/19/2010 02:03 PM, Chris Wright wrote: > >> * Anthony Liguori (anth...@codemonkey.ws) wrote: > >> > >>> I'm very much against having -cpu Nehalem. The whole point of this is > >>> to make things easier for a user and for most of the users I've > >>> encountered, -cpu Nehalem is just as obscure as -cpu > >>> qemu64,-sse3,+vmx,... > >>> > >> What name will these users know? FWIW, it makes sense to me as it is. > >> > > > > Whatever is in /proc/cpuinfo. > > $ grep name /proc/cpuinfo > model name : Intel(R) Core(TM)2 Duo CPU E8400 @ 3.00GHz > > Which is detailing that exact cpu vs. the class > of which it is a member. So are you suggesting > to map all instances of processors called out > in /proc/cpuinfo into one of the three defined > models? We can certainly do that however I was > looking for a more terse and simplified solution > at this level while deferring more ornate mapping > schemes to management tools. > > Still at the user facing CLI this doesn't strike > me as the most friendly encoding of a -cpu . To be honest all possible naming schemes for '-cpu ' are just as unfriendly as each other. The only user friendly option is '-cpu host'. IMHO, we should just pick a concise naming scheme & document it. Given they are all equally unfriendly, the one that has consistency with vmware naming seems like a mild winner. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..
On 01/20/2010 02:26 PM, Daniel P. Berrange wrote: To be honest all possible naming schemes for '-cpu' are just as unfriendly as each other. The only user friendly option is '-cpu host'. IMHO, we should just pick a concise naming scheme& document it. Given they are all equally unfriendly, the one that has consistency with vmware naming seems like a mild winner. IIUC, VMware uses "Group A, Group B", etc. which is pretty close to saying Family 10h IMHO. Regards, Anthony Liguori Daniel
[Qemu-devel] [PATCH] Tell users about out-of-memory errors
Aborting without an error message when memory is short is not helpful, so print the reason for the abort. Try qemu -m 100 to force an out-of-memory error. Signed-off-by: Stefan Weil --- osdep.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/osdep.c b/osdep.c index 1310684..5d4e810 100644 --- a/osdep.c +++ b/osdep.c @@ -52,6 +52,7 @@ static void *oom_check(void *ptr) { if (ptr == NULL) { +perror("qemu (memory allocation)"); abort(); } return ptr; @@ -91,8 +92,11 @@ void *qemu_memalign(size_t alignment, size_t size) int ret; void *ptr; ret = posix_memalign(&ptr, alignment, size); -if (ret != 0) +if (ret != 0) { +fprintf(stderr, "Failed to allocate %zu B: %s\n", +size, strerror(errno)); abort(); +} return ptr; #elif defined(CONFIG_BSD) return oom_check(valloc(size)); -- 1.6.5
[Qemu-devel] [PATCH] Documentation: Add missing texi description for command line options
Some more command line options had entries for command line help, but documentation for texi and derived formats (man, html, info) was missing. For conditional options, the texi documentation was added unconditionally. This seems reasonable because typically man pages are shared, and html users expect to see one documentation (not several nearly identical documents for the different systems). Signed-off-by: Stefan Weil --- qemu-options.hx | 35 +++ 1 files changed, 35 insertions(+), 0 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index f5bd671..559f9ac 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -566,6 +566,8 @@ DEF("g", 1, QEMU_OPTION_g , "-g WxH[xDEPTH] Set the initial graphical resolution and depth\n") #endif STEXI +...@item -g @var{widt...@var{height}[x@var{depth}] +Set the initial graphical resolution and depth (PPC, SPARC only). ETEXI DEF("vnc", HAS_ARG, QEMU_OPTION_vnc , @@ -1599,6 +1601,10 @@ non graphical mode. ETEXI DEF("qmp", HAS_ARG, QEMU_OPTION_qmp, \ "-qmp devlike -monitor but opens in 'control' mode\n") +STEXI +...@item -qmp @var{dev} +Like -monitor but opens in 'control' mode. +ETEXI DEF("mon", HAS_ARG, QEMU_OPTION_mon, \ "-mon chardev=[name][,mode=readline|control][,default]\n") @@ -1715,6 +1721,16 @@ DEF("xen-attach", 0, QEMU_OPTION_xen_attach, "-xen-attach attach to existing xen domain\n" "xend will use this when starting qemu\n") #endif +STEXI +...@item -xen-domid @var{id} +Specify xen guest domain @var{id} (XEN only). +...@item -xen-create +Create domain using xen hypercalls, bypassing xend. +Warning: should not be used when xend is in use (XEN only). +...@item -xen-attach +Attach to existing xen domain. +xend will use this when starting qemu (XEN only). +ETEXI DEF("no-reboot", 0, QEMU_OPTION_no_reboot, \ "-no-reboot exit instead of rebooting\n") @@ -1902,16 +1918,22 @@ ETEXI DEF("show-cursor", 0, QEMU_OPTION_show_cursor, \ "-show-cursorshow cursor\n") STEXI +...@item -show-cursor +Show cursor. ETEXI DEF("tb-size", HAS_ARG, QEMU_OPTION_tb_size, \ "-tb-size n set TB size\n") STEXI +...@item -tb-size @var{n} +Set TB size. ETEXI DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \ "-incoming p prepare for incoming migration, listen on port p\n") STEXI +...@item -incoming @var{port} +Prepare for incoming migration, listen on @var{port}. ETEXI DEF("nodefaults", 0, QEMU_OPTION_nodefaults, \ @@ -1946,14 +1968,27 @@ DEF("prom-env", HAS_ARG, QEMU_OPTION_prom_env, "-prom-env variable=value\n" "set OpenBIOS nvram variables\n") #endif +STEXI +...@item -prom-env @var{variabl...@var{value} +Set OpenBIOS nvram @var{variable} to given @var{value} (PPC, SPARC only). +ETEXI #if defined(TARGET_ARM) || defined(TARGET_M68K) DEF("semihosting", 0, QEMU_OPTION_semihosting, "-semihostingsemihosting mode\n") #endif +STEXI +...@item -semihosting +Semihosting mode (ARM, M68K only). +ETEXI #if defined(TARGET_ARM) DEF("old-param", 0, QEMU_OPTION_old_param, "-old-param old param mode\n") #endif +STEXI +...@item -old-param +Old param mode (ARM only). +ETEXI + DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig, "-readconfig \n") STEXI -- 1.6.5
Re: [Qemu-devel] [PATCH] Porting TCG to alpha platform
On 01/20/2010 09:19 AM, identifier scorpio wrote: Below i'll append my newly generated patch against stable-0.10, in case it is mangled, i also put it in the attachment. For the record, the inline portion was again mangled; the attachment is fine. in qemu_ld/st, we must use $16,$17,$18 as temporaries, because pass them as argument to helper functions such as qemu_ld/st_helpers[]. Ah, yes, I forgot about that. With I/J constraints you don't need this special casing. I'm not very familiar with I/J constraints and i'll study them later. I = uint8_t, to be used with the second arithmetic input. J = 0, to be used with the first arithmetic input (i.e. $31). +tcg_out_inst2(s, opc^4, TMP_REG1, 1); +/* record relocation infor */ +tcg_out_reloc(s, s->code_ptr, R_ALPHA_REFQUAD, label_index, 0); +s->code_ptr += 4; Bug: You've applied the relocation to the wrong instruction. Bug: What's with the "opc^4"? what did you mean that i "applied the relocation to the wrong instruction", couldn't i apply relocation to INDEX_op_brcond_i32 operation? With tcg_out_inst2 you emit the branch instruction, which calls tcg_out32, which increments s->code_ptr. Next you call tcg_out_reloc with the updated s->code_ptr, which means that the relocation gets applied to the instruction *after* the branch. Finally, you increment s->code_ptr *again*, which means that the instruction after the branch is in fact completely uninitialized. and opc^4 here is used to toggle between OP_BLBC(opcode 0x38) and OP_BLBS(opcode 0x3c), ugly code :) It does beg the question of why you're reversing the sense of the jump at all. Just because the branch is forward doesn't mean its condition should be changed. I think that's definitely a bug. The sense of the jump should be exactly the same, never mind the direction of the jump. Oh... I see what you're doing here -- you're generating the entire branch instruction in patch_reloc, and you're generating branch over branch here. That's both confusing and unnecessary. We should do static void patch_reloc(uint8_t *x_ptr, int type, tcg_target_long value, tcg_target_long addend) { uint32_t *code_ptr = (uint32_t *)x; uint32_t insn = *code_ptr; value += addend; switch (type) { case R_ALPHA_BRADDR: value -= (long)x_ptr + 4; if ((value & 3) || value < -0x40 || value >= 0x40) { tcg_abort(); } *code_ptr = insn | INSN_DISP21(val >> 2); break; default: tcg_abort(); } } so as to apply the branch address relocation to the existing insn. Which lets you write static void tcg_out_br(TCGContext *s, int opc, int ra, int label_index) { TCGLabel *l = &s->labels[label_index]; tcg_target_long value; if (l->has_value) { value = l->u.value; value -= (long)s->code_ptr + 4; if ((value & 3) || value < -0x40 || value >= 0x40) { tcg_abort(); } value >>= 2; } else { tcg_out_reloc(s, s->code_ptr, R_ALPHA_BRADDR, label_index, 0); value = 0; } tcg_out_fmt_br(s, opc, ra, value); } static void tcg_out_brcond(TCGContext *s, ...) { // Emit comparison into TMP_REG1. opc = (cond & 1) ? INSN_BLBC : INSN_BLBS; tcg_out_br(s, opc, TMP_REG1, label_index); } Isn't that much clearer? +tcg_out_mov(s, r1, addr_reg); +tcg_out_mov(s, r0, addr_reg); + +#if TARGET_LONG_BITS == 32 +/* if VM is of 32-bit arch, clear higher 32-bit of addr */ +tcg_out_fmt_opi(s, INSN_ZAPNOT, r0, 0x0f, r0); +tcg_out_fmt_opi(s, INSN_ZAPNOT, r1, 0x0f, r1); +#endif I suggest creating a static inline tcg_out_mov_addr(TCGContext *s, int rd, int rs) { if (TARGET_LONG_BITS == 32) { tcg_out_fmt_opi(s, INSN_ZAPNOT, rs, 0x0f, rd); } else { tcg_out_mov(s, rd, rs); } } and using that throughout qemu_ld/st. That will save some redundant moves you are creating there, as well as removing some conditional compilation. Indeed, I would suggest replacing all of the conditional compilation vs TARGET_LONG_BITS with normal if's. ... Of course, in this particular case, that zapnot is redundant with the INSN_AND that follows, for both R0 and R1. +tcg_out_movi(s, TCG_TYPE_I64, TMP_REG1, (tcg_target_long)qemu_ld_helpers[s_bits]); +tcg_out_push(s, addr_reg); +//tcg_out_push(s, TCG_REG_26); +//tcg_out_push(s, TCG_REG_15); +tcg_out_mov(s, TCG_REG_27, TMP_REG1); +tcg_out_fmt_jmp(s, INSN_CALL, TCG_REG_26, TMP_REG1, 0); +//tcg_out_pop(s, TCG_REG_15); +//tcg_out_pop(s, TCG_REG_26); +tcg_out_pop(s, addr_reg); You need not save and restore ADDR_REG; it is not used after the call. Also, you can load the address into $27 directly and not use the temp. +*(uint32_t *)label1_ptr = (uint32_t) \ +( INSN_BNE | ( TMP_REG1 << 21 ) | ( val & 0x1f)); Frankly, I don't really
[Qemu-devel] [PATCH 1/3] qdev: Add help for device properties
When called with property "?", a list of supported properties will be printed (instead of an error message). This is useful for command lines like qemu -device e1000,? and was already standard for other options like model=? Signed-off-by: Stefan Weil --- hw/qdev-properties.c | 15 +-- 1 files changed, 13 insertions(+), 2 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 277ff9e..8547ad2 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -544,8 +544,19 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value) prop = qdev_prop_find(dev, name); if (!prop) { -fprintf(stderr, "property \"%s.%s\" not found\n", -dev->info->name, name); +if (strcmp(name, "?") != 0) { +fprintf(stderr, "property \"%s.%s\" not found\n", +dev->info->name, name); +} else { +fprintf(stderr, "supported properties:\n"); +if (dev->info->props != NULL) { +Property *props = dev->info->props; +while (props->name) { +fprintf(stderr, "%s.%s\n", dev->info->name, props->name); +props++; +} +} +} return -1; } if (!prop->info->parse) { -- 1.6.5
[Qemu-devel] [PATCH 2/3] qdev: Add help for property value
When called with property value "?", a help text will be printed (instead of an error message). This is useful for command lines like qemu -device e1000,mac=? and is already standard for other command line options. A better help text could be provided by extending the Property structure with a desc field. Signed-off-by: Stefan Weil --- hw/qdev-properties.c |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 8547ad2..f5ca05f 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -565,8 +565,13 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value) return -1; } if (prop->info->parse(dev, prop, value) != 0) { -fprintf(stderr, "property \"%s.%s\": failed to parse \"%s\"\n", -dev->info->name, name, value); +if (strcmp(value, "?") != 0) { +fprintf(stderr, "property \"%s.%s\": failed to parse \"%s\"\n", +dev->info->name, name, value); +} else { +fprintf(stderr, "%s.%s=%s\n", +dev->info->name, name, prop->info->name); +} return -1; } return 0; -- 1.6.5
[Qemu-devel] [PATCH 3/3] Documentation: Improve command line help for -device option
* Fix column for help text. * Give some more help, especially for the new '?' parameters. Signed-off-by: Stefan Weil --- qemu-options.hx | 15 --- 1 files changed, 12 insertions(+), 3 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index ee60d8a..ef82cac 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -403,11 +403,20 @@ Network adapter that supports CDC ethernet and RNDIS protocols. ETEXI DEF("device", HAS_ARG, QEMU_OPTION_device, -"-device driver[,options] add device\n") -STEXI -...@item -device @var{driver}[,@var{option}[,...]] +"-device driver[,option[=value][,...]]\n" +"add device (based on driver) with default or\n" +"user defined options\n" +"use -device ? to print all possible drivers\n" +"use -device driver,? to print all possible options\n" +"use -device driver,option=? to print a help for value\n") +STEXI +...@item -device @var{driver}[,@var{option...@var{value}][,...]] Add device @var{driver}. Depending on the device type, -...@var{option} (typically @var{ke...@var{value}) may be useful. +...@var{option} (with default or given @var{value}) may be useful. +To get a help on possible @var{driver}s, @var{option}s or @var{value}s, use +...@code{-device ?}, +...@code{-device @var{driver},?} or +...@code{-device @var{driver},@var{option}=?}. ETEXI DEF("name", HAS_ARG, QEMU_OPTION_name, -- 1.6.5
Re: [Qemu-devel] [PATCH] Documentation: Add missing documentation for qdev related command line options
Markus Armbruster schrieb: > Stefan Weil writes: > >> Markus Armbruster schrieb: >>> Stefan Weil writes: >>> The command line options -device, -nodefaults, -readconfig, -writeconfig had entries for command line help, but documentation for texi and derived formats (man, html, info) was missing. This also required moving "@end table" to the end of qemu-options.hx again. Signed-off-by: Stefan Weil --- qemu-options.hx | 25 + 1 files changed, 21 insertions(+), 4 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index e2edd71..b2d04e2 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -404,6 +404,12 @@ ETEXI DEF("device", HAS_ARG, QEMU_OPTION_device, "-device driver[,options] add device\n") +STEXI +...@item -device @var{driver}[,@var{option}[,...]] +Add device @var{driver}. Depending on the device type, +...@var{option} (typically @var{ke...@var{value}) may be useful. +ETEXI + >>> While there, would you mind improving --help for -device a bit? It's >>> too terse, and it doesn't start the help text in column 16 like the >>> other options do. >> Hi Markus, >> >> this needs a little more work. I just had a look on the code, >> and there is no online help for the possible options (key, value). > > What I had in mind was just to bring it up to par with your patch to the > texi, but... > >> If you (and especially those who have commit rights) agree, >> I could provide these three additional patches: >> >> * Add online help for properties (qemu -device driver,?) >> * Add online help for property value (qemu -device driver,property=?) >> * Update documentation for command line option -device > > ... a patch to provide that is very desirable! > > I figure the best way to document available properties and there values > is a self-documenting struct PropertyInfo: add a doc member, extend > DEFINE_PROP() & friends to set it, fix up users to pass NULL, and so > forth. We can then replace the NULL by something useful at our leisure. > >> There is already an online help for the driver (qemu -device ?). I cannot spend too much time on this, but a very basic help for "?" is implemented by the patch series I just sent to the list. The new feature was already very helpful for me, but it still can be improved, of course: the driver list contains shows too many drivers and is not nicely formatted, the help text for the values could be more user friendly, ...
[Qemu-devel] Re: [PATCH 01/14] arm host: Fix compiler warning
Stefan Weil schrieb: > Compilation for arm (native or cross) results in this > warning: > > fpu/softfloat-native.c: In function ‘float64_round_to_int’: > fpu/softfloat-native.c:387: error: control reaches end of non-void function > > float64_round_to_int uses special assembler code for arm > and has no explicit return value. > > As there is no obvious reason why arm should need special > code, all fpu related conditionals were removed. > The remaining code is standard (C99) and compiles for arm, > too. > > Signed-off-by: Stefan Weil > --- > fpu/softfloat-native.c | 20 > fpu/softfloat-native.h |7 --- > 2 files changed, 0 insertions(+), 27 deletions(-) > The "subject" line was wrong: the patch is complete and can be applied as it is. I'm sorry if I created confusion. Stefan Weil
Re: [Qemu-devel] [PATCH] Tell users about out-of-memory errors
On Wed, 20 Jan 2010, Stefan Weil wrote: > Aborting without an error message when memory is short > is not helpful, so print the reason for the abort. > > Try > qemu -m 100 > to force an out-of-memory error. The patch is wrong for, at least, Windows. > > Signed-off-by: Stefan Weil > --- > osdep.c |6 +- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/osdep.c b/osdep.c > index 1310684..5d4e810 100644 > --- a/osdep.c > +++ b/osdep.c > @@ -52,6 +52,7 @@ > static void *oom_check(void *ptr) > { > if (ptr == NULL) { > +perror("qemu (memory allocation)"); > abort(); > } > return ptr; > @@ -91,8 +92,11 @@ void *qemu_memalign(size_t alignment, size_t size) > int ret; > void *ptr; > ret = posix_memalign(&ptr, alignment, size); > -if (ret != 0) > +if (ret != 0) { > +fprintf(stderr, "Failed to allocate %zu B: %s\n", > +size, strerror(errno)); > abort(); > +} > return ptr; > #elif defined(CONFIG_BSD) > return oom_check(valloc(size)); > -- mailto:av1...@comtv.ru
[Qemu-devel] Re: [PATCH] Add definitions for current cpu models..
On Monday 18 January 2010, john cooper wrote: > +.name = "Conroe", > +.level = 2, > +.vendor1 = CPUID_VENDOR_INTEL_1, > +.vendor2 = CPUID_VENDOR_INTEL_2, > +.vendor3 = CPUID_VENDOR_INTEL_3, > +.family = 6, /* P6 */ > +.model = 2, that looks wrong -- what is model 2 actually? > +.stepping = 3, > +.features = PPRO_FEATURES | > +CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |/* note 1 */ > +CPUID_PSE36,/* note 2 */ > +.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_SSSE3, > +.ext2_features = (PPRO_FEATURES & CPUID_EXT2_MASK) | > +CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, > +.ext3_features = CPUID_EXT3_LAHF_LM, > +.xlevel = 0x800A, > +.model_id = "Intel Celeron_4x0 (Conroe/Merom Class Core 2)", > +}, Celeron_4x0 is a rather bad example, because it is based on the single-core Conroe-L, which is family 6 / model 22 unlike all the dual- and quad-core Merom/Conroe that are model 15. > +{ > +.name = "Penryn", > +.level = 2, > +.vendor1 = CPUID_VENDOR_INTEL_1, > +.vendor2 = CPUID_VENDOR_INTEL_2, > +.vendor3 = CPUID_VENDOR_INTEL_3, > +.family = 6, /* P6 */ > +.model = 2, > +.stepping = 3, > +.features = PPRO_FEATURES | > +CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |/* note 1 */ > +CPUID_PSE36,/* note 2 */ > +.ext_features = CPUID_EXT_SSE3 | > +CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_SSE41, > +.ext2_features = (PPRO_FEATURES & CPUID_EXT2_MASK) | > +CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, > +.ext3_features = CPUID_EXT3_LAHF_LM, > +.xlevel = 0x800A, > +.model_id = "Intel Core 2 Duo P9xxx (Penryn Class Core 2)", > +}, This would be model 23 for Penryn-class Xeon/Core/Pentium/Celeron processors without L3 cache. > +{ > +.name = "Nehalem", > +.level = 2, > +.vendor1 = CPUID_VENDOR_INTEL_1, > +.vendor2 = CPUID_VENDOR_INTEL_2, > +.vendor3 = CPUID_VENDOR_INTEL_3, > +.family = 6, /* P6 */ > +.model = 2, > +.stepping = 3, > +.features = PPRO_FEATURES | > +CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |/* note 1 */ > +CPUID_PSE36,/* note 2 */ > +.ext_features = CPUID_EXT_SSE3 | > +CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_SSE41 | > +CPUID_EXT_SSE42 | CPUID_EXT_POPCNT, > +.ext2_features = (PPRO_FEATURES & CPUID_EXT2_MASK) | > +CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, > +.ext3_features = CPUID_EXT3_LAHF_LM, > +.xlevel = 0x800A, > +.model_id = "Intel Core i7 9xx (Nehalem Class Core i7)", > +}, Apparently, not all the i7-9xx CPUs are Nehalem, the i7-980X is supposed to be Westmere, which has more features. Because of the complexity, I'd recommend passing down the *model* number of the emulated CPU, the interesting Intel ones (those supported by KVM) being: 15-6: CedarMill/Presler/Dempsey/Tulsa (Pentium 4/Pentium D/Xeon 50xx/Xeon 71xx) 6-14: Yonah/Sossaman (Celeron M4xx, Core Solo/Duo, Pentium Dual-Core T1000, Xeon ULV) 6-15: Merom/Conroe/Kentsfield/Woodcrest/Clovertown/Tigerton (Celeron M5xx/E1xxx/T1xxx, Pentium T2xxx/T3xxx/E2xxx,Core 2 Solo U2xxx, Core 2 Duo E4xxx/E6xxx/Q6xxx/T5xxx/T7xxx/L7xxx/U7xxx/SP7xxx, Xeon 30xx/32xx/51xx/52xx/72xx/73xx) 6-22: Penryn/Wolfdale/Yorkfield/Harpertown (Celeron 7xx/9xx/SU2xxx/T3xxx/E3xxx, Pentium T4xxx/SU2xxx/SU4xxx/E5xxx/E6xxx, Core 2 Solo SU3xxx, Core 2 Duo P/SU/T6xxx/x8xxx/x9xxx, Xeon 31xx/33xx/52xx/54xx) 6-26: Gainestown/Bloomfield (Xeon 35xx/55xx, Core i7-9xx) 6-28: Atom 6-29: Dunnington (Xeon 74xx) 6-30: Lynnfield/Clarksfield/JasperForest (Xeon 34xx, Core i7-8xx, Core i7-xxxQM, Core i5-7xx) 6-37: Arrandale/Clarkdale (Dual-Core Core i3/i5/i7) 6-44: Gulftown (six-core) Arnd
Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..
* Daniel P. Berrange (berra...@redhat.com) wrote: > To be honest all possible naming schemes for '-cpu ' are just as > unfriendly as each other. The only user friendly option is '-cpu host'. > > IMHO, we should just pick a concise naming scheme & document it. Given > they are all equally unfriendly, the one that has consistency with vmware > naming seems like a mild winner. Heh, I completely agree, and was just saying the same thing to John earlier today. May as well be -cpu {foo,bar,baz} since the meaning for those command line options must be well-documented in the man page. This is from an EVC kb article[1]: ESX/ESXi 4.0 supports the following EVC modes: * AMD Opteron™ Generation 1 (Rev. E) * AMD Opteron™ Generation 2 (Rev. F) * AMD Opteron™ Generation 3 (Greyhound) * Intel® Xeon® Core2 (Merom) * Intel® Xeon® 45nm Core2 (Penryn) * Intel® Xeon® Core i7 (Nehalem) Not that different from John's proposal. thanks, -chris [1] http://kb.vmware.com/kb/1005764
Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..
Chris Wright wrote: > * Daniel P. Berrange (berra...@redhat.com) wrote: >> To be honest all possible naming schemes for '-cpu ' are just as >> unfriendly as each other. The only user friendly option is '-cpu host'. >> >> IMHO, we should just pick a concise naming scheme & document it. Given >> they are all equally unfriendly, the one that has consistency with vmware >> naming seems like a mild winner. > > Heh, I completely agree, and was just saying the same thing to John > earlier today. May as well be -cpu {foo,bar,baz} since the meaning for > those command line options must be well-documented in the man page. I can appreciate the concern of wanting to get this as "correct" as possible. But ultimately we just need three unique tags which ideally have some relation to their associated architectures. The diatribes available from /proc/cpuinfo while generally accurate don't really offer any more of a clue to the model group, and in their unmodified form are rather unwieldy as command line flags. > This is from an EVC kb article[1]: Here is a pointer to a more detailed version: http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1003212 We probably should also add an option to dump out the full set of qemu-side cpuid flags for the benefit of users and upper level tools. -john -- john.coo...@redhat.com
Re: [Qemu-devel] [PATCH] Porting TCG to alpha platform
Thank Mr. Weil for your reply. > > Maybe you can also try the TCG interpreter (TCI) from > http://repo.or.cz/w/qemu/ar7.git. > In theory, it supports any host architecture with or > without native TCG > support. > > It was tested successful with some basic tests on x86, > mips, ppc and arm, > so I hope it will run on alpha, too. > so that means i have to learn another set of interface? is TCI more simple or straightforward than TCG? Dong Weiyu. ___ 好玩贺卡等你发,邮箱贺卡全新上线! http://card.mail.cn.yahoo.com/
[Qemu-devel] possible qemu miscompilation by latest gcc
Hi folks- Just wanted to let you know that perhaps the function helper_neon_rshl_s8() is being miscompiled by the latest gcc. I'm using qemu 0.12.2 and gcc rev 156103, which is a pre-version of gcc 4.5. This is on an x86 machine running Ubuntu 9.10. At -O2 or higher this is the resulting object code: 2060 : 2060: 31 c0 xor%eax,%eax 2062: c3 ret If this is not the intended result, then either the function has a latent bug or else the compiler is doing something bad. Hope this is helpful, John Regehr
Re: [Qemu-devel] [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
On Wed, Jan 20, 2010 at 09:14:03PM +0100, Juan Quintela wrote: > From: Kirill A. Shutemov > > CCblock/vvfat.o > cc1: warnings being treated as errors > block/vvfat.c: In function 'commit_one_file': > block/vvfat.c:2259: error: ignoring return value of 'ftruncate', declared > with attribute warn_unused_result > make: *** [block/vvfat.o] Error 1 > CCblock/vvfat.o > In file included from /usr/include/stdio.h:912, > from ./qemu-common.h:19, > from block/vvfat.c:27: > In function 'snprintf', > inlined from 'init_directories' at block/vvfat.c:871, > inlined from 'vvfat_open' at block/vvfat.c:1068: > /usr/include/bits/stdio2.h:65: error: call to __builtin___snprintf_chk will > always overflow destination buffer > make: *** [block/vvfat.o] Error 1 > > Signed-off-by: Kirill A. Shutemov > Signed-off-by: Juan Quintela > --- > block/vvfat.c |9 +++-- > 1 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/block/vvfat.c b/block/vvfat.c > index 063f731..df957e5 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s, > { > direntry_t* entry=array_get_next(&(s->directory)); > entry->attributes=0x28; /* archive | volume label */ > - snprintf((char*)entry->name,11,"QEMU VVFAT"); > + memcpy(entry->name,"QEMU VVF",8); > + memcpy(entry->extension,"AT ",3); > } > Before the change extension was initialized to "AT\0" after it is "AT " > /* Now build FAT, and write back information into directory */ > @@ -2256,7 +2257,11 @@ static int commit_one_file(BDRVVVFATState* s, > c = c1; > } > > -ftruncate(fd, size); > +if (ftruncate(fd, size)) { > +perror("ftruncate()"); > +close(fd); > +return -4; > +} > close(fd); > > return commit_mappings(s, first_cluster, dir_index); > -- > 1.6.6 > > -- Gleb.
Re: [Qemu-devel] Stop using "which" in ./configure
On Tue, Jan 19, 2010, Stefan Weil wrote: > I did not test the whole patch, but I think this would be better: > +type "$local_command" >/dev/null 2>&1 Attaching an updated patch Thanks -- Loïc Minier >From 1c0b63fb9fc735a6d367a65a6ed1b998942fb6a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Minier?= Date: Tue, 19 Jan 2010 11:05:00 +0100 Subject: [PATCH] Add and use has() and path_of() funcs Add has() and path_of() funcs and use them across configure; has() will test whether a command or builtin is available; path_of() will search the PATH for executables and return the full pathname if found. --- configure | 53 - 1 files changed, 44 insertions(+), 9 deletions(-) diff --git a/configure b/configure index baa2800..db97a2c 100755 --- a/configure +++ b/configure @@ -27,6 +27,43 @@ compile_prog() { $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null } +# check whether a command is available to this shell (may be either an +# executable or a builtin) +has() { +local_command="$1" +type "$local_command" >/dev/null 2>&1 +} + +# search for an executable in PATH +path_of() { +local_command="$1" +local_ifs="$IFS" +local_dir="" + +# pathname has a dir component? +if [ "${local_command#*/}" != "$local_command" ]; then +if [ -x "$local_command" ] && [ ! -d "$local_command" ]; then +echo "$local_command" +return 0 +fi +fi +if [ -z "$local_command" ]; then +return 1 +fi + +IFS=: +for local_dir in $PATH; do +if [ -x "$local_dir/$local_command" ] && [ ! -d "$local_dir/$local_command" ]; then +echo "$local_dir/$local_command" +IFS="$local_ifs" +return 0 +fi +done +# not found +IFS="$local_ifs" +return 1 +} + # default parameters cpu="" prefix="" @@ -763,7 +800,7 @@ fi # Solaris specific configure tool chain decisions # if test "$solaris" = "yes" ; then - solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"` + solinst=`path_of $install` if test -z "$solinst" ; then echo "Solaris install program not found. Use --install=/usr/ucb/install or" echo "install fileutils from www.blastwave.org using pkg-get -i fileutils" @@ -776,7 +813,7 @@ if test "$solaris" = "yes" ; then echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install" exit 1 fi - sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"` + sol_ar=`path_of ar` if test -z "$sol_ar" ; then echo "Error: No path includes ar" if test -f /usr/ccs/bin/ar ; then @@ -969,7 +1006,7 @@ fi # pkgconfig probe pkgconfig="${cross_prefix}pkg-config" -if ! test -x "$(which $pkgconfig 2>/dev/null)"; then +if ! has $pkgconfig; then # likely not cross compiling, or hope for the best pkgconfig=pkg-config fi @@ -977,7 +1014,7 @@ fi ## # Sparse probe if test "$sparse" != "no" ; then - if test -x "$(which cgcc 2>/dev/null)"; then + if has cgcc; then sparse=yes else if test "$sparse" = "yes" ; then @@ -993,7 +1030,7 @@ fi if $pkgconfig sdl --modversion >/dev/null 2>&1; then sdlconfig="$pkgconfig sdl" _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'` -elif which sdl-config >/dev/null 2>&1; then +elif has sdl-config; then sdlconfig='sdl-config' _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'` else @@ -1424,8 +1461,7 @@ EOF fi else if test "$kvm" = "yes" ; then - if [ -x "`which awk 2>/dev/null`" ] && \ - [ -x "`which grep 2>/dev/null`" ]; then + if has awk && has grep; then kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \ | grep "error: " \ | awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'` @@ -1694,8 +1730,7 @@ fi # Check if tools are available to build documentation. if test "$docs" != "no" ; then - if test -x "`which texi2html 2>/dev/null`" -a \ - -x "`which pod2man 2>/dev/null`" ; then + if has texi2html && has pod2man; then docs=yes else if test "$docs" = "yes" ; then -- 1.6.5
Re: [Qemu-devel] [PATCH 07/10] qcow2: Improve error handling in update_refcount
Am 19.01.2010 19:51, schrieb Christoph Hellwig: > On Mon, Jan 18, 2010 at 01:11:33PM +0100, Kevin Wolf wrote: >> If update_refcount fails, try to undo any changes made so far to avoid >> inconsistencies in the image file. >> >> Signed-off-by: Kevin Wolf >> --- >> block/qcow2-refcount.c | 32 +--- >> 1 files changed, 25 insertions(+), 7 deletions(-) >> > >> +/* >> + * Try do undo any updates if an error is returned (This may succeed in >> + * some cases like ENOSPC for allocating a new refcount block) >> + */ >> +if (ret < 0) { >> +int dummy; >> +dummy = update_refcount(bs, offset, cluster_offset - offset, >> -addend); > > So we recursively call into update_refcount here. What happens an error > causes all updates to fail? We're only reverting writes that have succeeded. If reverting fails, there are no successful writes and update_refcount will be called with a length of 0 (which never fails). In the worst case we're only reverting one cluster less with each recursive call - however, I don't think that's a realistic scenario. Kevin
Re: [Qemu-devel] [PATCH 08/10] qcow2: Allow updating no refcounts
Am 19.01.2010 19:53, schrieb Christoph Hellwig: >> #endif >> -if (length <= 0) >> +if (length < 0) { >> return -EINVAL; >> +} >> + >> start = offset & ~(s->cluster_size - 1); >> last = (offset + length - 1) & ~(s->cluster_size - 1); >> for(cluster_offset = start; cluster_offset <= last; > > So for legnth = 0, last will equal start and we'll never go through > the loop. But should we really bother with all the other work in the > function or just return 0 early on? I'm not a big fan of special-casing for no real reason ("all the other work" basically is calculating start and last and skipping two ifs - and length = 0 is an unusual case anyway), but if you really mind we can change it. Kevin
Re: [Qemu-devel] [PATCH 10/10] qcow2: Don't ignore qcow2_alloc_clusters return value
Am 19.01.2010 19:58, schrieb Christoph Hellwig: > On Mon, Jan 18, 2010 at 01:11:36PM +0100, Kevin Wolf wrote: >> @@ -55,6 +55,9 @@ int qcow2_grow_l1_table(BlockDriverState *bs, int min_size) >> >> /* write new table (align to cluster) */ >> new_l1_table_offset = qcow2_alloc_clusters(bs, new_l1_size2); >> +if (new_l1_table_offset < 0) { >> +return new_l1_table_offset; >> +} > > I think the error return needs to free new_l1_table first. Right. Actually, I have found this bug already and included the fix in patch 11/10 which contains the fix to the problem you found with patch 1 (however, I wasn't aware any more that I introduced it myself). Maybe I should better respin the series instead of posting patch 11 on top. Kevin
Re: [Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
On Wed, Jan 20, 2010 at 08:19:26AM +0200, Kirill A. Shutemov wrote: > On Wed, Jan 20, 2010 at 1:56 AM, Juan Quintela wrote: > > From: Kirill A. Shutemov > > > > CC block/vvfat.o > > cc1: warnings being treated as errors > > block/vvfat.c: In function 'commit_one_file': > > block/vvfat.c:2259: error: ignoring return value of 'ftruncate', declared > > with attribute warn_unused_result > > make: *** [block/vvfat.o] Error 1 > > CC block/vvfat.o > > In file included from /usr/include/stdio.h:912, > > from ./qemu-common.h:19, > > from block/vvfat.c:27: > > In function 'snprintf', > > inlined from 'init_directories' at block/vvfat.c:871, > > inlined from 'vvfat_open' at block/vvfat.c:1068: > > /usr/include/bits/stdio2.h:65: error: call to __builtin___snprintf_chk will > > always overflow destination buffer > > make: *** [block/vvfat.o] Error 1 > > > > Signed-off-by: Kirill A. Shutemov > > Signed-off-by: Juan Quintela > > --- > > block/vvfat.c | 9 +++-- > > 1 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/block/vvfat.c b/block/vvfat.c > > index 063f731..df957e5 100644 > > --- a/block/vvfat.c > > +++ b/block/vvfat.c > > @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s, > > { > > direntry_t* entry=array_get_next(&(s->directory)); > > entry->attributes=0x28; /* archive | volume label */ > > - snprintf((char*)entry->name,11,"QEMU VVFAT"); > > + memcpy(entry->name,"QEMU VVF",8); > > + memcpy(entry->extension,"AT ",3); > > } > > Better to use > > memcpy(entry->name, "QEMU VVFAT", 11); > > memcpy() doesn't check bounds. It doesn't *currently* check bounds. If we want to explicitly fill 2 fields at once, then we should redeclare this to have a union with one part comprising the entire buffer, thus avoiding the need for delibrate buffer overruns. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Re: [Qemu-devel] Re: [PATCH v2 2/4] Clean-up a little bit the RW related bits of BDRV_O_FLAGS. BDRV_O_RDONLY gone (and so is BDRV_O_ACCESS). Default value for bdrv_flags (0/zero) is READ-ONLY. Need to
On Wed, Jan 20, 2010 at 08:26:56AM +0100, Markus Armbruster wrote: > Jamie Lokier writes: > > > Michael S. Tsirkin wrote: > >> On Mon, Jan 18, 2010 at 11:34:59AM +0100, Markus Armbruster wrote: > >> > BDRV_O_RDWR is a flag, just like BDRV_SNAPSHOT. We don't have > >> > BDRV_DONT_SNAPSHOT, either. > >> > >> Well, this just mirros the file access macros: we have RDONLY, WRONLY > >> and RDRW. I assume this similarity is just historical? > > > > To avoid confusion, why don't we just call the flag BDRV_O_WRITABLE. > > Then it's obvious what clearing that flag means. > > Sounds good to me. Won't it be confused with WRONLY? -- MST
Re: [Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
On Wed, Jan 20, 2010 at 12:33 PM, Daniel P. Berrange wrote: > On Wed, Jan 20, 2010 at 08:19:26AM +0200, Kirill A. Shutemov wrote: >> On Wed, Jan 20, 2010 at 1:56 AM, Juan Quintela wrote: >> > From: Kirill A. Shutemov >> > >> > CC block/vvfat.o >> > cc1: warnings being treated as errors >> > block/vvfat.c: In function 'commit_one_file': >> > block/vvfat.c:2259: error: ignoring return value of 'ftruncate', declared >> > with attribute warn_unused_result >> > make: *** [block/vvfat.o] Error 1 >> > CC block/vvfat.o >> > In file included from /usr/include/stdio.h:912, >> > from ./qemu-common.h:19, >> > from block/vvfat.c:27: >> > In function 'snprintf', >> > inlined from 'init_directories' at block/vvfat.c:871, >> > inlined from 'vvfat_open' at block/vvfat.c:1068: >> > /usr/include/bits/stdio2.h:65: error: call to __builtin___snprintf_chk >> > will always overflow destination buffer >> > make: *** [block/vvfat.o] Error 1 >> > >> > Signed-off-by: Kirill A. Shutemov >> > Signed-off-by: Juan Quintela >> > --- >> > block/vvfat.c | 9 +++-- >> > 1 files changed, 7 insertions(+), 2 deletions(-) >> > >> > diff --git a/block/vvfat.c b/block/vvfat.c >> > index 063f731..df957e5 100644 >> > --- a/block/vvfat.c >> > +++ b/block/vvfat.c >> > @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s, >> > { >> > direntry_t* entry=array_get_next(&(s->directory)); >> > entry->attributes=0x28; /* archive | volume label */ >> > - snprintf((char*)entry->name,11,"QEMU VVFAT"); >> > + memcpy(entry->name,"QEMU VVF",8); >> > + memcpy(entry->extension,"AT ",3); >> > } >> >> Better to use >> >> memcpy(entry->name, "QEMU VVFAT", 11); >> >> memcpy() doesn't check bounds. > > It doesn't *currently* check bounds. No. memcpy() will never check bounds. It's totaly different from strcpy, http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00419.html > If we want to explicitly > fill 2 fields at once, then we should redeclare this to have a > union with one part comprising the entire buffer, thus avoiding > the need for delibrate buffer overruns. > > Regards, > Daniel > -- > |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| > |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| >
Re: [Qemu-devel] Re: Stop using "which" in ./configure
On Tue, Jan 19, 2010, Måns Rullgård wrote: [...] > Why the extra variable? Using $1 directly seems just as obvious to me. [...] I'm attaching an updated patch which doesn't use this variable. Should be applied after the sdl-config patch. > Is the full path of these tools really important? Doesn't look like > it to me. I'm attaching a new patch which changes the tests a bit; I would prefer if someone with access to a Solaris build environment would do this though. -- Loïc Minier >From 27f151ef7be19fcffbbf9c2d3c6ee50750be854d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Minier?= Date: Tue, 19 Jan 2010 11:05:00 +0100 Subject: [PATCH 1/2] Add and use has() and path_of() funcs Add has() and path_of() funcs and use them across configure; has() will test whether a command or builtin is available; path_of() will search the PATH for executables and return the full pathname if found. --- configure | 52 +++- 1 files changed, 43 insertions(+), 9 deletions(-) diff --git a/configure b/configure index baa2800..90b3c18 100755 --- a/configure +++ b/configure @@ -27,6 +27,42 @@ compile_prog() { $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null } +# check whether a command is available to this shell (may be either an +# executable or a builtin) +has() { +type "$1" >/dev/null 2>&1 +} + +# search for an executable in PATH +path_of() { +local_command="$1" +local_ifs="$IFS" +local_dir="" + +# pathname has a dir component? +if [ "${local_command#*/}" != "$local_command" ]; then +if [ -x "$local_command" ] && [ ! -d "$local_command" ]; then +echo "$local_command" +return 0 +fi +fi +if [ -z "$local_command" ]; then +return 1 +fi + +IFS=: +for local_dir in $PATH; do +if [ -x "$local_dir/$local_command" ] && [ ! -d "$local_dir/$local_command" ]; then +echo "$local_dir/$local_command" +IFS="$local_ifs" +return 0 +fi +done +# not found +IFS="$local_ifs" +return 1 +} + # default parameters cpu="" prefix="" @@ -763,7 +799,7 @@ fi # Solaris specific configure tool chain decisions # if test "$solaris" = "yes" ; then - solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"` + solinst=`path_of $install` if test -z "$solinst" ; then echo "Solaris install program not found. Use --install=/usr/ucb/install or" echo "install fileutils from www.blastwave.org using pkg-get -i fileutils" @@ -776,7 +812,7 @@ if test "$solaris" = "yes" ; then echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install" exit 1 fi - sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"` + sol_ar=`path_of ar` if test -z "$sol_ar" ; then echo "Error: No path includes ar" if test -f /usr/ccs/bin/ar ; then @@ -969,7 +1005,7 @@ fi # pkgconfig probe pkgconfig="${cross_prefix}pkg-config" -if ! test -x "$(which $pkgconfig 2>/dev/null)"; then +if ! has $pkgconfig; then # likely not cross compiling, or hope for the best pkgconfig=pkg-config fi @@ -977,7 +1013,7 @@ fi ## # Sparse probe if test "$sparse" != "no" ; then - if test -x "$(which cgcc 2>/dev/null)"; then + if has cgcc; then sparse=yes else if test "$sparse" = "yes" ; then @@ -993,7 +1029,7 @@ fi if $pkgconfig sdl --modversion >/dev/null 2>&1; then sdlconfig="$pkgconfig sdl" _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'` -elif which sdl-config >/dev/null 2>&1; then +elif has sdl-config; then sdlconfig='sdl-config' _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'` else @@ -1424,8 +1460,7 @@ EOF fi else if test "$kvm" = "yes" ; then - if [ -x "`which awk 2>/dev/null`" ] && \ - [ -x "`which grep 2>/dev/null`" ]; then + if has awk && has grep; then kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \ | grep "error: " \ | awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'` @@ -1694,8 +1729,7 @@ fi # Check if tools are available to build documentation. if test "$docs" != "no" ; then - if test -x "`which texi2html 2>/dev/null`" -a \ - -x "`which pod2man 2>/dev/null`" ; then + if has texi2html && has pod2man; then docs=yes else if test "$docs" = "yes" ; then -- 1.6.5 >From 421c7a48b5e0a20ae09014cd67e94f3c0ba26a20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Minier?= Date: Wed, 20 Jan 2010 12:35:54 +0100 Subject: [PATCH 2/2] Solaris: test for presence of commands with has() --- configure |8 +++- 1 files changed, 3 insertions(+), 5 deletions(-) diff --git a/configure b/configure index 90b3c18..7e842ce 100755 --- a/configure +++ b/configure @@ -799,21 +799,19 @@ fi # Solaris specific configure tool chain decisions # if test "$sol
Re: [Qemu-devel] [PATCH] block: prevent multiwrite_merge from creating too large iovecs
Am 19.01.2010 22:15, schrieb Christoph Hellwig: > If we go over the maximum number of iovecs support by syscall we get > back EINVAL from the kernel which translate to I/O errors for the guest. > > Signed-off-by: Christoph Hellwig Is this really enough? We don't check for IOV_MAX in any other place, so can't we get a too big request directly from virtio-blk? What about handling it transparently in qemu_pwritev/preadv and laio_submit? Logically, it's a limitation of the backend anyway and not a generic restriction. To underline that it's a backend/platform dependent thing: Your patch breaks the mingw build for me. Kevin
Re: [Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
Am 20.01.2010 12:09, schrieb Kirill A. Shutemov: > On Wed, Jan 20, 2010 at 12:33 PM, Daniel P. Berrange > wrote: >> On Wed, Jan 20, 2010 at 08:19:26AM +0200, Kirill A. Shutemov wrote: >>> On Wed, Jan 20, 2010 at 1:56 AM, Juan Quintela wrote: From: Kirill A. Shutemov CCblock/vvfat.o cc1: warnings being treated as errors block/vvfat.c: In function 'commit_one_file': block/vvfat.c:2259: error: ignoring return value of 'ftruncate', declared with attribute warn_unused_result make: *** [block/vvfat.o] Error 1 CCblock/vvfat.o In file included from /usr/include/stdio.h:912, from ./qemu-common.h:19, from block/vvfat.c:27: In function 'snprintf', inlined from 'init_directories' at block/vvfat.c:871, inlined from 'vvfat_open' at block/vvfat.c:1068: /usr/include/bits/stdio2.h:65: error: call to __builtin___snprintf_chk will always overflow destination buffer make: *** [block/vvfat.o] Error 1 Signed-off-by: Kirill A. Shutemov Signed-off-by: Juan Quintela --- block/vvfat.c |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index 063f731..df957e5 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s, { direntry_t* entry=array_get_next(&(s->directory)); entry->attributes=0x28; /* archive | volume label */ - snprintf((char*)entry->name,11,"QEMU VVFAT"); + memcpy(entry->name,"QEMU VVF",8); + memcpy(entry->extension,"AT ",3); } >>> >>> Better to use >>> >>> memcpy(entry->name, "QEMU VVFAT", 11); >>> >>> memcpy() doesn't check bounds. >> >> It doesn't *currently* check bounds. > > No. memcpy() will never check bounds. It's totaly different from strcpy, > http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00419.html Regardless if deliberately overflowing the buffer works or doesn't making it explicit is better. Someone might reorder the struct or add new fields in between (okay, unlikely in this case, but still) and you'll overflow into fields you never wanted to touch. Kevin
[Qemu-devel] [PATCH v2 3/6] monitor: convert do_memory_save() to QError
Signed-off-by: Markus Armbruster --- monitor.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/monitor.c b/monitor.c index c22901f..e63d0a7 100644 --- a/monitor.c +++ b/monitor.c @@ -1317,7 +1317,7 @@ static void do_memory_save(Monitor *mon, const QDict *qdict, QObject **ret_data) f = fopen(filename, "wb"); if (!f) { -monitor_printf(mon, "could not open '%s'\n", filename); +qemu_error_new(QERR_OPEN_FILE_FAILED, filename); return; } while (size != 0) { -- 1.6.6
[Qemu-devel] [PATCH v2 1/6] monitor: Don't check for mon_get_cpu() failure
mon_get_cpu() can't return null pointer, because it passes its return value to cpu_synchronize_state() first, which crashes if its argument is null. Remove the (pretty cheesy) handling of this non-existing error. Signed-off-by: Markus Armbruster --- monitor.c | 39 +++ 1 files changed, 3 insertions(+), 36 deletions(-) diff --git a/monitor.c b/monitor.c index 938eb3b..c22901f 100644 --- a/monitor.c +++ b/monitor.c @@ -693,8 +693,6 @@ static void do_info_registers(Monitor *mon) { CPUState *env; env = mon_get_cpu(); -if (!env) -return; #ifdef TARGET_I386 cpu_dump_state(env, (FILE *)mon, monitor_fprintf, X86_DUMP_FPU); @@ -1128,7 +1126,7 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, int flags; flags = 0; env = mon_get_cpu(); -if (!env && !is_physical) +if (!is_physical) return; #ifdef TARGET_I386 if (wsize == 2) { @@ -1190,8 +1188,6 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize, cpu_physical_memory_rw(addr, buf, l, 0); } else { env = mon_get_cpu(); -if (!env) -break; if (cpu_memory_rw_debug(env, addr, buf, l, 0) < 0) { monitor_printf(mon, " Cannot access memory\n"); break; @@ -1318,8 +1314,6 @@ static void do_memory_save(Monitor *mon, const QDict *qdict, QObject **ret_data) uint8_t buf[1024]; env = mon_get_cpu(); -if (!env) -return; f = fopen(filename, "wb"); if (!f) { @@ -1754,8 +1748,6 @@ static void tlb_info(Monitor *mon) uint32_t pgd, pde, pte; env = mon_get_cpu(); -if (!env) -return; if (!(env->cr[0] & CR0_PG_MASK)) { monitor_printf(mon, "PG disabled\n"); @@ -1812,8 +1804,6 @@ static void mem_info(Monitor *mon) uint32_t pgd, pde, pte, start, end; env = mon_get_cpu(); -if (!env) -return; if (!(env->cr[0] & CR0_PG_MASK)) { monitor_printf(mon, "PG disabled\n"); @@ -2659,8 +2649,6 @@ typedef struct MonitorDef { static target_long monitor_get_pc (const struct MonitorDef *md, int val) { CPUState *env = mon_get_cpu(); -if (!env) -return 0; return env->eip + env->segs[R_CS].base; } #endif @@ -2672,9 +2660,6 @@ static target_long monitor_get_ccr (const struct MonitorDef *md, int val) unsigned int u; int i; -if (!env) -return 0; - u = 0; for (i = 0; i < 8; i++) u |= env->crf[i] << (32 - (4 * i)); @@ -2685,40 +2670,30 @@ static target_long monitor_get_ccr (const struct MonitorDef *md, int val) static target_long monitor_get_msr (const struct MonitorDef *md, int val) { CPUState *env = mon_get_cpu(); -if (!env) -return 0; return env->msr; } static target_long monitor_get_xer (const struct MonitorDef *md, int val) { CPUState *env = mon_get_cpu(); -if (!env) -return 0; return env->xer; } static target_long monitor_get_decr (const struct MonitorDef *md, int val) { CPUState *env = mon_get_cpu(); -if (!env) -return 0; return cpu_ppc_load_decr(env); } static target_long monitor_get_tbu (const struct MonitorDef *md, int val) { CPUState *env = mon_get_cpu(); -if (!env) -return 0; return cpu_ppc_load_tbu(env); } static target_long monitor_get_tbl (const struct MonitorDef *md, int val) { CPUState *env = mon_get_cpu(); -if (!env) -return 0; return cpu_ppc_load_tbl(env); } #endif @@ -2728,8 +2703,6 @@ static target_long monitor_get_tbl (const struct MonitorDef *md, int val) static target_long monitor_get_psr (const struct MonitorDef *md, int val) { CPUState *env = mon_get_cpu(); -if (!env) -return 0; return GET_PSR(env); } #endif @@ -2737,8 +2710,6 @@ static target_long monitor_get_psr (const struct MonitorDef *md, int val) static target_long monitor_get_reg(const struct MonitorDef *md, int val) { CPUState *env = mon_get_cpu(); -if (!env) -return 0; return env->regwptr[val]; } #endif @@ -2990,7 +2961,7 @@ static void expr_error(Monitor *mon, const char *msg) longjmp(expr_env, 1); } -/* return 0 if OK, -1 if not found, -2 if no CPU defined */ +/* return 0 if OK, -1 if not found */ static int get_monitor_def(target_long *pval, const char *name) { const MonitorDef *md; @@ -3002,8 +2973,6 @@ static int get_monitor_def(target_long *pval, const char *name) *pval = md->get_value(md, md->offset); } else { CPUState *env = mon_get_cpu(); -if (!env) -return -2; ptr = (uint8_t *)env + md->offset; switch(md->type) { case MD_I32: @@ -3090,10 +3059,8 @@ static int64_t expr_unary(Monitor *mon)
[Qemu-devel] [PATCH v2 6/6] monitor: convert do_cpu_set() to QObject, QError
Signed-off-by: Markus Armbruster --- monitor.c |4 ++-- qemu-monitor.hx |3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/monitor.c b/monitor.c index 816f6fd..b9166c3 100644 --- a/monitor.c +++ b/monitor.c @@ -808,11 +808,11 @@ static void do_info_cpus(Monitor *mon, QObject **ret_data) *ret_data = QOBJECT(cpu_list); } -static void do_cpu_set(Monitor *mon, const QDict *qdict) +static void do_cpu_set(Monitor *mon, const QDict *qdict, QObject **ret_data) { int index = qdict_get_int(qdict, "index"); if (mon_set_cpu(index) < 0) -monitor_printf(mon, "Invalid CPU index\n"); +qemu_error_new(QERR_INVALID_CPU_INDEX); } static void do_info_jit(Monitor *mon) diff --git a/qemu-monitor.hx b/qemu-monitor.hx index 1aa7818..415734a 100644 --- a/qemu-monitor.hx +++ b/qemu-monitor.hx @@ -573,7 +573,8 @@ ETEXI .args_type = "index:i", .params = "index", .help = "set the default CPU", -.mhandler.cmd = do_cpu_set, +.user_print = monitor_user_noop, +.mhandler.cmd_new = do_cpu_set, }, STEXI -- 1.6.6
[Qemu-devel] [PATCH v2 5/6] QError: New QERR_INVALID_CPU_INDEX
Signed-off-by: Markus Armbruster --- qerror.c |4 qerror.h |3 +++ 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/qerror.c b/qerror.c index 2f657f4..6c2aba0 100644 --- a/qerror.c +++ b/qerror.c @@ -81,6 +81,10 @@ static const QErrorStringTable qerror_table[] = { .desc = "Invalid block format %(name)", }, { +.error_fmt = QERR_INVALID_CPU_INDEX, +.desc = "Invalid CPU index", +}, +{ .error_fmt = QERR_INVALID_PARAMETER, .desc = "Invalid parameter %(name)", }, diff --git a/qerror.h b/qerror.h index ee59615..57c5b97 100644 --- a/qerror.h +++ b/qerror.h @@ -70,6 +70,9 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_INVALID_BLOCK_FORMAT \ "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }" +#define QERR_INVALID_CPU_INDEX \ +"{ 'class': 'InvalidCPUIndex', 'data': {} }" + #define QERR_INVALID_PARAMETER \ "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }" -- 1.6.6
[Qemu-devel] [PATCH v2 0/6] Convert memsave, pmemsave, cpu to QObject+QError
First patch is cleanup to get rid of an error that can't happen. Rest are straightforward conversions. v2: Renames suggested by Luiz, typo fixed, rebased Markus Armbruster (6): monitor: Don't check for mon_get_cpu() failure QError: New QERR_OPEN_FILE_FAILED monitor: convert do_memory_save() to QError monitor: convert do_physical_memory_save() to QError QError: New QERR_INVALID_CPU_INDEX monitor: convert do_cpu_set() to QObject, QError monitor.c | 47 +++ qemu-monitor.hx |3 ++- qerror.c|8 qerror.h|6 ++ 4 files changed, 23 insertions(+), 41 deletions(-)
[Qemu-devel] [PATCH v2 4/6] monitor: convert do_physical_memory_save() to QError
Signed-off-by: Markus Armbruster --- monitor.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/monitor.c b/monitor.c index e63d0a7..816f6fd 100644 --- a/monitor.c +++ b/monitor.c @@ -1344,7 +1344,7 @@ static void do_physical_memory_save(Monitor *mon, const QDict *qdict, f = fopen(filename, "wb"); if (!f) { -monitor_printf(mon, "could not open '%s'\n", filename); +qemu_error_new(QERR_OPEN_FILE_FAILED, filename); return; } while (size != 0) { -- 1.6.6
[Qemu-devel] [PATCH v2 2/6] QError: New QERR_OPEN_FILE_FAILED
Signed-off-by: Markus Armbruster --- qerror.c |4 qerror.h |3 +++ 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/qerror.c b/qerror.c index 5f8fc5d..2f657f4 100644 --- a/qerror.c +++ b/qerror.c @@ -73,6 +73,10 @@ static const QErrorStringTable qerror_table[] = { .desc = "No file descriptor supplied via SCM_RIGHTS", }, { +.error_fmt = QERR_OPEN_FILE_FAILED, +.desc = "Could not open '%(filename)'", +}, +{ .error_fmt = QERR_INVALID_BLOCK_FORMAT, .desc = "Invalid block format %(name)", }, diff --git a/qerror.h b/qerror.h index 9e220d6..ee59615 100644 --- a/qerror.h +++ b/qerror.h @@ -64,6 +64,9 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_FD_NOT_SUPPLIED \ "{ 'class': 'FdNotSupplied', 'data': {} }" +#define QERR_OPEN_FILE_FAILED \ +"{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }" + #define QERR_INVALID_BLOCK_FORMAT \ "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }" -- 1.6.6
Re: [Qemu-devel] Re: [PATCH v2 2/4] Clean-up a little bit the RW related bits of BDRV_O_FLAGS. BDRV_O_RDONLY gone (and so is BDRV_O_ACCESS). Default value for bdrv_flags (0/zero) is READ-ONLY. Need to
"Michael S. Tsirkin" writes: > On Wed, Jan 20, 2010 at 08:26:56AM +0100, Markus Armbruster wrote: >> Jamie Lokier writes: >> >> > Michael S. Tsirkin wrote: >> >> On Mon, Jan 18, 2010 at 11:34:59AM +0100, Markus Armbruster wrote: >> >> > BDRV_O_RDWR is a flag, just like BDRV_SNAPSHOT. We don't have >> >> > BDRV_DONT_SNAPSHOT, either. >> >> >> >> Well, this just mirros the file access macros: we have RDONLY, WRONLY >> >> and RDRW. I assume this similarity is just historical? >> > >> > To avoid confusion, why don't we just call the flag BDRV_O_WRITABLE. >> > Then it's obvious what clearing that flag means. >> >> Sounds good to me. > > Won't it be confused with WRONLY? I doubt it. "Writable" implies "write-only" no more than "readable" implies "read-only".
Re: [Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
Kevin Wolf writes: > Am 20.01.2010 12:09, schrieb Kirill A. Shutemov: >> On Wed, Jan 20, 2010 at 12:33 PM, Daniel P. Berrange >> wrote: >>> On Wed, Jan 20, 2010 at 08:19:26AM +0200, Kirill A. Shutemov wrote: On Wed, Jan 20, 2010 at 1:56 AM, Juan Quintela wrote: [...] > diff --git a/block/vvfat.c b/block/vvfat.c > index 063f731..df957e5 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s, > { >direntry_t* entry=array_get_next(&(s->directory)); >entry->attributes=0x28; /* archive | volume label */ > - snprintf((char*)entry->name,11,"QEMU VVFAT"); > + memcpy(entry->name,"QEMU VVF",8); > + memcpy(entry->extension,"AT ",3); > } Better to use memcpy(entry->name, "QEMU VVFAT", 11); memcpy() doesn't check bounds. No, this is evil, and may well be flagged by static analysis tools. >>> It doesn't *currently* check bounds. >> >> No. memcpy() will never check bounds. It's totaly different from strcpy, >> http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00419.html > > Regardless if deliberately overflowing the buffer works or doesn't > making it explicit is better. Someone might reorder the struct or add > new fields in between (okay, unlikely in this case, but still) and > you'll overflow into fields you never wanted to touch. Moreover, compilers are free to put padding between members name and extension.
[Qemu-devel] Re: Stop using "which" in ./configure
On 01/20/2010 12:37 PM, Loïc Minier wrote: +# not found +IFS="$local_ifs" If you do this, you should set IFS to space-tab-lf at the beginning of the script, like this: IFS=" "" "" " or this (better because it does not rely on embedding whitespace characters within the line): IFS=`printf ' \t'`" " Paolo
Re: [Qemu-devel] Re: [PATCH v2 2/4] Clean-up a little bit the RW related bits of BDRV_O_FLAGS. BDRV_O_RDONLY gone (and so is BDRV_O_ACCESS). Default value for bdrv_flags (0/zero) is READ-ONLY. Need to
On Wed, Jan 20, 2010 at 01:09:29PM +0100, Markus Armbruster wrote: > "Michael S. Tsirkin" writes: > > > On Wed, Jan 20, 2010 at 08:26:56AM +0100, Markus Armbruster wrote: > >> Jamie Lokier writes: > >> > >> > Michael S. Tsirkin wrote: > >> >> On Mon, Jan 18, 2010 at 11:34:59AM +0100, Markus Armbruster wrote: > >> >> > BDRV_O_RDWR is a flag, just like BDRV_SNAPSHOT. We don't have > >> >> > BDRV_DONT_SNAPSHOT, either. > >> >> > >> >> Well, this just mirros the file access macros: we have RDONLY, WRONLY > >> >> and RDRW. I assume this similarity is just historical? > >> > > >> > To avoid confusion, why don't we just call the flag BDRV_O_WRITABLE. > >> > Then it's obvious what clearing that flag means. > >> > >> Sounds good to me. > > > > Won't it be confused with WRONLY? > > I doubt it. "Writable" implies "write-only" no more than "readable" > implies "read-only". Yes :) But what I am saying is that the disk is readable as well. -- MST
Re: [Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
On Wed, Jan 20, 2010 at 2:15 PM, Markus Armbruster wrote: > Kevin Wolf writes: > >> Am 20.01.2010 12:09, schrieb Kirill A. Shutemov: >>> On Wed, Jan 20, 2010 at 12:33 PM, Daniel P. Berrange >>> wrote: On Wed, Jan 20, 2010 at 08:19:26AM +0200, Kirill A. Shutemov wrote: > On Wed, Jan 20, 2010 at 1:56 AM, Juan Quintela > wrote: > [...] >> diff --git a/block/vvfat.c b/block/vvfat.c >> index 063f731..df957e5 100644 >> --- a/block/vvfat.c >> +++ b/block/vvfat.c >> @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s, >> { >> direntry_t* entry=array_get_next(&(s->directory)); >> entry->attributes=0x28; /* archive | volume label */ >> - snprintf((char*)entry->name,11,"QEMU VVFAT"); >> + memcpy(entry->name,"QEMU VVF",8); >> + memcpy(entry->extension,"AT ",3); >> } > > Better to use > > memcpy(entry->name, "QEMU VVFAT", 11); > > memcpy() doesn't check bounds. > > No, this is evil, and may well be flagged by static analysis tools. If so, the tool is stupid. It doesn't *currently* check bounds. >>> >>> No. memcpy() will never check bounds. It's totaly different from strcpy, >>> http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00419.html >> >> Regardless if deliberately overflowing the buffer works or doesn't >> making it explicit is better. Someone might reorder the struct or add >> new fields in between (okay, unlikely in this case, but still) and >> you'll overflow into fields you never wanted to touch. > > Moreover, compilers are free to put padding between members name and > extension. No, compiler can't add anything between. 'char' is always byte-aligned.
[Qemu-devel] [PATCH] QMP: Fix asynchronous events delivery
Commit f039a563f200beee80cc10fd70b21ea396979dab introduces a regression as monitor_protocol_event() will return in the first user Monitor it finds in the QLIST_FOREACH() loop. The right thing to do is to only delivery an asynchronous event if the 'mon' is a QMP Monitor. The aforementioned commit was an early version, if it was applied to stable (it should) this one has to be applied there too. Signed-off-by: Luiz Capitulino --- monitor.c |7 +++ 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/monitor.c b/monitor.c index 938eb3b..b2b88c1 100644 --- a/monitor.c +++ b/monitor.c @@ -377,10 +377,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) } QLIST_FOREACH(mon, &mon_list, entry) { -if (!monitor_ctrl_mode(mon)) -return; - -monitor_json_emitter(mon, QOBJECT(qmp)); +if (monitor_ctrl_mode(mon)) { +monitor_json_emitter(mon, QOBJECT(qmp)); +} } QDECREF(qmp); } -- 1.6.6
[Qemu-devel] Re: Stop using "which" in ./configure
Loïc Minier wrote: > On Tue, Jan 19, 2010, Måns Rullgård wrote: > [...] >> Why the extra variable? Using $1 directly seems just as obvious to me. > [...] > > I'm attaching an updated patch which doesn't use this variable. Should > be applied after the sdl-config patch. > >> Is the full path of these tools really important? Doesn't look like >> it to me. > > I'm attaching a new patch which changes the tests a bit; I would prefer > if someone with access to a Solaris build environment would do this > though. Hi could you test this patch? I did it concurrently with yours. my prog_exist() is equal to your has(), and path_of() is only needed in a single place, that I think it is better handled with a single grep in that place. What do you think? >From 4580f69b022e7861bf3be1c694222c0ee82889b7 Mon Sep 17 00:00:00 2001 Message-Id: <4580f69b022e7861bf3be1c694222c0ee82889b7.1263991676.git.quint...@redhat.com> In-Reply-To: References: From: Juan Quintela Date: Wed, 20 Jan 2010 13:24:59 +0100 Subject: [PATCH 1/3] substitute all uses of which by type Except in one case, we are only interested in knowing if a command exist, not is path. Just create prog_exists() function that does this check. Signed-off-by: Juan Quintela --- configure | 25 ++--- 1 files changed, 14 insertions(+), 11 deletions(-) diff --git a/configure b/configure index 5631bbb..c1c9bb4 100755 --- a/configure +++ b/configure @@ -27,6 +27,11 @@ compile_prog() { $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > /dev/null 2> /dev/null } +prog_exist() { +prog="$1" +type $1 > /dev/null 2> /dev/null +} + # default parameters cpu="" prefix="" @@ -763,21 +768,19 @@ fi # Solaris specific configure tool chain decisions # if test "$solaris" = "yes" ; then - solinst=`which $install 2> /dev/null | /usr/bin/grep -v "no $install in"` - if test -z "$solinst" ; then + if ! prog_exist "$install" ; then echo "Solaris install program not found. Use --install=/usr/ucb/install or" echo "install fileutils from www.blastwave.org using pkg-get -i fileutils" echo "to get ginstall which is used by default (which lives in /opt/csw/bin)" exit 1 fi - if test "$solinst" = "/usr/sbin/install" ; then + if type install2 2> /dev/null | grep /usr/bin/install >/dev/null ; then echo "Error: Solaris /usr/sbin/install is not an appropriate install program." echo "try ginstall from the GNU fileutils available from www.blastwave.org" echo "using pkg-get -i fileutils, or use --install=/usr/ucb/install" exit 1 fi - sol_ar=`which ar 2> /dev/null | /usr/bin/grep -v "no ar in"` - if test -z "$sol_ar" ; then + if ! prog_exist "ar" ; then echo "Error: No path includes ar" if test -f /usr/ccs/bin/ar ; then echo "Add /usr/ccs/bin to your path and rerun configure" @@ -969,7 +972,7 @@ fi # pkgconfig probe pkgconfig="${cross_prefix}pkg-config" -if ! test -x "$(which $pkgconfig 2>/dev/null)"; then +if ! prog_exist "$pkgconfig" ; then # likely not cross compiling, or hope for the best pkgconfig=pkg-config fi @@ -977,7 +980,7 @@ fi ## # Sparse probe if test "$sparse" != "no" ; then - if test -x "$(which cgcc 2>/dev/null)"; then + if prog_exist "cgcc" ; then sparse=yes else if test "$sparse" = "yes" ; then @@ -1419,8 +1422,8 @@ EOF fi else if test "$kvm" = "yes" ; then - if [ -x "`which awk 2>/dev/null`" ] && \ - [ -x "`which grep 2>/dev/null`" ]; then + if prog_exist "awk" -a \ + prog_exist "grep"; then kvmerr=`LANG=C $cc $QEMU_CFLAGS -o $TMPE $kvm_cflags $TMPC 2>&1 \ | grep "error: " \ | awk -F "error: " '{if (NR>1) printf(", "); printf("%s",$2);}'` @@ -1689,8 +1692,8 @@ fi # Check if tools are available to build documentation. if test "$docs" != "no" ; then - if test -x "`which texi2html 2>/dev/null`" -a \ - -x "`which pod2man 2>/dev/null`" ; then + if prog_exist "texi2html" -a \ + prog_exist "pod2man" ; then docs=yes else if test "$docs" = "yes" ; then -- 1.6.6
Re: [Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
"Kirill A. Shutemov" writes: > On Wed, Jan 20, 2010 at 2:15 PM, Markus Armbruster wrote: >> Kevin Wolf writes: >> >>> Am 20.01.2010 12:09, schrieb Kirill A. Shutemov: On Wed, Jan 20, 2010 at 12:33 PM, Daniel P. Berrange wrote: > On Wed, Jan 20, 2010 at 08:19:26AM +0200, Kirill A. Shutemov wrote: >> On Wed, Jan 20, 2010 at 1:56 AM, Juan Quintela >> wrote: >> [...] >>> diff --git a/block/vvfat.c b/block/vvfat.c >>> index 063f731..df957e5 100644 >>> --- a/block/vvfat.c >>> +++ b/block/vvfat.c >>> @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s, >>> { >>> direntry_t* entry=array_get_next(&(s->directory)); >>> entry->attributes=0x28; /* archive | volume label */ >>> - snprintf((char*)entry->name,11,"QEMU VVFAT"); >>> + memcpy(entry->name,"QEMU VVF",8); >>> + memcpy(entry->extension,"AT ",3); >>> } >> >> Better to use >> >> memcpy(entry->name, "QEMU VVFAT", 11); >> >> memcpy() doesn't check bounds. >> >> No, this is evil, and may well be flagged by static analysis tools. > > If so, the tool is stupid. > > It doesn't *currently* check bounds. No. memcpy() will never check bounds. It's totaly different from strcpy, http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00419.html >>> >>> Regardless if deliberately overflowing the buffer works or doesn't >>> making it explicit is better. Someone might reorder the struct or add >>> new fields in between (okay, unlikely in this case, but still) and >>> you'll overflow into fields you never wanted to touch. >> >> Moreover, compilers are free to put padding between members name and >> extension. > > No, compiler can't add anything between. 'char' is always byte-aligned. You got some reading to do then.
Re: [Qemu-devel] Re: [PATCH v2 2/4] Clean-up a little bit the RW related bits of BDRV_O_FLAGS. BDRV_O_RDONLY gone (and so is BDRV_O_ACCESS). Default value for bdrv_flags (0/zero) is READ-ONLY. Need to
"Michael S. Tsirkin" writes: > On Wed, Jan 20, 2010 at 01:09:29PM +0100, Markus Armbruster wrote: >> "Michael S. Tsirkin" writes: >> >> > On Wed, Jan 20, 2010 at 08:26:56AM +0100, Markus Armbruster wrote: >> >> Jamie Lokier writes: >> >> >> >> > Michael S. Tsirkin wrote: >> >> >> On Mon, Jan 18, 2010 at 11:34:59AM +0100, Markus Armbruster wrote: >> >> >> > BDRV_O_RDWR is a flag, just like BDRV_SNAPSHOT. We don't have >> >> >> > BDRV_DONT_SNAPSHOT, either. >> >> >> >> >> >> Well, this just mirros the file access macros: we have RDONLY, WRONLY >> >> >> and RDRW. I assume this similarity is just historical? >> >> > >> >> > To avoid confusion, why don't we just call the flag BDRV_O_WRITABLE. >> >> > Then it's obvious what clearing that flag means. >> >> >> >> Sounds good to me. >> > >> > Won't it be confused with WRONLY? >> >> I doubt it. "Writable" implies "write-only" no more than "readable" >> implies "read-only". > > Yes :) But what I am saying is that the disk is readable as well. "Writable" doesn't imply "not readable" either :)
Re: [Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
On Wed, Jan 20, 2010 at 02:03:04PM +0100, Markus Armbruster wrote: > "Kirill A. Shutemov" writes: > > > On Wed, Jan 20, 2010 at 2:15 PM, Markus Armbruster > > wrote: > >> Kevin Wolf writes: > >> > >>> Am 20.01.2010 12:09, schrieb Kirill A. Shutemov: > On Wed, Jan 20, 2010 at 12:33 PM, Daniel P. Berrange > wrote: > > On Wed, Jan 20, 2010 at 08:19:26AM +0200, Kirill A. Shutemov wrote: > >> On Wed, Jan 20, 2010 at 1:56 AM, Juan Quintela > >> wrote: > >> [...] > >>> diff --git a/block/vvfat.c b/block/vvfat.c > >>> index 063f731..df957e5 100644 > >>> --- a/block/vvfat.c > >>> +++ b/block/vvfat.c > >>> @@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s, > >>> { > >>> direntry_t* entry=array_get_next(&(s->directory)); > >>> entry->attributes=0x28; /* archive | volume label */ > >>> - snprintf((char*)entry->name,11,"QEMU VVFAT"); > >>> + memcpy(entry->name,"QEMU VVF",8); > >>> + memcpy(entry->extension,"AT ",3); > >>> } > >> > >> Better to use > >> > >> memcpy(entry->name, "QEMU VVFAT", 11); > >> > >> memcpy() doesn't check bounds. > >> > >> No, this is evil, and may well be flagged by static analysis tools. > > > > If so, the tool is stupid. > > > > It doesn't *currently* check bounds. > > No. memcpy() will never check bounds. It's totaly different from strcpy, > http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00419.html > >>> > >>> Regardless if deliberately overflowing the buffer works or doesn't > >>> making it explicit is better. Someone might reorder the struct or add > >>> new fields in between (okay, unlikely in this case, but still) and > >>> you'll overflow into fields you never wanted to touch. > >> > >> Moreover, compilers are free to put padding between members name and > >> extension. > > > > No, compiler can't add anything between. 'char' is always byte-aligned. > > You got some reading to do then. > To be fair this structure is packed, but this is not the reason to write stupid code of course. -- Gleb.
[Qemu-devel] Re: Stop using "which" in ./configure
On 01/20/2010 01:49 PM, Juan Quintela wrote: + if prog_exist "awk" -a \ + prog_exist "grep"; then + if prog_exist "texi2html" -a \ + prog_exist "pod2man" ; then -a is wrong here. BTW, it's evil for test too because its precedence rules WRT ! and -f/-n/-z/-x/etc. are broken. Paolo
Re: [Qemu-devel] [PATCH v2 0/6] Convert memsave, pmemsave, cpu to QObject+QError
On Wed, 20 Jan 2010 13:07:29 +0100 Markus Armbruster wrote: > First patch is cleanup to get rid of an error that can't happen. Rest > are straightforward conversions. Looks good to me.
Re: [Qemu-devel] Re: [PATCH v2 2/4] Clean-up a little bit the RW related bits of BDRV_O_FLAGS. BDRV_O_RDONLY gone (and so is BDRV_O_ACCESS). Default value for bdrv_flags (0/zero) is READ-ONLY. Need to
Michael S. Tsirkin wrote: > On Wed, Jan 20, 2010 at 08:26:56AM +0100, Markus Armbruster wrote: > > Jamie Lokier writes: > > > > > Michael S. Tsirkin wrote: > > >> On Mon, Jan 18, 2010 at 11:34:59AM +0100, Markus Armbruster wrote: > > >> > BDRV_O_RDWR is a flag, just like BDRV_SNAPSHOT. We don't have > > >> > BDRV_DONT_SNAPSHOT, either. > > >> > > >> Well, this just mirros the file access macros: we have RDONLY, WRONLY > > >> and RDRW. I assume this similarity is just historical? > > > > > > To avoid confusion, why don't we just call the flag BDRV_O_WRITABLE. > > > Then it's obvious what clearing that flag means. > > > > Sounds good to me. > > Won't it be confused with WRONLY? No, because nobody sane would expect qemu's blockdevs to need WRONLY :-) -- Jamie
[Qemu-devel] [PATCH][rtc hack]: reduce number of reinjects on ACK
Windows 7 BSODs under load with HAL_RTC_IRQF_WILL_NOT_CLEAR error. It happens here: hal!HalpRtcUnmaskClock: 8281b93a 8bffmov edi,edi 8281b93c 56 pushesi 8281b93d 33f6xor esi,esi 8281b93f 6a0cpush0Ch 8281b941 e8b2ff callhal!CMOS_READ (8281b8f8) 8281b946 84c0testal,al 8281b948 7920jns hal!HalpRtcUnmaskClock+0x30 (8281b96a) 8281b94a 6a0apush0Ah 8281b94c 46 inc esi 8281b94d e854c8 callhal!KeStallExecutionProcessor (828181a6) 8281b952 83fe64 cmp esi,64h 8281b955 72e8jb hal!HalpRtcUnmaskClock+0x5 (8281b93f) 8281b957 6a00push0 8281b959 6a00push0 8281b95b 6a00push0 8281b95d 680a01 push10Ah 8281b962 6a5cpush5Ch 8281b964 ff1500c38082calldword ptr [hal!_imp__KeBugCheckEx (8280c300)] 8281b96a 5e pop esi 8281b96b c3 ret So it loops for 100(64h) times reading register C before BSOD. Lets reduce number of immediate reinjection well under this limit. Signed-off-by: Gleb Natapov diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c index e4d55c7..2616d0d 100644 --- a/hw/mc146818rtc.c +++ b/hw/mc146818rtc.c @@ -30,7 +30,7 @@ //#define DEBUG_CMOS -#define RTC_REINJECT_ON_ACK_COUNT 1000 +#define RTC_REINJECT_ON_ACK_COUNT 20 #define RTC_SECONDS 0 #define RTC_SECONDS_ALARM 1 -- Gleb.
[Qemu-devel] [PATCH 1/3] net: Make inet_strfamily() public
So that it can be used by other subsystems. Signed-off-by: Luiz Capitulino --- qemu-sockets.c |2 +- qemu_socket.h |1 + 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index 8850516..720de22 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -91,7 +91,7 @@ static void inet_setport(struct addrinfo *e, int port) } } -static const char *inet_strfamily(int family) +const char *inet_strfamily(int family) { switch (family) { case PF_INET6: return "ipv6"; diff --git a/qemu_socket.h b/qemu_socket.h index 86bdbf5..7ee46ac 100644 --- a/qemu_socket.h +++ b/qemu_socket.h @@ -44,6 +44,7 @@ int inet_listen(const char *str, char *ostr, int olen, int inet_connect_opts(QemuOpts *opts); int inet_connect(const char *str, int socktype); int inet_dgram_opts(QemuOpts *opts); +const char *inet_strfamily(int family); int unix_listen_opts(QemuOpts *opts); int unix_listen(const char *path, char *ostr, int olen); -- 1.6.6
[Qemu-devel] [PATCH 0/3]: Small VNC related cleanup
This series makes VNC events code use inet_strfamily() from qemu-socket instead of duplicating code, as suggested by Gerd. Thanks.
[Qemu-devel] [PATCH 2/3] net: inet_strfamily(): Better unknown family report
Returning "" is a bit meaningless, let's call it "unknown". Signed-off-by: Luiz Capitulino --- qemu-sockets.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index 720de22..d912fed 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -98,7 +98,7 @@ const char *inet_strfamily(int family) case PF_INET: return "ipv4"; case PF_UNIX: return "unix"; } -return ""; +return "unknown"; } static void inet_print_addrinfo(const char *tag, struct addrinfo *res) -- 1.6.6
[Qemu-devel] [PATCH 3/3] vnc: Use inet_strfamily()
Signed-off-by: Luiz Capitulino --- vnc.c | 22 +- 1 files changed, 1 insertions(+), 21 deletions(-) diff --git a/vnc.c b/vnc.c index c7d6652..cc2a26e 100644 --- a/vnc.c +++ b/vnc.c @@ -100,26 +100,6 @@ char *vnc_socket_remote_addr(const char *format, int fd) { return addr_to_string(format, &sa, salen); } -static QString *get_sock_family(const struct sockaddr_storage *sa) -{ -const char *name; - -switch (sa->ss_family) -{ -case AF_INET: -name = "ipv4"; -break; -case AF_INET6: -name = "ipv6"; -break; -default: -name = "unknown"; -break; -} - -return qstring_from_str(name); -} - static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa, socklen_t salen) { @@ -138,7 +118,7 @@ static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa, qdict_put(qdict, "host", qstring_from_str(host)); qdict_put(qdict, "service", qstring_from_str(serv)); -qdict_put(qdict, "family", get_sock_family(sa)); +qdict_put(qdict, "family",qstring_from_str(inet_strfamily(sa->ss_family))); return 0; } -- 1.6.6
Re: [Qemu-devel] Re: Stop using "which" in ./configure
On Wed, Jan 20, 2010, Paolo Bonzini wrote: > On 01/20/2010 12:37 PM, Loïc Minier wrote: > >+# not found > >+IFS="$local_ifs" > If you do this, you should set IFS to space-tab-lf at the beginning of > the script, like this: > > IFS=" "" "" > " Are you saying that I can't backup/restore IFS without setting it first? That would be odd; it works with bash, dash, and zsh here. Pointers to affected shell would help me avoid other issues with them. Alternatively, I could set IFS in a subshell. I checked the autoconf Portable Shell Programming section, but didn't find a similar recommendation: http://www.gnu.org/software/autoconf/manual/autoconf.html#index-IFS-1639 > or this (better because it does not rely on embedding whitespace > characters within the line): > IFS=`printf ' \t'`" > " If we go that route, perhaps IFS="`printf ' \t\n'`" would be more readable? I'm not sure how common printf is though. -- Loïc Minier
Re: [Qemu-devel] Re: [PATCH 07/17] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
On Wed, Jan 20, 2010 at 3:03 PM, Markus Armbruster wrote: > "Kirill A. Shutemov" writes: >> No, compiler can't add anything between. 'char' is always byte-aligned. > > You got some reading to do then. Do you want to say that it's not true? Could you provide an example when 'char' isn't byte-aligned?