[Qemu-devel] Re: [PATCHv2] qemu-kvm/vhost: fix up irqfd support
Looks fine except for the extra white space here and copied below. Thanks, Alex OK, I fixed this up and queued the patch with your ack. Thanks for the review! -- MST
Re: [Qemu-devel] [Bug 642304] Re: Solaris/x86 v10 hangs under KVM
07.10.2010 17:17, Nigel Horne wrote: 2) KVM doesn't have specific versions on Debian. The kernel is built with KVM included. The kernel is version 2.6.32-5 This is just plain wrong. _Every_ package in debian has specific version, just like in almost every other distribution. /mjt
[Qemu-devel] Re: UPD: qemu-kvm 0.12.5 virtio-net page allocation error
Peter Lieven wrote: Am 03.10.2010 um 01:48 schrieb Peter Lieven: Hi, running 0.12.5 with a Ubuntu LTS 10.04.1 64-bit kernel I see the following error after a few days with severe load (constant 100-200mbps input). # uname -a Linux ubuntu-newsfeed 2.6.32-24-server #43-Ubuntu SMP Thu Sep 16 16:05:42 UTC 2010 x86_64 GNU/Linux Linux newsfeed-spool 2.6.27.48-0.2-default seems not affected. I have a machine with even higher load (about 300-400mbits constant network load) up for 4 months without a crash. Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] swapper: page allocation failure. order:0, mode:0x20 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] Pid: 0, comm: swapper Not tainted 2.6.32-24-server #43-Ub untu Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] Call Trace: Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] IRQ [810fa7ae] __alloc_pages_slowpath+0x56e /0x580 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [810fa931] __alloc_pages_nodemask+0x171/0x180 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [8112d817] alloc_pages_current+0x87/0xd0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [813db792] try_fill_recv+0x182/0x200 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [813db9bd] virtnet_poll+0x10d/0x160 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [81473c5f] net_rx_action+0x10f/0x250 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [8106e317] __do_softirq+0xb7/0x1e0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [810c4dc0] ? handle_IRQ_event+0x60/0x170 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [810142ec] call_softirq+0x1c/0x30 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [81015cb5] do_softirq+0x65/0xa0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [8106e1b5] irq_exit+0x85/0x90 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [8155fd25] do_IRQ+0x75/0xf0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [81013b13] ret_from_intr+0x0/0x11 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] EOI [81038acb] ? native_safe_halt+0xb/0x10 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [8101b69d] ? default_idle+0x3d/0x90 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [81011e63] ? cpu_idle+0xb3/0x110 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [8154285b] ? rest_init+0x6b/0x80 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [8187edcc] ? start_kernel+0x368/0x371 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [8187e33a] ? x86_64_start_reservations+0x125/0x129 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] [8187e438] ? x86_64_start_kernel+0xfa/0x109 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] Mem-Info: Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] Node 0 DMA per-cpu: Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU0: hi:0, btch: 1 usd: 0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU1: hi:0, btch: 1 usd: 0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU2: hi:0, btch: 1 usd: 0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU3: hi:0, btch: 1 usd: 0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] Node 0 DMA32 per-cpu: Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU0: hi: 186, btch: 31 usd: 59 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU1: hi: 186, btch: 31 usd: 159 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU2: hi: 186, btch: 31 usd: 102 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU3: hi: 186, btch: 31 usd: 151 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] Node 0 Normal per-cpu: Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU0: hi: 186, btch: 31 usd: 51 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU1: hi: 186, btch: 31 usd: 157 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU2: hi: 186, btch: 31 usd: 162 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] CPU3: hi: 186, btch: 31 usd: 175 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] active_anon:1857 inactive_anon:1602 isolated_anon:0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] active_file:458063 inactive_file:467825 isolated_file:0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] unevictable:0 dirty:74195 writeback:18395 unstable:0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] free:5214 slab_reclaimable:50506 slab_unreclaimable:3256 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] mapped:4151 shmem:1592 pagetables:400 bounce:0 Oct 2 10:09:35 ubuntu-newsfeed kernel: [248407.790029] Node 0 DMA free:15896kB
[Qemu-devel] Re: [PATCHv2] qemu-kvm/vhost: fix up irqfd support
On Thu, 2010-10-07 at 11:57 +0200, Michael S. Tsirkin wrote:
On Wed, Oct 06, 2010 at 04:05:38PM -0600, Alex Williamson wrote:
On Wed, 2010-10-06 at 23:44 +0200, Michael S. Tsirkin wrote:
On Wed, Oct 06, 2010 at 11:24:24AM -0600, Alex Williamson wrote:
You could always keep the functions as separate wrapper callers of the
common function so you only need to keep true = unset, false = set
straight in one place. Thanks,
Just to show why it does not work, I did exactly this: as you see the
code is shorter but the true/false magic gets spread: it was in 2
places, (set/unset) now it is in 4 places and it is within the loop, in
code that is more complex.
You seem to have missed the wrapper function. I'm simply suggesting
something like this:
Good idea. I tweaked the code a bit more and came up with this:
I think I will keep this as an incremental patch just to
keep diffs small.
msix: factor out mask notifier code
Move some common code handling msix mask notifiers to a function.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
diff --git a/hw/msix.c b/hw/msix.c
index 3d4dd61..5f66d20 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -583,40 +583,26 @@ void msix_unuse_all_vectors(PCIDevice *dev)
msix_free_irq_entries(dev);
}
-static int msix_set_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
+/* Invoke the notifier if vector entry is used and unmasked. */
+static int msix_notify_if_unmasked(PCIDevice *dev, unsigned vector, int
masked)
{
-int r = 0;
-if (vector = dev-msix_entries_nr || !dev-msix_entry_used[vector])
-return 0;
-
assert(dev-msix_mask_notifier);
-
-/* Unmask the new notifier unless vector is masked. */
-if (!msix_is_masked(dev, vector)) {
-r = dev-msix_mask_notifier(dev, vector, false);
-if (r 0) {
-return r;
-}
+if (!dev-msix_entry_used[vector] || msix_is_masked(dev, vector)) {
+return 0;
}
-return r;
+return dev-msix_mask_notifier(dev, vector, masked);
}
-static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned
vector)
+static int msix_set_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
{
-int r = 0;
-if (vector = dev-msix_entries_nr || !dev-msix_entry_used[vector])
-return 0;
-
-assert(dev-msix_mask_notifier);
+ /* Notifier has been set. Invoke it on unmasked vectors. */
+ return msix_notify_if_unmasked(dev, vector, 0);
+}
Looks fine except for the extra white space here and copied below.
Thanks,
Alex
-/* Mask the old notifier unless it is already masked. */
-if (!msix_is_masked(dev, vector)) {
-r = dev-msix_mask_notifier(dev, vector, true);
-if (r 0) {
-return r;
-}
-}
-return r;
+static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned
vector)
+{
+ /* Notifier will be unset. Invoke it to mask unmasked entries. */
+ return msix_notify_if_unmasked(dev, vector, 1);
}
int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func f)
[Qemu-devel] [Bug 642304] Re: Solaris/x86 v10 hangs under KVM
As I mentioned in email reply, _every_ package in almost every distribution (the ones I know anyway), Debian included, has a version number attached. The git commit ID (58aebb946acff82c62383f350cab593e55cc13dc) appears to be in qemu or qemu-kvm git tree (it's found on both), somewhere past 0.13.0-rc0 (dated Sep 18 2010). But from this commit ID it's impossible to tell if you're using qemu or qemu-kvm. What are you using --enable-io-thread for? -- Solaris/x86 v10 hangs under KVM https://bugs.launchpad.net/bugs/642304 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Incomplete Bug description: Solaris/x86 10 guest hangs when running under KVM with the message Running Configuration Assistant. It runs fine when -enable-kvm isn't given as a command option. Host OS: Linux/x86_64 Guest OS: Solaris/x86 Command Line: qemu -hda solaris.img -m 192 -boot c -enable-kvm Build Configure: ./configure --enable-linux-aio --enable-io-thread --enable-kvm GIT commit: 58aebb946acff82c62383f350cab593e55cc13dc
[Qemu-devel] Re: [PATCH 3/6] Add more error handling to strtosz()
On 10/07/2010 05:01 PM, jes.soren...@redhat.com wrote: +if (tmpval ~(size_t)0) Since -1 is an error, this needs to be =. Paolo
[Qemu-devel] [PATCH] configure: Send error message from spice check to /dev/null
pkg-config is not always available (e.g. on win32 hosts),
but we don't want to see the 'command not found' error message.
Redirect stdout and stderr to /dev/null.
Cc: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
configure |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/configure b/configure
index e67d076..2d06d87 100755
--- a/configure
+++ b/configure
@@ -21,6 +21,9 @@ trap rm -f $TMPC $TMPO $TMPE EXIT INT QUIT TERM
rm -f config.log
compile_object() {
+ echo config.log
+ cat $TMPC config.log
+ echo config.log
echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC config.log
$cc $QEMU_CFLAGS -c -o $TMPO $TMPC config.log 21
}
@@ -28,6 +31,9 @@ compile_object() {
compile_prog() {
local_cflags=$1
local_ldflags=$2
+ echo config.log
+ cat $TMPC config.log
+ echo config.log
echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags
config.log
$cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags
config.log 21
}
@@ -2089,7 +2095,7 @@ int main(void) { spice_server_new(); return 0; }
EOF
spice_cflags=$($pkgconfig --cflags spice-protocol spice-server 2/dev/null)
spice_libs=$($pkgconfig --libs spice-protocol spice-server 2/dev/null)
- if $pkgconfig --atleast-version=0.5.3 spice-server \
+ if $pkgconfig --atleast-version=0.5.3 spice-server /dev/null 21 \
compile_prog $spice_cflags $spice_libs ; then
spice=yes
libs_softmmu=$libs_softmmu $spice_libs
--
1.7.1
[Qemu-devel] Re: [PATCH 5/6] Switch migrate_set_speed() to take an 'o' argument rather than a float.
On 10/07/2010 05:01 PM, jes.soren...@redhat.com wrote:
From: Jes Sorensenjes.soren...@redhat.com
Signed-off-by: Jes Sorensenjes.soren...@redhat.com
---
hmp-commands.hx |2 +-
migration.c |4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 81999aa..95bdb91 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -754,7 +754,7 @@ ETEXI
{
.name = migrate_set_speed,
-.args_type = value:f,
+.args_type = value:o,
.params = value,
.help = set maximum speed (in bytes) for migrations,
^^^
Is this still accurate?
Alternatively, you said it's a mess to distinguish the defaults on the
command line and the monitor, but is it harder than the attached patch?
Paolo
diff --git a/cutils.c b/cutils.c
index 012eb11..4a9ce0f 100644
--- a/cutils.c
+++ b/cutils.c
@@ -289,13 +289,13 @@ int fcntl_setfl(int fd, int flag)
* End pointer will be returned in *end, if end is valid.
* Return -1 on error.
*/
-ssize_t strtosz(const char *nptr, char **end)
+ssize_t strtosz(const char *nptr, char **end, int default_shift)
{
ssize_t retval = -1;
-int64_t tmpval;
+int64_t tmpval, mul;
char *endptr;
int mul_required = 0;
-double val, mul = 1;
+double val;
endptr = (char *)nptr + strspn(nptr, 0123456789);
if (*endptr == '.') {
@@ -308,15 +308,14 @@ ssize_t strtosz(const char *nptr, char **end)
goto fail;
switch (*endptr++) {
+case 0:
+case ' ':
+mul = (1ULL default_shift);
+break;
case 'K':
case 'k':
mul = 1 10;
break;
-case 0:
-case ' ':
-if (mul_required) {
-goto fail;
-}
case 'M':
case 'm':
mul = 1ULL 20;
@@ -333,6 +332,9 @@ ssize_t strtosz(const char *nptr, char **end)
goto fail;
}
+if (mul == 1 mul_required) {
+goto fail;
+}
tmpval = (val * mul);
if (tmpval ~(size_t)0)
goto fail;
diff --git a/monitor.c b/monitor.c
index 6678fb5..0d04b27 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3713,7 +3713,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor
*mon,
break;
}
}
-val = strtosz(p, end);
+val = strtosz(p, end, 0);
if (val 0) {
monitor_printf(mon, invalid size\n);
goto fail;
diff --git a/qemu-common.h b/qemu-common.h
index 0a062d4..cabd8f8 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -153,7 +153,7 @@ time_t mktimegm(struct tm *tm);
int qemu_fls(int i);
int qemu_fdatasync(int fd);
int fcntl_setfl(int fd, int flag);
-ssize_t strtosz(const char *nptr, char **end);
+ssize_t strtosz(const char *nptr, char **end, int default_shift);
/* path.c */
void init_paths(const char *prefix);
diff --git a/vl.c b/vl.c
index 6043fa2..ae4d6f3 100644
--- a/vl.c
+++ b/vl.c
@@ -735,7 +735,7 @@ static void numa_add(const char *optarg)
node_mem[nodenr] = 0;
} else {
ssize_t sval;
-sval = strtosz(option, NULL);
+sval = strtosz(option, NULL, 20);
if (sval 0) {
fprintf(stderr, qemu: invalid numa mem size: %s\n, optarg);
exit(1);
@@ -2162,7 +2162,7 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_m: {
ssize_t value;
-value = strtosz(optarg, NULL);
+value = strtosz(optarg, NULL, 20);
if (value 0) {
fprintf(stderr, qemu: invalid ram size: %s\n, optarg);
exit(1);
[Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate)
Avi, Marcelo, Assuming this gets merged to qemu.git, you'll need the attached trivial updates for the qemu-kvm.git merge. Thanks, Alex On Wed, 2010-10-06 at 14:58 -0600, Alex Williamson wrote: Our code paths for saving or migrating a VM are full of functions that return void, leaving no opportunity for a device to cancel a migration, either from error or incompatibility. The ivshmem driver attempted to solve this with a no_migrate flag on the save state entry. I think the more generic and flexible way to solve this is to allow driver save functions to fail. This series implements that and converts ivshmem to uses a set_params function to NAK migration much earlier in the processes. This touches a lot of files, but bulk of those changes are simply s/void/int/ and tacking a return 0 to the end of functions. Thanks, Alex --- Alex Williamson (6): savevm: Remove register_device_unmigratable() savevm: Allow set_params and save_live_state to error virtio: Allow virtio_save() errors pci: Allow pci_device_save() to return error savevm: Allow vmsd-pre_save to return error savevm: Allow SaveStateHandler() to return error block-migration.c |4 +- hw/adb.c|8 +++- hw/ads7846.c|4 +- hw/arm_gic.c|4 +- hw/arm_timer.c |6 ++- hw/armv7m_nvic.c|4 +- hw/cuda.c |4 +- hw/fdc.c|3 + hw/g364fb.c |4 +- hw/grackle_pci.c|4 +- hw/gt64xxx.c|4 +- hw/heathrow_pic.c |4 +- hw/hpet.c |3 + hw/hw.h | 12 ++ hw/i2c.c|3 + hw/ide/core.c |4 +- hw/ivshmem.c| 30 +++ hw/lsi53c895a.c |4 +- hw/m48t59.c |4 +- hw/mac_dbdma.c |4 +- hw/mac_nvram.c |4 +- hw/max111x.c|4 +- hw/mipsnet.c|4 +- hw/mst_fpga.c |3 + hw/nand.c |3 + hw/openpic.c|4 +- hw/pci.c|9 +++- hw/pci.h|2 - hw/piix4.c |4 +- hw/pl011.c |4 +- hw/pl022.c |4 +- hw/pl061.c |4 +- hw/ppc4xx_pci.c | 11 - hw/ppce500_pci.c| 11 - hw/pxa2xx.c | 28 ++ hw/pxa2xx_dma.c |4 +- hw/pxa2xx_gpio.c|4 +- hw/pxa2xx_keypad.c |3 + hw/pxa2xx_lcd.c |4 +- hw/pxa2xx_mmci.c|4 +- hw/pxa2xx_pic.c |4 +- hw/pxa2xx_timer.c |4 +- hw/rc4030.c |4 +- hw/rtl8139.c|4 +- hw/serial.c |3 + hw/spitz.c | 14 +-- hw/ssd0323.c|4 +- hw/ssi-sd.c |4 +- hw/stellaris.c | 20 +++--- hw/stellaris_enet.c |4 +- hw/stellaris_input.c|4 +- hw/syborg_fb.c |4 +- hw/syborg_interrupt.c |3 + hw/syborg_keyboard.c|3 + hw/syborg_pointer.c |3 + hw/syborg_rtc.c |4 +- hw/syborg_serial.c |4 +- hw/syborg_timer.c |4 +- hw/tsc2005.c|4 +- hw/tsc210x.c|4 +- hw/twl92230.c |3 + hw/unin_pci.c |4 +- hw/usb-uhci.c |3 + hw/virtio-balloon.c |9 +++- hw/virtio-blk.c | 10 - hw/virtio-net.c | 11 - hw/virtio-pci.c | 10 - hw/virtio-serial-bus.c | 10 - hw/virtio.c | 14 +-- hw/virtio.h |4 +- hw/wm8750.c |3 + hw/zaurus.c |4 +- qemu-common.h |2 - savevm.c| 88 +++ slirp/slirp.c |6 ++- target-arm/machine.c|3 + target-cris/machine.c |3 + target-i386/machine.c |7 ++- target-microblaze/machine.c |3 + target-mips/machine.c |3 + target-ppc/machine.c|3 + target-s390x/machine.c |3 + target-sparc/machine.c |3 + 83 files changed, 365 insertions(+), 181 deletions(-) kvm: update for: savevm: Allow SaveStateHandler() to return error From: Alex Williamson alex.william...@redhat.com Signed-off-by: Alex Williamson alex.william...@redhat.com --- kvm-tpr-opt.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/kvm-tpr-opt.c b/kvm-tpr-opt.c index 46890e2..632b6d6
[Qemu-devel] [PATCH] win32: Set unbuffered stdout
Win32 does not support line-buffering, but it allows
unbuffered output.
Unbuffered output is a good approximation. For typical output
statements which usually end with '\n', it's even identical.
Buffered output is unusable for program traces because of
its large delay.
Cc: Blue Swirl blauwir...@gmail.com
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
os-win32.c |6 ++
qemu-os-win32.h |3 +--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/os-win32.c b/os-win32.c
index dd46bf4..3c6f50f 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -221,6 +221,12 @@ char *os_find_datadir(const char *argv0)
return NULL;
}
+void os_set_line_buffering(void)
+{
+setbuf(stdout, NULL);
+setbuf(stderr, NULL);
+}
+
/*
* Parse OS specific command line options.
* return 0 if option handled, -1 otherwise
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index 2ff9f45..c63778d 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -45,8 +45,7 @@ void os_host_main_loop_wait(int *timeout);
static inline void os_setup_signal_handling(void) {}
static inline void os_daemonize(void) {}
static inline void os_setup_post(void) {}
-/* Win32 doesn't support line-buffering and requires size = 2 */
-static inline void os_set_line_buffering(void) {}
+void os_set_line_buffering(void);
static inline void os_set_proc_name(const char *dummy) {}
#if !defined(EPROTONOSUPPORT)
--
1.7.1
[Qemu-devel] Re: [PATCH 05/11] Delete write only variables
On Thu, Oct 7, 2010 at 7:27 AM, Paolo Bonzini pbonz...@redhat.com wrote: On 10/06/2010 11:32 PM, Blue Swirl wrote: @@ -91,8 +90,6 @@ static int vga_osi_call (CPUState *env) env-gpr[3] = 0; break; case 39: /* video_ctrl */ - if (env-gpr[6] == 0 || env-gpr[6] == 1) - vga_vbl_enabled = env-gpr[6]; env-gpr[3] = 0; break; case 47: NACK, this smells like an incomplete emulation. Why would we care about unfinished temporary Mac-on-Linux hack for video.x? Perhaps the correct fix is to remove vga_osi_call function and env-osi_call field entirely.
Re: [Qemu-devel] Load primitive in linux-user/signal.c
Mulyadi Santosa writes: I know nothing about your patch, but I suggest to rewrite this post so it follows patch submission format e.g comment, signed off then the patch itself :) No offense, ok? :) None taken. I've sent it again with (I suppose) the adequate format. Lluis -- And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer. -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
[Qemu-devel] [PATCH] Use '_raw' memory access primitives.
Using a pointer on the host should not go through lduw. Signed-off-by: Lluís xscr...@gmx.net --- linux-user/signal.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index 77683f7..097da9d 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -982,8 +982,8 @@ restore_sigcontext(CPUX86State *env, struct target_sigcontext *sc, int *peax) env-regs[R_ECX] = tswapl(sc-ecx); env-eip = tswapl(sc-eip); -cpu_x86_load_seg(env, R_CS, lduw(sc-cs) | 3); -cpu_x86_load_seg(env, R_SS, lduw(sc-ss) | 3); +cpu_x86_load_seg(env, R_CS, lduw_raw(sc-cs) | 3); +cpu_x86_load_seg(env, R_SS, lduw_raw(sc-ss) | 3); tmpflags = tswapl(sc-eflags); env-eflags = (env-eflags ~0x40DD5) | (tmpflags 0x40DD5); -- 1.7.1 -- And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer. -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] Re: [PATCH 09/11] i386: avoid a write only variable
On Thu, Oct 7, 2010 at 8:27 AM, malc av1...@comtv.ru wrote: On Thu, 7 Oct 2010, Paolo Bonzini wrote: On 10/06/2010 11:34 PM, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-i386/op_helper.c: In function 'switch_tss': /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' set but not used [-Werror=unused-but-set-variable] Fix by deleting the variable. Again, this warning tells us the emulation is incorrect, so it's wrong to remove it. http://support.amd.com/us/Processor_TechDocs/24593.pdf 12.2.5 T (Trap) Bit--■Bit 0 of byte 64h, static field. This bit, when set to1, causes a debug exception (#DB) to occur on a task switch. See Beakpoint Instruction (INT3) page 340 for additional information. Yes, I read that before making the patch. But page 340 didn't seem to give any relevant information.
[Qemu-devel] Re: [PATCH 09/11] i386: avoid a write only variable
On Thu, Oct 7, 2010 at 7:27 AM, Paolo Bonzini pbonz...@redhat.com wrote: On 10/06/2010 11:34 PM, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-i386/op_helper.c: In function 'switch_tss': /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' set but not used [-Werror=unused-but-set-variable] Fix by deleting the variable. Again, this warning tells us the emulation is incorrect, so it's wrong to remove it. The warning tells us that the emulation is unfinished. It's probably unfinished because debugging TSS switches hasn't ever been needed by anyone. If anyone in the future cares enough about T bit, it's easy to implement it even after this part has been removed. But if someone proposes a better patch, I'd be happy to use that instead of this.
Re: [Qemu-devel] Re: [PATCH 09/11] i386: avoid a write only variable
On Thu, 7 Oct 2010, Blue Swirl wrote: On Thu, Oct 7, 2010 at 8:27 AM, malc av1...@comtv.ru wrote: On Thu, 7 Oct 2010, Paolo Bonzini wrote: On 10/06/2010 11:34 PM, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-i386/op_helper.c: In function 'switch_tss': /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' set but not used [-Werror=unused-but-set-variable] Fix by deleting the variable. Again, this warning tells us the emulation is incorrect, so it's wrong to remove it. http://support.amd.com/us/Processor_TechDocs/24593.pdf 12.2.5 T (Trap) Bit--■Bit 0 of byte 64h, static field. This bit, when set to1, causes a debug exception (#DB) to occur on a task switch. See Beakpoint Instruction (INT3) page 340 for additional information. Yes, I read that before making the patch. But page 340 didn't seem to give any relevant information. 13.2.4 Breakpoint Instruction (INT3) Is what was referred to apparently. -- mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH 03/11] eepro100: initialize a variable in all cases
Am 07.10.2010 11:31, schrieb Markus Armbruster: Blue Swirl blauwir...@gmail.com writes: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/hw/eepro100.c: In function 'eepro100_read4': /src/qemu/hw/eepro100.c:1351:14: error: 'val' may be used uninitialized in this function [-Werror=uninitialized] /src/qemu/hw/eepro100.c: In function 'eepro100_read2': /src/qemu/hw/eepro100.c:1328:14: error: 'val' may be used uninitialized in this function [-Werror=uninitialized] /src/qemu/hw/eepro100.c: In function 'eepro100_read1': /src/qemu/hw/eepro100.c:1285:13: error: 'val' may be used uninitialized in this function [-Werror=uninitialized] Fix by initializing 'val' at start. I'm worried this sweeps bugs under the carpet. When addr is out of bounds, these function return garbage. Your patch makes them return 0 instead. Can that happen? Shouldn't we catch and flag it? We should. I'll test new code which uses an assertion instead of the if statements, so a new patch might be ready until end of next week. Blue Swirl's patch does no harm, so it could be applied nevertheless if compiler warnings should be fixed now (I had the same kind of patch in my queue). Stefan
Re: [Qemu-devel] [PATCH 03/11] eepro100: initialize a variable in all cases
On Thu, Oct 7, 2010 at 9:31 AM, Markus Armbruster arm...@redhat.com wrote: Blue Swirl blauwir...@gmail.com writes: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/hw/eepro100.c: In function 'eepro100_read4': /src/qemu/hw/eepro100.c:1351:14: error: 'val' may be used uninitialized in this function [-Werror=uninitialized] /src/qemu/hw/eepro100.c: In function 'eepro100_read2': /src/qemu/hw/eepro100.c:1328:14: error: 'val' may be used uninitialized in this function [-Werror=uninitialized] /src/qemu/hw/eepro100.c: In function 'eepro100_read1': /src/qemu/hw/eepro100.c:1285:13: error: 'val' may be used uninitialized in this function [-Werror=uninitialized] Fix by initializing 'val' at start. I'm worried this sweeps bugs under the carpet. When addr is out of bounds, these function return garbage. Your patch makes them return 0 instead. Can that happen? Shouldn't we catch and flag it? 0 is a nice default value. Adding an assert is not OK, guest shouldn't be able to terminate QEMU. Adding a check which spams stdio is not so nice either. I think it's even impossible with current QEMU for addr to be out of bounds, the area is registered with size PCI_MEM_SIZE.
Re: [Qemu-devel] [PATCH 07/11] cris: avoid write only variables
On Thu, Oct 7, 2010 at 11:07 AM, Edgar E. Iglesias
edgar.igles...@gmail.com wrote:
On Thu, Oct 07, 2010 at 12:08:05PM +0200, Markus Armbruster wrote:
Blue Swirl blauwir...@gmail.com writes:
Compiling with GCC 4.6.0 20100925 produced warnings:
/src/qemu/target-cris/op_helper.c: In function 'helper_movl_sreg_reg':
/src/qemu/target-cris/op_helper.c:145:8: error: variable 'tlb_v' set
but not used [-Werror=unused-but-set-variable]
In file included from /src/qemu/target-cris/translate.c:3154:0:
/src/qemu/target-cris/translate_v10.c: In function 'dec10_prep_move_m':
/src/qemu/target-cris/translate_v10.c:111:22: error: variable 'rd' set
but not used [-Werror=unused-but-set-variable]
Fix by making the variable declarations and their uses also conditional
to debug definition, delete rd.
Signed-off-by: Blue Swirl blauwir...@gmail.com
---
target-cris/op_helper.c | 6 ++
target-cris/translate_v10.c | 5 ++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index a60da94..28c79b1 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -142,7 +142,9 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg)
uint32_t idx;
uint32_t lo, hi;
uint32_t vaddr;
+#ifdef CRIS_OP_HELPER_DEBUG
int tlb_v;
+#endif
idx = set = env-sregs[SFR_RW_MM_TLB_SEL];
set = 4;
@@ -157,13 +159,17 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t
reg)
vaddr = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].hi,
13, 31);
vaddr = TARGET_PAGE_BITS;
+#ifdef CRIS_OP_HELPER_DEBUG
tlb_v = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].lo,
3, 3);
+#endif
env-tlbsets[srs - 1][set][idx].lo = lo;
env-tlbsets[srs - 1][set][idx].hi = hi;
+#ifdef CRIS_OP_HELPER_DEBUG
D_LOG(tlb flush vaddr=%x v=%d pc=%x\n,
vaddr, tlb_v, env-pc);
+#endif
tlb_flush_page(env, vaddr);
}
}
Could we eliminate the bothersome variable instead?
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index a60da94..94e3e27 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -142,7 +142,6 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg)
uint32_t idx;
uint32_t lo, hi;
uint32_t vaddr;
- int tlb_v;
idx = set = env-sregs[SFR_RW_MM_TLB_SEL];
set = 4;
@@ -157,13 +156,14 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg)
vaddr = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].hi,
13, 31);
vaddr = TARGET_PAGE_BITS;
- tlb_v = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].lo,
+ D_LOG(tlb flush vaddr=%x v=%d pc=%x\n,
+ vaddr,
+
EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].lo,
3, 3);
+ env-pc);
env-tlbsets[srs - 1][set][idx].lo = lo;
env-tlbsets[srs - 1][set][idx].hi = hi;
- D_LOG(tlb flush vaddr=%x v=%d pc=%x\n,
- vaddr, tlb_v, env-pc);
tlb_flush_page(env, vaddr);
}
}
Hi,
Sorry for the late answer.
Late? I think my timescale is slightly different than yours :-)
Markus, I agree that removing tlb_v would have been better than ifdefs,
but i think that the intent I originally had in mind was that there should
not be a need to flush the entry from the QEMU TLB if the old guest
entry was not valid.
The following patch works on my side:
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index a60da94..be9eb06 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -164,7 +164,9 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg)
D_LOG(tlb flush vaddr=%x v=%d pc=%x\n,
vaddr, tlb_v, env-pc);
- tlb_flush_page(env, vaddr);
+ if (tlb_v) {
+ tlb_flush_page(env, vaddr);
+ }
}
}
#endif
The target-cris/translate_v10.c hunk looks good.
Blue, can you incorporate the tlb_v change in your patch set?
Or if you prefer, I can commit that part on my side.
Please do.
Re: [Qemu-devel] [PATCH 2/5] spice: make compression configurable.
On Thu, Oct 7, 2010 at 7:55 AM, Gerd Hoffmann kra...@redhat.com wrote:
From: Yonit Halperin yhalp...@redhat.com
No description?
---
qemu-config.c | 9 ++
qemu-options.hx | 9 ++
ui/spice-core.c | 77 +-
3 files changed, 93 insertions(+), 2 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 26748a5..8b545b1 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -391,6 +391,15 @@ QemuOptsList qemu_spice_opts = {
},{
.name = tls-ciphers,
.type = QEMU_OPT_STRING,
+ },{
+ .name = image-compression,
+ .type = QEMU_OPT_STRING,
+ },{
+ .name = jpeg-wan-compression,
+ .type = QEMU_OPT_STRING,
+ },{
+ .name = zlib-glz-wan-compression,
+ .type = QEMU_OPT_STRING,
},
{ /* end if list */ }
},
diff --git a/qemu-options.hx b/qemu-options.hx
index 9d3f8ef..59db632 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -704,6 +704,15 @@ The x509 file names can also be configured individually.
�...@item tls-ciphers=list
Specify which ciphers to use.
+...@item image-compression=[auto_glz|auto_lz|quic|glz|lz|off]
+Configure image compression (lossless).
+Default is auto_glz.
+
+...@item jpeg-wan-compression=[auto|never|allways]
+...@item zlib-glz-wan-compression=[auto|never|allways]
'allways' does not match what the code uses:
+ [ SPICE_WAN_COMPRESSION_ALWAYS ] = always,
[Qemu-devel] Re: [PATCH 08/11] vnc: avoid write only variables
On Thu, Oct 7, 2010 at 7:23 AM, Paolo Bonzini pbonz...@redhat.com wrote:
On 10/06/2010 11:33 PM, Blue Swirl wrote:
+#if defined(CONFIG_VNC_TLS) || defined(CONFIG_VNC_SASL)
} else if (strncmp(options, acl, 3) == 0) {
acl = 1;
+#endif
Not sure it's okay to reject the option altogether (i.e. maybe the #if
should only include acl = 1.
It's OK with current code, it doesn't check for invalid options. This
also matches how other options are handled.
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
On Thu, Oct 7, 2010 at 7:12 AM, Anthony Liguori anth...@codemonkey.ws wrote:
On 08/03/2010 03:14 PM, Christian Brunner wrote:
+#include qemu-common.h
+#include qemu-error.h
+#includesys/types.h
+#includestdbool.h
+
+#includeqemu-common.h
This looks to be unnecessary. Generally, system includes shouldn't be
required so all of these should go away except rado/librados.h
Removed.
+
+#include rbd_types.h
+#include module.h
+#include block_int.h
+
+#includestdio.h
+#includestdlib.h
+#includerados/librados.h
+
+#includesignal.h
+
+
+int eventfd(unsigned int initval, int flags);
This is not quite right. Depending on eventfd is curious but in the very
least, you need to detect the presence of eventfd in configure and provide a
wrapper that redefines it as necessary.
Can fix that, though please see my later remarks.
+static int create_tmap_op(uint8_t op, const char *name, char **tmap_desc)
+{
+ uint32_t len = strlen(name);
+ /* total_len = encoding op + name + empty buffer */
+ uint32_t total_len = 1 + (sizeof(uint32_t) + len) + sizeof(uint32_t);
+ char *desc = NULL;
char is the wrong type to use here as it may be signed or unsigned. That
can have weird effects with binary data when you're directly manipulating
it.
Well, I can change it to uint8_t, so that it matches the op type, but
that'll require adding some other castings. In any case, you usually
get such a weird behavior when you cast to types of different sizes
and have the sign bit padded which is not the case in here.
+
+ desc = qemu_malloc(total_len);
+
+ *tmap_desc = desc;
+
+ *desc = op;
+ desc++;
+ memcpy(desc,len, sizeof(len));
+ desc += sizeof(len);
+ memcpy(desc, name, len);
+ desc += len;
+ len = 0;
+ memcpy(desc,len, sizeof(len));
+ desc += sizeof(len);
Shouldn't endianness be a concern?
Right. Fixed that.
+
+ return desc - *tmap_desc;
+}
+
+static void free_tmap_op(char *tmap_desc)
+{
+ qemu_free(tmap_desc);
+}
+
+static int rbd_register_image(rados_pool_t pool, const char *name)
+{
+ char *tmap_desc;
+ const char *dir = RBD_DIRECTORY;
+ int ret;
+
+ ret = create_tmap_op(CEPH_OSD_TMAP_SET, name,tmap_desc);
+ if (ret 0) {
+ return ret;
+ }
+
+ ret = rados_tmap_update(pool, dir, tmap_desc, ret);
+ free_tmap_op(tmap_desc);
+
+ return ret;
+}
This ops are all synchronous? IOW, rados_tmap_update() call blocks until
the operation is completed?
Yeah. And this is only called from the rbd_create() callback.
+ header_snap += strlen(header_snap) + 1;
+ if (header_snap end)
+ error_report(bad header, snapshot list broken);
Missing curly braces here.
Fixed.
+ if (strncmp(hbuf + 68, RBD_HEADER_VERSION, 8)) {
+ error_report(Unknown image version %s, hbuf + 68);
+ r = -EMEDIUMTYPE;
+ goto failed;
+ }
+
+ RbdHeader1 *header;
Don't mix variable definitions with code.
Fixed.
+ s-efd = eventfd(0, 0);
+ if (s-efd 0) {
+ error_report(error opening eventfd);
+ goto failed;
+ }
+ fcntl(s-efd, F_SETFL, O_NONBLOCK);
+ qemu_aio_set_fd_handler(s-efd, rbd_aio_completion_cb, NULL,
+ rbd_aio_flush_cb, NULL, s);
It looks like you just use the eventfd to signal aio completion callbacks.
A better way to do this would be to schedule a bottom half. eventfds are
Linux specific and specific to recent kernels.
Digging back why we introduced the eventfd, it was due to some issues
seen with do_savevm() hangs on qemu_aio_flush(). The reason seemed
that we had no fd associated with the block device, which seemed to
not work well with the qemu aio model. If that assumption is wrong,
we'd be happy to change it. In any case, there are other more portable
ways to generate fds, so if it's needed we can do that.
+static int rbd_rw(BlockDriverState *bs, int64_t sector_num,
+ uint8_t *buf, int nb_sectors, int write)
+{
+ BDRVRBDState *s = bs-opaque;
+ char n[RBD_MAX_SEG_NAME_SIZE];
+
You don't need to implement synchronous functions as long as you have the
async interfaces implemented.
Snipped.
+ */
+ if (sn_info-id_str[0] != '\0'
+ strcmp(sn_info-id_str, sn_info-name) != 0)
+ return -EINVAL;
I don't fully understand. Does this mean that snapshots are stored in a
shared namespace? IOW, if a user root creates a snapshot of in one VM, the
other VM running as root sees it too?
Snapshots are stored in a namespace for each block device. If you
share a block device between different vms, you'll also share its
snapshots.
Thanks,
Yehuda
Re: [Qemu-devel] [PATCH 01/11] block: avoid a write only variable
On Thu, Oct 7, 2010 at 11:55 AM, Kevin Wolf kw...@redhat.com wrote: Am 07.10.2010 11:37, schrieb Markus Armbruster: Blue Swirl blauwir...@gmail.com writes: Compiling with GCC 4.6.0 20100925 produced a warning: /src/qemu/block/qcow2-refcount.c: In function 'update_refcount': /src/qemu/block/qcow2-refcount.c:552:13: error: variable 'dummy' set but not used [-Werror=unused-but-set-variable] Fix by adding a function that does not generate a warning when the result is unused. What about a simple (void)dummy instead? I would prefer that, too. That also works. I'll use that in the next version.
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
On 10/07/2010 01:08 PM, Yehuda Sadeh Weinraub wrote:
On Thu, Oct 7, 2010 at 7:12 AM, Anthony Liguorianth...@codemonkey.ws wrote:
On 08/03/2010 03:14 PM, Christian Brunner wrote:
+#include qemu-common.h
+#include qemu-error.h
+#includesys/types.h
+#includestdbool.h
+
+#includeqemu-common.h
This looks to be unnecessary. Generally, system includes shouldn't be
required so all of these should go away except rado/librados.h
Removed.
+
+#include rbd_types.h
+#include module.h
+#include block_int.h
+
+#includestdio.h
+#includestdlib.h
+#includerados/librados.h
+
+#includesignal.h
+
+
+int eventfd(unsigned int initval, int flags);
This is not quite right. Depending on eventfd is curious but in the very
least, you need to detect the presence of eventfd in configure and provide a
wrapper that redefines it as necessary.
Can fix that, though please see my later remarks.
+static int create_tmap_op(uint8_t op, const char *name, char **tmap_desc)
+{
+uint32_t len = strlen(name);
+/* total_len = encoding op + name + empty buffer */
+uint32_t total_len = 1 + (sizeof(uint32_t) + len) + sizeof(uint32_t);
+char *desc = NULL;
char is the wrong type to use here as it may be signed or unsigned. That
can have weird effects with binary data when you're directly manipulating
it.
Well, I can change it to uint8_t, so that it matches the op type, but
that'll require adding some other castings. In any case, you usually
get such a weird behavior when you cast to types of different sizes
and have the sign bit padded which is not the case in here.
+
+desc = qemu_malloc(total_len);
+
+*tmap_desc = desc;
+
+*desc = op;
+desc++;
+memcpy(desc,len, sizeof(len));
+desc += sizeof(len);
+memcpy(desc, name, len);
+desc += len;
+len = 0;
+memcpy(desc,len, sizeof(len));
+desc += sizeof(len);
Shouldn't endianness be a concern?
Right. Fixed that.
+
+return desc - *tmap_desc;
+}
+
+static void free_tmap_op(char *tmap_desc)
+{
+qemu_free(tmap_desc);
+}
+
+static int rbd_register_image(rados_pool_t pool, const char *name)
+{
+char *tmap_desc;
+const char *dir = RBD_DIRECTORY;
+int ret;
+
+ret = create_tmap_op(CEPH_OSD_TMAP_SET, name,tmap_desc);
+if (ret0) {
+return ret;
+}
+
+ret = rados_tmap_update(pool, dir, tmap_desc, ret);
+free_tmap_op(tmap_desc);
+
+return ret;
+}
This ops are all synchronous? IOW, rados_tmap_update() call blocks until
the operation is completed?
Yeah. And this is only called from the rbd_create() callback.
+header_snap += strlen(header_snap) + 1;
+if (header_snapend)
+error_report(bad header, snapshot list broken);
Missing curly braces here.
Fixed.
+if (strncmp(hbuf + 68, RBD_HEADER_VERSION, 8)) {
+error_report(Unknown image version %s, hbuf + 68);
+r = -EMEDIUMTYPE;
+goto failed;
+}
+
+RbdHeader1 *header;
Don't mix variable definitions with code.
Fixed.
+s-efd = eventfd(0, 0);
+if (s-efd0) {
+error_report(error opening eventfd);
+goto failed;
+}
+fcntl(s-efd, F_SETFL, O_NONBLOCK);
+qemu_aio_set_fd_handler(s-efd, rbd_aio_completion_cb, NULL,
+rbd_aio_flush_cb, NULL, s);
It looks like you just use the eventfd to signal aio completion callbacks.
A better way to do this would be to schedule a bottom half. eventfds are
Linux specific and specific to recent kernels.
Digging back why we introduced the eventfd, it was due to some issues
seen with do_savevm() hangs on qemu_aio_flush(). The reason seemed
that we had no fd associated with the block device, which seemed to
not work well with the qemu aio model. If that assumption is wrong,
we'd be happy to change it. In any case, there are other more portable
ways to generate fds, so if it's needed we can do that.
There's no fd at all? How do you get notifications about an
asynchronous event completion?
Regards,
Anthony Liguori
Re: [Qemu-devel] Re: [PATCH 04/11] lsi53c895a: avoid a write only variable
On Thu, Oct 7, 2010 at 9:20 AM, Markus Armbruster arm...@redhat.com wrote: Paolo Bonzini pbonz...@redhat.com writes: On 10/06/2010 11:32 PM, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced a warning: /src/qemu/hw/lsi53c895a.c: In function 'lsi_do_msgout': /src/qemu/hw/lsi53c895a.c:848:9: error: variable 'len' set but not used [-Werror=unused-but-set-variable] Fix by making the variable declaration and its uses also conditional to debug definition. NACK, this uglifies the code and loses track of _what_ is that msgbyte we're reading. That information could be saved by adding a comment, I used that approach for vmstate.c. Seconded. If the warning bothers you, maybe (void)len can silence it in a less unsightly manner. I'll use that approach next.
Re: [Qemu-devel] [PATCH 3/5] spice: add config options for channel security.
Am 07.10.2010 09:55, schrieb Gerd Hoffmann:
This allows to enforce tls or plaintext usage for certain spice
channels.
---
qemu-config.c |6 ++
qemu-options.hx |8
ui/spice-core.c | 25 +
3 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 8b545b1..f52e50c 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -392,6 +392,12 @@ QemuOptsList qemu_spice_opts = {
.name = tls-ciphers,
.type = QEMU_OPT_STRING,
},{
+.name = tls-channel,
+.type = QEMU_OPT_STRING,
+},{
+.name = plaintext-channel,
+.type = QEMU_OPT_STRING,
+},{
.name = image-compression,
.type = QEMU_OPT_STRING,
},{
diff --git a/qemu-options.hx b/qemu-options.hx
index 59db632..bb45b67 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -704,6 +704,14 @@ The x509 file names can also be configured individually.
@item tls-ciphers=list
Specify which ciphers to use.
+...@item tls-channel=[main|display|inputs|record|playback|tunnel]
+...@item plaintext-channel=[main|display|inputs|record|playback|tunnel]
+Force specific channel to be used with or without TLS encryption. The
+options can be specified multiple times to configure multiple
+channels. The special name default can be used to set the default
+mode. For channels which are not explicitly forced into one mode the
+spice client is allowed to pick tls/plaintext as he pleases.
+
@item image-compression=[auto_glz|auto_lz|quic|glz|lz|off]
Configure image compression (lossless).
Default is auto_glz.
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 1567046..8f73848 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -192,6 +192,29 @@ static const char *wan_compression_names[] = {
/* functions for the rest of qemu */
+static int add_channel(const char *name, const char *value, void *opaque)
+{
+int security = 0;
+int rc;
+
+if (strcmp(name, tls-channel) == 0)
+security = SPICE_CHANNEL_SECURITY_SSL;
CODING_STYLE (if (...) { ... })? Same in next lines.
+if (strcmp(name, plaintext-channel) == 0)
+security = SPICE_CHANNEL_SECURITY_NONE;
+if (security == 0)
+return 0;
+if (strcmp(value, default) == 0) {
+rc = spice_server_set_channel_security(spice_server, NULL, security);
+} else {
+rc = spice_server_set_channel_security(spice_server, value, security);
+}
+if (rc != 0) {
+fprintf(stderr, spice: failed to set channel security for %s\n,
value);
+exit(1);
+}
+return 0;
+}
+
void qemu_spice_init(void)
{
QemuOpts *opts = QTAILQ_FIRST(qemu_spice_opts.head);
@@ -293,6 +316,8 @@ void qemu_spice_init(void)
}
spice_server_set_zlib_glz_compression(spice_server, wan_compr);
+qemu_opt_foreach(opts, add_channel, NULL, 0);
+
spice_server_init(spice_server,core_interface);
using_spice = 1;
[Qemu-devel] [STATUS] static instrumentation
All virtual memory accesses should now be instrumented on all architectures. Next steps (in order): * Separately instrument physical memory addresses for executed instructions, regular memory accesses and memory accesses to I/O space (if possible). This will need to add an extra field on CPUTLBEntry with the physical address of the page. * Instrument memory accesses performed by DMA operations. * See how it plays with KVM. The objective is to make it switch from KVM to emulation (and the other way around) when a backdoor instruction is found. * Finish implementation of used/defined register usage in x86. As always: git clone https://code.gso.ac.upc.edu/git/qemu-instrument https://projects.gso.ac.upc.edu/projects/qemu-instrument Lluis -- And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer. -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
On Thu, Oct 7, 2010 at 11:38 AM, Anthony Liguori anth...@codemonkey.ws wrote:
On 10/07/2010 01:08 PM, Yehuda Sadeh Weinraub wrote:
On Thu, Oct 7, 2010 at 7:12 AM, Anthony Liguorianth...@codemonkey.ws
wrote:
On 08/03/2010 03:14 PM, Christian Brunner wrote:
+#include qemu-common.h
+#include qemu-error.h
+#includesys/types.h
+#includestdbool.h
+
+#includeqemu-common.h
This looks to be unnecessary. Generally, system includes shouldn't be
required so all of these should go away except rado/librados.h
Removed.
+
+#include rbd_types.h
+#include module.h
+#include block_int.h
+
+#includestdio.h
+#includestdlib.h
+#includerados/librados.h
+
+#includesignal.h
+
+
+int eventfd(unsigned int initval, int flags);
This is not quite right. Depending on eventfd is curious but in the very
least, you need to detect the presence of eventfd in configure and
provide a
wrapper that redefines it as necessary.
Can fix that, though please see my later remarks.
+static int create_tmap_op(uint8_t op, const char *name, char
**tmap_desc)
+{
+ uint32_t len = strlen(name);
+ /* total_len = encoding op + name + empty buffer */
+ uint32_t total_len = 1 + (sizeof(uint32_t) + len) +
sizeof(uint32_t);
+ char *desc = NULL;
char is the wrong type to use here as it may be signed or unsigned. That
can have weird effects with binary data when you're directly manipulating
it.
Well, I can change it to uint8_t, so that it matches the op type, but
that'll require adding some other castings. In any case, you usually
get such a weird behavior when you cast to types of different sizes
and have the sign bit padded which is not the case in here.
+
+ desc = qemu_malloc(total_len);
+
+ *tmap_desc = desc;
+
+ *desc = op;
+ desc++;
+ memcpy(desc,len, sizeof(len));
+ desc += sizeof(len);
+ memcpy(desc, name, len);
+ desc += len;
+ len = 0;
+ memcpy(desc,len, sizeof(len));
+ desc += sizeof(len);
Shouldn't endianness be a concern?
Right. Fixed that.
+
+ return desc - *tmap_desc;
+}
+
+static void free_tmap_op(char *tmap_desc)
+{
+ qemu_free(tmap_desc);
+}
+
+static int rbd_register_image(rados_pool_t pool, const char *name)
+{
+ char *tmap_desc;
+ const char *dir = RBD_DIRECTORY;
+ int ret;
+
+ ret = create_tmap_op(CEPH_OSD_TMAP_SET, name,tmap_desc);
+ if (ret 0) {
+ return ret;
+ }
+
+ ret = rados_tmap_update(pool, dir, tmap_desc, ret);
+ free_tmap_op(tmap_desc);
+
+ return ret;
+}
This ops are all synchronous? IOW, rados_tmap_update() call blocks until
the operation is completed?
Yeah. And this is only called from the rbd_create() callback.
+ header_snap += strlen(header_snap) + 1;
+ if (header_snap end)
+ error_report(bad header, snapshot list broken);
Missing curly braces here.
Fixed.
+ if (strncmp(hbuf + 68, RBD_HEADER_VERSION, 8)) {
+ error_report(Unknown image version %s, hbuf + 68);
+ r = -EMEDIUMTYPE;
+ goto failed;
+ }
+
+ RbdHeader1 *header;
Don't mix variable definitions with code.
Fixed.
+ s-efd = eventfd(0, 0);
+ if (s-efd 0) {
+ error_report(error opening eventfd);
+ goto failed;
+ }
+ fcntl(s-efd, F_SETFL, O_NONBLOCK);
+ qemu_aio_set_fd_handler(s-efd, rbd_aio_completion_cb, NULL,
+ rbd_aio_flush_cb, NULL, s);
It looks like you just use the eventfd to signal aio completion
callbacks.
A better way to do this would be to schedule a bottom half. eventfds
are
Linux specific and specific to recent kernels.
Digging back why we introduced the eventfd, it was due to some issues
seen with do_savevm() hangs on qemu_aio_flush(). The reason seemed
that we had no fd associated with the block device, which seemed to
not work well with the qemu aio model. If that assumption is wrong,
we'd be happy to change it. In any case, there are other more portable
ways to generate fds, so if it's needed we can do that.
There's no fd at all? How do you get notifications about an asynchronous
event completion?
Regards,
Anthony Liguori
(resending to list, sorry)
The fd is hidden deep under in librados. We get callback notifications
for events completion.
Thanks,
Yehuda
[Qemu-devel] Re: [PATCH] configure: Send error message from spice check to /dev/null
Am 07.10.2010 21:05, schrieb Gerd Hoffmann:
On 10/07/10 17:58, Stefan Weil wrote:
pkg-config is not always available (e.g. on win32 hosts),
but we don't want to see the 'command not found' error message.
compile_object() {
+ echoconfig.log
+ cat $TMPCconfig.log
+ echoconfig.log
This looks unrelated.
Ups, that was the wrong patch (some test code).
Sorry - I'll fix that immediately.
Thanks for your feedback.
@@ -28,6 +31,9 @@ compile_object() {
compile_prog() {
local_cflags=$1
local_ldflags=$2
+ echoconfig.log
+ cat $TMPCconfig.log
+ echoconfig.log
This too.
spice_cflags=$($pkgconfig --cflags spice-protocol spice-server
2/dev/null)
spice_libs=$($pkgconfig --libs spice-protocol spice-server
2/dev/null)
- if $pkgconfig --atleast-version=0.5.3 spice-server\
+ if $pkgconfig --atleast-version=0.5.3 spice-server/dev/null
21 \
Fine with me, the other pkgconfig calls are covered already, the third
missing is just an oversight and the fix is ObliviouslyCorrect[tm].
cheers,
Gerd
[Qemu-devel] [PATCH v2] configure: Send error message from spice check to /dev/null
pkg-config is not always available (e.g. on win32 hosts),
but we don't want to see the 'command not found' error message.
Redirect stdout and stderr to /dev/null.
v2:
* Removed changes which should not have been here.
Cc: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
configure |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/configure b/configure
index e67d076..cc19d82 100755
--- a/configure
+++ b/configure
@@ -2089,7 +2089,7 @@ int main(void) { spice_server_new(); return 0; }
EOF
spice_cflags=$($pkgconfig --cflags spice-protocol spice-server 2/dev/null)
spice_libs=$($pkgconfig --libs spice-protocol spice-server 2/dev/null)
- if $pkgconfig --atleast-version=0.5.3 spice-server \
+ if $pkgconfig --atleast-version=0.5.3 spice-server /dev/null 21 \
compile_prog $spice_cflags $spice_libs ; then
spice=yes
libs_softmmu=$libs_softmmu $spice_libs
--
1.7.1
Re: [Qemu-devel] [STATUS] static instrumentation
On Thu, Oct 7, 2010 at 6:40 PM, Lluís xscr...@gmx.net wrote: All virtual memory accesses should now be instrumented on all architectures. Next steps (in order): * Separately instrument physical memory addresses for executed instructions, regular memory accesses and memory accesses to I/O space (if possible). This will need to add an extra field on CPUTLBEntry with the physical address of the page. * Instrument memory accesses performed by DMA operations. * See how it plays with KVM. The objective is to make it switch from KVM to emulation (and the other way around) when a backdoor instruction is found. * Finish implementation of used/defined register usage in x86. As always: git clone https://code.gso.ac.upc.edu/git/qemu-instrument https://projects.gso.ac.upc.edu/projects/qemu-instrument The patches there seem to be fixes or small changes to your other patches. This makes any review very difficult. But I'm not sure the architecture is what we'd like to see integrated in QEMU, though I may have missed the whole picture because of the patch fragmentation problem. Before doing any work to possibly go further in wrong direction, please try to submit early some preview version for comments and review. Just as an example, perhaps the existing tracing framework could be extended to cover also your needs.
[Qemu-devel] Re: [PATCH] configure: Send error message from spice check to /dev/null
On 10/07/10 17:58, Stefan Weil wrote:
pkg-config is not always available (e.g. on win32 hosts),
but we don't want to see the 'command not found' error message.
compile_object() {
+ echoconfig.log
+ cat $TMPCconfig.log
+ echoconfig.log
This looks unrelated.
@@ -28,6 +31,9 @@ compile_object() {
compile_prog() {
local_cflags=$1
local_ldflags=$2
+ echoconfig.log
+ cat $TMPCconfig.log
+ echoconfig.log
This too.
spice_cflags=$($pkgconfig --cflags spice-protocol spice-server 2/dev/null)
spice_libs=$($pkgconfig --libs spice-protocol spice-server 2/dev/null)
- if $pkgconfig --atleast-version=0.5.3 spice-server\
+ if $pkgconfig --atleast-version=0.5.3 spice-server/dev/null 21 \
Fine with me, the other pkgconfig calls are covered already, the third
missing is just an oversight and the fix is ObliviouslyCorrect[tm].
cheers,
Gerd
[Qemu-devel] Re: [PATCH 10/11] ppc: avoid write only variables
On Wed, Oct 6, 2010 at 9:39 PM, Alexander Graf ag...@suse.de wrote: On 06.10.2010, at 23:34, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-ppc/op_helper.c: In function 'helper_icbi': /src/qemu/target-ppc/op_helper.c:351:14: error: variable 'tmp' set but not used [-Werror=unused-but-set-variable] /src/qemu/target-ppc/op_helper.c: In function 'do_6xx_tlb': /src/qemu/target-ppc/op_helper.c:3805:28: error: variable 'EPN' set but not used [-Werror=unused-but-set-variable] /src/qemu/target-ppc/op_helper.c: In function 'do_74xx_tlb': /src/qemu/target-ppc/op_helper.c:3838:28: error: variable 'EPN' set but not used [-Werror=unused-but-set-variable] Fix by making the variable declarations and their uses also conditional to debug definition. Delete tmp. Maybe it would make more sense to get those LOG_* macros into static inline functions. But for the issue at hand, the solution looks good to me. Perhaps all conditionally enabled debug printf stuff should be transformed into tracepoints? That would be more flexible than uncommenting DEBUG_foobar. Would it help with bitrot too? Signed-off-by: Alexander Graf ag...@suse.de Thanks.
Re: [Qemu-devel] [PATCH 2/5] spice: make compression configurable.
On 10/07/10 20:12, Blue Swirl wrote: On Thu, Oct 7, 2010 at 7:55 AM, Gerd Hoffmannkra...@redhat.com wrote: From: Yonit Halperinyhalp...@redhat.com No description? Detailed description comes here: --- a/qemu-options.hx +++ b/qemu-options.hx @@ -704,6 +704,15 @@ The x509 file names can also be configured individually. @item tls-ciphers=list Specify which ciphers to use. +...@item image-compression=[auto_glz|auto_lz|quic|glz|lz|off] +Configure image compression (lossless). +Default is auto_glz. + +...@item jpeg-wan-compression=[auto|never|allways] +...@item zlib-glz-wan-compression=[auto|never|allways] 'allways' does not match what the code uses: +[ SPICE_WAN_COMPRESSION_ALWAYS ] = always, Good catch, will fix. thanks, Gerd
Re: [Qemu-devel] [PATCH 3/5] spice: add config options for channel security.
+ if (strcmp(name, tls-channel) == 0)
+ security = SPICE_CHANNEL_SECURITY_SSL;
CODING_STYLE (if (...) { ... })? Same in next lines.
Oops. Slipped through, will fix.
thanks,
Gerd
[Qemu-devel] [ANNOUNCE] Git User's Survey 2010 is now up!
This might be interesting for users of qemu-devel, too... Original-Nachricht Betreff:[ANNOUNCE] Git User's Survey 2010 is now up! Datum: Thu, 30 Sep 2010 01:20:21 +0200 Von:Jakub Narebski jna...@gmail.com An: Undisclosed.Recipients: ; Hello all, This announcement is sent to this mailing list because yout project uses Git as a version control system. We would like to ask you a few questions about your use of the Git version control system. This survey is mainly to understand who is using Git, how and why. The results will be published to the Git wiki on the GitSurvey2010 page (https://git.wiki.kernel.org/index.php/GitSurvey2010) and discussed on the git mailing list (g...@vger.kernel.org). The survey would be open from 1 September till 15 October 2010. Please devote a few minutes of your time to fill this simple questionnaire, it will help a lot the git community to understand your needs, what you like of Git, and of course what you don't like of it. The survey can be found here: http://tinyurl.com/GitSurvey2010 https://www.survs.com/survey/MUPYR8UJ4B Git User's Survey 2010 was originally announced on git mailing list in the following mail: http://article.gmane.org/gmane.comp.version-control.git/154986 -- Jakub Narebski on behalf of Git Development Community http://git-scm.com ___ lilypond-devel mailing list lilypond-de...@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [Qemu-devel] [PATCH 07/11] cris: avoid write only variables
On Thu, Oct 07, 2010 at 05:57:30PM +, Blue Swirl wrote:
On Thu, Oct 7, 2010 at 11:07 AM, Edgar E. Iglesias
edgar.igles...@gmail.com wrote:
On Thu, Oct 07, 2010 at 12:08:05PM +0200, Markus Armbruster wrote:
Blue Swirl blauwir...@gmail.com writes:
Compiling with GCC 4.6.0 20100925 produced warnings:
[...]
Markus, I agree that removing tlb_v would have been better than ifdefs,
but i think that the intent I originally had in mind was that there should
not be a need to flush the entry from the QEMU TLB if the old guest
entry was not valid.
The following patch works on my side:
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index a60da94..be9eb06 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -164,7 +164,9 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg)
D_LOG(tlb flush vaddr=%x v=%d pc=%x\n,
vaddr, tlb_v, env-pc);
- tlb_flush_page(env, vaddr);
+ if (tlb_v) {
+ tlb_flush_page(env, vaddr);
+ }
}
}
#endif
The target-cris/translate_v10.c hunk looks good.
Blue, can you incorporate the tlb_v change in your patch set?
Or if you prefer, I can commit that part on my side.
Please do.
OK, done.
Thanks
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
On 10/07/2010 01:41 PM, Yehuda Sadeh Weinraub wrote:
On Thu, Oct 7, 2010 at 11:38 AM, Anthony Liguorianth...@codemonkey.ws wrote:
On 10/07/2010 01:08 PM, Yehuda Sadeh Weinraub wrote:
On Thu, Oct 7, 2010 at 7:12 AM, Anthony Liguorianth...@codemonkey.ws
wrote:
On 08/03/2010 03:14 PM, Christian Brunner wrote:
+#include qemu-common.h
+#include qemu-error.h
+#includesys/types.h
+#includestdbool.h
+
+#includeqemu-common.h
This looks to be unnecessary. Generally, system includes shouldn't be
required so all of these should go away except rado/librados.h
Removed.
+
+#include rbd_types.h
+#include module.h
+#include block_int.h
+
+#includestdio.h
+#includestdlib.h
+#includerados/librados.h
+
+#includesignal.h
+
+
+int eventfd(unsigned int initval, int flags);
This is not quite right. Depending on eventfd is curious but in the very
least, you need to detect the presence of eventfd in configure and
provide a
wrapper that redefines it as necessary.
Can fix that, though please see my later remarks.
+static int create_tmap_op(uint8_t op, const char *name, char
**tmap_desc)
+{
+uint32_t len = strlen(name);
+/* total_len = encoding op + name + empty buffer */
+uint32_t total_len = 1 + (sizeof(uint32_t) + len) +
sizeof(uint32_t);
+char *desc = NULL;
char is the wrong type to use here as it may be signed or unsigned. That
can have weird effects with binary data when you're directly manipulating
it.
Well, I can change it to uint8_t, so that it matches the op type, but
that'll require adding some other castings. In any case, you usually
get such a weird behavior when you cast to types of different sizes
and have the sign bit padded which is not the case in here.
+
+desc = qemu_malloc(total_len);
+
+*tmap_desc = desc;
+
+*desc = op;
+desc++;
+memcpy(desc,len, sizeof(len));
+desc += sizeof(len);
+memcpy(desc, name, len);
+desc += len;
+len = 0;
+memcpy(desc,len, sizeof(len));
+desc += sizeof(len);
Shouldn't endianness be a concern?
Right. Fixed that.
+
+return desc - *tmap_desc;
+}
+
+static void free_tmap_op(char *tmap_desc)
+{
+qemu_free(tmap_desc);
+}
+
+static int rbd_register_image(rados_pool_t pool, const char *name)
+{
+char *tmap_desc;
+const char *dir = RBD_DIRECTORY;
+int ret;
+
+ret = create_tmap_op(CEPH_OSD_TMAP_SET, name,tmap_desc);
+if (ret 0) {
+return ret;
+}
+
+ret = rados_tmap_update(pool, dir, tmap_desc, ret);
+free_tmap_op(tmap_desc);
+
+return ret;
+}
This ops are all synchronous? IOW, rados_tmap_update() call blocks until
the operation is completed?
Yeah. And this is only called from the rbd_create() callback.
+header_snap += strlen(header_snap) + 1;
+if (header_snap end)
+error_report(bad header, snapshot list broken);
Missing curly braces here.
Fixed.
+if (strncmp(hbuf + 68, RBD_HEADER_VERSION, 8)) {
+error_report(Unknown image version %s, hbuf + 68);
+r = -EMEDIUMTYPE;
+goto failed;
+}
+
+RbdHeader1 *header;
Don't mix variable definitions with code.
Fixed.
+s-efd = eventfd(0, 0);
+if (s-efd 0) {
+error_report(error opening eventfd);
+goto failed;
+}
+fcntl(s-efd, F_SETFL, O_NONBLOCK);
+qemu_aio_set_fd_handler(s-efd, rbd_aio_completion_cb, NULL,
+rbd_aio_flush_cb, NULL, s);
It looks like you just use the eventfd to signal aio completion
callbacks.
A better way to do this would be to schedule a bottom half. eventfds
are
Linux specific and specific to recent kernels.
Digging back why we introduced the eventfd, it was due to some issues
seen with do_savevm() hangs on qemu_aio_flush(). The reason seemed
that we had no fd associated with the block device, which seemed to
not work well with the qemu aio model. If that assumption is wrong,
we'd be happy to change it. In any case, there are other more portable
ways to generate fds, so if it's needed we can do that.
There's no fd at all? How do you get notifications about an asynchronous
event completion?
Regards,
Anthony Liguori
(resending to list, sorry)
The fd is hidden deep under in librados. We get callback notifications
for events completion.
How is that possible? Are the callbacks delivered in the context of a
different thread? If so, don't you need locking?
Regards,
Anthony Liguori
Thanks,
Yehuda
Re: [Qemu-devel] [PATCH 2/3] vnc: support password expire
On 10/07/2010 06:15 AM, Gerd Hoffmann wrote:
This patch adds support for expiring passwords to vnc. It adds a new
lifetime parameter to the vnc_display_password() function, which
specifies the number of seconds the new password will be valid. Passing
zero as lifetime maintains current behavior (password never expires).
Signed-off-by: Gerd Hoffmannkra...@redhat.com
This has been posted before and I've never understood it. Why can't a
management tool just expire passwords on it's own?
How does password expiration help with security at all?
Regards,
Anthony Liguori
---
console.h |2 +-
monitor.c |3 +--
ui/vnc.c | 15 ++-
ui/vnc.h |1 +
4 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/console.h b/console.h
index aafb031..24670e5 100644
--- a/console.h
+++ b/console.h
@@ -368,7 +368,7 @@ void cocoa_display_init(DisplayState *ds, int full_screen);
void vnc_display_init(DisplayState *ds);
void vnc_display_close(DisplayState *ds);
int vnc_display_open(DisplayState *ds, const char *display);
-int vnc_display_password(DisplayState *ds, const char *password);
+int vnc_display_password(DisplayState *ds, const char *password, int lifetime);
void do_info_vnc_print(Monitor *mon, const QObject *data);
void do_info_vnc(Monitor *mon, QObject **ret_data);
char *vnc_display_local_addr(DisplayState *ds);
diff --git a/monitor.c b/monitor.c
index fbb678d..d82eb9e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -966,11 +966,10 @@ static int do_quit(Monitor *mon, const QDict *qdict,
QObject **ret_data)
static int change_vnc_password(const char *password)
{
-if (vnc_display_password(NULL, password) 0) {
+if (vnc_display_password(NULL, password, 0) 0) {
qerror_report(QERR_SET_PASSWD_FAILED);
return -1;
}
-
return 0;
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 1ef0fc5..51aa9ca 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2078,11 +2078,19 @@ static int protocol_client_auth_vnc(VncState *vs,
uint8_t *data, size_t len)
unsigned char response[VNC_AUTH_CHALLENGE_SIZE];
int i, j, pwlen;
unsigned char key[8];
+time_t now;
if (!vs-vd-password || !vs-vd-password[0]) {
VNC_DEBUG(No password configured on server);
goto reject;
}
+if (vs-vd-expires) {
+time(now);
+if (vs-vd-expires now) {
+VNC_DEBUG(Password is expired);
+goto reject;
+}
+}
memcpy(response, vs-challenge, VNC_AUTH_CHALLENGE_SIZE);
@@ -2474,7 +2482,7 @@ void vnc_display_close(DisplayState *ds)
#endif
}
-int vnc_display_password(DisplayState *ds, const char *password)
+int vnc_display_password(DisplayState *ds, const char *password, int lifetime)
{
VncDisplay *vs = ds ? (VncDisplay *)ds-opaque : vnc_display;
@@ -2492,6 +2500,11 @@ int vnc_display_password(DisplayState *ds, const char
*password)
if (vs-auth == VNC_AUTH_NONE) {
vs-auth = VNC_AUTH_VNC;
}
+if (lifetime) {
+vs-expires = time(NULL) + lifetime;
+} else {
+vs-expires = 0;
+}
} else {
vs-auth = VNC_AUTH_NONE;
}
diff --git a/ui/vnc.h b/ui/vnc.h
index 9619b24..4f895be 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -120,6 +120,7 @@ struct VncDisplay
char *display;
char *password;
+time_t expires;
int auth;
bool lossy;
#ifdef CONFIG_VNC_TLS
Re: [Qemu-devel] [PATCH 0/2] USB CCID device
On 10/07/2010 02:39 AM, Alon Levy wrote: - Anthony Liguorianth...@codemonkey.ws wrote: On 10/06/2010 03:55 AM, Gerd Hoffmann wrote: On 10/06/10 02:28, Alon Levy wrote: Does this work with live migration? I can't see how it would. No, it doesn't right now. It would require cooperation with the client, to tell it to reconnect to the target qemu (kind of like spice). I think until we have this migration should have pretty much the same effect as a chardev disconnect, i.e. detach the usb device (which the guest will see as unplug). Better yet, mark the guest as unmigrateable and let the management tool unplug the usb device before migration and replug it after migration. It's the same principle behind device assignment. Is there any way to also get a pre_migrate callback with register_device_unmigratable? I'd like to send a VSC_Reconnect message, then the guest sees an unplug, then migration, No. The disconnect needs to happen in the management tooling layer. Same is true for any device doing hardware passthrough. Automagic unplugging/plugging during migration is not a good idea universally so it's something that needs to happen as a policy at the management level. Regards, Anthony Liguori then (no plug yet since the device is marked as auto_attach=0) client reconnects (actually this happens before but to a paused machine waiting for migration), and then causes attachement, same as a new machine. Regards, Anthony Liguori Needs some code though, at minimum you'll have to xfer the connected state from the migration source and have some bits in post_load() which do attach/detach if needed. cheers, Gerd
Re: [Qemu-devel] [PATCH 0/2] USB CCID device
On 10/06/2010 05:12 PM, Alon Levy wrote: Actually, both are possible - but the later is the interesting use case (the former is mainly for debugging). To elaborate: the device is meant to allow a hardware reader to be available to the guest while still being available to the client (which is running on the computer with the real reader attached). So the real card is what we are talking to. The other usage is to have a virtual card, which would actually be more logical to put with the qemu device, but It isn't my current focus (the focus being the real card, and the virtual card being implemented in the client side is a testing measure). Okay, that makes sense. I'll do this. A side note: I tried migrating a QSIMPLEQ today - not a fun experience. I'm wondering if this is a result of a mentality that all devices should allocate memory upfront that makes using / migrating dynamic linked lists a non-starter, or just an oversight (no one wrote VMSTATE_QSIMPLEQ yet). As it stands migrating a QSIMPLEQ (or any other list) is much easier the old way without using VMSTATE. /rant. Yeah, complex types are not at all easy to migrate today. Something we need to tackle with vmstate2 eventually. Regards, Anthony Liguori Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH 10/11] ppc: avoid write only variables
Am 07.10.2010 um 20:59 schrieb Blue Swirl blauwir...@gmail.com: On Wed, Oct 6, 2010 at 9:39 PM, Alexander Graf ag...@suse.de wrote: On 06.10.2010, at 23:34, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-ppc/op_helper.c: In function 'helper_icbi': /src/qemu/target-ppc/op_helper.c:351:14: error: variable 'tmp' set but not used [-Werror=unused-but-set-variable] /src/qemu/target-ppc/op_helper.c: In function 'do_6xx_tlb': /src/qemu/target-ppc/op_helper.c:3805:28: error: variable 'EPN' set but not used [-Werror=unused-but-set-variable] /src/qemu/target-ppc/op_helper.c: In function 'do_74xx_tlb': /src/qemu/target-ppc/op_helper.c:3838:28: error: variable 'EPN' set but not used [-Werror=unused-but-set-variable] Fix by making the variable declarations and their uses also conditional to debug definition. Delete tmp. Maybe it would make more sense to get those LOG_* macros into static inline functions. But for the issue at hand, the solution looks good to me. Perhaps all conditionally enabled debug printf stuff should be transformed into tracepoints? That would be more flexible than uncommenting DEBUG_foobar. Would it help with bitrot too? I haven't looked at the qemu tracepoints framework yet, but some of this debug stuff is in hot paths and only very rarely used. So unless it is overhead free, I'd prefer debug function stubs. Alex Signed-off-by: Alexander Graf ag...@suse.de Thanks.
Re: [Qemu-devel] [STATUS] static instrumentation
Blue Swirl writes: The patches there seem to be fixes or small changes to your other patches. This makes any review very difficult. Sorry, I'm really new to git, so I don't really know how to maintain and grow an organized set of patches for review while I keep improving the work. But I'm not sure the architecture is what we'd like to see integrated in QEMU, though I may have missed the whole picture because of the patch fragmentation problem. Before doing any work to possibly go further in wrong direction, please try to submit early some preview version for comments and review. I can try to refactor the changes into a sensible set of patches, but please read first the motivation of this work (below). Just as an example, perhaps the existing tracing framework could be extended to cover also your needs. Well, for what I've seen of the tracing infrastructure (tracing from here on), it is geared towards tracing qemu internal events (for analyzing qemu performance), while static instrumentation (instrumentation from here on) is geared towards tracing guest events, in order to analyze guest behaviour. The reason for having static instrumentation points in the form of macros (diverging from the approach taken in tracing) is that the most common instrumentation points are invoked at code generation time. This way the user can dynamically select which extra code to call and/or generate (if any) on each of these points, with the addition that code can be selected for regeneration when the instrumentation points need to produce different code (e.g., no instrumentation, event counting, sketchy trace generation, detailed trace generation, etc.). So, I think they are geared towards different goals, although they are indeed composable (e.g., the user can expand the instrumentation macros into calls to the tracing infrastructure). Wether or not this fits into upstream qemu is not for me to say, but it provides the initial stepping stone that is commonly used in computer architecture research (e.g., http://www.ptlsim.org or http://sesc.sourceforge.net/), with the added bonus that qemu supports multiple ISAs, as well as executing standalone applications and a full-fledged system. Of course, the simulator itself would be out of qemu, thus the macros :) Lluis -- And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer. -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
[Qemu-devel] [PATCH 2/2] check for close() errors on qcow2_create()
Errors when closing the file we just created should not be ignored. I/O errors
may happen and qemu-img create should fail in those cases.
If we are already exiting due to an error, we will still return the original
error number, though.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
block/qcow2.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index c5fb28e..d3a056b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -882,7 +882,7 @@ static int qcow_create2(const char *filename, int64_t
total_size,
uint64_t old_ref_clusters;
QCowCreateState s1, *s = s1;
QCowExtension ext_bf = {0, 0};
-int ret;
+int ret, cret;
memset(s, 0, sizeof(*s));
@@ -1055,7 +1055,9 @@ exit:
qemu_free(s-refcount_block);
exit_close:
-close(fd);
+cret = close(fd);
+if (ret == 0 cret 0)
+ret = -errno;
/* Preallocate metadata */
if (ret == 0 prealloc) {
--
1.6.5.5
[Qemu-devel] [PATCH 0/2] qcow2_create() error handling fixes
From: Eduardo Habkost ehabk...@raisama.net Hi, Here are two small fixes on qcow2_create() error handling. Eduardo Habkost (2): fix fd leak on a qcow2_create2() error path check for close() errors on qcow2_create() block/qcow2.c | 11 --- 1 files changed, 8 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH 1/2] fix fd leak on one qcow2_create2() error path
When getting an invalid cluster size, the open fd must be closed before qcow2_create() returns an error. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- block/qcow2.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index ee3481b..c5fb28e 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -918,7 +918,8 @@ static int qcow_create2(const char *filename, int64_t total_size, %d and %dk\n, 1 MIN_CLUSTER_BITS, 1 (MAX_CLUSTER_BITS - 10)); -return -EINVAL; +ret = -EINVAL; +goto exit_close; } s-cluster_size = 1 s-cluster_bits; @@ -1052,6 +1053,8 @@ static int qcow_create2(const char *filename, int64_t total_size, exit: qemu_free(s-refcount_table); qemu_free(s-refcount_block); + +exit_close: close(fd); /* Preallocate metadata */ -- 1.6.5.5
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
On Thu, Oct 7, 2010 at 12:51 PM, Anthony Liguori anth...@codemonkey.ws wrote: On 10/07/2010 01:41 PM, Yehuda Sadeh Weinraub wrote: On Thu, Oct 7, 2010 at 11:38 AM, Anthony Liguorianth...@codemonkey.ws wrote: On 10/07/2010 01:08 PM, Yehuda Sadeh Weinraub wrote: On Thu, Oct 7, 2010 at 7:12 AM, Anthony Liguorianth...@codemonkey.ws ... There's no fd at all? How do you get notifications about an asynchronous event completion? Regards, Anthony Liguori (resending to list, sorry) The fd is hidden deep under in librados. We get callback notifications for events completion. How is that possible? Are the callbacks delivered in the context of a different thread? If so, don't you need locking? Not sure I'm completely following you. The callbacks are delivered in the context of a different thread, but won't run concurrently. Do you see any specific concurrency issue? We can add some mutex protection around at the aio callback, so that if librados turns multithreaded at this point we're covered. Thanks, Yehuda
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
On 10/07/2010 03:47 PM, Yehuda Sadeh Weinraub wrote: How is that possible? Are the callbacks delivered in the context of a different thread? If so, don't you need locking? Not sure I'm completely following you. The callbacks are delivered in the context of a different thread, but won't run concurrently. Concurrently to what? How do you prevent them from running concurrently with qemu? If you saw lock ups, I bet that's what it was from. Regards, Anthony Liguori Do you see any specific concurrency issue? We can add some mutex protection around at the aio callback, so that if librados turns multithreaded at this point we're covered. Thanks, Yehuda
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
On Thu, Oct 7, 2010 at 2:04 PM, Anthony Liguori anth...@codemonkey.ws wrote:
On 10/07/2010 03:47 PM, Yehuda Sadeh Weinraub wrote:
How is that possible? Are the callbacks delivered in the context of a
different thread? If so, don't you need locking?
Not sure I'm completely following you. The callbacks are delivered in
the context of a different thread, but won't run concurrently.
Concurrently to what? How do you prevent them from running concurrently
with qemu?
There are two types of callbacks. The first is for rados aio
completions, and the second one is the one added later for the fd glue
layer.
The first callback, called by librados whenever aio completes, runs in
the context of a single librados thread:
+static void rbd_finish_aiocb(rados_completion_t c, RADOSCB *rcb)
+{
+RBDAIOCB *acb = rcb-acb;
rcb is per a single aio. Was created before and will be destroyed
here, whereas acb is shared between a few aios, however, it was
generated before the first aio was created.
+int64_t r;
+uint64_t buf = 1;
+int i;
+
+acb-aiocnt--;
acb-aiocnt has been set before initiating all the aios, so it's ok to
touch it now. Same goes to all acb fields.
+r = rados_aio_get_return_value(c);
+rados_aio_release(c);
+if (acb-write) {
+if (r 0) {
+acb-ret = r;
+acb-error = 1;
+} else if (!acb-error) {
+acb-ret += rcb-segsize;
+}
+} else {
+if (r == -ENOENT) {
+memset(rcb-buf, 0, rcb-segsize);
+if (!acb-error) {
+acb-ret += rcb-segsize;
+}
+} else if (r 0) {
+acb-ret = r;
+acb-error = 1;
+} else if (r rcb-segsize) {
+memset(rcb-buf + r, 0, rcb-segsize - r);
+if (!acb-error) {
+acb-ret += rcb-segsize;
+}
+} else if (!acb-error) {
+acb-ret += r;
+}
+}
+if (write(acb-s-efd, buf, sizeof(buf)) 0)
This will wake up the io_read()
+error_report(failed writing to acb-s-efd\n);
+qemu_free(rcb);
+i = 0;
+if (!acb-aiocnt acb-bh) {
+qemu_bh_schedule(acb-bh);
This is the only qemu related call in here, seems safe to call it.
+}
+}
The scheduled bh function will be called only after all aios that
relate to this specific aio set are done, so the following seems ok,
as there's no more acb references.
+static void rbd_aio_bh_cb(void *opaque)
+{
+RBDAIOCB *acb = opaque;
+uint64_t buf = 1;
+
+if (!acb-write) {
+qemu_iovec_from_buffer(acb-qiov, acb-bounce, acb-qiov-size);
+}
+qemu_vfree(acb-bounce);
+acb-common.cb(acb-common.opaque, (acb-ret 0 ? 0 : acb-ret));
+qemu_bh_delete(acb-bh);
+acb-bh = NULL;
+
+if (write(acb-s-efd, buf, sizeof(buf)) 0)
+error_report(failed writing to acb-s-efd\n);
+qemu_aio_release(acb);
+}
Now, the second ones are the io_read(), in which we have our glue fd.
We send uint64 per each completed io
+static void rbd_aio_completion_cb(void *opaque)
+{
+BDRVRBDState *s = opaque;
+
+uint64_t val;
+ssize_t ret;
+
+do {
+if ((ret = read(s-efd, val, sizeof(val))) 0) {
+s-qemu_aio_count -= val;
There is an issue here with s-qemu_aio_count which needs to be
protected by a mutex. Other than that, it just reads from s-efd.
+ }
+} while (ret 0 errno == EINTR);
+
+return;
+}
+
+static int rbd_aio_flush_cb(void *opaque)
+{
+BDRVRBDState *s = opaque;
+
+return (s-qemu_aio_count 0);
Same here as with the previous one, needs a mutex around s-qemu_aio_count.
+}
If you saw lock ups, I bet that's what it was from.
As I explained before, before introducing the fd glue layer, the lack
of fd associated with our block device caused that there was no way
for qemu to check whether all aios were flushed or not, which didn't
work well when doing migration/savevm.
Thanks,
Yehuda
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
On 10/07/2010 04:49 PM, Yehuda Sadeh Weinraub wrote:
On Thu, Oct 7, 2010 at 2:04 PM, Anthony Liguorianth...@codemonkey.ws wrote:
On 10/07/2010 03:47 PM, Yehuda Sadeh Weinraub wrote:
How is that possible? Are the callbacks delivered in the context of a
different thread? If so, don't you need locking?
Not sure I'm completely following you. The callbacks are delivered in
the context of a different thread, but won't run concurrently.
Concurrently to what? How do you prevent them from running concurrently
with qemu?
There are two types of callbacks. The first is for rados aio
completions, and the second one is the one added later for the fd glue
layer.
This is a bad architecture for something like qemu. You could create a
pipe and use the pipe to signal to qemu. Same principle as eventfd.
Ideally, you would do this in the library itself.
Regards,
Anthony Liguori
The first callback, called by librados whenever aio completes, runs in
the context of a single librados thread:
+static void rbd_finish_aiocb(rados_completion_t c, RADOSCB *rcb)
+{
+RBDAIOCB *acb = rcb-acb;
rcb is per a single aio. Was created before and will be destroyed
here, whereas acb is shared between a few aios, however, it was
generated before the first aio was created.
+int64_t r;
+uint64_t buf = 1;
+int i;
+
+acb-aiocnt--;
acb-aiocnt has been set before initiating all the aios, so it's ok to
touch it now. Same goes to all acb fields.
+r = rados_aio_get_return_value(c);
+rados_aio_release(c);
+if (acb-write) {
+if (r 0) {
+acb-ret = r;
+acb-error = 1;
+} else if (!acb-error) {
+acb-ret += rcb-segsize;
+}
+} else {
+if (r == -ENOENT) {
+memset(rcb-buf, 0, rcb-segsize);
+if (!acb-error) {
+acb-ret += rcb-segsize;
+}
+} else if (r 0) {
+acb-ret = r;
+acb-error = 1;
+} else if (r rcb-segsize) {
+memset(rcb-buf + r, 0, rcb-segsize - r);
+if (!acb-error) {
+acb-ret += rcb-segsize;
+}
+} else if (!acb-error) {
+acb-ret += r;
+}
+}
+if (write(acb-s-efd,buf, sizeof(buf)) 0)
This will wake up the io_read()
+error_report(failed writing to acb-s-efd\n);
+qemu_free(rcb);
+i = 0;
+if (!acb-aiocnt acb-bh) {
+qemu_bh_schedule(acb-bh);
This is the only qemu related call in here, seems safe to call it.
+}
+}
The scheduled bh function will be called only after all aios that
relate to this specific aio set are done, so the following seems ok,
as there's no more acb references.
+static void rbd_aio_bh_cb(void *opaque)
+{
+RBDAIOCB *acb = opaque;
+uint64_t buf = 1;
+
+if (!acb-write) {
+qemu_iovec_from_buffer(acb-qiov, acb-bounce, acb-qiov-size);
+}
+qemu_vfree(acb-bounce);
+acb-common.cb(acb-common.opaque, (acb-ret 0 ? 0 : acb-ret));
+qemu_bh_delete(acb-bh);
+acb-bh = NULL;
+
+if (write(acb-s-efd,buf, sizeof(buf)) 0)
+error_report(failed writing to acb-s-efd\n);
+qemu_aio_release(acb);
+}
Now, the second ones are the io_read(), in which we have our glue fd.
We send uint64 per each completed io
+static void rbd_aio_completion_cb(void *opaque)
+{
+BDRVRBDState *s = opaque;
+
+uint64_t val;
+ssize_t ret;
+
+do {
+if ((ret = read(s-efd,val, sizeof(val))) 0) {
+s-qemu_aio_count -= val;
There is an issue here with s-qemu_aio_count which needs to be
protected by a mutex. Other than that, it just reads from s-efd.
+ }
+} while (ret 0 errno == EINTR);
+
+return;
+}
+
+static int rbd_aio_flush_cb(void *opaque)
+{
+BDRVRBDState *s = opaque;
+
+return (s-qemu_aio_count 0);
Same here as with the previous one, needs a mutex around s-qemu_aio_count.
+}
If you saw lock ups, I bet that's what it was from.
As I explained before, before introducing the fd glue layer, the lack
of fd associated with our block device caused that there was no way
for qemu to check whether all aios were flushed or not, which didn't
work well when doing migration/savevm.
Thanks,
Yehuda
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
On Thu, 7 Oct 2010, Anthony Liguori wrote:
On 10/07/2010 04:49 PM, Yehuda Sadeh Weinraub wrote:
On Thu, Oct 7, 2010 at 2:04 PM, Anthony Liguorianth...@codemonkey.ws
wrote:
On 10/07/2010 03:47 PM, Yehuda Sadeh Weinraub wrote:
How is that possible? Are the callbacks delivered in the context of a
different thread? If so, don't you need locking?
Not sure I'm completely following you. The callbacks are delivered in
the context of a different thread, but won't run concurrently.
Concurrently to what? How do you prevent them from running concurrently
with qemu?
There are two types of callbacks. The first is for rados aio
completions, and the second one is the one added later for the fd glue
layer.
This is a bad architecture for something like qemu. You could create a
pipe and use the pipe to signal to qemu. Same principle as eventfd.
Ideally, you would do this in the library itself.
I'm sorry, I'm having a hard time understanding what it is you're
objecting to, or what you would prefer, as there are two different things
we're talking about here (callbacks and fd glue/pipes). (Please bear with
me as I am not a qemu expert!)
The first is the aio completion. You said a few messages back:
It looks like you just use the eventfd to signal aio completion
callbacks. A better way to do this would be to schedule a bottom half.
This is what we're doing. The librados makes a callback to rbd.c's
rbd_finish_aiocb(), which updates some internal rbd accounting and then
calls qemu_bh_schedule(). Is that part right?
The second part is an fd (currently created via eventfd(), but I don't
think it matters where it comes from) that was later added because
qemu_aio_flush() wouldn't trigger when our aio's completed (and scheduled
the bottom halves). This was proposed by Simone Gotti, who had problems
with live migration:
http://www.mail-archive.com/qemu-devel@nongnu.org/msg35516.html
Apparently calling the bottom half isn't sufficient to wake up a blocked
qemu_aio_flush()? His solution was to create an eventfd() fd, write a
word to it in the aio completion callback (before we schedule the bh), and
add the necessary callbacks to make qemu_aio_flush() behave.
Is the problem simply that we should be using pipe(2) instead of
eventfd(2)?
So far I've heard that we should be scheduling the bottom halves (we are),
and we should be using a pipe to signal qemu (we're using an fd created by
eventfd(2)).
Thanks,
sage
Regards,
Anthony Liguori
The first callback, called by librados whenever aio completes, runs in
the context of a single librados thread:
+static void rbd_finish_aiocb(rados_completion_t c, RADOSCB *rcb)
+{
+RBDAIOCB *acb = rcb-acb;
rcb is per a single aio. Was created before and will be destroyed
here, whereas acb is shared between a few aios, however, it was
generated before the first aio was created.
+int64_t r;
+uint64_t buf = 1;
+int i;
+
+acb-aiocnt--;
acb-aiocnt has been set before initiating all the aios, so it's ok to
touch it now. Same goes to all acb fields.
+r = rados_aio_get_return_value(c);
+rados_aio_release(c);
+if (acb-write) {
+if (r 0) {
+acb-ret = r;
+acb-error = 1;
+} else if (!acb-error) {
+acb-ret += rcb-segsize;
+}
+} else {
+if (r == -ENOENT) {
+memset(rcb-buf, 0, rcb-segsize);
+if (!acb-error) {
+acb-ret += rcb-segsize;
+}
+} else if (r 0) {
+acb-ret = r;
+acb-error = 1;
+} else if (r rcb-segsize) {
+memset(rcb-buf + r, 0, rcb-segsize - r);
+if (!acb-error) {
+acb-ret += rcb-segsize;
+}
+} else if (!acb-error) {
+acb-ret += r;
+}
+}
+if (write(acb-s-efd,buf, sizeof(buf)) 0)
This will wake up the io_read()
+error_report(failed writing to acb-s-efd\n);
+qemu_free(rcb);
+i = 0;
+if (!acb-aiocnt acb-bh) {
+qemu_bh_schedule(acb-bh);
This is the only qemu related call in here, seems safe to call it.
+}
+}
The scheduled bh function will be called only after all aios that
relate to this specific aio set are done, so the following seems ok,
as there's no more acb references.
+static void rbd_aio_bh_cb(void *opaque)
+{
+RBDAIOCB *acb = opaque;
+uint64_t buf = 1;
+
+if (!acb-write) {
+qemu_iovec_from_buffer(acb-qiov, acb-bounce, acb-qiov-size);
+}
+qemu_vfree(acb-bounce);
+acb-common.cb(acb-common.opaque, (acb-ret 0 ? 0 : acb-ret));
+qemu_bh_delete(acb-bh);
+acb-bh = NULL;
+
+if (write(acb-s-efd,buf, sizeof(buf)) 0)
+
Re: [Qemu-devel] fix unsigned comparison warning in TCG
On 9/26/2010 11:33 AM, Blue Swirl wrote: On Sun, Sep 26, 2010 at 5:40 PM, Hollis Blanchard hol...@penguinppc.org wrote: TCGOpcode is an enum, which apparently can be unsigned. Signed-off-by: Hollis Blanchard hol...@penguinppc.org --- % ./configure --target-list=ppcemb-softmmu --enable-debug % make ... CCppcemb-softmmu/tcg/tcg.o cc1: warnings being treated as errors /home/hollisb/source/qemu.git/tcg/tcg.c: In function ‘tcg_add_target_add_op_defs’: /home/hollisb/source/qemu.git/tcg/tcg.c:1030: error: comparison of unsigned expression = 0 is always true % gcc -v gcc version 4.4.4 20100630 (Red Hat 4.4.4-10) (GCC) diff --git a/tcg/tcg.c b/tcg/tcg.c index e0a9030..7e96859 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -1027,7 +1027,7 @@ void tcg_add_target_add_op_defs(const TCGTargetOpDef *tdef if (tdefs-op == (TCGOpcode)-1) break; op = tdefs-op; -assert(op = 0 op NB_OPS); +assert(op NB_OPS); Please add int cast, like 95ee3914bfd551aeec49932a400530141865acad. What is the latest on this? Waiting for this fix as I use --enable-debug a lot. :) - JV
[Qemu-devel] segmentation fault running ppc linux user regression test
Hi guys: I am very new to qemu and not sure which place to go for help. So if this is not the proper forum, please point me to the appreciate one, if available. I just downloaded the newest qemu version: qemu-0.12.5 and build the ppc-linux-user. To test it, I also downloaded the linux-user-test-0.3. However, when I try to run the regression test: /projects/svdc/P4sbSW/sli/qemu_tmp/qemu-0.12.5/ppc-linux-user/qemu-ppc -L ./gnemul/qemu-ppc ppc/kill Segmentation fault file ppc/kill ppc/kill: ELF 32-bit MSB executable, PowerPC or cisco 4500, version 1 (SYSV), for GNU/Linux 2.2.0, dynamically linked (uses shared libs), stripped I see kill is a dynamically linked program. I then try with the good old hello_world, which is statically linked: file /projects/svdc/P4sbSW/sli/qemu_tmp/test/hello.o /projects/svdc/P4sbSW/sli/qemu_tmp/test/hello.o: ELF 32-bit MSB executable, PowerPC or cisco 4500, version 1 (SYSV), statically linked, not stripped /projects/svdc/P4sbSW/sli/qemu_tmp/qemu-0.12.5//ppc-linux-user/qemu-ppc -L ./gnemul/qemu-ppc /projects/svdc/P4sbSW/sli/qemu_tmp/test/hello.o qemu: Unsupported syscall: 17 Hello, world. It runs although with the Unsupported syscall. Could anyone help me to fix the Segmentation fault problem?
[Qemu-devel] Re: [patch uq/master 7/8] MCE: Relay UCR MCE to guest
On Thu, 2010-10-07 at 00:05 +0800, Marcelo Tosatti wrote: On Wed, Oct 06, 2010 at 10:58:36AM +0900, Hidetoshi Seto wrote: I got some more question: (2010/10/05 3:54), Marcelo Tosatti wrote: Index: qemu/target-i386/cpu.h === --- qemu.orig/target-i386/cpu.h +++ qemu/target-i386/cpu.h @@ -250,16 +250,32 @@ #define PG_ERROR_RSVD_MASK 0x08 #define PG_ERROR_I_D_MASK 0x10 -#define MCG_CTL_P(1UL8) /* MCG_CAP register available */ +#define MCG_CTL_P(1ULL8) /* MCG_CAP register available */ +#define MCG_SER_P(1ULL24) /* MCA recovery/new status bits */ -#define MCE_CAP_DEF MCG_CTL_P +#define MCE_CAP_DEF (MCG_CTL_P|MCG_SER_P) #define MCE_BANKS_DEF10 It seems that current kvm doesn't support SER_P, so injecting SRAO to guest will mean that guest receives VAL|UC|!PCC and RIPV event from virtual processor that doesn't have SER_P. Dean also noted this. I don't think it was deliberate choice to not expose SER_P. Huang? In fact, that should be a BUG. I will fix it as soon as possible. Best Regards, Huang Ying
[Qemu-devel] Re: [patch uq/master 7/8] MCE: Relay UCR MCE to guest
Hi, Seto, On Thu, 2010-10-07 at 11:41 +0800, Hidetoshi Seto wrote: (2010/10/07 3:10), Dean Nelson wrote: On 10/06/2010 11:05 AM, Marcelo Tosatti wrote: On Wed, Oct 06, 2010 at 10:58:36AM +0900, Hidetoshi Seto wrote: I got some more question: (2010/10/05 3:54), Marcelo Tosatti wrote: Index: qemu/target-i386/cpu.h === --- qemu.orig/target-i386/cpu.h +++ qemu/target-i386/cpu.h @@ -250,16 +250,32 @@ #define PG_ERROR_RSVD_MASK 0x08 #define PG_ERROR_I_D_MASK 0x10 -#define MCG_CTL_P(1UL8) /* MCG_CAP register available */ +#define MCG_CTL_P(1ULL8) /* MCG_CAP register available */ +#define MCG_SER_P(1ULL24) /* MCA recovery/new status bits */ -#define MCE_CAP_DEFMCG_CTL_P +#define MCE_CAP_DEF(MCG_CTL_P|MCG_SER_P) #define MCE_BANKS_DEF10 It seems that current kvm doesn't support SER_P, so injecting SRAO to guest will mean that guest receives VAL|UC|!PCC and RIPV event from virtual processor that doesn't have SER_P. Dean also noted this. I don't think it was deliberate choice to not expose SER_P. Huang? In my testing, I found that MCG_SER_P was not being set (and I was running on a Nehalem-EX system). Injecting a MCE resulted in the guest entering into panic() from mce_panic(). If crash_kexec() finds a kexec_crash_image the system ends up rebooting, otherwise, what happens next requires operator intervention. Good to know. What I'm concerning is that if memory scrubbing SRAO event is injected when !SER_P, linux guest with certain mce tolerant level might grade it as UC severity and continue running with none of panicking, killing and poisoning because of !PCC and RIPV. Could you provide the panic message of the guest in your test? I think it can tell me why the mce handler decided to go panic. That is a bug that the SER_P is not in KVM_MCE_CAP_SUPPORTED in kernel. I will fix it as soon as possible. And SRAO MCE should not be sent when !SER_P, we should add that condition in qemu-kvm. When I applied a patch to the guest's kernel which forces mce_ser to be set, as if MCG_SER_P was set (see __mcheck_cpu_cap_init()), I found that when the memory page was 'owned' by a guest process, the process would be killed (if the page was dirty), and the guest would stay running. The HWPoisoned page would be sidelined and not cause any more issues. Excellent. So while guest kernel knows which page is poisoned, guest processes are controlled not to touch the page. ... Therefore rebooting the vm and renewing kernel will lost the information where is poisoned. Yes. That is an issue. Dean suggests that make qemu-kvm to refuse reboot the guest if there is poisoned page and ask for user to intervention. I have another idea to replace the poison pages with good pages when reboot, that is, recover without user intervention. I think most OSes don't expect that it can receives MCE with !PCC on traditional x86 processor without SER_P. Q1: Is it safe to expect that guests can handle such !PCC event? This might be best answered by Huang, but as I mentioned above, without MCG_SER_P being set, the result was an orderly system panic on the guest. Though I'll wait Huang (I think he is on holiday), I believe that system panic is just a possible option for AO (Action Optional) event, no matter how the SER_P is. We should fix this as I said above. Q2: What is the expected behavior on the guest? I think I answered this above. Yeah, thanks. Q3: What happen if guest reboots itself in response to the MCE? That depends... And the following issue also holds for a guest that is rebooted at some point having successfully sidelined the bad page. After the guest has panic'd, a system_reset of the guest or a restart initiated by crash_kexec() (called by panic() on the guest), usually results in the guest hanging because the bad page still belongs to qemu-kvm and is now being referenced by the new guest in some way. Yes. In other words my concern about reboot is that new guest kernel including kdump kernel might try to read the bad page. If there is no AR-SIGBUS etc., we need some tricks to inhibit such accesses. (It actually may not hang, but successfully reboot and be runnable, with the bad page lurking in the background. It all seems to depend on where the bad page ends up, and whether it's ever referenced.) I know some tough guys using their PC with buggy DIMMs :-) I believe there was an attempt to deal with this in kvm on the host. See kvm_handle_bad_page(). This function was suppose to result in the sending of a BUS_MCEERR_AR flavored SIGBUS by do_sigbus() to qemu-kvm which in theory would result in the right thing happening. But commit 96054569190bdec375fe824e48ca1f4e3b53dd36 prevents the signal from being sent. So this mechanism needs to be re-worked, and the
[Qemu-devel] Re: [patch uq/master 7/8] MCE: Relay UCR MCE to guest
Hi, Huang-san, (2010/10/08 12:15), Huang Ying wrote: Hi, Seto, On Thu, 2010-10-07 at 11:41 +0800, Hidetoshi Seto wrote: (2010/10/07 3:10), Dean Nelson wrote: On 10/06/2010 11:05 AM, Marcelo Tosatti wrote: On Wed, Oct 06, 2010 at 10:58:36AM +0900, Hidetoshi Seto wrote: I got some more question: (2010/10/05 3:54), Marcelo Tosatti wrote: Index: qemu/target-i386/cpu.h === --- qemu.orig/target-i386/cpu.h +++ qemu/target-i386/cpu.h @@ -250,16 +250,32 @@ #define PG_ERROR_RSVD_MASK 0x08 #define PG_ERROR_I_D_MASK 0x10 -#define MCG_CTL_P(1UL8) /* MCG_CAP register available */ +#define MCG_CTL_P(1ULL8) /* MCG_CAP register available */ +#define MCG_SER_P(1ULL24) /* MCA recovery/new status bits */ -#define MCE_CAP_DEFMCG_CTL_P +#define MCE_CAP_DEF(MCG_CTL_P|MCG_SER_P) #define MCE_BANKS_DEF10 It seems that current kvm doesn't support SER_P, so injecting SRAO to guest will mean that guest receives VAL|UC|!PCC and RIPV event from virtual processor that doesn't have SER_P. Dean also noted this. I don't think it was deliberate choice to not expose SER_P. Huang? In my testing, I found that MCG_SER_P was not being set (and I was running on a Nehalem-EX system). Injecting a MCE resulted in the guest entering into panic() from mce_panic(). If crash_kexec() finds a kexec_crash_image the system ends up rebooting, otherwise, what happens next requires operator intervention. Good to know. What I'm concerning is that if memory scrubbing SRAO event is injected when !SER_P, linux guest with certain mce tolerant level might grade it as UC severity and continue running with none of panicking, killing and poisoning because of !PCC and RIPV. Could you provide the panic message of the guest in your test? I think it can tell me why the mce handler decided to go panic. That is a bug that the SER_P is not in KVM_MCE_CAP_SUPPORTED in kernel. I will fix it as soon as possible. And SRAO MCE should not be sent when !SER_P, we should add that condition in qemu-kvm. That makes sense. I think it is qemu's responsibility for what follows the AO-SIGBUS, what action should be taken depends on the KVM's capability. When I applied a patch to the guest's kernel which forces mce_ser to be set, as if MCG_SER_P was set (see __mcheck_cpu_cap_init()), I found that when the memory page was 'owned' by a guest process, the process would be killed (if the page was dirty), and the guest would stay running. The HWPoisoned page would be sidelined and not cause any more issues. Excellent. So while guest kernel knows which page is poisoned, guest processes are controlled not to touch the page. ... Therefore rebooting the vm and renewing kernel will lost the information where is poisoned. Yes. That is an issue. Dean suggests that make qemu-kvm to refuse reboot the guest if there is poisoned page and ask for user to intervention. I have another idea to replace the poison pages with good pages when reboot, that is, recover without user intervention. Sounds good. I think it may be worth something to reserve pages for the replacement before reboot is requested; at least we really don't want to fail rebooting with 'no memory'. I think most OSes don't expect that it can receives MCE with !PCC on traditional x86 processor without SER_P. Q1: Is it safe to expect that guests can handle such !PCC event? This might be best answered by Huang, but as I mentioned above, without MCG_SER_P being set, the result was an orderly system panic on the guest. Though I'll wait Huang (I think he is on holiday), I believe that system panic is just a possible option for AO (Action Optional) event, no matter how the SER_P is. We should fix this as I said above. Q2: What is the expected behavior on the guest? I think I answered this above. Yeah, thanks. Q3: What happen if guest reboots itself in response to the MCE? That depends... And the following issue also holds for a guest that is rebooted at some point having successfully sidelined the bad page. After the guest has panic'd, a system_reset of the guest or a restart initiated by crash_kexec() (called by panic() on the guest), usually results in the guest hanging because the bad page still belongs to qemu-kvm and is now being referenced by the new guest in some way. Yes. In other words my concern about reboot is that new guest kernel including kdump kernel might try to read the bad page. If there is no AR-SIGBUS etc., we need some tricks to inhibit such accesses. (It actually may not hang, but successfully reboot and be runnable, with the bad page lurking in the background. It all seems to depend on where the bad page ends up, and whether it's ever referenced.) I know some tough guys using their PC with buggy DIMMs :-) I believe there was an attempt to deal with this in kvm on the host. See kvm_handle_bad_page(). This
Re: [Qemu-devel] [PATCH 0/2] USB CCID device
On 10/06/10 21:00, Anthony Liguori wrote: Better yet, mark the guest as unmigrateable and let the management tool unplug the usb device before migration and replug it after migration. Yep, that will work too. cheers, Gerd
[Qemu-devel] Re: [PATCH 08/11] vnc: avoid write only variables
On 10/06/2010 11:33 PM, Blue Swirl wrote:
+#if defined(CONFIG_VNC_TLS) || defined(CONFIG_VNC_SASL)
} else if (strncmp(options, acl, 3) == 0) {
acl = 1;
+#endif
Not sure it's okay to reject the option altogether (i.e. maybe the #if
should only include acl = 1.
Paolo
[Qemu-devel] Re: [PATCH 04/11] lsi53c895a: avoid a write only variable
On 10/06/2010 11:32 PM, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced a warning: /src/qemu/hw/lsi53c895a.c: In function 'lsi_do_msgout': /src/qemu/hw/lsi53c895a.c:848:9: error: variable 'len' set but not used [-Werror=unused-but-set-variable] Fix by making the variable declaration and its uses also conditional to debug definition. NACK, this uglifies the code and loses track of _what_ is that msgbyte we're reading. Paolo
[Qemu-devel] Re: [PATCH 00/11] GCC 4.6.0 fixes
On 10/06/2010 11:31 PM, Blue Swirl wrote: Blue Swirl (11): block: avoid a write only variable cirrus: avoid write only variables eepro100: initialize a variable in all cases lsi53c895a: avoid a write only variable Delete write only variables mips_fulong2e: Delete write only variables cris: avoid write only variables vnc: avoid write only variables i386: avoid a write only variable ppc: avoid write only variables mips: avoid write only variables block/qcow2-refcount.c | 18 +- hw/cirrus_vga.c | 30 +- hw/cirrus_vga_rop.h | 38 +- hw/cirrus_vga_rop2.h| 12 +- hw/eepro100.c |6 +- hw/lsi53c895a.c |7 +- hw/mips_fulong2e.c |8 +- hw/mips_malta.c | 14 +- hw/mips_r4k.c |3 +- hw/ppc405_boards.c |5 +- hw/ppc405_uc.c | 28 - hw/ppc_newworld.c |3 +- hw/ppc_oldworld.c |6 +- hw/ppc_prep.c |3 +- hw/ppce500_mpc8544ds.c | 13 +- hw/tc6393xb_template.h |2 - hw/virtex_ml507.c |5 +- hw/wm8750.c |5 - net/tap-win32.c |5 +- savevm.c|4 +- target-cris/op_helper.c |6 + target-cris/translate_v10.c |5 +- target-i386/op_helper.c |4 +- target-mips/translate.c | 1222 +++ target-ppc/op_helper.c | 20 +- ui/vnc.c|8 + 26 files changed, 769 insertions(+), 711 deletions(-) I replied to the patches I disagree with. Otherwise a nice cleanup. Paolo
[Qemu-devel] Re: [PATCH 11/11] mips: avoid write only variables
On 10/06/2010 11:34 PM, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced a lot of warnings like: /src/qemu/target-mips/translate.c: In function 'gen_ld': /src/qemu/target-mips/translate.c:1039:17: error: variable 'opn' set but not used [-Werror=unused-but-set-variable] Fix by making the variable declarations and uses conditional to debugging. What about a set_opn macro instead of this one-off D idiom? Paolo
[Qemu-devel] Re: [PATCH 05/11] Delete write only variables
On 10/06/2010 11:32 PM, Blue Swirl wrote: @@ -91,8 +90,6 @@ static int vga_osi_call (CPUState *env) env-gpr[3] = 0; break; case 39: /* video_ctrl */ -if (env-gpr[6] == 0 || env-gpr[6] == 1) -vga_vbl_enabled = env-gpr[6]; env-gpr[3] = 0; break; case 47: NACK, this smells like an incomplete emulation. Paolo
[Qemu-devel] [PATCH 0/5] spice config options
Hi, This patch series adds a bunch of config options to spice, most notably it enables to configure TLS and thus using spice encrypted. The commit messages are not that verbose, but every patch comes with a patch chunk updating the spice section in the documentation. Please look there when reviewing the patches. The patches are also available in the git repository at: git://anongit.freedesktop.org/spice/qemu config.1 Gerd Hoffmann (4): spice: tls support spice: add config options for channel security. spice: add config options for the listening address spice: add misc config options Yonit Halperin (1): spice: make compression configurable. qemu-config.c | 57 +++ qemu-options.hx | 51 +- ui/spice-core.c | 205 +-- 3 files changed, 306 insertions(+), 7 deletions(-)
[Qemu-devel] [PATCH 4/5] spice: add config options for the listening address
Make listening address configurable. Also add options to
force using IPv4 or IPv6.
---
qemu-config.c |9 +
qemu-options.hx |7 +++
ui/spice-core.c | 13 +++--
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index f52e50c..5a62ae1 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -365,6 +365,15 @@ QemuOptsList qemu_spice_opts = {
.name = tls-port,
.type = QEMU_OPT_NUMBER,
},{
+.name = addr,
+.type = QEMU_OPT_STRING,
+},{
+.name = ipv4,
+.type = QEMU_OPT_BOOL,
+},{
+.name = ipv6,
+.type = QEMU_OPT_BOOL,
+},{
.name = password,
.type = QEMU_OPT_STRING,
},{
diff --git a/qemu-options.hx b/qemu-options.hx
index bb45b67..f74e380 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -682,6 +682,13 @@ Enable the spice remote desktop protocol. Valid options are
@item port=nr
Set the TCP port spice is listening on for plaintext channels.
+...@item addr=addr
+Set the IP address spice is listening on. Default is any address.
+
+...@item ipv4
+...@item ipv6
+Force using the specified IP version.
+
@item password=secret
Set the password you need to authenticate.
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 8f73848..b7f2cb3 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -218,14 +218,14 @@ static int add_channel(const char *name, const char
*value, void *opaque)
void qemu_spice_init(void)
{
QemuOpts *opts = QTAILQ_FIRST(qemu_spice_opts.head);
-const char *password, *str, *x509_dir,
+const char *password, *str, *x509_dir, *addr,
*x509_key_password = NULL,
*x509_dh_file = NULL,
*tls_ciphers = NULL;
char *x509_key_file = NULL,
*x509_cert_file = NULL,
*x509_cacert_file = NULL;
-int port, tls_port, len;
+int port, tls_port, len, addr_flags;
spice_image_compression_t compression;
spice_wan_compression_t wan_compr;
@@ -275,7 +275,16 @@ void qemu_spice_init(void)
tls_ciphers = qemu_opt_get(opts, tls-ciphers);
}
+addr = qemu_opt_get(opts, addr);
+addr_flags = 0;
+if (qemu_opt_get_bool(opts, ipv4, 0)) {
+addr_flags |= SPICE_ADDR_FLAG_IPV4_ONLY;
+} else if (qemu_opt_get_bool(opts, ipv6, 0)) {
+addr_flags |= SPICE_ADDR_FLAG_IPV6_ONLY;
+}
+
spice_server = spice_server_new();
+spice_server_set_addr(spice_server, addr ? addr : , addr_flags);
if (port) {
spice_server_set_port(spice_server, port);
}
--
1.7.1
[Qemu-devel] Re: [PATCH 09/11] i386: avoid a write only variable
On 10/06/2010 11:34 PM, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-i386/op_helper.c: In function 'switch_tss': /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' set but not used [-Werror=unused-but-set-variable] Fix by deleting the variable. Again, this warning tells us the emulation is incorrect, so it's wrong to remove it. Paolo
[Qemu-devel] [PATCH 2/5] spice: make compression configurable.
From: Yonit Halperin yhalp...@redhat.com
---
qemu-config.c |9 ++
qemu-options.hx |9 ++
ui/spice-core.c | 77 +-
3 files changed, 93 insertions(+), 2 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 26748a5..8b545b1 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -391,6 +391,15 @@ QemuOptsList qemu_spice_opts = {
},{
.name = tls-ciphers,
.type = QEMU_OPT_STRING,
+},{
+.name = image-compression,
+.type = QEMU_OPT_STRING,
+},{
+.name = jpeg-wan-compression,
+.type = QEMU_OPT_STRING,
+},{
+.name = zlib-glz-wan-compression,
+.type = QEMU_OPT_STRING,
},
{ /* end if list */ }
},
diff --git a/qemu-options.hx b/qemu-options.hx
index 9d3f8ef..59db632 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -704,6 +704,15 @@ The x509 file names can also be configured individually.
@item tls-ciphers=list
Specify which ciphers to use.
+...@item image-compression=[auto_glz|auto_lz|quic|glz|lz|off]
+Configure image compression (lossless).
+Default is auto_glz.
+
+...@item jpeg-wan-compression=[auto|never|allways]
+...@item zlib-glz-wan-compression=[auto|never|allways]
+Configure wan image compression (lossy for slow links).
+Default is auto.
+
@end table
ETEXI
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 51aa782..1567046 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -137,6 +137,59 @@ static SpiceCoreInterface core_interface = {
.watch_remove = watch_remove,
};
+/* config string parsing */
+
+static int name2enum(const char *string, const char *table[], int entries)
+{
+int i;
+
+if (string) {
+for (i = 0; i entries; i++) {
+if (!table[i]) {
+continue;
+}
+if (strcmp(string, table[i]) != 0) {
+continue;
+}
+return i;
+}
+}
+return -1;
+}
+
+static int parse_name(const char *string, const char *optname,
+ const char *table[], int entries)
+{
+int value = name2enum(string, table, entries);
+
+if (value != -1) {
+return value;
+}
+fprintf(stderr, spice: invalid %s: %s\n, optname, string);
+exit(1);
+}
+
+static const char *compression_names[] = {
+[ SPICE_IMAGE_COMPRESS_OFF ] = off,
+[ SPICE_IMAGE_COMPRESS_AUTO_GLZ ] = auto_glz,
+[ SPICE_IMAGE_COMPRESS_AUTO_LZ ] = auto_lz,
+[ SPICE_IMAGE_COMPRESS_QUIC ] = quic,
+[ SPICE_IMAGE_COMPRESS_GLZ ] = glz,
+[ SPICE_IMAGE_COMPRESS_LZ ] = lz,
+};
+#define parse_compression(_name)\
+parse_name(_name, image compression, \
+ compression_names, ARRAY_SIZE(compression_names))
+
+static const char *wan_compression_names[] = {
+[ SPICE_WAN_COMPRESSION_AUTO ] = auto,
+[ SPICE_WAN_COMPRESSION_NEVER ] = never,
+[ SPICE_WAN_COMPRESSION_ALWAYS ] = always,
+};
+#define parse_wan_compression(_name)\
+parse_name(_name, wan compression,\
+ wan_compression_names, ARRAY_SIZE(wan_compression_names))
+
/* functions for the rest of qemu */
void qemu_spice_init(void)
@@ -150,6 +203,8 @@ void qemu_spice_init(void)
*x509_cert_file = NULL,
*x509_cacert_file = NULL;
int port, tls_port, len;
+spice_image_compression_t compression;
+spice_wan_compression_t wan_compr;
if (!opts) {
return;
@@ -217,8 +272,26 @@ void qemu_spice_init(void)
spice_server_set_noauth(spice_server);
}
-/* TODO: make configurable via cmdline */
-spice_server_set_image_compression(spice_server,
SPICE_IMAGE_COMPRESS_AUTO_GLZ);
+compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ;
+str = qemu_opt_get(opts, image-compression);
+if (str) {
+compression = parse_compression(str);
+}
+spice_server_set_image_compression(spice_server, compression);
+
+wan_compr = SPICE_WAN_COMPRESSION_AUTO;
+str = qemu_opt_get(opts, jpeg-wan-compression);
+if (str) {
+wan_compr = parse_wan_compression(str);
+}
+spice_server_set_jpeg_compression(spice_server, wan_compr);
+
+wan_compr = SPICE_WAN_COMPRESSION_AUTO;
+str = qemu_opt_get(opts, zlib-glz-wan-compression);
+if (str) {
+wan_compr = parse_wan_compression(str);
+}
+spice_server_set_zlib_glz_compression(spice_server, wan_compr);
spice_server_init(spice_server, core_interface);
using_spice = 1;
--
1.7.1
[Qemu-devel] [PATCH 5/5] spice: add misc config options
This patch adds a few more options to tweak spice server behavior.
The documentation update chunk has the details ;)
---
qemu-config.c |9 +
qemu-options.hx |9 +
ui/spice-core.c | 29 -
3 files changed, 46 insertions(+), 1 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 5a62ae1..52f18be 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -415,6 +415,15 @@ QemuOptsList qemu_spice_opts = {
},{
.name = zlib-glz-wan-compression,
.type = QEMU_OPT_STRING,
+},{
+.name = streaming-video,
+.type = QEMU_OPT_STRING,
+},{
+.name = agent-mouse,
+.type = QEMU_OPT_BOOL,
+},{
+.name = playback-compression,
+.type = QEMU_OPT_BOOL,
},
{ /* end if list */ }
},
diff --git a/qemu-options.hx b/qemu-options.hx
index f74e380..eeb0a6c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -728,6 +728,15 @@ Default is auto_glz.
Configure wan image compression (lossy for slow links).
Default is auto.
+...@item streaming-video=[off|all|filter]
+Configure video stream detection. Default is filter.
+
+...@item agent-mouse=[on|off]
+Enable/disable passing mouse events via vdagent. Default is on.
+
+...@item playback-compression=[on|off]
+Enable/disable audio stream compression (using celt 0.5.1). Default is on.
+
@end table
ETEXI
diff --git a/ui/spice-core.c b/ui/spice-core.c
index b7f2cb3..c5574e1 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -169,6 +169,18 @@ static int parse_name(const char *string, const char
*optname,
exit(1);
}
+#if SPICE_SERVER_VERSION = 0x000600 /* 0.6.0 */
+
+static const char *stream_video_names[] = {
+[ SPICE_STREAM_VIDEO_OFF ]= off,
+[ SPICE_STREAM_VIDEO_ALL ]= all,
+[ SPICE_STREAM_VIDEO_FILTER ] = filter,
+};
+#define parse_stream_video(_name) \
+name2enum(_name, stream_video_names, ARRAY_SIZE(stream_video_names))
+
+#endif /* = 0.6.0 */
+
static const char *compression_names[] = {
[ SPICE_IMAGE_COMPRESS_OFF ] = off,
[ SPICE_IMAGE_COMPRESS_AUTO_GLZ ] = auto_glz,
@@ -225,7 +237,7 @@ void qemu_spice_init(void)
char *x509_key_file = NULL,
*x509_cert_file = NULL,
*x509_cacert_file = NULL;
-int port, tls_port, len, addr_flags;
+int port, tls_port, len, addr_flags, streaming_video;
spice_image_compression_t compression;
spice_wan_compression_t wan_compr;
@@ -325,6 +337,21 @@ void qemu_spice_init(void)
}
spice_server_set_zlib_glz_compression(spice_server, wan_compr);
+#if SPICE_SERVER_VERSION = 0x000600 /* 0.6.0 */
+
+str = qemu_opt_get(opts, streaming-video);
+if (str) {
+streaming_video = parse_stream_video(str);
+spice_server_set_streaming_video(spice_server, streaming_video);
+}
+
+spice_server_set_agent_mouse
+(spice_server, qemu_opt_get_bool(opts, agent-mouse, 1));
+spice_server_set_playback_compression
+(spice_server, qemu_opt_get_bool(opts, playback-compression, 1));
+
+#endif /* = 0.6.0 */
+
qemu_opt_foreach(opts, add_channel, NULL, 0);
spice_server_init(spice_server, core_interface);
--
1.7.1
[Qemu-devel] [PATCH 1/5] spice: tls support
Add options to the -spice command line switch to setup tls.
---
qemu-config.c | 24 +++
qemu-options.hx | 18 ++-
ui/spice-core.c | 67 +++---
3 files changed, 104 insertions(+), 5 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 32917cb..26748a5 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -362,11 +362,35 @@ QemuOptsList qemu_spice_opts = {
.name = port,
.type = QEMU_OPT_NUMBER,
},{
+.name = tls-port,
+.type = QEMU_OPT_NUMBER,
+},{
.name = password,
.type = QEMU_OPT_STRING,
},{
.name = disable-ticketing,
.type = QEMU_OPT_BOOL,
+},{
+.name = x509-dir,
+.type = QEMU_OPT_STRING,
+},{
+.name = x509-key-file,
+.type = QEMU_OPT_STRING,
+},{
+.name = x509-key-password,
+.type = QEMU_OPT_STRING,
+},{
+.name = x509-cert-file,
+.type = QEMU_OPT_STRING,
+},{
+.name = x509-cacert-file,
+.type = QEMU_OPT_STRING,
+},{
+.name = x509-dh-key-file,
+.type = QEMU_OPT_STRING,
+},{
+.name = tls-ciphers,
+.type = QEMU_OPT_STRING,
},
{ /* end if list */ }
},
diff --git a/qemu-options.hx b/qemu-options.hx
index 718d47a..9d3f8ef 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -680,7 +680,7 @@ Enable the spice remote desktop protocol. Valid options are
@table @option
@item port=nr
-Set the TCP port spice is listening on.
+Set the TCP port spice is listening on for plaintext channels.
@item password=secret
Set the password you need to authenticate.
@@ -688,6 +688,22 @@ Set the password you need to authenticate.
@item disable-ticketing
Allow client connects without authentication.
+...@item tls-port=nr
+Set the TCP port spice is listening on for encrypted channels.
+
+...@item x509-dir=dir
+Set the x509 file directory. Expects same filenames as -vnc $display,x509=$dir
+
+...@item x509-key-file=file
+...@item x509-key-password=file
+...@item x509-cert-file=file
+...@item x509-cacert-file=file
+...@item x509-dh-key-file=file
+The x509 file names can also be configured individually.
+
+...@item tls-ciphers=list
+Specify which ciphers to use.
+
@end table
ETEXI
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 8b5e4a8..51aa782 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -22,6 +22,7 @@
#include qemu-spice.h
#include qemu-timer.h
#include qemu-queue.h
+#include qemu-x509.h
#include monitor.h
/* core bits */
@@ -141,20 +142,74 @@ static SpiceCoreInterface core_interface = {
void qemu_spice_init(void)
{
QemuOpts *opts = QTAILQ_FIRST(qemu_spice_opts.head);
-const char *password;
-int port;
+const char *password, *str, *x509_dir,
+*x509_key_password = NULL,
+*x509_dh_file = NULL,
+*tls_ciphers = NULL;
+char *x509_key_file = NULL,
+*x509_cert_file = NULL,
+*x509_cacert_file = NULL;
+int port, tls_port, len;
if (!opts) {
return;
}
port = qemu_opt_get_number(opts, port, 0);
-if (!port) {
+tls_port = qemu_opt_get_number(opts, tls-port, 0);
+if (!port !tls_port) {
return;
}
password = qemu_opt_get(opts, password);
+if (tls_port) {
+x509_dir = qemu_opt_get(opts, x509-dir);
+if (NULL == x509_dir) {
+x509_dir = .;
+}
+len = strlen(x509_dir) + 32;
+
+str = qemu_opt_get(opts, x509-key-file);
+if (str) {
+x509_key_file = qemu_strdup(str);
+} else {
+x509_key_file = qemu_malloc(len);
+snprintf(x509_key_file, len, %s/%s, x509_dir,
X509_SERVER_KEY_FILE);
+}
+
+str = qemu_opt_get(opts, x509-cert-file);
+if (str) {
+x509_cert_file = qemu_strdup(str);
+} else {
+x509_cert_file = qemu_malloc(len);
+snprintf(x509_cert_file, len, %s/%s, x509_dir,
X509_SERVER_CERT_FILE);
+}
+
+str = qemu_opt_get(opts, x509-cacert-file);
+if (str) {
+x509_cacert_file = qemu_strdup(str);
+} else {
+x509_cacert_file = qemu_malloc(len);
+snprintf(x509_cacert_file, len, %s/%s, x509_dir,
X509_CA_CERT_FILE);
+}
+
+x509_key_password = qemu_opt_get(opts, x509-key-password);
+x509_dh_file = qemu_opt_get(opts, x509-dh-file);
+tls_ciphers = qemu_opt_get(opts, tls-ciphers);
+}
+
spice_server = spice_server_new();
-spice_server_set_port(spice_server, port);
+if (port) {
+spice_server_set_port(spice_server, port);
+}
+if (tls_port) {
+spice_server_set_tls(spice_server, tls_port,
+ x509_cacert_file,
+
Re: [Qemu-devel] [PATCH 0/2] USB CCID device
- Anthony Liguori anth...@codemonkey.ws wrote: On 10/06/2010 03:55 AM, Gerd Hoffmann wrote: On 10/06/10 02:28, Alon Levy wrote: Does this work with live migration? I can't see how it would. No, it doesn't right now. It would require cooperation with the client, to tell it to reconnect to the target qemu (kind of like spice). I think until we have this migration should have pretty much the same effect as a chardev disconnect, i.e. detach the usb device (which the guest will see as unplug). Better yet, mark the guest as unmigrateable and let the management tool unplug the usb device before migration and replug it after migration. It's the same principle behind device assignment. Is there any way to also get a pre_migrate callback with register_device_unmigratable? I'd like to send a VSC_Reconnect message, then the guest sees an unplug, then migration, then (no plug yet since the device is marked as auto_attach=0) client reconnects (actually this happens before but to a paused machine waiting for migration), and then causes attachement, same as a new machine. Regards, Anthony Liguori Needs some code though, at minimum you'll have to xfer the connected state from the migration source and have some bits in post_load() which do attach/detach if needed. cheers, Gerd
[Qemu-devel] [PATCH 3/5] spice: add config options for channel security.
This allows to enforce tls or plaintext usage for certain spice
channels.
---
qemu-config.c |6 ++
qemu-options.hx |8
ui/spice-core.c | 25 +
3 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 8b545b1..f52e50c 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -392,6 +392,12 @@ QemuOptsList qemu_spice_opts = {
.name = tls-ciphers,
.type = QEMU_OPT_STRING,
},{
+.name = tls-channel,
+.type = QEMU_OPT_STRING,
+},{
+.name = plaintext-channel,
+.type = QEMU_OPT_STRING,
+},{
.name = image-compression,
.type = QEMU_OPT_STRING,
},{
diff --git a/qemu-options.hx b/qemu-options.hx
index 59db632..bb45b67 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -704,6 +704,14 @@ The x509 file names can also be configured individually.
@item tls-ciphers=list
Specify which ciphers to use.
+...@item tls-channel=[main|display|inputs|record|playback|tunnel]
+...@item plaintext-channel=[main|display|inputs|record|playback|tunnel]
+Force specific channel to be used with or without TLS encryption. The
+options can be specified multiple times to configure multiple
+channels. The special name default can be used to set the default
+mode. For channels which are not explicitly forced into one mode the
+spice client is allowed to pick tls/plaintext as he pleases.
+
@item image-compression=[auto_glz|auto_lz|quic|glz|lz|off]
Configure image compression (lossless).
Default is auto_glz.
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 1567046..8f73848 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -192,6 +192,29 @@ static const char *wan_compression_names[] = {
/* functions for the rest of qemu */
+static int add_channel(const char *name, const char *value, void *opaque)
+{
+int security = 0;
+int rc;
+
+if (strcmp(name, tls-channel) == 0)
+security = SPICE_CHANNEL_SECURITY_SSL;
+if (strcmp(name, plaintext-channel) == 0)
+security = SPICE_CHANNEL_SECURITY_NONE;
+if (security == 0)
+return 0;
+if (strcmp(value, default) == 0) {
+rc = spice_server_set_channel_security(spice_server, NULL, security);
+} else {
+rc = spice_server_set_channel_security(spice_server, value, security);
+}
+if (rc != 0) {
+fprintf(stderr, spice: failed to set channel security for %s\n,
value);
+exit(1);
+}
+return 0;
+}
+
void qemu_spice_init(void)
{
QemuOpts *opts = QTAILQ_FIRST(qemu_spice_opts.head);
@@ -293,6 +316,8 @@ void qemu_spice_init(void)
}
spice_server_set_zlib_glz_compression(spice_server, wan_compr);
+qemu_opt_foreach(opts, add_channel, NULL, 0);
+
spice_server_init(spice_server, core_interface);
using_spice = 1;
--
1.7.1
[Qemu-devel] Re: [PATCH 1/3] Make kvm64 the default cpu model when kvm_enabled()
On Wed, Oct 06, 2010 at 03:24:59PM -0400, Anthony Liguori wrote:
+qemu_compat_version = machine-compat_version;
+
if (display_type == DT_NOGRAPHIC) {
if (default_parallel)
add_device_config(DEV_PARALLEL, null);
--
1.7.0.4
Looks fine to me, given CPUs are not in qdev. Anthony?
The idea is fine, but why not just add the default CPU to the machine
description?
If I remember correctly the reason was that the machine description was
not accessible in the cpuid initialization path because it is a function
local variable. I could have made it a global variable but considered
the compat_version approach simpler. The qemu_compat_version might also
be useful at other places.
Joerg
--
AMD Operating System Research Center
Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Re: [Qemu-devel] Re: [PATCH 09/11] i386: avoid a write only variable
On Thu, 7 Oct 2010, Paolo Bonzini wrote: On 10/06/2010 11:34 PM, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-i386/op_helper.c: In function 'switch_tss': /src/qemu/target-i386/op_helper.c:283:53: error: variable 'new_trap' set but not used [-Werror=unused-but-set-variable] Fix by deleting the variable. Again, this warning tells us the emulation is incorrect, so it's wrong to remove it. http://support.amd.com/us/Processor_TechDocs/24593.pdf 12.2.5 T (Trap) Bit--■Bit 0 of byte 64h, static field. This bit, when set to1, causes a debug exception (#DB) to occur on a task switch. See Beakpoint Instruction (INT3) page 340 for additional information. current code never checks and never traps, which is indeed not correct. -- mailto:av1...@comtv.ru
[Qemu-devel] [Bug 642304] Re: Solaris/x86 v10 hangs under KVM
Your bug report doesn't tell us anything about the host system (AMD, Intel, which CPU model etc), nor which version of KVM you are running, which distro etc? Did it work with older versions of qemu-kvm? Which version of Solaris/x86 (pointer to version preferably) Please provide appropriate information if you want a chance that anyone will look at this. Jes ** Changed in: qemu Status: New = Incomplete -- Solaris/x86 v10 hangs under KVM https://bugs.launchpad.net/bugs/642304 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Incomplete Bug description: Solaris/x86 10 guest hangs when running under KVM with the message Running Configuration Assistant. It runs fine when -enable-kvm isn't given as a command option. Host OS: Linux/x86_64 Guest OS: Solaris/x86 Command Line: qemu -hda solaris.img -m 192 -boot c -enable-kvm Build Configure: ./configure --enable-linux-aio --enable-io-thread --enable-kvm GIT commit: 58aebb946acff82c62383f350cab593e55cc13dc
Re: [Qemu-devel] Re: [PATCH 04/11] lsi53c895a: avoid a write only variable
Paolo Bonzini pbonz...@redhat.com writes: On 10/06/2010 11:32 PM, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced a warning: /src/qemu/hw/lsi53c895a.c: In function 'lsi_do_msgout': /src/qemu/hw/lsi53c895a.c:848:9: error: variable 'len' set but not used [-Werror=unused-but-set-variable] Fix by making the variable declaration and its uses also conditional to debug definition. NACK, this uglifies the code and loses track of _what_ is that msgbyte we're reading. Seconded. If the warning bothers you, maybe (void)len can silence it in a less unsightly manner.
Re: [Qemu-devel] [PATCH] acpi: Fix an infinite loop in acpi_table_add
On 10/05/10 02:23, Vincent Minet wrote: Commit d729bb9a7700e364b1c5f9893d61f07a9e002bce has a typo, causing an infinite loop in acpi_table_add. Signed-off-by: Vincent Minet vinc...@vincent-minet.net --- hw/acpi.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) OUCH! Good catch! Acked-by: Jes Sorensen jes.soren...@redhat.com Jes
[Qemu-devel] [Bug 643465] Re: Crash at network boot
Hi, The backtrace shows it crashes in lsi_update_irq() which is in the SCSI emulation, not the networking code. Please try and see if this happens if you use IDE or virtio-blk for your disk drives. Second, please provide proper information about your software stack: - kernel version - qemu-kvm version - Linux distro - hardware config Jes ** Changed in: qemu Status: New = Incomplete -- Crash at network boot https://bugs.launchpad.net/bugs/643465 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Incomplete Bug description: When I boot on lan, I crash qemu: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7491a710 (LWP 10614)] 0x005a1de8 in lsi_update_irq (s=0x125d5a0) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/hw/lsi53c895a.c:426 426 /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/hw/lsi53c895a.c: No such file or directory. in /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/hw/lsi53c895a.c (gdb) bt #0 0x005a1de8 in lsi_update_irq (s=0x125d5a0) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/hw/lsi53c895a.c:426 #1 0x005a4f67 in lsi_mmio_writew (opaque=0x125d5a0, addr=value optimized out, val=2) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/hw/lsi53c895a.c:1775 #2 0x004fdf3b in cpu_physical_memory_rw (addr=4043505728, buf=0x77ff2028 \002, len=2, is_write=1) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/exec.c:3215 #3 0x0042bf65 in handle_mmio (env=0xcaa6d0) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/qemu-kvm.c:831 #4 kvm_run (env=0xcaa6d0) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/qemu-kvm.c:979 #5 0x0042c249 in kvm_cpu_exec (env=0x125d5a0) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/qemu-kvm.c:1651 #6 0x0042c471 in kvm_main_loop_cpu (_env=value optimized out) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/qemu-kvm.c:1893 #7 ap_main_loop (_env=value optimized out) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/qemu-kvm.c:1943 #8 0x779c0894 in start_thread (arg=value optimized out) at pthread_create.c:297 #9 0x75ac927d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:115
Re: [Qemu-devel] [PATCH 01/11] block: avoid a write only variable
Blue Swirl blauwir...@gmail.com writes: Compiling with GCC 4.6.0 20100925 produced a warning: /src/qemu/block/qcow2-refcount.c: In function 'update_refcount': /src/qemu/block/qcow2-refcount.c:552:13: error: variable 'dummy' set but not used [-Werror=unused-but-set-variable] Fix by adding a function that does not generate a warning when the result is unused. What about a simple (void)dummy instead?
Re: [Qemu-devel] [PATCH 03/11] eepro100: initialize a variable in all cases
Blue Swirl blauwir...@gmail.com writes: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/hw/eepro100.c: In function 'eepro100_read4': /src/qemu/hw/eepro100.c:1351:14: error: 'val' may be used uninitialized in this function [-Werror=uninitialized] /src/qemu/hw/eepro100.c: In function 'eepro100_read2': /src/qemu/hw/eepro100.c:1328:14: error: 'val' may be used uninitialized in this function [-Werror=uninitialized] /src/qemu/hw/eepro100.c: In function 'eepro100_read1': /src/qemu/hw/eepro100.c:1285:13: error: 'val' may be used uninitialized in this function [-Werror=uninitialized] Fix by initializing 'val' at start. I'm worried this sweeps bugs under the carpet. When addr is out of bounds, these function return garbage. Your patch makes them return 0 instead. Can that happen? Shouldn't we catch and flag it?
Re: [Qemu-devel] [Bug 639651] Re: DRIVER_IRQL_NOT_LESS_OR_EQUAL booting WIndows XP with Synaptics driver installed
On 7 October 2010 11:05, Jes Sorensen 639...@bugs.launchpad.net wrote: Just to be sure, you are not using the virtio-blk driver for Windows here? I have seen similar crashes with the older version of virtio-blk when used on recent versions of KVM. -- DRIVER_IRQL_NOT_LESS_OR_EQUAL booting WIndows XP with Synaptics driver installed https://bugs.launchpad.net/bugs/639651 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Status in Debian GNU/Linux: New Bug description: Positng the issue here since I did not get any reply on the ML. I was trying to update some windows XP (SP3) images in kvm. It worked fine several times but last time I added mass storage drivers to sysprep and now on the second boot after reseal (the first is mini-setup) I get a BSOD with message DRIVER_IRQL_NOT_LESS_OR_EQUAL. It turns out that the error is unrelated to storage drivers. It is triggered by Synaptics driver installing for the PS2 mouse in kvm (which does not happen in VirtualBox or on real hardware). The image is originally created on hardware with MP acpi (not virtualization). qemu-kvm 0.12.5+dfsg-2 Actually the issue is caused by the Synaptics touchpad driver binding to the PS/2 mouse device in qemu. I have no idea how PS/2 devices are detected but the one present in qemu is misdetected as a synaptics touchapd by the Synaptics driver for Windows. As a workaround I have patched my qemu to not enable the PS/2 mouse device. Thanks Michal
[Qemu-devel] [Bug 655555] Re: -cpu ? doesn't work as documented
Seems like this isn't a bug after all, so closing. If you want to add a note to the manual, that is cool, but please submit a patch, otherwise I doubt it will get addressed. It should also be a more generic note since it would apply to all arguments that accept ? Cheers, Jes ** Changed in: qemu Status: New = Invalid -- -cpu ? doesn't work as documented https://bugs.launchpad.net/bugs/65 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Invalid Bug description: I've got qemu-kvm 0.12.5 on Gentoo Linux on an amd64 host. Both HTML documentation and compiled-in command line help state that -cpu ? should give me a list of possible values for the -cpu command line switch. In fact that switch results in an error message: Unable to find x86 CPU definition. Either the documentation or the implementation should be adjusted accordingly.
Re: [Qemu-devel] [Bug 639651] Re: DRIVER_IRQL_NOT_LESS_OR_EQUAL booting WIndows XP with Synaptics driver installed
On 10/07/10 11:51, Michal Suchanek wrote: Actually the issue is caused by the Synaptics touchpad driver binding to the PS/2 mouse device in qemu. I have no idea how PS/2 devices are detected but the one present in qemu is misdetected as a synaptics touchapd by the Synaptics driver for Windows. As a workaround I have patched my qemu to not enable the PS/2 mouse device. Hi Michal, If you have the time to look, it would be interesting to see what actually goes over the wire to/from the PS/2 driver in QEMU just prior to the crash. It would be good to get this fixed. Cheers, Jes
[Qemu-devel] [Bug 586420] Re: WinXP install cd hangs at boot time if machine started with floppy
Closing per tekditt's posting on 2010-07-22 ** Changed in: qemu Status: Incomplete = Fix Committed -- WinXP install cd hangs at boot time if machine started with floppy https://bugs.launchpad.net/bugs/586420 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Fix Committed Bug description: I have a second problem: I wanted to install Windows Server 2003 on a virtio drive, so I tried to start the machine with the install cd as the boot drive and a floppy image with the viostor drivers. The problem is, the install cd hangs at boot time. If I start VNC I just see a black ground with 640x480. I've also tried this with the install cd of Windows Server 2008 R2 and it works! Could it be that the BIOS screws up because the older install cds are using the floppy emulation to boot the setup?
[Qemu-devel] Re: [PATCHv2] qemu-kvm/vhost: fix up irqfd support
On Wed, Oct 06, 2010 at 04:05:38PM -0600, Alex Williamson wrote:
On Wed, 2010-10-06 at 23:44 +0200, Michael S. Tsirkin wrote:
On Wed, Oct 06, 2010 at 11:24:24AM -0600, Alex Williamson wrote:
You could always keep the functions as separate wrapper callers of the
common function so you only need to keep true = unset, false = set
straight in one place. Thanks,
Just to show why it does not work, I did exactly this: as you see the
code is shorter but the true/false magic gets spread: it was in 2
places, (set/unset) now it is in 4 places and it is within the loop, in
code that is more complex.
You seem to have missed the wrapper function. I'm simply suggesting
something like this:
Good idea. I tweaked the code a bit more and came up with this:
I think I will keep this as an incremental patch just to
keep diffs small.
msix: factor out mask notifier code
Move some common code handling msix mask notifiers to a function.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
diff --git a/hw/msix.c b/hw/msix.c
index 3d4dd61..5f66d20 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -583,40 +583,26 @@ void msix_unuse_all_vectors(PCIDevice *dev)
msix_free_irq_entries(dev);
}
-static int msix_set_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
+/* Invoke the notifier if vector entry is used and unmasked. */
+static int msix_notify_if_unmasked(PCIDevice *dev, unsigned vector, int masked)
{
-int r = 0;
-if (vector = dev-msix_entries_nr || !dev-msix_entry_used[vector])
-return 0;
-
assert(dev-msix_mask_notifier);
-
-/* Unmask the new notifier unless vector is masked. */
-if (!msix_is_masked(dev, vector)) {
-r = dev-msix_mask_notifier(dev, vector, false);
-if (r 0) {
-return r;
-}
+if (!dev-msix_entry_used[vector] || msix_is_masked(dev, vector)) {
+return 0;
}
-return r;
+return dev-msix_mask_notifier(dev, vector, masked);
}
-static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
+static int msix_set_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
{
-int r = 0;
-if (vector = dev-msix_entries_nr || !dev-msix_entry_used[vector])
-return 0;
-
-assert(dev-msix_mask_notifier);
+ /* Notifier has been set. Invoke it on unmasked vectors. */
+ return msix_notify_if_unmasked(dev, vector, 0);
+}
-/* Mask the old notifier unless it is already masked. */
-if (!msix_is_masked(dev, vector)) {
-r = dev-msix_mask_notifier(dev, vector, true);
-if (r 0) {
-return r;
-}
-}
-return r;
+static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
+{
+ /* Notifier will be unset. Invoke it to mask unmasked entries. */
+ return msix_notify_if_unmasked(dev, vector, 1);
}
int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func f)
Re: [Qemu-devel] [PATCH 07/11] cris: avoid write only variables
Blue Swirl blauwir...@gmail.com writes: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-cris/op_helper.c: In function 'helper_movl_sreg_reg': /src/qemu/target-cris/op_helper.c:145:8: error: variable 'tlb_v' set but not used [-Werror=unused-but-set-variable] In file included from /src/qemu/target-cris/translate.c:3154:0: /src/qemu/target-cris/translate_v10.c: In function 'dec10_prep_move_m': /src/qemu/target-cris/translate_v10.c:111:22: error: variable 'rd' set but not used [-Werror=unused-but-set-variable] Fix by making the variable declarations and their uses also conditional to debug definition, delete rd. Signed-off-by: Blue Swirl blauwir...@gmail.com --- target-cris/op_helper.c |6 ++ target-cris/translate_v10.c |5 ++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c index a60da94..28c79b1 100644 --- a/target-cris/op_helper.c +++ b/target-cris/op_helper.c @@ -142,7 +142,9 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg) uint32_t idx; uint32_t lo, hi; uint32_t vaddr; +#ifdef CRIS_OP_HELPER_DEBUG int tlb_v; +#endif idx = set = env-sregs[SFR_RW_MM_TLB_SEL]; set = 4; @@ -157,13 +159,17 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg) vaddr = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].hi, 13, 31); vaddr = TARGET_PAGE_BITS; +#ifdef CRIS_OP_HELPER_DEBUG tlb_v = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].lo, 3, 3); +#endif env-tlbsets[srs - 1][set][idx].lo = lo; env-tlbsets[srs - 1][set][idx].hi = hi; +#ifdef CRIS_OP_HELPER_DEBUG D_LOG(tlb flush vaddr=%x v=%d pc=%x\n, vaddr, tlb_v, env-pc); +#endif tlb_flush_page(env, vaddr); } } Could we eliminate the bothersome variable instead? diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c index a60da94..94e3e27 100644 --- a/target-cris/op_helper.c +++ b/target-cris/op_helper.c @@ -142,7 +142,6 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg) uint32_t idx; uint32_t lo, hi; uint32_t vaddr; - int tlb_v; idx = set = env-sregs[SFR_RW_MM_TLB_SEL]; set = 4; @@ -157,13 +156,14 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg) vaddr = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].hi, 13, 31); vaddr = TARGET_PAGE_BITS; - tlb_v = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].lo, + D_LOG(tlb flush vaddr=%x v=%d pc=%x\n, + vaddr, + EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].lo, 3, 3); + env-pc); env-tlbsets[srs - 1][set][idx].lo = lo; env-tlbsets[srs - 1][set][idx].hi = hi; - D_LOG(tlb flush vaddr=%x v=%d pc=%x\n, - vaddr, tlb_v, env-pc); tlb_flush_page(env, vaddr); } } Untested. [...]
Re: [Qemu-devel] [Bug 639651] Re: DRIVER_IRQL_NOT_LESS_OR_EQUAL booting WIndows XP with Synaptics driver installed
I have no idea how to log the data. I looked at the qemu man page but it does not even mention the PS/2 mouse as a chardev nor offers an option to log traffic of chardevs without attaching them to a file and thus detaching them from the emulated device. Thanks Michal -- DRIVER_IRQL_NOT_LESS_OR_EQUAL booting WIndows XP with Synaptics driver installed https://bugs.launchpad.net/bugs/639651 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Status in Debian GNU/Linux: New Bug description: Positng the issue here since I did not get any reply on the ML. I was trying to update some windows XP (SP3) images in kvm. It worked fine several times but last time I added mass storage drivers to sysprep and now on the second boot after reseal (the first is mini-setup) I get a BSOD with message DRIVER_IRQL_NOT_LESS_OR_EQUAL. It turns out that the error is unrelated to storage drivers. It is triggered by Synaptics driver installing for the PS2 mouse in kvm (which does not happen in VirtualBox or on real hardware). The image is originally created on hardware with MP acpi (not virtualization). qemu-kvm 0.12.5+dfsg-2
[Qemu-devel] Re: [PATCH] lsi53c895a: Add support for OS/2 Warp SYM8XX.ADD driver
Am 30.09.2010 07:07, schrieb Nicholas A. Bellinger: From: Nicholas Bellinger n...@linux-iscsi.org Greetings Paul, Jan, Kevin and co, This series is against my v0.12.5 qemu-kvm.git that contains QEMU SCSI layer SGL passthrough from Gerd Hoffman, 8708EM2 MegaSas emulation from Dr. Hannes Reinecke, and well as my own hw/scsi-bsg.c support. This tree is located here: http://git.kernel.org/?p=virt/kvm/nab/qemu-kvm.git;a=summary This first patch adds a missing qdev-reset() NOP caller in hw/scsi-generic.c that is now expected by lsi53c895a.c in = v0.12.5 code. You will want to apply this to all = v0.12.5 QEMU trees so scsi-generic does not segfault with lsi53c895a expecting a valid qdev-reset(). This one should already be fixed with Bernhard Kohl's commit f8b6d672, right? (Should have reached git master about two weeks ago) The second item is a bit more exotic.. 8-) So I have been thinking about how to get qemu-kvm.git scsi-generic - TCM_Loop to function with OS/2 Warp v4 (SP15) in guest for a while now, and I am happy to report that after sending some time in the last weeks getting OS/2 setup (hey, it has been +13 years) and finding a functioning sym53c895a driver, and finally finding a working SYM8XX.ADD and being able to fill in missing informational registers and adding a workaround to fix a bogus Destination ID register WRITE from the now +10 year old SYM8XX.ADD driver code. Here are some screenshots of the patch in action: *) Booting with BASEDEV=SYM8XX.ADD /V: http://linux-iscsi.org/index.php/Image:TCM_Loop-OS2Warp-QEMU-KVM-boot.png *) TCM_loop - LIO-ORG SPC-3 LUN from KVM Host in the OS/2 Workplace shell! http://linux-iscsi.org/index.php/Image:TCM_Loop-OS2Warp-QEMU-KVM-running.png Nice. :-) Kevin
Re: [Qemu-devel] [PATCH 07/11] cris: avoid write only variables
On Thu, Oct 07, 2010 at 12:08:05PM +0200, Markus Armbruster wrote:
Blue Swirl blauwir...@gmail.com writes:
Compiling with GCC 4.6.0 20100925 produced warnings:
/src/qemu/target-cris/op_helper.c: In function 'helper_movl_sreg_reg':
/src/qemu/target-cris/op_helper.c:145:8: error: variable 'tlb_v' set
but not used [-Werror=unused-but-set-variable]
In file included from /src/qemu/target-cris/translate.c:3154:0:
/src/qemu/target-cris/translate_v10.c: In function 'dec10_prep_move_m':
/src/qemu/target-cris/translate_v10.c:111:22: error: variable 'rd' set
but not used [-Werror=unused-but-set-variable]
Fix by making the variable declarations and their uses also conditional
to debug definition, delete rd.
Signed-off-by: Blue Swirl blauwir...@gmail.com
---
target-cris/op_helper.c |6 ++
target-cris/translate_v10.c |5 ++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index a60da94..28c79b1 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -142,7 +142,9 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg)
uint32_t idx;
uint32_t lo, hi;
uint32_t vaddr;
+#ifdef CRIS_OP_HELPER_DEBUG
int tlb_v;
+#endif
idx = set = env-sregs[SFR_RW_MM_TLB_SEL];
set = 4;
@@ -157,13 +159,17 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t
reg)
vaddr = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].hi,
13, 31);
vaddr = TARGET_PAGE_BITS;
+#ifdef CRIS_OP_HELPER_DEBUG
tlb_v = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].lo,
3, 3);
+#endif
env-tlbsets[srs - 1][set][idx].lo = lo;
env-tlbsets[srs - 1][set][idx].hi = hi;
+#ifdef CRIS_OP_HELPER_DEBUG
D_LOG(tlb flush vaddr=%x v=%d pc=%x\n,
vaddr, tlb_v, env-pc);
+#endif
tlb_flush_page(env, vaddr);
}
}
Could we eliminate the bothersome variable instead?
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index a60da94..94e3e27 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -142,7 +142,6 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg)
uint32_t idx;
uint32_t lo, hi;
uint32_t vaddr;
- int tlb_v;
idx = set = env-sregs[SFR_RW_MM_TLB_SEL];
set = 4;
@@ -157,13 +156,14 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg)
vaddr = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].hi,
13, 31);
vaddr = TARGET_PAGE_BITS;
- tlb_v = EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].lo,
+ D_LOG(tlb flush vaddr=%x v=%d pc=%x\n,
+ vaddr,
+ EXTRACT_FIELD(env-tlbsets[srs-1][set][idx].lo,
3, 3);
+ env-pc);
env-tlbsets[srs - 1][set][idx].lo = lo;
env-tlbsets[srs - 1][set][idx].hi = hi;
- D_LOG(tlb flush vaddr=%x v=%d pc=%x\n,
- vaddr, tlb_v, env-pc);
tlb_flush_page(env, vaddr);
}
}
Hi,
Sorry for the late answer.
Markus, I agree that removing tlb_v would have been better than ifdefs,
but i think that the intent I originally had in mind was that there should
not be a need to flush the entry from the QEMU TLB if the old guest
entry was not valid.
The following patch works on my side:
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index a60da94..be9eb06 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -164,7 +164,9 @@ void helper_movl_sreg_reg (uint32_t sreg, uint32_t reg)
D_LOG(tlb flush vaddr=%x v=%d pc=%x\n,
vaddr, tlb_v, env-pc);
- tlb_flush_page(env, vaddr);
+ if (tlb_v) {
+ tlb_flush_page(env, vaddr);
+ }
}
}
#endif
The target-cris/translate_v10.c hunk looks good.
Blue, can you incorporate the tlb_v change in your patch set?
Or if you prefer, I can commit that part on my side.
Cheers
[Qemu-devel] [PATCH] .gitignore: Ignore *-timestamp
Timestamp files were recently added to reduce make churn on source files that use tracing. The timestamp files should never be committed and should not be visible in git status. Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- .gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index 72bff98..a43e4d1 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ config-host.* config-target.* trace.h trace.c +*-timestamp *-softmmu *-darwin-user *-linux-user -- 1.7.1
[Qemu-devel] [PATCH 1/3] vnc: auth reject cleanup
protocol_client_auth_vnc() has two places where the auth can fail,
with identical code sending the reject message to the client.
Move the common code to the end of the function and make both
error paths jump there. No functional change.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
ui/vnc.c | 30 +-
1 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index c7a1831..1ef0fc5 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2081,15 +2081,7 @@ static int protocol_client_auth_vnc(VncState *vs,
uint8_t *data, size_t len)
if (!vs-vd-password || !vs-vd-password[0]) {
VNC_DEBUG(No password configured on server);
-vnc_write_u32(vs, 1); /* Reject auth */
-if (vs-minor = 8) {
-static const char err[] = Authentication failed;
-vnc_write_u32(vs, sizeof(err));
-vnc_write(vs, err, sizeof(err));
-}
-vnc_flush(vs);
-vnc_client_error(vs);
-return 0;
+goto reject;
}
memcpy(response, vs-challenge, VNC_AUTH_CHALLENGE_SIZE);
@@ -2105,14 +2097,7 @@ static int protocol_client_auth_vnc(VncState *vs,
uint8_t *data, size_t len)
/* Compare expected vs actual challenge response */
if (memcmp(response, data, VNC_AUTH_CHALLENGE_SIZE) != 0) {
VNC_DEBUG(Client challenge reponse did not match\n);
-vnc_write_u32(vs, 1); /* Reject auth */
-if (vs-minor = 8) {
-static const char err[] = Authentication failed;
-vnc_write_u32(vs, sizeof(err));
-vnc_write(vs, err, sizeof(err));
-}
-vnc_flush(vs);
-vnc_client_error(vs);
+goto reject;
} else {
VNC_DEBUG(Accepting VNC challenge response\n);
vnc_write_u32(vs, 0); /* Accept auth */
@@ -2121,6 +2106,17 @@ static int protocol_client_auth_vnc(VncState *vs,
uint8_t *data, size_t len)
start_client_init(vs);
}
return 0;
+
+reject:
+vnc_write_u32(vs, 1); /* Reject auth */
+if (vs-minor = 8) {
+static const char err[] = Authentication failed;
+vnc_write_u32(vs, sizeof(err));
+vnc_write(vs, err, sizeof(err));
+}
+vnc_flush(vs);
+vnc_client_error(vs);
+return 0;
}
void start_auth_vnc(VncState *vs)
--
1.7.1
[Qemu-devel] [PATCH 0/3] vnc/spice: add monitor command to change password.
Hi, This patch series adds a new monitor command to set the password with optional expiry time for spice and vnc remote desktop connections. The patches are also available in the git repository at: git://anongit.freedesktop.org/spice/qemu passwd.1 Gerd Hoffmann (3): vnc: auth reject cleanup vnc: support password expire vnc/spice: add set_passwd monitor command. console.h |2 +- hmp-commands.hx | 23 monitor.c | 61 +- ui/qemu-spice.h |3 ++ ui/spice-core.c |7 ++ ui/vnc.c| 43 +++--- ui/vnc.h|1 + 7 files changed, 120 insertions(+), 20 deletions(-)
[Qemu-devel] [PATCH 2/3] vnc: support password expire
This patch adds support for expiring passwords to vnc. It adds a new
lifetime parameter to the vnc_display_password() function, which
specifies the number of seconds the new password will be valid. Passing
zero as lifetime maintains current behavior (password never expires).
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
console.h |2 +-
monitor.c |3 +--
ui/vnc.c | 15 ++-
ui/vnc.h |1 +
4 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/console.h b/console.h
index aafb031..24670e5 100644
--- a/console.h
+++ b/console.h
@@ -368,7 +368,7 @@ void cocoa_display_init(DisplayState *ds, int full_screen);
void vnc_display_init(DisplayState *ds);
void vnc_display_close(DisplayState *ds);
int vnc_display_open(DisplayState *ds, const char *display);
-int vnc_display_password(DisplayState *ds, const char *password);
+int vnc_display_password(DisplayState *ds, const char *password, int lifetime);
void do_info_vnc_print(Monitor *mon, const QObject *data);
void do_info_vnc(Monitor *mon, QObject **ret_data);
char *vnc_display_local_addr(DisplayState *ds);
diff --git a/monitor.c b/monitor.c
index fbb678d..d82eb9e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -966,11 +966,10 @@ static int do_quit(Monitor *mon, const QDict *qdict,
QObject **ret_data)
static int change_vnc_password(const char *password)
{
-if (vnc_display_password(NULL, password) 0) {
+if (vnc_display_password(NULL, password, 0) 0) {
qerror_report(QERR_SET_PASSWD_FAILED);
return -1;
}
-
return 0;
}
diff --git a/ui/vnc.c b/ui/vnc.c
index 1ef0fc5..51aa9ca 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2078,11 +2078,19 @@ static int protocol_client_auth_vnc(VncState *vs,
uint8_t *data, size_t len)
unsigned char response[VNC_AUTH_CHALLENGE_SIZE];
int i, j, pwlen;
unsigned char key[8];
+time_t now;
if (!vs-vd-password || !vs-vd-password[0]) {
VNC_DEBUG(No password configured on server);
goto reject;
}
+if (vs-vd-expires) {
+time(now);
+if (vs-vd-expires now) {
+VNC_DEBUG(Password is expired);
+goto reject;
+}
+}
memcpy(response, vs-challenge, VNC_AUTH_CHALLENGE_SIZE);
@@ -2474,7 +2482,7 @@ void vnc_display_close(DisplayState *ds)
#endif
}
-int vnc_display_password(DisplayState *ds, const char *password)
+int vnc_display_password(DisplayState *ds, const char *password, int lifetime)
{
VncDisplay *vs = ds ? (VncDisplay *)ds-opaque : vnc_display;
@@ -2492,6 +2500,11 @@ int vnc_display_password(DisplayState *ds, const char
*password)
if (vs-auth == VNC_AUTH_NONE) {
vs-auth = VNC_AUTH_VNC;
}
+if (lifetime) {
+vs-expires = time(NULL) + lifetime;
+} else {
+vs-expires = 0;
+}
} else {
vs-auth = VNC_AUTH_NONE;
}
diff --git a/ui/vnc.h b/ui/vnc.h
index 9619b24..4f895be 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -120,6 +120,7 @@ struct VncDisplay
char *display;
char *password;
+time_t expires;
int auth;
bool lossy;
#ifdef CONFIG_VNC_TLS
--
1.7.1
[Qemu-devel] [PATCH 3/3] vnc/spice: add set_passwd monitor command.
This patch adds a new set_password monitor command which allows to
change the password for spice and vnc connections. See the doc update
patch chunk for details.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hmp-commands.hx | 23 +
monitor.c | 58 +++
ui/qemu-spice.h |3 ++
ui/spice-core.c |7 ++
4 files changed, 91 insertions(+), 0 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 81999aa..a972b80 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1116,6 +1116,29 @@ Set the encrypted device @var{device} password to
@var{password}
ETEXI
{
+.name = set_password,
+.args_type = protocol:s,password:s,expiration:i,connected:s?,
+.params = protocol password expiration action-if-connected,
+.help = set spice/vnc password,
+ .user_print = monitor_user_noop,
+.mhandler.cmd_new = set_password,
+},
+
+STEXI
+...@item set_password [ vnc | spice ] password expiration [
action-if-connected ]
+...@findex set_password
+
+Change spice/vnc password. @var{expiration} specifies the number of
+seconds the password will stay valid. Use zero to make the password
+stay valid forever. @var{action-if-connected} specifies what should
+happen in case a connection is established: @var{fail} makes the
+password change fail. @var{disconnect} changes the password and
+disconnects the client. @var{keep} changes the password and keeps the
+connection up. @var{keep} is the default.
+
+ETEXI
+
+{
.name = info,
.args_type = item:s?,
.params = [subcommand],
diff --git a/monitor.c b/monitor.c
index d82eb9e..32a20ba 100644
--- a/monitor.c
+++ b/monitor.c
@@ -34,6 +34,7 @@
#include net.h
#include net/slirp.h
#include qemu-char.h
+#include ui/qemu-spice.h
#include sysemu.h
#include monitor.h
#include readline.h
@@ -1021,6 +1022,63 @@ static int do_change(Monitor *mon, const QDict *qdict,
QObject **ret_data)
return ret;
}
+static int set_password(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+const char *protocol = qdict_get_str(qdict, protocol);
+const char *password = qdict_get_str(qdict, password);
+const char *connected = qdict_get_try_str(qdict, connected);
+int lifetime = qdict_get_int(qdict, expiration);
+int disconnect_if_connected = 0;
+int fail_if_connected = 0;
+int rc;
+
+if (connected) {
+if (strcmp(connected, fail) == 0) {
+fail_if_connected = 1;
+} else if (strcmp(connected, disconnect) == 0) {
+disconnect_if_connected = 1;
+} else if (strcmp(connected, keep) == 0) {
+/* nothing */
+} else {
+qerror_report(QERR_INVALID_PARAMETER, connected);
+return -1;
+}
+}
+
+if (strcmp(protocol, spice) == 0) {
+if (!using_spice) {
+/* correct one? spice isn't a device ,,, */
+qerror_report(QERR_DEVICE_NOT_ACTIVE, spice);
+return -1;
+}
+rc = qemu_spice_set_passwd(password, lifetime,
+ fail_if_connected,
+ disconnect_if_connected);
+if (rc != 0) {
+qerror_report(QERR_SET_PASSWD_FAILED);
+return -1;
+}
+return 0;
+}
+
+if (strcmp(protocol, vnc) == 0) {
+if (fail_if_connected || disconnect_if_connected) {
+/* vnc supports connected=keep only */
+qerror_report(QERR_INVALID_PARAMETER, connected);
+return -1;
+}
+rc = vnc_display_password(NULL, password, lifetime);
+if (rc != 0) {
+qerror_report(QERR_SET_PASSWD_FAILED);
+return -1;
+}
+return 0;
+}
+
+qerror_report(QERR_INVALID_PARAMETER, protocol);
+return -1;
+}
+
static int do_screen_dump(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
vga_hw_screen_dump(qdict_get_str(qdict, filename));
diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h
index 063c7dc..0e6e748 100644
--- a/ui/qemu-spice.h
+++ b/ui/qemu-spice.h
@@ -31,10 +31,13 @@ void qemu_spice_init(void);
void qemu_spice_input_init(void);
void qemu_spice_display_init(DisplayState *ds);
int qemu_spice_add_interface(SpiceBaseInstance *sin);
+int qemu_spice_set_passwd(const char *passwd, int lifetime,
+ bool fail_if_connected, bool
disconnect_if_connected);
#else /* CONFIG_SPICE */
#define using_spice 0
+#define qemu_spice_set_passwd(_p, _l, _f1, _f2) (-1)
#endif /* CONFIG_SPICE */
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 8b5e4a8..6d30755 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -176,6 +176,13 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin)
return spice_server_add_interface(spice_server, sin);
}
+int qemu_spice_set_passwd(const char *passwd,
[Qemu-devel] Re: [PATCH] lsi53c895a: add support for ABORT messages
Am 06.09.2010 16:42, schrieb Bernhard Kohl:
If these messages are not handled correctly the guest driver may hang.
Always mandatory:
- ABORT
- BUS DEVICE RESET
Mandatory if tagged queuing is implemented (which disks usually do):
- ABORT TAG
- CLEAR QUEUE
Signed-off-by: Bernhard Kohl bernhard.k...@nsn.com
Nicholas, as you seem to have touched the lsi code recently, care to
review this one? Assuming that you are reasonably familiar with both the
hardware and the code, you should be quicker than me with this.
Kevin
---
hw/lsi53c895a.c | 57
+++
1 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 5eaf69e..40f2d10 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -846,6 +846,18 @@ static void lsi_do_msgout(LSIState *s)
{
uint8_t msg;
int len;
+uint32_t current_tag;
+SCSIDevice *current_dev;
+lsi_request *p, *p_next;
+int id;
+
+if (s-current) {
+current_tag = s-current-tag;
+} else {
+current_tag = s-select_tag;
+}
+id = (current_tag 8) 0xf;
+current_dev = s-bus.devs[id];
DPRINTF(MSG out len=%d\n, s-dbc);
while (s-dbc) {
@@ -890,6 +902,51 @@ static void lsi_do_msgout(LSIState *s)
BADF(ORDERED queue not implemented\n);
s-select_tag |= lsi_get_msgbyte(s) | LSI_TAG_VALID;
break;
+case 0x0d:
+/* The ABORT TAG message clears the current I/O process only. */
+DPRINTF(MSG: ABORT TAG tag=0x%x\n, current_tag);
+current_dev-info-cancel_io(current_dev, current_tag);
+lsi_disconnect(s);
+break;
+case 0x06:
+case 0x0e:
+case 0x0c:
+/* The ABORT message clears all I/O processes for the selecting
+ initiator on the specified logical unit of the target. */
+if (msg == 0x06) {
+DPRINTF(MSG: ABORT tag=0x%x\n, current_tag);
+}
+/* The CLEAR QUEUE message clears all I/O processes for all
+ initiators on the specified logical unit of the target. */
+if (msg == 0x0e) {
+DPRINTF(MSG: CLEAR QUEUE tag=0x%x\n, current_tag);
+}
+/* The BUS DEVICE RESET message clears all I/O processes for all
+ initiators on all logical units of the target. */
+if (msg == 0x0c) {
+DPRINTF(MSG: BUS DEVICE RESET tag=0x%x\n, current_tag);
+}
+
+/* clear the current I/O process */
+current_dev-info-cancel_io(current_dev, current_tag);
+
+/* As the current implemented devices scsi_disk and scsi_generic
+ only support one LUN, we don't need to keep track of LUNs.
+ Clearing I/O processes for other initiators could be possible
+ for scsi_generic by sending a SG_SCSI_RESET to the /dev/sgX
+ device, but this is currently not implemented (and seems not
+ to be really necessary). So let's simply clear all queued
+ commands for the current device: */
+id = current_tag 0xff00;
+QTAILQ_FOREACH_SAFE(p, s-queue, next, p_next) {
+if ((p-tag 0xff00) == id) {
+current_dev-info-cancel_io(current_dev, p-tag);
+QTAILQ_REMOVE(s-queue, p, next);
+}
+}
+
+lsi_disconnect(s);
+break;
default:
if ((msg 0x80) == 0) {
goto bad;
[Qemu-devel] [Bug 643465] Re: Crash at network boot
Whan I put vga card it work perfectly. I have only the bug with wmware card and particulare version of linux - vanilla 2.6.32.22 - 0.12.5-r1 - gentoo - core i5 750 Guest: - gentoo kernel (based on vanilla kernel 2.6.32.16) - VMware SVGA video driver 11.0.1 - network boot via pxe and squashfs + aufs + tmpfs ** Changed in: qemu Status: Incomplete = New -- Crash at network boot https://bugs.launchpad.net/bugs/643465 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: When I boot on lan, I crash qemu: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7491a710 (LWP 10614)] 0x005a1de8 in lsi_update_irq (s=0x125d5a0) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/hw/lsi53c895a.c:426 426 /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/hw/lsi53c895a.c: No such file or directory. in /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/hw/lsi53c895a.c (gdb) bt #0 0x005a1de8 in lsi_update_irq (s=0x125d5a0) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/hw/lsi53c895a.c:426 #1 0x005a4f67 in lsi_mmio_writew (opaque=0x125d5a0, addr=value optimized out, val=2) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/hw/lsi53c895a.c:1775 #2 0x004fdf3b in cpu_physical_memory_rw (addr=4043505728, buf=0x77ff2028 \002, len=2, is_write=1) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/exec.c:3215 #3 0x0042bf65 in handle_mmio (env=0xcaa6d0) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/qemu-kvm.c:831 #4 kvm_run (env=0xcaa6d0) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/qemu-kvm.c:979 #5 0x0042c249 in kvm_cpu_exec (env=0x125d5a0) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/qemu-kvm.c:1651 #6 0x0042c471 in kvm_main_loop_cpu (_env=value optimized out) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/qemu-kvm.c:1893 #7 ap_main_loop (_env=value optimized out) at /var/tmp/portage/app-emulation/qemu-kvm-0.12.5-r1/work/qemu-kvm-0.12.5/qemu-kvm.c:1943 #8 0x779c0894 in start_thread (arg=value optimized out) at pthread_create.c:297 #9 0x75ac927d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:115
Re: [Qemu-devel] [Bug 639651] Re: DRIVER_IRQL_NOT_LESS_OR_EQUAL booting WIndows XP with Synaptics driver installed
On 10/07/10 12:17, Michal Suchanek wrote: I have no idea how to log the data. I looked at the qemu man page but it does not even mention the PS/2 mouse as a chardev nor offers an option to log traffic of chardevs without attaching them to a file and thus detaching them from the emulated device. I would attack it by hacking hw/ps.c a bit to monitor the transactions in ps2_queue() and ps2_read_data(). Cheers, Jes -- DRIVER_IRQL_NOT_LESS_OR_EQUAL booting WIndows XP with Synaptics driver installed https://bugs.launchpad.net/bugs/639651 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Status in Debian GNU/Linux: New Bug description: Positng the issue here since I did not get any reply on the ML. I was trying to update some windows XP (SP3) images in kvm. It worked fine several times but last time I added mass storage drivers to sysprep and now on the second boot after reseal (the first is mini-setup) I get a BSOD with message DRIVER_IRQL_NOT_LESS_OR_EQUAL. It turns out that the error is unrelated to storage drivers. It is triggered by Synaptics driver installing for the PS2 mouse in kvm (which does not happen in VirtualBox or on real hardware). The image is originally created on hardware with MP acpi (not virtualization). qemu-kvm 0.12.5+dfsg-2
Re: [Qemu-devel] [PATCH 01/11] block: avoid a write only variable
Am 07.10.2010 11:37, schrieb Markus Armbruster: Blue Swirl blauwir...@gmail.com writes: Compiling with GCC 4.6.0 20100925 produced a warning: /src/qemu/block/qcow2-refcount.c: In function 'update_refcount': /src/qemu/block/qcow2-refcount.c:552:13: error: variable 'dummy' set but not used [-Werror=unused-but-set-variable] Fix by adding a function that does not generate a warning when the result is unused. What about a simple (void)dummy instead? I would prefer that, too. Kevin
Re: [Qemu-devel] [PATCH] ceph/rbd block driver for qemu-kvm (v4)
Am 23.09.2010 04:21, schrieb Yehuda Sadeh Weinraub: Following up on this one, I'd like to know whether there is any pending issue preventing rbd from being included upstream. Basically for me the only problem at the moment is a lack of reviews. If nobody else picks it up, I'll get to it eventually myself, but I can't promise if I get to it next week. Unfortunately this is something that you as the authors can't do much about, except maybe asking more people to give it a review. Kevin
Re: [Qemu-devel] Re: [PATCH 08/11] vnc: avoid write only variables
Paolo Bonzini pbonz...@redhat.com writes:
On 10/06/2010 11:33 PM, Blue Swirl wrote:
+#if defined(CONFIG_VNC_TLS) || defined(CONFIG_VNC_SASL)
} else if (strncmp(options, acl, 3) == 0) {
acl = 1;
+#endif
Not sure it's okay to reject the option altogether (i.e. maybe the #if
Technically a regression. Do we care?
should only include acl = 1.
Re: [Qemu-devel] Re: [PATCH 11/11] mips: avoid write only variables
Paolo Bonzini pbonz...@redhat.com writes: On 10/06/2010 11:34 PM, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced a lot of warnings like: /src/qemu/target-mips/translate.c: In function 'gen_ld': /src/qemu/target-mips/translate.c:1039:17: error: variable 'opn' set but not used [-Werror=unused-but-set-variable] Fix by making the variable declarations and uses conditional to debugging. What about a set_opn macro instead of this one-off D idiom? Even simpler: a few judiciously placed (void)opn.
Re: [Qemu-devel] Re: [PATCH 10/11] ppc: avoid write only variables
Alexander Graf ag...@suse.de writes: On 06.10.2010, at 23:34, Blue Swirl wrote: Compiling with GCC 4.6.0 20100925 produced warnings: /src/qemu/target-ppc/op_helper.c: In function 'helper_icbi': /src/qemu/target-ppc/op_helper.c:351:14: error: variable 'tmp' set but not used [-Werror=unused-but-set-variable] /src/qemu/target-ppc/op_helper.c: In function 'do_6xx_tlb': /src/qemu/target-ppc/op_helper.c:3805:28: error: variable 'EPN' set but not used [-Werror=unused-but-set-variable] /src/qemu/target-ppc/op_helper.c: In function 'do_74xx_tlb': /src/qemu/target-ppc/op_helper.c:3838:28: error: variable 'EPN' set but not used [-Werror=unused-but-set-variable] Fix by making the variable declarations and their uses also conditional to debug definition. Delete tmp. Maybe it would make more sense to get those LOG_* macros into static inline functions. But for the issue at hand, the solution looks good to me. Or simply (void)EPN.
