[Qemu-devel] [Bug 595438] Re: KVM segmentation fault, using SCSI+writeback and linux 2.4 guest

2010-07-01 Thread Jes Sorensen
Could you try and run this in GDB and get the backtrace when it crashes?
Just do:

gdb /usr/bin/kvm
(gdb) set args -M pc-0.12 -enable-kvm -m 256 -smp 1 -name spamsender -uuid 
b9cacd5e-08f7-41fd-78c8-89cec59af881 -chardev 
socket,id=monitor,path=/var/lib/libvirt/qemu/spamsender.monitor,server,nowait 
-monitor chardev:monitor -boot d -drive 
file=/mnt/megadiff/cdiso_400_130.iso,if=ide,media=cdrom,index=2 -drive 
file=/home/mmarkk/spamsender2.img,if=scsi,index=0,format=qcow2,cache=writeback 
-net nic,macaddr=00:00:00:00:00:00,vlan=0,name=nic.0 -net tap,vlan=0,name=tap.0 
-chardev pty,id=serial0 -serial chardev:serial0 -parallel none -usb -vnc 
127.0.0.1:0 -vga cirrus
(gdb) run
after crash
(gdb) bt

Make sure to have all the debug packages installed, otherwise the
backtrace isn't really useful.

Unfortunately the core file you posted isn't of use to anyone who
doesn't have Ubuntu installed with the exact packages you have
installed. It just shows some random addresses in gdb.

Thanks,
Jes

-- 
KVM segmentation fault, using SCSI+writeback and linux 2.4 guest
https://bugs.launchpad.net/bugs/595438
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
I Use Ubuntu 32 bit 10.04 with standard KVM.
I have Intel E7600  @ 3.06GHz processor with VMX

In this system I Run:
LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/sbin:/sbin:/bin 
QEMU_AUDIO_DRV=none /usr/bin/kvm -M pc-0.12 -enable-kvm -m 256 -smp 1 -name 
spamsender -uuid b9cacd5e-08f7-41fd-78c8-89cec59af881 -chardev 
socket,id=monitor,path=/var/lib/libvirt/qemu/spamsender.monitor,server,nowait 
-monitor chardev:monitor -boot d -drive 
file=/mnt/megadiff/cdiso_400_130.iso,if=ide,media=cdrom,index=2 -drive 
file=/home/mmarkk/spamsender2.img,if=scsi,index=0,format=qcow2,cache=writeback 
-net nic,macaddr=00:00:00:00:00:00,vlan=0,name=nic.0 -net tap,vlan=0,name=tap.0 
-chardev pty,id=serial0 -serial chardev:serial0 -parallel none -usb -vnc 
127.0.0.1:0 -vga cirrus

.iso image contain custom distro of 2.4-linux kernel based system. During 
install process (when .tar.gz actively unpacked), kvm dead with segmentation 
fault.

And ONLY when I choose scsi virtual disk and writeback simultaneously. 
But, writeback+ide, writethrough+scsi works OK.

I use qcow2. It seems, that qcow does not have such problems.

Virtual machine get down at random time during file copy. It seems, when qcow2 
file size need to be expanded.







Re: [Qemu-devel] [PATCH 2/3] blkdebug: Free QemuOpts after having read the config

2010-07-01 Thread Markus Armbruster
Kevin Wolf kw...@redhat.com writes:

 Forgetting to free them means that the next instance inherits all rules and
 gets its own rules only additionally.

I also found a use for freeing a complete QemuOptsList, here's my
solution.  The code that needs it isn't ready, yet.  If you'd like to
use it, I can push it to my repo.


diff --git a/qemu-option.c b/qemu-option.c
index 7f70d0f..30327d4 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -719,6 +719,15 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char 
*id, int fail_if_exist
 return opts;
 }
 
+void qemu_opts_reset(QemuOptsList *list)
+{
+QemuOpts *opts, *next_opts;
+
+QTAILQ_FOREACH_SAFE(opts, list-head, next, next_opts) {
+qemu_opts_del(opts);
+}
+}
+
 int qemu_opts_set(QemuOptsList *list, const char *id,
   const char *name, const char *value)
 {
diff --git a/qemu-option.h b/qemu-option.h
index 4823219..9e2406c 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -115,6 +115,7 @@ int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc 
func, void *opaque,
 
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int 
fail_if_exists);
+void qemu_opts_reset(QemuOptsList *list);
 int qemu_opts_set(QemuOptsList *list, const char *id,
   const char *name, const char *value);
 const char *qemu_opts_id(QemuOpts *opts);



Re: [Qemu-devel] [PATCH 0/3] blkdebug: Fix config with multiple states

2010-07-01 Thread Markus Armbruster
Kevin Wolf kw...@redhat.com writes:

 Turns out that using more than one state doesn't really work well. I'm trying
 to reproduce a bug for which I need states, so now is the time to fix it.

I'm not familiar with blkdebug, but these look like obvious bug fixes.



[Qemu-devel] Re: [Bug 494500] Re: QEMU 0.12.0 does not support KVM with Kernel 2.6.29, bug in ./configure and kvm-all.c

2010-07-01 Thread Jan Kiszka
rowa wrote:
 Does the _lastest_ kvm-kmod's install the right headers for the used
 kernel, for example kernel 2.6.28-11?

It installs the headers required to build kvm support into the qemu that
allows full-featured kernel or kvm-kmod usage up to the version kvm-kmod
carries.

 
 As I wrote in this bug I had problems with a  kernel 2.6.28-11 and the
 _latest_ version of kvm-kmod. It works only with the right (not the
 latest) version of kvm-kmod.

Once you properly installed the kernel headers that kvm-kmod delivers,
qemu should detect their presence during configure and succeed with this
step. Of course, you also have to install the kvm-kmod modules which
will disable those that come with 2.6.28 to actually run qemu in kvm
mode later on.

Jan



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] Re: [Bug 599958] Re: Timedrift problems with Win7: hpet missing time drift fixups

2010-07-01 Thread Jan Kiszka
Anthony Liguori wrote:
 -no-hpet works in every version of qemu/qemu-kvm that has included HPET
 support.  RHEL disables HPET support by default unlike qemu and qemu-
 kvm.
 
 I've updated the bug priority and title to reflect what the issue is.
 
 We only support edge triggered interrupts with HPET which seems to be
 what most OSes use anyway.

qemu.git now also supports level triggering.

  We could potentially use the
 reset_irq_delivered/get_irq_delivered APIC functions to implement
 interrupt catch-up but I think it would be better to try to merge Jan's
 generic IRQ delivered API first.

Which one? I'm a bit tired of rebasing versions that will only be
rejected again. :)

 
 ** Summary changed:
 
 - Timedrift problems with Win7 + qemu-kvm
 + Timedrift problems with Win7: hpet missing time drift fixups
 
 ** Changed in: qemu
Importance: Undecided = Wishlist
 
 ** Changed in: qemu
Status: New = Confirmed
 

Jan



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v3 03/13] blockdev: Remove drive_get_serial()

2010-07-01 Thread Markus Armbruster
Unused since commit 6ced55a5.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 blockdev.c |   12 
 blockdev.h |1 -
 2 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3b8c606..e0495e5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -78,18 +78,6 @@ int drive_get_max_bus(BlockInterfaceType type)
 return max_bus;
 }
 
-const char *drive_get_serial(BlockDriverState *bdrv)
-{
-DriveInfo *dinfo;
-
-QTAILQ_FOREACH(dinfo, drives, next) {
-if (dinfo-bdrv == bdrv)
-return dinfo-serial;
-}
-
-return \0;
-}
-
 static void bdrv_format_print(void *opaque, const char *name)
 {
 fprintf(stderr,  %s, name);
diff --git a/blockdev.h b/blockdev.h
index 23ea576..a936785 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -40,7 +40,6 @@ extern DriveInfo *drive_get(BlockInterfaceType type, int bus, 
int unit);
 extern DriveInfo *drive_get_by_id(const char *id);
 extern int drive_get_max_bus(BlockInterfaceType type);
 extern void drive_uninit(DriveInfo *dinfo);
-extern const char *drive_get_serial(BlockDriverState *bdrv);
 
 extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
 extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
-- 
1.6.6.1




[Qemu-devel] [PATCH v3 01/13] scsi: scsi_bus_legacy_handle_cmdline() can fail, fix callers

2010-07-01 Thread Markus Armbruster
None of its callers checks for failure.  scsi_hot_add() can crash
because of that:

(qemu) drive_add 4 if=scsi,format=host_device,file=/dev/sg1
scsi-generic: scsi generic interface too old
Segmentation fault (core dumped)

Fix all callers, not just scsi_hot_add().

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/esp.c |3 +--
 hw/lsi53c895a.c  |2 +-
 hw/pci-hotplug.c |3 +++
 hw/scsi-bus.c|   11 +++
 hw/scsi.h|2 +-
 hw/usb-msd.c |3 +++
 6 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 7740879..349052a 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -679,8 +679,7 @@ static int esp_init1(SysBusDevice *dev)
 qdev_init_gpio_in(dev-qdev, parent_esp_reset, 1);
 
 scsi_bus_new(s-bus, dev-qdev, 0, ESP_MAX_DEVS, esp_command_complete);
-scsi_bus_legacy_handle_cmdline(s-bus);
-return 0;
+return scsi_bus_legacy_handle_cmdline(s-bus);
 }
 
 static SysBusDeviceInfo esp_info = {
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index f5a91ba..c2a8010 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -2176,7 +2176,7 @@ static int lsi_scsi_init(PCIDevice *dev)
 
 scsi_bus_new(s-bus, dev-qdev, 1, LSI_MAX_DEVS, lsi_command_complete);
 if (!dev-qdev.hotplugged) {
-scsi_bus_legacy_handle_cmdline(s-bus);
+return scsi_bus_legacy_handle_cmdline(s-bus);
 }
 return 0;
 }
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index c39e640..55c9fe3 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -90,6 +90,9 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
  */
 dinfo-unit = qemu_opt_get_number(dinfo-opts, unit, -1);
 scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo, dinfo-unit);
+if (!scsidev) {
+return -1;
+}
 dinfo-unit = scsidev-id;
 
 if (printinfo)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 24bd060..d5b66c1 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -83,7 +83,6 @@ void scsi_qdev_register(SCSIDeviceInfo *info)
 }
 
 /* handle legacy '-drive if=scsi,...' cmd line args */
-/* FIXME callers should check for failure, but don't */
 SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, DriveInfo *dinfo, int unit)
 {
 const char *driver;
@@ -98,18 +97,22 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, 
DriveInfo *dinfo, int unit)
 return DO_UPCAST(SCSIDevice, qdev, dev);
 }
 
-void scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
+int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
 {
 DriveInfo *dinfo;
-int unit;
+int res = 0, unit;
 
 for (unit = 0; unit  MAX_SCSI_DEVS; unit++) {
 dinfo = drive_get(IF_SCSI, bus-busnr, unit);
 if (dinfo == NULL) {
 continue;
 }
-scsi_bus_legacy_add_drive(bus, dinfo, unit);
+if (!scsi_bus_legacy_add_drive(bus, dinfo, unit)) {
+res = -1;
+break;
+}
 }
+return res;
 }
 
 void scsi_dev_clear_sense(SCSIDevice *dev)
diff --git a/hw/scsi.h b/hw/scsi.h
index b668e27..b1b5f73 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -98,7 +98,7 @@ static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
 }
 
 SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, DriveInfo *dinfo, int 
unit);
-void scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
+int scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
 
 void scsi_dev_clear_sense(SCSIDevice *dev);
 void scsi_dev_set_sense(SCSIDevice *dev, uint8_t key);
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 003bd8a..8e9718c 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -531,6 +531,9 @@ static int usb_msd_initfn(USBDevice *dev)
 s-dev.speed = USB_SPEED_FULL;
 scsi_bus_new(s-bus, s-dev.qdev, 0, 1, usb_msd_command_complete);
 s-scsi_dev = scsi_bus_legacy_add_drive(s-bus, s-conf.dinfo, 0);
+if (!s-scsi_dev) {
+return -1;
+}
 s-bus.qbus.allow_hotplug = 0;
 usb_msd_handle_reset(dev);
 
-- 
1.6.6.1




[Qemu-devel] [PATCH v3 08/13] block: Catch attempt to attach multiple devices to a blockdev

2010-07-01 Thread Markus Armbruster
For instance, -device scsi-disk,drive=foo -device scsi-disk,drive=foo
happily creates two SCSI disks connected to the same block device.
It's all downhill from there.

Device usb-storage deliberately attaches twice to the same blockdev,
which fails with the fix in place.  Detach before the second attach
there.

Also catch attempt to delete while a guest device model is attached.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 block.c  |   22 ++
 block.h  |3 +++
 block_int.h  |2 ++
 hw/fdc.c |   10 +-
 hw/ide/qdev.c|2 +-
 hw/pci-hotplug.c |6 +-
 hw/qdev-properties.c |   22 +-
 hw/qdev.h|3 ++-
 hw/s390-virtio.c |2 +-
 hw/scsi-bus.c|5 -
 hw/usb-msd.c |   12 
 11 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index e71a771..5e0ffa0 100644
--- a/block.c
+++ b/block.c
@@ -659,6 +659,8 @@ void bdrv_close_all(void)
 
 void bdrv_delete(BlockDriverState *bs)
 {
+assert(!bs-peer);
+
 /* remove from list, if necessary */
 if (bs-device_name[0] != '\0') {
 QTAILQ_REMOVE(bdrv_states, bs, list);
@@ -672,6 +674,26 @@ void bdrv_delete(BlockDriverState *bs)
 qemu_free(bs);
 }
 
+int bdrv_attach(BlockDriverState *bs, DeviceState *qdev)
+{
+if (bs-peer) {
+return -EBUSY;
+}
+bs-peer = qdev;
+return 0;
+}
+
+void bdrv_detach(BlockDriverState *bs, DeviceState *qdev)
+{
+assert(bs-peer == qdev);
+bs-peer = NULL;
+}
+
+DeviceState *bdrv_get_attached(BlockDriverState *bs)
+{
+return bs-peer;
+}
+
 /*
  * Run consistency checks on an image
  *
diff --git a/block.h b/block.h
index 6a157f4..88ac06e 100644
--- a/block.h
+++ b/block.h
@@ -71,6 +71,9 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename, int flags);
 int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
   BlockDriver *drv);
 void bdrv_close(BlockDriverState *bs);
+int bdrv_attach(BlockDriverState *bs, DeviceState *qdev);
+void bdrv_detach(BlockDriverState *bs, DeviceState *qdev);
+DeviceState *bdrv_get_attached(BlockDriverState *bs);
 int bdrv_check(BlockDriverState *bs);
 int bdrv_read(BlockDriverState *bs, int64_t sector_num,
   uint8_t *buf, int nb_sectors);
diff --git a/block_int.h b/block_int.h
index e60aed4..a94b801 100644
--- a/block_int.h
+++ b/block_int.h
@@ -148,6 +148,8 @@ struct BlockDriverState {
 BlockDriver *drv; /* NULL means no media */
 void *opaque;
 
+DeviceState *peer;
+
 char filename[1024];
 char backing_file[1024]; /* if non zero, the image is a diff of
 this file image */
diff --git a/hw/fdc.c b/hw/fdc.c
index 08712bc..1496cfa 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1860,10 +1860,10 @@ FDCtrl *fdctrl_init_isa(DriveInfo **fds)
 
 dev = isa_create(isa-fdc);
 if (fds[0]) {
-qdev_prop_set_drive(dev-qdev, driveA, fds[0]-bdrv);
+qdev_prop_set_drive_nofail(dev-qdev, driveA, fds[0]-bdrv);
 }
 if (fds[1]) {
-qdev_prop_set_drive(dev-qdev, driveB, fds[1]-bdrv);
+qdev_prop_set_drive_nofail(dev-qdev, driveB, fds[1]-bdrv);
 }
 if (qdev_init(dev-qdev)  0)
 return NULL;
@@ -1882,10 +1882,10 @@ FDCtrl *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
 fdctrl = sys-state;
 fdctrl-dma_chann = dma_chann; /* FIXME */
 if (fds[0]) {
-qdev_prop_set_drive(dev, driveA, fds[0]-bdrv);
+qdev_prop_set_drive_nofail(dev, driveA, fds[0]-bdrv);
 }
 if (fds[1]) {
-qdev_prop_set_drive(dev, driveB, fds[1]-bdrv);
+qdev_prop_set_drive_nofail(dev, driveB, fds[1]-bdrv);
 }
 qdev_init_nofail(dev);
 sysbus_connect_irq(sys-busdev, 0, irq);
@@ -1903,7 +1903,7 @@ FDCtrl *sun4m_fdctrl_init(qemu_irq irq, 
target_phys_addr_t io_base,
 
 dev = qdev_create(NULL, SUNW,fdtwo);
 if (fds[0]) {
-qdev_prop_set_drive(dev, drive, fds[0]-bdrv);
+qdev_prop_set_drive_nofail(dev, drive, fds[0]-bdrv);
 }
 qdev_init_nofail(dev);
 sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index a5fdab0..b34c473 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -83,7 +83,7 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo 
*drive)
 
 dev = qdev_create(bus-qbus, ide-drive);
 qdev_prop_set_uint32(dev, unit, unit);
-qdev_prop_set_drive(dev, drive, drive-bdrv);
+qdev_prop_set_drive_nofail(dev, drive, drive-bdrv);
 qdev_init_nofail(dev);
 return DO_UPCAST(IDEDevice, qdev, dev);
 }
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index d743192..fe468d6 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -214,7 +214,11 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
 return NULL;
 }
 dev = pci_create(bus, devfn, virtio-blk-pci);
-

Re: [Qemu-devel] [PATCH 2/3] blkdebug: Free QemuOpts after having read the config

2010-07-01 Thread Kevin Wolf
Am 01.07.2010 08:47, schrieb Markus Armbruster:
 Kevin Wolf kw...@redhat.com writes:
 
 Forgetting to free them means that the next instance inherits all rules and
 gets its own rules only additionally.
 
 I also found a use for freeing a complete QemuOptsList, here's my
 solution.  The code that needs it isn't ready, yet.  If you'd like to
 use it, I can push it to my repo.

If you have a use for it, it's the better solution. I'll rebase on top
of this as soon as you have sent it as a proper patch.

Kevin



[Qemu-devel] [PATCH v3 06/13] qdev: Decouple qdev_prop_drive from DriveInfo

2010-07-01 Thread Markus Armbruster
Make the property point to BlockDriverState, cutting out the DriveInfo
middleman.  This prepares the ground for block devices that don't have
a DriveInfo.

Currently all user-defined ones have a DriveInfo, because the only way
to define one is -drive  friends (they go through drive_init()).
DriveInfo is closely tied to -drive, and like -drive, it mixes
information about host and guest part of the block device.  I'm
working towards a new way to define block devices, with clean
host/guest separation, and I need to get DriveInfo out of the way for
that.

Fortunately, the device models are perfectly happy with
BlockDriverState, except for two places: ide_drive_initfn() and
scsi_disk_initfn() need to check the DriveInfo for a serial number set
with legacy -drive serial=...  Use drive_get_by_blockdev() there.

Device model code should now use DriveInfo only when explicitly
dealing with drives defined the old way, i.e. without -device.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 block_int.h  |6 ++
 hw/fdc.c |   22 ++
 hw/ide/core.c|   17 +
 hw/ide/internal.h|2 +-
 hw/ide/qdev.c|   12 
 hw/pci-hotplug.c |4 ++--
 hw/qdev-properties.c |   21 -
 hw/qdev.h|6 +++---
 hw/s390-virtio.c |2 +-
 hw/scsi-bus.c|8 
 hw/scsi-disk.c   |   16 +++-
 hw/scsi-generic.c|6 +++---
 hw/scsi.h|2 +-
 hw/usb-msd.c |   15 +++
 hw/virtio-blk.c  |2 +-
 hw/virtio-pci.c  |4 ++--
 16 files changed, 73 insertions(+), 72 deletions(-)

diff --git a/block_int.h b/block_int.h
index b64a009..e60aed4 100644
--- a/block_int.h
+++ b/block_int.h
@@ -210,10 +210,8 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size);
 int is_windows_drive(const char *filename);
 #endif
 
-struct DriveInfo;
-
 typedef struct BlockConf {
-struct DriveInfo *dinfo;
+BlockDriverState *bs;
 uint16_t physical_block_size;
 uint16_t logical_block_size;
 uint16_t min_io_size;
@@ -234,7 +232,7 @@ static inline unsigned int get_physical_block_exp(BlockConf 
*conf)
 }
 
 #define DEFINE_BLOCK_PROPERTIES(_state, _conf)  \
