Re: [Qemu-devel] [Qemu-stable] Patch Round-up for stable 2.1.1, freeze on 2014-09-03
On Wed, Sep 03, 2014 at 02:17:02AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 2:09 AM, Andrey Korolyov and...@xdel.ru wrote: On Wed, Sep 3, 2014 at 1:51 AM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 01:29:29AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 1:03 AM, Michael S. Tsirkin m...@redhat.com wrote: bad one is the Author: Jason Wang jasow...@redhat.com Date: Tue Sep 2 18:07:46 2014 +0300 vhost_net: start/stop guest notifiers properly upstream has this (pull request sent today): vhost_net: cleanup start/stop condition Could you apply it and see if it helps please? Michael, if it helps it should be before start/stop guest notifiers ideally to avoid bisect problems. It is already applied as shown from the list in the previous message (there are some aio fixes too on top of 2.1 I picked before but they should not impact vhost-net interaction in any mean). The symptoms are a bit interesting - VM crashes only at PCI device initalization (e.g. grub stage after reset and initrd unpacking are passing well, but then things getting ugly). I am running 3.14 guest i686-pae kernel from debian backports in guest, so it may be version-specific after all. If it`ll be hard to reproduce, I can try 64bit, expecting same behavior. Please find args in attached file. ok just to make sure - which tree do I clone exactly? https://github.com/mdroth/qemu.git stable-2.1-staging showing same behavior for me with those patches Forgot to mention important detail - I am playing with -mq now, so actually virtio-net working in a bit different way than it may expected (it also shown in args list from above, but someone may miss it): ... qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio ... OK I see at least one obvious bug there: does the following fix the crash for you? Separately, we need to debug why mq vhost is broken for you. Is this a regression? diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..1fe18c7 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +err: +r = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (r 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, r); +fflush(stderr); +} return r; }
[Qemu-devel] [PATCH V2] fix: unix sockets created for virtio-serail has insufficient permissions
Add umask to _virCommand, allow user to set umask to command. Set umask(002) to qemu process to overwrite default umask(022) so that unix sockets created for virtio-serial has expected permissions. Fix problem reported here: https://sourceware.org/bugzilla/show_bug.cgi?id=13078#c11 https://bugzilla.novell.com/show_bug.cgi?id=888166 To use virtio-serial device, unix socket created for chardev with default umask(022) has insufficient permissions. e.g.: -device virtio-serial \ -chardev socket,path=/tmp/foo,server,nowait,id=foo \ -device virtserialport,chardev=foo,name=org.fedoraproject.port.0 srwxr-xr-x 1 qemu qemu 0 21. Jul 14:19 /tmp/somefile.sock Other users in the same group (like real user, test engines, etc) cannot write to this socket. Signed-off-by: Chunyan Liu cy...@suse.com --- Changes: * set umask(002) to the whole qemu process instead of calling umask in qemu unix_listen_opts. V1 is here: http://www.mail-archive.com/libvir-list@redhat.com/msg101513.html src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 1 + src/util/vircommand.c| 11 +++ src/util/vircommand.h| 1 + 4 files changed, 14 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 71fc063..f136d24 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1171,6 +1171,7 @@ virCommandSetPidFile; virCommandSetPreExecHook; virCommandSetSELinuxLabel; virCommandSetUID; +virCommandSetUmask; virCommandSetWorkingDirectory; virCommandToString; virCommandWait; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f68dfbe..f76aa5a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4141,6 +4141,7 @@ int qemuProcessStart(virConnectPtr conn, virCommandSetPreExecHook(cmd, qemuProcessHook, hookData); virCommandSetMaxProcesses(cmd, cfg-maxProcesses); virCommandSetMaxFiles(cmd, cfg-maxFiles); +virCommandSetUmask(cmd, 0x002); VIR_DEBUG(Setting up security labelling); if (virSecurityManagerSetChildProcessLabel(driver-securityManager, diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 1d6dbd9..efb5f69 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -133,6 +133,7 @@ struct _virCommand { #if defined(WITH_SECDRIVER_APPARMOR) char *appArmorProfile; #endif +int umask; }; /* See virCommandSetDryRun for description for this variable */ @@ -582,6 +583,8 @@ virExec(virCommandPtr cmd) /* child */ +if (cmd-umask) +umask(cmd-umask); ret = EXIT_CANCELED; openmax = sysconf(_SC_OPEN_MAX); if (openmax 0) { @@ -1082,6 +1085,14 @@ virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files) cmd-maxFiles = files; } +void virCommandSetUmask(virCommandPtr cmd, int umask) +{ +if (!cmd || cmd-has_error) +return; + +cmd-umask = umask; +} + /** * virCommandClearCaps: * @cmd: the command to modify diff --git a/src/util/vircommand.h b/src/util/vircommand.h index d3b286d..bf65de4 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -72,6 +72,7 @@ void virCommandSetUID(virCommandPtr cmd, uid_t uid); void virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes); void virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs); void virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files); +void virCommandSetUmask(virCommandPtr cmd, int umask); void virCommandClearCaps(virCommandPtr cmd); -- 1.8.4.5
Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function
Hi, 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's pointer to del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a function named is_same_fw_dev_path() to handle this situation. When hot-unplugging virtio-net-pci I'd expect we free both virtio-net-pci and virtio-net-device (and therefore call del_boot_device_path twice, once for each device). Can you check that? thanks, Gerd
[Qemu-devel] [PATCH] vhost_net: initialize acked_features to a safe value during ack
commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. This will result an unexpected value of acked_features which may fail the features setting of vhost. This patch fixes it by recover this step. Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: Andrey Korolyov and...@xdel.ru Cc: Michael S. Tsirkin m...@redhat.com Cc: Michael Roth mdr...@linux.vnet.ibm.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com --- hw/net/vhost_net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index f87c798..b1d4b1f 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -115,6 +115,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) void vhost_net_ack_features(struct vhost_net *net, unsigned features) { +net-dev.acked_features = net-dev.backend_features; vhost_ack_features(net-dev, vhost_net_get_feature_bits(net), features); } -- 1.8.3.1
[Qemu-devel] [PULL 07/10] query-memdev: fix potential memory leaks
From: Chen Fan chen.fan.f...@cn.fujitsu.com Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Hu Tao hu...@cn.fujitsu.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- numa.c |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/numa.c b/numa.c index c78cec9..aa772aa 100644 --- a/numa.c +++ b/numa.c @@ -318,10 +318,11 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, static int query_memdev(Object *obj, void *opaque) { MemdevList **list = opaque; +MemdevList *m = NULL; Error *err = NULL; if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) { -MemdevList *m = g_malloc0(sizeof(*m)); +m = g_malloc0(sizeof(*m)); m-value = g_malloc0(sizeof(*m-value)); @@ -369,6 +370,9 @@ static int query_memdev(Object *obj, void *opaque) return 0; error: +g_free(m-value); +g_free(m); + return -1; } -- 1.7.10.4
[Qemu-devel] [PULL 00/10] Trivial patches for 2014-09-03
Here's a next trivial-patches batch. A few little things here and there. Please consider applying. Thanks, /mjt The following changes since commit 30eaca3acdf17d7bcbd1213eb149c02037edfb0b: Merge remote-tracking branch 'remotes/spice/tags/pull-spice-20140902-1' into staging (2014-09-02 10:26:10 +0100) are available in the git repository at: git://git.corpit.ru/qemu.git tags/trivial-patches-2014-09-03 for you to fetch changes up to 70381662aa97b294f3c81a085ed143f37f0ab0cb: slirp: Honour vlan/stack in hostfwd_remove commands (2014-09-02 22:38:16 +0400) trivial patches for 2014-09-03 Chen Fan (3): query-memdev: fix potential memory leaks qom/object.c, hmp.c: fix string_output_get_string() memory leak hmp: fix MemdevList memory leak Dmitry Fleytman (1): MAINTAINERS: Add VMWare devices maintainer Fam Zheng (1): scripts: Remove scripts/qtest Gonglei (1): Fix debug print warning Li Liu (2): device_tree.c: redirect load_device_tree err message to stderr device_tree.c: dump all err mesages with error_report Peter Maydell (1): slirp: Honour vlan/stack in hostfwd_remove commands Richard W.M. Jones (1): curl: The macro that you have to uncomment to get debugging is DEBUG_CURL. MAINTAINERS|6 ++ block/curl.c |2 +- device_tree.c | 55 hmp.c |8 +-- hw/i2c/pm_smbus.c |5 +++-- hw/i386/pc.c |2 +- hw/input/pckbd.c |4 ++-- hw/intc/i8259.c|2 +- hw/isa/apm.c |5 +++-- hw/timer/mc146818rtc.c |2 +- net/slirp.c|3 +-- numa.c | 15 ++--- qom/object.c | 12 +-- scripts/qtest |5 - 14 files changed, 70 insertions(+), 56 deletions(-) delete mode 100755 scripts/qtest
[Qemu-devel] [PULL 10/10] slirp: Honour vlan/stack in hostfwd_remove commands
From: Peter Maydell peter.mayd...@linaro.org The hostfwd_add and hostfwd_remove monitor commands allow the user to optionally specify a vlan/stack tuple. hostfwd_add honours this, but hostfwd_remove does not (it looks up the tuple but then ignores the SlirpState it has looked up and always uses the first stack in the list anyway). Correct this to honour what the user requested. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Michael Tokarev m...@tls.msk.ru --- net/slirp.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/slirp.c b/net/slirp.c index 647039e..c171119 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -345,8 +345,7 @@ void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict) host_port = atoi(p); -err = slirp_remove_hostfwd(QTAILQ_FIRST(slirp_stacks)-slirp, is_udp, - host_addr, host_port); +err = slirp_remove_hostfwd(s-slirp, is_udp, host_addr, host_port); monitor_printf(mon, host forwarding rule for %s %s\n, src_str, err ? not found : removed); -- 1.7.10.4
[Qemu-devel] [PULL 02/10] Fix debug print warning
From: Gonglei arei.gong...@huawei.com Steps: 1.enable qemu debug print, using simply scprit as below: grep //#define DEBUG * -rl | xargs sed -i s/\/\/#define DEBUG/#define DEBUG/g 2. make -j 3. get some warning: hw/i2c/pm_smbus.c: In function 'smb_ioport_writeb': hw/i2c/pm_smbus.c:142: warning: format '%04x' expects type 'unsigned int', but argument 2 has type 'hwaddr' hw/i2c/pm_smbus.c:142: warning: format '%02x' expects type 'unsigned int', but argument 3 has type 'uint64_t' hw/i2c/pm_smbus.c: In function 'smb_ioport_readb': hw/i2c/pm_smbus.c:209: warning: format '%04x' expects type 'unsigned int', but argument 2 has type 'hwaddr' hw/intc/i8259.c: In function 'pic_ioport_read': hw/intc/i8259.c:373: warning: format '%02x' expects type 'unsigned int', but argument 2 has type 'hwaddr' hw/input/pckbd.c: In function 'kbd_write_command': hw/input/pckbd.c:232: warning: format '%02x' expects type 'unsigned int', but argument 2 has type 'uint64_t' hw/input/pckbd.c: In function 'kbd_write_data': hw/input/pckbd.c:333: warning: format '%02x' expects type 'unsigned int', but argument 2 has type 'uint64_t' hw/isa/apm.c: In function 'apm_ioport_writeb': hw/isa/apm.c:44: warning: format '%x' expects type 'unsigned int', but argument 2 has type 'hwaddr' hw/isa/apm.c:44: warning: format '%02x' expects type 'unsigned int', but argument 3 has type 'uint64_t' hw/isa/apm.c: In function 'apm_ioport_readb': hw/isa/apm.c:67: warning: format '%x' expects type 'unsigned int', but argument 2 has type 'hwaddr' hw/timer/mc146818rtc.c: In function 'cmos_ioport_write': hw/timer/mc146818rtc.c:394: warning: format '%02x' expects type 'unsigned int', but argument 3 has type 'uint64_t' hw/i386/pc.c: In function 'port92_write': hw/i386/pc.c:479: warning: format '%02x' expects type 'unsigned int', but argument 2 has type 'uint64_t' Fix them. Cc: qemu-triv...@nongnu.org Signed-off-by: Gonglei arei.gong...@huawei.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- hw/i2c/pm_smbus.c |5 +++-- hw/i386/pc.c |2 +- hw/input/pckbd.c |4 ++-- hw/intc/i8259.c|2 +- hw/isa/apm.c |5 +++-- hw/timer/mc146818rtc.c |2 +- 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c index fedb5fb..ce1713d 100644 --- a/hw/i2c/pm_smbus.c +++ b/hw/i2c/pm_smbus.c @@ -139,7 +139,8 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, { PMSMBus *s = opaque; -SMBUS_DPRINTF(SMB writeb port=0x%04x val=0x%02x\n, addr, val); +SMBUS_DPRINTF(SMB writeb port=0x%04 HWADDR_PRIx + val=0x%02 PRIx64 \n, addr, val); switch(addr) { case SMBHSTSTS: s-smb_stat = (~(val 0xff)) s-smb_stat; @@ -206,7 +207,7 @@ static uint64_t smb_ioport_readb(void *opaque, hwaddr addr, unsigned width) val = 0; break; } -SMBUS_DPRINTF(SMB readb port=0x%04x val=0x%02x\n, addr, val); +SMBUS_DPRINTF(SMB readb port=0x%04 HWADDR_PRIx val=0x%02x\n, addr, val); return val; } diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 0ca4deb..b6c9b61 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -481,7 +481,7 @@ static void port92_write(void *opaque, hwaddr addr, uint64_t val, Port92State *s = opaque; int oldval = s-outport; -DPRINTF(port92: write 0x%02x\n, val); +DPRINTF(port92: write 0x%02 PRIx64 \n, val); s-outport = val; qemu_set_irq(*s-a20_out, (val 1) 1); if ((val 1) !(oldval 1)) { diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c index ca1cffc..2ab8c87 100644 --- a/hw/input/pckbd.c +++ b/hw/input/pckbd.c @@ -229,7 +229,7 @@ static void kbd_write_command(void *opaque, hwaddr addr, { KBDState *s = opaque; -DPRINTF(kbd: write cmd=0x%02x\n, val); +DPRINTF(kbd: write cmd=0x%02 PRIx64 \n, val); /* Bits 3-0 of the output port P2 of the keyboard controller may be pulsed * low for approximately 6 micro seconds. Bits 3-0 of the KBD_CCMD_PULSE @@ -330,7 +330,7 @@ static void kbd_write_data(void *opaque, hwaddr addr, { KBDState *s = opaque; -DPRINTF(kbd: write data=0x%02x\n, val); +DPRINTF(kbd: write data=0x%02 PRIx64 \n, val); switch(s-write_cmd) { case 0: diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c index a563b82..c51901b 100644 --- a/hw/intc/i8259.c +++ b/hw/intc/i8259.c @@ -370,7 +370,7 @@ static uint64_t pic_ioport_read(void *opaque, hwaddr addr, ret = s-imr; } } -DPRINTF(read: addr=0x%02x val=0x%02x\n, addr, ret); +DPRINTF(read: addr=0x%02 HWADDR_PRIx val=0x%02x\n, addr, ret); return ret; } diff --git a/hw/isa/apm.c b/hw/isa/apm.c index 054d529..26ab170 100644 --- a/hw/isa/apm.c +++ b/hw/isa/apm.c @@ -41,7 +41,8 @@ static void apm_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, { APMState *apm = opaque; addr = 1; -APM_DPRINTF(apm_ioport_writeb addr=0x%x val=0x%02x\n, addr, val); +APM_DPRINTF(apm_ioport_writeb
[Qemu-devel] [PULL 05/10] device_tree.c: dump all err mesages with error_report
From: Li Liu john.li...@huawei.com Signed-off-by: Li Liu john.li...@huawei.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- device_tree.c | 40 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/device_tree.c b/device_tree.c index 9d47195..df9eed9 100644 --- a/device_tree.c +++ b/device_tree.c @@ -60,13 +60,13 @@ void *create_device_tree(int *sizep) } ret = fdt_open_into(fdt, fdt, *sizep); if (ret) { -fprintf(stderr, Unable to copy device tree in memory\n); +error_report(Unable to copy device tree in memory); exit(1); } return fdt; fail: -fprintf(stderr, %s Couldn't create dt: %s\n, __func__, fdt_strerror(ret)); +error_report(%s Couldn't create dt: %s, __func__, fdt_strerror(ret)); exit(1); } @@ -124,8 +124,8 @@ static int findnode_nofail(void *fdt, const char *node_path) offset = fdt_path_offset(fdt, node_path); if (offset 0) { -fprintf(stderr, %s Couldn't find node %s: %s\n, __func__, node_path, -fdt_strerror(offset)); +error_report(%s Couldn't find node %s: %s, __func__, node_path, + fdt_strerror(offset)); exit(1); } @@ -139,8 +139,8 @@ int qemu_fdt_setprop(void *fdt, const char *node_path, r = fdt_setprop(fdt, findnode_nofail(fdt, node_path), property, val, size); if (r 0) { -fprintf(stderr, %s: Couldn't set %s/%s: %s\n, __func__, node_path, -property, fdt_strerror(r)); +error_report(%s: Couldn't set %s/%s: %s, __func__, node_path, + property, fdt_strerror(r)); exit(1); } @@ -154,8 +154,8 @@ int qemu_fdt_setprop_cell(void *fdt, const char *node_path, r = fdt_setprop_cell(fdt, findnode_nofail(fdt, node_path), property, val); if (r 0) { -fprintf(stderr, %s: Couldn't set %s/%s = %#08x: %s\n, __func__, -node_path, property, val, fdt_strerror(r)); +error_report(%s: Couldn't set %s/%s = %#08x: %s, __func__, + node_path, property, val, fdt_strerror(r)); exit(1); } @@ -176,8 +176,8 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path, r = fdt_setprop_string(fdt, findnode_nofail(fdt, node_path), property, string); if (r 0) { -fprintf(stderr, %s: Couldn't set %s/%s = %s: %s\n, __func__, -node_path, property, string, fdt_strerror(r)); +error_report(%s: Couldn't set %s/%s = %s: %s, __func__, + node_path, property, string, fdt_strerror(r)); exit(1); } @@ -194,8 +194,8 @@ const void *qemu_fdt_getprop(void *fdt, const char *node_path, } r = fdt_getprop(fdt, findnode_nofail(fdt, node_path), property, lenp); if (!r) { -fprintf(stderr, %s: Couldn't get %s/%s: %s\n, __func__, -node_path, property, fdt_strerror(*lenp)); +error_report(%s: Couldn't get %s/%s: %s, __func__, + node_path, property, fdt_strerror(*lenp)); exit(1); } return r; @@ -207,8 +207,8 @@ uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path, int len; const uint32_t *p = qemu_fdt_getprop(fdt, node_path, property, len); if (len != 4) { -fprintf(stderr, %s: %s/%s not 4 bytes long (not a cell?)\n, -__func__, node_path, property); +error_report(%s: %s/%s not 4 bytes long (not a cell?), + __func__, node_path, property); exit(1); } return be32_to_cpu(*p); @@ -220,8 +220,8 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char *path) r = fdt_get_phandle(fdt, findnode_nofail(fdt, path)); if (r == 0) { -fprintf(stderr, %s: Couldn't get phandle for %s: %s\n, __func__, -path, fdt_strerror(r)); +error_report(%s: Couldn't get phandle for %s: %s, __func__, + path, fdt_strerror(r)); exit(1); } @@ -266,8 +266,8 @@ int qemu_fdt_nop_node(void *fdt, const char *node_path) r = fdt_nop_node(fdt, findnode_nofail(fdt, node_path)); if (r 0) { -fprintf(stderr, %s: Couldn't nop node %s: %s\n, __func__, node_path, -fdt_strerror(r)); +error_report(%s: Couldn't nop node %s: %s, __func__, node_path, + fdt_strerror(r)); exit(1); } @@ -295,8 +295,8 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) retval = fdt_add_subnode(fdt, parent, basename); if (retval 0) { -fprintf(stderr, FDT: Failed to create subnode %s: %s\n, name, -fdt_strerror(retval)); +error_report(FDT: Failed to create subnode %s: %s, name, + fdt_strerror(retval)); exit(1); } -- 1.7.10.4
[Qemu-devel] [PULL 03/10] scripts: Remove scripts/qtest
From: Fam Zheng f...@redhat.com This is a dummy file with no user, drop it. Signed-off-by: Fam Zheng f...@redhat.com Reviewed-by: Stefan Hajnoczi stefa...@redhat.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- scripts/qtest |5 - 1 file changed, 5 deletions(-) delete mode 100755 scripts/qtest diff --git a/scripts/qtest b/scripts/qtest deleted file mode 100755 index 4ef6c1c..000 --- a/scripts/qtest +++ /dev/null @@ -1,5 +0,0 @@ -#!/bin/sh - -export QTEST_QEMU_BINARY=$1 -shift -$@ -- 1.7.10.4
[Qemu-devel] [PULL 09/10] hmp: fix MemdevList memory leak
From: Chen Fan chen.fan.f...@cn.fujitsu.com the memdev_list in hmp_info_memdev() is never freed. so we use existent method qapi_free_MemdevList() to free it. and also we can use qapi_free_MemdevList() to replace list loops to clean up the memdev list in error path. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Hu Tao hu...@cn.fujitsu.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- hmp.c |2 ++ numa.c |9 ++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/hmp.c b/hmp.c index ba40c75..40a90da 100644 --- a/hmp.c +++ b/hmp.c @@ -1715,4 +1715,6 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) } monitor_printf(mon, \n); + +qapi_free_MemdevList(memdev_list); } diff --git a/numa.c b/numa.c index aa772aa..f07149b 100644 --- a/numa.c +++ b/numa.c @@ -379,7 +379,7 @@ error: MemdevList *qmp_query_memdev(Error **errp) { Object *obj; -MemdevList *list = NULL, *m; +MemdevList *list = NULL; obj = object_resolve_path(/objects, NULL); if (obj == NULL) { @@ -393,11 +393,6 @@ MemdevList *qmp_query_memdev(Error **errp) return list; error: -while (list) { -m = list; -list = list-next; -g_free(m-value); -g_free(m); -} +qapi_free_MemdevList(list); return NULL; } -- 1.7.10.4
[Qemu-devel] [PULL 04/10] device_tree.c: redirect load_device_tree err message to stderr
From: Li Liu john.li...@huawei.com Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Signed-off-by: Li Liu john.li...@huawei.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- device_tree.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/device_tree.c b/device_tree.c index ca83504..9d47195 100644 --- a/device_tree.c +++ b/device_tree.c @@ -20,6 +20,7 @@ #include config.h #include qemu-common.h +#include qemu/error-report.h #include sysemu/device_tree.h #include sysemu/sysemu.h #include hw/loader.h @@ -79,8 +80,8 @@ void *load_device_tree(const char *filename_path, int *sizep) *sizep = 0; dt_size = get_image_size(filename_path); if (dt_size 0) { -printf(Unable to get size of device tree file '%s'\n, -filename_path); +error_report(Unable to get size of device tree file '%s', + filename_path); goto fail; } @@ -92,21 +93,21 @@ void *load_device_tree(const char *filename_path, int *sizep) dt_file_load_size = load_image(filename_path, fdt); if (dt_file_load_size 0) { -printf(Unable to open device tree file '%s'\n, - filename_path); +error_report(Unable to open device tree file '%s', + filename_path); goto fail; } ret = fdt_open_into(fdt, fdt, dt_size); if (ret) { -printf(Unable to copy device tree in memory\n); +error_report(Unable to copy device tree in memory); goto fail; } /* Check sanity of device tree */ if (fdt_check_header(fdt)) { -printf (Device tree file loaded into memory is invalid: %s\n, -filename_path); +error_report(Device tree file loaded into memory is invalid: %s, + filename_path); goto fail; } *sizep = dt_size; -- 1.7.10.4
[Qemu-devel] [PULL 01/10] curl: The macro that you have to uncomment to get debugging is DEBUG_CURL.
From: Richard W.M. Jones rjo...@redhat.com Signed-off-by: Richard W.M. Jones rjo...@redhat.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- block/curl.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/curl.c b/block/curl.c index 0258339..938f9d9 100644 --- a/block/curl.c +++ b/block/curl.c @@ -26,7 +26,7 @@ #include qapi/qmp/qbool.h #include curl/curl.h -// #define DEBUG +// #define DEBUG_CURL // #define DEBUG_VERBOSE #ifdef DEBUG_CURL -- 1.7.10.4
[Qemu-devel] [PULL 08/10] qom/object.c, hmp.c: fix string_output_get_string() memory leak
From: Chen Fan chen.fan.f...@cn.fujitsu.com string_output_get_string() uses g_string_free(str, false) to transfer the 'str' pointer to callers and never free it. Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Hu Tao hu...@cn.fujitsu.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- hmp.c|6 -- qom/object.c | 12 ++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/hmp.c b/hmp.c index 4d1838e..ba40c75 100644 --- a/hmp.c +++ b/hmp.c @@ -1687,6 +1687,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) MemdevList *memdev_list = qmp_query_memdev(err); MemdevList *m = memdev_list; StringOutputVisitor *ov; +char *str; int i = 0; @@ -1704,9 +1705,10 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) m-value-prealloc ? true : false); monitor_printf(mon, policy: %s\n, HostMemPolicy_lookup[m-value-policy]); -monitor_printf(mon, host nodes: %s\n, - string_output_get_string(ov)); +str = string_output_get_string(ov); +monitor_printf(mon, host nodes: %s\n, str); +g_free(str); string_output_visitor_cleanup(ov); m = m-next; i++; diff --git a/qom/object.c b/qom/object.c index 1b00831..79bd0cc 100644 --- a/qom/object.c +++ b/qom/object.c @@ -938,14 +938,18 @@ int object_property_get_enum(Object *obj, const char *name, { StringOutputVisitor *sov; StringInputVisitor *siv; +char *str; int ret; sov = string_output_visitor_new(false); object_property_get(obj, string_output_get_visitor(sov), name, errp); -siv = string_input_visitor_new(string_output_get_string(sov)); +str = string_output_get_string(sov); +siv = string_input_visitor_new(str); string_output_visitor_cleanup(sov); visit_type_enum(string_input_get_visitor(siv), ret, strings, NULL, name, errp); + +g_free(str); string_input_visitor_cleanup(siv); return ret; @@ -956,13 +960,17 @@ void object_property_get_uint16List(Object *obj, const char *name, { StringOutputVisitor *ov; StringInputVisitor *iv; +char *str; ov = string_output_visitor_new(false); object_property_get(obj, string_output_get_visitor(ov), name, errp); -iv = string_input_visitor_new(string_output_get_string(ov)); +str = string_output_get_string(ov); +iv = string_input_visitor_new(str); visit_type_uint16List(string_input_get_visitor(iv), list, NULL, errp); + +g_free(str); string_output_visitor_cleanup(ov); string_input_visitor_cleanup(iv); } -- 1.7.10.4
[Qemu-devel] [PULL 06/10] MAINTAINERS: Add VMWare devices maintainer
From: Dmitry Fleytman dmi...@daynix.com Signed-off-by: Dmitry Fleytman dmi...@daynix.com Reviewed-by: Stefan Hajnoczi stefa...@redhat.com Signed-off-by: Michael Tokarev m...@tls.msk.ru --- MAINTAINERS |6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 142f68a..8622e01 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -684,6 +684,12 @@ S: Maintained F: hw/*/xilinx_* F: include/hw/xilinx.h +Vmware +M: Dmitry Fleytman dmi...@daynix.com +S: Maintained +F: hw/net/vmxnet* +F: hw/scsi/vmw_pvscsi* + Subsystems -- Audio -- 1.7.10.4
Re: [Qemu-devel] [PATCH v6 02/27] bootindex: add del_boot_device_path function
Best regards, -Gonglei -Original Message- From: Gerd Hoffmann [mailto:kra...@redhat.com] Sent: Wednesday, September 03, 2014 2:25 PM To: Gonglei (Arei) Cc: Eduardo Habkost; qemu-devel@nongnu.org; aligu...@amazon.com; m...@redhat.com; pbonz...@redhat.com; ak...@redhat.com; hu...@cn.fujitsu.com; ebl...@redhat.com; afaer...@suse.de; arm...@redhat.com; imamm...@redhat.com; a...@ozlabs.ru; peter.crosthwa...@xilinx.com; lcapitul...@redhat.com; h...@linux.com; stefa...@redhat.com; ag...@suse.de; chenliang (T); Huangweidong (C); Luonengjun; Huangpeng (Peter); kw...@redhat.com Subject: Re: [PATCH v6 02/27] bootindex: add del_boot_device_path function Importance: High Hi, 4. When we hotplug the virtio-net-pci device, only pass virtio-net-pci's pointer to del_boot_device_path(). But virtio-net-pci != virtio-net-device, so I add a function named is_same_fw_dev_path() to handle this situation. When hot-unplugging virtio-net-pci I'd expect we free both virtio-net-pci and virtio-net-device (and therefore call del_boot_device_path twice, once for each device). Can you check that? Yes, I can. The del_boot_device_path() is called only once, just for virtio-net-pci. For its child, virtio-net-devcie's resource is cleaned by qbus-child unrealizing process, will not call device_finalize(). Command line: # ./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 -drive file=/mnt/sdb/gonglei/image/win7_32_2U,if=none,id=drive-ide0-0-0 -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 -drive file=/mnt/sdb/gonglei/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0-0-1 -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4 -vnc 0.0.0.0:10 -netdev type=user,id=net0 -device virtio-net-pci,netdev=net0,bootindex=3,id=nic1 QEMU 2.1.50 monitor - type 'help' for more information (qemu) device_del nic1 (qemu) The below is the backtrace by gdb: Breakpoint 1, virtio_net_device_unrealize (dev=0x7f1b1ea87d58, errp=0x7f1b17d8c350) at /mnt/sdb/gonglei/qemu.git/qemu/hw/net/virtio-net.c:1649 1649{ (gdb) bt #0 virtio_net_device_unrealize (dev=0x7f1b1ea87d58, errp=0x7f1b17d8c350) at /mnt/sdb/gonglei/qemu.git/qemu/hw/net/virtio-net.c:1649 #1 0x7f1b1dd69b71 in virtio_device_unrealize (dev=0x7f1b1ea87d58, errp=0x7f1b17d8c3c8) at /mnt/sdb/gonglei/qemu.git/qemu/hw/virtio/virtio.c:1312 #2 0x7f1b1de8a1a4 in device_set_realized (obj=0x7f1b1ea87d58, value=false, errp=0x7f1b17d8c568) at hw/core/qdev.c:885 #3 0x7f1b1dfa01f7 in property_set_bool (obj=0x7f1b1ea87d58, v=0x7f1b1eb29de0, opaque=0x7f1b1eaa72c0, name= 0x7f1b1e0c2b69 realized, errp=0x7f1b17d8c568) at qom/object.c:1467 #4 0x7f1b1df9e82f in object_property_set (obj=0x7f1b1ea87d58, v=0x7f1b1eb29de0, name=0x7f1b1e0c2b69 realized, errp= 0x7f1b17d8c568) at qom/object.c:814 #5 0x7f1b1dfa0c45 in object_property_set_qobject (obj=0x7f1b1ea87d58, value=0x7f1b1eb09c80, name=0x7f1b1e0c2b69 realized, errp=0x7f1b17d8c568) at qom/qom-qobject.c:24 #6 0x7f1b1df9ebd7 in object_property_set_bool (obj=0x7f1b1ea87d58, value=false, name=0x7f1b1e0c2b69 realized, errp= 0x7f1b17d8c568) at qom/object.c:878 #7 0x7f1b1de893fa in bus_set_realized (obj=0x7f1b1ea87ce0, value=false, errp=0x7f1b17d8c718) at hw/core/qdev.c:583 #8 0x7f1b1dfa01f7 in property_set_bool (obj=0x7f1b1ea87ce0, v=0x7f1b1ead5a10, opaque=0x7f1b1eaa1640, name= 0x7f1b1e0c2b69 realized, errp=0x7f1b17d8c718) at qom/object.c:1467 #9 0x7f1b1df9e82f in object_property_set (obj=0x7f1b1ea87ce0, v=0x7f1b1ead5a10, name=0x7f1b1e0c2b69 realized, errp= 0x7f1b17d8c718) at qom/object.c:814 #10 0x7f1b1dfa0c45 in object_property_set_qobject (obj=0x7f1b1ea87ce0, value=0x7f1b1eb09c60, name=0x7f1b1e0c2b69 realized, errp=0x7f1b17d8c718) at qom/qom-qobject.c:24 #11 0x7f1b1df9ebd7 in object_property_set_bool (obj=0x7f1b1ea87ce0, value=false, name=0x7f1b1e0c2b69 realized, errp= 0x7f1b17d8c718) at qom/object.c:878 #12 0x7f1b1de8a127 in device_set_realized (obj=0x7f1b1ea87380, value=false, errp=0x0) at hw/core/qdev.c:875 #13 0x7f1b1dfa01f7 in property_set_bool (obj=0x7f1b1ea87380, v=0x7f1b1eacf940, opaque=0x7f1b1ea89f20, name= 0x7f1b1e0c2b69 realized, errp=0x0) at qom/object.c:1467 #14 0x7f1b1df9e82f in object_property_set (obj=0x7f1b1ea87380, v=0x7f1b1eacf940, name=0x7f1b1e0c2b69 realized, errp=0x0) at qom/object.c:814 #15 0x7f1b1dfa0c45 in object_property_set_qobject (obj=0x7f1b1ea87380, value=0x7f1b1eb085e0, name=0x7f1b1e0c2b69 realized, errp=0x0) at qom/qom-qobject.c:24 #16 0x7f1b1df9ebd7 in object_property_set_bool (obj=0x7f1b1ea87380, value=false, name=0x7f1b1e0c2b69 realized, errp=0x0) at qom/object.c:878 #17 0x7f1b1de8a7d0 in device_unparent (obj=0x7f1b1ea87380) at hw/core/qdev.c:1010 #18 0x7f1b1df9f26b in object_finalize_child_property (obj=0x7f1b1e9f4f30, name=0x7f1b1eaa7ba0 nic1,
[Qemu-devel] Question about cow format with hexdump
Hi, I'm reading the source code of cow.c: https://github.com/qemu/qemu/blob/master/block/cow.c and try to understand the format better. I created a cow format imagefile and can run 'qume-img info' to query the header info --- -bash-4.1$ qemu-img create -f cow dummy 1M Formatting 'test/dummy', fmt=cow size=1048576 -bash-4.1$ qemu-img info dummy image: dummy file format: cow virtual size: 1.0M (1048576 bytes) disk size: 4.0K But when I used hexdump to dis the header part, I cannot find all info recorded: (compared the define of struct cow_header_v2 and cow_create()) -- 1) recognize the magic and version info: -bash-4.1$ hexdump -C dummy -n 8 4f 4f 4f 4d 00 00 00 02 |OOOM| 0008 2) backing_file and mtime fields are 0s: # I think the dummy should be recorded -bash-4.1$ hexdump -C dummy -n 1032 4f 4f 4f 4d 00 00 00 02 00 00 00 00 00 00 00 00 |OOOM| 0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 0400 00 00 00 00 00 00 00 00 || 0408 -bash-4.1$ hexdump -C dummy -n 1036 4f 4f 4f 4d 00 00 00 02 00 00 00 00 00 00 00 00 |OOOM| 0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 0400 00 00 00 00 00 00 00 00 00 00 00 00 || 040c 3) size field is 0s: -bash-4.1$ hexdump -C dummy -n 1044 # size should be 1M 4f 4f 4f 4d 00 00 00 02 00 00 00 00 00 00 00 00 |OOOM| 0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 0410 00 00 00 00 || 0414 4) sectorsize is 512: -bash-4.1$ hexdump -C dummy -n 1048 4f 4f 4f 4d 00 00 00 02 00 00 00 00 00 00 00 00 |OOOM| 0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 0410 00 00 00 00 00 10 00 00 || 0418 Can anyone help to explain this? Or how to debug further? -- Regards, shhuiw
[Qemu-devel] Fw:Re:What does COW mean?
Hi, Sorry to trouble you. I posted my question in the user mailing list, but can not get the right answer. So I forward it to the devel mailing list. Hope to find answer here. At 2014-09-02 04:33:50, shhuiw shh...@163.com wrote: Hi, I'm new to qemu community, and I'm trying the COW image format (old but simple:-). I have read through its source code, and didn't find anything about 'copy on write'. I wonder wthat COW stands for? Sorry for my unclear expression. I mean when copy-on-write happens if COW image format is used, and how the COW driver code handles cop-on-write? -- Regards, shhuiw -- Regards, shhuiw
Re: [Qemu-devel] tcmu-runner and QEMU
Il 03/09/2014 02:20, Andy Grover ha scritto: The qemu-lio tool would live in the QEMU codebase and reuse all the infrastructure. For example, it could include a QMP monitor just like the one you are adding to qemu-nbd. Benoit and I talked a little about QMP on another part of the thread... I said I didn't think we needed a QMP monitor in qemu-lio-tcmu, but let me spin up on qemu a little more and I'll be able to speak more intelligently. You do need it. If you think of it from the traditional NAS viewpoint, it's how you do things like snapshots, mirroring, RAID recovery, and all that. Paolo
Re: [Qemu-devel] [Qemu-stable] Patch Round-up for stable 2.1.1, freeze on 2014-09-03
On Wed, Sep 3, 2014 at 10:10 AM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 02:17:02AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 2:09 AM, Andrey Korolyov and...@xdel.ru wrote: On Wed, Sep 3, 2014 at 1:51 AM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 01:29:29AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 1:03 AM, Michael S. Tsirkin m...@redhat.com wrote: bad one is the Author: Jason Wang jasow...@redhat.com Date: Tue Sep 2 18:07:46 2014 +0300 vhost_net: start/stop guest notifiers properly upstream has this (pull request sent today): vhost_net: cleanup start/stop condition Could you apply it and see if it helps please? Michael, if it helps it should be before start/stop guest notifiers ideally to avoid bisect problems. It is already applied as shown from the list in the previous message (there are some aio fixes too on top of 2.1 I picked before but they should not impact vhost-net interaction in any mean). The symptoms are a bit interesting - VM crashes only at PCI device initalization (e.g. grub stage after reset and initrd unpacking are passing well, but then things getting ugly). I am running 3.14 guest i686-pae kernel from debian backports in guest, so it may be version-specific after all. If it`ll be hard to reproduce, I can try 64bit, expecting same behavior. Please find args in attached file. ok just to make sure - which tree do I clone exactly? https://github.com/mdroth/qemu.git stable-2.1-staging showing same behavior for me with those patches Forgot to mention important detail - I am playing with -mq now, so actually virtio-net working in a bit different way than it may expected (it also shown in args list from above, but someone may miss it): ... qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio ... OK I see at least one obvious bug there: does the following fix the crash for you? Separately, we need to debug why mq vhost is broken for you. Is this a regression? diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..1fe18c7 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +err: +r = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (r 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, r); +fflush(stderr); +} return r; } another bits of information: - the userspace fallback is not specific to mq (very unfortunately for me because I didn`t checked this exact regression week before when I saw it for mq and it is not specific for queued patches for 2.1.1), - bug itself is not specific to mq, reproduces every time even with more generic interface config without queues, - patch from above does not fix the issue. Strace output for all threads is available at http://xdel.ru/downloads/qemu.out.gz, attached just before reset.
Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property
Hi, -Original Message- From: Gonglei (Arei) Sent: Saturday, August 30, 2014 6:00 PM Subject: [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property From: Gonglei arei.gong...@huawei.com when we remove bootindex form qdev.property to qom.property, we can use those functions set/get bootindex property for all correlative devices. Signed-off-by: Gonglei arei.gong...@huawei.com --- include/sysemu/sysemu.h | 4 vl.c| 27 +++ 2 files changed, 31 insertions(+) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 672984c..ca231e4 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -208,6 +208,10 @@ void do_usb_del(Monitor *mon, const QDict *qdict); void usb_info(Monitor *mon, const QDict *qdict); void check_boot_index(int32_t bootindex, Error **errp); +void get_bootindex(int32_t *bootindex, Visitor *v, + const char *name, Error **errp); +void set_bootindex(int32_t *bootindex, Visitor *v, + const char *name, Error **errp); void del_boot_device_path(DeviceState *dev); void add_boot_device_path(int32_t bootindex, DeviceState *dev, const char *suffix); diff --git a/vl.c b/vl.c index f2c3b2d..4363185 100644 --- a/vl.c +++ b/vl.c @@ -1252,6 +1252,33 @@ void check_boot_index(int32_t bootindex, Error **errp) } } +void get_bootindex(int32_t *bootindex, Visitor *v, + const char *name, Error **errp) +{ +visit_type_int32(v, bootindex, name, errp); +} + +void set_bootindex(int32_t *bootindex, Visitor *v, + const char *name, Error **errp) +{ +int32_t boot_index; +Error *local_err = NULL; + +visit_type_int32(v, boot_index, name, local_err); + +if (local_err == NULL) { +/* check the bootindex existes or not in fw_boot_order list */ +check_boot_index(boot_index, local_err); +} + +if (local_err) { +error_propagate(errp, local_err); +return; +} +/* change bootindex to a new one */ +*bootindex = boot_index; +} + In this version, I just change devices' bootindex value, but not update fw_boot_order immediately. When we reboot the vm, we will call add_boot_device_path to update fw_boot_order list. Do you think it is a good method? This method will cause a problem that when we want set a existed bootindex which has been changed, we will get failure. Only after vm rebooting, we can set the same bootindex again. I have listed the example in PATCH 00/27 cover-letter: (qemu) qom-get /machine/peripheral/nic1 bootindex 3 (0x3) (qemu) qom-set /machine/peripheral/nic1 bootindex 3 The bootindex 3 has already been used (qemu) qom-set /machine/peripheral/nic1 bootindex 0 ---change nic1's bootindex to 0, successful (qemu) qom-get /machine/peripheral/nic1 bootindex 0 (0x0) (qemu) qom-set /machine/peripheral/floppy1 bootindexA 3 --set floppy1's bootindex to 3, failed The bootindex 3 has already been used (qemu) system_reset -- reboot vm (qemu) qom-get /machine/peripheral/nic1 bootindex 0 (0x0) (qemu) qom-get /machine/peripheral/floppy1 bootindexA 5 (0x5) (qemu) qom-set /machine/peripheral/floppy1 bootindexA 3 --set floppy1's bootindex to 3, successful (qemu) qom-get /machine/peripheral/floppy1 bootindexA 3 (0x3) (qemu) Any ideas will be appreciated! Best regards, -Gonglei static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst) { bool ret = false; -- 1.7.12.4
Re: [Qemu-devel] [PATCH 0/3] build-sys: Exclude empty object files when linking libqemuutil.a
Il 03/09/2014 05:44, Fam Zheng ha scritto: On Mac OS X, ranlib complains on a few empty objects: ARlibqemuutil.a /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libqemuutil.a(generated-tracers.o) has no symbols /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libqemuutil.a(host-utils.o) has no symbols /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: libqemuutil.a(getauxval.o) has no symbols This series fixes the warnings. Fam Fam Zheng (3): trace: Only link generated-tracers.o with simple backend util: Move general qemu_getauxval to util/getauxval.c util: Don't link host-utils.o if it's empty include/qemu/osdep.h | 4 trace/Makefile.objs | 3 +-- util/Makefile.objs | 3 ++- util/getauxval.c | 8 util/host-utils.c| 2 -- 5 files changed, 11 insertions(+), 9 deletions(-) Thanks, looks good. Stefan, can you ack the first patch? Paolo
Re: [Qemu-devel] [PATCH v2] dump: let dump_error return error info to caller
On 2014/9/3 3:56, Eric Blake wrote: On 09/02/2014 02:25 AM, zhanghailiang wrote: The second parameter of dump_error is unused, but one purpose of using this function is to report the error info. Use error_set to return the error info to the caller. Signed-off-by: zhanghailiangzhang.zhanghaili...@huawei.com --- V2: - Return the error reason to the caller which suggested by Luiz Capitulino. --- dump.c | 165 - 1 file changed, 82 insertions(+), 83 deletions(-) diff --git a/dump.c b/dump.c index 71d3e94..0ab72e7 100644 --- a/dump.c +++ b/dump.c @@ -81,9 +81,10 @@ static int dump_cleanup(DumpState *s) return 0; } -static void dump_error(DumpState *s, const char *reason) +static void dump_error(DumpState *s, Error **errp, const char *reason) The Error **errp is typically listed last. { dump_cleanup(s); +error_setg(errp, %s, reason); } static int fd_write_vmcore(const void *buf, size_t size, void *opaque) @@ -99,7 +100,7 @@ static int fd_write_vmcore(const void *buf, size_t size, void *opaque) return 0; } -static int write_elf64_header(DumpState *s) +static int write_elf64_header(DumpState *s, Error **errp) { Elf64_Ehdr elf_header; int ret; @@ -126,14 +127,14 @@ static int write_elf64_header(DumpState *s) ret = fd_write_vmcore(elf_header, sizeof(elf_header), s); if (ret 0) { -dump_error(s, dump: failed to write elf header.\n); +dump_error(s, errp, dump: failed to write elf header.\n); This ends up calling error_setg with a trailing newline, which should not be needed. It looks like all of your conversions to the new dump_error should drop the \n in the message. OK, i will fix it, Thanks.:)
Re: [Qemu-devel] [PATCH] block/iscsi: use 16 byte CDBs only when necessary
Am 02.09.2014 um 21:30 schrieb Peter Lieven p...@kamp.de: Looking at the code, is it possible that not the guest is causing trouble here, but multiwrite_merge code? From what I see the only limit it has when merging requests is the number of IOVs. Any thoughts? Mine are: a) Introducing bs-bl.max_request_size and set merge = 0 if the result would be too big. Default max request size to 32768 sectors (see below). b) Hardcoding the limit in multiwrite_merge for now limiting the merged size to 16MB (32768 sectors). Which is the limit we already use in bdrv_co_discard and bdrv_co_write_zeroes if we don't know better. or c) disabling multiwrite merge for RAW or only iSCSI completely. Peter Peter Am 02.09.2014 um 17:28 schrieb ronnie sahlberg: That is one big request. I assume the device reports no limit in the VPD page so we can not state it is the guest/application going beyond the allowed limit? I am not entirely sure what meaning the target assigns to Protocol Error means here. It could be that ~100M is way higher than MaxBurstLength ? What is the MaxBurstLength that was reported by the server during login negotiation? If so, we should make libiscsi check the maxburstlength and fail the request early. We would still fail the I/O so it will not really solve anything much but at least we should not send the request to the server. Best would probably be to take the smallest of a non-zero Block-Limits.max_transfer_length and iscsi-MaxBurstLength/block-size and pass this back to the guest in the emulated Block-Limits-VPD. At least then you have tried to tell the guest never do SCSI I/O bigger than this. I.e. even if the target reports BlockLimits.MaxTransferLength == 0 == no limit to QEMU, QEMU should probably take the iscsi transport limit into account and pass this to the guest by setting the emulated BlockLimits page it passes to scale to the maximum that MaxBurstLength allows. Then if BTRFS or SG_IO in the guest ignores the BlockLimits it is clearly a guest problem. (A different interpretation for ProtocolError could be the mismatch between the iscsi expected data transfer length and the scsi transfer length, but that should result in residuals, not protocol error.) Hypothetically there could be targets that support really huge MaxBurstLengths 32MB. For those you probably want to switch to WRITE16 when the SCSI transfer length goes 0x. - if (iscsilun-use_16_for_rw) { + if (iscsilun-use_16_for_rw || num_sectors 0x) { regards ronnie sahlberg On Mon, Sep 1, 2014 at 8:21 AM, Peter Lieven p...@kamp.de wrote: On 17.06.2014 13:46, Paolo Bonzini wrote: Il 17/06/2014 13:37, Peter Lieven ha scritto: On 17.06.2014 13:15, Paolo Bonzini wrote: Il 17/06/2014 08:14, Peter Lieven ha scritto: BTW, while debugging a case with a bigger storage supplier I found that open-iscsi seems to do exactly this undeterministic behaviour. I have a 3TB LUN. If I access 2TB sectors it uses READ10/WRITE10 and if I go beyond 2TB it changes to READ16/WRITE16. Isn't that exactly what your latest patch does for 64K sector writes? :) Not exactly, we choose the default by checking the LUN size. 10 Byte for 2TB and 16 Byte otherwise. Yeah, I meant introducing the non-determinism. My latest patch makes an exception if a request is bigger than 64K sectors and switches to 16 Byte requests. These would otherwise end in an I/O error. It could also be split at the block layer, like we do for unmap. I think there's also a maximum transfer size somewhere in the VPD, we could to READ16/WRITE16 if it is 64K sectors. It seems that there might be a real world example where Linux issues 32MB write requests. Maybe someone familiar with btrfs can advise. I see iSCSI Protocol Errors in my logs: Sep 1 10:10:14 libiscsi:0 PDU header: 01 a1 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 07 06 8f 30 00 00 00 00 06 00 00 00 0a 2a 00 01 09 9e 50 00 47 98 00 00 00 00 00 00 00 [XXX] Sep 1 10:10:14 qemu-2.0.0: iSCSI: Failed to write10 data to iSCSI lun. Request was rejected with reason: 0x04 (Protocol Error) Looking at the headers the xferlen in the iSCSI PDU is 110047232 Byte which is 214936 sectors. 214936 % 65536 = 18328 which is exactly the number of blocks in the SCSI WRITE10 CDB. Can someone advise if this is something that btrfs can cause or if I have to blame the customer that he issues very big write requests with Direct I/O? The user sseems something like this in the log: [34640.489284] BTRFS: bdev /dev/vda2 errs: wr 8232, rd 0, flush 0, corrupt 0, gen 0 [34640.490379] end_request: I/O error, dev vda, sector 17446880 [34640.491251] end_request: I/O error, dev vda, sector 5150144 [34640.491290] end_request: I/O error, dev vda, sector 17472080 [34640.492201] end_request: I/O error, dev vda, sector 17523488 [34640.492201] end_request: I/O error, dev
Re: [Qemu-devel] [Qemu-stable] Patch Round-up for stable 2.1.1, freeze on 2014-09-03
On Wed, Sep 03, 2014 at 11:43:54AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 10:10 AM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 02:17:02AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 2:09 AM, Andrey Korolyov and...@xdel.ru wrote: On Wed, Sep 3, 2014 at 1:51 AM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 01:29:29AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 1:03 AM, Michael S. Tsirkin m...@redhat.com wrote: bad one is the Author: Jason Wang jasow...@redhat.com Date: Tue Sep 2 18:07:46 2014 +0300 vhost_net: start/stop guest notifiers properly upstream has this (pull request sent today): vhost_net: cleanup start/stop condition Could you apply it and see if it helps please? Michael, if it helps it should be before start/stop guest notifiers ideally to avoid bisect problems. It is already applied as shown from the list in the previous message (there are some aio fixes too on top of 2.1 I picked before but they should not impact vhost-net interaction in any mean). The symptoms are a bit interesting - VM crashes only at PCI device initalization (e.g. grub stage after reset and initrd unpacking are passing well, but then things getting ugly). I am running 3.14 guest i686-pae kernel from debian backports in guest, so it may be version-specific after all. If it`ll be hard to reproduce, I can try 64bit, expecting same behavior. Please find args in attached file. ok just to make sure - which tree do I clone exactly? https://github.com/mdroth/qemu.git stable-2.1-staging showing same behavior for me with those patches Forgot to mention important detail - I am playing with -mq now, so actually virtio-net working in a bit different way than it may expected (it also shown in args list from above, but someone may miss it): ... qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio ... OK I see at least one obvious bug there: does the following fix the crash for you? Separately, we need to debug why mq vhost is broken for you. Is this a regression? diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..1fe18c7 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +err: +r = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (r 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, r); +fflush(stderr); +} return r; } another bits of information: - the userspace fallback is not specific to mq (very unfortunately for me because I didn`t checked this exact regression week before when I saw it for mq and it is not specific for queued patches for 2.1.1), - bug itself is not specific to mq, reproduces every time even with more generic interface config without queues, - patch from above does not fix the issue. Strace output for all threads is available at http://xdel.ru/downloads/qemu.out.gz, attached just before reset. OK does my patch help? Jason sent patches to fix the fallback to virtio bug - does that work for you?
Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property
Hi, +void set_bootindex(int32_t *bootindex, Visitor *v, + const char *name, Error **errp) +{ +int32_t boot_index; +Error *local_err = NULL; + +visit_type_int32(v, boot_index, name, local_err); + +if (local_err == NULL) { +/* check the bootindex existes or not in fw_boot_order list */ +check_boot_index(boot_index, local_err); +} + +if (local_err) { +error_propagate(errp, local_err); +return; +} +/* change bootindex to a new one */ +*bootindex = boot_index; +} + In this version, I just change devices' bootindex value, but not update fw_boot_order immediately. When we reboot the vm, we will call add_boot_device_path to update fw_boot_order list. Do you think it is a good method? This method will cause a problem that when we want set a existed bootindex which has been changed, we will get failure. Only after vm rebooting, we can set the same bootindex again. Good point. check_boot_index() doing the verification against stale data is pointless and may even throw an error in cases where it should not. I guess we should add a add_boot_device_path() call to the ${device}_set_bootindex functions. Which also means we don't need to do it on reset (patch #6 can be dropped). And I think we can also drop the add_boot_device_path() calls from ${device}_realize then. cheers, Gerd
Re: [Qemu-devel] [PATCH] vhost_net: initialize acked_features to a safe value during ack
On Wed, Sep 03, 2014 at 02:25:30PM +0800, Jason Wang wrote: commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. But acked features are set in vhost_ack_features. why would we need to initialize to backend_features? 0 is a better default. This will result an unexpected value of acked_features which may fail the features setting of vhost. This patch fixes it by recover this step. Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: Andrey Korolyov and...@xdel.ru Cc: Michael S. Tsirkin m...@redhat.com Cc: Michael Roth mdr...@linux.vnet.ibm.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com --- hw/net/vhost_net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index f87c798..b1d4b1f 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -115,6 +115,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) void vhost_net_ack_features(struct vhost_net *net, unsigned features) { +net-dev.acked_features = net-dev.backend_features; I think it's wrong: you don't want to set all features. vhost_ack_features(net-dev, vhost_net_get_feature_bits(net), features); } -- 1.8.3.1
Re: [Qemu-devel] [Qemu-stable] Patch Round-up for stable 2.1.1, freeze on 2014-09-03
On Wed, Sep 3, 2014 at 12:13 PM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 11:43:54AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 10:10 AM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 02:17:02AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 2:09 AM, Andrey Korolyov and...@xdel.ru wrote: On Wed, Sep 3, 2014 at 1:51 AM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 01:29:29AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 1:03 AM, Michael S. Tsirkin m...@redhat.com wrote: bad one is the Author: Jason Wang jasow...@redhat.com Date: Tue Sep 2 18:07:46 2014 +0300 vhost_net: start/stop guest notifiers properly upstream has this (pull request sent today): vhost_net: cleanup start/stop condition Could you apply it and see if it helps please? Michael, if it helps it should be before start/stop guest notifiers ideally to avoid bisect problems. It is already applied as shown from the list in the previous message (there are some aio fixes too on top of 2.1 I picked before but they should not impact vhost-net interaction in any mean). The symptoms are a bit interesting - VM crashes only at PCI device initalization (e.g. grub stage after reset and initrd unpacking are passing well, but then things getting ugly). I am running 3.14 guest i686-pae kernel from debian backports in guest, so it may be version-specific after all. If it`ll be hard to reproduce, I can try 64bit, expecting same behavior. Please find args in attached file. ok just to make sure - which tree do I clone exactly? https://github.com/mdroth/qemu.git stable-2.1-staging showing same behavior for me with those patches Forgot to mention important detail - I am playing with -mq now, so actually virtio-net working in a bit different way than it may expected (it also shown in args list from above, but someone may miss it): ... qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio ... OK I see at least one obvious bug there: does the following fix the crash for you? Separately, we need to debug why mq vhost is broken for you. Is this a regression? diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..1fe18c7 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +err: +r = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (r 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, r); +fflush(stderr); +} return r; } another bits of information: - the userspace fallback is not specific to mq (very unfortunately for me because I didn`t checked this exact regression week before when I saw it for mq and it is not specific for queued patches for 2.1.1), - bug itself is not specific to mq, reproduces every time even with more generic interface config without queues, - patch from above does not fix the issue. Strace output for all threads is available at http://xdel.ru/downloads/qemu.out.gz, attached just before reset. OK does my patch help? Jason sent patches to fix the fallback to virtio bug - does that work for you? Whoops, missed patch from Jason, meant yours above. The acceleration is fixed, thanks! Jason`s patch alone fixes both crash appearance and accel initialization while yours fixed initialization (while intended to fix assert appearance), with crash still in place.
Re: [Qemu-devel] [PATCH v6 07/27] vl.c: add setter/getter functions for bootindex property
From: Gerd Hoffmann [mailto:kra...@redhat.com] Hi, +void set_bootindex(int32_t *bootindex, Visitor *v, + const char *name, Error **errp) +{ +int32_t boot_index; +Error *local_err = NULL; + +visit_type_int32(v, boot_index, name, local_err); + +if (local_err == NULL) { +/* check the bootindex existes or not in fw_boot_order list */ +check_boot_index(boot_index, local_err); +} + +if (local_err) { +error_propagate(errp, local_err); +return; +} +/* change bootindex to a new one */ +*bootindex = boot_index; +} + In this version, I just change devices' bootindex value, but not update fw_boot_order immediately. When we reboot the vm, we will call add_boot_device_path to update fw_boot_order list. Do you think it is a good method? This method will cause a problem that when we want set a existed bootindex which has been changed, we will get failure. Only after vm rebooting, we can set the same bootindex again. Good point. check_boot_index() doing the verification against stale data is pointless and may even throw an error in cases where it should not. Yes. I guess we should add a add_boot_device_path() call to the ${device}_set_bootindex functions. Which also means we don't need to do it on reset (patch #6 can be dropped). And I think we can also drop the add_boot_device_path() calls from ${device}_realize then. Bravo! I can't agree more with you. Thanks, Gerd. Will do it. Best regards, -Gonglei
[Qemu-devel] [RFC] vfio: migration to trace points
This patch removes all DPRINTF and replace them by trace points. A few DPRINTF used in error cases were transformed into error_report. Signed-off-by: Eric Auger eric.au...@linaro.org --- - __func__ is removed since trace point name does the same job - HWADDR_PRIx were replaced by PRIx64 Besides those changes format strings were kept the same. in few cases however I was forced to change them due to parsing errors (always related to parenthesis handling). This is indicated in trace-events. Cases than are not correctly handled are given below: - (%04x:%02x:%02x.%x) need to be replaced by (%04x:%02x:%02x.%x) - %s read(%04x:%02x:%02x.%x:BAR%d+0x%PRIx64, %d) = 0x%PRIx64 - %s read(%04x:%02x:%02x.%x:BAR%d+0x%PRIx64, %d = 0x%PRIx64 - - %s write(%04x:%02x:%02x.%x:BAR%d+0x%PRIx64, 0x%PRIx64, %d) %s write(%04x:%02x:%02x.%x:BAR%d+0x%PRIx64, 0x%PRIx64, %d This is a temporary fix. - This leads to a too large amount of trace points which may not be eligible as trace points - I don't know?- - this transformation just is tested compiled on PCI. Tested on platform qemu configured with --enable-trace-backends=stderr - in future, format strings and calls may be simplified by using a single name argument instead of domain, bus, slot, function. --- hw/misc/vfio.c | 403 + trace-events | 79 +++ 2 files changed, 285 insertions(+), 197 deletions(-) diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c index 40dcaa6..6b6dee9 100644 --- a/hw/misc/vfio.c +++ b/hw/misc/vfio.c @@ -40,15 +40,7 @@ #include sysemu/kvm.h #include sysemu/sysemu.h #include hw/misc/vfio.h - -/* #define DEBUG_VFIO */ -#ifdef DEBUG_VFIO -#define DPRINTF(fmt, ...) \ -do { fprintf(stderr, vfio: fmt, ## __VA_ARGS__); } while (0) -#else -#define DPRINTF(fmt, ...) \ -do { } while (0) -#endif +#include trace.h /* Extra debugging, trap acceleration paths for more logging */ #define VFIO_ALLOW_MMAP 1 @@ -365,9 +357,9 @@ static void vfio_intx_interrupt(void *opaque) return; } -DPRINTF(%s(%04x:%02x:%02x.%x) Pin %c\n, __func__, vdev-host.domain, -vdev-host.bus, vdev-host.slot, vdev-host.function, -'A' + vdev-intx.pin); +trace_vfio_intx_interrupt(vdev-host.domain, vdev-host.bus, + vdev-host.slot, vdev-host.function, + 'A' + vdev-intx.pin); vdev-intx.pending = true; pci_irq_assert(vdev-pdev); @@ -384,8 +376,8 @@ static void vfio_eoi(VFIODevice *vdev) return; } -DPRINTF(%s(%04x:%02x:%02x.%x) EOI\n, __func__, vdev-host.domain, -vdev-host.bus, vdev-host.slot, vdev-host.function); +trace_vfio_eoi(vdev-host.domain, vdev-host.bus, + vdev-host.slot, vdev-host.function); vdev-intx.pending = false; pci_irq_deassert(vdev-pdev); @@ -454,9 +446,8 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev) vdev-intx.kvm_accel = true; -DPRINTF(%s(%04x:%02x:%02x.%x) KVM INTx accel enabled\n, -__func__, vdev-host.domain, vdev-host.bus, -vdev-host.slot, vdev-host.function); +trace_vfio_enable_intx_kvm(vdev-host.domain, vdev-host.bus, + vdev-host.slot, vdev-host.function); return; @@ -508,9 +499,8 @@ static void vfio_disable_intx_kvm(VFIODevice *vdev) /* If we've missed an event, let it re-fire through QEMU */ vfio_unmask_intx(vdev); -DPRINTF(%s(%04x:%02x:%02x.%x) KVM INTx accel disabled\n, -__func__, vdev-host.domain, vdev-host.bus, -vdev-host.slot, vdev-host.function); +trace_vfio_disable_intx_kvm(vdev-host.domain, vdev-host.bus, +vdev-host.slot, vdev-host.function); #endif } @@ -529,9 +519,9 @@ static void vfio_update_irq(PCIDevice *pdev) return; /* Nothing changed */ } -DPRINTF(%s(%04x:%02x:%02x.%x) IRQ moved %d - %d\n, __func__, -vdev-host.domain, vdev-host.bus, vdev-host.slot, -vdev-host.function, vdev-intx.route.irq, route.irq); +trace_vfio_update_irq(vdev-host.domain, vdev-host.bus, + vdev-host.slot, vdev-host.function, + vdev-intx.route.irq, route.irq); vfio_disable_intx_kvm(vdev); @@ -607,8 +597,8 @@ static int vfio_enable_intx(VFIODevice *vdev) vdev-interrupt = VFIO_INT_INTx; -DPRINTF(%s(%04x:%02x:%02x.%x)\n, __func__, vdev-host.domain, -vdev-host.bus, vdev-host.slot, vdev-host.function); +trace_vfio_enable_intx(vdev-host.domain, vdev-host.bus, + vdev-host.slot, vdev-host.function); return 0; } @@ -630,8 +620,8 @@ static void vfio_disable_intx(VFIODevice *vdev) vdev-interrupt = VFIO_INT_NONE; -DPRINTF(%s(%04x:%02x:%02x.%x)\n, __func__, vdev-host.domain, -vdev-host.bus, vdev-host.slot, vdev-host.function); +trace_vfio_disable_intx(vdev-host.domain, vdev-host.bus, +
[Qemu-devel] [PATCH] vhost_net: initialize acked_features to a safe value during ack
commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. As this field is now uninitialized, vhost initialization will sometimes fail. To fix, initialize field in core vhost code. As the next step, cleanup vhost scsi code as well. Reported-by: Jason Wang jasow...@redhat.com Reported-by: Andrey Korolyov and...@xdel.ru Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com Reviewed-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/vhost_net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 1fe18c7..e258fda 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -115,6 +115,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) void vhost_net_ack_features(struct vhost_net *net, unsigned features) { +net-dev.acked_features = net-dev.backend_features; vhost_ack_features(net-dev, vhost_net_get_feature_bits(net), features); } -- MST
Re: [Qemu-devel] [PATCH] vhost_net: initialize acked_features to a safe value during ack
On Wed, Sep 03, 2014 at 02:25:30PM +0800, Jason Wang wrote: commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. This will result an unexpected value of acked_features which may fail the features setting of vhost. This patch fixes it by recover this step. Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: Andrey Korolyov and...@xdel.ru Cc: Michael S. Tsirkin m...@redhat.com Cc: Michael Roth mdr...@linux.vnet.ibm.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com OK I get it and it's correct, but I think it's better to put the initialization in core vhost code. Patch sent, could you confirm that it works for you please? --- hw/net/vhost_net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index f87c798..b1d4b1f 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -115,6 +115,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) void vhost_net_ack_features(struct vhost_net *net, unsigned features) { +net-dev.acked_features = net-dev.backend_features; vhost_ack_features(net-dev, vhost_net_get_feature_bits(net), features); } -- 1.8.3.1
Re: [Qemu-devel] [PATCH V2] fix: unix sockets created for virtio-serail has insufficient permissions
On Wed, Sep 03, 2014 at 02:18:07PM +0800, Chunyan Liu wrote: Add umask to _virCommand, allow user to set umask to command. Set umask(002) to qemu process to overwrite default umask(022) so that unix sockets created for virtio-serial has expected permissions. Fix problem reported here: https://sourceware.org/bugzilla/show_bug.cgi?id=13078#c11 https://bugzilla.novell.com/show_bug.cgi?id=888166 To use virtio-serial device, unix socket created for chardev with default umask(022) has insufficient permissions. e.g.: -device virtio-serial \ -chardev socket,path=/tmp/foo,server,nowait,id=foo \ -device virtserialport,chardev=foo,name=org.fedoraproject.port.0 srwxr-xr-x 1 qemu qemu 0 21. Jul 14:19 /tmp/somefile.sock Other users in the same group (like real user, test engines, etc) cannot write to this socket. Signed-off-by: Chunyan Liu cy...@suse.com --- Changes: * set umask(002) to the whole qemu process instead of calling umask in qemu unix_listen_opts. V1 is here: http://www.mail-archive.com/libvir-list@redhat.com/msg101513.html src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 1 + src/util/vircommand.c| 11 +++ src/util/vircommand.h| 1 + 4 files changed, 14 insertions(+) ACK, you could possibly argue that it should be configurable in qemu.conf, but I think that would be overkill. We unconditionally put QEMU into a special group, so we really should make sure that stuff is accessible to that group via umask. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v2 2/3] target-sparc: address_mask(), asi_address_mask() are TARGET_SPARC64 only
On Tue, Sep 2, 2014 at 1:52 PM, Peter Maydell peter.mayd...@linaro.org wrote: The address_mask() and asi_address_mask() functions are only used in TARGET_SPARC64 configs, so guard with ifdefs to avoid warnings about unused functions in 32-bit builds. Since the main reason these functions were marked 'inline' was to suppress unused-function warnings with gcc, we remove the 'inline' as no longer necessary. I thought address_mask was inline because it's in the hot path. Wouldn't the removal hit performance? Artyom Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-sparc/ldst_helper.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c index 1a62e19..2d028a5 100644 --- a/target-sparc/ldst_helper.c +++ b/target-sparc/ldst_helper.c @@ -250,7 +250,8 @@ static void replace_tlb_1bit_lru(SparcTLBEntry *tlb, #endif -static inline target_ulong address_mask(CPUSPARCState *env1, target_ulong addr) +#if defined(TARGET_SPARC64) || defined(CONFIG_USER_ONLY) +static target_ulong address_mask(CPUSPARCState *env1, target_ulong addr) { #ifdef TARGET_SPARC64 if (AM_CHECK(env1)) { @@ -259,6 +260,7 @@ static inline target_ulong address_mask(CPUSPARCState *env1, target_ulong addr) #endif return addr; } +#endif /* returns true if access using this ASI is to have address translated by MMU otherwise access is to raw physical address */ @@ -287,8 +289,9 @@ static inline int is_translating_asi(int asi) #endif } -static inline target_ulong asi_address_mask(CPUSPARCState *env, -int asi, target_ulong addr) +#ifdef TARGET_SPARC64 +static target_ulong asi_address_mask(CPUSPARCState *env, + int asi, target_ulong addr) { if (is_translating_asi(asi)) { return address_mask(env, addr); @@ -296,6 +299,7 @@ static inline target_ulong asi_address_mask(CPUSPARCState *env, return addr; } } +#endif void helper_check_align(CPUSPARCState *env, target_ulong addr, uint32_t align) { -- 1.9.1 -- Regards, Artyom Tarasenko linux/sparc and solaris/sparc under qemu blog: http://tyom.blogspot.com/search/label/qemu
Re: [Qemu-devel] [PATCH] vhost_net: initialize acked_features to a safe value during ack
On Wed, Sep 03, 2014 at 11:50:05AM +0300, Michael S. Tsirkin wrote: commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. As this field is now uninitialized, vhost initialization will sometimes fail. To fix, initialize field in core vhost code. As the next step, cleanup vhost scsi code as well. Reported-by: Jason Wang jasow...@redhat.com Reported-by: Andrey Korolyov and...@xdel.ru Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com Reviewed-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com Oops, it's the wrong patch - original one from jason :( Sorry, resending. --- hw/net/vhost_net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 1fe18c7..e258fda 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -115,6 +115,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) void vhost_net_ack_features(struct vhost_net *net, unsigned features) { +net-dev.acked_features = net-dev.backend_features; vhost_ack_features(net-dev, vhost_net_get_feature_bits(net), features); } -- MST
[Qemu-devel] [PATCH] vhost_net: init acked_features to backend_features
commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. As this field is now uninitialized, vhost initialization will sometimes fail. To fix, initialize field in core vhost code. As the next step, cleanup vhost scsi code as well. Reported-by: Jason Wang jasow...@redhat.com Reported-by: Andrey Korolyov and...@xdel.ru Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com Reviewed-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/virtio/vhost.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 5d7c40a..e42e51f 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -873,6 +873,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, hdev-memory_changed = false; memory_listener_register(hdev-memory_listener, address_space_memory); hdev-force = force; +/* Set minimal required set of features. */ +hdev-acked_features = hdev-backend_features; + return 0; fail_vq: while (--i = 0) { -- MST
Re: [Qemu-devel] [PATCH] vhost_net: initialize acked_features to a safe value during ack
On Wed, Sep 3, 2014 at 12:52 PM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 02:25:30PM +0800, Jason Wang wrote: commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. This will result an unexpected value of acked_features which may fail the features setting of vhost. This patch fixes it by recover this step. Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: Andrey Korolyov and...@xdel.ru Cc: Michael S. Tsirkin m...@redhat.com Cc: Michael Roth mdr...@linux.vnet.ibm.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com OK I get it and it's correct, but I think it's better to put the initialization in core vhost code. Patch sent, could you confirm that it works for you please? --- hw/net/vhost_net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index f87c798..b1d4b1f 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -115,6 +115,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) void vhost_net_ack_features(struct vhost_net *net, unsigned features) { +net-dev.acked_features = net-dev.backend_features; vhost_ack_features(net-dev, vhost_net_get_feature_bits(net), features); } -- 1.8.3.1 Yes, this patch fixes both issues with vhost subsystem for me.
[Qemu-devel] [PATCH] vhost-scsi: init backend features earlier
As vhost core uses backend_features during init, clear it earlier to avoid using uninitialized memory. This is harmless since vhost scsi ignores the result anyway, but it avoids valgrind errors. Cc: qemu-sta...@nongnu.org Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/scsi/vhost-scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index ddfe76a..7146e0e 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -238,6 +238,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) s-dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs-conf.num_queues; s-dev.vqs = g_new(struct vhost_virtqueue, s-dev.nvqs); s-dev.vq_index = 0; +s-dev.backend_features = 0; ret = vhost_dev_init(s-dev, (void *)(uintptr_t)vhostfd, VHOST_BACKEND_TYPE_KERNEL, true); @@ -246,7 +247,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) strerror(-ret)); return; } -s-dev.backend_features = 0; error_setg(s-migration_blocker, vhost-scsi does not support migration); -- MST
Re: [Qemu-devel] [Qemu-stable] Patch Round-up for stable 2.1.1, freeze on 2014-09-03
On Wed, Sep 03, 2014 at 12:36:18PM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 12:13 PM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 11:43:54AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 10:10 AM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 02:17:02AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 2:09 AM, Andrey Korolyov and...@xdel.ru wrote: On Wed, Sep 3, 2014 at 1:51 AM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 01:29:29AM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 1:03 AM, Michael S. Tsirkin m...@redhat.com wrote: bad one is the Author: Jason Wang jasow...@redhat.com Date: Tue Sep 2 18:07:46 2014 +0300 vhost_net: start/stop guest notifiers properly upstream has this (pull request sent today): vhost_net: cleanup start/stop condition Could you apply it and see if it helps please? Michael, if it helps it should be before start/stop guest notifiers ideally to avoid bisect problems. It is already applied as shown from the list in the previous message (there are some aio fixes too on top of 2.1 I picked before but they should not impact vhost-net interaction in any mean). The symptoms are a bit interesting - VM crashes only at PCI device initalization (e.g. grub stage after reset and initrd unpacking are passing well, but then things getting ugly). I am running 3.14 guest i686-pae kernel from debian backports in guest, so it may be version-specific after all. If it`ll be hard to reproduce, I can try 64bit, expecting same behavior. Please find args in attached file. ok just to make sure - which tree do I clone exactly? https://github.com/mdroth/qemu.git stable-2.1-staging showing same behavior for me with those patches Forgot to mention important detail - I am playing with -mq now, so actually virtio-net working in a bit different way than it may expected (it also shown in args list from above, but someone may miss it): ... qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio ... OK I see at least one obvious bug there: does the following fix the crash for you? Separately, we need to debug why mq vhost is broken for you. Is this a regression? diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..1fe18c7 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +err: +r = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (r 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, r); +fflush(stderr); +} return r; } another bits of information: - the userspace fallback is not specific to mq (very unfortunately for me because I didn`t checked this exact regression week before when I saw it for mq and it is not specific for queued patches for 2.1.1), - bug itself is not specific to mq, reproduces every time even with more generic interface config without queues, - patch from above does not fix the issue. Strace output for all threads is available at http://xdel.ru/downloads/qemu.out.gz, attached just before reset. OK does my patch help? Jason sent patches to fix the fallback to virtio bug - does that work for you? Whoops, missed patch from Jason, meant yours above. The acceleration is fixed, thanks! Jason`s patch alone fixes both crash appearance and accel initialization while yours fixed initialization (while intended to fix assert appearance), OK so my patch fixes initialization likely by luck. with crash still in place. Hmm so something is still wrong with the userspace path. Could you please apply this debugging patch on top of all the stack that is now working for you, and see if assert still surfaces? diff --git a/hw/net/vhost_net.c
Re: [Qemu-devel] [RFC PATCH v1 0/4] Handle memory hotplug errors from guest OS.
Hi, Would anyone help to review this patch-set ? I'm not quit sure if this is a suitable way solve this problem. Thanks. On 08/27/2014 04:14 PM, tangchen wrote: Forgot to mention, this patch-set is based on the following patch-set: [RESEND PATCH v3 0/8] QEmu memory hot unplug support. https://www.mail-archive.com/qemu-devel@nongnu.org/msg253018.html Thanks. On 08/27/2014 04:09 PM, Tang Chen wrote: When doing memory hotplug, QEmu is not aware of guest OS error when hotplugging memory devices. Even if guest OS failed to hot-add memory, the pc-dimm device will be added to QEmu. Even if guest OS failed to hot-remove memory, QEmu will remove the pc-dimm device. An example is: for a Linux guest, the Linux kernel limited that the size of hot-added memory should be mutiple of memory section (128MB by default). If we add 130MB memory, the Linux kernel won't add it. We are not able to handle the size check in QEmu commmand line because different OS may have different limits. And also, QEmu outputs nothing but guest OS failed to hot-add memory will confuse users. We should at least report an error. So, we should report the error to users, and cancel the memory hotplug progress in QEmu. QEmu thread sends a SCI to guest OS and return immediately. The vcpu thread will emulate ACPI hardware operations. So this patch-set introduces a wait condition variable to synchronize these two threads. Tang Chen (4): Use macro to define ACPI notification event. Add event handling for memory device insertion. Introduce wait condition to catch guest OS memory hotplug error. Handle memory hotplug error from guest OS in QEmu. hw/acpi/memory_hotplug.c | 146 +-- include/hw/acpi/acpi.h | 15 - 2 files changed, 153 insertions(+), 8 deletions(-) .
Re: [Qemu-devel] [PATCH] vhost_net: initialize acked_features to a safe value during ack
On Wed, Sep 03, 2014 at 12:54:03PM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 12:52 PM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 02:25:30PM +0800, Jason Wang wrote: commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. This will result an unexpected value of acked_features which may fail the features setting of vhost. This patch fixes it by recover this step. Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: Andrey Korolyov and...@xdel.ru Cc: Michael S. Tsirkin m...@redhat.com Cc: Michael Roth mdr...@linux.vnet.ibm.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com OK I get it and it's correct, but I think it's better to put the initialization in core vhost code. Patch sent, could you confirm that it works for you please? --- hw/net/vhost_net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index f87c798..b1d4b1f 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -115,6 +115,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) void vhost_net_ack_features(struct vhost_net *net, unsigned features) { +net-dev.acked_features = net-dev.backend_features; vhost_ack_features(net-dev, vhost_net_get_feature_bits(net), features); } -- 1.8.3.1 Yes, this patch fixes both issues with vhost subsystem for me. Sorry posted a different one - can you pls try it out? We still have a bug somewhere in error handling I suspect, so let's keep debugging.
[Qemu-devel] [PATCH] vhost_net: cleanup recovery
commit aad4dce934649b3a398396fc2a76f215bb194ea4 vhost_net: start/stop guest notifiers properly changed the order of calls for guest notifiers, but did not recover in the correct (reverse) order. Fix it up. Cc: qemu-sta...@nongnu.org Cc: Andrey Korolyov and...@xdel.ru Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/vhost_net.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..1fe18c7 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +err: +r = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (r 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, r); +fflush(stderr); +} return r; } -- MST
Re: [Qemu-devel] [PATCH] vhost-scsi: init backend features earlier
Il 03/09/2014 11:02, Michael S. Tsirkin ha scritto: As vhost core uses backend_features during init, clear it earlier to avoid using uninitialized memory. This is harmless since vhost scsi ignores the result anyway, but it avoids valgrind errors. Cc: qemu-sta...@nongnu.org Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/scsi/vhost-scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index ddfe76a..7146e0e 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -238,6 +238,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) s-dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs-conf.num_queues; s-dev.vqs = g_new(struct vhost_virtqueue, s-dev.nvqs); s-dev.vq_index = 0; +s-dev.backend_features = 0; ret = vhost_dev_init(s-dev, (void *)(uintptr_t)vhostfd, VHOST_BACKEND_TYPE_KERNEL, true); @@ -246,7 +247,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) strerror(-ret)); return; } -s-dev.backend_features = 0; error_setg(s-migration_blocker, vhost-scsi does not support migration); Acked-by: Paolo Bonzini pbonz...@redhat.com want to keep it in your treee? Paolo
Re: [Qemu-devel] [PATCH] vhost-scsi: init backend features earlier
On Wed, Sep 03, 2014 at 11:08:00AM +0200, Paolo Bonzini wrote: Il 03/09/2014 11:02, Michael S. Tsirkin ha scritto: As vhost core uses backend_features during init, clear it earlier to avoid using uninitialized memory. This is harmless since vhost scsi ignores the result anyway, but it avoids valgrind errors. Cc: qemu-sta...@nongnu.org Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/scsi/vhost-scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index ddfe76a..7146e0e 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -238,6 +238,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) s-dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs-conf.num_queues; s-dev.vqs = g_new(struct vhost_virtqueue, s-dev.nvqs); s-dev.vq_index = 0; +s-dev.backend_features = 0; ret = vhost_dev_init(s-dev, (void *)(uintptr_t)vhostfd, VHOST_BACKEND_TYPE_KERNEL, true); @@ -246,7 +247,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) strerror(-ret)); return; } -s-dev.backend_features = 0; error_setg(s-migration_blocker, vhost-scsi does not support migration); Acked-by: Paolo Bonzini pbonz...@redhat.com want to keep it in your treee? Paolo Yes please.
Re: [Qemu-devel] [RFC] vfio: migration to trace points
On 03.09.14 10:45, Eric Auger wrote: This patch removes all DPRINTF and replace them by trace points. A few DPRINTF used in error cases were transformed into error_report. Signed-off-by: Eric Auger eric.au...@linaro.org --- - __func__ is removed since trace point name does the same job - HWADDR_PRIx were replaced by PRIx64 Besides those changes format strings were kept the same. in few cases however I was forced to change them due to parsing errors (always related to parenthesis handling). This is indicated in trace-events. Cases than are not correctly handled are given below: - (%04x:%02x:%02x.%x) need to be replaced by (%04x:%02x:%02x.%x) - %s read(%04x:%02x:%02x.%x:BAR%d+0x%PRIx64, %d) = 0x%PRIx64 - %s read(%04x:%02x:%02x.%x:BAR%d+0x%PRIx64, %d = 0x%PRIx64 - - %s write(%04x:%02x:%02x.%x:BAR%d+0x%PRIx64, 0x%PRIx64, %d) %s write(%04x:%02x:%02x.%x:BAR%d+0x%PRIx64, 0x%PRIx64, %d This is a temporary fix. - This leads to a too large amount of trace points which may not be eligible as trace points - I don't know?- - this transformation just is tested compiled on PCI. Tested on platform qemu configured with --enable-trace-backends=stderr - in future, format strings and calls may be simplified by using a single name argument instead of domain, bus, slot, function. I think it's a nice step into the right direction. Alex
Re: [Qemu-devel] [PATCH] vhost_net: initialize acked_features to a safe value during ack
On Wed, Sep 03, 2014 at 12:54:03PM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 12:52 PM, Michael S. Tsirkin m...@redhat.com wrote: On Wed, Sep 03, 2014 at 02:25:30PM +0800, Jason Wang wrote: commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. This will result an unexpected value of acked_features which may fail the features setting of vhost. This patch fixes it by recover this step. Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: Andrey Korolyov and...@xdel.ru Cc: Michael S. Tsirkin m...@redhat.com Cc: Michael Roth mdr...@linux.vnet.ibm.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com OK I get it and it's correct, but I think it's better to put the initialization in core vhost code. Patch sent, could you confirm that it works for you please? --- hw/net/vhost_net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index f87c798..b1d4b1f 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -115,6 +115,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) void vhost_net_ack_features(struct vhost_net *net, unsigned features) { +net-dev.acked_features = net-dev.backend_features; vhost_ack_features(net-dev, vhost_net_get_feature_bits(net), features); } -- 1.8.3.1 Yes, this patch fixes both issues with vhost subsystem for me. OK, applied, thanks! But let's hack on the assert a bit more: apparently something is wrong with the userspace fallback: after init failure, it should never trigger, and it does. -- MST
Re: [Qemu-devel] [PATCH] vhost-scsi: init backend features earlier
On 09/03/2014 05:02 PM, Michael S. Tsirkin wrote: As vhost core uses backend_features during init, clear it earlier to avoid using uninitialized memory. This is harmless since vhost scsi ignores the result anyway, but it avoids valgrind errors. Cc: qemu-sta...@nongnu.org Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/scsi/vhost-scsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index ddfe76a..7146e0e 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -238,6 +238,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) s-dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs-conf.num_queues; s-dev.vqs = g_new(struct vhost_virtqueue, s-dev.nvqs); s-dev.vq_index = 0; +s-dev.backend_features = 0; ret = vhost_dev_init(s-dev, (void *)(uintptr_t)vhostfd, VHOST_BACKEND_TYPE_KERNEL, true); @@ -246,7 +247,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) strerror(-ret)); return; } -s-dev.backend_features = 0; error_setg(s-migration_blocker, vhost-scsi does not support migration); Acked-by: Jason Wang jasow...@redhat.com
Re: [Qemu-devel] [PATCH] vhost_net: init acked_features to backend_features
On 09/03/2014 04:57 PM, Michael S. Tsirkin wrote: commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. As this field is now uninitialized, vhost initialization will sometimes fail. To fix, initialize field in core vhost code. As the next step, cleanup vhost scsi code as well. Reported-by: Jason Wang jasow...@redhat.com Reported-by: Andrey Korolyov and...@xdel.ru Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com Reviewed-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/virtio/vhost.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 5d7c40a..e42e51f 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -873,6 +873,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, hdev-memory_changed = false; memory_listener_register(hdev-memory_listener, address_space_memory); hdev-force = force; +/* Set minimal required set of features. */ +hdev-acked_features = hdev-backend_features; + return 0; fail_vq: while (--i = 0) { Since vhost_ack_features() never clear a bit, if rebooting from a guest w/ mrg rx buffer to a guest w/o it, network is broken?
Re: [Qemu-devel] [Qemu-stable] Patch Round-up for stable 2.1.1, freeze on 2014-09-03
OK so my patch fixes initialization likely by luck. with crash still in place. Hmm so something is still wrong with the userspace path. Could you please apply this debugging patch on top of all the stack that is now working for you, and see if assert still surfaces? diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 1fe18c7..a8f8826 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -314,7 +314,10 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, } for (i = 0; i total_queues; i++) { -r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); +if (i 0) +r = -11; +else +r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { goto err_start; Yes, with Jason`s patch on the top and this one both acceleration and re-initialization after reboot are broken, assert firing up again. Will check if vhost_net: cleanup recovery works as intended and reply to patch` thread.
Re: [Qemu-devel] [PATCH] vhost_net: cleanup recovery
On 09/03/2014 05:10 PM, Michael S. Tsirkin wrote: commit aad4dce934649b3a398396fc2a76f215bb194ea4 vhost_net: start/stop guest notifiers properly changed the order of calls for guest notifiers, but did not recover in the correct (reverse) order. Fix it up. Cc: qemu-sta...@nongnu.org Cc: Andrey Korolyov and...@xdel.ru Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/vhost_net.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..1fe18c7 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +err: +r = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (r 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, r); +fflush(stderr); +} We probably need a new label, since we only want to do this when guest notifiers has been set. return r; }
[Qemu-devel] [PATCH 0/5] cpu/acpi: convert cpu hot plug to hotplug_handler API
Gu Zheng (5): acpi/cpu: add cpu hotplug callback function to match hotplug_handler API acpi:ich9: convert cpu hotplug handle to hotplug_handler API acpi:piix4: convert cpu hotplug handle to hotplug_handler API pc: add cpu hotplug handler to PC_MACHINE cpu/hotplug: remove the left unused cpu hotplug notifier function hw/acpi/cpu_hotplug.c | 18 -- hw/acpi/ich9.c| 15 +++ hw/acpi/piix4.c | 16 +++- hw/i386/pc.c | 26 +- include/hw/acpi/cpu_hotplug.h |6 -- include/hw/acpi/ich9.h|1 - qom/cpu.c |1 - 7 files changed, 47 insertions(+), 36 deletions(-) -- 1.7.7
[Qemu-devel] [PATCH 1/5] acpi/cpu: add cpu hotplug callback function to match hotplug_handler API
Signed-off-by: Gu Zheng guz.f...@cn.fujitsu.com --- hw/acpi/cpu_hotplug.c | 17 + include/hw/acpi/cpu_hotplug.h |3 +++ 2 files changed, 20 insertions(+), 0 deletions(-) diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c index 2ad83a0..92c189b 100644 --- a/hw/acpi/cpu_hotplug.c +++ b/hw/acpi/cpu_hotplug.c @@ -36,6 +36,23 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = { }, }; +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, + AcpiCpuHotplug *g, DeviceState *dev) +{ +CPUState *cpu = CPU(dev); +CPUClass *k = CPU_GET_CLASS(cpu); +int64_t cpu_id; + +ar-gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS; +cpu_id = k-get_arch_id(cpu); +g_assert((cpu_id / 8) ACPI_GPE_PROC_LEN); +g-sts[cpu_id / 8] |= (1 (cpu_id % 8)); + +acpi_update_sci(ar, irq); + +cpu_resume(cpu); +} + void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu) { CPUClass *k = CPU_GET_CLASS(cpu); diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h index 9e5d30c..d025731 100644 --- a/include/hw/acpi/cpu_hotplug.h +++ b/include/hw/acpi/cpu_hotplug.h @@ -20,6 +20,9 @@ typedef struct AcpiCpuHotplug { uint8_t sts[ACPI_GPE_PROC_LEN]; } AcpiCpuHotplug; +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, + AcpiCpuHotplug *g, DeviceState *dev); + void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu); void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner, -- 1.7.7
[Qemu-devel] [PATCH 2/5] acpi:ich9: convert cpu hotplug handle to hotplug_handler API
Convert notifier based hotplug handle to hotplug_handler API. Signed-off-by: Gu Zheng guz.f...@cn.fujitsu.com --- hw/acpi/ich9.c | 13 ++--- include/hw/acpi/ich9.h |1 - 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 7b14bbb..89f97d7 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -209,15 +209,6 @@ static void pm_powerdown_req(Notifier *n, void *opaque) acpi_pm1_evt_power_down(pm-acpi_regs); } -static void ich9_cpu_added_req(Notifier *n, void *opaque) -{ -ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_added_notifier); - -assert(pm != NULL); -AcpiCpuHotplug_add(pm-acpi_regs.gpe, pm-gpe_cpu, CPU(opaque)); -acpi_update_sci(pm-acpi_regs, pm-irq); -} - void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, qemu_irq sci_irq) { @@ -246,8 +237,6 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, AcpiCpuHotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci), pm-gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE); -pm-cpu_added_notifier.notify = ich9_cpu_added_req; -qemu_register_cpu_added_notifier(pm-cpu_added_notifier); if (pm-acpi_memory_hotplug.is_enabled) { acpi_memory_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci), @@ -304,6 +293,8 @@ void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp) object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { acpi_memory_plug_cb(pm-acpi_regs, pm-irq, pm-acpi_memory_hotplug, dev, errp); +} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { +acpi_cpu_plug_cb(pm-acpi_regs, pm-irq, pm-gpe_cpu, dev); } else { error_setg(errp, acpi: device plug request for not supported device type: %s, object_get_typename(OBJECT(dev))); diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h index 7e42448..fe975e6 100644 --- a/include/hw/acpi/ich9.h +++ b/include/hw/acpi/ich9.h @@ -47,7 +47,6 @@ typedef struct ICH9LPCPMRegs { Notifier powerdown_notifier; AcpiCpuHotplug gpe_cpu; -Notifier cpu_added_notifier; MemHotplugState acpi_memory_hotplug; } ICH9LPCPMRegs; -- 1.7.7
[Qemu-devel] [PATCH 3/5] acpi:piix4: convert cpu hotplug handle to hotplug_handler API
Convert notifier based hotplug handle to hotplug_handler API. Signed-off-by: Gu Zheng guz.f...@cn.fujitsu.com --- hw/acpi/piix4.c | 14 ++ 1 files changed, 2 insertions(+), 12 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index b72b34e..6209385 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -83,7 +83,6 @@ typedef struct PIIX4PMState { uint8_t s4_val; AcpiCpuHotplug gpe_cpu; -Notifier cpu_added_notifier; MemHotplugState acpi_memory_hotplug; } PIIX4PMState; @@ -348,6 +347,8 @@ static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, errp); +} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { +acpi_cpu_plug_cb(s-ar, s-irq, s-gpe_cpu, dev); } else { error_setg(errp, acpi: device plug request for not supported device type: %s, object_get_typename(OBJECT(dev))); @@ -544,15 +545,6 @@ static const MemoryRegionOps piix4_gpe_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; -static void piix4_cpu_added_req(Notifier *n, void *opaque) -{ -PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_added_notifier); - -assert(s != NULL); -AcpiCpuHotplug_add(s-ar.gpe, s-gpe_cpu, CPU(opaque)); -acpi_update_sci(s-ar, s-irq); -} - static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, PCIBus *bus, PIIX4PMState *s) { @@ -565,8 +557,6 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, AcpiCpuHotplug_init(parent, OBJECT(s), s-gpe_cpu, PIIX4_CPU_HOTPLUG_IO_BASE); -s-cpu_added_notifier.notify = piix4_cpu_added_req; -qemu_register_cpu_added_notifier(s-cpu_added_notifier); if (s-acpi_memory_hotplug.is_enabled) { acpi_memory_hotplug_init(parent, OBJECT(s), s-acpi_memory_hotplug); -- 1.7.7
[Qemu-devel] [PATCH 4/5] pc: add cpu hotplug handler to PC_MACHINE
Add cpu hotplug handler to PC_MACHINE, which will perform the acpi cpu hotplug callback via hotplug_handler API. Signed-off-by: Gu Zheng guz.f...@cn.fujitsu.com --- hw/i386/pc.c | 26 +- qom/cpu.c|1 - 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 8fa8d2f..c2956f9 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1607,11 +1607,34 @@ out: error_propagate(errp, local_err); } +static void pc_cpu_plug(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ +HotplugHandlerClass *hhc; +Error *local_err = NULL; +PCMachineState *pcms = PC_MACHINE(hotplug_dev); + +if (dev-hotplugged) { +if (!pcms-acpi_dev) { +error_setg(local_err, + cpu hotplug is not enabled: missing acpi device); +goto out; +} + +hhc = HOTPLUG_HANDLER_GET_CLASS(pcms-acpi_dev); +hhc-plug(HOTPLUG_HANDLER(pcms-acpi_dev), dev, local_err); +out: +error_propagate(errp, local_err); +} +} + static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { pc_dimm_plug(hotplug_dev, dev, errp); +} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { +pc_cpu_plug(hotplug_dev, dev, errp); } } @@ -1620,7 +1643,8 @@ static HotplugHandler *pc_get_hotpug_handler(MachineState *machine, { PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine); -if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) { +if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) +|| object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { return HOTPLUG_HANDLER(machine); } diff --git a/qom/cpu.c b/qom/cpu.c index b32dd0a..af8e83f 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -304,7 +304,6 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp) if (dev-hotplugged) { cpu_synchronize_post_init(cpu); notifier_list_notify(cpu_added_notifiers, dev); -cpu_resume(cpu); } } -- 1.7.7
Re: [Qemu-devel] [PATCH 1/5] s390x/gdb: don't touch the cc if tcg is not enabled
On 02.09.14 09:07, Christian Borntraeger wrote: On 02/09/14 00:39, Alexander Graf wrote: On 29.08.14 15:52, Jens Freimann wrote: From: David Hildenbrand d...@linux.vnet.ibm.com When reading/writing the psw mask, the condition code may only be touched if running on tcg. Why? Shouldn't we be able to set CC from gdb as well? You can. What this patch does (and the patch description is a bit vague here) is to not modify the PSW it gets from KVM when passing it to gdb: The qemu core gets the PSW from KVM. Without this patch, we use cc_op to calculate the current CC of the PSW (No idea of TCG, I guess its evaluated lazy - at least this is how valgrind works). This is wrong for the KVM case, as cc_op does not contain any useful data for the KVM case. With this patch we simply pass the psw from KVM to gdb and back. The symptom was that the cc was always shown as zero. Ah, I see. I agree with the patch, but the patch description does not actually describe what the patch does. Please rework it. I also wouldn't mind if instead of hard coding this logic in the gdbstub, we'd extract it as helper functions to read and write the PSW.MASK in cpu.h. Alex
[Qemu-devel] [PATCH 5/5] cpu/hotplug: remove the left unused cpu hotplug notifier function
Remove the left unused cpu hotplug notifier function, and rename AcpiCpuHotplug_init -- acpi_cpu_hotplug_init AcpiCpuHotplug_ops -- acpi_cpu_hotplug_ops to match the coding style. Signed-off-by: Gu Zheng guz.f...@cn.fujitsu.com --- hw/acpi/cpu_hotplug.c | 17 +++-- hw/acpi/ich9.c|2 +- hw/acpi/piix4.c |2 +- include/hw/acpi/cpu_hotplug.h |3 +-- 4 files changed, 6 insertions(+), 18 deletions(-) diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c index 92c189b..494d22b 100644 --- a/hw/acpi/cpu_hotplug.c +++ b/hw/acpi/cpu_hotplug.c @@ -26,7 +26,7 @@ static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data, /* TODO: implement VCPU removal on guest signal that CPU can be removed */ } -static const MemoryRegionOps AcpiCpuHotplug_ops = { +static const MemoryRegionOps acpi_cpu_hotplug_ops = { .read = cpu_status_read, .write = cpu_status_write, .endianness = DEVICE_LITTLE_ENDIAN, @@ -53,18 +53,7 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, cpu_resume(cpu); } -void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu) -{ -CPUClass *k = CPU_GET_CLASS(cpu); -int64_t cpu_id; - -*gpe-sts = *gpe-sts | ACPI_CPU_HOTPLUG_STATUS; -cpu_id = k-get_arch_id(CPU(cpu)); -g_assert((cpu_id / 8) ACPI_GPE_PROC_LEN); -g-sts[cpu_id / 8] |= (1 (cpu_id % 8)); -} - -void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner, +void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner, AcpiCpuHotplug *gpe_cpu, uint16_t base) { CPUState *cpu; @@ -76,7 +65,7 @@ void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner, g_assert((id / 8) ACPI_GPE_PROC_LEN); gpe_cpu-sts[id / 8] |= (1 (id % 8)); } -memory_region_init_io(gpe_cpu-io, owner, AcpiCpuHotplug_ops, +memory_region_init_io(gpe_cpu-io, owner, acpi_cpu_hotplug_ops, gpe_cpu, acpi-cpu-hotplug, ACPI_GPE_PROC_LEN); memory_region_add_subregion(parent, base, gpe_cpu-io); } diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 89f97d7..adf5919 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -235,7 +235,7 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm, pm-powerdown_notifier.notify = pm_powerdown_req; qemu_register_powerdown_notifier(pm-powerdown_notifier); -AcpiCpuHotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci), +acpi_cpu_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci), pm-gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE); if (pm-acpi_memory_hotplug.is_enabled) { diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 6209385..6e91a92 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -555,7 +555,7 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, acpi_pcihp_init(s-acpi_pci_hotplug, bus, parent, s-use_acpi_pci_hotplug); -AcpiCpuHotplug_init(parent, OBJECT(s), s-gpe_cpu, +acpi_cpu_hotplug_init(parent, OBJECT(s), s-gpe_cpu, PIIX4_CPU_HOTPLUG_IO_BASE); if (s-acpi_memory_hotplug.is_enabled) { diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h index d025731..be2f516 100644 --- a/include/hw/acpi/cpu_hotplug.h +++ b/include/hw/acpi/cpu_hotplug.h @@ -23,8 +23,7 @@ typedef struct AcpiCpuHotplug { void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq, AcpiCpuHotplug *g, DeviceState *dev); -void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu); -void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner, +void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner, AcpiCpuHotplug *gpe_cpu, uint16_t base); #endif -- 1.7.7
Re: [Qemu-devel] [Qemu-stable] Patch Round-up for stable 2.1.1, freeze on 2014-09-03
On Wed, Sep 03, 2014 at 01:18:47PM +0400, Andrey Korolyov wrote: OK so my patch fixes initialization likely by luck. with crash still in place. Hmm so something is still wrong with the userspace path. Could you please apply this debugging patch on top of all the stack that is now working for you, and see if assert still surfaces? diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 1fe18c7..a8f8826 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -314,7 +314,10 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, } for (i = 0; i total_queues; i++) { -r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); +if (i 0) +r = -11; +else +r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { goto err_start; Yes, with Jason`s patch on the top and this one both acceleration and re-initialization after reboot are broken, assert firing up again. Will check if vhost_net: cleanup recovery works as intended and reply to patch` thread. Please test v2 though - Jason found a bug in v1. Thanks!
Re: [Qemu-devel] [PATCH] vhost_net: init acked_features to backend_features
On Wed, Sep 3, 2014 at 1:16 PM, Jason Wang jasow...@redhat.com wrote: On 09/03/2014 04:57 PM, Michael S. Tsirkin wrote: commit 2e6d46d77ed328d34a94688da8371bcbe243479b (vhost: add vhost_get_features and vhost_ack_features) removes the step that initializes the acked_features to backend_features. As this field is now uninitialized, vhost initialization will sometimes fail. To fix, initialize field in core vhost code. As the next step, cleanup vhost scsi code as well. Reported-by: Jason Wang jasow...@redhat.com Reported-by: Andrey Korolyov and...@xdel.ru Cc: Nikolay Nikolaev n.nikol...@virtualopensystems.com Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com Reviewed-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/virtio/vhost.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 5d7c40a..e42e51f 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -873,6 +873,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, hdev-memory_changed = false; memory_listener_register(hdev-memory_listener, address_space_memory); hdev-force = force; +/* Set minimal required set of features. */ +hdev-acked_features = hdev-backend_features; + return 0; fail_vq: while (--i = 0) { Since vhost_ack_features() never clear a bit, if rebooting from a guest w/ mrg rx buffer to a guest w/o it, network is broken? Networking is broken just after start, and buffer memory getting corrupted at reset: qemu-system-x86_64: /tmp/buildd/qemu-2.1.0+f1/memory.c:1614: memory_region_del_eventfd: Assertion `i != mr-ioeventfd_nb' failed.
Re: [Qemu-devel] [PATCH] vhost_net: cleanup recovery
On Wed, Sep 3, 2014 at 1:20 PM, Jason Wang jasow...@redhat.com wrote: On 09/03/2014 05:10 PM, Michael S. Tsirkin wrote: commit aad4dce934649b3a398396fc2a76f215bb194ea4 vhost_net: start/stop guest notifiers properly changed the order of calls for guest notifiers, but did not recover in the correct (reverse) order. Fix it up. Cc: qemu-sta...@nongnu.org Cc: Andrey Korolyov and...@xdel.ru Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/vhost_net.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..1fe18c7 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +err: +r = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (r 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, r); +fflush(stderr); +} We probably need a new label, since we only want to do this when guest notifiers has been set. return r; } Erm, no improvements for me from V1 - brand-new assert at reset and lack of connectivity.
Re: [Qemu-devel] [PATCH 5/5] gdb: provide the name of the architecture in the target.xml
[ccing Andreas in case he wants to review the QOM aspects of this, though they're fairly straightforward I think.] On 29 August 2014 14:52, Jens Freimann jf...@linux.vnet.ibm.com wrote: From: David Hildenbrand d...@linux.vnet.ibm.com This patch provides the name of the architecture in the target.xml if available. This allows the remote gdb to detect the target architecture on its own - so there is no need to specify it manually (e.g. if gdb is started without a binary) using set arch *arch_name*. This is neat; I didn't realise gdb let you do this. The name of the architecture has been added to all archs that provide a target.xml (by supplying a gdb_core_xml_file) and have a unique architecture name in gdb's feature xml files. What about 32-bit ARM? You set the architecture name for AArch64 but not the 32 bit case. Well, my point was to not break anything :) On my way through the possible architecture names (binutils-gdb/gdb/features/*.xml), I wasn't able to come up with the right name for arm 32 bit (arm-core.xml) - they don't specify any. This patch therefore adapts to the xml files from gdb. The architecture should be known at the same point when specifying the xml file. So if anyone can come up with the proper arm name in the future (or even some kind of detection algorithm), it can simply be set in target-arm/cpu.c (after arm-core.xml). David
[Qemu-devel] [PATCH v2] vhost_net: cleanup recovery
commit aad4dce934649b3a398396fc2a76f215bb194ea4 vhost_net: start/stop guest notifiers properly changed the order of calls for guest notifiers, but did not recover in the correct (reverse) order. Fix it up. Cc: qemu-sta...@nongnu.org Cc: Andrey Korolyov and...@xdel.ru Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/vhost_net.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..ddebd04 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +r = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (r 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, r); +fflush(stderr); +} +err: return r; } -- MST
Re: [Qemu-devel] [PATCH 5/5] gdb: provide the name of the architecture in the target.xml
On Wed, Sep 03, 2014 at 11:37:24AM +0200, David Hildenbrand wrote: [ccing Andreas in case he wants to review the QOM aspects of this, though they're fairly straightforward I think.] On 29 August 2014 14:52, Jens Freimann jf...@linux.vnet.ibm.com wrote: From: David Hildenbrand d...@linux.vnet.ibm.com This patch provides the name of the architecture in the target.xml if available. This allows the remote gdb to detect the target architecture on its own - so there is no need to specify it manually (e.g. if gdb is started without a binary) using set arch *arch_name*. This is neat; I didn't realise gdb let you do this. The name of the architecture has been added to all archs that provide a target.xml (by supplying a gdb_core_xml_file) and have a unique architecture name in gdb's feature xml files. What about 32-bit ARM? You set the architecture name for AArch64 but not the 32 bit case. Well, my point was to not break anything :) On my way through the possible architecture names (binutils-gdb/gdb/features/*.xml), I wasn't able to come up with the right name for arm 32 bit (arm-core.xml) - they don't specify any. This patch therefore adapts to the xml files from gdb. The architecture should be known at the same point when specifying the xml file. So if anyone can come up with the proper arm name in the future (or even some kind of detection algorithm), it can simply be set in target-arm/cpu.c (after arm-core.xml). Hi, I've got some similar patches in my tree. I used the following: commit 26932a453da466d111b67c37b93dec71fb3ae111 Author: Edgar E. Iglesias edgar.igles...@xilinx.com Date: Wed Aug 20 19:22:10 2014 +1000 gdbstub: Emit the CPUs GDB architecture if available Allows GDB to autodetect the architecture. Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com diff --git a/gdbstub.c b/gdbstub.c index 7f82186..5b62c50 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -604,6 +604,11 @@ static const char *get_feature_xml(const char *p, const char **newp, pstrcat(target_xml, sizeof(target_xml), r-xml); pstrcat(target_xml, sizeof(target_xml), \/); } +if (cc-gdb_arch) { +pstrcat(target_xml, sizeof(target_xml), architecture); +pstrcat(target_xml, sizeof(target_xml), cc-gdb_arch); +pstrcat(target_xml, sizeof(target_xml), /architecture); +} pstrcat(target_xml, sizeof(target_xml), /target); } return target_xml; commit a0b166c59491b154c05b963649f561c1e48aa706 Author: Edgar E. Iglesias edgar.igles...@xilinx.com Date: Wed Aug 20 19:21:28 2014 +1000 target-arm: Provide GDB arch names Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com diff --git a/target-arm/cpu.c b/target-arm/cpu.c index b726b6a..d1704f5 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -1163,6 +1163,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data) cc-get_phys_page_debug = arm_cpu_get_phys_page_debug; cc-vmsd = vmstate_arm_cpu; #endif +cc-gdb_arch = arm; cc-gdb_num_core_regs = 26; cc-gdb_core_xml_file = arm-core.xml; } diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c index fc9c991..c099d13 100644 --- a/target-arm/cpu64.c +++ b/target-arm/cpu64.c @@ -215,6 +215,7 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data) cc-gdb_write_register = aarch64_cpu_gdb_write_register; cc-gdb_num_core_regs = 34; cc-gdb_core_xml_file = aarch64-core.xml; +cc-gdb_arch = aarch64; } static void aarch64_cpu_register(const ARMCPUInfo *info) commit 0aca15f7829e6e8a94639ddd75bcfdfd3b034d2e Author: Edgar E. Iglesias edgar.igles...@xilinx.com Date: Wed Aug 20 19:20:35 2014 +1000 qom/cpu: Add a gdb_arch field to the CPUClass Used to optionally set the GDB architecture of the CPU. Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 4d0908b..46b72e7 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -147,6 +147,7 @@ typedef struct CPUClass { void *opaque); const struct VMStateDescription *vmsd; +const char *gdb_arch; int gdb_num_core_regs; const char *gdb_core_xml_file; } CPUClass; commit eaa8bac08e300c9516d04c3425c3794a1bd893b8 Author: Edgar E. Iglesias edgar.igles...@xilinx.com Date: Wed Aug 20 11:26:52 2014 +1000 configure: gdbstub: Include ARM xml descritions in AArch64 builds Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com diff --git a/configure b/configure index fce5801..3a95948 100755 --- a/configure +++ b/configure @@ -4976,7 +4976,9 @@ case $target_name in aarch64) TARGET_BASE_ARCH=arm bflt=yes -gdb_xml_files=aarch64-core.xml aarch64-fpu.xml +arm_gdb_xml_files=arm-core.xml arm-vfp.xml arm-vfp3.xml
[Qemu-devel] [PATCH] qtest: fix qtest log fd should be initialized before qtest chardev
From: Li Liu john.li...@huawei.com qtest_log_fp should be inited before qemu_chr_add_handlers. If not the log dumped from callback functions may be lost. easy to reproduce it by command: QTEST_LOG=1 QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 gtester -k --verbose -m=quick tests/qdev-monitor-test The log [I xx] OPENED should be printed out by qtest_event, but does not. Signed-off-by: Li Liu john.li...@huawei.com --- qtest.c |9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/qtest.c b/qtest.c index ef0d991..2a33d98 100644 --- a/qtest.c +++ b/qtest.c @@ -537,11 +537,6 @@ void qtest_init(const char *qtest_chrdev, const char *qtest_log, Error **errp) return; } -qemu_chr_add_handlers(chr, qtest_can_read, qtest_read, qtest_event, chr); -qemu_chr_fe_set_echo(chr, true); - -inbuf = g_string_new(); - if (qtest_log) { if (strcmp(qtest_log, none) != 0) { qtest_log_fp = fopen(qtest_log, w+); @@ -550,6 +545,10 @@ void qtest_init(const char *qtest_chrdev, const char *qtest_log, Error **errp) qtest_log_fp = stderr; } +qemu_chr_add_handlers(chr, qtest_can_read, qtest_read, qtest_event, chr); +qemu_chr_fe_set_echo(chr, true); + +inbuf = g_string_new(); qtest_chr = chr; } -- 1.7.9.5
[Qemu-devel] [PATCH v2 0/2] actual checks of KVM_CAP_IRQFD and KVM_CAP_IRQFD_RESAMPLE
This patch serie replaces direct settings of kvm_irqfds_allowed by actual checks of the KVM_CAP_IRQFD extension. Also A new kvm_resamplefds_enabled() enables to check KVM_CAP_IRQFD_RESAMPLE. in the second patch file the vfio device is the first user of kvm_resamplefds_enabled(). Eric Auger (2): KVM_CAP_IRQFD and KVM_CAP_IRQFD_RESAMPLE checks vfio: use kvm_resamplefds_enabled() hw/intc/openpic_kvm.c | 1 - hw/intc/xics_kvm.c| 1 - hw/misc/vfio.c| 5 ++--- include/sysemu/kvm.h | 10 ++ kvm-all.c | 7 +++ target-i386/kvm.c | 1 - target-s390x/kvm.c| 1 - 7 files changed, 19 insertions(+), 7 deletions(-) -- 1.8.3.2
[Qemu-devel] [PATCH v2 1/2] KVM_CAP_IRQFD and KVM_CAP_IRQFD_RESAMPLE checks
Compute kvm_irqfds_allowed by checking the KVM_CAP_IRQFD extension. Remove direct settings in architecture specific files. Add a new kvm_resamplefds_allowed variable, initialized by checking the KVM_CAP_IRQFD_RESAMPLE extension. Add a corresponding kvm_resamplefds_enabled() function. Signed-off-by: Eric Auger eric.au...@linaro.org --- in practice KVM_CAP_IRQFD_RESAMPLE seems to be always enabled as soon as kernel has HAVE_KVM_IRQFD so the resamplefd check may be unnecessary. --- hw/intc/openpic_kvm.c | 1 - hw/intc/xics_kvm.c| 1 - include/sysemu/kvm.h | 10 ++ kvm-all.c | 7 +++ target-i386/kvm.c | 1 - target-s390x/kvm.c| 1 - 6 files changed, 17 insertions(+), 4 deletions(-) diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c index e3bce04..6cef3b1 100644 --- a/hw/intc/openpic_kvm.c +++ b/hw/intc/openpic_kvm.c @@ -229,7 +229,6 @@ static void kvm_openpic_realize(DeviceState *dev, Error **errp) kvm_irqchip_add_irq_route(kvm_state, i, 0, i); } -kvm_irqfds_allowed = true; kvm_msi_via_irqfd_allowed = true; kvm_gsi_routing_allowed = true; diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c index 20b19e9..c15453f 100644 --- a/hw/intc/xics_kvm.c +++ b/hw/intc/xics_kvm.c @@ -448,7 +448,6 @@ static void xics_kvm_realize(DeviceState *dev, Error **errp) } kvm_kernel_irqchip = true; -kvm_irqfds_allowed = true; kvm_msi_via_irqfd_allowed = true; kvm_gsi_direct_mapping = true; diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 174ea36..69c4d0f 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -45,6 +45,7 @@ extern bool kvm_async_interrupts_allowed; extern bool kvm_halt_in_kernel_allowed; extern bool kvm_eventfds_allowed; extern bool kvm_irqfds_allowed; +extern bool kvm_resamplefds_allowed; extern bool kvm_msi_via_irqfd_allowed; extern bool kvm_gsi_routing_allowed; extern bool kvm_gsi_direct_mapping; @@ -102,6 +103,15 @@ extern bool kvm_readonly_mem_allowed; #define kvm_irqfds_enabled() (kvm_irqfds_allowed) /** + * kvm_resamplefds_enabled: + * + * Returns: true if we can use resamplefds to inject interrupts into + * a KVM CPU (ie the kernel supports resamplefds and we are running + * with a configuration where it is meaningful to use them). + */ +#define kvm_resamplefds_enabled() (kvm_resamplefds_allowed) + +/** * kvm_msi_via_irqfd_enabled: * * Returns: true if we can route a PCI MSI (Message Signaled Interrupt) diff --git a/kvm-all.c b/kvm-all.c index b240bf8..d635942 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -116,6 +116,7 @@ bool kvm_async_interrupts_allowed; bool kvm_halt_in_kernel_allowed; bool kvm_eventfds_allowed; bool kvm_irqfds_allowed; +bool kvm_resamplefds_allowed; bool kvm_msi_via_irqfd_allowed; bool kvm_gsi_routing_allowed; bool kvm_gsi_direct_mapping; @@ -1548,6 +1549,12 @@ int kvm_init(MachineClass *mc) kvm_eventfds_allowed = (kvm_check_extension(s, KVM_CAP_IOEVENTFD) 0); +kvm_irqfds_allowed = +(kvm_check_extension(s, KVM_CAP_IRQFD) 0); + +kvm_resamplefds_allowed = +(kvm_check_extension(s, KVM_CAP_IRQFD_RESAMPLE) 0); + ret = kvm_arch_init(s); if (ret 0) { goto err; diff --git a/target-i386/kvm.c b/target-i386/kvm.c index ddedc73..2320920 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -2544,7 +2544,6 @@ void kvm_arch_init_irq_routing(KVMState *s) * irqchip, so we can use irqfds, and on x86 we know * we can use msi via irqfd and GSI routing. */ -kvm_irqfds_allowed = true; kvm_msi_via_irqfd_allowed = true; kvm_gsi_routing_allowed = true; } diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index a85a480..f937568 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -1290,7 +1290,6 @@ void kvm_arch_init_irq_routing(KVMState *s) * have to override the common code kvm_halt_in_kernel_allowed setting. */ if (kvm_check_extension(s, KVM_CAP_IRQ_ROUTING)) { -kvm_irqfds_allowed = true; kvm_gsi_routing_allowed = true; kvm_halt_in_kernel_allowed = false; } -- 1.8.3.2
[Qemu-devel] [PATCH v2 2/2] vfio: use kvm_resamplefds_enabled()
Use the kvm_resamplefds_enabled function Signed-off-by: Eric Auger eric.au...@linaro.org --- hw/misc/vfio.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c index 40dcaa6..24f6a3a 100644 --- a/hw/misc/vfio.c +++ b/hw/misc/vfio.c @@ -406,7 +406,7 @@ static void vfio_enable_intx_kvm(VFIODevice *vdev) if (!VFIO_ALLOW_KVM_INTX || !kvm_irqfds_enabled() || vdev-intx.route.mode != PCI_INTX_ENABLED || -!kvm_check_extension(kvm_state, KVM_CAP_IRQFD_RESAMPLE)) { +!kvm_resamplefds_enabled()) { return; } @@ -568,8 +568,7 @@ static int vfio_enable_intx(VFIODevice *vdev) * Only conditional to avoid generating error messages on platforms * where we won't actually use the result anyway. */ -if (kvm_irqfds_enabled() -kvm_check_extension(kvm_state, KVM_CAP_IRQFD_RESAMPLE)) { +if (kvm_irqfds_enabled() kvm_resamplefds_enabled()) { vdev-intx.route = pci_device_route_intx_to_irq(vdev-pdev, vdev-intx.pin); } -- 1.8.3.2
[Qemu-devel] [PATCH v3] dump: let dump_error return error info to caller
The second parameter of dump_error is unused, but one purpose of using this function is to report the error info. Use error_set to return the error info to the caller. Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com --- V3: - Drop the '\n' in the message when call dump_error(comment of Eric Blake) V2: - Return the error reason to the caller which suggested by Luiz Capitulino. --- dump.c | 165 - 1 file changed, 82 insertions(+), 83 deletions(-) diff --git a/dump.c b/dump.c index 71d3e94..a08a711 100644 --- a/dump.c +++ b/dump.c @@ -81,9 +81,10 @@ static int dump_cleanup(DumpState *s) return 0; } -static void dump_error(DumpState *s, const char *reason) +static void dump_error(DumpState *s, Error **errp, const char *reason) { dump_cleanup(s); +error_setg(errp, %s, reason); } static int fd_write_vmcore(const void *buf, size_t size, void *opaque) @@ -99,7 +100,7 @@ static int fd_write_vmcore(const void *buf, size_t size, void *opaque) return 0; } -static int write_elf64_header(DumpState *s) +static int write_elf64_header(DumpState *s, Error **errp) { Elf64_Ehdr elf_header; int ret; @@ -126,14 +127,14 @@ static int write_elf64_header(DumpState *s) ret = fd_write_vmcore(elf_header, sizeof(elf_header), s); if (ret 0) { -dump_error(s, dump: failed to write elf header.\n); +dump_error(s, errp, dump: failed to write elf header.); return -1; } return 0; } -static int write_elf32_header(DumpState *s) +static int write_elf32_header(DumpState *s, Error **errp) { Elf32_Ehdr elf_header; int ret; @@ -160,7 +161,7 @@ static int write_elf32_header(DumpState *s) ret = fd_write_vmcore(elf_header, sizeof(elf_header), s); if (ret 0) { -dump_error(s, dump: failed to write elf header.\n); +dump_error(s, errp, dump: failed to write elf header.); return -1; } @@ -169,7 +170,7 @@ static int write_elf32_header(DumpState *s) static int write_elf64_load(DumpState *s, MemoryMapping *memory_mapping, int phdr_index, hwaddr offset, -hwaddr filesz) +hwaddr filesz, Error **errp) { Elf64_Phdr phdr; int ret; @@ -186,7 +187,7 @@ static int write_elf64_load(DumpState *s, MemoryMapping *memory_mapping, ret = fd_write_vmcore(phdr, sizeof(Elf64_Phdr), s); if (ret 0) { -dump_error(s, dump: failed to write program header table.\n); +dump_error(s, errp, dump: failed to write program header table.); return -1; } @@ -195,7 +196,7 @@ static int write_elf64_load(DumpState *s, MemoryMapping *memory_mapping, static int write_elf32_load(DumpState *s, MemoryMapping *memory_mapping, int phdr_index, hwaddr offset, -hwaddr filesz) +hwaddr filesz, Error **errp) { Elf32_Phdr phdr; int ret; @@ -212,14 +213,14 @@ static int write_elf32_load(DumpState *s, MemoryMapping *memory_mapping, ret = fd_write_vmcore(phdr, sizeof(Elf32_Phdr), s); if (ret 0) { -dump_error(s, dump: failed to write program header table.\n); +dump_error(s, errp, dump: failed to write program header table.); return -1; } return 0; } -static int write_elf64_note(DumpState *s) +static int write_elf64_note(DumpState *s, Error **errp) { Elf64_Phdr phdr; hwaddr begin = s-memory_offset - s-note_size; @@ -235,7 +236,7 @@ static int write_elf64_note(DumpState *s) ret = fd_write_vmcore(phdr, sizeof(Elf64_Phdr), s); if (ret 0) { -dump_error(s, dump: failed to write program header table.\n); +dump_error(s, errp, dump: failed to write program header table.); return -1; } @@ -247,7 +248,8 @@ static inline int cpu_index(CPUState *cpu) return cpu-cpu_index + 1; } -static int write_elf64_notes(WriteCoreDumpFunction f, DumpState *s) +static int write_elf64_notes(WriteCoreDumpFunction f, DumpState *s, + Error **errp) { CPUState *cpu; int ret; @@ -257,7 +259,7 @@ static int write_elf64_notes(WriteCoreDumpFunction f, DumpState *s) id = cpu_index(cpu); ret = cpu_write_elf64_note(f, cpu, id, s); if (ret 0) { -dump_error(s, dump: failed to write elf notes.\n); +dump_error(s, errp, dump: failed to write elf notes.); return -1; } } @@ -265,7 +267,7 @@ static int write_elf64_notes(WriteCoreDumpFunction f, DumpState *s) CPU_FOREACH(cpu) { ret = cpu_write_elf64_qemunote(f, cpu, s); if (ret 0) { -dump_error(s, dump: failed to write CPU status.\n); +dump_error(s, errp, dump: failed to write CPU status.); return -1; } } @@ -273,7 +275,7 @@ static int
Re: [Qemu-devel] [PATCH 5/5] gdb: provide the name of the architecture in the target.xml
On Wed, Sep 03, 2014 at 11:37:24AM +0200, David Hildenbrand wrote: [ccing Andreas in case he wants to review the QOM aspects of this, though they're fairly straightforward I think.] On 29 August 2014 14:52, Jens Freimann jf...@linux.vnet.ibm.com wrote: From: David Hildenbrand d...@linux.vnet.ibm.com This patch provides the name of the architecture in the target.xml if available. This allows the remote gdb to detect the target architecture on its own - so there is no need to specify it manually (e.g. if gdb is started without a binary) using set arch *arch_name*. This is neat; I didn't realise gdb let you do this. The name of the architecture has been added to all archs that provide a target.xml (by supplying a gdb_core_xml_file) and have a unique architecture name in gdb's feature xml files. What about 32-bit ARM? You set the architecture name for AArch64 but not the 32 bit case. Well, my point was to not break anything :) On my way through the possible architecture names (binutils-gdb/gdb/features/*.xml), I wasn't able to come up with the right name for arm 32 bit (arm-core.xml) - they don't specify any. This patch therefore adapts to the xml files from gdb. The architecture should be known at the same point when specifying the xml file. So if anyone can come up with the proper arm name in the future (or even some kind of detection algorithm), it can simply be set in target-arm/cpu.c (after arm-core.xml). Hi, I've got some similar patches in my tree. I used the following: Thanks! So arm seems to be the proper name for arm32, right? commit 26932a453da466d111b67c37b93dec71fb3ae111 Author: Edgar E. Iglesias edgar.igles...@xilinx.com Date: Wed Aug 20 19:22:10 2014 +1000 gdbstub: Emit the CPUs GDB architecture if available Allows GDB to autodetect the architecture. Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com diff --git a/gdbstub.c b/gdbstub.c index 7f82186..5b62c50 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -604,6 +604,11 @@ static const char *get_feature_xml(const char *p, const char **newp, pstrcat(target_xml, sizeof(target_xml), r-xml); pstrcat(target_xml, sizeof(target_xml), \/); } +if (cc-gdb_arch) { +pstrcat(target_xml, sizeof(target_xml), architecture); +pstrcat(target_xml, sizeof(target_xml), cc-gdb_arch); +pstrcat(target_xml, sizeof(target_xml), /architecture); +} Please not that gdb-target.dtd specifies the architecture to come directly at the beginning of the target section. Putting it after the xml-includes, to the end of the target section makes the whole XML failing to be recognized on my tests with s390x. David pstrcat(target_xml, sizeof(target_xml), /target); } return target_xml;
Re: [Qemu-devel] [PATCH 5/5] gdb: provide the name of the architecture in the target.xml
On Wed, Sep 03, 2014 at 11:59:45AM +0200, David Hildenbrand wrote: On Wed, Sep 03, 2014 at 11:37:24AM +0200, David Hildenbrand wrote: [ccing Andreas in case he wants to review the QOM aspects of this, though they're fairly straightforward I think.] On 29 August 2014 14:52, Jens Freimann jf...@linux.vnet.ibm.com wrote: From: David Hildenbrand d...@linux.vnet.ibm.com This patch provides the name of the architecture in the target.xml if available. This allows the remote gdb to detect the target architecture on its own - so there is no need to specify it manually (e.g. if gdb is started without a binary) using set arch *arch_name*. This is neat; I didn't realise gdb let you do this. The name of the architecture has been added to all archs that provide a target.xml (by supplying a gdb_core_xml_file) and have a unique architecture name in gdb's feature xml files. What about 32-bit ARM? You set the architecture name for AArch64 but not the 32 bit case. Well, my point was to not break anything :) On my way through the possible architecture names (binutils-gdb/gdb/features/*.xml), I wasn't able to come up with the right name for arm 32 bit (arm-core.xml) - they don't specify any. This patch therefore adapts to the xml files from gdb. The architecture should be known at the same point when specifying the xml file. So if anyone can come up with the proper arm name in the future (or even some kind of detection algorithm), it can simply be set in target-arm/cpu.c (after arm-core.xml). Hi, I've got some similar patches in my tree. I used the following: Thanks! So arm seems to be the proper name for arm32, right? commit 26932a453da466d111b67c37b93dec71fb3ae111 Author: Edgar E. Iglesias edgar.igles...@xilinx.com Date: Wed Aug 20 19:22:10 2014 +1000 gdbstub: Emit the CPUs GDB architecture if available Allows GDB to autodetect the architecture. Signed-off-by: Edgar E. Iglesias edgar.igles...@xilinx.com diff --git a/gdbstub.c b/gdbstub.c index 7f82186..5b62c50 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -604,6 +604,11 @@ static const char *get_feature_xml(const char *p, const char **newp, pstrcat(target_xml, sizeof(target_xml), r-xml); pstrcat(target_xml, sizeof(target_xml), \/); } +if (cc-gdb_arch) { +pstrcat(target_xml, sizeof(target_xml), architecture); +pstrcat(target_xml, sizeof(target_xml), cc-gdb_arch); +pstrcat(target_xml, sizeof(target_xml), /architecture); +} Please not that gdb-target.dtd specifies the architecture to come directly at the beginning of the target section. Putting it after the xml-includes, to the end of the target section makes the whole XML failing to be recognized on my tests with s390x. Hmm, interesting. It worked here with a multi-arch gdb. Cheers, Edgar David pstrcat(target_xml, sizeof(target_xml), /target); } return target_xml;
Re: [Qemu-devel] [libvirt] [PATCH V2] fix: unix sockets created for virtio-serail has insufficient permissions
On 09/03/2014 12:18 AM, Chunyan Liu wrote: s/serail/serial/ in the subject Actually, I'd go with a broader subject line: qemu: ensure sane umask for qemu process Add umask to _virCommand, allow user to set umask to command. Set umask(002) to qemu process to overwrite default umask(022) so that unix sockets created for virtio-serial has expected permissions. -- 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 v5] block: Introduce null drivers
On Wed, 09/03 12:10, Kevin Wolf wrote: Am 01.09.2014 um 11:22 hat Fam Zheng geschrieben: This is an analogue to Linux null_blk. It can be used for testing or benchmarking block device emulation and general block layer functionalities such as coroutines and throttling, where disk IO is not necessary or wanted. Use null-aio:// for AIO version, and null-co:// for coroutine version. Signed-off-by: Fam Zheng f...@redhat.com Reviewed-by: BenoƮt Canet benoit.ca...@nodalink.com +static BlockDriver bdrv_null_co = { +.format_name= null-co, +.protocol_name = null-co, +.instance_size = sizeof(BDRVNullState), + +.bdrv_file_open = null_file_open, +.bdrv_close = null_close, +.bdrv_getlength = null_getlength, + +.bdrv_read = null_co_read, +.bdrv_write = null_co_write, Any reason not to use the native .bdrv_co_readv/writev interfaces instead of the old, emulated versions that involve a bounce buffer? No good reason, I'll change it in the next version. Thanks, Fam
[Qemu-devel] [PATCH v3] vhost_net: cleanup recovery
commit aad4dce934649b3a398396fc2a76f215bb194ea4 vhost_net: start/stop guest notifiers properly changed the order of calls for guest notifiers, but did not recover in the correct (reverse) order. Fix it up. Cc: qemu-sta...@nongnu.org Cc: Andrey Korolyov and...@xdel.ru Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/vhost_net.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..3819044 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, e, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +e = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (e 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, e); +fflush(stderr); +} +err: return r; } -- MST
Re: [Qemu-devel] [PATCH] vhost_net: cleanup recovery
On Wed, Sep 03, 2014 at 01:37:33PM +0400, Andrey Korolyov wrote: On Wed, Sep 3, 2014 at 1:20 PM, Jason Wang jasow...@redhat.com wrote: On 09/03/2014 05:10 PM, Michael S. Tsirkin wrote: commit aad4dce934649b3a398396fc2a76f215bb194ea4 vhost_net: start/stop guest notifiers properly changed the order of calls for guest notifiers, but did not recover in the correct (reverse) order. Fix it up. Cc: qemu-sta...@nongnu.org Cc: Andrey Korolyov and...@xdel.ru Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/vhost_net.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..1fe18c7 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +err: +r = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (r 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, r); +fflush(stderr); +} We probably need a new label, since we only want to do this when guest notifiers has been set. return r; } Erm, no improvements for me from V1 - brand-new assert at reset and lack of connectivity. Heh, I see what's wrong. Let me try again. V3 sent. -- MST
Re: [Qemu-devel] [PATCH 09/12] spapr_pci: enable basic hotplug operations
On Tue, Aug 19, 2014 at 5:51 AM, Michael Roth mdr...@linux.vnet.ibm.com wrote: This enables hotplug for PHB bridges. Upon hotplug we generate the OF-nodes required by PAPR specification and IEEE 1275-1994 PCI Bus Binding to Open Firmware for the device. We associate the corresponding FDT for these nodes with the DrcEntry corresponding to the slot, which will be fetched via ibm,configure-connector RTAS calls by the guest as described by PAPR specification. The FDT is cleaned up in the case of unplug. Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com --- hw/ppc/spapr_pci.c | 235 +++-- include/hw/ppc/spapr.h | 1 + 2 files changed, 228 insertions(+), 8 deletions(-) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 96a57be..23864ab 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -87,6 +87,17 @@ #define ENCODE_DRC_STATE(val, m, s) \ (((uint32_t)(val) (s)) (uint32_t)(m)) +#define FDT_MAX_SIZE0x1 +#define _FDT(exp) \ +do { \ +int ret = (exp); \ +if (ret 0) { \ +return ret;\ +} \ +} while (0) + +static void spapr_drc_state_reset(sPAPRDrcEntry *drc_entry); + static sPAPRPHBState *find_phb(sPAPREnvironment *spapr, uint64_t buid) { sPAPRPHBState *sphb; @@ -476,6 +487,22 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr, /* encode the new value into the correct bit field */ shift = INDICATOR_ISOLATION_SHIFT; mask = INDICATOR_ISOLATION_MASK; +if (drc_entry) { +/* transition from unisolated to isolated for a hotplug slot + * entails completion of guest-side device unplug/cleanup, so + * we can now safely remove the device if qemu is waiting for + * it to be released + */ +if (DECODE_DRC_STATE(*pind, mask, shift) != indicator_state) { +if (indicator_state == 0 drc_entry-awaiting_release) { +/* device_del has been called and host is waiting + * for guest to release/isolate device, go ahead + * and remove it now + */ +spapr_drc_state_reset(drc_entry); +} +} +} break; case 9002: /* DR */ shift = INDICATOR_DR_SHIFT; @@ -816,6 +843,198 @@ static AddressSpace *spapr_pci_dma_iommu(PCIBus *bus, void *opaque, int devfn) return phb-iommu_as; } +static int spapr_populate_pci_child_dt(PCIDevice *dev, void *fdt, int offset, + int phb_index) +{ +int slot = PCI_SLOT(dev-devfn); +char slotname[16]; +bool is_bridge = 1; +sPAPRDrcEntry *drc_entry, *drc_entry_slot; +uint32_t reg[RESOURCE_CELLS_TOTAL * 8] = { 0 }; +uint32_t assigned[RESOURCE_CELLS_TOTAL * 8] = { 0 }; +int reg_size, assigned_size; + +drc_entry = spapr_phb_to_drc_entry(phb_index + SPAPR_PCI_BASE_BUID); +g_assert(drc_entry); +drc_entry_slot = drc_entry-child_entries[slot]; + +if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) == +PCI_HEADER_TYPE_NORMAL) { +is_bridge = 0; +} + +_FDT(fdt_setprop_cell(fdt, offset, vendor-id, + pci_default_read_config(dev, PCI_VENDOR_ID, 2))); +_FDT(fdt_setprop_cell(fdt, offset, device-id, + pci_default_read_config(dev, PCI_DEVICE_ID, 2))); +_FDT(fdt_setprop_cell(fdt, offset, revision-id, + pci_default_read_config(dev, PCI_REVISION_ID, 1))); +_FDT(fdt_setprop_cell(fdt, offset, class-code, + pci_default_read_config(dev, PCI_CLASS_DEVICE, 2) 8)); + +_FDT(fdt_setprop_cell(fdt, offset, interrupts, + pci_default_read_config(dev, PCI_INTERRUPT_PIN, 1))); + +/* if this device is NOT a bridge */ +if (!is_bridge) { +_FDT(fdt_setprop_cell(fdt, offset, min-grant, +pci_default_read_config(dev, PCI_MIN_GNT, 1))); +_FDT(fdt_setprop_cell(fdt, offset, max-latency, +pci_default_read_config(dev, PCI_MAX_LAT, 1))); +_FDT(fdt_setprop_cell(fdt, offset, subsystem-id, +pci_default_read_config(dev, PCI_SUBSYSTEM_ID, 2))); +_FDT(fdt_setprop_cell(fdt, offset, subsystem-vendor-id, +pci_default_read_config(dev, PCI_SUBSYSTEM_VENDOR_ID, 2))); +} + +_FDT(fdt_setprop_cell(fdt, offset, cache-line-size, +pci_default_read_config(dev, PCI_CACHE_LINE_SIZE, 1))); + +/* the following fdt cells are masked off the pci status register */ +int pci_status =
Re: [Qemu-devel] Query regarding bdrv_co_discard
On Tue, Sep 02, 2014 at 02:09:44AM -0700, Sanjay Kumar2 wrote: Please let me know how to test bdrv_co_discard interface. I mean what steps I need to do on KVM host and inside guest to do end to end testing of this interface. I have implemented a network protocol based block driver and defined bdrv_co_discard interface as well which free the blocks on the remote storage device. Use a virtio-scsi storage controller and then the guest can send SCSI discard commands. Make sure to use the -drive ...,discard=unmap option, otherwise discard commands are silently ignored. Stefan pgpnsj0KwnVvS.pgp Description: PGP signature
[Qemu-devel] Serial: possible hang during intensive interaction over the console
Given 2.1 and isa-serial output, set as ttyS0 for the guest VM with 9600 baud rate. The test case is quite simple - display as much data as possible over serial console and do not hang the system. While qemu-1.1 works perfectly, with complaining for lost interrupts (known bug for used guest kernel), 2.1 just hangs after some seconds, eating up all available cpu quota. Test case is 'while true; dmesg; done' in serial console. I`d like to ask to consider this bug as very serious as VM going completely unresponsive in matter of tens of seconds and there are a lot of side attacks to produce enough number of printk() to the ttyS0 with serial console being set up and default settings for almost any distro in such a way that message suppression would not work and VM can be DoSed by an unprivileged user.
Re: [Qemu-devel] [PATCH] block/archipelago: Use QEMU atomic builtins
On Tue, Sep 02, 2014 at 03:41:34PM +0300, Chrysostomos Nanakos wrote: Replace __sync builtins with ones provided by QEMU for atomic operations. Special thanks goes to Paolo Bonzini for his refactoring suggestion in order to use the already existing atomic builtins interface. Signed-off-by: Chrysostomos Nanakos cnana...@grnet.gr --- block/archipelago.c | 76 --- 1 file changed, 23 insertions(+), 53 deletions(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgppfALKp42lh.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v2 0/2] qemu-img: src_mode option documentation improvements
On Tue, Sep 02, 2014 at 11:01:01AM +0100, Stefan Hajnoczi wrote: v2: * Fix backing file files - backing files typo [Fam] These patches clarify and fix the documentation for the recent qemu-img src_mode (-T) option. Stefan Hajnoczi (2): qemu-img: clarify src_cache option documentation qemu-img: fix rebase src_cache option documentation qemu-img.c| 3 ++- qemu-img.texi | 7 --- 2 files changed, 6 insertions(+), 4 deletions(-) -- 1.9.3 Applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan pgp9vSdQUeopj.pgp Description: PGP signature
Re: [Qemu-devel] Query regarding bdrv_co_discard
Thanks Stefan. I using the same. Below is the XML. disk type='network' device='disk' driver name='qemu' type='raw' cache='none' io='native' discard='unmap'/ source protocol='oflame' name='/dev/of/vdisk/{a30c3997-0ad8-4495-8549-68431abfa84a}' host name='10.209.133.163' port=''/ /source target dev='sda' bus='scsi'/ address type='drive' controller='1' bus='0' target='0' unit='0'/ /disk controller type='scsi' index='1' model='virtio-scsi' address type='pci' domain='0x' bus='0x00' slot='0x07' function='0x0'/ /controller I am able to see the scsi disk, /dev/sda inside the guest, but does not showing TRIM supported in the hdparm output. Fstrim command is also failing. Regards, Sanjay -Original Message- From: Stefan Hajnoczi [mailto:stefa...@gmail.com] Sent: Wednesday, September 03, 2014 4:06 PM To: Sanjay Kumar2 Cc: qemu-devel@nongnu.org Subject: Re: [Qemu-devel] Query regarding bdrv_co_discard On Tue, Sep 02, 2014 at 02:09:44AM -0700, Sanjay Kumar2 wrote: Please let me know how to test bdrv_co_discard interface. I mean what steps I need to do on KVM host and inside guest to do end to end testing of this interface. I have implemented a network protocol based block driver and defined bdrv_co_discard interface as well which free the blocks on the remote storage device. Use a virtio-scsi storage controller and then the guest can send SCSI discard commands. Make sure to use the -drive ...,discard=unmap option, otherwise discard commands are silently ignored. Stefan
Re: [Qemu-devel] qemu-system-sparc64 hang (possibly virtio related?) with 2.1
On Tue, Sep 02, 2014 at 02:12:45PM +0100, Mark Cave-Ayland wrote: Fortunately I can reproduce the issue with a debug-enabled build of qemu-system-sparc64, and I've posted a backtrace obtained during the hung state at http://www.ilande.co.uk/tmp/sparc64-gdb-bt.txt. I can't see anything too obvious, other than that the ppoll() could possibly be waiting for a bad file descriptor? The backtrace looks like a normal QEMU run. Nothing obvious there. This suggests the QEMU monitor is still operational and the guest is still executing code. Does the I/O time out inside the guest? Normally messages are printed in dmesg if I/O requests are pending for too long. Stefan pgpXp8WlSChDW.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] virtio-net: don't run bh on vm stopped
On Tue, Sep 02, 2014 at 05:26:12PM +0300, Michael S. Tsirkin wrote: commit 783e7706937fe15523b609b545587a028a2bdd03 virtio-net: stop/start bh when appropriate is incomplete: BH might execute within the same main loop iteration but after vmstop, so in theory, we might trigger an assertion. I was unable to reproduce this in practice, but it seems clear enough that the potential is there, so worth fixing. Cc: qemu-sta...@nongnu.org Reported-by: Stefan Hajnoczi stefa...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/virtio-net.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan pgpp_k6dA5quz.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3] vhost_net: cleanup recovery
On Wed, Sep 3, 2014 at 2:35 PM, Michael S. Tsirkin m...@redhat.com wrote: commit aad4dce934649b3a398396fc2a76f215bb194ea4 vhost_net: start/stop guest notifiers properly changed the order of calls for guest notifiers, but did not recover in the correct (reverse) order. Fix it up. Cc: qemu-sta...@nongnu.org Cc: Andrey Korolyov and...@xdel.ru Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/vhost_net.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..3819044 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, e, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +e = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (e 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, e); +fflush(stderr); +} +err: return r; } -- MST Except thing that the accel reinitialization takes place now at every reset, everything is OK (problematic tree with just this patch applied): qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio ... reset... qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio qemu-system-x86_64: unable to start vhost net: 95: falling back on userspace virtio
Re: [Qemu-devel] [PATCH v3] vhost_net: cleanup recovery
On 09/03/2014 06:35 PM, Michael S. Tsirkin wrote: commit aad4dce934649b3a398396fc2a76f215bb194ea4 vhost_net: start/stop guest notifiers properly changed the order of calls for guest notifiers, but did not recover in the correct (reverse) order. Fix it up. Cc: qemu-sta...@nongnu.org Cc: Andrey Korolyov and...@xdel.ru Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/vhost_net.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..3819044 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, e, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +e = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (e 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, e); +fflush(stderr); +} +err: return r; } Acked-by: Jason Wang jasow...@redhat.com
Re: [Qemu-devel] Question about cow format with hexdump
On Wed, Sep 03, 2014 at 01:27:00PM +0800, shhuiw wrote: I'm reading the source code of cow.c: https://github.com/qemu/qemu/blob/master/block/cow.c and try to understand the format better. The 'cow' format is an old format that is rarely used. It's not a good example. qcow2 is actively developed and performs better. Unfortunately it is a lot more complex. I created a cow format imagefile and can run 'qume-img info' to query the header info --- -bash-4.1$ qemu-img create -f cow dummy 1M Formatting 'test/dummy', fmt=cow size=1048576 -bash-4.1$ qemu-img info dummy image: dummy file format: cow virtual size: 1.0M (1048576 bytes) disk size: 4.0K But when I used hexdump to dis the header part, I cannot find all info recorded: (compared the define of struct cow_header_v2 and cow_create()) -- 1) recognize the magic and version info: -bash-4.1$ hexdump -C dummy -n 8 4f 4f 4f 4d 00 00 00 02 |OOOM| 0008 2) backing_file and mtime fields are 0s: # I think the dummy should be recorded -bash-4.1$ hexdump -C dummy -n 1032 4f 4f 4f 4d 00 00 00 02 00 00 00 00 00 00 00 00 |OOOM| 0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 0400 00 00 00 00 00 00 00 00 || 0408 -bash-4.1$ hexdump -C dummy -n 1036 4f 4f 4f 4d 00 00 00 02 00 00 00 00 00 00 00 00 |OOOM| 0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 0400 00 00 00 00 00 00 00 00 00 00 00 00 || 040c 3) size field is 0s: -bash-4.1$ hexdump -C dummy -n 1044 # size should be 1M 4f 4f 4f 4d 00 00 00 02 00 00 00 00 00 00 00 00 |OOOM| 0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 0410 00 00 00 00 || 0414 $ sudo yum install dwarves $ pahole block/cow.o ... struct cow_header_v2 { uint32_t magic;/* 0 4 */ uint32_t version; /* 4 4 */ char backing_file[1024]; /* 8 1024 */ /* --- cacheline 16 boundary (1024 bytes) was 8 bytes ago --- */ int32_tmtime;/* 1032 4 */ /* XXX 4 bytes hole, try to pack */ uint64_t size; /* 1040 8 */ uint32_t sectorsize; /* 1048 4 */ /* size: 1056, cachelines: 17, members: 6 */ /* sum members: 1048, holes: 1, sum holes: 4 */ /* padding: 4 */ /* last cacheline: 32 bytes */ }; Ooops, the compiler has inserted padding into the struct because the size field was not aligned. We can't change this because QEMU must stay backwards compatible. Stefan pgpfsbQjbYwzm.pgp Description: PGP signature
Re: [Qemu-devel] Fw:Re:What does COW mean?
On Wed, Sep 03, 2014 at 12:47:44PM +0800, shhuiw wrote: At 2014-09-02 04:33:50, shhuiw shh...@163.com wrote: Hi, I'm new to qemu community, and I'm trying the COW image format (old but simple:-). I have read through its source code, and didn't find anything about 'copy on write'. I wonder wthat COW stands for? Sorry for my unclear expression. I mean when copy-on-write happens if COW image format is used, and how the COW driver code handles cop-on-write? It doesn't really copy on write but it is a sparse overlay. Only written sectors need to be allocated in the image file, other sectors will be read from the backing file. Stefan pgpGKtDoGMXyk.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v5] block: Introduce null drivers
Am 01.09.2014 um 11:22 hat Fam Zheng geschrieben: This is an analogue to Linux null_blk. It can be used for testing or benchmarking block device emulation and general block layer functionalities such as coroutines and throttling, where disk IO is not necessary or wanted. Use null-aio:// for AIO version, and null-co:// for coroutine version. Signed-off-by: Fam Zheng f...@redhat.com Reviewed-by: BenoƮt Canet benoit.ca...@nodalink.com +static BlockDriver bdrv_null_co = { +.format_name= null-co, +.protocol_name = null-co, +.instance_size = sizeof(BDRVNullState), + +.bdrv_file_open = null_file_open, +.bdrv_close = null_close, +.bdrv_getlength = null_getlength, + +.bdrv_read = null_co_read, +.bdrv_write = null_co_write, Any reason not to use the native .bdrv_co_readv/writev interfaces instead of the old, emulated versions that involve a bounce buffer? +.bdrv_co_flush_to_disk = null_co_flush, +}; Kevin
Re: [Qemu-devel] [PATCH v3] block: Introduce null driver
Am 29.08.2014 um 02:55 hat Fam Zheng geschrieben: On Thu, 08/28 16:23, Eric Blake wrote: On 08/27/2014 11:53 PM, Fam Zheng wrote: This is an analogue to Linux null_blk. It can be used for testing block device emulation and general block layer functionalities such as coroutines and throttling, where disk IO is not necessary or wanted. Use null:// for AIO version, and null-co:// for coroutine version. Signed-off-by: Fam Zheng f...@redhat.com --- +++ b/qapi/block-core.json @@ -1150,7 +1150,8 @@ 'data': [ 'archipelago', 'file', 'host_device', 'host_cdrom', 'host_floppy', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat', 'blkdebug', 'blkverify', 'bochs', 'cloop', 'cow', 'dmg', 'parallels', 'qcow', -'qcow2', 'qed', 'raw', 'vdi', 'vhdx', 'vmdk', 'vpc', 'quorum' ] } +'qcow2', 'qed', 'raw', 'vdi', 'vhdx', 'vmdk', 'vpc', 'quorum', +'null' ] } As mentioned elsewhere, you probably want 'null' AND 'null-co'. Bummer that this list is mostly alphabetical, but that 'quorum' and 'null' are out of order. I'll add null-co to the list. But this list is a bit far from alphabetical: archipelago, ..., vvfat, blkdebug,... Actually it's pretty close to alphabetical as the secondary criterion (the primary one is that all protocols are listed first, and only then the formats). Kevin
Re: [Qemu-devel] [PULL 00/13] pci, pc fixes, features
On Tue, Sep 02, 2014 at 06:07:01PM +0300, Michael S. Tsirkin wrote: The following changes since commit 187de915e8d06aaf82be206aebc551c82bf0670c: pcie: fix trailing whitespace (2014-08-25 00:16:07 +0200) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream for you to fetch changes up to aad4dce934649b3a398396fc2a76f215bb194ea4: vhost_net: start/stop guest notifiers properly (2014-09-02 17:33:37 +0300) pci, pc fixes, features A bunch of bugfixes - these will make sense for 2.1.1 Initial Intel IOMMU support. Signed-off-by: Michael S. Tsirkin m...@redhat.com Gonglei (1): ioh3420: remove unused ioh3420_init() declaration Jason Wang (1): vhost_net: start/stop guest notifiers properly Knut Omang (1): pci: avoid losing config updates to MSI/MSIX cap regs Le Tan (8): iommu: add is_write as a parameter to the translate function of MemoryRegionIOMMUOps intel-iommu: introduce Intel IOMMU (VT-d) emulation intel-iommu: add DMAR table to ACPI tables intel-iommu: add Intel IOMMU emulation to q35 and add a machine option iommu as a switch intel-iommu: fix coding style issues around in q35.c and machine.c intel-iommu: add supports for queued invalidation interface intel-iommu: add context-cache to cache context-entry intel-iommu: add IOTLB using hash table Michael S. Tsirkin (2): vhost_net: cleanup start/stop condition A problem was reported with this one. I fixed it up, will send v2 pull. virtio-net: don't run bh on vm stopped hw/i386/acpi-defs.h| 40 + hw/i386/intel_iommu_internal.h | 389 hw/pci-bridge/ioh3420.h|4 - include/exec/memory.h |2 +- include/hw/boards.h|1 + include/hw/i386/intel_iommu.h | 120 +++ include/hw/pci-host/q35.h |2 + exec.c |2 +- hw/alpha/typhoon.c |3 +- hw/core/machine.c | 27 +- hw/i386/acpi-build.c | 39 + hw/i386/intel_iommu.c | 1963 hw/net/vhost_net.c | 39 +- hw/net/virtio-net.c| 14 +- hw/pci-host/apb.c |3 +- hw/pci-host/q35.c | 58 +- hw/pci/pci.c |7 +- hw/ppc/spapr_iommu.c |3 +- hw/virtio/vhost.c |2 - vl.c |4 + hw/i386/Makefile.objs |1 + qemu-options.hx|5 +- 22 files changed, 2683 insertions(+), 45 deletions(-) create mode 100644 hw/i386/intel_iommu_internal.h create mode 100644 include/hw/i386/intel_iommu.h create mode 100644 hw/i386/intel_iommu.c
Re: [Qemu-devel] [PATCH v3] vhost_net: cleanup recovery
On Wed, Sep 03, 2014 at 06:52:46PM +0800, Jason Wang wrote: On 09/03/2014 06:35 PM, Michael S. Tsirkin wrote: commit aad4dce934649b3a398396fc2a76f215bb194ea4 vhost_net: start/stop guest notifiers properly changed the order of calls for guest notifiers, but did not recover in the correct (reverse) order. Fix it up. Cc: qemu-sta...@nongnu.org Cc: Andrey Korolyov and...@xdel.ru Cc: Jason Wang jasow...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/vhost_net.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index ba5d544..3819044 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -289,7 +289,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(dev))); VirtioBusState *vbus = VIRTIO_BUS(qbus); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); -int r, i = 0; +int r, e, i; if (!vhost_net_device_endian_ok(dev)) { error_report(vhost-net does not support cross-endian); @@ -317,16 +317,22 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, r = vhost_net_start_one(get_vhost_net(ncs[i].peer), dev); if (r 0) { -goto err; +goto err_start; } } return 0; -err: +err_start: while (--i = 0) { vhost_net_stop_one(get_vhost_net(ncs[i].peer), dev); } +e = k-set_guest_notifiers(qbus-parent, total_queues * 2, false); +if (e 0) { +fprintf(stderr, vhost guest notifier cleanup failed: %d\n, e); +fflush(stderr); +} +err: return r; } Acked-by: Jason Wang jasow...@redhat.com OK I will just roll this up into the original patch.
Re: [Qemu-devel] [PATCH] virtio-net: don't run bh on vm stopped
On Wed, Sep 03, 2014 at 11:45:57AM +0100, Stefan Hajnoczi wrote: On Tue, Sep 02, 2014 at 05:26:12PM +0300, Michael S. Tsirkin wrote: commit 783e7706937fe15523b609b545587a028a2bdd03 virtio-net: stop/start bh when appropriate is incomplete: BH might execute within the same main loop iteration but after vmstop, so in theory, we might trigger an assertion. I was unable to reproduce this in practice, but it seems clear enough that the potential is there, so worth fixing. Cc: qemu-sta...@nongnu.org Reported-by: Stefan Hajnoczi stefa...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/net/virtio-net.c | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) Thanks, applied to my net tree: https://github.com/stefanha/qemu/commits/net Stefan It was actually on my tree already but since I recalled my last pull request, no problem. Can you send pull request with this today please, so it can go into 2.1.1?
[Qemu-devel] [PATCH v4 00/20] block: Asynchronous request cancellation
v4: Drop AIOCBInfo.cancel. This series adds a new block layer API: void bdrv_aio_cancel_async(BlockDriverAIOCB *acb); And use it to emulate bdrv_aio_cancel. The function is similar to bdrv_aio_cancel in that it cancels an AIO request, but different that it doesn't block until the request is completely cancelled or done. More importantly, the completion callback, BlockDriverAIOCB.cb, is guaranteed to be called, so that the cb can take care of resource releasing and status reporting to guest, etc. In the following work, scsi emulation code will be shifted to use the async cancelling. One major benefit would be that when guest tries to cancel a request, where the request cannot be cancelled easily, (due to throttled BlockDriverState, a lost connection, or a large request queue), we don't need to block the whole vm with a busy loop, which is how bdrv_aio_cancel is implemented now. A test case that is easy to reproduce is, throttle a scsi-disk to a very low limit, for example 50 bps, then stress the guest block device with dd or fio. Currently, the vm will quickly hang when it loses patience and send a tmf command to cancel the request, at which point we will busy wait in bdrv_aio_cancel, until the request is slowly spit out from throttled_reqs. Later, we will change scsi device code to make this asynchronous, on top of bdrv_aio_cancel_async. Fam Fam Zheng (20): block: Add refcnt in BlockDriverAIOCB block: Add bdrv_aio_cancel_async block: Drop bdrv_em_co_aiocb_info.cancel block: Convert bdrv_em_aiocb_info.cancel to .cancel_async thread-pool: Convert thread_pool_aiocb_info.cancel to cancel_async linux-aio: Convert laio_aiocb_info.cancel to .cancel_async dma: Check iov pointer before unmap memory dma: Convert dma_aiocb_info.cancel to .cancel_async iscsi: Convert iscsi_aiocb_info.cancel to .cancel_async archipelago: Drop archipelago_aiocb_info.cancel blkdebug: Convert blkdebug_aiocb_info.cancel to .cancel_async blkverify: Drop blkverify_aiocb_info.cancel curl: Drop curl_aiocb_info.cancel qed: Drop qed_aiocb_info.cancel quorum: Convert quorum_aiocb_info.cancel to .cancel_async rbd: Drop rbd_aiocb_info.cancel sheepdog: Convert sd_aiocb_info.cancel to .cancel_async win32-aio: Drop win32_aiocb_info.cancel ide: Convert trim_aiocb_info.cancel to .cancel_async block: Drop AIOCBInfo.cancel block.c | 61 +--- block/archipelago.c | 17 +- block/blkdebug.c | 6 +++-- block/blkverify.c| 19 --- block/curl.c | 6 - block/iscsi.c| 17 ++ block/linux-aio.c| 30 +++- block/qed.c | 21 - block/quorum.c | 7 ++ block/rbd.c | 23 +- block/sheepdog.c | 46 +++- block/win32-aio.c| 14 --- dma-helpers.c| 23 ++ hw/ide/core.c| 12 -- include/block/aio.h | 5 +++- include/block/block.h| 1 + tests/test-thread-pool.c | 34 --- thread-pool.c| 32 +++-- 18 files changed, 131 insertions(+), 243 deletions(-) -- 2.1.0.27.g96db324
[Qemu-devel] [PATCH v4 03/20] block: Drop bdrv_em_co_aiocb_info.cancel
Also drop the now unused -done pointer. Signed-off-by: Fam Zheng f...@redhat.com --- block.c | 21 - 1 file changed, 21 deletions(-) diff --git a/block.c b/block.c index 2dfd1be..4aa1bd7 100644 --- a/block.c +++ b/block.c @@ -4757,22 +4757,8 @@ typedef struct BlockDriverAIOCBCoroutine { QEMUBH* bh; } BlockDriverAIOCBCoroutine; -static void bdrv_aio_co_cancel_em(BlockDriverAIOCB *blockacb) -{ -AioContext *aio_context = bdrv_get_aio_context(blockacb-bs); -BlockDriverAIOCBCoroutine *acb = -container_of(blockacb, BlockDriverAIOCBCoroutine, common); -bool done = false; - -acb-done = done; -while (!done) { -aio_poll(aio_context, true); -} -} - static const AIOCBInfo bdrv_em_co_aiocb_info = { .aiocb_size = sizeof(BlockDriverAIOCBCoroutine), -.cancel = bdrv_aio_co_cancel_em, }; static void bdrv_co_em_bh(void *opaque) @@ -4781,10 +4767,6 @@ static void bdrv_co_em_bh(void *opaque) acb-common.cb(acb-common.opaque, acb-req.error); -if (acb-done) { -*acb-done = true; -} - qemu_bh_delete(acb-bh); qemu_aio_release(acb); } @@ -4825,7 +4807,6 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs, acb-req.qiov = qiov; acb-req.flags = flags; acb-is_write = is_write; -acb-done = NULL; co = qemu_coroutine_create(bdrv_co_do_rw); qemu_coroutine_enter(co, acb); @@ -4852,7 +4833,6 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs, BlockDriverAIOCBCoroutine *acb; acb = qemu_aio_get(bdrv_em_co_aiocb_info, bs, cb, opaque); -acb-done = NULL; co = qemu_coroutine_create(bdrv_aio_flush_co_entry); qemu_coroutine_enter(co, acb); @@ -4882,7 +4862,6 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs, acb = qemu_aio_get(bdrv_em_co_aiocb_info, bs, cb, opaque); acb-req.sector = sector_num; acb-req.nb_sectors = nb_sectors; -acb-done = NULL; co = qemu_coroutine_create(bdrv_aio_discard_co_entry); qemu_coroutine_enter(co, acb); -- 2.1.0.27.g96db324
[Qemu-devel] [PATCH v4 07/20] dma: Check iov pointer before unmap memory
Not all the iov elements are always valid. Signed-off-by: Fam Zheng f...@redhat.com --- dma-helpers.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dma-helpers.c b/dma-helpers.c index 499b52b..3655d88 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -105,6 +105,9 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs) int i; for (i = 0; i dbs-iov.niov; ++i) { +if (!(dbs-iov.iov[i].iov_base dbs-iov.iov[i].iov_len)) { +break; +} dma_memory_unmap(dbs-sg-as, dbs-iov.iov[i].iov_base, dbs-iov.iov[i].iov_len, dbs-dir, dbs-iov.iov[i].iov_len); -- 2.1.0.27.g96db324
[Qemu-devel] [PATCH v4 02/20] block: Add bdrv_aio_cancel_async
This is the async version of bdrv_aio_cancel, which doesn't block the caller. It guarantees that the cb is called either before returning or some time later. bdrv_aio_cancel can base on bdrv_aio_cancel_async, later we can convert all .io_cancel implementations to .io_cancel_async, and the aio_poll is the common logic. In the end, .io_cancel can be dropped. Signed-off-by: Fam Zheng f...@redhat.com --- block.c | 27 ++- include/block/aio.h | 2 ++ include/block/block.h | 1 + 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index 78d68cb..2dfd1be 100644 --- a/block.c +++ b/block.c @@ -4634,7 +4634,32 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs) void bdrv_aio_cancel(BlockDriverAIOCB *acb) { -acb-aiocb_info-cancel(acb); +if (acb-aiocb_info-cancel) { +acb-aiocb_info-cancel(acb); +} else { +qemu_aio_ref(acb); +bdrv_aio_cancel_async(acb); +while (acb-refcnt 1) { +if (acb-aiocb_info-get_aio_context) { +aio_poll(acb-aiocb_info-get_aio_context(acb), true); +} else if (acb-bs) { +aio_poll(bdrv_get_aio_context(acb-bs), true); +} else { +abort(); +} +} +qemu_aio_release(acb); +} +} + +/* Async version of aio cancel. The caller is not blocked if the acb implements + * cancel_async, otherwise we do nothing and let the request normally complete. + * In either case the completion callback must be called. */ +void bdrv_aio_cancel_async(BlockDriverAIOCB *acb) +{ +if (acb-aiocb_info-cancel_async) { +acb-aiocb_info-cancel_async(acb); +} } /**/ diff --git a/include/block/aio.h b/include/block/aio.h index 2626fc7..ad361e3 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -27,6 +27,8 @@ typedef void BlockDriverCompletionFunc(void *opaque, int ret); typedef struct AIOCBInfo { void (*cancel)(BlockDriverAIOCB *acb); +void (*cancel_async)(BlockDriverAIOCB *acb); +AioContext *(*get_aio_context)(BlockDriverAIOCB *acb); size_t aiocb_size; } AIOCBInfo; diff --git a/include/block/block.h b/include/block/block.h index 8f4ad16..35a2448 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -336,6 +336,7 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BlockDriverCompletionFunc *cb, void *opaque); void bdrv_aio_cancel(BlockDriverAIOCB *acb); +void bdrv_aio_cancel_async(BlockDriverAIOCB *acb); typedef struct BlockRequest { /* Fields to be filled by multiwrite caller */ -- 2.1.0.27.g96db324
[Qemu-devel] [PATCH v4 01/20] block: Add refcnt in BlockDriverAIOCB
This will be useful in synchronous cancel emulation with bdrv_aio_cancel_async. Signed-off-by: Fam Zheng f...@redhat.com --- block.c | 12 +++- include/block/aio.h | 2 ++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index cb670fd..78d68cb 100644 --- a/block.c +++ b/block.c @@ -4885,13 +4885,23 @@ void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs, acb-bs = bs; acb-cb = cb; acb-opaque = opaque; +acb-refcnt = 1; return acb; } +void qemu_aio_ref(void *p) +{ +BlockDriverAIOCB *acb = p; +acb-refcnt++; +} + void qemu_aio_release(void *p) { BlockDriverAIOCB *acb = p; -g_slice_free1(acb-aiocb_info-aiocb_size, acb); +assert(acb-refcnt 0); +if (--acb-refcnt == 0) { +g_slice_free1(acb-aiocb_info-aiocb_size, acb); +} } /**/ diff --git a/include/block/aio.h b/include/block/aio.h index 4603c0f..2626fc7 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -35,11 +35,13 @@ struct BlockDriverAIOCB { BlockDriverState *bs; BlockDriverCompletionFunc *cb; void *opaque; +int refcnt; }; void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs, BlockDriverCompletionFunc *cb, void *opaque); void qemu_aio_release(void *p); +void qemu_aio_ref(void *p); typedef struct AioHandler AioHandler; typedef void QEMUBHFunc(void *opaque); -- 2.1.0.27.g96db324
[Qemu-devel] [PATCH v4 06/20] linux-aio: Convert laio_aiocb_info.cancel to .cancel_async
Just call io_cancel (2), if it fails, it means the request is not canceled, so the event loop will eventually call qemu_laio_process_completion. In qemu_laio_process_completion, change to call the cb unconditionally. It is required by bdrv_aio_cancel_async. Signed-off-by: Fam Zheng f...@redhat.com --- block/linux-aio.c | 30 -- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/block/linux-aio.c b/block/linux-aio.c index 9aca758..b67f56c 100644 --- a/block/linux-aio.c +++ b/block/linux-aio.c @@ -85,9 +85,8 @@ static void qemu_laio_process_completion(struct qemu_laio_state *s, ret = -EINVAL; } } - -laiocb-common.cb(laiocb-common.opaque, ret); } +laiocb-common.cb(laiocb-common.opaque, ret); qemu_aio_release(laiocb); } @@ -153,35 +152,22 @@ static void laio_cancel(BlockDriverAIOCB *blockacb) struct io_event event; int ret; -if (laiocb-ret != -EINPROGRESS) +if (laiocb-ret != -EINPROGRESS) { return; - -/* - * Note that as of Linux 2.6.31 neither the block device code nor any - * filesystem implements cancellation of AIO request. - * Thus the polling loop below is the normal code path. - */ +} ret = io_cancel(laiocb-ctx-ctx, laiocb-iocb, event); -if (ret == 0) { -laiocb-ret = -ECANCELED; +laiocb-ret = -ECANCELED; +if (ret != 0) { +/* iocb is not cancelled, cb will be called by the event loop later */ return; } -/* - * We have to wait for the iocb to finish. - * - * The only way to get the iocb status update is by polling the io context. - * We might be able to do this slightly more optimal by removing the - * O_NONBLOCK flag. - */ -while (laiocb-ret == -EINPROGRESS) { -qemu_laio_completion_cb(laiocb-ctx-e); -} +laiocb-common.cb(laiocb-common.opaque, laiocb-ret); } static const AIOCBInfo laio_aiocb_info = { .aiocb_size = sizeof(struct qemu_laiocb), -.cancel = laio_cancel, +.cancel_async = laio_cancel, }; static void ioq_init(LaioQueue *io_q) -- 2.1.0.27.g96db324
[Qemu-devel] [PATCH v4 15/20] quorum: Convert quorum_aiocb_info.cancel to .cancel_async
Before, we cancel all the child requests with bdrv_aio_cancel, then free the acb.. Now we just kick off asynchronous cancellation of child requests and return, we know quorum_aio_cb will be called later, so in the end quorum_aio_finalize will take care of calling the caller's cb. Signed-off-by: Fam Zheng f...@redhat.com --- block/quorum.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/block/quorum.c b/block/quorum.c index 093382e..0951d52 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -138,16 +138,13 @@ static void quorum_aio_cancel(BlockDriverAIOCB *blockacb) /* cancel all callbacks */ for (i = 0; i s-num_children; i++) { -bdrv_aio_cancel(acb-qcrs[i].aiocb); +bdrv_aio_cancel_async(acb-qcrs[i].aiocb); } - -g_free(acb-qcrs); -qemu_aio_release(acb); } static AIOCBInfo quorum_aiocb_info = { .aiocb_size = sizeof(QuorumAIOCB), -.cancel = quorum_aio_cancel, +.cancel_async = quorum_aio_cancel, }; static void quorum_aio_finalize(QuorumAIOCB *acb) -- 2.1.0.27.g96db324
[Qemu-devel] [PATCH v4 10/20] archipelago: Drop archipelago_aiocb_info.cancel
The cancelled flag is no longer useful. Later the request will complete as before, and cb will be called. Signed-off-by: Fam Zheng f...@redhat.com --- block/archipelago.c | 17 + 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/block/archipelago.c b/block/archipelago.c index 34f72dc..c0f288a 100644 --- a/block/archipelago.c +++ b/block/archipelago.c @@ -91,7 +91,6 @@ typedef struct ArchipelagoAIOCB { struct BDRVArchipelagoState *s; QEMUIOVector *qiov; ARCHIPCmd cmd; -bool cancelled; int status; int64_t size; int64_t ret; @@ -317,9 +316,7 @@ static void qemu_archipelago_complete_aio(void *opaque) aio_cb-common.cb(aio_cb-common.opaque, aio_cb-ret); aio_cb-status = 0; -if (!aio_cb-cancelled) { -qemu_aio_release(aio_cb); -} +qemu_aio_release(aio_cb); g_free(reqdata); } @@ -723,19 +720,8 @@ static int qemu_archipelago_create(const char *filename, return ret; } -static void qemu_archipelago_aio_cancel(BlockDriverAIOCB *blockacb) -{ -ArchipelagoAIOCB *aio_cb = (ArchipelagoAIOCB *) blockacb; -aio_cb-cancelled = true; -while (aio_cb-status == -EINPROGRESS) { -aio_poll(bdrv_get_aio_context(aio_cb-common.bs), true); -} -qemu_aio_release(aio_cb); -} - static const AIOCBInfo archipelago_aiocb_info = { .aiocb_size = sizeof(ArchipelagoAIOCB), -.cancel = qemu_archipelago_aio_cancel, }; static int archipelago_submit_request(BDRVArchipelagoState *s, @@ -918,7 +904,6 @@ static BlockDriverAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs, aio_cb-ret = 0; aio_cb-s = s; -aio_cb-cancelled = false; aio_cb-status = -EINPROGRESS; off = sector_num * BDRV_SECTOR_SIZE; -- 2.1.0.27.g96db324
[Qemu-devel] [PATCH v4 09/20] iscsi: Convert iscsi_aiocb_info.cancel to .cancel_async
Also drop the unused field canceled. Signed-off-by: Fam Zheng f...@redhat.com --- block/iscsi.c | 17 ++--- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 3e19202..a0aca5f 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -88,7 +88,6 @@ typedef struct IscsiAIOCB { struct scsi_task *task; uint8_t *buf; int status; -int canceled; int64_t sector_num; int nb_sectors; #ifdef __linux__ @@ -120,9 +119,7 @@ iscsi_bh_cb(void *p) g_free(acb-buf); acb-buf = NULL; -if (acb-canceled == 0) { -acb-common.cb(acb-common.opaque, acb-status); -} +acb-common.cb(acb-common.opaque, acb-status); if (acb-task != NULL) { scsi_free_scsi_task(acb-task); @@ -240,20 +237,15 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb) return; } -acb-canceled = 1; - /* send a task mgmt call to the target to cancel the task on the target */ iscsi_task_mgmt_abort_task_async(iscsilun-iscsi, acb-task, iscsi_abort_task_cb, acb); -while (acb-status == -EINPROGRESS) { -aio_poll(iscsilun-aio_context, true); -} } static const AIOCBInfo iscsi_aiocb_info = { .aiocb_size = sizeof(IscsiAIOCB), -.cancel = iscsi_aio_cancel, +.cancel_async = iscsi_aio_cancel, }; @@ -638,10 +630,6 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status, g_free(acb-buf); acb-buf = NULL; -if (acb-canceled != 0) { -return; -} - acb-status = 0; if (status 0) { error_report(Failed to ioctl(SG_IO) to iSCSI lun. %s, @@ -683,7 +671,6 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, acb = qemu_aio_get(iscsi_aiocb_info, bs, cb, opaque); acb-iscsilun = iscsilun; -acb-canceled= 0; acb-bh = NULL; acb-status = -EINPROGRESS; acb-buf = NULL; -- 2.1.0.27.g96db324