[Qemu-devel] [PATCH 1/2] balloon: Don't allow multiple balloon handler registrations

2011-07-27 Thread Amit Shah
Multiple balloon devices don't make sense; disallow more than one
registration attempt to register handlers.

Signed-off-by: Amit Shah 
---
 balloon.c |   11 +--
 balloon.h |4 ++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/balloon.c b/balloon.c
index a938475..cf9e3b2 100644
--- a/balloon.c
+++ b/balloon.c
@@ -36,12 +36,19 @@ static QEMUBalloonEvent *balloon_event_fn;
 static QEMUBalloonStatus *balloon_stat_fn;
 static void *balloon_opaque;
 
-void qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
-  QEMUBalloonStatus *stat_func, void *opaque)
+int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
+ QEMUBalloonStatus *stat_func, void *opaque)
 {
+if (balloon_event_fn || balloon_stat_fn || balloon_opaque) {
+/* We're already registered one balloon handler.  How many can
+ * a guest really have?
+ */
+return -1;
+}
 balloon_event_fn = event_func;
 balloon_stat_fn = stat_func;
 balloon_opaque = opaque;
+return 0;
 }
 
 static int qemu_balloon(ram_addr_t target)
diff --git a/balloon.h b/balloon.h
index a6c31d5..3df14e6 100644
--- a/balloon.h
+++ b/balloon.h
@@ -20,8 +20,8 @@ typedef void (QEMUBalloonEvent)(void *opaque, ram_addr_t 
target);
 typedef void (QEMUBalloonStatus)(void *opaque, MonitorCompletion cb,
  void *cb_data);
 
-void qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
-  QEMUBalloonStatus *stat_func, void *opaque);
+int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
+QEMUBalloonStatus *stat_func, void *opaque);
 
 void monitor_print_balloon(Monitor *mon, const QObject *data);
 int do_info_balloon(Monitor *mon, MonitorCompletion cb, void *opaque);
-- 
1.7.6




[Qemu-devel] [PATCH 2/2] virtio-balloon: Check if balloon registration failed

2011-07-27 Thread Amit Shah
Multiple balloon registrations are not allowed; check if the
registration with the qemu balloon api succeeded.  If not, fail the
device init.

Signed-off-by: Amit Shah 
---
 hw/virtio-balloon.c |   10 +-
 hw/virtio-pci.c |3 +++
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 2ba7e95..304a31a 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -269,6 +269,7 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, 
int version_id)
 VirtIODevice *virtio_balloon_init(DeviceState *dev)
 {
 VirtIOBalloon *s;
+int ret;
 
 s = (VirtIOBalloon *)virtio_common_init("virtio-balloon",
 VIRTIO_ID_BALLOON,
@@ -278,12 +279,19 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
 s->vdev.set_config = virtio_balloon_set_config;
 s->vdev.get_features = virtio_balloon_get_features;
 
+ret = qemu_add_balloon_handler(virtio_balloon_to_target,
+   virtio_balloon_stat, s);
+if (ret < 0) {
+error_report("Another balloon device already registered");
+virtio_cleanup(&s->vdev);
+return NULL;
+}
+
 s->ivq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
 s->dvq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
 s->svq = virtio_add_queue(&s->vdev, 128, virtio_balloon_receive_stats);
 
 reset_stats(s);
-qemu_add_balloon_handler(virtio_balloon_to_target, virtio_balloon_stat, s);
 
 register_savevm(dev, "virtio-balloon", -1, 1,
 virtio_balloon_save, virtio_balloon_load, s);
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index d685243..ca5f125 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -788,6 +788,9 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
 VirtIODevice *vdev;
 
 vdev = virtio_balloon_init(&pci_dev->qdev);
+if (!vdev) {
+return -1;
+}
 virtio_init_pci(proxy, vdev);
 return 0;
 }
-- 
1.7.6




Re: [Qemu-devel] [PATCH] virtio-serial-bus: replay guest_open on migration

2011-07-27 Thread Alon Levy
On Wed, Jul 27, 2011 at 07:45:25AM +0200, Markus Armbruster wrote:
> Alon Levy  writes:
> 
> > Signed-off-by: Alon Levy 
> > ---
> >  hw/virtio-serial-bus.c |8 +++-
> >  1 files changed, 7 insertions(+), 1 deletions(-)
> >
> > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > index c5eb931..7a652ff 100644
> > --- a/hw/virtio-serial-bus.c
> > +++ b/hw/virtio-serial-bus.c
> > @@ -618,14 +618,20 @@ static int virtio_serial_load(QEMUFile *f, void 
> > *opaque, int version_id)
> >  for (i = 0; i < nr_active_ports; i++) {
> >  uint32_t id;
> >  bool host_connected;
> > +VirtIOSerialPortInfo *info;
> >  
> >  id = qemu_get_be32(f);
> >  port = find_port_by_id(s, id);
> >  if (!port) {
> >  return -EINVAL;
> >  }
> > -
> >  port->guest_connected = qemu_get_byte(f);
> > +info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info);
> > +if (port->guest_connected && info->guest_open) {
> > +/* replay guest open */
> > +info->guest_open(port);
> > +
> > +}
> >  host_connected = qemu_get_byte(f);
> >  if (host_connected != port->host_connected) {
> >  /*
> 
> The patch makes enough sense to me, but the commit message is
> insufficient.  Why do you have to replay?  And what's being fixed?

When migrating a host with with a spice agent running the mouse becomes
non operational after the migration. This is rhbz #718463, currently on
spice-server but it seems this is a qemu-kvm issue. The problem is that
after migration spice doesn't know the guest agent is open. Spice is just
a char dev here. And a chardev cannot query it's device, the device has
to let the chardev know when it is open. Right now after migration the
chardev which is recreated is in it's default state, which assumes the
guest is disconnected. Char devices carry no information across migration,
but the virtio-serial does already carry the guest_connected state. This
patch passes that bit to the chardev.



Re: [Qemu-devel] [PATCH 1/1] Fix typo for qmp-commands.hx. Signed-off-by: Zhi Yong Wu

2011-07-27 Thread Stefan Hajnoczi
On Wed, Jul 27, 2011 at 7:32 AM, Zhi Yong Wu  wrote:
> ---
>  qmp-commands.hx |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks, merged into the trivial-patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

A few tips on sending patches:

If there is only one patch, a cover letter email is usually not
necessary.  Just send a single patch email.

Usually the commit message should be ": ".
Your cover letter subject was fine ("qmp: fix typo for
qmp-commands.hx") and should be used as the subject for this patch.

In this particular patch email your Signed-off-by is in the email
subject, it should be at the bottom of the commit description.

If in doubt about the formatting of patch emails, I suggest sending to
yourself first and looking at the generated emails to check that they
come out alright.

I have applied this fix-ups to the commit:
http://repo.or.cz/w/qemu/stefanha.git/commitdiff/0b2e110b0689e9250da78c3bf7a1a67b378856c9

Stefan



Re: [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Kevin Wolf
Am 26.07.2011 18:57, schrieb Corey Bryant:
> 
> Kevin, thanks for the input.
> 
> On 07/26/2011 11:18 AM, Kevin Wolf wrote:
>> Am 26.07.2011 14:51, schrieb Corey Bryant:
  sVirt provides SELinux MAC isolation for Qemu guest processes and their
  corresponding resources (image files). 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, which is needed
  for image file isolation when using the sVirt SELinux security driver
  in libvirt.

  The proposed solution entails a combination of Qemu, libvirt, and
  SELinux patches that work together to isolate multiple guests' images
  when they're stored in the same NFS mount. This results in an
  environment where sVirt isolation and NFS image file isolation can both
  be provided.

  This patch contains the Qemu code to support this solution. I would
  like to solicit input from the libvirt community prior to starting
  the libvirt patch.

  Currently, Qemu opens an image file in addition to performing the
  necessary read and write operations. The proposed solution will move
  the open out of Qemu and into libvirt. Once libvirt opens an image
  file for the guest, it will pass the file descriptor to Qemu via a
  new fd: protocol.

  If the image file resides in an NFS mount, the following SELinux policy
  changes will provide image isolation:

 - A new SELinux boolean is created (e.g. virt_read_write_nfs) to
   allow Qemu (svirt_t) to only have SELinux read and write
   permissions on nfs_t files

 - Qemu (svirt_t) also gets SELinux use permissions on libvirt
   (virtd_t) file descriptors

  Following is a sample invocation of Qemu using the fd: protocol on
  the command line:

   qemu -drive file=fd:4,format=qcow2

  The fd: protocol is also supported by the drive_add monitor command.
  This requires that the specified file descriptor is passed to the
  monitor alongside a prior getfd monitor command.

  There are some additional features provided by certain image types
  where Qemu reopens the image file. All of these scenarios will be
  unsupported for the fd: protocol, at least for this patch:

 - The -snapshot command line option
 - The savevm monitor command
 - The snapshot_blkdev monitor command
 - Use of copy-on-write image files
 - The -cdrom command line option
 - The -drive command line option with media=cdrom
 - The change monitor command

  The thought is that this support can be added in the future, but is
  not required for the initial fd: support.

  This patch was tested with the following formats: raw, cow, qcow,
  qcow2, qed, and vmdk, using the fd: protocol from the command line
  and the monitor. Tests were also run to verify existing file name
  support and qemu-img were not regressed. Non-valid file descriptors,
  fd: without format, snapshot and backing files, and cdrom were also
  tested.

  v2:
 - Add drive_add monitor command support
 - Fence off unsupported features that re-open image file

  v3:
 - Fence off cdrom and change monitor command support

  Signed-off-by: Corey Bryant
  ---
block.c   |   16 ++
block.h   |1 +
block/cow.c   |5 +++
block/qcow.c  |5 +++
block/qcow2.c |5 +++
block/qed.c   |4 ++
block/raw-posix.c |   81 
 +++--
block/vmdk.c  |5 +++
block_int.h   |1 +
blockdev.c|   19 
monitor.c |5 +++
monitor.h |1 +
qemu-options.hx   |8 +++--
qemu-tool.c   |5 +++
14 files changed, 149 insertions(+), 12 deletions(-)

  diff --git a/block.c b/block.c
  index 24a25d5..500db84 100644
  --- a/block.c
  +++ b/block.c
  @@ -536,6 +536,10 @@ int bdrv_open(BlockDriverState *bs, const char 
 *filename, int flags,
char tmp_filename[PATH_MAX];
char backing_filename[PATH_MAX];

  +if (bdrv_is_fd_protocol(bs)) {
  +return -ENOTSUP;
  +}
>> Hm, shouldn't that just work even with fd?
>>
> 
> My thought was that the proposed SELinux changes would prevent the open 
> of the temporary backing file if the file corresponding to fd resides on 
> NFS.  But perhaps the backing file is not opened on NFS?

Depends on how broken your SELinux restrictions are. ;-)

I would argue that allowing qemu to create temporary files is a
r

Re: [Qemu-devel] [Qemu-trivial] [PATCH] Makefile: add missing deps on $(GENERATED_HEADERS)

2011-07-27 Thread Stefan Hajnoczi
On Tue, Jul 26, 2011 at 11:39:24AM -0500, Michael Roth wrote:
> This fixes a build issue with make -j6+ due to qapi-generated files
> being built before $(GENERATED_HEADERS) have been created.
> 
> Signed-off-by: Michael Roth 
> ---
>  Makefile |4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)

Thanks, applied to the trivial patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

Stefan



Re: [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Daniel P. Berrange
On Wed, Jul 27, 2011 at 10:11:06AM +0200, Kevin Wolf wrote:
> Am 26.07.2011 18:57, schrieb Corey Bryant:
>   diff --git a/block/cow.c b/block/cow.c
>   index 4cf543c..e17f8e7 100644
>   --- a/block/cow.c
>   +++ b/block/cow.c
>   @@ -82,6 +82,11 @@ static int cow_open(BlockDriverState *bs, int flags)
> pstrcpy(bs->backing_file, sizeof(bs->backing_file),
> cow_header.backing_file);
> 
>   +if (bs->backing_file[0] != '\0'&&  bdrv_is_fd_protocol(bs)) {
>   +/* backing file currently not supported by fd: protocol */
>   +goto fail;
>   +}
> >> I don't think these checks are strictly needed. Without the check you
> >> can open the image itself using an fd, and the backing file using good
> >> old raw-posix. We shouldn't decide for our users that this isn't useful.
> >>
> >> In any case, it would have to move into block.c, you can't modify
> >> independent drivers for this.
> >>
> > 
> > I understand the point on not modifying independent drivers.
> > 
> > But if the backing file resides on NFS, wouldn't the the proposed 
> > SELinux changes prevent the open?
> 
> Probably. But what about cases where the backing file is local? Or even
> a non-libvirt, non-SELinux use case?
> 
> > Or are you talking about support where libvirt opens the backing file 
> > and passes the fd to Qemu?  If so there was some discussion about future 
> > support for this here: 
> > http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg01496.html
> 
> Not really, but implementing this will be a bit easier if you don't
> forbid using backing files with fd.

In any case, for 'fd:' to be actually used by libvirt, we need to have
backing files supported. There are major users of libvirt who rely on
NFS and also use backing files, so an 'fd:' impl which can't deal with
the backing file problem isn't much use to libvirt.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



[Qemu-devel] [PATCH 0/4] Fix virtio memleaks

2011-07-27 Thread Amit Shah
The memory allocated in virtio_common_init() wasn't being freed
anywhere.  Fix that.

The balloon handler wasn't unregistering its savevm section,
adding an exit handler fixes that as well.

This patchset is on top of the two balloon series I've sent out
yesterday and today.

Amit Shah (4):
  virtio-balloon: Add exit handler, fix memleaks
  virtio-blk: Fix memleak on exit
  virtio-net: Fix potential use-after-free
  virtio: Plug memleak by freeing vdev

 hw/virtio-balloon.c |9 +
 hw/virtio-blk.c |1 +
 hw/virtio-net.c |2 +-
 hw/virtio-pci.c |   11 ++-
 hw/virtio.c |1 +
 hw/virtio.h |1 +
 6 files changed, 23 insertions(+), 2 deletions(-)

-- 
1.7.6




[Qemu-devel] [PATCH 1/4] virtio-balloon: Add exit handler, fix memleaks

2011-07-27 Thread Amit Shah
Add an exit handler that will free up RAM and unregister the savevm
section after a virtio-balloon device is unplugged.

Signed-off-by: Amit Shah 
---
 hw/virtio-balloon.c |9 +
 hw/virtio-pci.c |   11 ++-
 hw/virtio.h |1 +
 3 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 304a31a..054454b 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -45,6 +45,7 @@ typedef struct VirtIOBalloon
 size_t stats_vq_offset;
 MonitorCompletion *stats_callback;
 void *stats_opaque_callback_data;
+DeviceState *qdev;
 } VirtIOBalloon;
 
 static VirtIOBalloon *to_virtio_balloon(VirtIODevice *vdev)
@@ -293,8 +294,16 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
 
 reset_stats(s);
 
+s->qdev = dev;
 register_savevm(dev, "virtio-balloon", -1, 1,
 virtio_balloon_save, virtio_balloon_load, s);
 
 return &s->vdev;
 }
+
+void virtio_balloon_exit(VirtIODevice *vdev)
+{
+VirtIOBalloon *s = DO_UPCAST(VirtIOBalloon, vdev, vdev);
+unregister_savevm(s->qdev, "virtio-balloon", s);
+virtio_cleanup(vdev);
+}
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index ca5f125..316bf92 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -795,6 +795,15 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
 return 0;
 }
 
+static int virtio_balloon_exit_pci(PCIDevice *pci_dev)
+{
+VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+
+virtio_pci_stop_ioeventfd(proxy);
+virtio_balloon_exit(proxy->vdev);
+return virtio_exit_pci(pci_dev);
+}
+
 static PCIDeviceInfo virtio_info[] = {
 {
 .qdev.name = "virtio-blk-pci",
@@ -869,7 +878,7 @@ static PCIDeviceInfo virtio_info[] = {
 .qdev.alias = "virtio-balloon",
 .qdev.size = sizeof(VirtIOPCIProxy),
 .init  = virtio_balloon_init_pci,
-.exit  = virtio_exit_pci,
+.exit  = virtio_balloon_exit_pci,
 .vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET,
 .device_id = PCI_DEVICE_ID_VIRTIO_BALLOON,
 .revision  = VIRTIO_PCI_ABI_VERSION,
diff --git a/hw/virtio.h b/hw/virtio.h
index 0fd0bb0..c129264 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -213,6 +213,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf 
*conf);
 void virtio_net_exit(VirtIODevice *vdev);
 void virtio_blk_exit(VirtIODevice *vdev);
 void virtio_serial_exit(VirtIODevice *vdev);
+void virtio_balloon_exit(VirtIODevice *vdev);
 
 #define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
DEFINE_PROP_BIT("indirect_desc", _state, _field, \
-- 
1.7.6




[Qemu-devel] [PATCH 2/4] virtio-blk: Fix memleak on exit

2011-07-27 Thread Amit Shah
Calling virtio_cleanup() will free up memory allocated in
virtio_common_init().

Signed-off-by: Amit Shah 
---
 hw/virtio-blk.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 6471ac8..836dbc3 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -594,4 +594,5 @@ void virtio_blk_exit(VirtIODevice *vdev)
 {
 VirtIOBlock *s = to_virtio_blk(vdev);
 unregister_savevm(s->qdev, "virtio-blk", s);
+virtio_cleanup(vdev);
 }
-- 
1.7.6




[Qemu-devel] vt82686b query

2011-07-27 Thread Michael S. Tsirkin
Does one of you maintain the vt82686b emulation?
I was doing an overview of pci devices and have some questions on it:

vt82c686b_write_config - this seems to assume that
config writes are done using single byte accesses.
E.g. a two byte access at 0x84 will modify the
register at offset 0x85 but isn't handled by the
emulation.  Is this intentional?

PCI_STATUS and PCI_CAPABILITY_LIST are initialized
in the reset callback. These are readonly so
should go into init - there's no guarantee
reset is invoked in time to set these correctly,
is there?

PCI_CAPABILITY_LIST is a single byte register.
Better set it using pci_set_byte or simple memory access?
Higer bytes in that word are reserved so zeroing them out is
harmless, but still ...

via_pm_info has a config write method that simply
invokes the pci_default_write_config directly -
makes sense to remove it and save some lines of code?

Will you be able to review/test patches addressing the above?
Thanks!

-- 
MST



[Qemu-devel] [PATCH 4/4] virtio: Plug memleak by freeing vdev

2011-07-27 Thread Amit Shah
virtio_common_init() allocates RAM for the vdev struct (and any
additional memory, depending on the size passed to the function).  This
memory wasn't being freed until now.

Signed-off-by: Amit Shah 
---
 hw/virtio.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index a8f4940..93dfb1e 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -834,6 +834,7 @@ void virtio_cleanup(VirtIODevice *vdev)
 if (vdev->config)
 qemu_free(vdev->config);
 qemu_free(vdev->vq);
+qemu_free(vdev);
 }
 
 static void virtio_vmstate_change(void *opaque, int running, int reason)
-- 
1.7.6




[Qemu-devel] [PATCH 3/4] virtio-net: Fix potential use-after-free

2011-07-27 Thread Amit Shah
virtio_cleanup() will remove the VirtIONet struct that gets allocated
via virtio_common_init().  Ensure we don't dereference the structure
after calling the cleanup function.

Signed-off-by: Amit Shah 
---
 hw/virtio-net.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index a32cc01..3f10391 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -1073,6 +1073,6 @@ void virtio_net_exit(VirtIODevice *vdev)
 qemu_bh_delete(n->tx_bh);
 }
 
-virtio_cleanup(&n->vdev);
 qemu_del_vlan_client(&n->nic->nc);
+virtio_cleanup(&n->vdev);
 }
-- 
1.7.6




Re: [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Kevin Wolf
Am 27.07.2011 10:22, schrieb Daniel P. Berrange:
> On Wed, Jul 27, 2011 at 10:11:06AM +0200, Kevin Wolf wrote:
>> Am 26.07.2011 18:57, schrieb Corey Bryant:
>>  diff --git a/block/cow.c b/block/cow.c
>>  index 4cf543c..e17f8e7 100644
>>  --- a/block/cow.c
>>  +++ b/block/cow.c
>>  @@ -82,6 +82,11 @@ static int cow_open(BlockDriverState *bs, int flags)
>>pstrcpy(bs->backing_file, sizeof(bs->backing_file),
>>cow_header.backing_file);
>>
>>  +if (bs->backing_file[0] != '\0'&&  bdrv_is_fd_protocol(bs)) {
>>  +/* backing file currently not supported by fd: protocol */
>>  +goto fail;
>>  +}
 I don't think these checks are strictly needed. Without the check you
 can open the image itself using an fd, and the backing file using good
 old raw-posix. We shouldn't decide for our users that this isn't useful.

 In any case, it would have to move into block.c, you can't modify
 independent drivers for this.

>>>
>>> I understand the point on not modifying independent drivers.
>>>
>>> But if the backing file resides on NFS, wouldn't the the proposed 
>>> SELinux changes prevent the open?
>>
>> Probably. But what about cases where the backing file is local? Or even
>> a non-libvirt, non-SELinux use case?
>>
>>> Or are you talking about support where libvirt opens the backing file 
>>> and passes the fd to Qemu?  If so there was some discussion about future 
>>> support for this here: 
>>> http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg01496.html
>>
>> Not really, but implementing this will be a bit easier if you don't
>> forbid using backing files with fd.
> 
> In any case, for 'fd:' to be actually used by libvirt, we need to have
> backing files supported. There are major users of libvirt who rely on
> NFS and also use backing files, so an 'fd:' impl which can't deal with
> the backing file problem isn't much use to libvirt.

I'm perfectly aware of that. But seriously, repeating it over and over
again isn't going to make it happen any sooner. It requires -blockdev
which we may or may not have by 1.0.

Kevin



Re: [Qemu-devel] [PATCH 1/1] block/vpc.c: Detect too-large vpc file

2011-07-27 Thread Kevin Wolf
Am 26.07.2011 22:26, schrieb Serge E. Hallyn:
> Quoting Kevin Wolf (kw...@redhat.com):
>> Am 26.07.2011 18:08, schrieb Serge E. Hallyn:
>>> Quoting Kevin Wolf (kw...@redhat.com):
 Am 25.07.2011 20:34, schrieb Serge E. Hallyn:
> VHD files technically can be up to 2Tb, but virtual pc is limited
> to 127G.  Currently qemu-img refused to create vpc files > 127G,
> but it is failing to return error when converting from a non-vpc
> VHD file which is >127G.  It returns success, but creates a truncated
> converted image.  Also, qemu-img info claims the vpc file is 127G
> (and clean).
>
> This patch detects a too-large vpc file and returns -EFBIG.  Without
> this patch,
>
> =
> root@ip-10-38-123-242:~/qemu-fixed# qemu-img info /mnt/140g-dynamic.vhd 
> image: /mnt/140g-dynamic.vhd
> file format: vpc
> virtual size: 127G (13683600 bytes)
> disk size: 284K
> root@ip-10-38-123-242:~/qemu-fixed# qemu-img convert -f vpc -O raw 
> /mnt/140g-dynamic.vhd /mnt/y
> root@ip-10-38-123-242:~/qemu-fixed# echo $?
> 0
> root@ip-10-38-123-242:~/qemu-fixed# qemu-img info /mnt/y
> image: /mnt/y
> file format: raw
> virtual size: 127G (13683600 bytes)
> disk size: 0
> =
>
> (The 140G image was truncated with no warning or error.)
>
> With the patch, I get:
>
> =
> root@ip-10-38-123-242:~/qemu-fixed# ./qemu-img info /mnt/140g-dynamic.vhd 
> qemu-img: Could not open '/mnt/140g-dynamic.vhd': File too large
> root@ip-10-38-123-242:~/qemu-fixed# ./qemu-img convert -f vpc -O raw 
> /mnt/140g-dynamic.vhd /mnt/y
> qemu-img: Could not open '/mnt/140g-dynamic.vhd': File too large
> qemu-img: Could not open '/mnt/140g-dynamic.vhd'
> =
>
> See https://bugs.launchpad.net/qemu/+bug/814222 for details.
>
> Signed-off-by: Serge Hallyn 
> ---
>  block/vpc.c |8 +++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/block/vpc.c b/block/vpc.c
> index 56865da..fdd5236 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -156,6 +156,7 @@ static int vpc_open(BlockDriverState *bs, int flags)
>  struct vhd_dyndisk_header* dyndisk_header;
>  uint8_t buf[HEADER_SIZE];
>  uint32_t checksum;
> +int err = -1;
>  
>  if (bdrv_pread(bs->file, 0, s->footer_buf, HEADER_SIZE) != 
> HEADER_SIZE)
>  goto fail;
> @@ -176,6 +177,11 @@ static int vpc_open(BlockDriverState *bs, int flags)
>  bs->total_sectors = (int64_t)
>  be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl;
>  
> +if (bs->total_sectors >= 65535 * 16 * 255) {
> +err = -EFBIG;
> +goto fail;
> +}

 I wonder why this works. If bs->total_sectors was right, shouldn't it
>>>
>>> bs->total_sectors is exactly 65535 * 16 * 255.  It never gets bigger.
>>> So really the check could be
>>>
>>> if (bs->total_sectors == 65535 * 16 * 255) {
>>>
>>> and it would still work.
>>
>> Oh, now I understand. I think this is a bit too restrictive as it
>> forbids the largest possible size.
> 
> Yeah, it does.
> 
 have converted the full 140 GB? I can't see where else we would limit it
 to 127 GB, so what I had expected is that the CHS geometry stored in the
 image header is already too small.
>>>
>>> If I understand you correctly, what you're saying is exactly what I'm
>>> finding.  total_sectors is not reflective of the true size.
>>>
>>> I assume the true size is somewhere to be found :)  But I haven't found a
>>> nice spec for these.
>>
>> I think that footer->size contains the real size. Maybe we should do
>> something like this:
>> if (footer->size > 65536 * 16 * 255 * 512) {
>> bs->total_sectors = footer->size / 512;
>> } else {
>> bs->total_sectors = (int64_t)
>> be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl
>> }
> 
> The footer->size appears to be double the 'real' size.  So I'm actually doing
> the blow.  Does this seem sensible?

Double size sounds really weird. 'qemu-img create' uses the size in
bytes for it. Is that wrong?

> Doing it this way, trying to convert the image gives me '-EPERM' rather than
> -EFBIG.  Not sure where we are best off catching that.

Hm, any idea where the -EPERM (-1) comes from? Maybe wrong total_sectors
number you're calculating?

> Subject: [PATCH 1/1] vpc: accurately detect file size
> 
> VHD files technically can be up to 2Tb, but virtual pc uses CHS which
> is limited to 127G.  Currently qemu-img refused to create vpc files > 127G,
> but it reports any file >= 127G as being 127G.  If asked to convert such a
> file, i

Re: [Qemu-devel] [PATCH 3/4] virtio-net: Fix potential use-after-free

2011-07-27 Thread Michael S. Tsirkin
On Wed, Jul 27, 2011 at 02:00:31PM +0530, Amit Shah wrote:
> virtio_cleanup() will remove the VirtIONet struct that gets allocated
> via virtio_common_init().  Ensure we don't dereference the structure
> after calling the cleanup function.
> 
> Signed-off-by: Amit Shah 

I see. It's not a use after free but will be once
you make virtio_cleanup free the vdev?

> ---
>  hw/virtio-net.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index a32cc01..3f10391 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -1073,6 +1073,6 @@ void virtio_net_exit(VirtIODevice *vdev)
>  qemu_bh_delete(n->tx_bh);
>  }
>  
> -virtio_cleanup(&n->vdev);
>  qemu_del_vlan_client(&n->nic->nc);
> +virtio_cleanup(&n->vdev);
>  }
> -- 
> 1.7.6



