Re: [Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not
Am 12.08.2012 12:32, schrieb Michael Tokarev: On 12.08.2012 01:24, Peter Maydell wrote: POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero msg.msg_iovlen (in particular the MacOS X implementation will do this). Handle the case where iov_send_recv() is passed a zero byte count explicitly, to avoid accidentally depending on the OS to treat zero msg_iovlen as a no-op. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- This is what was causing 'make check' to fail on MacOS X. The other option was to declare that a zero bytecount was illegal, I guess. Acked-by: Michael Tokarev m...@tls.msk.ru Kevin, does this fix the test-iov failure you're seeing on one of the build bots? It's not on a build bot but on my test machine, but yes, this does fix it indeed. Thanks for looking into it, Peter! Kevin
Re: [Qemu-devel] [RFC V2 03/10] quorum: Add quorum_open().
Am 10.08.2012 19:48, schrieb Benoît Canet: Le Tuesday 07 Aug 2012 à 20:30:09 (+), Blue Swirl a écrit : On Tue, Aug 7, 2012 at 1:44 PM, Benoît Canet benoit.ca...@gmail.com wrote: Signed-off-by: Benoit Canet ben...@irqsave.net --- block/quorum.c | 62 1 file changed, 62 insertions(+) diff --git a/block/quorum.c b/block/quorum.c index e0405b6..de58ab8 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -47,11 +47,73 @@ struct QuorumAIOCB { int vote_ret; }; +/* Valid quorum filenames look like + * quorum:path/to/a_image:path/to/b_image:path/to/c_image This syntax would mean that stacking for example curl or other network paths would not be possible. How about comma as separator? I just tried comma but it fail because the qemu command line parsing catch it believing that the string next to the coma is another file= like options. Is there any other separator we can use ? I would ignore that for now, or you can introduce your own escaping of colons. The real solution is, once again, -blockdev. Kevin
Re: [Qemu-devel] [PATCH] block: Set cdrom device read only flag
Am 12.08.2012 04:48, schrieb Kevin Shanahan: So qmp_change_blockdev uses bdrv_is_read_only() to check whether to try and open the backing file read only, which uses the -read_only member of struct BlockDriverState to decide whether to pass the BDRV_O_RDRW flag to qmp_bdrv_open_encypted() and then bdrv_open(). I would assume we want to set this flag in drive_init() when the block driver state is initialised. How about a patch like this instead? diff --git a/blockdev.c b/blockdev.c index 8669142..ba22064 100644 --- a/blockdev.c +++ b/blockdev.c @@ -526,6 +526,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) if_name[type], mediastr, unit_id); } dinfo-bdrv = bdrv_new(dinfo-id); +dinfo-bdrv-read_only = ro; dinfo-devaddr = devaddr; dinfo-type = type; dinfo-bus = bus_id; Ah, yes, this looks much more like the proper fix. Basically we need to set everything that is retained after a 'change' command. We have this code in qmp_change_blockdev(): bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR; bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0; qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp); bdrv_is_read_only() is covered by your patch, bdrv_is_snapshot() additionally requires bs-open_flags to be right. Markus, how will this look in the -blockdev world? There seem to be properties that belong to host state, but are not coupled to a medium. Kevin
Re: [Qemu-devel] TRIM, UNMAP and QCOW2 release of block information - Thin provisioning
On Sun, Aug 12, 2012 at 8:00 PM, Gerhard Wiesinger li...@wiesinger.com wrote: As far as I saw QEMU/KVM supports the trim command on IDE/SATA devices and the UNMAP command on SCSI devices/disks (thanks Paolo Bonzini). Will the qcow2 format (or other formats) use this information and also release the blocks for thin provisioning to save disk space on filesystems? VirtualBox also has such a feature in the new beta version: http://www.fb-developers.info/blog/2012/virtualbox-v4-2-is-coming/ This has recently been discussed, please search the list for more info from Kevin Wolf or Paolo Bonzini. qcow2 marks the discarded blocks as free and will reuse them in future allocations. It does *not* discard at the OS level. For raw files you can get discard to work on an xfs host file system. In the future ext4 support can also be added. Stefan
[Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection
From: Nicholas Bellinger n...@linux-iscsi.org This is required to get past the following assert with: commit 1523ed9e1d46b0b54540049d491475ccac7e6421 Author: Jan Kiszka jan.kis...@siemens.com Date: Thu May 17 10:32:39 2012 -0300 virtio/vhost: Add support for KVM in-kernel MSI injection Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Jan Kiszka jan.kis...@siemens.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Anthony Liguori aligu...@us.ibm.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- hw/msix.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/hw/msix.c b/hw/msix.c index 800fc32..c1e6dc3 100644 --- a/hw/msix.c +++ b/hw/msix.c @@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev) { int vector; +if (!dev-msix_vector_use_notifier !dev-msix_vector_release_notifier) +return; + assert(dev-msix_vector_use_notifier dev-msix_vector_release_notifier); -- 1.7.2.5
[Qemu-devel] [RFC-v2 0/6] vhost-scsi: Add support for host virtualized target
From: Nicholas Bellinger n...@linux-iscsi.org Hi Paolo, Stefan, QEMU folks, The following is the second RFC series for vhost-scsi patches against mainline QEMU v1.1.0. The series is available from the following working branch: git://git.kernel.org/pub/scm/virt/kvm/nab/qemu-kvm.git vhost-scsi-merge Apologies for the delayed follow-up on this series. The changes detailed below addresses Paolo's original comments on vhost-scsi code from the last weeks. As of this evening the tcm_vhost driver has now been merged into the mainline kernel for 3.6-rc2 here: tcm_vhost: Initial merge for vhost level target fabric driver http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297877 Also, after following up on the qemu-kvm IRQ injection changes (from Jan) that caused a performance regerssion with QEMU 1.1.0 code originally reported here: vhost-scsi port to v1.1.0 + MSI-X performance regression http://comments.gmane.org/gmane.linux.scsi.target.devel/2277 It turns out that setting explict virtio-queue IRQ affinity within guest appears to bring small block random IOPs performance back up to the pre IRQ injection conversion levels. I'm not sure why this ended up making so much of a difference post IRQ injection conversion, but setting virtio-queue affinity is now getting us back to pre IQN injection conversion levels. Changes from v1 - v2: - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as starting point for v3.6-rc code (Stefan + ALiguori + nab) - Fix upstream qemu conflict in hw/qdev-properties.c - Make GET_ABI_VERSION use int (nab + mst) - Drop unnecessary event-notifier changes (nab) - Fix vhost-scsi case lables in configure (reported by paolo) - Convert qdev_prop_vhost_scsi to use -get() + -set() following qdev_prop_netdev (reported by paolo) - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo) - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab) - Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab) - Drop usage of to_virtio_scsi() in virtio_scsi_set_status() (reported by paolo) - Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo) - Use s-conf-vhost_scsi instead of proxyconf-vhost_scsi in virtio_scsi_init() (reported by paolo) - Only register QEMU SCSI bus is vhost-scsi is not active (reported by paolo) - Fix incorrect VirtIOSCSI-cmd_vqs[0] definition (nab) Please have another look, and let me know if anything else needs to be addressed. Thanks! --nab Nicholas Bellinger (3): msix: Work-around for vhost-scsi with KVM in-kernel MSI injection virtio-scsi: Set max_target=0 during vhost-scsi operation virtio-scsi: Fix incorrect VirtIOSCSI-cmd_vqs[0] definition Stefan Hajnoczi (3): vhost: Pass device path to vhost_dev_init() vhost-scsi: add -vhost-scsi host device for use with tcm-vhost virtio-scsi: Add start/stop functionality for vhost-scsi configure| 10 +++ hw/Makefile.objs |1 + hw/msix.c|3 + hw/qdev-properties.c | 40 hw/qdev.h|3 + hw/vhost-scsi.c | 170 ++ hw/vhost-scsi.h | 50 +++ hw/vhost.c |5 +- hw/vhost.h |3 +- hw/vhost_net.c |2 +- hw/virtio-pci.c |1 + hw/virtio-scsi.c | 56 - hw/virtio-scsi.h |1 + qemu-common.h|1 + qemu-config.c| 16 + qemu-options.hx |4 + vl.c | 18 + 17 files changed, 378 insertions(+), 6 deletions(-) create mode 100644 hw/vhost-scsi.c create mode 100644 hw/vhost-scsi.h -- 1.7.2.5
[Qemu-devel] [RFC-v2 6/6] virtio-scsi: Fix incorrect VirtIOSCSI-cmd_vqs[0] definition
From: Nicholas Bellinger n...@linux-iscsi.org This patch fixes bug in the definition of VirtIOSCSI-cmd_vqs[0], where the return of virtio_add_queue() in virtio_scsi_init() ends up overwriting past the end of -cmd_vqs[0]. Since virtio_scsi currently assumes a single vqs for data, this patch simply changes -cmd_vqs[1] to handle the single VirtQueue. Cc: Paolo Bonzini pbonz...@redhat.com Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- hw/virtio-scsi.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index 5e2ff6b..2c70f89 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -150,7 +150,7 @@ typedef struct { bool events_dropped; VirtQueue *ctrl_vq; VirtQueue *event_vq; -VirtQueue *cmd_vqs[0]; +VirtQueue *cmd_vqs[1]; bool vhost_started; VHostSCSI *vhost_scsi; -- 1.7.2.5
Re: [Qemu-devel] [PATCH v6 5/7] add the QKeyCode enum and the key_defs table
On 08/03/2012 09:32 PM, Luiz Capitulino wrote: On Fri, 3 Aug 2012 10:48:40 +0800 Amos Kong ak...@redhat.com wrote: key_defs[] in monitor.c is a mapping table of keys and keycodes, this patch added a QKeyCode enum and a new key_defs table, Key's index in the enmu is same as keycode's index in new key_defs[]. Signed-off-by: Amos Kong ak...@redhat.com --- input.c | 146 ++ qapi-schema.json | 26 ++ 2 files changed, 172 insertions(+), 0 deletions(-) diff --git a/input.c b/input.c index 6968b31..680d756 100644 --- a/input.c +++ b/input.c @@ -37,6 +37,152 @@ static QTAILQ_HEAD(, QEMUPutMouseEntry) mouse_handlers = static NotifierList mouse_mode_notifiers = NOTIFIER_LIST_INITIALIZER(mouse_mode_notifiers); +static const int key_defs[] = { Weird, I expected this would brake the build, as the new table is unused. Anyway, what I suggested in my last review was to do the table move in a different patch, which includes adding the new accessors, dropping key_defs from the monitor and doing the necessary changes in the monitor functions which access key_defs directly. This way, the patch converting sendkey() to the qapi does just the conversion itself (vs. conversion plus refactorings). Ok. I would split convert patch to two patches, and do the refactorings in first patch. Thanks, Amos
Re: [Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection
On Mon, Aug 13, 2012 at 08:35:12AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This is required to get past the following assert with: commit 1523ed9e1d46b0b54540049d491475ccac7e6421 Author: Jan Kiszka jan.kis...@siemens.com Date: Thu May 17 10:32:39 2012 -0300 virtio/vhost: Add support for KVM in-kernel MSI injection Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Jan Kiszka jan.kis...@siemens.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Anthony Liguori aligu...@us.ibm.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org Could you please add a bit more explanation why this happens with virtio scsi and why this is valid? --- hw/msix.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/hw/msix.c b/hw/msix.c index 800fc32..c1e6dc3 100644 --- a/hw/msix.c +++ b/hw/msix.c @@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev) { int vector; +if (!dev-msix_vector_use_notifier !dev-msix_vector_release_notifier) +return; + assert(dev-msix_vector_use_notifier dev-msix_vector_release_notifier); -- 1.7.2.5
Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost
On Mon, Aug 13, 2012 at 08:35:14AM +, Nicholas A. Bellinger wrote: From: Stefan Hajnoczi stefa...@linux.vnet.ibm.com This patch adds a new type of host device that drives the vhost_scsi device. The syntax to add vhost-scsi is: qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123 The virtio-scsi emulated device will make use of vhost-scsi to process virtio-scsi requests inside the kernel and hand them to the in-kernel SCSI target stack using the tcm_vhost fabric driver. The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2, and the commit can be found here: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297 Changelog v1 - v2: - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as starting point for v3.6-rc code (Stefan + ALiguori + nab) - Fix upstream qemu conflict in hw/qdev-properties.c - Make GET_ABI_VERSION use int (nab + mst) - Fix vhost-scsi case lables in configure (reported by paolo) - Convert qdev_prop_vhost_scsi to use -get() + -set() following qdev_prop_netdev (reported by paolo) - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo) Changelog v0 - v1: - Add VHOST_SCSI_SET_ENDPOINT call (stefan) - Enable vhost notifiers for multiple queues (Zhi) - clear vhost-scsi endpoint on stopped (Zhi) - Add CONFIG_VHOST_SCSI for QEMU build configure (nab) - Rename vhost_vring_target - vhost_scsi_target (mst + nab) - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab) Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com Cc: Anthony Liguori aligu...@us.ibm.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- configure| 10 +++ hw/Makefile.objs |1 + hw/qdev-properties.c | 40 hw/qdev.h|3 + hw/vhost-scsi.c | 170 ++ hw/vhost-scsi.h | 50 +++ qemu-common.h|1 + qemu-config.c| 16 + qemu-options.hx |4 + vl.c | 18 + 10 files changed, 313 insertions(+), 0 deletions(-) create mode 100644 hw/vhost-scsi.c create mode 100644 hw/vhost-scsi.h diff --git a/configure b/configure index f0dbc03..1f03202 100755 --- a/configure +++ b/configure @@ -168,6 +168,7 @@ libattr= xfs= vhost_net=no +vhost_scsi=no kvm=no gprof=no debug_tcg=no @@ -513,6 +514,7 @@ Haiku) usb=linux kvm=yes vhost_net=yes + vhost_scsi=yes if [ $cpu = i386 -o $cpu = x86_64 ] ; then audio_possible_drivers=$audio_possible_drivers fmod fi @@ -818,6 +820,10 @@ for opt do ;; --enable-vhost-net) vhost_net=yes ;; + --disable-vhost-scsi) vhost_scsi=no + ;; + --enable-vhost-scsi) vhost_scsi=yes + ;; --disable-opengl) opengl=no ;; --enable-opengl) opengl=yes @@ -3116,6 +3122,7 @@ echo posix_madvise $posix_madvise echo uuid support $uuid echo libcap-ng support $cap_ng echo vhost-net support $vhost_net +echo vhost-scsi support $vhost_scsi echo Trace backend $trace_backend echo Trace output file $trace_file-pid echo spice support $spice @@ -3828,6 +3835,9 @@ case $target_arch2 in if test $vhost_net = yes ; then echo CONFIG_VHOST_NET=y $config_target_mak fi + if test $vhost_scsi = yes ; then +echo CONFIG_VHOST_SCSI=y $config_target_mak + fi fi esac case $target_arch2 in diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 3ba5dd0..6ab75ec 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o obj-$(CONFIG_SOFTMMU) += vhost_net.o obj-$(CONFIG_VHOST_NET) += vhost.o +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/ obj-$(CONFIG_NO_PCI) += pci-stub.o obj-$(CONFIG_VGA) += vga.o diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 8aca0d4..0266266 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -4,6 +4,7 @@ #include blockdev.h #include hw/block-common.h #include net/hub.h +#include vhost-scsi.h void *qdev_get_prop_ptr(DeviceState *dev, Property *prop) { @@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = { .set = set_vlan, }; +/* --- vhost-scsi --- */ + +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr) +{ + VHostSCSI *p; + + p = find_vhost_scsi(str); + if (p == NULL) + return -ENOENT; + + *ptr = p; + return 0; +} + +static const char *print_vhost_scsi_dev(void *ptr) +{ +VHostSCSI *p = ptr; + +return (p) ? vhost_scsi_get_id(p) : null; +} + +static void get_vhost_scsi_dev(Object *obj,
Re: [Qemu-devel] [PATCH 3/4] s390/kvm: Add a channel I/O based virtio transport driver.
On Wed, 08 Aug 2012 13:52:57 +0930 Rusty Russell ru...@rustcorp.com.au wrote: On Tue, 7 Aug 2012 16:52:47 +0200, Cornelia Huck cornelia.h...@de.ibm.com wrote: Add a driver for kvm guests that matches virtual ccw devices provided by the host as virtio bridge devices. Hi Cornelia, OK, this is a good opportunity to fix some limitations, just as we did for virtio_mmio (drivers/virtio/virtio_mmio.c). 1) Please don't limit yourself to 32 feature bits! If you look at how virtio_mmio does it, they use a selector to index into a theoretically-infinite array of feature bits: * 0x010 R HostFeatures Features supported by the host * 0x014 W HostFeaturesSel Set of host features to access via HostFeatures * * 0x020 W GuestFeaturesFeatures activated by the guest * 0x024 W GuestFeaturesSel Set of activated features to set via GuestFeatures It should be easy to extend the data processed by the feature ccws to a feature/index combination. Would it be practical to limit the index to an 8 bit value? 2) Please also allow the guest to set the alignment for virtio ring layout (it controls the spacing between the rings), eg: * 0x03c W QueueAlign Used Ring alignment for the current queue I think the set_vq ccw could be extended with that info. 3) Finally, make sure the guest can set the size of the queue, in case it can't allocate the size the host suggests, eg: * 0x034 R QueueNumMax Maximum size of the currently selected queue * 0x038 W QueueNum Queue size for the currently selected queue This means the host can suggest huge queues, knowing the guest won't simply fail if it does so. Makes sense, I didn't like just failing to allocate either. The actual size could probably go into the set_vq ccw as well. Note that we're also speculating a move to a new vring format, which will probably be little-endian. But you probably want a completely new ccw code for that anyway. Do you have a pointer to that discussion handy? If the host may support different vring formats, I'll probably want to add some kind of discovery mechanism for that as well (what discovery mechanism depends on whether this would be per-device or per-machine).
Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost
On Mon, Aug 13, 2012 at 08:35:14AM +, Nicholas A. Bellinger wrote: From: Stefan Hajnoczi stefa...@linux.vnet.ibm.com This patch adds a new type of host device that drives the vhost_scsi device. The syntax to add vhost-scsi is: qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123 The virtio-scsi emulated device will make use of vhost-scsi to process virtio-scsi requests inside the kernel and hand them to the in-kernel SCSI target stack using the tcm_vhost fabric driver. The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2, and the commit can be found here: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297 Changelog v1 - v2: - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as starting point for v3.6-rc code (Stefan + ALiguori + nab) - Fix upstream qemu conflict in hw/qdev-properties.c - Make GET_ABI_VERSION use int (nab + mst) - Fix vhost-scsi case lables in configure (reported by paolo) - Convert qdev_prop_vhost_scsi to use -get() + -set() following qdev_prop_netdev (reported by paolo) - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo) Changelog v0 - v1: - Add VHOST_SCSI_SET_ENDPOINT call (stefan) - Enable vhost notifiers for multiple queues (Zhi) - clear vhost-scsi endpoint on stopped (Zhi) - Add CONFIG_VHOST_SCSI for QEMU build configure (nab) - Rename vhost_vring_target - vhost_scsi_target (mst + nab) - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab) Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com Cc: Anthony Liguori aligu...@us.ibm.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org Sent mail too fast, sorry. More comments below. --- configure| 10 +++ hw/Makefile.objs |1 + hw/qdev-properties.c | 40 hw/qdev.h|3 + hw/vhost-scsi.c | 170 ++ hw/vhost-scsi.h | 50 +++ qemu-common.h|1 + qemu-config.c| 16 + qemu-options.hx |4 + vl.c | 18 + 10 files changed, 313 insertions(+), 0 deletions(-) create mode 100644 hw/vhost-scsi.c create mode 100644 hw/vhost-scsi.h diff --git a/configure b/configure index f0dbc03..1f03202 100755 --- a/configure +++ b/configure @@ -168,6 +168,7 @@ libattr= xfs= vhost_net=no +vhost_scsi=no kvm=no gprof=no debug_tcg=no @@ -513,6 +514,7 @@ Haiku) usb=linux kvm=yes vhost_net=yes + vhost_scsi=yes if [ $cpu = i386 -o $cpu = x86_64 ] ; then audio_possible_drivers=$audio_possible_drivers fmod fi @@ -818,6 +820,10 @@ for opt do ;; --enable-vhost-net) vhost_net=yes ;; + --disable-vhost-scsi) vhost_scsi=no + ;; + --enable-vhost-scsi) vhost_scsi=yes + ;; --disable-opengl) opengl=no ;; --enable-opengl) opengl=yes @@ -3116,6 +3122,7 @@ echo posix_madvise $posix_madvise echo uuid support $uuid echo libcap-ng support $cap_ng echo vhost-net support $vhost_net +echo vhost-scsi support $vhost_scsi echo Trace backend $trace_backend echo Trace output file $trace_file-pid echo spice support $spice @@ -3828,6 +3835,9 @@ case $target_arch2 in if test $vhost_net = yes ; then echo CONFIG_VHOST_NET=y $config_target_mak fi + if test $vhost_scsi = yes ; then +echo CONFIG_VHOST_SCSI=y $config_target_mak + fi fi esac case $target_arch2 in diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 3ba5dd0..6ab75ec 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o obj-$(CONFIG_SOFTMMU) += vhost_net.o obj-$(CONFIG_VHOST_NET) += vhost.o +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/ obj-$(CONFIG_NO_PCI) += pci-stub.o obj-$(CONFIG_VGA) += vga.o diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 8aca0d4..0266266 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -4,6 +4,7 @@ #include blockdev.h #include hw/block-common.h #include net/hub.h +#include vhost-scsi.h void *qdev_get_prop_ptr(DeviceState *dev, Property *prop) { @@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = { .set = set_vlan, }; +/* --- vhost-scsi --- */ + +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr) +{ + VHostSCSI *p; + + p = find_vhost_scsi(str); + if (p == NULL) + return -ENOENT; + + *ptr = p; + return 0; +} + +static const char *print_vhost_scsi_dev(void *ptr) +{ +VHostSCSI *p = ptr; + +return (p) ? vhost_scsi_get_id(p) : null; +} +
Re: [Qemu-devel] [PATCH] spice: abort on invalid streaming cmdline params
Ack On 08/13/2012 11:32 AM, Christophe Fergeau wrote: When parsing its command line parameters, spice aborts when it finds unexpected values, except for the 'streaming-video' option. This happens because the parsing of the parameters for this option is done using the 'name2enum' helper, which does not error out on unknown values. Using the 'parse_name' helper makes sure we error out in this case. Looking at git history, the use of 'name2enum' instead of 'parse_name' seems to have been an oversight, so let's change to that now. Fixes rhbz#831708 --- ui/spice-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ui/spice-core.c b/ui/spice-core.c index 4fc48f8..bb4f585 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -344,7 +344,8 @@ static const char *stream_video_names[] = { [ SPICE_STREAM_VIDEO_FILTER ] = filter, }; #define parse_stream_video(_name) \ -name2enum(_name, stream_video_names, ARRAY_SIZE(stream_video_names)) +parse_name(_name, stream video control, \ + stream_video_names, ARRAY_SIZE(stream_video_names)) static const char *compression_names[] = { [ SPICE_IMAGE_COMPRESS_OFF ] = off,
Re: [Qemu-devel] [RFC-v2 6/6] virtio-scsi: Fix incorrect VirtIOSCSI-cmd_vqs[0] definition
On Mon, Aug 13, 2012 at 08:35:17AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch fixes bug in the definition of VirtIOSCSI-cmd_vqs[0], where the return of virtio_add_queue() in virtio_scsi_init() ends up overwriting past the end of -cmd_vqs[0]. Since virtio_scsi currently assumes a single vqs for data, this patch simply changes -cmd_vqs[1] to handle the single VirtQueue. Cc: Paolo Bonzini pbonz...@redhat.com Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org This is a bugfix we need even without vhost, right? --- hw/virtio-scsi.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index 5e2ff6b..2c70f89 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -150,7 +150,7 @@ typedef struct { bool events_dropped; VirtQueue *ctrl_vq; VirtQueue *event_vq; -VirtQueue *cmd_vqs[0]; +VirtQueue *cmd_vqs[1]; bool vhost_started; VHostSCSI *vhost_scsi; -- 1.7.2.5
Re: [Qemu-devel] [RFC-v2 0/6] vhost-scsi: Add support for host virtualized target
On Mon, Aug 13, 2012 at 08:35:11AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi Paolo, Stefan, QEMU folks, The following is the second RFC series for vhost-scsi patches against mainline QEMU v1.1.0. The series is available from the following working branch: git://git.kernel.org/pub/scm/virt/kvm/nab/qemu-kvm.git vhost-scsi-merge Apologies for the delayed follow-up on this series. The changes detailed below addresses Paolo's original comments on vhost-scsi code from the last weeks. Looks good overall. I sent some comments on list. Thanks! As of this evening the tcm_vhost driver has now been merged into the mainline kernel for 3.6-rc2 here: tcm_vhost: Initial merge for vhost level target fabric driver http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297877 Also, after following up on the qemu-kvm IRQ injection changes (from Jan) that caused a performance regerssion with QEMU 1.1.0 code originally reported here: vhost-scsi port to v1.1.0 + MSI-X performance regression http://comments.gmane.org/gmane.linux.scsi.target.devel/2277 It turns out that setting explict virtio-queue IRQ affinity within guest appears to bring small block random IOPs performance back up to the pre IRQ injection conversion levels. I'm not sure why this ended up making so much of a difference post IRQ injection conversion, but setting virtio-queue affinity is now getting us back to pre IQN injection conversion levels. Changes from v1 - v2: - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as starting point for v3.6-rc code (Stefan + ALiguori + nab) - Fix upstream qemu conflict in hw/qdev-properties.c - Make GET_ABI_VERSION use int (nab + mst) - Drop unnecessary event-notifier changes (nab) - Fix vhost-scsi case lables in configure (reported by paolo) - Convert qdev_prop_vhost_scsi to use -get() + -set() following qdev_prop_netdev (reported by paolo) - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo) - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab) - Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab) - Drop usage of to_virtio_scsi() in virtio_scsi_set_status() (reported by paolo) - Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo) - Use s-conf-vhost_scsi instead of proxyconf-vhost_scsi in virtio_scsi_init() (reported by paolo) - Only register QEMU SCSI bus is vhost-scsi is not active (reported by paolo) - Fix incorrect VirtIOSCSI-cmd_vqs[0] definition (nab) Please have another look, and let me know if anything else needs to be addressed. Thanks! --nab Nicholas Bellinger (3): msix: Work-around for vhost-scsi with KVM in-kernel MSI injection virtio-scsi: Set max_target=0 during vhost-scsi operation virtio-scsi: Fix incorrect VirtIOSCSI-cmd_vqs[0] definition Stefan Hajnoczi (3): vhost: Pass device path to vhost_dev_init() vhost-scsi: add -vhost-scsi host device for use with tcm-vhost virtio-scsi: Add start/stop functionality for vhost-scsi configure| 10 +++ hw/Makefile.objs |1 + hw/msix.c|3 + hw/qdev-properties.c | 40 hw/qdev.h|3 + hw/vhost-scsi.c | 170 ++ hw/vhost-scsi.h | 50 +++ hw/vhost.c |5 +- hw/vhost.h |3 +- hw/vhost_net.c |2 +- hw/virtio-pci.c |1 + hw/virtio-scsi.c | 56 - hw/virtio-scsi.h |1 + qemu-common.h|1 + qemu-config.c| 16 + qemu-options.hx |4 + vl.c | 18 + 17 files changed, 378 insertions(+), 6 deletions(-) create mode 100644 hw/vhost-scsi.c create mode 100644 hw/vhost-scsi.h -- 1.7.2.5
[Qemu-devel] [RFC-v2 5/6] virtio-scsi: Set max_target=0 during vhost-scsi operation
From: Nicholas Bellinger n...@linux-iscsi.org This QEMU patch sets VirtIOSCSIConfig-max_target=0 for vhost-scsi operation to restrict virtio-scsi LLD guest scanning to max_id=0 (a single target ID instance) when connected to individual tcm_vhost endpoints. This ensures that virtio-scsi LLD only attempts to scan target IDs up to VIRTIO_SCSI_MAX_TARGET when connected via virtio-scsi-raw. Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com Cc: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- hw/virtio-scsi.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index 8130956..5e2ff6b 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -545,7 +545,11 @@ static void virtio_scsi_get_config(VirtIODevice *vdev, stl_raw(scsiconf-sense_size, s-sense_size); stl_raw(scsiconf-cdb_size, s-cdb_size); stl_raw(scsiconf-max_channel, VIRTIO_SCSI_MAX_CHANNEL); -stl_raw(scsiconf-max_target, VIRTIO_SCSI_MAX_TARGET); +if (s-vhost_scsi) { +stl_raw(scsiconf-max_target, 0); +} else { +stl_raw(scsiconf-max_target, VIRTIO_SCSI_MAX_TARGET); +} stl_raw(scsiconf-max_lun, VIRTIO_SCSI_MAX_LUN); } -- 1.7.2.5
[Qemu-devel] [RFC-v2 2/6] vhost: Pass device path to vhost_dev_init()
From: Stefan Hajnoczi stefa...@linux.vnet.ibm.com The path to /dev/vhost-net is currently hardcoded in vhost_dev_init(). This needs to be changed so that /dev/vhost-scsi can be used. Pass in the device path instead of hardcoding it. Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- hw/vhost.c |5 +++-- hw/vhost.h |3 ++- hw/vhost_net.c |2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/vhost.c b/hw/vhost.c index 0fd8da8..d0ce5aa 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -747,14 +747,15 @@ static void vhost_eventfd_del(MemoryListener *listener, { } -int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force) +int vhost_dev_init(struct vhost_dev *hdev, int devfd, const char *devpath, + bool force) { uint64_t features; int r; if (devfd = 0) { hdev-control = devfd; } else { -hdev-control = open(/dev/vhost-net, O_RDWR); +hdev-control = open(devpath, O_RDWR); if (hdev-control 0) { return -errno; } diff --git a/hw/vhost.h b/hw/vhost.h index 80e64df..0c47229 100644 --- a/hw/vhost.h +++ b/hw/vhost.h @@ -44,7 +44,8 @@ struct vhost_dev { bool force; }; -int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force); +int vhost_dev_init(struct vhost_dev *hdev, int devfd, const char *devpath, + bool force); void vhost_dev_cleanup(struct vhost_dev *hdev); bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev); int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev); diff --git a/hw/vhost_net.c b/hw/vhost_net.c index ecaa22d..df2c4a3 100644 --- a/hw/vhost_net.c +++ b/hw/vhost_net.c @@ -109,7 +109,7 @@ struct vhost_net *vhost_net_init(NetClientState *backend, int devfd, (1 VHOST_NET_F_VIRTIO_NET_HDR); net-backend = r; -r = vhost_dev_init(net-dev, devfd, force); +r = vhost_dev_init(net-dev, devfd, /dev/vhost-net, force); if (r 0) { goto fail; } -- 1.7.2.5
[Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost
From: Stefan Hajnoczi stefa...@linux.vnet.ibm.com This patch adds a new type of host device that drives the vhost_scsi device. The syntax to add vhost-scsi is: qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123 The virtio-scsi emulated device will make use of vhost-scsi to process virtio-scsi requests inside the kernel and hand them to the in-kernel SCSI target stack using the tcm_vhost fabric driver. The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2, and the commit can be found here: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297 Changelog v1 - v2: - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as starting point for v3.6-rc code (Stefan + ALiguori + nab) - Fix upstream qemu conflict in hw/qdev-properties.c - Make GET_ABI_VERSION use int (nab + mst) - Fix vhost-scsi case lables in configure (reported by paolo) - Convert qdev_prop_vhost_scsi to use -get() + -set() following qdev_prop_netdev (reported by paolo) - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo) Changelog v0 - v1: - Add VHOST_SCSI_SET_ENDPOINT call (stefan) - Enable vhost notifiers for multiple queues (Zhi) - clear vhost-scsi endpoint on stopped (Zhi) - Add CONFIG_VHOST_SCSI for QEMU build configure (nab) - Rename vhost_vring_target - vhost_scsi_target (mst + nab) - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab) Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com Cc: Anthony Liguori aligu...@us.ibm.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- configure| 10 +++ hw/Makefile.objs |1 + hw/qdev-properties.c | 40 hw/qdev.h|3 + hw/vhost-scsi.c | 170 ++ hw/vhost-scsi.h | 50 +++ qemu-common.h|1 + qemu-config.c| 16 + qemu-options.hx |4 + vl.c | 18 + 10 files changed, 313 insertions(+), 0 deletions(-) create mode 100644 hw/vhost-scsi.c create mode 100644 hw/vhost-scsi.h diff --git a/configure b/configure index f0dbc03..1f03202 100755 --- a/configure +++ b/configure @@ -168,6 +168,7 @@ libattr= xfs= vhost_net=no +vhost_scsi=no kvm=no gprof=no debug_tcg=no @@ -513,6 +514,7 @@ Haiku) usb=linux kvm=yes vhost_net=yes + vhost_scsi=yes if [ $cpu = i386 -o $cpu = x86_64 ] ; then audio_possible_drivers=$audio_possible_drivers fmod fi @@ -818,6 +820,10 @@ for opt do ;; --enable-vhost-net) vhost_net=yes ;; + --disable-vhost-scsi) vhost_scsi=no + ;; + --enable-vhost-scsi) vhost_scsi=yes + ;; --disable-opengl) opengl=no ;; --enable-opengl) opengl=yes @@ -3116,6 +3122,7 @@ echo posix_madvise $posix_madvise echo uuid support $uuid echo libcap-ng support $cap_ng echo vhost-net support $vhost_net +echo vhost-scsi support $vhost_scsi echo Trace backend $trace_backend echo Trace output file $trace_file-pid echo spice support $spice @@ -3828,6 +3835,9 @@ case $target_arch2 in if test $vhost_net = yes ; then echo CONFIG_VHOST_NET=y $config_target_mak fi + if test $vhost_scsi = yes ; then +echo CONFIG_VHOST_SCSI=y $config_target_mak + fi fi esac case $target_arch2 in diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 3ba5dd0..6ab75ec 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o obj-$(CONFIG_SOFTMMU) += vhost_net.o obj-$(CONFIG_VHOST_NET) += vhost.o +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/ obj-$(CONFIG_NO_PCI) += pci-stub.o obj-$(CONFIG_VGA) += vga.o diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 8aca0d4..0266266 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -4,6 +4,7 @@ #include blockdev.h #include hw/block-common.h #include net/hub.h +#include vhost-scsi.h void *qdev_get_prop_ptr(DeviceState *dev, Property *prop) { @@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = { .set = set_vlan, }; +/* --- vhost-scsi --- */ + +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr) +{ + VHostSCSI *p; + + p = find_vhost_scsi(str); + if (p == NULL) + return -ENOENT; + + *ptr = p; + return 0; +} + +static const char *print_vhost_scsi_dev(void *ptr) +{ +VHostSCSI *p = ptr; + +return (p) ? vhost_scsi_get_id(p) : null; +} + +static void get_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +get_pointer(obj, v, opaque, print_vhost_scsi_dev, name, errp); +} + +static void set_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque, +
[Qemu-devel] [RFC-v2 4/6] virtio-scsi: Add start/stop functionality for vhost-scsi
From: Stefan Hajnoczi stefa...@linux.vnet.ibm.com This patch starts and stops vhost as the virtio device transitions through its status phases. Vhost can only be started once the guest reports its driver has successfully initialized, which means the virtqueues have been set up by the guest. v2: - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab) - Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab) - Drop usage of to_virtio_scsi() in virtio_scsi_set_status() (reported by paolo) - Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo) - Use s-conf-vhost_scsi instead of proxyconf-vhost_scsi in virtio_scsi_init() (reported by paolo) - Only register QEMU SCSI bus is vhost-scsi is not active (reported by paolo) Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- hw/virtio-pci.c |1 + hw/virtio-scsi.c | 48 hw/virtio-scsi.h |1 + 3 files changed, 50 insertions(+), 0 deletions(-) diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 125eded..b29fc3b 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -1036,6 +1036,7 @@ static void virtio_scsi_exit_pci(PCIDevice *pci_dev) } static Property virtio_scsi_properties[] = { +DEFINE_PROP_VHOST_SCSI(vhost-scsi, VirtIOPCIProxy, scsi.vhost_scsi), DEFINE_PROP_BIT(ioeventfd, VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), DEFINE_PROP_UINT32(vectors, VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED), DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOPCIProxy, host_features, scsi), diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index 5f737ac..8130956 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -13,9 +13,13 @@ * */ +#include qemu-common.h +#include qemu-error.h +#include vhost-scsi.h #include virtio-scsi.h #include hw/scsi.h #include hw/scsi-defs.h +#include vhost.h #define VIRTIO_SCSI_VQ_SIZE 128 #define VIRTIO_SCSI_CDB_SIZE32 @@ -147,6 +151,9 @@ typedef struct { VirtQueue *ctrl_vq; VirtQueue *event_vq; VirtQueue *cmd_vqs[0]; + +bool vhost_started; +VHostSCSI *vhost_scsi; } VirtIOSCSI; typedef struct VirtIOSCSIReq { @@ -699,6 +706,38 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = { .load_request = virtio_scsi_load_request, }; +static bool virtio_scsi_started(VirtIOSCSI *s, uint8_t val) +{ +return (val VIRTIO_CONFIG_S_DRIVER_OK) s-vdev.vm_running; +} + +static void virtio_scsi_set_status(VirtIODevice *vdev, uint8_t val) +{ +VirtIOSCSI *s = (VirtIOSCSI *)vdev; +bool start = virtio_scsi_started(s, val); + +if (s-vhost_started == start) { +return; +} + +if (start) { +int ret; + +ret = vhost_scsi_start(s-vhost_scsi, vdev); +if (ret 0) { +error_report(virtio-scsi: unable to start vhost: %s\n, + strerror(-ret)); + +/* There is no userspace virtio-scsi fallback so exit */ +exit(1); +} +} else { +vhost_scsi_stop(s-vhost_scsi, vdev); +} + +s-vhost_started = start; +} + VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf) { VirtIOSCSI *s; @@ -712,12 +751,17 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf) s-qdev = dev; s-conf = proxyconf; +s-vhost_started = false; +s-vhost_scsi = s-conf-vhost_scsi; /* TODO set up vdev function pointers */ s-vdev.get_config = virtio_scsi_get_config; s-vdev.set_config = virtio_scsi_set_config; s-vdev.get_features = virtio_scsi_get_features; s-vdev.reset = virtio_scsi_reset; +if (s-vhost_scsi) { +s-vdev.set_status = virtio_scsi_set_status; +} s-ctrl_vq = virtio_add_queue(s-vdev, VIRTIO_SCSI_VQ_SIZE, virtio_scsi_handle_ctrl); @@ -743,5 +787,9 @@ void virtio_scsi_exit(VirtIODevice *vdev) { VirtIOSCSI *s = (VirtIOSCSI *)vdev; unregister_savevm(s-qdev, virtio-scsi, s); + +/* This will stop vhost backend if appropriate. */ +virtio_scsi_set_status(vdev, 0); + virtio_cleanup(vdev); } diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h index 4bc889d..74e9422 100644 --- a/hw/virtio-scsi.h +++ b/hw/virtio-scsi.h @@ -22,6 +22,7 @@ #define VIRTIO_ID_SCSI 8 struct VirtIOSCSIConf { +VHostSCSI *vhost_scsi; uint32_t num_queues; uint32_t max_sectors; uint32_t cmd_per_lun; -- 1.7.2.5
Re: [Qemu-devel] [PATCH] update-linux-headers.sh: Don't hard code list of architectures
Ping^2? On 3 August 2012 13:55, Peter Maydell peter.mayd...@linaro.org wrote: Ping? patchwork url: http://patchwork.ozlabs.org/patch/171628/ -- PMM On 18 July 2012 11:11, Peter Maydell peter.mayd...@linaro.org wrote: Rather than hardcoding the list of architectures in the kernel header update script, just import headers for every architecture which supports KVM (with a blacklist exception for ia64 which has KVM headers but is dead). This reduces the number of QEMU files which need to be updated to add support for a new KVM architecture. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- Changes v1-v2: * added a blacklist for ia64, to avoid noise and importing a pointless set of headers that will get dropped later scripts/update-linux-headers.sh | 16 +++- 1 files changed, 15 insertions(+), 1 deletions(-) diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh index 9d2a4bc..57ce69f 100755 --- a/scripts/update-linux-headers.sh +++ b/scripts/update-linux-headers.sh @@ -28,7 +28,21 @@ if [ -z $output ]; then output=$PWD fi -for arch in x86 powerpc s390; do +# This will pick up non-directories too (eg Kconfig) but we will +# ignore them in the next loop. +ARCHLIST=$(cd $linux/arch echo *) + +for arch in $ARCHLIST; do +# Discard anything which isn't a KVM-supporting architecture +if ! [ -e $linux/arch/$arch/include/asm/kvm.h ]; then +continue +fi + +# Blacklist architectures which have KVM headers but are actually dead +if [ $arch = ia64 ]; then +continue +fi + make -C $linux INSTALL_HDR_PATH=$tmpdir SRCARCH=$arch headers_install rm -rf $output/linux-headers/asm-$arch -- 1.7.5.4
Re: [Qemu-devel] [PATCH] update-linux-headers.sh: Pull in asm-generic/kvm_para.h
Ping^2 ? On 8 August 2012 13:34, Peter Maydell peter.mayd...@linaro.org wrote: Ping? patchwork url: http://patchwork.ozlabs.org/patch/173202/ -- PMM On 25 July 2012 16:29, Peter Maydell peter.mayd...@linaro.org wrote: Add asm-generic/kvm_para.h to the set of non-architecture specific KVM kernel headers we copy into QEMU. This header may be included by an architecture's kvm_para.h header. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- scripts/update-linux-headers.sh |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh index 9d2a4bc..a639c5b 100755 --- a/scripts/update-linux-headers.sh +++ b/scripts/update-linux-headers.sh @@ -46,6 +46,11 @@ mkdir -p $output/linux-headers/linux for header in kvm.h kvm_para.h vhost.h virtio_config.h virtio_ring.h; do cp $tmpdir/include/linux/$header $output/linux-headers/linux done +rm -rf $output/linux-headers/asm-generic +mkdir -p $output/linux-headers/asm-generic +for header in kvm_para.h; do +cp $tmpdir/include/asm-generic/$header $output/linux-headers/asm-generic +done if [ -L $linux/source ]; then cp $linux/source/COPYING $output/linux-headers else -- 1.7.5.4
Re: [Qemu-devel] [PULL 0/2] QOMify AXI stream for Xilinx AXI ethernet/DMA
On Fri, Aug 10, 2012 at 01:16:09PM +1000, Peter A. G. Crosthwaite wrote: The following changes since commit 3d1d9652978ac5a32a0beb4bdf6065ca39440d89: Bruce Rogers (1): handle device help before accelerator set up are available in the git repository at: git://developer.petalogix.com/public/qemu.git for-upstream/axi-stream.next Anthony Liguori (1): qom: Reimplement Interfaces Peter A. G. Crosthwaite (1): xilinx_axi*: Re-implemented interconnect Applied, thanks Peter. Cheers, Edgar hw/Makefile.objs |1 + hw/petalogix_ml605_mmu.c | 24 +++-- hw/stream.c | 23 + hw/stream.h | 31 +++ hw/xilinx.h | 22 ++--- hw/xilinx_axidma.c | 74 +--- hw/xilinx_axidma.h | 39 hw/xilinx_axienet.c | 32 --- include/qemu/object.h| 46 ++ qom/object.c | 220 ++ 10 files changed, 255 insertions(+), 257 deletions(-) create mode 100644 hw/stream.c create mode 100644 hw/stream.h delete mode 100644 hw/xilinx_axidma.h
Re: [Qemu-devel] [PATCH v6 0/2] GlusterFS support in QEMU - v6
On Thu, Aug 09, 2012 at 06:30:10PM +0530, Bharata B Rao wrote: Hi, This is the v6 of the patchset to support GlusterFS backend from QEMU. The changes include defining a default value for glusterfs feature in the configure script and a few minor cleanups suggested by Stefan. As I mentioned during a previous release, I have tested gluster backend with different gluster configurations like local, remote, distributed and replicated volumes. I believe that the patchset has undergone enough scrutiny now to be included in 1.2 release. Hi Kevin, Anthony, I understand its a bit late in the day for these patches to be included in 1.2. However let me put forth some points and see if I can convince you to take it for 1.2 :) - The first RFC patch for this was posted in June. - The 2nd version which used libgfapi interfaces was done July, ahead of the soft freeze. Apart from cleanups and changes in the way the gluster drive is specified, the core block driver code hasn't changed much. - The patches have undergone good amount of testing with various gluster scenarios (local and remote volumes, distributed and replicated volume setup etc). - All the versions have undergone review and I have addressed all the outstanding issues. - I have run FIO benchmark with this block backend and have shown how beneficial it is to use this block backend driver for running VMs from gluster volumes. (http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02718.html) (http://lists.gnu.org/archive/html/gluster-devel/2012-08/msg00063.html) Hence request you to consider these patches for 1.2 release! Regards, Bharata.
[Qemu-devel] KVM call agenda for Tuesday, August 14
Hi Please send in any agenda items you are interested in covering. Thanks, Juan.
Re: [Qemu-devel] github mirror still stale
Am 12.08.2012 00:03, schrieb Anthony Liguori: Peter Maydell peter.mayd...@linaro.org writes: On 18 July 2012 10:28, Peter Maydell peter.mayd...@linaro.org wrote: On 12 March 2012 20:12, Stefan Weil s...@weilnetz.de wrote: We also need more resources for technical maintenance of the QEMU infrastructure. For example, the official mirror of the QEMU git repository (https://github.com/qemu/QEMU) is several months behind, http://git.savannah.gnu.org/cgit/qemu.git is even older. The github mirror is still wildly out of date -- can we get the link to it removed from http://wiki.qemu.org/Download please? (I'd do it myself but the page is 'locked'.) Ping^2. It would be good to stop advertising stale git mirrors before we make the 1.2 release... I messed up setting it up and had to delete the whole thing and start over. We now have a proper organization on github and which let me setup a mirror from qemu.org that pushes every 30 minutes to github. It should be active now. I can also add other people to the github organization. Regards, Anthony Liguori http://git.savannah.gnu.org/cgit/qemu.git is also still online and very old. What about using it as a second mirror? If you want, I offer my help. Regards, Stefan Weil
[Qemu-devel] [PATCH] spice: abort on invalid streaming cmdline params
When parsing its command line parameters, spice aborts when it finds unexpected values, except for the 'streaming-video' option. This happens because the parsing of the parameters for this option is done using the 'name2enum' helper, which does not error out on unknown values. Using the 'parse_name' helper makes sure we error out in this case. Looking at git history, the use of 'name2enum' instead of 'parse_name' seems to have been an oversight, so let's change to that now. Fixes rhbz#831708 --- ui/spice-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ui/spice-core.c b/ui/spice-core.c index 4fc48f8..bb4f585 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -344,7 +344,8 @@ static const char *stream_video_names[] = { [ SPICE_STREAM_VIDEO_FILTER ] = filter, }; #define parse_stream_video(_name) \ -name2enum(_name, stream_video_names, ARRAY_SIZE(stream_video_names)) +parse_name(_name, stream video control, \ + stream_video_names, ARRAY_SIZE(stream_video_names)) static const char *compression_names[] = { [ SPICE_IMAGE_COMPRESS_OFF ] = off, -- 1.7.11.2
[Qemu-devel] [PATCH 2/2] hd-geometry.c: Integrate HDIO_GETGEO in guessing
From: Einar Lueck elelu...@linux.vnet.ibm.com This patch extends the function guess_disk_lchs. If no geo could be derived from reading disk content, HDIO_GETGEO ioctl is issued. If this is not successful (e.g. image files) geometry is derived from the size of the disk (as before). To achieve this, the MSDOS partition table reading logic and the translation computation logic have been moved into a separate function guess_disk_msdosgeo. The HDIO_GETGEO logic is captured in a new function guess_disk_ioctlgeo. guess_disk_lchs then encapsulates both (the overall guessing logic). The new HDIO_GETGEO logic is required for two use cases: a) Support for geometries of Direct Attached Storage Disks (DASD) on s390x configured as backing of virtio block devices. b) Support for FCP attached SCSI disks that do not yet have a partition table. Without this patch, fdisk -l on the host would return different results then fdisk -l in the guest. Signed-off-by: Einar Lueck elelu...@linux.vnet.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com --- hw/hd-geometry.c | 118 ++- 1 file changed, 81 insertions(+), 37 deletions(-) diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index 1cdb9fb..fc29075 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -33,6 +33,10 @@ #include block.h #include hw/block-common.h #include trace.h +#ifdef __linux__ +#include linux/fs.h +#include linux/hdreg.h +#endif struct partition { uint8_t boot_ind; /* 0x80 - active */ @@ -47,13 +51,59 @@ struct partition { uint32_t nr_sects; /* nr of sectors in partition */ } QEMU_PACKED; +static void guess_chs_for_size(BlockDriverState *bs, +uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs) +{ +uint64_t nb_sectors; +int cylinders; + +bdrv_get_geometry(bs, nb_sectors); + +cylinders = nb_sectors / (16 * 63); +if (cylinders 16383) { +cylinders = 16383; +} else if (cylinders 2) { +cylinders = 2; +} +*pcyls = cylinders; +*pheads = 16; +*psecs = 63; +} + +/* try to get geometry from disk via HDIO_GETGEO ioctl + Return 0 if OK, -1 if ioctl does not work (e.g. image file) */ +static inline int guess_disk_ioctlgeo(BlockDriverState *bs, + uint32_t *pcylinders, uint32_t *pheads, + uint32_t *psectors, int *ptranslation) +{ +#ifdef __linux__ +struct hd_geometry geo; + +if (bdrv_ioctl(bs, HDIO_GETGEO, geo)) { +return -1; +} + +*pheads = geo.heads; +*psectors = geo.sectors; +*pcylinders = geo.cylinders; +*ptranslation = BIOS_ATA_TRANSLATION_NONE; +trace_hd_geometry_lchs_guess(bs, *pcylinders, *pheads, *psectors); +return 0; +#else +return -1; +#endif +} + + /* try to guess the disk logical geometry from the MSDOS partition table. Return 0 if OK, -1 if could not guess */ -static int guess_disk_lchs(BlockDriverState *bs, - int *pcylinders, int *pheads, int *psectors) +static int guess_disk_msdosgeo(BlockDriverState *bs, + uint32_t *pcylinders, uint32_t *pheads, + uint32_t *psectors, int *ptranslation) { uint8_t buf[BDRV_SECTOR_SIZE]; -int i, heads, sectors, cylinders; +int i, translation; +uint32_t heads, sectors, cylinders; struct partition *p; uint32_t nr_sects; uint64_t nb_sectors; @@ -87,9 +137,26 @@ static int guess_disk_lchs(BlockDriverState *bs, if (cylinders 1 || cylinders 16383) { continue; } + +if (heads 16) { +/* LCHS guess with heads 16 means that a BIOS LBA + translation was active, so a standard physical disk + geometry is OK */ +guess_chs_for_size(bs, cylinders, heads, sectors); +translation = cylinders * heads = 131072 +? BIOS_ATA_TRANSLATION_LARGE +: BIOS_ATA_TRANSLATION_LBA; +} else { +/* LCHS guess with heads = 16: use as physical geometry */ +/* disable any translation to be in sync with + the logical geometry */ +translation = BIOS_ATA_TRANSLATION_NONE; +} *pheads = heads; *psectors = sectors; *pcylinders = cylinders; +*ptranslation = translation; + trace_hd_geometry_lchs_guess(bs, cylinders, heads, sectors); return 0; } @@ -97,51 +164,28 @@ static int guess_disk_lchs(BlockDriverState *bs, return -1; } -static void guess_chs_for_size(BlockDriverState *bs, -uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs) -{ -uint64_t nb_sectors; -int cylinders; - -bdrv_get_geometry(bs, nb_sectors); - -cylinders = nb_sectors / (16 * 63); -if
[Qemu-devel] [PATCH 0/2] s390: block size auto-sensing and geometry guessing changes in common code
From: Jens Freimann jf...@linux.vnet.ibm.com Here are two patches for block device characteristics detection. The first one implements block size auto-sensing for virtio-block and the second patch changes geometry guessing common code. More details in the patch descriptions. Einar Lueck (2): virtio-block: support auto-sensing of block sizes hd-geometry.c: Integrate HDIO_GETGEO in guessing hw/block-common.c| 23 ++ hw/block-common.h| 12 -- hw/hd-geometry.c | 118 +++ hw/qdev-properties.c | 4 +- hw/s390-virtio-bus.c | 2 +- hw/virtio-blk.c | 1 + 6 files changed, 118 insertions(+), 42 deletions(-) -- 1.7.11.4
[Qemu-devel] [PATCH 1/2] virtio-block: support auto-sensing of block sizes
From: Einar Lueck elelu...@linux.vnet.ibm.com Virtio-blk does not impose fixed block sizes for access to backing devices. This patch introduces support for auto lookup of the block sizes of the backing block device. This automatic lookup needs to be enabled explicitly. Users may do this by specifying (physical|logical)_block_size=0. Machine types may do this for their defaults, too. To achieve this, a new function blkconf_blocksizes is implemented. If physical or logical block size are zero a corresponding ioctl tries to find an appropriate value. If this does not work, 512 is used. blkconf_blocksizes is therefore only called w/in the virtio-blk context. For s390-virtio, this patch configures auto lookup as default. Signed-off-by: Einar Lueck elelu...@linux.vnet.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com --- hw/block-common.c| 23 +++ hw/block-common.h| 12 +--- hw/qdev-properties.c | 4 +++- hw/s390-virtio-bus.c | 2 +- hw/virtio-blk.c | 1 + 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/hw/block-common.c b/hw/block-common.c index f0196d7..444edd2 100644 --- a/hw/block-common.c +++ b/hw/block-common.c @@ -10,6 +10,9 @@ #include blockdev.h #include hw/block-common.h #include qemu-error.h +#ifdef __linux__ +#include linux/fs.h +#endif void blkconf_serial(BlockConf *conf, char **serial) { @@ -24,6 +27,26 @@ void blkconf_serial(BlockConf *conf, char **serial) } } +void blkconf_blocksizes(BlockConf *conf) +{ +int block_size; + +if (!conf-physical_block_size) { +if (bdrv_ioctl(conf-bs, BLKPBSZGET, block_size) == 0) { +conf-physical_block_size = (uint16_t) block_size; +} else { +conf-physical_block_size = BLOCK_PROPERTY_STD_BLKSIZE; +} +} +if (!conf-logical_block_size) { +if (bdrv_ioctl(conf-bs, BLKSSZGET, block_size) == 0) { +conf-logical_block_size = (uint16_t) block_size; +} else { +conf-logical_block_size = BLOCK_PROPERTY_STD_BLKSIZE; +} +} +} + int blkconf_geometry(BlockConf *conf, int *ptrans, unsigned cyls_max, unsigned heads_max, unsigned secs_max) { diff --git a/hw/block-common.h b/hw/block-common.h index bb808f7..d593128 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -40,18 +40,23 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) return exp; } -#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ +#define BLOCK_PROPERTY_STD_BLKSIZE 512 +#define DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, _blksize) \ DEFINE_PROP_DRIVE(drive, _state, _conf.bs), \ DEFINE_PROP_BLOCKSIZE(logical_block_size, _state, \ - _conf.logical_block_size, 512), \ + _conf.logical_block_size, _blksize), \ DEFINE_PROP_BLOCKSIZE(physical_block_size, _state,\ - _conf.physical_block_size, 512), \ + _conf.physical_block_size, _blksize), \ DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \ DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0),\ DEFINE_PROP_INT32(bootindex, _state, _conf.bootindex, -1),\ DEFINE_PROP_UINT32(discard_granularity, _state, \ _conf.discard_granularity, 0) +#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ +DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, \ + BLOCK_PROPERTY_STD_BLKSIZE) + #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ DEFINE_PROP_UINT32(cyls, _state, _conf.cyls, 0), \ DEFINE_PROP_UINT32(heads, _state, _conf.heads, 0), \ @@ -60,6 +65,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) /* Configuration helpers */ void blkconf_serial(BlockConf *conf, char **serial); +void blkconf_blocksizes(BlockConf *conf); int blkconf_geometry(BlockConf *conf, int *trans, unsigned cyls_max, unsigned heads_max, unsigned secs_max); diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 8aca0d4..e99dee4 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -904,7 +904,9 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, error_propagate(errp, local_err); return; } -if (value min || value max) { + +/* value == 0 indicates that block size should be sensed later on */ +if ((value min || value max) value 0) { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, dev-id?:, name, (int64_t)value, min, max); return; diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index a245684..562964e 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -401,7 +401,7 @@
[Qemu-devel] [PATCH 2/2] hd-geometry.c: Integrate HDIO_GETGEO in guessing
From: Einar Lueck elelu...@linux.vnet.ibm.com This patch extends the function guess_disk_lchs. If no geo could be derived from reading disk content, HDIO_GETGEO ioctl is issued. If this is not successful (e.g. image files) geometry is derived from the size of the disk (as before). To achieve this, the MSDOS partition table reading logic and the translation computation logic have been moved into a separate function guess_disk_msdosgeo. The HDIO_GETGEO logic is captured in a new function guess_disk_ioctlgeo. guess_disk_lchs then encapsulates both (the overall guessing logic). The new HDIO_GETGEO logic is required for two use cases: a) Support for geometries of Direct Attached Storage Disks (DASD) on s390x configured as backing of virtio block devices. b) Support for FCP attached SCSI disks that do not yet have a partition table. Without this patch, fdisk -l on the host would return different results then fdisk -l in the guest. Signed-off-by: Einar Lueck elelu...@linux.vnet.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com --- hw/hd-geometry.c | 118 ++- 1 file changed, 81 insertions(+), 37 deletions(-) diff --git a/hw/hd-geometry.c b/hw/hd-geometry.c index 1cdb9fb..fc29075 100644 --- a/hw/hd-geometry.c +++ b/hw/hd-geometry.c @@ -33,6 +33,10 @@ #include block.h #include hw/block-common.h #include trace.h +#ifdef __linux__ +#include linux/fs.h +#include linux/hdreg.h +#endif struct partition { uint8_t boot_ind; /* 0x80 - active */ @@ -47,13 +51,59 @@ struct partition { uint32_t nr_sects; /* nr of sectors in partition */ } QEMU_PACKED; +static void guess_chs_for_size(BlockDriverState *bs, +uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs) +{ +uint64_t nb_sectors; +int cylinders; + +bdrv_get_geometry(bs, nb_sectors); + +cylinders = nb_sectors / (16 * 63); +if (cylinders 16383) { +cylinders = 16383; +} else if (cylinders 2) { +cylinders = 2; +} +*pcyls = cylinders; +*pheads = 16; +*psecs = 63; +} + +/* try to get geometry from disk via HDIO_GETGEO ioctl + Return 0 if OK, -1 if ioctl does not work (e.g. image file) */ +static inline int guess_disk_ioctlgeo(BlockDriverState *bs, + uint32_t *pcylinders, uint32_t *pheads, + uint32_t *psectors, int *ptranslation) +{ +#ifdef __linux__ +struct hd_geometry geo; + +if (bdrv_ioctl(bs, HDIO_GETGEO, geo)) { +return -1; +} + +*pheads = geo.heads; +*psectors = geo.sectors; +*pcylinders = geo.cylinders; +*ptranslation = BIOS_ATA_TRANSLATION_NONE; +trace_hd_geometry_lchs_guess(bs, *pcylinders, *pheads, *psectors); +return 0; +#else +return -1; +#endif +} + + /* try to guess the disk logical geometry from the MSDOS partition table. Return 0 if OK, -1 if could not guess */ -static int guess_disk_lchs(BlockDriverState *bs, - int *pcylinders, int *pheads, int *psectors) +static int guess_disk_msdosgeo(BlockDriverState *bs, + uint32_t *pcylinders, uint32_t *pheads, + uint32_t *psectors, int *ptranslation) { uint8_t buf[BDRV_SECTOR_SIZE]; -int i, heads, sectors, cylinders; +int i, translation; +uint32_t heads, sectors, cylinders; struct partition *p; uint32_t nr_sects; uint64_t nb_sectors; @@ -87,9 +137,26 @@ static int guess_disk_lchs(BlockDriverState *bs, if (cylinders 1 || cylinders 16383) { continue; } + +if (heads 16) { +/* LCHS guess with heads 16 means that a BIOS LBA + translation was active, so a standard physical disk + geometry is OK */ +guess_chs_for_size(bs, cylinders, heads, sectors); +translation = cylinders * heads = 131072 +? BIOS_ATA_TRANSLATION_LARGE +: BIOS_ATA_TRANSLATION_LBA; +} else { +/* LCHS guess with heads = 16: use as physical geometry */ +/* disable any translation to be in sync with + the logical geometry */ +translation = BIOS_ATA_TRANSLATION_NONE; +} *pheads = heads; *psectors = sectors; *pcylinders = cylinders; +*ptranslation = translation; + trace_hd_geometry_lchs_guess(bs, cylinders, heads, sectors); return 0; } @@ -97,51 +164,28 @@ static int guess_disk_lchs(BlockDriverState *bs, return -1; } -static void guess_chs_for_size(BlockDriverState *bs, -uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs) -{ -uint64_t nb_sectors; -int cylinders; - -bdrv_get_geometry(bs, nb_sectors); - -cylinders = nb_sectors / (16 * 63); -if
[Qemu-devel] [PATCH 1/2] virtio-block: support auto-sensing of block sizes
From: Einar Lueck elelu...@linux.vnet.ibm.com Virtio-blk does not impose fixed block sizes for access to backing devices. This patch introduces support for auto lookup of the block sizes of the backing block device. This automatic lookup needs to be enabled explicitly. Users may do this by specifying (physical|logical)_block_size=0. Machine types may do this for their defaults, too. To achieve this, a new function blkconf_blocksizes is implemented. If physical or logical block size are zero a corresponding ioctl tries to find an appropriate value. If this does not work, 512 is used. blkconf_blocksizes is therefore only called w/in the virtio-blk context. For s390-virtio, this patch configures auto lookup as default. Signed-off-by: Einar Lueck elelu...@linux.vnet.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com --- hw/block-common.c| 23 +++ hw/block-common.h| 12 +--- hw/qdev-properties.c | 4 +++- hw/s390-virtio-bus.c | 2 +- hw/virtio-blk.c | 1 + 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/hw/block-common.c b/hw/block-common.c index f0196d7..444edd2 100644 --- a/hw/block-common.c +++ b/hw/block-common.c @@ -10,6 +10,9 @@ #include blockdev.h #include hw/block-common.h #include qemu-error.h +#ifdef __linux__ +#include linux/fs.h +#endif void blkconf_serial(BlockConf *conf, char **serial) { @@ -24,6 +27,26 @@ void blkconf_serial(BlockConf *conf, char **serial) } } +void blkconf_blocksizes(BlockConf *conf) +{ +int block_size; + +if (!conf-physical_block_size) { +if (bdrv_ioctl(conf-bs, BLKPBSZGET, block_size) == 0) { +conf-physical_block_size = (uint16_t) block_size; +} else { +conf-physical_block_size = BLOCK_PROPERTY_STD_BLKSIZE; +} +} +if (!conf-logical_block_size) { +if (bdrv_ioctl(conf-bs, BLKSSZGET, block_size) == 0) { +conf-logical_block_size = (uint16_t) block_size; +} else { +conf-logical_block_size = BLOCK_PROPERTY_STD_BLKSIZE; +} +} +} + int blkconf_geometry(BlockConf *conf, int *ptrans, unsigned cyls_max, unsigned heads_max, unsigned secs_max) { diff --git a/hw/block-common.h b/hw/block-common.h index bb808f7..d593128 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -40,18 +40,23 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) return exp; } -#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ +#define BLOCK_PROPERTY_STD_BLKSIZE 512 +#define DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, _blksize) \ DEFINE_PROP_DRIVE(drive, _state, _conf.bs), \ DEFINE_PROP_BLOCKSIZE(logical_block_size, _state, \ - _conf.logical_block_size, 512), \ + _conf.logical_block_size, _blksize), \ DEFINE_PROP_BLOCKSIZE(physical_block_size, _state,\ - _conf.physical_block_size, 512), \ + _conf.physical_block_size, _blksize), \ DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \ DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0),\ DEFINE_PROP_INT32(bootindex, _state, _conf.bootindex, -1),\ DEFINE_PROP_UINT32(discard_granularity, _state, \ _conf.discard_granularity, 0) +#define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ +DEFINE_BLOCK_PROPERTIES_EXTENDED(_state, _conf, \ + BLOCK_PROPERTY_STD_BLKSIZE) + #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ DEFINE_PROP_UINT32(cyls, _state, _conf.cyls, 0), \ DEFINE_PROP_UINT32(heads, _state, _conf.heads, 0), \ @@ -60,6 +65,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) /* Configuration helpers */ void blkconf_serial(BlockConf *conf, char **serial); +void blkconf_blocksizes(BlockConf *conf); int blkconf_geometry(BlockConf *conf, int *trans, unsigned cyls_max, unsigned heads_max, unsigned secs_max); diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 8aca0d4..e99dee4 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -904,7 +904,9 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, error_propagate(errp, local_err); return; } -if (value min || value max) { + +/* value == 0 indicates that block size should be sensed later on */ +if ((value min || value max) value 0) { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, dev-id?:, name, (int64_t)value, min, max); return; diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index a245684..562964e 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -401,7 +401,7 @@
[Qemu-devel] [PATCH 0/2] s390: block size auto-sensing and geometry guessing changes in common code
[resend because the first attempt didn't make it to the mailing list] Here are two patches for block device characteristics detection. The first one implements block size auto-sensing for virtio-block and the second patch changes geometry guessing common code. More details in the patch descriptions. Einar Lueck (2): virtio-block: support auto-sensing of block sizes hd-geometry.c: Integrate HDIO_GETGEO in guessing hw/block-common.c| 23 ++ hw/block-common.h| 12 -- hw/hd-geometry.c | 118 +++ hw/qdev-properties.c | 4 +- hw/s390-virtio-bus.c | 2 +- hw/virtio-blk.c | 1 + 6 files changed, 118 insertions(+), 42 deletions(-) -- 1.7.11.4
Re: [Qemu-devel] [qemu-devel] [PATCH V2 2/3] [RFC] libqblock-API design
于 2012-8-10 18:48, Paolo Bonzini 写道: Il 09/08/2012 12:12, Wenchao Xia ha scritto: +/* copy information and take care the member difference in differect version. + Assuming all new member are added in the tail, struct size is the first + member, this is old to new version, src have its struct_size set. */ +static void qboc_adjust_o2n(struct QBlockOptionCreate *dest, +struct QBlockOptionCreate *src) +{ +/* for simple it does memcpy now, need to take care of embbed structure */ +memcpy(dest, src, src-struct_size); You need an assertion that src-struct_size sizeof(*dest). However, the structure size perhaps wasn't my brightest idea ever. But still thanks for preparing this prototype! Now that we have a more complete API to discuss, we can iterate to something better. +assert(0 == set_option_parameter_int(param, +BLOCK_OPT_SIZE, o_cow-virt_size)); Assertions should not have side effects. what side effects would this line have? diff --git a/libqblock.h b/libqblock.h new file mode 100644 index 000..d2e9502 --- /dev/null +++ b/libqblock.h @@ -0,0 +1,447 @@ +/* + * Copyright IBM Corp. 2012 + * + * Authors: + * Wenchao Xia xiaw...@linux.vnet.ibm.com + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef LIBQBLOCK_H +#define LIBQBLOCK_H + +#include stdio.h +#include stdint.h +#include stdlib.h + +#define bool _Bool + +#define QB_ERR_MEM_ERR (-1) +#define QB_ERR_INTERNAL_ERR (-2) +#define QB_ERR_INVALID_PARAM (-3) + +/* this library is designed around this core struct. */ +struct QBlockState; + +/* + libarary init + This function get the library ready to use. + */ +void libqblock_init(void); + +/* + create a new qbs object + params: + qbs: out, pointer that will receive created obj. + return: + 0 on succeed, negative on failure. + */ +int qb_state_new(struct QBlockState **qbs); + +/* + delete a qbs object + params: + qbs: in, pointer that will be freed. *qbs will be set to NULL. + return: + void. + */ +void qb_state_free(struct QBlockState **qbs); + + +/* flag used in open and create */ +#define LIBQBLOCK_O_RDWR0x0002 +/* open the file read only and save writes in a snapshot */ +#define LIBQBLOCK_O_SNAPSHOT0x0008 I'd rather avoid exposing this for now. +/* do not use the host page cache */ +#define LIBQBLOCK_O_NOCACHE 0x0020 +/* use write-back caching */ +#define LIBQBLOCK_O_CACHE_WB0x0040 +/* use native AIO instead of the thread pool */ +#define LIBQBLOCK_O_NATIVE_AIO 0x0080 NATIVE_AIO should be an option for the file protocol. But it's mostly for performance and since we only support synchronous I/O we can drop it for now. +/* don't open the backing file */ +#define LIBQBLOCK_O_NO_BACKING 0x0100 +/* disable flushing on this disk */ +#define LIBQBLOCK_O_NO_FLUSH0x0200 +/* copy read backing sectors into image */ +#define LIBQBLOCK_O_COPY_ON_READ 0x0400 I'd rather avoid exposing this for now too. +/* consistency hint for incoming migration */ +#define LIBQBLOCK_O_INCOMING0x0800 This is internal to QEMU. Please add a mask of valid bits and an assertion that unknown bits are zero. OK to hide this flag now. Do you think this flag belongs to the control flag or I should sort them to anther option? In long term the library should provide full capablities as qemu block layer have so qemu can try migrate to it, so now I need to make sure the API designed have left room for all features qemu have support. + +#define LIBQBLOCK_O_CACHE_MASK \ + (LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | LIBQBLOCK_O_NO_FLUSH) + +enum QBlockProtocol { +QB_PROTO_FILE = 0, +QB_PROTO_MAX +}; + +enum QBlockFormat { +QB_FMT_NONE = 0, +QB_FMT_COW, +QB_FMT_QED, +QB_FMT_QCOW, +QB_FMT_QCOW2, +QB_FMT_RAW, +QB_FMT_RBD, +QB_FMT_SHEEPDOG, +QB_FMT_VDI, +QB_FMT_VMDK, +QB_FMT_VPC, +QB_FMT_MAX +}; + +/* block target location info, it include all information about how to find + the image */ +struct QBlockLocInfo { +int struct_size; +const char *filename; +enum QBlockProtocol protocol; +}; + +/* how to open the image */ +struct QBlockOptionOpen { +int struct_size; +struct QBlockLocInfo o_loc;
[Qemu-devel] [RFC 0/7] Migration stats
Hi This modifies the output of info migrate/qmp_query_migrate to add the stats that I got request for. - It moves total time to MigrationInfo instead of ram (luiz suggestion) - Prints the real downtime that we have had really, it prints the total downtime of the complete phase, but the downtime also includes the last ram_iterate phase. Working on fixing that one. - Prints the expected downtime of the last time that we synchronized the dirty bitmap with kvm. So we have one idea of what downtime value we need for migration to converge. - Prints the dirty_pages_rate, that is the number of pages that we have written in the last second. This one prints always zero. To fill it, I need the dirty bitmap changes on the migration_thread series. Patch series apply on top of the migration-next-20120808 series sent to anthony. What do I want to know: - is there any stat that you want? Once here, adding a new one should be easy. - examples are not done, waiting until people agree with what params are needed. - luiz added in case he has QMP commets. - erik added for libvirt comments. Added before is the link to the branch on my repository. The following changes since commit 346fe0c4c0b88f11a3d0c01c34d9a170d73429cc: Merge remote-tracking branch 'stefanha/trivial-patches' into staging (2012-08-11 19:49:03 -0500) are available in the git repository at: http://repo.or.cz/r/qemu/quintela.git migration-stats for you to fetch changes up to e0599012abfc4f9a68185c6f0a10a7b98c0a180f: migration: Add dirty_pages_rate to query migrate output (2012-08-13 12:33:35 +0200) Please review, and comment. Juan Quintela (7): migration: move total_time from ram stats to migration info migration: store end_time in a local variable migration: print total downtime for final phase of migration migration: rename expected_time to expected_downtime migration: export migration_get_current() migration: print expected downtime in info migrate migration: Add dirty_pages_rate to query migrate output arch_init.c | 19 +++ hmp.c| 16 ++-- migration.c | 19 ++- migration.h | 4 qapi-schema.json | 26 +++--- 5 files changed, 62 insertions(+), 22 deletions(-) -- 1.7.11.2
[Qemu-devel] [PATCH 2/7] migration: store end_time in a local variable
Signed-off-by: Juan Quintela quint...@redhat.com --- migration.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/migration.c b/migration.c index 8e4c508..159728d 100644 --- a/migration.c +++ b/migration.c @@ -326,6 +326,7 @@ static void migrate_fd_put_ready(void *opaque) migrate_fd_error(s); } else if (ret == 1) { int old_vm_running = runstate_is_running(); +int64_t end_time; DPRINTF(done iterating\n); qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER); @@ -336,7 +337,8 @@ static void migrate_fd_put_ready(void *opaque) } else { migrate_fd_completed(s); } -s-total_time = qemu_get_clock_ms(rt_clock) - s-total_time; +end_time = qemu_get_clock_ms(rt_clock); +s-total_time = end_time - s-total_time; if (s-state != MIG_STATE_COMPLETED) { if (old_vm_running) { vm_start(); -- 1.7.11.2
[Qemu-devel] [PATCH 3/7] migration: print total downtime for final phase of migration
Signed-off-by: Juan Quintela quint...@redhat.com --- hmp.c| 4 migration.c | 5 - migration.h | 1 + qapi-schema.json | 6 +- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/hmp.c b/hmp.c index c0b0a10..10fee1b 100644 --- a/hmp.c +++ b/hmp.c @@ -151,6 +151,10 @@ void hmp_info_migrate(Monitor *mon) monitor_printf(mon, Migration status: %s\n, info-status); monitor_printf(mon, total time: % PRIu64 milliseconds\n, info-total_time); +if (strcmp(info-status, completed) == 0) { +monitor_printf(mon, downtime: % PRIu64 milliseconds\n, + info-downtime); +} } if (info-has_ram) { diff --git a/migration.c b/migration.c index 159728d..4c2ac6c 100644 --- a/migration.c +++ b/migration.c @@ -194,6 +194,7 @@ MigrationInfo *qmp_query_migrate(Error **errp) info-has_status = true; info-status = g_strdup(completed); info-total_time = s-total_time; +info-downtime = s-downtime; info-has_ram = true; info-ram = g_malloc0(sizeof(*info-ram)); @@ -326,9 +327,10 @@ static void migrate_fd_put_ready(void *opaque) migrate_fd_error(s); } else if (ret == 1) { int old_vm_running = runstate_is_running(); -int64_t end_time; +int64_t start_time, end_time; DPRINTF(done iterating\n); +start_time = qemu_get_clock_ms(rt_clock); qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER); vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); @@ -339,6 +341,7 @@ static void migrate_fd_put_ready(void *opaque) } end_time = qemu_get_clock_ms(rt_clock); s-total_time = end_time - s-total_time; +s-downtime = end_time - start_time; if (s-state != MIG_STATE_COMPLETED) { if (old_vm_running) { vm_start(); diff --git a/migration.h b/migration.h index a9852fc..3462917 100644 --- a/migration.h +++ b/migration.h @@ -40,6 +40,7 @@ struct MigrationState void *opaque; MigrationParams params; int64_t total_time; +int64_t downtime; bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; int64_t xbzrle_cache_size; }; diff --git a/qapi-schema.json b/qapi-schema.json index 907051a..6df1ce3 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -319,13 +319,17 @@ #If migration has ended, it returns the total migration #time. (since 1.2) # +# @downtime: total downtime in milliseconds for the guest. +#(since 1.2) +# # Since: 0.14.0 ## { 'type': 'MigrationInfo', 'data': {'*status': 'str', '*ram': 'MigrationStats', '*disk': 'MigrationStats', '*xbzrle-cache': 'XBZRLECacheStats', - 'total-time': 'int'} } + 'total-time': 'int', + 'downtime': 'int'} } ## # @query-migrate -- 1.7.11.2
[Qemu-devel] [PATCH v2 0/2] two little build fixes
Hi, Patch #1 uses g_strdup_printf now, #2 is unchanged. cheers, Gerd Gerd Hoffmann (2): Avoid asprintf() which is not available on mingw scsi: fix warning hw/msix.c |8 ++-- hw/scsi-bus.c |2 ++ 2 files changed, 4 insertions(+), 6 deletions(-)
[Qemu-devel] [PATCH v2 2/2] scsi: fix warning
hw/scsi-bus.c:758: warning: ‘xfer’ may be used uninitialized in this function Isn't true, but older gcc versions (for example 4.1 as shipped in rhel5) are not clever enougth to figure, so sprinkle in a default: line to make them happy. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/scsi-bus.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index b8a857d..4981a02 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -761,6 +761,7 @@ static int ata_passthrough_12_xfer_size(SCSIDevice *dev, uint8_t *buf) switch (length) { case 0: case 3: /* USB-specific. */ +default: xfer = 0; break; case 1: @@ -784,6 +785,7 @@ static int ata_passthrough_16_xfer_size(SCSIDevice *dev, uint8_t *buf) switch (length) { case 0: case 3: /* USB-specific. */ +default: xfer = 0; break; case 1: -- 1.7.1
[Qemu-devel] [PATCH v2 1/2] Avoid asprintf() which is not available on mingw
Use g_strdup_printf() instead. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/msix.c |8 ++-- 1 files changed, 2 insertions(+), 6 deletions(-) diff --git a/hw/msix.c b/hw/msix.c index 800fc32..aea340b 100644 --- a/hw/msix.c +++ b/hw/msix.c @@ -307,13 +307,9 @@ int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries, return -EINVAL; } -if (asprintf(name, %s-msix, dev-name) == -1) { -return -ENOMEM; -} - +name = g_strdup_printf(%s-msix, dev-name); memory_region_init(dev-msix_exclusive_bar, name, MSIX_EXCLUSIVE_BAR_SIZE); - -free(name); +g_free(name); ret = msix_init(dev, nentries, dev-msix_exclusive_bar, bar_nr, MSIX_EXCLUSIVE_BAR_TABLE_OFFSET, dev-msix_exclusive_bar, -- 1.7.1
[Qemu-devel] [PATCH 6/7] migration: print expected downtime in info migrate
Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 2 ++ hmp.c| 4 migration.c | 2 ++ migration.h | 1 + qapi-schema.json | 4 5 files changed, 13 insertions(+) diff --git a/arch_init.c b/arch_init.c index 5559d68..b150852 100644 --- a/arch_init.c +++ b/arch_init.c @@ -538,6 +538,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) int ret; int i; uint64_t expected_downtime; +MigrationState *s = migrate_get_current(); bytes_transferred_last = bytes_transferred; bwidth = qemu_get_clock_ns(rt_clock); @@ -593,6 +594,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) if (expected_downtime = migrate_max_downtime()) { memory_global_sync_dirty_bitmap(get_system_memory()); expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth; +s-expected_downtime = expected_downtime / 100; /* ns - ms */ return expected_downtime = migrate_max_downtime(); } diff --git a/hmp.c b/hmp.c index 10fee1b..fc75ec3 100644 --- a/hmp.c +++ b/hmp.c @@ -151,6 +151,10 @@ void hmp_info_migrate(Monitor *mon) monitor_printf(mon, Migration status: %s\n, info-status); monitor_printf(mon, total time: % PRIu64 milliseconds\n, info-total_time); +if (info-expected_downtime) { +monitor_printf(mon, expected downtime: % PRIu64 milliseconds\n, + info-expected_downtime); +} if (strcmp(info-status, completed) == 0) { monitor_printf(mon, downtime: % PRIu64 milliseconds\n, info-downtime); diff --git a/migration.c b/migration.c index 12de941..28e23db 100644 --- a/migration.c +++ b/migration.c @@ -168,6 +168,7 @@ MigrationInfo *qmp_query_migrate(Error **errp) info-status = g_strdup(active); info-total_time = qemu_get_clock_ms(rt_clock) - s-total_time; +info-expected_downtime = s-expected_downtime; info-has_ram = true; info-ram = g_malloc0(sizeof(*info-ram)); @@ -195,6 +196,7 @@ MigrationInfo *qmp_query_migrate(Error **errp) info-status = g_strdup(completed); info-total_time = s-total_time; info-downtime = s-downtime; +info-expected_downtime = 0; info-has_ram = true; info-ram = g_malloc0(sizeof(*info-ram)); diff --git a/migration.h b/migration.h index dabc333..552200c 100644 --- a/migration.h +++ b/migration.h @@ -41,6 +41,7 @@ struct MigrationState MigrationParams params; int64_t total_time; int64_t downtime; +int64_t expected_downtime; bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; int64_t xbzrle_cache_size; }; diff --git a/qapi-schema.json b/qapi-schema.json index 6df1ce3..3dcc12f 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -322,6 +322,9 @@ # @downtime: total downtime in milliseconds for the guest. #(since 1.2) # +# @expected-downtime: expected downtime in milliseconds for the +#guest in last walk of the dirty bitmap. (sincte 1.2) +# # Since: 0.14.0 ## { 'type': 'MigrationInfo', @@ -329,6 +332,7 @@ '*disk': 'MigrationStats', '*xbzrle-cache': 'XBZRLECacheStats', 'total-time': 'int', + 'expected-downtime': 'int', 'downtime': 'int'} } ## -- 1.7.11.2
[Qemu-devel] [PATCH 7/7] migration: Add dirty_pages_rate to query migrate output
For now this is a placeholder, real info will appear once the bitmap changes in the migration thread series is integrated. Signed-off-by: Juan Quintela quint...@redhat.com --- hmp.c| 4 migration.c | 2 ++ migration.h | 1 + qapi-schema.json | 6 +- 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/hmp.c b/hmp.c index fc75ec3..dd40631 100644 --- a/hmp.c +++ b/hmp.c @@ -174,6 +174,10 @@ void hmp_info_migrate(Monitor *mon) info-ram-normal); monitor_printf(mon, normal bytes: % PRIu64 kbytes\n, info-ram-normal_bytes 10); +if (info-ram-dirty_pages_rate) { +monitor_printf(mon, dirty pages rate: % PRIu64 pagfes\n, + info-ram-dirty_pages_rate); +} } if (info-has_disk) { diff --git a/migration.c b/migration.c index 28e23db..8d67e9b 100644 --- a/migration.c +++ b/migration.c @@ -178,6 +178,8 @@ MigrationInfo *qmp_query_migrate(Error **errp) info-ram-duplicate = dup_mig_pages_transferred(); info-ram-normal = norm_mig_pages_transferred(); info-ram-normal_bytes = norm_mig_bytes_transferred(); +info-ram-dirty_pages_rate = s-dirty_pages_rate; + if (blk_mig_active()) { info-has_disk = true; diff --git a/migration.h b/migration.h index 552200c..66d7f68 100644 --- a/migration.h +++ b/migration.h @@ -42,6 +42,7 @@ struct MigrationState int64_t total_time; int64_t downtime; int64_t expected_downtime; +int64_t dirty_pages_rate; bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; int64_t xbzrle_cache_size; }; diff --git a/qapi-schema.json b/qapi-schema.json index 3dcc12f..55ef73c 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -266,11 +266,15 @@ # # @normal-bytes : number of normal bytes sent (since 1.2) # +# @dirty-pages-rate: number of pages dirtied by second by the +#guest. (since 1.2) +# # Since: 0.14.0 ## { 'type': 'MigrationStats', 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , - 'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int' } } + 'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int', + 'dirty-pages-rate' : 'int' } } ## # @XBZRLECacheStats -- 1.7.11.2
[Qemu-devel] [PATCH 1/7] migration: move total_time from ram stats to migration info
Signed-off-by: Juan Quintela quint...@redhat.com --- hmp.c| 4 ++-- migration.c | 6 +++--- qapi-schema.json | 14 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/hmp.c b/hmp.c index c13386b..c0b0a10 100644 --- a/hmp.c +++ b/hmp.c @@ -149,6 +149,8 @@ void hmp_info_migrate(Monitor *mon) if (info-has_status) { monitor_printf(mon, Migration status: %s\n, info-status); +monitor_printf(mon, total time: % PRIu64 milliseconds\n, + info-total_time); } if (info-has_ram) { @@ -158,8 +160,6 @@ void hmp_info_migrate(Monitor *mon) info-ram-remaining 10); monitor_printf(mon, total ram: % PRIu64 kbytes\n, info-ram-total 10); -monitor_printf(mon, total time: % PRIu64 milliseconds\n, - info-ram-total_time); monitor_printf(mon, duplicate: % PRIu64 pages\n, info-ram-duplicate); monitor_printf(mon, normal: % PRIu64 pages\n, diff --git a/migration.c b/migration.c index 653a3c1..8e4c508 100644 --- a/migration.c +++ b/migration.c @@ -166,14 +166,14 @@ MigrationInfo *qmp_query_migrate(Error **errp) case MIG_STATE_ACTIVE: info-has_status = true; info-status = g_strdup(active); +info-total_time = qemu_get_clock_ms(rt_clock) +- s-total_time; info-has_ram = true; info-ram = g_malloc0(sizeof(*info-ram)); info-ram-transferred = ram_bytes_transferred(); info-ram-remaining = ram_bytes_remaining(); info-ram-total = ram_bytes_total(); -info-ram-total_time = qemu_get_clock_ms(rt_clock) -- s-total_time; info-ram-duplicate = dup_mig_pages_transferred(); info-ram-normal = norm_mig_pages_transferred(); info-ram-normal_bytes = norm_mig_bytes_transferred(); @@ -193,13 +193,13 @@ MigrationInfo *qmp_query_migrate(Error **errp) info-has_status = true; info-status = g_strdup(completed); +info-total_time = s-total_time; info-has_ram = true; info-ram = g_malloc0(sizeof(*info-ram)); info-ram-transferred = ram_bytes_transferred(); info-ram-remaining = 0; info-ram-total = ram_bytes_total(); -info-ram-total_time = s-total_time; info-ram-duplicate = dup_mig_pages_transferred(); info-ram-normal = norm_mig_pages_transferred(); info-ram-normal_bytes = norm_mig_bytes_transferred(); diff --git a/qapi-schema.json b/qapi-schema.json index 56d9d7b..907051a 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -260,10 +260,6 @@ # # @total: total amount of bytes involved in the migration process # -# @total-time: total amount of ms since migration started. If -#migration has ended, it returns the total migration -#time. (since 1.2) -# # @duplicate: number of duplicate pages (since 1.2) # # @normal : number of normal pages (since 1.2) @@ -274,8 +270,7 @@ ## { 'type': 'MigrationStats', 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , - 'total-time': 'int', 'duplicate': 'int', 'normal': 'int', - 'normal-bytes': 'int' } } + 'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int' } } ## # @XBZRLECacheStats @@ -320,12 +315,17 @@ #migration statistics, only returned if XBZRLE feature is on and #status is 'active' or 'completed' (since 1.2) # +# @total-time: total amount of milliseconds since migration started. +#If migration has ended, it returns the total migration +#time. (since 1.2) +# # Since: 0.14.0 ## { 'type': 'MigrationInfo', 'data': {'*status': 'str', '*ram': 'MigrationStats', '*disk': 'MigrationStats', - '*xbzrle-cache': 'XBZRLECacheStats'} } + '*xbzrle-cache': 'XBZRLECacheStats', + 'total-time': 'int'} } ## # @query-migrate -- 1.7.11.2
Re: [Qemu-devel] [PATCH] vmstate: Add support for saving/loading bitmaps
Peter Maydell peter.mayd...@linaro.org wrote: Add support for saving/loading bitmap.h bitmaps in vmstate. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- This will be needed for saving/restoring the bitmap in sd.c which is introduced by Igor's latest patchset; the relevant VMSTATE line is: VMSTATE_BITMAP(wp_groups, SDState, 1, wpgrps_size), (and you'll need to make wpgrps_size an int32_t, not uint32_t). Igor: I've only tested this fairly lightly, you'll probably want to do things like testing save on 32 bit and load on 64 bit and vice-versa. savevm.c | 41 + vmstate.h | 13 + 2 files changed, 54 insertions(+) Reviewed-by: Juan Quintela quint...@redhat.com I haven't tested it, but Igor did, so O;-) I can add it to my next pull request, or let it on Igor one that is the one using it. Both ways work for me.
[Qemu-devel] [PATCH 4/7] migration: rename expected_time to expected_downtime
Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/arch_init.c b/arch_init.c index 9b46bfc..5559d68 100644 --- a/arch_init.c +++ b/arch_init.c @@ -537,7 +537,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) double bwidth = 0; int ret; int i; -uint64_t expected_time; +uint64_t expected_downtime; bytes_transferred_last = bytes_transferred; bwidth = qemu_get_clock_ns(rt_clock); @@ -576,24 +576,25 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) bwidth = qemu_get_clock_ns(rt_clock) - bwidth; bwidth = (bytes_transferred - bytes_transferred_last) / bwidth; -/* if we haven't transferred anything this round, force expected_time to a - * a very high value, but without crashing */ +/* if we haven't transferred anything this round, force + * expected_downtime to a a very high value, but without + * crashing */ if (bwidth == 0) { bwidth = 0.01; } qemu_put_be64(f, RAM_SAVE_FLAG_EOS); -expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth; +expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth; DPRINTF(ram_save_live: expected( PRIu64 ) = max( PRIu64 )?\n, -expected_time, migrate_max_downtime()); +expected_downtime, migrate_max_downtime()); -if (expected_time = migrate_max_downtime()) { +if (expected_downtime = migrate_max_downtime()) { memory_global_sync_dirty_bitmap(get_system_memory()); -expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth; +expected_downtime = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth; -return expected_time = migrate_max_downtime(); +return expected_downtime = migrate_max_downtime(); } return 0; } -- 1.7.11.2
[Qemu-devel] [PATCH 5/7] migration: export migration_get_current()
Signed-off-by: Juan Quintela quint...@redhat.com --- migration.c | 2 +- migration.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/migration.c b/migration.c index 4c2ac6c..12de941 100644 --- a/migration.c +++ b/migration.c @@ -53,7 +53,7 @@ static NotifierList migration_state_notifiers = migrations at once. For now we don't need to add dynamic creation of migration */ -static MigrationState *migrate_get_current(void) +MigrationState *migrate_get_current(void) { static MigrationState current_migration = { .state = MIG_STATE_SETUP, diff --git a/migration.h b/migration.h index 3462917..dabc333 100644 --- a/migration.h +++ b/migration.h @@ -81,6 +81,7 @@ void remove_migration_state_change_notifier(Notifier *notify); bool migration_is_active(MigrationState *); bool migration_has_finished(MigrationState *); bool migration_has_failed(MigrationState *); +MigrationState *migrate_get_current(void); uint64_t ram_bytes_remaining(void); uint64_t ram_bytes_transferred(void); -- 1.7.11.2
Re: [Qemu-devel] [qemu-devel] [PATCH V2 2/3] [RFC] libqblock-API design
于 2012-8-10 19:02, Kevin Wolf 写道: Am 09.08.2012 12:12, schrieb Wenchao Xia: This patch is the API design. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- libqblock.c | 670 +++ libqblock.h | 447 +++ 2 files changed, 1117 insertions(+), 0 deletions(-) create mode 100644 libqblock.c create mode 100644 libqblock.h Ignoring the implementation for now as the design should be visible in the header file. diff --git a/libqblock.h b/libqblock.h new file mode 100644 index 000..d2e9502 --- /dev/null +++ b/libqblock.h @@ -0,0 +1,447 @@ +/* + * Copyright IBM Corp. 2012 + * + * Authors: + * Wenchao Xia xiaw...@linux.vnet.ibm.com + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef LIBQBLOCK_H +#define LIBQBLOCK_H + +#include stdio.h +#include stdint.h +#include stdlib.h + +#define bool _Bool Why not use stdbool.h? forgot to use that, will change it, thanks. + +#define QB_ERR_MEM_ERR (-1) +#define QB_ERR_INTERNAL_ERR (-2) +#define QB_ERR_INVALID_PARAM (-3) qemu uses errno values internally, I think they would make sense in the library interface as well. + +/* this library is designed around this core struct. */ +struct QBlockState; + +/* + libarary init + This function get the library ready to use. + */ +void libqblock_init(void); + +/* + create a new qbs object + params: + qbs: out, pointer that will receive created obj. + return: + 0 on succeed, negative on failure. + */ +int qb_state_new(struct QBlockState **qbs); + +/* + delete a qbs object + params: + qbs: in, pointer that will be freed. *qbs will be set to NULL. + return: + void. + */ +void qb_state_free(struct QBlockState **qbs); What happens if the qbs is open? Will it be flushed and closed? If so, can it fail and we need to allow an error return? user should call qb_close() if he have called qb_open(), other wise unexpected thing happens such as this file is not flushed. I need document this in the comments. + +/* flag used in open and create */ +#define LIBQBLOCK_O_RDWR0x0002 +/* open the file read only and save writes in a snapshot */ +#define LIBQBLOCK_O_SNAPSHOT0x0008 +/* do not use the host page cache */ +#define LIBQBLOCK_O_NOCACHE 0x0020 +/* use write-back caching */ +#define LIBQBLOCK_O_CACHE_WB0x0040 +/* use native AIO instead of the thread pool */ +#define LIBQBLOCK_O_NATIVE_AIO 0x0080 +/* don't open the backing file */ +#define LIBQBLOCK_O_NO_BACKING 0x0100 +/* disable flushing on this disk */ +#define LIBQBLOCK_O_NO_FLUSH0x0200 +/* copy read backing sectors into image */ +#define LIBQBLOCK_O_COPY_ON_READ 0x0400 +/* consistency hint for incoming migration */ +#define LIBQBLOCK_O_INCOMING0x0800 + +#define LIBQBLOCK_O_CACHE_MASK \ + (LIBQBLOCK_O_NOCACHE | LIBQBLOCK_O_CACHE_WB | LIBQBLOCK_O_NO_FLUSH) + +enum QBlockProtocol { +QB_PROTO_FILE = 0, +QB_PROTO_MAX +}; + +enum QBlockFormat { +QB_FMT_NONE = 0, +QB_FMT_COW, +QB_FMT_QED, +QB_FMT_QCOW, +QB_FMT_QCOW2, +QB_FMT_RAW, +QB_FMT_RBD, +QB_FMT_SHEEPDOG, +QB_FMT_VDI, +QB_FMT_VMDK, +QB_FMT_VPC, +QB_FMT_MAX +}; Not sure if this is a good idea with respect to extensibility. Today you only need to create a new block/foo.c and add it to the Makefile in order to add a new format and protocol. It would be better if the library could make use of it without changing these enums, e.g. by referring to formats by string (possibly getting a struct referring to a format by something like qblk_get_format(raw)) qblk_get_format(raw) seems a good way solve the option difference, maybe user call it as: void *op = qblk_get_format_option(raw); struct QBlockOption_raw *o_raw = (struct QBlockOption_raw *)op; o_raw.virt_size = 16 * 1024; I think this is good, with the cost of user have to deal string instead of simpler enum integer. + +/* block target location info, it include all information about how to find + the image */ +struct QBlockLocInfo { +int struct_size; +const char *filename; +enum QBlockProtocol protocol; +}; This is a relatively simplistic view of the world. It works okay for local image files (which is probably the
Re: [Qemu-devel] [qemu-devel] [PATCH V2 0/3] [RFC] libqblock draft code v2
于 2012-8-11 20:18, Blue Swirl 写道: On Fri, Aug 10, 2012 at 8:04 AM, Wenchao Xia xiaw...@linux.vnet.ibm.com wrote: Thanks for your review, sorry I have forgot some fixing you mentioned before, will correct them this time. 于 2012-8-10 1:12, Blue Swirl 写道: On Thu, Aug 9, 2012 at 10:10 AM, Wenchao Xia xiaw...@linux.vnet.ibm.com wrote: This patch intrudce libqblock API, libqblock-test is used as a test case. V2: Using struct_size and [xxx]_new [xxx]_free to keep ABI. Using embbed structure to class format options in image creating. Format specific options were brought to surface. Image format was changed to enum interger instead of string. Some API were renamed. Internel error with errno was saved and with an API caller can get it. ALL flags used were defined in libqblock.h. Something need discuss: Embbed structure or union could make the model more friendly, but that make ABI more difficult, because we need to check every embbed structure's size and guess compiler's memory arrangement. This means #pragma pack(4) or struct_size, offset_next in every structure. Any better way to solve it? or make every structure a plain one? I'd still use accessor functions instead of structure passing, it would avoid these problems. Do you mean some function like : CreateOption_Filename_Set(const char *filename) CreateOption_Format_Set(const char *filename) Something like this: int qb_create_cow(struct QBlockState *qbs, const char *filename, size_t virt_size, const char *backing_file, int backing_flag); etc. for rest of the formats. Alternatively, we could have more generic versions like you describe, or even more generic still: void qb_set_property(struct QBlockState *qbs, const char *prop_name, const char *prop_value); so the create sequence (ignoring error handling) would be: s = qb_init(); qb_set_property(s, filename, c:autoexec.bat); qb_set_property(s, format, cow); qb_set_property(s, virt_size, 10GB); // use defaults for backing_file and backing_flag qb_create(s); Likewise for info structure: char *qb_get_property(struct QBlockState *qbs, const char *prop_name); foo = qb_get_property(s, format); foo = qb_get_property(s, encrypted); foo = qb_get_property(s, num_backing_files); foo = qb_get_property(s, virt_size); This would be helpful for the client to display parameters without much understanding of their contents: char **qb_list_properties(struct QBlockState *qbs); /* returns array of property names available for this file, use get_property to retrieve their contents */ But the clients can't be completely ignorant of all formats available, for example a second UI dialog needs to be added for formats with backing files, otherwise it won't be able to access some files at all. Maybe by adding type descriptions for each property (type for filename is path, for others string, bool, enum etc). Thanks. This seems heavy document is needed for that no structure can indicate what options sub format have, user can only get that info from returns or documents. I am not sure if this is good, because it looks more like a object oriented API that C. It can solve the issue, with a cost of more small APIs in header files that user should use. Not sure if there is a good way to make it more friendly as an object language: oc.filename = name; automatically call CreateOption_Filename_Set, API CreateOption_Filename_Set is invisible to user. Packing can even introduce a new set of problems since we don't control the CFLAGS of the client of the library. indeed, I tried to handle the difference in function qboo_adjust_o2n, found that structure member alignment is hard to deal. AIO is missing, need a prototype. Wenchao Xia (3): adding libqblock libqblock API libqblock test case Makefile |3 + block.c |2 +- block.h |1 + libqblock-test.c | 197 libqblock.c | 670 ++ libqblock.h | 447 6 files changed, 1319 insertions(+), 1 deletions(-) create mode 100644 libqblock-test.c create mode 100644 libqblock.c create mode 100644 libqblock.h -- Best Regards Wenchao Xia -- Best Regards Wenchao Xia
Re: [Qemu-devel] [PATCH] block: Set cdrom device read only flag
Kevin Wolf kw...@redhat.com writes: Am 12.08.2012 04:48, schrieb Kevin Shanahan: So qmp_change_blockdev uses bdrv_is_read_only() to check whether to try and open the backing file read only, which uses the -read_only member of struct BlockDriverState to decide whether to pass the BDRV_O_RDRW flag to qmp_bdrv_open_encypted() and then bdrv_open(). I would assume we want to set this flag in drive_init() when the block driver state is initialised. How about a patch like this instead? diff --git a/blockdev.c b/blockdev.c index 8669142..ba22064 100644 --- a/blockdev.c +++ b/blockdev.c @@ -526,6 +526,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) if_name[type], mediastr, unit_id); } dinfo-bdrv = bdrv_new(dinfo-id); +dinfo-bdrv-read_only = ro; dinfo-devaddr = devaddr; dinfo-type = type; dinfo-bus = bus_id; Ah, yes, this looks much more like the proper fix. Basically we need to set everything that is retained after a 'change' command. We have this code in qmp_change_blockdev(): bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR; bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0; qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp); bdrv_is_read_only() is covered by your patch, bdrv_is_snapshot() additionally requires bs-open_flags to be right. Markus, how will this look in the -blockdev world? There seem to be properties that belong to host state, but are not coupled to a medium. Really? Read-only is clearly a property of the medium. There's a separate read-only belonging to the device. A CD-ROM medium is read-only, even when loaded in an optical disk drive that can burn. An optical disk drive that can't burn is read-only, even when writable medium is loaded. Here's how I believe it should work: * BDS member read_only describes the medium. * block.h gets it right: you specify read-only with bdrv_open(), not with bdrv_new(). * -drive gets it right: parameter readonly gets ignored unless we're defining media. Not nice: it's ignored silently. * Monitor command change gets it wrong: it doesn't let you specify the new medium's read-only-ness. Easy enough to fix for QMP, just add a suitable argument. Even better: create a new, non-multiplexed command, and let change rot in peace. Not sure about the best way to fix it in the human monitor. * Device model has its own read-only predicate. * If a device model comes in both a read-only and a read-write flavor, it should have a bool property readonly. * A device model can only use a backend with a suitable media state (has media, is read-only, ...). For instance, ide-hd can't use a read-only backend. * Media change disabled while backend is attached to a device model with fixed media. * Convenient defaults (optional) * A device model property readonly could default to the media's read-only-ness. * Monitor command change could default to readonly when the backend is attached to a device model that can't write. * Both -drive and change could default to readonly when the image isn't writable. This leads to: 1. Replace monitor command change. 2. Optional: look for defaults to improve.
Re: [Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection
On 2012-08-13 10:35, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This is required to get past the following assert with: commit 1523ed9e1d46b0b54540049d491475ccac7e6421 Author: Jan Kiszka jan.kis...@siemens.com Date: Thu May 17 10:32:39 2012 -0300 virtio/vhost: Add support for KVM in-kernel MSI injection Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Cc: Jan Kiszka jan.kis...@siemens.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Anthony Liguori aligu...@us.ibm.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- hw/msix.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/hw/msix.c b/hw/msix.c index 800fc32..c1e6dc3 100644 --- a/hw/msix.c +++ b/hw/msix.c @@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev) { int vector; +if (!dev-msix_vector_use_notifier !dev-msix_vector_release_notifier) +return; + assert(dev-msix_vector_use_notifier dev-msix_vector_release_notifier); I think to remember pointing out that there is a bug somewhere in the reset code which deactivates a non-active vhost instance, no? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH for-1.2 v2] arm: Move some ARM devices into libhw
Avoids some unnecessary dependencies on cpu.h and prepares for a future armeb-softmmu where most machines would not be built. Defer touching the SoC devices since most have implicit or explicit dependencies on the CPU. Signed-off-by: Andreas Färber andreas.faer...@web.de --- default-configs/arm-softmmu.mak | 18 ++ hw/Makefile.objs| 20 hw/arm/Makefile.objs| 15 +++ 3 files changed, 41 insertions(+), 12 deletions(-) v1 - v2: * Move new lm4549.o alongside pl041.o (PMM). * Defer moving stellaris_enet.o until stellaris.o is cleaned up (PMM). diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index e542b4f..f335a72 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -27,3 +27,21 @@ CONFIG_SMC91C111=y CONFIG_DS1338=y CONFIG_PFLASH_CFI01=y CONFIG_PFLASH_CFI02=y + +CONFIG_ARM_TIMER=y +CONFIG_PL011=y +CONFIG_PL022=y +CONFIG_PL031=y +CONFIG_PL041=y +CONFIG_PL050=y +CONFIG_PL061=y +CONFIG_PL080=y +CONFIG_PL110=y +CONFIG_PL181=y +CONFIG_PL190=y +CONFIG_PL310=y +CONFIG_CADENCE=y +CONFIG_XGMAC=y + +CONFIG_VERSATILE_PCI=y +CONFIG_VERSATILE_I2C=y diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 6eee9a0..7f57ed5 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -74,6 +74,26 @@ hw-obj-$(CONFIG_PUV3) += puv3_gpio.o hw-obj-$(CONFIG_PUV3) += puv3_pm.o hw-obj-$(CONFIG_PUV3) += puv3_dma.o +# ARM devices +hw-obj-$(CONFIG_ARM_TIMER) += arm_timer.o +hw-obj-$(CONFIG_PL011) += pl011.o +hw-obj-$(CONFIG_PL022) += pl022.o +hw-obj-$(CONFIG_PL031) += pl031.o +hw-obj-$(CONFIG_PL041) += pl041.o lm4549.o +hw-obj-$(CONFIG_PL050) += pl050.o +hw-obj-$(CONFIG_PL061) += pl061.o +hw-obj-$(CONFIG_PL080) += pl080.o +hw-obj-$(CONFIG_PL110) += pl110.o +hw-obj-$(CONFIG_PL181) += pl181.o +hw-obj-$(CONFIG_PL190) += pl190.o +hw-obj-$(CONFIG_PL310) += arm_l2x0.o +hw-obj-$(CONFIG_VERSATILE_PCI) += versatile_pci.o +hw-obj-$(CONFIG_VERSATILE_I2C) += versatile_i2c.o +hw-obj-$(CONFIG_CADENCE) += cadence_uart.o +hw-obj-$(CONFIG_CADENCE) += cadence_ttc.o +hw-obj-$(CONFIG_CADENCE) += cadence_gem.o +hw-obj-$(CONFIG_XGMAC) += xgmac.o + # PCI watchdog devices hw-obj-$(CONFIG_PCI) += wdt_i6300esb.o diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs index c413780..2b39fb3 100644 --- a/hw/arm/Makefile.objs +++ b/hw/arm/Makefile.objs @@ -1,10 +1,5 @@ -obj-y = integratorcp.o versatilepb.o arm_pic.o arm_timer.o -obj-y += arm_boot.o pl011.o pl031.o pl050.o pl080.o pl110.o pl181.o pl190.o -obj-y += versatile_pci.o -obj-y += versatile_i2c.o -obj-y += cadence_uart.o -obj-y += cadence_ttc.o -obj-y += cadence_gem.o +obj-y = integratorcp.o versatilepb.o arm_pic.o +obj-y += arm_boot.o obj-y += xilinx_zynq.o zynq_slcr.o obj-y += arm_gic.o arm_gic_common.o obj-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o @@ -12,12 +7,9 @@ obj-y += exynos4210_gic.o exynos4210_combiner.o exynos4210.o obj-y += exynos4_boards.o exynos4210_uart.o exynos4210_pwm.o obj-y += exynos4210_pmu.o exynos4210_mct.o exynos4210_fimd.o obj-y += exynos4210_rtc.o exynos4210_i2c.o -obj-y += arm_l2x0.o obj-y += arm_mptimer.o a15mpcore.o -obj-y += armv7m.o armv7m_nvic.o stellaris.o pl022.o stellaris_enet.o +obj-y += armv7m.o armv7m_nvic.o stellaris.o stellaris_enet.o obj-y += highbank.o -obj-y += pl061.o -obj-y += xgmac.o obj-y += pxa2xx.o pxa2xx_pic.o pxa2xx_gpio.o pxa2xx_timer.o pxa2xx_dma.o obj-y += pxa2xx_lcd.o pxa2xx_mmci.o pxa2xx_pcmcia.o pxa2xx_keypad.o obj-y += gumstix.o @@ -37,7 +29,6 @@ obj-y += strongarm.o obj-y += collie.o obj-y += imx_serial.o imx_ccm.o imx_timer.o imx_avic.o obj-y += kzm.o -obj-y += pl041.o lm4549.o obj-$(CONFIG_FDT) += ../device_tree.o obj-y := $(addprefix ../,$(obj-y)) -- 1.7.7
Re: [Qemu-devel] [PATCH for-1.2 v2] arm: Move some ARM devices into libhw
Am 13.08.2012 14:11, schrieb Andreas Färber: Avoids some unnecessary dependencies on cpu.h and prepares for a future armeb-softmmu where most machines would not be built. Defer touching the SoC devices since most have implicit or explicit dependencies on the CPU. Signed-off-by: Andreas Färber andreas.faer...@web.de Err sorry, should be SoB afaer...@suse.de. (Muphry's law!) /-F --- default-configs/arm-softmmu.mak | 18 ++ hw/Makefile.objs| 20 hw/arm/Makefile.objs| 15 +++ 3 files changed, 41 insertions(+), 12 deletions(-) v1 - v2: * Move new lm4549.o alongside pl041.o (PMM). * Defer moving stellaris_enet.o until stellaris.o is cleaned up (PMM). diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index e542b4f..f335a72 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -27,3 +27,21 @@ CONFIG_SMC91C111=y CONFIG_DS1338=y CONFIG_PFLASH_CFI01=y CONFIG_PFLASH_CFI02=y + +CONFIG_ARM_TIMER=y +CONFIG_PL011=y +CONFIG_PL022=y +CONFIG_PL031=y +CONFIG_PL041=y +CONFIG_PL050=y +CONFIG_PL061=y +CONFIG_PL080=y +CONFIG_PL110=y +CONFIG_PL181=y +CONFIG_PL190=y +CONFIG_PL310=y +CONFIG_CADENCE=y +CONFIG_XGMAC=y + +CONFIG_VERSATILE_PCI=y +CONFIG_VERSATILE_I2C=y diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 6eee9a0..7f57ed5 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -74,6 +74,26 @@ hw-obj-$(CONFIG_PUV3) += puv3_gpio.o hw-obj-$(CONFIG_PUV3) += puv3_pm.o hw-obj-$(CONFIG_PUV3) += puv3_dma.o +# ARM devices +hw-obj-$(CONFIG_ARM_TIMER) += arm_timer.o +hw-obj-$(CONFIG_PL011) += pl011.o +hw-obj-$(CONFIG_PL022) += pl022.o +hw-obj-$(CONFIG_PL031) += pl031.o +hw-obj-$(CONFIG_PL041) += pl041.o lm4549.o +hw-obj-$(CONFIG_PL050) += pl050.o +hw-obj-$(CONFIG_PL061) += pl061.o +hw-obj-$(CONFIG_PL080) += pl080.o +hw-obj-$(CONFIG_PL110) += pl110.o +hw-obj-$(CONFIG_PL181) += pl181.o +hw-obj-$(CONFIG_PL190) += pl190.o +hw-obj-$(CONFIG_PL310) += arm_l2x0.o +hw-obj-$(CONFIG_VERSATILE_PCI) += versatile_pci.o +hw-obj-$(CONFIG_VERSATILE_I2C) += versatile_i2c.o +hw-obj-$(CONFIG_CADENCE) += cadence_uart.o +hw-obj-$(CONFIG_CADENCE) += cadence_ttc.o +hw-obj-$(CONFIG_CADENCE) += cadence_gem.o +hw-obj-$(CONFIG_XGMAC) += xgmac.o + # PCI watchdog devices hw-obj-$(CONFIG_PCI) += wdt_i6300esb.o diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs index c413780..2b39fb3 100644 --- a/hw/arm/Makefile.objs +++ b/hw/arm/Makefile.objs @@ -1,10 +1,5 @@ -obj-y = integratorcp.o versatilepb.o arm_pic.o arm_timer.o -obj-y += arm_boot.o pl011.o pl031.o pl050.o pl080.o pl110.o pl181.o pl190.o -obj-y += versatile_pci.o -obj-y += versatile_i2c.o -obj-y += cadence_uart.o -obj-y += cadence_ttc.o -obj-y += cadence_gem.o +obj-y = integratorcp.o versatilepb.o arm_pic.o +obj-y += arm_boot.o obj-y += xilinx_zynq.o zynq_slcr.o obj-y += arm_gic.o arm_gic_common.o obj-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o @@ -12,12 +7,9 @@ obj-y += exynos4210_gic.o exynos4210_combiner.o exynos4210.o obj-y += exynos4_boards.o exynos4210_uart.o exynos4210_pwm.o obj-y += exynos4210_pmu.o exynos4210_mct.o exynos4210_fimd.o obj-y += exynos4210_rtc.o exynos4210_i2c.o -obj-y += arm_l2x0.o obj-y += arm_mptimer.o a15mpcore.o -obj-y += armv7m.o armv7m_nvic.o stellaris.o pl022.o stellaris_enet.o +obj-y += armv7m.o armv7m_nvic.o stellaris.o stellaris_enet.o obj-y += highbank.o -obj-y += pl061.o -obj-y += xgmac.o obj-y += pxa2xx.o pxa2xx_pic.o pxa2xx_gpio.o pxa2xx_timer.o pxa2xx_dma.o obj-y += pxa2xx_lcd.o pxa2xx_mmci.o pxa2xx_pcmcia.o pxa2xx_keypad.o obj-y += gumstix.o @@ -37,7 +29,6 @@ obj-y += strongarm.o obj-y += collie.o obj-y += imx_serial.o imx_ccm.o imx_timer.o imx_avic.o obj-y += kzm.o -obj-y += pl041.o lm4549.o obj-$(CONFIG_FDT) += ../device_tree.o obj-y := $(addprefix ../,$(obj-y)) -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH for-1.2 v5 00/14] pci_host: Convert to QOM
Am 09.08.2012 17:09, schrieb Andreas Färber: Am 02.08.2012 10:30, schrieb Michael S. Tsirkin: On Thu, Aug 02, 2012 at 03:46:52AM +0200, Andreas Färber wrote: Here's a fixed version of the series making pci_host a first-class QOM type. MAINTAINERS entries for the ppc devices touched herein are stripped from the series but being used for sending. They can be applied later through ppc-next. This series is a prerequisite for the i440fx refactoring and q35 introduction. I have verified this to apply cleanly to both master and pci branch now. ACK patches 1-13. Ping! I don't see these on the pci branch. Should they be picked up by Anthony directly? Ping^2! If you want a PULL, let me know. /-F -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
Am 09.08.2012 15:02, schrieb Bharata B Rao: block: Support GlusterFS as a QEMU block backend. From: Bharata B Rao bhar...@linux.vnet.ibm.com This patch adds gluster as the new block backend in QEMU. This gives QEMU the ability to boot VM images from gluster volumes. Its already possible to boot from VM images on gluster volumes using FUSE mount, but this patchset provides the ability to boot VM images from gluster volumes by by-passing the FUSE layer in gluster. This is made possible by using libgfapi routines to perform IO on gluster volumes directly. VM Image on gluster volume is specified like this: file=gluster://server:[port]/volname/image[?transport=socket] 'gluster' is the protocol. 'server' specifies the server where the volume file specification for the given volume resides. This can be either hostname or ipv4 address or ipv6 address. ipv6 address needs to be with in square brackets [ ]. port' is the port number on which gluster management daemon (glusterd) is listening. This is optional and if not specified, QEMU will send 0 which will make libgfapi to use the default port. 'volname' is the name of the gluster volume which contains the VM image. 'image' is the path to the actual VM image in the gluster volume. 'transport' specifies the transport used to connect to glusterd. This is optional and if not specified, socket transport is used. Examples: file=gluster://1.2.3.4/testvol/a.img file=gluster://1.2.3.4:5000/testvol/dir/a.img?transport=socket file=gluster://[1:2:3:4:5:6:7:8]/testvol/dir/a.img file=gluster://[1:2:3:4:5:6:7:8]:5000/testvol/dir/a.img?transport=socket file=gluster://server.domain.com:5000/testvol/dir/a.img Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com --- block/Makefile.objs |1 block/gluster.c | 623 +++ 2 files changed, 624 insertions(+), 0 deletions(-) create mode 100644 block/gluster.c diff --git a/block/Makefile.objs b/block/Makefile.objs index b5754d3..a1ae67f 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -9,3 +9,4 @@ block-obj-$(CONFIG_POSIX) += raw-posix.o block-obj-$(CONFIG_LIBISCSI) += iscsi.o block-obj-$(CONFIG_CURL) += curl.o block-obj-$(CONFIG_RBD) += rbd.o +block-obj-$(CONFIG_GLUSTERFS) += gluster.o diff --git a/block/gluster.c b/block/gluster.c new file mode 100644 index 000..bbbaea8 --- /dev/null +++ b/block/gluster.c @@ -0,0 +1,623 @@ +/* + * GlusterFS backend for QEMU + * + * (AIO implementation is derived from block/rbd.c) Most parts of block/rbd.c are GPLv2 only. You're also missing the copyright statements from there. + * + * Copyright (C) 2012 Bharata B Rao bhar...@linux.vnet.ibm.com + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * (at your option) any later version. See the COPYING file in the top-level + * directory. + */ +#include glusterfs/api/glfs.h +#include block_int.h + +typedef struct GlusterAIOCB { +BlockDriverAIOCB common; +bool canceled; +int64_t size; +int ret; +} GlusterAIOCB; + +typedef struct BDRVGlusterState { +struct glfs *glfs; +int fds[2]; +struct glfs_fd *fd; +int qemu_aio_count; +} BDRVGlusterState; + +#define GLUSTER_FD_READ 0 +#define GLUSTER_FD_WRITE 1 + +typedef struct GlusterURI { +char *server; +int port; +char *volname; +char *image; +char *transport; +} GlusterURI; + +static void qemu_gluster_uri_free(GlusterURI *uri) +{ +g_free(uri-server); +g_free(uri-volname); +g_free(uri-image); +g_free(uri-transport); +g_free(uri); +} + +/* + * We don't validate the transport option obtained here but + * instead depend on gluster to flag an error. + */ +static int parse_transport(GlusterURI *uri, char *transport) +{ +char *token, *saveptr; +int ret = -EINVAL; + +if (!transport) { +uri-transport = g_strdup(socket); +ret = 0; +goto out; There is no cleanup code after out:, so please use a direct return 0. +} + +token = strtok_r(transport, =, saveptr); +if (!token) { +goto out; +} +if (strcmp(token, transport)) { +goto out; +} if (!token || strcmp(...)) { return -EINVAL; } Many more unnecessary goto out follow, I won't comment on each single one. +token = strtok_r(NULL, =, saveptr); +if (!token) { +goto out; +} +uri-transport = g_strdup(token); +ret = 0; +out: +return ret; +} + +static int parse_server(GlusterURI *uri, char *server) +{ It looks to me as if there are two options: Either this functionality already exists and is used in the various places where qemu deals with network addresses. Then you should reuse that code. Or it actually isn't implemented properly in other places, then this should be added as a
Re: [Qemu-devel] TSC in qem[-kvm] 1.1+ and in-kernel irqchip
On 2012-08-12 11:24, Michael Tokarev wrote: On 12.08.2012 12:10, Gleb Natapov wrote: [] Any chance to bisect it? The bisecion leads to this commit: commit 17ee47418e65b1593defb30edbab33ccd47fc1f8 Merge: 13b0496 5d17c0d Author: Jan Kiszka jan.kis...@siemens.com Date: Tue Apr 10 16:26:23 2012 +0200 Merge commit '5d17c0d2df4998598e6002b27b8e47e792899a0f' into queues/qemu-merge Conflicts: hw/pc.c diff --cc Makefile.target index 33a7255,1bd25a8..32c8e42 --- a/Makefile.target +++ b/Makefile.target @@@ -245,13 -244,8 +245,13 @@@ obj-i386-y += pci-hotplug.o smbios.o wd obj-i386-y += debugcon.o multiboot.o obj-i386-y += pc_piix.o obj-i386-y += pc_sysfw.o - obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o kvm/ioapic.o + obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o kvm/ioapic.o kvm/i8254.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o +obj-i386-y += testdev.o +obj-i386-y += acpi.o acpi_piix4.o + +obj-i386-y += i8254_common.o i8254.o +obj-i386-$(CONFIG_KVM_DEVICE_ASSIGNMENT) += device-assignment.o # shared objects obj-ppc-y = ppc.o ppc_booke.o diff --cc hw/pc.c index 74c19b9,bb9867b..feb6ef3 --- a/hw/pc.c +++ b/hw/pc.c @@@ -1116,8 -1118,12 +1122,12 @@@ void pc_basic_device_init(ISABus *isa_b qemu_register_boot_set(pc_boot_set, *rtc_state); - pit = pit_init(isa_bus, 0x40, pit_isa_irq, pit_alt_irq); + if (kvm_irqchip_in_kernel()) { + pit = kvm_pit_init(isa_bus, 0x40); + } else { + pit = pit_init(isa_bus, 0x40, pit_isa_irq, pit_alt_irq); + } -if (hpet) { +if (hpet !(kvm_enabled() kvm_irqchip_in_kernel())) { /* connect PIT to output control line of the HPET */ qdev_connect_gpio_out(hpet, 0, qdev_get_gpio_in(pit-qdev, 0)); } Note this commit itself talks about pit and irqchip. But I don't know what does it mean. Cc'ing Jan for help. The short story: tsc timer calibration broke in 1.1+ with in-kernel irqchip (only) for several apps (seabios and grub are two examples), the time is ticking about 100 times faster. In grub the timer is calibrated using pit. The above commit is the result of bisection. Did the versions you tested include the commit 0cdd3d1444 (kvm: i8254: Fix conversion of in-kernel to userspace state)? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges
Andreas Färber afaer...@suse.de writes: Uglify the parent field to enforce QOM-style access via casts. Don't just typedef PCIHostState, either use it directly or embed it. Signed-off-by: Andreas Färber afaer...@suse.de --- hw/alpha_typhoon.c |2 +- hw/dec_pci.c |2 +- hw/grackle_pci.c |2 +- hw/gt64xxx.c | 26 -- hw/piix_pci.c |6 -- hw/ppc4xx_pci.c|8 +--- hw/ppce500_pci.c |2 +- hw/prep_pci.c |8 +--- hw/spapr_pci.c | 12 +++- hw/spapr_pci.h |2 +- hw/unin_pci.c | 14 +++--- 11 files changed, 49 insertions(+), 35 deletions(-) diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c index 7667412..b7cf4e2 100644 --- a/hw/alpha_typhoon.c +++ b/hw/alpha_typhoon.c @@ -46,7 +46,7 @@ typedef struct TyphoonPchip { OBJECT_CHECK(TyphoonState, (obj), TYPE_TYPHOON_PCI_HOST_BRIDGE) typedef struct TyphoonState { -PCIHostState host; +PCIHostState parent_obj; TyphoonCchip cchip; TyphoonPchip pchip; diff --git a/hw/dec_pci.c b/hw/dec_pci.c index de16361..c30ade3 100644 --- a/hw/dec_pci.c +++ b/hw/dec_pci.c @@ -43,7 +43,7 @@ #define DEC_21154(obj) OBJECT_CHECK(DECState, (obj), TYPE_DEC_21154) typedef struct DECState { -PCIHostState host_state; +PCIHostState parent_obj; } DECState; static int dec_map_irq(PCIDevice *pci_dev, int irq_num) diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c index 066f6e1..67da307 100644 --- a/hw/grackle_pci.c +++ b/hw/grackle_pci.c @@ -41,7 +41,7 @@ OBJECT_CHECK(GrackleState, (obj), TYPE_GRACKLE_PCI_HOST_BRIDGE) typedef struct GrackleState { -PCIHostState host_state; +PCIHostState parent_obj; MemoryRegion pci_mmio; MemoryRegion pci_hole; diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c index 857758e..e95e664 100644 --- a/hw/gt64xxx.c +++ b/hw/gt64xxx.c @@ -235,7 +235,7 @@ OBJECT_CHECK(GT64120State, (obj), TYPE_GT64120_PCI_HOST_BRIDGE) typedef struct GT64120State { -PCIHostState pci; +PCIHostState parent_obj; uint32_t regs[GT_REGS]; PCI_MAPPING_ENTRY(PCI0IO); @@ -315,6 +315,7 @@ static void gt64120_writel (void *opaque, target_phys_addr_t addr, uint64_t val, unsigned size) { GT64120State *s = opaque; +PCIHostState *phb = PCI_HOST_BRIDGE(s); uint32_t saddr; if (!(s-regs[GT_CPU] 0x1000)) @@ -535,13 +536,15 @@ static void gt64120_writel (void *opaque, target_phys_addr_t addr, /* not implemented */ break; case GT_PCI0_CFGADDR: -s-pci.config_reg = val 0x80fc; +phb-config_reg = val 0x80fc; break; case GT_PCI0_CFGDATA: -if (!(s-regs[GT_PCI0_CMD] 1) (s-pci.config_reg 0x00fff800)) +if (!(s-regs[GT_PCI0_CMD] 1) (phb-config_reg 0x00fff800)) { val = bswap32(val); -if (s-pci.config_reg (1u 31)) -pci_data_write(s-pci.bus, s-pci.config_reg, val, 4); +} +if (phb-config_reg (1u 31)) { +pci_data_write(phb-bus, phb-config_reg, val, 4); +} break; /* Interrupts */ @@ -594,6 +597,7 @@ static uint64_t gt64120_readl (void *opaque, target_phys_addr_t addr, unsigned size) { GT64120State *s = opaque; +PCIHostState *phb = PCI_HOST_BRIDGE(s); uint32_t val; uint32_t saddr; @@ -775,15 +779,17 @@ static uint64_t gt64120_readl (void *opaque, /* PCI Internal */ case GT_PCI0_CFGADDR: -val = s-pci.config_reg; +val = phb-config_reg; break; case GT_PCI0_CFGDATA: -if (!(s-pci.config_reg (1 31))) +if (!(phb-config_reg (1 31))) { val = 0x; -else -val = pci_data_read(s-pci.bus, s-pci.config_reg, 4); -if (!(s-regs[GT_PCI0_CMD] 1) (s-pci.config_reg 0x00fff800)) +} else { +val = pci_data_read(phb-bus, phb-config_reg, 4); +} +if (!(s-regs[GT_PCI0_CMD] 1) (phb-config_reg 0x00fff800)) { val = bswap32(val); +} break; case GT_PCI0_CMD: diff --git a/hw/piix_pci.c b/hw/piix_pci.c index 04ceccf..537fc19 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -36,7 +36,9 @@ * http://download.intel.com/design/chipsets/datashts/29054901.pdf */ -typedef PCIHostState I440FXState; +typedef struct I440FXState { +PCIHostState parent_obj; +} I440FXState; #define PIIX_NUM_PIC_IRQS 16 /* i8259 * 2 */ #define PIIX_NUM_PIRQS 4ULL/* PIRQ[A-D] */ @@ -274,7 +276,7 @@ static PCIBus *i440fx_common_init(const char *device_name, dev = qdev_create(NULL, i440FX-pcihost); s = PCI_HOST_BRIDGE(dev); s-address_space = address_space_mem; -b = pci_bus_new(s-busdev.qdev, NULL,
Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges
On Thu, Aug 02, 2012 at 03:47:06AM +0200, Andreas Färber wrote: Uglify the parent field to enforce QOM-style access via casts. Don't just typedef PCIHostState, either use it directly or embed it. Signed-off-by: Andreas Färber afaer...@suse.de IMHO only one chunk from this patch should be applied (below). Below it is split out but needs to be rebased on top of patches 1-13. -- From: Andreas Färber andreas.faer...@web.de piix: minor code simplification There's no need to deal with qdev internals in piix - we get device state from qdev_create so just use that. Signed-off-by: Andreas Färber andreas.faer...@web.de Signed-off-by: Michael S. Tsirkin m...@redhat.com --- diff --git a/hw/piix_pci.c b/hw/piix_pci.c index c497a01..18554a6 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -274,7 +274,7 @@ static PCIBus *i440fx_common_init(const char *device_name, dev = qdev_create(NULL, i440FX-pcihost); s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev)); s-address_space = address_space_mem; -b = pci_bus_new(s-busdev.qdev, NULL, pci_address_space, +b = pci_bus_new(dev, NULL, pci_address_space, address_space_io, 0); s-bus = b; object_property_add_child(qdev_get_machine(), i440fx, OBJECT(dev), NULL);
Re: [Qemu-devel] TSC in qem[-kvm] 1.1+ and in-kernel irqchip
On 13.08.2012 17:07, Jan Kiszka wrote: [] The bisecion leads to this commit: commit 17ee47418e65b1593defb30edbab33ccd47fc1f8 Merge: 13b0496 5d17c0d Author: Jan Kiszka jan.kis...@siemens.com Date: Tue Apr 10 16:26:23 2012 +0200 Merge commit '5d17c0d2df4998598e6002b27b8e47e792899a0f' into queues/qemu-merge [] Cc'ing Jan for help. The short story: tsc timer calibration broke in 1.1+ with in-kernel irqchip (only) for several apps (seabios and grub are two examples), the time is ticking about 100 times faster. In grub the timer is calibrated using pit. The above commit is the result of bisection. Did the versions you tested include the commit 0cdd3d1444 (kvm: i8254: Fix conversion of in-kernel to userspace state)? While bisecting I didn't have this commit applied, since it were applied past (qemu)-1.1. It is included into qemu-kvm 1.1.0 (as 960d355dc60d9), and that version behaves _exactly_ the same - the time in grub is ticking 100 times faster. I mentioned in this thread that the problem persists in current qemu (and qemu-kvm) git too. I can repeat the bisection with this commit applied after the above bad commit. Should I? Thanks, /mjt
Re: [Qemu-devel] [PATCH] block: Set cdrom device read only flag
Am 13.08.2012 13:57, schrieb Markus Armbruster: Kevin Wolf kw...@redhat.com writes: Am 12.08.2012 04:48, schrieb Kevin Shanahan: So qmp_change_blockdev uses bdrv_is_read_only() to check whether to try and open the backing file read only, which uses the -read_only member of struct BlockDriverState to decide whether to pass the BDRV_O_RDRW flag to qmp_bdrv_open_encypted() and then bdrv_open(). I would assume we want to set this flag in drive_init() when the block driver state is initialised. How about a patch like this instead? diff --git a/blockdev.c b/blockdev.c index 8669142..ba22064 100644 --- a/blockdev.c +++ b/blockdev.c @@ -526,6 +526,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) if_name[type], mediastr, unit_id); } dinfo-bdrv = bdrv_new(dinfo-id); +dinfo-bdrv-read_only = ro; dinfo-devaddr = devaddr; dinfo-type = type; dinfo-bus = bus_id; Ah, yes, this looks much more like the proper fix. Basically we need to set everything that is retained after a 'change' command. We have this code in qmp_change_blockdev(): bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR; bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0; qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp); bdrv_is_read_only() is covered by your patch, bdrv_is_snapshot() additionally requires bs-open_flags to be right. Markus, how will this look in the -blockdev world? There seem to be properties that belong to host state, but are not coupled to a medium. Really? Read-only is clearly a property of the medium. There's a separate read-only belonging to the device. A CD-ROM medium is read-only, even when loaded in an optical disk drive that can burn. An optical disk drive that can't burn is read-only, even when writable medium is loaded. Here's how I believe it should work: * BDS member read_only describes the medium. * block.h gets it right: you specify read-only with bdrv_open(), not with bdrv_new(). * -drive gets it right: parameter readonly gets ignored unless we're defining media. Not nice: it's ignored silently. * Monitor command change gets it wrong: it doesn't let you specify the new medium's read-only-ness. Easy enough to fix for QMP, just add a suitable argument. Even better: create a new, non-multiplexed command, and let change rot in peace. Not sure about the best way to fix it in the human monitor. I think it's mostly the human monitor that needs this state. Of course, users will always want to have it, but in the case of QMP this work is offloaded to libvirt. So people expect that if they have started with a block device read-only, it remains read-only after they switch to a different image file (change the medium). They also expect that if they initially started with snapshot=on, the newly loaded image file will inherit this flag and be snapshotted as well. We know these expectations because 'change' didn't always retain these properties (or probably did so initially, but was broken at some point), and people complained, so it was fixed. So we can't just break it. This works today. Kevin's bug is about retaining the properties also when initially there's no medium in the drive, and it would probably be reasonable enough to say that we shouldn't lose them either when at runtime a medium is first ejected so that the drive is empty, and then a new medium is inserted. As always ignoring requirements is the easiest way, but it doesn't make users happy. * Device model has its own read-only predicate. * If a device model comes in both a read-only and a read-write flavor, it should have a bool property readonly. * A device model can only use a backend with a suitable media state (has media, is read-only, ...). For instance, ide-hd can't use a read-only backend. * Media change disabled while backend is attached to a device model with fixed media. * Convenient defaults (optional) * A device model property readonly could default to the media's read-only-ness. * Monitor command change could default to readonly when the backend is attached to a device model that can't write. * Both -drive and change could default to readonly when the image isn't writable. This doesn't give us today's behaviour with devices that can do both read-only and r/w media. Most common case for it are probably CD-ROMs which can never write, so it comes relatively close at least (at least until someone implements burning CDs in our emulation...) It doesn't do anything about snapshot=on. Kevin
Re: [Qemu-devel] [PATCH 0/3] VFIO-based PCI device assignment for QEMU 1.2
Alex Williamson alex.william...@redhat.com writes: VFIO kernel support was just merged into Linux, so I'd like to formally propose inclusion of the QEMU vfio-pci driver for QEMU 1.2. Included here is support for x86 PCI device assignment. PCI INTx is not yet enabled, but devices making use of either MSI or MSI-X work. The level irqfd and eoifd support I've proposed for KVM enable an accelerated patch for this through KVM. I'd like to get this base driver in first and enable the remaining support in-tree. I've split this version up a little from the RFC to make it a bit easier to review. Review comments from Blue Swirl and Avi are already incorporated, including Avi's requests to simplify both the PCI BAR mapping and unmapping paths. Hi Alex, Thanks for pushing this forward! Hopefully this will finally kill off qemu-kvm.git for good. I think this series is going to have to wait for 1.3 to open up. We have a very short release window for this release and I'd feel a lot more comfortable having such a significant feature spend some time in the development cycle getting testing/review. I'd like to see a few Reviewed-by's too for this series before it goes in. I expect they won't be hard to get but I also expect it will take a few more revisions of this series to get there. Regards, Anthony Liguori This series is also available at: git://github.com/awilliam/qemu-vfio.git tags/vfio-pci-for-qemu-1.2 Thanks, Alex --- Alex Williamson (3): vfio: Enable vfio-pci and mark supported vfio: vfio-pci device assignment driver vfio: Import vfio kernel header MAINTAINERS|5 configure | 12 hw/i386/Makefile.objs |1 hw/vfio_pci.c | 1853 hw/vfio_pci.h | 101 ++ linux-headers/linux/vfio.h | 368 + 6 files changed, 2340 insertions(+) create mode 100644 hw/vfio_pci.c create mode 100644 hw/vfio_pci.h create mode 100644 linux-headers/linux/vfio.h -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 00/35]: add new error format
On Sat, 11 Aug 2012 09:05:33 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: [...] This series implements the 'Plan for error handling in QMP' as described by Anthony in this email: http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg03764.html Basically, this replaces almost all error classes by GenericError (the exception are a few error classes used by libvirt) and drops the error data memeber. This also adds a free form string to error_set(). On the wire, we go from: { error: { class: DeviceNotRemovable, data: { device: virtio0 }, desc: Device 'virtio0' is not removable } } to: { error: { class: GenericError, desc: Device 'virtio0' is not removable } } Internally, we go from: void error_set(Error **err, const char *fmt, ...); to: void error_set(Error **err, ErrorClass err_class, const char *fmt, ...); Glad to see this change in good shape in time for the release. Thanks, Luiz! Thank you for your review! Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges
Am 13.08.2012 15:14, schrieb Anthony Liguori: Andreas Färber afaer...@suse.de writes: diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c index df70cd2..8937030 100644 --- a/hw/spapr_pci.c +++ b/hw/spapr_pci.c @@ -36,16 +36,18 @@ static PCIDevice *find_dev(sPAPREnvironment *spapr, uint64_t buid, uint32_t config_addr) { int devfn = (config_addr 8) 0xFF; -sPAPRPHBState *phb; +sPAPRPHBState *sphb; -QLIST_FOREACH(phb, spapr-phbs, list) { +QLIST_FOREACH(sphb, spapr-phbs, list) { +PCIHostState *phb; BusChild *kid; -if (phb-buid != buid) { +if (sphb-buid != buid) { continue; } -QTAILQ_FOREACH(kid, phb-host_state.bus-qbus.children, sibling) { +phb = PCI_HOST_BRIDGE(sphb); +QTAILQ_FOREACH(kid, BUS(phb-bus)-children, sibling) { PCIDevice *dev = (PCIDevice *)kid-child; if (dev-devfn == devfn) { return dev; @@ -319,7 +321,7 @@ static int spapr_phb_init(SysBusDevice *s) pci_spapr_set_irq, pci_spapr_map_irq, phb, phb-memspace, phb-iospace, PCI_DEVFN(0, 0), PCI_NUM_PINS); -phb-host_state.bus = bus; +PCI_HOST_BRIDGE(phb)-bus = bus; I think you meant: PCI_HOST_BRIDGE(sphb)-bus But really you meant: phb-bus = bus; The patch is misleading here, in the initfn phb historically is sPAPRPHBState, not PCIHostState. But you are right that this inline macro usage should be fixed - will solve by squashing phb - sphb renaming into the spapr_pci patch. Thanks, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 11/34] qmp: query-block: add 'valid_encryption_key' field
On Sat, 11 Aug 2012 09:45:14 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 10 Aug 2012 19:17:22 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 10 Aug 2012 18:35:26 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 10 Aug 2012 09:56:11 +0200 Markus Armbruster arm...@redhat.com wrote: Revisited this one on review of v2, replying here for context. Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 02 Aug 2012 13:35:54 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- block.c | 1 + qapi-schema.json | 7 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index b38940b..9c113b8 100644 --- a/block.c +++ b/block.c @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp) info-value-inserted-ro = bs-read_only; info-value-inserted-drv = g_strdup(bs-drv-format_name); info-value-inserted-encrypted = bs-encrypted; +info-value-inserted-valid_encryption_key = bs-valid_key; if (bs-backing_file[0]) { info-value-inserted-has_backing_file = true; info-value-inserted-backing_file = g_strdup(bs-backing_file); diff --git a/qapi-schema.json b/qapi-schema.json index bc55ed2..1b2d7f5 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -400,6 +400,8 @@ # # @encrypted: true if the backing device is encrypted # +# @valid_encryption_key: true if a valid encryption key has been set +# # @bps: total throughput limit in bytes per second is specified # # @bps_rd: read throughput limit in bytes per second is specified @@ -419,8 +421,9 @@ { 'type': 'BlockDeviceInfo', 'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str', '*backing_file': 'str', 'encrypted': 'bool', -'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int', -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} } +'valid_encryption_key': 'bool', 'bps': 'int', +'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int', +'iops_rd': 'int', 'iops_wr': 'int'} } ## # @BlockDeviceIoStatus: BlockDeviceInfo is API, isn't it? Yes. Note that bs-valid_key currently implies bs-encrypted. bs-valid_key !bs-encrypted is impossible. Should we make valid_encryption_key only available when encrypted? I don't think so. It's a bool, so it's ok for it to be false when encrypted is false. What bothers me is encrypted=false, valid_encryption_key=true. Disappearing keys is worse, IMHO (assuming that that situation is impossible in practice, of course). It's fundamentally three states: unencrypted, encrypted-no-key, encrypted-got-key. I'm fine with mapping these onto two bools, it's how the block layer does it. You may want to consider a single enumeration instead. That's arguable. But I like the bools slightly better because they allow clients to do a true/false check vs. having to check against an enum value. Again, that's arguable. valid_encryption_key is a bit long for my taste. Yours may be different. We should choose more descriptive and self-documenting names for the protocol. Besides, I can't think of anything shorter that won't get cryptic. Suggestions are always welcome though :) valid_encryption_key sounds like the value is the valid key. That's exactly what it is. Err, isn't the value bool? The key value is a string... Ah, sorry, I read sounds like true means the key is valid even for an invalid key. I've renamed it to encryption_key_missing, should be better (although I could also do encryption_key_is_missing). got_crypt_key? Also avoids valid. Good, because current encrypted formats don't actually validate the key; they happily accept any key. That's a block layer bug, not QMP's. QMP clients are going to be misguided by valid_encryption_key the same way they are with the block_passwd command or how we suffer from it internally when calling bdrv_set_key() (which also manifests itself in HMP). Fixing the bug where it is will automatically fix all its instances. It's not fixable for existing image formats, and thus existing images. Why
Re: [Qemu-devel] [PATCH] usb: selective endpoint initialization
On August 9, 2012 at 10:59 AM Gerd Hoffmann kra...@redhat.com wrote: Hi Gerd, sorry for the delays, I tested the latest pulled patch queue and it's now fine on my Intel board, too. The dongle gets detected again without assertions. Thanks for your work. Still remaining are the multiple usb resets on host side before the dongle gets finally detected / usable on the guest. Can you try if the attached patch makes a difference? thanks, Gerd Hi Gerd, sorry, no difference. Still the same behavior. Best regards, Erik
Re: [Qemu-devel] Funny -m arguments can crash
Avi Kivity a...@redhat.com writes: On 08/08/2012 12:04 PM, Markus Armbruster wrote: Yes please, maybe with a notice to the user. Next problem: minimum RAM size. For instance, -M pc -m X, where X 32KiB dies qemu: fatal: Trying to execute code outside RAM or ROM at [...] Aborted (core dumped) with TCG, and KVM internal error. Suberror: 1 with KVM. Should a minimum RAM size be enforced? Board-specific? It's really a BIOS bug causing a limitation of both kvm and tcg to be hit. The BIOS should recognize it doesn't have sufficient memory and hang gracefully (if you can picture that). It just assumes some low memory is available and tries to execute it with the results you got. SeaBIOS indeed assumes it got at least 1MiB of RAM. It doesn't bother to check CMOS for a smaller RAM size. However, that bug / feature is currently masked by a QEMU bug: we screw up CMOS contents when there's less than 1 MiB of RAM. pc_cmos_init(): int val, nb, i; [...] /* memory size */ val = 640; /* base memory in K */ rtc_set_memory(s, 0x15, val); rtc_set_memory(s, 0x16, val 8); val = (ram_size / 1024) - 1024; if (val 65535) val = 65535; rtc_set_memory(s, 0x17, val); rtc_set_memory(s, 0x18, val 8); If ram_size 1MiB, val goes negative. Oops. For instance, with -m 500k, we happily promise 640KiB base memory (CMOS addr 0x15..16), almost 64MiB extended memory (0x17..18 and 0x30..31), yet no memory above 16MiB (0x34..35). An easy way to fix this is to require 1MiB of RAM :) But if you like, I'll put sane values in CMOS instead. That'll expose the SeaBIOS bug. Anthony, you're the PC maintainer, got a preference? SeaBIOS thread: http://comments.gmane.org/gmane.comp.bios.coreboot.seabios/4341
Re: [Qemu-devel] TSC in qem[-kvm] 1.1+ and in-kernel irqchip
On 2012-08-13 15:16, Michael Tokarev wrote: On 13.08.2012 17:07, Jan Kiszka wrote: [] The bisecion leads to this commit: commit 17ee47418e65b1593defb30edbab33ccd47fc1f8 Merge: 13b0496 5d17c0d Author: Jan Kiszka jan.kis...@siemens.com Date: Tue Apr 10 16:26:23 2012 +0200 Merge commit '5d17c0d2df4998598e6002b27b8e47e792899a0f' into queues/qemu-merge [] Cc'ing Jan for help. The short story: tsc timer calibration broke in 1.1+ with in-kernel irqchip (only) for several apps (seabios and grub are two examples), the time is ticking about 100 times faster. In grub the timer is calibrated using pit. The above commit is the result of bisection. Did the versions you tested include the commit 0cdd3d1444 (kvm: i8254: Fix conversion of in-kernel to userspace state)? While bisecting I didn't have this commit applied, since it were applied past (qemu)-1.1. It is included into qemu-kvm 1.1.0 (as 960d355dc60d9), and that version behaves _exactly_ the same - the time in grub is ticking 100 times faster. I mentioned in this thread that the problem persists in current qemu (and qemu-kvm) git too. I can repeat the bisection with this commit applied after the above bad commit. Should I? Don't think this will make a difference. There is some other issue. I reproduced the bug, will see if I can analyze it. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v9 5/7] block: Convert close calls to qemu_close
On 08/11/2012 09:22 AM, Blue Swirl wrote: On Sat, Aug 11, 2012 at 1:14 PM, Corey Bryant cor...@linux.vnet.ibm.com wrote: This patch converts all block layer close calls, that correspond to qemu_open calls, to qemu_close. Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- v5: -This patch is new in v5. (kw...@redhat.com, ebl...@redhat.com) v6-v9: -No changes block/raw-posix.c | 24 block/raw-win32.c |2 +- block/vmdk.c |4 ++-- block/vpc.c |2 +- block/vvfat.c | 12 ++-- osdep.c |5 + qemu-common.h |1 + savevm.c |4 ++-- 8 files changed, 30 insertions(+), 24 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 08b997e..6be20b1 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -271,7 +271,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, out_free_buf: qemu_vfree(s-aligned_buf); out_close: -close(fd); +qemu_close(fd); return -errno; } @@ -376,7 +376,7 @@ static void raw_close(BlockDriverState *bs) { BDRVRawState *s = bs-opaque; if (s-fd = 0) { -close(s-fd); +qemu_close(s-fd); s-fd = -1; if (s-aligned_buf != NULL) qemu_vfree(s-aligned_buf); @@ -580,7 +580,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) if (ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) { result = -errno; } -if (close(fd) != 0) { +if (qemu_close(fd) != 0) { result = -errno; } } @@ -850,7 +850,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags) if (fd 0) { bsdPath[strlen(bsdPath)-1] = '1'; } else { -close(fd); +qemu_close(fd); } filename = bsdPath; } @@ -889,7 +889,7 @@ static int fd_open(BlockDriverState *bs) last_media_present = (s-fd = 0); if (s-fd = 0 (get_clock() - s-fd_open_time) = FD_OPEN_TIMEOUT) { -close(s-fd); +qemu_close(s-fd); s-fd = -1; #ifdef DEBUG_FLOPPY printf(Floppy closed\n); @@ -988,7 +988,7 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options) else if (lseek(fd, 0, SEEK_END) total_size * BDRV_SECTOR_SIZE) ret = -ENOSPC; -close(fd); +qemu_close(fd); return ret; } @@ -1038,7 +1038,7 @@ static int floppy_open(BlockDriverState *bs, const char *filename, int flags) return ret; /* close fd so that we can reopen it as needed */ -close(s-fd); +qemu_close(s-fd); s-fd = -1; s-fd_media_changed = 1; @@ -1072,7 +1072,7 @@ static int floppy_probe_device(const char *filename) prio = 100; outc: -close(fd); +qemu_close(fd); out: return prio; } @@ -1107,14 +1107,14 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag) int fd; if (s-fd = 0) { -close(s-fd); +qemu_close(s-fd); s-fd = -1; } fd = qemu_open(bs-filename, s-open_flags | O_NONBLOCK); if (fd = 0) { if (ioctl(fd, FDEJECT, 0) 0) perror(FDEJECT); -close(fd); +qemu_close(fd); } } @@ -1175,7 +1175,7 @@ static int cdrom_probe_device(const char *filename) prio = 100; outc: -close(fd); +qemu_close(fd); out: return prio; } @@ -1283,7 +1283,7 @@ static int cdrom_reopen(BlockDriverState *bs) * FreeBSD seems to not notice sometimes... */ if (s-fd = 0) -close(s-fd); +qemu_close(s-fd); fd = qemu_open(bs-filename, s-open_flags, 0644); if (fd 0) { s-fd = -1; diff --git a/block/raw-win32.c b/block/raw-win32.c index 8d7838d..c56bf83 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -261,7 +261,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) return -EIO; set_sparse(fd); ftruncate(fd, total_size * 512); -close(fd); +qemu_close(fd); return 0; } diff --git a/block/vmdk.c b/block/vmdk.c index 557dc1b..daee426 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1258,7 +1258,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, ret = 0; exit: -close(fd); +qemu_close(fd); return ret; } @@ -1506,7 +1506,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) } ret = 0; exit: -close(fd); +qemu_close(fd); return ret; } diff --git a/block/vpc.c b/block/vpc.c index 60ebf5a..c0b82c4 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -744,7 +744,7 @@ static int vpc_create(const char *filename, QEMUOptionParameter *options) } fail: -close(fd); +qemu_close(fd); return ret; } diff --git
Re: [Qemu-devel] [PATCH v9 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets
I'll send a new version shortly with these updates. -- Regards, Corey On 08/11/2012 10:16 AM, Eric Blake wrote: On 08/11/2012 07:14 AM, Corey Bryant wrote: This patch adds support that enables passing of file descriptors to the QEMU monitor where they will be stored in specified file descriptor sets. v9: -Use fdset-id rather than fdset_id. (ebl...@redhat.com) -Update example for query-fdsets. (ebl...@redhat.com) -Close fd immediately on remove-fd. (kw...@redhat.com, ebl...@redhat.com) -Drop fdset refcount, and check dup_fds instead (in a later patch). (ebl...@redhat.com) -Move mon_refcount code to a later patch. (kw...@redhat.com) +AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque, + const char *opaque, Error **errp) +{ +int fd; +Monitor *mon = cur_mon; +MonFdset *mon_fdset; +MonFdsetFd *mon_fdset_fd; +AddfdInfo *fdinfo; + +fd = qemu_chr_fe_get_msgfd(mon-chr); +if (fd == -1) { +error_set(errp, QERR_FD_NOT_SUPPLIED); +goto error; +} + +if (has_fdset_id) { +QLIST_FOREACH(mon_fdset, mon_fdsets, next) { +if (mon_fdset-id == fdset_id) { +break; +} +} +if (mon_fdset == NULL) { +error_set(errp, QERR_INVALID_PARAMETER_VALUE, fdset-id, + an existing fdset-id or no fdset-id); The 'no fdset-id' portion of this error message doesn't make sense - it can only be reached if has_fdset_id was true. + +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) +{ +MonFdset *mon_fdset; +MonFdsetFd *mon_fdset_fd; +char fd_str[60]; + +QLIST_FOREACH(mon_fdset, mon_fdsets, next) { ... +} + +error: +snprintf(fd_str, sizeof(fd_str), + fdset-id:% PRId64 , fd:% PRId64, fdset_id, fd); Oops - fd is uninitialized if has_fd is false and the outer loop failed to find fdset_id. You need two separate error messages here, based on whether fd was provided. +- { execute: query-fdsets } +- { return: [ + { + fds: [ + { + fd: 30, + opaque: rdonly:/path/to/file + }, + { + fd: 24, + opaque: rdwr:/path/to/file + } + ], + fdset-id: 1 + }, + { + fds: [ + { + fd: 28 + }, + { + fd: 29 + } + ], + fdset-id: 0 + }, No trailing comma here.
Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols
Peter Maydell peter.mayd...@linaro.org writes: On 10 August 2012 17:04, Anthony Liguori aligu...@us.ibm.com wrote: This lets us provide a default implementation of a symbol which targets can override. Signed-off-by: Anthony Liguori aligu...@us.ibm.com I'm sure you'll be thrilled to hear that this doesn't seem to break MacOS builds :-) Thank you for testing it. I neglected to mention that I did a fair bit of investigation before hand and was able to confirm that all platforms we care about (Windows, Linux/Unix, OS X) and all compilers we care about (GCC, LLVM) support weak symbols. There may be different attribute names across compilers--it wasn't very clear to me, but they definitely all have the feature in some form. Regards, Anthony Liguori -- PMM
Re: [Qemu-devel] [PATCH v9 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets
I'll send a new version shortly with these updates. -- Regards, Corey On 08/11/2012 10:16 AM, Eric Blake wrote: On 08/11/2012 07:14 AM, Corey Bryant wrote: This patch adds support that enables passing of file descriptors to the QEMU monitor where they will be stored in specified file descriptor sets. v9: -Use fdset-id rather than fdset_id. (ebl...@redhat.com) -Update example for query-fdsets. (ebl...@redhat.com) -Close fd immediately on remove-fd. (kw...@redhat.com, ebl...@redhat.com) -Drop fdset refcount, and check dup_fds instead (in a later patch). (ebl...@redhat.com) -Move mon_refcount code to a later patch. (kw...@redhat.com) +AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque, + const char *opaque, Error **errp) +{ +int fd; +Monitor *mon = cur_mon; +MonFdset *mon_fdset; +MonFdsetFd *mon_fdset_fd; +AddfdInfo *fdinfo; + +fd = qemu_chr_fe_get_msgfd(mon-chr); +if (fd == -1) { +error_set(errp, QERR_FD_NOT_SUPPLIED); +goto error; +} + +if (has_fdset_id) { +QLIST_FOREACH(mon_fdset, mon_fdsets, next) { +if (mon_fdset-id == fdset_id) { +break; +} +} +if (mon_fdset == NULL) { +error_set(errp, QERR_INVALID_PARAMETER_VALUE, fdset-id, + an existing fdset-id or no fdset-id); The 'no fdset-id' portion of this error message doesn't make sense - it can only be reached if has_fdset_id was true. + +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp) +{ +MonFdset *mon_fdset; +MonFdsetFd *mon_fdset_fd; +char fd_str[60]; + +QLIST_FOREACH(mon_fdset, mon_fdsets, next) { ... +} + +error: +snprintf(fd_str, sizeof(fd_str), + fdset-id:% PRId64 , fd:% PRId64, fdset_id, fd); Oops - fd is uninitialized if has_fd is false and the outer loop failed to find fdset_id. You need two separate error messages here, based on whether fd was provided. +- { execute: query-fdsets } +- { return: [ + { + fds: [ + { + fd: 30, + opaque: rdonly:/path/to/file + }, + { + fd: 24, + opaque: rdwr:/path/to/file + } + ], + fdset-id: 1 + }, + { + fds: [ + { + fd: 28 + }, + { + fd: 29 + } + ], + fdset-id: 0 + }, No trailing comma here.
Re: [Qemu-devel] [PATCH v9 6/7] block: Enable qemu_open/close to work with fd sets
I'll send a new version shortly with these updates also. -- Regards, Corey On 08/11/2012 10:28 AM, Eric Blake wrote: On 08/11/2012 07:14 AM, Corey Bryant wrote: When qemu_open is passed a filename of the /dev/fdset/nnn format (where nnn is the fdset ID), an fd with matching access mode flags will be searched for within the specified monitor fd set. If the fd is found, a dup of the fd will be returned from qemu_open. v9: -Drop fdset refcount and check dup_fds instead. (ebl...@redhat.com) -Fix dupfd leak in qemu_dup(). (ebl...@redhat.com) -Always set O_CLOEXEC in qemu_dup(). (kw...@redhat.com) -Change name of qemu_dup() to qemu_dup_flags(). (kw...@redhat.com) @@ -87,6 +146,40 @@ int qemu_open(const char *name, int flags, ...) int ret; int mode = 0; +#ifndef _WIN32 +const char *fdset_id_str; + +/* Attempt dup of fd from fd set */ +if (strstart(name, /dev/fdset/, fdset_id_str)) { +int64_t fdset_id; +int fd, dupfd; + +fdset_id = qemu_parse_fdset(fdset_id_str); +if (fdset_id == -1) { +errno = EINVAL; +return -1; +} + +fd = monitor_fdset_get_fd(fdset_id, flags); +if (fd == -1) { +return -1; +} + +dupfd = qemu_dup_flags(fd, flags); +if (fd == -1) { Checking the wrong condition: s/fd/dupfd/ +return -1; +} + +ret = monitor_fdset_dup_fd_add(fdset_id, dupfd); +if (ret == -1) { +close(dupfd); +return -1; This function appears to promise a reasonable errno on failure. However, I don't think monitor_fdset_dup_fd_add guarantees a reasonable errno, and even if it does, close() can corrupt errno. I think that prior to returning here, you either need an explicit errno=ENOMEM, or fix monitor_fdset_dup_fd to guarantee a nice errno, plus a save and restore of errno here. Unless no one cares about errno on failure, in which case your earlier errno=EINVAL can be dropped. -- Regards, Corey
Re: [Qemu-devel] [PATCH 11/34] qmp: query-block: add 'valid_encryption_key' field
Luiz Capitulino lcapitul...@redhat.com writes: On Sat, 11 Aug 2012 09:45:14 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 10 Aug 2012 19:17:22 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 10 Aug 2012 18:35:26 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 10 Aug 2012 09:56:11 +0200 Markus Armbruster arm...@redhat.com wrote: Revisited this one on review of v2, replying here for context. Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 02 Aug 2012 13:35:54 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- block.c | 1 + qapi-schema.json | 7 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index b38940b..9c113b8 100644 --- a/block.c +++ b/block.c @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp) info-value-inserted-ro = bs-read_only; info-value-inserted-drv = g_strdup(bs-drv-format_name); info-value-inserted-encrypted = bs-encrypted; +info-value-inserted-valid_encryption_key = bs-valid_key; if (bs-backing_file[0]) { info-value-inserted-has_backing_file = true; info-value-inserted-backing_file = g_strdup(bs-backing_file); diff --git a/qapi-schema.json b/qapi-schema.json index bc55ed2..1b2d7f5 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -400,6 +400,8 @@ # # @encrypted: true if the backing device is encrypted # +# @valid_encryption_key: true if a valid encryption key has been set +# # @bps: total throughput limit in bytes per second is specified # # @bps_rd: read throughput limit in bytes per second is specified @@ -419,8 +421,9 @@ { 'type': 'BlockDeviceInfo', 'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str', '*backing_file': 'str', 'encrypted': 'bool', -'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int', -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} } +'valid_encryption_key': 'bool', 'bps': 'int', +'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int', +'iops_rd': 'int', 'iops_wr': 'int'} } ## # @BlockDeviceIoStatus: BlockDeviceInfo is API, isn't it? Yes. Note that bs-valid_key currently implies bs-encrypted. bs-valid_key !bs-encrypted is impossible. Should we make valid_encryption_key only available when encrypted? I don't think so. It's a bool, so it's ok for it to be false when encrypted is false. What bothers me is encrypted=false, valid_encryption_key=true. Disappearing keys is worse, IMHO (assuming that that situation is impossible in practice, of course). It's fundamentally three states: unencrypted, encrypted-no-key, encrypted-got-key. I'm fine with mapping these onto two bools, it's how the block layer does it. You may want to consider a single enumeration instead. That's arguable. But I like the bools slightly better because they allow clients to do a true/false check vs. having to check against an enum value. Again, that's arguable. valid_encryption_key is a bit long for my taste. Yours may be different. We should choose more descriptive and self-documenting names for the protocol. Besides, I can't think of anything shorter that won't get cryptic. Suggestions are always welcome though :) valid_encryption_key sounds like the value is the valid key. That's exactly what it is. Err, isn't the value bool? The key value is a string... Ah, sorry, I read sounds like true means the key is valid even for an invalid key. I've renamed it to encryption_key_missing, should be better (although I could also do encryption_key_is_missing). got_crypt_key? Also avoids valid. Good, because current encrypted formats don't actually validate the key; they happily accept any key. That's a block layer bug, not QMP's. QMP clients are going to be misguided by valid_encryption_key the same way they are with the block_passwd command or how we suffer from it internally when calling bdrv_set_key() (which also manifests itself in HMP). Fixing the bug where it is will automatically fix all its instances.
Re: [Qemu-devel] Funny -m arguments can crash
On 08/13/2012 04:41 PM, Markus Armbruster wrote: Avi Kivity a...@redhat.com writes: On 08/08/2012 12:04 PM, Markus Armbruster wrote: Yes please, maybe with a notice to the user. Next problem: minimum RAM size. For instance, -M pc -m X, where X 32KiB dies qemu: fatal: Trying to execute code outside RAM or ROM at [...] Aborted (core dumped) with TCG, and KVM internal error. Suberror: 1 with KVM. Should a minimum RAM size be enforced? Board-specific? It's really a BIOS bug causing a limitation of both kvm and tcg to be hit. The BIOS should recognize it doesn't have sufficient memory and hang gracefully (if you can picture that). It just assumes some low memory is available and tries to execute it with the results you got. SeaBIOS indeed assumes it got at least 1MiB of RAM. It doesn't bother to check CMOS for a smaller RAM size. However, that bug / feature is currently masked by a QEMU bug: we screw up CMOS contents when there's less than 1 MiB of RAM. pc_cmos_init(): int val, nb, i; [...] /* memory size */ val = 640; /* base memory in K */ rtc_set_memory(s, 0x15, val); rtc_set_memory(s, 0x16, val 8); val = (ram_size / 1024) - 1024; if (val 65535) val = 65535; rtc_set_memory(s, 0x17, val); rtc_set_memory(s, 0x18, val 8); If ram_size 1MiB, val goes negative. Oops. For instance, with -m 500k, we happily promise 640KiB base memory (CMOS addr 0x15..16), almost 64MiB extended memory (0x17..18 and 0x30..31), yet no memory above 16MiB (0x34..35). An easy way to fix this is to require 1MiB of RAM :) But if you like, I'll put sane values in CMOS instead. That'll expose the SeaBIOS bug. IMO we need to fix CMOS reporting. (technically we shouldn't touch CMOS NVRAM at all; seabios should discover memory size via fwcfg and program it itself. But it's pointless to change it now) -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 0/3] VFIO-based PCI device assignment for QEMU 1.2
On 08/13/2012 04:27 PM, Anthony Liguori wrote: Thanks for pushing this forward! Hopefully this will finally kill off qemu-kvm.git for good. No, it won't. vfio requires a 3.6 kernel, which we cannot assume anyone has. We'll need the original device assignment code side-by-side. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 11/34] qmp: query-block: add 'valid_encryption_key' field
On Mon, 13 Aug 2012 15:50:13 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Sat, 11 Aug 2012 09:45:14 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 10 Aug 2012 19:17:22 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 10 Aug 2012 18:35:26 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: On Fri, 10 Aug 2012 09:56:11 +0200 Markus Armbruster arm...@redhat.com wrote: Revisited this one on review of v2, replying here for context. Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 02 Aug 2012 13:35:54 +0200 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- block.c | 1 + qapi-schema.json | 7 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index b38940b..9c113b8 100644 --- a/block.c +++ b/block.c @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp) info-value-inserted-ro = bs-read_only; info-value-inserted-drv = g_strdup(bs-drv-format_name); info-value-inserted-encrypted = bs-encrypted; +info-value-inserted-valid_encryption_key = bs-valid_key; if (bs-backing_file[0]) { info-value-inserted-has_backing_file = true; info-value-inserted-backing_file = g_strdup(bs-backing_file); diff --git a/qapi-schema.json b/qapi-schema.json index bc55ed2..1b2d7f5 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -400,6 +400,8 @@ # # @encrypted: true if the backing device is encrypted # +# @valid_encryption_key: true if a valid encryption key has been set +# # @bps: total throughput limit in bytes per second is specified # # @bps_rd: read throughput limit in bytes per second is specified @@ -419,8 +421,9 @@ { 'type': 'BlockDeviceInfo', 'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str', '*backing_file': 'str', 'encrypted': 'bool', -'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int', -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} } +'valid_encryption_key': 'bool', 'bps': 'int', +'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int', +'iops_rd': 'int', 'iops_wr': 'int'} } ## # @BlockDeviceIoStatus: BlockDeviceInfo is API, isn't it? Yes. Note that bs-valid_key currently implies bs-encrypted. bs-valid_key !bs-encrypted is impossible. Should we make valid_encryption_key only available when encrypted? I don't think so. It's a bool, so it's ok for it to be false when encrypted is false. What bothers me is encrypted=false, valid_encryption_key=true. Disappearing keys is worse, IMHO (assuming that that situation is impossible in practice, of course). It's fundamentally three states: unencrypted, encrypted-no-key, encrypted-got-key. I'm fine with mapping these onto two bools, it's how the block layer does it. You may want to consider a single enumeration instead. That's arguable. But I like the bools slightly better because they allow clients to do a true/false check vs. having to check against an enum value. Again, that's arguable. valid_encryption_key is a bit long for my taste. Yours may be different. We should choose more descriptive and self-documenting names for the protocol. Besides, I can't think of anything shorter that won't get cryptic. Suggestions are always welcome though :) valid_encryption_key sounds like the value is the valid key. That's exactly what it is. Err, isn't the value bool? The key value is a string... Ah, sorry, I read sounds like true means the key is valid even for an invalid key. I've renamed it to encryption_key_missing, should be better (although I could also do encryption_key_is_missing). got_crypt_key? Also avoids valid. Good, because current encrypted formats don't actually validate the key; they happily accept any key. That's a block layer bug, not QMP's. QMP clients are going to be misguided by valid_encryption_key
Re: [Qemu-devel] Funny -m arguments can crash
On Mon, Aug 13, 2012 at 04:56:53PM +0300, Avi Kivity wrote: On 08/13/2012 04:41 PM, Markus Armbruster wrote: Avi Kivity a...@redhat.com writes: On 08/08/2012 12:04 PM, Markus Armbruster wrote: Yes please, maybe with a notice to the user. Next problem: minimum RAM size. For instance, -M pc -m X, where X 32KiB dies qemu: fatal: Trying to execute code outside RAM or ROM at [...] Aborted (core dumped) with TCG, and KVM internal error. Suberror: 1 with KVM. Should a minimum RAM size be enforced? Board-specific? It's really a BIOS bug causing a limitation of both kvm and tcg to be hit. The BIOS should recognize it doesn't have sufficient memory and hang gracefully (if you can picture that). It just assumes some low memory is available and tries to execute it with the results you got. SeaBIOS indeed assumes it got at least 1MiB of RAM. It doesn't bother to check CMOS for a smaller RAM size. However, that bug / feature is currently masked by a QEMU bug: we screw up CMOS contents when there's less than 1 MiB of RAM. pc_cmos_init(): int val, nb, i; [...] /* memory size */ val = 640; /* base memory in K */ rtc_set_memory(s, 0x15, val); rtc_set_memory(s, 0x16, val 8); val = (ram_size / 1024) - 1024; if (val 65535) val = 65535; rtc_set_memory(s, 0x17, val); rtc_set_memory(s, 0x18, val 8); If ram_size 1MiB, val goes negative. Oops. For instance, with -m 500k, we happily promise 640KiB base memory (CMOS addr 0x15..16), almost 64MiB extended memory (0x17..18 and 0x30..31), yet no memory above 16MiB (0x34..35). An easy way to fix this is to require 1MiB of RAM :) But if you like, I'll put sane values in CMOS instead. That'll expose the SeaBIOS bug. IMO we need to fix CMOS reporting. (technically we shouldn't touch CMOS NVRAM at all; seabios should discover memory size via fwcfg and program it itself. But it's pointless to change it now) Chipset we emulate does not support all those crazy memory values you can give to -m. -- Gleb.
Re: [Qemu-devel] Funny -m arguments can crash
On 08/13/2012 05:02 PM, Gleb Natapov wrote: IMO we need to fix CMOS reporting. (technically we shouldn't touch CMOS NVRAM at all; seabios should discover memory size via fwcfg and program it itself. But it's pointless to change it now) Chipset we emulate does not support all those crazy memory values you can give to -m. Our chipset is a 440fx enhanced with fwcfg and other goodies, not a plain 440fx. But it's true it probably doesn't support 82.66642kB RAM. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 0/3] VFIO-based PCI device assignment for QEMU 1.2
On 2012-08-13 15:58, Avi Kivity wrote: On 08/13/2012 04:27 PM, Anthony Liguori wrote: Thanks for pushing this forward! Hopefully this will finally kill off qemu-kvm.git for good. No, it won't. vfio requires a 3.6 kernel, which we cannot assume anyone has. We'll need the original device assignment code side-by-side. ...which is on my to-do list for 1.3. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH v10 1/7] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg
Set the close-on-exec flag for the file descriptor received via SCM_RIGHTS. Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- v4 -This patch is new in v4 (ebl...@redhat.com) v5 -Fallback to FD_CLOEXEC if MSG_CMSG_CLOEXEC is not available (ebl...@redhat.com, stefa...@linux.vnet.ibm.com) v6 -Set cloexec on correct fd (ebl...@redhat.com) v7-v10 -No changes qemu-char.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index c2aaaee..ab4a928 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2238,6 +2238,9 @@ static void unix_process_msgfd(CharDriverState *chr, struct msghdr *msg) if (fd 0) continue; +#ifndef MSG_CMSG_CLOEXEC +qemu_set_cloexec(fd); +#endif if (s-msgfd != -1) close(s-msgfd); s-msgfd = fd; @@ -2253,6 +2256,7 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len) struct cmsghdr cmsg; char control[CMSG_SPACE(sizeof(int))]; } msg_control; +int flags = 0; ssize_t ret; iov[0].iov_base = buf; @@ -2263,9 +2267,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len) msg.msg_control = msg_control; msg.msg_controllen = sizeof(msg_control); -ret = recvmsg(s-fd, msg, 0); -if (ret 0 s-is_unix) +#ifdef MSG_CMSG_CLOEXEC +flags |= MSG_CMSG_CLOEXEC; +#endif +ret = recvmsg(s-fd, msg, flags); +if (ret 0 s-is_unix) { unix_process_msgfd(chr, msg); +} return ret; } -- 1.7.10.4
[Qemu-devel] [PATCH v10 4/7] block: Convert open calls to qemu_open
This patch converts all block layer open calls to qemu_open. Note that this adds the O_CLOEXEC flag to the changed open paths when the O_CLOEXEC macro is defined. Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- v2: -Convert calls to qemu_open instead of file_open (kw...@redhat.com) -Mention introduction of O_CLOEXEC (kw...@redhat.com) v3-v10: -No changes block/raw-posix.c | 18 +- block/raw-win32.c |4 ++-- block/vdi.c |5 +++-- block/vmdk.c | 21 + block/vpc.c |2 +- block/vvfat.c |4 ++-- 6 files changed, 26 insertions(+), 28 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index f606211..08b997e 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -572,8 +572,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) options++; } -fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, - 0644); +fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, + 0644); if (fd 0) { result = -errno; } else { @@ -846,7 +846,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags) if ( bsdPath[ 0 ] != '\0' ) { strcat(bsdPath,s0); /* some CDs don't have a partition 0 */ -fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE); +fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE); if (fd 0) { bsdPath[strlen(bsdPath)-1] = '1'; } else { @@ -903,7 +903,7 @@ static int fd_open(BlockDriverState *bs) #endif return -EIO; } -s-fd = open(bs-filename, s-open_flags ~O_NONBLOCK); +s-fd = qemu_open(bs-filename, s-open_flags ~O_NONBLOCK); if (s-fd 0) { s-fd_error_time = get_clock(); s-fd_got_error = 1; @@ -977,7 +977,7 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options) options++; } -fd = open(filename, O_WRONLY | O_BINARY); +fd = qemu_open(filename, O_WRONLY | O_BINARY); if (fd 0) return -errno; @@ -1057,7 +1057,7 @@ static int floppy_probe_device(const char *filename) prio = 50; } -fd = open(filename, O_RDONLY | O_NONBLOCK); +fd = qemu_open(filename, O_RDONLY | O_NONBLOCK); if (fd 0) { goto out; } @@ -1110,7 +1110,7 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag) close(s-fd); s-fd = -1; } -fd = open(bs-filename, s-open_flags | O_NONBLOCK); +fd = qemu_open(bs-filename, s-open_flags | O_NONBLOCK); if (fd = 0) { if (ioctl(fd, FDEJECT, 0) 0) perror(FDEJECT); @@ -1160,7 +1160,7 @@ static int cdrom_probe_device(const char *filename) int prio = 0; struct stat st; -fd = open(filename, O_RDONLY | O_NONBLOCK); +fd = qemu_open(filename, O_RDONLY | O_NONBLOCK); if (fd 0) { goto out; } @@ -1284,7 +1284,7 @@ static int cdrom_reopen(BlockDriverState *bs) */ if (s-fd = 0) close(s-fd); -fd = open(bs-filename, s-open_flags, 0644); +fd = qemu_open(bs-filename, s-open_flags, 0644); if (fd 0) { s-fd = -1; return -EIO; diff --git a/block/raw-win32.c b/block/raw-win32.c index e4b0b75..8d7838d 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -255,8 +255,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) options++; } -fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, - 0644); +fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, + 0644); if (fd 0) return -EIO; set_sparse(fd); diff --git a/block/vdi.c b/block/vdi.c index 57325d6..c4f1529 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -653,8 +653,9 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options) options++; } -fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, - 0644); +fd = qemu_open(filename, + O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, + 0644); if (fd 0) { return -errno; } diff --git a/block/vmdk.c b/block/vmdk.c index 18e9b4c..557dc1b 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1161,10 +1161,9 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, VMDK4Header header; uint32_t tmp, magic, grains, gd_size, gt_size, gt_count; -fd = open( -filename, -O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, -0644); +fd = qemu_open(filename, + O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, + 0644); if (fd 0) { return -errno; } @@ -1484,15 +1483,13 @@ static int vmdk_create(const char
[Qemu-devel] [PATCH v10 5/7] block: Convert close calls to qemu_close
This patch converts all block layer close calls, that correspond to qemu_open calls, to qemu_close. Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- v5: -This patch is new in v5. (kw...@redhat.com, ebl...@redhat.com) v6-v9: -No changes v10: -Don't use underscore prefix on functions. (blauwir...@gmail.com) block/raw-posix.c | 24 block/raw-win32.c |2 +- block/vmdk.c |4 ++-- block/vpc.c |2 +- block/vvfat.c | 12 ++-- osdep.c |5 + qemu-common.h |1 + savevm.c |4 ++-- 8 files changed, 30 insertions(+), 24 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 08b997e..6be20b1 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -271,7 +271,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, out_free_buf: qemu_vfree(s-aligned_buf); out_close: -close(fd); +qemu_close(fd); return -errno; } @@ -376,7 +376,7 @@ static void raw_close(BlockDriverState *bs) { BDRVRawState *s = bs-opaque; if (s-fd = 0) { -close(s-fd); +qemu_close(s-fd); s-fd = -1; if (s-aligned_buf != NULL) qemu_vfree(s-aligned_buf); @@ -580,7 +580,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) if (ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) { result = -errno; } -if (close(fd) != 0) { +if (qemu_close(fd) != 0) { result = -errno; } } @@ -850,7 +850,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags) if (fd 0) { bsdPath[strlen(bsdPath)-1] = '1'; } else { -close(fd); +qemu_close(fd); } filename = bsdPath; } @@ -889,7 +889,7 @@ static int fd_open(BlockDriverState *bs) last_media_present = (s-fd = 0); if (s-fd = 0 (get_clock() - s-fd_open_time) = FD_OPEN_TIMEOUT) { -close(s-fd); +qemu_close(s-fd); s-fd = -1; #ifdef DEBUG_FLOPPY printf(Floppy closed\n); @@ -988,7 +988,7 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options) else if (lseek(fd, 0, SEEK_END) total_size * BDRV_SECTOR_SIZE) ret = -ENOSPC; -close(fd); +qemu_close(fd); return ret; } @@ -1038,7 +1038,7 @@ static int floppy_open(BlockDriverState *bs, const char *filename, int flags) return ret; /* close fd so that we can reopen it as needed */ -close(s-fd); +qemu_close(s-fd); s-fd = -1; s-fd_media_changed = 1; @@ -1072,7 +1072,7 @@ static int floppy_probe_device(const char *filename) prio = 100; outc: -close(fd); +qemu_close(fd); out: return prio; } @@ -1107,14 +1107,14 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag) int fd; if (s-fd = 0) { -close(s-fd); +qemu_close(s-fd); s-fd = -1; } fd = qemu_open(bs-filename, s-open_flags | O_NONBLOCK); if (fd = 0) { if (ioctl(fd, FDEJECT, 0) 0) perror(FDEJECT); -close(fd); +qemu_close(fd); } } @@ -1175,7 +1175,7 @@ static int cdrom_probe_device(const char *filename) prio = 100; outc: -close(fd); +qemu_close(fd); out: return prio; } @@ -1283,7 +1283,7 @@ static int cdrom_reopen(BlockDriverState *bs) * FreeBSD seems to not notice sometimes... */ if (s-fd = 0) -close(s-fd); +qemu_close(s-fd); fd = qemu_open(bs-filename, s-open_flags, 0644); if (fd 0) { s-fd = -1; diff --git a/block/raw-win32.c b/block/raw-win32.c index 8d7838d..c56bf83 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -261,7 +261,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) return -EIO; set_sparse(fd); ftruncate(fd, total_size * 512); -close(fd); +qemu_close(fd); return 0; } diff --git a/block/vmdk.c b/block/vmdk.c index 557dc1b..daee426 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1258,7 +1258,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, ret = 0; exit: -close(fd); +qemu_close(fd); return ret; } @@ -1506,7 +1506,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) } ret = 0; exit: -close(fd); +qemu_close(fd); return ret; } diff --git a/block/vpc.c b/block/vpc.c index 60ebf5a..c0b82c4 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -744,7 +744,7 @@ static int vpc_create(const char *filename, QEMUOptionParameter *options) } fail: -close(fd); +qemu_close(fd); return ret; } diff --git a/block/vvfat.c b/block/vvfat.c index 22b586a..59d3c5b 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1105,7 +1105,7 @@ static inline
[Qemu-devel] [PATCH v10 3/7] block: Prevent detection of /dev/fdset/ as floppy
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- v8 -This patch is new in v8. It was reported on a prior fd passing approach and I realized it's needed in this series. (kw...@redhat.com) v9-v10 -No changes block/raw-posix.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 0dce089..f606211 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -1052,8 +1052,10 @@ static int floppy_probe_device(const char *filename) struct floppy_struct fdparam; struct stat st; -if (strstart(filename, /dev/fd, NULL)) +if (strstart(filename, /dev/fd, NULL) +!strstart(filename, /dev/fdset/, NULL)) { prio = 50; +} fd = open(filename, O_RDONLY | O_NONBLOCK); if (fd 0) { -- 1.7.10.4
[Qemu-devel] [PATCH v10 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets
This patch adds support that enables passing of file descriptors to the QEMU monitor where they will be stored in specified file descriptor sets. A file descriptor set can be used by a client like libvirt to store file descriptors for the same file. This allows the client to open a file with different access modes (O_RDWR, O_WRONLY, O_RDONLY) and add/remove the passed fds to/from an fd set as needed. This will allow QEMU to (in a later patch in this series) open and reopen the same file by dup()ing the fd in the fd set that corresponds to the file, where the fd has the matching access mode flag that QEMU requests. The new QMP commands are: add-fd: Add a file descriptor to an fd set remove-fd: Remove a file descriptor from an fd set query-fdsets: Return information describing all fd sets Note: These commands are not compatible with the existing getfd and closefd QMP commands. Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- v5: -This patch is new in v5 and replaces the pass-fd QMP command from v4. -By grouping fds in fd sets, we ease managability with an fd set per file, addressing concerns raised in v4 about handling reopens and preventing fd leakage. (ebl...@redhat.com, kw...@redhat.com, dberra...@redhat.com) v6 -Make @fd optional for remove-fd (ebl...@redhat.com) -Make @fdset-id optional for add-fd (ebl...@redhat.com) v7: -Share fd sets among all monitor connections (kw...@redhat.com) -Added mon_refcount to keep track of monitor connection count. v8: -Add opaque string to add-fd/query-fdsets. (stefa...@linux.vnet.ibm.com) -Use camel case for structures. (stefa...@linux.vnet.ibm.com) -Don't return in-use and refcount from query-fdsets. (stefa...@linux.vnet.ibm.com) -Don't return removed fd's from query-fdsets. (stefa...@linux.vnet.ibm.com) -Use fdset-id rather than fdset_id. (ebl...@redhat.com) -Fix fd leak in qmp_add_fd(). (stefa...@linux.vnet.ibm.com) -Update QMP errors. (stefa...@linux.vnet.ibm.com, ebl...@redhat.com) v9: -Use fdset-id rather than fdset_id. (ebl...@redhat.com) -Update example for query-fdsets. (ebl...@redhat.com) -Close fd immediately on remove-fd. (kw...@redhat.com, ebl...@redhat.com) -Drop fdset refcount, and check dup_fds instead (in a later patch). (ebl...@redhat.com) -Move mon_refcount code to a later patch. (kw...@redhat.com) v10 -No trailing comma in query-fdsets example. (ebl...@redhat.com) -Two separate messages in qmp_remove_fd. (ebl...@redhat.com) -Modify text in qmp_add_fd invalid parameter error message. (ebl...@redhat.com) monitor.c| 189 ++ qapi-schema.json | 98 qmp-commands.hx | 122 +++ 3 files changed, 409 insertions(+) diff --git a/monitor.c b/monitor.c index 49dccfe..6252d39 100644 --- a/monitor.c +++ b/monitor.c @@ -140,6 +140,23 @@ struct mon_fd_t { QLIST_ENTRY(mon_fd_t) next; }; +/* file descriptor associated with a file descriptor set */ +typedef struct MonFdsetFd MonFdsetFd; +struct MonFdsetFd { +int fd; +bool removed; +char *opaque; +QLIST_ENTRY(MonFdsetFd) next; +}; + +/* file descriptor set containing fds passed via SCM_RIGHTS */ +typedef struct MonFdset MonFdset; +struct MonFdset { +int64_t id; +QLIST_HEAD(, MonFdsetFd) fds; +QLIST_ENTRY(MonFdset) next; +}; + typedef struct MonitorControl { QObject *id; JSONMessageParser parser; @@ -211,6 +228,7 @@ static inline int mon_print_count_get(const Monitor *mon) { return 0; } #define QMP_ACCEPT_UNKNOWNS 1 static QLIST_HEAD(mon_list, Monitor) mon_list; +static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets; static mon_cmd_t mon_cmds[]; static mon_cmd_t info_cmds[]; @@ -2389,6 +2407,177 @@ int monitor_get_fd(Monitor *mon, const char *fdname) return -1; } +static void monitor_fdset_cleanup(MonFdset *mon_fdset) +{ +MonFdsetFd *mon_fdset_fd; +MonFdsetFd *mon_fdset_fd_next; + +QLIST_FOREACH_SAFE(mon_fdset_fd, mon_fdset-fds, next, mon_fdset_fd_next) { +if (mon_fdset_fd-removed) { +close(mon_fdset_fd-fd); +g_free(mon_fdset_fd-opaque); +QLIST_REMOVE(mon_fdset_fd, next); +g_free(mon_fdset_fd); +} +} + +if (QLIST_EMPTY(mon_fdset-fds)) { +QLIST_REMOVE(mon_fdset, next); +g_free(mon_fdset); +} +} + +AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque, + const char *opaque, Error **errp) +{ +int fd; +Monitor *mon = cur_mon; +MonFdset *mon_fdset; +MonFdsetFd *mon_fdset_fd; +AddfdInfo *fdinfo; + +fd = qemu_chr_fe_get_msgfd(mon-chr); +if (fd == -1) { +error_set(errp, QERR_FD_NOT_SUPPLIED); +goto error; +} + +if (has_fdset_id) { +QLIST_FOREACH(mon_fdset, mon_fdsets, next) { +if (mon_fdset-id == fdset_id) { +break; +} +} +
Re: [Qemu-devel] Funny -m arguments can crash
On Mon, Aug 13, 2012 at 05:04:30PM +0300, Avi Kivity wrote: On 08/13/2012 05:02 PM, Gleb Natapov wrote: IMO we need to fix CMOS reporting. (technically we shouldn't touch CMOS NVRAM at all; seabios should discover memory size via fwcfg and program it itself. But it's pointless to change it now) Chipset we emulate does not support all those crazy memory values you can give to -m. Our chipset is a 440fx enhanced with fwcfg and other goodies, not a plain 440fx. It has registers to program DIMM slots configs. fwcfg does not enhanced it in any way. And you cannot program more than 4G memory there may be even less. So what do you mean by program it itself? Program CMOS itself? (What for? QEMU does not care). Or program 440fx DIMM config? But it's true it probably doesn't support 82.66642kB RAM. It does not support much less exotic 4G. -- Gleb.
[Qemu-devel] [PATCH v10 7/7] monitor: Clean up fd sets on monitor disconnect
Fd sets are shared by all monitor connections. Fd sets are considered to be in use while at least one monitor is connected. When the last monitor disconnects, all fds that are members of an fd set with no outstanding dup references are closed. This prevents any fd leakage associated with a client disconnect prior to using a passed fd. Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- v5: -This patch is new in v5. -This support addresses concerns from v4 regarding fd leakage if the client disconnects unexpectedly. (ebl...@redhat.com, kw...@redhat.com, dberra...@redhat.com) v6: -No changes v7: -Removed monitor_fdsets_set_in_use() function since we now use mon_refcount to determine if fdsets are in use. -Added monitor_fdsets_cleanup() function, and increment/decrement of mon_refcount when monitor connects/disconnects. v8: -Use camel case for structures. (stefa...@linux.vnet.ibm.com) v9: -Define mon_refcount and add corresponding logic to monitor_fdset_cleanup here rather than earlier patch. (kw...@redhat.com) v10: -No changes monitor.c | 23 ++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index f56d6a1..0a4fa38 100644 --- a/monitor.c +++ b/monitor.c @@ -230,6 +230,7 @@ static inline int mon_print_count_get(const Monitor *mon) { return 0; } static QLIST_HEAD(mon_list, Monitor) mon_list; static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets; +static int mon_refcount; static mon_cmd_t mon_cmds[]; static mon_cmd_t info_cmds[]; @@ -2414,7 +2415,8 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) MonFdsetFd *mon_fdset_fd_next; QLIST_FOREACH_SAFE(mon_fdset_fd, mon_fdset-fds, next, mon_fdset_fd_next) { -if (mon_fdset_fd-removed) { +if (mon_fdset_fd-removed || +(QLIST_EMPTY(mon_fdset-dup_fds) mon_refcount == 0)) { close(mon_fdset_fd-fd); g_free(mon_fdset_fd-opaque); QLIST_REMOVE(mon_fdset_fd, next); @@ -2428,6 +2430,16 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) } } +static void monitor_fdsets_cleanup(void) +{ +MonFdset *mon_fdset; +MonFdset *mon_fdset_next; + +QLIST_FOREACH_SAFE(mon_fdset, mon_fdsets, next, mon_fdset_next) { +monitor_fdset_cleanup(mon_fdset); +} +} + AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque, const char *opaque, Error **errp) { @@ -4863,9 +4875,12 @@ static void monitor_control_event(void *opaque, int event) data = get_qmp_greeting(); monitor_json_emitter(mon, data); qobject_decref(data); +mon_refcount++; break; case CHR_EVENT_CLOSED: json_message_parser_destroy(mon-mc-parser); +mon_refcount--; +monitor_fdsets_cleanup(); break; } } @@ -4906,6 +4921,12 @@ static void monitor_event(void *opaque, int event) readline_show_prompt(mon-rs); } mon-reset_seen = 1; +mon_refcount++; +break; + +case CHR_EVENT_CLOSED: +mon_refcount--; +monitor_fdsets_cleanup(); break; } } -- 1.7.10.4
[Qemu-devel] [PATCH v10 0/7] file descriptor passing using fd sets
libvirt's sVirt security driver provides SELinux MAC isolation for Qemu guest processes and their corresponding image files. In other words, sVirt uses SELinux to prevent a QEMU process from opening files that do not belong to it. sVirt provides this support by labeling guests and resources with security labels that are stored in file system extended attributes. Some file systems, such as NFS, do not support the extended attribute security namespace, and therefore cannot support sVirt isolation. A solution to this problem is to provide fd passing support, where libvirt opens files and passes file descriptors to QEMU. This, along with SELinux policy to prevent QEMU from opening files, can provide image file isolation for NFS files stored on the same NFS mount. This patch series adds the add-fd, remove-fd, and query-fdsets QMP monitor commands, which allow file descriptors to be passed via SCM_RIGHTS, and assigned to specified fd sets. This allows fd sets to be created per file with fds having, for example, different access rights. When QEMU needs to reopen a file with different access rights, it can search for a matching fd in the fd set. Fd sets also allow for easy tracking of fds per file, helping to prevent fd leaks. Support is also added to the block layer to allow QEMU to dup an fd from an fdset when the filename is of the /dev/fdset/nnn format, where nnn is the fd set ID. No new SELinux policy is required to prevent open of NFS files (files with type nfs_t). The virt_use_nfs boolean type simply needs to be set to false, and open will be prevented (and dup will be allowed). For example: # setsebool virt_use_nfs 0 # getsebool virt_use_nfs virt_use_nfs -- off Corey Bryant (7): qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg qapi: Introduce add-fd, remove-fd, query-fdsets block: Prevent detection of /dev/fdset/ as floppy block: Convert open calls to qemu_open block: Convert close calls to qemu_close block: Enable qemu_open/close to work with fd sets monitor: Clean up fd sets on monitor disconnect block/raw-posix.c | 46 + block/raw-win32.c |6 +- block/vdi.c |5 +- block/vmdk.c | 25 ++--- block/vpc.c |4 +- block/vvfat.c | 16 +-- cutils.c |5 + monitor.c | 291 + monitor.h |5 + osdep.c | 114 + qapi-schema.json | 98 ++ qemu-char.c | 12 ++- qemu-common.h |2 + qemu-tool.c | 20 qmp-commands.hx | 122 ++ savevm.c |4 +- 16 files changed, 720 insertions(+), 55 deletions(-) -- 1.7.10.4
[Qemu-devel] [PATCH v10 6/7] block: Enable qemu_open/close to work with fd sets
When qemu_open is passed a filename of the /dev/fdset/nnn format (where nnn is the fdset ID), an fd with matching access mode flags will be searched for within the specified monitor fd set. If the fd is found, a dup of the fd will be returned from qemu_open. Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com --- v2: -Get rid of file_open and move dup code to qemu_open (kw...@redhat.com) -Use strtol wrapper instead of atoi (kw...@redhat.com) v3: -Add note about fd leakage (ebl...@redhat.com) v4 -Moved patch to be later in series (lcapitul...@redhat.com) -Update qemu_open to check access mode flags and set flags that can be set (ebl...@redhat.com, kw...@redhat.com) v5: -This patch was overhauled quite a bit in this version, with the addition of fd set and refcount support. -Use qemu_set_cloexec() on dup'd fd (ebl...@redhat.com) -Modify flags set by fcntl on dup'd fd (ebl...@redhat.com) -Reduce syscalls when setting flags for dup'd fd (ebl...@redhat.com) -Fix O_RDWR, O_RDONLY, O_WRONLY checks (ebl...@redhat.com) v6: -Pass only the fd to qemu_close() and keep track of dup fds per fd set. (kw...@redhat.com, ebl...@redhat.com) -Handle refcount incr/decr in new dup_fd_add/remove fd functions. -Use qemu_set_cloexec() appropriately in qemu_dup() (kw...@redhat.com) -Simplify setting of setfl_flags in qemu_dup() (kw...@redhat.com) -Add preprocessor checks for F_DUPFD_CLOEXEC (ebl...@redhat.com) -Simplify flag checking in monitor_fdset_get_fd() (kw...@redhat.com) v7: -Minor updates to reference global mon_fdsets, and to remove default_mon usage in osdep.c. (kw...@redhat.com) v8: -Use camel case for structures. (stefa...@linux.vnet.ibm.com) v9: -Drop fdset refcount and check dup_fds instead. (ebl...@redhat.com) -Fix dupfd leak in qemu_dup(). (ebl...@redhat.com) -Always set O_CLOEXEC in qemu_dup(). (kw...@redhat.com) -Change name of qemu_dup() to qemu_dup_flags(). (kw...@redhat.com) v10: -Don't use underscore prefix on functions. (blauwir...@gmail.com) -Set errno if monitor_fdset_dup_fd_add() fails. (ebl...@redhat.com) -Check dupfd for -1 on qemu_dup_flags() call. (ebl...@redhat.com) cutils.c |5 +++ monitor.c | 83 ++- monitor.h |5 +++ osdep.c | 109 + qemu-common.h |1 + qemu-tool.c | 20 +++ 6 files changed, 222 insertions(+), 1 deletion(-) diff --git a/cutils.c b/cutils.c index 9d4c570..8b0d2bb 100644 --- a/cutils.c +++ b/cutils.c @@ -382,3 +382,8 @@ int qemu_parse_fd(const char *param) } return fd; } + +int qemu_parse_fdset(const char *param) +{ +return qemu_parse_fd(param); +} diff --git a/monitor.c b/monitor.c index 6252d39..f56d6a1 100644 --- a/monitor.c +++ b/monitor.c @@ -154,6 +154,7 @@ typedef struct MonFdset MonFdset; struct MonFdset { int64_t id; QLIST_HEAD(, MonFdsetFd) fds; +QLIST_HEAD(, MonFdsetFd) dup_fds; QLIST_ENTRY(MonFdset) next; }; @@ -2421,7 +2422,7 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset) } } -if (QLIST_EMPTY(mon_fdset-fds)) { +if (QLIST_EMPTY(mon_fdset-fds) QLIST_EMPTY(mon_fdset-dup_fds)) { QLIST_REMOVE(mon_fdset, next); g_free(mon_fdset); } @@ -2578,6 +2579,86 @@ FdsetInfoList *qmp_query_fdsets(Error **errp) return fdset_list; } +int monitor_fdset_get_fd(int64_t fdset_id, int flags) +{ +MonFdset *mon_fdset; +MonFdsetFd *mon_fdset_fd; +int mon_fd_flags; + +QLIST_FOREACH(mon_fdset, mon_fdsets, next) { +if (mon_fdset-id != fdset_id) { +continue; +} +QLIST_FOREACH(mon_fdset_fd, mon_fdset-fds, next) { +mon_fd_flags = fcntl(mon_fdset_fd-fd, F_GETFL); +if (mon_fd_flags == -1) { +return -1; +} + +if ((flags O_ACCMODE) == (mon_fd_flags O_ACCMODE)) { +return mon_fdset_fd-fd; +} +} +errno = EACCES; +return -1; +} +errno = ENOENT; +return -1; +} + +int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd) +{ +MonFdset *mon_fdset; +MonFdsetFd *mon_fdset_fd_dup; + +QLIST_FOREACH(mon_fdset, mon_fdsets, next) { +if (mon_fdset-id != fdset_id) { +continue; +} +QLIST_FOREACH(mon_fdset_fd_dup, mon_fdset-dup_fds, next) { +if (mon_fdset_fd_dup-fd == dup_fd) { +return -1; +} +} +mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup)); +mon_fdset_fd_dup-fd = dup_fd; +QLIST_INSERT_HEAD(mon_fdset-dup_fds, mon_fdset_fd_dup, next); +return 0; +} +return -1; +} + +static int monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove) +{ +MonFdset *mon_fdset; +MonFdsetFd *mon_fdset_fd_dup; + +QLIST_FOREACH(mon_fdset, mon_fdsets, next) { +QLIST_FOREACH(mon_fdset_fd_dup, mon_fdset-dup_fds,
Re: [Qemu-devel] [PATCH for-1.2 v5 14/14] pci: Tidy up PCI host bridges
Michael S. Tsirkin m...@redhat.com writes: On Thu, Aug 02, 2012 at 03:47:06AM +0200, Andreas Färber wrote: Uglify the parent field to enforce QOM-style access via casts. Don't just typedef PCIHostState, either use it directly or embed it. Signed-off-by: Andreas Färber afaer...@suse.de IMHO only one chunk from this patch should be applied (below). Below it is split out but needs to be rebased on top of patches 1-13. I understand what your objection is but it's unreasonable IMHO. The purpose of QOM is to bring consistency across large swaths of code in QEMU that have historically done things there own way. This means expressing concepts like inheritence and casting in the same way across the board. The common way (the QOM way) is to make the parent type the first member of the struct (typically named parent or parent_obj) and then to use cast macros to upcast and downcast. This patch is 100% correct in that regard and I'm going to apply it once Andreas makes the change I requested. For my part, I'm long over due in writing up a device authoring style guide that I promised a few weeks ago. I'll write that up this afternoon and send it out today. We can debate the merits of this sort of thing in the style guide. Regards, Anthony Liguori -- From: Andreas Färber andreas.faer...@web.de piix: minor code simplification There's no need to deal with qdev internals in piix - we get device state from qdev_create so just use that. Signed-off-by: Andreas Färber andreas.faer...@web.de Signed-off-by: Michael S. Tsirkin m...@redhat.com --- diff --git a/hw/piix_pci.c b/hw/piix_pci.c index c497a01..18554a6 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -274,7 +274,7 @@ static PCIBus *i440fx_common_init(const char *device_name, dev = qdev_create(NULL, i440FX-pcihost); s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev)); s-address_space = address_space_mem; -b = pci_bus_new(s-busdev.qdev, NULL, pci_address_space, +b = pci_bus_new(dev, NULL, pci_address_space, address_space_io, 0); s-bus = b; object_property_add_child(qdev_get_machine(), i440fx, OBJECT(dev), NULL);
Re: [Qemu-devel] Funny -m arguments can crash
Markus Armbruster arm...@redhat.com writes: Avi Kivity a...@redhat.com writes: On 08/08/2012 12:04 PM, Markus Armbruster wrote: Yes please, maybe with a notice to the user. Next problem: minimum RAM size. For instance, -M pc -m X, where X 32KiB dies qemu: fatal: Trying to execute code outside RAM or ROM at [...] Aborted (core dumped) with TCG, and KVM internal error. Suberror: 1 with KVM. Should a minimum RAM size be enforced? Board-specific? It's really a BIOS bug causing a limitation of both kvm and tcg to be hit. The BIOS should recognize it doesn't have sufficient memory and hang gracefully (if you can picture that). It just assumes some low memory is available and tries to execute it with the results you got. SeaBIOS indeed assumes it got at least 1MiB of RAM. It doesn't bother to check CMOS for a smaller RAM size. However, that bug / feature is currently masked by a QEMU bug: we screw up CMOS contents when there's less than 1 MiB of RAM. pc_cmos_init(): int val, nb, i; [...] /* memory size */ val = 640; /* base memory in K */ rtc_set_memory(s, 0x15, val); rtc_set_memory(s, 0x16, val 8); val = (ram_size / 1024) - 1024; if (val 65535) val = 65535; rtc_set_memory(s, 0x17, val); rtc_set_memory(s, 0x18, val 8); If ram_size 1MiB, val goes negative. Oops. For instance, with -m 500k, we happily promise 640KiB base memory (CMOS addr 0x15..16), almost 64MiB extended memory (0x17..18 and 0x30..31), yet no memory above 16MiB (0x34..35). An easy way to fix this is to require 1MiB of RAM :) But if you like, I'll put sane values in CMOS instead. That'll expose the SeaBIOS bug. Anthony, you're the PC maintainer, got a preference? SeaBIOS thread: http://comments.gmane.org/gmane.comp.bios.coreboot.seabios/4341 I'd prefer fixing the CMOS values over limiting to 1MB of RAM. Having a 1MB limit is purely theoritical--not practical. There's no good reason for anyone to ask for 1MB unless they know what they're doing. If it's truly a mistake, then asking for 2MB is just as much of a mistake because no real guest will run with 2MB of memory anyway (you can't even load a kernel). So if we're just going for theoritical correctness, we ought to do it the Right Way which is fixing the CMOS values and putting the check in SeaBIOS. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 0/3] VFIO-based PCI device assignment for QEMU 1.2
On Mon, 2012-08-13 at 08:27 -0500, Anthony Liguori wrote: Alex Williamson alex.william...@redhat.com writes: VFIO kernel support was just merged into Linux, so I'd like to formally propose inclusion of the QEMU vfio-pci driver for QEMU 1.2. Included here is support for x86 PCI device assignment. PCI INTx is not yet enabled, but devices making use of either MSI or MSI-X work. The level irqfd and eoifd support I've proposed for KVM enable an accelerated patch for this through KVM. I'd like to get this base driver in first and enable the remaining support in-tree. I've split this version up a little from the RFC to make it a bit easier to review. Review comments from Blue Swirl and Avi are already incorporated, including Avi's requests to simplify both the PCI BAR mapping and unmapping paths. Hi Alex, Thanks for pushing this forward! Hopefully this will finally kill off qemu-kvm.git for good. I think this series is going to have to wait for 1.3 to open up. We have a very short release window for this release and I'd feel a lot more comfortable having such a significant feature spend some time in the development cycle getting testing/review. I'd like to see a few Reviewed-by's too for this series before it goes in. I expect they won't be hard to get but I also expect it will take a few more revisions of this series to get there. That's disappointing, but I can understand your reluctance. Blue Swirl reviewed the RFC and could perhaps add a Reviewed-by. Alexey has been working on the POWER port and I'm sure could provide a Reviewed-by. We also have a few early adopters that are already making use of this code. Towards accepting it, the driver is entirely self contained, there's really no risk to the rest of qemu. The only missing functionality is legacy interrupt support. Perhaps there's a compromise where this driver could be considered a tech preview in 1.2 (x-vfio-pci?). Thanks, Alex
Re: [Qemu-devel] [PATCH] update-linux-headers.sh: Pull in asm-generic/kvm_para.h
Peter Maydell peter.mayd...@linaro.org writes: Ping^2 ? In a previous thread, we all agreed that all changes to linux headers would come in through uq/master to ensure that we didn't have a repeat scenario of depending on a header that didn't make it to Avi's tree unchanged. Avi/Marcelo, can you guys pick this up through uq/master? Regards, Anthony Liguori On 8 August 2012 13:34, Peter Maydell peter.mayd...@linaro.org wrote: Ping? patchwork url: http://patchwork.ozlabs.org/patch/173202/ -- PMM On 25 July 2012 16:29, Peter Maydell peter.mayd...@linaro.org wrote: Add asm-generic/kvm_para.h to the set of non-architecture specific KVM kernel headers we copy into QEMU. This header may be included by an architecture's kvm_para.h header. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- scripts/update-linux-headers.sh |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh index 9d2a4bc..a639c5b 100755 --- a/scripts/update-linux-headers.sh +++ b/scripts/update-linux-headers.sh @@ -46,6 +46,11 @@ mkdir -p $output/linux-headers/linux for header in kvm.h kvm_para.h vhost.h virtio_config.h virtio_ring.h; do cp $tmpdir/include/linux/$header $output/linux-headers/linux done +rm -rf $output/linux-headers/asm-generic +mkdir -p $output/linux-headers/asm-generic +for header in kvm_para.h; do +cp $tmpdir/include/asm-generic/$header $output/linux-headers/asm-generic +done if [ -L $linux/source ]; then cp $linux/source/COPYING $output/linux-headers else -- 1.7.5.4
Re: [Qemu-devel] [PATCH 1/7] migration: move total_time from ram stats to migration info
On 08/13/2012 04:50 AM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com --- hmp.c| 4 ++-- migration.c | 6 +++--- qapi-schema.json | 14 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) Looks reasonable to me! Missed qmp-commands.hx for the examples (but your cover letter mentioned that). Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Funny -m arguments can crash
Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster arm...@redhat.com writes: Avi Kivity a...@redhat.com writes: On 08/08/2012 12:04 PM, Markus Armbruster wrote: Yes please, maybe with a notice to the user. Next problem: minimum RAM size. For instance, -M pc -m X, where X 32KiB dies qemu: fatal: Trying to execute code outside RAM or ROM at [...] Aborted (core dumped) with TCG, and KVM internal error. Suberror: 1 with KVM. Should a minimum RAM size be enforced? Board-specific? It's really a BIOS bug causing a limitation of both kvm and tcg to be hit. The BIOS should recognize it doesn't have sufficient memory and hang gracefully (if you can picture that). It just assumes some low memory is available and tries to execute it with the results you got. SeaBIOS indeed assumes it got at least 1MiB of RAM. It doesn't bother to check CMOS for a smaller RAM size. However, that bug / feature is currently masked by a QEMU bug: we screw up CMOS contents when there's less than 1 MiB of RAM. pc_cmos_init(): int val, nb, i; [...] /* memory size */ val = 640; /* base memory in K */ rtc_set_memory(s, 0x15, val); rtc_set_memory(s, 0x16, val 8); val = (ram_size / 1024) - 1024; if (val 65535) val = 65535; rtc_set_memory(s, 0x17, val); rtc_set_memory(s, 0x18, val 8); If ram_size 1MiB, val goes negative. Oops. For instance, with -m 500k, we happily promise 640KiB base memory (CMOS addr 0x15..16), almost 64MiB extended memory (0x17..18 and 0x30..31), yet no memory above 16MiB (0x34..35). An easy way to fix this is to require 1MiB of RAM :) But if you like, I'll put sane values in CMOS instead. That'll expose the SeaBIOS bug. Anthony, you're the PC maintainer, got a preference? SeaBIOS thread: http://comments.gmane.org/gmane.comp.bios.coreboot.seabios/4341 I'd prefer fixing the CMOS values over limiting to 1MB of RAM. Having a 1MB limit is purely theoritical--not practical. There's no good reason for anyone to ask for 1MB unless they know what they're doing. If it's truly a mistake, then asking for 2MB is just as much of a mistake because no real guest will run with 2MB of memory anyway (you can't even load a kernel). So if we're just going for theoritical correctness, we ought to do it the Right Way which is fixing the CMOS values and putting the check in SeaBIOS. Okay, I'll cook up a patch fixing pc_cmos_init().
Re: [Qemu-devel] [PATCH v2 1/2] linux-user: Factor out guest space probing into a function
On 27 July 2012 03:50, Meador Inge mead...@codesourcery.com wrote: Signed-off-by: Meador Inge mead...@codesourcery.com Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
Re: [Qemu-devel] [PATCH v2 2/2] linux-user: Use init_guest_space when -R and -B are specified
On 27 July 2012 03:50, Meador Inge mead...@codesourcery.com wrote: Roll the code used to initialize the guest memory space when -R or -B is used into 'init_guest_space' and then call 'init_guest_space' from the driver. This way the reserved guest memory space can be probed for. Calling 'mmap' just once as is currently done is not guaranteed to succeed since the host address space validation might fail. Signed-off-by: Meador Inge mead...@codesourcery.com --- +/* If the commpage lies within the already allocated guest space, + * then there is no way we can allocate it. + */ +if (test_page_addr = guest_base +test_page_addr = (guest_base + guest_size)) { + return -1; +} The indent here is busted (hardcoded tabs), as checkpatch.pl will tell you. Otherwise Reviewed-by: Peter Maydell peter.mayd...@linaro.org I'm currently putting together a linux-user pullreq (with Riku's agreement since he's currently a bit busy), so I'll just fix this as I put this patch in, though. -- PMM
Re: [Qemu-devel] [RFC 0/7] Migration stats
On 08/13/2012 04:50 AM, Juan Quintela wrote: Hi This modifies the output of info migrate/qmp_query_migrate to add the stats that I got request for. - It moves total time to MigrationInfo instead of ram (luiz suggestion) Now's the time to do this, since the stat is new to 1.2 and we haven't yet made a release with it. Should we also rename 'MigrationInfo' to 'MigrationRamInfo', so that we don't make a similar mistake in the future of putting a stat in the wrong category? What do I want to know: - is there any stat that you want? Once here, adding a new one should be easy. Should we have some sort of stat for the number of pages that are sent more than once, and/or for the maximum count of times that a given page was sent? Having hot/cold page analysis might make it easier to decide in the future which pages to avoid sending until the very end, and knowing how many pages are sent multiple times as well as the maximum times any one page is sent might help. - examples are not done, waiting until people agree with what params are needed. Fair enough for RFC purposes. - luiz added in case he has QMP commets. - erik added for libvirt comments. Eric, actually. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/7] migration: store end_time in a local variable
On 08/13/2012 04:50 AM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com --- migration.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 3/7] migration: print total downtime for final phase of migration
On 08/13/2012 04:50 AM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com --- hmp.c| 4 migration.c | 5 - migration.h | 1 + qapi-schema.json | 6 +- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/hmp.c b/hmp.c index c0b0a10..10fee1b 100644 --- a/hmp.c +++ b/hmp.c @@ -151,6 +151,10 @@ void hmp_info_migrate(Monitor *mon) monitor_printf(mon, Migration status: %s\n, info-status); monitor_printf(mon, total time: % PRIu64 milliseconds\n, info-total_time); +if (strcmp(info-status, completed) == 0) { +monitor_printf(mon, downtime: % PRIu64 milliseconds\n, + info-downtime); +} Very nice stat - useful at showing how 'live' a live migration really is. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 3/7] migration: print total downtime for final phase of migration
On 08/13/2012 09:02 AM, Eric Blake wrote: On 08/13/2012 04:50 AM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com --- hmp.c| 4 migration.c | 5 - migration.h | 1 + qapi-schema.json | 6 +- 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/hmp.c b/hmp.c index c0b0a10..10fee1b 100644 --- a/hmp.c +++ b/hmp.c @@ -151,6 +151,10 @@ void hmp_info_migrate(Monitor *mon) monitor_printf(mon, Migration status: %s\n, info-status); monitor_printf(mon, total time: % PRIu64 milliseconds\n, info-total_time); +if (strcmp(info-status, completed) == 0) { +monitor_printf(mon, downtime: % PRIu64 milliseconds\n, + info-downtime); +} Very nice stat - useful at showing how 'live' a live migration really is. Hit send too soon: Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 4/7] migration: rename expected_time to expected_downtime
On 08/13/2012 04:50 AM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com --- arch_init.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) Trivial rename. @@ -576,24 +576,25 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) bwidth = qemu_get_clock_ns(rt_clock) - bwidth; bwidth = (bytes_transferred - bytes_transferred_last) / bwidth; -/* if we haven't transferred anything this round, force expected_time to a - * a very high value, but without crashing */ +/* if we haven't transferred anything this round, force + * expected_downtime to a a very high value, but without s/a a/a/ Latent typo, but the reformatting exposes it more. With that fixed, Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 5/7] migration: export migration_get_current()
On 08/13/2012 04:50 AM, Juan Quintela wrote: Signed-off-by: Juan Quintela quint...@redhat.com --- migration.c | 2 +- migration.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature