Re: [Qemu-devel] qemu does not support PAPR

2014-06-05 Thread sonia verma
Hi Paolo

I'm running this on ubuntu with powerpc arch with kernel
3.8.13-rt9-QorIQ-SDK-V1.4.


Thanks



On Thu, Jun 5, 2014 at 11:21 AM, Paolo Bonzini pbonz...@redhat.com wrote:

 Il 05/06/2014 06:50, sonia verma ha scritto:

  Hi

 I'm trying to run libguestfs-test-tool which in turn is using kvm
 utility on powerpc ubuntu.Below are the logs when I'm trying to run the
 libguesfs-test-tool utility..


 What machine are you running on?  What kernel version?

 Paolo




Re: [Qemu-devel] [PATCH v7 1/4] spapr_iommu: Make in-kernel TCE table optional

2014-06-05 Thread Alexey Kardashevskiy
On 06/05/2014 03:49 PM, Alexey Kardashevskiy wrote:
 POWER KVM supports an KVM_CAP_SPAPR_TCE capability which allows allocating
 TCE tables in the host kernel memory and handle H_PUT_TCE requests
 targeted to specific LIOBN (logical bus number) right in the host without
 switching to QEMU. At the moment this is used for emulated devices only
 and the handler only puts TCE to the table. If the in-kernel H_PUT_TCE
 handler finds a LIOBN and corresponding table, it will put a TCE to
 the table and complete hypercall execution. The user space will not be
 notified.
 
 Upcoming VFIO support is going to use the same sPAPRTCETable device class
 so KVM_CAP_SPAPR_TCE is going to be used as well. That means that TCE
 tables for VFIO are going to be allocated in the host as well.
 However VFIO operates with real IOMMU tables and simple copying of
 a TCE to the real hardware TCE table will not work as guest physical
 to host physical address translation is requited.
 
 So until the host kernel gets VFIO support for H_PUT_TCE, we better not
 to register VFIO's TCE in the host.
 
 This adds a bool @kvm_accel flag to the sPAPRTCETable device telling
 that sPAPRTCETable should not try allocating TCE table in the host kernel.
 Instead, the table will be created in QEMU.
 
 This adds an kvm_accel parameter to spapr_tce_new_table() to let users
 choose whether to use acceleration or not. At the moment it is enabled
 for VIO and emulated PCI. Upcoming VFIO support will set it to false.
 
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---
 
 This is a workaround but it lets me have one IOMMU device for VIO, emulated
 PCI and VFIO which is a good thing.
 
 The other way around would be a new KVM_CAP_SPAPR_TCE_VFIO capability but
 this needs kernel update.


Never mind, I'll make it a capability. I'll post capability reservation
patch separately.


 ---
  hw/ppc/spapr_iommu.c   | 6 --
  hw/ppc/spapr_pci.c | 2 +-
  hw/ppc/spapr_vio.c | 2 +-
  include/hw/ppc/spapr.h | 4 +++-
  4 files changed, 9 insertions(+), 5 deletions(-)
 
 diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
 index 3b6e373..bfd3701 100644
 --- a/hw/ppc/spapr_iommu.c
 +++ b/hw/ppc/spapr_iommu.c
 @@ -115,7 +115,7 @@ static int spapr_tce_table_realize(DeviceState *dev)
  {
  sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
  
 -if (kvm_enabled()) {
 +if (tcet-kvm_accel  kvm_enabled()) {
  tcet-table = kvmppc_create_spapr_tce(tcet-liobn,
tcet-nb_table 
tcet-page_shift,
 @@ -143,7 +143,8 @@ static int spapr_tce_table_realize(DeviceState *dev)
  sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn,
 uint64_t bus_offset,
 uint32_t page_shift,
 -   uint32_t nb_table)
 +   uint32_t nb_table,
 +   bool kvm_accel)
  {
  sPAPRTCETable *tcet;
  
 @@ -162,6 +163,7 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, 
 uint32_t liobn,
  tcet-bus_offset = bus_offset;
  tcet-page_shift = page_shift;
  tcet-nb_table = nb_table;
 +tcet-kvm_accel = kvm_accel;
  
  object_property_add_child(OBJECT(owner), tce-table, OBJECT(tcet), 
 NULL);
  
 diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
 index ddfd8bb..6021f35 100644
 --- a/hw/ppc/spapr_pci.c
 +++ b/hw/ppc/spapr_pci.c
 @@ -658,7 +658,7 @@ static void spapr_phb_finish_realize(sPAPRPHBState *sphb, 
 Error **errp)
  tcet = spapr_tce_new_table(DEVICE(sphb), sphb-dma_liobn,
 0,
 SPAPR_TCE_PAGE_SHIFT,
 -   0x4000  SPAPR_TCE_PAGE_SHIFT);
 +   0x4000  SPAPR_TCE_PAGE_SHIFT, true);
  if (!tcet) {
  error_setg(errp, Unable to create TCE table for %s,
 sphb-dtbusname);
 diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
 index 48b0125..16385e4 100644
 --- a/hw/ppc/spapr_vio.c
 +++ b/hw/ppc/spapr_vio.c
 @@ -460,7 +460,7 @@ static int spapr_vio_busdev_init(DeviceState *qdev)
  0,
  SPAPR_TCE_PAGE_SHIFT,
  pc-rtce_window_size 
 -SPAPR_TCE_PAGE_SHIFT);
 +SPAPR_TCE_PAGE_SHIFT, true);
  address_space_init(dev-as, spapr_tce_get_iommu(dev-tcet), 
 qdev-id);
  }
  
 diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
 index 4ffb903..7db34ff 100644
 --- a/include/hw/ppc/spapr.h
 +++ b/include/hw/ppc/spapr.h
 @@ -402,6 +402,7 @@ struct sPAPRTCETable {
  uint32_t page_shift;
  uint64_t *table;
  bool bypass;
 +bool kvm_accel;
  int fd;
  MemoryRegion iommu;
  QLIST_ENTRY(sPAPRTCETable) list;
 @@ 

Re: [Qemu-devel] [PATCH 2/2] virtio-blk-test.c: add hotplug subtest

2014-06-05 Thread Markus Armbruster
Amos Kong ak...@redhat.com writes:

 On Thu, Jun 05, 2014 at 01:12:46PM +0800, Amos Kong wrote:
 On Wed, Jun 04, 2014 at 05:35:06PM +0200, Stefan Hajnoczi wrote:
  On Tue, May 27, 2014 at 03:14:41PM +0800, Amos Kong wrote:
   This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
   devices to guest, and try to hot-unplug them.
   
   Note: the hot-unplug can't work without cooperation of guest OS.
   
   Signed-off-by: Amos Kong ak...@redhat.com
   ---
tests/virtio-blk-test.c | 55 
   +
1 file changed, 55 insertions(+)
   
   diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
   index 0fdec01..54d1272 100644
   --- a/tests/virtio-blk-test.c
   +++ b/tests/virtio-blk-test.c
   @@ -7,11 +7,65 @@
 * See the COPYING file in the top-level directory.
 */

   +#include stdio.h
#include glib.h
#include string.h
#include libqtest.h
#include qemu/osdep.h

   +static void exec_hmp_cmd(const char *cmd, const char *expected_ret)
  
  This is a generic function, should it be moved to libqtest.c?
 
 OK
 
  
   +{
   +QDict *response;
   +const char *response_return;
   +
   +response = qmp({\execute\: \human-monitor-command\,
   +\arguments\: {
   +  \command-line\: \%s\
   +   }}, cmd);
  
  My only worry if we make this function generic is that cmd isn't escaped
  so callers need to be careful.
  Maybe something like g_strescape() should be used:
  
  https://developer.gnome.org/glib/2.37/glib-String-Utility-Functions.html#g-strescape
 
  
   +g_assert(response);
   +response_return = qdict_get_try_str(response, return);
   +g_assert(response_return);
   +g_assert(strcmp(response_return, expected_ret) == 0);
  
  Please use g_assert_cmpstr() so the error message will be pretty-printed
  with the values of response_return and expected_ret:
  https://developer.gnome.org/glib/stable/glib-Testing.html#g-assert-cmpstr
  
 OK
 
   +QDECREF(response);
   +}
  
   +static void test_blk_hotplug(void)
   +{
   +char addr[6];
   +char cmd[100];
   +int i, j;
   +
   +/* Start with no network/block device, slots 3~0x1f are free */
   +qtest_start(-net none);
   +
   +for (i = 3; i = 0x1f; i++) {
   +for (j = 7; j = 0; j--) {
   +sprintf(addr, %x.%x, i, j);
   + sprintf(cmd, drive_add 0x%s
   if=none,file=/dev/null,id=drv-%s,
   +addr, addr);
  
  Does the address matter with if=none?  I think you can just do
  drive_add 0 if=none,
 
 drive_add [[domain:]bus:]slot
 
 multiple function is enabled, slot parameter is needed.

 pci addr is assigned in device_add command, and it contains drive's id.
 I tried to just use drive_add 0 if=none,..., it also works.

So does

(qemu) drive_add  if=none,file=tmp.qcow2
OK

or even

(qemu) drive_add I'm a little teapot if=none,file=tmp.qcow2
OK

because the first argument is ignored.

Fortunately we're getting close to replacing this badly designed
command.



[Qemu-devel] [PATCH v8 3/4] VFIO: Introduce helper vfio_pci_container_ioctl()

2014-06-05 Thread Gavin Shan
The patch introduces helper function vfio_pci_container_ioctl() to
pass ioctl commands to the specified VFIO container that is identified
by IOMMU group id. On sPAPR platform, each container only has one
IOMMU group.

Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
 hw/misc/vfio.c | 31 +++
 include/hw/misc/vfio.h |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 0796abf..999d97d 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -4310,3 +4310,34 @@ put_group_exit:
 
 return n;
 }
+
+int vfio_pci_container_ioctl(int iommu_group_id, int req, int opt)
+{
+VFIOGroup *group;
+int ret, fd = 0;
+
+/* Search container's fd */
+QLIST_FOREACH(group, group_list, next) {
+if (group-groupid == iommu_group_id) {
+fd = group-container ? group-container-fd : 0;
+break;
+}
+}
+
+if (fd = 0) {
+return -ENOENT;
+}
+
+switch (req) {
+case VFIO_EEH_PE_OP: {
+   struct vfio_eeh_pe_op op = { .argsz = sizeof(op), .op = opt };
+
+ret = ioctl(fd, req, op);
+break;
+}
+default:
+ret = -EINVAL;
+}
+
+return ret;
+}
diff --git a/include/hw/misc/vfio.h b/include/hw/misc/vfio.h
index 53ec665..dc92fae 100644
--- a/include/hw/misc/vfio.h
+++ b/include/hw/misc/vfio.h
@@ -30,4 +30,6 @@ static inline long vfio_kvm_notify(Notifier *n, unsigned 
request, void *data)
 return p.ret;
 }
 
+extern int vfio_pci_container_ioctl(int iommu_group_id, int req, int opt);
+
 #endif
-- 
1.8.3.2




[Qemu-devel] [PATCH v8 4/4] sPAPR: Implement sPAPRPHBClass::eeh_handler

2014-06-05 Thread Gavin Shan
The patch implements sPAPRPHBClass::eeh_handler so that the
EEH RTAS requests can be routed to VFIO for further handling.

Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
 hw/ppc/spapr_pci_vfio.c | 54 +
 1 file changed, 54 insertions(+)

diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index decf3dd..94bebc2 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -86,6 +86,59 @@ static void spapr_phb_vfio_finish_realize(sPAPRPHBState 
*sphb, Error **errp)
   spapr_tce_get_iommu(tcet));
 }
 
+static int spapr_phb_vfio_eeh_handler(sPAPRPHBState *sphb, int req, int opt)
+{
+sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
+int cmd;
+
+switch (req) {
+case RTAS_EEH_REQ_SET_OPTION:
+switch (opt) {
+case RTAS_EEH_DISABLE:
+cmd = VFIO_EEH_PE_DISABLE;
+break;
+case RTAS_EEH_ENABLE:
+cmd = VFIO_EEH_PE_ENABLE;
+break;
+case RTAS_EEH_THAW_IO:
+cmd = VFIO_EEH_PE_UNFREEZE_IO;
+break;
+case RTAS_EEH_THAW_DMA:
+cmd = VFIO_EEH_PE_UNFREEZE_DMA;
+break;
+default:
+return -EINVAL;
+}
+break;
+case RTAS_EEH_REQ_GET_STATE:
+cmd = VFIO_EEH_PE_GET_STATE;
+break;
+case RTAS_EEH_REQ_RESET:
+switch (opt) {
+case RTAS_SLOT_RESET_DEACTIVATE:
+cmd = VFIO_EEH_PE_RESET_DEACTIVATE;
+break;
+case RTAS_SLOT_RESET_HOT:
+cmd = VFIO_EEH_PE_RESET_HOT;
+break;
+case RTAS_SLOT_RESET_FUNDAMENTAL:
+cmd = VFIO_EEH_PE_RESET_FUNDAMENTAL;
+break;
+default:
+return -EINVAL;
+}
+break;
+case RTAS_EEH_REQ_CONFIGURE:
+cmd = VFIO_EEH_PE_CONFIGURE;
+break;
+default:
+ return -EINVAL;
+}
+
+return vfio_pci_container_ioctl(svphb-iommugroupid,
+VFIO_EEH_PE_OP, cmd);
+}
+
 static void spapr_phb_vfio_reset(DeviceState *qdev)
 {
 /* Do nothing */
@@ -99,6 +152,7 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass, 
void *data)
 dc-props = spapr_phb_vfio_properties;
 dc-reset = spapr_phb_vfio_reset;
 spc-finish_realize = spapr_phb_vfio_finish_realize;
+spc-eeh_handler = spapr_phb_vfio_eeh_handler;
 }
 
 static const TypeInfo spapr_phb_vfio_info = {
-- 
1.8.3.2




[Qemu-devel] [PATCH v8 2/4] headers: Update kernel header

2014-06-05 Thread Gavin Shan
This updates kernel header (vfio.h) for EEH support on VFIO PCI
devices.

Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
 linux-headers/linux/vfio.h | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index f0aa97d..cf703d3 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -30,6 +30,9 @@
  */
 #define VFIO_DMA_CC_IOMMU  4
 
+/* Check if EEH is supported */
+#define VFIO_EEH   5
+
 /*
  * The IOCTL interface is designed for extensibility by embedding the
  * structure length (argsz) and flags into structures passed between
@@ -490,6 +493,38 @@ struct vfio_iommu_spapr_tce_reset {
 };
 #define VFIO_IOMMU_SPAPR_TCE_RESET _IO(VFIO_TYPE, VFIO_BASE + 20)
 
+/*
+ * EEH PE operation struct provides ways to:
+ * - enable/disable EEH functionality;
+ * - unfreeze IO/DMA for frozen PE;
+ * - read PE state;
+ * - reset PE;
+ * - configure PE.
+ */
+struct vfio_eeh_pe_op {
+   __u32 argsz;
+   __u32 flags;
+   __u32 op;
+};
+
+#define VFIO_EEH_PE_DISABLE0   /* Disable EEH functionality */
+#define VFIO_EEH_PE_ENABLE 1   /* Enable EEH functionality  */
+#define VFIO_EEH_PE_UNFREEZE_IO2   /* Enable IO for frozen 
PE   */
+#define VFIO_EEH_PE_UNFREEZE_DMA   3   /* Enable DMA for frozen PE  */
+#define VFIO_EEH_PE_GET_STATE  4   /* PE state retrieval*/
+#define VFIO_EEH_PE_RESET_DEACTIVATE   5   /* Deassert PE reset */
+#define VFIO_EEH_PE_RESET_HOT  6   /* Assert hot reset  */
+#define VFIO_EEH_PE_RESET_FUNDAMENTAL  7   /* Assert fundamental reset  */
+#define VFIO_EEH_PE_CONFIGURE  8   /* PE configuration  */
+
+#define VFIO_EEH_PE_STATE_NORMAL   0   /* PE in functional state*/
+#define VFIO_EEH_PE_STATE_RESET1   /* PE reset in progress 
 */
+#define VFIO_EEH_PE_STATE_STOPPED  2   /* Stopped DMA and IO*/
+#define VFIO_EEH_PE_STATE_STOPPED_DMA  4   /* Stopped DMA only  */
+#define VFIO_EEH_PE_STATE_UNAVAIL  5   /* State unavailable */
+
+#define VFIO_EEH_PE_OP _IO(VFIO_TYPE, VFIO_BASE + 21)
+
 /* * */
 
 #endif /* VFIO_H */
-- 
1.8.3.2




[Qemu-devel] [PATCH v8 0/4] EEH Support for VFIO PCI Device

2014-06-05 Thread Gavin Shan
The series of patches adds support EEH for VFIO PCI devices on sPAPR platform.
It requires corresponding host kernel support. Also, it is based on top of
Alexey's VFIO-for-sPAPR git repository.

QEMU:   git://github.com/aik/qemu.git (branch: vfio)
Kernel: git://github.com/aik/linux.git (branch: vfio)
Kernel: 
http://linuxppc.10917.n7.nabble.com/PATCH-v8-0-3-EEH-Support-for-VFIO-PCI-Device-td82940.html

The implementations notes are below. Please comment.

* RTAS calls are received in spapr_pci.c, sanity check is done there. RTAS
  handlers handle what they can. If there is something it cannot handle and
  sPAPRPHBClass::eeh_handler callback is defined, it is called.
* sPAPRPHBClass::eeh_handler is only implemented for VFIO now. It does ioctl()
  to the IOMMU container fd to complete the call. Error codes from that ioctl()
  are transferred back to the guest.

Gavin Shan (4):
  sPAPR: Implement EEH RTAS calls
  headers: Update kernel header
  VFIO: Introduce helper vfio_pci_container_ioctl()
  sPAPR: Implement sPAPRPHBClass::eeh_handler

 hw/misc/vfio.c  |  31 ++
 hw/ppc/spapr_pci.c  | 248 
 hw/ppc/spapr_pci_vfio.c |  54 ++
 include/hw/misc/vfio.h  |   2 +
 include/hw/pci-host/spapr.h |   7 ++
 include/hw/ppc/spapr.h  |  33 ++
 linux-headers/linux/vfio.h  |  35 +++
 7 files changed, 410 insertions(+)

-- 
1.8.3.2




[Qemu-devel] [PATCH v8 1/4] sPAPR: Implement EEH RTAS calls

2014-06-05 Thread Gavin Shan
The emulation for EEH RTAS requests from guest isn't covered
by QEMU yet and the patch implements them.

The patch defines constants used by EEH RTAS calls and adds
callback sPAPRPHBClass::eeh_handler, which is going to be used
this way:

1. RTAS calls are received in spapr_pci.c, sanity check is done
   there.
2. RTAS handlers handle what they can. If there is something it
   cannot handle and sPAPRPHBClass::eeh_handler callback is defined,
   it is called.
3. sPAPRPHBClass::eeh_handler is only implemented for VFIO now. It
   does ioctl() to the IOMMU container fd to complete the call. Error
   codes from that ioctl() are transferred back to the guest.

This adds 6 RTAS handlers, all defined in SPAPR specification:

1) ibm,set-eeh-option: disables/enables EEH on a device, removes PE from
stopped state;
2) ibm,get-config-addr-info2 - returns fabric configuration address (upper
PCI bridge or PHB if there is no bridge);
3) ibm,read-slot-reset-state2 - retrieve PE state;
4) ibm,set-slot-reset - issue PE reset;
5) ibm,configure-pe - configure PCI bridges in the affected PE;
6) ibm,slot-error-detail - retrieve EEH error log;

All calls use fabric configuration address (a.k.a. PE address) as a target
address except ibm,get-config-addr-info2 and one case (enable EEH on the
specified PCI function) for ibm,set-eeh-option.

Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
 hw/ppc/spapr_pci.c  | 248 
 include/hw/pci-host/spapr.h |   7 ++
 include/hw/ppc/spapr.h  |  33 ++
 3 files changed, 288 insertions(+)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index a9f307a..423e4ff 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -422,6 +422,241 @@ static void 
rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu,
 rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */
 }
 