Re: [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Daniel P. Berrange
On Wed, Jul 27, 2011 at 10:36:25AM +0200, Kevin Wolf wrote:
> Am 27.07.2011 10:22, schrieb Daniel P. Berrange:
> > On Wed, Jul 27, 2011 at 10:11:06AM +0200, Kevin Wolf wrote:
> >> Am 26.07.2011 18:57, schrieb Corey Bryant:
> >>  diff --git a/block/cow.c b/block/cow.c
> >>  index 4cf543c..e17f8e7 100644
> >>  --- a/block/cow.c
> >>  +++ b/block/cow.c
> >>  @@ -82,6 +82,11 @@ static int cow_open(BlockDriverState *bs, int 
> >> flags)
> >>pstrcpy(bs->backing_file, sizeof(bs->backing_file),
> >>cow_header.backing_file);
> >>
> >>  +if (bs->backing_file[0] != '\0'&&  bdrv_is_fd_protocol(bs)) {
> >>  +/* backing file currently not supported by fd: protocol */
> >>  +goto fail;
> >>  +}
>  I don't think these checks are strictly needed. Without the check you
>  can open the image itself using an fd, and the backing file using good
>  old raw-posix. We shouldn't decide for our users that this isn't useful.
> 
>  In any case, it would have to move into block.c, you can't modify
>  independent drivers for this.
> 
> >>>
> >>> I understand the point on not modifying independent drivers.
> >>>
> >>> But if the backing file resides on NFS, wouldn't the the proposed 
> >>> SELinux changes prevent the open?
> >>
> >> Probably. But what about cases where the backing file is local? Or even
> >> a non-libvirt, non-SELinux use case?
> >>
> >>> Or are you talking about support where libvirt opens the backing file 
> >>> and passes the fd to Qemu?  If so there was some discussion about future 
> >>> support for this here: 
> >>> http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg01496.html
> >>
> >> Not really, but implementing this will be a bit easier if you don't
> >> forbid using backing files with fd.
> > 
> > In any case, for 'fd:' to be actually used by libvirt, we need to have
> > backing files supported. There are major users of libvirt who rely on
> > NFS and also use backing files, so an 'fd:' impl which can't deal with
> > the backing file problem isn't much use to libvirt.
> 
> I'm perfectly aware of that. But seriously, repeating it over and over
> again isn't going to make it happen any sooner. It requires -blockdev
> which we may or may not have by 1.0.

Yes, I understand we need also -blockdev for this. Other messages in this
mail thread have impied that this proposed patch on its own is useful for
libvirt's requirements, so I just wanted to remind people in general that
we can't use this patch on its own until we have something like -blockdev.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



Re: [Qemu-devel] [PATCH 1/1] block/vpc.c: Detect too-large vpc file

2011-07-27 Thread Kevin Wolf
Am 26.07.2011 22:26, schrieb Serge E. Hallyn:
> Quoting Kevin Wolf (kw...@redhat.com):
>> I think that footer->size contains the real size. Maybe we should do
>> something like this:
>> if (footer->size > 65536 * 16 * 255 * 512) {
>> bs->total_sectors = footer->size / 512;
>> } else {
>> bs->total_sectors = (int64_t)
>> be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl
>> }
> 
> The footer->size appears to be double the 'real' size.  So I'm actually doing
> the blow.  Does this seem sensible?

Just looked it up in the spec:

"Current Size
This field stores the current size of the hard disk, in bytes, from the
perspective of the virtual machine.
This value is same as the original size when the hard disk is created.
This value can change depending on whether the hard disk is expanded."

And for the part about using CHS:

"When a hard disk is configured as an ATA hard disk, the CHS values
(that is, Cylinder, Heads, Sectors per track) are used by the ATA
controller to determine the size of the disk. When the user creates a
hard disk of a certain size, the size of the hard disk image in the
virtual machine is smaller than that created by the user. This is
because CHS value calculated from the hard disk size is rounded down."

Kevin



Re: [Qemu-devel] [PATCH 3/4] virtio-net: Fix potential use-after-free

2011-07-27 Thread Amit Shah
On (Wed) 27 Jul 2011 [11:43:44], Michael S. Tsirkin wrote:
> On Wed, Jul 27, 2011 at 02:00:31PM +0530, Amit Shah wrote:
> > virtio_cleanup() will remove the VirtIONet struct that gets allocated
> > via virtio_common_init().  Ensure we don't dereference the structure
> > after calling the cleanup function.
> > 
> > Signed-off-by: Amit Shah 
> 
> I see. It's not a use after free but will be once
> you make virtio_cleanup free the vdev?

Yes, the next patch.

Amit



Re: [Qemu-devel] [PATCH 04/25] Add hard build dependency on glib

2011-07-27 Thread Benjamin Herrenschmidt

> You're probably setting up your cross environment incorrectly which, 
> unfortunately, is very common.
> 
> The proper thing to do is to have GCC use a different system include 
> directory and a different prefix.  That will result in a directory where 
> there are gcc binaries with normal names installed in ${cross_prefix}/bin
> 
> You need to build and install pkg-config to this prefix too, and then 
> when it comes time to actually doing the QEMU configure, you should do 
> something like:
> 
> export PATH=${cross_prefix}/bin:$PATH
> export PKG_CONFIG_PATH=${cross_prefix}/lib/pkg-config:$PKG_CONFIG_PATH
> 
> Many automated cross compiler environment scripts will install specially 
> named versions of gcc and binutils in your normal $PATH.  The trouble 
> is, this is a bit of a hack and unless you know to make this hack work 
> with other build tools, it all comes tumbling down.

Well, that hard requirement is causing us problem on our 32/64-bit cross
builds as well.

It looks like glib (at least recent versions in -sid) can't be built
64-bit on a 32-bit system :-( At least not without fixing some horrid
bugs in there related to some generated include path from what David
says (I'll let him comment further).

In general, every time you add a library requirement without some config
option to disable it for cases such as ours, you add pain :-)

Now, in the specific case of glib, I understand why you would want to
get rid of re-invented wheels and use it, so I'm not specifically
criticizing that specific change, we'll eventually have to fix it
anyways. Just a heads up to be careful with hard requirements in
general.

Cheers,
Ben.





Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

2011-07-27 Thread Paolo Bonzini

On 07/26/2011 09:23 PM, Anthony Liguori wrote:

On 07/26/2011 01:26 PM, Paolo Bonzini wrote:

On 07/26/2011 05:34 PM, Anthony Liguori wrote:
You could just as well say that real life works like this:

class PciSocket {
PciBus *pci_bus;
uint8_t *config;
uint8_t *cmask;
uint8_t *wmask;
uint8_t *w1cmask;
uint8_t *used;
uint32_t devfn;
char name[64];
PCIIORegion io_regions[PCI_NUM_REGIONS];
...

};

class IsaSocket {
IsaBus *isa_bus;
uint32_t isairq[2]; // not sure this is a good idea, just
int nirqs; // mimicking qdev
uint16_t ioports[32];// statically assigned unlike PCI bars
int nioports;
}

class MyWeirdDevice : public MyBaseDevice {
PciSocket pci;
IsaSocket isa;
};


Hrm, I'm not sure I buy that entirely. I think it would be:

class MyWeirdPciView : public PciDevice {
PciBus *bus;
MyWeirdDevice *dev;
};

class MyWeirdIsaView : public IsaDevice {
IsaBus *bus;
MyWeirdDevice *dev;
};

class MyWeirdDevice : public MyBaseDevice {
MyWeirdPciView pci;
MyWeirdIsaView isa;
}

The views are basically bridges between PCI/ISA and an internal
interface (MyWeirdDevice).


That's yet another possibility.  There are two difference with what I 
had written:


1) the back-pointer from MyWeird*View to MyWeirdDevice replaced by 
container_of.  That's cosmetic;


2) I'm not considering the sockets to be separate devices.  That's the 
difference between "proxying PCI to another device" (wrong, hardware 
does not do that) and "isolating PCI knowledge within a sub-object of 
the device" (just an encapsulation choice, hardware is irrelevant).



I don't think the proxy design pattern is the right thing to use.
95% of the time, the device is intrinsically a PCI device.


It's not a proxy.  It's replacing inheritance with containment to get 
the flexibility of data MI without the complexity of diamond hierarchies.



The other 5% of the
time, the device has a well defined interface, and then there is
effectively a PCI bridge. But that PCI bridge isn't generic, it's
specific to that custom interface.


Yes, that can be represented by composition if you wish (an 
ISASerialState containing an 8250 is the canonical example; the 8139 
example below is another).



There's a pin in the IDE cable that determines master or slave depending
on whether it's raised high.


Yes, that's the "newer" way. There used to be jumpers to choose between
master, slave and cable-select.


That jumper raises the bit on the wire.


Ah ok, I was confusing it with the cable-select wire.  My point was that 
you can choose the jumper representation (low-level) or the cable-select 
representation (high-level, actually matches modern hardware, even 
better from the user POV).  One is easier to manage and better in all 
aspects, but both make sense.  It's irrelevant to this discussion anyway.



Interfaces are the right way to do this. Getting MI right is fairly
hard


But we don't need is-a, we need has-a. Multiple is-a is harder than
single is-a. Multiple has-a does not add any complication.


Yeah, that's what plug properties are for :-)


I agree, but at the cost of pointer chasing and making it harder to
implement get_device_for_socket for buses that need it (in the above
sketch it can be a simple container_of).


Can we be a bit more concrete as I'm having a hard time following your
logic. You're assuming a generic PciSocket class, right? I think that's
not the right approach, as an example:

class Rtl8139PciBridge : public PciDevice
{
Rtl8139 rtldev;
};

class Rtl8139IsaBridge : public IsaDevice
{
Rtl8139 rtldev;
};

With Avi's new memory API, we'd have:

class Rtl8139 : public Device
{
MemoryRegion region[2];
Pin irq;
};

And then the construction code for Rtl8139PciBridge would register the
regions as bars, and connect the PCI lnk to rtldev.irq.

The ISA code would do something similar.


Yes, this looks nice (modulo s/Rtl8139/Rtl8139 */).  But it is not that 
much more flexible than qdev 1.0.


You're right that for the case of two parents above we were looking at a 
contrived example.  The Goldfish platform provides a more interesting 
one.  There you have an interrupt controller and a bus enumerator 
device.  Most devices are connected to both, but conflating them is 
wrong for many reasons;


1) the bus enumerator itself uses an interrupt (raised on hotplug);

2) you can enumerate devices that do not have an interrupt line, and you 
shouldn't need to connect such a device to an interrupt controller;


3) the interrupt controller and bus enumerator use two separate MMIO areas;

4) in principle, other devices could use the interrupt controller (which 
is the only component connected to the CPU's interrupt line) without 
being enumerated.


5) A device with two interrupts has two "interrupt interfaces" and only 
one "enumerator interface".


6) The enumerator interface does not have bus semantics.  The enumerator 
also enumerates itself so it would act as both the bus host and a device 
on the bus.


Composition then lets you use something like this:

 

Re: [Qemu-devel] [PATCH 1/2] xen_mapcache: remove unused variable

2011-07-27 Thread Stefan Hajnoczi
On Mon, Jul 11, 2011 at 06:15:11PM +0200, Juan Quintela wrote:
> 
> Signed-off-by: Juan Quintela 
> ---
>  xen-mapcache.c |3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)

Since Stefano has acked it but it was not merged into qemu.git yet...

Thanks, applied to the trivial patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches

Stefan



Re: [Qemu-devel] [PATCH 2/4] virtio-blk: Fix memleak on exit

2011-07-27 Thread Kevin Wolf
Am 27.07.2011 10:30, schrieb Amit Shah:
> Calling virtio_cleanup() will free up memory allocated in
> virtio_common_init().
> 
> Signed-off-by: Amit Shah 

Acked-by: Kevin Wolf 



Re: [Qemu-devel] [PATCH 2/2] virtio-balloon: Check if balloon registration failed

