Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/06/2011 09:17 PM, Pekka Enberg wrote: No. I want to try new tool/old kernel and old tool/new kernel (kernel can be either guest or host, depending on the nature of the bug), and then bisect just one. (*) And that's the exceptional case, and only KVM tool developers really should have the need to do that. Exactly - having the source code in Linux kernel tree covers the exceptional case where we're unsure which part of the equation broke things (which are btw the nasties issues we've had so far). No, having the source code in Linux kernel tree is perfectly useless for the exceptional case, and forces you to go through extra hoops to build only one component. Small hoops such as adding -- tools/kvm to git bisect start perhaps, but still hoops that aren't traded for a practical advantage. You keep saying oh things have been so much better because it's so close to the kernel and it worked so great for perf, but you haven't brought any practical example that we can stare at in admiration. (BTW, I'm also convinced like Ted that not having a defined perf ABI might have made sense in the beginning, but it has now devolved into bad software engineering practice). I have no idea why you're trying to convince me that it doesn't matter. I'm not trying to convince you that it doesn't matter, I'm trying to convince you that it doesn't *make sense*. It's a hypervisor that implements virtio drivers, serial emulation, and mini-BIOS. ... all of which have a spec against which you should be working. Save perhaps for the mini-BIOS, if you develop against the kernel source rather than the spec you're doing it *wrong*. Very wrong. But you've been told this many times already. Paolo
Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?
On 11/06/2011 09:14 PM, Thomas Schmitt wrote: Hi, i have finished CD and DVD tests with drive file=/dev/sg2,if=virtio -cdrom /dvdbuffer/pseudo_drive There is substantial improvement towards if=scsi. Actually everything works like a charm now. :)) Cool. So you might have unveiled some kernel bugs too (host crashes are never desirable!) but most of them were likely in the LSI emulation. I'll try some of the testcases on vmw_pvscsi to see if some are generic to SCSI. Paolo
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 10:00 AM, Paolo Bonzini pbonz...@redhat.com wrote: No, having the source code in Linux kernel tree is perfectly useless for the exceptional case, and forces you to go through extra hoops to build only one component. Small hoops such as adding -- tools/kvm to git bisect start perhaps, but still hoops that aren't traded for a practical advantage. You keep saying oh things have been so much better because it's so close to the kernel and it worked so great for perf, but you haven't brought any practical example that we can stare at in admiration. The _practical example_ is the working software in tools/kvm! I have no idea why you're trying to convince me that it doesn't matter. I'm not trying to convince you that it doesn't matter, I'm trying to convince you that it doesn't *make sense*. It's a hypervisor that implements virtio drivers, serial emulation, and mini-BIOS. ... all of which have a spec against which you should be working. Save perhaps for the mini-BIOS, if you develop against the kernel source rather than the spec you're doing it *wrong*. Very wrong. But you've been told this many times already. I have zero interest in arguing with you about something you have no practical experience on. I've tried both out-of-tree and in-tree development for the KVM tool and I can tell you the latter is much more productive environment. We are obviously also using specifications but as you damn well should know, specifications don't matter nearly as much as working code. That's why it's important to have easy access to both. Pekka
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 10:00 AM, Paolo Bonzini pbonz...@redhat.com wrote: (BTW, I'm also convinced like Ted that not having a defined perf ABI might have made sense in the beginning, but it has now devolved into bad software engineering practice). I'm not a perf maintainer so I don't know what the situation with wrt. ABI breakage is. Your or Ted's comments don't match my assumptions or experience, though. Pekka
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/2011 09:09 AM, Pekka Enberg wrote: We are obviously also using specifications but as you damn well should know, specifications don't matter nearly as much as working code. Specifications matter much more than working code. Quirks are a fact of life but should always come second. To bring you an example from the kernel, there is a very boring list of PCI quirks and a lot of code for PCI specs, not the other way round. Paolo
Re: [Qemu-devel] [v8 Patch 0/6]Qemu: Host pagecache setting from cmdline and monitor
On 11/04/2011 07:59 AM, Zhi Yong Wu wrote: On Sun, Oct 30, 2011 at 6:33 PM, Supriya Kannery supri...@linux.vnet.ibm.com wrote: Currently cache setting of a block device cannot be changed without restarting a running VM. Following patchset is for enabling dynamic change of cache setting for block devices through qemu monitor. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. This patchset introduces a. monitor command 'block_set_hostcache' using which host pagecache setting for a block device can be changed dynamically. I got a bit confusion. Is it used to change host pagecache setting on hyperviser or on guest? This block device said by you is for guest, right? This command is used for changing pagecache setting of the image files in host which are acting as block devices for guest. -thanks, Supriya
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/2011 09:09 AM, Pekka Enberg wrote: We are obviously also using specifications but as you damn well should know, specifications don't matter nearly as much as working code. On Mon, 7 Nov 2011, Paolo Bonzini wrote: Specifications matter much more than working code. Quirks are a fact of life but should always come second. To quote Linus: And I have seen _lots_ of total crap work that was based on specs. It's _the_ single worst way to write software, because it by definition means that the software was written to match theory, not reality. [ http://kerneltrap.org/node/5725 ] So no, I don't agree with you at all. Pekka
Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?
On Thu, Nov 3, 2011 at 2:05 AM, Thomas Schmitt scdbac...@gmx.net wrote:
Hi,
i wrote:
The sense code is listed in MMC as:
B 00 06 I/O PROCESS TERMINATED
Paolo Bonzini wrote:
Is it good or bad? :) I see it even in the very first command.
It does not indicate success. MMC only has the meager info above.
SPC-3 says in table 27 about sense key B:
Bh : ABORTED COMMAND: Indicates that the device server aborted
the command. The application client may be able to recover by
trying the command again.
It does not come from the real drive, i am very sure.
So unless you find a particular reason in qemu, i doubt that repetition
will help.
What do these give on the host? Sounds like another LSI emulation bug.
All is well with the drive on the host.
This is my test machine, which checks build and function of xorriso
with GNU/Linux, FreeBSD 8, and OpenSolaris. It also tested the
freshly released libcdio-0.83 by Rocky Bernstein which uses
on Linux ioctl(CDROM_SEND_PACKET) rather than ioctl(SG_IO).
When i had trouble with vanishing udev links on Debian 6.0.2,
i also tested with wodim. It worked as good as can be expected.
The current release 1.1.6 will not recognize QEMU DVD-ROM because
of its inconsistent Mode Page 2Ah with short Page Length (18 decimal).
The page length is indeed 18 for IDE and 20 for SCSI. I made some changes
to that page recently, but left the 18 because I feared causing regression.
But if that is a bug, we can probably fix it in 1.0.
This riddles me.
In hw/scsi-disk.c:mode_sense_page() i see that the page 0x2A
(now MODE_PAGE_CAPABILITIES) gets filled with 22 bytes, compliant
to MMC-1. (Later MMCs have longer minimum sizes.)
But libburn receives only 20 of them, because the Page Length
is reported as 0x12 in the 10th byte of the reply:
MODE SENSE
5a 00 2a 00 00 00 00 00 1c 00
From drive: 28b
00 22 70 00 00 00 00 00 2a 12 00 00 71 60 29 00 02 c2 00 02
02 00 02 c2 00 00 00 00
Nevertheless i see in hw/scsi-disk.c
p[1] = 0x14;
So how is this altered to 0x12 in the further course of processing ?
Further, the Mode Data Length is 0x22, announcing 34 + 2 = 36 overall
bytes in the reply, which matches neither Page Length 0x12 nor 0x14,
but would match Page Length 26 decimal = 0x1a.
A real DVD-ROM drive ('HL-DT-ST' 'DVD-ROM GDR8162B') rather tells
MODE SENSE
5a 00 2a 00 00 00 00 00 1c 00
From drive: 28b
00 20 41 00 00 00 00 00 2a 18 3f 00 71 73 2b 23 21 13 01 00
01 00 0d c8 00 00 00 00
so that libburn repeats the transaction with adjusted Allocation Length
MODE SENSE
5a 00 2a 00 00 00 00 00 22 00
From drive: 34b
00 20 41 00 00 00 00 00 2a 18 3f 00 71 73 2b 23 21 13 01 00
01 00 0d c8 00 00 00 00 00 00 00 01 00 00
(Some Linux transports react badly if the Allocation Length surpasses
the actually available amount of bytes. So i adopted this two-step
strategy, which i learned from growisofs by Andy Polyakov.)
Is it a known bug that 'QEMU DVD-ROM' is always empty if it stems
from option -drive if=scsi ?
No, it is not a bug. Remember this is an IDE DVD-ROM. It is not
instantiated by -drive if=scsi: it is the same device that -cdrom crsates,
You are the expert. So i do not contradict.
But it behaves differently. The devices created by -cdrom do have
a medium loaded. The 'QEMU DVD-ROM' from -drive if=scsi have no medium.
(But the other drive 'CDDVDW SH-S223B' from -drive file=/dev/sg1,if=scsi
has a medium and replies the same as on the host system.)
I just checked with the git clone.
Readable are
-cdrom /dev/sr0
-cdrom /dvdbuffer/pseudo_drive (a regular file)
with dd and with xorriso (via guest SG_IO).
No medium is found in
-drive file=/dev/sr0,if=scsi,media=cdrom
-drive file=/dvdbuffer/pseudo_drive,if=scsi,media=cdrom
which report like the /dev/sg1 based 'QEMU DVD-ROM':
Yeah, i also met this same issue. The summary is very correct for me.
BTW: if the transfer mode is changed from DMA to PIO, it will fail.
/dev/sr0:
multcount = 0 (off)
IO_support= 1 (32-bit)
readonly = 0 (off)
readahead = 256 (on)
HDIO_GETGEO failed: Inappropriate ioctl for device
Model=QEMU DVD-ROM, FwRev=0.15.90, SerialNo=QM3
Config={ Fixed Removeable DTR=5Mbs DTR10Mbs nonMagnetic }
RawCHS=0/0/0, TrkSize=0, SectSize=0, ECCbytes=4
BuffType=DualPortCache, BuffSize=256kB, MaxMultSect=0
CurCHS=0/0/0, CurSects=0, LBA=yes, LBAsects=0
IORDY=no, tPIO={min:300,w/IORDY:180}, tDMA={min:180,rec:180}
PIO modes: pio0 pio3 pio4
DMA modes: sdma0 sdma1 sdma2 mdma0 mdma1 *mdma2
UDMA modes: udma0 udma1 udma2 udma3 udma4 udma5
AdvancedPM=no
Drive conforms to: Unspecified: ATA/ATAPI-1,2,3,4
* signifies the current active mode
If its transfer mode is changed:
/dev/sr0:
setting xfermode to 8 (PIO flow control mode0)
HDIO_DRIVE_CMD(setxfermode) failed: Invalid exchange
No current profile (00 00 in bytes 6 and 7 of reply):
GET CONFIGURATION
46 00 00 00 00 00 00 00 14 00
From drive:
Re: [Qemu-devel] [v8 Patch 0/6]Qemu: Host pagecache setting from cmdline and monitor
On Mon, Nov 7, 2011 at 4:38 PM, Supriya Kannery supri...@linux.vnet.ibm.com wrote: On 11/04/2011 07:59 AM, Zhi Yong Wu wrote: On Sun, Oct 30, 2011 at 6:33 PM, Supriya Kannery supri...@linux.vnet.ibm.com wrote: Currently cache setting of a block device cannot be changed without restarting a running VM. Following patchset is for enabling dynamic change of cache setting for block devices through qemu monitor. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. This patchset introduces a. monitor command 'block_set_hostcache' using which host pagecache setting for a block device can be changed dynamically. I got a bit confusion. Is it used to change host pagecache setting on hyperviser or on guest? This block device said by you is for guest, right? This command is used for changing pagecache setting of the image files in host which are acting as block devices for guest. So what is the difference between it and cache option? -thanks, Supriya -- Regards, Zhi Yong Wu
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/2011 09:45 AM, Pekka Enberg wrote: Specifications matter much more than working code. Quirks are a fact of life but should always come second. To quote Linus: And I have seen _lots_ of total crap work that was based on specs. It's _the_ single worst way to write software, because it by definition means that the software was written to match theory, not reality. All generalizations are false. Paolo
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/2011 09:45 AM, Pekka Enberg wrote: Specifications matter much more than working code. Quirks are a fact of life but should always come second. To quote Linus: And I have seen _lots_ of total crap work that was based on specs. It's _the_ single worst way to write software, because it by definition means that the software was written to match theory, not reality. On Mon, Nov 7, 2011 at 10:52 AM, Paolo Bonzini pbonz...@redhat.com wrote: All generalizations are false. What is that supposed to mean? You claimed we're doing it wrong and I explained you why we are doing it the way we are. Really, the way we do things in the KVM tool is not a bug, it's a feature. Pekka
Re: [Qemu-devel] [PATCH v2 4/7] block: take lock around bdrv_read implementations
Am 06.11.2011 15:27, schrieb Avi Kivity: On 10/20/2011 01:16 PM, Paolo Bonzini wrote: This does the first part of the conversion to coroutines, by wrapping bdrv_read implementations to take the read side of the rwlock. Drivers that implement bdrv_read rather than bdrv_co_readv can then benefit from asynchronous operation (at least if the underlying protocol supports it, which is not the case for raw-win32), even though they still operate with a bounce buffer. raw-win32 does not need the lock, because it cannot yield. nbd also doesn't probably, but better be safe. This patch (2914caa088e3fbbd) breaks autotest when a guest reboots after install; instead of rebooting, the guest is stuck in the bootloader or kernel. This was discovered in qemu-kvm, but applies to plain qemu too. The commit above is broken, it's parent is good. Does the autotest case use any of the block drivers that are changed by this patch? I would be surprised to learn that, but otherwise it doesn't make sense to me. block/bochs.c | 13 - block/cloop.c | 13 - block/cow.c | 13 - block/dmg.c | 13 - block/nbd.c | 13 - block/parallels.c | 13 - block/vmdk.c | 13 - block/vpc.c | 13 - block/vvfat.c | 13 - Kevin
Re: [Qemu-devel] backing_file Path
Am 07.11.2011 08:46, schrieb Zhi Hui Li: when I trace the code of qemu-img.c， I found in the image file, the backing_file use the relative path, I think Maybe it has some problems. for example: 1: qemu-img create -o backing_file=../ aa.img 5G 2: qemu-system-x86_64 aa.img if you change aa.img's the path, the exec is wrong,it can't find the backing_file path, because the backing_file use the relative path, Could we change the relative path to the absolute path? No, we can't. That would break compatibility of qemu-img create with older versions. Relative paths have well-defined semantics, they are relative to the image file. Kevin
Re: [Qemu-devel] [PATCH] readline: Fix buffer overrun on re-add to history
On Fri, Nov 04, 2011 at 11:10:01AM +0100, Markus Armbruster wrote: readline_hist_add() moves the history entry to the end of history. It uses memmove() to move rs-history[idx + 1..] to rs-history[idx..]. However, its size argument is off by two array elements, so it writes one element beyond rs-history[], and reads two. On my system, this clobbers rs-hist_entry and the hole right after it. Since the function assigns to rs-hist_entry in time, the bug has no ill effects for me. Spotted by Coverity. Signed-off-by: Markus Armbruster arm...@redhat.com --- readline.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Thanks, applied to the trivial patches tree: http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] xen-platform: Fix IO port read/write functions
On Fri, Nov 04, 2011 at 03:35:11PM +, Anthony PERARD wrote: Somehow, the read/write functions handle an offset that does not exist anymore. Signed-off-by: Anthony PERARD anthony.per...@citrix.com --- hw/xen_platform.c | 18 +- 1 files changed, 9 insertions(+), 9 deletions(-) Thanks, applied to the trivial patches tree: http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches Stefan
Re: [Qemu-devel] [PATCH v2] block: Use bdrv functions to replace file operation in cow.c
On Mon, Nov 07, 2011 at 02:52:17PM +0800, Li Zhi Hui wrote:
@@ -260,10 +261,15 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options)
options++;
}
-cow_fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
- 0644);
-if (cow_fd 0)
-return -errno;
+ret = bdrv_create_file(filename, options);
+if (ret 0) {
+return ret;
+}
+
+ret = bdrv_file_open(cow_bs, filename, BDRV_O_RDWR);
+if (ret 0) {
+return ret;
+}
memset(cow_header, 0, sizeof(cow_header));
cow_header.magic = cpu_to_be32(COW_MAGIC);
cow_header.version = cpu_to_be32(COW_VERSION);
@@ -271,16 +277,16 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options)
/* Note: if no file, we put a dummy mtime */
cow_header.mtime = cpu_to_be32(0);
-fd = open(image_filename, O_RDONLY | O_BINARY);
-if (fd 0) {
-close(cow_fd);
+ret = bdrv_file_open(image_bs, image_filename, BDRV_O_RDWR);
+if (ret 0) {
+bdrv_close(cow_bs);
This should be bdrv_delete() instead of bdrv_close() because
bdrv_file_open() does bdrv_new() to allocate the BlockDriverState - it
needs to be freed.
The same applies to the other hunks in this patch.
Stefan
Re: [Qemu-devel] Wierd hack to sound/pci/intel8x0.c
Hi, Agreed. If we can know the virtual device for KVM in a better way (e.g. any specific PCI SSID or such), we can narrow the condition more safely. PCI Subsystem ID 1af4:1100 is qemu/kvm (not only Intel HDA, most other emulated pci devices have it too). Complete entry: 00:04.0 0403: 8086:2668 (rev 01) Subsystem: 1af4:1100 Physical Slot: 4 Flags: bus master, fast devsel, latency 0, IRQ 11 Memory at e203 (32-bit, non-prefetchable) [size=16K] Kernel driver in use: HDA Intel Kernel modules: snd-hda-intel cheers, Gerd
[Qemu-devel] [PULL 0/5] Trivial patches for 2 to 7 November 2011
Anthony: These patches fix bugs and are small. I think we should consider including them in 1.0. The following changes since commit 932eacc158c064935c7bab920c88a93a629e1ca4: Merge branch 'xtensa' of git://jcmvbkbc.spb.ru/dumb/qemu-xtensa (2011-11-02 20:52:23 +) are available in the git repository at: ssh://repo.or.cz/srv/git/qemu/stefanha.git trivial-patches Anthony PERARD (1): xen-platform: Fix IO port read/write functions Markus Armbruster (1): readline: Fix buffer overrun on re-add to history Pavel Borzenkov (3): cmd: Fix coding style in cmd.c cmd: Fix potential NULL pointer dereference cmd: Fix potential memory leak cmd.c | 168 ++--- hw/xen_platform.c | 18 +++--- readline.c|2 +- 3 files changed, 92 insertions(+), 96 deletions(-) -- 1.7.7.1
[Qemu-devel] [PATCH 3/5] cmd: Fix potential memory leak
From: Pavel Borzenkov pavel.borzen...@gmail.com
Signed-off-by: Pavel Borzenkov pavel.borzen...@gmail.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
cmd.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/cmd.c b/cmd.c
index 75415d8..0806e18 100644
--- a/cmd.c
+++ b/cmd.c
@@ -329,16 +329,21 @@ char **breakline(char *input, int *count)
int c = 0;
char *p;
char **rval = calloc(sizeof(char *), 1);
+char **tmp;
while (rval (p = qemu_strsep(input, )) != NULL) {
if (!*p) {
continue;
}
c++;
-rval = realloc(rval, sizeof(*rval) * (c + 1));
-if (!rval) {
+tmp = realloc(rval, sizeof(*rval) * (c + 1));
+if (!tmp) {
+free(rval);
+rval = NULL;
c = 0;
break;
+} else {
+rval = tmp;
}
rval[c - 1] = p;
rval[c] = NULL;
--
1.7.7.1
[Qemu-devel] [PATCH 4/5] readline: Fix buffer overrun on re-add to history
From: Markus Armbruster arm...@redhat.com
readline_hist_add() moves the history entry to the end of history. It
uses memmove() to move rs-history[idx + 1..] to rs-history[idx..].
However, its size argument is off by two array elements, so it writes
one element beyond rs-history[], and reads two.
On my system, this clobbers rs-hist_entry and the hole right after
it. Since the function assigns to rs-hist_entry in time, the bug has
no ill effects for me.
Spotted by Coverity.
Signed-off-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
readline.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/readline.c b/readline.c
index 6a3160a..a6c0039 100644
--- a/readline.c
+++ b/readline.c
@@ -236,7 +236,7 @@ static void readline_hist_add(ReadLineState *rs, const char
*cmdline)
new_entry = hist_entry;
/* Put this entry at the end of history */
memmove(rs-history[idx], rs-history[idx + 1],
- (READLINE_MAX_CMDS - idx + 1) * sizeof(char *));
+ (READLINE_MAX_CMDS - (idx + 1)) * sizeof(char *));
rs-history[READLINE_MAX_CMDS - 1] = NULL;
for (; idx READLINE_MAX_CMDS; idx++) {
if (rs-history[idx] == NULL)
--
1.7.7.1
[Qemu-devel] [PATCH 2/5] cmd: Fix potential NULL pointer dereference
From: Pavel Borzenkov pavel.borzen...@gmail.com
Signed-off-by: Pavel Borzenkov pavel.borzen...@gmail.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
cmd.c | 11 +++
1 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/cmd.c b/cmd.c
index a6e3ef4..75415d8 100644
--- a/cmd.c
+++ b/cmd.c
@@ -47,7 +47,7 @@ compare(const void *a, const void *b)
void add_command(const cmdinfo_t *ci)
{
-cmdtab = realloc((void *)cmdtab, ++ncmds * sizeof(*cmdtab));
+cmdtab = g_realloc((void *)cmdtab, ++ncmds * sizeof(*cmdtab));
cmdtab[ncmds - 1] = *ci;
qsort(cmdtab, ncmds, sizeof(*cmdtab), compare);
}
@@ -122,12 +122,7 @@ find_command(
void add_user_command(char *optarg)
{
-ncmdline++;
-cmdline = realloc(cmdline, ncmdline * sizeof(char *));
-if (!cmdline) {
-perror(realloc);
-exit(1);
-}
+cmdline = g_realloc(cmdline, ++ncmdline * sizeof(char *));
cmdline[ncmdline-1] = optarg;
}
@@ -190,7 +185,7 @@ void command_loop(void)
doneline(input, v);
}
if (cmdline) {
-free(cmdline);
+g_free(cmdline);
return;
}
--
1.7.7.1
[Qemu-devel] [PATCH 1/5] cmd: Fix coding style in cmd.c
From: Pavel Borzenkov pavel.borzen...@gmail.com
Before the next patches, fix coding style of the affected functions.
Signed-off-by: Pavel Borzenkov pavel.borzen...@gmail.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
cmd.c | 168 -
1 files changed, 82 insertions(+), 86 deletions(-)
diff --git a/cmd.c b/cmd.c
index f77897e..a6e3ef4 100644
--- a/cmd.c
+++ b/cmd.c
@@ -45,13 +45,11 @@ compare(const void *a, const void *b)
((const cmdinfo_t *)b)-name);
}
-void
-add_command(
- const cmdinfo_t *ci)
+void add_command(const cmdinfo_t *ci)
{
- cmdtab = realloc((void *)cmdtab, ++ncmds * sizeof(*cmdtab));
- cmdtab[ncmds - 1] = *ci;
- qsort(cmdtab, ncmds, sizeof(*cmdtab), compare);
+cmdtab = realloc((void *)cmdtab, ++ncmds * sizeof(*cmdtab));
+cmdtab[ncmds - 1] = *ci;
+qsort(cmdtab, ncmds, sizeof(*cmdtab), compare);
}
static int
@@ -122,16 +120,15 @@ find_command(
return NULL;
}
-void
-add_user_command(char *optarg)
+void add_user_command(char *optarg)
{
- ncmdline++;
- cmdline = realloc(cmdline, sizeof(char*) * (ncmdline));
- if (!cmdline) {
- perror(realloc);
- exit(1);
- }
- cmdline[ncmdline-1] = optarg;
+ncmdline++;
+cmdline = realloc(cmdline, ncmdline * sizeof(char *));
+if (!cmdline) {
+perror(realloc);
+exit(1);
+}
+cmdline[ncmdline-1] = optarg;
}
static int
@@ -160,45 +157,44 @@ static void prep_fetchline(void *opaque)
static char *get_prompt(void);
-void
-command_loop(void)
+void command_loop(void)
{
- int c, i, j = 0, done = 0, fetchable = 0, prompted = 0;
- char*input;
- char**v;
- const cmdinfo_t *ct;
-
- for (i = 0; !done i ncmdline; i++) {
- input = strdup(cmdline[i]);
- if (!input) {
- fprintf(stderr,
- _(cannot strdup command '%s': %s\n),
- cmdline[i], strerror(errno));
- exit(1);
- }
- v = breakline(input, c);
- if (c) {
- ct = find_command(v[0]);
- if (ct) {
- if (ct-flags CMD_FLAG_GLOBAL)
- done = command(ct, c, v);
- else {
- j = 0;
- while (!done (j = args_command(j)))
- done = command(ct, c, v);
- }
- } else
- fprintf(stderr, _(command \%s\ not found\n),
- v[0]);
- }
- doneline(input, v);
- }
- if (cmdline) {
- free(cmdline);
- return;
+int c, i, j = 0, done = 0, fetchable = 0, prompted = 0;
+char *input;
+char **v;
+const cmdinfo_t *ct;
+
+for (i = 0; !done i ncmdline; i++) {
+input = strdup(cmdline[i]);
+if (!input) {
+fprintf(stderr, _(cannot strdup command '%s': %s\n),
+cmdline[i], strerror(errno));
+exit(1);
+}
+v = breakline(input, c);
+if (c) {
+ct = find_command(v[0]);
+if (ct) {
+if (ct-flags CMD_FLAG_GLOBAL) {
+done = command(ct, c, v);
+} else {
+j = 0;
+while (!done (j = args_command(j))) {
+done = command(ct, c, v);
+}
+}
+} else {
+fprintf(stderr, _(command \%s\ not found\n), v[0]);
+}
}
+doneline(input, v);
+}
+if (cmdline) {
+free(cmdline);
+return;
+}
- while (!done) {
+while (!done) {
if (!prompted) {
printf(%s, get_prompt());
fflush(stdout);
@@ -212,22 +208,24 @@ command_loop(void)
if (!fetchable) {
continue;
}
- if ((input = fetchline()) == NULL)
- break;
- v = breakline(input, c);
- if (c) {
- ct = find_command(v[0]);
- if (ct)
- done = command(ct, c, v);
- else
- fprintf(stderr, _(command \%s\ not found\n),
- v[0]);
- }
- doneline(input, v);
+input = fetchline();
+if (input == NULL) {
+break;
+}
+v = breakline(input, c);
+if (c) {
+ct = find_command(v[0]);
+if
[Qemu-devel] [PATCH 5/5] xen-platform: Fix IO port read/write functions
From: Anthony PERARD anthony.per...@citrix.com
Somehow, the read/write functions handle an offset that does not exist anymore.
Signed-off-by: Anthony PERARD anthony.per...@citrix.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
hw/xen_platform.c | 18 +-
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/xen_platform.c b/hw/xen_platform.c
index 6e3ba8b..5e792f5 100644
--- a/hw/xen_platform.c
+++ b/hw/xen_platform.c
@@ -113,7 +113,7 @@ static void platform_fixed_ioport_writew(void *opaque,
uint32_t addr, uint32_t v
{
PCIXenPlatformState *s = opaque;
-switch (addr - XEN_PLATFORM_IOPORT) {
+switch (addr) {
case 0:
/* Unplug devices. Value is a bitmask of which devices to
unplug, with bit 0 the IDE devices, bit 1 the network
@@ -152,7 +152,7 @@ static void platform_fixed_ioport_writew(void *opaque,
uint32_t addr, uint32_t v
static void platform_fixed_ioport_writel(void *opaque, uint32_t addr,
uint32_t val)
{
-switch (addr - XEN_PLATFORM_IOPORT) {
+switch (addr) {
case 0:
/* PV driver version */
break;
@@ -163,7 +163,7 @@ static void platform_fixed_ioport_writeb(void *opaque,
uint32_t addr, uint32_t v
{
PCIXenPlatformState *s = opaque;
-switch (addr - XEN_PLATFORM_IOPORT) {
+switch (addr) {
case 0: /* Platform flags */ {
hvmmem_type_t mem_type = (val PFFLAG_ROM_LOCK) ?
HVMMEM_ram_ro : HVMMEM_ram_rw;
@@ -186,7 +186,7 @@ static uint32_t platform_fixed_ioport_readw(void *opaque,
uint32_t addr)
{
PCIXenPlatformState *s = opaque;
-switch (addr - XEN_PLATFORM_IOPORT) {
+switch (addr) {
case 0:
if (s-drivers_blacklisted) {
/* The drivers will recognise this magic number and refuse
@@ -205,7 +205,7 @@ static uint32_t platform_fixed_ioport_readb(void *opaque,
uint32_t addr)
{
PCIXenPlatformState *s = opaque;
-switch (addr - XEN_PLATFORM_IOPORT) {
+switch (addr) {
case 0:
/* Platform flags */
return s-flags;
@@ -221,7 +221,7 @@ static void platform_fixed_ioport_reset(void *opaque)
{
PCIXenPlatformState *s = opaque;
-platform_fixed_ioport_writeb(s, XEN_PLATFORM_IOPORT, 0);
+platform_fixed_ioport_writeb(s, 0, 0);
}
const MemoryRegionPortio xen_platform_ioport[] = {
@@ -251,7 +251,7 @@ static void platform_fixed_ioport_init(PCIXenPlatformState*
s)
static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr)
{
if (addr == 0) {
-return platform_fixed_ioport_readb(opaque, XEN_PLATFORM_IOPORT);
+return platform_fixed_ioport_readb(opaque, 0);
} else {
return ~0u;
}
@@ -263,7 +263,7 @@ static void xen_platform_ioport_writeb(void *opaque,
uint32_t addr, uint32_t val
switch (addr) {
case 0: /* Platform flags */
-platform_fixed_ioport_writeb(opaque, XEN_PLATFORM_IOPORT, val);
+platform_fixed_ioport_writeb(opaque, 0, val);
break;
case 8:
log_writeb(s, val);
@@ -321,7 +321,7 @@ static int xen_platform_post_load(void *opaque, int
version_id)
{
PCIXenPlatformState *s = opaque;
-platform_fixed_ioport_writeb(s, XEN_PLATFORM_IOPORT, s-flags);
+platform_fixed_ioport_writeb(s, 0, s-flags);
return 0;
}
--
1.7.7.1
Re: [Qemu-devel] qemu-kvm crashes doing migration with disks + blkdebug files (does not happen with qemu)
Am 05.11.2011 03:16, schrieb Lucas Meneghel Rodrigues: Hi folks, qemu-kvm is segfaulting when executing migration with blkdebug files. 19:50:02 DEBUG| Git repo qemu_kvm uri: git://github.com/avikivity/qemu.git 19:50:02 DEBUG| Git repo qemu_kvm branch: master 19:50:30 INFO | Commit hash for qemu_kvm is 7879db7e9c09b92d9af1c143fbe2cc212ec89e4b (no tag found) How to reproduce: 1) create a origin vm like: /usr/local/autotest/tests/kvm/qemu -name 'vm1' -nodefaults -vga std -monitor unix:'/tmp/monitor-humanmonitor1-2004-200902-95j0',server,nowait -qmp unix:'/tmp/monitor-qmpmonitor1-2004-200902-95j0',server,nowait -serial unix:'/tmp/serial-2004-200902-95j0',server,nowait -drive file=blkdebug:/usr/local/autotest/virt/blkdebug/default.conf:/tmp/kvm_autotest_root/images/rhel6.1-64.qcow2,index=0,if=virtio,cache=none,rerror=stop,werror=stop -device virtio-net-pci,netdev=idtzhBVb,mac='9a:d0:7b:07:18:72',id='id9JW3ZV' -netdev tap,id=idtzhBVb,fd=23 -m 2048 -smp 2 -vnc :0 2) create a destination vm like: /usr/local/autotest/tests/kvm/qemu -name 'vm1' -nodefaults -vga std -monitor unix:'/tmp/monitor-humanmonitor1-2004-201329-Ia9o',server,nowait -qmp unix:'/tmp/monitor-qmpmonitor1-2004-201329-Ia9o',server,nowait -serial unix:'/tmp/serial-2004-201329-Ia9o',server,nowait -drive file=blkdebug:/usr/local/autotest/virt/blkdebug/default.conf:/tmp/kvm_autotest_root/images/rhel6.1-64.qcow2,index=0,if=virtio,cache=none,rerror=stop,werror=stop -device virtio-net-pci,netdev=idup1xAf,mac='9a:d0:7b:07:18:72',id='idyvOQf3' -netdev tap,id=idup1xAf,fd=19 -m 2048 -smp 2 -vnc :1 -S -incoming exec:nc -l 5200 Note that blkdebug file contains: [inject-error] state = 2 event = read_aio errno = 7 immediately = off once = on [set-state] state = 1 event = read_aio new_state = 2 [set-state] state = 2 event = read_aio new_state = 3 Start the migration (on this example, using exec, but it reproduces with tcp and unix sockets): 11/04 20:13:30 DEBUG|kvm_monito:0254| (monitor humanmonitor1) Sending command 'migrate -d exec:nc localhost 5200' Then you will have: 11/04 20:13:33 INFO | aexpect:0783| [qemu output] invalid runstate transition Invalid runstate transition is something for Luiz (CCed). Though probably he doesn't need to do anything in this case: I think we're not allowing the transition from I/O error to migrating. This might be fixed by 8a9236f1 in qemu.git, so please retest with upstream. Kevin
Re: [Qemu-devel] [v8 Patch 0/6]Qemu: Host pagecache setting from cmdline and monitor
On 11/07/2011 02:19 PM, Zhi Yong Wu wrote: On Mon, Nov 7, 2011 at 4:38 PM, Supriya Kannery supri...@linux.vnet.ibm.com wrote: On 11/04/2011 07:59 AM, Zhi Yong Wu wrote: On Sun, Oct 30, 2011 at 6:33 PM, Supriya Kannery supri...@linux.vnet.ibm.comwrote: Currently cache setting of a block device cannot be changed without restarting a running VM. Following patchset is for enabling dynamic change of cache setting for block devices through qemu monitor. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. This patchset introduces a. monitor command 'block_set_hostcache' using which host pagecache setting for a block device can be changed dynamically. I got a bit confusion. Is it used to change host pagecache setting on hyperviser or on guest? This block device said by you is for guest, right? This command is used for changing pagecache setting of the image files in host which are acting as block devices for guest. So what is the difference between it and cache option? Cache=xx sets a combination of status flags for image files among which host pagecache is just one. Intention here is to gradually replace cache=xx with more explicit settings like hostcache, flush, WCE.
Re: [Qemu-devel] Wierd hack to sound/pci/intel8x0.c
On 07.11.11 12:25, Gerd Hoffmann wrote: Hi, Agreed. If we can know the virtual device for KVM in a better way (e.g. any specific PCI SSID or such), we can narrow the condition more safely. PCI Subsystem ID 1af4:1100 is qemu/kvm (not only Intel HDA, most other emulated pci devices have it too). Complete entry: 00:04.0 0403: 8086:2668 (rev 01) Subsystem: 1af4:1100 Physical Slot: 4 Flags: bus master, fast devsel, latency 0, IRQ 11 Memory at e203 (32-bit, non-prefetchable) [size=16K] Kernel driver in use: HDA Intel Kernel modules: snd-hda-intel cheers, Gerd We discuss Intel ICH/AC'97 (snd-intel8x0) here, but I hope that PCI SSID is same. From Parallels VM side we also have specific PCI SSID (1ab8:0400): 00:1f.4 Multimedia audio controller: Intel Corporation 82801BA/BAM AC'97 Audio Controller (rev 02) Subsystem: Device 1ab8:0400 Flags: bus master, medium devsel, latency 0, IRQ 17 I/O ports at ee00 [size=256] I/O ports at f000 [size=256] Kernel driver in use: Intel ICH Kernel modules: snd-intel8x0 Now we can make detection code more adequate. I will make patch today.
Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?
Hi, drive file=/dev/sg2,if=virtio -cdrom /dvdbuffer/pseudo_drive Actually everything works like a charm now. :)) Cool. So you might have unveiled some kernel bugs too (host crashes are never desirable!) The USB drive is back at the test machine. I'm running the planned BD-RE tests on the host system to ensure that they work properly without qemu inbetween. Next i will try with if=virtio. Some questions: --- In a thread on linux-hotplug, /dev/sg* is declared to be deprecated in favor of /dev/bsg/* and /dev/sr*. Will this become a problem ? --- I did not succeed with googling for a way to get a block device running on top of file=/dev/sg*,if=virtio. Is this possible at all ? If so, can you give me instructions ? (It would be the cream cap on the cake.) --- The word passthrough does not show up in the context of drive emulation in any documentation inside the git clone. I only see statements about: passthrough security model. The word virtio is mentioned more often, but without much explanation of -drive use cases and pitfalls. (Only docs/qdev-device-use.txt has some more detailed information.) Is there an external documentation emerging ? I would like to read it and contribute my experiences. --- Have a nice day :) Thomas
Re: [Qemu-devel] [PATCH 1/2] net: truncate output file when using dump backend
On Sun, Nov 06, 2011 at 10:52:04PM +0100, Hervé Poussineau wrote: Signed-off-by: Hervé Poussineau hpous...@reactos.org --- net/dump.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
Hi, Usable - I've tried kvm-tool several times and still (today) fail to get a standard SUSE image (with a kernel I have to compile and provide separately...) up and running *). Likely a user mistake, but none that is very obvious. At least to me. Same here. No support for booting from CDROM. No support for booting from Network. Thus no way to install a new guest image. Booting an existing qcow2 guest image failed, the guest started throwing I/O errors. And even to try that I had to manually extract the kernel and initrd images from the guest. Maybe you should check with the Xen guys, they have a funky 'pygrub' which sort-of automates the copy-kernel-from-guest-image process. Booting the host kernel failed too. Standard distro kernel. The virtio bits are modular, not statically compiled into the kernel. kvm tool can't handle that. You have to build your own kernel and make sure you flip the correct config bits, then you can boot it to a shell prompt. Trying anything else just doesn't work today ... cheers, Gerd
Re: [Qemu-devel] GSoC mentor summit QEMU users session
On 04/11/2011 19:45, Lluís Vilanova wrote: Stefan Hajnoczi writes: On Thu, Nov 03, 2011 at 10:35:28AM +0100, Fabien Chouteau wrote: On 03/11/2011 08:44, Stefan Hajnoczi wrote: On Wed, Nov 2, 2011 at 5:39 PM, Fabien Chouteau chout...@adacore.com wrote: On 29/10/2011 15:52, Alexander Graf wrote: I took a quick peak at the qemu-trace.[ch] from couverture and it looks along the lines of the instrumentation that others have been doing too. I hope you have time to propose the coverage instrumentation for upstream QEMU. I don't know much about other instrumentations in Qemu (pointers are welcome :), but what we have in couverture-qemu is not trivial, especially when it comes to MC/DC analysis. You should take a look at 201005-erts2.pdf if you want technical details. My impression was that the QEMU portion of instrumentation was fairly simple - it writes out trace records at various interesting points during guest execution in TCG. I think fancy analysis scripts do not have to be part of QEMU but they could be added to scripts/ or put in a new contrib/ directory. I've only had a brief look into the changes, but I think the mechanism I implemented has a cleaner fit into QEMU, thanks to previous feedback from this list. I don't know about your implementation, can you give more information? What type of analysis is supported (object, statement, decision, MC/DC)? Which language? And maybe you can post a link to your repository. It explicitly separates the tracing mechanism (in QEMU itself) from the specific trace analysis (which resides in a separate library specified by the user at compile time, where most of couverture would go). As I understand everything is compiled within Qemu, right? GNATcoverage seems even more separate. We use Qemu to generate an execution trace file and the coverage analysis tool is a totally separate program. You can add support for another language or implement your own coverage tool without recompiling (redistribute) Qemu. I think that generate a trace file that can be analyzed by any tool is a better approach for coverage analysis. On the other hand, I have a complementary set of events, so we can definitely join the efforts on that side (e.g., I haven't yet went into the trouble of adding the begin/end TB or branch events). I don't know what do you mean by events, but we sure can join efforts on coverage with Qemu. Regards, -- Fabien Chouteau
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 12:11 PM, Gerd Hoffmann kra...@redhat.com wrote: No support for booting from CDROM. No support for booting from Network. Thus no way to install a new guest image. Sure. It's a pain point which we need to fix. On Mon, Nov 7, 2011 at 12:11 PM, Gerd Hoffmann kra...@redhat.com wrote: Booting an existing qcow2 guest image failed, the guest started throwing I/O errors. And even to try that I had to manually extract the kernel and initrd images from the guest. Maybe you should check with the Xen guys, they have a funky 'pygrub' which sort-of automates the copy-kernel-from-guest-image process. QCOW2 support is experimental. The I/O errors are caused by forced read-only mode. On Mon, Nov 7, 2011 at 12:11 PM, Gerd Hoffmann kra...@redhat.com wrote: Booting the host kernel failed too. Standard distro kernel. The virtio bits are modular, not statically compiled into the kernel. kvm tool can't handle that. I think we have some support for booting modular distro kernels too if you tell KVM tool where to find initrd. It sucks out-of-the-box though because nobody seems to be using it. On Mon, Nov 7, 2011 at 12:11 PM, Gerd Hoffmann kra...@redhat.com wrote: You have to build your own kernel and make sure you flip the correct config bits, then you can boot it to a shell prompt. Trying anything else just doesn't work today ... What can I say? Patches welcome? :-) Pekka
Re: [Qemu-devel] [qemu-kvm unittest regression] Re: Autotest | Job ID: 2011 Upstream qemu-kvm.git sanity 11-01-2011 00:04:02 | Status: 1 Completed | Success Rate: 94.74 %
On Tue, Nov 01, 2011 at 02:08:54PM -0200, Lucas Meneghel Rodrigues wrote: On 11/01/2011 12:17 PM, kvm-autotest wrote: Job ID: 2011 Job name: Upstream qemu-kvm.git sanity 11-01-2011 00:04:02 Summary: Host: Status: Completed Status: 1 Completed Execution time (HH:MM:SS): 01:17:02 User tests executed: 19 User tests passed: 18 User tests failed: 1 User tests success rate: 94.74 % Failures: Test Name Status Reason kvm.qemu-kvm-git.unittests FAIL Unit tests failed: emulator Hi Marcelo, Avi: We've seen emulator unittest failures during the last couple of jobs of qemu-kvm.git userspace + kvm.git kernel. Relevant hashes for the last failure seen: 11/01 09:33:59 INFO |virt_utils:0501| Commit hash for git://github.com/avikivity/kvm.git is b796a09c5d808f4013f27ad45953db604dac18fd (tag v3.1-rc4-10168-gb796a09) 11/01 09:50:57 DEBUG|virt_utils:2587| Git repo qemu_kvm uri: git://github.com/avikivity/qemu.git 11/01 09:51:52 INFO |virt_utils:2531| Commit hash for qemu_kvm is 7879db7e9c09b92d9af1c143fbe2cc212ec89e4b (no tag found) Cheers, Lucas Is there a log with more details available?
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
Hi, It's not just about code, it's as much about culture and development process. Indeed. The BSDs have both kernel and the base system in a single repository. There are probably good reasons for (and against) it. In Linux we don't have that culture. No tool (except perf) lives in the kernel repo. I fail to see why kvm-tool is that much different from udev, util-linux, iproute, filesystem tools, that it should be included. cheers, Gerd
Re: [Qemu-devel] [v8 Patch 0/6]Qemu: Host pagecache setting from cmdline and monitor
On Mon, Nov 7, 2011 at 5:35 PM, Supriya Kannery supri...@linux.vnet.ibm.com wrote: On 11/07/2011 02:19 PM, Zhi Yong Wu wrote: On Mon, Nov 7, 2011 at 4:38 PM, Supriya Kannery supri...@linux.vnet.ibm.com wrote: On 11/04/2011 07:59 AM, Zhi Yong Wu wrote: On Sun, Oct 30, 2011 at 6:33 PM, Supriya Kannery supri...@linux.vnet.ibm.com wrote: Currently cache setting of a block device cannot be changed without restarting a running VM. Following patchset is for enabling dynamic change of cache setting for block devices through qemu monitor. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. This patchset introduces a. monitor command 'block_set_hostcache' using which host pagecache setting for a block device can be changed dynamically. I got a bit confusion. Is it used to change host pagecache setting on hyperviser or on guest? This block device said by you is for guest, right? This command is used for changing pagecache setting of the image files in host which are acting as block devices for guest. So what is the difference between it and cache option? Cache=xx sets a combination of status flags for image files among which host pagecache is just one. Intention here is to gradually replace cache=xx with more explicit settings like hostcache, flush, WCE. Great, thanks. -- Regards, Zhi Yong Wu
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
Am 06.11.2011 19:31, schrieb Ted Ts'o: On Sun, Nov 06, 2011 at 11:08:10AM -0600, Anthony Liguori wrote: I'm quite happy with KVM tool and hope they continue working on it. My only real wish is that they wouldn't copy QEMU so much and would try bolder things that are fundamentally different from QEMU. My big wish is that they don't try to merge the KVM tool into the kernel code. It's a separate userspace project, and there's no reason for it to be bundled with kernel code. It just makes the kernel sources larger. In fact, the reverse is true as well: It makes kvm-tool's sources larger. Instead on just cloning a small repository I need to clone the whole kernel repository, even though I'm not a kernel developer and don't intend to touch anything but tools/kvm. Not too bad for me as I have a kernel repository lying around anyway and I can share most of the content, but there are people who don't. Still, having an additional 1.2 GB repository just for ~1 MB in which I'm really interested doesn't make me too happy. And dealing with a huge repository also means that even git becomes slower (which means, I had to turn off some functionality for my shell prompt in this repo, as I didn't like waiting for much more than a second or two) Makes it a lot less hackable for me unless you want to restrict the set of potential developers to Linux kernel developers... Kevin
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 12:23 PM, Gerd Hoffmann kra...@redhat.com wrote: Hi, It's not just about code, it's as much about culture and development process. Indeed. The BSDs have both kernel and the base system in a single repository. There are probably good reasons for (and against) it. In Linux we don't have that culture. No tool (except perf) lives in the kernel repo. I fail to see why kvm-tool is that much different from udev, util-linux, iproute, filesystem tools, that it should be included. tools/power was merged in just 2 versions ago, do you think that merging that was a mistake?
Re: [Qemu-devel] Wierd hack to sound/pci/intel8x0.c
Hi, We discuss Intel ICH/AC'97 (snd-intel8x0) here, but I hope that PCI SSID is same. Hmm, it's not, the ac97 emulation doesn't use the default qemu subsystem id for some reason. Here is the entry: [root@fedora ~]# lspci -vns6 00:06.0 0401: 8086:2415 (rev 01) Subsystem: 8086: Physical Slot: 6 Flags: bus master, medium devsel, latency 0, IRQ 10 I/O ports at c000 [size=1K] I/O ports at c400 [size=256] Kernel driver in use: Intel ICH Kernel modules: snd-intel8x0 I guess that should be fixed. 8086: isn't valid anyway and I doubt it serves any useful purpose other than avoiding a guest complaining about the subsystem id being unset. /me goes preparing a patch. cheers, Gerd
Re: [Qemu-devel] Wierd hack to sound/pci/intel8x0.c
At Mon, 07 Nov 2011 10:25:24 +0100, Gerd Hoffmann wrote: Hi, Agreed. If we can know the virtual device for KVM in a better way (e.g. any specific PCI SSID or such), we can narrow the condition more safely. PCI Subsystem ID 1af4:1100 is qemu/kvm (not only Intel HDA, most other emulated pci devices have it too). Complete entry: 00:04.0 0403: 8086:2668 (rev 01) Subsystem: 1af4:1100 Physical Slot: 4 Flags: bus master, fast devsel, latency 0, IRQ 11 Memory at e203 (32-bit, non-prefetchable) [size=16K] Kernel driver in use: HDA Intel Kernel modules: snd-hda-intel Thanks. BTW, this reminds me of a possible improvement in the current HD-audio driver. It has some workaround for the delayed DMA-position update (the DMA-position isn't completely updated when IRQ is issued) by offsetting the buffer position. This is controlled by bdl_pos_adj option of snd-hda-intel driver, and this could be zero for VMs. Takashi
Re: [Qemu-devel] Wierd hack to sound/pci/intel8x0.c
At Mon, 07 Nov 2011 11:35:49 +0100, Gerd Hoffmann wrote: Hi, We discuss Intel ICH/AC'97 (snd-intel8x0) here, but I hope that PCI SSID is same. Hmm, it's not, the ac97 emulation doesn't use the default qemu subsystem id for some reason. Here is the entry: [root@fedora ~]# lspci -vns6 00:06.0 0401: 8086:2415 (rev 01) Subsystem: 8086: Physical Slot: 6 Flags: bus master, medium devsel, latency 0, IRQ 10 I/O ports at c000 [size=1K] I/O ports at c400 [size=256] Kernel driver in use: Intel ICH Kernel modules: snd-intel8x0 I guess that should be fixed. 8086: isn't valid anyway and I doubt it serves any useful purpose other than avoiding a guest complaining about the subsystem id being unset. Yeah, especially the value 0 looks pretty bad. /me goes preparing a patch. Thanks! Takashi
Re: [Qemu-devel] [RFC 1/6] block: add request tracking
On Mon, Oct 17, 2011 at 11:47 PM, Stefan Hajnoczi
stefa...@linux.vnet.ibm.com wrote:
The block layer does not know about pending requests. This information
is necessary for copy-on-read since overlapping requests must be
serialized to prevent races that corrupt the image.
Add a simple mechanism to enable/disable request tracking. By default
request tracking is disabled.
The BlockDriverState gets a new tracked_request list field which
contains all pending requests. Each request is a BdrvTrackedRequest
record with sector_num, nb_sectors, and is_write fields.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
block.c | 83
++-
block_int.h | 8 +
2 files changed, 90 insertions(+), 1 deletions(-)
diff --git a/block.c b/block.c
index 9873b57..2d2c62a 100644
--- a/block.c
+++ b/block.c
@@ -978,6 +978,77 @@ void bdrv_commit_all(void)
}
}
+struct BdrvTrackedRequest {
+ BlockDriverState *bs;
+ int64_t sector_num;
+ int nb_sectors;
+ bool is_write;
+ QLIST_ENTRY(BdrvTrackedRequest) list;
+};
+
+/**
+ * Remove an active request from the tracked requests list
+ *
+ * If req is NULL, no operation is performed.
+ *
+ * This function should be called when a tracked request is completing.
+ */
+static void tracked_request_remove(BdrvTrackedRequest *req)
+{
+ if (req) {
+ QLIST_REMOVE(req, list);
+ g_free(req);
+ }
+}
+
+/**
+ * Add an active request to the tracked requests list
+ *
+ * Return NULL if request tracking is disabled, non-NULL otherwise.
+ */
+static BdrvTrackedRequest *tracked_request_add(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, bool is_write)
+{
+ BdrvTrackedRequest *req;
+
+ if (!bs-request_tracking) {
+ return NULL;
+ }
+
+ req = g_malloc(sizeof(*req));
+ req-bs = bs;
+ req-sector_num = sector_num;
+ req-nb_sectors = nb_sectors;
+ req-is_write = is_write;
+
+ QLIST_INSERT_HEAD(bs-tracked_requests, req, list);
+
+ return req;
+}
+
+/**
+ * Enable tracking of incoming requests
+ *
+ * Request tracking can be safely used by multiple users at the same time,
+ * there is an internal reference count to match start and stop calls.
+ */
+void bdrv_start_request_tracking(BlockDriverState *bs)
+{
+ bs-request_tracking++;
+}
+
+/**
+ * Disable tracking of incoming requests
+ *
+ * Note that in-flight requests are still tracked, this function only stops
+ * tracking incoming requests.
+ */
+void bdrv_stop_request_tracking(BlockDriverState *bs)
+{
+ bs-request_tracking--;
+}
I don't understand what the real intention for the above functions is.
IMHO, why can we not drop them?
+
/*
* Return values:
* 0 - success
@@ -1262,6 +1333,8 @@ static int coroutine_fn
bdrv_co_do_readv(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
{
BlockDriver *drv = bs-drv;
+ BdrvTrackedRequest *req;
+ int ret;
if (!drv) {
return -ENOMEDIUM;
@@ -1270,7 +1343,10 @@ static int coroutine_fn
bdrv_co_do_readv(BlockDriverState *bs,
return -EIO;
}
- return drv-bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+ req = tracked_request_add(bs, sector_num, nb_sectors, false);
+ ret = drv-bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
+ tracked_request_remove(req);
+ return ret;
}
int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
@@ -1288,6 +1364,7 @@ static int coroutine_fn
bdrv_co_do_writev(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
{
BlockDriver *drv = bs-drv;
+ BdrvTrackedRequest *req;
int ret;
if (!bs-drv) {
@@ -1300,6 +1377,8 @@ static int coroutine_fn
bdrv_co_do_writev(BlockDriverState *bs,
return -EIO;
}
+ req = tracked_request_add(bs, sector_num, nb_sectors, true);
+
ret = drv-bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
if (bs-dirty_bitmap) {
@@ -1310,6 +1389,8 @@ static int coroutine_fn
bdrv_co_do_writev(BlockDriverState *bs,
bs-wr_highest_sector = sector_num + nb_sectors - 1;
}
+ tracked_request_remove(req);
+
return ret;
}
diff --git a/block_int.h b/block_int.h
index f2f4f2d..87ce8b4 100644
--- a/block_int.h
+++ b/block_int.h
@@ -43,6 +43,8 @@
#define BLOCK_OPT_PREALLOC preallocation
#define BLOCK_OPT_SUBFMT subformat
+typedef struct BdrvTrackedRequest BdrvTrackedRequest;
+
typedef struct AIOPool {
void (*cancel)(BlockDriverAIOCB *acb);
int aiocb_size;
@@ -206,6 +208,9 @@ struct BlockDriverState {
int in_use; /* users other than guest access, eg. block migration */
QTAILQ_ENTRY(BlockDriverState) list;
void *private;
+
+ int
[Qemu-devel] [PATCH 1.0] ac97: don't override the pci subsystem id
This patch removes the code lines which set the subsystem id for the emulated ac97 card to 8086:. Due to the device id being zero the subsystem id isn't vaild anyway. With the patch applied the sound card gets the default qemu subsystem id (1af4:1100) instead. Cc: Takashi Iwai ti...@suse.de Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/ac97.c |6 -- 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/hw/ac97.c b/hw/ac97.c index 6800af4..b43e435 100644 --- a/hw/ac97.c +++ b/hw/ac97.c @@ -1305,12 +1305,6 @@ static int ac97_initfn (PCIDevice *dev) c[PCI_BASE_ADDRESS_0 + 6] = 0x00; c[PCI_BASE_ADDRESS_0 + 7] = 0x00; -c[PCI_SUBSYSTEM_VENDOR_ID] = 0x86; /* svid subsystem vendor id rwo */ -c[PCI_SUBSYSTEM_VENDOR_ID + 1] = 0x80; - -c[PCI_SUBSYSTEM_ID] = 0x00; /* sid subsystem id rwo */ -c[PCI_SUBSYSTEM_ID + 1] = 0x00; - c[PCI_INTERRUPT_LINE] = 0x00; /* intr_ln interrupt line rw */ c[PCI_INTERRUPT_PIN] = 0x01; /* intr_pn interrupt pin ro */ -- 1.7.1
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/2011 11:30 AM, Sasha Levin wrote: In Linux we don't have that culture. No tool (except perf) lives in the kernel repo. I fail to see why kvm-tool is that much different from udev, util-linux, iproute, filesystem tools, that it should be included. tools/power was merged in just 2 versions ago, do you think that merging that was a mistake? Indeed I do not see any advantage, since all the interfaces they use are stable anyway (sysfs, msr.ko). If they had gone in x86info, for example, my distro (F16, not exactly conservative) would have likely picked those tools up already, but it didn't. Paolo
Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?
On 11/07/2011 11:04 AM, Thomas Schmitt wrote: In a thread on linux-hotplug, /dev/sg* is declared to be deprecated in favor of/dev/bsg/* and /dev/sr*. Will this become a problem ? I think /dev/bsg is backwards-compatible more or less, but it would help if it had decent documentation in the kernel tree. I did not succeed with googling for a way to get a block device running on top of file=/dev/sg*,if=virtio. No, that's not possible with /dev/sg. But I think this helps: -drive file=/dev/sr0,if=none,id=scsicd -device virtio-blk,drive=scsicd,logical_block_size=2048,physical_block_size=2048 At least here, dd works with this command line. This probably will be fixed in QEMU 1.1 (i.e. the release after the next one). The word passthrough does not show up in the context of drive emulation in any documentation inside the git clone. I only see statements about: passthrough security model. The word virtio is mentioned more often, but without much explanation of -drive use cases and pitfalls. (Only docs/qdev-device-use.txt has some more detailed information.) Yes, docs/qdev-device-use.txt helps. qemu -device virtio-blk,? too. Is there an external documentation emerging ? I would like to read it and contribute my experiences. Hopefully, everything would just work. :) You can work on wiki.qemu.org in the meanwhile. Paolo
Re: [Qemu-devel] [PATCH 1/3] sonic: fix typo
Would be nice to add description:
Remove a doubled semicolon.
Am 06.11.2011 22:48, schrieb Hervé Poussineau:
Signed-off-by: Hervé Poussineau hpous...@reactos.org
Reviewed-by: Andreas Färber afaer...@suse.de
Andreas
---
hw/dp8393x.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/dp8393x.c b/hw/dp8393x.c
index f66844b..cfec4cb 100644
--- a/hw/dp8393x.c
+++ b/hw/dp8393x.c
@@ -515,7 +515,7 @@ static void write_register(dp8393xState *s, int reg,
uint16_t val)
switch (reg) {
/* Command register */
case SONIC_CR:
-do_command(s, val);;
+do_command(s, val);
break;
/* Prevent write to read-only registers */
case SONIC_CAP2:
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 2/3] sonic: fix netcard reset
Am 06.11.2011 22:48, schrieb Hervé Poussineau: From: Herv Poussineau hpous...@reactos.org Typo. At least to me it's not obvious why this is correct, so please add an explanatory patch description. Andreas Signed-off-by: Hervé Poussineau hpous...@reactos.org --- hw/dp8393x.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/hw/dp8393x.c b/hw/dp8393x.c index cfec4cb..acb1604 100644 --- a/hw/dp8393x.c +++ b/hw/dp8393x.c @@ -845,8 +845,7 @@ static void nic_reset(void *opaque) s-regs[SONIC_CR] = SONIC_CR_RST | SONIC_CR_STP | SONIC_CR_RXDIS; s-regs[SONIC_DCR] = ~(SONIC_DCR_EXBUS | SONIC_DCR_LBR); s-regs[SONIC_RCR] = ~(SONIC_RCR_LB0 | SONIC_RCR_LB1 | SONIC_RCR_BRD | SONIC_RCR_RNT); -s-regs[SONIC_TCR] |= SONIC_TCR_NCRS | SONIC_TCR_PTX; -s-regs[SONIC_TCR] = ~SONIC_TCR_BCM; +s-regs[SONIC_TCR] = SONIC_TCR_NCRS | SONIC_TCR_PTX; s-regs[SONIC_IMR] = 0; s-regs[SONIC_ISR] = 0; s-regs[SONIC_DCR2] = 0; -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] fix segfault on fd-migration starting
Wen Congyang we...@cn.fujitsu.com wrote: We set s-mon to NULL in migrate_init. But we will use it to search fd when do fd-migration, and it will cause qemu crashed. Signed-off-by: Wen Congyang we...@cn.fujitsu.com --- migration.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/migration.c b/migration.c index 4b17566..d094381 100644 --- a/migration.c +++ b/migration.c @@ -383,7 +383,7 @@ static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc) s-bandwidth_limit = bandwidth_limit; s-blk = blk; s-shared = inc; -s-mon = NULL; +s-mon = mon; s-bandwidth_limit = bandwidth_limit; s-state = MIG_STATE_SETUP; Problem is real. Patch is wrong, because now we resume the monitor even when we are doing detached migration. Looking into it. Thanks, Juan.
Re: [Qemu-devel] [PATCH v2] block: Use bdrv functions to replace file operation in cow.c
Am 07.11.2011 07:52, schrieb Li Zhi Hui:
Since common file operation functions lack of error detection,
so change them to bdrv series functions.
v2: Only contains the function modified.
v1: Fix coding style and convert file operation functions to bdrv functions.
Signed-off-by: Li Zhi Hui zhihu...@linux.vnet.ibm.com
---
block/cow.c | 35 +--
1 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/block/cow.c b/block/cow.c
index 707c0aa..808fe31 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -243,12 +243,13 @@ static void cow_close(BlockDriverState *bs)
static int cow_create(const char *filename, QEMUOptionParameter *options)
{
-int fd, cow_fd;
struct cow_header_v2 cow_header;
struct stat st;
int64_t image_sectors = 0;
const char *image_filename = NULL;
int ret;
+BlockDriverState *cow_bs;
+BlockDriverState *image_bs;
/* Read out options */
while (options options-name) {
@@ -260,10 +261,15 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options)
options++;
}
-cow_fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
- 0644);
-if (cow_fd 0)
-return -errno;
+ret = bdrv_create_file(filename, options);
+if (ret 0) {
+return ret;
+}
+
+ret = bdrv_file_open(cow_bs, filename, BDRV_O_RDWR);
+if (ret 0) {
+return ret;
+}
memset(cow_header, 0, sizeof(cow_header));
cow_header.magic = cpu_to_be32(COW_MAGIC);
cow_header.version = cpu_to_be32(COW_VERSION);
@@ -271,16 +277,16 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options)
/* Note: if no file, we put a dummy mtime */
cow_header.mtime = cpu_to_be32(0);
-fd = open(image_filename, O_RDONLY | O_BINARY);
-if (fd 0) {
-close(cow_fd);
+ret = bdrv_file_open(image_bs, image_filename, BDRV_O_RDWR);
+if (ret 0) {
+bdrv_close(cow_bs);
goto mtime_fail;
}
-if (fstat(fd, st) != 0) {
-close(fd);
+if (stat(image_filename, st) != 0) {
+bdrv_close(image_bs);
goto mtime_fail;
}
-close(fd);
+bdrv_close(image_bs);
Why do you open image_bs? The only thing you do with it is closing it.
cow_header.mtime = cpu_to_be32(st.st_mtime);
mtime_fail:
pstrcpy(cow_header.backing_file, sizeof(cow_header.backing_file),
@@ -288,21 +294,22 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options)
}
cow_header.sectorsize = cpu_to_be32(512);
cow_header.size = cpu_to_be64(image_sectors * 512);
-ret = qemu_write_full(cow_fd, cow_header, sizeof(cow_header));
+ret = bdrv_pwrite(cow_bs, 0, cow_header, sizeof(cow_header));
if (ret != sizeof(cow_header)) {
ret = -errno;
This is wrong, ret already has the right value, and errno might be
different.
goto exit;
}
/* resize to include at least all the bitmap */
-ret = ftruncate(cow_fd, sizeof(cow_header) + ((image_sectors + 7) 3));
+ret = bdrv_truncate(cow_bs,
+sizeof(cow_header) + ((image_sectors + 7) 3));
if (ret) {
ret = -errno;
Same here.
goto exit;
}
exit:
-close(cow_fd);
+bdrv_close(cow_bs);
return ret;
}
This should be bdrv_delete.
Kevin
Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?
On Mon, Nov 7, 2011 at 7:13 PM, Paolo Bonzini pbonz...@redhat.com wrote: On 11/07/2011 11:04 AM, Thomas Schmitt wrote: In a thread on linux-hotplug, /dev/sg* is declared to be deprecated in favor of/dev/bsg/* and /dev/sr*. Will this become a problem ? I think /dev/bsg is backwards-compatible more or less, but it would help if it had decent documentation in the kernel tree. I did not succeed with googling for a way to get a block device running on top of file=/dev/sg*,if=virtio. No, that's not possible with /dev/sg. But I think this helps: -drive file=/dev/sr0,if=none,id=scsicd -device virtio-blk,drive=scsicd,logical_block_size=2048,physical_block_size=2048 It didn't work for me I started up one os=rh6.1 guest with above options. On guest: [root@localhost ~]# mount /dev/sr0 /tmp/1 mount: you must specify the filesystem type [root@localhost ~]# At least here, dd works with this command line. This probably will be fixed in QEMU 1.1 (i.e. the release after the next one). The word passthrough does not show up in the context of drive emulation in any documentation inside the git clone. I only see statements about: passthrough security model. The word virtio is mentioned more often, but without much explanation of -drive use cases and pitfalls. (Only docs/qdev-device-use.txt has some more detailed information.) Yes, docs/qdev-device-use.txt helps. qemu -device virtio-blk,? too. Is there an external documentation emerging ? I would like to read it and contribute my experiences. Hopefully, everything would just work. :) You can work on wiki.qemu.org in the meanwhile. Paolo -- Regards, Zhi Yong Wu
Re: [Qemu-devel] Hi Natalia.. A query regarding QEMU
Hi, What I can tell you is that Android phones/tablets are USB devices, and QEMU have an incomplete support for REAL USB devices with QEMU behaving as an USB host. But QEMU is not designed currently to work as a USB device to a real USB host. If you want to code that support yourself, you'll need a virtual host controller implementation on the host operating system, and then implementing QEMU behavior as an Android device (be it USB Mass Storage Device or Android Debug device). Another option would be to explore the usbredir support recently added to qemu. The focus of the current implementation is to support forwarding usb devices over the network. You have a little server running which export a usb device, and qemu's usbredir backend connects to the server and allows the guest to access the device. Of course you can replace the server side with something emulating a usb device, for example a emulated android device with usb-host support, i.e. you have two qemu's running, one with android, one with a linux or windows guest os, and you'll link the two using the usbredir protocol. Cc'ing Hans who wrote the usbredir support. cheers, Gerd
Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?
On 11/07/2011 12:24 PM, Zhi Yong Wu wrote: It didn't work for me I started up one os=rh6.1 guest with above options. On guest: [root@localhost ~]# mount /dev/sr0 /tmp/1 mount: you must specify the filesystem type [root@localhost ~]# You asked for a virtio disk, not a SCSI disk. The CD-ROM will be under /dev/vdX. mount works here. Paolo
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, 7 Nov 2011, Gerd Hoffmann wrote: It's not just about code, it's as much about culture and development process. Indeed. The BSDs have both kernel and the base system in a single repository. There are probably good reasons for (and against) it. In Linux we don't have that culture. No tool (except perf) lives in the kernel repo. I fail to see why kvm-tool is that much different from udev, util-linux, iproute, filesystem tools, that it should be included. You seem to think perf is an exception - I think it's going to be the future norm for userspace components that are very close to the kernel. That's in fact what Ingo was arguing for when he suggested QEMU to be merged to the kernel tree. Pekka
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, 7 Nov 2011, Kevin Wolf wrote: Makes it a lot less hackable for me unless you want to restrict the set of potential developers to Linux kernel developers... We're not restricting potential developers to Linux kernel folks. We're making it easy for them because we believe that the KVM tool is a userspace component that requires the kind of low-level knowledge Linux kernel developers have. I think you're looking at the KVM tool with your QEMU glasses on without realizing that there's no point in comparing the two: we only support Linux on Linux and we avoid hardware emulation as much as possible. So what makes sense for QEMU, doesn't necessarily translate to the KVM tool project. Pekka
Re: [Qemu-devel] Do you have a use for a tester of virtio-scsi with CD drives ?
On Mon, Nov 7, 2011 at 7:29 PM, Paolo Bonzini pbonz...@redhat.com wrote: On 11/07/2011 12:24 PM, Zhi Yong Wu wrote: It didn't work for me I started up one os=rh6.1 guest with above options. On guest: [root@localhost ~]# mount /dev/sr0 /tmp/1 mount: you must specify the filesystem type [root@localhost ~]# You asked for a virtio disk, not a SCSI disk. The CD-ROM will be under Sorry, made one mistake. Yeah, It can work. This id=scsicd maning is a bit confusing... Moreover, When the option -device lsi -device scsi-cd,... is specified, it can also work. /dev/vdX. mount works here. Paolo -- Regards, Zhi Yong Wu
Re: [Qemu-devel] [RFC 1/6] block: add request tracking
On Mon, Nov 7, 2011 at 11:00 AM, Zhi Yong Wu zwu.ker...@gmail.com wrote:
On Mon, Oct 17, 2011 at 11:47 PM, Stefan Hajnoczi
stefa...@linux.vnet.ibm.com wrote:
+/**
+ * Enable tracking of incoming requests
+ *
+ * Request tracking can be safely used by multiple users at the same time,
+ * there is an internal reference count to match start and stop calls.
+ */
+void bdrv_start_request_tracking(BlockDriverState *bs)
+{
+ bs-request_tracking++;
+}
+
+/**
+ * Disable tracking of incoming requests
+ *
+ * Note that in-flight requests are still tracked, this function only stops
+ * tracking incoming requests.
+ */
+void bdrv_stop_request_tracking(BlockDriverState *bs)
+{
+ bs-request_tracking--;
+}
I don't understand what the real intention for the above functions is.
IMHO, why can we not drop them?
I have dropped them after removing the g_malloc() as Kevin suggested.
The idea was to avoid the overhead of request tracking when no feature
is using request tracking.
Stefan
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 1:02 PM, Paolo Bonzini pbonz...@redhat.com wrote: Indeed I do not see any advantage, since all the interfaces they use are stable anyway (sysfs, msr.ko). If they had gone in x86info, for example, my distro (F16, not exactly conservative) would have likely picked those tools up already, but it didn't. Distributing userspace tools in the kernel tree is a relatively new concept so it's not at all surprising distributions don't pick them up as quickly. That doesn't mean it's a fundamentally flawed approach, though. Also, I'm mostly interested in defending the KVM tool, so I'd prefer not to argue whether or not carrying userspace code in the kernel tree makes sense or not. The fact is that Linux is already doing it and I think the only relevant question is whether or not the KVM tool qualifies. I obviously think the answer is yes. Pekka
Re: [Qemu-devel] [RFC 4/6] block: request overlap detection
On Mon, Oct 17, 2011 at 11:47 PM, Stefan Hajnoczi
stefa...@linux.vnet.ibm.com wrote:
Detect overlapping requests and remember to align to cluster boundaries
if the image format uses them. This assumes that allocating I/O is
performed in cluster granularity - which is true for qcow2, qed, etc.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
block.c | 39 +--
1 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index cc3202c..0c22741 100644
--- a/block.c
+++ b/block.c
@@ -1052,21 +1052,56 @@ static BdrvTrackedRequest
*tracked_request_add(BlockDriverState *bs,
return req;
}
+/**
+ * Round a region to cluster boundaries
+ */
+static void round_to_clusters(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors,
+ int64_t *cluster_sector_num,
+ int *cluster_nb_sectors)
+{
+ BlockDriverInfo bdi;
+
+ if (bdrv_get_info(bs, bdi) 0 || bdi.cluster_size == 0) {
+ *cluster_sector_num = sector_num;
+ *cluster_nb_sectors = nb_sectors;
+ } else {
+ int64_t c = bdi.cluster_size / BDRV_SECTOR_SIZE;
+ *cluster_sector_num = (sector_num / c) * c;
I can understand the above formula, but the one below is
very magic. :) and can not be understood by me.
+ *cluster_nb_sectors = ((sector_num % c) + nb_sectors + c - 1) / c *
c;
+ }
+}
+
static bool tracked_request_overlaps(BdrvTrackedRequest *req,
int64_t sector_num, int nb_sectors) {
- return false; /* not yet implemented */
+ /* */
+ if (sector_num = req-sector_num + req-nb_sectors) {
+ return false;
+ }
+ /* */
+ if (req-sector_num = sector_num + nb_sectors) {
+ return false;
+ }
+ return true;
}
static void coroutine_fn wait_for_overlapping_requests(BlockDriverState *bs,
int64_t sector_num, int nb_sectors)
{
BdrvTrackedRequest *req;
+ int64_t cluster_sector_num;
+ int cluster_nb_sectors;
bool retry;
+ /* If we touch the same cluster it counts as an overlap */
+ round_to_clusters(bs, sector_num, nb_sectors,
+ cluster_sector_num, cluster_nb_sectors);
+
do {
retry = false;
QLIST_FOREACH(req, bs-tracked_requests, list) {
- if (tracked_request_overlaps(req, sector_num, nb_sectors)) {
+ if (tracked_request_overlaps(req, cluster_sector_num,
+ cluster_nb_sectors)) {
qemu_co_queue_wait(req-wait_queue);
retry = true;
break;
--
1.7.6.3
--
Regards,
Zhi Yong Wu
Re: [Qemu-devel] GSoC mentor summit QEMU users session
Fabien Chouteau writes: On 04/11/2011 19:45, Lluís Vilanova wrote: I've only had a brief look into the changes, but I think the mechanism I implemented has a cleaner fit into QEMU, thanks to previous feedback from this list. I don't know about your implementation, can you give more information? What type of analysis is supported (object, statement, decision, MC/DC)? Which language? And maybe you can post a link to your repository. I'll be posting the patches once QEMU opens up for new commits other than release stabilization. If this is too far away in time, please tell me. It explicitly separates the tracing mechanism (in QEMU itself) from the specific trace analysis (which resides in a separate library specified by the user at compile time, where most of couverture would go). As I understand everything is compiled within Qemu, right? GNATcoverage seems even more separate. We use Qemu to generate an execution trace file and the coverage analysis tool is a totally separate program. You can add support for another language or implement your own coverage tool without recompiling (redistribute) Qemu. The process is basically: * Add trace events that can work during TCG code generation (e.g., start TB, start instruction fetch, memory access, etc.) * Let the user select which trace events to instrument, including both regular trace events and TCG trace events (thus you instrument at both execution and translation time). * The user provides her own implementation of the instrumented trace events. As you can see, this system only gives you the hooks were code can be inserted. Whether your hooks implement everything inside QEMU or just write a trace file, that is up to you. [...] On the other hand, I have a complementary set of events, so we can definitely join the efforts on that side (e.g., I haven't yet went into the trouble of adding the begin/end TB or branch events). I don't know what do you mean by events, but we sure can join efforts on coverage with Qemu. Well, my target is not code coverage, but generating events that can be used for architecture simulation. In any case, there will surely be trace events that we're both interested in (e.g., TB start and branch). 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] KVM: Add wrapper script around QEMU to test kernels
Am 07.11.2011 12:38, schrieb Pekka Enberg: On Mon, 7 Nov 2011, Kevin Wolf wrote: Makes it a lot less hackable for me unless you want to restrict the set of potential developers to Linux kernel developers... We're not restricting potential developers to Linux kernel folks. We're making it easy for them because we believe that the KVM tool is a userspace component that requires the kind of low-level knowledge Linux kernel developers have. I think you're looking at the KVM tool with your QEMU glasses on without realizing that there's no point in comparing the two: we only support Linux on Linux and we avoid hardware emulation as much as possible. So what makes sense for QEMU, doesn't necessarily translate to the KVM tool project. I'm not comparing anything. I'm not even referring to the virtualization functionality of it. It could be doing anything else and it wouldn't make a difference. For KVM tool I am not much more than a mere user. Trying it out was tedious for me, as it is for anyone else who isn't a kernel developer. That's all I'm saying. Making things easier for some kernel developers but ignoring that at the same time it makes things harder for users I consider a not so clever move. Just wanted to point that out; feel free to ignore it, your priorities are probably different. Kevin
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
* Pekka Enberg penb...@cs.helsinki.fi wrote: On Mon, 7 Nov 2011, Gerd Hoffmann wrote: It's not just about code, it's as much about culture and development process. Indeed. The BSDs have both kernel and the base system in a single repository. There are probably good reasons for (and against) it. In Linux we don't have that culture. No tool (except perf) lives in the kernel repo. I fail to see why kvm-tool is that much different from udev, util-linux, iproute, filesystem tools, that it should be included. You seem to think perf is an exception - I think it's going to be the future norm for userspace components that are very close to the kernel. That's in fact what Ingo was arguing for when he suggested QEMU to be merged to the kernel tree. Yep, and the answer i got from the Qemu folks when i suggested that merge was a polite buzz off, along the lines of: We don't want to do that, but feel free to write your own tool, leave Qemu alone. Now that people have done exactly that some Qemu folks not only have changed their objection from write your own tool to erm, write your own tool but do it the way *we* prefer you to do it - they also started contributing *against* the KVM tool with predictable, once every 3 months objections against its upstream merge... That's not very nice and not very constructive. The only valid technical objection against tools/kvm/ that i can see would be that it's not useful enough yet for the upstream kernel versus other tools such as Qemu. In all fairness i think we might still be at that early stage of the project but it's clearly progressing very rapidly and i'm already using it on a daily basis for my own kernel testing purposes. During the Kernel Summit that's how i tested contemporary kernels on contemporary user-space remotely, without having to risk a physical reboot. Thanks, Ingo
Re: [Qemu-devel] [qemu-kvm unittest regression] Re: Autotest | Job ID: 2011 Upstream qemu-kvm.git sanity 11-01-2011 00:04:02 | Status: 1 Completed | Success Rate: 94.74 %
On 11/07/2011 07:21 AM, Marcelo Tosatti wrote: On Tue, Nov 01, 2011 at 02:08:54PM -0200, Lucas Meneghel Rodrigues wrote: On 11/01/2011 12:17 PM, kvm-autotest wrote: Job ID: 2011 Job name: Upstream qemu-kvm.git sanity 11-01-2011 00:04:02 Summary: Host: Status: Completed Status: 1 Completed Execution time (HH:MM:SS): 01:17:02 User tests executed: 19 User tests passed: 18 User tests failed: 1 User tests success rate: 94.74 % Failures: Test Name Status Reason kvm.qemu-kvm-git.unittests FAIL Unit tests failed: emulator Hi Marcelo, Avi: We've seen emulator unittest failures during the last couple of jobs of qemu-kvm.git userspace + kvm.git kernel. Relevant hashes for the last failure seen: 11/01 09:33:59 INFO |virt_utils:0501| Commit hash for git://github.com/avikivity/kvm.git is b796a09c5d808f4013f27ad45953db604dac18fd (tag v3.1-rc4-10168-gb796a09) 11/01 09:50:57 DEBUG|virt_utils:2587| Git repo qemu_kvm uri: git://github.com/avikivity/qemu.git 11/01 09:51:52 INFO |virt_utils:2531| Commit hash for qemu_kvm is 7879db7e9c09b92d9af1c143fbe2cc212ec89e4b (no tag found) Cheers, Lucas Is there a log with more details available? Marcelo, The Autotest landing page for this job is: http://autotest.virt.bos.redhat.com/afe/#tab_id=view_jobobject_id=2011 You can jump straight to the unittest logs by going to: http://autotest.virt.bos.redhat.com/results/2011-autotest/virtlab201.virt.bos.redhat.com/kvm.qemu-kvm-git.unittests/debug/ And this is the autotest log entries we got while running emulator: 11/01 09:54:32 INFO | unittest:0052| Running emulator 11/01 09:54:32 DEBUG|virt_env_p:0052| Preprocessing VM 'None' 11/01 09:54:32 INFO |kvm_vm:0790| Running qemu command: /usr/local/autotest/tests/kvm/qemu -name 'None' -nodefaults -vga std -monitor unix:'/tmp/monitor-humanmonitor1-2001-095328-ESmp',server,nowait -qmp unix:'/tmp/monitor-qmpmonitor1-2001-095328-ESmp',server,nowait -serial unix:'/tmp/serial-2001-095328-ESmp',server,nowait -m 512 -smp 2 -kernel '/usr/local/autotest/tests/kvm/unittests/emulator.flat' -vnc :0 -chardev file,id=testlog,path=/tmp/testlog-2001-095328-ESmp -device testdev,chardev=testlog -S 11/01 09:54:33 DEBUG|kvm_monito:0624| (monitor qmpmonitor1) Sending command 'qmp_capabilities' 11/01 09:54:33 DEBUG|kvm_vm:0851| VM appears to be alive with PID 18682 11/01 09:54:33 DEBUG|kvm_monito:0254| (monitor humanmonitor1) Sending command 'cont' 11/01 09:54:33 INFO | unittest:0096| Waiting for unittest emulator to complete, timeout 600, output in /tmp/testlog-2001-095328-ESmp 11/01 09:54:35 INFO | aexpect:0783| [qemu output] (Process terminated with status 1) 11/01 09:54:35 ERROR| unittest:0104| Unit test emulator failed 11/01 09:54:35 INFO | unittest:0113| Unit test log collected and available under /usr/local/autotest/results/default/kvm.qemu-kvm-git.unittests/debug/emulator.log Unfortunately, the log for the emulator test itself (emulator.log) is empty. If you think it's helpful, we can setup the same environment again, so that you can manually run and debug this failure. Thanks.
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/11 12:34, Pekka Enberg wrote: On Mon, 7 Nov 2011, Gerd Hoffmann wrote: It's not just about code, it's as much about culture and development process. Indeed. The BSDs have both kernel and the base system in a single repository. There are probably good reasons for (and against) it. In Linux we don't have that culture. No tool (except perf) lives in the kernel repo. I fail to see why kvm-tool is that much different from udev, util-linux, iproute, filesystem tools, that it should be included. You seem to think perf is an exception - I think it's going to be the future norm for userspace components that are very close to the kernel. perf *is* an exception today. It might make sense to change that. But IMHO it only makes sense if there is a really broad agreement on it and other core stuff moves into the kernel too. Then you'll be able to get advantages out of it. For example standardizing the process to create an initramfs (using the userspace tools shipped with the kernel) instead of having each distro creating its own way. I somehow doubt we'll see such an broad agreement though. Most people seem to be happy with the current model. There is a reason why the klibc + early-userspace-in-kernel-tree project died in the end ... cheers, Gerd
Re: [Qemu-devel] qemu-kvm crashes doing migration with disks + blkdebug files (does not happen with qemu)
On 11/07/2011 07:29 AM, Kevin Wolf wrote: Am 05.11.2011 03:16, schrieb Lucas Meneghel Rodrigues: Hi folks, qemu-kvm is segfaulting when executing migration with blkdebug files. 19:50:02 DEBUG| Git repo qemu_kvm uri: git://github.com/avikivity/qemu.git 19:50:02 DEBUG| Git repo qemu_kvm branch: master 19:50:30 INFO | Commit hash for qemu_kvm is 7879db7e9c09b92d9af1c143fbe2cc212ec89e4b (no tag found) How to reproduce: 1) create a origin vm like: /usr/local/autotest/tests/kvm/qemu -name 'vm1' -nodefaults -vga std -monitor unix:'/tmp/monitor-humanmonitor1-2004-200902-95j0',server,nowait -qmp unix:'/tmp/monitor-qmpmonitor1-2004-200902-95j0',server,nowait -serial unix:'/tmp/serial-2004-200902-95j0',server,nowait -drive file=blkdebug:/usr/local/autotest/virt/blkdebug/default.conf:/tmp/kvm_autotest_root/images/rhel6.1-64.qcow2,index=0,if=virtio,cache=none,rerror=stop,werror=stop -device virtio-net-pci,netdev=idtzhBVb,mac='9a:d0:7b:07:18:72',id='id9JW3ZV' -netdev tap,id=idtzhBVb,fd=23 -m 2048 -smp 2 -vnc :0 2) create a destination vm like: /usr/local/autotest/tests/kvm/qemu -name 'vm1' -nodefaults -vga std -monitor unix:'/tmp/monitor-humanmonitor1-2004-201329-Ia9o',server,nowait -qmp unix:'/tmp/monitor-qmpmonitor1-2004-201329-Ia9o',server,nowait -serial unix:'/tmp/serial-2004-201329-Ia9o',server,nowait -drive file=blkdebug:/usr/local/autotest/virt/blkdebug/default.conf:/tmp/kvm_autotest_root/images/rhel6.1-64.qcow2,index=0,if=virtio,cache=none,rerror=stop,werror=stop -device virtio-net-pci,netdev=idup1xAf,mac='9a:d0:7b:07:18:72',id='idyvOQf3' -netdev tap,id=idup1xAf,fd=19 -m 2048 -smp 2 -vnc :1 -S -incoming exec:nc -l 5200 Note that blkdebug file contains: [inject-error] state = 2 event = read_aio errno = 7 immediately = off once = on [set-state] state = 1 event = read_aio new_state = 2 [set-state] state = 2 event = read_aio new_state = 3 Start the migration (on this example, using exec, but it reproduces with tcp and unix sockets): 11/04 20:13:30 DEBUG|kvm_monito:0254| (monitor humanmonitor1) Sending command 'migrate -d exec:nc localhost 5200' Then you will have: 11/04 20:13:33 INFO | aexpect:0783| [qemu output] invalid runstate transition Invalid runstate transition is something for Luiz (CCed). Though probably he doesn't need to do anything in this case: I think we're not allowing the transition from I/O error to migrating. This might be fixed by 8a9236f1 in qemu.git, so please retest with upstream. In the end of my original message, I stated the same test does not yield a segfault in qemu.git, so the referred commit indeed fixes the issue. Thanks!
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/11 12:44, Pekka Enberg wrote: On Mon, Nov 7, 2011 at 1:02 PM, Paolo Bonzini pbonz...@redhat.com wrote: Indeed I do not see any advantage, since all the interfaces they use are stable anyway (sysfs, msr.ko). If they had gone in x86info, for example, my distro (F16, not exactly conservative) would have likely picked those tools up already, but it didn't. Distributing userspace tools in the kernel tree is a relatively new concept so it's not at all surprising distributions don't pick them up as quickly. That doesn't mean it's a fundamentally flawed approach, though. tools/ lacks a separation into kernel hacker's testing+debugging toolbox and userspace tools. It lacks proper buildsystem integration for the userspace tools, there is no make tools and also no make tools_install. Silently dropping new stuff into tools/ and expecting the world magically noticing isn't going to work. cheers, Gerd
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 2:18 PM, Gerd Hoffmann kra...@redhat.com wrote: tools/ lacks a separation into kernel hacker's testing+debugging toolbox and userspace tools. It lacks proper buildsystem integration for the userspace tools, there is no make tools and also no make tools_install. Silently dropping new stuff into tools/ and expecting the world magically noticing isn't going to work. No disagreement here. Pekka
Re: [Qemu-devel] [PATCH] Support for UDP unicast network backend
On 2011-11-07 15:01, Benjamin wrote:
Here is the updated patch, using only localaddr. mcast expects it to be
addr whereas udp expects addr:port. It's documented in the help output.
I also corrected the error message when checking udp parameters.
Sorry, missed that: please never forget to run your patches through
checkpatch.pl. Once fixed, consider this
Acked-by: Jan Kiszka jan.kis...@siemens.com
Signed-off-by: Benjamin MARSILI marsi...@epitech.eu
---
net.c |9 +-
net/socket.c| 66
+-
qemu-options.hx |2 +
3 files changed, 73 insertions(+), 4 deletions(-)
diff --git a/net.c b/net.c
index cb52050..c75be08 100644
--- a/net.c
+++ b/net.c
@@ -999,7 +999,11 @@ static const struct {
}, {
.name = localaddr,
.type = QEMU_OPT_STRING,
-.help = source address for multicast packets,
+.help = source address and port for multicast and udp
packets,
+}, {
+.name = udp,
+.type = QEMU_OPT_STRING,
+.help = UDP unicast address and port number,
},
{ /* end of list */ }
},
@@ -1070,7 +1074,8 @@ int net_client_init(Monitor *mon, QemuOpts *opts,
int is_netdev)
#ifdef CONFIG_VDE
strcmp(type, vde) != 0
#endif
-strcmp(type, socket) != 0) {
+strcmp(type, socket) != 0
+strcmp(type, udp) != 0) {
qerror_report(QERR_INVALID_PARAMETER_VALUE, type,
a netdev backend type);
return -1;
diff --git a/net/socket.c b/net/socket.c
index e9ef128..3601228 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -524,6 +524,52 @@ static int net_socket_mcast_init(VLANState *vlan,
}
+static int net_socket_udp_init(VLANState *vlan,
+ const char *model,
+ const char *name,
+ const char *rhost,
+ const char *lhost)
+{
+NetSocketState *s;
+int fd, val, ret;
+struct sockaddr_in laddr, raddr;
+
+if (parse_host_port(laddr, lhost) 0)
+return -1;
+
+if (parse_host_port(raddr, rhost) 0)
+return -1;
+
+fd = qemu_socket(PF_INET, SOCK_DGRAM, 0);
+if (fd 0) {
+perror(socket(PF_INET, SOCK_DGRAM));
+return -1;
+}
+val = 1;
+ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
+ (const char *)val, sizeof(val));
+if (ret 0) {
+ perror(setsockopt(SOL_SOCKET, SO_REUSEADDR));
+return -1;
+}
+ret = bind(fd, (struct sockaddr *)laddr, sizeof(laddr));
+if (ret 0) {
+perror(bind);
+return -1;
+}
+
+s = net_socket_fd_init(vlan, model, name, fd, 0);
+if (!s)
+return -1;
+
+s-dgram_dst = raddr;
+
+snprintf(s-nc.info_str, sizeof(s-nc.info_str),
+ socket: udp=%s:%d,
+ inet_ntoa(raddr.sin_addr), ntohs(raddr.sin_port));
+return 0;
+}
+
int net_init_socket(QemuOpts *opts,
Monitor *mon,
const char *name,
@@ -597,10 +643,26 @@ int net_init_socket(QemuOpts *opts,
if (net_socket_mcast_init(vlan, socket, name, mcast,
localaddr) == -1) {
return -1;
}
+} else if (qemu_opt_get(opts, udp)) {
+const char *udp, *localaddr;
+
+if (qemu_opt_get(opts, fd) ||
+qemu_opt_get(opts, connect) ||
+qemu_opt_get(opts, listen) ||
+qemu_opt_get(opts, mcast)) {
+error_report(fd=, connect=, listen= and mcast= is invalid
with udp=);
+return -1;
+}
+
+udp = qemu_opt_get(opts, udp);
+localaddr = qemu_opt_get(opts, localaddr);
+
+if (net_socket_udp_init(vlan, udp, name, udp, localaddr) ==
-1) {
+return -1;
+}
} else {
-error_report(-socket requires fd=, listen=, connect= or mcast=);
+error_report(-socket requires fd=, listen=, connect=, mcast=
or udp=);
return -1;
}
-
return 0;
}
diff --git a/qemu-options.hx b/qemu-options.hx
index 681eaf1..5495368 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1217,6 +1217,8 @@ DEF(net, HAS_ARG, QEMU_OPTION_net,
-net
socket[,vlan=n][,name=str][,fd=h][,mcast=maddr:port[,localaddr=addr]]\n
connect the vlan 'n' to multicast maddr and port\n
use 'localaddr=addr' to specify the host address
to send packets from\n
+-net
socket[,vlan=n][,name=str][,fd=h][,udp=host:port][,localaddr=host:port]\n
+connect the vlan 'n' to another VLAN using an UDP
tunnel\n
#ifdef CONFIG_VDE
-net
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/2011 12:30 PM, Sasha Levin wrote: On Mon, Nov 7, 2011 at 12:23 PM, Gerd Hoffmann kra...@redhat.com wrote: Hi, It's not just about code, it's as much about culture and development process. Indeed. The BSDs have both kernel and the base system in a single repository. There are probably good reasons for (and against) it. In Linux we don't have that culture. No tool (except perf) lives in the kernel repo. I fail to see why kvm-tool is that much different from udev, util-linux, iproute, filesystem tools, that it should be included. tools/power was merged in just 2 versions ago, do you think that merging that was a mistake? Things like tools/power may make sense, most of the code is tied to the kernel interfaces. tools/kvm is 20k lines and is likely to be 40k+ lines or more before it is generally usable. The proportion of the code that talks to the kernel is quite small. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 07, 2011 at 01:08:50PM +0100, Gerd Hoffmann wrote: perf *is* an exception today. It might make sense to change that. But IMHO it only makes sense if there is a really broad agreement on it and other core stuff moves into the kernel too. Then you'll be able to get advantages out of it. For example standardizing the process to create an initramfs (using the userspace tools shipped with the kernel) instead of having each distro creating its own way. I wish distributions had standardized on a single initramfs, sure. But that doesn't mean that the only way to do this is to merge userspace code into the kernel source tree. Everybody uses fsck, originally from the e2fsprogs source tree, and now from util-linux-ng, and that isn't merged into the kernel sources. And I think would be actively *harmful* to merge util-linux-ng into the kernel sources. For a variety of reasons, you may want to upgrade util-linux-ng, and not the kernel, or the kernel, and not util-linux-ng. If you package the two sources together, it becomes unclear what versions of the kernel will work with which versions of util-linux-ng, and vice versa. Suppose you need to fix a security bug in some program that lives in util-linux-ng. If it was bundled inside the kernel, a distribution would now have to release a kernel source package. Does that mean that it will have to ship the a new set of kernel binaries? Or does the distribution have to ship multiple binary packages that derive from the differently versioned source packages? And the same problems will exist with kvm-tool. What if you need to release a new version of kvm-tool? Does that mean that you have to release a new set of kernel binaries? It's a mess, and there's a reason why we don't have glibc, e2fsprogs, xfsprogs, util-linux-ng, etc., all packaged into the kernel sources. Because it's a stupid, idiotic thing to do. - Ted
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
Hi Avi, On Mon, Nov 7, 2011 at 2:26 PM, Avi Kivity a...@redhat.com wrote: tools/power was merged in just 2 versions ago, do you think that merging that was a mistake? Things like tools/power may make sense, most of the code is tied to the kernel interfaces. tools/kvm is 20k lines and is likely to be 40k+ lines or more before it is generally usable. The proportion of the code that talks to the kernel is quite small. So what do you think about perf then? The amount of code that talks to the kernel is much smaller than that of the KVM tool. Pekka
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
Hi Ted, On Mon, Nov 7, 2011 at 2:29 PM, Ted Ts'o ty...@mit.edu wrote: And the same problems will exist with kvm-tool. What if you need to release a new version of kvm-tool? Does that mean that you have to release a new set of kernel binaries? It's a mess, and there's a reason why we don't have glibc, e2fsprogs, xfsprogs, util-linux-ng, etc., all packaged into the kernel sources. If we need to release a new version, patches would go through the -stable tree just like with any other subsystem. On Mon, Nov 7, 2011 at 2:29 PM, Ted Ts'o ty...@mit.edu wrote: Because it's a stupid, idiotic thing to do. The discussion is turning into whether or not linux/tools makes sense or not. I wish you guys would have had it before perf was merged to the tree. Pekka
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 07, 2011 at 02:29:45PM +0200, Pekka Enberg wrote: So what do you think about perf then? The amount of code that talks to the kernel is much smaller than that of the KVM tool. I think it's a mess, because it's never clear whether perf needs to be upgraded when I upgrade the kernel, or vice versa. This is why I keep harping on the interface issues. Fortunately it seems less likely (since perf doesn't run with privileges) that security fixes will need to be released for perf, but if it did, given the typical regression testing requirements that many distributions have, and given that most distro packaging tools assume that all binaries from a single source package come from a single version of that source package, I predict you will hear screams from the distro release engineers. And by the way, there are use cases, where the guest OS kernel and root on the guest OS are not available to the untrusted users, where the userspace KVM program would be part of the security perimeter, and were security releases to the KVM part of the tool might very well be necessary, and it would be unfortunate if that forced the release of new kernel packages each time security fixes are needed to the kvm-tool userspace. Might kvm-tool be more secure than qemu? Quite possibly, given that it's going to do less than qemu. But please note that I've not been arguing that kvm-tool shouldn't be done; just that it not be included in the kernel sources. Just as sparse is not bundled into the kernel sources, for crying out loud! - Ted
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/2011 02:29 PM, Pekka Enberg wrote: Hi Avi, On Mon, Nov 7, 2011 at 2:26 PM, Avi Kivity a...@redhat.com wrote: tools/power was merged in just 2 versions ago, do you think that merging that was a mistake? Things like tools/power may make sense, most of the code is tied to the kernel interfaces. tools/kvm is 20k lines and is likely to be 40k+ lines or more before it is generally usable. The proportion of the code that talks to the kernel is quite small. So what do you think about perf then? The amount of code that talks to the kernel is much smaller than that of the KVM tool. Maybe it's outgrown the kernel repo too. Certainly something that has perl and python integration, a TUI, and one day hopefully a GUI, doesn't really need the kernel sources. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 07, 2011 at 02:42:57PM +0200, Pekka Enberg wrote: On Mon, Nov 7, 2011 at 2:29 PM, Ted Ts'o ty...@mit.edu wrote: Because it's a stupid, idiotic thing to do. The discussion is turning into whether or not linux/tools makes sense or not. I wish you guys would have had it before perf was merged to the tree. Perf was IMHO an overreaction caused by the fact that systemtap and oprofile people packaged and released the sources in a way that kernel developers didn't like. I don't think perf should be used as a precendent that now argues that any new kernel utility should be moved into the kernel sources. Does it make sense to move all of mount, fsck, login, etc., into the kernel sources? There are far more kernel tools outside of the kernel sources than inside the kernel sources. - Ted
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 2:47 PM, Ted Ts'o ty...@mit.edu wrote: Perf was IMHO an overreaction caused by the fact that systemtap and oprofile people packaged and released the sources in a way that kernel developers didn't like. I don't think perf should be used as a precendent that now argues that any new kernel utility should be moved into the kernel sources. Does it make sense to move all of mount, fsck, login, etc., into the kernel sources? There are far more kernel tools outside of the kernel sources than inside the kernel sources. There's two overlapping questions here: (1) Does it make sense to merge the KVM tool to Linux kernel tree? (2) Does it make sense to merge userspace tools to the kernel tree? I'm not trying to use perf to justify merging the KVM tool. However, you seem to be arguing that it shouldn't be merged because merging userspace tools in general doesn't make sense. That's why I brought up the situation with perf. Pekka
Re: [Qemu-devel] [PATCH 1.0] qxl: fix vga port initialization.
On Thu, Nov 03, 2011 at 06:21:54PM +0100, Gerd Hoffmann wrote:
Commit 0a039dc70096b768d3810afa50ba1d214768aaf4 broke vga modes for
qxl-vga by loosing vga_ioport_read windup. qxl needs to hook into
vga port writes only and used to realize that by letting vga_init() do
the work for both reads and writes, then overwrite the write function.
That little detail was missed while doing the conversion ...
This patch fixes it. It also switch qxl vga ioport registration to
portio lists while being at it.
Acked-by: Alon Levy al...@redhat.com
Cc: Hans de Goede hdego...@redhat.com
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/qxl.c | 22 +-
1 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 84ffd45..41500e9 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -896,6 +896,20 @@ static void qxl_vga_ioport_write(void *opaque, uint32_t
addr, uint32_t val)
vga_ioport_write(opaque, addr, val);
}
+static const MemoryRegionPortio qxl_vga_portio_list[] = {
+{ 0x04, 2, 1, .read = vga_ioport_read,
+ .write = qxl_vga_ioport_write }, /* 3b4 */
+{ 0x0a, 1, 1, .read = vga_ioport_read,
+ .write = qxl_vga_ioport_write }, /* 3ba */
+{ 0x10, 16, 1, .read = vga_ioport_read,
+ .write = qxl_vga_ioport_write }, /* 3c0 */
+{ 0x24, 2, 1, .read = vga_ioport_read,
+ .write = qxl_vga_ioport_write }, /* 3d4 */
+{ 0x2a, 1, 1, .read = vga_ioport_read,
+ .write = qxl_vga_ioport_write }, /* 3da */
+PORTIO_END_OF_LIST(),
+};
+
static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t
delta,
qxl_async_io async)
{
@@ -1595,6 +1609,7 @@ static int qxl_init_primary(PCIDevice *dev)
PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev);
VGACommonState *vga = qxl-vga;
ram_addr_t ram_size = msb_mask(qxl-vga.vram_size * 2 - 1);
+PortioList *qxl_vga_port_list = g_new(PortioList, 1);
qxl-id = 0;
@@ -1603,11 +1618,8 @@ static int qxl_init_primary(PCIDevice *dev)
}
vga_common_init(vga, ram_size);
vga_init(vga, pci_address_space(dev), pci_address_space_io(dev), false);
-register_ioport_write(0x3c0, 16, 1, qxl_vga_ioport_write, vga);
-register_ioport_write(0x3b4, 2, 1, qxl_vga_ioport_write, vga);
-register_ioport_write(0x3d4, 2, 1, qxl_vga_ioport_write, vga);
-register_ioport_write(0x3ba, 1, 1, qxl_vga_ioport_write, vga);
-register_ioport_write(0x3da, 1, 1, qxl_vga_ioport_write, vga);
+portio_list_init(qxl_vga_port_list, qxl_vga_portio_list, vga, vga);
+portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0);
vga-ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate,
qxl_hw_screen_dump, qxl_hw_text_update,
qxl);
--
1.7.1
Re: [Qemu-devel] [PATCH] Support for UDP unicast network backend
On 11/07/11 12:23, Jan Kiszka wrote:
On 2011-11-07 15:01, Benjamin wrote:
Here is the updated patch, using only localaddr. mcast expects it to be
addr whereas udp expects addr:port. It's documented in the help output.
I also corrected the error message when checking udp parameters.
Sorry, missed that: please never forget to run your patches through
checkpatch.pl. Once fixed, consider this
Acked-by: Jan Kiszkajan.kis...@siemens.com
Oh, my bad, I happen to have copied some parts from net/socket.c
that do no respect the coding style requirements. Well I guess I know
what my next submission will be.
Fixed, it's my first time submitting a patch here, is this enough?
Support for UDP unicast network backend
Signed-off-by: Benjamin MARSILI marsi...@epitech.eu
Acked-by: Jan Kiszka jan.kis...@siemens.com
---
net.c |9 +-
net/socket.c| 71
+-
qemu-options.hx |2 +
3 files changed, 78 insertions(+), 4 deletions(-)
diff --git a/net.c b/net.c
index cb52050..c75be08 100644
--- a/net.c
+++ b/net.c
@@ -999,7 +999,11 @@ static const struct {
}, {
.name = localaddr,
.type = QEMU_OPT_STRING,
-.help = source address for multicast packets,
+.help = source address and port for multicast and udp
packets,
+}, {
+.name = udp,
+.type = QEMU_OPT_STRING,
+.help = UDP unicast address and port number,
},
{ /* end of list */ }
},
@@ -1070,7 +1074,8 @@ int net_client_init(Monitor *mon, QemuOpts *opts,
int is_netdev)
#ifdef CONFIG_VDE
strcmp(type, vde) != 0
#endif
-strcmp(type, socket) != 0) {
+strcmp(type, socket) != 0
+strcmp(type, udp) != 0) {
qerror_report(QERR_INVALID_PARAMETER_VALUE, type,
a netdev backend type);
return -1;
diff --git a/net/socket.c b/net/socket.c
index e9ef128..ca183f2 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -524,6 +524,55 @@ static int net_socket_mcast_init(VLANState *vlan,
}
+static int net_socket_udp_init(VLANState *vlan,
+ const char *model,
+ const char *name,
+ const char *rhost,
+ const char *lhost)
+{
+NetSocketState *s;
+int fd, val, ret;
+struct sockaddr_in laddr, raddr;
+
+if (parse_host_port(laddr, lhost) 0) {
+return -1;
+}
+
+if (parse_host_port(raddr, rhost) 0) {
+return -1;
+}
+
+fd = qemu_socket(PF_INET, SOCK_DGRAM, 0);
+if (fd 0) {
+perror(socket(PF_INET, SOCK_DGRAM));
+return -1;
+}
+val = 1;
+ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
+ (const char *)val, sizeof(val));
+if (ret 0) {
+perror(setsockopt(SOL_SOCKET, SO_REUSEADDR));
+return -1;
+}
+ret = bind(fd, (struct sockaddr *)laddr, sizeof(laddr));
+if (ret 0) {
+perror(bind);
+return -1;
+}
+
+s = net_socket_fd_init(vlan, model, name, fd, 0);
+if (!s) {
+return -1;
+}
+
+s-dgram_dst = raddr;
+
+snprintf(s-nc.info_str, sizeof(s-nc.info_str),
+ socket: udp=%s:%d,
+ inet_ntoa(raddr.sin_addr), ntohs(raddr.sin_port));
+return 0;
+}
+
int net_init_socket(QemuOpts *opts,
Monitor *mon,
const char *name,
@@ -597,10 +646,28 @@ int net_init_socket(QemuOpts *opts,
if (net_socket_mcast_init(vlan, socket, name, mcast,
localaddr) == -1) {
return -1;
}
+} else if (qemu_opt_get(opts, udp)) {
+const char *udp, *localaddr;
+
+if (qemu_opt_get(opts, fd) ||
+qemu_opt_get(opts, connect) ||
+qemu_opt_get(opts, listen) ||
+qemu_opt_get(opts, mcast)) {
+error_report(fd=, connect=, listen=\
+ and mcast= is invalid with udp=);
+return -1;
+}
+
+udp = qemu_opt_get(opts, udp);
+localaddr = qemu_opt_get(opts, localaddr);
+
+if (net_socket_udp_init(vlan, udp, name, udp, localaddr) == -1) {
+return -1;
+}
} else {
-error_report(-socket requires fd=, listen=, connect= or mcast=);
+error_report(-socket requires fd=, listen=, \
+ connect=, mcast= or udp=);
return -1;
}
-
return 0;
}
diff --git a/qemu-options.hx b/qemu-options.hx
index 681eaf1..5495368 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1217,6 +1217,8 @@ DEF(net, HAS_ARG, QEMU_OPTION_net,
-net
socket[,vlan=n][,name=str][,fd=h][,mcast=maddr:port[,localaddr=addr]]\n
connect the vlan 'n' to multicast maddr and port\n
use 'localaddr=addr' to specify the
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On Mon, Nov 7, 2011 at 2:47 PM, Ted Ts'o ty...@mit.edu wrote: I don't think perf should be used as a precendent that now argues that any new kernel utility should be moved into the kernel sources. Does it make sense to move all of mount, fsck, login, etc., into the kernel sources? There are far more kernel tools outside of the kernel sources than inside the kernel sources. You seem to think that the KVM tool was developed in isolation and we simply copied the code to tools/kvm for the pull request. That's simply not true. We've done a lot of work to make the code feel like kernel code from locking primitive APIs to serial console emulation register names. We really consider KVM tool to be a new Linux subsystem. It's the long lost cousin or bastard child of KVM, depending on who you ask. I don't know if it makes sense to merge the tools you've mentioned above. My gut feeling is that it's probably not reasonable - there's already a community working on it with their own development process and coding style. I don't think there's a simple answer to this but I don't agree with your rather extreme position that all userspace tools should be kept out of the kernel tree. Pekka
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
On 11/07/2011 05:57 AM, Ingo Molnar wrote: * Pekka Enbergpenb...@cs.helsinki.fi wrote: On Mon, 7 Nov 2011, Gerd Hoffmann wrote: It's not just about code, it's as much about culture and development process. Indeed. The BSDs have both kernel and the base system in a single repository. There are probably good reasons for (and against) it. In Linux we don't have that culture. No tool (except perf) lives in the kernel repo. I fail to see why kvm-tool is that much different from udev, util-linux, iproute, filesystem tools, that it should be included. You seem to think perf is an exception - I think it's going to be the future norm for userspace components that are very close to the kernel. That's in fact what Ingo was arguing for when he suggested QEMU to be merged to the kernel tree. Yep, and the answer i got from the Qemu folks when i suggested that merge was a polite buzz off, along the lines of: We don't want to do that, but feel free to write your own tool, leave Qemu alone. At least it was polite :-) Now that people have done exactly that some Qemu folks not only have changed their objection from write your own tool to erm, write your own tool but do it the way *we* prefer you to do it - they also started contributing *against* the KVM tool with predictable, once every 3 months objections against its upstream merge... That's not very nice and not very constructive. I think it's fair to have an objection to upstream merge but I think these threads are not terribly constructive right now as it's just rehashing the same arguments. I've been thinking about the idea of merging more userspace tools into the kernel. I understand the basic reasoning. The kernel has a strong, established development process. It has good infrastructure and a robust hierarchy of maintainers. Good infrastructure can make a big difference to the success of a project. Expanding the kernel infrastructure to more projects does seem like an obvious thing to do when you think about it in that way. The approach other projects have taken to this is to form a formal incubator. Apache is a good example of this. There are clear (written) rules about what it takes for a project to join. Once a project joins, there's a clear governance structure. The project gets to consume all of the Apache infrastructure resources. Other foundations have a release cadence to ensure that multiple components form a cohesive individual release (oVirt). I think you are trying to do this in a more organic way by just merging things into the main git tree. Have you thought about creating a more formal kernel incubator program? Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 6/8] block: add eject request callback
I apologize for the lateness of this review.
Paolo Bonzini pbonz...@redhat.com writes:
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
The commit message should explain why we need this callback. The cover
letter says support for eject requests is required by udev 173.
Please elaborate on that.
---
block.c|7 +++
block.h|7 +++
blockdev.c |8 +---
3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 9873b57..53e21ba 100644
--- a/block.c
+++ b/block.c
@@ -821,6 +821,13 @@ bool bdrv_dev_has_removable_media(BlockDriverState *bs)
return !bs-dev || (bs-dev_ops bs-dev_ops-change_media_cb);
}
+void bdrv_dev_eject_request(BlockDriverState *bs, bool force)
+{
+if (bs-dev_ops bs-dev_ops-eject_request_cb) {
+bs-dev_ops-eject_request_cb(bs-dev_opaque, force);
+}
+}
+
bool bdrv_dev_is_tray_open(BlockDriverState *bs)
{
if (bs-dev_ops bs-dev_ops-is_tray_open) {
diff --git a/block.h b/block.h
index e77988e..d3c3d62 100644
--- a/block.h
+++ b/block.h
@@ -39,6 +39,12 @@ typedef struct BlockDevOps {
*/
void (*change_media_cb)(void *opaque, bool load);
/*
+ * Runs when an eject request is issued from the monitor, the tray
+ * is closed, and the medium is locked.
+ * Device models with removable media must implement this callback.
+ */
+void (*eject_request_cb)(void *opaque, bool force);
+/*
You implement it for IDE in PATCH 7/8 and SCSI in PATCH 8/8. You don't
implement it for floppy, despite the comment. That's okay; floppy has
no use for it. It's the comment that needs fixing. Devices that
implement is_medium_locked() must implement this one as well.
* Is the virtual tray open?
* Device models implement this only when the device has a tray.
*/
@@ -116,6 +122,7 @@ void bdrv_detach_dev(BlockDriverState *bs, void *dev);
void *bdrv_get_attached_dev(BlockDriverState *bs);
void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
void *opaque);
+void bdrv_dev_eject_request(BlockDriverState *bs, bool force);
bool bdrv_dev_has_removable_media(BlockDriverState *bs);
bool bdrv_dev_is_tray_open(BlockDriverState *bs);
bool bdrv_dev_is_medium_locked(BlockDriverState *bs);
diff --git a/blockdev.c b/blockdev.c
index 0827bf7..4cf333a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -635,9 +635,11 @@ static int eject_device(Monitor *mon, BlockDriverState
*bs, int force)
qerror_report(QERR_DEVICE_NOT_REMOVABLE, bdrv_get_device_name(bs));
return -1;
}
-if (!force !bdrv_dev_is_tray_open(bs)
- bdrv_dev_is_medium_locked(bs)) {
-qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
+if (bdrv_dev_is_medium_locked(bs) !bdrv_dev_is_tray_open(bs)) {
+bdrv_dev_eject_request(bs, force);
+if (!force) {
+qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
+}
return -1;
}
bdrv_close(bs);
Like Kevin, I'm not entirely comfortable with changing the meaning of
-f.
Here's my mental model of monitor command eject:
1. eject without -f behaves like the physical tray button. It has
immediate effect, unless the tray is locked closed. Then, the drive
just notifies the OS of the button push, so the OS can react to it. The
latter isn't implemented in QEMU.
2. eject with -f behaves like whatever physical way there is to pry the
tray open, locked or not. CD-ROM drives commonly have a little button
hidden in some hope you can reach with a bent paperclip.
Could you explain your mental model?
[Qemu-devel] [PATCH] hw/omap_gpio: Fix infinite recursion when doing 8/16 bit reads
Fix a long-standing bug which meant that any attempt to do an
8 or 16 bit read from the OMAP GPIO module would cause qemu to
crash due to an infinite recursion.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
This has actually been in the code since the original OMAP2 support
was added in 2008; we've never noticed before because the kernel happened
to always do 32 bit accesses...
Long term we should fix this by conversion to MemoryRegion; this is
the minimally invasive fix for 1.0.
hw/omap_gpio.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/hw/omap_gpio.c b/hw/omap_gpio.c
index d775df6..d630748 100644
--- a/hw/omap_gpio.c
+++ b/hw/omap_gpio.c
@@ -510,7 +510,7 @@ static void omap2_gpio_module_write(void *opaque,
target_phys_addr_t addr,
static uint32_t omap2_gpio_module_readp(void *opaque, target_phys_addr_t addr)
{
-return omap2_gpio_module_readp(opaque, addr) ((addr 3) 3);
+return omap2_gpio_module_read(opaque, addr ~3) ((addr 3) 3);
}
static void omap2_gpio_module_writep(void *opaque, target_phys_addr_t addr,
--
1.7.4.1
Re: [Qemu-devel] Summary of CD, DVD passthrough tests with -drive if=scsi
Thomas Schmitt scdbac...@gmx.net writes: Hi, since i had to give up the plan to test BD burning, i now post the first summary of passthrough testing on base of if=scsi . [Awesome stuff snipped...] Thanks a lot! We could use more tests like that. Has anyone thought of keeping your tests around for regression testing purposes?
Re: [Qemu-devel] [PATCH 6/8] block: add eject request callback
On 11/07/2011 02:21 PM, Markus Armbruster wrote: The commit message should explain why we need this callback. The cover letter says support for eject requests is required by udev 173. Please elaborate on that. Well, first and foremost eject requests are in the spec. :) The fact that recent guests need it is more a justification for pulling this in 1.0. You implement it for IDE in PATCH 7/8 and SCSI in PATCH 8/8. You don't implement it for floppy, despite the comment. That's okay; floppy has no use for it. It's the comment that needs fixing. Devices that implement is_medium_locked() must implement this one as well. Right. 1. eject without -f behaves like the physical tray button. It has immediate effect, unless the tray is locked closed. Then, the drive just notifies the OS of the button push, so the OS can react to it. The latter isn't implemented in QEMU. 2. eject with -f behaves like whatever physical way there is to pry the tray open, locked or not. CD-ROM drives commonly have a little button hidden in some hope you can reach with a bent paperclip. Could you explain your mental model? 1. eject without -f is as you mentioned. 2. eject with -f should really never be needed, but it does whatever is needed to be able to follow up with a change command. It turns out it is really unlock and ask the guest to eject combined, but that's the implementation, not the model. The difference from the paperclip model is that it gives a chance for the OS to clean up and eject safely. It wouldn't be hard to convince me otherwise though, especially if it can help getting the patch in 1.0. The eject -f+change can be replaced by eject, possibly followed by eject -f after a timeout, and then followed again by change. Paolo
Re: [Qemu-devel] [PATCH 6/8] block: add eject request callback
Am 07.11.2011 14:36, schrieb Paolo Bonzini: On 11/07/2011 02:21 PM, Markus Armbruster wrote: The commit message should explain why we need this callback. The cover letter says support for eject requests is required by udev 173. Please elaborate on that. Well, first and foremost eject requests are in the spec. :) The fact that recent guests need it is more a justification for pulling this in 1.0. You implement it for IDE in PATCH 7/8 and SCSI in PATCH 8/8. You don't implement it for floppy, despite the comment. That's okay; floppy has no use for it. It's the comment that needs fixing. Devices that implement is_medium_locked() must implement this one as well. Right. 1. eject without -f behaves like the physical tray button. It has immediate effect, unless the tray is locked closed. Then, the drive just notifies the OS of the button push, so the OS can react to it. The latter isn't implemented in QEMU. 2. eject with -f behaves like whatever physical way there is to pry the tray open, locked or not. CD-ROM drives commonly have a little button hidden in some hope you can reach with a bent paperclip. Could you explain your mental model? 1. eject without -f is as you mentioned. 2. eject with -f should really never be needed, but it does whatever is needed to be able to follow up with a change command. It turns out it is really unlock and ask the guest to eject combined, but that's the implementation, not the model. Does this give different results than just asking the guest to eject without forcefully unlocking? I would expect that a guest that responds to the eject request would also unlock the drive. In which case I think eject without -f should be enough? The difference from the paperclip model is that it gives a chance for the OS to clean up and eject safely. It wouldn't be hard to convince me otherwise though, especially if it can help getting the patch in 1.0. The eject -f+change can be replaced by eject, possibly followed by eject -f after a timeout, and then followed again by change. Kevin
Re: [Qemu-devel] GSoC mentor summit QEMU users session
On 07/11/2011 12:50, Lluís Vilanova wrote: Fabien Chouteau writes: On 04/11/2011 19:45, Lluís Vilanova wrote: I've only had a brief look into the changes, but I think the mechanism I implemented has a cleaner fit into QEMU, thanks to previous feedback from this list. I don't know about your implementation, can you give more information? What type of analysis is supported (object, statement, decision, MC/DC)? Which language? And maybe you can post a link to your repository. I'll be posting the patches once QEMU opens up for new commits other than release stabilization. If this is too far away in time, please tell me. It explicitly separates the tracing mechanism (in QEMU itself) from the specific trace analysis (which resides in a separate library specified by the user at compile time, where most of couverture would go). As I understand everything is compiled within Qemu, right? GNATcoverage seems even more separate. We use Qemu to generate an execution trace file and the coverage analysis tool is a totally separate program. You can add support for another language or implement your own coverage tool without recompiling (redistribute) Qemu. The process is basically: * Add trace events that can work during TCG code generation (e.g., start TB, start instruction fetch, memory access, etc.) * Let the user select which trace events to instrument, including both regular trace events and TCG trace events (thus you instrument at both execution and translation time). * The user provides her own implementation of the instrumented trace events. As you can see, this system only gives you the hooks were code can be inserted. Whether your hooks implement everything inside QEMU or just write a trace file, that is up to you. Interesting, what kind of analysis do you plan to perform with this? [...] On the other hand, I have a complementary set of events, so we can definitely join the efforts on that side (e.g., I haven't yet went into the trouble of adding the begin/end TB or branch events). I don't know what do you mean by events, but we sure can join efforts on coverage with Qemu. Well, my target is not code coverage, but generating events that can be used for architecture simulation. In any case, there will surely be trace events that we're both interested in (e.g., TB start and branch). OK I thought you were talking about coverage. I'm not sure if and how we can implement coverage using your events but for the moment both features can cohabit. -- Fabien Chouteau
[Qemu-devel] Guest hangs on reboot with Windows Server 2003
For QEMU 1.0 Test Day I'm hitting a guest hang with Windows Server 2003 whenever it tries to reboot. The QEMU monitor is available and working. I'm using a qcow2 image over IDE, but this may not be qcow2/block related. This is a regression and does not occur under Debian 0.14.1+dfsg-3. I am bisecting against v0.15.0 now and will report back. Just wanted to let you know so we don't duplicate work. Stefan
Re: [Qemu-devel] [PATCH 6/8] block: add eject request callback
On 11/07/2011 02:49 PM, Kevin Wolf wrote: 2. eject with -f should really never be needed, but it does whatever is needed to be able to follow up with a change command. It turns out it is really unlock and ask the guest to eject combined, but that's the implementation, not the model. Does this give different results than just asking the guest to eject without forcefully unlocking? I would expect that a guest that responds to the eject request would also unlock the drive. In which case I think eject without -f should be enough? Only if the guest is not buggy (e.g. locks the tray but stops polling for eject requests) and has not crashed. Paolo
Re: [Qemu-devel] [PATCH 6/8] block: add eject request callback
Am 07.11.2011 14:56, schrieb Paolo Bonzini: On 11/07/2011 02:49 PM, Kevin Wolf wrote: 2. eject with -f should really never be needed, but it does whatever is needed to be able to follow up with a change command. It turns out it is really unlock and ask the guest to eject combined, but that's the implementation, not the model. Does this give different results than just asking the guest to eject without forcefully unlocking? I would expect that a guest that responds to the eject request would also unlock the drive. In which case I think eject without -f should be enough? Only if the guest is not buggy (e.g. locks the tray but stops polling for eject requests) and has not crashed. If the guest is broken, forcefully unlocking and doing an eject request won't help either (the drive will be unlocked, but not ejected) and the only way to do anything useful with the drive is sending another eject command. This doesn't help with cleanly unmounting the medium, of course. So I think this is actually a point for the eject -f = paperclip model. Kevin
Re: [Qemu-devel] GSoC mentor summit QEMU users session
Fabien Chouteau writes: The process is basically: * Add trace events that can work during TCG code generation (e.g., start TB, start instruction fetch, memory access, etc.) * Let the user select which trace events to instrument, including both regular trace events and TCG trace events (thus you instrument at both execution and translation time). * The user provides her own implementation of the instrumented trace events. As you can see, this system only gives you the hooks were code can be inserted. Whether your hooks implement everything inside QEMU or just write a trace file, that is up to you. Interesting, what kind of analysis do you plan to perform with this? Deep full-system or application behaviour analysis, which in my case happens to be architecture simulation but it could well be data flow tracking, reverse engineering or anything else. All users care is about having the right hooks available and being able to plug in arbitrary code in them. [...] On the other hand, I have a complementary set of events, so we can definitely join the efforts on that side (e.g., I haven't yet went into the trouble of adding the begin/end TB or branch events). I don't know what do you mean by events, but we sure can join efforts on coverage with Qemu. Well, my target is not code coverage, but generating events that can be used for architecture simulation. In any case, there will surely be trace events that we're both interested in (e.g., TB start and branch). OK I thought you were talking about coverage. I'm not sure if and how we can implement coverage using your events but for the moment both features can cohabit. You would just plug in your code in the guest branch instruction and TB begin/end hooks. 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 1.0] ac97: don't override the pci subsystem id
On 11/07/2011 01:00 PM, Gerd Hoffmann wrote: This patch removes the code lines which set the subsystem id for the emulated ac97 card to 8086:. Due to the device id being zero the subsystem id isn't vaild anyway. With the patch applied the sound card gets the default qemu subsystem id (1af4:1100) instead. Cc: Takashi Iwai ti...@suse.de Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/ac97.c |6 -- 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/hw/ac97.c b/hw/ac97.c index 6800af4..b43e435 100644 --- a/hw/ac97.c +++ b/hw/ac97.c @@ -1305,12 +1305,6 @@ static int ac97_initfn (PCIDevice *dev) c[PCI_BASE_ADDRESS_0 + 6] = 0x00; c[PCI_BASE_ADDRESS_0 + 7] = 0x00; -c[PCI_SUBSYSTEM_VENDOR_ID] = 0x86; /* svid subsystem vendor id rwo */ -c[PCI_SUBSYSTEM_VENDOR_ID + 1] = 0x80; - -c[PCI_SUBSYSTEM_ID] = 0x00; /* sid subsystem id rwo */ -c[PCI_SUBSYSTEM_ID + 1] = 0x00; - This a guest ABI change. Do we want -M support for it? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Summary of CD, DVD passthrough tests with -drive if=scsi
Hi, Markus Armbruster wrote: Has anyone thought of keeping your tests around for regression testing purposes? The full tour lasts a whole workday and needs human attention. I still did not come to test the newest proposals of Paolo Bonzini and Zhi Yong Wu, because the last BD-R test was finished just five minutes ago. (I'm not working full-time on it but i have to attend it every half hour or so.) Now i do a last verification read run, to make sure that all went well. It comes to me that i forgot to test DVD-RAM. Thanks heaven that HD DVD-R never made it to the real-life market. For unattended tests i would propose to develop some dry exercises, which perform SCSI commands of all three payload direction classes: from_drive, to_drive, no_payload. The test machine would need a real burner drive with e.g. a blank CD-RW loaded, so that e.g. mode pages can be sent and read. Depending on the individual drive, it might even work with an empty drive. Sending a mode page 5 for write preparation is supposed to be harmless. One could announce to the drive a CD SAO run, read it back, announce a TAO run, read it back, and then just close the file descriptor to the drive. One could try to read from empty medium or drive and watch out for the sense code that should come as protest. This would not detect failures like of if=scsi with FORMAT UNIT or SET STREAMING. But those would need real burn runs to show up. For the next round of burn tests i will make a shorter cross section through the media zoo, with re-usable media only. (The BD-R contains a backup of about all interesting files of the host system and of the guest. It is now closed. 7738 MiB payload, 15 GiB wasted.) I will record the options of all xorriso test runs. But before this can be reproduced by other testers i will have to work on libburn, so that one does not need an udev hack to get the /dev/vda drive accepted as CD device. The next release will hopefully happen in a few weeks and be able to work in the guest system out of the box. GNU/Linux, FreeBSD, Solaris are supported on real systems, currently. (Can FreeBSD and Solaris use virtio drives ?) I am open to proposals of other systems which can make use of the qemu device setup, that works for Linux guests. Mainly i would need info about possible drive addresses and how to perform SCSI transactions from userspace. And help with installing the guest system, of course. Have a nice day :) Thomas
Re: [Qemu-devel] [PATCH 1.0] ac97: don't override the pci subsystem id
On 11/07/11 15:17, Avi Kivity wrote: On 11/07/2011 01:00 PM, Gerd Hoffmann wrote: This patch removes the code lines which set the subsystem id for the emulated ac97 card to 8086:. Due to the device id being zero the subsystem id isn't vaild anyway. With the patch applied the sound card gets the default qemu subsystem id (1af4:1100) instead. This a guest ABI change. Do we want -M support for it? Given that the old subsystem id isn't valid I'd say no unless someone comes up with a good reason. cheers, Gerd
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels
I know kgdb can test kernel,but I haven't succeed . -- Original -- From: Pekka Enberg; Date: 2011年11月7日(星期一) 下午4:57 To: Paolo Bonzini; Cc: Alexander Graf; k...@vger.kernel.org list; qemu-devel Developers; linux-ker...@vger.kernel.org List; Blue Swirl; Avi Kivity; Américo Wan; Ingo Molnar; Linus Torvalds; Subject: Re: [PATCH] KVM: Add wrapper script around QEMU to test kernels On 11/07/2011 09:45 AM, Pekka Enberg wrote: Specifications matter much more than working code. Quirks are a fact of life but should always come second. To quote Linus: And I have seen _lots_ of total crap work that was based on specs. It's _the_ single worst way to write software, because it by definition means that the software was written to match theory, not reality. On Mon, Nov 7, 2011 at 10:52 AM, Paolo Bonzini pbonz...@redhat.com wrote: All generalizations are false. What is that supposed to mean? You claimed we're doing it wrong and I explained you why we are doing it the way we are. Really, the way we do things in the KVM tool is not a bug, it's a feature. Pekka -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Qemu-devel] Summary of CD, DVD passthrough tests with -drive if=scsi
On 11/07/2011 03:31 PM, Thomas Schmitt wrote: The next release will hopefully happen in a few weeks and be able to work in the guest system out of the box. GNU/Linux, FreeBSD, Solaris are supported on real systems, currently. (Can FreeBSD and Solaris use virtio drives ?) Perhaps, but probably not with SG_IO so that's a no for your use case. Paolo
[Qemu-devel] [PATCH 1.0] Replace WriteFileEx with WriteFile in qemu_create_pidfile
The function that writes pidfile for win32 uses WriteFileEx which is an
asynchronous IO function. The arguments given to WriteFileEx are allocated on
the stack and one of them is in out. When the IO operation is actually
executed the calling function has already returned, so the arguments are no
longer allocated or allocated to another frame.
Signed-off-by: Fabien Chouteau chout...@adacore.com
---
os-win32.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/os-win32.c b/os-win32.c
index 7909401..8ad5fa1 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -130,14 +130,15 @@ int qemu_create_pidfile(const char *filename)
memset(overlap, 0, sizeof(overlap));
file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL,
- OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+ OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
if (file == INVALID_HANDLE_VALUE) {
return -1;
}
len = snprintf(buffer, sizeof(buffer), %d\n, getpid());
-ret = WriteFileEx(file, (LPCVOID)buffer, (DWORD)len,
- overlap, NULL);
+ret = WriteFile(file, (LPCVOID)buffer, (DWORD)len,
+NULL, overlap);
+CloseHandle(file);
if (ret == 0) {
return -1;
}
--
1.7.5.1
Re: [Qemu-devel] [RFC 4/6] block: request overlap detection
On Mon, Nov 7, 2011 at 11:49 AM, Zhi Yong Wu zwu.ker...@gmail.com wrote:
On Mon, Oct 17, 2011 at 11:47 PM, Stefan Hajnoczi
stefa...@linux.vnet.ibm.com wrote:
Detect overlapping requests and remember to align to cluster boundaries
if the image format uses them. This assumes that allocating I/O is
performed in cluster granularity - which is true for qcow2, qed, etc.
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
block.c | 39 +--
1 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index cc3202c..0c22741 100644
--- a/block.c
+++ b/block.c
@@ -1052,21 +1052,56 @@ static BdrvTrackedRequest
*tracked_request_add(BlockDriverState *bs,
return req;
}
+/**
+ * Round a region to cluster boundaries
+ */
+static void round_to_clusters(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors,
+ int64_t *cluster_sector_num,
+ int *cluster_nb_sectors)
+{
+ BlockDriverInfo bdi;
+
+ if (bdrv_get_info(bs, bdi) 0 || bdi.cluster_size == 0) {
+ *cluster_sector_num = sector_num;
+ *cluster_nb_sectors = nb_sectors;
+ } else {
+ int64_t c = bdi.cluster_size / BDRV_SECTOR_SIZE;
+ *cluster_sector_num = (sector_num / c) * c;
I can understand the above formula, but the one below is
very magic. :) and can not be understood by me.
+ *cluster_nb_sectors = ((sector_num % c) + nb_sectors + c - 1) / c *
c;
I agree this is ugly. Here is what is going on:
c = number of sectors per cluster
cluster_sector_num = sector number rounded *down* to cluster boundary
cluster_nb_sectors = number of sectors from cluster_sector_num to
rounded up sector_num+nb_sectors
So the magic expression is takes the original sector_num to
sector_num+nb_sectors region:
|---XXX|XXX---|
Where |-| is a cluster and is the region from sector_num to
sector_num+nb_sectors, then the output should be:
|RR|RR|
Everything has been rounded to clusters. So here is the expression broken down:
*cluster_nb_sectors = ((sector_num % c) + nb_sectors + c - 1) / c * c;
AAXX BB
|AAAXXX|XXXBBB|
A is actually equivalent to sector_num - cluster_sector_num.
X is the original unrounded region.
B is the rounding up to the next cluster bounary.
Another way of writing this:
*cluster_nb_sectors = ROUND_UP((sector_num - cluster_sector_num) +
nb_sectors, c);
I'll try to improve the code in the next revision.
Stefan
[Qemu-devel] Summary of Anthony's 'next' queue
Hi, This is a summary of the patches that I have queued in my next tree that were identified as 1.1 candidates. These patches will not be applied until after the 1.1 tree opens (December 1st). These patches have not been tested yet and may receive additional review comments. This note is meant to let submitters know that their patch has not been forgotten. Here's the full list: 000 pbonz...@redhat.com Mark future contributions to GPLv2-only files as GPLv2+ 001 cor...@linux.vnet.ibm.com Add basic version of bridge helper 002 cor...@linux.vnet.ibm.com Add access control support to qemu bridge helper 003 cor...@linux.vnet.ibm.com Add cap reduction support to enable use as SUID 004 cor...@linux.vnet.ibm.com Add support for net bridge 005 pingf...@linux.vnet.ibm.com 01/01 ACPI: Call ACPI remove handler when handling ACPI eject event 006 pingf...@linux.vnet.ibm.com Introduce a new bus ICC to connect APIC 007 smizr...@redhat.com 01/02 Better support for distros using /lib64 directories 008 smizr...@redhat.com 02/02 Added target to build libvdisk 009 jordan.l.jus...@intel.com pflash: Support read-only mode 010 jordan.l.jus...@intel.com pc: Support system flash memory with pflash 011 hpous...@reactos.org 01/11 isa: give ISABus/ISADevice to isa_create(), isa_bus_irqs() and isa_get_irq() functions 012 hpous...@reactos.org 02/11 isa: move ISABus structure definition to header file 013 hpous...@reactos.org 03/11 i8259: give ISA device to isa_register_ioport() 014 hpous...@reactos.org 04/11 pc: give ISA bus to ISA methods 015 hpous...@reactos.org 05/11 alpha: give ISA bus to ISA methods 016 hpous...@reactos.org 06/11 sun4u: give ISA bus to ISA methods 017 hpous...@reactos.org 07/11 fulong2e: give ISA bus to ISA methods 018 hpous...@reactos.org 08/11 malta: give ISA bus to ISA methods 019 hpous...@reactos.org 09/11 isa: always use provided ISA bus when creating an isa device 020 hpous...@reactos.org 10/11 isa: always use provided ISA bus in isa_bus_irqs() 021 hpous...@reactos.org 11/11 audio: remove unused parameter isa_pic 022 hpous...@reactos.org 01/03 sonic: fix typo 023 hpous...@reactos.org 02/03 sonic: fix netcard reset 024 hpous...@reactos.org 03/03 sonic: reset all bits of in_use field when required 025 n...@us.ibm.com monitor: add ability to dump SLB entries 026 mlspira...@gmail.com Support for UDP unicast network backend 027 mlspira...@gmail.com Re: Support for UDP unicast network backend 028 mlspira...@gmail.com Re: Support for UDP unicast network backend 029 stef...@linux.vnet.ibm.com Support for TPM command line options 030 stef...@linux.vnet.ibm.com Add TPM (frontend) hardware interface (TPM TIS) to Qemu 031 stef...@linux.vnet.ibm.com Add a debug register 032 stef...@linux.vnet.ibm.com Build the TPM frontend code 033 stef...@linux.vnet.ibm.com Add a TPM Passthrough backend driver implementation 034 stef...@linux.vnet.ibm.com Introduce --enable-tpm-passthrough configure option 035 stef...@linux.vnet.ibm.com Move parsing of filedescriptor into common function 036 stef...@linux.vnet.ibm.com Add fd parameter for TPM passthrough driver 037 mdr...@linux.vnet.ibm.com qapi: add Visitor interfaces for uint*_t and int*_t 038 mdr...@linux.vnet.ibm.com qapi: add QemuFileOutputVisitor 039 mdr...@linux.vnet.ibm.com qapi: add QemuFileInputVisitor 040 mdr...@linux.vnet.ibm.com savevm: move QEMUFile interfaces into qemu-file.c 041 mdr...@linux.vnet.ibm.com qapi: test cases for QEMUFile input/output visitors 042 mdr...@linux.vnet.ibm.com qemu-file: add QEMUFile-visitor lookup routines 043 mdr...@linux.vnet.ibm.com trace: qemu_(put|get)_(byte|buffer) events 044 mdr...@linux.vnet.ibm.com trace: add trace statements for visitor interface 045 mdr...@linux.vnet.ibm.com qapi: add trace statements to qapi-visit-core.c 046 mdr...@linux.vnet.ibm.com vmstate: use visitors 047 mdr...@linux.vnet.ibm.com 01/13 slirp: convert save/load function to visitor interface 048 mdr...@linux.vnet.ibm.com 02/13 ivshmem: convert save/load to visitor 049 mdr...@linux.vnet.ibm.com 03/13 virtio-pci: convert save/load to visitors 050 mdr...@linux.vnet.ibm.com 04/13 msix: convert save/load to visitors (including interfaces) 051 mdr...@linux.vnet.ibm.com 05/13 openpic: convert save/load to visitors 052 mdr...@linux.vnet.ibm.com 06/13 i440fx: convert save/load to visitors 053 mdr...@linux.vnet.ibm.com 07/13 pci: convert pci_device_(save|load) interfaces to accept Visitors 054 mdr...@linux.vnet.ibm.com 08/13 virtio: convert common virtio save/load to visitors 055 mdr...@linux.vnet.ibm.com 09/13 virtio-balloon: convert save/load to visitors 056 mdr...@linux.vnet.ibm.com 10/13 virtio-blk: convert save/load to visitors 057 mdr...@linux.vnet.ibm.com 11/13 virtio-net: convert save/load to visitors 058 mdr...@linux.vnet.ibm.com 12/13 virtio-serial: convert save/load to visitors 059 mdr...@linux.vnet.ibm.com 13/13 virtio: convert virtio_save/virtio_load interfaces to
Re: [Qemu-devel] [PATCH 1.0] ac97: don't override the pci subsystem id
On 11/07/2011 08:33 AM, Gerd Hoffmann wrote: On 11/07/11 15:17, Avi Kivity wrote: On 11/07/2011 01:00 PM, Gerd Hoffmann wrote: This patch removes the code lines which set the subsystem id for the emulated ac97 card to 8086:. Due to the device id being zero the subsystem id isn't vaild anyway. With the patch applied the sound card gets the default qemu subsystem id (1af4:1100) instead. This a guest ABI change. Do we want -M support for it? Given that the old subsystem id isn't valid I'd say no unless someone comes up with a good reason. Agreed. Regards, Anthony Liguori cheers, Gerd
Re: [Qemu-devel] [PATCH 1.0] Replace WriteFileEx with WriteFile in qemu_create_pidfile
On 11/07/2011 03:36 PM, Fabien Chouteau wrote:
The function that writes pidfile for win32 uses WriteFileEx which is an
asynchronous IO function. The arguments given to WriteFileEx are allocated on
the stack and one of them is in out. When the IO operation is actually
executed the calling function has already returned, so the arguments are no
longer allocated or allocated to another frame.
Signed-off-by: Fabien Chouteauchout...@adacore.com
---
os-win32.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/os-win32.c b/os-win32.c
index 7909401..8ad5fa1 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -130,14 +130,15 @@ int qemu_create_pidfile(const char *filename)
memset(overlap, 0, sizeof(overlap));
file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL,
- OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+ OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
This is s/tab/space/, unrelated but fine.
if (file == INVALID_HANDLE_VALUE) {
return -1;
}
len = snprintf(buffer, sizeof(buffer), %d\n, getpid());
-ret = WriteFileEx(file, (LPCVOID)buffer, (DWORD)len,
- overlap, NULL);
+ret = WriteFile(file, (LPCVOID)buffer, (DWORD)len,
+NULL,overlap);
+CloseHandle(file);
if (ret == 0) {
return -1;
}
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Paolo
Re: [Qemu-devel] [PATCH 1.0] ac97: don't override the pci subsystem id
On 11/07/2011 04:33 PM, Gerd Hoffmann wrote: On 11/07/11 15:17, Avi Kivity wrote: On 11/07/2011 01:00 PM, Gerd Hoffmann wrote: This patch removes the code lines which set the subsystem id for the emulated ac97 card to 8086:. Due to the device id being zero the subsystem id isn't vaild anyway. With the patch applied the sound card gets the default qemu subsystem id (1af4:1100) instead. This a guest ABI change. Do we want -M support for it? Given that the old subsystem id isn't valid I'd say no unless someone comes up with a good reason. Do we know that Windows won't complain about it? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 1.0] ac97: don't override the pci subsystem id
On 11/07/2011 08:42 AM, Avi Kivity wrote: On 11/07/2011 04:33 PM, Gerd Hoffmann wrote: On 11/07/11 15:17, Avi Kivity wrote: On 11/07/2011 01:00 PM, Gerd Hoffmann wrote: This patch removes the code lines which set the subsystem id for the emulated ac97 card to 8086:. Due to the device id being zero the subsystem id isn't vaild anyway. With the patch applied the sound card gets the default qemu subsystem id (1af4:1100) instead. This a guest ABI change. Do we want -M support for it? Given that the old subsystem id isn't valid I'd say no unless someone comes up with a good reason. Do we know that Windows won't complain about it? I thought the original motivation for the default subsystem ids was that some Windows test suite was explicitly complaining about having invalid subsystem ids? Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 1.0] ac97: don't override the pci subsystem id
On 11/07/2011 04:44 PM, Anthony Liguori wrote: This a guest ABI change. Do we want -M support for it? Given that the old subsystem id isn't valid I'd say no unless someone comes up with a good reason. Do we know that Windows won't complain about it? I thought the original motivation for the default subsystem ids was that some Windows test suite was explicitly complaining about having invalid subsystem ids? I think so, but that's unrelated. The worry is that some DRM code checksums your hardware and complains if it changed too much. Nothing to do with the test suite. The sense of Gerd's comment is reversed. We should preserve the ABI unless there is a strong reason not to. -- error compiling committee.c: too many arguments to function