-DEFINE_PROP_DRIVE(drive, _state, _conf.dinfo),\
+DEFINE_PROP_DRIVE(drive, _state, _conf.bs),   \
 DEFINE_PROP_UINT16(logical_block_size, _state,\
_conf.logical_block_size, 512),  \
 DEFINE_PROP_UINT16(physical_block_size, _state,   \
diff --git a/hw/fdc.c b/hw/fdc.c
index 45a876d..08712bc 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -80,7 +80,6 @@ typedef enum FDiskFlags {
 } FDiskFlags;
 
 typedef struct FDrive {
-DriveInfo *dinfo;
 BlockDriverState *bs;
 /* Drive status */
 FDriveType drive;
@@ -100,7 +99,6 @@ typedef struct FDrive {
 static void fd_init(FDrive *drv)
 {
 /* Drive */
-drv-bs = drv-dinfo ? drv-dinfo-bdrv : NULL;
 drv-drive = FDRIVE_DRV_NONE;
 drv-perpendicular = 0;
 /* Disk */
@@ -1862,10 +1860,10 @@ FDCtrl *fdctrl_init_isa(DriveInfo **fds)
 
 dev = isa_create(isa-fdc);
 if (fds[0]) {
-qdev_prop_set_drive(dev-qdev, driveA, fds[0]);
+qdev_prop_set_drive(dev-qdev, driveA, fds[0]-bdrv);
 }
 if (fds[1]) {
-qdev_prop_set_drive(dev-qdev, driveB, fds[1]);
+qdev_prop_set_drive(dev-qdev, driveB, fds[1]-bdrv);
 }
 if (qdev_init(dev-qdev)  0)
 return NULL;
@@ -1884,10 +1882,10 @@ FDCtrl *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
 fdctrl = sys-state;
 fdctrl-dma_chann = dma_chann; /* FIXME */
 if (fds[0]) {
-qdev_prop_set_drive(dev, driveA, fds[0]);
+qdev_prop_set_drive(dev, driveA, fds[0]-bdrv);
 }
 if (fds[1]) {
-qdev_prop_set_drive(dev, driveB, fds[1]);
+qdev_prop_set_drive(dev, driveB, fds[1]-bdrv);
 }
 qdev_init_nofail(dev);
 sysbus_connect_irq(sys-busdev, 0, irq);
@@ -1905,7 +1903,7 @@ FDCtrl *sun4m_fdctrl_init(qemu_irq irq, 
target_phys_addr_t io_base,
 
 dev = qdev_create(NULL, SUNW,fdtwo);
 if (fds[0]) {
-qdev_prop_set_drive(dev, drive, fds[0]);
+qdev_prop_set_drive(dev, drive, fds[0]-bdrv);
 }
 qdev_init_nofail(dev);
 sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
@@ -2030,8 +2028,8 @@ static ISADeviceInfo isa_fdc_info = {
 .qdev.vmsd  = vmstate_isa_fdc,
 .qdev.reset = fdctrl_external_reset_isa,
 .qdev.props = (Property[]) {
-DEFINE_PROP_DRIVE(driveA, FDCtrlISABus, state.drives[0].dinfo),
-DEFINE_PROP_DRIVE(driveB, FDCtrlISABus, state.drives[1].dinfo),
+DEFINE_PROP_DRIVE(driveA, FDCtrlISABus, state.drives[0].bs),
+DEFINE_PROP_DRIVE(driveB, FDCtrlISABus, state.drives[1].bs),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
@@ -2053,8 +2051,8 @@ static SysBusDeviceInfo 

[Qemu-devel] [PATCH v3 02/13] ide: Make it explicit that ide_create_drive() can't fail

2010-07-01 Thread Markus Armbruster
All callers of ide_create_drive() ignore its value.  Currently
harmless, because it fails only when qdev_init() fails, which fails
only when ide_drive_initfn() fails, which never fails.

Brittle.  Change it to die instead of silently ignoring failure.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/ide/qdev.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 0f9f22e..127478b 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -84,8 +84,7 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo 
*drive)
 dev = qdev_create(bus-qbus, ide-drive);
 qdev_prop_set_uint32(dev, unit, unit);
 qdev_prop_set_drive(dev, drive, drive);
-if (qdev_init(dev)  0)
-return NULL;
+qdev_init_nofail(dev);
 return DO_UPCAST(IDEDevice, qdev, dev);
 }
 
-- 
1.6.6.1




[Qemu-devel] [PATCH v3 00/13] More block-related fixes and cleanups

2010-07-01 Thread Markus Armbruster
I'm working on cleanly separating block device host and guest parts.
I'd like to route all this work through Kevin's block tree.  This is
still just preliminaries.

There will be at least one more round of cleanup  fixes before
blockdev_add proper.  I intend to start with a minimal QMP-only
version, then add features.

v3: cover close of snapshot device, not just delete
clean up bdrv_snapshots()

v2: plug leaks pointed out by Kevin Wolf
fix qdev_prop_set_drive() to set the property only when attach
succeeded (free_drive() will die without this)

Markus Armbruster (13):
  scsi: scsi_bus_legacy_handle_cmdline() can fail, fix callers
  ide: Make it explicit that ide_create_drive() can't fail
  blockdev: Remove drive_get_serial()
  blockdev: New drive_get_by_blockdev()
  blockdev: Clean up automatic drive deletion
  qdev: Decouple qdev_prop_drive from DriveInfo
  blockdev: drive_get_by_id() is no longer used, remove
  block: Catch attempt to attach multiple devices to a blockdev
  savevm: Survive hot-unplug of snapshot device
  block: Clean up bdrv_snapshots()
  block: Fix virtual media change for if=none
  ide: Make PIIX and ISA IDE init functions return the qdev
  pc: Fix CMOS info for drives defined with -device

 block.c  |   55 +
 block.h  |5 +++
 block_int.h  |8 ++--
 blockdev.c   |   45 +++-
 blockdev.h   |7 +++-
 hw/esp.c |3 +-
 hw/fdc.c |   32 +---
 hw/ide.h |   13 ---
 hw/ide/core.c|   18 +
 hw/ide/internal.h|2 +-
 hw/ide/isa.c |8 ++--
 hw/ide/piix.c|6 ++-
 hw/ide/qdev.c|   22 ---
 hw/lsi53c895a.c  |2 +-
 hw/pc.c  |   94 +++--
 hw/pc.h  |3 +-
 hw/pc_piix.c |   16 ++---
 hw/pci-hotplug.c |   11 +-
 hw/qdev-properties.c |   47 +
 hw/qdev.h|7 ++--
 hw/s390-virtio.c |2 +-
 hw/scsi-bus.c|   20 +++
 hw/scsi-disk.c   |   21 ++-
 hw/scsi-generic.c|7 ++--
 hw/scsi.h|4 +-
 hw/usb-msd.c |   30 +---
 hw/virtio-blk.c  |3 +-
 hw/virtio-pci.c  |4 +-
 savevm.c |   31 ++--
 29 files changed, 348 insertions(+), 178 deletions(-)




[Qemu-devel] [PATCH v3 04/13] blockdev: New drive_get_by_blockdev()

2010-07-01 Thread Markus Armbruster
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 blockdev.c |   12 
 blockdev.h |1 +
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e0495e5..ba4f66f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -78,6 +78,18 @@ int drive_get_max_bus(BlockInterfaceType type)
 return max_bus;
 }
 
+DriveInfo *drive_get_by_blockdev(BlockDriverState *bs)
+{
+DriveInfo *dinfo;
+
+QTAILQ_FOREACH(dinfo, drives, next) {
+if (dinfo-bdrv == bs) {
+return dinfo;
+}
+}
+return NULL;
+}
+
 static void bdrv_format_print(void *opaque, const char *name)
 {
 fprintf(stderr,  %s, name);
diff --git a/blockdev.h b/blockdev.h
index a936785..6ab592f 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -40,6 +40,7 @@ extern DriveInfo *drive_get(BlockInterfaceType type, int bus, 
int unit);
 extern DriveInfo *drive_get_by_id(const char *id);
 extern int drive_get_max_bus(BlockInterfaceType type);
 extern void drive_uninit(DriveInfo *dinfo);
+extern DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
 
 extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
 extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
-- 
1.6.6.1




[Qemu-devel] [PATCH v3 09/13] savevm: Survive hot-unplug of snapshot device

2010-07-01 Thread Markus Armbruster
savevm.c keeps a pointer to the snapshot block device.  If you manage
to get that device deleted, the pointer dangles, and the next snapshot
operation will crash  burn.  Unplugging a guest device that uses it
does the trick:

$ MALLOC_PERTURB_=234 qemu-system-x86_64 [...]
QEMU 0.12.50 monitor - type 'help' for more information
(qemu) info snapshots
No available block device supports snapshots
(qemu) drive_add auto if=none,file=tmp.qcow2
OK
(qemu) device_add usb-storage,id=foo,drive=none1
(qemu) info snapshots
Snapshot devices: none1
Snapshot list (from none1):
IDTAG VM SIZEDATE   VM CLOCK
(qemu) device_del foo
(qemu) info snapshots
Snapshot devices:
Segmentation fault (core dumped)

Move management of that pointer to block.c, and zap it when the device
it points becomes unusable.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 block.c  |   26 ++
 block.h  |1 +
 savevm.c |   31 ---
 3 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/block.c b/block.c
index 5e0ffa0..acb4182 100644
--- a/block.c
+++ b/block.c
@@ -63,6 +63,9 @@ static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
 static QLIST_HEAD(, BlockDriver) bdrv_drivers =
 QLIST_HEAD_INITIALIZER(bdrv_drivers);
 
+/* The device to use for VM snapshots */
+static BlockDriverState *bs_snapshots;
+
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
 
@@ -623,6 +626,9 @@ unlink_and_fail:
 void bdrv_close(BlockDriverState *bs)
 {
 if (bs-drv) {
+if (bs == bs_snapshots) {
+bs_snapshots = NULL;
+}
 if (bs-backing_hd) {
 bdrv_delete(bs-backing_hd);
 bs-backing_hd = NULL;
@@ -671,6 +677,7 @@ void bdrv_delete(BlockDriverState *bs)
 bdrv_delete(bs-file);
 }
 
+assert(bs != bs_snapshots);
 qemu_free(bs);
 }
 
@@ -1772,6 +1779,25 @@ int bdrv_can_snapshot(BlockDriverState *bs)
 return 1;
 }
 
+BlockDriverState *bdrv_snapshots(void)
+{
+BlockDriverState *bs;
+
+if (bs_snapshots)
+return bs_snapshots;
+
+bs = NULL;
+while ((bs = bdrv_next(bs))) {
+if (bdrv_can_snapshot(bs)) {
+goto ok;
+}
+}
+return NULL;
+ ok:
+bs_snapshots = bs;
+return bs;
+}
+
 int bdrv_snapshot_create(BlockDriverState *bs,
  QEMUSnapshotInfo *sn_info)
 {
diff --git a/block.h b/block.h
index 88ac06e..012c2a1 100644
--- a/block.h
+++ b/block.h
@@ -193,6 +193,7 @@ const char *bdrv_get_encrypted_filename(BlockDriverState 
*bs);
 void bdrv_get_backing_filename(BlockDriverState *bs,
char *filename, int filename_size);
 int bdrv_can_snapshot(BlockDriverState *bs);
+BlockDriverState *bdrv_snapshots(void);
 int bdrv_snapshot_create(BlockDriverState *bs,
  QEMUSnapshotInfo *sn_info);
 int bdrv_snapshot_goto(BlockDriverState *bs,
diff --git a/savevm.c b/savevm.c
index 20354a8..f1f450e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -83,9 +83,6 @@
 #include qemu_socket.h
 #include qemu-queue.h
 
-/* point to the block driver where the snapshots are managed */
-static BlockDriverState *bs_snapshots;
-
 #define SELF_ANNOUNCE_ROUNDS 5
 
 #ifndef ETH_P_RARP
@@ -1575,26 +1572,6 @@ out:
 return ret;
 }
 
-static BlockDriverState *get_bs_snapshots(void)
-{
-BlockDriverState *bs;
-
-if (bs_snapshots)
-return bs_snapshots;
-/* FIXME what if bs_snapshots gets hot-unplugged? */
-
-bs = NULL;
-while ((bs = bdrv_next(bs))) {
-if (bdrv_can_snapshot(bs)) {
-goto ok;
-}
-}
-return NULL;
- ok:
-bs_snapshots = bs;
-return bs;
-}
-
 static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
   const char *name)
 {
@@ -1674,7 +1651,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 }
 }
 
-bs = get_bs_snapshots();
+bs = bdrv_snapshots();
 if (!bs) {
 monitor_printf(mon, No block device can accept snapshots\n);
 return;
@@ -1769,7 +1746,7 @@ int load_vmstate(const char *name)
 }
 }
 
-bs = get_bs_snapshots();
+bs = bdrv_snapshots();
 if (!bs) {
 error_report(No block device supports snapshots);
 return -EINVAL;
@@ -1833,7 +1810,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
 int ret;
 const char *name = qdict_get_str(qdict, name);
 
-bs = get_bs_snapshots();
+bs = bdrv_snapshots();
 if (!bs) {
 monitor_printf(mon, No block device supports snapshots\n);
 return;
@@ -1863,7 +1840,7 @@ void do_info_snapshots(Monitor *mon)
 int nb_sns, i;
 char buf[256];
 
-bs = get_bs_snapshots();
+bs = bdrv_snapshots();
 if (!bs) {
 monitor_printf(mon, No available block device supports snapshots\n);
 return;
-- 
1.6.6.1




[Qemu-devel] [PATCH v3 05/13] blockdev: Clean up automatic drive deletion

2010-07-01 Thread Markus Armbruster
We automatically delete blockdev host parts on unplug of the guest
device.  Too much magic, but we can't change that now.

The delete happens early in the guest device teardown, before the
connection to the host part is severed.  Thus, the guest part's
pointer to the host part dangles for a brief time.  No actual harm
comes from this, but we'll catch such dangling pointers a few commits
down the road.  Clean up the dangling pointers by delaying the
automatic deletion until the guest part's pointer is gone.

Device usb-storage deliberately makes two qdev properties refer to the
same drive, because it automatically creates a second device.  Again,
too much magic we can't change now.  Multiple references worked okay
before, but now free_drive() dies for the second one.  Zap the extra
reference.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 blockdev.c   |   23 +++
 blockdev.h   |4 
 hw/qdev-properties.c |   10 ++
 hw/scsi-disk.c   |2 +-
 hw/scsi-generic.c|2 +-
 hw/usb-msd.c |   20 
 hw/virtio-pci.c  |2 +-
 7 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index ba4f66f..4848112 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -17,6 +17,29 @@
 
 static QTAILQ_HEAD(drivelist, DriveInfo) drives = 
QTAILQ_HEAD_INITIALIZER(drives);
 
+/*
+ * We automatically delete the drive when a device using it gets
+ * unplugged.  Questionable feature, but we can't just drop it.
+ * Device models call blockdev_mark_auto_del() to schedule the
+ * automatic deletion, and generic qdev code calls blockdev_auto_del()
+ * when deletion is actually safe.
+ */
+void blockdev_mark_auto_del(BlockDriverState *bs)
+{
+DriveInfo *dinfo = drive_get_by_blockdev(bs);
+
+dinfo-auto_del = 1;
+}
+
+void blockdev_auto_del(BlockDriverState *bs)
+{
+DriveInfo *dinfo = drive_get_by_blockdev(bs);
+
+if (dinfo-auto_del) {
+drive_uninit(dinfo);
+}
+}
+
 QemuOpts *drive_add(const char *file, const char *fmt, ...)
 {
 va_list ap;
diff --git a/blockdev.h b/blockdev.h
index 6ab592f..32e6979 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -13,6 +13,9 @@
 #include block.h
 #include qemu-queue.h
 
+void blockdev_mark_auto_del(BlockDriverState *bs);
+void blockdev_auto_del(BlockDriverState *bs);
+
 typedef enum {
 IF_NONE,
 IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
@@ -28,6 +31,7 @@ typedef struct DriveInfo {
 BlockInterfaceType type;
 int bus;
 int unit;
+int auto_del;   /* see blockdev_mark_auto_del() */
 QemuOpts *opts;
 char serial[BLOCK_SERIAL_STRLEN + 1];
 QTAILQ_ENTRY(DriveInfo) next;
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 5a8739d..15ca6d3 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -293,6 +293,15 @@ static int parse_drive(DeviceState *dev, Property *prop, 
const char *str)
 return 0;
 }
 
+static void free_drive(DeviceState *dev, Property *prop)
+{
+DriveInfo **ptr = qdev_get_prop_ptr(dev, prop);
+
+if (*ptr) {
+blockdev_auto_del((*ptr)-bdrv);
+}
+}
+
 static int print_drive(DeviceState *dev, Property *prop, char *dest, size_t 
len)
 {
 DriveInfo **ptr = qdev_get_prop_ptr(dev, prop);
@@ -305,6 +314,7 @@ PropertyInfo qdev_prop_drive = {
 .size  = sizeof(DriveInfo*),
 .parse = parse_drive,
 .print = print_drive,
+.free  = free_drive,
 };
 
 /* --- character device --- */
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2b38984..d76e640 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1043,7 +1043,7 @@ static void scsi_destroy(SCSIDevice *dev)
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
 
 scsi_disk_purge_requests(s);
-drive_uninit(s-qdev.conf.dinfo);
+blockdev_mark_auto_del(s-qdev.conf.dinfo-bdrv);
 }
 
 static int scsi_disk_initfn(SCSIDevice *dev)
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index e31060e..1859c94 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -453,7 +453,7 @@ static void scsi_destroy(SCSIDevice *d)
 r = DO_UPCAST(SCSIGenericReq, req, QTAILQ_FIRST(s-qdev.requests));
 scsi_remove_request(r);
 }
-drive_uninit(s-qdev.conf.dinfo);
+blockdev_mark_auto_del(s-qdev.conf.dinfo-bdrv);
 }
 
 static int scsi_generic_initfn(SCSIDevice *dev)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 8e9718c..3dbfcab 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -522,24 +522,36 @@ static void usb_msd_password_cb(void *opaque, int err)
 static int usb_msd_initfn(USBDevice *dev)
 {
 MSDState *s = DO_UPCAST(MSDState, dev, dev);
+DriveInfo *dinfo = s-conf.dinfo;
 
-if (!s-conf.dinfo || !s-conf.dinfo-bdrv) {
+if (!dinfo || !dinfo-bdrv) {
 error_report(usb-msd: drive property not set);
 return -1;
 }
 
+/*
+ * Hack alert: this pretends to be a block device, but it's really
+ * a SCSI bus that can serve 

[Qemu-devel] [PATCH v3 07/13] blockdev: drive_get_by_id() is no longer used, remove

2010-07-01 Thread Markus Armbruster
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 blockdev.c |   12 
 blockdev.h |1 -
 2 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 4848112..cecde2b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -75,18 +75,6 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int 
unit)
 return NULL;
 }
 
-DriveInfo *drive_get_by_id(const char *id)
-{
-DriveInfo *dinfo;
-
-QTAILQ_FOREACH(dinfo, drives, next) {
-if (strcmp(id, dinfo-id))
-continue;
-return dinfo;
-}
-return NULL;
-}
-
 int drive_get_max_bus(BlockInterfaceType type)
 {
 int max_bus;
diff --git a/blockdev.h b/blockdev.h
index 32e6979..37f3a01 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -41,7 +41,6 @@ typedef struct DriveInfo {
 #define MAX_SCSI_DEVS  7
 
 extern DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
-extern DriveInfo *drive_get_by_id(const char *id);
 extern int drive_get_max_bus(BlockInterfaceType type);
 extern void drive_uninit(DriveInfo *dinfo);
 extern DriveInfo *drive_get_by_blockdev(BlockDriverState *bs);
-- 
1.6.6.1




[Qemu-devel] [PATCH v3 12/13] ide: Make PIIX and ISA IDE init functions return the qdev

2010-07-01 Thread Markus Armbruster
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/ide.h  |   11 ++-
 hw/ide/isa.c  |8 
 hw/ide/piix.c |6 --
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/hw/ide.h b/hw/ide.h
index 0e7d540..f0cb320 100644
--- a/hw/ide.h
+++ b/hw/ide.h
@@ -1,17 +1,18 @@
 #ifndef HW_IDE_H
 #define HW_IDE_H
 
-#include qdev.h
+#include isa.h
+#include pci.h
 
 /* ide-isa.c */
-int isa_ide_init(int iobase, int iobase2, int isairq,
- DriveInfo *hd0, DriveInfo *hd1);
+ISADevice *isa_ide_init(int iobase, int iobase2, int isairq,
+DriveInfo *hd0, DriveInfo *hd1);
 
 /* ide-pci.c */
 void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
  int secondary_ide_enabled);
-void pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
-void pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
+PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
+PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
 
 /* ide-macio.c */
 int pmac_ide_init (DriveInfo **hd_table, qemu_irq irq,
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index b6c6347..10777ca 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -75,8 +75,8 @@ static int isa_ide_initfn(ISADevice *dev)
 return 0;
 };
 
-int isa_ide_init(int iobase, int iobase2, int isairq,
- DriveInfo *hd0, DriveInfo *hd1)
+ISADevice *isa_ide_init(int iobase, int iobase2, int isairq,
+DriveInfo *hd0, DriveInfo *hd1)
 {
 ISADevice *dev;
 ISAIDEState *s;
@@ -86,14 +86,14 @@ int isa_ide_init(int iobase, int iobase2, int isairq,
 qdev_prop_set_uint32(dev-qdev, iobase2, iobase2);
 qdev_prop_set_uint32(dev-qdev, irq, isairq);
 if (qdev_init(dev-qdev)  0)
-return -1;
+return NULL;
 
 s = DO_UPCAST(ISAIDEState, dev, dev);
 if (hd0)
 ide_create_drive(s-bus, 0, hd0);
 if (hd1)
 ide_create_drive(s-bus, 1, hd1);
-return 0;
+return dev;
 }
 
 static ISADeviceInfo isa_ide_info = {
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index dad6e86..fa6 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -160,22 +160,24 @@ static int pci_piix4_ide_initfn(PCIDevice *dev)
 
 /* hd_table must contain 4 block drivers */
 /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
-void pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
+PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
 {
 PCIDevice *dev;
 
 dev = pci_create_simple(bus, devfn, piix3-ide);
 pci_ide_create_devs(dev, hd_table);
+return dev;
 }
 
 /* hd_table must contain 4 block drivers */
 /* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
-void pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
+PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
 {
 PCIDevice *dev;
 
 dev = pci_create_simple(bus, devfn, piix4-ide);
 pci_ide_create_devs(dev, hd_table);
+return dev;
 }
 
 static PCIDeviceInfo piix_ide_info[] = {
-- 
1.6.6.1




[Qemu-devel] [PATCH] Fix broken --kerneldir

2010-07-01 Thread Prerna Saxena
Set up host kernel include paths specified by --kerneldir

When host kernel headers are placed in non-standard paths, the 
KVM_CFLAGS are presently invoked only for a few .c files (kvm*.c,vhost*.c) 
and not for other files like machine.c, cpus.c ..etc which also depend on 
linux/kvm.h

Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---
 Makefile.target |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index d58b201..b433112 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -29,12 +29,15 @@ QEMU_PROG=qemu-system-$(TARGET_ARCH2)$(EXESUF)
 endif
 endif
 
+# Set up host kernel include paths specified by --kerneldir
+ifdef CONFIG_KVM
+QEMU_CFLAGS+=$(KVM_CFLAGS)
+endif
+
 PROGS=$(QEMU_PROG)
 
 LIBS+=-lm
 
-kvm.o kvm-all.o vhost.o vhost_net.o: QEMU_CFLAGS+=$(KVM_CFLAGS)
-
 config-target.h: config-target.h-timestamp
 config-target.h-timestamp: config-target.mak
 
-- 
1.6.2.5



-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India




[Qemu-devel] [PATCH v3 10/13] block: Clean up bdrv_snapshots()

2010-07-01 Thread Markus Armbruster

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 block.c |9 -
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index acb4182..603874d 100644
--- a/block.c
+++ b/block.c
@@ -1783,19 +1783,18 @@ BlockDriverState *bdrv_snapshots(void)
 {
 BlockDriverState *bs;
 
-if (bs_snapshots)
+if (bs_snapshots) {
 return bs_snapshots;
+}
 
 bs = NULL;
 while ((bs = bdrv_next(bs))) {
 if (bdrv_can_snapshot(bs)) {
-goto ok;
+bs_snapshots = bs;
+return bs;
 }
 }
 return NULL;
- ok:
-bs_snapshots = bs;
-return bs;
 }
 
 int bdrv_snapshot_create(BlockDriverState *bs,
-- 
1.6.6.1




[Qemu-devel] [PATCH v3 13/13] pc: Fix CMOS info for drives defined with -device

2010-07-01 Thread Markus Armbruster
Drives defined with -drive if=ide get get created along with the IDE
controller, inside machine-init().  That's before cmos_init().
Drives defined with -device get created during generic device init.
That's after cmos_init().  Because of that, CMOS has no information on
them (type, geometry, translation).  Older versions of Windows such as
XP reportedly choke on that.

Split off the part of CMOS initialization that needs to know about
-device devices, and turn it into a reset handler, so it runs after
device creation.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/ide.h  |2 +
 hw/ide/qdev.c |7 
 hw/pc.c   |   94 +++-
 hw/pc.h   |3 +-
 hw/pc_piix.c  |   16 +++---
 5 files changed, 81 insertions(+), 41 deletions(-)

diff --git a/hw/ide.h b/hw/ide.h
index f0cb320..4ccb580 100644
--- a/hw/ide.h
+++ b/hw/ide.h
@@ -23,4 +23,6 @@ void mmio_ide_init (target_phys_addr_t membase, 
target_phys_addr_t membase2,
 qemu_irq irq, int shift,
 DriveInfo *hd0, DriveInfo *hd1);
 
+void ide_get_bs(BlockDriverState *bs[], BusState *qbus);
+
 #endif /* HW_IDE_H */
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index b34c473..2977a16 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -88,6 +88,13 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo 
*drive)
 return DO_UPCAST(IDEDevice, qdev, dev);
 }
 
+void ide_get_bs(BlockDriverState *bs[], BusState *qbus)
+{
+IDEBus *bus = DO_UPCAST(IDEBus, qbus, qbus);
+bs[0] = bus-master ? bus-master-conf.bs : NULL;
+bs[1] = bus-slave  ? bus-slave-conf.bs  : NULL;
+}
+
 /* - */
 
 typedef struct IDEDrive {
diff --git a/hw/pc.c b/hw/pc.c
index 25ebafa..b577fb1 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -25,6 +25,7 @@
 #include pc.h
 #include apic.h
 #include fdc.h
+#include ide.h
 #include pci.h
 #include vmware_vga.h
 #include monitor.h
@@ -275,14 +276,65 @@ static int pc_boot_set(void *opaque, const char 
*boot_device)
 return set_boot_dev(opaque, boot_device, 0);
 }
 
-/* hd_table must contain 4 block drivers */
+typedef struct pc_cmos_init_late_arg {
+ISADevice *rtc_state;
+BusState *idebus0, *idebus1;
+} pc_cmos_init_late_arg;
+
+static void pc_cmos_init_late(void *opaque)
+{
+pc_cmos_init_late_arg *arg = opaque;
+ISADevice *s = arg-rtc_state;
+int val;
+BlockDriverState *hd_table[4];
+int i;
+
+ide_get_bs(hd_table, arg-idebus0);
+ide_get_bs(hd_table + 2, arg-idebus1);
+
+rtc_set_memory(s, 0x12, (hd_table[0] ? 0xf0 : 0) | (hd_table[1] ? 0x0f : 
0));
+if (hd_table[0])
+cmos_init_hd(0x19, 0x1b, hd_table[0], s);
+if (hd_table[1])
+cmos_init_hd(0x1a, 0x24, hd_table[1], s);
+
+val = 0;
+for (i = 0; i  4; i++) {
+if (hd_table[i]) {
+int cylinders, heads, sectors, translation;
+/* NOTE: bdrv_get_geometry_hint() returns the physical
+geometry.  It is always such that: 1 = sects = 63, 1
+= heads = 16, 1 = cylinders = 16383. The BIOS
+geometry can be different if a translation is done. */
+translation = bdrv_get_translation_hint(hd_table[i]);
+if (translation == BIOS_ATA_TRANSLATION_AUTO) {
+bdrv_get_geometry_hint(hd_table[i], cylinders, heads, 
sectors);
+if (cylinders = 1024  heads = 16  sectors = 63) {
+/* No translation. */
+translation = 0;
+} else {
+/* LBA translation. */
+translation = 1;
+}
+} else {
+translation--;
+}
+val |= translation  (i * 2);
+}
+}
+rtc_set_memory(s, 0x39, val);
+
+qemu_unregister_reset(pc_cmos_init_late, opaque);
+}
+
 void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
-  const char *boot_device, DriveInfo **hd_table,
+  const char *boot_device,
+  BusState *idebus0, BusState *idebus1,
   FDCtrl *floppy_controller, ISADevice *s)
 {
 int val;
 int fd0, fd1, nb;
-int i;
+static pc_cmos_init_late_arg arg;
 
 /* various important CMOS locations needed by PC/Bochs bios */
 
@@ -351,38 +403,10 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t 
above_4g_mem_size,
 rtc_set_memory(s, REG_EQUIPMENT_BYTE, val);
 
 /* hard drives */
-
-rtc_set_memory(s, 0x12, (hd_table[0] ? 0xf0 : 0) | (hd_table[1] ? 0x0f : 
0));
-if (hd_table[0])
-cmos_init_hd(0x19, 0x1b, hd_table[0]-bdrv, s);
-if (hd_table[1])
-cmos_init_hd(0x1a, 0x24, hd_table[1]-bdrv, s);
-
-val = 0;
-for (i = 0; i  4; i++) {
-if (hd_table[i]) {
-int cylinders, heads, sectors, translation;
-/* NOTE: bdrv_get_geometry_hint() returns the physical
-geometry.  

[Qemu-devel] [PATCH v3 11/13] block: Fix virtual media change for if=none

2010-07-01 Thread Markus Armbruster
BlockDriverState member removable controls whether virtual media
change (monitor commands change, eject) is allowed.  It is set when
the type hint is BDRV_TYPE_CDROM or BDRV_TYPE_FLOPPY.

The type hint is only set by drive_init().  It sets BDRV_TYPE_FLOPPY
for if=floppy.  It sets BDRV_TYPE_CDROM for media=cdrom and if=ide,
scsi, xen, or none.

if=ide and if=scsi work, because the type hint makes it a CD-ROM.
if=xen likewise, I think.

For the same reason, if=none works when it's used by ide-drive or
scsi-disk.  For other guest devices, there are problems:

* fdc: you can't change virtual media

$ qemu [...] -drive if=none,id=foo,... -global isa-fdc.driveA=foo
QEMU 0.12.50 monitor - type 'help' for more information
(qemu) eject foo
Device 'foo' is not removable

  unless you add media=cdrom, but that makes it readonly.

* virtio: if you add media=cdrom, you can change virtual media.  If
  you eject, the guest gets I/O errors.  If you change, the guest sees
  the drive's contents suddenly change.

* scsi-generic: if you add media=cdrom, you can change virtual media.
  I didn't test what that does to the guest or the physical device,
  but it can't be pretty.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 block.c   |8 
 block.h   |1 +
 hw/fdc.c  |   10 --
 hw/ide/core.c |1 +
 hw/scsi-disk.c|5 -
 hw/scsi-generic.c |1 +
 hw/virtio-blk.c   |1 +
 7 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 603874d..018e04e 100644
--- a/block.c
+++ b/block.c
@@ -1293,6 +1293,14 @@ BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, 
int is_read)
 return is_read ? bs-on_read_error : bs-on_write_error;
 }
 
+void bdrv_set_removable(BlockDriverState *bs, int removable)
+{
+bs-removable = removable;
+if (removable  bs == bs_snapshots) {
+bs_snapshots = NULL;
+}
+}
+
 int bdrv_is_removable(BlockDriverState *bs)
 {
 return bs-removable;
diff --git a/block.h b/block.h
index 012c2a1..3d03b3e 100644
--- a/block.h
+++ b/block.h
@@ -162,6 +162,7 @@ int bdrv_get_translation_hint(BlockDriverState *bs);
 void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
BlockErrorAction on_write_error);
 BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read);
+void bdrv_set_removable(BlockDriverState *bs, int removable);
 int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
 int bdrv_is_sg(BlockDriverState *bs);
diff --git a/hw/fdc.c b/hw/fdc.c
index 1496cfa..6c74878 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1847,10 +1847,16 @@ static void fdctrl_result_timer(void *opaque)
 static void fdctrl_connect_drives(FDCtrl *fdctrl)
 {
 unsigned int i;
+FDrive *drive;
 
 for (i = 0; i  MAX_FD; i++) {
-fd_init(fdctrl-drives[i]);
-fd_revalidate(fdctrl-drives[i]);
+drive = fdctrl-drives[i];
+
+fd_init(drive);
+fd_revalidate(drive);
+if (drive-bs) {
+bdrv_set_removable(drive-bs, 1);
+}
 }
 }
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index cc4591b..ebdceb5 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2629,6 +2629,7 @@ void ide_init_drive(IDEState *s, BlockDriverState *bs,
 pstrcpy(s-version, sizeof(s-version), QEMU_VERSION);
 }
 ide_reset(s);
+bdrv_set_removable(bs, s-is_cdrom);
 }
 
 static void ide_init1(IDEBus *bus, int unit)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index e900eff..3e41011 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1049,6 +1049,7 @@ static void scsi_destroy(SCSIDevice *dev)
 static int scsi_disk_initfn(SCSIDevice *dev)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+int is_cd;
 DriveInfo *dinfo;
 
 if (!s-qdev.conf.bs) {
@@ -1056,6 +1057,7 @@ static int scsi_disk_initfn(SCSIDevice *dev)
 return -1;
 }
 s-bs = s-qdev.conf.bs;
+is_cd = bdrv_get_type_hint(s-bs) == BDRV_TYPE_CDROM;
 
 if (!s-serial) {
 /* try to fall back to value set with legacy -drive serial=... */
@@ -1072,7 +1074,7 @@ static int scsi_disk_initfn(SCSIDevice *dev)
 return -1;
 }
 
-if (bdrv_get_type_hint(s-bs) == BDRV_TYPE_CDROM) {
+if (is_cd) {
 s-qdev.blocksize = 2048;
 } else {
 s-qdev.blocksize = s-qdev.conf.logical_block_size;
@@ -1081,6 +1083,7 @@ static int scsi_disk_initfn(SCSIDevice *dev)
 
 s-qdev.type = TYPE_DISK;
 qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
+bdrv_set_removable(s-bs, is_cd);
 return 0;
 }
 
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 79347f4..3915e78 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -509,6 +509,7 @@ static int scsi_generic_initfn(SCSIDevice *dev)
 DPRINTF(block size %d\n, s-qdev.blocksize);
 s-driver_status = 0;
 memset(s-sensebuf, 0, sizeof(s-sensebuf));
+bdrv_set_removable(s-bs, 

Re: [Qemu-devel] Re: [Bug 599958] Re: Timedrift problems with Win7: hpet missing time drift fixups

2010-07-01 Thread Gleb Natapov
On Thu, Jul 01, 2010 at 09:13:48AM +0200, Jan Kiszka wrote:
   We could potentially use the
  reset_irq_delivered/get_irq_delivered APIC functions to implement
  interrupt catch-up but I think it would be better to try to merge Jan's
  generic IRQ delivered API first.
 
 Which one? I'm a bit tired of rebasing versions that will only be
 rejected again. :)
 
Since it solves existing problem and is rejected without any rational
explanation and without proposing alternative solution (in form of code)
it should be committed.

--
Gleb.



[Qemu-devel] Re: [PATCH v3 00/13] More block-related fixes and cleanups

2010-07-01 Thread Kevin Wolf
Am 01.07.2010 09:30, schrieb Markus Armbruster:
 I'm working on cleanly separating block device host and guest parts.
 I'd like to route all this work through Kevin's block tree.  This is
 still just preliminaries.
 
 There will be at least one more round of cleanup  fixes before
 blockdev_add proper.  I intend to start with a minimal QMP-only
 version, then add features.
 
 v3: cover close of snapshot device, not just delete
 clean up bdrv_snapshots()
 
 v2: plug leaks pointed out by Kevin Wolf
 fix qdev_prop_set_drive() to set the property only when attach
 succeeded (free_drive() will die without this)
 
 Markus Armbruster (13):
   scsi: scsi_bus_legacy_handle_cmdline() can fail, fix callers
   ide: Make it explicit that ide_create_drive() can't fail
   blockdev: Remove drive_get_serial()
   blockdev: New drive_get_by_blockdev()
   blockdev: Clean up automatic drive deletion
   qdev: Decouple qdev_prop_drive from DriveInfo
   blockdev: drive_get_by_id() is no longer used, remove
   block: Catch attempt to attach multiple devices to a blockdev
   savevm: Survive hot-unplug of snapshot device
   block: Clean up bdrv_snapshots()
   block: Fix virtual media change for if=none
   ide: Make PIIX and ISA IDE init functions return the qdev
   pc: Fix CMOS info for drives defined with -device

Thanks, applied all to the block branch.

Kevin



Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.

2010-07-01 Thread Sripathi Kodi
On Thu, 01 Jul 2010 11:01:15 +0530
Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote:

 On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi sripat...@in.ibm.com 
 wrote:
  From: M. Mohan Kumar mo...@in.ibm.com
  
  SYNOPSIS
  
size[4] Tgetattr tag[2] fid[4]
  
size[4] Rgetattr tag[2] lstat[n]
  
 DESCRIPTION
  
The getattr transaction inquires about the file identified by fid.
The reply will contain a machine-independent directory entry,
laid out as follows:
  
   qid.type[1]
  the type of the file (directory, etc.), represented as a bit
  vector corresponding to the high 8 bits of the file's mode
  word.
  
   qid.vers[4]
  version number for given path
  
   qid.path[8]
  the file server's unique identification for the file
  
   st_mode[4]
  Permission and flags
  
   st_nlink[8]
  Number of hard links
  
   st_uid[4]
  User id of owner
  
   st_gid[4]
  Group ID of owner
  
   st_rdev[8]
  Device ID (if special file)
  
   st_size[8]
  Size, in bytes
  
   st_blksize[8]
  Block size for file system IO
  
   st_blocks[8]
  Number of file system blocks allocated
  
   st_atime_sec[8]
  Time of last access, seconds
  
   st_atime_nsec[8]
  Time of last access, nanoseconds
  
   st_mtime_sec[8]
  Time of last modification, seconds
  
   st_mtime_nsec[8]
  Time of last modification, nanoseconds
  
   st_ctime_sec[8]
  Time of last status change, seconds
  
   st_ctime_nsec[8]
  Time of last status change, nanoseconds
  
  
  This patch implements the client side of getattr implementation for 
  9P2000.L.
  It introduces a new structure p9_stat_dotl for getting Linux stat 
  information
  along with QID. The data layout is similar to stat structure in Linux user
  space with the following major differences:
  
  inode (st_ino) is not part of data. Instead qid is.
  
  device (st_dev) is not part of data because this doesn't make sense on the
  client.
  
  All time variables are 64 bit wide on the wire. The kernel seems to use
  32 bit variables for these variables. However, some of the architectures
  have used 64 bit variables and glibc exposes 64 bit variables to user
  space on some architectures. Hence to be on the safer side we have made
  these 64 bit in the protocol. Refer to the comments in
  include/asm-generic/stat.h
  
  
 
 Can we just hold on this patch. There is a discussion to add
 i_generation and inode create time to a variant of stat. So may be the
 protocol bits need those
 

IMHO, we can put this in now and change it later if needed based on how
the discussion about VFS changes go because:
a) 9P2000.L is still at experimental stage, so it allows us to change
the protocol later. 
b) The kernel patch for this is already in linux-next. Without these
patches in QEMU it won't be possible to use 9P2000.L.

Thanks,
Sripathi.

 -aneesh
 



Re: [Qemu-devel] [PATCH 01/14] Add new data type for fprintf like function pointers

2010-07-01 Thread Stefan Weil

Am 09.04.2010 13:20, schrieb Stefan Weil:

Aurelien Jarno schrieb:

On Mon, Mar 29, 2010 at 09:16:52PM +0200, Stefan Weil wrote:

The compiler should check the arguments for these functions.

gcc can do this, but only if the function pointer's prototype
includes the __attribute__ flag.

As the necessary declaration is a bit lengthy, we use a new
data type 'fprintf_function'.

It is not easy to find a single header file which is included
everywhere, so fprint_function had to be declared in several
header files.


I don't really think it is a good idea to duplicate that. It will only
causes problem in the future. Are you sure there is no header for that?
Worst case scenario it's probably better to create a new header.


I had no better idea. As long as the duplicate declarations
always observe the same pattern, they should not really cause
problems. Anybody who knows this pattern (which is also quite
common in system include files) will know that there are duplicates.

I did not want to create a new header because it is really a worst
case scenario with several disadvantages.

In the meantime I noticed that dis-asm.h also uses fprintf like
function pointers, so there is one more header which needs
the same declaration.

Maybe the best solution would be using qemu-common.h in
cpu-exec.c, *-dis.c, */translate.c, and more files.
That would involve a lot of modifications, for example
removing code which re-implements parts of stdio.h in
dyngen-exec.h. Some restrictions why qemu-common.h was
not used might be no longer valid (I think they came
from pre-tcg times). Nevertheless, cris-dis.c even says
that it cannot include qemu-common.h (without giving a
reason). Reordering include statements or adding new
includes can have unwanted side effects which are
difficult to detect.

So this last solution needs a lot of discussion and time.
That's the reason why I did not choose it. Maybe I was wrong
and more developers want to clean up includes, so it can be done.


More files use qemu-common.h now, so it seems possible to declare
the new data type only once (in qemu-common.h).

There are undetected format errors in current code. Without
argument checking by the compiler, even new format errors
are introduced from time to time. Therefore the motivation
for these patches is still given.

Before I send updated patches, I'd like to ask a simple question:

There is already a data type named 'fprintf_type' (without __attribute__
flag) which is only used in *-dis.c. So which name is preferred
for the new data type with __attribute__ flag?

* fprintf_type (already used in *-dis.c)?
* fprintf_function (which I used in my patches)
* any other name (coding rules?)





[Qemu-devel] Re: [PATCH][Tracing] Fix build errors for target i386-linux-user

2010-07-01 Thread Stefan Hajnoczi
On Wed, Jun 30, 2010 at 09:11:45PM +0530, Prerna Saxena wrote:
 [PATCH 1/1] Move definitions of monitor command handlers (do_info_trace, 
 do_info_all_trace_events) to monitor.c. This removes build errors for 
 user targets such as i386-linux-user, which are not linked with monitor.
 
 The export of trace_buf and trace_idx is an unfortunate side effect, 
 since these are needed by do_info_trace which dumps buffer 
 contents.

git grep monitor_printf suggests that other source files do their own
monitor printing.  Did you look into alternatives that don't expose the
trace buffer internals?

I feel #ifndef CONFIG_USER_ONLY in simpletrace.c would be would be nicer
than exposing the internals.  (CONFIG_USER_ONLY is just a guess, I
haven't investigated which is the right thing to test for.)

Stefan

 
 Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
 ---
  monitor.c |   21 +
  simpletrace.c |   39 ++-
  tracetool |   16 
  3 files changed, 39 insertions(+), 37 deletions(-)
 
 diff --git a/monitor.c b/monitor.c
 index 433a3ec..9b5d65a 100644
 --- a/monitor.c
 +++ b/monitor.c
 @@ -540,6 +540,27 @@ static void do_change_trace_event_state(Monitor *mon, 
 const QDict *qdict)
  bool new_state = qdict_get_bool(qdict, option);
  change_trace_event_state(tp_name, new_state);
  }
 +
 +void do_info_trace(Monitor *mon)
 +{
 +unsigned int i;
 +
 +for (i = 0; i  trace_idx ; i++) {
 +monitor_printf(mon, Event %lu : %lx %lx %lx %lx %lx\n,
 +  trace_buf[i].event, trace_buf[i].x1, 
 trace_buf[i].x2,
 +trace_buf[i].x3, trace_buf[i].x4, 
 trace_buf[i].x5);
 +}
 +}
 +
 +void do_info_all_trace_events(Monitor *mon)
 +{
 +unsigned int i;
 +
 +for (i = 0; i  NR_TRACE_EVENTS; i++) {
 +monitor_printf(mon, %s [Event ID %u] : state %u\n,
 +trace_list[i].tp_name, i, 
 trace_list[i].state);
 +}
 +}
  #endif
 
  static void user_monitor_complete(void *opaque, QObject *ret_data)
 diff --git a/simpletrace.c b/simpletrace.c
 index 57c41fc..834b4c1 100644
 --- a/simpletrace.c
 +++ b/simpletrace.c
 @@ -1,23 +1,9 @@
  #include stdlib.h
  #include stdio.h
 -#include monitor.h
  #include trace.h
 
 -typedef struct {
 -unsigned long event;
 -unsigned long x1;
 -unsigned long x2;
 -unsigned long x3;
 -unsigned long x4;
 -unsigned long x5;
 -} TraceRecord;
 -
 -enum {
 -TRACE_BUF_LEN = 64 * 1024 / sizeof(TraceRecord),
 -};
 -
 -static TraceRecord trace_buf[TRACE_BUF_LEN];
 -static unsigned int trace_idx;
 +TraceRecord trace_buf[TRACE_BUF_LEN];
 +unsigned int trace_idx;
  static FILE *trace_fp;
 
  static void trace(TraceEventID event, unsigned long x1,
 @@ -69,27 +55,6 @@ void trace5(TraceEventID event, unsigned long x1, unsigned 
 long x2, unsigned lon
  trace(event, x1, x2, x3, x4, x5);
  }
 
 -void do_info_trace(Monitor *mon)
 -{
 -unsigned int i;
 -
 -for (i = 0; i  trace_idx ; i++) {
 -monitor_printf(mon, Event %lu : %lx %lx %lx %lx %lx\n,
 -  trace_buf[i].event, trace_buf[i].x1, 
 trace_buf[i].x2,
 -trace_buf[i].x3, trace_buf[i].x4, 
 trace_buf[i].x5);
 -}
 -}
 -
 -void do_info_all_trace_events(Monitor *mon)
 -{
 -unsigned int i;
 -
 -for (i = 0; i  NR_TRACE_EVENTS; i++) {
 -monitor_printf(mon, %s [Event ID %u] : state %u\n,
 -trace_list[i].tp_name, i, 
 trace_list[i].state);
 -}
 -}
 -
  static TraceEvent* find_trace_event_by_name(const char *tname)
  {
  unsigned int i;
 diff --git a/tracetool b/tracetool
 index c77280d..c7a690d 100755
 --- a/tracetool
 +++ b/tracetool
 @@ -125,6 +125,22 @@ typedef struct {
  bool state;
  } TraceEvent;
 
 +typedef struct {
 +unsigned long event;
 +unsigned long x1;
 +unsigned long x2;
 +unsigned long x3;
 +unsigned long x4;
 +unsigned long x5;
 +} TraceRecord;
 +
 +enum {
 +TRACE_BUF_LEN = 64 * 1024 / sizeof(TraceRecord),
 +};
 +
 +extern TraceRecord trace_buf[TRACE_BUF_LEN];
 +extern unsigned int trace_idx;
 +
  void trace1(TraceEventID event, unsigned long x1);
  void trace2(TraceEventID event, unsigned long x1, unsigned long x2);
  void trace3(TraceEventID event, unsigned long x1, unsigned long x2, unsigned 
 long x3);
 -- 
 1.6.2.5
 
 
 
 -- 
 Prerna Saxena
 
 Linux Technology Centre,
 IBM Systems and Technology Lab,
 Bangalore, India
 



[Qemu-devel] [PATCH 1/2] virtio-serial: Check if virtio queue is ready before consuming data

2010-07-01 Thread Amit Shah
If a virtio-serial port is removed before the guest comes up and
initialises the virtqueues, qemu exits with the message

Guest moved used index from 0 to 61440

This happens because we try to clear any pending buffers from the
virtqueue.

Ensure the virtqueue is initialised before calling any virtqueue
operations.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 hw/virtio-serial-bus.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 7f9d28f..b89daa6 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -139,6 +139,9 @@ static void flush_queued_data(VirtIOSerialPort *port, bool 
discard)
 {
 assert(port);
 
+if (!virtio_queue_ready(port-ovq)) {
+return;
+}
 do_flush_queued_data(port, port-ovq, port-vser-vdev, discard);
 }
 
-- 
1.7.0.1




[Qemu-devel] Re: [PATCH] qemu-img: avoid calling exit(1) to release resources properly

2010-07-01 Thread Kevin Wolf
Am 20.06.2010 21:26, schrieb MORITA Kazutaka:
 This patch removes exit(1) from error(), and properly releases
 resources such as a block driver and an allocated memory.
 
 For testing the Sheepdog block driver with qemu-iotests, it is
 necessary to call bdrv_delete() before the program exits.  Because the
 driver releases the lock of VM images in the close handler.
 
 Signed-off-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp

Thanks, applied to the block branch.

Kevin



[Qemu-devel] [PATCH 2/2] virtio-serial: Assert for virtio queue ready before virtqueue operations

2010-07-01 Thread Amit Shah
In addition to the previous fix for calling do_flush_queued_data() only
when the virtqueue is ready, ensure do_flush_queued_data() gets a vq
that's suitably initialised.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 hw/virtio-serial-bus.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index b89daa6..93cffa2 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -117,6 +117,7 @@ static void do_flush_queued_data(VirtIOSerialPort *port, 
VirtQueue *vq,
 VirtQueueElement elem;
 
 assert(port || discard);
+assert(virtio_queue_ready(vq));
 
 while ((discard || !port-throttled)  virtqueue_pop(vq, elem)) {
 uint8_t *buf;
-- 
1.7.0.1




Re: [Qemu-devel] Re: [Bug 494500] Re: QEMU 0.12.0 does not support KVM with Kernel 2.6.29, bug in ./configure and kvm-all.c

2010-07-01 Thread Jes Sorensen
On 07/01/10 09:04, Jan Kiszka wrote:
 As I wrote in this bug I had problems with a  kernel 2.6.28-11 and the
 _latest_ version of kvm-kmod. It works only with the right (not the
 latest) version of kvm-kmod.
 
 Once you properly installed the kernel headers that kvm-kmod delivers,
 qemu should detect their presence during configure and succeed with this
 step. Of course, you also have to install the kvm-kmod modules which
 will disable those that come with 2.6.28 to actually run qemu in kvm
 mode later on.

Hi Jan,

Actually I would like to apologize, I hadn't realized the situation
around mr Warnke's bug. Had I looked at this and the other bugs he
constantly reopens when it's clear they are not real bugs, I wouldn't
have posted to it in the first place.

I suggest we leave them and don't add anymore to these bugs to avoid
unescessary noise on the list.

Cheers,
Jes



[Qemu-devel] [PATCH] win32: Add missing function setenv

2010-07-01 Thread Stefan Weil
Mingw32 does not provide a declaration and implementation of function
setenv (which is used in sdl.c), so this patch adds both.

Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 os-win32.c |   15 +++
 osdep.h|2 ++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/os-win32.c b/os-win32.c
index d98fd77..dd46bf4 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -34,6 +34,21 @@
 #include qemu-options.h
 
 /***/
+/* Functions missing in mingw */
+
+int setenv(const char *name, const char *value, int overwrite)
+{
+int result = 0;
+if (overwrite || !getenv(name)) {
+size_t length = strlen(name) + strlen(value) + 2;
+char *string = qemu_malloc(length);
+snprintf(string, length, %s=%s, name, value);
+result = putenv(string);
+}
+return result;
+}
+
+/***/
 /* Polling handling */
 
 typedef struct PollingEntry {
diff --git a/osdep.h b/osdep.h
index 75b5816..1cdc7e2 100644
--- a/osdep.h
+++ b/osdep.h
@@ -95,6 +95,8 @@ int qemu_create_pidfile(const char *filename);
 #ifdef _WIN32
 int ffs(int i);
 
+int setenv(const char *name, const char *value, int overwrite);
+
 typedef struct {
 long tv_sec;
 long tv_usec;
-- 
1.7.1




[Qemu-devel] Add argument checking for a number of functions

2010-07-01 Thread Stefan Weil
gcc can check printf like variable arguments.
These patches tell gcc to do so for several functions.

[PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)
[PATCH 2/4] darwin-user: Add GCC attribute (check format arguments)
[PATCH 3/4] qemu-char.h: Add GCC attribute (check format arguments)
[PATCH 4/4] slirp.h: Add GCC attribute (check format arguments)



[Qemu-devel] [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)

2010-07-01 Thread Stefan Weil
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 blockdev.h |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/blockdev.h b/blockdev.h
index 23ea576..3c5c85d 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -42,7 +42,8 @@ extern int drive_get_max_bus(BlockInterfaceType type);
 extern void drive_uninit(DriveInfo *dinfo);
 extern const char *drive_get_serial(BlockDriverState *bdrv);
 
-extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
+extern QemuOpts *drive_add(const char *file, const char *fmt, ...)
+__attribute__ ((__format__ (__printf__, 2, 3)));
 extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
  int *fatal_error);
 
-- 
1.7.1




[Qemu-devel] [PATCH 2/4] darwin-user: Add GCC attribute (check format arguments)

2010-07-01 Thread Stefan Weil
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 darwin-user/qemu.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/darwin-user/qemu.h b/darwin-user/qemu.h
index 462bbda..c8adce7 100644
--- a/darwin-user/qemu.h
+++ b/darwin-user/qemu.h
@@ -100,7 +100,7 @@ int do_sigaction(int sig, const struct sigaction *act,
 int do_sigaltstack(const struct sigaltstack *ss, struct sigaltstack *oss);
 
 void gemu_log(const char *fmt, ...) __attribute__((format(printf,1,2)));
-void qerror(const char *fmt, ...);
+void qerror(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 
 void write_dt(void *ptr, unsigned long addr, unsigned long limit, int flags);
 
-- 
1.7.1




[Qemu-devel] [PATCH 4/4] slirp.h: Add GCC attribute (check format arguments)

2010-07-01 Thread Stefan Weil
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 slirp/slirp.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/slirp/slirp.h b/slirp/slirp.h
index 98a2644..c30c19e 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -294,7 +294,7 @@ void if_start(struct ttys *);
  long gethostid(void);
 #endif
 
-void lprint(const char *, ...);
+void lprint(const char *, ...) __attribute__ ((__format__ (__printf__, 1, 2)));
 
 #ifndef _WIN32
 #include netdb.h
-- 
1.7.1




[Qemu-devel] [PATCH v2 0/4] blkdebug: Fix config with multiple states

2010-07-01 Thread Kevin Wolf
Turns out that using more than one state doesn't really work well. I'm trying
to reproduce a bug for which I need states, so now is the time to fix it.

v2:
- Use Markus' qemu_opts_reset for freeing QemuOpts. Makes the code reusable and
  fixes a use-after-free bug.

Kevin Wolf (3):
  blkdebug: Fix set_state_opts definition
  blkdebug: Free QemuOpts after having read the config
  blkdebug: Initialize state as 1

Markus Armbruster (1):
  qemu-option: New qemu_opts_reset()

 block/blkdebug.c |7 ++-
 qemu-option.c|9 +
 qemu-option.h|1 +
 3 files changed, 16 insertions(+), 1 deletions(-)




[Qemu-devel] [PATCH v2 1/4] qemu-option: New qemu_opts_reset()

2010-07-01 Thread Kevin Wolf
From: Markus Armbruster arm...@redhat.com

Signed-off-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Kevin Wolf kw...@redhat.com
---
 qemu-option.c |9 +
 qemu-option.h |1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 7f70d0f..30327d4 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -719,6 +719,15 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char 
*id, int fail_if_exist
 return opts;
 }
 
+void qemu_opts_reset(QemuOptsList *list)
+{
+QemuOpts *opts, *next_opts;
+
+QTAILQ_FOREACH_SAFE(opts, list-head, next, next_opts) {
+qemu_opts_del(opts);
+}
+}
+
 int qemu_opts_set(QemuOptsList *list, const char *id,
   const char *name, const char *value)
 {
diff --git a/qemu-option.h b/qemu-option.h
index 4823219..9e2406c 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -115,6 +115,7 @@ int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc 
func, void *opaque,
 
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int 
fail_if_exists);
+void qemu_opts_reset(QemuOptsList *list);
 int qemu_opts_set(QemuOptsList *list, const char *id,
   const char *name, const char *value);
 const char *qemu_opts_id(QemuOpts *opts);
-- 
1.6.6.1




[Qemu-devel] [PATCH 3/4] qemu-char.h: Add GCC attribute (check format arguments)

2010-07-01 Thread Stefan Weil
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 qemu-char.h |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/qemu-char.h b/qemu-char.h
index e3a0783..1bbac6e 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -76,7 +76,8 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
 void (*init)(struct CharDriverState *s));
 CharDriverState *qemu_chr_open(const char *label, const char *filename, void 
(*init)(struct CharDriverState *s));
 void qemu_chr_close(CharDriverState *chr);
-void qemu_chr_printf(CharDriverState *s, const char *fmt, ...);
+void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)
+__attribute__ ((__format__ (__printf__, 2, 3)));
 int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len);
 void qemu_chr_send_event(CharDriverState *s, int event);
 void qemu_chr_add_handlers(CharDriverState *s,
-- 
1.7.1




[Qemu-devel] [PATCH v2 2/4] blkdebug: Fix set_state_opts definition

2010-07-01 Thread Kevin Wolf
The list head was initialized to point to the wrong list, so all actions ended
up being handled as inject-error even if they were set-state in fact.

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 block/blkdebug.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 98fed94..4ec8ca6 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -111,7 +111,7 @@ static QemuOptsList inject_error_opts = {
 
 static QemuOptsList set_state_opts = {
 .name = set-state,
-.head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head),
+.head = QTAILQ_HEAD_INITIALIZER(set_state_opts.head),
 .desc = {
 {
 .name = event,
-- 
1.6.6.1




[Qemu-devel] [PATCH v2 4/4] blkdebug: Initialize state as 1

2010-07-01 Thread Kevin Wolf
state = 0 in rules means that the rule is valid for any state. Therefore it's
impossible to have a rule that works only in the initial state. This changes
the initial state from 0 to 1 to make this possible.

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 block/blkdebug.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 78cbff4..2a63df9 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -301,6 +301,9 @@ static int blkdebug_open(BlockDriverState *bs, const char 
*filename, int flags)
 }
 filename = c + 1;
 
+/* Set initial state */
+s-vars.state = 1;
+
 /* Open the backing file */
 ret = bdrv_file_open(bs-file, filename, flags);
 if (ret  0) {
-- 
1.6.6.1




Re: [Qemu-devel] [PATCH] win32: Add missing function setenv

2010-07-01 Thread Jes Sorensen
On 07/01/10 12:47, Stefan Weil wrote:
 Mingw32 does not provide a declaration and implementation of function
 setenv (which is used in sdl.c), so this patch adds both.
 
 Signed-off-by: Stefan Weil w...@mail.berlios.de
[snip]
 diff --git a/osdep.h b/osdep.h
 index 75b5816..1cdc7e2 100644
 --- a/osdep.h
 +++ b/osdep.h
 @@ -95,6 +95,8 @@ int qemu_create_pidfile(const char *filename);
  #ifdef _WIN32
  int ffs(int i);
  
 +int setenv(const char *name, const char *value, int overwrite);
 +
  typedef struct {
  long tv_sec;
  long tv_usec;

Please move this to qemu-os-win32.h instead, otherwise the build will
fail on POSIX systems due to setenv being redefined.

Thanks,
Jes



Re: [Qemu-devel] [PATCH] make qemu.log name unique

2010-07-01 Thread Christophe LYON



Maybe adding a -logfile option would allow what you're trying to achieve
without affecting other use cases? I've always thought that it's strange
that you can only change the logfile location in the monitor and not on
the command line.

Kevin



Here is patch that does what you suggest.

Christophe.
 linux-user/main.c |   26 --
 1 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index a6af2a5..8d1db46 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -34,7 +34,8 @@
 
 #include envlist.h
 
-#define DEBUG_LOGFILE /tmp/qemu.log
+#define DEFAULTDEBUG_LOGFILE /tmp/qemu.log
+static const char *debug_logfile = DEFAULTDEBUG_LOGFILE;
 
 char *exec_path;
 
@@ -2454,7 +2455,8 @@ static void usage(void)
 #endif
\n
Debug options:\n
-   -d options   activate log (logfile=%s)\n
+   -d options   activate logfile\n
+   -logfile filename  set log file name to 'filename' (default %s)\n
-p pagesize  set the host page size to 'pagesize'\n
-singlestep  always run in singlestep mode\n
-strace  log system calls\n
@@ -2472,7 +2474,7 @@ static void usage(void)
TARGET_ARCH,
interp_prefix,
x86_stack_size,
-   DEBUG_LOGFILE);
+   debug_logfile);
 exit(1);
 }
 
@@ -2531,15 +2533,13 @@ int main(int argc, char **argv, char **envp)
 const char *argv0 = NULL;
 int i;
 int ret;
+int log_mask = 0;
 
 if (argc = 1)
 usage();
 
 qemu_cache_utils_init(envp);
 
-/* init debug */
-cpu_set_log_filename(DEBUG_LOGFILE);
-
 if ((envlist = envlist_create()) == NULL) {
 (void) fprintf(stderr, Unable to allocate envlist\n);
 exit(1);
@@ -2562,23 +2562,23 @@ int main(int argc, char **argv, char **envp)
 r++;
 if (!strcmp(r, -)) {
 break;
+} else if (!strcmp(r, logfile)) {
+  debug_logfile = argv[optind++];
 } else if (!strcmp(r, d)) {
-int mask;
 const CPULogItem *item;
 
 	if (optind = argc)
 		break;
 
 	r = argv[optind++];
-mask = cpu_str_to_log_mask(r);
-if (!mask) {
+log_mask = cpu_str_to_log_mask(r);
+if (!log_mask) {
 printf(Log items (comma separated):\n);
 for(item = cpu_log_items; item-mask != 0; item++) {
 printf(%-10s %s\n, item-name, item-help);
 }
 exit(1);
 }
-cpu_set_log(mask);
 } else if (!strcmp(r, E)) {
 r = argv[optind++];
 if (envlist_setenv(envlist, r) != 0)
@@ -2648,6 +2648,12 @@ int main(int argc, char **argv, char **envp)
 filename = argv[optind];
 exec_path = argv[optind];
 
+/* init debug */
+if (log_mask) {
+  cpu_set_log_filename(debug_logfile);
+  cpu_set_log(log_mask);
+}
+
 /* Zero out regs */
 memset(regs, 0, sizeof(struct target_pt_regs));
 


Re: [Qemu-devel] [PATCH v2 0/4] blkdebug: Fix config with multiple states

2010-07-01 Thread Markus Armbruster
Looks good to me.



[Qemu-devel] [Bug 600589] [NEW] xchg r8,rax treated as nop

2010-07-01 Thread Vic3Dexe
Public bug reported:

xchg r8,rax (49h 90h) executed as nop (90h) in long mode, in other words
REX not used.

qemu 0.12.4, host Win 7 x64,  running qemu-system-x86_64.exe.

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
xchg r8,rax treated as nop
https://bugs.launchpad.net/bugs/600589
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
xchg r8,rax (49h 90h) executed as nop (90h) in long mode, in other words REX 
not used.

qemu 0.12.4, host Win 7 x64,  running qemu-system-x86_64.exe.





Re: [Qemu-devel] Re: [V9fs-developer] [PATCH] virtio-9p: getattr server implementation for 9P2000.L protocol.

2010-07-01 Thread Aneesh Kumar K. V
On Thu, 1 Jul 2010 14:26:13 +0530, Sripathi Kodi sripat...@in.ibm.com wrote:
 On Thu, 01 Jul 2010 11:01:15 +0530
 Aneesh Kumar K. V aneesh.ku...@linux.vnet.ibm.com wrote:
 
  On Fri, 28 May 2010 16:08:43 +0530, Sripathi Kodi sripat...@in.ibm.com 
  wrote:
   From: M. Mohan Kumar mo...@in.ibm.com
   
   SYNOPSIS
   
 size[4] Tgetattr tag[2] fid[4]
   
 size[4] Rgetattr tag[2] lstat[n]
   
  DESCRIPTION
   
 The getattr transaction inquires about the file identified by fid.
 The reply will contain a machine-independent directory entry,
 laid out as follows:
   
qid.type[1]
   the type of the file (directory, etc.), represented as a bit
   vector corresponding to the high 8 bits of the file's mode
   word.
   
qid.vers[4]
   version number for given path
   
qid.path[8]
   the file server's unique identification for the file
   
st_mode[4]
   Permission and flags
   
st_nlink[8]
   Number of hard links
   
st_uid[4]
   User id of owner
   
st_gid[4]
   Group ID of owner
   
st_rdev[8]
   Device ID (if special file)
   
st_size[8]
   Size, in bytes
   
st_blksize[8]
   Block size for file system IO
   
st_blocks[8]
   Number of file system blocks allocated
   
st_atime_sec[8]
   Time of last access, seconds
   
st_atime_nsec[8]
   Time of last access, nanoseconds
   
st_mtime_sec[8]
   Time of last modification, seconds
   
st_mtime_nsec[8]
   Time of last modification, nanoseconds
   
st_ctime_sec[8]
   Time of last status change, seconds
   
st_ctime_nsec[8]
   Time of last status change, nanoseconds
   
   
   This patch implements the client side of getattr implementation for 
   9P2000.L.
   It introduces a new structure p9_stat_dotl for getting Linux stat 
   information
   along with QID. The data layout is similar to stat structure in Linux user
   space with the following major differences:
   
   inode (st_ino) is not part of data. Instead qid is.
   
   device (st_dev) is not part of data because this doesn't make sense on the
   client.
   
   All time variables are 64 bit wide on the wire. The kernel seems to use
   32 bit variables for these variables. However, some of the architectures
   have used 64 bit variables and glibc exposes 64 bit variables to user
   space on some architectures. Hence to be on the safer side we have made
   these 64 bit in the protocol. Refer to the comments in
   include/asm-generic/stat.h
   
   
  
  Can we just hold on this patch. There is a discussion to add
  i_generation and inode create time to a variant of stat. So may be the
  protocol bits need those
  
 
 IMHO, we can put this in now and change it later if needed based on how
 the discussion about VFS changes go because:
 a) 9P2000.L is still at experimental stage, so it allows us to change
 the protocol later. 
 b) The kernel patch for this is already in linux-next. Without these
 patches in QEMU it won't be possible to use 9P2000.L.
 

The comment was w.r.t kernel and qemu patches. I am not sure whether we
would reach a conclusion about how the syscall interface soon. But i
guess BSD already support birth time. So in any way having a protocol
update to support i_generation and birth time is a good thing to do,
because we already know that NFS and CIFS would need it from a file system.

-aneesh



Re: [Qemu-devel] [PATCH] make qemu.log name unique

2010-07-01 Thread Stefan Weil

Am 01.07.2010 13:53, schrieb Christophe LYON:



Maybe adding a -logfile option would allow what you're trying to achieve
without affecting other use cases? I've always thought that it's strange
that you can only change the logfile location in the monitor and not on
the command line.

Kevin



Here is patch that does what you suggest.

Christophe.


Patches need a Signed-off-by, and for the code review
it's better to send them inline.

What about patching the documentation, too?

Regards
Stefan




Re: [Qemu-devel] [PATCH] win32: Add missing function setenv

2010-07-01 Thread Stefan Weil

Am 01.07.2010 13:50, schrieb Jes Sorensen:

On 07/01/10 12:47, Stefan Weil wrote:
   

Mingw32 does not provide a declaration and implementation of function
setenv (which is used in sdl.c), so this patch adds both.

Signed-off-by: Stefan Weilw...@mail.berlios.de
 

[snip]
   

diff --git a/osdep.h b/osdep.h
index 75b5816..1cdc7e2 100644
--- a/osdep.h
+++ b/osdep.h
@@ -95,6 +95,8 @@ int qemu_create_pidfile(const char *filename);
  #ifdef _WIN32
  int ffs(int i);

+int setenv(const char *name, const char *value, int overwrite);
+
  typedef struct {
  long tv_sec;
  long tv_usec;
 

Please move this to qemu-os-win32.h instead, otherwise the build will
fail on POSIX systems due to setenv being redefined.

Thanks,
Jes
   


It won't fail for two reasons:

* It is not redefined (at least for linux systems) because I used the 
POSIX declaration.


* It is compiled only for _WIN32 (see line 95).

Regards
Stefan




Re: [Qemu-devel] [PATCH] win32: Add missing function setenv

2010-07-01 Thread Jes Sorensen
On 07/01/10 15:22, Stefan Weil wrote:
 It won't fail for two reasons:
 
 * It is not redefined (at least for linux systems) because I used the
 POSIX declaration.

This still fails with strict compiler flags.

 * It is compiled only for _WIN32 (see line 95).

True, but we need to move stuff out of osdep.h and into the other files
as much as possible, so it is still preferred that you move it.

Cheers,
Jes



[Qemu-devel] [PATCH 2/2] block: Handle multiwrite errors only when all requests have completed

2010-07-01 Thread Kevin Wolf
Don't try to be clever by freeing all temporary data and calling all callbacks
when the return value (an error) is certain. Doing so has at least two
important problems:

* The temporary data that is freed (qiov, possibly zero buffer) is still used
  by the requests that have not yet completed.
* Calling the callbacks for all requests in the multiwrite means for the caller
  that it may free buffers etc. which are still in use.

Just remember the error value and do the cleanup when all requests have
completed.

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 block.c |5 +
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 9719649..bff7d5a 100644
--- a/block.c
+++ b/block.c
@@ -2056,14 +2056,11 @@ static void multiwrite_cb(void *opaque, int ret)
 
 if (ret  0  !mcb-error) {
 mcb-error = ret;
-multiwrite_user_cb(mcb);
 }
 
 mcb-num_requests--;
 if (mcb-num_requests == 0) {
-if (mcb-error == 0) {
-multiwrite_user_cb(mcb);
-}
+multiwrite_user_cb(mcb);
 qemu_free(mcb);
 }
 }
-- 
1.6.6.1




[Qemu-devel] [PATCH 1/2] block: Fix too early free in multiwrite

2010-07-01 Thread Kevin Wolf
bdrv_aio_writev may call the callback immediately (and it will commonly do so
in error cases). If num_requests doesn't have its final value yet,
multiwrite_cb will falsely detect that all requests are completed and frees
the mcb. However, the mcb is still used by other requests that are started only
afterwards. When all requests are completed, it is freed for the second time.

Fix this by setting the right num_requests from the beginning.

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 block.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index c40dd2c..9719649 100644
--- a/block.c
+++ b/block.c
@@ -2198,6 +2198,7 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, 
BlockRequest *reqs, int num_reqs)
 num_reqs = multiwrite_merge(bs, reqs, num_reqs, mcb);
 
 // Run the aio requests
+mcb-num_requests = num_reqs;
 for (i = 0; i  num_reqs; i++) {
 acb = bdrv_aio_writev(bs, reqs[i].sector, reqs[i].qiov,
 reqs[i].nb_sectors, multiwrite_cb, mcb);
@@ -2206,16 +2207,13 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, 
BlockRequest *reqs, int num_reqs)
 // We can only fail the whole thing if no request has been
 // submitted yet. Otherwise we'll wait for the submitted AIOs to
 // complete and report the error in the callback.
-if (mcb-num_requests == 0) {
+if (i == 0) {
 reqs[i].error = -EIO;
 goto fail;
 } else {
-mcb-num_requests++;
 multiwrite_cb(mcb, -EIO);
 break;
 }
-} else {
-mcb-num_requests++;
 }
 }
 
-- 
1.6.6.1




[Qemu-devel] [PATCH 0/2] block: Fix multiwrite error handling

2010-07-01 Thread Kevin Wolf
The bdrv_aio_multiwrite error handling has some bugs that lead to premature
cleanup, causing use-after-free and double free problems.

Kevin Wolf (2):
  block: Fix too early free in multiwrite
  block: Handle multiwrite errors only when all requests have completed

 block.c |   11 +++
 1 files changed, 3 insertions(+), 8 deletions(-)




Re: [Qemu-devel] Re: [Bug 599958] Re: Timedrift problems with Win7: hpet missing time drift fixups

2010-07-01 Thread Paul Brook
 Since it solves existing problem and is rejected without any rational
 explanation and without proposing alternative solution (in form of code)
 it should be committed.

No. This is not sufficient justification for applying a patch. We should not 
be accepting patches just because they exist.

If a feature[1] is important enough that we need to implement it, then it 
should also warrant getting a good solution. Otherwise we're going to end up 
in exactly the same situation next time someone starts using a nwew 
timesource.

Paul

[1] Time-drift hacks are a new *feature*, not a bugfix. At best they're hiding 
more fundamental flaws, e.g. kvm being incapable of emulating realtime 
behavior.



Re: [Qemu-devel] [PATCH] win32: Add missing function setenv

2010-07-01 Thread Stefan Weil

Am 01.07.2010 15:24, schrieb Jes Sorensen:

On 07/01/10 15:22, Stefan Weil wrote:

It won't fail for two reasons:

* It is not redefined (at least for linux systems) because I used the
POSIX declaration.


This still fails with strict compiler flags.


* It is compiled only for _WIN32 (see line 95).


True, but we need to move stuff out of osdep.h and into the other files
as much as possible, so it is still preferred that you move it.


That's a valid argument. As there is more stuff to move out of osdep.h,
I suggest doing that in a second step. Now, it is most important to get
Windows builds working again (they fail currently with a linker error).



Cheers,
Jes



Cheers,
Stefan




Re: [Qemu-devel] [PATCH] win32: Add missing function setenv

2010-07-01 Thread Jes Sorensen
On 07/01/10 17:51, Stefan Weil wrote:
 Am 01.07.2010 15:24, schrieb Jes Sorensen:
 On 07/01/10 15:22, Stefan Weil wrote:
 It won't fail for two reasons:

 * It is not redefined (at least for linux systems) because I used the
 POSIX declaration.

 This still fails with strict compiler flags.

 * It is compiled only for _WIN32 (see line 95).

 True, but we need to move stuff out of osdep.h and into the other files
 as much as possible, so it is still preferred that you move it.
 
 That's a valid argument. As there is more stuff to move out of osdep.h,
 I suggest doing that in a second step. Now, it is most important to get
 Windows builds working again (they fail currently with a linker error).

Rather than add it to remove it in the next patch, please update your
patch and repost it. That causes less noise and will get win32 building
again just as fast.

Cheers,
Jes





[Qemu-devel] Re: [PATCH] Add vapic.bin to .gitignore

2010-07-01 Thread Marcelo Tosatti
On Thu, Jul 01, 2010 at 12:31:16PM +0900, Hidetoshi Seto wrote:
 # This patch is for qemu-kvm.git
 
 The vapic.bin is a generated binary file.
 
 Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
 ---
  .gitignore |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

Applied, thanks.




Re: [Qemu-devel] [Bug 600589] [NEW] xchg r8,rax treated as nop

2010-07-01 Thread Richard Henderson
On 07/01/2010 05:04 AM, Vic3Dexe wrote:
 Public bug reported:
 
 xchg r8,rax (49h 90h) executed as nop (90h) in long mode, in other words
 REX not used.
 
 qemu 0.12.4, host Win 7 x64,  running qemu-system-x86_64.exe.
 
 ** Affects: qemu
  Importance: Undecided
  Status: New
 

Verified.  Test case for x86_64-linux-user:

.globl  main
.type   main, @function
main:
movl$0, %r8d
movl$1, %eax
xchgq   %r8, %rax
ret

Expected result is exit status 0.


r~



[Qemu-devel] [PATCH] target-i386: Fix xchg rax,r8

2010-07-01 Thread Richard Henderson
We were ignoring REX_B while special-casing NOP, i.e. xchg eax,eax.

Signed-off-by: Richard Henderson r...@twiddle.net
---
 target-i386/translate.c |9 +++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 708b0a1..8cb5cf0 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -5293,6 +5293,7 @@ static target_ulong disas_insn(DisasContext *s, 
target_ulong pc_start)
 break;
 
 case 0x91 ... 0x97: /* xchg R, EAX */
+do_xchg_reg_eax:
 ot = dflag + OT_WORD;
 reg = (b  7) | REX_B(s);
 rm = R_EAX;
@@ -6663,10 +6664,14 @@ static target_ulong disas_insn(DisasContext *s, 
target_ulong pc_start)
 //
 /* misc */
 case 0x90: /* nop */
-/* XXX: xchg + rex handling */
 /* XXX: correct lock test for all insn */
-if (prefixes  PREFIX_LOCK)
+if (prefixes  PREFIX_LOCK) {
 goto illegal_op;
+}
+/* If REX_B is set, then this is xchg eax, r8d, not a nop.  */
+if (REX_B(s)) {
+goto do_xchg_reg_eax;
+}
 if (prefixes  PREFIX_REPZ) {
 gen_svm_check_intercept(s, pc_start, SVM_EXIT_PAUSE);
 }
-- 
1.7.0.1




Re: [Qemu-devel] [Bug 600589] [NEW] xchg r8,rax treated as nop

2010-07-01 Thread malc
On Thu, 1 Jul 2010, Richard Henderson wrote:

 On 07/01/2010 05:04 AM, Vic3Dexe wrote:
  Public bug reported:
  
  xchg r8,rax (49h 90h) executed as nop (90h) in long mode, in other words
  REX not used.
  
  qemu 0.12.4, host Win 7 x64,  running qemu-system-x86_64.exe.
  
  ** Affects: qemu
   Importance: Undecided
   Status: New
  
 
 Verified.  Test case for x86_64-linux-user:
 
   .globl  main
   .type   main, @function
 main:
   movl$0, %r8d
   movl$1, %eax
   xchgq   %r8, %rax
   ret
 
 Expected result is exit status 0.
 

No surprise really:

target-i386/translate.c lines 6665-...

case 0x90: /* nop */
/* XXX: xchg + rex handling */
/* XXX: correct lock test for all insn */

The code to handle that just isn't there.

-- 
mailto:av1...@comtv.ru



Re: [Qemu-devel] Add argument checking for a number of functions

2010-07-01 Thread Richard Henderson
On 07/01/2010 04:08 AM, Stefan Weil wrote:
 gcc can check printf like variable arguments.
 These patches tell gcc to do so for several functions.
 
 [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)
 [PATCH 2/4] darwin-user: Add GCC attribute (check format arguments)
 [PATCH 3/4] qemu-char.h: Add GCC attribute (check format arguments)
 [PATCH 4/4] slirp.h: Add GCC attribute (check format arguments)

Acked-by: Richard Henderson r...@redhat.com


r~



[Qemu-devel] [Bug 588955] Re: qemu segfaults when trying to install winvista64 sp2 64 bit on VM

2010-07-01 Thread Lucas Meneghel Rodrigues
** Description changed:

  When trying to install windows vista sp2 64bit on a KVM VM, we get
  consistently a segfault.
  
  Version of qemu affected: Commit hash for
  git://git.savannah.nongnu.org/qemu.git is
  d9b73e47a3d596c5b33802597ec5bd91ef3348e2 (no tag found)
  
  Backtrace:
  
- [r...@virtlab7 qemu]# gdb 
/usr/local/autotest/tests/kvm/build/bin/qemu-system-x86_64 -c ../core 
+ [r...@virtlab7 qemu]# gdb 
/usr/local/autotest/tests/kvm/build/bin/qemu-system-x86_64 -c ../core
  GNU gdb (GDB) Red Hat Enterprise Linux (7.1-24.el6)
  Copyright (C) 2010 Free Software Foundation, Inc.
  License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
  This is free software: you are free to change and redistribute it.
  There is NO WARRANTY, to the extent permitted by law.  Type show copying
  and show warranty for details.
  This GDB was configured as x86_64-redhat-linux-gnu.
  For bug reporting instructions, please see:
  http://www.gnu.org/software/gdb/bugs/...
  Reading symbols from 
/usr/local/autotest/tests/kvm/build/bin/qemu-system-x86_64...done.
  
  warning: core file may not match specified executable file.
  [New Thread 12852]
  [New Thread 12898]
- Missing separate debuginfo for 
+ Missing separate debuginfo for
  Try: yum --disablerepo='*' --enablerepo='*-debuginfo' install 
/usr/lib/debug/.build-id/da/38811550a55156d7072260d3b89fc8aeb79abf
  Reading symbols from /lib64/librt-2.12.so...Reading symbols from 
/usr/lib/debug/lib64/librt-2.12.so.debug...done.
  done.
  Loaded symbols for /lib64/librt-2.12.so
  Reading symbols from /lib64/libpthread-2.12.so...Reading symbols from 
/usr/lib/debug/lib64/libpthread-2.12.so.debug...done.
  done.
  Loaded symbols for /lib64/libpthread-2.12.so
  Reading symbols from /lib64/libutil-2.12.so...Reading symbols from 
/usr/lib/debug/lib64/libutil-2.12.so.debug...done.
  done.
  Loaded symbols for /lib64/libutil-2.12.so
  Reading symbols from /lib64/libncurses.so.5...(no debugging symbols 
found)...done.
  Loaded symbols for /lib64/libncurses.so.5
  Reading symbols from /usr/lib64/libsasl2.so.2...(no debugging symbols 
found)...done.
  Loaded symbols for /usr/lib64/libsasl2.so.2
  Reading symbols from /usr/lib64/libSDL-1.2.so.0...(no debugging symbols 
found)...done.
  Loaded symbols for /usr/lib64/libSDL-1.2.so.0
  Reading symbols from /usr/lib64/libX11.so.6...(no debugging symbols 
found)...done.
  Loaded symbols for /usr/lib64/libX11.so.6
  Reading symbols from /lib64/libm-2.12.so...Reading symbols from 
/usr/lib/debug/lib64/libm-2.12.so.debug...done.
  done.
  Loaded symbols for /lib64/libm-2.12.so
  Reading symbols from /lib64/libz.so.1...(no debugging symbols found)...done.
  Loaded symbols for /lib64/libz.so.1
  Reading symbols from /lib64/libc-2.12.so...Reading symbols from 
/usr/lib/debug/lib64/libc-2.12.so.debug...done.
  done.
  Loaded symbols for /lib64/libc-2.12.so
  Reading symbols from /lib64/libtinfo.so.5...(no debugging symbols 
found)...done.
  Loaded symbols for /lib64/libtinfo.so.5
  Reading symbols from /lib64/ld-2.12.so...Reading symbols from 
/usr/lib/debug/lib64/ld-2.12.so.debug...done.
  done.
  Loaded symbols for /lib64/ld-2.12.so
  Reading symbols from /lib64/libdl-2.12.so...Reading symbols from 
/usr/lib/debug/lib64/libdl-2.12.so.debug...done.
  done.
  Loaded symbols for /lib64/libdl-2.12.so
  Reading symbols from /lib64/libresolv-2.12.so...Reading symbols from 
/usr/lib/debug/lib64/libresolv-2.12.so.debug...done.
  done.
  Loaded symbols for /lib64/libresolv-2.12.so
  Reading symbols from /lib64/libcrypt-2.12.so...Reading symbols from 
/usr/lib/debug/lib64/libcrypt-2.12.so.debug...done.
  done.
  Loaded symbols for /lib64/libcrypt-2.12.so
  Reading symbols from /usr/lib64/libxcb.so.1...(no debugging symbols 
found)...done.
  Loaded symbols for /usr/lib64/libxcb.so.1
  Reading symbols from /lib64/libfreebl3.so...(no debugging symbols 
found)...done.
  Loaded symbols for /lib64/libfreebl3.so
  Reading symbols from /usr/lib64/libXau.so.6...(no debugging symbols 
found)...done.
  Loaded symbols for /usr/lib64/libXau.so.6
  Reading symbols from /usr/lib64/sasl2/libcrammd5.so...(no debugging symbols 
found)...done.
  Loaded symbols for /usr/lib64/sasl2/libcrammd5.so
  Reading symbols from /usr/lib64/sasl2/libsasldb.so...(no debugging symbols 
found)...done.
  Loaded symbols for /usr/lib64/sasl2/libsasldb.so
  Reading symbols from /lib64/libdb-4.7.so...(no debugging symbols 
found)...done.
  Loaded symbols for /lib64/libdb-4.7.so
  Reading symbols from /usr/lib64/sasl2/liblogin.so...(no debugging symbols 
found)...done.
  Loaded symbols for /usr/lib64/sasl2/liblogin.so
  Reading symbols from /usr/lib64/sasl2/libplain.so...(no debugging symbols 
found)...done.
  Loaded symbols for /usr/lib64/sasl2/libplain.so
  Reading symbols from /usr/lib64/sasl2/libdigestmd5.so...(no debugging symbols 
found)...done.
  Loaded symbols for /usr/lib64/sasl2/libdigestmd5.so
  Reading symbols from /usr/lib64/libcrypto.so.10...(no debugging 

Re: [Qemu-devel] [PATCH] win32: Add missing function setenv

2010-07-01 Thread Stefan Weil

Am 01.07.2010 17:53, schrieb Jes Sorensen:

On 07/01/10 17:51, Stefan Weil wrote:
   

Am 01.07.2010 15:24, schrieb Jes Sorensen:
 

On 07/01/10 15:22, Stefan Weil wrote:
   

It won't fail for two reasons:

* It is not redefined (at least for linux systems) because I used the
POSIX declaration.
 

This still fails with strict compiler flags.

   

* It is compiled only for _WIN32 (see line 95).
 

True, but we need to move stuff out of osdep.h and into the other files
as much as possible, so it is still preferred that you move it.
   

That's a valid argument. As there is more stuff to move out of osdep.h,
I suggest doing that in a second step. Now, it is most important to get
Windows builds working again (they fail currently with a linker error).
 

Rather than add it to remove it in the next patch, please update your
patch and repost it. That causes less noise and will get win32 building
again just as fast.

Cheers,
Jes
   


Two patches are needed anyway.

For reasons of economy, I won't send a new patch.
Feel free do send one which meets your criteria.

Regards,
Stefan




Re: [Qemu-devel] Re: [Bug 599958] Re: Timedrift problems with Win7: hpet missing time drift fixups

2010-07-01 Thread Anthony Liguori

On 07/01/2010 02:13 AM, Jan Kiszka wrote:

Anthony Liguori wrote:
   

-no-hpet works in every version of qemu/qemu-kvm that has included HPET
support.  RHEL disables HPET support by default unlike qemu and qemu-
kvm.

I've updated the bug priority and title to reflect what the issue is.

We only support edge triggered interrupts with HPET which seems to be
what most OSes use anyway.
 

qemu.git now also supports level triggering.
   


Indeed it does :-)

   

  We could potentially use the
reset_irq_delivered/get_irq_delivered APIC functions to implement
interrupt catch-up but I think it would be better to try to merge Jan's
generic IRQ delivered API first.
 

Which one? I'm a bit tired of rebasing versions that will only be
rejected again. :)
   


Yeah, I appreciate your frustration here.

Regards,

Anthony Liguori



Re: [Qemu-devel] Re: [Bug 599958] Re: Timedrift problems with Win7: hpet missing time drift fixups

2010-07-01 Thread Anthony Liguori

On 07/01/2010 10:45 AM, Paul Brook wrote:

Since it solves existing problem and is rejected without any rational
explanation and without proposing alternative solution (in form of code)
it should be committed.
 

No. This is not sufficient justification for applying a patch. We should not
be accepting patches just because they exist.

If a feature[1] is important enough that we need to implement it, then it
should also warrant getting a good solution.


I think it's important to try to be constructive.  It's easy to say that 
something is bad but it's far more productive to take the time to offer 
a better alternative.



  Otherwise we're going to end up
in exactly the same situation next time someone starts using a nwew
timesource.

Paul

[1] Time-drift hacks are a new *feature*, not a bugfix. At best they're hiding
more fundamental flaws, e.g. kvm being incapable of emulating realtime
behavior.
   


I think a fundamental problem in this discussion is that you're 
asserting that the true problem is broken guests.  I contend that 
there is no such thing as broken guests.  Guests behave the way they do 
and to the extent that most hardware accommodates that behavior, our 
goal in QEMU should be to also accommodate that behavior.


Sometimes that means breaking nice abstractions but that's the cost of 
functionality.


I really see no tangible objection to Jan's patches.  They don't impact 
any other code.  They don't inhibit flexibility in the infrastructure.  
You might consider it to be a hack but so what.  QEMU is filled with 
hacks.  It would be useless without them because there would be very 
little code.


Taking these patches really does no harm.  I really think you ought to 
reconsider your position.


Regards,

Anthony Liguori





[Qemu-devel] [PATCH 00/23][PULL]: QMP/Monitor queue

2010-07-01 Thread Luiz Capitulino
Hi Anthony,

The following QMP/Monitor patches have been sent to the list and look good
to me. I also did some basic testing on them.

Most of these changes are bug fixes, the only exception is my new argument
checker series, which is a complete rewrite (that also fix bugs...).

The changes (since 449041d4db1f82f281fe097e832f07cd9ee1e864) are available
in the following repository:

git://repo.or.cz/qemu/qmp-unstable.git for-anthony

Jan Kiszka (6):
  monitor: Fix leakage during completion processing
  monitor: Fix command completion vs. boolean switches
  monitor: Establish cmd flags and convert the async tag
  QMP: Teach basic capability negotiation to python example
  QMP: Fix python helper /wrt long return strings
  monitor: Allow to exclude commands from QMP

Luiz Capitulino (16):
  QMP: Fix error reporting in the async API
  QError: Enhance QERR_DEVICE_NOT_ACTIVE's user desc
  QDict: Rename 'err_value'
  QDict: Small terminology change
  QDict: Introduce functions to retrieve QDictEntry values
  QDict: Introduce new iteration API
  check-qdict: Introduce test for the new iteration API
  QDict: Introduce qdict_get_try_bool()
  Monitor: handle optional '-' arg as a bool
  QMP: New argument checker (first part)
  QMP: New argument checker (second part)
  QMP: Drop old client argument checker
  QError: Introduce QERR_QMP_EXTRA_MEMBER
  QMP: Introduce qmp_check_input_obj()
  QMP: Drop old input object checking
  QMP: handle_qmp_command(): Small cleanup

Yoshiaki Tamura (1):
  net: delete QemuOpts when net_client_init() fails.

 QMP/qmp-shell   |1 +
 QMP/qmp.py  |6 +-
 QMP/vm-info |1 +
 blockdev.c  |2 +-
 check-qdict.c   |   33 -
 migration.c |   16 +-
 monitor.c   |  435 ---
 monitor.h   |4 +
 net.c   |4 +
 qdict.c |  106 --
 qdict.h |   11 +-
 qemu-monitor.hx |2 +-
 qerror.c|6 +-
 qerror.h|3 +
 14 files changed, 418 insertions(+), 212 deletions(-)



[Qemu-devel] [PATCH 01/23] monitor: Fix leakage during completion processing

2010-07-01 Thread Luiz Capitulino
From: Jan Kiszka jan.kis...@siemens.com

Given too many arguments or an invalid command, we were leaking the
duplicated argument strings.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |   23 +++
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/monitor.c b/monitor.c
index 170b269..42ae154 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3882,8 +3882,9 @@ static void monitor_find_completion(const char *cmdline)
next arg */
 len = strlen(cmdline);
 if (len  0  qemu_isspace(cmdline[len - 1])) {
-if (nb_args = MAX_ARGS)
-return;
+if (nb_args = MAX_ARGS) {
+goto cleanup;
+}
 args[nb_args++] = qemu_strdup();
 }
 if (nb_args = 1) {
@@ -3898,12 +3899,15 @@ static void monitor_find_completion(const char *cmdline)
 }
 } else {
 /* find the command */
-for(cmd = mon_cmds; cmd-name != NULL; cmd++) {
-if (compare_cmd(args[0], cmd-name))
-goto found;
+for (cmd = mon_cmds; cmd-name != NULL; cmd++) {
+if (compare_cmd(args[0], cmd-name)) {
+break;
+}
 }
-return;
-found:
+if (!cmd-name) {
+goto cleanup;
+}
+
 ptype = next_arg_type(cmd-args_type);
 for(i = 0; i  nb_args - 2; i++) {
 if (*ptype != '\0') {
@@ -3953,8 +3957,11 @@ static void monitor_find_completion(const char *cmdline)
 break;
 }
 }
-for(i = 0; i  nb_args; i++)
+
+cleanup:
+for (i = 0; i  nb_args; i++) {
 qemu_free(args[i]);
+}
 }
 
 static int monitor_can_read(void *opaque)
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 02/23] monitor: Fix command completion vs. boolean switches

2010-07-01 Thread Luiz Capitulino
From: Jan Kiszka jan.kis...@siemens.com

We now have to move forward to the next argument type via next_arg_type.
This patch fixes completion for 'eject' and maybe also other commands.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 42ae154..b375f10 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3918,7 +3918,7 @@ static void monitor_find_completion(const char *cmdline)
 }
 str = args[nb_args - 1];
 if (*ptype == '-'  ptype[1] != '\0') {
-ptype += 2;
+ptype = next_arg_type(ptype);
 }
 switch(*ptype) {
 case 'F':
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 05/23] QMP: Fix python helper /wrt long return strings

2010-07-01 Thread Luiz Capitulino
From: Jan Kiszka jan.kis...@siemens.com

Remove the arbitrary limitation of 1024 characters per return string and
read complete lines instead. Required for device_show.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 QMP/qmp.py |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/QMP/qmp.py b/QMP/qmp.py
index d9da603..4062f84 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -63,10 +63,14 @@ class QEMUMonitorProtocol:
 
 def __json_read(self):
 try:
-return json.loads(self.sock.recv(1024))
+while True:
+line = json.loads(self.sockfile.readline())
+if not 'event' in line:
+return line
 except ValueError:
 return
 
 def __init__(self, filename):
 self.filename = filename
 self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+self.sockfile = self.sock.makefile()
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 03/23] monitor: Establish cmd flags and convert the async tag

2010-07-01 Thread Luiz Capitulino
From: Jan Kiszka jan.kis...@siemens.com

As we want to add more flags to monitor commands, convert the only so
far existing one accordingly.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c   |6 +++---
 monitor.h   |3 +++
 qemu-monitor.hx |2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index b375f10..980e98d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -112,7 +112,7 @@ typedef struct mon_cmd_t {
 int  (*cmd_async)(Monitor *mon, const QDict *params,
   MonitorCompletion *cb, void *opaque);
 } mhandler;
-int async;
+int flags;
 } mon_cmd_t;
 
 /* file descriptors passed via SCM_RIGHTS */
@@ -327,7 +327,7 @@ static inline int monitor_handler_ported(const mon_cmd_t 
*cmd)
 
 static inline bool monitor_handler_is_async(const mon_cmd_t *cmd)
 {
-return cmd-async != 0;
+return cmd-flags  MONITOR_CMD_ASYNC;
 }
 
 static inline int monitor_has_error(const Monitor *mon)
@@ -2536,7 +2536,7 @@ static const mon_cmd_t info_cmds[] = {
 .help   = show balloon information,
 .user_print = monitor_print_balloon,
 .mhandler.info_async = do_info_balloon,
-.async  = 1,
+.flags  = MONITOR_CMD_ASYNC,
 },
 {
 .name   = qtree,
diff --git a/monitor.h b/monitor.h
index ea15469..9582b9c 100644
--- a/monitor.h
+++ b/monitor.h
@@ -15,6 +15,9 @@ extern Monitor *default_mon;
 #define MONITOR_USE_READLINE  0x02
 #define MONITOR_USE_CONTROL   0x04
 
+/* flags for monitor commands */
+#define MONITOR_CMD_ASYNC   0x0001
+
 /* QMP events */
 typedef enum MonitorEvent {
 QEVENT_SHUTDOWN,
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 9f62b94..2af3de6 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -1287,7 +1287,7 @@ ETEXI
 .help   = request VM to change its memory allocation (in MB),
 .user_print = monitor_user_noop,
 .mhandler.cmd_async = do_balloon,
-.async  = 1,
+.flags  = MONITOR_CMD_ASYNC,
 },
 
 STEXI
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 07/23] QMP: Fix error reporting in the async API

2010-07-01 Thread Luiz Capitulino
The current asynchronous command API doesn't return a QMP response
when the async command fails.

This is easy to reproduce with the balloon command (the sole async
command we have so far): run qemu w/o the '-balloon virtio' option
and try to issue the balloon command via QMP: no response will be
sent to the client.

This commit fixes the problem by making qmp_async_cmd_handler()
return the handler's error code and then calling
monitor_protocol_emitter() if the handler has returned an error.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |   12 
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 980e98d..58b060b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -546,10 +546,10 @@ static void qmp_monitor_complete(void *opaque, QObject 
*ret_data)
 monitor_protocol_emitter(opaque, ret_data);
 }
 
-static void qmp_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
-  const QDict *params)
+static int qmp_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
+ const QDict *params)
 {
-cmd-mhandler.cmd_async(mon, params, qmp_monitor_complete, mon);
+return cmd-mhandler.cmd_async(mon, params, qmp_monitor_complete, mon);
 }
 
 static void qmp_async_info_handler(Monitor *mon, const mon_cmd_t *cmd)
@@ -4239,7 +4239,11 @@ static void handle_qmp_command(JSONMessageParser 
*parser, QList *tokens)
 }
 
 if (monitor_handler_is_async(cmd)) {
-qmp_async_cmd_handler(mon, cmd, args);
+err = qmp_async_cmd_handler(mon, cmd, args);
+if (err) {
+/* emit the error response */
+goto err_out;
+}
 } else {
 monitor_call_handler(mon, cmd, args);
 }
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 06/23] net: delete QemuOpts when net_client_init() fails.

