Re: [Qemu-devel] [PATCH] qom: fix refcounting in object_property_del_child()

2012-05-11 Thread Paolo Bonzini
Il 11/05/2012 04:15, Amos Kong ha scritto:
 Start VM with 8 multiple-function block devs, hot-removing
 those block devs by 'device_del ...' would cause qemu abort.
 
 object_ref() is called in object_property_add_child(),
 but we don't unref it in object_property_del_child().
 
 | (qemu) device_del virti0-0-0
 | (qemu) **
 | ERROR:qom/object.c:389:object_delete: assertion failed: (obj-ref == 0)
 
 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  qom/object.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/qom/object.c b/qom/object.c
 index e721fc2..9da6b59 100644
 --- a/qom/object.c
 +++ b/qom/object.c
 @@ -320,6 +320,7 @@ static void object_property_del_child(Object *obj, Object 
 *child, Error **errp)
  QTAILQ_FOREACH(prop, obj-properties, node) {
  if (strstart(prop-type, child, NULL)  prop-opaque == child) {
  object_property_del(obj, prop-name, errp);
 +object_unref(child);

This should be called by object_finalize_child_property instead, can you
check why this is not the case?

Paolo

  break;
  }
  }
 




Re: [Qemu-devel] [PATCH v12 0/4] support to migrate with IPv6 address

2012-05-11 Thread Amos Kong
On 05/11/2012 02:12 AM, Michael Roth wrote:
 On Thu, May 10, 2012 at 11:29:48AM -0600, Eric Blake wrote:
 On 05/10/2012 10:27 AM, Amos Kong wrote:
 Those patches updated help functions in qemu-socket.c,
 and used them in migrate-tcp.c to supporting IPv6 migration.


 ---

 Amos Kong (4):
   qerror: add five qerror strings
   sockets: change inet_connect() to support nonblock socket
   sockets: use error class to pass listen error
   use inet_listen()/inet_connect() to support ipv6 migration

 I'm trying to understand the scope of this series, and how it might
 impact what libvirt needs to support for migration in an IPv6
 environment.  Is the point of this patch that IPv6 migration was
 previously not possible, and is now possible using [IP6addr]:port notation?
 
 Looks like Amos went offline: but yes, in a nutshell.
 
 addr parsing now relies on qemu-sockets.c:inet_parse(), which has supported
 [ip6addr]:port for a while, as opposed to net.c:parse_host_port(), which
 didn't.

yeah.

I didn't change qemu monitor cmd interface in this patchset,
and the transport of data is done by qemu, not libvirt.

I guess libvirt only needs to update addr string parse,
for example:

 GOOD
start a VM:
# qemu-kvm --enable-kvm -boot n -incoming tcp:ipv6alias:16514 -vnc :1
-monitor stdio -name qemu-vm1

try to migrate vm by virsh with addr alias
# virsh migrate libivrt-vm2 tcp:ipv6alias
(connection can establish)

--- FAIL
start a VM:
# qemu-kvm-apply-my-patches --enable-kvm -boot n -incoming
tcp:[2002::3:4]:16514 -vnc :1 -monitor stdio -name qemu-vm1

try to migrate vm by virsh with ipv6 addr:
# virsh migrate libvirt-vm2 tcp:[2002::3:4]
error: invalid argument: could not parse connection URI tcp:[2002::3:4]


Thanks, Amos.

 Additional Error-handling was added to
 qemu-sockets.c:inet_connect()/inet_listen(), along with non-blocking support
 for inet_connect(), to subsume the ad-hoc client/server setup in
 migration-tcp.c


-- 
Amos.



Re: [Qemu-devel] [PATCH next v2 37/74] arm_boot: Pass ARMCPU to do_cpu_reset()

2012-05-11 Thread Peter Maydell
On 10 May 2012 23:40, Andreas Färber afaer...@suse.de wrote:
 Am 10.05.2012 23:41, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Allows us to use cpu_reset() in place of cpu_state_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
  hw/arm_boot.c |    9 ++---
  1 files changed, 6 insertions(+), 3 deletions(-)

 diff --git a/hw/arm_boot.c b/hw/arm_boot.c
 index 7447f5c..eb2d176 100644
 --- a/hw/arm_boot.c
 +++ b/hw/arm_boot.c
 [...]
 @@ -302,6 +303,7 @@ static void do_cpu_reset(void *opaque)

  void arm_load_kernel(CPUARMState *env, struct arm_boot_info *info)
  {
 +    ARMCPU *cpu;
     int kernel_size;
     int initrd_size;
     int n;

 This variable could be kept in the more limited scope inside
 the for loop below, right?

 It could. The function currently has a CPUARMState *env argument. That
 is supposed to change to ARMCPU *cpu in one of the followup series, in
 which case the for loop can reuse that parameter. I generally try to
 keep cpu and env close together so that it's visible in diff/grep
 context, as explained for your cpu_reset_model_id() series.

Ah, I see.

Acked-by: Peter Maydell peter.mayd...@linaro.org

-- PMM



[Qemu-devel] [PATCH] qemu/xendisk: set maximum number of grants to be used

2012-05-11 Thread Jan Beulich
Legacy (non-pvops) gntdev drivers may require this to be done when the
number of grants intended to be used simultaneously exceeds a certain
driver specific default limit.

Signed-off-by: Jan Beulich jbeul...@suse.com

--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -536,6 +536,10 @@ static void blk_alloc(struct XenDevice *
 if (xen_mode != XEN_EMULATE) {
 batch_maps = 1;
 }
+if (xc_gnttab_set_max_grants(xendev-gnttabdev,
+max_requests * BLKIF_MAX_SEGMENTS_PER_REQUEST + 1)  0)
+xen_be_printf(xendev, 0, xc_gnttab_set_max_grants failed: %s\n,
+  strerror(errno));
 }
 
 static int blk_init(struct XenDevice *xendev)



qemu/xendisk: set maximum number of grants to be used

Legacy (non-pvops) gntdev drivers may require this to be done when the
number of grants intended to be used simultaneously exceeds a certain
driver specific default limit.

Signed-off-by: Jan Beulich jbeul...@suse.com

--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -536,6 +536,10 @@ static void blk_alloc(struct XenDevice *
 if (xen_mode != XEN_EMULATE) {
 batch_maps = 1;
 }
+if (xc_gnttab_set_max_grants(xendev-gnttabdev,
+max_requests * BLKIF_MAX_SEGMENTS_PER_REQUEST + 1)  0)
+xen_be_printf(xendev, 0, xc_gnttab_set_max_grants failed: %s\n,
+  strerror(errno));
 }
 
 static int blk_init(struct XenDevice *xendev)


[Qemu-devel] [PATCH] qemu: whitelist kvm pv eoi feature

2012-05-11 Thread Michael S. Tsirkin
Whitelist kvm pv eoi feature. The feature is enabled
with -cpu kvm64. To disable: -cpu kvm64,-kvm_eoi.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---

Sending a copy to kernel list as this is needed
to test the pv eoi feature recently submitted.

 target-i386/cpuid.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 465ea15..c421b19 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -75,7 +75,7 @@ static const char *ext3_feature_name[] = {
 };
 
 static const char *kvm_feature_name[] = {
-kvmclock, kvm_nopiodelay, kvm_mmu, kvmclock, kvm_asyncpf, NULL, 
NULL, NULL,
+kvmclock, kvm_nopiodelay, kvm_mmu, kvmclock, kvm_asyncpf, NULL, 
kvm_eoi, NULL,
 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-- 
MST



[Qemu-devel] [RFC PATCH] qemu pci: pci_add_capability enhancement to prevent damaging config space

2012-05-11 Thread Alexey Kardashevskiy
Normally the pci_add_capability is called on devices to add new
capability. This is ok for emulated devices which capabilities list
is being built by QEMU.

In the case of VFIO the capability may already exist and adding new
capability into the beginning of the linked list may create a loop.

For example, the old code destroys the following config
of PCIe Intel E1000E:

before adding PCI_CAP_ID_MSI (0x05):
0x34: 0xC8
0xC8: 0x01 0xD0
0xD0: 0x05 0xE0
0xE0: 0x10 0x00

after:
0x34: 0xD0
0xC8: 0x01 0xD0
0xD0: 0x05 0xC8
0xE0: 0x10 0x00

As result capabilities 0x01 and 0x05 point to each other.

The proposed patch does not change capability pointers when
the same type capability is about to add.

Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
 hw/pci.c |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index aa0c0b8..1f7c924 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1794,10 +1794,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
 }

 config = pdev-config + offset;
-config[PCI_CAP_LIST_ID] = cap_id;
-config[PCI_CAP_LIST_NEXT] = pdev-config[PCI_CAPABILITY_LIST];
-pdev-config[PCI_CAPABILITY_LIST] = offset;
-pdev-config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
+if (config[PCI_CAP_LIST_ID] != cap_id) {
+config[PCI_CAP_LIST_ID] = cap_id;
+config[PCI_CAP_LIST_NEXT] = pdev-config[PCI_CAPABILITY_LIST];
+pdev-config[PCI_CAPABILITY_LIST] = offset;
+pdev-config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
+}
 memset(pdev-used + offset, 0xFF, size);
 /* Make capability read-only by default */
 memset(pdev-wmask + offset, 0, size);


-- 
Alexey



[Qemu-devel] [RFC PATCH] qemu pci: pci_add_capability enhancement to prevent damaging config space

2012-05-11 Thread Alexey Kardashevskiy
Normally the pci_add_capability is called on devices to add new
capability. This is ok for emulated devices which capabilities list
is being built by QEMU.

In the case of VFIO the capability may already exist and adding new
capability into the beginning of the linked list may create a loop.

For example, the old code destroys the following config
of PCIe Intel E1000E:

before adding PCI_CAP_ID_MSI (0x05):
0x34: 0xC8
0xC8: 0x01 0xD0
0xD0: 0x05 0xE0
0xE0: 0x10 0x00

after:
0x34: 0xD0
0xC8: 0x01 0xD0
0xD0: 0x05 0xC8
0xE0: 0x10 0x00

As result capabilities 0x01 and 0x05 point to each other.

The proposed patch does not change capability pointers when
the same type capability is about to add.

Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
 hw/pci.c |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index aa0c0b8..1f7c924 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1794,10 +1794,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
 }

 config = pdev-config + offset;
-config[PCI_CAP_LIST_ID] = cap_id;
-config[PCI_CAP_LIST_NEXT] = pdev-config[PCI_CAPABILITY_LIST];
-pdev-config[PCI_CAPABILITY_LIST] = offset;
-pdev-config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
+if (config[PCI_CAP_LIST_ID] != cap_id) {
+config[PCI_CAP_LIST_ID] = cap_id;
+config[PCI_CAP_LIST_NEXT] = pdev-config[PCI_CAPABILITY_LIST];
+pdev-config[PCI_CAPABILITY_LIST] = offset;
+pdev-config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
+}
 memset(pdev-used + offset, 0xFF, size);
 /* Make capability read-only by default */
 memset(pdev-wmask + offset, 0, size);


-- 
Alexey



Re: [Qemu-devel] [PATCH v2 03/10] xhci: Fix reset of MSI function

2012-05-11 Thread Gerd Hoffmann
On 05/10/12 22:08, Jan Kiszka wrote:
 Call msi_reset on device reset as still required by the core.

Note: msi on xhci is disabled by default (and also broken as far I know).

 +static void xhci_reset(void *opaque)
 +{
 +XHCIState *xhci = opaque;
 +

if (xhci-msi)

 +msi_reset(xhci-pci_dev);

}

 +xhci_reset_full(xhci);
 +}

And can't we let the pci core handle it so we don't need ugly wrappers
like this?

cheers,
  Gerd




Re: [Qemu-devel] [SeaBIOS] [RESEND PATCH v3] hotplug: add device per func in ACPI DSDT tables

