Re: [Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not

2012-08-13 Thread Kevin Wolf
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().

2012-08-13 Thread Kevin Wolf
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

2012-08-13 Thread Kevin Wolf
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

2012-08-13 Thread Stefan Hajnoczi
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

2012-08-13 Thread Nicholas A. Bellinger
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

2012-08-13 Thread Nicholas A. Bellinger
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

2012-08-13 Thread Nicholas A. Bellinger
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

2012-08-13 Thread Amos Kong
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

2012-08-13 Thread Michael S. Tsirkin
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

2012-08-13 Thread Michael S. Tsirkin
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.

2012-08-13 Thread Cornelia Huck
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

2012-08-13 Thread Michael S. Tsirkin
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

2012-08-13 Thread Yonit Halperin

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

2012-08-13 Thread Michael S. Tsirkin
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

2012-08-13 Thread Michael S. Tsirkin
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

2012-08-13 Thread Nicholas A. Bellinger
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()

2012-08-13 Thread Nicholas A. Bellinger
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

2012-08-13 Thread Nicholas A. Bellinger
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

2012-08-13 Thread Nicholas A. Bellinger
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

2012-08-13 Thread Peter Maydell
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

2012-08-13 Thread Peter Maydell
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

2012-08-13 Thread Edgar E. Iglesias
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

2012-08-13 Thread Bharata B Rao
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

2012-08-13 Thread Juan Quintela

Hi

Please send in any agenda items you are interested in covering.

Thanks, Juan.



Re: [Qemu-devel] github mirror still stale

2012-08-13 Thread Stefan Weil

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

2012-08-13 Thread Christophe Fergeau
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

2012-08-13 Thread Jens Freimann
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

2012-08-13 Thread Jens Freimann
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

2012-08-13 Thread Jens Freimann
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

2012-08-13 Thread Jens Freimann
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

2012-08-13 Thread Jens Freimann
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

2012-08-13 Thread Jens Freimann
[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-08-13 Thread Wenchao Xia

于 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

2012-08-13 Thread Juan Quintela
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

2012-08-13 Thread Juan Quintela
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

2012-08-13 Thread Juan Quintela
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

2012-08-13 Thread Gerd Hoffmann
  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

2012-08-13 Thread Gerd Hoffmann
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

2012-08-13 Thread Gerd Hoffmann
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

2012-08-13 Thread Juan Quintela
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

2012-08-13 Thread Juan Quintela
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

2012-08-13 Thread Juan Quintela
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

2012-08-13 Thread Juan Quintela
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

2012-08-13 Thread Juan Quintela
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()

2012-08-13 Thread Juan Quintela
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-08-13 Thread Wenchao Xia

于 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-08-13 Thread Wenchao Xia

于 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

2012-08-13 Thread 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.

* 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

2012-08-13 Thread Jan Kiszka
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

2012-08-13 Thread 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
---
 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

2012-08-13 Thread Andreas Färber
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

2012-08-13 Thread Andreas Färber
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

2012-08-13 Thread Kevin Wolf
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

2012-08-13 Thread Jan Kiszka
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

2012-08-13 Thread Anthony Liguori
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

2012-08-13 Thread Michael S. Tsirkin
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

2012-08-13 Thread Michael Tokarev
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

2012-08-13 Thread Kevin Wolf
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

2012-08-13 Thread Anthony Liguori
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

2012-08-13 Thread Luiz Capitulino
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

2012-08-13 Thread Andreas Färber
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

2012-08-13 Thread Luiz Capitulino
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

2012-08-13 Thread Erik Rull

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

2012-08-13 Thread Markus Armbruster
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

2012-08-13 Thread Jan Kiszka
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

2012-08-13 Thread Corey Bryant



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

2012-08-13 Thread Corey Bryant

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

2012-08-13 Thread Anthony Liguori
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

2012-08-13 Thread Corey Bryant

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

2012-08-13 Thread Corey Bryant

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

2012-08-13 Thread Markus Armbruster
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

2012-08-13 Thread Avi Kivity
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

2012-08-13 Thread Avi Kivity
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

2012-08-13 Thread Luiz Capitulino
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

2012-08-13 Thread Gleb Natapov
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

2012-08-13 Thread Avi Kivity
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

2012-08-13 Thread Jan Kiszka
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

2012-08-13 Thread Corey Bryant
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

2012-08-13 Thread Corey Bryant
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

2012-08-13 Thread Corey Bryant
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

2012-08-13 Thread Corey Bryant
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

2012-08-13 Thread Corey Bryant
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

2012-08-13 Thread Gleb Natapov
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

2012-08-13 Thread Corey Bryant
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

2012-08-13 Thread Corey Bryant
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

2012-08-13 Thread Corey Bryant
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

2012-08-13 Thread Anthony Liguori
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

2012-08-13 Thread Anthony Liguori
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

2012-08-13 Thread Alex Williamson
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

2012-08-13 Thread Anthony Liguori
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

2012-08-13 Thread Eric Blake
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

2012-08-13 Thread Markus Armbruster
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

2012-08-13 Thread Peter Maydell
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

2012-08-13 Thread Peter Maydell
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

2012-08-13 Thread Eric Blake
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

2012-08-13 Thread Eric Blake
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

2012-08-13 Thread Eric Blake
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

2012-08-13 Thread Eric Blake
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

2012-08-13 Thread Eric Blake
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()

2012-08-13 Thread Eric Blake
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


  1   2   3   >