[Qemu-devel] Re: [PATCH] fix linux-user microblaze ELF_ARCH definition

[Edgar E. Iglesias]

On Sun, Jan 17, 2010 at 01:15:05AM -0500, Mike Frysinger wrote:
> Signed-off-by: Mike Frysinger 
> ---
>  linux-user/elfload.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)


Applied, thanks.


> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 9b65005..abc5332 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -652,7 +652,7 @@ static void elf_core_copy_regs(target_elf_gregset_t 
> *regs, const CPUState *env)
>  
>  #define ELF_CLASS   ELFCLASS32
>  #define ELF_DATA ELFDATA2MSB
> -#define ELF_ARCHEM_MIPS
> +#define ELF_ARCHEM_XILINX_MICROBLAZE
>  
>  static inline void init_thread(struct target_pt_regs *regs, struct 
> image_info *infop)
>  {
> -- 
> 1.6.6
> 

[Qemu-devel] [PATCH] block: fix cache flushing in bdrv_commit

[Christoph Hellwig]

On Thu, Jan 14, 2010 at 11:21:15AM +0100, Kevin Wolf wrote:
> Anthony, you seem to have missed the v2 patch that considered my review
> comments. Can you please apply the diff between v1 and v2 on top?

Here is the differences in patch form:

---
From: Christoph Hellwig 
Subject: block: fix cache flushing in bdrv_commit

As pointed out by Kevin Wolf the previous patch returned early if we
there is a bdrv_make_empty method and it missed a cache flush for the
frontend device after the bdev_make_empty call.  This patch fixes it
up.

Signed-off-by: Christoph Hellwig 


Index: qemu/block.c
===
--- qemu.orig/block.c   2010-01-17 12:27:03.589006970 +0100
+++ qemu/block.c2010-01-17 12:27:55.718008519 +0100
@@ -589,6 +589,7 @@ int bdrv_commit(BlockDriverState *bs)
 BlockDriver *drv = bs->drv;
 int64_t i, total_sectors;
 int n, j;
+int ret = 0;
 unsigned char sector[512];
 
 if (!drv)
@@ -620,8 +621,10 @@ int bdrv_commit(BlockDriverState *bs)
 }
 }
 
-if (drv->bdrv_make_empty)
-   return drv->bdrv_make_empty(bs);
+if (drv->bdrv_make_empty) {
+ret = drv->bdrv_make_empty(bs);
+bdrv_flush(bs);
+}
 
 /*
  * Make sure all data we wrote to the backing device is actually
@@ -629,7 +632,7 @@ int bdrv_commit(BlockDriverState *bs)
  */
 if (bs->backing_hd)
 bdrv_flush(bs->backing_hd);
-return 0;
+return ret;
 }
 
 /*

[Qemu-devel] Re: [PATCH] qemu-iotests: Test qemu-img rebase

[Christoph Hellwig]

Looks good, thanks!

Re: [Qemu-devel] [PATCH] linux-user: adapt uname machine to emulated CPU

[Loïc Minier]

On Thu, Jan 14, 2010, Aurelien Jarno wrote:
> This patch look ok, but is missing a Signed-off-by:

 Attaching a rebased patch with a Signed-off-by.  [ I didn't retest, but
 it applied without any merge issue and there was only two
 (arch-specific) commits in linux-user since the previous version. ]

   Thanks,
-- 
Loïc Minier
>From c4396cafac0041cfa2fed686f935e9df3636e05b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= 
Date: Tue, 29 Dec 2009 22:06:13 +0100
Subject: [PATCH] linux-user: adapt uname machine to emulated CPU

---
 Makefile.target|2 +-
 linux-user/cpu-uname.c |   72 
 linux-user/cpu-uname.h |1 +
 linux-user/syscall.c   |3 +-
 4 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 linux-user/cpu-uname.c
 create mode 100644 linux-user/cpu-uname.h

diff --git a/Makefile.target b/Makefile.target
index e661478..9f5bd17 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -95,7 +95,7 @@ $(call set-vpath, $(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR
 
 QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user -I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
 obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \
-  elfload.o linuxload.o uaccess.o gdbstub.o
+  elfload.o linuxload.o uaccess.o gdbstub.o cpu-uname.o
 
 obj-$(TARGET_HAS_BFLT) += flatload.o
 obj-$(TARGET_HAS_ELFLOAD32) += elfload32.o
diff --git a/linux-user/cpu-uname.c b/linux-user/cpu-uname.c
new file mode 100644
index 000..ddc37be
--- /dev/null
+++ b/linux-user/cpu-uname.c
@@ -0,0 +1,72 @@
+/*
+ *  cpu to uname machine name map
+ *
+ *  Copyright (c) 2009 Loïc Minier
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#include 
+
+#include "qemu.h"
+//#include "qemu-common.h"
+#include "cpu-uname.h"
+
+/* return highest utsname machine name for emulated instruction set
+ *
+ * NB: the default emulated CPU ("any") might not match any existing CPU, e.g.
+ * on ARM it has all features turned on, so there is no perfect arch string to
+ * return here */
+const char *cpu_to_uname_machine(void *cpu_env)
+{
+#ifdef TARGET_ARM
+/* utsname machine name on linux arm is CPU arch name + endianness, e.g.
+ * armv7l; to get a list of CPU arch names from the linux source, use:
+ * grep arch_name: -A1 linux/arch/arm/mm/proc-*.S
+ * see arch/arm/kernel/setup.c: setup_processor()
+ *
+ * to test by CPU id, compare cpu_env->cp15.c0_cpuid to ARM_CPUID_*
+ * defines and to test by CPU feature, use arm_feature(cpu_env,
+ * ARM_FEATURE_*) */
+
+/* in theory, endianness is configurable on some ARM CPUs, but this isn't
+ * used in user mode emulation */
+#ifdef TARGET_WORDS_BIGENDIAN
+#define utsname_suffix "b"
+#else
+#define utsname_suffix "l"
+#endif
+if (arm_feature(cpu_env, ARM_FEATURE_V7))
+return "armv7" utsname_suffix;
+if (arm_feature(cpu_env, ARM_FEATURE_V6))
+return "armv6" utsname_suffix;
+/* earliest emulated CPU is ARMv5TE; qemu can emulate the 1026, but not its
+ * Jazelle support */
+return "armv5te" utsname_suffix;
+#elif defined(TARGET_X86_64)
+return "x86-64";
+#elif defined(TARGET_I386)
+/* see arch/x86/kernel/cpu/bugs.c: check_bugs(), 386, 486, 586, 686 */
+uint32_t cpuid_version = ((CPUX86State *)cpu_env)->cpuid_version;
+int family = ((cpuid_version >> 8) & 0x0f) + ((cpuid_version >> 20) & 0xff);
+if (family == 4)
+return "i486";
+if (family == 5)
+return "i586";
+return "i686";
+#else
+/* default is #define-d in each arch/ subdir */
+return UNAME_MACHINE;
+#endif
+}
diff --git a/linux-user/cpu-uname.h b/linux-user/cpu-uname.h
new file mode 100644
index 000..32492de
--- /dev/null
+++ b/linux-user/cpu-uname.h
@@ -0,0 +1 @@
+const char *cpu_to_uname_machine(void *cpu_env);
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f2dd39e..9fb493f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -82,6 +82,7 @@
 #include 
 #include 
 #include "linux_loop.h"