+static int rtas_finish_eeh_request(sPAPRPHBState *sphb,
+   uint32_t req, uint32_t opt,
+   target_ulong rets)
+{
+sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
+int ret;
+
+ret = info-eeh_handler(sphb, req, opt);
+if (ret = 0) {
+rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+} else {
+rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+}
+
+return ret;
+}
+
+static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
+sPAPREnvironment *spapr,
+uint32_t token, uint32_t nargs,
+target_ulong args, uint32_t nret,
+target_ulong rets)
+{
+uint32_t addr, option;
+uint64_t buid = ((uint64_t)rtas_ld(args, 1)  32) | rtas_ld(args, 2);
+sPAPRPHBState *sphb = spapr_find_phb(spapr, buid);
+sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
+
+if (!sphb || !info-eeh_handler) {
+goto param_error_exit;
+}
+
+if ((nargs != 4) || (nret != 1)) {
+goto param_error_exit;
+}
+
+addr = rtas_ld(args, 0);
+option = rtas_ld(args, 3);
+switch (option) {
+case RTAS_EEH_ENABLE:
+if (!find_dev(spapr, buid, addr)) {
+goto param_error_exit;
+}
+break;
+case RTAS_EEH_DISABLE:
+case RTAS_EEH_THAW_IO:
+case RTAS_EEH_THAW_DMA:
+break;
+default:
+goto param_error_exit;
+}
+
+rtas_finish_eeh_request(sphb, RTAS_EEH_REQ_SET_OPTION, option, rets);
+return;
+
+param_error_exit:
+rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+}
+
+static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
+   sPAPREnvironment *spapr,
+   uint32_t token, uint32_t nargs,
+   target_ulong args, uint32_t nret,
+   target_ulong rets)
+{
+uint32_t addr, option;
+uint64_t buid = ((uint64_t)rtas_ld(args, 1)  32) | rtas_ld(args, 2);
+sPAPRPHBState *sphb = spapr_find_phb(spapr, buid);
+sPAPRPHBClass *info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
+PCIDevice *pdev;
+
+if (!sphb || !info-eeh_handler) {
+goto param_error_exit;
+}
+
+if ((nargs != 4) || (nret != 2)) {
+goto param_error_exit;
+}
+
+addr = rtas_ld(args, 0);
+option = rtas_ld(args, 3);
+if (option != RTAS_GET_PE_ADDR  option != RTAS_GET_PE_MODE) {
+goto param_error_exit;
+}
+
+pdev = find_dev(spapr, buid, addr);
+if (!pdev) {
+goto param_error_exit;
+}
+
+/*
+ * For now, we always have bus level PE whose address
+ * has format 00BBSS00. The guest OS might regard
+ * PE address 0 as invalid. We avoid that simply by
+ * extending it with one.
+ */
+rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+if (option == RTAS_GET_PE_ADDR) {
+rtas_st(rets, 1, (pci_bus_num(pdev-bus)  16) + 1);
+} else {
+rtas_st(rets, 

Re: [Qemu-devel] active block commit bug?

2014-06-05 Thread Markus Armbruster
Eric Blake ebl...@redhat.com writes:

 On 06/04/2014 08:09 PM, Fam Zheng wrote:

 Sounds like we have an off-by-one condition if empty files behave
 differently from other files.  We ought to fix that bug (not that your
 normal guest will ever have a 0-length backing file, but this was what I
 was trying to use for libvirt's probing of whether active commit is
 supported)

 
 Yes, agreed, this special case is only going to make management confused. I
 will send a patch to fix this.

 Thanks.

 
 Eric, is this a good way to probe the active commit? I was expecting full
 instrospection of QMP could do it, but I don't know about the status of that
 piece of work. Amos, any ideas?

 Introspection already missed qemu 2.0 when active commit was added; and
 we're close enough to soft freeze for 2.1 that I'm guessing it will miss
 2.1 as well :(

Almost certainly.  It has non-trivial design issues.  To have a chance
to make it into 2.x, it needs to be posted for review early in the 2.x
cycle.

[...]



Re: [Qemu-devel] [PATCH v2 2/4] qtest: introduce qmp_exec_hmp_cmd()

2014-06-05 Thread Gonglei (Arei)
 -Original Message-
 From: qemu-devel-bounces+arei.gonglei=huawei@nongnu.org
 [mailto:qemu-devel-bounces+arei.gonglei=huawei@nongnu.org] On
 Behalf Of Amos Kong
 Sent: Thursday, June 05, 2014 1:45 PM
 To: qemu-devel@nongnu.org
 Cc: stefa...@gmail.com; afaer...@suse.de; m...@redhat.com
 Subject: [Qemu-devel] [PATCH v2 2/4] qtest: introduce qmp_exec_hmp_cmd()
 
 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  tests/libqtest.c | 16 
  tests/libqtest.h |  8 
  2 files changed, 24 insertions(+)
 
Nice job :)

I will use it for usb host adapters hotplug/unplug qtests.


Best regards,
-Gonglei



Re: [Qemu-devel] qemu does not support PAPR

2014-06-05 Thread Paolo Bonzini

Il 05/06/2014 08:09, sonia verma ha scritto:

Hi Paolo

I'm running this on ubuntu with powerpc arch with kernel
3.8.13-rt9-QorIQ-SDK-V1.4.


I'll note that this is a fairly old kernel for PowerPC KVM.  However, 
the problem looks like libguestfs supporting only server PowerPC (also 
known as book3s), not embedded (booke).


Paolo



Re: [Qemu-devel] qemu does not support PAPR

2014-06-05 Thread sonia verma
So what can be the solution for this?



On Thu, Jun 5, 2014 at 1:13 PM, Paolo Bonzini pbonz...@redhat.com wrote:

 Il 05/06/2014 08:09, sonia verma ha scritto:

  Hi Paolo

 I'm running this on ubuntu with powerpc arch with kernel
 3.8.13-rt9-QorIQ-SDK-V1.4.


 I'll note that this is a fairly old kernel for PowerPC KVM.  However, the
 problem looks like libguestfs supporting only server PowerPC (also known
 as book3s), not embedded (booke).

 Paolo



Re: [Qemu-devel] [PATCHv3 RESEND] block: introduce BDRV_O_SEQUENTIAL

2014-06-05 Thread Stefan Hajnoczi
On Wed, Jun 04, 2014 at 05:31:48PM +0200, Peter Lieven wrote:
 Am 04.06.2014 17:12, schrieb Stefan Hajnoczi:
  On Fri, May 30, 2014 at 11:40:37PM +0200, Peter Lieven wrote:
  this patch introduces a new flag to indicate that we are going to 
  sequentially
  read from a file and do not plan to reread/reuse the data after it has 
  been read.
 
  The current use of this flag is to open the source(s) of a qemu-img convert
  process. If a protocol from block/raw-posix.c is used posix_fadvise is 
  utilized
  to advise to the kernel that we are going to read sequentially from the
  file and a POSIX_FADV_DONTNEED advise is issued after each write to 
  indicate
  that there is no advantage keeping the blocks in the buffers.
 
  Consider the following test case that was created to confirm the behaviour 
  of
  the new flag:
 
  A 10G logical volume was created and filled with random data.
  Then the logical volume was exported via qemu-img convert to an iscsi 
  target.
  Before the export was started all caches of the linux kernel where dropped.
 
  Old behavior:
   - The convert process took 3m45s and the buffer cache grew up to 9.67 GB 
  close
 to the end of the conversion. After qemu-img terminated all the buffers 
  were
 freed by the kernel.
 
  New behavior with the -N switch:
   - The convert process took 3m43s and the buffer cache grew up to 15.48 MB 
  close
 to the end with some small peaks up to 30 MB during the conversion.
  FADVISE_SEQUENTIAL can be good since it doubles read-ahead on Linux.
 
  I'm skeptical of the effort to avoid buffer cache usage using
  FADVISE_DONTNEED.  The performance results tell me that less buffer
  cache was used but that number doesn't have a direct effect on
  application performance.
 
  Let's check GNU coreutils:
 
$ cd coreutils
$ git grep FADVISE_DONTNEED
gl/lib/fadvise.h:  FADVISE_DONTNEED =   POSIX_FADV_DONTNEED,
gl/lib/fadvise.h:  FADVISE_DONTNEED,
$
 
  GNU cp(1) does not care about minimizing impact on buffer cache using
  FADVISE_DONTNEED.  It just sets FADVISE_SEQUENTIAL on the source file
  and calls read() (plus uses FIEMAP to check extents for sparseness).
 
  I want to avoid adding code just for the heck of it.  We need a deeper
  understanding:
 
  Please drop FADVISE_DONTNEED and compare again to see if it changes the
  benchmark.
 
  By the way, did you perform several runs to check the variance of the
  running time?  I don't know if the 2 seconds difference were noise or
  because FADVISE_SEQUENTIAL or because FADVISE_DONTNEED or because both.
 
 There was no effect on the runtime as far as I remember. I ran
 some tests, but not a number large enough to filter out the noise.
 
 I created this one because we saw it helps under memory pressure.
 Maybe its too specific to add it into mainline qemu, but I wanted to
 avoid to have too much individual changes we need to maintain.

I'm open to merging it if the improvement can be quantified.  Right now
this might be a workaround for Linux memory management heuristics or it
might not have any effect, I don't know.

 
  diff --git a/block/raw-posix.c b/block/raw-posix.c
  index 6586a0c..9768cc4 100644
  --- a/block/raw-posix.c
  +++ b/block/raw-posix.c
  @@ -447,6 +447,13 @@ static int raw_open_common(BlockDriverState *bs, 
  QDict *options,
   }
   #endif
   
  +#ifdef POSIX_FADV_SEQUENTIAL
  +if (bs-open_flags  BDRV_O_SEQUENTIAL 
  +!(bs-open_flags  BDRV_O_NOCACHE)) {
  +posix_fadvise(s-fd, 0, 0, POSIX_FADV_SEQUENTIAL);
  +}
  +#endif
  This is only true if the image format is raw.  If the image format on
  top of this raw-posix BDS is non-raw then the read pattern may not be
  sequential.
 
 You are right, but will the other formats set BDRV_O_SEQUENTIAL?

If the user specifies qemu-img convert -N then it will be set for any
image format.

Maybe qemu-img convert can always set BDRV_O_SEQUENTIAL and the have the
raw_bsd.c format propagate it to bs-file while other formats do not.
Then the user doesn't have to specify a command-line option and we don't
set it for non-raw image formats.

Stefan



Re: [Qemu-devel] [Qemu-ppc] qemu does not support PAPR

2014-06-05 Thread Alexander Graf


 Am 05.06.2014 um 09:45 schrieb sonia verma soniaverma9...@gmail.com:
 
 
 So what can be the solution for this?

Please don't top post.

You're running on QorIQ which is a Freescale chip. The pseries target only 
works on IBM PPC chips.

The target you're looking for is ppce500:

  $ qemu-system-ppc64 -M ppce500 -nographic -kernel uImage ...

I don't know about the status of libvirt in your sdk version, but I'm sure 
Stuart does, so let's ask him ;).


Alex




Re: [Qemu-devel] 答复: Expansion Ratio Issue

2014-06-05 Thread Alex Bennée

Chaos Shu writes:

 Hi

 I'm running SPEC CPU2006 on three kinds of situation, native aarch64 binary 
 and emulator x86_64 system running SPEC CPU2006 and linux user mode level 
 running x86_64 SPEC CPU2006 binary.

 To find where the performance lose, translator ? or execution of instruction 
 after TCG? Or something else

 I guess most of  time, up to 90% should be spent on exec the
 instruction of TCG, does that mean the quality of translating lead to
 the performance lost directly ?

It really depends on the type of code you are executing but yes most of
the time should be spent in TCG generated code. However if you are
running a lot of FP heavy code you'll find it spends a lot of time in
helper routines calling the internal softfloat code.

I posted some patches a few months ago that enabled output to help the
Linux perf tool track this. I haven't got time to re-work at the
moment but it might give you a head start to instrumentation:

https://patches.linaro.org/27229/


 Thanks
 Chaos

 On 29.05.2014 13:04, Peter Maydell wrote:
 No, we don't in general have any benchmarking of TCG codegen. I think 
 if we did do benchmarking we'd be interested in performance 
 benchmarking -- code expansion ratio doesn't seem like a very 
 interesting thing to measure to me.

 Hi,

 I have a plan to play with TCG performance benchmarking. And then try to 
 implement some optimizations. So maybe there would be some suggestions on how 
 to perform such benchmarking? What tests seems to be appropriate for this 
 task? I think the benchmarking should reflect real TCG use cases. So what the 
 most typical use cases for TCG are there? Seems that system and user modes 
 may be different from this point.

 Appreciate any help.

 Thanks,
 Sergey.

-- 
Alex Bennée



Re: [Qemu-devel] [PATCHv3 RESEND] block: introduce BDRV_O_SEQUENTIAL

2014-06-05 Thread Peter Lieven

On 05.06.2014 09:53, Stefan Hajnoczi wrote:

On Wed, Jun 04, 2014 at 05:31:48PM +0200, Peter Lieven wrote:

Am 04.06.2014 17:12, schrieb Stefan Hajnoczi:

On Fri, May 30, 2014 at 11:40:37PM +0200, Peter Lieven wrote:

this patch introduces a new flag to indicate that we are going to sequentially
read from a file and do not plan to reread/reuse the data after it has been 
read.

The current use of this flag is to open the source(s) of a qemu-img convert
process. If a protocol from block/raw-posix.c is used posix_fadvise is utilized
to advise to the kernel that we are going to read sequentially from the
file and a POSIX_FADV_DONTNEED advise is issued after each write to indicate
that there is no advantage keeping the blocks in the buffers.

Consider the following test case that was created to confirm the behaviour of
the new flag:

A 10G logical volume was created and filled with random data.
Then the logical volume was exported via qemu-img convert to an iscsi target.
Before the export was started all caches of the linux kernel where dropped.

Old behavior:
  - The convert process took 3m45s and the buffer cache grew up to 9.67 GB close
to the end of the conversion. After qemu-img terminated all the buffers were
freed by the kernel.

New behavior with the -N switch:
  - The convert process took 3m43s and the buffer cache grew up to 15.48 MB 
close
to the end with some small peaks up to 30 MB during the conversion.

FADVISE_SEQUENTIAL can be good since it doubles read-ahead on Linux.

I'm skeptical of the effort to avoid buffer cache usage using
FADVISE_DONTNEED.  The performance results tell me that less buffer
cache was used but that number doesn't have a direct effect on
application performance.

Let's check GNU coreutils:

   $ cd coreutils
   $ git grep FADVISE_DONTNEED
   gl/lib/fadvise.h:  FADVISE_DONTNEED =   POSIX_FADV_DONTNEED,
   gl/lib/fadvise.h:  FADVISE_DONTNEED,
   $

GNU cp(1) does not care about minimizing impact on buffer cache using
FADVISE_DONTNEED.  It just sets FADVISE_SEQUENTIAL on the source file
and calls read() (plus uses FIEMAP to check extents for sparseness).

I want to avoid adding code just for the heck of it.  We need a deeper
understanding:

Please drop FADVISE_DONTNEED and compare again to see if it changes the
benchmark.

By the way, did you perform several runs to check the variance of the
running time?  I don't know if the 2 seconds difference were noise or
because FADVISE_SEQUENTIAL or because FADVISE_DONTNEED or because both.

There was no effect on the runtime as far as I remember. I ran
some tests, but not a number large enough to filter out the noise.

I created this one because we saw it helps under memory pressure.
Maybe its too specific to add it into mainline qemu, but I wanted to
avoid to have too much individual changes we need to maintain.

I'm open to merging it if the improvement can be quantified.  Right now
this might be a workaround for Linux memory management heuristics or it
might not have any effect, I don't know.


I understand that you are critical about it. I can just say it solved
the problem with the specific setup, kernel version etc.

I found that FADVISE_DONTNEED solves problems also in other applications.
Offtopic: i have an raspberry pi running as tvheadend and observed desync
of the DVBS2 signal at some times. Since I FADV_DONTNEED all written
frames away it runs smothly. I this case the feeing of the page cache was
CPU intensive for the small device and caused the desync.




diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6586a0c..9768cc4 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -447,6 +447,13 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
  }
  #endif
  
+#ifdef POSIX_FADV_SEQUENTIAL

+if (bs-open_flags  BDRV_O_SEQUENTIAL 
+!(bs-open_flags  BDRV_O_NOCACHE)) {
+posix_fadvise(s-fd, 0, 0, POSIX_FADV_SEQUENTIAL);
+}
+#endif

This is only true if the image format is raw.  If the image format on
top of this raw-posix BDS is non-raw then the read pattern may not be
sequential.

You are right, but will the other formats set BDRV_O_SEQUENTIAL?

If the user specifies qemu-img convert -N then it will be set for any
image format.


Of course, but when e.g. qcow2 opens its underlying file, then BDRV_O_SEQUENTIAL
is not passed on, or is it?



Maybe qemu-img convert can always set BDRV_O_SEQUENTIAL and the have the
raw_bsd.c format propagate it to bs-file while other formats do not.
Then the user doesn't have to specify a command-line option and we don't
set it for non-raw image formats.


This would be an option.

Peter



Re: [Qemu-devel] [PATCHv3 RESEND] block: introduce BDRV_O_SEQUENTIAL

2014-06-05 Thread Kevin Wolf
Am 05.06.2014 um 10:09 hat Peter Lieven geschrieben:
 On 05.06.2014 09:53, Stefan Hajnoczi wrote:
 On Wed, Jun 04, 2014 at 05:31:48PM +0200, Peter Lieven wrote:
 Am 04.06.2014 17:12, schrieb Stefan Hajnoczi:
 On Fri, May 30, 2014 at 11:40:37PM +0200, Peter Lieven wrote:
 this patch introduces a new flag to indicate that we are going to 
 sequentially
 read from a file and do not plan to reread/reuse the data after it has 
 been read.
 
 The current use of this flag is to open the source(s) of a qemu-img 
 convert
 process. If a protocol from block/raw-posix.c is used posix_fadvise is 
 utilized
 to advise to the kernel that we are going to read sequentially from the
 file and a POSIX_FADV_DONTNEED advise is issued after each write to 
 indicate
 that there is no advantage keeping the blocks in the buffers.
 
 Consider the following test case that was created to confirm the 
 behaviour of
 the new flag:
 
 A 10G logical volume was created and filled with random data.
 Then the logical volume was exported via qemu-img convert to an iscsi 
 target.
 Before the export was started all caches of the linux kernel where 
 dropped.
 
 Old behavior:
   - The convert process took 3m45s and the buffer cache grew up to 9.67 
  GB close
 to the end of the conversion. After qemu-img terminated all the 
  buffers were
 freed by the kernel.
 
 New behavior with the -N switch:
   - The convert process took 3m43s and the buffer cache grew up to 15.48 
  MB close
 to the end with some small peaks up to 30 MB during the conversion.
 FADVISE_SEQUENTIAL can be good since it doubles read-ahead on Linux.
 
 I'm skeptical of the effort to avoid buffer cache usage using
 FADVISE_DONTNEED.  The performance results tell me that less buffer
 cache was used but that number doesn't have a direct effect on
 application performance.
 
 Let's check GNU coreutils:
 
$ cd coreutils
$ git grep FADVISE_DONTNEED
gl/lib/fadvise.h:  FADVISE_DONTNEED =   POSIX_FADV_DONTNEED,
gl/lib/fadvise.h:  FADVISE_DONTNEED,
$
 
 GNU cp(1) does not care about minimizing impact on buffer cache using
 FADVISE_DONTNEED.  It just sets FADVISE_SEQUENTIAL on the source file
 and calls read() (plus uses FIEMAP to check extents for sparseness).
 
 I want to avoid adding code just for the heck of it.  We need a deeper
 understanding:
 
 Please drop FADVISE_DONTNEED and compare again to see if it changes the
 benchmark.
 
 By the way, did you perform several runs to check the variance of the
 running time?  I don't know if the 2 seconds difference were noise or
 because FADVISE_SEQUENTIAL or because FADVISE_DONTNEED or because both.
 There was no effect on the runtime as far as I remember. I ran
 some tests, but not a number large enough to filter out the noise.
 
 I created this one because we saw it helps under memory pressure.
 Maybe its too specific to add it into mainline qemu, but I wanted to
 avoid to have too much individual changes we need to maintain.
 I'm open to merging it if the improvement can be quantified.  Right now
 this might be a workaround for Linux memory management heuristics or it
 might not have any effect, I don't know.
 
 I understand that you are critical about it. I can just say it solved
 the problem with the specific setup, kernel version etc.
 
 I found that FADVISE_DONTNEED solves problems also in other applications.
 Offtopic: i have an raspberry pi running as tvheadend and observed desync
 of the DVBS2 signal at some times. Since I FADV_DONTNEED all written
 frames away it runs smothly. I this case the feeing of the page cache was
 CPU intensive for the small device and caused the desync.
 
 
 diff --git a/block/raw-posix.c b/block/raw-posix.c
 index 6586a0c..9768cc4 100644
 --- a/block/raw-posix.c
 +++ b/block/raw-posix.c
 @@ -447,6 +447,13 @@ static int raw_open_common(BlockDriverState *bs, 
 QDict *options,
   }
   #endif
 +#ifdef POSIX_FADV_SEQUENTIAL
 +if (bs-open_flags  BDRV_O_SEQUENTIAL 
 +!(bs-open_flags  BDRV_O_NOCACHE)) {
 +posix_fadvise(s-fd, 0, 0, POSIX_FADV_SEQUENTIAL);
 +}
 +#endif
 This is only true if the image format is raw.  If the image format on
 top of this raw-posix BDS is non-raw then the read pattern may not be
 sequential.
 You are right, but will the other formats set BDRV_O_SEQUENTIAL?
 If the user specifies qemu-img convert -N then it will be set for any
 image format.
 
 Of course, but when e.g. qcow2 opens its underlying file, then 
 BDRV_O_SEQUENTIAL
 is not passed on, or is it?

It isn't qcow2 but block.c that opens bs-file, and unless you
explicitly filter out a flag, bs-file inherits it. (If it didn't do
that, your patch would have no effect for raw either.)

 Maybe qemu-img convert can always set BDRV_O_SEQUENTIAL and the have the
 raw_bsd.c format propagate it to bs-file while other formats do not.
 Then the user doesn't have to specify a command-line option and we don't
 set it for non-raw image formats.
 
 This would be an option.

I agree, 

Re: [Qemu-devel] [RFC 3/5] nbd: Use aio_set_fd_handler2()

2014-06-05 Thread Stefan Hajnoczi
On Wed, Jun 04, 2014 at 08:02:06PM +0200, Paolo Bonzini wrote:
 Il 04/06/2014 14:37, Stefan Hajnoczi ha scritto:
 Why is this design cleaner?  Because NBD code doesn't have to worry
 about fd handlers.  It uses straightforward coroutine send/recv for
 socket I/O inside nbd_read_req() and nbd_write_resp().  It's easy to see
 that only one coroutine receives from the socket and that only one
 coroutine writes to the socket.
 
 I don't understand how this would work without managing fd handlers.

fd handlers still need to be managed, but not by NBD code.  They must be
managed by coroutine recv/send utility functions.  In other words, fd
handlers are used locally, not globally.

def co_recv(fd, buf):
while True:
nbytes = recv(fd, buf, len(buf))
if nbytes == -1:
if errno == EINTR:
continue
if errno == EAGAIN or errno == EWOULDBLOCK:
aio_set_fd_read_handler(fd, co_recv_cb)
qemu_coroutine_yield()
aio_set_fd_read_handler(fd, NULL)
continue
return nbytes

The send function is similar.

This does require an extension to the AioContext API.  We need to be
able to modify the read/write callback independently without clobbering
the other callback.  This way full duplex I/O is possible.

 - If you don't want to receive anything (because you reached the maximum
 queue depth), and the client sends something, the read handler will busy
 wait.  The current code solves it with can_read; you could do it by enabling
 or disabling the read handler as you need, but the idea is the same.
 
 - If you're not sending anything, a socket that has a write handler will
 have POLLOUT continuously signaled and again you'd busy wait.  Since there
 is no can_write, nbd.c instead enables/disables the write handler around
 nbd_co_send_reply.

You only install an fd handler when you want to read/write.  This does
mean that the request coroutine needs to be woken up by the response
coroutine if we were at maximum queue depth.

Stefan



Re: [Qemu-devel] active block commit bug?

2014-06-05 Thread Kevin Wolf
Am 05.06.2014 um 09:06 hat Markus Armbruster geschrieben:
 Eric Blake ebl...@redhat.com writes:
 
  On 06/04/2014 08:09 PM, Fam Zheng wrote:
 
  Sounds like we have an off-by-one condition if empty files behave
  differently from other files.  We ought to fix that bug (not that your
  normal guest will ever have a 0-length backing file, but this was what I
  was trying to use for libvirt's probing of whether active commit is
  supported)
 
  
  Yes, agreed, this special case is only going to make management confused. I
  will send a patch to fix this.
 
  Thanks.
 
  
  Eric, is this a good way to probe the active commit? I was expecting full
  instrospection of QMP could do it, but I don't know about the status of 
  that
  piece of work. Amos, any ideas?
 
  Introspection already missed qemu 2.0 when active commit was added; and
  we're close enough to soft freeze for 2.1 that I'm guessing it will miss
  2.1 as well :(
 
 Almost certainly.  It has non-trivial design issues.  To have a chance
 to make it into 2.x, it needs to be posted for review early in the 2.x
 cycle.

Why is nobody discussing these non-trivial design issues then? Patches
have been posted even in a 1.x development phase, so that's not what is
blocking us. If anything, it's the missing discussion.

Kevin



[Qemu-devel] [PATCH v4 0/5] This series adds iotest case for IO throttling.

2014-06-05 Thread Fam Zheng
There is a change in qemu-io sub-commands aio_read and aio_write, which
makes the aio requests accounted and the statistics reflected in blockstats.

Note that IO throttling implementation allows overcommiting of requests, so the
actual IO happened in a time unit may be a bit larger than given limits. In the
test case, the stats numbers are compared against 110%, to make room for such
flexibility in order to improve determinism.

v4: Rebase to master. Add Benoit's rev-by lines to all the patches.

Fam

Fam Zheng (5):
  qemu-io: Account IO by aio_read and aio_write
  qtest: Add scripts/qtest/qtest.py
  qemu-iotests: Add VM method qtest() to iotests.py
  qemu-iotests: Allow caller to disable underscore convertion for qmp
  qemu-iotests: Add 093 for IO throttling

 qemu-io-cmds.c|  10 +++
 scripts/qtest |   5 --
 scripts/qtest/qtest.py|  74 +++
 tests/qemu-iotests/093| 164 ++
 tests/qemu-iotests/093.out|   5 ++
 tests/qemu-iotests/iotests.py |  24 +--
 6 files changed, 273 insertions(+), 9 deletions(-)
 delete mode 100755 scripts/qtest
 create mode 100644 scripts/qtest/qtest.py
 create mode 100755 tests/qemu-iotests/093
 create mode 100644 tests/qemu-iotests/093.out

-- 
2.0.0




[Qemu-devel] [PATCH v4 1/5] qemu-io: Account IO by aio_read and aio_write

2014-06-05 Thread Fam Zheng
This will enable accounting of aio requests issued from qemu-io aio
read/write commands.

Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Benoit Canet ben...@irqsave.net
---
 qemu-io-cmds.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 60c1ceb..ae6c299 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1346,6 +1346,7 @@ out:
 }
 
 struct aio_ctx {
+BlockDriverState *bs;
 QEMUIOVector qiov;
 int64_t offset;
 char *buf;
@@ -1353,6 +1354,7 @@ struct aio_ctx {
 int vflag;
 int Cflag;
 int Pflag;
+BlockAcctCookie acct;
 int pattern;
 struct timeval t1;
 };
@@ -1370,6 +1372,8 @@ static void aio_write_done(void *opaque, int ret)
 goto out;
 }
 
+bdrv_acct_done(ctx-bs, ctx-acct);
+
 if (ctx-qflag) {
 goto out;
 }
@@ -1407,6 +1411,8 @@ static void aio_read_done(void *opaque, int ret)
 g_free(cmp_buf);
 }
 
+bdrv_acct_done(ctx-bs, ctx-acct);
+
 if (ctx-qflag) {
 goto out;
 }
@@ -1462,6 +1468,7 @@ static int aio_read_f(BlockDriverState *bs, int argc, 
char **argv)
 int nr_iov, c;
 struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
 
+ctx-bs = bs;
 while ((c = getopt(argc, argv, CP:qv)) != EOF) {
 switch (c) {
 case 'C':
@@ -1515,6 +1522,7 @@ static int aio_read_f(BlockDriverState *bs, int argc, 
char **argv)
 }
 
 gettimeofday(ctx-t1, NULL);
+bdrv_acct_start(bs, ctx-acct, ctx-qiov.size, BDRV_ACCT_READ);
 bdrv_aio_readv(bs, ctx-offset  9, ctx-qiov,
ctx-qiov.size  9, aio_read_done, ctx);
 return 0;
@@ -1558,6 +1566,7 @@ static int aio_write_f(BlockDriverState *bs, int argc, 
char **argv)
 int pattern = 0xcd;
 struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
 
+ctx-bs = bs;
 while ((c = getopt(argc, argv, CqP:)) != EOF) {
 switch (c) {
 case 'C':
@@ -1607,6 +1616,7 @@ static int aio_write_f(BlockDriverState *bs, int argc, 
char **argv)
 }
 
 gettimeofday(ctx-t1, NULL);
+bdrv_acct_start(bs, ctx-acct, ctx-qiov.size, BDRV_ACCT_WRITE);
 bdrv_aio_writev(bs, ctx-offset  9, ctx-qiov,
 ctx-qiov.size  9, aio_write_done, ctx);
 return 0;
-- 
2.0.0




[Qemu-devel] [PATCH v4 2/5] qtest: Add scripts/qtest/qtest.py

2014-06-05 Thread Fam Zheng
This removes the dummy scripts/qtest and adds scripts/qtest/qtest.py as
a python library for qtest protocol.

This is a skeleton with a basic cmd method to execute a command,
reading and parsing of qtest output will be added later on demand.

Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Benoit Canet ben...@irqsave.net
---
 scripts/qtest  |  5 
 scripts/qtest/qtest.py | 74 ++
 2 files changed, 74 insertions(+), 5 deletions(-)
 delete mode 100755 scripts/qtest
 create mode 100644 scripts/qtest/qtest.py

diff --git a/scripts/qtest b/scripts/qtest
deleted file mode 100755
index 4ef6c1c..000
--- a/scripts/qtest
+++ /dev/null
@@ -1,5 +0,0 @@
-#!/bin/sh
-
-export QTEST_QEMU_BINARY=$1
-shift
-$@
diff --git a/scripts/qtest/qtest.py b/scripts/qtest/qtest.py
new file mode 100644
index 000..16c6713
--- /dev/null
+++ b/scripts/qtest/qtest.py
@@ -0,0 +1,74 @@
+# QEMU qtest library
+#
+# Copyright (C) 2014 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng f...@redhat.com
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+# Based on qmp.py.
+#
+
+import errno
+import socket
+
+class QEMUQtestProtocol:
+def __init__(self, address, server=False):
+
+Create a QEMUQtestProtocol object.
+
+@param address: QEMU address, can be either a unix socket path (string)
+or a tuple in the form ( address, port ) for a TCP
+connection
+@param server: server mode listens on the socket (bool)
+@raise socket.error on socket connection errors
+@note No connection is established, this is done by the connect() or
+  accept() methods
+
+self.__address = address
+self.__sock = self.__get_sock()
+if server:
+self.__sock.bind(self.__address)
+self.__sock.listen(1)
+
+def __get_sock(self):
+if isinstance(self.__address, tuple):
+family = socket.AF_INET
+else:
+family = socket.AF_UNIX
+return socket.socket(family, socket.SOCK_STREAM)
+
+def connect(self):
+
+Connect to the qtest socket.
+
+@raise socket.error on socket connection errors
+
+self.__sock.connect(self.__address)
+self.__sockfile = self.__sock.makefile()
+
+def accept(self):
+
+Await connection from QEMU.
+
+@raise socket.error on socket connection errors
+
+self.__sock, _ = self.__sock.accept()
+self.__sockfile = self.__sock.makefile()
+
+def cmd(self, qtest_cmd):
+
+Send a qtest command on the wire.
+
+@param qtest_cmd: qtest command text to be sent
+
+self.__sock.sendall(qtest_cmd + \n)
+
+def close(self):
+self.__sockfile.close()
+self.__sock.close()
+
+def settimeout(self, timeout):
+self.__sock.settimeout(timeout)
-- 
2.0.0




[Qemu-devel] [PATCH v4 3/5] qemu-iotests: Add VM method qtest() to iotests.py

2014-06-05 Thread Fam Zheng
This will allow test case to run command in qtest protocol. It's
write-only for now.

Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Benoit Canet ben...@irqsave.net
---
 tests/qemu-iotests/iotests.py | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f6c437c..914b4d3 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -21,9 +21,13 @@ import re
 import subprocess
 import string
 import unittest
-import sys; sys.path.append(os.path.join(os.path.dirname(__file__), '..', 
'..', 'scripts', 'qmp'))
+import sys
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts', 
'qmp'))
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts', 
'qtest'))
 import qmp
+import qtest
 import struct
+import socket
 
 __all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', 'qemu_io',
'VM', 'QMPTestCase', 'notrun', 'main']
@@ -80,10 +84,12 @@ class VM(object):
 def __init__(self):
 self._monitor_path = os.path.join(test_dir, 'qemu-mon.%d' % 
os.getpid())
 self._qemu_log_path = os.path.join(test_dir, 'qemu-log.%d' % 
os.getpid())
+self._qtest_path = os.path.join(test_dir, 'qemu-qtest.%d' % 
os.getpid())
 self._args = qemu_args + ['-chardev',
  'socket,id=mon,path=' + self._monitor_path,
  '-mon', 'chardev=mon,mode=control',
- '-qtest', 'stdio', '-machine', 'accel=qtest',
+ '-qtest', 'unix:path=' + self._qtest_path,
+ '-machine', 'accel=qtest',
  '-display', 'none', '-vga', 'none']
 self._num_drives = 0
 
@@ -159,9 +165,11 @@ class VM(object):
 qemulog = open(self._qemu_log_path, 'wb')
 try:
 self._qmp = qmp.QEMUMonitorProtocol(self._monitor_path, 
server=True)
+self._qtest = qtest.QEMUQtestProtocol(self._qtest_path, 
server=True)
 self._popen = subprocess.Popen(self._args, stdin=devnull, 
stdout=qemulog,
stderr=subprocess.STDOUT)
 self._qmp.accept()
+self._qtest.accept()
 except:
 os.remove(self._monitor_path)
 raise
@@ -172,6 +180,7 @@ class VM(object):
 self._qmp.cmd('quit')
 self._popen.wait()
 os.remove(self._monitor_path)
+os.remove(self._qtest_path)
 os.remove(self._qemu_log_path)
 self._popen = None
 
@@ -184,6 +193,10 @@ class VM(object):
 
 return self._qmp.cmd(cmd, args=qmp_args)
 
+def qtest(self, cmd):
+'''Send a qtest command to guest'''
+return self._qtest.cmd(cmd)
+
 def get_qmp_event(self, wait=False):
 '''Poll for one queued QMP events and return it'''
 return self._qmp.pull_event(wait=wait)
-- 
2.0.0




Re: [Qemu-devel] [RFC] image-fuzzer: Trivial test runner

2014-06-05 Thread M.Kustova
On Wed, Jun 4, 2014 at 6:26 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Sat, May 31, 2014 at 01:56:46PM +0400, Maria Kustova wrote:

 Please add a --format qcow2 (or maybe --image-generator) option using
 __import__() to load the image generator module.  That way people can
 drop in new image generator modules in the future and we don't hard-code
 their names into the runner.

 This version of test runner executes only one test. In future it will be
 extended to execute multiple tests in a run.

 Signed-off-by: Maria Kustova mari...@catit.be
 ---
  tests/image-fuzzer/runner.py | 225 
 +++
  1 file changed, 225 insertions(+)
  create mode 100644 tests/image-fuzzer/runner.py

 diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
 new file mode 100644
 index 000..1dea8ef
 --- /dev/null
 +++ b/tests/image-fuzzer/runner.py
 @@ -0,0 +1,225 @@
 +# Tool for running fuzz tests
 +#
 +# Copyright (C) 2014 Maria Kustova mari...@catit.be
 +#
 +# This program is free software: you can redistribute it and/or modify
 +# it under the terms of the GNU General Public License as published by
 +# the Free Software Foundation, either version 3 of the License, or
 +# (at your option) any later version.
 +#
 +# This program is distributed in the hope that it will be useful,
 +# but WITHOUT ANY WARRANTY; without even the implied warranty of
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 +# GNU General Public License for more details.
 +#
 +# You should have received a copy of the GNU General Public License
 +# along with this program.  If not, see http://www.gnu.org/licenses/.
 +#
 +
 +import sys, os, signal
 +import qcow2
 +from time import gmtime, strftime
 +import subprocess
 +from shutil import rmtree
 +import getopt
 +# -For local test environment only
 +import resource
 +resource.setrlimit(resource.RLIMIT_CORE, (-1, -1))
 +# -

 Enabling core dumps is important.  I'm not sure why it says For local
 test environment only.

 This assumes that core dumps are written to the current working
 directory.  On Linux this is configurable and some distros default to a
 fancier setup where core dumps are not written to the current working
 directory.  For now, please add a note to the documentation explaining
 that core dumps should be configured to use the current working
 directory.

 +
 +def multilog(msg, *output):
 + Write an object to all of specified file descriptors
 +
 +
 +for fd in output:
 +fd.write(msg)
 +fd.flush()
 +
 +
 +def str_signal(sig):
 + Convert a numeric value of a system signal to the string one
 +defined by the current operational system
 +
 +
 +for k, v in signal.__dict__.items():
 +if v == sig:
 +return k
 +
 +
 +class TestEnv(object):
 + Trivial test object
 +
 +The class sets up test environment, generates a test image and executes
 +qemu_img with specified arguments and a test image provided. All logs
 +are collected.
 +Summary log will contain short descriptions and statuses of all tests in
 +a run.
 +Test log will include application ('qemu-img') logs besides info sent
 +to the summary log.
 +
 +
 +def __init__(self, work_dir, run_log, exec_bin=None, cleanup=True):
 +Set test environment in a specified work directory.
 +
 +Path to qemu_img will be retrieved from 'QEMU_IMG' environment
 +variable, if not specified.
 +
 +
 +self.init_path = os.getcwd()
 +self.work_dir = work_dir
 +self.current_dir = os.path.join(work_dir, 
 strftime(%Y_%m_%d_%H-%M-%S,
 +   gmtime()))
 +if exec_bin is not None:
 +self.exec_bin = exec_bin.strip().split(' ')
 +else:
 +self.exec_bin = os.environ.get('QEMU_IMG', 'qemu-img').strip()\
 +.split(' ')
 +
 +try:
 +os.makedirs(self.current_dir)
 +except OSError:
 +e = sys.exc_info()[1]
 +print sys.stderr, 'Error: The working directory cannot be 
 used.'\
 +' Reason: %s' %e[1]
 +raise Exception('Internal error')

 I guess this exception is really sys.exit(1).

 +
 +self.log = open(os.path.join(self.current_dir, test.log), w)
 +self.parent_log = open(run_log, a)
 +self.result = False
 +self.cleanup = cleanup
 +
 +def _qemu_img(self, q_args):
 + Start qemu_img with specified arguments and return an exit code 
 or
 +a kill signal depending on result of an execution.
 +
 +devnull = open('/dev/null', 'r+')
 +return subprocess.call(self.exec_bin \
 +   + q_args +
 +   ['test_image.qcow2'], stdin=devnull,
 +   stdout=self.log, stderr=self.log)
 +
 +
 +def execute(self, q_args, seed, size=8*512):

[Qemu-devel] [PATCH v4 5/5] qemu-iotests: Add 093 for IO throttling

2014-06-05 Thread Fam Zheng
This case utilizes qemu-io command aio_{read,write} -a -q to verify
the effectiveness of IO throttling options.

The -a option will cause qemu-io requests to be accounted.

It's implemented by driving the vm timer from qtest protocol, so the
throttling timers are signaled with determinied time duration. Then we
verify the completed IO requests are within 110% of bps and iops limits.

Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Benoit Canet ben...@irqsave.net
---
 tests/qemu-iotests/093 | 164 +
 tests/qemu-iotests/093.out |   5 ++
 2 files changed, 169 insertions(+)
 create mode 100755 tests/qemu-iotests/093
 create mode 100644 tests/qemu-iotests/093.out

diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
new file mode 100755
index 000..222bb37
--- /dev/null
+++ b/tests/qemu-iotests/093
@@ -0,0 +1,164 @@
+#!/usr/bin/env python
+#
+# Tests for IO throttling
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/.
+#
+
+import time
+import os
+import iotests
+from iotests import qemu_img
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+
+class ThrottleTestCase(iotests.QMPTestCase):
+image_len = 80 * 1024 * 1024 # MB
+
+def blockstats(self, device):
+result = self.vm.qmp(query-blockstats)
+for r in result['return']:
+if r['device'] == device:
+stat = r['stats']
+return stat['rd_bytes'], stat['rd_operations'], 
stat['wr_bytes'], stat['wr_operations']
+raise Exception(Device not found for blockstats: %s % device)
+
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, test_img, 1G)
+#self.vm = iotests.VM().add_drive(test_img, bps=1024,bps_max=1)
+self.vm = iotests.VM().add_drive(test_img)
+self.vm.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(test_img)
+
+def do_test_throttle(self, seconds=10, **limits):
+def check_limit(limit, num):
+# IO throttling algorithm is discrete, allow 10% error so the test
+# is more deterministic
+return limit == 0 or num  seconds * limit * 1.1
+
+nsec_per_sec = 10
+
+limits['bps_max'] = 1
+limits['iops_max'] = 1
+
+# Enqueue many requests to throttling queue
+result = self.vm.qmp(block_set_io_throttle, conv_keys=False, 
**limits)
+self.assert_qmp(result, 'return', {})
+
+# Set vm clock to a known value
+ns = nsec_per_sec
+self.vm.qtest_cmd(clock_step %d % ns)
+
+# Append many requests into the throttle queue
+# They drain bps_max and iops_max
+# The rest requests won't get executed until qtest clock is driven
+for i in range(1000):
+self.vm.hmp_qemu_io(drive0, aio_read -a -q 0 512)
+self.vm.hmp_qemu_io(drive0, aio_write -a -q 0 512)
+
+start_rd_bytes, start_rd_iops, start_wr_bytes, start_wr_iops = 
self.blockstats('drive0')
+
+ns += seconds * nsec_per_sec
+self.vm.qtest_cmd(clock_step %d % ns)
+# wait for a while to let requests take off
+time.sleep(1)
+end_rd_bytes, end_rd_iops, end_wr_bytes, end_wr_iops = 
self.blockstats('drive0')
+
+rd_bytes = end_rd_bytes - start_rd_bytes
+rd_iops = end_rd_iops - start_rd_iops
+wr_bytes = end_wr_bytes - start_wr_bytes
+wr_iops = end_wr_iops - start_wr_iops
+
+assert check_limit(limits['bps'], rd_bytes)
+assert check_limit(limits['bps_rd'], rd_bytes)
+assert check_limit(limits['bps'], wr_bytes)
+assert check_limit(limits['bps_wr'], wr_bytes)
+assert check_limit(limits['iops'], rd_iops)
+assert check_limit(limits['iops_rd'], rd_iops)
+assert check_limit(limits['iops'], wr_iops)
+assert check_limit(limits['iops_wr'], wr_iops)
+
+def test_bps(self):
+self.do_test_throttle(**{
+'device': 'drive0',
+'bps': 1000,
+'bps_rd': 0,
+'bps_wr': 0,
+'iops': 0,
+'iops_rd': 0,
+'iops_wr': 0,
+})
+
+def test_bps_rd(self):
+self.do_test_throttle(**{
+'device': 'drive0',
+'bps': 0,
+'bps_rd': 1000,
+'bps_wr': 0,
+

[Qemu-devel] [PATCH v4 4/5] qemu-iotests: Allow caller to disable underscore convertion for qmp

2014-06-05 Thread Fam Zheng
QMP command block_set_io_throttle expects underscores in parameters
instead of dashes: {iops,bps}_{rd,wr,max}.

Add optional argument conv_keys (defaults to True, backward compatible),
it will be used in IO throttling test case.

Reviewed-by: Benoit Canet ben...@irqsave.net
Signed-off-by: Fam Zheng f...@redhat.com
---
 tests/qemu-iotests/iotests.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 914b4d3..0b696fb 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -185,11 +185,14 @@ class VM(object):
 self._popen = None
 
 underscore_to_dash = string.maketrans('_', '-')
-def qmp(self, cmd, **args):
+def qmp(self, cmd, conv_keys=True, **args):
 '''Invoke a QMP command and return the result dict'''
 qmp_args = dict()
 for k in args.keys():
-qmp_args[k.translate(self.underscore_to_dash)] = args[k]
+if conv_keys:
+qmp_args[k.translate(self.underscore_to_dash)] = args[k]
+else:
+qmp_args[k] = args[k]
 
 return self._qmp.cmd(cmd, args=qmp_args)
 
-- 
2.0.0




Re: [Qemu-devel] [PATCH] block/iscsi: use 16 byte CDBs only when necessary

2014-06-05 Thread Michael Tokarev
04.06.2014 18:00, ronnie sahlberg wrote:
 That would mean you get to use the 10 version of the cdb even for very
 large devices (as long as the IO is for blocks at the beginning of the
 device) and thus provide partial avoidance of this issue for those
 large devices.

That may make some bugs ghosty, so to say.  Ie, if there's a bug in/with
16 version of a command, you'll hit it only when you actually try to access
a far area of a drive.  Which means you're unlikely to hit it while trying
to reproduce in a clean environment, even after using a large device.  Or,
the bug will be triggered at random, since data placement on the filesystem
is effectively (from user PoV) random.

To my taste it is better to make it a bit more deterministic.

Thanks,

/mjt



Re: [Qemu-devel] [PATCH v2 8/8] virtio-blk: Fill in VirtIOBlockReq.out in dataplane code

2014-06-05 Thread Fam Zheng
On Thu, 06/05 06:09, Paolo Bonzini wrote:
 Il 05/06/2014 05:50, Fam Zheng ha scritto:
  Can you try moving the req allocation and assignments inside 
  process_request
  instead?  Then you can fill in req-out directly without the struct
  assignment.
 
 The owners of req are do_rdwr_cmd and do_flush_cmd, but do_scsi_cmd and
 do_get_id_cmd don't need to allocate.
 
 They don't need it, but using req there and freeing it in
 complete_request_early perhaps could simplify the code.
 
 After all, the first three arguments of complete_request_early (s, elem,
 inhdr) are a duplicate of VirtIOBlockReq and do_flush_cmd is already doing a
 free after complete_request_early.
 

Yes. Although It's not really early complete from do_flush_cmd, it's actually
a cb complete.

But the point makes sense for me. I'll do it.

Fam



[Qemu-devel] [PATCH v28 06/33] QemuOpts: move qemu_opt_del ahead for later calling

2014-06-05 Thread Chunyan Liu
In later patch, qemu_opt_get_del functions will be added, they will
first get the option value, then call qemu_opt_del to remove the option
from opt list. To prepare for that purpose, move qemu_opt_del ahead first.

Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Leandro Dorileo l...@dorileo.org
Signed-off-by: Chunyan Liu cy...@suse.com
---
 util/qemu-option.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index d4fd7b5..7124483 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -567,6 +567,14 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char 
*name)
 return NULL;
 }
 
+static void qemu_opt_del(QemuOpt *opt)
+{
+QTAILQ_REMOVE(opt-opts-head, opt, next);
+g_free(opt-name);
+g_free(opt-str);
+g_free(opt);
+}
+
 const char *qemu_opt_get(QemuOpts *opts, const char *name)
 {
 QemuOpt *opt = qemu_opt_find(opts, name);
@@ -661,14 +669,6 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp)
 }
 }
 
-static void qemu_opt_del(QemuOpt *opt)
-{
-QTAILQ_REMOVE(opt-opts-head, opt, next);
-g_free(opt-name);
-g_free(opt-str);
-g_free(opt);
-}
-
 static bool opts_accepts_any(const QemuOpts *opts)
 {
 return opts-list-desc[0].name == NULL;
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 05/33] QemuOpts: change opt-name|str from (const char *) to (char *)

2014-06-05 Thread Chunyan Liu
qemu_opt_del() already assumes that all QemuOpt instances contain
malloc'd name and value; but it had to cast away const because
opts_start_struct() was doing its own thing and using static storage
instead.  By using the correct type and malloced strings everywhere, the
usage of this struct becomes clearer.

Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Leandro Dorileo l...@dorileo.org
Signed-off-by: Chunyan Liu cy...@suse.com
---
 include/qemu/option_int.h |  4 ++--
 qapi/opts-visitor.c   | 10 +++---
 util/qemu-option.c|  4 ++--
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h
index 8212fa4..6432c1a 100644
--- a/include/qemu/option_int.h
+++ b/include/qemu/option_int.h
@@ -30,8 +30,8 @@
 #include qemu/error-report.h
 
 struct QemuOpt {
-const char   *name;
-const char   *str;
+char *name;
+char *str;
 
 const QemuOptDesc *desc;
 union {
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 16382e7..f2ad6d7 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -143,8 +143,8 @@ opts_start_struct(Visitor *v, void **obj, const char *kind,
 if (ov-opts_root-id != NULL) {
 ov-fake_id_opt = g_malloc0(sizeof *ov-fake_id_opt);
 
-ov-fake_id_opt-name = id;
-ov-fake_id_opt-str = ov-opts_root-id;
+ov-fake_id_opt-name = g_strdup(id);
+ov-fake_id_opt-str = g_strdup(ov-opts_root-id);
 opts_visitor_insert(ov-unprocessed_opts, ov-fake_id_opt);
 }
 }
@@ -177,7 +177,11 @@ opts_end_struct(Visitor *v, Error **errp)
 }
 g_hash_table_destroy(ov-unprocessed_opts);
 ov-unprocessed_opts = NULL;
-g_free(ov-fake_id_opt);
+if (ov-fake_id_opt) {
+g_free(ov-fake_id_opt-name);
+g_free(ov-fake_id_opt-str);
+g_free(ov-fake_id_opt);
+}
 ov-fake_id_opt = NULL;
 }
 
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 5af39a2..d4fd7b5 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -664,8 +664,8 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp)
 static void qemu_opt_del(QemuOpt *opt)
 {
 QTAILQ_REMOVE(opt-opts-head, opt, next);
-g_free((/* !const */ char*)opt-name);
-g_free((/* !const */ char*)opt-str);
+g_free(opt-name);
+g_free(opt-str);
 g_free(opt);
 }
 
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 03/33] QemuOpts: add def_value_str to QemuOptDesc

2014-06-05 Thread Chunyan Liu
Add def_value_str (default value) to QemuOptDesc, to replace function of the
default value in QEMUOptionParameter.

Improve qemu_opts_get_* functions: if find opt, return opt-str; otherwise,
if desc-def_value_str is set, return desc-def_value_str; otherwise, return
input defval.

Improve qemu_opts_print: if option is set, print opt-str; otherwise, if
desc-def_value_str is set, print it.

Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 include/qemu/option.h |  1 +
 util/qemu-option.c| 56 ---
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 1077b69..c3b0a91 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -99,6 +99,7 @@ typedef struct QemuOptDesc {
 const char *name;
 enum QemuOptType type;
 const char *help;
+const char *def_value_str;
 } QemuOptDesc;
 
 struct QemuOptsList {
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 2c46791..5af39a2 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -570,6 +570,13 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char 
*name)
 const char *qemu_opt_get(QemuOpts *opts, const char *name)
 {
 QemuOpt *opt = qemu_opt_find(opts, name);
+
+if (!opt) {
+const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
+if (desc  desc-def_value_str) {
+return desc-def_value_str;
+}
+}
 return opt ? opt-str : NULL;
 }
 
@@ -589,8 +596,13 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, 
bool defval)
 {
 QemuOpt *opt = qemu_opt_find(opts, name);
 
-if (opt == NULL)
+if (opt == NULL) {
+const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
+if (desc  desc-def_value_str) {
+parse_option_bool(name, desc-def_value_str, defval, 
error_abort);
+}
 return defval;
+}
 assert(opt-desc  opt-desc-type == QEMU_OPT_BOOL);
 return opt-value.boolean;
 }
@@ -599,8 +611,14 @@ uint64_t qemu_opt_get_number(QemuOpts *opts, const char 
*name, uint64_t defval)
 {
 QemuOpt *opt = qemu_opt_find(opts, name);
 
-if (opt == NULL)
+if (opt == NULL) {
+const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
+if (desc  desc-def_value_str) {
+parse_option_number(name, desc-def_value_str, defval,
+error_abort);
+}
 return defval;
+}
 assert(opt-desc  opt-desc-type == QEMU_OPT_NUMBER);
 return opt-value.uint;
 }
@@ -609,8 +627,13 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char 
*name, uint64_t defval)
 {
 QemuOpt *opt = qemu_opt_find(opts, name);
 
-if (opt == NULL)
+if (opt == NULL) {
+const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
+if (desc  desc-def_value_str) {
+parse_option_size(name, desc-def_value_str, defval, 
error_abort);
+}
 return defval;
+}
 assert(opt-desc  opt-desc-type == QEMU_OPT_SIZE);
 return opt-value.uint;
 }
@@ -898,11 +921,30 @@ void qemu_opts_del(QemuOpts *opts)
 void qemu_opts_print(QemuOpts *opts)
 {
 QemuOpt *opt;
+QemuOptDesc *desc = opts-list-desc;
 
-printf(%s: %s:, opts-list-name,
-   opts-id ? opts-id : noid);
-QTAILQ_FOREACH(opt, opts-head, next) {
-printf( %s=\%s\, opt-name, opt-str);
+if (desc[0].name == NULL) {
+QTAILQ_FOREACH(opt, opts-head, next) {
+printf(%s=\%s\ , opt-name, opt-str);
+}
+return;
+}
+for (; desc  desc-name; desc++) {
+const char *value;
+QemuOpt *opt = qemu_opt_find(opts, desc-name);
+
+value = opt ? opt-str : desc-def_value_str;
+if (!value) {
+continue;
+}
+if (desc-type == QEMU_OPT_STRING) {
+printf(%s='%s' , desc-name, value);
+} else if ((desc-type == QEMU_OPT_SIZE ||
+desc-type == QEMU_OPT_NUMBER)  opt) {
+printf(%s=% PRId64  , desc-name, opt-value.uint);
+} else {
+printf(%s=%s , desc-name, value);
+}
 }
 }
 
-- 
1.7.12.4




Re: [Qemu-devel] active block commit bug?

2014-06-05 Thread Markus Armbruster
Kevin Wolf kw...@redhat.com writes:

 Am 05.06.2014 um 09:06 hat Markus Armbruster geschrieben:
 Eric Blake ebl...@redhat.com writes:
 
  On 06/04/2014 08:09 PM, Fam Zheng wrote:
 
  Sounds like we have an off-by-one condition if empty files behave
  differently from other files.  We ought to fix that bug (not that your
  normal guest will ever have a 0-length backing file, but this was what I
  was trying to use for libvirt's probing of whether active commit is
  supported)
 
  
  Yes, agreed, this special case is only going to make management confused. 
  I
  will send a patch to fix this.
 
  Thanks.
 
  
  Eric, is this a good way to probe the active commit? I was expecting full
  instrospection of QMP could do it, but I don't know about the
  status of that
  piece of work. Amos, any ideas?
 
  Introspection already missed qemu 2.0 when active commit was added; and
  we're close enough to soft freeze for 2.1 that I'm guessing it will miss
  2.1 as well :(
 
 Almost certainly.  It has non-trivial design issues.  To have a chance
 to make it into 2.x, it needs to be posted for review early in the 2.x
 cycle.

 Why is nobody discussing these non-trivial design issues then? Patches
 have been posted even in a 1.x development phase, so that's not what is
 blocking us. If anything, it's the missing discussion.

If I remember correctly, review of the patches made us realize
introspection is more complex than initially thought.  We discussed a
proper solution at some length, but concluded implementing it in time
for 2.0 was not in the cards (we were running close to the hard freeze
already).  Unfortunately, nobody has stepped up to implement it in time
for 2.1 either.

To make further progress, we need a sucker^Wvolunteer pushing the
feature.



[Qemu-devel] [PATCH v28 16/33] iscsi.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/iscsi.c | 34 --
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 3892cc5..76d4623 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1500,8 +1500,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t 
offset)
 return 0;
 }
 
-static int iscsi_create(const char *filename, QEMUOptionParameter *options,
-Error **errp)
+static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int ret = 0;
 int64_t total_size = 0;
@@ -1512,13 +1511,8 @@ static int iscsi_create(const char *filename, 
QEMUOptionParameter *options,
 bs = bdrv_new(, error_abort);
 
 /* Read out options */
-while (options  options-name) {
-if (!strcmp(options-name, size)) {
-total_size = options-value.n / BDRV_SECTOR_SIZE;
-}
-options++;
-}
-
+total_size =
+qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
 bs-opaque = g_malloc0(sizeof(struct IscsiLun));
 iscsilun = bs-opaque;
 
@@ -1563,13 +1557,17 @@ static int iscsi_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 return 0;
 }
 
-static QEMUOptionParameter iscsi_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{ NULL }
+static QemuOptsList iscsi_create_opts = {
+.name = iscsi-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(iscsi_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{ /* end of list */ }
+}
 };
 
 static BlockDriver bdrv_iscsi = {
@@ -1580,8 +1578,8 @@ static BlockDriver bdrv_iscsi = {
 .bdrv_needs_filename = true,
 .bdrv_file_open  = iscsi_open,
 .bdrv_close  = iscsi_close,
-.bdrv_create = iscsi_create,
-.create_options  = iscsi_create_options,
+.bdrv_create2= iscsi_create,
+.create_opts = iscsi_create_opts,
 .bdrv_reopen_prepare  = iscsi_reopen_prepare,
 
 .bdrv_getlength  = iscsi_getlength,
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 04/33] qapi: output def_value_str when query command line options

2014-06-05 Thread Chunyan Liu
Change qapi interfaces to output the newly added def_value_str when querying
command line options.

Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Leandro Dorileo l...@dorileo.org
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 qapi-schema.json   | 5 -
 qmp-commands.hx| 2 ++
 util/qemu-config.c | 4 
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 7bc33ea..ee4d8f4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4096,12 +4096,15 @@
 #
 # @help: #optional human readable text string, not suitable for parsing.
 #
+# @default: #optional default value string (since 2.1)
+#
 # Since 1.5
 ##
 { 'type': 'CommandLineParameterInfo',
   'data': { 'name': 'str',
 'type': 'CommandLineParameterType',
-'*help': 'str' } }
+'*help': 'str',
+'*default': 'str' } }
 
 ##
 # @CommandLineOptionInfo:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d8aa4ed..d6bb0f4 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2898,6 +2898,8 @@ Each array entry contains the following:
   or 'size')
 - help: human readable description of the parameter
   (json-string, optional)
+- default: default value string for the parameter
+ (json-string, optional)
 
 Example:
 
diff --git a/util/qemu-config.c b/util/qemu-config.c
index f4e4f38..ba375c0 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -82,6 +82,10 @@ static CommandLineParameterInfoList 
*query_option_descs(const QemuOptDesc *desc)
 info-has_help = true;
 info-help = g_strdup(desc[i].help);
 }
+if (desc[i].def_value_str) {
+info-has_q_default = true;
+info-q_default = g_strdup(desc[i].def_value_str);
+}
 
 entry = g_malloc0(sizeof(*entry));
 entry-value = info;
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 07/33] QemuOpts: add qemu_opt_get_*_del functions for replace work

2014-06-05 Thread Chunyan Liu
Add qemu_opt_get_del, qemu_opt_get_bool_del, qemu_opt_get_number_del and
qemu_opt_get_size_del to replace the same handling of QEMUOptionParameter
(get and delete).

Several drivers are coded to parse a known subset of options, then
remove them from the list before handing all remaining options to a
second driver for further option processing.  get_*_del makes it easier
to retrieve a known option (or its default) and remove it from the list
all in one action.

Share common helper function:

For qemu_opt_get_bool/size/number, they and their get_*_del counterpart
could share most of the code except whether or not deleting the opt from
option list, so generate common helper functions.

For qemu_opt_get and qemu_opt_get_del, keep code duplication, since
1. qemu_opt_get_del returns malloc'd memory while qemu_opt_get returns
in-place memory
2. qemu_opt_get_del returns (char *), qemu_opt_get returns (const char *),
and could not change to (char *), since in one case, it will return
desc-def_value_str, which is (const char *).

Reviewed-by: Eric Blake ebl...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 include/qemu/option.h |   6 +++
 util/qemu-option.c| 117 --
 2 files changed, 110 insertions(+), 13 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index c3b0a91..6653e43 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -111,6 +111,7 @@ struct QemuOptsList {
 };
 
 const char *qemu_opt_get(QemuOpts *opts, const char *name);
+char *qemu_opt_get_del(QemuOpts *opts, const char *name);
 /**
  * qemu_opt_has_help_opt:
  * @opts: options to search for a help request
@@ -126,6 +127,11 @@ bool qemu_opt_has_help_opt(QemuOpts *opts);
 bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval);
 uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t 
defval);
 uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
+bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval);
+uint64_t qemu_opt_get_number_del(QemuOpts *opts, const char *name,
+ uint64_t defval);
+uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name,
+   uint64_t defval);
 int qemu_opt_unset(QemuOpts *opts, const char *name);
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
 void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 7124483..a7330c6 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -575,6 +575,20 @@ static void qemu_opt_del(QemuOpt *opt)
 g_free(opt);
 }
 
+/* qemu_opt_set allows many settings for the same option.
+ * This function deletes all settings for an option.
+ */
+static void qemu_opt_del_all(QemuOpts *opts, const char *name)
+{
+QemuOpt *opt, *next_opt;
+
+QTAILQ_FOREACH_SAFE(opt, opts-head, next, next_opt) {
+if (!strcmp(opt-name, name)) {
+qemu_opt_del(opt);
+}
+}
+}
+
 const char *qemu_opt_get(QemuOpts *opts, const char *name)
 {
 QemuOpt *opt = qemu_opt_find(opts, name);
@@ -588,6 +602,34 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
 return opt ? opt-str : NULL;
 }
 
+/* Get a known option (or its default) and remove it from the list
+ * all in one action. Return a malloced string of the option value.
+ * Result must be freed by caller with g_free().
+ */
+char *qemu_opt_get_del(QemuOpts *opts, const char *name)
+{
+QemuOpt *opt;
+const QemuOptDesc *desc;
+char *str = NULL;
+
+if (opts == NULL) {
+return NULL;
+}
+
+opt = qemu_opt_find(opts, name);
+if (!opt) {
+desc = find_desc_by_name(opts-list-desc, name);
+if (desc  desc-def_value_str) {
+str = g_strdup(desc-def_value_str);
+}
+return str;
+}
+str = opt-str;
+opt-str = NULL;
+qemu_opt_del_all(opts, name);
+return str;
+}
+
 bool qemu_opt_has_help_opt(QemuOpts *opts)
 {
 QemuOpt *opt;
@@ -600,50 +642,99 @@ bool qemu_opt_has_help_opt(QemuOpts *opts)
 return false;
 }
 
-bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval)
+static bool qemu_opt_get_bool_helper(QemuOpts *opts, const char *name,
+ bool defval, bool del)
 {
 QemuOpt *opt = qemu_opt_find(opts, name);
+bool ret = defval;
 
 if (opt == NULL) {
 const QemuOptDesc *desc = find_desc_by_name(opts-list-desc, name);
 if (desc  desc-def_value_str) {
-parse_option_bool(name, desc-def_value_str, defval, 
error_abort);
+parse_option_bool(name, desc-def_value_str, ret, error_abort);
 }
-return defval;
+return ret;
 }
 assert(opt-desc  opt-desc-type == QEMU_OPT_BOOL);
-return opt-value.boolean;
+ret = 

[Qemu-devel] [PATCH v28 01/33] QemuOpts: move find_desc_by_name ahead for later calling

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 util/qemu-option.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 324e4c5..c188c5c 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -173,6 +173,20 @@ static void parse_option_number(const char *name, const 
char *value,
 }
 }
 
+static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
+const char *name)
+{
+int i;
+
+for (i = 0; desc[i].name != NULL; i++) {
+if (strcmp(desc[i].name, name) == 0) {
+return desc[i];
+}
+}
+
+return NULL;
+}
+
 void parse_option_size(const char *name, const char *value,
uint64_t *ret, Error **errp)
 {
@@ -637,20 +651,6 @@ static bool opts_accepts_any(const QemuOpts *opts)
 return opts-list-desc[0].name == NULL;
 }
 
-static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
-const char *name)
-{
-int i;
-
-for (i = 0; desc[i].name != NULL; i++) {
-if (strcmp(desc[i].name, name) == 0) {
-return desc[i];
-}
-}
-
-return NULL;
-}
-
 int qemu_opt_unset(QemuOpts *opts, const char *name)
 {
 QemuOpt *opt = qemu_opt_find(opts, name);
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 18/33] qcow.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/qcow.c | 72 ++--
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 32651eb..bcacfb8 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -693,35 +693,29 @@ static void qcow_close(BlockDriverState *bs)
 error_free(s-migration_blocker);
 }
 
-static int qcow_create(const char *filename, QEMUOptionParameter *options,
-   Error **errp)
+static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int header_size, backing_filename_len, l1_size, shift, i;
 QCowHeader header;
 uint8_t *tmp;
 int64_t total_size = 0;
-const char *backing_file = NULL;
+char *backing_file = NULL;
 int flags = 0;
 Error *local_err = NULL;
 int ret;
 BlockDriverState *qcow_bs;
 
 /* Read out options */
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-total_size = options-value.n / 512;
-} else if (!strcmp(options-name, BLOCK_OPT_BACKING_FILE)) {
-backing_file = options-value.s;
-} else if (!strcmp(options-name, BLOCK_OPT_ENCRYPT)) {
-flags |= options-value.n ? BLOCK_FLAG_ENCRYPT : 0;
-}
-options++;
+total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
+if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
+flags |= BLOCK_FLAG_ENCRYPT;
 }
 
-ret = bdrv_create_file(filename, options, NULL, local_err);
+ret = bdrv_create_file(filename, NULL, opts, local_err);
 if (ret  0) {
 error_propagate(errp, local_err);
-return ret;
+goto cleanup;
 }
 
 qcow_bs = NULL;
@@ -729,7 +723,7 @@ static int qcow_create(const char *filename, 
QEMUOptionParameter *options,
 BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, local_err);
 if (ret  0) {
 error_propagate(errp, local_err);
-return ret;
+goto cleanup;
 }
 
 ret = bdrv_truncate(qcow_bs, 0);
@@ -800,6 +794,8 @@ static int qcow_create(const char *filename, 
QEMUOptionParameter *options,
 ret = 0;
 exit:
 bdrv_unref(qcow_bs);
+cleanup:
+g_free(backing_file);
 return ret;
 }
 
@@ -912,24 +908,28 @@ static int qcow_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 return 0;
 }
 
-
-static QEMUOptionParameter qcow_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{
-.name = BLOCK_OPT_BACKING_FILE,
-.type = OPT_STRING,
-.help = File name of a base image
-},
-{
-.name = BLOCK_OPT_ENCRYPT,
-.type = OPT_FLAG,
-.help = Encrypt the image
-},
-{ NULL }
+static QemuOptsList qcow_create_opts = {
+.name = qcow-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(qcow_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{
+.name = BLOCK_OPT_BACKING_FILE,
+.type = QEMU_OPT_STRING,
+.help = File name of a base image
+},
+{
+.name = BLOCK_OPT_ENCRYPT,
+.type = QEMU_OPT_BOOL,
+.help = Encrypt the image,
+.def_value_str = off
+},
+{ /* end of list */ }
+}
 };
 
 static BlockDriver bdrv_qcow = {
@@ -938,8 +938,8 @@ static BlockDriver bdrv_qcow = {
 .bdrv_probe= qcow_probe,
 .bdrv_open = qcow_open,
 .bdrv_close= qcow_close,
-.bdrv_reopen_prepare = qcow_reopen_prepare,
-.bdrv_create   = qcow_create,
+.bdrv_reopen_prepare= qcow_reopen_prepare,
+.bdrv_create2   = qcow_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 
 .bdrv_co_readv  = qcow_co_readv,
@@ -951,7 +951,7 @@ static BlockDriver bdrv_qcow = {
 .bdrv_write_compressed  = qcow_write_compressed,
 .bdrv_get_info  = qcow_get_info,
 
-.create_options = qcow_create_options,
+.create_opts= qcow_create_opts,
 };
 
 static void bdrv_qcow_init(void)
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 09/33] QemuOpts: add conversion between QEMUOptionParameter to QemuOpts

2014-06-05 Thread Chunyan Liu
Add two temp conversion functions between QEMUOptionParameter to QemuOpts,
so that next patch can use it. It will simplify later patch for easier
review. And will be finally removed after all backend drivers switch to
QemuOpts.

Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Leandro Dorileo l...@dorileo.org
Signed-off-by: Chunyan Liu cy...@suse.com
---
 include/qemu/option.h |   9 +++
 util/qemu-option.c| 153 ++
 2 files changed, 162 insertions(+)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index fbf5dc2..e98e8ef 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -103,6 +103,12 @@ typedef struct QemuOptDesc {
 } QemuOptDesc;
 
 struct QemuOptsList {
+/* FIXME: Temp used for QEMUOptionParamter-QemuOpts conversion to
+ * indicate the need to free memory. Will remove after all drivers
+ * switch to QemuOpts.
+ */
+bool allocated;
+
 const char *name;
 const char *implied_opt_name;
 bool merge_lists;  /* Merge multiple uses of option into a single list? */
@@ -167,5 +173,8 @@ void qemu_opts_print(QemuOpts *opts);
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void 
*opaque,
   int abort_on_failure);
 void qemu_opts_print_help(QemuOptsList *list);
+void qemu_opts_free(QemuOptsList *list);
+QEMUOptionParameter *opts_to_params(QemuOpts *opts);
+QemuOptsList *params_to_opts(QEMUOptionParameter *list);
 
 #endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index cd03eb4..3c5484b 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1346,3 +1346,156 @@ int qemu_opts_foreach(QemuOptsList *list, 
qemu_opts_loopfunc func, void *opaque,
 loc_pop(loc);
 return rc;
 }
