Re: [Qemu-devel] [RFC PATCH V2 00/10] Qemu: Add live migration support for SRIOV NIC

[Michael S. Tsirkin]

On Fri, Dec 04, 2015 at 02:42:36PM +0800, Lan, Tianyu wrote:
> 
> On 12/2/2015 10:31 PM, Michael S. Tsirkin wrote:
> >>>We hope
> >>>to find a better way to make SRIOV NIC work in these cases and this is
> >>>worth to do since SRIOV NIC provides better network performance compared
> >>>with PV NIC.
> >If this is a performance optimization as the above implies,
> >you need to include some numbers, and document how did
> >you implement the switch and how did you measure the performance.
> >
> 
> OK. Some ideas of my patches come from paper "CompSC: Live Migration with
> Pass-through Devices".
> http://www.cl.cam.ac.uk/research/srg/netos/vee_2012/papers/p109.pdf
> 
> It compared performance data between the solution of switching PV and VF and
> VF migration.(Chapter 7: Discussion)
> 

I haven't read it, but I would like to note you can't rely on research
papers.  If you propose a patch to be merged you need to measure what is
its actual effect on modern linux at the end of 2015.

> >>>Current patches have some issues. I think we can find
> >>>solution for them andimprove them step by step.

Re: [Qemu-devel] Posible regressions around spapr-dr-connector property drc-connector[]

[Markus Armbruster]

David Gibson  writes:

> On Thu, Dec 03, 2015 at 04:30:31PM +0100, Markus Armbruster wrote:
>> 1. Before commit 94649d4 "spapr: Don't use QOM [*] syntax for DR
>>connectors", the indexes were small integers:
>> 
>>(qemu) info qom-tree
>>/machine (pseries-2.4-machine)
>>  /unattached (container)
>>[...]
>>/device[5] (spapr-pci-host-bridge)
>>  /p...@8002000.mmio[0] (qemu:memory-region)
>>  /p...@8002000.mmio-alias[0] (qemu:memory-region)
>>  /p...@8002000.io[0] (qemu:memory-region)
>>  /p...@8002000.io-alias[0] (qemu:memory-region)
>>  /pci.0 (PCI)
>>  /pci@8002000.iommu-root[0] (qemu:memory-region)
>>  /dr-connector[0] (spapr-dr-connector)
>>  /dr-connector[1] (spapr-dr-connector)
>>  /dr-connector[2] (spapr-dr-connector)
>>  [...]
>> 
>>Since then, they're big ones:
>> 
>>  /dr-connector[1073741824] (spapr-dr-connector)
>>  /dr-connector[1073741825] (spapr-dr-connector)
>>  /dr-connector[1073741826] (spapr-dr-connector)
>> 
>>The commit message doesn't quite spell out this change, and I'm
>>therefore double-checkint it's intentional.  Is it?
>
> Yes, it's intentional.  The small integers were arbitrarily allocated
> by the QOM magic [*] code, whereas the big integers are actually
> meaningful values (essentially the DRC's global ID for the dynamic
> reconfiguration hypervisor interfaces).

Good.

>> 2. Before commit 6c2f9a1 "qapi: Make output visitor return qnull()
>>instead of NULL", qom-get returned {}:
>> 
>>Since then, it returns null:
>> 
>>QMP> { "execute": "qom-get", "arguments": { "path": 
>> "/machine/unattached/device[5]/dr-connector[1073741950]", "property": "fdt" 
>> } }
>>{"return": null}
>> 
>>Does anyone care?
>
> Hm, I'm guessing this is a case where fdt is NULL internally.  Which I

Yes.

> think will happen before a device gets hotplugged into the DRC.  In
> that case null seems more correct to me than {}, since {} would also
> be what's shown for a present-but-empty device tree.

It was {} in 2.4.  Changing it to null so we can distingish "nothing"
from "empty" is an incompatible change.  May make sense anyway, but I
can't judge it.

Thanks!

Re: [Qemu-devel] [PATCH for-2.5 2/3] spapr_drc: Change value of property "fdt" from null back to {}

[Markus Armbruster]

Eric Blake  writes:

> On 12/03/2015 04:54 PM, David Gibson wrote:
>> On Thu, Dec 03, 2015 at 05:37:39PM +0100, Markus Armbruster wrote:
>>> prop_get_fdt() misuses the visitor API: when fdt is null, it doesn't
>>> visit anything.  object_property_get_qobject() happily
>>> object_property_get_qobject().  Amazingly, the latter survives the
>>> misuse.  Turns out we've papered over it long before prop_get_fdt()
>>> existed, in commit 1d10b44.
>>>
>>> However, commit 6c2f9a1 changed how we paper over it, and as a side
>>> effect changed qom-get's value from {} to null.  Change it right back
>>> by fixing the visitor misuse.
>>>
>>> Signed-off-by: Markus Armbruster 
>>> ---
>>>  hw/ppc/spapr_drc.c | 5 +
>>>  1 file changed, 5 insertions(+)
>>>
>
>> I'm not entirely convinced by this.  IIUC, this makes the output in
>> the case of NULL (i.e. missing) fdt identical to the output in the
>> case of an empty, valid fdt - in dtc syntax, this:
>>  / {
>>  };
>> 
>> Those are different cases from the point of view of the code which
>> actually uses the fdt, and for purposes of debugging it, I suspect we
>> want to expose that difference.
>
> Expressing null may be the right thing, but it should be a conscious
> decision, and not a side-effect of an unrelated patch.  This patch is
> just about avoiding a regression for 2.5, because outputting {} for both
> a missing fdt and an empty one was the behavior we had back in 2.4 (that
> is, we've already returned {} in at least one release, so it won't hurt
> to do it for one more).  For 2.6 we can revisit things to actually
> express what is wanted.

Yes.

>> I don't know what the QOMishly correct way of doing that is, though.
>> Can we somehow make the "fdt" property disappear entirely if fdt is
>> NULL?
>
> In qapi terms, if a variable is marked optional and has_FOO is false,
> then the variable disappears completely.

If I understand QOM correctly, we should be able to add the property
dynamically, so that it exists exactly when fdt is non-null.

>   But I'm not sure if that maps
> over to qom.  Maybe you do it by setting errp if drc->fdt is NULL, so
> that prop_get_fdt() only succeeds when there is something for it to
> return.

Works, but is it really an erroneous state or operation?  If not, a
special value seems more appropriate than an error.

>  Or maybe returning qnull() is right after all, but in that
> case, explicitly calling 'QObject *n = qnull(); visit_type_any(v, ,
> NULL, ) seems like a nicer way than relying on side effects of how
> the qmp output visitor behaves when nothing was visited.

I think we should have visit_none(), and attempting to retrieve a visits
value when you haven't visited anything should be an error.

Re: [Qemu-devel] [PATCH for-2.5] tcg: Increase the highwater reservation

[Aurelien Jarno]

On 2015-12-03 13:19, Aurelien Jarno wrote:
> On 2015-12-02 10:36, Richard Henderson wrote:
> > On 12/01/2015 08:32 AM, Aurelien Jarno wrote:
> > >On 2015-12-01 08:19, Richard Henderson wrote:
> > >>If there are a lot of guest memory ops in the TB, the amount of
> > >>code generated by tcg_out_tb_finalize could be well more than 1k.
> > >>In the short term, increase the reservation larger than any TB
> > >>seen in practice.
> > >>
> > >>Reported-by: Aurelien Jarno 
> > >>Signed-off-by: Richard Henderson 
> > >
> > >Thanks! I was testing the same patch locally after our discussion, and I
> > >confirm it fixes the issue.
> > 
> > Here's the proper patch for 2.6.  If you'd like to run it through your same
> > tests, please?
> 
> The patch looks fine to me, the only tiny nitpick I have is that you can
> return a bool instead of a hint.
> 
> I am currently testing it (which is basically running a glibc build and
> testsuite multiple time in a VM), I'll let you know the result by
> tomorrow. In the meantime, you can add:
> 
> Reviewed-by: Aurelien Jarno 

I haven't seen any crash so far, which is more time than it usually
needs to trigger the issue. So I guess we can consider that the patch is
correct. Therefore:

Tested-by: Aurelien Jarno 

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

[Qemu-devel] [PATCH] fixup! tests: Use proper functions types instead of void (*fn)

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 tests/libqtest.c | 5 +++--
 tests/libqtest.h | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index c52ceb2..fa314e1 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -760,14 +760,15 @@ void qtest_memread(QTestState *s, uint64_t addr, void 
*data, size_t size)
 g_strfreev(args);
 }
 
-void qtest_add_func(const char *str, GTestFunc fn)
+void qtest_add_func(const char *str, void (*fn)(void))
 {
 gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
 g_test_add_func(path, fn);
 g_free(path);
 }
 