+#include "cpu-uname.h"
 
 #include "qemu.h"
 #include "qemu-common.h"
@@ -5739,7 +5740,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 if (!is_error(ret)) {
 /* Overrite the native machine name with whatever is being
emulated. */
-strcpy (

[Qemu-devel] [PATCH] Fix missing symbols in .rela.plt sections

[Loïc Minier]

Hi there,

 Static builds of qemu on x86-64 (and probably i386) fail with:
gcc -I/home/lool/git/savannah/qemu/slirp -Werror -m64 -Wold-style-definition 
-Wold-style-declaration -I. -I/home/lool/git/savannah/qemu -U_FORTIFY_SOURCE 
-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wendif-labels -Wwrite-strings 
-Wmissing-prototypes -fno-strict-aliasing  -I/home/lool/git/savannah/qemu/fpu 
-I/home/lool/git/savannah/qemu/tcg -I/home/lool/git/savannah/qemu/tcg/x86_64  
-I.. -I/home/lool/git/savannah/qemu/target-arm -DNEED_CPU_H 
-I/home/lool/git/savannah/qemu/linux-user 
-I/home/lool/git/savannah/qemu/linux-user/arm -O2 -g  -static -Wl,--warn-common 
-m64 -g  -Wl,-T../config-host.ld -Wl,-T,/home/lool/git/savannah/qemu/x86_64.ld  
-o qemu-arm main.o syscall.o strace.o mmap.o signal.o thunk.o elfload.o 
linuxload.o uaccess.o gdbstub.o flatload.o gdbstub-xml.o nwfpe/fpa11.o 
nwfpe/fpa11_cpdo.o nwfpe/fpa11_cpdt.o nwfpe/fpa11_cprt.o nwfpe/fpopcode.o 
nwfpe/single_cpdo.o nwfpe/double_cpdo.o nwfpe/extended_cpdo.o arm-semi.o 
-Wl,--whole-archive ../libuser/libuser.a libqemu.a -Wl,--no-whole-archive -lrt 
-lpthread  -lm
/usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../../../lib/libc.a(elf-init.o): In 
function `__libc_csu_irel':
(.text+0xd4): undefined reference to `__rela_iplt_end'
/usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../../../lib/libc.a(elf-init.o): In 
function `__libc_csu_irel':
(.text+0xe5): undefined reference to `__rela_iplt_start'
/usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../../../lib/libc.a(elf-init.o): In 
function `__libc_csu_irel':
(.text+0x100): undefined reference to `__rela_iplt_start'
/usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../../../lib/libc.a(elf-init.o): In 
function `__libc_csu_irel':
(.text+0x10a): undefined reference to `__rela_iplt_start'
/usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../../../lib/libc.a(elf-init.o): In 
function `__libc_csu_irel':
(.text+0x10f): undefined reference to `__rela_iplt_start'
/usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../../../lib/libc.a(elf-init.o): In 
function `__libc_csu_irel':
(.text+0x114): undefined reference to `__rela_iplt_start'
collect2: ld returned 1 exit status

 This is due to changes in binutils + glibc, qemu's linker script need
 to be adjusted to include these symbols.  qemu's scripts weren't
 copying the .rela.iplt section at all, so I included this section and
 the __rela_iplt_start and __rela_iplt_end arount it.

 Tested by building qemu in static and shared mode for the
 
arm-softmmu,i386-softmmu,x86_64-softmmu,arm-linux-user,i386-linux-user,x86_64-linux-user
 target-list; I also ran qemu for almost all combinations.

   Thanks,
-- 
Loïc Minier
>From ac0fe6dabaf05bf434e9c57d4f9b61d73f660e3c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= 
Date: Sun, 17 Jan 2010 12:09:38 +0100
Subject: [PATCH] Fix missing symbols in .rela.plt sections
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix .rela.plt sections in the output to not only include .rela.plt
sections from the input but also the .rela.iplt sections and to define
the hidden symbols __rela_iplt_start and __rela_iplt_end around
.rela.iplt as otherwise we get undefined references to these when
linking statically to a multilib libc.a.

Signed-off-by: Loïc Minier 
---
 i386.ld   |8 +++-
 x86_64.ld |8 +++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/i386.ld b/i386.ld
index f2dafec..bb245a7 100644
--- a/i386.ld
+++ b/i386.ld
@@ -40,7 +40,13 @@ SECTIONS
   .rel.bss   : { *(.rel.bss)		}
   .rela.bss  : { *(.rela.bss)		}
   .rel.plt   : { *(.rel.plt)		}
-  .rela.plt  : { *(.rela.plt)		}
+  .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..684d2d7 100644
--- a/x86_64.ld
+++ b/x86_64.ld
@@ -36,7 +36,13 @@ SECTIONS
   .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) }
+  .rela.plt   :
+  {
+*(.rela.plt)
+PROVIDE_HIDDEN (__rela_iplt_start = .);
+*(.rela.iplt)
+PROVIDE_HIDDEN (__rela_iplt_end = .);
+  }
   .init   :
   {
 KEEP (*(.init))
-- 
1.6.5

[Qemu-devel] [PATCH] Add -static earlier to LDFLAGS for compile_prog()

[Loïc Minier]

Hi

 When configure qemu with --static, it might autodetect support for some
 features by looking at available shared libraries instead of static
 libraries.  e.g. if you have libbluetooth.so, bluez support will be
 turned on, but the build will fail at link stage.

 Setting LDFLAGS earlier to include -static will cause the
 compile_prog() tests to try to link against static libs.  This did
 disable bluetooth support properly on my system where libbluetooth.a
 isn't available.  Tested by building qemu configured with:
./configure --static 
--target-list=arm-softmmu,i386-softmmu,x86_64-softmmu,arm-linux-user,i386-linux-user,x86_64-linux-user
 and again without --static.

   Thanks,
-- 
Loïc Minier
>From 3df135de4babc35849578cde901ef9ffb04a2ab3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= 
Date: Sun, 17 Jan 2010 12:43:21 +0100
Subject: [PATCH] Add -static earlier to LDFLAGS for compile_prog()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 
---
 configure |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 5631bbb..e094111 100755
--- a/configure
+++ b/configure
@@ -451,7 +451,9 @@ for opt do
   ;;
   --enable-gprof) gprof="yes"
   ;;