2010-07-01 Thread Luiz Capitulino
From: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp

This fixes the following scenario using QMP.

First, put a bogus argument foo to type, which results in an error.
{execute: netdev_add, arguments: { type: foo, id: netdev1 } }
Then, call it again with correct argument user.
{execute: netdev_add, arguments: { type: user, id: netdev1 } }
This results in DuplicatedId error.

Because the first command was invalid, it should be able to reuse the
same id, and the second command should work.

Reported-by: Luiz Capitulino lcapitul...@redhat.com
Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 net.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net.c b/net.c
index 90bd5a9..8ddf872 100644
--- a/net.c
+++ b/net.c
@@ -1208,6 +1208,10 @@ int do_netdev_add(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 }
 
 res = net_client_init(mon, opts, 1);
+if (res  0) {
+qemu_opts_del(opts);
+}
+
 return res;
 }
 
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 04/23] QMP: Teach basic capability negotiation to python example

2010-07-01 Thread Luiz Capitulino
From: Jan Kiszka jan.kis...@siemens.com

As sending qmp_capabilities on session start became mandatory, both
python examples were broken.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 QMP/qmp-shell |1 +
 QMP/vm-info   |1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-shell b/QMP/qmp-shell
index f89b9af..a5b72d1 100755
--- a/QMP/qmp-shell
+++ b/QMP/qmp-shell
@@ -42,6 +42,7 @@ def main():
 
 qemu = qmp.QEMUMonitorProtocol(argv[1])
 qemu.connect()