-void qtest_add_data_func(const char *str, const void *data, GTestDataFunc fn)
+void qtest_add_data_func(const char *str, const void *data,
+ void (*fn)(const void *))
 {
 gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
 g_test_add_data_func(path, data, fn);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index d11b5a4..ebdd5bb 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -416,7 +416,7 @@ const char *qtest_get_arch(void);
  * The path is prefixed with the architecture under test, as
  * returned by qtest_get_arch().
  */
-void qtest_add_func(const char *str, GTestFunc fn);
+void qtest_add_func(const char *str, void (*fn)(void));
 
 /**
  * qtest_add_data_func:
@@ -428,7 +428,8 @@ void qtest_add_func(const char *str, GTestFunc fn);
  * The path is prefixed with the architecture under test, as
  * returned by qtest_get_arch().
  */
-void qtest_add_data_func(const char *str, const void *data, GTestDataFunc fn);
+void qtest_add_data_func(const char *str, const void *data,
+ void (*fn)(const void *));
 
 /**
  * qtest_add:
-- 
2.4.3

Re: [Qemu-devel] [PATCH] tests/vhost-user-test: Fix potential use-after-free

[David Gibson]

On Wed, Dec 02, 2015 at 05:36:49AM -0500, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
> > ae31fb5 "vhost-user-test: wrap server in TestServer struct" cleaned up
> > the handling of the test server in vhost-user-test.  Unfortunately it
> > introduced a subtle use-after-free if a race goes the wrong way.
> > 
> > When the server structure is freed inside test_server_free() the GThread
> > started earlier is still running inside g_main_loop_run().  That GMainLoop
> > still has handlers active which reference the server structure, so if those
> > trip before the program exits there's a use-after-free.
> > 
> > I've had difficulty reproducing this locally, but for some reason it seems
> > to trip every time on Travis builds - this has been breaking all my test
> > builds there, which is why I notced it.
> > 
> > This patch prevents the use after free.  Unfortunately it looks like there
> > are additional problems still breaking my Travis builds, but one problem
> > at a time.
> > 
> > Signed-off-by: David Gibson 
> 
> The fix is on the ML for a few days, see "vhost-user-test: fix chardriver 
> race"
> The last series of fixes is "[PATCH for-2.5 v4 0/4] vhost-user-test
> fixes"

Drat, wish I'd spotted it.  Oh well.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 1/5] vmxnet3: Change offsets of msi/msix pci capabilities

[Jason Wang]

On 12/02/2015 11:26 PM, Shmulik Ladkani wrote:
> Place device reported PCI capabilities at the same offsets as placed by
> the VMware virtual hardware: MSI at [84], MSI-X at [9c].
>
> For compatability, preserve old offsets using 'x-old-msi-offsets' toggle.
>
> Signed-off-by: Shmulik Ladkani 
> ---
>  hw/net/vmxnet3.c| 20 +---
>  include/hw/compat.h |  4 
>  2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 5e3a233..1985dcf 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -36,6 +36,16 @@
>  #define VMXNET3_MSIX_BAR_SIZE 0x2000
>  #define MIN_BUF_SIZE 60
>  
> +/* Compatability flags for migration */
> +#define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT 0
> +#define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS \
> +(1 << VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT)
> +
> +#define VMXNET3_MSI_OFFSET(s) \
> +((s)->compat_flags & VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS ? 0x50 : 0x84)
> +#define VMXNET3_MSIX_OFFSET(s) \
> +((s)->compat_flags & VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS ? 0 : 0x9c)
> +
>  #define VMXNET3_BAR0_IDX  (0)
>  #define VMXNET3_BAR1_IDX  (1)
>  #define VMXNET3_MSIX_BAR_IDX  (2)
> @@ -313,6 +323,9 @@ typedef struct {
>  MACAddr *mcast_list;
>  uint32_t mcast_list_len;
>  uint32_t mcast_list_buff_size; /* needed for live migration. */
> +
> +/* Compatability flags for migration */
> +uint32_t compat_flags;
>  } VMXNET3State;
>  
>  /* Interrupt management */
> @@ -2103,7 +2116,7 @@ vmxnet3_init_msix(VMXNET3State *s)
>  VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_TABLE,
>  >msix_bar,
>  VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA,
> -0);
> +VMXNET3_MSIX_OFFSET(s));
>  
>  if (0 > res) {
>  VMW_WRPRN("Failed to initialize MSI-X, error %d", res);
> @@ -2131,7 +2144,6 @@ vmxnet3_cleanup_msix(VMXNET3State *s)
>  }
>  }
>  
> -#define VMXNET3_MSI_OFFSET(0x50)
>  #define VMXNET3_USE_64BIT (true)
>  #define VMXNET3_PER_VECTOR_MASK   (false)
>  
> @@ -2141,7 +2153,7 @@ vmxnet3_init_msi(VMXNET3State *s)
>  PCIDevice *d = PCI_DEVICE(s);
>  int res;
>  
> -res = msi_init(d, VMXNET3_MSI_OFFSET, VMXNET3_MAX_NMSIX_INTRS,
> +res = msi_init(d, VMXNET3_MSI_OFFSET(s), VMXNET3_MAX_NMSIX_INTRS,
> VMXNET3_USE_64BIT, VMXNET3_PER_VECTOR_MASK);
>  if (0 > res) {
>  VMW_WRPRN("Failed to initialize MSI, error %d", res);
> @@ -2552,6 +2564,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
>  
>  static Property vmxnet3_properties[] = {
>  DEFINE_NIC_PROPERTIES(VMXNET3State, conf),
> +DEFINE_PROP_BIT("x-old-msi-offsets", VMXNET3State, compat_flags,
> +VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT, false),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index d0b1c4f..01e326d 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -18,6 +18,10 @@
>  .driver   = "virtio-pci",\
>  .property = "migrate-extra",\
>  .value= "off",\
> +},{\
> +.driver   = "vmxnet3",\
> +.property = "x-old-msi-offsets",\
> +.value= "on",\
>  },

To have a better bisection behavior, we'd better enable this and compat
it in patch 2.

>  
>  #define HW_COMPAT_2_3 \

Re: [Qemu-devel] [PATCH v2 4/5] vmxnet3: The vmxnet3 device is a PCIE endpoint

[Jason Wang]

On 12/02/2015 11:26 PM, Shmulik Ladkani wrote:
> Report the 'express endpoint' capability if on a PCIE bus.
>
> The 'x-disable-pcie' property is used for backwards compatability.
>
> Signed-off-by: Shmulik Ladkani 
> ---
>  hw/net/vmxnet3.c| 55 
> -
>  include/hw/compat.h |  4 
>  2 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index d007314..1d0fb66 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -40,7 +40,11 @@
>  #define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT 0
>  #define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS \
>  (1 << VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT)
> +#define VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT 1
> +#define VMXNET3_COMPAT_FLAG_DISABLE_PCIE \
> +(1 << VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT)
>  
> +#define VMXNET3_EXP_EP_OFFSET (0x48)
>  #define VMXNET3_MSI_OFFSET(s) \
>  ((s)->compat_flags & VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS ? 0x50 : 0x84)
>  #define VMXNET3_MSIX_OFFSET(s) \
> @@ -121,6 +125,7 @@
>  
>  typedef struct VMXNET3Class {
>  PCIDeviceClass parent_class;
> +DeviceRealize parent_dc_realize;
>  } VMXNET3Class;
>  
>  #define TYPE_VMXNET3 "vmxnet3"
> @@ -2257,6 +2262,10 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, 
> Error **errp)
>  
>  vmxnet3_net_init(s);
>  
> +if (pci_is_express(pci_dev) && pci_bus_is_express(pci_dev->bus)) {

Looks like pci_bus_is_express() has been checked in
pcie_endpoint_cap_init().

> +pcie_endpoint_cap_init(pci_dev, VMXNET3_EXP_EP_OFFSET);
> +}
> +
>  register_savevm(dev, "vmxnet3-msix", -1, 1,
>  vmxnet3_msix_save, vmxnet3_msix_load, s);
>  }
> @@ -2526,6 +2535,29 @@ static const VMStateInfo int_state_info = {
>  .put = vmxnet3_put_int_state
>  };
>  
> +static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
> +{
> +VMXNET3State *s = VMXNET3(opaque);
> +
> +return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
> +}
> +
> +static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
> +{
> +return !vmxnet3_vmstate_need_pcie_device(opaque);
> +}
> +
> +static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> +.name = "vmxnet3/pcie",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = vmxnet3_vmstate_need_pcie_device,
> +.fields = (VMStateField[]) {
> +VMSTATE_PCIE_DEVICE(parent_obj, VMXNET3State),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
>  static const VMStateDescription vmstate_vmxnet3 = {
>  .name = "vmxnet3",
>  .version_id = 1,
> @@ -2533,7 +2565,9 @@ static const VMStateDescription vmstate_vmxnet3 = {
>  .pre_save = vmxnet3_pre_save,
>  .post_load = vmxnet3_post_load,
>  .fields = (VMStateField[]) {
> -VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> +VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
> +vmxnet3_vmstate_test_pci_device, 0,
> +vmstate_pci_device, PCIDevice),
>  VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
>  VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
>  VMSTATE_BOOL(lro_supported, VMXNET3State),
> @@ -2568,6 +2602,7 @@ static const VMStateDescription vmstate_vmxnet3 = {
>  },
>  .subsections = (const VMStateDescription*[]) {
>  _vmxnet3_mcast_list,
> +_vmxnet3_pcie_device,
>  NULL
>  }
>  };
> @@ -2576,13 +2611,29 @@ static Property vmxnet3_properties[] = {
>  DEFINE_NIC_PROPERTIES(VMXNET3State, conf),   
>  DEFINE_PROP_BIT("x-old-msi-offsets", VMXNET3State, compat_flags,
>  VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT, false),
> +DEFINE_PROP_BIT("x-disable-pcie", VMXNET3State, compat_flags,
> +VMXNET3_COMPAT_FLAG_DISABLE_PCIE_BIT, false),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> +static void vmxnet3_realize(DeviceState *qdev, Error **errp)
> +{
> +VMXNET3Class *vc = VMXNET3_DEVICE_GET_CLASS(qdev);
> +PCIDevice *pci_dev = PCI_DEVICE(qdev);
> +VMXNET3State *s = VMXNET3(qdev);
> +
> +if (!(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE)) {
> +pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
> +}
> +
> +vc->parent_dc_realize(qdev, errp);
> +}

It's not clear that how the class helps here. Why not simply do
everthing in vmxnet3_pci_realize()?

> +
>  static void vmxnet3_class_init(ObjectClass *class, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(class);
>  PCIDeviceClass *c = PCI_DEVICE_CLASS(class);
> +VMXNET3Class *vc = VMXNET3_DEVICE_CLASS(class);
>  
>  c->realize = vmxnet3_pci_realize;
>  c->exit = vmxnet3_pci_uninit;
> @@ -2592,6 +2643,8 @@ static void vmxnet3_class_init(ObjectClass *class, void 
> *data)
>  c->class_id = PCI_CLASS_NETWORK_ETHERNET;
>  c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE;
>

[Qemu-devel] [PATCH 2/2] net/vmxnet3: remove redundant VMW_SHPRN(...) definition

[Miao Yan]

Macro VMW_SHPRN(...) is already defined vmxnet3_debug.h,
so remove the duplication

Signed-off-by: Miao Yan 
---
 hw/net/vmware_utils.h | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/net/vmware_utils.h b/hw/net/vmware_utils.h
index 1099df6..c2c2f90 100644
--- a/hw/net/vmware_utils.h
+++ b/hw/net/vmware_utils.h
@@ -18,10 +18,7 @@
 #define VMWARE_UTILS_H
 
 #include "qemu/range.h"
-
-#ifndef VMW_SHPRN
-#define VMW_SHPRN(fmt, ...) do {} while (0)
-#endif
+#include "vmxnet_debug.h"
 
 /*
  * Shared memory access functions with byte swap support
-- 
1.9.1

[Qemu-devel] [PATCH 1/2] net/vmxnet3: fix debug macro pattern for vmxnet3

[Miao Yan]

Vmxnet3 uses the following debug macro style:

 #ifdef SOME_DEBUG
 #  define debug(...) do{ printf(...); } while (0)
 # else
 #  define debug(...) do{ } while (0)
 #endif

If SOME_DEBUG is undefined, then format string inside the
debug macro will never be checked by compiler. Code will
likely to break in the future when SOME_DEBUG is enabled
 because of lacking of testing. This patch changes this
to the following:

 #define debug(...) \
  do { if (SOME_DEBUG_ENABLED) printf(...); } while (0)

Signed-off-by: Miao Yan 
---
 hw/net/vmxnet_debug.h | 139 +++---
 1 file changed, 86 insertions(+), 53 deletions(-)

diff --git a/hw/net/vmxnet_debug.h b/hw/net/vmxnet_debug.h
index 96dae0f..96495db 100644
--- a/hw/net/vmxnet_debug.h
+++ b/hw/net/vmxnet_debug.h
@@ -20,94 +20,127 @@
 
 #define VMXNET_DEVICE_NAME "vmxnet3"
 
-/* #define VMXNET_DEBUG_CB */
 #define VMXNET_DEBUG_WARNINGS
 #define VMXNET_DEBUG_ERRORS
-/* #define VMXNET_DEBUG_INTERRUPTS */
-/* #define VMXNET_DEBUG_CONFIG */
-/* #define VMXNET_DEBUG_RINGS */
-/* #define VMXNET_DEBUG_PACKETS */
-/* #define VMXNET_DEBUG_SHMEM_ACCESS */
+
+#undef VMXNET_DEBUG_CB
+#undef VMXNET_DEBUG_INTERRUPTS
+#undef VMXNET_DEBUG_CONFIG
+#undef VMXNET_DEBUG_RINGS
+#undef VMXNET_DEBUG_PACKETS
+#undef VMXNET_DEBUG_SHMEM_ACCESS
+
+#ifdef VMXNET_DEBUG_CB
+#  define VMXNET_DEBUG_CB_ENABLED 1
+#else
+#  define VMXNET_DEBUG_CB_ENABLED 0
+#endif
+
+#ifdef VMXNET_DEBUG_WARNINGS
+#  define VMXNET_DEBUG_WARNINGS_ENABLED 1
+#else
+#  define VMXNET_DEBUG_WARNINGS_ENABLED 0
+#endif
+
+#ifdef VMXNET_DEBUG_ERRORS
+#  define VMXNET_DEBUG_ERRORS_ENABLED 1
+#else
+#  define VMXNET_DEBUG_ERRORS_ENABLED 0
+#endif
+
+#ifdef VMXNET_DEBUG_CONFIG
+#  define VMXNET_DEBUG_CONFIG_ENABLED 1
+#else
+#  define VMXNET_DEBUG_CONFIG_ENABLED 0
+#endif
+
+#ifdef VMXNET_DEBUG_RINGS
+#  define VMXNET_DEBUG_RINGS_ENABLED 1
+#else
+#  define VMXNET_DEBUG_RINGS_ENABLED 0
+#endif
+
+#ifdef VMXNET_DEBUG_PACKETS
+#  define VMXNET_DEBUG_PACKETS_ENABLED 1
+#else
+#  define VMXNET_DEBUG_PACKETS_ENABLED 0
+#endif
+
+#ifdef VMXNET_DEBUG_INTERRUPTS
+#  define VMXNET_DEBUG_INTERRUPTS_ENABLED 1
+#else
+#  define VMXNET_DEBUG_INTERRUPTS_ENABLED 0
+#endif
 
 #ifdef VMXNET_DEBUG_SHMEM_ACCESS
+#  define VMXNET_DEBUG_SHMEM_ACCESS_ENABLED 1
+#else
+#  define VMXNET_DEBUG_SHMEM_ACCESS_ENABLED 0
+#endif
+
 #define VMW_SHPRN(fmt, ...)   \
 do {  \
-printf("[%s][SH][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__,   \
-## __VA_ARGS__);  \
+if (VMXNET_DEBUG_SHMEM_ACCESS_ENABLED) {  \
+printf("[%s][SH][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__,   \
+## __VA_ARGS__);  \
+   }  \
 } while (0)
-#else
-#define VMW_SHPRN(fmt, ...) do {} while (0)
-#endif
 
-#ifdef VMXNET_DEBUG_CB
 #define VMW_CBPRN(fmt, ...)   \
 do {  \
-printf("[%s][CB][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__,   \
-## __VA_ARGS__);  \
+if (VMXNET_DEBUG_CB_ENABLED) {\
+printf("[%s][CB][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__,   \
+## __VA_ARGS__);  \
+} \
 } while (0)
-#else
-#define VMW_CBPRN(fmt, ...) do {} while (0)
-#endif
 
-#ifdef VMXNET_DEBUG_PACKETS
 #define VMW_PKPRN(fmt, ...)   \
 do {  \
-printf("[%s][PK][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__,   \
-## __VA_ARGS__);  \
+if (VMXNET_DEBUG_PACKETS_ENABLED) {   \
+printf("[%s][PK][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__,   \
+## __VA_ARGS__);  \
+} \
 } while (0)
-#else
-#define VMW_PKPRN(fmt, ...) do {} while (0)
-#endif
 
-#ifdef VMXNET_DEBUG_WARNINGS
 #define VMW_WRPRN(fmt, ...)   \
 do {  \
-printf("[%s][WR][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__,   \
-## __VA_ARGS__);  \
+if 

Re: [Qemu-devel] Posible regressions around spapr-dr-connector property drc-connector[]

[Markus Armbruster]

David Gibson  writes:

> On Fri, Dec 04, 2015 at 09:05:51AM +0100, Markus Armbruster wrote:
>> David Gibson  writes:
>> 
>> > On Thu, Dec 03, 2015 at 04:30:31PM +0100, Markus Armbruster wrote:
>> >> 1. Before commit 94649d4 "spapr: Don't use QOM [*] syntax for DR
>> >>connectors", the indexes were small integers:
>> >> 
>> >>(qemu) info qom-tree
>> >>/machine (pseries-2.4-machine)
>> >>  /unattached (container)
>> >>[...]
>> >>/device[5] (spapr-pci-host-bridge)
>> >>  /p...@8002000.mmio[0] (qemu:memory-region)
>> >>  /p...@8002000.mmio-alias[0] (qemu:memory-region)
>> >>  /p...@8002000.io[0] (qemu:memory-region)
>> >>  /p...@8002000.io-alias[0] (qemu:memory-region)
>> >>  /pci.0 (PCI)
>> >>  /pci@8002000.iommu-root[0] (qemu:memory-region)
>> >>  /dr-connector[0] (spapr-dr-connector)
>> >>  /dr-connector[1] (spapr-dr-connector)
>> >>  /dr-connector[2] (spapr-dr-connector)
>> >>  [...]
>> >> 
>> >>Since then, they're big ones:
>> >> 
>> >>  /dr-connector[1073741824] (spapr-dr-connector)
>> >>  /dr-connector[1073741825] (spapr-dr-connector)
>> >>  /dr-connector[1073741826] (spapr-dr-connector)
>> >> 
>> >>The commit message doesn't quite spell out this change, and I'm
>> >>therefore double-checkint it's intentional.  Is it?
>> >
>> > Yes, it's intentional.  The small integers were arbitrarily allocated
>> > by the QOM magic [*] code, whereas the big integers are actually
>> > meaningful values (essentially the DRC's global ID for the dynamic
>> > reconfiguration hypervisor interfaces).
>> 
>> Good.
>> 
>> >> 2. Before commit 6c2f9a1 "qapi: Make output visitor return qnull()
>> >>instead of NULL", qom-get returned {}:
>> >> 
>> >>Since then, it returns null:
>> >> 
>> >>QMP> { "execute": "qom-get", "arguments": { "path": 
>> >> "/machine/unattached/device[5]/dr-connector[1073741950]", "property": 
>> >> "fdt" } }
>> >>{"return": null}
>> >> 
>> >>Does anyone care?
>> >
>> > Hm, I'm guessing this is a case where fdt is NULL internally.  Which I
>> 
>> Yes.
>> 
>> > think will happen before a device gets hotplugged into the DRC.  In
>> > that case null seems more correct to me than {}, since {} would also
>> > be what's shown for a present-but-empty device tree.
>> 
>> It was {} in 2.4.  Changing it to null so we can distingish "nothing"
>> from "empty" is an incompatible change.  May make sense anyway, but I
>> can't judge it.
>
> Strictly speaking it's an incompatible change, yes.  But I find it
> hard to imagine anything would be relying on the {} behaviour.  This
> property is essentially a debugging interface to start with, and the
> missing / empty case is examining it in a state that's unlikely to be
> interesting.

I'm not against changing it, I just want it changed intentionally rather
than by accidental side effect :)

If you tell me you want null here going forward, I'll make sure it gets
changed to null in the next development cycle, with a nice commit
message.

If you want it to be null in 2.5, NAK "[PATCH for-2.5 2/3] spapr_drc:
Change value of property "fdt" from null back to {}".  It'll remain an
implicit change then, not documented in commit messages.  I expect we'll
eventually get a patch similar to the NAKed one regardless, because
we'll tighten up the visitor contracts, and returning without visiting
anything should become a programming error then.

Re: [Qemu-devel] [RFC PATCH 7/9] net/colo-proxy: add packet enqueue and handle function

[Dr. David Alan Gilbert]

* Zhang Chen (zhangchen.f...@cn.fujitsu.com) wrote:
> Hi,Dave
> 
> 
> On 12/03/2015 05:09 PM, Dr. David Alan Gilbert wrote:
> >* Zhang Chen (zhangchen.f...@cn.fujitsu.com) wrote:
> >>Hi,Dave
> >>
> >>On 12/02/2015 12:12 AM, Dr. David Alan Gilbert wrote:
> >>>* Zhang Chen (zhangchen.f...@cn.fujitsu.com) wrote:
> From: zhangchen 
> 
> Add common packet handle function and enqueue
> packet distinguished connection,then we can
> lookup one connection packet to compare
> 
> Signed-off-by: zhangchen 
> ---
>   net/colo-proxy.c | 167 
>  ++-
>   1 file changed, 166 insertions(+), 1 deletion(-)
> 
> diff --git a/net/colo-proxy.c b/net/colo-proxy.c
> index 08a852f..a664e6d 100644
> --- a/net/colo-proxy.c
> +++ b/net/colo-proxy.c
> @@ -24,6 +24,170 @@
>   static char *mode;
>   static bool colo_do_checkpoint;
> +static void packet_destroy(void *opaque, void *user_data);
> +
> +static uint32_t connection_key_hash(const void *opaque)
> +{
> +const Connection_key *key = opaque;
> +uint32_t a, b, c;
> +
> +/* Jenkins hash */
> +a = b = c = JHASH_INITVAL + sizeof(*key);
> +a += key->src;
> +b += key->dst;
> +c += key->ports;
> +__jhash_mix(a, b, c);
> +
> +a += key->ip_proto;
> +__jhash_final(a, b, c);
> +
> +return c;
> +}
> +
> +static int connection_key_equal(const void *opaque1, const void *opaque2)
> +{
> +return memcmp(opaque1, opaque2, sizeof(Connection_key)) == 0;
> +}
> +
> +static void connection_destroy(void *opaque)
> +{
> +Connection *connection = opaque;
> +g_queue_foreach(>primary_list, packet_destroy, NULL);
> +g_queue_free(>primary_list);
> +g_queue_foreach(>secondary_list, packet_destroy, NULL);
> +g_queue_free(>secondary_list);
> +g_slice_free(Connection, connection);
> +}
> +
> +static Connection *connection_new(void)
> +{
> +Connection *connection = g_slice_new(Connection);
> +
> +g_queue_init(>primary_list);
> +g_queue_init(>secondary_list);
> +connection->processing = false;
> +
> +return connection;
> +}
> +
> +/* Return 0 on success, or return -1 if the pkt is corrpted */
> +static int parse_packet_early(Packet *pkt, Connection_key *key)
> +{
> +int network_length;
> +uint8_t *data = pkt->data;
> +
> +pkt->network_layer = data + ETH_HLEN;
> +if (ntohs(*(uint16_t *)(data + 12)) != ETH_P_IP) {
> +if (ntohs(*(uint16_t *)(data + 12)) == ETH_P_ARP) {
> +return -1;
> +}
> +return 0;
> +}
> >>>Can you use some of the functions/macros in include/net/eth.h to
> >>>make this easier? Maybe eth_get_l3_proto ?
> >>>Do you plan to do IPv6 at some point?
> >>I will use include/net/eth.h in next version
> >>
> >>IPv6 currently not support, still colo framework be merged
> >>
> +network_length = pkt->ip->ip_hl * 4;
> +pkt->transport_layer = pkt->network_layer + network_length;
> +key->ip_proto = pkt->ip->ip_p;
> +key->src = pkt->ip->ip_src;
> +key->dst = pkt->ip->ip_dst;
> +
> +switch (key->ip_proto) {
> +case IPPROTO_TCP:
> +case IPPROTO_UDP:
> +case IPPROTO_DCCP:
> +case IPPROTO_ESP:
> +case IPPROTO_SCTP:
> +case IPPROTO_UDPLITE:
> +key->ports = *(uint32_t *)(pkt->transport_layer);
> +break;
> +case IPPROTO_AH:
> +key->ports = *(uint32_t *)(pkt->transport_layer + 4);
> >>>Interesting; I don't see any other code in QEMU to handle AH,
> >>>and I don't know much about it.
> >>>
> +break;
> +default:
> +break;
> +}
> +
> +return 0;
> +}
> +
> +static Packet *packet_new(ColoProxyState *s, const void *data,
> +  int size, Connection_key *key, NetClientState 
> *sender)
> +{
> +Packet *pkt = g_slice_new(Packet);
> +
> +pkt->data = g_malloc(size);
> +memcpy(pkt->data, data, size);
> >>>g_memdup might be useful for these:
> >>>https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-memdup
> >>I will fix it in next version
> >>
> +pkt->size = size;
> +pkt->s = s;
> +pkt->sender = sender;
> +pkt->should_be_sent = false;
> +
> +if (parse_packet_early(pkt, key)) {
> +packet_destroy(pkt, NULL);
> +pkt = NULL;
> +}
> +
> +return pkt;
> +}
> +
> +static void packet_destroy(void *opaque, void *user_data)
> +{
> +Packet *pkt = opaque;
> +g_free(pkt->data);
> +

Re: [Qemu-devel] [2.5 issue] virtio-1 in virtio-net and old vhost

[Cornelia Huck]

On Fri, 4 Dec 2015 10:06:58 +0800
Jason Wang  wrote:

> The problem is for pci: without this patch, guest may always see modern
> bar is "disable-modern=false". But with this patch, on an old kernel
> that does not support VERSION_1, even "disable-modern=false" were
> specified, guest can not see modern bar anymore. Looks like a guest
> visible change.

But the guest even see a modern bar if the host is not able to present
a spec-compliant device?

What we have now is a device that looks like a modern device but that
does not offer VERSION_1. Probably not spec compliant. If virtio-pci is
fixed to not present such broken devices to the guest, this is
guest-visible, yes; but only in the sense that the guest will no longer
see broken devices, but simply legacy devices if configured.

Re: [Qemu-devel] [PATCH 1/2] qemu-file: fix flaws of qemu_put_compression_data

[Juan Quintela]

Liang Li  wrote:
> There are some flaws in qemu_put_compression_data, this patch tries
> to fix it. Now it can be used by other code.
>
> Signed-off-by: Liang Li 
> ---
>  migration/qemu-file.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 0bbd257..ef9cd4a 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -616,7 +616,9 @@ ssize_t qemu_put_compression_data(QEMUFile *f, const 
> uint8_t *p, size_t size,
>  ssize_t blen = IO_BUF_SIZE - f->buf_index - sizeof(int32_t);
>  
>  if (blen < compressBound(size)) {
> -return 0;
> +if (f->ops->writev_buffer || f->ops->put_buffer) {
> +qemu_fflush(f);
> +}
>  }

With your change, when we arrive here:

- blen could still be smaller that compressBound(size), you need to
  recheck
- blen could have changed, but you don't take that in account for the
  following caller.

So, I think code has a bug?

Later, Juan.

>  if (compress2(f->buf + f->buf_index + sizeof(int32_t), (uLongf *),
>(Bytef *)p, size, level) != Z_OK) {
> @@ -624,7 +626,13 @@ ssize_t qemu_put_compression_data(QEMUFile *f, const 
> uint8_t *p, size_t size,
>  return 0;
>  }
>  qemu_put_be32(f, blen);
> +if (f->ops->writev_buffer) {
> +add_to_iovec(f, f->buf + f->buf_index, blen);
> +}
>  f->buf_index += blen;
> +if (f->buf_index == IO_BUF_SIZE) {
> +qemu_fflush(f);
> +}
>  return blen + sizeof(int32_t);
>  }

Re: [Qemu-devel] Networking documentation for a Mac OS X guest

[Mark Cave-Ayland]

On 04/12/15 00:02, Peter Maydell wrote:

> On 3 December 2015 at 23:24, Programmingkid  wrote:
>> I would like to make a little tutorial on how to make networking
>> work for a Mac OS X guest. Where would you suggest I put such
>> documentation? The qemu-doc file is what I was thinking about using.
> 
> Hmm. I think for "doing stuff for a particular guest" type tutorials
> we've got a few links and pages on the wiki; perhaps an entry in
> http://wiki.qemu.org/Manual under "for users that target for a specific
> platform" for OSX guests in general? Then you could have a page giving
> suggestions for the command line in general as well as networking
> in particular. (The existing links in that subsection are all
> external but there's no reason we couldn't have an internal-to-the-wiki
> link there too.)

Is it worth setting up per-arch homepages on the wiki? This is something
I've thought about recently since most of the information out on the
internet regarding QEMU's ability to run various systems is badly out of
date with regard to SPARC/PPC. Even better if they could go in the left
hand list with easy to remember URLs like /SPARC, /PPC, /ARM etc...


ATB,

Mark.

Re: [Qemu-devel] [PATCH 0/2] fix the flaws of qemu_put_compression_data

[Juan Quintela]

Liang Li  wrote:
> This patch fixed the flaws in qemu_put_compression_data function.
> and cleanup the code based on the change.

Hi

We are in hard freeze.  My understanding is that this are
"optimizations" that can wait for 2.6:
- my understanding from commit from message one and from quick look at
  the code is that this change is not needed for current users, is that correct?
- we avoid a copy at the beginning of each block (micro-optimization)

If my understanding is correct, I am delaying this two patches to 2.6.

What do you think?

Later, Juan.

BTW, amit address was wrongly typed, I just fixed it.


>
> Liang Li (2):
>   qemu-file: fix flaws of qemu_put_compression_data
>   migration: code clean up.
>
>  migration/qemu-file.c | 10 +-
>  migration/ram.c   | 20 
>  2 files changed, 17 insertions(+), 13 deletions(-)

Re: [Qemu-devel] Posible regressions around spapr-dr-connector property drc-connector[]

[David Gibson]

On Fri, Dec 04, 2015 at 09:05:51AM +0100, Markus Armbruster wrote:
> David Gibson  writes:
> 
> > On Thu, Dec 03, 2015 at 04:30:31PM +0100, Markus Armbruster wrote:
> >> 1. Before commit 94649d4 "spapr: Don't use QOM [*] syntax for DR
> >>connectors", the indexes were small integers:
> >> 
> >>(qemu) info qom-tree
> >>/machine (pseries-2.4-machine)
> >>  /unattached (container)
> >>[...]
> >>/device[5] (spapr-pci-host-bridge)
> >>  /p...@8002000.mmio[0] (qemu:memory-region)
> >>  /p...@8002000.mmio-alias[0] (qemu:memory-region)
> >>  /p...@8002000.io[0] (qemu:memory-region)
> >>  /p...@8002000.io-alias[0] (qemu:memory-region)
> >>  /pci.0 (PCI)
> >>  /pci@8002000.iommu-root[0] (qemu:memory-region)
> >>  /dr-connector[0] (spapr-dr-connector)
> >>  /dr-connector[1] (spapr-dr-connector)
> >>  /dr-connector[2] (spapr-dr-connector)
> >>  [...]
> >> 
> >>Since then, they're big ones:
> >> 
> >>  /dr-connector[1073741824] (spapr-dr-connector)
> >>  /dr-connector[1073741825] (spapr-dr-connector)
> >>  /dr-connector[1073741826] (spapr-dr-connector)
> >> 
> >>The commit message doesn't quite spell out this change, and I'm
> >>therefore double-checkint it's intentional.  Is it?
> >
> > Yes, it's intentional.  The small integers were arbitrarily allocated
> > by the QOM magic [*] code, whereas the big integers are actually
> > meaningful values (essentially the DRC's global ID for the dynamic
> > reconfiguration hypervisor interfaces).
> 
> Good.
> 
> >> 2. Before commit 6c2f9a1 "qapi: Make output visitor return qnull()
> >>instead of NULL", qom-get returned {}:
> >> 
> >>Since then, it returns null:
> >> 
> >>QMP> { "execute": "qom-get", "arguments": { "path": 
> >> "/machine/unattached/device[5]/dr-connector[1073741950]", "property": 
> >> "fdt" } }
> >>{"return": null}
> >> 
> >>Does anyone care?
> >
> > Hm, I'm guessing this is a case where fdt is NULL internally.  Which I
> 
> Yes.
> 
> > think will happen before a device gets hotplugged into the DRC.  In
> > that case null seems more correct to me than {}, since {} would also
> > be what's shown for a present-but-empty device tree.
> 
> It was {} in 2.4.  Changing it to null so we can distingish "nothing"
> from "empty" is an incompatible change.  May make sense anyway, but I
> can't judge it.

Strictly speaking it's an incompatible change, yes.  But I find it
hard to imagine anything would be relying on the {} behaviour.  This
property is essentially a debugging interface to start with, and the
missing / empty case is examining it in a state that's unlikely to be
interesting.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/3] hw/sd/pxa2xx_mmci: convert to SysBusDevice object

[Peter Maydell]

On 4 December 2015 at 07:30, Markus Armbruster  wrote:
> Peter Maydell  writes:
>
>> On 7 September 2015 at 17:57, Markus Armbruster  wrote:
>>> Peter Maydell  writes:
>>>
 On 7 September 2015 at 17:40, Markus Armbruster  wrote:
> Peter Maydell  writes:
>
>> Convert the pxa2xx_mmci device to be a sysbus device.

>> +static Property pxa2xx_mmci_properties[] = {
>> +/* Note: pointer property 'drive' may remain NULL, thus no need
>> + * for dc->cannot_instantiate_with_device_add_yet = true;
>> + * Unfortunately this can't be a DEFINE_PROP_DRIVE, because
>> + * setting a 'drive' property results in a call to blk_attach_dev()
>> + * attaching the BlockBackend to this device; that then means that
>> + * the call in sd_init() to blk_attach_dev_nofail() which tries to
>> + * attach the BlockBackend to the SD card object aborts.
>> + */
>> +DEFINE_PROP_PTR("drive", PXA2xxMMCIState, blk),
>> +DEFINE_PROP_END_OF_LIST(),
>> +};
>
> As far as I can tell, this problem is an artifact of our interface to
> the common sd-card code, namely sd_init().  sd_init() was made for the
> pre-qdev world: it creates and completely initializes the common
> SDState.
>
> In qdev, we do this in three separate steps: create, set properties,
> realize.  Split up sd_init(), and the problem should go away.

 Yes, but I don't really want to gate QOMification of mmc
 controller devices on the more complicated problem of
 QOMifying sd.c itself, especially since we already have several
 QOMified mmc controllers...
>>>
>>> Is serial.c QOMified?  I don't think so, it's merely structured in a
>>> QOM-friendly way: typedef SerialState, realize helper
>>> serial_realize_core(), unrealize helper serial_exit_core().  If
>>> SerialState had more properties, we'd also need a macro to define them.
>>
>> It looks like since we had this conversation the problem has been
>> dealt with in commit 5ec911c30ff433 by simply turning the sd_init() call
>> to blk_attach_dev_nofail() into a call to blk_attach_dev() which ignores
>> its error return. So I should be able to do this with a DEFINE_PROP_DRIVE
>> now I think...
>
> Ignoring the error is intentional according to the comment, but why is
> it appropriate?

That seems like a question to ask the author and reviewer of that
commit :-) [cc'd].

The intention seems to have been to allow sdhci to do the same thing
I want -- take a drive property (which attaches the BlockBackend to
the controller device) and then hand the BlockBackend to sd_init()
without having it blow up.

Incidentally, in an ideal world wouldn't the block/drive properties
be on the SD card object rather than the controller object ? At least,
we seem to have that split for IDE and SCSI disks.

thanks
-- PMM

Re: [Qemu-devel] Networking documentation for a Mac OS X guest

[Peter Maydell]

On 4 December 2015 at 11:36, Mark Cave-Ayland
 wrote:
> Is it worth setting up per-arch homepages on the wiki? This is something
> I've thought about recently since most of the information out on the
> internet regarding QEMU's ability to run various systems is badly out of
> date with regard to SPARC/PPC. Even better if they could go in the left
> hand list with easy to remember URLs like /SPARC, /PPC, /ARM etc...

Sounds like a good idea to me. If you'd like to do it please go ahead :-)

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH V2 00/10] Qemu: Add live migration support for SRIOV NIC

[Lan, Tianyu]

On 12/4/2015 4:05 PM, Michael S. Tsirkin wrote:

I haven't read it, but I would like to note you can't rely on research
papers.  If you propose a patch to be merged you need to measure what is
its actual effect on modern linux at the end of 2015.


Sure. Will do that.

Re: [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output

[Markus Armbruster]

阎淼  writes:

> 2015-12-04 0:40 GMT+08:00 Markus Armbruster :
>> Eric Blake  writes:
>>
>>> On 12/02/2015 10:08 PM, Miao Yan wrote:
 Macro MAC_FMT and MAC_ARG are not defined, but used in vmxnet3_net_init().
 This will cause build error when debug level is raised in
 vmxnet3_debug.h (enable all VMXNET3_DEBUG_xxx).

 Use VMXNET_MF and VXMNET_MA instead.

 Signed-off-by: Miao Yan 
 ---
  hw/net/vmxnet3.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
 index 5e3a233..ea3d9b7 100644
 --- a/hw/net/vmxnet3.c
 +++ b/hw/net/vmxnet3.c
 @@ -2044,7 +2044,7 @@ static void vmxnet3_net_init(VMXNET3State *s)

  s->link_status_and_speed = VMXNET3_LINK_SPEED | 
 VMXNET3_LINK_STATUS_UP;

 -VMW_CFPRN("Permanent MAC: " MAC_FMT, MAC_ARG(s->perm_mac.a));
 +VMW_CFPRN("Permanent MAC: " VMXNET_MF, VMXNET_MA(s->perm_mac.a));
>>>
>>> This is a classic example of why dead code debug statements are evil.
>>>
>>> You should consider also providing a patch to hw/net/vmxnet_debug.h to
>>> fix ALL of the broken debug macros in that file to instead use a sane
>>> pattern, so that the format string is ALWAYS compiled and just optimized
>>> out when debugging is disabled.
>>>
>>> Here's a conversion of one of the macros for an example of what to do:
>>>
>>> Instead of:
>>>
 #ifdef VMXNET_DEBUG_CONFIG
 #define VMW_CFPRN(fmt, ...)
  \
 do {   
  \
 printf("[%s][CF][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__,
  \
 ## __VA_ARGS__);   
  \
 } while (0)
 #else
 #define VMW_CFPRN(fmt, ...) do {} while (0)
 #endif
>>>
>>> you should do:
>>>
>>> #ifdef VMXNET_DEBUG_CONFIG
>>> # define VMXNET_DEBUG_CONFIG_FLAG 1
>>> #else
>>> # define VMXNET_DEBUG_CONFIG_FLAG 0
>>> #endif
>>> #define VMW_CFPRN(fmt, ...)   \
>>> do {  \
>>> if (VMXNET_DEBUG_CONFIG_FLAG) {   \
>>> printf("[%s][CF][%s]: " fmt "\n", VMXNET_DEVICE_NAME, \
>>>__func__, ## __VA_ARGS__); \
>>> } \
>>> } while (0);
>>>
>>> With that pattern, VMW_CFPRN() will now always check that its arguments
>>> can compile, even though it has no impact to the code size when
>>> VMXNET_DEBUG_CONFIG is not defined.  Note that once you repair all of
>>> the broken macros (I count 8 in that file), you may have some fallout of
>>> other broken dead code that needs to be fixed.
>>
>> You may want to consider tracepoints instead of hand-rolled debugging
>> macros: docs/tracing.txt.
>
> Hi Markus,
>
>   Thanks for your suggestion. It seems trace functions must have
> fixed number of parameters, so, for example,
> VMW_PRN("abc") and VMW_PRN("aaa %d", a) would need two trace functions,
> right ?

The purpose of debug print helpers is to facilitate enabling and
disabling (sets of related) debug prints at compile time, for debugging
purposes.

Tracepoints are more structured: they are meant for tracing the flow of
control as it passes through points of interest.  In general, you want a
seperate tracepoint for each point of interest, so you can follow the
flow of control in the trace.

Tracepoints can be configured with various backends for various purposes
beyond debugging by developers.  You can ship code with tracepoints
compiled in, but disabled, then enable selected tracepoints in the field
for troubleshooting.

>   I looked at the code in vmxnet3.c, IMO not all debug output are
>  suitable to be converted to trace points. Maybe I can work out a
>  patch to fix the mmio part.

I can't tell you which of your debug prints should be tracepoints.  I
just want to make you aware of tracepoints, so you can decide for
yourself.

Re: [Qemu-devel] [Intel-gfx] [Announcement] 2015-Q3 release of XenGT - a Mediated Graphics Passthrough Solution from Intel

[Gerd Hoffmann]

  Hi,

> btw some questions here:
> 
> for non-gl and gl rendering in Qemu, are they based on dma-buf already?

> once we can export guest framebuffer in dma-buf, is there additional work
> required or just straightforward to integrate with SPICE?

Right now we are busy integrating dma-buf support into spice, which will
be used for the gl rendering path, for virtio-gpu.

For intel-vgpu the wireup inside qemu will be slightly different:  We'll
get a dma-buf handle from the igd driver, whereas virtio-gpu renders
into a texture, then exports that texture as dma-buf.

But in both cases we'll go pass the dma-buf with the guest framebuffer
(and meta-data such as fourcc and size) to spice-server, which in turn
will pass on the dma-buf to spice-client for (local) display.  So we
have a common code path in spice for both virtio-gpu and intel-vgpu,
based on dma-bufs.  spice-server even doesn't need to know what kind of
graphics device the guest has, it'll go just process the dma-bufs.

longer-term we also plan to support video-encoding for a remote display.
Again based on dma-bufs, by sending them to the gpu video encoder.


The non-gl rendering path needs to be figured out.

With virtio-gpu we'll go simply turn off 3d support, so the guest will
fallback to do software rendering, we'll get a classic DisplaySurface
and the vnc server can work with that.

That isn't going to fly with intel-vgpu though, so we need something
else.  Import dma-buf, then glReadPixels into a DisplaySurface would
work.  But as mentioned before I'd prefer a code path which doesn't
require opengl support in qemu, and one option for that would be the
special vfio region.  I've written up a quick draft meanwhile:


diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 751b69f..91b928d 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -596,6 +596,28 @@ struct vfio_iommu_spapr_tce_remove {
 };
 #define VFIO_IOMMU_SPAPR_TCE_REMOVE_IO(VFIO_TYPE, VFIO_BASE + 20)
 
+/*  Additional API for vGPU  */
+
+/*
+ * framebuffer meta data
+ * subregion located at the end of the framebuffer region
+ */
+struct vfio_framebuffer {
+   __u32 argsz;
+
+   /* out */
+   __u32 format;/* drm fourcc */
+   __u32 offset;/* relative to region start */
+   __u32 width; /* in pixels */
+   __u32 height;/* in pixels */
+   __u32 stride;/* in bytes  */
+
+   /* in+out */
+#define VFIO_FB_STATE_REQUEST_UPDATE   1  /* userspace requests update
*/
+#define VFIO_FB_STATE_UPDATE_COMPLETE  2  /* kernel signals completion
*/
+   __u32 state; /* VFIO_FB_STATE_ */
+};
+
 /* * */
 
 #endif /* _UAPIVFIO_H */

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] net/vmxnet3.c: fix a build error when enabling debug output

[阎淼]

2015-12-04 0:40 GMT+08:00 Markus Armbruster :
> Eric Blake  writes:
>
>> On 12/02/2015 10:08 PM, Miao Yan wrote:
>>> Macro MAC_FMT and MAC_ARG are not defined, but used in vmxnet3_net_init().
>>> This will cause build error when debug level is raised in
>>> vmxnet3_debug.h (enable all VMXNET3_DEBUG_xxx).
>>>
>>> Use VMXNET_MF and VXMNET_MA instead.
>>>
>>> Signed-off-by: Miao Yan 
>>> ---
>>>  hw/net/vmxnet3.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
>>> index 5e3a233..ea3d9b7 100644
>>> --- a/hw/net/vmxnet3.c
>>> +++ b/hw/net/vmxnet3.c
>>> @@ -2044,7 +2044,7 @@ static void vmxnet3_net_init(VMXNET3State *s)
>>>
>>>  s->link_status_and_speed = VMXNET3_LINK_SPEED | VMXNET3_LINK_STATUS_UP;
>>>
>>> -VMW_CFPRN("Permanent MAC: " MAC_FMT, MAC_ARG(s->perm_mac.a));
>>> +VMW_CFPRN("Permanent MAC: " VMXNET_MF, VMXNET_MA(s->perm_mac.a));
>>
>> This is a classic example of why dead code debug statements are evil.
>>
>> You should consider also providing a patch to hw/net/vmxnet_debug.h to
>> fix ALL of the broken debug macros in that file to instead use a sane
>> pattern, so that the format string is ALWAYS compiled and just optimized
>> out when debugging is disabled.
>>
>> Here's a conversion of one of the macros for an example of what to do:
>>
>> Instead of:
>>
>>> #ifdef VMXNET_DEBUG_CONFIG
>>> #define VMW_CFPRN(fmt, ...) 
>>> \
>>> do {
>>> \
>>> printf("[%s][CF][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__, 
>>> \
>>> ## __VA_ARGS__);
>>> \
>>> } while (0)
>>> #else
>>> #define VMW_CFPRN(fmt, ...) do {} while (0)
>>> #endif
>>
>> you should do:
>>
>> #ifdef VMXNET_DEBUG_CONFIG
>> # define VMXNET_DEBUG_CONFIG_FLAG 1
>> #else
>> # define VMXNET_DEBUG_CONFIG_FLAG 0
>> #endif
>> #define VMW_CFPRN(fmt, ...)   \
>> do {  \
>> if (VMXNET_DEBUG_CONFIG_FLAG) {   \
>> printf("[%s][CF][%s]: " fmt "\n", VMXNET_DEVICE_NAME, \
>>__func__, ## __VA_ARGS__); \
>> } \
>> } while (0);
>>
>> With that pattern, VMW_CFPRN() will now always check that its arguments
>> can compile, even though it has no impact to the code size when
>> VMXNET_DEBUG_CONFIG is not defined.  Note that once you repair all of
>> the broken macros (I count 8 in that file), you may have some fallout of
>> other broken dead code that needs to be fixed.
>
> You may want to consider tracepoints instead of hand-rolled debugging
> macros: docs/tracing.txt.

Hi Markus,

  Thanks for your suggestion. It seems trace functions must have
fixed number of parameters, so, for example,
VMW_PRN("abc") and VMW_PRN("aaa %d", a) would need two trace functions,
right ?

  I looked at the code in vmxnet3.c, IMO not all debug output are
 suitable to be converted to trace points. Maybe I can work out a
 patch to fix the mmio part.

Miao

Re: [Qemu-devel] Posible regressions around spapr-dr-connector property drc-connector[]

[Peter Maydell]

On 4 December 2015 at 10:11, Markus Armbruster  wrote:
> If you want it to be null in 2.5, NAK "[PATCH for-2.5 2/3] spapr_drc:
> Change value of property "fdt" from null back to {}".

That commit is now in master, so if you don't want it you need to
actively send a revert for it.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/3] ppc-for-2.5 queue 20151204

[Peter Maydell]

On 4 December 2015 at 06:46, David Gibson <da...@gibson.dropbear.id.au> wrote:
> The following changes since commit 4c65fed8bdf96780735dbdb92a8bd0d6b6526cc3:
>
>   ui: vnc: avoid floating point exception (2015-12-03 13:34:50 +)
>
> are available in the git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.5-20151204
>
> for you to fetch changes up to ab8bf1d735133df5b5847bef0b0bea496d614a5a:
>
>   spapr_drc: Change value of property "fdt" from null back to {} (2015-12-04 
> 16:50:59 +1100)
>
>
> Peter,
>
> Not sure if you need a pull req for this, or you could just merge
> Markus' patches directly, but to avoid another round trip if you would
> prefer a pull req, here is one.

Applied, thanks.

I do prefer pull requests. That's partly because they're a little
easier for me to process, but mostly because a pull is an unambigious
statement from a submaintainer that they're happy with these patches
and want me to apply them to master, in a format that I can
automatically filter out of the mass of qemu-devel emails and into
a folder where it gets my attention. I don't have anything similar
for "please apply this patch", so that kind of request is more at
risk of getting missed.

thanks
-- PMM

Re: [Qemu-devel] [PULL for-2.5 0/9] Trivial patches for 2015-12-04

[Peter Maydell]

On 4 December 2015 at 06:57, Michael Tokarev  wrote:
> Hello.
>
> During the freeze period, let me please add my trivial pull request too ;)
>
> There are just 9 patches this time, which are either (small) bugfixes,
> or just prettifying one or another part of code to look nicely in the
> release tarball.  Here are all of them:
>
> aniel P. Berrange (1):
>   crypto: avoid two coverity false positive error reports
> this is a cleanup together with a small tracing bugfix,
> an easy patch.  An making coverity even more useful is
> nice too.
>
> Hervé Poussineau (1):
>   scsi: remove scsi_req_free prototype
> just removing a prototype of a single unused function
>
> John Snow (1):
>   util/id: fully allocate names table
> a small bugfix avoiding accessing "random" memory
>
> Markus Armbruster (1):
>   typedefs: Put them back into alphabetical order
> this one sorts include/qemu/typedefs.h in alphabetical order,
> and removes duplicates while at it.  Actual set of typedefs
> isn't changed.  We should have good-looking code conforming
> to our own standards in the released tarball :)
>
> Paolo Bonzini (3):
>   bt: avoid unintended sign extension
> a simple bt bugfix
>
>   gt64xxx: fix decoding of ISD register
> this is a small mips platform bugfix
>
>   bt: check struct sizes
> this one is rather large, but it tries to fix real mess in BT
> device emulation which we have now
>
> Peter Maydell (1):
>   configure: Diagnose broken linkers directly
> A simple patch trying to verify whenever linker works too, not
> only compiler, at configure time before doing more complex
> tests.
>
> Rodrigo Rebello (1):
>   configure: use appropriate code fragment for -fstack-protector checks
> this is an interesting patch in context of the freeze time.
> In short, current test for -fstack-protector gives false positives
> (claiming the feature is available) in some configuration, because
> it only verifies whenever compiler accepts this option but not
> whenever it can compile and link a code which actually uses stack.
>
> Please consider applying.
>
> Thanks,
>
> /mjt
>
>
> The following changes since commit 4c65fed8bdf96780735dbdb92a8bd0d6b6526cc3:
>
>   ui: vnc: avoid floating point exception (2015-12-03 13:34:50 +)
>
> are available in the git repository at:
>
>   git://git.corpit.ru/qemu.git tags/pull-trivial-patches-2015-12-04
>
> for you to fetch changes up to 98475746b357f6c048caf9e001998d8a0618b2e4:
>
>   bt: check struct sizes (2015-12-04 09:39:55 +0300)
>
> 
> trivial patches for 2015-12-04
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] sdhci: Make device "sdhci-pci" unavailable with -device again

[Markus Armbruster]

Paolo Bonzini  writes:

> On 04/12/2015 15:07, Markus Armbruster wrote:
>> We made it unavailable in commit 1910913 because its use of
>> drive_get_next() instead of a property.  Commit 5ec911c replaced
>> drive_get_next() and made the device available, but the property isn't
>> quite right, and the code dangerously ignores blk_attach_dev()
>> failure.  Disable it again before the property becomes ABI, and mark
>> the dangerous spot FIXME.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  hw/sd/sd.c| 1 +
>>  hw/sd/sdhci.c | 6 ++
>>  2 files changed, 7 insertions(+)
>> 
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index ce4d44b..d0be5ea 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -494,6 +494,7 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
>>  if (sd->blk) {
>>  /* Attach dev if not already attached.  (This call ignores an
>>   * error return code if sd->blk is already attached.) */
>> +/* FIXME ignoring blk_attach_dev() failure is wrong and dangerous */
>
> No, it's not (it is tricky though) because blk_attach_dev actually will
> always  fail here when using the drive= property, and never when using
> drive_get_next.
>
> In the drive= case, the successful call (and also the one that will
> catch possible mistakes) is from parse_drive in
> hw/core/qdev-properties-system.c:
>
> $ x86_64-softmmu/qemu-system-x86_64 -drive
> if=none,driver=null-aio,id=foo -device virtio-blk-pci,drive=foo
> -device sdhci-pci,drive=foo
> qemu-system-x86_64: -device sdhci-pci,drive=foo: Drive 'foo' is
> already in use by another device
>
> Did you have something else in mind?

My comment makes two claims: "wrong" and "dangerous".

First "dangerous".  You're making a non-local argument why it's not
actually broken, and you might be right.  If you are, it's just fragile,
not broken.  We could debate whether to call it dangerous or fragile,
but I don't really care.  If you'd prefer to call it fragile, let's
update the comment and the commit message.

Now "wrong".  The qdev property belongs to the SD card (the thing we're
initializing here), not the SD controller sdhci-pci.  Unfortunately, the
SD card still hasn't been qdevified.  But if we permit tacking the
property to the controller now, we're stuck with having it there
forever.  No harm if the SD card never becomes an object in its own
right.  But if it does, it'll end up in the same unhappy place as
usb-storage, where we hackishly jump through hoops to somehow transfer
the backend from the controller to the SCSI device.  This has caused so
much trouble that we replaced the whole thing by usb-bot.  I'm not keen
on repeating the experience.

Re: [Qemu-devel] [PATCH v3 0/3] target-i386: add memory protection-key support

[Eduardo Habkost]

On Wed, Nov 18, 2015 at 10:20:14AM +0800, Huaitong Han wrote:
> Changes in v3:
> *Fix cpuid_7_0_ecx_feature_name error.
> 
> Changes in v2:
> *Fix memcpy error for xsave state.
> *Fix TCG_7_0_ECX_FEATURES to 0.
> *Make subjects more readable.
> 
> The protection-key feature provides an additional mechanism by which IA-32e
> paging controls access to usermode addresses.
> 
> Hardware support for protection keys for user pages is enumerated with CPUID
> feature flag CPUID.7.0.ECX[3]:PKU. Software support is CPUID.7.0.ECX[4]:OSPKE
> with the setting of CR4.PKE(bit 22).
> 
> The PKRU register is XSAVE-managed state CPUID.D.0.EAX[9], the size of XSAVE
> state component for PKRU is 8 bytes, the offset is 0xa80.
> 
> The specification of Protection Keys can be found at SDM (4.6.2, volume 3)
> http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf.

Reviewed-by: Eduardo Habkost 

The patches were squashed together and queued in x86-next branch
for 2.6.

> 
> Huaitong Han (3):
>   target-i386: add pkeys support for cpuid handling
>   target-i386: add pkeys support for xsave state handling
>   target-i386: add pkeys support for vm migration
> 
>  target-i386/cpu.c | 23 ++-
>  target-i386/cpu.h |  7 +++
>  target-i386/kvm.c |  3 +++
>  target-i386/machine.c | 23 +++
>  4 files changed, 55 insertions(+), 1 deletion(-)
> 
> -- 
> 2.4.3
> 
> 

-- 
Eduardo

[Qemu-devel] [PATCH v3 3/3] target-i386: kvm: Use X86XSaveArea struct for xsave save/load

[Eduardo Habkost]

Instead of using offset macros and bit operations in a uint32_t
array, use the X86XSaveArea struct to perform the loading/saving
operations in kvm_put_xsave() and kvm_get_xsave().

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Use uint8_t pointers when loading/saving xmm, ymmh, zmmh,
  keeping the same load/save logic from previous code
* Keep the QEMU_BUILD_BUG_ON lines

Changes v2 -> v3:
* Added PKRU state
* Now the xmm_regs helper macros are named ZMM_Q, no XMM_Q
---
 target-i386/kvm.c | 78 +++
 1 file changed, 38 insertions(+), 40 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index a702b33..3cd8e35 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1285,9 +1285,8 @@ ASSERT_OFFSET(XSAVE_PKRU, pkru_state);
 static int kvm_put_xsave(X86CPU *cpu)
 {
 CPUX86State *env = >env;
-struct kvm_xsave* xsave = env->kvm_xsave_buf;
+X86XSaveArea *xsave = env->kvm_xsave_buf;
 uint16_t cwd, swd, twd;
-uint8_t *xmm, *ymmh, *zmmh;
 int i, r;
 
 if (!has_xsave) {
@@ -1302,25 +1301,26 @@ static int kvm_put_xsave(X86CPU *cpu)
 for (i = 0; i < 8; ++i) {
 twd |= (!env->fptags[i]) << i;
 }
-xsave->region[XSAVE_FCW_FSW] = (uint32_t)(swd << 16) + cwd;
-xsave->region[XSAVE_FTW_FOP] = (uint32_t)(env->fpop << 16) + twd;
-memcpy(>region[XSAVE_CWD_RIP], >fpip, sizeof(env->fpip));
-memcpy(>region[XSAVE_CWD_RDP], >fpdp, sizeof(env->fpdp));
-memcpy(>region[XSAVE_ST_SPACE], env->fpregs,
+xsave->legacy.fcw = cwd;
+xsave->legacy.fsw = swd;
+xsave->legacy.ftw = twd;
+xsave->legacy.fpop = env->fpop;
+xsave->legacy.fpip = env->fpip;
+xsave->legacy.fpdp = env->fpdp;
+memcpy(>legacy.fpregs, env->fpregs,
 sizeof env->fpregs);
-xsave->region[XSAVE_MXCSR] = env->mxcsr;
-*(uint64_t *)>region[XSAVE_XSTATE_BV] = env->xstate_bv;
-memcpy(>region[XSAVE_BNDREGS], env->bnd_regs,
+xsave->legacy.mxcsr = env->mxcsr;
+xsave->header.xstate_bv = env->xstate_bv;
+memcpy(>bndreg_state.bnd_regs, env->bnd_regs,
 sizeof env->bnd_regs);
-memcpy(>region[XSAVE_BNDCSR], >bndcs_regs,
-sizeof(env->bndcs_regs));
-memcpy(>region[XSAVE_OPMASK], env->opmask_regs,
+xsave->bndcsr_state.bndcsr = env->bndcs_regs;
+memcpy(>opmask_state.opmask_regs, env->opmask_regs,
 sizeof env->opmask_regs);
 
-xmm = (uint8_t *)>region[XSAVE_XMM_SPACE];
-ymmh = (uint8_t *)>region[XSAVE_YMMH_SPACE];
-zmmh = (uint8_t *)>region[XSAVE_ZMM_Hi256];
-for (i = 0; i < CPU_NB_REGS; i++, xmm += 16, ymmh += 16, zmmh += 32) {
+for (i = 0; i < CPU_NB_REGS; i++) {
+uint8_t *xmm = xsave->legacy.xmm_regs[i];
+uint8_t *ymmh = xsave->avx_state.ymmh[i];
+uint8_t *zmmh = xsave->zmm_hi256_state.zmm_hi256[i];
 stq_p(xmm, env->xmm_regs[i].ZMM_Q(0));
 stq_p(xmm+8,   env->xmm_regs[i].ZMM_Q(1));
 stq_p(ymmh,env->xmm_regs[i].ZMM_Q(2));
@@ -1332,9 +1332,9 @@ static int kvm_put_xsave(X86CPU *cpu)
 }
 
 #ifdef TARGET_X86_64
-memcpy(>region[XSAVE_Hi16_ZMM], >xmm_regs[16],
+memcpy(>hi16_zmm_state.hi16_zmm, >xmm_regs[16],
 16 * sizeof env->xmm_regs[16]);
-memcpy(>region[XSAVE_PKRU], >pkru, sizeof env->pkru);
+memcpy(>pkru_state, >pkru, sizeof env->pkru);
 #endif
 r = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_XSAVE, xsave);
 return r;
@@ -1669,9 +1669,8 @@ static int kvm_get_fpu(X86CPU *cpu)
 static int kvm_get_xsave(X86CPU *cpu)
 {
 CPUX86State *env = >env;
-struct kvm_xsave* xsave = env->kvm_xsave_buf;
+X86XSaveArea *xsave = env->kvm_xsave_buf;
 int ret, i;
-const uint8_t *xmm, *ymmh, *zmmh;
 uint16_t cwd, swd, twd;
 
 if (!has_xsave) {
@@ -1683,33 +1682,32 @@ static int kvm_get_xsave(X86CPU *cpu)
 return ret;
 }
 
-cwd = (uint16_t)xsave->region[XSAVE_FCW_FSW];
-swd = (uint16_t)(xsave->region[XSAVE_FCW_FSW] >> 16);
-twd = (uint16_t)xsave->region[XSAVE_FTW_FOP];
-env->fpop = (uint16_t)(xsave->region[XSAVE_FTW_FOP] >> 16);
+cwd = xsave->legacy.fcw;
+swd = xsave->legacy.fsw;
+twd = xsave->legacy.ftw;
+env->fpop = xsave->legacy.fpop;
 env->fpstt = (swd >> 11) & 7;
 env->fpus = swd;
 env->fpuc = cwd;
 for (i = 0; i < 8; ++i) {
 env->fptags[i] = !((twd >> i) & 1);
 }
-memcpy(>fpip, >region[XSAVE_CWD_RIP], sizeof(env->fpip));
-memcpy(>fpdp, >region[XSAVE_CWD_RDP], sizeof(env->fpdp));
-env->mxcsr = xsave->region[XSAVE_MXCSR];
-memcpy(env->fpregs, >region[XSAVE_ST_SPACE],
+env->fpip = xsave->legacy.fpip;
+env->fpdp = xsave->legacy.fpdp;
+env->mxcsr = xsave->legacy.mxcsr;
+memcpy(env->fpregs, >legacy.fpregs,
 sizeof env->fpregs);
-env->xstate_bv = *(uint64_t *)>region[XSAVE_XSTATE_BV];
-memcpy(env->bnd_regs, >region[XSAVE_BNDREGS],
+env->xstate_bv = xsave->header.xstate_bv;
+

[Qemu-devel] [PATCH v6 2/4] kobject: export kset_find_obj() for module use

[Gabriel L. Somlo]

From: Gabriel Somlo 

Signed-off-by: Gabriel Somlo 
---
 lib/kobject.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/kobject.c b/lib/kobject.c
index 7cbccd2..90d1be6 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -861,6 +861,7 @@ struct kobject *kset_find_obj(struct kset *kset, const char 
*name)
spin_unlock(>list_lock);
return ret;
 }
+EXPORT_SYMBOL(kset_find_obj);
 
 static void kset_release(struct kobject *kobj)
 {
-- 
2.4.3

[Qemu-devel] [PATCH v6 0/4] SysFS driver for QEMU fw_cfg device

[Gabriel L. Somlo]

Allow access to QEMU firmware blobs, passed into the guest VM via
the fw_cfg device, through SysFS entries. Blob meta-data (e.g. name,
size, and fw_cfg key), as well as the raw binary blob data may be
accessed.

The SysFS access location is /sys/firmware/qemu_fw_cfg/... and was
selected based on overall similarity to the type of information
exposed under /sys/firmware/dmi/entries/...

New since v5:

- fixed typos in documentation files (Patches 1/4 and 4/4

- printf/scanf type modifier for phys_addr_t now matches
  arch-specific width (u32 vs. u64), avoiding compiler warnings.
  (tested on i386 with and without PAE, and on armv7hl with and
   without lpae -- the latter pair took quite a while on an
   emulated QEMU guest :) )

Thanks,
  --Gabriel

>New since v4:
>
>   Documentation (Patches 1/4 and 4/4) now points to the authoritative
>   file in the QEMU source tree for any details related to the "hardware
>   interface" of the fw_cfg device; Only details specific to sysfs (1/4) 
>   and DT (4/4) should stay in the kernel docs.
>
>>New (since v3):
>>
>>  Patch 1/4: Device probing now works with either ACPI, DT, or
>> optionally by manually specifying a base, size, and
>> register offsets on the command line. This way, all
>> architectures offering fw_cfg can be supported, although
>> x86 and ARM get *automatic* support via ACPI and/or DT.
>>
>> HUGE thanks to Laszlo Ersek  for
>> pointing out drivers/virtio/virtio_mmio.c, as an example
>> on how to pull this off !!!
>>
>> Stefan: I saw Marc's DMA patches to fw_cfg. Since only
>> x86 and ARM will support it starting with QEMU 2.5, and
>> since I expect to get lots of otherwise interesting (but
>> otherwise orthogonal) feedback on this series, I'd like
>> to stick with ioread8() across the board for now. We can
>> always patch in DMA support in a backward compatible way
>> later, once this series gets (hopefully) accepted :)
>>
>>  Patch 2/4: (was 3/4 in v3): unchanged. Exports kset_find_obj() so
>> modules can call it.
>>
>>  Patch 3/4: (was 4/4 in v3): rebased, but otherwise the same.
>> Essentially, creates a "human readable" directory
>> hierarchy from "path-like" tokens making up fw_cfg
>> blob names. I'm not really sure there's a way to make
>> this happen via udev rules, but I have at least one
>> potential use case for doing it *before* udev becomes
>> available (cc: Andy Lutomirski ),
>> so I'd be happy to leave this functionality in the
>> kernel module. See further below for an illustration
>> of this.
>>
>>  Patch 4/4: Updates the existing ARM DT documentation for fw_cfg,
>> mainly by pointing at the more comprehensive document
>> introduced with Patch 1/4 for details on the fw_cfg
>> device interface, leaving only the specific ARM/DT
>> address/size node information in place.
>>
>>>  In addition to the "by_key" blob listing, e.g.:
>>>  
>>>  $ tree /sys/firmware/qemu_fw_cfg/
>>>  /sys/firmware/qemu_fw_cfg/
>>>  |-- by_key
>>>  |   |-- 32
>>>  |   |   |-- key
>>>  |   |   |-- name("etc/boot-fail-wait")
>>>  |   |   |-- raw
>>>  |   |   `-- size
>>>  |   |-- 33
>>>  |   |   |-- key
>>>  |   |   |-- name("etc/smbios/smbios-tables")
>>>  |   |   |-- raw
>>>  |   |   `-- size
>>>  |   |-- 34
>>>  |   |   |-- key
>>>  |   |   |-- name("etc/smbios/smbios-anchor")
>>>  |   |   |-- raw
>>>  |   |   `-- size
>>>  |   |-- 35
>>>  |   |   |-- key
>>>  |   |   |-- name("etc/e820")
>>>  |   |   |-- raw
>>>  |   |   `-- size
>>>  |   |-- 36
>>>  |   |   |-- key
>>>  |   |   |-- name("genroms/kvmvapic.bin")
>>>  |   |   |-- raw
>>>  |   |   `-- size
>>>  |   |-- 37
>>>  |   |   |-- key
>>>  |   |   |-- name("etc/system-states")
>>>  |   |   |-- raw
>>>  |   |   `-- size
>>>  |   |-- 38
>>>  |   |   |-- key
>>>  |   |   |-- name("etc/acpi/tables")
>>>  |   |   |-- raw
>>>  |   |   `-- size
>>>  |   |-- 39
>>>  |   |   |-- key
>>>  |   |   |-- name("etc/table-loader")
>>>  |   |   |-- raw
>>>  |   |   `-- size
>>>  |   |-- 40
>>>  |   |   |-- key
>>>  |   |   |-- name("etc/tpm/log")
>>>  |   |   |-- raw
>>>  |   |   `-- size
>>>  |   |-- 41
>>>  |   |   |-- key
>>>  |   |   |-- name("etc/acpi/rsdp")
>>>  |   |   |-- raw
>>>  |   |   `-- size
>>>  |   `-- 42
>>>  |  

Re: [Qemu-devel] [PATCH] xen_pt: fix failure of attaching & detaching a PCI device to VM repeatedly

[Stefano Stabellini]

On Thu, 3 Dec 2015, Jianzhong,Chang wrote:
> From: jianzhong,Chang 
> 
> Add pci = [ '$VF_BDF', '$VF_BDF', '$VF_BDF'] in

This is a bit confusing: it is not actually correct to assign the same
device, even an SR_IOV VF, multiple times, so these must be all
different. More like:

pci = [ '$VF_BDF1', '$VF_BDF2', '$VF_BDF3']


> hvm guest configuration file. After the guest boot up,
> detach the VFs in sequence by "xl pci-detach $DOMID $VF_BDF",
> reattach the VFs by "xl pci-attach $VF_BDF" in sequence.

So do you mean:

xl pci-detach $DOMID $VF_BDF1
xl pci-detach $DOMID $VF_BDF2
xl pci-detach $DOMID $VF_BDF3
xl pci-attach $DOMID $VF_BDF1
xl pci-attach $DOMID $VF_BDF2
xl pci-attach $DOMID $VF_BDF3

?


> An error message will be reported like this:
> "libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received
> an error message from QMP server: Duplicate ID 'pci-pt-07_10.1' for device"
> 
> When xen_pt_region_add/del() is called, MemoryRegion
> may not belong to the XenPCIPassthroughState.
> xen_pt_region_update() checks it but memory_region_ref/unref() does not.
> This case causes obj->ref issue and affects the release of related objects.
> So, the detection operation is moved from
> xen_pt_region_update to xen_pt_region_add/del.
> 
> Signed-off-by: Jianzhong,Chang 
> ---
>  hw/xen/xen_pt.c |   23 ---
>  1 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index aa96288..95b4970 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -523,6 +523,14 @@ static int xen_pt_bar_from_region(XenPCIPassthroughState 
> *s, MemoryRegion *mr)
>  }
>  return -1;
>  }
> +static bool xen_pt_region_in_state(XenPCIPassthroughState *s, MemoryRegion 
> *mr)
> +{
> +int bar = xen_pt_bar_from_region(s, mr);
> +if (-1 == bar && (!s->msix || >msix->mmio != mr)) {
> +return false;
> +}
> +return true;
> +}
>  
>  /*
>   * This function checks if an io_region overlaps an io_region from another
> @@ -587,9 +595,6 @@ static void xen_pt_region_update(XenPCIPassthroughState 
> *s,
>  };
>  
>  bar = xen_pt_bar_from_region(s, mr);
> -if (bar == -1 && (!s->msix || >msix->mmio != mr)) {
> -return;
> -}
>  
>  if (s->msix && >msix->mmio == mr) {
>  if (adding) {
> @@ -642,6 +647,9 @@ static void xen_pt_region_add(MemoryListener *l, 
> MemoryRegionSection *sec)
>  XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState,
>   memory_listener);
>  
> +if (!xen_pt_region_in_state(s, sec->mr)) {
> +return;
> +}
>  memory_region_ref(sec->mr);
>  xen_pt_region_update(s, sec, true);
>  }
> @@ -651,6 +659,9 @@ static void xen_pt_region_del(MemoryListener *l, 
> MemoryRegionSection *sec)
>  XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState,
>   memory_listener);
>  
> +if (!xen_pt_region_in_state(s, sec->mr)) {
> +return;
> +}
>  xen_pt_region_update(s, sec, false);
>  memory_region_unref(sec->mr);
>  }
> @@ -660,6 +671,9 @@ static void xen_pt_io_region_add(MemoryListener *l, 
> MemoryRegionSection *sec)
>  XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState,
>   io_listener);
>  
> +if (!xen_pt_region_in_state(s, sec->mr)) {
> +return;
> +}
>  memory_region_ref(sec->mr);
>  xen_pt_region_update(s, sec, true);
>  }
> @@ -669,6 +683,9 @@ static void xen_pt_io_region_del(MemoryListener *l, 
> MemoryRegionSection *sec)
>  XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState,
>   io_listener);
>  
> +if (!xen_pt_region_in_state(s, sec->mr)) {
> +return;
> +}
>  xen_pt_region_update(s, sec, false);
>  memory_region_unref(sec->mr);
>  }

Wouldn't it make more sense to move the
memory_region_ref/memory_region_unref calls inside xen_pt_region_update?

[Qemu-devel] [PATCH v3 1/3] target-i386: Define structs for layout of xsave area

[Eduardo Habkost]

Add structs that define the layout of the xsave areas used by
Intel processors. Add some QEMU_BUILD_BUG_ON lines to ensure the
structs match the XSAVE_* macros in target-i386/kvm.c and the
offsets and sizes at target-i386/cpu.c:ext_save_areas.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Use uint8_t[8*n] instead of uint64_t[n] for register data

Changes v2 -> v3:
* Added PKRU state
---
 target-i386/cpu.h | 95 +++
 target-i386/kvm.c | 23 ++
 2 files changed, 118 insertions(+)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 54dfe52..ee58306 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -807,6 +807,101 @@ typedef struct {
 
 #define NB_OPMASK_REGS 8
 
+typedef union X86LegacyXSaveArea {
+struct {
+uint16_t fcw;
+uint16_t fsw;
+uint8_t ftw;
+uint8_t reserved;
+uint16_t fpop;
+uint64_t fpip;
+uint64_t fpdp;
+uint32_t mxcsr;
+uint32_t mxcsr_mask;
+FPReg fpregs[8];
+uint8_t xmm_regs[16][16];
+};
+uint8_t data[512];
+} X86LegacyXSaveArea;
+
+typedef struct X86XSaveHeader {
+uint64_t xstate_bv;
+uint64_t xcomp_bv;
+uint8_t reserved[48];
+} X86XSaveHeader;
+
+/* Ext. save area 2: AVX State */
+typedef struct XSaveAVX {
+uint8_t ymmh[16][16];
+} XSaveAVX;
+
+/* Ext. save area 3: BNDREG */
+typedef struct XSaveBNDREG {
+BNDReg bnd_regs[4];
+} XSaveBNDREG;
+
+/* Ext. save area 4: BNDCSR */
+typedef union XSaveBNDCSR {
+BNDCSReg bndcsr;
+uint8_t data[64];
+} XSaveBNDCSR;
+
+/* Ext. save area 5: Opmask */
+typedef struct XSaveOpmask {
+uint64_t opmask_regs[NB_OPMASK_REGS];
+} XSaveOpmask;
+
+/* Ext. save area 6: ZMM_Hi256 */
+typedef struct XSaveZMM_Hi256 {
+uint8_t zmm_hi256[16][32];
+} XSaveZMM_Hi256;
+
+/* Ext. save area 7: Hi16_ZMM */
+typedef struct XSaveHi16_ZMM {
+uint8_t hi16_zmm[16][64];
+} XSaveHi16_ZMM;
+
+/* Ext. save area 9: PKRU state */
+typedef struct XSavePKRU {
+uint32_t pkru;
+uint32_t padding;
+} XSavePKRU;
+
+typedef struct X86XSaveArea {
+X86LegacyXSaveArea legacy;
+X86XSaveHeader header;
+
+/* Extended save areas: */
+
+/* AVX State: */
+XSaveAVX avx_state;
+uint8_t padding[960-576-sizeof(XSaveAVX)];
+/* MPX State: */
+XSaveBNDREG bndreg_state;
+XSaveBNDCSR bndcsr_state;
+/* AVX-512 State: */
+XSaveOpmask opmask_state;
+XSaveZMM_Hi256 zmm_hi256_state;
+XSaveHi16_ZMM hi16_zmm_state;
+/* PKRU State: */
+XSavePKRU pkru_state;
+} X86XSaveArea;
+
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, avx_state) != 0x240);
+QEMU_BUILD_BUG_ON(sizeof(XSaveAVX) != 0x100);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, bndreg_state) != 0x3c0);
+QEMU_BUILD_BUG_ON(sizeof(XSaveBNDREG) != 0x40);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, bndcsr_state) != 0x400);
+QEMU_BUILD_BUG_ON(sizeof(XSaveBNDCSR) != 0x40);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, opmask_state) != 0x440);
+QEMU_BUILD_BUG_ON(sizeof(XSaveOpmask) != 0x40);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, zmm_hi256_state) != 0x480);
+QEMU_BUILD_BUG_ON(sizeof(XSaveZMM_Hi256) != 0x200);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, hi16_zmm_state) != 0x680);
+QEMU_BUILD_BUG_ON(sizeof(XSaveHi16_ZMM) != 0x400);
+QEMU_BUILD_BUG_ON(offsetof(X86XSaveArea, pkru_state) != 0xA80);
+QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8);
+
 typedef enum TPRAccess {
 TPR_ACCESS_READ,
 TPR_ACCESS_WRITE,
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 75eb60b..a702b33 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1259,6 +1259,29 @@ static int kvm_put_fpu(X86CPU *cpu)
 #define XSAVE_Hi16_ZMM416
 #define XSAVE_PKRU672
 
+#define XSAVE_BYTE_OFFSET(word_offset) \
+((word_offset)*sizeof(((struct kvm_xsave*)0)->region[0]))
+
+#define ASSERT_OFFSET(word_offset, field) \
+QEMU_BUILD_BUG_ON(XSAVE_BYTE_OFFSET(word_offset) != \
+  offsetof(X86XSaveArea, field))
+
+ASSERT_OFFSET(XSAVE_FCW_FSW, legacy.fcw);
+ASSERT_OFFSET(XSAVE_FTW_FOP, legacy.ftw);
+ASSERT_OFFSET(XSAVE_CWD_RIP, legacy.fpip);
+ASSERT_OFFSET(XSAVE_CWD_RDP, legacy.fpdp);
+ASSERT_OFFSET(XSAVE_MXCSR, legacy.mxcsr);
+ASSERT_OFFSET(XSAVE_ST_SPACE, legacy.fpregs);
+ASSERT_OFFSET(XSAVE_XMM_SPACE, legacy.xmm_regs);
+ASSERT_OFFSET(XSAVE_XSTATE_BV, header.xstate_bv);
+ASSERT_OFFSET(XSAVE_YMMH_SPACE, avx_state);
+ASSERT_OFFSET(XSAVE_BNDREGS, bndreg_state);
+ASSERT_OFFSET(XSAVE_BNDCSR, bndcsr_state);
+ASSERT_OFFSET(XSAVE_OPMASK, opmask_state);
+ASSERT_OFFSET(XSAVE_ZMM_Hi256, zmm_hi256_state);
+ASSERT_OFFSET(XSAVE_Hi16_ZMM, hi16_zmm_state);
+ASSERT_OFFSET(XSAVE_PKRU, pkru_state);
+
 static int kvm_put_xsave(X86CPU *cpu)
 {
 CPUX86State *env = >env;
-- 
2.1.0

[Qemu-devel] [PATCH v6 4/4] devicetree: update documentation for fw_cfg ARM bindings

[Gabriel L. Somlo]

From: Gabriel Somlo 

Remove fw_cfg hardware interface details from
Documentation/devicetree/bindings/arm/fw-cfg.txt,
and replace them with a pointer to the authoritative
documentation in the QEMU source tree.

Signed-off-by: Gabriel Somlo 
Cc: Laszlo Ersek 
Acked-by: Rob Herring 
Reviewed-by: Laszlo Ersek 
---
 Documentation/devicetree/bindings/arm/fw-cfg.txt | 38 ++--
 1 file changed, 2 insertions(+), 36 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/fw-cfg.txt 
b/Documentation/devicetree/bindings/arm/fw-cfg.txt
index 953fb64..fd54e1d 100644
--- a/Documentation/devicetree/bindings/arm/fw-cfg.txt
+++ b/Documentation/devicetree/bindings/arm/fw-cfg.txt
@@ -11,43 +11,9 @@ QEMU exposes the control and data register to ARM guests as 
memory mapped
 registers; their location is communicated to the guest's UEFI firmware in the
 DTB that QEMU places at the bottom of the guest's DRAM.
 
-The guest writes a selector value (a key) to the selector register, and then
-can read the corresponding data (produced by QEMU) via the data register. If
-the selected entry is writable, the guest can rewrite it through the data
-register.
+The authoritative guest-side hardware interface documentation to the fw_cfg
+device can be found in "docs/specs/fw_cfg.txt" in the QEMU source tree.
 
-The selector register takes keys in big endian byte order.
-
-The data register allows accesses with 8, 16, 32 and 64-bit width (only at
-offset 0 of the register). Accesses larger than a byte are interpreted as
-arrays, bundled together only for better performance. The bytes constituting
-such a word, in increasing address order, correspond to the bytes that would
-have been transferred by byte-wide accesses in chronological order.
-
-The interface allows guest firmware to download various parameters and blobs
-that affect how the firmware works and what tables it installs for the guest
-OS. For example, boot order of devices, ACPI tables, SMBIOS tables, kernel and
-initrd images for direct kernel booting, virtual machine UUID, SMP information,
-virtual NUMA topology, and so on.
-
-The authoritative registry of the valid selector values and their meanings is
-the QEMU source code; the structure of the data blobs corresponding to the
-individual key values is also defined in the QEMU source code.
-
-The presence of the registers can be verified by selecting the "signature" blob
-with key 0x, and reading four bytes from the data register. The returned
-signature is "QEMU".
-
-The outermost protocol (involving the write / read sequences of the control and
-data registers) is expected to be versioned, and/or described by feature bits.
-The interface revision / feature bitmap can be retrieved with key 0x0001. The
-blob to be read from the data register has size 4, and it is to be interpreted
-as a uint32_t value in little endian byte order. The current value
-(corresponding to the above outer protocol) is zero.
-
-The guest kernel is not expected to use these registers (although it is
-certainly allowed to); the device tree bindings are documented here because
-this is where device tree bindings reside in general.
 
 Required properties:
 
-- 
2.4.3

[Qemu-devel] [PATCH v6 3/4] firmware: create directory hierarchy for sysfs fw_cfg entries

[Gabriel L. Somlo]

From: Gabriel Somlo 

Each fw_cfg entry of type "file" has an associated 56-char,
nul-terminated ASCII string which represents its name. While
the fw_cfg device doesn't itself impose any specific naming
convention, QEMU developers have traditionally used path name
semantics (i.e. "etc/acpi/rsdp") to descriptively name the
various fw_cfg "blobs" passed into the guest.

This patch attempts, on a best effort basis, to create a
directory hierarchy representing the content of fw_cfg file
names, under /sys/firmware/qemu_fw_cfg/by_name.

Upon successful creation of all directories representing the
"dirname" portion of a fw_cfg file, a symlink will be created
to represent the "basename", pointing at the appropriate
/sys/firmware/qemu_fw_cfg/by_key entry. If a file name is not
suitable for this procedure (e.g., if its basename or dirname
components collide with an already existing dirname component
or basename, respectively) the corresponding fw_cfg blob is
skipped and will remain available in sysfs only by its selector
key value.

Signed-off-by: Gabriel Somlo 
Cc: Andy Lutomirski 
---
 .../ABI/testing/sysfs-firmware-qemu_fw_cfg |  42 
 drivers/firmware/qemu_fw_cfg.c | 109 -
 2 files changed, 148 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg 
b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
index e9e58d4..011dda4 100644
--- a/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
+++ b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
@@ -56,3 +56,45 @@ Description:
  entry via the control register, and reading a number
  of bytes equal to the blob size from the data
  register.
+
+   --- Listing fw_cfg blobs by file name ---
+
+   While the fw_cfg device does not impose any specific naming
+   convention on the blobs registered in the file directory,
+   QEMU developers have traditionally used path name semantics
+   to give each blob a descriptive name. For example:
+
+   "bootorder"
+   "genroms/kvmvapic.bin"
+   "etc/e820"
+   "etc/boot-fail-wait"
+   "etc/system-states"
+   "etc/table-loader"
+   "etc/acpi/rsdp"
+   "etc/acpi/tables"
+   "etc/smbios/smbios-tables"
+   "etc/smbios/smbios-anchor"
+   ...
+
+   In addition to the listing by unique selector key described
+   above, the fw_cfg sysfs driver also attempts to build a tree
+   of directories matching the path name components of fw_cfg
+   blob names, ending in symlinks to the by_key entry for each
+   "basename", as illustrated below (assume current directory is
+   /sys/firmware):
+
+   qemu_fw_cfg/by_name/bootorder -> ../by_key/38
+   qemu_fw_cfg/by_name/etc/e820 -> ../../by_key/35
+   qemu_fw_cfg/by_name/etc/acpi/rsdp -> ../../../by_key/41
+   ...
+
+   Construction of the directory tree and symlinks is done on a
+   "best-effort" basis, as there is no guarantee that components
+   of fw_cfg blob names are always "well behaved". I.e., there is
+   the possibility that a symlink (basename) will conflict with
+   a dirname component of another fw_cfg blob, in which case the
+   creation of the offending /sys/firmware/qemu_fw_cfg/by_name
+   entry will be skipped.
+
+   The authoritative list of entries will continue to be found
+   under the /sys/firmware/qemu_fw_cfg/by_key directory.
diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index 5b00d97..85f532a 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -318,9 +318,103 @@ static struct bin_attribute fw_cfg_sysfs_attr_raw = {
.read = fw_cfg_sysfs_read_raw,
 };
 
-/* kobjects representing top-level and by_key folders */
+/*
+ * Create a kset subdirectory matching each '/' delimited dirname token
+ * in 'name', starting with sysfs kset/folder 'dir'; At the end, create
+ * a symlink directed at the given 'target'.
+ * NOTE: We do this on a best-effort basis, since 'name' is not guaranteed
+ * to be a well-behaved path name. Whenever a symlink vs. kset directory
+ * name collision occurs, the kernel will issue big scary warnings while
+ * refusing to add the offending link or directory. We follow up with our
+ * own, slightly less scary error messages explaining the situation :)
+ */
+static int fw_cfg_build_symlink(struct kset *dir,
+   struct kobject *target, 

Re: [Qemu-devel] [PATCH 01/23] userfaultfd: linux/Documentation/vm/userfaultfd.txt

[Michael Kerrisk (man-pages)]

Hi Andrea,

On 09/11/2015 10:47 AM, Michael Kerrisk (man-pages) wrote:
> On 05/14/2015 07:30 PM, Andrea Arcangeli wrote:
>> Add documentation.
> 
> Hi Andrea,
> 
> I do not recall... Did you write a man page also for this new system call?

No response to my last mail, so I'll try again... Did you 
write any man page for this interface?

Thanks,

Michael


>> Signed-off-by: Andrea Arcangeli 
>> ---
>>  Documentation/vm/userfaultfd.txt | 140 
>> +++
>>  1 file changed, 140 insertions(+)
>>  create mode 100644 Documentation/vm/userfaultfd.txt
>>
>> diff --git a/Documentation/vm/userfaultfd.txt 
>> b/Documentation/vm/userfaultfd.txt
>> new file mode 100644
>> index 000..c2f5145
>> --- /dev/null
>> +++ b/Documentation/vm/userfaultfd.txt
>> @@ -0,0 +1,140 @@
>> += Userfaultfd =
>> +
>> +== Objective ==
>> +
>> +Userfaults allow the implementation of on-demand paging from userland
>> +and more generally they allow userland to take control various memory
>> +page faults, something otherwise only the kernel code could do.
>> +
>> +For example userfaults allows a proper and more optimal implementation
>> +of the PROT_NONE+SIGSEGV trick.
>> +
>> +== Design ==
>> +
>> +Userfaults are delivered and resolved through the userfaultfd syscall.
>> +
>> +The userfaultfd (aside from registering and unregistering virtual
>> +memory ranges) provides two primary functionalities:
>> +
>> +1) read/POLLIN protocol to notify a userland thread of the faults
>> +   happening
>> +
>> +2) various UFFDIO_* ioctls that can manage the virtual memory regions
>> +   registered in the userfaultfd that allows userland to efficiently
>> +   resolve the userfaults it receives via 1) or to manage the virtual
>> +   memory in the background
>> +
>> +The real advantage of userfaults if compared to regular virtual memory
>> +management of mremap/mprotect is that the userfaults in all their
>> +operations never involve heavyweight structures like vmas (in fact the
>> +userfaultfd runtime load never takes the mmap_sem for writing).
>> +
>> +Vmas are not suitable for page- (or hugepage) granular fault tracking
>> +when dealing with virtual address spaces that could span
>> +Terabytes. Too many vmas would be needed for that.
>> +
>> +The userfaultfd once opened by invoking the syscall, can also be
>> +passed using unix domain sockets to a manager process, so the same
>> +manager process could handle the userfaults of a multitude of
>> +different processes without them being aware about what is going on
>> +(well of course unless they later try to use the userfaultfd
>> +themselves on the same region the manager is already tracking, which
>> +is a corner case that would currently return -EBUSY).
>> +
>> +== API ==
>> +
>> +When first opened the userfaultfd must be enabled invoking the
>> +UFFDIO_API ioctl specifying a uffdio_api.api value set to UFFD_API (or
>> +a later API version) which will specify the read/POLLIN protocol
>> +userland intends to speak on the UFFD. The UFFDIO_API ioctl if
>> +successful (i.e. if the requested uffdio_api.api is spoken also by the
>> +running kernel), will return into uffdio_api.features and
>> +uffdio_api.ioctls two 64bit bitmasks of respectively the activated
>> +feature of the read(2) protocol and the generic ioctl available.
>> +
>> +Once the userfaultfd has been enabled the UFFDIO_REGISTER ioctl should
>> +be invoked (if present in the returned uffdio_api.ioctls bitmask) to
>> +register a memory range in the userfaultfd by setting the
>> +uffdio_register structure accordingly. The uffdio_register.mode
>> +bitmask will specify to the kernel which kind of faults to track for
>> +the range (UFFDIO_REGISTER_MODE_MISSING would track missing
>> +pages). The UFFDIO_REGISTER ioctl will return the
>> +uffdio_register.ioctls bitmask of ioctls that are suitable to resolve
>> +userfaults on the range registered. Not all ioctls will necessarily be
>> +supported for all memory types depending on the underlying virtual
>> +memory backend (anonymous memory vs tmpfs vs real filebacked
>> +mappings).
>> +
>> +Userland can use the uffdio_register.ioctls to manage the virtual
>> +address space in the background (to add or potentially also remove
>> +memory from the userfaultfd registered range). This means a userfault
>> +could be triggering just before userland maps in the background the
>> +user-faulted page.
>> +
>> +The primary ioctl to resolve userfaults is UFFDIO_COPY. That
>> +atomically copies a page into the userfault registered range and wakes
>> +up the blocked userfaults (unless uffdio_copy.mode &
>> +UFFDIO_COPY_MODE_DONTWAKE is set). Other ioctl works similarly to
>> +UFFDIO_COPY.
>> +
>> +== QEMU/KVM ==
>> +
>> +QEMU/KVM is using the userfaultfd syscall to implement postcopy live
>> +migration. Postcopy live migration is one form of memory
>> +externalization consisting of a virtual machine running with part or
>> +all of its memory residing on a different 

[Qemu-devel] [PATCH v3 0/3] target-i386: Use C struct for xsave area layout, offsets & sizes

[Eduardo Habkost]

target-i386/cpu.c:ext_save_area uses magic numbers for the xsave
area offets and sizes, and target-i386/kvm.c:kvm_{put,get}_xsave()
uses offset macros and bit manipulation to access the xsave area.
This series changes both to use C structs for those operations.

I still need to figure out a way to write unit tests for the new
code. Maybe I will just copy and paste the new and old functions,
and test them locally (checking if they give the same results
when translating blobs of random bytes).

Changes v1 -> v2:
* Use uint8_t[8*n] instead of uint64_t[n] for register data
* Keep the QEMU_BUILD_BUG_ON lines

Changes v2 -> v3:
* XMM_Q helper is now ZMM_Q
* Added PKRU state

Eduardo Habkost (3):
  target-i386: Define structs for layout of xsave area
  target-i386: Use xsave structs for ext_save_area
  target-i386: kvm: Use X86XSaveArea struct for xsave save/load

 target-i386/cpu.c |  21 
 target-i386/cpu.h |  95 ++
 target-i386/kvm.c | 101 +-
 3 files changed, 170 insertions(+), 47 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v3 2/3] target-i386: Use xsave structs for ext_save_area

[Eduardo Habkost]

This doesn't introduce any change in the code, as the offsets and
struct sizes match what was present in the table. This can be
validated by the QEMU_BUILD_BUG_ON lines on target-i386/cpu.h,
which ensures the struct sizes and offsets match the existing
values in ext_save_area.

Signed-off-by: Eduardo Habkost 
---
Changes series v1 -> v2
* (none)

Changes series v2 -> v3:
* Added PKRU state
---
 target-i386/cpu.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a3de18d..c4cfedb 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -477,19 +477,26 @@ typedef struct ExtSaveArea {
 
 static const ExtSaveArea ext_save_areas[] = {
 [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
-.offset = 0x240, .size = 0x100 },
+.offset = offsetof(X86XSaveArea, avx_state),
+.size = sizeof(XSaveAVX) },
 [3] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
-.offset = 0x3c0, .size = 0x40  },
+.offset = offsetof(X86XSaveArea, bndreg_state),
+.size = sizeof(XSaveBNDREG)  },
 [4] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_MPX,
-.offset = 0x400, .size = 0x40  },
+.offset = offsetof(X86XSaveArea, bndcsr_state),
+.size = sizeof(XSaveBNDCSR)  },
 [5] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
-.offset = 0x440, .size = 0x40 },
+.offset = offsetof(X86XSaveArea, opmask_state),
+.size = sizeof(XSaveOpmask) },
 [6] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
-.offset = 0x480, .size = 0x200 },
+.offset = offsetof(X86XSaveArea, zmm_hi256_state),
+.size = sizeof(XSaveZMM_Hi256) },
 [7] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
-.offset = 0x680, .size = 0x400 },
+.offset = offsetof(X86XSaveArea, hi16_zmm_state),
+.size = sizeof(XSaveHi16_ZMM) },
 [9] = { .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU,
-.offset = 0xA80, .size = 0x8 },
+.offset = offsetof(X86XSaveArea, pkru_state),
+.size = sizeof(XSavePKRU) },
 };
 
 const char *get_register_name_32(unsigned int reg)
-- 
2.1.0

Re: [Qemu-devel] [PATCH QEMU-XEN v6 4/8] xen: Switch uses of xc_map_foreign_range into xc_map_foreign_pages

[Stefano Stabellini]

On Thu, 3 Dec 2015, Ian Campbell wrote:
> In Xen 4.7 we are refactoring parts libxenctrl into a number of
> separate libraries which will provide backward and forward API and ABI
> compatiblity.
> 
> One such library will be libxenforeignmemory which provides access to
> privileged foreign mappings and which will provide an interface
> equivalent to xc_map_foreign_{pages,bulk}.
> 
> In preparation for this switch all uses of xc_map_foreign_range to
> xc_map_foreign_pages. This is trivial because size was always
> XC_PAGE_SIZE so the necessary adjustments are trivial:
> 
>   * Pass  (an array of length 1) instead of mfn. The function
> takes a pointer to const, so there is no possibily of mfn changing
> due to this change.
>   * Pass nr_pages=1 instead of size=XC_PAGE_SIZE
> 
> There is one wrinkle in xen_console.c:con_initialise() where
> con->ring_ref is an int but can in some code paths (when !xendev->dev)
> be treated as an mfn. I think this is an existing latent truncation
> hazard on platforms where xen_pfn_t is 64-bit and int is 32-bit (e.g.
> amd64, both arm* variants). I'm unsure under what circumstances
> xendev->dev can be NULL or if anything elsewhere ensures the value
> fits into an int. For now I just use a temporary xen_pfn_t to in
> effect upcast the pointer from int* to xen_pfn_t*.
> 
> Signed-off-by: Ian Campbell 

Reviewed-by: Stefano Stabellini 


> v6: Switch to xc_map_foreign_pages rather than _bulk. Switching to
> _bulk without checking the value of err[0] risked missing errors.
> The new xenforeignmemory_map coming later in this series will
> DTRT with err==NULL.
> 
> Dropped Stefano's Reviewed-by due to this change.
> 
> v4: Ran checkpatch, fixed all errors
> Mention the const-ness of the mfn array in the commit log
> ---
>  hw/char/xen_console.c |  8 
>  hw/display/xenfb.c|  5 ++---
>  xen-hvm.c | 14 +++---
>  3 files changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index 8c06c19..56f1b1a 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -228,10 +228,10 @@ static int con_initialise(struct XenDevice *xendev)
>   con->buffer.max_capacity = limit;
>  
>  if (!xendev->dev) {
> -con->sring = xc_map_foreign_range(xen_xc, con->xendev.dom,
> -  XC_PAGE_SIZE,
> -  PROT_READ|PROT_WRITE,
> -  con->ring_ref);
> +xen_pfn_t mfn = con->ring_ref;
> +con->sring = xc_map_foreign_pages(xen_xc, con->xendev.dom,
> + PROT_READ|PROT_WRITE,
> + , 1);
>  } else {
>  con->sring = xengnttab_map_grant_ref(xendev->gnttabdev, 
> con->xendev.dom,
>   con->ring_ref,
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 5e324ef..c96d974 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -104,9 +104,8 @@ static int common_bind(struct common *c)
>  if (xenstore_read_fe_int(>xendev, "event-channel", 
> >xendev.remote_port) == -1)
>   return -1;
>  
> -c->page = xc_map_foreign_range(xen_xc, c->xendev.dom,
> -XC_PAGE_SIZE,
> -PROT_READ | PROT_WRITE, mfn);
> +c->page = xc_map_foreign_pages(xen_xc, c->xendev.dom,
> +   PROT_READ | PROT_WRITE, , 1);
>  if (c->page == NULL)
>   return -1;
>  
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 6680782..2f4e109 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -1240,8 +1240,9 @@ int xen_hvm_init(PCMachineState *pcms,
>  DPRINTF("buffered io page at pfn %lx\n", bufioreq_pfn);
>  DPRINTF("buffered io evtchn is %x\n", bufioreq_evtchn);
>  
> -state->shared_page = xc_map_foreign_range(xen_xc, xen_domid, 
> XC_PAGE_SIZE,
> -  PROT_READ|PROT_WRITE, 
> ioreq_pfn);
> +state->shared_page = xc_map_foreign_pages(xen_xc, xen_domid,
> +  PROT_READ|PROT_WRITE,
> +  _pfn, 1);
>  if (state->shared_page == NULL) {
>  hw_error("map shared IO page returned error %d handle=" 
> XC_INTERFACE_FMT,
>   errno, xen_xc);
> @@ -1251,8 +1252,8 @@ int xen_hvm_init(PCMachineState *pcms,
>  if (!rc) {
>  DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn);
>  state->shared_vmport_page =
> -xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
> - PROT_READ|PROT_WRITE, ioreq_pfn);
> +xc_map_foreign_pages(xen_xc, xen_domid, PROT_READ|PROT_WRITE,
> + _pfn, 1);
>  if (state->shared_vmport_page == NULL) {
>  

Re: [Qemu-devel] [PATCH QEMU-XEN v6 5/8] xen: Switch uses of xc_map_foreign_{pages, bulk} to use libxenforeignmemory API.

[Stefano Stabellini]

On Thu, 3 Dec 2015, Ian Campbell wrote:
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index 2cadd4f..8fbaf07 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -41,6 +41,7 @@ static inline void *xc_map_foreign_bulk(int xc_handle, 
> uint32_t dom, int prot,
>  typedef int XenXC;
>  typedef int xenevtchn_handle;
>  typedef int xengnttab_handle;
> +typedef int xenforeignmemory_handle;
>  
>  #  define XC_INTERFACE_FMT "%i"
>  #  define XC_HANDLER_INITIAL_VALUE-1
> @@ -104,6 +105,19 @@ static inline XenXC xen_xc_interface_open(void *logger, 
> void *dombuild_logger,
>  return xc_interface_open();
>  }
>  
> +#define xenforeignmemory_open(l, f) _xc
> +static inline void *xenforeignmemory_map(XenXC *h, uint32_t dom,
> + int prot, size_t pages,
> + const xen_pfn_t arr[/*pages*/],
> + int err[/*pages*/])
> +{
> +if (err)
> +return xc_map_foreign_bulk(*h, dom, prot, arr, err, pages);
> +else
> +return xc_map_foreign_pages(*h, dom, prot, arr, pages);
> +}
> +#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE)
> +
>  static inline int xc_fd(int xen_xc)
>  {
>  return xen_xc;
> @@ -149,6 +163,7 @@ static inline void xs_close(struct xs_handle *xsh)
>  #else
>  
>  typedef xc_interface *XenXC;
> +typedef xc_interface *xenforeignmemory_handle;
>  typedef xc_evtchn xenevtchn_handle;
>  typedef xc_gnttab xengnttab_handle;
>  
> @@ -178,6 +193,20 @@ static inline XenXC xen_xc_interface_open(void *logger, 
> void *dombuild_logger,
>  return xc_interface_open(logger, dombuild_logger, open_flags);
>  }
>  
> +#define xenforeignmemory_open(l, f) _xc
> +static inline void *xenforeignmemory_map(XenXC *h, uint32_t dom,
> + int prot, size_t pages,
> + const xen_pfn_t arr[/*pages*/],
> + int err[/*pages*/])
> +{
> +if (err)
> +return xc_map_foreign_bulk(*h, dom, prot, arr, err, pages);
> +else
> +return xc_map_foreign_pages(*h, dom, prot, arr, pages);
> +}
> +
> +#define xenforeignmemory_unmap(h, p, s) munmap(p, s * XC_PAGE_SIZE)
> +
>  /* FIXME There is now way to have the xen fd */
>  static inline int xc_fd(xc_interface *xen_xc)
>  {

The two definitions are the same. Please introduce just one, at the
bottom under the ifdef CONFIG_XEN_CTRL_INTERFACE_VERSION < 470 case.

[Qemu-devel] [PATCH v6 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device

[Gabriel L. Somlo]

From: Gabriel Somlo 

Make fw_cfg entries of type "file" available via sysfs. Entries
are listed under /sys/firmware/qemu_fw_cfg/by_key, in folders
named after each entry's selector key. Filename, selector value,
and size read-only attributes are included for each entry. Also,
a "raw" attribute allows retrieval of the full binary content of
each entry.

The fw_cfg device can be instantiated automatically from ACPI or
the Device Tree, or manually by using a kernel module (or command
line) parameter, with a syntax outlined in the documentation file.

Signed-off-by: Gabriel Somlo 
---
 .../ABI/testing/sysfs-firmware-qemu_fw_cfg |  58 ++
 drivers/firmware/Kconfig   |  19 +
 drivers/firmware/Makefile  |   1 +
 drivers/firmware/qemu_fw_cfg.c | 632 +
 4 files changed, 710 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
 create mode 100644 drivers/firmware/qemu_fw_cfg.c

diff --git a/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg 
b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
new file mode 100644
index 000..e9e58d4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
@@ -0,0 +1,58 @@
+What:  /sys/firmware/qemu_fw_cfg/
+Date:  August 2015
+Contact:   Gabriel Somlo 
+Description:
+   Several different architectures supported by QEMU (x86, arm,
+   sun4*, ppc/mac) are provisioned with a firmware configuration
+   (fw_cfg) device, originally intended as a way for the host to
+   provide configuration data to the guest firmware. Starting
+   with QEMU v2.4, arbitrary fw_cfg file entries may be specified
+   by the user on the command line, which makes fw_cfg additionally
+   useful as an out-of-band, asynchronous mechanism for providing
+   configuration data to the guest userspace.
+
+   The authoritative guest-side hardware interface documentation
+   to the fw_cfg device can be found in "docs/specs/fw_cfg.txt"
+   in the QEMU source tree.
+
+   === SysFS fw_cfg Interface ===
+
+   The fw_cfg sysfs interface described in this document is only
+   intended to display discoverable blobs (i.e., those registered
+   with the file directory), as there is no way to determine the
+   presence or size of "legacy" blobs (with selector keys between
+   0x0002 and 0x0018) programmatically.
+
+   All fw_cfg information is shown under:
+
+   /sys/firmware/qemu_fw_cfg/
+
+   The only legacy blob displayed is the fw_cfg device revision:
+
+   /sys/firmware/qemu_fw_cfg/rev
+
+   --- Discoverable fw_cfg blobs by selector key ---
+
+   All discoverable blobs listed in the fw_cfg file directory are
+   displayed as entries named after their unique selector key
+   value, e.g.:
+
+   /sys/firmware/qemu_fw_cfg/by_key/32
+   /sys/firmware/qemu_fw_cfg/by_key/33
+   /sys/firmware/qemu_fw_cfg/by_key/34
+   ...
+
+   Each such fw_cfg sysfs entry has the following values exported
+   as attributes:
+
+   name: The 56-byte nul-terminated ASCII string used as the
+ blob's 'file name' in the fw_cfg directory.
+   size: The length of the blob, as given in the fw_cfg
+ directory.
+   key : The value of the blob's selector key as given in the
+ fw_cfg directory. This value is the same as used in
+ the parent directory name.
+   raw : The raw bytes of the blob, obtained by selecting the
+ entry via the control register, and reading a number
+ of bytes equal to the blob size from the data
+ register.
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index cf478fe..3cf920a 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -161,6 +161,25 @@ config RASPBERRYPI_FIRMWARE
  This option enables support for communicating with the firmware on the
  Raspberry Pi.
 
+config FW_CFG_SYSFS
+   tristate "QEMU fw_cfg device support in sysfs"
+   depends on SYSFS
+   default n
+   help
+ Say Y or M here to enable the exporting of the QEMU firmware
+ configuration (fw_cfg) file entries via sysfs. Entries are
+ found under /sys/firmware/fw_cfg when this option is enabled
+ and loaded.
+
+config FW_CFG_SYSFS_CMDLINE
+   bool "QEMU fw_cfg device parameter parsing"
+   depends on 

Re: [Qemu-devel] [PATCH QEMU-XEN v6 6/8] xen: Use stable library interfaces when they are available.

[Stefano Stabellini]

On Thu, 3 Dec 2015, Ian Campbell wrote:
> In Xen 4.7 we are refactoring parts libxenctrl into a number of
> separate libraries which will provide backward and forward API and ABI
> compatiblity.
> 
> Specifically libxenevtchn, libxengnttab and libxenforeignmemory.
> 
> Previous patches have already laid the groundwork for using these by
> switching the existing compatibility shims to reflect the intefaces to
> these libraries.
> 
> So all which remains is to update configure to detect the libraries
> and enable their use. Although they are notionally independent we take
> an all or nothing approach to the three libraries since they were
> added at the same time.
> 
> The only non-obvious bit is that we now open a proper xenforeignmemory
> handle for xen_fmem instead of reusing the xen_xc handle.
> 
> Build tested with 4.0 .. 4.6 (inclusive) and the patches targetting
> 4.7 which adds these libraries.
> 
> Signed-off-by: Ian Campbell 

Reviewed-by: Stefano Stabellini 


> v6: Two minor formatting things.
> Rebase onto "xen: fix usage of xc_domain_create in domain
> builder", required adjusting the configure script changes.
> 
> The rebase wasn't entirely trivial (semantically), so dropped
> Stefano's reviwed by.
> 
> v5: Remove ifdef check when undeffing the compat stuff.
> s/now way/no way/
> 
> v4: xenforeignmemory_open is now a compat wrapper, so no ifdef.
> 
> Simplify configury by asserting that interface version 470 will
> always have the libraries (lack of them makes it 460).
> 
> Ran checkpatch and fixed everything except:
> 
> ERROR: need consistent spacing around '*' (ctx:WxV)
> +typedef xc_interface *XenXC;
> 
> Which I think is a false +ve.
> 
> Simpler configure stuff
> ---
>  configure   | 42 +-
>  include/hw/xen/xen_common.h | 35 +--
>  2 files changed, 74 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 979bc55..cb72115 100755
> --- a/configure
> +++ b/configure
> @@ -1923,6 +1923,7 @@ fi
>  
>  if test "$xen" != "no" ; then
>xen_libs="-lxenstore -lxenctrl -lxenguest"
> +  xen_stable_libs="-lxenforeignmemory -lxengnttab -lxenevtchn"
>  
># First we test whether Xen headers and libraries are available.
># If no, we are done and there is no Xen support.
> @@ -1945,16 +1946,52 @@ EOF
># Xen unstable
>elif
>cat > $TMPC < +/*
> + * If we have stable libs the we don't want the libxc compat
> + * layers, regardless of what CFLAGS we may have been given.
> + */
> +#undef XC_WANT_COMPAT_EVTCHN_API
> +#undef XC_WANT_COMPAT_GNTTAB_API
> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
>  #include 
> +#include 
> +#if !defined(HVM_MAX_VCPUS)
> +# error HVM_MAX_VCPUS not defined
> +#endif
>  int main(void) {
>xc_interface *xc = NULL;
> +  xenforeignmemory_handle *xfmem;
> +  xenevtchn_handle *xe;
> +  xengnttab_handle *xg;
>xen_domain_handle_t handle;
> +
> +  xs_daemon_open();
> +
> +  xc = xc_interface_open(0, 0, 0);
> +  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
> +  xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
> +  xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
> +  xc_hvm_create_ioreq_server(xc, 0, HVM_IOREQSRV_BUFIOREQ_ATOMIC, NULL);
>xc_domain_create(xc, 0, handle, 0, NULL, NULL);
> +
> +  xfmem = xenforeignmemory_open(0, 0);
> +  xenforeignmemory_map(xfmem, 0, 0, 0, 0, 0);
> +
> +  xe = xenevtchn_open(0, 0);
> +  xenevtchn_fd(xe);
> +
> +  xg = xengnttab_open(0, 0);
> +  xengnttab_map_grant_ref(xg, 0, 0, 0);
> +
>return 0;
>  }
>  EOF
> -  compile_prog "" "$xen_libs"
> +  compile_prog "" "$xen_libs $xen_stable_libs"
>  then
>  xen_ctrl_version=470
>  xen=yes
> @@ -2138,6 +2175,9 @@ EOF
>fi
>  
>if test "$xen" = yes; then
> +if test $xen_ctrl_version -ge 470  ; then
> +  libs_softmmu="$xen_stable_libs $libs_softmmu"
> +fi
>  libs_softmmu="$xen_libs $libs_softmmu"
>fi
>  fi
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index 8fbaf07..884fbd1 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -6,6 +6,15 @@
>  #include 
>  #include 
>  
> +/*
> + * If we have new enough libxenctrl then we do not want/need these compat
> + * interfaces, despite what the user supplied cflags might say. They
> + * must be undefined before including xenctrl.h
> + */
> +#undef XC_WANT_COMPAT_EVTCHN_API
> +#undef XC_WANT_COMPAT_GNTTAB_API
> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
> +
>  #include 
>  #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 420
>  #  include 
> @@ -159,8 +168,8 @@ static inline void xs_close(struct xs_handle *xsh)
>  }
>  
>  
> -/* Xen 4.1 */
> -#else
> +/* Xen 4.1 thru 4.6 */
> +#elif CONFIG_XEN_CTRL_INTERFACE_VERSION < 470
>  
>  typedef xc_interface *XenXC;
>  typedef 

[Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

[Alex Zuepke]

LEON3 allows the CASA instruction to be used from user space
if the ASI is set to 0xa (user data).

Signed-off-by: Alex Zuepke 
---
 target-sparc/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 41a3319..63440dd 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5097,7 +5097,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned 
int insn)
 if (IS_IMM) {
 goto illegal_insn;
 }
-if (!supervisor(dc)) {
+/* LEON3 allows CASA from user space with ASI 0xa */
+if ((GET_FIELD(insn, 19, 26) != 0xa) && !supervisor(dc)) {
 goto priv_insn;
 }
 #endif
-- 
1.9.1

Re: [Qemu-devel] [PATCH] ivshmem: Store file descriptor for vhost-user negotiation

[Tetsuya Mukawa]

2015-12-04 22:20 GMT+09:00 Marc-André Lureau :
> Hi
>
> On Fri, Dec 4, 2015 at 4:52 AM, Tetsuya Mukawa  wrote:
>> If virtio-net driver allocates memory in vishmem shared memory,
>
> s/vishmem/ivshmem

Thanks, I will fix it.

>
> How can virtio-net allocate memory in ivshmem memory bar?
>
> What's the use case or test case?

One of example is userspace device driver like DPDK PMD.
Actually, I've found this fd related behavior using DPDK virtio-net PMD.
Could you please check below to know my use case more?
http://thread.gmane.org/gmane.comp.networking.dpdk.devel/28467/focus=28493

Tetsuya,

>
>> vhost-net will work correctly, but vhost-user will not work because
>> a fd of shared memory will not be sent to vhost-user backend.
>> This patch fixes ivshmem to store file descriptor of shared memory.
>> It will be used when vhost-user negotiates vhost-user backend.
>>
>> Signed-off-by: Tetsuya Mukawa 
>> ---
>>  exec.c  | 10 ++
>>  hw/misc/ivshmem.c   |  9 +++--
>>  include/exec/ram_addr.h |  1 +
>>  3 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 0bf0a6e..908c4bf 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1796,6 +1796,16 @@ int qemu_get_ram_fd(ram_addr_t addr)
>>  return fd;
>>  }
>>
>> +void qemu_set_ram_fd(ram_addr_t addr, int fd)
>> +{
>> +RAMBlock *block;
>> +
>> +rcu_read_lock();
>> +block = qemu_get_ram_block(addr);
>> +block->fd = fd;
>> +rcu_read_unlock();
>> +}
>> +
>>  void *qemu_get_ram_block_host_ptr(ram_addr_t addr)
>>  {
>>  RAMBlock *block;
>> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
>> index f73f0c2..df585de 100644
>> --- a/hw/misc/ivshmem.c
>> +++ b/hw/misc/ivshmem.c
>> @@ -29,6 +29,7 @@
>>  #include "sysemu/char.h"
>>  #include "sysemu/hostmem.h"
>>  #include "qapi/visitor.h"
>> +#include "exec/ram_addr.h"
>>
>>  #include "hw/misc/ivshmem.h"
>>
>> @@ -422,6 +423,7 @@ static int create_shared_memory_BAR(IVShmemState *s, int 
>> fd, uint8_t attr,
>>
>>  memory_region_init_ram_ptr(>ivshmem, OBJECT(s), "ivshmem.bar2",
>> s->ivshmem_size, ptr);
>> +qemu_set_ram_fd(s->ivshmem.ram_addr, fd);
>>  vmstate_register_ram(>ivshmem, DEVICE(s));
>>  memory_region_add_subregion(>bar, 0, >ivshmem);
>>
>> @@ -682,6 +684,7 @@ static void ivshmem_read(void *opaque, const uint8_t 
>> *buf, int size)
>>  }
>>  memory_region_init_ram_ptr(>ivshmem, OBJECT(s),
>> "ivshmem.bar2", s->ivshmem_size, 
>> map_ptr);
>> +qemu_set_ram_fd(s->ivshmem.ram_addr, incoming_fd);
>>  vmstate_register_ram(>ivshmem, DEVICE(s));
>>
>>  IVSHMEM_DPRINTF("guest h/w addr = %p, size = %" PRIu64 "\n",
>> @@ -689,7 +692,6 @@ static void ivshmem_read(void *opaque, const uint8_t 
>> *buf, int size)
>>
>>  memory_region_add_subregion(>bar, 0, >ivshmem);
>>
>> -close(incoming_fd);
>>  return;
>>  }
>>
>> @@ -991,7 +993,6 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error 
>> **errp)
>>  }
>>
>>  create_shared_memory_BAR(s, fd, attr, errp);
>> -close(fd);
>>  }
>>  }
>>
>> @@ -1010,11 +1011,15 @@ static void pci_ivshmem_exit(PCIDevice *dev)
>>  if (memory_region_is_mapped(>ivshmem)) {
>>  if (!s->hostmem) {
>>  void *addr = memory_region_get_ram_ptr(>ivshmem);
>> +int fd;
>>
>>  if (munmap(addr, s->ivshmem_size) == -1) {
>>  error_report("Failed to munmap shared memory %s",
>>   strerror(errno));
>>  }
>> +
>> +if ((fd = qemu_get_ram_fd(s->ivshmem.ram_addr)) != -1)
>> +close(fd);
>>  }
>>
>>  vmstate_unregister_ram(>ivshmem, DEVICE(dev));
>> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
>> index 7115154..9ca659a 100644
>> --- a/include/exec/ram_addr.h
>> +++ b/include/exec/ram_addr.h
>> @@ -72,6 +72,7 @@ ram_addr_t qemu_ram_alloc_resizeable(ram_addr_t size, 
>> ram_addr_t max_size,
>>   void *host),
>>   MemoryRegion *mr, Error **errp);
>>  int qemu_get_ram_fd(ram_addr_t addr);
>> +void qemu_set_ram_fd(ram_addr_t addr, int fd);
>>  void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
>>  void *qemu_get_ram_ptr(ram_addr_t addr);
>>  void qemu_ram_free(ram_addr_t addr);
>> --
>> 2.1.4
>>
>>
>
>
>
> --
> Marc-André Lureau

Re: [Qemu-devel] [PATCH] sdhci: Make device "sdhci-pci" unavailable with -device again

[Paolo Bonzini]

On 04/12/2015 15:59, Markus Armbruster wrote:
> My comment makes two claims: "wrong" and "dangerous".
> 
> First "dangerous".  You're making a non-local argument why it's not
> actually broken, and you might be right.  If you are, it's just fragile,
> not broken.  We could debate whether to call it dangerous or fragile,
> but I don't really care.  If you'd prefer to call it fragile, let's
> update the comment and the commit message.

Indeed I didn't claim it wasn't dangerous. :)  Still, that doesn't
justify disabling device-add.

> Now "wrong".  The qdev property belongs to the SD card (the thing we're
> initializing here), not the SD controller sdhci-pci.  Unfortunately, the
> SD card still hasn't been qdevified.  But if we permit tacking the
> property to the controller now, we're stuck with having it there
> forever.  No harm if the SD card never becomes an object in its own
> right.  But if it does, it'll end up in the same unhappy place as
> usb-storage, where we hackishly jump through hoops to somehow transfer
> the backend from the controller to the SCSI device.  This has caused so
> much trouble that we replaced the whole thing by usb-bot.  I'm not keen
> on repeating the experience.

That's a different kind of wrong, unrelated to blk_attach_dev.

I cannot say that it will never happen, and I'm not keen on having
another usb-storage.  That said, we have another example of automagic
property setting on children, which is virtio-blk-pci.  That one works
well, and I wonder whether lessons taught by virtio-blk-pci could be
applied to cleanup usb-storage.

But perfect is the enemy of good.  sdhci-pci is a useful device for
testing, and I don't see the point of disabling it with no qdevification
in sight for the SD card.

FWIW, I don't think the SD card will be qdevified because it doesn't
need a bus.  A host controller controls exactly one SD card, the SSI
bridge is also for exactly one SD card, etc.  So there's not much to
gain from qdevification of the card itself.  There would be a gain from
qdevification of the OMAP and StrongARM controllers (i.e. drive_get_next
is moved to the board, the problematic blk_attach_dev can disappear, etc.).

Paolo

[Qemu-devel] [PATCH] arm: soc-dma: use hwaddr instead of target_ulong in printf

[Paolo Bonzini]

This is a first baby step towards removing widespread inclusion of
cpu.h and compiling more devices once (so that arm, aarch64 and
in the future target-multi can share the object files).

Signed-off-by: Paolo Bonzini 
---
 hw/dma/soc_dma.c | 37 -
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
index c06aabb..ac395c5 100644
--- a/hw/dma/soc_dma.c
+++ b/hw/dma/soc_dma.c
@@ -269,11 +269,10 @@ void soc_dma_port_add_fifo(struct soc_dma_s *soc, hwaddr 
virt_base,
 if (entry->type == soc_dma_port_mem) {
 if (entry->addr <= virt_base &&
 entry->addr + entry->u.mem.size > virt_base) {
-fprintf(stderr, "%s: FIFO at " TARGET_FMT_lx
-" collides with RAM region at " TARGET_FMT_lx
-"-" TARGET_FMT_lx "\n", __FUNCTION__,
-(target_ulong) virt_base,
-(target_ulong) entry->addr, (target_ulong)
+fprintf(stderr, "%s: FIFO at %"PRIx64
+" collides with RAM region at %"PRIx64
+"-%"PRIx64 "\n", __FUNCTION__,
+virt_base, entry->addr,
 (entry->addr + entry->u.mem.size));
 exit(-1);
 }
@@ -284,10 +283,9 @@ void soc_dma_port_add_fifo(struct soc_dma_s *soc, hwaddr 
virt_base,
 while (entry < dma->memmap + dma->memmap_size &&
 entry->addr <= virt_base) {
 if (entry->addr == virt_base && entry->u.fifo.out == out) {
-fprintf(stderr, "%s: FIFO at " TARGET_FMT_lx
-" collides FIFO at " TARGET_FMT_lx "\n",
-__FUNCTION__, (target_ulong) virt_base,
-(target_ulong) entry->addr);
+fprintf(stderr, "%s: FIFO at %"PRIx64
+" collides FIFO at %"PRIx64 "\n",
+__FUNCTION__, virt_base, entry->addr);
 exit(-1);
 }
 
@@ -322,13 +320,11 @@ void soc_dma_port_add_mem(struct soc_dma_s *soc, uint8_t 
*phys_base,
 if ((entry->addr >= virt_base && entry->addr < virt_base + size) ||
 (entry->addr <= virt_base &&
  entry->addr + entry->u.mem.size > virt_base)) {
-fprintf(stderr, "%s: RAM at " TARGET_FMT_lx "-" TARGET_FMT_lx
-" collides with RAM region at " TARGET_FMT_lx
-"-" TARGET_FMT_lx "\n", __FUNCTION__,
-(target_ulong) virt_base,
-(target_ulong) (virt_base + size),
-(target_ulong) entry->addr, (target_ulong)
-(entry->addr + entry->u.mem.size));
+fprintf(stderr, "%s: RAM at %"PRIx64 "-%"PRIx64
+" collides with RAM region at %"PRIx64
+"-%"PRIx64 "\n", __FUNCTION__,
+virt_base, virt_base + size,
+entry->addr, entry->addr + entry->u.mem.size);
 exit(-1);
 }
 
@@ -337,12 +333,11 @@ void soc_dma_port_add_mem(struct soc_dma_s *soc, uint8_t 
*phys_base,
 } else {
 if (entry->addr >= virt_base &&
 entry->addr < virt_base + size) {
-fprintf(stderr, "%s: RAM at " TARGET_FMT_lx "-" TARGET_FMT_lx
-" collides with FIFO at " TARGET_FMT_lx
+fprintf(stderr, "%s: RAM at %"PRIx64 "-%"PRIx64
+" collides with FIFO at %"PRIx64
 "\n", __FUNCTION__,
-(target_ulong) virt_base,
-(target_ulong) (virt_base + size),
-(target_ulong) entry->addr);
+virt_base, virt_base + size,
+entry->addr);
 exit(-1);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH] arm: explicitly mark loads as little-endian

[Paolo Bonzini]

ARM softmmu is always compiled as little endian; BE8/BE32 can be
done as part of CPU emulation.  Thus, devices need not use the
endian-dependent loads and swaps.

Signed-off-by: Paolo Bonzini 
---
 hw/display/omap_lcd_template.h | 4 ++--
 hw/display/pxa2xx_lcd.c| 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/display/omap_lcd_template.h b/hw/display/omap_lcd_template.h
index e5dd447..f0ce71f 100644
--- a/hw/display/omap_lcd_template.h
+++ b/hw/display/omap_lcd_template.h
@@ -136,7 +136,7 @@ static void glue(draw_line12_, DEPTH)(void *opaque,
 uint8_t r, g, b;
 
 do {
-v = lduw_p((void *) s);
+v = lduw_le_p((void *) s);
 r = (v >> 4) & 0xf0;
 g = v & 0xf0;
 b = (v << 4) & 0xf0;
@@ -159,7 +159,7 @@ static void glue(draw_line16_, DEPTH)(void *opaque,
 uint8_t r, g, b;
 
 do {
-v = lduw_p((void *) s);
+v = lduw_le_p((void *) s);
 r = (v >> 8) & 0xf8;
 g = (v >> 3) & 0xfc;
 b = (v << 3) & 0xf8;
diff --git a/hw/display/pxa2xx_lcd.c b/hw/display/pxa2xx_lcd.c
index 494700d..4d36c94 100644
--- a/hw/display/pxa2xx_lcd.c
+++ b/hw/display/pxa2xx_lcd.c
@@ -309,10 +309,10 @@ static void pxa2xx_descriptor_load(PXA2xxLCDState *s)
 }
 
 cpu_physical_memory_read(descptr, , sizeof(desc));
-s->dma_ch[i].descriptor = tswap32(desc.fdaddr);
-s->dma_ch[i].source = tswap32(desc.fsaddr);
-s->dma_ch[i].id = tswap32(desc.fidr);
-s->dma_ch[i].command = tswap32(desc.ldcmd);
+s->dma_ch[i].descriptor = le32_to_cpu(desc.fdaddr);
+s->dma_ch[i].source = le32_to_cpu(desc.fsaddr);
+s->dma_ch[i].id = le32_to_cpu(desc.fidr);
+s->dma_ch[i].command = le32_to_cpu(desc.ldcmd);
 }
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2] net: convert qemu_log to error_report, fix message

[Paolo Bonzini]

Ensure that the error is printed with the proper timestamp.

Signed-off-by: Paolo Bonzini 
---
v1->v2: remove reference to -net dump, dumping can be a network filter
---
 net/dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dump.c b/net/dump.c
index ce16a4b..1c05f78 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -84,7 +84,7 @@ static ssize_t dump_receive_iov(DumpState *s, const struct 
iovec *iov, int cnt)
 cnt = iov_copy([1], cnt, iov, cnt, 0, caplen);
 
 if (writev(s->fd, dumpiov, cnt + 1) != sizeof(hdr) + caplen) {
-qemu_log("-net dump write error - stop dump\n");
+error_report("network dump write error - stopping dump");
 close(s->fd);
 s->fd = -1;
 }
-- 
2.5.0

Re: [Qemu-devel] [PATCH for-2.5 2/3] spapr_drc: Change value of property "fdt" from null back to {}

[Eric Blake]

On 12/03/2015 10:55 AM, Markus Armbruster wrote:

> I'm incapable of proof-reading anything I wrote myself %-}

I know the feeling :)

> 
> prop_get_fdt() misuses the visitor API: when fdt is null, it doesn't
> visit anything.  object_property_get_qobject() happily calls
> qmp_output_get_qobject() then.  Amazingly, the latter survives the
> misuse.  Turns out we've papered over it long before prop_get_fdt()
> existed, in commit 1d10b44.

Oh well, it got pushed to master with original confusing spelling after
all (commit ab8bf1d7).  Not the end of the world :)

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 11/21] block: Add infrastructure for option inheritance

[Kevin Wolf]

Options are not actually inherited from the parent node yet, but this
commit lays the grounds for doing so.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 block.c   | 52 ---
 include/block/block_int.h |  3 ++-
 2 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index 235aa72..5b0183e 100644
--- a/block.c
+++ b/block.c
@@ -695,11 +695,14 @@ static int bdrv_temp_snapshot_flags(int flags)
 }
 
 /*
- * Returns the flags that bs->file should get if a protocol driver is expected,
- * based on the given flags for the parent BDS
+ * Returns the options and flags that bs->file should get if a protocol driver
+ * is expected, based on the given options and flags for the parent BDS
  */
-static int bdrv_inherited_flags(int flags)
+static void bdrv_inherited_options(int *child_flags, QDict *child_options,
+   int parent_flags, QDict *parent_options)
 {
+int flags = parent_flags;
+
 /* Enable protocol handling, disable format probing for bs->file */
 flags |= BDRV_O_PROTOCOL;
 
@@ -710,45 +713,51 @@ static int bdrv_inherited_flags(int flags)
 /* Clear flags that only apply to the top layer */
 flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ);
 
-return flags;
+*child_flags = flags;
 }
 
 const BdrvChildRole child_file = {
-.inherit_flags = bdrv_inherited_flags,
+.inherit_options = bdrv_inherited_options,
 };
 
 /*
- * Returns the flags that bs->file should get if the use of formats (and not
- * only protocols) is permitted for it, based on the given flags for the parent
- * BDS
+ * Returns the options and flags that bs->file should get if the use of formats
+ * (and not only protocols) is permitted for it, based on the given options and
+ * flags for the parent BDS
  */
-static int bdrv_inherited_fmt_flags(int parent_flags)
+static void bdrv_inherited_fmt_options(int *child_flags, QDict *child_options,
+   int parent_flags, QDict *parent_options)
 {
-int flags = child_file.inherit_flags(parent_flags);
-return flags & ~BDRV_O_PROTOCOL;
+child_file.inherit_options(child_flags, child_options,
+   parent_flags, parent_options);
+
+*child_flags &= ~BDRV_O_PROTOCOL;
 }
 
 const BdrvChildRole child_format = {
-.inherit_flags = bdrv_inherited_fmt_flags,
+.inherit_options = bdrv_inherited_fmt_options,
 };
 
 /*
- * Returns the flags that bs->backing should get, based on the given flags
- * for the parent BDS
+ * Returns the options and flags that bs->backing should get, based on the
+ * given options and flags for the parent BDS
  */
-static int bdrv_backing_flags(int flags)
+static void bdrv_backing_options(int *child_flags, QDict *child_options,
+ int parent_flags, QDict *parent_options)
 {
+int flags = parent_flags;
+
 /* backing files always opened read-only */
 flags &= ~(BDRV_O_RDWR | BDRV_O_COPY_ON_READ);
 
 /* snapshot=on is handled on the top layer */
 flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_TEMPORARY);
 
-return flags;
+*child_flags = flags;
 }
 
 static const BdrvChildRole child_backing = {
-.inherit_flags = bdrv_backing_flags,
+.inherit_options = bdrv_backing_options,
 };
 
 static int bdrv_open_flags(BlockDriverState *bs, int flags)
@@ -1480,7 +1489,8 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 
 if (child_role) {
 bs->inherits_from = parent;
-flags = child_role->inherit_flags(parent->open_flags);
+child_role->inherit_options(, options,
+parent->open_flags, parent->options);
 }
 
 ret = bdrv_fill_options(, , , _err);
@@ -1518,7 +1528,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 }
 if (flags & BDRV_O_SNAPSHOT) {
 snapshot_flags = bdrv_temp_snapshot_flags(flags);
-flags = bdrv_backing_flags(flags);
+bdrv_backing_options(, options, flags, options);
 }
 
 bs->open_flags = flags;
@@ -1721,14 +1731,14 @@ static BlockReopenQueue 
*bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
  * 1. Explicitly passed in options (highest)
  * 2. TODO Set in flags (only for top level)
  * 3. TODO Retained from explicitly set options of bs
- * 4. TODO Inherited from parent node
+ * 4. Inherited from parent node
  * 5. Retained from effective options of bs
  */
 
 /* Inherit from parent node */
 if (parent_options) {
 assert(!flags);
-flags = role->inherit_flags(parent_flags);
+role->inherit_options(, options, parent_flags, parent_options);
 }
 
 /* Old values are used for options that aren't set yet */
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 

Re: [Qemu-devel] [2.5 issue] virtio-1 in virtio-net and old vhost

[Michael S. Tsirkin]

On Fri, Dec 04, 2015 at 11:15:48AM +0100, Cornelia Huck wrote:
> On Fri, 4 Dec 2015 10:06:58 +0800
> Jason Wang  wrote:
> 
> > The problem is for pci: without this patch, guest may always see modern
> > bar is "disable-modern=false". But with this patch, on an old kernel
> > that does not support VERSION_1, even "disable-modern=false" were
> > specified, guest can not see modern bar anymore. Looks like a guest
> > visible change.
> 
> But the guest even see a modern bar if the host is not able to present
> a spec-compliant device?
> 
> What we have now is a device that looks like a modern device but that
> does not offer VERSION_1. Probably not spec compliant. If virtio-pci is
> fixed to not present such broken devices to the guest, this is
> guest-visible, yes; but only in the sense that the guest will no longer
> see broken devices, but simply legacy devices if configured.

I agree - we don't have to keep what's broken, broken.
If guests are known not to work, we don't worry about
how well they migrate.

-- 
MST

[Qemu-devel] [PATCH v3 12/21] block: Split out parse_json_protocol()

[Kevin Wolf]

The next patch distinguishes options that were explicitly set and
options that were derived. bdrv_fill_option() added options of both
types: Options given by json: syntax should be counted as explicit, but
the rest is derived.

In preparation for the distinction, move json: parse to a separate
function.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 block.c | 50 --
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/block.c b/block.c
index 5b0183e..8ac1669 100644
--- a/block.c
+++ b/block.c
@@ -1018,37 +1018,45 @@ static QDict *parse_json_filename(const char *filename, 
Error **errp)
 return options;
 }
 
+static void parse_json_protocol(QDict *options, const char **pfilename,
+Error **errp)
+{
+QDict *json_options;
+Error *local_err = NULL;
+
+/* Parse json: pseudo-protocol */
+if (!*pfilename || !g_str_has_prefix(*pfilename, "json:")) {
+return;
+}
+
+json_options = parse_json_filename(*pfilename, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+/* Options given in the filename have lower priority than options
+ * specified directly */
+qdict_join(options, json_options, false);
+QDECREF(json_options);
+*pfilename = NULL;
+}
+
 /*
  * Fills in default options for opening images and converts the legacy
  * filename/flags pair to option QDict entries.
  * The BDRV_O_PROTOCOL flag in *flags will be set or cleared accordingly if a
  * block driver has been specified explicitly.
  */
-static int bdrv_fill_options(QDict **options, const char **pfilename,
+static int bdrv_fill_options(QDict **options, const char *filename,
  int *flags, Error **errp)
 {
-const char *filename = *pfilename;
 const char *drvname;
 bool protocol = *flags & BDRV_O_PROTOCOL;
 bool parse_filename = false;
 BlockDriver *drv = NULL;
 Error *local_err = NULL;
 
-/* Parse json: pseudo-protocol */
-if (filename && g_str_has_prefix(filename, "json:")) {
-QDict *json_options = parse_json_filename(filename, _err);
-if (local_err) {
-error_propagate(errp, local_err);
-return -EINVAL;
-}
-
-/* Options given in the filename have lower priority than options
- * specified directly */
-qdict_join(*options, json_options, false);
-QDECREF(json_options);
-*pfilename = filename = NULL;
-}
-
 drvname = qdict_get_try_str(*options, "driver");
 if (drvname) {
 drv = bdrv_find_format(drvname);
@@ -1487,13 +1495,19 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 options = qdict_new();
 }
 
+parse_json_protocol(options, , _err);
+if (local_err) {
+ret = -EINVAL;
+goto fail;
+}
+
 if (child_role) {
 bs->inherits_from = parent;
 child_role->inherit_options(, options,
 parent->open_flags, parent->options);
 }
 
-ret = bdrv_fill_options(, , , _err);
+ret = bdrv_fill_options(, filename, , _err);
 if (local_err) {
 goto fail;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 15/21] qemu-iotests: Remove cache mode test without medium

[Kevin Wolf]

Specifying the cache mode for a driver without a medium is not a useful
thing to do: As long as there is no medium, the cache mode doesn't make
a difference, and once the 'change' command is used to insert a medium,
it ignores the old cache mode and makes the new medium use
cache=writethrough.

Later patches will make it an error to specify the cache mode for an
empty drive. Remove the corresponding test case.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 tests/qemu-iotests/051| 12 ++--
 tests/qemu-iotests/051.out| 14 +++---
 tests/qemu-iotests/iotests.py |  2 +-
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 17dbf04..f6f0f4d 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -183,12 +183,12 @@ echo
 # Cannot use the test image because cache=none might not work on the host FS
 # Use cdrom so that we won't get errors about missing media
 
-run_qemu -drive media=cdrom,cache=none
-run_qemu -drive media=cdrom,cache=directsync
-run_qemu -drive media=cdrom,cache=writeback
-run_qemu -drive media=cdrom,cache=writethrough
-run_qemu -drive media=cdrom,cache=unsafe
-run_qemu -drive media=cdrom,cache=invalid_value
+run_qemu -drive driver=null-co,cache=none
+run_qemu -drive driver=null-co,cache=directsync
+run_qemu -drive driver=null-co,cache=writeback
+run_qemu -drive driver=null-co,cache=writethrough
+run_qemu -drive driver=null-co,cache=unsafe
+run_qemu -drive driver=null-co,cache=invalid_value
 
 echo
 echo === Specifying the protocol layer ===
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 7765aa0..7a459a3 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -215,28 +215,28 @@ QEMU X.Y.Z monitor - type 'help' for more information
 
 === Cache modes ===
 
-Testing: -drive media=cdrom,cache=none
+Testing: -drive driver=null-co,cache=none
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) qququiquit
 
-Testing: -drive media=cdrom,cache=directsync
+Testing: -drive driver=null-co,cache=directsync
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) qququiquit
 
-Testing: -drive media=cdrom,cache=writeback
+Testing: -drive driver=null-co,cache=writeback
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) qququiquit
 
-Testing: -drive media=cdrom,cache=writethrough
+Testing: -drive driver=null-co,cache=writethrough
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) qququiquit
 
-Testing: -drive media=cdrom,cache=unsafe
+Testing: -drive driver=null-co,cache=unsafe
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) qququiquit
 
-Testing: -drive media=cdrom,cache=invalid_value
-QEMU_PROG: -drive media=cdrom,cache=invalid_value: invalid cache option
+Testing: -drive driver=null-co,cache=invalid_value
+QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
 
 
 === Specifying the protocol layer ===
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 27eddec..0a238ec 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -148,12 +148,12 @@ class VM(object):
 def add_drive(self, path, opts='', interface='virtio'):
 '''Add a virtio-blk drive to the VM'''
 options = ['if=%s' % interface,
-   'cache=%s' % cachemode,
'id=drive%d' % self._num_drives]
 
 if path is not None:
 options.append('file=%s' % path)
 options.append('format=%s' % imgfmt)
+options.append('cache=%s' % cachemode)
 
 if opts:
 options.append(opts)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/2] qemu-file: fix flaws of qemu_put_compression_data

[Li, Liang Z]

> > There are some flaws in qemu_put_compression_data, this patch tries to
> > fix it. Now it can be used by other code.
> >
> > Signed-off-by: Liang Li 
> > ---
> >  migration/qemu-file.c | 10 +-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c index
> > 0bbd257..ef9cd4a 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -616,7 +616,9 @@ ssize_t qemu_put_compression_data(QEMUFile *f,
> const uint8_t *p, size_t size,
> >  ssize_t blen = IO_BUF_SIZE - f->buf_index - sizeof(int32_t);
> >
> >  if (blen < compressBound(size)) {
> > -return 0;
> > +if (f->ops->writev_buffer || f->ops->put_buffer) {
> > +qemu_fflush(f);
> > +}
> >  }
> 
> With your change, when we arrive here:
> 
> - blen could still be smaller that compressBound(size), you need to
>   recheck
> - blen could have changed, but you don't take that in account for the
>   following caller.
> 
> So, I think code has a bug?

Yes, there is a bug, I should consider the case QEMUFile with empty ops.
The right code should be like:

if (blen < compressBound(size)) {
if (f->ops->writev_buffer || f->ops->put_buffer) {
qemu_fflush(f);
} else {
return 0;
}
}


It is enough?

Liang



> 
> Later, Juan.

Re: [Qemu-devel] [PATCH] arm: soc-dma: use hwaddr instead of target_ulong in printf

[Peter Maydell]

On 4 December 2015 at 12:28, Paolo Bonzini  wrote:
> This is a first baby step towards removing widespread inclusion of
> cpu.h and compiling more devices once (so that arm, aarch64 and
> in the future target-multi can share the object files).
>
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/dma/soc_dma.c | 37 -
>  1 file changed, 16 insertions(+), 21 deletions(-)
>
> diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
> index c06aabb..ac395c5 100644
> --- a/hw/dma/soc_dma.c
> +++ b/hw/dma/soc_dma.c
> @@ -269,11 +269,10 @@ void soc_dma_port_add_fifo(struct soc_dma_s *soc, 
> hwaddr virt_base,
>  if (entry->type == soc_dma_port_mem) {
>  if (entry->addr <= virt_base &&
>  entry->addr + entry->u.mem.size > virt_base) {
> -fprintf(stderr, "%s: FIFO at " TARGET_FMT_lx
> -" collides with RAM region at " TARGET_FMT_lx
> -"-" TARGET_FMT_lx "\n", __FUNCTION__,
> -(target_ulong) virt_base,
> -(target_ulong) entry->addr, (target_ulong)
> +fprintf(stderr, "%s: FIFO at %"PRIx64
> +" collides with RAM region at %"PRIx64
> +"-%"PRIx64 "\n", __FUNCTION__,
> +virt_base, entry->addr,
>  (entry->addr + entry->u.mem.size));

Is using the HWADDR_PRI* macros for printing hwaddrs deprecated now?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] arm: explicitly mark loads as little-endian

[Peter Maydell]

On 4 December 2015 at 12:28, Paolo Bonzini  wrote:
> ARM softmmu is always compiled as little endian; BE8/BE32 can be
> done as part of CPU emulation.  Thus, devices need not use the
> endian-dependent loads and swaps.

The patch code is right, but I think the more significant point
here is that the behaviour of the PXA2xx and OMAP devices being
emulated here doesn't (shouldn't) depend on the CPU endianness
anyway, and these devices are little endian when they do DMA
accesses.

> Signed-off-by: Paolo Bonzini 
> ---
>  hw/display/omap_lcd_template.h | 4 ++--
>  hw/display/pxa2xx_lcd.c| 8 
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/display/omap_lcd_template.h b/hw/display/omap_lcd_template.h
> index e5dd447..f0ce71f 100644
> --- a/hw/display/omap_lcd_template.h
> +++ b/hw/display/omap_lcd_template.h
> @@ -136,7 +136,7 @@ static void glue(draw_line12_, DEPTH)(void *opaque,
>  uint8_t r, g, b;
>
>  do {
> -v = lduw_p((void *) s);
> +v = lduw_le_p((void *) s);
>  r = (v >> 4) & 0xf0;
>  g = v & 0xf0;
>  b = (v << 4) & 0xf0;
> @@ -159,7 +159,7 @@ static void glue(draw_line16_, DEPTH)(void *opaque,
>  uint8_t r, g, b;
>
>  do {
> -v = lduw_p((void *) s);
> +v = lduw_le_p((void *) s);
>  r = (v >> 8) & 0xf8;
>  g = (v >> 3) & 0xfc;
>  b = (v << 3) & 0xf8;
> diff --git a/hw/display/pxa2xx_lcd.c b/hw/display/pxa2xx_lcd.c
> index 494700d..4d36c94 100644
> --- a/hw/display/pxa2xx_lcd.c
> +++ b/hw/display/pxa2xx_lcd.c
> @@ -309,10 +309,10 @@ static void pxa2xx_descriptor_load(PXA2xxLCDState *s)
>  }
>
>  cpu_physical_memory_read(descptr, , sizeof(desc));
> -s->dma_ch[i].descriptor = tswap32(desc.fdaddr);
> -s->dma_ch[i].source = tswap32(desc.fsaddr);
> -s->dma_ch[i].id = tswap32(desc.fidr);
> -s->dma_ch[i].command = tswap32(desc.ldcmd);
> +s->dma_ch[i].descriptor = le32_to_cpu(desc.fdaddr);
> +s->dma_ch[i].source = le32_to_cpu(desc.fsaddr);
> +s->dma_ch[i].id = le32_to_cpu(desc.fidr);
> +s->dma_ch[i].command = le32_to_cpu(desc.ldcmd);
>  }
>  }
>
> --
> 1.8.3.1

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH] arm: soc-dma: use hwaddr instead of target_ulong in printf

[Peter Maydell]

On 4 December 2015 at 12:36, Paolo Bonzini  wrote:
>
>
> On 04/12/2015 13:33, Peter Maydell wrote:
>> On 4 December 2015 at 12:28, Paolo Bonzini  wrote:
>>> This is a first baby step towards removing widespread inclusion of
>>> cpu.h and compiling more devices once (so that arm, aarch64 and
>>> in the future target-multi can share the object files).
>>>
>>> Signed-off-by: Paolo Bonzini 
>>> ---
>>>  hw/dma/soc_dma.c | 37 -
>>>  1 file changed, 16 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
>>> index c06aabb..ac395c5 100644
>>> --- a/hw/dma/soc_dma.c
>>> +++ b/hw/dma/soc_dma.c
>>> @@ -269,11 +269,10 @@ void soc_dma_port_add_fifo(struct soc_dma_s *soc, 
>>> hwaddr virt_base,
>>>  if (entry->type == soc_dma_port_mem) {
>>>  if (entry->addr <= virt_base &&
>>>  entry->addr + entry->u.mem.size > virt_base) {
>>> -fprintf(stderr, "%s: FIFO at " TARGET_FMT_lx
>>> -" collides with RAM region at " 
>>> TARGET_FMT_lx
>>> -"-" TARGET_FMT_lx "\n", __FUNCTION__,
>>> -(target_ulong) virt_base,
>>> -(target_ulong) entry->addr, (target_ulong)
>>> +fprintf(stderr, "%s: FIFO at %"PRIx64
>>> +" collides with RAM region at %"PRIx64
>>> +"-%"PRIx64 "\n", __FUNCTION__,
>>> +virt_base, entry->addr,
>>>  (entry->addr + entry->u.mem.size));
>>
>> Is using the HWADDR_PRI* macros for printing hwaddrs deprecated now?
>
> It's the first time I hear about them. :)  There are still 130-odd
> usages of HWADDR_PRIx and friends, so at least it's an incomplete
> transition.  So I can use them if the maintainer tells me to.

Well, we needed HWADDR_PRI* when hwaddr wasn't unconditionally 64-bits
(that changed back in 2012 with commit 4be403c8158e1b6, back when it
was still called target_phys_addr_t and the macros were TARGET_PRI*).
So back in 2012 we'd have noticed uses of PRIx64 to print hwaddrs
because it would have meant a compile failure (at least in some bits
of code). But these days we probably wouldn't catch uses in code
review and they wouldn't be compile time failures.

I don't think we've ever said "we should transition away from HWADDR_*",
but whether we should is an interesting question, which is why I asked.
Does retaining the format macros to go with the typedef give us
useful flexibility, or is it just confusing?

(Also TARGET_FMT_plx, which is even more heavily used and now
rather out of step with the type name.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 00/13] acpi: Make piix-specific and q35-specific code generic

[Igor Mammedov]

On Thu, 3 Dec 2015 15:16:21 -0200
Eduardo Habkost  wrote:

> On Thu, Dec 03, 2015 at 04:19:19PM +0100, Igor Mammedov wrote:
> > On Wed,  2 Dec 2015 20:22:45 -0200
> > Eduardo Habkost  wrote:
> > 
> > > This series removes piix-specific and q35-specific code from
> > > acpi-build.c, making it generic and without direct dependencies
> > > to piix and q35 code.
> > we are conflicting reshuffling acpi-build.c at the same time
> > could be cleanup done on top of my refactoring
> > drop_ASL_support_v1 branch:
> > 
> > https://github.com/imammedo/qemu/commits/drop_ASL_support_v1
> 
> Do you plan to submit it soon?
I plan to submit it as soon as 2.5 is released since Michael is busy
with 2.5 and possibly the only reviewer.
But if it help I can submit it earlier and add you in CC.

> 
> It looks like my series and yours go in opposite directions. I am
> trying to remove piix4-specific code from acpi-build, and you are
> addding a is_piix4 field to AcpiMiscInfo. Your series makes much
> more difficult to remove piix4-specific code from acpi-build.c,
> but this sounds like a reasonable price for moving the dsdt.dsl
> logic to C. Maybe I should change my goal from "removing
> piix4-specific code" to just simplifying the code.
It should be ok to remove explicit detection of piix4/q35 from
acpi-build.c, what I wouldn't wish though is to pull ACPI deps
into boards codebase. What we have in acpi-build.c is mostly
firmware generation code and it would be better to keep it there.

After merging refactoring, I plan to simplify it by moving out
generic memory/cpu hotplug parts into hw/acpi/,
but the rest of DSDT will probably stay there and we can
merge SSDT into DSDT which also should help to simplify it.

But for now it's dumb refactoring which is byte compatible with
DSDTs we have in ASL to avoid regressions in such huge series.

> 
> I will redo this series on top of yours, taking that into
> account.
Thanks!

> 
> There are a few conflicts with the other series, but they are
> very easy to solve:
> 
> > > This series needs to be applied after the following:
> > > * [PATCH v3 0/6] pc: Initialization and compat function cleanup
> 
> No conflicts.
> 
> > > * [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
> 
> Trivial conflict at crs_range_merge().
> 
> > > * [PATCH 00/16] pc: Eliminate struct PcGuestInfo
> 
> Simple conflict at build_ssdt().
> 

[Qemu-devel] [PATCH v3 00/21] block: Cache mode for children etc.

[Kevin Wolf]

This is part three (or four, depending on whether you count the bdrv_swap
removal) of what I had sent earlier as "[PATCH 00/34] block: Cache mode for
children, reopen overhaul and more". Most of the patches were actually already
reviewed in v1.

This part contains the remaining functional changes that the cover letter for
v1 advertised, and a bit more:

- You can now use node name references for backing files
- bdrv_reopen() works now properly for inherited options (don't exist before
  this series; after the series the cache options)
- bdrv_reopen() works now properly with semantically overlapping options
- bdrv_reopen() can change child node options
- And finally you can set cache mode options for backing files and other
  children now (and the reopen behaviour even makes sense

v3:
- Patch 3 ('mirror: Error out when a BDS would get two BBs')
  Fixed typo in a comment [Max]

- Patch 4 ('block: Allow references for backing files')
  Removed meanwhile redundant assertion [Max]
  s/backing_hd/backing/ in the commit message [Max]

- Patch 6 ('block: Exclude nested options only for children in 
append_open_options()')
  g_strdup() the child name to avoid use after free [Wen Congyang]

- Patch 14 ('blockdev: Set 'format' indicates non-empty drive')
  Fix qemu commandline in a qtest case [Wen Congyang]

- Patch 20 ('qemu-iotests: Test cache mode option inheritance')
  s/backing_hd/backing/ in some comments [Max]
  Enabled commented out 'grep Cache' filter for saner output [Max]


Kevin Wolf (21):
  qcow2: Add .bdrv_join_options callback
  block: Fix reopen with semantically overlapping options
  mirror: Error out when a BDS would get two BBs
  block: Allow references for backing files
  block: Consider all block layer options in append_open_options
  block: Exclude nested options only for children in
append_open_options()
  block: Pass driver-specific options to .bdrv_refresh_filename()
  block: Keep "driver" in bs->options
  block: Allow specifying child options in reopen
  block: reopen: Document option precedence and refactor accordingly
  block: Add infrastructure for option inheritance
  block: Split out parse_json_protocol()
  block: Introduce bs->explicit_options
  blockdev: Set 'format' indicates non-empty drive
  qemu-iotests: Remove cache mode test without medium
  block: reopen: Extract QemuOpts for generic block layer options
  block: Move cache options into options QDict
  blkdebug: Enable reopen
  qemu-iotests: Try setting cache mode for children
  qemu-iotests: Test cache mode option inheritance
  qemu-iotests: Test reopen with node-name/driver options

 block.c   | 459 +++--
 block/blkdebug.c  |  24 +-
 block/blkverify.c |   2 +-
 block/mirror.c|  30 +-
 block/nbd.c   |  10 +-
 block/qcow2.c |  47 +++
 block/quorum.c|   2 +-
 blockdev.c|  57 +---
 include/block/block.h |   4 +-
 include/block/block_int.h |   8 +-
 tests/hd-geo-test.c   |   4 +-
 tests/qemu-iotests/051|  22 +-
 tests/qemu-iotests/051.out|  74 +++-
 tests/qemu-iotests/133|  90 +
 tests/qemu-iotests/133.out|  22 ++
 tests/qemu-iotests/142| 354 +++
 tests/qemu-iotests/142.out| 773 ++
 tests/qemu-iotests/group  |   2 +
 tests/qemu-iotests/iotests.py |   4 +-
 19 files changed, 1811 insertions(+), 177 deletions(-)
 create mode 100755 tests/qemu-iotests/133
 create mode 100644 tests/qemu-iotests/133.out
 create mode 100755 tests/qemu-iotests/142
 create mode 100644 tests/qemu-iotests/142.out

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 04/21] block: Allow references for backing files

[Kevin Wolf]

For bs->file, using references to existing BDSes has been possible for a
while already. This patch enables the same for bs->backing.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 block.c   | 48 +---
 block/mirror.c|  2 +-
 include/block/block.h |  3 ++-
 3 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/block.c b/block.c
index 675e5a8..0241acd 100644
--- a/block.c
+++ b/block.c
@@ -1182,30 +1182,43 @@ out:
 /*
  * Opens the backing file for a BlockDriverState if not yet open
  *
- * options is a QDict of options to pass to the block drivers, or NULL for an
- * empty set of options. The reference to the QDict is transferred to this
- * function (even on failure), so if the caller intends to reuse the 
dictionary,
- * it needs to use QINCREF() before calling bdrv_file_open.
+ * bdref_key specifies the key for the image's BlockdevRef in the options 
QDict.
+ * That QDict has to be flattened; therefore, if the BlockdevRef is a QDict
+ * itself, all options starting with "${bdref_key}." are considered part of the
+ * BlockdevRef.
+ *
+ * TODO Can this be unified with bdrv_open_image()?
  */
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
+   const char *bdref_key, Error **errp)
 {
 char *backing_filename = g_malloc0(PATH_MAX);
+char *bdref_key_dot;
+const char *reference = NULL;
 int ret = 0;
 BlockDriverState *backing_hd;
+QDict *options;
+QDict *tmp_parent_options = NULL;
 Error *local_err = NULL;
 
 if (bs->backing != NULL) {
-QDECREF(options);
 goto free_exit;
 }
 
 /* NULL means an empty set of options */
-if (options == NULL) {
-options = qdict_new();
+if (parent_options == NULL) {
+tmp_parent_options = qdict_new();
+parent_options = tmp_parent_options;
 }
 
 bs->open_flags &= ~BDRV_O_NO_BACKING;
-if (qdict_haskey(options, "file.filename")) {
+
+bdref_key_dot = g_strdup_printf("%s.", bdref_key);
+qdict_extract_subqdict(parent_options, , bdref_key_dot);
+g_free(bdref_key_dot);
+
+reference = qdict_get_try_str(parent_options, bdref_key);
+if (reference || qdict_haskey(options, "file.filename")) {
 backing_filename[0] = '\0';
 } else if (bs->backing_file[0] == '\0' && qdict_size(options) == 0) {
 QDECREF(options);
@@ -1228,19 +1241,16 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*options, Error **errp)
 goto free_exit;
 }
 
-backing_hd = bdrv_new();
-
 if (bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) {
 qdict_put(options, "driver", qstring_from_str(bs->backing_format));
 }
 
-assert(bs->backing == NULL);
+backing_hd = NULL;
 ret = bdrv_open_inherit(_hd,
 *backing_filename ? backing_filename : NULL,
-NULL, options, 0, bs, _backing, _err);
+reference, options, 0, bs, _backing,
+_err);
 if (ret < 0) {
-bdrv_unref(backing_hd);
-backing_hd = NULL;
 bs->open_flags |= BDRV_O_NO_BACKING;
 error_setg(errp, "Could not open backing file: %s",
error_get_pretty(local_err));
@@ -1253,8 +1263,11 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*options, Error **errp)
 bdrv_set_backing_hd(bs, backing_hd);
 bdrv_unref(backing_hd);
 
+qdict_del(parent_options, bdref_key);
+
 free_exit:
 g_free(backing_filename);
+QDECREF(tmp_parent_options);
 return ret;
 }
 
@@ -1537,10 +1550,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 
 /* If there is a backing file, use it */
 if ((flags & BDRV_O_NO_BACKING) == 0) {
-QDict *backing_options;
-
-qdict_extract_subqdict(options, _options, "backing.");
-ret = bdrv_open_backing_file(bs, backing_options, _err);
+ret = bdrv_open_backing_file(bs, options, "backing", _err);
 if (ret < 0) {
 goto close_and_fail;
 }
diff --git a/block/mirror.c b/block/mirror.c
index 8e3f524..fc34a9c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -652,7 +652,7 @@ static void mirror_complete(BlockJob *job, Error **errp)
 Error *local_err = NULL;
 int ret;
 
-ret = bdrv_open_backing_file(s->target, NULL, _err);
+ret = bdrv_open_backing_file(s->target, NULL, "backing", _err);
 if (ret < 0) {
 error_propagate(errp, local_err);
 return;
diff --git a/include/block/block.h b/include/block/block.h
index 3477328..589be29 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -210,7 +210,8 @@ BdrvChild *bdrv_open_child(const char *filename,
const BdrvChildRole *child_role,

[Qemu-devel] [PATCH v3 13/21] block: Introduce bs->explicit_options

[Kevin Wolf]

bs->options doesn't only contain options that the user explicitly
requested, but also option that were derived from flags, the filename or
inherited from the parent node.

For reopen, it is important to know the difference because reopening the
parent can change inherited values in child nodes, but it shouldn't
change any options that were explicitly specified for the child.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 block.c   | 24 ++--
 include/block/block.h |  1 +
 include/block/block_int.h |  1 +
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 8ac1669..2dd2830 100644
--- a/block.c
+++ b/block.c
@@ -1495,12 +1495,15 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 options = qdict_new();
 }
 
+/* json: syntax counts as explicit options, as if in the QDict */
 parse_json_protocol(options, , _err);
 if (local_err) {
 ret = -EINVAL;
 goto fail;
 }
 
+bs->explicit_options = qdict_clone_shallow(options);
+
 if (child_role) {
 bs->inherits_from = parent;
 child_role->inherit_options(, options,
@@ -1655,6 +1658,7 @@ fail:
 if (file != NULL) {
 bdrv_unref_child(bs, file);
 }
+QDECREF(bs->explicit_options);
 QDECREF(bs->options);
 QDECREF(options);
 bs->options = NULL;
@@ -1729,7 +1733,7 @@ static BlockReopenQueue 
*bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
 
 BlockReopenQueueEntry *bs_entry;
 BdrvChild *child;
-QDict *old_options;
+QDict *old_options, *explicit_options;
 
 if (bs_queue == NULL) {
 bs_queue = g_new0(BlockReopenQueue, 1);
@@ -1744,11 +1748,18 @@ static BlockReopenQueue 
*bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
  * Precedence of options:
  * 1. Explicitly passed in options (highest)
  * 2. TODO Set in flags (only for top level)
- * 3. TODO Retained from explicitly set options of bs
+ * 3. Retained from explicitly set options of bs
  * 4. Inherited from parent node
  * 5. Retained from effective options of bs
  */
 
+/* Old explicitly set values (don't overwrite by inherited value) */
+old_options = qdict_clone_shallow(bs->explicit_options);
+bdrv_join_options(bs, options, old_options);
+QDECREF(old_options);
+
+explicit_options = qdict_clone_shallow(options);
+
 /* Inherit from parent node */
 if (parent_options) {
 assert(!flags);
@@ -1787,6 +1798,7 @@ static BlockReopenQueue 
*bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
 
 bs_entry->state.bs = bs;
 bs_entry->state.options = options;
+bs_entry->state.explicit_options = explicit_options;
 bs_entry->state.flags = flags;
 
 return bs_queue;
@@ -1846,6 +1858,8 @@ cleanup:
 QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
 if (ret && bs_entry->prepared) {
 bdrv_reopen_abort(_entry->state);
+} else if (ret) {
+QDECREF(bs_entry->state.explicit_options);
 }
 QDECREF(bs_entry->state.options);
 g_free(bs_entry);
@@ -1980,6 +1994,9 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
 }
 
 /* set BDS specific flags now */
+QDECREF(reopen_state->bs->explicit_options);
+
+reopen_state->bs->explicit_options   = reopen_state->explicit_options;
 reopen_state->bs->open_flags = reopen_state->flags;
 reopen_state->bs->enable_write_cache = !!(reopen_state->flags &
   BDRV_O_CACHE_WB);
@@ -2003,6 +2020,8 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
 if (drv->bdrv_reopen_abort) {
 drv->bdrv_reopen_abort(reopen_state);
 }
+
+QDECREF(reopen_state->explicit_options);
 }
 
 
@@ -2061,6 +2080,7 @@ void bdrv_close(BlockDriverState *bs)
 bs->sg = 0;
 bs->zero_beyond_eof = false;
 QDECREF(bs->options);
+QDECREF(bs->explicit_options);
 bs->options = NULL;
 QDECREF(bs->full_open_options);
 bs->full_open_options = NULL;
diff --git a/include/block/block.h b/include/block/block.h
index 589be29..17557c3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -150,6 +150,7 @@ typedef struct BDRVReopenState {
 BlockDriverState *bs;
 int flags;
 QDict *options;
+QDict *explicit_options;
 void *opaque;
 } BDRVReopenState;
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 04daa55..6d7bd3b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -459,6 +459,7 @@ struct BlockDriverState {
 QLIST_HEAD(, BdrvChild) parents;
 
 QDict *options;
+QDict *explicit_options;
 BlockdevDetectZeroesOptions detect_zeroes;
 
 /* The error object in use for blocking operations on backing_hd */
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 21/21] qemu-iotests: Test reopen with node-name/driver options

[Kevin Wolf]

'node-name' and 'driver' should not be changed during a reopen
operation. It is, however, valid to specify them with the same value as
they already had.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 tests/qemu-iotests/133 | 90 ++
 tests/qemu-iotests/133.out | 22 
 tests/qemu-iotests/group   |  1 +
 3 files changed, 113 insertions(+)
 create mode 100755 tests/qemu-iotests/133
 create mode 100644 tests/qemu-iotests/133.out

diff --git a/tests/qemu-iotests/133 b/tests/qemu-iotests/133
new file mode 100755
index 000..8587102
--- /dev/null
+++ b/tests/qemu-iotests/133
@@ -0,0 +1,90 @@
+#!/bin/bash
+#
+# Test for reopen
+#
+# Copyright (C) 2015 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 .
+#
+
+# creator
+owner=kw...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+_make_test_img -b "$TEST_IMG.base"
+
+echo
+echo "=== Check that node-name can't be changed ==="
+echo
+
+$QEMU_IO -c 'reopen -o node-name=foo' $TEST_IMG
+$QEMU_IO -c 'reopen -o file.node-name=foo' $TEST_IMG
+$QEMU_IO -c 'reopen -o backing.node-name=foo' $TEST_IMG
+
+echo
+echo "=== Check that unchanged node-name is okay ==="
+echo
+
+# Explicitly repeated
+$QEMU_IO -c "open -o node-name=foo $TEST_IMG" -c 'reopen -o node-name=foo'
+$QEMU_IO -c "open -o file.node-name=foo $TEST_IMG" -c 'reopen -o 
file.node-name=foo'
+$QEMU_IO -c "open -o backing.node-name=foo $TEST_IMG" -c 'reopen -o 
backing.node-name=foo'
+
+# Implicitly retained
+$QEMU_IO -c "open -o node-name=foo $TEST_IMG" -c 'reopen'
+$QEMU_IO -c "open -o file.node-name=foo $TEST_IMG" -c 'reopen'
+$QEMU_IO -c "open -o backing.node-name=foo $TEST_IMG" -c 'reopen'
+
+echo
+echo "=== Check that driver can't be changed ==="
+echo
+
+$QEMU_IO -c 'reopen -o driver=raw' $TEST_IMG
+$QEMU_IO -c 'reopen -o file.driver=qcow2' $TEST_IMG
+$QEMU_IO -c 'reopen -o backing.driver=file' $TEST_IMG
+
+echo
+echo "=== Check that unchanged driver is okay ==="
+echo
+
+# Explicitly repeated (implicit case is covered in node-name test)
+$QEMU_IO -c 'reopen -o driver=qcow2' $TEST_IMG
+$QEMU_IO -c 'reopen -o file.driver=file' $TEST_IMG
+$QEMU_IO -c 'reopen -o backing.driver=qcow2' $TEST_IMG
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/133.out b/tests/qemu-iotests/133.out
new file mode 100644
index 000..cc86b94
--- /dev/null
+++ b/tests/qemu-iotests/133.out
@@ -0,0 +1,22 @@
+QA output created by 133
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
backing_file=TEST_DIR/t.IMGFMT.base
+
+=== Check that node-name can't be changed ===
+
+Cannot change the option 'node-name'
+Cannot change the option 'node-name'
+Cannot change the option 'node-name'
+
+=== Check that unchanged node-name is okay ===
+
+
+=== Check that driver can't be changed ===
+
+Cannot change the option 'driver'
+Cannot change the option 'driver'
+Cannot change the option 'driver'
+
+=== Check that unchanged driver is okay ===
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index ed6ce09..d6e9219 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -134,6 +134,7 @@
 130 rw auto quick
 131 rw auto quick
 132 rw auto quick
+133 auto quick
 134 rw auto quick
 135 rw auto
 136 rw auto
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/2] net/vmxnet3: remove redundant VMW_SHPRN(...) definition

[Eric Blake]

On 12/04/2015 02:19 AM, Miao Yan wrote:
> Macro VMW_SHPRN(...) is already defined vmxnet3_debug.h,
> so remove the duplication
> 
> Signed-off-by: Miao Yan 
> ---
>  hw/net/vmware_utils.h | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/hw/net/vmware_utils.h b/hw/net/vmware_utils.h
> index 1099df6..c2c2f90 100644
> --- a/hw/net/vmware_utils.h
> +++ b/hw/net/vmware_utils.h
> @@ -18,10 +18,7 @@
>  #define VMWARE_UTILS_H
>  
>  #include "qemu/range.h"
> -
> -#ifndef VMW_SHPRN
> -#define VMW_SHPRN(fmt, ...) do {} while (0)
> -#endif
> +#include "vmxnet_debug.h"
>  
>  /*
>   * Shared memory access functions with byte swap support
> 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 18/21] blkdebug: Enable reopen

[Kevin Wolf]

Just reopening the children (as block.c does now) is enough.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: Alberto Garcia 
---
 block/blkdebug.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 48b0812..7132e2c 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -775,6 +775,12 @@ static void blkdebug_refresh_filename(BlockDriverState 
*bs, QDict *options)
 bs->full_open_options = opts;
 }
 
+static int blkdebug_reopen_prepare(BDRVReopenState *reopen_state,
+   BlockReopenQueue *queue, Error **errp)
+{
+return 0;
+}
+
 static BlockDriver bdrv_blkdebug = {
 .format_name= "blkdebug",
 .protocol_name  = "blkdebug",
@@ -783,6 +789,7 @@ static BlockDriver bdrv_blkdebug = {
 .bdrv_parse_filename= blkdebug_parse_filename,
 .bdrv_file_open = blkdebug_open,
 .bdrv_close = blkdebug_close,
+.bdrv_reopen_prepare= blkdebug_reopen_prepare,
 .bdrv_getlength = blkdebug_getlength,
 .bdrv_truncate  = blkdebug_truncate,
 .bdrv_refresh_filename  = blkdebug_refresh_filename,
-- 
1.8.3.1

[Qemu-devel] [PATCH] xen_disk: treat "vhd" as "vpc"

[Stefano Stabellini]

The Xen toolstack uses "vhd" to specify a disk in VHD format, however
the name of the driver in QEMU is "vpc". Replace "vhd" with "vpc", so
that QEMU can find the right driver to use for it.

Signed-off-by: Stefano Stabellini 

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 267d8a8..37e14d1 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -811,6 +811,9 @@ static int blk_init(struct XenDevice *xendev)
 if (!strcmp("aio", blkdev->fileproto)) {
 blkdev->fileproto = "raw";
 }
+if (!strcmp("vhd", blkdev->fileproto)) {
+blkdev->fileproto = "vpc";
+}
 if (blkdev->mode == NULL) {
 blkdev->mode = xenstore_read_be_str(>xendev, "mode");
 }

Re: [Qemu-devel] [PATCH 1/3] hw/sd/pxa2xx_mmci: convert to SysBusDevice object

[Markus Armbruster]

Peter Maydell  writes:

> On 4 December 2015 at 12:50, Markus Armbruster  wrote:
>> To finally answer your question: the proper owner of the property
>> connecting the backend is the frontend half of the split device.  So, if
>> we have separate device models for controller and the block devices
>> connected to it, the block backend property belongs to the latter, not
>> the former.  If we have separate device models for SD controller and
>> card, the block backend property belongs to the card, not the
>> controller.
>
> I thought this might be your answer, but this is problematic because
> we now have a device in the tree (the sdhci pci card) which has the
> property on the controller, and so that's now user-facing command
> line ABI which we can't change...

As far as I can tell, device sdhci-pci hasn't been in a release, and we
can still keep it out of 2.5:

* Created in commit 224d10f, v2.3.0.

* Made unavailable in commit 1910913, v2.3.0

* Made available in commit 5ec911c, not yet released.

I'll post a patch making it unavailable again right away, to give you
options.

Re: [Qemu-devel] [Qemu-block] [PATCH v2 03/21] mirror: Error out when a BDS would get two BBs

[Kevin Wolf]

Am 30.11.2015 um 15:51 hat Alberto Garcia geschrieben:
> On Mon 23 Nov 2015 04:59:42 PM CET, Kevin Wolf wrote:
> 
> > @@ -370,11 +371,22 @@ static void mirror_exit(BlockJob *job, void *opaque)
> >  if (s->to_replace) {
> >  to_replace = s->to_replace;
> >  }
> > +
> > +/* This was checked in mirror_start_job(), but meanwhile one of the
> > + * nodes could have been newly attached to a BlockBackend. */
> > +if (to_replace->blk && s->target->blk) {
> > +error_report("block job: Can't create node with two 
> > BlockBackends");
> > +data->ret = -EINVAL;
> > +goto out;
> > +}
> 
> Does it make sense to even allow attaching a BDS to a Block Backend
> during this block job? Is there any use case for that?

Well, is there a good reason for forbidding it? I can imagine that
someone could have good reasons to start an NBD server on any node,
including nodes that are going to be replaced at some point.

The only reason for not allowing this is that a BDS can only have a
single BB, which is a limitation of the implementation rather than a
fundamental problem.

Kevin

Re: [Qemu-devel] [PATCH 1/2] net/vmxnet3: fix debug macro pattern for vmxnet3

[Eric Blake]

On 12/04/2015 02:19 AM, Miao Yan wrote:
> Vmxnet3 uses the following debug macro style:
> 

When sending a patch series, it's best to include a 0/2 cover letter
(some of the maintainer tooling works better if it can reply to the
cover letter).

Also, this patch depends on your earlier patch "net/vmxnet3.c: fix a
build error when enabling debug output" in order to not cause a
temporary compile failure; you should state that dependency so that the
patches get applied in the correct order.  (Or just make the series be 3
patches, with that one as 1/3)

>  #ifdef SOME_DEBUG
>  #  define debug(...) do{ printf(...); } while (0)
>  # else
>  #  define debug(...) do{ } while (0)
>  #endif
> 
> If SOME_DEBUG is undefined, then format string inside the
> debug macro will never be checked by compiler. Code will
> likely to break in the future when SOME_DEBUG is enabled

s/will/is

>  because of lacking of testing. This patch changes this

s/lacking/lack/

> to the following:
> 
>  #define debug(...) \
>   do { if (SOME_DEBUG_ENABLED) printf(...); } while (0)
> 
> Signed-off-by: Miao Yan 
> ---
>  hw/net/vmxnet_debug.h | 139 
> +++---
>  1 file changed, 86 insertions(+), 53 deletions(-)
> 

Changes themselves look okay, so with the commit message grammar improved,

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for 2.6 v3 1/1] qemu-char: append opt to stop truncation of serial file

[Paolo Bonzini]

On 04/12/2015 07:42, Denis V. Lunev wrote:
> From: Olga Krishtal 
> 
> Our QA team wants to preserve serial output of the guest in between QEMU
> runs to perform post-analysis.
> 
> By default this behavior is off (file is truncated each time QEMU is
> started or device is plugged).
> 
> Signed-off-by: Olga Krishtal 
> Signed-off-by: Denis V. Lunev 
> CC: Eric Blake 
> CC: Markus Armbruster 
> CC: Paolo Bonzini 
> ---
> Changes from v1:
>   - s/parallels.com/virtuozzo.com/ :(
> 
> Changes from v2:
>   - fixed QAPI description
>   - added has_append filling and checking
> 
> qapi-schema.json |  5 -
>  qemu-char.c  | 14 +-
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 18c9a6c..89201a9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3020,11 +3020,14 @@
>  #
>  # @in:  #optional The name of the input file
>  # @out: The name of the output file
> +# @append: #optional Open the file in append mode (default false to
> +#  truncate) (Since 2.6)
>  #
>  # Since: 1.4
>  ##
>  { 'struct': 'ChardevFile', 'data': { '*in' : 'str',
> -   'out' : 'str' } }
> +   'out' : 'str',
> +   '*append': 'bool' } }
>  
>  ##
>  # @ChardevHostdev:
> diff --git a/qemu-char.c b/qemu-char.c
> index 2969c44..66703e3 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3484,6 +3484,9 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, 
> ChardevBackend *backend,
>  }
>  backend->u.file = g_new0(ChardevFile, 1);
>  backend->u.file->out = g_strdup(path);
> +
> +backend->u.file->has_append = true;
> +backend->u.file->append = qemu_opt_get_bool(opts, "append", false);
>  }
>  
>  static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend,
> @@ -4041,6 +4044,9 @@ QemuOptsList qemu_chardev_opts = {
>  },{
>  .name = "chardev",
>  .type = QEMU_OPT_STRING,
> +},{
> +.name = "append",
> +.type = QEMU_OPT_BOOL,
>  },
>  { /* end of list */ }
>  },
> @@ -4101,7 +4107,13 @@ static CharDriverState *qmp_chardev_open_file(const 
> char *id,
>  ChardevFile *file = backend->u.file;
>  int flags, in = -1, out;
>  
> -flags = O_WRONLY | O_TRUNC | O_CREAT | O_BINARY;
> +flags = O_WRONLY | O_CREAT | O_BINARY;
> +if (file->has_append && file->append) {
> +flags |= O_APPEND;
> +} else {
> +flags |= O_TRUNC;
> +}
> +
>  out = qmp_chardev_open_file_source(file->out, flags, errp);
>  if (out < 0) {
>  return NULL;
> 

Queued for 2.6, thanks.

Paolo

[Qemu-devel] [PATCH v3 09/21] block: Allow specifying child options in reopen

[Kevin Wolf]

If the child was defined in the same context (-drive argument or
blockdev-add QMP command) as its parent, a reopen of the parent should
work the same and allow changing options of the child.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: Alberto Garcia 
---
 block.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 2728bc5..a800d9e 100644
--- a/block.c
+++ b/block.c
@@ -1720,15 +1720,23 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue 
*bs_queue,
 flags &= ~BDRV_O_PROTOCOL;
 
 QLIST_FOREACH(child, >children, next) {
+QDict *new_child_options;
+char *child_key_dot;
 int child_flags;
 
+/* reopen can only change the options of block devices that were
+ * implicitly created and inherited options. For other (referenced)
+ * block devices, a syntax like "backing.foo" results in an error. */
 if (child->bs->inherits_from != bs) {
 continue;
 }
 
+child_key_dot = g_strdup_printf("%s.", child->name);
+qdict_extract_subqdict(options, _child_options, child_key_dot);
+g_free(child_key_dot);
+
 child_flags = child->role->inherit_flags(flags);
-/* TODO Pass down child flags (backing.*, extents.*, ...) */
-bdrv_reopen_queue(bs_queue, child->bs, NULL, child_flags);
+bdrv_reopen_queue(bs_queue, child->bs, new_child_options, child_flags);
 }
 
 bs_entry = g_new0(BlockReopenQueueEntry, 1);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 16/21] block: reopen: Extract QemuOpts for generic block layer options

[Kevin Wolf]

This patch adds a QemuOpts for generic block layer options to
bdrv_reopen_prepare(). The only two options that currently exist
(node-name and driver) cannot be changed, so the only thing we do is
putting them right back into the QDict so that we check at the end that
they are indeed unchanged.

We will add new options soon that can actually be changed.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 block.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/block.c b/block.c
index 2dd2830..07763ff 100644
--- a/block.c
+++ b/block.c
@@ -1907,11 +1907,34 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
 int ret = -1;
 Error *local_err = NULL;
 BlockDriver *drv;
+QemuOpts *opts;
+const char *value;
 
 assert(reopen_state != NULL);
 assert(reopen_state->bs->drv != NULL);
 drv = reopen_state->bs->drv;
 
+/* Process generic block layer options */
+opts = qemu_opts_create(_runtime_opts, NULL, 0, _abort);
+qemu_opts_absorb_qdict(opts, reopen_state->options, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto error;
+}
+
+/* node-name and driver must be unchanged. Put them back into the QDict, so
+ * that they are checked at the end of this function. */
+value = qemu_opt_get(opts, "node-name");
+if (value) {
+qdict_put(reopen_state->options, "node-name", qstring_from_str(value));
+}
+
+value = qemu_opt_get(opts, "driver");
+if (value) {
+qdict_put(reopen_state->options, "driver", qstring_from_str(value));
+}
+
 /* if we are to stay read-only, do not allow permission change
  * to r/w */
 if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
@@ -1972,6 +1995,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
 ret = 0;
 
 error:
+qemu_opts_del(opts);
 return ret;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 20/21] qemu-iotests: Test cache mode option inheritance

[Kevin Wolf]

This is doing a more complete test on setting cache modes both while
opening an image (i.e. in a -drive command line) and in reopen
situations. It checks that reopen can specify options for child nodes
and that cache modes are correctly inherited from parent nodes where
they are not specified.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 tests/qemu-iotests/142 | 354 +
 tests/qemu-iotests/142.out | 773 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 1128 insertions(+)
 create mode 100755 tests/qemu-iotests/142
 create mode 100644 tests/qemu-iotests/142.out

diff --git a/tests/qemu-iotests/142 b/tests/qemu-iotests/142
new file mode 100755
index 000..8aa50f8
--- /dev/null
+++ b/tests/qemu-iotests/142
@@ -0,0 +1,354 @@
+#!/bin/bash
+#
+# Test for configuring cache modes of arbitrary nodes (requires O_DIRECT)
+#
+# Copyright (C) 2015 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 .
+#
+
+# creator
+owner=kw...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+rm -f $TEST_IMG.snap
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# We test all cache modes anyway, but O_DIRECT needs to be supported
+_default_cache_mode none
+_supported_cache_modes none directsync
+
+function do_run_qemu()
+{
+echo Testing: "$@"
+(
+if ! test -t 0; then
+while read cmd; do
+echo $cmd
+done
+fi
+echo quit
+) | $QEMU -nographic -monitor stdio -nodefaults "$@"
+echo
+}
+
+function run_qemu()
+{
+do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu
+}
+
+size=128M
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $size
+TEST_IMG="$TEST_IMG.snap" _make_test_img $size
+_make_test_img -b "$TEST_IMG.base" $size
+
+echo
+echo === Simple test for all cache modes ===
+echo
+
+run_qemu -drive file="$TEST_IMG",cache=none
+run_qemu -drive file="$TEST_IMG",cache=directsync
+run_qemu -drive file="$TEST_IMG",cache=writeback
+run_qemu -drive file="$TEST_IMG",cache=writethrough
+run_qemu -drive file="$TEST_IMG",cache=unsafe
+run_qemu -drive file="$TEST_IMG",cache=invalid_value
+
+echo
+echo === Check inheritance of cache modes ===
+echo
+
+files="if=none,file=$TEST_IMG,backing.file.filename=$TEST_IMG.base"
+ids="node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file"
+
+function check_cache_all()
+{
+# cache.direct is supposed to be inherited by both bs->file and
+# bs->backing
+
+echo -e "cache.direct=on on none0"
+echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.direct=on | grep 
"Cache"
+echo -e "\ncache.direct=on on file"
+echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.direct=on | 
grep "Cache"
+echo -e "\ncache.direct=on on backing"
+echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.direct=on 
| grep "Cache"
+echo -e "\ncache.direct=on on backing-file"
+echo "$hmp_cmds" | run_qemu -drive 
"$files","$ids",backing.file.cache.direct=on | grep "Cache"
+
+# cache.writeback is supposed to be inherited by bs->backing; bs->file
+# always gets cache.writeback=on
+
+echo -e "\n\ncache.writeback=off on none0"
+echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.writeback=off | 
grep "Cache"
+echo -e "\ncache.writeback=off on file"
+echo "$hmp_cmds" | run_qemu -drive 
"$files","$ids",file.cache.writeback=off | grep "Cache"
+echo -e "\ncache.writeback=off on backing"
+echo "$hmp_cmds" | run_qemu -drive 
"$files","$ids",backing.cache.writeback=off | grep "Cache"
+echo -e "\ncache.writeback=off on backing-file"
+echo "$hmp_cmds" | run_qemu -drive 
"$files","$ids",backing.file.cache.writeback=off | grep "Cache"
+
+# cache.no-flush is supposed to be inherited by both bs->file and 
bs->backing
+
+echo -e "\n\ncache.no-flush=on on none0"
+echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.no-flush=on | 
grep "Cache"
+echo -e "\ncache.no-flush=on on file"
+echo "$hmp_cmds" | run_qemu -drive 

[Qemu-devel] [PATCH v3 19/21] qemu-iotests: Try setting cache mode for children

[Kevin Wolf]

This is a basic test for specifying cache modes for child nodes on the
command line. It doesn't take much time and works without O_DIRECT
support.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 tests/qemu-iotests/051 | 10 +++-
 tests/qemu-iotests/051.out | 60 ++
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index f6f0f4d..da90f59 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -61,7 +61,7 @@ function do_run_qemu()
 
 function run_qemu()
 {
-do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu
+do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | 
_filter_generated_node_ids
 }
 
 size=128M
@@ -190,6 +190,14 @@ run_qemu -drive driver=null-co,cache=writethrough
 run_qemu -drive driver=null-co,cache=unsafe
 run_qemu -drive driver=null-co,cache=invalid_value
 
+# Can't test direct=on here because O_DIRECT might not be supported on this FS
+# Test 142 checks the direct=on cases
+
+for cache in writeback writethrough unsafe invalid_value; do
+echo -e "info block\ninfo block file\ninfo block backing\ninfo block 
backing-file" | \
+run_qemu -drive 
file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file
 -nodefaults
+done
+
 echo
 echo === Specifying the protocol layer ===
 echo
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 7a459a3..070d318 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -238,6 +238,66 @@ QEMU X.Y.Z monitor - type 'help' for more information
 Testing: -drive driver=null-co,cache=invalid_value
 QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
 
+Testing: -drive 
file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file
 -nodefaults
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) iininfinfoinfo 
info binfo 
blinfo bloinfo 
blocinfo block
+ide0-hd0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+Cache mode:   writeback
+Backing file: TEST_DIR/t.qcow2.base (chain depth: 1)
+(qemu) iininfinfoinfo 
info binfo 
blinfo bloinfo 
blocinfo 
blockinfo block 
info block 
finfo block 
fiinfo block 
filinfo block file
+
+file: TEST_DIR/t.qcow2 (file)
+Cache mode:   writeback
+(qemu) iininfinfoinfo 
info binfo 
blinfo bloinfo 
blocinfo 
blockinfo block 
info block 
binfo block 
bainfo block 
bacinfo block 
backinfo block 
backiinfo block 
backininfo block 
backing
+backing: TEST_DIR/t.qcow2.base (qcow2, read-only)
+Cache mode:   writeback, ignore flushes
+(qemu) iininfinfoinfo 
info binfo 
blinfo bloinfo 
blocinfo 
blockinfo block 
info block 
binfo block 
bainfo block 
bacinfo block 
backinfo block 
backiinfo block 
backininfo block 
backinginfo block 
backing-info block 
backing-finfo 
block backing-fi!
 info block 

Re: [Qemu-devel] [PATCH 00/13] acpi: Make piix-specific and q35-specific code generic

[Eduardo Habkost]

On Fri, Dec 04, 2015 at 02:24:41PM +0100, Igor Mammedov wrote:
> On Thu, 3 Dec 2015 15:16:21 -0200
> Eduardo Habkost  wrote:
> 
> > On Thu, Dec 03, 2015 at 04:19:19PM +0100, Igor Mammedov wrote:
> > > On Wed,  2 Dec 2015 20:22:45 -0200
> > > Eduardo Habkost  wrote:
> > > 
> > > > This series removes piix-specific and q35-specific code from
> > > > acpi-build.c, making it generic and without direct dependencies
> > > > to piix and q35 code.
> > > we are conflicting reshuffling acpi-build.c at the same time
> > > could be cleanup done on top of my refactoring
> > > drop_ASL_support_v1 branch:
> > > 
> > > https://github.com/imammedo/qemu/commits/drop_ASL_support_v1
> > 
> > Do you plan to submit it soon?
> I plan to submit it as soon as 2.5 is released since Michael is busy
> with 2.5 and possibly the only reviewer.
> But if it help I can submit it earlier and add you in CC.

I have been submitting for-2.6 patches, already. It helps get the
patches reviewed earlier so they have a chance to get applied as
soon as 2.5 is released.

> 
> > 
> > It looks like my series and yours go in opposite directions. I am
> > trying to remove piix4-specific code from acpi-build, and you are
> > addding a is_piix4 field to AcpiMiscInfo. Your series makes much
> > more difficult to remove piix4-specific code from acpi-build.c,
> > but this sounds like a reasonable price for moving the dsdt.dsl
> > logic to C. Maybe I should change my goal from "removing
> > piix4-specific code" to just simplifying the code.
> It should be ok to remove explicit detection of piix4/q35 from
> acpi-build.c, what I wouldn't wish though is to pull ACPI deps
> into boards codebase. What we have in acpi-build.c is mostly
> firmware generation code and it would be better to keep it there.

Agreed. And your refactoring makes the piix4/q35 differences much
more visible.

After making the piix4/q35 differences visible, I wonder if we
could make the reason for each "if (piix4)" line clearer. For
example, I assume some of the "if (piix4)" lines could become
"if (acpi_pci_hotplug_enabled())", but I don't know which ones.

> 
> After merging refactoring, I plan to simplify it by moving out
> generic memory/cpu hotplug parts into hw/acpi/,
> but the rest of DSDT will probably stay there and we can
> merge SSDT into DSDT which also should help to simplify it.
> 
> But for now it's dumb refactoring which is byte compatible with
> DSDTs we have in ASL to avoid regressions in such huge series.

I will let you make the cleanups you are planning, first. After
that is settled, I will check which of the patches in this series
still make sense. My original plan was to only suggest 1 or 2
simple cleanups after a discussion with Marcel. :)

-- 
Eduardo

Re: [Qemu-devel] [PATCH] sdhci: Make device "sdhci-pci" unavailable with -device again

[Paolo Bonzini]

On 04/12/2015 15:07, Markus Armbruster wrote:
> We made it unavailable in commit 1910913 because its use of
> drive_get_next() instead of a property.  Commit 5ec911c replaced
> drive_get_next() and made the device available, but the property isn't
> quite right, and the code dangerously ignores blk_attach_dev()
> failure.  Disable it again before the property becomes ABI, and mark
> the dangerous spot FIXME.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  hw/sd/sd.c| 1 +
>  hw/sd/sdhci.c | 6 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index ce4d44b..d0be5ea 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -494,6 +494,7 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
>  if (sd->blk) {
>  /* Attach dev if not already attached.  (This call ignores an
>   * error return code if sd->blk is already attached.) */
> +/* FIXME ignoring blk_attach_dev() failure is wrong and dangerous */

No, it's not (it is tricky though) because blk_attach_dev actually will
always  fail here when using the drive= property, and never when using
drive_get_next.

In the drive= case, the successful call (and also the one that will
catch possible mistakes) is from parse_drive in 
hw/core/qdev-properties-system.c:

$ x86_64-softmmu/qemu-system-x86_64 -drive if=none,driver=null-aio,id=foo 
-device virtio-blk-pci,drive=foo -device sdhci-pci,drive=foo
qemu-system-x86_64: -device sdhci-pci,drive=foo: Drive 'foo' is already in use 
by another device

Did you have something else in mind?

Paolo

>  blk_attach_dev(sd->blk, sd);
>  blk_set_dev_ops(sd->blk, _block_ops, sd);
>  }
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index d70d1a6..0a338ed 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1261,6 +1261,12 @@ static void sdhci_pci_class_init(ObjectClass *klass, 
> void *data)
>  set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>  dc->vmsd = _vmstate;
>  dc->props = sdhci_pci_properties;
> +/*
> + * Reason: realize() method uses sd_init(), which ignores
> + * blk_attach_dev() failure (potentially dangerous), and the block
> + * properties really belong to the card, not the controller.
> + */
> +dc->cannot_instantiate_with_device_add_yet = true;
>  }
>  
>  static const TypeInfo sdhci_pci_info = {
> 

Re: [Qemu-devel] [PATCH v2 1/9] drivers/hv: replace enum hv_message_type by u32

[Denis V. Lunev]

On 12/02/2015 03:22 PM, Paolo Bonzini wrote:


On 30/11/2015 17:22, Andrey Smetanin wrote:

enum hv_message_type inside struct hv_message, hv_post_message
is not size portable. Replace enum by u32.

It's only non-portable inside structs.  Okay to apply just these:

@@ -172,7 +174,7 @@ union hv_message_flags {

  /* Define synthetic interrupt controller message header. */
  struct hv_message_header {
-   u32 message_type;
+   enum hv_message_type message_type;
u8 payload_size;
union hv_message_flags message_flags;
u8 reserved[2];
@@ -345,7 +347,7 @@ enum hv_call_code {
  struct hv_input_post_message {
union hv_connection_id connectionid;
u32 reserved;
-   u32 message_type;
+   enum hv_message_type message_type;
u32 payload_size;
u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
  };

?

Paolo

sorry for the delay.

Andrey is on vacation till the end of the week.

This could be not enough for some compilers as this exact
enum could be signed and unsigned depends upon the
implementation of the compiler and if it is signed we
can face signed/unsigned comparison in ifs.

Vitaly, by the way, this code is a bit rotten. The only caller of
hv_post_message calls it with message type exactly written
as "1", which is not defined in the enum.

/*
 * vmbus_post_msg - Send a msg on the vmbus's message connection
 */
int vmbus_post_msg(void *buffer, size_t buflen)
{
...
ret = hv_post_message(conn_id, 1, buffer, buflen);

Den

[Qemu-devel] [PATCH 4/5] migration: improve error reporting for load_vmstate

[Denis V. Lunev]

The patch adds Error ** parameter to load_vmstate call and fills error
inside. The caller after that properly reports error either through
monitor or via local stderr facility during VM start.

This helper will be useful too for qmp_loadvm implementation.

Signed-off-by: Denis V. Lunev 
CC: Juan Quintela 
CC: Amit Shah 
CC: Markus Armbruster 
CC: Eric Blake 
---
 include/sysemu/sysemu.h |  2 +-
 migration/savevm.c  | 28 
 monitor.c   |  6 +-
 vl.c|  4 +++-
 4 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 3bb8897..d9ccf45 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -78,7 +78,7 @@ void qemu_remove_exit_notifier(Notifier *notify);
 void qemu_add_machine_init_done_notifier(Notifier *notify);
 
 void hmp_savevm(Monitor *mon, const QDict *qdict);
-int load_vmstate(const char *name);
+int load_vmstate(const char *name, Error **errp);
 void hmp_delvm(Monitor *mon, const QDict *qdict);
 void hmp_info_snapshots(Monitor *mon, const QDict *qdict);
 
diff --git a/migration/savevm.c b/migration/savevm.c
index f87a061..7846437 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2026,7 +2026,7 @@ void qmp_xen_save_devices_state(const char *filename, 
Error **errp)
 }
 }
 
-int load_vmstate(const char *name)
+int load_vmstate(const char *name, Error **errp)
 {
 BlockDriverState *bs, *bs_vm_state;
 QEMUSnapshotInfo sn;
@@ -2035,20 +2035,22 @@ int load_vmstate(const char *name)
 AioContext *aio_context;
 
 if (!bdrv_all_can_snapshot()) {
-error_report("Device '%s' is writable but does not support snapshots.",
- bdrv_get_device_name(bs));
+error_setg(errp,
+   "Device '%s' is writable but does not support snapshots.",
+   bdrv_get_device_name(bs));
 return -ENOTSUP;
 }
 ret = bdrv_all_find_snapshot(name, );
 if (ret < 0) {
-error_report("Device '%s' does not have the requested snapshot '%s'",
- bdrv_get_device_name(bs), name);
+error_setg(errp,
+   "Device '%s' does not have the requested snapshot '%s'",
+   bdrv_get_device_name(bs), name);
 return ret;
 }
 
 bs_vm_state = bdrv_all_find_vmstate_bs();
 if (!bs_vm_state) {
-error_report("No block device supports snapshots");
+error_setg(errp, "No block device supports snapshots");
 return -ENOTSUP;
 }
 aio_context = bdrv_get_aio_context(bs_vm_state);
@@ -2058,10 +2060,11 @@ int load_vmstate(const char *name)
 ret = bdrv_snapshot_find(bs_vm_state, , name);
 aio_context_release(aio_context);
 if (ret < 0) {
+error_setg_errno(errp, ret, "Snapshot '%s' not found", name);
 return ret;
 } else if (sn.vm_state_size == 0) {
-error_report("This is a disk-only snapshot. Revert to it offline "
-"using qemu-img.");
+error_setg(errp, "This is a disk-only snapshot. Revert to it offline "
+   "using qemu-img.");
 return -EINVAL;
 }
 
@@ -2070,15 +2073,16 @@ int load_vmstate(const char *name)
 
 ret = bdrv_all_goto_snapshot(name, );
 if (ret < 0) {
-error_report("Error %d while activating snapshot '%s' on '%s'",
- ret, name, bdrv_get_device_name(bs));
+error_setg_errno(errp, ret,
+ "Error while activating snapshot '%s' on '%s'",
+ name, bdrv_get_device_name(bs));
 return ret;
 }
 
 /* restore the VM state */
 f = qemu_fopen_bdrv(bs_vm_state, 0);
 if (!f) {
-error_report("Could not open VM state file");
+error_setg(errp, "Could not open VM state file");
 return -EINVAL;
 }
 
@@ -2092,7 +2096,7 @@ int load_vmstate(const char *name)
 
 migration_incoming_state_destroy();
 if (ret < 0) {
-error_report("Error %d while loading VM state", ret);
+error_setg_errno(errp, ret, "Error while loading VM state");
 return ret;
 }
 
diff --git a/monitor.c b/monitor.c
index 9a35d72..3857724 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1739,10 +1739,14 @@ static void hmp_loadvm(Monitor *mon, const QDict *qdict)
 {
 int saved_vm_running  = runstate_is_running();
 const char *name = qdict_get_str(qdict, "name");
+Error *local_err = NULL;
 
 vm_stop(RUN_STATE_RESTORE_VM);
 
-if (load_vmstate(name) == 0 && saved_vm_running) {
+if (load_vmstate(name, _err) < 0) {
+error_report_err(local_err);
+}
+if (saved_vm_running) {
 vm_start();
 }
 }
diff --git a/vl.c b/vl.c
index 4211ff1..ee34edb 100644
--- a/vl.c
+++ b/vl.c
@@ -4648,8 +4648,10 @@ int main(int argc, char **argv, char **envp)
 

[Qemu-devel] [PATCH 3/5] qmp: create qmp_delvm command

[Denis V. Lunev]

The patch also moves hmp_delvm implementation into hmp.c. This function
is just a simple wrapper now and does not have knowledge about
migration internals.

Signed-off-by: Denis V. Lunev 
CC: Juan Quintela 
CC: Amit Shah 
CC: Markus Armbruster 
CC: Eric Blake 
---
 hmp.c  | 11 +++
 migration/savevm.c | 16 +++-
 qapi-schema.json   | 13 +
 qmp-commands.hx| 23 +++
 4 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/hmp.c b/hmp.c
index c9c7100..a342c8c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2390,3 +2390,14 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
 error_report_err(local_err);
 }
 }
+
+void hmp_delvm(Monitor *mon, const QDict *qdict)
+{
+Error *local_err = NULL;
+
+qmp_delvm(qdict_get_str(qdict, "name"), _err);
+
+if (local_err != NULL) {
+error_report_err(local_err);
+}
+}
diff --git a/migration/savevm.c b/migration/savevm.c
index 65e0081..f87a061 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2099,17 +2099,15 @@ int load_vmstate(const char *name)
 return 0;
 }
 
-void hmp_delvm(Monitor *mon, const QDict *qdict)
+void qmp_delvm(const char *name, Error **errp)
 {
 BlockDriverState *bs;
-Error *err;
-const char *name = qdict_get_str(qdict, "name");
-
-if (bdrv_all_delete_snapshot(name, , ) < 0) {
-monitor_printf(mon,
-   "Error while deleting snapshot on device '%s': %s\n",
-   bdrv_get_device_name(bs), error_get_pretty(err));
-error_free(err);
+Error *local_err = NULL;
+
+if (bdrv_all_delete_snapshot(name, , _err) < 0) {
+error_setg(errp, "Error while deleting snapshot on device '%s': %s",
+   bdrv_get_device_name(bs), error_get_pretty(local_err));
+error_free(local_err);
 }
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 0114132..18c9a6c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3984,3 +3984,16 @@
 # Since 2.6
 ##
 { 'command': 'savevm', 'data': {'name': 'str'} }
+
+##
+# @delvm
+#
+# Delete a VM snapshot
+#
+# @name: identifier of a snapshot to be deleted
+#
+# Returns: Nothing on success
+#
+# Since 2.6
+##
+{ 'command': 'delvm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index f216c5e..a630e8a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4764,3 +4764,26 @@ EQMP
 .args_type  = "name:s?",
 .mhandler.cmd_new = qmp_marshal_savevm,
 },
+
+SQMP
+delvm
+--
+
+Delete a VM snapshot
+
+Arguments:
+
+- "name": snapshot name
+
+Example:
+
+-> { "execute": "delvm", "arguments": { "name": "snapshot1" } }
+<- { "return": {} }
+
+EQMP
+
+{
+.name   = "delvm",
+.args_type  = "name:s",
+.mhandler.cmd_new = qmp_marshal_delvm,
+},
-- 
2.5.0

[Qemu-devel] [PATCH v3 02/21] block: Fix reopen with semantically overlapping options

[Kevin Wolf]

This fixes bdrv_reopen() calls like the following one:

qemu-io -c 'open -o overlap-check.template=all /tmp/test.qcow2' \
-c 'reopen -o overlap-check=none'

The approach taken so far would result in an options QDict that has both
"overlap-check.template=all" and "overlap-check=none", which obviously
conflicts. In this case, the old option should be overridden by the
newly specified option.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: Alberto Garcia 
---
 block.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 3a7324b..675e5a8 100644
--- a/block.c
+++ b/block.c
@@ -624,6 +624,20 @@ static int refresh_total_sectors(BlockDriverState *bs, 
int64_t hint)
 }
 
 /**
+ * Combines a QDict of new block driver @options with any missing options taken
+ * from @old_options, so that leaving out an option defaults to its old value.
+ */
+static void bdrv_join_options(BlockDriverState *bs, QDict *options,
+  QDict *old_options)
+{
+if (bs->drv && bs->drv->bdrv_join_options) {
+bs->drv->bdrv_join_options(options, old_options);
+} else {
+qdict_join(options, old_options, false);
+}
+}
+
+/**
  * Set open flags for a given discard mode
  *
  * Return 0 on success, -1 if the discard mode was invalid.
@@ -1663,7 +1677,7 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue 
*bs_queue,
 }
 
 old_options = qdict_clone_shallow(bs->options);
-qdict_join(options, old_options, false);
+bdrv_join_options(bs, options, old_options);
 QDECREF(old_options);
 
 /* bdrv_open() masks this flag out */
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 17/21] block: Move cache options into options QDict

[Kevin Wolf]

This adds the cache mode options to the QDict, so that they can be
specified for child nodes (e.g. backing.cache.direct=off).

The cache modes are not removed from the flags at this point; instead,
options and flags are kept in sync. If the user specifies both flags and
options, the options take precedence.

Child node inherit cache modes as options now, they don't use flags any
more.

Note that this forbids specifying the cache mode for empty drives. It
didn't make sense anyway to specify it there, because it didn't have any
effect. blockdev_init() considers the cache options now bdrv_open()
options and therefore doesn't create an empty drive any more but calls
into bdrv_open(). This in turn will fail with no driver and filename
specified.

Signed-off-by: Kevin Wolf 
---
 block.c| 100 ++---
 blockdev.c |  52 ++--
 2 files changed, 111 insertions(+), 41 deletions(-)

diff --git a/block.c b/block.c
index 07763ff..99db955 100644
--- a/block.c
+++ b/block.c
@@ -29,6 +29,7 @@
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "qapi/qmp/qerror.h"
+#include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qjson.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/sysemu.h"
@@ -706,9 +707,16 @@ static void bdrv_inherited_options(int *child_flags, QDict 
*child_options,
 /* Enable protocol handling, disable format probing for bs->file */
 flags |= BDRV_O_PROTOCOL;
 
+/* If the cache mode isn't explicitly set, inherit direct and no-flush from
+ * the parent. */
+qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
+qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
+
 /* Our block drivers take care to send flushes and respect unmap policy,
- * so we can enable both unconditionally on lower layers. */
-flags |= BDRV_O_CACHE_WB | BDRV_O_UNMAP;
+ * so we can default to enable both on lower layers regardless of the
+ * corresponding parent options. */
+qdict_set_default_str(child_options, BDRV_OPT_CACHE_WB, "on");
+flags |= BDRV_O_UNMAP;
 
 /* Clear flags that only apply to the top layer */
 flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ);
@@ -747,6 +755,11 @@ static void bdrv_backing_options(int *child_flags, QDict 
*child_options,
 {
 int flags = parent_flags;
 
+/* The cache mode is inherited unmodified for backing files */
+qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_WB);
+qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
+qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
+
 /* backing files always opened read-only */
 flags &= ~(BDRV_O_RDWR | BDRV_O_COPY_ON_READ);
 
@@ -780,6 +793,42 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
 return open_flags;
 }
 
+static void update_flags_from_options(int *flags, QemuOpts *opts)
+{
+*flags &= ~BDRV_O_CACHE_MASK;
+
+assert(qemu_opt_find(opts, BDRV_OPT_CACHE_WB));
+if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, false)) {
+*flags |= BDRV_O_CACHE_WB;
+}
+
+assert(qemu_opt_find(opts, BDRV_OPT_CACHE_NO_FLUSH));
+if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) {
+*flags |= BDRV_O_NO_FLUSH;
+}
+
+assert(qemu_opt_find(opts, BDRV_OPT_CACHE_DIRECT));
+if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_DIRECT, false)) {
+*flags |= BDRV_O_NOCACHE;
+}
+}
+
+static void update_options_from_flags(QDict *options, int flags)
+{
+if (!qdict_haskey(options, BDRV_OPT_CACHE_WB)) {
+qdict_put(options, BDRV_OPT_CACHE_WB,
+  qbool_from_bool(flags & BDRV_O_CACHE_WB));
+}
+if (!qdict_haskey(options, BDRV_OPT_CACHE_DIRECT)) {
+qdict_put(options, BDRV_OPT_CACHE_DIRECT,
+  qbool_from_bool(flags & BDRV_O_NOCACHE));
+}
+if (!qdict_haskey(options, BDRV_OPT_CACHE_NO_FLUSH)) {
+qdict_put(options, BDRV_OPT_CACHE_NO_FLUSH,
+  qbool_from_bool(flags & BDRV_O_NO_FLUSH));
+}
+}
+
 static void bdrv_assign_node_name(BlockDriverState *bs,
   const char *node_name,
   Error **errp)
@@ -831,6 +880,21 @@ static QemuOptsList bdrv_runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "Block driver to use for the node",
 },
+{
+.name = BDRV_OPT_CACHE_WB,
+.type = QEMU_OPT_BOOL,
+.help = "Enable writeback mode",
+},
+{
+.name = BDRV_OPT_CACHE_DIRECT,
+.type = QEMU_OPT_BOOL,
+.help = "Bypass software writeback cache on the host",
+},
+{
+.name = BDRV_OPT_CACHE_NO_FLUSH,
+.type = QEMU_OPT_BOOL,
+.help = "Ignore flush requests",
+},
 { /* end of list */ 

[Qemu-devel] [PATCH v3 06/21] block: Exclude nested options only for children in append_open_options()

[Kevin Wolf]

Some drivers have nested options (e.g. blkdebug rule arrays), which
don't belong to a child node and shouldn't be removed. Don't remove all
options with "." in their name, but check for the complete prefixes of
actually existing child nodes.

Signed-off-by: Kevin Wolf 
---
 block.c   | 20 
 include/block/block_int.h |  1 +
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 73f0816..0dfff7a 100644
--- a/block.c
+++ b/block.c
@@ -1101,11 +1101,13 @@ static int bdrv_fill_options(QDict **options, const 
char **pfilename,
 
 static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
 BlockDriverState *child_bs,
+const char *child_name,
 const BdrvChildRole *child_role)
 {
 BdrvChild *child = g_new(BdrvChild, 1);
 *child = (BdrvChild) {
 .bs = child_bs,
+.name   = g_strdup(child_name),
 .role   = child_role,
 };
 
@@ -1119,6 +1121,7 @@ static void bdrv_detach_child(BdrvChild *child)
 {
 QLIST_REMOVE(child, next);
 QLIST_REMOVE(child, next_parent);
+g_free(child->name);
 g_free(child);
 }
 
@@ -1165,7 +1168,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
BlockDriverState *backing_hd)
 bs->backing = NULL;
 goto out;
 }
-bs->backing = bdrv_attach_child(bs, backing_hd, _backing);
+bs->backing = bdrv_attach_child(bs, backing_hd, "backing", _backing);
 bs->open_flags &= ~BDRV_O_NO_BACKING;
 pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
 pstrcpy(bs->backing_format, sizeof(bs->backing_format),
@@ -1321,7 +1324,7 @@ BdrvChild *bdrv_open_child(const char *filename,
 goto done;
 }
 
-c = bdrv_attach_child(parent, bs, child_role);
+c = bdrv_attach_child(parent, bs, bdref_key, child_role);
 
 done:
 qdict_del(options, bdref_key);
@@ -3951,13 +3954,22 @@ static bool append_open_options(QDict *d, 
BlockDriverState *bs)
 {
 const QDictEntry *entry;
 QemuOptDesc *desc;
+BdrvChild *child;
 bool found_any = false;
+const char *p;
 
 for (entry = qdict_first(bs->options); entry;
  entry = qdict_next(bs->options, entry))
 {
-/* Only take options for this level */
-if (strchr(qdict_entry_key(entry), '.')) {
+/* Exclude options for children */
+QLIST_FOREACH(child, >children, next) {
+if (strstart(qdict_entry_key(entry), child->name, )
+&& (!*p || *p == '.'))
+{
+break;
+}
+}
+if (child) {
 continue;
 }
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 77dc165..7265247 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -351,6 +351,7 @@ extern const BdrvChildRole child_format;
 
 struct BdrvChild {
 BlockDriverState *bs;
+char *name;
 const BdrvChildRole *role;
 QLIST_ENTRY(BdrvChild) next;
 QLIST_ENTRY(BdrvChild) next_parent;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 14/21] blockdev: Set 'format' indicates non-empty drive

[Kevin Wolf]

Creating an empty drive while specifying 'format' doesn't make sense.
The specified format driver would simply be ignored.

Make a set 'format' option an indication that a non-empty drive should
be created. This makes 'format' consistent with 'driver' and allows
using it with a block driver that doesn't need any other options (like
null-co/null-aio).

Signed-off-by: Kevin Wolf 
---
 blockdev.c| 5 +
 tests/hd-geo-test.c   | 4 ++--
 tests/qemu-iotests/iotests.py | 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 313841b..afaeef9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -490,7 +490,6 @@ static BlockBackend *blockdev_init(const char *file, QDict 
*bs_opts,
 QDict *interval_dict = NULL;
 QList *interval_list = NULL;
 const char *id;
-bool has_driver_specific_opts;
 BlockdevDetectZeroesOptions detect_zeroes =
 BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
 const char *throttling_group = NULL;
@@ -514,8 +513,6 @@ static BlockBackend *blockdev_init(const char *file, QDict 
*bs_opts,
 qdict_del(bs_opts, "id");
 }
 
-has_driver_specific_opts = !!qdict_size(bs_opts);
-
 /* extract parameters */
 snapshot = qemu_opt_get_bool(opts, "snapshot", 0);
 
@@ -578,7 +575,7 @@ static BlockBackend *blockdev_init(const char *file, QDict 
*bs_opts,
 }
 
 /* init */
-if ((!file || !*file) && !has_driver_specific_opts) {
+if ((!file || !*file) && !qdict_size(bs_opts)) {
 BlockBackendRootState *blk_rs;
 
 blk = blk_new(qemu_opts_id(opts), errp);
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 00afc20..13b763d 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -206,13 +206,13 @@ static int setup_ide(int argc, char *argv[], int argv_sz,
 {
 char *s1, *s2, *s3;
 
-s1 = g_strdup_printf("-drive id=drive%d,if=%s,format=raw",
+s1 = g_strdup_printf("-drive id=drive%d,if=%s",
  ide_idx, dev ? "none" : "ide");
 s2 = dev ? g_strdup("") : g_strdup_printf(",index=%d", ide_idx);
 
 if (img_secs[img_idx] >= 0) {
 setup_mbr(img_idx, mbr);
-s3 = g_strdup_printf(",file=%s", img_file_name[img_idx]);
+s3 = g_strdup_printf(",format=raw,file=%s", img_file_name[img_idx]);
 } else {
 s3 = g_strdup(",media=cdrom");
 }
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e02245e..27eddec 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -148,12 +148,12 @@ class VM(object):
 def add_drive(self, path, opts='', interface='virtio'):
 '''Add a virtio-blk drive to the VM'''
 options = ['if=%s' % interface,
-   'format=%s' % imgfmt,
'cache=%s' % cachemode,
'id=drive%d' % self._num_drives]
 
 if path is not None:
 options.append('file=%s' % path)
+options.append('format=%s' % imgfmt)
 
 if opts:
 options.append(opts)
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 05/21] block: Consider all block layer options in append_open_options

[Kevin Wolf]

The code already special-cased "node-name", which is currently the only
option passed in the QDict that isn't driver-specific. Generalise the
code to take all general block layer options into consideration.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Reviewed-by: Alberto Garcia 
---
 block.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 0241acd..73f0816 100644
--- a/block.c
+++ b/block.c
@@ -3950,20 +3950,30 @@ out:
 static bool append_open_options(QDict *d, BlockDriverState *bs)
 {
 const QDictEntry *entry;
+QemuOptDesc *desc;
 bool found_any = false;
 
 for (entry = qdict_first(bs->options); entry;
  entry = qdict_next(bs->options, entry))
 {
-/* Only take options for this level and exclude all non-driver-specific
- * options */
-if (!strchr(qdict_entry_key(entry), '.') &&
-strcmp(qdict_entry_key(entry), "node-name"))
-{
-qobject_incref(qdict_entry_value(entry));
-qdict_put_obj(d, qdict_entry_key(entry), qdict_entry_value(entry));
-found_any = true;
+/* Only take options for this level */
+if (strchr(qdict_entry_key(entry), '.')) {
+continue;
 }
+
+/* And exclude all non-driver-specific options */
+for (desc = bdrv_runtime_opts.desc; desc->name; desc++) {
+if (!strcmp(qdict_entry_key(entry), desc->name)) {
+break;
+}
+}
+if (desc->name) {
+continue;
+}
+
+qobject_incref(qdict_entry_value(entry));
+qdict_put_obj(d, qdict_entry_key(entry), qdict_entry_value(entry));
+found_any = true;
 }
 
 return found_any;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 6/8] hw/arm/virt-acpi-build: Add _E03 for Power Button

[Igor Mammedov]

On Mon, 16 Nov 2015 21:23:07 +0800
shannon.z...@linaro.org wrote:

> From: Shannon Zhao 
> 
> Here GPIO pin 3 is used for Power Button, add _E03 in ACPI DSDT table.
> 
> Signed-off-by: Shannon Zhao 
> Signed-off-by: Shannon Zhao 
> Tested-by: Wei Huang 
> ---
>  hw/arm/virt-acpi-build.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index b25c90b..12c9e8b 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -337,6 +337,19 @@ static void acpi_dsdt_add_gpio(Aml *scope, const 
> MemMapEntry *gpio_memmap,
>  aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
>AML_EXCLUSIVE, gpio_irq));
>  aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +Aml *aei = aml_resource_template();
> +/* Pin 3 for power button */
> +int32_t pin_num[1] = {3};
> +aml_append(aei, aml_gpio_int(AML_CONSUMER_PRODUCER, AML_EDGE,
> + AML_ACTIVE_HIGH, AML_EXCLUSIVE, 
> AML_PULL_UP, 0,
> + pin_num, 1, "GPO0", NULL));
> +aml_append(dev, aml_name_decl("_AEI", aei));
> +
> +/* _E03 is handle for power button */
> +Aml *method = aml_method("_E03", 0);
> +aml_append(method, aml_notify(aml_name("PWRB"), aml_int(0x80)));
you use "PWRB" here the second time, perhaps it would be better to use macro
#define ACPI_POWER_BUTTON_DEVICE "PWRB"

> +aml_append(dev, method);
>  aml_append(scope, dev);
>  }
>  

Re: [Qemu-devel] [PATCH 1/2] qemu-file: fix flaws of qemu_put_compression_data

[Li, Liang Z]

> On (Fri) 04 Dec 2015 [11:53:09], Liang Li wrote:
> > There are some flaws in qemu_put_compression_data, this patch tries to
> > fix it. Now it can be used by other code.
> 
> Can you please write a better description here?  What are the flaws?
> What is being fixed?  What other users, and how is it now safer for the other
> users to use this code?
> 

no problem, will add it in next version.  

Thanks,

Liang

> Thanks,
> 
>   Amit

[Qemu-devel] [PATCH v2 for 2.6 0/5] QMP wrappers for VM snapshot operations

[Denis V. Lunev]

EFI based VM with pflash storage for NVRAM could not be snapshoted as
libvirt configures storage as 'raw' and writable. OK, this is a libvirt
problem.

Another problem is that libvirt can not detect this failure at all
as it uses HMP for this operation. This create snapshot/delete snapshot
sequence passes silently.

The patchset adds QMP wrappers for the purpose.

Signed-off-by: "Denis V. Lunev" 
CC: Juan Quintela 
CC: Amit Shah 
CC: Markus Armbruster 
CC: Eric Blake 

Changes from v1:
- cosmetic fixes suggested by Markus. I pray I have added all of them :)
- patch 5 is rewritten completely. Original one was deadbeaf

Denis V. Lunev (5):
  migration: split hmp_savevm to do_savevm and hmp_savevm wrapper
  qmp: create qmp_savevm command
  qmp: create qmp_delvm command
  migration: improve error reporting for hmp_loadvm
  qmp: create QMP implementation of loadvm command

 include/sysemu/sysemu.h |   2 +-
 migration/savevm.c  | 100 +++-
 monitor.c   |   7 +++-
 qapi-schema.json|  39 +++
 qmp-commands.hx |  71 ++
 vl.c|   5 ++-
 6 files changed, 185 insertions(+), 39 deletions(-)

[Qemu-devel] [PATCH 5/5] qmp: create QMP implementation of loadvm command

[Denis V. Lunev]

Unfortunately load_vmstate has a return code (int) and this code is checked
in the other places. Thus we could not just rename it to qmp_loadvm as
returns void.

Signed-off-by: Denis V. Lunev 
CC: Juan Quintela 
CC: Amit Shah 
CC: Markus Armbruster 
CC: Eric Blake 
---
 migration/savevm.c | 12 
 monitor.c  | 12 ++--
 qapi-schema.json   | 13 +
 qmp-commands.hx| 23 +++
 4 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 7846437..07b0bf4 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2103,6 +2103,18 @@ int load_vmstate(const char *name, Error **errp)
 return 0;
 }
 
+void qmp_loadvm(const char *name, Error **errp)
+{
+int saved_vm_running  = runstate_is_running();
+vm_stop(RUN_STATE_RESTORE_VM);
+
+load_vmstate(name, errp);
+
+if (saved_vm_running) {
+vm_start();
+}
+}
+
 void qmp_delvm(const char *name, Error **errp)
 {
 BlockDriverState *bs;
diff --git a/monitor.c b/monitor.c
index 3857724..aba2c62 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1737,18 +1737,18 @@ void qmp_closefd(const char *fdname, Error **errp)
 
 static void hmp_loadvm(Monitor *mon, const QDict *qdict)
 {
-int saved_vm_running  = runstate_is_running();
 const char *name = qdict_get_str(qdict, "name");
 Error *local_err = NULL;
 
-vm_stop(RUN_STATE_RESTORE_VM);
+if (name == NULL) {
+monitor_printf(mon, "Snapshot name is not set for hmp_loadvm");
+return;
+}
 
-if (load_vmstate(name, _err) < 0) {
+qmp_loadvm(name, _err);
+if (local_err != NULL) {
 error_report_err(local_err);
 }
-if (saved_vm_running) {
-vm_start();
-}
 }
 
 int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
diff --git a/qapi-schema.json b/qapi-schema.json
index 18c9a6c..9747df8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3997,3 +3997,16 @@
 # Since 2.6
 ##
 { 'command': 'delvm', 'data': {'name': 'str'} }
+
+##
+# @loadvm
+#
+# Load a VM snapshot
+#
+# @name: identifier of a snapshot to be loaded
+#
+# Returns: Nothing on success
+#
+# Since 2.6
+##
+{ 'command': 'loadvm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a630e8a..4dedb62 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4787,3 +4787,26 @@ EQMP
 .args_type  = "name:s",
 .mhandler.cmd_new = qmp_marshal_delvm,
 },
+
+SQMP
+loadvm
+--
+
+Load a VM snapshot
+
+Arguments:
+
+- "name": snapshot name
+
+Example:
+
+-> { "execute": "loadvm", "arguments": { "name": "snapshot1" } }
+<- { "return": {} }
+
+EQMP
+
+{
+.name   = "loadvm",
+.args_type  = "name:s",
+.mhandler.cmd_new = qmp_marshal_loadvm,
+},
-- 
2.5.0

[Qemu-devel] [PATCH 2/5] qmp: create qmp_savevm command

[Denis V. Lunev]

'name' attribute is made mandatory in distinction with HMP command.

The patch also moves hmp_savevm implementation into hmp.c. This function
is just a simple wrapper now and does not have knowledge about
migration internals.

Signed-off-by: Denis V. Lunev 
CC: Juan Quintela 
CC: Amit Shah 
CC: Markus Armbruster 
CC: Eric Blake 
---
 hmp.c  | 12 
 migration/savevm.c | 13 +
 qapi-schema.json   | 13 +
 qmp-commands.hx| 25 +
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/hmp.c b/hmp.c
index 2140605..c9c7100 100644
--- a/hmp.c
+++ b/hmp.c
@@ -32,6 +32,7 @@
 #include "ui/console.h"
 #include "block/qapi.h"
 #include "qemu-io.h"
+#include "sysemu/sysemu.h"
 
 #ifdef CONFIG_SPICE
 #include 
@@ -2378,3 +2379,14 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict 
*qdict)
 
 qapi_free_RockerOfDpaGroupList(list);
 }
+
+void hmp_savevm(Monitor *mon, const QDict *qdict)
+{
+Error *local_err = NULL;
+
+qmp_savevm(qdict_get_try_str(qdict, "name"), _err);
+
+if (local_err != NULL) {
+error_report_err(local_err);
+}
+}
diff --git a/migration/savevm.c b/migration/savevm.c
index b7278ac..65e0081 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1905,7 +1905,7 @@ int qemu_loadvm_state(QEMUFile *f)
 return ret;
 }
 
-static void do_savevm(const char *name, Error **errp)
+void qmp_savevm(const char *name, Error **errp)
 {
 BlockDriverState *bs, *bs1;
 QEMUSnapshotInfo sn1, *sn = , old_sn1, *old_sn = _sn1;
@@ -1999,17 +1999,6 @@ static void do_savevm(const char *name, Error **errp)
 }
 }
 
-void hmp_savevm(Monitor *mon, const QDict *qdict)
-{
-Error *local_err = NULL;
-
-do_savevm(qdict_get_try_str(qdict, "name"), _err);
-
-if (local_err != NULL) {
-error_report_err(local_err);
-}
-}
-
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
 QEMUFile *f;
diff --git a/qapi-schema.json b/qapi-schema.json
index 8b1a423..0114132 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3971,3 +3971,16 @@
 ##
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
+
+##
+# @savevm
+#
+# Save a VM snapshot. Without a name new snapshot is created",
+#
+# @name: identifier of a snapshot to be created
+#
+# Returns: Nothing on success
+#
+# Since 2.6
+##
+{ 'command': 'savevm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9d8b42f..f216c5e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4739,3 +4739,28 @@ Example:
  {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
   "pop-vlan": 1, "id": 251658240}
]}
+
+EQMP
+
+SQMP
+savevm
+--
+
+Save a VM snapshot. If no tag or id are provided, a new snapshot is created
+
+Arguments:
+
+- "name": snapshot name
+
+Example:
+
+-> { "execute": "savevm", "arguments": { "name": "snapshot1" } }
+<- { "return": {} }
+
+EQMP
+
+{
+.name   = "savevm",
+.args_type  = "name:s?",
+.mhandler.cmd_new = qmp_marshal_savevm,
+},
-- 
2.5.0

[Qemu-devel] [PATCH 1/5] migration: split hmp_savevm to do_savevm and hmp_savevm wrapper

[Denis V. Lunev]

This would be useful in the next step when QMP version of this call will
be introduced.

Signed-off-by: Denis V. Lunev 
Reviewed-by: Juan Quintela 
CC: Amit Shah 
CC: Markus Armbruster 
CC: Eric Blake 
---
 migration/savevm.c | 38 +++---
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 0ad1b93..b7278ac 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1905,7 +1905,7 @@ int qemu_loadvm_state(QEMUFile *f)
 return ret;
 }
 
-void hmp_savevm(Monitor *mon, const QDict *qdict)
+static void do_savevm(const char *name, Error **errp)
 {
 BlockDriverState *bs, *bs1;
 QEMUSnapshotInfo sn1, *sn = , old_sn1, *old_sn = _sn1;
@@ -1915,28 +1915,27 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
 uint64_t vm_state_size;
 qemu_timeval tv;
 struct tm tm;
-const char *name = qdict_get_try_str(qdict, "name");
 Error *local_err = NULL;
 AioContext *aio_context;
 
 if (!bdrv_all_can_snapshot()) {
-monitor_printf(mon, "Device '%s' is writable but does not "
-   "support snapshots.\n", bdrv_get_device_name(bs));
+error_setg(errp,
+   "Device '%s' is writable but does not support snapshots.",
+   bdrv_get_device_name(bs));
 return;
 }
 
 /* Delete old snapshots of the same name */
 if (name && bdrv_all_delete_snapshot(name, , _err) < 0) {
-monitor_printf(mon,
-   "Error while deleting snapshot on device '%s': %s\n",
-   bdrv_get_device_name(bs1), error_get_pretty(local_err));
+error_setg(errp, "Error while deleting snapshot on device '%s': %s",
+   bdrv_get_device_name(bs1), error_get_pretty(local_err));
 error_free(local_err);
 return;
 }
 
 bs = bdrv_all_find_vmstate_bs();
 if (bs == NULL) {
-monitor_printf(mon, "No block device can accept snapshots\n");
+error_setg(errp, "No block device can accept snapshots");
 return;
 }
 aio_context = bdrv_get_aio_context(bs);
@@ -1945,7 +1944,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
 
 ret = global_state_store();
 if (ret) {
-monitor_printf(mon, "Error saving global state\n");
+error_setg(errp, "Error saving global state");
 return;
 }
 vm_stop(RUN_STATE_SAVE_VM);
@@ -1977,22 +1976,20 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
 /* save the VM state */
 f = qemu_fopen_bdrv(bs, 1);
 if (!f) {
-monitor_printf(mon, "Could not open VM state file\n");
+error_setg(errp, "Could not open VM state file");
 goto the_end;
 }
-ret = qemu_savevm_state(f, _err);
+ret = qemu_savevm_state(f, errp);
 vm_state_size = qemu_ftell(f);
 qemu_fclose(f);
 if (ret < 0) {
-monitor_printf(mon, "%s\n", error_get_pretty(local_err));
-error_free(local_err);
 goto the_end;
 }
 
 ret = bdrv_all_create_snapshot(sn, bs, vm_state_size, );
 if (ret < 0) {
-monitor_printf(mon, "Error while creating snapshot on '%s'\n",
-   bdrv_get_device_name(bs));
+error_setg(errp, "Error while creating snapshot on '%s'",
+   bdrv_get_device_name(bs));
 }
 
  the_end:
@@ -2002,6 +1999,17 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
 }
 }
 
+void hmp_savevm(Monitor *mon, const QDict *qdict)
+{
+Error *local_err = NULL;
+
+do_savevm(qdict_get_try_str(qdict, "name"), _err);
+
+if (local_err != NULL) {
+error_report_err(local_err);
+}
+}
+
 void qmp_xen_save_devices_state(const char *filename, Error **errp)
 {
 QEMUFile *f;
-- 
2.5.0

Re: [Qemu-devel] [PATCH 1/2] qemu-file: fix flaws of qemu_put_compression_data

[Amit Shah]

On (Fri) 04 Dec 2015 [11:53:09], Liang Li wrote:
> There are some flaws in qemu_put_compression_data, this patch tries
> to fix it. Now it can be used by other code.

Can you please write a better description here?  What are the flaws?
What is being fixed?  What other users, and how is it now safer for
the other users to use this code?

Thanks,

Amit

Re: [Qemu-devel] [PATCH 1/3] hw/sd/pxa2xx_mmci: convert to SysBusDevice object

[Peter Maydell]

On 4 December 2015 at 12:50, Markus Armbruster  wrote:
> To finally answer your question: the proper owner of the property
> connecting the backend is the frontend half of the split device.  So, if
> we have separate device models for controller and the block devices
> connected to it, the block backend property belongs to the latter, not
> the former.  If we have separate device models for SD controller and
> card, the block backend property belongs to the card, not the
> controller.

I thought this might be your answer, but this is problematic because
we now have a device in the tree (the sdhci pci card) which has the
property on the controller, and so that's now user-facing command
line ABI which we can't change...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] arm: soc-dma: use hwaddr instead of target_ulong in printf

[Markus Armbruster]

Paolo Bonzini  writes:

> This is a first baby step towards removing widespread inclusion of
> cpu.h and compiling more devices once (so that arm, aarch64 and
> in the future target-multi can share the object files).

Sounds like something that should be covered in
http://qemu-project.org/CodeTransitions

[Qemu-devel] [PATCH v2] scripts/gdb: Fix a python exception in mtree.py

[Yang Wei]

The following exception is threw:
Python Exception  name 'long' is not defined:
Error occurred in Python command: name 'long' is not defined

Python 2.4+, int()/long() have been unified, so replace long
with int.

Signed-off-by: Yang Wei 
---
 scripts/qemugdb/mtree.py | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/qemugdb/mtree.py b/scripts/qemugdb/mtree.py
index 06011c3..cc8131c 100644
--- a/scripts/qemugdb/mtree.py
+++ b/scripts/qemugdb/mtree.py
@@ -21,7 +21,7 @@ def isnull(ptr):
 return ptr == gdb.Value(0).cast(ptr.type)

 def int128(p):
-return long(p['lo']) + (long(p['hi']) << 64)
+return int(p['lo']) + (int(p['hi']) << 64)

 class MtreeCommand(gdb.Command):
 '''Display the memory tree hierarchy'''
@@ -40,11 +40,11 @@ class MtreeCommand(gdb.Command):
 def process_queue(self):
 while self.queue:
 ptr = self.queue.pop(0)
-if long(ptr) in self.seen:
+if int(ptr) in self.seen:
 continue
 self.print_item(ptr)
 def print_item(self, ptr, offset = gdb.Value(0), level = 0):
-self.seen.add(long(ptr))
+self.seen.add(int(ptr))
 addr = ptr['addr']
 addr += offset
 size = int128(ptr['size'])
@@ -58,8 +58,8 @@ class MtreeCommand(gdb.Command):
 klass = ' (RAM)'
 gdb.write('%s%016x-%016x %s%s (@ %s)\n'
   % ('  ' * level,
- long(addr),
- long(addr + (size - 1)),
+ int(addr),
+ int(addr + (size - 1)),
  ptr['name'].string(),
  klass,
  ptr,
--
1.9.1

Re: [Qemu-devel] [PATCH] ivshmem: Store file descriptor for vhost-user negotiation

[Marc-André Lureau]

Hi

On Fri, Dec 4, 2015 at 4:52 AM, Tetsuya Mukawa  wrote:
> If virtio-net driver allocates memory in vishmem shared memory,

s/vishmem/ivshmem

How can virtio-net allocate memory in ivshmem memory bar?

What's the use case or test case?

> vhost-net will work correctly, but vhost-user will not work because
> a fd of shared memory will not be sent to vhost-user backend.
> This patch fixes ivshmem to store file descriptor of shared memory.
> It will be used when vhost-user negotiates vhost-user backend.
>
> Signed-off-by: Tetsuya Mukawa 
> ---
>  exec.c  | 10 ++
>  hw/misc/ivshmem.c   |  9 +++--
>  include/exec/ram_addr.h |  1 +
>  3 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 0bf0a6e..908c4bf 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1796,6 +1796,16 @@ int qemu_get_ram_fd(ram_addr_t addr)
>  return fd;
>  }
>
> +void qemu_set_ram_fd(ram_addr_t addr, int fd)
> +{
> +RAMBlock *block;
> +
> +rcu_read_lock();
> +block = qemu_get_ram_block(addr);
> +block->fd = fd;
> +rcu_read_unlock();
> +}
> +
>  void *qemu_get_ram_block_host_ptr(ram_addr_t addr)
>  {
>  RAMBlock *block;
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index f73f0c2..df585de 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -29,6 +29,7 @@
>  #include "sysemu/char.h"
>  #include "sysemu/hostmem.h"
>  #include "qapi/visitor.h"
> +#include "exec/ram_addr.h"
>
>  #include "hw/misc/ivshmem.h"
>
> @@ -422,6 +423,7 @@ static int create_shared_memory_BAR(IVShmemState *s, int 
> fd, uint8_t attr,
>
>  memory_region_init_ram_ptr(>ivshmem, OBJECT(s), "ivshmem.bar2",
> s->ivshmem_size, ptr);
> +qemu_set_ram_fd(s->ivshmem.ram_addr, fd);
>  vmstate_register_ram(>ivshmem, DEVICE(s));
>  memory_region_add_subregion(>bar, 0, >ivshmem);
>
> @@ -682,6 +684,7 @@ static void ivshmem_read(void *opaque, const uint8_t 
> *buf, int size)
>  }
>  memory_region_init_ram_ptr(>ivshmem, OBJECT(s),
> "ivshmem.bar2", s->ivshmem_size, map_ptr);
> +qemu_set_ram_fd(s->ivshmem.ram_addr, incoming_fd);
>  vmstate_register_ram(>ivshmem, DEVICE(s));
>
>  IVSHMEM_DPRINTF("guest h/w addr = %p, size = %" PRIu64 "\n",
> @@ -689,7 +692,6 @@ static void ivshmem_read(void *opaque, const uint8_t 
> *buf, int size)
>
>  memory_region_add_subregion(>bar, 0, >ivshmem);
>
> -close(incoming_fd);
>  return;
>  }
>
> @@ -991,7 +993,6 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error 
> **errp)
>  }
>
>  create_shared_memory_BAR(s, fd, attr, errp);
> -close(fd);
>  }
>  }
>
> @@ -1010,11 +1011,15 @@ static void pci_ivshmem_exit(PCIDevice *dev)
>  if (memory_region_is_mapped(>ivshmem)) {
>  if (!s->hostmem) {
>  void *addr = memory_region_get_ram_ptr(>ivshmem);
> +int fd;
>
>  if (munmap(addr, s->ivshmem_size) == -1) {
>  error_report("Failed to munmap shared memory %s",
>   strerror(errno));
>  }
> +
> +if ((fd = qemu_get_ram_fd(s->ivshmem.ram_addr)) != -1)
> +close(fd);
>  }
>
>  vmstate_unregister_ram(>ivshmem, DEVICE(dev));
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 7115154..9ca659a 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -72,6 +72,7 @@ ram_addr_t qemu_ram_alloc_resizeable(ram_addr_t size, 
> ram_addr_t max_size,
>   void *host),
>   MemoryRegion *mr, Error **errp);
>  int qemu_get_ram_fd(ram_addr_t addr);
> +void qemu_set_ram_fd(ram_addr_t addr, int fd);
>  void *qemu_get_ram_block_host_ptr(ram_addr_t addr);
>  void *qemu_get_ram_ptr(ram_addr_t addr);
>  void qemu_ram_free(ram_addr_t addr);
> --
> 2.1.4
>
>



-- 
Marc-André Lureau

[Qemu-devel] [PATCH v3 01/21] qcow2: Add .bdrv_join_options callback

[Kevin Wolf]

qcow2 accepts a few driver-specific options that overlap semantically
(e.g. "overlap-check" is an alias of "overlap-check.template", and any
missing cache size option is derived from the given ones).

When bdrv_reopen() merges the set of updated options with left out
options that should be kept at their old value, we need to consider this
and filter out any duplicates (which would generally cause errors
because new and old value would contradict each other).

This patch adds a .bdrv_join_options callback to BlockDriver and
implements it for qcow2.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: Alberto Garcia 
---
 block/qcow2.c | 47 +++
 include/block/block_int.h |  1 +
 2 files changed, 48 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 88f56c8..9baaf4d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1282,6 +1282,52 @@ static void qcow2_reopen_abort(BDRVReopenState *state)
 g_free(state->opaque);
 }
 
+static void qcow2_join_options(QDict *options, QDict *old_options)
+{
+bool has_new_overlap_template =
+qdict_haskey(options, QCOW2_OPT_OVERLAP) ||
+qdict_haskey(options, QCOW2_OPT_OVERLAP_TEMPLATE);
+bool has_new_total_cache_size =
+qdict_haskey(options, QCOW2_OPT_CACHE_SIZE);
+bool has_all_cache_options;
+
+/* New overlap template overrides all old overlap options */
+if (has_new_overlap_template) {
+qdict_del(old_options, QCOW2_OPT_OVERLAP);
+qdict_del(old_options, QCOW2_OPT_OVERLAP_TEMPLATE);
+qdict_del(old_options, QCOW2_OPT_OVERLAP_MAIN_HEADER);
+qdict_del(old_options, QCOW2_OPT_OVERLAP_ACTIVE_L1);
+qdict_del(old_options, QCOW2_OPT_OVERLAP_ACTIVE_L2);
+qdict_del(old_options, QCOW2_OPT_OVERLAP_REFCOUNT_TABLE);
+qdict_del(old_options, QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK);
+qdict_del(old_options, QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE);
+qdict_del(old_options, QCOW2_OPT_OVERLAP_INACTIVE_L1);
+qdict_del(old_options, QCOW2_OPT_OVERLAP_INACTIVE_L2);
+}
+
+/* New total cache size overrides all old options */
+if (qdict_haskey(options, QCOW2_OPT_CACHE_SIZE)) {
+qdict_del(old_options, QCOW2_OPT_L2_CACHE_SIZE);
+qdict_del(old_options, QCOW2_OPT_REFCOUNT_CACHE_SIZE);
+}
+
+qdict_join(options, old_options, false);
+
+/*
+ * If after merging all cache size options are set, an old total size is
+ * overwritten. Do keep all options, however, if all three are new. The
+ * resulting error message is what we want to happen.
+ */
+has_all_cache_options =
+qdict_haskey(options, QCOW2_OPT_CACHE_SIZE) ||
+qdict_haskey(options, QCOW2_OPT_L2_CACHE_SIZE) ||
+qdict_haskey(options, QCOW2_OPT_REFCOUNT_CACHE_SIZE);
+
+if (has_all_cache_options && !has_new_total_cache_size) {
+qdict_del(options, QCOW2_OPT_CACHE_SIZE);
+}
+}
+
 static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
 int64_t sector_num, int nb_sectors, int *pnum)
 {
@@ -3145,6 +3191,7 @@ BlockDriver bdrv_qcow2 = {
 .bdrv_reopen_prepare  = qcow2_reopen_prepare,
 .bdrv_reopen_commit   = qcow2_reopen_commit,
 .bdrv_reopen_abort= qcow2_reopen_abort,
+.bdrv_join_options= qcow2_join_options,
 .bdrv_create= qcow2_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 .bdrv_co_get_block_status = qcow2_co_get_block_status,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 4012e36..77dc165 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -121,6 +121,7 @@ struct BlockDriver {
BlockReopenQueue *queue, Error **errp);
 void (*bdrv_reopen_commit)(BDRVReopenState *reopen_state);
 void (*bdrv_reopen_abort)(BDRVReopenState *reopen_state);
+void (*bdrv_join_options)(QDict *options, QDict *old_options);
 
 int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags,
  Error **errp);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 03/21] mirror: Error out when a BDS would get two BBs

[Kevin Wolf]

bdrv_replace_in_backing_chain() asserts that not both old and new
BlockDdriverState have a BlockBackend attached to them because both
would have to end up pointing to the new BDS and we don't support more
than one BB per BDS yet.

Before we can safely allow references to existing nodes as backing
files, we need to make sure that even if a backing file has a BB on it,
this doesn't crash qemu.

There are probably also some cases with the 'replaces' option set where
drive-mirror could fail this assertion today. They are fixed with this
error check as well.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 block/mirror.c | 28 
 1 file changed, 28 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c
index 0e8f556..8e3f524 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -18,6 +18,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qemu/ratelimit.h"
 #include "qemu/bitmap.h"
+#include "qemu/error-report.h"
 
 #define SLICE_TIME1ULL /* ns */
 #define MAX_IN_FLIGHT 16
@@ -370,11 +371,22 @@ static void mirror_exit(BlockJob *job, void *opaque)
 if (s->to_replace) {
 to_replace = s->to_replace;
 }
+
+/* This was checked in mirror_start_job(), but meanwhile one of the
+ * nodes could have been newly attached to a BlockBackend. */
+if (to_replace->blk && s->target->blk) {
+error_report("block job: Can't create node with two 
BlockBackends");
+data->ret = -EINVAL;
+goto out;
+}
+
 if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
 bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
 }
 bdrv_replace_in_backing_chain(to_replace, s->target);
 }
+
+out:
 if (s->to_replace) {
 bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
 error_free(s->replace_blocker);
@@ -705,6 +717,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
  bool is_none_mode, BlockDriverState *base)
 {
 MirrorBlockJob *s;
+BlockDriverState *replaced_bs;
 
 if (granularity == 0) {
 granularity = bdrv_get_default_bitmap_granularity(target);
@@ -728,6 +741,21 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 buf_size = DEFAULT_MIRROR_BUF_SIZE;
 }
 
+/* We can't support this case as long as the block layer can't handle
+ * multiple BlockBackends per BlockDriverState. */
+if (replaces) {
+replaced_bs = bdrv_lookup_bs(replaces, replaces, errp);
+if (replaced_bs == NULL) {
+return;
+}
+} else {
+replaced_bs = bs;
+}
+if (replaced_bs->blk && target->blk) {
+error_setg(errp, "Can't create node with two BlockBackends");
+return;
+}
+
 s = block_job_create(driver, bs, speed, cb, opaque, errp);
 if (!s) {
 return;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 08/21] block: Keep "driver" in bs->options

[Kevin Wolf]

Instead of passing a separate drv argument to bdrv_open_common(), just
make sure that a "driver" option is set in the QDict. This also means
that a "driver" entry is consistently present in bs->options now.

This is another step towards keeping all options in the QDict (which is
the represenation of the blockdev-add QMP command).

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 block.c | 57 -
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/block.c b/block.c
index 5ca7c71..2728bc5 100644
--- a/block.c
+++ b/block.c
@@ -817,6 +817,11 @@ static QemuOptsList bdrv_runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "Node name of the block device node",
 },
+{
+.name = "driver",
+.type = QEMU_OPT_STRING,
+.help = "Block driver to use for the node",
+},
 { /* end of list */ }
 },
 };
@@ -827,18 +832,31 @@ static QemuOptsList bdrv_runtime_opts = {
  * Removes all processed options from *options.
  */
 static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
-QDict *options, int flags, BlockDriver *drv, Error **errp)
+QDict *options, int flags, Error **errp)
 {
 int ret, open_flags;
 const char *filename;
+const char *driver_name = NULL;
 const char *node_name = NULL;
 QemuOpts *opts;
+BlockDriver *drv;
 Error *local_err = NULL;
 
-assert(drv != NULL);
 assert(bs->file == NULL);
 assert(options != NULL && bs->options != options);
 
+opts = qemu_opts_create(_runtime_opts, NULL, 0, _abort);
+qemu_opts_absorb_qdict(opts, options, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto fail_opts;
+}
+
+driver_name = qemu_opt_get(opts, "driver");
+drv = bdrv_find_format(driver_name);
+assert(drv != NULL);
+
 if (file != NULL) {
 filename = file->bs->filename;
 } else {
@@ -848,19 +866,12 @@ static int bdrv_open_common(BlockDriverState *bs, 
BdrvChild *file,
 if (drv->bdrv_needs_filename && !filename) {
 error_setg(errp, "The '%s' block driver requires a file name",
drv->format_name);
-return -EINVAL;
-}
-
-trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
-
-opts = qemu_opts_create(_runtime_opts, NULL, 0, _abort);
-qemu_opts_absorb_qdict(opts, options, _err);
-if (local_err) {
-error_propagate(errp, local_err);
 ret = -EINVAL;
 goto fail_opts;
 }
 
+trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
+
 node_name = qemu_opt_get(opts, "node-name");
 bdrv_assign_node_name(bs, node_name, _err);
 if (local_err) {
@@ -1477,11 +1488,14 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 goto fail;
 }
 
+bs->open_flags = flags;
+bs->options = options;
+options = qdict_clone_shallow(options);
+
 /* Find the right image format driver */
 drvname = qdict_get_try_str(options, "driver");
 if (drvname) {
 drv = bdrv_find_format(drvname);
-qdict_del(options, "driver");
 if (!drv) {
 error_setg(errp, "Unknown driver: '%s'", drvname);
 ret = -EINVAL;
@@ -1497,10 +1511,6 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 qdict_del(options, "backing");
 }
 
-bs->open_flags = flags;
-bs->options = options;
-options = qdict_clone_shallow(options);
-
 /* Open image file without format layer */
 if ((flags & BDRV_O_PROTOCOL) == 0) {
 if (flags & BDRV_O_RDWR) {
@@ -1528,6 +1538,19 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 if (ret < 0) {
 goto fail;
 }
+/*
+ * This option update would logically belong in bdrv_fill_options(),
+ * but we first need to open bs->file for the probing to work, while
+ * opening bs->file already requires the (mostly) final set of options
+ * so that cache mode etc. can be inherited.
+ *
+ * Adding the driver later is somewhat ugly, but it's not an option
+ * that would ever be inherited, so it's correct. We just need to make
+ * sure to update both bs->options (which has the full effective
+ * options for bs) and options (which has file.* already removed).
+ */
+qdict_put(bs->options, "driver", qstring_from_str(drv->format_name));
+qdict_put(options, "driver", qstring_from_str(drv->format_name));
 } else if (!drv) {
 error_setg(errp, "Must specify either driver or file");
 ret = -EINVAL;
@@ -1541,7 +1564,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 assert(!(flags & BDRV_O_PROTOCOL) || !file);

[Qemu-devel] [PATCH v3 07/21] block: Pass driver-specific options to .bdrv_refresh_filename()

[Kevin Wolf]

In order to decide whether a blkdebug: filename can be produced or a
json: one is necessary, blkdebug checked whether bs->options had more
options than just "config", "x-image" or "image" (the latter including
nested options). That doesn't work well when generic block layer options
are present.

This patch passes an option QDict to the driver that contains only
driver-specific options, i.e. the options for the general block layer as
well as child nodes are already filtered out. Works much better this
way.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: Alberto Garcia 
---
 block.c   |  5 -
 block/blkdebug.c  | 17 ++---
 block/blkverify.c |  2 +-
 block/nbd.c   | 10 +-
 block/quorum.c|  2 +-
 include/block/block_int.h |  2 +-
 6 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/block.c b/block.c
index 0dfff7a..5ca7c71 100644
--- a/block.c
+++ b/block.c
@@ -4027,7 +4027,10 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 bs->full_open_options = NULL;
 }
 
-drv->bdrv_refresh_filename(bs);
+opts = qdict_new();
+append_open_options(opts, bs);
+drv->bdrv_refresh_filename(bs, opts);
+QDECREF(opts);
 } else if (bs->file) {
 /* Try to reconstruct valid information from the underlying file */
 bool has_open_options;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index dee3a0e..48b0812 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -731,17 +731,15 @@ static int blkdebug_truncate(BlockDriverState *bs, 
int64_t offset)
 return bdrv_truncate(bs->file->bs, offset);
 }
 
-static void blkdebug_refresh_filename(BlockDriverState *bs)
+static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
 {
 QDict *opts;
 const QDictEntry *e;
 bool force_json = false;
 
-for (e = qdict_first(bs->options); e; e = qdict_next(bs->options, e)) {
+for (e = qdict_first(options); e; e = qdict_next(options, e)) {
 if (strcmp(qdict_entry_key(e), "config") &&
-strcmp(qdict_entry_key(e), "x-image") &&
-strcmp(qdict_entry_key(e), "image") &&
-strncmp(qdict_entry_key(e), "image.", strlen("image.")))
+strcmp(qdict_entry_key(e), "x-image"))
 {
 force_json = true;
 break;
@@ -757,7 +755,7 @@ static void blkdebug_refresh_filename(BlockDriverState *bs)
 if (!force_json && bs->file->bs->exact_filename[0]) {
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
  "blkdebug:%s:%s",
- qdict_get_try_str(bs->options, "config") ?: "",
+ qdict_get_try_str(options, "config") ?: "",
  bs->file->bs->exact_filename);
 }
 
@@ -767,11 +765,8 @@ static void blkdebug_refresh_filename(BlockDriverState *bs)
 QINCREF(bs->file->bs->full_open_options);
 qdict_put_obj(opts, "image", QOBJECT(bs->file->bs->full_open_options));
 
-for (e = qdict_first(bs->options); e; e = qdict_next(bs->options, e)) {
-if (strcmp(qdict_entry_key(e), "x-image") &&
-strcmp(qdict_entry_key(e), "image") &&
-strncmp(qdict_entry_key(e), "image.", strlen("image.")))
-{
+for (e = qdict_first(options); e; e = qdict_next(options, e)) {
+if (strcmp(qdict_entry_key(e), "x-image")) {
 qobject_incref(qdict_entry_value(e));
 qdict_put_obj(opts, qdict_entry_key(e), qdict_entry_value(e));
 }
diff --git a/block/blkverify.c b/block/blkverify.c
index c5f8e8d..1d75449 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -307,7 +307,7 @@ static void blkverify_attach_aio_context(BlockDriverState 
*bs,
 bdrv_attach_aio_context(s->test_file->bs, new_context);
 }
 
-static void blkverify_refresh_filename(BlockDriverState *bs)
+static void blkverify_refresh_filename(BlockDriverState *bs, QDict *options)
 {
 BDRVBlkverifyState *s = bs->opaque;
 
diff --git a/block/nbd.c b/block/nbd.c
index cd6a587..416f42b 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -342,13 +342,13 @@ static void nbd_attach_aio_context(BlockDriverState *bs,
 nbd_client_attach_aio_context(bs, new_context);
 }
 
-static void nbd_refresh_filename(BlockDriverState *bs)
+static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
 {
 QDict *opts = qdict_new();
-const char *path   = qdict_get_try_str(bs->options, "path");
-const char *host   = qdict_get_try_str(bs->options, "host");
-const char *port   = qdict_get_try_str(bs->options, "port");
-const char *export = qdict_get_try_str(bs->options, "export");
+const char *path   = qdict_get_try_str(options, "path");
+const char *host   = qdict_get_try_str(options, "host");
+const char *port   = qdict_get_try_str(options, "port");
+const char *export = qdict_get_try_str(options, "export");
 
 

Re: [Qemu-devel] [PATCH 0/2] fix the flaws of qemu_put_compression_data

[Li, Liang Z]

> Hi
> 
> We are in hard freeze.  My understanding is that this are "optimizations" that
> can wait for 2.6:
> - my understanding from commit from message one and from quick look at
>   the code is that this change is not needed for current users, is that 
> correct?
> - we avoid a copy at the beginning of each block (micro-optimization)
> 
> If my understanding is correct, I am delaying this two patches to 2.6.
> 
> What do you think?
> 

Yes, you are wright. I think it's ok to delay to 2.6.

Thanks.

Liang.

Re: [Qemu-devel] Linux vhost-user interrupt management fixes

[Didier Pallard]

On 12/03/2015 10:53 AM, Didier Pallard wrote:

Hi,

I recently did some stress tests of a vhost-user interface using an UDP
traffic generator. Traffic generator was connected to 2 physical ports
that are in turn connected to 2 virtio ports through a linux bridge, VM
(running linux) doing routing to forward packets between the 2 virtio ports.
When traffic reaches high pps rates of small packets, I faced the 2 following
problems:

- at some time, my qemu socket becomes full, causing qemu to send incomplete
SET_VRING_CALL messages to vhost-user backend (without proper fd set in
ancillary data).
- after some time, some interrupts are lost, causing the VM to stop
transmitting packets.

Both problems come from the fact that interrupt masking/unmasking of the VM
is deferred to vhost-user backend through the linux socket.
First problem comes from the fact that socket buffer gets full; it is corrected
in the first patch of the serie.
Second problem is a bit more complex. From what i understand of the code,
when VM wants to mask/unmask interrupts, qemu traps the command and sends a
SET_VRING_CALL to the vhost-user to swap interrupt notifications either to
a dummy descriptor or to an fd that was given to kvm module to route
interruption to the VM. After sending SET_VRING_CALL message through
the socket, VM code continues to run, assuming that the interrupts are now
masked; but due to linux socket, this message may be buffered and not currently
treated by the vhost-user backend, ie interrupts are not really masked/unmasked
as they ought to be.
I think it can be solved in two different ways:
- by waiting for an acknowledgement of vhost-user backend before exiting
interrupt masking function, this ensures that interrupt masking is correctly
taken into account before giving back hand to the VM, but it has a very high
cost in term of cycles. Moreover, unless specifying a new option, it will
break current API, since existing vhost-user implementations do not
expect a return message on a SET_VRING_CALL call.
- second way could be, in case vhost-user is involved, to restore the
initial behaviour of interrupt masking (ie masking/unmasking interrupts
by taking/giving vring_call fd from/to kernel kvm module).
Patches 2 and 3 of the serie propose an implementation of this second option.

Didier

Didier Pallard (3):
   char: fix vhost-user socket full
   virtio-pci: add an option to bypass guest_notifier_mask
   vhost-net: force guest_notifier_mask bypass in vhost-user case

  hw/net/vhost_net.c | 19 ++-
  hw/virtio/vhost.c  | 13 +
  hw/virtio/virtio-pci.c | 29 +++--
  hw/virtio/virtio-pci.h |  6 ++
  qemu-char.c| 10 ++
  5 files changed, 70 insertions(+), 7 deletions(-)



+maintainers

sorry, i forgot them in initial mail

didier