+
+static size_t count_opts_list(QemuOptsList *list)
+{
+QemuOptDesc *desc = NULL;
+size_t num_opts = 0;
+
+if (!list) {
+return 0;
+}
+
+desc = list-desc;
+while (desc  desc-name) {
+num_opts++;
+desc++;
+}
+
+return num_opts;
+}
+
+/* Convert QEMUOptionParameter to QemuOpts
+ * FIXME: this function will be removed after all drivers
+ * switch to QemuOpts
+ */
+QemuOptsList *params_to_opts(QEMUOptionParameter *list)
+{
+QemuOptsList *opts = NULL;
+size_t num_opts, i = 0;
+
+if (!list) {
+return NULL;
+}
+
+num_opts = count_option_parameters(list);
+opts = g_malloc0(sizeof(QemuOptsList) +
+ (num_opts + 1) * sizeof(QemuOptDesc));
+QTAILQ_INIT(opts-head);
+/* (const char *) members will point to malloced space and need to free */
+opts-allocated = true;
+
+while (list  list-name) {
+opts-desc[i].name = g_strdup(list-name);
+opts-desc[i].help = g_strdup(list-help);
+switch (list-type) {
+case OPT_FLAG:
+opts-desc[i].type = QEMU_OPT_BOOL;
+opts-desc[i].def_value_str =
+g_strdup(list-value.n ? on : off);
+break;
+
+case OPT_NUMBER:
+opts-desc[i].type = QEMU_OPT_NUMBER;
+if (list-value.n) {
+opts-desc[i].def_value_str =
+g_strdup_printf(% PRIu64, list-value.n);
+}
+break;
+
+case OPT_SIZE:
+opts-desc[i].type = QEMU_OPT_SIZE;
+if (list-value.n) {
+opts-desc[i].def_value_str =
+g_strdup_printf(% PRIu64, list-value.n);
+}
+break;
+
+case OPT_STRING:
+opts-desc[i].type = QEMU_OPT_STRING;
+opts-desc[i].def_value_str = g_strdup(list-value.s);
+break;
+}
+
+i++;
+list++;
+}
+
+return opts;
+}
+
+/* convert QemuOpts to QEMUOptionParameter
+ * Note: result QEMUOptionParameter has shorter lifetime than
+ * input QemuOpts.
+ * FIXME: this function will be removed after all drivers
+ * switch to QemuOpts
+ */
+QEMUOptionParameter *opts_to_params(QemuOpts *opts)
+{
+QEMUOptionParameter *dest = NULL;
+QemuOptDesc *desc;
+size_t num_opts, i = 0;
+const char *tmp;
+
+if (!opts || !opts-list || !opts-list-desc) {
+return NULL;
+}
+assert(!opts_accepts_any(opts));
+
+num_opts = count_opts_list(opts-list);
+dest = g_malloc0((num_opts + 1) * sizeof(QEMUOptionParameter));
+
+desc = opts-list-desc;
+while (desc  desc-name) {
+dest[i].name = desc-name;
+dest[i].help = desc-help;
+dest[i].assigned = qemu_opt_find(opts, desc-name) ? true : false;
+switch (desc-type) {
+case QEMU_OPT_STRING:
+dest[i].type = OPT_STRING;
+tmp = qemu_opt_get(opts, desc-name);
+dest[i].value.s = g_strdup(tmp);
+break;
+
+case QEMU_OPT_BOOL:
+dest[i].type = OPT_FLAG;
+dest[i].value.n = qemu_opt_get_bool(opts, desc-name, 0) ? 1 : 0;
+break;

[Qemu-devel] [PATCH v28 10/33] QemuOpts: add qemu_opts_append to replace append_option_parameters

2014-06-05 Thread Chunyan Liu
For later merge .create_opts of drv and proto_drv in qemu-img commands.

Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Leandro Dorileo l...@dorileo.org
Signed-off-by: Chunyan Liu cy...@suse.com
---
 include/qemu/option.h |  5 
 util/qemu-option.c| 67 +++
 2 files changed, 72 insertions(+)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index e98e8ef..44d9961 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -176,5 +176,10 @@ void qemu_opts_print_help(QemuOptsList *list);
 void qemu_opts_free(QemuOptsList *list);
 QEMUOptionParameter *opts_to_params(QemuOpts *opts);
 QemuOptsList *params_to_opts(QEMUOptionParameter *list);
+/* FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
+ */
+QemuOptsList *qemu_opts_append(QemuOptsList *dst,
+   QemuOptsList *list,
+   QEMUOptionParameter *param);
 
 #endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 3c5484b..6acfb0e 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1499,3 +1499,70 @@ void qemu_opts_free(QemuOptsList *list)
 
 g_free(list);
 }
+
+/* Realloc dst option list and append options either from an option list (list)
+ * or a QEMUOptionParameter (param) to it. dst could be NULL or a malloced 
list.
+ * FIXME: will remove QEMUOptionParameter after all drivers switch to QemuOpts.
+ */
+QemuOptsList *qemu_opts_append(QemuOptsList *dst,
+   QemuOptsList *list,
+   QEMUOptionParameter *param)
+{
+size_t num_opts, num_dst_opts;
+QemuOptDesc *desc;
+bool need_init = false;
+
+assert(!(list  param));
+if (!param  !list) {
+return dst;
+}
+
+if (param) {
+list = params_to_opts(param);
+}
+
+/* If dst is NULL, after realloc, some area of dst should be initialized
+ * before adding options to it.
+ */
+if (!dst) {
+need_init = true;
+}
+
+num_opts = count_opts_list(dst);
+num_dst_opts = num_opts;
+num_opts += count_opts_list(list);
+dst = g_realloc(dst, sizeof(QemuOptsList) +
+(num_opts + 1) * sizeof(QemuOptDesc));
+if (need_init) {
+dst-name = NULL;
+dst-implied_opt_name = NULL;
+QTAILQ_INIT(dst-head);
+dst-allocated = true;
+dst-merge_lists = false;
+}
+dst-desc[num_dst_opts].name = NULL;
+
+/* (const char *) members of result dst are malloced, need free. */
+assert(dst-allocated);
+/* append list-desc to dst-desc */
+if (list) {
+desc = list-desc;
+while (desc  desc-name) {
+if (find_desc_by_name(dst-desc, desc-name) == NULL) {
+dst-desc[num_dst_opts].name = g_strdup(desc-name);
+dst-desc[num_dst_opts].type = desc-type;
+dst-desc[num_dst_opts].help = g_strdup(desc-help);
+dst-desc[num_dst_opts].def_value_str =
+ g_strdup(desc-def_value_str);
+num_dst_opts++;
+dst-desc[num_dst_opts].name = NULL;
+}
+desc++;
+}
+}
+
+if (param) {
+qemu_opts_free(list);
+}
+return dst;
+}
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 11/33] QemuOpts: check NULL input for qemu_opts_del

2014-06-05 Thread Chunyan Liu
To simplify later using of qemu_opts_del, accept NULL input.

Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Leandro Dorileo l...@dorileo.org
Signed-off-by: Chunyan Liu cy...@suse.com
---
 util/qemu-option.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 6acfb0e..40e1ff3 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1011,6 +1011,10 @@ void qemu_opts_del(QemuOpts *opts)
 {
 QemuOpt *opt;
 
+if (opts == NULL) {
+return;
+}
+
 for (;;) {
 opt = QTAILQ_FIRST(opts-head);
 if (opt == NULL)
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 17/33] nfs.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/nfs.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index 539bd95..20a90e2 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -361,20 +361,14 @@ static int nfs_file_open(BlockDriverState *bs, QDict 
*options, int flags,
 return 0;
 }
 
-static int nfs_file_create(const char *url, QEMUOptionParameter *options,
-   Error **errp)
+static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
 {
 int ret = 0;
 int64_t total_size = 0;
 NFSClient *client = g_malloc0(sizeof(NFSClient));
 
 /* Read out options */
-while (options  options-name) {
-if (!strcmp(options-name, size)) {
-total_size = options-value.n;
-}
-options++;
-}
+total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
 
 ret = nfs_client_open(client, url, O_CREAT, errp);
 if (ret  0) {
@@ -431,7 +425,7 @@ static BlockDriver bdrv_nfs = {
 
 .bdrv_file_open  = nfs_file_open,
 .bdrv_close  = nfs_file_close,
-.bdrv_create = nfs_file_create,
+.bdrv_create2= nfs_file_create,
 
 .bdrv_co_readv = nfs_co_readv,
 .bdrv_co_writev= nfs_co_writev,
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 15/33] gluster.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/gluster.c | 81 ++---
 1 file changed, 42 insertions(+), 39 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index d0726ec..98dd886 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -476,13 +476,14 @@ static inline int qemu_gluster_zerofill(struct glfs_fd 
*fd, int64_t offset,
 #endif
 
 static int qemu_gluster_create(const char *filename,
-QEMUOptionParameter *options, Error **errp)
+   QemuOpts *opts, Error **errp)
 {
 struct glfs *glfs;
 struct glfs_fd *fd;
 int ret = 0;
 int prealloc = 0;
 int64_t total_size = 0;
+char *tmp = NULL;
 GlusterConf *gconf = g_malloc0(sizeof(GlusterConf));
 
 glfs = qemu_gluster_init(gconf, filename, errp);
@@ -491,24 +492,21 @@ static int qemu_gluster_create(const char *filename,
 goto out;
 }
 
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-total_size = options-value.n / BDRV_SECTOR_SIZE;
-} else if (!strcmp(options-name, BLOCK_OPT_PREALLOC)) {
-if (!options-value.s || !strcmp(options-value.s, off)) {
-prealloc = 0;
-} else if (!strcmp(options-value.s, full) 
-gluster_supports_zerofill()) {
-prealloc = 1;
-} else {
-error_setg(errp, Invalid preallocation mode: '%s'
- or GlusterFS doesn't support zerofill API,
-   options-value.s);
-ret = -EINVAL;
-goto out;
-}
-}
-options++;
+total_size =
+qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
+
+tmp = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
+if (!tmp || !strcmp(tmp, off)) {
+prealloc = 0;
+} else if (!strcmp(tmp, full) 
+   gluster_supports_zerofill()) {
+prealloc = 1;
+} else {
+error_setg(errp, Invalid preallocation mode: '%s'
+ or GlusterFS doesn't support zerofill API,
+tmp);
+ret = -EINVAL;
+goto out;
 }
 
 fd = glfs_creat(glfs, gconf-image,
@@ -530,6 +528,7 @@ static int qemu_gluster_create(const char *filename,
 }
 }
 out:
+g_free(tmp);
 qemu_gluster_gconf_free(gconf);
 if (glfs) {
 glfs_fini(glfs);
@@ -693,18 +692,22 @@ static int qemu_gluster_has_zero_init(BlockDriverState 
*bs)
 return 0;
 }
 
-static QEMUOptionParameter qemu_gluster_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{
-.name = BLOCK_OPT_PREALLOC,
-.type = OPT_STRING,
-.help = Preallocation mode (allowed values: off, full)
-},
-{ NULL }
+static QemuOptsList qemu_gluster_create_opts = {
+.name = qemu-gluster-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(qemu_gluster_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{
+.name = BLOCK_OPT_PREALLOC,
+.type = QEMU_OPT_STRING,
+.help = Preallocation mode (allowed values: off, full)
+},
+{ /* end of list */ }
+}
 };
 
 static BlockDriver bdrv_gluster = {
@@ -717,7 +720,7 @@ static BlockDriver bdrv_gluster = {
 .bdrv_reopen_commit   = qemu_gluster_reopen_commit,
 .bdrv_reopen_abort= qemu_gluster_reopen_abort,
 .bdrv_close   = qemu_gluster_close,
-.bdrv_create  = qemu_gluster_create,
+.bdrv_create2 = qemu_gluster_create,
 .bdrv_getlength   = qemu_gluster_getlength,
 .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
 .bdrv_truncate= qemu_gluster_truncate,
@@ -731,7 +734,7 @@ static BlockDriver bdrv_gluster = {
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
 .bdrv_co_write_zeroes = qemu_gluster_co_write_zeroes,
 #endif
-.create_options   = qemu_gluster_create_options,
+.create_opts  = qemu_gluster_create_opts,
 };
 
 static BlockDriver bdrv_gluster_tcp = {
@@ -744,7 +747,7 @@ static BlockDriver bdrv_gluster_tcp = {
 .bdrv_reopen_commit   = qemu_gluster_reopen_commit,
 .bdrv_reopen_abort= qemu_gluster_reopen_abort,
 .bdrv_close   = qemu_gluster_close,
-.bdrv_create  = qemu_gluster_create,
+.bdrv_create2 = qemu_gluster_create,
 .bdrv_getlength   = qemu_gluster_getlength,
 .bdrv_get_allocated_file_size = qemu_gluster_allocated_file_size,
 

[Qemu-devel] [PATCH v28 26/33] sheepdog.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/sheepdog.c | 111 ---
 1 file changed, 56 insertions(+), 55 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 4ecbf5f..c971117 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1636,12 +1636,13 @@ static int parse_redundancy(BDRVSheepdogState *s, const 
char *opt)
 return 0;
 }
 
-static int sd_create(const char *filename, QEMUOptionParameter *options,
+static int sd_create(const char *filename, QemuOpts *opts,
  Error **errp)
 {
 int ret = 0;
 uint32_t vid = 0;
 char *backing_file = NULL;
+char *buf = NULL;
 BDRVSheepdogState *s;
 char tag[SD_MAX_VDI_TAG_LEN];
 uint32_t snapid;
@@ -1660,33 +1661,27 @@ static int sd_create(const char *filename, 
QEMUOptionParameter *options,
 goto out;
 }
 
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-s-inode.vdi_size = options-value.n;
-} else if (!strcmp(options-name, BLOCK_OPT_BACKING_FILE)) {
-backing_file = options-value.s;
-} else if (!strcmp(options-name, BLOCK_OPT_PREALLOC)) {
-if (!options-value.s || !strcmp(options-value.s, off)) {
-prealloc = false;
-} else if (!strcmp(options-value.s, full)) {
-prealloc = true;
-} else {
-error_setg(errp, Invalid preallocation mode: '%s',
-   options-value.s);
-ret = -EINVAL;
-goto out;
-}
-} else if (!strcmp(options-name, BLOCK_OPT_REDUNDANCY)) {
-if (options-value.s) {
-ret = parse_redundancy(s, options-value.s);
-if (ret  0) {
-error_setg(errp, Invalid redundancy mode: '%s',
-   options-value.s);
-goto out;
-}
-}
+s-inode.vdi_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
+buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
+if (!buf || !strcmp(buf, off)) {
+prealloc = false;
+} else if (!strcmp(buf, full)) {
+prealloc = true;
+} else {
+error_setg(errp, Invalid preallocation mode: '%s', buf);
+ret = -EINVAL;
+goto out;
+}
+
+g_free(buf);
+buf = qemu_opt_get_del(opts, BLOCK_OPT_REDUNDANCY);
+if (buf) {
+ret = parse_redundancy(s, buf);
+if (ret  0) {
+error_setg(errp, Invalid redundancy mode: '%s', buf);
+goto out;
 }
-options++;
 }
 
 if (s-inode.vdi_size  SD_MAX_VDI_SIZE) {
@@ -1736,6 +1731,8 @@ static int sd_create(const char *filename, 
QEMUOptionParameter *options,
 ret = sd_prealloc(filename, errp);
 }
 out:
+g_free(backing_file);
+g_free(buf);
 g_free(s);
 return ret;
 }
@@ -2529,28 +2526,32 @@ static int64_t 
sd_get_allocated_file_size(BlockDriverState *bs)
 return size;
 }
 
-static QEMUOptionParameter sd_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{
-.name = BLOCK_OPT_BACKING_FILE,
-.type = OPT_STRING,
-.help = File name of a base image
-},
-{
-.name = BLOCK_OPT_PREALLOC,
-.type = OPT_STRING,
-.help = Preallocation mode (allowed values: off, full)
-},
-{
-.name = BLOCK_OPT_REDUNDANCY,
-.type = OPT_STRING,
-.help = Redundancy of the image
-},
-{ NULL }
+static QemuOptsList sd_create_opts = {
+.name = sheepdog-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(sd_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{
+.name = BLOCK_OPT_BACKING_FILE,
+.type = QEMU_OPT_STRING,
+.help = File name of a base image
+},
+{
+.name = BLOCK_OPT_PREALLOC,
+.type = QEMU_OPT_STRING,
+.help = Preallocation mode (allowed values: off, full)
+},
+{
+.name = BLOCK_OPT_REDUNDANCY,
+.type = QEMU_OPT_STRING,
+.help = Redundancy of the image
+},
+{ /* end of list */ }
+}
 };
 
 static BlockDriver bdrv_sheepdog = {
@@ -2560,7 +2561,7 @@ static BlockDriver bdrv_sheepdog = {
 .bdrv_needs_filename = true,
 .bdrv_file_open = sd_open,
 .bdrv_close = sd_close,
-.bdrv_create= sd_create,
+.bdrv_create2   = sd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 .bdrv_getlength = sd_getlength,
 .bdrv_get_allocated_file_size = 

[Qemu-devel] [PATCH v28 20/33] qcow2.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
Changes:
  * same as v27
  * changes to v26:
split v26 qcow2.c patch into two, move qemu_opt_find to a separate patch.

 block/qcow2.c | 261 +++---
 1 file changed, 139 insertions(+), 122 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6b95a7f..7150df6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -31,6 +31,7 @@
 #include qapi/qmp/qerror.h
 #include qapi/qmp/qbool.h
 #include trace.h
+#include qemu/option_int.h
 
 /*
   Differences with QCOW:
@@ -1594,7 +1595,7 @@ static int preallocate(BlockDriverState *bs)
 static int qcow2_create2(const char *filename, int64_t total_size,
  const char *backing_file, const char *backing_format,
  int flags, size_t cluster_size, int prealloc,
- QEMUOptionParameter *options, int version,
+ QemuOpts *opts, int version,
  Error **errp)
 {
 /* Calculate cluster_bits */
@@ -1626,7 +1627,7 @@ static int qcow2_create2(const char *filename, int64_t 
total_size,
 Error *local_err = NULL;
 int ret;
 
-ret = bdrv_create_file(filename, options, NULL, local_err);
+ret = bdrv_create_file(filename, NULL, opts, local_err);
 if (ret  0) {
 error_propagate(errp, local_err);
 return ret;
@@ -1762,11 +1763,11 @@ out:
 return ret;
 }
 
-static int qcow2_create(const char *filename, QEMUOptionParameter *options,
-Error **errp)
+static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
 {
-const char *backing_file = NULL;
-const char *backing_fmt = NULL;
+char *backing_file = NULL;
+char *backing_fmt = NULL;
+char *buf = NULL;
 uint64_t sectors = 0;
 int flags = 0;
 size_t cluster_size = DEFAULT_CLUSTER_SIZE;
@@ -1776,64 +1777,66 @@ static int qcow2_create(const char *filename, 
QEMUOptionParameter *options,
 int ret;
 
 /* Read out options */
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-sectors = options-value.n / 512;
-} else if (!strcmp(options-name, BLOCK_OPT_BACKING_FILE)) {
-backing_file = options-value.s;
-} else if (!strcmp(options-name, BLOCK_OPT_BACKING_FMT)) {
-backing_fmt = options-value.s;
-} else if (!strcmp(options-name, BLOCK_OPT_ENCRYPT)) {
-flags |= options-value.n ? BLOCK_FLAG_ENCRYPT : 0;
-} else if (!strcmp(options-name, BLOCK_OPT_CLUSTER_SIZE)) {
-if (options-value.n) {
-cluster_size = options-value.n;
-}
-} else if (!strcmp(options-name, BLOCK_OPT_PREALLOC)) {
-if (!options-value.s || !strcmp(options-value.s, off)) {
-prealloc = 0;
-} else if (!strcmp(options-value.s, metadata)) {
-prealloc = 1;
-} else {
-error_setg(errp, Invalid preallocation mode: '%s',
-   options-value.s);
-return -EINVAL;
-}
-} else if (!strcmp(options-name, BLOCK_OPT_COMPAT_LEVEL)) {
-if (!options-value.s) {
-/* keep the default */
-} else if (!strcmp(options-value.s, 0.10)) {
-version = 2;
-} else if (!strcmp(options-value.s, 1.1)) {
-version = 3;
-} else {
-error_setg(errp, Invalid compatibility level: '%s',
-   options-value.s);
-return -EINVAL;
-}
-} else if (!strcmp(options-name, BLOCK_OPT_LAZY_REFCOUNTS)) {
-flags |= options-value.n ? BLOCK_FLAG_LAZY_REFCOUNTS : 0;
-}
-options++;
+sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
+backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
+if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
+flags |= BLOCK_FLAG_ENCRYPT;
+}
+cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
+ DEFAULT_CLUSTER_SIZE);
+buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC);
+if (!buf || !strcmp(buf, off)) {
+prealloc = 0;
+} else if (!strcmp(buf, metadata)) {
+prealloc = 1;
+} else {
+error_setg(errp, Invalid preallocation mode: '%s', buf);
+ret = -EINVAL;
+goto finish;
+}
+g_free(buf);
+buf = qemu_opt_get_del(opts, BLOCK_OPT_COMPAT_LEVEL);
+if (!buf) {
+/* keep the default */
+} else if (!strcmp(buf, 0.10)) {
+version = 2;
+} else if (!strcmp(buf, 1.1)) {
+version = 3;
+} else {
+error_setg(errp, Invalid compatibility level: '%s', buf);
+ret = 

[Qemu-devel] [PATCH v28 12/33] change block layer to support both QemuOpts and QEMUOptionParamter

2014-06-05 Thread Chunyan Liu
Change block layer to support both QemuOpts and QEMUOptionParameter.
After this patch, it will change backend drivers one by one. At the end,
QEMUOptionParameter will be removed and only QemuOpts is kept.

Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 block.c| 160 +++--
 block/cow.c|   2 +-
 block/qcow.c   |   2 +-
 block/qcow2.c  |   2 +-
 block/qed.c|   2 +-
 block/raw_bsd.c|   2 +-
 block/vhdx.c   |   2 +-
 block/vmdk.c   |   4 +-
 block/vvfat.c  |   2 +-
 include/block/block.h  |   7 +-
 include/block/block_int.h  |  13 +++-
 qemu-img.c |  96 +--
 tests/qemu-iotests/049.out |   2 +-
 tests/qemu-iotests/061.out |   2 +-
 14 files changed, 184 insertions(+), 114 deletions(-)

diff --git a/block.c b/block.c
index 310ea89..ebb76cf 100644
--- a/block.c
+++ b/block.c
@@ -328,6 +328,13 @@ void bdrv_register(BlockDriver *bdrv)
 }
 }
 
+if (bdrv-bdrv_create) {
+assert(!bdrv-bdrv_create2  !bdrv-create_opts);
+assert(!bdrv-bdrv_amend_options2);
+} else if (bdrv-bdrv_create2) {
+assert(!bdrv-bdrv_create  !bdrv-create_options);
+assert(!bdrv-bdrv_amend_options);
+}
 QLIST_INSERT_HEAD(bdrv_drivers, bdrv, list);
 }
 
@@ -423,6 +430,7 @@ typedef struct CreateCo {
 BlockDriver *drv;
 char *filename;
 QEMUOptionParameter *options;
+QemuOpts *opts;
 int ret;
 Error *err;
 } CreateCo;
@@ -434,8 +442,28 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
 
 CreateCo *cco = opaque;
 assert(cco-drv);
+assert(!(cco-options  cco-opts));
 
-ret = cco-drv-bdrv_create(cco-filename, cco-options, local_err);
+if (cco-drv-bdrv_create2) {
+QemuOptsList *opts_list = NULL;
+if (cco-options) {
+opts_list = params_to_opts(cco-options);
+cco-opts = qemu_opts_create(opts_list, NULL, 0, error_abort);
+}
+ret = cco-drv-bdrv_create2(cco-filename, cco-opts, local_err);
+if (cco-options) {
+qemu_opts_del(cco-opts);
+qemu_opts_free(opts_list);
+}
+} else {
+if (cco-opts) {
+cco-options = opts_to_params(cco-opts);
+}
+ret = cco-drv-bdrv_create(cco-filename, cco-options, local_err);
+if (cco-opts) {
+free_option_parameters(cco-options);
+}
+}
 if (local_err) {
 error_propagate(cco-err, local_err);
 }
@@ -443,7 +471,8 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
 }
 
 int bdrv_create(BlockDriver *drv, const char* filename,
-QEMUOptionParameter *options, Error **errp)
+QEMUOptionParameter *options,
+QemuOpts *opts, Error **errp)
 {
 int ret;
 
@@ -452,11 +481,12 @@ int bdrv_create(BlockDriver *drv, const char* filename,
 .drv = drv,
 .filename = g_strdup(filename),
 .options = options,
+.opts = opts,
 .ret = NOT_DONE,
 .err = NULL,
 };
 
-if (!drv-bdrv_create) {
+if (!drv-bdrv_create  !drv-bdrv_create2) {
 error_setg(errp, Driver '%s' does not support image creation, 
drv-format_name);
 ret = -ENOTSUP;
 goto out;
@@ -488,7 +518,7 @@ out:
 }
 
 int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
- Error **errp)
+ QemuOpts *opts, Error **errp)
 {
 BlockDriver *drv;
 Error *local_err = NULL;
@@ -500,7 +530,7 @@ int bdrv_create_file(const char* filename, 
QEMUOptionParameter *options,
 return -ENOENT;
 }
 
-ret = bdrv_create(drv, filename, options, local_err);
+ret = bdrv_create(drv, filename, options, opts, local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 }
@@ -1245,7 +1275,8 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, int 
flags, Error **errp)
 char *tmp_filename = g_malloc0(PATH_MAX + 1);
 int64_t total_size;
 BlockDriver *bdrv_qcow2;
-QEMUOptionParameter *create_options;
+QemuOptsList *create_opts = NULL;
+QemuOpts *opts = NULL;
 QDict *snapshot_options;
 BlockDriverState *bs_snapshot;
 Error *local_err;
@@ -1270,13 +1301,20 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, 
int flags, Error **errp)
 }
 
 bdrv_qcow2 = bdrv_find_format(qcow2);
-create_options = parse_option_parameters(, bdrv_qcow2-create_options,
- NULL);
-
-set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
 
-ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, local_err);
-free_option_parameters(create_options);
+assert(!(bdrv_qcow2-create_options  bdrv_qcow2-create_opts));
+

[Qemu-devel] [PATCH v28 13/33] vvfat.c: handle cross_driver's create_options and create_opts

2014-06-05 Thread Chunyan Liu
vvfat shares create options of qcow driver. To avoid vvfat breaking when
qcow driver changes from QEMUOptionParameter to QemuOpts, let it able
to handle both cases.

Signed-off-by: Chunyan Liu cy...@suse.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 block/vvfat.c | 24 +---
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index d895582..b1ab195 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2910,8 +2910,9 @@ static BlockDriver vvfat_write_target = {
 
 static int enable_write_target(BDRVVVFATState *s, Error **errp)
 {
-BlockDriver *bdrv_qcow;
-QEMUOptionParameter *options;
+BlockDriver *bdrv_qcow = NULL;
+QemuOptsList *create_opts = NULL;
+QemuOpts *opts = NULL;
 int ret;
 int size = sector2cluster(s, s-sector_count);
 s-used_clusters = calloc(size, 1);
@@ -2926,12 +2927,21 @@ static int enable_write_target(BDRVVVFATState *s, Error 
**errp)
 }
 
 bdrv_qcow = bdrv_find_format(qcow);
-options = parse_option_parameters(, bdrv_qcow-create_options, NULL);
-set_option_parameter_int(options, BLOCK_OPT_SIZE, s-sector_count * 512);
-set_option_parameter(options, BLOCK_OPT_BACKING_FILE, fat:);
+assert(!(bdrv_qcow-create_opts  bdrv_qcow-create_options));
+if (bdrv_qcow-create_options) {
+create_opts = params_to_opts(bdrv_qcow-create_options);
+} else {
+create_opts = bdrv_qcow-create_opts;
+}
+opts = qemu_opts_create(create_opts, NULL, 0, error_abort);
+qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s-sector_count * 512);
+qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, fat:);
 
-ret = bdrv_create(bdrv_qcow, s-qcow_filename, options, NULL, errp);
-free_option_parameters(options);
+ret = bdrv_create(bdrv_qcow, s-qcow_filename, NULL, opts, errp);
+qemu_opts_del(opts);
+if (bdrv_qcow-create_options) {
+qemu_opts_free(create_opts);
+}
 if (ret  0) {
 goto err;
 }
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 28/33] vdi.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/vdi.c | 73 +
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 27737af..fe5cad2 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -673,8 +673,7 @@ static int vdi_co_write(BlockDriverState *bs,
 return ret;
 }
 
-static int vdi_create(const char *filename, QEMUOptionParameter *options,
-  Error **errp)
+static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int fd;
 int result = 0;
@@ -689,25 +688,18 @@ static int vdi_create(const char *filename, 
QEMUOptionParameter *options,
 logout(\n);
 
 /* Read out options. */
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-bytes = options-value.n;
+bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
 #if defined(CONFIG_VDI_BLOCK_SIZE)
-} else if (!strcmp(options-name, BLOCK_OPT_CLUSTER_SIZE)) {
-if (options-value.n) {
-/* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */
-block_size = options-value.n;
-}
+/* TODO: Additional checks (SECTOR_SIZE * 2^n, ...). */
+block_size = qemu_opt_get_size_del(opts,
+   BLOCK_OPT_CLUSTER_SIZE,
+   DEFAULT_CLUSTER_SIZE);
 #endif
 #if defined(CONFIG_VDI_STATIC_IMAGE)
-} else if (!strcmp(options-name, BLOCK_OPT_STATIC)) {
-if (options-value.n) {
-image_type = VDI_TYPE_STATIC;
-}
-#endif
-}
-options++;
+if (qemu_opt_get_bool_del(opts, BLOCK_OPT_STATIC, false)) {
+image_type = VDI_TYPE_STATIC;
 }
+#endif
 
 if (bytes  VDI_DISK_SIZE_MAX) {
 result = -ENOTSUP;
@@ -802,29 +794,34 @@ static void vdi_close(BlockDriverState *bs)
 error_free(s-migration_blocker);
 }
 
-static QEMUOptionParameter vdi_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
+static QemuOptsList vdi_create_opts = {
+.name = vdi-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(vdi_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
 #if defined(CONFIG_VDI_BLOCK_SIZE)
-{
-.name = BLOCK_OPT_CLUSTER_SIZE,
-.type = OPT_SIZE,
-.help = VDI cluster (block) size,
-.value = { .n = DEFAULT_CLUSTER_SIZE },
-},
+{
+.name = BLOCK_OPT_CLUSTER_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = VDI cluster (block) size,
+.def_value_str = stringify(DEFAULT_CLUSTER_SIZE)
+},
 #endif
 #if defined(CONFIG_VDI_STATIC_IMAGE)
-{
-.name = BLOCK_OPT_STATIC,
-.type = OPT_FLAG,
-.help = VDI static (pre-allocated) image
-},
+{
+.name = BLOCK_OPT_STATIC,
+.type = QEMU_OPT_BOOL,
+.help = VDI static (pre-allocated) image,
+.def_value_str = off
+},
 #endif
-/* TODO: An additional option to set UUID values might be useful. */
-{ NULL }
+/* TODO: An additional option to set UUID values might be useful. */
+{ /* end of list */ }
+}
 };
 
 static BlockDriver bdrv_vdi = {
@@ -834,7 +831,7 @@ static BlockDriver bdrv_vdi = {
 .bdrv_open = vdi_open,
 .bdrv_close = vdi_close,
 .bdrv_reopen_prepare = vdi_reopen_prepare,
-.bdrv_create = vdi_create,
+.bdrv_create2 = vdi_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 .bdrv_co_get_block_status = vdi_co_get_block_status,
 .bdrv_make_empty = vdi_make_empty,
@@ -846,7 +843,7 @@ static BlockDriver bdrv_vdi = {
 
 .bdrv_get_info = vdi_get_info,
 
-.create_options = vdi_create_options,
+.create_opts = vdi_create_opts,
 .bdrv_check = vdi_check,
 };
 
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 21/33] qed.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
One extra change is to define QED_DEFAULT_CLUSTER_SIZE = 65536 instead
of 64 * 1024; because:
according to existing create_options, cluster size has default value =
QED_DEFAULT_CLUSTER_SIZE, after switching to create_opts, this has to be
stringized and set to .def_value_str. That is,
  .def_value_str = stringify(QED_DEFAULT_CLUSTER_SIZE),
so the QED_DEFAULT_CLUSTER_SIZE could not be a expression.

Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/qed.c | 114 
 block/qed.h |   3 +-
 2 files changed, 61 insertions(+), 56 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index 2982640..9376996 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -621,55 +621,53 @@ out:
 return ret;
 }
 
-static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options,
-   Error **errp)
+static int bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 uint64_t image_size = 0;
 uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
 uint32_t table_size = QED_DEFAULT_TABLE_SIZE;