-  --static) static="yes"
+  --static)
+static="yes"
+LDFLAGS="-static $LDFLAGS"
   ;;
   --disable-sdl) sdl="no"
   ;;
@@ -1968,7 +1970,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.5

[Qemu-devel] [PATCH] Check for sdl-config before calling it

[Loïc Minier]

Hi

 On systems were sdl-config isn't installed, ./configure triggers this
 warning:
./configure: 957: sdl-config: not found

 The attached patch fixes the warning for me.

   Thanks,
-- 
Loïc Minier
>From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= 
Date: Sun, 17 Jan 2010 13:42:04 +0100
Subject: [PATCH] Check for sdl-config before calling it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Check whether sdl-config is available before calling it, otherwise
./configure triggers a warning:
./configure: 957: sdl-config: not found

If neither the .pc file not sdl-config are present, disable SDL support.

Signed-off-by: Loïc Minier 
---
 configure |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 5631bbb..450e1a2 100755
--- a/configure
+++ b/configure
@@ -993,9 +993,11 @@ 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
+elif which sdl-config >/dev/null 2>&1; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
+else
+  sdl=no
 fi
 
 sdl_too_old=no
-- 
1.6.5

[Qemu-devel] Re: New kvm-related qemu patch queue

[Avi Kivity]

On 01/11/2010 05:30 PM, Anthony Liguori wrote:

On 01/10/2010 06:02 AM, Avi Kivity wrote:
In order to improve qemu.git kvm integration quality wrt performance, 
features, and reliability Marcelo and I will begin to maintain a 
patch queue based on qemu.git containing kvm-related patches.  We 
will review and apply patches to this queue, test them using the same 
test suite that is used for qemu-kvm.git, and regularly submit them 
for inclusion in qemu.git, mimicking the relationship between kvm.git 
and Linus' linux-2.6.git.


Thanks for setting this up Avi!

I just want to stress that everyone continue CC'ing qemu-devel on all 
KVM patches.  Even if the patch is qemu-kvm specific for the moment, I 
think it's important for qemu-devel to be exposed to the refactoring 
work.


It might be good to prefix qemu-kvm.git patches in some manner to make 
it clear which repository they belong to.


[patch kvm] - qemu-kvm.git uq/master
[patch qemu-kvm] - qemu-kvm.git master
[patch kvm stable-0.12] - qemu-kvm uq/stable-0.12

etc.

--
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH] Check for sdl-config before calling it

[Måns Rullgård]

Loïc Minier  writes:

> Hi
>
>  On systems were sdl-config isn't installed, ./configure triggers this
>  warning:
> ./configure: 957: sdl-config: not found
>
>  The attached patch fixes the warning for me.
>
>Thanks,
> -- 
> Loïc Minier
>
> From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Lo=C3=AFc=20Minier?= 
> Date: Sun, 17 Jan 2010 13:42:04 +0100
> Subject: [PATCH] Check for sdl-config before calling it
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Check whether sdl-config is available before calling it, otherwise
> ./configure triggers a warning:
> ./configure: 957: sdl-config: not found
>
> If neither the .pc file not sdl-config are present, disable SDL support.
>
> Signed-off-by: Loïc Minier 
> ---
>  configure |4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index 5631bbb..450e1a2 100755
> --- a/configure
> +++ b/configure
> @@ -993,9 +993,11 @@ 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
> +elif which sdl-config >/dev/null 2>&1; then
>sdlconfig='sdl-config'
>_sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
> +else
> +  sdl=no
>  fi

"which" isn't a standard command.  Would simply 2>/dev/null do the
trick just as well?  If you really need to test for the existence of
something, use "type".

-- 
Måns Rullgård
m...@mansr.com

Re: [Qemu-devel] Re: [PATCH] rtl8139: fix clang reporting unused assignment of VLAN tagging data

[Simon Horman]