+qemu.send(qmp_capabilities)
 
 print 'Connected!'
 
diff --git a/QMP/vm-info b/QMP/vm-info
index 8ebaeb3..be5b038 100755
--- a/QMP/vm-info
+++ b/QMP/vm-info
@@ -24,6 +24,7 @@ def main():
 
 qemu = qmp.QEMUMonitorProtocol(argv[1])
 qemu.connect()
+qemu.send(qmp_capabilities)
 
 for cmd in [ 'version', 'kvm', 'status', 'uuid', 'balloon' ]:
 print cmd + ': ' + str(qemu.send('query-' + cmd))
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 10/23] QDict: Small terminology change

2010-07-01 Thread Luiz Capitulino
Let's call a 'hash' only what is returned by our hash function,
anything else is a 'bucket'.

This helps avoiding confusion with regard to how we traverse
our table.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 check-qdict.c |2 +-
 qdict.c   |   24 
 qdict.h   |4 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/check-qdict.c b/check-qdict.c
index 2c3089f..1b070f4 100644
--- a/check-qdict.c
+++ b/check-qdict.c
@@ -50,7 +50,7 @@ START_TEST(qdict_put_obj_test)
 qdict_put_obj(qdict, , QOBJECT(qint_from_int(num)));
 
 fail_unless(qdict_size(qdict) == 1);