2012-05-11 Thread Gleb Natapov
On Fri, May 11, 2012 at 01:46:17AM +0800, Jiang Liu wrote:
 On 05/11/2012 01:42 AM, Michael S. Tsirkin wrote:
  On Fri, May 11, 2012 at 01:17:38AM +0800, Jiang Liu wrote:
  On 05/09/2012 03:24 PM, Amos Kong wrote:
 
  ---
   src/ssdt-pcihp.dsl |   17 
   src/ssdt-pcihp.hex | 8869 
  +++-
   2 files changed, 8781 insertions(+), 105 deletions(-)
 
  diff --git a/src/ssdt-pcihp.dsl b/src/ssdt-pcihp.dsl
  index 4b435b8..2a3c326 100644
  --- a/src/ssdt-pcihp.dsl
  +++ b/src/ssdt-pcihp.dsl
  @@ -17,14 +17,23 @@ DefinitionBlock (ssdt-pcihp.aml, SSDT, 0x01, 
  BXPC, BXSSDTPCIHP, 0x1)
   // at runtime, if the slot is detected to not support hotplug.
   // Extract the offset of the address dword and the
   // _EJ0 name to allow this patching.
  -#define hotplug_slot(slot)  \
  -Device (S##slot) {  \
  +#define hotplug_func(slot, fn)  \
  +Device (S##slot##fn) {  \
  ACPI_EXTRACT_NAME_DWORD_CONST aml_adr_dword  \
  -   Name (_ADR, 0x##slot##)  \
  +   Name (_ADR, 0x##slot##000##fn)   \
  ACPI_EXTRACT_METHOD_STRING aml_ej0_name  \
  Method (_EJ0, 1) { Return(PCEJ(0x##slot)) }  \
  Name (_SUN, 0x##slot)\
   }
  It would be perfect if the Device object could also support _PS0 and _STA
  methods.
  
  It needs qemu support, and some backward compatibility hack.
  Why?
  
  Could we re-add the slot back after hot-removing it from the guest
  OS with this ACPI implementation? Say execute following scripts from guest 
  OS.
  echo 0  /sys/bus/pci/slot/xx/power
  echo 1  /sys/bus/pci/slot/xx/power
  
  No because qemu removes device after eject.
  Do you have a need for this functionality? What is it?
 
 I'm not familiar with qemu:(
 On native OS, admin could trigger PCI device hotplug operations through
 /sys/bus/pci/slot/xx/power. Not sure whether that's needed for guest OS too. 
 
Why is it needed on physical HW? May be it is needed in a VM for the
same reason?

--
Gleb.



[Qemu-devel] [PATCH 0/1] ISCSI: Only set read-event if we have i/o in flight

2012-05-11 Thread Ronnie Sahlberg

List, Kevin

Please find a small patch to the iscsi driver to only set the fd handler for 
read events IF we have i/o in flight and we are thus waiting for replies coming 
back from the target.

There are situations where qemu_aio_set_fd_handler(fd, read_callback, ...
will invoke the read_callback even if there is no fd/socket error and there are 
also no bytes to read from the descriptor.
This interacts bad with libiscsi which when invoked for POLLIN assumes that 
there is data to read and if ioctl(...FIONREAD...) returns 0 bytes that this 
indicates a socket failure/error/closure.


I dont know if that is expected behaviour from qemu_aio_set_fd_handler or not,
but aside from target-initiated NOPs, we shouldnt expect there to be any reason 
to read from the socket anyway unless we have i/o in flight and are waiting for 
replies, so this would also avoid eating some cpu invoking the read_callback 
when falsely.


regards
ronnie sahlberg




[Qemu-devel] [PATCH] ISCSI: Only set up the read-event if we are actually waiting for data to come back in from the target.

2012-05-11 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg ronniesahlb...@gmail.com
---
 block/iscsi.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index d37c4ee..989b5e9 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -105,7 +105,9 @@ iscsi_set_events(IscsiLun *iscsilun)
 {
 struct iscsi_context *iscsi = iscsilun-iscsi;
 
-qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
+qemu_aio_set_fd_handler(iscsi_get_fd(iscsi),
+   (iscsi_queue_length(iscsi)  0)
+   ? iscsi_process_read : NULL,
(iscsi_which_events(iscsi)  POLLOUT)
? iscsi_process_write : NULL,
iscsi_process_flush, iscsilun);
-- 
1.7.3.1




Re: [Qemu-devel] [RFC PATCH] qemu pci: pci_add_capability enhancement to prevent damaging config space

2012-05-11 Thread Alexander Graf

On 11.05.2012, at 08:45, Alexey Kardashevskiy wrote:

 Normally the pci_add_capability is called on devices to add new
 capability. This is ok for emulated devices which capabilities list
 is being built by QEMU.
 
 In the case of VFIO the capability may already exist and adding new
 capability into the beginning of the linked list may create a loop.
 
 For example, the old code destroys the following config
 of PCIe Intel E1000E:
 
 before adding PCI_CAP_ID_MSI (0x05):
 0x34: 0xC8
 0xC8: 0x01 0xD0
 0xD0: 0x05 0xE0
 0xE0: 0x10 0x00
 
 after:
 0x34: 0xD0
 0xC8: 0x01 0xD0
 0xD0: 0x05 0xC8
 0xE0: 0x10 0x00
 
 As result capabilities 0x01 and 0x05 point to each other.
 
 The proposed patch does not change capability pointers when
 the same type capability is about to add.
 
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---
 hw/pci.c |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)
 
 diff --git a/hw/pci.c b/hw/pci.c
 index aa0c0b8..1f7c924 100644
 --- a/hw/pci.c
 +++ b/hw/pci.c
 @@ -1794,10 +1794,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t 
 cap_id,
 }
 
 config = pdev-config + offset;
 -config[PCI_CAP_LIST_ID] = cap_id;
 -config[PCI_CAP_LIST_NEXT] = pdev-config[PCI_CAPABILITY_LIST];
 -pdev-config[PCI_CAPABILITY_LIST] = offset;
 -pdev-config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
 +if (config[PCI_CAP_LIST_ID] != cap_id) {

This doesn't scale. Capabilities are a list of CAPs. You'll have to do a loop 
through all capabilities, check if the one you want to add is there already and 
if so either

  * replace the existing one or
  * drop out and not write the new one in.

I'm not sure which way would be more natural.

 +config[PCI_CAP_LIST_ID] = cap_id;
 +config[PCI_CAP_LIST_NEXT] = pdev-config[PCI_CAPABILITY_LIST];
 +pdev-config[PCI_CAPABILITY_LIST] = offset;
 +pdev-config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
 +}
 memset(pdev-used + offset, 0xFF, size);
 /* Make capability read-only by default */
 memset(pdev-wmask + offset, 0, size);
 
 
 -- 
 Alexey




Re: [Qemu-devel] [PATCH for-1.1] target-mips: Remove commented-out function declaration

2012-05-11 Thread Andreas Färber
Am 06.05.2012 10:30, schrieb Stefan Weil:
 Am 05.05.2012 13:40, schrieb Andreas Färber:
 There is no function cpu_mips_get_clock(), so drop it.

 Signed-off-by: Andreas Färberafaer...@suse.de
 Cc: Stefan Weils...@weilnetz.de
 ---
   target-mips/cpu.h |1 -
   1 files changed, 0 insertions(+), 1 deletions(-)

 diff --git a/target-mips/cpu.h b/target-mips/cpu.h
 index c0f8826..44c1152 100644
 --- a/target-mips/cpu.h
 +++ b/target-mips/cpu.h
 @@ -627,7 +627,6 @@ enum {

   int cpu_mips_exec(CPUMIPSState *s);
   CPUMIPSState *cpu_mips_init(const char *cpu_model);
 -//~ uint32_t cpu_mips_get_clock (void);
   int cpu_mips_signal_handler(int host_signum, void *pinfo, void *puc);

   /* mips_timer.c */
 
 Acked-by: Stefan Weil s...@weilnetz.de

Thanks, applied to qom-next for now:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

I'll queue it for a 1.1-rc2 pull.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH next v2 33/74] pxa2xx: Use cpu_arm_init() and store ARMCPU

2012-05-11 Thread Peter Maydell
On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Also use cpu_reset() in place of cpu_state_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de

The pxa2xx stuff is probably going to clash with the cp15
rework, but I guess we'll sort that out when one or the
other of these series hits master.

Acked-by: Peter Maydell peter.mayd...@linaro.org

(The stuff pxa2xx.c is doing to CPUARMState is a really
gross layering violation, incidentally.)

-- PMM



Re: [Qemu-devel] [PATCH next v2 35/74] armv7m: Use cpu_arm_init() to obtain ARMCPU

2012-05-11 Thread Peter Maydell
On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Needed for armv7m_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de

Acked-by: Peter Maydell peter.mayd...@linaro.org

I'd have preferred it if you hadn't made the style change
from if (!foo) to if (foo == NULL), incidentally, but I'm
not going to insist on a respin for it.

-- PMM



Re: [Qemu-devel] [PATCH next v2 36/74] armv7m: Pass ARMCPU to armv7m_reset()

2012-05-11 Thread Peter Maydell
On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Allows us to use cpu_reset() in place of cpu_state_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de

Acked-by: Peter Maydell peter.mayd...@linaro.org


-- PMM



Re: [Qemu-devel] [PATCH next v2 73/74] linux-user: Use cpu_reset() after cpu_init() / cpu_copy()

2012-05-11 Thread Peter Maydell
On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Eliminates cpu_state_reset() usage.

 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
  linux-user/main.c    |    2 +-
  linux-user/syscall.c |    2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/linux-user/main.c b/linux-user/main.c
 index 191b750..49108b8 100644
 --- a/linux-user/main.c
 +++ b/linux-user/main.c
 @@ -3405,7 +3405,7 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
  #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
 -    cpu_state_reset(env);
 +    cpu_reset(ENV_GET_CPU(env));
  #endif

     thread_env = env;
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index 20d2a74..539af3f 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -4262,7 +4262,7 @@ static int do_fork(CPUArchState *env, unsigned int 
 flags, abi_ulong newsp,
         /* we create a new CPU instance. */
         new_env = cpu_copy(env);
  #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
 -        cpu_state_reset(new_env);
 +        cpu_reset(ENV_GET_CPU(new_env));
  #endif
         /* Init regs that differ from the parent.  */
         cpu_clone_regs(new_env, newsp);
 --

Do you have any plans to try to rationalise the handling of reset
so that we consistently either do or don't reset the cpu here,
rather than having it done based on a TARGET_* ifdef ?

-- PMM



Re: [Qemu-devel] [PATCH next v2 33/74] pxa2xx: Use cpu_arm_init() and store ARMCPU

2012-05-11 Thread Andreas Färber
Am 11.05.2012 13:16, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Also use cpu_reset() in place of cpu_state_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de
 
 The pxa2xx stuff is probably going to clash with the cp15
 rework, but I guess we'll sort that out when one or the
 other of these series hits master.

My plan is to pull cp15 in to qom-next actually. :)
I hope to get the ARM parts of this series applied and anything needed
for cpu_copy(), then some way to fix cpu_copy(), and then apply your
cp15 on top. That's the whole point of qom-next: avoiding clashes on master.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH next v2 01/74] target-arm: Use cpu_reset() in cpu_arm_init()

2012-05-11 Thread Peter Maydell
On 10 May 2012 01:13, Andreas Färber afaer...@suse.de wrote:
 Commit 3c30dd5a68e9fee6af67cfd0d14ed7520820f36a (target-arm: Move reset
 handling to arm_cpu_reset) QOM'ified CPU reset. Complete it by replacing
 cpu_state_reset() with cpu_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de

Acked-by: Peter Maydell peter.mayd...@linaro.org

-- PMM



Re: [Qemu-devel] [PATCH next v2 73/74] linux-user: Use cpu_reset() after cpu_init() / cpu_copy()

2012-05-11 Thread Andreas Färber
Am 11.05.2012 13:22, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Eliminates cpu_state_reset() usage.

 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
  linux-user/main.c|2 +-
  linux-user/syscall.c |2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/linux-user/main.c b/linux-user/main.c
 index 191b750..49108b8 100644
 --- a/linux-user/main.c
 +++ b/linux-user/main.c
 @@ -3405,7 +3405,7 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
  #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
 -cpu_state_reset(env);
 +cpu_reset(ENV_GET_CPU(env));
  #endif

 thread_env = env;
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index 20d2a74..539af3f 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -4262,7 +4262,7 @@ static int do_fork(CPUArchState *env, unsigned int 
 flags, abi_ulong newsp,
 /* we create a new CPU instance. */
 new_env = cpu_copy(env);
  #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
 -cpu_state_reset(new_env);
 +cpu_reset(ENV_GET_CPU(new_env));
  #endif
 /* Init regs that differ from the parent.  */
 cpu_clone_regs(new_env, newsp);
 --
 
 Do you have any plans to try to rationalise the handling of reset
 so that we consistently either do or don't reset the cpu here,
 rather than having it done based on a TARGET_* ifdef ?

Igor had an RFC for x86; sparc and ppc reset I haven't looked into yet.
Cc'ing Alex and Blue.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH] ISCSI: Only set up the read-event if we are actually waiting for data to come back in from the target.

2012-05-11 Thread Paolo Bonzini
Il 11/05/2012 12:22, Ronnie Sahlberg ha scritto:
 Signed-off-by: Ronnie Sahlberg ronniesahlb...@gmail.com
 ---
  block/iscsi.c |4 +++-
  1 files changed, 3 insertions(+), 1 deletions(-)
 
 diff --git a/block/iscsi.c b/block/iscsi.c
 index d37c4ee..989b5e9 100644
 --- a/block/iscsi.c
 +++ b/block/iscsi.c
 @@ -105,7 +105,9 @@ iscsi_set_events(IscsiLun *iscsilun)
  {
  struct iscsi_context *iscsi = iscsilun-iscsi;
  
 -qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
 +qemu_aio_set_fd_handler(iscsi_get_fd(iscsi),
 +   (iscsi_queue_length(iscsi)  0)
 +   ? iscsi_process_read : NULL,
 (iscsi_which_events(iscsi)  POLLOUT)
 ? iscsi_process_write : NULL,
 iscsi_process_flush, iscsilun);

I wonder if iscsi is also susceptible to the same race condition I saw
with NBD, where you can have:

1) select in the iothread exiting and reporting readability

2) the iothread subsequently blocking on the mutex

3) a VCPU thread's qemu_aio_wait() calling iscsi_process_read

4) when the VCPU releases the mutex, the iothread will call
iscsi_process_read again.

This should be easily reproducible with IDE drives, but the above patch
would not fix it.  Perhaps it's better to call iscsi_queue_length in
iscsi_process_read instead.

Paolo



Re: [Qemu-devel] [PATCH next v2 73/74] linux-user: Use cpu_reset() after cpu_init() / cpu_copy()

2012-05-11 Thread Alexander Graf

On 11.05.2012, at 13:26, Andreas Färber wrote:

 Am 11.05.2012 13:22, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Eliminates cpu_state_reset() usage.
 
 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
 linux-user/main.c|2 +-
 linux-user/syscall.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/linux-user/main.c b/linux-user/main.c
 index 191b750..49108b8 100644
 --- a/linux-user/main.c
 +++ b/linux-user/main.c
 @@ -3405,7 +3405,7 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
 #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
 -cpu_state_reset(env);
 +cpu_reset(ENV_GET_CPU(env));
 #endif
 
thread_env = env;
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index 20d2a74..539af3f 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -4262,7 +4262,7 @@ static int do_fork(CPUArchState *env, unsigned int 
 flags, abi_ulong newsp,
/* we create a new CPU instance. */
new_env = cpu_copy(env);
 #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
 -cpu_state_reset(new_env);
 +cpu_reset(ENV_GET_CPU(new_env));
 #endif
/* Init regs that differ from the parent.  */
cpu_clone_regs(new_env, newsp);
 --
 
 Do you have any plans to try to rationalise the handling of reset
 so that we consistently either do or don't reset the cpu here,
 rather than having it done based on a TARGET_* ifdef ?
 
 Igor had an RFC for x86; sparc and ppc reset I haven't looked into yet.
 Cc'ing Alex and Blue.

Why would you ever not want to reset the CPU for a new thread?


Alex




Re: [Qemu-devel] [PATCH 07/25] qdev: use object_property_print in info qtree

2012-05-11 Thread Paolo Bonzini
Il 10/05/2012 22:58, Jan Kiszka ha scritto:
  Otherwise, non-string properties without a legacy counterpart are missed.
  Also fix error propagation in object_property_print itself, otherwise
  pointer properties are printed as null.
 
  Signed-off-by: Paolo Bonzinipbonz...@redhat.com
  
  Reviewed-by: Anthony Liguori aligu...@us.ibm.com
  
 The bug that this fixes is still present in current 1.1-rc. Any chance
 that the patch makes it into the release?

Andreas, are you queueing this patch and 6/25 for 1.1-rc2?

Paolo



Re: [Qemu-devel] [PATCH next v2 64/74] mips_jazz: Use cpu_mips_init() to obtain MIPSCPU

2012-05-11 Thread Andreas Färber
Am 10.05.2012 02:14, schrieb Andreas Färber:
 Needed for main_cpu_reset().
 
 Signed-off-by: Andreas Färber afaer...@suse.de

Got to forget something:

Acked-by: Hervé Poussineau hpous...@reactos.org

Depends on the target-mips patch though, so need a review of that from
someone first.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH next v2 65/74] mips_jazz: Pass MIPSCPU to main_cpu_reset()

2012-05-11 Thread Andreas Färber
Am 10.05.2012 02:14, schrieb Andreas Färber:
 Allows us to use cpu_reset() in place of cpu_state_reset().
 
 Signed-off-by: Andreas Färber afaer...@suse.de

Forgot:

Acked-by: Hervé Poussineau hpous...@reactos.org

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH next v2 73/74] linux-user: Use cpu_reset() after cpu_init() / cpu_copy()

2012-05-11 Thread Peter Maydell
On 11 May 2012 12:28, Alexander Graf ag...@suse.de wrote:

 On 11.05.2012, at 13:26, Andreas Färber wrote:

 Am 11.05.2012 13:22, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Eliminates cpu_state_reset() usage.

 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
 linux-user/main.c    |    2 +-
 linux-user/syscall.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/linux-user/main.c b/linux-user/main.c
 index 191b750..49108b8 100644
 --- a/linux-user/main.c
 +++ b/linux-user/main.c
 @@ -3405,7 +3405,7 @@ int main(int argc, char **argv, char **envp)
        exit(1);
    }
 #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
 -    cpu_state_reset(env);
 +    cpu_reset(ENV_GET_CPU(env));
 #endif

    thread_env = env;
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index 20d2a74..539af3f 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -4262,7 +4262,7 @@ static int do_fork(CPUArchState *env, unsigned int 
 flags, abi_ulong newsp,
        /* we create a new CPU instance. */
        new_env = cpu_copy(env);
 #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
 -        cpu_state_reset(new_env);
 +        cpu_reset(ENV_GET_CPU(new_env));
 #endif
        /* Init regs that differ from the parent.  */
        cpu_clone_regs(new_env, newsp);
 --

 Do you have any plans to try to rationalise the handling of reset
 so that we consistently either do or don't reset the cpu here,
 rather than having it done based on a TARGET_* ifdef ?

 Igor had an RFC for x86; sparc and ppc reset I haven't looked into yet.
 Cc'ing Alex and Blue.

 Why would you ever not want to reset the CPU for a new thread?

The question is whether cpu_copy() and cpu_init() give you back a
CPU object which has been already reset, or one which you need to
reset yourself. At the moment I think we differ between targets
about this (eg cpu_arm_init() calls cpu_state_reset()).

(Also, copy this CPU so it has the same register contents as this
other one followed by reset it is pretty weird, what is the
thing we need to copy that means we can't do a cpu_init() of a
fresh clean CPU?)

-- PMM



Re: [Qemu-devel] [PATCH next v2 73/74] linux-user: Use cpu_reset() after cpu_init() / cpu_copy()

2012-05-11 Thread Andreas Färber
Am 11.05.2012 13:28, schrieb Alexander Graf:
 
 On 11.05.2012, at 13:26, Andreas Färber wrote:
 
 Am 11.05.2012 13:22, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index 20d2a74..539af3f 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -4262,7 +4262,7 @@ static int do_fork(CPUArchState *env, unsigned int 
 flags, abi_ulong newsp,
/* we create a new CPU instance. */
new_env = cpu_copy(env);
 #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
 -cpu_state_reset(new_env);
 +cpu_reset(ENV_GET_CPU(new_env));
 #endif
/* Init regs that differ from the parent.  */
cpu_clone_regs(new_env, newsp);
 --

 Do you have any plans to try to rationalise the handling of reset
 so that we consistently either do or don't reset the cpu here,
 rather than having it done based on a TARGET_* ifdef ?

 Igor had an RFC for x86; sparc and ppc reset I haven't looked into yet.
 Cc'ing Alex and Blue.
 
 Why would you ever not want to reset the CPU for a new thread?

cpu_copy() - cpu_init() do so themselves for some architectures.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH 07/25] qdev: use object_property_print in info qtree

2012-05-11 Thread Andreas Färber
Am 11.05.2012 13:28, schrieb Paolo Bonzini:
 Il 10/05/2012 22:58, Jan Kiszka ha scritto:
 Otherwise, non-string properties without a legacy counterpart are missed.
 Also fix error propagation in object_property_print itself, otherwise
 pointer properties are printed as null.

 Signed-off-by: Paolo Bonzinipbonz...@redhat.com

 Reviewed-by: Anthony Liguori aligu...@us.ibm.com

 The bug that this fixes is still present in current 1.1-rc. Any chance
 that the patch makes it into the release?
 
 Andreas, are you queueing this patch and 6/25 for 1.1-rc2?

I need to review it first and I try to pick the latest one (which would
presumable be in the QBus series), but yes, this and one other of yours
(the one with the prop-set or so check) is on my radar for 1.1-rc2.

I think I'll start a qom-1.1 branch to make this more transparent.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH for-1.1] target-i386: Defer MCE init

2012-05-11 Thread Andreas Färber
Am 10.05.2012 21:14, schrieb Igor Mammedov:
 - Original Message -
 From: Andreas Färber afaer...@suse.de
 To: qemu-devel@nongnu.org
 Cc: Eduardo Habkost ehabk...@redhat.com, Michael Roth 
 mdr...@linux.vnet.ibm.com, Anthony Liguori
 anth...@codemonkey.ws, Paolo Bonzini pbonz...@redhat.com, 
 imamm...@redhat.com, Andreas Färber
 afaer...@suse.de
 Sent: Thursday, May 10, 2012 12:09:10 AM
 Subject: [Qemu-devel] [PATCH for-1.1] target-i386: Defer MCE init

 Commit de024815e3b523addf58f1f79846b7fe74643678 (target-i386: QOM'ify
 CPU init) moved mce_init() call from helper.c:cpu_x86_init() into
 X86CPU's cpu.c:x86_cpu_initfn().
 mce_init() checks for a family = 6 though, so we could end up with a
 sequence such as for -cpu somecpu,family=6:

   x86_cpu_initfn = X86CPU::family == 5
 mce_init = no-op
   cpu_x86_register = X86CPU::family = 6
   = MCE unexpectedly not init'ed

 or for -cpu someothercpu,family=5:

   x86_cpu_initfn = X86CPU::family == 6
 mce_init = init'ed
   cpu_x86_register = X86CPU::family = 5
   = MCE unexpectedly init'ed

 Therefore partially revert the above commit. To avoid moving
 mce_init() back into helper.c, foresightedly move it into a
 new x86_cpu_realize() function and, in lack of ObjectClass::realize,
 call it directly from cpu_x86_init().

 While at it, move the qemu_init_vcpu() call that used to follow
 mce_init() in cpu_x86_init() into the new realizefn as well.

 Reported-by: Igor Mammedov imamm...@redhat.com
 Signed-off-by: Andreas Färber afaer...@suse.de
 Cc: Anthony Liguori anth...@codemonkey.ws
 Cc: Paolo Bonzini pbonz...@redhat.com
 Cc: Eduardo Habkost ehabk...@redhat.com
 Cc: Michael Roth mdr...@linux.vnet.ibm.com
 ---
  target-i386/cpu-qom.h |4 
  target-i386/cpu.c |9 -
  target-i386/helper.c  |2 +-
  3 files changed, 13 insertions(+), 2 deletions(-)
 
 Looks good to me.
 
 Reviewed-by: Igor Mammedov imamm...@redhat.com

Thanks, I've applied it to qom-1.1 and qom-next branches for now:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-1.1
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH next v2 01/74] target-arm: Use cpu_reset() in cpu_arm_init()

2012-05-11 Thread Andreas Färber
Am 11.05.2012 13:24, schrieb Peter Maydell:
 On 10 May 2012 01:13, Andreas Färber afaer...@suse.de wrote:
 Commit 3c30dd5a68e9fee6af67cfd0d14ed7520820f36a (target-arm: Move reset
 handling to arm_cpu_reset) QOM'ified CPU reset. Complete it by replacing
 cpu_state_reset() with cpu_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de
 
 Acked-by: Peter Maydell peter.mayd...@linaro.org

Thanks, applied to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH] ISCSI: Only set up the read-event if we are actually waiting for data to come back in from the target.

2012-05-11 Thread ronnie sahlberg
On Fri, May 11, 2012 at 9:27 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 11/05/2012 12:22, Ronnie Sahlberg ha scritto:
 Signed-off-by: Ronnie Sahlberg ronniesahlb...@gmail.com
 ---
  block/iscsi.c |    4 +++-
  1 files changed, 3 insertions(+), 1 deletions(-)

 diff --git a/block/iscsi.c b/block/iscsi.c
 index d37c4ee..989b5e9 100644
 --- a/block/iscsi.c
 +++ b/block/iscsi.c
 @@ -105,7 +105,9 @@ iscsi_set_events(IscsiLun *iscsilun)
  {
      struct iscsi_context *iscsi = iscsilun-iscsi;

 -    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
 +    qemu_aio_set_fd_handler(iscsi_get_fd(iscsi),
 +                           (iscsi_queue_length(iscsi)  0)
 +                           ? iscsi_process_read : NULL,
                             (iscsi_which_events(iscsi)  POLLOUT)
                             ? iscsi_process_write : NULL,
                             iscsi_process_flush, iscsilun);

 I wonder if iscsi is also susceptible to the same race condition I saw
 with NBD, where you can have:

 1) select in the iothread exiting and reporting readability

 2) the iothread subsequently blocking on the mutex

 3) a VCPU thread's qemu_aio_wait() calling iscsi_process_read

 4) when the VCPU releases the mutex, the iothread will call
 iscsi_process_read again.

 This should be easily reproducible with IDE drives, but the above patch
 would not fix it.  Perhaps it's better to call iscsi_queue_length in
 iscsi_process_read instead.


So there is a known condition where the event callbacks can be
triggered like this.
Thanks for confirming!

Let me do a new patch along your suggestion, test it a bit and I will
re-send to the list.


regards
ronnie sahlberg



Re: [Qemu-devel] [PATCH next v2 33/74] pxa2xx: Use cpu_arm_init() and store ARMCPU

2012-05-11 Thread Andreas Färber
Am 11.05.2012 13:16, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Also use cpu_reset() in place of cpu_state_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de
 
 The pxa2xx stuff is probably going to clash with the cp15
 rework, but I guess we'll sort that out when one or the
 other of these series hits master.
 
 Acked-by: Peter Maydell peter.mayd...@linaro.org

Thanks, applied to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH v12 0/4] support to migrate with IPv6 address

2012-05-11 Thread Eric Blake
[adding libvirt]

On 05/11/2012 12:46 AM, Amos Kong wrote:
 On 05/11/2012 02:12 AM, Michael Roth wrote:
 On Thu, May 10, 2012 at 11:29:48AM -0600, Eric Blake wrote:
 On 05/10/2012 10:27 AM, Amos Kong wrote:
 Those patches updated help functions in qemu-socket.c,
 and used them in migrate-tcp.c to supporting IPv6 migration.

 addr parsing now relies on qemu-sockets.c:inet_parse(), which has supported
 [ip6addr]:port for a while, as opposed to net.c:parse_host_port(), which
 didn't.
 
 yeah.
 
 I didn't change qemu monitor cmd interface in this patchset,
 and the transport of data is done by qemu, not libvirt.
 
 I guess libvirt only needs to update addr string parse,
 for example:
 
  GOOD
 start a VM:
 # qemu-kvm --enable-kvm -boot n -incoming tcp:ipv6alias:16514 -vnc :1
 -monitor stdio -name qemu-vm1
 
 try to migrate vm by virsh with addr alias
 # virsh migrate libivrt-vm2 tcp:ipv6alias
 (connection can establish)
 
 --- FAIL
 start a VM:
 # qemu-kvm-apply-my-patches --enable-kvm -boot n -incoming
 tcp:[2002::3:4]:16514 -vnc :1 -monitor stdio -name qemu-vm1
 
 try to migrate vm by virsh with ipv6 addr:
 # virsh migrate libvirt-vm2 tcp:[2002::3:4]
 error: invalid argument: could not parse connection URI tcp:[2002::3:4]

Thanks for researching that.  Looks like it should be fixed in libvirt
to match, then.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RFC PATCH] qemu pci: pci_add_capability enhancement to prevent damaging config space

2012-05-11 Thread Alexey Kardashevskiy
11.05.2012 20:52, Alexander Graf написал:
 
 On 11.05.2012, at 08:45, Alexey Kardashevskiy wrote:
 
 Normally the pci_add_capability is called on devices to add new
 capability. This is ok for emulated devices which capabilities list
 is being built by QEMU.

 In the case of VFIO the capability may already exist and adding new
 capability into the beginning of the linked list may create a loop.

 For example, the old code destroys the following config
 of PCIe Intel E1000E:

 before adding PCI_CAP_ID_MSI (0x05):
 0x34: 0xC8
 0xC8: 0x01 0xD0
 0xD0: 0x05 0xE0
 0xE0: 0x10 0x00

 after:
 0x34: 0xD0
 0xC8: 0x01 0xD0
 0xD0: 0x05 0xC8
 0xE0: 0x10 0x00

 As result capabilities 0x01 and 0x05 point to each other.

 The proposed patch does not change capability pointers when
 the same type capability is about to add.

 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---
 hw/pci.c |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

 diff --git a/hw/pci.c b/hw/pci.c
 index aa0c0b8..1f7c924 100644
 --- a/hw/pci.c
 +++ b/hw/pci.c
 @@ -1794,10 +1794,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t 
 cap_id,
 }

 config = pdev-config + offset;
 -config[PCI_CAP_LIST_ID] = cap_id;
 -config[PCI_CAP_LIST_NEXT] = pdev-config[PCI_CAPABILITY_LIST];
 -pdev-config[PCI_CAPABILITY_LIST] = offset;
 -pdev-config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
 +if (config[PCI_CAP_LIST_ID] != cap_id) {
 
 This doesn't scale. Capabilities are a list of CAPs. You'll have to do a loop 
 through all capabilities, check if the one you want to add is there already 
 and if so either
   * replace the existing one or
   * drop out and not write the new one in.
 
 I'm not sure which way would be more natural.

There is a third option - add another function, lets call it
pci_fixup_capability() which would do whatever pci_add_capability() does
but won't touch list pointers.

When vfio, pci_add_capability() is called from the code which knows
exactly that the capability exists and where it is and it calls
pci_add_capability() based on this knowledge so doing additional loops
just for imaginery scalability is a bit weird, no?


 +config[PCI_CAP_LIST_ID] = cap_id;
 +config[PCI_CAP_LIST_NEXT] = pdev-config[PCI_CAPABILITY_LIST];
 +pdev-config[PCI_CAPABILITY_LIST] = offset;
 +pdev-config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
 +}
 memset(pdev-used + offset, 0xFF, size);
 /* Make capability read-only by default */
 memset(pdev-wmask + offset, 0, size);


-- 
With best regards

Alexey Kardashevskiy



Re: [Qemu-devel] [PATCH] qemu: whitelist kvm pv eoi feature

2012-05-11 Thread Eric Blake
[adding libvirt, dropping kernel]

On 05/11/2012 01:43 AM, Michael S. Tsirkin wrote:
 Whitelist kvm pv eoi feature. The feature is enabled
 with -cpu kvm64. To disable: -cpu kvm64,-kvm_eoi.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
 
 Sending a copy to kernel list as this is needed
 to test the pv eoi feature recently submitted.
 
  target-i386/cpuid.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
 index 465ea15..c421b19 100644
 --- a/target-i386/cpuid.c
 +++ b/target-i386/cpuid.c
 @@ -75,7 +75,7 @@ static const char *ext3_feature_name[] = {
  };
  
  static const char *kvm_feature_name[] = {
 -kvmclock, kvm_nopiodelay, kvm_mmu, kvmclock, kvm_asyncpf, 
 NULL, NULL, NULL,
 +kvmclock, kvm_nopiodelay, kvm_mmu, kvmclock, kvm_asyncpf, 
 NULL, kvm_eoi, NULL,

Should libvirt also be recognizing this processor capability in its XML
in order to drive the -kvm_eoi option?

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH next v2 34/74] omap: Use cpu_arm_init() to store ARMCPU in omap_mpu_state_s