On Fri, Jan 15, 2010 at 08:08:53AM +0100, Paolo Bonzini wrote:
> 
> >diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> >index 1f4f585..f04dd54 100644
> >--- a/hw/rtl8139.c
> >+++ b/hw/rtl8139.c
> >@@ -1909,6 +1909,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
> >
> >  cpu_physical_memory_read(cplus_tx_ring_desc,(uint8_t *)&val, 4);
> >  txdw0 = le32_to_cpu(val);
> >+/* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 
> >*/
> >  cpu_physical_memory_read(cplus_tx_ring_desc+4,  (uint8_t *)&val, 4);
> >  txdw1 = le32_to_cpu(val);
> >  cpu_physical_memory_read(cplus_tx_ring_desc+8,  (uint8_t *)&val, 4);
> >@@ -1920,6 +1921,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
> > descriptor,
> > txdw0, txdw1, txbufLO, txbufHI));
> >
> >+/* TODO: the following discard cast should clean clang analyzer output 
> >*/
> >+(void)txdw1;
> 
> I don't like this, why not comment it out like here:
> 
> >+/* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 
> >*/
> >  //val = cpu_to_le32(txdw1);
> >  //cpu_physical_memory_write(cplus_tx_ring_desc+4,&val, 4);
> 
> (and maybe change this one as well to #if 0...#endif, I don't know).

Personally I'd rather that all of this was removed
until the feature is implemented.

Re: [Qemu-devel] [PATCH] Check for sdl-config before calling it

[Stefan Weil]

Loïc Minier schrieb:
> Hi
>
> On systems were sdl-config isn't installed, ./configure triggers this
> warning:
> ./configure: 957: sdl-config: not found
>
> The attached patch fixes the warning for me.
>
> Thanks,

Hi,

which version did you test?
Git master has no sdl-config call at configure:957.

But I get warning messages, too, when pkg-config or
sdl-config are missing.

Your patch fixes the warning for sdl-config and sets
sdl=no. If configure was called with --enable-sdl,
this solution is wrong: configure should abort with
an error message (feature_not_found).

Regards,
Stefan

Re: [Qemu-devel] Re: [PATCH] Check for sdl-config before calling it

[Stefan Weil]

Måns Rullgård schrieb:
> Loïc Minier  writes:
>
>   
>> Hi
>>
>>  On systems were sdl-config isn't installed, ./configure triggers this
>>  warning:
>> ./configure: 957: sdl-config: not found
>>
>>  The attached patch fixes the warning for me.
>>
>>Thanks,
>> -- 
>> Loïc Minier
>>
>> From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Lo=C3=AFc=20Minier?= 
>> Date: Sun, 17 Jan 2010 13:42:04 +0100
>> Subject: [PATCH] Check for sdl-config before calling it
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> Check whether sdl-config is available before calling it, otherwise
>> ./configure triggers a warning:
>> ./configure: 957: sdl-config: not found
>>
>> If neither the .pc file not sdl-config are present, disable SDL support.
>>
>> Signed-off-by: Loïc Minier 
>> ---
>>  configure |4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 5631bbb..450e1a2 100755
>> --- a/configure
>> +++ b/configure
>> @@ -993,9 +993,11 @@ 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
>> +elif which sdl-config >/dev/null 2>&1; then
>>sdlconfig='sdl-config'
>>_sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
>> +else
>> +  sdl=no
>>  fi
>> 
>
> "which" isn't a standard command.  Would simply 2>/dev/null do the
> trick just as well?  If you really need to test for the existence of
> something, use "type".
>   

"which" is already used several times in configure,
so this would not make things worse.

A macro for all these tests might be a better solution.
And that macro should use "type" or "type -t".

Regards,
Stefan

[Qemu-devel] Re: [PATCH] Check for sdl-config before calling it

[Måns Rullgård]

Stefan Weil  writes:

> Måns Rullgård schrieb:
>> Loïc Minier  writes:
>>
>>   
>>> Hi
>>>
>>>  On systems were sdl-config isn't installed, ./configure triggers this
>>>  warning:
>>> ./configure: 957: sdl-config: not found
>>>
>>>  The attached patch fixes the warning for me.
>>>
>>>Thanks,
>>> -- 
>>> Loïc Minier
>>>
>>> From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
>>> From: =?UTF-8?q?Lo=C3=AFc=20Minier?= 
>>> Date: Sun, 17 Jan 2010 13:42:04 +0100
>>> Subject: [PATCH] Check for sdl-config before calling it
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> Check whether sdl-config is available before calling it, otherwise
>>> ./configure triggers a warning:
>>> ./configure: 957: sdl-config: not found
>>>
>>> If neither the .pc file not sdl-config are present, disable SDL support.
>>>
>>> Signed-off-by: Loïc Minier 
>>> ---
>>>  configure |4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index 5631bbb..450e1a2 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -993,9 +993,11 @@ 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
>>> +elif which sdl-config >/dev/null 2>&1; then
>>>sdlconfig='sdl-config'
>>>_sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
>>> +else
>>> +  sdl=no
>>>  fi
>>> 
>>
>> "which" isn't a standard command.  Would simply 2>/dev/null do the
>> trick just as well?  If you really need to test for the existence of
>> something, use "type".
>>   
>
> "which" is already used several times in configure,

Then those should be fixed.

> so this would not make things worse.

One error does not justify another.

> A macro for all these tests might be a better solution.
> And that macro should use "type" or "type -t".

"type -t" is also not standard.  The standard "type" has no options.

In most cases the simpler solution is still probably to try the
command and let it fail.

-- 
Måns Rullgård
m...@mansr.com

[Qemu-devel] [PATCH v2 0/4] Modifications to the drives' readonly attribute

[Naphtali Sprei]

This is version 2, to replace previous set.
Addresses all Kevin comments.


Naphtali Sprei (4):
  Make CDROM a read-only drive
  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 explicitly request
READ-WRITE.
  Added drives' readonly option
  Disable fall-back to read-only when cannot open drive's file for
read-write

 block.c   |   29 +
 block.h   |2 --
 block/raw-posix.c |2 +-
 block/raw-win32.c |4 ++--
 block/vmdk.c  |9 +
 hw/xen_disk.c |2 +-
 monitor.c |2 +-
 qemu-img.c|   10 ++
 qemu-io.c |   14 ++
 qemu-nbd.c|2 +-
 qemu-options.hx   |2 +-
 vl.c  |   13 ++---
 12 files changed, 47 insertions(+), 44 deletions(-)

[Qemu-devel] [PATCH v2 1/4] Make CDROM a read-only drive

[Naphtali Sprei]

Signed-off-by: Naphtali Sprei 
---
 vl.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 06cb40d..76ef8ca 100644
--- a/vl.c
+++ b/vl.c
@@ -2233,6 +2233,13 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
 }
 (void)bdrv_set_read_only(dinfo->bdrv, 1);
 }
