Re: [Qemu-devel] BUG: RTC issue when Windows guest is idle
Hi Alex, I've been doing some testing with the latest git version and so far I haven't seen a guest freeze in the same circumstances as before. A weird thing that has been happening is the RTC timer stopping after a live migration. This happened in both 1.6.1 and the 1.6.50 git build. To replicate the issue I was migrating to/from the same machine and anywhere between 1 and 3 migrations the guest clock would stop. Connecting to the VNC console would not get it running again. I've tried to replicate the issue with the HPET enabled but the guest clock works flawlessly with it enabled. Guest VM is Windows 2008R2. Host is Ubuntu 13.04 (kernel 3.8.0-25-generic) Command line - /usr/local/bin/qemu-system-x86_64 -enable-kvm -nodefconfig -nodefaults -daemonize -usb -chardev socket,id=charmonitor,path=/var/run/based1/monitor/525ce3d009c437d67802.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -chardev socket,path=/var/run/based1/ga/525ce3d009c437d67802.guestagent,server,nowait,id=qga0 -device virtio-serial -device virtserialport,chardev=qga0,name=org.qemu.guest_agent.0 -pidfile /var/run/based1/pid/525ce3d009c437d67802.pid -vga cirrus -vnc 0.0.0.0:1 -M pc-i440fx-1.6 -m 1024 -smp sockets=1,cores=1,threads=1 -cpu qemu64,+vme,+dts,+acpi,+dtes64,+vmx,+smx,+ssse3,+sse4_1,+sse4_2,+tpr_shadow,+vnmi,+flexpriority,+ept,+vpid,hv_relaxed,hv_spinlocks=0x,hv_vapic -rtc base=utc,driftfix=slew --no-hpet -drive aio=native,file=rbd:sata/525ce3d009c437d67803,if=virtio,id=disk-525ce3d009c437d67805,format=raw,cache=writeback,media=disk,index=0,addr=0xa -netdev tap,id=netdev-5264b6d46e53c81719000236,vhost=off,ifname=tap2,script=no,downscript=no -device virtio-net-pci,netdev=netdev-5264b6d46e53c81719000236,id=interface-5264b6d46e53c81719000236,mac=9a:a5:63:64:6f:76,bus=pci.0,addr=0xb -incoming tcp:0:3004 Thanks -Matt -Original Message- From: Alex Bligh [mailto:a...@alex.org.uk] Sent: Tuesday, 22 October 2013 5:36 PM To: Xiexiangyou; Matthew Anderson Cc: Stefan Hajnoczi; qemu-devel@nongnu.org; Alex Bligh Subject: Re: [Qemu-devel] BUG: RTC issue when Windows guest is idle --On 22 October 2013 08:28:08 + Xiexiangyou wrote: > Hi: > > I have run windows2008r2 guest with qemu-1.5.1/1.6(I have not test the > the newer version) for long time, the issue (guest in hangup state) > will come out.When guest in hangup state, QEMU main thread is blocked > in g_poll loop. >> 398c398,399 >> < uint32_t timeout = UINT32_MAX; >> --- >>> /* uint32_t timeout = UINT32_MAX; */ >>> uint32_t timeout = 1000; >> > It seems can fix the problem, and rtc/hpet interrupt can inject into > guest again because of the "timeout", and guest will wake up . But > maybe the issue is also exist, because during the time before timeout > , guest also will lose rtc/hpet ticks. I do not think that is the correct fix for 1.5.1/1.6; what you are basically doing is limiting the wait in the mainloop to one second (1.5.1/1.6 are in milliseconds); however, I believe there may be other code that looks for infinite timeouts. Either there is some other bug that this is masking (in which case it may or may not be fixed in master / 1.7), or its a bug in the timer stuff in 1.5.1/1.6 (which would not surprise me) which is likely to have been fixed in master / 1.7. -- Alex Bligh
Re: [Qemu-devel] [PATCH] net/hub: remove can_receive handler
On 10/21/2013 03:52 PM, Fedorov Sergey wrote: On 10/21/2013 03:44 PM, Fedorov Sergey wrote: On 04/23/2013 04:00 PM, Stefan Hajnoczi wrote: On Tue, Apr 23, 2013 at 11:41:42AM +0400, Fedorov Sergey wrote: Beyond that, we also want to avoid growing net queues indefinitely. If the hub does not implement .can_receive() then it relies on growing queues (keeping packets buffered in memory). No, net_hub_receive() calls qemu_send_packet(). If the destination queue cannot receive the packet qemu_net_queue_append() will take care of queue->nq_maxlen. You are right, sorry. We do discard packets at nq_maxlen. The problem with ignoring .can_receive() on the hub is that it breaks flow control. For example, net/tap.c is designed to avoid reading more packets if its peer cannot receive (see tap_can_send()). If the hub claims it can always receive we waste cycles reading packets from the tap device only to discard them. Since qemu.git already has a fix which preserves flow control, I am not going to merge your patch. Stefan Dear, Stefan Hajnoczi, After our discussion about this patch I decided to keep my patch in our branch until rebase onto a new release. Recently I have rebased our branch onto v1.5.3 and reverted my patch. Then I face an issue when using user-mode networking with USB network device for mounting root file system through NFS. Fragmented UDP packets from host to guest does not handled properly. Seems that some fragments is lost or somehow stalled. See guest tcpdump log below. 03:16:52.259690 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto UDP (17), length 164) 10.0.2.15.3369105030 > 10.0.2.2.nfs: 136 readdirplus fh Unknown/01000700040012002873593C9B3C43388E23748B0BAD870C 512 bytes @ 0 max 4096 verf 03:16:52.262323 IP (tos 0x0, ttl 64, id 16, offset 0, flags [+], proto UDP (17), length 1500) 10.0.2.2.nfs > 10.0.2.15.3369105030: reply ok 1472 readdirplus POST: DIR 40777 ids 0/0 sz 4096 verf 03:16:52.264592 IP (tos 0x0, ttl 64, id 16, offset 1480, flags [+], proto UDP (17), length 1500) 10.0.2.2 > 10.0.2.15: udp 03:16:54.462961 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto UDP (17), length 164) 10.0.2.15.3369105030 > 10.0.2.2.nfs: 136 readdirplus fh Unknown/01000700040012002873593C9B3C43388E23748B0BAD870C 512 bytes @ 0 max 4096 verf 03:16:54.466300 IP (tos 0x0, ttl 64, id 17, offset 0, flags [+], proto UDP (17), length 1500) 10.0.2.2.nfs > 10.0.2.15.3369105030: reply ok 1472 readdirplus POST: DIR 40777 ids 0/0 sz 4096 verf 03:16:54.467084 IP (tos 0x0, ttl 64, id 17, offset 1480, flags [+], proto UDP (17), length 1500) 10.0.2.2 > 10.0.2.15: udp ... I didn't investigate the cause of the problem in detail. I just reverted commit 199ee608f0d08510b5c6c37f31a7fbff211d63c4 Author: Luigi Rizzo Date: Tue Feb 5 17:53:31 2013 +0100 net: fix qemu_flush_queued_packets() in presence of a hub And then applied my patch. After that everything works fine for me. See guest tcpdump log below. 04:45:15.897245 IP (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto UDP (17), length 164) 10.0.2.15.3642011847 > 10.0.2.2.nfs: 136 readdirplus fh Unknown/01000700040012002873593C9B3C43388E23748B0BAD870C 512 bytes @ 0 max 4096 verf 04:45:15.899686 IP (tos 0x0, ttl 64, id 15, offset 0, flags [+], proto UDP (17), length 1500) 10.0.2.2.nfs > 10.0.2.15.3642011847: reply ok 1472 readdirplus POST: DIR 40777 ids 0/0 sz 4096 verf 04:45:15.906253 IP (tos 0x0, ttl 64, id 15, offset 1480, flags [+], proto UDP (17), length 1500) 10.0.2.2 > 10.0.2.15: udp 04:45:15.906687 IP (tos 0x0, ttl 64, id 15, offset 2960, flags [none], proto UDP (17), length 240) 10.0.2.2 > 10.0.2.15: udp So there must be something wrong with already applied patch. What could you suggest? Sorry, I missed that Anthony Liguori email address has been changed. So I resend the email. Ping. -- Best regards, Sergey Fedorov, Junior Software Engineer, Samsung R&D Institute Rus. E-mail: s.fedo...@samsung.com
Re: [Qemu-devel] BUG: RTC issue when Windows guest is idle
On 28 Oct 2013, at 06:58, Matthew Anderson wrote: > I've been doing some testing with the latest git version and so far I haven't > seen a guest freeze in the same circumstances as before. That's good news. > A weird thing that has been happening is the RTC timer stopping after a live > migration. This happened in both 1.6.1 and the 1.6.50 git build. To replicate > the issue I was migrating to/from the same machine and anywhere between 1 and > 3 migrations the guest clock would stop. Connecting to the VNC console would > not get it running again. I've tried to replicate the issue with the HPET > enabled but the guest clock works flawlessly with it enabled. Does that one happen on master/1.7 as well? Oddly we saw this one or something like it on Xen+qemu. -- Alex Bligh
[Qemu-devel] kvm/hyper-v: obtaining client machine id
Hi, do you know if it is possible to obtain the client machine id of a Windows vServer via a Hyper-V hypercall? I would need an information to check vServer activations against our KMS. Thanks, Peter
Re: [Qemu-devel] [PATCH] vnc: Fix qemu crash on vnc client disconnection
Hi, > diff --git a/ui/vnc.c b/ui/vnc.c > index 5601cc3..2177704 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -876,7 +876,8 @@ static int find_and_clear_dirty_height(struct VncState > *vs, > static int vnc_update_client_sync(VncState *vs, int has_dirty) > { > int ret = vnc_update_client(vs, has_dirty); > -vnc_jobs_join(vs); > +if (ret >= 0) > +vnc_jobs_join(vs); What happens with any running jobs if you skip the jouin call here? cheers, Gerd
Re: [Qemu-devel] [Xen-devel] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Hi, Please post any comments or questions if you get around to testing them. Your comments are very welcome! Best regards, -Gonglei > > >>> On 22.10.13 at 06:08, "Gonglei (Arei)" > wrote: > > > Hi, guys. The new patch has been modified based on the principles you > > > suggested, thank you so much. > > > Last time I test the patch based on the codes of 4.3.0. > > > This time, I found that the system based on the codes of trunk causes the > VM > > > reboot again and again, which I have not found out the reason. > > > So i can not test the patch based on the codes of trunk (details in > > > EJ0_ACPI_PCI_Hotplug.patch).. > > > > I'm afraid we will need you to figure out that problem first, and > > then do the verification on -unstable. Even if the code shouldn't > > be that different from 4.3, we still don't want to apply completely > > untested stuff. > > Hi, Jan. We found that the reason that we used a wrong seabios PATH, and the > hvmloader can't load the bios.bin. > So the VM restart again and again after we start it. That's our fault. > > Now I test the patch based on the codes of trunk, which works well. > The patch has been modified after your suggestion. > The patch works well with upstream qemu and doesn't affect the system with > traditional qemu. > > > > > > > > --- a/tools/firmware/hvmloader/ovmf.c > > > +++ b/tools/firmware/hvmloader/ovmf.c > > > @@ -79,7 +79,11 @@ static void ovmf_acpi_build_tables(void) > > > .dsdt_anycpu = dsdt_anycpu, > > > .dsdt_anycpu_len = dsdt_anycpu_len, > > > .dsdt_15cpu = NULL, > > > -.dsdt_15cpu_len = 0 > > > +.dsdt_15cpu_len = 0, > > > +.aml_ej0_name = NULL, > > > +.aml_adr_dword = NULL, > > > +.aml_ej0_name_len = 0, > > > +.aml_adr_dword_len = 0, > > > > I don't see why you're adding these. > > > Insurance purposes is that just initialize the struct. > > Signed-off-by: Gaowei > Signed-off-by: gonglei > --- > tools/firmware/hvmloader/acpi/Makefile | 37 ++- > tools/firmware/hvmloader/acpi/acpi2_0.h| 4 + > tools/firmware/hvmloader/acpi/build.c | 21 +- > tools/firmware/hvmloader/acpi/dsdt.asl | 1 + > tools/firmware/hvmloader/acpi/mk_dsdt.c| 2 + > tools/firmware/hvmloader/ovmf.c| 6 +- > tools/firmware/hvmloader/rombios.c | 4 + > tools/firmware/hvmloader/seabios.c | 8 + > tools/firmware/hvmloader/tools/acpi_extract.py | 308 > + > .../hvmloader/tools/acpi_extract_preprocess.py | 41 +++ > 10 files changed, 419 insertions(+), 13 deletions(-) > create mode 100644 tools/firmware/hvmloader/tools/acpi_extract.py > create mode 100644 > tools/firmware/hvmloader/tools/acpi_extract_preprocess.py > > diff --git a/tools/firmware/hvmloader/acpi/Makefile > b/tools/firmware/hvmloader/acpi/Makefile > index 2c50851..b96e058 100644 > --- a/tools/firmware/hvmloader/acpi/Makefile > +++ b/tools/firmware/hvmloader/acpi/Makefile > @@ -24,30 +24,45 @@ OBJS = $(patsubst %.c,%.o,$(C_SRC)) > CFLAGS += $(CFLAGS_xeninclude) > > vpath iasl $(PATH) > + > +.DELETE_ON_ERROR: $(filter dsdt_%.c,$(C_SRC)) > + > all: acpi.a > > ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl > iasl -vs -p $* -tc $< > - sed -e 's/AmlCode/$*/g' $*.hex >$@ > + sed -e 's/AmlCode/$*/g' $*.hex > $@.tmp > + $(call move-if-changed,$@.tmp $@) > rm -f $*.hex $*.aml > > mk_dsdt: mk_dsdt.c > $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c > > dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt > - awk 'NR > 1 {print s} {s=$$0}' $< > $@ > - ./mk_dsdt --dm-version qemu-xen >> $@ > + awk 'NR > 1 {print s} {s=$$0}' $< > $@.tmp > + sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@.tmp > + ./mk_dsdt --dm-version qemu-xen >> $@.tmp > + sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g' > $@.tmp > + sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g' > $@.tmp > + $(call move-if-changed,$@.tmp $@) > > # NB. awk invocation is a portable alternative to 'head -n -1' > dsdt_%cpu.asl: dsdt.asl mk_dsdt > - awk 'NR > 1 {print s} {s=$$0}' $< > $@ > - ./mk_dsdt --maxcpu $* >> $@ > + awk 'NR > 1 {print s} {s=$$0}' $< > $@.tmp > + sed -i 's/AmlCode/dsdt_$*cpu/g' $@.tmp > + ./mk_dsdt --maxcpu $* >> $@.tmp > + $(call move-if-changed,$@.tmp $@) > > -$(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl > - iasl -vs -p $* -tc $*.asl > - sed -e 's/AmlCode/$*/g' $*.hex >$@ > - echo "int $*_len=sizeof($*);" >>$@ > - rm -f $*.aml $*.hex > +$(filter dsdt_%.c,$(C_SRC)): %.c: %.asl iasl > + cpp -P $< > $<.i.orig > + $(PYTHON) ../tools/acpi_extract_preprocess.py $<.i.orig > $<.i > + iasl -vs -l -tc -p $* $<.i > + $(PYTHON) ../tools/acpi_extract.py $*.lst > $@.tmp > + echo "int $*_len=sizeof($*);" >> $@.tmp > + if grep -q "$*_aml_ej0_name" $@.tmp; then echo "i
Re: [Qemu-devel] BUG: RTC issue when Windows guest is idle
On 28 Oct 2013, at 07:44, Alex Bligh wrote: >> >> A weird thing that has been happening is the RTC timer stopping after a live >> migration. This happened in both 1.6.1 and the 1.6.50 git build. To >> replicate the issue I was migrating to/from the same machine and anywhere >> between 1 and 3 migrations the guest clock would stop. Connecting to the VNC >> console would not get it running again. I've tried to replicate the issue >> with the HPET enabled but the guest clock works flawlessly with it enabled. > > Does that one happen on master/1.7 as well? Oddly we saw this one or > something like it on Xen+qemu. ... and master actually calls itself 1.6.50 which I'd never realised, so this is broken on master it seems. If it works with HPET it looks like a clock source / migration bug. -- Alex Bligh
Re: [Qemu-devel] [PATCH] Python-lang gdb script to extract x86_64 guest vmcore from qemu coredump
On 10/11/13 19:54, Stefan Hajnoczi wrote: > On Thu, Sep 12, 2013 at 9:46 PM, Laszlo Ersek wrote: >> When qemu dies unexpectedly, for example in response to an explicit >> abort() call, or (more importantly) when an external signal is delivered >> to it that results in a coredump, sometimes it is useful to extract the >> guest vmcore from the qemu process' memory image. The guest vmcore might >> help understand an emulation problem in qemu, or help debug the guest. >> >> This script reimplements (and cuts many features of) the >> qmp_dump_guest_memory() command in gdb/Python, >> >> https://sourceware.org/gdb/current/onlinedocs/gdb/Python-API.html >> >> working off the saved memory image of the qemu process. The docstring in >> the patch (serving as gdb help text) describes the limitations relative to >> the QMP command. >> >> Dependencies of qmp_dump_guest_memory() have been reimplemented as needed. >> I sought to follow the general structure, sticking to original function >> names where possible. However, keeping it simple prevailed in some places. >> >> The patch has been tested with a 4 VCPU, 768 MB, RHEL-6.4 >> (2.6.32-358.el6.x86_64) guest: > > I tried this out with qemu-kvm-1.1.2 and it worked after a few minor > tweaks due to memory data structure changes. I did hit a problem with > crash since the vmlinux was 32-bit and the guest dump was 64-bit. But > from what I can tell dump-guest-memory works as advertised. > > Reviewed-by: Stefan Hajnoczi Could someone please pick up the patch? Thanks! Laszlo
Re: [Qemu-devel] [RFC] block io lost in the guest , possible related to qemu?
Hello Kevin & Stefan Any comments or wild guess about the bug? Regards, Jack On 10/25/2013 05:01 PM, Jack Wang wrote: > Hi Experts, > > We've seen guest block io lost in a VM.any response will be helpful > > environment is: > guest os: Ubuntu 1304 > running busy database workload with xfs on a disk export with virtio-blk > > the exported vdb has very high infight io over 300. Some times later a > lot io process in D state, looks a lot requests is lost in below storage > stack. > > We're use qemu-kvm 1.0, host kernel 3.4.51 > > In qemu log of virtio-blk.c > I found below commit, I wonder is it possible the workload generate some > unknown reqests to qemu that lost in virtio_blk_handle_read? > I do some fio test myself, I cann't generate so call unknown request type. > > Any response will be helpful. > > Jack > > > commit 9e72c45033770b81b536ac6091e91807247cc25a > Author: Alexey Zaytsev > Date: Thu Dec 13 09:03:43 2012 +0200 > > virtio-blk: Return UNSUPP for unknown request types > > Currently, all unknown requests are treated as VIRTIO_BLK_T_IN > > Signed-off-by: Alexey Zaytsev > Signed-off-by: Stefan Hajnoczi > > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > index 92c745a..df57b35 100644 > --- a/hw/virtio-blk.c > +++ b/hw/virtio-blk.c > @@ -398,10 +398,14 @@ static void > virtio_blk_handle_request(VirtIOBlockReq *req, > qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1], > req->elem.out_num - 1); > virtio_blk_handle_write(req, mrb); > -} else { > +} else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) { > +/* VIRTIO_BLK_T_IN is 0, so we can't just & it. */ > qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0], > req->elem.in_num - 1); > virtio_blk_handle_read(req); > +} else { > +virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); > +g_free(req); > } > } >
Re: [Qemu-devel] [Xen-devel] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
>>> On 24.10.13 at 14:17, "Gonglei (Arei)" wrote: > Now I test the patch based on the codes of trunk, which works well. > The patch has been modified after your suggestion. Partly. I looks reasonable now, but still not pretty. But the tools maintainers will have to have the final say here anyway. Jan
Re: [Qemu-devel] [PATCH] vnc: Fix qemu crash on vnc client disconnection
Hi, Any comments will be welcome. Best regards, -Gonglei > -Original Message- > From: Gonglei (Arei) > Sent: Thursday, October 24, 2013 1:14 PM > To: qemu-devel@nongnu.org; 'Gerd Hoffmann'; 'Stefan Hajnoczi' > Cc: Luonengjun; Huangweidong (Hardware); Yanqiangjun > Subject: [PATCH] vnc: Fix qemu crash on vnc client disconnection > > Hi, > > I encount a qemu crash when the vnc client disconnection, and I got the next > log: > > qemu: qemu_mutex_lock: Invalid argument > > and the backtrace listed: > > Core was generated by > `/mnt/sdd/gonglei/kvm/qemu-unstable/x86_64-softmmu/qemu-system-x86_64 > -name suse'. > Program terminated with signal 6, Aborted. > #0 0x7fab8498ed95 in raise () from /lib64/libc.so.6 > (gdb) bt > #0 0x7fab8498ed95 in raise () from /lib64/libc.so.6 > #1 0x7fab849902ab in abort () from /lib64/libc.so.6 > #2 0x7fab87c22915 in error_exit (err=22, msg=0x7fab87c97a70 > <__func__.4762> "qemu_mutex_lock") at util/qemu-thread-posix.c:28 > #3 0x7fab87c22a19 in qemu_mutex_lock (mutex=0x7fab8858e688) at > util/qemu-thread-posix.c:59 > #4 0x7fab87ae52ea in vnc_lock_output (vs=0x7fab885823f0) at > ui/vnc-jobs.h:63 > #5 0x7fab87ae5217 in vnc_jobs_consume_buffer (vs=0x7fab885823f0) > at ui/vnc-jobs.c:166 > #6 0x7fab87ae51dd in vnc_jobs_join (vs=0x7fab885823f0) at > ui/vnc-jobs.c:159 > #7 0x7fab87aea776 in vnc_update_client_sync (vs=0x7fab885823f0, > has_dirty=1) at ui/vnc.c:880 > #8 0x7fab87aea007 in vnc_dpy_copy (dcl=0x7fab8088f048, src_x=746, > src_y=578, dst_x=772, dst_y=578, w=22, h=22) at ui/vnc.c:753 > #9 0x7fab87ac8df6 in dpy_gfx_copy (con=0x7fab885cdb40, src_x=746, > src_y=578, dst_x=772, dst_y=578, w=22, h=22) > at ui/console.c:1455 > #10 0x7fab87ac9fd7 in qemu_console_copy (con=0x7fab885cdb40, > src_x=746, src_y=578, dst_x=772, dst_y=578, w=22, h=22) > at ui/console.c:1837 > #11 0x7fab8799bd91 in cirrus_do_copy (s=0x7fab885ff450, dst=1228339, > src=1228287, w=22, h=22) at hw/display/cirrus_vga.c:738 > #12 0x7fab8799bf03 in cirrus_bitblt_videotovideo_copy (s=0x7fab885ff450) > at hw/display/cirrus_vga.c:757 > #13 0x7fab8799c48c in cirrus_bitblt_videotovideo (s=0x7fab885ff450) at > hw/display/cirrus_vga.c:879 > #14 0x7fab8799cc00 in cirrus_bitblt_start (s=0x7fab885ff450) at > hw/display/cirrus_vga.c:1020 > #15 0x7fab8799cfbb in cirrus_write_bitblt (s=0x7fab885ff450, reg_value=2) > at hw/display/cirrus_vga.c:1041 > #16 0x7fab8799dedb in cirrus_vga_write_gr (s=0x7fab885ff450, > reg_index=49, reg_value=2) at hw/display/cirrus_vga.c:1536 > #17 0x7fab8799e721 in cirrus_mmio_blt_write (s=0x7fab885ff450, > address=64, value=2 '\002') at hw/display/cirrus_vga.c:1890 > #18 0x7fab879a068d in cirrus_mmio_write (opaque=0x7fab885ff450, > addr=320, val=2, size=1) at hw/display/cirrus_vga.c:2670 > #19 0x7fab87b77921 in memory_region_write_accessor > (mr=0x7fab8860fe90, addr=320, value=0x7fab818d5cc8, size=1, shift=0, mask= > 255) at /mnt/sdd/gonglei/kvm/qemu-unstable/memory.c:440 > #20 0x7fab87b77a5d in access_with_adjusted_size (addr=320, > value=0x7fab818d5cc8, size=4, access_size_min=1, access_size_max=1, > access=0x7fab87b77898 , > mr=0x7fab8860fe90) at /mnt/sdd/gonglei/kvm/qemu-unstable/memory.c:477 > #21 0x7fab87b7a8c0 in memory_region_dispatch_write > (mr=0x7fab8860fe90, addr=320, data=18446744073709551362, size=4) > at /mnt/sdd/gonglei/kvm/qemu-unstable/memory.c:984 > #22 0x7fab87b7e176 in io_mem_write (mr=0x7fab8860fe90, addr=320, > val=18446744073709551362, size=4) > at /mnt/sdd/gonglei/kvm/qemu-unstable/memory.c:1748 > #23 0x7fab87b0e91e in address_space_rw (as=0x7fab8848b960 > , addr=4273832256, buf= > 0x7fab87830028 , len=4, > is_write=true) at /mnt/sdd/gonglei/kvm/qemu-unstable/exec.c:1963 > #24 0x7fab87b0eec0 in cpu_physical_memory_rw (addr=4273832256, > buf=0x7fab87830028 , len= > 4, is_write=1) at /mnt/sdd/gonglei/kvm/qemu-unstable/exec.c:2042 > #25 0x7fab87b74b47 in kvm_cpu_exec (cpu=0x7fab88520c20) at > /mnt/sdd/gonglei/kvm/qemu-unstable/kvm-all.c:1673 > #26 0x7fab87b022e9 in qemu_kvm_cpu_thread_fn (arg=0x7fab88520c20) > at /mnt/sdd/gonglei/kvm/qemu-unstable/cpus.c:785 > #27 0x7fab85b07f05 in start_thread () from /lib64/libpthread.so.0 > #28 0x7fab84a3353d in clone () from /lib64/libc.so.6 > > When Vnc client was disconnected, the vs->csock will be set to -1 in function > vnc_disconnect_start. And on the next loop, in case of the function transfer: > vnc_dpy_copy-->vnc_update_client_sync-->vnc_update_client-->vnc_disconnec > t_finish(vs) > and > vnc_dpy_copy-->vnc_update_client_sync--> > vnc_jobs_consume_buffer-->vnc_lock_output(vs)--> > qemu_mutex_lock(&vs->output_mutex); > because the vs has been freed, the qemu_mutex_lock(&vs->output_mutex) will > cause qemu abort. > > The patch fixed the bug: > > when the vs object be freed, function vnc_update_client return -1, > and vnc_update_client_sync do not deal with th
Re: [Qemu-devel] [RFC] block io lost in the guest , possible related to qemu?
On 10/28/2013 10:54 AM, Alexey Zaytsev wrote: > Hey. > > I very much doubt this commit could be causing the problem, as qemu > would never set wrong request type in the first place. You can easily > check by either reverting it, or adding a printk() before > virtio_blk_req_complete(VIRTIO_BLK_S_UNSUPP). Hi Alexey, Thanks for you input. According to my test results, yes, as you said, virtio-blk never generate wrong request type. So the commit is only a small cosmetic extra check:( As there's nothing abnormal in host server and storage, there must be some hidden bug somewhere, damn it. Regards, Jack > > On Mon, Oct 28, 2013 at 10:15 AM, Jack Wang wrote: >> Hello Kevin & Stefan >> >> Any comments or wild guess about the bug? >> >> Regards, >> Jack >> >> On 10/25/2013 05:01 PM, Jack Wang wrote: >>> Hi Experts, >>> >>> We've seen guest block io lost in a VM.any response will be helpful >>> >>> environment is: >>> guest os: Ubuntu 1304 >>> running busy database workload with xfs on a disk export with virtio-blk >>> >>> the exported vdb has very high infight io over 300. Some times later a >>> lot io process in D state, looks a lot requests is lost in below storage >>> stack. >>> >>> We're use qemu-kvm 1.0, host kernel 3.4.51 >>> >>> In qemu log of virtio-blk.c >>> I found below commit, I wonder is it possible the workload generate some >>> unknown reqests to qemu that lost in virtio_blk_handle_read? >>> I do some fio test myself, I cann't generate so call unknown request type. >>> >>> Any response will be helpful. >>> >>> Jack >>> >>> >>> commit 9e72c45033770b81b536ac6091e91807247cc25a >>> Author: Alexey Zaytsev >>> Date: Thu Dec 13 09:03:43 2012 +0200 >>> >>> virtio-blk: Return UNSUPP for unknown request types >>> >>> Currently, all unknown requests are treated as VIRTIO_BLK_T_IN >>> >>> Signed-off-by: Alexey Zaytsev >>> Signed-off-by: Stefan Hajnoczi >>> >>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c >>> index 92c745a..df57b35 100644 >>> --- a/hw/virtio-blk.c >>> +++ b/hw/virtio-blk.c >>> @@ -398,10 +398,14 @@ static void >>> virtio_blk_handle_request(VirtIOBlockReq *req, >>> qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1], >>> req->elem.out_num - 1); >>> virtio_blk_handle_write(req, mrb); >>> -} else { >>> +} else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) { >>> +/* VIRTIO_BLK_T_IN is 0, so we can't just & it. */ >>> qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0], >>> req->elem.in_num - 1); >>> virtio_blk_handle_read(req); >>> +} else { >>> +virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); >>> +g_free(req); >>> } >>> } >>> >>
Re: [Qemu-devel] [RFC v3 0/2] use sizes.h macros for power-of-two sizes
Il 23/10/2013 14:16, Antony Pavlov ha scritto: > On Tue, 24 Sep 2013 08:32:10 +0400 > Antony Pavlov wrote: > > ping-ping > >> On Fri, 13 Sep 2013 11:33:24 +0400 >> Antony Pavlov wrote: >> >> ping >> >>> Changes since v2: >>> * commit messages: drop ALL 'Reviewed-by' tags. >>> Drop Aurelien Jarno's tag because the patchseries >>> was completely reworked, so it need additional review. >>> >>> Changes since v1: >>> >>> * include/sizes.h -> include/qemu/sizes.h >>> * fix copyright header; >>> * fix formatting: drop tabs; >>> * use the BIT() macro, so it's easy-to-read the constants column; >>> also the BIT() macro casts constant to UL; >>> * rebase on updated master; >>> * take into account the "mips_malta: support up to 2GiB RAM" commit. >>> >>> [RFC v3 1/2] include/qemu: introduce sizes.h >>> [RFC v3 2/2] hw/mips: use sizes.h macros >>> >>> The sizes.h macros is a easy-to-read method of >>> power-of-two memory sizes representation. The sizes.h >>> macros are actively used in linux kernel and other >>> projects, so let's use them in QEMU too. >> >> -- >> Best regards, >> Antony Pavlov > > This unfortunately will miss 1.7. Please repost after 1.7 is released, without the "RFC" in the subject -- just use PATCH instead. Paolo
Re: [Qemu-devel] [PATCH 3/6] qapi: rename prefix QEVENT to Q_EVENT
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Il 21/10/2013 22:41, Eric Blake ha scritto: > On 10/21/2013 03:16 AM, Wenchao Xia wrote: >> The define will be moved to qapi-schema.json later, so rename >> the prefix to match its naming style. > > Wouldn't it be simpler to fix the code generator to special case > QEvent to turn into QEVENT, instead of having to go through this > churn? But if we _like_ the Q_EVENT_ prefix, then this looks > fairly mechanical: No, I disagree. However, since something _is_ being renamed, we might as well rename every QEVENT_ to MONITOR_EVENT_ and avoid patch 2. Paolo -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJSbj/xAAoJEBvWZb6bTYbyNkQP/R0l+qwakF5zBfwoiEc7Ru4r YnG6B2HkCn7Bc5juDnZXYIvPdX1+n3x9tvGTcw2eP+8hiUQBmVGQEWNIlvOkPQoW EtW7wzf0uRvROeYy0aqr5P+xU/0uDLonr3wONxv3aMc99CwZvJ918X889is5z4ps P7CrMrnh9Ng99SgmkFgYS9KxwM004iIKyGyjKbYC+BYTKrJd7chzGfb41r687AbR 7CMb3gBCUvFRU2E2+yCdRI/cRVgRDSCMZ4Ic5VNK4DFKa6eq+yzP4zEhILkStl1o yvPYlxpfHNLLle83iOb4rJFmBYz6mrAkVhfUOXdrKPegU6l+hod0vpuXcm3tMIO8 YJ9iMxeFrI2tFsIXVzQgugz4jaSY0BDapCP15aL9zqIzACiTpYZeaVden/gWu1Oy iupuue9xzT2l40FCQwNXrGULOiOVRuiry8JstJ0A/valQi/xRy28cky+FI+PglNt 3eaj3GYK0oF1UyG3e22cFensIUOTWE1Znbm7/5ON12xcFOTE1e5bzvuPWUDOJ1wh 2I/p7Bn7uewQLstL14++gN1iMP+GCXJ0ce7xc0QtTP7jvtYCeTt+yBDC7jlan4Zb N/CEISOti9obFYlDKiQL+Hdk4ivQsFjT2B1p7feGSd5JAZtLthNxBOSQrS4wg5fd WW6UX7otKyj6MADQl3Jk =Q+3Z -END PGP SIGNATURE-
Re: [Qemu-devel] BUG: RTC issue when Windows guest is idle
Il 28/10/2013 07:58, Matthew Anderson ha scritto: > Hi Alex, > > I've been doing some testing with the latest git version and so far I haven't > seen a guest freeze in the same circumstances as before. > > A weird thing that has been happening is the RTC timer stopping after a live > migration. This happened in both 1.6.1 and the 1.6.50 git build. To replicate > the issue I was migrating to/from the same machine and anywhere between 1 and > 3 migrations the guest clock would stop. Connecting to the VNC console would > not get it running again. I've tried to replicate the issue with the HPET > enabled but the guest clock works flawlessly with it enabled. Please try this: (1) reproduce it with migration to file + restore from file to save, from QEMU monitor: migrate exec:cat>migr.ckp to restore, from command line: ... -incoming 'exec:cat migr.ckp' (2) If the RTC stops after restoring, compress migr.ckp and place it somewhere I can download it. Thanks, Paolo > Guest VM is Windows 2008R2. Host is Ubuntu 13.04 (kernel 3.8.0-25-generic) > Command line - > /usr/local/bin/qemu-system-x86_64 -enable-kvm -nodefconfig -nodefaults > -daemonize -usb -chardev > socket,id=charmonitor,path=/var/run/based1/monitor/525ce3d009c437d67802.monitor,server,nowait > -mon chardev=charmonitor,id=monitor,mode=control -chardev > socket,path=/var/run/based1/ga/525ce3d009c437d67802.guestagent,server,nowait,id=qga0 > -device virtio-serial -device > virtserialport,chardev=qga0,name=org.qemu.guest_agent.0 -pidfile > /var/run/based1/pid/525ce3d009c437d67802.pid -vga cirrus -vnc 0.0.0.0:1 > -M pc-i440fx-1.6 -m 1024 -smp sockets=1,cores=1,threads=1 -cpu > qemu64,+vme,+dts,+acpi,+dtes64,+vmx,+smx,+ssse3,+sse4_1,+sse4_2,+tpr_shadow,+vnmi,+flexpriority,+ept,+vpid,hv_relaxed,hv_spinlocks=0x,hv_vapic > -rtc base=utc,driftfix=slew --no-hpet -drive > aio=native,file=rbd:sata/525ce3d009c437d67803,if=virtio,id=disk-525ce3d009c437d67805,format=raw,cache=writeback,media=disk,index=0,addr=0xa > -netdev > tap,id=netdev-5264b6d46e53c81719000236,vhost=off,ifname=tap2,script=no,do w nscript=no -device virtio-net-pci,netdev=netdev-5264b6d46e53c81719000236,id=interface-5264b6d46e53c81719000236,mac=9a:a5:63:64:6f:76,bus=pci.0,addr=0xb -incoming tcp:0:3004 > > Thanks > -Matt > > > -Original Message- > From: Alex Bligh [mailto:a...@alex.org.uk] > Sent: Tuesday, 22 October 2013 5:36 PM > To: Xiexiangyou; Matthew Anderson > Cc: Stefan Hajnoczi; qemu-devel@nongnu.org; Alex Bligh > Subject: Re: [Qemu-devel] BUG: RTC issue when Windows guest is idle > > > > --On 22 October 2013 08:28:08 + Xiexiangyou > wrote: > >> Hi: >> >> I have run windows2008r2 guest with qemu-1.5.1/1.6(I have not test the >> the newer version) for long time, the issue (guest in hangup state) >> will come out.When guest in hangup state, QEMU main thread is blocked >> in g_poll loop. >>> 398c398,399 >>> < uint32_t timeout = UINT32_MAX; >>> --- /* uint32_t timeout = UINT32_MAX; */ uint32_t timeout = 1000; >>> >> It seems can fix the problem, and rtc/hpet interrupt can inject into >> guest again because of the "timeout", and guest will wake up . But >> maybe the issue is also exist, because during the time before timeout >> , guest also will lose rtc/hpet ticks. > > I do not think that is the correct fix for 1.5.1/1.6; what you are basically > doing is limiting the wait in the mainloop to one second (1.5.1/1.6 are in > milliseconds); however, I believe there may be other code that looks for > infinite timeouts. Either there is some other bug that this is masking (in > which case it may or may not be fixed in master / 1.7), or its a bug in the > timer stuff in 1.5.1/1.6 (which would not surprise me) which is likely to > have been fixed in master / 1.7. > > -- > Alex Bligh > >
Re: [Qemu-devel] [PATCH] vnc: Fix qemu crash on vnc client disconnection
> -Original Message- > From: Gerd Hoffmann [mailto:kra...@redhat.com] > Sent: Monday, October 28, 2013 3:53 PM > To: Gonglei (Arei) > Cc: qemu-devel@nongnu.org; Stefan Hajnoczi; Yanqiangjun; Luonengjun; > Huangweidong (Hardware) > Subject: Re: [Qemu-devel] [PATCH] vnc: Fix qemu crash on vnc client > disconnection > > Hi, > > > diff --git a/ui/vnc.c b/ui/vnc.c > > index 5601cc3..2177704 100644 > > --- a/ui/vnc.c > > +++ b/ui/vnc.c > > @@ -876,7 +876,8 @@ static int find_and_clear_dirty_height(struct > VncState *vs, > > static int vnc_update_client_sync(VncState *vs, int has_dirty) > > { > > int ret = vnc_update_client(vs, has_dirty); > > -vnc_jobs_join(vs); > > +if (ret >= 0) > > +vnc_jobs_join(vs); > > What happens with any running jobs if you skip the jouin call here? Hi, Gerd. The other jobs are unaffected, and other clients still work. Best regards, -Gonglei
Re: [Qemu-devel] [PATCH V9 03/11] quorum: Add quorum_aio_writev and its dependencies.
Le Friday 04 Oct 2013 à 16:35:18 (+0200), Max Reitz a écrit : > On 2013-10-02 14:39, Benoît Canet wrote: > >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(); > >+} > Hm, wouldn't it be better to pass the cancel to the children? > > Max Hi Max, Hi don't understand how you would do this. Best regards Benoît > > >+} > >+ > >+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) > >
Re: [Qemu-devel] [PATCH V9 06/11] quorum: Add quorum mechanism.
Le Friday 04 Oct 2013 à 16:48:12 (+0200), Max Reitz a écrit : > On 2013-10-02 14:39, Benoît Canet wrote: > >Use gnutls's SHA-256 to compare versions. > Wouldn't CRC32 suffice? (I don't really oppose using SHA, but taking > in gnutls as a dependency just for comparing several memory areas > seems a bit much to me) Initially it gzip's addler32 was used but someone was concerned with the risk of collisions. Anyway the code fallback using hashes only when something wrong is detected so it won't impact the normal case. Best regards Benoît > > >Signed-off-by: Benoit Canet > >--- > > block/Makefile.objs | 2 +- > > block/quorum.c| 321 > > +- > > configure | 36 ++ > > include/monitor/monitor.h | 2 + > > monitor.c | 2 + > > 5 files changed, 361 insertions(+), 2 deletions(-) > > > >diff --git a/block/Makefile.objs b/block/Makefile.objs > >index 05a65c2..adcdc21 100644 > >--- a/block/Makefile.objs > >+++ b/block/Makefile.objs > >@@ -3,7 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o > >qcow2-snapshot.o qcow2-c > > block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o > > block-obj-y += qed-check.o > > block-obj-y += vhdx.o > >-block-obj-y += quorum.o > >+block-obj-$(CONFIG_QUORUM) += quorum.o > > block-obj-y += parallels.o blkdebug.o blkverify.o > > block-obj-y += snapshot.o qapi.o > > block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o > >diff --git a/block/quorum.c b/block/quorum.c > >index f0fc0e9..e235ac1 100644 > >--- a/block/quorum.c > >+++ b/block/quorum.c > >@@ -13,7 +13,43 @@ > > * See the COPYING file in the top-level directory. > > */ > >+#include > >+#include > > #include "block/block_int.h" > >+#include "qapi/qmp/qjson.h" > >+ > >+#define HASH_LENGTH 32 > >+ > >+/* This union hold a vote hash value */ > *holds > > >+typedef union QuorumVoteValue { > >+char h[HASH_LENGTH]; /* SHA-256 hash */ > >+int64_t l; /* simpler 64 bits hash */ > >+} QuorumVoteValue; > >+ > >+/* A vote item */ > >+typedef struct QuorumVoteItem { > >+int index; > >+QLIST_ENTRY(QuorumVoteItem) next; > >+} QuorumVoteItem; > >+ > >+/* this structure is a vote version. A version is the set of vote sharing > >the > *set of votes > > >+ * same vote value. > >+ * The set of vote will be tracked with the items field and it's count is > *set of votes or *vote set; also s/it's count/its cardinality/ or > something like that > > >+ * vote_count. > >+ */ > >+typedef struct QuorumVoteVersion { > >+QuorumVoteValue value; > >+int index; > >+int vote_count; > >+QLIST_HEAD(, QuorumVoteItem) items; > >+QLIST_ENTRY(QuorumVoteVersion) next; > >+} QuorumVoteVersion; > >+ > >+/* this structure hold a group of vote versions together */ > *holds > > >+typedef struct QuorumVotes { > >+QLIST_HEAD(, QuorumVoteVersion) vote_list; > >+int (*compare)(QuorumVoteValue *a, QuorumVoteValue *b); > >+} QuorumVotes; > > /* the following structure hold the state of one quorum instance */ > > typedef struct { > >@@ -60,10 +96,14 @@ struct QuorumAIOCB { > > int success_count; /* number of successfully completed AIOCB > > */ > > bool *finished; /* completion signal for cancel */ > >+QuorumVotes votes; > >+ > > bool is_read; > > int vote_ret; > > }; > >+static void quorum_vote(QuorumAIOCB *acb); > >+ > > static void quorum_aio_cancel(BlockDriverAIOCB *blockacb) > > { > > QuorumAIOCB *acb = container_of(blockacb, QuorumAIOCB, common); > >@@ -111,6 +151,10 @@ static void quorum_aio_finalize(QuorumAIOCB *acb) > > acb->aios[i].ret = 0; > > } > >+if (acb->vote_ret) { > >+ret = acb->vote_ret; > >+} > >+ > > acb->common.cb(acb->common.opaque, ret); > > if (acb->finished) { > > *acb->finished = true; > >@@ -122,6 +166,11 @@ static void quorum_aio_finalize(QuorumAIOCB *acb) > > qemu_aio_release(acb); > > } > >+static int quorum_sha256_compare(QuorumVoteValue *a, QuorumVoteValue *b) > >+{ > >+return memcmp(a->h, b->h, HASH_LENGTH); > >+} > >+ > > static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, > > BlockDriverState *bs, > > QEMUIOVector *qiov, > >@@ -141,6 +190,7 @@ static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, > > acb->count = 0; > > acb->success_count = 0; > > acb->finished = NULL; > >+acb->votes.compare = quorum_sha256_compare; > > acb->is_read = false; > > acb->vote_ret = 0; > >@@ -170,9 +220,278 @@ static void quorum_aio_cb(void *opaque, int ret) > > return; > > } > >+/* Do the vote on read */ > >+if (acb->is_read) { > >+quorum_vote(acb); > >+} > >+ > > quorum_aio_finalize(acb); > > } > >+static void quorum_report_bad(QuorumAIOCB *acb, int index) > >+{ > >+QObject *data; > >
Re: [Qemu-devel] [RFC] block io lost in the guest , possible related to qemu?
Hey. I very much doubt this commit could be causing the problem, as qemu would never set wrong request type in the first place. You can easily check by either reverting it, or adding a printk() before virtio_blk_req_complete(VIRTIO_BLK_S_UNSUPP). On Mon, Oct 28, 2013 at 10:15 AM, Jack Wang wrote: > Hello Kevin & Stefan > > Any comments or wild guess about the bug? > > Regards, > Jack > > On 10/25/2013 05:01 PM, Jack Wang wrote: >> Hi Experts, >> >> We've seen guest block io lost in a VM.any response will be helpful >> >> environment is: >> guest os: Ubuntu 1304 >> running busy database workload with xfs on a disk export with virtio-blk >> >> the exported vdb has very high infight io over 300. Some times later a >> lot io process in D state, looks a lot requests is lost in below storage >> stack. >> >> We're use qemu-kvm 1.0, host kernel 3.4.51 >> >> In qemu log of virtio-blk.c >> I found below commit, I wonder is it possible the workload generate some >> unknown reqests to qemu that lost in virtio_blk_handle_read? >> I do some fio test myself, I cann't generate so call unknown request type. >> >> Any response will be helpful. >> >> Jack >> >> >> commit 9e72c45033770b81b536ac6091e91807247cc25a >> Author: Alexey Zaytsev >> Date: Thu Dec 13 09:03:43 2012 +0200 >> >> virtio-blk: Return UNSUPP for unknown request types >> >> Currently, all unknown requests are treated as VIRTIO_BLK_T_IN >> >> Signed-off-by: Alexey Zaytsev >> Signed-off-by: Stefan Hajnoczi >> >> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c >> index 92c745a..df57b35 100644 >> --- a/hw/virtio-blk.c >> +++ b/hw/virtio-blk.c >> @@ -398,10 +398,14 @@ static void >> virtio_blk_handle_request(VirtIOBlockReq *req, >> qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1], >> req->elem.out_num - 1); >> virtio_blk_handle_write(req, mrb); >> -} else { >> +} else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) { >> +/* VIRTIO_BLK_T_IN is 0, so we can't just & it. */ >> qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0], >> req->elem.in_num - 1); >> virtio_blk_handle_read(req); >> +} else { >> +virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); >> +g_free(req); >> } >> } >> >
Re: [Qemu-devel] [PATCH V9 06/11] quorum: Add quorum mechanism.
Le Friday 04 Oct 2013 à 16:48:12 (+0200), Max Reitz a écrit : > On 2013-10-02 14:39, Benoît Canet wrote: > >Use gnutls's SHA-256 to compare versions. > Wouldn't CRC32 suffice? (I don't really oppose using SHA, but taking > in gnutls as a dependency just for comparing several memory areas > seems a bit much to me) > > >Signed-off-by: Benoit Canet > >--- > > block/Makefile.objs | 2 +- > > block/quorum.c| 321 > > +- > > configure | 36 ++ > > include/monitor/monitor.h | 2 + > > monitor.c | 2 + > > 5 files changed, 361 insertions(+), 2 deletions(-) > > > >diff --git a/block/Makefile.objs b/block/Makefile.objs > >index 05a65c2..adcdc21 100644 > >--- a/block/Makefile.objs > >+++ b/block/Makefile.objs > >@@ -3,7 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o > >qcow2-snapshot.o qcow2-c > > block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o > > block-obj-y += qed-check.o > > block-obj-y += vhdx.o > >-block-obj-y += quorum.o > >+block-obj-$(CONFIG_QUORUM) += quorum.o > > block-obj-y += parallels.o blkdebug.o blkverify.o > > block-obj-y += snapshot.o qapi.o > > block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o > >diff --git a/block/quorum.c b/block/quorum.c > >index f0fc0e9..e235ac1 100644 > >--- a/block/quorum.c > >+++ b/block/quorum.c > >@@ -13,7 +13,43 @@ > > * See the COPYING file in the top-level directory. > > */ > >+#include > >+#include > > #include "block/block_int.h" > >+#include "qapi/qmp/qjson.h" > >+ > >+#define HASH_LENGTH 32 > >+ > >+/* This union hold a vote hash value */ > *holds > > >+typedef union QuorumVoteValue { > >+char h[HASH_LENGTH]; /* SHA-256 hash */ > >+int64_t l; /* simpler 64 bits hash */ > >+} QuorumVoteValue; > >+ > >+/* A vote item */ > >+typedef struct QuorumVoteItem { > >+int index; > >+QLIST_ENTRY(QuorumVoteItem) next; > >+} QuorumVoteItem; > >+ > >+/* this structure is a vote version. A version is the set of vote sharing > >the > *set of votes > > >+ * same vote value. > >+ * The set of vote will be tracked with the items field and it's count is > *set of votes or *vote set; also s/it's count/its cardinality/ or > something like that > > >+ * vote_count. > >+ */ > >+typedef struct QuorumVoteVersion { > >+QuorumVoteValue value; > >+int index; > >+int vote_count; > >+QLIST_HEAD(, QuorumVoteItem) items; > >+QLIST_ENTRY(QuorumVoteVersion) next; > >+} QuorumVoteVersion; > >+ > >+/* this structure hold a group of vote versions together */ > *holds > > >+typedef struct QuorumVotes { > >+QLIST_HEAD(, QuorumVoteVersion) vote_list; > >+int (*compare)(QuorumVoteValue *a, QuorumVoteValue *b); > >+} QuorumVotes; > > /* the following structure hold the state of one quorum instance */ > > typedef struct { > >@@ -60,10 +96,14 @@ struct QuorumAIOCB { > > int success_count; /* number of successfully completed AIOCB > > */ > > bool *finished; /* completion signal for cancel */ > >+QuorumVotes votes; > >+ > > bool is_read; > > int vote_ret; > > }; > >+static void quorum_vote(QuorumAIOCB *acb); > >+ > > static void quorum_aio_cancel(BlockDriverAIOCB *blockacb) > > { > > QuorumAIOCB *acb = container_of(blockacb, QuorumAIOCB, common); > >@@ -111,6 +151,10 @@ static void quorum_aio_finalize(QuorumAIOCB *acb) > > acb->aios[i].ret = 0; > > } > >+if (acb->vote_ret) { > >+ret = acb->vote_ret; > >+} > >+ > > acb->common.cb(acb->common.opaque, ret); > > if (acb->finished) { > > *acb->finished = true; > >@@ -122,6 +166,11 @@ static void quorum_aio_finalize(QuorumAIOCB *acb) > > qemu_aio_release(acb); > > } > >+static int quorum_sha256_compare(QuorumVoteValue *a, QuorumVoteValue *b) > >+{ > >+return memcmp(a->h, b->h, HASH_LENGTH); > >+} > >+ > > static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, > > BlockDriverState *bs, > > QEMUIOVector *qiov, > >@@ -141,6 +190,7 @@ static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, > > acb->count = 0; > > acb->success_count = 0; > > acb->finished = NULL; > >+acb->votes.compare = quorum_sha256_compare; > > acb->is_read = false; > > acb->vote_ret = 0; > >@@ -170,9 +220,278 @@ static void quorum_aio_cb(void *opaque, int ret) > > return; > > } > >+/* Do the vote on read */ > >+if (acb->is_read) { > >+quorum_vote(acb); > >+} > >+ > > quorum_aio_finalize(acb); > > } > >+static void quorum_report_bad(QuorumAIOCB *acb, int index) > >+{ > >+QObject *data; > >+data = qobject_from_jsonf("{ 'children-index': %i" > I'd prefer child-index. Generally, remember that the singular of > "children" is "child". > > >+ ", 'sector-num': %" PRId64 > >+
[Qemu-devel] [Bug 1119281] Re: The virtio network device breaks UuidCreateSequential()
This bug is still present in QEM 1.6.0 (qemu-system-x86 1.6.0+dfsg-1) and/or Virtio 0.1-65. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1119281 Title: The virtio network device breaks UuidCreateSequential() Status in QEMU: New Bug description: UuidCreateSequential() usually creates version 1 UUIDs (1) which means they contain the main network card's MAC address. However when using a virtio network card and driver the UUIDs contain random data instead of the guest's MAC address. Changing the network card to either the default rtl8139 one or the e1000 one fixes the issue. Here is the software I have tested this with: * qemu 1.1.2+dfsg-5 and 1.4.0~rc0+dfsg-1exp (from Debian Testing and Experimental respectively) * The 0.1-49 and 0.1-52 Windows virtio drivers from https://alt.fedoraproject.org/pub/alt/virtio-win/latest/images/bin/ * Both a 32-bit Windows XP guest and a 64-bit Windows 7 one. Here is how to test for this issue: * Set up a Windows guest with a single network card(2), a virtio one and install the corresponding driver. * Boot the guest and copy the uuidtest.exe file (see attachement) to it * On the command line, type 'ipconfig /all'. Give you the correct network card's MAC address on a line like the one below: Physical Address. . . . . . . . . : 52-54-00-C7-0E-97 * Run uuidtest.exe. It will show the VM returning a UUID with the wrong MAC address, and quite possibly even a multicast MAC address! (3). In the example below 'f75292c62787' should have been the MAC address. Note that on Windows XP UuidCreateSequential() returns RPC_S_UUID_LOCAL_ONLY for virtio cards but that on Windows 7 it returns 0. UuidCreateSequential() returned 0 uuid={56e1ffe4-71d8-11e2-b1cc-f75292c62787} Got a version 1 UUID The UUID does not contain a non-multicast MAC address * Reboot and notice uuidtest.exe now reports a different value where the MAC address should be. * Shut down the VM and switch the network card to rtl8139, install the drivers, run uuidtest.exe and notice that the last group of digits finally contains the correct MAC address. (1) https://en.wikipedia.org/wiki/Globally_unique_identifier#Algorithm (2) Best do it with a single card to avoid confusion over which is the primary one. (3) If the first byte of the address is odd then this is a multicast address. https://en.wikipedia.org/wiki/MAC_address#Address_details To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1119281/+subscriptions
Re: [Qemu-devel] [PATCH 1/1] sd: pl181: fix fifo count read support
On 18:44 Fri 25 Oct , Peter Maydell wrote: > On 25 October 2013 12:04, Jean-Christophe PLAGNIOL-VILLARD > wrote: > > On 11:33 Sat 19 Oct , Jean-Christophe PLAGNIOL-VILLARD wrote: > >> as it's depend on current direction > > > > ony change to get that applied? > > > > Barebox relay on it so it can work on both qemu and real hw > > I can't see anything obvious in the PL181 data sheet that > says this register should change behaviour like this based > on the direction of transfer, so I'm afraid I can't accept > this patch without a much more detailed analysis of why > it is correct. (Just as a for-starters, how does this change > relate to the comment immediately above that mentions vagueness > in the documentation and claims we don't need to emulate things > to an exact level of detail? Is this change supposed to fix > that? Does the comment need to change? Which bit of the > PL181 documentation describes the behaviour the patch is > affecting? etc) it's the register is supposed to report the status of the fifo and in qemu if always report the datacnt which is the number of byte to write and in case of reading it's always 0 so bootloader that use polling will never be get any data. And yes the readl hw does report the fifo lenght in case of read and write not always 0 when reading. Best Regards, J.
Re: [Qemu-devel] [PATCH 1/1] sd: pl181: fix fifo count read support
On 28 October 2013 13:24, Jean-Christophe PLAGNIOL-VILLARD wrote: > the register is supposed to report the status of the fifo > > and in qemu if always report the datacnt which is the number of byte to > write and in case of reading it's always 0 so bootloader that use polling will > never be get any data. No, datacnt in our implementation is the number of bytes to transfer: we use it to count number of bytes to read for read transfers (see pl181_fifo_run() which loops based on it both for pushing data into the fifo and for popping data out of it.) This is also the value we report as the hardware datacnt register, which according to the spec is used for both read and write transfers. -- PMM
Re: [Qemu-devel] [patch 1/2] qemu: mempath: prefault pages manually
Il 08/10/2013 02:41, Marcelo Tosatti ha scritto: > MAP_POPULATE mmap flag does not cause mmap to fail if allocation > of the entire area is not performed. HugeTLBfs performs reservation > of pages on a global basis: any further restriction to the reserved memory > such as cpusets placement or numa node policy is performed at > fault time only. > > Manually fault in pages at allocation time. This allows memory restrictions > to be applied before guest initialization. > > Signed-off-by: Marcelo Tosatti > > Index: qemu/exec.c > === > --- qemu.orig/exec.c > +++ qemu/exec.c Please produce individual patches with git format-patch. This lets "git am" do a 3-way merge, and would ignore automatically generated files such as qemu-options.def. Also: > @@ -918,6 +918,13 @@ static long gethugepagesize(const char * > return fs.f_bsize; > } > > +sigjmp_buf sigjump; Please make this static. > + > +static void sigbus_handler(int signal) > +{ > +siglongjmp(sigjump, 1); > +} > + > static void *file_ram_alloc(RAMBlock *block, > ram_addr_t memory, > const char *path) > @@ -927,9 +934,6 @@ static void *file_ram_alloc(RAMBlock *bl > char *c; > void *area; > int fd; > -#ifdef MAP_POPULATE > -int flags; > -#endif > unsigned long hpagesize; > > hpagesize = gethugepagesize(path); > @@ -977,21 +981,57 @@ static void *file_ram_alloc(RAMBlock *bl > if (ftruncate(fd, memory)) > perror("ftruncate"); > > -#ifdef MAP_POPULATE > -/* NB: MAP_POPULATE won't exhaustively alloc all phys pages in the case > - * MAP_PRIVATE is requested. For mem_prealloc we mmap as MAP_SHARED > - * to sidestep this quirk. > - */ > -flags = mem_prealloc ? MAP_POPULATE | MAP_SHARED : MAP_PRIVATE; > -area = mmap(0, memory, PROT_READ | PROT_WRITE, flags, fd, 0); > -#else > area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); > -#endif > if (area == MAP_FAILED) { > perror("file_ram_alloc: can't mmap RAM pages"); > close(fd); > return (NULL); > } > + > +if (mem_prealloc) { > +int ret, i; > +struct sigaction act, oldact; > +sigset_t set, oldset; > + > +memset(&act, 0, sizeof(act)); > +act.sa_handler = &sigbus_handler; > +act.sa_flags = 0; > + > +ret = sigaction(SIGBUS, &act, &oldact); > +if (ret) { > +perror("file_ram_alloc: fail to install signal handler"); > +exit(1); > +} > + > +/* unblock SIGBUS */ > +pthread_sigmask(SIG_BLOCK, NULL, &oldset); This is not needed, just pass &oldset in the SIG_UNBLOCK call below. > +sigemptyset(&set); > +sigaddset(&set, SIGBUS); > +pthread_sigmask(SIG_UNBLOCK, &set, NULL); > + > +if (sigsetjmp(sigjump, 1)) { > +fprintf(stderr, "file_ram_alloc: failed to preallocate pages\n"); > +exit(1); > +} > + > +/* MAP_POPULATE silently ignores failures */ > +for (i = 0; i < (memory/hpagesize)-1; i++) { > +memset(area + (hpagesize*i), 0, 1); > +} > + > +ret = sigaction(SIGBUS, &oldact, NULL); > +if (ret) { > +perror("file_ram_alloc: fail to reinstall signal handler"); > +exit(1); > +} > + > +if (sigismember(&oldset, SIGBUS)) { > +sigemptyset(&set); > +sigaddset(&set, SIGBUS); > +pthread_sigmask(SIG_BLOCK, &set, NULL); > +} Just use SIG_SETMASK with oldset, unconditionally. Ok with these changes. Paolo > +} > + > block->fd = fd; > return area; > } > Index: qemu/vl.c > === > --- qemu.orig/vl.c > +++ qemu/vl.c > @@ -188,9 +188,7 @@ static int display_remote; > const char* keyboard_layout = NULL; > ram_addr_t ram_size; > const char *mem_path = NULL; > -#ifdef MAP_POPULATE > int mem_prealloc = 0; /* force preallocation of physical target memory */ > -#endif > int nb_nics; > NICInfo nd_table[MAX_NICS]; > int autostart; > @@ -3205,11 +3203,9 @@ int main(int argc, char **argv, char **e > case QEMU_OPTION_mempath: > mem_path = optarg; > break; > -#ifdef MAP_POPULATE > case QEMU_OPTION_mem_prealloc: > mem_prealloc = 1; > break; > -#endif > case QEMU_OPTION_d: > log_mask = optarg; > break; > Index: qemu/qemu-options.def > === > --- qemu.orig/qemu-options.def > +++ qemu/qemu-options.def > @@ -66,11 +66,9 @@ stringify(DEFAULT_RAM_SIZE) "]\n", QEMU_ > DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath, > "-mem-path FILE provide backing storage for guest RAM\n", QEMU_ARCH_ALL) > > -#ifdef M
Re: [Qemu-devel] [patch 2/2] i386: pc: align gpa<->hpa on 1GB boundary
On Sun, Oct 27, 2013 at 04:20:44PM +0100, igor Mammedov wrote: > > Yes, thought of that, unfortunately its cumbersome to add an interface > > for the user to supply both 2MB and 1GB hugetlbfs pages. > Could 2Mb tails be automated, meaning if host uses 1Gb hugepages and > there is/are tail/s, QEMU should be able to figure out alignment > issues and allocate with appropriate pages. Yes that would be ideal but the problem with hugetlbfs is that pages are preallocated. So in the end you'd have to expose the split of guest RAM in 2MB/1GB types to the user (it would be necessary for the user to calculate the size of the hole, etc). > Goal is separate host part allocation aspect from guest related one, > aliasing 32-bit hole size at the end doesn't help it at all, it's quite > opposite, it's making current code more complicated and harder to fix > in the future. You can simply back the 1GB areas which the hole reside with 2MB pages. Can't see why having the tail of RAM map to the hole is problematic. Understand your concern, but the complication is necessary: the host virtual/physical address and guest physical addresses must be aligned on largepage boundaries. Do you foresee any problem with memory hotplug? Could add a warning to memory API: if memory region is larger than 1GB and RAM is 1GB backed, and not properly aligned, warn.
Re: [Qemu-devel] [patch 2/2] i386: pc: align gpa<->hpa on 1GB boundary
On Mon, Oct 28, 2013 at 12:04:06PM -0200, Marcelo Tosatti wrote: > On Sun, Oct 27, 2013 at 04:20:44PM +0100, igor Mammedov wrote: > > > Yes, thought of that, unfortunately its cumbersome to add an interface > > > for the user to supply both 2MB and 1GB hugetlbfs pages. > > Could 2Mb tails be automated, meaning if host uses 1Gb hugepages and > > there is/are tail/s, QEMU should be able to figure out alignment > > issues and allocate with appropriate pages. > > Yes that would be ideal but the problem with hugetlbfs is that pages are > preallocated. > > So in the end you'd have to expose the split of guest RAM in 2MB/1GB types > to the user (it would be necessary for the user to calculate the size of > the hole, etc). Note the assumption here is this: its easier for the hugetlbfs user to manage number of 1GB hugepages = size of guest RAM Than for him to calculate the size of the hole (which depends on machine type), allocate 1GB/2MB hugepages accordingly. And the benefit of that would be to save one 1GB hugepage (which is preallocated during boot, in the first place). So matching number of 1GB hugepages and guest RAM seems the easier choice. > > Goal is separate host part allocation aspect from guest related one, > > aliasing 32-bit hole size at the end doesn't help it at all, it's quite > > opposite, it's making current code more complicated and harder to fix > > in the future. What is the problem to be fixed, exactly? > You can simply back the 1GB areas which the hole reside with 2MB pages. > Can't see why having the tail of RAM map to the hole is problematic. > > Understand your concern, but the complication is necessary: the host > virtual/physical address and guest physical addresses must be aligned on > largepage boundaries. > > Do you foresee any problem with memory hotplug? > > Could add a warning to memory API: if memory region is larger than 1GB > and RAM is 1GB backed, and not properly aligned, warn.
Re: [Qemu-devel] About VM fork in QEMU
On 10/26/2013 11:37 AM, Xinyang Ge wrote: >> External snapshots (via the blockdev-snapshot-sync QMP command) can be >> taken in a matter of milliseconds if you only care about disk state. >> Furthermore, if you want to take a snapshot of both memory and disk >> state, such that the clone can be resumed from the same time, you can do >> that with a guest downtime that only lasts as long as the >> blockdev-snapshot-sync, by first doing a migrate to file then doing the >> disk snapshot when the VM pauses at the end of migration. Resuming the >> original guest is fast; resuming from the migration file is a bit >> longer, but it is still the fastest way possible to resume from a >> memory+disk snapshot. If you need anything faster, then yes, you would >> have to write patches to qemu to attempt cloning via fork() that makes >> sure to modify the active disk in use by the fork child so as not to >> interfere with the fork parent. > > I think migrating memory to file then doing external disk snapshot is > exactly what we want. Since we are using libvirt to manage different > VMs, could you give us some specific guides (or references) that how > we could migrate memory state to file using virsh interfaces and do > external snapshots? virsh snapshot-create-as $dom $name --live --memspec /path/to/memoryfile Libvirt usage questions might be better directed to the libvirt lists. -- 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: Don't copy backing file name on error
Le Saturday 26 Oct 2013 à 15:44:43 (+0200), Max Reitz a écrit : > bdrv_open_backing_file() tries to copy the backing file name using > pstrcpy directly after calling bdrv_open() to open the backing file > without checking whether that was actually successful. If it was not, > ps->backing_hd->file will probably be NULL and qemu will crash. > > Fix this by moving pstrcpy after checking whether bdrv_open() succeeded. > > Signed-off-by: Max Reitz > --- > block.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block.c b/block.c > index 4474012..61795fe 100644 > --- a/block.c > +++ b/block.c > @@ -1005,8 +1005,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict > *options, Error **errp) > ret = bdrv_open(bs->backing_hd, > *backing_filename ? backing_filename : NULL, options, > back_flags, back_drv, &local_err); > -pstrcpy(bs->backing_file, sizeof(bs->backing_file), > -bs->backing_hd->file->filename); > if (ret < 0) { > bdrv_unref(bs->backing_hd); > bs->backing_hd = NULL; > @@ -1014,6 +1012,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict > *options, Error **errp) > error_propagate(errp, local_err); > return ret; > } > +pstrcpy(bs->backing_file, sizeof(bs->backing_file), > +bs->backing_hd->file->filename); > return 0; > } > > -- > 1.8.4.1 > > Reviewed-by: Benoit Canet
Re: [Qemu-devel] [PATCH v2] qcow2: Flush image after creation
Le Thursday 24 Oct 2013 à 20:35:06 (+0200), Max Reitz a écrit : > Opening the qcow2 image with BDRV_O_NO_FLUSH prevents any flushes during > the image creation. This means that the image has not yet been flushed > to disk when qemu-img create exits. This flush is delayed until the next > operation on the image involving opening it without BDRV_O_NO_FLUSH and > closing (or directly flushing) it. For large images and/or images with a > small cluster size and preallocated metadata, this flush may take a > significant amount of time and may occur unexpectedly. > > Reopening the image without BDRV_O_NO_FLUSH right before the end of > qcow2_create2() results in hoisting the potentially costly flush into > the image creation, which is expected to take some time (whereas > successive image operations may be not). > > Signed-off-by: Max Reitz > Reviewed-by: Eric Blake > --- > v2: > - implemented Kevin's remarks: >- left ret = 0; in the code >- added goto out; in error handling of bdrv_open > - implemented Eric's remarks: >- changed indentation of bdrv_open arguments >- s/preponing/hoisting/ in the commit message > --- > block/qcow2.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/block/qcow2.c b/block/qcow2.c > index 01269f9..6e5d98d 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1584,6 +1584,16 @@ static int qcow2_create2(const char *filename, int64_t > total_size, > } > } > > +bdrv_close(bs); > + > +/* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning > */ > +ret = bdrv_open(bs, filename, NULL, > +BDRV_O_RDWR | BDRV_O_CACHE_WB, drv, &local_err); > +if (error_is_set(&local_err)) { > +error_propagate(errp, local_err); > +goto out; > +} > + > ret = 0; > out: > bdrv_unref(bs); > -- > 1.8.3.1 > > Reviewed-by: Benoit Canet
Re: [Qemu-devel] [PATCH] Fix COR by disabling BDRV_O_COPY_ON_READ before opening the backing_file.
Le Friday 25 Oct 2013 à 02:15:07 (+0200), Thibaut LAURENT a écrit : > Since commit 0ebd24e0a203cf2852c310b59fbe050190dc6c8c, > bdrv_open_common will throw an error when trying to open a file > read-only with the BDRV_O_COPY_ON_READ flag set. > Although BDRV_O_RDWR is unset for the backing files, > BDRV_O_COPY_ON_READ is still passed on if copy-on-read was requested > for the drive. Let's unset this flag too before opening the backing > file, or bdrv_open_common will fail. > > Signed-off-by: Thibaut LAURENT > --- > block.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/block.c b/block.c > index fd05a80..4474012 100644 > --- a/block.c > +++ b/block.c > @@ -999,7 +999,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict > *options, Error **errp) > } > > /* backing files always opened read-only */ > -back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT); > +back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | > +BDRV_O_COPY_ON_READ); > > ret = bdrv_open(bs->backing_hd, > *backing_filename ? backing_filename : NULL, options, > -- > 1.8.4.1 > > Reviewed-by: Benoit Canet
[Qemu-devel] How to introduce bs->node_name ?
Hi list, After a discussion on irc we have two potential solution in order to introduce a new bs->node_name member in order to be able to manipulate the graph from the monitors. The first one is to make the QMP device parameter of the block commands optional and add the node-name parameter as a second optional parameter. This is Markus prefered solution and Eric is ok with making mandatory parameters optional in QMP. The second one suggested by Kevin Would be to add some magic to the new node_name member by making it equal to device_name for backends and then making the qmp commands operate only on node-names. My personnal suggestion would be that non specified node-name would be set to "undefined" meaning that no operation could occur on this bs. For QMP access the device_name is accessed via bdrv_find() in a few place in blockdev. Here are the occurences of it: commit -- void do_commit(Monitor *mon, const QDict *qdict) { const char *device = qdict_get_str(qdict, "device"); BlockDriverState *bs; int ret; if (!strcmp(device, "all")) { ret = bdrv_commit_all(); } else { bs = bdrv_find(device); if (!bs) { monitor_printf(mon, "Device '%s' not found\n", device); return; } ret = bdrv_commit(bs); } if (ret < 0) { monitor_printf(mon, "'commit' error for '%s': %s\n", device, strerror(-ret)); } } internal snapshot deletion -- SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device, bool has_id, const char *id, bool has_name, const char *name, Error **errp) { BlockDriverState *bs = bdrv_find(device); QEMUSnapshotInfo sn; Error *local_err = NULL; SnapshotInfo *info = NULL; Internal snapshot preparation - static void internal_snapshot_prepare(BlkTransactionState *common, Error **errp) { const char *device; const char *name; BlockDriverState *bs; QEMUSnapshotInfo old_sn, *sn; bool ret; qemu_timeval tv; BlockdevSnapshotInternal *internal; InternalSnapshotState *state; int ret1; g_assert(common->action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC); internal = common->action->blockdev_snapshot_internal_sync; state = DO_UPCAST(InternalSnapshotState, common, common); /* 1. parse input */ device = internal->device; name = internal->name; /* 2. check for validation */ bs = bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } Drive backup static void drive_backup_prepare(BlkTransactionState *common, Error **errp) { DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common); DriveBackup *backup; Error *local_err = NULL; assert(common->action->kind == TRANSACTION_ACTION_KIND_DRIVE_BACKUP); backup = common->action->drive_backup; qmp_drive_backup(backup->device, backup->target, backup->has_format, backup->format, backup->sync, backup->has_mode, backup->mode, backup->has_speed, backup->speed, backup->has_on_source_error, backup->on_source_error, backup->has_on_target_error, backup->on_target_error, &local_err); if (error_is_set(&local_err)) { error_propagate(errp, local_err); state->bs = NULL; state->job = NULL; return; } state->bs = bdrv_find(backup->device); state->job = state->bs->job; } Eject which should operate on backends -- void qmp_eject(const char *device, bool has_force, bool force, Error **errp) { BlockDriverState *bs; bs = bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } eject_device(bs, force, errp); } QCow2 crypto void qmp_block_passwd(const char *device, const char *password, Error **errp) { BlockDriverState *bs; int err; bs = bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); return; } err = bdrv_set_key(bs, password); if (err == -EINVAL) { error_set(errp, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs)); return; } else if (err < 0) { error_set(errp, QERR_INVALID_PASSWORD); return; } } Change blockdev (I don't know what it is used for) -- void qmp_change_blockdev(const char *device, con
Re: [Qemu-devel] [patch 1/2] qemu: mempath: prefault pages manually
On 8 October 2013 01:41, Marcelo Tosatti wrote: > +ret = sigaction(SIGBUS, &oldact, NULL); > +if (ret) { > +perror("file_ram_alloc: fail to reinstall signal handler"); "failed". thanks -- PMM
[Qemu-devel] [PATCH 1.7] timers: fix stop/cont with -icount
Stop/cont commands are broken with -icount due to a deadlock. The real problem is that the computation of timers_state.cpu_ticks_offset makes no sense with -icount enabled: we set it to an icount clock value in cpu_disable_ticks, and subtract a TSC (or similar, whatever cpu_get_real_ticks happens to return) value in cpu_enable_ticks. The fix is simple. timers_state.cpu_ticks_offset is only used together with cpu_get_real_ticks, so we can use cpu_get_real_ticks in cpu_disable_ticks. There is no need to update cpu_ticks_prev at the time cpu_disable_ticks is called; instead, we can do it the next time cpu_get_ticks is called. The change to cpu_disable_ticks is the important part of the patch. The rest modifies the code to always check timers_state.cpu_ticks_prev, even when the ticks are not advancing (i.e. the VM is stopped). It also makes a similar change to cpu_get_clock_locked, so that the code remains similar for cpu_get_ticks and cpu_get_clock_locked. Signed-off-by: Paolo Bonzini --- cpus.c | 42 ++ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/cpus.c b/cpus.c index 398229e..c2c6864 100644 --- a/cpus.c +++ b/cpus.c @@ -165,36 +165,38 @@ int64_t cpu_get_icount(void) /* Caller must hold the BQL */ int64_t cpu_get_ticks(void) { +int64_t ticks; + if (use_icount) { return cpu_get_icount(); } -if (!timers_state.cpu_ticks_enabled) { -return timers_state.cpu_ticks_offset; -} else { -int64_t ticks; -ticks = cpu_get_real_ticks(); -if (timers_state.cpu_ticks_prev > ticks) { -/* Note: non increasing ticks may happen if the host uses - software suspend */ -timers_state.cpu_ticks_offset += timers_state.cpu_ticks_prev - ticks; -} -timers_state.cpu_ticks_prev = ticks; -return ticks + timers_state.cpu_ticks_offset; + +ticks = timers_state.cpu_ticks_offset; +if (timers_state.cpu_ticks_enabled) { +ticks += cpu_get_real_ticks(); +} + +if (timers_state.cpu_ticks_prev > ticks) { +/* Note: non increasing ticks may happen if the host uses + software suspend */ +timers_state.cpu_ticks_offset += timers_state.cpu_ticks_prev - ticks; +ticks = timers_state.cpu_ticks_prev; } + +timers_state.cpu_ticks_prev = ticks; +return ticks; } static int64_t cpu_get_clock_locked(void) { -int64_t ti; +int64_t ticks; -if (!timers_state.cpu_ticks_enabled) { -ti = timers_state.cpu_clock_offset; -} else { -ti = get_clock(); -ti += timers_state.cpu_clock_offset; +ticks = timers_state.cpu_clock_offset; +if (timers_state.cpu_ticks_enabled) { +ticks += get_clock(); } -return ti; +return ticks; } /* return the host CPU monotonic timer and handle stop/restart */ @@ -235,7 +237,7 @@ void cpu_disable_ticks(void) /* Here, the really thing protected by seqlock is cpu_clock_offset. */ seqlock_write_lock(&timers_state.vm_clock_seqlock); if (timers_state.cpu_ticks_enabled) { -timers_state.cpu_ticks_offset = cpu_get_ticks(); +timers_state.cpu_ticks_offset += cpu_get_real_ticks(); timers_state.cpu_clock_offset = cpu_get_clock_locked(); timers_state.cpu_ticks_enabled = 0; } -- 1.8.3.1
Re: [Qemu-devel] [PATCH] block: Don't copy backing file name on error
Am 26.10.2013 um 15:44 hat Max Reitz geschrieben: > bdrv_open_backing_file() tries to copy the backing file name using > pstrcpy directly after calling bdrv_open() to open the backing file > without checking whether that was actually successful. If it was not, > ps->backing_hd->file will probably be NULL and qemu will crash. > > Fix this by moving pstrcpy after checking whether bdrv_open() succeeded. > > Signed-off-by: Max Reitz Thanks, applied. A test case wouldn't hurt, though. Kevin
Re: [Qemu-devel] [sheepdog] [PATCH v2 0/2] sheepdog: make use of copy_policy
Am 25.10.2013 um 20:03 hat MORITA Kazutaka geschrieben: > At Wed, 23 Oct 2013 16:51:50 +0800, > Liu Yuan wrote: > > > > v2: > > - merge the reserved bits > > > > This patch set makes use of copy_policy in struct SheepdogInode in order to > > support recently introduced erasure coding volume in sheepdog. > > > > Thanks > > Yuan > > > > Liu Yuan (2): > > sheepdog: explicitly set copies as type uint8_t > > sheepdog: pass copy_policy in the request > > > > block/sheepdog.c | 30 +++--- > > 1 file changed, 19 insertions(+), 11 deletions(-) > > Acked-by: MORITA Kazutaka Thanks, applied both patches to the block branch. Kevin
[Qemu-devel] [WIP PATCH 00/24] IDE cleanups, initial work on AHCI rerror/werror=stop
Hi all, this is some cleanup to the IDE code that I started long ago and picked up again on my flight back from KVM Forum... The patches clean up the callbacks to prepare for implementing request retry for AHCI. After these patches, PIO retry should work for AHCI and non-NCQ retry should be very close (you need an implementation of the new restart_dma callback). NCQ retry and save/restore is AHCI-specific and has to be done separately, similar to how we do it for SCSI already. Patches 1-4 set up a new (incomplete) testcase. Patches 5-13 are no-op cleanups, or so I hope. Patches 14-23 genericize retry and make it "just work" for all adapters for PIO. Patch 24 is another cleanup that could probably be moved earlier in the series. Throwing it out since I know Kevin and mst are working on AHCI, to avoid doing duplicate work. Patches 2 and 4 are known to be racy and incomplete. Extremely light testing, if something breaks tells me and I'll look at it. Paolo Paolo Bonzini (24): blkdebug: report errors on flush too libqtest: return progress from qmp/qmpv libqtest: add QTEST_LOG for debugging qtest testcases ide-test: add test for werror=stop ide: simplify reset callbacks ide: simplify set_inactive callbacks ide: simplify async_cmd_done callbacks ide: simplify start_transfer callbacks ide: wrap start_dma callback ide: add trigger_irq callback ide: fold add_status callback into set_inactive ide: move BM_STATUS bits to pci.[ch] ide: move retry constants out of BM_STATUS_* namespace ide: start extracting ide_restart_dma out of bmdma_restart_dma ide: prepare to move restart to common code ide: introduce ide_register_restart_cb ide: do not use BMDMA in restart callback ide: pass IDEBus to the restart_cb ide: move restart callback to common code ide: remove restart_cb callback ide: replace set_unit callback with more IDEBus state ide: place initial state of the current request to IDEBus ide: migrate initial request state via IDEBus ide: commonize io_buffer_index initialization block/blkdebug.c | 20 hw/ide/ahci.c | 45 +++- hw/ide/atapi.c| 14 +++-- hw/ide/cmd646.c | 3 +- hw/ide/core.c | 151 +++--- hw/ide/internal.h | 48 - hw/ide/macio.c| 15 -- hw/ide/pci.c | 130 +++--- hw/ide/pci.h | 19 --- hw/ide/piix.c | 3 +- hw/ide/via.c | 3 +- tests/ide-test.c | 74 ++ tests/libqtest.c | 17 -- tests/libqtest.h | 17 +++--- 14 files changed, 307 insertions(+), 252 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH 01/24] blkdebug: report errors on flush too
Signed-off-by: Paolo Bonzini --- block/blkdebug.c | 20 1 file changed, 20 insertions(+) diff --git a/block/blkdebug.c b/block/blkdebug.c index 16d2b91..4626c43 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -484,6 +484,25 @@ static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs, return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque); } +static BlockDriverAIOCB *blkdebug_aio_flush(BlockDriverState *bs, +BlockDriverCompletionFunc *cb, void *opaque) +{ +BDRVBlkdebugState *s = bs->opaque; +BlkdebugRule *rule = NULL; + +QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) { +if (rule->options.inject.sector == -1) { +break; +} +} + +if (rule && rule->options.inject.error) { +return inject_error(bs, cb, opaque, rule); +} + +return bdrv_aio_flush(bs->file, cb, opaque); +} + static void blkdebug_close(BlockDriverState *bs) { @@ -636,6 +655,7 @@ static BlockDriver bdrv_blkdebug = { .bdrv_aio_readv = blkdebug_aio_readv, .bdrv_aio_writev= blkdebug_aio_writev, +.bdrv_aio_flush = blkdebug_aio_flush, .bdrv_debug_event = blkdebug_debug_event, .bdrv_debug_breakpoint = blkdebug_debug_breakpoint, -- 1.8.3.1
[Qemu-devel] [PATCH 09/24] ide: wrap start_dma callback
Make it optional and prepare for the next patches. Signed-off-by: Paolo Bonzini --- hw/ide/atapi.c| 6 ++ hw/ide/core.c | 15 --- hw/ide/internal.h | 1 + 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index 05e60b1..a7688bf 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -255,8 +255,7 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int max_size) if (s->atapi_dma) { bdrv_acct_start(s->bs, &s->acct, size, BDRV_ACCT_READ); s->status = READY_STAT | SEEK_STAT | DRQ_STAT; -s->bus->dma->ops->start_dma(s->bus->dma, s, - ide_atapi_cmd_read_dma_cb); +ide_start_dma(s, ide_atapi_cmd_read_dma_cb); } else { s->status = READY_STAT | SEEK_STAT; ide_atapi_cmd_reply_end(s); @@ -375,8 +374,7 @@ static void ide_atapi_cmd_read_dma(IDEState *s, int lba, int nb_sectors, /* XXX: check if BUSY_STAT should be set */ s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT; -s->bus->dma->ops->start_dma(s->bus->dma, s, - ide_atapi_cmd_read_dma_cb); +ide_start_dma(s, ide_atapi_cmd_read_dma_cb); } static void ide_atapi_cmd_read(IDEState *s, int lba, int nb_sectors, diff --git a/hw/ide/core.c b/hw/ide/core.c index e64dfff..bd4a9f2 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -722,7 +722,14 @@ static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd) break; } -s->bus->dma->ops->start_dma(s->bus->dma, s, ide_dma_cb); +ide_start_dma(s, ide_dma_cb); +} + +void ide_start_dma(IDEState *s, BlockDriverCompletionFunc *cb) +{ +if (s->bus->dma->ops->start_dma) { +s->bus->dma->ops->start_dma(s->bus->dma, s, cb); +} } static void ide_sector_write_timer_cb(void *opaque) @@ -2173,11 +2180,6 @@ static void ide_init1(IDEBus *bus, int unit) ide_sector_write_timer_cb, s); } -static void ide_nop_start(IDEDMA *dma, IDEState *s, - BlockDriverCompletionFunc *cb) -{ -} - static int ide_nop_int(IDEDMA *dma, int x) { return 0; @@ -2188,7 +2190,6 @@ static void ide_nop_restart(void *opaque, int x, RunState y) } static const IDEDMAOps ide_dma_nop_ops = { -.start_dma = ide_nop_start, .prepare_buf= ide_nop_int, .rw_buf = ide_nop_int, .set_unit = ide_nop_int, diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 92d2351..92f25ef 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -532,6 +532,7 @@ void ide_bus_reset(IDEBus *bus); int64_t ide_get_sector(IDEState *s); void ide_set_sector(IDEState *s, int64_t sector_num); +void ide_start_dma(IDEState *s, BlockDriverCompletionFunc *cb); void ide_dma_error(IDEState *s); void ide_atapi_cmd_ok(IDEState *s); -- 1.8.3.1
[Qemu-devel] [PATCH 03/24] libqtest: add QTEST_LOG for debugging qtest testcases
Signed-off-by: Paolo Bonzini --- tests/libqtest.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/libqtest.c b/tests/libqtest.c index 5205a43..6e42d01 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -129,11 +129,12 @@ QTestState *qtest_init(const char *extra_args) if (pid == 0) { command = g_strdup_printf("%s " "-qtest unix:%s,nowait " - "-qtest-log /dev/null " + "-qtest-log %s " "-qmp unix:%s,nowait " "-pidfile %s " "-machine accel=qtest " "%s", qemu_binary, s->socket_path, + getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null", s->qmp_socket_path, pid_file, extra_args ?: ""); execlp("/bin/sh", "sh", "-c", command, NULL); @@ -295,6 +296,7 @@ bool qtest_qmpv(QTestState *s, const char *fmt, va_list ap) { bool has_reply = false; int nesting = 0; +int log = getenv("QTEST_LOG") != NULL; /* Send QMP request */ socket_sendf(s->qmp_fd, fmt, ap); @@ -314,6 +316,9 @@ bool qtest_qmpv(QTestState *s, const char *fmt, va_list ap) exit(1); } +if (log) { +len = write(2, &c, 1); +} switch (c) { case '{': nesting++; -- 1.8.3.1
[Qemu-devel] [PATCH 15/24] ide: prepare to move restart to common code
Using ide_start_dma instead of bmdma_start_dma introduces a new assignment "s->bus->dma->unit = s->unit". This introduces no change because ide_handle_rw_error has already done the same assignment. Signed-off-by: Paolo Bonzini --- hw/ide/internal.h | 1 + hw/ide/pci.c | 12 +++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 041f173..96969d9 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -432,6 +432,7 @@ struct IDEDMAOps { DMAIntFunc *prepare_buf; DMAIntFunc *rw_buf; DMAIntFunc *set_unit; +DMAVoidFunc *restart_dma; DMAVoidFunc *trigger_irq; DMAStopFunc *set_inactive; DMAVoidFunc *async_cmd_done; diff --git a/hw/ide/pci.c b/hw/ide/pci.c index c7fbe51..4a1328d 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -175,8 +175,9 @@ static void bmdma_set_inactive(IDEDMA *dma, bool more) } } -static void bmdma_restart_dma(BMDMAState *bm) +static void bmdma_restart_dma(IDEDMA *dma) { +BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); IDEState *s = bmdma_active_if(bm); ide_set_sector(s, bm->sector_num); @@ -186,13 +187,13 @@ static void bmdma_restart_dma(BMDMAState *bm) static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd) { -BMDMAState *bm = DO_UPCAST(BMDMAState, dma, s->bus->dma); - -bmdma_restart_dma(bm); +if (s->bus->dma->ops->restart_dma) { +s->bus->dma->ops->restart_dma(s->bus->dma); +} s->io_buffer_index = 0; s->io_buffer_size = 0; s->dma_cmd = dma_cmd; -bmdma_start_dma(&bm->dma, s, ide_dma_cb); +ide_start_dma(s, ide_dma_cb); } /* TODO This should be common IDE code */ @@ -516,6 +517,7 @@ static const struct IDEDMAOps bmdma_ops = { .prepare_buf = bmdma_prepare_buf, .rw_buf = bmdma_rw_buf, .set_unit = bmdma_set_unit, +.restart_dma = bmdma_restart_dma, .trigger_irq = bmdma_trigger_irq, .set_inactive = bmdma_set_inactive, .restart_cb = bmdma_restart_cb, -- 1.8.3.1
[Qemu-devel] [PATCH 05/24] ide: simplify reset callbacks
Drop the unused return value and make the callback optional. Signed-off-by: Paolo Bonzini --- hw/ide/ahci.c | 6 -- hw/ide/core.c | 5 +++-- hw/ide/internal.h | 3 ++- hw/ide/macio.c| 1 - hw/ide/pci.c | 4 +--- 5 files changed, 6 insertions(+), 13 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index a8be62c..d74b6ab 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1137,11 +1137,6 @@ static void ahci_dma_restart_cb(void *opaque, int running, RunState state) { } -static int ahci_dma_reset(IDEDMA *dma) -{ -return 0; -} - static const IDEDMAOps ahci_dma_ops = { .start_dma = ahci_start_dma, .start_transfer = ahci_start_transfer, @@ -1152,7 +1147,6 @@ static const IDEDMAOps ahci_dma_ops = { .set_inactive = ahci_dma_set_inactive, .async_cmd_done = ahci_async_cmd_done, .restart_cb = ahci_dma_restart_cb, -.reset = ahci_dma_reset, }; void ahci_init(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports) diff --git a/hw/ide/core.c b/hw/ide/core.c index 399b1ba..8437ead 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2057,7 +2057,9 @@ void ide_bus_reset(IDEBus *bus) } /* reset dma provider too */ -bus->dma->ops->reset(bus->dma); +if (bus->dma->ops->reset) { +bus->dma->ops->reset(bus->dma); +} } static bool ide_cd_is_tray_open(void *opaque) @@ -2195,7 +2197,6 @@ static const IDEDMAOps ide_dma_nop_ops = { .add_status = ide_nop_int, .set_inactive = ide_nop, .restart_cb = ide_nop_restart, -.reset = ide_nop, }; static IDEDMA ide_dma_nop = { diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 5d1cf87..84f3a7a 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -320,6 +320,7 @@ typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind; typedef void EndTransferFunc(IDEState *); typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockDriverCompletionFunc *); +typedef void DMAVoidFunc(IDEDMA *); typedef int DMAFunc(IDEDMA *); typedef int DMAIntFunc(IDEDMA *, int); typedef void DMARestartFunc(void *, int, RunState); @@ -435,7 +436,7 @@ struct IDEDMAOps { DMAFunc *set_inactive; DMAFunc *async_cmd_done; DMARestartFunc *restart_cb; -DMAFunc *reset; +DMAVoidFunc *reset; }; struct IDEDMA { diff --git a/hw/ide/macio.c b/hw/ide/macio.c index da94580..b3cb6ca 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -528,7 +528,6 @@ static const IDEDMAOps dbdma_ops = { .add_status = ide_nop_int, .set_inactive = ide_nop, .restart_cb = ide_nop_restart, -.reset = ide_nop, }; static void macio_ide_realizefn(DeviceState *dev, Error **errp) diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 91151fc..a9eb5bf 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -247,7 +247,7 @@ static void bmdma_cancel(BMDMAState *bm) } } -static int bmdma_reset(IDEDMA *dma) +static void bmdma_reset(IDEDMA *dma) { BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); @@ -264,8 +264,6 @@ static int bmdma_reset(IDEDMA *dma) bm->cur_prd_len = 0; bm->sector_num = 0; bm->nsector = 0; - -return 0; } static int bmdma_start_transfer(IDEDMA *dma) -- 1.8.3.1
[Qemu-devel] [PATCH 12/24] ide: move BM_STATUS bits to pci.[ch]
They are not used by AHCI, and should not be even available there. Signed-off-by: Paolo Bonzini --- hw/ide/internal.h | 11 --- hw/ide/pci.c | 4 hw/ide/pci.h | 7 +++ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/hw/ide/internal.h b/hw/ide/internal.h index cd5e77c..f2d8ccc 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -485,10 +485,6 @@ struct IDEDevice { uint64_t wwn; }; -#define BM_STATUS_DMAING 0x01 -#define BM_STATUS_ERROR 0x02 -#define BM_STATUS_INT0x04 - /* FIXME These are not status register bits */ #define BM_STATUS_DMA_RETRY 0x08 #define BM_STATUS_PIO_RETRY 0x10 @@ -496,13 +492,6 @@ struct IDEDevice { #define BM_STATUS_RETRY_FLUSH 0x40 #define BM_STATUS_RETRY_TRIM 0x80 -#define BM_MIGRATION_COMPAT_STATUS_BITS \ -(BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \ -BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH) - -#define BM_CMD_START 0x01 -#define BM_CMD_READ 0x08 - static inline IDEState *idebus_active_if(IDEBus *bus) { return bus->ifs + bus->unit; diff --git a/hw/ide/pci.c b/hw/ide/pci.c index e4fcf5a..e61a82f 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -33,6 +33,10 @@ #define BMDMA_PAGE_SIZE 4096 +#define BM_MIGRATION_COMPAT_STATUS_BITS \ +(BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \ +BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH) + static void bmdma_start_dma(IDEDMA *dma, IDEState *s, BlockDriverCompletionFunc *dma_cb) { diff --git a/hw/ide/pci.h b/hw/ide/pci.h index 2428275..517711f 100644 --- a/hw/ide/pci.h +++ b/hw/ide/pci.h @@ -3,6 +3,13 @@ #include +#define BM_STATUS_DMAING 0x01 +#define BM_STATUS_ERROR 0x02 +#define BM_STATUS_INT0x04 + +#define BM_CMD_START 0x01 +#define BM_CMD_READ 0x08 + typedef struct BMDMAState { IDEDMA dma; uint8_t cmd; -- 1.8.3.1
[Qemu-devel] [PATCH 07/24] ide: simplify async_cmd_done callbacks
Drop the unused return value. Signed-off-by: Paolo Bonzini --- hw/ide/ahci.c | 2 +- hw/ide/internal.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 265e3f2..b4b8386 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1106,7 +1106,7 @@ static int ahci_dma_add_status(IDEDMA *dma, int status) return 0; } -static int ahci_async_cmd_done(IDEDMA *dma) +static void ahci_async_cmd_done(IDEDMA *dma) { AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 7c68f8e..43b8f43 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -434,7 +434,7 @@ struct IDEDMAOps { DMAIntFunc *set_unit; DMAIntFunc *add_status; DMAVoidFunc *set_inactive; -DMAFunc *async_cmd_done; +DMAVoidFunc *async_cmd_done; DMARestartFunc *restart_cb; DMAVoidFunc *reset; }; -- 1.8.3.1
[Qemu-devel] [PATCH 17/24] ide: do not use BMDMA in restart callback
Whenever an error stops the VM, ide_handle_rw_error does "s->bus->dma->unit = s->unit". So we can just use idebus_active_if. Signed-off-by: Paolo Bonzini --- hw/ide/pci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 4a1328d..4df50dc 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -208,17 +208,17 @@ static void bmdma_restart_bh(void *opaque) qemu_bh_delete(bm->bh); bm->bh = NULL; -if (bm->unit == (uint8_t) -1) { +error_status = bus->error_status; +if (bus->error_status == 0) { return; } -s = bmdma_active_if(bm); +s = idebus_active_if(bus); is_read = (bus->error_status & IDE_RETRY_READ) != 0; /* The error status must be cleared before resubmitting the request: The * request may fail again, and this case can only be distinguished if the * called function can set a new error status. */ -error_status = bus->error_status; bus->error_status = 0; if (error_status & IDE_RETRY_DMA) { -- 1.8.3.1
[Qemu-devel] [PATCH 16/24] ide: introduce ide_register_restart_cb
Signed-off-by: Paolo Bonzini --- hw/ide/cmd646.c | 3 +-- hw/ide/core.c | 5 + hw/ide/internal.h | 1 + hw/ide/piix.c | 3 +-- hw/ide/via.c | 3 +-- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c index 0500a7a..4753543 100644 --- a/hw/ide/cmd646.c +++ b/hw/ide/cmd646.c @@ -294,8 +294,7 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev) bmdma_init(&d->bus[i], &d->bmdma[i], d); d->bmdma[i].bus = &d->bus[i]; -qemu_add_vm_change_state_handler(d->bus[i].dma->ops->restart_cb, - &d->bmdma[i].dma); +ide_register_restart_cb(&d->bus[i]); } vmstate_register(DEVICE(dev), 0, &vmstate_ide_pci, d); diff --git a/hw/ide/core.c b/hw/ide/core.c index 1e3108c..93cfd46 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2193,6 +2193,11 @@ static const IDEDMAOps ide_dma_nop_ops = { .restart_cb = ide_nop_restart, }; +void ide_register_restart_cb(IDEBus *bus) +{ +qemu_add_vm_change_state_handler(bus->dma->ops->restart_cb, bus->dma); +} + static IDEDMA ide_dma_nop = { .ops = &ide_dma_nop_ops, .aiocb = NULL, diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 96969d9..678b33c 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -548,6 +548,7 @@ void ide_init2(IDEBus *bus, qemu_irq irq); void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1, qemu_irq irq); void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2); +void ide_register_restart_cb(IDEBus *bus); void ide_exec_cmd(IDEBus *bus, uint32_t val); void ide_dma_cb(void *opaque, int ret); diff --git a/hw/ide/piix.c b/hw/ide/piix.c index ab36749..d7f5118 100644 --- a/hw/ide/piix.c +++ b/hw/ide/piix.c @@ -143,8 +143,7 @@ static void pci_piix_init_ports(PCIIDEState *d) { bmdma_init(&d->bus[i], &d->bmdma[i], d); d->bmdma[i].bus = &d->bus[i]; -qemu_add_vm_change_state_handler(d->bus[i].dma->ops->restart_cb, - &d->bmdma[i].dma); +ide_register_restart_cb(&d->bus[i]); } } diff --git a/hw/ide/via.c b/hw/ide/via.c index 99468c7..ae52164 100644 --- a/hw/ide/via.c +++ b/hw/ide/via.c @@ -166,8 +166,7 @@ static void vt82c686b_init_ports(PCIIDEState *d) { bmdma_init(&d->bus[i], &d->bmdma[i], d); d->bmdma[i].bus = &d->bus[i]; -qemu_add_vm_change_state_handler(d->bus[i].dma->ops->restart_cb, - &d->bmdma[i].dma); +ide_register_restart_cb(&d->bus[i]); } } -- 1.8.3.1
[Qemu-devel] [PATCH 10/24] ide: add trigger_irq callback
AHCIDevice does not have a dma_status field. The add_status callback thus does not make sense, start moving its functionality to new callbacks. Signed-off-by: Paolo Bonzini --- hw/ide/ahci.c | 12 hw/ide/atapi.c| 4 +++- hw/ide/internal.h | 1 + hw/ide/pci.c | 7 +++ 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 23f3f22..7b47053 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1097,13 +1097,16 @@ static int ahci_dma_add_status(IDEDMA *dma, int status) AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); DPRINTF(ad->port_no, "set status: %x\n", status); -if (status & BM_STATUS_INT) { -ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_DSS); -} - return 0; } +static void ahci_dma_trigger_irq(IDEDMA *dma) +{ +AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); +DPRINTF(ad->port_no, "trigger irq\n"); +ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_DSS); +} + static void ahci_async_cmd_done(IDEDMA *dma) { AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); @@ -1134,6 +1137,7 @@ static const IDEDMAOps ahci_dma_ops = { .prepare_buf = ahci_dma_prepare_buf, .rw_buf = ahci_dma_rw_buf, .set_unit = ahci_dma_set_unit, +.trigger_irq = ahci_dma_trigger_irq, .add_status = ahci_dma_add_status, .async_cmd_done = ahci_async_cmd_done, .restart_cb = ahci_dma_restart_cb, diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index a7688bf..814cffb 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -355,7 +355,9 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret) eot: bdrv_acct_done(s->bs, &s->acct); -s->bus->dma->ops->add_status(s->bus->dma, BM_STATUS_INT); +if (s->bus->dma->ops->trigger_irq) { +s->bus->dma->ops->trigger_irq(s->bus->dma); +} ide_set_inactive(s); } diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 92f25ef..08230be 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -432,6 +432,7 @@ struct IDEDMAOps { DMAIntFunc *rw_buf; DMAIntFunc *set_unit; DMAIntFunc *add_status; +DMAVoidFunc *trigger_irq; DMAVoidFunc *set_inactive; DMAVoidFunc *async_cmd_done; DMARestartFunc *restart_cb; diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 78c47a3..68d560d 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -152,6 +152,12 @@ static int bmdma_set_unit(IDEDMA *dma, int unit) return 0; } +static void bmdma_trigger_irq(IDEDMA *dma) +{ +BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); +bm->status |= BM_STATUS_INT; +} + static int bmdma_add_status(IDEDMA *dma, int status) { BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); @@ -502,6 +508,7 @@ static const struct IDEDMAOps bmdma_ops = { .prepare_buf = bmdma_prepare_buf, .rw_buf = bmdma_rw_buf, .set_unit = bmdma_set_unit, +.trigger_irq = bmdma_trigger_irq, .add_status = bmdma_add_status, .set_inactive = bmdma_set_inactive, .restart_cb = bmdma_restart_cb, -- 1.8.3.1
[Qemu-devel] [PATCH 06/24] ide: simplify set_inactive callbacks
Drop the unused return value and make the callback optional. Signed-off-by: Paolo Bonzini --- hw/ide/ahci.c | 8 hw/ide/core.c | 5 +++-- hw/ide/internal.h | 2 +- hw/ide/macio.c| 1 - hw/ide/pci.c | 4 +--- 5 files changed, 5 insertions(+), 15 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index d74b6ab..265e3f2 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1106,11 +1106,6 @@ static int ahci_dma_add_status(IDEDMA *dma, int status) return 0; } -static int ahci_dma_set_inactive(IDEDMA *dma) -{ -return 0; -} - static int ahci_async_cmd_done(IDEDMA *dma) { AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); @@ -1125,8 +1120,6 @@ static int ahci_async_cmd_done(IDEDMA *dma) ad->check_bh = qemu_bh_new(ahci_check_cmd_bh, ad); qemu_bh_schedule(ad->check_bh); } - -return 0; } static void ahci_irq_set(void *opaque, int n, int level) @@ -1144,7 +1137,6 @@ static const IDEDMAOps ahci_dma_ops = { .rw_buf = ahci_dma_rw_buf, .set_unit = ahci_dma_set_unit, .add_status = ahci_dma_add_status, -.set_inactive = ahci_dma_set_inactive, .async_cmd_done = ahci_async_cmd_done, .restart_cb = ahci_dma_restart_cb, }; diff --git a/hw/ide/core.c b/hw/ide/core.c index 8437ead..926b90a 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -578,7 +578,9 @@ static void ide_async_cmd_done(IDEState *s) void ide_set_inactive(IDEState *s) { s->bus->dma->aiocb = NULL; -s->bus->dma->ops->set_inactive(s->bus->dma); +if (s->bus->dma->ops->set_inactive) { +s->bus->dma->ops->set_inactive(s->bus->dma); +} ide_async_cmd_done(s); } @@ -2195,7 +2197,6 @@ static const IDEDMAOps ide_dma_nop_ops = { .rw_buf = ide_nop_int, .set_unit = ide_nop_int, .add_status = ide_nop_int, -.set_inactive = ide_nop, .restart_cb = ide_nop_restart, }; diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 84f3a7a..7c68f8e 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -433,7 +433,7 @@ struct IDEDMAOps { DMAIntFunc *rw_buf; DMAIntFunc *set_unit; DMAIntFunc *add_status; -DMAFunc *set_inactive; +DMAVoidFunc *set_inactive; DMAFunc *async_cmd_done; DMARestartFunc *restart_cb; DMAVoidFunc *reset; diff --git a/hw/ide/macio.c b/hw/ide/macio.c index b3cb6ca..013326d 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -526,7 +526,6 @@ static const IDEDMAOps dbdma_ops = { .rw_buf = ide_nop_int, .set_unit = ide_nop_int, .add_status = ide_nop_int, -.set_inactive = ide_nop, .restart_cb = ide_nop_restart, }; diff --git a/hw/ide/pci.c b/hw/ide/pci.c index a9eb5bf..b781149 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -160,15 +160,13 @@ static int bmdma_add_status(IDEDMA *dma, int status) return 0; } -static int bmdma_set_inactive(IDEDMA *dma) +static void bmdma_set_inactive(IDEDMA *dma) { BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); bm->status &= ~BM_STATUS_DMAING; bm->dma_cb = NULL; bm->unit = -1; - -return 0; } static void bmdma_restart_dma(BMDMAState *bm, enum ide_dma_cmd dma_cmd) -- 1.8.3.1
[Qemu-devel] [PATCH 21/24] ide: replace set_unit callback with more IDEBus state
Start moving the initial state of the current request to IDEBus, so that AHCI can use it. The set_unit callback is not used anymore once this is done. Signed-off-by: Paolo Bonzini --- hw/ide/ahci.c | 7 --- hw/ide/core.c | 6 -- hw/ide/internal.h | 2 +- hw/ide/macio.c| 1 - hw/ide/pci.c | 19 ++- hw/ide/pci.h | 7 --- 6 files changed, 15 insertions(+), 27 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 9cafee9..b7e0d18 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1086,12 +1086,6 @@ static int ahci_dma_rw_buf(IDEDMA *dma, int is_write) return 1; } -static int ahci_dma_set_unit(IDEDMA *dma, int unit) -{ -/* only a single unit per link */ -return 0; -} - static void ahci_dma_trigger_irq(IDEDMA *dma) { AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); @@ -1124,7 +1118,6 @@ static const IDEDMAOps ahci_dma_ops = { .start_transfer = ahci_start_transfer, .prepare_buf = ahci_dma_prepare_buf, .rw_buf = ahci_dma_rw_buf, -.set_unit = ahci_dma_set_unit, .trigger_irq = ahci_dma_trigger_irq, .async_cmd_done = ahci_async_cmd_done, }; diff --git a/hw/ide/core.c b/hw/ide/core.c index c07c0d1..970f372 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -580,6 +580,7 @@ static void ide_async_cmd_done(IDEState *s) void ide_set_inactive(IDEState *s, bool more) { s->bus->dma->aiocb = NULL; +s->bus->retry_unit = -1; if (s->bus->dma->ops->set_inactive) { s->bus->dma->ops->set_inactive(s->bus->dma, more); } @@ -601,7 +602,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) BlockErrorAction action = bdrv_get_error_action(s->bs, is_read, error); if (action == BDRV_ACTION_STOP) { -s->bus->dma->ops->set_unit(s->bus->dma, s->unit); +assert(s->bus->retry_unit == s->unit); s->bus->error_status = op; } else if (action == BDRV_ACTION_REPORT) { if (op & IDE_RETRY_DMA) { @@ -724,6 +725,7 @@ static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd) void ide_start_dma(IDEState *s, BlockDriverCompletionFunc *cb) { +s->bus->retry_unit = s->unit; if (s->bus->dma->ops->start_dma) { s->bus->dma->ops->start_dma(s->bus->dma, s, cb); } @@ -2185,11 +2187,11 @@ static int ide_nop_int(IDEDMA *dma, int x) static const IDEDMAOps ide_dma_nop_ops = { .prepare_buf= ide_nop_int, .rw_buf = ide_nop_int, -.set_unit = ide_nop_int, }; static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd) { +s->unit = s->bus->retry_unit; s->bus->dma->ops->restart_dma(s->bus->dma); s->io_buffer_index = 0; s->io_buffer_size = 0; diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 1c77ad9..b1529be 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -431,7 +431,6 @@ struct IDEDMAOps { DMAVoidFunc *start_transfer; DMAIntFunc *prepare_buf; DMAIntFunc *rw_buf; -DMAIntFunc *set_unit; DMAVoidFunc *restart_dma; DMAVoidFunc *trigger_irq; DMAStopFunc *set_inactive; @@ -461,6 +460,7 @@ struct IDEBus { qemu_irq irq; int error_status; +uint8_t retry_unit; }; #define TYPE_IDE_DEVICE "ide-device" diff --git a/hw/ide/macio.c b/hw/ide/macio.c index d1d40d8..c41706c 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -514,7 +514,6 @@ static const IDEDMAOps dbdma_ops = { .start_dma = ide_dbdma_start, .prepare_buf= ide_nop_int, .rw_buf = ide_nop_int, -.set_unit = ide_nop_int, }; static void macio_ide_realizefn(DeviceState *dev, Error **errp) diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 7568f1e..43507ec 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -42,7 +42,6 @@ static void bmdma_start_dma(IDEDMA *dma, IDEState *s, { BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); -bm->unit = s->unit; bm->dma_cb = dma_cb; bm->cur_prd_last = 0; bm->cur_prd_addr = 0; @@ -148,14 +147,6 @@ static int bmdma_rw_buf(IDEDMA *dma, int is_write) return 1; } -static int bmdma_set_unit(IDEDMA *dma, int unit) -{ -BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); -bm->unit = unit; - -return 0; -} - static void bmdma_trigger_irq(IDEDMA *dma) { BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); @@ -167,7 +158,6 @@ static void bmdma_set_inactive(IDEDMA *dma, bool more) BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); bm->dma_cb = NULL; -bm->unit = -1; if (more) { bm->status |= BM_STATUS_DMAING; } else { @@ -326,6 +316,7 @@ static void ide_bmdma_pre_save(void *opaque) BMDMAState *bm = opaque; uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS; +bm->migration_retry_unit = bm->bus->retry_unit; bm->migration_compat_status = (bm->status & ~abused_bits) | (bm->bus->error_status & abused_bits); } @@ -342,6 +333,9 @@ static int ide_bmdma_post_load(void *opaque, int version_id
[Qemu-devel] [PATCH 11/24] ide: fold add_status callback into set_inactive
It is now called only after the set_inactive callback. Put the two together. Signed-off-by: Paolo Bonzini --- hw/ide/ahci.c | 9 - hw/ide/atapi.c| 2 +- hw/ide/core.c | 12 hw/ide/internal.h | 6 +++--- hw/ide/macio.c| 1 - hw/ide/pci.c | 19 +++ 6 files changed, 15 insertions(+), 34 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 7b47053..1d4fb17 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1092,14 +1092,6 @@ static int ahci_dma_set_unit(IDEDMA *dma, int unit) return 0; } -static int ahci_dma_add_status(IDEDMA *dma, int status) -{ -AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); -DPRINTF(ad->port_no, "set status: %x\n", status); - -return 0; -} - static void ahci_dma_trigger_irq(IDEDMA *dma) { AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); @@ -1138,7 +1130,6 @@ static const IDEDMAOps ahci_dma_ops = { .rw_buf = ahci_dma_rw_buf, .set_unit = ahci_dma_set_unit, .trigger_irq = ahci_dma_trigger_irq, -.add_status = ahci_dma_add_status, .async_cmd_done = ahci_async_cmd_done, .restart_cb = ahci_dma_restart_cb, }; diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index 814cffb..6b3f6c1 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -358,7 +358,7 @@ eot: if (s->bus->dma->ops->trigger_irq) { s->bus->dma->ops->trigger_irq(s->bus->dma); } -ide_set_inactive(s); +ide_set_inactive(s, false); } /* start a CD-CDROM read command with DMA */ diff --git a/hw/ide/core.c b/hw/ide/core.c index bd4a9f2..a62654f 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -577,11 +577,11 @@ static void ide_async_cmd_done(IDEState *s) } } -void ide_set_inactive(IDEState *s) +void ide_set_inactive(IDEState *s, bool more) { s->bus->dma->aiocb = NULL; if (s->bus->dma->ops->set_inactive) { -s->bus->dma->ops->set_inactive(s->bus->dma); +s->bus->dma->ops->set_inactive(s->bus->dma, more); } ide_async_cmd_done(s); } @@ -591,7 +591,7 @@ void ide_dma_error(IDEState *s) ide_transfer_stop(s); s->error = ABRT_ERR; s->status = READY_STAT | ERR_STAT; -ide_set_inactive(s); +ide_set_inactive(s, false); ide_set_irq(s->bus); } @@ -696,10 +696,7 @@ eot: if (s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) { bdrv_acct_done(s->bs, &s->acct); } -ide_set_inactive(s); -if (stay_active) { -s->bus->dma->ops->add_status(s->bus->dma, BM_STATUS_DMAING); -} +ide_set_inactive(s, stay_active); } static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd) @@ -2193,7 +2190,6 @@ static const IDEDMAOps ide_dma_nop_ops = { .prepare_buf= ide_nop_int, .rw_buf = ide_nop_int, .set_unit = ide_nop_int, -.add_status = ide_nop_int, .restart_cb = ide_nop_restart, }; diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 08230be..cd5e77c 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -322,6 +322,7 @@ typedef void EndTransferFunc(IDEState *); typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockDriverCompletionFunc *); typedef void DMAVoidFunc(IDEDMA *); typedef int DMAIntFunc(IDEDMA *, int); +typedef void DMAStopFunc(IDEDMA *, bool); typedef void DMARestartFunc(void *, int, RunState); struct unreported_events { @@ -431,9 +432,8 @@ struct IDEDMAOps { DMAIntFunc *prepare_buf; DMAIntFunc *rw_buf; DMAIntFunc *set_unit; -DMAIntFunc *add_status; DMAVoidFunc *trigger_irq; -DMAVoidFunc *set_inactive; +DMAStopFunc *set_inactive; DMAVoidFunc *async_cmd_done; DMARestartFunc *restart_cb; DMAVoidFunc *reset; @@ -568,7 +568,7 @@ void ide_flush_cache(IDEState *s); void ide_transfer_start(IDEState *s, uint8_t *buf, int size, EndTransferFunc *end_transfer_func); void ide_transfer_stop(IDEState *s); -void ide_set_inactive(IDEState *s); +void ide_set_inactive(IDEState *s, bool more); BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, BlockDriverCompletionFunc *cb, void *opaque); diff --git a/hw/ide/macio.c b/hw/ide/macio.c index 56e2b0c..3635ddf 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -519,7 +519,6 @@ static const IDEDMAOps dbdma_ops = { .prepare_buf= ide_nop_int, .rw_buf = ide_nop_int, .set_unit = ide_nop_int, -.add_status = ide_nop_int, .restart_cb = ide_nop_restart, }; diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 68d560d..e4fcf5a 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -158,21 +158,17 @@ static void bmdma_trigger_irq(IDEDMA *dma) bm->status |= BM_STATUS_INT; } -static int bmdma_add_status(IDEDMA *dma, int status) +static void bmdma_set_inactive(IDEDMA *dma, bool more) { BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); -bm->status |= status; -return 0; -} - -static vo
[Qemu-devel] [PATCH 08/24] ide: simplify start_transfer callbacks
Drop the unused return value and make the callback optional. Signed-off-by: Paolo Bonzini --- hw/ide/ahci.c | 4 +--- hw/ide/core.c | 10 +++--- hw/ide/internal.h | 3 +-- hw/ide/macio.c| 6 -- hw/ide/pci.c | 6 -- 5 files changed, 5 insertions(+), 24 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index b4b8386..23f3f22 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -981,7 +981,7 @@ out: } /* DMA dev <-> ram */ -static int ahci_start_transfer(IDEDMA *dma) +static void ahci_start_transfer(IDEDMA *dma) { AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); IDEState *s = &ad->port.ifs[0]; @@ -1031,8 +1031,6 @@ out: /* done with DMA */ ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_DSS); } - -return 0; } static void ahci_start_dma(IDEDMA *dma, IDEState *s, diff --git a/hw/ide/core.c b/hw/ide/core.c index 926b90a..e64dfff 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -434,7 +434,9 @@ void ide_transfer_start(IDEState *s, uint8_t *buf, int size, if (!(s->status & ERR_STAT)) { s->status |= DRQ_STAT; } -s->bus->dma->ops->start_transfer(s->bus->dma); +if (s->bus->dma->ops->start_transfer) { +s->bus->dma->ops->start_transfer(s->bus->dma); +} } void ide_transfer_stop(IDEState *s) @@ -2176,11 +2178,6 @@ static void ide_nop_start(IDEDMA *dma, IDEState *s, { } -static int ide_nop(IDEDMA *dma) -{ -return 0; -} - static int ide_nop_int(IDEDMA *dma, int x) { return 0; @@ -2192,7 +2189,6 @@ static void ide_nop_restart(void *opaque, int x, RunState y) static const IDEDMAOps ide_dma_nop_ops = { .start_dma = ide_nop_start, -.start_transfer = ide_nop, .prepare_buf= ide_nop_int, .rw_buf = ide_nop_int, .set_unit = ide_nop_int, diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 43b8f43..92d2351 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -321,7 +321,6 @@ typedef void EndTransferFunc(IDEState *); typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockDriverCompletionFunc *); typedef void DMAVoidFunc(IDEDMA *); -typedef int DMAFunc(IDEDMA *); typedef int DMAIntFunc(IDEDMA *, int); typedef void DMARestartFunc(void *, int, RunState); @@ -428,7 +427,7 @@ struct IDEState { struct IDEDMAOps { DMAStartFunc *start_dma; -DMAFunc *start_transfer; +DMAVoidFunc *start_transfer; DMAIntFunc *prepare_buf; DMAIntFunc *rw_buf; DMAIntFunc *set_unit; diff --git a/hw/ide/macio.c b/hw/ide/macio.c index 013326d..56e2b0c 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -495,11 +495,6 @@ static void macio_ide_reset(DeviceState *dev) ide_bus_reset(&d->bus); } -static int ide_nop(IDEDMA *dma) -{ -return 0; -} - static int ide_nop_int(IDEDMA *dma, int x) { return 0; @@ -521,7 +516,6 @@ static void ide_dbdma_start(IDEDMA *dma, IDEState *s, static const IDEDMAOps dbdma_ops = { .start_dma = ide_dbdma_start, -.start_transfer = ide_nop, .prepare_buf= ide_nop_int, .rw_buf = ide_nop_int, .set_unit = ide_nop_int, diff --git a/hw/ide/pci.c b/hw/ide/pci.c index b781149..78c47a3 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -264,11 +264,6 @@ static void bmdma_reset(IDEDMA *dma) bm->nsector = 0; } -static int bmdma_start_transfer(IDEDMA *dma) -{ -return 0; -} - static void bmdma_irq(void *opaque, int n, int level) { BMDMAState *bm = opaque; @@ -504,7 +499,6 @@ void pci_ide_create_devs(PCIDevice *dev, DriveInfo **hd_table) static const struct IDEDMAOps bmdma_ops = { .start_dma = bmdma_start_dma, -.start_transfer = bmdma_start_transfer, .prepare_buf = bmdma_prepare_buf, .rw_buf = bmdma_rw_buf, .set_unit = bmdma_set_unit, -- 1.8.3.1
[Qemu-devel] [PATCH 14/24] ide: start extracting ide_restart_dma out of bmdma_restart_dma
Signed-off-by: Paolo Bonzini --- hw/ide/pci.c | 28 ++-- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/hw/ide/pci.c b/hw/ide/pci.c index cc910de..c7fbe51 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -175,18 +175,24 @@ static void bmdma_set_inactive(IDEDMA *dma, bool more) } } -static void bmdma_restart_dma(BMDMAState *bm, enum ide_dma_cmd dma_cmd) +static void bmdma_restart_dma(BMDMAState *bm) { IDEState *s = bmdma_active_if(bm); ide_set_sector(s, bm->sector_num); +s->nsector = bm->nsector; +bm->cur_addr = bm->addr; +} + +static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd) +{ +BMDMAState *bm = DO_UPCAST(BMDMAState, dma, s->bus->dma); + +bmdma_restart_dma(bm); s->io_buffer_index = 0; s->io_buffer_size = 0; -s->nsector = bm->nsector; s->dma_cmd = dma_cmd; -bm->cur_addr = bm->addr; -bm->dma_cb = ide_dma_cb; -bmdma_start_dma(&bm->dma, s, bm->dma_cb); +bmdma_start_dma(&bm->dma, s, ide_dma_cb); } /* TODO This should be common IDE code */ @@ -194,6 +200,7 @@ static void bmdma_restart_bh(void *opaque) { BMDMAState *bm = opaque; IDEBus *bus = bm->bus; +IDEState *s; bool is_read; int error_status; @@ -204,6 +211,7 @@ static void bmdma_restart_bh(void *opaque) return; } +s = bmdma_active_if(bm); is_read = (bus->error_status & IDE_RETRY_READ) != 0; /* The error status must be cleared before resubmitting the request: The @@ -214,18 +222,18 @@ static void bmdma_restart_bh(void *opaque) if (error_status & IDE_RETRY_DMA) { if (error_status & IDE_RETRY_TRIM) { -bmdma_restart_dma(bm, IDE_DMA_TRIM); +ide_restart_dma(s, IDE_DMA_TRIM); } else { -bmdma_restart_dma(bm, is_read ? IDE_DMA_READ : IDE_DMA_WRITE); +ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE); } } else if (error_status & IDE_RETRY_PIO) { if (is_read) { -ide_sector_read(bmdma_active_if(bm)); +ide_sector_read(s); } else { -ide_sector_write(bmdma_active_if(bm)); +ide_sector_write(s); } } else if (error_status & IDE_RETRY_FLUSH) { -ide_flush_cache(bmdma_active_if(bm)); +ide_flush_cache(s); } } -- 1.8.3.1
[Qemu-devel] [PATCH 13/24] ide: move retry constants out of BM_STATUS_* namespace
Signed-off-by: Paolo Bonzini --- hw/ide/core.c | 20 ++-- hw/ide/internal.h | 12 ++-- hw/ide/pci.c | 14 +++--- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index a62654f..1e3108c 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -511,8 +511,8 @@ static void ide_sector_read_cb(void *opaque, int ret) bdrv_acct_done(s->bs, &s->acct); if (ret != 0) { -if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY | -BM_STATUS_RETRY_READ)) { +if (ide_handle_rw_error(s, -ret, IDE_RETRY_PIO | +IDE_RETRY_READ)) { return; } } @@ -597,14 +597,14 @@ void ide_dma_error(IDEState *s) static int ide_handle_rw_error(IDEState *s, int error, int op) { -bool is_read = (op & BM_STATUS_RETRY_READ) != 0; +bool is_read = (op & IDE_RETRY_READ) != 0; BlockErrorAction action = bdrv_get_error_action(s->bs, is_read, error); if (action == BDRV_ACTION_STOP) { s->bus->dma->ops->set_unit(s->bus->dma, s->unit); s->bus->error_status = op; } else if (action == BDRV_ACTION_REPORT) { -if (op & BM_STATUS_DMA_RETRY) { +if (op & IDE_RETRY_DMA) { dma_buf_commit(s); ide_dma_error(s); } else { @@ -623,12 +623,12 @@ void ide_dma_cb(void *opaque, int ret) bool stay_active = false; if (ret < 0) { -int op = BM_STATUS_DMA_RETRY; +int op = IDE_RETRY_DMA; if (s->dma_cmd == IDE_DMA_READ) -op |= BM_STATUS_RETRY_READ; +op |= IDE_RETRY_READ; else if (s->dma_cmd == IDE_DMA_TRIM) -op |= BM_STATUS_RETRY_TRIM; +op |= IDE_RETRY_TRIM; if (ide_handle_rw_error(s, -ret, op)) { return; @@ -746,7 +746,7 @@ static void ide_sector_write_cb(void *opaque, int ret) s->status &= ~BUSY_STAT; if (ret != 0) { -if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY)) { +if (ide_handle_rw_error(s, -ret, IDE_RETRY_PIO)) { return; } } @@ -813,7 +813,7 @@ static void ide_flush_cb(void *opaque, int ret) if (ret < 0) { /* XXX: What sector number to set here? */ -if (ide_handle_rw_error(s, -ret, BM_STATUS_RETRY_FLUSH)) { +if (ide_handle_rw_error(s, -ret, IDE_RETRY_FLUSH)) { return; } } @@ -2356,7 +2356,7 @@ static bool ide_drive_pio_state_needed(void *opaque) IDEState *s = opaque; return ((s->status & DRQ_STAT) != 0) -|| (s->bus->error_status & BM_STATUS_PIO_RETRY); +|| (s->bus->error_status & IDE_RETRY_PIO); } static bool ide_tray_state_needed(void *opaque) diff --git a/hw/ide/internal.h b/hw/ide/internal.h index f2d8ccc..041f173 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -485,12 +485,12 @@ struct IDEDevice { uint64_t wwn; }; -/* FIXME These are not status register bits */ -#define BM_STATUS_DMA_RETRY 0x08 -#define BM_STATUS_PIO_RETRY 0x10 -#define BM_STATUS_RETRY_READ 0x20 -#define BM_STATUS_RETRY_FLUSH 0x40 -#define BM_STATUS_RETRY_TRIM 0x80 +/* These are used for the error_status field of IDEBus */ +#define IDE_RETRY_DMA 0x08 +#define IDE_RETRY_PIO 0x10 +#define IDE_RETRY_READ 0x20 +#define IDE_RETRY_FLUSH 0x40 +#define IDE_RETRY_TRIM 0x80 static inline IDEState *idebus_active_if(IDEBus *bus) { diff --git a/hw/ide/pci.c b/hw/ide/pci.c index e61a82f..cc910de 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -34,8 +34,8 @@ #define BMDMA_PAGE_SIZE 4096 #define BM_MIGRATION_COMPAT_STATUS_BITS \ -(BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \ -BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH) +(IDE_RETRY_DMA | IDE_RETRY_PIO | \ +IDE_RETRY_READ | IDE_RETRY_FLUSH) static void bmdma_start_dma(IDEDMA *dma, IDEState *s, BlockDriverCompletionFunc *dma_cb) @@ -204,7 +204,7 @@ static void bmdma_restart_bh(void *opaque) return; } -is_read = (bus->error_status & BM_STATUS_RETRY_READ) != 0; +is_read = (bus->error_status & IDE_RETRY_READ) != 0; /* The error status must be cleared before resubmitting the request: The * request may fail again, and this case can only be distinguished if the @@ -212,19 +212,19 @@ static void bmdma_restart_bh(void *opaque) error_status = bus->error_status; bus->error_status = 0; -if (error_status & BM_STATUS_DMA_RETRY) { -if (error_status & BM_STATUS_RETRY_TRIM) { +if (error_status & IDE_RETRY_DMA) { +if (error_status & IDE_RETRY_TRIM) { bmdma_restart_dma(bm, IDE_DMA_TRIM); } else { bmdma_restart_dma(bm, is_read ? IDE_DMA_READ : IDE_DMA_WRITE); } -} else if (error_status & BM_STATUS_PIO_RETRY) { +} else if (error_status & IDE_RETRY_PIO) { if (is_read) { ide_sector_read(bmdma_act
[Qemu-devel] [PATCH 19/24] ide: move restart callback to common code
Signed-off-by: Paolo Bonzini --- hw/ide/core.c | 66 - hw/ide/internal.h | 2 ++ hw/ide/pci.c | 68 --- hw/ide/pci.h | 1 - 4 files changed, 67 insertions(+), 70 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 5820fad..256f62a 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2193,9 +2193,73 @@ static const IDEDMAOps ide_dma_nop_ops = { .restart_cb = ide_nop_restart, }; +static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd) +{ +s->bus->dma->ops->restart_dma(s->bus->dma); +s->io_buffer_index = 0; +s->io_buffer_size = 0; +s->dma_cmd = dma_cmd; +ide_start_dma(s, ide_dma_cb); +} + +static void ide_restart_bh(void *opaque) +{ +IDEBus *bus = opaque; +IDEState *s; +bool is_read; +int error_status; + +qemu_bh_delete(bus->bh); +bus->bh = NULL; + +error_status = bus->error_status; +if (bus->error_status == 0) { +return; +} + +s = idebus_active_if(bus); +is_read = (bus->error_status & IDE_RETRY_READ) != 0; + +/* The error status must be cleared before resubmitting the request: The + * request may fail again, and this case can only be distinguished if the + * called function can set a new error status. */ +bus->error_status = 0; + +if (error_status & IDE_RETRY_DMA) { +if (error_status & IDE_RETRY_TRIM) { +ide_restart_dma(s, IDE_DMA_TRIM); +} else { +ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE); +} +} else if (error_status & IDE_RETRY_PIO) { +if (is_read) { +ide_sector_read(s); +} else { +ide_sector_write(s); +} +} else if (error_status & IDE_RETRY_FLUSH) { +ide_flush_cache(s); +} +} + +static void ide_restart_cb(void *opaque, int running, RunState state) +{ +IDEBus *bus = opaque; + +if (!running) +return; + +if (!bus->bh) { +bus->bh = qemu_bh_new(ide_restart_bh, bus); +qemu_bh_schedule(bus->bh); +} +} + void ide_register_restart_cb(IDEBus *bus) { -qemu_add_vm_change_state_handler(bus->dma->ops->restart_cb, bus); +if (bus->dma->ops->restart_dma) { +qemu_add_vm_change_state_handler(ide_restart_cb, bus); +} } static IDEDMA ide_dma_nop = { diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 678b33c..e24f6cb 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -452,6 +452,8 @@ struct IDEBus { IDEDevice *master; IDEDevice *slave; IDEState ifs[2]; +QEMUBH *bh; + int bus_id; int max_units; IDEDMA *dma; diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 9d26a26..7568f1e 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -185,73 +185,6 @@ static void bmdma_restart_dma(IDEDMA *dma) bm->cur_addr = bm->addr; } -static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd) -{ -if (s->bus->dma->ops->restart_dma) { -s->bus->dma->ops->restart_dma(s->bus->dma); -} -s->io_buffer_index = 0; -s->io_buffer_size = 0; -s->dma_cmd = dma_cmd; -ide_start_dma(s, ide_dma_cb); -} - -/* TODO This should be common IDE code */ -static void bmdma_restart_bh(void *opaque) -{ -IDEBus *bus = opaque; -BMDMAState *bm = DO_UPCAST(BMDMAState, dma, bus->dma); -IDEState *s; -bool is_read; -int error_status; - -qemu_bh_delete(bm->bh); -bm->bh = NULL; - -error_status = bus->error_status; -if (bus->error_status == 0) { -return; -} - -s = idebus_active_if(bus); -is_read = (bus->error_status & IDE_RETRY_READ) != 0; - -/* The error status must be cleared before resubmitting the request: The - * request may fail again, and this case can only be distinguished if the - * called function can set a new error status. */ -bus->error_status = 0; - -if (error_status & IDE_RETRY_DMA) { -if (error_status & IDE_RETRY_TRIM) { -ide_restart_dma(s, IDE_DMA_TRIM); -} else { -ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE); -} -} else if (error_status & IDE_RETRY_PIO) { -if (is_read) { -ide_sector_read(s); -} else { -ide_sector_write(s); -} -} else if (error_status & IDE_RETRY_FLUSH) { -ide_flush_cache(s); -} -} - -static void bmdma_restart_cb(void *opaque, int running, RunState state) -{ -IDEBus *bus = opaque; -BMDMAState *bm = DO_UPCAST(BMDMAState, dma, bus->dma); - -if (!running) -return; - -if (!bm->bh) { -bm->bh = qemu_bh_new(bmdma_restart_bh, &bm->dma); -qemu_bh_schedule(bm->bh); -} -} - static void bmdma_cancel(BMDMAState *bm) { if (bm->status & BM_STATUS_DMAING) { @@ -520,7 +453,6 @@ static const struct IDEDMAOps bmdma_ops = { .restart_dma = bmdma_restart_dma, .trigger_irq = bmdma_trigger_irq,
[Qemu-devel] [PATCH 18/24] ide: pass IDEBus to the restart_cb
Signed-off-by: Paolo Bonzini --- hw/ide/core.c | 2 +- hw/ide/pci.c | 8 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 93cfd46..5820fad 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2195,7 +2195,7 @@ static const IDEDMAOps ide_dma_nop_ops = { void ide_register_restart_cb(IDEBus *bus) { -qemu_add_vm_change_state_handler(bus->dma->ops->restart_cb, bus->dma); +qemu_add_vm_change_state_handler(bus->dma->ops->restart_cb, bus); } static IDEDMA ide_dma_nop = { diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 4df50dc..9d26a26 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -199,8 +199,8 @@ static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd) /* TODO This should be common IDE code */ static void bmdma_restart_bh(void *opaque) { -BMDMAState *bm = opaque; -IDEBus *bus = bm->bus; +IDEBus *bus = opaque; +BMDMAState *bm = DO_UPCAST(BMDMAState, dma, bus->dma); IDEState *s; bool is_read; int error_status; @@ -240,8 +240,8 @@ static void bmdma_restart_bh(void *opaque) static void bmdma_restart_cb(void *opaque, int running, RunState state) { -IDEDMA *dma = opaque; -BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); +IDEBus *bus = opaque; +BMDMAState *bm = DO_UPCAST(BMDMAState, dma, bus->dma); if (!running) return; -- 1.8.3.1
[Qemu-devel] [PATCH 04/24] ide-test: add test for werror=stop
Signed-off-by: Paolo Bonzini --- tests/ide-test.c | 74 1 file changed, 74 insertions(+) diff --git a/tests/ide-test.c b/tests/ide-test.c index 7307f1d..560dd2c 100644 --- a/tests/ide-test.c +++ b/tests/ide-test.c @@ -105,6 +105,7 @@ static QPCIBus *pcibus = NULL; static QGuestAllocator *guest_malloc; static char tmp_path[] = "/tmp/qtest.XX"; +static char debug_path[] = "/tmp/qtest-blkdebug.XX"; static void ide_test_start(const char *cmdline_fmt, ...) { @@ -465,6 +466,72 @@ static void test_flush(void) ide_test_quit(); } +static void prepare_blkdebug_script(const char *debug_path, const char *event) +{ +FILE *debug_file = fopen(debug_path, "w"); +int ret; + +fprintf(debug_file, "[inject-error]\n"); +fprintf(debug_file, "event = \"%s\"\n", event); +fprintf(debug_file, "errno = \"5\"\n"); +fprintf(debug_file, "state = \"1\"\n"); +fprintf(debug_file, "immediately = \"off\"\n"); +fprintf(debug_file, "once = \"on\"\n"); + +fprintf(debug_file, "[set-state]\n"); +fprintf(debug_file, "event = \"%s\"\n", event); +fprintf(debug_file, "new_state = \"2\"\n"); +fflush(debug_file); +g_assert(!ferror(debug_file)); + +ret = fclose(debug_file); +g_assert(ret == 0); +} + +static void test_retry_flush(void) +{ +uint8_t data; +const char *s; + +prepare_blkdebug_script(debug_path, "flush_to_disk"); + +ide_test_start( +"-vnc none " +"-drive file=blkdebug:%s:%s,if=ide,cache=writeback,rerror=stop,werror=stop", +debug_path, tmp_path); + +/* FLUSH CACHE command on device 0*/ +outb(IDE_BASE + reg_device, 0); +outb(IDE_BASE + reg_command, CMD_FLUSH_CACHE); + +/* Check status while request is in flight*/ +data = inb(IDE_BASE + reg_status); +assert_bit_set(data, BSY | DRDY); +assert_bit_clear(data, DF | ERR | DRQ); + +sleep(1);/* HACK: wait for event */ + +/* Complete the command */ +s = "{'execute':'cont' }"; +while (!qmp(s)) { +s = ""; +sleep(1); +} + +/* Check registers */ +data = inb(IDE_BASE + reg_device); +g_assert_cmpint(data & DEV, ==, 0); + +do { +data = inb(IDE_BASE + reg_status); +} while (data & BSY); + +assert_bit_set(data, DRDY); +assert_bit_clear(data, BSY | DF | ERR | DRQ); + +ide_test_quit(); +} + int main(int argc, char **argv) { const char *arch = qtest_get_arch(); @@ -477,6 +544,11 @@ int main(int argc, char **argv) return 0; } +/* Create temporary blkdebug instructions */ +fd = mkstemp(debug_path); +g_assert(fd >= 0); +close(fd); + /* Create a temporary raw image */ fd = mkstemp(tmp_path); g_assert(fd >= 0); @@ -497,6 +569,8 @@ int main(int argc, char **argv) qtest_add_func("/ide/flush", test_flush); +qtest_add_func("/ide/retry/flush", test_retry_flush); + ret = g_test_run(); /* Cleanup */ -- 1.8.3.1
[Qemu-devel] [PATCH 22/24] ide: place initial state of the current request to IDEBus
This moves more common restarting logic to the core IDE code. Signed-off-by: Paolo Bonzini --- hw/ide/core.c | 6 ++ hw/ide/internal.h | 2 ++ hw/ide/pci.c | 15 ++- hw/ide/pci.h | 4 ++-- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 970f372..d3f2c4f 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -581,6 +581,8 @@ void ide_set_inactive(IDEState *s, bool more) { s->bus->dma->aiocb = NULL; s->bus->retry_unit = -1; +s->bus->retry_sector_num = 0; +s->bus->retry_nsector = 0; if (s->bus->dma->ops->set_inactive) { s->bus->dma->ops->set_inactive(s->bus->dma, more); } @@ -726,6 +728,8 @@ static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd) void ide_start_dma(IDEState *s, BlockDriverCompletionFunc *cb) { s->bus->retry_unit = s->unit; +s->bus->retry_sector_num = ide_get_sector(s); +s->bus->retry_nsector = s->nsector; if (s->bus->dma->ops->start_dma) { s->bus->dma->ops->start_dma(s->bus->dma, s, cb); } @@ -2192,6 +2196,8 @@ static const IDEDMAOps ide_dma_nop_ops = { static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd) { s->unit = s->bus->retry_unit; +ide_set_sector(s, s->bus->retry_sector_num); +s->nsector = s->bus->retry_nsector; s->bus->dma->ops->restart_dma(s->bus->dma); s->io_buffer_index = 0; s->io_buffer_size = 0; diff --git a/hw/ide/internal.h b/hw/ide/internal.h index b1529be..6d2781a 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -461,6 +461,8 @@ struct IDEBus { int error_status; uint8_t retry_unit; +int64_t retry_sector_num; +uint32_t retry_nsector; }; #define TYPE_IDE_DEVICE "ide-device" diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 43507ec..9332844 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -46,8 +46,6 @@ static void bmdma_start_dma(IDEDMA *dma, IDEState *s, bm->cur_prd_last = 0; bm->cur_prd_addr = 0; bm->cur_prd_len = 0; -bm->sector_num = ide_get_sector(s); -bm->nsector = s->nsector; if (bm->status & BM_STATUS_DMAING) { bm->dma_cb(bmdma_active_if(bm), 0); @@ -168,10 +166,7 @@ static void bmdma_set_inactive(IDEDMA *dma, bool more) static void bmdma_restart_dma(IDEDMA *dma) { BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma); -IDEState *s = bmdma_active_if(bm); -ide_set_sector(s, bm->sector_num); -s->nsector = bm->nsector; bm->cur_addr = bm->addr; } @@ -198,8 +193,6 @@ static void bmdma_reset(IDEDMA *dma) bm->cur_prd_last = 0; bm->cur_prd_addr = 0; bm->cur_prd_len = 0; -bm->sector_num = 0; -bm->nsector = 0; } static void bmdma_irq(void *opaque, int n, int level) @@ -317,6 +310,8 @@ static void ide_bmdma_pre_save(void *opaque) uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS; bm->migration_retry_unit = bm->bus->retry_unit; +bm->migration_retry_sector_num = bm->bus->retry_sector_num; +bm->migration_retry_nsector = bm->bus->retry_nsector; bm->migration_compat_status = (bm->status & ~abused_bits) | (bm->bus->error_status & abused_bits); } @@ -334,6 +329,8 @@ static int ide_bmdma_post_load(void *opaque, int version_id) bm->bus->error_status |= bm->migration_compat_status & abused_bits; } if (bm->bus->error_status) { +bm->bus->retry_sector_num = bm->migration_retry_sector_num; +bm->bus->retry_nsector = bm->migration_retry_nsector; bm->bus->retry_unit = bm->migration_retry_unit; } @@ -375,8 +372,8 @@ static const VMStateDescription vmstate_bmdma = { VMSTATE_UINT8(cmd, BMDMAState), VMSTATE_UINT8(migration_compat_status, BMDMAState), VMSTATE_UINT32(addr, BMDMAState), -VMSTATE_INT64(sector_num, BMDMAState), -VMSTATE_UINT32(nsector, BMDMAState), +VMSTATE_INT64(migration_retry_sector_num, BMDMAState), +VMSTATE_UINT32(migration_retry_nsector, BMDMAState), VMSTATE_UINT8(migration_retry_unit, BMDMAState), VMSTATE_END_OF_LIST() }, diff --git a/hw/ide/pci.h b/hw/ide/pci.h index 6f28ce4..3d19070 100644 --- a/hw/ide/pci.h +++ b/hw/ide/pci.h @@ -23,8 +23,6 @@ typedef struct BMDMAState { uint32_t cur_prd_addr; uint32_t cur_prd_len; BlockDriverCompletionFunc *dma_cb; -int64_t sector_num; -uint32_t nsector; MemoryRegion addr_ioport; MemoryRegion extra_io; qemu_irq irq; @@ -33,6 +31,8 @@ typedef struct BMDMAState { * Bit 3-6: bus->error_status */ uint8_t migration_compat_status; uint8_t migration_retry_unit; +int64_t migration_retry_sector_num; +uint32_t migration_retry_nsector; struct PCIIDEState *pci_dev; } BMDMAState; -- 1.8.3.1
[Qemu-devel] [PATCH 20/24] ide: remove restart_cb callback
Signed-off-by: Paolo Bonzini --- hw/ide/ahci.c | 5 - hw/ide/core.c | 5 - hw/ide/internal.h | 1 - hw/ide/macio.c| 5 - 4 files changed, 16 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 1d4fb17..9cafee9 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1119,10 +1119,6 @@ static void ahci_irq_set(void *opaque, int n, int level) { } -static void ahci_dma_restart_cb(void *opaque, int running, RunState state) -{ -} - static const IDEDMAOps ahci_dma_ops = { .start_dma = ahci_start_dma, .start_transfer = ahci_start_transfer, @@ -1131,7 +1127,6 @@ static const IDEDMAOps ahci_dma_ops = { .set_unit = ahci_dma_set_unit, .trigger_irq = ahci_dma_trigger_irq, .async_cmd_done = ahci_async_cmd_done, -.restart_cb = ahci_dma_restart_cb, }; void ahci_init(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports) diff --git a/hw/ide/core.c b/hw/ide/core.c index 256f62a..c07c0d1 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2182,15 +2182,10 @@ static int ide_nop_int(IDEDMA *dma, int x) return 0; } -static void ide_nop_restart(void *opaque, int x, RunState y) -{ -} - static const IDEDMAOps ide_dma_nop_ops = { .prepare_buf= ide_nop_int, .rw_buf = ide_nop_int, .set_unit = ide_nop_int, -.restart_cb = ide_nop_restart, }; static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd) diff --git a/hw/ide/internal.h b/hw/ide/internal.h index e24f6cb..1c77ad9 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -436,7 +436,6 @@ struct IDEDMAOps { DMAVoidFunc *trigger_irq; DMAStopFunc *set_inactive; DMAVoidFunc *async_cmd_done; -DMARestartFunc *restart_cb; DMAVoidFunc *reset; }; diff --git a/hw/ide/macio.c b/hw/ide/macio.c index 3635ddf..d1d40d8 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -500,10 +500,6 @@ static int ide_nop_int(IDEDMA *dma, int x) return 0; } -static void ide_nop_restart(void *opaque, int x, RunState y) -{ -} - static void ide_dbdma_start(IDEDMA *dma, IDEState *s, BlockDriverCompletionFunc *cb) { @@ -519,7 +515,6 @@ static const IDEDMAOps dbdma_ops = { .prepare_buf= ide_nop_int, .rw_buf = ide_nop_int, .set_unit = ide_nop_int, -.restart_cb = ide_nop_restart, }; static void macio_ide_realizefn(DeviceState *dev, Error **errp) -- 1.8.3.1
[Qemu-devel] [PATCH 23/24] ide: migrate initial request state via IDEBus
This only breaks backwards migration compatibility if the bus is in an error state. It is in principle possible to avoid this by making two subsections (one for version 1, and one for version 2, but with the same name) with different "_needed" callbacks. The v1 callback would return true if error_status != 0 and the bus is PATA; the v2 callback would return true if error_status != 0 and the bus is AHCI. Forward migration keeps working. Signed-off-by: Paolo Bonzini --- hw/ide/core.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index d3f2c4f..01deeeb 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2544,11 +2544,14 @@ const VMStateDescription vmstate_ide_drive = { static const VMStateDescription vmstate_ide_error_status = { .name ="ide_bus/error", -.version_id = 1, +.version_id = 2, .minimum_version_id = 1, .minimum_version_id_old = 1, .fields = (VMStateField []) { VMSTATE_INT32(error_status, IDEBus), +VMSTATE_INT64_V(retry_sector_num, IDEBus, 2), +VMSTATE_UINT32_V(retry_nsector, IDEBus, 2), +VMSTATE_UINT8_V(retry_unit, IDEBus, 2), VMSTATE_END_OF_LIST() } }; -- 1.8.3.1
[Qemu-devel] [PATCH 24/24] ide: commonize io_buffer_index initialization
Signed-off-by: Paolo Bonzini --- hw/ide/atapi.c | 2 -- hw/ide/core.c | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index 6b3f6c1..249b8b1 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -250,7 +250,6 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int max_size) s->packet_transfer_size = size; s->io_buffer_size = size;/* dma: send the reply data as one chunk */ s->elementary_transfer_size = 0; -s->io_buffer_index = 0; if (s->atapi_dma) { bdrv_acct_start(s->bs, &s->acct, size, BDRV_ACCT_READ); @@ -368,7 +367,6 @@ static void ide_atapi_cmd_read_dma(IDEState *s, int lba, int nb_sectors, { s->lba = lba; s->packet_transfer_size = nb_sectors * sector_size; -s->io_buffer_index = 0; s->io_buffer_size = 0; s->cd_sector_size = sector_size; diff --git a/hw/ide/core.c b/hw/ide/core.c index 01deeeb..c3817bd 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -705,7 +705,6 @@ eot: static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd) { s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT; -s->io_buffer_index = 0; s->io_buffer_size = 0; s->dma_cmd = dma_cmd; @@ -727,6 +726,7 @@ static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd) void ide_start_dma(IDEState *s, BlockDriverCompletionFunc *cb) { +s->io_buffer_index = 0; s->bus->retry_unit = s->unit; s->bus->retry_sector_num = ide_get_sector(s); s->bus->retry_nsector = s->nsector; @@ -2199,7 +2199,6 @@ static void ide_restart_dma(IDEState *s, enum ide_dma_cmd dma_cmd) ide_set_sector(s, s->bus->retry_sector_num); s->nsector = s->bus->retry_nsector; s->bus->dma->ops->restart_dma(s->bus->dma); -s->io_buffer_index = 0; s->io_buffer_size = 0; s->dma_cmd = dma_cmd; ide_start_dma(s, ide_dma_cb); -- 1.8.3.1
[Qemu-devel] [Bug 1119686] Re: Incorrect handling of icebp
This bug is still present in QEMU 1.6.0 (as per Debian's qemu-system-x86 1.6.0+dfsg-1 package). -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1119686 Title: Incorrect handling of icebp Status in QEMU: New Bug description: Wine conformance suite tests the behavior of various low-level Windows API functions. One of the tests involves checking the interaction of breakpoints and exceptions, and in particular the 'icebp' breakpoint. This test works on a Windows XP machine running either on the metal or in VMware ESX but fails when run in QEmu. To reproduce the issue grab the attached 'exception.exe' file and run it. If you get 'Test failed' lines like below then it means the problem is still present: exception.c:202: exception 0: 8004 flags:0 addr:003F exception.c:208: Test failed: 0: Wrong exception address 003F/003F0001 exception.c:214: this is the last test seen before the exception exception: unhandled exception 8004 at 003F exception.c:202: exception 0: c027 flags:2 addr:7C80E0B9 exception.c:205: Test failed: 0: Wrong exception code c027/8004 exception.c:208: Test failed: 0: Wrong exception address 7C80E0B9/003F0001 Note that this bug was not present in QEmu 1.1.2+dfsg-5 (Debian Testing) but is now present in 1.4.0~rc0+dfsg-1exp (Debian Experimental). To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1119686/+subscriptions
[Qemu-devel] [Bug 1245543] [NEW] Wrong implementation of SSE4.1 pmovzxbw and similar instructions
Public bug reported: QEMU 1.5.0 (and git version, as far as I can tell from the source code) has incorrect implementation of pmovzxbw and similar SSE4.1 instructions. The instruction zero-extends the first 8 8-bit elements of a vector to 16bit vector and puts them to another vector. The current implementation applies this operation only to the first element and zeros out the rest. To verify, compile the attached program for SSE4.1 (g++ -msse4.1 cvtint.cc). On real hardware, it produces the following output: $ ./a.out 1 0 2 0 3 0 4 0 5 0 6 0 7 0 8 0 On QEMU, the output is as follows: $ ./a.out 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 QEMU is invoked as: qemu-system-x86_64 \ -M pc -cpu Haswell,+sse4.1,+avx,+avx2,+fma,enforce -m 512 \ -serial stdio -no-reboot \ -kernel vmlinuz -initrd initrd.img \ -netdev user,id=user.0 -device rtl8139,netdev=user.0 -redir tcp:::22 \ -hda ubuntu-amd64.ext3 \ --append "rw console=tty root=/dev/sda" ** Affects: qemu Importance: Undecided Status: New ** Attachment added: "Program to verify this bug" https://bugs.launchpad.net/bugs/1245543/+attachment/3893823/+files/cvtint.cc -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1245543 Title: Wrong implementation of SSE4.1 pmovzxbw and similar instructions Status in QEMU: New Bug description: QEMU 1.5.0 (and git version, as far as I can tell from the source code) has incorrect implementation of pmovzxbw and similar SSE4.1 instructions. The instruction zero-extends the first 8 8-bit elements of a vector to 16bit vector and puts them to another vector. The current implementation applies this operation only to the first element and zeros out the rest. To verify, compile the attached program for SSE4.1 (g++ -msse4.1 cvtint.cc). On real hardware, it produces the following output: $ ./a.out 1 0 2 0 3 0 4 0 5 0 6 0 7 0 8 0 On QEMU, the output is as follows: $ ./a.out 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 QEMU is invoked as: qemu-system-x86_64 \ -M pc -cpu Haswell,+sse4.1,+avx,+avx2,+fma,enforce -m 512 \ -serial stdio -no-reboot \ -kernel vmlinuz -initrd initrd.img \ -netdev user,id=user.0 -device rtl8139,netdev=user.0 -redir tcp:::22 \ -hda ubuntu-amd64.ext3 \ --append "rw console=tty root=/dev/sda" To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1245543/+subscriptions
[Qemu-devel] Attempt to add support for NBD in MinGW
Hi, I try to add support for NBD in MinGW. Basicalyl this means adding support for win32 sockets to aio-win32.c. What I did was rewrite the file (attachment provided). problem here is that revents for the node related to the nbd socket is always 0 after g_poll() even when I see that data is returned from and nbd server (file attached). What am I doing wrong ? This was done against QEMU-1.6.0 Thanks, Or dumpfile.pcap Description: Binary data /* * QEMU aio implementation * * Copyright IBM, Corp. 2008 * * Authors: * Anthony Liguori * * This work is licensed under the terms of the GNU GPL, version 2. See * the COPYING file in the top-level directory. * * Contributions after 2012-01-13 are licensed under the terms of the * GNU GPL, version 2 or (at your option) any later version. */ #include "qemu-common.h" #include "block/block.h" #include "qemu/queue.h" #include "qemu/sockets.h" struct AioHandler { GPollFD pfd; EventNotifier *e; int socket; GIOChannel *channel; IOHandler *io_read; IOHandler *io_write; EventNotifierHandler *io_notify; AioFlushHandler *io_flush; int deleted; int pollfds_idx; void *opaque; QLIST_ENTRY(AioHandler) node; }; static AioHandler *find_aio_handler(AioContext *ctx, int socket) { AioHandler *node; QLIST_FOREACH(node, &ctx->aio_handlers, node) { if (node->socket == socket) if (!node->deleted) return node; } return NULL; } void aio_set_fd_handler(AioContext *ctx, int fd, IOHandler *io_read, IOHandler *io_write, AioFlushHandler *io_flush, void *opaque) { AioHandler *node; node = find_aio_handler(ctx, fd); /* Are we deleting the fd handler? */ if (!io_read && !io_write) { if (node) { g_source_remove_poll(&ctx->source, &node->pfd); g_io_channel_unref(node->channel); /* If the lock is held, just mark the node as deleted */ if (ctx->walking_handlers) { node->deleted = 1; node->pfd.revents = 0; } else { /* Otherwise, delete it for real. We can't just mark it as * deleted because deleted nodes are only cleaned up after * releasing the walking_handlers lock. */ QLIST_REMOVE(node, node); g_free(node); } } } else { if (node == NULL) { /* Alloc and insert if it's not already there */ node = g_malloc0(sizeof(AioHandler)); node->socket = fd; node->channel = g_io_channel_win32_new_socket(fd); g_io_channel_win32_make_pollfd(node->channel, (io_read ? G_IO_IN | G_IO_HUP | G_IO_ERR : 0)|(io_write ? G_IO_OUT | G_IO_ERR : 0), &node->pfd); QLIST_INSERT_HEAD(&ctx->aio_handlers, node, node); g_source_add_poll(&ctx->source, &node->pfd); } /* Update handler with latest information */ node->io_read = io_read; node->io_write = io_write; node->io_flush = io_flush; node->opaque = opaque; node->pollfds_idx = -1; } aio_notify(ctx); } void aio_set_event_notifier(AioContext *ctx, EventNotifier *notifier, EventNotifierHandler *io_notify, AioFlushEventNotifierHandler *io_flush) { AioHandler *node; QLIST_FOREACH(node, &ctx->aio_handlers, node) { if (node->e == notifier && !node->deleted) { break; } } /* Are we deleting the fd handler? */ if (!io_notify) { if (node) { g_source_remove_poll(&ctx->source, &node->pfd); /* If the lock is held, just mark the node as deleted */ if (ctx->walking_handlers) { node->deleted = 1; node->pfd.revents = 0; } else { /* Otherwise, delete it for real. We can't just mark it as * deleted because deleted nodes are only cleaned up after * releasing the walking_handlers lock. */ QLIST_REMOVE(node, node); g_free(node); } } } else { if (node == NULL) { /* Alloc and insert if it's not already there */ node = g_malloc0(sizeof(AioHandler)); node->e = notifier; node->pfd.fd = (uintptr_t)event_notifier_get_handle(notifier); node->pfd.events = G_IO_IN; QLIST_INSERT_HEAD(&ctx->aio_handlers, node, node); g_source_add_poll(&ctx->source, &node->pfd); } /* Update handler with latest information */ node->io_notify = io_notify; node->io_flush = io_flush; } aio_notify(ctx); } bool aio_pending(AioContext *ctx) { AioHandler *node; QLIST_FOREACH(node, &ctx->aio_handlers, node) { // HANDLE ? if (node->pfd.revents && node->io_notify) { return true; } // SOCKET ? int revents; revents = node->pfd.revents & node->pfd.events; if ((revents & G_I
Re: [Qemu-devel] [WIP PATCH 00/24] IDE cleanups, initial work on AHCI rerror/werror=stop
On Mon, Oct 28, 2013 at 05:43:16PM +0100, Paolo Bonzini wrote: > Hi all, > > this is some cleanup to the IDE code that I started long ago > and picked up again on my flight back from KVM Forum... > > The patches clean up the callbacks to prepare for implementing request > retry for AHCI. After these patches, PIO retry should work for AHCI and > non-NCQ retry should be very close (you need an implementation of the > new restart_dma callback). NCQ retry and save/restore is AHCI-specific > and has to be done separately, similar to how we do it for SCSI already. > > Patches 1-4 set up a new (incomplete) testcase. Patches 5-13 are no-op > cleanups, or so I hope. Patches 14-23 genericize retry and make it > "just work" for all adapters for PIO. Patch 24 is another cleanup that > could probably be moved earlier in the series. > > Throwing it out since I know Kevin and mst are working on AHCI, to avoid > doing duplicate work. Patches 2 and 4 are known to be racy and > incomplete. Extremely light testing, if something breaks tells me and > I'll look at it. > > Paolo Well ATM win7 boot is broken with non NCQ (crash on boot). Do you patches help? Also, without NCQ win 7 boot will sometime hang. The following patch by Alex makes it behave better: it still sometimes hangs for a very long time but with the patch it will after a long time proceed. Sometimes it proceeds normally. diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index a8be62c..fbea9e8 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -961,7 +961,8 @@ static int handle_cmd(AHCIState *s, int port, int slot) /* We're ready to process the command in FIS byte 2. */ ide_exec_cmd(&s->dev[port].port, cmd_fis[2]); -if (s->dev[port].port.ifs[0].status & READY_STAT) { +if ((s->dev[port].port.ifs[0].status & (READY_STAT|DRQ_STAT|BUSY_STAT)) == +READY_STAT) { ahci_write_fis_d2h(&s->dev[port], cmd_fis); } }
[Qemu-devel] [PATCH 02/24] libqtest: return progress from qmp/qmpv
Signed-off-by: Paolo Bonzini --- tests/libqtest.c | 10 +++--- tests/libqtest.h | 17 +++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/libqtest.c b/tests/libqtest.c index bb82069..5205a43 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -291,7 +291,7 @@ redo: return words; } -void qtest_qmpv(QTestState *s, const char *fmt, va_list ap) +bool qtest_qmpv(QTestState *s, const char *fmt, va_list ap) { bool has_reply = false; int nesting = 0; @@ -324,15 +324,19 @@ void qtest_qmpv(QTestState *s, const char *fmt, va_list ap) break; } } +return has_reply; } -void qtest_qmp(QTestState *s, const char *fmt, ...) +bool qtest_qmp(QTestState *s, const char *fmt, ...) { va_list ap; +bool has_reply; va_start(ap, fmt); -qtest_qmpv(s, fmt, ap); +has_reply = qtest_qmpv(s, fmt, ap); va_end(ap); + +return has_reply; } const char *qtest_get_arch(void) diff --git a/tests/libqtest.h b/tests/libqtest.h index a6e99bd..e8a4e34 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -48,9 +48,10 @@ void qtest_quit(QTestState *s); * @s: #QTestState instance to operate on. * @fmt...: QMP message to send to qemu * - * Sends a QMP message to QEMU + * Sends a QMP message to QEMU. Returns true if there + * was a reply. */ -void qtest_qmp(QTestState *s, const char *fmt, ...); +bool qtest_qmp(QTestState *s, const char *fmt, ...); /** * qtest_qmpv: @@ -58,9 +59,10 @@ void qtest_qmp(QTestState *s, const char *fmt, ...); * @fmt: QMP message to send to QEMU * @ap: QMP message arguments * - * Sends a QMP message to QEMU. + * Sends a QMP message to QEMU. Returns true if there + * was a reply. */ -void qtest_qmpv(QTestState *s, const char *fmt, va_list ap); +bool qtest_qmpv(QTestState *s, const char *fmt, va_list ap); /** * qtest_get_irq: @@ -336,13 +338,16 @@ static inline void qtest_end(void) * * Sends a QMP message to QEMU */ -static inline void qmp(const char *fmt, ...) +static inline bool qmp(const char *fmt, ...) { va_list ap; +bool has_reply; va_start(ap, fmt); -qtest_qmpv(global_qtest, fmt, ap); +has_reply = qtest_qmpv(global_qtest, fmt, ap); va_end(ap); + +return has_reply; } /** -- 1.8.3.1
Re: [Qemu-devel] [WIP PATCH 00/24] IDE cleanups, initial work on AHCI rerror/werror=stop
Il 28/10/2013 18:16, Michael S. Tsirkin ha scritto: > On Mon, Oct 28, 2013 at 05:43:16PM +0100, Paolo Bonzini wrote: >> Hi all, >> >> this is some cleanup to the IDE code that I started long ago >> and picked up again on my flight back from KVM Forum... >> >> The patches clean up the callbacks to prepare for implementing request >> retry for AHCI. After these patches, PIO retry should work for AHCI and >> non-NCQ retry should be very close (you need an implementation of the >> new restart_dma callback). NCQ retry and save/restore is AHCI-specific >> and has to be done separately, similar to how we do it for SCSI already. >> >> Patches 1-4 set up a new (incomplete) testcase. Patches 5-13 are no-op >> cleanups, or so I hope. Patches 14-23 genericize retry and make it >> "just work" for all adapters for PIO. Patch 24 is another cleanup that >> could probably be moved earlier in the series. >> >> Throwing it out since I know Kevin and mst are working on AHCI, to avoid >> doing duplicate work. Patches 2 and 4 are known to be racy and >> incomplete. Extremely light testing, if something breaks tells me and >> I'll look at it. > > Well ATM win7 boot is broken with non NCQ (crash on boot). > Do you patches help? No, these are for another feature that is missing for AHCI, that is support for stopping the VM on I/O errors. It is https://bugzilla.redhat.com/show_bug.cgi?id=887844 > Also, without NCQ win 7 boot will sometime hang. > The following patch by Alex makes it behave better: > it still sometimes hangs for a very long time but with the patch it > will after a long time proceed. Sometimes it proceeds normally. Yeah, I saw you and Kevin discuss it on IRC. Paolo > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index a8be62c..fbea9e8 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -961,7 +961,8 @@ static int handle_cmd(AHCIState *s, int port, int slot) > /* We're ready to process the command in FIS byte 2. */ > ide_exec_cmd(&s->dev[port].port, cmd_fis[2]); > > -if (s->dev[port].port.ifs[0].status & READY_STAT) { > +if ((s->dev[port].port.ifs[0].status & > (READY_STAT|DRQ_STAT|BUSY_STAT)) == > +READY_STAT) { > ahci_write_fis_d2h(&s->dev[port], cmd_fis); > } > } >
Re: [Qemu-devel] About VM fork in QEMU
>>> External snapshots (via the blockdev-snapshot-sync QMP command) can be >>> taken in a matter of milliseconds if you only care about disk state. >>> Furthermore, if you want to take a snapshot of both memory and disk >>> state, such that the clone can be resumed from the same time, you can do >>> that with a guest downtime that only lasts as long as the >>> blockdev-snapshot-sync, by first doing a migrate to file then doing the >>> disk snapshot when the VM pauses at the end of migration. Resuming the >>> original guest is fast; resuming from the migration file is a bit >>> longer, but it is still the fastest way possible to resume from a >>> memory+disk snapshot. If you need anything faster, then yes, you would >>> have to write patches to qemu to attempt cloning via fork() that makes >>> sure to modify the active disk in use by the fork child so as not to >>> interfere with the fork parent. >> >> I think migrating memory to file then doing external disk snapshot is >> exactly what we want. Since we are using libvirt to manage different >> VMs, could you give us some specific guides (or references) that how >> we could migrate memory state to file using virsh interfaces and do >> external snapshots? > > virsh snapshot-create-as $dom $name --live --memspec /path/to/memoryfile I have tried this command on libvirt v1.1.3 and it returns "error: invalid argument: qemuDomainSnapshotCreateXML: unsupported flags (0x100)". Looks like --live is not supported yet. Could you let us know which version we should of libvirt we should use in order to use this feature? Thanks, Xinyang -- Xinyang GE Department of Computer Science & Engineering The Pennsylvania State University Homepage: http://www.cse.psu.edu/~xxg113/
Re: [Qemu-devel] [PATCH v3 0/6] Improve getauxval support
Ping. r~ On 10/17/2013 08:29 AM, Richard Henderson wrote: > Changes from v2: > * Rebase for trivial conflicts > * Fix codingstyle errors > * Drop the static fallback code > * Use getauxval for AT_EXECFD > > > r~ > > > Richard Henderson (6): > osdep: Create qemu_getauxval and qemu_init_auxval > tcg-ppc64: Use qemu_getauxval > tcg-arm: Use qemu_getauxval > tcg-s390: Use qemu_getauxval in query_facilities > util: Use qemu_getauxval in linux qemu_cache_utils_init > linux-user: Use qemu_getauxval for AT_EXECFD > > include/elf.h | 73 +++ > include/qemu/cache-utils.h | 4 +- > include/qemu/osdep.h | 25 > linux-user/main.c | 35 - > tcg/arm/tcg-target.c | 14 +++ > tcg/ppc64/tcg-target.c | 11 +- > tcg/s390/tcg-target.c | 95 > ++ > util/Makefile.objs | 1 + > util/cache-utils.c | 51 ++--- > util/getauxval.c | 74 > vl.c | 3 +- > 11 files changed, 224 insertions(+), 162 deletions(-) > create mode 100644 util/getauxval.c >
Re: [Qemu-devel] [WIP PATCH 00/24] IDE cleanups, initial work on AHCI rerror/werror=stop
On Mon, Oct 28, 2013 at 06:26:58PM +0100, Paolo Bonzini wrote: > Il 28/10/2013 18:16, Michael S. Tsirkin ha scritto: > > On Mon, Oct 28, 2013 at 05:43:16PM +0100, Paolo Bonzini wrote: > >> Hi all, > >> > >> this is some cleanup to the IDE code that I started long ago > >> and picked up again on my flight back from KVM Forum... > >> > >> The patches clean up the callbacks to prepare for implementing request > >> retry for AHCI. After these patches, PIO retry should work for AHCI and > >> non-NCQ retry should be very close (you need an implementation of the > >> new restart_dma callback). NCQ retry and save/restore is AHCI-specific > >> and has to be done separately, similar to how we do it for SCSI already. > >> > >> Patches 1-4 set up a new (incomplete) testcase. Patches 5-13 are no-op > >> cleanups, or so I hope. Patches 14-23 genericize retry and make it > >> "just work" for all adapters for PIO. Patch 24 is another cleanup that > >> could probably be moved earlier in the series. > >> > >> Throwing it out since I know Kevin and mst are working on AHCI, to avoid > >> doing duplicate work. Patches 2 and 4 are known to be racy and > >> incomplete. Extremely light testing, if something breaks tells me and > >> I'll look at it. > > > > Well ATM win7 boot is broken with non NCQ (crash on boot). > > Do you patches help? > > No, these are for another feature that is missing for AHCI, that is > support for stopping the VM on I/O errors. It is > https://bugzilla.redhat.com/show_bug.cgi?id=887844 Hmm. Actually maybe there was an io error... is there an easy way to find out? > > Also, without NCQ win 7 boot will sometime hang. > > The following patch by Alex makes it behave better: > > it still sometimes hangs for a very long time but with the patch it > > will after a long time proceed. Sometimes it proceeds normally. > > Yeah, I saw you and Kevin discuss it on IRC. > > Paolo > > > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > > index a8be62c..fbea9e8 100644 > > --- a/hw/ide/ahci.c > > +++ b/hw/ide/ahci.c > > @@ -961,7 +961,8 @@ static int handle_cmd(AHCIState *s, int port, int slot) > > /* We're ready to process the command in FIS byte 2. */ > > ide_exec_cmd(&s->dev[port].port, cmd_fis[2]); > > > > -if (s->dev[port].port.ifs[0].status & READY_STAT) { > > +if ((s->dev[port].port.ifs[0].status & > > (READY_STAT|DRQ_STAT|BUSY_STAT)) == > > +READY_STAT) { > > ahci_write_fis_d2h(&s->dev[port], cmd_fis); > > } > > } > >
Re: [Qemu-devel] [WIP PATCH 00/24] IDE cleanups, initial work on AHCI rerror/werror=stop
Il 28/10/2013 19:00, Michael S. Tsirkin ha scritto: > > No, these are for another feature that is missing for AHCI, that is > > support for stopping the VM on I/O errors. It is > > https://bugzilla.redhat.com/show_bug.cgi?id=887844 > > Hmm. Actually maybe there was an io error... is there > an easy way to find out? This would be a *host* I/O error, so I doubt. Paolo
Re: [Qemu-devel] [WIP PATCH 00/24] IDE cleanups, initial work on AHCI rerror/werror=stop
On Mon, Oct 28, 2013 at 07:36:32PM +0100, Paolo Bonzini wrote: > Il 28/10/2013 19:00, Michael S. Tsirkin ha scritto: > > > No, these are for another feature that is missing for AHCI, that is > > > support for stopping the VM on I/O errors. It is > > > https://bugzilla.redhat.com/show_bug.cgi?id=887844 > > > > Hmm. Actually maybe there was an io error... is there > > an easy way to find out? > > This would be a *host* I/O error, so I doubt. > > Paolo Yes, I know. Something like an out of range access on a disk can trigger it perhaps?
Re: [Qemu-devel] [PATCH] configure: Add config.status to recreate the current configuration
Am 20.10.2013 18:39, schrieb Stefan Weil: > The latest configure invocation was saved in config-host.mak and could > be extracted from that file to recreate the configuration. > > Now it is saved in a new file config.status which can be directly executed > to recreate the configuration. The file name and the comments were copied > from GNU autoconf. > > Makefile now uses config.status, but also includes transitional code > for the old mechanism. > > Signed-off-by: Stefan Weil > --- > > This patch still does not escape special characters in the command line. > I expect that most users don't use such characters, so they have no problem. > > It's easy to fix that for hosts with bash: simply replace %s by %q. > If required, this can be done in a later patch. > > Regards, > Stefan Weil > > Makefile |9 - > configure | 15 +-- > 2 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index b15003f..073f18b 100644 > --- a/Makefile > +++ b/Makefile > @@ -28,7 +28,14 @@ CONFIG_ALL=y > include $(SRC_PATH)/rules.mak > config-host.mak: $(SRC_PATH)/configure > @echo $@ is out-of-date, running configure > - @sed -n "/.*Configured with/s/[^:]*: //p" $@ | sh > + @# TODO: The next lines include code which supports a smooth > + @# transition from old configurations without config.status. > + @# This code can be removed after QEMU 1.7. > + @if test -x config.status; then \ > + ./config.status; \ > +else \ > + sed -n "/.*Configured with/s/[^:]*: //p" $@ | sh; \ > + fi > else > config-host.mak: > ifneq ($(filter-out %clean,$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail)) > diff --git a/configure b/configure > index 57ee62a..125d0c5 100755 > --- a/configure > +++ b/configure > @@ -27,6 +27,19 @@ printf " '%s'" "$0" "$@" >> config.log > echo >> config.log > echo "#" >> config.log > > +# Save the configure command line for later reuse. > +cat+#!/bin/sh > +# Generated by configure. > +# Run this file to recreate the current configuration. > +# Compiler output produced by configure, useful for debugging > +# configure, is in config.log if it exists. > +EOD > +printf "exec" >>config.status > +printf " '%s'" "$0" "$@" >>config.status > +echo >>config.status > +chmod +x config.status > + > error_exit() { > echo > echo "ERROR: $1" > @@ -3765,8 +3778,6 @@ config_host_mak="config-host.mak" > echo "# Automatically generated by configure - do not modify" > >config-all-disas.mak > > echo "# Automatically generated by configure - do not modify" > > $config_host_mak > -printf "# Configured with:" >> $config_host_mak > -printf " '%s'" "$0" "$@" >> $config_host_mak > echo >> $config_host_mak > > echo all: >> $config_host_mak ping?
[Qemu-devel] [PATCH] ahci fix: windows boots fine. need to review with kwolf
From: Alexander Graf When AHCI executes an asynchronous IDE command, it checked DRDY without checking either DRQ or BSY. This sometimes caused interrupt to be sent before command is actually completed. This resulted in a race condition: if guest then managed to access the device before command has completed, it would hang waiting for an interrupt. This was observed with windows 7 guests. To fix, check for DRQ or BSY in additiona to DRDY, if set, the command is asynchronous so delay the interrupt until asynchronous done callback is invoked. Reported-by: Michael S. Tsirkin Reviewed-by: Michael S. Tsirkin Tested-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/ide/ahci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index a8be62c..fbea9e8 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -961,7 +961,8 @@ static int handle_cmd(AHCIState *s, int port, int slot) /* We're ready to process the command in FIS byte 2. */ ide_exec_cmd(&s->dev[port].port, cmd_fis[2]); -if (s->dev[port].port.ifs[0].status & READY_STAT) { +if ((s->dev[port].port.ifs[0].status & (READY_STAT|DRQ_STAT|BUSY_STAT)) == +READY_STAT) { ahci_write_fis_d2h(&s->dev[port], cmd_fis); } } -- MST
[Qemu-devel] [PATCH repost] ahci: fix win7 hang on boot
From: Alexander Graf When AHCI executes an asynchronous IDE command, it checked DRDY without checking either DRQ or BSY. This sometimes caused interrupt to be sent before command is actually completed. This resulted in a race condition: if guest then managed to access the device before command has completed, it would hang waiting for an interrupt. This was observed with windows 7 guests. To fix, check for DRQ or BSY in additiona to DRDY, if set, the command is asynchronous so delay the interrupt until asynchronous done callback is invoked. Reported-by: Michael S. Tsirkin Reviewed-by: Michael S. Tsirkin Tested-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/ide/ahci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index a8be62c..fbea9e8 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -961,7 +961,8 @@ static int handle_cmd(AHCIState *s, int port, int slot) /* We're ready to process the command in FIS byte 2. */ ide_exec_cmd(&s->dev[port].port, cmd_fis[2]); -if (s->dev[port].port.ifs[0].status & READY_STAT) { +if ((s->dev[port].port.ifs[0].status & (READY_STAT|DRQ_STAT|BUSY_STAT)) == +READY_STAT) { ahci_write_fis_d2h(&s->dev[port], cmd_fis); } } -- MST
Re: [Qemu-devel] [PATCH] ahci fix: windows boots fine. need to review with kwolf
On Mon, Oct 28, 2013 at 09:00:15PM +0200, Michael S. Tsirkin wrote: > From: Alexander Graf > > When AHCI executes an asynchronous IDE command, it checked DRDY without > checking either DRQ or BSY. This sometimes caused interrupt to be sent > before command is actually completed. > > This resulted in a race condition: if guest then managed to access the > device before command has completed, it would hang waiting for an > interrupt. > This was observed with windows 7 guests. > > To fix, check for DRQ or BSY in additiona to DRDY, if set, > the command is asynchronous so delay the interrupt until > asynchronous done callback is invoked. > > Reported-by: Michael S. Tsirkin > Reviewed-by: Michael S. Tsirkin > Tested-by: Michael S. Tsirkin > Signed-off-by: Michael S. Tsirkin Sorry about the subject, forgot to update it. I reposted with a fixed subject. > --- > hw/ide/ahci.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index a8be62c..fbea9e8 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -961,7 +961,8 @@ static int handle_cmd(AHCIState *s, int port, int slot) > /* We're ready to process the command in FIS byte 2. */ > ide_exec_cmd(&s->dev[port].port, cmd_fis[2]); > > -if (s->dev[port].port.ifs[0].status & READY_STAT) { > +if ((s->dev[port].port.ifs[0].status & > (READY_STAT|DRQ_STAT|BUSY_STAT)) == > +READY_STAT) { > ahci_write_fis_d2h(&s->dev[port], cmd_fis); > } > } > -- > MST
[Qemu-devel] s390/s390x
I would like to clarify about the support given to s390/s390x from QEMU. Would you please give me an address where I can read about? I'm curious about it. Up to now, I used Hercules to deploy z/OS... can I deploy z/OS or z/VM directly under QEMU? Thanks in advance, Carlos
Re: [Qemu-devel] [PATCH] cpu-exec: Fix compiler warning (-Werror=clobbered)
Am 18.09.2013 09:48, schrieb Jan Kiszka: > On 2013-09-18 09:26, Peter Maydell wrote: [...] >> And gcc's documentation of the 'noreturn' attribute specifically >> says it does not affect the exceptional path where the function >> returns via longjmp. > OK, that is the clarifying bit of information. > > Now the question is if want to drop support for faulty compilers again, > work around the false-positive warning, or avoid the issue differently > than via reloading. > > Jan Recently commit 6c78f29a2424622bfc9c30dfbbc13404481eacb6 added a third variable which is reloaded now. Obviously the clang compiler needs this workaround. Jan, can you remember whether the initial problems were also caused by clang? If yes, we might restrict the code to that compiler. This would avoid the -Wclobbered warnings with newer gcc while still fixing the code generated by clang. Stefan
[Qemu-devel] qemu: mempath: prefault pages manually (v3)
MAP_POPULATE mmap flag does not cause mmap to fail if allocation of the entire area is not performed. HugeTLBfs performs reservation of pages on a global basis: any further restriction to the reserved memory such as cpusets placement or numa node policy is performed at fault time only. Manually fault in pages at allocation time. This allows memory restrictions to be applied before guest initialization. Signed-off-by: Marcelo Tosatti diff --git a/exec.c b/exec.c index 2e31ffc..e6bdcec 100644 --- a/exec.c +++ b/exec.c @@ -902,6 +902,13 @@ static long gethugepagesize(const char *path) return fs.f_bsize; } +static sigjmp_buf sigjump; + +static void sigbus_handler(int signal) +{ +siglongjmp(sigjump, 1); +} + static void *file_ram_alloc(RAMBlock *block, ram_addr_t memory, const char *path) @@ -911,9 +918,6 @@ static void *file_ram_alloc(RAMBlock *block, char *c; void *area; int fd; -#ifdef MAP_POPULATE -int flags; -#endif unsigned long hpagesize; hpagesize = gethugepagesize(path); @@ -961,21 +965,52 @@ static void *file_ram_alloc(RAMBlock *block, if (ftruncate(fd, memory)) perror("ftruncate"); -#ifdef MAP_POPULATE -/* NB: MAP_POPULATE won't exhaustively alloc all phys pages in the case - * MAP_PRIVATE is requested. For mem_prealloc we mmap as MAP_SHARED - * to sidestep this quirk. - */ -flags = mem_prealloc ? MAP_POPULATE | MAP_SHARED : MAP_PRIVATE; -area = mmap(0, memory, PROT_READ | PROT_WRITE, flags, fd, 0); -#else area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); -#endif if (area == MAP_FAILED) { perror("file_ram_alloc: can't mmap RAM pages"); close(fd); return (NULL); } + +if (mem_prealloc) { +int ret, i; +struct sigaction act, oldact; +sigset_t set, oldset; + +memset(&act, 0, sizeof(act)); +act.sa_handler = &sigbus_handler; +act.sa_flags = 0; + +ret = sigaction(SIGBUS, &act, &oldact); +if (ret) { +perror("file_ram_alloc: fail to install signal handler"); +exit(1); +} + +/* unblock SIGBUS */ +sigemptyset(&set); +sigaddset(&set, SIGBUS); +pthread_sigmask(SIG_UNBLOCK, &set, &oldset); + +if (sigsetjmp(sigjump, 1)) { +fprintf(stderr, "file_ram_alloc: failed to preallocate pages\n"); +exit(1); +} + +/* MAP_POPULATE silently ignores failures */ +for (i = 0; i < (memory/hpagesize)-1; i++) { +memset(area + (hpagesize*i), 0, 1); +} + +ret = sigaction(SIGBUS, &oldact, NULL); +if (ret) { +perror("file_ram_alloc: fail to reinstall signal handler"); +exit(1); +} + +pthread_sigmask(SIG_SETMASK, &oldset, NULL); +} + block->fd = fd; return area; } diff --git a/qemu-options.hx b/qemu-options.hx index 5dc8b75..3674b3c 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -228,7 +228,6 @@ STEXI Allocate guest RAM from a temporarily created file in @var{path}. ETEXI -#ifdef MAP_POPULATE DEF("mem-prealloc", 0, QEMU_OPTION_mem_prealloc, "-mem-prealloc preallocate guest memory (use with -mem-path)\n", QEMU_ARCH_ALL) @@ -237,7 +236,6 @@ STEXI @findex -mem-prealloc Preallocate memory when using -mem-path. ETEXI -#endif DEF("k", HAS_ARG, QEMU_OPTION_k, "-k language use keyboard layout (for example 'fr' for French)\n", diff --git a/vl.c b/vl.c index b42ac67..1e28c5c 100644 --- a/vl.c +++ b/vl.c @@ -188,9 +188,7 @@ static int display_remote; const char* keyboard_layout = NULL; ram_addr_t ram_size; const char *mem_path = NULL; -#ifdef MAP_POPULATE int mem_prealloc = 0; /* force preallocation of physical target memory */ -#endif int nb_nics; NICInfo nd_table[MAX_NICS]; int autostart; @@ -3205,11 +3203,9 @@ int main(int argc, char **argv, char **envp) case QEMU_OPTION_mempath: mem_path = optarg; break; -#ifdef MAP_POPULATE case QEMU_OPTION_mem_prealloc: mem_prealloc = 1; break; -#endif case QEMU_OPTION_d: log_mask = optarg; break;
[Qemu-devel] qemu: mempath: prefault pages manually (v4)
v4: s/fail/failed/ (Peter Maydell) --- MAP_POPULATE mmap flag does not cause mmap to fail if allocation of the entire area is not performed. HugeTLBfs performs reservation of pages on a global basis: any further restriction to the reserved memory such as cpusets placement or numa node policy is performed at fault time only. Manually fault in pages at allocation time. This allows memory restrictions to be applied before guest initialization. Signed-off-by: Marcelo Tosatti diff --git a/exec.c b/exec.c index 2e31ffc..e6bdcec 100644 --- a/exec.c +++ b/exec.c @@ -902,6 +902,13 @@ static long gethugepagesize(const char *path) return fs.f_bsize; } +static sigjmp_buf sigjump; + +static void sigbus_handler(int signal) +{ +siglongjmp(sigjump, 1); +} + static void *file_ram_alloc(RAMBlock *block, ram_addr_t memory, const char *path) @@ -911,9 +918,6 @@ static void *file_ram_alloc(RAMBlock *block, char *c; void *area; int fd; -#ifdef MAP_POPULATE -int flags; -#endif unsigned long hpagesize; hpagesize = gethugepagesize(path); @@ -961,21 +965,52 @@ static void *file_ram_alloc(RAMBlock *block, if (ftruncate(fd, memory)) perror("ftruncate"); -#ifdef MAP_POPULATE -/* NB: MAP_POPULATE won't exhaustively alloc all phys pages in the case - * MAP_PRIVATE is requested. For mem_prealloc we mmap as MAP_SHARED - * to sidestep this quirk. - */ -flags = mem_prealloc ? MAP_POPULATE | MAP_SHARED : MAP_PRIVATE; -area = mmap(0, memory, PROT_READ | PROT_WRITE, flags, fd, 0); -#else area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); -#endif if (area == MAP_FAILED) { perror("file_ram_alloc: can't mmap RAM pages"); close(fd); return (NULL); } + +if (mem_prealloc) { +int ret, i; +struct sigaction act, oldact; +sigset_t set, oldset; + +memset(&act, 0, sizeof(act)); +act.sa_handler = &sigbus_handler; +act.sa_flags = 0; + +ret = sigaction(SIGBUS, &act, &oldact); +if (ret) { +perror("file_ram_alloc: failed to install signal handler"); +exit(1); +} + +/* unblock SIGBUS */ +sigemptyset(&set); +sigaddset(&set, SIGBUS); +pthread_sigmask(SIG_UNBLOCK, &set, &oldset); + +if (sigsetjmp(sigjump, 1)) { +fprintf(stderr, "file_ram_alloc: failed to preallocate pages\n"); +exit(1); +} + +/* MAP_POPULATE silently ignores failures */ +for (i = 0; i < (memory/hpagesize)-1; i++) { +memset(area + (hpagesize*i), 0, 1); +} + +ret = sigaction(SIGBUS, &oldact, NULL); +if (ret) { +perror("file_ram_alloc: failed to reinstall signal handler"); +exit(1); +} + +pthread_sigmask(SIG_SETMASK, &oldset, NULL); +} + block->fd = fd; return area; } diff --git a/qemu-options.hx b/qemu-options.hx index 5dc8b75..3674b3c 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -228,7 +228,6 @@ STEXI Allocate guest RAM from a temporarily created file in @var{path}. ETEXI -#ifdef MAP_POPULATE DEF("mem-prealloc", 0, QEMU_OPTION_mem_prealloc, "-mem-prealloc preallocate guest memory (use with -mem-path)\n", QEMU_ARCH_ALL) @@ -237,7 +236,6 @@ STEXI @findex -mem-prealloc Preallocate memory when using -mem-path. ETEXI -#endif DEF("k", HAS_ARG, QEMU_OPTION_k, "-k language use keyboard layout (for example 'fr' for French)\n", diff --git a/vl.c b/vl.c index b42ac67..1e28c5c 100644 --- a/vl.c +++ b/vl.c @@ -188,9 +188,7 @@ static int display_remote; const char* keyboard_layout = NULL; ram_addr_t ram_size; const char *mem_path = NULL; -#ifdef MAP_POPULATE int mem_prealloc = 0; /* force preallocation of physical target memory */ -#endif int nb_nics; NICInfo nd_table[MAX_NICS]; int autostart; @@ -3205,11 +3203,9 @@ int main(int argc, char **argv, char **envp) case QEMU_OPTION_mempath: mem_path = optarg; break; -#ifdef MAP_POPULATE case QEMU_OPTION_mem_prealloc: mem_prealloc = 1; break; -#endif case QEMU_OPTION_d: log_mask = optarg; break;
[Qemu-devel] How to prevent write to partitcular sector of disk
I am trying to prevent write from DomU for particular sector of hardisk which is passed through QEMU device. I am putting an error condition in ide.c using API ide_handle_write_error(s, -ret, BM_STATUS_ERROR) called from ide_write_dma_cb, however DomU is still going ahead and writing to those sectors. Is there a way to prevent DomU from writing to certain sector of hard disk as well as propagate those error back to DomU saying, "Write error" or some message so that user is notified of error writing to certain sectors of disk. Regards Shakil ---
Re: [Qemu-devel] s390/s390x
On 10/28/2013 12:09 PM, Carlos Morales wrote: > can I deploy z/OS or z/VM directly under QEMU? No. QEMU's support is fairly Linux specific. r~
[Qemu-devel] [Bug 1245703] [NEW] LD_PREFIX option reads directories recursively in an endless loop
Public bug reported: If I run qemu user emulation with -L /path/to/my/sysroot/ in which also the proc and dev filesystem is mounted QEMU eats my memory until it gets killed by the kernel. According to the strace output it follows the symbolic links in the proc filesystem running forever in a recursive loop. The easiest solution would be to add in the function "add_dir_maybe" in the file util/path.c an additional check for symbolic links that it don't follow them. Also I don't really understand the need of doing this. A lot of ressources are wasted everytime QEMU-user is started just by having the directory structure in memory. In my case this are more than 2 entries which QEMU is loading every time. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1245703 Title: LD_PREFIX option reads directories recursively in an endless loop Status in QEMU: New Bug description: If I run qemu user emulation with -L /path/to/my/sysroot/ in which also the proc and dev filesystem is mounted QEMU eats my memory until it gets killed by the kernel. According to the strace output it follows the symbolic links in the proc filesystem running forever in a recursive loop. The easiest solution would be to add in the function "add_dir_maybe" in the file util/path.c an additional check for symbolic links that it don't follow them. Also I don't really understand the need of doing this. A lot of ressources are wasted everytime QEMU-user is started just by having the directory structure in memory. In my case this are more than 2 entries which QEMU is loading every time. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1245703/+subscriptions
Re: [Qemu-devel] [Bug 1245703] [NEW] LD_PREFIX option reads directories recursively in an endless loop
On 28 October 2013 23:15, Sebastian Macke wrote: > If I run qemu user emulation with -L /path/to/my/sysroot/ in which also > the proc and dev filesystem is mounted QEMU eats my memory until it gets > killed by the kernel. > > According to the strace output it follows the symbolic links in the proc > filesystem running forever in a recursive loop. > > The easiest solution would be to add in the function "add_dir_maybe" in > the file util/path.c an additional check for symbolic links that it > don't follow them. Yeah, this -L code is just busted. It's really only intended to work with extremely simple sysroot directories which don't have weird stuff like proc mounts or symlinks and aren't very big. If the thing you're looking at isn't like that then you might be better off using the "static qemu and chroot into the directory" approach instead. -- PMM
[Qemu-devel] [Bug 1245703] Re: LD_PREFIX option reads directories recursively in an endless loop
Ok, thanks for the info. For me it looks like removing the whole path code and putting a one-liner combining two string is the best solution. But maybe I am missing something. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1245703 Title: LD_PREFIX option reads directories recursively in an endless loop Status in QEMU: New Bug description: If I run qemu user emulation with -L /path/to/my/sysroot/ in which also the proc and dev filesystem is mounted QEMU eats my memory until it gets killed by the kernel. According to the strace output it follows the symbolic links in the proc filesystem running forever in a recursive loop. The easiest solution would be to add in the function "add_dir_maybe" in the file util/path.c an additional check for symbolic links that it don't follow them. Also I don't really understand the need of doing this. A lot of ressources are wasted everytime QEMU-user is started just by having the directory structure in memory. In my case this are more than 2 entries which QEMU is loading every time. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1245703/+subscriptions
Re: [Qemu-devel] How to introduce bs->node_name ?
On Mon, 10/28 16:40, Benoît Canet wrote: > > Hi list, > > After a discussion on irc we have two potential solution in order to introduce > a new bs->node_name member in order to be able to manipulate the graph from > the > monitors. > > The first one is to make the QMP device parameter of the block commands > optional > and add the node-name parameter as a second optional parameter. > This is Markus prefered solution and Eric is ok with making mandatory > parameters > optional in QMP. > > The second one suggested by Kevin Would be to add some magic to the new > node_name member by making it equal to device_name for backends and then > making > the qmp commands operate only on node-names. > My personnal suggestion would be that non specified node-name would be set to > "undefined" meaning that no operation could occur on this bs. > > For QMP access the device_name is accessed via bdrv_find() in a few place in > blockdev. > > Here are the occurences of it: > > commit > -- > void do_commit(Monitor *mon, const QDict *qdict) > { > const char *device = qdict_get_str(qdict, "device"); > BlockDriverState *bs; > int ret; > > if (!strcmp(device, "all")) { > ret = bdrv_commit_all(); > } else { > bs = bdrv_find(device); > if (!bs) { > monitor_printf(mon, "Device '%s' not found\n", device); > return; > } > ret = bdrv_commit(bs); > } > if (ret < 0) { > monitor_printf(mon, "'commit' error for '%s': %s\n", device, >strerror(-ret)); > } > } > > internal snapshot deletion > -- > SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device, > bool has_id, > const char *id, > bool has_name, > const char *name, > Error **errp) > { > BlockDriverState *bs = bdrv_find(device); > QEMUSnapshotInfo sn; > Error *local_err = NULL; > SnapshotInfo *info = NULL; > > > Internal snapshot preparation > - > static void internal_snapshot_prepare(BlkTransactionState *common, > Error **errp) > { > const char *device; > const char *name; > > BlockDriverState *bs; > QEMUSnapshotInfo old_sn, *sn; > bool ret; > qemu_timeval tv; > BlockdevSnapshotInternal *internal; > InternalSnapshotState *state; > int ret1; > > g_assert(common->action->kind == > TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC); > internal = common->action->blockdev_snapshot_internal_sync; > state = DO_UPCAST(InternalSnapshotState, common, common); > > /* 1. parse input */ > device = internal->device; > name = internal->name; > > /* 2. check for validation */ > bs = bdrv_find(device); > if (!bs) { > error_set(errp, QERR_DEVICE_NOT_FOUND, device); > return; > } > > Drive backup > > static void drive_backup_prepare(BlkTransactionState *common, Error **errp) > { > DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common); > DriveBackup *backup; > Error *local_err = NULL; > > assert(common->action->kind == TRANSACTION_ACTION_KIND_DRIVE_BACKUP); > backup = common->action->drive_backup; > > qmp_drive_backup(backup->device, backup->target, > backup->has_format, backup->format, > backup->sync, > backup->has_mode, backup->mode, > backup->has_speed, backup->speed, > backup->has_on_source_error, backup->on_source_error, > backup->has_on_target_error, backup->on_target_error, > &local_err); > if (error_is_set(&local_err)) { > error_propagate(errp, local_err); > state->bs = NULL; > state->job = NULL; > return; > } > > state->bs = bdrv_find(backup->device); > state->job = state->bs->job; > } > > Eject which should operate on backends > -- > void qmp_eject(const char *device, bool has_force, bool force, Error **errp) > { > BlockDriverState *bs; > > bs = bdrv_find(device); > if (!bs) { > error_set(errp, QERR_DEVICE_NOT_FOUND, device); > return; > } > > eject_device(bs, force, errp); > } > > QCow2 crypto > > void qmp_block_passwd(const char *device, const char *password, Error **errp) > { > BlockDriverState *bs; > int err; > > bs = bdrv_find(device); > if (!bs) { > error_set(errp, QERR_DEVICE_NOT_FOUND, device); > return; > } > > err = bdrv_set_key(bs, password); > if (
Re: [Qemu-devel] [PATCH] configure: Add config.status to recreate the current configuration
On Sun, 10/20 18:39, Stefan Weil wrote: > The latest configure invocation was saved in config-host.mak and could > be extracted from that file to recreate the configuration. > > Now it is saved in a new file config.status which can be directly executed > to recreate the configuration. The file name and the comments were copied > from GNU autoconf. > > Makefile now uses config.status, but also includes transitional code > for the old mechanism. > > Signed-off-by: Stefan Weil > --- > > This patch still does not escape special characters in the command line. > I expect that most users don't use such characters, so they have no problem. > > It's easy to fix that for hosts with bash: simply replace %s by %q. > If required, this can be done in a later patch. > > Regards, > Stefan Weil > > Makefile |9 - > configure | 15 +-- > 2 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index b15003f..073f18b 100644 > --- a/Makefile > +++ b/Makefile > @@ -28,7 +28,14 @@ CONFIG_ALL=y > include $(SRC_PATH)/rules.mak > config-host.mak: $(SRC_PATH)/configure > @echo $@ is out-of-date, running configure > - @sed -n "/.*Configured with/s/[^:]*: //p" $@ | sh > + @# TODO: The next lines include code which supports a smooth > + @# transition from old configurations without config.status. > + @# This code can be removed after QEMU 1.7. > + @if test -x config.status; then \ > + ./config.status; \ > +else \ > + sed -n "/.*Configured with/s/[^:]*: //p" $@ | sh; \ > + fi > else > config-host.mak: > ifneq ($(filter-out %clean,$(MAKECMDGOALS)),$(if $(MAKECMDGOALS),,fail)) > diff --git a/configure b/configure > index 57ee62a..125d0c5 100755 > --- a/configure > +++ b/configure > @@ -27,6 +27,19 @@ printf " '%s'" "$0" "$@" >> config.log > echo >> config.log > echo "#" >> config.log > > +# Save the configure command line for later reuse. > +cat+#!/bin/sh > +# Generated by configure. > +# Run this file to recreate the current configuration. > +# Compiler output produced by configure, useful for debugging > +# configure, is in config.log if it exists. > +EOD > +printf "exec" >>config.status > +printf " '%s'" "$0" "$@" >>config.status > +echo >>config.status > +chmod +x config.status > + > error_exit() { > echo > echo "ERROR: $1" > @@ -3765,8 +3778,6 @@ config_host_mak="config-host.mak" > echo "# Automatically generated by configure - do not modify" > >config-all-disas.mak > > echo "# Automatically generated by configure - do not modify" > > $config_host_mak > -printf "# Configured with:" >> $config_host_mak > -printf " '%s'" "$0" "$@" >> $config_host_mak > echo >> $config_host_mak > > echo all: >> $config_host_mak Reviewed-by: Fam Zheng
[Qemu-devel] [PATCH V15 02/11] NUMA: check if the total numa memory size is equal to ram_size
If the total number of the assigned numa nodes memory is not equal to the assigned ram size, it will write the wrong data to ACPI talb, then the guest will ignore the wrong ACPI table and recognize all memory to one node. It's buggy, we should check it to ensure that we write the right data to ACPI table. Signed-off-by: Wanlong Gao --- numa.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/numa.c b/numa.c index ce7736a..beda80e 100644 --- a/numa.c +++ b/numa.c @@ -150,6 +150,16 @@ void set_numa_nodes(void) node_mem[i] = ram_size - usedmem; } +uint64_t numa_total = 0; +for (i = 0; i < nb_numa_nodes; i++) { +numa_total += node_mem[i]; +} +if (numa_total != ram_size) { +fprintf(stderr, "qemu: numa nodes total memory size " +"should equal to ram_size\n"); +exit(1); +} + for (i = 0; i < nb_numa_nodes; i++) { if (!bitmap_empty(node_cpumask[i], MAX_CPUMASK_BITS)) { break; -- 1.8.4.1.600.g3d092bf
[Qemu-devel] [PATCH V15 01/11] NUMA: move numa related code to new file numa.c
Signed-off-by: Wanlong Gao --- Makefile.target | 2 +- cpus.c | 14 include/sysemu/cpus.h | 1 - include/sysemu/sysemu.h | 3 + numa.c | 182 vl.c| 139 +--- 6 files changed, 187 insertions(+), 154 deletions(-) create mode 100644 numa.c diff --git a/Makefile.target b/Makefile.target index af6ac7e..0197c17 100644 --- a/Makefile.target +++ b/Makefile.target @@ -109,7 +109,7 @@ endif #CONFIG_BSD_USER # # System emulator target ifdef CONFIG_SOFTMMU -obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o +obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o obj-y += qtest.o obj-y += hw/ obj-$(CONFIG_FDT) += device_tree.o diff --git a/cpus.c b/cpus.c index 398229e..473f655 100644 --- a/cpus.c +++ b/cpus.c @@ -1295,20 +1295,6 @@ static void tcg_exec_all(void) exit_request = 0; } -void set_numa_modes(void) -{ -CPUState *cpu; -int i; - -CPU_FOREACH(cpu) { -for (i = 0; i < nb_numa_nodes; i++) { -if (test_bit(cpu->cpu_index, node_cpumask[i])) { -cpu->numa_node = i; -} -} -} -} - void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg) { /* XXX: implement xxx_cpu_list for targets that still miss it */ diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h index 6502488..4f79081 100644 --- a/include/sysemu/cpus.h +++ b/include/sysemu/cpus.h @@ -23,7 +23,6 @@ extern int smp_threads; #define smp_threads 1 #endif -void set_numa_modes(void); void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg); #endif diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index cd5791e..e58ef3f 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -136,6 +136,9 @@ extern QEMUClockType rtc_clock; extern int nb_numa_nodes; extern uint64_t node_mem[MAX_NODES]; extern unsigned long *node_cpumask[MAX_NODES]; +void numa_add(const char *optarg); +void set_numa_nodes(void); +void set_numa_modes(void); #define MAX_OPTION_ROMS 16 typedef struct QEMUOptionRom { diff --git a/numa.c b/numa.c new file mode 100644 index 000..ce7736a --- /dev/null +++ b/numa.c @@ -0,0 +1,182 @@ +/* + * QEMU System Emulator + * + * Copyright (c) 2013 Fujitsu Ltd. + * Author: Wanlong Gao + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "sysemu/sysemu.h" + +static void numa_node_parse_cpus(int nodenr, const char *cpus) +{ +char *endptr; +unsigned long long value, endvalue; + +/* Empty CPU range strings will be considered valid, they will simply + * not set any bit in the CPU bitmap. + */ +if (!*cpus) { +return; +} + +if (parse_uint(cpus, &value, &endptr, 10) < 0) { +goto error; +} +if (*endptr == '-') { +if (parse_uint_full(endptr + 1, &endvalue, 10) < 0) { +goto error; +} +} else if (*endptr == '\0') { +endvalue = value; +} else { +goto error; +} + +if (endvalue >= MAX_CPUMASK_BITS) { +endvalue = MAX_CPUMASK_BITS - 1; +fprintf(stderr, +"qemu: NUMA: A max of %d VCPUs are supported\n", + MAX_CPUMASK_BITS); +} + +if (endvalue < value) { +goto error; +} + +bitmap_set(node_cpumask[nodenr], value, endvalue-value+1); +return; + +error: +fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus); +exit(1); +} + +void numa_add(const char *optarg) +{ +char option[128]; +char *endptr; +unsigned long long nodenr; + +optarg = get_opt_name(option, 128, optarg, ','); +if (*optarg == ',') { +optarg++; +} +if (!strcmp(option, "node")) { + +if (nb_numa_nodes >= MAX_NODES) { +fprintf(s
[Qemu-devel] [PATCH V15 08/11] NUMA: parse guest numa nodes memory policy
The memory policy setting format is like: policy={default|membind|interleave|preferred}[,relative=true],host-nodes=N-N And we are adding this setting as a suboption of "-numa mem,", the memory policy then can be set like following: -numa node,nodeid=0,cpus=0 \ -numa node,nodeid=1,cpus=1 \ -numa mem,nodeid=0,size=1G,policy=membind,host-nodes=0-1 \ -numa mem,nodeid=1,size=1G,policy=interleave,relative=true,host-nodes=1 Signed-off-by: Wanlong Gao --- include/sysemu/sysemu.h | 3 +++ numa.c | 18 ++ qapi-schema.json| 33 +++-- vl.c| 3 +++ 4 files changed, 55 insertions(+), 2 deletions(-) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 995cf3b..9707195 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -139,6 +139,9 @@ extern int nb_numa_mem_nodes; typedef struct node_info { uint64_t node_mem; DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS); +DECLARE_BITMAP(host_mem, MAX_NODES); +NumaNodePolicy policy; +bool relative; } NodeInfo; extern NodeInfo numa_info[MAX_NODES]; void set_numa_nodes(void); diff --git a/numa.c b/numa.c index c676c5e..da4dbbd 100644 --- a/numa.c +++ b/numa.c @@ -78,6 +78,7 @@ static int numa_mem_parse(NumaMemOptions *opts) { uint16_t nodenr; uint64_t mem_size; +uint16List *nodes; if (opts->has_nodeid) { nodenr = opts->nodeid; @@ -96,6 +97,23 @@ static int numa_mem_parse(NumaMemOptions *opts) numa_info[nodenr].node_mem = mem_size; } +if (opts->has_policy) { +numa_info[nodenr].policy = opts->policy; +} + +if (opts->has_relative) { +numa_info[nodenr].relative = opts->relative; +} + +for (nodes = opts->host_nodes; nodes; nodes = nodes->next) { +if (nodes->value > MAX_NODES) { +fprintf(stderr, "qemu: node number %" PRIu16 " is bigger than %d\n", +nodes->value, MAX_NODES); +continue; +} +bitmap_set(numa_info[nodenr].host_mem, nodes->value, 1); +} + return 0; } diff --git a/qapi-schema.json b/qapi-schema.json index a19e453..804d44a 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4221,6 +4221,26 @@ '*mem':'str' }} ## +# @NumaNodePolicy +# +# NUMA node policy types +# +# @default: restore default policy, remove any nondefault policy +# +# @preferred: set the preferred node for allocation +# +# @membind: a strict policy that restricts memory allocation to the +# nodes specified +# +# @interleave: the page allocations is interleaved across the set +# of nodes specified +# +# Since 1.7 +## +{ 'enum': 'NumaNodePolicy', + 'data': [ 'default', 'preferred', 'membind', 'interleave' ] } + +## # @NumaMemOptions # # Set memory information of guest NUMA node. (for OptsVisitor) @@ -4229,9 +4249,18 @@ # # @size: #optional memory size of this node # +# @policy: #optional memory policy of this node +# +# @relative: #optional if the nodes specified are relative +# +# @host-nodes: #optional host nodes for its memory policy +# # Since 1.7 ## { 'type': 'NumaMemOptions', 'data': { - '*nodeid': 'uint16', - '*size': 'size' }} + '*nodeid': 'uint16', + '*size': 'size', + '*policy': 'NumaNodePolicy', + '*relative': 'bool', + '*host-nodes': ['uint16'] }} diff --git a/vl.c b/vl.c index 190d5d7..0d7a663 100644 --- a/vl.c +++ b/vl.c @@ -2814,6 +2814,9 @@ int main(int argc, char **argv, char **envp) for (i = 0; i < MAX_NODES; i++) { numa_info[i].node_mem = 0; bitmap_zero(numa_info[i].node_cpu, MAX_CPUMASK_BITS); +bitmap_zero(numa_info[i].host_mem, MAX_NODES); +numa_info[i].policy = NUMA_NODE_POLICY_DEFAULT; +numa_info[i].relative = false; } nb_numa_nodes = 0; -- 1.8.4.1.600.g3d092bf
[Qemu-devel] [PATCH V15 00/11] Add support for binding guest numa nodes to host numa nodes
As you know, QEMU can't direct it's memory allocation now, this may cause guest cross node access performance regression. And, the worse thing is that if PCI-passthrough is used, direct-attached-device uses DMA transfer between device and qemu process. All pages of the guest will be pinned by get_user_pages(). KVM_ASSIGN_PCI_DEVICE ioctl kvm_vm_ioctl_assign_device() =>kvm_assign_device() => kvm_iommu_map_memslots() => kvm_iommu_map_pages() => kvm_pin_pages() So, with direct-attached-device, all guest page's page count will be +1 and any page migration will not work. AutoNUMA won't too. So, we should set the guest nodes memory allocation policy before the pages are really mapped. According to this patch set, we are able to set guest nodes memory policy like following: -numa node,nodeid=0,cpus=0, \ -numa mem,size=1024M,policy=membind,host-nodes=0-1 \ -numa node,nodeid=1,cpus=1 \ -numa mem,size=1024M,policy=interleave,host-nodes=1 This supports "policy={default|membind|interleave|preferred},relative=true,host-nodes=N-N" like format. And add a QMP command "query-numa" to show numa info through this API. And convert the "info numa" monitor command to use this QMP command "query-numa". This version removes "set-mem-policy" qmp and hmp commands temporarily as Marcelo and Paolo suggested. V1->V2: change to use QemuOpts in numa options (Paolo) handle Error in mpol parser (Paolo) change qmp command format to mem-policy=membind,mem-hostnode=0-1 like (Paolo) V2->V3: also handle Error in cpus parser (5/10) split out common parser from cpus and hostnode parser (Bandan 6/10) V3-V4: rebase to request for comments V4->V5: use OptVisitor and split -numa option (Paolo) - s/set-mpol/set-mem-policy (Andreas) - s/mem-policy/policy - s/mem-hostnode/host-nodes fix hmp command process after error (Luiz) add qmp command query-numa and convert info numa to it (Luiz) V5->V6: remove tabs in json file (Laszlo, Paolo) add back "-numa node,mem=xxx" as legacy (Paolo) change cpus and host-nodes to array (Laszlo, Eric) change "nodeid" to "uint16" add NumaMemPolicy enum type (Eric) rebased on Laszlo's "OptsVisitor: support / flatten integer ranges for repeating options" patch set, thanks for Laszlo's help V6-V7: change UInt16 to uint16 (Laszlo) fix a typo in adding qmp command set-mem-policy V7-V8: rebase to current master with Laszlo's V2 of OptsVisitor patch set fix an adding white space line error V8->V9: rebase to current master check if total numa memory size is equal to ram_size (Paolo) add comments to the OptsVisitor stuff in qapi-schema.json (Eric, Laszlo) replace the use of numa_num_configured_nodes() (Andrew) avoid abusing the fact i==nodeid (Andrew) V9->V10: rebase to current master remove libnuma (Andrew) MAX_NODES=64 -> MAX_NODES=128 since libnuma selected 128 (Andrew) use MAX_NODES instead of MAX_CPUMASK_BITS for host_mem bitmap (Andrew) remove a useless clear_bit() operation (Andrew) V10->V11: rebase to current master fix "maxnode" argument of mbind(2) V11->V12: rebase to current master split patch 02/11 of V11 (Eduardo) add some max value check (Eduardo) split MAX_NODES change patch (Eduardo) V12->V13: rebase to current master thanks for Luiz's review (Luiz) doc hmp command set-mem-policy (Luiz) rename: NUMAInfo -> NUMANode (Luiz) V13->V14: remove "set-mem-policy" qmp and hmp commands (Marcelo, Paolo) V14->V15: rebase to the current master *I hope this can catch up the train of 1.7.* Wanlong Gao (11): NUMA: move numa related code to new file numa.c NUMA: check if the total numa memory size is equal to ram_size NUMA: Add numa_info structure to contain numa nodes info NUMA: convert -numa option to use OptsVisitor NUMA: introduce NumaMemOptions NUMA: add "-numa mem," options NUMA: expand MAX_NODES from 64 to 128 NUMA: parse guest numa nodes memory policy NUMA: set guest numa nodes memory policy NUMA: add qmp command query-numa NUMA: convert hmp command info_numa to use qmp command query_numa Makefile.target | 2 +- cpus.c | 14 -- hmp.c | 57 +++ hmp.h | 1 + hw/i386/pc.c| 4 +- include/sysemu/cpus.h | 1 - include/sysemu/sysemu.h | 18 ++- monitor.c | 21 +-- numa.c | 395 qapi-schema.json| 112 ++ qemu-options.hx | 6 +- qmp-commands.hx | 49 ++ vl.c| 160 +++- 13 files changed, 655 insertions(+), 185 deletions(-) create mode 100644 numa.c -- 1.8.4.1.600.g3d092bf
[Qemu-devel] [PATCH V15 03/11] NUMA: Add numa_info structure to contain numa nodes info
Add the numa_info structure to contain the numa nodes memory, VCPUs information and the future added numa nodes host memory policies. Reviewed-by: Eduardo Habkost Signed-off-by: Andre Przywara Signed-off-by: Wanlong Gao --- hw/i386/pc.c| 4 ++-- include/sysemu/sysemu.h | 8 ++-- monitor.c | 2 +- numa.c | 23 --- vl.c| 7 +++ 5 files changed, 24 insertions(+), 20 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 0c313fe..b0fddd0 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -652,14 +652,14 @@ static FWCfgState *bochs_bios_init(void) unsigned int apic_id = x86_cpu_apic_id_from_index(i); assert(apic_id < apic_id_limit); for (j = 0; j < nb_numa_nodes; j++) { -if (test_bit(i, node_cpumask[j])) { +if (test_bit(i, numa_info[j].node_cpu)) { numa_fw_cfg[apic_id + 1] = cpu_to_le64(j); break; } } } for (i = 0; i < nb_numa_nodes; i++) { -numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(node_mem[i]); +numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(numa_info[i].node_mem); } fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg, (1 + apic_id_limit + nb_numa_nodes) * diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index e58ef3f..3f3764d 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -9,6 +9,7 @@ #include "qapi-types.h" #include "qemu/notify.h" #include "qemu/main-loop.h" +#include "qemu/bitmap.h" /* vl.c */ @@ -134,8 +135,11 @@ extern QEMUClockType rtc_clock; #define MAX_NODES 64 #define MAX_CPUMASK_BITS 255 extern int nb_numa_nodes; -extern uint64_t node_mem[MAX_NODES]; -extern unsigned long *node_cpumask[MAX_NODES]; +typedef struct node_info { +uint64_t node_mem; +DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS); +} NodeInfo; +extern NodeInfo numa_info[MAX_NODES]; void numa_add(const char *optarg); void set_numa_nodes(void); void set_numa_modes(void); diff --git a/monitor.c b/monitor.c index 74f3f1b..be34488 100644 --- a/monitor.c +++ b/monitor.c @@ -2010,7 +2010,7 @@ static void do_info_numa(Monitor *mon, const QDict *qdict) } monitor_printf(mon, "\n"); monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i, -node_mem[i] >> 20); +numa_info[i].node_mem >> 20); } } diff --git a/numa.c b/numa.c index beda80e..1bc0fad 100644 --- a/numa.c +++ b/numa.c @@ -61,7 +61,7 @@ static void numa_node_parse_cpus(int nodenr, const char *cpus) goto error; } -bitmap_set(node_cpumask[nodenr], value, endvalue-value+1); +bitmap_set(numa_info[nodenr].node_cpu, value, endvalue-value+1); return; error: @@ -101,7 +101,7 @@ void numa_add(const char *optarg) } if (get_param_value(option, 128, "mem", optarg) == 0) { -node_mem[nodenr] = 0; +numa_info[nodenr].node_mem = 0; } else { int64_t sval; sval = strtosz(option, &endptr); @@ -109,7 +109,7 @@ void numa_add(const char *optarg) fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg); exit(1); } -node_mem[nodenr] = sval; +numa_info[nodenr].node_mem = sval; } if (get_param_value(option, 128, "cpus", optarg) != 0) { numa_node_parse_cpus(nodenr, option); @@ -134,7 +134,7 @@ void set_numa_nodes(void) * and distribute the available memory equally across all nodes */ for (i = 0; i < nb_numa_nodes; i++) { -if (node_mem[i] != 0) +if (numa_info[i].node_mem != 0) break; } if (i == nb_numa_nodes) { @@ -144,15 +144,16 @@ void set_numa_nodes(void) * the final node gets the rest. */ for (i = 0; i < nb_numa_nodes - 1; i++) { -node_mem[i] = (ram_size / nb_numa_nodes) & ~((1 << 23UL) - 1); -usedmem += node_mem[i]; +numa_info[i].node_mem = (ram_size / nb_numa_nodes) & +~((1 << 23UL) - 1); +usedmem += numa_info[i].node_mem; } -node_mem[i] = ram_size - usedmem; +numa_info[i].node_mem = ram_size - usedmem; } uint64_t numa_total = 0; for (i = 0; i < nb_numa_nodes; i++) { -numa_total += node_mem[i]; +numa_total += numa_info[i].node_mem; } if (numa_total != ram_size) { fprintf(stderr, "qemu: numa nodes total memory size " @@ -161,7 +162,7 @@ void set_numa_nodes(void) } for (i = 0; i < nb_numa_nodes; i++) { -if (!bitmap_empty(node_cpumask[i], MAX_CPUMASK_BITS)) { +if (!bitmap_empty(numa_info[i].node_cpu, MAX_CPUMASK_BITS)) {
[Qemu-devel] [PATCH V15 04/11] NUMA: convert -numa option to use OptsVisitor
Signed-off-by: Wanlong Gao --- include/sysemu/sysemu.h | 3 +- numa.c | 148 +++- qapi-schema.json| 30 ++ vl.c| 11 +++- 4 files changed, 114 insertions(+), 78 deletions(-) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 3f3764d..c9fb2c7 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -140,9 +140,10 @@ typedef struct node_info { DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS); } NodeInfo; extern NodeInfo numa_info[MAX_NODES]; -void numa_add(const char *optarg); void set_numa_nodes(void); void set_numa_modes(void); +extern QemuOptsList qemu_numa_opts; +int numa_init_func(QemuOpts *opts, void *opaque); #define MAX_OPTION_ROMS 16 typedef struct QEMUOptionRom { diff --git a/numa.c b/numa.c index 1bc0fad..c4fa665 100644 --- a/numa.c +++ b/numa.c @@ -24,101 +24,97 @@ */ #include "sysemu/sysemu.h" - -static void numa_node_parse_cpus(int nodenr, const char *cpus) +#include "qapi-visit.h" +#include "qapi/opts-visitor.h" +#include "qapi/dealloc-visitor.h" +QemuOptsList qemu_numa_opts = { +.name = "numa", +.implied_opt_name = "type", +.head = QTAILQ_HEAD_INITIALIZER(qemu_numa_opts.head), +.desc = { { 0 } } /* validated with OptsVisitor */ +}; + +static int numa_node_parse(NumaNodeOptions *opts) { -char *endptr; -unsigned long long value, endvalue; - -/* Empty CPU range strings will be considered valid, they will simply - * not set any bit in the CPU bitmap. - */ -if (!*cpus) { -return; -} +uint16_t nodenr; +uint16List *cpus = NULL; -if (parse_uint(cpus, &value, &endptr, 10) < 0) { -goto error; -} -if (*endptr == '-') { -if (parse_uint_full(endptr + 1, &endvalue, 10) < 0) { -goto error; -} -} else if (*endptr == '\0') { -endvalue = value; +if (opts->has_nodeid) { +nodenr = opts->nodeid; } else { -goto error; +nodenr = nb_numa_nodes; } -if (endvalue >= MAX_CPUMASK_BITS) { -endvalue = MAX_CPUMASK_BITS - 1; -fprintf(stderr, -"qemu: NUMA: A max of %d VCPUs are supported\n", - MAX_CPUMASK_BITS); +if (nodenr >= MAX_NODES) { +fprintf(stderr, "qemu: Max number of NUMA nodes reached: %" +PRIu16 "\n", nodenr); +return -1; } -if (endvalue < value) { -goto error; +for (cpus = opts->cpus; cpus; cpus = cpus->next) { +if (cpus->value > MAX_CPUMASK_BITS) { +fprintf(stderr, "qemu: cpu number %" PRIu16 " is bigger than %d", +cpus->value, MAX_CPUMASK_BITS); +continue; +} +bitmap_set(numa_info[nodenr].node_cpu, cpus->value, 1); } -bitmap_set(numa_info[nodenr].node_cpu, value, endvalue-value+1); -return; +if (opts->has_mem) { +int64_t mem_size; +char *endptr; +mem_size = strtosz(opts->mem, &endptr); +if (mem_size < 0 || *endptr) { +fprintf(stderr, "qemu: invalid numa mem size: %s\n", opts->mem); +return -1; +} +numa_info[nodenr].node_mem = mem_size; +} -error: -fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus); -exit(1); +return 0; } -void numa_add(const char *optarg) +int numa_init_func(QemuOpts *opts, void *opaque) { -char option[128]; -char *endptr; -unsigned long long nodenr; - -optarg = get_opt_name(option, 128, optarg, ','); -if (*optarg == ',') { -optarg++; +NumaOptions *object = NULL; +Error *err = NULL; +int ret = 0; + +{ +OptsVisitor *ov = opts_visitor_new(opts); +visit_type_NumaOptions(opts_get_visitor(ov), &object, NULL, &err); +opts_visitor_cleanup(ov); } -if (!strcmp(option, "node")) { - -if (nb_numa_nodes >= MAX_NODES) { -fprintf(stderr, "qemu: too many NUMA nodes\n"); -exit(1); -} -if (get_param_value(option, 128, "nodeid", optarg) == 0) { -nodenr = nb_numa_nodes; -} else { -if (parse_uint_full(option, &nodenr, 10) < 0) { -fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option); -exit(1); -} -} - -if (nodenr >= MAX_NODES) { -fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr); -exit(1); -} +if (error_is_set(&err)) { +fprintf(stderr, "qemu: %s\n", error_get_pretty(err)); +error_free(err); +ret = -1; +goto error; +} -if (get_param_value(option, 128, "mem", optarg) == 0) { -numa_info[nodenr].node_mem = 0; -} else { -int64_t sval; -sval = strtosz(option, &endptr); -if (sval < 0 || *endptr) { -fprintf(stderr, "qemu: invalid numa mem size: %s\n"
[Qemu-devel] [PATCH V15 10/11] NUMA: add qmp command query-numa
Add qmp command query-numa to show guest NUMA information. Reviewed-by: Luiz Capitulino Signed-off-by: Wanlong Gao --- numa.c | 66 qapi-schema.json | 36 +++ qmp-commands.hx | 49 + 3 files changed, 151 insertions(+) diff --git a/numa.c b/numa.c index 915a67a..b392190 100644 --- a/numa.c +++ b/numa.c @@ -28,6 +28,7 @@ #include "qapi/opts-visitor.h" #include "qapi/dealloc-visitor.h" #include "exec/memory.h" +#include "qmp-commands.h" #ifdef __linux__ #include @@ -327,3 +328,68 @@ void set_numa_modes(void) } } } + +NUMANodeList *qmp_query_numa(Error **errp) +{ +NUMANodeList *head = NULL, *cur_item = NULL; +CPUState *cpu; +int i; + +for (i = 0; i < nb_numa_nodes; i++) { +NUMANodeList *info; +uint16List *cur_cpu_item = NULL; +info = g_malloc0(sizeof(*info)); +info->value = g_malloc0(sizeof(*info->value)); +info->value->nodeid = i; +CPU_FOREACH(cpu) { +if (cpu->numa_node == i) { +uint16List *node_cpu = g_malloc0(sizeof(*node_cpu)); +node_cpu->value = cpu->cpu_index; + +if (!cur_cpu_item) { +info->value->cpus = cur_cpu_item = node_cpu; +} else { +cur_cpu_item->next = node_cpu; +cur_cpu_item = node_cpu; +} +} +} +info->value->memory = numa_info[i].node_mem; + +#ifdef __linux__ +info->value->policy = numa_info[i].policy; +info->value->relative = numa_info[i].relative; + +unsigned long first, next; +next = first = find_first_bit(numa_info[i].host_mem, MAX_NODES); +if (first == MAX_NODES) { +goto end; +} +uint16List *cur_node_item = g_malloc0(sizeof(*cur_node_item)); +cur_node_item->value = first; +info->value->host_nodes = cur_node_item; +do { +next = find_next_bit(numa_info[i].host_mem, MAX_NODES, + next + 1); +if (next == MAX_NODES) { +break; +} + +uint16List *host_node = g_malloc0(sizeof(*host_node)); +host_node->value = next; +cur_node_item->next = host_node; +cur_node_item = host_node; +} while (true); +end: +#endif + +if (!cur_item) { +head = cur_item = info; +} else { +cur_item->next = info; +cur_item = info; +} +} + +return head; +} diff --git a/qapi-schema.json b/qapi-schema.json index 804d44a..86e001e 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4264,3 +4264,39 @@ '*policy': 'NumaNodePolicy', '*relative': 'bool', '*host-nodes': ['uint16'] }} + +## +# @NUMANode: +# +# Information of guest NUMA node +# +# @nodeid: NUMA node ID +# +# @cpus: VCPUs contained in this node +# +# @memory: memory size of this node +# +# @policy: memory policy of this node +# +# @relative: if host nodes are relative for memory policy +# +# @host-nodes: host nodes for its memory policy +# +# Since: 1.7 +# +## +{ 'type': 'NUMANode', + 'data': {'nodeid': 'uint16', 'cpus': ['uint16'], 'memory': 'uint64', + 'policy': 'NumaNodePolicy', 'relative': 'bool', + 'host-nodes': ['uint16'] }} + +## +# @query-numa: +# +# Returns a list of information about each guest node. +# +# Returns: a list of @NUMANode for each guest node +# +# Since: 1.7 +## +{ 'command': 'query-numa', 'returns': ['NUMANode'] } diff --git a/qmp-commands.hx b/qmp-commands.hx index fba15cd..c2bc508 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -3295,3 +3295,52 @@ Example (2): <- { "return": {} } EQMP + +{ +.name = "query-numa", +.args_type = "", +.mhandler.cmd_new = qmp_marshal_input_query_numa, +}, + +SQMP +query-numa +- + +Show NUMA information. + +Return a json-array. Each NUMA node is represented by a json-object, +which contains: + +- "nodeid": NUMA node ID (json-int) +- "cpus": a json-arry of contained VCPUs +- "memory": amount of memory in each node in Byte (json-int) +- "policy": memory policy of this node (json-string) +- "relative": if host nodes is relative for its memory policy (json-bool) +- "host-nodes": a json-array of host nodes for its memory policy + +Arguments: + +Example: + +-> { "excute": "query-numa" } +<- { "return":[ +{ +"nodeid": 0, +"cpus": [0, 1], +"memory": 536870912, +"policy": "membind", +"relative": false, +"host-nodes": [0, 1] +}, +{ +"nodeid": 1, +"cpus": [2, 3], +"memory": 536870912, +"policy": "interleave", +"relative": false, +"host-nodes": [1] +} + ] + } + +EQMP --
[Qemu-devel] [PATCH V15 06/11] NUMA: add "-numa mem," options
Add "-numa mem," option like following as Paolo suggested: -numa mem,nodeid=0,size=1G This new option will make later coming memory hotplug better. We will use the new options to specify nodes memory info, and just remain "-numa node,mem=xx" as legacy. Reviewed-by: Laszlo Ersek Signed-off-by: Wanlong Gao --- include/sysemu/sysemu.h | 1 + numa.c | 36 qemu-options.hx | 6 -- vl.c| 2 ++ 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index c9fb2c7..861cd77 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -135,6 +135,7 @@ extern QEMUClockType rtc_clock; #define MAX_NODES 64 #define MAX_CPUMASK_BITS 255 extern int nb_numa_nodes; +extern int nb_numa_mem_nodes; typedef struct node_info { uint64_t node_mem; DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS); diff --git a/numa.c b/numa.c index c4fa665..c676c5e 100644 --- a/numa.c +++ b/numa.c @@ -74,6 +74,31 @@ static int numa_node_parse(NumaNodeOptions *opts) return 0; } +static int numa_mem_parse(NumaMemOptions *opts) +{ +uint16_t nodenr; +uint64_t mem_size; + +if (opts->has_nodeid) { +nodenr = opts->nodeid; +} else { +nodenr = nb_numa_mem_nodes; +} + +if (nodenr >= MAX_NODES) { +fprintf(stderr, "qemu: Max number of NUMA nodes reached: %" +PRIu16 "\n", nodenr); +return -1; +} + +if (opts->has_size) { +mem_size = opts->size; +numa_info[nodenr].node_mem = mem_size; +} + +return 0; +} + int numa_init_func(QemuOpts *opts, void *opaque) { NumaOptions *object = NULL; @@ -101,6 +126,13 @@ int numa_init_func(QemuOpts *opts, void *opaque) } nb_numa_nodes++; break; +case NUMA_OPTIONS_KIND_MEM: +ret = numa_mem_parse(object->mem); +if (ret) { +goto error; +} +nb_numa_mem_nodes++; +break; default: fprintf(stderr, "qemu: Invalid NUMA options type.\n"); ret = -1; @@ -119,6 +151,10 @@ error: void set_numa_nodes(void) { +if (nb_numa_mem_nodes > nb_numa_nodes) { +nb_numa_nodes = nb_numa_mem_nodes; +} + if (nb_numa_nodes > 0) { int i; diff --git a/qemu-options.hx b/qemu-options.hx index 5dc8b75..98fa25d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -95,11 +95,13 @@ specifies the maximum number of hotpluggable CPUs. ETEXI DEF("numa", HAS_ARG, QEMU_OPTION_numa, -"-numa node[,mem=size][,cpus=cpu[-cpu]][,nodeid=node]\n", QEMU_ARCH_ALL) +"-numa node[,nodeid=node][,cpus=cpu[-cpu]]\n" +"-numa mem[,nodeid=node][,size=size]\n" +, QEMU_ARCH_ALL) STEXI @item -numa @var{opts} @findex -numa -Simulate a multi node NUMA system. If mem and cpus are omitted, resources +Simulate a multi node NUMA system. If @var{size} and @var{cpus} are omitted, resources are split equally. ETEXI diff --git a/vl.c b/vl.c index d55d717..190d5d7 100644 --- a/vl.c +++ b/vl.c @@ -250,6 +250,7 @@ static QTAILQ_HEAD(, FWBootEntry) fw_boot_order = QTAILQ_HEAD_INITIALIZER(fw_boot_order); int nb_numa_nodes; +int nb_numa_mem_nodes; NodeInfo numa_info[MAX_NODES]; uint8_t qemu_uuid[16]; @@ -2816,6 +2817,7 @@ int main(int argc, char **argv, char **envp) } nb_numa_nodes = 0; +nb_numa_mem_nodes = 0; nb_nics = 0; bdrv_init_with_whitelist(); -- 1.8.4.1.600.g3d092bf
[Qemu-devel] [PATCH V15 09/11] NUMA: set guest numa nodes memory policy
Set the guest numa nodes memory policies using the mbind(2) system call node by node. After this patch, we are able to set guest nodes memory policies through the QEMU options, this arms to solve the guest cross nodes memory access performance issue. And as you all know, if PCI-passthrough is used, direct-attached-device uses DMA transfer between device and qemu process. All pages of the guest will be pinned by get_user_pages(). KVM_ASSIGN_PCI_DEVICE ioctl kvm_vm_ioctl_assign_device() =>kvm_assign_device() => kvm_iommu_map_memslots() => kvm_iommu_map_pages() => kvm_pin_pages() So, with direct-attached-device, all guest page's page count will be +1 and any page migration will not work. AutoNUMA won't too. So, we should set the guest nodes memory allocation policies before the pages are really mapped. Signed-off-by: Andre Przywara Signed-off-by: Wanlong Gao --- numa.c | 86 ++ 1 file changed, 86 insertions(+) diff --git a/numa.c b/numa.c index da4dbbd..915a67a 100644 --- a/numa.c +++ b/numa.c @@ -27,6 +27,16 @@ #include "qapi-visit.h" #include "qapi/opts-visitor.h" #include "qapi/dealloc-visitor.h" +#include "exec/memory.h" + +#ifdef __linux__ +#include +#ifndef MPOL_F_RELATIVE_NODES +#define MPOL_F_RELATIVE_NODES (1 << 14) +#define MPOL_F_STATIC_NODES (1 << 15) +#endif +#endif + QemuOptsList qemu_numa_opts = { .name = "numa", .implied_opt_name = "type", @@ -228,6 +238,75 @@ void set_numa_nodes(void) } } +#ifdef __linux__ +static int node_parse_bind_mode(unsigned int nodeid) +{ +int bind_mode; + +switch (numa_info[nodeid].policy) { +case NUMA_NODE_POLICY_DEFAULT: +case NUMA_NODE_POLICY_PREFERRED: +case NUMA_NODE_POLICY_MEMBIND: +case NUMA_NODE_POLICY_INTERLEAVE: +bind_mode = numa_info[nodeid].policy; +break; +default: +bind_mode = NUMA_NODE_POLICY_DEFAULT; +return bind_mode; +} + +bind_mode |= numa_info[nodeid].relative ? +MPOL_F_RELATIVE_NODES : MPOL_F_STATIC_NODES; + +return bind_mode; +} +#endif + +static int set_node_mem_policy(int nodeid) +{ +#ifdef __linux__ +void *ram_ptr; +RAMBlock *block; +ram_addr_t len, ram_offset = 0; +int bind_mode; +int i; + +QTAILQ_FOREACH(block, &ram_list.blocks, next) { +if (!strcmp(block->mr->name, "pc.ram")) { +break; +} +} + +if (block->host == NULL) { +return -1; +} + +ram_ptr = block->host; +for (i = 0; i < nodeid; i++) { +len = numa_info[i].node_mem; +ram_offset += len; +} + +len = numa_info[nodeid].node_mem; +bind_mode = node_parse_bind_mode(nodeid); +unsigned long *nodes = numa_info[nodeid].host_mem; + +/* This is a workaround for a long standing bug in Linux' + * mbind implementation, which cuts off the last specified + * node. To stay compatible should this bug be fixed, we + * specify one more node and zero this one out. + */ +unsigned long maxnode = find_last_bit(nodes, MAX_NODES); +if (syscall(SYS_mbind, ram_ptr + ram_offset, len, bind_mode, +nodes, maxnode + 2, 0)) { +perror("mbind"); +return -1; +} +#endif + +return 0; +} + void set_numa_modes(void) { CPUState *cpu; @@ -240,4 +319,11 @@ void set_numa_modes(void) } } } + +for (i = 0; i < nb_numa_nodes; i++) { +if (set_node_mem_policy(i) == -1) { +fprintf(stderr, +"qemu: can not set host memory policy for node%d\n", i); +} +} } -- 1.8.4.1.600.g3d092bf
[Qemu-devel] [PATCH V15 07/11] NUMA: expand MAX_NODES from 64 to 128
libnuma choosed 128 for MAX_NODES, so we follow libnuma here. Signed-off-by: Wanlong Gao --- include/sysemu/sysemu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 861cd77..995cf3b 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -132,7 +132,7 @@ extern size_t boot_splash_filedata_size; extern uint8_t qemu_extra_params_fw[2]; extern QEMUClockType rtc_clock; -#define MAX_NODES 64 +#define MAX_NODES 128 #define MAX_CPUMASK_BITS 255 extern int nb_numa_nodes; extern int nb_numa_mem_nodes; -- 1.8.4.1.600.g3d092bf
[Qemu-devel] [PATCH V15 11/11] NUMA: convert hmp command info_numa to use qmp command query_numa
Reviewed-by: Luiz Capitulino Signed-off-by: Wanlong Gao --- hmp.c | 57 + hmp.h | 1 + monitor.c | 21 + 3 files changed, 59 insertions(+), 20 deletions(-) diff --git a/hmp.c b/hmp.c index 32ee285..d6dedd2 100644 --- a/hmp.c +++ b/hmp.c @@ -24,6 +24,10 @@ #include "ui/console.h" #include "block/qapi.h" #include "qemu-io.h" +#include "qapi-visit.h" +#include "qapi/opts-visitor.h" +#include "qapi/dealloc-visitor.h" +#include "sysemu/sysemu.h" static void hmp_handle_error(Monitor *mon, Error **errp) { @@ -1564,3 +1568,56 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, &err); } + +void hmp_info_numa(Monitor *mon, const QDict *qdict) +{ +NUMANodeList *node_list, *node; +uint16List *head; +int nodeid; +char *policy_str = NULL; + +node_list = qmp_query_numa(NULL); + +monitor_printf(mon, "%d nodes\n", nb_numa_nodes); +for (node = node_list; node; node = node->next) { +nodeid = node->value->nodeid; +monitor_printf(mon, "node %d cpus:", nodeid); +head = node->value->cpus; +for (head = node->value->cpus; head != NULL; head = head->next) { +monitor_printf(mon, " %d", (int)head->value); +} +monitor_printf(mon, "\n"); +monitor_printf(mon, "node %d size: %" PRId64 " MB\n", + nodeid, node->value->memory >> 20); +switch (node->value->policy) { +case NUMA_NODE_POLICY_DEFAULT: +policy_str = g_strdup("default"); +break; +case NUMA_NODE_POLICY_PREFERRED: +policy_str = g_strdup("preferred"); +break; +case NUMA_NODE_POLICY_MEMBIND: +policy_str = g_strdup("membind"); +break; +case NUMA_NODE_POLICY_INTERLEAVE: +policy_str = g_strdup("interleave"); +break; +default: +break; +} +monitor_printf(mon, "node %d policy: %s\n", + nodeid, policy_str ? : " "); +if (policy_str) { +free(policy_str); +} +monitor_printf(mon, "node %d relative: %s\n", nodeid, + node->value->relative ? "true" : "false"); +monitor_printf(mon, "node %d host-nodes:", nodeid); +for (head = node->value->host_nodes; head != NULL; head = head->next) { +monitor_printf(mon, " %d", (int)head->value); +} +monitor_printf(mon, "\n"); +} + +qapi_free_NUMANodeList(node_list); +} diff --git a/hmp.h b/hmp.h index 54cf71f..4f8d39b 100644 --- a/hmp.h +++ b/hmp.h @@ -37,6 +37,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict); void hmp_info_pci(Monitor *mon, const QDict *qdict); void hmp_info_block_jobs(Monitor *mon, const QDict *qdict); void hmp_info_tpm(Monitor *mon, const QDict *qdict); +void hmp_info_numa(Monitor *mon, const QDict *qdict); void hmp_quit(Monitor *mon, const QDict *qdict); void hmp_stop(Monitor *mon, const QDict *qdict); void hmp_system_reset(Monitor *mon, const QDict *qdict); diff --git a/monitor.c b/monitor.c index be34488..ce9dfe7 100644 --- a/monitor.c +++ b/monitor.c @@ -1995,25 +1995,6 @@ static void do_info_mtree(Monitor *mon, const QDict *qdict) mtree_info((fprintf_function)monitor_printf, mon); } -static void do_info_numa(Monitor *mon, const QDict *qdict) -{ -int i; -CPUState *cpu; - -monitor_printf(mon, "%d nodes\n", nb_numa_nodes); -for (i = 0; i < nb_numa_nodes; i++) { -monitor_printf(mon, "node %d cpus:", i); -CPU_FOREACH(cpu) { -if (cpu->numa_node == i) { -monitor_printf(mon, " %d", cpu->cpu_index); -} -} -monitor_printf(mon, "\n"); -monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i, -numa_info[i].node_mem >> 20); -} -} - #ifdef CONFIG_PROFILER int64_t qemu_time; @@ -2781,7 +2762,7 @@ static mon_cmd_t info_cmds[] = { .args_type = "", .params = "", .help = "show NUMA information", -.mhandler.cmd = do_info_numa, +.mhandler.cmd = hmp_info_numa, }, { .name = "usb", -- 1.8.4.1.600.g3d092bf
[Qemu-devel] [PATCH V15 05/11] NUMA: introduce NumaMemOptions
Signed-off-by: Wanlong Gao --- qapi-schema.json | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/qapi-schema.json b/qapi-schema.json index b7f0b15..a19e453 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4198,7 +4198,8 @@ ## { 'union': 'NumaOptions', 'data': { -'node': 'NumaNodeOptions' }} +'node': 'NumaNodeOptions', +'mem' : 'NumaMemOptions' }} ## # @NumaNodeOptions @@ -4218,3 +4219,19 @@ '*nodeid': 'uint16', '*cpus': ['uint16'], '*mem':'str' }} + +## +# @NumaMemOptions +# +# Set memory information of guest NUMA node. (for OptsVisitor) +# +# @nodeid: #optional NUMA node ID +# +# @size: #optional memory size of this node +# +# Since 1.7 +## +{ 'type': 'NumaMemOptions', + 'data': { + '*nodeid': 'uint16', + '*size': 'size' }} -- 1.8.4.1.600.g3d092bf
[Qemu-devel] [Bug 1245724] [NEW] libfdt.a git compilation fail
Public bug reported: I don't know the commit tags but I checked out dtc on the 28 of october at 20:27 in the tree of qemu (also git checkout out tonight). The compilation fail at line 234 in qemu/dtc/Makefile so I inserted that line: @$ /usr/bin/strace -o /usr/src/qemu_build/error.log.txt /usr/bin/ar $@ into the makefile at position 234 to see what is the exact problem but the strace log is inconclusive. for the error: /usr/bin/ar: deux operations différentes spécifiées liberal translation is: two different operation specified. the distribution is arch linux with binutils 2.23.2, gcc 4.8.2 and kernel kvm-3.12.0-rc5 from git. ** Affects: qemu Importance: Undecided Status: New ** Attachment added: "strace log" https://bugs.launchpad.net/bugs/1245724/+attachment/3894285/+files/error.log.txt -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1245724 Title: libfdt.a git compilation fail Status in QEMU: New Bug description: I don't know the commit tags but I checked out dtc on the 28 of october at 20:27 in the tree of qemu (also git checkout out tonight). The compilation fail at line 234 in qemu/dtc/Makefile so I inserted that line: @$ /usr/bin/strace -o /usr/src/qemu_build/error.log.txt /usr/bin/ar $@ into the makefile at position 234 to see what is the exact problem but the strace log is inconclusive. for the error: /usr/bin/ar: deux operations différentes spécifiées liberal translation is: two different operation specified. the distribution is arch linux with binutils 2.23.2, gcc 4.8.2 and kernel kvm-3.12.0-rc5 from git. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1245724/+subscriptions
Re: [Qemu-devel] [PATCH] vl: fix build when configured with no graphic support
Am 29.10.2013 05:36, schrieb Antony Pavlov: > The following error occurs when building no graphic output support: > > vl.c: In function ‘main’: > vl.c:2829:19: error: variable ‘ds’ set but not used > [-Werror=unused-but-set-variable] >DisplayState *ds; > ^ > cc1: all warnings being treated as errors > > To reproduce this issue, just run: > > $ ./configure \ > --disable-curses \ > --disable-sdl \ > --disable-cocoa \ > --disable-gtk \ > --disable-vnc \ > --enable-werror > $ make vl.o > > Signed-off-by: Antony Pavlov > --- > vl.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/vl.c b/vl.c > index b42ac67..8fbf25e 100644 > --- a/vl.c > +++ b/vl.c > @@ -4269,6 +4269,7 @@ int main(int argc, char **argv, char **envp) > /* init local displays */ > switch (display_type) { > case DT_NOGRAPHIC: > +(void)ds; > break; > #if defined(CONFIG_CURSES) > case DT_CURSES: Reviewed-by: Stefan Weil
Re: [Qemu-devel] [PATCH 3/6] qapi: rename prefix QEVENT to Q_EVENT
于 2013/10/28 18:44, Paolo Bonzini 写道: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Il 21/10/2013 22:41, Eric Blake ha scritto: On 10/21/2013 03:16 AM, Wenchao Xia wrote: The define will be moved to qapi-schema.json later, so rename the prefix to match its naming style. Wouldn't it be simpler to fix the code generator to special case QEvent to turn into QEVENT, instead of having to go through this churn? But if we _like_ the Q_EVENT_ prefix, then this looks fairly mechanical: No, I disagree. However, since something _is_ being renamed, we might as well rename every QEVENT_ to MONITOR_EVENT_ and avoid patch 2. Paolo MONITOR_EVENT seems tide to monitor too much, since it will be present in qapi-schema, I think Q_EVENT_ or QMP_EVENT_KIND would be better? I am coding v2, which fully support event define in qapi-schema. There is another thing I hope to know your opinion: Should we support use enum as discriminator? { 'enum': 'QEvent', 'data': [ 'SHUTDOWN', 'POWERDOWN'] { 'type': 'QmpEventBase', 'data': { 'event': 'QEvent', 'timestamp': 'EventTimestamp' } } { 'Union': 'QmpEvent', 'base': 'QmpEventBase', 'discriminator': 'event', 'data': { 'SHUTDOWN' : 'EventShutdown', 'POWERDOWN' : 'EventPowerdown' } } By default 'QmpEvent' will generate a hidden enum type 'QmpEventKind'. but the hidden type define is needed by query-event, so there is two way to archieve it: 1 just use the hidden type in qapi-schema.json. 2 modified the script to support use predefined enum type. In my draft code, both can work, but which one do you prefer?