-const char *backing_file = NULL;
-const char *backing_fmt = NULL;
-
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-image_size = options-value.n;
-} else if (!strcmp(options-name, BLOCK_OPT_BACKING_FILE)) {
-backing_file = options-value.s;
-} else if (!strcmp(options-name, BLOCK_OPT_BACKING_FMT)) {
-backing_fmt = options-value.s;
-} else if (!strcmp(options-name, BLOCK_OPT_CLUSTER_SIZE)) {
-if (options-value.n) {
-cluster_size = options-value.n;
-}
-} else if (!strcmp(options-name, BLOCK_OPT_TABLE_SIZE)) {
-if (options-value.n) {
-table_size = options-value.n;
-}
-}
-options++;
-}
+char *backing_file = NULL;
+char *backing_fmt = NULL;
+int ret;
+
+image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
+backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
+cluster_size = qemu_opt_get_size_del(opts,
+ BLOCK_OPT_CLUSTER_SIZE,
+ QED_DEFAULT_CLUSTER_SIZE);
+table_size = qemu_opt_get_size_del(opts, BLOCK_OPT_TABLE_SIZE,
+   QED_DEFAULT_TABLE_SIZE);
 
 if (!qed_is_cluster_size_valid(cluster_size)) {
 error_setg(errp, QED cluster size must be within range [%u, %u] 
  and power of 2,
QED_MIN_CLUSTER_SIZE, QED_MAX_CLUSTER_SIZE);
-return -EINVAL;
+ret = -EINVAL;
+goto finish;
 }
 if (!qed_is_table_size_valid(table_size)) {
 error_setg(errp, QED table size must be within range [%u, %u] 
  and power of 2,
QED_MIN_TABLE_SIZE, QED_MAX_TABLE_SIZE);
-return -EINVAL;
+ret = -EINVAL;
+goto finish;
 }
 if (!qed_is_image_size_valid(image_size, cluster_size, table_size)) {
 error_setg(errp, QED image size must be a non-zero multiple of 
  cluster size and less than % PRIu64  bytes,
qed_max_image_size(cluster_size, table_size));
-return -EINVAL;
+ret = -EINVAL;
+goto finish;
 }
 
-return qed_create(filename, cluster_size, image_size, table_size,
-  backing_file, backing_fmt, errp);
+ret = qed_create(filename, cluster_size, image_size, table_size,
+ backing_file, backing_fmt, errp);
+
+finish:
+g_free(backing_file);
+g_free(backing_fmt);
+return ret;
 }
 
 typedef struct {
@@ -1595,43 +1593,51 @@ static int bdrv_qed_check(BlockDriverState *bs, 
BdrvCheckResult *result,
 return qed_check(s, result, !!fix);
 }
 
-static QEMUOptionParameter qed_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size (in bytes)
-}, {
-.name = BLOCK_OPT_BACKING_FILE,
-.type = OPT_STRING,
-.help = File name of a base image
-}, {
-.name = BLOCK_OPT_BACKING_FMT,
-.type = OPT_STRING,
-.help = Image format of the base image
-}, {
-.name = BLOCK_OPT_CLUSTER_SIZE,
-.type = OPT_SIZE,
-.help = Cluster size (in bytes),
-.value = { .n = QED_DEFAULT_CLUSTER_SIZE },
-}, {
-.name = BLOCK_OPT_TABLE_SIZE,
-.type = OPT_SIZE,
-.help = L1/L2 table size (in clusters)
-},
-{ /* end of list */ }
+static QemuOptsList qed_create_opts = {
+.name = qed-create-opts,
+.head = 

[Qemu-devel] [PATCH v28 25/33] rbd.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/rbd.c | 63 +
 1 file changed, 30 insertions(+), 33 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 09af484..d7f22af 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -289,8 +289,7 @@ static int qemu_rbd_set_conf(rados_t cluster, const char 
*conf, Error **errp)
 return ret;
 }
 
-static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
-   Error **errp)
+static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 Error *local_err = NULL;
 int64_t bytes = 0;
@@ -315,24 +314,18 @@ static int qemu_rbd_create(const char *filename, 
QEMUOptionParameter *options,
 }
 
 /* Read out options */
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-bytes = options-value.n;
-} else if (!strcmp(options-name, BLOCK_OPT_CLUSTER_SIZE)) {
-if (options-value.n) {
-objsize = options-value.n;
-if ((objsize - 1)  objsize) {/* not a power of 2? */
-error_setg(errp, obj size needs to be power of 2);
-return -EINVAL;
-}
-if (objsize  4096) {
-error_setg(errp, obj size too small);
-return -EINVAL;
-}
-obj_order = ffs(objsize) - 1;
-}
+bytes = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+objsize = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE, 0);
+if (objsize) {
+if ((objsize - 1)  objsize) {/* not a power of 2? */
+error_setg(errp, obj size needs to be power of 2);
+return -EINVAL;
+}
+if (objsize  4096) {
+error_setg(errp, obj size too small);
+return -EINVAL;
 }
-options++;
+obj_order = ffs(objsize) - 1;
 }
 
 clientname = qemu_rbd_parse_clientname(conf, clientname_buf);
@@ -907,18 +900,22 @@ static BlockDriverAIOCB* 
qemu_rbd_aio_discard(BlockDriverState *bs,
 }
 #endif
 
-static QEMUOptionParameter qemu_rbd_create_options[] = {
-{
- .name = BLOCK_OPT_SIZE,
- .type = OPT_SIZE,
- .help = Virtual disk size
-},
-{
- .name = BLOCK_OPT_CLUSTER_SIZE,
- .type = OPT_SIZE,
- .help = RBD object size
-},
-{NULL}
+static QemuOptsList qemu_rbd_create_opts = {
+.name = rbd-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(qemu_rbd_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{
+.name = BLOCK_OPT_CLUSTER_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = RBD object size
+},
+{ /* end of list */ }
+}
 };
 
 static BlockDriver bdrv_rbd = {
@@ -927,10 +924,10 @@ static BlockDriver bdrv_rbd = {
 .bdrv_needs_filename = true,
 .bdrv_file_open = qemu_rbd_open,
 .bdrv_close = qemu_rbd_close,
-.bdrv_create= qemu_rbd_create,
+.bdrv_create2   = qemu_rbd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 .bdrv_get_info  = qemu_rbd_getinfo,
-.create_options = qemu_rbd_create_options,
+.create_opts= qemu_rbd_create_opts,
 .bdrv_getlength = qemu_rbd_getlength,
 .bdrv_truncate  = qemu_rbd_truncate,
 .protocol_name  = rbd,
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 14/33] cow.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/cow.c | 54 ++
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index 7e61024..af85753 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -324,31 +324,24 @@ static void cow_close(BlockDriverState *bs)
 {
 }
 
-static int cow_create(const char *filename, QEMUOptionParameter *options,
-  Error **errp)
+static int cow_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 struct cow_header_v2 cow_header;
 struct stat st;
 int64_t image_sectors = 0;
-const char *image_filename = NULL;
+char *image_filename = NULL;
 Error *local_err = NULL;
 int ret;
 BlockDriverState *cow_bs;
 
 /* Read out options */
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-image_sectors = options-value.n / 512;
-} else if (!strcmp(options-name, BLOCK_OPT_BACKING_FILE)) {
-image_filename = options-value.s;
-}
-options++;
-}
+image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
+image_filename = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
 
-ret = bdrv_create_file(filename, options, NULL, local_err);
+ret = bdrv_create_file(filename, NULL, opts, local_err);
 if (ret  0) {
 error_propagate(errp, local_err);
-return ret;
+goto exit;
 }
 
 cow_bs = NULL;
@@ -356,7 +349,7 @@ static int cow_create(const char *filename, 
QEMUOptionParameter *options,
 BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, local_err);
 if (ret  0) {
 error_propagate(errp, local_err);
-return ret;
+goto exit;
 }
 
 memset(cow_header, 0, sizeof(cow_header));
@@ -389,22 +382,27 @@ static int cow_create(const char *filename, 
QEMUOptionParameter *options,
 }
 
 exit:
+g_free(image_filename);
 bdrv_unref(cow_bs);
 return ret;
 }
 
-static QEMUOptionParameter cow_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{
-.name = BLOCK_OPT_BACKING_FILE,
-.type = OPT_STRING,
-.help = File name of a base image
-},
-{ NULL }
+static QemuOptsList cow_create_opts = {
+.name = cow-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(cow_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{
+.name = BLOCK_OPT_BACKING_FILE,
+.type = QEMU_OPT_STRING,
+.help = File name of a base image
+},
+{ /* end of list */ }
+}
 };
 
 static BlockDriver bdrv_cow = {
@@ -414,14 +412,14 @@ static BlockDriver bdrv_cow = {
 .bdrv_probe = cow_probe,
 .bdrv_open  = cow_open,
 .bdrv_close = cow_close,
-.bdrv_create= cow_create,
+.bdrv_create2   = cow_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 
 .bdrv_read  = cow_co_read,
 .bdrv_write = cow_co_write,
 .bdrv_co_get_block_status   = cow_co_get_block_status,
 
-.create_options = cow_create_options,
+.create_opts= cow_create_opts,
 };
 
 static void bdrv_cow_init(void)
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 29/33] vhdx.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/vhdx.c | 99 +---
 block/vhdx.h |  1 +
 2 files changed, 48 insertions(+), 52 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index ca7d533..e36f897 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1723,8 +1723,7 @@ exit:
  *. ~ --- ~  ~  ~ ---.
  *   1MB
  */
-static int vhdx_create(const char *filename, QEMUOptionParameter *options,
-   Error **errp)
+static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int ret = 0;
 uint64_t image_size = (uint64_t) 2 * GiB;
@@ -1737,24 +1736,15 @@ static int vhdx_create(const char *filename, 
QEMUOptionParameter *options,
 gunichar2 *creator = NULL;
 glong creator_items;
 BlockDriverState *bs;
-const char *type = NULL;
+char *type = NULL;
 VHDXImageType image_type;
 Error *local_err = NULL;
 
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-image_size = options-value.n;
-} else if (!strcmp(options-name, VHDX_BLOCK_OPT_LOG_SIZE)) {
-log_size = options-value.n;
-} else if (!strcmp(options-name, VHDX_BLOCK_OPT_BLOCK_SIZE)) {
-block_size = options-value.n;
-} else if (!strcmp(options-name, BLOCK_OPT_SUBFMT)) {
-type = options-value.s;
-} else if (!strcmp(options-name, VHDX_BLOCK_OPT_ZERO)) {
-use_zero_blocks = options-value.n != 0;
-}
-options++;
-}
+image_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
+block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
+type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
+use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, false);
 
 if (image_size  VHDX_MAX_IMAGE_SIZE) {
 error_setg_errno(errp, EINVAL, Image size too large; max of 64TB);
@@ -1763,7 +1753,7 @@ static int vhdx_create(const char *filename, 
QEMUOptionParameter *options,
 }
 
 if (type == NULL) {
-type = dynamic;
+type = g_strdup(dynamic);
 }
 
 if (!strcmp(type, dynamic)) {
@@ -1803,7 +1793,7 @@ static int vhdx_create(const char *filename, 
QEMUOptionParameter *options,
 block_size = block_size  VHDX_BLOCK_SIZE_MAX ? VHDX_BLOCK_SIZE_MAX :
 block_size;
 
-ret = bdrv_create_file(filename, options, NULL, local_err);
+ret = bdrv_create_file(filename, NULL, opts, local_err);
 if (ret  0) {
 error_propagate(errp, local_err);
 goto exit;
@@ -1863,6 +1853,7 @@ static int vhdx_create(const char *filename, 
QEMUOptionParameter *options,
 delete_and_exit:
 bdrv_unref(bs);
 exit:
+g_free(type);
 g_free(creator);
 return ret;
 }
@@ -1885,37 +1876,41 @@ static int vhdx_check(BlockDriverState *bs, 
BdrvCheckResult *result,
 return 0;
 }
 
-static QEMUOptionParameter vhdx_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size; max of 64TB.
-},
-{
-.name = VHDX_BLOCK_OPT_LOG_SIZE,
-.type = OPT_SIZE,
-.value.n = 1 * MiB,
-.help = Log size; min 1MB.
-},
-{
-.name = VHDX_BLOCK_OPT_BLOCK_SIZE,
-.type = OPT_SIZE,
-.value.n = 0,
-.help = Block Size; min 1MB, max 256MB.  \
-0 means auto-calculate based on image size.
-},
-{
-.name = BLOCK_OPT_SUBFMT,
-.type = OPT_STRING,
-.help = VHDX format type, can be either 'dynamic' or 'fixed'. \
-Default is 'dynamic'.
-},
-{
-.name = VHDX_BLOCK_OPT_ZERO,
-.type = OPT_FLAG,
-.help = Force use of payload blocks of type 'ZERO'.  Non-standard.
-},
-{ NULL }
+static QemuOptsList vhdx_create_opts = {
+.name = vhdx-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(vhdx_create_opts.head),
+.desc = {
+{
+   .name = BLOCK_OPT_SIZE,
+   .type = QEMU_OPT_SIZE,
+   .help = Virtual disk size; max of 64TB.
+   },
+   {
+   .name = VHDX_BLOCK_OPT_LOG_SIZE,
+   .type = QEMU_OPT_SIZE,
+   .def_value_str = stringify(DEFAULT_LOG_SIZE),
+   .help = Log size; min 1MB.
+   },
+   {
+   .name = VHDX_BLOCK_OPT_BLOCK_SIZE,
+   .type = QEMU_OPT_SIZE,
+   .def_value_str = stringify(0),
+   .help = Block Size; min 1MB, max 256MB.  \
+   0 means auto-calculate based on image size.
+   },
+   {
+   .name = BLOCK_OPT_SUBFMT,
+   .type = QEMU_OPT_STRING,
+   .help = VHDX format type, can be either 'dynamic' 

[Qemu-devel] [PATCH v28 19/33] QemuOpts: export qemu_opt_find

2014-06-05 Thread Chunyan Liu
Export qemu_opt_find for qcow2 driver using it.
After replacing QEMUOptionParameter with QemuOpts, qcow2 driver will
use qemu_opt_find to judge if an option is explicitly set, to replace
the usage of .assigned in QEMUOptionParameter.

Signed-off-by: Chunyan Liu cy...@suse.com
---
Changes:
  * same as v27.
  * changes to v26:
Following Eric's comment, split v26 qcow2.c patch into two:
export qemu_opt_find first and then qcow2.c patch

 include/qemu/option.h | 1 +
 util/qemu-option.c| 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 44d9961..3455267 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -130,6 +130,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name);
  * Returns: true if @opts includes 'help' or equivalent.
  */
 bool qemu_opt_has_help_opt(QemuOpts *opts);
+QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name);
 bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval);
 uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t 
defval);
 uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 40e1ff3..0d9d3ec 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -568,7 +568,7 @@ void qemu_opts_print_help(QemuOptsList *list)
 }
 /* -- */
 
-static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
+QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
 {
 QemuOpt *opt;
 
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 27/33] ssh.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/ssh.c | 32 +++-
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index b212971..b6d55bc 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -675,17 +675,20 @@ static int ssh_file_open(BlockDriverState *bs, QDict 
*options, int bdrv_flags,
 return ret;
 }
 
-static QEMUOptionParameter ssh_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{ NULL }
+static QemuOptsList ssh_create_opts = {
+.name = ssh-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(ssh_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{ /* end of list */ }
+}
 };
 
-static int ssh_create(const char *filename, QEMUOptionParameter *options,
-  Error **errp)
+static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int r, ret;
 int64_t total_size = 0;
@@ -697,12 +700,7 @@ static int ssh_create(const char *filename, 
QEMUOptionParameter *options,
 ssh_state_init(s);
 
 /* Get desired file size. */
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-total_size = options-value.n;
-}
-options++;
-}
+total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
 DPRINTF(total_size=% PRIi64, total_size);
 
 uri_options = qdict_new();
@@ -1075,14 +1073,14 @@ static BlockDriver bdrv_ssh = {
 .instance_size= sizeof(BDRVSSHState),
 .bdrv_parse_filename  = ssh_parse_filename,
 .bdrv_file_open   = ssh_file_open,
-.bdrv_create  = ssh_create,
+.bdrv_create2 = ssh_create,
 .bdrv_close   = ssh_close,
 .bdrv_has_zero_init   = ssh_has_zero_init,
 .bdrv_co_readv= ssh_co_readv,
 .bdrv_co_writev   = ssh_co_writev,
 .bdrv_getlength   = ssh_getlength,
 .bdrv_co_flush_to_disk= ssh_co_flush,
-.create_options   = ssh_create_options,
+.create_opts  = ssh_create_opts,
 };
 
 static void bdrv_ssh_init(void)
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 31/33] vpc.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/vpc.c | 62 +
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 2e25f57..8ebf424 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -738,12 +738,11 @@ static int create_fixed_disk(int fd, uint8_t *buf, 
int64_t total_size)
 return ret;
 }
 
-static int vpc_create(const char *filename, QEMUOptionParameter *options,
-  Error **errp)
+static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 uint8_t buf[1024];
 VHDFooter *footer = (VHDFooter *) buf;
-QEMUOptionParameter *disk_type_param;
+char *disk_type_param;
 int fd, i;
 uint16_t cyls = 0;
 uint8_t heads = 0;
@@ -754,16 +753,16 @@ static int vpc_create(const char *filename, 
QEMUOptionParameter *options,
 int ret = -EIO;
 
 /* Read out options */
-total_size = get_option_parameter(options, BLOCK_OPT_SIZE)-value.n;
-
-disk_type_param = get_option_parameter(options, BLOCK_OPT_SUBFMT);
-if (disk_type_param  disk_type_param-value.s) {
-if (!strcmp(disk_type_param-value.s, dynamic)) {
+total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+disk_type_param = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
+if (disk_type_param) {
+if (!strcmp(disk_type_param, dynamic)) {
 disk_type = VHD_DYNAMIC;
-} else if (!strcmp(disk_type_param-value.s, fixed)) {
+} else if (!strcmp(disk_type_param, fixed)) {
 disk_type = VHD_FIXED;
 } else {
-return -EINVAL;
+ret = -EINVAL;
+goto out;
 }
 } else {
 disk_type = VHD_DYNAMIC;
@@ -772,7 +771,8 @@ static int vpc_create(const char *filename, 
QEMUOptionParameter *options,
 /* Create the file */
 fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
 if (fd  0) {
-return -EIO;
+ret = -EIO;
+goto out;
 }
 
 /*
@@ -837,8 +837,10 @@ static int vpc_create(const char *filename, 
QEMUOptionParameter *options,
 ret = create_fixed_disk(fd, buf, total_size);
 }
 
- fail:
+fail:
 qemu_close(fd);
+out:
+g_free(disk_type_param);
 return ret;
 }
 
@@ -866,20 +868,24 @@ static void vpc_close(BlockDriverState *bs)
 error_free(s-migration_blocker);
 }
 
-static QEMUOptionParameter vpc_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{
-.name = BLOCK_OPT_SUBFMT,
-.type = OPT_STRING,
-.help =
-Type of virtual hard disk format. Supported formats are 
-{dynamic (default) | fixed} 
-},
-{ NULL }
+static QemuOptsList vpc_create_opts = {
+.name = vpc-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(vpc_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{
+.name = BLOCK_OPT_SUBFMT,
+.type = QEMU_OPT_STRING,
+.help =
+Type of virtual hard disk format. Supported formats are 
+{dynamic (default) | fixed} 
+},
+{ /* end of list */ }
+}
 };
 
 static BlockDriver bdrv_vpc = {
@@ -890,14 +896,14 @@ static BlockDriver bdrv_vpc = {
 .bdrv_open  = vpc_open,
 .bdrv_close = vpc_close,
 .bdrv_reopen_prepare= vpc_reopen_prepare,
-.bdrv_create= vpc_create,
+.bdrv_create2   = vpc_create,
 
 .bdrv_read  = vpc_co_read,
 .bdrv_write = vpc_co_write,
 
 .bdrv_get_info  = vpc_get_info,
 
-.create_options = vpc_create_options,
+.create_opts= vpc_create_opts,
 .bdrv_has_zero_init = vpc_has_zero_init,
 };
 
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 23/33] raw-win32.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/raw-win32.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index 064ea31..e5ac297 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -478,8 +478,7 @@ static int64_t raw_get_allocated_file_size(BlockDriverState 
*bs)
 return st.st_size;
 }
 
-static int raw_create(const char *filename, QEMUOptionParameter *options,
-  Error **errp)
+static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int fd;
 int64_t total_size = 0;
@@ -487,12 +486,8 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options,
 strstart(filename, file:, filename);
 
 /* Read out options */
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-total_size = options-value.n / 512;
-}
-options++;
-}
+total_size =
+qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
 
 fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
0644);
@@ -506,13 +501,18 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options,
 return 0;
 }
 
-static QEMUOptionParameter raw_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{ NULL }
+
+static QemuOptsList raw_create_opts = {
+.name = raw-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{ /* end of list */ }
+}
 };
 
 static BlockDriver bdrv_file = {
@@ -521,9 +521,9 @@ static BlockDriver bdrv_file = {
 .instance_size = sizeof(BDRVRawState),
 .bdrv_needs_filename = true,
 .bdrv_parse_filename = raw_parse_filename,
-.bdrv_file_open= raw_open,
-.bdrv_close= raw_close,
-.bdrv_create   = raw_create,
+.bdrv_file_open = raw_open,
+.bdrv_close = raw_close,
+.bdrv_create2   = raw_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 
 .bdrv_aio_readv = raw_aio_readv,
@@ -535,7 +535,7 @@ static BlockDriver bdrv_file = {
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
 
-.create_options = raw_create_options,
+.create_opts= raw_create_opts,
 };
 
 /***/
-- 
1.7.12.4




Re: [Qemu-devel] [PATCH] block/iscsi: handle BUSY condition

2014-06-05 Thread Peter Lieven

On 30.05.2014 11:05, Paolo Bonzini wrote:

Il 30/05/2014 11:04, Peter Lieven ha scritto:

Am 30.05.2014 07:50, schrieb Paolo Bonzini:

Il 29/05/2014 22:25, Peter Lieven ha scritto:

Am 29.05.2014 21:17, schrieb Paolo Bonzini:

Il 29/05/2014 20:41, Peter Lieven ha scritto:

static inline unsigned geo_rand_range(double m, double n) {
return exp((log(m) + (double)rand() /
   (RAND_MAX / (log(n) - log(m) + log(1)) + log(1;
}


Yes, exactly (except that log(1) is zero, and was only needed in your case 
because you were using integers).


Regarding aio_timer_new vs. aio_timer_init. Shall we really have the complete 
QEMU_TIMER struct in each
iTask? We need this retry_timer only in the really rare case of a BUSY.


Ok, makes sense.  Though the overall cost of IscsiTasks should be less than 4k 
per block device with a separate QemuTimer, and 8k if you embed it.

Otherwise the patch looks good, thanks!


Okay, when are you preparing the next pull request?


After the dataplane patches go in.  But you can send the patch now, I'll rebase.


It might be that we need a LIBS+= -lm to the Makefile in the root directory. I 
had strange problems when linking qemu-img/qemu-io/qemu-nbd.

Peter



[Qemu-devel] [PATCH v28 32/33] cleanup QEMUOptionParameter

2014-06-05 Thread Chunyan Liu
Now that all backend drivers are using QemuOpts, remove all
QEMUOptionParameter related codes.

Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block.c   |  93 ++
 block/cow.c   |   4 +-
 block/gluster.c   |   8 +-
 block/iscsi.c |   2 +-
 block/nfs.c   |   2 +-
 block/qcow.c  |   4 +-
 block/qcow2.c |   6 +-
 block/qed.c   |   4 +-
 block/raw-posix.c |  10 +-
 block/raw-win32.c |   2 +-
 block/raw_bsd.c   |   4 +-
 block/rbd.c   |   2 +-
 block/sheepdog.c  |   6 +-
 block/ssh.c   |   2 +-
 block/vdi.c   |   2 +-
 block/vhdx.c  |   4 +-
 block/vmdk.c  |   6 +-
 block/vpc.c   |   2 +-
 block/vvfat.c |  14 +-
 include/block/block.h |   8 +-
 include/block/block_int.h |  18 +-
 include/qemu/option.h |  48 +-
 qemu-img.c|  19 +--
 util/qemu-option.c| 427 +-
 24 files changed, 67 insertions(+), 630 deletions(-)

diff --git a/block.c b/block.c
index ebb76cf..3fc4598 100644
--- a/block.c
+++ b/block.c
@@ -328,13 +328,6 @@ void bdrv_register(BlockDriver *bdrv)
 }
 }
 
-if (bdrv-bdrv_create) {
-assert(!bdrv-bdrv_create2  !bdrv-create_opts);
-assert(!bdrv-bdrv_amend_options2);
-} else if (bdrv-bdrv_create2) {
-assert(!bdrv-bdrv_create  !bdrv-create_options);
-assert(!bdrv-bdrv_amend_options);
-}
 QLIST_INSERT_HEAD(bdrv_drivers, bdrv, list);
 }
 
@@ -429,7 +422,6 @@ BlockDriver *bdrv_find_whitelisted_format(const char 
*format_name,
 typedef struct CreateCo {
 BlockDriver *drv;
 char *filename;
-QEMUOptionParameter *options;
 QemuOpts *opts;
 int ret;
 Error *err;
@@ -442,28 +434,8 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
 
 CreateCo *cco = opaque;
 assert(cco-drv);
-assert(!(cco-options  cco-opts));
 
-if (cco-drv-bdrv_create2) {
-QemuOptsList *opts_list = NULL;
-if (cco-options) {
-opts_list = params_to_opts(cco-options);
-cco-opts = qemu_opts_create(opts_list, NULL, 0, error_abort);
-}
-ret = cco-drv-bdrv_create2(cco-filename, cco-opts, local_err);
-if (cco-options) {
-qemu_opts_del(cco-opts);
-qemu_opts_free(opts_list);
-}
-} else {
-if (cco-opts) {
-cco-options = opts_to_params(cco-opts);
-}
-ret = cco-drv-bdrv_create(cco-filename, cco-options, local_err);
-if (cco-opts) {
-free_option_parameters(cco-options);
-}
-}
+ret = cco-drv-bdrv_create(cco-filename, cco-opts, local_err);
 if (local_err) {
 error_propagate(cco-err, local_err);
 }
@@ -471,7 +443,6 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
 }
 
 int bdrv_create(BlockDriver *drv, const char* filename,
-QEMUOptionParameter *options,
 QemuOpts *opts, Error **errp)
 {
 int ret;
@@ -480,13 +451,12 @@ int bdrv_create(BlockDriver *drv, const char* filename,
 CreateCo cco = {
 .drv = drv,
 .filename = g_strdup(filename),
-.options = options,
 .opts = opts,
 .ret = NOT_DONE,
 .err = NULL,
 };
 
-if (!drv-bdrv_create  !drv-bdrv_create2) {
+if (!drv-bdrv_create) {
 error_setg(errp, Driver '%s' does not support image creation, 
drv-format_name);
 ret = -ENOTSUP;
 goto out;
@@ -517,8 +487,7 @@ out:
 return ret;
 }
 
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
- QemuOpts *opts, Error **errp)
+int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
 {
 BlockDriver *drv;
 Error *local_err = NULL;
@@ -530,7 +499,7 @@ int bdrv_create_file(const char* filename, 
QEMUOptionParameter *options,
 return -ENOENT;
 }
 
-ret = bdrv_create(drv, filename, options, opts, local_err);
+ret = bdrv_create(drv, filename, opts, local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 }
@@ -1275,7 +1244,6 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, int 
flags, Error **errp)
 char *tmp_filename = g_malloc0(PATH_MAX + 1);
 int64_t total_size;
 BlockDriver *bdrv_qcow2;
-QemuOptsList *create_opts = NULL;
 QemuOpts *opts = NULL;
 QDict *snapshot_options;
 BlockDriverState *bs_snapshot;
@@ -1301,20 +1269,11 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, 
int flags, Error **errp)
 }
 
 bdrv_qcow2 = bdrv_find_format(qcow2);
-
-assert(!(bdrv_qcow2-create_options  bdrv_qcow2-create_opts));
-if (bdrv_qcow2-create_options) {
-create_opts = params_to_opts(bdrv_qcow2-create_options);
-} else {
-create_opts = 

Re: [Qemu-devel] [PATCH v2 2/4] qtest: introduce qmp_exec_hmp_cmd()

2014-06-05 Thread Stefan Hajnoczi
On Thu, Jun 05, 2014 at 01:45:16PM +0800, Amos Kong wrote:
 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  tests/libqtest.c | 16 
  tests/libqtest.h |  8 
  2 files changed, 24 insertions(+)
 
 diff --git a/tests/libqtest.c b/tests/libqtest.c
 index 71468ac..ceb1734 100644
 --- a/tests/libqtest.c
 +++ b/tests/libqtest.c
 @@ -646,3 +646,19 @@ void qmp_discard_response(const char *fmt, ...)
  qtest_qmpv_discard_response(global_qtest, fmt, ap);
  va_end(ap);
  }
 +
 +void qmp_exec_hmp_cmd(const char *cmd, const char *expected_ret)
 +{
 +QDict *response;
 +const char *response_return;
 +
 +response = qmp({\execute\: \human-monitor-command\,
 +\arguments\: {
 +  \command-line\: \%s\
 +   }}, g_strescape(cmd, NULL));

You need to free the g_strescape() return value after using it.

 +g_assert(response);
 +response_return = qdict_get_try_str(response, return);
 +g_assert(response_return);
 +g_assert_cmpstr(response_return, ==, expected_ret);
 +QDECREF(response);
 +}
 diff --git a/tests/libqtest.h b/tests/libqtest.h
 index 8f323c7..e095df2 100644
 --- a/tests/libqtest.h
 +++ b/tests/libqtest.h
 @@ -375,6 +375,14 @@ QDict *qmp(const char *fmt, ...);
  void qmp_discard_response(const char *fmt, ...);
  
  /**
 + * qmp_exec_hmp_cmd:
 + * @fmt...: HMP command to execute

'fmt...' doesn't exist.  Please document cmd and expected_ret.

 + *
 + * Executes HMP command by 'human-monitor-command'.
 + */
 +void qmp_exec_hmp_cmd(const char *cmd, const char *expected_ret);
 +
 +/**
   * qmp_receive:
   *
   * Reads a QMP message from QEMU and returns the response.
 -- 
 1.9.3
 



[Qemu-devel] [PATCH v28 30/33] vmdk.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/vmdk.c | 123 ++-
 1 file changed, 63 insertions(+), 60 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 79d55b0..78d7312 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1695,17 +1695,16 @@ static int filename_decompose(const char *filename, 
char *path, char *prefix,
 return VMDK_OK;
 }
 
-static int vmdk_create(const char *filename, QEMUOptionParameter *options,
-   Error **errp)
+static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int idx = 0;
 BlockDriverState *new_bs = NULL;
 Error *local_err = NULL;
 char *desc = NULL;
 int64_t total_size = 0, filesize;
-const char *adapter_type = NULL;
-const char *backing_file = NULL;
-const char *fmt = NULL;
+char *adapter_type = NULL;
+char *backing_file = NULL;
+char *fmt = NULL;
 int flags = 0;
 int ret = 0;
 bool flat, split, compress;
@@ -1745,24 +1744,19 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options,
 goto exit;
 }
 /* Read out options */
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-total_size = options-value.n;
-} else if (!strcmp(options-name, BLOCK_OPT_ADAPTER_TYPE)) {
-adapter_type = options-value.s;
-} else if (!strcmp(options-name, BLOCK_OPT_BACKING_FILE)) {
-backing_file = options-value.s;
-} else if (!strcmp(options-name, BLOCK_OPT_COMPAT6)) {
-flags |= options-value.n ? BLOCK_FLAG_COMPAT6 : 0;
-} else if (!strcmp(options-name, BLOCK_OPT_SUBFMT)) {
-fmt = options-value.s;
-} else if (!strcmp(options-name, BLOCK_OPT_ZEROED_GRAIN)) {
-zeroed_grain |= options-value.n;
-}
-options++;
+total_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
+adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE);
+backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
+if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
+flags |= BLOCK_FLAG_COMPAT6;
+}
+fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
+if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) {
+zeroed_grain = true;
 }
