[Qemu-devel] An issue in block-migration
Hi all,In block-migration.c file, line 435, if (bdrv_get_dirty(bmds->bs, sector)) {It looks like this "if statement" is used to check whether a chunk is dirty or not. If it is dirty, system will migrate a whole chunk, 1MB data, to the destination. Otherwise, the cur_dirty will increase by 1MB/512B sectors.However, in my understanding, this function, bdrv_get_dirty(bmds->bs, sector), only check this sector (512B) is dirty or not, rather than a whole chunk (1MB). Could someone tell me the reason?Thanks!Yaodong
Re: [Qemu-devel] [PATCH 0/8 RFC] migration: Introduce side channel for RAM
On 09/26/2013 08:44 PM, Lei Li wrote: On 09/25/2013 11:02 PM, Paolo Bonzini wrote: Il 25/09/2013 16:32, Lei Li ha scritto: This RFC patch series tries to introduce a mechanism using side channel pipe for RAM via SCM_RIGHTS with unix domain socket protocol migration. This side channel will be used for the page flipping by vmsplice, which will be the internal mechanism for localhost migration that we are trying to add. The previous patch series for localhost migration as link, http://lists.nongnu.org/archive/html/qemu-devel/2013-08/msg02916.html After this series, will adjust the process of current migration for the localhost migration and involve the vmsplice based on the previous patch set as link above. Please let me know if it is the proper way for it or there is anything need to be improved. Your suggestions and comments are very welcome, and thanks for Paolo for his review and useful suggestions. Lei Li (8): migration-local: add pipe protocol for QEMUFileOps migration-local: add qemu_fopen_pipe() migration-local: add send_pipefd() migration-local: add recv_pipefd() QAPI: introduce magration capability unix_page_flipping migration: add migrate_unix_page_flipping() migration-unix: side channel support on unix outgoing migration-unix: side channel support on unix incoming Makefile.target |1 + include/migration/migration.h |3 + include/migration/qemu-file.h |4 + migration-local.c | 247 + migration-unix.c | 48 +++- migration.c |9 ++ qapi-schema.json |8 +- 7 files changed, 315 insertions(+), 5 deletions(-) create mode 100644 migration-local.c Yes, this is much closer! There are two problems to be fixed, but it is getting there. First, it breaks migration from old QEMU to new QEMU, and also migration where the source uses "unix:" and the destination uses "fd:" migration (this should work as long as page flipping is disabled). The problem is that recv_pipefd() "eats" one byte, and old versions of QEMU do not send that byte. Hi Paolo, I didn't consider this, thanks for pointing it out! The second problem is that you are not really using a side channel; you are still using the QEMUFile and relying on the normal migration code to send pages on the pipe. This will not be possible when you use vmsplice. Yes, you are right, and I am trying to involve the vmsplice. Both problems can be addressed with a single change in your approach: always use the Unix socket QEMUFile but, if page flipping is enabled, only transmit page addresses on the socket; page data will be on the pipe. You can use hooks such as before_ram_iterate, save_page and hook_ram_load to do all your customizations: send the pipe file descriptor, read the pipe file descriptor, and use the pipe as a side channel. To fix the first problem, you can use the before_ram_iterate callback to send the fd, and the hook_ram_load callback to receive it. The before_ram_iterate callback can write a special 8-byte record (with the RAM_SAVE_FLAG_HOOK set) that will trigger the hook, followed by send_pipefd(). The load_hook callback is called after the first 8-byte record is sent, and can just do recv_pipefd(). Hi Paolo, When debugging the code, I realized that this problem might still exist. In the incoming part, it will qemu_fopen_pipe() in unix_accept_incoming_migration first to enable the load_hook callback, the check action of this RAM_SAVE_FLAG_HOOK flags would lead to 8 bytes taken. Turns out, it will break normal unix migration (without unix-page-flipping), because no matter normal unix migration or unix-page-flipping migration, the incoming side has to check this 8-byes flags first to decide whether the load_hook is called, and normal unix migration did not send this 8-byte flags. I wonder if I didn't understand your suggestion correctly? To fix the second problem, and really use the pipe as a side channel, you can use the save_page QEMUFile callback on the send side. This callback must return RAM_SAVE_CONTROL_NOT_SUPP if page flipping is disabled. If it is enabled, it should write another 8-byte record with the RAM_SAVE_FLAG_HOOK bit, this time with the address of the page on the Unix socket; then write the page data on the pipe, and return 0. On the receive side, the 8-byte page address will once more cause the load_hook callback to be called. This time you already have a file descriptor, so you do not need to call recv_pipefd(): you just extract the page address from the 8-byte record and read the page data from the pipe. Thanks for your comprehensive suggestions, really nice ideas! The basis of your code will still be the socket-based QEMUFile, but you'll need your own QEMUFile since you're adding Unix-specific functionality. For this it is not a problem to have two copies the QEMUFile code for sockets, one in savevm.c and one in
Re: [Qemu-devel] [PATCH V13 10/13] NUMA: add qmp command set-mem-policy to set memory policy for NUMA node
On Tue, Sep 17, 2013 at 11:16:22AM +0800, Wanlong Gao wrote: > This QMP command allows user set guest node's memory policy > through the QMP protocol. The qmp-shell command is like: > set-mem-policy nodeid=0 policy=membind relative=true host-nodes=0-1 > > Reviewed-by: Luiz Capitulino > Signed-off-by: Wanlong Gao Wanlong Gao, 1) Exposing mbind via QMP/HMP on a live guest is interesting because, see mbind manpage: "By default, mbind() only has an effect for new allocations; if the pages inside the range have been already touched before setting the policy, then the policy has no effect. This default behavior may be overridden by the MPOL_MF_MOVE and MPOL_MF_MOVE_ALL flags described below." This means that executing set-mem-policy on a live guest is unpredictable: it depends on which pages have been faulted in already. Should the command be restricted to offline guests? 2) Have you tested the patchset with hugetlbfs (-mem-path) backing ?
Re: [Qemu-devel] [OpenRISC] [PATCH] Correction of the TLB handling of the OpenRISC target
On Wed, Oct 2, 2013 at 2:15 PM, Stefan Kristiansson
wrote:
> On Wed, Oct 2, 2013 at 8:33 AM, Jia Liu wrote:
>>
>> Hi Sebastian,
>>
>> On Wed, Oct 2, 2013 at 1:12 PM, Sebastian Macke
>> wrote:
>> > Hi,
>> >
>> > this patch corrects two problems for the OpenRISC Target in QEMU. The
>> > first
>> > one corrects one obvious bug
>> > concerning the handling of page faults while reading from a page.
>>
>> @@ -102,7 +102,7 @@ int cpu_openrisc_get_phys_data(OpenRISCCPU *cpu,
>> }
>> }
>>
>> -if ((rw & 0) && ((right & PAGE_READ) == 0)) {
>> +if (!(rw & 1) && ((right & PAGE_READ) == 0)) {
>> return TLBRET_BADADDR;
>> }
>> if ((rw & 1) && ((right & PAGE_WRITE) == 0)) {
>>
>> They are just two type of one code...
>
>
> No, (rw & 0) always evaluates to 0.
>
>>
>>
>> > The second
>> > part removes a non-conforming behavior for the first page of the memory.
>>
>> @@ -122,13 +122,6 @@ static int cpu_openrisc_get_phys_addr(OpenRISCCPU
>> *cpu,
>> {
>> int ret = TLBRET_MATCH;
>>
>> -/* [0x--0x2000]: unmapped */
>> -if (address < 0x2000 && (cpu->env.sr & SR_SM)) {
>> -*physical = address;
>> -*prot = PAGE_READ | PAGE_WRITE;
>> -return ret;
>> -}
>> -
>>
>> May you please explain more about why the first page is non-conforming?
>> The Arch manual told me 0x--0x2000 is unmapped.
>
>
> It shows an example where *software* leaves 0x - 0x2000 unmapped,
> the hardware should still allow for this area to be mapped.
OK, thank you. I will send a PULL Request soon.
Reviewed-by: Jia Liu
>
> Stefan
Regards,
Jia
Re: [Qemu-devel] [PATCH] hmp: Print \n after [not inserted]
On 10/02/2013 10:08 PM, Lucas Meneghel Rodrigues wrote:
I've noticed this when virt-test QEMU monitor protocol
code was getting all confused with output like:
'Removable device: not locked, tray closed\n [not inserted](qemu)'
Since it was breaking some assumptions on that code. I've
fixed the prompt matching code to be more lenient, but here
the human monitor is supposed to print the newline anyway.
Oh well, it seems that we're not supposed to have the newline there by
the look of the resulting output. Please ignore this patch.
CC: Stefan Hajnoczi
Signed-off-by: Lucas Meneghel Rodrigues
---
hmp.c| 2 +-
roms/seabios | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hmp.c b/hmp.c
index 5891507..2d2e5f8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -367,7 +367,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
info->value->inserted->iops_wr_max,
info->value->inserted->iops_size);
} else {
-monitor_printf(mon, " [not inserted]");
+monitor_printf(mon, " [not inserted]\n");
}
if (verbose) {
diff --git a/roms/seabios b/roms/seabios
index ece025f..7093aa5 16
--- a/roms/seabios
+++ b/roms/seabios
@@ -1 +1 @@
-Subproject commit ece025f5980bae88fa677bc9c0d24d2e580e205d
+Subproject commit 7093aa58046f8685025b0198708e7768733a017d
Re: [Qemu-devel] [PATCH] Update MAINTAINERS
On 10/02/2013 06:40:18 PM, Laurent Vivier wrote: Le 02/10/2013 20:42, Rob Landley a écrit : Laurent Vivier has an m68k gitorious branch to add the q800 target, which I've occasionally tested and would really really like to see finished and merged. Alas, last time I tested it the sucker died during the kernel boot as soon as mmu setup tried to enable the page tables. But before that I got three lines printk'd! (Woo! Progress!) Now, kernel is able to try to load its first userspace process but fails in ld.so somewhere when it is trying to map process memory (another MMU bug...) Progress! Still: possible m68k guy? Maybe? If he's interested? Yes, I can do that. There is not a lot of activity on this, anyway. I still have m68k images built form current kernels, I just can't test them. I'd poke at it and report bugs, but you know what the bugs are as well as I do. I don't know what the thing _should_ be doing, so... Rob
[Qemu-devel] [PATCH] hmp: Print \n after [not inserted]
I've noticed this when virt-test QEMU monitor protocol
code was getting all confused with output like:
'Removable device: not locked, tray closed\n [not inserted](qemu) '
Since it was breaking some assumptions on that code. I've
fixed the prompt matching code to be more lenient, but here
the human monitor is supposed to print the newline anyway.
CC: Stefan Hajnoczi
Signed-off-by: Lucas Meneghel Rodrigues
---
hmp.c| 2 +-
roms/seabios | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hmp.c b/hmp.c
index 5891507..2d2e5f8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -367,7 +367,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
info->value->inserted->iops_wr_max,
info->value->inserted->iops_size);
} else {
-monitor_printf(mon, " [not inserted]");
+monitor_printf(mon, " [not inserted]\n");
}
if (verbose) {
diff --git a/roms/seabios b/roms/seabios
index ece025f..7093aa5 16
--- a/roms/seabios
+++ b/roms/seabios
@@ -1 +1 @@
-Subproject commit ece025f5980bae88fa677bc9c0d24d2e580e205d
+Subproject commit 7093aa58046f8685025b0198708e7768733a017d
--
1.8.3.1
Re: [Qemu-devel] [PATCH] Update MAINTAINERS
Le 02/10/2013 20:42, Rob Landley a écrit : On 10/02/2013 12:09:58 PM, Anthony Liguori wrote: All of Paul's emails are bouncing and he hasn't been active for some time. ... M68K -M: Paul Brook -S: Odd Fixes +S: Orphan F: target-m68k/ F: hw/m68k/ Laurent Vivier has an m68k gitorious branch to add the q800 target, which I've occasionally tested and would really really like to see finished and merged. Alas, last time I tested it the sucker died during the kernel boot as soon as mmu setup tried to enable the page tables. But before that I got three lines printk'd! (Woo! Progress!) Now, kernel is able to try to load its first userspace process but fails in ld.so somewhere when it is trying to map process memory (another MMU bug...) Still: possible m68k guy? Maybe? If he's interested? Yes, I can do that. There is not a lot of activity on this, anyway. Regards, Laurent
[Qemu-devel] [PULL] Update OpenBIOS images to r1229
Hi Anthony, The following changes since commit a684f3cf9b9b9c3cb82be87aafc463de8974610c: Merge remote-tracking branch 'kraxel/seabios-1.7.3.2' into staging (2013-09-30 17:15:27 -0500) are available in the git repository at: https://github.com/mcayland/qemu.git qemu-openbios for you to fetch changes up to ad98acb9b1d610c4d243f53d9fb380e500d4abbe: Update OpenBIOS images (2013-10-03 00:04:20 +0100) Mark Cave-Ayland (1): Update OpenBIOS images pc-bios/README |2 +- pc-bios/openbios-ppc | Bin 733976 -> 729880 bytes pc-bios/openbios-sparc32 | Bin 381484 -> 381488 bytes pc-bios/openbios-sparc64 | Bin 1598328 -> 1598328 bytes roms/openbios|2 +- 5 files changed, 2 insertions(+), 2 deletions(-)
[Qemu-devel] 82574/82571 emulation?
Hi qemu-devel; We're using qemu to emulate a platform that uses Intel 82574, and 82571 based NICs (which use the e1000e driver). AFAICT, an emulation of 82574/82571 devices is not available in qemu. Couple of questions: 1) is someone already working on a 82574/82571 emulation? (and if so, can you point me to a repo?) 2) would a 82574/82571 emulation be welcome into upstream qemu? (This'd be the first intended-for-upstream qemu work I've done, so any other advice, or pointers that you have would be appreciated.) Thanks. -- Arthur
Re: [Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: set interrupts using pci irq wrappers
On Wed, 2013-10-02 at 09:58 -0600, Alex Williamson wrote:
> On Wed, 2013-10-02 at 15:41 +0300, Marcel Apfelbaum wrote:
> > pci_set_irq and the other pci irq wrappers use
> > PCI_INTERRUPT_PIN config register to compute device
> > INTx pin to assert/deassert.
> >
> > Save INTx pin into the config register before calling
> > pci_set_irq
> >
> > Signed-off-by: Marcel Apfelbaum
> > ---
> > hw/misc/vfio.c | 11 ++-
> > 1 file changed, 6 insertions(+), 5 deletions(-)
>
> Seems ok, but why not take advantage of the pci_irq_raise/lower()
> wrappers? BTW, with PCI being active low, should those be
> assert/deassert to avoid confusion confusion with the actual signal
> level? Thanks,
Thanks for the review!
I can use pci_irq_raise/lower(), but I wanted to preserve
the current form, re-factoring:
qemu_set_irq -> pci_set_irq,
qemu_irq_lower -> pci_irq_lower
...
If you think is worth it, I'll change it. (in all the places)
About assert/deassert instead of lower/raise, I am afraid
it will confuse users having two different set of naming
for interrupts usage.
Is easier to understand that pci_irq_lower behaves the same
as qemu_pci_lower, then pci_irq_deassert.
What do you think?
Thanks,
Marcel
>
> Alex
>
> >
> > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> > index a1c08fb..3d7297c 100644
> > --- a/hw/misc/vfio.c
> > +++ b/hw/misc/vfio.c
> > @@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque)
> > 'A' + vdev->intx.pin);
> >
> > vdev->intx.pending = true;
> > -qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1);
> > +pci_set_irq(&vdev->pdev, 1);
> > vfio_mmap_set_enabled(vdev, false);
> > if (vdev->intx.mmap_timeout) {
> > timer_mod(vdev->intx.mmap_timer,
> > @@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev)
> > vdev->host.bus, vdev->host.slot, vdev->host.function);
> >
> > vdev->intx.pending = false;
> > -qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > +pci_set_irq(&vdev->pdev, 0);
> > vfio_unmask_intx(vdev);
> > }
> >
> > @@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev)
> > qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
> > vfio_mask_intx(vdev);
> > vdev->intx.pending = false;
> > -qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > +pci_set_irq(&vdev->pdev, 0);
> >
> > /* Get an eventfd for resample/unmask */
> > if (event_notifier_init(&vdev->intx.unmask, 0)) {
> > @@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev)
> > */
> > vfio_mask_intx(vdev);
> > vdev->intx.pending = false;
> > -qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > +pci_set_irq(&vdev->pdev, 0);
> >
> > /* Tell KVM to stop listening for an INTx irqfd */
> > if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) {
> > @@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev)
> > vfio_disable_interrupts(vdev);
> >
> > vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
> > +pci_config_set_interrupt_pin(vdev->pdev.config, pin);
> >
> > #ifdef CONFIG_KVM
> > /*
> > @@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev)
> > vfio_disable_intx_kvm(vdev);
> > vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX);
> > vdev->intx.pending = false;
> > -qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> > +pci_set_irq(&vdev->pdev, 0);
> > vfio_mmap_set_enabled(vdev, true);
> >
> > fd = event_notifier_get_fd(&vdev->intx.interrupt);
>
>
>
Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs
On Wed, 2013-10-02 at 17:21 +0200, Paolo Bonzini wrote:
> Il 02/10/2013 14:41, Marcel Apfelbaum ha scritto:
> > +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> > +{
> > +pci_irq_lower(pci_dev);
> > +pci_irq_raise(pci_dev);
> > +}
> > +
>
> Why is this in the opposite order, compared to qemu_irq_pulse?
It is a bug, thank you for catching it!
Marcel
>
> Paolo
Re: [Qemu-devel] [Bug 1180777] Re: RDP traffic freeze on quiet network
Thanks for the tip Dumitrescu, I'll rive it a try AMD report here che result. Regards Il 29/set/2013 14:50 "Vasile Dumitrescu" ha scritto: > I added a rtl8139c netcard to the VM and connected through it by RDP - > no more freezes. > > It looks like kvm does not play well with virtio network cards and RDP. > > Red Hat virtio net windows driver version: 62.65.104.6500, 6/19/2013 > > I left the RH adapter on the VM, I just connect via RDP through the > rtl8139c network card. > > -- > You received this bug notification because you are subscribed to the bug > report. > https://bugs.launchpad.net/bugs/1180777 > > Title: > RDP traffic freeze on quiet network > > Status in QEMU: > New > Status in “qemu-kvm” package in Ubuntu: > Confirmed > Status in Debian GNU/Linux: > New > > Bug description: > To summarize what I think has been found so far, > > 1. The main symptom is that RDP connections hang after some time > 2. This bug affects qemu 1.0 .. 1.6.5 > 3. This bug affects at least windows xp and windows 7 guests > 4. Keeping another network connection open, such as vnc, prevents the > RDP connection from hanging. > > > Hi, > > I have recently setup a Windows 7 VM on KVM and started using it > through remote desktop. > > What happens is that, after some hours of usage, the remote desktop > connection freezes. I thought it was a remmina bug, as the it was > enough to kill and restart it to successfully connect again to the VM. > > However, today I've switched to a different RDP client (2X Client > chromium app) and the freeze just happened again! > > Some information: > - the host and the VM are completely idle when the freeze occurs > - I've tried sniffing the network packets toward the RDP port during the > freeze and found that the client is sending packets but no packet is sent > back > > Could this be a KVM issue? How can I further debug this one (I expect > the freeze to happen again...)? > > ProblemType: Bug > DistroRelease: Ubuntu 12.04 > Package: kvm 1:84+dfsg-0ubuntu16+1.0+noroms+0ubuntu14.8 > ProcVersionSignature: Ubuntu 3.2.0-41.66-generic 3.2.42 > Uname: Linux 3.2.0-41-generic x86_64 > ApportVersion: 2.0.1-0ubuntu17.2 > Architecture: amd64 > Date: Thu May 16 14:12:40 2013 > MachineType: Hewlett-Packard HP ProBook 4520s > MarkForUpload: True > ProcEnviron: >TERM=xterm >PATH=(custom, no user) >LANG=en_US.UTF-8 >SHELL=/bin/bash > ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinuz-3.2.0-41-generic > root=UUID=D2E20BC3E20BAAB5 loop=/hostname/disks/root.disk ro quiet splash > vt.handoff=7 > SourcePackage: qemu-kvm > UpgradeStatus: No upgrade log present (probably fresh install) > dmi.bios.date: 08/26/2010 > dmi.bios.vendor: Hewlett-Packard > dmi.bios.version: 68AZZ Ver. F.0A > dmi.board.name: 1411 > dmi.board.vendor: Hewlett-Packard > dmi.board.version: KBC Version 57.30 > dmi.chassis.type: 10 > dmi.chassis.vendor: Hewlett-Packard > dmi.modalias: > dmi:bvnHewlett-Packard:bvr68AZZVer.F.0A:bd08/26/2010:svnHewlett-Packard:pnHPProBook4520s:pvr:rvnHewlett-Packard:rn1411:rvrKBCVersion57.30:cvnHewlett-Packard:ct10:cvr: > dmi.product.name: HP ProBook 4520s > dmi.sys.vendor: Hewlett-Packard > > To manage notifications about this bug go to: > https://bugs.launchpad.net/qemu/+bug/1180777/+subscriptions > -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1180777 Title: RDP traffic freeze on quiet network Status in QEMU: New Status in “qemu-kvm” package in Ubuntu: Confirmed Status in Debian GNU/Linux: New Bug description: To summarize what I think has been found so far, 1. The main symptom is that RDP connections hang after some time 2. This bug affects qemu 1.0 .. 1.6.5 3. This bug affects at least windows xp and windows 7 guests 4. Keeping another network connection open, such as vnc, prevents the RDP connection from hanging. Hi, I have recently setup a Windows 7 VM on KVM and started using it through remote desktop. What happens is that, after some hours of usage, the remote desktop connection freezes. I thought it was a remmina bug, as the it was enough to kill and restart it to successfully connect again to the VM. However, today I've switched to a different RDP client (2X Client chromium app) and the freeze just happened again! Some information: - the host and the VM are completely idle when the freeze occurs - I've tried sniffing the network packets toward the RDP port during the freeze and found that the client is sending packets but no packet is sent back Could this be a KVM issue? How can I further debug this one (I expect the freeze to happen again...)? ProblemType: Bug DistroRelease: Ubuntu 12.04 Package: kvm 1:84+dfsg-0ubuntu16+1.0+noroms+0ubuntu14
Re: [Qemu-devel] [PATCH 6/8] [PATCH RFC v3] s390-qemu: cpu hotplug - s390 cpu init improvements for hotplug
On 09/13/2013 11:24 AM, Jason J. Herne wrote:
On 09/05/2013 08:28 AM, Andreas Färber wrote:
Am 01.08.2013 16:12, schrieb Jason J. Herne:
From: "Jason J. Herne"
s390_new_cpu is created to encapsulate the creation of a new QOM
S390CPU
object given a cpuid and a model string.
All actual cpu initialization code is moved from boot time
specific functions to
s390_cpu_initfn (qom init routine) or to s390_new_cpu. This is
done to allow us
to use the same basic code path for a cpu created at boot time
and one created
during a hotplug operation.
Intentionally indented?
Signed-off-by: Jason J. Herne
---
hw/s390x/s390-virtio.c | 25 -
target-s390x/cpu.c |4 ++--
target-s390x/cpu.h |1 +
target-s390x/helper.c | 12
4 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 5ad9cf3..103f32e 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
...
@@ -197,19 +202,13 @@ void s390_init_cpus(const char *cpu_model)
cpu_model = "host";
}
-ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
-
-for (i = 0; i < smp_cpus; i++) {
-S390CPU *cpu;
-CPUState *cs;
+ipi_states = g_malloc(sizeof(S390CPU *) * max_cpus);
-cpu = cpu_s390x_init(cpu_model);
-cs = CPU(cpu);
-
-ipi_states[i] = cpu;
-cs->halted = 1;
-cpu->env.exception_index = EXCP_HLT;
-cpu->env.storage_keys = s390_get_storage_keys();
+for (i = 0; i < max_cpus; i++) {
+ipi_states[i] = NULL;
Using g_malloc0() above would hopefully be more efficient and would
allow to leave the loop untouched for easier review.
I don't follow. I'm completely changing this loop. I do not believe we
can obtain the same functionality implemented here while not touching
the loop.
+if (i < smp_cpus) {
+s390_new_cpu(cpu_model, i);
+}
}
}
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 6be6c08..c90a91c 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -116,7 +116,6 @@ static void s390_cpu_initfn(Object *obj)
S390CPU *cpu = S390_CPU(obj);
CPUS390XState *env = &cpu->env;
static bool inited;
-static int cpu_num = 0;
#if !defined(CONFIG_USER_ONLY)
struct tm tm;
#endif
@@ -135,8 +134,9 @@ static void s390_cpu_initfn(Object *obj)
* cpu counter in s390_cpu_reset to a negative number at
* initial ipl */
cs->halted = 1;
+cpu->env.exception_index = EXCP_HLT;
+env->storage_keys = s390_get_storage_keys();
4/8?
Are you asking if this belongs in patch #4? if so, I would say no. It
does deal with storage keys, yes. But we're not changing storage key
semantics here (as we are in patch 4), we're just moving where the
storage key ptr gets set. This is in support of re-organizing how cpus
are initialized as per patch title/description.
Ping? :)
--
-- Jason J. Herne (jjhe...@linux.vnet.ibm.com)
Re: [Qemu-devel] [PATCH 5/8] [PATCH RFC v3] s390-qemu: cpu hotplug - ipi_states enhancements
On 09/19/2013 04:19 PM, Jason J. Herne wrote:
On 09/05/2013 08:01 AM, Andreas Färber wrote:
Am 01.08.2013 16:12, schrieb Jason J. Herne:
From: "Jason J. Herne"
...
This is what got us into the link<> discussion last time. If we do
for (i = 0; i < ARRAY_SIZE(ipi_states); i++) {
name = g_strdup_printf("cpu[%i]", i);
object_property_add_link(qdev_get_machine(), name, TYPE_S390_CPU,
&ipi_states[i], &err);
}
then we get said /machine/cpu[n] link<> properties, at a QMP level
either returning nothing or the canonical path to the CPU object.
On IRC I didn't get an answer of whether it was being done the above way
because there is infrastructure missing, and a look at object.h now
confirms that suspicion. CC'ing Anthony and Paolo.
Since object_property_add_link() uses a NULL opaque, my idea would be to
add a single setter hook argument passed through as opaque to
object_set_link_property(), which would call it with the old and the new
value.
The purpose would be to avoid growing our own internal setter API, which
is disjoint from the QMP qom-set we are targetting at.
I wrote the code, very close to how you suggested and it appears to be
working when tested with qom-list. I'm still not certain why we need
the ability to set the opaque of object_set_link_property()?
For reference here is what I did:
void s390_init_cpus(const char *cpu_model)
{
int i;
char* name;
if (cpu_model == NULL) {
cpu_model = "host";
}
ipi_states = g_malloc0(sizeof(S390CPU *) * max_cpus);
for (i = 0; i < max_cpus; i++) {
name = g_strdup_printf("cpu[%i]", i);
object_property_add_link(qdev_get_machine(), name, TYPE_S390_CPU,
(Object **)&ipi_states[i], NULL);
}
for (i = 0; i < smp_cpus; i++) {
cpu_s390x_init(cpu_model);
}
}
Yep, I know cpu_model is going away ;).
Ping? :)
--
-- Jason J. Herne (jjhe...@linux.vnet.ibm.com)
Re: [Qemu-devel] [PATCH] coroutine: add ./configure --disable-coroutine-pool
Am 02.10.2013 10:54, schrieb Stefan Hajnoczi: > This is an interesting backtrace. The 'current' thread-local variable > from coroutine-win32.c is NULL or doesn't have a caller assigned. > > Please post 'thread apply all bt' so we can identify the other threads. > > Stefan The other threads don't have a reasonable backtrace: (gdb) thread apply all bt Thread 3 (Thread 5708.0x15bc): #0 0x76118e76 in msvcrt!abort () from C:\Windows\syswow64\msvcrt.dll #1 0x7611680c in msvcrt!_assert () from C:\Windows\syswow64\msvcrt.dll #2 0x0051957d in qemu_co_queue_restart_all (queue=queue@entry=0x6e5fe90) at c:/cygwin/home/stweil/src/qemu/qemu.org/qemu/qemu-coroutine-lock.c:99 #3 0x0040f4d1 in tracked_request_end (req=0x6e5fe6c) at c:/cygwin/home/stweil/src/qemu/qemu.org/qemu/block.c:1963 #4 bdrv_co_do_readv (bs=0x31e78f0, sector_num=, nb_sectors=4, qiov=0x745f9d0, flags=) at c:/cygwin/home/stweil/src/qemu/qemu.org/qemu/block.c:2675 #5 0x0040f4a2 in bdrv_co_do_readv (bs=0x31e5ea8, sector_num=, nb_sectors=4, qiov=0x745f9d0, flags=) at c:/cygwin/home/stweil/src/qemu/qemu.org/qemu/block.c:2645 #6 0x0041060c in bdrv_rw_co_entry (opaque=0x745f968) at c:/cygwin/home/stweil/src/qemu/qemu.org/qemu/block.c:2276 #7 0x00442238 in coroutine_trampoline (co_=0x31e8bf0) at c:/cygwin/home/stweil/src/qemu/qemu.org/qemu/coroutine-win32.c:57 #8 0x7549bfa2 in KERNEL32!GetQueuedCompletionStatus () from C:\Windows\syswow64\kernel32.dll #9 0x031e8bf0 in ?? () #10 0x7549bf5a in KERNEL32!GetQueuedCompletionStatus () from C:\Windows\syswow64\kernel32.dll #11 0x016d24e0 in ?? () Thread 2 (Thread 5708.0xe1c): #0 0x7749f8d1 in ntdll!RtlUpdateClonedSRWLock () from C:\Windows\system32\ntdll.dll #1 0x7749f8d1 in ntdll!RtlUpdateClonedSRWLock () from C:\Windows\system32\ntdll.dll #2 0x759c149d in WaitForSingleObjectEx () from C:\Windows\syswow64\KernelBase.dll #3 0x0150 in ?? () #4 0x in ?? () Thread 1 (Thread 5708.0x1600): #0 0x7749f8d1 in ntdll!RtlUpdateClonedSRWLock () from C:\Windows\system32\ntdll.dll #1 0x7749f8d1 in ntdll!RtlUpdateClonedSRWLock () from C:\Windows\system32\ntdll.dll #2 0x774b8e44 in ntdll!TpCallbackMayRunLong () from C:\Windows\system32\ntdll.dll #3 0x0160 in ?? () #4 0x in ?? ()
[Qemu-devel] [PATCH] util/path: Fix type which is longer than 8 bit for MinGW
While dirent->d_type is 8 bit for most systems, it is 32 bit for MinGW.
Reducing it to 8 bit results in a compiler warning because the macro
is_dir_maybe compares that 8 bit value with 32 bit constants.
Using 'unsigned' instead of 'unsigned char' matches the declaration for
MinGW and does not harm the other systems.
MinGW-w32 is not affected: it does not declare d_type.
Signed-off-by: Stefan Weil
---
util/path.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/util/path.c b/util/path.c
index f0c6962..623219e 100644
--- a/util/path.c
+++ b/util/path.c
@@ -39,7 +39,7 @@ static int strneq(const char *s1, unsigned int n, const char
*s2)
}
static struct pathelem *add_entry(struct pathelem *root, const char *name,
- unsigned char type);
+ unsigned type);
static struct pathelem *new_entry(const char *root,
struct pathelem *parent,
@@ -82,7 +82,7 @@ static struct pathelem *add_dir_maybe(struct pathelem *path)
}
static struct pathelem *add_entry(struct pathelem *root, const char *name,
- unsigned char type)
+ unsigned type)
{
struct pathelem **e;
--
1.7.10.4
Re: [Qemu-devel] [Qemu-trivial] [PATCH] migration: Fix compiler warning ('caps' may be used uninitialized)
Am 02.10.2013 21:02, schrieb Michael Tokarev:
> How about this:
>
> diff --git a/migration.c b/migration.c
> index b4f8462..6066ab4 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -146,22 +146,16 @@ uint64_t migrate_max_downtime(void)
> MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error
> **errp)
> {
> MigrationCapabilityStatusList *head = NULL;
> -MigrationCapabilityStatusList *caps;
> +MigrationCapabilityStatusList **capp = &head;
> MigrationState *s = migrate_get_current();
> int i;
>
> for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
> -if (head == NULL) {
> -head = g_malloc0(sizeof(*caps));
> -caps = head;
> -} else {
> -caps->next = g_malloc0(sizeof(*caps));
> -caps = caps->next;
> -}
> -caps->value =
> -g_malloc(sizeof(*caps->value));
> -caps->value->capability = i;
> -caps->value->state = s->enabled_capabilities[i];
> +*capp = g_malloc0(sizeof(*head));
> +(*capp)->value = g_malloc(sizeof(*head->value));
> +(*capp)->value->capability = i;
> +(*capp)->value->state = s->enabled_capabilities[i];
> + capp = &(*capp)->next;
> }
>
> return head;
>
>
> This is what I had in mind at the very beginnig, but only now tried
> to make a patch...
>
> Thanks,
>
> /mjt
> Thanks,
That's a possible solution. Paolo also sent a sketch of a similar
solution. And here are two more:
MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
{
MigrationCapabilityStatusList *head = NULL;
MigrationState *s = migrate_get_current();
MigrationCapability i = MIGRATION_CAPABILITY_MAX;
do {
MigrationCapabilityStatusList *caps =
g_new(MigrationCapabilityStatusList, 1);
i--;
caps->next = head;
caps->value = g_new(MigrationCapabilityStatus, 1);
caps->value->capability = i;
caps->value->state = s->enabled_capabilities[i];
head = caps;
} while (i > 0);
return head;
}
MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
{
MigrationCapabilityStatusList *head = NULL;
MigrationCapabilityStatusList *prev = NULL;
MigrationState *s = migrate_get_current();
MigrationCapability i;
for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
MigrationCapabilityStatusList *caps =
g_new(MigrationCapabilityStatusList, 1);
if (prev == NULL) {
head = caps;
} else {
prev->next = caps;
prev = caps;
}
caps->value = g_new(MigrationCapabilityStatus, 1);
caps->value->capability = i;
caps->value->state = s->enabled_capabilities[i];
}
return head;
}
Which one do we take? Any correct solution which fixes the compiler
warning is fine for me (although I prefer g_new instead of g_malloc as
you might have guessed). :-)
Regards,
Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] migration: Fix compiler warning ('caps' may be used uninitialized)
How about this:
diff --git a/migration.c b/migration.c
index b4f8462..6066ab4 100644
--- a/migration.c
+++ b/migration.c
@@ -146,22 +146,16 @@ uint64_t migrate_max_downtime(void)
MigrationCapabilityStatusList *qmp_query_migrate_capabilities(Error **errp)
{
MigrationCapabilityStatusList *head = NULL;
-MigrationCapabilityStatusList *caps;
+MigrationCapabilityStatusList **capp = &head;
MigrationState *s = migrate_get_current();
int i;
for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
-if (head == NULL) {
-head = g_malloc0(sizeof(*caps));
-caps = head;
-} else {
-caps->next = g_malloc0(sizeof(*caps));
-caps = caps->next;
-}
-caps->value =
-g_malloc(sizeof(*caps->value));
-caps->value->capability = i;
-caps->value->state = s->enabled_capabilities[i];
+*capp = g_malloc0(sizeof(*head));
+(*capp)->value = g_malloc(sizeof(*head->value));
+(*capp)->value->capability = i;
+(*capp)->value->state = s->enabled_capabilities[i];
+ capp = &(*capp)->next;
}
return head;
This is what I had in mind at the very beginnig, but only now tried
to make a patch...
Thanks,
/mjt
Thanks,
Re: [Qemu-devel] [Qemu-trivial] [PATCH v3 1/2] tests: Fix schema parser test for in-tree build
24.09.2013 11:43, arm...@redhat.com wrote: From: Markus Armbruster Commit 4f193e3 added the test, but screwed up in-tree builds (SRCDIR=.): the tests's output overwrites the expected output, and is thus compared to itself. Thanks, applied to the trivial patches queue. The `clean' target for tests/ would be nice to have, too... ;) /mjt
Re: [Qemu-devel] [PATCH] Update MAINTAINERS
On 10/02/2013 12:09:58 PM, Anthony Liguori wrote: All of Paul's emails are bouncing and he hasn't been active for some time. ... M68K -M: Paul Brook -S: Odd Fixes +S: Orphan F: target-m68k/ F: hw/m68k/ Laurent Vivier has an m68k gitorious branch to add the q800 target, which I've occasionally tested and would really really like to see finished and merged. Alas, last time I tested it the sucker died during the kernel boot as soon as mmu setup tried to enable the page tables. But before that I got three lines printk'd! (Woo! Progress!) Still: possible m68k guy? Maybe? If he's interested? Rob
[Qemu-devel] [PATCH] block: use correct filename
The content filename point to may be erased by qemu_opts_absorb_qdict()
in raw_open_common() in drv->bdrv_file_open()
So it's better to use bs->filename.
Signed-off-by: Dunrong Huang
---
block.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 93e113a..469d842 100644
--- a/block.c
+++ b/block.c
@@ -824,8 +824,8 @@ static int bdrv_open_common(BlockDriverState *bs,
BlockDriverState *file,
#ifndef _WIN32
if (bs->is_temporary) {
-assert(filename != NULL);
-unlink(filename);
+assert(bs->filename[0] != '\0');
+unlink(bs->filename);
}
#endif
return 0;
--
1.7.12.4 (Apple Git-37)
Re: [Qemu-devel] [PATCH] Update MAINTAINERS
Am 02.10.2013 19:09, schrieb Anthony Liguori: > All of Paul's emails are bouncing and he hasn't been active for > some time. > > Signed-off-by: Anthony Liguori > --- > MAINTAINERS | 21 + > 1 file changed, 5 insertions(+), 16 deletions(-) $ echo -n p...@codesourcery.com | md5sum e38859654f155fa02b0996bb01b883fe - => http://www.gravatar.com/avatar/e38859654f155fa02b0996bb01b883fe Same image can be found on https://github.com/pbrook?tab=activity which shows recent activity, so he's not on offline vacation. I had sent an email to the address used in his commit messages (CC'ed) on Sep 4th asking to update or remove his email address in our MAINTAINERS file, without response nearly a month later. Pings on OFTC #qemu remained unanswered, too. Therefore removal seems the right thing to do under the circumstances, Reviewed-by: Andreas Färber Thanks. > diff --git a/MAINTAINERS b/MAINTAINERS > index 5c3c70c..ab8166a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS [...] > @@ -567,8 +557,7 @@ F: hw/scsi/* > T: git git://github.com/bonzini/qemu.git scsi-next > > LSI53C895A > -M: Paul Brook > -S: Odd Fixes > +S: Orphan > F: hw/scsi/lsi53c895a.c > > SSI CC'ing Paolo as SCSI maintainer. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v6 0/5] Do not set SO_REUSEADDR on Windows
Am 02.10.2013 14:28, schrieb Eric Blake: > On 10/02/2013 04:23 AM, Sebastian Ottlik wrote: >> This patchset disables most uses of SO_REUSEADDR on Windows and replaces it with >> calls to the new function socket_set_fast_reuse. On Windows systems the default >> behaviour is equivalent to SO_REUSEADDR on other operating systems. SO_REUSEADDR >> can still be set but results in undesired behaviour in most cases. It may even >> lead to situations were system behaviour is unspecified. More information on >> this can be found at: >> http://msdn.microsoft.com/en-us/library/windows/desktop/ms740621.aspx >> >> I originally encountered this issue when accidentally launching two QEMU >> instances with identical GDB ports at the same time. In which case QEMU won't >> fail as one might expect. >> >> Note that patch #4 fails checkpatch.pl. This is intentional (see v3 changes). >> >> v6 Changes: >> - dropped error output and the silent parameter in favor of an assertion >> >> Actually I wanted to remove the return value from the function too, as the >> assertion pretty much states that the function will not fail and thus always >> return 0. However this would make the code a little ugly to prevent unused >> variable warnings if NDEBUG is set (see patch 1) and also would require some >> ugly changes to slirp/socket.c (see patch 4). Thus I decided to keep it. >> >> - Rebased to current master (a684f3cf9b9b9c3cb82be87aafc463de8974610c) >> > > Series: Reviewed-by: Eric Blake Thanks for the patches and for the review. I applied all to my mingw patch queue and have sent a pull request. Stefan
[Qemu-devel] [PULL 5/5] util: call socket_set_fast_reuse instead of setting SO_REUSEADDR
From: Sebastian Ottlik
SO_REUSEADDR should be avoided on Windows but is desired on other operating
systems. So instead of setting it we call socket_set_fast_reuse that will result
in the appropriate behaviour on all operating systems.
Signed-off-by: Sebastian Ottlik
Reviewed-by: Eric Blake
Signed-off-by: Stefan Weil
---
util/qemu-sockets.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 095716e..6b97dc1 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -155,7 +155,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error
**errp)
continue;
}
-qemu_setsockopt(slisten, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
+socket_set_fast_reuse(slisten);
#ifdef IPV6_V6ONLY
if (e->ai_family == PF_INET6) {
/* listen on both ipv4 and ipv6 */
@@ -274,7 +274,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool
*in_progress,
error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
return -1;
}
-qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
+socket_set_fast_reuse(sock);
if (connect_state != NULL) {
qemu_set_nonblock(sock);
}
@@ -455,7 +455,7 @@ int inet_dgram_opts(QemuOpts *opts, Error **errp)
error_set_errno(errp, errno, QERR_SOCKET_CREATE_FAILED);
goto err;
}
-qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
+socket_set_fast_reuse(sock);
/* bind socket */
if (bind(sock, local->ai_addr, local->ai_addrlen) < 0) {
--
1.7.10.4
[Qemu-devel] [PULL 3/5] net: call socket_set_fast_reuse instead of setting SO_REUSEADDR
From: Sebastian Ottlik
SO_REUSEADDR should be avoided on Windows but is desired on other operating
systems. So instead of setting it we call socket_set_fast_reuse that will result
in the appropriate behaviour on all operating systems.
An exception to this rule are multicast sockets where it is sensible to have
multiple sockets listen on the same ip and port and we should set SO_REUSEADDR
on windows.
Signed-off-by: Sebastian Ottlik
Reviewed-by: Eric Blake
Signed-off-by: Stefan Weil
---
net/socket.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index e61309d..fb21e20 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -262,6 +262,11 @@ static int net_socket_mcast_create(struct sockaddr_in
*mcastaddr, struct in_addr
return -1;
}
+/* Allow multiple sockets to bind the same multicast ip and port by setting
+ * SO_REUSEADDR. This is the only situation where SO_REUSEADDR should be
set
+ * on windows. Use socket_set_fast_reuse otherwise as it sets SO_REUSEADDR
+ * only on posix systems.
+ */
val = 1;
ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
if (ret < 0) {
@@ -510,7 +515,7 @@ static int net_socket_listen_init(NetClientState *peer,
NetClientState *nc;
NetSocketState *s;
struct sockaddr_in saddr;
-int fd, val, ret;
+int fd, ret;
if (parse_host_port(&saddr, host_str) < 0)
return -1;
@@ -522,9 +527,7 @@ static int net_socket_listen_init(NetClientState *peer,
}
qemu_set_nonblock(fd);
-/* allow fast reuse */
-val = 1;
-qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
+socket_set_fast_reuse(fd);
ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
if (ret < 0) {
@@ -645,7 +648,7 @@ static int net_socket_udp_init(NetClientState *peer,
const char *lhost)
{
NetSocketState *s;
-int fd, val, ret;
+int fd, ret;
struct sockaddr_in laddr, raddr;
if (parse_host_port(&laddr, lhost) < 0) {
@@ -661,11 +664,9 @@ static int net_socket_udp_init(NetClientState *peer,
perror("socket(PF_INET, SOCK_DGRAM)");
return -1;
}
-val = 1;
-ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
- &val, sizeof(val));
+
+ret = socket_set_fast_reuse(fd);
if (ret < 0) {
-perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
closesocket(fd);
return -1;
}
--
1.7.10.4
[Qemu-devel] [PULL 1/5] util: add socket_set_fast_reuse function which will replace setting SO_REUSEADDR
From: Sebastian Ottlik
If a socket is closed it remains in TIME_WAIT state for some time. On operating
systems using BSD sockets the endpoint of the socket may not be reused while in
this state unless SO_REUSEADDR was set on the socket. On windows on the other
hand the default behaviour is to allow reuse (i.e. identical to SO_REUSEADDR on
other operating systems) and setting SO_REUSEADDR on a socket allows it to be
bound to a endpoint even if the endpoint is already used by another socket
independently of the other sockets state. This can even result in undefined
behaviour.
Many sockets used by QEMU should not block the use of their endpoint after being
closed while they are still in TIME_WAIT state. Currently QEMU sets SO_REUSEADDR
for such sockets, which can lead to problems on Windows. This patch introduces
the function socket_set_fast_reuse that should be used instead of setting
SO_REUSEADDR when fast socket reuse is desired and behaves correctly on all
operating systems.
As a failure of this function can only be caused by bad QEMU internal errors, an
assertion handles these situations. The return value is still passed on, to
minimize changes in client code and prevent unused variable warnings if NDEBUG
is defined.
Signed-off-by: Sebastian Ottlik
Reviewed-by: Eric Blake
Signed-off-by: Stefan Weil
---
include/qemu/sockets.h |1 +
util/oslib-posix.c | 12
util/oslib-win32.c | 10 ++
3 files changed, 23 insertions(+)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index c5174d7..45588d7 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -39,6 +39,7 @@ int socket_set_cork(int fd, int v);
int socket_set_nodelay(int fd);
void qemu_set_block(int fd);
void qemu_set_nonblock(int fd);
+int socket_set_fast_reuse(int fd);
int send_all(int fd, const void *buf, int len1);
int recv_all(int fd, void *buf, int len1, bool single_read);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 253bc3d..e00a44c 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -157,6 +157,18 @@ void qemu_set_nonblock(int fd)
fcntl(fd, F_SETFL, f | O_NONBLOCK);
}
+int socket_set_fast_reuse(int fd)
+{
+int val = 1, ret;
+
+ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
+ (const char *)&val, sizeof(val));
+
+assert(ret == 0);
+
+return ret;
+}
+
void qemu_set_cloexec(int fd)
{
int f;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 983b7a2..776ccfa 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -124,6 +124,16 @@ void qemu_set_nonblock(int fd)
qemu_fd_register(fd);
}
+int socket_set_fast_reuse(int fd)
+{
+/* Enabling the reuse of an endpoint that was used by a socket still in
+ * TIME_WAIT state is usually performed by setting SO_REUSEADDR. On Windows
+ * fast reuse is the default and SO_REUSEADDR does strange things. So we
+ * don't have to do anything here. More info can be found at:
+ * http://msdn.microsoft.com/en-us/library/windows/desktop/ms740621.aspx */
+return 0;
+}
+
int inet_aton(const char *cp, struct in_addr *ia)
{
uint32_t addr = inet_addr(cp);
--
1.7.10.4
[Qemu-devel] [PULL 2/5] gdbstub: call socket_set_fast_reuse instead of setting SO_REUSEADDR
From: Sebastian Ottlik
SO_REUSEADDR should be avoided on Windows but is desired on other operating
systems. So instead of setting it we call socket_set_fast_reuse that will result
in the appropriate behaviour on all operating systems.
Signed-off-by: Sebastian Ottlik
Reviewed-by: Eric Blake
Signed-off-by: Stefan Weil
---
gdbstub.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 2b7f22b..0e5a3f5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1553,7 +1553,7 @@ static void gdb_accept(void)
static int gdbserver_open(int port)
{
struct sockaddr_in sockaddr;
-int fd, val, ret;
+int fd, ret;
fd = socket(PF_INET, SOCK_STREAM, 0);
if (fd < 0) {
@@ -1564,9 +1564,7 @@ static int gdbserver_open(int port)
fcntl(fd, F_SETFD, FD_CLOEXEC);
#endif
-/* allow fast reuse */
-val = 1;
-qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
+socket_set_fast_reuse(fd);
sockaddr.sin_family = AF_INET;
sockaddr.sin_port = htons(port);
--
1.7.10.4
[Qemu-devel] [PULL 4/5] slirp: call socket_set_fast_reuse instead of setting SO_REUSEADDR
From: Sebastian Ottlik
SO_REUSEADDR should be avoided on Windows but is desired on other operating
systems. So instead of setting it we call socket_set_fast_reuse that will result
in the appropriate behaviour on all operating systems.
Signed-off-by: Sebastian Ottlik
Reviewed-by: Eric Blake
Signed-off-by: Stefan Weil
---
slirp/misc.c |3 +--
slirp/socket.c |4 +---
slirp/tcp_subr.c |6 ++
slirp/udp.c |4 ++--
4 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/slirp/misc.c b/slirp/misc.c
index c0d4899..6c1636f 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -212,8 +212,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
so->s = accept(s, (struct sockaddr *)&addr, &addrlen);
} while (so->s < 0 && errno == EINTR);
closesocket(s);
-opt = 1;
-qemu_setsockopt(so->s, SOL_SOCKET, SO_REUSEADDR, &opt,
sizeof(int));
+socket_set_fast_reuse(so->s);
opt = 1;
qemu_setsockopt(so->s, SOL_SOCKET, SO_OOBINLINE, &opt,
sizeof(int));
qemu_set_nonblock(so->s);
diff --git a/slirp/socket.c b/slirp/socket.c
index 25d60e7..37ac5cf 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -627,9 +627,7 @@ tcp_listen(Slirp *slirp, uint32_t haddr, u_int hport,
uint32_t laddr,
addr.sin_port = hport;
if (((s = qemu_socket(AF_INET,SOCK_STREAM,0)) < 0) ||
-#ifndef _WIN32
- (qemu_setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(int)) <
0) ||
-#endif
+ (socket_set_fast_reuse(s) < 0) ||
(bind(s,(struct sockaddr *)&addr, sizeof(addr)) < 0) ||
(listen(s,1) < 0)) {
int tmperrno = errno; /* Don't clobber the real reason we
failed */
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index 043f28f..7571c5a 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -337,8 +337,7 @@ int tcp_fconnect(struct socket *so)
struct sockaddr_in addr;
qemu_set_nonblock(s);
-opt = 1;
-qemu_setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
+socket_set_fast_reuse(s);
opt = 1;
qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(opt));
@@ -426,8 +425,7 @@ void tcp_connect(struct socket *inso)
return;
}
qemu_set_nonblock(s);
-opt = 1;
-qemu_setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(int));
+socket_set_fast_reuse(s);
opt = 1;
qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(int));
socket_set_nodelay(s);
diff --git a/slirp/udp.c b/slirp/udp.c
index b105f87..8cc6cb6 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -354,7 +354,7 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport,
uint32_t laddr,
{
struct sockaddr_in addr;
struct socket *so;
- socklen_t addrlen = sizeof(struct sockaddr_in), opt = 1;
+ socklen_t addrlen = sizeof(struct sockaddr_in);
so = socreate(slirp);
if (!so) {
@@ -372,7 +372,7 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport,
uint32_t laddr,
udp_detach(so);
return NULL;
}
- qemu_setsockopt(so->s, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(int));
+ socket_set_fast_reuse(so->s);
getsockname(so->s,(struct sockaddr *)&addr,&addrlen);
so->so_fport = addr.sin_port;
--
1.7.10.4
[Qemu-devel] [PULL 0/5] net: Add and use new function socket_set_fast_reuse
The following changes since commit a684f3cf9b9b9c3cb82be87aafc463de8974610c: Merge remote-tracking branch 'kraxel/seabios-1.7.3.2' into staging (2013-09-30 17:15:27 -0500) are available in the git repository at: git://qemu.weilnetz.de/qemu.git mingw for you to fetch changes up to 04fd1c789677fe121cb9546c652d088c994477fb: util: call socket_set_fast_reuse instead of setting SO_REUSEADDR (2013-10-02 19:20:31 +0200) Sebastian Ottlik (5): util: add socket_set_fast_reuse function which will replace setting SO_REUSEADDR gdbstub: call socket_set_fast_reuse instead of setting SO_REUSEADDR net: call socket_set_fast_reuse instead of setting SO_REUSEADDR slirp: call socket_set_fast_reuse instead of setting SO_REUSEADDR util: call socket_set_fast_reuse instead of setting SO_REUSEADDR gdbstub.c |6 ++ include/qemu/sockets.h |1 + net/socket.c | 19 ++- slirp/misc.c |3 +-- slirp/socket.c |4 +--- slirp/tcp_subr.c |6 ++ slirp/udp.c|4 ++-- util/oslib-posix.c | 12 util/oslib-win32.c | 10 ++ util/qemu-sockets.c|6 +++--- 10 files changed, 44 insertions(+), 27 deletions(-) [PULL 1/5] util: add socket_set_fast_reuse function which will [PULL 2/5] gdbstub: call socket_set_fast_reuse instead of setting [PULL 3/5] net: call socket_set_fast_reuse instead of setting [PULL 4/5] slirp: call socket_set_fast_reuse instead of setting [PULL 5/5] util: call socket_set_fast_reuse instead of setting
Re: [Qemu-devel] ioh3420: Add a map_irq function
On 10/02/2013 10:12 AM, Alex Williamson wrote:
> On Fri, 2013-09-27 at 15:10 -0700, Alexander Duyck wrote:
>> On 02/28/2013 10:49 AM, Alex Williamson wrote:
>>> Every bridge needs to know how to map IRQs from it's secondary bus to
>>> the primary bus. We seem to be direct mapped on ioh3420. This avoids
>>> segfaults when trying to put assigned devices behind root ports.
>>>
>>> Signed-off-by: Alex Williamson
>>>
>>> ---
>>> hw/ioh3420.c |7 +++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/hw/ioh3420.c b/hw/ioh3420.c
>>> index 95bceb5..6ac4fe7 100644
>>> --- a/hw/ioh3420.c
>>> +++ b/hw/ioh3420.c
>>> @@ -90,6 +90,11 @@ static void ioh3420_reset(DeviceState *qdev)
>>> pci_bridge_disable_base_limit(d);
>>> }
>>>
>>> +static int ioh3420_map_irq(PCIDevice *pci_dev, int irq_num)
>>> +{
>>> +return irq_num;
>>> +}
>>> +
>>> static int ioh3420_initfn(PCIDevice *d)
>>> {
>>> PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
>>> @@ -97,6 +102,8 @@ static int ioh3420_initfn(PCIDevice *d)
>>> PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
>>> int rc;
>>>
>>> +pci_bridge_map_irq(br, NULL, ioh3420_map_irq);
>>> +
>>> rc = pci_bridge_initfn(d);
>>> if (rc < 0) {
>>> return rc;
>>>
>> What became of this patch? I believe I am seeing the issue described
>> when I assign a MSI-X capable virtual device to the secondary bus of an
>> ioh3420, and applying this patch seems to resolve the issue. Was there
>> an alternate patch proposed and if so where can I find it?
> Hmm, I thought it was no longer necessary with
> pci_device_route_intx_to_irq(), especially if you're assigning a VF that
> doesn't support INTx. Are you using the latest qemu bits? pci-assign
> or vfio-pci? Maybe provide a backtrace? Thanks,
>
> Alex
I'm seeing it with an emulated interface using MSI-X, not with a VF or
assigned device.
I'll try to get you a backtrack when I have some time. I'm currently
chasing down another issue and will try to remember to follow up on this
one.
Thanks,
Alex
[Qemu-devel] [Bug 1233225] Re: mips/mipsel linux user float division problem
For system emulation I used the default which is malta.
cheers, josch
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1233225
Title:
mips/mipsel linux user float division problem
Status in QEMU:
Confirmed
Bug description:
Hi,
I tested the following with the qemu git HEAD as of 2013-09-30 on
Debian stable and testing. My host runs amd64 but I also tried this
out inside a i386 chroot with the same result. The problem occurs for
mips and mipsel. Given the following program:
#include
int main(int argc, char **argv)
{
int a = 1;
double d = a/2.0;
printf("%f\n", d);
return 0;
}
Instead of printing 0.5, it will print 2.0 if executed in qemu user
mode.
$ mipsel-linux-gnu-gcc mipstest.c
$ ~/qemu/mipsel-linux-user/qemu-mipsel ./a.out
2.0
Expecting this to be a problem with my cross compiler (gcc-4.4 from
emdebian) I ran a fully emulated debian squeeze environment inside
qemu. There, I compiled the same program natively with gcc and as
expected got 0.5 as the output. I also copied the cross compiled
binary inside the emulated environment and also got 0.5 when I ran it.
So the same mips/mipsel binary produces different output depending on
whether it is run in a fully emulated environment or qemu user mode.
Can anybody else reproduce this problem?
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1233225/+subscriptions
Re: [Qemu-devel] ioh3420: Add a map_irq function
On Fri, 2013-09-27 at 15:10 -0700, Alexander Duyck wrote:
> On 02/28/2013 10:49 AM, Alex Williamson wrote:
> > Every bridge needs to know how to map IRQs from it's secondary bus to
> > the primary bus. We seem to be direct mapped on ioh3420. This avoids
> > segfaults when trying to put assigned devices behind root ports.
> >
> > Signed-off-by: Alex Williamson
> >
> > ---
> > hw/ioh3420.c |7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/hw/ioh3420.c b/hw/ioh3420.c
> > index 95bceb5..6ac4fe7 100644
> > --- a/hw/ioh3420.c
> > +++ b/hw/ioh3420.c
> > @@ -90,6 +90,11 @@ static void ioh3420_reset(DeviceState *qdev)
> > pci_bridge_disable_base_limit(d);
> > }
> >
> > +static int ioh3420_map_irq(PCIDevice *pci_dev, int irq_num)
> > +{
> > +return irq_num;
> > +}
> > +
> > static int ioh3420_initfn(PCIDevice *d)
> > {
> > PCIBridge* br = DO_UPCAST(PCIBridge, dev, d);
> > @@ -97,6 +102,8 @@ static int ioh3420_initfn(PCIDevice *d)
> > PCIESlot *s = DO_UPCAST(PCIESlot, port, p);
> > int rc;
> >
> > +pci_bridge_map_irq(br, NULL, ioh3420_map_irq);
> > +
> > rc = pci_bridge_initfn(d);
> > if (rc < 0) {
> > return rc;
> >
>
> What became of this patch? I believe I am seeing the issue described
> when I assign a MSI-X capable virtual device to the secondary bus of an
> ioh3420, and applying this patch seems to resolve the issue. Was there
> an alternate patch proposed and if so where can I find it?
Hmm, I thought it was no longer necessary with
pci_device_route_intx_to_irq(), especially if you're assigning a VF that
doesn't support INTx. Are you using the latest qemu bits? pci-assign
or vfio-pci? Maybe provide a backtrace? Thanks,
Alex
Re: [Qemu-devel] [PATCHv3 20/20] block/raw: copy BlockLimits on raw_open
On 09/24/2013 07:35 AM, Peter Lieven wrote:
> Signed-off-by: Peter Lieven
> ---
> block/raw_bsd.c |1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> index 8dc7bba..7af26ad 100644
> --- a/block/raw_bsd.c
> +++ b/block/raw_bsd.c
> @@ -159,6 +159,7 @@ static int raw_open(BlockDriverState *bs, QDict *options,
> int flags,
> Error **errp)
> {
> bs->sg = bs->file->sg;
> +memcpy(&bs->bl, &bs->file->bl, sizeof(struct BlockLimits));
Personally, I think that sizeof(var) is more robust, because if the
declaration of var is ever changed, you don't have to remember to also
touch up the memcpy. As in:
memcpy(&bs->bl, &bs->file->bl, sizeof(bs->bl));
But there's plenty of examples of sizeof(type) in the codebase even when
a var is handy, so you are not alone, and that's not a reason for me to
request a respin.
On the other hand, why use memcpy() at all?
bs->bl = bs->file->bl;
should do the same trick, with less typing.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH] Update MAINTAINERS
All of Paul's emails are bouncing and he hasn't been active for some time. Signed-off-by: Anthony Liguori --- MAINTAINERS | 21 + 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 5c3c70c..ab8166a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -51,7 +51,6 @@ Descriptions of section entries: General Project Administration -- M: Anthony Liguori -M: Paul Brook Guest CPU cores (TCG): -- @@ -62,7 +61,6 @@ F: target-alpha/ F: hw/alpha/ ARM -M: Paul Brook M: Peter Maydell S: Maintained F: target-arm/ @@ -83,8 +81,7 @@ F: hw/lm32/ F: hw/char/lm32_* M68K -M: Paul Brook -S: Odd Fixes +S: Orphan F: target-m68k/ F: hw/m68k/ @@ -248,7 +245,6 @@ F: hw/*/imx* F: hw/arm/kzm.c Integrator CP -M: Paul Brook M: Peter Maydell S: Maintained F: hw/arm/integratorcp.c @@ -274,7 +270,6 @@ S: Maintained F: hw/arm/palm.c Real View -M: Paul Brook M: Peter Maydell S: Maintained F: hw/arm/realview* @@ -285,13 +280,11 @@ S: Maintained F: hw/arm/spitz.c Stellaris -M: Paul Brook M: Peter Maydell S: Maintained F: hw/*/stellaris* Versatile PB -M: Paul Brook M: Peter Maydell S: Maintained F: hw/*/versatile* @@ -327,18 +320,15 @@ F: hw/lm32/milkymist.c M68K Machines - an5206 -M: Paul Brook -S: Maintained +S: Orphan F: hw/m68k/an5206.c dummy_m68k -M: Paul Brook -S: Maintained +S: Orphan F: hw/m68k/dummy_m68k.c mcf5208 -M: Paul Brook -S: Maintained +S: Orphan F: hw/m68k/mcf5208.c MicroBlaze Machines @@ -567,8 +557,7 @@ F: hw/scsi/* T: git git://github.com/bonzini/qemu.git scsi-next LSI53C895A -M: Paul Brook -S: Odd Fixes +S: Orphan F: hw/scsi/lsi53c895a.c SSI -- 1.8.0
Re: [Qemu-devel] [PATCH] block/iscsi: reenable iscsi_co_get_block_status
Am 02.10.2013 13:52, schrieb Peter Lieven:
> Commit f35c934a accidently disabled iscsi_co_get_block_status for all
> libiscsi versions. Its not possible to check for enumeration constants
> in the C preprocessor. This patch changes the check to the preprocessor
> constant LIBISCSI_FEATURE_IOVECTOR which was introduced shortly after
> get_lba_status support was added to libiscsi.
>
> Signed-off-by: Peter Lieven
> ---
> block/iscsi.c |6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 6152ef1..a2a961e 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -811,7 +811,7 @@ iscsi_getlength(BlockDriverState *bs)
> return len;
> }
>
> -#if defined(SCSI_PROVISIONING_TYPE_DEALLOCATED)
> +#if defined(LIBISCSI_FEATURE_IOVECTOR)
>
> static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
>int64_t sector_num,
> @@ -903,7 +903,7 @@ out:
> return ret;
> }
>
> -#endif /* SCSI_PROVISIONING_TYPE_DEALLOCATED */
> +#endif /* LIBISCSI_FEATURE_IOVECTOR */
>
> static int
> coroutine_fn iscsi_co_discard(BlockDriverState *bs, int64_t sector_num,
> @@ -1529,7 +1529,7 @@ static BlockDriver bdrv_iscsi = {
> .bdrv_getlength = iscsi_getlength,
> .bdrv_truncate = iscsi_truncate,
>
> -#if defined(SCSI_PROVISIONING_TYPE_DEALLOCATED)
> +#if defined(LIBISCSI_FEATURE_IOVECTOR)
> .bdrv_co_get_block_status = iscsi_co_get_block_status,
> #endif
> .bdrv_co_discard = iscsi_co_discard,
Reviewed-by: Stefan Weil
Re: [Qemu-devel] [PATCHv3 18/20] qemu-img: add support for fully allocated images
On 09/24/2013 07:35 AM, Peter Lieven wrote: > Signed-off-by: Peter Lieven > --- > qemu-img.c |8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) Missing a counterpart change to qemu-img.texi. > > diff --git a/qemu-img.c b/qemu-img.c > index 926f0a0..c6eff15 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -100,8 +100,10 @@ static void help(void) > " '-h' with or without a command shows this help and lists the > supported formats\n" > " '-p' show progress of command (only certain commands)\n" > " '-q' use Quiet mode - do not print any output (except > errors)\n" > - " '-S' indicates the consecutive number of bytes that must > contain only zeros\n" > - " for qemu-img to create a sparse image during conversion\n" > + " '-S' indicates the consecutive number of bytes (defaults to > 4k) that must\n" > + " contain only zeros for qemu-img to create a sparse image > during\n" > + " conversion. if the number of bytes is 0 sparse files are > disabled and\n" > + " images will always be fully allocated\n" the texi page should have similar wording. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCHv3 13/20] block: introduce bdrv_zeroize
On 09/24/2013 07:35 AM, Peter Lieven wrote: > this patch adds a call to completely zero out a block device. > the operation is sped up by checking the block status and > only writing zeroes to the device if they currently do not > return zeroes. optionally the zero writing can be sped up > by setting the flag BDRV_REQ_MAY_UNMAP to emulate the zero > write by unmapping if the driver supports it. > > Signed-off-by: Peter Lieven > --- > block.c | 37 + > include/block/block.h |1 + > 2 files changed, 38 insertions(+) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCHv3 11/20] iscsi: add bdrv_has_discard_zeroes and bdrv_has_discard_write_zeroes
On 09/24/2013 07:35 AM, Peter Lieven wrote: > Signed-off-by: Peter Lieven > --- > block/iscsi.c | 16 +++- > 1 file changed, 15 insertions(+), 1 deletion(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCHv3 10/20] iscsi: set limits in BlockDriverState
On 09/24/2013 07:35 AM, Peter Lieven wrote: > Signed-off-by: Peter Lieven > --- > block/iscsi.c | 14 ++ > 1 file changed, 14 insertions(+) Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCHv3 08/20] block: honour BlockLimits in bdrv_co_discard
On 09/24/2013 07:35 AM, Peter Lieven wrote: > Signed-off-by: Peter Lieven > --- > block.c | 37 - > 1 file changed, 36 insertions(+), 1 deletion(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCHv3 07/20] block: honour BlockLimits in bdrv_co_do_write_zeroes
On 09/24/2013 07:35 AM, Peter Lieven wrote:
> Signed-off-by: Peter Lieven
> ---
> block.c | 65
> +++
> 1 file changed, 49 insertions(+), 16 deletions(-)
>
> diff --git a/block.c b/block.c
> index ac35cb5..580570a 100644
> --- a/block.c
> +++ b/block.c
> @@ -2710,32 +2710,65 @@ int coroutine_fn
> bdrv_co_copy_on_readv(BlockDriverState *bs,
> BDRV_REQ_COPY_ON_READ);
> }
>
> +
> +if (ret == -ENOTSUP) {
> +/* Fall back to bounce buffer if write zeroes is unsupported */
> +iov.iov_len = num * BDRV_SECTOR_SIZE;
Why 2 spaces?
But that's trivial.
Reviewed-by: Eric Blake
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-trivial] [PATCH] hw/9pfs: Fix errno value for xattr functions
01.10.2013 15:28, Daniel P. Berrange пишет: From: "Daniel P. Berrange" If there is no operation driver for the xattr type the functions return '-1' and set errno to '-EOPNOTSUPP'. When the calling code sets 'ret = -errno' this turns into a large positive number. In Linux 3.11, the kernel has switched to using 9p version 9p2000.L, instead of 9p2000.u, which enables support for xattr operations. This on its own is harmless, but for another change which makes it request the xattr with a name 'security.capability'. The result is that the guest sees a succesful return of 95 bytes of data, instead of a failure with errno set to 95. Since the kernel expects a maximum of 20 bytes for an xattr return this gets translated to the unexpected errno ERANGE. This all means that when running a binary off a 9p fs in 3.11 kernels you get a fun result of: # ./date sh: ./date: Numerical result out of range The only workaround is to pass 'version=9p2000.u' when mounting the 9p fs in the guest, to disable all use of xattrs. Thanks, applied to the trivial patches queue. /mjt
Re: [Qemu-devel] [Qemu-trivial] [PATCH] vl: Clean up unnecessary boot_order complications
01.10.2013 15:47, arm...@redhat.com пишет: From: Markus Armbruster Messed up in commit 8281abd. Thanks, applied to the trivial patches queue. /mjt
Re: [Qemu-devel] [Qemu-trivial] [PATCH] qemu-char: Fix potential out of bounds access to local arrays
01.10.2013 01:04, Stefan Weil wrote: Latest gcc-4.8 supports a new option -fsanitize=address which activates an AddressSanitizer. This AddressSanitizer stops the QEMU system emulation very early because two character arrays of size 8 are potentially written with 9 bytes. Commit 6ea314d91439741e95772dfbab98b4135e04bebb added the code. There is no obvious reason why width or height could need 8 characters, so reduce it to 7 characters which together with the terminating '\0' fit into the arrays. A good one. Thanks, applied to the trivial patches queue. /mjt
Re: [Qemu-devel] [PATCH v3 uq/master 2/2] x86: cpuid: reconstruct leaf 0Dh data
On Wed, Oct 02, 2013 at 05:54:57PM +0200, Paolo Bonzini wrote:
> The data in leaf 0Dh depends on information from other feature bits.
> Instead of passing it blindly from the host, compute it based on
> whether these feature bits are enabled.
>
Applied both. Thanks.
> Signed-off-by: Paolo Bonzini
> ---
> target-i386/cpu.c | 65
> ---
> 1 file changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index ac83106..1addb18 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -328,6 +328,15 @@ X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
> };
> #undef REGISTER
>
> +typedef struct ExtSaveArea {
> +uint32_t feature, bits;
> +uint32_t offset, size;
> +} ExtSaveArea;
> +
> +static const ExtSaveArea ext_save_areas[] = {
> +[2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
> +.offset = 0x100, .size = 0x240 },
> +};
>
> const char *get_register_name_32(unsigned int reg)
> {
> @@ -2169,29 +2178,51 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
> *edx = 0;
> }
> break;
> -case 0xD:
> +case 0xD: {
> +KVMState *s = cs->kvm_state;
> +uint64_t kvm_mask;
> +int i;
> +
> /* Processor Extended State */
> -if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
> -*eax = 0;
> -*ebx = 0;
> -*ecx = 0;
> -*edx = 0;
> +*eax = 0;
> +*ebx = 0;
> +*ecx = 0;
> +*edx = 0;
> +if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) ||
> !kvm_enabled()) {
> break;
> }
> -if (kvm_enabled()) {
> -KVMState *s = cs->kvm_state;
> +kvm_mask =
> +kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) |
> +((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) << 32);
>
> -*eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX);
> -*ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX);
> -*ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX);
> -*edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX);
> -} else {
> -*eax = 0;
> -*ebx = 0;
> -*ecx = 0;
> -*edx = 0;
> +if (count == 0) {
> +*ecx = 0x240;
> +for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) {
> +const ExtSaveArea *esa = &ext_save_areas[i];
> +if ((env->features[esa->feature] & esa->bits) == esa->bits &&
> +(kvm_mask & (1 << i)) != 0) {
> +if (i < 32) {
> +*eax |= 1 << i;
> +} else {
> +*edx |= 1 << (i - 32);
> +}
> +*ecx = MAX(*ecx, esa->offset + esa->size);
> +}
> +}
> +*eax |= kvm_mask & (XSTATE_FP | XSTATE_SSE);
> +*ebx = *ecx;
> +} else if (count == 1) {
> +*eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX);
> +} else if (count < ARRAY_SIZE(ext_save_areas)) {
> +const ExtSaveArea *esa = &ext_save_areas[count];
> +if ((env->features[esa->feature] & esa->bits) == esa->bits &&
> +(kvm_mask & (1 << count)) != 0) {
> +*eax = esa->offset;
> +*ebx = esa->size;
> +}
> }
> break;
> +}
> case 0x8000:
> *eax = env->cpuid_xlevel;
> *ebx = env->cpuid_vendor1;
> --
> 1.8.3.1
--
Gleb.
Re: [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin
On Wed, 2013-10-02 at 16:05 +0300, Marcel Apfelbaum wrote: > On Wed, 2013-10-02 at 15:58 +0300, Michael S. Tsirkin wrote: > > On Wed, Oct 02, 2013 at 03:41:25PM +0300, Marcel Apfelbaum wrote: > > > Note: Added RFC because not all affected devices were > > > checked yet. > > > > What do you have in mind exactly? > Sanity test for *all* modified devices. > Any other thoughts? > > From self code review, I can say that besides 2 devices > (vmxnet3 and vfio), the code is re-factored with > no side effects. > > vmxnet3: I reviewed linux driver code and when using legacy interrupts > only INTx 0 is used. > vfio: Also here only INTx 0 is used, so the interface selecting > the interrupt line seems to be unnecessary at least for > legacy interrupts. vfio uses the same pin as the physical device, so you can't guarantee that's always INTA. Thanks, Alex > > > > > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN > > > register during device initialization. Devices should not call > > > directly qemu_set_irq and specify the INTx pin. > > > > > > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise, > > > qemu_irq_lower and qemu_irq_pulse, setting the irq > > > based on PCI_INTERRUPT_PIN. > > > > > > Added interface to allocate and free single irq. > > > Added pci_allocate_irq wrapper to be used by devices that > > > still need PCIDevice infrastructure to assert irqs. > > > > > > Removed irq field from PCIDevice, not needed anymore. > > > > > > Special cases of replacements were done in separate patches, > > > all others in one patch "hw: set interrupts using pci irq wrappers" > > > > Looks good to me overall. > > Acked-by: Michael S. Tsirkin > > > > > > > Changes from v1: > > > - Addressed Michael S. Tsirkin's comments: > > >- pci_set_irq directly calls pci_irq handler > > >- removed irq field from PCIDevice > > > - Added qemu interface to allocate single irq > > > - Added pci wrappers to allocate and free pci irq > > > - Added pci irq wrappers for all qemu methods > > >setting irq and not only qemu_set_irq > > > - Replace all qemu irq setters with pci > > >wrappers > > > > > > Marcel Apfelbaum (9): > > > hw/core: Add interface to allocate and free a single IRQ > > > hw/pci: add pci wrappers for allocating and asserting irqs > > > hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init > > > hw/vmxnet3: set interrupts using pci irq wrappers > > > hw/vfio: set interrupts using pci irq wrappers > > > hw/xhci: set interrupts using pci irq wrappers > > > hw: set interrupts using pci irq wrappers > > > hw/pcie: AER and hot-plug events must use device's interrupt > > > hw/pci: removed irq field from PCIDevice > > > > > > hw/audio/ac97.c| 4 ++-- > > > hw/audio/es1370.c | 4 ++-- > > > hw/audio/intel-hda.c | 2 +- > > > hw/block/nvme.c| 2 +- > > > hw/char/serial-pci.c | 5 +++-- > > > hw/char/tpci200.c | 8 > > > hw/core/irq.c | 16 > > > hw/display/qxl.c | 2 +- > > > hw/ide/cmd646.c| 2 +- > > > hw/ide/ich.c | 3 ++- > > > hw/isa/vt82c686.c | 2 +- > > > hw/misc/ivshmem.c | 2 +- > > > hw/misc/vfio.c | 11 ++- > > > hw/net/e1000.c | 2 +- > > > hw/net/eepro100.c | 4 ++-- > > > hw/net/ne2000.c| 3 ++- > > > hw/net/pcnet-pci.c | 3 ++- > > > hw/net/rtl8139.c | 2 +- > > > hw/net/vmxnet3.c | 13 +++-- > > > hw/pci-bridge/pci_bridge_dev.c | 2 +- > > > hw/pci/pci.c | 26 +- > > > hw/pci/pcie.c | 4 ++-- > > > hw/pci/pcie_aer.c | 4 ++-- > > > hw/pci/shpc.c | 2 +- > > > hw/scsi/esp-pci.c | 3 ++- > > > hw/scsi/lsi53c895a.c | 2 +- > > > hw/scsi/megasas.c | 6 +++--- > > > hw/scsi/vmw_pvscsi.c | 2 +- > > > hw/usb/hcd-ehci-pci.c | 2 +- > > > hw/usb/hcd-ohci.c | 2 +- > > > hw/usb/hcd-uhci.c | 2 +- > > > hw/usb/hcd-xhci.c | 7 ++- > > > hw/virtio/virtio-pci.c | 4 ++-- > > > include/hw/irq.h | 7 +++ > > > include/hw/pci/pci.h | 22 +++--- > > > include/hw/pci/pcie.h | 18 -- > > > 36 files changed, 127 insertions(+), 78 deletions(-) > > > > > > -- > > > 1.8.3.1 > > >
Re: [Qemu-devel] [PATCHv4] block/get_block_status: avoid redundant callouts on raw devices
Am 02.10.2013 17:13, schrieb Paolo Bonzini:
> Il 02/10/2013 17:06, Stefan Hajnoczi ha scritto:
>> Sorry I didn't review this earlier but this flag looks hacky and I'm not
>> confident about merging the patch yet.
>>
>> The patch makes me wonder if the raw_bsd driver should avoid calling
>> bs->file itself:
>>
>> return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID |
>>(sector_num << BDRV_SECTOR_BITS);
>>
>> Let block.c:bdrv_co_get_block_status() call down into bs->file.
>>
>> The problem is then the protocol cannot report unallocated sectors with
>> this approach.
>>
>> I think we want to preserve bs' offset while taking the other flags from
>> bs->file (DATA, ZERO).
> This would cause other changes. For example, a qcow2 with full metadata
> preallocation (i.e. all L2 tables are there but it points to holes)
> would not return DATA anymore. I think this is wrong, and especially a
> change from the old is_allocated API.
>
> However, a variant on this idea could be to return
>
>BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
> (sector_num << BDRV_SECTOR_BITS);
>
> and then BDRV_BLOCK_RAW would mean "take DATA and ZERO from bs->file".
Like this?
diff --git a/block.c b/block.c
index 93e113a..71fab1f 100644
--- a/block.c
+++ b/block.c
@@ -3146,6 +3146,10 @@ static int64_t coroutine_fn
bdrv_co_get_block_status(BlockDriverState *bs,
*pnum = 0;
return ret;
}
+
+if (ret & BDRV_BLOCK_RAW) {
+return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum);
+}
if (!(ret & BDRV_BLOCK_DATA)) {
if (bdrv_has_zero_init(bs)) {
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index d4ace60..308d605 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -62,7 +62,8 @@ static int64_t coroutine_fn
raw_co_get_block_status(BlockDriverState *bs,
int64_t sector_num,
int nb_sectors, int *pnum)
{
-return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum);
+return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
+ (sector_num << BDRV_SECTOR_BITS);
}
Re: [Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: set interrupts using pci irq wrappers
On Wed, 2013-10-02 at 15:41 +0300, Marcel Apfelbaum wrote:
> pci_set_irq and the other pci irq wrappers use
> PCI_INTERRUPT_PIN config register to compute device
> INTx pin to assert/deassert.
>
> Save INTx pin into the config register before calling
> pci_set_irq
>
> Signed-off-by: Marcel Apfelbaum
> ---
> hw/misc/vfio.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
Seems ok, but why not take advantage of the pci_irq_raise/lower()
wrappers? BTW, with PCI being active low, should those be
assert/deassert to avoid confusion confusion with the actual signal
level? Thanks,
Alex
>
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index a1c08fb..3d7297c 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque)
> 'A' + vdev->intx.pin);
>
> vdev->intx.pending = true;
> -qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1);
> +pci_set_irq(&vdev->pdev, 1);
> vfio_mmap_set_enabled(vdev, false);
> if (vdev->intx.mmap_timeout) {
> timer_mod(vdev->intx.mmap_timer,
> @@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev)
> vdev->host.bus, vdev->host.slot, vdev->host.function);
>
> vdev->intx.pending = false;
> -qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> +pci_set_irq(&vdev->pdev, 0);
> vfio_unmask_intx(vdev);
> }
>
> @@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev)
> qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
> vfio_mask_intx(vdev);
> vdev->intx.pending = false;
> -qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> +pci_set_irq(&vdev->pdev, 0);
>
> /* Get an eventfd for resample/unmask */
> if (event_notifier_init(&vdev->intx.unmask, 0)) {
> @@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev)
> */
> vfio_mask_intx(vdev);
> vdev->intx.pending = false;
> -qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> +pci_set_irq(&vdev->pdev, 0);
>
> /* Tell KVM to stop listening for an INTx irqfd */
> if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) {
> @@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev)
> vfio_disable_interrupts(vdev);
>
> vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
> +pci_config_set_interrupt_pin(vdev->pdev.config, pin);
>
> #ifdef CONFIG_KVM
> /*
> @@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev)
> vfio_disable_intx_kvm(vdev);
> vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX);
> vdev->intx.pending = false;
> -qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
> +pci_set_irq(&vdev->pdev, 0);
> vfio_mmap_set_enabled(vdev, true);
>
> fd = event_notifier_get_fd(&vdev->intx.interrupt);
Re: [Qemu-devel] [PATCH] block/iscsi: introduce bdrv_co_{readv, writev, flush_to_disk}
Am 02.10.2013 17:55, schrieb Paolo Bonzini: > Il 02/10/2013 17:54, Peter Lieven ha scritto: I'm not sure it's already the time for this... Cancellation sucks in QEMU, and this is going to make things even worse. >> Ok, I was not aware. I talked with Kevin about this some months ago >> and promised (list CCed) to prepare this for 1.7.0. Nobody objected ;-) >> >> But, anyway the patch is there as a basis as soon as the time has come. > Yes, I'll stash it in my all-things-SCSI branch. :) It was nice to see that the code collapsed to 1/3, btw. Peter
Re: [Qemu-devel] [PATCH] block/iscsi: introduce bdrv_co_{readv, writev, flush_to_disk}
Il 02/10/2013 17:54, Peter Lieven ha scritto: >> > I'm not sure it's already the time for this... Cancellation sucks in >> > QEMU, and this is going to make things even worse. > Ok, I was not aware. I talked with Kevin about this some months ago > and promised (list CCed) to prepare this for 1.7.0. Nobody objected ;-) > > But, anyway the patch is there as a basis as soon as the time has come. Yes, I'll stash it in my all-things-SCSI branch. :) Paolo
[Qemu-devel] [PATCH v3 uq/master 2/2] x86: cpuid: reconstruct leaf 0Dh data
The data in leaf 0Dh depends on information from other feature bits.
Instead of passing it blindly from the host, compute it based on
whether these feature bits are enabled.
Signed-off-by: Paolo Bonzini
---
target-i386/cpu.c | 65 ---
1 file changed, 48 insertions(+), 17 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ac83106..1addb18 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -328,6 +328,15 @@ X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
};
#undef REGISTER
+typedef struct ExtSaveArea {
+uint32_t feature, bits;
+uint32_t offset, size;
+} ExtSaveArea;
+
+static const ExtSaveArea ext_save_areas[] = {
+[2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
+.offset = 0x100, .size = 0x240 },
+};
const char *get_register_name_32(unsigned int reg)
{
@@ -2169,29 +2178,51 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
*edx = 0;
}
break;
-case 0xD:
+case 0xD: {
+KVMState *s = cs->kvm_state;
+uint64_t kvm_mask;
+int i;
+
/* Processor Extended State */
-if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
-*eax = 0;
-*ebx = 0;
-*ecx = 0;
-*edx = 0;
+*eax = 0;
+*ebx = 0;
+*ecx = 0;
+*edx = 0;
+if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) || !kvm_enabled()) {
break;
}
-if (kvm_enabled()) {
-KVMState *s = cs->kvm_state;
+kvm_mask =
+kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) |
+((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) << 32);
-*eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX);
-*ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX);
-*ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX);
-*edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX);
-} else {
-*eax = 0;
-*ebx = 0;
-*ecx = 0;
-*edx = 0;
+if (count == 0) {
+*ecx = 0x240;
+for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) {
+const ExtSaveArea *esa = &ext_save_areas[i];
+if ((env->features[esa->feature] & esa->bits) == esa->bits &&
+(kvm_mask & (1 << i)) != 0) {
+if (i < 32) {
+*eax |= 1 << i;
+} else {
+*edx |= 1 << (i - 32);
+}
+*ecx = MAX(*ecx, esa->offset + esa->size);
+}
+}
+*eax |= kvm_mask & (XSTATE_FP | XSTATE_SSE);
+*ebx = *ecx;
+} else if (count == 1) {
+*eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX);
+} else if (count < ARRAY_SIZE(ext_save_areas)) {
+const ExtSaveArea *esa = &ext_save_areas[count];
+if ((env->features[esa->feature] & esa->bits) == esa->bits &&
+(kvm_mask & (1 << count)) != 0) {
+*eax = esa->offset;
+*ebx = esa->size;
+}
}
break;
+}
case 0x8000:
*eax = env->cpuid_xlevel;
*ebx = env->cpuid_vendor1;
--
1.8.3.1
Re: [Qemu-devel] [PATCH] block/iscsi: introduce bdrv_co_{readv, writev, flush_to_disk}
Am 02.10.2013 17:46, schrieb Paolo Bonzini: > Il 02/10/2013 17:41, Peter Lieven ha scritto: >> this converts read, write and flush functions from aio to coroutines. > I'm not sure it's already the time for this... Cancellation sucks in > QEMU, and this is going to make things even worse. Ok, I was not aware. I talked with Kevin about this some months ago and promised (list CCed) to prepare this for 1.7.0. Nobody objected ;-) But, anyway the patch is there as a basis as soon as the time has come. Peter
Re: [Qemu-devel] [PATCHv3 06/20] block: add BlockLimits structure to BlockDriverState
On 09/24/2013 07:35 AM, Peter Lieven wrote:
> this patch adds BlockLimits which introduces discard and write_zeroes
> limits and alignment information to the BlockDriverState.
>
> Signed-off-by: Peter Lieven
> ---
> include/block/block_int.h | 17 +
> 1 file changed, 17 insertions(+)
>
> +struct BlockLimits {
Should this be a typedef? Either in include/qemu/typedefs.h (where
BlockDriverState is listed), or locally (see BdrvTrackedRequest in the
same file for an example)?
> @@ -280,6 +294,9 @@ struct BlockDriverState {
> uint64_t total_time_ns[BDRV_MAX_IOTYPE];
> uint64_t wr_highest_sector;
>
> +/* I/O Limits */
> +struct BlockLimits bl;
> +
All other struct/pointer-to-struct members in BlockDriverState are
listed by typedef name, rather than calling out 'struct foo'.
My question is one of style/consistency, not of C correctness; so unless
a maintainer actually agrees that a typedef change is needed so that you
comply with project coding standards, feel free to add:
Reviewed-by: Eric Blake
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] block/iscsi: introduce bdrv_co_{readv, writev, flush_to_disk}
Il 02/10/2013 17:41, Peter Lieven ha scritto:
> this converts read, write and flush functions from aio to coroutines.
I'm not sure it's already the time for this... Cancellation sucks in
QEMU, and this is going to make things even worse.
Paolo
> Signed-off-by: Peter Lieven
> ---
> block/iscsi.c | 405
> +++--
> 1 file changed, 107 insertions(+), 298 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 5c7baee..f37e3c5 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -237,44 +237,6 @@ iscsi_process_write(void *arg)
> iscsi_set_events(iscsilun);
> }
>
> -static int
> -iscsi_aio_writev_acb(IscsiAIOCB *acb);
> -
> -static void
> -iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
> - void *command_data, void *opaque)
> -{
> -IscsiAIOCB *acb = opaque;
> -
> -trace_iscsi_aio_write16_cb(iscsi, status, acb, acb->canceled);
> -
> -g_free(acb->buf);
> -acb->buf = NULL;
> -
> -if (acb->canceled != 0) {
> -return;
> -}
> -
> -acb->status = 0;
> -if (status != 0) {
> -if (status == SCSI_STATUS_CHECK_CONDITION
> -&& acb->task->sense.key == SCSI_SENSE_UNIT_ATTENTION
> -&& acb->retries-- > 0) {
> -scsi_free_scsi_task(acb->task);
> -acb->task = NULL;
> -if (iscsi_aio_writev_acb(acb) == 0) {
> -iscsi_set_events(acb->iscsilun);
> -return;
> -}
> -}
> -error_report("Failed to write16 data to iSCSI lun. %s",
> - iscsi_get_error(iscsi));
> -acb->status = -EIO;
> -}
> -
> -iscsi_schedule_bh(acb);
> -}
> -
> static int64_t sector_lun2qemu(int64_t sector, IscsiLun *iscsilun)
> {
> return sector * iscsilun->block_size / BDRV_SECTOR_SIZE;
> @@ -299,324 +261,172 @@ static bool is_request_lun_aligned(int64_t
> sector_num, int nb_sectors,
> return 1;
> }
>
> -static int
> -iscsi_aio_writev_acb(IscsiAIOCB *acb)
> +static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
> +int64_t sector_num, int nb_sectors,
> +QEMUIOVector *iov)
> {
> -struct iscsi_context *iscsi = acb->iscsilun->iscsi;
> -size_t size;
> -uint32_t num_sectors;
> +IscsiLun *iscsilun = bs->opaque;
> +struct IscsiTask iTask;
> uint64_t lba;
> -#if !defined(LIBISCSI_FEATURE_IOVECTOR)
> -struct iscsi_data data;
> -#endif
> -int ret;
> -
> -acb->canceled = 0;
> -acb->bh = NULL;
> -acb->status = -EINPROGRESS;
> -acb->buf= NULL;
> +uint32_t num_sectors;
> +uint8_t *data = NULL;
> +uint8_t *buf = NULL;
>
> -/* this will allow us to get rid of 'buf' completely */
> -size = acb->nb_sectors * BDRV_SECTOR_SIZE;
> +if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
> +return -EINVAL;
> +}
>
> +lba = sector_qemu2lun(sector_num, iscsilun);
> +num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
> #if !defined(LIBISCSI_FEATURE_IOVECTOR)
> -data.size = MIN(size, acb->qiov->size);
> -
> /* if the iovec only contains one buffer we can pass it directly */
> -if (acb->qiov->niov == 1) {
> -data.data = acb->qiov->iov[0].iov_base;
> +if (iov->niov == 1) {
> +data = iov->iov[0].iov_base;
> } else {
> -acb->buf = g_malloc(data.size);
> -qemu_iovec_to_buf(acb->qiov, 0, acb->buf, data.size);
> -data.data = acb->buf;
> +size_t size = MIN(nb_sectors * BDRV_SECTOR_SIZE, iov->size);
> +buf = g_malloc(size);
> +qemu_iovec_to_buf(iov, 0, buf, size);
> +data = buf;
> }
> #endif
> -
> -acb->task = malloc(sizeof(struct scsi_task));
> -if (acb->task == NULL) {
> -error_report("iSCSI: Failed to allocate task for scsi WRITE16 "
> - "command. %s", iscsi_get_error(iscsi));
> -return -1;
> +iscsi_co_init_iscsitask(iscsilun, &iTask);
> +retry:
> +iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba,
> +data, num_sectors * iscsilun->block_size,
> +iscsilun->block_size, 0, 0, 0, 0, 0,
> +iscsi_co_generic_cb, &iTask);
> +if (iTask.task == NULL) {
> +g_free(buf);
> +return -EIO;
> }
> -memset(acb->task, 0, sizeof(struct scsi_task));
> -
> -acb->task->xfer_dir = SCSI_XFER_WRITE;
> -acb->task->cdb_size = 16;
> -acb->task->cdb[0] = 0x8a;
> -lba = sector_qemu2lun(acb->sector_num, acb->iscsilun);
> -*(uint32_t *)&acb->task->cdb[2] = htonl(lba >> 32);
> -*(uint32_t *)&acb->task->cdb[6] = htonl(lba & 0x);
> -num_sectors = sector_qemu2lun(acb->nb_sectors, acb->iscsilun);
> -*(uint32_t *)&acb->task->cdb[10] = htonl(num_sectors);
> -
Re: [Qemu-devel] [PATCHv3 05/20] block/raw: add bdrv_has_discard_zeroes and bdrv_has_discard_write_zeroes
On 09/24/2013 07:34 AM, Peter Lieven wrote: > Signed-off-by: Peter Lieven > --- > block/raw_bsd.c | 56 > +-- > 1 file changed, 34 insertions(+), 22 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH V4] block: Add BlockDriver.bdrv_check_ext_snapshot.
On 10/02/2013 09:33 AM, Max Reitz wrote:
> On 2013-10-02 14:33, Benoît Canet wrote:
>> This field is used by blkverify to disable external snapshots creation.
>> I will also be used by block filters like quorum to disable external
>> snapshots
>> creation.
> Oh, I nearly missed it: s/I will also be/It will also be/ and probably
> s/external snapshots creation/external snapshot creation/, but I'm not
> too sure about the last one.
As a native speaker, yes, that last change sounds better ("to disable
external snapshot creation").
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCHv3 04/20] block: introduce bdrv_has_discard_zeroes and bdrv_has_discard_write_zeroes
On 09/24/2013 07:34 AM, Peter Lieven wrote: > Signed-off-by: Peter Lieven > --- > block.c | 29 + > include/block/block.h |2 ++ > include/block/block_int.h | 13 + > 3 files changed, 44 insertions(+) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] block/iscsi: introduce bdrv_co_{readv, writev, flush_to_disk}
this converts read, write and flush functions from aio to coroutines.
Signed-off-by: Peter Lieven
---
block/iscsi.c | 405 +++--
1 file changed, 107 insertions(+), 298 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 5c7baee..f37e3c5 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -237,44 +237,6 @@ iscsi_process_write(void *arg)
iscsi_set_events(iscsilun);
}
-static int
-iscsi_aio_writev_acb(IscsiAIOCB *acb);
-
-static void
-iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
- void *command_data, void *opaque)
-{
-IscsiAIOCB *acb = opaque;
-
-trace_iscsi_aio_write16_cb(iscsi, status, acb, acb->canceled);
-
-g_free(acb->buf);
-acb->buf = NULL;
-
-if (acb->canceled != 0) {
-return;
-}
-
-acb->status = 0;
-if (status != 0) {
-if (status == SCSI_STATUS_CHECK_CONDITION
-&& acb->task->sense.key == SCSI_SENSE_UNIT_ATTENTION
-&& acb->retries-- > 0) {
-scsi_free_scsi_task(acb->task);
-acb->task = NULL;
-if (iscsi_aio_writev_acb(acb) == 0) {
-iscsi_set_events(acb->iscsilun);
-return;
-}
-}
-error_report("Failed to write16 data to iSCSI lun. %s",
- iscsi_get_error(iscsi));
-acb->status = -EIO;
-}
-
-iscsi_schedule_bh(acb);
-}
-
static int64_t sector_lun2qemu(int64_t sector, IscsiLun *iscsilun)
{
return sector * iscsilun->block_size / BDRV_SECTOR_SIZE;
@@ -299,324 +261,172 @@ static bool is_request_lun_aligned(int64_t sector_num,
int nb_sectors,
return 1;
}
-static int
-iscsi_aio_writev_acb(IscsiAIOCB *acb)
+static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
+int64_t sector_num, int nb_sectors,
+QEMUIOVector *iov)
{
-struct iscsi_context *iscsi = acb->iscsilun->iscsi;
-size_t size;
-uint32_t num_sectors;
+IscsiLun *iscsilun = bs->opaque;
+struct IscsiTask iTask;
uint64_t lba;
-#if !defined(LIBISCSI_FEATURE_IOVECTOR)
-struct iscsi_data data;
-#endif
-int ret;
-
-acb->canceled = 0;
-acb->bh = NULL;
-acb->status = -EINPROGRESS;
-acb->buf= NULL;
+uint32_t num_sectors;
+uint8_t *data = NULL;
+uint8_t *buf = NULL;
-/* this will allow us to get rid of 'buf' completely */
-size = acb->nb_sectors * BDRV_SECTOR_SIZE;
+if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
+return -EINVAL;
+}
+lba = sector_qemu2lun(sector_num, iscsilun);
+num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
#if !defined(LIBISCSI_FEATURE_IOVECTOR)
-data.size = MIN(size, acb->qiov->size);
-
/* if the iovec only contains one buffer we can pass it directly */
-if (acb->qiov->niov == 1) {
-data.data = acb->qiov->iov[0].iov_base;
+if (iov->niov == 1) {
+data = iov->iov[0].iov_base;
} else {
-acb->buf = g_malloc(data.size);
-qemu_iovec_to_buf(acb->qiov, 0, acb->buf, data.size);
-data.data = acb->buf;
+size_t size = MIN(nb_sectors * BDRV_SECTOR_SIZE, iov->size);
+buf = g_malloc(size);
+qemu_iovec_to_buf(iov, 0, buf, size);
+data = buf;
}
#endif
-
-acb->task = malloc(sizeof(struct scsi_task));
-if (acb->task == NULL) {
-error_report("iSCSI: Failed to allocate task for scsi WRITE16 "
- "command. %s", iscsi_get_error(iscsi));
-return -1;
+iscsi_co_init_iscsitask(iscsilun, &iTask);
+retry:
+iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba,
+data, num_sectors * iscsilun->block_size,
+iscsilun->block_size, 0, 0, 0, 0, 0,
+iscsi_co_generic_cb, &iTask);
+if (iTask.task == NULL) {
+g_free(buf);
+return -EIO;
}
-memset(acb->task, 0, sizeof(struct scsi_task));
-
-acb->task->xfer_dir = SCSI_XFER_WRITE;
-acb->task->cdb_size = 16;
-acb->task->cdb[0] = 0x8a;
-lba = sector_qemu2lun(acb->sector_num, acb->iscsilun);
-*(uint32_t *)&acb->task->cdb[2] = htonl(lba >> 32);
-*(uint32_t *)&acb->task->cdb[6] = htonl(lba & 0x);
-num_sectors = sector_qemu2lun(acb->nb_sectors, acb->iscsilun);
-*(uint32_t *)&acb->task->cdb[10] = htonl(num_sectors);
-acb->task->expxferlen = size;
-
#if defined(LIBISCSI_FEATURE_IOVECTOR)
-ret = iscsi_scsi_command_async(iscsi, acb->iscsilun->lun, acb->task,
- iscsi_aio_write16_cb,
- NULL,
- acb);
-#else
-ret = iscsi_scsi_command_async(iscsi, acb->iscsilun->lun, acb->task,
- iscsi_aio_write16_cb,
-
Re: [Qemu-devel] [PATCH v2 uq/master 2/2] x86: cpuid: reconstruct leaf 0Dh data
On Wed, Oct 02, 2013 at 05:37:31PM +0200, Paolo Bonzini wrote:
> Il 02/10/2013 17:21, Gleb Natapov ha scritto:
> >> -if (kvm_enabled()) {
> >> -KVMState *s = cs->kvm_state;
> >> +kvm_mask =
> >> +kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) |
> >> +((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) <<
> >> 32);
> >>
> >> -*eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX);
> >> -*ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX);
> >> -*ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX);
> >> -*edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX);
> >> -} else {
> >> -*eax = 0;
> >> -*ebx = 0;
> >> -*ecx = 0;
> >> -*edx = 0;
> >> +if (count == 0) {
> >> +*ecx = 0x240;
> >> +for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) {
> >> +const ExtSaveArea *esa = &ext_save_areas[i];
> >> +if ((env->features[esa->feature] & esa->bits) ==
> >> esa->bits &&
> >> +(kvm_mask & (1 << i)) != 0) {
> >> +if (i < 32) {
> >> +*eax |= 1 << i;
> >> +} else {
> >> +*edx |= 1 << (i - 32);
> >> +}
> >> +*ecx = MAX(*ecx, esa->offset + esa->size);
> >> +}
> >> +}
> >> +*eax |= kvm_mask & 3;
> > Lets use define from previous patch.
>
> Right.
>
> >> +*ebx = *ecx;
> >> +} else if (count == 1) {
> >> +*eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX);
> >> +} else if (count < ARRAY_SIZE(ext_save_areas)) {
> >> +const ExtSaveArea *esa = &ext_save_areas[count];
> >> +if ((env->features[esa->feature] & esa->bits) == esa->bits &&
> >> +(kvm_mask & (1 << count)) != 0) {
> >> +*eax = esa->offset;
> >> +*ebx = esa->size;
> > Why do you hard code them instead of querying kernel? What if they
> > depend on cpu type? (well if this happens we can forget about
> > migration, but still...)
>
> HPA confirmed (on xen-devel) that they will not depend on the CPU type.
> All offsets are documented in the SDM and in the additional Skylake
> manual except for MPX, and he reported that he'd ask for MPX to be
> documented as well. As you said, if they changed it would be a total mess.
>
> I hardcoded them because this is not KVM-specific knowledge. TCG could
> in principle reuse the same code, just skipping the part where it masks
> away features not supported by KVM.
>
OK. Can you send new version with defines please?
--
Gleb.
Re: [Qemu-devel] [PATCH v2 uq/master 2/2] x86: cpuid: reconstruct leaf 0Dh data
Il 02/10/2013 17:21, Gleb Natapov ha scritto:
>> -if (kvm_enabled()) {
>> -KVMState *s = cs->kvm_state;
>> +kvm_mask =
>> +kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) |
>> +((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) <<
>> 32);
>>
>> -*eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX);
>> -*ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX);
>> -*ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX);
>> -*edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX);
>> -} else {
>> -*eax = 0;
>> -*ebx = 0;
>> -*ecx = 0;
>> -*edx = 0;
>> +if (count == 0) {
>> +*ecx = 0x240;
>> +for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) {
>> +const ExtSaveArea *esa = &ext_save_areas[i];
>> +if ((env->features[esa->feature] & esa->bits) == esa->bits
>> &&
>> +(kvm_mask & (1 << i)) != 0) {
>> +if (i < 32) {
>> +*eax |= 1 << i;
>> +} else {
>> +*edx |= 1 << (i - 32);
>> +}
>> +*ecx = MAX(*ecx, esa->offset + esa->size);
>> +}
>> +}
>> +*eax |= kvm_mask & 3;
> Lets use define from previous patch.
Right.
>> +*ebx = *ecx;
>> +} else if (count == 1) {
>> +*eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX);
>> +} else if (count < ARRAY_SIZE(ext_save_areas)) {
>> +const ExtSaveArea *esa = &ext_save_areas[count];
>> +if ((env->features[esa->feature] & esa->bits) == esa->bits &&
>> +(kvm_mask & (1 << count)) != 0) {
>> +*eax = esa->offset;
>> +*ebx = esa->size;
> Why do you hard code them instead of querying kernel? What if they
> depend on cpu type? (well if this happens we can forget about
> migration, but still...)
HPA confirmed (on xen-devel) that they will not depend on the CPU type.
All offsets are documented in the SDM and in the additional Skylake
manual except for MPX, and he reported that he'd ask for MPX to be
documented as well. As you said, if they changed it would be a total mess.
I hardcoded them because this is not KVM-specific knowledge. TCG could
in principle reuse the same code, just skipping the part where it masks
away features not supported by KVM.
Paolo
Re: [Qemu-devel] [PATCH V4] block: Add BlockDriver.bdrv_check_ext_snapshot.
On 2013-10-02 14:33, Benoît Canet wrote:
This field is used by blkverify to disable external snapshots creation.
I will also be used by block filters like quorum to disable external snapshots
creation.
Signed-off-by: Benoit Canet
---
block.c | 17 +
block/blkverify.c | 2 ++
blockdev.c| 5 +
include/block/block.h | 14 ++
include/block/block_int.h | 6 ++
5 files changed, 44 insertions(+)
diff --git a/block.c b/block.c
index 93e113a..e891522 100644
--- a/block.c
+++ b/block.c
@@ -4632,3 +4632,20 @@ int bdrv_amend_options(BlockDriverState *bs,
QEMUOptionParameter *options)
}
return bs->drv->bdrv_amend_options(bs, options);
}
+
+ExtSnapshotPerm bdrv_check_ext_snapshot(BlockDriverState *bs)
+{
+/* external snashots are allowed by defaults */
I don't really know whether it should rather be "by default", but it
certainly should be s/snashots/snapshots/. ;)
Also, I'd move this comment below the if statements and above the return
EXT_SNAPSHOT_ALLOWED.
+if (bs->drv->bdrv_check_ext_snapshot) {
+return bs->drv->bdrv_check_ext_snapshot(bs);
+}
+if (bs->file && bs->file->drv && bs->file->drv->bdrv_check_ext_snapshot) {
+return bs->file->drv->bdrv_check_ext_snapshot(bs);
+}
+return EXT_SNAPSHOT_ALLOWED;
+}
+
+ExtSnapshotPerm bdrv_check_ext_snapshot_forbidden(BlockDriverState *bs)
+{
+return EXT_SNAPSHOT_FORBIDDEN;
+}
diff --git a/block/blkverify.c b/block/blkverify.c
index 2077d8a..b64c638 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -313,6 +313,8 @@ static BlockDriver bdrv_blkverify = {
.bdrv_aio_readv = blkverify_aio_readv,
.bdrv_aio_writev= blkverify_aio_writev,
.bdrv_aio_flush = blkverify_aio_flush,
+
+.bdrv_check_ext_snapshot = bdrv_check_ext_snapshot_forbidden,
};
static void bdrv_blkverify_init(void)
diff --git a/blockdev.c b/blockdev.c
index 8aa66a9..92029d8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1131,6 +1131,11 @@ static void
external_snapshot_prepare(BlkTransactionState *common,
}
}
+if (bdrv_check_ext_snapshot(state->old_bs) != EXT_SNAPSHOT_ALLOWED) {
+error_set(errp, QERR_FEATURE_DISABLED, "snapshot");
+return;
+}
+
flags = state->old_bs->open_flags;
/* create new image w/backing file */
diff --git a/include/block/block.h b/include/block/block.h
index f808550..2bf0e12 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -244,6 +244,20 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix);
int bdrv_amend_options(BlockDriverState *bs_new, QEMUOptionParameter *options);
+/* external snapshots */
+
+typedef enum {
+EXT_SNAPSHOT_ALLOWED,
+EXT_SNAPSHOT_FORBIDDEN,
+} ExtSnapshotPerm;
+
+/* return EXT_SNAPSHOT_ALLOWED if external snapshot is allowed
+ * return EXT_SNAPSHOT_FORBIDEN if external snapshot is forbiden
s/FORBIDEN/FORBIDDEN/, s/forbiden/forbidden/
+ */
+ExtSnapshotPerm bdrv_check_ext_snapshot(BlockDriverState *bs);
+/* helper used to forbid external snapshots like in blkverify */
+ExtSnapshotPerm bdrv_check_ext_snapshot_forbidden(BlockDriverState *bs);
+
/* async block I/O */
typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,
int sector_num);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 211087a..e6a14ae 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -67,6 +67,12 @@ typedef struct BdrvTrackedRequest {
struct BlockDriver {
const char *format_name;
int instance_size;
+
+/* if not defined external snapshots are allowed
+ * future block filters will query their children to build the response
+ */
+ExtSnapshotPerm (*bdrv_check_ext_snapshot)(BlockDriverState *bs);
+
int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
int (*bdrv_probe_device)(const char *filename);
Aside from the misspellings (and regardless of whether you decide to
move the comment or not):
Reviewed-by: Max Reitz
Re: [Qemu-devel] [PATCHv4] block/get_block_status: avoid redundant callouts on raw devices
Am 02.10.2013 17:13, schrieb Paolo Bonzini: > Il 02/10/2013 17:06, Stefan Hajnoczi ha scritto: >> Sorry I didn't review this earlier but this flag looks hacky and I'm not >> confident about merging the patch yet. >> >> The patch makes me wonder if the raw_bsd driver should avoid calling >> bs->file itself: >> >> return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | >>(sector_num << BDRV_SECTOR_BITS); >> >> Let block.c:bdrv_co_get_block_status() call down into bs->file. >> >> The problem is then the protocol cannot report unallocated sectors with >> this approach. >> >> I think we want to preserve bs' offset while taking the other flags from >> bs->file (DATA, ZERO). > This would cause other changes. For example, a qcow2 with full metadata > preallocation (i.e. all L2 tables are there but it points to holes) > would not return DATA anymore. I think this is wrong, and especially a > change from the old is_allocated API. > > However, a variant on this idea could be to return > >BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | > (sector_num << BDRV_SECTOR_BITS); > > and then BDRV_BLOCK_RAW would mean "take DATA and ZERO from bs->file". I am fine with that. Peter
Re: [Qemu-devel] [PATCH V4] block: Add BlockDriver.bdrv_check_ext_snapshot.
On 2013-10-02 14:33, Benoît Canet wrote: This field is used by blkverify to disable external snapshots creation. I will also be used by block filters like quorum to disable external snapshots creation. Oh, I nearly missed it: s/I will also be/It will also be/ and probably s/external snapshots creation/external snapshot creation/, but I'm not too sure about the last one.
Re: [Qemu-devel] [PATCHv3 02/20] block: add flags to bdrv_*_write_zeroes
On 09/24/2013 07:34 AM, Peter Lieven wrote: > Signed-off-by: Peter Lieven > --- > block-migration.c |2 +- > block.c | 20 +++- > block/backup.c|3 ++- > block/qcow2-cluster.c |2 +- > block/qcow2.c |2 +- > block/qed.c |3 ++- > block/raw_bsd.c |5 +++-- > block/vmdk.c |3 ++- > include/block/block.h |4 ++-- > include/block/block_int.h |2 +- > qemu-io-cmds.c|2 +- > 11 files changed, 27 insertions(+), 21 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2 0/6] Improve getauxval support
On 10/01/2013 06:42 PM, Peter Maydell wrote: > On 2 October 2013 06:25, Richard Henderson wrote: >> Ping. > > Does this need updating now that commit 03cfd8faa (which manually > walks through the auxval in get_execfd()) has been committed? Well, such a patch could be added to the series, but it should not conflict with my patch set. r~
Re: [Qemu-devel] [PATCH v2 uq/master 2/2] x86: cpuid: reconstruct leaf 0Dh data
On Fri, Sep 13, 2013 at 03:55:58PM +0200, Paolo Bonzini wrote:
> The data in leaf 0Dh depends on information from other feature bits.
> Instead of passing it blindly from the host, compute it based on
> whether these feature bits are enabled.
>
> Signed-off-by: Paolo Bonzini
> ---
> target-i386/cpu.c | 63 +++-
> 1 file changed, 47 insertions(+), 16 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index ac83106..e6179f4 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -328,6 +328,15 @@ X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
> };
> #undef REGISTER
>
> +typedef struct ExtSaveArea {
> +uint32_t feature, bits;
> +uint32_t offset, size;
> +} ExtSaveArea;
> +
> +static const ExtSaveArea ext_save_areas[] = {
> +[2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
> +.offset = 0x100, .size = 0x240 },
> +};
>
> const char *get_register_name_32(unsigned int reg)
> {
> @@ -2169,29 +2178,51 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
> *edx = 0;
> }
> break;
> -case 0xD:
> +case 0xD: {
> +KVMState *s = cs->kvm_state;
> +uint64_t kvm_mask;
> +int i;
> +
> /* Processor Extended State */
> -if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE)) {
> -*eax = 0;
> -*ebx = 0;
> -*ecx = 0;
> -*edx = 0;
> +*eax = 0;
> +*ebx = 0;
> +*ecx = 0;
> +*edx = 0;
> +if (!(env->features[FEAT_1_ECX] & CPUID_EXT_XSAVE) ||
> !kvm_enabled()) {
> break;
> }
> -if (kvm_enabled()) {
> -KVMState *s = cs->kvm_state;
> +kvm_mask =
> +kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EAX) |
> +((uint64_t)kvm_arch_get_supported_cpuid(s, 0xd, 0, R_EDX) << 32);
>
> -*eax = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EAX);
> -*ebx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EBX);
> -*ecx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_ECX);
> -*edx = kvm_arch_get_supported_cpuid(s, 0xd, count, R_EDX);
> -} else {
> -*eax = 0;
> -*ebx = 0;
> -*ecx = 0;
> -*edx = 0;
> +if (count == 0) {
> +*ecx = 0x240;
> +for (i = 2; i < ARRAY_SIZE(ext_save_areas); i++) {
> +const ExtSaveArea *esa = &ext_save_areas[i];
> +if ((env->features[esa->feature] & esa->bits) == esa->bits &&
> +(kvm_mask & (1 << i)) != 0) {
> +if (i < 32) {
> +*eax |= 1 << i;
> +} else {
> +*edx |= 1 << (i - 32);
> +}
> +*ecx = MAX(*ecx, esa->offset + esa->size);
> +}
> +}
> +*eax |= kvm_mask & 3;
Lets use define from previous patch.
> +*ebx = *ecx;
> +} else if (count == 1) {
> +*eax = kvm_arch_get_supported_cpuid(s, 0xd, 1, R_EAX);
> +} else if (count < ARRAY_SIZE(ext_save_areas)) {
> +const ExtSaveArea *esa = &ext_save_areas[count];
> +if ((env->features[esa->feature] & esa->bits) == esa->bits &&
> +(kvm_mask & (1 << count)) != 0) {
> +*eax = esa->offset;
> +*ebx = esa->size;
Why do you hard code them instead of querying kernel? What if they
depend on cpu type? (well if this happens we can forget about
migration, but still...)
> +}
> }
> break;
> +}
> case 0x8000:
> *eax = env->cpuid_xlevel;
> *ebx = env->cpuid_vendor1;
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gleb.
Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs
Il 02/10/2013 14:41, Marcel Apfelbaum ha scritto:
> +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> +{
> +pci_irq_lower(pci_dev);
> +pci_irq_raise(pci_dev);
> +}
> +
Why is this in the opposite order, compared to qemu_irq_pulse?
Paolo
Re: [Qemu-devel] [PATCHv4] block/get_block_status: avoid redundant callouts on raw devices
Il 02/10/2013 17:06, Stefan Hajnoczi ha scritto: > Sorry I didn't review this earlier but this flag looks hacky and I'm not > confident about merging the patch yet. > > The patch makes me wonder if the raw_bsd driver should avoid calling > bs->file itself: > > return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | >(sector_num << BDRV_SECTOR_BITS); > > Let block.c:bdrv_co_get_block_status() call down into bs->file. > > The problem is then the protocol cannot report unallocated sectors with > this approach. > > I think we want to preserve bs' offset while taking the other flags from > bs->file (DATA, ZERO). This would cause other changes. For example, a qcow2 with full metadata preallocation (i.e. all L2 tables are there but it points to holes) would not return DATA anymore. I think this is wrong, and especially a change from the old is_allocated API. However, a variant on this idea could be to return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | (sector_num << BDRV_SECTOR_BITS); and then BDRV_BLOCK_RAW would mean "take DATA and ZERO from bs->file". Paolo > Peter, Paolo: What do you think of this approach?
Re: [Qemu-devel] [PATCHv4] block/get_block_status: avoid redundant callouts on raw devices
On 10/02/2013 08:20 AM, Peter Lieven wrote: > if a raw device like an iscsi target or host device is used > the current implementation makes a second call out to get > the block status of bs->file. however, the raw driver already > has called bdrv_get_block_status on bs->file. > > v4: use a flag to detect the raw driver instead of the strncmp > hack. The v4 designation... > > Signed-off-by: Peter Lieven > --- ...should appear after the --- separator (useful to reviewers, but doesn't need to be part of the git history). > block.c |4 ++-- > block/raw_bsd.c |6 +- > include/block/block.h |3 +++ > 3 files changed, 10 insertions(+), 3 deletions(-) Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCHv4] block/get_block_status: avoid redundant callouts on raw devices
On Wed, Oct 02, 2013 at 04:20:36PM +0200, Peter Lieven wrote:
> if a raw device like an iscsi target or host device is used
> the current implementation makes a second call out to get
> the block status of bs->file. however, the raw driver already
> has called bdrv_get_block_status on bs->file.
>
> v4: use a flag to detect the raw driver instead of the strncmp
> hack.
>
> Signed-off-by: Peter Lieven
> ---
> block.c |4 ++--
> block/raw_bsd.c |6 +-
> include/block/block.h |3 +++
> 3 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/block.c b/block.c
> index 93e113a..7fa2e43 100644
> --- a/block.c
> +++ b/block.c
> @@ -3161,7 +3161,7 @@ static int64_t coroutine_fn
> bdrv_co_get_block_status(BlockDriverState *bs,
>
> if (bs->file &&
> (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
> -(ret & BDRV_BLOCK_OFFSET_VALID)) {
> +(ret & BDRV_BLOCK_OFFSET_VALID) && !(ret & BDRV_BLOCK_RAW)) {
> ret2 = bdrv_co_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS,
> *pnum, pnum);
> if (ret2 >= 0) {
> @@ -3172,7 +3172,7 @@ static int64_t coroutine_fn
> bdrv_co_get_block_status(BlockDriverState *bs,
> }
> }
>
> -return ret;
> +return ret & ~BDRV_BLOCK_RAW;
> }
>
> /* Coroutine wrapper for bdrv_get_block_status() */
> diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> index d4ace60..a9e0209 100644
> --- a/block/raw_bsd.c
> +++ b/block/raw_bsd.c
> @@ -62,7 +62,11 @@ static int64_t coroutine_fn
> raw_co_get_block_status(BlockDriverState *bs,
> int64_t sector_num,
> int nb_sectors, int *pnum)
> {
> -return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum);
> +int64_t ret = bdrv_get_block_status(bs->file, sector_num, nb_sectors,
> pnum);
> +if (ret < 0) {
> +return ret;
> +}
> +return ret | BDRV_BLOCK_RAW;
> }
>
> static int coroutine_fn raw_co_write_zeroes(BlockDriverState *bs,
> diff --git a/include/block/block.h b/include/block/block.h
> index f808550..cb7019b 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -84,6 +84,8 @@ typedef struct BlockDevOps {
> /* BDRV_BLOCK_DATA: data is read from bs->file or another file
> * BDRV_BLOCK_ZERO: sectors read as zero
> * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
> + * BDRV_BLOCK_RAW: used internally to indicate that the request
> + * was piped through the raw driver
Sorry I didn't review this earlier but this flag looks hacky and I'm not
confident about merging the patch yet.
The patch makes me wonder if the raw_bsd driver should avoid calling
bs->file itself:
return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID |
(sector_num << BDRV_SECTOR_BITS);
Let block.c:bdrv_co_get_block_status() call down into bs->file.
The problem is then the protocol cannot report unallocated sectors with
this approach.
I think we want to preserve bs' offset while taking the other flags from
bs->file (DATA, ZERO).
Peter, Paolo: What do you think of this approach?
Stefan
[Qemu-devel] [PATCH] qemu-iotests: Correct 026 output
Because l2_allocate now frees the unused L2 cluster on error, the according test cases in 026 don't result in one leaked cluster anymore. Signed-off-by: Max Reitz --- This patch depends on and is a follow-up to "qcow2: Free allocated L2 cluster on error" which was part of the "qcow2: Small error path fixes for l2_allocate" series. This series is fully merged to Kevin's block branch, however, this one patch was excluded from his pull request last Friday, therefore, it is missing from master. --- tests/qemu-iotests/026.out | 32 tests/qemu-iotests/026.out.nocache | 32 2 files changed, 16 insertions(+), 48 deletions(-) diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out index 0764389..1504579 100644 --- a/tests/qemu-iotests/026.out +++ b/tests/qemu-iotests/026.out @@ -5,16 +5,12 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l1_update; errno: 5; imm: off; once: on; write write failed: Input/output error - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l1_update; errno: 5; imm: off; once: on; write -b write failed: Input/output error - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l1_update; errno: 5; imm: off; once: off; write @@ -33,16 +29,12 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l1_update; errno: 28; imm: off; once: on; write write failed: No space left on device - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l1_update; errno: 28; imm: off; once: on; write -b write failed: No space left on device - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l1_update; errno: 28; imm: off; once: off; write @@ -181,16 +173,12 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l2_alloc.write; errno: 5; imm: off; once: on; write write failed: Input/output error - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l2_alloc.write; errno: 5; imm: off; once: on; write -b write failed: Input/output error - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l2_alloc.write; errno: 5; imm: off; once: off; write @@ -207,16 +195,12 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l2_alloc.write; errno: 28; imm: off; once: on; write write failed: No space left on device - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l2_alloc.write; errno: 28; imm: off; once: on; write -b write failed: No space left on device - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l2_alloc.write; errno: 28; imm: off; once: off; write diff --git a/tests/qemu-iotests/026.out.nocache b/tests/qemu-iotests/026.out.nocache index 33bad0d..c9d242e 100644 --- a/tests/qemu-iotests/026.out.nocache +++ b/tests/qemu-iotests/026.out.nocache @@ -5,16 +5,12 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l1_update; errno: 5; imm: off; once: on; write write failed: Input/output error - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l1_update; errno: 5; imm: off; once: on; write -b write failed: Input/output error - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l1_update; errno: 5; imm: off; once: off; write @@ -33,16 +29,12 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 Event: l1_update; errno: 28; imm: off; once: on; write write failed: No space left on device - -1 leaked clusters were found on the image. -This means waste of disk space, but no harm to data. +No errors
Re: [Qemu-devel] [PATCH RFC 10/11] s390/qemu: cpu model QMP query-cpu-model
On Wed, 02 Oct 2013 06:06:32 -0600
Eric Blake wrote:
> On 10/02/2013 05:33 AM, Michael Mueller wrote:
> > This patch implements a new QMP request named "cpu-model". It returns
> > the cpu model of cpu 0. eg:
> >
> > request: '{"execute" : "query-cpu-model" }
> > answer: '{"return" : "2817-ga1" }
> >
> > Alias names are resolved to their respactive machine type and GA names
> > already during cpu instantiation. Thus, also a cpu name like "host",
> > which is iplemented as alias, will return its normalized cpu model name.
> >
> > Signed-off-by: Michael Mueller
> > ---
>
> > +++ b/qapi-schema.json
> > @@ -3129,6 +3129,29 @@
> > ##
> > { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
> >
> > +##
> > +# @CpuModelInfo:
> > +#
> > +# Virtual CPU model definition.
> > +#
> > +# @name: the name of the CPU model definition
> > +#
> > +# Since: 1.x.0
>
> s/x/7/ - you are targetting 1.7.0 after all.
welcome
>
> > +##
> > +{ 'type': 'CpuModelInfo',
> > + 'data': { 'name': 'str' } }
> > +
> > +##
> > +# @query-cpu-model:
> > +#
> > +# Return to current virtual CPU model
> > +#
> > +# Returns: CpuModelInfo
> > +#
> > +# Since: 1.2.0
>
> s/2/7/
yep
>
signature.asc
Description: PGP signature
[Qemu-devel] [PATCHv4] block/get_block_status: avoid redundant callouts on raw devices
if a raw device like an iscsi target or host device is used
the current implementation makes a second call out to get
the block status of bs->file. however, the raw driver already
has called bdrv_get_block_status on bs->file.
v4: use a flag to detect the raw driver instead of the strncmp
hack.
Signed-off-by: Peter Lieven
---
block.c |4 ++--
block/raw_bsd.c |6 +-
include/block/block.h |3 +++
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 93e113a..7fa2e43 100644
--- a/block.c
+++ b/block.c
@@ -3161,7 +3161,7 @@ static int64_t coroutine_fn
bdrv_co_get_block_status(BlockDriverState *bs,
if (bs->file &&
(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
-(ret & BDRV_BLOCK_OFFSET_VALID)) {
+(ret & BDRV_BLOCK_OFFSET_VALID) && !(ret & BDRV_BLOCK_RAW)) {
ret2 = bdrv_co_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS,
*pnum, pnum);
if (ret2 >= 0) {
@@ -3172,7 +3172,7 @@ static int64_t coroutine_fn
bdrv_co_get_block_status(BlockDriverState *bs,
}
}
-return ret;
+return ret & ~BDRV_BLOCK_RAW;
}
/* Coroutine wrapper for bdrv_get_block_status() */
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index d4ace60..a9e0209 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -62,7 +62,11 @@ static int64_t coroutine_fn
raw_co_get_block_status(BlockDriverState *bs,
int64_t sector_num,
int nb_sectors, int *pnum)
{
-return bdrv_get_block_status(bs->file, sector_num, nb_sectors, pnum);
+int64_t ret = bdrv_get_block_status(bs->file, sector_num, nb_sectors,
pnum);
+if (ret < 0) {
+return ret;
+}
+return ret | BDRV_BLOCK_RAW;
}
static int coroutine_fn raw_co_write_zeroes(BlockDriverState *bs,
diff --git a/include/block/block.h b/include/block/block.h
index f808550..cb7019b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -84,6 +84,8 @@ typedef struct BlockDevOps {
/* BDRV_BLOCK_DATA: data is read from bs->file or another file
* BDRV_BLOCK_ZERO: sectors read as zero
* BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
+ * BDRV_BLOCK_RAW: used internally to indicate that the request
+ * was piped through the raw driver
*
* If BDRV_BLOCK_OFFSET_VALID is set, bits 9-62 represent the offset in
* bs->file where sector data can be read from as raw data.
@@ -105,6 +107,7 @@ typedef struct BlockDevOps {
#define BDRV_BLOCK_DATA 1
#define BDRV_BLOCK_ZERO 2
#define BDRV_BLOCK_OFFSET_VALID 4
+#define BDRV_BLOCK_RAW 8
#define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK
typedef enum {
--
1.7.9.5
Re: [Qemu-devel] [PATCH -V4 RESEND 0/6] target-ppc: Add support for dumping guest memory using qemu gdb server
On 01.10.2013, at 18:19, Aneesh Kumar K.V wrote: > Hi, > > This patch series implement support for dumping guest memory using qemu gdb > server. The last patch also enable qemu monitor command dump-guest-memory Thanks, applied all but 2/6 to ppc-next. I think the core dump bits should be more smart eventually to determine whether we want to save ppc32 or ppc64 dumps, as qemu-system-ppc64 can execute 32bit CPUs. But for now your patch as is should work. Alex
Re: [Qemu-devel] [PATCH v7 00/27] qemu: generate acpi tables for the guest
On Wed, Oct 02, 2013 at 03:52:17PM +0200, Igor Mammedov wrote: > On Wed, 2 Oct 2013 16:30:36 +0300 > "Michael S. Tsirkin" wrote: > > > On Wed, Oct 02, 2013 at 03:05:52PM +0200, Igor Mammedov wrote: > > > On Wed, 2 Oct 2013 00:26:11 +0300 > > > "Michael S. Tsirkin" wrote: > > > > > > > This code can also be found here: > > > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi > > > > > > > > While this patch still uses info not available in QOM, I think it's > > > > reasonable > > > > to merge it and then refactor as QOM properties cover more ground. > > > > > > > > In particular, merging this patchset blocks other projects so > > > > I think its preferable to merge now and not wait > > > > for all required QOM properties to materialize. > > > > > > > > I added QOM properties in ich/piix where I knew how to > > > > do this. > > > > > > > > If you already reviewed v4 or later then not much changed. > > > > > > > > Patches 1-2 are trivial style changes. > > > > Igor posted a drop-in replacement for them: see > > > > [PATCH] cleanup object.h: include error.h directly > > > > that can be used instead if it's deemed preferable, > > > > patchset was tested with that too and works the same. > > > > > > > > Patches 3-4 are QOM patches really. They are > > > > same as the patchset I posted previously, > > > > included here for completeness. > > > > > > > > Igor suggested dropping patches 1-2 and including error.h directly. > > > > OTOH Paolo already acked 1-4. > > > > I think I agree with Paolo so I kept 1-2 around for now. > > > > I hope that's ok. > > > > They are trivial one-liners so will be easy to revert later > > > > if we want to. > > > > > > > > If everything's in order, I intend to merge this through my tree. > > > > > > It breaks hotpluged CPUs ACPI tables. > > > > > > steps to reproduce: > > > 1. qemu-system-x86_64 -enable-kvm -m 1024 -monitor stdio -smp > > > 1,maxcpus=4 -qmp unix:/tmp/qmp-sock,server,nowait test.qcow2 > > > 2. hotadd CPU to guest: http://wiki.qemu.org/Features/CPUHotplug > > > 3. reboot guest > > > > Does it work if you *don't* reboot? > it does, until reboot. I see, we need to patch this info on FW CFG read too (not on reset since bios might have onlined some CPUs). > > > > > 4. check that ALL CPUs are running/visible to guest > > > > > > Actual result: only initial CPUs are visible to guest > > > (qemu) info cpus > > > * CPU #0: pc=0x000f891a thread_id=22059 > > > CPU #1: pc=0x000f6a37 (halted) thread_id=22060 > > > > > > should be something like: > > > (qemu) info cpus > > > * CPU #0: pc=0x81042086 (halted) thread_id=22059 > > > CPU #1: pc=0x81042086 (halted) thread_id=22060 > > > > > > > > > > > Please review, and comment. > > > > > > > > Changes from v6: > > > > - fix 64 bit window bug reported by Igor > > > > - tweak comments in error.h > > > > > > > > Changes from v5: > > > > - update generated files to fix build on systems without iasl > > > > - fix mcfg failure reported by Gerd > > > > Changes from v4: > > > > - address comments by Paolo: > > > > rename loader interface > > > > reuse macro for hpet name > > > > better struct names > > > > move internal headers to hw/i386/ > > > > - fix typos resulting in bugs reported by Gerd > > > > > > > > Changes from v3: > > > > - reworked code to use QOM properties > > > > some info isn't yet available in QOM, > > > > use old-style APIs and lookups by type > > > > - address comments by Gerd: tables are now updated > > > > on guest access after pci configuration > > > > > > > > Changes from v2 repost: > > > > - address comment by Anthony - convert to use APIs implemented > > > > using QOM > > > > - address comment by Anthony - avoid tricky pointer path, > > > > use GArray from glib instead > > > > - Address lots of comments by Hu Tao and Laszlo Ersek > > > > > > > > Changes from v2: > > > > - added missing patches to make it actually build > > > > Changes from v1 RFC: > > > > - added code to address cross version compatibility > > > > - rebased to latest bits > > > > - updated seabios code to latest bits (added pvpanic device) > > > > > > > > This patchset moves all generation of ACPI tables > > > > from guest BIOS to the hypervisor. > > > > > > > > Although ACPI tables come from a system BIOS on real hw, > > > > it makes sense that the ACPI tables are coupled with the > > > > virtual machine, since they have to abstract the x86 machine to > > > > the OS's. > > > > > > > > This is widely desired as a way to avoid the churn > > > > and proliferation of QEMU-specific interfaces > > > > associated with ACPI tables in bios code. > > > > > > > > In particular, this patchset (together with coreboot > > > > support written by Gerd) allows coreboot to work correctly > > > > with all the ACPI features that plain seabios has. > > > > > > > > There's a bit of code duplication where we > > > > already declare similar acpi structures in qemu.
Re: [Qemu-devel] [PATCH -V4 2/4] target-ppc: Fix page table lookup with kvm enabled
On 01.10.2013, at 03:27, Aneesh Kumar K.V wrote:
> Alexander Graf writes:
>
>> On 09/05/2013 10:16 AM, Aneesh Kumar K.V wrote:
>>> From: "Aneesh Kumar K.V"
>>>
>>> With kvm enabled, we store the hash page table information in the
>>> hypervisor.
>>> Use ioctl to read the htab contents. Without this we get the below error
>>> when
>>> trying to read the guest address
>>>
>>> (gdb) x/10 do_fork
>>> 0xc0098660: Cannot access memory at address
>>> 0xc0098660
>>> (gdb)
>>>
>>> Signed-off-by: Aneesh Kumar K.V
>>> ---
>>> target-ppc/kvm.c| 59
>>> +
>>> target-ppc/kvm_ppc.h| 12 +-
>>> target-ppc/mmu-hash64.c | 57
>>> ---
>>> 3 files changed, 104 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>> index 1838465..05b066c 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -1888,3 +1888,62 @@ int kvm_arch_on_sigbus(int code, void *addr)
>>> void kvm_arch_init_irq_routing(KVMState *s)
>>> {
>>> }
>>> +
>>> +hwaddr kvmppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
>>> + bool secondary, target_ulong ptem,
>>> + target_ulong *hpte0, target_ulong *hpte1)
>>> +{
>>> +int htab_fd;
>>> +uint64_t index;
>>> +hwaddr pte_offset;
>>> +target_ulong pte0, pte1;
>>> +struct kvm_get_htab_fd ghf;
>>> +struct kvm_get_htab_buf {
>>> +struct kvm_get_htab_header header;
>>> +/*
>>> + * Older kernel required one extra byte.
>>
>> Older than what?
>>
>>> + */
>
> Since we decided to drop that kernel patch, that should be updated as
> "kernel requires one extra byte".
>
>>> +unsigned long hpte[(HPTES_PER_GROUP * 2) + 1];
>>> +} hpte_buf;
>>> +
>>> +index = (hash * HPTES_PER_GROUP)& cpu->env.htab_mask;
>>> +*hpte0 = 0;
>>> +*hpte1 = 0;
>>> +if (!cap_htab_fd) {
>>> +return 0;
>>> +}
>>> +
>
> .
>
>>>
>>> -static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr pteg_off,
>>> +static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr hash,
>>> bool secondary, target_ulong ptem,
>>> ppc_hash_pte64_t *pte)
>>> {
>>> -hwaddr pte_offset = pteg_off;
>>> +hwaddr pte_offset;
>>> target_ulong pte0, pte1;
>>> -int i;
>>> -
>>> -for (i = 0; i< HPTES_PER_GROUP; i++) {
>>> -pte0 = ppc_hash64_load_hpte0(env, pte_offset);
>>> -pte1 = ppc_hash64_load_hpte1(env, pte_offset);
>>> -
>>> -if ((pte0& HPTE64_V_VALID)
>>> -&& (secondary == !!(pte0& HPTE64_V_SECONDARY))
>>> -&& HPTE64_V_COMPARE(pte0, ptem)) {
>>> -pte->pte0 = pte0;
>>> -pte->pte1 = pte1;
>>> -return pte_offset;
>>> +int i, ret = 0;
>>> +
>>> +if (kvm_enabled()) {
>>> +ret = kvmppc_hash64_pteg_search(ppc_env_get_cpu(env), hash,
>>> +secondary, ptem,
>>> +&pte->pte0,&pte->pte1);
>>
>> Instead of duplicating the search, couldn't you just hook yourself into
>> ppc_hash64_load_hpte0/1 and return the respective ptes from there? Just
>> cache the current pteg to ensure things don't become dog slow.
>>
>
> Can you explain this better ?
You're basically doing
hwaddr ppc_hash64_pteg_search(...)
{
if (kvm) {
pteg = read_from_kvm();
foreach pte in pteg {
if (match) return offset;
}
return -1;
} else {
foreach pte in pteg {
pte = read_pte_from_memory();
if (match) return offset;
}
return -1;
}
}
This is massive code duplication. The only difference between kvm and tcg are
the source for the pteg read. David already abstracted the actual pte0/pte1
reads away in ppc_hash64_load_hpte0 and ppc_hash64_load_hpte1 wrapper functions.
Now imagine we could add a temporary pteg store in env. Then have something
like this in ppc_hash64_load_hpte0:
if (need_kvm_htab_access) {
if (env->current_cached_pteg != this_pteg) (
read_pteg(env->cached_pteg);
return env->cached_pteg[x].pte0;
}
} else {
}
That way the actual resolver doesn't care where the PTEG comes from, as it only
ever checks pte0/pte1 and leaves all the magic on where those come from to the
load function.
Alex
Re: [Qemu-devel] [PATCH v7 00/27] qemu: generate acpi tables for the guest
On Wed, 2 Oct 2013 16:30:36 +0300 "Michael S. Tsirkin" wrote: > On Wed, Oct 02, 2013 at 03:05:52PM +0200, Igor Mammedov wrote: > > On Wed, 2 Oct 2013 00:26:11 +0300 > > "Michael S. Tsirkin" wrote: > > > > > This code can also be found here: > > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi > > > > > > While this patch still uses info not available in QOM, I think it's > > > reasonable > > > to merge it and then refactor as QOM properties cover more ground. > > > > > > In particular, merging this patchset blocks other projects so > > > I think its preferable to merge now and not wait > > > for all required QOM properties to materialize. > > > > > > I added QOM properties in ich/piix where I knew how to > > > do this. > > > > > > If you already reviewed v4 or later then not much changed. > > > > > > Patches 1-2 are trivial style changes. > > > Igor posted a drop-in replacement for them: see > > > [PATCH] cleanup object.h: include error.h directly > > > that can be used instead if it's deemed preferable, > > > patchset was tested with that too and works the same. > > > > > > Patches 3-4 are QOM patches really. They are > > > same as the patchset I posted previously, > > > included here for completeness. > > > > > > Igor suggested dropping patches 1-2 and including error.h directly. > > > OTOH Paolo already acked 1-4. > > > I think I agree with Paolo so I kept 1-2 around for now. > > > I hope that's ok. > > > They are trivial one-liners so will be easy to revert later > > > if we want to. > > > > > > If everything's in order, I intend to merge this through my tree. > > > > It breaks hotpluged CPUs ACPI tables. > > > > steps to reproduce: > > 1. qemu-system-x86_64 -enable-kvm -m 1024 -monitor stdio -smp 1,maxcpus=4 > > -qmp unix:/tmp/qmp-sock,server,nowait test.qcow2 > > 2. hotadd CPU to guest: http://wiki.qemu.org/Features/CPUHotplug > > 3. reboot guest > > Does it work if you *don't* reboot? it does, until reboot. > > > 4. check that ALL CPUs are running/visible to guest > > > > Actual result: only initial CPUs are visible to guest > > (qemu) info cpus > > * CPU #0: pc=0x000f891a thread_id=22059 > > CPU #1: pc=0x000f6a37 (halted) thread_id=22060 > > > > should be something like: > > (qemu) info cpus > > * CPU #0: pc=0x81042086 (halted) thread_id=22059 > > CPU #1: pc=0x81042086 (halted) thread_id=22060 > > > > > > > > Please review, and comment. > > > > > > Changes from v6: > > > - fix 64 bit window bug reported by Igor > > > - tweak comments in error.h > > > > > > Changes from v5: > > > - update generated files to fix build on systems without iasl > > > - fix mcfg failure reported by Gerd > > > Changes from v4: > > > - address comments by Paolo: > > > rename loader interface > > > reuse macro for hpet name > > > better struct names > > > move internal headers to hw/i386/ > > > - fix typos resulting in bugs reported by Gerd > > > > > > Changes from v3: > > > - reworked code to use QOM properties > > > some info isn't yet available in QOM, > > > use old-style APIs and lookups by type > > > - address comments by Gerd: tables are now updated > > > on guest access after pci configuration > > > > > > Changes from v2 repost: > > > - address comment by Anthony - convert to use APIs implemented > > > using QOM > > > - address comment by Anthony - avoid tricky pointer path, > > > use GArray from glib instead > > > - Address lots of comments by Hu Tao and Laszlo Ersek > > > > > > Changes from v2: > > > - added missing patches to make it actually build > > > Changes from v1 RFC: > > > - added code to address cross version compatibility > > > - rebased to latest bits > > > - updated seabios code to latest bits (added pvpanic device) > > > > > > This patchset moves all generation of ACPI tables > > > from guest BIOS to the hypervisor. > > > > > > Although ACPI tables come from a system BIOS on real hw, > > > it makes sense that the ACPI tables are coupled with the > > > virtual machine, since they have to abstract the x86 machine to > > > the OS's. > > > > > > This is widely desired as a way to avoid the churn > > > and proliferation of QEMU-specific interfaces > > > associated with ACPI tables in bios code. > > > > > > In particular, this patchset (together with coreboot > > > support written by Gerd) allows coreboot to work correctly > > > with all the ACPI features that plain seabios has. > > > > > > There's a bit of code duplication where we > > > already declare similar acpi structures in qemu. > > > > > > I think it's best to do it in this order: port > > > code directly, and apply cleanups and reduce duplication > > > that results, on top. > > > This way it's much easier to see that we don't introduce > > > regressions. > > > > > > In particular, I booted a guest on qemu with and without the > > > change, and verified that ACPI tables are > > > unchanged except for trivial pointer address changes, > >
[Qemu-devel] [Bug 1234179] [NEW] QEMU segfaults during Windows 7 unattended install
Public bug reported: During today's automated qemu.git testing, a segmentation fault while installing Windows 7 SP1 happened. qemu.git top commit: 10/02 01:30:24 INFO | git:0150| git commit ID is a684f3cf9b9b9c3cb82be87aafc463de8974610c (tag v1.4.0-4237-ga684f3c) commit a684f3cf9b9b9c3cb82be87aafc463de8974610c Merge: 349cd52 1cf9412 Author: Anthony Liguori Date: Mon Sep 30 17:15:27 2013 -0500 Merge remote-tracking branch 'kraxel/seabios-1.7.3.2' into staging # By Gerd Hoffmann # Via Gerd Hoffmann * kraxel/seabios-1.7.3.2: update seabios from 1.7.2.2 to 1.7.3.2 Message-id: 1380533055-24960-1-git-send-email-kra...@redhat.com We have the core file saved in our test servers, we can make arrangements to transfer it if there's someone interested in investigating further. The framework saved the 'bt full' of the core file, that was missing some debug info: [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". Core was generated by `/usr/local/autotest/tests/virt/qemu/qemu -S -name virt-tests-vm1 -M pc -nodefau'. Program terminated with signal 11, Segmentation fault. #0 0x7ffc8fb86cf0 in pixman_image_get_data () from /lib64/libpixman-1.so.0 #0 0x7ffc8fb86cf0 in pixman_image_get_data () from /lib64/libpixman-1.so.0 No symbol table info available. #1 0x7ffc9165b05c in ?? () No symbol table info available. #2 0x7ffc9382b540 in ?? () No symbol table info available. #3 0x7ffc8f359a8d in clock_gettime () from /lib64/libc.so.6 No symbol table info available. #4 0x7ffc9382b5a8 in ?? () No symbol table info available. #5 0x00019382b4c0 in ?? () No symbol table info available. #6 0x in ?? () No symbol table info available. Extra info: Commits for the submodules: 10/02 01:30:29 DEBUG|base_utils:0134| [stdout] Submodule path 'dtc': checked out 'bc895d6d09695d05ceb8b52486ffe861d6cfbdde' 10/02 01:30:51 DEBUG|base_utils:0134| [stdout] Submodule path 'pixman': checked out '97336fad32acf802003855cd8bd6477fa49a12e3' 10/02 01:30:58 DEBUG|base_utils:0134| [stdout] Submodule path 'roms/SLOF': checked out '8cfdfc43f4c4c8c8dfa4b7cf16f7c19c84eee812' 10/02 01:31:16 DEBUG|base_utils:0134| [stdout] Submodule path 'roms/ipxe': checked out '09c5109b8585178172c7608de8d52e9d9af0b680' 10/02 01:31:20 DEBUG|base_utils:0134| [stdout] Submodule path 'roms/openbios': checked out '0f3d51ef22ec9166beb3ed434d253029ed7cfe84' 10/02 01:31:21 DEBUG|base_utils:0134| [stdout] Submodule path 'roms/qemu-palcode': checked out 'c87a92639b28ac42bc8f6c67443543b405dc479b' 10/02 01:31:27 DEBUG|base_utils:0134| [stdout] Submodule path 'roms/seabios': checked out 'ece025f5980bae88fa677bc9c0d24d2e580e205d' 10/02 01:31:28 DEBUG|base_utils:0134| [stdout] Submodule path 'roms/sgabios': checked out '23d474943dcd55d0550a3d20b3d30e9040a4f15b' 10/02 01:31:31 DEBUG|base_utils:0134| [stdout] Submodule path 'roms/vgabios': checked out '19ea12c230ded95928ecaef0db47a82231c2e485' Configure options: 10/02 01:31:32 DEBUG|base_utils:0099| Running '/usr/local/autotest/tmp/virt/src/qemu/configure --target-list=x86_64-softmmu --disable-strip --prefix=/usr/local/autotest/tests/virt/qemu/install_root' 10/02 01:31:35 DEBUG|env_proces:0829| (address cache) DHCP lease OK: 00:30:48:c5:d6:e2 --> 10.16.72.38 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] Install prefix /usr/local/autotest/tests/virt/qemu/install_root 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] BIOS directory /usr/local/autotest/tests/virt/qemu/install_root/share/qemu 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] binary directory /usr/local/autotest/tests/virt/qemu/install_root/bin 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] library directory /usr/local/autotest/tests/virt/qemu/install_root/lib 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] libexec directory /usr/local/autotest/tests/virt/qemu/install_root/libexec 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] include directory /usr/local/autotest/tests/virt/qemu/install_root/include 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] config directory /usr/local/autotest/tests/virt/qemu/install_root/etc 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] local state directory /usr/local/autotest/tests/virt/qemu/install_root/var 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] Manual directory /usr/local/autotest/tests/virt/qemu/install_root/share/man 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] ELF interp prefix /usr/gnemul/qemu-%M 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] Source path /usr/local/autotest/tmp/virt/src/qemu 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] C compilercc 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] Host C compiler cc 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] C++ compiler c++ 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] Objective-C compiler cc 10/02 01:31:40 DEBUG|base_utils:0134| [stdout] CFLAGS-O2 -U_FORTIFY_SOURCE
Re: [Qemu-devel] [PATCH] block: use correct filename for error report
On Wed, Oct 2, 2013 at 5:48 PM, Stefan Hajnoczi wrote:
> On Tue, Sep 24, 2013 at 06:14:01PM +0800, Dunrong Huang wrote:
> > The content filename point to will be erased by qemu_opts_absorb_qdict()
> > in raw_open_common() in drv->bdrv_file_open()
> >
> > So it's better to use bs->filename.
> >
> > Signed-off-by: Dunrong Huang
> > ---
> > block.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> The same issue affects the rest of the function:
>
> #ifndef _WIN32
> if (bs->is_temporary) {
> assert(filename != NULL);
> unlink(filename);
> }
> #endif
>
> Do you want to send a separate patch to fix this?
>
Sure.
>
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block
>
> Stefan
>
--
Best Regards,
Dunrong Huang
Homepage: http://mathslinux.org
Re: [Qemu-devel] [PATCH] qcow2: Switch L1 table in a single sequence
On Mon, Sep 30, 2013 at 05:57:21PM +0200, Max Reitz wrote: > Switching the L1 table in memory should be an atomic operation, as far > as possible. Calling qcow2_free_clusters on the old L1 table on disk is > not a good idea when the old L1 table is no longer valid and the address > to the new one hasn't yet been written into the corresponding > BDRVQcowState field. To be more specific, this can lead to segfaults due > to qcow2_check_metadata_overlap trying to access the L1 table during the > free operation. > > Signed-off-by: Max Reitz > --- > block/qcow2-cluster.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH v7 00/27] qemu: generate acpi tables for the guest
On Wed, Oct 02, 2013 at 03:05:52PM +0200, Igor Mammedov wrote: > On Wed, 2 Oct 2013 00:26:11 +0300 > "Michael S. Tsirkin" wrote: > > > This code can also be found here: > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi > > > > While this patch still uses info not available in QOM, I think it's > > reasonable > > to merge it and then refactor as QOM properties cover more ground. > > > > In particular, merging this patchset blocks other projects so > > I think its preferable to merge now and not wait > > for all required QOM properties to materialize. > > > > I added QOM properties in ich/piix where I knew how to > > do this. > > > > If you already reviewed v4 or later then not much changed. > > > > Patches 1-2 are trivial style changes. > > Igor posted a drop-in replacement for them: see > > [PATCH] cleanup object.h: include error.h directly > > that can be used instead if it's deemed preferable, > > patchset was tested with that too and works the same. > > > > Patches 3-4 are QOM patches really. They are > > same as the patchset I posted previously, > > included here for completeness. > > > > Igor suggested dropping patches 1-2 and including error.h directly. > > OTOH Paolo already acked 1-4. > > I think I agree with Paolo so I kept 1-2 around for now. > > I hope that's ok. > > They are trivial one-liners so will be easy to revert later > > if we want to. > > > > If everything's in order, I intend to merge this through my tree. > > It breaks hotpluged CPUs ACPI tables. > > steps to reproduce: > 1. qemu-system-x86_64 -enable-kvm -m 1024 -monitor stdio -smp 1,maxcpus=4 > -qmp unix:/tmp/qmp-sock,server,nowait test.qcow2 > 2. hotadd CPU to guest: http://wiki.qemu.org/Features/CPUHotplug > 3. reboot guest Does it work if you *don't* reboot? > 4. check that ALL CPUs are running/visible to guest > > Actual result: only initial CPUs are visible to guest > (qemu) info cpus > * CPU #0: pc=0x000f891a thread_id=22059 > CPU #1: pc=0x000f6a37 (halted) thread_id=22060 > > should be something like: > (qemu) info cpus > * CPU #0: pc=0x81042086 (halted) thread_id=22059 > CPU #1: pc=0x81042086 (halted) thread_id=22060 > > > > > Please review, and comment. > > > > Changes from v6: > > - fix 64 bit window bug reported by Igor > > - tweak comments in error.h > > > > Changes from v5: > > - update generated files to fix build on systems without iasl > > - fix mcfg failure reported by Gerd > > Changes from v4: > > - address comments by Paolo: > > rename loader interface > > reuse macro for hpet name > > better struct names > > move internal headers to hw/i386/ > > - fix typos resulting in bugs reported by Gerd > > > > Changes from v3: > > - reworked code to use QOM properties > > some info isn't yet available in QOM, > > use old-style APIs and lookups by type > > - address comments by Gerd: tables are now updated > > on guest access after pci configuration > > > > Changes from v2 repost: > > - address comment by Anthony - convert to use APIs implemented > > using QOM > > - address comment by Anthony - avoid tricky pointer path, > > use GArray from glib instead > > - Address lots of comments by Hu Tao and Laszlo Ersek > > > > Changes from v2: > > - added missing patches to make it actually build > > Changes from v1 RFC: > > - added code to address cross version compatibility > > - rebased to latest bits > > - updated seabios code to latest bits (added pvpanic device) > > > > This patchset moves all generation of ACPI tables > > from guest BIOS to the hypervisor. > > > > Although ACPI tables come from a system BIOS on real hw, > > it makes sense that the ACPI tables are coupled with the > > virtual machine, since they have to abstract the x86 machine to > > the OS's. > > > > This is widely desired as a way to avoid the churn > > and proliferation of QEMU-specific interfaces > > associated with ACPI tables in bios code. > > > > In particular, this patchset (together with coreboot > > support written by Gerd) allows coreboot to work correctly > > with all the ACPI features that plain seabios has. > > > > There's a bit of code duplication where we > > already declare similar acpi structures in qemu. > > > > I think it's best to do it in this order: port > > code directly, and apply cleanups and reduce duplication > > that results, on top. > > This way it's much easier to see that we don't introduce > > regressions. > > > > In particular, I booted a guest on qemu with and without the > > change, and verified that ACPI tables are > > unchanged except for trivial pointer address changes, > > and the SSDT P_BLK change in the last patch. > > > > Such binary compatibility makes it easier to be > > confident that this change won't break things. > > > > > > Michael S. Tsirkin (27): > > qemu: add Error to typedefs > > qom: pull in qemu/typedefs > > qom: cleanup struct Error references > > qom: add pointer to int property he
Re: [Qemu-devel] [PATCH V3 0/7] qcow2: rollback the modification on fail in snapshot creation
On Mon, Sep 09, 2013 at 10:57:55AM +0800, Wenchao Xia wrote: > V2: > 1: all fail case will goto fail section. > 2: add the goto code. > v3: > Address Stefan's comments: > 2: don't goto fail after allocation failure. > 3: use sn->l1size correctly in qcow2_free_cluster(). > 4-7: add test case to verify the error paths. > Other: > 1: new patch fix a existing bug, which will be exposed in error path test. > > > Wenchao Xia (7): > 1 qcow2: restore nb_snapshots when fail in snapshot creation > 2 qcow2: free allocated cluster on fail in qcow2_write_snapshots() > 3 qcow2: cancel the modification on fail in qcow2_snapshot_create() > 4 blkdebug: add debug events for snapshot > 5 qcow2: use debug events for snapshot > 6 qcow2: print message for error path in snapshot creation > 7 qemu-iotests: add test for qcow2 snapshot > > block/blkdebug.c |4 + > block/qcow2-snapshot.c | 80 ++-- > include/block/block.h |4 + > tests/qemu-iotests/063 | 229 > > tests/qemu-iotests/063.out | 37 +++ > tests/qemu-iotests/group |1 + > 6 files changed, 348 insertions(+), 7 deletions(-) > create mode 100755 tests/qemu-iotests/063 > create mode 100644 tests/qemu-iotests/063.out Makes sense but keep in mind that it's better to leak clusters than to corrupt the image further. We have to be very careful in these error paths, especially when a big operation could have failed partway through. I left comments where I thought the patches need to be changed.
Re: [Qemu-devel] [PATCH V3 1/7] qcow2: restore nb_snapshots when fail in snapshot creation
On Mon, Sep 09, 2013 at 10:57:56AM +0800, Wenchao Xia wrote: > If it is not restored after qcow2_write_snapshots() fail, a core > dump will happen in bdrv_close() since access of invalid pointer. > > Signed-off-by: Wenchao Xia > --- > block/qcow2-snapshot.c |4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) Good candidate for -stable. Please add: Cc: qemu-sta...@nongnu.org
Re: [Qemu-devel] [PATCH] block: vhdx - add migration blocker
On Tue, Oct 01, 2013 at 11:59:20AM -0400, Jeff Cody wrote: > This blocks migration for VHDX image files, until the > functionality can be supported. > > Signed-off-by: Jeff Cody > --- > block/vhdx.c | 10 ++ > 1 file changed, 10 insertions(+) Strictly speaking live migration is safe since the format is read-only. The source machine will never write to the image, we don't need to worry about cache consistency. But given that your log reply and write patches are quite far along, let's just merge this now. We'll need it soon. Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH v7 00/27] qemu: generate acpi tables for the guest
On Wed, 2 Oct 2013 00:26:11 +0300 "Michael S. Tsirkin" wrote: > This code can also be found here: > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi > > While this patch still uses info not available in QOM, I think it's reasonable > to merge it and then refactor as QOM properties cover more ground. > > In particular, merging this patchset blocks other projects so > I think its preferable to merge now and not wait > for all required QOM properties to materialize. > > I added QOM properties in ich/piix where I knew how to > do this. > > If you already reviewed v4 or later then not much changed. > > Patches 1-2 are trivial style changes. > Igor posted a drop-in replacement for them: see > [PATCH] cleanup object.h: include error.h directly > that can be used instead if it's deemed preferable, > patchset was tested with that too and works the same. > > Patches 3-4 are QOM patches really. They are > same as the patchset I posted previously, > included here for completeness. > > Igor suggested dropping patches 1-2 and including error.h directly. > OTOH Paolo already acked 1-4. > I think I agree with Paolo so I kept 1-2 around for now. > I hope that's ok. > They are trivial one-liners so will be easy to revert later > if we want to. > > If everything's in order, I intend to merge this through my tree. It breaks hotpluged CPUs ACPI tables. steps to reproduce: 1. qemu-system-x86_64 -enable-kvm -m 1024 -monitor stdio -smp 1,maxcpus=4 -qmp unix:/tmp/qmp-sock,server,nowait test.qcow2 2. hotadd CPU to guest: http://wiki.qemu.org/Features/CPUHotplug 3. reboot guest 4. check that ALL CPUs are running/visible to guest Actual result: only initial CPUs are visible to guest (qemu) info cpus * CPU #0: pc=0x000f891a thread_id=22059 CPU #1: pc=0x000f6a37 (halted) thread_id=22060 should be something like: (qemu) info cpus * CPU #0: pc=0x81042086 (halted) thread_id=22059 CPU #1: pc=0x81042086 (halted) thread_id=22060 > > Please review, and comment. > > Changes from v6: > - fix 64 bit window bug reported by Igor > - tweak comments in error.h > > Changes from v5: > - update generated files to fix build on systems without iasl > - fix mcfg failure reported by Gerd > Changes from v4: > - address comments by Paolo: > rename loader interface > reuse macro for hpet name > better struct names > move internal headers to hw/i386/ > - fix typos resulting in bugs reported by Gerd > > Changes from v3: > - reworked code to use QOM properties > some info isn't yet available in QOM, > use old-style APIs and lookups by type > - address comments by Gerd: tables are now updated > on guest access after pci configuration > > Changes from v2 repost: > - address comment by Anthony - convert to use APIs implemented > using QOM > - address comment by Anthony - avoid tricky pointer path, > use GArray from glib instead > - Address lots of comments by Hu Tao and Laszlo Ersek > > Changes from v2: > - added missing patches to make it actually build > Changes from v1 RFC: > - added code to address cross version compatibility > - rebased to latest bits > - updated seabios code to latest bits (added pvpanic device) > > This patchset moves all generation of ACPI tables > from guest BIOS to the hypervisor. > > Although ACPI tables come from a system BIOS on real hw, > it makes sense that the ACPI tables are coupled with the > virtual machine, since they have to abstract the x86 machine to > the OS's. > > This is widely desired as a way to avoid the churn > and proliferation of QEMU-specific interfaces > associated with ACPI tables in bios code. > > In particular, this patchset (together with coreboot > support written by Gerd) allows coreboot to work correctly > with all the ACPI features that plain seabios has. > > There's a bit of code duplication where we > already declare similar acpi structures in qemu. > > I think it's best to do it in this order: port > code directly, and apply cleanups and reduce duplication > that results, on top. > This way it's much easier to see that we don't introduce > regressions. > > In particular, I booted a guest on qemu with and without the > change, and verified that ACPI tables are > unchanged except for trivial pointer address changes, > and the SSDT P_BLK change in the last patch. > > Such binary compatibility makes it easier to be > confident that this change won't break things. > > > Michael S. Tsirkin (27): > qemu: add Error to typedefs > qom: pull in qemu/typedefs > qom: cleanup struct Error references > qom: add pointer to int property helpers > pci: fix up w64 size calculation helper > fw_cfg: interface to trigger callback on read > loader: support for unmapped ROM blobs > pcie_host: expose UNMAPPED macro > pcie_host: expose address format > q35: use macro for MCFG property name > q35: expose mmcfg size as a property > i386: add ACPI table files from seabios > acpi: add rules to compile
Re: [Qemu-devel] [PATCH 1/7] usb: remove old usb-host code
On 2013-09-19 11:34, Gerd Hoffmann wrote: > The usb-host code has been rewritten for qemu 1.5 to use libusb, > the old code has been left in as temporary fallback. Now we are > two releases further out, targeting the 1.7 release. No major > issues with the new code poped up until now. Time to remove it > from tre tree. Should we ever need it again for some reason -- > git has a copy for us in the history. Well, maybe not many users were actually exploiting the new libusb code, thus were able to produce feedback. Only very recent distros fulfill the need of >= 1.0.13, so you naturally fall back to this code. I just realized that even the factory build of OpenSUSE is still on libusb-1.0.9. Current Ubuntu versions are on 1.0.12 at best. Didn't check others so far. So isn't this step a bit too early? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [Bug 1233225] Re: mips/mipsel linux user float division problem
On 2 October 2013 14:22, Stefan Weil wrote:
> The original bug report said that it runs in QEMU system emulation
> (which I did not test because of lack of time). As system emulation
> uses the same cpu, it should be fine.
...that's what prompted me to ask, actually -- system emulation will
pick a CPU based on whichever board you're emulating, which is
quite likely to be a different one from the default picked by linux-user.
The original bug report doesn't seem to say which board they used
for system emulation so I don't know which CPU it would be using.
-- PMM
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1233225
Title:
mips/mipsel linux user float division problem
Status in QEMU:
Confirmed
Bug description:
Hi,
I tested the following with the qemu git HEAD as of 2013-09-30 on
Debian stable and testing. My host runs amd64 but I also tried this
out inside a i386 chroot with the same result. The problem occurs for
mips and mipsel. Given the following program:
#include
int main(int argc, char **argv)
{
int a = 1;
double d = a/2.0;
printf("%f\n", d);
return 0;
}
Instead of printing 0.5, it will print 2.0 if executed in qemu user
mode.
$ mipsel-linux-gnu-gcc mipstest.c
$ ~/qemu/mipsel-linux-user/qemu-mipsel ./a.out
2.0
Expecting this to be a problem with my cross compiler (gcc-4.4 from
emdebian) I ran a fully emulated debian squeeze environment inside
qemu. There, I compiled the same program natively with gcc and as
expected got 0.5 as the output. I also copied the cross compiled
binary inside the emulated environment and also got 0.5 when I ran it.
So the same mips/mipsel binary produces different output depending on
whether it is run in a fully emulated environment or qemu user mode.
Can anybody else reproduce this problem?
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1233225/+subscriptions
Re: [Qemu-devel] [PATCH v2 0/9] target-s390 tcg improvements
On 01.10.2013, at 19:17, Richard Henderson wrote: > Changes v1-v2: > * Squashed patches 1, 2, 4. The feedback from patch 1 called for a >re-ordering of the patch set to better deal with STFL, but I could >not find an ordering that worked well. I think this is probably >just as clear as 3 separate patches. > > * STFL and STIDP implementations adjusted for feedback. > > * Minor tweaks to SAM implementations. > > * Split out z9-109 hack to a separate patch. Acked-by: Alexander Graf Alex
Re: [Qemu-devel] [PATCH RFC v2 1/9] hw/core: Add interface to allocate and free a single IRQ
On Wed, Oct 02, 2013 at 03:41:26PM +0300, Marcel Apfelbaum wrote:
> qemu_allocate_irq returns a single qemu_irq.
> The interface allows to specify an interrupt number.
>
> qemu_free_irq frees it.
>
> Signed-off-by: Marcel Apfelbaum
> ---
> hw/core/irq.c| 16
> include/hw/irq.h | 7 +++
> 2 files changed, 23 insertions(+)
>
> diff --git a/hw/core/irq.c b/hw/core/irq.c
> index 2078542..03c8cb3 100644
> --- a/hw/core/irq.c
> +++ b/hw/core/irq.c
> @@ -68,6 +68,17 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler,
> void *opaque, int n)
> return qemu_extend_irqs(NULL, 0, handler, opaque, n);
> }
>
> +qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
> +{
> +struct IRQState *irq;
> +
> +irq = g_new(struct IRQState, 1);
> +irq->handler = handler;
> +irq->opaque = opaque;
> +irq->n = n;
> +
> +return irq;
> +}
>
> void qemu_free_irqs(qemu_irq *s)
> {
> @@ -75,6 +86,11 @@ void qemu_free_irqs(qemu_irq *s)
> g_free(s);
> }
>
> +void qemu_free_irq(qemu_irq irq)
> +{
> +g_free(irq);
> +}
> +
> static void qemu_notirq(void *opaque, int line, int level)
> {
> struct IRQState *irq = opaque;
> diff --git a/include/hw/irq.h b/include/hw/irq.h
> index 610e6b7..f560dea 100644
> --- a/include/hw/irq.h
> +++ b/include/hw/irq.h
> @@ -30,6 +30,12 @@ static inline void qemu_irq_pulse(qemu_irq irq)
> */
> qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n);
>
> +/*
> + * Allocates a single IRQ. The irq is assigned with a handler, an opaque
> + * data and the interrupt number
Add period at end of line :)
no need to repost for this.
> + */
> +qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n);
> +
> /* Extends an Array of IRQs. Old IRQs have their handlers and opaque data
> * preserved. New IRQs are assigned the argument handler and opaque data.
> */
> @@ -37,6 +43,7 @@ qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old,
> qemu_irq_handler handler,
> void *opaque, int n);
>
> void qemu_free_irqs(qemu_irq *s);
> +void qemu_free_irq(qemu_irq irq);
>
> /* Returns a new IRQ with opposite polarity. */
> qemu_irq qemu_irq_invert(qemu_irq irq);
> --
> 1.8.3.1
Re: [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin
On Wed, 2013-10-02 at 15:58 +0300, Michael S. Tsirkin wrote: > On Wed, Oct 02, 2013 at 03:41:25PM +0300, Marcel Apfelbaum wrote: > > Note: Added RFC because not all affected devices were > > checked yet. > > What do you have in mind exactly? Sanity test for *all* modified devices. Any other thoughts? >From self code review, I can say that besides 2 devices (vmxnet3 and vfio), the code is re-factored with no side effects. vmxnet3: I reviewed linux driver code and when using legacy interrupts only INTx 0 is used. vfio: Also here only INTx 0 is used, so the interface selecting the interrupt line seems to be unnecessary at least for legacy interrupts. > > > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN > > register during device initialization. Devices should not call > > directly qemu_set_irq and specify the INTx pin. > > > > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise, > > qemu_irq_lower and qemu_irq_pulse, setting the irq > > based on PCI_INTERRUPT_PIN. > > > > Added interface to allocate and free single irq. > > Added pci_allocate_irq wrapper to be used by devices that > > still need PCIDevice infrastructure to assert irqs. > > > > Removed irq field from PCIDevice, not needed anymore. > > > > Special cases of replacements were done in separate patches, > > all others in one patch "hw: set interrupts using pci irq wrappers" > > Looks good to me overall. > Acked-by: Michael S. Tsirkin > > > > Changes from v1: > > - Addressed Michael S. Tsirkin's comments: > >- pci_set_irq directly calls pci_irq handler > >- removed irq field from PCIDevice > > - Added qemu interface to allocate single irq > > - Added pci wrappers to allocate and free pci irq > > - Added pci irq wrappers for all qemu methods > >setting irq and not only qemu_set_irq > > - Replace all qemu irq setters with pci > >wrappers > > > > Marcel Apfelbaum (9): > > hw/core: Add interface to allocate and free a single IRQ > > hw/pci: add pci wrappers for allocating and asserting irqs > > hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init > > hw/vmxnet3: set interrupts using pci irq wrappers > > hw/vfio: set interrupts using pci irq wrappers > > hw/xhci: set interrupts using pci irq wrappers > > hw: set interrupts using pci irq wrappers > > hw/pcie: AER and hot-plug events must use device's interrupt > > hw/pci: removed irq field from PCIDevice > > > > hw/audio/ac97.c| 4 ++-- > > hw/audio/es1370.c | 4 ++-- > > hw/audio/intel-hda.c | 2 +- > > hw/block/nvme.c| 2 +- > > hw/char/serial-pci.c | 5 +++-- > > hw/char/tpci200.c | 8 > > hw/core/irq.c | 16 > > hw/display/qxl.c | 2 +- > > hw/ide/cmd646.c| 2 +- > > hw/ide/ich.c | 3 ++- > > hw/isa/vt82c686.c | 2 +- > > hw/misc/ivshmem.c | 2 +- > > hw/misc/vfio.c | 11 ++- > > hw/net/e1000.c | 2 +- > > hw/net/eepro100.c | 4 ++-- > > hw/net/ne2000.c| 3 ++- > > hw/net/pcnet-pci.c | 3 ++- > > hw/net/rtl8139.c | 2 +- > > hw/net/vmxnet3.c | 13 +++-- > > hw/pci-bridge/pci_bridge_dev.c | 2 +- > > hw/pci/pci.c | 26 +- > > hw/pci/pcie.c | 4 ++-- > > hw/pci/pcie_aer.c | 4 ++-- > > hw/pci/shpc.c | 2 +- > > hw/scsi/esp-pci.c | 3 ++- > > hw/scsi/lsi53c895a.c | 2 +- > > hw/scsi/megasas.c | 6 +++--- > > hw/scsi/vmw_pvscsi.c | 2 +- > > hw/usb/hcd-ehci-pci.c | 2 +- > > hw/usb/hcd-ohci.c | 2 +- > > hw/usb/hcd-uhci.c | 2 +- > > hw/usb/hcd-xhci.c | 7 ++- > > hw/virtio/virtio-pci.c | 4 ++-- > > include/hw/irq.h | 7 +++ > > include/hw/pci/pci.h | 22 +++--- > > include/hw/pci/pcie.h | 18 -- > > 36 files changed, 127 insertions(+), 78 deletions(-) > > > > -- > > 1.8.3.1
[Qemu-devel] [PATCH V9 03/11] quorum: Add quorum_aio_writev and its dependencies.
Signed-off-by: Benoit Canet
---
block/quorum.c | 123 +
1 file changed, 123 insertions(+)
diff --git a/block/quorum.c b/block/quorum.c
index 9557e61..b49e3c6 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -64,11 +64,134 @@ struct QuorumAIOCB {
int vote_ret;
};
+static void quorum_aio_cancel(BlockDriverAIOCB *blockacb)
+{
+QuorumAIOCB *acb = container_of(blockacb, QuorumAIOCB, common);
+bool finished = false;
+
+/* Wait for the request to finish */
+acb->finished = &finished;
+while (!finished) {
+qemu_aio_wait();
+}
+}
+
+static AIOCBInfo quorum_aiocb_info = {
+.aiocb_size = sizeof(QuorumAIOCB),
+.cancel = quorum_aio_cancel,
+};
+
+/* return the first error code get by each individual callbacks */
+static int quorum_get_first_error(QuorumAIOCB *acb)
+{
+BDRVQuorumState *s = acb->bqs;
+int i, ret = 0;
+
+for (i = 0; i < s->total; i++) {
+ret = acb->aios[i].ret;
+if (ret) {
+return ret;
+}
+}
+
+/* should not pass here */
+assert(false);
+}
+
+static void quorum_aio_finalize(QuorumAIOCB *acb)
+{
+BDRVQuorumState *s = acb->bqs;
+int ret;
+
+ret = s->threshold <= acb->success_count ? 0 : quorum_get_first_error(acb);
+
+acb->common.cb(acb->common.opaque, ret);
+if (acb->finished) {
+*acb->finished = true;
+}
+g_free(acb->aios);
+qemu_aio_release(acb);
+}
+
+static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s,
+ BlockDriverState *bs,
+ QEMUIOVector *qiov,
+ uint64_t sector_num,
+ int nb_sectors,
+ BlockDriverCompletionFunc *cb,
+ void *opaque)
+{
+QuorumAIOCB *acb = qemu_aio_get(&quorum_aiocb_info, bs, cb, opaque);
+int i;
+
+acb->bqs = s;
+acb->sector_num = sector_num;
+acb->nb_sectors = nb_sectors;
+acb->qiov = qiov;
+acb->aios = g_new0(QuorumSingleAIOCB, s->total);
+acb->count = 0;
+acb->success_count = 0;
+acb->finished = NULL;
+acb->is_read = false;
+acb->vote_ret = 0;
+
+for (i = 0; i < s->total; i++) {
+acb->aios[i].buf = NULL;
+acb->aios[i].ret = 0;
+acb->aios[i].parent = acb;
+}
+
+return acb;
+}
+
+static void quorum_aio_cb(void *opaque, int ret)
+{
+QuorumSingleAIOCB *sacb = opaque;
+QuorumAIOCB *acb = sacb->parent;
+BDRVQuorumState *s = acb->bqs;
+
+sacb->ret = ret;
+acb->count++;
+if (ret == 0) {
+acb->success_count++;
+}
+assert(acb->count <= s->total);
+assert(acb->success_count <= s->total);
+if (acb->count < s->total) {
+return;
+}
+
+quorum_aio_finalize(acb);
+}
+
+static BlockDriverAIOCB *quorum_aio_writev(BlockDriverState *bs,
+ int64_t sector_num,
+ QEMUIOVector *qiov,
+ int nb_sectors,
+ BlockDriverCompletionFunc *cb,
+ void *opaque)
+{
+BDRVQuorumState *s = bs->opaque;
+QuorumAIOCB *acb = quorum_aio_get(s, bs, qiov, sector_num, nb_sectors,
+ cb, opaque);
+int i;
+
+for (i = 0; i < s->total; i++) {
+acb->aios[i].aiocb = bdrv_aio_writev(&s->bs[i], sector_num, qiov,
+ nb_sectors, &quorum_aio_cb,
+ &acb->aios[i]);
+}
+
+return &acb->common;
+}
+
static BlockDriver bdrv_quorum = {
.format_name= "quorum",
.protocol_name = "quorum",
.instance_size = sizeof(BDRVQuorumState),
+
+.bdrv_aio_writev= quorum_aio_writev,
};
static void bdrv_quorum_init(void)
--
1.8.1.2
Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs
On Wed, 2013-10-02 at 15:54 +0300, Michael S. Tsirkin wrote:
> On Wed, Oct 02, 2013 at 03:41:27PM +0300, Marcel Apfelbaum wrote:
> > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> > register during device initialization. Devices should not call
> > directly qemu_set_irq and specify the INTx pin on each call.
> >
> > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> > qemu_irq_lower and qemu_irq_pulse, setting the irq
> > based on PCI_INTERRUPT_PIN.
> >
> > Added pci_allocate_irq wrapper to be used by devices that
> > still need PCIDevice infrastructure to assert irqs.
> >
> > Renamed a static method which was named already pci_set_irq.
> >
> > Signed-off-by: Marcel Apfelbaum
> > ---
> > Changes from v1:
> > - Added pci irq wrappers for all qemu methods
> >setting irq and not only qemu_set_irq
> > - Added pci wrappers to allocate and pci irq
> >
> > hw/pci/pci.c | 26 ++
> > include/hw/pci/pci.h | 19 +++
> > 2 files changed, 41 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 00554a0..fbfd8f7 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -83,7 +83,7 @@ static const TypeInfo pcie_bus_info = {
> >
> > static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
> > static void pci_update_mappings(PCIDevice *d);
> > -static void pci_set_irq(void *opaque, int irq_num, int level);
> > +static void pci_irq_handler(void *opaque, int irq_num, int level);
> > static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
> > static void pci_del_option_rom(PCIDevice *pdev);
> >
> > @@ -161,7 +161,7 @@ void pci_device_deassert_intx(PCIDevice *dev)
> > {
> > int i;
> > for (i = 0; i < PCI_NUM_PINS; ++i) {
> > -qemu_set_irq(dev->irq[i], 0);
> > +pci_irq_handler(dev, i, 0);
> > }
> > }
> >
> > @@ -863,7 +863,7 @@ static PCIDevice *do_pci_register_device(PCIDevice
> > *pci_dev, PCIBus *bus,
> > pci_dev->config_read = config_read;
> > pci_dev->config_write = config_write;
> > bus->devices[devfn] = pci_dev;
> > -pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
> > +pci_dev->irq = qemu_allocate_irqs(pci_irq_handler, pci_dev,
> > PCI_NUM_PINS);
> > pci_dev->version_id = 2; /* Current pci device vmstate version */
> > return pci_dev;
> > }
> > @@ -1175,7 +1175,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t
> > addr, uint32_t val, int l)
> > /* generic PCI irq support */
> >
> > /* 0 <= irq_num <= 3. level must be 0 or 1 */
> > -static void pci_set_irq(void *opaque, int irq_num, int level)
> > +static void pci_irq_handler(void *opaque, int irq_num, int level)
> > {
> > PCIDevice *pci_dev = opaque;
> > int change;
> > @@ -1191,6 +1191,24 @@ static void pci_set_irq(void *opaque, int irq_num,
> > int level)
> > pci_change_irq_level(pci_dev, irq_num, change);
> > }
> >
> > +static inline int pci_intx(PCIDevice *pci_dev)
> > +{
> > +return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
> > +}
> > +
> > +qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
> > +{
> > +int intx = pci_intx(pci_dev);
> > +
> > +return qemu_allocate_irq(pci_irq_handler, pci_dev, intx);
> > +}
> > +
> > +void pci_set_irq(PCIDevice *pci_dev, int level)
> > +{
> > +int intx = pci_intx(pci_dev);
> > +pci_irq_handler(pci_dev, intx, level);
> > +}
> > +
> > /* Special hooks used by device assignment */
> > void pci_bus_set_route_irq_fn(PCIBus *bus, pci_route_irq_fn
> > route_intx_to_irq)
> > {
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 4b90e5d..df7d316 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -632,6 +632,25 @@ PCIDevice *pci_create_simple_multifunction(PCIBus
> > *bus, int devfn,
> > PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
> > PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
> >
> > +qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
> > +void pci_set_irq(PCIDevice *pci_dev, int level);
> > +
> > +static inline void pci_irq_raise(PCIDevice *pci_dev)
> > +{
> > +pci_set_irq(pci_dev, 1);
> > +}
> > +
> > +static inline void pci_irq_lower(PCIDevice *pci_dev)
> > +{
> > +pci_set_irq(pci_dev, 0);
> > +}
> > +
>
> I'd like to add that any users of pci_irq_pulse
> are immediately suspect. PCI does not work this way.
> If you resend this, maybe add a comment that all users
> of this should be fixed.
Sure, thanks!
Marcel
>
> > +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> > +{
> > +pci_irq_lower(pci_dev);
> > +pci_irq_raise(pci_dev);
> > +}
> > +
> > static inline int pci_is_express(const PCIDevice *d)
> > {
> > return d->cap_present & QEMU_PCI_CAP_EXPRESS;
> > --
> > 1.8.3.1
[Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin
Note: Added RFC because not all affected devices were checked yet. Interrupt pin is selected and saved into PCI_INTERRUPT_PIN register during device initialization. Devices should not call directly qemu_set_irq and specify the INTx pin. Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise, qemu_irq_lower and qemu_irq_pulse, setting the irq based on PCI_INTERRUPT_PIN. Added interface to allocate and free single irq. Added pci_allocate_irq wrapper to be used by devices that still need PCIDevice infrastructure to assert irqs. Removed irq field from PCIDevice, not needed anymore. Special cases of replacements were done in separate patches, all others in one patch "hw: set interrupts using pci irq wrappers" Changes from v1: - Addressed Michael S. Tsirkin's comments: - pci_set_irq directly calls pci_irq handler - removed irq field from PCIDevice - Added qemu interface to allocate single irq - Added pci wrappers to allocate and free pci irq - Added pci irq wrappers for all qemu methods setting irq and not only qemu_set_irq - Replace all qemu irq setters with pci wrappers Marcel Apfelbaum (9): hw/core: Add interface to allocate and free a single IRQ hw/pci: add pci wrappers for allocating and asserting irqs hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init hw/vmxnet3: set interrupts using pci irq wrappers hw/vfio: set interrupts using pci irq wrappers hw/xhci: set interrupts using pci irq wrappers hw: set interrupts using pci irq wrappers hw/pcie: AER and hot-plug events must use device's interrupt hw/pci: removed irq field from PCIDevice hw/audio/ac97.c| 4 ++-- hw/audio/es1370.c | 4 ++-- hw/audio/intel-hda.c | 2 +- hw/block/nvme.c| 2 +- hw/char/serial-pci.c | 5 +++-- hw/char/tpci200.c | 8 hw/core/irq.c | 16 hw/display/qxl.c | 2 +- hw/ide/cmd646.c| 2 +- hw/ide/ich.c | 3 ++- hw/isa/vt82c686.c | 2 +- hw/misc/ivshmem.c | 2 +- hw/misc/vfio.c | 11 ++- hw/net/e1000.c | 2 +- hw/net/eepro100.c | 4 ++-- hw/net/ne2000.c| 3 ++- hw/net/pcnet-pci.c | 3 ++- hw/net/rtl8139.c | 2 +- hw/net/vmxnet3.c | 13 +++-- hw/pci-bridge/pci_bridge_dev.c | 2 +- hw/pci/pci.c | 26 +- hw/pci/pcie.c | 4 ++-- hw/pci/pcie_aer.c | 4 ++-- hw/pci/shpc.c | 2 +- hw/scsi/esp-pci.c | 3 ++- hw/scsi/lsi53c895a.c | 2 +- hw/scsi/megasas.c | 6 +++--- hw/scsi/vmw_pvscsi.c | 2 +- hw/usb/hcd-ehci-pci.c | 2 +- hw/usb/hcd-ohci.c | 2 +- hw/usb/hcd-uhci.c | 2 +- hw/usb/hcd-xhci.c | 7 ++- hw/virtio/virtio-pci.c | 4 ++-- include/hw/irq.h | 7 +++ include/hw/pci/pci.h | 22 +++--- include/hw/pci/pcie.h | 18 -- 36 files changed, 127 insertions(+), 78 deletions(-) -- 1.8.3.1
Re: [Qemu-devel] [PATCH] block/qcow2: Use bdrv_truncate for size amend
On 2013-10-02 14:32, Stefan Hajnoczi wrote:
On Mon, Sep 09, 2013 at 12:17:41PM +0200, Max Reitz wrote:
When amending the size option for a qcow2 image, use bdrv_truncate
instead of qcow2_truncate directly, since the latter will not adjust the
total_sectors count in the BDS structure (whereas the former will).
Signed-off-by: Max Reitz
---
Depends on (follow-up to):
- block/qcow2: Image file option amendment (series, v5)
---
block/qcow2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index d29547b..259edfd 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1997,7 +1997,7 @@ static int qcow2_amend_options(BlockDriverState *bs,
}
if (new_size) {
-ret = qcow2_truncate(bs, new_size);
+ret = bdrv_truncate(bs, new_size);
if (ret < 0) {
return ret;
}
It seems this patch was already squashed into
9296b3ed7050cc6e0645fbc3b0aea74406d7eeb2 ("qcow2: Implement
bdrv_amend_options").
Yes, as Kevin stated in his reply to that series:
Thanks, applied to the block branch with the following changes:
- Updated patch 5 with
[PATCH] block/qcow2: Use bdrv_truncate for size amend
Max
[Qemu-devel] [PATCH RFC v2 5/9] hw/vfio: set interrupts using pci irq wrappers
pci_set_irq and the other pci irq wrappers use
PCI_INTERRUPT_PIN config register to compute device
INTx pin to assert/deassert.
Save INTx pin into the config register before calling
pci_set_irq
Signed-off-by: Marcel Apfelbaum
---
hw/misc/vfio.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index a1c08fb..3d7297c 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -297,7 +297,7 @@ static void vfio_intx_interrupt(void *opaque)
'A' + vdev->intx.pin);
vdev->intx.pending = true;
-qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 1);
+pci_set_irq(&vdev->pdev, 1);
vfio_mmap_set_enabled(vdev, false);
if (vdev->intx.mmap_timeout) {
timer_mod(vdev->intx.mmap_timer,
@@ -315,7 +315,7 @@ static void vfio_eoi(VFIODevice *vdev)
vdev->host.bus, vdev->host.slot, vdev->host.function);
vdev->intx.pending = false;
-qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+pci_set_irq(&vdev->pdev, 0);
vfio_unmask_intx(vdev);
}
@@ -341,7 +341,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev)
qemu_set_fd_handler(irqfd.fd, NULL, NULL, vdev);
vfio_mask_intx(vdev);
vdev->intx.pending = false;
-qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+pci_set_irq(&vdev->pdev, 0);
/* Get an eventfd for resample/unmask */
if (event_notifier_init(&vdev->intx.unmask, 0)) {
@@ -417,7 +417,7 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev)
*/
vfio_mask_intx(vdev);
vdev->intx.pending = false;
-qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+pci_set_irq(&vdev->pdev, 0);
/* Tell KVM to stop listening for an INTx irqfd */
if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) {
@@ -488,6 +488,7 @@ static int vfio_enable_intx(VFIODevice *vdev)
vfio_disable_interrupts(vdev);
vdev->intx.pin = pin - 1; /* Pin A (1) -> irq[0] */
+pci_config_set_interrupt_pin(vdev->pdev.config, pin);
#ifdef CONFIG_KVM
/*
@@ -547,7 +548,7 @@ static void vfio_disable_intx(VFIODevice *vdev)
vfio_disable_intx_kvm(vdev);
vfio_disable_irqindex(vdev, VFIO_PCI_INTX_IRQ_INDEX);
vdev->intx.pending = false;
-qemu_set_irq(vdev->pdev.irq[vdev->intx.pin], 0);
+pci_set_irq(&vdev->pdev, 0);
vfio_mmap_set_enabled(vdev, true);
fd = event_notifier_get_fd(&vdev->intx.interrupt);
--
1.8.3.1
[Qemu-devel] [PATCH RFC v2 9/9] hw/pci: removed irq field from PCIDevice
Instead of exposing the the irq field,
pci wrappers to qemu_set_irq or qemu_irq_*
can be used.
Signed-off-by: Marcel Apfelbaum
---
hw/pci/pci.c | 2 --
include/hw/pci/pci.h | 3 ---
2 files changed, 5 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index fbfd8f7..f8d0b81 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -863,14 +863,12 @@ static PCIDevice *do_pci_register_device(PCIDevice
*pci_dev, PCIBus *bus,
pci_dev->config_read = config_read;
pci_dev->config_write = config_write;
bus->devices[devfn] = pci_dev;
-pci_dev->irq = qemu_allocate_irqs(pci_irq_handler, pci_dev, PCI_NUM_PINS);
pci_dev->version_id = 2; /* Current pci device vmstate version */
return pci_dev;
}
static void do_pci_unregister_device(PCIDevice *pci_dev)
{
-qemu_free_irqs(pci_dev->irq);
pci_dev->bus->devices[pci_dev->devfn] = NULL;
pci_config_free(pci_dev);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index df7d316..e9346bd 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -247,9 +247,6 @@ struct PCIDevice {
PCIConfigReadFunc *config_read;
PCIConfigWriteFunc *config_write;
-/* IRQ objects for the INTA-INTD pins. */
-qemu_irq *irq;
-
/* Legacy PCI VGA regions */
MemoryRegion *vga_regions[QEMU_PCI_VGA_NUM_REGIONS];
bool has_vga;
--
1.8.3.1
Re: [Qemu-devel] [PATCH RFC v2 0/9] hw/pci: set irq without selecting INTx pin
On Wed, Oct 02, 2013 at 03:41:25PM +0300, Marcel Apfelbaum wrote: > Note: Added RFC because not all affected devices were > checked yet. What do you have in mind exactly? > Interrupt pin is selected and saved into PCI_INTERRUPT_PIN > register during device initialization. Devices should not call > directly qemu_set_irq and specify the INTx pin. > > Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise, > qemu_irq_lower and qemu_irq_pulse, setting the irq > based on PCI_INTERRUPT_PIN. > > Added interface to allocate and free single irq. > Added pci_allocate_irq wrapper to be used by devices that > still need PCIDevice infrastructure to assert irqs. > > Removed irq field from PCIDevice, not needed anymore. > > Special cases of replacements were done in separate patches, > all others in one patch "hw: set interrupts using pci irq wrappers" Looks good to me overall. Acked-by: Michael S. Tsirkin > Changes from v1: > - Addressed Michael S. Tsirkin's comments: >- pci_set_irq directly calls pci_irq handler >- removed irq field from PCIDevice > - Added qemu interface to allocate single irq > - Added pci wrappers to allocate and free pci irq > - Added pci irq wrappers for all qemu methods >setting irq and not only qemu_set_irq > - Replace all qemu irq setters with pci >wrappers > > Marcel Apfelbaum (9): > hw/core: Add interface to allocate and free a single IRQ > hw/pci: add pci wrappers for allocating and asserting irqs > hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init > hw/vmxnet3: set interrupts using pci irq wrappers > hw/vfio: set interrupts using pci irq wrappers > hw/xhci: set interrupts using pci irq wrappers > hw: set interrupts using pci irq wrappers > hw/pcie: AER and hot-plug events must use device's interrupt > hw/pci: removed irq field from PCIDevice > > hw/audio/ac97.c| 4 ++-- > hw/audio/es1370.c | 4 ++-- > hw/audio/intel-hda.c | 2 +- > hw/block/nvme.c| 2 +- > hw/char/serial-pci.c | 5 +++-- > hw/char/tpci200.c | 8 > hw/core/irq.c | 16 > hw/display/qxl.c | 2 +- > hw/ide/cmd646.c| 2 +- > hw/ide/ich.c | 3 ++- > hw/isa/vt82c686.c | 2 +- > hw/misc/ivshmem.c | 2 +- > hw/misc/vfio.c | 11 ++- > hw/net/e1000.c | 2 +- > hw/net/eepro100.c | 4 ++-- > hw/net/ne2000.c| 3 ++- > hw/net/pcnet-pci.c | 3 ++- > hw/net/rtl8139.c | 2 +- > hw/net/vmxnet3.c | 13 +++-- > hw/pci-bridge/pci_bridge_dev.c | 2 +- > hw/pci/pci.c | 26 +- > hw/pci/pcie.c | 4 ++-- > hw/pci/pcie_aer.c | 4 ++-- > hw/pci/shpc.c | 2 +- > hw/scsi/esp-pci.c | 3 ++- > hw/scsi/lsi53c895a.c | 2 +- > hw/scsi/megasas.c | 6 +++--- > hw/scsi/vmw_pvscsi.c | 2 +- > hw/usb/hcd-ehci-pci.c | 2 +- > hw/usb/hcd-ohci.c | 2 +- > hw/usb/hcd-uhci.c | 2 +- > hw/usb/hcd-xhci.c | 7 ++- > hw/virtio/virtio-pci.c | 4 ++-- > include/hw/irq.h | 7 +++ > include/hw/pci/pci.h | 22 +++--- > include/hw/pci/pcie.h | 18 -- > 36 files changed, 127 insertions(+), 78 deletions(-) > > -- > 1.8.3.1
Re: [Qemu-devel] [PATCH RFC v2 2/9] hw/pci: add pci wrappers for allocating and asserting irqs
On Wed, Oct 02, 2013 at 03:41:27PM +0300, Marcel Apfelbaum wrote:
> Interrupt pin is selected and saved into PCI_INTERRUPT_PIN
> register during device initialization. Devices should not call
> directly qemu_set_irq and specify the INTx pin on each call.
>
> Added pci_* wrappers to replace qemu_set_irq, qemu_irq_raise,
> qemu_irq_lower and qemu_irq_pulse, setting the irq
> based on PCI_INTERRUPT_PIN.
>
> Added pci_allocate_irq wrapper to be used by devices that
> still need PCIDevice infrastructure to assert irqs.
>
> Renamed a static method which was named already pci_set_irq.
>
> Signed-off-by: Marcel Apfelbaum
> ---
> Changes from v1:
> - Added pci irq wrappers for all qemu methods
>setting irq and not only qemu_set_irq
> - Added pci wrappers to allocate and pci irq
>
> hw/pci/pci.c | 26 ++
> include/hw/pci/pci.h | 19 +++
> 2 files changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 00554a0..fbfd8f7 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -83,7 +83,7 @@ static const TypeInfo pcie_bus_info = {
>
> static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
> static void pci_update_mappings(PCIDevice *d);
> -static void pci_set_irq(void *opaque, int irq_num, int level);
> +static void pci_irq_handler(void *opaque, int irq_num, int level);
> static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom);
> static void pci_del_option_rom(PCIDevice *pdev);
>
> @@ -161,7 +161,7 @@ void pci_device_deassert_intx(PCIDevice *dev)
> {
> int i;
> for (i = 0; i < PCI_NUM_PINS; ++i) {
> -qemu_set_irq(dev->irq[i], 0);
> +pci_irq_handler(dev, i, 0);
> }
> }
>
> @@ -863,7 +863,7 @@ static PCIDevice *do_pci_register_device(PCIDevice
> *pci_dev, PCIBus *bus,
> pci_dev->config_read = config_read;
> pci_dev->config_write = config_write;
> bus->devices[devfn] = pci_dev;
> -pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
> +pci_dev->irq = qemu_allocate_irqs(pci_irq_handler, pci_dev,
> PCI_NUM_PINS);
> pci_dev->version_id = 2; /* Current pci device vmstate version */
> return pci_dev;
> }
> @@ -1175,7 +1175,7 @@ void pci_default_write_config(PCIDevice *d, uint32_t
> addr, uint32_t val, int l)
> /* generic PCI irq support */
>
> /* 0 <= irq_num <= 3. level must be 0 or 1 */
> -static void pci_set_irq(void *opaque, int irq_num, int level)
> +static void pci_irq_handler(void *opaque, int irq_num, int level)
> {
> PCIDevice *pci_dev = opaque;
> int change;
> @@ -1191,6 +1191,24 @@ static void pci_set_irq(void *opaque, int irq_num, int
> level)
> pci_change_irq_level(pci_dev, irq_num, change);
> }
>
> +static inline int pci_intx(PCIDevice *pci_dev)
> +{
> +return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
> +}
> +
> +qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
> +{
> +int intx = pci_intx(pci_dev);
> +
> +return qemu_allocate_irq(pci_irq_handler, pci_dev, intx);
> +}
> +
> +void pci_set_irq(PCIDevice *pci_dev, int level)
> +{
> +int intx = pci_intx(pci_dev);
> +pci_irq_handler(pci_dev, intx, level);
> +}
> +
> /* Special hooks used by device assignment */
> void pci_bus_set_route_irq_fn(PCIBus *bus, pci_route_irq_fn
> route_intx_to_irq)
> {
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 4b90e5d..df7d316 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -632,6 +632,25 @@ PCIDevice *pci_create_simple_multifunction(PCIBus *bus,
> int devfn,
> PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name);
> PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
>
> +qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
> +void pci_set_irq(PCIDevice *pci_dev, int level);
> +
> +static inline void pci_irq_raise(PCIDevice *pci_dev)
> +{
> +pci_set_irq(pci_dev, 1);
> +}
> +
> +static inline void pci_irq_lower(PCIDevice *pci_dev)
> +{
> +pci_set_irq(pci_dev, 0);
> +}
> +
I'd like to add that any users of pci_irq_pulse
are immediately suspect. PCI does not work this way.
If you resend this, maybe add a comment that all users
of this should be fixed.
> +static inline void pci_irq_pulse(PCIDevice *pci_dev)
> +{
> +pci_irq_lower(pci_dev);
> +pci_irq_raise(pci_dev);
> +}
> +
> static inline int pci_is_express(const PCIDevice *d)
> {
> return d->cap_present & QEMU_PCI_CAP_EXPRESS;
> --
> 1.8.3.1
[Qemu-devel] [PATCH RFC v2 8/9] hw/pcie: AER and hot-plug events must use device's interrupt
The fields hpev_intx and aer_intx were removed because
both AER and hot-plug events must use device's interrupt.
Assert/deassert interrupts using pci_set_irq wrapper instead.
Signed-off-by: Marcel Apfelbaum
---
hw/pci/pcie.c | 4 ++--
hw/pci/pcie_aer.c | 4 ++--
include/hw/pci/pcie.h | 18 --
3 files changed, 4 insertions(+), 22 deletions(-)
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 50af3c1..d4dd005 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -187,7 +187,7 @@ static void hotplug_event_notify(PCIDevice *dev)
} else if (msi_enabled(dev)) {
msi_notify(dev, pcie_cap_flags_get_vector(dev));
} else {
-qemu_set_irq(dev->irq[dev->exp.hpev_intx], dev->exp.hpev_notified);
+pci_set_irq(dev, dev->exp.hpev_notified);
}
}
@@ -195,7 +195,7 @@ static void hotplug_event_clear(PCIDevice *dev)
{
hotplug_event_update_event_status(dev);
if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) {
-qemu_set_irq(dev->irq[dev->exp.hpev_intx], 0);
+pci_set_irq(dev, 0);
}
}
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index ca762ab..03b1e1f 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -285,7 +285,7 @@ static void pcie_aer_root_notify(PCIDevice *dev)
} else if (msi_enabled(dev)) {
msi_notify(dev, pcie_aer_root_get_vector(dev));
} else {
-qemu_set_irq(dev->irq[dev->exp.aer_intx], 1);
+pci_set_irq(dev, 1);
}
}
@@ -768,7 +768,7 @@ void pcie_aer_root_write_config(PCIDevice *dev,
uint32_t root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
/* 6.2.4.1.2 Interrupt Generation */
if (!msix_enabled(dev) && !msi_enabled(dev)) {
-qemu_set_irq(dev->irq[dev->exp.aer_intx], !!(root_cmd & enabled_cmd));
+pci_set_irq(dev, !!(root_cmd & enabled_cmd));
return;
}
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index c010007..1966169 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -64,15 +64,6 @@ struct PCIExpressDevice {
uint8_t exp_cap;
/* SLOT */
-unsigned int hpev_intx; /* INTx for hot plug event (0-3:INT[A-D]#)
- * default is 0 = INTA#
- * If the chip wants to use other interrupt
- * line, initialize this member with the
- * desired number.
- * If the chip dynamically changes this member,
- * also initialize it when loaded as
- * appropreately.
- */
bool hpev_notified; /* Logical AND of conditions for hot plug event.
Following 6.7.3.4:
Software Notification of Hot-Plug Events, an interrupt
@@ -82,15 +73,6 @@ struct PCIExpressDevice {
/* AER */
uint16_t aer_cap;
PCIEAERLog aer_log;
-unsigned int aer_intx; /* INTx for error reporting
- * default is 0 = INTA#
- * If the chip wants to use other interrupt
- * line, initialize this member with the
- * desired number.
- * If the chip dynamically changes this member,
- * also initialize it when loaded as
- * appropreately.
- */
};
/* PCI express capability helper functions */
--
1.8.3.1