-ent = QLIST_FIRST(qdict-table[12345 % QDICT_HASH_SIZE]);
+ent = QLIST_FIRST(qdict-table[12345 % QDICT_BUCKET_MAX]);
 qi = qobject_to_qint(ent-value);
 fail_unless(qint_get_int(qi) == num);
 
diff --git a/qdict.c b/qdict.c
index c974d6f..71be2eb 100644
--- a/qdict.c
+++ b/qdict.c
@@ -86,11 +86,11 @@ static QDictEntry *alloc_entry(const char *key, QObject 
*value)
  * qdict_find(): List lookup function
  */
 static QDictEntry *qdict_find(const QDict *qdict,
-  const char *key, unsigned int hash)
+  const char *key, unsigned int bucket)
 {
 QDictEntry *entry;
 
-QLIST_FOREACH(entry, qdict-table[hash], next)
+QLIST_FOREACH(entry, qdict-table[bucket], next)
 if (!strcmp(entry-key, key))
 return entry;
 
@@ -110,11 +110,11 @@ static QDictEntry *qdict_find(const QDict *qdict,
  */
 void qdict_put_obj(QDict *qdict, const char *key, QObject *value)
 {
-unsigned int hash;
+unsigned int bucket;
 QDictEntry *entry;
 
-hash = tdb_hash(key) % QDICT_HASH_SIZE;
-entry = qdict_find(qdict, key, hash);
+bucket = tdb_hash(key) % QDICT_BUCKET_MAX;
+entry = qdict_find(qdict, key, bucket);
 if (entry) {
 /* replace key's value */
 qobject_decref(entry-value);
@@ -122,7 +122,7 @@ void qdict_put_obj(QDict *qdict, const char *key, QObject 
*value)
 } else {
 /* allocate a new entry */
 entry = alloc_entry(key, value);
-QLIST_INSERT_HEAD(qdict-table[hash], entry, next);
+QLIST_INSERT_HEAD(qdict-table[bucket], entry, next);
 qdict-size++;
 }
 }
@@ -137,7 +137,7 @@ QObject *qdict_get(const QDict *qdict, const char *key)
 {
 QDictEntry *entry;
 
-entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_HASH_SIZE);
+entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_BUCKET_MAX);
 return (entry == NULL ? NULL : entry-value);
 }
 