+/* 
+ * cdrom is read-only. Set it now, after above interface checking
+ * since readonly attribute not explicitly required, so no error.
+ */
+if (media == MEDIA_CDROM) {
+(void)bdrv_set_read_only(dinfo->bdrv, 1);
+}
 
 if (bdrv_open2(dinfo->bdrv, file, bdrv_flags, drv) < 0) {
 fprintf(stderr, "qemu: could not open disk image %s: %s\n",
-- 
1.6.3.3

[Qemu-devel] [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 explici

[Naphtali Sprei]

Instead of using the field 'readonly' of the BlockDriverState struct for 
passing the request,
pass the request in the flags parameter to the function.

Signed-off-by: Naphtali Sprei 
---
 block.c   |   31 +--
 block.h   |2 --
 block/raw-posix.c |2 +-
 block/raw-win32.c |4 ++--
 block/vmdk.c  |9 +
 hw/xen_disk.c |2 +-
 monitor.c |2 +-
 qemu-img.c|   10 ++
 qemu-io.c |   14 ++
 qemu-nbd.c|2 +-
 vl.c  |8 
 11 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/block.c b/block.c
index 115e591..8def3c4 100644
--- a/block.c
+++ b/block.c
@@ -310,7 +310,7 @@ static BlockDriver *find_image_format(const char *filename)
 if (drv && strcmp(drv->format_name, "vvfat") == 0)
 return drv;
 
-ret = bdrv_file_open(&bs, filename, BDRV_O_RDONLY);
+ret = bdrv_file_open(&bs, filename, 0);
 if (ret < 0)
 return NULL;
 ret = bdrv_pread(bs, 0, buf, sizeof(buf));
@@ -356,7 +356,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, 
int flags)
 int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
BlockDriver *drv)
 {
-int ret, open_flags, try_rw;
+int ret, open_flags;
 char tmp_filename[PATH_MAX];
 char backing_filename[PATH_MAX];
 
@@ -446,19 +446,23 @@ int bdrv_open2(BlockDriverState *bs, const char 
*filename, int flags,
 
 /* Note: for compatibility, we open disk image files as RDWR, and
RDONLY as fallback */
-try_rw = !bs->read_only || bs->is_temporary;
-if (!(flags & BDRV_O_FILE))
-open_flags = (try_rw ? BDRV_O_RDWR : 0) |
-(flags & (BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
-else
+bs->read_only = (flags & BDRV_O_RDWR) == 0;
+if (!(flags & BDRV_O_FILE)) {
+open_flags = (flags & (BDRV_O_RDWR | 
BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
+if (bs->is_temporary) { /* snapshot should be writeable */
+open_flags |= BDRV_O_RDWR;
+}
+} else {
 open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
-if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv))
+}
+if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
 ret = -ENOTSUP;
-else
+} else {
 ret = drv->bdrv_open(bs, filename, open_flags);
-if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
-ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
-bs->read_only = 1;
+if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
+ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
+bs->read_only = 1;
+}
 }
 if (ret < 0) {
 qemu_free(bs->opaque);
@@ -481,14 +485,13 @@ int bdrv_open2(BlockDriverState *bs, const char 
*filename, int flags,
 /* if there is a backing file, use it */
 BlockDriver *back_drv = NULL;
 bs->backing_hd = bdrv_new("");
-/* pass on read_only property to the backing_hd */
-bs->backing_hd->read_only = bs->read_only;
 path_combine(backing_filename, sizeof(backing_filename),
  filename, bs->backing_file);
 if (bs->backing_format[0] != '\0')
 back_drv = bdrv_find_format(bs->backing_format);
 ret = bdrv_open2(bs->backing_hd, backing_filename, open_flags,
  back_drv);
+bs->backing_hd->read_only =  (open_flags & BDRV_O_RDWR) == 0;
 if (ret < 0) {
 bdrv_close(bs);
 return ret;
diff --git a/block.h b/block.h
index bee9ec5..fd4e8dd 100644
--- a/block.h
+++ b/block.h
@@ -27,9 +27,7 @@ typedef struct QEMUSnapshotInfo {
 uint64_t vm_clock_nsec; /* VM clock relative to boot */
 } QEMUSnapshotInfo;
 
-#define BDRV_O_RDONLY  0x
 #define BDRV_O_RDWR0x0002
-#define BDRV_O_ACCESS  0x0003
 #define BDRV_O_CREAT   0x0004 /* create an empty file */
 #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes 
in a snapshot */
 #define BDRV_O_FILE0x0010 /* open as a raw file (do not try to
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 5a6a22b..b427211 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -138,7 +138,7 @@ static int raw_open_common(BlockDriverState *bs, const char 
*filename,
 
 s->open_flags = open_flags | O_BINARY;
 s->open_flags &= ~O_ACCMODE;
-if ((bdrv_flags & BDRV_O_ACCESS) == BDRV_O_RDWR) {
+if (bdrv_flags & BDRV_O_RDWR) {
 s->open_flags |= O_RDWR;
 } else {
 s->open_flags |= O_RDONLY;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 72acad5..01a6d25 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -81,7 +81,7 @@ static int raw_open(BlockDriverState *bs, const char 
*filename, int flags)
 
 s->type = FTYPE_FILE;
 
-if ((flags & BDRV_O_ACCESS) == O_RDWR) {
+if (flags & BDRV_O_R

[Qemu-devel] [PATCH v2 3/4] Added drives' readonly option

[Naphtali Sprei]

Signed-off-by: Naphtali Sprei 
---
 qemu-options.hx |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index e2edd71..4617867 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -103,7 +103,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
 "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
 "   [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
 "   [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
-"   [,addr=A][,id=name][,aio=threads|native]\n"
+"   [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n"
 "use 'file' as a drive image\n")
 DEF("set", HAS_ARG, QEMU_OPTION_set,
 "-set group.id.arg=value\n"
-- 
1.6.3.3

[Qemu-devel] [PATCH v2 4/4] Disable fall-back to read-only when cannot open drive's file for read-write

[Naphtali Sprei]

Signed-off-by: Naphtali Sprei 
---
 block.c |6 --
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 8def3c4..f90e983 100644
--- a/block.c
+++ b/block.c
@@ -444,8 +444,6 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
 if (flags & (BDRV_O_CACHE_WB|BDRV_O_NOCACHE))
 bs->enable_write_cache = 1;
 
-/* Note: for compatibility, we open disk image files as RDWR, and
-   RDONLY as fallback */
 bs->read_only = (flags & BDRV_O_RDWR) == 0;
 if (!(flags & BDRV_O_FILE)) {
 open_flags = (flags & (BDRV_O_RDWR | 
BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
@@ -459,10 +457,6 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
 ret = -ENOTSUP;
 } else {
 ret = drv->bdrv_open(bs, filename, open_flags);
-if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
-ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
-bs->read_only = 1;
-}
 }
 if (ret < 0) {
 qemu_free(bs->opaque);
-- 
1.6.3.3

[Qemu-devel] Re: [PATCH v2 4/4] Disable fall-back to read-only when cannot open drive's file for read-write

[Michael S. Tsirkin]

On Sun, Jan 17, 2010 at 04:48:15PM +0200, Naphtali Sprei wrote:
> 
> Signed-off-by: Naphtali Sprei 
> ---
>  block.c |6 --
>  1 files changed, 0 insertions(+), 6 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 8def3c4..f90e983 100644
> --- a/block.c
> +++ b/block.c
> @@ -444,8 +444,6 @@ int bdrv_open2(BlockDriverState *bs, const char 
> *filename, int flags,
>  if (flags & (BDRV_O_CACHE_WB|BDRV_O_NOCACHE))
>  bs->enable_write_cache = 1;
>  
> -/* Note: for compatibility, we open disk image files as RDWR, and
> -   RDONLY as fallback */
>  bs->read_only = (flags & BDRV_O_RDWR) == 0;
>  if (!(flags & BDRV_O_FILE)) {
>  open_flags = (flags & (BDRV_O_RDWR | 
> BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
> @@ -459,10 +457,6 @@ int bdrv_open2(BlockDriverState *bs, const char 
> *filename, int flags,
>  ret = -ENOTSUP;
>  } else {
>  ret = drv->bdrv_open(bs, filename, open_flags);
> -if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
> -ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
> -bs->read_only = 1;
> -}

Maybe print an error message explaining the problem and
suggesting the solution?

>  }
>  if (ret < 0) {
>  qemu_free(bs->opaque);
> -- 
> 1.6.3.3
> 
> 

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

[Michael S. Tsirkin]

On Sun, Jan 17, 2010 at 04:48:13PM +0200, Naphtali Sprei wrote:
> Instead of using the field 'readonly' of the BlockDriverState struct for 
> passing the request,
> pass the request in the flags parameter to the function.
> 
> Signed-off-by: Naphtali Sprei 

Many changes seem to be about passing 0 to bdrv_open. This is not what
the changelog says the patch does. Better split unrelated changes to a
separate patch.

One of the things you seem to do is get rid of BDRV_O_RDONLY.  Why is
this an improvement? Symbolic name like BDRV_O_RDONLY seems better than
0.

> ---
>  block.c   |   31 +--
>  block.h   |2 --
>  block/raw-posix.c |2 +-
>  block/raw-win32.c |4 ++--
>  block/vmdk.c  |9 +
>  hw/xen_disk.c |2 +-
>  monitor.c |2 +-
>  qemu-img.c|   10 ++
>  qemu-io.c |   14 ++
>  qemu-nbd.c|2 +-
>  vl.c  |8 
>  11 files changed, 44 insertions(+), 42 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 115e591..8def3c4 100644
> --- a/block.c
> +++ b/block.c
> @@ -310,7 +310,7 @@ static BlockDriver *find_image_format(const char 
> *filename)
>  if (drv && strcmp(drv->format_name, "vvfat") == 0)
>  return drv;
>  
> -ret = bdrv_file_open(&bs, filename, BDRV_O_RDONLY);
> +ret = bdrv_file_open(&bs, filename, 0);
>  if (ret < 0)
>  return NULL;
>  ret = bdrv_pread(bs, 0, buf, sizeof(buf));
> @@ -356,7 +356,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, 
> int flags)
>  int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
> BlockDriver *drv)
>  {
> -int ret, open_flags, try_rw;
> +int ret, open_flags;
>  char tmp_filename[PATH_MAX];
>  char backing_filename[PATH_MAX];
>  
> @@ -446,19 +446,23 @@ int bdrv_open2(BlockDriverState *bs, const char 
> *filename, int flags,
>  
>  /* Note: for compatibility, we open disk image files as RDWR, and
> RDONLY as fallback */
> -try_rw = !bs->read_only || bs->is_temporary;
> -if (!(flags & BDRV_O_FILE))
> -open_flags = (try_rw ? BDRV_O_RDWR : 0) |
> -(flags & (BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
> -else
> +bs->read_only = (flags & BDRV_O_RDWR) == 0;

!(flags & BDRV_O_RDWR) is a standard idiom, and it's more readable IMO.

> +if (!(flags & BDRV_O_FILE)) {
> +open_flags = (flags & (BDRV_O_RDWR | 
> BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
> +if (bs->is_temporary) { /* snapshot should be writeable */
> +open_flags |= BDRV_O_RDWR;
> +}
> +} else {
>  open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
> -if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv))
> +}
> +if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
>  ret = -ENOTSUP;
> -else
> +} else {
>  ret = drv->bdrv_open(bs, filename, open_flags);
> -if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
> -ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
> -bs->read_only = 1;
> +if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
> +ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
> +bs->read_only = 1;
> +}
>  }
>  if (ret < 0) {
>  qemu_free(bs->opaque);
> @@ -481,14 +485,13 @@ int bdrv_open2(BlockDriverState *bs, const char 
> *filename, int flags,
>  /* if there is a backing file, use it */
>  BlockDriver *back_drv = NULL;
>  bs->backing_hd = bdrv_new("");
> -/* pass on read_only property to the backing_hd */
> -bs->backing_hd->read_only = bs->read_only;
>  path_combine(backing_filename, sizeof(backing_filename),
>   filename, bs->backing_file);
>  if (bs->backing_format[0] != '\0')
>  back_drv = bdrv_find_format(bs->backing_format);
>  ret = bdrv_open2(bs->backing_hd, backing_filename, open_flags,
>   back_drv);
> +bs->backing_hd->read_only =  (open_flags & BDRV_O_RDWR) == 0;

!(open_flags & BDRV_O_RDWR) is a standard idiom and it's more readable
IMO.
Further, pls don't put two spaces after ==.

>  if (ret < 0) {
>  bdrv_close(bs);
>  return ret;
> diff --git a/block.h b/block.h
> index bee9ec5..fd4e8dd 100644
> --- a/block.h
> +++ b/block.h
> @@ -27,9 +27,7 @@ typedef struct QEMUSnapshotInfo {
>  uint64_t vm_clock_nsec; /* VM clock relative to boot */
>  } QEMUSnapshotInfo;
>  
> -#define BDRV_O_RDONLY  0x
>  #define BDRV_O_RDWR0x0002
> -#define BDRV_O_ACCESS  0x0003
>  #define BDRV_O_CREAT   0x0004 /* create an empty file */
>  #define BDRV_O_SNAPSHOT0x0008 /* open the file read only and save writes 
> in a snapshot */
>  #define BDRV_O_FILE0x0010 /* open as a raw file (do not try to
> diff --git a

Re: [Qemu-devel] [PATCH] Check for sdl-config before calling it

[Loïc Minier]

On Sun, Jan 17, 2010, Stefan Weil wrote:
> > On systems were sdl-config isn't installed, ./configure triggers this
> > warning:
> > ./configure: 957: sdl-config: not found
> 
> which version did you test?
> Git master has no sdl-config call at configure:957.
> 
> But I get warning messages, too, when pkg-config or
> sdl-config are missing.

 Yes, the line is bogus for me as well; not sure why.  I'm using master.

> Your patch fixes the warning for sdl-config and sets
> sdl=no. If configure was called with --enable-sdl,
> this solution is wrong: configure should abort with
> an error message (feature_not_found).

 Ack; how about the attached one instead?

-- 
Loïc Minier
>From eb4ba45b0d71d13cb89f6362443ffbe50b65eba0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= 
Date: Sun, 17 Jan 2010 13:42:04 +0100
Subject: [PATCH] Check for sdl-config before calling it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Check whether sdl-config is available before calling it, otherwise
./configure triggers a warning:
./configure: 957: sdl-config: not found

If neither the .pc file not sdl-config are present, disable SDL support.

Signed-off-by: Loïc Minier 
---
 configure |7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 5631bbb..baa2800 100755
--- a/configure
+++ b/configure
@@ -993,9 +993,14 @@ 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
+elif which sdl-config >/dev/null 2>&1; 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
 
 sdl_too_old=no
-- 
1.6.5

[Qemu-devel] [RFC] Don't send local debug output to stdout (was: pm_smbus: remove #ifdef DEBUG)

[Stefan Weil]

Isaku Yamahata schrieb:
> On Wed, Jan 06, 2010 at 12:42:28PM +0100, Stefan Weil wrote:
>> Isaku Yamahata schrieb:
>>> remove #ifdef DEBUG by using macro.
>>>
>>> Signed-off-by: Isaku Yamahata 
>>> Acked-by: Gerd Hoffmann 
>>> ---
>>> hw/pm_smbus.c | 21 -
>>> 1 files changed, 12 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/hw/pm_smbus.c b/hw/pm_smbus.c
>>> index 6ef6b9e..9929d72 100644
>>> --- a/hw/pm_smbus.c
>>> +++ b/hw/pm_smbus.c
>>> @@ -37,6 +37,15 @@
>>> #define SMBHSTDAT1 0x06
>>> #define SMBBLKDAT 0x07
>>>
>>> +//#define DEBUG
>>> +
>>> +#ifdef DEBUG
>>> +# define SMBUS_DPRINTF(format, ...) printf(format, ## __VA_ARGS__)
>> Debug output should go to stderr. So this would be even better:
>>
>> +# define SMBUS_DPRINTF(format, ...) fprintf(stderr, format, ##
>> __VA_ARGS__)
>>
>
> Yes, in general.
> However the original code sends debug output to stdout and
> Most of debug output goes to stdout than stderr. You can easily
> see it by greping with DEBUG.
>
> So at this time, I'd like to send debug output to stdout.
> (And hopefully later create a framework and make debug output go
> to stderr consistently over sources.)

You are correct: there is much code which writes debug output to stdout.

When qemu is running in system mode with curses interface (-curses) or
serial console only (-nographic), debug output and system output will
be mixed on stdout. In many cases, this makes the output unreadable.

Therefore I prefer a separate output channel for debug messages.
The usual (and easiest) solution is directing them to stderr.
This allows mixing normal and debug output (sometimes a useful
feature) or separating them by using standard I/O redirection.

A more advanced solution could provide special logging functions.
Those could write debug messages to qemu.log (or some other file).
Debug messages from drivers could be prefixed using the qdev driver
name, we could support levels (info, warning, error, ),
more than one output channel - everything typical logging frameworks
like log4c (or others) support.

I suggest these steps:

1. Debug output to stdout is no longer accepted for new / modified code.

2. New or modified debug messages should go to stderr.

3. Patches which switch debug output from stdout to stderr are accepted.

4. We discuss whether debug output to stderr is sufficient
   or which logging framework should be used.

Please note that I talk here about debug log messages
which are activated by conditional compilation and
which are disabled normally (not those going to qemu.log).

Any comments are welcome.

Kind regards,
Stefan Weil

[Qemu-devel] [PATCH] [For stable-0.12] Sync OSS_GETVERSION handling with head

[Juergen Lock]

As suggested by Andreas Färber, here is a cumulative patch that syncs
OSS_GETVERSION handling with head by merging the following commits:

1. oss: issue OSS_GETVERSION ioctl only when needed
   6d246526ce3c145b2831285def6983f5de6190d3

2. oss: fix fragment setting
   3d709fe73a77c40e263b3af6e650fd4b519c3562

3. Workaround for broken OSS_GETVERSION on FreeBSD, part two
   72ff25e4e98d6dba9286d032b9ff5432553bbad5

Signed-off-by: Juergen Lock 

--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -38,6 +38,10 @@
 #define AUDIO_CAP "oss"
 #include "audio_int.h"
 
+#if defined OSS_GETVERSION && defined SNDCTL_DSP_POLICY
+#define USE_DSP_POLICY
+#endif
+
 typedef struct OSSVoiceOut {
 HWVoiceOut hw;
 void *pcm_buf;
@@ -236,14 +240,39 @@ static void oss_dump_info (struct oss_pa
 }
 #endif
 
+#ifdef USE_DSP_POLICY
+static int oss_get_version (int fd, int *version, const char *typ)
+{
+if (ioctl (fd, OSS_GETVERSION, &version)) {
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+/*
+ * Looks like atm (20100109) FreeBSD knows OSS_GETVERSION
+ * since 7.x, but currently only on the mixer device (or in
+ * the Linuxolator), and in the native version that part of
+ * the code is in fact never reached so the ioctl fails anyway.
+ * Until this is fixed, just check the errno and if its what
+ * FreeBSD's sound drivers return atm assume they are new enough.
+ */
+if (errno == EINVAL) {
+*version = 0x04;
+return 0;
+}
+#endif
+oss_logerr2 (errno, typ, "Failed to get OSS version\n");
+return -1;
+}
+return 0;
+}
+#endif
+
 static int oss_open (int in, struct oss_params *req,
  struct oss_params *obt, int *pfd)
 {
 int fd;
-int version;
 int oflags = conf.exclusive ? O_EXCL : 0;
 audio_buf_info abinfo;
 int fmt, freq, nchannels;
+int setfragment = 1;
 const char *dspname = in ? conf.devpath_in : conf.devpath_out;
 const char *typ = in ? "ADC" : "DAC";
 
@@ -281,27 +310,30 @@ static int oss_open (int in, struct oss_
 goto err;
 }
 
-if (ioctl (fd, OSS_GETVERSION, &version)) {
-oss_logerr2 (errno, typ, "Failed to get OSS version\n");
-version = 0;
-}
+#ifdef USE_DSP_POLICY
+if (conf.policy >= 0) {
+int version;
 
-if (conf.debug) {
-dolog ("OSS version = %#x\n", version);
-}
+if (!oss_get_version (fd, &version, typ)) {
+if (conf.debug) {
+dolog ("OSS version = %#x\n", version);
+}
 
-#ifdef SNDCTL_DSP_POLICY
-if (conf.policy >= 0 && version >= 0x04) {
-int policy = conf.policy;
-if (ioctl (fd, SNDCTL_DSP_POLICY, &policy)) {
-oss_logerr2 (errno, typ, "Failed to set timing policy to %d\n",
- conf.policy);
-goto err;
+if (version >= 0x04) {
+int policy = conf.policy;
+if (ioctl (fd, SNDCTL_DSP_POLICY, &policy)) {
+oss_logerr2 (errno, typ,
+ "Failed to set timing policy to %d\n",
+ conf.policy);
+goto err;
+}
+setfragment = 0;
+}
 }
 }
-else
 #endif
-{
+
+if (setfragment) {
 int  = (req->nfrags << 16) | ctz32 (req->fragsize);
 if (ioctl (fd, SNDCTL_DSP_SETFRAGMENT, &)) {
 oss_logerr2 (errno, typ, "Failed to set buffer length (%d, %d)\n",
@@ -857,7 +889,7 @@ static struct audio_option oss_options[]
 .valp  = &conf.exclusive,
 .descr = "Open device in exclusive mode (vmix wont work)"
 },
-#ifdef SNDCTL_DSP_POLICY
+#ifdef USE_DSP_POLICY
 {
 .name  = "POLICY",
 .tag   = AUD_OPT_INT,

[Qemu-devel] Re: sparc32 do not clear interrupts when masking

[Artyom Tarasenko]

2010/1/16 Blue Swirl :
> On Sat, Jan 16, 2010 at 8:47 AM, Blue Swirl  wrote:
>> On Sat, Jan 16, 2010 at 8:16 AM, Blue Swirl  wrote:
>>> On Fri, Jan 15, 2010 at 10:37 PM, Artyom Tarasenko
>>>  wrote:
 Don't clear interrupts on disabling, because
 * Sun4M_SystemArchitecture_edited2.pdf doesn't describe
  that masking or un-masking IRQ shall clear pending ones.

 * Field tests also show that SPARCstation-20 doesn't
  clear them.
>>>
>>> Awesome work!
>>>
 * The patch makes Solaris 2.5.1/2.6 boot ~1500 times
  faster (~20 seconds instead of ~8 hours)
>>>
>>> Unfortunately there is some problem with the patch (or more likely
>>> there is some other bug that this uncovers), because all my Linux test
>>> now panic. NetBSD and OpenBSD are unaffected.
>>>
>>> For example, sparc-test:
>>> eth0: LANCE 52:54:00:12:34:56
>>> esp0: no command in esp_handle()
>>> Kernel panic - not syncing: esp_handle: current_SC == penguin within 
>>> interrupt!
>>>  <0>Press L1-A to return to the boot prom
>>>
>>> The bug may be in ESP interrupt handling (or in the interrupt chain
>>> between ESP, DMA controller and interrupt controller) as the panic
>>> always happens in ESP probe.
>>
>> In fact the bug was that OpenBIOS didn't clear ESP interrupts after
>> issuing commands, so the IRQ line was left raised which confused
>> Linux.
>>
>
> Thanks, applied. I fixed the OpenBIOS bug and updated images in pc-bios.

Looks like this version of the OpenBIOS has a sort of a regression: it
doesn't recognize
NetBSD 1.3.3 miniroot anymore:

qemu-system-sparc -nographic  -hda ../images/miniroot-133.fs -m 64 -M SS-20

Welcome to OpenBIOS v1.0 built on Jan 16 2010 08:54
  Type 'help' for detailed information

[sparc] Booting file 'disk' with parameters ''
Trying disk (disk)
Trying disk:d (disk:d)
Unsupported image format

I say "sort of" because it may be the correct behavior: due to the bad
disk labels OBP doesn't boot miniroots either.

On the other hand it doesn't recognize Solaris cdroms too:

0 > boot cdrom
[sparc] Booting file 'cdrom' with parameters ''
Trying cdrom (cdrom)
Trying cdrom:d (cdrom:d)
Unsupported image format
 ok


-- 
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/

[Qemu-devel] Re: sparc32 do not clear interrupts when masking

[Blue Swirl]

On Sun, Jan 17, 2010 at 6:42 PM, Artyom Tarasenko
 wrote:
> 2010/1/16 Blue Swirl :
>> On Sat, Jan 16, 2010 at 8:47 AM, Blue Swirl  wrote:
>>> On Sat, Jan 16, 2010 at 8:16 AM, Blue Swirl  wrote:
 On Fri, Jan 15, 2010 at 10:37 PM, Artyom Tarasenko
  wrote:
> Don't clear interrupts on disabling, because
> * Sun4M_SystemArchitecture_edited2.pdf doesn't describe
>  that masking or un-masking IRQ shall clear pending ones.
>
> * Field tests also show that SPARCstation-20 doesn't
>  clear them.

 Awesome work!

> * The patch makes Solaris 2.5.1/2.6 boot ~1500 times
>  faster (~20 seconds instead of ~8 hours)

 Unfortunately there is some problem with the patch (or more likely
 there is some other bug that this uncovers), because all my Linux test
 now panic. NetBSD and OpenBSD are unaffected.

 For example, sparc-test:
 eth0: LANCE 52:54:00:12:34:56
 esp0: no command in esp_handle()
 Kernel panic - not syncing: esp_handle: current_SC == penguin within 
 interrupt!
  <0>Press L1-A to return to the boot prom

 The bug may be in ESP interrupt handling (or in the interrupt chain
 between ESP, DMA controller and interrupt controller) as the panic
 always happens in ESP probe.
>>>
>>> In fact the bug was that OpenBIOS didn't clear ESP interrupts after
>>> issuing commands, so the IRQ line was left raised which confused
>>> Linux.
>>>
>>
>> Thanks, applied. I fixed the OpenBIOS bug and updated images in pc-bios.
>
> Looks like this version of the OpenBIOS has a sort of a regression: it
> doesn't recognize
> NetBSD 1.3.3 miniroot anymore:
>
> qemu-system-sparc -nographic  -hda ../images/miniroot-133.fs -m 64 -M SS-20
>
> Welcome to OpenBIOS v1.0 built on Jan 16 2010 08:54
>  Type 'help' for detailed information
>
> [sparc] Booting file 'disk' with parameters ''
> Trying disk (disk)
> Trying disk:d (disk:d)
> Unsupported image format
>
> I say "sort of" because it may be the correct behavior: due to the bad
> disk labels OBP doesn't boot miniroots either.
>
> On the other hand it doesn't recognize Solaris cdroms too:
>
> 0 > boot cdrom
> [sparc] Booting file 'cdrom' with parameters ''
> Trying cdrom (cdrom)
> Trying cdrom:d (cdrom:d)
> Unsupported image format
>  ok

Yes, that's what I meant with committing the wrong fix,
694b9309462f07307d16f492961f01271f10c245 doesn't boot anything. That
will cause bisection problems, sorry.

OpenBIOS r666 should work as before. I'm considering whether to update
the ROMs, but it seems lame to update them so soon.