2012-05-11 Thread Andreas Färber
Am 10.05.2012 23:38, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Fix tab indentations of comments, add braces, use cpu_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
  hw/nseries.c  |6 +++---
  hw/omap.h |2 +-
  hw/omap1.c|   20 +++-
  hw/omap2.c|8 
  hw/omap_sx1.c |2 +-
  hw/palm.c |2 +-
  6 files changed, 21 insertions(+), 19 deletions(-)
 
 There are some structs here which use the name 'cpu' for
 a 'struct omap_mpu_state_s *', which leads to ugliness
 like s-cpu-cpu-env. (We shouldn't be trying to fix that
 here, just a passing comment.)
 
 Acked-by: Peter Maydell peter.mayd...@linaro.org

Thanks, applied to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

Now that we have qom-next, we are no longer limited to the concept of
self-contained series so I can try to turn that into s-mpu-cpu-env
the weekend. Not perfect either, but better distinguishable.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH v2 03/10] xhci: Fix reset of MSI function

2012-05-11 Thread Jan Kiszka
On 2012-05-11 05:16, Gerd Hoffmann wrote:
 On 05/10/12 22:08, Jan Kiszka wrote:
 Call msi_reset on device reset as still required by the core.
 
 Note: msi on xhci is disabled by default (and also broken as far I know).

OK, then we can likely skip this patch for 1.1/stable.

 
 +static void xhci_reset(void *opaque)
 +{
 +XHCIState *xhci = opaque;
 +
 
 if (xhci-msi)

Oops.

 
 +msi_reset(xhci-pci_dev);
 
 }
 
 +xhci_reset_full(xhci);
 +}
 
 And can't we let the pci core handle it so we don't need ugly wrappers
 like this?

That's what patches later in the series do. But Michael was preferring
this approach for 1.1 and the cleanup for 1.2.

Jan

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



Re: [Qemu-devel] [PATCH v2 03/10] xhci: Fix reset of MSI function

2012-05-11 Thread Gerd Hoffmann
  Hi,

 And can't we let the pci core handle it so we don't need ugly wrappers
 like this?
 
 That's what patches later in the series do. But Michael was preferring
 this approach for 1.1 and the cleanup for 1.2.

Ah, ok, good.  Yea, lets leave it alone for 1.1 and fix it properly in 1.2

cheers,
  Gerd



Re: [Qemu-devel] [PATCH next v2 35/74] armv7m: Use cpu_arm_init() to obtain ARMCPU

2012-05-11 Thread Andreas Färber
Am 11.05.2012 13:18, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Needed for armv7m_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de
 
 Acked-by: Peter Maydell peter.mayd...@linaro.org

Thanks, applied to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

 I'd have preferred it if you hadn't made the style change
 from if (!foo) to if (foo == NULL), incidentally, but I'm
 not going to insist on a respin for it.

For the record, Peter's request here was to not propagate such stylistic
changes unneccessarily throughout old code. In these two cases on one
line I added the missing opening brace and on the other I had to change
the variable name so it does not add to the patch.

Both styles are valid C and can be expected to produce identical machine
code with an optimizing compiler.

What we might do though is assist the compiler in determining which code
paths are likely() vs. unlikely(). For stylistic reasons I prefer to
have the unlikely error case in the if, e.g. in cpu_sh4_init(). These
are not hot paths though so it shouldn't matter much.

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH qemu v3 0/6] -no-user-config option, move CPU models to /usr/share

2012-05-11 Thread Eduardo Habkost

I just noticed it didn't get into -rc1 either. So, this is definitely
not going to be in qemu-1.1, I guess?


On Wed, May 02, 2012 at 01:07:24PM -0300, Eduardo Habkost wrote:
 Changes v2 - v3:
  - Actually change 'defconfig' type declaration to bool
  - Rebase against latest qemu.git (commit 
 563987d0a799f90b58a575b190a57546c335191b)
 
 Changes v1 - v2:
  - Move qemu_read_default_config_files() prototype to qemu-config.h
  - Make defconfig and userconfig variable bool
  - Coding style change
 
 Patches 1 to 4 just move some code around, patch 5 just adds the new option
 without adding any new config file. Patch 6 finally creates a /usr/share/qemu
 /cpus-x86_64.conf file, with the CPU models we currently have on Qemu.
 
 Reference to previous discussion:
  - http://marc.info/?l=qemu-develm=133278877315665
 
 
 Eduardo Habkost (6):
   move code to read default config files to a separate function (v2)
   eliminate arch_config_name variable
   move list of default config files to an array
   vl.c: change 'defconfig' variable to bool (v2)
   implement -no-user-config command-line option (v3)
   move CPU definitions to /usr/share/qemu/cpus-x86_64.conf (v2)
 
  Makefile |   12 +++-
  arch_init.c  |   32 -
  arch_init.h  |2 -
  qemu-config.h|4 +
  qemu-options.hx  |   16 -
  sysconfigs/target/cpus-x86_64.conf   |  128 
 ++
  sysconfigs/target/target-x86_64.conf |  128 
 --
  vl.c |   18 ++---
  8 files changed, 193 insertions(+), 147 deletions(-)
  create mode 100644 sysconfigs/target/cpus-x86_64.conf
 
 -- 
 1.7.3.2
 

-- 
Eduardo



Re: [Qemu-devel] [PATCH next v2 36/74] armv7m: Pass ARMCPU to armv7m_reset()

2012-05-11 Thread Andreas Färber
Am 11.05.2012 13:19, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Allows us to use cpu_reset() in place of cpu_state_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de
 
 Acked-by: Peter Maydell peter.mayd...@linaro.org

Thanks, applied to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH qemu v3 0/6] -no-user-config option, move CPU models to /usr/share

2012-05-11 Thread Anthony Liguori

On 05/11/2012 08:34 AM, Eduardo Habkost wrote:


I just noticed it didn't get into -rc1 either. So, this is definitely
not going to be in qemu-1.1, I guess?


I've got this applied and am testing it right now for -rc2.

Regards,

Anthony Liguori




On Wed, May 02, 2012 at 01:07:24PM -0300, Eduardo Habkost wrote:

Changes v2 -  v3:
  - Actually change 'defconfig' type declaration to bool
  - Rebase against latest qemu.git (commit 
563987d0a799f90b58a575b190a57546c335191b)

Changes v1 -  v2:
  - Move qemu_read_default_config_files() prototype to qemu-config.h
  - Make defconfig and userconfig variable bool
  - Coding style change

Patches 1 to 4 just move some code around, patch 5 just adds the new option
without adding any new config file. Patch 6 finally creates a /usr/share/qemu
/cpus-x86_64.conf file, with the CPU models we currently have on Qemu.

Reference to previous discussion:
  - http://marc.info/?l=qemu-develm=133278877315665


Eduardo Habkost (6):
   move code to read default config files to a separate function (v2)
   eliminate arch_config_name variable
   move list of default config files to an array
   vl.c: change 'defconfig' variable to bool (v2)
   implement -no-user-config command-line option (v3)
   move CPU definitions to /usr/share/qemu/cpus-x86_64.conf (v2)

  Makefile |   12 +++-
  arch_init.c  |   32 -
  arch_init.h  |2 -
  qemu-config.h|4 +
  qemu-options.hx  |   16 -
  sysconfigs/target/cpus-x86_64.conf   |  128 ++
  sysconfigs/target/target-x86_64.conf |  128 --
  vl.c |   18 ++---
  8 files changed, 193 insertions(+), 147 deletions(-)
  create mode 100644 sysconfigs/target/cpus-x86_64.conf

--
1.7.3.2








Re: [Qemu-devel] [PATCH next v2 37/74] arm_boot: Pass ARMCPU to do_cpu_reset()

2012-05-11 Thread Andreas Färber
Am 11.05.2012 09:17, schrieb Peter Maydell:
 On 10 May 2012 23:40, Andreas Färber afaer...@suse.de wrote:
 Am 10.05.2012 23:41, schrieb Peter Maydell:
 On 10 May 2012 01:14, Andreas Färber afaer...@suse.de wrote:
 Allows us to use cpu_reset() in place of cpu_state_reset().

 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
  hw/arm_boot.c |9 ++---
  1 files changed, 6 insertions(+), 3 deletions(-)
 
 Acked-by: Peter Maydell peter.mayd...@linaro.org

Thanks, applied to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH qemu v3 0/6] -no-user-config option, move CPU models to /usr/share

2012-05-11 Thread Eduardo Habkost
On Fri, May 11, 2012 at 08:37:33AM -0500, Anthony Liguori wrote:
 On 05/11/2012 08:34 AM, Eduardo Habkost wrote:
 
 I just noticed it didn't get into -rc1 either. So, this is definitely
 not going to be in qemu-1.1, I guess?
 
 I've got this applied and am testing it right now for -rc2.

Awesome. Thanks!  :-)

-- 
Eduardo

 
 Regards,
 
 Anthony Liguori
 
 
 
 On Wed, May 02, 2012 at 01:07:24PM -0300, Eduardo Habkost wrote:
 Changes v2 -  v3:
   - Actually change 'defconfig' type declaration to bool
   - Rebase against latest qemu.git (commit 
  563987d0a799f90b58a575b190a57546c335191b)
 
 Changes v1 -  v2:
   - Move qemu_read_default_config_files() prototype to qemu-config.h
   - Make defconfig and userconfig variable bool
   - Coding style change
 
 Patches 1 to 4 just move some code around, patch 5 just adds the new option
 without adding any new config file. Patch 6 finally creates a 
 /usr/share/qemu
 /cpus-x86_64.conf file, with the CPU models we currently have on Qemu.
 
 Reference to previous discussion:
   - http://marc.info/?l=qemu-develm=133278877315665
 
 
 Eduardo Habkost (6):
move code to read default config files to a separate function (v2)
eliminate arch_config_name variable
move list of default config files to an array
vl.c: change 'defconfig' variable to bool (v2)
implement -no-user-config command-line option (v3)
move CPU definitions to /usr/share/qemu/cpus-x86_64.conf (v2)
 
   Makefile |   12 +++-
   arch_init.c  |   32 -
   arch_init.h  |2 -
   qemu-config.h|4 +
   qemu-options.hx  |   16 -
   sysconfigs/target/cpus-x86_64.conf   |  128 
  ++
   sysconfigs/target/target-x86_64.conf |  128 
  --
   vl.c |   18 ++---
   8 files changed, 193 insertions(+), 147 deletions(-)
   create mode 100644 sysconfigs/target/cpus-x86_64.conf
 
 --
 1.7.3.2
 
 
 



Re: [Qemu-devel] [PATCH qemu v3 0/6] -no-user-config option, move CPU models to /usr/share

2012-05-11 Thread Jiri Denemark
On Fri, May 11, 2012 at 08:37:33 -0500, Anthony Liguori wrote:
 On 05/11/2012 08:34 AM, Eduardo Habkost wrote:
 
  I just noticed it didn't get into -rc1 either. So, this is definitely
  not going to be in qemu-1.1, I guess?
 
 I've got this applied and am testing it right now for -rc2.

Oh, having this in rc2 would be awesome. I already tested this patch set (at
the time it was submitted) with my patches for libvirt which make use of it
and it all worked very nicely and I was finally able to use all the new fancy
CPUs :-) If it helps, you can count this as

Tested-by: Jiri Denemark jdene...@redhat.com

Jirka



Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels

2012-05-11 Thread Alexander Graf

On 06.11.2011, at 14:54, Jan Kiszka wrote:

 On 2011-08-24 23:38, Alexander Graf wrote:
 On LinuxCon I had a nice chat with Linus on what he thinks kvm-tool
 would be doing and what he expects from it. Basically he wants a
 small and simple tool he and other developers can run to try out and
 see if the kernel they just built actually works.
 
 Fortunately, QEMU can do that today already! The only piece that was
 missing was the simple piece of the equation, so here is a script
 that wraps around QEMU and executes a kernel you just built.
 
 If you do have KVM around and are not cross-compiling, it will use
 KVM. But if you don't, you can still fall back to emulation mode and
 at least check if your kernel still does what you expect. I only
 implemented support for s390x and ppc there, but it's easily extensible
 to more platforms, as QEMU can emulate (and virtualize) pretty much
 any platform out there.
 
 If you don't have qemu installed, please do so before using this script. Your
 distro should provide a package for it (might even call it kvm). If not,
 just compile it from source - it's not hard!
 
 To quickly get going, just execute the following as user:
 
$ ./Documentation/run-qemu.sh -r / -a init=/bin/bash
 
 This will drop you into a shell on your rootfs.
 
 Happy hacking!
 
 Signed-off-by: Alexander Graf ag...@suse.de
 
 ---
 
 v1 - v2:
 
  - fix naming of QEMU
  - use grep -q for has_config
  - support multiple -a args
  - spawn gdb on execution
  - pass through qemu options
  - dont use qemu-system-x86_64 on i386
  - add funny sentence to startup text
  - more helpful error messages
 ---
 scripts/run-qemu.sh |  334 
 +++
 1 files changed, 334 insertions(+), 0 deletions(-)
 create mode 100755 scripts/run-qemu.sh
 
 diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
 new file mode 100755
 index 000..5d4e185
 --- /dev/null
 +++ b/scripts/run-qemu.sh
 @@ -0,0 +1,334 @@
 +#!/bin/bash
 +#
 +# QEMU Launcher
 +#
 +# This script enables simple use of the KVM and QEMU tool stack for
 +# easy kernel testing. It allows to pass either a host directory to
 +# the guest or a disk image. Example usage:
 +#
 +# Run the host root fs inside a VM:
 +#
 +# $ ./scripts/run-qemu.sh -r /
 +#
 +# Run the same with SDL:
 +#
 +# $ ./scripts/run-qemu.sh -r / --sdl
 +# 
 +# Or with a PPC build:
 +#
 +# $ ARCH=ppc ./scripts/run-qemu.sh -r /
 +# 
 +# PPC with a mac99 model by passing options to QEMU:
 +#
 +# $ ARCH=ppc ./scripts/run-qemu.sh -r / -- -M mac99
 +#
 +
 +USE_SDL=
 +USE_VNC=
 +USE_GDB=1
 +KERNEL_BIN=arch/x86/boot/bzImage
 +MON_STDIO=
 +KERNEL_APPEND2=
 +SERIAL=ttyS0
 +SERIAL_KCONFIG=SERIAL_8250
 +BASENAME=$(basename $0)
 +
 +function usage() {
 +echo 
 +$BASENAME allows you to execute a virtual machine with the Linux kernel
 +that you just built. To only execute a simple VM, you can just run it
 +on your root fs with \-r / -a init=/bin/bash\
 +
 +-a, --append parameters
 +Append the given parameters to the kernel command line.
 +
 +-d, --disk image
 +Add the image file as disk into the VM.
 +
 +-D, --no-gdb
 +Don't run an xterm with gdb attached to the guest.
 +
 +-r, --root directory
 +Use the specified directory as root directory inside the guest.
 +
 +-s, --sdl
 +Enable SDL graphical output.
 +
 +-S, --smp cpus
 +Set number of virtual CPUs.
 +
 +-v, --vnc
 +Enable VNC graphical output.
 +
 +Examples:
 +
 +Run the host root fs inside a VM:
 +$ ./scripts/run-qemu.sh -r /
 +
 +Run the same with SDL:
 +$ ./scripts/run-qemu.sh -r / --sdl
 +
 +Or with a PPC build:
 +$ ARCH=ppc ./scripts/run-qemu.sh -r /
 +
 +PPC with a mac99 model by passing options to QEMU:
 +$ ARCH=ppc ./scripts/run-qemu.sh -r / -- -M mac99
 +
 +}
 +
 +function require_config() {
 +if [ $(grep CONFIG_$1=y .config) ]; then
 +return
 +fi
 +
 +echo You need to enable CONFIG_$1 for run-qemu to work properly
 +exit 1
 +}
 +
 +function has_config() {
 +grep -q CONFIG_$1=y .config
 +}
 +
 +function drive_if() {
 +if has_config VIRTIO_BLK; then
 +echo virtio
 +elif has_config ATA_PIIX; then
 +echo ide
 
 + require_config BLK_DEV_SD
 
 Maybe there should also be a warning if no standard FS (ext[34], btrfs,
 xfs etc.) is build into the kernel.
 
 Another thing, but that's just a recommendation for initrd-free mode:
 DEVTMPFS_MOUNT
 
 +else
 +echo \
 +Your kernel must have either VIRTIO_BLK or ATA_PIIX
 +enabled for block device assignment 2
 +exit 1
 +fi
 +}
 +
 +GETOPT=`getopt -o a:d:Dhr:sS:v --long 
 append,disk:,no-gdb,help,root:,sdl,smp:,vnc \
 +-n $(basename \$0\) -- $@`
 +
 +if [ $? != 0 ]; then
 +echo Terminating... 2
 +exit 1
 +fi
 +
 +eval set -- $GETOPT
 +
 +while true; do
 +case $1 in
 +-a|--append)
 +

Re: [Qemu-devel] [PATCH v3] pci: clean all funcs when hot-removing multifunc device

