[Qemu-devel] RE: Printed Roll-up Banners USD7 Only (GIGAPrint Overseas)
Dear He? Justus GIGAPrint Ltd. | 11/F, Fu Hop Fty Bldg, 209-211 Wai Yip St, Kwun Tong, Kowloon, HK | 23892088 您收到這郵件是由於我們相信其中的訊息與您相關。如欲取消接收所有關於本機構的產品或服務的訊息請按 http://vps.web-123.com/webm/tracer/u.php?g=8447.532556.690247798 You are receiving this message because we believed that it is relevant to you. If you do not wish to receive any materials regarding our products or services from us, please click http://vps.web-123.com/webm/tracer/u.php?g=8447.532556.690247798
Re: [Qemu-devel] [PATCH] `qdev_free` when unplug a pci device
At 03/01/2011 03:13 PM, Isaku Yamahata Write: > On Tue, Mar 01, 2011 at 02:58:44PM +0800, Wen Congyang wrote: >> At 03/01/2011 12:11 PM, Isaku Yamahata Write: >>> Hi. >>> >>> I don't suppose that just introducing pending bits solve the issue. >>> Your test only use single hot plug/unplug. How about mixing of multiple >>> hot plug/unplug with different slots. >> >> The qemu uses the same thread to deal with monitor command and I/O request >> from guest OS. So I think mixing of multiple hot plug/unplug with different >> slots can also work fine. >> >>> Zeroing up/down on piix4_device_hotplug() is the culprit. >>> >>> State machine of (up, down) would be needed. >>> (up, down) of each slots should be changed following >>> something like >> >> Why we need this feature? We access s->pci0_status only in main thread and >> do not accesss it when we deal with signal, so there is no competition to >> access it. > > We are talking about losing interrupt and events, aren't we? > Not race caused by threads. I am sorry, I do not understand what you are talk about. > The issue is, Qemu injected sci interrupt into guest, > but before the guest completes to handled it, > users/qemu can start to inject the next sci event triggered by hot > plug/unplug. > Thus qemu loses sci interrupt and up/down status. Yes, you are right. But I still do not understand why introducing pending bits can not solve the issue? Thanks Wen Congyang > > thanks >> >>> >>> - on power-on (0, 0) >>> - on hot plug (0, 0) -> (1, 0) >>>if other combination -> error >>> - on hot unpug (1, 0) or (0, 0) -> (0, 1) >>>(0, 0) is for cold plugged devices. >>>(1, 0) is for hot plugged devices. >>>if other combination -> error >>> - on pciej_write(write on PCI_EJ_BASE) >>>(0, 1) -> (0, 0) and qdev_free() >>>if other combination -> ignore >>> >>> When enabling sci, those bits are checked and raise sci if necessary. >>> >>> Any comments on this? >>> Anyway the current pci hotplug-related commands are somewhat broken, >>> and needs clean up with multifunction hotplug support which is >>> on my todo list. >>> >>> thanks >>> >>> On Mon, Feb 28, 2011 at 10:52:40AM +0800, Wen Congyang wrote: Hi Markus Armbruster At 02/23/2011 04:30 PM, Markus Armbruster Write: > Isaku Yamahata writes: > > > I don't think this patch is correct. Let me explain. > > Device hot unplug is *not* guaranteed to succeed. > > For some buses, such as USB, it always succeeds immediately, i.e. when > the device_del monitor command finishes, the device is gone. Live is > good. > > But for PCI, device_del merely initiates the ACPI unplug rain dance. It > doesn't wait for the dance to complete. Why? The dance can take an > unpredictable amount of time, including forever. > > Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI > slot, and the unplug has not yet completed (race condition), or it > failed. Yes, Virginia, PCI hotplug *can* fail. > > When unplug succeeds, the qdev is automatically destroyed. > pciej_write() does that for PIIX4. Looks like pcie_cap_slot_event() > does it for PCIE. I got a similar problem. When I unplug a pci device by hand, it works as expected, and I can hotplug it again. But when I use a srcipt to do the same thing, sometimes it failed. I think I may find another bug. Steps to reproduce this bug: 1. cat ./test-e1000.sh # RHEL6RC is domain name #! /bin/bash while true; do virsh attach-interface RHEL6RC network default --mac 52:54:00:1f:db:c7 --model e1000 if [[ $? -ne 0 ]]; then break fi virsh detach-interface RHEL6RC network --mac 52:54:00:1f:db:c7 if [[ $? -ne 0 ]]; then break fi sleep 5 done 2. ./test-e1000.sh Interface attached successfully Interface detached successfully Interface attached successfully Interface detached successfully error: Failed to attach interface error: operation failed: adding e1000,netdev=hostnet1,id=net1,mac=52:54:00:1f:db:c7,bus=pci.0,addr=0x8 device failed: Duplicate ID 'net1' for device I use ftrace to trace whether sci interrupt is passed to guest OS, here is log: # cat trace | grep 'irq=9' -0 [000] 118342.634772: irq_handler_entry: irq=9 name=acpi -0 [000] 118342.634831: irq_handler_exit: irq=9 ret=handled -0 [000] 118342.693216: irq_handler_entry: irq=9 name=acpi -0 [000] 118342.693254: irq_handler_exit: irq=9 ret=handled -0 [000] 118
[Qemu-devel] Link to UEFI OVMF on downloads page
Hi all, Would it be possible to add a link to OVMF: http://sourceforge.net/apps/mediawiki/tianocore/index.php?title=OVMF on the qemu links page: http://wiki.qemu.org/Links for UEFI support on x86 & x86-64 guests? Thanks, -Jordan
Re: [Qemu-devel] Re: [PATCH] Split machine creation from the main loop
On 02/28/2011 06:01 AM, Anthony Liguori wrote: It would be a pity to divorce the monitor from chardevs, they're really flexible. Couple considerations: 1) chardevs don't support multiple simultaneous connections. I view this as a blocker for QMP. What do you mean by that? Something like ,server which keeps on listening after it a connection is established? ,server won't allow multiple simultaneous connections. CharDriverStates simply don't have a connection semantic. There can only be one thing connected to it at a time. This is why we don't use CharDriverState for VNC. I meant an extension to ,server that keeps on listening. We should have another abstraction for connection based backend. I'll take a go at this when I'm ready to try to get those patches in. Shouldn't each new connection return a chardev? Just to be clear though, there is a CharDriverState version of the new QMP server. This would be a second option for creating a QMP server and it takes a different command line sytnax. 2) Because chardevs don't support multiple connections, we can't reasonably hook on things like connect/disconnect which means that fd's sent via SCM_RIGHTs have to be handled in a very special way. By going outside of the chardev layer, we can let fd's via SCM_RIGHTS queue up naturally and have getfd/setfd refer to the fd at the top of the queue. It makes it quite a bit easier to work with (I believe Daniel had actually requested this a while ago). I really don't follow... what's the connection between SCM_RIGHTS and multiple connections? Monitors have a single fd. That fd is associated with the monitor and lives beyond the length of the connection to the monitor (recall that chardevs don't have a notion of connection life cycle). This means if a management tool forgets to do a closefd on an unused fd, there's no easy way for QEMU to automatically clean that up. IOW, a crashed management tool == fd leak in QEMU. I guess we could add a close() event to chardev? -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH (resend, rebased)] char: Prevent multiple devices opening same chardev
Prevent: -chardev socket,path=/tmp/foo,server,nowait,id=c0 \ -device virtserialport,chardev=c0,id=vs0 \ -device virtserialport,chardev=c0,id=vs1 Reported-by: Mike Cao Signed-off-by: Amit Shah --- hw/qdev-properties.c |7 ++- qemu-char.h |1 + 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index a45b61e..1088a26 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -351,8 +351,13 @@ static int parse_chr(DeviceState *dev, Property *prop, const char *str) CharDriverState **ptr = qdev_get_prop_ptr(dev, prop); *ptr = qemu_chr_find(str); -if (*ptr == NULL) +if (*ptr == NULL) { return -ENOENT; +} +if ((*ptr)->assigned) { +return -EEXIST; +} +(*ptr)->assigned = 1; return 0; } diff --git a/qemu-char.h b/qemu-char.h index 56d9954..fb96eef 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -70,6 +70,7 @@ struct CharDriverState { char *label; char *filename; int opened; +int assigned; /* chardev assigned to a device */ QTAILQ_ENTRY(CharDriverState) next; }; -- 1.7.4
Re: [Qemu-devel] Re: [patch 2/3] Add support for live block copy
On 02/27/2011 07:41 PM, Anthony Liguori wrote: I agree 100% the management tool cannot be the authoritative source of state. My position is: - the management tool should be 100% in control of configuration (how the guest is put together from its components) - qemu should be 100% in control of state (memory, disk state, NVRAM in various components, cd-rom eject state, explosive bolts for payload separation, self-destruct mechanism, etc.) There simply is not such a clean separation between the two because things that the guest does affects the configuration of the guest. Hot plug, I don't think hotunplug works this way. When the guest ejects the pci or usb device, it simply stops working with the device and disconnects the power. There is nothing non-volatile going on, no spring-loaded lever that pushes the device out. If the server reboots immediately after hotunplug, but before the user physically removes the device, then the server will see the device when it boots up. removable media eject, Here, we do have a single bit of non-volatile storage. persistent device settings (whether it's CMOS or EEPROM) all disrupt this model. These are just arrays of bits, most of them with no standard interpretation. So a block device fits them perfectly. If you really wanted to have this separation, you'd have to be very strict about making all guest settings not be specified in config. You would need to do: qemu-img create -f e1000-eprom -o macaddr=12:23:45:67:78:90 e1000.0.rom qemu-img create -f e1000-eprom -o macaddr=12:23:45:67:78:91 e1000.1.rom qemu -device e1000,id=e1000.0,eeprom=e1000.0.rom -device e1000,id=e1000.1,eeprom=e1000.1.rom And now I need a tool that lets me modify e1000-eprom images if I want to change the mac address dynamically (say I'm trying to clone a VM). This type of model can be workable but as I said earlier, I think it's overengineering the problem. In fact I don't think anyone wants this. Usually management wants the assigned MAC to be used without the guest playing games with it. So it's more or less pointless however it's implemented. We don't separate configuration from guest state today. Instead of setting ourselves up for failure by setting an unrealistic standard that we try to achieve and never do, let's embrace the system that is working for us today. We are authoritative for everything and guest state is intimately tied to the virtual machine configuration. "we are authoritative for everything" is a clean break from everything that's being done today. It's also a clean break from the model of central management plus database. We can't force it on people. Non-volatile state is not intimately tied to configuration. We store block device state completely outside the configuration. What's left is the CD-ROM tray, CMOS memory, and network card EEPROM. We could argue back and forth about where exactly they belong, but they aren't really worth the conversation since they are meaningless for real-life use. But beyond those races, QEMU is the only entity that knows with certainty what bits of information are important to persist in order to preserve a guest across shutdown/restart. The fact that we've punted this problem for so long has only ensured that management tools are either intrinsically broken or only support the most minimal subset of functionality we actually support. I'm not arguing about that. I just want to stress again the difference between state and configuration. Qemu has no authority, in my mind, as to configuration. Only state. Being the one that creates a guest based on configuration, I would say that we most certainly do. That is not what being authoritative means. In a virt-manager deployment, libvirt is the authoritative source of guest configuration. In a RHEV-M deployment, the RHEV-M database is the authoritative source of guest configuration. You can completely replace the host machine and your guest will recreate just fine as long as the authoritative source is intact. Currently they contain the required guest configuration, a representation of what's the current live configuration, and they issue monitor commands to move the live configuration towards the required configuration (or just generate a qemu command line). What you're describing is completely different, I'm not even sure what it is. Management tools shouldn't have to think about how the monitor commands they issue impact the invocation options of QEMU. They have to, when creating a guest from scratch. But I admit, this throws a new light (for me) on things. What's the implications? - must have a qemu instance running when editing configuration, even when the guest is down QMP is an API. Whether a qemu instance is launched is an implementation detail. This could all be hidden completely with libqmp. QMP is first and foremost a protocol. - cannot add additional
Re: [Qemu-devel] [PATCH v3] rtl8139: add vlan support
On 02/26/2011 08:39 AM, Benjamin Poirier wrote: I've posted v2 of these patches back in november http://article.gmane.org/gmane.comp.emulators.qemu/84252 Changes since v2: insertion: * moved insertion later in the process, to handle tso * use qemu_sendv_packet() to insert the tag for us * added dot1q_buf parameter to rtl8139_do_receive() to avoid some memcpy() in loopback mode. Note that the code path through that function is unchanged when dot1q_buf is NULL. extraction: * reduced the amount of copying by moving the "frame too short" logic after the removal of the vlan tag (as is done in e1000.c for example). Unfortunately, that logic can no longer be shared betwen C+ and C mode. I've tested on the following combinations of guest and hosts: host: x86_64, guest: x86_64 host: x86_64, guest: ppc32 host: ppc32, guest: ppc32 Testing on the x86_64 host used '-net tap' and consisted of: * making an http transfert on the untagged interface. * ping -s 0-1472 to another host on a vlan. * making an scp upload to another host on a vlan. Testing on the ppc32 host used '-net socket' connected to an x86_64 qemu-kvm running the virtio nic and consisted of: * establishing an ssh connection between the two using an untagged interface. * ping -s 0-1472 to the ppc32 using a vlan. * making an scp transfer in both directions using a vlan. All that was successful. Nevertheless, it doesn't exercise all code paths so care is in order. Thanks for the patch and it's better to test tso also. Please note that the lack of vlan support in rtl8139 has taken a few people aback: https://bugzilla.redhat.com/show_bug.cgi?id=516587 http://article.gmane.org/gmane.linux.network.general/14266 Thanks, -Ben
RE: [Qemu-devel] [PATCH 1/2] virtio-net: fix cross-endianness support
> -Original Message- > From: qemu-devel-bounces+yu.liu=freescale@nongnu.org > [mailto:qemu-devel-bounces+yu.liu=freescale@nongnu.org] > On Behalf Of Aurelien Jarno > Sent: Sunday, January 16, 2011 3:28 AM > To: qemu-devel@nongnu.org > Cc: Anthony Liguori; Aurelien Jarno > Subject: [Qemu-devel] [PATCH 1/2] virtio-net: fix > cross-endianness support > > virtio-net used to work on cross-endianness configurations, > but doesn't > anymore with recent guest kernels, as the new features don't handle > endianness correctly. > > This patch fixes wrong conversion, and add missing ones to make > virtio-net working. Tested on the following configurations: > - i386 guest on x86_64 host > - ppc guest on x86_64 host > - i386 guest on mips host > - ppc guest on mips host > > Cc: Anthony Liguori > Signed-off-by: Aurelien Jarno > --- > hw/virtio-net.c | 13 +++-- > 1 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > index ec1bf8d..515fb19 100644 > --- a/hw/virtio-net.c > +++ b/hw/virtio-net.c > @@ -81,7 +81,7 @@ static void > virtio_net_get_config(VirtIODevice *vdev, uint8_t *config) > VirtIONet *n = to_virtio_net(vdev); > struct virtio_net_config netcfg; > > -netcfg.status = n->status; > +netcfg.status = lduw_p(&n->status); > memcpy(netcfg.mac, n->mac, ETH_ALEN); > memcpy(config, &netcfg, sizeof(netcfg)); > } > @@ -340,7 +340,7 @@ static int > virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > n->mac_table.multi_overflow = 0; > memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN); > > -mac_data.entries = ldl_le_p(elem->out_sg[1].iov_base); > +mac_data.entries = ldl_p(elem->out_sg[1].iov_base); > > if (sizeof(mac_data.entries) + > (mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len) > @@ -356,7 +356,7 @@ static int > virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > n->mac_table.first_multi = n->mac_table.in_use; > > -mac_data.entries = ldl_le_p(elem->out_sg[2].iov_base); > +mac_data.entries = ldl_p(elem->out_sg[2].iov_base); > Hello Aurelien, Not clear what is happen, but this commit break e500(PowerPC big endian) kvm. Looks like e500 is happy to use ldl_le_p(), but this patch change it to use ldl_be_p(). Any thoughts about that? Thanks, Yu
[Qemu-devel] Re: [PATCH] Split machine creation from the main loop
On 02/28/2011 09:20 AM, Avi Kivity wrote: We should have another abstraction for connection based backend. I'll take a go at this when I'm ready to try to get those patches in. Shouldn't each new connection return a chardev? You would need a kind of "factory" interface that knows how to create a new {monitor,serial port,you name it} for each new connection. Actually it doesn't make much sense except for monitors. Paolo
Re: [Qemu-devel] Re: [patch 2/3] Add support for live block copy
On 02/27/2011 07:25 PM, Anthony Liguori wrote: On 02/27/2011 10:02 AM, Dor Laor wrote: On 02/27/2011 03:49 PM, Anthony Liguori wrote: On 02/27/2011 03:55 AM, Dor Laor wrote: What about a simpler approach were QMP events will be written to a event-log-file (or even named pipe). The management tool can just use a small daemon that does nothing other than write QMP events to a log. There's no real need for this code to live in QEMU. IIUC in case the management daemon will run qemu using named pipe for qmp it will happen automatically. No, the event model is changing (it was always intended to change though). Events will need explicit registration so it's necessary to have bidirectional communication. But the mgmt->qemu direction is only about event registration, it's not valuable info. It's a simpler approach than relaying on another config file and it is safer than having a separate mgmt daemon to log these events. Cross posting to libvirt-list to hear their view. So you can't just do -qmp foo -chardev file:event.log,id=foo. You won't actually see most of the events. Regards, Anthony Liguori If you agree to this approach it will simplify the more complex config file option (although it is nice to have as independent option for single hosts managed by simpler mgmts) Since events are posted, even if we wrote it in QEMU, the event wouldn't be guaranteed on disk by the time the event invocation returns. Regards, Anthony Liguori
[Qemu-devel] Re: GSoC 2011 project ideas
On 2011-02-28 00:44, Natalia Portillo wrote: > Hi there, > > El 23/02/2011, a las 20:42, Luiz Capitulino escribió: > >> Hi there, >> >> Google will begin accepting mentoring organizations applications next week, >> but >> we count only with three projects so far. >> >> Although there doesn't seem to exist a hard deadline associated with the >> ideas >> page, nor with the number of projects, we had more than 20 projects >> suggestions last year and a number of volunteering mentors. > > Is there any problem to repeat previously proposed and not chosen projects? > I don't think so, and I've just ported one of my old proposals to this year's page. I've also added some USB topics which may or may no longer apply to reality (specifically, I didn't find Gerd's EHCI branch to check its state). Gerd, once you are back, please have a look, correct or extend the list. If anyone else sees open topics in the USB area, feel free to contribute to the task lists as well. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v3 uq/master 08/22] remove CONFIG_THREAD
Signed-off-by: Paolo Bonzini --- configure |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 3036faf..4de26d2 100755 --- a/configure +++ b/configure @@ -2662,7 +2662,6 @@ if test "$vnc_png" != "no" ; then fi if test "$vnc_thread" != "no" ; then echo "CONFIG_VNC_THREAD=y" >> $config_host_mak - echo "CONFIG_THREAD=y" >> $config_host_mak fi if test "$fnmatch" = "yes" ; then echo "CONFIG_FNMATCH=y" >> $config_host_mak @@ -2758,7 +2757,6 @@ if test "$xen" = "yes" ; then fi if test "$io_thread" = "yes" ; then echo "CONFIG_IOTHREAD=y" >> $config_host_mak - echo "CONFIG_THREAD=y" >> $config_host_mak fi if test "$linux_aio" = "yes" ; then echo "CONFIG_LINUX_AIO=y" >> $config_host_mak -- 1.7.4
[Qemu-devel] Re: [PATCH] w32: Add support for curses
On 02/27/2011 08:23 PM, Stefan Weil wrote: MinGW optionally includes pdcurses, so add support for it. Signed-off-by: Stefan Weil --- configure |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/configure b/configure index 47779b6..ca15632 100755 --- a/configure +++ b/configure @@ -1562,7 +1562,11 @@ fi ## # curses probe -curses_list="-lncurses -lcurses" +if test "$mingw32" = "yes" ; then +curses_list="-lpdcurses" +else +curses_list="-lncurses -lcurses" +fi if test "$curses" != "no" ; then curses_found=no ACK Paolo
[Qemu-devel] [PATCH v3 uq/master 02/22] implement win32 dynticks timer
Signed-off-by: Paolo Bonzini --- qemu-timer.c |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/qemu-timer.c b/qemu-timer.c index 88c7b28..122e7ed 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -1004,6 +1004,7 @@ static void win32_stop_timer(struct qemu_alarm_timer *t) static void win32_rearm_timer(struct qemu_alarm_timer *t) { struct qemu_alarm_win32 *data = t->priv; +int nearest_delta_ms; assert(alarm_has_dynticks(t)); if (!active_timers[QEMU_CLOCK_REALTIME] && @@ -1013,7 +1014,11 @@ static void win32_rearm_timer(struct qemu_alarm_timer *t) timeKillEvent(data->timerId); -data->timerId = timeSetEvent(1, +nearest_delta_ms = (qemu_next_alarm_deadline() + 99) / 100; +if (nearest_delta_ms < 1) { +nearest_delta_ms = 1; +} +data->timerId = timeSetEvent(nearest_delta_ms, data->period, host_alarm_handler, (DWORD)t, -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 05/22] add win32 qemu-thread implementation
For now, qemu_cond_timedwait and qemu_mutex_timedlock are left as POSIX-only functions. They can be removed later, once the patches that remove their uses are in. Signed-off-by: Paolo Bonzini --- Makefile.objs|4 +- qemu-thread.c => qemu-thread-posix.c |0 qemu-thread-posix.h | 18 +++ qemu-thread-win32.c | 260 ++ qemu-thread-win32.h | 21 +++ qemu-thread.h| 27 ++-- 6 files changed, 313 insertions(+), 17 deletions(-) rename qemu-thread.c => qemu-thread-posix.c (100%) create mode 100644 qemu-thread-posix.h create mode 100644 qemu-thread-win32.c create mode 100644 qemu-thread-win32.h diff --git a/Makefile.objs b/Makefile.objs index 9e98a66..a52f42f 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -142,8 +142,8 @@ endif common-obj-y += $(addprefix ui/, $(ui-obj-y)) common-obj-y += iov.o acl.o -common-obj-$(CONFIG_THREAD) += qemu-thread.o -common-obj-$(CONFIG_POSIX) += compatfd.o +common-obj-$(CONFIG_POSIX) += qemu-thread-posix.o compatfd.o +common-obj-$(CONFIG_WIN32) += qemu-thread-win32.o common-obj-y += notify.o event_notifier.o common-obj-y += qemu-timer.o qemu-timer-common.o diff --git a/qemu-thread.c b/qemu-thread-posix.c similarity index 100% rename from qemu-thread.c rename to qemu-thread-posix.c diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h new file mode 100644 index 000..7af371c --- /dev/null +++ b/qemu-thread-posix.h @@ -0,0 +1,18 @@ +#ifndef __QEMU_THREAD_POSIX_H +#define __QEMU_THREAD_POSIX_H 1 +#include "pthread.h" + +struct QemuMutex { +pthread_mutex_t lock; +}; + +struct QemuCond { +pthread_cond_t cond; +}; + +struct QemuThread { +pthread_t thread; +}; + +void qemu_thread_signal(QemuThread *thread, int sig); +#endif diff --git a/qemu-thread-win32.c b/qemu-thread-win32.c new file mode 100644 index 000..2edcb1a --- /dev/null +++ b/qemu-thread-win32.c @@ -0,0 +1,260 @@ +/* + * Win32 implementation for mutex/cond/thread functions + * + * Copyright Red Hat, Inc. 2010 + * + * Author: + * Paolo Bonzini + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ +#include "qemu-common.h" +#include "qemu-thread.h" +#include +#include +#include + +static void error_exit(int err, const char *msg) +{ +char *pstr; + +FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER, + NULL, err, 0, (LPTSTR)&pstr, 2, NULL); +fprintf(stderr, "qemu: %s: %s\n", msg, pstr); +LocalFree(pstr); +exit(1); +} + +void qemu_mutex_init(QemuMutex *mutex) +{ +mutex->owner = 0; +InitializeCriticalSection(&mutex->lock); +} + +void qemu_mutex_lock(QemuMutex *mutex) +{ +EnterCriticalSection(&mutex->lock); + +/* Win32 CRITICAL_SECTIONs are recursive. Assert that we're not + * using them as such. + */ +assert(mutex->owner == 0); +mutex->owner = GetCurrentThreadId(); +} + +int qemu_mutex_trylock(QemuMutex *mutex) +{ +int owned; + +owned = TryEnterCriticalSection(&mutex->lock); +if (owned) { +assert(mutex->owner == 0); +mutex->owner = GetCurrentThreadId(); +} +return !owned; +} + +void qemu_mutex_unlock(QemuMutex *mutex) +{ +assert(mutex->owner == GetCurrentThreadId()); +mutex->owner = 0; +LeaveCriticalSection(&mutex->lock); +} + +void qemu_cond_init(QemuCond *cond) +{ +memset(cond, 0, sizeof(*cond)); + +cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL); +if (!cond->sema) { +error_exit(GetLastError(), __func__); +} +cond->continue_event = CreateEvent(NULL,/* security */ + FALSE, /* auto-reset */ + FALSE, /* not signaled */ + NULL); /* name */ +if (!cond->continue_event) { +error_exit(GetLastError(), __func__); +} +} + +void qemu_cond_signal(QemuCond *cond) +{ +DWORD result; + +/* + * Signal only when there are waiters. cond->waiters is + * incremented by pthread_cond_wait under the external lock, + * so we are safe about that. + */ +if (cond->waiters == 0) { +return; +} + +/* + * Waiting threads decrement it outside the external lock, but + * only if another thread is executing pthread_cond_broadcast and + * has the mutex. So, it also cannot be decremented concurrently + * with this particular access. + */ +cond->target = cond->waiters - 1; +result = SignalObjectAndWait(cond->sema, cond->continue_event, + INFINITE, FALSE); +if (result == WAIT_ABANDONED || result == WAIT_FAILED) { +error_exit(GetLastError(), __func__); +} +} + +void qemu_cond_broadcast(QemuCond *cond) +{ +BOOLEAN result; +/* + * As in pthread_cond_signal, access
[Qemu-devel] Re: [PATCH] Split machine creation from the main loop
On 02/28/2011 10:57 AM, Paolo Bonzini wrote: On 02/28/2011 09:20 AM, Avi Kivity wrote: We should have another abstraction for connection based backend. I'll take a go at this when I'm ready to try to get those patches in. Shouldn't each new connection return a chardev? You would need a kind of "factory" interface that knows how to create a new {monitor,serial port,you name it} for each new connection. Actually it doesn't make much sense except for monitors. Monitors and vnc. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v3 uq/master 13/22] always signal pause_cond after stopping a VCPU
Signed-off-by: Paolo Bonzini --- cpus.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/cpus.c b/cpus.c index 785a104..6cfb45b 100644 --- a/cpus.c +++ b/cpus.c @@ -1012,8 +1012,10 @@ void qemu_notify_event(void) void cpu_stop_current(void) { if (cpu_single_env) { +cpu_single_env->stop = 0; cpu_single_env->stopped = 1; cpu_exit(cpu_single_env); +qemu_cond_signal(&qemu_pause_cond); } } -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 00/22] Win32 iothread support
After gathering the comments about the two series I sent separately, here is the full series for Win32 iothread support. It is based on master, but as it touches (mostly, indeed) OS-independent parts it is safer to get it in through uq/master. Patches 1 to 5 are generic Win32 improvements, including the qemu-thread implementation. To simplify the dependencies, I think it's better if this part is also routed through uq/master. Patches 6 to 8 are generic threading improvements, including using PTHREAD_MUTEX_ERRORCHECK as suggested by Jan. Patches 9 to 17 eliminate polling, replacing condition variable timedwait with wait. Patch 18 removes a redundant condition from the TCG cpu_exec_all function. Patches 19 to 21 add all necessary stubs to make iothread compile with Win32, except the IPI calls. These are provided by patch 22. Tested on Wine and Linux, not on "real" Windows. The series introduces a dependency on Windows 2K or newer. I don't think either 95/98/ME or Windows NT 3.x are reasonable host systems for QEMU, anyway. v1->v2 I incorporated all suggestions from Jan, including his renaming patch for qemu_*_is_self, and included Aurelien's sh4 tweak to cpu_halted. v2->v3 Fixed structure naming, renamed qemu_signalfd_init, dropped owner tracking in QemuMutex for now. Aurelien Jarno (1): target-sh4: move intr_at_halt out of cpu_halted() Jan Kiszka (1): Refactor thread retrieval and check Paolo Bonzini (20): unlock iothread during WaitForMultipleObjects implement win32 dynticks timer use win32 timer queues add win32 qemu-thread implementation include qemu-thread.h early add assertions on the owner of a QemuMutex remove CONFIG_THREAD inline cpu_halted into sole caller always qemu_cpu_kick after unhalting a cpu exit round-robin vcpu loop if cpu->stopped is true always signal pause_cond after stopping a VCPU do not use timedwait on qemu_halt_cond do not use timedwait on qemu_system_cond do not use timedwait on qemu_pause_cond do not use timedwait on qemu_cpu_cond iothread stops the vcpu thread via IPI move blocking of signals to qemu_signalfd_init provide dummy signal init functions for win32 protect qemu_cpu_kick_self for Win32 add Win32 IPI service Makefile.objs|4 +- configure|2 - cpu-exec.c |9 +- cpus.c | 298 +- exec.c |2 +- hw/ppc.c |2 + hw/sun4m.c | 10 +- hw/sun4u.c |4 +- os-win32.c |2 + qemu-common.h|2 +- qemu-thread.c => qemu-thread-posix.c | 38 +++-- qemu-thread-posix.h | 18 ++ qemu-thread-win32.c | 260 + qemu-thread-win32.h | 21 +++ qemu-thread.h| 31 ++-- qemu-timer.c | 89 +-- target-alpha/exec.h | 11 -- target-arm/exec.h| 13 -- target-cris/exec.h | 11 -- target-i386/exec.h | 12 -- target-i386/kvm.c|4 +- target-m68k/exec.h | 10 -- target-microblaze/exec.h | 11 -- target-mips/exec.h | 11 -- target-ppc/exec.h| 11 -- target-s390x/exec.h | 12 -- target-s390x/kvm.c |1 + target-sh4/cpu.h |2 +- target-sh4/exec.h| 11 -- target-sh4/helper.c |4 +- target-sh4/op_helper.c |1 + target-sparc/exec.h | 10 -- ui/vnc-jobs-async.c |2 +- 33 files changed, 562 insertions(+), 367 deletions(-) rename qemu-thread.c => qemu-thread-posix.c (81%) create mode 100644 qemu-thread-posix.h create mode 100644 qemu-thread-win32.c create mode 100644 qemu-thread-win32.h -- 1.7.4
[Qemu-devel] Re: [PATCH v3 uq/master 05/22] add win32 qemu-thread implementation
On Mon, Feb 28, 2011 at 9:10 AM, Paolo Bonzini wrote: > +static unsigned __stdcall win32_start_routine(void *arg) > +{ > + struct QemuThreadData data = *(struct QemuThreadData *) arg; > + QemuThread *thread = data.thread; > + > + free(arg); qemu_free(arg); Stefan
[Qemu-devel] [PATCH] simpletrace: Thread-safe tracing
Trace events outside the global mutex cannot be used with the simple trace backend since it is not thread-safe. There is no check to prevent them being enabled so people sometimes learn this the hard way. This patch restructures the simple trace backend with a ring buffer suitable for multiple concurrent writers. A writeout thread empties the trace buffer when threshold fill levels are reached. Should the writeout thread be unable to keep up with trace generation, records will simply be dropped. Each time events are dropped a special record is written to the trace file indicating how many events were dropped. The event ID is 0xfffe and its signature is dropped(uint32_t count). Signed-off-by: Stefan Hajnoczi --- v2: * Add 'dropped' event so we know when events were lost. docs/tracing.txt |5 - scripts/simpletrace.py |3 +- simpletrace.c | 309 simpletrace.h |8 ++ vl.c | 16 +-- 5 files changed, 219 insertions(+), 122 deletions(-) diff --git a/docs/tracing.txt b/docs/tracing.txt index a6cc56f..f15069c 100644 --- a/docs/tracing.txt +++ b/docs/tracing.txt @@ -141,11 +141,6 @@ source tree. It may not be as powerful as platform-specific or third-party trace backends but it is portable. This is the recommended trace backend unless you have specific needs for more advanced backends. -Warning: the "simple" backend is not thread-safe so only enable trace events -that are executed while the global mutex is held. Much of QEMU meets this -requirement but some utility functions like qemu_malloc() or thread-related -code cannot be safely traced using the "simple" backend. - Monitor commands * info trace diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py index 9fe3dda..2ad5699 100755 --- a/scripts/simpletrace.py +++ b/scripts/simpletrace.py @@ -16,6 +16,7 @@ import inspect header_event_id = 0x header_magic= 0xf2b177cb0aa429b4 header_version = 0 +dropped_event_id = 0xfffe trace_fmt = '=' trace_len = struct.calcsize(trace_fmt) @@ -28,7 +29,7 @@ def parse_events(fobj): """Extract argument names from a parameter list.""" return tuple(arg.split()[-1].lstrip('*') for arg in args.split(',')) -events = {} +events = {dropped_event_id: ('dropped', 'count')} event_num = 0 for line in fobj: m = event_re.match(line.strip()) diff --git a/simpletrace.c b/simpletrace.c index 9ea0d1f..9926ab3 100644 --- a/simpletrace.c +++ b/simpletrace.c @@ -12,6 +12,9 @@ #include #include #include +#include +#include +#include "qerror.h" #include "qemu-timer.h" #include "trace.h" @@ -24,6 +27,12 @@ /** Trace file version number, bump if format changes */ #define HEADER_VERSION 0 +/** Records were dropped event ID */ +#define DROPPED_EVENT_ID (~(uint64_t)0 - 1) + +/** Trace record is valid */ +#define TRACE_RECORD_VALID ((uint64_t)1 << 63) + /** Trace buffer entry */ typedef struct { uint64_t event; @@ -37,126 +46,135 @@ typedef struct { } TraceRecord; enum { -TRACE_BUF_LEN = 64 * 1024 / sizeof(TraceRecord), +TRACE_BUF_LEN = 4096, +TRACE_BUF_FLUSH_THRESHOLD = TRACE_BUF_LEN / 4, }; +/* + * Trace records are written out by a dedicated thread. The thread waits for + * records to become available, writes them out, and then waits again. + */ +static pthread_mutex_t trace_lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t trace_available_cond = PTHREAD_COND_INITIALIZER; +static pthread_cond_t trace_empty_cond = PTHREAD_COND_INITIALIZER; +static bool trace_available; +static bool trace_writeout_enabled; + static TraceRecord trace_buf[TRACE_BUF_LEN]; static unsigned int trace_idx; static FILE *trace_fp; static char *trace_file_name = NULL; -static bool trace_file_enabled = false; - -void st_print_trace_file_status(FILE *stream, int (*stream_printf)(FILE *stream, const char *fmt, ...)) -{ -stream_printf(stream, "Trace file \"%s\" %s.\n", - trace_file_name, trace_file_enabled ? "on" : "off"); -} - -static bool write_header(FILE *fp) -{ -static const TraceRecord header = { -.event = HEADER_EVENT_ID, -.timestamp_ns = HEADER_MAGIC, -.x1 = HEADER_VERSION, -}; - -return fwrite(&header, sizeof header, 1, fp) == 1; -} /** - * set_trace_file : To set the name of a trace file. - * @file : pointer to the name to be set. - * If NULL, set to the default name- set at config time. + * Read a trace record from the trace buffer + * + * @idx Trace buffer index + * @record Trace record to fill + * + * Returns false if the record is not valid. */ -bool st_set_trace_file(const char *file) +static bool get_trace_record(unsigned int idx, TraceRecord *record) { -st_set_trace_file_enabled(false); - -free(trace_file_name); - -if (!file) { -if (asprintf(&trace_file_name, CONFIG_TRA
[Qemu-devel] Re: [PATCH] simpletrace: Thread-safe tracing
On Sun, Feb 27, 2011 at 6:16 PM, Paolo Bonzini wrote: > On 02/27/2011 06:02 PM, Stefan Hajnoczi wrote: >> >> On Sun, Feb 27, 2011 at 3:13 PM, Paolo Bonzini >> wrote: >>> >>> On 02/27/2011 03:58 PM, Stefan Hajnoczi wrote: + * Trace records are written out by a dedicated thread. The thread waits for + * records to become available, writes them out, and then waits again. + */ +static pthread_mutex_t trace_lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t trace_available_cond = PTHREAD_COND_INITIALIZER; +static pthread_cond_t trace_empty_cond = PTHREAD_COND_INITIALIZER; +static bool trace_available; +static bool trace_writeout_enabled; >>> >>> Please use QemuThread. >> >> The tracing code itself should use avoid core QEMU code. Otherwise we >> can't trace QemuThread - we'd have an infinite loop. > > Hmm, right... they'll use stdio to trace Win32 then... :) I was actually > thinking more of the code duplication. > > But do you really need tracing at such a low level? I'd expect tracing > wrappers like qemu_lock_mutex_iothread, not mutexes in general. We can get away with it for some codepaths but it adds new constraints that are hard to check against. For example, simpletrace uses get_clock() but we need to limit ourselves as much as possible. If that function calls any other core QEMU code like qemu_malloc(), then we risk infinite recursion because qemu_malloc() has a useful trace event. Stefan
Re: [Qemu-devel] [PATCH v3 2/2] rtl8139: add vlan tag extraction
On 02/26/2011 08:40 AM, Benjamin Poirier wrote: Add support to the emulated hardware to extract vlan tags in packets going from the network to the guest. Signed-off-by: Benjamin Poirier Cc: Igor V. Kovalenko Cc: Jason Wang Cc: Michael S. Tsirkin -- AFAIK, extraction is optional to get vlans working. The driver requests rx detagging but should not assume that it was done. Under Linux, the mac layer will catch the vlan ethertype. I only added this part for completeness (to emulate the hardware more truthfully.) Linux driver use the NETIF_F_HW_VLAN_RX by default, so it's needed. --- hw/rtl8139.c | 89 +- 1 files changed, 63 insertions(+), 26 deletions(-) diff --git a/hw/rtl8139.c b/hw/rtl8139.c index 35ccd3d..f3aaebc 100644 --- a/hw/rtl8139.c +++ b/hw/rtl8139.c @@ -835,10 +835,11 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque; int size_ = buf_size + (dot1q_buf ? VLAN_HDR_LEN : 0); int size = size_; +const uint8_t *next_part; +size_t next_part_size; uint32_t packet_header = 0; -uint8_t buf1[60]; static const uint8_t broadcast_macaddr[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; @@ -950,21 +951,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, } } -/* if too small buffer, then expand it */ -if (size< MIN_BUF_SIZE) { -if (unlikely(dot1q_buf)) { -memcpy(buf1, buf, 2 * ETHER_ADDR_LEN); -memcpy(buf1 + 2 * ETHER_ADDR_LEN, dot1q_buf, VLAN_HDR_LEN); -memcpy(buf1 + 2 * ETHER_ADDR_LEN + VLAN_HDR_LEN, buf + 2 * -ETHER_ADDR_LEN, buf_size - 2 * ETHER_ADDR_LEN); -} else { -memcpy(buf1, buf, size); -} -memset(buf1 + size, 0, MIN_BUF_SIZE - size); -buf = buf1; -size = MIN_BUF_SIZE; -} - Since the your tx loopback changes depends on rx changes, I suggest you to put rx patch first which can also make review easier. if (rtl8139_cp_receiver_enabled(s)) { DEBUG_PRINT(("RTL8139: in C+ Rx mode \n")); @@ -1025,6 +1011,44 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, uint32_t rx_space = rxdw0& CP_RX_BUFFER_SIZE_MASK; +/* write VLAN info to descriptor variables */ +/* next_part starts right after the vlan header (if any), at the + * ethertype for the payload */ +next_part =&buf[ETHER_ADDR_LEN * 2]; +if (s->CpCmd& CPlusRxVLAN&& (dot1q_buf || be16_to_cpup((uint16_t *) +&buf[ETHER_ADDR_LEN * 2]) == ETHERTYPE_VLAN)) { +if (!dot1q_buf) { +/* the tag is in the buffer */ +dot1q_buf =&buf[ETHER_ADDR_LEN * 2]; +next_part += VLAN_HDR_LEN; +} +size -= VLAN_HDR_LEN; + +rxdw1&= ~CP_RX_VLAN_TAG_MASK; +/* BE + ~le_to_cpu()~ + cpu_to_le() = BE */ +rxdw1 |= CP_RX_TAVA | le16_to_cpup((uint16_t *) +&buf[ETHER_HDR_LEN]); + +DEBUG_PRINT(("RTL8139: C+ Rx mode : extracted vlan tag with tci: " +"%u\n", be16_to_cpup((uint16_t *)&buf[ETHER_HDR_LEN]))); +} else { +/* reset VLAN tag flag */ +rxdw1&= ~CP_RX_TAVA; +} +next_part_size = buf + buf_size - next_part; + +/* if too small buffer, then expand it */ +if (size< MIN_BUF_SIZE) { +size_t tmp_size = MIN_BUF_SIZE - ETHER_ADDR_LEN * 2; +uint8_t *tmp = alloca(tmp_size); + +memcpy(tmp, next_part, next_part_size); +memset(tmp + next_part_size, 0, tmp_size - next_part_size); +next_part = tmp; +next_part_size = tmp_size; +size = MIN_BUF_SIZE; +} + /* TODO: scatter the packet over available receive ring descriptors space */ if (size+4> rx_space) @@ -1049,14 +1073,11 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, if (unlikely(dot1q_buf)) { cpu_physical_memory_write(rx_addr, buf, 2 * ETHER_ADDR_LEN); val = rtl8139_crc32(0, buf, 2 * ETHER_ADDR_LEN); -cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN, dot1q_buf, -VLAN_HDR_LEN); val = rtl8139_crc32(val, dot1q_buf, VLAN_HDR_LEN); -cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN + -VLAN_HDR_LEN, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 * -ETHER_ADDR_LEN); -val = rtl8139_crc32(val, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 * -ETHER_ADDR_LEN); +cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN, next_part, +next_part_size); +val = rtl8139_crc32(val, buf + 2 * ETHER_ADDR_LEN, +next_part_size);
[Qemu-devel] [PATCH v3 uq/master 07/22] add assertions on the owner of a QemuMutex
These are already present in the Win32 implementation, add them to the pthread wrappers as well. Use PTHREAD_MUTEX_ERRORCHECK for mutex operations. Later we'll add tracking of the owner for cond_signal/broadcast. Signed-off-by: Paolo Bonzini --- qemu-thread-posix.c | 23 - 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c index e307773..a4c6e25 100644 --- a/qemu-thread-posix.c +++ b/qemu-thread-posix.c @@ -28,8 +31,12 @@ static void error_exit(int err, const char *msg) void qemu_mutex_init(QemuMutex *mutex) { int err; +pthread_mutexattr_t mutexattr; -err = pthread_mutex_init(&mutex->lock, NULL); +pthread_mutexattr_init(&mutexattr); +pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_ERRORCHECK); +err = pthread_mutex_init(&mutex->lock, &mutexattr); +pthread_mutexattr_destroy(&mutexattr); if (err) error_exit(err, __func__); } -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 18/22] iothread stops the vcpu thread via IPI
Signed-off-by: Paolo Bonzini --- cpus.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/cpus.c b/cpus.c index 4305184..32e9352 100644 --- a/cpus.c +++ b/cpus.c @@ -1086,9 +1086,11 @@ bool cpu_exec_all(void) qemu_clock_enable(vm_clock, (env->singlestep_enabled & SSTEP_NOTIMER) == 0); +#ifndef CONFIG_IOTHREAD if (qemu_alarm_pending()) { break; } +#endif if (cpu_can_run(env)) { if (kvm_enabled()) { r = kvm_cpu_exec(env); -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 04/22] Refactor thread retrieval and check
From: Jan Kiszka We have qemu_cpu_self and qemu_thread_self. The latter is retrieving the current thread, the former is checking for equality (using CPUState). We also have qemu_thread_equal which is only used like qemu_cpu_self. This refactors the interfaces, creating qemu_cpu_is_self and qemu_thread_is_self as well ass qemu_thread_get_self. Signed-off-by: Jan Kiszka Signed-off-by: Paolo Bonzini --- cpus.c | 22 -- exec.c |2 +- qemu-common.h |2 +- qemu-thread.c |6 +++--- qemu-thread.h |4 ++-- target-i386/kvm.c |4 ++-- ui/vnc-jobs-async.c |2 +- 7 files changed, 18 insertions(+), 24 deletions(-) diff --git a/cpus.c b/cpus.c index 0f33945..09ce6fe 100644 --- a/cpus.c +++ b/cpus.c @@ -531,7 +531,7 @@ void qemu_init_vcpu(void *_env) } } -int qemu_cpu_self(void *env) +int qemu_cpu_is_self(void *env) { return 1; } @@ -699,7 +699,7 @@ int qemu_init_main_loop(void) qemu_mutex_init(&qemu_global_mutex); qemu_mutex_lock(&qemu_global_mutex); -qemu_thread_self(&io_thread); +qemu_thread_get_self(&io_thread); return 0; } @@ -714,7 +714,7 @@ void run_on_cpu(CPUState *env, void (*func)(void *data), void *data) { struct qemu_work_item wi; -if (qemu_cpu_self(env)) { +if (qemu_cpu_is_self(env)) { func(data); return; } @@ -808,7 +808,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg) int r; qemu_mutex_lock(&qemu_global_mutex); -qemu_thread_self(env->thread); +qemu_thread_get_self(env->thread); r = kvm_init_vcpu(env); if (r < 0) { @@ -845,7 +845,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) CPUState *env = arg; qemu_tcg_init_cpu_signals(); -qemu_thread_self(env->thread); +qemu_thread_get_self(env->thread); /* signal CPU creation */ qemu_mutex_lock(&qemu_global_mutex); @@ -888,14 +888,11 @@ void qemu_cpu_kick_self(void) } } -int qemu_cpu_self(void *_env) +int qemu_cpu_is_self(void *_env) { CPUState *env = _env; -QemuThread this; -qemu_thread_self(&this); - -return qemu_thread_equal(&this, env->thread); +return qemu_thread_is_self(env->thread); } void qemu_mutex_lock_iothread(void) @@ -1023,10 +1020,7 @@ void cpu_stop_current(void) void vm_stop(int reason) { -QemuThread me; -qemu_thread_self(&me); - -if (!qemu_thread_equal(&me, &io_thread)) { +if (!qemu_thread_is_self(&io_thread)) { qemu_system_vmstop_request(reason); /* * FIXME: should not return to device code in case diff --git a/exec.c b/exec.c index d611100..439a6f6 100644 --- a/exec.c +++ b/exec.c @@ -1640,7 +1640,7 @@ void cpu_interrupt(CPUState *env, int mask) * If called from iothread context, wake the target cpu in * case its halted. */ -if (!qemu_cpu_self(env)) { +if (!qemu_cpu_is_self(env)) { qemu_cpu_kick(env); return; } diff --git a/qemu-common.h b/qemu-common.h index 40dad52..958a002 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -289,7 +289,7 @@ void qemu_notify_event(void); /* Unblock cpu */ void qemu_cpu_kick(void *env); void qemu_cpu_kick_self(void); -int qemu_cpu_self(void *env); +int qemu_cpu_is_self(void *env); /* work queue */ struct qemu_work_item { diff --git a/qemu-thread.c b/qemu-thread.c index fbc78fe..e307773 100644 --- a/qemu-thread.c +++ b/qemu-thread.c @@ -176,14 +176,14 @@ void qemu_thread_signal(QemuThread *thread, int sig) error_exit(err, __func__); } -void qemu_thread_self(QemuThread *thread) +void qemu_thread_get_self(QemuThread *thread) { thread->thread = pthread_self(); } -int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2) +int qemu_thread_is_self(QemuThread *thread) { - return pthread_equal(thread1->thread, thread2->thread); + return pthread_equal(pthread_self(), thread->thread); } void qemu_thread_exit(void *retval) diff --git a/qemu-thread.h b/qemu-thread.h index 19bb30c..add97bf 100644 --- a/qemu-thread.h +++ b/qemu-thread.h @@ -37,8 +37,8 @@ void qemu_thread_create(QemuThread *thread, void *(*start_routine)(void*), void *arg); void qemu_thread_signal(QemuThread *thread, int sig); -void qemu_thread_self(QemuThread *thread); -int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2); +void qemu_thread_get_self(QemuThread *thread); +int qemu_thread_is_self(QemuThread *thread); void qemu_thread_exit(void *retval); #endif diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 0aa0a41..1f12cbf 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -1355,7 +1355,7 @@ int kvm_arch_put_registers(CPUState *env, int level) { int ret; -assert(cpu_is_stopped(env) || qemu_cpu_self(env)); +assert(cpu_is_stopped(env) || qemu_cpu_is_self(env)); ret = kvm_getput_regs(env, 1); if (ret < 0) { @@ -1403,7 +1403,7 @@ int kvm_arch_g
[Qemu-devel] [PATCH v3 uq/master 06/22] include qemu-thread.h early
Signed-off-by: Paolo Bonzini --- cpus.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/cpus.c b/cpus.c index 09ce6fe..c5743c4 100644 --- a/cpus.c +++ b/cpus.c @@ -32,6 +32,7 @@ #include "kvm.h" #include "exec-all.h" +#include "qemu-thread.h" #include "cpus.h" #include "compatfd.h" @@ -592,8 +593,6 @@ void vm_stop(int reason) #else /* CONFIG_IOTHREAD */ -#include "qemu-thread.h" - QemuMutex qemu_global_mutex; static QemuMutex qemu_fair_mutex; -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 16/22] do not use timedwait on qemu_pause_cond
all_vcpus_paused can start returning true after penv->stopped changes from 0 to 1. When this is done, qemu_pause_cond is always signaled. Signed-off-by: Paolo Bonzini --- cpus.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/cpus.c b/cpus.c index e367b3b..8f169ad 100644 --- a/cpus.c +++ b/cpus.c @@ -938,7 +938,7 @@ void pause_all_vcpus(void) } while (!all_vcpus_paused()) { -qemu_cond_timedwait(&qemu_pause_cond, &qemu_global_mutex, 100); +qemu_cond_wait(&qemu_pause_cond, &qemu_global_mutex); penv = first_cpu; while (penv) { qemu_cpu_kick(penv); -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 19/22] merge all signal initialization with qemu_signalfd_init, rename
Signed-off-by: Paolo Bonzini --- cpus.c | 87 ++- 1 files changed, 36 insertions(+), 51 deletions(-) diff --git a/cpus.c b/cpus.c index 32e9352..2b491a9 100644 --- a/cpus.c +++ b/cpus.c @@ -346,11 +346,37 @@ static void sigfd_handler(void *opaque) } } -static int qemu_signalfd_init(sigset_t mask) +static int qemu_signal_init(void) { int sigfd; +sigset_t set; -sigfd = qemu_signalfd(&mask); +#ifdef CONFIG_IOTHREAD +/* SIGUSR2 used by posix-aio-compat.c */ +sigemptyset(&set); +sigaddset(&set, SIGUSR2); +pthread_sigmask(SIG_UNBLOCK, &set, NULL); + +sigemptyset(&set); +sigaddset(&set, SIGIO); +sigaddset(&set, SIGALRM); +sigaddset(&set, SIG_IPI); +sigaddset(&set, SIGBUS); +pthread_sigmask(SIG_BLOCK, &set, NULL); +#else +sigemptyset(&set); +sigaddset(&set, SIGBUS); +if (kvm_enabled()) { +/* + * We need to process timer signals synchronously to avoid a race + * between exit_request check and KVM vcpu entry. + */ +sigaddset(&set, SIGIO); +sigaddset(&set, SIGALRM); +} +#endif + +sigfd = qemu_signalfd(&set); if (sigfd == -1) { fprintf(stderr, "failed to create signalfd\n"); return -errno; @@ -438,6 +464,12 @@ static void qemu_event_increment(void) static void qemu_kvm_eat_signals(CPUState *env) { } + +static int qemu_signal_init(void) +{ +return 0; +} + #endif /* _WIN32 */ #ifndef CONFIG_IOTHREAD @@ -471,39 +503,14 @@ static void qemu_kvm_init_cpu_signals(CPUState *env) #endif } -#ifndef _WIN32 -static sigset_t block_synchronous_signals(void) -{ -sigset_t set; - -sigemptyset(&set); -sigaddset(&set, SIGBUS); -if (kvm_enabled()) { -/* - * We need to process timer signals synchronously to avoid a race - * between exit_request check and KVM vcpu entry. - */ -sigaddset(&set, SIGIO); -sigaddset(&set, SIGALRM); -} - -return set; -} -#endif - int qemu_init_main_loop(void) { -#ifndef _WIN32 -sigset_t blocked_signals; int ret; -blocked_signals = block_synchronous_signals(); - -ret = qemu_signalfd_init(blocked_signals); +ret = qemu_signal_init(); if (ret) { return ret; } -#endif qemu_init_sigbus(); @@ -651,35 +658,13 @@ static void qemu_tcg_init_cpu_signals(void) pthread_sigmask(SIG_UNBLOCK, &set, NULL); } -static sigset_t block_io_signals(void) -{ -sigset_t set; - -/* SIGUSR2 used by posix-aio-compat.c */ -sigemptyset(&set); -sigaddset(&set, SIGUSR2); -pthread_sigmask(SIG_UNBLOCK, &set, NULL); - -sigemptyset(&set); -sigaddset(&set, SIGIO); -sigaddset(&set, SIGALRM); -sigaddset(&set, SIG_IPI); -sigaddset(&set, SIGBUS); -pthread_sigmask(SIG_BLOCK, &set, NULL); - -return set; -} - int qemu_init_main_loop(void) { int ret; -sigset_t blocked_signals; qemu_init_sigbus(); -blocked_signals = block_io_signals(); - -ret = qemu_signalfd_init(blocked_signals); +ret = qemu_signal_init(); if (ret) { return ret; } -- 1.7.4
[Qemu-devel] Re: [PATCH] Split machine creation from the main loop
On 02/28/2011 10:13 AM, Avi Kivity wrote: You would need a kind of "factory" interface that knows how to create a new {monitor,serial port,you name it} for each new connection. Actually it doesn't make much sense except for monitors. Monitors and vnc. True, but vnc is not a chardev and it probably wouldn't make much sense for it to be one. Actually, it may make some sense for virtio-serial... Paolo
[Qemu-devel] Re: [PATCH v3 uq/master 00/22] Win32 iothread support
On 02/28/2011 10:16 AM, Avi Kivity wrote: Why is that? uq/master is for kvm code, and this touches two or three lines? iothread code is also going in via uq/master often. I started with uq/master because I depended on Jan's changes which are now upstream, I have no problems with this going directly to qemu.git.
[Qemu-devel] [PATCH v3 uq/master 17/22] do not use timedwait on qemu_cpu_cond
Whenever env->created becomes true, qemu_cpu_cond is signaled by {kvm,tcg}_cpu_thread_fn. Signed-off-by: Paolo Bonzini --- cpus.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpus.c b/cpus.c index 8f169ad..4305184 100644 --- a/cpus.c +++ b/cpus.c @@ -970,7 +970,7 @@ static void qemu_tcg_init_vcpu(void *_env) qemu_cond_init(env->halt_cond); qemu_thread_create(env->thread, qemu_tcg_cpu_thread_fn, env); while (env->created == 0) { -qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100); +qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex); } tcg_cpu_thread = env->thread; tcg_halt_cond = env->halt_cond; @@ -987,7 +987,7 @@ static void qemu_kvm_start_vcpu(CPUState *env) qemu_cond_init(env->halt_cond); qemu_thread_create(env->thread, qemu_kvm_cpu_thread_fn, env); while (env->created == 0) { -qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100); +qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex); } } -- 1.7.4
[Qemu-devel] [PATCH v4 uq/master] add win32 qemu-thread implementation
For now, qemu_cond_timedwait and qemu_mutex_timedlock are left as POSIX-only functions. They can be removed later, once the patches that remove their uses are in. Signed-off-by: Paolo Bonzini --- Makefile.objs|4 +- qemu-thread.c => qemu-thread-posix.c |0 qemu-thread-posix.h | 18 +++ qemu-thread-win32.c | 260 ++ qemu-thread-win32.h | 21 +++ qemu-thread.h| 27 ++-- 6 files changed, 313 insertions(+), 17 deletions(-) rename qemu-thread.c => qemu-thread-posix.c (100%) create mode 100644 qemu-thread-posix.h create mode 100644 qemu-thread-win32.c create mode 100644 qemu-thread-win32.h diff --git a/Makefile.objs b/Makefile.objs index 9e98a66..a52f42f 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -142,8 +142,8 @@ endif common-obj-y += $(addprefix ui/, $(ui-obj-y)) common-obj-y += iov.o acl.o -common-obj-$(CONFIG_THREAD) += qemu-thread.o -common-obj-$(CONFIG_POSIX) += compatfd.o +common-obj-$(CONFIG_POSIX) += qemu-thread-posix.o compatfd.o +common-obj-$(CONFIG_WIN32) += qemu-thread-win32.o common-obj-y += notify.o event_notifier.o common-obj-y += qemu-timer.o qemu-timer-common.o diff --git a/qemu-thread.c b/qemu-thread-posix.c similarity index 100% rename from qemu-thread.c rename to qemu-thread-posix.c diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h new file mode 100644 index 000..7af371c --- /dev/null +++ b/qemu-thread-posix.h @@ -0,0 +1,18 @@ +#ifndef __QEMU_THREAD_POSIX_H +#define __QEMU_THREAD_POSIX_H 1 +#include "pthread.h" + +struct QemuMutex { +pthread_mutex_t lock; +}; + +struct QemuCond { +pthread_cond_t cond; +}; + +struct QemuThread { +pthread_t thread; +}; + +void qemu_thread_signal(QemuThread *thread, int sig); +#endif diff --git a/qemu-thread-win32.c b/qemu-thread-win32.c new file mode 100644 index 000..7c14cc4 --- /dev/null +++ b/qemu-thread-win32.c @@ -0,0 +1,260 @@ +/* + * Win32 implementation for mutex/cond/thread functions + * + * Copyright Red Hat, Inc. 2010 + * + * Author: + * Paolo Bonzini + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ +#include "qemu-common.h" +#include "qemu-thread.h" +#include +#include +#include + +static void error_exit(int err, const char *msg) +{ +char *pstr; + +FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER, + NULL, err, 0, (LPTSTR)&pstr, 2, NULL); +fprintf(stderr, "qemu: %s: %s\n", msg, pstr); +LocalFree(pstr); +exit(1); +} + +void qemu_mutex_init(QemuMutex *mutex) +{ +mutex->owner = 0; +InitializeCriticalSection(&mutex->lock); +} + +void qemu_mutex_lock(QemuMutex *mutex) +{ +EnterCriticalSection(&mutex->lock); + +/* Win32 CRITICAL_SECTIONs are recursive. Assert that we're not + * using them as such. + */ +assert(mutex->owner == 0); +mutex->owner = GetCurrentThreadId(); +} + +int qemu_mutex_trylock(QemuMutex *mutex) +{ +int owned; + +owned = TryEnterCriticalSection(&mutex->lock); +if (owned) { +assert(mutex->owner == 0); +mutex->owner = GetCurrentThreadId(); +} +return !owned; +} + +void qemu_mutex_unlock(QemuMutex *mutex) +{ +assert(mutex->owner == GetCurrentThreadId()); +mutex->owner = 0; +LeaveCriticalSection(&mutex->lock); +} + +void qemu_cond_init(QemuCond *cond) +{ +memset(cond, 0, sizeof(*cond)); + +cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL); +if (!cond->sema) { +error_exit(GetLastError(), __func__); +} +cond->continue_event = CreateEvent(NULL,/* security */ + FALSE, /* auto-reset */ + FALSE, /* not signaled */ + NULL); /* name */ +if (!cond->continue_event) { +error_exit(GetLastError(), __func__); +} +} + +void qemu_cond_signal(QemuCond *cond) +{ +DWORD result; + +/* + * Signal only when there are waiters. cond->waiters is + * incremented by pthread_cond_wait under the external lock, + * so we are safe about that. + */ +if (cond->waiters == 0) { +return; +} + +/* + * Waiting threads decrement it outside the external lock, but + * only if another thread is executing pthread_cond_broadcast and + * has the mutex. So, it also cannot be decremented concurrently + * with this particular access. + */ +cond->target = cond->waiters - 1; +result = SignalObjectAndWait(cond->sema, cond->continue_event, + INFINITE, FALSE); +if (result == WAIT_ABANDONED || result == WAIT_FAILED) { +error_exit(GetLastError(), __func__); +} +} + +void qemu_cond_broadcast(QemuCond *cond) +{ +BOOLEAN result; +/* + * As in pthread_cond_signal, access
[Qemu-devel] [PATCH v3 uq/master 09/22] target-sh4: move intr_at_halt out of cpu_halted()
From: Aurelien Jarno All targets except SH4 have the same cpu_halted() routine, and it has only one caller. It is therefore a good candidate for inlining. The difference is the handling of the intr_at_halt, which is necessary to ignore SR.BL when sleeping. Move intr_at_halt handling out of it, by setting this variable while executing the sleep instruction, and clearing it when the CPU has been woken-up by an interrupt, whatever the state of SR.BL. Also rename this variable in_sleep. Cc: Paolo Bonzini Signed-off-by: Aurelien Jarno --- target-sh4/cpu.h |2 +- target-sh4/exec.h |1 - target-sh4/helper.c|4 ++-- target-sh4/op_helper.c |1 + 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h index 789d188..74ff97a 100644 --- a/target-sh4/cpu.h +++ b/target-sh4/cpu.h @@ -184,7 +184,7 @@ typedef struct CPUSH4State { uint32_t cvr; /* Cache Version Register */ void *intc_handle; -int intr_at_halt; /* SR_BL ignored during sleep */ +int in_sleep; /* SR_BL ignored during sleep */ memory_content *movcal_backup; memory_content **movcal_backup_tail; } CPUSH4State; diff --git a/target-sh4/exec.h b/target-sh4/exec.h index 2999c02..61bc121 100644 --- a/target-sh4/exec.h +++ b/target-sh4/exec.h @@ -37,7 +37,6 @@ static inline int cpu_halted(CPUState *env) { return 0; if (cpu_has_work(env)) { env->halted = 0; -env->intr_at_halt = 1; return 0; } return EXCP_HALTED; diff --git a/target-sh4/helper.c b/target-sh4/helper.c index d2038bd..8f36d31 100644 --- a/target-sh4/helper.c +++ b/target-sh4/helper.c @@ -90,11 +90,11 @@ void do_interrupt(CPUState * env) if (do_exp && env->exception_index != 0x1e0) { env->exception_index = 0x000; /* masked exception -> reset */ } -if (do_irq && !env->intr_at_halt) { +if (do_irq && !env->in_sleep) { return; /* masked */ } -env->intr_at_halt = 0; } +env->in_sleep = 0; if (do_irq) { irq_vector = sh_intc_get_pending_vector(env->intc_handle, diff --git a/target-sh4/op_helper.c b/target-sh4/op_helper.c index 30f9842..b8f4ca2 100644 --- a/target-sh4/op_helper.c +++ b/target-sh4/op_helper.c @@ -119,6 +119,7 @@ void helper_debug(void) void helper_sleep(uint32_t next_pc) { env->halted = 1; +env->in_sleep = 1; env->exception_index = EXCP_HLT; env->pc = next_pc; cpu_loop_exit(); -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 12/22] exit round-robin vcpu loop if cpu->stopped is true
Sometimes vcpus are stopped directly without going through ->stop = 1. Exit the VCPU execution loop in this case as well. Signed-off-by: Paolo Bonzini --- cpus.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/cpus.c b/cpus.c index c5743c4..785a104 100644 --- a/cpus.c +++ b/cpus.c @@ -1098,7 +1098,7 @@ bool cpu_exec_all(void) cpu_handle_debug_exception(env); break; } -} else if (env->stop) { +} else if (env->stop || env->stopped) { break; } } -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 14/22] do not use timedwait on qemu_halt_cond
The following conditions can cause cpu_has_work(env) to become true: - env->queued_work_first: run_on_cpu is already kicking the VCPU - env->stop = 1: pause_all_vcpus is already kicking the VCPU - env->stopped = 0: resume_all_vcpus is already kicking the VCPU - vm_running = 1: vm_start is calling resume_all_vcpus - env->halted = 0: see previous patch - qemu_cpu_has_work(env): when it becomes true, board code should set env->halted = 0 too. Signed-off-by: Paolo Bonzini --- cpus.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpus.c b/cpus.c index 6cfb45b..4c3837f 100644 --- a/cpus.c +++ b/cpus.c @@ -771,7 +771,7 @@ static void qemu_tcg_wait_io_event(void) CPUState *env; while (all_cpu_threads_idle()) { -qemu_cond_timedwait(tcg_halt_cond, &qemu_global_mutex, 1000); +qemu_cond_wait(tcg_halt_cond, &qemu_global_mutex); } qemu_mutex_unlock(&qemu_global_mutex); @@ -794,7 +794,7 @@ static void qemu_tcg_wait_io_event(void) static void qemu_kvm_wait_io_event(CPUState *env) { while (cpu_thread_is_idle(env)) { -qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000); +qemu_cond_wait(env->halt_cond, &qemu_global_mutex); } qemu_kvm_eat_signals(env); -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 21/22] protect qemu_cpu_kick_self for Win32
Signed-off-by: Paolo Bonzini --- cpus.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/cpus.c b/cpus.c index e0bcb5c..7559a02 100644 --- a/cpus.c +++ b/cpus.c @@ -867,12 +867,16 @@ void qemu_cpu_kick(void *_env) void qemu_cpu_kick_self(void) { +#ifndef _WIN32 assert(cpu_single_env); if (!cpu_single_env->thread_kicked) { qemu_thread_signal(cpu_single_env->thread, SIG_IPI); cpu_single_env->thread_kicked = true; } +#else +abort(); +#endif } int qemu_cpu_is_self(void *_env) -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 20/22] provide dummy signal init functions for win32
Signed-off-by: Paolo Bonzini --- cpus.c | 143 --- 1 files changed, 73 insertions(+), 70 deletions(-) diff --git a/cpus.c b/cpus.c index 2b491a9..e0bcb5c 100644 --- a/cpus.c +++ b/cpus.c @@ -196,6 +196,16 @@ static void cpu_handle_debug_exception(CPUState *env) #endif } +#ifdef CONFIG_IOTHREAD +static void cpu_signal(int sig) +{ +if (cpu_single_env) { +cpu_exit(cpu_single_env); +} +exit_request = 1; +} +#endif + #ifdef CONFIG_LINUX static void sigbus_reraise(void) { @@ -390,6 +400,61 @@ static int qemu_signal_init(void) return 0; } +static void qemu_kvm_init_cpu_signals(CPUState *env) +{ +int r; +sigset_t set; +struct sigaction sigact; + +memset(&sigact, 0, sizeof(sigact)); +sigact.sa_handler = dummy_signal; +sigaction(SIG_IPI, &sigact, NULL); + +#ifdef CONFIG_IOTHREAD +pthread_sigmask(SIG_BLOCK, NULL, &set); +sigdelset(&set, SIG_IPI); +sigdelset(&set, SIGBUS); +r = kvm_set_signal_mask(env, &set); +if (r) { +fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); +exit(1); +} +#else +sigemptyset(&set); +sigaddset(&set, SIG_IPI); +sigaddset(&set, SIGIO); +sigaddset(&set, SIGALRM); +pthread_sigmask(SIG_BLOCK, &set, NULL); + +pthread_sigmask(SIG_BLOCK, NULL, &set); +sigdelset(&set, SIGIO); +sigdelset(&set, SIGALRM); +#endif +sigdelset(&set, SIG_IPI); +sigdelset(&set, SIGBUS); +r = kvm_set_signal_mask(env, &set); +if (r) { +fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); +exit(1); +} +} + +static void qemu_tcg_init_cpu_signals(void) +{ +#ifdef CONFIG_IOTHREAD +sigset_t set; +struct sigaction sigact; + +memset(&sigact, 0, sizeof(sigact)); +sigact.sa_handler = cpu_signal; +sigaction(SIG_IPI, &sigact, NULL); + +sigemptyset(&set); +sigaddset(&set, SIG_IPI); +pthread_sigmask(SIG_UNBLOCK, &set, NULL); +#endif +} + static void qemu_kvm_eat_signals(CPUState *env) { struct timespec ts = { 0, 0 }; @@ -470,39 +535,17 @@ static int qemu_signal_init(void) return 0; } -#endif /* _WIN32 */ - -#ifndef CONFIG_IOTHREAD static void qemu_kvm_init_cpu_signals(CPUState *env) { -#ifndef _WIN32 -int r; -sigset_t set; -struct sigaction sigact; - -memset(&sigact, 0, sizeof(sigact)); -sigact.sa_handler = dummy_signal; -sigaction(SIG_IPI, &sigact, NULL); - -sigemptyset(&set); -sigaddset(&set, SIG_IPI); -sigaddset(&set, SIGIO); -sigaddset(&set, SIGALRM); -pthread_sigmask(SIG_BLOCK, &set, NULL); +abort(); +} -pthread_sigmask(SIG_BLOCK, NULL, &set); -sigdelset(&set, SIG_IPI); -sigdelset(&set, SIGBUS); -sigdelset(&set, SIGIO); -sigdelset(&set, SIGALRM); -r = kvm_set_signal_mask(env, &set); -if (r) { -fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); -exit(1); -} -#endif +static void qemu_tcg_init_cpu_signals(void) +{ } +#endif /* _WIN32 */ +#ifndef CONFIG_IOTHREAD int qemu_init_main_loop(void) { int ret; @@ -536,6 +579,8 @@ void qemu_init_vcpu(void *_env) exit(1); } qemu_kvm_init_cpu_signals(env); +} else { +qemu_tcg_init_cpu_signals(); } } @@ -616,48 +661,6 @@ static QemuCond qemu_system_cond; static QemuCond qemu_pause_cond; static QemuCond qemu_work_cond; -static void cpu_signal(int sig) -{ -if (cpu_single_env) { -cpu_exit(cpu_single_env); -} -exit_request = 1; -} - -static void qemu_kvm_init_cpu_signals(CPUState *env) -{ -int r; -sigset_t set; -struct sigaction sigact; - -memset(&sigact, 0, sizeof(sigact)); -sigact.sa_handler = dummy_signal; -sigaction(SIG_IPI, &sigact, NULL); - -pthread_sigmask(SIG_BLOCK, NULL, &set); -sigdelset(&set, SIG_IPI); -sigdelset(&set, SIGBUS); -r = kvm_set_signal_mask(env, &set); -if (r) { -fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); -exit(1); -} -} - -static void qemu_tcg_init_cpu_signals(void) -{ -sigset_t set; -struct sigaction sigact; - -memset(&sigact, 0, sizeof(sigact)); -sigact.sa_handler = cpu_signal; -sigaction(SIG_IPI, &sigact, NULL); - -sigemptyset(&set); -sigaddset(&set, SIG_IPI); -pthread_sigmask(SIG_UNBLOCK, &set, NULL); -} - int qemu_init_main_loop(void) { int ret; -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 03/22] use win32 timer queues
Multimedia timers are only useful for compatibility with Windows NT 4.0 and earlier. Plus, the implementation in Wine is extremely heavyweight. Signed-off-by: Paolo Bonzini --- qemu-timer.c | 86 +++-- 1 files changed, 35 insertions(+), 51 deletions(-) diff --git a/qemu-timer.c b/qemu-timer.c index 122e7ed..1939d6b 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -200,11 +200,6 @@ static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t) #ifdef _WIN32 -struct qemu_alarm_win32 { -MMRESULT timerId; -unsigned int period; -} alarm_win32_data = {0, 0}; - static int win32_start_timer(struct qemu_alarm_timer *t); static void win32_stop_timer(struct qemu_alarm_timer *t); static void win32_rearm_timer(struct qemu_alarm_timer *t); @@ -298,9 +293,9 @@ static struct qemu_alarm_timer alarm_timers[] = { {"unix", unix_start_timer, unix_stop_timer, NULL, NULL}, #else {"dynticks", win32_start_timer, - win32_stop_timer, win32_rearm_timer, &alarm_win32_data}, + win32_stop_timer, win32_rearm_timer, NULL}, {"win32", win32_start_timer, - win32_stop_timer, NULL, &alarm_win32_data}, + win32_stop_timer, NULL, NULL}, #endif {NULL, } }; @@ -636,9 +631,7 @@ void qemu_run_all_timers(void) static int64_t qemu_next_alarm_deadline(void); #ifdef _WIN32 -static void CALLBACK host_alarm_handler(UINT uTimerID, UINT uMsg, -DWORD_PTR dwUser, DWORD_PTR dw1, -DWORD_PTR dw2) +static void CALLBACK host_alarm_handler(PVOID lpParam, BOOLEAN unused) #else static void host_alarm_handler(int host_signum) #endif @@ -961,50 +954,45 @@ static void unix_stop_timer(struct qemu_alarm_timer *t) static int win32_start_timer(struct qemu_alarm_timer *t) { -TIMECAPS tc; -struct qemu_alarm_win32 *data = t->priv; -UINT flags; - -memset(&tc, 0, sizeof(tc)); -timeGetDevCaps(&tc, sizeof(tc)); - -data->period = tc.wPeriodMin; -timeBeginPeriod(data->period); - -flags = TIME_CALLBACK_FUNCTION; -if (alarm_has_dynticks(t)) -flags |= TIME_ONESHOT; -else -flags |= TIME_PERIODIC; - -data->timerId = timeSetEvent(1, // interval (ms) -data->period, // resolution -host_alarm_handler, // function -(DWORD)t, // parameter -flags); - -if (!data->timerId) { +HANDLE hTimer; +BOOLEAN success; + +/* If you call ChangeTimerQueueTimer on a one-shot timer (its period + is zero) that has already expired, the timer is not updated. Since + creating a new timer is relatively expensive, set a bogus one-hour + interval in the dynticks case. */ +success = CreateTimerQueueTimer(&hTimer, + NULL, + host_alarm_handler, + t, + 1, + alarm_has_dynticks(t) ? 360 : 1, + WT_EXECUTEINTIMERTHREAD); + +if (!success) { fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n", GetLastError()); -timeEndPeriod(data->period); return -1; } +t->priv = (PVOID) hTimer; return 0; } static void win32_stop_timer(struct qemu_alarm_timer *t) { -struct qemu_alarm_win32 *data = t->priv; +HANDLE hTimer = t->priv; -timeKillEvent(data->timerId); -timeEndPeriod(data->period); +if (hTimer) { +DeleteTimerQueueTimer(NULL, hTimer, NULL); +} } static void win32_rearm_timer(struct qemu_alarm_timer *t) { -struct qemu_alarm_win32 *data = t->priv; +HANDLE hTimer = t->priv; int nearest_delta_ms; +BOOLEAN success; assert(alarm_has_dynticks(t)); if (!active_timers[QEMU_CLOCK_REALTIME] && @@ -1012,25 +1000,21 @@ static void win32_rearm_timer(struct qemu_alarm_timer *t) !active_timers[QEMU_CLOCK_HOST]) return; -timeKillEvent(data->timerId); - nearest_delta_ms = (qemu_next_alarm_deadline() + 99) / 100; if (nearest_delta_ms < 1) { nearest_delta_ms = 1; } -data->timerId = timeSetEvent(nearest_delta_ms, -data->period, -host_alarm_handler, -(DWORD)t, -TIME_ONESHOT | TIME_CALLBACK_FUNCTION); - -if (!data->timerId) { -fprintf(stderr, "Failed to re-arm win32 alarm timer %ld\n", -GetLastError()); +success = ChangeTimerQueueTimer(NULL, +hTimer, +nearest_delta_ms, +360); -timeEndPeriod(data->period); -exit(1); +if (!success) { +fprintf(stderr, "Failed to rearm win32 alarm timer: %ld\n", +GetLastError(
[Qemu-devel] [PATCH v3 uq/master 10/22] inline cpu_halted into sole caller
All implementations are now the same, and there is only one caller, so inline the function there. Signed-off-by: Paolo Bonzini --- cpu-exec.c |9 +++-- target-alpha/exec.h | 11 --- target-arm/exec.h| 13 - target-cris/exec.h | 11 --- target-i386/exec.h | 12 target-m68k/exec.h | 10 -- target-microblaze/exec.h | 11 --- target-mips/exec.h | 11 --- target-ppc/exec.h| 11 --- target-s390x/exec.h | 12 target-sh4/exec.h| 10 -- target-sparc/exec.h | 10 -- 12 files changed, 7 insertions(+), 124 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index b03b3a7..eed9282 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -208,8 +208,13 @@ int cpu_exec(CPUState *env1) uint8_t *tc_ptr; unsigned long next_tb; -if (cpu_halted(env1) == EXCP_HALTED) -return EXCP_HALTED; +if (env1->halted) { +if (!cpu_has_work(env1)) { +return EXCP_HALTED; +} + +env1->halted = 0; +} cpu_single_env = env1; diff --git a/target-alpha/exec.h b/target-alpha/exec.h index a8a38d2..6ae96d1 100644 --- a/target-alpha/exec.h +++ b/target-alpha/exec.h @@ -42,17 +42,6 @@ static inline int cpu_has_work(CPUState *env) return (env->interrupt_request & CPU_INTERRUPT_HARD); } -static inline int cpu_halted(CPUState *env) -{ -if (!env->halted) -return 0; -if (cpu_has_work(env)) { -env->halted = 0; -return 0; -} -return EXCP_HALTED; -} - static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb) { env->pc = tb->pc; diff --git a/target-arm/exec.h b/target-arm/exec.h index e4c35a3..44e1b55 100644 --- a/target-arm/exec.h +++ b/target-arm/exec.h @@ -32,19 +32,6 @@ static inline int cpu_has_work(CPUState *env) (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB)); } -static inline int cpu_halted(CPUState *env) { -if (!env->halted) -return 0; -/* An interrupt wakes the CPU even if the I and F CPSR bits are - set. We use EXITTB to silently wake CPU without causing an - actual interrupt. */ -if (cpu_has_work(env)) { -env->halted = 0; -return 0; -} -return EXCP_HALTED; -} - #if !defined(CONFIG_USER_ONLY) #include "softmmu_exec.h" #endif diff --git a/target-cris/exec.h b/target-cris/exec.h index 34c0132..2d5d297 100644 --- a/target-cris/exec.h +++ b/target-cris/exec.h @@ -33,17 +33,6 @@ static inline int cpu_has_work(CPUState *env) return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)); } -static inline int cpu_halted(CPUState *env) { - if (!env->halted) - return 0; - - if (cpu_has_work(env)) { - env->halted = 0; - return 0; - } - return EXCP_HALTED; -} - static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb) { env->pc = tb->pc; diff --git a/target-i386/exec.h b/target-i386/exec.h index fc8945b..3e7386e 100644 --- a/target-i386/exec.h +++ b/target-i386/exec.h @@ -304,18 +304,6 @@ static inline int cpu_has_work(CPUState *env) return work; } -static inline int cpu_halted(CPUState *env) { -/* handle exit of HALTED state */ -if (!env->halted) -return 0; -/* disable halt condition */ -if (cpu_has_work(env)) { -env->halted = 0; -return 0; -} -return EXCP_HALTED; -} - /* load efer and update the corresponding hflags. XXX: do consistency checks with cpuid bits ? */ static inline void cpu_load_efer(CPUState *env, uint64_t val) diff --git a/target-m68k/exec.h b/target-m68k/exec.h index f31e06e..91daa6b 100644 --- a/target-m68k/exec.h +++ b/target-m68k/exec.h @@ -33,16 +33,6 @@ static inline int cpu_has_work(CPUState *env) return (env->interrupt_request & (CPU_INTERRUPT_HARD)); } -static inline int cpu_halted(CPUState *env) { -if (!env->halted) -return 0; -if (cpu_has_work(env)) { -env->halted = 0; -return 0; -} -return EXCP_HALTED; -} - static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb) { env->pc = tb->pc; diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h index ab19828..1efff30 100644 --- a/target-microblaze/exec.h +++ b/target-microblaze/exec.h @@ -32,17 +32,6 @@ static inline int cpu_has_work(CPUState *env) return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)); } -static inline int cpu_halted(CPUState *env) { - if (!env->halted) - return 0; - - if (cpu_has_work(env)) { - env->halted = 0; - return 0; - } - return EXCP_HALTED; -} - static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb) { env->sregs[SR_PC] = tb->pc; diff --git a/target-mips/exec.h b/target-mips/exec.
[Qemu-devel] Re: [PATCH v3 uq/master 00/22] Win32 iothread support
On 02/28/2011 11:10 AM, Paolo Bonzini wrote: After gathering the comments about the two series I sent separately, here is the full series for Win32 iothread support. It is based on master, but as it touches (mostly, indeed) OS-independent parts it is safer to get it in through uq/master. Why is that? uq/master is for kvm code, and this touches two or three lines? -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v3 uq/master 15/22] do not use timedwait on qemu_system_cond
qemu_main_loop_start is the only place where qemu_system_ready is set to 1. Signed-off-by: Paolo Bonzini --- cpus.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpus.c b/cpus.c index 4c3837f..e367b3b 100644 --- a/cpus.c +++ b/cpus.c @@ -823,7 +823,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg) /* and wait for machine initialization */ while (!qemu_system_ready) { -qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100); +qemu_cond_wait(&qemu_system_cond, &qemu_global_mutex); } while (1) { @@ -855,7 +855,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) /* and wait for machine initialization */ while (!qemu_system_ready) { -qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100); +qemu_cond_wait(&qemu_system_cond, &qemu_global_mutex); } while (1) { -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 01/22] unlock iothread during WaitForMultipleObjects
Signed-off-by: Paolo Bonzini --- os-win32.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/os-win32.c b/os-win32.c index b214e6a..c971d92 100644 --- a/os-win32.c +++ b/os-win32.c @@ -140,7 +140,9 @@ void os_host_main_loop_wait(int *timeout) int err; WaitObjects *w = &wait_objects; +qemu_mutex_unlock_iothread(); ret = WaitForMultipleObjects(w->num, w->events, FALSE, *timeout); +qemu_mutex_lock_iothread(); if (WAIT_OBJECT_0 + 0 <= ret && ret <= WAIT_OBJECT_0 + w->num - 1) { if (w->func[ret - WAIT_OBJECT_0]) w->func[ret - WAIT_OBJECT_0](w->opaque[ret - WAIT_OBJECT_0]); -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 22/22] add Win32 IPI service
Signed-off-by: Paolo Bonzini --- cpus.c | 25 ++--- qemu-thread-posix.c |9 - qemu-thread-posix.h |1 - 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/cpus.c b/cpus.c index 7559a02..077729c 100644 --- a/cpus.c +++ b/cpus.c @@ -854,13 +854,32 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) return NULL; } +static void qemu_cpu_kick_thread(CPUState *env) +{ +#ifndef _WIN32 +int err; + +err = pthread_kill(env->thread->thread, SIG_IPI); +if (err) { +fprintf(stderr, "qemu:%s: %s", __func__, strerror(err)); +exit(1); +} +#else /* _WIN32 */ +if (!qemu_cpu_is_self(env)) { +SuspendThread(env->thread->thread); +cpu_signal(0); +ResumeThread(env->thread->thread); +} +#endif +} + void qemu_cpu_kick(void *_env) { CPUState *env = _env; qemu_cond_broadcast(env->halt_cond); if (!env->thread_kicked) { -qemu_thread_signal(env->thread, SIG_IPI); +qemu_cpu_kick_thread(env); env->thread_kicked = true; } } @@ -871,7 +890,7 @@ void qemu_cpu_kick_self(void) assert(cpu_single_env); if (!cpu_single_env->thread_kicked) { -qemu_thread_signal(cpu_single_env->thread, SIG_IPI); +qemu_cpu_kick_thread(cpu_single_env); cpu_single_env->thread_kicked = true; } #else @@ -893,7 +912,7 @@ void qemu_mutex_lock_iothread(void) } else { qemu_mutex_lock(&qemu_fair_mutex); if (qemu_mutex_trylock(&qemu_global_mutex)) { -qemu_thread_signal(tcg_cpu_thread, SIG_IPI); +qemu_cpu_kick_thread(first_cpu); qemu_mutex_lock(&qemu_global_mutex); } qemu_mutex_unlock(&qemu_fair_mutex); diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c index a4c6e25..9cceda7 100644 --- a/qemu-thread-posix.c +++ b/qemu-thread-posix.c @@ -186,15 +186,6 @@ void qemu_thread_create(QemuThread *thread, pthread_sigmask(SIG_SETMASK, &oldset, NULL); } -void qemu_thread_signal(QemuThread *thread, int sig) -{ -int err; - -err = pthread_kill(thread->thread, sig); -if (err) -error_exit(err, __func__); -} - void qemu_thread_get_self(QemuThread *thread) { thread->thread = pthread_self(); diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h index 11978db..35e0a8b 100644 --- a/qemu-thread-posix.h +++ b/qemu-thread-posix.h @@ -15,5 +15,4 @@ struct QemuThread { pthread_t thread; }; -void qemu_thread_signal(QemuThread *thread, int sig); #endif -- 1.7.4
[Qemu-devel] [PATCH v3 uq/master 11/22] always qemu_cpu_kick after unhalting a cpu
This ensures env->halt_cond is broadcast, and the loop in qemu_tcg_wait_io_event and qemu_kvm_wait_io_event is exited naturally rather than through a timeout. Signed-off-by: Paolo Bonzini --- hw/ppc.c |2 ++ hw/sun4m.c | 10 -- hw/sun4u.c |4 ++-- target-s390x/kvm.c |1 + 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/hw/ppc.c b/hw/ppc.c index 968aec1..de02d33 100644 --- a/hw/ppc.c +++ b/hw/ppc.c @@ -208,6 +208,7 @@ static void ppc970_set_irq (void *opaque, int pin, int level) } else { LOG_IRQ("%s: restart the CPU\n", __func__); env->halted = 0; +qemu_cpu_kick(env); } break; case PPC970_INPUT_HRESET: @@ -300,6 +301,7 @@ static void ppc40x_set_irq (void *opaque, int pin, int level) } else { LOG_IRQ("%s: restart the CPU\n", __func__); env->halted = 0; +qemu_cpu_kick(env); } break; case PPC40x_INPUT_DEBUG: diff --git a/hw/sun4m.c b/hw/sun4m.c index 30e8a21..df3aa32 100644 --- a/hw/sun4m.c +++ b/hw/sun4m.c @@ -253,15 +253,21 @@ void cpu_check_irqs(CPUState *env) } } +static void cpu_kick_irq(CPUState *env) +{ +env->halted = 0; +cpu_check_irqs(env); +qemu_cpu_kick(env); +} + static void cpu_set_irq(void *opaque, int irq, int level) { CPUState *env = opaque; if (level) { trace_sun4m_cpu_set_irq_raise(irq); -env->halted = 0; env->pil_in |= 1 << irq; -cpu_check_irqs(env); +cpu_kick_irq(env); } else { trace_sun4m_cpu_set_irq_lower(irq); env->pil_in &= ~(1 << irq); diff --git a/hw/sun4u.c b/hw/sun4u.c index 90b1ce2..d282324 100644 --- a/hw/sun4u.c +++ b/hw/sun4u.c @@ -298,6 +298,7 @@ static void cpu_kick_irq(CPUState *env) { env->halted = 0; cpu_check_irqs(env); +qemu_cpu_kick(env); } static void cpu_set_irq(void *opaque, int irq, int level) @@ -306,9 +307,8 @@ static void cpu_set_irq(void *opaque, int irq, int level) if (level) { CPUIRQ_DPRINTF("Raise CPU IRQ %d\n", irq); -env->halted = 0; env->pil_in |= 1 << irq; -cpu_check_irqs(env); +cpu_kick_irq(env); } else { CPUIRQ_DPRINTF("Lower CPU IRQ %d\n", irq); env->pil_in &= ~(1 << irq); diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index b349812..6e94274 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -194,6 +194,7 @@ static void kvm_s390_interrupt_internal(CPUState *env, int type, uint32_t parm, env->halted = 0; env->exception_index = -1; +qemu_cpu_kick(env); kvmint.type = type; kvmint.parm = parm; -- 1.7.4
[Qemu-devel] Fix build errors introduced by vnc jpeg changes
The recent changes to handling of jpegs over tight vnc connections cause build errors if qemu is configured with --disable-vnc-jpeg. The patch below corrects the errors, by adding some left out #ifdefs. Please apply. Signed-off-by: David Gibson --- ui/vnc-enc-tight.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c index 5933394..2522936 100644 --- a/ui/vnc-enc-tight.c +++ b/ui/vnc-enc-tight.c @@ -1536,8 +1536,10 @@ static int send_sub_rect(VncState *vs, int x, int y, int w, int h) uint32_t bg = 0, fg = 0; int colors; int ret = 0; +#ifdef CONFIG_VNC_JPEG bool force_jpeg = false; bool allow_jpeg = true; +#endif vnc_framebuffer_update(vs, x, y, w, h, vs->tight.type); @@ -1711,6 +1713,7 @@ static int tight_send_framebuffer_update(VncState *vs, int x, int y, vs->tight.pixel24 = false; } +#ifdef CONFIG_VNC_JPEG if (vs->tight.quality != (uint8_t)-1) { double freq = vnc_update_freq(vs, x, y, w, h); @@ -1718,6 +1721,7 @@ static int tight_send_framebuffer_update(VncState *vs, int x, int y, return send_rect_simple(vs, x, y, w, h, false); } } +#endif if (w * h < VNC_TIGHT_MIN_SPLIT_RECT_SIZE) { return send_rect_simple(vs, x, y, w, h, true); -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
[Qemu-devel] Fix build breakage to kvm on ppc
Recent changes to the generic kvm support code broke compile of kvm for ppc. The patch below fixes the errors by adjusting types in the ppc code, and adding a missing #ifdef. Please apply. Signed-off-by: David Gibson --- kvm-all.c|2 ++ target-ppc/kvm.c |6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index e6a7de4..fb44e2e 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -998,7 +998,9 @@ int kvm_cpu_exec(CPUState *env) } ret = EXCP_INTERRUPT; +#ifdef KVM_CAP_SET_GUEST_DEBUG out: +#endif /* KVM_CAP_SET_GUEST_DEBUG */ env->exit_request = 0; cpu_single_env = NULL; return ret; diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 04b94a3..4fa1be3 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -223,7 +223,7 @@ int kvmppc_set_interrupt(CPUState *env, int irq, int level) #define PPC_INPUT_INT PPC6xx_INPUT_INT #endif -int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) +void kvm_arch_pre_run(CPUState *env, struct kvm_run *run) { int r; unsigned irq; @@ -254,15 +254,15 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run) /* We don't know if there are more interrupts pending after this. However, * the guest will return to userspace in the course of handling this one * anyways, so we will get a chance to deliver the rest. */ -return 0; } void kvm_arch_post_run(CPUState *env, struct kvm_run *run) { } -void kvm_arch_process_irqchip_events(CPUState *env) +int kvm_arch_process_irqchip_events(CPUState *env) { +return 0; } static int kvmppc_handle_halt(CPUState *env) -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
[Qemu-devel] [PATCH (resend, rebase) 1/3] virtio-serial: Use a struct to pass config information from proxy
Instead of using a single variable to pass to the virtio_serial_init function, use a struct so that expanding the number of variables to be passed on later is easier. Signed-off-by: Amit Shah --- hw/virtio-pci.c| 12 ++-- hw/virtio-serial-bus.c | 16 hw/virtio-serial.h |5 + hw/virtio.h|3 ++- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 3911b09..952b5d2 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -18,6 +18,7 @@ #include "virtio.h" #include "virtio-blk.h" #include "virtio-net.h" +#include "virtio-serial.h" #include "pci.h" #include "qemu-error.h" #include "msix.h" @@ -109,8 +110,7 @@ typedef struct { #ifdef CONFIG_LINUX V9fsConf fsconf; #endif -/* Max. number of ports we can have for a the virtio-serial device */ -uint32_t max_virtserial_ports; +virtio_serial_conf serial; virtio_net_conf net; bool ioeventfd_disabled; bool ioeventfd_started; @@ -770,12 +770,12 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev) proxy->class_code != PCI_CLASS_OTHERS) /* qemu-kvm */ proxy->class_code = PCI_CLASS_COMMUNICATION_OTHER; -vdev = virtio_serial_init(&pci_dev->qdev, proxy->max_virtserial_ports); +vdev = virtio_serial_init(&pci_dev->qdev, &proxy->serial); if (!vdev) { return -1; } vdev->nvectors = proxy->nvectors == DEV_NVECTORS_UNSPECIFIED -? proxy->max_virtserial_ports + 1 +? proxy->serial.max_virtserial_ports + 1 : proxy->nvectors; virtio_init_pci(proxy, vdev, PCI_VENDOR_ID_REDHAT_QUMRANET, @@ -902,8 +902,8 @@ static PCIDeviceInfo virtio_info[] = { DEV_NVECTORS_UNSPECIFIED), DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), -DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, max_virtserial_ports, - 31), +DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, + serial.max_virtserial_ports, 31), DEFINE_PROP_END_OF_LIST(), }, .qdev.reset = virtio_pci_reset, diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 8446bc2..c6feb43 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -811,19 +811,19 @@ void virtio_serial_port_qdev_register(VirtIOSerialPortInfo *info) qdev_register(&info->qdev); } -VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports) +VirtIODevice *virtio_serial_init(DeviceState *dev, virtio_serial_conf *conf) { VirtIOSerial *vser; VirtIODevice *vdev; uint32_t i, max_supported_ports; -if (!max_nr_ports) +if (!conf->max_virtserial_ports) return NULL; /* Each port takes 2 queues, and one pair is for the control queue */ max_supported_ports = VIRTIO_PCI_QUEUE_MAX / 2 - 1; -if (max_nr_ports > max_supported_ports) { +if (conf->max_virtserial_ports > max_supported_ports) { error_report("maximum ports supported: %u", max_supported_ports); return NULL; } @@ -839,9 +839,9 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports) vser->bus->vser = vser; QTAILQ_INIT(&vser->ports); -vser->bus->max_nr_ports = max_nr_ports; -vser->ivqs = qemu_malloc(max_nr_ports * sizeof(VirtQueue *)); -vser->ovqs = qemu_malloc(max_nr_ports * sizeof(VirtQueue *)); +vser->bus->max_nr_ports = conf->max_virtserial_ports; +vser->ivqs = qemu_malloc(conf->max_virtserial_ports * sizeof(VirtQueue *)); +vser->ovqs = qemu_malloc(conf->max_virtserial_ports * sizeof(VirtQueue *)); /* Add a queue for host to guest transfers for port 0 (backward compat) */ vser->ivqs[0] = virtio_add_queue(vdev, 128, handle_input); @@ -866,8 +866,8 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports) vser->ovqs[i] = virtio_add_queue(vdev, 128, handle_output); } -vser->config.max_nr_ports = max_nr_ports; -vser->ports_map = qemu_mallocz(((max_nr_ports + 31) / 32) +vser->config.max_nr_ports = conf->max_virtserial_ports; +vser->ports_map = qemu_mallocz(((conf->max_virtserial_ports + 31) / 32) * sizeof(vser->ports_map[0])); /* * Reserve location 0 for a console port for backward compat diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h index 8cb9fbe..87cd7a1 100644 --- a/hw/virtio-serial.h +++ b/hw/virtio-serial.h @@ -45,6 +45,11 @@ struct virtio_console_control { uint16_t value;/* Extra information for the key */ }; +struct virtio_serial_conf { +/* Max. number of ports we can have for a the virtio-serial device */ +uint32_t max_virtserial_ports; +};
[Qemu-devel] [PULL (resend, rebase) 0/3] virtio-serial fixes, ioeventfd support
Rebased version from last week: The following changes since commit 417131fb9ad3f6dd7177a338cc5f143dec4d75f0: HACKING: Update status of format checking (2011-02-25 16:31:05 -0600) are available in the git repository at: git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git for-anthony Amit Shah (3): virtio-serial: Use a struct to pass config information from proxy virtio-serial: Disallow generic ports at id 0 virtio-serial: Enable ioeventfd hw/virtio-console.c| 11 ++- hw/virtio-pci.c| 15 +-- hw/virtio-serial-bus.c | 16 hw/virtio-serial.h |5 + hw/virtio.h|3 ++- 5 files changed, 34 insertions(+), 16 deletions(-) -- 1.7.4
[Qemu-devel] [PATCH (resend, rebase) 2/3] virtio-serial: Disallow generic ports at id 0
It was found libvirt was using port 0 for generic ports. It has been fixed in libvirt commit 8e28c5d40200b4c5d483bd585d237b9d870372e5. Port 0 is reserved for virtconsole devices for backward compatibility with the old -virtioconsole (from qemu 0.12) device type. Ensure we don't allow instantiating a port at id 0 as well. Signed-off-by: Amit Shah --- hw/virtio-console.c | 11 ++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/hw/virtio-console.c b/hw/virtio-console.c index c235b27..699f190 100644 --- a/hw/virtio-console.c +++ b/hw/virtio-console.c @@ -11,6 +11,7 @@ */ #include "qemu-char.h" +#include "qemu-error.h" #include "virtio-serial.h" typedef struct VirtConsole { @@ -113,7 +114,15 @@ static int virtserialport_initfn(VirtIOSerialPort *port) { VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port); -return generic_port_init(vcon, port); +if (port->id == 0) { +/* + * Disallow a generic port at id 0, that's reserved for + * console ports. + */ +error_report("Port number 0 on virtio-serial devices reserved for virtconsole devices for backward compatibility."); +return -1; +} +return generic_port_init(vcon, dev); } static VirtIOSerialPortInfo virtserialport_info = { -- 1.7.4
Re: [Qemu-devel] Fix build errors introduced by vnc jpeg changes
On 28 February 2011 10:48, David Gibson wrote: > The recent changes to handling of jpegs over tight vnc connections > cause build errors if qemu is configured with --disable-vnc-jpeg. The > patch below corrects the errors, by adding some left out #ifdefs. > > Please apply. > > Signed-off-by: David Gibson This is already fixed in master: http://git.qemu.org/qemu.git/commit/?id=cf76a1ce8b7cf4b92429d67d3f4626a92b2d8a37 -- PMM
[Qemu-devel] [PATCH (resend, rebase) 3/3] virtio-serial: Enable ioeventfd
Enable ioeventfd for virtio-serial devices by default. Commit 25db9ebe15125deb32958c6df74996f745edf1f9 lists the benefits of using ioeventfd. Copying a file from guest to host over a virtio-serial channel didn't show much difference in time or io_exit rate. Signed-off-by: Amit Shah --- hw/virtio-pci.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 952b5d2..ef65590 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -789,6 +789,7 @@ static int virtio_serial_exit_pci(PCIDevice *pci_dev) { VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); +virtio_pci_stop_ioeventfd(proxy); virtio_serial_exit(proxy->vdev); return virtio_exit_pci(pci_dev); } @@ -898,6 +899,8 @@ static PCIDeviceInfo virtio_info[] = { .init = virtio_serial_init_pci, .exit = virtio_serial_exit_pci, .qdev.props = (Property[]) { +DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, +VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED), DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), -- 1.7.4
[Qemu-devel] [V6 PATCH 8/9] virtio-9p: Move file post creation changes to none security model
After creating a file object, its permission and ownership details are updated as per 9p client's request for both passthrough and none security model. But with chrooted environment its not required for passthrough security model. Move all post file creation changes to none security model. Signed-off-by: M. Mohan Kumar --- hw/9pfs/virtio-9p-local.c | 19 ++- 1 files changed, 6 insertions(+), 13 deletions(-) diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index c92c5dd..f5dba35 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -163,21 +163,14 @@ static int local_set_xattr(const char *path, FsCred *credp) return 0; } -static int local_post_create_passthrough(FsContext *fs_ctx, const char *path, +static int local_post_create_none(FsContext *fs_ctx, const char *path, FsCred *credp) { +int retval; if (chmod(rpath(fs_ctx, path), credp->fc_mode & 0) < 0) { return -1; } -if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid) < 0) { -/* - * If we fail to change ownership and if we are - * using security model none. Ignore the error - */ -if (fs_ctx->fs_sm != SM_NONE) { -return -1; -} -} +retval = lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid); return 0; } @@ -318,7 +311,7 @@ static int local_mknod(FsContext *fs_ctx, const char *path, FsCred *credp) if (err == -1) { return err; } -err = local_post_create_passthrough(fs_ctx, path, credp); +err = local_post_create_none(fs_ctx, path, credp); if (err == -1) { serrno = errno; goto err_end; @@ -360,7 +353,7 @@ static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred *credp) if (err == -1) { return err; } -err = local_post_create_passthrough(fs_ctx, path, credp); +err = local_post_create_none(fs_ctx, path, credp); if (err == -1) { serrno = errno; goto err_end; @@ -435,7 +428,7 @@ static int local_open2(FsContext *fs_ctx, const char *path, int flags, if (fd == -1) { return fd; } -err = local_post_create_passthrough(fs_ctx, path, credp); +err = local_post_create_none(fs_ctx, path, credp); if (err == -1) { serrno = errno; goto err_end; -- 1.7.3.4
[Qemu-devel] [V6 PATCH 1/9] Implement qemu_read_full
Add qemu_read_full function Signed-off-by: M. Mohan Kumar --- osdep.c | 32 qemu-common.h |2 ++ 2 files changed, 34 insertions(+), 0 deletions(-) diff --git a/osdep.c b/osdep.c index 327583b..8d84a88 100644 --- a/osdep.c +++ b/osdep.c @@ -127,6 +127,38 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count) } /* + * A variant of read(2) which handles interrupted read. + * Simlar to qemu_write_full function + * + * Return the number of bytes read. + * + * This function does not work with non-blocking fd's. + * errno is set if fewer than `count' bytes are read because of any + * error + */ +ssize_t qemu_read_full(int fd, void *buf, size_t count) +{ +ssize_t ret = 0; +ssize_t total = 0; + +while (count) { +ret = read(fd, buf, count); +if (ret <= 0) { +if (errno == EINTR) { +continue; +} +break; +} + +count -= ret; +buf += ret; +total += ret; +} + +return total; +} + +/* * Opens a socket with FD_CLOEXEC set */ int qemu_socket(int domain, int type, int protocol) diff --git a/qemu-common.h b/qemu-common.h index 40dad52..325b16a 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -207,6 +207,8 @@ void qemu_mutex_unlock_iothread(void); int qemu_open(const char *name, int flags, ...); ssize_t qemu_write_full(int fd, const void *buf, size_t count) QEMU_WARN_UNUSED_RESULT; +ssize_t qemu_read_full(int fd, void *buf, size_t count) +QEMU_WARN_UNUSED_RESULT; void qemu_set_cloexec(int fd); #ifndef _WIN32 -- 1.7.3.4
[Qemu-devel] [V6 PATCH 2/9] virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled
9p Chroot environment needs APIs defined in qemu-thread.c, so enable CONFIG_THREAD if virtfs is enabled Signed-off-by: M. Mohan Kumar --- configure |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/configure b/configure index 2560357..9eddd38 100755 --- a/configure +++ b/configure @@ -2767,6 +2767,7 @@ fi if test "$linux" = "yes" ; then if test "$attr" = "yes" ; then echo "CONFIG_VIRTFS=y" >> $config_host_mak +echo "CONFIG_THREAD=y" >> $config_host_mak fi fi if test "$blobs" = "yes" ; then -- 1.7.3.4
[Qemu-devel] [V6 PATCH 3/9] virtio-9p: Provide chroot daemon side interfaces
Implement chroot daemon side interfaces like sending the file descriptor to qemu process, reading the object request from socket etc. Also add chroot main function and other helper routines. Signed-off-by: M. Mohan Kumar --- Makefile.objs |1 + hw/9pfs/virtio-9p-chroot-dm.c | 186 + hw/9pfs/virtio-9p-chroot.h| 57 + hw/9pfs/virtio-9p.c | 32 +++ hw/file-op-9p.h |4 + 5 files changed, 280 insertions(+), 0 deletions(-) create mode 100644 hw/9pfs/virtio-9p-chroot-dm.c create mode 100644 hw/9pfs/virtio-9p-chroot.h diff --git a/Makefile.objs b/Makefile.objs index 6a9f765..f09cdee 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -283,6 +283,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y) 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p-debug.o 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o +9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-chroot-dm.o hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y)) $(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS += -I$(SRC_PATH)/hw/ diff --git a/hw/9pfs/virtio-9p-chroot-dm.c b/hw/9pfs/virtio-9p-chroot-dm.c new file mode 100644 index 000..c1d8c6e --- /dev/null +++ b/hw/9pfs/virtio-9p-chroot-dm.c @@ -0,0 +1,186 @@ +/* + * Virtio 9p chroot environment for contained access to the exported path + * Code path handles chroot daemon interfaces + * Copyright IBM, Corp. 2011 + * + * Authors: + * M. Mohan Kumar + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the copying file in the top-level directory + * + */ + +#include +#include +#include +#include "virtio.h" +#include "qemu_socket.h" +#include "qemu-thread.h" +#include "qerror.h" +#include "virtio-9p.h" +#include "virtio-9p-chroot.h" + +/* Send file descriptor and error status to qemu process */ +static void chroot_sendfd(int sockfd, const FdInfo *fd_info) +{ +struct msghdr msg = { }; +struct iovec iov; +struct cmsghdr *cmsg; +int retval; +union MsgControl msg_control; + +iov.iov_base = (void *)fd_info; +iov.iov_len = sizeof(*fd_info); + +memset(&msg, 0, sizeof(msg)); +msg.msg_iov = &iov; +msg.msg_iovlen = 1; +/* No ancillary data on error/fd invalid flag */ +if (!(fd_info->fi_flags & FI_FD_INVALID)) { +msg.msg_control = &msg_control; +msg.msg_controllen = sizeof(msg_control); + +cmsg = &msg_control.cmsg; +cmsg->cmsg_len = CMSG_LEN(sizeof(fd_info->fi_fd)); +cmsg->cmsg_level = SOL_SOCKET; +cmsg->cmsg_type = SCM_RIGHTS; +memcpy(CMSG_DATA(cmsg), &fd_info->fi_fd, sizeof(fd_info->fi_fd)); +} +retval = sendmsg(sockfd, &msg, 0); +if (retval < 0) { +_exit(1); +} +if (!(fd_info->fi_flags & FI_FD_INVALID)) { +close(fd_info->fi_fd); +} +} + +/* Read V9fsFileObjectRequest sent by QEMU process */ +static int chroot_read_request(int sockfd, V9fsFileObjectRequest *request) +{ +int retval; +retval = qemu_read_full(sockfd, request, sizeof(*request)); +if (retval != sizeof(*request)) { +if (errno == EBADF) { +_exit(1); +} +return -EIO; +} +return 0; +} + +/* + * Helper routine to open a file and return response through + * FdInfo structure + */ +static void chroot_do_open(V9fsFileObjectRequest *request, FdInfo *fd_info) +{ +fd_info->fi_fd = open(request->path.path, request->data.flags); +if (fd_info->fi_fd < 0) { +fd_info->fi_fd = -errno; +fd_info->fi_flags = FI_FD_INVALID; +} +} + +static int chroot_daemonize(int chroot_sock) +{ +sigset_t sigset; +struct rlimit nr_fd; +int fd; + +/* Block all signals for this process */ +sigfillset(&sigset); +sigprocmask(SIG_SETMASK, &sigset, NULL); + +/* Close other file descriptors */ +getrlimit(RLIMIT_NOFILE, &nr_fd); +for (fd = 0; fd < nr_fd.rlim_cur; fd++) { +if (fd != chroot_sock) { +close(fd); +} +} +chdir("/"); +/* Create files with mode as per request */ +umask(0); +return 0; +} + +/* + * Fork a process and chroot into the share path. Communication + * between qemu process and chroot process happens via socket. + * All file descriptors (including stdout and stderr) are closed + * except one socket descriptor (which is used for communicating + * between qemu process and chroot process). + * Note: To avoid errors in forked process in multi threaded environment + * only async-signal safe functions used. For more information see + * man fork(3p), signal(7) + */ +int v9fs_chroot(FsContext *fs_ctx) +{ +int fd_pair[2], chroot_sock, error; +V9fsFileObjectRequest request; +pid_t pid; +uint64_t code; +FdInfo fd_info; + +if (socketpair(PF_UNIX, SOCK_STREAM, 0, fd_pair) < 0) { +error_report("socketpair %s", strerror
[Qemu-devel] [V6 PATCH 6/9] virtio-9p: Create support in chroot environment
Add both chroot deamon & qemu side interfaces to create regular files in chroot environment Signed-off-by: M. Mohan Kumar --- hw/9pfs/virtio-9p-chroot-dm.c | 39 +++ hw/9pfs/virtio-9p-local.c | 21 +++-- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/hw/9pfs/virtio-9p-chroot-dm.c b/hw/9pfs/virtio-9p-chroot-dm.c index c1d8c6e..985d42b 100644 --- a/hw/9pfs/virtio-9p-chroot-dm.c +++ b/hw/9pfs/virtio-9p-chroot-dm.c @@ -83,6 +83,42 @@ static void chroot_do_open(V9fsFileObjectRequest *request, FdInfo *fd_info) } } +/* + * Helper routine to create a file and return the file descriptor and + * error status in FdInfo structure. + */ +static void chroot_do_create(V9fsFileObjectRequest *request, FdInfo *fd_info) +{ +uid_t cur_uid; +gid_t cur_gid; + +cur_uid = geteuid(); +cur_gid = getegid(); + +fd_info->fi_fd = -1; + +if (setfsuid(request->data.uid) < 0) { +fd_info->fi_fd = -errno; +fd_info->fi_flags = FI_FD_INVALID; +return; +} +if (setfsgid(request->data.gid) < 0) { +fd_info->fi_fd = -errno; +fd_info->fi_flags = FI_FD_INVALID; +goto unset_uid; +} + +fd_info->fi_fd = open(request->path.path, request->data.flags, +request->data.mode); +if (fd_info->fi_fd < 0) { +fd_info->fi_fd = -errno; +fd_info->fi_flags = FI_FD_INVALID; +} +setfsgid(cur_gid); +unset_uid: +setfsuid(cur_uid); +} + static int chroot_daemonize(int chroot_sock) { sigset_t sigset; @@ -177,6 +213,9 @@ int v9fs_chroot(FsContext *fs_ctx) case T_OPEN: chroot_do_open(&request, &fd_info); break; +case T_CREATE: +chroot_do_create(&request, &fd_info); +break; default: fd_info.fi_flags = FI_FD_SOCKERR; break; diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index 0c55d35..3fed16c 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -58,6 +58,22 @@ static int passthrough_open(FsContext *fs_ctx, const char *path, int flags) return fd; } +static int passthrough_create(FsContext *fs_ctx, const char *path, int flags, +FsCred *credp) +{ +V9fsFileObjectRequest request; +int fd; + +fd = fill_fileobjectrequest(&request, path, credp); +if (fd < 0) { +return fd; +} +request.data.flags = flags; +request.data.type = T_CREATE; +fd = v9fs_request(fs_ctx, &request); +return fd; +} + static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf) { int err; @@ -382,8 +398,7 @@ static int local_open2(FsContext *fs_ctx, const char *path, int flags, serrno = errno; goto err_end; } -} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) || - (fs_ctx->fs_sm == SM_NONE)) { +} else if (fs_ctx->fs_sm == SM_NONE) { fd = open(rpath(fs_ctx, path), flags, credp->fc_mode); if (fd == -1) { return fd; @@ -393,6 +408,8 @@ static int local_open2(FsContext *fs_ctx, const char *path, int flags, serrno = errno; goto err_end; } +} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) { +fd = passthrough_create(fs_ctx, path, flags, credp); } return fd; -- 1.7.3.4
[Qemu-devel] [V6 PATCH 4/9] virtio-9p: Add qemu side interfaces for chroot environment
QEMU side interfaces to communicate with chroot daemon process. Signed-off-by: M. Mohan Kumar --- Makefile.objs |2 +- hw/9pfs/virtio-9p-chroot-qemu.c | 97 +++ hw/9pfs/virtio-9p-chroot.h |1 + 3 files changed, 99 insertions(+), 1 deletions(-) create mode 100644 hw/9pfs/virtio-9p-chroot-qemu.c diff --git a/Makefile.objs b/Makefile.objs index f09cdee..9ea7946 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -283,7 +283,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y) 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p-debug.o 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o -9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-chroot-dm.o +9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-chroot-dm.o virtio-9p-chroot-qemu.o hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y)) $(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS += -I$(SRC_PATH)/hw/ diff --git a/hw/9pfs/virtio-9p-chroot-qemu.c b/hw/9pfs/virtio-9p-chroot-qemu.c new file mode 100644 index 000..d2d8ab9 --- /dev/null +++ b/hw/9pfs/virtio-9p-chroot-qemu.c @@ -0,0 +1,97 @@ +/* + * Virtio 9p chroot environment for contained access to exported path + * Code handles qemu side interfaces to communicate with chroot daemon process + * Copyright IBM, Corp. 2011 + * + * Authors: + * M. Mohan Kumar + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the copying file in the top-level directory + * + */ + +#include +#include +#include +#include "virtio.h" +#include "qemu_socket.h" +#include "qemu-thread.h" +#include "qerror.h" +#include "virtio-9p.h" +#include "virtio-9p-chroot.h" + +/* + * Return received file descriptor on success and -errno on failure. + * sock_error is set to 1 whenever there is error in socket IO + */ +static int v9fs_receivefd(int sockfd, int *sock_error) +{ +struct msghdr msg = { }; +struct iovec iov; +union MsgControl msg_control; +struct cmsghdr *cmsg; +int retval, fd; +FdInfo fd_info; + +iov.iov_base = &fd_info; +iov.iov_len = sizeof(fd_info); + +*sock_error = 0; +memset(&msg, 0, sizeof(msg)); +msg.msg_iov = &iov; +msg.msg_iovlen = 1; +msg.msg_control = &msg_control; +msg.msg_controllen = sizeof(msg_control); + +retval = recvmsg(sockfd, &msg, 0); +if (retval < 0) { +*sock_error = 1; +return -EIO; +} +if (fd_info.fi_flags & FI_FD_SOCKERR) { +*sock_error = 1; +return -EIO; +} +/* If fd is invalid, ancillary data is not present */ +if (fd_info.fi_fd < 0 || fd_info.fi_flags & FI_FD_INVALID) { +return fd_info.fi_fd; +} + +for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) { +if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) || +cmsg->cmsg_level != SOL_SOCKET || +cmsg->cmsg_type != SCM_RIGHTS) { +continue; +} +fd = *((int *)CMSG_DATA(cmsg)); +return fd; +} +*sock_error = 1; +return -EIO; +} + +/* + * V9fsFileObjectRequest is written into the socket by QEMU process. + * Then this request is read by chroot process using v9fs_read_request function + */ +static int v9fs_write_request(int sockfd, V9fsFileObjectRequest *request) +{ +int retval; +retval = qemu_write_full(sockfd, request, sizeof(*request)); +if (retval != sizeof(*request)) { +return -EIO; +} +return 0; +} + +/* + * This patch adds v9fs_receivefd and v9fs_write_request functions, + * but there is no callers. To avoid compiler warning message, + * refer these two functions + */ +void chroot_dummy(void) +{ +(void)v9fs_receivefd; +(void)v9fs_write_request; +} diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h index a5b2a41..bd186dd 100644 --- a/hw/9pfs/virtio-9p-chroot.h +++ b/hw/9pfs/virtio-9p-chroot.h @@ -53,5 +53,6 @@ typedef struct V9fsFileObjectRequest } V9fsFileObjectRequest; int v9fs_chroot(FsContext *fs_ctx); +void chroot_dummy(void); #endif /* _QEMU_VIRTIO_9P_CHROOT_H */ -- 1.7.3.4
[Qemu-devel] [V6 PATCH 5/9] virtio-9p: Add support to open a file in chroot environment
This patch adds both chroot deamon and qemu side support to open a file/ directory in the chroot environment Signed-off-by: M. Mohan Kumar --- hw/9pfs/virtio-9p-chroot-qemu.c | 24 +++- hw/9pfs/virtio-9p-chroot.h |2 +- hw/9pfs/virtio-9p-local.c | 58 --- 3 files changed, 71 insertions(+), 13 deletions(-) diff --git a/hw/9pfs/virtio-9p-chroot-qemu.c b/hw/9pfs/virtio-9p-chroot-qemu.c index d2d8ab9..41f9db2 100644 --- a/hw/9pfs/virtio-9p-chroot-qemu.c +++ b/hw/9pfs/virtio-9p-chroot-qemu.c @@ -85,13 +85,21 @@ static int v9fs_write_request(int sockfd, V9fsFileObjectRequest *request) return 0; } -/* - * This patch adds v9fs_receivefd and v9fs_write_request functions, - * but there is no callers. To avoid compiler warning message, - * refer these two functions - */ -void chroot_dummy(void) +/* Return opened file descriptor on success or -errno on error */ +int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *request) { -(void)v9fs_receivefd; -(void)v9fs_write_request; +int fd, sock_error; +qemu_mutex_lock(&fs_ctx->chroot_mutex); +if (fs_ctx->chroot_ioerror) { +fd = -EIO; +goto unlock; +} +v9fs_write_request(fs_ctx->chroot_socket, request); +fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error); +if (fd < 0 && sock_error) { +fs_ctx->chroot_ioerror = 1; +} +unlock: +qemu_mutex_unlock(&fs_ctx->chroot_mutex); +return fd; } diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h index bd186dd..4592807 100644 --- a/hw/9pfs/virtio-9p-chroot.h +++ b/hw/9pfs/virtio-9p-chroot.h @@ -53,6 +53,6 @@ typedef struct V9fsFileObjectRequest } V9fsFileObjectRequest; int v9fs_chroot(FsContext *fs_ctx); -void chroot_dummy(void); +int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *or); #endif /* _QEMU_VIRTIO_9P_CHROOT_H */ diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index 0a015de..0c55d35 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -13,6 +13,9 @@ #include "virtio.h" #include "virtio-9p.h" #include "virtio-9p-xattr.h" +#include "qemu_socket.h" +#include "fsdev/qemu-fsdev.h" +#include "virtio-9p-chroot.h" #include #include #include @@ -20,6 +23,40 @@ #include #include +/* Helper routine to fill V9fsFileObjectRequest structure */ +static int fill_fileobjectrequest(V9fsFileObjectRequest *request, +const char *path, FsCred *credp) +{ +if (strlen(path) > PATH_MAX) { +return -ENAMETOOLONG; +} +memset(request, 0, sizeof(*request)); +request->data.path_len = strlen(path); +strcpy(request->path.path, path); +if (credp) { +request->data.mode = credp->fc_mode; +request->data.uid = credp->fc_uid; +request->data.gid = credp->fc_gid; +request->data.dev = credp->fc_rdev; +} +return 0; +} + +static int passthrough_open(FsContext *fs_ctx, const char *path, int flags) +{ +V9fsFileObjectRequest request; +int fd; + +fd = fill_fileobjectrequest(&request, path, NULL); +if (fd < 0) { +errno = -fd; +return -1; +} +request.data.flags = flags; +request.data.type = T_OPEN; +fd = v9fs_request(fs_ctx, &request); +return fd; +} static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf) { @@ -138,14 +175,27 @@ static int local_closedir(FsContext *ctx, DIR *dir) return closedir(dir); } -static int local_open(FsContext *ctx, const char *path, int flags) +static int local_open(FsContext *fs_ctx, const char *path, int flags) { -return open(rpath(ctx, path), flags); +if (fs_ctx->fs_sm == SM_PASSTHROUGH) { +return passthrough_open(fs_ctx, path, flags); +} else { +return open(rpath(fs_ctx, path), flags); +} } -static DIR *local_opendir(FsContext *ctx, const char *path) +static DIR *local_opendir(FsContext *fs_ctx, const char *path) { -return opendir(rpath(ctx, path)); +if (fs_ctx->fs_sm == SM_PASSTHROUGH) { +int fd; +fd = passthrough_open(fs_ctx, path, O_DIRECTORY); +if (fd < 0) { +return NULL; +} +return fdopendir(fd); +} else { +return opendir(rpath(fs_ctx, path)); +} } static void local_rewinddir(FsContext *ctx, DIR *dir) -- 1.7.3.4
[Qemu-devel] [V6 PATCH 7/9] virtio-9p: Support for creating special files
Add both chroot deamon and qemu side interfaces to create special files (directory, device nodes, links and symbolic links) Signed-off-by: M. Mohan Kumar --- hw/9pfs/virtio-9p-chroot-dm.c | 57 hw/9pfs/virtio-9p-chroot-qemu.c | 19 hw/9pfs/virtio-9p-chroot.h |1 + hw/9pfs/virtio-9p-local.c | 93 ++- 4 files changed, 139 insertions(+), 31 deletions(-) diff --git a/hw/9pfs/virtio-9p-chroot-dm.c b/hw/9pfs/virtio-9p-chroot-dm.c index 985d42b..0ead017 100644 --- a/hw/9pfs/virtio-9p-chroot-dm.c +++ b/hw/9pfs/virtio-9p-chroot-dm.c @@ -119,6 +119,57 @@ unset_uid: setfsuid(cur_uid); } +/* + * Create directory, symbolic link, link, device node and regular files + * Similar to create, but it does not return the fd of created object + * Returns 0 as file descriptor on success and -errno on failure in FdInfo + * structure + */ +static void chroot_do_create_special(V9fsFileObjectRequest *request, +FdInfo *fd_info) +{ +int cur_uid, cur_gid; + +cur_uid = geteuid(); +cur_gid = getegid(); + +fd_info->fi_fd = -1; +/* fd is not valid for create operations */ +fd_info->fi_flags = FI_FD_INVALID; + +if (setfsuid(request->data.uid) < 0) { +fd_info->fi_fd = -errno; +return; +} +if (setfsgid(request->data.gid) < 0) { +fd_info->fi_fd = -errno; +goto unset_uid; +} + +switch (request->data.type) { +case T_MKDIR: +fd_info->fi_fd = mkdir(request->path.path, request->data.mode); +break; +case T_SYMLINK: +fd_info->fi_fd = symlink(request->path.old_path, request->path.path); +break; +case T_LINK: +fd_info->fi_fd = link(request->path.old_path, request->path.path); +break; +default: +fd_info->fi_fd = mknod(request->path.path, request->data.mode, +request->data.dev); +break; +} + +if (fd_info->fi_fd < 0) { +fd_info->fi_fd = -errno; +} +setfsgid(cur_gid); +unset_uid: +setfsuid(cur_uid); +} + static int chroot_daemonize(int chroot_sock) { sigset_t sigset; @@ -216,6 +267,12 @@ int v9fs_chroot(FsContext *fs_ctx) case T_CREATE: chroot_do_create(&request, &fd_info); break; +case T_MKDIR: +case T_SYMLINK: +case T_LINK: +case T_MKNOD: +chroot_do_create_special(&request, &fd_info); +break; default: fd_info.fi_flags = FI_FD_SOCKERR; break; diff --git a/hw/9pfs/virtio-9p-chroot-qemu.c b/hw/9pfs/virtio-9p-chroot-qemu.c index 41f9db2..1a42dc2 100644 --- a/hw/9pfs/virtio-9p-chroot-qemu.c +++ b/hw/9pfs/virtio-9p-chroot-qemu.c @@ -103,3 +103,22 @@ unlock: qemu_mutex_unlock(&fs_ctx->chroot_mutex); return fd; } + +/* Return 0 on success or -errno on error */ +int v9fs_create_special(FsContext *fs_ctx, V9fsFileObjectRequest *request) +{ +int fd, sock_error; +qemu_mutex_lock(&fs_ctx->chroot_mutex); +if (fs_ctx->chroot_ioerror) { +fd = -EIO; +goto unlock; +} +v9fs_write_request(fs_ctx->chroot_socket, request); +fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error); +if (fd < 0 && sock_error) { +fs_ctx->chroot_ioerror = 1; +} +unlock: +qemu_mutex_unlock(&fs_ctx->chroot_mutex); +return fd; +} diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h index 4592807..f113ff1 100644 --- a/hw/9pfs/virtio-9p-chroot.h +++ b/hw/9pfs/virtio-9p-chroot.h @@ -54,5 +54,6 @@ typedef struct V9fsFileObjectRequest int v9fs_chroot(FsContext *fs_ctx); int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *or); +int v9fs_create_special(FsContext *fs_ctx, V9fsFileObjectRequest *request); #endif /* _QEMU_VIRTIO_9P_CHROOT_H */ diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index 3fed16c..c92c5dd 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -74,6 +74,28 @@ static int passthrough_create(FsContext *fs_ctx, const char *path, int flags, return fd; } +static int passthrough_create_special(FsContext *fs_ctx, const char *oldpath, +const char *path, FsCred *credp, int type) +{ +V9fsFileObjectRequest request; +int retval; + +retval = fill_fileobjectrequest(&request, path, credp); +if (retval < 0) { +return retval; +} +request.data.type = type; +if (oldpath) { +request.data.oldpath_len = strlen(oldpath); +if (strlen(oldpath) > PATH_MAX) { +return -ENAMETOOLONG; +} +strcpy(request.path.old_path, oldpath); +} +retval = v9fs_create_special(fs_ctx, &request); +return retval; +} + static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf) { int err; @@ -291,8 +313,7 @@ static int local_mknod(FsContext *fs_ctx, const char *path, FsCred *cr
[Qemu-devel] [V6 PATCH 9/9] virtio-9p: Chroot environment for other functions
Add chroot functionality for systemcalls that can operate on a file using relative directory file descriptor. Signed-off-by: M. Mohan Kumar --- hw/9pfs/virtio-9p-local.c | 227 +++-- 1 files changed, 197 insertions(+), 30 deletions(-) diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c index f5dba35..b44ad55 100644 --- a/hw/9pfs/virtio-9p-local.c +++ b/hw/9pfs/virtio-9p-local.c @@ -22,6 +22,7 @@ #include #include #include +#include /* Helper routine to fill V9fsFileObjectRequest structure */ static int fill_fileobjectrequest(V9fsFileObjectRequest *request, @@ -96,14 +97,35 @@ static int passthrough_create_special(FsContext *fs_ctx, const char *oldpath, return retval; } +/* + * Returns file descriptor of dirname(path) + * This fd can be used by *at functions + */ +static int get_dirfd(FsContext *fs_ctx, const char *path) +{ +V9fsFileObjectRequest request; +int fd; +char *dpath = qemu_strdup(path); + +fd = fill_fileobjectrequest(&request, dirname(dpath), NULL); +if (fd < 0) { +return fd; +} +request.data.type = T_OPEN; +fd = v9fs_request(fs_ctx, &request); +qemu_free(dpath); +return fd; +} + static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf) { int err; -err = lstat(rpath(fs_ctx, path), stbuf); -if (err) { -return err; -} + if (fs_ctx->fs_sm == SM_MAPPED) { +err = lstat(rpath(fs_ctx, path), stbuf); +if (err) { +return err; +} /* Actual credentials are part of extended attrs */ uid_t tmp_uid; gid_t tmp_gid; @@ -125,6 +147,27 @@ static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf) sizeof(dev_t)) > 0) { stbuf->st_rdev = tmp_dev; } +} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) { +int pfd, serrno = 0; +char *tmp_path; + +pfd = get_dirfd(fs_ctx, path); +if (pfd < 0) { +return -1; +} +tmp_path = qemu_strdup(path); +err = fstatat(pfd, basename(tmp_path), stbuf, AT_SYMLINK_NOFOLLOW); +if (err < 0) { +serrno = errno; +} +close(pfd); +qemu_free(tmp_path); +errno = serrno; +} else { +err = lstat(rpath(fs_ctx, path), stbuf); +if (err) { +return err; +} } return err; } @@ -189,9 +232,23 @@ static ssize_t local_readlink(FsContext *fs_ctx, const char *path, } while (tsize == -1 && errno == EINTR); close(fd); return tsize; -} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) || - (fs_ctx->fs_sm == SM_NONE)) { +} else if (fs_ctx->fs_sm == SM_NONE) { tsize = readlink(rpath(fs_ctx, path), buf, bufsz); +} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) { +int pfd, serrno = 0; +char *tmp_path; +pfd = get_dirfd(fs_ctx, path); +if (pfd < 0) { +return -1; +} +tmp_path = qemu_strdup(path); +tsize = readlinkat(pfd, basename(tmp_path), buf, bufsz); +if (tsize < 0) { +serrno = 0; +} +close(pfd); +qemu_free(tmp_path); +errno = serrno; } return tsize; } @@ -283,8 +340,23 @@ static int local_chmod(FsContext *fs_ctx, const char *path, FsCred *credp) { if (fs_ctx->fs_sm == SM_MAPPED) { return local_set_xattr(rpath(fs_ctx, path), credp); -} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) || - (fs_ctx->fs_sm == SM_NONE)) { +} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) { +int pfd, err, serrno = 0; +char *tmp_path; +pfd = get_dirfd(fs_ctx, path); +if (pfd < 0) { +return -1; +} +tmp_path = qemu_strdup(path); +err = fchmodat(pfd, basename(tmp_path), credp->fc_mode, 0); +if (err == -1) { +serrno = errno; +} +qemu_free(tmp_path); +close(pfd); +errno = serrno; +return err; +} else if (fs_ctx->fs_sm == SM_NONE) { return chmod(rpath(fs_ctx, path), credp->fc_mode); } return -1; @@ -532,53 +604,148 @@ static int local_link(FsContext *fs_ctx, const char *oldpath, static int local_truncate(FsContext *ctx, const char *path, off_t size) { -return truncate(rpath(ctx, path), size); +if (ctx->fs_sm == SM_PASSTHROUGH) { +int fd, retval; +fd = passthrough_open(ctx, path, O_RDWR); +if (fd < 0) { +errno = -fd; +return -1; +} +retval = ftruncate(fd, size); +close(fd); +return retval; +} else { +return truncate(rpath(ctx, path), size); +} } static int local_rename(FsContext *ctx, const char *oldpath, const char *newpath) { -char *tmp; -int err; - -tmp = qemu_strdup
[Qemu-devel] KVM call agend for Mar 1
Please send in any agenda items you are interested in covering. Thanks, Juan.
[Qemu-devel] [RFC][PATCH 0/2] Allow cache settings for block devices to be changed at runtime.
The following patchset introduces monitor commands: 1. set_cache DEVICE CACHE-SETTING Change cache settings for block device, DEVICE, through the monitor. (Available options : 'none', 'writeback', 'writethrough') Eg, (qemu)set_cache ide0-hd0 none -> Changes cache setting for ide0-hd0 to 'none' 2. info block Now extended to display cache settings for available block devices. TODOS : --- 1. Support 'unsafe' cache mode. 2. Display current cache setting for device, if the CACHE-SETTING option is not supplied by the user. Eg, (qemu)set_cache ide0-hd0 presently errors out. Ideally, it should display current cache setting for the given device ide0-hd0 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
[Qemu-devel] jitter in Audio
Hi, Iam using qemu 0.13.0..whenever Iam playing any file using ffplay.sometimes it happens that audio stops and then after sometime gain it starts playing..but i dont see this problem with aplay. so whats going wrong.Plz update me as soon as possible. ---Thannx AK
[Qemu-devel] Re: [PATCH v3 uq/master 00/22] Win32 iothread support
On 2011-02-28 11:10, Paolo Bonzini wrote: > On 02/28/2011 10:16 AM, Avi Kivity wrote: >> Why is that? uq/master is for kvm code, and this touches two or three >> lines? > > iothread code is also going in via uq/master often. I started with > uq/master because I depended on Jan's changes which are now upstream, I > have no problems with this going directly to qemu.git. As this touches the iothread, a central subsystem full of kvm code, and as uq/master is known to be checked against autotest, I still think uq/master is the right choice. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] [V6 PATCH 0/9] virtio-9p: Use chroot to safely access files in passthrough security model
In passthrough security model, following symbolic links in the server side could result in TOCTTOU vulnerabilities. This patchset resolves this issue by creating a dedicated process which chroots into the share path and all file object access is done in the chroot environment. This patchset implements chroot enviroment, provides necessary functions that can be used by the passthrough function calls. Changes from version V5: * Return errno on failure instead of setting errno * Minor cleanups like updated comments, enable CONFIG_THREAD if CONFIG_VIRTFS is enabled Changes from version V4: * Avoid using malloc/free inside chroot process * Seperate chroot server and client functions Changes from version V3 * Return EIO incase of socket read/write fail instead of exiting * Changed data types as suggested by Blue Swirl * Chroot process reports error through qemu process Changes from version V2 * Treat socket IO errors as fatal, ie qemu will exit * Split patchset based on chroot side (server) and qemu side(client) functionalities M. Mohan Kumar (9): Implement qemu_read_full virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled virtio-9p: Provide chroot daemon side interfaces virtio-9p: Add qemu side interfaces for chroot environment virtio-9p: Add support to open a file in chroot environment virtio-9p: Create support in chroot environment virtio-9p: Support for creating special files virtio-9p: Move file post creation changes to none security model virtio-9p: Chroot environment for other functions Makefile.objs |1 + configure |1 + hw/9pfs/virtio-9p-chroot-dm.c | 282 ++ hw/9pfs/virtio-9p-chroot-qemu.c | 124 hw/9pfs/virtio-9p-chroot.h | 59 ++ hw/9pfs/virtio-9p-local.c | 418 +++ hw/9pfs/virtio-9p.c | 32 +++ hw/file-op-9p.h |4 + osdep.c | 32 +++ qemu-common.h |2 + 10 files changed, 875 insertions(+), 80 deletions(-) create mode 100644 hw/9pfs/virtio-9p-chroot-dm.c create mode 100644 hw/9pfs/virtio-9p-chroot-qemu.c create mode 100644 hw/9pfs/virtio-9p-chroot.h -- 1.7.3.4
[Qemu-devel] Re: [PATCH] Split machine creation from the main loop
On 02/28/2011 03:13 AM, Avi Kivity wrote: On 02/28/2011 10:57 AM, Paolo Bonzini wrote: On 02/28/2011 09:20 AM, Avi Kivity wrote: We should have another abstraction for connection based backend. I'll take a go at this when I'm ready to try to get those patches in. Shouldn't each new connection return a chardev? You would need a kind of "factory" interface that knows how to create a new {monitor,serial port,you name it} for each new connection. Actually it doesn't make much sense except for monitors. Monitors and vnc. VNC also does it's own buffering when reading and writing to a file descriptor. The chardev layer doesn't really have a concept of flow control. Regards, Anthony Liguori
[Qemu-devel] [RFC][PATCH 1/2] Add monitor command 'set-cache' to change cache settings for a block device.
Usage : (qemu) set_cache DEVICE CACHE-MODE where CACHE-MODE can be one of writeback/ writethrough/ none. At present, the image file is closed and re-opened with appropriate flags. It might potentially cause problems if the underlying image is deleted while a running qemu instance is using it. A change in cache operations will cause the image file to be closed, and a deleted file will be gone. Suggestions to fix this ? --- blockdev.c | 76 +++ blockdev.h |1 + hmp-commands.hx | 13 + 3 files changed, 90 insertions(+), 0 deletions(-) diff --git a/blockdev.c b/blockdev.c index 0690cc8..6735205 100644 --- a/blockdev.c +++ b/blockdev.c @@ -636,6 +636,82 @@ out: return ret; } +int do_set_cache(Monitor *mon, const QDict *qdict, QObject **ret_data) +{ +const char *device = qdict_get_str(qdict, "device"); +const char *cache = qdict_get_str(qdict, "cache"); +BlockDriverState *bs; +BlockDriver *drv; +int ret = 0; +int bdrv_flags = 0; + +if (!cache) { + /* TODO: in the absence of a change request, + simply display current cache setting. + Currently one needs 'info block' to query this */ +qerror_report(QERR_MISSING_PARAMETER, "cache"); +return -1; +} + +bs = bdrv_find(device); +if (!bs) { +qerror_report(QERR_DEVICE_NOT_FOUND, device); +return -1; +} + +/* Clear old flags */ +bdrv_flags = bs->open_flags; +if (bdrv_flags & BDRV_O_CACHE_MASK) { +bdrv_flags &= ~BDRV_O_CACHE_MASK; +} + +/* Determine flags for requested cache setting */ +if (!strcmp(cache, "none")) { +bdrv_flags |= BDRV_O_NOCACHE; +} else if (!strcmp(cache, "writeback")) { +bdrv_flags |= BDRV_O_CACHE_WB; +} else if (!strcmp(cache, "unsafe")) { + /* TODO : Support unsafe mode */ +qerror_report(QERR_INVALID_PARAMETER_VALUE, cache, + "writeback, writethrough, none"); +return -1; +} else if (!strcmp(cache, "writethrough")) { +/* Default setting */ +} else { +qerror_report(QERR_INVALID_PARAMETER_VALUE, cache, + "'cache' must be one of writeback, writethrough, none"); +return -1; +} + +/* Verify that the cache setting specified is different from current. + * Does NOT call for error return, since the 'request' is already + * honoured. + */ +if (bdrv_flags == bs->open_flags) { +qerror_report(QERR_PROPERTY_VALUE_IN_USE, device, "cache", cache); +return 0; +} + +/* Quiesce IO for the given block device */ +qemu_aio_flush(); +bdrv_flush(bs); + +/* Change cache value and restart IO on the block device */ +printf("Setting cache=%s for device %s [ filename %s ]", cache, device, +bs->filename ); +drv = bs->drv; +bdrv_close(bs); +ret = bdrv_open(bs, bs->filename, bdrv_flags, drv); +/* + * A failed attempt to reopen the image file must lead to 'abort()' + */ +if (ret != 0) { +abort(); +} + +return ret; +} + static int eject_device(Monitor *mon, BlockDriverState *bs, int force) { if (!force) { diff --git a/blockdev.h b/blockdev.h index 2c9e780..9f35817 100644 --- a/blockdev.h +++ b/blockdev.h @@ -63,6 +63,7 @@ int do_change_block(Monitor *mon, const char *device, const char *filename, const char *fmt); int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data); int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data); +int do_set_cache(Monitor *mon, const QDict *qdict, QObject **ret_data); int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data); #endif diff --git a/hmp-commands.hx b/hmp-commands.hx index 372bef4..18761cf 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1066,7 +1066,20 @@ STEXI @findex watchdog_action Change watchdog action. ETEXI +{ +.name = "set_cache", +.args_type = "device:B,cache:s", +.params = "device writeback|writethrough|none", +.help = "change cache settings for device", +.user_print = monitor_user_noop, +.mhandler.cmd_new = do_set_cache, +}, +STEXI +@item set_cache +@findex set_cache +Set cache options for a block device. +ETEXI { .name = "acl_show", .args_type = "aclname:s", -- 1.7.2.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
Re: [Qemu-devel] Link to UEFI OVMF on downloads page
On 02/28/2011 02:04 AM, Jordan Justen wrote: Hi all, Would it be possible to add a link to OVMF: http://sourceforge.net/apps/mediawiki/tianocore/index.php?title=OVMF on the qemu links page: http://wiki.qemu.org/Links for UEFI support on x86& x86-64 guests? Just create an account and add it yourself. Regards, Anthony Liguori Thanks, -Jordan
[Qemu-devel] [RFC][PATCH 2/2] Extend monitor command 'info block' to display cache settings for block devices.
(qemu)info block SAMPLE output : ide0-hd0: type=hd removable=0 cache=none file=/tmp/abc.img ro=0 drv=qcow2 encrypted=0 --- block.c | 22 -- 1 files changed, 20 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index f7d91a2..c717888 100644 --- a/block.c +++ b/block.c @@ -1707,6 +1707,23 @@ static void bdrv_print_dict(QObject *obj, void *opaque) monitor_printf(mon, " locked=%d", qdict_get_bool(bs_dict, "locked")); } +if (qdict_haskey(bs_dict, "open_flags") && +!strcmp(qdict_get_str(bs_dict, "type"), "hd")) { +int open_flags = qdict_get_int(bs_dict, "open_flags"); +if (open_flags & BDRV_O_NOCACHE) { +monitor_printf(mon, " cache=none"); +} else if (open_flags & BDRV_O_CACHE_WB) { +if (open_flags & BDRV_O_NO_FLUSH) { +monitor_printf(mon, " cache=unsafe"); +} +else { +monitor_printf(mon, " cache=writeback"); +} +} else { +monitor_printf(mon, " cache=writethrough"); +} +} + if (qdict_haskey(bs_dict, "inserted")) { QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted")); @@ -1756,9 +1773,10 @@ void bdrv_info(Monitor *mon, QObject **ret_data) } bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': %s, " -"'removable': %i, 'locked': %i }", +"'removable': %i, 'locked': %i, " +"'open_flags': %d }", bs->device_name, type, bs->removable, -bs->locked); +bs->locked, bs->open_flags); if (bs->drv) { QObject *obj; -- 1.7.2.3 -- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India
[Qemu-devel] Re: [PATCH v3 uq/master 00/22] Win32 iothread support
On 02/28/2011 01:57 PM, Jan Kiszka wrote: On 2011-02-28 11:10, Paolo Bonzini wrote: > On 02/28/2011 10:16 AM, Avi Kivity wrote: >> Why is that? uq/master is for kvm code, and this touches two or three >> lines? > > iothread code is also going in via uq/master often. I started with > uq/master because I depended on Jan's changes which are now upstream, I > have no problems with this going directly to qemu.git. As this touches the iothread, a central subsystem full of kvm code, and as uq/master is known to be checked against autotest, I still think uq/master is the right choice. If there's a git tree of this I'll be happy to do an autotest run. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Fix build errors introduced by vnc jpeg changes
On Mon, Feb 28, 2011 at 11:15:51AM +, Peter Maydell wrote: > On 28 February 2011 10:48, David Gibson wrote: > > The recent changes to handling of jpegs over tight vnc connections > > cause build errors if qemu is configured with --disable-vnc-jpeg. The > > patch below corrects the errors, by adding some left out #ifdefs. > > > > Please apply. > > > > Signed-off-by: David Gibson > > This is already fixed in master: > > http://git.qemu.org/qemu.git/commit/?id=cf76a1ce8b7cf4b92429d67d3f4626a92b2d8a37 Ah, sorry. Sent that mail several days ago, but it got stuck due to a mail configuration error. Forgot to recheck when I unstuck it. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
[Qemu-devel] Re: virtio-serial semantics for binary data and guest agents
On (Thu) 24 Feb 2011 [08:44:07], Anthony Liguori wrote: > For instance, if the host side disconnects, then reconnects before > we read(), we may never get the read()=0, and our FD remains valid. > Whereas with a tcp/unix socket our FD is no longer valid, and the > read()=0 is an event we can check for at any point after the other > end does a close/disconnect. > >>>There's SIGIO support, so host connect-disconnect notifications can be > >>>caught via the signal. > >>I recall looking into this at some pointbut don't we get a SIGIO > >>for read/write-ability in general? > >I don't get you -- the virtio_console driver emits the SIGIO signal > >only when the host side connects or disconnects. See > > Um, that's not the expected semantics of SIGIO. SIGIO can be > delivered for any number of reasons (including on a normal file > descriptor) so if there's no way to poll for the specific event then > the mechanism is inherently racy. Well -- for a custom driver something's going to be non-standard: if it's baking in connection info into the open/close semantics or having SIGIO delivered when needed. However, I think a generic library needs to be used for guest-host communications, abstracting away virtio-serial entirely. This way, apps should just communicate over tcp, the library should do the necessary virtio-serial (or even virt-9pfs) calls. This way, all guest apps written will be infrastructure-agnostic, and if there's a better protocol to communicate with, virtio-serial can easily be dropped w/o modifications to apps. Amit
Re: [Qemu-devel] Re: [patch 2/3] Add support for live block copy
On 02/28/2011 02:38 AM, Avi Kivity wrote: We don't separate configuration from guest state today. Instead of setting ourselves up for failure by setting an unrealistic standard that we try to achieve and never do, let's embrace the system that is working for us today. We are authoritative for everything and guest state is intimately tied to the virtual machine configuration. "we are authoritative for everything" is a clean break from everything that's being done today. It's also a clean break from the model of central management plus database. We can't force it on people. No, it isn't. This has been the way QEMU has always been. QEMU has always and will always continue to be useful in the absence of a management layer. That means that it will mix modifications to configuration with guest actions. To avoid races, a management tool needs to either poll QEMU or listen for events to know when QEMU initiates a configuration change. This is how tools are written today. The only thing being discussed is how to handle a very small and rare race condition whereas QEMU may send an notification to a crashed management tool *and* QEMU crashes before the management tool restarts. The only two options to solve this problem are synchronous notifications or a QEMU global state file. Since the former is a radical change to our existing API, the later is a much more reasonable option. If a management tool doesn't care about this race, it can simply not use the global state file. Regards, Anthony Liguori
Re: [Qemu-devel] Re: kvm crashes with spice while loading qxl
On Sun, Feb 27, 2011 at 8:03 PM, Alon Levy wrote: > On Sat, Feb 26, 2011 at 01:29:01PM +0100, Jan Kiszka wrote: >> On 2011-02-26 12:43, xming wrote: >> > When trying to start X (and it loads qxl driver) the kvm process just >> > crashes. > > This is fixed by Gerd's attached patch (taken from rhel repository, don't know > why it wasn't pushed to qemu-kvm upstream). I'll send it to kvm list as well > (separate email). I can confirm that this patch fixes the issue, thanks a lot cheers
[Qemu-devel] Re: GSoC 2011 project ideas
On Mon, 28 Feb 2011 10:09:43 +0100 Jan Kiszka wrote: > On 2011-02-28 00:44, Natalia Portillo wrote: > > Hi there, > > > > El 23/02/2011, a las 20:42, Luiz Capitulino escribió: > > > >> Hi there, > >> > >> Google will begin accepting mentoring organizations applications next > >> week, but > >> we count only with three projects so far. > >> > >> Although there doesn't seem to exist a hard deadline associated with the > >> ideas > >> page, nor with the number of projects, we had more than 20 projects > >> suggestions last year and a number of volunteering mentors. > > > > Is there any problem to repeat previously proposed and not chosen projects? > > > > I don't think so, and I've just ported one of my old proposals to this > year's page. This is ok, as long as the project is still valid of course. Thanks!
Re: [Qemu-devel] Re: [patch 2/3] Add support for live block copy
On 02/28/2011 02:45 PM, Anthony Liguori wrote: On 02/28/2011 02:38 AM, Avi Kivity wrote: We don't separate configuration from guest state today. Instead of setting ourselves up for failure by setting an unrealistic standard that we try to achieve and never do, let's embrace the system that is working for us today. We are authoritative for everything and guest state is intimately tied to the virtual machine configuration. "we are authoritative for everything" is a clean break from everything that's being done today. It's also a clean break from the model of central management plus database. We can't force it on people. No, it isn't. This has been the way QEMU has always been. QEMU has always and will always continue to be useful in the absence of a management layer. That means that it will mix modifications to configuration with guest actions. To avoid races, a management tool needs to either poll QEMU or listen for events to know when QEMU initiates a configuration change. This is how tools are written today. The only thing being discussed is how to handle a very small and rare race condition whereas QEMU may send an notification to a crashed management tool *and* QEMU crashes before the management tool restarts. The only two options to solve this problem are synchronous notifications or a QEMU global state file. Since the former is a radical change to our existing API, the later is a much more reasonable option. If a management tool doesn't care about this race, it can simply not use the global state file. You're just ignoring what I've written. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH v3 uq/master 00/22] Win32 iothread support
On 02/28/2011 01:13 PM, Avi Kivity wrote: If there's a git tree of this I'll be happy to do an autotest run. Sure, it's branch iothread-win32 of git://github.com/bonzini/qemu.git Paolo
Re: [Qemu-devel] [RFC][PATCH 0/2] Allow cache settings for block devices to be changed at runtime.
Am 28.02.2011 12:49, schrieb Prerna Saxena: > The following patchset introduces monitor commands: > > 1. set_cache DEVICE CACHE-SETTING > Change cache settings for block device, DEVICE, through the monitor. > (Available options : 'none', 'writeback', 'writethrough') > Eg, > (qemu)set_cache ide0-hd0 none > -> Changes cache setting for ide0-hd0 to 'none' Not sure if adding this interface is a good idea. I see that you only add it for HMP, and we may consider that, but it's definitely not suitable for QMP. One reason is that none/writethrough/writeback/unsafe isn't really what we want to use long term. We want to separate advertising a write cache (which is guest visible) from things like whether to use O_DIRECT or not. In the past, Christoph mentioned that he had patches to make these separate and even let the guest change the "write cache enabled" flag, which would probably solve most of the use cases of this patch. Christoph, what's the status of these patches? > 2. info block > Now extended to display cache settings for available block devices. Have you checked that libvirt is okay with this? Kevin
Re: [Qemu-devel] [RFC][PATCH 2/2] Extend monitor command 'info block' to display cache settings for block devices.
Am 28.02.2011 13:11, schrieb Prerna Saxena: > (qemu)info block > SAMPLE output : > ide0-hd0: type=hd removable=0 cache=none file=/tmp/abc.img ro=0 > drv=qcow2 encrypted=0 > > --- > block.c | 22 -- > 1 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/block.c b/block.c > index f7d91a2..c717888 100644 > --- a/block.c > +++ b/block.c > @@ -1707,6 +1707,23 @@ static void bdrv_print_dict(QObject *obj, void *opaque) > monitor_printf(mon, " locked=%d", qdict_get_bool(bs_dict, "locked")); > } > > +if (qdict_haskey(bs_dict, "open_flags") && > +!strcmp(qdict_get_str(bs_dict, "type"), "hd")) { > +int open_flags = qdict_get_int(bs_dict, "open_flags"); > +if (open_flags & BDRV_O_NOCACHE) { > +monitor_printf(mon, " cache=none"); > +} else if (open_flags & BDRV_O_CACHE_WB) { > +if (open_flags & BDRV_O_NO_FLUSH) { > +monitor_printf(mon, " cache=unsafe"); > +} > +else { > +monitor_printf(mon, " cache=writeback"); > +} > +} else { > +monitor_printf(mon, " cache=writethrough"); > +} > +} > + > if (qdict_haskey(bs_dict, "inserted")) { > QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted")); > > @@ -1756,9 +1773,10 @@ void bdrv_info(Monitor *mon, QObject **ret_data) > } > > bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': %s, " > -"'removable': %i, 'locked': %i }", > +"'removable': %i, 'locked': %i, " > +"'open_flags': %d }", > bs->device_name, type, bs->removable, > -bs->locked); > +bs->locked, bs->open_flags); IIUC, this is the structure that a QMP client will receive for a query-block command. The open_flags bitmask is not considered an ABI and completely meaningless outside qemu. Kevin
Re: [Qemu-devel] [PATCH (resend, rebase) 3/3] virtio-serial: Enable ioeventfd
On Mon, Feb 28, 2011 at 11:12 AM, Amit Shah wrote: > Enable ioeventfd for virtio-serial devices by default. Commit > 25db9ebe15125deb32958c6df74996f745edf1f9 lists the benefits of using > ioeventfd. > > Copying a file from guest to host over a virtio-serial channel didn't > show much difference in time or io_exit rate. The cost of enabling ioeventfd is one eventfd file descriptor and KVM in-kernel device slot per virtqueue. The current maximum number per VM is 200, this is a kernel limit in include/linux/kvm_host.h:NR_IOBUS_DEVS. Do you really want to use ioeventfd for virtio-serial? Perhaps this is more useful for high-frequency device interfaces. Stefan
Re: [Qemu-devel] [RFC][PATCH 0/2] Allow cache settings for block devices to be changed at runtime.
On Mon, Feb 28, 2011 at 3:12 PM, Kevin Wolf wrote: > Am 28.02.2011 12:49, schrieb Prerna Saxena: >> The following patchset introduces monitor commands: >> >> 1. set_cache DEVICE CACHE-SETTING >> Change cache settings for block device, DEVICE, through the monitor. >> (Available options : 'none', 'writeback', 'writethrough') >> Eg, >> (qemu)set_cache ide0-hd0 none >> -> Changes cache setting for ide0-hd0 to 'none' > > Not sure if adding this interface is a good idea. I see that you only > add it for HMP, and we may consider that, but it's definitely not > suitable for QMP. > > One reason is that none/writethrough/writeback/unsafe isn't really what > we want to use long term. We want to separate advertising a write cache > (which is guest visible) from things like whether to use O_DIRECT or not. > > In the past, Christoph mentioned that he had patches to make these > separate and even let the guest change the "write cache enabled" flag, > which would probably solve most of the use cases of this patch. Toggling host page cache at runtime is useful too because it saves having to restart VMs. I agree that the guest should control the emulated drive cache at runtime and we probably don't want to allow toggling that from the host - it could be dangerous :). Stefan
Re: [Qemu-devel] [RFC][PATCH 0/2] Allow cache settings for block devices to be changed at runtime.
Am 28.02.2011 16:35, schrieb Stefan Hajnoczi: > On Mon, Feb 28, 2011 at 3:12 PM, Kevin Wolf wrote: >> Am 28.02.2011 12:49, schrieb Prerna Saxena: >>> The following patchset introduces monitor commands: >>> >>> 1. set_cache DEVICE CACHE-SETTING >>> Change cache settings for block device, DEVICE, through the monitor. >>> (Available options : 'none', 'writeback', 'writethrough') >>> Eg, >>> (qemu)set_cache ide0-hd0 none >>> -> Changes cache setting for ide0-hd0 to 'none' >> >> Not sure if adding this interface is a good idea. I see that you only >> add it for HMP, and we may consider that, but it's definitely not >> suitable for QMP. >> >> One reason is that none/writethrough/writeback/unsafe isn't really what >> we want to use long term. We want to separate advertising a write cache >> (which is guest visible) from things like whether to use O_DIRECT or not. >> >> In the past, Christoph mentioned that he had patches to make these >> separate and even let the guest change the "write cache enabled" flag, >> which would probably solve most of the use cases of this patch. > > Toggling host page cache at runtime is useful too because it saves > having to restart VMs. Not sure why I wanted to change that during runtime, but agreed, allowing to change parameters using the monitor is generally a good thing. However, I'm not sure if a command for changing the cache mode is the right solution, or if it should be something like a command to change block device options. (For example, what about toggling read-only or snapshot mode?) > I agree that the guest should control the > emulated drive cache at runtime and we probably don't want to allow > toggling that from the host - it could be dangerous :). Good point. That's a NACK for this patch as long as we haven't separated WCE from the host cache setting. Kevin
[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD
On 02/01/2011 11:15 PM, Jan Kiszka wrote: From: Jan Kiszka Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. It's unused so far, but this infrastructure will be required for self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As Windows doesn't support signal services, we need to provide a stub for the init function. This patch breaks qemu-kvm after merging. The symptoms are that Windows XP x64 does not respond when netcat connects to some server in it, via -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, which is consistent with signals being messed up. I verified that 981085dd465c1 merged with ff48eb5fe79ad works, while 981085dd465c1 merged with ff48eb5fe79ad^ fails. diff --git a/cpus.c b/cpus.c index 42717ba..a33e470 100644 --- a/cpus.c +++ b/cpus.c @@ -231,11 +231,9 @@ fail: return err; } -#ifdef CONFIG_IOTHREAD static void dummy_signal(int sig) { } -#endif #else /* _WIN32 */ @@ -267,6 +265,32 @@ static void qemu_event_increment(void) #endif /* _WIN32 */ #ifndef CONFIG_IOTHREAD +static void qemu_kvm_init_cpu_signals(CPUState *env) +{ +#ifndef _WIN32 +int r; +sigset_t set; +struct sigaction sigact; + +memset(&sigact, 0, sizeof(sigact)); +sigact.sa_handler = dummy_signal; +sigaction(SIG_IPI,&sigact, NULL); + +sigemptyset(&set); +sigaddset(&set, SIG_IPI); +pthread_sigmask(SIG_BLOCK,&set, NULL); + +pthread_sigmask(SIG_BLOCK, NULL,&set); +sigdelset(&set, SIG_IPI); +sigdelset(&set, SIGBUS); +r = kvm_set_signal_mask(env,&set); +if (r) { +fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); +exit(1); +} +#endif +} + int qemu_init_main_loop(void) { cpu_set_debug_excp_handler(cpu_debug_handler); @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r)); exit(1); } +qemu_kvm_init_cpu_signals(env); } } -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD
On 2011-02-28 16:55, Avi Kivity wrote: > On 02/01/2011 11:15 PM, Jan Kiszka wrote: >> From: Jan Kiszka >> >> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. >> It's unused so far, but this infrastructure will be required for >> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As >> Windows doesn't support signal services, we need to provide a stub for >> the init function. >> > > This patch breaks qemu-kvm after merging. The symptoms are that Windows > XP x64 does not respond when netcat connects to some server in it, via > -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, > which is consistent with signals being messed up. Does the same test case work with qemu, iothread on and off? Just to ensure we are not hunting an issue with the patch itself but of the merge. Will have a look as well. Jan > > I verified that 981085dd465c1 merged with ff48eb5fe79ad works, > while 981085dd465c1 merged with ff48eb5fe79ad^ fails. > > >> diff --git a/cpus.c b/cpus.c >> index 42717ba..a33e470 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -231,11 +231,9 @@ fail: >> return err; >> } >> >> -#ifdef CONFIG_IOTHREAD >> static void dummy_signal(int sig) >> { >> } >> -#endif >> >> #else /* _WIN32 */ >> >> @@ -267,6 +265,32 @@ static void qemu_event_increment(void) >> #endif /* _WIN32 */ >> >> #ifndef CONFIG_IOTHREAD >> +static void qemu_kvm_init_cpu_signals(CPUState *env) >> +{ >> +#ifndef _WIN32 >> +int r; >> +sigset_t set; >> +struct sigaction sigact; >> + >> +memset(&sigact, 0, sizeof(sigact)); >> +sigact.sa_handler = dummy_signal; >> +sigaction(SIG_IPI,&sigact, NULL); >> + >> +sigemptyset(&set); >> +sigaddset(&set, SIG_IPI); >> +pthread_sigmask(SIG_BLOCK,&set, NULL); >> + >> +pthread_sigmask(SIG_BLOCK, NULL,&set); >> +sigdelset(&set, SIG_IPI); >> +sigdelset(&set, SIGBUS); >> +r = kvm_set_signal_mask(env,&set); >> +if (r) { >> +fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); >> +exit(1); >> +} >> +#endif >> +} >> + >> int qemu_init_main_loop(void) >> { >> cpu_set_debug_excp_handler(cpu_debug_handler); >> @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) >> fprintf(stderr, "kvm_init_vcpu failed: %s\n", >> strerror(-r)); >> exit(1); >> } >> +qemu_kvm_init_cpu_signals(env); >> } >> } >> > > -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD
On 2011-02-28 17:02, Jan Kiszka wrote: > On 2011-02-28 16:55, Avi Kivity wrote: >> On 02/01/2011 11:15 PM, Jan Kiszka wrote: >>> From: Jan Kiszka >>> >>> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. >>> It's unused so far, but this infrastructure will be required for >>> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As >>> Windows doesn't support signal services, we need to provide a stub for >>> the init function. >>> >> >> This patch breaks qemu-kvm after merging. The symptoms are that Windows >> XP x64 does not respond when netcat connects to some server in it, via >> -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, >> which is consistent with signals being messed up. > > Does the same test case work with qemu, iothread on and off? Just to Err, "iothread on" makes no sense here, of course. > ensure we are not hunting an issue with the patch itself but of the merge. > > Will have a look as well. > Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] [Bug 726619] [NEW] loadvm does not load (offline) snapshot anymore
Public bug reported: qemu Version: 0.14.0 The problem is present in the current code from git master as well. Loading a snapshot that was created while qemu was not running (using qemu-img) does not seem to work anymore. Using "loadvm " in the qemu monitor does not have the desired effect. Not even an error message is displayed. I was able to track down the problem (using git bisect) to this commit: http://git.qemu.org/qemu.git/commit/?id=f0aa7a8b2d518c54430e4382309281b93e51981a After reverting that commit in my local git checkout everything is workin as expected again. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/726619 Title: loadvm does not load (offline) snapshot anymore Status in QEMU: New Bug description: qemu Version: 0.14.0 The problem is present in the current code from git master as well. Loading a snapshot that was created while qemu was not running (using qemu-img) does not seem to work anymore. Using "loadvm " in the qemu monitor does not have the desired effect. Not even an error message is displayed. I was able to track down the problem (using git bisect) to this commit: http://git.qemu.org/qemu.git/commit/?id=f0aa7a8b2d518c54430e4382309281b93e51981a After reverting that commit in my local git checkout everything is workin as expected again.
[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD
On 2011-02-28 16:55, Avi Kivity wrote: > On 02/01/2011 11:15 PM, Jan Kiszka wrote: >> From: Jan Kiszka >> >> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. >> It's unused so far, but this infrastructure will be required for >> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As >> Windows doesn't support signal services, we need to provide a stub for >> the init function. >> > > This patch breaks qemu-kvm after merging. The symptoms are that Windows > XP x64 does not respond when netcat connects to some server in it, via > -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, > which is consistent with signals being messed up. > > I verified that 981085dd465c1 merged with ff48eb5fe79ad works, > while 981085dd465c1 merged with ff48eb5fe79ad^ fails. > > >> diff --git a/cpus.c b/cpus.c >> index 42717ba..a33e470 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -231,11 +231,9 @@ fail: >> return err; >> } >> >> -#ifdef CONFIG_IOTHREAD >> static void dummy_signal(int sig) >> { >> } >> -#endif >> >> #else /* _WIN32 */ >> >> @@ -267,6 +265,32 @@ static void qemu_event_increment(void) >> #endif /* _WIN32 */ >> >> #ifndef CONFIG_IOTHREAD >> +static void qemu_kvm_init_cpu_signals(CPUState *env) >> +{ >> +#ifndef _WIN32 >> +int r; >> +sigset_t set; >> +struct sigaction sigact; >> + >> +memset(&sigact, 0, sizeof(sigact)); >> +sigact.sa_handler = dummy_signal; >> +sigaction(SIG_IPI,&sigact, NULL); >> + >> +sigemptyset(&set); >> +sigaddset(&set, SIG_IPI); >> +pthread_sigmask(SIG_BLOCK,&set, NULL); >> + >> +pthread_sigmask(SIG_BLOCK, NULL,&set); >> +sigdelset(&set, SIG_IPI); >> +sigdelset(&set, SIGBUS); >> +r = kvm_set_signal_mask(env,&set); >> +if (r) { >> +fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); >> +exit(1); >> +} >> +#endif >> +} >> + >> int qemu_init_main_loop(void) >> { >> cpu_set_debug_excp_handler(cpu_debug_handler); >> @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) >> fprintf(stderr, "kvm_init_vcpu failed: %s\n", >> strerror(-r)); >> exit(1); >> } >> +qemu_kvm_init_cpu_signals(env); Just comment that out as long as qemu-kvm is (mis-)using !IOTHREAD mode. I thought it would run before setup_kernel_sigmask, but it's the other way around, and then the wrong non-iothread signal setup is applied. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] QEMU: Discussion of separating core functionality vs supportive features
Hi, On last week's call we discussed the issue of splitting non core features of QEMU into it's own process to reduce the security risks etc. I wrote up a summary of my thoughts on this to try to cover the various issues. Feedback welcome and hopefully we can continue the discussion on a future call - maybe next week? I would like to be part of the discussion, but it's a public holiday here March 1st, so I won't be on that call. Cheers, Jes Separating host-side virtagent and other tasks from core QEMU = To improve auditing of the core QEMU code, it would be ideal to be able to separate the core QEMU functionality from utility functionality by moving the utility functionality into its own process. This process will be referred to as the QEMU client below. Components which are candidates for moving out of QEMU include: - virtagent - vnc server (and other graphical displays such as SDL, spice and curses) - human monitor The idea is to have QEMU launch as a daemon, and then allow for one of more client processes to connect to it. These will then offer the various services. The main issue to discuss is how to handle various state information, reconnects, and migration. Security The primary reason for this discussion is security, however there are other benefits from this split, as will be mentioned below. During a demo of virtagent I hit a case where a bug in the agent command handling code caused a crash of the host QEMU process. While it is probably a simple bug, it shows how adding more complexity to the QEMU process increases the risk of adding security problems that could potentially be exploited by a hostile guest. By splitting non core functionality into a QEMU client process, the host process will be isolated from a large number of potential security problems, ie. in case a client process is killed or crashes, it should not affect the main QEMU process. In addition it makes it easier to audit the core QEMU functionality. virtagent = In short virtagent provides a set of simple commands, most of which do not have state associated with them. These include shutdown, ping, fsfreeze/fsthaw, etc. Other commands might be multi-stage commands which could fail if the client is disconnected from the daemon while the command is in progress. These include copy-paste and file copy. vnc server == The vnc server simply needs a connection to the video memory of the QEMU process, video mode state, as well as channels for sending keyboard and mouse events. It is stateless by nature and supports reconnects. This applies to the other graphical display engines (SDL, spice, and curses) as well. Human monitor = The human monitor is effectively stateless. It issues commands and prints the result. There is no state in the monitor and it can be built directly on top of QMP. An additional benefit here is that it would allow for multiple monitors. Disconnects === It must be possible for a client process getting killed or disconnected from the QEMU process, in which case is should be possible to launch a new client that connects to the QEMU process. In this case, commands needs to be provided allowing the client process to query the QEMU process and virtagent for current state information. In-progress commands may fail, and will need to be re-run, such as copy-paste and and file copy. However neither of these are vital commands and a re-run of such commands is acceptable behavior. Migration = Given that migration moves the guest to a new QEMU process, normally on a different host, any connection from management tools to the monitor, QMP sockets, virtio-serial, etc. require a new connection through the new QEMU process. Stateless connections, such as the monitor, QMP and the vnc-server handles reconnects without problems, and should not have any issues during migration that are different from the issues in the current implementation. The main issues causing problems are stateful events such as copy-paste, which is handled via a guest agent. The copy-paste problem can be handled by blocking copy-paste operations during migration, until the guest agent is reachable through the QEMU process on the new host. This does mean that copy-paste can block or fail temporarily (depending on whether is it implemented as -EBUSY or just blocks), however it is not a mission critical feature, and it can also block or fail temporarily during normal operation on a non virtual system. Per the nature of the operation, a file copy via a guest agent is going to fail and will have to be restarted after migration has completed, in case it does not complete. This is again a non critical feature and requiring a restart is acceptable.
[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD
On 02/28/2011 06:16 PM, Jan Kiszka wrote: On 2011-02-28 16:55, Avi Kivity wrote: > On 02/01/2011 11:15 PM, Jan Kiszka wrote: >> From: Jan Kiszka >> >> Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. >> It's unused so far, but this infrastructure will be required for >> self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As >> Windows doesn't support signal services, we need to provide a stub for >> the init function. >> > > This patch breaks qemu-kvm after merging. The symptoms are that Windows > XP x64 does not respond when netcat connects to some server in it, via > -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, > which is consistent with signals being messed up. > > I verified that 981085dd465c1 merged with ff48eb5fe79ad works, > while 981085dd465c1 merged with ff48eb5fe79ad^ fails. > > >> diff --git a/cpus.c b/cpus.c >> index 42717ba..a33e470 100644 >> --- a/cpus.c >> +++ b/cpus.c >> @@ -231,11 +231,9 @@ fail: >>return err; >>} >> >> -#ifdef CONFIG_IOTHREAD >>static void dummy_signal(int sig) >>{ >>} >> -#endif >> >>#else /* _WIN32 */ >> >> @@ -267,6 +265,32 @@ static void qemu_event_increment(void) >>#endif /* _WIN32 */ >> >>#ifndef CONFIG_IOTHREAD >> +static void qemu_kvm_init_cpu_signals(CPUState *env) >> +{ >> +#ifndef _WIN32 >> +int r; >> +sigset_t set; >> +struct sigaction sigact; >> + >> +memset(&sigact, 0, sizeof(sigact)); >> +sigact.sa_handler = dummy_signal; >> +sigaction(SIG_IPI,&sigact, NULL); >> + >> +sigemptyset(&set); >> +sigaddset(&set, SIG_IPI); >> +pthread_sigmask(SIG_BLOCK,&set, NULL); >> + >> +pthread_sigmask(SIG_BLOCK, NULL,&set); >> +sigdelset(&set, SIG_IPI); >> +sigdelset(&set, SIGBUS); >> +r = kvm_set_signal_mask(env,&set); >> +if (r) { >> +fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); >> +exit(1); >> +} >> +#endif >> +} >> + >>int qemu_init_main_loop(void) >>{ >>cpu_set_debug_excp_handler(cpu_debug_handler); >> @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) >>fprintf(stderr, "kvm_init_vcpu failed: %s\n", >> strerror(-r)); >>exit(1); >>} >> +qemu_kvm_init_cpu_signals(env); Just comment that out as long as qemu-kvm is (mis-)using !IOTHREAD mode. I thought it would run before setup_kernel_sigmask, but it's the other way around, and then the wrong non-iothread signal setup is applied. That's what I tried, and it didn't work?! Maybe I forgot to compile or something. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD
On 02/28/2011 06:45 PM, Avi Kivity wrote: That's what I tried, and it didn't work?! Maybe I forgot to compile or something. I misspelled #ifdef. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD
On 2011-02-28 17:45, Avi Kivity wrote: > On 02/28/2011 06:16 PM, Jan Kiszka wrote: >> On 2011-02-28 16:55, Avi Kivity wrote: >>> On 02/01/2011 11:15 PM, Jan Kiszka wrote: From: Jan Kiszka Block SIG_IPI, unblock it during KVM_RUN, just like in io-thread mode. It's unused so far, but this infrastructure will be required for self-IPIs and to process SIGBUS plus, in KVM mode, SIGIO and SIGALRM. As Windows doesn't support signal services, we need to provide a stub for the init function. >>> >>> This patch breaks qemu-kvm after merging. The symptoms are that Windows >>> XP x64 does not respond when netcat connects to some server in it, via >>> -net user,hostfwd. The vcpu thread loops indefinitely on KVM_EXIT_INTR, >>> which is consistent with signals being messed up. >>> >>> I verified that 981085dd465c1 merged with ff48eb5fe79ad works, >>> while 981085dd465c1 merged with ff48eb5fe79ad^ fails. >>> >>> diff --git a/cpus.c b/cpus.c index 42717ba..a33e470 100644 --- a/cpus.c +++ b/cpus.c @@ -231,11 +231,9 @@ fail: return err; } -#ifdef CONFIG_IOTHREAD static void dummy_signal(int sig) { } -#endif #else /* _WIN32 */ @@ -267,6 +265,32 @@ static void qemu_event_increment(void) #endif /* _WIN32 */ #ifndef CONFIG_IOTHREAD +static void qemu_kvm_init_cpu_signals(CPUState *env) +{ +#ifndef _WIN32 +int r; +sigset_t set; +struct sigaction sigact; + +memset(&sigact, 0, sizeof(sigact)); +sigact.sa_handler = dummy_signal; +sigaction(SIG_IPI,&sigact, NULL); + +sigemptyset(&set); +sigaddset(&set, SIG_IPI); +pthread_sigmask(SIG_BLOCK,&set, NULL); + +pthread_sigmask(SIG_BLOCK, NULL,&set); +sigdelset(&set, SIG_IPI); +sigdelset(&set, SIGBUS); +r = kvm_set_signal_mask(env,&set); +if (r) { +fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(-r)); +exit(1); +} +#endif +} + int qemu_init_main_loop(void) { cpu_set_debug_excp_handler(cpu_debug_handler); @@ -292,6 +316,7 @@ void qemu_init_vcpu(void *_env) fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r)); exit(1); } +qemu_kvm_init_cpu_signals(env); >> >> Just comment that out as long as qemu-kvm is (mis-)using !IOTHREAD mode. >> I thought it would run before setup_kernel_sigmask, but it's the other >> way around, and then the wrong non-iothread signal setup is applied. > > That's what I tried, and it didn't work?! Maybe I forgot to compile or > something. Well, it maybe failed to build as qemu_kvm_init_cpu_signals became unused and the compiler should have bailed out? Probably it's better to disable it directly in the function. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH v2 13/24] kvm: Set up signal mask also for !CONFIG_IOTHREAD
On 02/28/2011 06:49 PM, Jan Kiszka wrote: > > That's what I tried, and it didn't work?! Maybe I forgot to compile or > something. Well, it maybe failed to build as qemu_kvm_init_cpu_signals became unused and the compiler should have bailed out? Probably it's better to disable it directly in the function. That's what I did, with #ifdefs, but brokenly (#ifndef). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [patch 2/3] Add support for live block copy
On Feb 28, 2011 7:21 AM, "Avi Kivity" wrote: > > On 02/28/2011 02:45 PM, Anthony Liguori wrote: >> >> On 02/28/2011 02:38 AM, Avi Kivity wrote: We don't separate configuration from guest state today. Instead of setting ourselves up for failure by setting an unrealistic standard that we try to achieve and never do, let's embrace the system that is working for us today. We are authoritative for everything and guest state is intimately tied to the virtual machine configuration. >>> >>> >>> "we are authoritative for everything" is a clean break from everything that's being done today. It's also a clean break from the model of central management plus database. We can't force it on people. >> >> >> No, it isn't. This has been the way QEMU has always been. >> >> QEMU has always and will always continue to be useful in the absence of a management layer. That means that it will mix modifications to configuration with guest actions. >> >> To avoid races, a management tool needs to either poll QEMU or listen for events to know when QEMU initiates a configuration change. This is how tools are written today. >> >> The only thing being discussed is how to handle a very small and rare race condition whereas QEMU may send an notification to a crashed management tool *and* QEMU crashes before the management tool restarts. >> >> The only two options to solve this problem are synchronous notifications or a QEMU global state file. Since the former is a radical change to our existing API, the later is a much more reasonable option. >> >> If a management tool doesn't care about this race, it can simply not use the global state file. > > > You're just ignoring what I've written. No, you're just impervious to my subtle attempt to refocus the discussion on solving a practical problem. There's a lot of good, reasonably straight forward changes we can make that have a high return on investment. The only suggestion I'm making beyond Marcelo's original patch is that we use a structured format and that we make it possible to use the same file to solve this problem in multiple places. I don't think this creates a fundamental break in how management tools interact with QEMU. I don't think introducing RAID support in the block layer is a reasonable alternative. Regards, Anthony Liguori > > > -- > error compiling committee.c: too many arguments to function >
Re: [Qemu-devel] QEMU: Discussion of separating core functionality vs supportive features
On Feb 28, 2011 10:44 AM, "Jes Sorensen" wrote: > > Hi, > > On last week's call we discussed the issue of splitting non core > features of QEMU into it's own process to reduce the security risks etc. > > I wrote up a summary of my thoughts on this to try to cover the various > issues. Feedback welcome and hopefully we can continue the discussion on > a future call - maybe next week? > > I would like to be part of the discussion, but it's a public holiday > here March 1st, so I won't be on that call. > > Cheers, > Jes > > > Separating host-side virtagent and other tasks from core QEMU > = > > To improve auditing of the core QEMU code, it would be ideal to be > able to separate the core QEMU functionality from utility > functionality by moving the utility functionality into its own > process. This process will be referred to as the QEMU client below. Separating as in moving code outside of qemu.git, making code optionally built in, making code optionally built in or loadable as a separate executable that is automatically launched, or making code always built outside the main executable? I'm very nervous about having a large number of daemons necessary to run QEMU. I think a reasonable approach would be a single front-end daemond. Once QAPI is merged, there is a very incremental approach we can take for this sort of work. Take your favorite subsystem (like gdbstub or SDL) and make it only use QMP apis. Once we're only using QMP internally in a subsystem, then building it out of the main QEMU and using libqmp should be fairly easy. Regards, Anthony Liguori > Components which are candidates for moving out of QEMU include: > - virtagent > - vnc server (and other graphical displays such as SDL, spice and > curses) > - human monitor These are all much harder than they may seem. There's a ton of QMP functions that would be needed before we could even try to do this. > The idea is to have QEMU launch as a daemon, and then allow for one of > more client processes to connect to it. These will then offer the > various services. The main issue to discuss is how to handle various > state information, reconnects, and migration. > > Security > > > The primary reason for this discussion is security, however there are > other benefits from this split, as will be mentioned below. During a > demo of virtagent I hit a case where a bug in the agent command > handling code caused a crash of the host QEMU process. While it is > probably a simple bug, it shows how adding more complexity to the QEMU > process increases the risk of adding security problems that could > potentially be exploited by a hostile guest. > > By splitting non core functionality into a QEMU client process, the > host process will be isolated from a large number of potential > security problems, ie. in case a client process is killed or crashes, > it should not affect the main QEMU process. In addition it makes it > easier to audit the core QEMU functionality. > > virtagent > = > > In short virtagent provides a set of simple commands, most of which > do not have state associated with them. These include shutdown, ping, > fsfreeze/fsthaw, etc. Other commands might be multi-stage commands > which could fail if the client is disconnected from the daemon while > the command is in progress. These include copy-paste and file copy. > > vnc server > == > > The vnc server simply needs a connection to the video memory of the > QEMU process, video mode state, as well as channels for sending > keyboard and mouse events. It is stateless by nature and supports > reconnects. This applies to the other graphical display engines (SDL, > spice, and curses) as well. > > Human monitor > = > > The human monitor is effectively stateless. It issues commands and > prints the result. There is no state in the monitor and it can be > built directly on top of QMP. > > An additional benefit here is that it would allow for multiple > monitors. > > Disconnects > === > > It must be possible for a client process getting killed or > disconnected from the QEMU process, in which case is should be > possible to launch a new client that connects to the QEMU > process. In this case, commands needs to be provided allowing the > client process to query the QEMU process and virtagent for current > state information. In-progress commands may fail, and will need to be > re-run, such as copy-paste and and file copy. However neither of > these are vital commands and a re-run of such commands is acceptable > behavior. > > Migration > = > > Given that migration moves the guest to a new QEMU process, normally > on a different host, any connection from management tools to the > monitor, QMP sockets, virtio-serial, etc. require a new connection > through the new QEMU process. Stateless connections, such as the > monitor, QMP and the vnc-server handles reconnects without problems, >
Re: [Qemu-devel] Re: [patch 2/3] Add support for live block copy
On 02/28/2011 07:33 PM, Anthony Liguori wrote: > > You're just ignoring what I've written. No, you're just impervious to my subtle attempt to refocus the discussion on solving a practical problem. There's a lot of good, reasonably straight forward changes we can make that have a high return on investment. Is making qemu the authoritative source of configuration information a straightforward change? Is the return on it high? Is the investment low? "No" to all three (ignoring for the moment whether it is good or not, which we were debating). The only suggestion I'm making beyond Marcelo's original patch is that we use a structured format and that we make it possible to use the same file to solve this problem in multiple places. No, you're suggesting a lot more than that. I don't think this creates a fundamental break in how management tools interact with QEMU. I don't think introducing RAID support in the block layer is a reasonable alternative. Why not? Something that avoids the whole state thing altogether: - instead of atomically switching when live copy is done, keep on issuing writes to both the origin and the live copy - issue a notification to management - management receives the notification, and issues an atomic blockdev switch command this is really the RAID-1 solution but without the state file (credit Dor). An advantage is that there is no additional latency when trying to catch up to the dirty bitmap. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [patch 2/3] Add support for live block copy
On Feb 28, 2011 11:47 AM, "Avi Kivity" wrote: > > On 02/28/2011 07:33 PM, Anthony Liguori wrote: >> >> >> > >> > You're just ignoring what I've written. >> >> No, you're just impervious to my subtle attempt to refocus the discussion on solving a practical problem. >> >> There's a lot of good, reasonably straight forward changes we can make that have a high return on investment. >> > > Is making qemu the authoritative source of configuration information a straightforward change? Is the return on it high? Is the investment low? I think this is where we fundamentally disagree. My position is that QEMU is already the authoritative source. Having a state file doesn't change anything. Do a hot unplug of a network device with upstream libvirt with acpiphp unloaded, consult libvirt and then consult the monitor to see who has the right view of the guests config. To me, that's the definition of authoritative. > "No" to all three (ignoring for the moment whether it is good or not, which we were debating). > > >> The only suggestion I'm making beyond Marcelo's original patch is that we use a structured format and that we make it possible to use the same file to solve this problem in multiple places. >> > > No, you're suggesting a lot more than that. That's exactly what I'm suggesting from a technical perspective. >> I don't think this creates a fundamental break in how management tools interact with QEMU. I don't think introducing RAID support in the block layer is a reasonable alternative. >> >> > > Why not? Because its a lot of complexity and code that can go wrong while only solving the race for one specific case. Not to mention that we double the iop rate. > Something that avoids the whole state thing altogether: > > - instead of atomically switching when live copy is done, keep on issuing writes to both the origin and the live copy > - issue a notification to management > - management receives the notification, and issues an atomic blockdev switch command > this is really the RAID-1 solution but without the state file (credit Dor). An advantage is that there is no additional latency when trying to catch up to the dirty bitmap. It still suffers from the two generals problem. You cannot solve this without making one node reliable and that takes us back to it being either QEMU (posted event and state file) or the management tool (sync event). Regards, Anthony Liguori > > -- > error compiling committee.c: too many arguments to function >
[Qemu-devel] Re: GSoC 2011 project ideas
I have added my 2010 still valid projects (all), and three more. I also added myself as mentor for the USB projects, as I recently got experience on how the QEMU's USB stack works. One of the projects I suggest, implementing USB 3.0 XHCI may need to be merged with clean up of Gerd's USB 2.0 EHCI emulation patches. I think just cleaning up for mainstream the patches done by Gerd is too simple and fast-to-be-done for GSoC, but as it's Jan's proposition I will not merge them. Jan if you agree with me, feel free to merge both projects, I'll mentor it. Regards, Natalia Portillo
[Qemu-devel] SymbianOS, MeeGO, WebOS and QEMU
Hi all, Last time I checked SymbianOS source repository I found references to QEMU. Are they using QEMU for the simulator? And for MeeGO? May HP also be using it for WebOS? We may propose putting their modifications upstream as a GSoC 2011 project if it's the case. Regards, Natalia Portillo
Re: [Qemu-devel] Re: [patch 2/3] Add support for live block copy
On Mon, Feb 28, 2011 at 07:47:13PM +0200, Avi Kivity wrote: > On 02/28/2011 07:33 PM, Anthony Liguori wrote: > > > >> > >> You're just ignoring what I've written. > > > >No, you're just impervious to my subtle attempt to refocus the > >discussion on solving a practical problem. > > > >There's a lot of good, reasonably straight forward changes we can > >make that have a high return on investment. > > > > Is making qemu the authoritative source of configuration information > a straightforward change? Is the return on it high? Is the > investment low? > > "No" to all three (ignoring for the moment whether it is good or > not, which we were debating). > > >The only suggestion I'm making beyond Marcelo's original patch is > >that we use a structured format and that we make it possible to > >use the same file to solve this problem in multiple places. > > > > No, you're suggesting a lot more than that. > > >I don't think this creates a fundamental break in how management > >tools interact with QEMU. I don't think introducing RAID support > >in the block layer is a reasonable alternative. > > > > > > Why not? > > Something that avoids the whole state thing altogether: > > - instead of atomically switching when live copy is done, keep on > issuing writes to both the origin and the live copy > - issue a notification to management > - management receives the notification, and issues an atomic > blockdev switch command How do you know if QEMU performed the switch or not? That is, how can the switch command be atomic? > this is really the RAID-1 solution but without the state file > (credit Dor). An advantage is that there is no additional latency > when trying to catch up to the dirty bitmap. So you're implementing a virtual block driver not visible to the user as an image format. The image format would allow storage of persistent copy progress, etc. so you lose all that. All of that to avoid introducing a commit file which is not part of global qemu state.
Re: [Qemu-devel] [patch 2/3] Add support for live block copy
On Sat, Feb 26, 2011 at 07:45:44AM -0600, Anthony Liguori wrote: > >>>+- "commit_filename": target commit filename (json-string, optional) > >>I think we should drop this. > >Why? Sorry but this can't wait for non-config persistent storage. This > >mistake was made in the past with irqchip for example, lets not repeat > >it. > > > >Its OK to deprecate "commit_filename" in favour of its location in > >non-config persistent storage. > > > >Its not the end of the world for a mgmt app to handle change (not saying > >its not a good principle) such as this. > > Even as a one off, it's not a very good solution to the problem. > We'd be way better of just having nothing here than using the commit > file. What are the semantics of a half written file? How does a > management tool detect a half written file? If the commit file contains the full commit message, it can be considered valid. Otherwise, it should be considered invalid. Stopping the guest and waiting for mgmt to issue a continue command is a solution, but it has drawbacks introduced by reliance on mgmt app (what if mgmt app crashes, latency, etc). But it seems that it is preferred over a commit file. Addressing the other comments in the meantime, thanks for input.