@@ -148,8 +148,8 @@ QObject *qdict_get(const QDict *qdict, const char *key)
  */
 int qdict_haskey(const QDict *qdict, const char *key)
 {
-unsigned int hash = tdb_hash(key) % QDICT_HASH_SIZE;
-return (qdict_find(qdict, key, hash) == NULL ? 0 : 1);
+unsigned int bucket = tdb_hash(key) % QDICT_BUCKET_MAX;
+return (qdict_find(qdict, key, bucket) == NULL ? 0 : 1);
 }
 
 /**
@@ -318,7 +318,7 @@ void qdict_iter(const QDict *qdict,
 int i;
 QDictEntry *entry;
 
-for (i = 0; i  QDICT_HASH_SIZE; i++) {
+for (i = 0; i  QDICT_BUCKET_MAX; i++) {
 QLIST_FOREACH(entry, qdict-table[i], next)
 iter(entry-key, entry-value, opaque);
 }
@@ -347,7 +347,7 @@ void qdict_del(QDict *qdict, const char *key)
 {
 QDictEntry *entry;
 
-entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_HASH_SIZE);
+entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_BUCKET_MAX);
 if (entry) {
 QLIST_REMOVE(entry, next);
 qentry_destroy(entry);
@@ -366,7 +366,7 @@ static void qdict_destroy_obj(QObject *obj)
 assert(obj != NULL);
 qdict = qobject_to_qdict(obj);
 
-for (i = 0; i  QDICT_HASH_SIZE; i++) {
+for (i = 0; i  QDICT_BUCKET_MAX; i++) {
 QDictEntry *entry = QLIST_FIRST(qdict-table[i]);
 while (entry) {
 QDictEntry *tmp = QLIST_NEXT(entry, next);
diff --git a/qdict.h b/qdict.h
index 72ea563..dcd2b29 100644
--- a/qdict.h
+++ b/qdict.h
@@ -18,7 +18,7 @@
 #include qemu-queue.h
 #include stdint.h
 
-#define QDICT_HASH_SIZE 512
+#define QDICT_BUCKET_MAX 512
 
 typedef struct QDictEntry {
 char *key;
@@ -29,7 +29,7 @@ typedef struct QDictEntry {
 typedef struct QDict {
 QObject_HEAD;
 size_t size;
-QLIST_HEAD(,QDictEntry) table[QDICT_HASH_SIZE];
+QLIST_HEAD(,QDictEntry) table[QDICT_BUCKET_MAX];
 } QDict;
 
 /* Object API */
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 09/23] QDict: Rename 'err_value'

2010-07-01 Thread Luiz Capitulino
A missing key is not an error.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 qdict.c |6 +++---
 qdict.h |2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/qdict.c b/qdict.c
index 175bc17..c974d6f 100644
--- a/qdict.c
+++ b/qdict.c
@@ -272,16 +272,16 @@ const char *qdict_get_str(const QDict *qdict, const char 
*key)
  *
  * Return integer mapped by 'key', if it is not present in
  * the dictionary or if the stored object is not of QInt type
- * 'err_value' will be returned.
+ * 'def_value' will be returned.
  */
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
-  int64_t err_value)
+  int64_t def_value)
 {
 QObject *obj;
 
 obj = qdict_get(qdict, key);
 if (!obj || qobject_type(obj) != QTYPE_QINT)
-return err_value;
+return def_value;
 
 return qint_get_int(qobject_to_qint(obj));
 }
diff --git a/qdict.h b/qdict.h
index 5e5902c..72ea563 100644
--- a/qdict.h
+++ b/qdict.h
@@ -56,7 +56,7 @@ QList *qdict_get_qlist(const QDict *qdict, const char *key);
 QDict *qdict_get_qdict(const QDict *qdict, const char *key);
 const char *qdict_get_str(const QDict *qdict, const char *key);
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
-  int64_t err_value);
+  int64_t def_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
 #endif /* QDICT_H */
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 12/23] QDict: Introduce new iteration API

2010-07-01 Thread Luiz Capitulino
It's composed of functions qdict_first() and qdict_next(), plus
functions to access QDictEntry values.

This API was suggested by Markus Armbruster arm...@redhat.com and
it offers full control over the iteration process.

The usage is simple, the following example prints all keys in 'qdict'
(it's hopefully better than any English description):

   QDict *qdict;
   const QDictEntry *ent;

   [...]

   for (ent = qdict_first(qdict); ent; ent = qdict_next(qdict, ent)) {
printf(%s , qdict_entry_key(ent));
}

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 qdict.c |   37 +
 qdict.h |2 ++
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/qdict.c b/qdict.c
index c467763..a28a0a9 100644
--- a/qdict.c
+++ b/qdict.c
@@ -345,6 +345,43 @@ void qdict_iter(const QDict *qdict,
 }
 }
 
+static QDictEntry *qdict_next_entry(const QDict *qdict, int first_bucket)
+{
+int i;
+
+for (i = first_bucket; i  QDICT_BUCKET_MAX; i++) {
+if (!QLIST_EMPTY(qdict-table[i])) {
+return QLIST_FIRST(qdict-table[i]);
+}
+}
+
+return NULL;
+}
+
+/**
+ * qdict_first(): Return first qdict entry for iteration.
+ */
+const QDictEntry *qdict_first(const QDict *qdict)
+{
+return qdict_next_entry(qdict, 0);
+}
+
+/**
+ * qdict_next(): Return next qdict entry in an iteration.
+ */
+const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry)
+{
+QDictEntry *ret;
+
+ret = QLIST_NEXT(entry, next);
+if (!ret) {
+unsigned int bucket = tdb_hash(entry-key) % QDICT_BUCKET_MAX;
+ret = qdict_next_entry(qdict, bucket + 1);
+}
+
+return ret;
+}
+
 /**
  * qentry_destroy(): Free all the memory allocated by a QDictEntry
  */
diff --git a/qdict.h b/qdict.h
index 0c8de3c..0e7a43f 100644
--- a/qdict.h
+++ b/qdict.h
@@ -45,6 +45,8 @@ QDict *qobject_to_qdict(const QObject *obj);
 void qdict_iter(const QDict *qdict,
 void (*iter)(const char *key, QObject *obj, void *opaque),
 void *opaque);
+const QDictEntry *qdict_first(const QDict *qdict);
+const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry);
 
 /* Helper to qdict_put_obj(), accepts any object */
 #define qdict_put(qdict, key, obj) \
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 11/23] QDict: Introduce functions to retrieve QDictEntry values

2010-07-01 Thread Luiz Capitulino
Next commit will introduce a new QDict iteration API which
returns QDictEntry entries, but we don't want users to directly
access its members since QDictEntry should be private to QDict.

In the near future this kind of data type will be turned into a
forward reference.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 qdict.c |   21 +
 qdict.h |2 ++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/qdict.c b/qdict.c
index 71be2eb..c467763 100644
--- a/qdict.c
+++ b/qdict.c
@@ -83,6 +83,27 @@ static QDictEntry *alloc_entry(const char *key, QObject 
*value)
 }
 
 /**
+ * qdict_entry_value(): Return qdict entry value
+ *
+ * Return weak reference.
+ */
+QObject *qdict_entry_value(const QDictEntry *entry)
+{
+return entry-value;
+}
+
+/**
+ * qdict_entry_key(): Return qdict entry key
+ *
+ * Return a *pointer* to the string, it has to be duplicated before being
+ * stored.
+ */
+const char *qdict_entry_key(const QDictEntry *entry)
+{
+return entry-key;
+}
+
+/**
  * qdict_find(): List lookup function
  */
 static QDictEntry *qdict_find(const QDict *qdict,
diff --git a/qdict.h b/qdict.h
index dcd2b29..0c8de3c 100644
--- a/qdict.h
+++ b/qdict.h
@@ -34,6 +34,8 @@ typedef struct QDict {
 
 /* Object API */
 QDict *qdict_new(void);
+const char *qdict_entry_key(const QDictEntry *entry);
+QObject *qdict_entry_value(const QDictEntry *entry);
 size_t qdict_size(const QDict *qdict);
 void qdict_put_obj(QDict *qdict, const char *key, QObject *value);
 void qdict_del(QDict *qdict, const char *key);
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 13/23] check-qdict: Introduce test for the new iteration API

2010-07-01 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 check-qdict.c |   31 +++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/check-qdict.c b/check-qdict.c
index 1b070f4..6afce5a 100644
--- a/check-qdict.c
+++ b/check-qdict.c
@@ -194,6 +194,36 @@ START_TEST(qobject_to_qdict_test)
 }
 END_TEST
 
+START_TEST(qdict_iterapi_test)
+{
+int count;
+const QDictEntry *ent;
+
+fail_unless(qdict_first(tests_dict) == NULL);
+
+qdict_put(tests_dict, key1, qint_from_int(1));
+qdict_put(tests_dict, key2, qint_from_int(2));
+qdict_put(tests_dict, key3, qint_from_int(3));
+
+count = 0;
+for (ent = qdict_first(tests_dict); ent; ent = qdict_next(tests_dict, 
ent)){
+fail_unless(qdict_haskey(tests_dict, qdict_entry_key(ent)) == 1);
+count++;
+}
+
+fail_unless(count == qdict_size(tests_dict));
+
+/* Do it again to test restarting */
+count = 0;
+for (ent = qdict_first(tests_dict); ent; ent = qdict_next(tests_dict, 
ent)){
+fail_unless(qdict_haskey(tests_dict, qdict_entry_key(ent)) == 1);
+count++;
+}
+
+fail_unless(count == qdict_size(tests_dict));
+}
+END_TEST
+
 /*
  * Errors test-cases
  */
@@ -338,6 +368,7 @@ static Suite *qdict_suite(void)
 tcase_add_test(qdict_public2_tcase, qdict_haskey_test);
 tcase_add_test(qdict_public2_tcase, qdict_del_test);
 tcase_add_test(qdict_public2_tcase, qobject_to_qdict_test);
+tcase_add_test(qdict_public2_tcase, qdict_iterapi_test);
 
 qdict_errors_tcase = tcase_create(Errors);
 suite_add_tcase(s, qdict_errors_tcase);
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 16/23] QMP: New argument checker (first part)

2010-07-01 Thread Luiz Capitulino
Current QMP's argument checker is more complex than it should be
and has (at least) one serious bug: it ignores unknown arguments.

To solve both problems we introduce a new argument checker. It's
added on top of the existing one, so that there are no regressions
during the transition.

This commit introduces the first part of the new checker, which
is run by qmp_check_client_args() and does the following:

  1. Check if all mandatory arguments were provided
  2. Set flags for argument validation

In order to do that, we transform the args_type string (from
qemu-montor.hx) into a qdict and iterate over it.

Next commit adds the new checker's second part: type checking and
invalid argument detection.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |  106 +
 1 files changed, 106 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 568bfb2..222622b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -177,6 +177,9 @@ static inline void mon_print_count_init(Monitor *mon) { }
 static inline int mon_print_count_get(const Monitor *mon) { return 0; }
 #endif /* CONFIG_DEBUG_MONITOR */
 
+/* QMP checker flags */
+#define QMP_ACCEPT_UNKNOWNS 1
+
 static QLIST_HEAD(mon_list, Monitor) mon_list;
 
 static const mon_cmd_t mon_cmds[];
@@ -4147,6 +4150,104 @@ static int invalid_qmp_mode(const Monitor *mon, const 
char *cmd_name)
 return (qmp_cmd_mode(mon) ? is_cap : !is_cap);
 }
 
+/*
+ * - Check if the client has passed all mandatory args
+ * - Set special flags for argument validation
+ */
+static int check_mandatory_args(const QDict *cmd_args,
+const QDict *client_args, int *flags)
+{
+const QDictEntry *ent;
+
+for (ent = qdict_first(cmd_args); ent; ent = qdict_next(cmd_args, ent)) {
+const char *cmd_arg_name = qdict_entry_key(ent);
+QString *type = qobject_to_qstring(qdict_entry_value(ent));
+assert(type != NULL);
+
+if (qstring_get_str(type)[0] == 'O') {
+assert((*flags  QMP_ACCEPT_UNKNOWNS) == 0);
+*flags |= QMP_ACCEPT_UNKNOWNS;
+} else if (qstring_get_str(type)[0] != '-' 
+   qstring_get_str(type)[1] != '?' 
+   !qdict_haskey(client_args, cmd_arg_name)) {
+qerror_report(QERR_MISSING_PARAMETER, cmd_arg_name);
+return -1;
+}
+}
+
+return 0;
+}
+
+static QDict *qdict_from_args_type(const char *args_type)
+{
+int i;
+QDict *qdict;
+QString *key, *type, *cur_qs;
+
+assert(args_type != NULL);
+
+qdict = qdict_new();
+
+if (args_type == NULL || args_type[0] == '\0') {
+/* no args, empty qdict */
+goto out;
+}
+
+key = qstring_new();
+type = qstring_new();
+
+cur_qs = key;
+
+for (i = 0;; i++) {
+switch (args_type[i]) {
+case ',':
+case '\0':
+qdict_put(qdict, qstring_get_str(key), type);
+QDECREF(key);
+if (args_type[i] == '\0') {
+goto out;
+}
+type = qstring_new(); /* qdict has ref */
+cur_qs = key = qstring_new();
+break;
+case ':':
+cur_qs = type;
+break;
+default:
+qstring_append_chr(cur_qs, args_type[i]);
+break;
+}
+}
+
+out:
+return qdict;
+}
+
+/*
+ * Client argument checking rules:
+ *
+ * 1. Client must provide all mandatory arguments
+ */
+static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
+{
+int flags, err;
+QDict *cmd_args;
+
+cmd_args = qdict_from_args_type(cmd-args_type);
+
+flags = 0;
+err = check_mandatory_args(cmd_args, client_args, flags);
+if (err) {
+goto out;
+}
+
+/* TODO: Check client args type */
+
+out:
+QDECREF(cmd_args);
+return err;
+}
+
 static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 {
 int err;
@@ -4222,6 +4323,11 @@ static void handle_qmp_command(JSONMessageParser 
*parser, QList *tokens)
 
 QDECREF(input);
 
+err = qmp_check_client_args(cmd, args);
+if (err  0) {
+goto err_out;
+}
+
 err = monitor_check_qmp_args(cmd, args);
 if (err  0) {
 goto err_out;
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 15/23] Monitor: handle optional '-' arg as a bool

2010-07-01 Thread Luiz Capitulino
Historically, user monitor arguments beginning with '-' (eg. '-f')
were passed as integers down to handlers.

I've maintained this behavior in the new monitor because we didn't
have a boolean type at the very beginning of QMP. Today we have it
and this behavior is causing trouble to QMP's argument checker.

This commit fixes the problem by doing the following changes:

1. User Monitor

   Before: the optional arg was represented as a QInt, we'd pass 1
   down to handlers if the user specified the argument or
   0 otherwise

   This commit: the optional arg is represented as a QBool, we pass
true down to handlers if the user specified the
argument, otherwise _nothing_ is passed

2. QMP

   Before: the client was required to pass the arg as QBool, but we'd
   convert it to QInt internally. If the argument wasn't passed,
   we'd pass 0 down

   This commit: still require a QBool, but doesn't do any conversion and
doesn't pass any default value

3. Convert existing handlers (do_eject()/do_migrate()) to the new way

   Before: Both handlers would expect a QInt value, either 0 or 1

   This commit: Change the handlers to accept a QBool, they handle the
following cases:

   A) true is passed: the option is enabled
   B) false is passed: the option is disabled
   C) nothing is passed: option not specified, use
 default behavior

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 blockdev.c  |2 +-
 migration.c |   16 +++-
 monitor.c   |   17 +++--
 3 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3b8c606..4dcfad8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -521,7 +521,7 @@ static int eject_device(Monitor *mon, BlockDriverState *bs, 
int force)
 int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 BlockDriverState *bs;
-int force = qdict_get_int(qdict, force);
+int force = qdict_get_try_bool(qdict, force, 0);
 const char *filename = qdict_get_str(qdict, device);
 
 bs = bdrv_find(filename);