+
 if (!adapter_type) {
-adapter_type = ide;
+adapter_type = g_strdup(ide);
 } else if (strcmp(adapter_type, ide) 
strcmp(adapter_type, buslogic) 
strcmp(adapter_type, lsilogic) 
@@ -1778,7 +1772,7 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options,
 }
 if (!fmt) {
 /* Default format to monolithicSparse */
-fmt = monolithicSparse;
+fmt = g_strdup(monolithicSparse);
 } else if (strcmp(fmt, monolithicFlat) 
strcmp(fmt, monolithicSparse) 
strcmp(fmt, twoGbMaxExtentSparse) 
@@ -1879,7 +1873,7 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options,
 if (!split  !flat) {
 desc_offset = 0x200;
 } else {
-ret = bdrv_create_file(filename, options, NULL, local_err);
+ret = bdrv_create_file(filename, NULL, opts, local_err);
 if (ret  0) {
 error_propagate(errp, local_err);
 goto exit;
@@ -1909,6 +1903,9 @@ exit:
 if (new_bs) {
 bdrv_unref(new_bs);
 }
+g_free(adapter_type);
+g_free(backing_file);
+g_free(fmt);
 g_free(desc);
 g_string_free(ext_desc_lines, true);
 return ret;
@@ -2096,41 +2093,47 @@ static int vmdk_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 return 0;
 }
 
-static QEMUOptionParameter vmdk_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{
-.name = BLOCK_OPT_ADAPTER_TYPE,
-.type = OPT_STRING,
-.help = Virtual adapter type, can be one of 
-ide (default), lsilogic, buslogic or legacyESX
-},
-{
-.name = BLOCK_OPT_BACKING_FILE,
-.type = OPT_STRING,
-.help = File name of a base image
-},
-{
-.name = BLOCK_OPT_COMPAT6,
-.type = OPT_FLAG,
-.help = VMDK version 6 image
-},
-{
-.name = BLOCK_OPT_SUBFMT,
-.type = OPT_STRING,
-.help =
-VMDK flat extent format, can be one of 
-{monolithicSparse (default) | monolithicFlat | 
twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} 
-},
-{
-.name = BLOCK_OPT_ZEROED_GRAIN,
-.type = OPT_FLAG,
-.help = Enable efficient zero writes using the zeroed-grain GTE 
feature
-},
-{ NULL }
+static QemuOptsList vmdk_create_opts = {
+.name = vmdk-create-opts,
+

Re: [Qemu-devel] [PATCH v2 3/4] virtio-blk-test.c: add hotplug subtest

2014-06-05 Thread Stefan Hajnoczi
On Thu, Jun 05, 2014 at 01:45:17PM +0800, Amos Kong wrote:
 This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
 devices to guest, and try to hot-unplug them.
 
 Note: the hot-unplug can't work without cooperation of guest OS.
 
 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  tests/virtio-blk-test.c | 38 ++
  1 file changed, 38 insertions(+)
 
 diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
 index 0fdec01..616599a 100644
 --- a/tests/virtio-blk-test.c
 +++ b/tests/virtio-blk-test.c
 @@ -7,11 +7,48 @@
   * See the COPYING file in the top-level directory.
   */
  
 +#include stdio.h
  #include glib.h
  #include string.h
  #include libqtest.h
  #include qemu/osdep.h
  
 +static void test_blk_hotplug(void)
 +{
 +char addr[6];
 +char cmd[100];
 +int i, j;
 +
 +/* start with no network/block device, slots 3~0x1f are free */
 +qtest_start(-net none);
 +
 +for (i = 3; i = 0x1f; i++) {
 +for (j = 7; j = 0; j--) {
 +sprintf(addr, %x.%x, i, j);
 +sprintf(cmd, drive_add 0 if=none,file=/dev/null,id=drv-%s, 
 addr);
 +qmp_exec_hmp_cmd(cmd, OK\r\n);
 +
 +sprintf(cmd, device_add virtio-blk-pci,id=dev-%s,drive=drv-%s,
 + addr=0x%s,multifunction=on, addr, addr, addr);
 +qmp_exec_hmp_cmd(cmd, );
 +}
 +}
 +
 +/* hot-unplug doesn't work without cooperation of guest OS */
 +for (i = 3; i = 0x1f; i++) {
 +for (j = 7; j = 0; j--) {
 +sprintf(addr, %x.%x, i, j);
 +sprintf(cmd, drive_del drv-%s, addr);
 +qmp_exec_hmp_cmd(cmd, );
 +
 +sprintf(cmd, device_del dev-%s, addr);
 +qmp_exec_hmp_cmd(cmd, );
 +}
 +}

All of this sprintf() usage makes me wonder about:
qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...);

This way callers don't need to manage formatting buffers themselves.

Besides that:
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com



Re: [Qemu-devel] [PATCH v2 4/4] qtest: use qmp_exec_hmp_cmd() in blockdev-test

2014-06-05 Thread Stefan Hajnoczi
On Thu, Jun 05, 2014 at 01:45:18PM +0800, Amos Kong wrote:
 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  tests/blockdev-test.c | 23 ++-
  1 file changed, 2 insertions(+), 21 deletions(-)

Reviewed-by: Stefan Hajnoczi stefa...@redhat.com



[Qemu-devel] [PATCH] linux-user: Tell guest about big host page sizes

2014-06-05 Thread Alexander Graf
We tell the guest its page size via AUX vectors. The guest process then uses
this page size as information on which boundaries it can mmap() things.

However, if the host has a bigger page size granularity than the guest, it can
not fulfill these mmap() requests - which falls apart when MAP_FIXED is passed
to mmap.

So in that case, let the guest know that we're running on a bigger page size
granularity than the target would require.

This fixes running qemu-ppc (TARGET_PAGE_SIZE=4k) on a 64k page size ppc64 host
for me.

Signed-off-by: Alexander Graf ag...@suse.de
---
 linux-user/elfload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 0af6292..127c565 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1545,7 +1545,7 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, 
int envc,
 NEW_AUX_ENT(AT_PHDR, (abi_ulong)(info-load_addr + exec-e_phoff));
 NEW_AUX_ENT(AT_PHENT, (abi_ulong)(sizeof (struct elf_phdr)));
 NEW_AUX_ENT(AT_PHNUM, (abi_ulong)(exec-e_phnum));
-NEW_AUX_ENT(AT_PAGESZ, (abi_ulong)(TARGET_PAGE_SIZE));
+NEW_AUX_ENT(AT_PAGESZ, (abi_ulong)(MAX(TARGET_PAGE_SIZE, getpagesize(;
 NEW_AUX_ENT(AT_BASE, (abi_ulong)(interp_info ? interp_info-load_addr : 
0));
 NEW_AUX_ENT(AT_FLAGS, (abi_ulong)0);
 NEW_AUX_ENT(AT_ENTRY, info-entry);
-- 
1.8.1.4




[Qemu-devel] [PATCH v28 02/33] QemuOpts: repurpose qemu_opts_print to replace print_option_parameters

2014-06-05 Thread Chunyan Liu
Currently this function is not used anywhere. In later patches, it will
replace print_option_parameters. To avoid print info changes, change
qemu_opts_print from fprintf stderr to printf, and remove last printf.

Signed-off-by: Chunyan Liu cy...@suse.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 include/qemu/option.h |  2 +-
 util/qemu-option.c| 10 --
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 8c0ac34..1077b69 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -156,7 +156,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
 void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp);
 
 typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void *opaque);
-int qemu_opts_print(QemuOpts *opts, void *dummy);
+void qemu_opts_print(QemuOpts *opts);
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void 
*opaque,
   int abort_on_failure);
 
diff --git a/util/qemu-option.c b/util/qemu-option.c
index c188c5c..2c46791 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -895,17 +895,15 @@ void qemu_opts_del(QemuOpts *opts)
 g_free(opts);
 }
 
-int qemu_opts_print(QemuOpts *opts, void *dummy)
+void qemu_opts_print(QemuOpts *opts)
 {
 QemuOpt *opt;
 
-fprintf(stderr, %s: %s:, opts-list-name,
-opts-id ? opts-id : noid);
+printf(%s: %s:, opts-list-name,
+   opts-id ? opts-id : noid);
 QTAILQ_FOREACH(opt, opts-head, next) {
-fprintf(stderr,  %s=\%s\, opt-name, opt-str);
+printf( %s=\%s\, opt-name, opt-str);
 }
-fprintf(stderr, \n);
-return 0;
 }
 
 static int opts_do_parse(QemuOpts *opts, const char *params,
-- 
1.7.12.4




Re: [Qemu-devel] [PATCH] block/iscsi: handle BUSY condition

2014-06-05 Thread Paolo Bonzini

Il 05/06/2014 11:29, Peter Lieven ha scritto:


It might be that we need a LIBS+= -lm to the Makefile in the root
directory. I had strange problems when linking qemu-img/qemu-io/qemu-nbd.


cat  $TMPC  EOF
#include math.h
int main(void) { return isnan(sin(0.0)); }
EOF
if compile_prog   ; then
  :
elif compile_prog  -lm ; then
  LIBS=-lm $LIBS
  libs_qga=-lm $libs_qga
else
  error_exit libm check failed
fi


Shouldn't be necessary, should it?

Paolo



Re: [Qemu-devel] [PATCH v4 1/4] cpus: Define callback for QEMU nmi command

2014-06-05 Thread Peter Maydell
On 5 June 2014 00:36, Alexey Kardashevskiy a...@ozlabs.ru wrote:
 I'd rather remove return value at all, my mistake, I cannot see how NMI
 handler can possibly fail - noone can mask it after all :) If callback
 exists, I just call it and that's it. If it does not exist, it is
 unsupported and that's it. Would that be ok?

You're assuming the 'emergency prod the guest' mechanism
in all CPUs will always be a can't possibly fail' one. You
need to either provide a failure code or have the documentation
specifically say this is a best-effort kind of command (the
latter would be fine I expect).

thanks
-- PMM



Re: [Qemu-devel] [PATCH 3/3] nbd: Shutdown socket before closing.

2014-06-05 Thread Hani Benhabiles
On Thu, Jun 05, 2014 at 03:55:40AM +0200, Paolo Bonzini wrote:
 Il 05/06/2014 00:33, Hani Benhabiles ha scritto:
  IIUC, what this does is ensure that the other side gets a FIN before it 
  gets
  a RST.  Is this correct?
 
 Yes. Without shutdown(), this could be reproduced (unreliably) on multiple
 tries. This is done in nbd_client_close() too, for the same reasons AFAICT.
 
 Actually, nbd_client_close() is different because it's an abortive close of
 the socket.  nbd_client_close() doesn't care about FIN vs. RST, it does the
 shutdown to force all the requests to fail (with either an error for writes,
 or a short read if they're receiving).  This will cause a flurry of
 nbd_client_put() calls soon after nbd_clint_close() returns, until the last
 reference is dropped and the socket is closed.
 

I see, thanks for the explanation.

 I'll apply the patch.

Will you apply it directly or should I resend it in v3 ?



[Qemu-devel] [PATCH v2] target-arm: Prepare cpreg writefns/readfns for EL3/SecExt

2014-06-05 Thread Fabian Aggeler
This patch changes some readfns/writefns to use raw_write
and raw_read functions, which use the fieldoffset specified
in ARMCPRegInfo instead of directly accessing the field.
This will simplify patches for EL3  Security Extensions.

Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Signed-off-by: Fabian Aggeler aggel...@ethz.ch
---
v1 - v2:
* corrected typo (wich  which)
* rebased

v1: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03415.html

 target-arm/helper.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index ec031f5..87c4f04 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -319,7 +319,7 @@ static void dacr_write(CPUARMState *env, const ARMCPRegInfo 
*ri, uint64_t value)
 {
 ARMCPU *cpu = arm_env_get_cpu(env);
 
-env-cp15.c3 = value;
+raw_write(env, ri, value);
 tlb_flush(CPU(cpu), 1); /* Flush TLB as domain not tracked in TLB */
 }
 
@@ -327,12 +327,12 @@ static void fcse_write(CPUARMState *env, const 
ARMCPRegInfo *ri, uint64_t value)
 {
 ARMCPU *cpu = arm_env_get_cpu(env);
 
-if (env-cp15.c13_fcse != value) {
+if (raw_read(env, ri) != value) {
 /* Unlike real hardware the qemu TLB uses virtual addresses,
  * not modified virtual addresses, so this causes a TLB flush.
  */
 tlb_flush(CPU(cpu), 1);
-env-cp15.c13_fcse = value;
+raw_write(env, ri, value);
 }
 }
 
@@ -341,7 +341,7 @@ static void contextidr_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 {
 ARMCPU *cpu = arm_env_get_cpu(env);
 
-if (env-cp15.contextidr_el1 != value  !arm_feature(env, ARM_FEATURE_MPU)
+if (raw_read(env, ri) != value  !arm_feature(env, ARM_FEATURE_MPU)
  !extended_addresses_enabled(env)) {
 /* For VMSA (when not using the LPAE long descriptor page table
  * format) this register includes the ASID, so do a TLB flush.
@@ -349,7 +349,7 @@ static void contextidr_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
  */
 tlb_flush(CPU(cpu), 1);
 }
-env-cp15.contextidr_el1 = value;
+raw_write(env, ri, value);
 }
 
 static void tlbiall_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -693,7 +693,7 @@ static uint64_t ccsidr_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
 static void csselr_write(CPUARMState *env, const ARMCPRegInfo *ri,
  uint64_t value)
 {
-env-cp15.c0_cssel = value  0xf;
+raw_write(env, ri, value  0xf);
 }
 
 static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri)
@@ -1216,11 +1216,11 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
 static void par_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 {
 if (arm_feature(env, ARM_FEATURE_LPAE)) {
-env-cp15.par_el1 = value;
+raw_write(env, ri, value);
 } else if (arm_feature(env, ARM_FEATURE_V7)) {
-env-cp15.par_el1 = value  0xf6ff;
+raw_write(env, ri, value  0xf6ff);
 } else {
-env-cp15.par_el1 = value  0xf1ff;
+raw_write(env, ri, value  0xf1ff);
 }
 }
 
@@ -1423,7 +1423,7 @@ static void vmsa_ttbcr_raw_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
  * for long-descriptor tables the TTBCR fields are used differently
  * and the c2_mask and c2_base_mask values are meaningless.
  */
-env-cp15.c2_control = value;
+raw_write(env, ri, value);
 env-cp15.c2_mask = ~(((uint32_t)0xu)  maskshift);
 env-cp15.c2_base_mask = ~((uint32_t)0x3fffu  maskshift);
 }
@@ -1445,7 +1445,7 @@ static void vmsa_ttbcr_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 static void vmsa_ttbcr_reset(CPUARMState *env, const ARMCPRegInfo *ri)
 {
 env-cp15.c2_base_mask = 0xc000u;
-env-cp15.c2_control = 0;
+raw_write(env, ri, 0);
 env-cp15.c2_mask = 0;
 }
 
@@ -1456,7 +1456,7 @@ static void vmsa_tcr_el1_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 
 /* For AArch64 the A1 bit could result in a change of ASID, so TLB flush. 
*/
 tlb_flush(CPU(cpu), 1);
-env-cp15.c2_control = value;
+raw_write(env, ri, value);
 }
 
 static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -2151,14 +2151,14 @@ static void sctlr_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 {
 ARMCPU *cpu = arm_env_get_cpu(env);
 
-if (env-cp15.c1_sys == value) {
+if (raw_read(env, ri) == value) {
 /* Skip the TLB flush if nothing actually changed; Linux likes
  * to do a lot of pointless SCTLR writes.
  */
 return;
 }
 
-env-cp15.c1_sys = value;
+raw_write(env, ri, value);
 /* ??? Lots of these bits are not implemented.  */
 /* This may enable/disable the MMU, so do a TLB flush.  */
 tlb_flush(CPU(cpu), 1);
-- 
1.8.3.2




Re: [Qemu-devel] [RFC 3/5] nbd: Use aio_set_fd_handler2()

2014-06-05 Thread Paolo Bonzini

Il 05/06/2014 10:12, Stefan Hajnoczi ha scritto:

On Wed, Jun 04, 2014 at 08:02:06PM +0200, Paolo Bonzini wrote:

Il 04/06/2014 14:37, Stefan Hajnoczi ha scritto:

Why is this design cleaner?  Because NBD code doesn't have to worry
about fd handlers.  It uses straightforward coroutine send/recv for
socket I/O inside nbd_read_req() and nbd_write_resp().  It's easy to see
that only one coroutine receives from the socket and that only one
coroutine writes to the socket.


I don't understand how this would work without managing fd handlers.


fd handlers still need to be managed, but not by NBD code.  They must be
managed by coroutine recv/send utility functions.  In other words, fd
handlers are used locally, not globally.

def co_recv(fd, buf):
while True:
nbytes = recv(fd, buf, len(buf))
if nbytes == -1:
if errno == EINTR:
continue
if errno == EAGAIN or errno == EWOULDBLOCK:
aio_set_fd_read_handler(fd, co_recv_cb)
qemu_coroutine_yield()
aio_set_fd_read_handler(fd, NULL)
continue
return nbytes

The send function is similar.


I see what you mean now---this however is not how qemu_co_recv and 
qemu_co_send work.  NBD uses them, but they require the caller to set up 
the handlers manually.


Paolo



Re: [Qemu-devel] [RFC 2/5] acpi: introduce TYPE_ACPI_DEVICE_IF interface

2014-06-05 Thread Igor Mammedov
On Fri, 30 May 2014 09:44:20 -0600
Eric Blake ebl...@redhat.com wrote:

 On 05/30/2014 09:39 AM, Igor Mammedov wrote:
 
  +# @source_event: Arg0 - An Integer containing the source event
  +#
  +# @status_code: Arg1 – An Integer containing the status code
 
 
  +{ 'type': 'ACPIOSTInfo',
  +  'data'  : { 'device': 'str',
  +  'source': 'int',
  +  'status': 'int',
  +  'slot': 'int' } }
 
  ...this type declaration.  I have no idea which one of the two is wrong.
  What do you mean under wrong?
 
 Sorry for not being clear enough.  I'm not sure whether you meant to
 document four fields (device, source, status, and slot) or whether the
 command should have been just two fields ( 'data': { 'source_event':
 'int', 'status_code': 'int' } ).
 
 Although re-reading what I just wrote, it appears your 'source' field
 matches the 'source_event' documentation, the 'status' field matches the
 'status_code' documentation, and you omitted the 'device' and 'slot'
 documentation.
Thanks, I'll fix it.

 
 And my question in 4/5 remains - should 'source' and/or 'status' be
 defined as an enum rather than an open-coded int?
Using enum for 'source' event, might be possible if we restrict ourselves
to a limit set of supported values and ignore the rest of
unknown/not implemented values. It might be not a good idea to lose
events because QEMU doesn't know about them.
Also from maintainability PoV it would add a bunch of mapping code
from 'int' into our enum, which would essentially prevent
new source events be exposed to users until QEMU adds support for
them.

For 'status' code it'd add a lot more mapping code to translate its
known values into enum since it's changing depending on 'source'.
Especially if it comes to expanding range of known values in
table 6-160 of ACPI5.0 spec.

I think it would be better to expose raw values the guest reported
via _OST and let management to pick-up ones it's interested in and
allow it to handle not implemented ones as it wishes rather than
hiding them at QEMU level.





[Qemu-devel] [PATCH v28 08/33] QemuOpts: add qemu_opts_print_help to replace print_option_help

2014-06-05 Thread Chunyan Liu
print_option_help takes QEMUOptionParameter as parameter, add
qemu_opts_print_help to take QemuOptsList as parameter for later
replace work.

Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Leandro Dorileo l...@dorileo.org
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 include/qemu/option.h |  1 +
 util/qemu-option.c| 13 +
 2 files changed, 14 insertions(+)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 6653e43..fbf5dc2 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -166,5 +166,6 @@ typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void 
*opaque);
 void qemu_opts_print(QemuOpts *opts);
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void 
*opaque,
   int abort_on_failure);
+void qemu_opts_print_help(QemuOptsList *list);
 
 #endif
diff --git a/util/qemu-option.c b/util/qemu-option.c
index a7330c6..cd03eb4 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -553,6 +553,19 @@ void print_option_help(QEMUOptionParameter *list)
 }
 }
 
+void qemu_opts_print_help(QemuOptsList *list)
+{
+QemuOptDesc *desc;
+
+assert(list);
+desc = list-desc;
+printf(Supported options:\n);
+while (desc  desc-name) {
+printf(%-16s %s\n, desc-name,
+   desc-help ? desc-help : No description available);
+desc++;
+}
+}
 /* -- */
 
 static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
-- 
1.7.12.4




Re: [Qemu-devel] [PATCH] block/iscsi: handle BUSY condition

2014-06-05 Thread Peter Lieven

On 05.06.2014 11:44, Paolo Bonzini wrote:

Il 05/06/2014 11:29, Peter Lieven ha scritto:


It might be that we need a LIBS+= -lm to the Makefile in the root
directory. I had strange problems when linking qemu-img/qemu-io/qemu-nbd.


cat  $TMPC  EOF
#include math.h
int main(void) { return isnan(sin(0.0)); }
EOF
if compile_prog   ; then
  :
elif compile_prog  -lm ; then
  LIBS=-lm $LIBS
  libs_qga=-lm $libs_qga
else
  error_exit libm check failed
fi


Shouldn't be necessary, should it?


I don't know actually. On my workstation it compiles, but on my build system I 
need to add this here

diff --git a/Makefile b/Makefile
index 423e373..69e1022 100644
--- a/Makefile
+++ b/Makefile
@@ -74,6 +74,10 @@ $(call set-vpath, $(SRC_PATH))

 LIBS+=-lz $(LIBS_TOOLS)

+ifndef CONFIG_HAIKU
+LIBS+=-lm
+endif
+
 HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)

 ifdef BUILD_DOCS

Otherwise I get unresolved references to log and exp...

I found that the above is only in /x86_64-softmmu/Makefile, but  not in 
/Makefile

Peter



[Qemu-devel] [PATCH v28 00/33] replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
This patch series is to replace QEMUOptionParameter with QemuOpts, so that only
one Qemu Option structure is kept in QEMU code.

---
Changes to v27:
  * rebase to git master (code has been updated in this part since v27 sent
so rebase to make the patch series work.)

All patches are also available from:
https://github.com/chunyanliu/qemu/commits/QemuOpts


Chunyan Liu (33):
  QemuOpts: move find_desc_by_name ahead for later calling
  QemuOpts: repurpose qemu_opts_print to replace
print_option_parameters
  QemuOpts: add def_value_str to QemuOptDesc
  qapi: output def_value_str when query command line options
  QemuOpts: change opt-name|str from (const char *) to (char *)
  QemuOpts: move qemu_opt_del ahead for later calling
  QemuOpts: add qemu_opt_get_*_del functions for replace work
  QemuOpts: add qemu_opts_print_help to replace print_option_help
  QemuOpts: add conversion between QEMUOptionParameter to QemuOpts
  QemuOpts: add qemu_opts_append to replace append_option_parameters
  QemuOpts: check NULL input for qemu_opts_del
  change block layer to support both QemuOpts and QEMUOptionParamter
  vvfat.c: handle cross_driver's create_options and create_opts
  cow.c: replace QEMUOptionParameter with QemuOpts
  gluster.c: replace QEMUOptionParameter with QemuOpts
  iscsi.c: replace QEMUOptionParameter with QemuOpts
  nfs.c: replace QEMUOptionParameter with QemuOpts
  qcow.c: replace QEMUOptionParameter with QemuOpts
  QemuOpts: export qemu_opt_find
  qcow2.c: replace QEMUOptionParameter with QemuOpts
  qed.c: replace QEMUOptionParameter with QemuOpts
  raw-posix.c: replace QEMUOptionParameter with QemuOpts
  raw-win32.c: replace QEMUOptionParameter with QemuOpts
  raw_bsd.c: replace QEMUOptionParameter with QemuOpts
  rbd.c: replace QEMUOptionParameter with QemuOpts
  sheepdog.c: replace QEMUOptionParameter with QemuOpts
  ssh.c: replace QEMUOptionParameter with QemuOpts
  vdi.c: replace QEMUOptionParameter with QemuOpts
  vhdx.c: replace QEMUOptionParameter with QemuOpts
  vmdk.c: replace QEMUOptionParameter with QemuOpts
  vpc.c: replace QEMUOptionParameter with QemuOpts
  cleanup QEMUOptionParameter
  QemuOpts: cleanup tmp 'allocated' member from QemuOptsList

 block.c|  99 
 block/cow.c|  52 ++--
 block/gluster.c|  73 +++---
 block/iscsi.c  |  32 ++-
 block/nfs.c|  10 +-
 block/qcow.c   |  72 +++---
 block/qcow2.c  | 259 ++--
 block/qed.c| 112 +
 block/qed.h|   3 +-
 block/raw-posix.c  |  55 ++---
 block/raw-win32.c  |  38 +--
 block/raw_bsd.c|  25 +-
 block/rbd.c|  61 +++--
 block/sheepdog.c   | 105 
 block/ssh.c|  30 ++-
 block/vdi.c|  71 +++---
 block/vhdx.c   |  97 
 block/vhdx.h   |   1 +
 block/vmdk.c   | 121 +-
 block/vpc.c|  60 ++---
 block/vvfat.c  |  14 +-
 include/block/block.h  |   7 +-
 include/block/block_int.h  |   9 +-
 include/qemu/option.h  |  53 +---
 include/qemu/option_int.h  |   4 +-
 qapi-schema.json   |   5 +-
 qapi/opts-visitor.c|  10 +-
 qemu-img.c |  91 ---
 qmp-commands.hx|   2 +
 tests/qemu-iotests/049.out |   2 +-
 tests/qemu-iotests/061.out |   2 +-
 util/qemu-config.c |   4 +
 util/qemu-option.c | 590 -
 33 files changed, 1037 insertions(+), 1132 deletions(-)

-- 
1.7.12.4




Re: [Qemu-devel] [PATCH 3/3] nbd: Shutdown socket before closing.

2014-06-05 Thread Paolo Bonzini

Il 05/06/2014 11:58, Hani Benhabiles ha scritto:

On Thu, Jun 05, 2014 at 03:55:40AM +0200, Paolo Bonzini wrote:

Il 05/06/2014 00:33, Hani Benhabiles ha scritto:

IIUC, what this does is ensure that the other side gets a FIN before it gets
a RST.  Is this correct?


Yes. Without shutdown(), this could be reproduced (unreliably) on multiple
tries. This is done in nbd_client_close() too, for the same reasons AFAICT.


Actually, nbd_client_close() is different because it's an abortive close of
the socket.  nbd_client_close() doesn't care about FIN vs. RST, it does the
shutdown to force all the requests to fail (with either an error for writes,
or a short read if they're receiving).  This will cause a flurry of
nbd_client_put() calls soon after nbd_clint_close() returns, until the last
reference is dropped and the socket is closed.



I see, thanks for the explanation.


I'll apply the patch.


Will you apply it directly or should I resend it in v3 ?



This is independent, so I can apply it first.

Paolo



Re: [Qemu-devel] [PATCH] scsi-disk.c: Fix compilation with -DDEBUG_SCSI

2014-06-05 Thread Paolo Bonzini

Il 05/06/2014 07:33, Paul Janzen ha scritto:

In scsi-disk.c, if you #define DEBUG_SCSI=1, you get:
hw/scsi/scsi-disk.c: In function 'scsi_disk_emulate_command':
hw/scsi/scsi-disk.c:2018: error: 'SCSIRequest' has no member named 'buf'

Change the debugging statement to match the actual value tested.

Signed-off-by: Paul Janzen p...@pauljanzen.org
---
 hw/scsi/scsi-disk.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 48a28ae..5ff4ae4 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2015,7 +2015,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest 
*req, uint8_t *buf)
 case VERIFY_10:
 case VERIFY_12:
 case VERIFY_16:
-DPRINTF(Verify (bytchk %lu)\n, (r-req.buf[1]  1)  3);
+DPRINTF(Verify (bytchk %d)\n, (req-cmd.buf[1]  1)  3);
 if (req-cmd.buf[1]  6) {
 goto illegal_request;
 }



Thanks, applied to scsi-next.

Paolo



[Qemu-devel] [PATCH v28 22/33] raw-posix.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/raw-posix.c | 59 +--
 1 file changed, 27 insertions(+), 32 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index b7f0f26..e0a75db 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1240,8 +1240,7 @@ static int64_t 
raw_get_allocated_file_size(BlockDriverState *bs)
 return (int64_t)st.st_blocks * 512;
 }
 
-static int raw_create(const char *filename, QEMUOptionParameter *options,
-  Error **errp)
+static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 int fd;
 int result = 0;
@@ -1250,12 +1249,8 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options,
 strstart(filename, file:, filename);
 
 /* Read out options */
-while (options  options-name) {
-if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
-total_size = options-value.n / BDRV_SECTOR_SIZE;
-}
-options++;
-}
+total_size =
+qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
 
 fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
0644);
@@ -1440,13 +1435,17 @@ static int raw_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 return 0;
 }
 