2012-05-11 Thread Jiang Liu
On 05/11/2012 08:24 AM, Amos Kong wrote:
 On 05/11/2012 07:54 AM, Amos Kong wrote:
 On 05/11/2012 02:55 AM, Michael S. Tsirkin wrote:
 On Fri, May 11, 2012 at 01:09:13AM +0800, Jiang Liu wrote:
 On 05/10/2012 11:44 PM, Amos Kong wrote:

 diff --git a/drivers/pci/hotplug/acpiphp_glue.c 
 b/drivers/pci/hotplug/acpiphp_glue.c
 index 806c44f..a7442d9 100644
 --- a/drivers/pci/hotplug/acpiphp_glue.c
 +++ b/drivers/pci/hotplug/acpiphp_glue.c
 @@ -885,7 +885,7 @@ static void disable_bridges(struct pci_bus *bus)
  static int disable_device(struct acpiphp_slot *slot)
  {
   struct acpiphp_func *func;
 - struct pci_dev *pdev;
 + struct pci_dev *pdev, *tmp;
   struct pci_bus *bus = slot-bridge-pci_bus;
  
   /* The slot will be enabled when func 0 is added, so check
 @@ -902,9 +902,10 @@ static int disable_device(struct acpiphp_slot *slot)
   func-bridge = NULL;
   }
  
 - pdev = pci_get_slot(slot-bridge-pci_bus,
 - PCI_DEVFN(slot-device, func-function));
 - if (pdev) {
 + list_for_each_entry_safe(pdev, tmp, bus-devices, bus_list) {
 + if (PCI_SLOT(pdev-devfn) != slot-device)
 + continue;
 +
 The pci_bus_sem lock should be acquired when walking the bus-devices list.
 Otherwise it may cause invalid memory access if another thread is modifying
 the bus-devices list concurrently.
 
 pci_bus_sem lock is only request for writing bus-devices list, right ?
 and this protection already exists in pci_destory_dev().
That's for writer. For reader to walk the pci_bus-devices list, you also need
to acquire the reader lock by down_read(pci_bus_sem). Please refer to 
pci_get_slot() for example. This especially import for native OS because there
may be multiple PCI slots/devices on the bus.




Re: [Qemu-devel] [PATCH 07/21] qdev: fix -device foo,?

2012-05-11 Thread Andreas Färber
Am 02.05.2012 13:30, schrieb Paolo Bonzini:
 Since most property types do not have a parse property now, this was
 broken.  Fix it by looking at the setter instead.
 
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com

Reviewed-by: Anthony Liguori aligu...@us.ibm.com
Acked-by: Andreas Färber afaer...@suse.de

I'm guessing Anthony has this one in his 1.1-rc2 queue already; I'm
applying it to qom-1.1 and qom-next branches:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-1.1
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels

2012-05-11 Thread Jan Kiszka
On 2012-05-11 10:42, Alexander Graf wrote:
 
 On 06.11.2011, at 14:54, Jan Kiszka wrote:
 
 On 2011-08-24 23:38, Alexander Graf wrote:
 On LinuxCon I had a nice chat with Linus on what he thinks kvm-tool
 would be doing and what he expects from it. Basically he wants a
 small and simple tool he and other developers can run to try out and
 see if the kernel they just built actually works.

 Fortunately, QEMU can do that today already! The only piece that was
 missing was the simple piece of the equation, so here is a script
 that wraps around QEMU and executes a kernel you just built.

 If you do have KVM around and are not cross-compiling, it will use
 KVM. But if you don't, you can still fall back to emulation mode and
 at least check if your kernel still does what you expect. I only
 implemented support for s390x and ppc there, but it's easily extensible
 to more platforms, as QEMU can emulate (and virtualize) pretty much
 any platform out there.

 If you don't have qemu installed, please do so before using this script. 
 Your
 distro should provide a package for it (might even call it kvm). If not,
 just compile it from source - it's not hard!

 To quickly get going, just execute the following as user:

$ ./Documentation/run-qemu.sh -r / -a init=/bin/bash

 This will drop you into a shell on your rootfs.

 Happy hacking!

 Signed-off-by: Alexander Graf ag...@suse.de

 ---

 v1 - v2:

  - fix naming of QEMU
  - use grep -q for has_config
  - support multiple -a args
  - spawn gdb on execution
  - pass through qemu options
  - dont use qemu-system-x86_64 on i386
  - add funny sentence to startup text
  - more helpful error messages
 ---
 scripts/run-qemu.sh |  334 
 +++
 1 files changed, 334 insertions(+), 0 deletions(-)
 create mode 100755 scripts/run-qemu.sh

 diff --git a/scripts/run-qemu.sh b/scripts/run-qemu.sh
 new file mode 100755
 index 000..5d4e185
 --- /dev/null
 +++ b/scripts/run-qemu.sh
 @@ -0,0 +1,334 @@
 +#!/bin/bash
 +#
 +# QEMU Launcher
 +#
 +# This script enables simple use of the KVM and QEMU tool stack for
 +# easy kernel testing. It allows to pass either a host directory to
 +# the guest or a disk image. Example usage:
 +#
 +# Run the host root fs inside a VM:
 +#
 +# $ ./scripts/run-qemu.sh -r /
 +#
 +# Run the same with SDL:
 +#
 +# $ ./scripts/run-qemu.sh -r / --sdl
 +# 
 +# Or with a PPC build:
 +#
 +# $ ARCH=ppc ./scripts/run-qemu.sh -r /
 +# 
 +# PPC with a mac99 model by passing options to QEMU:
 +#
 +# $ ARCH=ppc ./scripts/run-qemu.sh -r / -- -M mac99
 +#
 +
 +USE_SDL=
 +USE_VNC=
 +USE_GDB=1
 +KERNEL_BIN=arch/x86/boot/bzImage
 +MON_STDIO=
 +KERNEL_APPEND2=
 +SERIAL=ttyS0
 +SERIAL_KCONFIG=SERIAL_8250
 +BASENAME=$(basename $0)
 +
 +function usage() {
 +   echo 
 +$BASENAME allows you to execute a virtual machine with the Linux kernel
 +that you just built. To only execute a simple VM, you can just run it
 +on your root fs with \-r / -a init=/bin/bash\
 +
 +   -a, --append parameters
 +   Append the given parameters to the kernel command line.
 +
 +   -d, --disk image
 +   Add the image file as disk into the VM.
 +
 +   -D, --no-gdb
 +   Don't run an xterm with gdb attached to the guest.
 +
 +   -r, --root directory
 +   Use the specified directory as root directory inside the guest.
 +
 +   -s, --sdl
 +   Enable SDL graphical output.
 +
 +   -S, --smp cpus
 +   Set number of virtual CPUs.
 +
 +   -v, --vnc
 +   Enable VNC graphical output.
 +
 +Examples:
 +
 +   Run the host root fs inside a VM:
 +   $ ./scripts/run-qemu.sh -r /
 +
 +   Run the same with SDL:
 +   $ ./scripts/run-qemu.sh -r / --sdl
 +   
 +   Or with a PPC build:
 +   $ ARCH=ppc ./scripts/run-qemu.sh -r /
 +   
 +   PPC with a mac99 model by passing options to QEMU:
 +   $ ARCH=ppc ./scripts/run-qemu.sh -r / -- -M mac99
 +
 +}
 +
 +function require_config() {
 +   if [ $(grep CONFIG_$1=y .config) ]; then
 +   return
 +   fi
 +
 +   echo You need to enable CONFIG_$1 for run-qemu to work properly
 +   exit 1
 +}
 +
 +function has_config() {
 +   grep -q CONFIG_$1=y .config
 +}
 +
 +function drive_if() {
 +   if has_config VIRTIO_BLK; then
 +   echo virtio
 +   elif has_config ATA_PIIX; then
 +   echo ide

 + require_config BLK_DEV_SD

 Maybe there should also be a warning if no standard FS (ext[34], btrfs,
 xfs etc.) is build into the kernel.

 Another thing, but that's just a recommendation for initrd-free mode:
 DEVTMPFS_MOUNT

 +   else
 +   echo \
 +Your kernel must have either VIRTIO_BLK or ATA_PIIX
 +enabled for block device assignment 2
 +   exit 1
 +   fi
 +}
 +
 +GETOPT=`getopt -o a:d:Dhr:sS:v --long 
 append,disk:,no-gdb,help,root:,sdl,smp:,vnc \
 +   -n $(basename \$0\) -- $@`
 +
 +if [ $? != 0 ]; then
 +   echo Terminating... 2
 +   exit 1
 +fi
 +
 +eval set -- $GETOPT
 +
 +while true; do
 +   case $1 in
 +   -a|--append)
 +   

Re: [Qemu-devel] [PATCH 07/25] qdev: use object_property_print in info qtree

2012-05-11 Thread Andreas Färber
Am 03.04.2012 15:05, schrieb Paolo Bonzini:
 Il 03/04/2012 14:28, Jan Kiszka ha scritto:
  if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
  value = object_property_get_str(OBJECT(dev), legacy_name, 
 err);
 This is not a criticism on this patch, but I'd like to underline that
 the line above only works because legacy props also define print
  
 
 You mean always, I guess.
 
 But this statement above is still inconsistent. We should either
 assert(print  parse) or handle their non-existence properly.
 
 Yes, asserting that print/parse are always present in pairs (if at all)
 would be good.

Paolo, has this issue been addressed somewhere?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [RFC PATCH] qemu pci: pci_add_capability enhancement to prevent damaging config space

2012-05-11 Thread Alexander Graf

On 11.05.2012, at 14:47, Alexey Kardashevskiy wrote:

 11.05.2012 20:52, Alexander Graf написал:
 
 On 11.05.2012, at 08:45, Alexey Kardashevskiy wrote:
 
 Normally the pci_add_capability is called on devices to add new
 capability. This is ok for emulated devices which capabilities list
 is being built by QEMU.
 
 In the case of VFIO the capability may already exist and adding new
 capability into the beginning of the linked list may create a loop.
 
 For example, the old code destroys the following config
 of PCIe Intel E1000E:
 
 before adding PCI_CAP_ID_MSI (0x05):
 0x34: 0xC8
 0xC8: 0x01 0xD0
 0xD0: 0x05 0xE0
 0xE0: 0x10 0x00
 
 after:
 0x34: 0xD0
 0xC8: 0x01 0xD0
 0xD0: 0x05 0xC8
 0xE0: 0x10 0x00
 
 As result capabilities 0x01 and 0x05 point to each other.
 
 The proposed patch does not change capability pointers when
 the same type capability is about to add.
 
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---
 hw/pci.c |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)
 
 diff --git a/hw/pci.c b/hw/pci.c
 index aa0c0b8..1f7c924 100644
 --- a/hw/pci.c
 +++ b/hw/pci.c
 @@ -1794,10 +1794,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t 
 cap_id,
}
 
config = pdev-config + offset;
 -config[PCI_CAP_LIST_ID] = cap_id;
 -config[PCI_CAP_LIST_NEXT] = pdev-config[PCI_CAPABILITY_LIST];
 -pdev-config[PCI_CAPABILITY_LIST] = offset;
 -pdev-config[PCI_STATUS] |= PCI_STATUS_CAP_LIST;
 +if (config[PCI_CAP_LIST_ID] != cap_id) {
 
 This doesn't scale. Capabilities are a list of CAPs. You'll have to do a 
 loop through all capabilities, check if the one you want to add is there 
 already and if so either
  * replace the existing one or
  * drop out and not write the new one in.

  * hw_error :)

 
 I'm not sure which way would be more natural.
 
 There is a third option - add another function, lets call it
 pci_fixup_capability() which would do whatever pci_add_capability() does
 but won't touch list pointers.

What good is a function that breaks internal consistency?

 When vfio, pci_add_capability() is called from the code which knows
 exactly that the capability exists and where it is and it calls
 pci_add_capability() based on this knowledge so doing additional loops
 just for imaginery scalability is a bit weird, no?

Not sure I understand your proposal. The more generic a framework is, the 
better, no? In this code path we don't care about speed. We only care about 
consistency and reliability.


Alex




Re: [Qemu-devel] [Xen-devel] [PATCH] qemu/xendisk: set maximum number of grants to be used

2012-05-11 Thread Jan Beulich
 On 11.05.12 at 09:19, Jan Beulich jbeul...@suse.com wrote:
 Legacy (non-pvops) gntdev drivers may require this to be done when the
 number of grants intended to be used simultaneously exceeds a certain
 driver specific default limit.
 
 Signed-off-by: Jan Beulich jbeul...@suse.com
 
 --- a/hw/xen_disk.c
 +++ b/hw/xen_disk.c
 @@ -536,6 +536,10 @@ static void blk_alloc(struct XenDevice *
  if (xen_mode != XEN_EMULATE) {
  batch_maps = 1;
  }
 +if (xc_gnttab_set_max_grants(xendev-gnttabdev,
 +max_requests * BLKIF_MAX_SEGMENTS_PER_REQUEST + 1)  0)

In more extensive testing it appears that very rarely this value is still
too low:

xen be: qdisk-768: can't map 11 grant refs (Cannot allocate memory, 342 maps)

342 + 11 = 353  352 = 32 * 11

Could someone help out here? I first thought this might be due to
use_aio being non-zero, but ioreq_start() doesn't permit more than
max_requests struct ioreqs-s to be around.

Additionally, shouldn't the driver be smarter and gracefully handle
grant mapping failures (as the per-domain map track table in the
hypervisor is a finite resource)?

Jan

 +xen_be_printf(xendev, 0, xc_gnttab_set_max_grants failed: %s\n,
 +  strerror(errno));
  }
  
  static int blk_init(struct XenDevice *xendev)






Re: [Qemu-devel] [PATCH 08/21] qdev: use object_property_print in info qtree

2012-05-11 Thread Andreas Färber
Am 02.05.2012 13:31, schrieb Paolo Bonzini:
 Otherwise, non-string properties without a legacy counterpart are missed.
 Also fix error propagation in object_property_print itself.
 
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  hw/qdev-monitor.c |2 +-
  qom/object.c  |2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
 index 41b9e2c..eed781d 100644
 --- a/hw/qdev-monitor.c
 +++ b/hw/qdev-monitor.c
 @@ -493,7 +493,7 @@ static void qdev_print_props(Monitor *mon, DeviceState 
 *dev, Property *props,
  if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
  value = object_property_get_str(OBJECT(dev), legacy_name, err);
  } else {
 -value = object_property_get_str(OBJECT(dev), props-name, err);
 +value = object_property_print(OBJECT(dev), props-name, err);
  }
  g_free(legacy_name);
  
 diff --git a/qom/object.c b/qom/object.c
 index 464fc8f..b4f6c1d 100644
 --- a/qom/object.c
 +++ b/qom/object.c
 @@ -839,7 +839,7 @@ char *object_property_print(Object *obj, const char *name,
  char *string;

char *string = NULL;

  
  mo = string_output_visitor_new();
 -object_property_get(obj, string_output_get_visitor(mo), name, NULL);
 +object_property_get(obj, string_output_get_visitor(mo), name, errp);

If we do error checking we should be consequent and do:

if (!error_is_set(errp)) {

  string = string_output_get_string(mo);

}

  string_output_visitor_cleanup(mo);
  return string;

Otherwise looks good and a 1.1 candidate.

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH] qcow2: Don't ignore failure to clear autoclear flags

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

diff --git a/block/qcow2.c b/block/qcow2.c
index 3bae2d8..655799c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -300,7 +300,10 @@ static int qcow2_open(BlockDriverState *bs, int flags)
 
 if (!bs-read_only  s-autoclear_features != 0) {
 s-autoclear_features = 0;
-qcow2_update_header(bs);
+ret = qcow2_update_header(bs);
+if (ret  0) {
+goto fail;
+}
 }
 
 /* Check support for various header values */
-- 
1.7.6.5




[Qemu-devel] [PATCH] xhci: Clean up reset function

2012-05-11 Thread Jan Kiszka
Properly register reset function via the device class.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---

Broken out from the MSI series where I will no longer touch xhci.

 hw/usb/hcd-xhci.c |9 -
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 5cf1a64..4bc1e0e 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -2296,9 +2296,9 @@ static void xhci_update_port(XHCIState *xhci, XHCIPort 
*port, int is_detach)
 }
 }
 
-static void xhci_reset(void *opaque)
+static void xhci_reset(DeviceState *dev)
 {
-XHCIState *xhci = opaque;
+XHCIState *xhci = DO_UPCAST(XHCIState, pci_dev.qdev, dev);
 int i;
 
 DPRINTF(xhci: full reset\n);
@@ -2506,7 +2506,7 @@ static void xhci_oper_write(XHCIState *xhci, uint32_t 
reg, uint32_t val)
 }
 xhci-usbcmd = val  0xc0f;
 if (val  USBCMD_HCRST) {
-xhci_reset(xhci);
+xhci_reset(xhci-pci_dev.qdev);
 }
 xhci_irq_update(xhci);
 break;
@@ -2831,8 +2831,6 @@ static void usb_xhci_init(XHCIState *xhci, DeviceState 
*dev)
 for (i = 0; i  MAXSLOTS; i++) {
 xhci-slots[i].enabled = 0;
 }
-
-qemu_register_reset(xhci_reset, xhci);
 }
 
 static int usb_xhci_initfn(struct PCIDevice *dev)
@@ -2895,6 +2893,7 @@ static void xhci_class_init(ObjectClass *klass, void 
*data)
 
 dc-vmsd= vmstate_xhci;
 dc-props   = xhci_properties;
+dc-reset   = xhci_reset;
 k-init = usb_xhci_initfn;
 k-vendor_id= PCI_VENDOR_ID_NEC;
 k-device_id= PCI_DEVICE_ID_NEC_UPD720200;
-- 
1.7.3.4



[Qemu-devel] [PATCH v3 0/8] msi: Refactorings and reset fixes

2012-05-11 Thread Jan Kiszka
v3 is v2 - xhci changes as MSI is instable there anyway. So, only
patches 1 and 2 are for 1.1/stable now.

CC: Alexander Graf ag...@suse.de
CC: Gerd Hoffmann kra...@redhat.com
CC: Isaku Yamahata yamah...@valinux.co.jp
CC: qemu-sta...@nongnu.org

Jan Kiszka (8):
  ahci: Fix reset of MSI function
  intel-hda: Fix reset of MSI function
  ahci: Clean up reset functions
  msi: Guard msi_reset with msi_present
  msi: Invoke msi/msix_reset from PCI core
  msi: Guard msi/msix_write_config with msi_present
  msi: Invoke msi/msix_write_config from PCI core
  msi: Use msi/msix_present more consistently

 hw/ide/ahci.c   |   25 +++--
 hw/ide/ahci.h   |2 +-
 hw/ide/ich.c|   19 ---
 hw/intel-hda.c  |   12 
 hw/ioh3420.c|3 +--
 hw/msi.c|   11 ---
 hw/msix.c   |   15 +--
 hw/pci.c|8 
 hw/pci_bridge.c |4 
 hw/virtio-pci.c |3 ---
 hw/xio3130_downstream.c |3 +--
 hw/xio3130_upstream.c   |3 +--
 12 files changed, 56 insertions(+), 52 deletions(-)

-- 
1.7.3.4




[Qemu-devel] [PATCH v3 2/8] intel-hda: Fix reset of MSI function

2012-05-11 Thread Jan Kiszka
Call msi_reset on device reset as still required by the core.

CC: Gerd Hoffmann kra...@redhat.com
CC: qemu-sta...@nongnu.org
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/intel-hda.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index bb11af2..e38861e 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -1107,6 +1107,9 @@ static void intel_hda_reset(DeviceState *dev)
 DeviceState *qdev;
 HDACodecDevice *cdev;
 
+if (d-msi) {
+msi_reset(d-pci);
+}
 intel_hda_regs_reset(d);
 d-wall_base_ns = qemu_get_clock_ns(vm_clock);
 
-- 
1.7.3.4




[Qemu-devel] [PATCH v3 8/8] msi: Use msi/msix_present more consistently

2012-05-11 Thread Jan Kiszka
Replace some open-coded msi/msix_present checks.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/msi.c  |2 +-
 hw/msix.c |   13 -
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/msi.c b/hw/msi.c
index 556c7c4..5233204 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -175,7 +175,7 @@ void msi_uninit(struct PCIDevice *dev)
 uint16_t flags;
 uint8_t cap_size;
 
