Re: [Qemu-devel] [PATCH 1/2] ioport: add function to check whenever a port is assigned or not
Gerd Hoffmann writes:
> On 05/24/10 14:32, Paul Brook wrote:
>>> +int is_ioport_assigned(pio_addr_t addr)
>>
>> Shouldn't we move this into register_ioport_{read,write}, and have that fail
>> if the port has already been assigned?
>
> It already checks and fails with hw_error(). Problem with that is
> that this kills qemu in case you try to pci hot-plug a vga card. So
> I've added a way to check before-hand, so we can fail gracefully in
> the few places where we need it (see second patch of the series).
If we needed in more than a few places, then a solution like the one we
adopted for qdev_init() could make sense: have register_ioport_FOO()
return an error, convert all users that don't want to check it to
register_ioport_FOO_nofail(), which hw_error()s out.
Re: [Qemu-devel] Re: [RFT][PATCH 07/15] qemu_irq: Add IRQ handlers with delivery feedback
On Tue, May 25, 2010 at 11:44:50PM +0200, Jan Kiszka wrote: > > > >> I think the real solution to coalescing is put the logic inside one > >> device, in this case APIC because it has the information about irq > >> delivery. APIC could monitor incoming RTC irqs for frequency > >> information and whether they get delivered or not. If not, an internal > >> timer is installed which injects the lost irqs. > > That won't fly as the IRQs will already arrive at the APIC with a > sufficiently high jitter. At the bare minimum, you need to tell the > interrupt controller about the fact that a particular IRQ should be > delivered at a specific regular rate. For this, you also need a generic > interface - nothing really "won". > Not only that. Suppose RTC runs with 64Hz frequency and N interrupts were coalesced during some period. Now the guest reprograms RTC to 1024Hz. N should be scaled accordingly otherwise reinjection will not fix the drift. Do you propose to put this logic into APIC to? -- Gleb.
Re: [Qemu-devel] [PATCH 0/5] vnc: desktop size patches.
On Tue, May 25, 2010 at 06:25:15PM +0200, Gerd Hoffmann wrote: > Hi, > > This series brings a bunch of vnc desktop size patches, fixing the > issues discussed in the "Possible race condition in VNC display > resizing" thread. Check list archive here: > > http://lists.gnu.org/archive/html/qemu-devel/2010-04/msg01778.html > > cheers, > Gerd > > Gerd Hoffmann (5): > vnc: factor out vnc_desktop_resize() > vnc: send desktopresize event as reply to set encodings > vnc: keep track of client desktop size > vnc: don't send invalid screen updates. > vnc: move size-changed check into the vnc_desktop_resize function. > > vnc.c | 50 +- > vnc.h |2 ++ > 2 files changed, 35 insertions(+), 17 deletions(-) ACK, these patches look good for solving both of the problems I encountered (the resize race + the out-of-bounds updates) in that quoted thread. Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
[Qemu-devel] [PATCH] all vga: refuse hotplugging.
Try to pci hotplug a vga card, watch qemu die with hw_error().
This patch fixes it.
Signed-off-by: Gerd Hoffmann
---
hw/cirrus_vga.c |4
hw/vga-pci.c|4
hw/vmware_vga.c |4
3 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index ba48289..5175725 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -3188,6 +3188,10 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
uint8_t *pci_conf = d->dev.config;
int device_id = CIRRUS_ID_CLGD5446;
+ if (dev->qdev.hotplugged) {
+ return -1;
+ }
+
/* setup VGA */
vga_common_init(&s->vga, VGA_RAM_SIZE);
cirrus_init_common(s, device_id, 1);
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index c8d260c..a2de201 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -79,6 +79,10 @@ static int pci_vga_initfn(PCIDevice *dev)
VGACommonState *s = &d->vga;
uint8_t *pci_conf = d->dev.config;
+ if (dev->qdev.hotplugged) {
+ return -1;
+ }
+
// vga + console init
vga_common_init(s, VGA_RAM_SIZE);
vga_init(s);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 4e7a75d..d12d86f 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1232,6 +1232,10 @@ static int pci_vmsvga_initfn(PCIDevice *dev)
struct pci_vmsvga_state_s *s =
DO_UPCAST(struct pci_vmsvga_state_s, card, dev);
+if (dev->qdev.hotplugged) {
+return -1;
+}
+
pci_config_set_vendor_id(s->card.config, PCI_VENDOR_ID_VMWARE);
pci_config_set_device_id(s->card.config, SVGA_PCI_DEVICE_ID);
s->card.config[PCI_COMMAND]= PCI_COMMAND_IO |
--
1.6.6.1
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
Am 26.05.2010 03:31, schrieb Anthony Liguori: > On 05/25/2010 04:01 PM, Aurelien Jarno wrote: >> >> I really think this patch can be useful, in my own case when testing >> debian-installer (I already cache=writeback). In short all that is about >> developing and testing, as opposed to run a VM in production, can >> benefit about that. This was one of the original use case of QEMU before >> KVM arrived. >> >> Unless someone can convince me not to do it, I seriously considering >> applying this patch. >> > > There really needs to be an indication in the --help output of what the > ramifications of this option are, in the very least. It should also be > removable via a ./configure option because no sane distribution should > enable this for end users. We know better what you stupid user want? There are valid use cases for this cache option, most notably installation. I could agree with requesting that the option should be called cache=unsafe (or even Alex' cache=destroys_your_image), but that should really be enough to tell everyone that his data is not safe with this option. Kevin
[Qemu-devel] Re: [PATCH] Release usb devices on shutdown and usb_del command
On Tue, May 25, 2010 at 10:58:50AM +0200, Gerd Hoffmann wrote: > Date: Tue, 25 May 2010 10:58:50 +0200 > From: Gerd Hoffmann > To: Shahar Havivi > CC: "David S. Ahern" , > Markus Armbruster , qemu-devel@nongnu.org > Subject: Re: [PATCH] Release usb devices on shutdown and usb_del command > > On 05/21/10 19:55, Shahar Havivi wrote: > >Remove usb_host_device_release and using usb_host_close to handle usb_del > >command. > >Gerd, What do you think about the usb_cleanup()? > > We need a mechanism to handle this for sure. I don't like that > usb-specific approach very much though. > > I think we should either do that at qdev level, then at exit walk > the whole device tree and call cleanup functions (if present). So > every device has the chance to do cleanups when needed. > > Or we could have a exit notifier, which can be used for device (and > also other) cleanup work. > > I tend to think that a exit notifier will be better. We probably > have only a few devices which actually have to do some cleanup work > (usb passthrough, maybe pci passthrough too), so building qdev > infrastructure for that feels a bit like overkill. And exit > notifiers are more generic, i.e. it will also work for non-device > stuff. > > cheers, > Gerd > We can add at exit to the first device that added in usb-linux.c via usb_host_device_open() or usb_host_initfn() and the exit will iterate all usb devices in linux only, How is that solution?
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On Tue, May 25, 2010 at 08:31:20PM -0500, Anthony Liguori wrote: > On 05/25/2010 04:01 PM, Aurelien Jarno wrote: > > > >I really think this patch can be useful, in my own case when testing > >debian-installer (I already cache=writeback). In short all that is about > >developing and testing, as opposed to run a VM in production, can > >benefit about that. This was one of the original use case of QEMU before > >KVM arrived. > > > >Unless someone can convince me not to do it, I seriously considering > >applying this patch. > > There really needs to be an indication in the --help output of what > the ramifications of this option are, in the very least. It should That's indeed something than can be done, but to avoid double standards, it should also be done for other features that can lead to data corruption. I am talking for example on the qcow format, which is not really supported anymore. > also be removable via a ./configure option because no sane > distribution should enable this for end users. > I totally disagree. All the examples I have given apply to qemu *users*, not qemu developers. They surely don't want to recompile qemu for their usage. Note also that what is proposed in this patch was the default not too long ago, and that a lot of users complained about the new default for their usage, they see it as a regression. We even had to put a note explaining that in the Debian package to avoid to many bug reports. cache=writeback only answer partially to this use case. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [RFC PATCH 1/1] ceph/rbd block driver for qemu-kvm
On 05/25/2010 05:02 PM, Anthony Liguori wrote: On 05/25/2010 08:57 AM, Avi Kivity wrote: On 05/25/2010 04:54 PM, Anthony Liguori wrote: On 05/25/2010 08:36 AM, Avi Kivity wrote: We'd need a kernel-level generic snapshot API for this eventually. or (2) implement BUSE to complement FUSE and CUSE to enable proper userspace block devices. Likely slow due do lots of copying. Also needs a snapshot API. The kernel could use splice. Still can't make guest memory appear in (A)BUSE process memory without either mmu tricks (vmsplice in reverse) or a copy. May be workable for an (A)BUSE driver that talks over a network, and thus can splice() its way out. splice() actually takes offset parameter so it may be possible to treat that offset parameter as a file offset. That would essentially allow you to implement a splice() based thread pool where splice() replaces preadv/pwritev. Right. (note: need splicev() here) It's not quite linux-aio, but it should take you pretty far. I think the main point is that the problem of allowing block plugins to qemu is the same as block plugins for the kernel. The kernel doesn't provide a stable interface (and we probably can't for the same reasons) and it's generally discourage from a code quality perspective. The kernel does provide a stable interface for FUSE, and it could provide a stable interface for ABUSE. Why can the kernel support these and qemu can't support essentially the same thing? That said, making an external program work well as a block backend is identical to making userspace block devices fast. More or less, yes. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH] add support for protocol driver create_options
Am 26.05.2010 04:35, schrieb MORITA Kazutaka:
> At Tue, 25 May 2010 15:43:17 +0200,
> Kevin Wolf wrote:
>>
>> Am 24.05.2010 08:34, schrieb MORITA Kazutaka:
>>> At Fri, 21 May 2010 18:57:36 +0200,
>>> Kevin Wolf wrote:
Am 20.05.2010 07:36, schrieb MORITA Kazutaka:
> +
> +/*
> + * Append an option list (list) to an option list (dest).
> + *
> + * If dest is NULL, a new copy of list is created.
> + *
> + * Returns a pointer to the first element of dest (or the newly
> allocated copy)
> + */
> +QEMUOptionParameter *append_option_parameters(QEMUOptionParameter *dest,
> +QEMUOptionParameter *list)
> +{
> +size_t num_options, num_dest_options;
> +
> +num_options = count_option_parameters(dest);
> +num_dest_options = num_options;
> +
> +num_options += count_option_parameters(list);
> +
> +dest = qemu_realloc(dest, (num_options + 1) *
> sizeof(QEMUOptionParameter));
> +
> +while (list && list->name) {
> +if (get_option_parameter(dest, list->name) == NULL) {
> +dest[num_dest_options++] = *list;
You need to add a dest[num_dest_options].name = NULL; here. Otherwise
the next loop iteration works on uninitialized memory and possibly an
unterminated list. I got a segfault for that reason.
>>>
>>> I forgot to add it, sorry.
>>> Fixed version is below.
>>>
>>> Thanks,
>>>
>>> Kazutaka
>>>
>>> ==
>>> This patch enables protocol drivers to use their create options which
>>> are not supported by the format. For example, protcol drivers can use
>>> a backing_file option with raw format.
>>>
>>> Signed-off-by: MORITA Kazutaka
>>
>> $ ./qemu-img create -f qcow2 -o cluster_size=4k /tmp/test.qcow2 4G
>> Unknown option 'cluster_size'
>> qemu-img: Invalid options for file format 'qcow2'.
>>
>> I think you added another num_dest_options++ which shouldn't be there.
>>
>
> Sorry again. I wrongly added `dest[num_dest_options++].name = NULL;'
> instead of `dest[num_dest_options].name = NULL;'.
Thanks, now it looks good and passes all qemu-iotests tests again.
Applied the patch to the block branch.
Kevin
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
Am 26.05.2010 10:52, schrieb Aurelien Jarno: > On Tue, May 25, 2010 at 08:31:20PM -0500, Anthony Liguori wrote: >> On 05/25/2010 04:01 PM, Aurelien Jarno wrote: >>> >>> I really think this patch can be useful, in my own case when testing >>> debian-installer (I already cache=writeback). In short all that is about >>> developing and testing, as opposed to run a VM in production, can >>> benefit about that. This was one of the original use case of QEMU before >>> KVM arrived. >>> >>> Unless someone can convince me not to do it, I seriously considering >>> applying this patch. >> >> There really needs to be an indication in the --help output of what >> the ramifications of this option are, in the very least. It should > > That's indeed something than can be done, but to avoid double standards, > it should also be done for other features that can lead to data > corruption. I am talking for example on the qcow format, which is not > really supported anymore. That said, qcow1 is probably in a better state than half of the other drivers. Basically only raw and qcow2 are really maintained, you'd need to warn about any other format then. Kevin
Re: [Qemu-devel] [RFC PATCH 1/1] ceph/rbd block driver for qemu-kvm
On 05/25/2010 08:12 PM, Sage Weil wrote: On Tue, 25 May 2010, Avi Kivity wrote: What's the reason for not having these drivers upstream? Do we gain anything by hiding them from our users and requiring them to install the drivers separately from somewhere else? Six months. FWIW, we (Ceph) aren't complaining about the 6 month lag time (and I don't think the Sheepdog guys are either). In that case (and if there are no other potential users), then there's no need for a plugin API. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v5] KVM: VMX: Enable XSAVE/XRSTORE for guest
From: Dexuan Cui
This patch enable guest to use XSAVE/XRSTORE instructions.
We assume that host_xcr0 would use all possible bits that OS supported.
And we loaded xcr0 in the same way we handled fpu - do it as late as we can.
Signed-off-by: Dexuan Cui
Signed-off-by: Sheng Yang
---
I've done a prototype of LM support, would send out tomorrow. But the test
case in QEmu side seems got something wrong. I always got an segfault at:
qemu-kvm/hw/fw_cfg.c:223
223 s->entries[arch][key].data = data;
Haven't looked into it yet. But maybe someone knows the reason...
arch/x86/include/asm/kvm_host.h |3 +
arch/x86/include/asm/vmx.h |1 +
arch/x86/kvm/kvm_cache_regs.h |6 ++
arch/x86/kvm/vmx.c | 26 ++
arch/x86/kvm/x86.c | 101 ---
include/linux/kvm_host.h|2 +-
6 files changed, 131 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index d08bb4a..532b220 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -302,6 +302,7 @@ struct kvm_vcpu_arch {
} update_pte;
struct fpu guest_fpu;
+ u64 xcr0;
gva_t mmio_fault_cr2;
struct kvm_pio_request pio;
@@ -564,6 +565,8 @@ u8 kvm_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t
gfn);
extern bool tdp_enabled;
+extern u64 host_xcr0;
+
enum emulation_result {
EMULATE_DONE, /* no further processing */
EMULATE_DO_MMIO, /* kvm_run filled with mmio request */
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 9e6779f..346ea66 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -266,6 +266,7 @@ enum vmcs_field {
#define EXIT_REASON_EPT_VIOLATION 48
#define EXIT_REASON_EPT_MISCONFIG 49
#define EXIT_REASON_WBINVD 54
+#define EXIT_REASON_XSETBV 55
/*
* Interruption-information format
diff --git a/arch/x86/kvm/kvm_cache_regs.h b/arch/x86/kvm/kvm_cache_regs.h
index d2a98f8..6491ac8 100644
--- a/arch/x86/kvm/kvm_cache_regs.h
+++ b/arch/x86/kvm/kvm_cache_regs.h
@@ -71,4 +71,10 @@ static inline ulong kvm_read_cr4(struct kvm_vcpu *vcpu)
return kvm_read_cr4_bits(vcpu, ~0UL);
}
+static inline u64 kvm_read_edx_eax(struct kvm_vcpu *vcpu)
+{
+ return (kvm_register_read(vcpu, VCPU_REGS_RAX) & -1u)
+ | ((u64)(kvm_register_read(vcpu, VCPU_REGS_RDX) & -1u) << 32);
+}
+
#endif
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 99ae513..a946841 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -36,6 +36,8 @@
#include
#include
#include
+#include
+#include
#include "trace.h"
@@ -3354,6 +3356,29 @@ static int handle_wbinvd(struct kvm_vcpu *vcpu)
return 1;
}
+static int handle_xsetbv(struct kvm_vcpu *vcpu)
+{
+ u64 new_bv = kvm_read_edx_eax(vcpu);
+
+ if (kvm_register_read(vcpu, VCPU_REGS_RCX) != 0)
+ goto err;
+ if (vmx_get_cpl(vcpu) != 0)
+ goto err;
+ if (!(new_bv & XSTATE_FP))
+ goto err;
+ if ((new_bv & XSTATE_YMM) && !(new_bv & XSTATE_SSE))
+ goto err;
+ if (new_bv & ~host_xcr0)
+ goto err;
+ vcpu->arch.xcr0 = new_bv;
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
+ skip_emulated_instruction(vcpu);
+ return 1;
+err:
+ kvm_inject_gp(vcpu, 0);
+ return 1;
+}
+
static int handle_apic_access(struct kvm_vcpu *vcpu)
{
return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
@@ -3632,6 +3657,7 @@ static int (*kvm_vmx_exit_handlers[])(struct kvm_vcpu
*vcpu) = {
[EXIT_REASON_TPR_BELOW_THRESHOLD] = handle_tpr_below_threshold,
[EXIT_REASON_APIC_ACCESS] = handle_apic_access,
[EXIT_REASON_WBINVD] = handle_wbinvd,
+ [EXIT_REASON_XSETBV] = handle_xsetbv,
[EXIT_REASON_TASK_SWITCH] = handle_task_switch,
[EXIT_REASON_MCE_DURING_VMENTRY] = handle_machine_check,
[EXIT_REASON_EPT_VIOLATION] = handle_ept_violation,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7be1d36..c04d3cb 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -64,6 +64,7 @@
(~(unsigned long)(X86_CR4_VME | X86_CR4_PVI | X86_CR4_TSD | X86_CR4_DE\
| X86_CR4_PSE | X86_CR4_PAE | X86_CR4_MCE \
| X86_CR4_PGE | X86_CR4_PCE | X86_CR4_OSFXSR \
+ | X86_CR4_OSXSAVE \
| X86_CR4_OSXMMEXCPT | X86_CR4_VMXE))
#define CR8_RESERVED_BITS (~(unsigned long)X86_CR8_TPR)
@@ -149,6 +150,14 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
{ NULL }
};
+u64 __read_mostly host_xcr0;
+EXPORT_SYMBOL_GPL(host_xcr0);
+
+static inline u32 bit(int bitno)
+{
+ return 1 << (bitno & 31);
+}
+
static v
[Qemu-devel] [PATCH] drive: allow rerror, werror and readonly for if=none
When creating guest disks the qdev way using ...
-drive if=none,id=$name,args
-device $driver,drive=$name
it is not possible to specify rerror, werror and readonly arguments
for drive as drive_init allows/blocks them based on the interface (if=)
specified and none isn't white-listed there.
Signed-off-by: Gerd Hoffmann
---
vl.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/vl.c b/vl.c
index 328395e..71346ac 100644
--- a/vl.c
+++ b/vl.c
@@ -950,7 +950,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
on_write_error = BLOCK_ERR_STOP_ENOSPC;
if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
-if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO) {
+if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type !=
IF_NONE) {
fprintf(stderr, "werror is no supported by this format\n");
return NULL;
}
@@ -963,7 +963,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
on_read_error = BLOCK_ERR_REPORT;
if ((buf = qemu_opt_get(opts, "rerror")) != NULL) {
-if (type != IF_IDE && type != IF_VIRTIO) {
+if (type != IF_IDE && type != IF_VIRTIO && type != IF_NONE) {
fprintf(stderr, "rerror is no supported by this format\n");
return NULL;
}
@@ -,7 +,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
/* CDROM is fine for any interface, don't check. */
ro = 1;
} else if (ro == 1) {
-if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY) {
+if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && type
!= IF_NONE) {
fprintf(stderr, "qemu: readonly flag not supported for drive with
this interface\n");
return NULL;
}
--
1.6.6.1
Re: [Qemu-devel] [PATCH 1/2 v4] Support for multiple keyboard devices
On Mon, May 10, 2010 at 03:18:29PM -0500, Anthony Liguori wrote:
> Date: Mon, 10 May 2010 15:18:29 -0500
> From: Anthony Liguori
> To: Shahar Havivi
> CC: qemu-devel@nongnu.org
> Subject: Re: [Qemu-devel] [PATCH 1/2 v4] Support for multiple keyboard
> devices
>
> On 04/18/2010 02:21 PM, Shahar Havivi wrote:
> >Patch add QEMUPutKbdEntry structure - handling each keyboard entry, the
> >structure handled
> >by qemu tail queue.
> >Adding a new keyboard add to the list and select it, removing keyboard
> >select the previous
> >keyboard in list.
> >
> >Signed-off-by: Shahar Havivi
>
> Applied all. Thanks.
>
> Regards,
>
> Anthony Liguori
>
Anthony, I don't think that you applied this changes,
Thanks
Shahar Havivi.
> >---
> > console.h| 14 -
> > hw/adb.c |2 +-
> > hw/escc.c|3 +-
> > hw/musicpal.c|2 +-
> > hw/nseries.c |4 +-
> > hw/palm.c|2 +-
> > hw/ps2.c |2 +-
> > hw/pxa2xx_keypad.c |3 +-
> > hw/spitz.c |3 +-
> > hw/stellaris_input.c |2 +-
> > hw/syborg_keyboard.c |2 +-
> > hw/usb-hid.c | 10 ++--
> > hw/xenfb.c |5 ++-
> > input.c | 51
> > -
> > 14 files changed, 78 insertions(+), 27 deletions(-)
> >
> >diff --git a/console.h b/console.h
> >index 6def115..91b66ea 100644
> >--- a/console.h
> >+++ b/console.h
> >@@ -41,7 +41,19 @@ typedef struct QEMUPutLEDEntry {
> > QTAILQ_ENTRY(QEMUPutLEDEntry) next;
> > } QEMUPutLEDEntry;
> >
> >-void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque);
> >+typedef struct QEMUPutKbdEntry {
> >+char *qemu_put_kbd_name;
> >+QEMUPutKBDEvent *qemu_put_kbd_event;
> >+void *qemu_put_kbd_event_opaque;
> >+int index;
> >+
> >+QTAILQ_ENTRY(QEMUPutKbdEntry) node;
> >+} QEMUPutKbdEntry;
> >+
> >+QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
> >+void *opaque,
> >+const char *name);
> >+void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry);
> > QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
> > void *opaque, int absolute,
> > const char *name);
> >diff --git a/hw/adb.c b/hw/adb.c
> >index 4fb7a62..09afcf9 100644
> >--- a/hw/adb.c
> >+++ b/hw/adb.c
> >@@ -304,7 +304,7 @@ void adb_kbd_init(ADBBusState *bus)
> > s = qemu_mallocz(sizeof(KBDState));
> > d = adb_register_device(bus, ADB_KEYBOARD, adb_kbd_request,
> > adb_kbd_reset, s);
> >-qemu_add_kbd_event_handler(adb_kbd_put_keycode, d);
> >+qemu_add_kbd_event_handler(adb_kbd_put_keycode, d, "adb");
> > register_savevm("adb_kbd", -1, 1, adb_kbd_save,
> > adb_kbd_load, s);
> > }
> >diff --git a/hw/escc.c b/hw/escc.c
> >index 6d2fd36..2b21d98 100644
> >--- a/hw/escc.c
> >+++ b/hw/escc.c
> >@@ -919,7 +919,8 @@ static int escc_init1(SysBusDevice *dev)
> > "QEMU Sun Mouse");
> > }
> > if (s->chn[1].type == kbd) {
> >-qemu_add_kbd_event_handler(sunkbd_event,&s->chn[1]);
> >+qemu_add_kbd_event_handler(sunkbd_event,&s->chn[1],
> >+ "QEMU Sun Keyboard");
> > }
> >
> > return 0;
> >diff --git a/hw/musicpal.c b/hw/musicpal.c
> >index ebd933e..e1a3b6a 100644
> >--- a/hw/musicpal.c
> >+++ b/hw/musicpal.c
> >@@ -1447,7 +1447,7 @@ static int musicpal_key_init(SysBusDevice *dev)
> >
> > qdev_init_gpio_out(&dev->qdev, s->out, ARRAY_SIZE(s->out));
> >
> >-qemu_add_kbd_event_handler(musicpal_key_event, s);
> >+qemu_add_kbd_event_handler(musicpal_key_event, s, "Musicpal");
> >
> > return 0;
> > }
> >diff --git a/hw/nseries.c b/hw/nseries.c
> >index 0273eee..abfcec3 100644
> >--- a/hw/nseries.c
> >+++ b/hw/nseries.c
> >@@ -262,7 +262,7 @@ static void n800_tsc_kbd_setup(struct n800_s *s)
> > if (n800_keys[i]>= 0)
> > s->keymap[n800_keys[i]] = i;
> >
> >-qemu_add_kbd_event_handler(n800_key_event, s);
> >+qemu_add_kbd_event_handler(n800_key_event, s, "Nokia n800");
> >
> > tsc210x_set_transform(s->ts.chip,&n800_pointercal);
> > }
> >@@ -371,7 +371,7 @@ static void n810_kbd_setup(struct n800_s *s)
> > if (n810_keys[i]> 0)
> > s->keymap[n810_keys[i]] = i;
> >
> >-qemu_add_kbd_event_handler(n810_key_event, s);
> >+qemu_add_kbd_event_handler(n810_key_event, s, "Nokia n810");
> >
> > /* Attach the LM8322 keyboard to the I2C bus,
> > * should happen in n8x0_i2c_setup and s->kbd be initialised here. */
> >diff --git a/hw/palm.c b/hw/palm.c
> >index 6d19167..1b405d4 100644
> >--- a/hw/palm.c
> >+++ b/hw/palm.c
> >@@ -228,7 +228,7 @@ static void palmte_init(ram_addr_t ram_si
Re: [Qemu-devel] [PATCH] drive: allow rerror, werror and readonly for if=none
Am 26.05.2010 10:44, schrieb Gerd Hoffmann: > When creating guest disks the qdev way using ... > > -drive if=none,id=$name,args > -device $driver,drive=$name > > it is not possible to specify rerror, werror and readonly arguments > for drive as drive_init allows/blocks them based on the interface (if=) > specified and none isn't white-listed there. > > Signed-off-by: Gerd Hoffmann Thanks, applied to the block branch. Kevin
[Qemu-devel] Re: [PATCH v5] KVM: VMX: Enable XSAVE/XRSTORE for guest
On 05/26/2010 12:19 PM, Sheng Yang wrote:
From: Dexuan Cui
This patch enable guest to use XSAVE/XRSTORE instructions.
We assume that host_xcr0 would use all possible bits that OS supported.
And we loaded xcr0 in the same way we handled fpu - do it as late as we can.
Looks really good now, only a couple of minor comments and I think we're
done.
I've done a prototype of LM support, would send out tomorrow. But the test
case in QEmu side seems got something wrong. I always got an segfault at:
qemu-kvm/hw/fw_cfg.c:223
223 s->entries[arch][key].data = data;
Haven't looked into it yet. But maybe someone knows the reason...
I saw this too, then it disappeared, can't remember why. Perhaps a
clean build is needed?
+static int handle_xsetbv(struct kvm_vcpu *vcpu)
+{
+ u64 new_bv = kvm_read_edx_eax(vcpu);
+
+ if (kvm_register_read(vcpu, VCPU_REGS_RCX) != 0)
+ goto err;
+ if (vmx_get_cpl(vcpu) != 0)
+ goto err;
+ if (!(new_bv& XSTATE_FP))
+ goto err;
+ if ((new_bv& XSTATE_YMM)&& !(new_bv& XSTATE_SSE))
+ goto err;
+ if (new_bv& ~host_xcr0)
+ goto err;
+ vcpu->arch.xcr0 = new_bv;
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, vcpu->arch.xcr0);
Please move all the code above to kvm_set_xcr0() in x86.c, since it's
not vendor specific. This would also allow you to make host_xcr0 local
to x86.c.
+ skip_emulated_instruction(vcpu);
+ return 1;
+err:
+ kvm_inject_gp(vcpu, 0);
+ return 1;
+}
/*
* List of msr numbers which we expose to userspace through KVM_GET_MSRS
* and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
@@ -1813,6 +1847,14 @@ static int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu
*vcpu,
r = 0;
kvm_apic_set_version(vcpu);
kvm_x86_ops->cpuid_update(vcpu);
+ update_cpuid(vcpu);
+
+ /*
+* Ensure guest xcr0 is valid for loading, also using as
+* the indicator of if guest cpuid has XSAVE
+*/
+ if (guest_cpuid_has_xsave(vcpu))
+ vcpu->arch.xcr0 = XSTATE_FP;
This is problematic because it enforces an ordering between KVM_SET_XCR
and KVM_SET_CPUID. So I think you can use kvm_read_cr4_bits(OSXSAVE)
instead of checking vcpu->arch.xcr0. Sorry for the bad advice earlier.
@@ -5134,12 +5207,26 @@ void kvm_load_guest_fpu(struct kvm_vcpu *vcpu)
vcpu->guest_fpu_loaded = 1;
unlazy_fpu(current);
+ /*
+* Restore all possible states in the guest,
+* and assume host would use all available bits.
+* Guest xcr0 would be loaded later.
+*/
+ if (cpu_has_xsave&& vcpu->arch.xcr0) {
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
+ vcpu->guest_xcr0_loaded = 0;
+ }
Has to be before unlazy_fpu(), so host fpu uses host xcr0.
It's sufficient to check for guest cr4.osxsave, no need to check for
cpu_has_xsave. But you need to check for guest_xcr0_loaded!
fpu_restore_checking(&vcpu->arch.guest_fpu);
trace_kvm_fpu(1);
}
void kvm_put_guest_fpu(struct kvm_vcpu *vcpu)
{
+ if (vcpu->guest_xcr0_loaded) {
+ vcpu->guest_xcr0_loaded = 0;
+ xsetbv(XCR_XFEATURE_ENABLED_MASK, host_xcr0);
+ }
+
This duplicates the above. So better to have
kvm_load_guest_xcr0()/kvm_put_guest_xcr0().
--
error compiling committee.c: too many arguments to function
[Qemu-devel] hw/serial.c: Xmit fifo never used
Hi,
the xmit fifo of the serial device is never used. If qemu_chr_write()
fails (interface currently not able to send characters) then the
transmit_timer should be engaged to try to send the current character
from the fifo again after some time. The code is
} else if (qemu_chr_write(s->chr, &s->tsr, 1) != 1) {
if ((s->tsr_retry > 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
s->tsr_retry++;
qemu_mod_timer(s->transmit_timer,
new_xmit_ts + s->char_transmit_time);
return;
}
...
}
The problem is that this path is never used as tsr_retry is never > 0
initially. So if qemu_chr_write() fails, we never try again but drop
the character.
I assume the correct condition would be '>= 0', that is
...
if ((s->tsr_retry >= 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
s->tsr_retry++;
...
Kind regards,
Frank
--
Dr.-Ing. Frank Mehnert
Sitz der Gesellschaft:
Sun Microsystems GmbH, Sonnenallee 1, 85551 Kirchheim-Heimstetten
Amtsgericht München: HRB 161028
Geschäftsführer: Jürgen Kunz
signature.asc
Description: This is a digitally signed message part.
Re: [Qemu-devel] Re: [PATCH 3/5] QMP: Introduce MIGRATION events
On Tue, May 25, 2010 at 11:33:15AM -0500, Anthony Liguori wrote: > On 05/25/2010 11:25 AM, Daniel P. Berrange wrote: > > With some disk locking > >approaches we need todo a lock transfer before allowing the dest > >to continue running. > > QEMU is going to read the disk before the migration completes so the > lock transfer is not going to work with the current implementation (it > needs to read the disk to do probing). I assume this is not something > that's actually been implemented. Lock transfer upon migration is a two-phase process. When the QEMU process for incoming migration starts a shared lock is held. Upon completion of migration an attempt is made to upgrade this to an exclusive lock. Only if this upgrade succeeds the dest is allowed to start running. > > It could be optimized to avoid the -S /cont > >in cases where those two scenarios aren't relevant, but only if > >we can get a separate async notification of when migration starts > >and completes on the destination, so we can notify mgmt apps that > >need this lifecycle event. > > > > Migration completes == guest starts running. You'll get a notification > of that but you're not getting that now because you're doing -S which > I'd argue is a functional problem on the part of libvirt (you're > breaking the downtime contract). If doing non-live migration, or restoring a saved image which was originally in the paused state, there is no 'guest starts running' event. You cannot reliably infer that migration completed by looking for a RESUME event. Hence the need for an explicit notification for the end of migration events. > I'm not sure why you would need a notification of when migration starts > (since you know when you've started migration). The destination only knows that it has started a QEMU instance ready for incoming migration. It doesn't know when / if an incoming migration has actually been accepted & started processing. Hence the need for an explicit notification for the start of migration. Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
[Qemu-devel] [PATCH] qdev-properties: Fix (u)intXX parsers
scanf calls must not use PRI constants, they have probably the wrong size and
corrupt memory. We could replace them by SCN ones, but strtol is simpler than
scanf here anyway. While at it, also fix the parsers to reject garbage after
the number ("4096xyz" was accepted before).
Signed-off-by: Kevin Wolf
---
hw/qdev-properties.c | 50 +++---
1 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 9ffdba7..9a61ca2 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -68,12 +68,14 @@ PropertyInfo qdev_prop_bit = {
static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
{
uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
-const char *fmt;
+char *end;
/* accept both hex and decimal */
-fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx8 : "%" PRIu8;
-if (sscanf(str, fmt, ptr) != 1)
+*ptr = strtoul(str, &end, 0);
+if (end != str + strlen(str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -96,12 +98,14 @@ PropertyInfo qdev_prop_uint8 = {
static int parse_uint16(DeviceState *dev, Property *prop, const char *str)
{
uint16_t *ptr = qdev_get_prop_ptr(dev, prop);
-const char *fmt;
+char *end;
/* accept both hex and decimal */
-fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx16 : "%" PRIu16;
-if (sscanf(str, fmt, ptr) != 1)
+*ptr = strtoul(str, &end, 0);
+if (end != str + strlen(str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -124,12 +128,14 @@ PropertyInfo qdev_prop_uint16 = {
static int parse_uint32(DeviceState *dev, Property *prop, const char *str)
{
uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
-const char *fmt;
+char *end;
/* accept both hex and decimal */
-fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx32 : "%" PRIu32;
-if (sscanf(str, fmt, ptr) != 1)
+*ptr = strtoul(str, &end, 0);
+if (end != str + strlen(str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -150,9 +156,13 @@ PropertyInfo qdev_prop_uint32 = {
static int parse_int32(DeviceState *dev, Property *prop, const char *str)
{
int32_t *ptr = qdev_get_prop_ptr(dev, prop);
+char *end;
-if (sscanf(str, "%" PRId32, ptr) != 1)
+*ptr = strtol(str, &end, 10);
+if (end != str + strlen(str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -175,9 +185,13 @@ PropertyInfo qdev_prop_int32 = {
static int parse_hex32(DeviceState *dev, Property *prop, const char *str)
{
uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
+char *end;
-if (sscanf(str, "%" PRIx32, ptr) != 1)
+*ptr = strtoul(str, &end, 16);
+if (end != str + strlen(str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -200,12 +214,14 @@ PropertyInfo qdev_prop_hex32 = {
static int parse_uint64(DeviceState *dev, Property *prop, const char *str)
{
uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
-const char *fmt;
+char *end;
/* accept both hex and decimal */
-fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx64 : "%" PRIu64;
-if (sscanf(str, fmt, ptr) != 1)
+*ptr = strtoull(str, &end, 0);
+if (end != str + strlen(str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -228,9 +244,13 @@ PropertyInfo qdev_prop_uint64 = {
static int parse_hex64(DeviceState *dev, Property *prop, const char *str)
{
uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
+char *end;
-if (sscanf(str, "%" PRIx64, ptr) != 1)
+*ptr = strtoull(str, &end, 16);
+if (end != str + strlen(str)) {
return -EINVAL;
+}
+
return 0;
}
--
1.6.6.1
[Qemu-devel] Re: hw/serial.c: Xmit fifo never used
Frank Mehnert wrote:
> Hi,
>
> the xmit fifo of the serial device is never used. If qemu_chr_write()
> fails (interface currently not able to send characters) then the
> transmit_timer should be engaged to try to send the current character
> from the fifo again after some time. The code is
>
> } else if (qemu_chr_write(s->chr, &s->tsr, 1) != 1) {
> if ((s->tsr_retry > 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
> s->tsr_retry++;
> qemu_mod_timer(s->transmit_timer,
>new_xmit_ts + s->char_transmit_time);
> return;
> }
> ...
> }
>
> The problem is that this path is never used as tsr_retry is never > 0
> initially. So if qemu_chr_write() fails, we never try again but drop
> the character.
>
> I assume the correct condition would be '>= 0', that is
>
> ...
> if ((s->tsr_retry >= 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
> s->tsr_retry++;
> ...
Makes sense, patch welcome.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] qdev-properties: Fix (u)intXX parsers
On Wed, May 26, 2010 at 12:28:13PM +0200, Kevin Wolf wrote:
> scanf calls must not use PRI constants, they have probably the wrong size and
> corrupt memory. We could replace them by SCN ones, but strtol is simpler than
> scanf here anyway. While at it, also fix the parsers to reject garbage after
> the number ("4096xyz" was accepted before).
>
> Signed-off-by: Kevin Wolf
> ---
> hw/qdev-properties.c | 50
> +++---
> 1 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 9ffdba7..9a61ca2 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -68,12 +68,14 @@ PropertyInfo qdev_prop_bit = {
> static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
> {
> uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
> -const char *fmt;
> +char *end;
>
> /* accept both hex and decimal */
> -fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx8 : "%" PRIu8;
> -if (sscanf(str, fmt, ptr) != 1)
> +*ptr = strtoul(str, &end, 0);
> +if (end != str + strlen(str)) {
> return -EINVAL;
> +}
I think you can avoid the O(n) operation here & in the other cases with
a test like this:
if ((end == str) || (*end != '\0'))
return -EINVAL
Regards,
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Re: [Qemu-devel] [PATCH] resent: x86/cpuid: propagate further CPUID leafs when -cpu host
Anthony Liguori wrote: On 05/25/2010 08:21 AM, Andre Przywara wrote: What's the benefit of exposing this information to the guest? That is mostly to propagate the cache size and organization parameters to the guest: +/* safe CPUID leafs to propagate to guest if -cpu host is specified + * Intel defined leafs: + * Cache descriptors (0x02) + * Deterministic cache parameters (0x04) + * Monitor/MWAIT parameters (0x05) + * + * AMD defined leafs: + * L1 Cache and TLB (0x05) + * L2+L3 TLB (0x06) + * LongMode address size (0x08) + * 1GB page TLB (0x19) + * Performance optimization (0x1A) + */ Since at least L1 and L2 caches are mostly private to vCPUs, I see no reason to disguise them. But in practice, what is it useful for? Just because we can expose it doesn't mean we should. Beside the obvious high performance libraries (like the AMD ACML) also JVMs (and probably other managed code runtimes) use the cache information for optimization. Given the fact that we have a fairly large range of actual L2 cache sizes (from 256KB to 6MB on Intel) the fixed cache size that QEMU reports (1MB) is quite a bit off most of the times. -cpu host is targeted to give the best performance to the user, with the drawback of missing or very limited migration experience. So we should expose as many features as possible. Regards, Andre. -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany Tel: +49 351 448-3567-12
Re: [Qemu-devel] [PATCH] qdev-properties: Fix (u)intXX parsers
Am 26.05.2010 12:45, schrieb Daniel P. Berrange:
> On Wed, May 26, 2010 at 12:28:13PM +0200, Kevin Wolf wrote:
>> scanf calls must not use PRI constants, they have probably the wrong size and
>> corrupt memory. We could replace them by SCN ones, but strtol is simpler than
>> scanf here anyway. While at it, also fix the parsers to reject garbage after
>> the number ("4096xyz" was accepted before).
>>
>> Signed-off-by: Kevin Wolf
>> ---
>> hw/qdev-properties.c | 50
>> +++---
>> 1 files changed, 35 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
>> index 9ffdba7..9a61ca2 100644
>> --- a/hw/qdev-properties.c
>> +++ b/hw/qdev-properties.c
>> @@ -68,12 +68,14 @@ PropertyInfo qdev_prop_bit = {
>> static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
>> {
>> uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
>> -const char *fmt;
>> +char *end;
>>
>> /* accept both hex and decimal */
>> -fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx8 : "%" PRIu8;
>> -if (sscanf(str, fmt, ptr) != 1)
>> +*ptr = strtoul(str, &end, 0);
>> +if (end != str + strlen(str)) {
>> return -EINVAL;
>> +}
>
> I think you can avoid the O(n) operation here & in the other cases with
> a test like this:
>
> if ((end == str) || (*end != '\0'))
>return -EINVAL
It probably doesn't really make a difference here, but you're right.
I'll send another version with this change.
Kevin
Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.
On Wed, 26 May 2010, Gerd Hoffmann wrote: > Try to pci hotplug a vga card, watch qemu die with hw_error(). > This patch fixes it. > Do you know the reason why we get hw_error()? Theoretically vga hotplug should be possible at least for secondary graphic cards (even though I suspect most operating systems wouldn't cope).
[Qemu-devel] Re: [PATCH v5] KVM: VMX: Enable XSAVE/XRSTORE for guest
Avi Kivity wrote: > On 05/26/2010 12:19 PM, Sheng Yang wrote: >> I've done a prototype of LM support, would send out tomorrow. But the >> test >> case in QEmu side seems got something wrong. I always got an segfault at: >> qemu-kvm/hw/fw_cfg.c:223 >> 223 s->entries[arch][key].data = data; >> >> Haven't looked into it yet. But maybe someone knows the reason... >> > > I saw this too, then it disappeared, can't remember why. Perhaps a > clean build is needed? qemu-kvm apparently lacks upstream commit a71cd2a523f9b35ffeba8de3c536e494e255e6ea which should resolve at least some of those mysterious crashes. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] Re: hw/serial.c: Xmit fifo never used
On Wednesday 26 May 2010, Jan Kiszka wrote:
> Frank Mehnert wrote:
> > I assume the correct condition would be '>= 0', that is
> >
> > ...
> > if ((s->tsr_retry >= 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
> > s->tsr_retry++;
> > ...
>
> Makes sense, patch welcome.
Attached.
Kind regards,
Frank
--
Dr.-Ing. Frank Mehnert
Sitz der Gesellschaft:
Sun Microsystems GmbH, Sonnenallee 1, 85551 Kirchheim-Heimstetten
Amtsgericht München: HRB 161028
Geschäftsführer: Jürgen Kunz
diff --git a/hw/serial.c b/hw/serial.c
index 9102edb..0b1550b 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -327,7 +327,7 @@ static void serial_xmit(void *opaque)
/* in loopback mode, say that we just received a char */
serial_receive1(s, &s->tsr, 1);
} else if (qemu_chr_write(s->chr, &s->tsr, 1) != 1) {
-if ((s->tsr_retry > 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
+if ((s->tsr_retry >= 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
s->tsr_retry++;
qemu_mod_timer(s->transmit_timer, new_xmit_ts + s->char_transmit_time);
return;
signature.asc
Description: This is a digitally signed message part.
Re: [Qemu-devel] [PATCH] qdev-properties: Fix (u)intXX parsers
Kevin Wolf writes:
> scanf calls must not use PRI constants, they have probably the wrong size and
> corrupt memory. We could replace them by SCN ones, but strtol is simpler than
> scanf here anyway. While at it, also fix the parsers to reject garbage after
> the number ("4096xyz" was accepted before).
Do we have more misuse of PRI with scanf elsewhere? No need to fix them
all in one commit (and thus delay this fix); I just want to make sure
somebody looks.
[Qemu-devel] Re: hw/serial.c: Xmit fifo never used
Frank Mehnert wrote:
> On Wednesday 26 May 2010, Jan Kiszka wrote:
>> Frank Mehnert wrote:
>>> I assume the correct condition would be '>= 0', that is
>>>
>>> ...
>>> if ((s->tsr_retry >= 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
>>> s->tsr_retry++;
>>> ...
>> Makes sense, patch welcome.
>
> Attached.
Signed-off, subject, and description please. And I would keep Stefano as
the original author of this feature in CC. A cross-check by him might be
good as we enable a code path that was apparently dead so far.
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] qdev-properties: Fix (u)intXX parsers
Am 26.05.2010 13:13, schrieb Markus Armbruster:
> Kevin Wolf writes:
>
>> scanf calls must not use PRI constants, they have probably the wrong size and
>> corrupt memory. We could replace them by SCN ones, but strtol is simpler than
>> scanf here anyway. While at it, also fix the parsers to reject garbage after
>> the number ("4096xyz" was accepted before).
>
> Do we have more misuse of PRI with scanf elsewhere? No need to fix them
> all in one commit (and thus delay this fix); I just want to make sure
> somebody looks.
I saw another one in some Xen file that Gerd should fix some time. I'm
not aware of any other.
Kevin
Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.
On 05/26/10 12:59, Stefano Stabellini wrote: On Wed, 26 May 2010, Gerd Hoffmann wrote: Try to pci hotplug a vga card, watch qemu die with hw_error(). This patch fixes it. Do you know the reason why we get hw_error()? Because the card tries to register the legacy vga ports which are already taken by the primary card. I also don't know whenever you can pci hot-plug hardware which uses non-pci ressources at all. Theoretically vga hotplug should be possible at least for secondary graphic cards (even though I suspect most operating systems wouldn't cope). Yes. Assuming the virtual hardware in question can actually act as secondary, i.e. is fully programmable without the legacy vga ports. The standard vga can't. The cirrus looks doable, at least you can access the vga ports using the mmio bar. cheers, Gerd
[Qemu-devel] Re: [PATCH v5] KVM: VMX: Enable XSAVE/XRSTORE for guest
On 05/26/2010 01:52 PM, Jan Kiszka wrote: Avi Kivity wrote: On 05/26/2010 12:19 PM, Sheng Yang wrote: I've done a prototype of LM support, would send out tomorrow. But the test case in QEmu side seems got something wrong. I always got an segfault at: qemu-kvm/hw/fw_cfg.c:223 223 s->entries[arch][key].data = data; Haven't looked into it yet. But maybe someone knows the reason... I saw this too, then it disappeared, can't remember why. Perhaps a clean build is needed? qemu-kvm apparently lacks upstream commit a71cd2a523f9b35ffeba8de3c536e494e255e6ea which should resolve at least some of those mysterious crashes. That explains the fact that rebuild fixed it. I'll merge qemu.git shortly. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v2] qdev-properties: Fix (u)intXX parsers
scanf calls must not use PRI constants, they have probably the wrong size and
corrupt memory. We could replace them by SCN ones, but strtol is simpler than
scanf here anyway. While at it, also fix the parsers to reject garbage after
the number ("4096xyz" was accepted before).
Signed-off-by: Kevin Wolf
---
hw/qdev-properties.c | 50 +++---
1 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 9ffdba7..64d83ce 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -68,12 +68,14 @@ PropertyInfo qdev_prop_bit = {
static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
{
uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
-const char *fmt;
+char *end;
/* accept both hex and decimal */
-fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx8 : "%" PRIu8;
-if (sscanf(str, fmt, ptr) != 1)
+*ptr = strtoul(str, &end, 0);
+if ((*end != '\0') || (end == str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -96,12 +98,14 @@ PropertyInfo qdev_prop_uint8 = {
static int parse_uint16(DeviceState *dev, Property *prop, const char *str)
{
uint16_t *ptr = qdev_get_prop_ptr(dev, prop);
-const char *fmt;
+char *end;
/* accept both hex and decimal */
-fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx16 : "%" PRIu16;
-if (sscanf(str, fmt, ptr) != 1)
+*ptr = strtoul(str, &end, 0);
+if ((*end != '\0') || (end == str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -124,12 +128,14 @@ PropertyInfo qdev_prop_uint16 = {
static int parse_uint32(DeviceState *dev, Property *prop, const char *str)
{
uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
-const char *fmt;
+char *end;
/* accept both hex and decimal */
-fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx32 : "%" PRIu32;
-if (sscanf(str, fmt, ptr) != 1)
+*ptr = strtoul(str, &end, 0);
+if ((*end != '\0') || (end == str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -150,9 +156,13 @@ PropertyInfo qdev_prop_uint32 = {
static int parse_int32(DeviceState *dev, Property *prop, const char *str)
{
int32_t *ptr = qdev_get_prop_ptr(dev, prop);
+char *end;
-if (sscanf(str, "%" PRId32, ptr) != 1)
+*ptr = strtol(str, &end, 10);
+if ((*end != '\0') || (end == str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -175,9 +185,13 @@ PropertyInfo qdev_prop_int32 = {
static int parse_hex32(DeviceState *dev, Property *prop, const char *str)
{
uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
+char *end;
-if (sscanf(str, "%" PRIx32, ptr) != 1)
+*ptr = strtoul(str, &end, 16);
+if ((*end != '\0') || (end == str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -200,12 +214,14 @@ PropertyInfo qdev_prop_hex32 = {
static int parse_uint64(DeviceState *dev, Property *prop, const char *str)
{
uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
-const char *fmt;
+char *end;
/* accept both hex and decimal */
-fmt = strncasecmp(str, "0x",2) == 0 ? "%" PRIx64 : "%" PRIu64;
-if (sscanf(str, fmt, ptr) != 1)
+*ptr = strtoull(str, &end, 0);
+if ((*end != '\0') || (end == str)) {
return -EINVAL;
+}
+
return 0;
}
@@ -228,9 +244,13 @@ PropertyInfo qdev_prop_uint64 = {
static int parse_hex64(DeviceState *dev, Property *prop, const char *str)
{
uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
+char *end;
-if (sscanf(str, "%" PRIx64, ptr) != 1)
+*ptr = strtoull(str, &end, 16);
+if ((*end != '\0') || (end == str)) {
return -EINVAL;
+}
+
return 0;
}
--
1.6.6.1
[Qemu-devel] Re: [PATCH 3/5] QMP: Introduce MIGRATION events
On Tue, May 25, 2010 at 06:43:13PM +0200, Juan Quintela wrote: > Anthony Liguori wrote: > > On 05/25/2010 11:25 AM, Daniel P. Berrange wrote: > >> On Tue, May 25, 2010 at 06:04:17PM +0200, Juan Quintela wrote: > >> > >>> Anthony Liguori wrote: > > > I'm not sure why you would need a notification of when migration > > starts (since you know when you've started migration). > > But you don't know if the other end "knows" that it has also started. > > started is needed only in incoming part, because we don't have a > monitor to ask if migration has started. If we ever want to get closer to allowing multiple monitors, or allowing apps to issue QMP commands directly via libvirt, then we still need the 'migration started' event on the source, because something else can have issued the 'migrate' command without the mgmt app knowing. MIGRATED_STARTED+STOPPED really *is* needed if we're to make QMP cope with all possible use cases. If we rely on inferring it from STOP+RESUME events, it is going to exclude a significant set of use cases, and likely result in this being proposed all over again in 12 months time :-( Regards, Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Re: [Qemu-devel] Re: hw/serial.c: Xmit fifo never used
On Wed, 26 May 2010, Jan Kiszka wrote:
> Frank Mehnert wrote:
> > On Wednesday 26 May 2010, Jan Kiszka wrote:
> >> Frank Mehnert wrote:
> >>> I assume the correct condition would be '>= 0', that is
> >>>
> >>> ...
> >>> if ((s->tsr_retry >= 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
> >>> s->tsr_retry++;
> >>> ...
> >> Makes sense, patch welcome.
> >
> > Attached.
>
> Signed-off, subject, and description please. And I would keep Stefano as
> the original author of this feature in CC. A cross-check by him might be
> good as we enable a code path that was apparently dead so far.
>
I think the patch is correct.
Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.
On Wed, 26 May 2010, Gerd Hoffmann wrote: > On 05/26/10 12:59, Stefano Stabellini wrote: > > On Wed, 26 May 2010, Gerd Hoffmann wrote: > >> Try to pci hotplug a vga card, watch qemu die with hw_error(). > >> This patch fixes it. > >> > > > > Do you know the reason why we get hw_error()? > > Because the card tries to register the legacy vga ports which are > already taken by the primary card. I also don't know whenever you can > pci hot-plug hardware which uses non-pci ressources at all. > > > Theoretically vga hotplug should be possible at least for secondary > > graphic cards (even though I suspect most operating systems wouldn't > > cope). > > Yes. Assuming the virtual hardware in question can actually act as > secondary, i.e. is fully programmable without the legacy vga ports. The > standard vga can't. The cirrus looks doable, at least you can access > the vga ports using the mmio bar. I see, good point. I guess the right fix here would be to return -1 in the stdvga case but continue in the cirrus case and avoid registering the vga ioports when used as secondary adapter.
Re: [Qemu-devel] Re: irq problems after live migration with 0.12.4
Michael Tokarev wrote: 25.05.2010 15:03, Peter Lieven wrote: Michael Tokarev wrote: 23.05.2010 13:55, Peter Lieven wrote: [] [64442.298521] irq 10: nobody cared (try booting with the "irqpoll" option) [] [64442.299433] handlers: [64442.299840] [] (e1000_intr+0x0/0x190 [e1000]) [64442.300046] Disabling IRQ #10 Apparently, for some reason, e1000_intr decided it's not interesting IRQ or somehow wrong or not for that NIC. I dunno. But something fishy is going on with IRQs here. See also LP bug #584131 (https://bugs.launchpad.net/bugs/584131) and original Debian bug#580649 (http://bugs.debian.org/580649) Not sure if they're related... It looks they are actually the same thing, but happens with different devices and/or IRQs. Either spurious, or unwanted, or unrecognized or somesuch IRQ which is not recognized by the irq handler, which results in disabling that IRQ by the kernel, which is a bad thing (In your case it works because e1000 works in 2 modes, interrupts and polling). michael, do you have any ideas what i got do to debug whats happening? Unfortunately, no idea. I don't know neither kernel nor kvm internals. I would be very greatful if someone with deeper knowledge would hook up. I'm also not familiar with internals, unfortunately. looking at launchpad and debian bug tracker i found other bugs also with a maybe related problem. so this issue might be greater... Can you share your findings? I don't know other debian bugs which are similar to this one. I suspect that other reports regarding crashed VMs after migration might be related. If I take my test VM with that I can trigger the bug and change the Network Adapter from e1000 to rtl8139 and leave everything else untouched the VM hangs at 100% CPU.. Thanks! /mjt
Re: [Qemu-devel] RFC: ehci -> uhci handoff suggestions
On 05/25/10 15:40, David S. Ahern wrote: USB 2.0 leverages companion UHCI or OHCI host controllers for full and low speed devices. I do not see an appropriate means for doing that bus transition and could use some suggestions. Hmm. Well. That doesn't really fit into the qdev tree model ... As I understand the code at this point it is a top down setup: device added, bus found, device attached. Devices are always added to some bus. In the case of usb the devices can also be attached/detached. Emulated devices usually attached right after creating them. Host devices are attached when a matching physical device shows up. ie., key point is the expectation that the bus to which the device is assigned is known early in the code path. Yes. You can even specify the bus you want attach the device to. | EHCI controller |--->|UHCI / OHCI | | | | USB device model || USB device model | |(or driver )||(or driver )| high speed full / low speed To know which bus to attach it to the device needs to be queried/probed for basic information - something the current architecture does not have. USB devices can support both 1.1 and 2.0, right? Who decides which protocol is used then? I think the OS can speak 1.1 to the device even in case a ehci controller is present (but unused by the OS), right? Suggestions? Maybe it makes more sense to look at ehci/uhci as *one* (physical) device with multiple interfaces? They share the physical ports after all, at least on real hardware. The tricky case is assigning host devices, right? For the emulated ones we can probably could get away by simply forcing them into 2.0-only or 1.1-only mode depending on which bus they got attached to. cheers, Gerd
Re: [Qemu-devel] RFC: ehci -> uhci handoff suggestions
Am 26.05.2010 13:47, schrieb Gerd Hoffmann: > On 05/25/10 15:40, David S. Ahern wrote: >> >> USB 2.0 leverages companion UHCI or OHCI host controllers for full and >> low speed devices. I do not see an appropriate means for doing that bus >> transition and could use some suggestions. > > Hmm. Well. That doesn't really fit into the qdev tree model ... > >> As I understand the code at this point it is a top down setup: device >> added, bus found, device attached. > > Devices are always added to some bus. In the case of usb the devices > can also be attached/detached. Emulated devices usually attached right > after creating them. Host devices are attached when a matching physical > device shows up. > >> ie., key point is the expectation that the bus to which the device is >> assigned is known early in the code path. > > Yes. You can even specify the bus you want attach the device to. > >> >> | EHCI controller |--->|UHCI / OHCI | >> >> | | >> >> | USB device model || USB device model | >> |(or driver )||(or driver )| >> >> high speed full / low speed >> >> >> To know which bus to attach it to the device needs to be queried/probed >> for basic information - something the current architecture does not have. > > USB devices can support both 1.1 and 2.0, right? Who decides which > protocol is used then? I think the OS can speak 1.1 to the device even > in case a ehci controller is present (but unused by the OS), right? AFAIK the OS must tell the EHCI that it should hand the device off to the UHCI/OHCI companion before it can use it there. >> Suggestions? > > Maybe it makes more sense to look at ehci/uhci as *one* (physical) > device with multiple interfaces? They share the physical ports after > all, at least on real hardware. > > The tricky case is assigning host devices, right? For the emulated ones > we can probably could get away by simply forcing them into 2.0-only or > 1.1-only mode depending on which bus they got attached to. If they should be accessed via the EHCI or a companion controller depends on what the OS requests. And USB 2.0 says that any device that supports High Speed must also support Full Speed and therefore be accessible using the companion (at least that's what I understand). Kevin
[Qemu-devel] [Bug 532733] Re: apt/dpkg in qemu-system-arm hangs if a big task is installed
I am seeing similar issue on Lucid.
Rootstock version: rootstock-0.1.99.3
Here is my command line:
sudo ./rootstock --fqdn beagleboard --login ubuntu --password beagle
--imagesize 4G --swapsize 512 --components "main,universe,multiverse" --seed
openssh-server,build-essential,apache2,postgresql-server-dev-8.4,postgresql-contrib,sqlite3,libsqlite3-dev
--dist lucid --serial ttyS2 --kernel-image
http://rcn-ee.net/deb/kernel/beagle/lucid/v2.6.32.11-l13/linux-image-2.6.32.11-l13_1.0lucid_armel.deb
Fails with:
/bin/installer: line 55: 157 Segmentation fault apt-get -y install
openssh-server build-essential apache2 postgresql-server-dev-8.4
postgresql-contrib sqlite3 libsqlite3-dev
E: Second stage build in Virtual Machine failed !
E: Please see the log to see what went wrong.
I: Cleaning up...
../rootstock: line 54: 5051 Killed qemu-system-arm $QEMUOPTS
-append "${APPEND}" > $QEMUFIFO 2>&1
I: A logfile was saved as
/home/asingh/met/v_4_1_0/ubuntu-sd/rootstock-201005260742.log
I: done ...
mkimage: Can't open ./vmlinuz-*: No such file or directory
This succeeds if I just give 2 packages as seed.
This was and is all working in Jaunty without any issues.
--
apt/dpkg in qemu-system-arm hangs if a big task is installed
https://bugs.launchpad.net/bugs/532733
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
Status in QEMU: New
Status in “qemu-kvm” package in Ubuntu: Incomplete
Status in “qemu-kvm” source package in Lucid: Incomplete
Bug description:
Binary package hint: qemu-kvm
running rootstock and installing ubuntu-netbook^ makes the VM hang in
"unpacking iso-codes" this is reproducable every time in rootstock as well as
in a standard qemu-system-arm vm that contains a minimal ubuntu with running
apt-get install ubuntu-netbook
[Qemu-devel] Re: [PATCH v2] qdev-properties: Fix (u)intXX parsers
On 05/26/10 13:27, Kevin Wolf wrote:
scanf calls must not use PRI constants, they have probably the wrong size and
corrupt memory. We could replace them by SCN ones, but strtol is simpler than
scanf here anyway. While at it, also fix the parsers to reject garbage after
the number ("4096xyz" was accepted before).
Looks good to me.
Acked-by: Gerd Hoffmann
cheers,
Gerd
[Qemu-devel] [Bug 532733] Re: apt/dpkg in qemu-system-arm hangs if a big task is installed
@arvind this is not related to this bug, please open a new one -- apt/dpkg in qemu-system-arm hangs if a big task is installed https://bugs.launchpad.net/bugs/532733 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Status in “qemu-kvm” package in Ubuntu: Incomplete Status in “qemu-kvm” source package in Lucid: Incomplete Bug description: Binary package hint: qemu-kvm running rootstock and installing ubuntu-netbook^ makes the VM hang in "unpacking iso-codes" this is reproducable every time in rootstock as well as in a standard qemu-system-arm vm that contains a minimal ubuntu with running apt-get install ubuntu-netbook
[Qemu-devel] [Bug 532733] Re: apt/dpkg in qemu-system-arm hangs if a big task is installed
Sure Thanks. -- apt/dpkg in qemu-system-arm hangs if a big task is installed https://bugs.launchpad.net/bugs/532733 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Status in “qemu-kvm” package in Ubuntu: Incomplete Status in “qemu-kvm” source package in Lucid: Incomplete Bug description: Binary package hint: qemu-kvm running rootstock and installing ubuntu-netbook^ makes the VM hang in "unpacking iso-codes" this is reproducable every time in rootstock as well as in a standard qemu-system-arm vm that contains a minimal ubuntu with running apt-get install ubuntu-netbook
Re: [Qemu-devel] RFC: ehci -> uhci handoff suggestions
Hi, USB devices can support both 1.1 and 2.0, right? Who decides which protocol is used then? I think the OS can speak 1.1 to the device even in case a ehci controller is present (but unused by the OS), right? AFAIK the OS must tell the EHCI that it should hand the device off to the UHCI/OHCI companion before it can use it there. Huh? Compatibility-wise it makes sense to do it the other way around (i.e. have it @ UHCI/OHCI by default and move to EHCI on request), so a OS which knows nothing about EHCI can cope. If they should be accessed via the EHCI or a companion controller depends on what the OS requests. And USB 2.0 says that any device that supports High Speed must also support Full Speed and therefore be accessible using the companion (at least that's what I understand). Hmm, ok, so no shortcut even for emulated devices. Not that it would have helped much as we have to cover host devices anyway. Also I think one ehci controller can have multiple uhci companion controllers. At least lspci on my T60 suggests that: 00:1d.0 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI Controller #1 (rev 02) 00:1d.1 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI Controller #2 (rev 02) 00:1d.2 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI Controller #3 (rev 02) 00:1d.3 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI Controller #4 (rev 02) 00:1d.7 USB Controller: Intel Corporation 82801G (ICH7 Family) USB2 EHCI Controller (rev 02) cheers, Gerd
Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.
Hi, Yes. Assuming the virtual hardware in question can actually act as secondary, i.e. is fully programmable without the legacy vga ports. The standard vga can't. The cirrus looks doable, at least you can access the vga ports using the mmio bar. I see, good point. I guess the right fix here would be to return -1 in the stdvga case but continue in the cirrus case and avoid registering the vga ioports when used as secondary adapter. Except that this most likely is a non-trivial effort as we have to find and test sane ways to handle multiple guest displays. I think having two gfx screens mapped to two qemu consoles, then be able to switch between them via Ctrl-Alt- (like you switch today to text consoles) could be doable without too much effort. Question is how useful this would be as you can't see your two screens at the same time. With qxl+spice the spice client will open a new window for the secondary display. With vnc+sdl you'll see the primary display only. cheers, Gerd
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On 05/17/2010 03:58 PM, Anthony Liguori wrote: On 05/17/2010 05:14 AM, Alexander Graf wrote: Usually the guest can tell the host to flush data to disk. In some cases we don't want to flush though, but try to keep everything in cache. So let's add a new cache value to -drive that allows us to set the cache policy to most aggressive, disabling flushes. We call this mode "volatile", as guest data is not guaranteed to survive host crashes anymore. This patch also adds a noop function for aio, so we can do nothing in AIO fashion. Signed-off-by: Alexander Graf I'd like to see some performance data with at least an ext3 host file system and an ext4 file system. My concern is that ext3 exaggerates the cost of fsync() which will result in diminishing value over time for this feature as people move to ext4/btrfs. In fact, btrfs is currently unusable for virt because O_SYNC writes inflate a guest write to a host write. by a huge factor (50x-100x). cache=writethrough is 100% unusable, cache=writeback is barely tolerable. As of 2.6.32, cache=volatile is probably required to get something resembling reasonable performance on btrfs. Of course, we expect that btrfs will improve in time, but still it doesn't seem to be fsync friendly. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] RFC: ehci -> uhci handoff suggestions
On 05/26/2010 06:48 AM, Gerd Hoffmann wrote: > > Hi, > >>> USB devices can support both 1.1 and 2.0, right? Who decides which >>> protocol is used then? I think the OS can speak 1.1 to the device even >>> in case a ehci controller is present (but unused by the OS), right? >> >> AFAIK the OS must tell the EHCI that it should hand the device off to >> the UHCI/OHCI companion before it can use it there. > > Huh? Compatibility-wise it makes sense to do it the other way around > (i.e. have it @ UHCI/OHCI by default and move to EHCI on request), so a > OS which knows nothing about EHCI can cope. > >> If they should be accessed via the EHCI or a companion controller >> depends on what the OS requests. And USB 2.0 says that any device that >> supports High Speed must also support Full Speed and therefore be >> accessible using the companion (at least that's what I understand). > > Hmm, ok, so no shortcut even for emulated devices. Not that it would > have helped much as we have to cover host devices anyway. > > Also I think one ehci controller can have multiple uhci companion > controllers. At least lspci on my T60 suggests that: > > 00:1d.0 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI > Controller #1 (rev 02) > 00:1d.1 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI > Controller #2 (rev 02) > 00:1d.2 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI > Controller #3 (rev 02) > 00:1d.3 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI > Controller #4 (rev 02) > 00:1d.7 USB Controller: Intel Corporation 82801G (ICH7 Family) USB2 EHCI > Controller (rev 02) > > cheers, > Gerd > Yes, that is the ehci feature to be implemented. My understanding is that the port routing happens internally to the host controller based on device speed - section 4.2 (pag 64) of: http://www.intel.com/technology/usb/download/ehci-r10.pdf ehci does have more overhead from an emulation perspective, so it would be best to keep mice, keyboard, serial ports, etc on the uhci/ohci bus and have storage devices and webcams and such on ehci. And any transition should happen automagically within the device model. David
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On 05/25/2010 09:48 PM, Anthony Liguori wrote: On 05/25/2010 12:59 PM, Alexander Graf wrote: I see it as the equivalent to the Taint bit in Linux. I want to make it clear to users up front that if you use this option, and you have data loss issues, don't complain. Just putting something in qemu-doc.texi is not enough IMHO. Few people actually read it. So what exactly is the conclusion here? I really want to see this getting merged. Make it more scary and try again. How about ./configure --with-cache-volatile to enable it at all? We wants it. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.
On Wed, 26 May 2010, Gerd Hoffmann wrote: >Hi, > > >> Yes. Assuming the virtual hardware in question can actually act as > >> secondary, i.e. is fully programmable without the legacy vga ports. The > >> standard vga can't. The cirrus looks doable, at least you can access > >> the vga ports using the mmio bar. > > > > I see, good point. > > > > I guess the right fix here would be to return -1 in the stdvga case but > > continue in the cirrus case and avoid registering the vga ioports when > > used as secondary adapter. > > Except that this most likely is a non-trivial effort as we have to find > and test sane ways to handle multiple guest displays. > > I think having two gfx screens mapped to two qemu consoles, then be able > to switch between them via Ctrl-Alt- (like you switch today to text > consoles) could be doable without too much effort. Question is how > useful this would be as you can't see your two screens at the same time. > Actually I was thinking of registering multiple graphic consoles, each of them could be rendered by a different frontend (sdl/vnc) independently. We would have multiple DisplayStates for that. > With qxl+spice the spice client will open a new window for the secondary > display. With vnc+sdl you'll see the primary display only. So you are doing exactly what I wrote above, right?
Re: [Qemu-devel] [PATCH 3/5] QMP: Introduce MIGRATION events
On Tue, 25 May 2010 13:51:01 -0500
Anthony Liguori wrote:
> On 05/25/2010 01:31 PM, Luiz Capitulino wrote:
> > On Tue, 25 May 2010 16:21:03 +0200
> > Juan Quintela wrote:
> >
> >
> >> They are emitted when migration starts, ends, has a failure or is canceled.
> >>
> >> Signed-off-by: Juan Quintela
> >> ---
> >> QMP/qmp-events.txt | 50
> >> ++
> >> monitor.c | 12
> >> monitor.h |4
> >> 3 files changed, 66 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> >> index 01ec85f..93caa4d 100644
> >> --- a/QMP/qmp-events.txt
> >> +++ b/QMP/qmp-events.txt
> >> @@ -26,6 +26,56 @@ Example:
> >> Note: If action is "stop", a STOP event will eventually follow the
> >> BLOCK_IO_ERROR event.
> >>
> >> +MIGRATION_CANCELED
> >> +--
> >> +
> >> +Emitted when migration is canceled. This is emitted in the source.
> >>
> > Shouldn't this one be emitted in the destination?
> >
>
> Destination can't distinguish a cancelled from a closed pipe. But the
> idea is that a third party is talking to both source and destination so
> it knows if it's cancelled the migration.
Yeah, got it.
> >> +Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
> >> +and CANCELED migration for target).
> >> +
> >> +Data: None
> >> +
> >> +Example:
> >> +
> >> +{ "event": "MIGRATION_CANCELED",
> >> +"timestamp": {"seconds": 1274687575, "microseconds": 592483} }
> >> +
> >> +MIGRATION_ENDED
> >> +---
> >> +
> >> +Emitted when migration ends (both in source and target)
> >> +
> >> +Data: None
> >> +
> >> +Example:
> >> +
> >> +{ "event": "MIGRATION_ENDED",
> >> +"timestamp": {"seconds": 1274687575, "microseconds": 592483} }
> >> +
> >> +MIGRATION_FAILED
> >> +
> >> +
> >> +Emitted when migration fails (both is source and target).
> >> +
> >> +Data: None
> >> +
> >> +Example:
> >> +
> >> +{ "event": "MIGRATION_FAILED",
> >> +"timestamp": {"seconds": 1274687575, "microseconds": 592483} }
> >>
> > What about a MIGRATION_FINISHED event, which contains a 'success'
> > key which is a bool?
> >
> > The only disadvantage of this is if we decide to add more information
> > to the event (say, stats) then it'd get ugly. Otherwise, one event is
> > enough.
> >
> > Anyway, the counterpart of MIGRATION_FAILED is MIGRATION_SUCCEEDED.
> >
>
> I see MIGRATION_FAILED as being very similar to block I/O error events.
> I think we'll need a very similar solution for both. It boils down to,
> how do we raise asynchronous events when something fails?
You mean we should have a sort of error specific events?
Re: [Qemu-devel] RFC: ehci -> uhci handoff suggestions
Am 26.05.2010 15:06, schrieb David S. Ahern: > > > On 05/26/2010 06:48 AM, Gerd Hoffmann wrote: >> >> Hi, >> USB devices can support both 1.1 and 2.0, right? Who decides which protocol is used then? I think the OS can speak 1.1 to the device even in case a ehci controller is present (but unused by the OS), right? >>> >>> AFAIK the OS must tell the EHCI that it should hand the device off to >>> the UHCI/OHCI companion before it can use it there. >> >> Huh? Compatibility-wise it makes sense to do it the other way around >> (i.e. have it @ UHCI/OHCI by default and move to EHCI on request), so a >> OS which knows nothing about EHCI can cope. Ah, the page referenced by David explains this, so what I knew is only half of it. There is a Configured Flag that tells if the EHCI is used - and only when the OS has activated the EHCI this way it needs to explicitly hand off per device. >>> If they should be accessed via the EHCI or a companion controller >>> depends on what the OS requests. And USB 2.0 says that any device that >>> supports High Speed must also support Full Speed and therefore be >>> accessible using the companion (at least that's what I understand). >> >> Hmm, ok, so no shortcut even for emulated devices. Not that it would >> have helped much as we have to cover host devices anyway. >> >> Also I think one ehci controller can have multiple uhci companion >> controllers. At least lspci on my T60 suggests that: Yes, I think any number is allowed, and from a specification point of view it's even okay to have no companion controller at all. You just couldn't use Low/Full Speed devices in the ports of that controller then. >> 00:1d.0 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI >> Controller #1 (rev 02) >> 00:1d.1 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI >> Controller #2 (rev 02) >> 00:1d.2 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI >> Controller #3 (rev 02) >> 00:1d.3 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI >> Controller #4 (rev 02) >> 00:1d.7 USB Controller: Intel Corporation 82801G (ICH7 Family) USB2 EHCI >> Controller (rev 02) >> >> cheers, >> Gerd >> > > Yes, that is the ehci feature to be implemented. > > My understanding is that the port routing happens internally to the host > controller based on device speed - section 4.2 (pag 64) of: > http://www.intel.com/technology/usb/download/ehci-r10.pdf The routing may happen internally, but the OHCI/UHCI appears just like a normal controller to the OS. You can't access the devices on a companion with your EHCI driver. > ehci does have more overhead from an emulation perspective, so it would > be best to keep mice, keyboard, serial ports, etc on the uhci/ohci bus > and have storage devices and webcams and such on ehci. And any > transition should happen automagically within the device model. I think in reality things like keyboards are Low Speed anyway, so they would need to be handed off to a OHCI/UHCI anyway. Any transition between High Speed (directly handled by EHCI) and Low/Full Speed (OHCI/UHCI companion controller) must not happen automagically, but be requested by the guest OS. And you probably don't want to re-implement UHCI or OHCI inside the EHCI emulation, so you can't keep things inside the EHCI device model. Kevin
Re: [Qemu-devel] [PATCH] all vga: refuse hotplugging.
Hi, I think having two gfx screens mapped to two qemu consoles, then be able to switch between them via Ctrl-Alt- (like you switch today to text consoles) could be doable without too much effort. Question is how useful this would be as you can't see your two screens at the same time. Actually I was thinking of registering multiple graphic consoles, each of them could be rendered by a different frontend (sdl/vnc) independently. We would have multiple DisplayStates for that. Possible, but certainly alot of work. Tons of code in qemu can't handle multiple DisplayStates. Try it and you'll see. With qxl+spice the spice client will open a new window for the secondary display. With vnc+sdl you'll see the primary display only. So you are doing exactly what I wrote above, right? No. The secondary display has no DisplayState. That is the reason why only the spice client will see it. cheers, Gerd
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On 05/26/2010 03:43 AM, Kevin Wolf wrote: Am 26.05.2010 03:31, schrieb Anthony Liguori: On 05/25/2010 04:01 PM, Aurelien Jarno wrote: I really think this patch can be useful, in my own case when testing debian-installer (I already cache=writeback). In short all that is about developing and testing, as opposed to run a VM in production, can benefit about that. This was one of the original use case of QEMU before KVM arrived. Unless someone can convince me not to do it, I seriously considering applying this patch. There really needs to be an indication in the --help output of what the ramifications of this option are, in the very least. It should also be removable via a ./configure option because no sane distribution should enable this for end users. We know better what you stupid user want? What percentage of qemu users do you think have actually read qemu-doc.texi? It's not a stretch for someone to have heard that cache options can improve performance, and then see cache=volatile in the help output, try it, and then start using it because they observe a performance improvement. That's not being stupid. I think it's a reasonable expectation for a user to have that their data is safe. Regards, Anthony Liguori There are valid use cases for this cache option, most notably installation. I could agree with requesting that the option should be called cache=unsafe (or even Alex' cache=destroys_your_image), but that should really be enough to tell everyone that his data is not safe with this option. Kevin
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On 05/26/2010 03:52 AM, Aurelien Jarno wrote: On Tue, May 25, 2010 at 08:31:20PM -0500, Anthony Liguori wrote: On 05/25/2010 04:01 PM, Aurelien Jarno wrote: I really think this patch can be useful, in my own case when testing debian-installer (I already cache=writeback). In short all that is about developing and testing, as opposed to run a VM in production, can benefit about that. This was one of the original use case of QEMU before KVM arrived. Unless someone can convince me not to do it, I seriously considering applying this patch. There really needs to be an indication in the --help output of what the ramifications of this option are, in the very least. It should That's indeed something than can be done, but to avoid double standards, it should also be done for other features that can lead to data corruption. I am talking for example on the qcow format, which is not really supported anymore. I agree. also be removable via a ./configure option because no sane distribution should enable this for end users. I totally disagree. All the examples I have given apply to qemu *users*, not qemu developers. They surely don't want to recompile qemu for their usage. Note also that what is proposed in this patch was the default not too long ago, and that a lot of users complained about the new default for their usage, they see it as a regression. We even had to put a note explaining that in the Debian package to avoid to many bug reports. cache=writeback only answer partially to this use case. It's hard for me to consider this a performance regression because ultimately, you're getting greater than bare metal performance (because of extremely aggressive caching). It might be a regression from the previous performance, but that was at the cost of safety. We might get 100 bug reports about this "regression" but they concern much less than 1 bug report of image corruption because of power failure/host crash. A reputation of being unsafe is very difficult to get rid of and is something that I hear concerns about frequently. I'm not suggesting that the compile option should be disabled by default upstream. But the option should be there for distributions because I hope that any enterprise distro disables it. Regards, Anthony Liguori
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On 05/26/2010 08:06 AM, Avi Kivity wrote: On 05/17/2010 03:58 PM, Anthony Liguori wrote: On 05/17/2010 05:14 AM, Alexander Graf wrote: Usually the guest can tell the host to flush data to disk. In some cases we don't want to flush though, but try to keep everything in cache. So let's add a new cache value to -drive that allows us to set the cache policy to most aggressive, disabling flushes. We call this mode "volatile", as guest data is not guaranteed to survive host crashes anymore. This patch also adds a noop function for aio, so we can do nothing in AIO fashion. Signed-off-by: Alexander Graf I'd like to see some performance data with at least an ext3 host file system and an ext4 file system. My concern is that ext3 exaggerates the cost of fsync() which will result in diminishing value over time for this feature as people move to ext4/btrfs. In fact, btrfs is currently unusable for virt because O_SYNC writes inflate a guest write to a host write. by a huge factor (50x-100x). cache=writethrough is 100% unusable, cache=writeback is barely tolerable. As of 2.6.32, cache=volatile is probably required to get something resembling reasonable performance on btrfs. Of course, we expect that btrfs will improve in time, but still it doesn't seem to be fsync friendly. So you're suggesting that anyone who uses virt on btrfs should be prepared to deal with data corruption on host failure? That sounds to me like btrfs isn't ready for real workloads. Regards, Anthony Liguori
[Qemu-devel] [Bug 532733] Re: apt/dpkg in qemu-system-arm hangs if a big task is installed
beagleboard is not in upstream QEMU. Please do not mark bugs as affects upstream QEMU unless you've actually reproduced the problem with upstream QEMU. ** Changed in: qemu Status: New => Invalid -- apt/dpkg in qemu-system-arm hangs if a big task is installed https://bugs.launchpad.net/bugs/532733 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Invalid Status in “qemu-kvm” package in Ubuntu: Incomplete Status in “qemu-kvm” source package in Lucid: Incomplete Bug description: Binary package hint: qemu-kvm running rootstock and installing ubuntu-netbook^ makes the VM hang in "unpacking iso-codes" this is reproducable every time in rootstock as well as in a standard qemu-system-arm vm that contains a minimal ubuntu with running apt-get install ubuntu-netbook
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
Am 26.05.2010 15:42, schrieb Anthony Liguori: > On 05/26/2010 03:43 AM, Kevin Wolf wrote: >> Am 26.05.2010 03:31, schrieb Anthony Liguori: >> >>> On 05/25/2010 04:01 PM, Aurelien Jarno wrote: >>> I really think this patch can be useful, in my own case when testing debian-installer (I already cache=writeback). In short all that is about developing and testing, as opposed to run a VM in production, can benefit about that. This was one of the original use case of QEMU before KVM arrived. Unless someone can convince me not to do it, I seriously considering applying this patch. >>> There really needs to be an indication in the --help output of what the >>> ramifications of this option are, in the very least. It should also be >>> removable via a ./configure option because no sane distribution should >>> enable this for end users. >>> >> We know better what you stupid user want? > > What percentage of qemu users do you think have actually read qemu-doc.texi? As I said, put the warning in the option name like cache=unsafe or something even more scary and I'm all for it. > It's not a stretch for someone to have heard that cache options can > improve performance, and then see cache=volatile in the help output, try > it, and then start using it because they observe a performance improvement. > > That's not being stupid. I think it's a reasonable expectation for a > user to have that their data is safe. You seem to think that the user is too stupid to allow him to use this option even if he's perfectly aware what it's doing. It's a useful option if it's used right. We need to make clear that it's dangerous when it's used in the wrong cases (for example by naming), but just disabling is not a solution for that. You don't suggest that "no sane distribution" should ship rm, because it's dangerous if you use it wrong, do you? Kevin
Re: [Qemu-devel] RFC: ehci -> uhci handoff suggestions
On 05/26/2010 07:23 AM, Kevin Wolf wrote: > Am 26.05.2010 15:06, schrieb David S. Ahern: >> >> >> On 05/26/2010 06:48 AM, Gerd Hoffmann wrote: >>> >>> Hi, >>> > USB devices can support both 1.1 and 2.0, right? Who decides which > protocol is used then? I think the OS can speak 1.1 to the device even > in case a ehci controller is present (but unused by the OS), right? AFAIK the OS must tell the EHCI that it should hand the device off to the UHCI/OHCI companion before it can use it there. >>> >>> Huh? Compatibility-wise it makes sense to do it the other way around >>> (i.e. have it @ UHCI/OHCI by default and move to EHCI on request), so a >>> OS which knows nothing about EHCI can cope. > > Ah, the page referenced by David explains this, so what I knew is only > half of it. There is a Configured Flag that tells if the EHCI is used - > and only when the OS has activated the EHCI this way it needs to > explicitly hand off per device. > If they should be accessed via the EHCI or a companion controller depends on what the OS requests. And USB 2.0 says that any device that supports High Speed must also support Full Speed and therefore be accessible using the companion (at least that's what I understand). >>> >>> Hmm, ok, so no shortcut even for emulated devices. Not that it would >>> have helped much as we have to cover host devices anyway. >>> >>> Also I think one ehci controller can have multiple uhci companion >>> controllers. At least lspci on my T60 suggests that: > > Yes, I think any number is allowed, and from a specification point of > view it's even okay to have no companion controller at all. You just > couldn't use Low/Full Speed devices in the ports of that controller then. > >>> 00:1d.0 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI >>> Controller #1 (rev 02) >>> 00:1d.1 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI >>> Controller #2 (rev 02) >>> 00:1d.2 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI >>> Controller #3 (rev 02) >>> 00:1d.3 USB Controller: Intel Corporation 82801G (ICH7 Family) USB UHCI >>> Controller #4 (rev 02) >>> 00:1d.7 USB Controller: Intel Corporation 82801G (ICH7 Family) USB2 EHCI >>> Controller (rev 02) >>> >>> cheers, >>> Gerd >>> >> >> Yes, that is the ehci feature to be implemented. >> >> My understanding is that the port routing happens internally to the host >> controller based on device speed - section 4.2 (pag 64) of: >> http://www.intel.com/technology/usb/download/ehci-r10.pdf > > The routing may happen internally, but the OHCI/UHCI appears just like a > normal controller to the OS. You can't access the devices on a companion > with your EHCI driver. > >> ehci does have more overhead from an emulation perspective, so it would >> be best to keep mice, keyboard, serial ports, etc on the uhci/ohci bus >> and have storage devices and webcams and such on ehci. And any >> transition should happen automagically within the device model. > > I think in reality things like keyboards are Low Speed anyway, so they > would need to be handed off to a OHCI/UHCI anyway. > > Any transition between High Speed (directly handled by EHCI) and > Low/Full Speed (OHCI/UHCI companion controller) must not happen > automagically, but be requested by the guest OS. And you probably don't > want to re-implement UHCI or OHCI inside the EHCI emulation, so you > can't keep things inside the EHCI device model. > > Kevin I'm still confused by the guest OS interaction -- more code/spec reading I guess. Key points are that lspci in the VM shows both buses, and the qemu monitor would still scan both buses and show devices. And definitely no code duplication - some kind of movement to current uhci/ohci ports is what I am after. David
[Qemu-devel] [Bug 532733] Re: apt/dpkg in qemu-system-arm hangs if a big task is installed
@Anthony, this bug has nothing to do with beagleboards it happens if qemu is run on x86 systems -- apt/dpkg in qemu-system-arm hangs if a big task is installed https://bugs.launchpad.net/bugs/532733 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Invalid Status in “qemu-kvm” package in Ubuntu: Incomplete Status in “qemu-kvm” source package in Lucid: Incomplete Bug description: Binary package hint: qemu-kvm running rootstock and installing ubuntu-netbook^ makes the VM hang in "unpacking iso-codes" this is reproducable every time in rootstock as well as in a standard qemu-system-arm vm that contains a minimal ubuntu with running apt-get install ubuntu-netbook
[Qemu-devel] [Bug 532733] Re: apt/dpkg in qemu-system-arm hangs if a big task is installed
@Anthony please see all the above comments before judging and please reopen it upstream again -- apt/dpkg in qemu-system-arm hangs if a big task is installed https://bugs.launchpad.net/bugs/532733 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Invalid Status in “qemu-kvm” package in Ubuntu: Incomplete Status in “qemu-kvm” source package in Lucid: Incomplete Bug description: Binary package hint: qemu-kvm running rootstock and installing ubuntu-netbook^ makes the VM hang in "unpacking iso-codes" this is reproducable every time in rootstock as well as in a standard qemu-system-arm vm that contains a minimal ubuntu with running apt-get install ubuntu-netbook
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On 05/26/2010 09:12 AM, Aurelien Jarno wrote: It's hard for me to consider this a performance regression because ultimately, you're getting greater than bare metal performance (because of extremely aggressive caching). It might be a regression from the previous performance, but that was at the cost of safety. For people who don't care about safety it's still a regression. And it is a common usage of QEMU. It's not a functional change. It's a change in performance. There are tons of changes in performance characteristics of qemu from version to version. It's not even a massive one. We might get 100 bug reports about this "regression" but they concern much less than 1 bug report of image corruption because of power failure/host crash. A reputation of being unsafe is very difficult to get rid of and is something that I hear concerns about frequently. I'm not suggesting that the compile option should be disabled by default upstream. But the option should be there for distributions because I hope that any enterprise distro disables it. Which basically means those distro don't care about some use cases of QEMU, that were for most of them the original uses cases. It's sad. This isn't a feature. This is a change in performance. No one is not able to satisfy their use case from this behavior. Sometimes I really whishes that KVM never tried to reintegrate code into QEMU, it doesn't bring only good things. I highly doubt that this is even visible on benchmarks without using KVM. The improvement on a microbenchmark was relatively small and the cost from TCG would almost certainly dwarf it. Also, remember before KVM, we had single threaded IO and posix-aio (which is still single threaded). If KVM never happened, block performance would be far, far worse than it is today with cache=writeback. Regards, Anthony Liguori
[Qemu-devel] [PATCH 11/14] introduce more --xyzdir options
Signed-off-by: Paolo Bonzini
---
configure | 26 +-
1 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/configure b/configure
index 4dc75c2..2e59f9b 100755
--- a/configure
+++ b/configure
@@ -68,6 +68,10 @@ cpu=""
prefix=""
interp_prefix="/usr/gnemul/qemu-%M"
static="no"
+mandir=""
+datadir=""
+docdir=""
+bindir=""
sysconfdir=""
sparc_cpu=""
cross_prefix=""
@@ -501,6 +505,14 @@ for opt do
static="yes"
LDFLAGS="-static $LDFLAGS"
;;
+ --mandir=*) mandir="$optarg"
+ ;;
+ --bindir=*) bindir="$optarg"
+ ;;
+ --datadir=*) datadir="$optarg"
+ ;;
+ --docdir=*) docdir="$optarg"
+ ;;
--sysconfdir=*) sysconfdir="$optarg"
;;
--disable-sdl) sdl="no"
@@ -755,7 +767,11 @@ echo " --extra-ldflags=LDFLAGS append extra linker flags
LDFLAGS"
echo " --make=MAKE use specified make [$make]"
echo " --install=INSTALLuse specified install [$install]"
echo " --static enable static build [$static]"
-echo " --sysconfdir=PATHinstall config in PATH"
+echo " --mandir=PATHinstall man pages in PATH"
+echo " --datadir=PATH install firmware in PATH"
+echo " --docdir=PATHinstall documentation in PATH"
+echo " --bindir=PATHinstall binaries in PATH"
+echo " --sysconfdir=PATHinstall config in PATH/qemu"
echo " --enable-debug-tcg enable TCG debugging"
echo " --disable-debug-tcg disable TCG debugging (default)"
echo " --enable-debug enable common debug build options"
@@ -1985,10 +2001,10 @@ else
confsuffix="/qemu"
fi
-mandir="\${prefix}$mansuffix"
-datadir="\${prefix}$datasuffix"
-docdir="\${prefix}$docsuffix"
-bindir="\${prefix}$binsuffix"
+: ${mandir:="\${prefix}$mansuffix"}
+: ${datadir:="\${prefix}$datasuffix"}
+: ${docdir:="\${prefix}$docsuffix"}
+: ${bindir:="\${prefix}$binsuffix"}
: ${sysconfdir:="\${prefix}$sysconfsuffix"}
confdir=$sysconfdir$confsuffix
--
1.6.6.1
[Qemu-devel] Re: migrating guest with msi-x interrupts
On Tue, May 25, 2010 at 04:09:13PM -0600, Cam Macdonell wrote:
> Hi,
>
> I'm trying to migrate a guest device with MSI-X interrupts. However,
> the interrupts are not injected into the guest. I've added some
> tracing to msix.c and it seems that the MSI-X vectors are masked when
> the guest is resumed (I'm testing with static migration).
>
> In particular, in msix.c, msix_is_masked(...) is returning true when
> the guest resumes which causes msix_set_pending() to be called instead
> of msix_set_irq().
>
> /* Send an MSI-X message */
> void msix_notify(PCIDevice *dev, unsigned vector)
> {
> uint8_t *table_entry = dev->msix_table_page + vector * MSIX_ENTRY_SIZE;
> uint64_t address;
> uint32_t data;
>
> if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector])
> return;
>
> if (msix_is_masked(dev, vector)) {
> msix_set_pending(dev, vector);
> return;
> }
>
> ...
>
> Does migrating a guest device that uses MSI-X require
> msix_load()/save() to be called explicity in a pre/post_save/load
> function?
>
> Any pointers or comments would be helpful,
> Cam
Yes. Look at how virtio does this.
Incidentially, I think reusing virtio for configuration would save
effort, and have other advantages such as making it possible to support
non-pci guests.
--
MST
[Qemu-devel] [PATCH 07/14] rename CONFIG_QEMU_PREFIX
Signed-off-by: Paolo Bonzini --- bsd-user/main.c |2 +- configure |2 +- linux-user/main.c |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bsd-user/main.c b/bsd-user/main.c index 05cc3d9..aff9f13 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -43,7 +43,7 @@ unsigned long guest_base; int have_guest_base; #endif -static const char *interp_prefix = CONFIG_QEMU_PREFIX; +static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX; const char *qemu_uname_release = CONFIG_UNAME_RELEASE; extern char **environ; enum BSDType bsd_type; diff --git a/configure b/configure index 435e765..c561132 100755 --- a/configure +++ b/configure @@ -2427,7 +2427,7 @@ echo "# Automatically generated by configure - do not modify" > $config_target_m bflt="no" target_nptl="no" interp_prefix1=`echo "$interp_prefix" | sed "s/%M/$target_arch2/g"` -echo "CONFIG_QEMU_PREFIX=\"$interp_prefix1\"" >> $config_target_mak +echo "CONFIG_QEMU_INTERP_PREFIX=\"$interp_prefix1\"" >> $config_target_mak gdb_xml_files="" TARGET_ARCH="$target_arch2" diff --git a/linux-user/main.c b/linux-user/main.c index b240f29..456feb0 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -46,7 +46,7 @@ unsigned long guest_base; int have_guest_base; #endif -static const char *interp_prefix = CONFIG_QEMU_PREFIX; +static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX; const char *qemu_uname_release = CONFIG_UNAME_RELEASE; /* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so -- 1.6.6.1
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On 05/26/2010 09:03 AM, Kevin Wolf wrote: Am 26.05.2010 15:42, schrieb Anthony Liguori: On 05/26/2010 03:43 AM, Kevin Wolf wrote: Am 26.05.2010 03:31, schrieb Anthony Liguori: On 05/25/2010 04:01 PM, Aurelien Jarno wrote: I really think this patch can be useful, in my own case when testing debian-installer (I already cache=writeback). In short all that is about developing and testing, as opposed to run a VM in production, can benefit about that. This was one of the original use case of QEMU before KVM arrived. Unless someone can convince me not to do it, I seriously considering applying this patch. There really needs to be an indication in the --help output of what the ramifications of this option are, in the very least. It should also be removable via a ./configure option because no sane distribution should enable this for end users. We know better what you stupid user want? What percentage of qemu users do you think have actually read qemu-doc.texi? As I said, put the warning in the option name like cache=unsafe or something even more scary and I'm all for it. It's not a stretch for someone to have heard that cache options can improve performance, and then see cache=volatile in the help output, try it, and then start using it because they observe a performance improvement. That's not being stupid. I think it's a reasonable expectation for a user to have that their data is safe. You seem to think that the user is too stupid to allow him to use this option even if he's perfectly aware what it's doing. It's a useful option if it's used right. No, that's not what I said. I'm saying we need to try hard to make a user aware of what they're doing. If it spit out a warning on stdio, I wouldn't think a compile option is needed. Even with help output text, I'm concerned that someone is going to find a bad example on the internet. cache=unsafe addresses the problem although I think it's a bit hokey. We need to make clear that it's dangerous when it's used in the wrong cases (for example by naming), but just disabling is not a solution for that. You don't suggest that "no sane distribution" should ship rm, because it's dangerous if you use it wrong, do you? You realize that quite a lot of distributions carry a patch to rm that prevents a user from doing rm -rf /? Regards, Anthony Liguori Kevin
[Qemu-devel] [PATCH 13/14] move directory defaults earlier
Unify with existing special-purpose configure code for win32.
Signed-off-by: Paolo Bonzini
---
configure | 47 ++-
1 files changed, 14 insertions(+), 33 deletions(-)
diff --git a/configure b/configure
index b036c40..488ef07 100755
--- a/configure
+++ b/configure
@@ -65,14 +65,8 @@ path_of() {
# default parameters
cpu=""
-prefix=""
interp_prefix="/usr/gnemul/qemu-%M"
static="no"
-mandir=""
-datadir=""
-docdir=""
-bindir=""
-sysconfdir=""
sparc_cpu=""
cross_prefix=""
cc="gcc"
@@ -280,6 +274,13 @@ strip_opt="yes"
bigendian="no"
mingw32="no"
EXESUF=""
+prefix="/usr/local"
+mandir="\${prefix}/share/man"
+datadir="\${prefix}/share/qemu"
+docdir="\${prefix}/share/doc/qemu"
+bindir="\${prefix}/bin"
+sysconfdir="\${prefix}/etc"
+confsuffix="/qemu"
slirp="yes"
fmod_lib=""
fmod_inc=""
@@ -454,6 +455,13 @@ if test "$mingw32" = "yes" ; then
# enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS"
LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS"
+ prefix="c:/Program Files/Qemu"
+ mandir="\${prefix}"
+ datadir="\${prefix}"
+ docdir="\${prefix}"
+ bindir="\${prefix}"
+ sysconfdir="\${prefix}"
+ confsuffix=""
fi
# find source path
@@ -1981,33 +1989,6 @@ if test "$solaris" = "no" ; then
fi
fi
-if test "$mingw32" = "yes" ; then
- if test -z "$prefix" ; then
- prefix="c:/Program Files/Qemu"
- fi
- mansuffix=""
- datasuffix=""
- docsuffix=""
- binsuffix=""
- sysconfsuffix=""
- confsuffix=""
-else
- if test -z "$prefix" ; then
- prefix="/usr/local"
- fi
- mansuffix="/share/man"
- datasuffix="/share/qemu"
- docsuffix="/share/doc/qemu"
- binsuffix="/bin"
- sysconfsuffix="/etc"
- confsuffix="/qemu"
-fi
-
-: ${mandir:="\${prefix}$mansuffix"}
-: ${datadir:="\${prefix}$datasuffix"}
-: ${docdir:="\${prefix}$docsuffix"}
-: ${bindir:="\${prefix}$binsuffix"}
-: ${sysconfdir:="\${prefix}$sysconfsuffix"}
confdir=$sysconfdir$confsuffix
echo "Install prefix$prefix"
--
1.6.6.1
[Qemu-devel] [PATCH 00/14] configure --xyzdir options cleanup
This series cleans up the handling of --xyzdir options and
improves the customizability of the directory layout.
Patches 1/2/3/14 are somewhat unrelated to the main purpose
of the patch, but they conflict with other patches in the
series so I sent them together.
Paolo Bonzini (14):
bail out early on invalid -cpu option
avoid using expr in configure
dyngen is long time gone
delete duplicate create_config case stanza
move all directory entries in config-host.mak close
introduce sysconfsuffix
introduce confdir and confsuffix
rename CONFIG_QEMU_PREFIX
expand ${prefix} in create_config
unify handling of xyzdir variables
introduce more --xyzdir options
ignore unknown --xyzdir options
move directory defaults earlier
move computation of tools and roms outside of config-host.mak generation
bsd-user/main.c |2 +-
configure | 138 +++--
create_config | 12 +++--
linux-user/main.c |2 +-
vl.c |2 +-
5 files changed, 82 insertions(+), 74 deletions(-)
[Qemu-devel] [Bug 532733] Re: apt/dpkg in qemu-system-arm hangs if a big task is installed
and just for reference so there isnt coming up any confusion again, the used qemu call that breaks is for a versatilbepb machine using the versatile kernel from http://ports.ubuntu.com/ubuntu- ports/dists/lucid/main/installer- armel/current/images/versatile/netboot/vmlinuz -- apt/dpkg in qemu-system-arm hangs if a big task is installed https://bugs.launchpad.net/bugs/532733 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Invalid Status in “qemu-kvm” package in Ubuntu: Incomplete Status in “qemu-kvm” source package in Lucid: Incomplete Bug description: Binary package hint: qemu-kvm running rootstock and installing ubuntu-netbook^ makes the VM hang in "unpacking iso-codes" this is reproducable every time in rootstock as well as in a standard qemu-system-arm vm that contains a minimal ubuntu with running apt-get install ubuntu-netbook
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
Am 26.05.2010 16:08, schrieb Anthony Liguori: > On 05/26/2010 09:03 AM, Kevin Wolf wrote: >> Am 26.05.2010 15:42, schrieb Anthony Liguori: >> >>> On 05/26/2010 03:43 AM, Kevin Wolf wrote: >>> Am 26.05.2010 03:31, schrieb Anthony Liguori: > On 05/25/2010 04:01 PM, Aurelien Jarno wrote: > > >> I really think this patch can be useful, in my own case when testing >> debian-installer (I already cache=writeback). In short all that is about >> developing and testing, as opposed to run a VM in production, can >> benefit about that. This was one of the original use case of QEMU before >> KVM arrived. >> >> Unless someone can convince me not to do it, I seriously considering >> applying this patch. >> >> >> > There really needs to be an indication in the --help output of what the > ramifications of this option are, in the very least. It should also be > removable via a ./configure option because no sane distribution should > enable this for end users. > > We know better what you stupid user want? >>> What percentage of qemu users do you think have actually read qemu-doc.texi? >>> >> As I said, put the warning in the option name like cache=unsafe or >> something even more scary and I'm all for it. >> >> >>> It's not a stretch for someone to have heard that cache options can >>> improve performance, and then see cache=volatile in the help output, try >>> it, and then start using it because they observe a performance improvement. >>> >>> That's not being stupid. I think it's a reasonable expectation for a >>> user to have that their data is safe. >>> >> You seem to think that the user is too stupid to allow him to use this >> option even if he's perfectly aware what it's doing. It's a useful >> option if it's used right. >> > > No, that's not what I said. I'm saying we need to try hard to make a > user aware of what they're doing. > > If it spit out a warning on stdio, I wouldn't think a compile option is > needed. Even with help output text, I'm concerned that someone is going > to find a bad example on the internet. > > cache=unsafe addresses the problem although I think it's a bit hokey. Then let's do it this way. I'm not opposed to a stdio message either, even though I don't think it's really necessary with a name like cache=unsafe. I just say that disabling the option is not a solution because it prevents valid use. >> We need to make clear that it's dangerous when it's used in the wrong >> cases (for example by naming), but just disabling is not a solution for >> that. You don't suggest that "no sane distribution" should ship rm, >> because it's dangerous if you use it wrong, do you? >> > > You realize that quite a lot of distributions carry a patch to rm that > prevents a user from doing rm -rf /? Most rm invocations that I regretted later were not rm -rf /. Actually, I think rm -rf / is not among them at all. ;-) And I seem to remember that even these rm patches still allow the protection to be overridden by some force flag. But I've never tried it out. Kevin
[Qemu-devel] [PATCH 6/8] enable event_notifier to use pipes
Signed-off-by: Paolo Bonzini
---
event_notifier.c | 69 +++---
event_notifier.h |3 +-
2 files changed, 52 insertions(+), 20 deletions(-)
diff --git a/event_notifier.c b/event_notifier.c
index 066adb9..a33f3c5 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -10,51 +10,82 @@
* the COPYING file in the top-level directory.
*/
+#include "qemu-common.h"
#include "event_notifier.h"
#include "qemu-char.h"
-#ifdef CONFIG_EVENTFD
-#include
-#endif
int event_notifier_init(EventNotifier *e, int active)
{
-#ifdef CONFIG_EVENTFD
-int fd = eventfd(!!active, EFD_NONBLOCK | EFD_CLOEXEC);
-if (fd < 0)
+int fds[2];
+int err;
+if (qemu_eventfd (fds) < 0)
return -errno;
-e->fd = fd;
+
+err = fcntl_setfl(fds[0], O_NONBLOCK);
+if (err < 0)
+goto fail;
+
+err = fcntl_setfl(fds[1], O_NONBLOCK);
+if (err < 0)
+goto fail;
+
+e->rfd = fds[0];
+e->wfd = fds[1];
+if (active)
+event_notifier_set(e);
return 0;
-#else
-return -ENOSYS;
-#endif
+
+fail:
+close(fds[0]);
+close(fds[1]);
+return err;
}
void event_notifier_cleanup(EventNotifier *e)
{
-close(e->fd);
+close(e->rfd);
+close(e->wfd);
}
int event_notifier_get_fd(EventNotifier *e)
{
-return e->fd;
+return e->wfd;
}
int event_notifier_set_handler(EventNotifier *e,
EventNotifierHandler *handler)
{
-return qemu_set_fd_handler(e->fd, (IOHandler *)handler, NULL, e);
+return qemu_set_fd_handler(e->rfd, (IOHandler *)handler, NULL, e);
}
int event_notifier_set(EventNotifier *e)
{
-uint64_t value = 1;
-int r = write(e->fd, &value, sizeof(value));
-return r == sizeof(value);
+static const uint64_t value = 1;
+ssize_t ret;
+
+do {
+ret = write(e->wfd, &value, sizeof(value));
+} while (ret < 0 && errno == EINTR);
+
+/* EAGAIN is fine, a read must be pending. */
+if (ret < 0 && errno != EAGAIN) {
+return -1;
+}
+return 0;
}
int event_notifier_test_and_clear(EventNotifier *e)
{
-uint64_t value;
-int r = read(e->fd, &value, sizeof(value));
-return r == sizeof(value);
+int value;
+ssize_t len;
+char buffer[512];
+
+/* Drain the notify pipe. For eventfd, only 8 bytes will be read. */
+value = 0;
+do {
+len = read(e->rfd, buffer, sizeof(buffer));
+value |= (len > 0);
+} while ((len == -1 && errno == EINTR) || len == sizeof(buffer));
+
+return value;
}
diff --git a/event_notifier.h b/event_notifier.h
index f6ec9ef..ff9d6f2 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -4,7 +4,8 @@
#include "qemu-common.h"
struct EventNotifier {
- int fd;
+int rfd;
+int wfd;
};
typedef void EventNotifierHandler(EventNotifier *);
--
1.6.6.1
[Qemu-devel] [PATCH 7/8] add Win32 implementation of event notifiers
Signed-off-by: Paolo Bonzini
---
Compile-tested only. iothread is broken anyway on Win32
due to missing implementation of qemu-threads.
event_notifier.c | 33 +
event_notifier.h |8
2 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/event_notifier.c b/event_notifier.c
index a33f3c5..0705dc5 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -11,11 +11,13 @@
*/
#include "qemu-common.h"
+#include "sysemu.h"
#include "event_notifier.h"
#include "qemu-char.h"
int event_notifier_init(EventNotifier *e, int active)
{
+#ifndef _WIN32
int fds[2];
int err;
if (qemu_eventfd (fds) < 0)
@@ -39,27 +41,50 @@ fail:
close(fds[0]);
close(fds[1]);
return err;
+#else
+e->event = CreateEvent(NULL, FALSE, FALSE, NULL);
+assert(e->event);
+return 0;
+#endif
}
void event_notifier_cleanup(EventNotifier *e)
{
+#ifndef _WIN32
close(e->rfd);
close(e->wfd);
+#else
+CloseHandle(e->event);
+#endif
}
int event_notifier_get_fd(EventNotifier *e)
{
+#ifndef _WIN32
return e->wfd;
+#else
+abort();
+#endif
}
int event_notifier_set_handler(EventNotifier *e,
EventNotifierHandler *handler)
{
+#ifndef _WIN32
return qemu_set_fd_handler(e->rfd, (IOHandler *)handler, NULL, e);
+#else
+if (handler) {
+return qemu_add_wait_object(e->event, (IOHandler *)handler, e);
+} else {
+qemu_del_wait_object(e->event, (IOHandler *)handler, e);
+return 0;
+}
+#endif
}
int event_notifier_set(EventNotifier *e)
{
+#ifndef _WIN32
static const uint64_t value = 1;
ssize_t ret;
@@ -72,10 +97,15 @@ int event_notifier_set(EventNotifier *e)
return -1;
}
return 0;
+#else
+SetEvent(e->event);
+return 0;
+#endif
}
int event_notifier_test_and_clear(EventNotifier *e)
{
+#ifndef _WIN32
int value;
ssize_t len;
char buffer[512];
@@ -88,4 +118,7 @@ int event_notifier_test_and_clear(EventNotifier *e)
} while ((len == -1 && errno == EINTR) || len == sizeof(buffer));
return value;
+#else
+return WaitForSingleObject(e->event, 0) == WAIT_OBJECT_0;
+#endif
}
diff --git a/event_notifier.h b/event_notifier.h
index ff9d6f2..f3c272a 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -3,9 +3,17 @@
#include "qemu-common.h"
+#ifdef _WIN32
+#include
+#endif
+
struct EventNotifier {
+#ifdef _WIN32
+HANDLE event;
+#else
int rfd;
int wfd;
+#endif
};
typedef void EventNotifierHandler(EventNotifier *);
--
1.6.6.1
[Qemu-devel] [PATCH 05/14] introduce sysconfsuffix
Signed-off-by: Paolo Bonzini
---
configure | 10 --
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/configure b/configure
index f96..cf19ebf 100755
--- a/configure
+++ b/configure
@@ -1972,9 +1972,7 @@ if test "$mingw32" = "yes" ; then
confsuffix=""
docsuffix=""
binsuffix=""
- if test -z "$sysconfdir" ; then
- sysconfdir="${prefix}"
- fi
+ sysconfsuffix=""
else
if test -z "$prefix" ; then
prefix="/usr/local"
@@ -1983,11 +1981,11 @@ else
datasuffix="/share/qemu"
docsuffix="/share/doc/qemu"
binsuffix="/bin"
- if test -z "$sysconfdir" ; then
- sysconfdir="${prefix}/etc"
- fi
+ sysconfsuffix="/etc"
fi
+: ${sysconfdir:="${prefix}$sysconfsuffix"}
+
echo "Install prefix$prefix"
echo "BIOS directory$prefix$datasuffix"
echo "binary directory $prefix$binsuffix"
--
1.6.6.1
[Qemu-devel] [PATCH 06/14] introduce confdir and confsuffix
confsuffix was write-only, flesh it out.
Signed-off-by: Paolo Bonzini
---
configure | 10 --
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/configure b/configure
index cf19ebf..435e765 100755
--- a/configure
+++ b/configure
@@ -1969,10 +1969,10 @@ if test "$mingw32" = "yes" ; then
fi
mansuffix=""
datasuffix=""
- confsuffix=""
docsuffix=""
binsuffix=""
sysconfsuffix=""
+ confsuffix=""
else
if test -z "$prefix" ; then
prefix="/usr/local"
@@ -1982,9 +1982,11 @@ else
docsuffix="/share/doc/qemu"
binsuffix="/bin"
sysconfsuffix="/etc"
+ confsuffix="/qemu"
fi
: ${sysconfdir:="${prefix}$sysconfsuffix"}
+confdir=$sysconfdir$confsuffix
echo "Install prefix$prefix"
echo "BIOS directory$prefix$datasuffix"
@@ -2062,11 +2064,7 @@ printf " '%s'" "$0" "$@" >> $config_host_mak
echo >> $config_host_mak
echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> $config_host_mak
-if test "$mingw32" = "yes" ; then
- echo "CONFIG_QEMU_CONFDIR=\"$sysconfdir\"" >> $config_host_mak
-else
- echo "CONFIG_QEMU_CONFDIR=\"${sysconfdir}/qemu\"" >> $config_host_mak
-fi
+echo "CONFIG_QEMU_CONFDIR=\"$confdir\"" >> $config_host_mak
case "$cpu" in
i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|ppc|ppc64|s390|s390x|sparc|sparc64)
--
1.6.6.1
[Qemu-devel] [PATCH 1/8] move event_notifier into the main directory
Signed-off-by: Paolo Bonzini --- hw/event_notifier.c => event_notifier.c |1 - hw/event_notifier.h => event_notifier.h |0 2 files changed, 0 insertions(+), 1 deletions(-) rename hw/event_notifier.c => event_notifier.c (98%) rename hw/event_notifier.h => event_notifier.h (100%) diff --git a/hw/event_notifier.c b/event_notifier.c similarity index 98% rename from hw/event_notifier.c rename to event_notifier.c index 13f3656..2c73555 100644 --- a/hw/event_notifier.c +++ b/event_notifier.c @@ -10,7 +10,6 @@ * the COPYING file in the top-level directory. */ -#include "hw.h" #include "event_notifier.h" #ifdef CONFIG_EVENTFD #include diff --git a/hw/event_notifier.h b/event_notifier.h similarity index 100% rename from hw/event_notifier.h rename to event_notifier.h -- 1.6.6.1
[Qemu-devel] [PATCH 02/14] avoid using expr in configure
Just a personal preference against duplicating hieroglyphics.
Signed-off-by: Paolo Bonzini
---
configure | 11 +--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index e8dd2ef..5a7cb6e 100755
--- a/configure
+++ b/configure
@@ -928,6 +928,13 @@ if test -z "$target_list" ; then
echo "No targets enabled"
exit 1
fi
+# see if system emulation was really requested
+case " $target_list " in
+ *"-softmmu "*) softmmu=yes
+ ;;
+ *) softmmu=no
+ ;;
+esac
feature_not_found() {
feature=$1
@@ -2282,7 +2289,7 @@ bsd)
esac
tools=
-if test `expr "$target_list" : ".*softmmu.*"` != 0 ; then
+if test "$softmmu" = yes ; then
tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
tools="qemu-nbd\$(EXESUF) $tools"
@@ -2298,7 +2305,7 @@ echo "TOOLS=$tools" >> $config_host_mak
roms=
if test \( "$cpu" = "i386" -o "$cpu" = "x86_64" \) -a \
"$targetos" != "Darwin" -a "$targetos" != "SunOS" -a \
-`expr "$target_list" : ".*softmmu.*"` != 0 ; then
+"$softmmu" = yes ; then
roms="optionrom"
fi
echo "ROMS=$roms" >> $config_host_mak
--
1.6.6.1
[Qemu-devel] [PATCH 10/14] expand ${prefix} in create_config
Signed-off-by: Paolo Bonzini
---
configure |3 +--
create_config |9 +
vl.c |2 +-
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 9736942..4dc75c2 100755
--- a/configure
+++ b/configure
@@ -2073,8 +2073,7 @@ echo "mandir=$mandir" >> $config_host_mak
echo "datadir=$datadir" >> $config_host_mak
echo "sysconfdir=$sysconfdir" >> $config_host_mak
echo "docdir=$docdir" >> $config_host_mak
-echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> $config_host_mak
-echo "CONFIG_QEMU_CONFDIR=\"$confdir\"" >> $config_host_mak
+echo "confdir=$confdir" >> $config_host_mak
case "$cpu" in
i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|ppc|ppc64|s390|s390x|sparc|sparc64)
diff --git a/create_config b/create_config
index edcad25..23c0cd5 100755
--- a/create_config
+++ b/create_config
@@ -13,6 +13,15 @@ case $line in
pkgversion=${line#*=}
echo "#define QEMU_PKGVERSION \"$pkgversion\""
;;
+ prefix=* | *dir=*) # directory configuration
+name=${line%=*}
+value=${line#*=}
+define_name=`echo $name | tr '[:lower:]' '[:upper:]'`
+eval "define_value=\"$value\""
+echo "#define CONFIG_QEMU_$define_name \"$define_value\""
+# save for the next definitions
+eval "$name=\$define_value"
+;;
CONFIG_AUDIO_DRIVERS=*)
drivers=${line#*=}
echo "#define CONFIG_AUDIO_DRIVERS \\"
diff --git a/vl.c b/vl.c
index 328395e..18bddde 100644
--- a/vl.c
+++ b/vl.c
@@ -3420,7 +3420,7 @@ int main(int argc, char **argv, char **envp)
}
/* If all else fails use the install patch specified when building. */
if (!data_dir) {
-data_dir = CONFIG_QEMU_SHAREDIR;
+data_dir = CONFIG_QEMU_DATADIR;
}
/*
--
1.6.6.1
[Qemu-devel] [PATCH 3/8] remove event_notifier_test
This is broken; since the eventfd is used in nonblocking mode there
is a race between reading and writing.
Signed-off-by: Paolo Bonzini
---
event_notifier.c | 15 ---
event_notifier.h |1 -
2 files changed, 0 insertions(+), 16 deletions(-)
diff --git a/event_notifier.c b/event_notifier.c
index 3f50568..0f7b8da 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -51,18 +51,3 @@ int event_notifier_test_and_clear(EventNotifier *e)
int r = read(e->fd, &value, sizeof(value));
return r == sizeof(value);
}
-
-int event_notifier_test(EventNotifier *e)
-{
-uint64_t value;
-int r = read(e->fd, &value, sizeof(value));
-if (r == sizeof(value)) {
-/* restore previous value. */
-int s = write(e->fd, &value, sizeof(value));
-/* never blocks because we use EFD_SEMAPHORE.
- * If we didn't we'd get EAGAIN on overflow
- * and we'd have to write code to ignore it. */
-assert(s == sizeof(value));
-}
-return r == sizeof(value);
-}
diff --git a/event_notifier.h b/event_notifier.h
index 8d5735f..714d690 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -12,6 +12,5 @@ void event_notifier_cleanup(EventNotifier *);
int event_notifier_get_fd(EventNotifier *);
int event_notifier_set(EventNotifier *);
int event_notifier_test_and_clear(EventNotifier *);
-int event_notifier_test(EventNotifier *);
#endif
--
1.6.6.1
[Qemu-devel] [PATCH 12/14] ignore unknown --xyzdir options
Signed-off-by: Paolo Bonzini --- configure |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/configure b/configure index 2e59f9b..b036c40 100755 --- a/configure +++ b/configure @@ -673,6 +673,8 @@ for opt do ;; --enable-vhost-net) vhost_net="yes" ;; + --*dir) + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac -- 1.6.6.1
[Qemu-devel] [PATCH 08/14] unify handling of xyzdir variables
Making an xyzdir variable for each directory prepares for the next
patches introducing config-host.h defines and configure options for them.
It also fixes the problem where overriding prefix at "make install"
time did not override it for sysconfdir.
Removes some of the differences between sysconfdir and other variables,
the rest will go away later.
Signed-off-by: Paolo Bonzini
---
configure | 20
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/configure b/configure
index c561132..fee9665 100755
--- a/configure
+++ b/configure
@@ -1985,14 +1985,18 @@ else
confsuffix="/qemu"
fi
-: ${sysconfdir:="${prefix}$sysconfsuffix"}
+mandir="\${prefix}$mansuffix"
+datadir="\${prefix}$datasuffix"
+docdir="\${prefix}$docsuffix"
+bindir="\${prefix}$binsuffix"
+: ${sysconfdir:="\${prefix}$sysconfsuffix"}
confdir=$sysconfdir$confsuffix
echo "Install prefix$prefix"
-echo "BIOS directory$prefix$datasuffix"
-echo "binary directory $prefix$binsuffix"
+echo "BIOS directory`eval echo $datadir`"
+echo "binary directory `eval echo $bindir`"
if test "$mingw32" = "no" ; then
-echo "Manual directory $prefix$mansuffix"
+echo "Manual directory `eval echo $mandir`"
echo "ELF interp prefix $interp_prefix"
fi
echo "Source path $source_path"
@@ -2308,11 +2312,11 @@ fi
echo "ROMS=$roms" >> $config_host_mak
echo "prefix=$prefix" >> $config_host_mak
-echo "bindir=\${prefix}$binsuffix" >> $config_host_mak
-echo "mandir=\${prefix}$mansuffix" >> $config_host_mak
-echo "datadir=\${prefix}$datasuffix" >> $config_host_mak
+echo "bindir=$bindir" >> $config_host_mak
+echo "mandir=$mandir" >> $config_host_mak
+echo "datadir=$datadir" >> $config_host_mak
echo "sysconfdir=$sysconfdir" >> $config_host_mak
-echo "docdir=\${prefix}$docsuffix" >> $config_host_mak
+echo "docdir=$docdir" >> $config_host_mak
echo "MAKE=$make" >> $config_host_mak
echo "INSTALL=$install" >> $config_host_mak
echo "INSTALL_DIR=$install -d -m0755 -p" >> $config_host_mak
--
1.6.6.1
[Qemu-devel] [PATCH 5/8] add and use event_notifier_set_handler
Signed-off-by: Paolo Bonzini
---
event_notifier.c |7 +++
event_notifier.h |3 +++
hw/virtio-pci.c |9 +++--
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/event_notifier.c b/event_notifier.c
index 0f7b8da..066adb9 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -11,6 +11,7 @@
*/
#include "event_notifier.h"
+#include "qemu-char.h"
#ifdef CONFIG_EVENTFD
#include
#endif
@@ -38,6 +39,12 @@ int event_notifier_get_fd(EventNotifier *e)
return e->fd;
}
+int event_notifier_set_handler(EventNotifier *e,
+ EventNotifierHandler *handler)
+{
+return qemu_set_fd_handler(e->fd, (IOHandler *)handler, NULL, e);
+}
+
int event_notifier_set(EventNotifier *e)
{
uint64_t value = 1;
diff --git a/event_notifier.h b/event_notifier.h
index 714d690..f6ec9ef 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -7,10 +7,13 @@ struct EventNotifier {
int fd;
};
+typedef void EventNotifierHandler(EventNotifier *);
+
int event_notifier_init(EventNotifier *, int active);
void event_notifier_cleanup(EventNotifier *);
int event_notifier_get_fd(EventNotifier *);
int event_notifier_set(EventNotifier *);
int event_notifier_test_and_clear(EventNotifier *);
+int event_notifier_set_handler(EventNotifier *, EventNotifierHandler *);
#endif
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 988c75c..a7ebc83 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -419,9 +419,8 @@ static unsigned virtio_pci_get_features(void *opaque)
return proxy->host_features;
}
-static void virtio_pci_guest_notifier_read(void *opaque)
+static void virtio_pci_guest_notifier_read(EventNotifier *n)
{
-EventNotifier *n = opaque;
VirtQueue *vq = virtqueue_from_guest_notifier(n);
if (event_notifier_test_and_clear(n)) {
virtio_irq(vq);
@@ -439,11 +438,9 @@ static int virtio_pci_set_guest_notifier(void *opaque, int
n, bool assign)
if (r < 0) {
return r;
}
-qemu_set_fd_handler(event_notifier_get_fd(notifier),
-virtio_pci_guest_notifier_read, NULL, notifier);
+event_notifier_set_handler(notifier, virtio_pci_guest_notifier_read);
} else {
-qemu_set_fd_handler(event_notifier_get_fd(notifier),
-NULL, NULL, NULL);
+event_notifier_set_handler(notifier, NULL);
event_notifier_cleanup(notifier);
}
--
1.6.6.1
[Qemu-devel] Re: [PATCH 3/5] QMP: Introduce MIGRATION events
On 05/26/2010 05:33 AM, Daniel P. Berrange wrote: I'm not sure why you would need a notification of when migration starts (since you know when you've started migration). But you don't know if the other end "knows" that it has also started. started is needed only in incoming part, because we don't have a monitor to ask if migration has started. If we ever want to get closer to allowing multiple monitors, or allowing apps to issue QMP commands directly via libvirt, then we still need the 'migration started' event on the source, because something else can have issued the 'migrate' command without the mgmt app knowing. Migration started doesn't help multiple monitors. You need locking of some sort. Part of the problem is the QMP migrate command is implemented as a synchronous command. It really ought to be an asynchronous command. That tells you when the migration has actually completed without polling. On the source end, I can't think of any events that would be useful. The migrate command can complete with a failure so that gives you failure notifications. On the destination side, we're really limited by the fact that we don't do live incoming migrations. The monitor doesn't get a chance to run at all with exec: migration, for instance. For tcp: and unix:, a CONNECTED event absolutely makes sense (every socket server should emit a CONNECTED event). Unfortunately, after CONNECTED you lose the monitor until migration is complete. If something bad happens, you have to exit qemu so once the monitor returns, migration has completed successfully. If we introduce live incoming migration, we'll need to rethink things. I would actually suggest that we deprecate the incoming command if we do that and make incoming migration a monitor command. I would think it should have the same semantics as migrate (as an asynchronous command). A CONNECTED event still makes sense for tcp and unix protocols but I don't think events make sense for start stop vs. an asynchronous command completion. Regards, Anthony Liguori MIGRATED_STARTED+STOPPED really *is* needed if we're to make QMP cope with all possible use cases. If we rely on inferring it from STOP+RESUME events, it is going to exclude a significant set of use cases, and likely result in this being proposed all over again in 12 months time :-( Regards, Daniel
[Qemu-devel] [PATCH 4/8] add and use virtqueue_from_guest_notifier
This changes the opaque pointer passed to the handler, from being
the virtqueue to being the eventnotifier. It is useful as soon as
the eventnotifier will be able to set its own (type-safe) handler.
Signed-off-by: Paolo Bonzini
---
I don't have a vhost-enabled machine yet. So only compile-tested
for now, but pretty trivial.
hw/virtio-pci.c |6 +++---
hw/virtio.c |5 +
hw/virtio.h |1 +
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 7ddf612..988c75c 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -421,8 +421,8 @@ static unsigned virtio_pci_get_features(void *opaque)
static void virtio_pci_guest_notifier_read(void *opaque)
{
-VirtQueue *vq = opaque;
-EventNotifier *n = virtio_queue_get_guest_notifier(vq);
+EventNotifier *n = opaque;
+VirtQueue *vq = virtqueue_from_guest_notifier(n);
if (event_notifier_test_and_clear(n)) {
virtio_irq(vq);
}
@@ -440,7 +440,7 @@ static int virtio_pci_set_guest_notifier(void *opaque, int
n, bool assign)
return r;
}
qemu_set_fd_handler(event_notifier_get_fd(notifier),
-virtio_pci_guest_notifier_read, NULL, vq);
+virtio_pci_guest_notifier_read, NULL, notifier);
} else {
qemu_set_fd_handler(event_notifier_get_fd(notifier),
NULL, NULL, NULL);
diff --git a/hw/virtio.c b/hw/virtio.c
index 4475bb3..bfce44b 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -805,6 +805,11 @@ VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n)
return vdev->vq + n;
}
+VirtQueue *virtqueue_from_guest_notifier(EventNotifier *e)
+{
+return container_of(e, VirtQueue, guest_notifier);
+}
+
EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
{
return &vq->guest_notifier;
diff --git a/hw/virtio.h b/hw/virtio.h
index e4306cd..d6a5b00 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -210,6 +210,7 @@ target_phys_addr_t virtio_queue_get_ring_size(VirtIODevice
*vdev, int n);
uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n);
void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
+VirtQueue *virtqueue_from_guest_notifier(EventNotifier *vq);
EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
void virtio_irq(VirtQueue *vq);
--
1.6.6.1
[Qemu-devel] [Bug 532733] Re: apt/dpkg in qemu-system-arm hangs if a big task is installed
If someone reproduces the bug against upstream qemu, feel free to refile the bug with the appropriate information. -- apt/dpkg in qemu-system-arm hangs if a big task is installed https://bugs.launchpad.net/bugs/532733 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Invalid Status in “qemu-kvm” package in Ubuntu: Incomplete Status in “qemu-kvm” source package in Lucid: Incomplete Bug description: Binary package hint: qemu-kvm running rootstock and installing ubuntu-netbook^ makes the VM hang in "unpacking iso-codes" this is reproducable every time in rootstock as well as in a standard qemu-system-arm vm that contains a minimal ubuntu with running apt-get install ubuntu-netbook
[Qemu-devel] [PATCH 09/14] move all directory entries in config-host.mak close
Signed-off-by: Paolo Bonzini --- configure | 13 ++--- 1 files changed, 6 insertions(+), 7 deletions(-) diff --git a/configure b/configure index fee9665..9736942 100755 --- a/configure +++ b/configure @@ -2067,6 +2067,12 @@ printf "# Configured with:" >> $config_host_mak printf " '%s'" "$0" "$@" >> $config_host_mak echo >> $config_host_mak +echo "prefix=$prefix" >> $config_host_mak +echo "bindir=$bindir" >> $config_host_mak +echo "mandir=$mandir" >> $config_host_mak +echo "datadir=$datadir" >> $config_host_mak +echo "sysconfdir=$sysconfdir" >> $config_host_mak +echo "docdir=$docdir" >> $config_host_mak echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> $config_host_mak echo "CONFIG_QEMU_CONFDIR=\"$confdir\"" >> $config_host_mak @@ -2310,13 +2316,6 @@ if test \( "$cpu" = "i386" -o "$cpu" = "x86_64" \) -a \ roms="optionrom" fi echo "ROMS=$roms" >> $config_host_mak - -echo "prefix=$prefix" >> $config_host_mak -echo "bindir=$bindir" >> $config_host_mak -echo "mandir=$mandir" >> $config_host_mak -echo "datadir=$datadir" >> $config_host_mak -echo "sysconfdir=$sysconfdir" >> $config_host_mak -echo "docdir=$docdir" >> $config_host_mak echo "MAKE=$make" >> $config_host_mak echo "INSTALL=$install" >> $config_host_mak echo "INSTALL_DIR=$install -d -m0755 -p" >> $config_host_mak -- 1.6.6.1
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On 05/26/2010 04:50 PM, Anthony Liguori wrote: In fact, btrfs is currently unusable for virt because O_SYNC writes inflate a guest write to a host write. by a huge factor (50x-100x). cache=writethrough is 100% unusable, cache=writeback is barely tolerable. As of 2.6.32, cache=volatile is probably required to get something resembling reasonable performance on btrfs. Of course, we expect that btrfs will improve in time, but still it doesn't seem to be fsync friendly. So you're suggesting that anyone who uses virt on btrfs should be prepared to deal with data corruption on host failure? No. That sounds to me like btrfs isn't ready for real workloads. The btrfs developers aren't saying anything different. But people still want to try it out (to wire up snapshotting to management, for example). -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH 0/8] Make event_notifier more useful and more used
Hi, this patch adds all the eventfd bells and whistles from vl.c/cpus.c to event_notifier, including pipe emulation and Win32 support. It then modifies the iothread code to use it instead. Paolo Bonzini (8): move event_notifier into the main directory add event_notifier_set remove event_notifier_test add and use virtqueue_from_guest_notifier add and use event_notifier_set_handler enable event_notifier to use pipes add Win32 implementation of event notifiers change ioevent to use event notifiers cpus.c | 95 +++--- event_notifier.c| 124 +++ event_notifier.h| 28 +++ hw/event_notifier.c | 62 - hw/event_notifier.h | 16 --- hw/virtio-pci.c | 11 ++--- hw/virtio.c |5 ++ hw/virtio.h |1 + 8 files changed, 171 insertions(+), 171 deletions(-) create mode 100644 event_notifier.c create mode 100644 event_notifier.h delete mode 100644 hw/event_notifier.c delete mode 100644 hw/event_notifier.h
[Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On 05/26/2010 03:48 PM, Anthony Liguori wrote: We might get 100 bug reports about this "regression" but they concern much less than 1 bug report of image corruption because of power failure/host crash. A reputation of being unsafe is very difficult to get rid of and is something that I hear concerns about frequently. True, but how many people will use cache=volatile? Nobody is going to make it the default. If a blog post appears "hey look cache=volatile will speedup your virtual machine", and gets so much momentum that people start using it and lose data because of it (which is highly hypothetical and unlikely), QEMU developers are not the ones to be blamed. I'm not suggesting that the compile option should be disabled by default upstream. But the option should be there for distributions because I hope that any enterprise distro disables it. Actually it's perfectly possible that they will _enable_ it if a configure option is required to enable cache=volatile. RHEL for example doesn't support at all running qemu directly, only via libvirt. If libvirt doesn't pass cache=volatile to qemu, they're safe. (Well, virt-install uses libvirt, so it couldn't use cache=volatile either, so I admit it's not a great example). Paolo
[Qemu-devel] [PATCH 8/8] change ioevent to use event notifiers
Signed-off-by: Paolo Bonzini
---
cpus.c | 95 +---
1 files changed, 9 insertions(+), 86 deletions(-)
diff --git a/cpus.c b/cpus.c
index 8341f6c..7a1dd06 100644
--- a/cpus.c
+++ b/cpus.c
@@ -26,6 +26,7 @@
#include "config-host.h"
#include "monitor.h"
+#include "event_notifier.h"
#include "sysemu.h"
#include "gdbstub.h"
#include "dma.h"
@@ -141,97 +142,19 @@ static int tcg_has_work(void)
return 0;
}
-#ifndef _WIN32
-static int io_thread_fd = -1;
-
-static void qemu_event_increment(void)
-{
-/* Write 8 bytes to be compatible with eventfd. */
-static const uint64_t val = 1;
-ssize_t ret;
-
-if (io_thread_fd == -1)
-return;
-
-do {
-ret = write(io_thread_fd, &val, sizeof(val));
-} while (ret < 0 && errno == EINTR);
-
-/* EAGAIN is fine, a read must be pending. */
-if (ret < 0 && errno != EAGAIN) {
-fprintf(stderr, "qemu_event_increment: write() filed: %s\n",
-strerror(errno));
-exit (1);
-}
-}
-
-static void qemu_event_read(void *opaque)
-{
-int fd = (unsigned long)opaque;
-ssize_t len;
-char buffer[512];
-
-/* Drain the notify pipe. For eventfd, only 8 bytes will be read. */
-do {
-len = read(fd, buffer, sizeof(buffer));
-} while ((len == -1 && errno == EINTR) || len == sizeof(buffer));
-}
+static EventNotifier io_thread_notifier;
static int qemu_event_init(void)
{
int err;
-int fds[2];
-
-err = qemu_eventfd(fds);
-if (err == -1)
-return -errno;
+err = event_notifier_init(&io_thread_notifier, 0);
+if (err == 0)
+event_notifier_set_handler(&io_thread_notifier,
+ (EventNotifierHandler *)
+ event_notifier_test_and_clear);
-err = fcntl_setfl(fds[0], O_NONBLOCK);
-if (err < 0)
-goto fail;
-
-err = fcntl_setfl(fds[1], O_NONBLOCK);
-if (err < 0)
-goto fail;
-
-qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL,
- (void *)(unsigned long)fds[0]);
-
-io_thread_fd = fds[1];
-return 0;
-
-fail:
-close(fds[0]);
-close(fds[1]);
return err;
}
-#else
-HANDLE qemu_event_handle;
-
-static void dummy_event_handler(void *opaque)
-{
-}
-
-static int qemu_event_init(void)
-{
-qemu_event_handle = CreateEvent(NULL, FALSE, FALSE, NULL);
-if (!qemu_event_handle) {
-fprintf(stderr, "Failed CreateEvent: %ld\n", GetLastError());
-return -1;
-}
-qemu_add_wait_object(qemu_event_handle, dummy_event_handler, NULL);
-return 0;
-}
-
-static void qemu_event_increment(void)
-{
-if (!SetEvent(qemu_event_handle)) {
-fprintf(stderr, "qemu_event_increment: SetEvent failed: %ld\n",
-GetLastError());
-exit (1);
-}
-}
-#endif
#ifndef CONFIG_IOTHREAD
int qemu_init_main_loop(void)
@@ -281,7 +203,7 @@ void qemu_notify_event(void)
{
CPUState *env = cpu_single_env;
-qemu_event_increment ();
+event_notifier_set(&io_thread_notifier);
if (env) {
cpu_exit(env);
}
@@ -709,7 +631,7 @@ void qemu_init_vcpu(void *_env)
void qemu_notify_event(void)
{
-qemu_event_increment();
+event_notifier_set(&io_thread_notifier);
}
static void qemu_system_vmstop_request(int reason)
--
1.6.6.1
[Qemu-devel] [PATCH 01/14] bail out early on invalid -cpu option
It would fail later anyway. Signed-off-by: Paolo Bonzini --- configure |7 ++- 1 files changed, 2 insertions(+), 5 deletions(-) diff --git a/configure b/configure index 3cd2c5f..e8dd2ef 100755 --- a/configure +++ b/configure @@ -232,7 +232,8 @@ case "$cpu" in cpu="sparc" ;; *) -cpu="unknown" +echo "Unsupported CPU = $cpu" +exit 1 ;; esac @@ -2068,10 +2069,6 @@ case "$cpu" in armv4b|armv4l) ARCH=arm ;; - *) -echo "Unsupported CPU = $cpu" -exit 1 - ;; esac echo "ARCH=$ARCH" >> $config_host_mak if test "$debug_tcg" = "yes" ; then -- 1.6.6.1
[Qemu-devel] [PATCH 2/8] add event_notifier_set
Signed-off-by: Paolo Bonzini
---
event_notifier.c |7 +++
event_notifier.h |1 +
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/event_notifier.c b/event_notifier.c
index 2c73555..3f50568 100644
--- a/event_notifier.c
+++ b/event_notifier.c
@@ -38,6 +38,13 @@ int event_notifier_get_fd(EventNotifier *e)
return e->fd;
}
+int event_notifier_set(EventNotifier *e)
+{
+uint64_t value = 1;
+int r = write(e->fd, &value, sizeof(value));
+return r == sizeof(value);
+}
+
int event_notifier_test_and_clear(EventNotifier *e)
{
uint64_t value;
diff --git a/event_notifier.h b/event_notifier.h
index 24117ea..8d5735f 100644
--- a/event_notifier.h
+++ b/event_notifier.h
@@ -10,6 +10,7 @@ struct EventNotifier {
int event_notifier_init(EventNotifier *, int active);
void event_notifier_cleanup(EventNotifier *);
int event_notifier_get_fd(EventNotifier *);
+int event_notifier_set(EventNotifier *);
int event_notifier_test_and_clear(EventNotifier *);
int event_notifier_test(EventNotifier *);
--
1.6.6.1
[Qemu-devel] [PATCH 03/14] dyngen is long time gone
Signed-off-by: Paolo Bonzini --- configure |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/configure b/configure index 5a7cb6e..f96 100755 --- a/configure +++ b/configure @@ -748,7 +748,8 @@ echo "Advanced options (experts only):" echo " --source-path=PATH path of source code [$source_path]" echo " --cross-prefix=PREFIXuse PREFIX for compile tools [$cross_prefix]" echo " --cc=CC use C compiler CC [$cc]" -echo " --host-cc=CC use C compiler CC [$host_cc] for dyngen etc." +echo " --host-cc=CC use C compiler CC [$host_cc] for code run at" +echo " build time" echo " --extra-cflags=CFLAGSappend extra C compiler flags QEMU_CFLAGS" echo " --extra-ldflags=LDFLAGS append extra linker flags LDFLAGS" echo " --make=MAKE use specified make [$make]" -- 1.6.6.1
[Qemu-devel] [PATCH 04/14] delete duplicate create_config case stanza
Signed-off-by: Paolo Bonzini
---
create_config |5 -
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/create_config b/create_config
index 2f052ae..edcad25 100755
--- a/create_config
+++ b/create_config
@@ -13,11 +13,6 @@ case $line in
pkgversion=${line#*=}
echo "#define QEMU_PKGVERSION \"$pkgversion\""
;;
- ARCH=*) # configuration
-arch=${line#*=}
-arch_name=`echo $arch | tr '[:lower:]' '[:upper:]'`
-echo "#define HOST_$arch_name 1"
-;;
CONFIG_AUDIO_DRIVERS=*)
drivers=${line#*=}
echo "#define CONFIG_AUDIO_DRIVERS \\"
--
1.6.6.1
[Qemu-devel] [PATCH 14/14] move computation of tools and roms outside of config-host.mak generation
Signed-off-by: Paolo Bonzini --- configure | 40 +--- 1 files changed, 21 insertions(+), 19 deletions(-) diff --git a/configure b/configure index 488ef07..d9983b0 100755 --- a/configure +++ b/configure @@ -1991,6 +1991,27 @@ fi confdir=$sysconfdir$confsuffix +tools= +if test "$softmmu" = yes ; then + tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools" + if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then + tools="qemu-nbd\$(EXESUF) $tools" +if [ "$check_utests" = "yes" ]; then + tools="check-qint check-qstring check-qdict check-qlist $tools" + tools="check-qfloat check-qjson $tools" +fi + fi +fi + +# Mac OS X ships with a broken assembler +roms= +if test \( "$cpu" = "i386" -o "$cpu" = "x86_64" \) -a \ +"$targetos" != "Darwin" -a "$targetos" != "SunOS" -a \ +"$softmmu" = yes ; then + roms="optionrom" +fi + + echo "Install prefix$prefix" echo "BIOS directory`eval echo $datadir`" echo "binary directory `eval echo $bindir`" @@ -2293,26 +2314,7 @@ bsd) ;; esac -tools= -if test "$softmmu" = yes ; then - tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools" - if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then - tools="qemu-nbd\$(EXESUF) $tools" -if [ "$check_utests" = "yes" ]; then - tools="check-qint check-qstring check-qdict check-qlist $tools" - tools="check-qfloat check-qjson $tools" -fi - fi -fi echo "TOOLS=$tools" >> $config_host_mak - -# Mac OS X ships with a broken assembler -roms= -if test \( "$cpu" = "i386" -o "$cpu" = "x86_64" \) -a \ -"$targetos" != "Darwin" -a "$targetos" != "SunOS" -a \ -"$softmmu" = yes ; then - roms="optionrom" -fi echo "ROMS=$roms" >> $config_host_mak echo "MAKE=$make" >> $config_host_mak echo "INSTALL=$install" >> $config_host_mak -- 1.6.6.1
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
Anthony Liguori a écrit : > On 05/26/2010 03:52 AM, Aurelien Jarno wrote: >> On Tue, May 25, 2010 at 08:31:20PM -0500, Anthony Liguori wrote: >> >>> On 05/25/2010 04:01 PM, Aurelien Jarno wrote: >>> I really think this patch can be useful, in my own case when testing debian-installer (I already cache=writeback). In short all that is about developing and testing, as opposed to run a VM in production, can benefit about that. This was one of the original use case of QEMU before KVM arrived. Unless someone can convince me not to do it, I seriously considering applying this patch. >>> There really needs to be an indication in the --help output of what >>> the ramifications of this option are, in the very least. It should >>> >> That's indeed something than can be done, but to avoid double standards, >> it should also be done for other features that can lead to data >> corruption. I am talking for example on the qcow format, which is not >> really supported anymore. >> > > I agree. > >>> also be removable via a ./configure option because no sane >>> distribution should enable this for end users. >>> >>> >> I totally disagree. All the examples I have given apply to qemu *users*, >> not qemu developers. They surely don't want to recompile qemu for their >> usage. Note also that what is proposed in this patch was the default not >> too long ago, and that a lot of users complained about the new default >> for their usage, they see it as a regression. We even had to put a note >> explaining that in the Debian package to avoid to many bug reports. >> cache=writeback only answer partially to this use case. >> > > It's hard for me to consider this a performance regression because > ultimately, you're getting greater than bare metal performance (because > of extremely aggressive caching). It might be a regression from the > previous performance, but that was at the cost of safety. For people who don't care about safety it's still a regression. And it is a common usage of QEMU. > We might get 100 bug reports about this "regression" but they concern > much less than 1 bug report of image corruption because of power > failure/host crash. A reputation of being unsafe is very difficult to > get rid of and is something that I hear concerns about frequently. > > I'm not suggesting that the compile option should be disabled by default > upstream. But the option should be there for distributions because I > hope that any enterprise distro disables it. > Which basically means those distro don't care about some use cases of QEMU, that were for most of them the original uses cases. It's sad. Sometimes I really whishes that KVM never tried to reintegrate code into QEMU, it doesn't bring only good things. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
Kevin Wolf a écrit : > Am 26.05.2010 15:42, schrieb Anthony Liguori: >> On 05/26/2010 03:43 AM, Kevin Wolf wrote: >>> Am 26.05.2010 03:31, schrieb Anthony Liguori: >>> On 05/25/2010 04:01 PM, Aurelien Jarno wrote: > I really think this patch can be useful, in my own case when testing > debian-installer (I already cache=writeback). In short all that is about > developing and testing, as opposed to run a VM in production, can > benefit about that. This was one of the original use case of QEMU before > KVM arrived. > > Unless someone can convince me not to do it, I seriously considering > applying this patch. > > There really needs to be an indication in the --help output of what the ramifications of this option are, in the very least. It should also be removable via a ./configure option because no sane distribution should enable this for end users. >>> We know better what you stupid user want? >> What percentage of qemu users do you think have actually read qemu-doc.texi? > > As I said, put the warning in the option name like cache=unsafe or > something even more scary and I'm all for it. > I am also fine for an option likes this, sounds the way to go. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] Re: [PATCH 3/5] QMP: Introduce MIGRATION events
On Wed, May 26, 2010 at 09:54:22AM -0500, Anthony Liguori wrote:
> On 05/26/2010 05:33 AM, Daniel P. Berrange wrote:
> >>>I'm not sure why you would need a notification of when migration
> >>>starts (since you know when you've started migration).
> >>>
> >>But you don't know if the other end "knows" that it has also started.
> >>
> >>started is needed only in incoming part, because we don't have a
> >>monitor to ask if migration has started.
> >>
> >If we ever want to get closer to allowing multiple monitors, or allowing
> >apps to issue QMP commands directly via libvirt, then we still need the
> >'migration started' event on the source, because something else can
> >have issued the 'migrate' command without the mgmt app knowing.
> >
>
> Migration started doesn't help multiple monitors. You need locking of
> some sort.
>
> Part of the problem is the QMP migrate command is implemented as a
> synchronous command. It really ought to be an asynchronous command.
> That tells you when the migration has actually completed without polling.
Handling asynchronous commands is alot more complicated and error
prone for client apps, than providing a asynchronous event notification
of the lifecycle stages. If you want to also query status while waiting
for the completion, it means you can have to deal with overlapping
command execute+return pairs within a single monitor connection.
AFAICT this requires a change to QMP to require a unique ID to be
sent with the {'execute'..} command and be sent back with the later
corresponding {'return'...} data, so you can actually correlate
reliably.
> On the destination side, we're really limited by the fact that we don't
> do live incoming migrations. The monitor doesn't get a chance to run at
> all with exec: migration, for instance.
If QEMU let you issue a monitor command for starting incoming
migration, instead of using -incoming this wouldn't such a bad
problem. eg you can launch QEMU in the desired config, with CPUs
stopped, do the normal QMP handshake + whatever else is required
then issue 'migrate_incoming URI' which blocked the caller for
the duration, to allow completion to be detected.
> For tcp: and unix:, a CONNECTED event absolutely makes sense (every
> socket server should emit a CONNECTED event). Unfortunately, after
> CONNECTED you lose the monitor until migration is complete. If
> something bad happens, you have to exit qemu so once the monitor
> returns, migration has completed successfully.
>
> If we introduce live incoming migration, we'll need to rethink things.
> I would actually suggest that we deprecate the incoming command if we do
> that and make incoming migration a monitor command. I would think it
> should have the same semantics as migrate (as an asynchronous command).
> A CONNECTED event still makes sense for tcp and unix protocols but I
> don't think events make sense for start stop vs. an asynchronous command
> completion.
Do you actually mean 'deprecate -incoming arg' here ?
Daniel
--
|: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
Anthony Liguori a écrit : > On 05/26/2010 09:12 AM, Aurelien Jarno wrote: >>> It's hard for me to consider this a performance regression because >>> ultimately, you're getting greater than bare metal performance (because >>> of extremely aggressive caching). It might be a regression from the >>> previous performance, but that was at the cost of safety. >>> >> For people who don't care about safety it's still a regression. And it >> is a common usage of QEMU. >> > > It's not a functional change. It's a change in performance. There are > tons of changes in performance characteristics of qemu from version to > version. It's not even a massive one. > >>> We might get 100 bug reports about this "regression" but they concern >>> much less than 1 bug report of image corruption because of power >>> failure/host crash. A reputation of being unsafe is very difficult to >>> get rid of and is something that I hear concerns about frequently. >>> >>> I'm not suggesting that the compile option should be disabled by default >>> upstream. But the option should be there for distributions because I >>> hope that any enterprise distro disables it. >>> >>> >> Which basically means those distro don't care about some use cases of >> QEMU, that were for most of them the original uses cases. It's sad. >> > > This isn't a feature. This is a change in performance. No one is not > able to satisfy their use case from this behavior. > >> Sometimes I really whishes that KVM never tried to reintegrate code into >> QEMU, it doesn't bring only good things. >> > > I highly doubt that this is even visible on benchmarks without using > KVM. The improvement on a microbenchmark was relatively small and the > cost from TCG would almost certainly dwarf it. It is something clearly visible. Before fsync() was not used, and it happens this syscall can be very expensive (ie a few seconds, especially with some other i/o load on the system) on ext3 with not so old kernels. A google search for "firefox fsync" will give you a few pointers. > Also, remember before KVM, we had single threaded IO and posix-aio > (which is still single threaded). If KVM never happened, block > performance would be far, far worse than it is today with cache=writeback. > io thread is not enable by default in QEMU. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] [PATCH] Add cache=unsafe parameter to -drive
Usually the guest can tell the host to flush data to disk. In some cases we
don't want to flush though, but try to keep everything in cache.
So let's add a new cache value to -drive that allows us to set the cache
policy to most aggressive, disabling flushes. We call this mode "unsafe",
as guest data is not guaranteed to survive host crashes anymore.
This patch also adds a noop function for aio, so we can do nothing in AIO
fashion.
Signed-off-by: Alexander Graf
---
v2 -> v3:
- Add description of cache=volatile
- Squash aio noop noop patch into this one
v3 -> v4:
- Rename cache=volatile to cache=unsafe
---
block.c | 28
block.h |1 +
qemu-config.c |2 +-
qemu-options.hx | 13 ++---
vl.c|3 +++
5 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index 0b0966c..3c0249b 100644
--- a/block.c
+++ b/block.c
@@ -50,6 +50,8 @@ static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState
*bs,
BlockDriverCompletionFunc *cb, void *opaque);
static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
+static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque);
static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
@@ -1312,6 +1314,10 @@ const char *bdrv_get_device_name(BlockDriverState *bs)
void bdrv_flush(BlockDriverState *bs)
{
+if (bs->open_flags & BDRV_O_NO_FLUSH) {
+return;
+}
+
if (bs->drv && bs->drv->bdrv_flush)
bs->drv->bdrv_flush(bs);
}
@@ -2099,6 +2105,10 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
{
BlockDriver *drv = bs->drv;
+if (bs->open_flags & BDRV_O_NO_FLUSH) {
+return bdrv_aio_noop_em(bs, cb, opaque);
+}
+
if (!drv)
return NULL;
return drv->bdrv_aio_flush(bs, cb, opaque);
@@ -2214,6 +2224,24 @@ static BlockDriverAIOCB
*bdrv_aio_flush_em(BlockDriverState *bs,
return &acb->common;
}
+static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BlockDriverAIOCBSync *acb;
+
+acb = qemu_aio_get(&bdrv_em_aio_pool, bs, cb, opaque);
+acb->is_write = 1; /* don't bounce in the completion hadler */
+acb->qiov = NULL;
+acb->bounce = NULL;
+acb->ret = 0;
+
+if (!acb->bh)
+acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb);
+
+qemu_bh_schedule(acb->bh);
+return &acb->common;
+}
+
/**/
/* sync block device emulation */
diff --git a/block.h b/block.h
index 278259c..24efeb6 100644
--- a/block.h
+++ b/block.h
@@ -33,6 +33,7 @@ typedef struct QEMUSnapshotInfo {
#define BDRV_O_CACHE_WB0x0040 /* use write-back caching */
#define BDRV_O_NATIVE_AIO 0x0080 /* use native AIO instead of the thread pool
*/
#define BDRV_O_NO_BACKING 0x0100 /* don't open the backing file */
+#define BDRV_O_NO_FLUSH0x0200 /* disable flushing on this disk */
#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB)
diff --git a/qemu-config.c b/qemu-config.c
index aa376d4..5a4e61b 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -54,7 +54,7 @@ QemuOptsList qemu_drive_opts = {
},{
.name = "cache",
.type = QEMU_OPT_STRING,
-.help = "host cache usage (none, writeback, writethrough)",
+.help = "host cache usage (none, writeback, writethrough, unsafe)",
},{
.name = "aio",
.type = QEMU_OPT_STRING,
diff --git a/qemu-options.hx b/qemu-options.hx
index 03e95fd..cea9b72 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -118,8 +118,9 @@ ETEXI
DEF("drive", HAS_ARG, QEMU_OPTION_drive,
"-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
" [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
-" [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
-" [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n"
+" [,cache=writethrough|writeback|unsafe|none][,format=f]\n"
+" [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
+" [,readonly=on|off]\n"
"use 'file' as a drive image\n", QEMU_ARCH_ALL)
STEXI
@item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
@@ -148,7 +149,7 @@ These options have the same definition as they have in
@option{-hdachs}.
@item snapsh...@var{snapshot}
@var{snapshot} is "on" or "off" and allows to enable snapshot for given drive
(see @option{-snapshot}).
@item cac...@var{cache}
-...@var{cache} is "none", "writeback", or "writethrough" and controls how the
host cache is used to access block data.
+...@var{cache} is "none", "writeback", "unsafe",
Re: [Qemu-devel] Re: [PATCH] Add cache=volatile parameter to -drive
On 05/26/2010 10:40 AM, Aurelien Jarno wrote: I highly doubt that this is even visible on benchmarks without using KVM. The improvement on a microbenchmark was relatively small and the cost from TCG would almost certainly dwarf it. It is something clearly visible. Before fsync() was not used, and it happens this syscall can be very expensive (ie a few seconds, especially with some other i/o load on the system) on ext3 with not so old kernels. A google search for "firefox fsync" will give you a few pointers. I'm well aware. I've asked multiple times for benchmark data with ext3 because of this issue. So far, it hasn't been noticable. Also, remember before KVM, we had single threaded IO and posix-aio (which is still single threaded). If KVM never happened, block performance would be far, far worse than it is today with cache=writeback. io thread is not enable by default in QEMU. But the thread pool based aio implementation is always enabled regardless of io thread. I was going to build and run 0.9.1 to do a benchmark of TCG comparing that to the latest git, but I don't have an easy way to get a version of gcc-3. I've forgotten how annoying that used to be :-) I would be very surprised if TCG + IDE had any observable performance difference pre/post the ide flush implementation. I'm almost inclined to suggestion that flush controls should be a qdev property verses a drive property since that's really where the change happened. Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH] Add cache=unsafe parameter to -drive
On 05/26/2010 10:51 AM, Alexander Graf wrote:
Usually the guest can tell the host to flush data to disk. In some cases we
don't want to flush though, but try to keep everything in cache.
So let's add a new cache value to -drive that allows us to set the cache
policy to most aggressive, disabling flushes. We call this mode "unsafe",
as guest data is not guaranteed to survive host crashes anymore.
This patch also adds a noop function for aio, so we can do nothing in AIO
fashion.
Signed-off-by: Alexander Graf
I'm happy merging this for now, but I think it might make more sense to
do this as per-device flush control qdev options since those bits are
actually guest visible. Christoph, does that seem reasonable?
Regards,
Anthony Liguori
---
v2 -> v3:
- Add description of cache=volatile
- Squash aio noop noop patch into this one
v3 -> v4:
- Rename cache=volatile to cache=unsafe
---
block.c | 28
block.h |1 +
qemu-config.c |2 +-
qemu-options.hx | 13 ++---
vl.c|3 +++
5 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index 0b0966c..3c0249b 100644
--- a/block.c
+++ b/block.c
@@ -50,6 +50,8 @@ static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState
*bs,
BlockDriverCompletionFunc *cb, void *opaque);
static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque);
+static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque);
static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors);
static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
@@ -1312,6 +1314,10 @@ const char *bdrv_get_device_name(BlockDriverState *bs)
void bdrv_flush(BlockDriverState *bs)
{
+if (bs->open_flags& BDRV_O_NO_FLUSH) {
+return;
+}
+
if (bs->drv&& bs->drv->bdrv_flush)
bs->drv->bdrv_flush(bs);
}
@@ -2099,6 +2105,10 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
{
BlockDriver *drv = bs->drv;
+if (bs->open_flags& BDRV_O_NO_FLUSH) {
+return bdrv_aio_noop_em(bs, cb, opaque);
+}
+
if (!drv)
return NULL;
return drv->bdrv_aio_flush(bs, cb, opaque);
@@ -2214,6 +2224,24 @@ static BlockDriverAIOCB
*bdrv_aio_flush_em(BlockDriverState *bs,
return&acb->common;
}
+static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BlockDriverAIOCBSync *acb;
+
+acb = qemu_aio_get(&bdrv_em_aio_pool, bs, cb, opaque);
+acb->is_write = 1; /* don't bounce in the completion hadler */
+acb->qiov = NULL;
+acb->bounce = NULL;
+acb->ret = 0;
+
+if (!acb->bh)
+acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb);
+
+qemu_bh_schedule(acb->bh);
+return&acb->common;
+}
+
/**/
/* sync block device emulation */
diff --git a/block.h b/block.h
index 278259c..24efeb6 100644
--- a/block.h
+++ b/block.h
@@ -33,6 +33,7 @@ typedef struct QEMUSnapshotInfo {
#define BDRV_O_CACHE_WB0x0040 /* use write-back caching */
#define BDRV_O_NATIVE_AIO 0x0080 /* use native AIO instead of the thread
pool */
#define BDRV_O_NO_BACKING 0x0100 /* don't open the backing file */
+#define BDRV_O_NO_FLUSH0x0200 /* disable flushing on this disk */
#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB)
diff --git a/qemu-config.c b/qemu-config.c
index aa376d4..5a4e61b 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -54,7 +54,7 @@ QemuOptsList qemu_drive_opts = {
},{
.name = "cache",
.type = QEMU_OPT_STRING,
-.help = "host cache usage (none, writeback, writethrough)",
+.help = "host cache usage (none, writeback, writethrough, unsafe)",
},{
.name = "aio",
.type = QEMU_OPT_STRING,
diff --git a/qemu-options.hx b/qemu-options.hx
index 03e95fd..cea9b72 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -118,8 +118,9 @@ ETEXI
DEF("drive", HAS_ARG, QEMU_OPTION_drive,
"-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
" [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
-" [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
-" [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n"
+" [,cache=writethrough|writeback|unsafe|none][,format=f]\n"
+" [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
+" [,readonly=on|off]\n"
"use 'file' as a drive image\n", QEMU_ARCH_ALL)
STEXI
@item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
@@ -148,7 +149,7 @@ These options have the same definition as they have in
@option{-hdachs}.
@i
[Qemu-devel] Re: [PATCH] Add cache=unsafe parameter to -drive
On 26.05.2010, at 18:17, Anthony Liguori wrote: > On 05/26/2010 10:51 AM, Alexander Graf wrote: >> Usually the guest can tell the host to flush data to disk. In some cases we >> don't want to flush though, but try to keep everything in cache. >> >> So let's add a new cache value to -drive that allows us to set the cache >> policy to most aggressive, disabling flushes. We call this mode "unsafe", >> as guest data is not guaranteed to survive host crashes anymore. >> >> This patch also adds a noop function for aio, so we can do nothing in AIO >> fashion. >> >> Signed-off-by: Alexander Graf >> > > I'm happy merging this for now, but I think it might make more sense to do > this as per-device flush control qdev options since those bits are actually > guest visible. Christoph, does that seem reasonable? The way it's implemented in this patch it's not guest visible. The guest still gets the flush capability exposed, but the actual operation returns a nop. With cache=unsafe qemu basically bevaves the same as a battery backed raid controller. Alex