diff --git a/migration.c b/migration.c
index b49964c..650eb78 100644
--- a/migration.c
+++ b/migration.c
@@ -75,7 +75,9 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 {
 MigrationState *s = NULL;
 const char *p;
-int detach = qdict_get_int(qdict, detach);
+int detach = qdict_get_try_bool(qdict, detach, 0);
+int blk = qdict_get_try_bool(qdict, blk, 0);
+int inc = qdict_get_try_bool(qdict, inc, 0);
 const char *uri = qdict_get_str(qdict, uri);
 
 if (current_migration 
@@ -86,21 +88,17 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 
 if (strstart(uri, tcp:, p)) {
 s = tcp_start_outgoing_migration(mon, p, max_throttle, detach,
- (int)qdict_get_int(qdict, blk), 
- (int)qdict_get_int(qdict, inc));
+ blk, inc);
 #if !defined(WIN32)
 } else if (strstart(uri, exec:, p)) {
 s = exec_start_outgoing_migration(mon, p, max_throttle, detach,
-  (int)qdict_get_int(qdict, blk), 
-  (int)qdict_get_int(qdict, inc));
+  blk, inc);
 } else if (strstart(uri, unix:, p)) {
 s = unix_start_outgoing_migration(mon, p, max_throttle, detach,
- (int)qdict_get_int(qdict, blk), 
-  (int)qdict_get_int(qdict, inc));
+  blk, inc);
 } else if (strstart(uri, fd:, p)) {
 s = fd_start_outgoing_migration(mon, p, max_throttle, detach, 
-(int)qdict_get_int(qdict, blk), 
-(int)qdict_get_int(qdict, inc));
+blk, inc);
 #endif
 } else {
 monitor_printf(mon, unknown migration protocol: %s\n, uri);
diff --git a/monitor.c b/monitor.c
index 58b060b..568bfb2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3565,7 +3565,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 case '-':
 {
 const char *tmp = p;
-int has_option, skip_key = 0;
+int skip_key = 0;
 /* option */
 
 c = *typestr++;
@@ -3573,7 +3573,6 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 goto bad_type;
 while (qemu_isspace(*p))
 p++;
-has_option = 0;
 if (*p == '-') {
 p++;
 if(c != *p) {
@@ -3589,11 +3588,11 @@ static const mon_cmd_t 

[Qemu-devel] [PATCH 14/23] QDict: Introduce qdict_get_try_bool()

2010-07-01 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 qdict.c |   18 ++
 qdict.h |1 +
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/qdict.c b/qdict.c
index a28a0a9..dee0fb4 100644
--- a/qdict.c
+++ b/qdict.c
@@ -308,6 +308,24 @@ int64_t qdict_get_try_int(const QDict *qdict, const char 
*key,
 }
 
 /**
+ * qdict_get_try_bool(): Try to get a bool mapped by 'key'
+ *
+ * Return bool mapped by 'key', if it is not present in the
+ * dictionary or if the stored object is not of QBool type
+ * 'def_value' will be returned.
+ */
+int qdict_get_try_bool(const QDict *qdict, const char *key, int def_value)
+{
+QObject *obj;
+
+obj = qdict_get(qdict, key);
+if (!obj || qobject_type(obj) != QTYPE_QBOOL)
+return def_value;
+
+return qbool_get_int(qobject_to_qbool(obj));
+}
+
+/**
  * qdict_get_try_str(): Try to get a pointer to the stored string
  * mapped by 'key'
  *
diff --git a/qdict.h b/qdict.h
index 0e7a43f..929d8d2 100644
--- a/qdict.h
+++ b/qdict.h
@@ -61,6 +61,7 @@ QDict *qdict_get_qdict(const QDict *qdict, const char *key);
 const char *qdict_get_str(const QDict *qdict, const char *key);
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
   int64_t def_value);
+int qdict_get_try_bool(const QDict *qdict, const char *key, int def_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
 #endif /* QDICT_H */
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 17/23] QMP: New argument checker (second part)

2010-07-01 Thread Luiz Capitulino
This commit introduces the second (and last) part of QMP's new
argument checker.

The job is done by check_client_args_type(), it iterates over
the client's argument qdict and for for each argument it checks
if it exists and if its type is valid.

It's important to observe the following changes from the existing
argument checker:

  - If the handler accepts an O-type argument, unknown arguments
are passed down to it. It's up to O-type handlers to validate
their arguments

  - Boolean types (eg. 'b' and '-') don't accept integers anymore,
only json-bool

  - Argument types '/' and '.' are currently unsupported under QMP,
thus they're not handled

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |   94 -
 1 files changed, 93 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 222622b..2fd8005 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4151,6 +4151,95 @@ static int invalid_qmp_mode(const Monitor *mon, const 
char *cmd_name)
 }
 
 /*
+ * Argument validation rules:
+ *
+ * 1. The argument must exist in cmd_args qdict
+ * 2. The argument type must be the expected one
+ *
+ * Special case: If the argument doesn't exist in cmd_args and
+ *   the QMP_ACCEPT_UNKNOWNS flag is set, then the
+ *   checking is skipped for it.
+ */
+static int check_client_args_type(const QDict *client_args,
+  const QDict *cmd_args, int flags)
+{
+const QDictEntry *ent;
+
+for (ent = qdict_first(client_args); ent;ent = 
qdict_next(client_args,ent)){
+QObject *obj;
+QString *arg_type;
+const QObject *client_arg = qdict_entry_value(ent);
+const char *client_arg_name = qdict_entry_key(ent);
+
+obj = qdict_get(cmd_args, client_arg_name);
+if (!obj) {
+if (flags  QMP_ACCEPT_UNKNOWNS) {
+/* handler accepts unknowns */
+continue;
+}
+/* client arg doesn't exist */
+qerror_report(QERR_INVALID_PARAMETER, client_arg_name);
+return -1;
+}
+
+arg_type = qobject_to_qstring(obj);
+assert(arg_type != NULL);
+
+/* check if argument's type is correct */
+switch (qstring_get_str(arg_type)[0]) {
+case 'F':
+case 'B':
+case 's':
+if (qobject_type(client_arg) != QTYPE_QSTRING) {
+qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+  string);
+return -1;
+}
+break;
+case 'i':
+case 'l':
+case 'M':
+if (qobject_type(client_arg) != QTYPE_QINT) {
+qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+  int);
+return -1; 
+}
+break;
+case 'f':
+case 'T':
+if (qobject_type(client_arg) != QTYPE_QINT 
+qobject_type(client_arg) != QTYPE_QFLOAT) {
+qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+  number);
+   return -1; 
+}
+break;
+case 'b':
+case '-':
+if (qobject_type(client_arg) != QTYPE_QBOOL) {
+qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+  bool);
+   return -1; 
+}
+break;
+case 'O':
+assert(flags  QMP_ACCEPT_UNKNOWNS);
+break;
+case '/':
+case '.':
+/*
+ * These types are not supported by QMP and thus are not
+ * handled here. Fall through.
+ */
+default:
+abort();
+}
+}
+
+return 0;
+}
+
+/*
  * - Check if the client has passed all mandatory args
  * - Set special flags for argument validation
  */
@@ -4227,6 +4316,9 @@ out:
  * Client argument checking rules:
  *
  * 1. Client must provide all mandatory arguments
+ * 2. Each argument provided by the client must be expected
+ * 3. Each argument provided by the client must have the type expected
+ *by the command
  */
 static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
 {
@@ -4241,7 +4333,7 @@ static int qmp_check_client_args(const mon_cmd_t *cmd, 
QDict *client_args)
 goto out;
 }
 
-/* TODO: Check client args type */
+err = check_client_args_type(client_args, cmd_args, flags);
 
 out:
 QDECREF(cmd_args);
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 19/23] QError: Introduce QERR_QMP_EXTRA_MEMBER

2010-07-01 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 qerror.c |4 
 qerror.h |3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index cce1e7b..2f6f590 100644
--- a/qerror.c
+++ b/qerror.c
@@ -177,6 +177,10 @@ static const QErrorStringTable qerror_table[] = {
 .desc  = QMP input object member '%(member)' expects 
'%(expected)',
 },
 {
+.error_fmt = QERR_QMP_EXTRA_MEMBER,
+.desc  = QMP input object member '%(member)' is unexpected,
+},
+{
 .error_fmt = QERR_SET_PASSWD_FAILED,
 .desc  = Could not set password,
 },
diff --git a/qerror.h b/qerror.h
index 77ae574..9ad00b4 100644
--- a/qerror.h
+++ b/qerror.h
@@ -148,6 +148,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
 { 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': 
%s } }
 
+#define QERR_QMP_EXTRA_MEMBER \
+{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }
+
 #define QERR_SET_PASSWD_FAILED \
 { 'class': 'SetPasswdFailed', 'data': {} }
 
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 18/23] QMP: Drop old client argument checker

2010-07-01 Thread Luiz Capitulino
Previous two commits added qmp_check_client_args(), which
fully replaces this code and is way better.

It's important to note that the new checker doesn't support
the '/' arg type. As we don't have any of those handlers
converted to QMP, this is just dead code.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |  176 -
 1 files changed, 0 insertions(+), 176 deletions(-)

diff --git a/monitor.c b/monitor.c
index 2fd8005..172dc21 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3973,177 +3973,6 @@ static int monitor_can_read(void *opaque)
 return (mon-suspend_cnt == 0) ? 1 : 0;
 }
 
-typedef struct CmdArgs {
-QString *name;
-int type;
-int flag;
-int optional;
-} CmdArgs;
-
-static int check_opt(const CmdArgs *cmd_args, const char *name, QDict *args)
-{
-if (!cmd_args-optional) {
-qerror_report(QERR_MISSING_PARAMETER, name);
-return -1;
-}
-
-return 0;
-}
-
-static int check_arg(const CmdArgs *cmd_args, QDict *args)
-{
-QObject *value;
-const char *name;
-
-name = qstring_get_str(cmd_args-name);
-
-if (!args) {
-return check_opt(cmd_args, name, args);
-}
-
-value = qdict_get(args, name);
-if (!value) {
-return check_opt(cmd_args, name, args);
-}
-
-switch (cmd_args-type) {
-case 'F':
-case 'B':
-case 's':
-if (qobject_type(value) != QTYPE_QSTRING) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, string);
-return -1;
-}
-break;
-case '/': {
-int i;
-const char *keys[] = { count, format, size, NULL };
-
-for (i = 0; keys[i]; i++) {
-QObject *obj = qdict_get(args, keys[i]);
-if (!obj) {
-qerror_report(QERR_MISSING_PARAMETER, name);
-return -1;
-}
-if (qobject_type(obj) != QTYPE_QINT) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, int);
-return -1;
-}
-}
-break;
-}
-case 'i':
-case 'l':
-case 'M':
-if (qobject_type(value) != QTYPE_QINT) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, int);
-return -1;
-}
-break;
-case 'f':
-case 'T':
-if (qobject_type(value) != QTYPE_QINT  qobject_type(value) != 
QTYPE_QFLOAT) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, number);
-return -1;
-}
-break;
-case 'b':
-if (qobject_type(value) != QTYPE_QBOOL) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, bool);
-return -1;
-}
-break;
-case '-':
-if (qobject_type(value) != QTYPE_QINT 
-qobject_type(value) != QTYPE_QBOOL) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, bool);
-return -1;
-}
-break;
-case 'O':
-default:
-/* impossible */
-abort();
-}
-
-return 0;
-}
-
-static void cmd_args_init(CmdArgs *cmd_args)
-{
-cmd_args-name = qstring_new();
-cmd_args-type = cmd_args-flag = cmd_args-optional = 0;
-}
-
-static int check_opts(QemuOptsList *opts_list, QDict *args)
-{
-assert(!opts_list-desc-name);
-return 0;
-}
-
-/*
- * This is not trivial, we have to parse Monitor command's argument
- * type syntax to be able to check the arguments provided by clients.
- *
- * In the near future we will be using an array for that and will be
- * able to drop all this parsing...
- */
-static int monitor_check_qmp_args(const mon_cmd_t *cmd, QDict *args)
-{
-int err;
-const char *p;
-CmdArgs cmd_args;
-QemuOptsList *opts_list;
-
-if (cmd-args_type == NULL) {
-return (qdict_size(args) == 0 ? 0 : -1);
-}
-
-err = 0;
-cmd_args_init(cmd_args);
-opts_list = NULL;
-
-for (p = cmd-args_type;; p++) {
-if (*p == ':') {
-cmd_args.type = *++p;
-p++;
-if (cmd_args.type == '-') {
-cmd_args.flag = *p++;
-cmd_args.optional = 1;
-} else if (cmd_args.type == 'O') {
-opts_list = qemu_find_opts(qstring_get_str(cmd_args.name));
-assert(opts_list);
-} else if (*p == '?') {
-cmd_args.optional = 1;
-p++;
-}
-
-assert(*p == ',' || *p == '\0');
-if (opts_list) {
-err = check_opts(opts_list, args);
-opts_list = NULL;
-} else {
-err = check_arg(cmd_args, args);
-QDECREF(cmd_args.name);
-cmd_args_init(cmd_args);
-

[Qemu-devel] Re: Status update

2010-07-01 Thread Eduard - Gabriel Munteanu
On Wed, Jun 30, 2010 at 09:37:31AM +0100, Stefan Hajnoczi wrote:
 On Tue, Jun 29, 2010 at 6:25 PM, Eduard - Gabriel Munteanu
 eduard.munte...@linux360.ro wrote:
  On the other hand, we could just leave it alone for now. Changing
  mappings during DMA is stupid anyway: I don't think the guest can
  recover the results of DMA safely, even though it might be used on
  transfers in progress you simply don't care about anymore. Paul Brook
  suggested we could update the cpu_physical_memory_map() mappings
  somehow, but I think that's kinda difficult to accomplish.
 
 A malicious or broken guest shouldn't be able to crash or corrupt QEMU
 process memory.  The IOMMU can only map from bus addresses to guest
 physical RAM (?) so the worst the guest can do here is corrupt itself?
 
 Stefan

That's true, but it's fair to be concerned about the guest itself.
Imagine it runs some possibly malicious apps which program the hardware
to do DMA. That should be safe when a IOMMU is present.

But suddenly the guest OS changes mappings and expects the IOMMU to
enforce them as soon as invalidation commands are completed. The guest
then reclaims the old space for other uses. This leaves an opportunity
for those processes to corrupt or read sensitive data.

If the guest OS is prone to changing mappings during DMA, some process
could continually set up, e.g., IDE DMA write transfers hoping to expose
useful data it can't read otherwise. The buffer can be poisoned to see
if someone went for the bait and wrote in that space.

Actually I'm not that sure changing mappings during DMA is stupid,
as the OS might want to reassign devices (where this is possible) to
various processes. Reclaiming mappings seems normal when a processes
dies during DMA, as the kernel has no way of telling whether DMA
completed (or even started).


Eduard




[Qemu-devel] [PATCH 23/23] monitor: Allow to exclude commands from QMP

2010-07-01 Thread Luiz Capitulino
From: Jan Kiszka jan.kis...@siemens.com

Ported commands that are marked 'user_only' will not be considered for
QMP monitor sessions. This allows to implement new commands that do not
(yet) provide a sufficiently stable interface for QMP use.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |   18 +++---
 monitor.h |1 +
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index 00a627a..f319fee 100644
--- a/monitor.c
+++ b/monitor.c
@@ -333,6 +333,11 @@ static inline bool monitor_handler_is_async(const 
mon_cmd_t *cmd)
 return cmd-flags  MONITOR_CMD_ASYNC;
 }
 
+static inline bool monitor_cmd_user_only(const mon_cmd_t *cmd)
+{
+return (cmd-flags  MONITOR_CMD_USER_ONLY);
+}
+
 static inline int monitor_has_error(const Monitor *mon)
 {
 return mon-error != NULL;
@@ -615,6 +620,11 @@ static int do_info(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 goto help;
 }
 
+if (monitor_ctrl_mode(mon)  monitor_cmd_user_only(cmd)) {
+qerror_report(QERR_COMMAND_NOT_FOUND, item);
+return -1;
+}
+
 if (monitor_handler_is_async(cmd)) {
 if (monitor_ctrl_mode(mon)) {
 qmp_async_info_handler(mon, cmd);
@@ -712,13 +722,14 @@ static void do_info_commands(Monitor *mon, QObject 
**ret_data)
 cmd_list = qlist_new();
 
 for (cmd = mon_cmds; cmd-name != NULL; cmd++) {
-if (monitor_handler_ported(cmd)  !compare_cmd(cmd-name, info)) {
+if (monitor_handler_ported(cmd)  !monitor_cmd_user_only(cmd) 
+!compare_cmd(cmd-name, info)) {
 qlist_append_obj(cmd_list, get_cmd_dict(cmd-name));
 }
 }
 
 for (cmd = info_cmds; cmd-name != NULL; cmd++) {
-if (monitor_handler_ported(cmd)) {
+if (monitor_handler_ported(cmd)  !monitor_cmd_user_only(cmd)) {
 char buf[128];
 snprintf(buf, sizeof(buf), query-%s, cmd-name);
 qlist_append_obj(cmd_list, get_cmd_dict(buf));
@@ -4271,7 +4282,8 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
   qobject_from_jsonf({ 'item': %s }, info_item));
 } else {
 cmd = monitor_find_command(cmd_name);
-if (!cmd || !monitor_handler_ported(cmd)) {
+if (!cmd || !monitor_handler_ported(cmd)
+|| monitor_cmd_user_only(cmd)) {
 qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
 goto err_out;
 }
diff --git a/monitor.h b/monitor.h
index 9582b9c..38b22a4 100644
--- a/monitor.h
+++ b/monitor.h
@@ -17,6 +17,7 @@ extern Monitor *default_mon;
 
 /* flags for monitor commands */
 #define MONITOR_CMD_ASYNC   0x0001
+#define MONITOR_CMD_USER_ONLY   0x0002
 
 /* QMP events */
 typedef enum MonitorEvent {
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 20/23] QMP: Introduce qmp_check_input_obj()

2010-07-01 Thread Luiz Capitulino
This is similar to qmp_check_client_args(), but it checks if
the input object follows the specification (QMP/qmp-spec.txt
section 2.3).

As we're limited to three keys, the work here is quite simple:
we iterate over the input object, checking each time if the
current argument complies to the specification.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |   62 -
 1 files changed, 61 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 172dc21..22e0650 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4169,6 +4169,62 @@ out:
 return err;
 }
 
+/*
+ * Input object checking rules
+ *
+ * 1. Input object must be a dict
+ * 2. The execute key must exist
+ * 3. The execute key must be a string
+ * 4. If the arguments key exists, it must be a dict
+ * 5. If the id key exists, it can be anything (ie. json-value)
+ * 6. Any argument not listed above is considered invalid
+ */
+static QDict *qmp_check_input_obj(QObject *input_obj)
+{
+const QDictEntry *ent;
+int has_exec_key = 0;
+QDict *input_dict;
+
+if (qobject_type(input_obj) != QTYPE_QDICT) {
+qerror_report(QERR_QMP_BAD_INPUT_OBJECT, object);
+return NULL;
+}
+
+input_dict = qobject_to_qdict(input_obj);
+
+for (ent = qdict_first(input_dict); ent; ent = qdict_next(input_dict, 
ent)){
+const char *arg_name = qdict_entry_key(ent);
+const QObject *arg_obj = qdict_entry_value(ent);
+
+if (!strcmp(arg_name, execute)) {
+if (qobject_type(arg_obj) != QTYPE_QSTRING) {
+qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, execute,
+  string);
+return NULL;
+}
+has_exec_key = 1;
+} else if (!strcmp(arg_name, arguments)) {
+if (qobject_type(arg_obj) != QTYPE_QDICT) {
+qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, arguments,
+  object);
+return NULL;
+}
+} else if (!strcmp(arg_name, id)) {
+/* FIXME: check duplicated IDs for async commands */
+} else {
+qerror_report(QERR_QMP_EXTRA_MEMBER, arg_name);
+return NULL;
+}
+}
+
+if (!has_exec_key) {
+qerror_report(QERR_QMP_BAD_INPUT_OBJECT, execute);
+return NULL;
+}
+
+return input_dict;
+}
+
 static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 {
 int err;
@@ -4191,7 +4247,11 @@ static void handle_qmp_command(JSONMessageParser 
*parser, QList *tokens)
 goto err_out;
 }
 
-input = qobject_to_qdict(obj);
+input = qmp_check_input_obj(obj);
+if (!input) {
+qobject_decref(obj);
+goto err_out;
+}
 
 mon-mc-id = qdict_get(input, id);
 qobject_incref(mon-mc-id);
-- 
1.7.2.rc0




[Qemu-devel] [PATCH 21/23] QMP: Drop old input object checking

2010-07-01 Thread Luiz Capitulino
Previous commit added qmp_check_input_obj(), it does all the
checking we need.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |   19 +--
 1 files changed, 1 insertions(+), 18 deletions(-)

diff --git a/monitor.c b/monitor.c
index 22e0650..1c8992b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4241,10 +4241,6 @@ static void handle_qmp_command(JSONMessageParser 
*parser, QList *tokens)
 // FIXME: should be triggered in json_parser_parse()
 qerror_report(QERR_JSON_PARSING);
 goto err_out;
-} else if (qobject_type(obj) != QTYPE_QDICT) {
-qerror_report(QERR_QMP_BAD_INPUT_OBJECT, object);
-qobject_decref(obj);
-goto err_out;
 }
 
 input = qmp_check_input_obj(obj);
@@ -4256,17 +4252,7 @@ static void handle_qmp_command(JSONMessageParser 
*parser, QList *tokens)
 mon-mc-id = qdict_get(input, id);
 qobject_incref(mon-mc-id);
 
-obj = qdict_get(input, execute);
-if (!obj) {
-qerror_report(QERR_QMP_BAD_INPUT_OBJECT, execute);
-goto err_input;
-} else if (qobject_type(obj) != QTYPE_QSTRING) {
-qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, execute, string);
-goto err_input;
-}
-
-cmd_name = qstring_get_str(qobject_to_qstring(obj));
-
+cmd_name = qdict_get_str(input, execute);
 if (invalid_qmp_mode(mon, cmd_name)) {
 qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
 goto err_input;
@@ -4294,9 +4280,6 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
 obj = qdict_get(input, arguments);
 if (!obj) {
 args = qdict_new();
-} else if (qobject_type(obj) != QTYPE_QDICT) {
-qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, arguments, object);
-goto err_input;
 } else {
 args = qobject_to_qdict(obj);
 QINCREF(args);
-- 
1.7.2.rc0




Re: [Qemu-devel] [PATCH] Makefile: add fsdev/*.{o,d} to clean

2010-07-01 Thread Blue Swirl
2010/7/1 Hidetoshi Seto seto.hideto...@jp.fujitsu.com:
 There were fsdev/qemu-fsdev.{o,d} not removed at make clean.

 Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
 ---
  Makefile |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/Makefile b/Makefile
 index 560eac6..ce5f0e6 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -159,7 +159,7 @@ clean:
  # avoid old build problems by removing potentially incorrect old files
        rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
 gen-op-arm.h
        rm -f *.o *.d *.a $(TOOLS) TAGS cscope.* *.pod *~ */*~
 -       rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d 
 net/*.o net/*.d
 +       rm -f {slirp,audio,block,net,fsdev}/*.{o,d}

Not every shell out there support {}:
dash -c 'echo {a,b}*.c'
{a,b}*.c

bash -c 'echo {a,b}*.c'
acl.c aes.c aio.c alpha-dis.c arch_init.c arm-dis.c arm-semi.c async.c
balloon.c block-migration.c block.c blockdev.c bt-host.c bt-vhci.c
buffered_file.c



[Qemu-devel] [PATCH 22/23] QMP: handle_qmp_command(): Small cleanup

2010-07-01 Thread Luiz Capitulino
Drop a unneeded label and QDECREF() call.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |   14 ++
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/monitor.c b/monitor.c
index 1c8992b..00a627a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4234,7 +4234,7 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
 Monitor *mon = cur_mon;
 const char *cmd_name, *info_item;
 
-args = NULL;
+args = input = NULL;
 
 obj = json_parser_parse(tokens, NULL);
 if (!obj) {
@@ -4255,7 +4255,7 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
 cmd_name = qdict_get_str(input, execute);
 if (invalid_qmp_mode(mon, cmd_name)) {
 qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
-goto err_input;
+goto err_out;
 }
 
 /*
@@ -4264,7 +4264,7 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
  */
 if (compare_cmd(cmd_name, info)) {
 qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
-goto err_input;
+goto err_out;
 } else if (strstart(cmd_name, query-, info_item)) {
 cmd = monitor_find_command(info);
 qdict_put_obj(input, arguments,
@@ -4273,7 +4273,7 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
 cmd = monitor_find_command(cmd_name);
 if (!cmd || !monitor_handler_ported(cmd)) {
 qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
-goto err_input;
+goto err_out;
 }
 }
 
@@ -4285,8 +4285,6 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
 QINCREF(args);
 }
 
-QDECREF(input);
-
 err = qmp_check_client_args(cmd, args);
 if (err  0) {
 goto err_out;
@@ -4301,13 +4299,13 @@ static void handle_qmp_command(JSONMessageParser 
*parser, QList *tokens)
 } else {
 monitor_call_handler(mon, cmd, args);
 }
+
 goto out;
 
-err_input:
-QDECREF(input);
 err_out:
 monitor_protocol_emitter(mon, NULL);
 out:
+QDECREF(input);
 QDECREF(args);
 }
 
-- 
1.7.2.rc0




[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

2010-07-01 Thread Blue Swirl
On Wed, Jun 30, 2010 at 8:56 AM, Paolo Bonzini pbonz...@redhat.com wrote:
 Wouldn't it be better to just put this in dyngen-exec.h ?
 AFAICT there's a direct correlation between NEED_GLOBAL_ENV and #include
 exec.h.

 True, see cover letter in 0/4.  I was told to make each file request
 explicitly the global variable though.  So I'd have to leave the #ifdef even
 if I moved it into dyngen-exec.h.

 Well, I only said I'd rename global 'env' to 'global_reg_env', not
 something about each file requesting it. But NEED_GLOBAL_ENV wasn't so
 bad idea in my opinion.

 It doesn't matter what's the name of the global.

Yes it does. Unfortunately, 'env' is a name which is used for other
purposes than the global register variable pointing to CPUState. Any
of these uses could accidentally refer to global 'env'. Also one bug
was where the function had parameter 'env1', but some code inside
still used 'env'. These kind of bugs can be avoided by simply renaming
'env' to less collision prone name.

  What matters is
 whether it's defined at all.  For this reason it's bad to bury it
 in dyngen-exec.h which is included only indirectly.  It's better to
 leave it in all */exec.h files as Paul explained---and I agree with
 him.

Fine by me too.

 I also gave reason why unpoisoning env globally is not a problem at
 all.  For target-dependent files, they did not (and do not) poison
 anything, so my first patch series didn't change anything WRT current
 QEMU sources.  exec.h always includes cpu.h, so there's no way exec.h
 can be included by mistake in a target-independent file.  I can make
 exec.h error out if NEED_CPU_H is not defined, but I think it's a
 worthless complication.

I still maintain that 'env' may not be unpoisoned until the name is
less likely to invite accidents.


 So, can someone please apply patches 1 to 3 of this series so that
 we can move on?

I think they are nice cleanups regardless of the approach for 'env'.
If nobody has any objections, I'll apply them on Saturday.



Re: [Qemu-devel] [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)

2010-07-01 Thread Blue Swirl
On Thu, Jul 1, 2010 at 11:08 AM, Stefan Weil w...@mail.berlios.de wrote:
 Signed-off-by: Stefan Weil w...@mail.berlios.de
 ---
  blockdev.h |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

 diff --git a/blockdev.h b/blockdev.h
 index 23ea576..3c5c85d 100644
 --- a/blockdev.h
 +++ b/blockdev.h
 @@ -42,7 +42,8 @@ extern int drive_get_max_bus(BlockInterfaceType type);
  extern void drive_uninit(DriveInfo *dinfo);
  extern const char *drive_get_serial(BlockDriverState *bdrv);

 -extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
 +extern QemuOpts *drive_add(const char *file, const char *fmt, ...)
 +    __attribute__ ((__format__ (__printf__, 2, 3)));
  extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
                              int *fatal_error);

I lost the cover letter, so this applies to all patches: Wouldn't it
make sense to make GCC_FMT_ATTR(n, m) from audio/audio_int.h available
universally and then use that?



Re: [Qemu-devel] [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)

2010-07-01 Thread Stefan Weil

Am 01.07.2010 22:10, schrieb Blue Swirl:

On Thu, Jul 1, 2010 at 11:08 AM, Stefan Weil w...@mail.berlios.de wrote:

Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 blockdev.h |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/blockdev.h b/blockdev.h
index 23ea576..3c5c85d 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -42,7 +42,8 @@ extern int drive_get_max_bus(BlockInterfaceType type);
 extern void drive_uninit(DriveInfo *dinfo);
 extern const char *drive_get_serial(BlockDriverState *bdrv);

-extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
+extern QemuOpts *drive_add(const char *file, const char *fmt, ...)
+__attribute__ ((__format__ (__printf__, 2, 3)));
 extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
 int *fatal_error);


I lost the cover letter, so this applies to all patches: Wouldn't it
make sense to make GCC_FMT_ATTR(n, m) from audio/audio_int.h available
universally and then use that?



That's a matter of personal taste:

GCC_FMT_ATTR(n, m) is shorter, but a human reader has to
look it up once to see what it does (ok, some readers might
guess it right). The compiler has to look it up, too, so
a common header file is needed.

When I prepared the patches, I did not notice that some
functions used GCC_FMT_ATTR. I added the __attribute__
macro to these functions and got a compiler error...
This shows that at least for me GCC_FMT_ATTR was confusing
(of course it no longer is).

Some people prefer GCC_FMT_ATTR because they want to
be able to redefine it for non-gcc compilers.

__attribute__ can also be redefined for that case, so that
is not a very strong argument.

I prefer using __attribute__ without intermediate macro,
but don't mind if a different style is preferred for qemu.

Regards
Stefan




[Qemu-devel] [PATCH] ARM v4t/arm920t support

2010-07-01 Thread Rob Landley
I just confirmed that Vincent Sanders' patch (which he posted on May 29, 2009,
and again on November 27, 2009) still applies to (and works with )current
qemu-git.

It adds a -cpu arm920t option to qemu-system-arm which boots a Linux kernel
configured with CONFIG_CPU_ARM920T=y, which isn't possible without this patch.

Here is the patch again.  There may be more work to be done on top of this,
but this patch staying out of tree hasn't noticeably accelerated that work in
the past year and change.  Could it please be merged?

Thanks,

Rob

From 5a242e7cdc76c654d599b482cc85ec5bd85e36a3 Mon Sep 17 00:00:00 2001
From: Vincent Sanders vi...@kyllikki.org
Date: Fri, 29 May 2009 13:41:16 +0100
Subject: [PATCH] Update ARM emulation to include version 4t.

Update ARM emulation to be version 4t by default and add v5 as a
feature. Implementation is very similar to the way the v6 features are
presented.

The affected instructions and program counter load behaviour are made
CPU version dependant and the ARM920T cpu id is introduced.

Signed-off-by: Vincent Sanders vi...@simtec.co.uk
---
 target-arm/cpu.h   |2 ++
 target-arm/helper.c|   16 
 target-arm/translate.c |   22 +++---
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 4a1c53f..6b9a64f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -334,6 +334,7 @@ enum arm_features {
 ARM_FEATURE_AUXCR,  /* ARM1026 Auxiliary control register.  */
 ARM_FEATURE_XSCALE, /* Intel XScale extensions.  */
 ARM_FEATURE_IWMMXT, /* Intel iwMMXt extension.  */
+ARM_FEATURE_V5,
 ARM_FEATURE_V6,
 ARM_FEATURE_V6K,
 ARM_FEATURE_V7,
@@ -374,6 +375,7 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define ARM_CPUID_ARM1026 0x4106a262
 #define ARM_CPUID_ARM926  0x41069265
 #define ARM_CPUID_ARM946  0x41059461
+#define ARM_CPUID_ARM920T 0x41129200
 #define ARM_CPUID_TI915T  0x54029152
 #define ARM_CPUID_TI925T  0x54029252
 #define ARM_CPUID_PXA250  0x69052100
diff --git a/target-arm/helper.c b/target-arm/helper.c
index b3aec99..426c544 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -44,18 +44,25 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
id)
 {
 env-cp15.c0_cpuid = id;
 switch (id) {
+case ARM_CPUID_ARM920T:
+env-cp15.c0_cachetype = 0x0d172172;
+env-cp15.c1_sys = 0x0078;
+break;
 case ARM_CPUID_ARM926:
+set_feature(env, ARM_FEATURE_V5);
 set_feature(env, ARM_FEATURE_VFP);
 env-vfp.xregs[ARM_VFP_FPSID] = 0x41011090;
 env-cp15.c0_cachetype = 0x1dd20d2;
 env-cp15.c1_sys = 0x00090078;
 break;
 case ARM_CPUID_ARM946:
+set_feature(env, ARM_FEATURE_V5);
 set_feature(env, ARM_FEATURE_MPU);
 env-cp15.c0_cachetype = 0x0f004006;
 env-cp15.c1_sys = 0x0078;
 break;
 case ARM_CPUID_ARM1026:
+set_feature(env, ARM_FEATURE_V5);
 set_feature(env, ARM_FEATURE_VFP);
 set_feature(env, ARM_FEATURE_AUXCR);
 env-vfp.xregs[ARM_VFP_FPSID] = 0x410110a0;
@@ -64,6 +71,7 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 break;
 case ARM_CPUID_ARM1136_R2:
 case ARM_CPUID_ARM1136:
+set_feature(env, ARM_FEATURE_V5);
 set_feature(env, ARM_FEATURE_V6);
 set_feature(env, ARM_FEATURE_VFP);
 set_feature(env, ARM_FEATURE_AUXCR);
@@ -75,6 +83,7 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 env-cp15.c0_cachetype = 0x1dd20d2;
 break;
 case ARM_CPUID_ARM11MPCORE:
+set_feature(env, ARM_FEATURE_V5);
 set_feature(env, ARM_FEATURE_V6);
 set_feature(env, ARM_FEATURE_V6K);
 set_feature(env, ARM_FEATURE_VFP);
@@ -87,6 +96,7 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 env-cp15.c0_cachetype = 0x1dd20d2;
 break;
 case ARM_CPUID_CORTEXA8:
+set_feature(env, ARM_FEATURE_V5);
 set_feature(env, ARM_FEATURE_V6);
 set_feature(env, ARM_FEATURE_V6K);
 set_feature(env, ARM_FEATURE_V7);
@@ -129,6 +139,7 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
id)
 env-cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
 break;
 case ARM_CPUID_CORTEXM3:
+set_feature(env, ARM_FEATURE_V5);
 set_feature(env, ARM_FEATURE_V6);
 set_feature(env, ARM_FEATURE_THUMB2);
 set_feature(env, ARM_FEATURE_V7);
@@ -136,6 +147,7 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
id)
 set_feature(env, ARM_FEATURE_DIV);
 break;
 case ARM_CPUID_ANY: /* For userspace emulation.  */
+set_feature(env, ARM_FEATURE_V5);
 set_feature(env, ARM_FEATURE_V6);
 set_feature(env, ARM_FEATURE_V6K);
 set_feature(env, ARM_FEATURE_V7);
@@ -149,6 +161,7 @@ static void cpu_reset_model_id(CPUARMState 

Re: [Qemu-devel] Re: [Bug 599958] Re: Timedrift problems with Win7: hpet missing time drift fixups

2010-07-01 Thread Paul Brook
 I really see no tangible objection to Jan's patches.  They don't impact
 any other code.  They don't inhibit flexibility in the infrastructure.
 You might consider it to be a hack but so what.  QEMU is filled with
 hacks.  It would be useless without them because there would be very
 little code.

I object strongly to anything that makes qemu_irq a message passing API.
if you want message passing then you should not be using qemu_irq.

Paul



[Qemu-devel] Re: [PATCH] ARM v4t/arm920t support

2010-07-01 Thread Paul Brook
 Here is the patch again.  There may be more work to be done on top of this,
 but this patch staying out of tree hasn't noticeably accelerated that work
 in the past year and change.  Could it please be merged?

As mentioned previously, V5 should be split into its component parts.

Paul



Re: [Qemu-devel] [PATCH 0/0] fix ARM parallel instructions implementation bug

2010-07-01 Thread Aurelien Jarno
On Mon, Jun 28, 2010 at 11:54:03PM +0800, Chih-Min Chao wrote:
  The three patches focuse on Bugs 595906  Bug 591320. The first is related to
  Bug 595906 and the other solve Bug 591320.
 
  The series are also attached in the threads, listed below
  https://bugs.launchpad.net/qemu/+bug/595906
  https://bugs.launchpad.net/qemu/+bug/591320
 
  [PATCH 1/3] target-arm: fix addsub/subadd implementation
  [PATCH 2/3] target-arm : fix thumb2 parallel add/sub opcode decoding
  [PATCH 3/3] target-arm : fix parallel saturated subtraction implementation
 
 

Thanks, all applied.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] Makefile: add qemu-options.def to distclean

2010-07-01 Thread Aurelien Jarno
On Thu, Jul 01, 2010 at 12:32:32PM +0900, Hidetoshi Seto wrote:
 Remove generated qemu-options.def at make distclean.
 
 Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
 ---
  Makefile |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

Thanks, applied.

 diff --git a/Makefile b/Makefile
 index 221fbd8..560eac6 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -168,6 +168,7 @@ clean:
  
  distclean: clean
   rm -f config-host.mak config-host.h* config-host.ld $(DOCS) 
 qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi
 + rm -f qemu-options.def
   rm -f config-all-devices.mak
   rm -f roms/seabios/config.mak roms/vgabios/config.mak
   rm -f qemu-{doc,tech}.{info,aux,cp,dvi,fn,info,ky,log,pdf,pg,toc,tp,vr}
 -- 
 1.7.0
 
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] Add QMP/qmp-commands.txt to .gitignore

2010-07-01 Thread Aurelien Jarno
On Thu, Jul 01, 2010 at 12:30:23PM +0900, Hidetoshi Seto wrote:
 QMP/qmp-commands.txt is a generated file.
 
 Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
 ---
  .gitignore |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

Thanks, applied.
 
 diff --git a/.gitignore b/.gitignore
 index ce66ed5..a32b7c4 100644
 --- a/.gitignore
 +++ b/.gitignore
 @@ -28,6 +28,7 @@ qemu-img-cmds.texi
  qemu-img-cmds.h
  qemu-io
  qemu-monitor.texi
 +QMP/qmp-commands.txt
  .gdbinit
  *.a
  *.aux
 -- 
 1.7.0
 
 
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] target-i386: Fix xchg rax,r8

2010-07-01 Thread Aurelien Jarno
On Thu, Jul 01, 2010 at 09:42:21AM -0700, Richard Henderson wrote:
 We were ignoring REX_B while special-casing NOP, i.e. xchg eax,eax.
 
 Signed-off-by: Richard Henderson r...@twiddle.net
 ---
  target-i386/translate.c |9 +++--
  1 files changed, 7 insertions(+), 2 deletions(-)

Applied, thanks.

 diff --git a/target-i386/translate.c b/target-i386/translate.c
 index 708b0a1..8cb5cf0 100644
 --- a/target-i386/translate.c
 +++ b/target-i386/translate.c
 @@ -5293,6 +5293,7 @@ static target_ulong disas_insn(DisasContext *s, 
 target_ulong pc_start)
  break;
  
  case 0x91 ... 0x97: /* xchg R, EAX */
 +do_xchg_reg_eax:
  ot = dflag + OT_WORD;
  reg = (b  7) | REX_B(s);
  rm = R_EAX;
 @@ -6663,10 +6664,14 @@ static target_ulong disas_insn(DisasContext *s, 
 target_ulong pc_start)
  //
  /* misc */
  case 0x90: /* nop */
 -/* XXX: xchg + rex handling */
  /* XXX: correct lock test for all insn */
 -if (prefixes  PREFIX_LOCK)
 +if (prefixes  PREFIX_LOCK) {
  goto illegal_op;
 +}
 +/* If REX_B is set, then this is xchg eax, r8d, not a nop.  */
 +if (REX_B(s)) {
 +goto do_xchg_reg_eax;
 +}
  if (prefixes  PREFIX_REPZ) {
  gen_svm_check_intercept(s, pc_start, SVM_EXIT_PAUSE);
  }
 -- 
 1.7.0.1
 
 
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH 1/4] blockdev.h: Add GCC attribute (check format arguments)