-if (!(dev-cap_present  QEMU_PCI_CAP_MSI)) {
+if (!msi_present(dev)) {
 return;
 }
 flags = pci_get_word(dev-config + msi_flags_off(dev));
diff --git a/hw/msix.c b/hw/msix.c
index 84915d8..7c314bb 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -285,8 +285,9 @@ static void msix_free_irq_entries(PCIDevice *dev)
 /* Clean up resources for the device. */
 int msix_uninit(PCIDevice *dev, MemoryRegion *bar)
 {
-if (!(dev-cap_present  QEMU_PCI_CAP_MSIX))
+if (!msix_present(dev)) {
 return 0;
+}
 pci_del_capability(dev, PCI_CAP_ID_MSIX, MSIX_CAP_LENGTH);
 dev-msix_cap = 0;
 msix_free_irq_entries(dev);
@@ -305,7 +306,7 @@ void msix_save(PCIDevice *dev, QEMUFile *f)
 {
 unsigned n = dev-msix_entries_nr;
 
-if (!(dev-cap_present  QEMU_PCI_CAP_MSIX)) {
+if (!msix_present(dev)) {
 return;
 }
 
@@ -318,7 +319,7 @@ void msix_load(PCIDevice *dev, QEMUFile *f)
 {
 unsigned n = dev-msix_entries_nr;
 
-if (!(dev-cap_present  QEMU_PCI_CAP_MSIX)) {
+if (!msix_present(dev)) {
 return;
 }
 
@@ -370,8 +371,9 @@ void msix_notify(PCIDevice *dev, unsigned vector)
 
 void msix_reset(PCIDevice *dev)
 {
-if (!(dev-cap_present  QEMU_PCI_CAP_MSIX))
+if (!msix_present(dev)) {
 return;
+}
 msix_free_irq_entries(dev);
 dev-config[dev-msix_cap + MSIX_CONTROL_OFFSET] =
~dev-wmask[dev-msix_cap + MSIX_CONTROL_OFFSET];
@@ -410,7 +412,8 @@ void msix_vector_unuse(PCIDevice *dev, unsigned vector)
 
 void msix_unuse_all_vectors(PCIDevice *dev)
 {
-if (!(dev-cap_present  QEMU_PCI_CAP_MSIX))
+if (!msix_present(dev)) {
 return;
+}
 msix_free_irq_entries(dev);
 }
-- 
1.7.3.4




Re: [Qemu-devel] [PATCH 08/21] qdev: use object_property_print in info qtree

2012-05-11 Thread Paolo Bonzini
Il 11/05/2012 16:20, Andreas Färber ha scritto:
 char *string = NULL;
 
   
   mo = string_output_visitor_new();
  -object_property_get(obj, string_output_get_visitor(mo), name, NULL);
  +object_property_get(obj, string_output_get_visitor(mo), name, errp);
 If we do error checking we should be consequent and do:
 
 if (!error_is_set(errp)) {
 
   string = string_output_get_string(mo);
 }
 
   string_output_visitor_cleanup(mo);

Or just say that it is part of the string_output_visitor interface that
in case of an error the returned string is NULL.

Paolo



Re: [Qemu-devel] [PATCH] qom: fix refcounting in object_property_del_child()

2012-05-11 Thread Amos Kong
On 05/11/2012 02:42 PM, Paolo Bonzini wrote:
 Il 11/05/2012 04:15, Amos Kong ha scritto:
 Start VM with 8 multiple-function block devs, hot-removing
 those block devs by 'device_del ...' would cause qemu abort.

 object_ref() is called in object_property_add_child(),
 but we don't unref it in object_property_del_child().

 | (qemu) device_del virti0-0-0
 | (qemu) **
 | ERROR:qom/object.c:389:object_delete: assertion failed: (obj-ref == 0)

 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  qom/object.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

 diff --git a/qom/object.c b/qom/object.c
 index e721fc2..9da6b59 100644
 --- a/qom/object.c
 +++ b/qom/object.c
 @@ -320,6 +320,7 @@ static void object_property_del_child(Object *obj, 
 Object *child, Error **errp)
  QTAILQ_FOREACH(prop, obj-properties, node) {
  if (strstart(prop-type, child, NULL)  prop-opaque == child) {
  object_property_del(obj, prop-name, errp);
 +object_unref(child);
 
 This should be called by object_finalize_child_property instead, can you
 check why this is not the case?

Yes, original ref/unref are right.
I will post another patch to fix this issue.


NAK this patch.


 Paolo

Thanks!

-- 
Amos.



[Qemu-devel] [PATCH] pci: unplug all devs of same slot once

2012-05-11 Thread Amos Kong
The whole PCI slot should be removed once. Currently only one func
is cleaned in pci_unplug_device(), if you try to remove a single
func by monitor cmd.

Start VM with 8 multiple-function block devs, hot-removing
those block devs by 'device_del ...' would cause qemu abort.

| (qemu) device_del virti0-0-0
| (qemu) **
|ERROR:qom/object.c:389:object_delete: assertion failed: (obj-ref == 0)

Execute 'device_del $blkid' in monitor
 \_handle_user_command()
\_qmp_device_del()
   \_qdev_unplug()
  \_pci_unplug_device()
   | //only one obj(func) is unpluged
   v //need process funcs here
   object_unparent()
\_object_finalize_child_property()

Guest sets pci dev by ioport write (eject from acpi)
 \_kvm_handle_io()
\_pciej_write()
  \_acpi_piix_eject_slot()
   |
   v  //all qdevs(funcs) will be free
 QTAILQ_FOREACH_SAFE(qdev, bus-children, sibling, next) {
 PCIDevice *dev = PCI_DEVICE(qdev);
 if (PCI_SLOT(dev-devfn) == slot) {
 qdev_free()

Signed-off-by: Amos Kong ak...@redhat.com
---
 hw/pci.c |   34 ++
 1 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index b706e69..960cf53 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1520,16 +1520,34 @@ static int pci_qdev_init(DeviceState *qdev)
 
 static int pci_unplug_device(DeviceState *qdev)
 {
-PCIDevice *dev = PCI_DEVICE(qdev);
-PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+DeviceState *dev, *next;
+PCIDevice *cur;
+int ret, slot = PCI_SLOT(PCI_DEVICE(qdev)-devfn);
+BusState *bus = qdev-parent_bus;
+
+ret = 0;
+QTAILQ_FOREACH_SAFE(dev, bus-children, sibling, next) {
+cur = PCI_DEVICE(dev);
+
+if (PCI_SLOT(cur-devfn) == slot) {
+PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(cur);
+if (pc-no_hotplug) {
+qerror_report(QERR_DEVICE_NO_HOTPLUG,
+  object_get_typename(OBJECT(dev)));
+return -1;
+}
 
-if (pc-no_hotplug) {
-qerror_report(QERR_DEVICE_NO_HOTPLUG, 
object_get_typename(OBJECT(dev)));
-return -1;
+object_unparent(OBJECT(cur));
+ret = cur-bus-hotplug(cur-bus-hotplug_qdev, cur,
+PCI_HOTPLUG_DISABLED);
+if (ret  0) {
+qerror_report(QERR_UNDEFINED_ERROR,
+  object_get_typename(OBJECT(dev)));
+break;
+}
+}
 }
-object_unparent(OBJECT(dev));
-return dev-bus-hotplug(dev-bus-hotplug_qdev, dev,
- PCI_HOTPLUG_DISABLED);
+return ret;
 }
 
 PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,




Re: [Qemu-devel] [PATCH] xhci: Clean up reset function

2012-05-11 Thread Gerd Hoffmann
On 05/11/12 16:36, Jan Kiszka wrote:
 Properly register reset function via the device class.

Patch added to usb patch queue.

thanks,
  Gerd



Re: [Qemu-devel] [PATCH v2 02/24] qom: add object_child_foreach

2012-05-11 Thread Andreas Färber
Am 12.04.2012 18:03, schrieb Andreas Färber:
 Am 11.04.2012 23:30, schrieb Paolo Bonzini:
 A utility function that will be used to implement hierarchical realization.

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com

Reviewed-by: Anthony Liguori aligu...@us.ibm.com

 ---
  include/qemu/object.h |   14 +-
  qom/object.c  |   17 +
  2 files changed, 30 insertions(+), 1 deletion(-)

 diff --git a/include/qemu/object.h b/include/qemu/object.h
 index f814521..470efe3 100644
 --- a/include/qemu/object.h
 +++ b/include/qemu/object.h
 @@ -918,6 +918,19 @@ void object_property_add_str(Object *obj, const char 
 *name,
   struct Error **errp);
  
  /**
 + * object_child_foreach:
 + * @obj: the object whose children will be navigated
 + * @fn: the iterator function to be called
 + * @opaque: an opaque value that will be passed to the iterator
 + *
 + * Call @fn passing each child of @obj and @opaque to it, until @fn returns
 + * non-zero.  Return the last value returned by @fn, or 0 if there is no
 + * child.

I've turned the last sentence into a gtk-doc Returns: statement.

 + */
 +int object_child_foreach(Object *obj, int (*fn)(Object *child, void 
 *opaque),
 + void *opaque);
 +
 +/**
   * container_get:
   * @path: path to the container
   *
 @@ -928,5 +941,4 @@ void object_property_add_str(Object *obj, const char 
 *name,
   */
  Object *container_get(const char *path);
  
 -
 
 Unrelated whitespace change.

Thanks, applied to qom-next (with whitespace change dropped):
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH] usb-host: handle guest-issued clear halt

2012-05-11 Thread Gerd Hoffmann
On 05/08/12 14:06, Gerd Hoffmann wrote:
 Most important here is to update our internal endpoint state so we know
 the endpoint isn't in halted state any more.  Without this usb-host
 tries to clear halt again with the next data transfer submitted.  Doing
 this twice is (a) not correct and (b) confuses some usb devices,
 rendering them non-functional in the guest.

Anthony, can you pick this directly?
It is the only 1.1 usb patch in the queue right now ...

cheers,
  Gerd




[Qemu-devel] [PATCH v3 5/8] msi: Invoke msi/msix_reset from PCI core

2012-05-11 Thread Jan Kiszka
There is no point in pushing this burden to the devices, they may rather
forget to call them (like intel-hda, ahci, xhci did). Instead, reset
functions are now called from pci_device_reset and pci_bridge_reset.
They do nothing if MSI/MSI-X is not in use.

CC: Alexander Graf ag...@suse.de
CC: Gerd Hoffmann kra...@redhat.com
CC: Isaku Yamahata yamah...@valinux.co.jp
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/ide/ich.c|1 -
 hw/intel-hda.c  |3 ---
 hw/ioh3420.c|2 +-
 hw/pci.c|5 +
 hw/pci_bridge.c |4 
 hw/virtio-pci.c |1 -
 hw/xio3130_downstream.c |2 +-
 hw/xio3130_upstream.c   |2 +-
 8 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index e7026bb..d3bc822 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -88,7 +88,6 @@ static void pci_ich9_reset(DeviceState *dev)
 {
 struct AHCIPCIState *d = DO_UPCAST(struct AHCIPCIState, card.qdev, dev);
 
-msi_reset(d-card);
 ahci_reset(d-ahci);
 }
 
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index e38861e..bb11af2 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -1107,9 +1107,6 @@ static void intel_hda_reset(DeviceState *dev)
 DeviceState *qdev;
 HDACodecDevice *cdev;
 
-if (d-msi) {
-msi_reset(d-pci);
-}
 intel_hda_regs_reset(d);
 d-wall_base_ns = qemu_get_clock_ns(vm_clock);
 
diff --git a/hw/ioh3420.c b/hw/ioh3420.c
index 1632d31..d1499da 100644
--- a/hw/ioh3420.c
+++ b/hw/ioh3420.c
@@ -81,7 +81,7 @@ static void ioh3420_write_config(PCIDevice *d,
 static void ioh3420_reset(DeviceState *qdev)
 {
 PCIDevice *d = PCI_DEVICE(qdev);
-msi_reset(d);
+
 ioh3420_aer_vector_update(d);
 pcie_cap_root_reset(d);
 pcie_cap_deverr_reset(d);
diff --git a/hw/pci.c b/hw/pci.c
index b706e69..2148245 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -31,6 +31,8 @@
 #include loader.h
 #include range.h
 #include qmp-commands.h
+#include msi.h
+#include msix.h
 
 //#define DEBUG_PCI
 #ifdef DEBUG_PCI
@@ -188,6 +190,9 @@ void pci_device_reset(PCIDevice *dev)
 }
 }
 pci_update_mappings(dev);
+
+msi_reset(dev);
+msix_reset(dev);
 }
 
 /*
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index 866f0b6..b533574 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -32,6 +32,8 @@
 #include pci_bridge.h
 #include pci_internals.h
 #include range.h
+#include msi.h
+#include msix.h
 
 /* PCI bridge subsystem vendor ID helper functions */
 #define PCI_SSVID_SIZEOF8
@@ -296,6 +298,8 @@ void pci_bridge_reset(DeviceState *qdev)
 {
 PCIDevice *dev = PCI_DEVICE(qdev);
 pci_bridge_reset_reg(dev);
+msi_reset(dev);
+msix_reset(dev);
 }
 
 /* default qdev initialization function for PCI-to-PCI bridge */
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 4a4413d..3395a02 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -277,7 +277,6 @@ void virtio_pci_reset(DeviceState *d)
 VirtIOPCIProxy *proxy = container_of(d, VirtIOPCIProxy, pci_dev.qdev);
 virtio_pci_stop_ioeventfd(proxy);
 virtio_reset(proxy-vdev);
-msix_reset(proxy-pci_dev);
 proxy-flags = ~VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
 }
 
diff --git a/hw/xio3130_downstream.c b/hw/xio3130_downstream.c
index 319624f..3716e45 100644
--- a/hw/xio3130_downstream.c
+++ b/hw/xio3130_downstream.c
@@ -48,7 +48,7 @@ static void xio3130_downstream_write_config(PCIDevice *d, 
uint32_t address,
 static void xio3130_downstream_reset(DeviceState *qdev)
 {
 PCIDevice *d = PCI_DEVICE(qdev);
-msi_reset(d);
+
 pcie_cap_deverr_reset(d);
 pcie_cap_slot_reset(d);
 pcie_cap_ari_reset(d);
diff --git a/hw/xio3130_upstream.c b/hw/xio3130_upstream.c
index 34a99bb..962d48e 100644
--- a/hw/xio3130_upstream.c
+++ b/hw/xio3130_upstream.c
@@ -47,7 +47,7 @@ static void xio3130_upstream_write_config(PCIDevice *d, 
uint32_t address,
 static void xio3130_upstream_reset(DeviceState *qdev)
 {
 PCIDevice *d = PCI_DEVICE(qdev);
-msi_reset(d);
+
 pci_bridge_reset(qdev);
 pcie_cap_deverr_reset(d);
 }
-- 
1.7.3.4




Re: [Qemu-devel] [PATCH v5 0/7] add fixed-width visitors and serialization tests/fixes

2012-05-11 Thread Michael Roth
On Fri, May 11, 2012 at 03:22:15AM +0200, Andreas Färber wrote:
 Am 27.04.2012 22:21, schrieb Michael Roth:
  These patches apply on top of qemu.git master, and can also be obtained 
  from:
  git://github.com/mdroth/qemu.git visitor-fixed-width-v5
  
  Some of these were being carried as part of Paolo's realize series due to 
  some
  conflicts, but that looks to be targetted for 1.2 now, and there's a QMP
  visitor bug and a small issue with String visitor that were caught by the 
  test
  infrastructure introduced here and fixed as part of this series, so I'd 
  like to
  get this in for 1.1
 
 Thanks, I've applied v6 to qom-next (as usual massaging the commit
 messages a bit, in particular extending the last one):
 http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next
 
 Reasoning:
 This series has been around since end of February. v3 fixed a breakage
 reported by Anthony (use of signed rather than unsigned visitors); since
 then changes were mostly rebasing, and v5/v6 pass make check and my
 smoke tests.
 While this is not strictly a QOM series, I am picking it as a
 prerequisite since Paolo had picked up patches 1, 6, 7 for his Object
 properties movement and because patch 1 is handy for newly added
 properties such as of the x86 CPU.
 Further, Paolo agreed to rebase onto this series.
 
 However, it is my understanding that patches 2 and 4 are independent
 bugfixes and as such should go into 1.1.

Agreed, my intention as well.

Thanks for taking these in!

 
 Luiz, should I send Anthony a PULL for 1.1-rc2 including those two? Can
 you ack then? Or do you want to cherry-pick them from qom-next yourself?
 
 Andreas
 
  CHANGES SINCE v4:
   - Rebased on master (a8b69b8e2431edfcb6c4cfb069787e9071d6235b) and 
  re-tested
   - Re-ordered patches so visitor bugs are applied before the test cases that
 that trigger them.
  
  CHANGES SINCE V3:
   - Rebased on master and re-tested
  
  CHANGES SINCE V2:
   - Fix qemu-test errors due to now-strict bounds-checking we doing 
  assignment
 between signed/unsigned types.
   - uint* property getters/setters no longer use int* getters/setters.
   - valid devfn range is now explicitly enforced.
  
  CHANGES SINCE V1:
   - unit tests: covert QmpOutputVisitor qobject to json before passing it to
 QmpInputVisitor*. I.e., actually do the serialization :)
   - QmpInputVisitor, add handling for when a serialized QFloat gets read back
 as a QInt
   - unit tests: add coverage for String visitor
   - StringOutputVisitor: use %f for float representation
  
  These patches add fixed-width visitor interfaces and switches all qdev users
  over to using them.
  
  We also add a test suite which covers these interfaces, and also does some
  sanity checking on Visitors (Qmp/String currently, with a pluggable 
  interface
  for future implementations) to ensure Visitor input/output handling remain
  self-consistent, which is not covered by the current visitor tests which 
  mostly
  test input/output seperately. Maintaining this invariant is necessary to 
  ensure
  that visitors can be used for serialization/deserialization in the future.
  
   hw/mc146818rtc.c   |7 -
   hw/pci.c   |2 +-
   hw/pci.h   |2 +-
   hw/qdev-addr.c |4 +-
   hw/qdev-properties.c   |  161 +---
   hw/qdev.h  |2 +-
   qapi/qapi-visit-core.c |  139 +++
   qapi/qapi-visit-core.h |   16 +
   qapi/qmp-input-visitor.c   |9 +-
   qapi/string-output-visitor.c   |2 +-
   tests/Makefile |4 +-
   tests/test-string-output-visitor.c |2 +-
   tests/test-visitor-serialization.c |  784 
  
   13 files changed, 1049 insertions(+), 85 deletions(-)
 
 -- 
 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
 GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
 



[Qemu-devel] [RFC PATCH 0/2] Split fdd devices off the floppy controller

2012-05-11 Thread Markus Armbruster
This is probaly more about QOM than about floppy.  It's not a working
solution, yet.  I'm posting it to give us something concrete to
discuss.

Based on Kevin's block branch, commit ad431215.

Markus Armbruster (2):
  Un-inline fdctrl_init_isa()
  Split fdd devices off the floppy controller

 hw/fdc.c  |  132 
 hw/fdc.h  |   24 +-
 hw/ide/piix.c |3 +-
 hw/isa.h  |2 -
 hw/pc_sysfw.c |1 +
 qemu-common.h |1 +
 6 files changed, 91 insertions(+), 72 deletions(-)

-- 
1.7.6.5




[Qemu-devel] [RFC PATCH 1/2] Un-inline fdctrl_init_isa()

2012-05-11 Thread Markus Armbruster

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/fdc.c  |   20 
 hw/fdc.h  |   24 ++--
 hw/ide/piix.c |3 ++-
 hw/isa.h  |2 --
 hw/pc_sysfw.c |1 +
 qemu-common.h |1 +
 6 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index cb4cd25..d9c4fbf 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1886,6 +1886,26 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl)
 return 0;
 }
 
+ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds)
+{
+ISADevice *dev;
+
+dev = isa_try_create(bus, isa-fdc);
+if (!dev) {
+return NULL;
+}
+
+if (fds[0]) {
+qdev_prop_set_drive_nofail(dev-qdev, driveA, fds[0]-bdrv);
+}
+if (fds[1]) {
+qdev_prop_set_drive_nofail(dev-qdev, driveB, fds[1]-bdrv);
+}
+qdev_init_nofail(dev-qdev);
+
+return dev;
+}
+
 void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
 target_phys_addr_t mmio_base, DriveInfo **fds)
 {
diff --git a/hw/fdc.h b/hw/fdc.h
index 55a8d73..1b32b17 100644
--- a/hw/fdc.h
+++ b/hw/fdc.h
@@ -1,32 +1,12 @@
 #ifndef HW_FDC_H
 #define HW_FDC_H
 
-#include isa.h
-#include blockdev.h
+#include qemu-common.h
 
 /* fdc.c */
 #define MAX_FD 2
 
-static inline ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds)
-{
-ISADevice *dev;
-
-dev = isa_try_create(bus, isa-fdc);
-if (!dev) {
-return NULL;
-}
-
-if (fds[0]) {
-qdev_prop_set_drive_nofail(dev-qdev, driveA, fds[0]-bdrv);
-}
-if (fds[1]) {
-qdev_prop_set_drive_nofail(dev-qdev, driveB, fds[1]-bdrv);
-}
-qdev_init_nofail(dev-qdev);
-
-return dev;
-}
-
+ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
 void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
 target_phys_addr_t mmio_base, DriveInfo **fds);
 void sun4m_fdctrl_init(qemu_irq irq, target_phys_addr_t io_base,
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index bcaa400..f5a74c2 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -22,11 +22,12 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+
 #include hw/hw.h
 #include hw/pc.h
 #include hw/pci.h
 #include hw/isa.h
-#include block.h
+#include blockdev.h
 #include sysemu.h
 #include dma.h
 
diff --git a/hw/isa.h b/hw/isa.h
index f7bc4b5..6c6fd7f 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -9,8 +9,6 @@
 
 #define ISA_NUM_IRQS 16
 
-typedef struct ISADevice ISADevice;
-
 #define TYPE_ISA_DEVICE isa-device
 #define ISA_DEVICE(obj) \
  OBJECT_CHECK(ISADevice, (obj), TYPE_ISA_DEVICE)
diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
index f0d7c21..b45f0ac 100644
--- a/hw/pc_sysfw.c
+++ b/hw/pc_sysfw.c
@@ -23,6 +23,7 @@
  * THE SOFTWARE.
  */
 
+#include blockdev.h
 #include sysbus.h
 #include hw.h
 #include pc.h
diff --git a/qemu-common.h b/qemu-common.h
index cccfb42..0bb1078 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -239,6 +239,7 @@ typedef struct VLANState VLANState;
 typedef struct VLANClientState VLANClientState;
 typedef struct i2c_bus i2c_bus;
 typedef struct ISABus ISABus;
+typedef struct ISADevice ISADevice;
 typedef struct SMBusDevice SMBusDevice;
 typedef struct PCIHostState PCIHostState;
 typedef struct PCIExpressHost PCIExpressHost;
-- 
1.7.6.5




[Qemu-devel] [RFC PATCH 2/2] Split fdd devices off the floppy controller

2012-05-11 Thread Markus Armbruster
For historical reasons, and unlike other block devices, our floppy
devices isa-fdc, sysbus-fdc and SUNW,fdtwo integrate the controller
and the drive(s) in a single qdev.  This makes them weird: we need
-global to set up floppy drives, unlike every other optional device.

This patch explores separating them.  It's not a working solution,
yet.  I'm posting it to give us something concrete to discuss.

Separating them involves splitting the per-drive state (struct FDrive)
into a part that belongs to the controller (remains in FDrive), and a
part that belongs to the drive (moves to new struct FDD).  I should
perhaps rename FDrive to reflect that.

An example for state that clearly belongs to the drive is the block
backend.  This patch moves it.  More members of FDrive need moving,
e.g. drive.  To be done in separate commits.  Might impact migration.

I put the fdd objects right into /machine/.  Maybe they should go
elsewhere.  For what it's worth, IDE drives are in
/machine/peripheral/.

Bug: I give all of them the same name fdd.  QOM happily accepts it.

Instead of definining a full-blown qbus to connect controller and
drives, I link them directly.

There's no use of QOM links in the tree, yet, and the QOM
documentation isn't terribly helpful there.  Please review my
guesswork.

I add one link per fdd the board supports, but I set it only for the
fdds actually present.  With one of two fdds present, qom-fuse shows:

$ ls -l machine/unattached/device\[18\]/fdd*
lrwxr-xr-x. 2 1000 1000 4096 Jan  1  1970 
machine/unattached/device[18]/fdd0 - ../../../machine/fdd
lrwxr-xr-x. 2 1000 1000 4096 Jan  1  1970 
machine/unattached/device[18]/fdd1 - ../../..

The second one is weird.

Unfortunately, eliding the qbus means I can't make the floppy disk a
qdev (sub-class of TYPE_DEVICE), because qdevs can only connect to a
qbus.  Anthony tells me that restriction is gone in his latest QOM
series.

Since it's not a qdev, -device fdd does not work.  Pity, because it
defeats the stated purpose of making floppy disk drives work like
other existing optional devices.

There doesn't seem to be a way to create QOM objects via command line
or monitor.

Speaking of monitor: the QOM commands are only implemented in QMP, and
they are entirely undocumented.  Sets a bad example; wonder how it got
past the maintainer ;)

Note: I *break* -global isa-fdc.driveA=...  The properties are simply
gone.  Fixable if we need backwards compatibility there.

The floppy controller device should probably be a child of a super I/O
device, grandchild of a south bridge device, or similar, depending on
the board.  Outside this commit's scope.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/fdc.c |  124 +++--
 1 files changed, 71 insertions(+), 53 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index d9c4fbf..98ff87a 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -54,6 +54,33 @@
 //
 /* Floppy drive emulation   */
 
+typedef struct FDD {
+Object obj;
+BlockDriverState *bs;
+} FDD;
+
+#define TYPE_FDD fdd
+
+static TypeInfo fdd_info = {
+.name  = TYPE_FDD,
+.parent= TYPE_OBJECT,
+.instance_size = sizeof(FDD),
+};
+
+static void fdd_create(Object *fdc, const char *link_name,
+   BlockDriverState *bs)
+{
+Object *obj = object_new(TYPE_FDD);
+FDD *fdd = OBJECT_CHECK(FDD, obj, TYPE_FDD);
+
+fdd-bs = bs;
+object_property_add_child(qdev_get_machine(), fdd, obj, NULL);
+object_property_set_link(fdc, obj, link_name, NULL);
+}
+
+//
+/* Intel 82078 floppy disk controller emulation  */
+
 #define GET_CUR_DRV(fdctrl) ((fdctrl)-cur_drv)
 #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)-cur_drv = (drive))
 
@@ -71,7 +98,7 @@ typedef enum FDiskFlags {
 
 typedef struct FDrive {
 FDCtrl *fdctrl;
-BlockDriverState *bs;
+FDD *fdd;
 /* Drive status */
 FDriveType drive;
 uint8_t perpendicular;/* 2.88 MB access mode*/
@@ -179,9 +206,9 @@ static void fd_revalidate(FDrive *drv)
 FDriveRate rate;
 
 FLOPPY_DPRINTF(revalidate\n);
-if (drv-bs != NULL  bdrv_is_inserted(drv-bs)) {
-ro = bdrv_is_read_only(drv-bs);
-bdrv_get_floppy_geometry_hint(drv-bs, nb_heads, max_track,
+if (drv-fdd != NULL  bdrv_is_inserted(drv-fdd-bs)) {
+ro = bdrv_is_read_only(drv-fdd-bs);
+bdrv_get_floppy_geometry_hint(drv-fdd-bs, nb_heads, max_track,
   last_sect, drv-drive, drive, rate);
 if (nb_heads != 0  max_track != 0  last_sect != 0) {
 FLOPPY_DPRINTF(User defined disk (%d %d %d),
@@ -208,9 +235,6 @@ static void fd_revalidate(FDrive *drv)
 }
 }
 
-//
-/* Intel 82078 floppy disk controller emulation  */
-
 static void 

Re: [Qemu-devel] [RFC PATCH 0/2] Split fdd devices off the floppy controller

2012-05-11 Thread Markus Armbruster
Forgot to cc: Paolo and Mike, apologies!



[Qemu-devel] [PATCH v3 3/8] ahci: Clean up reset functions

2012-05-11 Thread Jan Kiszka
Properly register reset functions via the device class.

CC: Alexander Graf ag...@suse.de
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/ide/ahci.c |   25 +++--
 hw/ide/ahci.h |2 +-
 hw/ide/ich.c  |   10 --
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index a883a92..e992d76 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -339,7 +339,7 @@ static void ahci_mem_write(void *opaque, target_phys_addr_t 
addr,
 case HOST_CTL: /* R/W */
 if (val  HOST_CTL_RESET) {
 DPRINTF(-1, HBA Reset\n);
-ahci_reset(container_of(s, AHCIPCIState, ahci));
+ahci_reset(s);
 } else {
 s-control_regs.ghc = (val  0x3) | HOST_CTL_AHCI_EN;
 ahci_check_irq(s);
@@ -1149,21 +1149,20 @@ void ahci_uninit(AHCIState *s)
 g_free(s-dev);
 }
 
-void ahci_reset(void *opaque)
+void ahci_reset(AHCIState *s)
 {
-struct AHCIPCIState *d = opaque;
 AHCIPortRegs *pr;
 int i;
 
-d-ahci.control_regs.irqstatus = 0;
-d-ahci.control_regs.ghc = 0;
+s-control_regs.irqstatus = 0;
+s-control_regs.ghc = 0;
 
-for (i = 0; i  d-ahci.ports; i++) {
-pr = d-ahci.dev[i].port_regs;
+for (i = 0; i  s-ports; i++) {
+pr = s-dev[i].port_regs;
 pr-irq_stat = 0;
 pr-irq_mask = 0;
 pr-scr_ctl = 0;
-ahci_reset_port(d-ahci, i);
+ahci_reset_port(s, i);
 }
 }
 
@@ -1178,6 +1177,13 @@ static const VMStateDescription vmstate_sysbus_ahci = {
 .unmigratable = 1,
 };
 
+static void sysbus_ahci_reset(DeviceState *dev)
+{
+SysbusAHCIState *s = DO_UPCAST(SysbusAHCIState, busdev.qdev, dev);
+
+ahci_reset(s-ahci);
+}
+
 static int sysbus_ahci_init(SysBusDevice *dev)
 {
 SysbusAHCIState *s = FROM_SYSBUS(SysbusAHCIState, dev);
@@ -1185,8 +1191,6 @@ static int sysbus_ahci_init(SysBusDevice *dev)
 
 sysbus_init_mmio(dev, s-ahci.mem);
 sysbus_init_irq(dev, s-ahci.irq);
-
-qemu_register_reset(ahci_reset, s-ahci);
 return 0;
 }
 
@@ -1203,6 +1207,7 @@ static void sysbus_ahci_class_init(ObjectClass *klass, 
void *data)
 sbc-init = sysbus_ahci_init;
 dc-vmsd = vmstate_sysbus_ahci;
 dc-props = sysbus_ahci_properties;
+dc-reset = sysbus_ahci_reset;
 }
 
 static TypeInfo sysbus_ahci_info = {
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index b223d2c..ec1b6a5 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -332,6 +332,6 @@ typedef struct NCQFrame {
 void ahci_init(AHCIState *s, DeviceState *qdev, int ports);
 void ahci_uninit(AHCIState *s);
 
-void ahci_reset(void *opaque);
+void ahci_reset(AHCIState *s);
 
 #endif /* HW_IDE_AHCI_H */
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 242254e..e7026bb 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -84,12 +84,12 @@ static const VMStateDescription vmstate_ahci = {
 .unmigratable = 1,
 };
 
-static void pci_ich9_reset(void *opaque)
+static void pci_ich9_reset(DeviceState *dev)
 {
-struct AHCIPCIState *d = opaque;
+struct AHCIPCIState *d = DO_UPCAST(struct AHCIPCIState, card.qdev, dev);
 
 msi_reset(d-card);
-ahci_reset(opaque);
+ahci_reset(d-ahci);
 }
 
 static int pci_ich9_ahci_init(PCIDevice *dev)
@@ -110,8 +110,6 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
 /* XXX Software should program this register */
 d-card.config[0x90]   = 1  6; /* Address Map Register - AHCI mode */
 
-qemu_register_reset(pci_ich9_reset, d);
-
 msi_init(dev, 0x50, 1, true, false);
 d-ahci.irq = d-card.irq[0];
 
@@ -141,7 +139,6 @@ static int pci_ich9_uninit(PCIDevice *dev)
 d = DO_UPCAST(struct AHCIPCIState, card, dev);
 
 msi_uninit(dev);
-qemu_unregister_reset(pci_ich9_reset, d);
 ahci_uninit(d-ahci);
 
 return 0;
@@ -167,6 +164,7 @@ static void ich_ahci_class_init(ObjectClass *klass, void 
*data)
 k-revision = 0x02;
 k-class_id = PCI_CLASS_STORAGE_SATA;
 dc-vmsd = vmstate_ahci;
+dc-reset = pci_ich9_reset;
 }
 
 static TypeInfo ich_ahci_info = {
-- 
1.7.3.4




[Qemu-devel] [PATCH v3 6/8] msi: Guard msi/msix_write_config with msi_present

2012-05-11 Thread Jan Kiszka
Terminate msi/msix_write_config early if support is not enabled. This
allows to remove checks at the caller site if MSI is optional.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/msi.c  |3 ++-
 hw/msix.c |2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/msi.c b/hw/msi.c
index b2903fc..da12f33 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -276,7 +276,8 @@ void msi_write_config(PCIDevice *dev, uint32_t addr, 
uint32_t val, int len)
 unsigned int vector;
 uint32_t pending;
 
-if (!ranges_overlap(addr, len, dev-msi_cap, msi_cap_sizeof(flags))) {
+if (!msi_present(dev) ||
+!ranges_overlap(addr, len, dev-msi_cap, msi_cap_sizeof(flags))) {
 return;
 }
 
diff --git a/hw/msix.c b/hw/msix.c
index 3835eaa..84915d8 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -157,7 +157,7 @@ void msix_write_config(PCIDevice *dev, uint32_t addr,
 int vector;
 bool was_masked;
 
-if (!range_covers_byte(addr, len, enable_pos)) {
+if (!msix_present(dev) || !range_covers_byte(addr, len, enable_pos)) {
 return;
 }
 
-- 
1.7.3.4




[Qemu-devel] [PATCH v3 1/8] ahci: Fix reset of MSI function

2012-05-11 Thread Jan Kiszka
Call msi_reset on device reset as still required by the core.

CC: Alexander Graf ag...@suse.de
CC: qemu-sta...@nongnu.org
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/ide/ich.c |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 560ae37..242254e 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -84,6 +84,14 @@ static const VMStateDescription vmstate_ahci = {
 .unmigratable = 1,
 };
 
+static void pci_ich9_reset(void *opaque)
+{
+struct AHCIPCIState *d = opaque;
+
+msi_reset(d-card);
+ahci_reset(opaque);
+}
+
 static int pci_ich9_ahci_init(PCIDevice *dev)
 {
 struct AHCIPCIState *d;
@@ -102,7 +110,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
 /* XXX Software should program this register */
 d-card.config[0x90]   = 1  6; /* Address Map Register - AHCI mode */
 
-qemu_register_reset(ahci_reset, d);
+qemu_register_reset(pci_ich9_reset, d);
 
 msi_init(dev, 0x50, 1, true, false);
 d-ahci.irq = d-card.irq[0];
@@ -133,7 +141,7 @@ static int pci_ich9_uninit(PCIDevice *dev)
 d = DO_UPCAST(struct AHCIPCIState, card, dev);
 
 msi_uninit(dev);
-qemu_unregister_reset(ahci_reset, d);
+qemu_unregister_reset(pci_ich9_reset, d);
 ahci_uninit(d-ahci);
 
 return 0;
-- 
1.7.3.4




[Qemu-devel] [PATCH v3 4/8] msi: Guard msi_reset with msi_present

2012-05-11 Thread Jan Kiszka
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/msi.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/hw/msi.c b/hw/msi.c
index 5d6ceb6..b2903fc 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -191,6 +191,10 @@ void msi_reset(PCIDevice *dev)
 uint16_t flags;
 bool msi64bit;
 
+if (!msi_present(dev)) {
+return;
+}
+
 flags = pci_get_word(dev-config + msi_flags_off(dev));
 flags = ~(PCI_MSI_FLAGS_QSIZE | PCI_MSI_FLAGS_ENABLE);
 msi64bit = flags  PCI_MSI_FLAGS_64BIT;
-- 
1.7.3.4




[Qemu-devel] [PATCH v3 7/8] msi: Invoke msi/msix_write_config from PCI core

2012-05-11 Thread Jan Kiszka
Also this functions is better invoked by the core than by each and every
device. This allows to drop the config_write callbacks from ich and
intel-hda.

CC: Alexander Graf ag...@suse.de
CC: Gerd Hoffmann kra...@redhat.com
CC: Isaku Yamahata yamah...@valinux.co.jp
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/ide/ich.c|8 
 hw/intel-hda.c  |   12 
 hw/ioh3420.c|1 -
 hw/msi.c|2 +-
 hw/pci.c|3 +++
 hw/virtio-pci.c |2 --
 hw/xio3130_downstream.c |1 -
 hw/xio3130_upstream.c   |1 -
 8 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index d3bc822..e3eaaea 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -143,13 +143,6 @@ static int pci_ich9_uninit(PCIDevice *dev)
 return 0;
 }
 
-static void pci_ich9_write_config(PCIDevice *pci, uint32_t addr,
-  uint32_t val, int len)
-{
-pci_default_write_config(pci, addr, val, len);
-msi_write_config(pci, addr, val, len);
-}
-
 static void ich_ahci_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -157,7 +150,6 @@ static void ich_ahci_class_init(ObjectClass *klass, void 
*data)
 
 k-init = pci_ich9_ahci_init;
 k-exit = pci_ich9_uninit;
-k-config_write = pci_ich9_write_config;
 k-vendor_id = PCI_VENDOR_ID_INTEL;
 k-device_id = PCI_DEVICE_ID_INTEL_82801IR;
 k-revision = 0x02;
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index bb11af2..8f3b70b 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -1153,17 +1153,6 @@ static int intel_hda_exit(PCIDevice *pci)
 return 0;
 }
 
-static void intel_hda_write_config(PCIDevice *pci, uint32_t addr,
-   uint32_t val, int len)
-{
-IntelHDAState *d = DO_UPCAST(IntelHDAState, pci, pci);
-
-pci_default_write_config(pci, addr, val, len);
-if (d-msi) {
-msi_write_config(pci, addr, val, len);
-}
-}
-
 static int intel_hda_post_load(void *opaque, int version)
 {
 IntelHDAState* d = opaque;
@@ -1252,7 +1241,6 @@ static void intel_hda_class_init(ObjectClass *klass, void 
*data)
 
 k-init = intel_hda_init;
 k-exit = intel_hda_exit;
-k-config_write = intel_hda_write_config;
 k-vendor_id = PCI_VENDOR_ID_INTEL;
 k-device_id = 0x2668;
 k-revision = 1;
diff --git a/hw/ioh3420.c b/hw/ioh3420.c
index d1499da..0a2601c 100644
--- a/hw/ioh3420.c
+++ b/hw/ioh3420.c
@@ -71,7 +71,6 @@ static void ioh3420_write_config(PCIDevice *d,
 pci_get_long(d-config + d-exp.aer_cap + PCI_ERR_ROOT_COMMAND);
 
 pci_bridge_write_config(d, address, val, len);
-msi_write_config(d, address, val, len);
 ioh3420_aer_vector_update(d);
 pcie_cap_slot_write_config(d, address, val, len);
 pcie_aer_write_config(d, address, val, len);
diff --git a/hw/msi.c b/hw/msi.c
index da12f33..556c7c4 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -264,7 +264,7 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
 stl_le_phys(address, data);
 }
 
-/* call this function after updating configs by pci_default_write_config(). */
+/* Normally called by pci_default_write_config(). */
 void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len)
 {
 uint16_t flags = pci_get_word(dev-config + msi_flags_off(dev));
diff --git a/hw/pci.c b/hw/pci.c
index 2148245..439f3ce 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1042,6 +1042,9 @@ void pci_default_write_config(PCIDevice *d, uint32_t 
addr, uint32_t val, int l)
 
 if (range_covers_byte(addr, l, PCI_COMMAND))
 pci_update_irq_disabled(d, was_irq_disabled);
+
+msi_write_config(d, addr, val, l);
+msix_write_config(d, addr, val, l);
 }
 
 /***/
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 3395a02..985acfc 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -519,8 +519,6 @@ static void virtio_write_config(PCIDevice *pci_dev, 
uint32_t address,
 virtio_set_status(proxy-vdev,
   proxy-vdev-status  ~VIRTIO_CONFIG_S_DRIVER_OK);
 }
-
-msix_write_config(pci_dev, address, val, len);
 }
 
 static unsigned virtio_pci_get_features(void *opaque)
diff --git a/hw/xio3130_downstream.c b/hw/xio3130_downstream.c
index 3716e45..56d1b35 100644
--- a/hw/xio3130_downstream.c
+++ b/hw/xio3130_downstream.c
@@ -41,7 +41,6 @@ static void xio3130_downstream_write_config(PCIDevice *d, 
uint32_t address,
 pci_bridge_write_config(d, address, val, len);
 pcie_cap_flr_write_config(d, address, val, len);
 pcie_cap_slot_write_config(d, address, val, len);
-msi_write_config(d, address, val, len);
 pcie_aer_write_config(d, address, val, len);
 }
 
diff --git a/hw/xio3130_upstream.c b/hw/xio3130_upstream.c
index 962d48e..7972581 100644
--- a/hw/xio3130_upstream.c
+++ b/hw/xio3130_upstream.c
@@ -40,7 +40,6 @@ static void 

Re: [Qemu-devel] [SeaBIOS] [RESEND PATCH v3] hotplug: add device per func in ACPI DSDT tables

2012-05-11 Thread Jiang Liu
On 05/11/2012 06:14 PM, Gleb Natapov wrote:
 I'm not familiar with qemu:(
 On native OS, admin could trigger PCI device hotplug operations through
 /sys/bus/pci/slot/xx/power. Not sure whether that's needed for guest OS too. 

 Why is it needed on physical HW? May be it is needed in a VM for the
 same reason?
As Amos has mentioned, it's used power on/off a PCI device instead of physical
hotplug. Not sure whether it's needed in guest OS.
--gerry



Re: [Qemu-devel] [PATCH] linux-user: Fix stale tbs after mmap

2012-05-11 Thread Peter Maydell
On 7 May 2012 12:38, Alexander Graf ag...@suse.de wrote:

 On 07.05.2012, at 13:32, Alexander Graf wrote:


 On 07.05.2012, at 12:37, Peter Maydell wrote:

 On 7 May 2012 10:30, Alexander Graf ag...@suse.de wrote:
 @@ -587,6 +587,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
 int prot,
    page_dump(stdout);
    printf(\n);
 #endif
 +    tb_invalidate_phys_page_range(start, start + len, 0);
    mmap_unlock();
    return start;

 The comment at the top of tb_invalidate_phys_page_range() says
 start and end must refer to the same physical page -- is it
 out of date or does that not apply to user-mode?

 Do you need to also invalidate the range on munmap() and
 mprotect-to-not-executable in order to correctly fault on
 the case of:
 map something
 execute it
 unmap it
 try to execute it again

 ? (haven't tested that case but it seems like it might be an issue)

 Yeah, the issue does exist:

 And the below patch on top of my revised patch fixes it.

I think these two patches look correct (and as you pointed out
on irc I was wrong about mprotect, which effectively already
handles flushing the tb if needed). If you can roll them together
into a single patch with a commit message and signed-off-by
you can add my Reviewed-by: tag to it.

thanks
-- PMM



[Qemu-devel] [PATCH] KVM: Add wrapper script around QEMU to test kernels

2012-05-11 Thread Alexander Graf
On LinuxCon I had a nice chat with Linus on what he thinks kvm-tool
would be doing and what he expects from it. Basically he wants a
small and simple tool he and other developers can run to try out and
see if the kernel they just built actually works.

Fortunately, QEMU can do that today already! The only piece that was
missing was the simple piece of the equation, so here is a script
that wraps around QEMU and executes a kernel you just built.

If you do have KVM around and are not cross-compiling, it will use
KVM. But if you don't, you can still fall back to emulation mode and
at least check if your kernel still does what you expect. I only
implemented support for s390x and ppc there, but it's easily extensible
to more platforms, as QEMU can emulate (and virtualize) pretty much
any platform out there.

If you don't have qemu installed, please do so before using this script. Your
distro should provide a package for it (might even call it kvm). If not,
just compile it from source - it's not hard!

To quickly get going, just execute the following as user:

$ ./tools/testing/run-qemu/run-qemu.sh -r / -a init=/bin/bash

This will drop you into a shell on your rootfs.

Or you can also use a ready made image:

$ IMG=http://people.debian.org/~aurel32/qemu/amd64
$ IMG=$IMG/debian_squeeze_amd64_standard.qcow2
$ ./tools/testing/run-qemu/run-qemu.sh -d $IMG \
-a root=/dev/vda1 init=/bin/bash

Keep in mind that http is read-only, so no writing to the image :).

Either way, you provide the kernel. QEMU provides the test environment!
Easy, eh?

Happy hacking!

Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - fix naming of QEMU
  - use grep -q for has_config
  - support multiple -a args
  - spawn gdb on execution
  - pass through qemu options
  - dont use qemu-system-x86_64 on i386
  - add funny sentence to startup text
  - more helpful error messages

v2 - v3:

  - move to tools/testing
  - fix running: message

v3 - v4:

  - update to new path everywhere
  - check for sd module in ide case
  - fix -a
  - simplify KERNEL_BIN setting code
  - check for qemu-system-i386
  - require CONFIG_9P_FS in 9p case
  - add QEMU version check
  - check for success of drive_if
  - mention QEMU_BIN environment variable in help
---
 tools/testing/run-qemu/run-qemu.sh |  361 
 1 files changed, 361 insertions(+), 0 deletions(-)
 create mode 100755 tools/testing/run-qemu/run-qemu.sh

diff --git a/tools/testing/run-qemu/run-qemu.sh 
b/tools/testing/run-qemu/run-qemu.sh
new file mode 100755
index 000..f4fa6e7
--- /dev/null
+++ b/tools/testing/run-qemu/run-qemu.sh
@@ -0,0 +1,361 @@
+#!/bin/bash
+#
+# QEMU Launcher
+#
+# This script enables simple use of the KVM and QEMU tool stack for
+# easy kernel testing. It allows to pass either a host directory to
+# the guest or a disk image. Example usage:
+#
+# Run the host root fs inside a VM:
+#
+# $ ./tools/testing/run-qemu/run-qemu.sh -r /
+#
+# Run the same with SDL:
+#
+# $ ./tools/testing/run-qemu/run-qemu.sh -r / --sdl
+#
+# Or with a PPC build:
+#
+# $ ARCH=ppc ./tools/testing/run-qemu/run-qemu.sh -r /
+#
+# PPC with a mac99 model by passing options to QEMU:
+#
+# $ ARCH=ppc ./tools/testing/run-qemu/run-qemu.sh -r / -- -M mac99
+#
+
+USE_SDL=
+USE_VNC=
+USE_GDB=1
+KERNEL_BIN=arch/x86/boot/bzImage
+MON_STDIO=
+KERNEL_APPEND2=
+SERIAL=ttyS0
+SERIAL_KCONFIG=SERIAL_8250
+BASENAME=$(basename $0)
+
+function usage() {
+   echo 
+$BASENAME allows you to execute a virtual machine with the Linux kernel
+that you just built. To only execute a simple VM, you can just run it
+on your root fs with \-r / -a init=/bin/bash\
+
+   -a, --append parameters
+   Append the given parameters to the kernel command line.
+
+   -d, --disk image
+   Add the image file as disk into the VM.
+
+   -D, --no-gdb
+   Don't run an xterm with gdb attached to the guest.
+
+   -r, --root directory
+   Use the specified directory as root directory inside the guest.
+
+   -s, --sdl
+   Enable SDL graphical output.
+
+   -S, --smp cpus
+   Set number of virtual CPUs.
+
+   -v, --vnc
+   Enable VNC graphical output.
+
+Examples:
+
+   Run the host root fs inside a VM:
+   $ ./tools/testing/run-qemu/run-qemu.sh -r /
+
+   Run the same with SDL:
+   $ ./tools/testing/run-qemu/run-qemu.sh -r / --sdl
+
+   Or with a PPC build:
+   $ ARCH=ppc ./tools/testing/run-qemu/run-qemu.sh -r /
+
+   PPC with a mac99 model by passing options to QEMU:
+   $ ARCH=ppc ./tools/testing/run-qemu/run-qemu.sh -r / -- -M mac99
+
+}
+
+function require_config() {
+   if [ $(grep CONFIG_$1=y .config) ]; then
+   return
+   fi
+
+   echo You need to enable CONFIG_$1 for run-qemu to work properly
+   exit 1
+}
+
+function has_config() {
+   grep -q CONFIG_$1=y .config
+}
+
+function drive_if() {
+   

Re: [Qemu-devel] qemu/xendisk: properly update stats in ioreq_release()

2012-05-11 Thread Stefano Stabellini
On Thu, 10 May 2012, Jan Beulich wrote:
 While for the normal case (called from blk_send_response_all())
 decrementing requests_finished is correct, doing so in the parse error
 case is wrong; requests_inflight needs to be decremented instead.
 
 Signed-off-by: Jan Beulich jbeul...@suse.com
 
 --- a/hw/xen_disk.c
 +++ b/hw/xen_disk.c
 @@ -154,7 +154,7 @@ static void ioreq_finish(struct ioreq *i
  blkdev-requests_finished++;
  }
  
 -static void ioreq_release(struct ioreq *ioreq)
 +static void ioreq_release(struct ioreq *ioreq, bool finish)
  {
  struct XenBlkDev *blkdev = ioreq-blkdev;
  
 @@ -162,7 +162,10 @@ static void ioreq_release(struct ioreq *
  memset(ioreq, 0, sizeof(*ioreq));
  ioreq-blkdev = blkdev;
  QLIST_INSERT_HEAD(blkdev-freelist, ioreq, list);
 -blkdev-requests_finished--;
 +if (finish)
 +blkdev-requests_finished--;
 +else
 +blkdev-requests_inflight--;
  }
  
  /*
 @@ -449,7 +452,7 @@ static void blk_send_response_all(struct
  while (!QLIST_EMPTY(blkdev-finished)) {
  ioreq = QLIST_FIRST(blkdev-finished);
  send_notify += blk_send_response_one(ioreq);
 -ioreq_release(ioreq);
 +ioreq_release(ioreq, true);
  }
  if (send_notify) {
  xen_be_send_notify(blkdev-xendev);
 @@ -505,7 +508,7 @@ static void blk_handle_requests(struct X
  if (blk_send_response_one(ioreq)) {
  xen_be_send_notify(blkdev-xendev);
  }
 -ioreq_release(ioreq);
 +ioreq_release(ioreq, false);
  continue;
  }

In case of an error returned by ioreq_parse requests_finished should
also be decremented.



[Qemu-devel] [PATCH] linux-user: Fix stale tbs after mmap

2012-05-11 Thread Alexander Graf
If we execute linux-user code that does the following:

  * A = mmap()
  * execute code in A
  * munmap(A)
  * B = mmap(), but mmap returns the same address as A
  * execute code in B

we end up executing a stale cached tb that contains translated code
from A, while we want new code from B.

This patch adds a TB flush for mmap'ed regions, before we return them,
avoiding the whole issue. It also adds a flush for munmap, so that we
don't execute stale TBs instead of getting a segfault.

Reported-by: Peter Maydell peter.mayd...@linaro.org
Signed-off-by: Alexander Graf ag...@suse.de

---

v1 - v2:

  - rebase on current git
  - fix munmap too
---
 exec-all.h|2 ++
 exec.c|   17 +
 linux-user/mmap.c |6 +-
 3 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index c1b7e1f..9bda7f7 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -96,6 +96,8 @@ void QEMU_NORETURN cpu_loop_exit(CPUArchState *env1);
 int page_unprotect(target_ulong address, uintptr_t pc, void *puc);
 void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
int is_cpu_write_access);
+void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end,
+  int is_cpu_write_access);
 #if !defined(CONFIG_USER_ONLY)
 /* cputlb.c */
 void tlb_flush_page(CPUArchState *env, target_ulong addr);
diff --git a/exec.c b/exec.c
index 0607c9b..a0494c7 100644
--- a/exec.c
+++ b/exec.c
@@ -1075,6 +1075,23 @@ TranslationBlock *tb_gen_code(CPUArchState *env,
 return tb;
 }
 
+/*
+ * invalidate all TBs which intersect with the target physical pages
+ * starting in range [start;end[. NOTE: start and end may refer to
+ * different physical pages. 'is_cpu_write_access' should be true if called
+ * from a real cpu write access: the virtual CPU will exit the current
+ * TB if code is modified inside this TB.
+ */
+void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end,
+  int is_cpu_write_access)
+{
+while (start  end) {
+tb_invalidate_phys_page_range(start, end, is_cpu_write_access);
+start = TARGET_PAGE_MASK;
+start += TARGET_PAGE_SIZE;
+}
+}
+
 /* invalidate all TBs which intersect with the target physical page
starting in range [start;end[. NOTE: start and end must refer to
the same physical page. 'is_cpu_write_access' should be true if called
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 7125d1c..d9468fe 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -573,6 +573,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
prot,
 page_dump(stdout);
 printf(\n);
 #endif
+tb_invalidate_phys_range(start, start + len, 0);
 mmap_unlock();
 return start;
 fail:
@@ -675,8 +676,10 @@ int target_munmap(abi_ulong start, abi_ulong len)
 }
 }
 
-if (ret == 0)
+if (ret == 0) {
 page_set_flags(start, start + len, 0);
+tb_invalidate_phys_range(start, start + len, 0);
+}
 mmap_unlock();
 return ret;
 }
@@ -754,6 +757,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong 
old_size,
 page_set_flags(old_addr, old_addr + old_size, 0);
 page_set_flags(new_addr, new_addr + new_size, prot | PAGE_VALID);
 }
+tb_invalidate_phys_range(new_addr, new_addr + new_size, 0);
 mmap_unlock();
 return new_addr;
 }
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH] linux-user: Fix stale tbs after mmap

2012-05-11 Thread Alexander Graf

On 11.05.2012, at 17:46, Peter Maydell wrote:

 On 7 May 2012 12:38, Alexander Graf ag...@suse.de wrote:
 
 On 07.05.2012, at 13:32, Alexander Graf wrote:
 
 
 On 07.05.2012, at 12:37, Peter Maydell wrote:
 
 On 7 May 2012 10:30, Alexander Graf ag...@suse.de wrote:
 @@ -587,6 +587,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
 int prot,
page_dump(stdout);
printf(\n);
 #endif
 +tb_invalidate_phys_page_range(start, start + len, 0);
mmap_unlock();
return start;
 
 The comment at the top of tb_invalidate_phys_page_range() says
 start and end must refer to the same physical page -- is it
 out of date or does that not apply to user-mode?
 
 Do you need to also invalidate the range on munmap() and
 mprotect-to-not-executable in order to correctly fault on
 the case of:
 map something
 execute it
 unmap it
 try to execute it again
 
 ? (haven't tested that case but it seems like it might be an issue)
 
 Yeah, the issue does exist:
 
 And the below patch on top of my revised patch fixes it.
 
 I think these two patches look correct (and as you pointed out
 on irc I was wrong about mprotect, which effectively already
 handles flushing the tb if needed). If you can roll them together
 into a single patch with a commit message and signed-off-by
 you can add my Reviewed-by: tag to it.

Well, if we invalidate on unmap, do we need to invalidate on mmap() still? Or 
is only invalidating on unmap enough? Maybe when you use fixed addresses... hrm

Ah whatever, let's just flush everywhere now and then optimize later.


Alex




Re: [Qemu-devel] [PATCH 2/7] qapi: QMP input visitor, handle floats parsed as ints

2012-05-11 Thread Luiz Capitulino
On Fri, 27 Apr 2012 15:21:18 -0500
Michael Roth mdr...@linux.vnet.ibm.com wrote:

 JSON numbers can be interpreted as either integers or floating point
 values depending on their representation. As a result, QMP input visitor
 might visit a QInt when it was expecting a QFloat, so add handling to
 account for this.
 
 Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
 ---
  qapi/qmp-input-visitor.c |9 +++--
  1 files changed, 7 insertions(+), 2 deletions(-)
 
 diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
 index 4cdc47d..bc91134 100644
 --- a/qapi/qmp-input-visitor.c
 +++ b/qapi/qmp-input-visitor.c
 @@ -246,13 +246,18 @@ static void qmp_input_type_number(Visitor *v, double 
 *obj, const char *name,
  QmpInputVisitor *qiv = to_qiv(v);
  QObject *qobj = qmp_input_get_object(qiv, name);
  
 -if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
 +if (!qobj || (qobject_type(qobj) != QTYPE_QFLOAT 
 +qobject_type(qobj) != QTYPE_QINT)) {
  error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : null,
double);

s/double/number

It's also important to note that migrate_set_downtime is (positively) affected
by this change. Today, it only accepts a double, eg.:

{ execute: migrate_set_downtime, arguments: { value: 1 } }
{
error: {
class: InvalidParameterType, 
desc: Invalid parameter type for 'value', expected: double, 
data: {
name: value, 
expected: double
}
}
}

That's a bug (as it's documented to accept a number) and this patch fixes it.
There's an interface change, but I think it won't cause problems in practice.

Please, fix the error message above and would be nice to get this (and patch
02/07) as a separate series for 1.1.


  return;
  }
  
 -*obj = qfloat_get_double(qobject_to_qfloat(qobj));
 +if (qobject_type(qobj) == QTYPE_QINT) {
 +*obj = qint_get_int(qobject_to_qint(qobj));
 +} else {
 +*obj = qfloat_get_double(qobject_to_qfloat(qobj));
 +}
  }
  
  static void qmp_input_start_optional(Visitor *v, bool *present,




Re: [Qemu-devel] [PATCH v3 4/8] pc: Enable MSI support at APIC level

2012-05-11 Thread Stefano Stabellini
On Thu, 10 May 2012, Jan Kiszka wrote:
 Push msi_supported enabling to the APIC implementations where we can
 encapsulate the decision more cleanly, hiding the details from the
 generic code.
 
 CC: Stefano Stabellini stefano.stabell...@eu.citrix.com
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com

This patch is missing the KVM part, but assuming that it is done in a
later patch:

Acked-by: Stefano Stabellini stefano.stabell...@eu.citrix.com



  hw/apic.c |2 ++
  hw/pc.c   |9 -
  hw/xen.h  |   10 --
  hw/xen_apic.c |5 +
  4 files changed, 7 insertions(+), 19 deletions(-)
 
 diff --git a/hw/apic.c b/hw/apic.c
 index 4eeaf88..a337790 100644
 --- a/hw/apic.c
 +++ b/hw/apic.c
 @@ -862,6 +862,8 @@ static void apic_init(APICCommonState *s)
  
  s-timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
  local_apics[s-idx] = s;
 +
 +msi_supported = true;
  }
  
  static void apic_class_init(ObjectClass *klass, void *data)
 diff --git a/hw/pc.c b/hw/pc.c
 index 4d34a33..6691b18 100644
 --- a/hw/pc.c
 +++ b/hw/pc.c
 @@ -911,15 +911,6 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
  apic_mapped = 1;
  }
  
 -/* KVM does not support MSI yet. */
 -if (!kvm_irqchip_in_kernel()) {
 -msi_supported = true;
 -}
 -
 -if (xen_msi_support()) {
 -msi_supported = true;
 -}
 -
  return dev;
  }
  
 diff --git a/hw/xen.h b/hw/xen.h
 index 3ae4cd0..e5926b7 100644
 --- a/hw/xen.h
 +++ b/hw/xen.h
 @@ -57,14 +57,4 @@ void xen_register_framebuffer(struct MemoryRegion *mr);
  #  define HVM_MAX_VCPUS 32
  #endif
  
 -static inline int xen_msi_support(void)
 -{
 -#if defined(CONFIG_XEN_CTRL_INTERFACE_VERSION) \
 - CONFIG_XEN_CTRL_INTERFACE_VERSION = 420
 -return xen_enabled();
 -#else
 -return 0;
 -#endif
 -}
 -
  #endif /* QEMU_HW_XEN_H */
 diff --git a/hw/xen_apic.c b/hw/xen_apic.c
 index 1725ff6..a9e101f 100644
 --- a/hw/xen_apic.c
 +++ b/hw/xen_apic.c
 @@ -40,6 +40,11 @@ static void xen_apic_init(APICCommonState *s)
  {
  memory_region_init_io(s-io_memory, xen_apic_io_ops, s, xen-apic-msi,
MSI_SPACE_SIZE);
 +
 +#if defined(CONFIG_XEN_CTRL_INTERFACE_VERSION) \
 + CONFIG_XEN_CTRL_INTERFACE_VERSION = 420
 +msi_supported = true;
 +#endif
  }
  
  static void xen_apic_set_base(APICCommonState *s, uint64_t val)
 -- 
 1.7.3.4
 



Re: [Qemu-devel] [PATCH] linux-user: Fix stale tbs after mmap

2012-05-11 Thread Peter Maydell
On 11 May 2012 09:40, Alexander Graf ag...@suse.de wrote:
 If we execute linux-user code that does the following:

  * A = mmap()
  * execute code in A
  * munmap(A)
  * B = mmap(), but mmap returns the same address as A
  * execute code in B

 we end up executing a stale cached tb that contains translated code
 from A, while we want new code from B.

 This patch adds a TB flush for mmap'ed regions, before we return them,
 avoiding the whole issue. It also adds a flush for munmap, so that we
 don't execute stale TBs instead of getting a segfault.

 Reported-by: Peter Maydell peter.mayd...@linaro.org
 Signed-off-by: Alexander Graf ag...@suse.de

Reviewed-by: Peter Maydell peter.mayd...@linaro.org

-- PMM



Re: [Qemu-devel] [PATCH next v2 41/74] target-lm32: Let cpu_lm32_init() return LM32CPU

2012-05-11 Thread Michael Walle
 Make the include paths for cpu-qom.h consistent to allow using LM32CPU
 in cpu.h.
 
 Turn cpu_init macro into a static inline function returning CPULM32State
 for backwards compatibility.
 
 Signed-off-by: Andreas Färber afaer...@suse.de
Acked-by: Michael Walle mich...@walle.cc

-- 
Michael



Re: [Qemu-devel] [PATCH 4/7] qapi: String visitor, use %f represenation for floats

2012-05-11 Thread Luiz Capitulino
On Fri, 27 Apr 2012 15:21:20 -0500
Michael Roth mdr...@linux.vnet.ibm.com wrote:

 Currently string-output-visitor formats floats as %g, which is nice in
 that trailing 0's are automatically truncated, but otherwise this causes
 some issues:
 
  - it 6 uses significant figures instead of 6 decimal places, which
means something like 155777.5 (which even has an exact floating point
representation) will be rounded to 155778 when converted to a string.
 
  - output will be presented in scientific notation when the normalized
form requires a 10^x multiplier. Not a huge deal, but arguably less
readable for command-line arguments.
 
  - due to using sig figs instead of hard-defined decimal places, it
fails a lot of the test-visitor-serialization unit tests for floats.
 
 Instead, let's just use %f, which is what the QJSON and the QMP visitors
 use.
 
 Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
 ---
  qapi/string-output-visitor.c   |2 +-
  tests/test-string-output-visitor.c |2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
 index 92b0305..34e525e 100644
 --- a/qapi/string-output-visitor.c
 +++ b/qapi/string-output-visitor.c
 @@ -52,7 +52,7 @@ static void print_type_number(Visitor *v, double *obj, 
 const char *name,
Error **errp)
  {
  StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
 -string_output_set(sov, g_strdup_printf(%g, *obj));
 +string_output_set(sov, g_strdup_printf(%f, *obj));

Doesn't look like a bug fix worth it for 1.1, am I wrong?

  }
  
  char *string_output_get_string(StringOutputVisitor *sov)
 diff --git a/tests/test-string-output-visitor.c 
 b/tests/test-string-output-visitor.c
 index 22909b8..608f14a 100644
 --- a/tests/test-string-output-visitor.c
 +++ b/tests/test-string-output-visitor.c
 @@ -84,7 +84,7 @@ static void test_visitor_out_number(TestOutputVisitorData 
 *data,
  
  str = string_output_get_string(data-sov);
  g_assert(str != NULL);
 -g_assert_cmpstr(str, ==, 3.14);
 +g_assert_cmpstr(str, ==, 3.14);
  g_free(str);
  }
  




Re: [Qemu-devel] [PATCH 2/7] qapi: QMP input visitor, handle floats parsed as ints

2012-05-11 Thread Michael Roth
On Fri, May 11, 2012 at 01:22:33PM -0300, Luiz Capitulino wrote:
 On Fri, 27 Apr 2012 15:21:18 -0500
 Michael Roth mdr...@linux.vnet.ibm.com wrote:
 
  JSON numbers can be interpreted as either integers or floating point
  values depending on their representation. As a result, QMP input visitor
  might visit a QInt when it was expecting a QFloat, so add handling to
  account for this.
  
  Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
  ---
   qapi/qmp-input-visitor.c |9 +++--
   1 files changed, 7 insertions(+), 2 deletions(-)
  
  diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
  index 4cdc47d..bc91134 100644
  --- a/qapi/qmp-input-visitor.c
  +++ b/qapi/qmp-input-visitor.c
  @@ -246,13 +246,18 @@ static void qmp_input_type_number(Visitor *v, double 
  *obj, const char *name,
   QmpInputVisitor *qiv = to_qiv(v);
   QObject *qobj = qmp_input_get_object(qiv, name);
   
  -if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
  +if (!qobj || (qobject_type(qobj) != QTYPE_QFLOAT 
  +qobject_type(qobj) != QTYPE_QINT)) {
   error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : null,
 double);
 
 s/double/number
 
 It's also important to note that migrate_set_downtime is (positively) affected
 by this change. Today, it only accepts a double, eg.:
 
 { execute: migrate_set_downtime, arguments: { value: 1 } }
 {
 error: {
 class: InvalidParameterType, 
 desc: Invalid parameter type for 'value', expected: double, 
 data: {
 name: value, 
 expected: double
 }
 }
 }
 
 That's a bug (as it's documented to accept a number) and this patch fixes it.
 There's an interface change, but I think it won't cause problems in practice.
 
 Please, fix the error message above and would be nice to get this (and patch
 02/07) as a separate series for 1.1.

Hmm, this is kinda awkward because I don't think we can change it without
breaking any trees based off qom-next.

Since the error msg predates the bug fix (it just becomes more obviously
wrong as a result of the fix), can you pull these commits as is into your
QMP tree?

In the meantime I can send another patch, based on qom-next or qmp, that
fixes the error msg. We can then get all 3 into 1.1/master via QMP tree, and I
think qom-next should still merge cleanly back into master after 1.1

 
 
   return;
   }
   
  -*obj = qfloat_get_double(qobject_to_qfloat(qobj));
  +if (qobject_type(qobj) == QTYPE_QINT) {
  +*obj = qint_get_int(qobject_to_qint(qobj));
  +} else {
  +*obj = qfloat_get_double(qobject_to_qfloat(qobj));
  +}
   }
   
   static void qmp_input_start_optional(Visitor *v, bool *present,
 



Re: [Qemu-devel] [Xen-devel] [PATCH] qemu/xendisk: set maximum number of grants to be used

2012-05-11 Thread Stefano Stabellini
On Fri, 11 May 2012, Jan Beulich wrote:
  On 11.05.12 at 09:19, Jan Beulich jbeul...@suse.com wrote:
  Legacy (non-pvops) gntdev drivers may require this to be done when the
  number of grants intended to be used simultaneously exceeds a certain
  driver specific default limit.
  
  Signed-off-by: Jan Beulich jbeul...@suse.com
  
  --- a/hw/xen_disk.c
  +++ b/hw/xen_disk.c
  @@ -536,6 +536,10 @@ static void blk_alloc(struct XenDevice *
   if (xen_mode != XEN_EMULATE) {
   batch_maps = 1;
   }
  +if (xc_gnttab_set_max_grants(xendev-gnttabdev,
  +max_requests * BLKIF_MAX_SEGMENTS_PER_REQUEST + 1)  0)
 
 In more extensive testing it appears that very rarely this value is still
 too low:
 
 xen be: qdisk-768: can't map 11 grant refs (Cannot allocate memory, 342 maps)
 
 342 + 11 = 353  352 = 32 * 11
 
 Could someone help out here? I first thought this might be due to
 use_aio being non-zero, but ioreq_start() doesn't permit more than
 max_requests struct ioreqs-s to be around.

Actually 342 + 11 = 353, that should be still OK because it is equal to
32 * 11 + 1, where the additional 1 is for the ring, right?


 Additionally, shouldn't the driver be smarter and gracefully handle
 grant mapping failures (as the per-domain map track table in the
 hypervisor is a finite resource)?

yes, probably



Re: [Qemu-devel] [PATCH 2/7] qapi: QMP input visitor, handle floats parsed as ints

2012-05-11 Thread Andreas Färber
Am 11.05.2012 19:04, schrieb Michael Roth:
 On Fri, May 11, 2012 at 01:22:33PM -0300, Luiz Capitulino wrote:
 On Fri, 27 Apr 2012 15:21:18 -0500
 Michael Roth mdr...@linux.vnet.ibm.com wrote:

 JSON numbers can be interpreted as either integers or floating point
 values depending on their representation. As a result, QMP input visitor
 might visit a QInt when it was expecting a QFloat, so add handling to
 account for this.

 Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
 ---
  qapi/qmp-input-visitor.c |9 +++--
  1 files changed, 7 insertions(+), 2 deletions(-)

 diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
 index 4cdc47d..bc91134 100644
 --- a/qapi/qmp-input-visitor.c
 +++ b/qapi/qmp-input-visitor.c
 @@ -246,13 +246,18 @@ static void qmp_input_type_number(Visitor *v, double 
 *obj, const char *name,
  QmpInputVisitor *qiv = to_qiv(v);
  QObject *qobj = qmp_input_get_object(qiv, name);
  
 -if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
 +if (!qobj || (qobject_type(qobj) != QTYPE_QFLOAT 
 +qobject_type(qobj) != QTYPE_QINT)) {
  error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : null,
double);

 s/double/number

 It's also important to note that migrate_set_downtime is (positively) 
 affected
 by this change. Today, it only accepts a double, eg.:

 { execute: migrate_set_downtime, arguments: { value: 1 } }
 {
 error: {
 class: InvalidParameterType, 
 desc: Invalid parameter type for 'value', expected: double, 
 data: {
 name: value, 
 expected: double
 }
 }
 }

 That's a bug (as it's documented to accept a number) and this patch fixes it.
 There's an interface change, but I think it won't cause problems in practice.

 Please, fix the error message above and would be nice to get this (and patch
 02/07) as a separate series for 1.1.
 
 Hmm, this is kinda awkward because I don't think we can change it without
 breaking any trees based off qom-next.

That's no problem at all, it is announced as rebasing and I have scripts
in place to pull and recursively rebase all my QOM branches.

For such a trivial textual change I can even fix it up manually. :)

Since Luiz is now taking care of this one I will simply drop it from my
qom-1.1 branch to avoid any confusion.

Andreas

 Since the error msg predates the bug fix (it just becomes more obviously
 wrong as a result of the fix), can you pull these commits as is into your
 QMP tree?
 
 In the meantime I can send another patch, based on qom-next or qmp, that
 fixes the error msg. We can then get all 3 into 1.1/master via QMP tree, and I
 think qom-next should still merge cleanly back into master after 1.1
 


  return;
  }
  
 -*obj = qfloat_get_double(qobject_to_qfloat(qobj));
 +if (qobject_type(qobj) == QTYPE_QINT) {
 +*obj = qint_get_int(qobject_to_qint(qobj));
 +} else {
 +*obj = qfloat_get_double(qobject_to_qfloat(qobj));
 +}
  }
  
  static void qmp_input_start_optional(Visitor *v, bool *present,

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH 4/7] qapi: String visitor, use %f represenation for floats

2012-05-11 Thread Michael Roth
On Fri, May 11, 2012 at 01:34:01PM -0300, Luiz Capitulino wrote:
 On Fri, 27 Apr 2012 15:21:20 -0500
 Michael Roth mdr...@linux.vnet.ibm.com wrote:
 
  Currently string-output-visitor formats floats as %g, which is nice in
  that trailing 0's are automatically truncated, but otherwise this causes
  some issues:
  
   - it 6 uses significant figures instead of 6 decimal places, which
 means something like 155777.5 (which even has an exact floating point
 representation) will be rounded to 155778 when converted to a string.
  
   - output will be presented in scientific notation when the normalized
 form requires a 10^x multiplier. Not a huge deal, but arguably less
 readable for command-line arguments.
  
   - due to using sig figs instead of hard-defined decimal places, it
 fails a lot of the test-visitor-serialization unit tests for floats.
  
  Instead, let's just use %f, which is what the QJSON and the QMP visitors
  use.
  
  Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
  ---
   qapi/string-output-visitor.c   |2 +-
   tests/test-string-output-visitor.c |2 +-
   2 files changed, 2 insertions(+), 2 deletions(-)
  
  diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
  index 92b0305..34e525e 100644
  --- a/qapi/string-output-visitor.c
  +++ b/qapi/string-output-visitor.c
  @@ -52,7 +52,7 @@ static void print_type_number(Visitor *v, double *obj, 
  const char *name,
 Error **errp)
   {
   StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
  -string_output_set(sov, g_strdup_printf(%g, *obj));
  +string_output_set(sov, g_strdup_printf(%f, *obj));
 
 Doesn't look like a bug fix worth it for 1.1, am I wrong?

Well, object_property_print() is the only string-output-visitor user,
and it's not currently used. I don't see this changing for 1.1, so this
can probably wait.

 
   }
   
   char *string_output_get_string(StringOutputVisitor *sov)
  diff --git a/tests/test-string-output-visitor.c 
  b/tests/test-string-output-visitor.c
  index 22909b8..608f14a 100644
  --- a/tests/test-string-output-visitor.c
  +++ b/tests/test-string-output-visitor.c
  @@ -84,7 +84,7 @@ static void test_visitor_out_number(TestOutputVisitorData 
  *data,
   
   str = string_output_get_string(data-sov);
   g_assert(str != NULL);
  -g_assert_cmpstr(str, ==, 3.14);
  +g_assert_cmpstr(str, ==, 3.14);
   g_free(str);
   }
   
 



Re: [Qemu-devel] [PATCH 2/7] qapi: QMP input visitor, handle floats parsed as ints

2012-05-11 Thread Michael Roth
On Fri, May 11, 2012 at 07:16:18PM +0200, Andreas Färber wrote:
 Am 11.05.2012 19:04, schrieb Michael Roth:
  On Fri, May 11, 2012 at 01:22:33PM -0300, Luiz Capitulino wrote:
  On Fri, 27 Apr 2012 15:21:18 -0500
  Michael Roth mdr...@linux.vnet.ibm.com wrote:
 
  JSON numbers can be interpreted as either integers or floating point
  values depending on their representation. As a result, QMP input visitor
  might visit a QInt when it was expecting a QFloat, so add handling to
  account for this.
 
  Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
  ---
   qapi/qmp-input-visitor.c |9 +++--
   1 files changed, 7 insertions(+), 2 deletions(-)
 
  diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
  index 4cdc47d..bc91134 100644
  --- a/qapi/qmp-input-visitor.c
  +++ b/qapi/qmp-input-visitor.c
  @@ -246,13 +246,18 @@ static void qmp_input_type_number(Visitor *v, 
  double *obj, const char *name,
   QmpInputVisitor *qiv = to_qiv(v);
   QObject *qobj = qmp_input_get_object(qiv, name);
   
  -if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
  +if (!qobj || (qobject_type(qobj) != QTYPE_QFLOAT 
  +qobject_type(qobj) != QTYPE_QINT)) {
   error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : 
  null,
 double);
 
  s/double/number
 
  It's also important to note that migrate_set_downtime is (positively) 
  affected
  by this change. Today, it only accepts a double, eg.:
 
  { execute: migrate_set_downtime, arguments: { value: 1 } }
  {
  error: {
  class: InvalidParameterType, 
  desc: Invalid parameter type for 'value', expected: double, 
  data: {
  name: value, 
  expected: double
  }
  }
  }
 
  That's a bug (as it's documented to accept a number) and this patch fixes 
  it.
  There's an interface change, but I think it won't cause problems in 
  practice.
 
  Please, fix the error message above and would be nice to get this (and 
  patch
  02/07) as a separate series for 1.1.
  
  Hmm, this is kinda awkward because I don't think we can change it without
  breaking any trees based off qom-next.
 
 That's no problem at all, it is announced as rebasing and I have scripts
 in place to pull and recursively rebase all my QOM branches.
 
 For such a trivial textual change I can even fix it up manually. :)
 
 Since Luiz is now taking care of this one I will simply drop it from my
 qom-1.1 branch to avoid any confusion.

That'll work, thanks.

Luiz, I'll send a separate patch shortly with the error msg fixed.

 
 Andreas
 
  Since the error msg predates the bug fix (it just becomes more obviously
  wrong as a result of the fix), can you pull these commits as is into your
  QMP tree?
  
  In the meantime I can send another patch, based on qom-next or qmp, that
  fixes the error msg. We can then get all 3 into 1.1/master via QMP tree, 
  and I
  think qom-next should still merge cleanly back into master after 1.1
  
 
 
   return;
   }
   
  -*obj = qfloat_get_double(qobject_to_qfloat(qobj));
  +if (qobject_type(qobj) == QTYPE_QINT) {
  +*obj = qint_get_int(qobject_to_qint(qobj));
  +} else {
  +*obj = qfloat_get_double(qobject_to_qfloat(qobj));
  +}
   }
   
   static void qmp_input_start_optional(Visitor *v, bool *present,
 
 -- 
 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
 GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
 



[Qemu-devel] [PATCH 1.1] qapi: QMP input visitor, handle floats parsed as ints

2012-05-11 Thread Michael Roth
JSON numbers can be interpreted as either integers or floating point
values depending on their representation. As a result, QMP input visitor
might visit a QInt when it was expecting a QFloat, so add handling to
account for this.

Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
---
 qapi/qmp-input-visitor.c |   11 ---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 4cdc47d..107d8d3 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -246,13 +246,18 @@ static void qmp_input_type_number(Visitor *v, double 
*obj, const char *name,
 QmpInputVisitor *qiv = to_qiv(v);
 QObject *qobj = qmp_input_get_object(qiv, name);
 
-if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
+if (!qobj || (qobject_type(qobj) != QTYPE_QFLOAT 
+qobject_type(qobj) != QTYPE_QINT)) {
 error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : null,
-  double);
+  number);
 return;
 }
 
-*obj = qfloat_get_double(qobject_to_qfloat(qobj));
+if (qobject_type(qobj) == QTYPE_QINT) {
+*obj = qint_get_int(qobject_to_qint(qobj));
+} else {
+*obj = qfloat_get_double(qobject_to_qfloat(qobj));
+}
 }
 
 static void qmp_input_start_optional(Visitor *v, bool *present,
-- 
1.7.4.1




Re: [Qemu-devel] [PATCH 4/7] qapi: String visitor, use %f represenation for floats

2012-05-11 Thread Andreas Färber
Am 11.05.2012 19:32, schrieb Michael Roth:
 On Fri, May 11, 2012 at 01:34:01PM -0300, Luiz Capitulino wrote:
 On Fri, 27 Apr 2012 15:21:20 -0500
 Michael Roth mdr...@linux.vnet.ibm.com wrote:

 Currently string-output-visitor formats floats as %g, which is nice in
 that trailing 0's are automatically truncated, but otherwise this causes
 some issues:

  - it 6 uses significant figures instead of 6 decimal places, which
means something like 155777.5 (which even has an exact floating point
representation) will be rounded to 155778 when converted to a string.

  - output will be presented in scientific notation when the normalized
form requires a 10^x multiplier. Not a huge deal, but arguably less
readable for command-line arguments.

  - due to using sig figs instead of hard-defined decimal places, it
fails a lot of the test-visitor-serialization unit tests for floats.

 Instead, let's just use %f, which is what the QJSON and the QMP visitors
 use.

 Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
 ---
  qapi/string-output-visitor.c   |2 +-
  tests/test-string-output-visitor.c |2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
 index 92b0305..34e525e 100644
 --- a/qapi/string-output-visitor.c
 +++ b/qapi/string-output-visitor.c
 @@ -52,7 +52,7 @@ static void print_type_number(Visitor *v, double *obj, 
 const char *name,
Error **errp)
  {
  StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
 -string_output_set(sov, g_strdup_printf(%g, *obj));
 +string_output_set(sov, g_strdup_printf(%f, *obj));

 Doesn't look like a bug fix worth it for 1.1, am I wrong?
 
 Well, object_property_print() is the only string-output-visitor user,
 and it's not currently used. I don't see this changing for 1.1, so this
 can probably wait.

Actually it might be in 1.1: there's a pending patch by Paolo to use
that for info qtree, where some properties were missing. My review
comment has been resolved, so I will queue that patch for 1.1 and next.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH 1.1] qapi: QMP input visitor, handle floats parsed as ints

2012-05-11 Thread Andreas Färber
Am 11.05.2012 19:43, schrieb Michael Roth:
 JSON numbers can be interpreted as either integers or floating point
 values depending on their representation. As a result, QMP input visitor
 might visit a QInt when it was expecting a QFloat, so add handling to
 account for this.
 
 Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com

Acked-by: Andreas Färber afaer...@suse.de

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [SeaBIOS] [RESEND PATCH v3] hotplug: add device per func in ACPI DSDT tables

2012-05-11 Thread Gleb Natapov
On Fri, May 11, 2012 at 11:44:02PM +0800, Jiang Liu wrote:
 On 05/11/2012 06:14 PM, Gleb Natapov wrote:
  I'm not familiar with qemu:(
  On native OS, admin could trigger PCI device hotplug operations through
  /sys/bus/pci/slot/xx/power. Not sure whether that's needed for guest OS 
  too. 
 
  Why is it needed on physical HW? May be it is needed in a VM for the
  same reason?
 As Amos has mentioned, it's used power on/off a PCI device instead of physical
 hotplug. Not sure whether it's needed in guest OS.
Probably for assigned devices.

--
Gleb.



[Qemu-devel] [PATCH 3/3] qcow2: Support for fixing refcount inconsistencies

2012-05-11 Thread Kevin Wolf
Signed-off-by: Kevin Wolf kw...@redhat.com
---
 block/qcow2-refcount.c |   27 +--
 block/qcow2.c  |6 +-
 block/qcow2.h  |3 ++-
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 443c021..6a9a672 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1122,7 +1122,8 @@ fail:
  * Returns 0 if no errors are found, the number of errors in case the image is
  * detected as corrupted, and -errno when an internal error occurred.
  */
-int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res)
+int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
+  BdrvCheckMode fix)
 {
 BDRVQcowState *s = bs-opaque;
 int64_t size;
@@ -1205,9 +1206,31 @@ int qcow2_check_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res)
 
 refcount2 = refcount_table[i];
 if (refcount1 != refcount2) {
+
+/* Check if we're allowed to fix the mismatch */
+int *num_fixed = NULL;
+if (refcount1  refcount2  (fix  BDRV_FIX_LEAKS)) {
+num_fixed = res-leaks_fixed;
+} else if (refcount1  refcount2  (fix  BDRV_FIX_ERRORS)) {
+num_fixed = res-corruptions_fixed;
+}
+
 fprintf(stderr, %s cluster %d refcount=%d reference=%d\n,
-   refcount1  refcount2 ? ERROR : Leaked,
+   num_fixed != NULL ? Repairing :
+   refcount1  refcount2 ? ERROR :
+   Leaked,
i, refcount1, refcount2);
+
+if (num_fixed) {
+ret = update_refcount(bs, i  s-cluster_bits, 1,
+  refcount2 - refcount1);
+if (ret = 0) {
+(*num_fixed)++;
+continue;
+}
+}
+
+/* And if we couldn't, print an error */
 if (refcount1  refcount2) {
 res-corruptions++;
 } else {
diff --git a/block/qcow2.c b/block/qcow2.c
index 23cc920..cc751d2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1511,11 +1511,7 @@ static int qcow2_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
BdrvCheckMode fix)
 {
-if (fix) {
-return -ENOTSUP;
-}
-
-return qcow2_check_refcounts(bs, result);
+return qcow2_check_refcounts(bs, result, fix);
 }
 
 #if 0
diff --git a/block/qcow2.h b/block/qcow2.h
index 9b7b2d6..5fb7f61 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -275,7 +275,8 @@ void qcow2_free_any_clusters(BlockDriverState *bs,
 int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 int64_t l1_table_offset, int l1_size, int addend);
 
-int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res);
+int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
+  BdrvCheckMode fix);
 
 /* qcow2-cluster.c functions */
 int qcow2_grow_l1_table(BlockDriverState *bs, int min_size, bool exact_size);
-- 
1.7.6.5




Re: [Qemu-devel] [PATCH 2/7] qapi: QMP input visitor, handle floats parsed as ints

2012-05-11 Thread Luiz Capitulino
On Fri, 11 May 2012 12:04:47 -0500
Michael Roth mdr...@linux.vnet.ibm.com wrote:

 On Fri, May 11, 2012 at 01:22:33PM -0300, Luiz Capitulino wrote:
  On Fri, 27 Apr 2012 15:21:18 -0500
  Michael Roth mdr...@linux.vnet.ibm.com wrote:
  
   JSON numbers can be interpreted as either integers or floating point
   values depending on their representation. As a result, QMP input visitor
   might visit a QInt when it was expecting a QFloat, so add handling to
   account for this.
   
   Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
   ---
qapi/qmp-input-visitor.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
   
   diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
   index 4cdc47d..bc91134 100644
   --- a/qapi/qmp-input-visitor.c
   +++ b/qapi/qmp-input-visitor.c
   @@ -246,13 +246,18 @@ static void qmp_input_type_number(Visitor *v, 
   double *obj, const char *name,
QmpInputVisitor *qiv = to_qiv(v);
QObject *qobj = qmp_input_get_object(qiv, name);

   -if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
   +if (!qobj || (qobject_type(qobj) != QTYPE_QFLOAT 
   +qobject_type(qobj) != QTYPE_QINT)) {
error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : 
   null,
  double);
  
  s/double/number
  
  It's also important to note that migrate_set_downtime is (positively) 
  affected
  by this change. Today, it only accepts a double, eg.:
  
  { execute: migrate_set_downtime, arguments: { value: 1 } }
  {
  error: {
  class: InvalidParameterType, 
  desc: Invalid parameter type for 'value', expected: double, 
  data: {
  name: value, 
  expected: double
  }
  }
  }
  
  That's a bug (as it's documented to accept a number) and this patch fixes 
  it.
  There's an interface change, but I think it won't cause problems in 
  practice.
  
  Please, fix the error message above and would be nice to get this (and patch
  02/07) as a separate series for 1.1.
 
 Hmm, this is kinda awkward because I don't think we can change it without
 breaking any trees based off qom-next.
 
 Since the error msg predates the bug fix (it just becomes more obviously
 wrong as a result of the fix), can you pull these commits as is into your
 QMP tree?
 
 In the meantime I can send another patch, based on qom-next or qmp, that
 fixes the error msg. We can then get all 3 into 1.1/master via QMP tree, and I
 think qom-next should still merge cleanly back into master after 1.1

I think Andreas answered you here (in the sense that it's ok to respin this
patch), but I also would like to add that patch 4/7 doesn't seem a bug fix
to me (at least not worth it for 1.1).



Re: [Qemu-devel] PCI: Hot-removing a virtio-blk causes guest panic

2012-05-11 Thread Michael S. Tsirkin
On Fri, May 11, 2012 at 12:37:49PM +0800, Amos Kong wrote:
 good: 3.3.0 guest kernel  qemu-kvm-rhel6
 guest panic:  3.3.0 guest kernel  qemu-upstream (contains fix [1])
 
 I didn't change anything of guest kernel,
 It seems a bug of qemu-upstream.
 
 [1] http://marc.info/?l=qemu-develm=133670266801022w=2
 [PATCH] qom: fix refcounting in object_property_del_child()
 
 
  Start VM with one block device:
 qemu-upstream --enable-kvm  -name 'vm1' -nodefaults -drive
 file='nolvm.qcow2',index=0,if=virtio,cache=none,snapshot=on -net none -m
 2000 -smp 2 -vnc :0  -kernel vmlinuz-3.3.0 -append 'ro root=/dev/vda1
 console=tty0 console=ttyS0,115200'   -drive
 file=images/u0,if=none,id=drive-virtio0-0-0,format=qcow2,cache=none
 -device virtio-blk-pci,drive=drive-virtio0-0-0,id=virti0-0-0 -monitor
 unix:/tmp/m,nowait,server
 
  hot-remove the virtio disk
 (qemu)# echo device_del virti0-0-0 | nc -U /tmp/m
 
  guest panic:


Find a working version and bisect?



[Qemu-devel] [PATCH 1/3] qemu-img check -r for repairing images

2012-05-11 Thread Kevin Wolf
The QED block driver already provides the functionality to not only
detect inconsistencies in images, but also fix them. However, this
functionality cannot be manually invoked with qemu-img, but the
check happens only automatically during bdrv_open().

This adds a -r switch to qemu-img check that allows manual invocation
of an image repair.

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 block.c  |4 ++--
 block.h  |7 ++-
 block/qcow2.c|7 ++-
 block/qed.c  |5 +++--
 block/vdi.c  |7 ++-
 block_int.h  |3 ++-
 qemu-img-cmds.hx |4 ++--
 qemu-img.c   |   25 ++---
 qemu-img.texi|7 ++-
 9 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/block.c b/block.c
index acebe46..8e928b9 100644
--- a/block.c
+++ b/block.c
@@ -1211,14 +1211,14 @@ bool bdrv_dev_is_medium_locked(BlockDriverState *bs)
  * free of errors) or -errno when an internal error occurred. The results of 
the
  * check are stored in res.
  */
-int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res)
+int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix)
 {
 if (bs-drv-bdrv_check == NULL) {
 return -ENOTSUP;
 }
 
 memset(res, 0, sizeof(*res));
-return bs-drv-bdrv_check(bs, res);
+return bs-drv-bdrv_check(bs, res, fix);
 }
 
 #define COMMIT_BUF_SECTORS 2048
diff --git a/block.h b/block.h
index 799cf48..61b7e8e 100644
--- a/block.h
+++ b/block.h
@@ -190,7 +190,12 @@ typedef struct BdrvCheckResult {
 BlockFragInfo bfi;
 } BdrvCheckResult;
 
-int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res);
+typedef enum {
+BDRV_FIX_LEAKS= 1,
+BDRV_FIX_ERRORS   = 2,
+} BdrvCheckMode;
+
+int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix);
 
 /* async block I/O */
 typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,