-static QEMUOptionParameter raw_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{ NULL }
+static QemuOptsList raw_create_opts = {
+.name = raw-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{ /* end of list */ }
+}
 };
 
 static BlockDriver bdrv_file = {
@@ -1461,7 +1460,7 @@ static BlockDriver bdrv_file = {
 .bdrv_reopen_commit = raw_reopen_commit,
 .bdrv_reopen_abort = raw_reopen_abort,
 .bdrv_close = raw_close,
-.bdrv_create = raw_create,
+.bdrv_create2 = raw_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 .bdrv_co_get_block_status = raw_co_get_block_status,
 .bdrv_co_write_zeroes = raw_co_write_zeroes,
@@ -1478,7 +1477,7 @@ static BlockDriver bdrv_file = {
 .bdrv_get_allocated_file_size
 = raw_get_allocated_file_size,
 
-.create_options = raw_create_options,
+.create_opts = raw_create_opts,
 };
 
 /***/
@@ -1799,7 +1798,7 @@ static coroutine_fn int 
hdev_co_write_zeroes(BlockDriverState *bs,
 return -ENOTSUP;
 }
 
-static int hdev_create(const char *filename, QEMUOptionParameter *options,
+static int hdev_create(const char *filename, QemuOpts *opts,
Error **errp)
 {
 int fd;
@@ -1820,12 +1819,8 @@ static int hdev_create(const char *filename, 
QEMUOptionParameter *options,
 (void)has_prefix;
 
 /* Read out options */
-while (options  options-name) {
-if (!strcmp(options-name, size)) {
-total_size = options-value.n / BDRV_SECTOR_SIZE;
-}
-options++;
-}
+total_size =
+qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / BDRV_SECTOR_SIZE;
 
 fd = qemu_open(filename, O_WRONLY | O_BINARY);
 if (fd  0) {
@@ -1862,8 +1857,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_reopen_prepare = raw_reopen_prepare,
 .bdrv_reopen_commit  = raw_reopen_commit,
 .bdrv_reopen_abort   = raw_reopen_abort,
-.bdrv_create= hdev_create,
-.create_options = raw_create_options,
+.bdrv_create2= hdev_create,
+.create_opts = raw_create_opts,
 .bdrv_co_write_zeroes = hdev_co_write_zeroes,
 
 .bdrv_aio_readv= raw_aio_readv,
@@ -2006,8 +2001,8 @@ static BlockDriver bdrv_host_floppy = {
 .bdrv_reopen_prepare = raw_reopen_prepare,
 .bdrv_reopen_commit  = raw_reopen_commit,
 .bdrv_reopen_abort   = raw_reopen_abort,
-.bdrv_create= hdev_create,
-.create_options = raw_create_options,
+.bdrv_create2= hdev_create,
+.create_opts = raw_create_opts,
 
 .bdrv_aio_readv = raw_aio_readv,
 .bdrv_aio_writev= raw_aio_writev,
@@ -2131,8 +2126,8 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_reopen_prepare = raw_reopen_prepare,
 .bdrv_reopen_commit  = raw_reopen_commit,
 .bdrv_reopen_abort   = raw_reopen_abort,
-.bdrv_create= hdev_create,
-.create_options = raw_create_options,
+.bdrv_create2= hdev_create,
+.create_opts = raw_create_opts,
 
 .bdrv_aio_readv = raw_aio_readv,
 .bdrv_aio_writev= raw_aio_writev,
@@ -2262,8 +2257,8 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_reopen_prepare = raw_reopen_prepare,
 .bdrv_reopen_commit  = 

[Qemu-devel] [PATCH v2] target-arm: implement PD0/PD1 bits for TTBCR

2014-06-05 Thread Fabian Aggeler
Corrected handling of writes to TTBCR for ARMv8 (previously UNK/SBZP
bits are not RES0) and ARMv7 (new bits PD0/PD1 for CPUs with Security
Extensions).

Bits PD0/PD1 are now respected in get_phys_addr_v6/v5() and
get_level1_table_address.

Signed-off-by: Fabian Aggeler aggel...@ethz.ch
---
v1 - v2:
* dropped changes in get_phys_addr_lpae()
* simplified get_level1_table_address as suggested
* fixed checkpath issues

v1: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg06590.html

 target-arm/cpu.h| 16 ++
 target-arm/helper.c | 60 ++---
 2 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 8d04385..b0e8b0c 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -430,6 +430,22 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, 
int rw,
 /* Execution state bits.  MRS read as zero, MSR writes ignored.  */
 #define CPSR_EXEC (CPSR_T | CPSR_IT | CPSR_J)
 
+#define TTBCR_N  (7U  0) /* TTBCR.EAE==0 */
+#define TTBCR_T0SZ   (7U  0) /* TTBCR.EAE==1 */
+#define TTBCR_PD0(1U  4)
+#define TTBCR_PD1(1U  5)
+#define TTBCR_EPD0   (1U  7)
+#define TTBCR_IRGN0  (3U  8)
+#define TTBCR_ORGN0  (3U  10)
+#define TTBCR_SH0(3U  12)
+#define TTBCR_T1SZ   (3U  16)
+#define TTBCR_A1 (1U  22)
+#define TTBCR_EPD1   (1U  23)
+#define TTBCR_IRGN1  (3U  24)
+#define TTBCR_ORGN1  (3U  26)
+#define TTBCR_SH1(1U  28)
+#define TTBCR_EAE(1U  31)
+
 /* Bit definitions for ARMv8 SPSR (PSTATE) format.
  * Only these are valid when in AArch64 mode; in
  * AArch32 mode SPSRs are basically CPSR-format.
diff --git a/target-arm/helper.c b/target-arm/helper.c
index ec031f5..0b2ebc3 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -312,7 +312,7 @@ static inline bool extended_addresses_enabled(CPUARMState 
*env)
 {
 return arm_el_is_aa64(env, 1)
 || ((arm_feature(env, ARM_FEATURE_LPAE)
-  (env-cp15.c2_control  (1U  31;
+  (env-cp15.c2_control  TTBCR_EAE)));
 }
 
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
value)
@@ -1413,11 +1413,22 @@ static void vmsa_ttbcr_raw_write(CPUARMState *env, 
const ARMCPRegInfo *ri,
 {
 int maskshift = extract32(value, 0, 3);
 
-if (arm_feature(env, ARM_FEATURE_LPAE)  (value  (1  31))) {
-value = ~((7  19) | (3  14) | (0xf  3));
-} else {
-value = 7;
+if (!arm_feature(env, ARM_FEATURE_V8)) {
+if (arm_feature(env, ARM_FEATURE_LPAE)  (value  TTBCR_EAE)) {
+/* Pre ARMv8 bits [21:19], [15:14] and [6:3] are UNK/SBZP when
+ * using Long-desciptor translation table format */
+value = ~((7  19) | (3  14) | (0xf  3));
+} else if (arm_feature(env, ARM_FEATURE_EL3)) {
+/* In an implementation that includes the Security Extensions
+ * TTBCR has additional fields PD0 [4] and PD1 [5] for
+ * Short-descriptor translation table format.
+ */
+value = TTBCR_PD1 | TTBCR_PD0 | TTBCR_N;
+} else {
+value = TTBCR_N;
+}
 }
+
 /* Note that we always calculate c2_mask and c2_base_mask, but
  * they are only used for short-descriptor tables (ie if EAE is 0);
  * for long-descriptor tables the TTBCR fields are used differently
@@ -3540,17 +3551,24 @@ static inline int check_ap(CPUARMState *env, int ap, 
int domain_prot,
   }
 }
 
-static uint32_t get_level1_table_address(CPUARMState *env, uint32_t address)
+static bool get_level1_table_address(CPUARMState *env, uint32_t *table,
+ uint32_t address)
 {
-uint32_t table;
-
-if (address  env-cp15.c2_mask)
-table = env-cp15.ttbr1_el1  0xc000;
-else
-table = env-cp15.ttbr0_el1  env-cp15.c2_base_mask;
-
-table |= (address  18)  0x3ffc;
-return table;
+if (address  env-cp15.c2_mask) {
+if ((env-cp15.c2_control  TTBCR_PD1)) {
+/* Translation table walk disabled for TTBR1 */
+return false;
+}
+*table = env-cp15.ttbr1_el1  0xc000;
+} else {
+if ((env-cp15.c2_control  TTBCR_PD0)) {
+/* Translation table walk disabled for TTBR0 */
+return false;
+}
+*table = env-cp15.ttbr0_el1  env-cp15.c2_base_mask;
+}
+*table |= (address  18)  0x3ffc;
+return true;
 }
 
 static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int 
access_type,
@@ -3569,7 +3587,11 @@ static int get_phys_addr_v5(CPUARMState *env, uint32_t 
address, int access_type,
 
 /* Pagetable walk.  */
 /* Lookup l1 descriptor.  */
-table = get_level1_table_address(env, address);
+if (!get_level1_table_address(env, table, address)) {
+/* Section translation fault if page walk is disabled by PD0 or PD1 */
+code = 5;
+goto do_fault;
+}
 desc = ldl_phys(cs-as, table);
 type = (desc 

[Qemu-devel] [PATCH v28 24/33] raw_bsd.c: replace QEMUOptionParameter with QemuOpts

2014-06-05 Thread Chunyan Liu
Reviewed-by: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
Signed-off-by: Chunyan Liu cy...@suse.com
---
 block/raw_bsd.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 9ae5fc2..ee797fd 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -29,13 +29,17 @@
 #include block/block_int.h
 #include qemu/option.h
 
-static QEMUOptionParameter raw_create_options[] = {
-{
-.name = BLOCK_OPT_SIZE,
-.type = OPT_SIZE,
-.help = Virtual disk size
-},
-{ 0 }
+static QemuOptsList raw_create_opts = {
+.name = raw-create-opts,
+.head = QTAILQ_HEAD_INITIALIZER(raw_create_opts.head),
+.desc = {
+{
+.name = BLOCK_OPT_SIZE,
+.type = QEMU_OPT_SIZE,
+.help = Virtual disk size
+},
+{ /* end of list */ }
+}
 };
 
 static int raw_reopen_prepare(BDRVReopenState *reopen_state,
@@ -139,13 +143,12 @@ static int raw_has_zero_init(BlockDriverState *bs)
 return bdrv_has_zero_init(bs-file);
 }
 
-static int raw_create(const char *filename, QEMUOptionParameter *options,
-  Error **errp)
+static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
 Error *local_err = NULL;
 int ret;
 
-ret = bdrv_create_file(filename, options, NULL, local_err);
+ret = bdrv_create_file(filename, NULL, opts, local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 }
@@ -177,7 +180,7 @@ static BlockDriver bdrv_raw = {
 .bdrv_reopen_prepare  = raw_reopen_prepare,
 .bdrv_open= raw_open,
 .bdrv_close   = raw_close,
-.bdrv_create  = raw_create,
+.bdrv_create2 = raw_create,
 .bdrv_co_readv= raw_co_readv,
 .bdrv_co_writev   = raw_co_writev,
 .bdrv_co_write_zeroes = raw_co_write_zeroes,
@@ -194,7 +197,7 @@ static BlockDriver bdrv_raw = {
 .bdrv_lock_medium = raw_lock_medium,
 .bdrv_ioctl   = raw_ioctl,
 .bdrv_aio_ioctl   = raw_aio_ioctl,
-.create_options   = raw_create_options[0],
+.create_opts  = raw_create_opts,
 .bdrv_has_zero_init   = raw_has_zero_init
 };
 
-- 
1.7.12.4




[Qemu-devel] [PATCH v28 33/33] QemuOpts: cleanup tmp 'allocated' member from QemuOptsList

2014-06-05 Thread Chunyan Liu
Now only qemu_opts_append uses 'allocated' to indicate free memory.
For this function only, we can also let result list's (const char *)
members point to input list's members, only if the input list has
longer lifetime than result list. In current code, that is true.
So, we can remove the 'allocated' member from QemuOptsList definition
to keep code clean.

Signed-off-by: Chunyan Liu cy...@suse.com
---
 include/qemu/option.h |  6 --
 util/qemu-option.c| 27 +++
 2 files changed, 3 insertions(+), 30 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 921eccd..59bea75 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -63,12 +63,6 @@ typedef struct QemuOptDesc {
 } QemuOptDesc;
 
 struct QemuOptsList {
-/* FIXME: Temp used for QEMUOptionParamter-QemuOpts conversion to
- * indicate the need to free memory. Will remove after all drivers
- * switch to QemuOpts.
- */
-bool allocated;
-
 const char *name;
 const char *implied_opt_name;
 bool merge_lists;  /* Merge multiple uses of option into a single list? */
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 8ac8111..836055a 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1077,26 +1077,13 @@ static size_t count_opts_list(QemuOptsList *list)
 
 void qemu_opts_free(QemuOptsList *list)
 {
-/* List members point to new malloced space and need to be freed.
- * FIXME:
- * Introduced for QEMUOptionParamter-QemuOpts conversion.
- * Will remove after all drivers switch to QemuOpts.
- */
-if (list  list-allocated) {
-QemuOptDesc *desc = list-desc;
-while (desc  desc-name) {
-g_free((char *)desc-name);
-g_free((char *)desc-help);
-g_free((char *)desc-def_value_str);
-desc++;
-}
-}
-
 g_free(list);
 }
 
 /* Realloc dst option list and append options from an option list (list)
  * to it. dst could be NULL or a malloced list.
+ * The lifetime of dst must be shorter than the input list because the
+ * QemuOptDesc-name, -help, and -def_value_str strings are shared.
  */
 QemuOptsList *qemu_opts_append(QemuOptsList *dst,
QemuOptsList *list)
@@ -1125,24 +1112,16 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst,
 dst-name = NULL;
 dst-implied_opt_name = NULL;
 QTAILQ_INIT(dst-head);
-dst-allocated = true;
 dst-merge_lists = false;
 }
 dst-desc[num_dst_opts].name = NULL;
 
-/* (const char *) members of result dst are malloced, need free. */
-assert(dst-allocated);
 /* append list-desc to dst-desc */
 if (list) {
 desc = list-desc;
 while (desc  desc-name) {
 if (find_desc_by_name(dst-desc, desc-name) == NULL) {
-dst-desc[num_dst_opts].name = g_strdup(desc-name);
-dst-desc[num_dst_opts].type = desc-type;
-dst-desc[num_dst_opts].help = g_strdup(desc-help);
-dst-desc[num_dst_opts].def_value_str =
- g_strdup(desc-def_value_str);
-num_dst_opts++;
+dst-desc[num_dst_opts++] = *desc;
 dst-desc[num_dst_opts].name = NULL;
 }
 desc++;
-- 
1.7.12.4




Re: [Qemu-devel] [PATCH] block/iscsi: use 16 byte CDBs only when necessary

2014-06-05 Thread Peter Lieven

On 05.06.2014 11:12, Michael Tokarev wrote:

04.06.2014 18:00, ronnie sahlberg wrote:

That would mean you get to use the 10 version of the cdb even for very
large devices (as long as the IO is for blocks at the beginning of the
device) and thus provide partial avoidance of this issue for those
large devices.

That may make some bugs ghosty, so to say.  Ie, if there's a bug in/with
16 version of a command, you'll hit it only when you actually try to access
a far area of a drive.  Which means you're unlikely to hit it while trying
to reproduce in a clean environment, even after using a large device.  Or,
the bug will be triggered at random, since data placement on the filesystem
is effectively (from user PoV) random.

To my taste it is better to make it a bit more deterministic.


Yes, that was my fear as well. We make the decision for the whole target
if 48bit adressing is needed, we use 16 Byte CDBs for all requests independend
of the LBA offset.

Peter



Thanks,

/mjt






Re: [Qemu-devel] [patch 2/3] add object_property_add_alias

2014-06-05 Thread Paolo Bonzini

Il 04/06/2014 19:52, mtosa...@redhat.com ha scritto:

Allowing addition of a link without keeping pointer-to-pointer.

Signed-off-by: Marcelo Tosatti mtosa...@redhat.com

---
 include/qom/object.h |   13 +
 qom/object.c |   73 +--
 2 files changed, 72 insertions(+), 14 deletions(-)

Index: qemu/include/qom/object.h
===
--- qemu.orig/include/qom/object.h  2014-06-02 23:12:51.875693325 -0300
+++ qemu/include/qom/object.h   2014-06-02 23:14:13.045432426 -0300
@@ -1073,6 +1073,19 @@
 } ObjectPropertyLinkFlags;

 /**
+ * object_property_add_alias:
+ * @obj: the object to add a property to
+ * @name: the name of the property
+ * @alias: the alias object
+ * @errp: if an error occurs, a pointer to an area to store the area
+ *
+ * Add a link under obj, named name, pointing to alias.
+ *
+ */
+void object_property_add_alias(Object *obj, const char *name,
+   Object *alias, Error **errp);
+
+/**
  * object_property_allow_set_link:
  *
  * The default implementation of the object_property_add_link() check()
Index: qemu/qom/object.c
===
--- qemu.orig/qom/object.c  2014-06-02 23:12:51.875693325 -0300
+++ qemu/qom/object.c   2014-06-02 23:14:13.046432423 -0300
@@ -1023,27 +1023,71 @@
 g_free(type);
 }

+typedef struct {
+Object *child;
+Object **childp;


These field names are ugly... not your fault, but perhaps

Object **linkp;
Object *alias_dest;

would be better.  It would also avoid the mistake below:


@@ -1096,7 +1140,7 @@
 {
 Error *local_err = NULL;
 LinkProperty *prop = opaque;
-Object **child = prop-child;
+Object **child = prop-childp;
 Object *old_target = *child;
 Object *new_target = NULL;
 char *path = NULL;


This is object_set_link_property.  It writes *child but not prop-child, 
and subsequent calls to object_get_link_property incorrect.


However, since a similar need arose recently in one of Peter 
Crosthwaite's patches, let's add a generic resolve mechanism.  I'll post 
a short series in a second, as soon as I finish testing it.


Paolo


@@ -1133,8 +1177,8 @@
 {
 LinkProperty *prop = opaque;

-if ((prop-flags  OBJ_PROP_LINK_UNREF_ON_RELEASE)  *prop-child) {
-object_unref(*prop-child);
+if ((prop-flags  OBJ_PROP_LINK_UNREF_ON_RELEASE)  prop-child) {
+object_unref(prop-child);
 }
 g_free(prop);
 }
@@ -1150,7 +1194,8 @@
 LinkProperty *prop = g_malloc(sizeof(*prop));
 gchar *full_type;

-prop-child = child;
+prop-childp = child;
+prop-child = *child;
 prop-check = check;
 prop-flags = flags;

@@ -1227,7 +1272,7 @@

 if (object_property_is_link(prop)) {
 LinkProperty *lprop = prop-opaque;
-return *lprop-child;
+return lprop-child;
 } else if (object_property_is_child(prop)) {
 return prop-opaque;
 } else {







Re: [Qemu-devel] [PATCH 1/3] mirror: Go through ready - complete process for 0 len image

2014-06-05 Thread Stefan Hajnoczi
On Thu, Jun 05, 2014 at 11:42:34AM +0800, Fam Zheng wrote:
 diff --git a/block/mirror.c b/block/mirror.c
 index 94c8661..2bef5f3 100644
 --- a/block/mirror.c
 +++ b/block/mirror.c
 @@ -324,9 +324,18 @@ static void coroutine_fn mirror_run(void *opaque)
  }
  
  s-common.len = bdrv_getlength(bs);
 -if (s-common.len = 0) {
 +if (s-common.len  0) {
  ret = s-common.len;
  goto immediate_exit;
 +} else if (s-common.len == 0) {
 +/* Report BLOCK_JOB_READY and wait for complete. */
 +block_job_ready(s-common);
 +s-synced = true;
 +while (!block_job_is_cancelled(s-common)  !s-should_complete) {
 +block_job_sleep_ns(s-common, QEMU_CLOCK_REALTIME, SLICE_TIME);

Please take a look at block_job_resume() and how it's used when
cancelling or completing block jobs.  There is no need to sleep, instead
we can yield until we get resumed.



Re: [Qemu-devel] [PATCH 2/3] qemu-iotests: Test BLOCK_JOB_READY event for 0Kb image active commit

2014-06-05 Thread Stefan Hajnoczi
On Thu, Jun 05, 2014 at 11:42:35AM +0800, Fam Zheng wrote:
 @@ -48,8 +49,11 @@ class ImageCommitTestCase(iotests.QMPTestCase):
  self.assert_qmp(event, 'data/device', 'drive0')
  self.assert_qmp(event, 'data/offset', self.image_len)
  self.assert_qmp(event, 'data/len', self.image_len)
 +if need_ready:
 +assert ready, Expceting BLOCK_JOB_COMPLETED event

s/Expceting/Expecting/

Please use TestCase.assert*() methods instead of the assert keyword:
self.assertTrue(ready, msg='Expecting BLOCK_JOB_COMPLETED event')

The assert keyword raises the built-in AssertionError, which may not be
supported by the unittest module:
https://docs.python.org/2/library/unittest.html



[Qemu-devel] [RFC PATCH 0/2] qom: custom link properties

2014-06-05 Thread Paolo Bonzini
Both Marcelo's rtc patches and Peter's MemoryRegion patches showed
the interest in having link properties with a custom representation
and/or a different way to store the property.

Such properties would still be links for the outside world, but
the internal implementation in QEMU would be different.

This series proposes a way to introduce such custom link properties,
using a new property callback resolve that generalizes the if/else
ladder in object_resolve_path_component.

The second patch shows how the mechanism could be used to add a simple
immutable alias link.  Example:

$ ./qom-list -s /tmp/m1 /machine
@rtc/  # this is an alias
i440fx/
fw_cfg/
icc-bridge/
unattached/
peripheral/
peripheral-anon/
...

$ ./qom-get -s /tmp/m1 /machine.rtc
/machine/unattached/device[8]  # later could become /machine/piix3/rtc
   # or /machine/ich9/rtc and so on

$ ./qom-get -s /tmp/m1 /machine/rtc.date
tm_sec: 58
tm_hour: 11
tm_mday: 5
tm_year: 114
tm_mon: 5
tm_min: 16

Paolo

Paolo Bonzini (2):
  qom: add a generic mechanism to resolve paths
  qom: add object_property_add_alias

 include/qom/object.h | 74 
 qom/object.c | 68 +--
 2 files changed, 123 insertions(+), 19 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH 2/2] qom: add object_property_add_alias

2014-06-05 Thread Paolo Bonzini
Similar to object_property_add_link, alias properties provide an
alternative, non-canonical path to an object.  In fact, external
observers cannot distinguish alias properties from other links.

Aliases differ from links in that they are immutable and typically
managed by the target of the alias  in order to add a link to itself
at a well-known location.  For example, a real-time clock device might
add a link to itself at /machine/rtc.  Such well-known locations can
then expose a standard set of properties that can be accessed via the
qom-get and qom-set commands.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 include/qom/object.h | 25 +
 qom/object.c | 13 +
 2 files changed, 38 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index f8ab845..fc80078 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1116,6 +1116,31 @@ Object *object_resolve_path_component(Object *parent, 
const gchar *part);
 void object_property_add_child(Object *obj, const char *name,
Object *child, Error **errp);
 
+/**
+ * object_property_add_alias:
+ * @obj: the object to add a property to
+ * @name: the name of the property
+ * @dest: the destination of the alias
+ * @errp: if an error occurs, a pointer to an area to store the area
+ *
+ * Similar to @object_property_add_link, alias properties provide an
+ * alternative, non-canonical path to an object.  In fact, external
+ * observers cannot distinguish alias properties from other links.
+ *
+ * Aliases differ from links in that they are immutable and typically
+ * managed by the target of the alias (@dest) in order to add a link to
+ * itself at a well-known location.  For example, a real-time clock device
+ * might add a link to itself at /machine/rtc.  Such well-known locations
+ * can then expose a standard set of properties that can be accessed via
+ * the qom-get and qom-set commands.
+ *
+ * @object_property_add_alias does not add a reference to @dest.
+ * It is @dest's responsibility to remove the alias in its
+ * @instance_finalize function.
+ */
+void object_property_add_alias(Object *obj, const char *name,
+   Object *dest, Error **errp);
+
 typedef enum {
 /* Unref the link pointer when the property is deleted */
 OBJ_PROP_LINK_UNREF_ON_RELEASE = 0x1,
diff --git a/qom/object.c b/qom/object.c
index fcdd0da..46d60c8 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1036,6 +1036,19 @@ out:
 g_free(type);
 }
 
+void object_property_add_alias(Object *obj, const char *name,
+   Object *dest, Error **errp)
+{
+gchar *type;
+
+type = g_strdup_printf(link%s, object_get_typename(OBJECT(dest)));
+
+object_property_add_full(obj, name, type, object_get_child_property, NULL,
+ object_resolve_child_property,
+ NULL, dest, errp);
+g_free(type);
+}
+
 void object_property_allow_set_link(Object *obj, const char *name,
 Object *val, Error **errp)
 {
-- 
1.8.3.1




[Qemu-devel] [PATCH 1/2] qom: add a generic mechanism to resolve paths

2014-06-05 Thread Paolo Bonzini
It may be desirable to have custom link properties that do more
than just store an object.  Even the addition of a check
function is not enough if setting the link has side effects
or if a non-standard reference counting is preferrable.

Avoid the assumption that the opaque field of a link is a
LinkProperty struct, by adding a generic resolve callback
to ObjectProperty.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 include/qom/object.h | 49 ++
 qom/object.c | 55 ++--
 2 files changed, 85 insertions(+), 19 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index a641dcd..f8ab845 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -304,6 +304,23 @@ typedef void (ObjectPropertyAccessor)(Object *obj,
   Error **errp);
 
 /**
+ * ObjectPropertyResolve:
+ * @obj: the object that owns the property
+ * @opaque: the opaque registered with the property
+ * @part: the name of the property
+ *
+ * If @path is the path that led to @obj, the function should
+ * return the Object corresponding to @path/@part.  If #NULL
+ * is returned, @path/@part is not a valid object path.
+ *
+ * The returned object can also be used as a starting point
+ * to resolve a relative path starting with @part.
+ */
+typedef Object *(ObjectPropertyResolve)(Object *obj,
+void *opaque,
+const char *part);
+
+/**
  * ObjectPropertyRelease:
  * @obj: the object that owns the property
  * @name: the name of the property
@@ -321,6 +338,7 @@ typedef struct ObjectProperty
 gchar *type;
 ObjectPropertyAccessor *get;
 ObjectPropertyAccessor *set;
+ObjectPropertyResolve *resolve;
 ObjectPropertyRelease *release;
 void *opaque;
 
@@ -769,6 +787,37 @@ void object_ref(Object *obj);
 void object_unref(Object *obj);
 
 /**
+ * object_property_add_full:
+ * @obj: the object to add a property to
+ * @name: the name of the property.  This can contain any character except for
+ *  a forward slash.  In general, you should use hyphens '-' instead of
+ *  underscores '_' when naming properties.
+ * @type: the type name of the property.  This namespace is pretty loosely
+ *   defined.  Sub namespaces are constructed by using a prefix and then
+ *   to angle brackets.  For instance, the type 'virtio-net-pci' in the
+ *   'link' namespace would be 'linkvirtio-net-pci'.
+ * @get: The getter to be called to read a property.  If this is NULL, then
+ *   the property cannot be read.
+ * @set: the setter to be called to write a property.  If this is NULL,
+ *   then the property cannot be written.
+ * @resolve: called when the property name is used as part of an object
+ *   path.  This is meant for cases when you want to have custom link
+ *   properties.  If it is NULL, the property name cannot be used as part
+ *   of a valid object path.
+ * @release: called when the property is removed from the object.  This is
+ *   meant to allow a property to free its opaque upon object
+ *   destruction.  This may be NULL.
+ * @opaque: an opaque pointer to pass to the callbacks for the property
+ * @errp: returns an error if this function fails
+ */
+void object_property_add_full(Object *obj, const char *name, const char *type,
+ ObjectPropertyAccessor *get,
+ ObjectPropertyAccessor *set,
+ ObjectPropertyResolve *resolve,
+ ObjectPropertyRelease *release,
+ void *opaque, Error **errp);
+
+/**
  * object_property_add:
  * @obj: the object to add a property to
  * @name: the name of the property.  This can contain any character except for
diff --git a/qom/object.c b/qom/object.c
index e42b254..fcdd0da 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -355,11 +355,6 @@ static inline bool object_property_is_child(ObjectProperty 
*prop)
 return strstart(prop-type, child, NULL);
 }
 
-static inline bool object_property_is_link(ObjectProperty *prop)
-{
-return strstart(prop-type, link, NULL);
-}
-
 static void object_property_del_all(Object *obj)
 {
 while (!QTAILQ_EMPTY(obj-properties)) {
@@ -727,9 +722,10 @@ void object_unref(Object *obj)
 }
 }
 
-void object_property_add(Object *obj, const char *name, const char *type,
+void object_property_add_full(Object *obj, const char *name, const char *type,
  ObjectPropertyAccessor *get,
  ObjectPropertyAccessor *set,
+ ObjectPropertyResolve *resolve,
  ObjectPropertyRelease *release,
  void *opaque, Error **errp)
 {
@@ -751,12 +747,23 @@ void object_property_add(Object *obj, const char *name, 
const char *type,
 
 prop-get = get;
 prop-set = set;
+prop-resolve = resolve;
 prop-release = release;
 

Re: [Qemu-devel] [PATCH 1/3] mirror: Go through ready - complete process for 0 len image

2014-06-05 Thread Paolo Bonzini

Il 05/06/2014 13:17, Stefan Hajnoczi ha scritto:

On Thu, Jun 05, 2014 at 11:42:34AM +0800, Fam Zheng wrote:

diff --git a/block/mirror.c b/block/mirror.c
index 94c8661..2bef5f3 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -324,9 +324,18 @@ static void coroutine_fn mirror_run(void *opaque)
 }

 s-common.len = bdrv_getlength(bs);
-if (s-common.len = 0) {
+if (s-common.len  0) {
 ret = s-common.len;
 goto immediate_exit;
+} else if (s-common.len == 0) {
+/* Report BLOCK_JOB_READY and wait for complete. */
+block_job_ready(s-common);
+s-synced = true;
+while (!block_job_is_cancelled(s-common)  !s-should_complete) {
+block_job_sleep_ns(s-common, QEMU_CLOCK_REALTIME, SLICE_TIME);


Please take a look at block_job_resume() and how it's used when
cancelling or completing block jobs.  There is no need to sleep, instead
we can yield until we get resumed.



You still need to set job-busy = false.  So I guess we need a new 
function block_job_yield.


Paolo



Re: [Qemu-devel] [PATCH 3/3] qemu-iotests: Test 0-length image for mirror

2014-06-05 Thread Stefan Hajnoczi
On Thu, Jun 05, 2014 at 11:42:36AM +0800, Fam Zheng wrote:
 +class TestSingleDriveZeroLength(TestSingleDrive):
 +def setUp(self):
 +TestSingleDrive.image_len = 0
 +return TestSingleDrive.setUp(self)

This is buggy since it assigns to TestSingleDrive.image_len.  It
modifies the class variable for everyone else!

I think we need something like this instead:

   class Foo(object):
  ... a = 1
  ... def test(self):
  ... return self.a
  ...
   Foo().a
  1
   Foo().test()
  1
   class Bar(Foo):
  ... a = 2
  ...
   Bar().a
  2
   Bar().test()
  2

Please also fix the previous patch.  It uses the same pattern.



[Qemu-devel] [PATCH v2 0/5] Extract QAPI block commands

2014-06-05 Thread Benoît Canet
in v2:
squash commits [Stefan]
Get rid of dependency on quorum maintainance series [Stefan]
Add proper GPL V2 license in headers [Eric]

Benoît Canet (5):
  qapi: Extract qapi/common.json definitions
  qapi: create two block related json modules
  qapi: Extract qapi/block-core.json definitions
  qapi: Extract qapi/block.json definitions
  qapi: Set QAPI descriptions files under the GPL V2 license

 qapi-schema.json | 1671 +-
 qapi/block-core.json | 1416 ++
 qapi/block.json  |  170 +
 qapi/common.json |   93 +++
 4 files changed, 1694 insertions(+), 1656 deletions(-)
 create mode 100644 qapi/block-core.json
 create mode 100644 qapi/block.json
 create mode 100644 qapi/common.json

-- 
1.9.1




[Qemu-devel] [PATCH v2 5/5] qapi: Set QAPI descriptions files under the GPL V2 license

2014-06-05 Thread Benoît Canet
Signed-off-by: Benoit Canet ben...@irqsave.net
---
 qapi-schema.json | 4 
 qapi/block-core.json | 4 
 qapi/block.json  | 4 
 qapi/common.json | 4 
 4 files changed, 16 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 14b498b..870d3f6 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1,5 +1,9 @@
 # -*- Mode: Python -*-
 #
+# This file is part of the QEMU project.
+#
+# This file is licensed under the GPL V2.
+#
 # QAPI Schema
 
 # QAPI common definitions
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7215e48..33bd93f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1,5 +1,9 @@
 # -*- Mode: Python -*-
 #
+# This file is part of the QEMU project.
+#
+# This file is licensed under the GPL V2.
+#
 # QAPI block core definitions (vm unrelated)
 
 # QAPI common definitions
diff --git a/qapi/block.json b/qapi/block.json
index 61c463a..b5cf325 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -1,5 +1,9 @@
 # -*- Mode: Python -*-
 #
+# This file is part of the QEMU project.
+#
+# This file is licensed under the GPL V2.
+#
 # QAPI block definitions (vm related)
 
 # QAPI block core definitions
diff --git a/qapi/common.json b/qapi/common.json
index 4e9a21f..6d8a308 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -1,5 +1,9 @@
 # -*- Mode: Python -*-
 #
+# This file is part of the QEMU project.
+#
+# This file is licensed under the GPL V2.
+#
 # QAPI common definitions
 
 ##
-- 
1.9.1




[Qemu-devel] [PATCH v2 1/5] qapi: Extract qapi/common.json definitions

2014-06-05 Thread Benoît Canet
Signed-off-by: Benoit Canet ben...@irqsave.net
---
 qapi-schema.json | 87 ++
 qapi/common.json | 89 
 2 files changed, 91 insertions(+), 85 deletions(-)
 create mode 100644 qapi/common.json

diff --git a/qapi-schema.json b/qapi-schema.json
index 7bc33ea..cc71b27 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2,32 +2,8 @@
 #
 # QAPI Schema
 
-##
-# @ErrorClass
-#
-# QEMU error classes
-#
-# @GenericError: this is used for errors that don't require a specific error
-#class. This should be the default case for most errors
-#
-# @CommandNotFound: the requested command has not been found
-#
-# @DeviceEncrypted: the requested operation can't be fulfilled because the
-#   selected device is encrypted
-#
-# @DeviceNotActive: a device has failed to be become active
-#
-# @DeviceNotFound: the requested device has not been found
-#
-# @KVMMissingCap: the requested operation can't be fulfilled because a
-# required KVM capability is missing
-#
-# Since: 1.2
-##
-{ 'enum': 'ErrorClass',
-  'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
-'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
-
+# QAPI common definitions
+{ 'include': 'qapi/common.json' }
 
 ##
 # LostTickPolicy:
@@ -134,43 +110,6 @@
 { 'command': 'query-name', 'returns': 'NameInfo' }
 
 ##
-# @VersionInfo:
-#
-# A description of QEMU's version.
-#
-# @qemu.major:  The major version of QEMU
-#
-# @qemu.minor:  The minor version of QEMU
-#
-# @qemu.micro:  The micro version of QEMU.  By current convention, a micro
-#   version of 50 signifies a development branch.  A micro version
-#   greater than or equal to 90 signifies a release candidate for
-#   the next minor version.  A micro version of less than 50
-#   signifies a stable release.
-#
-# @package: QEMU will always set this field to an empty string.  Downstream
-#   versions of QEMU should set this to a non-empty string.  The
-#   exact format depends on the downstream however it highly
-#   recommended that a unique name is used.
-#
-# Since: 0.14.0
-##
-{ 'type': 'VersionInfo',
-  'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
-   'package': 'str'} }
-
-##
-# @query-version:
-#
-# Returns the current version of QEMU.
-#
-# Returns:  A @VersionInfo object describing the current version of QEMU.
-#
-# Since: 0.14.0
-##
-{ 'command': 'query-version', 'returns': 'VersionInfo' }
-
-##
 # @KvmInfo:
 #
 # Information about support for KVM acceleration
@@ -584,28 +523,6 @@
   'returns': 'str' }
 
 ##
-# @CommandInfo:
-#
-# Information about a QMP command
-#
-# @name: The command name
-#
-# Since: 0.14.0
-##
-{ 'type': 'CommandInfo', 'data': {'name': 'str'} }
-
-##
-# @query-commands:
-#
-# Return a list of supported QMP commands by this server
-#
-# Returns: A list of @CommandInfo for all supported commands
-#
-# Since: 0.14.0
-##
-{ 'command': 'query-commands', 'returns': ['CommandInfo'] }
-
-##
 # @EventInfo:
 #
 # Information about a QMP event
diff --git a/qapi/common.json b/qapi/common.json
new file mode 100644
index 000..4e9a21f
--- /dev/null
+++ b/qapi/common.json
@@ -0,0 +1,89 @@
+# -*- Mode: Python -*-
+#
+# QAPI common definitions
+
+##
+# @ErrorClass
+#
+# QEMU error classes
+#
+# @GenericError: this is used for errors that don't require a specific error
+#class. This should be the default case for most errors
+#
+# @CommandNotFound: the requested command has not been found
+#
+# @DeviceEncrypted: the requested operation can't be fulfilled because the
+#   selected device is encrypted
+#
+# @DeviceNotActive: a device has failed to be become active
+#
+# @DeviceNotFound: the requested device has not been found
+#
+# @KVMMissingCap: the requested operation can't be fulfilled because a
+# required KVM capability is missing
+#
+# Since: 1.2
+##
+{ 'enum': 'ErrorClass',
+  'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
+'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
+
+##
+# @VersionInfo:
+#
+# A description of QEMU's version.
+#
+# @qemu.major:  The major version of QEMU
+#
+# @qemu.minor:  The minor version of QEMU
+#
+# @qemu.micro:  The micro version of QEMU.  By current convention, a micro
+#   version of 50 signifies a development branch.  A micro version
+#   greater than or equal to 90 signifies a release candidate for
+#   the next minor version.  A micro version of less than 50
+#   signifies a stable release.
+#
+# @package: QEMU will always set this field to an empty string.  Downstream
+#   versions of QEMU should set this to a non-empty string.  The
+#   exact format depends on the downstream however it highly
+#  

[Qemu-devel] [PATCH v2 4/5] qapi: Extract qapi/block.json definitions

2014-06-05 Thread Benoît Canet
Signed-off-by: Benoit Canet ben...@irqsave.net
---
 qapi-schema.json | 159 ---
 qapi/block.json  | 159 +++
 2 files changed, 159 insertions(+), 159 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index c888d23..14b498b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -32,40 +32,6 @@
 { 'enum': 'LostTickPolicy',
   'data': ['discard', 'delay', 'merge', 'slew' ] }
 
-##
-# BiosAtaTranslation:
-#
-# Policy that BIOS should use to interpret cylinder/head/sector
-# addresses.  Note that Bochs BIOS and SeaBIOS will not actually
-# translate logical CHS to physical; instead, they will use logical
-# block addressing.
-#
-# @auto: If cylinder/heads/sizes are passed, choose between none and LBA
-#depending on the size of the disk.  If they are not passed,
-#choose none if QEMU can guess that the disk had 16 or fewer
-#heads, large if QEMU can guess that the disk had 131072 or
-#fewer tracks across all heads (i.e. cylinders*heads131072),
-#otherwise LBA.
-#
-# @none: The physical disk geometry is equal to the logical geometry.
-#
-# @lba: Assume 63 sectors per track and one of 16, 32, 64, 128 or 255
-#   heads (if fewer than 255 are enough to cover the whole disk
-#   with 1024 cylinders/head).  The number of cylinders/head is
-#   then computed based on the number of sectors and heads.
-#
-# @large: The number of cylinders per head is scaled down to 1024
-# by correspondingly scaling up the number of heads.
-#
-# @rechs: Same as @large, but first convert a 16-head geometry to
-# 15-head, by proportionally scaling up the number of
-# cylinders/head.
-#
-# Since: 2.0
-##
-{ 'enum': 'BiosAtaTranslation',
-  'data': ['auto', 'none', 'lba', 'large', 'rechs']}
-
 # @add_client
 #
 # Allow client connections for VNC, Spice and socket based
@@ -1201,22 +1167,6 @@
 { 'command': 'balloon', 'data': {'value': 'int'} }
 
 ##
-# @BlockdevSnapshotInternal
-#
-# @device: the name of the device to generate the snapshot from
-#
-# @name: the name of the internal snapshot to be created
-#
-# Notes: In transaction, if @name is empty, or any snapshot matching @name
-#exists, the operation will fail. Only some image formats support it,
-#for example, qcow2, rbd, and sheepdog.
-#
-# Since: 1.7
-##
-{ 'type': 'BlockdevSnapshotInternal',
-  'data': { 'device': 'str', 'name': 'str' } }
-
-##
 # @Abort
 #
 # This action can be used to test transaction failure.
@@ -1263,53 +1213,6 @@
   'data': { 'actions': [ 'TransactionAction' ] } }
 
 ##
-# @blockdev-snapshot-internal-sync
-#
-# Synchronously take an internal snapshot of a block device, when the format
-# of the image used supports it.
-#
-# For the arguments, see the documentation of BlockdevSnapshotInternal.
-#
-# Returns: nothing on success
-#  If @device is not a valid block device, DeviceNotFound
-#  If any snapshot matching @name exists, or @name is empty,
-#  GenericError
-#  If the format of the image used does not support it,
-#  BlockFormatFeatureNotSupported
-#
-# Since 1.7
-##
-{ 'command': 'blockdev-snapshot-internal-sync',
-  'data': 'BlockdevSnapshotInternal' }
-
-##
-# @blockdev-snapshot-delete-internal-sync
-#
-# Synchronously delete an internal snapshot of a block device, when the format
-# of the image used support it. The snapshot is identified by name or id or
-# both. One of the name or id is required. Return SnapshotInfo for the
-# successfully deleted snapshot.
-#
-# @device: the name of the device to delete the snapshot from
-#
-# @id: optional the snapshot's ID to be deleted
-#
-# @name: optional the snapshot's name to be deleted
-#
-# Returns: SnapshotInfo on success
-#  If @device is not a valid block device, DeviceNotFound
-#  If snapshot not found, GenericError
-#  If the format of the image used does not support it,
-#  BlockFormatFeatureNotSupported
-#  If @id and @name are both not specified, GenericError
-#
-# Since 1.7
-##
-{ 'command': 'blockdev-snapshot-delete-internal-sync',
-  'data': { 'device': 'str', '*id': 'str', '*name': 'str'},
-  'returns': 'SnapshotInfo' }
-
-##
 # @human-monitor-command:
 #
 # Execute a command on the human monitor and return the output.
@@ -1553,25 +1456,6 @@
 { 'command': 'expire_password', 'data': {'protocol': 'str', 'time': 'str'} }
 
 ##
-# @eject:
-#
-# Ejects a device from a removable drive.
-#
-# @device:  The name of the device
-#
-# @force:   @optional If true, eject regardless of whether the drive is locked.
-#   If not specified, the default value is false.
-#
-# Returns:  Nothing on success
-#   If @device is not a valid block device, DeviceNotFound
-#
-# Notes:Ejecting a device will no media results in success
-#
-# Since: 0.14.0
-##
-{ 'command': 'eject', 'data': {'device': 'str', '*force': 'bool'} 

[Qemu-devel] [PATCH v2 2/5] qapi: create two block related json modules

2014-06-05 Thread Benoît Canet
qapi/block-core.json contains block definitions unrelated to emulation.

qapi/block.json is a superset of the previous and contains definitions related
to emulation.

The purpose of these extractions is to be able to hook qapi/block-core.json
generated code on qemu-nbd.

Signed-off-by: Benoit Canet ben...@irqsave.net
---
 qapi-schema.json | 3 +++
 qapi/block-core.json | 6 ++
 qapi/block.json  | 7 +++
 3 files changed, 16 insertions(+)
 create mode 100644 qapi/block-core.json
 create mode 100644 qapi/block.json

diff --git a/qapi-schema.json b/qapi-schema.json
index cc71b27..7d47f4d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -5,6 +5,9 @@
 # QAPI common definitions
 { 'include': 'qapi/common.json' }
 
+# QAPI block definitions
+{ 'include': 'qapi/block.json' }
+
 ##
 # LostTickPolicy:
 #
diff --git a/qapi/block-core.json b/qapi/block-core.json
new file mode 100644
index 000..11bd8c2
--- /dev/null
+++ b/qapi/block-core.json
@@ -0,0 +1,6 @@
+# -*- Mode: Python -*-
+#
+# QAPI block core definitions (vm unrelated)
+
+# QAPI common definitions
+{ 'include': 'common.json' }
diff --git a/qapi/block.json b/qapi/block.json
new file mode 100644
index 000..e2b882f
--- /dev/null
+++ b/qapi/block.json
@@ -0,0 +1,7 @@
+# -*- Mode: Python -*-
+#
+# QAPI block definitions (vm related)
+
+# QAPI block core definitions
+{ 'include': 'block-core.json' }
+
-- 
1.9.1




Re: [Qemu-devel] [PATCH V2 4/4] hw/machine: qemu machine opts as properties to QemuMachineState

2014-06-05 Thread Marcel Apfelbaum
On Mon, 2014-06-02 at 12:21 -0300, Eduardo Habkost wrote:
 On Sun, Jun 01, 2014 at 11:21:49AM +0300, Marcel Apfelbaum wrote:
  On Fri, 2014-05-30 at 16:25 -0300, Eduardo Habkost wrote:
   On Mon, May 26, 2014 at 03:40:58PM +0300, Marcel Apfelbaum wrote:
   [...]
+static void machine_initfn(Object *obj)
+{
+object_property_add_str(obj, accel,
+machine_get_accel, machine_set_accel, 
NULL);
+object_property_add_bool(obj, kernel_irqchip,
+ machine_get_kernel_irqchip,
+ machine_set_kernel_irqchip,
+ NULL);
   
   In the case of kernel_irqchip, the information contained in MachineState
   is not a superset of the information contained on
   qemu_get_machine_opts().
   
   See hw/ppc/{e500,spapr}.c. They use kernel_irqchip like this:
   
   bool irqchip_allowed = qemu_opt_get_bool(machine_opts,
   kernel_irqchip, true);
   bool irqchip_required = qemu_opt_get_bool(machine_opts,
 kernel_irqchip, false);
   
   if (irqchip_allowed) {
   dev = ppce500_init_mpic_kvm(params, irqs);
   }
   
   if (irqchip_required  !dev) {
   fprintf(stderr, %s: irqchip requested but unavailable\n,
   __func__);
   abort();
   }
   
   This means kernel_irqchip have three possible states: disabled, 
   required,
   and allowed.
  I already had a patch adding property_is_set to QMP, but was not accepted
  as there is no way yet to unset a property. (I can point you to the 
  series)
  
   
   This means that MachineState.kernel_irqchip is not usable by current
   code that uses the kernel_irqchip option. I suppose we plan to address
   this on MachineState, too, to not get stuck with a global
   qemu_get_machine_opts() forever?
  I completely agree with you and I already had a patch tackling it,
  based on property_is_set, but was no accepted yet, obviously.
  I was instructed to set the default value in the machine init function
  and some way (I don't remember now) to emulate  required/allowed.
 
 I don't see a need to change to the object model and API. Just add
 MachineState-specific properties/fields that are capable of representing
 the state we need.
 
 I see two simple solutions:
 
 * Two boolean properties: require-kernel-irqchip and
 disable-kernel-irqchip. The default being both set to false (meaning
 irqchip is enabled automatically if available). We may still have a
 third kernel_irqchip property for compatibility, that will change both
 require-kernel-irqchip and disable-kernel-irqchip at the same time when
 set.
 
 * A string kernel_irqchip property which accepts three values: on,
 off, and auto.
 
 Example of partial implementation of the first approach, below. I still
 didn't add the two extra properties, and just let the code access the
 require_kernel_irqchip and disable_kernel_irqchip fields directly.
 
 Note that this is on top of some other changes I have been experimenting
 with, changing the accelerator init functions to get MachineState as
 argument. Git tree containing all work in progress can be seen at:
 https://github.com/ehabkost/qemu-hacks/commits/work/machine-irqchip-tristate

Hi Eduardo, thanks for the example.

I would also chose with the first solution, but use {require, allowed}
instead of {required, disabled}, but in the end is the same logic.

Thanks,
Marcel

 
 diff --git a/hw/core/machine.c b/hw/core/machine.c
 index cbba679..0797bc1 100644
 --- a/hw/core/machine.c
 +++ b/hw/core/machine.c
 @@ -31,14 +31,15 @@ static bool machine_get_kernel_irqchip(Object *obj, 
 Error **errp)
  {
  MachineState *ms = MACHINE(obj);
  
 -return ms-kernel_irqchip;
 +return !ms-disable_kernel_irqchip;
  }
  
  static void machine_set_kernel_irqchip(Object *obj, bool value, Error 
 **errp)
  {
  MachineState *ms = MACHINE(obj);
  
 -ms-kernel_irqchip = value;
 +ms-require_kernel_irqchip = value;
 +ms-disable_kernel_irqchip = !value;
  }
  
  static void machine_get_kvm_shadow_mem(Object *obj, Visitor *v,
 diff --git a/include/hw/boards.h b/include/hw/boards.h
 index 0389933..4a2daee 100644
 --- a/include/hw/boards.h
 +++ b/include/hw/boards.h
 @@ -99,7 +99,8 @@ struct MachineState {
  /* public */
  
  char *accel;
 -bool kernel_irqchip;
 +bool require_kernel_irqchip;
 +bool disable_kernel_irqchip;
  int kvm_shadow_mem;
  char *dtb;
  char *dumpdtb;
 diff --git a/kvm-all.c b/kvm-all.c
 index d2f4d7f..120bf70 100644
 --- a/kvm-all.c
 +++ b/kvm-all.c
 @@ -1315,11 +1315,11 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState 
 *s, EventNotifier *n, int virq)
 false);
  }
  

Re: [Qemu-devel] [PATCH v4 10/10] block: add QAPI command to allow live backing file change

2014-06-05 Thread Benoît Canet
The Wednesday 04 Jun 2014 à 09:51:12 (-0400), Jeff Cody wrote :
 This allows a user to make a live change to the backing file recorded in
 an open image.
 
 The image file to modify can be specified 2 ways:
 
 1) image filename
 2) image node-name
 
 Note: this does not cause the backing file itself to be reopened; it
 merely changes the backing filename in the image file structure, and
 in internal BDS structures.
 
 It is the responsibility of the user to pass a filename string that
 can be resolved when the image chain is reopened, and the filename
 string is not validated.
 
 A good analogy for this command is that it is a live version of
 'qemu-img rebase -u', with respect to changing the backing file string.

If this command was triggering the reopen of the file and drive-mirror was 
accepting
a node-name argument we would have a way to manage tiered storage.

For example we could move one of the backing file in the chain from harddisk to
SSD by mirroring it and swapping the new one in place.

Best regards

Benoît

 
 Signed-off-by: Jeff Cody jc...@redhat.com
 ---
  blockdev.c   | 102 
 +++
  qapi-schema.json |  60 
  qmp-commands.hx  |  74 
  3 files changed, 236 insertions(+)
 
 diff --git a/blockdev.c b/blockdev.c
 index 23a76eb..61150b4 100644
 --- a/blockdev.c
 +++ b/blockdev.c
 @@ -2408,6 +2408,108 @@ void qmp_block_job_complete(const char *device, Error 
 **errp)
  block_job_complete(job, errp);
  }
  
 +void qmp_change_backing_file(const char *device,
 + bool has_image, const char *image,
 + bool has_image_node_name,
 + const char *image_node_name,
 + const char *backing_file,
 + Error **errp)
 +{
 +BlockDriverState *bs = NULL;
 +BlockDriverState *image_bs = NULL;
 +Error *local_err = NULL;
 +bool ro;
 +int open_flags;
 +int ret;
 +
 +/* validate argument combinations */
 +if (has_image  has_image_node_name) {
 +error_setg(errp, 'image' and 'image-node-name' 
 + are mutually exclusive);
 +return;
 +}
 +
 +/* find the top layer BDS of the chain */
 +bs = bdrv_find(device);
 +if (!bs) {
 +error_set(errp, QERR_DEVICE_NOT_FOUND, device);
 +return;
 +}
 +
 +if (has_image_node_name) {
 +image_bs = bdrv_lookup_bs(NULL, image_node_name, local_err);
 +if (local_err) {
 +error_propagate(errp, local_err);
 +return;
 +}
 +}
 +
 +if (has_image) {
 +if (!strcmp(bs-filename, image)) {
 +image_bs = bs;
 +} else {
 +image_bs = bdrv_find_backing_image(bs, image);
 +}
 +}
 +
 +if (!has_image  !has_image_node_name) {
 +image_bs = bs;
 +}
 +
 +if (!image_bs) {
 +error_setg(errp, image file not found);
 +return;
 +}
 +
 +if (bdrv_find_base(image_bs) == image_bs) {
 +error_setg(errp, not allowing backing file change on an image 
 + without a backing file);
 +return;
 +}
 +
 +/* even though we are not necessarily operating on bs, we need it to
 + * determine if block ops are currently prohibited on the chain */
 +if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_CHANGE, errp)) {
 +return;
 +}
 +
 +/* final sanity check */
 +if (!bdrv_chain_contains(bs, image_bs)) {
 +error_setg(errp, '%s' and image file are not in the same chain,
 +   device);
 +return;
 +}
 +
 +/* if not r/w, reopen to make r/w */
 +open_flags = image_bs-open_flags;
 +ro = bdrv_is_read_only(image_bs);
 +
 +if (ro) {
 +bdrv_reopen(image_bs, open_flags | BDRV_O_RDWR, local_err);
 +if (local_err) {
 +error_propagate(errp, local_err);
 +return;
 +}
 +}
 +
 +ret = bdrv_change_backing_file(image_bs, backing_file,
 +   image_bs-drv ? image_bs-drv-format_name : 
 );
 +
 +if (ret  0) {
 +error_setg_errno(errp, -ret, Could not change backing file to '%s',
 + backing_file);
 +/* don't exit here, so we can try to restore open flags if
 + * appropriate */
 +}
 +
 +if (ro) {
 +bdrv_reopen(image_bs, open_flags, local_err);
 +if (local_err) {
 +error_propagate(errp, local_err); /* will preserve prior errp */
 +}
 +}
 +}
 +
  void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
  {
  QmpOutputVisitor *ov = qmp_output_visitor_new();
 diff --git a/qapi-schema.json b/qapi-schema.json
 index 3400561..e57396b 100644
 --- a/qapi-schema.json
 +++ b/qapi-schema.json
 @@ -2092,6 +2092,66 @@
'returns': 'str' }
  
  ##
 +# 

Re: [Qemu-devel] [PATCH v8 1/4] sPAPR: Implement EEH RTAS calls

2014-06-05 Thread Alexander Graf


On 05.06.14 08:53, Gavin Shan wrote:

The emulation for EEH RTAS requests from guest isn't covered
by QEMU yet and the patch implements them.

The patch defines constants used by EEH RTAS calls and adds
callback sPAPRPHBClass::eeh_handler, which is going to be used
this way:

1. RTAS calls are received in spapr_pci.c, sanity check is done
there.
2. RTAS handlers handle what they can. If there is something it
cannot handle and sPAPRPHBClass::eeh_handler callback is defined,
it is called.
3. sPAPRPHBClass::eeh_handler is only implemented for VFIO now. It
does ioctl() to the IOMMU container fd to complete the call. Error
codes from that ioctl() are transferred back to the guest.

This adds 6 RTAS handlers, all defined in SPAPR specification:

1) ibm,set-eeh-option: disables/enables EEH on a device, removes PE from
stopped state;
2) ibm,get-config-addr-info2 - returns fabric configuration address (upper
PCI bridge or PHB if there is no bridge);
3) ibm,read-slot-reset-state2 - retrieve PE state;
4) ibm,set-slot-reset - issue PE reset;
5) ibm,configure-pe - configure PCI bridges in the affected PE;
6) ibm,slot-error-detail - retrieve EEH error log;

