Re: [Qemu-devel] [Spice-devel] Possible SPICE/QEMU deadlock on fast client reconnect
Hi, Sounds like https://bugzilla.redhat.com/show_bug.cgi?id=746950 and https://bugs.freedesktop.org/show_bug.cgi?id=41858 Alon's patch: http://lists.freedesktop.org/archives/spice-devel/2011-September /005369.html Should solve it. Cheers, Yonit. On 10/21/2011 05:26 PM, Daniel P. Berrange wrote: In testing my patches for 'add_client' support with SPICE, I noticed deadlock in the QEMU/SPICE code. It only happens if I close a SPICE client and then immediately reconnect within about 1 second. If I wait a couple of seconds before reconnecting the SPICE client I don't see the deadlock. I'm using QEMU& SPICE git master, and GDB has this to say (gdb) thread apply all bt Thread 3 (Thread 0x7f269e142700 (LWP 17233)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:165 #1 0x004e80e9 in qemu_cond_wait (cond=, mutex=) at qemu-thread-posix.c:113 #2 0x00556469 in qemu_kvm_wait_io_event (env=) at /home/berrange/src/virt/qemu/cpus.c:626 #3 qemu_kvm_cpu_thread_fn (arg=0x29217a0) at /home/berrange/src/virt/qemu/cpus.c:661 #4 0x003a83207b31 in start_thread (arg=0x7f269e142700) at pthread_create.c:305 #5 0x003a82edfd2d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:115 Thread 2 (Thread 0x7f26822fc700 (LWP 17234)): #0 __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:140 #1 0x003a83209ad6 in _L_lock_752 () from /lib64/libpthread.so.0 #2 0x003a832099d7 in __pthread_mutex_lock (mutex=0x1182f60) at pthread_mutex_lock.c:65 #3 0x004e7ec9 in qemu_mutex_lock (mutex=) at qemu-thread-posix.c:54 #4 0x00507df5 in channel_event (event=3, info=0x2a3c610) at ui/spice-core.c:234 #5 0x7f269f77be87 in reds_stream_free (s=0x2a3c590) at reds.c:4073 #6 0x7f269f75b64f in red_channel_client_disconnect (rcc=0x7f267c069ec0) at red_channel.c:961 #7 0x7f269f75a131 in red_peer_handle_incoming (handler=0x7f267c06a5a0, stream=0x2a3c590) at red_channel.c:150 #8 red_channel_client_receive (rcc=0x7f267c069ec0) at red_channel.c:158 #9 0x7f269f761fbc in handle_channel_events (in_listener=0x7f267c06a5f8, events=) at red_worker.c:9566 #10 0x7f269f776672 in red_worker_main (arg=) at red_worker.c:10813 #11 0x003a83207b31 in start_thread (arg=0x7f26822fc700) at pthread_create.c:305 #12 0x003a82edfd2d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:115 Thread 1 (Thread 0x7f269f72e9c0 (LWP 17232)): #0 0x003a8320e01d in read () at ../sysdeps/unix/syscall-template.S:82 #1 0x7f269f75daaa in receive_data (n=4, in_buf=0x7fffe7a5a02c, fd=18) at red_worker.h:150 #2 read_message (message=0x7fffe7a5a02c, fd=18) at red_worker.h:163 #3 red_dispatcher_disconnect_cursor_peer (rcc=0x7f267c0c0f60) at red_dispatcher.c:157 #4 0x7f269f75c20d in red_client_destroy (client=0x2a35400) at red_channel.c:1189 #5 0x7f269f778335 in reds_client_disconnect (client=0x2a35400) at reds.c:624 ---Type to continue, or q to quit--- #6 0x7f269f75a131 in red_peer_handle_incoming (handler=0x2a35b50, stream=0x2b037d0) at red_channel.c:150 #7 red_channel_client_receive (rcc=0x2a35470) at red_channel.c:158 #8 0x7f269f75b1e8 in red_channel_client_event (fd=, event=, data=) at red_channel.c:774 #9 0x0045e561 in fd_trampoline (chan=, cond=, opaque=) at iohandler.c:97 #10 0x003a84e427ed in g_main_dispatch (context=0x27fc510) at gmain.c:2441 #11 g_main_context_dispatch (context=0x27fc510) at gmain.c:3014 #12 0x004c3da3 in glib_select_poll (err=false, xfds=0x7fffe7a5a2e0, wfds=0x7fffe7a5a260, rfds= 0x7fffe7a5a1e0) at /home/berrange/src/virt/qemu/vl.c:1495 #13 main_loop_wait (nonblocking=) at /home/berrange/src/virt/qemu/vl.c:1541 #14 0x0040fdd1 in main_loop () at /home/berrange/src/virt/qemu/vl.c:1570 #15 main (argc=, argv=, envp=) at /home/berrange/src/virt/qemu/vl.c:3563 (gdb)
Re: [Qemu-devel] [PATCH 3/4] loader: Add rom_add_file_buf for adding roms from a buffer
On Tue, Oct 18, 2011 at 21:17, Jordan Justen wrote:
> On Tue, Oct 18, 2011 at 11:05, Blue Swirl wrote:
>> On Mon, Oct 17, 2011 at 7:16 PM, Jordan Justen
>> wrote:
>>> rom_add_file_buf is similar to rom_add_file, except the rom's
>>> contents are provided in a buffer.
>>>
>>> rom_add_file is modified to call rom_add_file_buf after
>>> reading the rom's contents from the file.
>>>
>>> Signed-off-by: Jordan Justen
>>> ---
>>> hw/loader.c | 71
>>> +++---
>>> hw/loader.h | 5
>>> 2 files changed, 53 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/hw/loader.c b/hw/loader.c
>>> index 5676c18..d1a4a98 100644
>>> --- a/hw/loader.c
>>> +++ b/hw/loader.c
>>> @@ -557,11 +557,11 @@ static void rom_insert(Rom *rom)
>>> QTAILQ_INSERT_TAIL(&roms, rom, next);
>>> }
>>>
>>> -int rom_add_file(const char *file, const char *fw_dir,
>>> - target_phys_addr_t addr, int32_t bootindex)
>>> +int rom_add_file_buf(const char *file, const void *data, size_t size,
>>> + const char *fw_dir,
>>> + target_phys_addr_t addr, int32_t bootindex)
>>> {
>>> Rom *rom;
>>> - int rc, fd = -1;
>>> char devpath[100];
>>>
>>> rom = g_malloc0(sizeof(*rom));
>>> @@ -571,28 +571,16 @@ int rom_add_file(const char *file, const char *fw_dir,
>>> rom->path = g_strdup(file);
>>> }
>>>
>>> - fd = open(rom->path, O_RDONLY | O_BINARY);
>>> - if (fd == -1) {
>>> - fprintf(stderr, "Could not open option rom '%s': %s\n",
>>> - rom->path, strerror(errno));
>>> - goto err;
>>> - }
>>> -
>>> if (fw_dir) {
>>> rom->fw_dir = g_strdup(fw_dir);
>>> rom->fw_file = g_strdup(file);
>>> }
>>> rom->addr = addr;
>>> - rom->romsize = lseek(fd, 0, SEEK_END);
>>> + rom->romsize = size;
>>> rom->data = g_malloc0(rom->romsize);
>>> - lseek(fd, 0, SEEK_SET);
>>> - rc = read(fd, rom->data, rom->romsize);
>>> - if (rc != rom->romsize) {
>>> - fprintf(stderr, "rom: file %-20s: read error: rc=%d (expected
>>> %zd)\n",
>>> - rom->name, rc, rom->romsize);
>>> - goto err;
>>> - }
>>> - close(fd);
>>> +
>>> + memcpy(rom->data, data, rom->romsize);
>>
>> This is not optimal, instead the data should be used directly. That
>> way also mmap()ed, deduplicated ROM files are possible.
>
> In my 4th patch I use a buffer from a memory region via
> memory_region_get_ram_ptr. Comments for memory_region_get_ram_ptr say
> 'Use with care'.
>
> So, would the best thing be for me to allocate a new buffer in my 4th
> patch, do the memcpy there, and then use the pointer directly here?
No, instead of memcpy just do
rom->data = data;
Then also the corresponding g_free(data) below should be removed.
The line g_free(rom->data) in the error path would be a problem for
the future mmap() case though. Should be solvable with with some
refactoring then, we'd need to be able to munmap() anyway.
> Thanks,
>
> -Jordan
>
>>
>>> +
>>> rom_insert(rom);
>>> if (rom->fw_file && fw_cfg) {
>>> const char *basename;
>>> @@ -614,14 +602,51 @@ int rom_add_file(const char *file, const char *fw_dir,
>>>
>>> add_boot_device_path(bootindex, NULL, devpath);
>>> return 0;
>>> +}
>>> +
>>> +int rom_add_file(const char *file, const char *fw_dir,
>>> + target_phys_addr_t addr, int32_t bootindex)
>>> +{
>>> + char *filename;
>>> + void *data = NULL;
>>> + size_t size;
>>> + int rc, fd = -1;
>>> +
>>> + filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, file);
>>> + if (filename == NULL) {
>>> + filename = g_strdup(file);
>>> + }
>>> +
>>> + fd = open(filename, O_RDONLY | O_BINARY);
>>> + if (fd == -1) {
>>> + fprintf(stderr, "Could not open option rom '%s': %s\n",
>>> + filename, strerror(errno));
>>> + goto err;
>>> + }
>>> +
>>> + size = lseek(fd, 0, SEEK_END);
>>> + data = g_malloc0(size);
>>> + lseek(fd, 0, SEEK_SET);
>>> + rc = read(fd, data, size);
>>
>> It should be easy to replace this with mmap(), maybe later.
>>
>>> + if (rc != size) {
>>> + fprintf(stderr, "rom: file %-20s: read error: rc=%d (expected
>>> %zd)\n",
>>> + filename, rc, size);
>>> + goto err;
>>> + }
>>> + close(fd);
>>> +
>>> + rc = rom_add_file_buf(filename, data, size, fw_dir, addr, bootindex);
>>> + if (rc != 0) {
>>> + goto err;
>>> + }
>>> +
>>> + g_free(data);
>>> + return 0;
>>>
>>> err:
>>> if (fd != -1)
>>> close(fd);
>>> - g_free(rom->data);
>>> - g_free(rom->path);
>>> - g_free(rom->name);
>>> - g_free(rom);
>>> + g_free(data);
>>> return -1;
>>> }
>>>
>>> diff --git a/hw/loader.h b/hw/loader.h
>>> index fc6bdff..9efe64a 100644
>>> --- a/hw/loader.h
>>> +++ b/hw/loader.h
>>> @@ -21,6 +21,9 @@ void pstrcpy_targphys(const char *name,
>>>
Re: [Qemu-devel] [PATCH 06/21] target-sparc: Extract common code for floating-point operations.
On Tue, Oct 18, 2011 at 22:21, Richard Henderson wrote: > On 10/18/2011 01:24 PM, Blue Swirl wrote: >>> #ifdef TARGET_SPARC64 >>> -float64 helper_fabsd(CPUState *env, float64 src) >>> +float64 helper_fabsd(float64 src) >> >> This probably should go to previous patch. > > Sure. > >>> +/* Turn off the stupid always-inline hack in osdep.h. This gets in the >>> + way of the callback mechanisms we use in this file, generating warnings >>> + for always-inline functions called indirectly. */ >>> +#define always_inline inline >> >> It would be better to just delete the offending (or all) inlines. > > I certainly would like to delete the offending hack in osdep.h. > > The inline markers themselves are generated by def-helper.h, and are required > so that we don't wind up with a corresponding number of defined-but-not-used > errors from the helper.h definitions. > > I really didn't know any one way to handle this situation that would be > immediately acceptable to everyone. I assumed limiting the change to > the sparc front-end would minimize the pushback. It should also be possible to add non-inlined wrapper functions to inlined functions. >>> +static void gen_ne_fop_FF(DisasContext *dc, int rd, int rs, >> >> 'ne' is for no exception? How about noexcp or something? > > no-exception when it's first introduced. Then after patch 11 it would > become no-env. Preferences for the intermediate stage? Nevermind then.
Re: [Qemu-devel] [PATCH 00/26] AREG0 conversion
On Wed, Oct 19, 2011 at 17:25, Richard Henderson wrote:
> On 10/09/2011 12:20 PM, Blue Swirl wrote:
>>> I didn't bother to attach the patches, if someone wants to try, the
>>> patch set can be found here:
>>> git://repo.or.cz/qemu/blueswirl.git
>>> http://repo.or.cz/r/qemu/blueswirl.git
>>
>> I pushed the patch set to repo.or.cz so if someone wants to comment or
>> test, they are there.
>>
>> It's mostly the same stuff as before, though I split int_helper.c to
>> int32_helper.c and int64_helper.c since they have nothing in common
>> and extracted TCG iargs/oargs changes.
>>
>>> Blue Swirl (26):
>>> Document softmmu templates
>>> softmmu_header: pass CPUState to tlb_fill
>>> Move GETPC from dyngen-exec.h to exec-all.h
>
> I don't see these three in the repo.
Because I applied them earlier...
>>> Sparc: fix coding style
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: split helper.c
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: move trivial functions from op_helper.c
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: avoid AREG0 for raise_exception and helper_debug
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: fix coding style
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: split FPU and VIS op helpers
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: avoid AREG0 for float and VIS ops
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: split lazy condition code handling op helpers
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: avoid AREG0 for lazy condition code helpers
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: split CWP and PSTATE op helpers
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: avoid AREG0 for CWP and PSTATE helpers
>
> Reviewed-by: Richard Henderson
> An especially nice cleanup too, avoiding all of the saved_env frobbing.
>
>>> Sparc: avoid AREG0 for softint op helpers and Leon cache control
>
> This one loses do_modify_softint in the move. Which gets re-instated
> in your following "convert int_helper to trace framework" patch. But
> in the meantime the series is non-bisectable.
Nice catch, will fix. I think I'll apply the patches before this one
now to shorten the series a bit.
>>> Sparc: avoid AREG0 for division op helpers
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: fix coding style in helper.c
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: split MMU helpers
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: convert mmu_helper to trace framework
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: convert int_helper to trace framework
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: convert win_helper to trace framework
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: split load and store op helpers
>
> Reviewed-by: Richard Henderson
>
>>> TCG: add 5 arg helpers to def-helper.h
>
> Reviewed-by: Richard Henderson
>
>>> Sparc: avoid AREG0 for memory access helpers
>
>> +#define WRAP_LD(rettype, fn) \
>> + rettype cpu_ ## fn (CPUState *env1, target_ulong addr) \
>> + { \
>> + CPUState *saved_env; \
>> + rettype ret; \
>> + \
>> + saved_env = env; \
>> + env = env1; \
>> + ret = fn(addr); \
>> + env = saved_env; \
>> + return ret; \
>> + }
>
> I don't think this actually works in the fault case. In particular GETPC
> will return an incorrect address. OTOH, I suppose we already have this
> problem from the other ldst helpers, e.g. ld_asi, which is where these new
> routines are going to be called from. So this doesn't really change the
> state of affairs much. I have no good ideas for solving this problem.
OK, maybe it's better to leave this and the 5 arg patch from 1.0.
> Reviewed-by: Richard Henderson
>
>>> softmmu templates: optionally pass CPUState to memory access
>>> functions
>>> Sparc: avoid AREG0 wrappers for memory access helpers
>
> Both look ok. I'm certainly not fond of the intermediate state. If we
> convert target-sparc and tcg-i386, we should convert all of them, and
> not leave that intermediate state for long.
>
> Something that's clearly not going to happen for the 1.0 release.
Fully agree. Thanks for the review.
Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus "ICC" to connect APIC
On Wed, Oct 19, 2011 at 01:55, wrote:
> From: Liu Ping Fan
>
> Introduce a new structure CPUS as the controller of ICC (INTERRUPT
> CONTROLLER COMMUNICATIONS), and new bus "ICC" to hold APIC,instead
> of sysbus. So we can support APIC hot-plug feature.
Is this ICC bus or APIC hot plugging documented somewhere?
> Signed-off-by: liu ping fan
> ---
> Makefile.target | 1 +
> hw/apic.c | 25 +++
> hw/apic.h | 1 +
> hw/icc_bus.c | 91
> +++
> hw/icc_bus.h | 56 ++
> hw/pc.c | 11 --
> 6 files changed, 174 insertions(+), 11 deletions(-)
> create mode 100644 hw/icc_bus.c
> create mode 100644 hw/icc_bus.h
>
> diff --git a/Makefile.target b/Makefile.target
> index 9011f28..5607c6d 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -241,6 +241,7 @@ obj-i386-$(CONFIG_KVM) += kvmclock.o
> obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
> obj-i386-y += testdev.o
> obj-i386-y += acpi.o acpi_piix4.o
> +obj-i386-y += icc_bus.o
>
> obj-i386-y += pcspk.o i8254.o
> obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o
> diff --git a/hw/apic.c b/hw/apic.c
> index 69d6ac5..00d2297 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -21,9 +21,10 @@
> #include "ioapic.h"
> #include "qemu-timer.h"
> #include "host-utils.h"
> -#include "sysbus.h"
> +#include "icc_bus.h"
> #include "trace.h"
> #include "kvm.h"
> +#include "exec-memory.h"
>
> /* APIC Local Vector Table */
> #define APIC_LVT_TIMER 0
> @@ -80,7 +81,7 @@
> typedef struct APICState APICState;
>
> struct APICState {
> - SysBusDevice busdev;
> + ICCBusDevice busdev;
> MemoryRegion io_memory;
> void *cpu_env;
> uint32_t apicbase;
> @@ -1104,9 +1105,20 @@ static const MemoryRegionOps apic_io_ops = {
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> -static int apic_init1(SysBusDevice *dev)
> +/**/
> +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base)
> {
> - APICState *s = FROM_SYSBUS(APICState, dev);
> + APICState *s = DO_UPCAST(APICState, busdev.qdev, dev);
> +
> + memory_region_add_subregion(get_system_memory(),
> + base,
> + &s->io_memory);
> + return 0;
> +}
> +
> +static int apic_init1(ICCBusDevice *dev)
> +{
> + APICState *s = DO_UPCAST(APICState, busdev, dev);
> static int last_apic_idx;
>
> if (last_apic_idx >= MAX_APICS) {
> @@ -1114,7 +1126,6 @@ static int apic_init1(SysBusDevice *dev)
> }
> memory_region_init_io(&s->io_memory, &apic_io_ops, s, "apic",
> MSI_ADDR_SIZE);
> - sysbus_init_mmio_region(dev, &s->io_memory);
>
> s->timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
> s->idx = last_apic_idx++;
> @@ -1122,7 +1133,7 @@ static int apic_init1(SysBusDevice *dev)
> return 0;
> }
>
> -static SysBusDeviceInfo apic_info = {
> +static ICCBusDeviceInfo apic_info = {
> .init = apic_init1,
> .qdev.name = "apic",
> .qdev.size = sizeof(APICState),
> @@ -1138,7 +1149,7 @@ static SysBusDeviceInfo apic_info = {
>
> static void apic_register_devices(void)
> {
> - sysbus_register_withprop(&apic_info);
> + iccbus_register_devinfo(&apic_info);
> }
>
> device_init(apic_register_devices)
> diff --git a/hw/apic.h b/hw/apic.h
> index c857d52..e2c0af5 100644
> --- a/hw/apic.h
> +++ b/hw/apic.h
> @@ -20,6 +20,7 @@ void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
> uint8_t cpu_get_apic_tpr(DeviceState *s);
> void apic_init_reset(DeviceState *s);
> void apic_sipi(DeviceState *s);
> +int apic_mmio_map(DeviceState *dev, target_phys_addr_t base);
>
> /* pc.c */
> int cpu_is_bsp(CPUState *env);
> diff --git a/hw/icc_bus.c b/hw/icc_bus.c
> new file mode 100644
> index 000..61a408e
> --- /dev/null
> +++ b/hw/icc_bus.c
> @@ -0,0 +1,91 @@
> +/* icc_bus.c
> + * emulate x86 ICC(INTERRUPT CONTROLLER COMMUNICATIONS) bus
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see
>
Re: [Qemu-devel] [PATCH] gdbstub: Fix memory leak
On Thu, Oct 20, 2011 at 08:10, Stuart Brady wrote: > On Wed, Oct 19, 2011 at 01:59:04AM +0100, Stuart Brady wrote: >> On Tue, Oct 18, 2011 at 06:18:11PM +, Blue Swirl wrote: >> >> > Cool. Please include the spatch with the commit message. >> >> Thanks, will do! > > Okay, submitted. > > This is the first time I've used git send-email, so let me know if I've > missed anything. The patches are OK, though when sending multiple patches, a cover letter (--cover-letter) is appreciated.
Re: [Qemu-devel] [PATCH v2 1/4] Add basic version of bridge helper
On Fri, Oct 21, 2011 at 15:07, Corey Bryant wrote:
> This patch adds a helper that can be used to create a tap device attached to
> a bridge device. Since this helper is minimal in what it does, it can be
> given CAP_NET_ADMIN which allows qemu to avoid running as root while still
> satisfying the majority of what users tend to want to do with tap devices.
>
> The way this all works is that qemu launches this helper passing a bridge
> name and the name of an inherited file descriptor. The descriptor is one
> end of a socketpair() of domain sockets. This domain socket is used to
> transmit a file descriptor of the opened tap device from the helper to qemu.
>
> The helper can then exit and let qemu use the tap device.
>
> Signed-off-by: Anthony Liguori
> Signed-off-by: Richa Marwaha
> Signed-off-by: Corey Bryant
> ---
> Makefile | 12 +++-
> configure | 1 +
> qemu-bridge-helper.c | 205
> ++
> 3 files changed, 216 insertions(+), 2 deletions(-)
> create mode 100644 qemu-bridge-helper.c
>
> diff --git a/Makefile b/Makefile
> index f63fc02..d9b447e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -35,6 +35,8 @@ $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw)
>
> LIBS+=-lz $(LIBS_TOOLS)
>
> +HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
> +
> ifdef BUILD_DOCS
> DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8
> QMP/qmp-commands.txt
> else
> @@ -75,7 +77,7 @@ defconfig:
>
> -include config-all-devices.mak
>
> -build-all: $(DOCS) $(TOOLS) recurse-all
> +build-all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all
>
> config-host.h: config-host.h-timestamp
> config-host.h-timestamp: config-host.mak
> @@ -153,6 +155,8 @@ qemu-img$(EXESUF): qemu-img.o $(tools-obj-y)
> qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y)
> qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y)
>
> +qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
> +
> qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
> $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@," GEN
> $@")
>
> @@ -221,7 +225,7 @@ clean:
> # avoid old build problems by removing potentially incorrect old files
> rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h
> gen-op-arm.h
> rm -f qemu-options.def
> - rm -f *.o *.d *.a *.lo $(TOOLS) qemu-ga TAGS cscope.* *.pod *~ */*~
> + rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.*
> *.pod *~ */*~
> rm -Rf .libs
> rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d
> net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o
> qga/*.d
> rm -f qemu-img-cmds.h
> @@ -289,6 +293,10 @@ install: all $(if $(BUILD_DOCS),install-doc)
> install-sysconfig
> ifneq ($(TOOLS),)
> $(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
> endif
> +ifneq ($(HELPERS-y),)
> + $(INSTALL_DIR) "$(DESTDIR)$(libexecdir)"
> + $(INSTALL_PROG) $(STRIP_OPT) $(HELPERS-y) "$(DESTDIR)$(libexecdir)"
> +endif
> ifneq ($(BLOBS),)
> $(INSTALL_DIR) "$(DESTDIR)$(datadir)"
> set -e; for x in $(BLOBS); do \
> diff --git a/configure b/configure
> index 4f87e0a..6c8b659 100755
> --- a/configure
> +++ b/configure
> @@ -2768,6 +2768,7 @@ echo "datadir=$datadir" >> $config_host_mak
> echo "sysconfdir=$sysconfdir" >> $config_host_mak
> echo "docdir=$docdir" >> $config_host_mak
> echo "confdir=$confdir" >> $config_host_mak
> +echo "libexecdir=\${prefix}/libexec" >> $config_host_mak
>
> case "$cpu" in
>
> i386|x86_64|alpha|cris|hppa|ia64|lm32|m68k|microblaze|mips|mips64|ppc|ppc64|s390|s390x|sparc|sparc64|unicore32)
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> new file mode 100644
> index 000..2ce82fb
> --- /dev/null
> +++ b/qemu-bridge-helper.c
> @@ -0,0 +1,205 @@
> +/*
> + * QEMU Bridge Helper
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + * Anthony Liguori
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "config-host.h"
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include
> +
> +#include "net/tap-linux.h"
> +
> +static int has_vnet_hdr(int fd)
> +{
> + unsigned int features = 0;
> + struct ifreq ifreq;
> +
> + if (ioctl(fd, TUNGETFEATURES, &features) == -1) {
> + return -errno;
> + }
> +
> + if (!(features & IFF_VNET_HDR)) {
> + return -ENOTSUP;
> + }
> +
> + if (ioctl(fd, TUNGETIFF, &ifreq) != -1 || errno != EBADFD) {
> + return -ENOTSUP;
> + }
> +
> + return 1;
> +}
> +
> +static void prep_ifreq(struct ifreq *ifr, const char *ifname)
> +{
> + memset(ifr, 0, sizeof(*ifr));
> + snprintf(ifr->ifr_name, IFNAMSIZ, "%s", ifname);
> +}
> +
> +static int send_fd
Re: [Qemu-devel] [PATCH v2 2/4] Add access control support to qemu bridge helper
On Fri, Oct 21, 2011 at 15:07, Corey Bryant wrote:
> We go to great lengths to restrict ourselves to just cap_net_admin as an OS
> enforced security mechanism. However, we further restrict what we allow users
> to do to simply adding a tap device to a bridge interface by virtue of the
> fact
> that this is the only functionality we expose.
>
> This is not good enough though. An administrator is likely to want to
> restrict
> the bridges that an unprivileged user can access, in particular, to restrict
> an unprivileged user from putting a guest on what should be isolated networks.
>
> This patch implements an ACL mechanism that is enforced by qemu-bridge-helper.
> The ACLs are fairly simple whitelist/blacklist mechanisms with a wildcard of
> 'all'. All users are blacklisted by default, and deny takes precedence over
> allow.
>
> An interesting feature of this ACL mechanism is that you can include external
> ACL files. The main reason to support this is so that you can set different
> file system permissions on those external ACL files. This allows an
> administrator to implement rather sophisicated ACL policies based on
> user/group
sophisticated
> policies via the file system.
>
> As an example:
>
> /etc/qemu/bridge.conf root:qemu 0640
>
> allow br0
> include /etc/qemu/alice.conf
> include /etc/qemu/bob.conf
> include /etc/qemu/charlie.conf
>
> /etc/qemu/alice.conf root:alice 0640
> allow br1
>
> /etc/qemu/bob.conf root:bob 0640
> allow br2
>
> /etc/qemu/charlie.conf root:charlie 0640
> deny all
I think syntax 'include /etc/qemu/user.d/*.conf' or 'includedir
/etc/qemu/user.d' could be also useful.
> This ACL pattern allows any user in the qemu group to get a tap device
> connected to br0 (which is bridged to the physical network).
>
> Users in the alice group can additionally get a tap device connected to br1.
> This allows br1 to act as a private bridge for the alice group.
>
> Users in the bob group can additionally get a tap device connected to br2.
> This allows br2 to act as a private bridge for the bob group.
>
> Users in the charlie group cannot get a tap device connected to any bridge.
>
> Under no circumstance can the bob group get access to br1 or can the alice
> group get access to br2. And under no cicumstance can the charlie group
> get access to any bridge.
>
> Signed-off-by: Anthony Liguori
> Signed-off-by: Richa Marwaha
> Signed-off-by: Corey Bryant
> ---
> qemu-bridge-helper.c | 141
> ++
> 1 files changed, 141 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> index 2ce82fb..db257d5 100644
> --- a/qemu-bridge-helper.c
> +++ b/qemu-bridge-helper.c
> @@ -33,6 +33,105 @@
>
> #include "net/tap-linux.h"
>
> +#define MAX_ACLS (128)
If all users (or groups) in the system have an ACL, this number could
be way too low. Please use a list instead.
> +#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> +
> +enum {
> + ACL_ALLOW = 0,
> + ACL_ALLOW_ALL,
> + ACL_DENY,
> + ACL_DENY_ALL,
> +};
> +
> +typedef struct ACLRule {
> + int type;
> + char iface[IFNAMSIZ];
> +} ACLRule;
> +
> +static int parse_acl_file(const char *filename, ACLRule *acls, int
> *pacl_count)
> +{
> + int acl_count = *pacl_count;
> + FILE *f;
> + char line[4096];
> +
> + f = fopen(filename, "r");
> + if (f == NULL) {
> + return -1;
> + }
> +
> + while (acl_count != MAX_ACLS &&
> + fgets(line, sizeof(line), f) != NULL) {
> + char *ptr = line;
> + char *cmd, *arg, *argend;
> +
> + while (isspace(*ptr)) {
> + ptr++;
> + }
> +
> + /* skip comments and empty lines */
> + if (*ptr == '#' || *ptr == 0) {
> + continue;
> + }
> +
> + cmd = ptr;
> + arg = strchr(cmd, ' ');
> + if (arg == NULL) {
> + arg = strchr(cmd, '\t');
> + }
> +
> + if (arg == NULL) {
> + fprintf(stderr, "Invalid config line:\n %s\n", line);
> + fclose(f);
> + errno = EINVAL;
> + return -1;
> + }
> +
> + *arg = 0;
> + arg++;
> + while (isspace(*arg)) {
> + arg++;
> + }
> +
> + argend = arg + strlen(arg);
> + while (arg != argend && isspace(*(argend - 1))) {
> + argend--;
> + }
These while loops to skip spaces are repeated, but the comment
skipping part is not, so it is not possible to have comments after
rules or split rules to several lines. I'd add a simple state variable
to track at which stage we are in parsing instead.
> + *argend = 0;
> +
> + if (strcmp(cmd, "deny") == 0) {
> + if (strcmp(arg, "all") == 0) {
> + acls[acl_count].type = ACL_DENY_ALL;
> + } else {
> + acls[acl_count].type = ACL_DENY;
> + snprintf(acls[acl_count].iface, IFNAMSIZ, "%s", arg);
> +
Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
On Fri, Oct 21, 2011 at 15:07, Corey Bryant wrote:
> The ideal way to use qemu-bridge-helper is to give it an fscap of using:
>
> setcap cap_net_admin=ep qemu-bridge-helper
>
> Unfortunately, most distros still do not have a mechanism to package files
> with fscaps applied. This means they'll have to SUID the qemu-bridge-helper
> binary.
>
> To improve security, use libcap to reduce our capability set to just
> cap_net_admin, then reduce privileges down to the calling user. This is
> hopefully close to equivalent to fscap support from a security perspective.
>
> Signed-off-by: Anthony Liguori
> Signed-off-by: Richa Marwaha
> Signed-off-by: Corey Bryant
> ---
> configure | 34 ++
> qemu-bridge-helper.c | 39 +++
> 2 files changed, 73 insertions(+), 0 deletions(-)
>
> diff --git a/configure b/configure
> index 6c8b659..fed66b0 100755
> --- a/configure
> +++ b/configure
> @@ -128,6 +128,7 @@ vnc_thread="no"
> xen=""
> xen_ctrl_version=""
> linux_aio=""
> +cap=""
> attr=""
> xfs=""
>
> @@ -653,6 +654,10 @@ for opt do
> ;;
> --enable-kvm) kvm="yes"
> ;;
> + --disable-cap) cap="no"
> + ;;
> + --enable-cap) cap="yes"
> + ;;
> --disable-spice) spice="no"
> ;;
> --enable-spice) spice="yes"
> @@ -1032,6 +1037,8 @@ echo " --disable-vde disable support for
> vde network"
> echo " --enable-vde enable support for vde network"
> echo " --disable-linux-aio disable Linux AIO support"
> echo " --enable-linux-aio enable Linux AIO support"
> +echo " --disable-cap disable libcap-ng support"
> +echo " --enable-cap enable libcap-ng support"
> echo " --disable-attr disables attr and xattr support"
> echo " --enable-attr enable attr and xattr support"
> echo " --disable-blobs disable installing provided firmware blobs"
> @@ -1638,6 +1645,29 @@ EOF
> fi
>
> ##
> +# libcap-ng library probe
> +if test "$cap" != "no" ; then
> + cap_libs="-lcap-ng"
> + cat > $TMPC << EOF
> +#include
> +int main(void)
> +{
> + capng_capability_to_name(CAPNG_EFFECTIVE);
> + return 0;
> +}
> +EOF
> + if compile_prog "" "$cap_libs" ; then
> + cap=yes
> + libs_tools="$cap_libs $libs_tools"
> + else
> + if test "$cap" = "yes" ; then
> + feature_not_found "cap"
> + fi
> + cap=no
> + fi
> +fi
> +
> +##
> # Sound support libraries probe
>
> audio_drv_probe()
> @@ -2735,6 +2765,7 @@ echo "fdatasync $fdatasync"
> echo "madvise $madvise"
> echo "posix_madvise $posix_madvise"
> echo "uuid support $uuid"
> +echo "libcap-ng support $cap"
> echo "vhost-net support $vhost_net"
> echo "Trace backend $trace_backend"
> echo "Trace output file $trace_file-"
> @@ -2846,6 +2877,9 @@ fi
> if test "$vde" = "yes" ; then
> echo "CONFIG_VDE=y" >> $config_host_mak
> fi
> +if test "$cap" = "yes" ; then
> + echo "CONFIG_LIBCAP=y" >> $config_host_mak
> +fi
> for card in $audio_card_list; do
> def=CONFIG_`echo $card | tr '[:lower:]' '[:upper:]'`
> echo "$def=y" >> $config_host_mak
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> index db257d5..b1562eb 100644
> --- a/qemu-bridge-helper.c
> +++ b/qemu-bridge-helper.c
> @@ -33,6 +33,10 @@
>
> #include "net/tap-linux.h"
>
> +#ifdef CONFIG_LIBCAP
> +#include
> +#endif
> +
> #define MAX_ACLS (128)
> #define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>
> @@ -185,6 +189,27 @@ static int send_fd(int c, int fd)
> return sendmsg(c, &msg, 0);
> }
>
> +#ifdef CONFIG_LIBCAP
> +static int drop_privileges(void)
> +{
> + /* clear all capabilities */
> + capng_clear(CAPNG_SELECT_BOTH);
> +
> + if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
> + CAP_NET_ADMIN) < 0) {
> + return -1;
> + }
> +
> + /* change to calling user's real uid and gid, retaining supplemental
> + * groups and CAP_NET_ADMIN */
> + if (capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING)) {
> + return -1;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> int main(int argc, char **argv)
> {
> struct ifreq ifr;
> @@ -198,6 +223,20 @@ int main(int argc, char **argv)
> int acl_count = 0;
> int i, access_allowed, access_denied;
>
> + /* if we're run from an suid binary, immediately drop privileges
> preserving
> + * cap_net_admin -- exit immediately if libcap not configured */
> + if (geteuid() == 0 && getuid() != geteuid()) {
> +#ifdef CONFIG_LIBCAP
> + if (drop_privileges() == -1) {
> + fprintf(stderr, "failed to drop privileges\n");
> + return 1;
> + }
> +#else
> + fprintf(stderr, "failed to drop privileges\n");
This makes the tool useless without CONFIG_LIBCAP. Wouldn't it be
possible to use setfsuid() instead for Linux?
Some fork+setui
Re: [Qemu-devel] [PATCH 5/5] Convert remaining calls to g_malloc(sizeof(type)) to g_new()
On Fri, Oct 21, 2011 at 00:26, Stuart Brady wrote: > On Thu, Oct 20, 2011 at 11:05:33AM +0200, Paolo Bonzini wrote: >> On 10/20/2011 10:03 AM, Stuart Brady wrote: >> >Coccinelle did not match these when matching assignments involving an >> >expression corresponding to the type used for determining the size to >> >allocate. >> >> They all look okay, perhaps the include path you passed to >> Coccinelle is incomplete? > > Ah, good point! I'm not sure what include dirs are needed, though... > anyone have any advice? > > Blue Swirl, I gather you're one of the few other people to have used > Coccinelle with Qemu's source... Yes, but I can't find the commands from command line history, sorry. Note to self: always make scripts from complex command invocations and remember to use the Internet mirrors for storage. >> Reviewed-by: Paolo Bonzini >> >> but would be nicer if you fixed the problem. > > Agreed... I'm new to Coccinelle, but I'll experiment and see if I can > produce a better patch. After applying my patch series, it seems there > are still some additional calls that need converting, so I can readily > believe that the semantic patches I've used can be improved! > > Cheers, > -- > Stuart > >
Re: [Qemu-devel] build with trace enabled is broken by the commit c572f23a3e7180dbeab5e86583e43ea2afed6271 hw/9pfs: Introduce tracing for 9p pdu handlers
On 2011-10-21 17:10, Aneesh Kumar K.V wrote: > On Thu, 20 Oct 2011 23:20:54 +0400, Max Filippov wrote: >> Hi. >> >> Current git head build with trace enabled is broken by the commit >> c572f23a3e7180dbeab5e86583e43ea2afed6271 hw/9pfs: Introduce tracing for 9p >> pdu handlers. >> Error messages: >> >> In file included from trace.c:2:0: >> trace.h: In function ‘trace_v9fs_attach’: >> trace.h:2850:9: error: too many arguments for format >> [-Werror=format-extra-args] >> trace.h: In function ‘trace_v9fs_wstat’: >> trace.h:3039:9: error: too many arguments for format >> [-Werror=format-extra-args] >> trace.h: In function ‘trace_v9fs_mkdir’: >> trace.h:3088:9: error: too many arguments for format >> [-Werror=format-extra-args] >> trace.h: In function ‘trace_v9fs_mkdir_return’: >> trace.h:3095:9: error: too many arguments for format >> [-Werror=format-extra-args] >> cc1: all warnings being treated as errors >> >> Prototypes in the trace-events do not match format strings, e.g. >> >> v9fs_attach(uint16_t tag, uint8_t id, int32_t fid, int32_t afid, char* >> uname, char* aname) "tag %d id %d fid %d afid %d aname %s" >> >> The following patch fixes it, but I'm not sure the format lines are >> appropriate. > > Can you send the patch with signed-off-by: I will add it in the next > pull request. There are more breakages with tracing enabled: CClibhw64/9pfs/virtio-9p.o cc1: warnings being treated as errors /data/qemu/hw/9pfs/virtio-9p.c: In function ‘v9fs_create’: /data/qemu/hw/9pfs/virtio-9p.c:2225:9: error: ‘iounit’ may be used uninitialized in this function /data/qemu/hw/9pfs/virtio-9p.c: In function ‘v9fs_readdir’: /data/qemu/hw/9pfs/virtio-9p.c:2063:13: error: ‘count’ may be used uninitialized in this function /data/qemu/hw/9pfs/virtio-9p.c: In function ‘v9fs_xattrwalk’: /data/qemu/hw/9pfs/virtio-9p.c:3103:13: error: ‘size’ may be used uninitialized in this function /data/qemu/hw/9pfs/virtio-9p.c: In function ‘v9fs_lcreate’: /data/qemu/hw/9pfs/virtio-9p.c:1730:13: error: ‘iounit’ may be used uninitialized in this function /data/qemu/hw/9pfs/virtio-9p.c: In function ‘v9fs_open’: /data/qemu/hw/9pfs/virtio-9p.c:1651:9: error: ‘iounit’ may be used uninitialized in this function Not sure what the undefined variables are supposed to contain for the trace output, so I refrain from writing any patch. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/2] block: Write out internal caches even with cache=unsafe
On 10/22/2011 05:07 PM, Alexander Graf wrote: On 21.10.2011, at 11:44, Paolo Bonzini wrote: On 10/21/2011 07:08 PM, Kevin Wolf wrote: Avi complained that not even writing out qcow2's cache on bdrv_flush() made cache=unsafe too unsafe to be useful. He's got a point. Why? cache=unsafe is explicitly allowing to s/data/manure/ on crash. Exactly, but not on kill. By not flushing internal caches you're almost guaranteed to get an inconsistent qcow2 image. This should be covered already by termsig_handler. bdrv_close_all closes all block devices, and qcow2_close does flush the caches. SIGKILL doesn't give any guarantee of course but it does not in general, even without cache=unsafe; you might get a SIGKILL "a moment before" a bdrv_flush even without cache=unsafe, and get unclean qcow2 metadata. By not flushing internal caches you're almost guaranteed to get an inconsistent qcow2 image. Of course the inconsistencies with cache=unsafe will be massive if you don't have a clean exit, but that's expected. If in some cases you want a clean exit, but right now you don't, the place to fix those cases doesn't seem to be the block layer, but the main loop. Also, 1) why should cache=unsafe differentiate an OS that sends a flush from one that doesn't (e.g. MS-DOS), from the point of view of image metadata? 2) why should the guest actually send a flush if cache=unsafe? Currently if (flags & BDRV_O_CACHE_WB) bs->enable_write_cache = 1; covers cache=unsafe. However, in the end write cache enable means "do I need to flush data", and the answer is "no" when cache=unsafe, because the flushes are useless and guests are free to reorder requests. Perhaps what you want is to make qcow2 caches writethrough in cache=unsafe mode, so that at least a try is made to write the metadata (even though the underlying raw protocol won't flush it)? I'm not sure that is particularly useful, but maybe it can help me understanding the benefit of this change. Paolo
[Qemu-devel] [PATCH 2/2] libcacard/vscclient: fix error paths for socket creation
Signed-off-by: Alon Levy
---
libcacard/vscclient.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 2191f60..e317a25 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -357,6 +357,7 @@ connect_to_qemu(
if (sock < 0) {
/* Error */
fprintf(stderr, "Error opening socket!\n");
+return -1;
}
memset(&hints, 0, sizeof(struct addrinfo));
@@ -370,13 +371,13 @@ connect_to_qemu(
if (ret != 0) {
/* Error */
fprintf(stderr, "getaddrinfo failed\n");
-return 5;
+return -1;
}
if (connect(sock, server->ai_addr, server->ai_addrlen) < 0) {
/* Error */
fprintf(stderr, "Could not connect\n");
-return 5;
+return -1;
}
if (verbose) {
printf("Connected (sizeof Header=%zd)!\n", sizeof(VSCMsgHeader));
@@ -505,6 +506,10 @@ main(
qemu_host = strdup(argv[argc - 2]);
qemu_port = strdup(argv[argc - 1]);
sock = connect_to_qemu(qemu_host, qemu_port);
+if (sock == -1) {
+fprintf(stderr, "error opening socket, exiting.\n");
+exit(5);
+}
qemu_mutex_init(&write_lock);
qemu_mutex_init(&pending_reader_lock);
--
1.7.6.4
[Qemu-devel] [PATCH 0/2] libcacard coverity found fixes
Two fixes, the first means memory for a vcard applet was never freed, the second fixes vscclient handling of errors when opening it's socket. Alon Levy (2): libcacard/cac: fix typo in cac_delete_pki_applet_private libcacard/vscclient: fix error paths for socket creation libcacard/cac.c |3 ++- libcacard/vscclient.c |9 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) -- 1.7.6.4
[Qemu-devel] [PATCH 1/2] libcacard/cac: fix typo in cac_delete_pki_applet_private
Signed-off-by: Alon Levy
---
libcacard/cac.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/libcacard/cac.c b/libcacard/cac.c
index f4b0b1b..927a4ca 100644
--- a/libcacard/cac.c
+++ b/libcacard/cac.c
@@ -266,7 +266,8 @@ static void
cac_delete_pki_applet_private(VCardAppletPrivate *applet_private)
{
CACPKIAppletData *pki_applet_data = NULL;
-if (pki_applet_data == NULL) {
+
+if (applet_private == NULL) {
return;
}
pki_applet_data = &(applet_private->u.pki_data);
--
1.7.6.4
[Qemu-devel] [PATCH] qxl: reset update_surface
update init_qxl_ram to reset update_surface to 0. This fixes one case of breakage when installing an old driver in a vm that had a new driver installed. The newer driver would know about surface creation and would change update_surface to !=0, then a reset would happen, all surfaces are destroyed, then the old driver is initialized and issues an UPDATE_AREA, and spice server aborts on invalid surface. RHBZ: 690427 Signed-off-by: Alon Levy --- hw/qxl.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index 03848ed..632658d 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -330,6 +330,7 @@ static void init_qxl_ram(PCIQXLDevice *d) d->ram->magic = cpu_to_le32(QXL_RAM_MAGIC); d->ram->int_pending = cpu_to_le32(0); d->ram->int_mask= cpu_to_le32(0); +d->ram->update_surface = 0; SPICE_RING_INIT(&d->ram->cmd_ring); SPICE_RING_INIT(&d->ram->cursor_ring); SPICE_RING_INIT(&d->ram->release_ring); -- 1.7.6.4
Re: [Qemu-devel] [PATCH v3] add add-cow file format
On 2011-10-13 0:23, Dong Xu Wang wrote:
Add add-cow file format
Signed-off-by: Dong Xu Wang
---
Makefile.objs |1 +
block.c|2 +-
block.h|1 +
block/add-cow.c| 412
block_int.h|1 +
docs/specs/add-cow.txt | 45 ++
6 files changed, 461 insertions(+), 1 deletions(-)
create mode 100644 block/add-cow.c
create mode 100644 docs/specs/add-cow.txt
diff --git a/Makefile.objs b/Makefile.objs
index c849e51..624c04c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -31,6 +31,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o
vvfat.o
block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
qcow2-cache.o
+block-nested-y += add-cow.o
block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
block-nested-y += qed-check.o
block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
diff --git a/block.c b/block.c
index e865fab..c25241d 100644
--- a/block.c
+++ b/block.c
@@ -106,7 +106,7 @@ int is_windows_drive(const char *filename)
#endif
/* check if the path starts with ":" */
-static int path_has_protocol(const char *path)
+int path_has_protocol(const char *path)
{
#ifdef _WIN32
if (is_windows_drive(path) ||
diff --git a/block.h b/block.h
index 16bfa0a..8b09f12 100644
--- a/block.h
+++ b/block.h
@@ -256,6 +256,7 @@ char *bdrv_snapshot_dump(char *buf, int buf_size,
QEMUSnapshotInfo *sn);
char *get_human_readable_size(char *buf, int buf_size, int64_t size);
int path_is_absolute(const char *path);
+int path_has_protocol(const char *path);
void path_combine(char *dest, int dest_size,
const char *base_path,
const char *filename);
diff --git a/block/add-cow.c b/block/add-cow.c
new file mode 100644
index 000..d2538a2
--- /dev/null
+++ b/block/add-cow.c
@@ -0,0 +1,412 @@
+#include "qemu-common.h"
+#include "block_int.h"
+#include "module.h"
+
+#define ADD_COW_MAGIC (((uint64_t)'A'<< 56) | ((uint64_t)'D'<< 48) | \
+((uint64_t)'D'<< 40) | ((uint64_t)'_'<< 32) | \
+((uint64_t)'C'<< 24) | ((uint64_t)'O'<< 16) | \
+((uint64_t)'W'<< 8) | 0xFF)
+#define ADD_COW_VERSION 1
+
+typedef struct AddCowHeader {
+uint64_t magic;
+uint32_t version;
+char backing_file[1024];
+char image_file[1024];
1024 is a magic number for me. Can we have a meaningful macro?
+uint64_t size;
+} QEMU_PACKED AddCowHeader;
+
+typedef struct BDRVAddCowState {
+char image_file[1024];
+BlockDriverState *image_hd;
+uint8_t *bitmap;
+uint64_t bitmap_size;
+} BDRVAddCowState;
+
+static int add_cow_probe(const uint8_t *buf, int buf_size, const char
*filename)
+{
+const AddCowHeader *header = (const void *)buf;
+
+if (be64_to_cpu(header->magic) == ADD_COW_MAGIC&&
+be32_to_cpu(header->version) == ADD_COW_VERSION) {
+return 100;
+} else {
+return 0;
+}
+}
+
+static int add_cow_open(BlockDriverState *bs, int flags)
+{
+AddCowHeader header;
+int64_t size;
+char image_filename[1024];
+int image_flags;
+BlockDriver *image_drv = NULL;
+int ret;
+BDRVAddCowState *state = (BDRVAddCowState *)(bs->opaque);
+
+ret = bdrv_pread(bs->file, 0,&header, sizeof(header));
+if (ret != sizeof(header)) {
+goto fail;
+}
+
+if (be64_to_cpu(header.magic) != ADD_COW_MAGIC ||
+be32_to_cpu(header.version) != ADD_COW_VERSION) {
+ret = -1;
+goto fail;
+}
+
+size = be64_to_cpu(header.size);
+bs->total_sectors = size / BDRV_SECTOR_SIZE;
+
+QEMU_BUILD_BUG_ON(sizeof(state->image_file) != sizeof(header.image_file));
+pstrcpy(bs->backing_file, sizeof(bs->backing_file),
+header.backing_file);
+pstrcpy(state->image_file, sizeof(state->image_file),
+header.image_file);
+
+state->bitmap_size = ((bs->total_sectors + 7)>> 3);
+state->bitmap = g_malloc0(state->bitmap_size);
+
+ret = bdrv_pread(bs->file, sizeof(header), state->bitmap,
+state->bitmap_size);
+if (ret != state->bitmap_size) {
+goto fail;
+}
+ /* If there is a image_file, must be together with backing_file */
+if (state->image_file[0] != '\0') {
+state->image_hd = bdrv_new("");
+/* Relative to image or working dir, need discussion */
+if (path_has_protocol(state->image_file)) {
+pstrcpy(image_filename, sizeof(image_filename),
+state->image_file);
+} else {
+path_combine(image_filename, sizeof(image_filename),
+ bs->filename, state->image_file);
+}
+
+image_drv = bdrv_find_format("raw");
+image_flags =
+ (flags& (~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKIN
[Qemu-devel] [PATCH RFC v2 0/2] Initial support for Microsoft Hyper-V.
With the following series of patches we are starting to implement some basic Microsoft Hyper-V Enlightenment functionality. This series is mostly about adding support for relaxed timing, spinlock, and virtual apic. For more Hyper-V related information please see: "Hypervisor Functional Specification v2.0: For Windows Server 2008 R2" at http://www.microsoft.com/download/en/details.aspx?displaylang=en&id=18673 Changelog: v2->v1 - remove KVM_CAP_IRQCHIP ifdef, - remove CONFIG_HYPERV config option, - move KVM leaves to new location (0x4100), - cosmetic changes. v0->v1 - move hyper-v parameters under cpu category, - move hyper-v stuff to target-i386 directory, - make CONFIG_HYPERV enabled by default for i386-softmmu and x86_64-softmmu configurations, - rearrange the patches from v0, - set HV_X64_MSR_HYPERCALL, HV_X64_MSR_GUEST_OS_ID, and HV_X64_MSR_APIC_ASSIST_PAGE to 0 on system reset. Vadim Rozenfeld (2): hyper-v: introduce Hyper-V support infrastructure. hyper-v: initialize Hyper-V CPUID leaves. Makefile.target |2 + target-i386/cpuid.c | 14 + target-i386/hyperv.c | 65 target-i386/hyperv.h | 37 + target-i386/kvm.c| 73 - 5 files changed, 189 insertions(+), 2 deletions(-) create mode 100644 target-i386/hyperv.c create mode 100644 target-i386/hyperv.h -- 1.7.4.4
[Qemu-devel] [PATCH 2/2] [PATCH RFC v2 2/2] hyper-v: initialize Hyper-V CPUID leaves.
---
target-i386/kvm.c | 73 +++-
1 files changed, 71 insertions(+), 2 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 82fec8c..c061e3b 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -29,6 +29,7 @@
#include "hw/pc.h"
#include "hw/apic.h"
#include "ioport.h"
+#include "hyperv.h"
//#define DEBUG_KVM
@@ -380,11 +381,16 @@ int kvm_arch_init_vcpu(CPUState *env)
cpuid_i = 0;
/* Paravirtualization CPUIDs */
-memcpy(signature, "KVMKVMKVM\0\0\0", 12);
c = &cpuid_data.entries[cpuid_i++];
memset(c, 0, sizeof(*c));
c->function = KVM_CPUID_SIGNATURE;
-c->eax = 0;
+if (!hyperv_enabled()) {
+memcpy(signature, "KVMKVMKVM\0\0\0", 12);
+c->eax = 0;
+} else {
+memcpy(signature, "Microsoft Hv", 12);
+c->eax = HYPERV_CPUID_MIN;
+}
c->ebx = signature[0];
c->ecx = signature[1];
c->edx = signature[2];
@@ -395,6 +401,54 @@ int kvm_arch_init_vcpu(CPUState *env)
c->eax = env->cpuid_kvm_features &
kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX);
+if (hyperv_enabled()) {
+memcpy(signature, "Hv#1\0\0\0\0\0\0\0\0", 12);
+c->eax = signature[0];
+
+c = &cpuid_data.entries[cpuid_i++];
+memset(c, 0, sizeof(*c));
+c->function = HYPERV_CPUID_VERSION;
+c->eax = 0x1bbc;
+c->ebx = 0x00060001;
+
+c = &cpuid_data.entries[cpuid_i++];
+memset(c, 0, sizeof(*c));
+c->function = HYPERV_CPUID_FEATURES;
+if (hyperv_relaxed_timing_enabled()) {
+c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE;
+}
+if (hyperv_vapic_recommended()) {
+c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE;
+c->eax |= HV_X64_MSR_APIC_ACCESS_AVAILABLE;
+}
+
+c = &cpuid_data.entries[cpuid_i++];
+memset(c, 0, sizeof(*c));
+c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
+if (hyperv_relaxed_timing_enabled()) {
+c->eax |= HV_X64_RELAXED_TIMING_RECOMMENDED;
+}
+if (hyperv_vapic_recommended()) {
+c->eax |= HV_X64_APIC_ACCESS_RECOMMENDED;
+}
+c->ebx = hyperv_get_spinlock_retries();
+
+c = &cpuid_data.entries[cpuid_i++];
+memset(c, 0, sizeof(*c));
+c->function = HYPERV_CPUID_IMPLEMENT_LIMITS;
+c->eax = 0x40;
+c->ebx = 0x40;
+
+c = &cpuid_data.entries[cpuid_i++];
+memset(c, 0, sizeof(*c));
+c->function = KVM_CPUID_SIGNATURE_NEXT;
+memcpy(signature, "KVMKVMKVM\0\0\0", 12);
+c->eax = 0;
+c->ebx = signature[0];
+c->ecx = signature[1];
+c->edx = signature[2];
+}
+
has_msr_async_pf_en = c->eax & (1 << KVM_FEATURE_ASYNC_PF);
cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
@@ -953,6 +1007,13 @@ static int kvm_put_msrs(CPUState *env, int level)
kvm_msr_entry_set(&msrs[n++], MSR_KVM_ASYNC_PF_EN,
env->async_pf_en_msr);
}
+if (hyperv_hypercall_available()) {
+kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_GUEST_OS_ID, 0);
+kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_HYPERCALL, 0);
+}
+if (hyperv_vapic_recommended()) {
+kvm_msr_entry_set(&msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
+}
}
if (env->mcg_cap) {
int i;
@@ -1190,6 +1251,14 @@ static int kvm_get_msrs(CPUState *env)
msrs[n++].index = MSR_KVM_ASYNC_PF_EN;
}
+if (hyperv_hypercall_available()) {
+msrs[n++].index = HV_X64_MSR_GUEST_OS_ID;
+msrs[n++].index = HV_X64_MSR_HYPERCALL;
+}
+if (hyperv_vapic_recommended()) {
+msrs[n++].index = HV_X64_MSR_APIC_ASSIST_PAGE;
+}
+
if (env->mcg_cap) {
msrs[n++].index = MSR_MCG_STATUS;
msrs[n++].index = MSR_MCG_CTL;
--
1.7.4.4
[Qemu-devel] [PATCH 1/2] [PATCH RFC v2 1/2] hyper-v: introduce Hyper-V support infrastructure.
---
Makefile.target |2 +
target-i386/cpuid.c | 14 ++
target-i386/hyperv.c | 65 ++
target-i386/hyperv.h | 37
4 files changed, 118 insertions(+), 0 deletions(-)
create mode 100644 target-i386/hyperv.c
create mode 100644 target-i386/hyperv.h
diff --git a/Makefile.target b/Makefile.target
index 325f26e..446fabc 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -202,6 +202,8 @@ obj-$(CONFIG_NO_KVM) += kvm-stub.o
obj-y += memory.o
LIBS+=-lz
+obj-i386-y +=hyperv.o
+
QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
QEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 1e8bcff..261c168 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -27,6 +27,8 @@
#include "qemu-option.h"
#include "qemu-config.h"
+#include "hyperv.h"
+
/* feature flags taken from "Intel Processor Identification and the CPUID
* Instruction" and AMD's "CPUID Specification". In cases of disagreement
* between feature naming conventions, aliases may be added.
@@ -716,6 +718,14 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def,
const char *cpu_model)
goto error;
}
x86_cpu_def->tsc_khz = tsc_freq / 1000;
+} else if (!strcmp(featurestr, "hv_spinlocks")) {
+ char *err;
+ numvalue = strtoul(val, &err, 0);
+ if (!*val || *err) {
+fprintf(stderr, "bad numerical value %s\n", val);
+goto error;
+}
+hyperv_set_spinlock_retries(numvalue);
} else {
fprintf(stderr, "unrecognized feature %s\n", featurestr);
goto error;
@@ -724,6 +734,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def,
const char *cpu_model)
check_cpuid = 1;
} else if (!strcmp(featurestr, "enforce")) {
check_cpuid = enforce_cpuid = 1;
+} else if (!strcmp(featurestr, "hv_relaxed")) {
+hyperv_enable_relaxed_timing(true);
+} else if (!strcmp(featurestr, "hv_vapic")) {
+hyperv_enable_vapic_recommended(true);
} else {
fprintf(stderr, "feature string `%s' not in format
(+feature|-feature|feature=xyz)\n", featurestr);
goto error;
diff --git a/target-i386/hyperv.c b/target-i386/hyperv.c
new file mode 100644
index 000..b2e57ad
--- /dev/null
+++ b/target-i386/hyperv.c
@@ -0,0 +1,65 @@
+/*
+ * QEMU Hyper-V support
+ *
+ * Copyright Red Hat, Inc. 2011
+ *
+ * Author: Vadim Rozenfeld
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "hyperv.h"
+
+static bool hyperv_vapic;
+static bool hyperv_relaxed_timing;
+static int hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
+
+void hyperv_enable_vapic_recommended(bool val)
+{
+hyperv_vapic = val;
+}
+
+void hyperv_enable_relaxed_timing(bool val)
+{
+hyperv_relaxed_timing = val;
+}
+
+void hyperv_set_spinlock_retries(int val)
+{
+hyperv_spinlock_attempts = val;
+if (hyperv_spinlock_attempts < 0xFFF) {
+hyperv_spinlock_attempts = 0xFFF;
+}
+}
+
+bool hyperv_enabled(void)
+{
+return hyperv_hypercall_available() || hyperv_relaxed_timing_enabled();
+}
+
+bool hyperv_hypercall_available(void)
+{
+if (hyperv_vapic ||
+(hyperv_spinlock_attempts != HYPERV_SPINLOCK_NEVER_RETRY)) {
+ return true;
+}
+return false;
+}
+
+bool hyperv_vapic_recommended(void)
+{
+return hyperv_vapic;
+}
+
+bool hyperv_relaxed_timing_enabled(void)
+{
+return hyperv_relaxed_timing;
+}
+
+int hyperv_get_spinlock_retries(void)
+{
+return hyperv_spinlock_attempts;
+}
+
diff --git a/target-i386/hyperv.h b/target-i386/hyperv.h
new file mode 100644
index 000..0d742f8
--- /dev/null
+++ b/target-i386/hyperv.h
@@ -0,0 +1,37 @@
+/*
+ * QEMU Hyper-V support
+ *
+ * Copyright Red Hat, Inc. 2011
+ *
+ * Author: Vadim Rozenfeld
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_HW_HYPERV_H
+#define QEMU_HW_HYPERV_H 1
+
+#include "qemu-common.h"
+#include
+
+#ifndef HYPERV_SPINLOCK_NEVER_RETRY
+#define HYPERV_SPINLOCK_NEVER_RETRY 0x
+#endif
+
+#ifndef KVM_CPUID_SIGNATURE_NEXT
+#define KVM_CPUID_SIGNATURE_NEXT0x4100
+#endif
+
+void hyperv_enable_vapic_recommended(bool val);
+void hyperv_enable_relaxed_timing(bool val);
+void hyperv_set_spinlock_retries(int val);
+
+bool hyperv_enabled(void);
+bool hyperv_hypercall_available(void);
+bool hyperv_vapic_recommended(void);
+bool hyperv_relaxed_timing_enabled(void);
+int hyperv_get_spinlock_retries(void);
+
+#endif /* QEMU_HW_HYPERV_H */
--
1.7.4.4
Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus "ICC" to connect APIC
On 2011-10-23 14:40, Blue Swirl wrote: > I'm not sure that a full bus is needed for now, even if it could match > real HW better, since the memory API already provides the separation > needed. Perhaps this would be needed later to make IRQs per-CPU also, > or to put IOAPIC also to the bus? The ICC interconnects LAPICs and IOAPICs. So it should next take over the management of the local_apics array from apic.c and the ioapics array from ioapic.c. It could implement generic message delivery services. Every bus participant would then have a reception handler that first checks the type and additional fields of a generic ICC message and, on match, forwards it to the corresponding device model functions. That would allow for something nicer than global apic_deliver_irq or ioapic_eoi_broadcast. That's clearly beyond the scope of this series but a good reason to model the ICC as accurately as qdev allows right from the start. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus "ICC" to connect APIC
On Sun, Oct 23, 2011 at 15:45, Jan Kiszka wrote: > On 2011-10-23 14:40, Blue Swirl wrote: >> I'm not sure that a full bus is needed for now, even if it could match >> real HW better, since the memory API already provides the separation >> needed. Perhaps this would be needed later to make IRQs per-CPU also, >> or to put IOAPIC also to the bus? > > The ICC interconnects LAPICs and IOAPICs. But not between CPU core and its LAPIC? > So it should next take over > the management of the local_apics array from apic.c and the ioapics > array from ioapic.c. It could implement generic message delivery > services. Every bus participant would then have a reception handler that > first checks the type and additional fields of a generic ICC message > and, on match, forwards it to the corresponding device model functions. > That would allow for something nicer than global apic_deliver_irq or > ioapic_eoi_broadcast. > > That's clearly beyond the scope of this series but a good reason to > model the ICC as accurately as qdev allows right from the start. OK then, ICC could be a major cleanup.
Re: [Qemu-devel] [PATCH 1/1] Introduce a new bus "ICC" to connect APIC
On 2011-10-23 17:54, Blue Swirl wrote: > On Sun, Oct 23, 2011 at 15:45, Jan Kiszka wrote: >> On 2011-10-23 14:40, Blue Swirl wrote: >>> I'm not sure that a full bus is needed for now, even if it could match >>> real HW better, since the memory API already provides the separation >>> needed. Perhaps this would be needed later to make IRQs per-CPU also, >>> or to put IOAPIC also to the bus? >> >> The ICC interconnects LAPICs and IOAPICs. > > But not between CPU core and its LAPIC? Nope, that link is core-internal and has nothing to do with the ICC. See e.g. Figure 2-2 of Intel's "MultiProcessor Specification". Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [Spice-devel] Possible SPICE/QEMU deadlock on fast client reconnect
On Sun, Oct 23, 2011 at 09:37:59AM +0200, Yonit Halperin wrote: > Hi, > > Sounds like https://bugzilla.redhat.com/show_bug.cgi?id=746950 and > https://bugs.freedesktop.org/show_bug.cgi?id=41858 > Alon's patch: > http://lists.freedesktop.org/archives/spice-devel/2011-September > /005369.html > Should solve it. > Could you add comments about reproducing to the RH bug? the QE wants to know if it is user triggerable. Thanks. > Cheers, > Yonit. > On 10/21/2011 05:26 PM, Daniel P. Berrange wrote: > >In testing my patches for 'add_client' support with SPICE, I noticed > >deadlock in the QEMU/SPICE code. It only happens if I close a SPICE > >client and then immediately reconnect within about 1 second. If I > >wait a couple of seconds before reconnecting the SPICE client I don't > >see the deadlock. > > > >I'm using QEMU& SPICE git master, and GDB has this to say > > > >(gdb) thread apply all bt > > > >Thread 3 (Thread 0x7f269e142700 (LWP 17233)): > >#0 pthread_cond_wait@@GLIBC_2.3.2 () at > >../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:165 > >#1 0x004e80e9 in qemu_cond_wait (cond=, > >mutex=) > > at qemu-thread-posix.c:113 > >#2 0x00556469 in qemu_kvm_wait_io_event (env=) > > at /home/berrange/src/virt/qemu/cpus.c:626 > >#3 qemu_kvm_cpu_thread_fn (arg=0x29217a0) at > >/home/berrange/src/virt/qemu/cpus.c:661 > >#4 0x003a83207b31 in start_thread (arg=0x7f269e142700) at > >pthread_create.c:305 > >#5 0x003a82edfd2d in clone () at > >../sysdeps/unix/sysv/linux/x86_64/clone.S:115 > > > >Thread 2 (Thread 0x7f26822fc700 (LWP 17234)): > >#0 __lll_lock_wait () at > >../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:140 > >#1 0x003a83209ad6 in _L_lock_752 () from /lib64/libpthread.so.0 > >#2 0x003a832099d7 in __pthread_mutex_lock (mutex=0x1182f60) at > >pthread_mutex_lock.c:65 > >#3 0x004e7ec9 in qemu_mutex_lock (mutex=) at > >qemu-thread-posix.c:54 > >#4 0x00507df5 in channel_event (event=3, info=0x2a3c610) at > >ui/spice-core.c:234 > >#5 0x7f269f77be87 in reds_stream_free (s=0x2a3c590) at reds.c:4073 > >#6 0x7f269f75b64f in red_channel_client_disconnect (rcc=0x7f267c069ec0) > >at red_channel.c:961 > >#7 0x7f269f75a131 in red_peer_handle_incoming (handler=0x7f267c06a5a0, > >stream=0x2a3c590) > > at red_channel.c:150 > >#8 red_channel_client_receive (rcc=0x7f267c069ec0) at red_channel.c:158 > >#9 0x7f269f761fbc in handle_channel_events (in_listener=0x7f267c06a5f8, > >events=) > > at red_worker.c:9566 > >#10 0x7f269f776672 in red_worker_main (arg=) at > >red_worker.c:10813 > >#11 0x003a83207b31 in start_thread (arg=0x7f26822fc700) at > >pthread_create.c:305 > >#12 0x003a82edfd2d in clone () at > >../sysdeps/unix/sysv/linux/x86_64/clone.S:115 > > > >Thread 1 (Thread 0x7f269f72e9c0 (LWP 17232)): > >#0 0x003a8320e01d in read () at ../sysdeps/unix/syscall-template.S:82 > >#1 0x7f269f75daaa in receive_data (n=4, in_buf=0x7fffe7a5a02c, fd=18) > >at red_worker.h:150 > >#2 read_message (message=0x7fffe7a5a02c, fd=18) at red_worker.h:163 > >#3 red_dispatcher_disconnect_cursor_peer (rcc=0x7f267c0c0f60) at > >red_dispatcher.c:157 > >#4 0x7f269f75c20d in red_client_destroy (client=0x2a35400) at > >red_channel.c:1189 > >#5 0x7f269f778335 in reds_client_disconnect (client=0x2a35400) at > >reds.c:624 > >---Type to continue, or q to quit--- > >#6 0x7f269f75a131 in red_peer_handle_incoming (handler=0x2a35b50, > >stream=0x2b037d0) at red_channel.c:150 > >#7 red_channel_client_receive (rcc=0x2a35470) at red_channel.c:158 > >#8 0x7f269f75b1e8 in red_channel_client_event (fd=, > >event=, > > data=) at red_channel.c:774 > >#9 0x0045e561 in fd_trampoline (chan=, > >cond=, opaque=) > > at iohandler.c:97 > >#10 0x003a84e427ed in g_main_dispatch (context=0x27fc510) at gmain.c:2441 > >#11 g_main_context_dispatch (context=0x27fc510) at gmain.c:3014 > >#12 0x004c3da3 in glib_select_poll (err=false, xfds=0x7fffe7a5a2e0, > >wfds=0x7fffe7a5a260, rfds= > > 0x7fffe7a5a1e0) at /home/berrange/src/virt/qemu/vl.c:1495 > >#13 main_loop_wait (nonblocking=) at > >/home/berrange/src/virt/qemu/vl.c:1541 > >#14 0x0040fdd1 in main_loop () at > >/home/berrange/src/virt/qemu/vl.c:1570 > >#15 main (argc=, argv=, envp=) > > at /home/berrange/src/virt/qemu/vl.c:3563 > >(gdb) > > > > > > ___ > Spice-devel mailing list > spice-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Qemu-devel] [PATCH] pci_bridge: fix typo
On Sun, Oct 16, 2011 at 15:11, Avi Kivity wrote: > On 10/16/2011 04:44 PM, Blue Swirl wrote: >> Signed-off-by: Blue Swirl >> --- >> hw/pci_bridge.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c >> index b6287cd..650d165 100644 >> --- a/hw/pci_bridge.c >> +++ b/hw/pci_bridge.c >> @@ -319,7 +319,7 @@ int pci_bridge_initfn(PCIDevice *dev) >> sec_bus->parent_dev = dev; >> sec_bus->map_irq = br->map_irq; >> sec_bus->address_space_mem = &br->address_space_mem; >> - memory_region_init(&br->address_space_mem, "pci_pridge_pci", INT64_MAX); >> + memory_region_init(&br->address_space_mem, "pci_bridge_pci", INT64_MAX); >> sec_bus->address_space_io = &br->address_space_io; >> memory_region_init(&br->address_space_io, "pci_bridge_io", 65536); >> pci_bridge_region_init(br); > > Reviewed-by: Avi Kivity Thanks for the review, applied.
Re: [Qemu-devel] [PATCH V5] Add stdio char device on windows
Thanks, applied.
On Thu, Oct 6, 2011 at 14:37, Fabien Chouteau wrote:
> Simple implementation of an stdio char device on Windows.
>
> Signed-off-by: Fabien Chouteau
> ---
> qemu-char.c | 227
> ++-
> 1 files changed, 225 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 09d2309..b9381be 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -538,6 +538,9 @@ int send_all(int fd, const void *_buf, int len1)
> }
> #endif /* !_WIN32 */
>
> +#define STDIO_MAX_CLIENTS 1
> +static int stdio_nb_clients;
> +
> #ifndef _WIN32
>
> typedef struct {
> @@ -545,8 +548,6 @@ typedef struct {
> int max_size;
> } FDCharDriver;
>
> -#define STDIO_MAX_CLIENTS 1
> -static int stdio_nb_clients = 0;
>
> static int fd_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
> {
> @@ -1451,6 +1452,8 @@ static int qemu_chr_open_pp(QemuOpts *opts,
> CharDriverState **_chr)
>
> #else /* _WIN32 */
>
> +static CharDriverState *stdio_clients[STDIO_MAX_CLIENTS];
> +
> typedef struct {
> int max_size;
> HANDLE hcom, hrecv, hsend;
> @@ -1459,6 +1462,14 @@ typedef struct {
> DWORD len;
> } WinCharState;
>
> +typedef struct {
> + HANDLE hStdIn;
> + HANDLE hInputReadyEvent;
> + HANDLE hInputDoneEvent;
> + HANDLE hInputThread;
> + uint8_t win_stdio_buf;
> +} WinStdioCharState;
> +
> #define NSENDBUF 2048
> #define NRECVBUF 2048
> #define MAXCONNECT 1
> @@ -1809,6 +1820,217 @@ static int qemu_chr_open_win_file_out(QemuOpts *opts,
> CharDriverState **_chr)
>
> return qemu_chr_open_win_file(fd_out, _chr);
> }
> +
> +static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len)
> +{
> + HANDLE hStdOut = GetStdHandle(STD_OUTPUT_HANDLE);
> + DWORD dwSize;
> + int len1;
> +
> + len1 = len;
> +
> + while (len1 > 0) {
> + if (!WriteFile(hStdOut, buf, len1, &dwSize, NULL)) {
> + break;
> + }
> + buf += dwSize;
> + len1 -= dwSize;
> + }
> +
> + return len - len1;
> +}
> +
> +static void win_stdio_wait_func(void *opaque)
> +{
> + CharDriverState *chr = opaque;
> + WinStdioCharState *stdio = chr->opaque;
> + INPUT_RECORD buf[4];
> + int ret;
> + DWORD dwSize;
> + int i;
> +
> + ret = ReadConsoleInput(stdio->hStdIn, buf, sizeof(buf) / sizeof(*buf),
> + &dwSize);
> +
> + if (!ret) {
> + /* Avoid error storm */
> + qemu_del_wait_object(stdio->hStdIn, NULL, NULL);
> + return;
> + }
> +
> + for (i = 0; i < dwSize; i++) {
> + KEY_EVENT_RECORD *kev = &buf[i].Event.KeyEvent;
> +
> + if (buf[i].EventType == KEY_EVENT && kev->bKeyDown) {
> + int j;
> + if (kev->uChar.AsciiChar != 0) {
> + for (j = 0; j < kev->wRepeatCount; j++) {
> + if (qemu_chr_be_can_write(chr)) {
> + uint8_t c = kev->uChar.AsciiChar;
> + qemu_chr_be_write(chr, &c, 1);
> + }
> + }
> + }
> + }
> + }
> +}
> +
> +static DWORD WINAPI win_stdio_thread(LPVOID param)
> +{
> + CharDriverState *chr = param;
> + WinStdioCharState *stdio = chr->opaque;
> + int ret;
> + DWORD dwSize;
> +
> + while (1) {
> +
> + /* Wait for one byte */
> + ret = ReadFile(stdio->hStdIn, &stdio->win_stdio_buf, 1, &dwSize,
> NULL);
> +
> + /* Exit in case of error, continue if nothing read */
> + if (!ret) {
> + break;
> + }
> + if (!dwSize) {
> + continue;
> + }
> +
> + /* Some terminal emulator returns \r\n for Enter, just pass \n */
> + if (stdio->win_stdio_buf == '\r') {
> + continue;
> + }
> +
> + /* Signal the main thread and wait until the byte was eaten */
> + if (!SetEvent(stdio->hInputReadyEvent)) {
> + break;
> + }
> + if (WaitForSingleObject(stdio->hInputDoneEvent, INFINITE)
> + != WAIT_OBJECT_0) {
> + break;
> + }
> + }
> +
> + qemu_del_wait_object(stdio->hInputReadyEvent, NULL, NULL);
> + return 0;
> +}
> +
> +static void win_stdio_thread_wait_func(void *opaque)
> +{
> + CharDriverState *chr = opaque;
> + WinStdioCharState *stdio = chr->opaque;
> +
> + if (qemu_chr_be_can_write(chr)) {
> + qemu_chr_be_write(chr, &stdio->win_stdio_buf, 1);
> + }
> +
> + SetEvent(stdio->hInputDoneEvent);
> +}
> +
> +static void qemu_chr_set_echo_win_stdio(CharDriverState *chr, bool echo)
> +{
> + WinStdioCharState *stdio = chr->opaque;
> + DWORD dwMode = 0;
> +
> + GetConsoleMode(stdio->hStdIn, &dwMode);
> +
> + if (echo) {
> + SetConsoleMode(stdio->hStdIn, dwMode | ENABLE_ECHO_INPUT);
> + } else {
> + SetConsoleMode(st
[Qemu-devel] Help: Email archive search broken
Hello, Search within the qemu-devel Email archives appears broken. Articles are returned, but it seems only up to around June 2011. For example, the following query searching for "dvd": > http://lists.nongnu.org/archive/cgi-bin/namazu.cgi?query=dvd&submit=Search%21&idxname=qemu-devel&max=20&result=normal&sort=date%3Alate Many thanks if that could be fixed. -Juan
Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: Fail configure when libfdt is not available
On Fri, Oct 21, 2011 at 09:34:22AM +0200, Paolo Bonzini wrote: > On 10/20/2011 08:35 PM, Gerd Hoffmann wrote: > >Hi, > > > >> If there are build problems with libfdt on any platform let me know > >> about them. I would like it to build clean as widely as possible, but > >> I don't have that great a diversity of build environments, so I have > >> to reply on bug reports. > > > > Fails to build on RHEL-5: > > > > CC convert-dtsv0-lexer.lex.o > > cc1: warnings being treated as errors > > convert-dtsv0-lexer.lex.c:693: warning: no previous prototype for 'yylex' > > make: *** [convert-dtsv0-lexer.lex.o] Error 1 > > > > Removing -Werror from the Makefile gets me a bit further: > > > > CC dtc-lexer.lex.o > > dtc-lexer.lex.c:683: warning: no previous prototype for 'yylex' > > dtc-lexer.l: In function 'push_input_file': > > dtc-lexer.l:192: warning: implicit declaration of function > > 'yypush_buffer_state' > > dtc-lexer.l:192: warning: nested extern declaration of 'yypush_buffer_state' > > dtc-lexer.l: In function 'pop_input_file': > > dtc-lexer.l:201: warning: implicit declaration of function > > 'yypop_buffer_state' > > dtc-lexer.l:201: warning: nested extern declaration of 'yypop_buffer_state' > > CC dtc-parser.tab.o > > LD dtc > > dtc-lexer.lex.o: In function `push_input_file': > > /home/buildbot/git/dtc/dtc-lexer.l:192: undefined reference to > > `yypush_buffer_state' > > dtc-lexer.lex.o: In function `pop_input_file': > > /home/buildbot/git/dtc/dtc-lexer.l:201: undefined reference to > > `yypop_buffer_state' > > collect2: ld returned 1 exit status > > make: *** [dtc] Error 1 > > > > I guess the flex version shipped with RHEL-5 is too old. > > > > $ rpm -qf $(which lex) > > flex-2.5.4a-41.fc6 > > flex is only used by dtc, not libfdt, so you can probably patch it > out. Well, you can just "make libfdt" instead of "make all". > However, the usual convention is that lex- and yacc-generated files > are shipped in the tarball, with a "make dist" that wraps tar and/or > git-archive. See the following patch. Well, it's _a_ convention that's used sometimes. Particularly for projects where the lex/yacc based parser is some little side thing, rather than the core component. That said, a "make dist" target for dtc is probably not a bad idea. Even though we don't really build tarballs with the frequency we probably should. [snip] > diff --git a/Makefile b/Makefile > index b32409b..edfdb9c 100644 > --- a/Makefile > +++ b/Makefile > @@ -246,4 +246,27 @@ $(LIBFDT_lib): > @$(VECHO) BISON $@ > $(BISON) -d $< > > +.PHONY: distdir dist-gz dist-xz dist > + > +distdir = dtc-$(dtc_version)/ > +distdir: $(DTC_GEN_SRCS) $(CONVERT_GEN_SRCS) > + mkdir $(distdir) > + @$(VECHO) DISTDIR $@ > + git archive --format=tar HEAD --prefix=$(distdir) | tar -xf - I'm a little uncomfortable with this, since it means you can only make dist from a git tree; you can't make dist to recreate a tarball from itself. > + @for i in $^; do \ > + $(if $(V),echo cp $$i $(distdir),:); \ > + cp $$i $(distdir); \ > + done > + chmod -R ug+w $(distdir) > + > +dist-gz: distdir > + @$(VECHO) TAR dtc-$(dtc_version).tar.gz > + tar -chozf dtc-$(dtc_version).tar.gz $(distdir) > +dist-xz: distdir > + @$(VECHO) TAR dtc-$(dtc_version).tar.xz > + tar -Ixz -chof dtc-$(dtc_version).tar.xz $(distdir) > + > +dist: dist-gz dist-xz > + rm -rf $(distdir) > + > FORCE: > diff --git a/Makefile.dtc b/Makefile.dtc > index bece49b..0b2c869 100644 > --- a/Makefile.dtc > +++ b/Makefile.dtc > @@ -14,5 +14,5 @@ DTC_SRCS = \ > treesource.c \ > util.c > > -DTC_GEN_SRCS = dtc-lexer.lex.c dtc-parser.tab.c > -DTC_OBJS = $(DTC_SRCS:%.c=%.o) $(DTC_GEN_SRCS:%.c=%.o) > +DTC_GEN_SRCS = dtc-lexer.lex.c dtc-parser.tab.c dtc-parser.tab.h > +DTC_OBJS = $(patsubst %.c,%.o,$(DTC_SRCS) $(filter %.c, $(DTC_GEN_SRCS))) This is wrong though. The Makefile.* fragments are designed to be usable from other make systems, when libfdt or whatever is embedded in other projects. Therefore, I don't want to change the semantics of this variable from the present meaning of "generated files which need to be compiled with a C compiler and linked into the dtc binary". Instead you should create a new variable to cover other generated files which you can also use from the make dist target. With that fixed, you can send this to j...@jdl.com (dtc maintainer) and devicetree-disc...@lists.ozlabs.org which is the usual forum for dtc patches. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [Question] dump memory when host pci device is used by guest
At 10/21/2011 09:02 PM, Dave Anderson Write: > > > - Original Message - >> At 10/21/2011 03:11 PM, Jan Kiszka Write: >>> On 2011-10-20 12:03, Wen Congyang wrote: At 10/20/2011 05:41 PM, Jan Kiszka Write: > On 2011-10-20 03:22, Wen Congyang wrote: I didn't read full story but 'crash' is used for investigating kernel core generated by kdump for several years. Considering support service guys, virsh dump should support a format for crash because they can't work well at investigating vmcore by gdb. crash has several functionality useful for them as 'show kerne log', 'focus on a cpu' 'for-each-task', 'for-each-vma', 'extract ftrace log' etc. Anyway, if a man, who is not developper of qemu/kvm, should learn 2 tools for investigating kernel dump, it sounds harmful. >>> >>> Right, that's why everything (live debugging & crash analysis) should be >>> consolidated on the long run over gdb. crash is architecturally obsolete >>> today - not saying it is useless! >> >> I do not know why crash is obsoleted today. Is there a new better tool >> to instead >> crash? > > I'm not aware of equally powerful (python) scripts for gdb as > replacement, but I think it's worth starting a porting effort at > some point. > >> >> At least, I always use crash to live debugging & crash analysis. > > Then you may answer some questions to me: > - Can you attach to a remote target (kgdb, qemu, etc.) and how? No. crash's live debugging only can work the kernel is live. I can use it get some var's value, or some other information from kernel. If kernel panics, we can use gdb to attach to a remote target as you said. But on end user machine, we can not do it, we should dump the memory into a file and analyze it in another machine while the end user's guest can be restart. > - Can you use it with latest gdb versions or is the gdb functionality >hard-wired due to an embedded gdb core in crash (that's how I >understood Christoph's reply to this topic) If I use crash, I can not use latest gdb versions. Do we always need to use the latest gdb versions? Currently, gdb-7.0 is embedded into crash, and it is enough to me. If the gdb embedded into crash cannot anaylze the vmcore, I think we can update it and rebuild crash. >>> >>> crash is simply designed the wrong way around (from today's >>> perspective): it should augment upstream gdb instead of forking it. >> >> Cc Dave Anderson. He knows how crash uses gdb. >> >> I think that crash does not fork a task to execute gdb, and gdb is a >> part of crash. > > I'm not sure what the question is, but you can consider crash as a huge The question is that: 'virsh dump' can not be used when host pci device is used by guest. We are discussing how to fix the problem. We have determined that introduce a new monitor command dump. Jan suggested that the core file's format is gdb standard core format. Does crash support such format? If no, is it possible to support such format? Thanks Wen Congyang > wrapper around its embedded gdb, which it invokes as "gdb vmlinux", and > then takes over the user interface. It doesn't have a clue as to what > the memory source is, i.e., whether it's one of the almost 20 different > dumpfile formats that it supports (including "virsh dump"), or if it's > running against a live system. It has its own command set, although > you can enter some gdb commands, write gdb scripts, etc. But the main > purpose of the embedded gdb is for the crash-level sources to be able > to gather data structure information, disassemble text, add-symbol-file > kernel modules, and so on. There is no kgdb remote linkage. > > It's currently embedding gdb-7.0, although as we speak I'm updating it > to gdb-7.3.1 because the compiler guys have decided that dwarf4 should be > used by default. > > It would be kind of cool if there was a "/dev/mem"-like interface > to a KVM guest's physical memory, so that you could sit on a KVM host > and enter "crash vmlinux-of-guest /dev/mem-of-guest" in order to > run live analysis of a guest. > > Anyway, sorry if it doesn't meet your needs... > > Dave >
[Qemu-devel] [PATCH] ppc: Fix up usermode only builds
The recent usage of MemoryRegion in kvm_ppc.h breaks builds with
CONFIG_USER_ONLY=y. This patch fixes it.
Signed-off-by: David Gibson
---
target-ppc/kvm_ppc.h |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index b0d6fb6..f9c0198 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -23,9 +23,11 @@ int kvmppc_get_hypercall(CPUState *env, uint8_t *buf, int
buf_len);
int kvmppc_set_interrupt(CPUState *env, int irq, int level);
void kvmppc_set_papr(CPUState *env);
int kvmppc_smt_threads(void);
+#ifndef CONFIG_USER_ONLY
off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem);
void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd);
int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t window_size);
+#endif /* !CONFIG_USER_ONLY */
const ppc_def_t *kvmppc_host_cpu_def(void);
#else
@@ -69,6 +71,7 @@ static inline int kvmppc_smt_threads(void)
return 1;
}
+#ifndef CONFIG_USER_ONLY
static inline off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem)
{
return 0;
@@ -85,6 +88,7 @@ static inline int kvmppc_remove_spapr_tce(void *table, int
pfd,
{
return -1;
}
+#endif /* !CONFIG_USER_ONLY */
static inline const ppc_def_t *kvmppc_host_cpu_def(void)
{
--
1.7.6.3
Re: [Qemu-devel] [PATCH] ppc: Fix up usermode only builds
On 23.10.2011, at 20:25, David Gibson wrote:
> The recent usage of MemoryRegion in kvm_ppc.h breaks builds with
> CONFIG_USER_ONLY=y. This patch fixes it.
>
> Signed-off-by: David Gibson
Ouch. Thanks a lot for the fix!
Alex
> ---
> target-ppc/kvm_ppc.h |4
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index b0d6fb6..f9c0198 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -23,9 +23,11 @@ int kvmppc_get_hypercall(CPUState *env, uint8_t *buf, int
> buf_len);
> int kvmppc_set_interrupt(CPUState *env, int irq, int level);
> void kvmppc_set_papr(CPUState *env);
> int kvmppc_smt_threads(void);
> +#ifndef CONFIG_USER_ONLY
> off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem);
> void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd);
> int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t window_size);
> +#endif /* !CONFIG_USER_ONLY */
> const ppc_def_t *kvmppc_host_cpu_def(void);
>
> #else
> @@ -69,6 +71,7 @@ static inline int kvmppc_smt_threads(void)
> return 1;
> }
>
> +#ifndef CONFIG_USER_ONLY
> static inline off_t kvmppc_alloc_rma(const char *name, MemoryRegion *sysmem)
> {
> return 0;
> @@ -85,6 +88,7 @@ static inline int kvmppc_remove_spapr_tce(void *table, int
> pfd,
> {
> return -1;
> }
> +#endif /* !CONFIG_USER_ONLY */
>
> static inline const ppc_def_t *kvmppc_host_cpu_def(void)
> {
> --
> 1.7.6.3
>
Re: [Qemu-devel] [RFC v2 PATCH 5/4 PATCH] virtio-net: send gratuitous packet when needed
On Sat, 22 Oct 2011 13:43:11 +0800, Jason Wang wrote: > This make let virtio-net driver can send gratituous packet by a new > config bit - VIRTIO_NET_S_ANNOUNCE in each config update > interrupt. When this bit is set by backend, the driver would schedule > a workqueue to send gratituous packet through NETDEV_NOTIFY_PEERS. > > This feature is negotiated through bit VIRTIO_NET_F_GUEST_ANNOUNCE. > > Signed-off-by: Jason Wang This seems like a huge layering violation. Imagine this in real hardware, for example. There may be a good reason why virtual devices might want this kind of reconfiguration cheat, which is unnecessary for normal machines, but it'd have to be spelled out clearly in the spec to justify it... Cheers, Rusty.
Re: [Qemu-devel] [RFC v2 PATCH 5/4 PATCH] virtio-net: send gratuitous packet when needed
On Mon, Oct 24, 2011 at 02:54:59PM +1030, Rusty Russell wrote: > On Sat, 22 Oct 2011 13:43:11 +0800, Jason Wang wrote: > > This make let virtio-net driver can send gratituous packet by a new > > config bit - VIRTIO_NET_S_ANNOUNCE in each config update > > interrupt. When this bit is set by backend, the driver would schedule > > a workqueue to send gratituous packet through NETDEV_NOTIFY_PEERS. > > > > This feature is negotiated through bit VIRTIO_NET_F_GUEST_ANNOUNCE. > > > > Signed-off-by: Jason Wang > > This seems like a huge layering violation. Imagine this in real > hardware, for example. commits 06c4648d46d1b757d6b9591a86810be79818b60c and 99606477a5888b0ead0284fecb13417b1da8e3af document the need for this: NETDEV_NOTIFY_PEERS notifier indicates that a device moved to a different physical link. and In real hardware such notifications are only generated when the device comes up or the address changes. So hypervisor could get the same behaviour by sending link up/down events, this is just an optimization so guest won't do unecessary stuff like try to reconfigure an IP address. Maybe LOCATION_CHANGE would be a better name? > There may be a good reason why virtual devices might want this kind of > reconfiguration cheat, which is unnecessary for normal machines, I think yes, the difference with real hardware is guest can change location without link getting dropped. FWIW, Xen seems to use this capability too. > but > it'd have to be spelled out clearly in the spec to justify it... > > Cheers, > Rusty. Agree, and I'd like to see the spec too. The interface seems to involve the guest clearing the status bit when it detects an event? Also - how does it interact with the link up event? We probably don't want to schedule this when we detect a link status change or during initialization, as this patch seems to do? What if link goes down while the work is running? Is that OK? -- MST
Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Correct vmx/dfp handling in both KVM and TCG cases
On Thu, Oct 20, 2011 at 11:49:40PM -0700, Alexander Graf wrote: > > On 20.10.2011, at 22:06, David Gibson wrote: > > > On Thu, Oct 20, 2011 at 07:40:00PM -0700, Alexander Graf wrote: > >> On 20.10.2011, at 17:41, David Gibson wrote: > >>> On Thu, Oct 20, 2011 at 10:12:51AM -0700, Alexander Graf wrote: > On 17.10.2011, at 21:15, David Gibson wrote: > > [snip] > >>> So, I really don't follow what the logic you want is. It sounds more > >>> like what I have already, so I'm not sure how -cpu host comes into > >>> this. > >> > >> Well, I want something very simple, layered: > >> > >> -cpu host only searches for pvr matches and selects a different CPU > >> -type based on this > > > > Hrm, ok, well I can do this if you like, but note that this is quite > > different from how -cpu host behaves on x86. There it builds the CPU > > spec from scratch based on querying the host cpuid, rather than > > selecting from an existing list of cpus. I selected from the existing > > table based on host PVR because that was the easiest source for some > > of the info in the cpu_spec, but my intention was that anything we > > _can_ query directly from the host would override the table. > > > > It seems to be your approach is giving up on the possibility of > > allowing -cpu host to work (and give you full access to the host > > features) when qemu doesn't recognize the precise PVR of the host cpu. > > I disagree :). This is what x86 does: > > * -cpu host fetches CPUID info from host, puts it into vcpu > * vcpu CPUID info gets ANDed with KVM capability CPUIDs > > I want basically the same thing. I want to have 2 different layers > for 2 different semantics. One for what the host CPU would be able > to do and one for what we can emulate, and two different steps to > ensure control over them. > > The thing I think I'm apparently not bringing over yet is that I'm > more than happy to get rid of the PVR searching step for -cpu host > and instead use a full host capability inquiry mechanism. But that > inquiry should indicate what the host CPU can do. It has nothing to > do with KVM yet. The masking with KVM capabilities should be the > next separate step. > > My goal is really to separate different layers into actual different > layers :). Hrm. I think I see what you're getting at. Although nothing in that patch is about kvm capabilities - it's all about working out what the host's cpu can do. > > This gets further complicated in the case of the w-i-p patch I have to > > properly advertise page sizes, where it's not just presence or absence > > of a feature, but the specific SLB and HPTE encodings must be > > advertised to the guest. > > Yup, so we'd read out the host dt to find the host possible > encodings (probably a bad idea, but that's a different story) Um, a different story perhaps, but one I kind of need an answer to in the near future... I can query the host cpu's page sizes easily enough, but I'm really not sure where this should be stashed before filtering as suggested below. > and > then ask KVM what encodings it supports and expose the ANDed product > of them to the guest. > > > > >> We have 2 masks of available flags: TCG emulatable flags and KVM > >> virtualizable flags. The KVM flags need to be generated dynamically, > >> from the host dt for now. TCG flags are constant. > >> > >> Then we always AND the inst feature bits with the mask. This tells > >> every other layer what features are available. That way even running > >> -cpu G5 on a p7 works properly by not exposing DFP for example. > > > > That case was already fine. > > > > Are you suggesting doing the AND in the per-machine code (so, as we > > build the guest dt in the spapr case) or when we build the env->insn_flags > > from the spec->insn_flags? > > I suggest doing that in translate_init.c where we actually build the > env->insn_flags from the spec. Ok, that makes sense. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] build with trace enabled is broken by the commit c572f23a3e7180dbeab5e86583e43ea2afed6271 hw/9pfs: Introduce tracing for 9p pdu handlers
On Sun, 23 Oct 2011 15:54:59 +0200, Jan Kiszka wrote: > On 2011-10-21 17:10, Aneesh Kumar K.V wrote: > > On Thu, 20 Oct 2011 23:20:54 +0400, Max Filippov wrote: > >> Hi. > >> > >> Current git head build with trace enabled is broken by the commit > >> c572f23a3e7180dbeab5e86583e43ea2afed6271 hw/9pfs: Introduce tracing for 9p > >> pdu handlers. > >> Error messages: > >> > >> In file included from trace.c:2:0: > >> trace.h: In function ‘trace_v9fs_attach’: > >> trace.h:2850:9: error: too many arguments for format > >> [-Werror=format-extra-args] > >> trace.h: In function ‘trace_v9fs_wstat’: > >> trace.h:3039:9: error: too many arguments for format > >> [-Werror=format-extra-args] > >> trace.h: In function ‘trace_v9fs_mkdir’: > >> trace.h:3088:9: error: too many arguments for format > >> [-Werror=format-extra-args] > >> trace.h: In function ‘trace_v9fs_mkdir_return’: > >> trace.h:3095:9: error: too many arguments for format > >> [-Werror=format-extra-args] > >> cc1: all warnings being treated as errors > >> > >> Prototypes in the trace-events do not match format strings, e.g. > >> > >> v9fs_attach(uint16_t tag, uint8_t id, int32_t fid, int32_t afid, char* > >> uname, char* aname) "tag %d id %d fid %d afid %d aname %s" > >> > >> The following patch fixes it, but I'm not sure the format lines are > >> appropriate. > > > > Can you send the patch with signed-off-by: I will add it in the next > > pull request. > > There are more breakages with tracing enabled: > > CClibhw64/9pfs/virtio-9p.o > cc1: warnings being treated as errors > /data/qemu/hw/9pfs/virtio-9p.c: In function ‘v9fs_create’: > /data/qemu/hw/9pfs/virtio-9p.c:2225:9: error: ‘iounit’ may be used > uninitialized in this function > > > /data/qemu/hw/9pfs/virtio-9p.c: In function ‘v9fs_readdir’: > /data/qemu/hw/9pfs/virtio-9p.c:2063:13: error: ‘count’ may be used > uninitialized in this function > > > /data/qemu/hw/9pfs/virtio-9p.c: In function ‘v9fs_xattrwalk’: > /data/qemu/hw/9pfs/virtio-9p.c:3103:13: error: ‘size’ may be used > uninitialized in this function > > > /data/qemu/hw/9pfs/virtio-9p.c: In function ‘v9fs_lcreate’: > /data/qemu/hw/9pfs/virtio-9p.c:1730:13: error: ‘iounit’ may be used > uninitialized in this function > > > /data/qemu/hw/9pfs/virtio-9p.c: In function ‘v9fs_open’: > /data/qemu/hw/9pfs/virtio-9p.c:1651:9: error: ‘iounit’ may be used > uninitialized in this function > > > > Not sure what the undefined variables are supposed to contain for the > trace output, so I refrain from writing any patch. Ok i am testing VirtFS tracing and will send a patch that should fix these issues. Sorry for all inconvenience. Even though i do test on three different distros, all of them were done without tracing enabled. -aneesh
Re: [Qemu-devel] [Qemu-discuss] [Qemu-discussion] QEMU via GDB
It dosen't work. GDB returns the same error. - Original Message - From: davide.ferrare...@studenti.univr.it Date: Monday, October 24, 2011 8:37 Subject: Re: [Qemu-discuss] [Qemu-discussion] QEMU via GDB To: davide.ferrare...@studenti.univr.it > It dosen't work. GDB return the same error. > > - Original Message - > From: davide.ferrare...@studenti.univr.it > Date: Friday, October 21, 2011 16:18 > Subject: [Qemu-discuss] [Qemu-discussion] QEMU via GDB > To: qemu-disc...@nongnu.org > > > Dear all, > > I am trying to debug QEMU via GDB. > > > > > > I configured and compiled QEMU with debugging flags, i.e., > > # CFLAGS="-g3 -O0" ./configure --disable-gfx-check > > > > > > and run gdb: > > > > > > # gdb ./i386-linux-user/qemu-i386 > > > > > > (gdb) break main > > (gdb) run > > > > Starting program: /home/test/femu/i386-linux-user/qemu-i386 > > Failed to read a valid object file image from memory. > > Warning: > > Cannot insert breakpoint 1. > > Error accessing memory address 0x2f7df: Input/output error. > > > > > > Is there any extra flag to be specified with the GDB for QEMU debugging? > > I am wondering if the QEMU virtual machine creates any problem to the > > GDB virtual machine. > > > > > > Thanks.
[Qemu-devel] Question about intermediate code generation in qemu (tcg)
Hi, I was wondering if someone could help me understand some aspects of the current qemu code generation routines. How are floating point and SSE ops currently handled? I do not see specific tcg routines for these cases(the README seems to indicate that FP and vector ops are not handled using the current IL). Thanks in advanced, Carter.