diff --git a/block/qcow2.c b/block/qcow2.c
index 3997e19..23cc920 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1508,8 +1508,13 @@ static int qcow2_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 }
 
 
-static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result)
+static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
+   BdrvCheckMode fix)
 {
+if (fix) {
+return -ENOTSUP;
+}
+
 return qcow2_check_refcounts(bs, result);
 }
 
diff --git a/block/qed.c b/block/qed.c
index 30a31f9..ab59724 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1517,11 +1517,12 @@ static void bdrv_qed_invalidate_cache(BlockDriverState 
*bs)
 bdrv_qed_open(bs, bs-open_flags);
 }
 
-static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result)
+static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result,
+  BdrvCheckMode fix)
 {
 BDRVQEDState *s = bs-opaque;
 
-return qed_check(s, result, false);
+return qed_check(s, result, !!fix);
 }
 
 static QEMUOptionParameter qed_create_options[] = {
diff --git a/block/vdi.c b/block/vdi.c
index 119d3c7..57325d6 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -277,7 +277,8 @@ static void vdi_header_print(VdiHeader *header)
 }
 #endif
 
-static int vdi_check(BlockDriverState *bs, BdrvCheckResult *res)
+static int vdi_check(BlockDriverState *bs, BdrvCheckResult *res,
+ BdrvCheckMode fix)
 {
 /* TODO: additional checks possible. */
 BDRVVdiState *s = (BDRVVdiState *)bs-opaque;
@@ -286,6 +287,10 @@ static int vdi_check(BlockDriverState *bs, BdrvCheckResult 
*res)
 uint32_t *bmap;
 logout(\n);
 
+if (fix) {
+return -ENOTSUP;
+}
+
 bmap = g_malloc(s-header.blocks_in_image * sizeof(uint32_t));
 memset(bmap, 0xff, s-header.blocks_in_image * sizeof(uint32_t));
 
diff --git a/block_int.h b/block_int.h
index b80e66d..a0fffe1 100644
--- a/block_int.h
+++ b/block_int.h
@@ -241,7 +241,8 @@ struct BlockDriver {
  * Returns 0 for completed check, -errno for internal errors.
  * The check results are stored in result.
  */
-int (*bdrv_check)(BlockDriverState* bs, BdrvCheckResult *result);
+int (*bdrv_check)(BlockDriverState* bs, BdrvCheckResult *result,
+BdrvCheckMode fix);
 
 void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event);
 
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 49dce7c..39419a0 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -10,9 +10,9 @@ STEXI
 ETEXI
 
 DEF(check, img_check,
-check [-f fmt] filename)
+check [-f fmt] [-r [leaks | all]] filename)
 STEXI