2010-07-01 Thread malc
On Thu, 1 Jul 2010, Stefan Weil wrote:

 Am 01.07.2010 22:10, schrieb Blue Swirl:
  On Thu, Jul 1, 2010 at 11:08 AM, Stefan Weil w...@mail.berlios.de wrote:
   Signed-off-by: Stefan Weil w...@mail.berlios.de
   ---
blockdev.h |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
   
   diff --git a/blockdev.h b/blockdev.h
   index 23ea576..3c5c85d 100644
   --- a/blockdev.h
   +++ b/blockdev.h
   @@ -42,7 +42,8 @@ extern int drive_get_max_bus(BlockInterfaceType type);
extern void drive_uninit(DriveInfo *dinfo);
extern const char *drive_get_serial(BlockDriverState *bdrv);
   
   -extern QemuOpts *drive_add(const char *file, const char *fmt, ...);
   +extern QemuOpts *drive_add(const char *file, const char *fmt, ...)
   +__attribute__ ((__format__ (__printf__, 2, 3)));
extern DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi,
int *fatal_error);
  
  I lost the cover letter, so this applies to all patches: Wouldn't it
  make sense to make GCC_FMT_ATTR(n, m) from audio/audio_int.h available
  universally and then use that?
 
 
 That's a matter of personal taste:
 
 GCC_FMT_ATTR(n, m) is shorter, but a human reader has to
 look it up once to see what it does (ok, some readers might
 guess it right). The compiler has to look it up, too, so
 a common header file is needed.
 
 When I prepared the patches, I did not notice that some
 functions used GCC_FMT_ATTR. I added the __attribute__
 macro to these functions and got a compiler error...
 This shows that at least for me GCC_FMT_ATTR was confusing
 (of course it no longer is).
 
 Some people prefer GCC_FMT_ATTR because they want to
 be able to redefine it for non-gcc compilers.
 
 __attribute__ can also be redefined for that case, so that
 is not a very strong argument.

Just to be pedantic, you can _not_ redfined __attribute the
name is reserved..

 
 I prefer using __attribute__ without intermediate macro,
 but don't mind if a different style is preferred for qemu.
 
 Regards
 Stefan
 
 

-- 
mailto:av1...@comtv.ru



[Qemu-devel] [PATCH] [RFC] target-mips: add loongson 2E 2F integer instructions

2010-07-01 Thread Aurelien Jarno
This patch adds support for loongson 2E  2F instructions. They are the
same instructions, but differ by the opcode encoding.

This patch has still a few problems (hence the RFC), but is enough to
boot a fulong 2E kernel built with the -mloongson2e flag:
- I am unable to understand the difference between DMULT.G and DMULTU.G
  instructions. As they are both a 64x64 = 64 multiplication, the
  signedness should not make any difference. They are implemented the
  same way in this patch.
- Division by 0 is not supposed to trigger any arithmetic exception,
  however the manual doesn't explain which result is return in such
  condition. With this patch, QEMU will crash with a SIGFPE in sich
  a situation.

Signed-off-by: Aurelien Jarno aurel...@aurel32.net
---
 target-mips/translate.c |  143 +++
 1 files changed, 143 insertions(+), 0 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 0ab23d3..91882b7 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -267,6 +267,19 @@ enum {
 OPC_MUL  = 0x02 | OPC_SPECIAL2,
 OPC_MSUB = 0x04 | OPC_SPECIAL2,
 OPC_MSUBU= 0x05 | OPC_SPECIAL2,
+/* Loongson 2F */
+OPC_MULT_G_2F   = 0x10 | OPC_SPECIAL2,
+OPC_DMULT_G_2F  = 0x11 | OPC_SPECIAL2,
+OPC_MULTU_G_2F  = 0x12 | OPC_SPECIAL2,
+OPC_DMULTU_G_2F = 0x13 | OPC_SPECIAL2,
+OPC_DIV_G_2F= 0x14 | OPC_SPECIAL2,
+OPC_DDIV_G_2F   = 0x15 | OPC_SPECIAL2,
+OPC_DIVU_G_2F   = 0x16 | OPC_SPECIAL2,
+OPC_DDIVU_G_2F  = 0x17 | OPC_SPECIAL2,
+OPC_MOD_G_2F= 0x1c | OPC_SPECIAL2,
+OPC_DMOD_G_2F   = 0x1d | OPC_SPECIAL2,
+OPC_MODU_G_2F   = 0x1e | OPC_SPECIAL2,
+OPC_DMODU_G_2F  = 0x1f | OPC_SPECIAL2,
 /* Misc */
 OPC_CLZ  = 0x20 | OPC_SPECIAL2,
 OPC_CLO  = 0x21 | OPC_SPECIAL2,
@@ -293,6 +306,20 @@ enum {
 OPC_BSHFL= 0x20 | OPC_SPECIAL3,
 OPC_DBSHFL   = 0x24 | OPC_SPECIAL3,
 OPC_RDHWR= 0x3B | OPC_SPECIAL3,
+
+/* Loongson 2E */
+OPC_MULT_G_2E   = 0x18 | OPC_SPECIAL3,
+OPC_MULTU_G_2E  = 0x19 | OPC_SPECIAL3,
+OPC_DIV_G_2E= 0x1A | OPC_SPECIAL3,
+OPC_DIVU_G_2E   = 0x1B | OPC_SPECIAL3,
+OPC_DMULT_G_2E  = 0x1C | OPC_SPECIAL3,
+OPC_DMULTU_G_2E = 0x1D | OPC_SPECIAL3,
+OPC_DDIV_G_2E   = 0x1E | OPC_SPECIAL3,
+OPC_DDIVU_G_2E  = 0x1F | OPC_SPECIAL3,
+OPC_MOD_G_2E= 0x22 | OPC_SPECIAL3,
+OPC_MODU_G_2E   = 0x23 | OPC_SPECIAL3,
+OPC_DMOD_G_2E   = 0x26 | OPC_SPECIAL3,
+OPC_DMODU_G_2E  = 0x27 | OPC_SPECIAL3,
 };
 
 /* BSHFL opcodes */
@@ -2325,6 +2352,110 @@ static void gen_cl (DisasContext *ctx, uint32_t opc,
 tcg_temp_free(t0);
 }
 
+#if defined(TARGET_MIPS64)
+/* Godson integer instructions */
+static void gen_loongson_integer (DisasContext *ctx, uint32_t opc,
+int rd, int rs, int rt)
+{
+const char *opn = loongson;
+TCGv t0, t1;
+
+if (rd == 0) {
+/* Treat as NOP. */
+MIPS_DEBUG(NOP);
+return;
+}
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+
+gen_load_gpr(t0, rs);
+gen_load_gpr(t1, rt);
+switch (opc) {
+case OPC_MULT_G_2E:
+case OPC_MULT_G_2F:
+tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
+tcg_gen_ext32s_tl(cpu_gpr[rd], cpu_gpr[rd]);
+opn = mult.g;
+break;
+case OPC_MULTU_G_2E:
+case OPC_MULTU_G_2F:
+tcg_gen_ext32u_tl(t0, t0);
+tcg_gen_ext32u_tl(t1, t1);
+tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
+tcg_gen_ext32s_tl(cpu_gpr[rd], cpu_gpr[rd]);
+opn = multu.g;
+break;
+case OPC_DMULT_G_2E:
+case OPC_DMULT_G_2F:
+tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
+opn = dmult.g;
+break;
+case OPC_DMULTU_G_2E:
+case OPC_DMULTU_G_2F:
+tcg_gen_mul_tl(cpu_gpr[rd], t0, t1);
+opn = dmultu.g;
+break;
+case OPC_DIV_G_2E:
+case OPC_DIV_G_2F:
+tcg_gen_ext32s_tl(t0, t0);
+tcg_gen_ext32s_tl(t1, t1);
+tcg_gen_div_tl(cpu_gpr[rd], t0, t1);
+tcg_gen_ext32s_tl(cpu_gpr[rd], cpu_gpr[rd]);
+opn = div.g;
+break;
+case OPC_DIVU_G_2E:
+case OPC_DIVU_G_2F:
+tcg_gen_ext32u_tl(t0, t0);
+tcg_gen_ext32u_tl(t1, t1);
+tcg_gen_divu_tl(cpu_gpr[rd], t0, t1);
+tcg_gen_ext32s_tl(cpu_gpr[rd], cpu_gpr[rd]);
+opn = divu.g;
+break;
+case OPC_DDIV_G_2E:
+case OPC_DDIV_G_2F:
+tcg_gen_div_tl(cpu_gpr[rd], t0, t1);
+opn = ddiv.g;
+break;
+case OPC_DDIVU_G_2E:
+case OPC_DDIVU_G_2F:
+tcg_gen_div_tl(cpu_gpr[rd], t0, t1);
+opn = ddivu.g;
+break;
+case OPC_MOD_G_2E:
+case OPC_MOD_G_2F:
+tcg_gen_ext32s_tl(t0, t0);
+tcg_gen_ext32s_tl(t1, t1);
+tcg_gen_rem_tl(cpu_gpr[rd], t0, t1);
+tcg_gen_ext32s_tl(cpu_gpr[rd], cpu_gpr[rd]);
+opn = mod.g;
+break;
+case OPC_MODU_G_2E:
+case OPC_MODU_G_2F:
+ 

  1   2   >