2011-07-27 Thread Michael S. Tsirkin
On Wed, Jul 27, 2011 at 12:31:08PM +0530, Amit Shah wrote:
> Multiple balloon registrations are not allowed; check if the
> registration with the qemu balloon api succeeded.  If not, fail the
> device init.
> 
> Signed-off-by: Amit Shah 
> ---
>  hw/virtio-balloon.c |   10 +-
>  hw/virtio-pci.c |3 +++
>  2 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> index 2ba7e95..304a31a 100644
> --- a/hw/virtio-balloon.c
> +++ b/hw/virtio-balloon.c
> @@ -269,6 +269,7 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, 
> int version_id)
>  VirtIODevice *virtio_balloon_init(DeviceState *dev)
>  {
>  VirtIOBalloon *s;
> +int ret;
>  
>  s = (VirtIOBalloon *)virtio_common_init("virtio-balloon",
>  VIRTIO_ID_BALLOON,
> @@ -278,12 +279,19 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
>  s->vdev.set_config = virtio_balloon_set_config;
>  s->vdev.get_features = virtio_balloon_get_features;
>  
> +ret = qemu_add_balloon_handler(virtio_balloon_to_target,
> +   virtio_balloon_stat, s);
> +if (ret < 0) {
> +error_report("Another balloon device already registered");

We don't know why it failed at this point. I think
error report should be triggered by qemu_add_balloon_handler,
and this function would just cleanup and exit.


> +virtio_cleanup(&s->vdev);
> +return NULL;
> +}
> +
>  s->ivq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
>  s->dvq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
>  s->svq = virtio_add_queue(&s->vdev, 128, virtio_balloon_receive_stats);
>  
>  reset_stats(s);
> -qemu_add_balloon_handler(virtio_balloon_to_target, virtio_balloon_stat, 
> s);
>  
>  register_savevm(dev, "virtio-balloon", -1, 1,
>  virtio_balloon_save, virtio_balloon_load, s);
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index d685243..ca5f125 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -788,6 +788,9 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
>  VirtIODevice *vdev;
>  
>  vdev = virtio_balloon_init(&pci_dev->qdev);
> +if (!vdev) {
> +return -1;
> +}
>  virtio_init_pci(proxy, vdev);
>  return 0;
>  }
> -- 
> 1.7.6
> 



Re: [Qemu-devel] [PATCH 0/4] Fix virtio memleaks

2011-07-27 Thread Michael S. Tsirkin
On Wed, Jul 27, 2011 at 02:00:28PM +0530, Amit Shah wrote:
> The memory allocated in virtio_common_init() wasn't being freed
> anywhere.  Fix that.
> 
> The balloon handler wasn't unregistering its savevm section,
> adding an exit handler fixes that as well.
> 
> This patchset is on top of the two balloon series I've sent out
> yesterday and today.

This looks good to me. What do you say I put
patches 2-4 on my tree, and you put patch 1 on yours?

> Amit Shah (4):
>   virtio-balloon: Add exit handler, fix memleaks
>   virtio-blk: Fix memleak on exit
>   virtio-net: Fix potential use-after-free
>   virtio: Plug memleak by freeing vdev
> 
>  hw/virtio-balloon.c |9 +
>  hw/virtio-blk.c |1 +
>  hw/virtio-net.c |2 +-
>  hw/virtio-pci.c |   11 ++-
>  hw/virtio.c |1 +
>  hw/virtio.h |1 +
>  6 files changed, 23 insertions(+), 2 deletions(-)
> 
> -- 
> 1.7.6
> 



[Qemu-devel] [Bug 816860] [NEW] Guest machine freezes when NFS mount goes offline

2011-07-27 Thread Igor Blanco
Public bug reported:

I have a virtual KVM machine that has 2 CDROM units with ISOs mounted
from a NFS mount point. When NFS server goes offline the virtual machine
blocks completely instead of throwing read errors for the CDROM device.

Host: Proxmox VE 1.8-11 (Debian GNU/Linux 5.0)
KVM commandline version: QEMU emulator version 0.14.1 (qemu-kvm-devel)
Guest: Windows 7 professional SP 1

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/816860

Title:
  Guest machine freezes when NFS mount goes offline

Status in QEMU:
  New

Bug description:
  I have a virtual KVM machine that has 2 CDROM units with ISOs mounted
  from a NFS mount point. When NFS server goes offline the virtual
  machine blocks completely instead of throwing read errors for the
  CDROM device.

  Host: Proxmox VE 1.8-11 (Debian GNU/Linux 5.0)
  KVM commandline version: QEMU emulator version 0.14.1 (qemu-kvm-devel)
  Guest: Windows 7 professional SP 1

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/816860/+subscriptions



[Qemu-devel] [PATCH 7/7] xen_mapcache: remove unused variable

2011-07-27 Thread Stefan Hajnoczi
From: Juan Quintela 

Signed-off-by: Juan Quintela 
Acked-by: Stefano Stabellini 
Signed-off-by: Stefan Hajnoczi 
---
 xen-mapcache.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/xen-mapcache.c b/xen-mapcache.c
index 007136a..15d1241 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -237,7 +237,7 @@ uint8_t *xen_map_cache(target_phys_addr_t phys_addr, 
target_phys_addr_t size,
 
 ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
 {
-MapCacheEntry *entry = NULL, *pentry = NULL;
+MapCacheEntry *entry = NULL;
 MapCacheRev *reventry;
 target_phys_addr_t paddr_index;
 target_phys_addr_t size;
@@ -263,7 +263,6 @@ ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
 
 entry = &mapcache->entry[paddr_index % mapcache->nr_buckets];
 while (entry && (entry->paddr_index != paddr_index || entry->size != 
size)) {
-pentry = entry;
 entry = entry->next;
 }
 if (!entry) {
-- 
1.7.5.4




[Qemu-devel] [PATCH 6/7] Makefile: add missing deps on $(GENERATED_HEADERS)

2011-07-27 Thread Stefan Hajnoczi
From: Michael Roth 

This fixes a build issue with make -j6+ due to qapi-generated files
being built before $(GENERATED_HEADERS) have been created.

Tested-by: Stefan Berger 
Signed-off-by: Michael Roth 
Signed-off-by: Stefan Hajnoczi 
---
 Makefile |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index cbd614a..daae310 100644
--- a/Makefile
+++ b/Makefile
@@ -192,8 +192,10 @@ test-qmp-commands.o: $(addprefix $(qapi-dir)/, 
test-qapi-types.c test-qapi-types
 test-qmp-commands: test-qmp-commands.o qfloat.o qint.o qdict.o qstring.o 
qlist.o qbool.o $(qapi-obj-y) error.o osdep.o qemu-malloc.o $(oslib-obj-y) 
qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o 
qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o 
$(qapi-dir)/test-qmp-marshal.o module.o
 
 QGALIB=qga/guest-agent-command-state.o qga/guest-agent-commands.o
+QGALIB_GEN=$(addprefix $(qapi-dir)/, qga-qapi-types.c qga-qapi-types.h 
qga-qapi-visit.c qga-qmp-marshal.c)
 
-qemu-ga.o: $(addprefix $(qapi-dir)/, qga-qapi-types.c qga-qapi-types.h 
qga-qapi-visit.c qga-qmp-marshal.c) $(qapi-obj-y)
+$(QGALIB_GEN): $(GENERATED_HEADERS)
+$(QGALIB) qemu-ga.o: $(QGALIB_GEN) $(qapi-obj-y)
 qemu-ga$(EXESUF): qemu-ga.o $(QGALIB) qemu-tool.o qemu-error.o error.o 
$(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) 
$(qapi-obj-y) qemu-timer-common.o qemu-sockets.o module.o qapi/qmp-dispatch.o 
qapi/qmp-registry.o $(qapi-dir)/qga-qapi-visit.o $(qapi-dir)/qga-qapi-types.o 
$(qapi-dir)/qga-qmp-marshal.o
 
 QEMULIBS=libhw32 libhw64 libuser libdis libdis-user
-- 
1.7.5.4




Re: [Qemu-devel] [PATCH] virtio-serial-bus: replay guest_open on migration

2011-07-27 Thread Markus Armbruster
Alon Levy  writes:

> On Wed, Jul 27, 2011 at 07:45:25AM +0200, Markus Armbruster wrote:
>> Alon Levy  writes:
>> 
>> > Signed-off-by: Alon Levy 
>> > ---
>> >  hw/virtio-serial-bus.c |8 +++-
>> >  1 files changed, 7 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
>> > index c5eb931..7a652ff 100644
>> > --- a/hw/virtio-serial-bus.c
>> > +++ b/hw/virtio-serial-bus.c
>> > @@ -618,14 +618,20 @@ static int virtio_serial_load(QEMUFile *f, void 
>> > *opaque, int version_id)
>> >  for (i = 0; i < nr_active_ports; i++) {
>> >  uint32_t id;
>> >  bool host_connected;
>> > +VirtIOSerialPortInfo *info;
>> >  
>> >  id = qemu_get_be32(f);
>> >  port = find_port_by_id(s, id);
>> >  if (!port) {
>> >  return -EINVAL;
>> >  }
>> > -
>> >  port->guest_connected = qemu_get_byte(f);
>> > +info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info);
>> > +if (port->guest_connected && info->guest_open) {
>> > +/* replay guest open */
>> > +info->guest_open(port);
>> > +
>> > +}
>> >  host_connected = qemu_get_byte(f);
>> >  if (host_connected != port->host_connected) {
>> >  /*
>> 
>> The patch makes enough sense to me, but the commit message is
>> insufficient.  Why do you have to replay?  And what's being fixed?
>
> When migrating a host with with a spice agent running the mouse becomes
> non operational after the migration. This is rhbz #718463, currently on
> spice-server but it seems this is a qemu-kvm issue. The problem is that
> after migration spice doesn't know the guest agent is open. Spice is just
> a char dev here. And a chardev cannot query it's device, the device has
> to let the chardev know when it is open. Right now after migration the
> chardev which is recreated is in it's default state, which assumes the
> guest is disconnected. Char devices carry no information across migration,
> but the virtio-serial does already carry the guest_connected state. This
> patch passes that bit to the chardev.

Put this information in the commit message and resend?



[Qemu-devel] [PATCH 0/6] Add various VMDK subformats support

2011-07-27 Thread Fam Zheng
Add subformats support for:
twoGbMaxExtentFlat
twoGbMaxExtentSparse
streamOptimized

Fam Zheng (6):
  VMDK: enable twoGbMaxExtentFlat
  VMDK: add twoGbMaxExtentSparse support
  VMDK: separate vmdk_read_extent/vmdk_write_extent
  VMDK: Opening compressed extent.
  VMDK: read/write compressed extent
  VMDK: creating streamOptimized subformat

 block/vmdk.c |  316 +-
 1 files changed, 248 insertions(+), 68 deletions(-)




[Qemu-devel] [PATCH 1/6] VMDK: enable twoGbMaxExtentFlat

2011-07-27 Thread Fam Zheng
Enable the createType 'twoGbMaxExtentFlat'. The supporting code is
already in.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 37478d2..9e6c67a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -551,7 +551,8 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
flags)
 if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
 return -EINVAL;
 }
-if (strcmp(ct, "monolithicFlat")) {
+if (strcmp(ct, "monolithicFlat") &&
+strcmp(ct, "twoGbMaxExtentFlat")) {
 fprintf(stderr,
 "VMDK: Not supported image type \"%s\""".\n", ct);
 return -ENOTSUP;



[Qemu-devel] [PATCH 2/6] VMDK: add twoGbMaxExtentSparse support

2011-07-27 Thread Fam Zheng
Add twoGbMaxExtentSparse support. Only opening code is changed.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |  124 --
 1 files changed, 77 insertions(+), 47 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 9e6c67a..9d1ae32 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -357,7 +357,9 @@ static int vmdk_init_tables(BlockDriverState *bs, 
VmdkExtent *extent)
 return ret;
 }
 
-static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
+static int vmdk_open_vmdk3(BlockDriverState *bs,
+   BlockDriverState *file,
+   int flags)
 {
 int ret;
 uint32_t magic;
@@ -365,10 +367,9 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
 BDRVVmdkState *s = bs->opaque;
 VmdkExtent *extent;
 
-s->desc_offset = 0x200;
-ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
+ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
 if (ret < 0) {
-goto fail;
+return ret;
 }
 extent = vmdk_add_extent(bs,
  bs->file, false,
@@ -378,17 +379,16 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int 
flags)
  le32_to_cpu(header.granularity));
 ret = vmdk_init_tables(bs, extent);
 if (ret) {
-/* vmdk_init_tables cleans up on fail, so only free allocation of
- * vmdk_add_extent here. */
-goto fail;
+/* free extent allocated by vmdk_add_extent */
+s->num_extents--;
+qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
 }
-return 0;
- fail:
-vmdk_free_extents(bs);
 return ret;
 }
 
-static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
+static int vmdk_open_vmdk4(BlockDriverState *bs,
+   BlockDriverState *file,
+   int flags)
 {
 int ret;
 uint32_t magic;
@@ -397,39 +397,30 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int 
flags)
 BDRVVmdkState *s = bs->opaque;
 VmdkExtent *extent;
 
-s->desc_offset = 0x200;
-ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
+ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
 if (ret < 0) {
-goto fail;
+return ret;
 }
 l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
 * le64_to_cpu(header.granularity);
+if (l1_entry_sectors <= 0) {
+return -EINVAL;
+}
 l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
 / l1_entry_sectors;
-extent = vmdk_add_extent(bs, bs->file, false,
+extent = vmdk_add_extent(bs, file, false,
   le64_to_cpu(header.capacity),
   le64_to_cpu(header.gd_offset) << 9,
   le64_to_cpu(header.rgd_offset) << 9,
   l1_size,
   le32_to_cpu(header.num_gtes_per_gte),
   le64_to_cpu(header.granularity));
-if (extent->l1_entry_sectors <= 0) {
-ret = -EINVAL;
-goto fail;
-}
-/* try to open parent images, if exist */
-ret = vmdk_parent_open(bs);
-if (ret) {
-goto fail;
-}
-s->parent_cid = vmdk_read_cid(bs, 1);
 ret = vmdk_init_tables(bs, extent);
 if (ret) {
-goto fail;
+/* free extent allocated by vmdk_add_extent */
+s->num_extents--;
+qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
 }
-return 0;
- fail:
-vmdk_free_extents(bs);
 return ret;
 }
 
@@ -460,6 +451,35 @@ static int vmdk_parse_description(const char *desc, const 
char *opt_name,
 return 0;
 }
 
+/* Open an extent file and append to bs array */
+static int vmdk_open_sparse(BlockDriverState *bs,
+BlockDriverState *file,
+int flags)
+{
+int ret;
+uint32_t magic;
+
+if (bdrv_pread(file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
+return -EIO;
+}
+
+magic = be32_to_cpu(magic);
+if (magic == VMDK3_MAGIC) {
+ret = vmdk_open_vmdk3(bs, file, flags);
+if (ret) {
+return ret;
+}
+} else if (magic == VMDK4_MAGIC) {
+ret = vmdk_open_vmdk4(bs, file, flags);
+if (ret) {
+return ret;
+}
+} else {
+return -EINVAL;
+}
+return 0;
+}
+
 static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
 const char *desc_file_path)
 {
@@ -470,6 +490,8 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 const char *p = desc;
 int64_t sectors = 0;
 int64_t flat_offset;
+char extent_path[PATH_MAX];
+BlockDriverState *extent_file;
 
 while (*p) {
 /* parse extent line:
@@ -504,22 +526,28 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,

[Qemu-devel] [PATCH 3/6] VMDK: separate vmdk_read_extent/vmdk_write_extent

2011-07-27 Thread Fam Zheng
Factor out read/write extent code, since there will be more things to
take care of once reading/writing compressed clusters is introduced.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |   54 +-
 1 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 9d1ae32..0d989f6 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -829,6 +829,43 @@ static int vmdk_is_allocated(BlockDriverState *bs, int64_t 
sector_num,
 return ret;
 }
 
+static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
+int64_t offset_in_cluster, const uint8_t *buf,
+int nb_sectors, int64_t sector_num)
+{
+int ret;
+const uint8_t *write_buf = buf;
+int write_len = nb_sectors * 512;
+
+ret = bdrv_pwrite(extent->file,
+cluster_offset + offset_in_cluster,
+write_buf,
+write_len);
+if (ret != write_len) {
+ret = ret < 0 ? ret : -EIO;
+goto out;
+}
+ret = 0;
+ out:
+return ret;
+}
+
+static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
+int64_t offset_in_cluster, uint8_t *buf,
+int nb_sectors)
+{
+int ret;
+
+ret = bdrv_pread(extent->file,
+  cluster_offset + offset_in_cluster,
+  buf, nb_sectors * 512);
+if (ret == nb_sectors * 512) {
+return 0;
+} else {
+return -EIO;
+}
+}
+
 static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
 uint8_t *buf, int nb_sectors)
 {
@@ -865,10 +902,10 @@ static int vmdk_read(BlockDriverState *bs, int64_t 
sector_num,
 memset(buf, 0, 512 * n);
 }
 } else {
-ret = bdrv_pread(extent->file,
-cluster_offset + index_in_cluster * 512,
-buf, n * 512);
-if (ret < 0) {
+ret = vmdk_read_extent(extent,
+cluster_offset, index_in_cluster * 512,
+buf, n);
+if (ret) {
 return ret;
 }
 }
@@ -917,11 +954,10 @@ static int vmdk_write(BlockDriverState *bs, int64_t 
sector_num,
 n = nb_sectors;
 }
 
-ret = bdrv_pwrite(extent->file,
-cluster_offset + index_in_cluster * 512,
-buf,
-n * 512);
-if (ret < 0) {
+ret = vmdk_write_extent(extent,
+cluster_offset, index_in_cluster * 512,
+buf, n, sector_num);
+if (ret) {
 return ret;
 }
 if (m_data.valid) {



[Qemu-devel] [PATCH 4/6] VMDK: Opening compressed extent.

2011-07-27 Thread Fam Zheng
Added flags field for compressed/streamOptimized extents, open and save
image configuration.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |   16 
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 0d989f6..5f1638e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -26,9 +26,13 @@
 #include "qemu-common.h"
 #include "block_int.h"
 #include "module.h"
+#include "zlib.h"
 
 #define VMDK3_MAGIC (('C' << 24) | ('O' << 16) | ('W' << 8) | 'D')
 #define VMDK4_MAGIC (('K' << 24) | ('D' << 16) | ('M' << 8) | 'V')
+#define VMDK4_COMPRESSION_DEFLATE 1
+#define VMDK4_FLAG_COMPRESS (1 << 16)
+#define VMDK4_FLAG_MARKER (1 << 17)
 
 typedef struct {
 uint32_t version;
@@ -56,6 +60,7 @@ typedef struct {
 int64_t grain_offset;
 char filler[1];
 char check_bytes[4];
+uint16_t compressAlgorithm;
 } __attribute__((packed)) VMDK4Header;
 
 #define L2_CACHE_SIZE 16
@@ -63,6 +68,8 @@ typedef struct {
 typedef struct VmdkExtent {
 BlockDriverState *file;
 bool flat;
+bool compressed;
+bool has_marker;
 int64_t sectors;
 int64_t end_sector;
 int64_t flat_start_offset;
@@ -98,6 +105,12 @@ typedef struct VmdkMetaData {
 int valid;
 } VmdkMetaData;
 
+typedef struct VmdkGrainMarker {
+uint64_t lba;
+uint32_t size;
+uint8_t  data[0];
+} VmdkGrainMarker;
+
 static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
 uint32_t magic;
@@ -415,6 +428,9 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
   l1_size,
   le32_to_cpu(header.num_gtes_per_gte),
   le64_to_cpu(header.granularity));
+extent->compressed =
+le16_to_cpu(header.compressAlgorithm) == VMDK4_COMPRESSION_DEFLATE;
+extent->has_marker = header.flags & VMDK4_FLAG_MARKER;
 ret = vmdk_init_tables(bs, extent);
 if (ret) {
 /* free extent allocated by vmdk_add_extent */



[Qemu-devel] [PATCH 5/6] VMDK: read/write compressed extent

2011-07-27 Thread Fam Zheng
Add support for reading/writing compressed extent.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |  115 --
 1 files changed, 103 insertions(+), 12 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 5f1638e..4799aa5 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -771,10 +771,12 @@ static int get_cluster_offset(BlockDriverState *bs,
 
 /* Avoid the L2 tables update for the images that have snapshots. */
 *cluster_offset = bdrv_getlength(extent->file);
-bdrv_truncate(
-extent->file,
-*cluster_offset + (extent->cluster_sectors << 9)
-);
+if (!extent->compressed) {
+bdrv_truncate(
+extent->file,
+*cluster_offset + (extent->cluster_sectors << 9)
+);
+}
 
 *cluster_offset >>= 9;
 tmp = cpu_to_le32(*cluster_offset);
@@ -850,9 +852,28 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t 
cluster_offset,
 int nb_sectors, int64_t sector_num)
 {
 int ret;
+VmdkGrainMarker *data = NULL;
+uLongf buf_len;
 const uint8_t *write_buf = buf;
 int write_len = nb_sectors * 512;
 
+if (extent->compressed) {
+if (!extent->has_marker) {
+ret = -EINVAL;
+goto out;
+}
+buf_len = (extent->cluster_sectors << 9) * 2;
+data = qemu_mallocz(buf_len + sizeof(VmdkGrainMarker));
+if (compress(data->data, &buf_len, buf, nb_sectors << 9) != Z_OK ||
+buf_len == 0) {
+ret = -EINVAL;
+goto out;
+}
+data->lba = sector_num;
+data->size = buf_len;
+write_buf = (uint8_t *)data;
+write_len = buf_len + sizeof(VmdkGrainMarker);
+}
 ret = bdrv_pwrite(extent->file,
 cluster_offset + offset_in_cluster,
 write_buf,
@@ -863,6 +884,9 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t 
cluster_offset,
 }
 ret = 0;
  out:
+if (data) {
+qemu_free(data);
+}
 return ret;
 }
 
@@ -871,15 +895,65 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t 
cluster_offset,
 int nb_sectors)
 {
 int ret;
-
+int cluster_bytes, buf_bytes;
+uint8_t *cluster_buf, *compressed_data;
+uint8_t *uncomp_buf;
+uint32_t data_len;
+VmdkGrainMarker *marker;
+uLongf buf_len;
+ 
+
+if (!extent->compressed) {
+ret = bdrv_pread(extent->file,
+  cluster_offset + offset_in_cluster,
+  buf, nb_sectors * 512);
+if (ret == nb_sectors * 512) {
+return 0;
+} else {
+return -EIO;
+}
+}
+cluster_bytes = extent->cluster_sectors * 512;
+/* Read two clusters in case GrainMarker + compressed data > one cluster */
+buf_bytes = cluster_bytes * 2;
+cluster_buf = qemu_mallocz(buf_bytes);
+uncomp_buf = qemu_mallocz(cluster_bytes);
 ret = bdrv_pread(extent->file,
-  cluster_offset + offset_in_cluster,
-  buf, nb_sectors * 512);
-if (ret == nb_sectors * 512) {
-return 0;
-} else {
-return -EIO;
+cluster_offset,
+cluster_buf, buf_bytes);
+if (ret < 0) {
+goto out;
+}
+compressed_data = cluster_buf;
+buf_len = cluster_bytes;
+data_len = cluster_bytes;
+if (extent->has_marker) {
+marker = (VmdkGrainMarker *)cluster_buf;
+compressed_data = marker->data;
+data_len = marker->size;
+}
+if (!data_len || data_len > buf_bytes) {
+ret = -EINVAL;
+goto out;
+}
+ret = uncompress(uncomp_buf, &buf_len, compressed_data, data_len);
+if (ret != Z_OK) {
+ret = -EINVAL;
+goto out;
+
+}
+if (offset_in_cluster < 0 ||
+offset_in_cluster + nb_sectors * 512 > buf_len) {
+ret = -EINVAL;
+goto out;
 }
+memcpy(buf, uncomp_buf + offset_in_cluster, nb_sectors * 512);
+ret = 0;
+
+ out:
+qemu_free(uncomp_buf);
+qemu_free(cluster_buf);
+return ret;
 }
 
 static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
@@ -959,8 +1033,25 @@ static int vmdk_write(BlockDriverState *bs, int64_t 
sector_num,
 bs,
 extent,
 &m_data,
-sector_num << 9, 1,
+sector_num << 9, !extent->compressed,
 &cluster_offset);
+if (extent->compressed) {
+if (ret == 0) {
+/* Refuse write to allocated cluster for streamOptimized */
+fprintf(stderr,
+"VMDK: can't write to allocated cluster"
+" for streamOptimized\n");
+

[Qemu-devel] [PATCH 6/6] VMDK: creating streamOptimized subformat

2011-07-27 Thread Fam Zheng
Creating streamOptimized subformat. Added subformat option
'streamOptimized', to create a image with compression enabled and each
cluster with a GrainMarker.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |   18 --
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 4799aa5..52e518d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1088,7 +1088,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t 
sector_num,
 }
 
 
-static int vmdk_create_extent(const char *filename, int64_t filesize, bool 
flat)
+static int vmdk_create_extent(const char *filename, int64_t filesize,
+  bool flat, bool compress)
 {
 int ret, i;
 int fd = 0;
@@ -1112,7 +1113,9 @@ static int vmdk_create_extent(const char *filename, 
int64_t filesize, bool flat)
 magic = cpu_to_be32(VMDK4_MAGIC);
 memset(&header, 0, sizeof(header));
 header.version = 1;
-header.flags = 3; /* ?? */
+header.flags =
+3 | (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0);
+header.compressAlgorithm = compress ? VMDK4_COMPRESSION_DEFLATE : 0;
 header.capacity = filesize / 512;
 header.granularity = 128;
 header.num_gtes_per_gte = 512;
@@ -1142,6 +1145,7 @@ static int vmdk_create_extent(const char *filename, 
int64_t filesize, bool flat)
 header.rgd_offset = cpu_to_le64(header.rgd_offset);
 header.gd_offset = cpu_to_le64(header.gd_offset);
 header.grain_offset = cpu_to_le64(header.grain_offset);
+header.compressAlgorithm = cpu_to_le16(header.compressAlgorithm);
 
 header.check_bytes[0] = 0xa;
 header.check_bytes[1] = 0x20;
@@ -1283,7 +1287,7 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 const char *fmt = NULL;
 int flags = 0;
 int ret = 0;
-bool flat, split;
+bool flat, split, compress;
 char ext_desc_lines[BUF_SIZE] = "";
 char path[PATH_MAX], prefix[PATH_MAX], postfix[PATH_MAX];
 const int64_t split_size = 0x8000;  /* VMDK has constant split size */
@@ -1332,7 +1336,8 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 } else if (strcmp(fmt, "monolithicFlat") &&
strcmp(fmt, "monolithicSparse") &&
strcmp(fmt, "twoGbMaxExtentSparse") &&
-   strcmp(fmt, "twoGbMaxExtentFlat")) {
+   strcmp(fmt, "twoGbMaxExtentFlat") &&
+   strcmp(fmt, "streamOptimized")) {
 fprintf(stderr, "VMDK: Unknown subformat: %s\n", fmt);
 return -EINVAL;
 }
@@ -1340,6 +1345,7 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
   strcmp(fmt, "twoGbMaxExtentSparse"));
 flat = !(strcmp(fmt, "monolithicFlat") &&
  strcmp(fmt, "twoGbMaxExtentFlat"));
+compress = !strcmp(fmt, "streamOptimized");
 if (flat) {
 desc_extent_line = "RW %lld FLAT \"%s\" 0\n";
 } else {
@@ -1394,7 +1400,7 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 snprintf(ext_filename, sizeof(ext_filename), "%s%s",
 path, desc_filename);
 
-if (vmdk_create_extent(ext_filename, size, flat)) {
+if (vmdk_create_extent(ext_filename, size, flat, compress)) {
 return -EINVAL;
 }
 filesize -= size;
@@ -1508,7 +1514,7 @@ static QEMUOptionParameter vmdk_create_options[] = {
 .type = OPT_STRING,
 .help =
 "VMDK flat extent format, can be one of "
-"{monolithicSparse (default) | monolithicFlat | 
twoGbMaxExtentSparse | twoGbMaxExtentFlat} "
+"{monolithicSparse (default) | monolithicFlat | 
twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} "
 },
 { NULL }
 };



Re: [Qemu-devel] [PATCH 1/2] pflash: Support read-only mode

2011-07-27 Thread Jan Kiszka
On 2011-07-25 23:34, Jordan Justen wrote:
> Read-only mode is indicated by bdrv_is_read_only
> 
> When read-only mode is enabled, no changes will be made
> to the flash image in memory, and no bdrv_write calls will be
> made.
> 
> Signed-off-by: Jordan Justen 
> Cc: Jan Kiszka 
> Cc: Aurelien Jarno 
> Cc: Anthony Liguori 
> ---
>  blockdev.c|3 +-
>  hw/pflash_cfi01.c |   36 ++-
>  hw/pflash_cfi02.c |   82 
>  3 files changed, 68 insertions(+), 53 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 0b8d3a4..566a502 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -521,7 +521,8 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>  /* CDROM is fine for any interface, don't check.  */
>  ro = 1;
>  } else if (ro == 1) {
> -if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && 
> type != IF_NONE) {
> +if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY &&
> +type != IF_NONE && type != IF_PFLASH) {
>  error_report("readonly not supported by this bus type");
>  goto err;
>  }
> diff --git a/hw/pflash_cfi01.c b/hw/pflash_cfi01.c
> index 90fdc84..11ac490 100644
> --- a/hw/pflash_cfi01.c
> +++ b/hw/pflash_cfi01.c
> @@ -284,8 +284,10 @@ static void pflash_write(pflash_t *pfl, 
> target_phys_addr_t offset,
>  TARGET_FMT_plx "\n",
>  __func__, offset, pfl->sector_len);
>  
> -memset(p + offset, 0xff, pfl->sector_len);
> -pflash_update(pfl, offset, pfl->sector_len);
> +if (!pfl->ro) {
> +memset(p + offset, 0xff, pfl->sector_len);
> +pflash_update(pfl, offset, pfl->sector_len);
> +}
>  pfl->status |= 0x80; /* Ready! */
>  break;
>  case 0x50: /* Clear status bits */
> @@ -324,8 +326,10 @@ static void pflash_write(pflash_t *pfl, 
> target_phys_addr_t offset,
>  case 0x10: /* Single Byte Program */
>  case 0x40: /* Single Byte Program */
>  DPRINTF("%s: Single Byte Program\n", __func__);
> -pflash_data_write(pfl, offset, value, width, be);
> -pflash_update(pfl, offset, width);
> +if (!pfl->ro) {
> +pflash_data_write(pfl, offset, value, width, be);
> +pflash_update(pfl, offset, width);
> +}
>  pfl->status |= 0x80; /* Ready! */
>  pfl->wcycle = 0;
>  break;
> @@ -373,7 +377,9 @@ static void pflash_write(pflash_t *pfl, 
> target_phys_addr_t offset,
>  case 2:
>  switch (pfl->cmd) {
>  case 0xe8: /* Block write */
> -pflash_data_write(pfl, offset, value, width, be);
> +if (!pfl->ro) {
> +pflash_data_write(pfl, offset, value, width, be);
> +}
>  
>  pfl->status |= 0x80;
>  
> @@ -383,8 +389,10 @@ static void pflash_write(pflash_t *pfl, 
> target_phys_addr_t offset,
>  
>  DPRINTF("%s: block write finished\n", __func__);
>  pfl->wcycle++;
> -/* Flush the entire write buffer onto backing storage.  */
> -pflash_update(pfl, offset & mask, pfl->writeblock_size);
> +if (!pfl->ro) {
> +/* Flush the entire write buffer onto backing storage.  
> */
> +pflash_update(pfl, offset & mask, pfl->writeblock_size);
> +}
>  }
>  
>  pfl->counter--;
> @@ -621,13 +629,13 @@ pflash_t *pflash_cfi01_register(target_phys_addr_t 
> base, ram_addr_t off,
>  return NULL;
>  }
>  }
> -#if 0 /* XXX: there should be a bit to set up read-only,
> -   *  the same way the hardware does (with WP pin).
> -   */
> -pfl->ro = 1;
> -#else
> -pfl->ro = 0;
> -#endif
> +
> +if (pfl->bs) {
> +pfl->ro = bdrv_is_read_only(pfl->bs);
> +} else {
> +pfl->ro = 0;
> +}
> +
>  pfl->timer = qemu_new_timer_ns(vm_clock, pflash_timer, pfl);
>  pfl->base = base;
>  pfl->sector_len = sector_len;
> diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
> index 725cd1e..920209d 100644
> --- a/hw/pflash_cfi02.c
> +++ b/hw/pflash_cfi02.c
> @@ -315,35 +315,37 @@ static void pflash_write (pflash_t *pfl, 
> target_phys_addr_t offset,
>  DPRINTF("%s: write data offset " TARGET_FMT_plx " %08x %d\n",
>  __func__, offset, value, width);
>  p = pfl->storage;
> -switch (width) {
> -case 1:
> -p[offset] &= value;
> -pflash_update(pfl, offset, 1);
> -break;
> -case 2:
> -if (be) {
> -p[offset] &= value >> 8;
> -p[offset + 1] &= value;
> -} else {
> +if (!pfl->ro) {
> +  

Re: [Qemu-devel] [PATCH 2/2] [SLIRP] Delayed IP packets

2011-07-27 Thread Jan Kiszka
On 2011-07-26 18:21, Fabien Chouteau wrote:
> In the current implementation, if Slirp tries to send an IP packet to a client
> with an unknown hardware address, the packet is simply dropped and an ARP
> request is sent (if_encap in slirp/slirp.c).
> 
> This patch adds a list of delayed IP packets to handle such cases. If the
> hardware address is unknown, Slirp inserts the packet in delayed list and 
> sends
> an ARP request. Each time the ARP table is updated Slirp retries to send the
> packet.

Haven't looked at details yet, just two general thoughts so far:

We already have queues for outgoing packets, why can't we reuse that
infrastructure? That would also avoid additional memory allocations.
Delayed packets should be requeued at the end and only one attempt to
send them should be performed per queue flush.

I think we need some timeout for the delayed packets. If invalid or
non-reactive clients are addressed, we will otherwise pile them up until
qemu terminates, right?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH 1/7] vhost build fix for i386

2011-07-27 Thread Stefan Hajnoczi
From: Wolfgang Mauerer 

vhost.c uses __sync_fetch_and_and(), which is only
available for -march=i486 and above (see
https://bugzilla.redhat.com/show_bug.cgi?id=624279).

Signed-off-by: Wolfgang Mauerer 
Signed-off-by: Stefan Hajnoczi 
---
 configure |   23 +++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 38e3724..9bfe917 100755
--- a/configure
+++ b/configure
@@ -2510,6 +2510,29 @@ if test "$trace_backend" = "dtrace"; then
 fi
 
 ##
+# __sync_fetch_and_and requires at least -march=i486. Many toolchains
+# use i686 as default anyway, but for those that don't, an explicit
+# specification is necessary
+if test $vhost_net = "yes" && test $cpu = "i386"; then
+  cat > $TMPC << EOF
+int sfaa(unsigned *ptr)
+{
+  return __sync_fetch_and_and(ptr, 0);
+}
+
+int main(int argc, char **argv)
+{
+  int val = 42;
+  sfaa(&val);
+  return val;
+}
+EOF
+  if ! compile_prog "" "" ; then
+CFLAGS+="-march=i486"
+  fi
+fi
+
+##
 # End of CC checks
 # After here, no more $cc or $ld runs
 
-- 
1.7.5.4




[Qemu-devel] [PATCH 5/7] qmp: fix efect -> effect typo in qmp-commands.hx

2011-07-27 Thread Stefan Hajnoczi
From: Zhi Yong Wu 

Signed-off-by: Zhi Yong Wu 
Signed-off-by: Stefan Hajnoczi 
---
 qmp-commands.hx |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index 54e313c..03f67da 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -42,7 +42,7 @@ and we're going to establish a deprecation policy for badly 
defined commands.
 
 If you're planning to adopt QMP, please observe the following:
 
-1. The deprecation policy will take efect and be documented soon, please
+1. The deprecation policy will take effect and be documented soon, please
check the documentation of each used command as soon as a new release of
QEMU is available
 
-- 
1.7.5.4




Re: [Qemu-devel] [PATCH 2/2] virtio-balloon: Check if balloon registration failed

2011-07-27 Thread Amit Shah
On (Wed) 27 Jul 2011 [12:06:32], Michael S. Tsirkin wrote:
> On Wed, Jul 27, 2011 at 12:31:08PM +0530, Amit Shah wrote:
> > Multiple balloon registrations are not allowed; check if the
> > registration with the qemu balloon api succeeded.  If not, fail the
> > device init.
> > 
> > Signed-off-by: Amit Shah 
> > ---
> >  hw/virtio-balloon.c |   10 +-
> >  hw/virtio-pci.c |3 +++
> >  2 files changed, 12 insertions(+), 1 deletions(-)
> > 
> > diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> > index 2ba7e95..304a31a 100644
> > --- a/hw/virtio-balloon.c
> > +++ b/hw/virtio-balloon.c
> > @@ -269,6 +269,7 @@ static int virtio_balloon_load(QEMUFile *f, void 
> > *opaque, int version_id)
> >  VirtIODevice *virtio_balloon_init(DeviceState *dev)
> >  {
> >  VirtIOBalloon *s;
> > +int ret;
> >  
> >  s = (VirtIOBalloon *)virtio_common_init("virtio-balloon",
> >  VIRTIO_ID_BALLOON,
> > @@ -278,12 +279,19 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
> >  s->vdev.set_config = virtio_balloon_set_config;
> >  s->vdev.get_features = virtio_balloon_get_features;
> >  
> > +ret = qemu_add_balloon_handler(virtio_balloon_to_target,
> > +   virtio_balloon_stat, s);
> > +if (ret < 0) {
> > +error_report("Another balloon device already registered");
> 
> We don't know why it failed at this point. I think
> error report should be triggered by qemu_add_balloon_handler,
> and this function would just cleanup and exit.

OK, makes sense.

Amit



[Qemu-devel] [PATCH 4/7] Makefile: fix out-of-tree builds

2011-07-27 Thread Stefan Hajnoczi
From: Alexandre Raymond 

This patch fixes a minor bugs which prevented QEMU from being built
out of tree.

Signed-off-by: Alexandre Raymond 
Signed-off-by: Stefan Hajnoczi 
---
 Makefile |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index eb1c788..cbd614a 100644
--- a/Makefile
+++ b/Makefile
@@ -46,7 +46,7 @@ config-all-devices.mak: $(SUBDIR_DEVICES_MAK)
 
 -include $(SUBDIR_DEVICES_MAK_DEP)
 
-%/config-devices.mak: default-configs/%.mak
+%/config-devices.mak: $(SRC_PATH)/default-configs/%.mak
$(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh 
$@ $<, "  GEN   $@")
@if test -f $@; then \
  if cmp -s $@.old $@; then \
-- 
1.7.5.4




Re: [Qemu-devel] [PATCH] user: Restore debug usage message for '-d ?' in user mode emulation

2011-07-27 Thread Peter Maydell
Oops, I forgot to cc Justin when I said "should also go into 0.15"...

-- PMM

On 25 July 2011 09:43, Peter Maydell  wrote:
> Ping? Since this is a regression in our command line handling
> I think it should also go into 0.15...
>
> thanks
> -- PMM
>
>
> On 18 July 2011 11:44, Peter Maydell  wrote:
>> The code which prints the debug usage message on '-d ?' for *-user
>> has to come before the check for "not enough arguments", so that
>> "qemu-foo -d ?" prints the list of possible debug log items rather than
>> the generic usage message. (This was inadvertently broken in commit
>> c235d73.)
>>
>> Signed-off-by: Peter Maydell 
>> ---
>> NB that I've tested the linux-user part of this fix but don't have access to
>> bsd/darwin to test those files; however the change is identical for all three
>> files so it should be OK...
>>
>>  bsd-user/main.c    |    8 +---
>>  darwin-user/main.c |    8 +---
>>  linux-user/main.c  |   11 ++-
>>  3 files changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/bsd-user/main.c b/bsd-user/main.c
>> index 6018a41..a63b877 100644
>> --- a/bsd-user/main.c
>> +++ b/bsd-user/main.c
>> @@ -856,9 +856,6 @@ int main(int argc, char **argv)
>>             usage();
>>         }
>>     }
>> -    if (optind >= argc)
>> -        usage();
>> -    filename = argv[optind];
>>
>>     /* init debug */
>>     cpu_set_log_filename(log_file);
>> @@ -877,6 +874,11 @@ int main(int argc, char **argv)
>>         cpu_set_log(mask);
>>     }
>>
>> +    if (optind >= argc) {
>> +        usage();
>> +    }
>> +    filename = argv[optind];
>> +
>>     /* Zero out regs */
>>     memset(regs, 0, sizeof(struct target_pt_regs));
>>
>> diff --git a/darwin-user/main.c b/darwin-user/main.c
>> index 35196a1..72307ad 100644
>> --- a/darwin-user/main.c
>> +++ b/darwin-user/main.c
>> @@ -809,9 +809,6 @@ int main(int argc, char **argv)
>>             usage();
>>         }
>>     }
>> -    if (optind >= argc)
>> -        usage();
>> -    filename = argv[optind];
>>
>>     /* init debug */
>>     cpu_set_log_filename(log_file);
>> @@ -830,6 +827,11 @@ int main(int argc, char **argv)
>>         cpu_set_log(mask);
>>     }
>>
>> +    if (optind >= argc) {
>> +        usage();
>> +    }
>> +    filename = argv[optind];
>> +
>>     /* Zero out regs */
>>     memset(regs, 0, sizeof(struct target_pt_regs));
>>
>> diff --git a/linux-user/main.c b/linux-user/main.c
>> index 289054b..8976b60 100644
>> --- a/linux-user/main.c
>> +++ b/linux-user/main.c
>> @@ -3019,11 +3019,6 @@ int main(int argc, char **argv, char **envp)
>>             usage();
>>         }
>>     }
>> -    if (optind >= argc)
>> -        usage();
>> -    filename = argv[optind];
>> -    exec_path = argv[optind];
>> -
>>     /* init debug */
>>     cpu_set_log_filename(log_file);
>>     if (log_mask) {
>> @@ -3041,6 +3036,12 @@ int main(int argc, char **argv, char **envp)
>>         cpu_set_log(mask);
>>     }
>>
>> +    if (optind >= argc) {
>> +        usage();
>> +    }
>> +    filename = argv[optind];
>> +    exec_path = argv[optind];
>> +
>>     /* Zero out regs */
>>     memset(regs, 0, sizeof(struct target_pt_regs));
>>
>> --
>> 1.7.1



[Qemu-devel] [STABLE v2] virtio-serial-bus: replay guest_open on migration

2011-07-27 Thread Alon Levy
When migrating a host with with a spice agent running the mouse becomes
non operational after the migration. This is rhbz #725965.

The problem is that after migration spice doesn't know the guest agent is open.
Spice is just a char dev here. And a chardev cannot query it's device, the
device has to let the chardev know when it is open. Right now after migration
the chardev which is recreated is in it's default state, which assumes the
guest is disconnected.

Char devices carry no information across migration, but the virtio-serial does
already carry the guest_connected state. This patch passes that bit to the
chardev.

Signed-off-by: Alon Levy 
---
 hw/virtio-serial-bus.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index c5eb931..4a6d932 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -618,6 +618,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, 
int version_id)
 for (i = 0; i < nr_active_ports; i++) {
 uint32_t id;
 bool host_connected;
+VirtIOSerialPortInfo *info;
 
 id = qemu_get_be32(f);
 port = find_port_by_id(s, id);
@@ -626,6 +627,11 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, 
int version_id)
 }
 
 port->guest_connected = qemu_get_byte(f);
+info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info);
+if (port->guest_connected && info->guest_open) {
+/* replay guest open */
+info->guest_open(port);
+}
 host_connected = qemu_get_byte(f);
 if (host_connected != port->host_connected) {
 /*
-- 
1.7.6




[Qemu-devel] [PATCH 2/7] Makefile: Minor cscope fixups

2011-07-27 Thread Stefan Hajnoczi
From: Alexandre Raymond 

Create cscope symbols for assembly files in addition to .c/.h files.
Create cscope database with full path instead of relative path so cscope
can be used with CSCOPE_DB in any directory.

Signed-off-by: Alexandre Raymond 
Signed-off-by: Stefan Hajnoczi 
---
 Makefile |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index daa3aa0..eb1c788 100644
--- a/Makefile
+++ b/Makefile
@@ -291,7 +291,7 @@ TAGS:
 
 cscope:
rm -f ./cscope.*
-   find . -name "*.[ch]" -print | sed 's,^\./,,' > ./cscope.files
+   find "$(SRC_PATH)" -name "*.[chsS]" -print | sed 's,^\./,,' > 
./cscope.files
cscope -b
 
 # documentation
-- 
1.7.5.4




Re: [Qemu-devel] [PATCH v8 0/5] Coroutines for better asynchronous programming

2011-07-27 Thread Aneesh Kumar K.V
On Tue, 26 Jul 2011 10:21:12 +0100, Stefan Hajnoczi 
 wrote:
> QEMU is event-driven and suffers when blocking operations are performed 
> because
> VM execution may be stopped until the operation completes.  Therefore many
> operations that could block are performed asynchronously and a callback is
> invoked when the operation has completed.  This allows QEMU to continue
> executing while the operation is pending.
> 
> The downside to callbacks is that they split up code into many smaller
> functions, each of which is a single step in a state machine that quickly
> becomes complex and hard to understand.  Callback functions also result in 
> lots
> of noise as variables are packed and unpacked into temporary structs that pass
> state to the callback function.
> 
> This patch series introduces coroutines as a solution for writing asynchronous
> code while still having a nice sequential control flow.  The semantics are
> explained in the second patch.  The fourth patch adds automated tests.
> 
> A nice feature of coroutines is that it is relatively easy to take synchronous
> code and lift it into a coroutine to make it asynchronous.  Work has been done
> to move qcow2 request processing into coroutines and thereby make it
> asynchronous (today qcow2 will perform synchronous metadata accesses).  This
> qcow2 work is still ongoing and not quite ready for mainline yet.
> 
> v8:
>  * Bisectability: introduce gthread implementation before ucontext/fibers

Can we also get CoMutex and CoQueue patches also merged. I use them in
the VirtFS series. 

http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/coroutine-devel
http://repo.or.cz/w/qemu/v9fs.git/commit/318ef0b9b01cd296f8c30d8288139b9bed859892

-aneesh 



[Qemu-devel] [PATCH] HMP: Remove the duplicated info "info kvm" in hmp-commands.hx.

2011-07-27 Thread Zhi Yong Wu
Signed-off-by: Zhi Yong Wu 
---
 hmp-commands.hx |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index aceba74..3498f0f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1324,8 +1324,6 @@ show virtual to physical memory mappings (i386, SH4 and 
SPARC only)
 show the active virtual memory mappings (i386 only)
 @item info jit
 show dynamic compiler info
-@item info kvm
-show KVM information
 @item info numa
 show NUMA information
 @item info kvm
-- 
1.7.2.3




[Qemu-devel] [PATCH 3/7] slirp: Fix unusual "comments" in unused code

2011-07-27 Thread Stefan Hajnoczi
From: Stefan Weil 

cppcheck detected two rather strange comments which were not
correctly written as C comments.

They did not cause any harm because they were framed by
#ifdef notdef ... #endif, so they were never compiled.

Fix them nevertheless (we could also remove the unused code).

Signed-off-by: Stefan Weil 
Signed-off-by: Stefan Hajnoczi 
---
 slirp/ip_input.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/slirp/ip_input.c b/slirp/ip_input.c
index 5e67631..c7b3eb4 100644
--- a/slirp/ip_input.c
+++ b/slirp/ip_input.c
@@ -511,7 +511,7 @@ typedef uint32_t n_time;
 */
break;
}
-   off--;  / * 0 origin *  /
+off--; /* 0 origin */
if (off > optlen - sizeof(struct in_addr)) {
/*
 * End of source route.  Should be for us.
@@ -554,7 +554,7 @@ typedef uint32_t n_time;
/*
 * If no space remains, ignore.
 */
-   off--;   * 0 origin *
+off--; /* 0 origin */
if (off > optlen - sizeof(struct in_addr))
break;
bcopy((caddr_t)(&ip->ip_dst), (caddr_t)&ipaddr.sin_addr,
-- 
1.7.5.4




Re: [Qemu-devel] [PATCH v8 0/5] Coroutines for better asynchronous programming

2011-07-27 Thread Kevin Wolf
Am 27.07.2011 11:45, schrieb Aneesh Kumar K.V:
> On Tue, 26 Jul 2011 10:21:12 +0100, Stefan Hajnoczi 
>  wrote:
>> QEMU is event-driven and suffers when blocking operations are performed 
>> because
>> VM execution may be stopped until the operation completes.  Therefore many
>> operations that could block are performed asynchronously and a callback is
>> invoked when the operation has completed.  This allows QEMU to continue
>> executing while the operation is pending.
>>
>> The downside to callbacks is that they split up code into many smaller
>> functions, each of which is a single step in a state machine that quickly
>> becomes complex and hard to understand.  Callback functions also result in 
>> lots
>> of noise as variables are packed and unpacked into temporary structs that 
>> pass
>> state to the callback function.
>>
>> This patch series introduces coroutines as a solution for writing 
>> asynchronous
>> code while still having a nice sequential control flow.  The semantics are
>> explained in the second patch.  The fourth patch adds automated tests.
>>
>> A nice feature of coroutines is that it is relatively easy to take 
>> synchronous
>> code and lift it into a coroutine to make it asynchronous.  Work has been 
>> done
>> to move qcow2 request processing into coroutines and thereby make it
>> asynchronous (today qcow2 will perform synchronous metadata accesses).  This
>> qcow2 work is still ongoing and not quite ready for mainline yet.
>>
>> v8:
>>  * Bisectability: introduce gthread implementation before ucontext/fibers
> 
> Can we also get CoMutex and CoQueue patches also merged. I use them in
> the VirtFS series. 
> 
> http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/coroutine-devel
> http://repo.or.cz/w/qemu/v9fs.git/commit/318ef0b9b01cd296f8c30d8288139b9bed859892

I introduce these in my block coroutine patches. I posted a RFC last
week and the first "real" patch series yesterday, so I hope they get
review and can be merged into master soon.

Kevin



[Qemu-devel] [PULL 0/7] Trivial patches for June 25 to July 27 2011

2011-07-27 Thread Stefan Hajnoczi
The following changes since commit c886edfb851c0c590d4e77f058f2ec8ed95ad1b5:

  Let users select their pythons (2011-07-25 16:50:12 +)

are available in the git repository at:
  ssh://repo.or.cz/srv/git/qemu/stefanha.git trivial-patches

Alexandre Raymond (2):
  Makefile: Minor cscope fixups
  Makefile: fix out-of-tree builds

Juan Quintela (1):
  xen_mapcache: remove unused variable

Michael Roth (1):
  Makefile: add missing deps on $(GENERATED_HEADERS)

Stefan Weil (1):
  slirp: Fix unusual "comments" in unused code

Wolfgang Mauerer (1):
  vhost build fix for i386

Zhi Yong Wu (1):
  qmp: fix efect -> effect typo in qmp-commands.hx

 Makefile |8 +---
 configure|   23 +++
 qmp-commands.hx  |2 +-
 slirp/ip_input.c |4 ++--
 xen-mapcache.c   |3 +--
 5 files changed, 32 insertions(+), 8 deletions(-)

-- 
1.7.5.4




[Qemu-devel] [PATCH v7] showing a splash picture when start

2011-07-27 Thread Wayne Xia
From: wayne 

Added options to let qemu transfer two configuration files to bios:
"bootsplash.bmp" and "etc/boot-menu-wait", which could be specified by command
-boot splash=P,splash-time=T
P is jpg/bmp file name or an absolute path, T have a max value of 0x, unit
is ms. With these two options, if user invoke qemu with menu=on option, then
a splash picture would be showed in a given time. For example:
qemu -boot menu=on,splash=/root/boot.bmp,splash-time=5000
would make boot.bmp shown as a brand with 5 seconds in the booting up process.
This feature need the new seabios's support, which could be got from git.

Signed-off-by: Wayne Xia 
---
 hw/fw_cfg.c |  140 ++-
 qemu-config.c   |   27 +++
 qemu-options.hx |   16 ++-
 sysemu.h|3 +
 vl.c|   17 ++-
 5 files changed, 199 insertions(+), 4 deletions(-)

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 34e7526..a29db90 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -26,6 +26,7 @@
 #include "isa.h"
 #include "fw_cfg.h"
 #include "sysbus.h"
+#include "qemu-error.h"
 
 /* debug firmware config */
 //#define DEBUG_FW_CFG
@@ -56,6 +57,143 @@ struct FWCfgState {
 Notifier machine_ready;
 };
 
+#define JPG_FILE 0
+#define BMP_FILE 1
+
+static FILE *probe_splashfile(char *filename, int *file_sizep, int *file_typep)
+{
+FILE *fp = NULL;
+int fop_ret;
+int file_size;
+int file_type = -1;
+unsigned char buf[2] = {0, 0};
+unsigned int filehead_value = 0;
+int bmp_bpp;
+
+fp = fopen(filename, "rb");
+if (fp == NULL) {
+error_report("failed to open file '%s'.", filename);
+return fp;
+}
+/* check file size */
+fseek(fp, 0L, SEEK_END);
+file_size = ftell(fp);
+if (file_size < 2) {
+error_report("file size is less than 2 bytes '%s'.", filename);
+fclose(fp);
+fp = NULL;
+return fp;
+}
+/* check magic ID */
+fseek(fp, 0L, SEEK_SET);
+fop_ret = fread(buf, 1, 2, fp);
+filehead_value = (buf[0] + (buf[1] << 8)) & 0x;
+if (filehead_value == 0xd8ff) {
+file_type = JPG_FILE;
+} else {
+if (filehead_value == 0x4d42) {
+file_type = BMP_FILE;
+}
+}
+if (file_type < 0) {
+error_report("'%s' not jpg/bmp file,head:0x%x.",
+ filename, filehead_value);
+fclose(fp);
+fp = NULL;
+return fp;
+}
+/* check BMP bpp */
+if (file_type == BMP_FILE) {
+fseek(fp, 28, SEEK_SET);
+fop_ret = fread(buf, 1, 2, fp);
+bmp_bpp = (buf[0] + (buf[1] << 8)) & 0x;
+if (bmp_bpp != 24) {
+error_report("only 24bpp bmp file is supported.");
+fclose(fp);
+fp = NULL;
+return fp;
+}
+}
+/* return values */
+*file_sizep = file_size;
+*file_typep = file_type;
+return fp;
+}
+
+static void fw_cfg_bootsplash(FWCfgState *s)
+{
+int boot_splash_time = -1;
+const char *boot_splash_filename = NULL;
+char *p;
+char *filename;
+FILE *fp;
+int fop_ret;
+int file_size;
+int file_type = -1;
+const char *temp;
+
+/* get user configuration */
+QemuOptsList *plist = qemu_find_opts("boot-opts");
+QemuOpts *opts = QTAILQ_FIRST(&plist->head);
+if (opts != NULL) {
+temp = qemu_opt_get(opts, "splash");
+if (temp != NULL) {
+boot_splash_filename = temp;
+}
+temp = qemu_opt_get(opts, "splash-time");
+if (temp != NULL) {
+p = (char *)temp;
+boot_splash_time = strtol(p, (char **)&p, 10);
+}
+}
+
+/* insert splash time if user configurated */
+if (boot_splash_time >= 0) {
+/* validate the input */
+if (boot_splash_time > 0x) {
+error_report("splash time is big than 65535, force it to 65535.");
+boot_splash_time = 0x;
+}
+/* use little endian format */
+qemu_extra_params_fw[0] = (uint8_t)(boot_splash_time & 0xff);
+qemu_extra_params_fw[1] = (uint8_t)((boot_splash_time >> 8) & 0xff);
+fw_cfg_add_file(s, "etc/boot-menu-wait", qemu_extra_params_fw, 2);
+}
+
+/* insert splash file if user configurated */
+if (boot_splash_filename != NULL) {
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, boot_splash_filename);
+if (filename == NULL) {
+error_report("failed to find file '%s'.", boot_splash_filename);
+return;
+}
+/* probing the file */
+fp = probe_splashfile(filename, &file_size, &file_type);
+if (fp == NULL) {
+qemu_free(filename);
+return;
+}
+/* loading file data */
+if (boot_splash_filedata != NULL) {
+qemu_free(boot_splash_filedata);
+}
+boot_splash_filedata = qemu_malloc(file_size);
+boot_sp

Re: [Qemu-devel] [PATCH v6] showing a splash picture when start

2011-07-27 Thread Wayne Xia

Thanks, following is my comments.


On 07/10/2011 05:09 AM, Wayne Xia wrote:

Added options to let qemu transfer two configuration files to bios:
"bootsplash.bmp" and "etc/boot-menu-wait", which could be specified by
command
-boot splash=P,splash-time=T
P is jpg/bmp file name or an absolute path, T have a max value of
0x, unit
is ms. With these two options, if user invoke qemu with menu=on
option, then
a splash picture would be showed in a given time. For example:
qemu -boot menu=on,splash=/root/boot.bmp,splash-time=5000
would make boot.bmp shown as a brand with 5 seconds in the booting up
process.
This feature need the new seabios's support, which could be got from git.


Please include documentation in qemu-doc.texi including information on
what image formats are supported and what restrictions are present (for
instance 24-bit depth bitmaps with what resolution?).

Regards,

Anthony Liguori


added the information in qemu-options.hx.



Signed-off-by: Wayne Xia
---
hw/fw_cfg.c | 140
-
qemu-config.c | 27 +++
sysemu.h | 3 +
vl.c | 17 +++-
4 files changed, 185 insertions(+), 2 deletions(-)

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 85c8c3c..434fc96 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -26,6 +26,7 @@
#include "isa.h"
#include "fw_cfg.h"
#include "sysbus.h"
+#include "qemu-error.h"

/* debug firmware config */
//#define DEBUG_FW_CFG
@@ -56,6 +57,143 @@ struct FWCfgState {
Notifier machine_ready;
};

+#define JPG_FILE 0
+#define BMP_FILE 1
+
+static FILE *probe_splashfile(char *filename, int *file_sizep, int
*file_typep)
+{
+ FILE *fp = NULL;
+ int fop_ret;
+ int file_size;
+ int file_type = -1;
+ unsigned char buf[2] = {0, 0};
+ unsigned int filehead_value = 0;
+ int bmp_bpp;
+
+ fp = fopen(filename, "rb");
+ if (fp == NULL) {
+ error_report("failed to open file '%s'.", filename);
+ return fp;
+ }
+ /* check file size */
+ fseek(fp, 0L, SEEK_END);
+ file_size = ftell(fp);
+ if (file_size< 2) {
+ error_report("file size is less than 2 bytes '%s'.", filename);
+ fclose(fp);
+ fp = NULL;
+ return fp;
+ }
+ /* check magic ID */
+ fseek(fp, 0L, SEEK_SET);
+ fop_ret = fread(buf, 1, 2, fp);
+ filehead_value = (buf[0] + (buf[1]<< 8))& 0x;
+ if (filehead_value == 0xd8ff) {
+ file_type = JPG_FILE;
+ } else {
+ if (filehead_value == 0x4d42) {
+ file_type = BMP_FILE;
+ }
+ }
+ if (file_type< 0) {
+ error_report("'%s' not jpg/bmp file,head:0x%x.",
+ filename, filehead_value);
+ fclose(fp);
+ fp = NULL;
+ return fp;
+ }
+ /* check BMP bpp */
+ if (file_type == BMP_FILE) {
+ fseek(fp, 28, SEEK_SET);
+ fop_ret = fread(buf, 1, 2, fp);
+ bmp_bpp = (buf[0] + (buf[1]<< 8))& 0x;
+ if (bmp_bpp != 24) {
+ error_report("only 24bpp bmp file is supported.");
+ fclose(fp);
+ fp = NULL;
+ return fp;
+ }
+ }
+ /* return values */
+ *file_sizep = file_size;
+ *file_typep = file_type;
+ return fp;
+}
+
+static void fw_cfg_bootsplash(FWCfgState *s)
+{
+ int boot_splash_time = -1;
+ const char *boot_splash_filename = NULL;
+ char *p;
+ char *filename;
+ FILE *fp;
+ int fop_ret;
+ int file_size;
+ int file_type = -1;
+ const char *temp;
+
+ /* get user configuration */
+ QemuOptsList *plist = qemu_find_opts("boot-opts");
+ QemuOpts *opts = QTAILQ_FIRST(&plist->head);
+ if (opts != NULL) {
+ temp = qemu_opt_get(opts, "splash");
+ if (temp != NULL) {
+ boot_splash_filename = temp;
+ }
+ temp = qemu_opt_get(opts, "splash-time");
+ if (temp != NULL) {
+ p = (char *)temp;
+ boot_splash_time = strtol(p, (char **)&p, 10);
+ }
+ }
+
+ /* insert splash time if user configurated */
+ if (boot_splash_time>= 0) {
+ /* validate the input */
+ if (boot_splash_time> 0x) {
+ error_report("splash time is big than 65535, force it to 65535.");
+ boot_splash_time = 0x;
+ }
+ /* use little endian format */
+ qemu_extra_params_fw[0] = (uint8_t)(boot_splash_time& 0xff);
+ qemu_extra_params_fw[1] = (uint8_t)((boot_splash_time>> 8)& 0xff);
+ fw_cfg_add_file(s, "etc/boot-menu-wait", qemu_extra_params_fw, 2);
+ }
+
+ /* insert splash file if user configurated */
+ if (boot_splash_filename != NULL) {
+ filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, boot_splash_filename);
+ if (filename == NULL) {
+ error_report("failed to find file '%s'.", boot_splash_filename);
+ return;
+ }
+ /* probing the file */
+ fp = probe_splashfile(filename,&file_size,&file_type);
+ if (fp == NULL) {
+ qemu_free(filename);
+ return;
+ }
+ /* loading file data */
+ if (boot_splash_filedata != NULL) {
+ qemu_free(boot_splash_filedata);
+ }
+ boot_splash_filedata = qemu_malloc(file_size);
+ boot_splash_filedata_size = file_size;
+ fseek(fp, 0L, SEEK_SET);
+ fop_ret = fread(boot_splash_filedata, 1, file_size, fp);
+ fclose(fp);
+ /* insert data */
+ if (file_type == JPG_FILE) {
+ fw_cfg_add_file(s, "bootsplash.jpg",
+ boot_splash_filedata, boot_splash_filedata_size);
+ } else {
+ fw_cfg_add_file(s, "bootsplash.bmp",
+ boot_splash_filedata, boot_splash_filedata_size);
+ }
+ qemu_free(filename);
+ }
+

Re: [Qemu-devel] [PATCH 2/2] [SLIRP] Delayed IP packets

2011-07-27 Thread Fabien Chouteau
On 27/07/2011 11:30, Jan Kiszka wrote:
> On 2011-07-26 18:21, Fabien Chouteau wrote:
>> In the current implementation, if Slirp tries to send an IP packet to a 
>> client
>> with an unknown hardware address, the packet is simply dropped and an ARP
>> request is sent (if_encap in slirp/slirp.c).
>>
>> This patch adds a list of delayed IP packets to handle such cases. If the
>> hardware address is unknown, Slirp inserts the packet in delayed list and 
>> sends
>> an ARP request. Each time the ARP table is updated Slirp retries to send the
>> packet.
>
> Haven't looked at details yet, just two general thoughts so far:
>
> We already have queues for outgoing packets, why can't we reuse that
> infrastructure? That would also avoid additional memory allocations.
> Delayed packets should be requeued at the end and only one attempt to
> send them should be performed per queue flush.

Sure, I didn't know about these queues. Where are they implemented?

>
> I think we need some timeout for the delayed packets. If invalid or
> non-reactive clients are addressed, we will otherwise pile them up until
> qemu terminates, right?

Yes that's a good idea.

Regards,

-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH 0/4] Fix virtio memleaks

2011-07-27 Thread Amit Shah
On (Wed) 27 Jul 2011 [12:08:52], Michael S. Tsirkin wrote:
> On Wed, Jul 27, 2011 at 02:00:28PM +0530, Amit Shah wrote:
> > The memory allocated in virtio_common_init() wasn't being freed
> > anywhere.  Fix that.
> > 
> > The balloon handler wasn't unregistering its savevm section,
> > adding an exit handler fixes that as well.
> > 
> > This patchset is on top of the two balloon series I've sent out
> > yesterday and today.
> 
> This looks good to me. What do you say I put
> patches 2-4 on my tree, and you put patch 1 on yours?

Sure.

Amit



Re: [Qemu-devel] [PATCH v2 1/1] The codes V2 for QEMU disk I/O limits.

2011-07-27 Thread Zhi Yong Wu
On Wed, Jul 27, 2011 at 3:26 AM, Marcelo Tosatti  wrote:
> On Tue, Jul 26, 2011 at 04:59:06PM +0800, Zhi Yong Wu wrote:
>> Welcome to give me your comments, thanks.
>>
>> Signed-off-by: Zhi Yong Wu 
>> ---
>>  Makefile.objs     |    2 +-
>>  block.c           |  288 
>> +++--
>>  block.h           |    1 -
>>  block/blk-queue.c |  116 +
>>  block/blk-queue.h |   70 +
>>  block_int.h       |   28 +
>>  blockdev.c        |   21 
>>  qemu-config.c     |   24 +
>>  qemu-option.c     |   17 +++
>>  qemu-option.h     |    1 +
>>  qemu-options.hx   |    1 +
>>  11 files changed, 559 insertions(+), 10 deletions(-)
>>  create mode 100644 block/blk-queue.c
>>  create mode 100644 block/blk-queue.h
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 9f99ed4..06f2033 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -23,7 +23,7 @@ block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o 
>> dmg.o bochs.o vpc.o vv
>>  block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o 
>> qcow2-cache.o
>>  block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>>  block-nested-y += qed-check.o
>> -block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>> +block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o 
>> blk-queue.o
>>  block-nested-$(CONFIG_WIN32) += raw-win32.o
>>  block-nested-$(CONFIG_POSIX) += raw-posix.o
>>  block-nested-$(CONFIG_CURL) += curl.o
>> diff --git a/block.c b/block.c
>> index 24a25d5..e54e59c 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -29,6 +29,9 @@
>>  #include "module.h"
>>  #include "qemu-objects.h"
>>
>> +#include "qemu-timer.h"
>> +#include "block/blk-queue.h"
>> +
>>  #ifdef CONFIG_BSD
>>  #include 
>>  #include 
>> @@ -58,6 +61,13 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t 
>> sector_num,
>>  static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
>>                           const uint8_t *buf, int nb_sectors);
>>
>> +static bool bdrv_exceed_bps_limits(BlockDriverState *bs, int nb_sectors,
>> +        bool is_write, double elapsed_time, uint64_t *wait);
>> +static bool bdrv_exceed_iops_limits(BlockDriverState *bs, bool is_write,
>> +        double elapsed_time, uint64_t *wait);
>> +static bool bdrv_exceed_io_limits(BlockDriverState *bs, int nb_sectors,
>> +        bool is_write, uint64_t *wait);
>> +
>>  static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
>>      QTAILQ_HEAD_INITIALIZER(bdrv_states);
>>
>> @@ -90,6 +100,20 @@ int is_windows_drive(const char *filename)
>>  }
>>  #endif
>>
>> +static int bdrv_io_limits_enable(BlockIOLimit *io_limits)
>> +{
>> +    if ((io_limits->bps[0] == 0)
>> +         && (io_limits->bps[1] == 0)
>> +         && (io_limits->bps[2] == 0)
>> +         && (io_limits->iops[0] == 0)
>> +         && (io_limits->iops[1] == 0)
>> +         && (io_limits->iops[2] == 0)) {
>> +        return 0;
>> +    }
>> +
>> +    return 1;
>> +}
>> +
>>  /* check if the path starts with ":" */
>>  static int path_has_protocol(const char *path)
>>  {
>> @@ -167,6 +191,28 @@ void path_combine(char *dest, int dest_size,
>>      }
>>  }
>>
>> +static void bdrv_block_timer(void *opaque)
>> +{
>> +    BlockDriverState *bs = opaque;
>> +    BlockQueue *queue = bs->block_queue;
>> +
>> +    while (!QTAILQ_EMPTY(&queue->requests)) {
>> +        BlockIORequest *request;
>> +        int ret;
>> +
>> +        request = QTAILQ_FIRST(&queue->requests);
>> +        QTAILQ_REMOVE(&queue->requests, request, entry);
>> +
>> +        ret = qemu_block_queue_handler(request);
>> +        if (ret == 0) {
>> +            QTAILQ_INSERT_HEAD(&queue->requests, request, entry);
>> +            break;
>> +        }
>> +
>> +        qemu_free(request);
>> +    }
>> +}
>> +
>>  void bdrv_register(BlockDriver *bdrv)
>>  {
>>      if (!bdrv->bdrv_aio_readv) {
>> @@ -642,6 +688,15 @@ int bdrv_open(BlockDriverState *bs, const char 
>> *filename, int flags,
>>              bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
>>      }
>>
>> +    /* throttling disk I/O limits */
>> +    if (bdrv_io_limits_enable(&bs->io_limits)) {
>> +        bs->block_queue = qemu_new_block_queue();
>> +        bs->block_timer = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
>> +
>> +        bs->slice_start[0] = qemu_get_clock_ns(vm_clock);
>> +        bs->slice_start[1] = qemu_get_clock_ns(vm_clock);
>> +    }
>> +
>
> It should be possible to tune the limits on the flight, please introduce
> QMP commands for that.
Yeah, I am working on this.
>
>>      return 0;
>>
>>  unlink_and_fail:
>> @@ -680,6 +735,16 @@ void bdrv_close(BlockDriverState *bs)
>>          if (bs->change_cb)
>>              bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
>>      }
>> +
>> +    /* throttling disk I/O limits */
>> +    if (bs->block_queue) {
>> +        qemu_del_block_queue(bs->block_queue);
>> +    }
>> +
>> +    if (bs->block_timer) {
>> +      

Re: [Qemu-devel] [PATCH] virtio-serial-bus: replay guest_open on migration

2011-07-27 Thread Amit Shah
On (Wed) 27 Jul 2011 [10:07:56], Alon Levy wrote:
> On Wed, Jul 27, 2011 at 07:45:25AM +0200, Markus Armbruster wrote:
> > Alon Levy  writes:
> > 
> > > Signed-off-by: Alon Levy 
> > > ---
> > >  hw/virtio-serial-bus.c |8 +++-
> > >  1 files changed, 7 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > > index c5eb931..7a652ff 100644
> > > --- a/hw/virtio-serial-bus.c
> > > +++ b/hw/virtio-serial-bus.c
> > > @@ -618,14 +618,20 @@ static int virtio_serial_load(QEMUFile *f, void 
> > > *opaque, int version_id)
> > >  for (i = 0; i < nr_active_ports; i++) {
> > >  uint32_t id;
> > >  bool host_connected;
> > > +VirtIOSerialPortInfo *info;
> > >  
> > >  id = qemu_get_be32(f);
> > >  port = find_port_by_id(s, id);
> > >  if (!port) {
> > >  return -EINVAL;
> > >  }
> > > -
> > >  port->guest_connected = qemu_get_byte(f);
> > > +info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info);
> > > +if (port->guest_connected && info->guest_open) {
> > > +/* replay guest open */
> > > +info->guest_open(port);
> > > +
> > > +}
> > >  host_connected = qemu_get_byte(f);
> > >  if (host_connected != port->host_connected) {
> > >  /*
> > 
> > The patch makes enough sense to me, but the commit message is
> > insufficient.  Why do you have to replay?  And what's being fixed?
> 
> When migrating a host with with a spice agent running the mouse becomes
> non operational after the migration. This is rhbz #718463, currently on
> spice-server but it seems this is a qemu-kvm issue. The problem is that
> after migration spice doesn't know the guest agent is open. Spice is just
> a char dev here. And a chardev cannot query it's device, the device has
> to let the chardev know when it is open. Right now after migration the
> chardev which is recreated is in it's default state, which assumes the
> guest is disconnected. Char devices carry no information across migration,
> but the virtio-serial does already carry the guest_connected state. This
> patch passes that bit to the chardev.

It's not guaranteed all ports will be chardevs.

My thinking was this can be handled by qemu-char-spice.c since it can
add a new migration section and if an image is being restored and the
guest agent channel is open after migration finishes, it can continue
its work from there.  What's the benefit of all virtio-serial ports
receiving a guest_open() event in this case?

Also, we'll be lying that a guest opened, since a guest was opened
much earlier, before migration.  Nothing has changed as far as the
guest is concerned, this is just some host-side tracking that has to
be done post-migrate, which belongs in individual devices / ports.

So I'm not completely sure that this is the right place for such a
notification.  However, if others feel this is fine, I'll accept the
patch.

(Also, when resending, make sure the whitespace changes don't go
through.)

Thanks,
Amit



Re: [Qemu-devel] [PATCH 2/2] [SLIRP] Delayed IP packets

2011-07-27 Thread Jan Kiszka
On 2011-07-27 12:14, Fabien Chouteau wrote:
> On 27/07/2011 11:30, Jan Kiszka wrote:
>> On 2011-07-26 18:21, Fabien Chouteau wrote:
>>> In the current implementation, if Slirp tries to send an IP packet to a 
>>> client
>>> with an unknown hardware address, the packet is simply dropped and an ARP
>>> request is sent (if_encap in slirp/slirp.c).
>>>
>>> This patch adds a list of delayed IP packets to handle such cases. If the
>>> hardware address is unknown, Slirp inserts the packet in delayed list and 
>>> sends
>>> an ARP request. Each time the ARP table is updated Slirp retries to send the
>>> packet.
>>
>> Haven't looked at details yet, just two general thoughts so far:
>>
>> We already have queues for outgoing packets, why can't we reuse that
>> infrastructure? That would also avoid additional memory allocations.
>> Delayed packets should be requeued at the end and only one attempt to
>> send them should be performed per queue flush.
> 
> Sure, I didn't know about these queues. Where are they implemented?

Check e.g. what happens in and is documented for if_start().

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH 1/2] [SLIRP] Simple ARP table

2011-07-27 Thread Jan Kiszka
On 2011-07-26 18:21, Fabien Chouteau wrote:
> This patch adds a simple ARP table in Slirp and also adds handling of
> gratuitous ARP requests.
> 
> Signed-off-by: Fabien Chouteau 
> ---
>  Makefile.objs |2 +-
>  slirp/arp_table.c |   61 +++
>  slirp/bootp.c |   15 ++-
>  slirp/slirp.c |   68 
>  slirp/slirp.h |   51 +--
>  5 files changed, 139 insertions(+), 58 deletions(-)
>  create mode 100644 slirp/arp_table.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 6991a9f..0c10557 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -151,7 +151,7 @@ common-obj-y += qemu-timer.o qemu-timer-common.o
> 
>  slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o
>  slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o
> -slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
> +slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o
>  common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
> 
>  # xen backend driver support
> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
> new file mode 100644
> index 000..e3251ed
> --- /dev/null
> +++ b/slirp/arp_table.c
> @@ -0,0 +1,61 @@
> +#include "slirp.h"
> +
> +void arp_table_flush(ArpTable *arptbl)
> +{
> +int i;
> +
> +assert(arptbl != NULL);
> +
> +for (i = 0; i < ARP_TABLE_SIZE; i++) {
> +arptbl->table[i].ar_sip = 0;
> +}
> +arptbl->next_victim = 0;
> +}

arp_table_flush is only used for initialization, but the memory it
touches is already zero-initialized (on alloc in slirp_init). So you can
save this service.

> +
> +void arp_table_add(ArpTable  *arptbl,
> +   struct arphdr *ahdr)

Let's stick with

void arp_table_add(ArpTable *arptbl, struct arphdr *ahdr)

formatting. That's more consistent with other parts of QEMU.

> +{
> +int i;
> +
> +DEBUG_CALL("arp_table_add");
> +DEBUG_ARG("ip = 0x%x", ahdr->ar_sip);
> +DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
> +ahdr->ar_sha[0], ahdr->ar_sha[1], ahdr->ar_sha[2],
> +ahdr->ar_sha[3], ahdr->ar_sha[4], ahdr->ar_sha[5]));
> +
> +/* Search for an entry */
> +for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++) {
> +if (arptbl->table[i].ar_sip == ahdr->ar_sip) {
> +/* Update the entry */
> +memcpy(arptbl->table[i].ar_sha,  ahdr->ar_sha, ETH_ALEN);
> +return;
> +}
> +}
> +
> +/* No entry found, create a new one */
> +arptbl->table[arptbl->next_victim].ar_sip = ahdr->ar_sip;
> +memcpy(arptbl->table[arptbl->next_victim].ar_sha,  ahdr->ar_sha, 
> ETH_ALEN);
> +arptbl->next_victim = (arptbl->next_victim + 1) % ARP_TABLE_SIZE;

Pragmatic aging. But I guess that's OK, we shouldn't need anything
smarter here.

> +}
> +
> +bool arp_table_search(ArpTable *arptbl,
> +  int   in_ip_addr,
> +  uint8_t   out_ethaddr[ETH_ALEN])
> +{
> +int i;
> +
> +DEBUG_CALL("arp_table_search");
> +DEBUG_ARG("ip = 0x%x", in_ip_addr);
> +
> +for (i = 0; i < ARP_TABLE_SIZE; i++) {
> +if (arptbl->table[i].ar_sip == in_ip_addr) {
> +memcpy(out_ethaddr, arptbl->table[i].ar_sha,  ETH_ALEN);
> +DEBUG_ARGS((dfd, " found hw addr = 
> %02x:%02x:%02x:%02x:%02x:%02x\n",
> +out_ethaddr[0], out_ethaddr[1], out_ethaddr[2],
> +out_ethaddr[3], out_ethaddr[4], out_ethaddr[5]));
> +return 1;
> +}
> +}
> +
> +return 0;
> +}
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index 1eb2ed1..ccca93b 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -149,6 +149,7 @@ static void bootp_reply(Slirp *slirp, const struct 
> bootp_t *bp)
>  struct in_addr preq_addr;
>  int dhcp_msg_type, val;
>  uint8_t *q;
> +uint8_t client_ethaddr[ETH_ALEN];
> 
>  /* extract exact DHCP msg type */
>  dhcp_decode(bp, &dhcp_msg_type, &preq_addr);
> @@ -165,7 +166,7 @@ static void bootp_reply(Slirp *slirp, const struct 
> bootp_t *bp)
>  dhcp_msg_type != DHCPREQUEST)
>  return;
>  /* XXX: this is a hack to get the client mac address */
> -memcpy(slirp->client_ethaddr, bp->bp_hwaddr, 6);
> +memcpy(client_ethaddr, bp->bp_hwaddr, ETH_ALEN);

Makes me wonder if the comment above still applies.

> 
>  m = m_get(slirp);
>  if (!m) {
> @@ -178,25 +179,25 @@ static void bootp_reply(Slirp *slirp, const struct 
> bootp_t *bp)
> 
>  if (dhcp_msg_type == DHCPDISCOVER) {
>  if (preq_addr.s_addr != htonl(0L)) {
> -bc = request_addr(slirp, &preq_addr, slirp->client_ethaddr);
> +bc = request_addr(slirp, &preq_addr, client_ethaddr);
>  if (bc) {
>  daddr.sin_addr = preq_addr;
>  }

Re: [Qemu-devel] [PATCH] virtio-serial-bus: replay guest_open on migration

2011-07-27 Thread Alon Levy
On Wed, Jul 27, 2011 at 03:50:11PM +0530, Amit Shah wrote:
> On (Wed) 27 Jul 2011 [10:07:56], Alon Levy wrote:
> > On Wed, Jul 27, 2011 at 07:45:25AM +0200, Markus Armbruster wrote:
> > > Alon Levy  writes:
> > > 
> > > > Signed-off-by: Alon Levy 
> > > > ---
> > > >  hw/virtio-serial-bus.c |8 +++-
> > > >  1 files changed, 7 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > > > index c5eb931..7a652ff 100644
> > > > --- a/hw/virtio-serial-bus.c
> > > > +++ b/hw/virtio-serial-bus.c
> > > > @@ -618,14 +618,20 @@ static int virtio_serial_load(QEMUFile *f, void 
> > > > *opaque, int version_id)
> > > >  for (i = 0; i < nr_active_ports; i++) {
> > > >  uint32_t id;
> > > >  bool host_connected;
> > > > +VirtIOSerialPortInfo *info;
> > > >  
> > > >  id = qemu_get_be32(f);
> > > >  port = find_port_by_id(s, id);
> > > >  if (!port) {
> > > >  return -EINVAL;
> > > >  }
> > > > -
> > > >  port->guest_connected = qemu_get_byte(f);
> > > > +info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info);
> > > > +if (port->guest_connected && info->guest_open) {
> > > > +/* replay guest open */
> > > > +info->guest_open(port);
> > > > +
> > > > +}
> > > >  host_connected = qemu_get_byte(f);
> > > >  if (host_connected != port->host_connected) {
> > > >  /*
> > > 
> > > The patch makes enough sense to me, but the commit message is
> > > insufficient.  Why do you have to replay?  And what's being fixed?
> > 
> > When migrating a host with with a spice agent running the mouse becomes
> > non operational after the migration. This is rhbz #718463, currently on
> > spice-server but it seems this is a qemu-kvm issue. The problem is that
> > after migration spice doesn't know the guest agent is open. Spice is just
> > a char dev here. And a chardev cannot query it's device, the device has
> > to let the chardev know when it is open. Right now after migration the
> > chardev which is recreated is in it's default state, which assumes the
> > guest is disconnected. Char devices carry no information across migration,
> > but the virtio-serial does already carry the guest_connected state. This
> > patch passes that bit to the chardev.
> 
> It's not guaranteed all ports will be chardevs.
> 

The wording may be off, but the code isn't - it doesn't check for chardev,
it calls the same guest_open callback that is called when guest_connected
is changed from 0 to 1. So the logic is:
1. Start from stratch.
2. guest_connected 0->1
3. info->guest_open(port)

And on migration target:
5. if (guest_connected)
6.  info->guest_open(port)

If that callback was non NULL it would be called in a non migration scenario
as well, so no reason to care if it is specifically a chardev or not, let alone
specifically a spice chardev or not.

> My thinking was this can be handled by qemu-char-spice.c since it can
> add a new migration section and if an image is being restored and the
> guest agent channel is open after migration finishes, it can continue
> its work from there.  What's the benefit of all virtio-serial ports
> receiving a guest_open() event in this case?
> 

Consistency between non migration guest open and migrating when guest connected.

> Also, we'll be lying that a guest opened, since a guest was opened
> much earlier, before migration.  Nothing has changed as far as the
> guest is concerned, this is just some host-side tracking that has to
> be done post-migrate, which belongs in individual devices / ports.

The callback is there on purpose, some qemu side users exist surely. While
I understand the lying part about the time, it is worst to lie completely
by not mentioning the guest has opened the port.

> 
> So I'm not completely sure that this is the right place for such a
> notification.  However, if others feel this is fine, I'll accept the
> patch.
> 
> (Also, when resending, make sure the whitespace changes don't go
> through.)
> 

I removed them. We can continue this on the patch - I didn't cc you since
I thought you were already agreed and just wanted Armbru/Juan to take a look.

> Thanks,
>   Amit
> 



[Qemu-devel] [V5 Patch 1/4]Qemu: Enhance "info block" to display host cache setting

2011-07-27 Thread Supriya Kannery
Enhance "info block" to display hostcache setting for each
block device.

Example:
(qemu) info block
ide0-hd0: type=hd removable=0 file=../rhel6-32.qcow2 ro=0 drv=qcow2
encrypted=0

Enhanced to display "hostcache" setting:
(qemu) info block
ide0-hd0: type=hd removable=0 hostcache=true file=../rhel6-32.qcow2
ro=0 drv=qcow2 encrypted=0

Signed-off-by: Supriya Kannery 

---
 block.c |   21 +
 qmp-commands.hx |2 ++
 2 files changed, 19 insertions(+), 4 deletions(-)

Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -1713,6 +1713,14 @@ static void bdrv_print_dict(QObject *obj
 monitor_printf(mon, " locked=%d", qdict_get_bool(bs_dict, "locked"));
 }
 
+if (qdict_haskey(bs_dict, "open_flags")) {
+int open_flags = qdict_get_int(bs_dict, "open_flags");
+if (open_flags & BDRV_O_NOCACHE)
+monitor_printf(mon, " hostcache=0");
+else
+monitor_printf(mon, " hostcache=1");
+}
+
 if (qdict_haskey(bs_dict, "inserted")) {
 QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted"));
 
@@ -1749,13 +1757,18 @@ void bdrv_info(Monitor *mon, QObject **r
 QObject *bs_obj;
 
 bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', "
-"'removable': %i, 'locked': %i }",
-bs->device_name, bs->removable,
-bs->locked);
+ "'removable': %i, 'locked': %i, "
+ "'hostcache': %s }",
+ bs->device_name, bs->removable,
+ bs->locked,
+ (bs->open_flags & BDRV_O_NOCACHE) ?
+ "false" : "true");
+
+QDict *bs_dict = qobject_to_qdict(bs_obj);
+qdict_put(bs_dict, "open_flags", qint_from_int(bs->open_flags));
 
 if (bs->drv) {
 QObject *obj;
-QDict *bs_dict = qobject_to_qdict(bs_obj);
 
 obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, "
  "'encrypted': %i }",
Index: qemu/qmp-commands.hx
===
--- qemu.orig/qmp-commands.hx
+++ qemu/qmp-commands.hx
@@ -1131,6 +1131,7 @@ Each json-object contain the following:
  - Possible values: "unknown"
 - "removable": true if the device is removable, false otherwise (json-bool)
 - "locked": true if the device is locked, false otherwise (json-bool)
+- "hostcache": true if hostcache enabled, false otherwise (json-bool)
 - "inserted": only present if the device is inserted, it is a json-object
containing the following:
  - "file": device file name (json-string)
@@ -1152,6 +1153,7 @@ Example:
  {
 "device":"ide0-hd0",
 "locked":false,
+"hostcache":false,
 "removable":false,
 "inserted":{
"ro":false,



[Qemu-devel] [V5 Patch 2/4]Qemu: qerrors for file reopen, data sync and cmd syntax

2011-07-27 Thread Supriya Kannery
New error classes defined for file reopen failure, data
sync error and incorrect command syntax

Signed-off-by: Supriya Kannery 

---
 qerror.c |   12 
 qerror.h |8 
 2 files changed, 20 insertions(+)

Index: qemu/qerror.c
===
--- qemu.orig/qerror.c
+++ qemu/qerror.c
@@ -97,6 +97,14 @@ static const QErrorStringTable qerror_ta
 .desc  = "Device '%(device)' is not removable",
 },
 {
+.error_fmt = QERR_DATA_SYNC_FAILED,
+.desc  = "Syncing of data failed for device '%(device)'",
+},
+{
+.error_fmt = QERR_REOPEN_FILE_FAILED,
+.desc  = "Could not reopen '%(filename)'",
+},
+{
 .error_fmt = QERR_DEVICE_NO_BUS,
 .desc  = "Device '%(device)' has no child bus",
 },
@@ -230,6 +238,10 @@ static const QErrorStringTable qerror_ta
 .error_fmt = QERR_QGA_COMMAND_FAILED,
 .desc  = "Guest agent command failed, error was '%(message)'",
 },
+{
+.error_fmt = QERR_INCORRECT_COMMAND_SYNTAX,
+.desc  = "Usage: %(syntax)",
+},
 {}
 };
 
Index: qemu/qerror.h
===
--- qemu.orig/qerror.h
+++ qemu/qerror.h
@@ -85,6 +85,9 @@ QError *qobject_to_qerror(const QObject 
 #define QERR_DEVICE_NOT_FOUND \
 "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
 
+#define QERR_DATA_SYNC_FAILED \
+"{ 'class': 'DataSyncFailed', 'data': { 'device': %s } }"
+
 #define QERR_DEVICE_NOT_REMOVABLE \
 "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
 
@@ -142,6 +145,9 @@ QError *qobject_to_qerror(const QObject 
 #define QERR_OPEN_FILE_FAILED \
 "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
 
+#define QERR_REOPEN_FILE_FAILED \
+"{ 'class': 'ReopenFileFailed', 'data': { 'filename': %s } }"
+
 #define QERR_PROPERTY_NOT_FOUND \
 "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
 
@@ -193,4 +199,6 @@ QError *qobject_to_qerror(const QObject 
 #define QERR_QGA_COMMAND_FAILED \
 "{ 'class': 'QgaCommandFailed', 'data': { 'message': %s } }"
 
+#define QERR_INCORRECT_COMMAND_SYNTAX \
+"{ 'class': 'IncorrectCommandSyntax', 'data': { 'syntax': %s } }"
 #endif /* QERROR_H */



[Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change

2011-07-27 Thread Supriya Kannery
New command "block_set" added for dynamically changing any of the block
device parameters. For now, dynamic setting of hostcache params using this
command is implemented. Other block device parameter changes, can be 
integrated in similar lines.

Signed-off-by: Supriya Kannery 

---
 block.c |   54 +
 block.h |2 +
 blockdev.c  |   61 
 blockdev.h  |1 
 hmp-commands.hx |   14 
 qemu-config.c   |   13 +++
 qemu-option.c   |   25 ++
 qemu-option.h   |2 +
 qmp-commands.hx |   28 +
 9 files changed, 200 insertions(+)

Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -651,6 +651,34 @@ unlink_and_fail:
 return ret;
 }
 
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
+{
+BlockDriver *drv = bs->drv;
+int ret = 0, open_flags;
+
+/* Quiesce IO for the given block device */
+qemu_aio_flush();
+if (bdrv_flush(bs)) {
+qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
+return ret;
+}
+open_flags = bs->open_flags;
+bdrv_close(bs);
+
+ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
+if (ret < 0) {
+/* Reopen failed. Try to open with original flags */
+qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+ret = bdrv_open(bs, bs->filename, open_flags, drv);
+if (ret < 0) {
+/* Reopen failed with orig and modified flags */
+abort();
+}
+}
+
+return ret;
+}
+
 void bdrv_close(BlockDriverState *bs)
 {
 if (bs->drv) {
@@ -691,6 +719,32 @@ void bdrv_close_all(void)
 }
 }
 
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache)
+{
+int bdrv_flags = bs->open_flags;
+
+/* set hostcache flags (without changing WCE/flush bits) */
+if (enable_host_cache) {
+bdrv_flags &= ~BDRV_O_NOCACHE;
+} else {
+bdrv_flags |= BDRV_O_NOCACHE;
+}
+
+/* If no change in flags, no need to reopen */
+if (bdrv_flags == bs->open_flags) {
+return 0;
+}
+
+if (bdrv_is_inserted(bs)) {
+/* Reopen file with changed set of flags */
+return bdrv_reopen(bs, bdrv_flags);
+} else {
+/* Save hostcache change for future use */
+bs->open_flags = bdrv_flags;
+return 0;
+}
+}
+
 /* make a BlockDriverState anonymous by removing from bdrv_state list.
Also, NULL terminate the device_name to prevent double remove */
 void bdrv_make_anon(BlockDriverState *bs)
Index: qemu/block.h
===
--- qemu.orig/block.h
+++ qemu/block.h
@@ -71,6 +71,7 @@ void bdrv_delete(BlockDriverState *bs);
 int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
 int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
   BlockDriver *drv);
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags);
 void bdrv_close(BlockDriverState *bs);
 int bdrv_attach(BlockDriverState *bs, DeviceState *qdev);
 void bdrv_detach(BlockDriverState *bs, DeviceState *qdev);
@@ -97,6 +98,7 @@ void bdrv_commit_all(void);
 int bdrv_change_backing_file(BlockDriverState *bs,
 const char *backing_file, const char *backing_fmt);
 void bdrv_register(BlockDriver *bdrv);
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache);
 
 
 typedef struct BdrvCheckResult {
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -793,3 +793,64 @@ int do_block_resize(Monitor *mon, const 
 
 return 0;
 }
+
+
+/*
+ * Handle changes to block device settings, like hostcache,
+ * while guest is running.
+*/
+int do_block_set(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+BlockDriverState *bs = NULL;
+QemuOpts *opts;
+int enable;
+const char *device, *driver;
+int ret;
+char usage[50];
+
+/* Validate device */
+device = qdict_get_str(qdict, "device");
+bs = bdrv_find(device);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict);
+if (opts == NULL) {
+return -1;
+}
+
+/* If input not in "param=value" format, display error */
+driver = qemu_opt_get(opts, "driver");
+if (driver != NULL) {
+qerror_report(QERR_INVALID_PARAMETER, driver);
+return -1;
+}
+
+/* Before validating parameters, remove "device" option */
+ret = qemu_opt_delete(opts, "device");
+if (ret == 1) {
+strcpy(usage,"block_set device [prop=value][,...]");
+qerror_report(QERR_INCORRECT_COMMAND_SYNTAX, usage);
+return 0;
+}
+
+/* Validate parameters with "-drive" parameter

[Qemu-devel] [V5 Patch 4/4]Qemu: Add commandline -drive option 'hostcache'

2011-07-27 Thread Supriya Kannery
qemu command option 'hostcache' added to -drive for block devices.
While starting a VM from qemu commandline, this option can be used 
for setting host cache usage for block data access. It is not 
allowed to specify both 'hostcache' and 'cache' options in the same 
commandline. User has to specify only one among these.

Signed-off-by: Supriya Kannery 

---
 blockdev.c  |   13 +
 qemu-config.c   |4 
 qemu-options.hx |2 +-
 3 files changed, 18 insertions(+), 1 deletion(-)

Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -238,6 +238,7 @@ DriveInfo *drive_init(QemuOpts *opts, in
 DriveInfo *dinfo;
 int snapshot = 0;
 int ret;
+int hostcache = 0;
 
 translation = BIOS_ATA_TRANSLATION_AUTO;
 media = MEDIA_DISK;
@@ -320,7 +321,19 @@ DriveInfo *drive_init(QemuOpts *opts, in
}
 }
 
+if ((hostcache = qemu_opt_get_bool(opts, "hostcache", -1)) != -1) {
+if (!hostcache) {
+bdrv_flags |= BDRV_O_NOCACHE;
+} else {
+bdrv_flags &= ~BDRV_O_NOCACHE;
+}
+}
+
 if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
+if (hostcache != -1) {
+error_report("'hostcache' and 'cache' cannot co-exist");
+return NULL;
+}
 if (!strcmp(buf, "off") || !strcmp(buf, "none")) {
 bdrv_flags |= BDRV_O_NOCACHE | BDRV_O_CACHE_WB;
 } else if (!strcmp(buf, "writeback")) {
Index: qemu/qemu-options.hx
===
--- qemu.orig/qemu-options.hx
+++ qemu/qemu-options.hx
@@ -135,7 +135,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
 "   [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
 "   [,cache=writethrough|writeback|none|unsafe][,format=f]\n"
 "   [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
-"   [,readonly=on|off]\n"
+"   [,readonly=on|off][,hostcache=on|off]\n"
 "use 'file' as a drive image\n", QEMU_ARCH_ALL)
 STEXI
 @item -drive @var{option}[,@var{option}[,@var{option}[,...]]]
Index: qemu/qemu-config.c
===
--- qemu.orig/qemu-config.c
+++ qemu/qemu-config.c
@@ -84,6 +84,10 @@ static QemuOptsList qemu_drive_opts = {
 .name = "readonly",
 .type = QEMU_OPT_BOOL,
 .help = "open drive file as read-only",
+},{
+.name = "hostcache",
+.type = QEMU_OPT_BOOL,
+.help = "set or reset hostcache (on/off)"
 },
 { /* end of list */ }
 },