-@item check [-f @var{fmt}] @var{filename}
+@item check [-f @var{fmt}] [-r [leaks | all]] @var{filename}
 ETEXI
 
 DEF(create, img_create,
diff --git a/qemu-img.c b/qemu-img.c
index 5434ddc..b151076 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -85,6 +85,12 @@ static void help(void)
  '-S' indicates the consecutive number of bytes that must contain 
only zeros\n
   for 

[Qemu-devel] [PATCH block-next 0/3] qemu-img check/qcow2: Allow fixing refcounts

2012-05-11 Thread Kevin Wolf
A prerequisite for a QED mode in qcow2, which doesn't update the refcount
table except on clean shutdown, is that refcounts can be repaired when the
image is opened the next time after a crash.

This series adds a qemu-img check option that doesn't only check, but also
tries to fix the errors that it found.

Kevin Wolf (3):
  qemu-img check -r for repairing images
  qemu-img check: Print fixed clusters and recheck
  qcow2: Support for fixing refcount inconsistencies

 block.c|4 ++--
 block.h|9 -
 block/qcow2-refcount.c |   27 +--
 block/qcow2.c  |5 +++--
 block/qcow2.h  |3 ++-
 block/qed-check.c  |2 ++
 block/qed.c|5 +++--
 block/vdi.c|7 ++-
 block_int.h|3 ++-
 qemu-img-cmds.hx   |4 ++--
 qemu-img.c |   35 ---
 qemu-img.texi  |7 ++-
 12 files changed, 93 insertions(+), 18 deletions(-)

-- 
1.7.6.5




  1   2   >