All calls use fabric configuration address (a.k.a. PE address) as a target
address except ibm,get-config-addr-info2 and one case (enable EEH on the
specified PCI function) for ibm,set-eeh-option.

Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
  hw/ppc/spapr_pci.c  | 248 
  include/hw/pci-host/spapr.h |   7 ++
  include/hw/ppc/spapr.h  |  33 ++
  3 files changed, 288 insertions(+)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index a9f307a..423e4ff 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -422,6 +422,241 @@ static void 
rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu,
  rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */
  }
  
+static int rtas_finish_eeh_request(sPAPRPHBState *sphb,

+   uint32_t req, uint32_t opt,
+   target_ulong rets)


The only thing I dislike about this patch is the name of this function. 
It doesn't finish the eeh request - it actually does it :).



Alex




Re: [Qemu-devel] [PATCH v8 3/4] VFIO: Introduce helper vfio_pci_container_ioctl()

2014-06-05 Thread Alexander Graf


On 05.06.14 08:53, Gavin Shan wrote:

The patch introduces helper function vfio_pci_container_ioctl() to
pass ioctl commands to the specified VFIO container that is identified
by IOMMU group id. On sPAPR platform, each container only has one
IOMMU group.

Signed-off-by: Gavin Shan gws...@linux.vnet.ibm.com
---
  hw/misc/vfio.c | 31 +++
  include/hw/misc/vfio.h |  2 ++
  2 files changed, 33 insertions(+)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 0796abf..999d97d 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -4310,3 +4310,34 @@ put_group_exit:
  
  return n;

  }
+
+int vfio_pci_container_ioctl(int iommu_group_id, int req, int opt)
+{
+VFIOGroup *group;
+int ret, fd = 0;
+
+/* Search container's fd */
+QLIST_FOREACH(group, group_list, next) {
+if (group-groupid == iommu_group_id) {
+fd = group-container ? group-container-fd : 0;
+break;
+}
+}
+
+if (fd = 0) {


fd 0 is a valid file descriptor.


Alex




Re: [Qemu-devel] [PATCH v8 0/4] EEH Support for VFIO PCI Device

2014-06-05 Thread Alexander Graf


On 05.06.14 08:53, Gavin Shan wrote:

The series of patches adds support EEH for VFIO PCI devices on sPAPR platform.
It requires corresponding host kernel support. Also, it is based on top of
Alexey's VFIO-for-sPAPR git repository.

QEMU:   git://github.com/aik/qemu.git (branch: vfio)
Kernel: git://github.com/aik/linux.git (branch: vfio)
Kernel: 
http://linuxppc.10917.n7.nabble.com/PATCH-v8-0-3-EEH-Support-for-VFIO-PCI-Device-td82940.html

The implementations notes are below. Please comment.


Looks pretty straight forward, but let's wait for Alex's comments too ;).


Alex




[Qemu-devel] [PATCH 3/3] virtio-blk: Treat read/write beyond end as invalid

2014-06-05 Thread Markus Armbruster
The block layer fails such reads and writes just fine.  However, they
then get treated like valid operations that fail: the error action
gets executed.  Unwanted; reporting the error to the guest is the only
sensible action.

Reject them before passing them to the block layer.  This bypasses the
error action and I/O accounting.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/block/virtio-blk.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 2c68d0d..0b38049 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -284,12 +284,19 @@ static void virtio_blk_handle_flush(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
 static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
  uint64_t sector, size_t size)
 {
+uint64_t nb_sectors = size  BDRV_SECTOR_BITS;
+uint64_t total_sectors;
+
 if (sector  dev-sector_mask) {
 return false;
 }
 if (size % dev-conf-logical_block_size) {
 return false;
 }
+bdrv_get_geometry(dev-bs, total_sectors);
+if (sector  total_sectors || nb_sectors  total_sectors - sector) {
+return false;
+}
 return true;
 }
 
-- 
1.9.3




[Qemu-devel] [PATCH 2/3] virtio-blk: Bypass error action and I/O accounting on invalid r/w

2014-06-05 Thread Markus Armbruster
When a device model's I/O operation fails, we execute the error
action.  This lets layers above QEMU implement thin provisioning, or
attempt to correct errors before they reach the guest.  But when the
I/O operation fails because its invalid, reporting the error to the
guest is the only sensible action.

If the guest's read or write asks for an invalid sector range, fail
the request right away, without considering the error action.  No
change with error action BDRV_ACTION_REPORT.

Furthermore, bypass I/O accounting, because we want to track only I/O
that actually reaches the block layer.

The next commit will extend invalid sector range to cover attempts
to read/write beyond the end of the medium.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/block/virtio-blk.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index f2b4dca..2c68d0d 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -300,15 +300,16 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
 
 sector = ldq_p(req-out-sector);
 
-bdrv_acct_start(req-dev-bs, req-acct, req-qiov.size, BDRV_ACCT_WRITE);
-
 trace_virtio_blk_handle_write(req, sector, req-qiov.size / 512);
 
 if (!virtio_blk_sect_range_ok(req-dev, sector, req-qiov.size)) {
-virtio_blk_rw_complete(req, -EIO);
+virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
+g_free(req);
 return;
 }
 
+bdrv_acct_start(req-dev-bs, req-acct, req-qiov.size, BDRV_ACCT_WRITE);
+
 if (mrb-num_writes == 32) {
 virtio_submit_multiwrite(req-dev-bs, mrb);
 }
@@ -330,14 +331,15 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req)
 
 sector = ldq_p(req-out-sector);
 
-bdrv_acct_start(req-dev-bs, req-acct, req-qiov.size, BDRV_ACCT_READ);
-
 trace_virtio_blk_handle_read(req, sector, req-qiov.size / 512);
 
 if (!virtio_blk_sect_range_ok(req-dev, sector, req-qiov.size)) {
-virtio_blk_rw_complete(req, -EIO);
+virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
+g_free(req);
 return;
 }
+
+bdrv_acct_start(req-dev-bs, req-acct, req-qiov.size, BDRV_ACCT_READ);
 bdrv_aio_readv(req-dev-bs, sector, req-qiov,
req-qiov.size / BDRV_SECTOR_SIZE,
virtio_blk_rw_complete, req);
-- 
1.9.3




[Qemu-devel] [PATCH 0/3] virtio-blk: Suppress error action on r/w beyond end

2014-06-05 Thread Markus Armbruster
When a device model's I/O operation fails, we execute the error
action.  This lets layers above QEMU implement thin provisioning, or
attempt to correct errors before they reach the guest.  But when the
I/O operation fails because its invalid, reporting the error to the
guest is the only sensible action.

This short series does exactly that for virtio-blk.  I intend to do
the same for IDE and SCSI.

Markus Armbruster (3):
  virtio-blk: Factor common checks out of virtio_blk_handle_read/write()
  virtio-blk: Bypass error action and I/O accounting on invalid r/w
  virtio-blk: Treat read/write beyond end as invalid

 hw/block/virtio-blk.c | 45 +
 1 file changed, 29 insertions(+), 16 deletions(-)

-- 
1.9.3




  1   2   3   4   5   >