[Qemu-devel] [V5 Patch 0/4]Qemu: Set host cache from cmdline and monitor

2011-07-27 Thread Supriya Kannery

  Currently cache setting of a block device cannot be changed 
without restarting a running VM. Following patchset is for 
enabling dynamic change of host cache setting for block devices 
through qemu monitor. Code changes are based on patches 
from Christoph Hellwig and Prerna Saxena.

  This patchset introduces monitor command 'block_set'
using which various parameters for block devices can be 
changed dynamically. Dynamic changing of host cache 
setting is implemented for now. This patchset also 
introduces 'hostcache', a new option for setting host
cache from qemu command line. 'Hostcache and 'cache' options 
cannot be used simultaneously from commandline.

Changes from patchset V4:
 1. Defined a new qerror class for incorrect command
syntax.
 2. Changed some of the error_report() calls to
qerror_report() calls


New block command added:
"block_set" 
-- Sets block device parameters while guest is running.
Usage:
 block_set
= block device
 = parameter (say, "hostcache")
 = on/off

New 'hostcache' option added to -drive:
 -drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"

"   [,readonly=on|off][,hostcache=on|off]\n"

 1/4 Enhance "info block" to display hostcache setting
 2/4 New qerrors for file reopen, data sync and command syntax
 3/4 Command "block_set" for dynamic params change for block device
 4/4 'hostcache' option added to -drive in qemu commandline

 qemu/block.c |   75 +
 qemu/block.h |2 +
 qemu/blockdev.c  |   74 +++
 qemu/blockdev.h  |1
 qemu/hmp-commands.hx |   14 +++
 qemu/qemu-config.c   |   17 ++
 qemu/qemu-option.c   |   25 
 qemu/qemu-option.h   |2 +
 qemu/qemu-options.hx |2 -
 qemu/qerror.c|   12 ++
 qemu/qerror.h|8 ++
 qemu/qmp-commands.hx |   30 +++
 12 files changed, 257 insertions(+), 2 deletions(-)
~   






Re: [Qemu-devel] [PATCH] Fix configure display for non-Linux OS's and the KVM / vhost-net features to show "no"

2011-07-27 Thread Jan Kiszka
On 2011-07-27 05:11, Brad wrote:
> diff --git a/configure b/configure
> index dc9a1fe..ef8a4c8 100755
> --- a/configure
> +++ b/configure
> @@ -113,7 +113,6 @@ curl=""
>  curses=""
>  docs=""
>  fdt=""
> -kvm=""
>  nptl=""
>  sdl=""
>  vnc="yes"
> @@ -129,9 +128,10 @@ xen=""
>  xen_ctrl_version=""
>  linux_aio=""
>  attr=""
> -vhost_net=""
>  xfs=""
>  
> +vhost_net="no"
> +kvm="no"
>  gprof="no"
>  debug_tcg="no"
>  debug_mon="no"
> -- 1.7.6

Acked-by: Jan Kiszka 

Should probably be fine for the trivial tree.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH v8 0/5] Coroutines for better asynchronous programming

2011-07-27 Thread Aneesh Kumar K.V
On Wed, 27 Jul 2011 15:15:20 +0530, "Aneesh Kumar K.V" 
 wrote:
> On Tue, 26 Jul 2011 10:21:12 +0100, Stefan Hajnoczi 
>  wrote:
> > QEMU is event-driven and suffers when blocking operations are performed 
> > because
> > VM execution may be stopped until the operation completes.  Therefore many
> > operations that could block are performed asynchronously and a callback is
> > invoked when the operation has completed.  This allows QEMU to continue
> > executing while the operation is pending.
> > 
> > The downside to callbacks is that they split up code into many smaller
> > functions, each of which is a single step in a state machine that quickly
> > becomes complex and hard to understand.  Callback functions also result in 
> > lots
> > of noise as variables are packed and unpacked into temporary structs that 
> > pass
> > state to the callback function.
> > 
> > This patch series introduces coroutines as a solution for writing 
> > asynchronous
> > code while still having a nice sequential control flow.  The semantics are
> > explained in the second patch.  The fourth patch adds automated tests.
> > 
> > A nice feature of coroutines is that it is relatively easy to take 
> > synchronous
> > code and lift it into a coroutine to make it asynchronous.  Work has been 
> > done
> > to move qcow2 request processing into coroutines and thereby make it
> > asynchronous (today qcow2 will perform synchronous metadata accesses).  This
> > qcow2 work is still ongoing and not quite ready for mainline yet.
> > 
> > v8:
> >  * Bisectability: introduce gthread implementation before ucontext/fibers
> 
> Can we also get CoMutex and CoQueue patches also merged. I use them in
> the VirtFS series. 
> 
> http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/coroutine-devel
> http://repo.or.cz/w/qemu/v9fs.git/commit/318ef0b9b01cd296f8c30d8288139b9bed859892
> 

I have pushed VirtFS related changes to

git://repo.or.cz/qemu/v9fs.git for-upstream-1

Once the co-routine patches are merged I can send a proper pull request.
There are more changes pending for VirtFS. We will push them in batches.

The following changes since commit c729e893aba79680bf410a2b9c671fb9e2c2c1a4:

  coroutine: add test-coroutine --benchmark-lifecycle (2011-07-27 16:30:26 
+0530)

are available in the git repository at:
  git://repo.or.cz/qemu/v9fs.git for-upstream-1

Aneesh Kumar K.V (32):
  hw/9pfs: Add yield support for readdir related coroutines
  hw/9pfs: Update v9fs_readdir to use coroutines
  hw/9pfs: Add yield support to statfs coroutine
  hw/9pfs: Update v9fs_statfs to use coroutines
  hw/9pfs: Add yield support to lstat coroutine
  hw/9pfs: Update v9fs_getattr to use coroutines
  hw/9pfs: Add yield support to setattr related coroutines
  hw/9pfs: Update v9fs_setattr to use coroutines
  hw/9pfs: Add yield support to xattr related coroutine
  hw/9pfs: Update v9fs_xattrwalk to coroutines
  hw/9pfs: Update v9fs_xattrcreate to use coroutines
  hw/9pfs: Add yield support to mknod coroutine
  hw/9pfs: Update v9fs_mknod to use coroutines
  hw/9pfs: Add yeild support to rename coroutine
  hw/9pfs: Update vfs_rename to use coroutines
  hw/9pfs: Add yeild support for fstat coroutine
  hw/9pfs: Update v9fs_lock to use coroutines
  hw/9pfs: Update v9fs_getlock to use coroutines
  hw/9pfs: Add yield support for open and opendir coroutine
  hw/9pfs: Update v9fs_open to use coroutines
  hw/9pfs: Update v9fs_stat to use coroutines
  hw/9pfs: Update v9fs_walk to use coroutines
  hw/9pfs: Add yeild support for clunk related coroutine
  hw/9pfs: Update v9fs_clunk to use coroutines
  hw/9pfs: Add yield support for fsync coroutine
  hw/9pfs: Update v9fs_fsync to use coroutines
  hw/9pfs: Add yield support for pwritev coroutine
  hw/9pfs: Update v9fs_write to use coroutines
  hw/9pfs: Update v9fs_wstat to use coroutines
  hw/9pfs: Update v9fs_attach to use coroutines
  hw/9pfs: Add yield support for preadv coroutine
  hw/9pfs: Update v9fs_read to use coroutines

Harsh Prateek Bora (1):
  use readdir_r instead of readdir for reentrancy

Venkateswararao Jujjuri (JV) (20):
  [virtio-9p] Add infrastructure to support glib threads and coroutines.
  [virtio-9p] Change all pdu handlers to coroutines.
  [virtio-9p] Remove post functions for v9fs_readlink.
  [virtio-9p] clean up v9fs_readlink.
  [virtio-9p] coroutines for readlink
  [virtio-9p] Remove post functions for v9fs_mkdir.
  [virtio-9p] clean up v9fs_mkdir.
  [virtio-9p] coroutine and threading for mkdir
  [virtio-9p] Remove post functions for v9fs_remove
  [virtio-9p] clean up v9fs_remove.
  [virtio-9p] coroutine and threading for remove/unlink
  [virtio-9p] Remove post functions for v9fs_lcreate
  [virtio-9p] clean up v9fs_lcreate
  [virtio-9p] coroutine and threading   for open2
  [virtio-9p] Remove post functions for v9fs_

Re: [Qemu-devel] [PATCH v8 0/5] Coroutines for better asynchronous programming

2011-07-27 Thread Aneesh Kumar K.V
On Wed, 27 Jul 2011 12:03:57 +0200, Kevin Wolf  wrote:
> Am 27.07.2011 11:45, schrieb Aneesh Kumar K.V:
> > On Tue, 26 Jul 2011 10:21:12 +0100, Stefan Hajnoczi 
> >  wrote:
> >> QEMU is event-driven and suffers when blocking operations are performed 
> >> because
> >> VM execution may be stopped until the operation completes.  Therefore many
> >> operations that could block are performed asynchronously and a callback is
> >> invoked when the operation has completed.  This allows QEMU to continue
> >> executing while the operation is pending.
> >>
> >> The downside to callbacks is that they split up code into many smaller
> >> functions, each of which is a single step in a state machine that quickly
> >> becomes complex and hard to understand.  Callback functions also result in 
> >> lots
> >> of noise as variables are packed and unpacked into temporary structs that 
> >> pass
> >> state to the callback function.
> >>
> >> This patch series introduces coroutines as a solution for writing 
> >> asynchronous
> >> code while still having a nice sequential control flow.  The semantics are
> >> explained in the second patch.  The fourth patch adds automated tests.
> >>
> >> A nice feature of coroutines is that it is relatively easy to take 
> >> synchronous
> >> code and lift it into a coroutine to make it asynchronous.  Work has been 
> >> done
> >> to move qcow2 request processing into coroutines and thereby make it
> >> asynchronous (today qcow2 will perform synchronous metadata accesses).  
> >> This
> >> qcow2 work is still ongoing and not quite ready for mainline yet.
> >>
> >> v8:
> >>  * Bisectability: introduce gthread implementation before ucontext/fibers
> > 
> > Can we also get CoMutex and CoQueue patches also merged. I use them in
> > the VirtFS series. 
> > 
> > http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/coroutine-devel
> > http://repo.or.cz/w/qemu/v9fs.git/commit/318ef0b9b01cd296f8c30d8288139b9bed859892
> 
> I introduce these in my block coroutine patches. I posted a RFC last
> week and the first "real" patch series yesterday, so I hope they get
> review and can be merged into master soon.
> 

Can you review the patch that add CoRWlock ?

http://article.gmane.org/gmane.comp.emulators.qemu/105402
Message-id:1307382497-3737-2-git-send-email-aneesh.ku...@linux.vnet.ibm.com

commit 8c787d8b81aca1f4f7be45adb67b9e1a6dde7f1f
Author: Aneesh Kumar K.V 
Date:   Tue May 24 22:14:04 2011 +0530

coroutine: Add CoRwlock support

Signed-off-by: Aneesh Kumar K.V 

diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index 5071fb8..5ecaa94 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -115,3 +115,47 @@ void qemu_co_mutex_unlock(CoMutex *mutex)
 
 trace_qemu_co_mutex_unlock_return(mutex, self);
 }
+
+void qemu_co_rwlock_init(CoRwlock *lock)
+{
+memset(lock, 0, sizeof(*lock));
+qemu_co_queue_init(&lock->queue);
+}
+
+void qemu_co_rwlock_rdlock(CoRwlock *lock)
+{
+while (lock->writer) {
+qemu_co_queue_wait(&lock->queue);
+}
+lock->reader++;
+}
+
+void qemu_co_rwlock_unlock(CoRwlock *lock)
+{
+assert(qemu_in_coroutine());
+if (lock->writer) {
+lock->writer--;;
+assert(lock->writer == 0);
+while (!qemu_co_queue_empty(&lock->queue)) {
+/*
+ * Wakeup every body. This will include some
+ * writers too.
+ */
+qemu_co_queue_next(&lock->queue);
+}
+} else {
+lock->reader--;
+assert(lock->reader >= 0);
+/* Wakeup only one waiting writer */
+qemu_co_queue_next(&lock->queue);
+}
+}
+
+void qemu_co_rwlock_wrlock(CoRwlock *lock)
+{
+while (lock->writer || lock->reader) {
+qemu_co_queue_wait(&lock->queue);
+}
+lock->writer++;
+assert(lock->writer == 1);
+}
diff --git a/qemu-coroutine.h b/qemu-coroutine.h
index 71cfa5a..a9735fb 100644
--- a/qemu-coroutine.h
+++ b/qemu-coroutine.h
@@ -17,6 +17,7 @@
 #include 
 
 #include "qemu-queue.h"
+#include "qemu-thread.h"
 
 /**
  * Coroutines are a mechanism for stack switching and can be used for
@@ -114,4 +115,15 @@ void qemu_co_mutex_init(CoMutex *mutex);
 void qemu_co_mutex_lock(CoMutex *mutex);
 void qemu_co_mutex_unlock(CoMutex *mutex);
 
+typedef struct CoRwlock {
+bool writer;
+int reader;
+CoQueue queue;
+} CoRwlock;
+
+void qemu_co_rwlock_init(CoRwlock *lock);
+void qemu_co_rwlock_rdlock(CoRwlock *lock);
+void qemu_co_rwlock_wrlock(CoRwlock *lock);
+void qemu_co_rwlock_unlock(CoRwlock *lock);
+
 #endif /* QEMU_COROUTINE_H */



Re: [Qemu-devel] [Qemu-trivial] [PATCH] Fix configure display for non-Linux OS's and the KVM / vhost-net features to show "no"

2011-07-27 Thread Stefan Hajnoczi
On Wed, Jul 27, 2011 at 12:24 PM, Jan Kiszka  wrote:
> On 2011-07-27 05:11, Brad wrote:
>> diff --git a/configure b/configure
>> index dc9a1fe..ef8a4c8 100755
>> --- a/configure
>> +++ b/configure
>> @@ -113,7 +113,6 @@ curl=""
>>  curses=""
>>  docs=""
>>  fdt=""
>> -kvm=""
>>  nptl=""
>>  sdl=""
>>  vnc="yes"
>> @@ -129,9 +128,10 @@ xen=""
>>  xen_ctrl_version=""
>>  linux_aio=""
>>  attr=""
>> -vhost_net=""
>>  xfs=""
>>
>> +vhost_net="no"
>> +kvm="no"
>>  gprof="no"
>>  debug_tcg="no"
>>  debug_mon="no"
>> -- 1.7.6
>
> Acked-by: Jan Kiszka 
>
> Should probably be fine for the trivial tree.

I sent out a pull request for trivial-patches.  This patch will be
added for the next pull request.

Stefan



Re: [Qemu-devel] [PATCH] HMP: Remove the duplicated info "info kvm" in hmp-commands.hx.

2011-07-27 Thread Stefan Hajnoczi
On Wed, Jul 27, 2011 at 10:48 AM, Zhi Yong Wu  wrote:
> Signed-off-by: Zhi Yong Wu 
> ---
>  hmp-commands.hx |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)

Thanks, will add to the next trivial-patches pull request.

Stefan



[Qemu-devel] [PATCH 1/1] balloon: Ignore negative balloon values

2011-07-27 Thread Amit Shah
Negative balloon values don't make sense, ignore them.

Reported-by: Mike Cao 
Signed-off-by: Amit Shah 
---
I'm not sure if error_report is the right thing to use or should a new
qerror_report() be used.  Luiz, comments?

 balloon.c |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/balloon.c b/balloon.c
index cf9e3b2..e0ff97f 100644
--- a/balloon.c
+++ b/balloon.c
@@ -51,12 +51,16 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
 return 0;
 }
 
-static int qemu_balloon(ram_addr_t target)
+static int qemu_balloon(long long target)
 {
 if (!balloon_event_fn) {
 return 0;
 }
 trace_balloon_event(balloon_opaque, target);
+if (target < 0) {
+error_report("Ignoring negative balloon value");
+return -1;
+}
 balloon_event_fn(balloon_opaque, target);
 return 1;
 }
@@ -150,6 +154,8 @@ int do_balloon(Monitor *mon, const QDict *params,
 if (ret == 0) {
 qerror_report(QERR_DEVICE_NOT_ACTIVE, "balloon");
 return -1;
+} else if (ret < 0) {
+return -1;
 }
 
 cb(opaque, NULL);
-- 
1.7.6




[Qemu-devel] [ANNOUNCE] xen-stable-0.15 qemu branch

2011-07-27 Thread Stefano Stabellini
Hi all,
you might have noticed that there is a number of Xen patches to Qemu
floating around and it is difficul to keep track of them for both Qemu
and Xen maintainers.
For this reason I have setup a git branch to collect them all in a
single place:

git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-stable-0.15

it is based on stable-0.15 and contains all the currently unapplied
patches to run Qemu as device model on Xen. If I have missed one or more
patches please let me know. All of them have been sent to qemu-devel
already, some of them have been acked and not applied, some of them
haven't been acked at all.
In the future I might have to force-push the branch in case one or more
patches require changes by upstream maintainers before being applied,
however I'll try to avoid it if I can.

This is not a new Qemu fork and is not going to become one: if the
author of a patch doesn't work with the upstream Community to get his
patches accepted I am just going to drop them (and I won't even feel bad
about it :) ).

Cheers,

Stefano



Re: [Qemu-devel] [Bug 816860] [NEW] Guest machine freezes when NFS mount goes offline

2011-07-27 Thread Stefan Hajnoczi
On Wed, Jul 27, 2011 at 10:09 AM, Igor Blanco <816...@bugs.launchpad.net> wrote:
> Public bug reported:
>
> I have a virtual KVM machine that has 2 CDROM units with ISOs mounted
> from a NFS mount point. When NFS server goes offline the virtual machine
> blocks completely instead of throwing read errors for the CDROM device.
>
> Host: Proxmox VE 1.8-11 (Debian GNU/Linux 5.0)
> KVM commandline version: QEMU emulator version 0.14.1 (qemu-kvm-devel)
> Guest: Windows 7 professional SP 1

Thanks for reporting this.  There are instances where QEMU performs
blocking operations in a thread that will prevent the guest from
running.  I suspect you are hitting this case and refactoring work
needs to be done to ensure that QEMU threads never block.

Stefan



Re: [Qemu-devel] [PATCH] virtio-serial-bus: replay guest_open on migration

2011-07-27 Thread Amit Shah
On (Wed) 27 Jul 2011 [14:09:45], Alon Levy wrote:

> > Also, we'll be lying that a guest opened, since a guest was opened
> > much earlier, before migration.  Nothing has changed as far as the
> > guest is concerned, this is just some host-side tracking that has to
> > be done post-migrate, which belongs in individual devices / ports.
> 
> The callback is there on purpose, some qemu side users exist surely. While
> I understand the lying part about the time, it is worst to lie completely
> by not mentioning the guest has opened the port.

Guest has not re-opened the port.  When the guest opened the port,
spice did get the callback called.  After migration, guest state has
not changed.  Why should you get the callback again?  If you depend on
guest connectedness, after migration, just ensure you do whatever is
necessary.  I think there's no need to involve any other code in this.

Amit



Re: [Qemu-devel] [PATCH v8 0/5] Coroutines for better asynchronous programming

2011-07-27 Thread Kevin Wolf
Am 27.07.2011 13:39, schrieb Aneesh Kumar K.V:
> Can you review the patch that add CoRWlock ?
> 
> http://article.gmane.org/gmane.comp.emulators.qemu/105402
> Message-id:1307382497-3737-2-git-send-email-aneesh.ku...@linux.vnet.ibm.com
> 
> commit 8c787d8b81aca1f4f7be45adb67b9e1a6dde7f1f
> Author: Aneesh Kumar K.V 
> Date:   Tue May 24 22:14:04 2011 +0530
> 
> coroutine: Add CoRwlock support
> 
> Signed-off-by: Aneesh Kumar K.V 

Nice! I think I'll need this, too.

> diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
> index 5071fb8..5ecaa94 100644
> --- a/qemu-coroutine-lock.c
> +++ b/qemu-coroutine-lock.c
> @@ -115,3 +115,47 @@ void qemu_co_mutex_unlock(CoMutex *mutex)
>  
>  trace_qemu_co_mutex_unlock_return(mutex, self);
>  }
> +
> +void qemu_co_rwlock_init(CoRwlock *lock)
> +{
> +memset(lock, 0, sizeof(*lock));
> +qemu_co_queue_init(&lock->queue);
> +}
> +
> +void qemu_co_rwlock_rdlock(CoRwlock *lock)
> +{
> +while (lock->writer) {
> +qemu_co_queue_wait(&lock->queue);
> +}
> +lock->reader++;
> +}
> +
> +void qemu_co_rwlock_unlock(CoRwlock *lock)
> +{
> +assert(qemu_in_coroutine());
> +if (lock->writer) {
> +lock->writer--;;

Please don't do arithmetics on bools, just say lock->write = false;

Also there's a double semicolon.

> +assert(lock->writer == 0);
> +while (!qemu_co_queue_empty(&lock->queue)) {
> +/*
> + * Wakeup every body. This will include some
> + * writers too.
> + */
> +qemu_co_queue_next(&lock->queue);
> +}
> +} else {
> +lock->reader--;
> +assert(lock->reader >= 0);
> +/* Wakeup only one waiting writer */
> +qemu_co_queue_next(&lock->queue);

This is only useful if lock->reader == 0.

> +}
> +}
> +
> +void qemu_co_rwlock_wrlock(CoRwlock *lock)
> +{
> +while (lock->writer || lock->reader) {
> +qemu_co_queue_wait(&lock->queue);
> +}
> +lock->writer++;
> +assert(lock->writer == 1);
> +}

I wonder if we should have a mechanism that stops new readers from
taking the lock while a writer is waiting in order to avoid starvation.

Anyway, the locking itself looks correct.

Kevin



Re: [Qemu-devel] [PATCH v5 01/18] Add hard build dependency on glib

2011-07-27 Thread Kenneth Salerno
--- On Tue, 7/26/11, Kenneth Salerno  wrote:

> From: Kenneth Salerno 
> Subject: Re: [Qemu-devel] [PATCH v5 01/18] Add hard build dependency on glib
> To: qemu-devel@nongnu.org
> Date: Tuesday, July 26, 2011, 10:02 AM
> From:     Michael
> Roth
> Subject:     [Qemu-devel] [PATCH v5 01/18]
> Add hard build dependency on glib
> Date:     Tue, 5 Jul 2011 08:02:28 -0500
> 
> > From: Anthony Liguori 
> >
> > GLib is an extremely common library that has a
> portable thread implementation
> > along with tons of other goodies.
> >
> > GLib and GObject have a fantastic amount of
> infrastructure we can leverage in
> > QEMU including an object oriented programming
> infrastructure.
> >
> > Short term, it has a very nice thread pool
> implementation that we could leverage
> > in something like virtio-9p.  It also has a test
> harness implementation that
> > this series will use.
> >
> > Signed-off-by: Anthony Liguori 
> > Signed-off-by: Michael Roth 
> > ---
> > Makefile        |    2
> ++
> > Makefile.objs   |    2 ++
> > Makefile.target |    1 +
> > configure   
>    |   13 +
> > 4 files changed, 18 insertions(+), 0 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index b3ffbe2..42ae4e5 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -106,6 +106,8 @@ audio/audio.o audio/fmodaudio.o:
> QEMU_CFLAGS += 
> > $(FMOD_CFLAGS)
> > 
> > QEMU_CFLAGS+=$(CURL_CFLAGS)
> > 
> > +QEMU_CFLAGS+=$(GLIB_CFLAGS)
> > +
> > ui/cocoa.o: ui/cocoa.m
> > 
> > ui/sdl.o audio/sdlaudio.o ui/sdl_zoom.o baum.o:
> QEMU_CFLAGS += $(SDL_CFLAGS)
> > diff --git a/Makefile.objs b/Makefile.objs
> > index cea15e4..493c988 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -376,3 +376,5 @@ vl.o:
> QEMU_CFLAGS+=$(GPROF_CFLAGS)
> >  
> >  vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
> >  
> > +vl.o: QEMU_CFLAGS+=$(GLIB_CFLAGS)
> > +
> > diff --git a/Makefile.target b/Makefile.target
> > index a53a2ff..b8256ae 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -203,6 +203,7 @@ QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
> >  QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
> >  QEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
> >  QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
> > +QEMU_CFLAGS += $(GLIB_CFLAGS)
> >  
> >  # xen backend driver support
> >  obj-i386-$(CONFIG_XEN) += xen_machine_pv.o
> xen_domainbuild.o
> > diff --git a/configure b/configure
> > index 88159ac..63156a2 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1803,6 +1803,18 @@ EOF
> >  fi
>  
> >  ##
> > +# glib support probe
> > +if $pkg_config --modversion gthread-2.0 gio-2.0 >
> /dev/null 2>&1 ; then
> > +    glib_cflags=`$pkg_config --cflags
> gthread-2.0 gio-2.0 2>/dev/null`
> > +    glib_libs=`$pkg_config --libs
> gthread-2.0 gio-2.0 2>/dev/null`
> > +    libs_softmmu="$glib_libs
> $libs_softmmu"
> > +    libs_tools="$glib_libs $libs_tools"
> > +else
> > +    echo "glib-2.0 required to compile
> QEMU"
> > +    exit 1
> > +fi
> > +
> > +##
> >  # pthread probe
> >  PTHREADLIBS_LIST="-lpthread -lpthreadGC2"
> >  
> > @@ -2849,6 +2861,7 @@ if test "$bluez" = "yes" ; then
> >    echo "CONFIG_BLUEZ=y" >>
> $config_host_mak
> >    echo "BLUEZ_CFLAGS=$bluez_cflags"
> >> $config_host_mak
> >  fi
> > +echo "GLIB_CFLAGS=$glib_cflags" >>
> $config_host_mak
> >  if test "$xen" = "yes" ; then
> >    echo "CONFIG_XEN=y" >>
> $config_host_mak
> >    echo
> "CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version"
> >> 
> > $config_host_mak
> > -- 
> > 1.7.0.4
> 
> 
> 
> That sounds like a good idea, especially if it will help
> resolve the multi-threading issue in Cygwin which creates
> the need to pin QEMU to one CPU core.
> 
> However, this has broken the MinGW build in Cygwin because
> there is no Glib2 (or gettext or libiconv) built and
> distributed for MinGW by the Cygwin project which means I
> now have to compile those packages myself as a prerequisite
> if I want to try to build the latest snapshot of QEMU
> development code (and future releases of QEMU stable for
> that matter)...
> 


Ignore. My apologies - I didn't have pkg-config installed.

Ken



Re: [Qemu-devel] [PATCH] virtio-serial-bus: replay guest_open on migration

2011-07-27 Thread Alon Levy
On Wed, Jul 27, 2011 at 05:35:28PM +0530, Amit Shah wrote:
> On (Wed) 27 Jul 2011 [14:09:45], Alon Levy wrote:
> 
> > > Also, we'll be lying that a guest opened, since a guest was opened
> > > much earlier, before migration.  Nothing has changed as far as the
> > > guest is concerned, this is just some host-side tracking that has to
> > > be done post-migrate, which belongs in individual devices / ports.
> > 
> > The callback is there on purpose, some qemu side users exist surely. While
> > I understand the lying part about the time, it is worst to lie completely
> > by not mentioning the guest has opened the port.
> 
> Guest has not re-opened the port.  When the guest opened the port,
> spice did get the callback called.  After migration, guest state has
> not changed.  Why should you get the callback again?
Again, my point is that from the chardev pov this *is* the first callback.
You say the timing is wrong - correct, but I don't see any part in the api
that talks about timing, i.e. no gurantee to be broken.

Actually searching for current users it appears it is just spicevmc 
(spice-qemu-char.c).

> If you depend on
> guest connectedness, after migration, just ensure you do whatever is
> necessary.  I think there's no need to involve any other code in this.

There is no migration mechanism for char devices. There is no migration
mechanism for the spice server. All data that is passed is done via device 
migration -
qxl or in this case I suggest virtio-serial.

Do you think chardevices should be migratable? that would also work.

Do you think a separate callback would be better worded? or add a "migrated" 
boolean?
both are ugly, I agree.

The chardev does receive parameters, it's possible to have the remote vm start 
with
a spicevmc chardev that has "migrated=1" as part of the command line. That 
looks much
uglier then this patch to me.

> 
>   Amit
> 



[Qemu-devel] Block layer roadmap

2011-07-27 Thread Stefan Hajnoczi
Hi,
Here is a list of block layer and storage changes that have been discussed.  It
is useful to have a roadmap of changes in order to avoid duplication, allow
more developers to contribute, and to communicate the direction of storage in
QEMU.

I suggest we first do a braindump of all changes that have been discussed.
Later we can discuss specific changes and if/when they fit into the roadmap -
please don't jump into discussion about specific changes yet.

Kevin: I hope this a useful starting point.  Here are all the items
that I am aware of:

=Material for next QEMU release=

Coroutines in the block layer [Kevin]
 * Programming model to simplify block drivers without blocking QEMU threads

Generic copy-on-read [Stefan]
 * Populate image file to avoid fetching same block from backing file
again later

Generic image streaming [Stefan]
 * Make block_stream commands available for all image formats that
support backing files

Live block copy [Marcelo/Kevin/Stefan?]
 * Copy the contents of an image file while a guest is using it

In-place qcow2 <-> qed conversion [Devin, GSoC 2011]:
 * Fast conversion between qcow2 and qed image formats without copy all data

VMDK enhancements [Fam, GSoC 2011]
 * Implement latest VMDK specs to support modern image files

Block I/O limits [Zhi Yong]
 * Resource control for guest I/O bandwidth/iops consumption

=Changes where I am not aware of firm plans=

Cow overlay [Dong Xu "Robert"]
 * Allow live block copy and image streaming to raw destination files

snapshot_blkdev and Backup API [Jes, Jagane]
 * Support for consistent disk snapshots and dirty block tracking
 * Allow backup software to integrate with QEMU

-blockdev [Markus?]
 * Explicit user control over block device trees

QCOW3
 * Extend qcow2 format to address current and future image format challenges

iSCSI/NBD/Remote block device integration
 * Enable remote access to disk images for live migration and other tasks

Pre/post block copy
 * Working block migration

Avoid blocking QEMU threads
 * Today loss of NFS connectivity can hang guests
 * It's critical never to block the vcpu thread
 * The iothread should also not block while the qemu mutex is held
 * All blocking operations must be done asynchronously or in a worker thread

virtio-scsi [Paolo/Stefan]
 * The next step after virtio-blk, full SCSI command set and appears
as SCSI HBA in guest

tcm_vhost [Stefan]
 * Directly connect virtio-scsi with Linux in-kernel SCSI target
 * Pass-through of host SCSI devices



Re: [Qemu-devel] [PATCH RESEND v3] xen: implement unplug protocol in xen_platform

2011-07-27 Thread Alexander Graf

On 07/18/2011 06:07 PM, stefano.stabell...@eu.citrix.com wrote:

From: Stefano Stabellini

The unplug protocol is necessary to support PV drivers in the guest: the
drivers expect to be able to "unplug" emulated disks and nics before
initializing the Xen PV interfaces.
It is responsibility of the guest to make sure that the unplug is done
before the emulated devices or the PV interface start to be used.

We use pci_for_each_device to walk the PCI bus, identify the devices and
disks that we want to disable and dynamically unplug them.

Changes in v2:

- use PCI_CLASS constants;

- replace pci_unplug_device with qdev_unplug;

- do not import hw/ide/internal.h in xen_platform.c;


Changes in v3:

- introduce piix3-ide-xen, that support hot-unplug;

- move the unplug code to hw/ide/piix.c;

- just call qdev_unplug from xen_platform.c to unplug the IDE disks;

Signed-off-by: Stefano Stabellini


Kevin, please ack.


Alex




Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

2011-07-27 Thread Anthony Liguori

On 07/27/2011 03:55 AM, Paolo Bonzini wrote:

Yes, this looks nice (modulo s/Rtl8139/Rtl8139 */). But it is not that
much more flexible than qdev 1.0.

You're right that for the case of two parents above we were looking at a
contrived example. The Goldfish platform provides a more interesting
one. There you have an interrupt controller and a bus enumerator device.
Most devices are connected to both, but conflating them is wrong for
many reasons;

1) the bus enumerator itself uses an interrupt (raised on hotplug);

2) you can enumerate devices that do not have an interrupt line, and you
shouldn't need to connect such a device to an interrupt controller;

3) the interrupt controller and bus enumerator use two separate MMIO areas;

4) in principle, other devices could use the interrupt controller (which
is the only component connected to the CPU's interrupt line) without
being enumerated.

5) A device with two interrupts has two "interrupt interfaces" and only
one "enumerator interface".

6) The enumerator interface does not have bus semantics. The enumerator
also enumerates itself so it would act as both the bus host and a device
on the bus.


I think I understand what your saying here.  But interrupt routing is an 
interesting problem and I've been thinking about it differently then we 
do it today.


The core is:

class Pin : public Device {
   bool level;
};

You can connect a signal to the level to detect edge changes.  You can 
also connect two pins together such that if one raises high, the other 
raises high.


An interrupt controller looks like:

struct InterruptController
{
   Pin *irq[256];
};

In the simple case, you have:

struct UART : public Device
{
   Pin irq;
};

And then you'd set the irq by doing:

apic = new InterruptController()
uart = new UART()

apic->irq[0] = &uart->irq;

Or at the qmp layer:

(qemu) plug_get uart irq
uart::irq
(qemu) plug_set apic irq[0] uart::irq

I mention this because I don't think your example below assumes a model 
like this.



Composition then lets you use something like this:

class GoldfishPIC : Device {
Pin parent;
GoldfishInterruptPin *children[32];
Pin (*init_socket_for_irq_num) (GoldfishInterruptPin *, int);
};


So the idea here is that the PIC will multiplex a bunch of interrupts 
into a single line?  I would do this simply as:


class GoldfishPIC : Device {
   Pin out;
   Pin *in[32];
};

The IRQ number is implicit in the socket index, no?  I'm also not sure 
that there's a strong need to have a typed Pin.




class GoldfishInterruptPin {
GoldfishPIC *pic;
Pin irqnum;
};

class GoldfishEnumerator : Device {
GoldfishInterruptPin irq;
GoldfishBusDeviceInfo info;
List allDevices;
...
};

class GoldfishBusDeviceInfo {
GoldfishEnumerator *parent;
char *name;
int id;
ram_addr_t mmio;
int mmio_size;
int irq_base;
int irq_count;
};


Is the enumerator something that has an interface to devices where the 
devices hold this info?  Or is the enumerator just a bank of flash 
that's preprogrammed with fixed info?


If it's the later, I would suggest that we model is in that fashion.  No 
need to teach every single device about the enumerator if they wouldn't 
normally have information about it.



We need lots of new transitions, we need to strive to make things
better. But we need to do things in such a way that:

(1) we have a good idea of what we're going to end up with at the end of
the day

(2) there is incremental value being added to QEMU at every step of
the way

This cannot be done by simply hacking some bits here and there. It
requires design, planning, and flag days when appropriate.


Agreed. The problem I have with QOM is (2). I am not sure we do get
incremental value at every step of the way. We do get incremental value
in the char layer, but we also get additional complexity until the
transition is over.


So roughly speaking, my plan is:

1) Char layer
 - we get dynamic add of CDS, which enables hot plug of virtio-serial
 - we get ability to change CDS properties dynamically

2) Block layer
 - we get -blockdev

3) Display layer
 - we get dynamic add of VGA devices

4) fsdev
 - dynamic add of virtio-9p devices

5) network layer
 - no new features, but better commonality

At each phase, we also get significantly better modularity.  The block 
layer is already good here, but the other backends aren't.


My only real concern is how to attack the device layer incrementally.  I 
don't think it's impossible but it requires careful thought.


I think we can probably retrofit DeviceState as a QOM base class and 
just systematically convert the types over to QOM.  The next step would 
be to change the property registration to be QOM-like.  I think they we 
could probably push out a lot of what's in DeviceState today into 
another base class, then introduce a better Device base class.  Since a 
lot of subsystems should just inherit from Device, that gives us a nice 
way to attack things one subsystem at a time.


I think an approach like this can have increm

Re: [Qemu-devel] Block layer roadmap

2011-07-27 Thread Anthony Liguori

On 07/27/2011 07:37 AM, Stefan Hajnoczi wrote:

Hi,
Here is a list of block layer and storage changes that have been discussed.  It
is useful to have a roadmap of changes in order to avoid duplication, allow
more developers to contribute, and to communicate the direction of storage in
QEMU.


Thanks for writing this up Stefan!


I suggest we first do a braindump of all changes that have been discussed.
Later we can discuss specific changes and if/when they fit into the roadmap -
please don't jump into discussion about specific changes yet.

Kevin: I hope this a useful starting point.  Here are all the items
that I am aware of:

=Material for next QEMU release=

Coroutines in the block layer [Kevin]
  * Programming model to simplify block drivers without blocking QEMU threads

Generic copy-on-read [Stefan]
  * Populate image file to avoid fetching same block from backing file
again later

Generic image streaming [Stefan]
  * Make block_stream commands available for all image formats that
support backing files

Live block copy [Marcelo/Kevin/Stefan?]
  * Copy the contents of an image file while a guest is using it

In-place qcow2<->  qed conversion [Devin, GSoC 2011]:
  * Fast conversion between qcow2 and qed image formats without copy all data

VMDK enhancements [Fam, GSoC 2011]
  * Implement latest VMDK specs to support modern image files

Block I/O limits [Zhi Yong]
  * Resource control for guest I/O bandwidth/iops consumption

=Changes where I am not aware of firm plans=

Cow overlay [Dong Xu "Robert"]
  * Allow live block copy and image streaming to raw destination files

snapshot_blkdev and Backup API [Jes, Jagane]
  * Support for consistent disk snapshots and dirty block tracking
  * Allow backup software to integrate with QEMU

-blockdev [Markus?]
  * Explicit user control over block device trees


I'm planning on helping out here however I can.

Regards,

Anthony Liguori


QCOW3
  * Extend qcow2 format to address current and future image format challenges

iSCSI/NBD/Remote block device integration
  * Enable remote access to disk images for live migration and other tasks

Pre/post block copy
  * Working block migration

Avoid blocking QEMU threads
  * Today loss of NFS connectivity can hang guests
  * It's critical never to block the vcpu thread
  * The iothread should also not block while the qemu mutex is held
  * All blocking operations must be done asynchronously or in a worker thread

virtio-scsi [Paolo/Stefan]
  * The next step after virtio-blk, full SCSI command set and appears
as SCSI HBA in guest

tcm_vhost [Stefan]
  * Directly connect virtio-scsi with Linux in-kernel SCSI target
  * Pass-through of host SCSI devices






Re: [Qemu-devel] [PATCH V2 3/3] vl.c: Check the asked ram_size later.

2011-07-27 Thread Alexander Graf

On 07/20/2011 08:17 PM, Anthony PERARD wrote:

As a Xen guest can have more than 2GB of RAM on a 32bit host, we move
the conditions after than we now if we run one Xen or not.

Signed-off-by: Anthony PERARD
---
  vl.c |   14 --
  1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/vl.c b/vl.c
index fcd7395..c2efedf 100644
--- a/vl.c
+++ b/vl.c
@@ -2433,11 +2433,6 @@ int main(int argc, char **argv, char **envp)
  exit(1);
  }

-/* On 32-bit hosts, QEMU is limited by virtual address space */
-if (value>  (2047<<  20)&&  HOST_LONG_BITS == 32) {
-fprintf(stderr, "qemu: at most 2047 MB RAM can be 
simulated\n");
-exit(1);
-}
  if (value != (uint64_t)(ram_addr_t)value) {
  fprintf(stderr, "qemu: ram size too large\n");
  exit(1);
@@ -3091,8 +3086,15 @@ int main(int argc, char **argv, char **envp)
  exit(1);

  /* init the memory */
-if (ram_size == 0)
+if (ram_size == 0) {
  ram_size = DEFAULT_RAM_SIZE * 1024 * 1024;
+} else if (!xen_enabled()) {


I don't see why those are mutually exclusive.


+/* On 32-bit hosts, QEMU is limited by virtual address space */
+if (ram_size>  (2047<<  20)&&  HOST_LONG_BITS == 32) {
+fprintf(stderr, "qemu: at most 2047 MB RAM can be simulated\n");
+exit(1);
+}
+}

  /* init the dynamic translator */
  cpu_exec_init_all(tb_size * 1024 * 1024);


Alex




Re: [Qemu-devel] [PATCH RESEND v3] xen: implement unplug protocol in xen_platform

2011-07-27 Thread Kevin Wolf
Am 27.07.2011 14:44, schrieb Alexander Graf:
> On 07/18/2011 06:07 PM, stefano.stabell...@eu.citrix.com wrote:
>> From: Stefano Stabellini
>>
>> The unplug protocol is necessary to support PV drivers in the guest: the
>> drivers expect to be able to "unplug" emulated disks and nics before
>> initializing the Xen PV interfaces.
>> It is responsibility of the guest to make sure that the unplug is done
>> before the emulated devices or the PV interface start to be used.
>>
>> We use pci_for_each_device to walk the PCI bus, identify the devices and
>> disks that we want to disable and dynamically unplug them.
>>
>> Changes in v2:
>>
>> - use PCI_CLASS constants;
>>
>> - replace pci_unplug_device with qdev_unplug;
>>
>> - do not import hw/ide/internal.h in xen_platform.c;
>>
>>
>> Changes in v3:
>>
>> - introduce piix3-ide-xen, that support hot-unplug;
>>
>> - move the unplug code to hw/ide/piix.c;
>>
>> - just call qdev_unplug from xen_platform.c to unplug the IDE disks;
>>
>> Signed-off-by: Stefano Stabellini
> 
> Kevin, please ack.

Trivial rebase of the version I already acked.

Acked-by: Kevin Wolf 



Re: [Qemu-devel] [PATCH 1/2] [SLIRP] Simple ARP table

2011-07-27 Thread Fabien Chouteau
On 27/07/2011 12:49, Jan Kiszka wrote:
> On 2011-07-26 18:21, Fabien Chouteau wrote:
>> This patch adds a simple ARP table in Slirp and also adds handling of
>> gratuitous ARP requests.
>>
>> Signed-off-by: Fabien Chouteau 
>> ---
>>  Makefile.objs |2 +-
>>  slirp/arp_table.c |   61 +++
>>  slirp/bootp.c |   15 ++-
>>  slirp/slirp.c |   68 
>> 
>>  slirp/slirp.h |   51 +--
>>  5 files changed, 139 insertions(+), 58 deletions(-)
>>  create mode 100644 slirp/arp_table.c
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 6991a9f..0c10557 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -151,7 +151,7 @@ common-obj-y += qemu-timer.o qemu-timer-common.o
>>
>>  slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o
>>  slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o 
>> tcp_output.o
>> -slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
>> +slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o
>>  common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
>>
>>  # xen backend driver support
>> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
>> new file mode 100644
>> index 000..e3251ed
>> --- /dev/null
>> +++ b/slirp/arp_table.c
>> @@ -0,0 +1,61 @@
>> +#include "slirp.h"
>> +
>> +void arp_table_flush(ArpTable *arptbl)
>> +{
>> +int i;
>> +
>> +assert(arptbl != NULL);
>> +
>> +for (i = 0; i < ARP_TABLE_SIZE; i++) {
>> +arptbl->table[i].ar_sip = 0;
>> +}
>> +arptbl->next_victim = 0;
>> +}
> 
> arp_table_flush is only used for initialization, but the memory it
> touches is already zero-initialized (on alloc in slirp_init). So you can
> save this service.

OK, it was just in case slirp can be re-initialized.

>> +{
>> +int i;
>> +
>> +DEBUG_CALL("arp_table_add");
>> +DEBUG_ARG("ip = 0x%x", ahdr->ar_sip);
>> +DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
>> +ahdr->ar_sha[0], ahdr->ar_sha[1], ahdr->ar_sha[2],
>> +ahdr->ar_sha[3], ahdr->ar_sha[4], ahdr->ar_sha[5]));
>> +
>> +/* Search for an entry */
>> +for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++) {
>> +if (arptbl->table[i].ar_sip == ahdr->ar_sip) {
>> +/* Update the entry */
>> +memcpy(arptbl->table[i].ar_sha,  ahdr->ar_sha, ETH_ALEN);
>> +return;
>> +}
>> +}
>> +
>> +/* No entry found, create a new one */
>> +arptbl->table[arptbl->next_victim].ar_sip = ahdr->ar_sip;
>> +memcpy(arptbl->table[arptbl->next_victim].ar_sha,  ahdr->ar_sha, 
>> ETH_ALEN);
>> +arptbl->next_victim = (arptbl->next_victim + 1) % ARP_TABLE_SIZE;
>
> Pragmatic aging. But I guess that's OK, we shouldn't need anything
> smarter here.

Yes, simple round-robin.

>> diff --git a/slirp/bootp.c b/slirp/bootp.c
>> index 1eb2ed1..ccca93b 100644
>> --- a/slirp/bootp.c
>> +++ b/slirp/bootp.c
>> @@ -149,6 +149,7 @@ static void bootp_reply(Slirp *slirp, const struct 
>> bootp_t *bp)
>>  struct in_addr preq_addr;
>>  int dhcp_msg_type, val;
>>  uint8_t *q;
>> +uint8_t client_ethaddr[ETH_ALEN];
>>
>>  /* extract exact DHCP msg type */
>>  dhcp_decode(bp, &dhcp_msg_type, &preq_addr);
>> @@ -165,7 +166,7 @@ static void bootp_reply(Slirp *slirp, const struct 
>> bootp_t *bp)
>>  dhcp_msg_type != DHCPREQUEST)
>>  return;
>>  /* XXX: this is a hack to get the client mac address */
>> -memcpy(slirp->client_ethaddr, bp->bp_hwaddr, 6);
>> +memcpy(client_ethaddr, bp->bp_hwaddr, ETH_ALEN);
> 
> Makes me wonder if the comment above still applies.

Actually I don't think it is a hack at all.

Makes me think I should update the ARP table here, with the IP address assigned
to this client.

Thanks for your review, I will fix the others style problems.

Regards,

-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH V2 0/3] Enable QEMU to handle more than 2GB with Xen.

2011-07-27 Thread Alexander Graf

On 07/20/2011 08:17 PM, Anthony PERARD wrote:

Hi all,

Update on this series:
   - Use a RAM address of 64bits only on 64bits targets when Xen is enable.
   - Add some comment on the memory registration done for Xen.


Xen is not limited by the QEMU's virtual address space for the allocation of
the guest RAM. So even with a QEMU 32bits, a Xen guest can have more than 4 GB
of RAM.

With this serie, we will be able to run a guest with more than 4GB. The main
point is to change ram_addr_t from ulong to uin64 when QEMU is configure with
Xen. The second point is better register the memory in QEMU.

Regards,

Anthony PERARD (3):
   cpu-common: Have a ram_addr_t of uint64 with Xen.
   xen: Fix the memory registration to reflect of what is done by Xen.
   vl.c: Check the asked ram_size later.

  cpu-common.h |8 
  exec.c   |9 +
  vl.c |   14 --
  xen-all.c|   29 +
  4 files changed, 42 insertions(+), 18 deletions(-)


Thanks, applied all to xen-next. I also squashed the following patch 
into 3/3:


diff --git a/vl.c b/vl.c
index 24df37f..d8c7c01 100644
--- a/vl.c
+++ b/vl.c
@@ -3096,7 +3096,9 @@ int main(int argc, char **argv, char **envp)
 /* init the memory */
 if (ram_size == 0) {
 ram_size = DEFAULT_RAM_SIZE * 1024 * 1024;
-} else if (!xen_enabled()) {
+}
+
+if (!xen_enabled()) {
 /* On 32-bit hosts, QEMU is limited by virtual address space */
 if (ram_size > (2047 << 20) && HOST_LONG_BITS == 32) {
 fprintf(stderr, "qemu: at most 2047 MB RAM can be 
simulated\n");



Alex




Re: [Qemu-devel] [PATCH v2 1/1] The codes V2 for QEMU disk I/O limits.

2011-07-27 Thread Stefan Hajnoczi
On Wed, Jul 27, 2011 at 11:17 AM, Zhi Yong Wu  wrote:
> On Wed, Jul 27, 2011 at 3:26 AM, Marcelo Tosatti  wrote:
>> On Tue, Jul 26, 2011 at 04:59:06PM +0800, Zhi Yong Wu wrote:
>>> Welcome to give me your comments, thanks.
>>>
>>> Signed-off-by: Zhi Yong Wu 
>>> ---
>>>  Makefile.objs     |    2 +-
>>>  block.c           |  288 
>>> +++--
>>>  block.h           |    1 -
>>>  block/blk-queue.c |  116 +
>>>  block/blk-queue.h |   70 +
>>>  block_int.h       |   28 +
>>>  blockdev.c        |   21 
>>>  qemu-config.c     |   24 +
>>>  qemu-option.c     |   17 +++
>>>  qemu-option.h     |    1 +
>>>  qemu-options.hx   |    1 +
>>>  11 files changed, 559 insertions(+), 10 deletions(-)
>>>  create mode 100644 block/blk-queue.c
>>>  create mode 100644 block/blk-queue.h
>>>
>>> diff --git a/Makefile.objs b/Makefile.objs
>>> index 9f99ed4..06f2033 100644
>>> --- a/Makefile.objs
>>> +++ b/Makefile.objs
>>> @@ -23,7 +23,7 @@ block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o 
>>> dmg.o bochs.o vpc.o vv
>>>  block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o 
>>> qcow2-snapshot.o qcow2-cache.o
>>>  block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o 
>>> qed-cluster.o
>>>  block-nested-y += qed-check.o
>>> -block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>>> +block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o 
>>> blk-queue.o
>>>  block-nested-$(CONFIG_WIN32) += raw-win32.o
>>>  block-nested-$(CONFIG_POSIX) += raw-posix.o
>>>  block-nested-$(CONFIG_CURL) += curl.o
>>> diff --git a/block.c b/block.c
>>> index 24a25d5..e54e59c 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -29,6 +29,9 @@
>>>  #include "module.h"
>>>  #include "qemu-objects.h"
>>>
>>> +#include "qemu-timer.h"
>>> +#include "block/blk-queue.h"
>>> +
>>>  #ifdef CONFIG_BSD
>>>  #include 
>>>  #include 
>>> @@ -58,6 +61,13 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t 
>>> sector_num,
>>>  static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
>>>                           const uint8_t *buf, int nb_sectors);
>>>
>>> +static bool bdrv_exceed_bps_limits(BlockDriverState *bs, int nb_sectors,
>>> +        bool is_write, double elapsed_time, uint64_t *wait);
>>> +static bool bdrv_exceed_iops_limits(BlockDriverState *bs, bool is_write,
>>> +        double elapsed_time, uint64_t *wait);
>>> +static bool bdrv_exceed_io_limits(BlockDriverState *bs, int nb_sectors,
>>> +        bool is_write, uint64_t *wait);
>>> +
>>>  static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
>>>      QTAILQ_HEAD_INITIALIZER(bdrv_states);
>>>
>>> @@ -90,6 +100,20 @@ int is_windows_drive(const char *filename)
>>>  }
>>>  #endif
>>>
>>> +static int bdrv_io_limits_enable(BlockIOLimit *io_limits)
>>> +{
>>> +    if ((io_limits->bps[0] == 0)
>>> +         && (io_limits->bps[1] == 0)
>>> +         && (io_limits->bps[2] == 0)
>>> +         && (io_limits->iops[0] == 0)
>>> +         && (io_limits->iops[1] == 0)
>>> +         && (io_limits->iops[2] == 0)) {
>>> +        return 0;
>>> +    }
>>> +
>>> +    return 1;
>>> +}
>>> +
>>>  /* check if the path starts with ":" */
>>>  static int path_has_protocol(const char *path)
>>>  {
>>> @@ -167,6 +191,28 @@ void path_combine(char *dest, int dest_size,
>>>      }
>>>  }
>>>
>>> +static void bdrv_block_timer(void *opaque)
>>> +{
>>> +    BlockDriverState *bs = opaque;
>>> +    BlockQueue *queue = bs->block_queue;
>>> +
>>> +    while (!QTAILQ_EMPTY(&queue->requests)) {
>>> +        BlockIORequest *request;
>>> +        int ret;
>>> +
>>> +        request = QTAILQ_FIRST(&queue->requests);
>>> +        QTAILQ_REMOVE(&queue->requests, request, entry);
>>> +
>>> +        ret = qemu_block_queue_handler(request);
>>> +        if (ret == 0) {
>>> +            QTAILQ_INSERT_HEAD(&queue->requests, request, entry);
>>> +            break;
>>> +        }
>>> +
>>> +        qemu_free(request);
>>> +    }
>>> +}
>>> +
>>>  void bdrv_register(BlockDriver *bdrv)
>>>  {
>>>      if (!bdrv->bdrv_aio_readv) {
>>> @@ -642,6 +688,15 @@ int bdrv_open(BlockDriverState *bs, const char 
>>> *filename, int flags,
>>>              bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
>>>      }
>>>
>>> +    /* throttling disk I/O limits */
>>> +    if (bdrv_io_limits_enable(&bs->io_limits)) {
>>> +        bs->block_queue = qemu_new_block_queue();
>>> +        bs->block_timer = qemu_new_timer_ns(vm_clock, bdrv_block_timer, 
>>> bs);
>>> +
>>> +        bs->slice_start[0] = qemu_get_clock_ns(vm_clock);
>>> +        bs->slice_start[1] = qemu_get_clock_ns(vm_clock);
>>> +    }
>>> +
>>
>> It should be possible to tune the limits on the flight, please introduce
>> QMP commands for that.
> Yeah, I am working on this.

It's worth mentioning that the I/O limits commands can use Supriya's
new block_set command for changing block device parameters at runtime.
 So I think the runtime limits changing can be a sepa

Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change

2011-07-27 Thread Anthony Liguori

On 07/27/2011 06:30 AM, Supriya Kannery wrote:

New command "block_set" added for dynamically changing any of the block
device parameters. For now, dynamic setting of hostcache params using this
command is implemented. Other block device parameter changes, can be
integrated in similar lines.

Signed-off-by: Supriya Kannery

---
  block.c |   54 +
  block.h |2 +
  blockdev.c  |   61 

  blockdev.h  |1
  hmp-commands.hx |   14 
  qemu-config.c   |   13 +++
  qemu-option.c   |   25 ++
  qemu-option.h   |2 +
  qmp-commands.hx |   28 +
  9 files changed, 200 insertions(+)

Index: qemu/block.c
===
--- qemu.orig/block.c
+++ qemu/block.c
@@ -651,6 +651,34 @@ unlink_and_fail:
  return ret;
  }

+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
+{
+BlockDriver *drv = bs->drv;
+int ret = 0, open_flags;
+
+/* Quiesce IO for the given block device */
+qemu_aio_flush();
+if (bdrv_flush(bs)) {
+qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
+return ret;
+}
+open_flags = bs->open_flags;
+bdrv_close(bs);
+
+ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
+if (ret<  0) {
+/* Reopen failed. Try to open with original flags */
+qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+ret = bdrv_open(bs, bs->filename, open_flags, drv);
+if (ret<  0) {
+/* Reopen failed with orig and modified flags */
+abort();
+}
+}
+
+return ret;
+}
+
  void bdrv_close(BlockDriverState *bs)
  {
  if (bs->drv) {
@@ -691,6 +719,32 @@ void bdrv_close_all(void)
  }
  }

+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache)
+{
+int bdrv_flags = bs->open_flags;
+
+/* set hostcache flags (without changing WCE/flush bits) */
+if (enable_host_cache) {
+bdrv_flags&= ~BDRV_O_NOCACHE;
+} else {
+bdrv_flags |= BDRV_O_NOCACHE;
+}
+
+/* If no change in flags, no need to reopen */
+if (bdrv_flags == bs->open_flags) {
+return 0;
+}
+
+if (bdrv_is_inserted(bs)) {
+/* Reopen file with changed set of flags */
+return bdrv_reopen(bs, bdrv_flags);
+} else {
+/* Save hostcache change for future use */
+bs->open_flags = bdrv_flags;
+return 0;
+}
+}
+
  /* make a BlockDriverState anonymous by removing from bdrv_state list.
 Also, NULL terminate the device_name to prevent double remove */
  void bdrv_make_anon(BlockDriverState *bs)
Index: qemu/block.h
===
--- qemu.orig/block.h
+++ qemu/block.h
@@ -71,6 +71,7 @@ void bdrv_delete(BlockDriverState *bs);
  int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
  int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
BlockDriver *drv);
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags);
  void bdrv_close(BlockDriverState *bs);
  int bdrv_attach(BlockDriverState *bs, DeviceState *qdev);
  void bdrv_detach(BlockDriverState *bs, DeviceState *qdev);
@@ -97,6 +98,7 @@ void bdrv_commit_all(void);
  int bdrv_change_backing_file(BlockDriverState *bs,
  const char *backing_file, const char *backing_fmt);
  void bdrv_register(BlockDriver *bdrv);
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache);


  typedef struct BdrvCheckResult {
Index: qemu/blockdev.c
===
--- qemu.orig/blockdev.c
+++ qemu/blockdev.c
@@ -793,3 +793,64 @@ int do_block_resize(Monitor *mon, const

  return 0;
  }
+
+
+/*
+ * Handle changes to block device settings, like hostcache,
+ * while guest is running.
+*/
+int do_block_set(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+BlockDriverState *bs = NULL;
+QemuOpts *opts;
+int enable;
+const char *device, *driver;
+int ret;
+char usage[50];
+
+/* Validate device */
+device = qdict_get_str(qdict, "device");
+bs = bdrv_find(device);
+if (!bs) {
+qerror_report(QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict);
+if (opts == NULL) {
+return -1;
+}
+
+/* If input not in "param=value" format, display error */
+driver = qemu_opt_get(opts, "driver");
+if (driver != NULL) {
+qerror_report(QERR_INVALID_PARAMETER, driver);
+return -1;
+}
+
+/* Before validating parameters, remove "device" option */
+ret = qemu_opt_delete(opts, "device");
+if (ret == 1) {
+strcpy(usage,"block_set device [prop=value][,...]");
+qerror_report(QERR_INCORRECT_COMMAND_SYNTAX, usage);
+  

Re: [Qemu-devel] [PATCH v3] Add support for fd: protocol

2011-07-27 Thread Corey Bryant



On 07/27/2011 04:43 AM, Daniel P. Berrange wrote:

On Wed, Jul 27, 2011 at 10:36:25AM +0200, Kevin Wolf wrote:

Am 27.07.2011 10:22, schrieb Daniel P. Berrange:

On Wed, Jul 27, 2011 at 10:11:06AM +0200, Kevin Wolf wrote:

Am 26.07.2011 18:57, schrieb Corey Bryant:

  diff --git a/block/cow.c b/block/cow.c
  index 4cf543c..e17f8e7 100644
  --- a/block/cow.c
  +++ b/block/cow.c
  @@ -82,6 +82,11 @@ static int cow_open(BlockDriverState *bs, int flags)
pstrcpy(bs->backing_file, sizeof(bs->backing_file),
cow_header.backing_file);

  +if (bs->backing_file[0] != '\0'&&   bdrv_is_fd_protocol(bs)) {
  +/* backing file currently not supported by fd: protocol */
  +goto fail;
  +}

I don't think these checks are strictly needed. Without the check you
can open the image itself using an fd, and the backing file using good
old raw-posix. We shouldn't decide for our users that this isn't useful.

In any case, it would have to move into block.c, you can't modify
independent drivers for this.



I understand the point on not modifying independent drivers.

But if the backing file resides on NFS, wouldn't the the proposed
SELinux changes prevent the open?


Probably. But what about cases where the backing file is local? Or even
a non-libvirt, non-SELinux use case?


Or are you talking about support where libvirt opens the backing file
and passes the fd to Qemu?  If so there was some discussion about future
support for this here:
http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg01496.html


Not really, but implementing this will be a bit easier if you don't
forbid using backing files with fd.


In any case, for 'fd:' to be actually used by libvirt, we need to have
backing files supported. There are major users of libvirt who rely on
NFS and also use backing files, so an 'fd:' impl which can't deal with
the backing file problem isn't much use to libvirt.


I'm perfectly aware of that. But seriously, repeating it over and over
again isn't going to make it happen any sooner. It requires -blockdev
which we may or may not have by 1.0.


Yes, I understand we need also -blockdev for this. Other messages in this
mail thread have impied that this proposed patch on its own is useful for
libvirt's requirements, so I just wanted to remind people in general that
we can't use this patch on its own until we have something like -blockdev.

Regards,
Daniel


Kevin/Daniel, thanks a lot for your input.

In terms of the support that libvirt requires, I just want to make sure 
all bases are covered.


In order for this support to be useful to libvirt, the following are 
required (sorry if this is repetitive):


1) -blockdev (backing file support)
2) savevm (snapshot support)
3) snapshot_blkdev (snapshot support)
4) 'change' monitor command
5) -cdrom

and as far as I know, the status of the above is:

1) Someone is slated to work on this (not me)
2) I need to figure out how to "re-open" file without an open (me)
3) This will be covered by live snapshots feature (not me)
4) Should just work as is (me)
5) May also just work as is (me)

In other words, 1 and 3 will not be implemented by me (except perhaps 
some re-usable infrastructure).  Could you confirm my understanding?


Regards,
Corey



Re: [Qemu-devel] [PATCH 04/25] Add hard build dependency on glib

2011-07-27 Thread Yoder Stuart-B08248


> -Original Message-
> From: Anthony Liguori [mailto:aligu...@us.ibm.com]
> Sent: Tuesday, July 26, 2011 5:10 PM
> To: Yoder Stuart-B08248
> Cc: qemu-devel@nongnu.org
> Subject: Re: [PATCH 04/25] Add hard build dependency on glib
> 
> On 07/26/2011 04:51 PM, Yoder Stuart-B08248 wrote:
> >
> > I am having issues with this in a cross compilation
> > environment.   In Power embedded, almost all our
> > development is using cross toolchains.
> >
> > Neither glib or pkg-config are in our cross build environment and I'm
> > having issues getting them built and installed.
> > Not even sure if pkg-config is even supposed to work in a cross
> > development environment...I'm new to that tool and poking around a bit
> > with google raises some questions.
> 
> You're probably setting up your cross environment incorrectly which, 
> unfortunately, is very
> common.

I got glib to build without too much trouble, however, 'make install' tries to
re-link some stuff and at that point there seems to be a bug somewhere where 
libtool
fails to use the correct CFLAGS and PATH, and thus the make install partially
installs glib before erroring out.

> The proper thing to do is to have GCC use a different system include 
> directory and a different
> prefix.  That will result in a directory where there are gcc binaries with 
> normal names
> installed in ${cross_prefix}/bin
> 
> You need to build and install pkg-config to this prefix too, and then when it 
> comes time to
> actually doing the QEMU configure, you should do something like:
> 
> export PATH=${cross_prefix}/bin:$PATH
> export PKG_CONFIG_PATH=${cross_prefix}/lib/pkg-config:$PKG_CONFIG_PATH
> 
> Many automated cross compiler environment scripts will install specially 
> named versions of gcc
> and binutils in your normal $PATH.  The trouble is, this is a bit of a hack 
> and unless you
> know to make this hack work with other build tools, it all comes tumbling 
> down.

Note-- this is not just a matter of me getting this to work in my
own private build environment, I'm working with a cross toolchain
that gets delivered to our customers that I have little control
over, and I need to get it working in that environment.

Looks like our cross tools have both a specially named version of the tool
(e.g. powerpc-linux-gnu-gcc) and plain (e.g. gcc).  Unfortunately the plain
version of the tools don't seem to be functional (and have never been used as
far as I know).

Will keep fiddling with this...

Stuart




Re: [Qemu-devel] qemu crashes on Mac OS X

2011-07-27 Thread Damjan Marion

Hi Alexandre,

I tried your patch and it works OK.

Then I tried without it and seems that it also works ok. It might be 2 reasons:
 - I upgraded to Mac OS X 10.7 Lion
 - Something changed in qemu master branch

I dont remember what was exact version I used when I reported this problem.

Thanks,

Damjan

On Jul 26, 2011, at 6:58 AM, Alexandre Raymond wrote:

> Hi Damjan,
> 
> I've been able to reproduce the crash you're describing.
> 
> Could you try the following patch, to see if it solves it?
> 
> Alexandre
> 
> On Tue, Jul 5, 2011 at 3:03 PM, Alexandre Raymond  wrote:
>> Hi again Damjan,
>> 
>> On Mon, Jul 4, 2011 at 6:35 PM, Damjan Marion  
>> wrote:
>>> 
>>> On Jul 4, 2011, at 6:59 PM, Alexandre Raymond wrote:
>>> 
 Hi Damjan,
 
 
 Can you try applying the following two patches and see if it solves
 your problem?
 
 http://patchwork.ozlabs.org/patch/100348/
 http://patchwork.ozlabs.org/patch/100477/
 
>>> 
>>> Unfortunately same thing happens: segmentation fault.
>> You might also want to have a look at the following patches by Paolo:
>> http://www.mail-archive.com/qemu-devel@nongnu.org/msg67088.html
>> 
>> Finally, you might want to disable io-thead (if you've enabled it),
>> which doesn't work properly on OS X.
>> 
>> Alexandre
>> 
> <0001-Darwin-catch-invalid-return-of-sigwait.patch>




[Qemu-devel] [PULL 0/7] ARM patch queue (for master)

2011-07-27 Thread Peter Maydell
This is a pull request for various outstanding ARM related
patches; they've been on the list for a week or so.

Some of these are bug fixes which I want to get into 0.15; I'm
going to do a parallel pullreq for the 0.15 patches.

Thanks
-- PMM


The following changes since commit c886edfb851c0c590d4e77f058f2ec8ed95ad1b5:

  Let users select their pythons (2011-07-25 16:50:12 +)

are available in the git repository at:
  git://git.linaro.org/people/pmaydell/qemu-arm.git for-upstream

Jamie Iles (2):
  target-arm: make VMSAv7 remapping and AP dependent on V6K
  target-arm: support for ARM1176JZF-s cores

Peter Maydell (5):
  target-arm: Mark 1136r1 as a v6K core
  target-arm: Support v6 barriers in linux-user mode
  target-arm: Handle UNDEF and UNPREDICTABLE cases for VLDM, VSTM
  target-arm: UNDEF on a VCVTT/VCVTB UNPREDICTABLE to avoid TCG assert
  target-arm: Don't print debug messages for various UNDEF cases

 target-arm/cpu.h   |2 +
 target-arm/helper.c|   47 ++-
 target-arm/translate.c |  114 +++
 3 files changed, 121 insertions(+), 42 deletions(-)



[Qemu-devel] [PATCH 7/7] target-arm: Don't print debug messages for various UNDEF cases

2011-07-27 Thread Peter Maydell
Remove some stray printfs for cases which don't generally happen
(some VFP UNDEF cases, reads and writes to unknown cp14 registers);
we should simply generate an UNDEF when the instruction is executed.

Signed-off-by: Peter Maydell 
---
 target-arm/translate.c |6 --
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index fcb41d1..75c0ad4 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -3288,12 +3288,10 @@ static int disas_vfp_insn(CPUState * env, DisasContext 
*s, uint32_t insn)
 gen_vfp_toul(dp, 32 - rm, 0);
 break;
 default: /* undefined */
-printf ("rn:%d\n", rn);
 return 1;
 }
 break;
 default: /* undefined */
-printf ("op:%d\n", op);
 return 1;
 }
 
@@ -6372,8 +6370,6 @@ static int disas_cp14_read(CPUState * env, DisasContext 
*s, uint32_t insn)
 return 0;
 }
 }
-fprintf(stderr, "Unknown cp14 read op1:%d crn:%d crm:%d op2:%d\n",
-op1, crn, crm, op2);
 return 1;
 }
 
@@ -6405,8 +6401,6 @@ static int disas_cp14_write(CPUState * env, DisasContext 
*s, uint32_t insn)
 return 0;
 }
 }
-fprintf(stderr, "Unknown cp14 write op1:%d crn:%d crm:%d op2:%d\n",
-op1, crn, crm, op2);
 return 1;
 }
 
-- 
1.7.1




Re: [Qemu-devel] [PATCH RESEND v3] xen: implement unplug protocol in xen_platform

2011-07-27 Thread Alexander Graf

On 07/27/2011 02:58 PM, Kevin Wolf wrote:

Am 27.07.2011 14:44, schrieb Alexander Graf:

On 07/18/2011 06:07 PM, stefano.stabell...@eu.citrix.com wrote:

From: Stefano Stabellini

The unplug protocol is necessary to support PV drivers in the guest: the
drivers expect to be able to "unplug" emulated disks and nics before
initializing the Xen PV interfaces.
It is responsibility of the guest to make sure that the unplug is done
before the emulated devices or the PV interface start to be used.

We use pci_for_each_device to walk the PCI bus, identify the devices and
disks that we want to disable and dynamically unplug them.

Changes in v2:

- use PCI_CLASS constants;

- replace pci_unplug_device with qdev_unplug;

- do not import hw/ide/internal.h in xen_platform.c;


Changes in v3:

- introduce piix3-ide-xen, that support hot-unplug;

- move the unplug code to hw/ide/piix.c;

- just call qdev_unplug from xen_platform.c to unplug the IDE disks;

Signed-off-by: Stefano Stabellini

Kevin, please ack.

Trivial rebase of the version I already acked.


Thanks :). Applied to the xen-next branch.


Alex




[Qemu-devel] [PATCH 5/9] xen: make xen_enabled even more clever

2011-07-27 Thread Alexander Graf
When using xen_enabled() we're currently only checking if xen is enabled
at all during the build. But what if you want to build multiple targets
out of which only one can potentially run xen code?

That means that for generic code we'll still have to fall back to the
variable and potentially slow the code down, but it's not as important as
that is mostly xen device emulation which is not touched for non-xen targets.

The target specific code however can with this patch see that it's unable to
ever execute xen code. We can thus always return 0 on xen_enabled(), giving
gcc enough hints to evict the mapcache code from the target memory management
code.

Signed-off-by: Alexander Graf 
Acked-by: Anthony PERARD 
---
 configure |5 +
 hw/xen.h  |2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index cd399dc..bc3495c 100755
--- a/configure
+++ b/configure
@@ -3290,7 +3290,12 @@ case "$target_arch2" in
 if test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then
   target_phys_bits=64
   echo "CONFIG_XEN=y" >> $config_target_mak
+else
+  echo "CONFIG_NO_XEN=y" >> $config_target_mak
 fi
+;;
+  *)
+echo "CONFIG_NO_XEN=y" >> $config_target_mak
 esac
 case "$target_arch2" in
   i386|x86_64|ppcemb|ppc|ppc64|s390x)
diff --git a/hw/xen.h b/hw/xen.h
index 43b95d6..2162111 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -24,7 +24,7 @@ extern int xen_allowed;
 
 static inline int xen_enabled(void)
 {
-#ifdef CONFIG_XEN_BACKEND
+#if defined(CONFIG_XEN_BACKEND) && !defined(CONFIG_NO_XEN)
 return xen_allowed;
 #else
 return 0;
-- 
1.6.0.2




[Qemu-devel] [PATCH 6/9] cpu-common: Have a ram_addr_t of uint64 with Xen.

2011-07-27 Thread Alexander Graf
From: Anthony PERARD 

In Xen case, memory can be bigger than the host memory. that mean a
32bits host (and QEMU) should be able to handle a RAM address of 64bits.

Signed-off-by: Anthony PERARD 
Signed-off-by: Alexander Graf 
---
 cpu-common.h |8 
 exec.c   |9 +
 xen-all.c|2 +-
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/cpu-common.h b/cpu-common.h
index 44b04b3..0700101 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -27,7 +27,15 @@ enum device_endian {
 };
 
 /* address in the RAM (different from a physical address) */
+#if defined(CONFIG_XEN_BACKEND) && TARGET_PHYS_ADDR_BITS == 64
+typedef uint64_t ram_addr_t;
+#  define RAM_ADDR_MAX UINT64_MAX
+#  define RAM_ADDR_FMT "%" PRIx64
+#else
 typedef unsigned long ram_addr_t;
+#  define RAM_ADDR_MAX ULONG_MAX
+#  define RAM_ADDR_FMT "%lx"
+#endif
 
 /* memory API */
 
diff --git a/exec.c b/exec.c
index 0393d39..bfc9a43 100644
--- a/exec.c
+++ b/exec.c
@@ -2863,13 +2863,13 @@ static void *file_ram_alloc(RAMBlock *block,
 static ram_addr_t find_ram_offset(ram_addr_t size)
 {
 RAMBlock *block, *next_block;
-ram_addr_t offset = 0, mingap = ULONG_MAX;
+ram_addr_t offset = 0, mingap = RAM_ADDR_MAX;
 
 if (QLIST_EMPTY(&ram_list.blocks))
 return 0;
 
 QLIST_FOREACH(block, &ram_list.blocks, next) {
-ram_addr_t end, next = ULONG_MAX;
+ram_addr_t end, next = RAM_ADDR_MAX;
 
 end = block->offset + block->length;
 
@@ -3081,7 +3081,8 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
 #endif
 }
 if (area != vaddr) {
-fprintf(stderr, "Could not remap addr: %lx@%lx\n",
+fprintf(stderr, "Could not remap addr: "
+RAM_ADDR_FMT "@" RAM_ADDR_FMT "\n",
 length, addr);
 exit(1);
 }
@@ -4052,7 +4053,7 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
 target_phys_addr_t page;
 unsigned long pd;
 PhysPageDesc *p;
-ram_addr_t raddr = ULONG_MAX;
+ram_addr_t raddr = RAM_ADDR_MAX;
 ram_addr_t rlen;
 void *ret;
 
diff --git a/xen-all.c b/xen-all.c
index 83c5476..53296bf 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -184,7 +184,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size)
 }
 
 if (xc_domain_populate_physmap_exact(xen_xc, xen_domid, nr_pfn, 0, 0, 
pfn_list)) {
-hw_error("xen: failed to populate ram at %lx", ram_addr);
+hw_error("xen: failed to populate ram at " RAM_ADDR_FMT, ram_addr);
 }
 
 qemu_free(pfn_list);
-- 
1.6.0.2




[Qemu-devel] [PATCH 1/9] xen: introduce xen_change_state_handler

2011-07-27 Thread Alexander Graf
From: Anthony PERARD 

Remove the call to xenstore_record_dm_state from xen_main_loop_prepare
that is HVM specific.
Add a new vm_change_state_handler shared between xen_pv and xen_hvm
machines to record the VM state to xenstore.

Signed-off-by: Anthony PERARD 
Signed-off-by: Stefano Stabellini 
Signed-off-by: Alexander Graf 
---
 xen-all.c |   25 ++---
 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/xen-all.c b/xen-all.c
index 167bed6..83c5476 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -797,12 +797,17 @@ void xenstore_store_pv_console_info(int i, 
CharDriverState *chr)
 }
 }
 
-static void xenstore_record_dm_state(XenIOState *s, const char *state)
+static void xenstore_record_dm_state(struct xs_handle *xs, const char *state)
 {
 char path[50];
 
+if (xs == NULL) {
+fprintf(stderr, "xenstore connection not initialized\n");
+exit(1);
+}
+
 snprintf(path, sizeof (path), "/local/domain/0/device-model/%u/state", 
xen_domid);
-if (!xs_write(s->xenstore, XBT_NULL, path, state, strlen(state))) {
+if (!xs_write(xs, XBT_NULL, path, state, strlen(state))) {
 fprintf(stderr, "error recording dm state\n");
 exit(1);
 }
@@ -823,15 +828,20 @@ static void xen_main_loop_prepare(XenIOState *state)
 if (evtchn_fd != -1) {
 qemu_set_fd_handler(evtchn_fd, cpu_handle_ioreq, NULL, state);
 }
-
-/* record state running */
-xenstore_record_dm_state(state, "running");
 }
 
 
 /* Initialise Xen */
 
-static void xen_vm_change_state_handler(void *opaque, int running, int reason)
+static void xen_change_state_handler(void *opaque, int running, int reason)
+{
+if (running) {
+/* record state running */
+xenstore_record_dm_state(xenstore, "running");
+}
+}
+
+static void xen_hvm_change_state_handler(void *opaque, int running, int reason)
 {
 XenIOState *state = opaque;
 if (running) {
@@ -854,6 +864,7 @@ int xen_init(void)
 xen_be_printf(NULL, 0, "can't open xen interface\n");
 return -1;
 }
+qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
 
 return 0;
 }
@@ -915,7 +926,7 @@ int xen_hvm_init(void)
 xen_map_cache_init();
 xen_ram_init(ram_size);
 
-qemu_add_vm_change_state_handler(xen_vm_change_state_handler, state);
+qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state);
 
 state->client = xen_cpu_phys_memory_client;
 QLIST_INIT(&state->physmap);
-- 
1.6.0.2




[Qemu-devel] [PATCH 8/9] vl.c: Check the asked ram_size later.

2011-07-27 Thread Alexander Graf
From: Anthony PERARD 

As a Xen guest can have more than 2GB of RAM on a 32bit host, we move
the conditions after than we now if we run one Xen or not.

[agraf] separate xen branch from ram_size check

Signed-off-by: Anthony PERARD 
Signed-off-by: Alexander Graf 
---
 vl.c |   16 ++--
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/vl.c b/vl.c
index 4b6688b..d8c7c01 100644
--- a/vl.c
+++ b/vl.c
@@ -2440,11 +2440,6 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
 
-/* On 32-bit hosts, QEMU is limited by virtual address space */
-if (value > (2047 << 20) && HOST_LONG_BITS == 32) {
-fprintf(stderr, "qemu: at most 2047 MB RAM can be 
simulated\n");
-exit(1);
-}
 if (value != (uint64_t)(ram_addr_t)value) {
 fprintf(stderr, "qemu: ram size too large\n");
 exit(1);
@@ -3099,8 +3094,17 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 
 /* init the memory */
-if (ram_size == 0)
+if (ram_size == 0) {
 ram_size = DEFAULT_RAM_SIZE * 1024 * 1024;
+}
+
+if (!xen_enabled()) {
+/* On 32-bit hosts, QEMU is limited by virtual address space */
+if (ram_size > (2047 << 20) && HOST_LONG_BITS == 32) {
+fprintf(stderr, "qemu: at most 2047 MB RAM can be simulated\n");
+exit(1);
+}
+}
 
 /* init the dynamic translator */
 cpu_exec_init_all(tb_size * 1024 * 1024);
-- 
1.6.0.2




[Qemu-devel] [PATCH 2/9] xen: Fix xen_enabled().

2011-07-27 Thread Alexander Graf
From: Anthony PERARD 

Use the "host" CONFIG_ define instead of the "target" one.

Signed-off-by: Anthony PERARD 
Acked-by: Paolo Bonzini 
Signed-off-by: Alexander Graf 
---
 hw/xen.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/xen.h b/hw/xen.h
index e432705..43b95d6 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -24,7 +24,7 @@ extern int xen_allowed;
 
 static inline int xen_enabled(void)
 {
-#ifdef CONFIG_XEN
+#ifdef CONFIG_XEN_BACKEND
 return xen_allowed;
 #else
 return 0;
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH 04/25] Add hard build dependency on glib

2011-07-27 Thread David Gibson
On Wed, Jul 27, 2011 at 06:54:09PM +1000, Benjamin Herrenschmidt wrote:
> 
> > You're probably setting up your cross environment incorrectly which, 
> > unfortunately, is very common.
> > 
> > The proper thing to do is to have GCC use a different system include 
> > directory and a different prefix.  That will result in a directory where 
> > there are gcc binaries with normal names installed in ${cross_prefix}/bin
> > 
> > You need to build and install pkg-config to this prefix too, and then 
> > when it comes time to actually doing the QEMU configure, you should do 
> > something like:
> > 
> > export PATH=${cross_prefix}/bin:$PATH
> > export PKG_CONFIG_PATH=${cross_prefix}/lib/pkg-config:$PKG_CONFIG_PATH
> > 
> > Many automated cross compiler environment scripts will install specially 
> > named versions of gcc and binutils in your normal $PATH.  The trouble 
> > is, this is a bit of a hack and unless you know to make this hack work 
> > with other build tools, it all comes tumbling down.

We're not, as a rule, cross building.  We're doing compiles of ppc64
binaries on a ppc32.  Although that can be approached as a
cross-build, it's a common enough special case that it should be able
to handle this without setting a full cross-build environment.  At the
moment this does seem to work for building x86_64 binaries on a 32-bit
x86 system, but I suspect this is only accident.

> Well, that hard requirement is causing us problem on our 32/64-bit cross
> builds as well.
> 
> It looks like glib (at least recent versions in -sid) can't be built
> 64-bit on a 32-bit system :-( At least not without fixing some horrid
> bugs in there related to some generated include path from what David
> says (I'll let him comment further).

Actually, I think it can (provided a 64-bit glib is installed,
including a 64-bit version of glibconfig.h), and it's not *as* painful
to set up as I previously thought, although it's still not nice.

> In general, every time you add a library requirement without some config
> option to disable it for cases such as ours, you add pain :-)
> 
> Now, in the specific case of glib, I understand why you would want to
> get rid of re-invented wheels and use it, so I'm not specifically
> criticizing that specific change, we'll eventually have to fix it
> anyways. Just a heads up to be careful with hard requirements in
> general.
> 
> Cheers,
> Ben.
> 
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson



Re: [Qemu-devel] [PATCH 21/28] PPC: E500: Add PV spinning code

2011-07-27 Thread Alexander Graf

On 07/25/2011 10:40 PM, Scott Wood wrote:

On Sat, 23 Jul 2011 12:50:05 +0200
Alexander Graf  wrote:


+typedef struct spin_info {
+uint64_t addr;
+uint64_t r3;
+uint32_t resv;
+uint32_t pir;
+uint64_t r6;
+} __attribute__ ((packed)) SpinInfo;

Note that r6 isn't part of the ePAPR spin table -- I think it may have been
in an early draft and gotten into U-Boot that way.  A future ePAPR could
assign another use to that position in the spin table.


How is the size defined then?


Do we support any host ABIs strange enough that __attribute__((packed))
would be needed here?


I don't think we do, but in general I prefer to be safe rather than 
sorry. It doesn't hurt, right?



+static void mmubooke_create_initial_mapping(CPUState *env,
+ target_ulong va,
+ target_phys_addr_t pa,
+ target_phys_addr_t len)
+{
+ppcmas_tlb_t *tlb = booke206_get_tlbm(env, 1, 0, 0);
+target_phys_addr_t size;
+
+size = (booke206_page_size_to_tlb(len)<<  MAS1_TSIZE_SHIFT);
+tlb->mas1 = MAS1_VALID | size;
+tlb->mas2 = va&  TARGET_PAGE_MASK;
+tlb->mas7_3 = pa&  TARGET_PAGE_MASK;
+tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX;
+}

[snip]

+mmubooke_create_initial_mapping(env, env->nip, env->nip, map_size);

ePAPR says:

   The Secondary IMA (SIMA) mapping in the MMU shall map effective address 0
   to the entry_addr field in the spin table, aligned down to the SIMA size.


So it jumps to nip & ~64MB?


Note that it's possible for the physical entry point to be>  4GiB on a
32-bit target.

Also, MAS2[M] should be set, even if it doesn't affect anything real under
qemu/kvm.


Ok :)


I know that U-Boot has the same behavior on both counts.  U-Boot is wrong.


If you say so, I'll align it with ePAPR then.


Alex




Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change

2011-07-27 Thread Michael Tokarev
27.07.2011 15:30, Supriya Kannery wrote:
> New command "block_set" added for dynamically changing any of the block
> device parameters. For now, dynamic setting of hostcache params using this
> command is implemented. Other block device parameter changes, can be 
> integrated in similar lines.
> 
> Signed-off-by: Supriya Kannery 
> 
> ---
>  block.c |   54 +
>  block.h |2 +
>  blockdev.c  |   61 
> 
>  blockdev.h  |1 
>  hmp-commands.hx |   14 
>  qemu-config.c   |   13 +++
>  qemu-option.c   |   25 ++
>  qemu-option.h   |2 +
>  qmp-commands.hx |   28 +
>  9 files changed, 200 insertions(+)
> 
> Index: qemu/block.c
> ===
> --- qemu.orig/block.c
> +++ qemu/block.c
> @@ -651,6 +651,34 @@ unlink_and_fail:
>  return ret;
>  }
>  
> +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
> +{
> +BlockDriver *drv = bs->drv;
> +int ret = 0, open_flags;
> +
> +/* Quiesce IO for the given block device */
> +qemu_aio_flush();
> +if (bdrv_flush(bs)) {
> +qerror_report(QERR_DATA_SYNC_FAILED, bs->device_name);
> +return ret;
> +}
> +open_flags = bs->open_flags;
> +bdrv_close(bs);
> +
> +ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
> +if (ret < 0) {
> +/* Reopen failed. Try to open with original flags */
> +qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
> +ret = bdrv_open(bs, bs->filename, open_flags, drv);
> +if (ret < 0) {
> +/* Reopen failed with orig and modified flags */
> +abort();
> +}

Can we please avoid this stuff completely?  Just keep the
old device open still, until you're sure new one is ok.

Or else it will be quite dangerous command in many cases.
For example, after -runas/-chroot, or additional selinux
settings or whatnot.  And in this case, instead of merely
returning an error, we'll see abort().  Boom.

Thanks,

/mjt



  1   2   3   >