Re: [Qemu-devel] [PATCH] vnc: Set default kbd delay to 10ms

[Alexander Graf]

On 05.07.17 10:42, Markus Armbruster wrote:

Alexander Graf  writes:


The default keyboard delay time in the input layer is 10ms. I don't know


Do you mean "is 1ms"?


Yes, of course :). Sorry.


Alex




how that number came to be, but empirical tests on some OpenQA driven ARM
systems show that 10ms really is a reasonable default number for the delay.

With the current 1ms we're constantly typing faster than the guest receives
keyboard events from our XHCI attached USB HID device.

This patch moves the delay to 10ms. That way our default is much safer (good!)
and also consistent with the input layer default (also good!).

Signed-off-by: Alexander Graf 

Re: [Qemu-devel] [PATCH v8 2/4] net/socket: Convert several helper functions to Error

[Markus Armbruster]

Mao Zhongyi  writes:

> Currently, net_socket_mcast_create(), net_socket_fd_init_dgram() and
> net_socket_fd_init() use the function such as fprintf(), perror() to
> report an error message.
>
> Now, convert these functions to Error.
>
> Cc: jasow...@redhat.com
> Cc: arm...@redhat.com
> Cc: berra...@redhat.com
> Signed-off-by: Mao Zhongyi 
> Reviewed-by: Daniel P. Berrange 

Reviewed-by: Markus Armbruster 

[Qemu-devel] [PULL 4/8] s390x: fix realize inheritance for kvm-flic

[Christian Borntraeger]

From: Halil Pasic 

Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
2016-12-09) introduces a common realize (intended to be common for all
the subclasses) for flic, but fails to make sure the kvm-flic which had
its own is actually calling this common realize.

This omission fortunately does not result in a grave problem. The common
realize was only supposed to catch a possible programming mistake by
validating a value of a property set via the compat machine macros. Since
there was no programming mistake we don't need this fixed for stable.

Let's fix this problem by making sure kvm flic honors the realize of its
parent class.

Let us also improve on the error message we would hypothetically emit
when the validation fails.

Signed-off-by: Halil Pasic 
Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
Reviewed-by: Dong Jia Shi 
Reviewed-by: Yi Min Zhao 
Reviewed-by: Cornelia Huck 
Signed-off-by: Christian Borntraeger 
---
 hw/intc/s390_flic.c |  4 ++--
 hw/intc/s390_flic_kvm.c | 17 +
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index a99a350..837158b 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev, 
Error **errp)
 uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
 
 if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
-error_setg(errp, "flic adapter_routes_max_batch too big"
-   "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
+error_setg(errp, "flic property adapter_routes_max_batch too big"
+   " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);
 }
 }
 
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index 56f1b81..0bcd49f 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -392,6 +392,17 @@ static const VMStateDescription kvm_s390_flic_vmstate = {
 }
 };
 
+typedef struct KVMS390FLICStateClass {
+S390FLICStateClass parent_class;
+DeviceRealize parent_realize;
+} KVMS390FLICStateClass;
+
+#define KVM_S390_FLIC_GET_CLASS(obj) \
+OBJECT_GET_CLASS(KVMS390FLICStateClass, (obj), TYPE_KVM_S390_FLIC)
+
+#define KVM_S390_FLIC_CLASS(klass) \
+OBJECT_CLASS_CHECK(KVMS390FLICStateClass, (klass), TYPE_KVM_S390_FLIC)
+
 static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
 {
 KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
@@ -400,6 +411,10 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error 
**errp)
 int ret;
 Error *errp_local = NULL;
 
+KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);
+if (errp_local) {
+goto fail;
+}
 flic_state->fd = -1;
 if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
 error_setg_errno(&errp_local, errno, "KVM is missing capability"
@@ -454,6 +469,7 @@ static void kvm_s390_flic_class_init(ObjectClass *oc, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(oc);
 S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc);
 
+KVM_S390_FLIC_CLASS(oc)->parent_realize = dc->realize;
 dc->realize = kvm_s390_flic_realize;
 dc->vmsd = &kvm_s390_flic_vmstate;
 dc->reset = kvm_s390_flic_reset;
@@ -468,6 +484,7 @@ static const TypeInfo kvm_s390_flic_info = {
 .name  = TYPE_KVM_S390_FLIC,
 .parent= TYPE_S390_FLIC_COMMON,
 .instance_size = sizeof(KVMS390FLICState),
+.class_size= sizeof(KVMS390FLICStateClass),
 .class_init= kvm_s390_flic_class_init,
 };
 
-- 
2.7.4

[Qemu-devel] [PULL 8/8] hw/s390x/ipl: Fix endianness problem with netboot_start_addr

[Christian Borntraeger]

From: Thomas Huth 

The start address has to be stored in big endian byte order
in the iplb.ccw block for the guest.

Signed-off-by: Thomas Huth 
Message-Id: <1499268345-12552-1-git-send-email-th...@redhat.com>
Reviewed-by: Cornelia Huck 
Signed-off-by: Christian Borntraeger 
---
 hw/s390x/ipl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 4e6469d..cc36003 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -418,7 +418,7 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
 error_report_err(err);
 vm_stop(RUN_STATE_INTERNAL_ERROR);
 }
-ipl->iplb.ccw.netboot_start_addr = ipl->start_addr;
+ipl->iplb.ccw.netboot_start_addr = cpu_to_be64(ipl->start_addr);
 }
 }
 
-- 
2.7.4

[Qemu-devel] [PULL 5/8] s390x/MAINTAINERS: Update my email address

[Christian Borntraeger]

From: Cornelia Huck 

Signed-off-by: Cornelia Huck 
Message-Id: <20170704092215.13742-2-coh...@redhat.com>
Signed-off-by: Christian Borntraeger 
---
 MAINTAINERS | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 839f7ca..4e17216 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -299,7 +299,7 @@ F: target/ppc/kvm.c
 
 S390
 M: Christian Borntraeger 
-M: Cornelia Huck 
+M: Cornelia Huck 
 M: Alexander Graf 
 S: Maintained
 F: target/s390x/kvm.c
@@ -778,7 +778,7 @@ F: include/hw/sparc/grlib.h
 S390 Machines
 -
 S390 Virtio-ccw
-M: Cornelia Huck 
+M: Cornelia Huck 
 M: Christian Borntraeger 
 M: Alexander Graf 
 S: Supported
@@ -1006,7 +1006,7 @@ F: hw/vfio/*
 F: include/hw/vfio/
 
 vfio-ccw
-M: Cornelia Huck 
+M: Cornelia Huck 
 S: Supported
 F: hw/vfio/ccw.c
 F: hw/s390x/s390-ccw.c
@@ -1048,7 +1048,7 @@ F: tests/virtio-blk-test.c
 T: git git://github.com/stefanha/qemu.git block
 
 virtio-ccw
-M: Cornelia Huck 
+M: Cornelia Huck 
 M: Christian Borntraeger 
 S: Supported
 F: hw/s390x/virtio-ccw.[hc]
-- 
2.7.4

[Qemu-devel] [PULL 3/8] s390x: fix error propagation in kvm-flic's realize

[Christian Borntraeger]

From: Halil Pasic 

>From the moment it was introduced by commit a2875e6f98 ("s390x/kvm:
implement floating-interrupt controller device", 2013-07-16) the kvm-flic
is not making realize fail properly in case it's impossible to create the
KVM device which basically serves as a backend and is absolutely
essential for having an operational kvm-flic.

Let's fix this by making sure we do proper error propagation in realize.

Signed-off-by: Halil Pasic 
Fixes: a2875e6f98 "s390x/kvm: implement floating-interrupt controller device"
Reviewed-by: Dong Jia Shi 
Reviewed-by: Yi Min Zhao 
Signed-off-by: Christian Borntraeger 
---
 hw/intc/s390_flic_kvm.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index b4c61d8..56f1b81 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -15,6 +15,7 @@
 #include "cpu.h"
 #include 
 #include "qemu/error-report.h"
+#include "qapi/error.h"
 #include "hw/sysbus.h"
 #include "sysemu/kvm.h"
 #include "hw/s390x/s390_flic.h"
@@ -397,18 +398,22 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error 
**errp)
 struct kvm_create_device cd = {0};
 struct kvm_device_attr test_attr = {0};
 int ret;
+Error *errp_local = NULL;
 
 flic_state->fd = -1;
 if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
+error_setg_errno(&errp_local, errno, "KVM is missing capability"
+ " KVM_CAP_DEVICE_CTRL");
 trace_flic_no_device_api(errno);
-return;
+goto fail;
 }
 
 cd.type = KVM_DEV_TYPE_FLIC;
 ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
 if (ret < 0) {
+error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
 trace_flic_create_device(errno);
-return;
+goto fail;
 }
 flic_state->fd = cd.fd;
 
@@ -417,6 +422,9 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error 
**errp)
 flic_state->clear_io_supported = !ioctl(flic_state->fd,
 KVM_HAS_DEVICE_ATTR, test_attr);
 
+return;
+fail:
+error_propagate(errp, errp_local);
 }
 
 static void kvm_s390_flic_reset(DeviceState *dev)
-- 
2.7.4

[Qemu-devel] [PULL 1/8] s390x: vmstatify config migration for virtio-ccw

[Christian Borntraeger]

From: Halil Pasic 

Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for
flexibility (extending using subsections) and for fun.

To achieve this we need to hack the config_vector, which is VirtIODevice
(that is common virtio) state, in the middle of the VirtioCcwDevice state
representation.  This is somewhat ugly, but we have no choice because the
stream format needs to be preserved.

Almost no changes in behavior. Exception is everything that comes with
vmstate like extra bookkeeping about what's in the stream, and maybe some
extra checks and better error reporting.

Signed-off-by: Halil Pasic 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Reviewed-by: Cornelia Huck 
Message-Id: <20170703213414.94298-1-pa...@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger 
---
 hw/intc/s390_flic.c  |  28 
 hw/s390x/ccw-device.c|  10 ++
 hw/s390x/ccw-device.h|   4 +
 hw/s390x/css.c   | 378 +--
 hw/s390x/virtio-ccw.c| 158 +-
 include/hw/s390x/css.h   |  12 +-
 include/hw/s390x/s390_flic.h |   5 +
 7 files changed, 358 insertions(+), 237 deletions(-)

diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index a26e906..a99a350 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -17,6 +17,7 @@
 #include "trace.h"
 #include "hw/qdev.h"
 #include "qapi/error.h"
+#include "hw/s390x/s390-virtio-ccw.h"
 
 S390FLICState *s390_get_flic(void)
 {
@@ -136,3 +137,30 @@ static void qemu_s390_flic_register_types(void)
 }
 
 type_init(qemu_s390_flic_register_types)
+
+const VMStateDescription vmstate_adapter_info = {
+.name = "s390_adapter_info",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT64(ind_offset, AdapterInfo),
+/*
+ * We do not have to migrate neither the id nor the addresses.
+ * The id is set by css_register_io_adapter and the addresses
+ * are set based on the IndAddr objects after those get mapped.
+ */
+VMSTATE_END_OF_LIST()
+},
+};
+
+const VMStateDescription vmstate_adapter_routes = {
+
+.name = "s390_adapter_routes",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_STRUCT(adapter, AdapterRoutes, 1, vmstate_adapter_info,
+   AdapterInfo),
+VMSTATE_END_OF_LIST()
+}
+};
diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
index fb8d640..f9bfa15 100644
--- a/hw/s390x/ccw-device.c
+++ b/hw/s390x/ccw-device.c
@@ -50,6 +50,16 @@ static void ccw_device_class_init(ObjectClass *klass, void 
*data)
 dc->props = ccw_device_properties;
 }
 
+const VMStateDescription vmstate_ccw_dev = {
+.name = "s390_ccw_dev",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_STRUCT_POINTER(sch, CcwDevice, vmstate_subch_dev, SubchDev),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static const TypeInfo ccw_device_info = {
 .name = TYPE_CCW_DEVICE,
 .parent = TYPE_DEVICE,
diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
index 89c8e5d..4e6af28 100644
--- a/hw/s390x/ccw-device.h
+++ b/hw/s390x/ccw-device.h
@@ -27,6 +27,10 @@ typedef struct CcwDevice {
 CssDevId subch_id;
 } CcwDevice;
 
+extern const VMStateDescription vmstate_ccw_dev;
+#define VMSTATE_CCW_DEVICE(_field, _state) \
+VMSTATE_STRUCT(_field, _state, 1, vmstate_ccw_dev, CcwDevice)
+
 typedef struct CCWDeviceClass {
 DeviceClass parent_class;
 void (*unplug)(HotplugHandler *, DeviceState *, Error **);
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 599805d..d67fffa 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -22,6 +22,7 @@
 #include "hw/s390x/css.h"
 #include "trace.h"
 #include "hw/s390x/s390_flic.h"
+#include "hw/s390x/s390-virtio-ccw.h"
 
 typedef struct CrwContainer {
 CRW crw;
@@ -40,6 +41,181 @@ typedef struct SubchSet {
 unsigned long devnos_used[BITS_TO_LONGS(MAX_SCHID + 1)];
 } SubchSet;
 
+static const VMStateDescription vmstate_scsw = {
+.name = "s390_scsw",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT16(flags, SCSW),
+VMSTATE_UINT16(ctrl, SCSW),
+VMSTATE_UINT32(cpa, SCSW),
+VMSTATE_UINT8(dstat, SCSW),
+VMSTATE_UINT8(cstat, SCSW),
+VMSTATE_UINT16(count, SCSW),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static const VMStateDescription vmstate_pmcw = {
+.name = "s390_pmcw",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(intparm, PMCW),
+VMSTATE_UINT16(flags, PMCW),
+VMSTATE_UINT16(devno, PMCW),
+VMSTATE_UINT8(lpm, PMCW),
+VMSTATE_UINT8(pnom, PMCW),
+VMSTATE_UINT8(lpum, PMCW),
+VMSTATE_UINT8(pim, PMCW),
+VMSTATE_UINT16(mbi, PMCW),
+VMSTATE_UINT8(pom,

[Qemu-devel] [PULL 6/8] s390x: return unavailable features via query-cpu-definitions

[Christian Borntraeger]

From: Viktor Mihajlovski 

The response for query-cpu-definitions didn't include the
unavailable-features field, which is used by libvirt to figure
out whether a certain cpu model is usable on the host.

The unavailable features are now computed by obtaining the host CPU
model and comparing it against the known CPU models. The comparison
takes into account the generation, the GA level and the feature
bitmaps. In the case of a CPU generation/GA level mismatch
a feature called "type" is reported to be missing.

As a result, the output of virsh domcapabilities would change
from something like
 ...
 
  z10EC-base
  z9EC-base
  z196.2-base
  z900-base
  z990
 ...
to
 ...
 
  z10EC-base
  z9EC-base
  z196.2-base
  z900-base
  z990
 ...

Signed-off-by: Viktor Mihajlovski 
Message-Id: <1499082529-16970-1-git-send-email-mihaj...@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand 
Acked-by: Cornelia Huck 
Signed-off-by: Christian Borntraeger 
---
 target/s390x/cpu_models.c | 62 +++
 1 file changed, 57 insertions(+), 5 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 63903c2..7cb55dc 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -283,10 +283,41 @@ void s390_cpu_list(FILE *f, fprintf_function print)
 }
 }
 
+static S390CPUModel *get_max_cpu_model(Error **errp);
+
 #ifndef CONFIG_USER_ONLY
+static void list_add_feat(const char *name, void *opaque);
+
+static void check_unavailable_features(const S390CPUModel *max_model,
+   const S390CPUModel *model,
+   strList **unavailable)
+{
+S390FeatBitmap missing;
+
+/* check general model compatibility */
+if (max_model->def->gen < model->def->gen ||
+(max_model->def->gen == model->def->gen &&
+ max_model->def->ec_ga < model->def->ec_ga)) {
+list_add_feat("type", unavailable);
+}
+
+/* detect missing features if any to properly report them */
+bitmap_andnot(missing, model->features, max_model->features,
+  S390_FEAT_MAX);
+if (!bitmap_empty(missing, S390_FEAT_MAX)) {
+s390_feat_bitmap_to_ascii(missing, unavailable, list_add_feat);
+}
+}
+
+struct CpuDefinitionInfoListData {
+CpuDefinitionInfoList *list;
+S390CPUModel *model;
+};
+
 static void create_cpu_model_list(ObjectClass *klass, void *opaque)
 {
-CpuDefinitionInfoList **cpu_list = opaque;
+struct CpuDefinitionInfoListData *cpu_list_data = opaque;
+CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
 CpuDefinitionInfoList *entry;
 CpuDefinitionInfo *info;
 char *name = g_strdup(object_class_get_name(klass));
@@ -300,7 +331,19 @@ static void create_cpu_model_list(ObjectClass *klass, void 
*opaque)
 info->migration_safe = scc->is_migration_safe;
 info->q_static = scc->is_static;
 info->q_typename = g_strdup(object_class_get_name(klass));
-
+/* check for unavailable features */
+if (cpu_list_data->model) {
+Object *obj;
+S390CPU *sc;
+obj = object_new(object_class_get_name(klass));
+sc = S390_CPU(obj);
+if (sc->model) {
+info->has_unavailable_features = true;
+check_unavailable_features(cpu_list_data->model, sc->model,
+   &info->unavailable_features);
+}
+object_unref(obj);
+}
 
 entry = g_malloc0(sizeof(*entry));
 entry->value = info;
@@ -310,11 +353,20 @@ static void create_cpu_model_list(ObjectClass *klass, 
void *opaque)
 
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
 {
-CpuDefinitionInfoList *list = NULL;
+struct CpuDefinitionInfoListData list_data = {
+.list = NULL,
+};
 
-object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false, &list);
+list_data.model = get_max_cpu_model(errp);
+if (*errp) {
+error_free(*errp);
+*errp = NULL;
+}
 
-return list;
+object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
+ &list_data);
+
+return list_data.list;
 }
 
 static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
-- 
2.7.4

Re: [Qemu-devel] [PATCH v8 4/4] net/socket: Improve -net socket error reporting

[Markus Armbruster]

Mao Zhongyi  writes:

> When -net socket fails, it first reports a specific error, then
> a generic one, like this:
>
> $ qemu-system-x86_64 -net socket,
> qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=, 
> mcast= or udp= is required
> qemu-system-x86_64: -net socket: Device 'socket' could not be initialized
>
> Convert net_socket_*_init() to Error to get rid of the superfluous second
> error message. After the patch, the effect like this:
>
> $ qemu-system-x86_64 -net socket,
> qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=, 
> mcast= or udp= is required
>
> At the same time, add many explicit error handling message when it fails.

Suggest "This also fixes a few silent failures to report an error."

Perhaps Jason can touch this up for you.

> Cc: jasow...@redhat.com
> Cc: arm...@redhat.com
> Cc: berra...@redhat.com
> Signed-off-by: Mao Zhongyi 
> ---
>  net/socket.c | 92 
> +---
>  1 file changed, 44 insertions(+), 48 deletions(-)
>
> diff --git a/net/socket.c b/net/socket.c
> index 1d4c515..ca1878f 100644
> --- a/net/socket.c
> +++ b/net/socket.c
[...]
> @@ -746,7 +742,7 @@ int net_init_socket(const Netdev *netdev, const char 
> *name,
>  /* if sock->localaddr is missing, it has been initialized to "all 
> bits
>   * zero" */
>  if (net_socket_mcast_init(peer, "socket", name, sock->mcast,
> -sock->localaddr) == -1) {
> +sock->localaddr, errp) < 0) {

Indent like this:

 sock->localaddr, errp) < 0) {

Perhaps Jason can touch this up for you.

>  return -1;
>  }
>  return 0;
> @@ -754,11 +750,11 @@ int net_init_socket(const Netdev *netdev, const char 
> *name,
>  
>  assert(sock->has_udp);
>  if (!sock->has_localaddr) {
> -error_report("localaddr= is mandatory with udp=");
> +error_setg(errp, "localaddr= is mandatory with udp=");
>  return -1;
>  }
> -if (net_socket_udp_init(peer, "socket", name, sock->udp, 
> sock->localaddr) ==
> --1) {
> +if (net_socket_udp_init(peer, "socket", name, sock->udp, sock->localaddr,
> +errp) < 0) {

Likewise.

>  return -1;
>  }
>  return 0;

With the indentation fixed:
Reviewed-by: Markus Armbruster 

[Qemu-devel] [PULL 2/8] s390x/3270: fix instruction interception handler

[Christian Borntraeger]

From: Dong Jia Shi 

Commit bab482d7405f ("s390x/css: ccw translation infrastructure")
introduced instruction interception handler for different types of
subchannels. For emulated 3270 devices, we should assign the virtual
subchannel handler to them during device realization process, or 3270
will not work.

Fixes: bab482d7405f ("s390x/css: ccw translation infrastructure")

Reviewed-by: Jing Liu 
Reviewed-by: Halil Pasic 
Reviewed-by: Cornelia Huck 
Signed-off-by: Dong Jia Shi 
Signed-off-by: Christian Borntraeger 
---
 hw/s390x/3270-ccw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/s390x/3270-ccw.c b/hw/s390x/3270-ccw.c
index 6e6eee4..1554aa2 100644
--- a/hw/s390x/3270-ccw.c
+++ b/hw/s390x/3270-ccw.c
@@ -126,6 +126,7 @@ static void emulated_ccw_3270_realize(DeviceState *ds, 
Error **errp)
 sch->id.cu_type = EMULATED_CCW_3270_CU_TYPE;
 css_sch_build_virtual_schib(sch, (uint8_t)chpid,
 EMULATED_CCW_3270_CHPID_TYPE);
+sch->do_subchannel_work = do_subchannel_work_virtual;
 sch->ccw_cb = emulated_ccw_3270_cb;
 
 ck->init(dev, &err);
-- 
2.7.4

[Qemu-devel] [PULL 7/8] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[Christian Borntraeger]

From: QingFeng Hao 

This patch is based on a similar patch from Stefan Hajnoczi -
commit c324fd0a39c ("virtio-pci: use ioeventfd even when KVM is disabled")

Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.
Currently we don't have an equivalent to "memory: emulate ioeventfd"
for ccw yet, but that this doesn't hurt and qemu-iotests 068 can pass with
skipping iothread arguments.

I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.

This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.

Signed-off-by: QingFeng Hao 
Reviewed-by: Cornelia Huck 
Message-Id: <20170704132350.11874-2-ha...@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Christian Borntraeger 
---
 hw/s390x/virtio-ccw.c | 2 +-
 target/s390x/cpu.h| 6 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index f0e7fc8..e18fd26 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -789,7 +789,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, 
Error **errp)
 sch->cssid, sch->ssid, sch->schid, sch->devno,
 ccw_dev->devno.valid ? "user-configured" : "auto-configured");
 
-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
 dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
 }
 
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 9faca04..bdb9bdb 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1264,7 +1264,11 @@ static inline int 
s390_assign_subch_ioeventfd(EventNotifier *notifier,
   uint32_t sch_id, int vq,
   bool assign)
 {
-return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+if (kvm_enabled()) {
+return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+} else {
+return 0;
+}
 }
 
 static inline void s390_crypto_reset(void)
-- 
2.7.4

[Qemu-devel] [Bug 1702621] [NEW] colo: secondary vm crash during loadvm

[pork-prince]

Public bug reported:

Following document 'COLO-FT.txt', I test colo feature on my hosts. It seems 
goes well. But after a while the secondary vm crash.  The stack is as follows:
#0  0x7f191456dc37 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x7f1914571028 in abort () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x7f1914566bf6 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#3  0x7f1914566ca2 in __assert_fail () from /lib/x86_64-linux-gnu/libc.so.6
#4  0x564154ad9147 in pcibus_reset (qbus=0x564156760d10) at 
../hw/pci/pci.c:311
#5  0x564154a07cdb in qbus_reset_one (bus=0x564156760d10, opaque=0x0) at 
hw/core/qdev.c:319
#6  0x564154a0d721 in qbus_walk_children (bus=0x564156760d10, pre_devfn=0, 
pre_busfn=0, 
post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
at hw/core/bus.c:68
#7  0x564154a08b4d in qdev_walk_children (dev=0x56415675f2b0, pre_devfn=0, 
pre_busfn=0, 
post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
at hw/core/qdev.c:617
#8  0x564154a0d6e5 in qbus_walk_children (bus=0x564156594d30, pre_devfn=0, 
pre_busfn=0, 
post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
at hw/core/bus.c:59
#9  0x564154a07df5 in qbus_reset_all (bus=0x564156594d30) at 
hw/core/qdev.c:336
#10 0x564154a07e3a in qbus_reset_all_fn (opaque=0x564156594d30) at 
hw/core/qdev.c:342
#11 0x564154a0e222 in qemu_devices_reset () at hw/core/reset.c:69
#12 0x5641548b3b47 in pc_machine_reset () at /vms/git/qemu/hw/i386/pc.c:2234
#13 0x564154972ca7 in qemu_system_reset (report=false) at vl.c:1697
#14 0x564154b9d007 in colo_process_incoming_thread (opaque=0x5641553c1280) 
at migration/colo.c:617
#15 0x7f1914907184 in start_thread () from 
/lib/x86_64-linux-gnu/libpthread.so.0
#16 0x7f1914634bed in clone () from /lib/x86_64-linux-gnu/libc.so.6

(gdb) frame 4
#4  0x564154ad9147 in pcibus_reset (qbus=0x564156760d10) at 
../hw/pci/pci.c:311
warning: Source file is more recent than executable.
311 assert(bus->irq_count[i] == 0);
(gdb) ^CQuit
(gdb) p bus->irq_count[i]
$1 = -1

** Affects: qemu
 Importance: Undecided
 Status: New

** Attachment added: "command on primary and secondary vm"
   
https://bugs.launchpad.net/bugs/1702621/+attachment/4910131/+files/colo-commands.txt

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

Title:
  colo: secondary vm crash during loadvm

Status in QEMU:
  New

Bug description:
  Following document 'COLO-FT.txt', I test colo feature on my hosts. It seems 
goes well. But after a while the secondary vm crash.  The stack is as follows:
  #0  0x7f191456dc37 in raise () from /lib/x86_64-linux-gnu/libc.so.6
  #1  0x7f1914571028 in abort () from /lib/x86_64-linux-gnu/libc.so.6
  #2  0x7f1914566bf6 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
  #3  0x7f1914566ca2 in __assert_fail () from 
/lib/x86_64-linux-gnu/libc.so.6
  #4  0x564154ad9147 in pcibus_reset (qbus=0x564156760d10) at 
../hw/pci/pci.c:311
  #5  0x564154a07cdb in qbus_reset_one (bus=0x564156760d10, opaque=0x0) at 
hw/core/qdev.c:319
  #6  0x564154a0d721 in qbus_walk_children (bus=0x564156760d10, 
pre_devfn=0, pre_busfn=0, 
  post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
  at hw/core/bus.c:68
  #7  0x564154a08b4d in qdev_walk_children (dev=0x56415675f2b0, 
pre_devfn=0, pre_busfn=0, 
  post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
  at hw/core/qdev.c:617
  #8  0x564154a0d6e5 in qbus_walk_children (bus=0x564156594d30, 
pre_devfn=0, pre_busfn=0, 
  post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
  at hw/core/bus.c:59
  #9  0x564154a07df5 in qbus_reset_all (bus=0x564156594d30) at 
hw/core/qdev.c:336
  #10 0x564154a07e3a in qbus_reset_all_fn (opaque=0x564156594d30) at 
hw/core/qdev.c:342
  #11 0x564154a0e222 in qemu_devices_reset () at hw/core/reset.c:69
  #12 0x5641548b3b47 in pc_machine_reset () at 
/vms/git/qemu/hw/i386/pc.c:2234
  #13 0x564154972ca7 in qemu_system_reset (report=false) at vl.c:1697
  #14 0x564154b9d007 in colo_process_incoming_thread 
(opaque=0x5641553c1280) at migration/colo.c:617
  #15 0x7f1914907184 in start_thread () from 
/lib/x86_64-linux-gnu/libpthread.so.0
  #16 0x7f1914634bed in clone () from /lib/x86_64-linux-gnu/libc.so.6

  (gdb) frame 4
  #4  0x564154ad9147 in pcibus_reset (qbus=0x564156760d10) at 
../hw/pci/pci.c:311
  warning: Source file is more recent than executable.
  311 assert(bus->irq_count[i] == 0);
  (gdb) ^CQuit
  (gdb) p bus->irq_count[i]
  $1 = -1

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

[Qemu-devel] [PULL 0/8] s390x/kvm/migration: fixes, enhancements and cleanups

[Christian Borntraeger]

Peter,

the following changes since commit 2185c93ba80f81bfa27ce6f259c7f2ef4f08b668:

  Merge remote-tracking branch 
'remotes/edgar/tags/edgar/xilinx-next.for-upstream' into staging (2017-07-04 
13:05:30 +0100)

are available in the git repository at:

  git://github.com/borntraeger/qemu.git tags/s390x-20170706

for you to fetch changes up to 1045e3cdafaf1218d9f771080ba5c9d4f64346e4:

  hw/s390x/ipl: Fix endianness problem with netboot_start_addr (2017-07-05 
19:46:30 +0200)


s390x/kvm/migration: fixes, enhancements and cleanups

- new email address for Cornelia
- Fixes: 3270, flic, virtio-scsi-ccw, ipl
- Enhancements, cpumodel, migration


Cornelia Huck (1):
  s390x/MAINTAINERS: Update my email address

Dong Jia Shi (1):
  s390x/3270: fix instruction interception handler

Halil Pasic (3):
  s390x: vmstatify config migration for virtio-ccw
  s390x: fix error propagation in kvm-flic's realize
  s390x: fix realize inheritance for kvm-flic

QingFeng Hao (1):
  virtio-scsi-ccw: use ioeventfd even when KVM is disabled

Thomas Huth (1):
  hw/s390x/ipl: Fix endianness problem with netboot_start_addr

Viktor Mihajlovski (1):
  s390x: return unavailable features via query-cpu-definitions

 MAINTAINERS  |   8 +-
 hw/intc/s390_flic.c  |  32 +++-
 hw/intc/s390_flic_kvm.c  |  29 +++-
 hw/s390x/3270-ccw.c  |   1 +
 hw/s390x/ccw-device.c|  10 ++
 hw/s390x/ccw-device.h|   4 +
 hw/s390x/css.c   | 378 +--
 hw/s390x/ipl.c   |   2 +-
 hw/s390x/virtio-ccw.c| 160 +-
 include/hw/s390x/css.h   |  12 +-
 include/hw/s390x/s390_flic.h |   5 +
 target/s390x/cpu.h   |   6 +-
 target/s390x/cpu_models.c|  62 ++-
 13 files changed, 456 insertions(+), 253 deletions(-)

Re: [Qemu-devel] [PATCH] qom: enforce readonly nature of link's check callback

[Fam Zheng]

On Thu, 07/06 00:16, no-re...@patchew.org wrote:
> /var/tmp/patchew-tester-tmp-s55a7o8g/src/hw/virtio/virtio-crypto.c: In 
> function ‘virtio_crypto_instance_init’:
> /var/tmp/patchew-tester-tmp-s55a7o8g/src/hw/virtio/virtio-crypto.c:918:30: 
> error: passing argument 5 of ‘object_property_add_link’ from incompatible 
> pointer type [-Werror=incompatible-pointer-types]
>   virtio_crypto_check_cryptodev_is_used,
>   ^
> In file included from 
> /var/tmp/patchew-tester-tmp-s55a7o8g/src/include/hw/hw.h:10:0,
>  from 
> /var/tmp/patchew-tester-tmp-s55a7o8g/src/include/hw/qdev.h:4,
>  from 
> /var/tmp/patchew-tester-tmp-s55a7o8g/src/hw/virtio/virtio-crypto.c:15:
> /var/tmp/patchew-tester-tmp-s55a7o8g/src/include/qom/object.h:1353:6: note: 
> expected ‘void (*)(const Object *, const char *, Object *, Error **) {aka 
> void (*)(const struct Object *, const char *, struct Object *, struct Error 
> **)}’ but argument is of type ‘void (*)(Object *, const char *, Object *, 
> Error **) {aka void (*)(struct Object *, const char *, struct Object *, 
> struct Error **)}’
>  void object_property_add_link(Object *obj, const char *name,
>   ^~~~

No worry, I've taken care of it when including this patch in my series.

Fam

Re: [Qemu-devel] [PATCH] ppc/kvm: have the "family" CPU alias to point to TYPE_HOST_POWERPC_CPU

[David Gibson]

On Wed, Jul 05, 2017 at 10:49:52AM +0200, Greg Kurz wrote:
> When running KVM on POWER, we allow the user to pass "-cpu POWERx" instead
> of "-cpu host". This is achieved by patching the ppc_cpu_aliases[] array
> so that "POWERx" points to the CPU class with the same PVR as the host CPU.
> This causes CPUs to be instantiated from this CPU class instead of the
> TYPE_HOST_POWERPC_CPU class which is used with "-cpu host". These CPUs thus
> miss all the KVM specific tuning from kvmppc_host_cpu_class_init().
> 
> This currently causes QEMU with "-cpu POWER9" to fail when running KVM on a
> POWER9 DD1 host:
> 
> qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only
>  "-cpu host" is possible
> kvm_init_vcpu failed: Invalid argument
> 
> Let's have the "POWERx" alias to point to TYPE_HOST_POWERPC_CPU directly,
> so that "-cpu POWERx" instantiates CPUs from the same class as "-cpu host".
> 
> Signed-off-by: Greg Kurz 

Nice.  I had mistakenly thought it already did that.

I've applied this to ppc-for-2.10.

Just to make sure I've understood, this obsoletes Laurent's earlier
patches moving DD1 workarounds about, yes?

-- 
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] ppc/kvm: have the "family" CPU alias to point to TYPE_HOST_POWERPC_CPU

[Greg Kurz]

On Thu, 6 Jul 2017 17:32:33 +1000
David Gibson  wrote:

> On Wed, Jul 05, 2017 at 10:49:52AM +0200, Greg Kurz wrote:
> > When running KVM on POWER, we allow the user to pass "-cpu POWERx" instead
> > of "-cpu host". This is achieved by patching the ppc_cpu_aliases[] array
> > so that "POWERx" points to the CPU class with the same PVR as the host CPU.
> > This causes CPUs to be instantiated from this CPU class instead of the
> > TYPE_HOST_POWERPC_CPU class which is used with "-cpu host". These CPUs thus
> > miss all the KVM specific tuning from kvmppc_host_cpu_class_init().
> > 
> > This currently causes QEMU with "-cpu POWER9" to fail when running KVM on a
> > POWER9 DD1 host:
> > 
> > qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only
> >  "-cpu host" is possible
> > kvm_init_vcpu failed: Invalid argument
> > 
> > Let's have the "POWERx" alias to point to TYPE_HOST_POWERPC_CPU directly,
> > so that "-cpu POWERx" instantiates CPUs from the same class as "-cpu host".
> > 
> > Signed-off-by: Greg Kurz   
> 
> Nice.  I had mistakenly thought it already did that.
> 
> I've applied this to ppc-for-2.10.
> 
> Just to make sure I've understood, this obsoletes Laurent's earlier
> patches moving DD1 workarounds about, yes?
> 

Yes it does.

Cheers,

--
Greg


pgpZJlp4_KV6b.pgp
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1702621] Re: colo: secondary vm crash during loadvm

[pork-prince]

The qemu version is 2.9.0 release.
The 'irq_count' and 'irq_state' are sent by private vm, and loaded by secondary 
vm.  When they sent by private vm, they maybe not in a consistent state. So 
sometimes 'bus->irq_count[i]' becomes '-1' on secondary vm.
I deleted the assertions and then tested it several times, it worked well

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

Title:
  colo: secondary vm crash during loadvm

Status in QEMU:
  New

Bug description:
  Following document 'COLO-FT.txt', I test colo feature on my hosts. It seems 
goes well. But after a while the secondary vm crash.  The stack is as follows:
  #0  0x7f191456dc37 in raise () from /lib/x86_64-linux-gnu/libc.so.6
  #1  0x7f1914571028 in abort () from /lib/x86_64-linux-gnu/libc.so.6
  #2  0x7f1914566bf6 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
  #3  0x7f1914566ca2 in __assert_fail () from 
/lib/x86_64-linux-gnu/libc.so.6
  #4  0x564154ad9147 in pcibus_reset (qbus=0x564156760d10) at 
../hw/pci/pci.c:311
  #5  0x564154a07cdb in qbus_reset_one (bus=0x564156760d10, opaque=0x0) at 
hw/core/qdev.c:319
  #6  0x564154a0d721 in qbus_walk_children (bus=0x564156760d10, 
pre_devfn=0, pre_busfn=0, 
  post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
  at hw/core/bus.c:68
  #7  0x564154a08b4d in qdev_walk_children (dev=0x56415675f2b0, 
pre_devfn=0, pre_busfn=0, 
  post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
  at hw/core/qdev.c:617
  #8  0x564154a0d6e5 in qbus_walk_children (bus=0x564156594d30, 
pre_devfn=0, pre_busfn=0, 
  post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
  at hw/core/bus.c:59
  #9  0x564154a07df5 in qbus_reset_all (bus=0x564156594d30) at 
hw/core/qdev.c:336
  #10 0x564154a07e3a in qbus_reset_all_fn (opaque=0x564156594d30) at 
hw/core/qdev.c:342
  #11 0x564154a0e222 in qemu_devices_reset () at hw/core/reset.c:69
  #12 0x5641548b3b47 in pc_machine_reset () at 
/vms/git/qemu/hw/i386/pc.c:2234
  #13 0x564154972ca7 in qemu_system_reset (report=false) at vl.c:1697
  #14 0x564154b9d007 in colo_process_incoming_thread 
(opaque=0x5641553c1280) at migration/colo.c:617
  #15 0x7f1914907184 in start_thread () from 
/lib/x86_64-linux-gnu/libpthread.so.0
  #16 0x7f1914634bed in clone () from /lib/x86_64-linux-gnu/libc.so.6

  (gdb) frame 4
  #4  0x564154ad9147 in pcibus_reset (qbus=0x564156760d10) at 
../hw/pci/pci.c:311
  warning: Source file is more recent than executable.
  311 assert(bus->irq_count[i] == 0);
  (gdb) ^CQuit
  (gdb) p bus->irq_count[i]
  $1 = -1

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

Re: [Qemu-devel] [PATCH] hmp: Update info vnc

[Daniel P. Berrange]

On Wed, Jul 05, 2017 at 05:33:24PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrange (berra...@redhat.com) wrote:
> > On Wed, Jul 05, 2017 at 03:43:02PM +0100, Dr. David Alan Gilbert (git) 
> > wrote:
> > > From: "Dr. David Alan Gilbert" 
> > > 
> > > The QMP query-vnc interfaces have gained a lot more information that
> > > the HMP interfaces hasn't got yet. Update it.
> > > 
> > > Note the output format has changed, but this is HMP so that's OK.
> > > 
> > > In particular, this now includes client information for reverse
> > > connections:
> > > 
> > > -vnc :0
> > > (qemu) info vnc
> > > default:
> > >   Server: 0.0.0.0:5900 (ipv4)
> > > Auth: none (Sub: none)
> > >   Auth: none (Sub: none)
> > 
> > If you're reporting Auth: against the individual Server entry,
> > there's no reason to report it at the top level too. We only
> > keep that duplication in QMP for sake of backcompat, which
> > is not important for HMP
> 
> Where would it get reported for a 'reverse' connection then?
> That has no server entry.

Ah right. So lets only report it at the top level, if operating
in reverse mode.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 12/17] migration: add postcopy migration of dirty bitmaps

[Vladimir Sementsov-Ogievskiy]

06.07.2017 00:46, John Snow wrote:


On 07/05/2017 05:24 AM, Vladimir Sementsov-Ogievskiy wrote:

16.02.2017 16:04, Fam Zheng wrote:

+dbms->node_name = bdrv_get_node_name(bs);
+if (!dbms->node_name || dbms->node_name[0] == '\0') {
+dbms->node_name = bdrv_get_device_name(bs);
+}
+dbms->bitmap = bitmap;

What protects the case that the bitmap is released before migration
completes?


What is the source of such deletion? qmp command? Theoretically possible.

I see the following variants:

1. additional variable BdrvDirtyBItmap.migration, which forbids bitmap
deletion

2. make bitmap anonymous (bdrv_dirty_bitmap_make_anon) - it will not be
available through qmp


Making the bitmap anonymous would forbid us to query the bitmap, which
there is no general reason to do, excepting the idea that a third party
attempting to use the bitmap during a migration is probably a bad idea.
I don't really like the idea of "hiding" information from the user,
though, because then we'd have to worry about name collisions when we
de-anonymized the bitmap again. That's not so palatable.


what do you think?


The modes for bitmaps are getting messy.

As a reminder, the officially exposed "modes" of a bitmap are currently:

FROZEN: Cannot be reset/deleted. Implication is that the bitmap is
otherwise "ACTIVE."
DISABLED: Not recording any writes (by choice.)
ACTIVE: Actively recording writes.

These are documented in the public API as possibilities for
DirtyBitmapStatus in block-core.json. We didn't add a new condition for
"readonly" either, which I think is actually required:

READONLY: Not recording any writes (by necessity.)


Your new use case here sounds like Frozen to me, but it simply does not
have an anonymous successor to force it to be recognized as "frozen." We
can add a `bool protected` or `bool frozen` field to force recognition
of this status and adjust the json documentation accordingly.


Bitmaps are selected for migration when source is running, so we should 
protect them (from deletion (or frozing or disabling), not from chaning 
bits) before source stop, so that is not like frozen. Bitmaps may be 
changed in this state.

It is more like ACTIVE.

We can move bitmap selection on the point after precopy migration, after 
source stop, but I'm not sure that it would be good.




I think then we'd have four recognized states:

FROZEN: Cannot be reset/deleted. Bitmap is in-use by a block job or
other internal process. Bitmap is otherwise ACTIVE.


? Frozen means that all writes goes to the successor and frozen bitmap 
itself is unchanged, no?



DISABLED: Not recording any writes (by choice.)
READONLY: Not able to record any writes (by necessity.)
ACTIVE: Normal bitmap status.

Sound right?



--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v8 4/4] net/socket: Improve -net socket error reporting

[Mao Zhongyi]

Hi, Markus

On 07/06/2017 03:24 PM, Markus Armbruster wrote:

Mao Zhongyi  writes:


When -net socket fails, it first reports a specific error, then
a generic one, like this:

$ qemu-system-x86_64 -net socket,
qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=, 
mcast= or udp= is required
qemu-system-x86_64: -net socket: Device 'socket' could not be initialized

Convert net_socket_*_init() to Error to get rid of the superfluous second
error message. After the patch, the effect like this:

$ qemu-system-x86_64 -net socket,
qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=, 
mcast= or udp= is required

At the same time, add many explicit error handling message when it fails.


Suggest "This also fixes a few silent failures to report an error."

Perhaps Jason can touch this up for you.


Cc: jasow...@redhat.com
Cc: arm...@redhat.com
Cc: berra...@redhat.com
Signed-off-by: Mao Zhongyi 
---
 net/socket.c | 92 +---
 1 file changed, 44 insertions(+), 48 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 1d4c515..ca1878f 100644
--- a/net/socket.c
+++ b/net/socket.c

[...]

@@ -746,7 +742,7 @@ int net_init_socket(const Netdev *netdev, const char *name,
 /* if sock->localaddr is missing, it has been initialized to "all bits
  * zero" */
 if (net_socket_mcast_init(peer, "socket", name, sock->mcast,
-sock->localaddr) == -1) {
+sock->localaddr, errp) < 0) {


Indent like this:

 sock->localaddr, errp) < 0) {

Perhaps Jason can touch this up for you.


 return -1;
 }
 return 0;
@@ -754,11 +750,11 @@ int net_init_socket(const Netdev *netdev, const char 
*name,

 assert(sock->has_udp);
 if (!sock->has_localaddr) {
-error_report("localaddr= is mandatory with udp=");
+error_setg(errp, "localaddr= is mandatory with udp=");
 return -1;
 }
-if (net_socket_udp_init(peer, "socket", name, sock->udp, sock->localaddr) 
==
--1) {
+if (net_socket_udp_init(peer, "socket", name, sock->udp, sock->localaddr,
+errp) < 0) {


Likewise.


 return -1;
 }
 return 0;


With the indentation fixed:


Thanks for your review :)
I will fix and resend it as soon as possible.

Thanks,
Mao


Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [RFC v3 2/3] qemu-error: Implement a more generic error reporting

[Daniel P. Berrange]

On Thu, Jul 06, 2017 at 08:15:54AM +0200, Markus Armbruster wrote:
> Alistair Francis  writes:
> 
> > This patch converts the existing error_vreport() function into a generic
> > qmesg_vreport() function that takes an enum describing the
> > information to be reported.
> >
> > As part of this change a new qmesg_report() function is added as well with 
> > the
> > same capability.
> >
> > To maintain full compatibility the original error_report() function is
> > maintained and no changes to the way errors are printed have been made.
> > To improve access to the new informaiton and warning options wrapper 
> > functions
> > similar to error_report() have been added for warnings and information
> > printing.
> >
> > Signed-off-by: Alistair Francis 

> > diff --git a/util/qemu-error.c b/util/qemu-error.c
> > index 1c5e35ecdb..63fdc0e174 100644
> > --- a/util/qemu-error.c
> > +++ b/util/qemu-error.c
> > @@ -179,17 +179,29 @@ static void print_loc(void)
> >  
> >  bool enable_timestamp_msg;
> >  /*
> > - * Print an error message to current monitor if we have one, else to 
> > stderr.
> > + * Print a message to current monitor if we have one, else to stderr.
> >   * Format arguments like vsprintf().  The resulting message should be
> >   * a single phrase, with no newline or trailing punctuation.
> >   * Prepend the current location and append a newline.
> >   * It's wrong to call this in a QMP monitor.  Use error_setg() there.
> >   */
> > -void error_vreport(const char *fmt, va_list ap)
> > +void qmsg_vreport(report_type type, const char *fmt, va_list ap)
> >  {
> >  GTimeVal tv;
> >  gchar *timestr;
> >  
> > +switch (type) {
> > +case REPORT_TYPE_ERROR:
> > +/* To maintain compatibility we don't add anything here */
> 
> I feel the comment isn't going to be useful in the future.  Let's drop
> it.

Do we really need to care about compatibility of the precise way we output
error messages. It has never been something we call a "stable API", as we
don't guarantee error message text will remain the same across releases. So
anyone relying on scraping QEMU stderr to match some error message has always
been liable to break.

IOW, just add an "error: " prefix to the text

> 
> > +break;
> > +case REPORT_TYPE_WARNING:
> > +error_printf("warning: ");
> > +break;
> > +case REPORT_TYPE_INFO:
> > +error_printf("info: ");
> > +break;
> 
>default:
>assert(0);
> 
> > +}
> > +
> >  if (enable_timestamp_msg && !cur_mon) {
> >  g_get_current_time(&tv);
> >  timestr = g_time_val_to_iso8601(&tv);

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2.1 3/4] doc: add item for "-M enforce-config-section"

[Greg Kurz]

On Thu,  6 Jul 2017 10:08:41 +0800
Peter Xu  wrote:

> It's never documented, and now we have one more parameter for it (which
> obsoletes this one). Document it properly.
> 
> Although now when enforce-config-section is set, it'll override the
> other "-global" parameter, that is not necessarily a rule. Forbid that
> usage in the document.
> 
> Suggested-by: Eduardo Habkost 
> Signed-off-by: Peter Xu 
> ---
> v2.1:
> - remove the "undefined behavior" sentence [Markus]
> 
>  qemu-options.hx | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 297bd8a..1ce7a37 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -85,6 +85,14 @@ Enables or disables NVDIMM support. The default is off.
>  @item s390-squash-mcss=on|off
>  Enables or disables squashing subchannels into the default css.
>  The default is off.
> +@item enforce-config-section=on|off
> +If @option{enforce-config-section} is set to @var{on}, force migration
> +code to send configuration section even if the machine-type sets the
> +@option{migration.send-configuration} property to @var{off}.
> +NOTE: this parameter is deprecated. Please use @option{-global}
> +@option{migration.send-configuration}=@var{on|off} instead.
> +@option{enforce-config-section} cannot be used together with
> +@option{-global} @option{migration.send-configuration}.

Is this last sentence correct ?

My understanding is that "enforce-config-section" overrides
"migration.send-configuration"...

>  @end table
>  ETEXI
>  



pgpL9bo3KivME.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 05/11] linux-user/sh4: Notice gUSA regions during signal delivery

[John Paul Adrian Glaubitz]

Hi!

Wow, great to see that there is actually now support for handling gUSA
in qemu-user on sh4. I wonder whether this will help resolve the
lock-up issues on qemu-sh4-user when building Debian packages for
sh4.

The lockups occur fairly often when any process uses
multi-threading. Would this help here?

Was gUSA support recently added to qemu or is this just a fix?

I will give this a try soon. Thanks Laurent for letting me know.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [Qemu-devel] The maximum limit of virtual network device

[Daniel P. Berrange]

On Thu, Jul 06, 2017 at 06:20:54AM +, Wu, Jiaxin wrote:
> Hello experts,
> 
> We know QEMU has the capability to create the multiple network devices in one 
> QEMU guest with the -device syntax. But I met the below failure when I'm 
> trying to create more than 30 virtual devices with the each TAP backend:
> 
> qemu-system-x86_64: -device e1000: PCI: no slot/function available for e1000, 
> all in use.
> 
> The corresponding QEMU command shows as following:

[snip]
 
> From above,  the max limit of virtual network device in one guest is about
> 29? If not, how can I avoid such failure? My use case is to create more than
> 150 network devices in one guest. Please provide your comments on this.

As the error message above shows, you have run out of PCI slots on the default
PCI bus. There are two ways to get around this. The first is to make use of
multifunction=on and specify individual PCI functions. There are 7 functions
per slot, so that would take you to 203 devices. The downside of multifunction
is that you can't hotplug individual functions.  The second approach is to
just add a bunch of PCI bridges, so that you have more slots available. 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 1/2] scripts: use build_ prefix for string not piped through cgen()

[Markus Armbruster]

Marc-André Lureau  writes:

> Suggested by Markus Armbruster:

Replacing the line above with ...

> The gen_ prefix is awkward.  Generated C should go through cgen()
> exactly once (see commit 1f9a7a1).  The common way to get this wrong is
> passing a foo=gen_foo() keyword argument to mcgen().  I'd like us to
> adopt a naming convention where gen_ means "something that's been piped
> through cgen(), and thus must not be passed to cgen() or mcgen()".
> Requires renaming gen_params(), gen_marshal_proto() and
> gen_event_send_proto().

Suggested-by: Markus Armbruster 

> Signed-off-by: Marc-André Lureau 

Applied, thanks!

Re: [Qemu-devel] [PATCH v1 0/3] Windows runtime improvements

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v1 0/3]  Windows runtime improvements
Type: series
Message-id: cover.1498756113.git.alistair.fran...@xilinx.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/1499242883-2184-1-git-send-email-pet...@redhat.com 
-> patchew/1499242883-2184-1-git-send-email-pet...@redhat.com
Switched to a new branch 'test'
fede661 util/oslib-win32: Remove if conditional
f464b14 util/oslib-win32: Remove invalid check
29fb20d util/aio-win32: Only select on what we are actually waiting for

=== OUTPUT BEGIN ===
Checking PATCH 1/3: util/aio-win32: Only select on what we are actually waiting 
for...
Checking PATCH 2/3: util/oslib-win32: Remove invalid check...
Checking PATCH 3/3: util/oslib-win32: Remove if conditional...
ERROR: spaces required around that '-' (ctx:VxV)
#30: FILE: util/oslib-win32.c:442:
+handles[i-1] = handles[i];
  ^

total: 1 errors, 0 warnings, 12 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] The maximum limit of virtual network device

[Laszlo Ersek]

Hi Jiaxin,

it's nice to see a question from you on qemu-devel! :)

On 07/06/17 08:20, Wu, Jiaxin wrote:
> Hello experts,
>
> We know QEMU has the capability to create the multiple network devices
> in one QEMU guest with the -device syntax. But I met the below failure
> when I'm trying to create more than 30 virtual devices with the each
> TAP backend:
>
> qemu-system-x86_64: -device e1000: PCI: no slot/function available for
> e1000, all in use.
>
> The corresponding QEMU command shows as following:
>
> sudo qemu-system-x86_64 \
>   -pflash OVMF.fd \
>   -global e1000.romfile="" \
>   -netdev tap,id=hostnet0,ifname=tap0,script=no,downscript=no \
>   -device e1000,netdev=hostnet0 \
>   -netdev tap,id=hostnet1,ifname=tap1,script=no,downscript=no \
>   -device e1000,netdev=hostnet1 \
>   -netdev tap,id=hostnet2,ifname=tap2,script=no,downscript=no \
>   -device e1000,netdev=hostnet2 \
>   -netdev tap,id=hostnet3,ifname=tap3,script=no,downscript=no \
>   -device e1000,netdev=hostnet3 \
>   -netdev tap,id=hostnet4,ifname=tap4,script=no,downscript=no \
>   -device e1000,netdev=hostnet4 \
>   -netdev tap,id=hostnet5,ifname=tap5,script=no,downscript=no \
>   -device e1000,netdev=hostnet5 \
>   -netdev tap,id=hostnet6,ifname=tap6,script=no,downscript=no \
>   -device e1000,netdev=hostnet6 \
>   -netdev tap,id=hostnet7,ifname=tap7,script=no,downscript=no \
>   -device e1000,netdev=hostnet7 \
>   -netdev tap,id=hostnet8,ifname=tap8,script=no,downscript=no \
>   -device e1000,netdev=hostnet8 \
>   -netdev tap,id=hostnet9,ifname=tap9,script=no,downscript=no \
>   -device e1000,netdev=hostnet9 \
>   -netdev tap,id=hostnet10,ifname=tap10,script=no,downscript=no \
>   -device e1000,netdev=hostnet10 \
>   -netdev tap,id=hostnet11,ifname=tap11,script=no,downscript=no \
>   -device e1000,netdev=hostnet11 \
>   -netdev tap,id=hostnet12,ifname=tap12,script=no,downscript=no \
>   -device e1000,netdev=hostnet12 \
>   -netdev tap,id=hostnet13,ifname=tap13,script=no,downscript=no \
>   -device e1000,netdev=hostnet13 \
>   -netdev tap,id=hostnet14,ifname=tap14,script=no,downscript=no \
>   -device e1000,netdev=hostnet14 \
>   -netdev tap,id=hostnet15,ifname=tap15,script=no,downscript=no \
>   -device e1000,netdev=hostnet15 \
>   -netdev tap,id=hostnet16,ifname=tap16,script=no,downscript=no \
>   -device e1000,netdev=hostnet16 \
>   -netdev tap,id=hostnet17,ifname=tap17,script=no,downscript=no \
>   -device e1000,netdev=hostnet17 \
>   -netdev tap,id=hostnet18,ifname=tap18,script=no,downscript=no \
>   -device e1000,netdev=hostnet18 \
>   -netdev tap,id=hostnet19,ifname=tap19,script=no,downscript=no \
>   -device e1000,netdev=hostnet19 \
>   -netdev tap,id=hostnet20,ifname=tap20,script=no,downscript=no \
>   -device e1000,netdev=hostnet20 \
>   -netdev tap,id=hostnet21,ifname=tap21,script=no,downscript=no \
>   -device e1000,netdev=hostnet21 \
>   -netdev tap,id=hostnet22,ifname=tap22,script=no,downscript=no \
>   -device e1000,netdev=hostnet22 \
>   -netdev tap,id=hostnet23,ifname=tap23,script=no,downscript=no \
>   -device e1000,netdev=hostnet23 \
>   -netdev tap,id=hostnet24,ifname=tap24,script=no,downscript=no \
>   -device e1000,netdev=hostnet24 \
>   -netdev tap,id=hostnet25,ifname=tap25,script=no,downscript=no \
>   -device e1000,netdev=hostnet25 \
>   -netdev tap,id=hostnet26,ifname=tap26,script=no,downscript=no \
>   -device e1000,netdev=hostnet26 \
>   -netdev tap,id=hostnet27,ifname=tap27,script=no,downscript=no \
>   -device e1000,netdev=hostnet27 \
>   -netdev tap,id=hostnet28,ifname=tap28,script=no,downscript=no \
>   -device e1000,netdev=hostnet28 \
>   -netdev tap,id=hostnet29,ifname=tap29,script=no,downscript=no \
>   -device e1000,netdev=hostnet29
>
> From above,  the max limit of virtual network device in one guest is
> about 29? If not, how can I avoid such failure? My use case is to
> create more than 150 network devices in one guest. Please provide your
> comments on this.

You are seeing the above symptom because the above command line
instructs QEMU to do the following:
- use the i440fx machine type,
- use a single PCI bus (= the main root bridge),
- add the e1000 cards to separate slots (always using function 0) on
  that bus.

Accordingly, there are three things you can do to remedy this:

- Use the Q35 machine type and work with a PCI Express hierarchy rather
  than a PCI hierarchy. I'm mentioning this only for completeness,
  because it won't directly help your use case. But, I certainly want to
  highlight "docs/pcie.txt". Please read it sometime; it has nice
  examples and makes good points.

- Use multiple PCI bridges to attach the devices. For this, several ways
  are possible:

  - use multiple root buses, with the pxb or pxb-pcie devices (see
"docs/pci_expander_bridge.txt" and "docs/pcie.txt")

  - use multiple normal PCI bridges

  - use multiple PCI Express root ports or downstream ports (but for
this, you'll likely have to use the PCI Express variant of the
e1000, namely e1000e)

- I

Re: [Qemu-devel] [PATCH] backends: remove empty trace-events file

[Stefan Hajnoczi]

On Thu, Jun 29, 2017 at 05:20:46PM +0100, Daniel P. Berrange wrote:
> The content of the backends/trace-events file was entirely
> removed in
> 
>   commit 6b10e573d15ef82dbc5c5b3726028e6642e134f6
>   Author: Marc-André Lureau 
>   Date:   Mon May 29 12:39:42 2017 +0400
> 
> char: move char devices to chardev/
> 
> Leaving the empty file around, causes tracetool to generate
> an empty .dtrace file which makes the dtrace compiler throw
> a syntax error.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  Makefile.objs | 1 -
>  backends/trace-events | 0
>  2 files changed, 1 deletion(-)
>  delete mode 100644 backends/trace-events

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 05/11] linux-user/sh4: Notice gUSA regions during signal delivery

[Laurent Vivier]

Le 06/07/2017 à 10:10, John Paul Adrian Glaubitz a écrit :
> Hi!
> 
> Wow, great to see that there is actually now support for handling gUSA
> in qemu-user on sh4. I wonder whether this will help resolve the
> lock-up issues on qemu-sh4-user when building Debian packages for
> sh4.
> 
> The lockups occur fairly often when any process uses
> multi-threading. Would this help here?
> 
> Was gUSA support recently added to qemu or is this just a fix?

This is a patch series proposed by Richard, it is not included in qemu
at the moment:

http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg01196.html

I think it would be interesting if you generate a qemu binary with it
and use it in your buildds to test it.

I guess the series can be pulled directly from Richard's branch:

git://github.com/rth7680/qemu.git tgt-sh4

Thanks,
Laurent


0x3F2FBE3C.asc
Description: application/pgp-keys

Re: [Qemu-devel] qemu_system_reset_request() broken w.r.t BQL locking regime

[Alex Bennée]

Paolo Bonzini  writes:

> On 05/07/2017 18:14, Peter Maydell wrote:
>>>   - Guest resets board, writing to some hw address (e.g.
>>> arm_sysctl_write)
>>>   - This triggers qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET)
>>>   - We exit iowrite and drop the BQL
>>>   - vl.c schedules qemu_system_reset->qemu_devices_reset...arm_cpu_reset
>>>   - we start writing new values to CPU env while still in TCG code
>>>   - CHAOS!
>>>
>>> The general solution for this is to ensure these sort of tasks are done
>>> with safe work in the CPUs context when we know nothing else is running.
>>> It seems this is probably best done by modifying
>>> qemu_system_reset_request to queue work up on current_cpu and execute it
>>> as safe work - I don't think the vl.c thread should ever be messing
>>> about with calling cpu_reset directly.
>> My first thought is that qemu_system_reset() should absolutely
>> stop every CPU (or other runnable thing like a DMA agent) in the
>> system. The semantics are basically "like a power cycle", so
>> that should include a complete stop of the world. (Is this
>> what vm_stop() does? Dunno...)
>
> I agree, it should do vm_stop() as the first thing and, if applicable,
> vm_start() as the last thing, similar to e.g. savevm.

OK I did some more digging and basically the problem is cpu_stop_current
does the wrong thing. It can set cpu->stopped while still in the vCPU
thread which means when the vl.c thread does pause_all_vcpus() it thinks
the thread is paused when in fact it isn't leading to the chaos. I think
the fix is to tighten up our usage of these two functions. So my current
plan is:

* pause_all_vcpus() should never be called from vCPU/HW emulation

One case in kvm_apic has been fixed by Pranith. The other case in s390
should be converted to use async_safe_work. Once this is done we can
assert that pause_all_vcpus() is not in a vCPU thread and keep it for
qmp,hmp and gdb type operations.

* vm_stop() is probably being misused by vCPU threads

There are more uses than pause_all_vcpus here but they all seem to be
for error handling bail-out type things.

* cpu_stop_current() is probably superfluous now

It certainly shouldn't be called directly from the vCPU code
(rtas_power_off) and once we know pause_all_vcpus() can't be called
directly at least one call is gone. I think the current_cpu handling is
a relic of the days of single-threaded handling when it was a global.

Does that sound reasonable?

--
Alex Bennée

Re: [Qemu-devel] [PATCH v2.1 3/4] doc: add item for "-M enforce-config-section"

[Peter Xu]

On Thu, Jul 06, 2017 at 10:09:19AM +0200, Greg Kurz wrote:
> On Thu,  6 Jul 2017 10:08:41 +0800
> Peter Xu  wrote:
> 
> > It's never documented, and now we have one more parameter for it (which
> > obsoletes this one). Document it properly.
> > 
> > Although now when enforce-config-section is set, it'll override the
> > other "-global" parameter, that is not necessarily a rule. Forbid that
> > usage in the document.
> > 
> > Suggested-by: Eduardo Habkost 
> > Signed-off-by: Peter Xu 
> > ---
> > v2.1:
> > - remove the "undefined behavior" sentence [Markus]
> > 
> >  qemu-options.hx | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 297bd8a..1ce7a37 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -85,6 +85,14 @@ Enables or disables NVDIMM support. The default is off.
> >  @item s390-squash-mcss=on|off
> >  Enables or disables squashing subchannels into the default css.
> >  The default is off.
> > +@item enforce-config-section=on|off
> > +If @option{enforce-config-section} is set to @var{on}, force migration
> > +code to send configuration section even if the machine-type sets the
> > +@option{migration.send-configuration} property to @var{off}.
> > +NOTE: this parameter is deprecated. Please use @option{-global}
> > +@option{migration.send-configuration}=@var{on|off} instead.
> > +@option{enforce-config-section} cannot be used together with
> > +@option{-global} @option{migration.send-configuration}.
> 
> Is this last sentence correct ?
> 
> My understanding is that "enforce-config-section" overrides
> "migration.send-configuration"...

Yes, in the codes it is.

But as mentioned in the commit message, this rule is actually not
really necessary, so it does not really conflict when we declare in
the document that it is not allowed to happen (the code just did
something extra). After all, we want to finally remove
enforce-config-section one day, so before that I would like to avoid
taking more responsibility on this old parameter.

Also, I hope people (especially who read this document) don't use
these two parameters at the same time, or better, use the new one,
rather than still using "enforce-config-section" after he knows that
"anyway, it'll override the global parameter, so this old one is still
better"...

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [Qemu-block] [PATCH v2 1/3] block: pass bdrv_* methods to bs->file by default

[Stefan Hajnoczi]

On Mon, Jul 03, 2017 at 01:40:57PM -0500, Eric Blake wrote:
> On 07/03/2017 10:56 AM, Eric Blake wrote:
> > On 06/29/2017 01:43 PM, Manos Pitsidianakis wrote:
> >> The following functions fail if bs->drv does not implement them:
> 
> One other suggestion: most patches in the tree use 'topic: Capital',
> while yours used 'topic: lowercase'.  It's probably worth posting a v3
> that addresses all of the nits I've pointed out, including tweaking the
> commit messages.

Plenty of regular contributors use "topic: lowercase" including Michael
Tsirkin, Mark Cave-Ayland, Peter Xu, and myself (just from looking at
the first page of git log --oneline on qemu.git/master).

If we want to enforce a consistent style in the future then it should be
discussed separately.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 2/2] qapi: add location comment for generated types

[Markus Armbruster]

Marc-André Lureau  writes:

> This may help to find where the origin of the type was declared in the
> json (when greping isn't easy enough).
>
> Generates the following kind of C comment before types:
>
>   /* /home/elmarco/src/qemu/qapi/introspect.json:94 */
>   typedef struct SchemaInfo SchemaInfo;

Can we do relative file names?  I.e. just qapi/introspect.json.

Would such comments be useful for things other than typedefs?

> Signed-off-by: Marc-André Lureau 
> ---
>  scripts/qapi.py   | 12 +---
>  scripts/qapi-event.py |  2 +-
>  scripts/qapi-types.py | 18 +-
>  3 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index d37a6157e0..7f9935eec0 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1852,15 +1852,21 @@ const char *const %(c_name)s_lookup[] = {
>  return ret
>  
>  
> -def gen_enum(name, values, prefix=None):
> +def build_src_loc_comment(info):
> +if info:
> +return '\n/* %s:%d */' % (info['file'], info['line'])

Leading instead of trailing newline, hmm.  Let's see how it works out.

> +else:
> +return ''

Let's drop the else: line.

> +
> +def gen_enum(info, name, values, prefix=None):

Make that name, info, values, ... for consistency with other functions
taking both name and info.  More of the same below.

>  # append automatically generated _MAX value
>  enum_values = values + ['_MAX']
>  
>  ret = mcgen('''
> -
> +%(comment)s

Loses the blank line when not info.

>  typedef enum %(c_name)s {
>  ''',
> -c_name=c_name(name))
> +c_name=c_name(name), comment=build_src_loc_comment(info))
>  
>  i = 0
>  for value in enum_values:
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> index bcbef1035f..cf5e282011 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -159,7 +159,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaVisitor):
>  self._event_names = []
>  
>  def visit_end(self):
> -self.decl += gen_enum(event_enum_name, self._event_names)
> +self.decl += gen_enum(None, event_enum_name, self._event_names)
>  self.defn += gen_enum_lookup(event_enum_name, self._event_names)
>  self._event_names = None
>  
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index b45e7b5634..9c8a3e277b 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -19,12 +19,12 @@ from qapi import *
>  objects_seen = set()
>  
>  
> -def gen_fwd_object_or_array(name):
> +def gen_fwd_object_or_array(info, name):
>  return mcgen('''
> -
> +%(comment)s

Likewise.

I think we should drop the newline from build_src_loc_comment()'s value,
and keep the blank line before its two uses.

>  typedef struct %(c_name)s %(c_name)s;
>  ''',
> - c_name=c_name(name))
> + c_name=c_name(name), comment=build_src_loc_comment(info))
>  
>  
>  def gen_array(name, element_type):
> @@ -199,22 +199,22 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>  # Special case for our lone builtin enum type
>  # TODO use something cleaner than existence of info
>  if not info:
> -self._btin += gen_enum(name, values, prefix)
> +self._btin += gen_enum(None, name, values, prefix)
>  if do_builtins:
>  self.defn += gen_enum_lookup(name, values, prefix)
>  else:
> -self._fwdecl += gen_enum(name, values, prefix)
> +self._fwdecl += gen_enum(info, name, values, prefix)
>  self.defn += gen_enum_lookup(name, values, prefix)
>  
>  def visit_array_type(self, name, info, element_type):
>  if isinstance(element_type, QAPISchemaBuiltinType):
> -self._btin += gen_fwd_object_or_array(name)
> +self._btin += gen_fwd_object_or_array(None, name)
>  self._btin += gen_array(name, element_type)
>  self._btin += gen_type_cleanup_decl(name)
>  if do_builtins:
>  self.defn += gen_type_cleanup(name)
>  else:
> -self._fwdecl += gen_fwd_object_or_array(name)
> +self._fwdecl += gen_fwd_object_or_array(info, name)
>  self.decl += gen_array(name, element_type)
>  self._gen_type_cleanup(name)
>  
> @@ -222,7 +222,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>  # Nothing to do for the special empty builtin
>  if name == 'q_empty':
>  return
> -self._fwdecl += gen_fwd_object_or_array(name)
> +self._fwdecl += gen_fwd_object_or_array(info, name)
>  self.decl += gen_object(name, base, members, variants)
>  if base and not base.is_implicit():
>  self.decl += gen_upcast(name, base)
> @@ -233,7 +233,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>  self._gen_type_cleanup(name)
>  
>  def visit_alternate_type(self, name, in

Re: [Qemu-devel] [PATCH v2 6/6] nbd: use generic trace subsystem instead of TRACE macro

[Vladimir Sementsov-Ogievskiy]

05.07.2017 18:29, Paolo Bonzini wrote:


On 21/06/2017 17:34, Vladimir Sementsov-Ogievskiy wrote:

Starting from this patch to enable traces use -trace option of qemu or
-T, --trace option of qemu-img, qemu-io and qemu-nbd. For qemu traces
also can be managed by qmp commands trace-event-{get,set}-state.

Recompilation with CFLAGS=-DDEBUG_NBD is no more needed, furthermore,
DEBUG_NBD macro is removed from the code.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  Makefile.objs  |  1 +
  nbd/client.c   | 74 +---
  nbd/nbd-internal.h | 19 -
  nbd/server.c   | 83 ++
  nbd/trace-events   | 68 
  5 files changed, 141 insertions(+), 104 deletions(-)
  create mode 100644 nbd/trace-events

diff --git a/Makefile.objs b/Makefile.objs
index b2e6322ef0..a66ea34cc4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -168,6 +168,7 @@ trace-events-subdirs += linux-user
  trace-events-subdirs += qapi
  trace-events-subdirs += accel/tcg
  trace-events-subdirs += accel/kvm
+trace-events-subdirs += nbd
  
  trace-events-files = $(SRC_PATH)/trace-events $(trace-events-subdirs:%=$(SRC_PATH)/%/trace-events)
  
diff --git a/nbd/client.c b/nbd/client.c

index 5a4825ebe0..75b28f4ccf 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -19,6 +19,7 @@
  
  #include "qemu/osdep.h"

  #include "qapi/error.h"
+#include "trace.h"
  #include "nbd-internal.h"
  
  static int nbd_errno_to_system_errno(int err)

@@ -44,7 +45,7 @@ static int nbd_errno_to_system_errno(int err)
  ret = ESHUTDOWN;
  break;
  default:
-TRACE("Squashing unexpected error %d to EINVAL", err);
+trace_nbd_unknown_error(err);
  /* fallthrough */
  case NBD_EINVAL:
  ret = EINVAL;
@@ -103,7 +104,7 @@ static int nbd_send_option_request(QIOChannel *ioc, 
uint32_t opt,
  if (len == -1) {
  req.length = len = strlen(data);
  }
-TRACE("Sending option request %" PRIu32", len %" PRIu32, opt, len);
+trace_nbd_send_option_request(opt, len);
  
  stq_be_p(&req.magic, NBD_OPTS_MAGIC);

  stl_be_p(&req.option, opt);
@@ -153,8 +154,7 @@ static int nbd_receive_option_reply(QIOChannel *ioc, 
uint32_t opt,
  be32_to_cpus(&reply->type);
  be32_to_cpus(&reply->length);
  
-TRACE("Received option reply %" PRIx32", type %" PRIx32", len %" PRIu32,

-  reply->option, reply->type, reply->length);
+trace_nbd_receive_option_reply(reply->option, reply->type, reply->length);
  
  if (reply->magic != NBD_REP_MAGIC) {

  error_setg(errp, "Unexpected option reply magic");
@@ -201,8 +201,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, 
nbd_opt_reply *reply,
  
  switch (reply->type) {

  case NBD_REP_ERR_UNSUP:
-TRACE("server doesn't understand request %" PRIx32
-  ", attempting fallback", reply->option);
+trace_nbd_reply_err_unsup(reply->option);
  result = 0;
  goto cleanup;
  
@@ -342,12 +341,12 @@ static int nbd_receive_query_exports(QIOChannel *ioc,

  {
  bool foundExport = false;
  
-TRACE("Querying export list for '%s'", wantname);

+trace_nbd_receive_query_exports_start(wantname);
  if (nbd_send_option_request(ioc, NBD_OPT_LIST, 0, NULL, errp) < 0) {
  return -1;
  }
  
-TRACE("Reading available export names");

+trace_nbd_receive_query_exports_loop();
  while (1) {
  int ret = nbd_receive_list(ioc, wantname, &foundExport, errp);
  
@@ -362,7 +361,7 @@ static int nbd_receive_query_exports(QIOChannel *ioc,

  nbd_send_opt_abort(ioc);
  return -1;
  }
-TRACE("Found desired export name '%s'", wantname);
+trace_nbd_receive_query_exports_success(wantname);
  return 0;
  }
  }
@@ -376,12 +375,12 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
  QIOChannelTLS *tioc;
  struct NBDTLSHandshakeData data = { 0 };
  
-TRACE("Requesting TLS from server");

+trace_nbd_receive_starttls_request();
  if (nbd_send_option_request(ioc, NBD_OPT_STARTTLS, 0, NULL, errp) < 0) {
  return NULL;
  }
  
-TRACE("Getting TLS reply from server");

+trace_nbd_receive_starttls_reply();
  if (nbd_receive_option_reply(ioc, NBD_OPT_STARTTLS, &reply, errp) < 0) {
  return NULL;
  }
@@ -400,14 +399,14 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
  return NULL;
  }
  
-TRACE("TLS request approved, setting up TLS");

+trace_nbd_receive_starttls_new_client();
  tioc = qio_channel_tls_new_client(ioc, tlscreds, hostname, errp);
  if (!tioc) {
  return NULL;
  }
  qio_channel_set_name(QIO_CHANNEL(tioc), "nbd-client-tls");
  data.loop = g_main_loop_new(g_main_context_default(), FALSE);
-TRACE("Starting TLS handshake");
+trace

Re: [Qemu-devel] [PATCH 1/3] include/hw/boards.h: Document memory_region_allocate_system_memory()

[Paolo Bonzini]

On 06/07/2017 05:18, Philippe Mathieu-Daudé wrote:
> Hi Peter, Paolo,
> 
> On 07/04/2017 02:02 PM, Peter Maydell wrote:
>> Add a documentation comment for memory_region_allocate_system_memory().
>>
>> In particular, the reason for this function's existence and the
>> requirement on board code to call it exactly once are non-obvious.
>>
>> Signed-off-by: Peter Maydell 
>> ---
>>   include/hw/boards.h | 28 
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 76ce021..1bc5389 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -9,6 +9,34 @@
>>   #include "qom/object.h"
>>   #include "qom/cpu.h"
>>   +/**
>> + * memory_region_allocate_system_memory - Allocate a board's main memory
>> + * @mr: the #MemoryRegion to be initialized
>> + * @owner: the object that tracks the region's reference count
>> + * @name: name of the memory region
>> + * @ram_size: size of the region in bytes
>> + *
>> + * This function allocates the main memory for a board model, and
>> + * initializes @mr appropriately. It also arranges for the memory
>> + * to be migrated (by calling vmstate_register_ram_global()).
>> + *
>> + * Memory allocated via this function will be backed with the memory
>> + * backend the user provided using -mem-path if appropriate; this
>> + * is typically used to cause host huge pages to be used.
>> + * This function should therefore be called by a board exactly once,
> 
> Using memory-backend-file objects one can use different mem-path.
> 
> Maybe removing the global mem_path used by vl.c for "main memory" (which
> is a memory-backend-file without naming it) this "exactly once" case can
> be avoided.

It's already the case that you can use different mem-paths and different
host node bindings, though you still have to associate one memory
backend to each "-numa node" option (via "-numa node,memdev=...").

The function has to be called only once because it consumes all the "-m"
and "-numa node,memdev=..." options.

Paolo

>> + * for the primary or largest RAM area it implements.
>> + *
>> + * For boards where the major RAM is split into two parts in the memory
>> + * map, you can deal with this by calling
>> memory_region_allocate_system_memory()
>> + * once to get a MemoryRegion with enough RAM for both parts, and then
>> + * creating alias MemoryRegions via memory_region_init_alias() which
>> + * alias into different parts of the RAM MemoryRegion and can be mapped
>> + * into the memory map in the appropriate places.
>> + *
>> + * Smaller pieces of memory (display RAM, static RAMs, etc) don't need
>> + * to be backed via the -mem-path memory backend and can simply
>> + * be created via memory_region_init_ram().
>> + */
>>   void memory_region_allocate_system_memory(MemoryRegion *mr, Object
>> *owner,
>> const char *name,
>> uint64_t ram_size);
>>

[Qemu-devel] [PATCH v8 0/4] Improve error reporting

[Mao Zhongyi]

v8:
* PATCH 02 & 04
  -resetting the error message for the user to read.   [Markus Armbruster]
  -fix the indentation and commit message.  [Markus Armbruster]
  
v7:
* PATCH 01
  -fix the error message.[Daniel P. Berrange]
  -adjust the indentation problem.[Eric Blake]
* PATCH 03
  -print a generic message when gethostbyname() failed in parse_host_port(),
   drop the misleading ": unkonwn host" part.[Markus Armbruster]

v6:
* PATCH 02
  -rename the subject
  -drop the "qemu: error: " prefix.
  -correct inappropriate error information settings.
* PATCH 03,04
  -correct inappropriate error information settings.[Markus Armbruster]

v5:
* PATCH 01 make the commit message more exact about the actual function.
[Markus Armbruster]
* PATCH 02, 03, 04 still retains the original function, but specific
   content and order of each patch has been adjusted substantially, 
   so that ensure each patch is a completed fix.[Markus Armbruster]

v4: 
* PATCH 01 is redoing previous patch 1, replace the fprintf() with 
error_report()
 in the 'default' case of net_socket_fd_init() [Markus 
Armbruster]

v3:
* PATCH 01 is suggested by Markus and Daniel that removes the dubious 'default' 
case
   in the net_socket_fd_init(). Jason agreed.
* PATCH 02 is redoing previous patch 4.
* PATCH 04 is redoing previous patch 2, improves sort of error messages. 

v2:
* PATCH 02 reworking of patch 2 following Markus's suggestion that convert 
error_report()
   in the function called by net_socket_*_init() to Error. Also add 
many error 
   handling information.
* PATCH 03 net_socket_mcast_create(), net_socket_fd_init_dgram() and 
net_socket_fd_init() 
   use the function such as fprintf, perror to report an error message. 
Convert it 
   to Error.
* PATCH 04 parse_host_port() may fail without reporting an error. Now, fix it 
to set an
   error when it fails.

Cc: jasow...@redhat.com
Cc: arm...@redhat.com
Cc: berra...@redhat.com
Cc: kra...@redhat.com
Cc: pbonz...@redhat.com
Cc: ebl...@redhat.com

Mao Zhongyi (4):
  net/socket: Don't treat odd socket type as SOCK_STREAM
  net/socket: Convert several helper functions to Error
  net/net: Convert parse_host_port() to Error
  net/socket: Improve -net socket error reporting

 include/qemu/sockets.h |   3 +-
 net/net.c  |  22 +--
 net/socket.c   | 153 -
 3 files changed, 106 insertions(+), 72 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH v8 3/4] net/net: Convert parse_host_port() to Error

[Mao Zhongyi]

Cc: berra...@redhat.com
Cc: kra...@redhat.com
Cc: pbonz...@redhat.com
Cc: jasow...@redhat.com
Cc: arm...@redhat.com
Cc: ebl...@redhat.com
Signed-off-by: Mao Zhongyi 
Reviewed-by: Markus Armbruster 
---
 include/qemu/sockets.h |  3 ++-
 net/net.c  | 22 +-
 net/socket.c   | 19 ++-
 3 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 5c326db..78e2b30 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -53,7 +53,8 @@ void socket_listen_cleanup(int fd, Error **errp);
 int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp);
 
 /* Old, ipv4 only bits.  Don't use for new code. */
-int parse_host_port(struct sockaddr_in *saddr, const char *str);
+int parse_host_port(struct sockaddr_in *saddr, const char *str,
+Error **errp);
 int socket_init(void);
 
 /**
diff --git a/net/net.c b/net/net.c
index 6235aab..77b6deb 100644
--- a/net/net.c
+++ b/net/net.c
@@ -100,7 +100,8 @@ static int get_str_sep(char *buf, int buf_size, const char 
**pp, int sep)
 return 0;
 }
 
-int parse_host_port(struct sockaddr_in *saddr, const char *str)
+int parse_host_port(struct sockaddr_in *saddr, const char *str,
+Error **errp)
 {
 char buf[512];
 struct hostent *he;
@@ -108,24 +109,35 @@ int parse_host_port(struct sockaddr_in *saddr, const char 
*str)
 int port;
 
 p = str;
-if (get_str_sep(buf, sizeof(buf), &p, ':') < 0)
+if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+error_setg(errp, "host address '%s' doesn't contain ':' "
+   "separating host from port", str);
 return -1;
+}
 saddr->sin_family = AF_INET;
 if (buf[0] == '\0') {
 saddr->sin_addr.s_addr = 0;
 } else {
 if (qemu_isdigit(buf[0])) {
-if (!inet_aton(buf, &saddr->sin_addr))
+if (!inet_aton(buf, &saddr->sin_addr)) {
+error_setg(errp, "host address '%s' is not a valid "
+   "IPv4 address", buf);
 return -1;
+}
 } else {
-if ((he = gethostbyname(buf)) == NULL)
+he = gethostbyname(buf);
+if (he == NULL) {
+error_setg(errp, "can't resolve host address '%s'", buf);
 return - 1;
+}
 saddr->sin_addr = *(struct in_addr *)he->h_addr;
 }
 }
 port = strtol(p, (char **)&r, 0);
-if (r == p)
+if (r == p) {
+error_setg(errp, "port number '%s' is invalid", p);
 return -1;
+}
 saddr->sin_port = htons(port);
 return 0;
 }
diff --git a/net/socket.c b/net/socket.c
index 3dd4459..1d4c515 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -497,9 +497,12 @@ static int net_socket_listen_init(NetClientState *peer,
 NetSocketState *s;
 struct sockaddr_in saddr;
 int fd, ret;
+Error *err = NULL;
 
-if (parse_host_port(&saddr, host_str) < 0)
+if (parse_host_port(&saddr, host_str, &err) < 0) {
+error_report_err(err);
 return -1;
+}
 
 fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
 if (fd < 0) {
@@ -544,8 +547,10 @@ static int net_socket_connect_init(NetClientState *peer,
 struct sockaddr_in saddr;
 Error *err = NULL;
 
-if (parse_host_port(&saddr, host_str) < 0)
+if (parse_host_port(&saddr, host_str, &err) < 0) {
+error_report_err(err);
 return -1;
+}
 
 fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
 if (fd < 0) {
@@ -597,8 +602,10 @@ static int net_socket_mcast_init(NetClientState *peer,
 struct in_addr localaddr, *param_localaddr;
 Error *err = NULL;
 
-if (parse_host_port(&saddr, host_str) < 0)
+if (parse_host_port(&saddr, host_str, &err) < 0) {
+error_report_err(err);
 return -1;
+}
 
 if (localaddr_str != NULL) {
 if (inet_aton(localaddr_str, &localaddr) == 0)
@@ -640,11 +647,13 @@ static int net_socket_udp_init(NetClientState *peer,
 struct sockaddr_in laddr, raddr;
 Error *err = NULL;
 
-if (parse_host_port(&laddr, lhost) < 0) {
+if (parse_host_port(&laddr, lhost, &err) < 0) {
+error_report_err(err);
 return -1;
 }
 
-if (parse_host_port(&raddr, rhost) < 0) {
+if (parse_host_port(&raddr, rhost, &err) < 0) {
+error_report_err(err);
 return -1;
 }
 
-- 
2.9.4

[Qemu-devel] [PATCH v8 1/4] net/socket: Don't treat odd socket type as SOCK_STREAM

[Mao Zhongyi]

In net_socket_fd_init(), the 'default' case is odd: it warns,
then continues as if the socket type was SOCK_STREAM. The
comment explains "this could be a eg. a pty", but that makes
no sense. If @fd really was a pty, getsockopt() would fail
with ENOTSOCK. If @fd was a socket, but neither SOCK_DGRAM nor
SOCK_STREAM. It should not be treated as if it was SOCK_STREAM.

Turn this case into an Error. If there is a genuine reason to
support something like SOCK_RAW, it should be explicitly
handled.

Cc: jasow...@redhat.com
Cc: arm...@redhat.com
Cc: berra...@redhat.com
Cc: arm...@redhat.com
Cc: ebl...@redhat.com
Suggested-by: Markus Armbruster 
Suggested-by: Daniel P. Berrange 
Signed-off-by: Mao Zhongyi 
Reviewed-by: Markus Armbruster 
---
 net/socket.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index dcae1ae..7d05e70 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -449,9 +449,9 @@ static NetSocketState *net_socket_fd_init(NetClientState 
*peer,
 case SOCK_STREAM:
 return net_socket_fd_init_stream(peer, model, name, fd, is_connected);
 default:
-/* who knows ... this could be a eg. a pty, do warn and continue as 
stream */
-fprintf(stderr, "qemu: warning: socket type=%d for fd=%d is not 
SOCK_DGRAM or SOCK_STREAM\n", so_type, fd);
-return net_socket_fd_init_stream(peer, model, name, fd, is_connected);
+error_report("socket type=%d for fd=%d must be either"
+ " SOCK_DGRAM or SOCK_STREAM", so_type, fd);
+closesocket(fd);
 }
 return NULL;
 }
-- 
2.9.4

[Qemu-devel] [PATCH v8 2/4] net/socket: Convert several helper functions to Error

[Mao Zhongyi]

Currently, net_socket_mcast_create(), net_socket_fd_init_dgram() and
net_socket_fd_init() use the function such as fprintf(), perror() to
report an error message.

Now, convert these functions to Error.

Cc: jasow...@redhat.com
Cc: arm...@redhat.com
Cc: berra...@redhat.com
Signed-off-by: Mao Zhongyi 
Reviewed-by: Daniel P. Berrange 
Reviewed-by: Markus Armbruster 
---
 net/socket.c | 80 
 1 file changed, 48 insertions(+), 32 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 7d05e70..3dd4459 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -209,7 +209,9 @@ static void net_socket_send_dgram(void *opaque)
 }
 }
 
-static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct 
in_addr *localaddr)
+static int net_socket_mcast_create(struct sockaddr_in *mcastaddr,
+   struct in_addr *localaddr,
+   Error **errp)
 {
 struct ip_mreq imr;
 int fd;
@@ -221,16 +223,16 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr, struct in_addr
 #endif
 
 if (!IN_MULTICAST(ntohl(mcastaddr->sin_addr.s_addr))) {
-fprintf(stderr, "qemu: error: specified mcastaddr \"%s\" (0x%08x) "
-"does not contain a multicast address\n",
-inet_ntoa(mcastaddr->sin_addr),
-(int)ntohl(mcastaddr->sin_addr.s_addr));
+error_setg(errp, "specified mcastaddr %s (0x%08x) "
+   "does not contain a multicast address",
+   inet_ntoa(mcastaddr->sin_addr),
+   (int)ntohl(mcastaddr->sin_addr.s_addr));
 return -1;
-
 }
+
 fd = qemu_socket(PF_INET, SOCK_DGRAM, 0);
 if (fd < 0) {
-perror("socket(PF_INET, SOCK_DGRAM)");
+error_setg_errno(errp, errno, "can't create datagram socket");
 return -1;
 }
 
@@ -242,13 +244,15 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr, struct in_addr
 val = 1;
 ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 if (ret < 0) {
-perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)");
+error_setg_errno(errp, errno,
+ "can't set socket option SO_REUSEADDR");
 goto fail;
 }
 
 ret = bind(fd, (struct sockaddr *)mcastaddr, sizeof(*mcastaddr));
 if (ret < 0) {
-perror("bind");
+error_setg_errno(errp, errno, "can't bind ip=%s to socket",
+ inet_ntoa(mcastaddr->sin_addr));
 goto fail;
 }
 
@@ -263,7 +267,9 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr, struct in_addr
 ret = qemu_setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
   &imr, sizeof(struct ip_mreq));
 if (ret < 0) {
-perror("setsockopt(IP_ADD_MEMBERSHIP)");
+error_setg_errno(errp, errno,
+ "can't add socket to multicast group %s",
+ inet_ntoa(imr.imr_multiaddr));
 goto fail;
 }
 
@@ -272,7 +278,8 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr, struct in_addr
 ret = qemu_setsockopt(fd, IPPROTO_IP, IP_MULTICAST_LOOP,
   &loop, sizeof(loop));
 if (ret < 0) {
-perror("setsockopt(SOL_IP, IP_MULTICAST_LOOP)");
+error_setg_errno(errp, errno,
+ "can't force multicast message to loopback");
 goto fail;
 }
 
@@ -281,7 +288,8 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr, struct in_addr
 ret = qemu_setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF,
   localaddr, sizeof(*localaddr));
 if (ret < 0) {
-perror("setsockopt(IP_MULTICAST_IF)");
+error_setg_errno(errp, errno,
+ "can't set the default network send interface");
 goto fail;
 }
 }
@@ -320,7 +328,8 @@ static NetClientInfo net_dgram_socket_info = {
 static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
 const char *model,
 const char *name,
-int fd, int is_connected)
+int fd, int is_connected,
+Error **errp)
 {
 struct sockaddr_in saddr;
 int newfd;
@@ -337,14 +346,12 @@ static NetSocketState 
*net_socket_fd_init_dgram(NetClientState *peer,
 if (getsockname(fd, (struct sockaddr *) &saddr, &saddr_len) == 0) {
 /* must be bound */
 if (saddr.sin_addr.s_addr == 0) {
-fprintf(stderr, "qemu: error: init_dgram: fd=%d unbound, "
-"cannot setup multicast dst addr\n", fd);
+error_setg(errp, "can't setup multicast destination address");
 go

[Qemu-devel] [PATCH v8 4/4] net/socket: Improve -net socket error reporting

[Mao Zhongyi]

When -net socket fails, it first reports a specific error, then
a generic one, like this:

$ qemu-system-x86_64 -net socket,
qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=, 
mcast= or udp= is required
qemu-system-x86_64: -net socket: Device 'socket' could not be initialized

Convert net_socket_*_init() to Error to get rid of the superfluous second
error message. After the patch, the effect like this:

$ qemu-system-x86_64 -net socket,
qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=, 
mcast= or udp= is required

This also fixes a few silent failures to report an error.

Cc: jasow...@redhat.com
Cc: arm...@redhat.com
Cc: berra...@redhat.com
Signed-off-by: Mao Zhongyi 
Reviewed-by: Markus Armbruster 
---
 net/socket.c | 92 +---
 1 file changed, 44 insertions(+), 48 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 1d4c515..ad1582e 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -491,22 +491,21 @@ static void net_socket_accept(void *opaque)
 static int net_socket_listen_init(NetClientState *peer,
   const char *model,
   const char *name,
-  const char *host_str)
+  const char *host_str,
+  Error **errp)
 {
 NetClientState *nc;
 NetSocketState *s;
 struct sockaddr_in saddr;
 int fd, ret;
-Error *err = NULL;
 
-if (parse_host_port(&saddr, host_str, &err) < 0) {
-error_report_err(err);
+if (parse_host_port(&saddr, host_str, errp) < 0) {
 return -1;
 }
 
 fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
 if (fd < 0) {
-perror("socket");
+error_setg_errno(errp, errno, "can't create stream socket");
 return -1;
 }
 qemu_set_nonblock(fd);
@@ -515,13 +514,14 @@ static int net_socket_listen_init(NetClientState *peer,
 
 ret = bind(fd, (struct sockaddr *)&saddr, sizeof(saddr));
 if (ret < 0) {
-perror("bind");
+error_setg_errno(errp, errno, "can't bind ip=%s to socket",
+ inet_ntoa(saddr.sin_addr));
 closesocket(fd);
 return -1;
 }
 ret = listen(fd, 0);
 if (ret < 0) {
-perror("listen");
+error_setg_errno(errp, errno, "can't listen on socket");
 closesocket(fd);
 return -1;
 }
@@ -540,21 +540,20 @@ static int net_socket_listen_init(NetClientState *peer,
 static int net_socket_connect_init(NetClientState *peer,
const char *model,
const char *name,
-   const char *host_str)
+   const char *host_str,
+   Error **errp)
 {
 NetSocketState *s;
 int fd, connected, ret;
 struct sockaddr_in saddr;
-Error *err = NULL;
 
-if (parse_host_port(&saddr, host_str, &err) < 0) {
-error_report_err(err);
+if (parse_host_port(&saddr, host_str, errp) < 0) {
 return -1;
 }
 
 fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
 if (fd < 0) {
-perror("socket");
+error_setg_errno(errp, errno, "can't create stream socket");
 return -1;
 }
 qemu_set_nonblock(fd);
@@ -570,7 +569,7 @@ static int net_socket_connect_init(NetClientState *peer,
errno == EINVAL) {
 break;
 } else {
-perror("connect");
+error_setg_errno(errp, errno, "can't connect socket");
 closesocket(fd);
 return -1;
 }
@@ -579,9 +578,8 @@ static int net_socket_connect_init(NetClientState *peer,
 break;
 }
 }
-s = net_socket_fd_init(peer, model, name, fd, connected, &err);
+s = net_socket_fd_init(peer, model, name, fd, connected, errp);
 if (!s) {
-error_report_err(err);
 return -1;
 }
 snprintf(s->nc.info_str, sizeof(s->nc.info_str),
@@ -594,36 +592,36 @@ static int net_socket_mcast_init(NetClientState *peer,
  const char *model,
  const char *name,
  const char *host_str,
- const char *localaddr_str)
+ const char *localaddr_str,
+ Error **errp)
 {
 NetSocketState *s;
 int fd;
 struct sockaddr_in saddr;
 struct in_addr localaddr, *param_localaddr;
-Error *err = NULL;
 
-if (parse_host_port(&saddr, host_str, &err) < 0) {
-error_report_err(err);
+if (parse_host_port(&saddr, host_str, errp) < 0) {
 return -1;
 }
 
 if (localaddr_str != NULL) {
-if (inet_aton(localaddr_str, &localaddr) == 0)
+if (inet_aton(localaddr_str, &localaddr

Re: [Qemu-devel] Managing architectural restrictions with -device and libvirt

[Marcel Apfelbaum]

On 05/07/2017 21:05, Markus Armbruster wrote:

Mark Cave-Ayland  writes:


On 05/07/17 16:46, Markus Armbruster wrote:


I've been working on a patchset that brings the sun4u machine on
qemu-system-sparc64 much closer to a real Ultra 5, however due to
various design restrictions I need to be able to restrict how devices
are added to the machine with -device.

On a real Ultra 5, the root PCI bus (sabre) has 2 PCI bridges (simba A
and simba B) with the onboard devices attached to simba A with 2 free
slots, and an initially empty simba B.

Firstly, is it possible to restrict the machine so that devices cannot
be directly plugged into the root PCI bus, but only behind one of the
PCI bridges? There is also an additional restriction in that slot 0
behind simba A must be left empty to ensure that the ebus (containing
the onboard devices) is the first device allocated.


I figure sabre, simba A, simba B and the onboard devices attached to
simba A are all created by MachineClass init().


Yes that is effectively correct, although the Simba devices are created
as part of the PCI host bridge (apb) creation in pci_apb_init().


Anything that runs within init() counts as "created by init()".


Okay, in that case we should be fine here.


What device provides "the ebus", and how is it created?


It's actually just an ISA bus, so the ebus device is effectively a
PCI-ISA bridge for legacy devices.


Is this bridge created by init()?


Yes, it too is called via the machine init function.


Can you provide a list of all onboard PCI devices and how they are
connected?  Diagram would be best.


I can try and come up with something more concise later, however I can
quickly give you the OpenBIOS DT from my WIP patchset if that helps:

0 > show-devs
ffe1bf38 /
ffe1c110 /aliases
ffe1c238 /openprom (BootROM)
ffe26b50 /openprom/client-services
ffe1c4f0 /options
ffe1c5d0 /chosen
ffe1c710 /builtin
ffe1c838 /builtin/console
ffe26618 /packages
ffe28640 /packages/cmdline
ffe28890 /packages/disk-label
ffe2c8d8 /packages/deblocker
ffe2cef0 /packages/grubfs-files
ffe2d300 /packages/sun-parts
ffe2d718 /packages/elf-loader
ffe2b210 /memory@0,0 (memory)
ffe2b370 /virtual-memory
ffe2d878 /pci@1fe,0 (pci)
ffe2e1a8 /pci@1fe,0/pci@1,1 (pci)
ffe2e960 /pci@1fe,0/pci@1,1/ebus@1
ffe2f1b0 /pci@1fe,0/pci@1,1/ebus@1/eeprom@0
ffe2f328 /pci@1fe,0/pci@1,1/ebus@1/fdthree@0 (block)
ffe2f878 /pci@1fe,0/pci@1,1/ebus@1/su@0 (serial)
ffe2fc08 /pci@1fe,0/pci@1,1/ebus@1/8042@0 (8042)
ffe2fe00 /pci@1fe,0/pci@1,1/ebus@1/8042@0/kb_ps2@0 (serial)
ffe301b0 /pci@1fe,0/pci@1,1/NE2000@1,1 (network)
ffe307c8 /pci@1fe,0/pci@1,1/QEMU,VGA@2 (display)
ffe31e40 /pci@1fe,0/pci@1,1/ide@3 (ide)
ffe32398 /pci@1fe,0/pci@1,1/ide@3/ide0@4100 (ide)
ffe32678 /pci@1fe,0/pci@1,1/ide@3/ide1@4200 (ide)
ffe32910 /pci@1fe,0/pci@1,1/ide@3/ide1@4200/cdrom@0 (block)
ffe32f98 /pci@1fe,0/pci@1 (pci)
ffe336e8 /SUNW,UltraSPARC-IIi (cpu)
  ok

For comparison you can see the DT from a real Ultra 5 here:
http://www.pearsonitcertification.com/articles/article.aspx?p=440286&seqNum=7


The real sabre has two slots, and doesn't support hot (un)plug.  Can we
simply model that?  If yes, the root PCI bus is full after init(), and
remains full.  Takes care of "cannot directly plugged into the root PCI
bus".


Right. So what you're saying is that if we add the 2 simba devices to
the sabre PCI host bridge during machine init and then mark the sabre
PCI root bus as not hotplug-able then that will prevent people adding
extra devices from the command line via -device? I will see if I can
find time to try this later this evening.


No.  Marking the bus "not hotpluggable" only prevents *hotplug*,
i.e. plug/unplug after machine initialization completed, commonly with
device_add.  -device is *cold* plug; it happens during machine
initialization.

However, if you limit sabre's bus to two slots (modelling real hardware
faithfully), then you can't cold plug anything (there's no free slot).
If you additionally mark the bus or both simba devices not hotpluggable
(again modelling real hardware faithfully), you can't unplug the simbas.
I believe that's what you want.


It seems like limiting the size of the bus would solve the majority of
the problem. I've had a quick look around pci.c and while I can see that
the PCIBus creation functions take a devfn_min parameter, I can't see
anything that limits the number of slots available on the bus?


Marcel?



Hi Markus,
Sorry for my late reply.

Indeed, we don't have currently a restriction on the number of usable
slots on a bus, however deriving from PCIBus class and implementing
the new policy should not be much trouble.



And presumably if the user did try and coldplug something into a full
bus then they would get the standard "PCI: no slot/function
available..." error?


That's what I'd expect.


My understanding from reading various bits of documentation is that the
the empty simba bridge (bus B) can hold a maximum of 4 devices, whilst
the non-empty simba bridge (bus A) can 

Re: [Qemu-devel] [PATCH v2 6/6] nbd: use generic trace subsystem instead of TRACE macro

[Paolo Bonzini]

On 06/07/2017 10:45, Vladimir Sementsov-Ogievskiy wrote:
>>> +do_nbd_trip_read(uint32_t len) "Read %" PRIu32" byte(s)"
>> This one is good.
> 
> why do you like this and do not like nbd_trip_write_zeros?

I'm not sure I understand: this one is after blk_pread returns.  It
tells you that the socket is about to receive the payload.

For write, nothing special happened since do_nbd_trip_cmd_write.

For write zeroes, nothing special happened since
nbd_co_receive_request_decode_type.

>>
>>> +do_nbd_trip_cmd_write(void) "Request type is WRITE"
>>> +do_nbd_trip_cmd_write_readonly(void) "Server is read-only, return
>>> error"
>> These can be removed.
> 
> I think the second is informative, isn't it?

Can you instead add a trace point at the beginning of nbd_co_send_reply,
with handle/error/len?  Then you can remove do_nbd_trip_read and
do_nbd_trip_complete, too.

>>> +do_nbd_trip_write(void) "Writing to device"
>> Please add the handle here.
> 
> handle will be printed in nbd_co_receive_request_decode_type..

You need it in case there are multiple requests in flight.

>>> +do_nbd_trip_cmd_write_zeroes(void) "Request type is WRITE_ZEROES"
>>> +do_nbd_trip_write_zeroes(void) "Writing to device"
>>> +do_nbd_trip_cmd_flush(void) "Request type is FLUSH"
>>> +do_nbd_trip_cmd_trim(void) "Request type is TRIM"
>> Can all be removed.
> 
> then, remove nbd_trip_write too ?

Write is different because it happens after nbd_read(client->ioc,
req->data, request->len, NULL) has completed.  You can add a tracepoint
to nbd_co_receive_request instead.

Paolo

Re: [Qemu-devel] [PATCH 05/11] linux-user/sh4: Notice gUSA regions during signal delivery

[John Paul Adrian Glaubitz]

On Thu, Jul 06, 2017 at 10:35:15AM +0200, Laurent Vivier wrote:
> This is a patch series proposed by Richard, it is not included in qemu
> at the moment:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg01196.html

Aye, awesome!

> I think it would be interesting if you generate a qemu binary with it
> and use it in your buildds to test it.
> 
> I guess the series can be pulled directly from Richard's branch:
> 
> git://github.com/rth7680/qemu.git tgt-sh4

You bet, I will!

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [Qemu-devel] [Qemu-block] [PATCH v2 1/3] block: pass bdrv_* methods to bs->file by default

[Stefan Hajnoczi]

On Thu, Jun 29, 2017 at 09:43:18PM +0300, Manos Pitsidianakis wrote:
> diff --git a/block.c b/block.c
> index 69439628..9e8d34ad 100644
> --- a/block.c
> +++ b/block.c
> @@ -494,6 +494,8 @@ int bdrv_probe_blocksizes(BlockDriverState *bs, 
> BlockSizes *bsz)
>  
>  if (drv && drv->bdrv_probe_blocksizes) {
>  return drv->bdrv_probe_blocksizes(bs, bsz);
> +} else if (drv && bs->file && bs->file->bs) {
> +return bdrv_probe_blocksizes(bs->file->bs, bsz);
>  }
>  
>  return -ENOTSUP;

Currently only raw-format.c and file-posix.c implement
bdrv_probe_blocksizes().  qcow2 will start reporting bs->file's
blocksizes after this patch.

This can lead to a change in blocksizes when live migrating from an old
QEMU to a new QEMU.  Guest operating systems and applications can be
confused if the device suddenly changes beneath them.

On the other hand, it's already possible to hit that today by migrating
from a raw image to a qcow2 image.

I think this change makes sense - we should propagate blocksizes through
the graph - but it may introduce an incompatibility.

Kevin: Do you think this is safe?

> @@ -3406,11 +3410,15 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, 
> Error **errp)
>  
>  assert(child->perm & BLK_PERM_RESIZE);
>  
> +/* if bs->drv == NULL, bs is closed, so there's nothing to do here */
>  if (!drv) {
>  error_setg(errp, "No medium inserted");
>  return -ENOMEDIUM;
>  }
>  if (!drv->bdrv_truncate) {
> +if (bs->file && bs->file->bs) {
> +return bdrv_truncate(bs->file, offset, errp);
> +}
>  error_setg(errp, "Image format driver does not support resize");
>  return -ENOTSUP;
>  }

This is not safe because existing image formats (e.g. vmdk, dmg) do not
implement .bdrv_truncate().  If we begin truncating the underlying host
file ("foo.vmdk") the disk image will be corrupted.

It is only safe to forward .bdrv_truncate() in filter drivers.  I
suggest providing a default implementation instead:

int bdrv_truncate_file(...);

Then filters can do:

  .bdrv_truncate = bdrv_truncate_file,

> diff --git a/block/io.c b/block/io.c
> index c72d7015..a1aee01d 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2401,6 +2401,11 @@ int bdrv_co_ioctl(BlockDriverState *bs, int req, void 
> *buf)
>  };
>  BlockAIOCB *acb;
>  
> +if (drv && !drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl &&
> +bs->file && bs->file->bs) {
> +return bdrv_co_ioctl(bs->file->bs, req, buf);
> +}
> +
>  bdrv_inc_in_flight(bs);
>  if (!drv || (!drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl)) {
>  co.ret = -ENOTSUP;

This operation cannot be allowed by default for the same reason as
.bdrv_truncate().  It could change the host file in ways that the format
driver can't handle.  A separate function is needed here:
bdrv_co_ioctl_file().


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 05/11] linux-user/sh4: Notice gUSA regions during signal delivery

[John Paul Adrian Glaubitz]

On Thu, Jul 06, 2017 at 10:35:15AM +0200, Laurent Vivier wrote:
> I think it would be interesting if you generate a qemu binary with it
> and use it in your buildds to test it.
> 
> I guess the series can be pulled directly from Richard's branch:
> 
> git://github.com/rth7680/qemu.git tgt-sh4

Pull from this tree and built qemu-sh4-user as static from the tgt-sh4
branch. Copied in Debian/sh4 chroot and tried to enter it:

root@nofan:/local_scratch/sid-sh4-sbuild> chroot .
bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault
root@nofan:/local_scratch/sid-sh4-sbuild>

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [Qemu-devel] [PATCH 05/11] linux-user/sh4: Notice gUSA regions during signal delivery

[Laurent Vivier]

Le 06/07/2017 à 11:13, John Paul Adrian Glaubitz a écrit :
> On Thu, Jul 06, 2017 at 10:35:15AM +0200, Laurent Vivier wrote:
>> I think it would be interesting if you generate a qemu binary with it
>> and use it in your buildds to test it.
>>
>> I guess the series can be pulled directly from Richard's branch:
>>
>> git://github.com/rth7680/qemu.git tgt-sh4
> 
> Pull from this tree and built qemu-sh4-user as static from the tgt-sh4
> branch. Copied in Debian/sh4 chroot and tried to enter it:
> 
> root@nofan:/local_scratch/sid-sh4-sbuild> chroot .
> bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> Segmentation fault
> root@nofan:/local_scratch/sid-sh4-sbuild>

Could you try origin/master to see if the regression is introduced by
this series or by a previous one?

Thanks,
Laurent

Re: [Qemu-devel] The maximum limit of virtual network device

[Marcel Apfelbaum]

On 06/07/2017 11:31, Laszlo Ersek wrote:

Hi Jiaxin,

it's nice to see a question from you on qemu-devel! :)

On 07/06/17 08:20, Wu, Jiaxin wrote:

Hello experts,

We know QEMU has the capability to create the multiple network devices
in one QEMU guest with the -device syntax. But I met the below failure
when I'm trying to create more than 30 virtual devices with the each
TAP backend:

qemu-system-x86_64: -device e1000: PCI: no slot/function available for
e1000, all in use.

The corresponding QEMU command shows as following:

sudo qemu-system-x86_64 \
   -pflash OVMF.fd \
   -global e1000.romfile="" \
   -netdev tap,id=hostnet0,ifname=tap0,script=no,downscript=no \
   -device e1000,netdev=hostnet0 \


[...]


   -netdev tap,id=hostnet29,ifname=tap29,script=no,downscript=no \
   -device e1000,netdev=hostnet29

 From above,  the max limit of virtual network device in one guest is
about 29? If not, how can I avoid such failure? My use case is to
create more than 150 network devices in one guest. Please provide your
comments on this.


You are seeing the above symptom because the above command line
instructs QEMU to do the following:
- use the i440fx machine type,
- use a single PCI bus (= the main root bridge),
- add the e1000 cards to separate slots (always using function 0) on
   that bus.

Accordingly, there are three things you can do to remedy this:

- Use the Q35 machine type and work with a PCI Express hierarchy rather
   than a PCI hierarchy. I'm mentioning this only for completeness,
   because it won't directly help your use case. But, I certainly want to
   highlight "docs/pcie.txt". Please read it sometime; it has nice
   examples and makes good points.

- Use multiple PCI bridges to attach the devices. For this, several ways
   are possible:

   - use multiple root buses, with the pxb or pxb-pcie devices (see
 "docs/pci_expander_bridge.txt" and "docs/pcie.txt")

   - use multiple normal PCI bridges

   - use multiple PCI Express root ports or downstream ports (but for
 this, you'll likely have to use the PCI Express variant of the
 e1000, namely e1000e)

- If you don't need hot-plug / hot-unplug, aggregate eights of e1000
   NICs into multifunction PCI devices.
> Now, I would normally recommend sticking with i440fx for simplicity.
However, each PCI bridge requires 4KB of IO space (meaning (1 + 5) * 4KB
= 24KB),  and OVMF on the i440fx does not support that much (only
0x4000). So, I'll recommend Q35 for IO space purposes; OVMF on Q35
provides 0xA000 (40KB).


So if we use OVMF, going for Q35 gives us actually more IO space, nice!
However recommending Q35 for IO space seems odd :)



For scaling higher than this, a PCI Express hierarchy should be used
with PCI Express devices that require no IO space at all. However, that
setup is even more problematic *for now*; please see section "3. IO
space issues" in "docs/pcie.txt". We have open OVMF and QEMU BZs for
limiting IO space allocation to cases when it is really necessary:

   https://bugzilla.redhat.com/show_bug.cgi?id=1344299
   https://bugzilla.redhat.com/show_bug.cgi?id=1434740

Therefore I guess the simplest example I can give now is:
- use Q35 (for a larger IO space),
- plug a DMI-PCI bridge into the root bridge,
- plug 5 PCI bridges into the DMI-PCI bridge,
- plug 31 NICs per PCI bridge, each NIC into a separate slot.



The setup looks OK to me (assuming OVMF is needed, otherwise
PC + pci-bridges will result in more devices),
I do have a little concern.
We want to deprecate the dmi-pci bridge since it does not support 
hot-plug (for itself or devices behind it).

Alexandr (CCed) is a GSOC student working on a generic
pcie-pci bridge that can (eventually) be hot-plugged
into a PCIe Root Port and keeps the machine cleaner.

See:
https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg05498.html

If is a "lab" project it doesn't really matter, but I wanted
to point out the direction.

Thanks,
Marcel


This follows the following recommendation of "2.3 PCI only hierarchy" in
"docs/pcie.txt" (slightly rewrapped here):


2.3 PCI only hierarchy
==
Legacy PCI devices can be plugged into pcie.0 as Integrated Endpoints,
but, as mentioned in section 5, doing so means the legacy PCI
device in question will be incapable of hot-unplugging.
Besides that use DMI-PCI Bridges (i82801b11-bridge) in combination
with PCI-PCI Bridges (pci-bridge) to start PCI hierarchies.

Prefer flat hierarchies. For most scenarios a single DMI-PCI Bridge
(having 32 slots) and several PCI-PCI Bridges attached to it (each
supporting also 32 slots) will support hundreds of legacy devices. The
recommendation is to populate one PCI-PCI Bridge under the DMI-PCI
Bridge until is full and then plug a new PCI-PCI Bridge...


Here's a command line. Please note that the OVMF boot may take quite
long with this, as the E3522X2.EFI driver from BootUtil (-D
E1000_ENABLE) binds all 150 e1000 NICs in succession! Watching the OVMF
debug log is recommended.

qemu-s

Re: [Qemu-devel] [PATCH] hmp: Update info vnc

[Dr. David Alan Gilbert]

* Marc-André Lureau (marcandre.lur...@redhat.com) wrote:
> 
> 
> - Original Message -
> > From: "Dr. David Alan Gilbert" 
> > 
> > The QMP query-vnc interfaces have gained a lot more information that
> > the HMP interfaces hasn't got yet. Update it.
> > 
> > Note the output format has changed, but this is HMP so that's OK.
> > 
> > In particular, this now includes client information for reverse
> > connections:
> > 
> > -vnc :0
> > (qemu) info vnc
> > default:
> >   Server: 0.0.0.0:5900 (ipv4)
> > Auth: none (Sub: none)
> >   Auth: none (Sub: none)
> > 
> >   (Now connect a client)
> > 
> > (qemu) info vnc
> > default:
> >   Server: 0.0.0.0:5900 (ipv4)
> > Auth: none (Sub: none)
> >   Client: 127.0.0.1:51828 (ipv4)
> > x509_dname: none
> > username: none
> >   Auth: none (Sub: none)
> > 
> > -vnc localhost:7000,reverse
> > (qemu) info vnc
> > default:
> >   Client: ::1:7000 (ipv6)
> > x509_dname: none
> > username: none
> >   Auth: none (Sub: none)
> > 
> > -vnc :1,password,id=rev -vnc localhost:7000,reverse
> > (qemu) info vnc
> > default:
> >   Client: ::1:7000 (ipv6)
> > x509_dname: none
> > username: none
> >   Auth: none (Sub: none)
> > rev:
> >   Server: 0.0.0.0:5901 (ipv4)
> > Auth: vnc (Sub: none)
> >   Client: 127.0.0.1:53616 (ipv4)
> > x509_dname: none
> > username: none
> >   Auth: vnc (Sub: none)
> > 
> > This was originally RH bz 1461682
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  hmp.c | 98
> >  ++-
> >  1 file changed, 67 insertions(+), 31 deletions(-)
> > 
> > diff --git a/hmp.c b/hmp.c
> > index dee40284c1..c11a5fe2c6 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -600,50 +600,86 @@ void hmp_info_blockstats(Monitor *mon, const QDict
> > *qdict)
> >  qapi_free_BlockStatsList(stats_list);
> >  }
> >  
> > +/* Helper for hmp_info_vnc_clients, _servers */
> > +static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,
> > +  const char *name)
> > +{
> > +monitor_printf(mon, "  %s: %s:%s (%s%s)\n",
> > +   name,
> > +   info->host,
> > +   info->service,
> > +   NetworkAddressFamily_lookup[info->family],
> > +   info->websocket ? " (Websocket)" : "");
> > +}
> > +
> > +/* Helper displaying and euth and crypt info */
> > +static void hmp_info_vnc_authcrypt(Monitor *mon, const char *indent,
> > +   VncPrimaryAuth auth,
> > +   VncVencryptSubAuth *vencrypt)
> > +{
> > +monitor_printf(mon, "%sAuth: %s (Sub: %s)\n", indent,
> > +   VncPrimaryAuth_lookup[auth],
> > +   vencrypt ? VncVencryptSubAuth_lookup[*vencrypt] :
> > "none");
> > +}
> > +
> > +static void hmp_info_vnc_clients(Monitor *mon, VncClientInfoList *client)
> > +{
> > +while (client) {
> > +VncClientInfo *cinfo = client->value;
> > +
> > +hmp_info_VncBasicInfo(mon, (VncBasicInfo *)cinfo, "Client");
> > +monitor_printf(mon, "x509_dname: %s\n",
> > +   cinfo->x509_dname ?
> 
> Why not use has_x509_dname ?

Interesting; that line I just moved from the old code;  fixed.

> > +   cinfo->x509_dname : "none");
> > +monitor_printf(mon, "username: %s\n",
> 
> sasl_username would be more clear

Also from the old code; also fixed.

Thanks,

Dave

> > +   cinfo->has_sasl_username ?
> > +   cinfo->sasl_username : "none");
> > +
> > +client = client->next;
> > +}
> > +}
> > +
> > +static void hmp_info_vnc_servers(Monitor *mon, VncServerInfo2List *server)
> > +{
> > +while (server) {
> > +VncServerInfo2 *sinfo = server->value;
> > +hmp_info_VncBasicInfo(mon, (VncBasicInfo *)sinfo, "Server");
> > +hmp_info_vnc_authcrypt(mon, "", sinfo->auth,
> > +   sinfo->has_vencrypt ? &sinfo->vencrypt :
> > NULL);
> > +server = server->next;
> > +}
> > +}
> > +
> >  void hmp_info_vnc(Monitor *mon, const QDict *qdict)
> >  {
> > -VncInfo *info;
> > +VncInfo2List *info2l;
> >  Error *err = NULL;
> > -VncClientInfoList *client;
> >  
> > -info = qmp_query_vnc(&err);
> > +info2l = qmp_query_vnc_servers(&err);
> >  if (err) {
> >  error_report_err(err);
> >  return;
> >  }
> > -
> > -if (!info->enabled) {
> > -monitor_printf(mon, "Server: disabled\n");
> > -goto out;
> > -}
> > -
> > -monitor_printf(mon, "Server:\n");
> > -if (info->has_host && info->has_service) {
> > -monitor_printf(mon, " address: %s:%s\n", info->host,
> > info->service);
> > -}
> > -if (info->has_auth) {
> > -monitor_printf(mon, "auth: %s\n", info->auth);
> > +if (!info2l) {
> > +monitor_printf(mon, "None\n");
> > + 

Re: [Qemu-devel] [PATCH v2 2/3] block: use defaults of bdrv_* callbacks in raw

[Stefan Hajnoczi]

On Thu, Jun 29, 2017 at 09:43:19PM +0300, Manos Pitsidianakis wrote:
> Now that passing the call to bs->file is the default for some bdrv_*
> callbacks, remove the duplicate implementations in block/raw-format.c
> 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  block/raw-format.c | 32 +---
>  1 file changed, 1 insertion(+), 31 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] DIRTY_MEMORY_BLOCK_SIZE;

[Stefan Hajnoczi]

On Wed, Jul 05, 2017 at 05:06:41PM +0100, Dr. David Alan Gilbert wrote:
> * Stefan Hajnoczi (stefa...@redhat.com) wrote:
> > On Fri, Jun 30, 2017 at 12:26:10PM +0100, Dr. David Alan Gilbert wrote:
> > > * ali saeedi (ali.saeed...@gmail.com) wrote:
> > > > Hello
> > > > what does 'DIRTY_MEMORY_BLOCK_SIZE' mean?
> > > > is it the number of words in a block? or number of pages in a block? or
> > > > number of bytes in a block?
> > > > thanks a lot
> > > 
> > > (cc'ing Stefan)
> > > I think that DIRTY_MEMORY_BLOCK_SIZE is the number of TARGET_PAGEs
> > > within one DIRTY_MEMORY_BLOCK
> > > So with the common 4k target page that's 4k*256k*8=8GB/dirty memory
> > > block - note these are just the size of structure sin qemu, it's still
> > > got the ranularity ot mark individual target pages as dirty.
> > 
> > Right, the calculation is shown in the comment above the code:
> > 
> >  *   rcu_read_lock();
> >  *
> >  *   DirtyMemoryBlocks *blocks =
> >  *   atomic_rcu_read(&ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]);
> >  *
> >  *   ram_addr_t idx = (addr >> TARGET_PAGE_BITS) / DIRTY_MEMORY_BLOCK_SIZE;
> >  *   unsigned long *block = blocks.blocks[idx];
> >  *   ...access block bitmap...
> >  *
> >  *   rcu_read_unlock();
> > 
> > Rather than focussing on DIRTY_MEMORY_BLOCK_SIZE, make sure you
> > understand how DirtyMemoryBlocks works.  It is an array of bitmap
> > pointers.
> > 
> > Instead of directly indexing into a single huge dirty memory bitmap,
> > QEMU divides the dirty memory bitmap into fixed-sized chunks.  Each
> > chunk covers DIRTY_MEMORY_BLOCK_SIZE pages.
> > 
> > The reason for this layer of indirection is so that the dirty memory
> > bitmap can be accessed without taking a traditional lock (just RCU) and
> > also supports memory hotplug.
> > 
> > Without indirection it would be difficult to grow the bitmap while other
> > threads are writing to it.  Thanks to the indirection, it's possible to
> > allocate new chunks and continue using the old chunks when growth
> > occurs.
> 
> I guess this works like the old non-chunk version, in that there's no
> direct correspondence between DirtyMemoryBlocks and RAMBlock's - i.e.
> one RAMBlock might span two DirtyMemoryBlocks even if it's smaller
> than DIRTY_MEMORY_BLOCK_SIZE.

Yes.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] The maximum limit of virtual network device

[Laszlo Ersek]

On 07/06/17 11:24, Marcel Apfelbaum wrote:
> On 06/07/2017 11:31, Laszlo Ersek wrote:

>> > Now, I would normally recommend sticking with i440fx for simplicity.
>> However, each PCI bridge requires 4KB of IO space (meaning (1 + 5) * 4KB
>> = 24KB),  and OVMF on the i440fx does not support that much (only
>> 0x4000). So, I'll recommend Q35 for IO space purposes; OVMF on Q35
>> provides 0xA000 (40KB).
> 
> So if we use OVMF, going for Q35 gives us actually more IO space, nice!
> However recommending Q35 for IO space seems odd :)

OVMF used to have only 0x4000 bytes for PCI IO aperture, since the
beginning. In  I
investigated how much I could grow the aperture. On Q35, it was possible
to grow it to 0xA000 bytes (but even then you have to disable vmport,
which sort of sits in the middle otherwise). On i440fx, the IO ports in
use by platform devices were so badly distributed that moving beyond
0x4000 was not possible. See in particular:

https://bugzilla.redhat.com/show_bug.cgi?id=1333238#c16
https://bugzilla.redhat.com/show_bug.cgi?id=1333238#c19

>> Therefore I guess the simplest example I can give now is:
>> - use Q35 (for a larger IO space),
>> - plug a DMI-PCI bridge into the root bridge,
>> - plug 5 PCI bridges into the DMI-PCI bridge,
>> - plug 31 NICs per PCI bridge, each NIC into a separate slot.
>>
> 
> The setup looks OK to me (assuming OVMF is needed, otherwise
> PC + pci-bridges will result in more devices),

OVMF is quite needed; Jiaxin is one of the edk2 networking maintainers,
and I think he's using QEMU and OVMF as a testbed for otherwise
physically-oriented UEFI development.

> I do have a little concern.
> We want to deprecate the dmi-pci bridge since it does not support
> hot-plug (for itself or devices behind it).
> Alexandr (CCed) is a GSOC student working on a generic
> pcie-pci bridge that can (eventually) be hot-plugged
> into a PCIe Root Port and keeps the machine cleaner.

Nice!

Please include an update to "docs/pcie.txt" in the scope :)

Thanks!
Laszlo

Re: [Qemu-devel] Page Block word

[Stefan Hajnoczi]

On Fri, Jun 30, 2017 at 04:31:30PM +0430, ali saeedi wrote:
> what is the difference between word, page and block in qemu?

The meaning of these terms depends on the context.

It's not possible to answer since you didn't give any context.  It's
like asking, "What is the meaning of 'element' in QEMU?".  There are
many meanings, it depends which area you are looking at.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH 0/5] spapr: DRC cleanups (part VI)

[David Gibson]

On Wed, Jul 05, 2017 at 05:41:40PM -0500, Michael Roth wrote:
> Quoting Daniel Henrique Barboza (2017-07-05 16:53:57)
> > 
> > 
> > On 07/05/2017 08:04 AM, David Gibson wrote:
> > > On Tue, Jul 04, 2017 at 06:13:31PM -0300, Daniel Henrique Barboza wrote:
> > >> I just tested this patch set on top of current ppc-for-2.10 branch (which
> > >> contains
> > >> the patches from part V). It applied cleanly but required a couple of
> > >> trivial
> > >> fixes to build probably because it was made on top of an older code base.
> > > Right, I fixed that up locally already, but haven't gotten around to
> > > reposting yet.  You can look at the 'drcVI' branch on my github tree,
> > > if you're interested.
> > >
> > >> The trivial migration test worked fine. The libvirt scenario (attaching a
> > >> device on
> > >> target before migration, try to unplug after migration) isn't working as
> > >> expected
> > >> but we have a different result with this series. Instead of silently 
> > >> failing
> > >> to unplug
> > >> with error messages on dmesg, the hot unplug works on QEMU level:
> > > Thanks for testing.  Just to clarify what you're saying here, you
> > > haven't spotted a regression with this series, but there is a case
> > > which was broken and is still broken with slightly different
> > > symptoms.  Yes?
> > In my opinion, yes. It is debatable if the patch series made it worse 
> > because
> > the guest is now misbehaving, but the feature per se wasn't working
> > prior to it.
> 
> I think it's the removal of awaiting_allocation.

So.. yes, in the sense that I think we've rediscovered the problem
which prompter the awaiting_allocation flag in the first place.  I
still don't think awaiting_allocation is a sensible fix for.. well,
anything.

So we need to find the right fix for this problem.

> We know currently that
> in the libvirt scenario the DRC is exposed in an pre-hotplug state of
> ISOLATED/UNALLOCATED.

Right, need to understand exactly how we get there.

> In that state, spapr_drc_detach() completes
> immediately because from the perspective of QEMU it apparently has not
> been exposed to the guest yet, or the guest has already quiesced it on
> it's end.

Right.

> awaiting_allocation guarded against this, as it's intention was to make
> sure that resource was put into an ALLOCATED state prior to getting moved
> back into an UNALLOCATED state,

Right, but that seems broken to me.  It means if a cpu is hotplug when
the guest isn't paying attention (e.g. early boot, guest is
halted/crashed), then you can't remove it until you either boot an OS
that allocates then releases it, or you reset (and then release).

> so we didn't immediately unplug a CPU
> while the hotplug was in progress.

But in what sense is the hotplug "in progress"?  The guest has been
given a notification, but it hasn't touched the device yet.  Moving
the state away from UNALLOCATED is never guaranteed to work,
regardless of what notifications have been received.

> So in your scenario the CPU is just mysteriously vanishing out from
> under the guest,

Because the DRC is ending up in UNUSABLE state on the destination when
it should be in some other state (and was on the source)?  Yeah, that
sounds plausible.

> which probably explains the hang. The fix for this
> particular scenario is to fix the initial DRC on the target. I think
> we have a plan for that so I wouldn't consider this a regression
> necessarily.

Yeah.. I'm not quite sure how it's ending up wrong; why isn't the
incoming migration state setting it correctly?

-- 
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 3/4] xen/mapcache: introduce xen_replace_cache_entry()

[Paul Durrant]

> -Original Message-
> > > >
> > > > The problem really comes down to defining
> xenforeignmemory_map2() in
> > > terms of xenforeignmemory_map(). It basically can't be safely done.
> Could
> > > you define xenforeignmemory_map2() as abort() in the compat case
> > > instead?
> > > >
> > >
> > > xen_replace_cache_entry() is not called in patch #3. Which means it's
> > > safe to use a fallback version (xenforeignmemory_map) in
> > > xen_remap_bucket here.
> >
> > I still don't like the fact that the compat definition of
> xenforeignmemory_map2() loses the extra argument. That's going to catch
> someone out one day. Is there any way you could re-work it so that
> xenforeignmemory_map() is uses in the cases where the memory
> placement does not matter?
> 
> We could assert(vaddr == NULL) in the compat implementation of
> xenforeignmemory_map2. Would that work?
> 

Yes, if the patch was changed from being a straight #define as it is now to an 
inline that had such an assertion then that would be ok.

  Cheers,

Paul

Re: [Qemu-devel] [PATCH] hmp: Update info vnc

[Dr. David Alan Gilbert]

* Daniel P. Berrange (berra...@redhat.com) wrote:
> On Wed, Jul 05, 2017 at 05:33:24PM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrange (berra...@redhat.com) wrote:
> > > On Wed, Jul 05, 2017 at 03:43:02PM +0100, Dr. David Alan Gilbert (git) 
> > > wrote:
> > > > From: "Dr. David Alan Gilbert" 
> > > > 
> > > > The QMP query-vnc interfaces have gained a lot more information that
> > > > the HMP interfaces hasn't got yet. Update it.
> > > > 
> > > > Note the output format has changed, but this is HMP so that's OK.
> > > > 
> > > > In particular, this now includes client information for reverse
> > > > connections:
> > > > 
> > > > -vnc :0
> > > > (qemu) info vnc
> > > > default:
> > > >   Server: 0.0.0.0:5900 (ipv4)
> > > > Auth: none (Sub: none)
> > > >   Auth: none (Sub: none)
> > > 
> > > If you're reporting Auth: against the individual Server entry,
> > > there's no reason to report it at the top level too. We only
> > > keep that duplication in QMP for sake of backcompat, which
> > > is not important for HMP
> > 
> > Where would it get reported for a 'reverse' connection then?
> > That has no server entry.
> 
> Ah right. So lets only report it at the top level, if operating
> in reverse mode.

OK, new version coming up.

Dave

> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v2] hmp: Update info vnc

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

The QMP query-vnc interfaces have gained a lot more information that
the HMP interfaces hasn't got yet. Update it.

Note the output format has changed, but this is HMP so that's OK.

In particular, this now includes client information for reverse
connections:

-vnc :0
(qemu) info vnc
default:
  Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)

  (Now connect a client)

(qemu) info vnc
default:
  Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
  Client: 127.0.0.1:51828 (ipv4)
x509_dname: none
sasl_username: none

-vnc localhost:7000,reverse
(qemu) info vnc
default:
  Client: ::1:7000 (ipv6)
x509_dname: none
sasl_username: none
  Auth: none (Sub: none)

-vnc :1,password,id=pass -vnc localhost:7000,reverse
(qemu) info vnc
default:
  Client: ::1:7000 (ipv6)
x509_dname: none
sasl_username: none
  Auth: none (Sub: none)
rev:
  Server: 0.0.0.0:5901 (ipv4)
Auth: vnc (Sub: none)
  Client: 127.0.0.1:53616 (ipv4)
x509_dname: none
sasl_username: none

This was originally RH bz 1461682

Signed-off-by: Dr. David Alan Gilbert 

--
v2
  Say 'sasl_username' - from Marc-André's comment
  use 'has_x509_dname' - from Marc-André's comment
  Don't display top level auth info except in reverse
  case - Dan's comment
---
 hmp.c | 104 ++
 1 file changed, 73 insertions(+), 31 deletions(-)

diff --git a/hmp.c b/hmp.c
index dee40284c1..7db797ed9d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -600,50 +600,92 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)
 qapi_free_BlockStatsList(stats_list);
 }
 
+/* Helper for hmp_info_vnc_clients, _servers */
+static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,
+  const char *name)
+{
+monitor_printf(mon, "  %s: %s:%s (%s%s)\n",
+   name,
+   info->host,
+   info->service,
+   NetworkAddressFamily_lookup[info->family],
+   info->websocket ? " (Websocket)" : "");
+}
+
+/* Helper displaying and euth and crypt info */
+static void hmp_info_vnc_authcrypt(Monitor *mon, const char *indent,
+   VncPrimaryAuth auth,
+   VncVencryptSubAuth *vencrypt)
+{
+monitor_printf(mon, "%sAuth: %s (Sub: %s)\n", indent,
+   VncPrimaryAuth_lookup[auth],
+   vencrypt ? VncVencryptSubAuth_lookup[*vencrypt] : "none");
+}
+
+static void hmp_info_vnc_clients(Monitor *mon, VncClientInfoList *client)
+{
+while (client) {
+VncClientInfo *cinfo = client->value;
+
+hmp_info_VncBasicInfo(mon, (VncBasicInfo *)cinfo, "Client");
+monitor_printf(mon, "x509_dname: %s\n",
+   cinfo->has_x509_dname ?
+   cinfo->x509_dname : "none");
+monitor_printf(mon, "sasl_username: %s\n",
+   cinfo->has_sasl_username ?
+   cinfo->sasl_username : "none");
+
+client = client->next;
+}
+}
+
+static void hmp_info_vnc_servers(Monitor *mon, VncServerInfo2List *server)
+{
+while (server) {
+VncServerInfo2 *sinfo = server->value;
+hmp_info_VncBasicInfo(mon, (VncBasicInfo *)sinfo, "Server");
+hmp_info_vnc_authcrypt(mon, "", sinfo->auth,
+   sinfo->has_vencrypt ? &sinfo->vencrypt : NULL);
+server = server->next;
+}
+}
+
 void hmp_info_vnc(Monitor *mon, const QDict *qdict)
 {
-VncInfo *info;
+VncInfo2List *info2l;
 Error *err = NULL;
-VncClientInfoList *client;
 
-info = qmp_query_vnc(&err);
+info2l = qmp_query_vnc_servers(&err);
 if (err) {
 error_report_err(err);
 return;
 }
-
-if (!info->enabled) {
-monitor_printf(mon, "Server: disabled\n");
-goto out;
-}
-
-monitor_printf(mon, "Server:\n");
-if (info->has_host && info->has_service) {
-monitor_printf(mon, " address: %s:%s\n", info->host, 
info->service);
-}
-if (info->has_auth) {
-monitor_printf(mon, "auth: %s\n", info->auth);
+if (!info2l) {
+monitor_printf(mon, "None\n");
+return;
 }
 
-if (!info->has_clients || info->clients == NULL) {
-monitor_printf(mon, "Client: none\n");
-} else {
-for (client = info->clients; client; client = client->next) {
-monitor_printf(mon, "Client:\n");
-monitor_printf(mon, " address: %s:%s\n",
-   client->value->host,
-   client->value->service);
-monitor_printf(mon, "  x509_dname: %s\n",
-   client->value->x509_dname ?
-   client->value->x509_dname : "none");
-monitor_printf(mon, "username: %s\n",
-   client->value->has_sasl_username ?
- 

Re: [Qemu-devel] [PATCH 7/7] MAINTAINERS: add Dump maintainers

[Marc-André Lureau]

Hi

- Original Message -
> On 06/29/17 15:23, Marc-André Lureau wrote:
> > Proposing myself, since I have some familiarity with the code now.
> > 
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  MAINTAINERS | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 839f7ca063..45a0eb4cb0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1272,6 +1272,13 @@ S: Maintained
> >  F: device_tree.c
> >  F: include/sysemu/device_tree.h
> >  
> > +Dump
> > +S: Supported
> > +M: Marc-André Lureau 
> > +F: dump.c
> > +F: include/sysemu/dump.h
> > +F: include/sysemu/dump-arch.h
> > +
> >  Error reporting
> >  M: Markus Armbruster 
> >  S: Supported
> > 
> 
> That's very kind of you, thanks!
> 
> Do you have suggestions for the following files?
> 
> scripts/dump-guest-memory.py

This one is yours, no? :) But I am ok to "support" it, meaning I'll take time 
to review the patches, and eventually make the pull-requests.
 
> stubs/dump.c

I can add that one, although it is also maintained by Paolo

> target/arm/arch_dump.c
> target/i386/arch_dump.c
> target/ppc/arch_dump.c
> target/s390x/arch_dump.c

I'd rather have those maintained by the respective arch maintainers, as they 
are. But I imagine it would make sense to also cover them with the rest of dump.

thanks

Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] block: remove timer canceling in throttle_config()

[Stefan Hajnoczi]

On Sun, Jul 02, 2017 at 01:06:46PM +0300, Manos Pitsidianakis wrote:
> throttle_config() cancels the timers of the calling BlockBackend. This
> doesn't make sense because other BlockBackends in the group remain
> untouched. There's no need to cancel the timers in the one specific
> BlockBackend so let's not do that. Throttled requests will run as
> scheduled and future requests will follow the new configuration. This
> also allows a throttle group's configuration to be changed even when it
> has no members.
> 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  block/throttle-groups.c | 10 +-
>  fsdev/qemu-fsdev-throttle.c |  2 +-
>  include/qemu/throttle.h |  1 -
>  tests/test-throttle.c   |  4 ++--
>  util/throttle.c | 14 --
>  5 files changed, 4 insertions(+), 27 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] block: add clock_type field to ThrottleGroup

[Stefan Hajnoczi]

On Sun, Jul 02, 2017 at 01:06:45PM +0300, Manos Pitsidianakis wrote:
> Clock type in throttling is currently inferred by the ThrottleTimer's
> clock type even though it is a per-ThrottleGroup property; it doesn't
> make sense to have different clock types in the same group. Moving this
> to a field in ThrottleGroup can simplify some of the throttle functions.
> 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  block/throttle-groups.c | 20 ++--
>  fsdev/qemu-fsdev-throttle.c |  2 +-
>  include/qemu/throttle.h |  1 +
>  tests/test-throttle.c   |  4 ++--
>  util/throttle.c |  4 +++-
>  5 files changed, 17 insertions(+), 14 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC v2 0/8] VIRTIO-IOMMU device

[Jean-Philippe Brucker]

On 05/07/17 09:49, Bharat Bhushan wrote:>>> Also when setup msi-route
kvm_irqchip_add_msi_route() we needed to
>> provide the translated address.
>>> According to my understanding this is required because kernel does no go
>> through viommu translation when generating interrupt, no?
>>
>> yes this is needed when KVM MSI routes are set up, ie. along with GICV3 ITS.
>> With GICv2M, qemu direct gsi mapping is used and this is not needed.
>>
>> So I do not understand your previous sentence saying "MSI interrupts works
>> without any change".
> 
> I have almost completed vfio integration with virtio-iommu and now testing 
> the changes by assigning e1000 device to VM. For this I have changed 
> virtio-iommu driver to use IOMMU_RESV_MSI rather than sw-msi and this does 
> not need changed in vfio_get_addr()  and kvm_irqchip_add_msi_route()

I understand you're reserving region 0x0800-0x0810 as
IOMMU_RESV_MSI instead of IOMMU_RESV_SW_MSI? I think this only works
because Qemu places the vgic in that area as well (in hw/arm/virt.c). It's
not a coincidence if the addresses are the same, because Eric chose them
for the Linux SMMU drivers and I copied them.

We can't rely on that behavior, though, it will break MSIs in emulated
devices. And if Qemu happens to move the MSI doorbell in future machine
revisions, then it would also break VFIO.

Just for my own understanding -- what happens, I think, is that in Linux
iova_reserve_iommu_regions initially reserves the guest-physical doorbell
0x0800-0x0810. Then much later, when the device driver requests an
MSI, the irqchip driver calls iommu_dma_map_msi_msg with the
guest-physical gicv2m address 0x0802. The function finds the right
page in msi_page_list, which was added by cookie_init_hw_msi_region,
therefore bypassing the viommu and the GPA gets written in the MSI-X table.

If an emulated device such as virtio-net-pci were to generate an MSI, then
Qemu would attempt to access the doorbell written by Linux into the MSI-X
table, 0x0802, and fault because that address wasn't mapped in the viommu.

So for VFIO, you either need to translate the MSI-X entry using the
viommu, or just assume that the vaddr corresponds to the only MSI doorbell
accessible by this device (because how can we be certain that the guest
already mapped the doorbell before writing the entry?)

For ARM machines it's probably best to stick with IOMMU_RESV_SW_MSI.
However, a nice way to use IOMMU_RESV_MSI would be for the virtio-iommu
device to advertise identity-mapped/reserved regions, and bypass
translation on these regions. Then the driver could reserve those with
IOMMU_RESV_MSI. For x86 we will need such a system, with an added IRQ
remapping feature.

Thanks,
Jean

Re: [Qemu-devel] [PATCH 5/7] kdump: add vmcoreinfo ELF note

[Marc-André Lureau]

Hi

On Wed, Jul 5, 2017 at 2:07 AM, Laszlo Ersek  wrote:
> On 06/29/17 15:23, Marc-André Lureau wrote:
>> kdump header provides offset and size of the vmcoreinfo ELF note,
>> append it if available.
>>
>> Signed-off-by: Marc-André Lureau 
>> ---
>>  dump.c | 48 
>>  1 file changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/dump.c b/dump.c
>> index 8fda5cc1ed..b78bc1fda7 100644
>> --- a/dump.c
>> +++ b/dump.c
>> @@ -788,8 +788,9 @@ static void create_header32(DumpState *s, Error **errp)
>>  uint32_t sub_hdr_size;
>>  uint32_t bitmap_blocks;
>>  uint32_t status = 0;
>> -uint64_t offset_note;
>> +uint64_t offset_note, offset_vmcoreinfo, size_vmcoreinfo = 0;
>>  Error *local_err = NULL;
>> +uint8_t *vmcoreinfo = NULL;
>>
>>  /* write common header, the version of kdump-compressed format is 6th */
>>  size = sizeof(DiskDumpHeader32);
>> @@ -838,7 +839,18 @@ static void create_header32(DumpState *s, Error **errp)
>>  kh->phys_base = cpu_to_dump32(s, s->dump_info.phys_base);
>>  kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
>>
>> -offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
>> +offset_vmcoreinfo = DISKDUMP_HEADER_BLOCKS * block_size + size;
>> +if (s->vmcoreinfo) {
>> +uint64_t hsize, name_size;
>> +
>> +get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size, 
>> &size_vmcoreinfo);
>
> Should we round up "size_vmcoreinfo" as well? (Without the rounding,
> offset_note might become unaligned, plus I simply don't know what
> alignment is expected from kh->size_vmcoreinfo.)

In the last iteration, it only sets the kh->offset/size_vmcoreinfo. I
don't see any alignment requirement in crash.
Dave, any comment?

>
>> +vmcoreinfo =
>> +s->vmcoreinfo + (DIV_ROUND_UP(hsize, 4) + 
>> DIV_ROUND_UP(name_size, 4)) * 4;
>> +kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
>> +kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo);
>> +}
>> +
>> +offset_note = offset_vmcoreinfo + size_vmcoreinfo;
>>  kh->offset_note = cpu_to_dump64(s, offset_note);
>>  kh->note_size = cpu_to_dump32(s, s->note_size);
>>
>> @@ -848,6 +860,14 @@ static void create_header32(DumpState *s, Error **errp)
>>  goto out;
>>  }
>>
>> +if (vmcoreinfo) {
>> +if (write_buffer(s->fd, offset_vmcoreinfo, vmcoreinfo,
>> + size_vmcoreinfo) < 0) {
>> +error_setg(errp, "dump: failed to vmcoreinfo");
>
> The verb "write" is missing from the message.
>
> Same comments for create_header64() below.
>

gone

thanks

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] qemu-thread: Assert locks are initialized before using

[Stefan Hajnoczi]

On Tue, Jul 04, 2017 at 08:23:25PM +0800, Fam Zheng wrote:
> Not all platforms check whether a lock is initialized before used.  In
> particular Linux seems to be more permissive than OSX.
> 
> Check initialization state explicitly in our code to catch such bugs
> earlier.
> 
> Signed-off-by: Fam Zheng 
> ---
>  include/qemu/thread-posix.h |  4 
>  include/qemu/thread-win32.h |  5 +
>  util/qemu-thread-posix.c| 27 +++
>  util/qemu-thread-win32.c| 34 +-
>  4 files changed, 69 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2] hmp: Update info vnc

[Daniel P. Berrange]

On Thu, Jul 06, 2017 at 10:55:45AM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> The QMP query-vnc interfaces have gained a lot more information that
> the HMP interfaces hasn't got yet. Update it.
> 
> Note the output format has changed, but this is HMP so that's OK.
> 
> In particular, this now includes client information for reverse
> connections:
> 
> -vnc :0
> (qemu) info vnc
> default:
>   Server: 0.0.0.0:5900 (ipv4)
> Auth: none (Sub: none)
> 
>   (Now connect a client)
> 
> (qemu) info vnc
> default:
>   Server: 0.0.0.0:5900 (ipv4)
> Auth: none (Sub: none)
>   Client: 127.0.0.1:51828 (ipv4)
> x509_dname: none
> sasl_username: none
> 
> -vnc localhost:7000,reverse
> (qemu) info vnc
> default:
>   Client: ::1:7000 (ipv6)
> x509_dname: none
> sasl_username: none
>   Auth: none (Sub: none)
> 
> -vnc :1,password,id=pass -vnc localhost:7000,reverse
> (qemu) info vnc
> default:
>   Client: ::1:7000 (ipv6)
> x509_dname: none
> sasl_username: none
>   Auth: none (Sub: none)
> rev:
>   Server: 0.0.0.0:5901 (ipv4)
> Auth: vnc (Sub: none)
>   Client: 127.0.0.1:53616 (ipv4)
> x509_dname: none
> sasl_username: none
> 
> This was originally RH bz 1461682
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v2 0/7] KASLR kernel dump support

[Marc-André Lureau]

Recent linux kernels enable KASLR to randomize phys/virt memory
addresses. This series aims to provide enough information in qemu
dumps so that crash utility can work with randomized kernel too (it
hasn't been tested on other archs than x86 though, help welcome).

The vmcoreinfo device is an emulated ACPI device that exposes a 4k
memory range to the guest to store various informations useful to
debug the guest OS. (it is greatly inspired by the VMGENID device
implementation). The version field with value 0 is meant to give
paddr/size of the VMCOREINFO ELF PT_NOTE, other values can be used for
different purposes or OSes. (note: some wanted to see pvpanic somehow
merged with this device, I have no clear idea how to do that, nor do I
think this is a good idea since the devices are quite different, used
at different time for different purposes. And it can be done as a
future iteration if it is appropriate, feel free to send patches)

Crash 7.1.9 will parse the "phys_base" value from the VMCOREINFO note,
and thus will work with KASLR-dump produced by this series.

By priority, VMCOREINFO "phys_base" value is the most accurate. If not
available, qemu will keep the current guessed value.

The series implements the VMCOREINFO note addition in qemu ELF/kdump,
as well as the python scripts/dump-guest-memory.py.

To test:

Compile and run a guest kernel with CONFIG_RANDOMIZE_BASE=y.

Run qemu with -device vmcoreinfo.

Load the experimental vmcoreinfo module in guest
https://github.com/elmarco/vmgenid-test/blob/master/qemuvmci-test.c.

Produce an ELF dump:
{ "execute": "dump-guest-memory", "arguments": { "protocol": "file:dump", 
"paging": false } }

Produce a kdump:
{ "execute": "dump-guest-memory", "arguments": { "protocol": "file:dump", 
"paging": false, "format": "kdump-zlib" } }

Or with (gdb) dump-guest-memory, with scripts/dump-guest-memory.py script.

Analyze with crash >= 7.1.9
$ crash vmlinux dump

v2: from Laszlo review
- vmci: fix guest endianess handling
- vmci: fix wrong sizeof()
- vmci: add back reset logic from vmgenid
- dump: have 1MB size limit for vmcoreinfo
- dump: fix potential off-by-1 buffer manipulation
- dump: use temporary variable for qemu_strtou64
- dump: fixed VMCOREINFO duplication in kdump
- update gdb script to not call into qemu process
- update MAINTAINERS with some new files

Marc-André Lureau (7):
  vmgenid: replace x-write-pointer-available hack
  acpi: add vmcoreinfo device
  tests: add simple vmcoreinfo test
  dump: add vmcoreinfo ELF note
  kdump: add vmcoreinfo ELF note
  scripts/dump-guest-memory.py: add vmcoreinfo
  MAINTAINERS: add Dump maintainers

 scripts/dump-guest-memory.py |  40 +++
 include/hw/acpi/aml-build.h  |   1 +
 include/hw/acpi/bios-linker-loader.h |   2 +
 include/hw/acpi/vmcoreinfo.h |  36 ++
 include/hw/compat.h  |   4 -
 include/sysemu/dump.h|   2 +
 dump.c   | 145 
 hw/acpi/aml-build.c  |   2 +
 hw/acpi/bios-linker-loader.c |  10 ++
 hw/acpi/vmcoreinfo.c | 211 +++
 hw/acpi/vmgenid.c|   9 +-
 hw/i386/acpi-build.c |  14 +++
 tests/vmcoreinfo-test.c  | 130 +
 MAINTAINERS  |   9 ++
 default-configs/arm-softmmu.mak  |   1 +
 default-configs/i386-softmmu.mak |   1 +
 default-configs/x86_64-softmmu.mak   |   1 +
 docs/specs/vmcoreinfo.txt| 138 +++
 hw/acpi/Makefile.objs|   1 +
 tests/Makefile.include   |   2 +
 20 files changed, 747 insertions(+), 12 deletions(-)
 create mode 100644 include/hw/acpi/vmcoreinfo.h
 create mode 100644 hw/acpi/vmcoreinfo.c
 create mode 100644 tests/vmcoreinfo-test.c
 create mode 100644 docs/specs/vmcoreinfo.txt

-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH v2 1/7] vmgenid: replace x-write-pointer-available hack

[Marc-André Lureau]

This compat property sole function is to prevent the device from being
instantiated. Instead of requiring an extra compat property, check if
fw_cfg has DMA enabled.

fw_cfg is a built-in device that is initialized very early by the
machine init code.  We have at least one other device that also
assumes fw_cfg_find() can be safely used on realize: pvpanic.

This has the additional benefit of handling other cases properly, like:

  $ qemu-system-x86_64 -device vmgenid -machine none
  qemu-system-x86_64: -device vmgenid: vmgenid requires DMA write support in 
fw_cfg, which this machine type does not provide
  $ qemu-system-x86_64 -device vmgenid -machine pc-i440fx-2.9 -global 
fw_cfg.dma_enabled=off
  qemu-system-x86_64: -device vmgenid: vmgenid requires DMA write support in 
fw_cfg, which this machine type does not provide
  $ qemu-system-x86_64 -device vmgenid -machine pc-i440fx-2.6 -global 
fw_cfg.dma_enabled=on
  [boots normally]

Suggested-by: Eduardo Habkost 
Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Ben Warren 
---
 include/hw/acpi/bios-linker-loader.h |  2 ++
 include/hw/compat.h  |  4 
 hw/acpi/bios-linker-loader.c | 10 ++
 hw/acpi/vmgenid.c|  9 +
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/include/hw/acpi/bios-linker-loader.h 
b/include/hw/acpi/bios-linker-loader.h
index efe17b0b9c..a711dbced8 100644
--- a/include/hw/acpi/bios-linker-loader.h
+++ b/include/hw/acpi/bios-linker-loader.h
@@ -7,6 +7,8 @@ typedef struct BIOSLinker {
 GArray *file_list;
 } BIOSLinker;
 
+bool bios_linker_loader_can_write_pointer(void);
+
 BIOSLinker *bios_linker_loader_init(void);
 
 void bios_linker_loader_alloc(BIOSLinker *linker,
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 08f36004da..f414786604 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -150,10 +150,6 @@
 .driver   = "fw_cfg_io",\
 .property = "dma_enabled",\
 .value= "off",\
-},{\
-.driver   = "vmgenid",\
-.property = "x-write-pointer-available",\
-.value= "off",\
 },
 
 #define HW_COMPAT_2_3 \
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
index 046183a0f1..388d932538 100644
--- a/hw/acpi/bios-linker-loader.c
+++ b/hw/acpi/bios-linker-loader.c
@@ -168,6 +168,16 @@ bios_linker_find_file(const BIOSLinker *linker, const char 
*name)
 return NULL;
 }
 
+/*
+ * board code must realize fw_cfg first, as a fixed device, before
+ * another device realize function call bios_linker_loader_can_write_pointer()
+*/
+bool bios_linker_loader_can_write_pointer(void)
+{
+FWCfgState *fw_cfg = fw_cfg_find();
+return fw_cfg && fw_cfg_dma_enabled(fw_cfg);
+}
+
 /*
  * bios_linker_loader_alloc: ask guest to load file into guest memory.
  *
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
index a32b847fe0..ab5da293fd 100644
--- a/hw/acpi/vmgenid.c
+++ b/hw/acpi/vmgenid.c
@@ -205,17 +205,11 @@ static void vmgenid_handle_reset(void *opaque)
 memset(vms->vmgenid_addr_le, 0, ARRAY_SIZE(vms->vmgenid_addr_le));
 }
 
-static Property vmgenid_properties[] = {
-DEFINE_PROP_BOOL("x-write-pointer-available", VmGenIdState,
- write_pointer_available, true),
-DEFINE_PROP_END_OF_LIST(),
-};
-
 static void vmgenid_realize(DeviceState *dev, Error **errp)
 {
 VmGenIdState *vms = VMGENID(dev);
 
-if (!vms->write_pointer_available) {
+if (!bios_linker_loader_can_write_pointer()) {
 error_setg(errp, "%s requires DMA write support in fw_cfg, "
"which this machine type does not provide", VMGENID_DEVICE);
 return;
@@ -239,7 +233,6 @@ static void vmgenid_device_class_init(ObjectClass *klass, 
void *data)
 dc->vmsd = &vmstate_vmgenid;
 dc->realize = vmgenid_realize;
 dc->hotpluggable = false;
-dc->props = vmgenid_properties;
 
 object_class_property_add_str(klass, VMGENID_GUID, NULL,
   vmgenid_set_guid, NULL);
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH v2 7/7] MAINTAINERS: add Dump maintainers

[Marc-André Lureau]

Proposing myself, since I have some familiarity with the code now.

Signed-off-by: Marc-André Lureau 
---
 MAINTAINERS | 9 +
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 839f7ca063..ba17ce5b85 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1272,6 +1272,15 @@ S: Maintained
 F: device_tree.c
 F: include/sysemu/device_tree.h
 
+Dump
+S: Supported
+M: Marc-André Lureau 
+F: dump.c
+F: stubs/dump.c
+F: include/sysemu/dump.h
+F: include/sysemu/dump-arch.h
+F: scripts/dump-guest-memory.py
+
 Error reporting
 M: Markus Armbruster 
 S: Supported
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH v2 4/7] dump: add vmcoreinfo ELF note

[Marc-André Lureau]

Read the vmcoreinfo ELF PT_NOTE from guest memory when vmcoreinfo
device provides the location, and write it as an ELF note in the dump.

There are now 2 possible sources of phys_base information.

(1) arch guessed value from cpu_dump_info_get()
(2) vmcoreinfo ELF note NUMBER(phys_base)= field

NUMBER(phys_base) in vmcoreinfo has only been recently introduced
in Linux 4.10 (401721ecd1dc "kexec: export the value of phys_base
instead of symbol address").

Since (2) has better chances to be accurate, the guessed value is
replaced by the value from the vmcoreinfo ELF note.

The phys_base value is stored in the same dump field locations as
before, and may duplicate the information available in the vmcoreinfo
ELF PT_NOTE. Crash tools should be prepared to handle this case.

Signed-off-by: Marc-André Lureau 
---
 include/sysemu/dump.h |   2 +
 dump.c| 125 ++
 2 files changed, 127 insertions(+)

diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 2672a15f8b..111a7dcaa4 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -192,6 +192,8 @@ typedef struct DumpState {
   * this could be used to calculate
   * how much work we have
   * finished. */
+uint8_t *vmcoreinfo; /* ELF note content */
+size_t vmcoreinfo_size;
 } DumpState;
 
 uint16_t cpu_to_dump16(DumpState *s, uint16_t val);
diff --git a/dump.c b/dump.c
index d9090a24cc..f699198204 100644
--- a/dump.c
+++ b/dump.c
@@ -26,6 +26,8 @@
 #include "qapi/qmp/qerror.h"
 #include "qmp-commands.h"
 #include "qapi-event.h"
+#include "qemu/error-report.h"
+#include "hw/acpi/vmcoreinfo.h"
 
 #include 
 #ifdef CONFIG_LZO
@@ -38,6 +40,11 @@
 #define ELF_MACHINE_UNAME "Unknown"
 #endif
 
+#define ELF_NOTE_SIZE(hdr_size, name_size, desc_size)   \
+((DIV_ROUND_UP((hdr_size), 4) + \
+  DIV_ROUND_UP((name_size), 4) +\
+  DIV_ROUND_UP((desc_size), 4)) * 4)
+
 uint16_t cpu_to_dump16(DumpState *s, uint16_t val)
 {
 if (s->dump_info.d_endian == ELFDATA2LSB) {
@@ -76,6 +83,8 @@ static int dump_cleanup(DumpState *s)
 guest_phys_blocks_free(&s->guest_phys_blocks);
 memory_mapping_list_free(&s->list);
 close(s->fd);
+g_free(s->vmcoreinfo);
+s->vmcoreinfo = NULL;
 if (s->resume) {
 if (s->detached) {
 qemu_mutex_lock_iothread();
@@ -235,6 +244,19 @@ static inline int cpu_index(CPUState *cpu)
 return cpu->cpu_index + 1;
 }
 
+static void write_vmcoreinfo_note(WriteCoreDumpFunction f, DumpState *s,
+  Error **errp)
+{
+int ret;
+
+if (s->vmcoreinfo) {
+ret = f(s->vmcoreinfo, s->vmcoreinfo_size, s);
+if (ret < 0) {
+error_setg(errp, "dump: failed to write vmcoreinfo");
+}
+}
+}
+
 static void write_elf64_notes(WriteCoreDumpFunction f, DumpState *s,
   Error **errp)
 {
@@ -258,6 +280,8 @@ static void write_elf64_notes(WriteCoreDumpFunction f, 
DumpState *s,
 return;
 }
 }
+
+write_vmcoreinfo_note(f, s, errp);
 }
 
 static void write_elf32_note(DumpState *s, Error **errp)
@@ -303,6 +327,8 @@ static void write_elf32_notes(WriteCoreDumpFunction f, 
DumpState *s,
 return;
 }
 }
+
+write_vmcoreinfo_note(f, s, errp);
 }
 
 static void write_elf_section(DumpState *s, int type, Error **errp)
@@ -714,6 +740,44 @@ static int buf_write_note(const void *buf, size_t size, 
void *opaque)
 return 0;
 }
 
+/*
+ * This function retrieves various sizes from an elf header.
+ *
+ * @note has to be a valid ELF note. The return sizes are unmodified
+ * (not padded or rounded up to be multiple of 4).
+ */
+static void get_note_sizes(DumpState *s, const void *note,
+   uint64_t *note_head_size,
+   uint64_t *name_size,
+   uint64_t *desc_size)
+{
+uint64_t note_head_sz;
+uint64_t name_sz;
+uint64_t desc_sz;
+
+if (s->dump_info.d_class == ELFCLASS64) {
+const Elf64_Nhdr *hdr = note;
+note_head_sz = sizeof(Elf64_Nhdr);
+name_sz = tswap64(hdr->n_namesz);
+desc_sz = tswap64(hdr->n_descsz);
+} else {
+const Elf32_Nhdr *hdr = note;
+note_head_sz = sizeof(Elf32_Nhdr);
+name_sz = tswap32(hdr->n_namesz);
+desc_sz = tswap32(hdr->n_descsz);
+}
+
+if (note_head_size) {
+*note_head_size = note_head_sz;
+}
+if (name_size) {
+*name_size = name_sz;
+}
+if (desc_size) {
+*desc_size = desc_sz;
+}
+}
+
 /* write common header, sub header and elf note to vmcore */
 static void create_header32(DumpState *s, Error **errp)
 {
@@ -1488,10 +1552,40 @@ static int64_t dump_calculate_size(DumpState *s)
 return total;
 }
 
+static void vmcoreinfo_update_phys

[Qemu-devel] [PATCH v2 5/7] kdump: add vmcoreinfo ELF note

[Marc-André Lureau]

kdump header provides offset and size of the vmcoreinfo ELF note,
append it if available.

Signed-off-by: Marc-André Lureau 
---
 dump.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/dump.c b/dump.c
index f699198204..dd416ad271 100644
--- a/dump.c
+++ b/dump.c
@@ -839,6 +839,16 @@ static void create_header32(DumpState *s, Error **errp)
 kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
 
 offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
+if (s->vmcoreinfo) {
+uint64_t hsize, name_size, size_vmcoreinfo_desc, offset_vmcoreinfo;
+
+get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size, 
&size_vmcoreinfo_desc);
+offset_vmcoreinfo = offset_note + s->note_size - s->vmcoreinfo_size +
+(DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
+kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
+kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo_desc);
+}
+
 kh->offset_note = cpu_to_dump64(s, offset_note);
 kh->note_size = cpu_to_dump32(s, s->note_size);
 
@@ -939,6 +949,16 @@ static void create_header64(DumpState *s, Error **errp)
 kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
 
 offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
+if (s->vmcoreinfo) {
+uint64_t hsize, name_size, size_vmcoreinfo_desc, offset_vmcoreinfo;
+
+get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size, 
&size_vmcoreinfo_desc);
+offset_vmcoreinfo = offset_note + s->note_size - s->vmcoreinfo_size +
+(DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
+kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
+kh->size_vmcoreinfo = cpu_to_dump64(s, size_vmcoreinfo_desc);
+}
+
 kh->offset_note = cpu_to_dump64(s, offset_note);
 kh->note_size = cpu_to_dump64(s, s->note_size);
 
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH v2 2/7] acpi: add vmcoreinfo device

[Marc-André Lureau]

The VM coreinfo (vmcoreinfo) device is an emulated device which
exposes a 4k memory range to the guest to store various informations
useful to debug the guest OS. (it is greatly inspired by the VMGENID
device implementation)

This is an early-boot alternative to the qemu-ga VMDUMP_INFO event
proposed in "[PATCH 00/21] WIP: dump: add kaslr support".

A proof-of-concept kernel module:
https://github.com/elmarco/vmgenid-test/blob/master/qemuvmci-test.c

Signed-off-by: Marc-André Lureau 
---
 include/hw/acpi/aml-build.h|   1 +
 include/hw/acpi/vmcoreinfo.h   |  36 +++
 hw/acpi/aml-build.c|   2 +
 hw/acpi/vmcoreinfo.c   | 208 +
 hw/i386/acpi-build.c   |  14 +++
 default-configs/arm-softmmu.mak|   1 +
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 docs/specs/vmcoreinfo.txt  | 138 
 hw/acpi/Makefile.objs  |   1 +
 10 files changed, 403 insertions(+)
 create mode 100644 include/hw/acpi/vmcoreinfo.h
 create mode 100644 hw/acpi/vmcoreinfo.c
 create mode 100644 docs/specs/vmcoreinfo.txt

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 88d0738d76..cf781bcd34 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -211,6 +211,7 @@ struct AcpiBuildTables {
 GArray *rsdp;
 GArray *tcpalog;
 GArray *vmgenid;
+GArray *vmcoreinfo;
 BIOSLinker *linker;
 } AcpiBuildTables;
 
diff --git a/include/hw/acpi/vmcoreinfo.h b/include/hw/acpi/vmcoreinfo.h
new file mode 100644
index 00..9f72a1f263
--- /dev/null
+++ b/include/hw/acpi/vmcoreinfo.h
@@ -0,0 +1,36 @@
+#ifndef ACPI_VMCOREINFO_H
+#define ACPI_VMCOREINFO_H
+
+#include "hw/acpi/bios-linker-loader.h"
+#include "hw/qdev.h"
+
+#define VMCOREINFO_DEVICE   "vmcoreinfo"
+#define VMCOREINFO_FW_CFG_FILE  "etc/vmcoreinfo"
+#define VMCOREINFO_ADDR_FW_CFG_FILE "etc/vmcoreinfo-addr"
+
+#define VMCOREINFO_FW_CFG_SIZE  4096 /* Occupy a page of memory */
+#define VMCOREINFO_OFFSET   40   /* allow space for
+  * OVMF SDT Header Probe Supressor
+  */
+
+#define VMCOREINFO(obj) OBJECT_CHECK(VMCoreInfoState, (obj), VMCOREINFO_DEVICE)
+
+typedef struct VMCoreInfoState {
+DeviceClass parent_obj;
+uint8_t vmcoreinfo_addr_le[8];   /* Address of memory region */
+bool write_pointer_available;
+} VMCoreInfoState;
+
+/* returns NULL unless there is exactly one device */
+static inline Object *find_vmcoreinfo_dev(void)
+{
+return object_resolve_path_type("", VMCOREINFO_DEVICE, NULL);
+}
+
+void vmcoreinfo_build_acpi(VMCoreInfoState *vis, GArray *table_data,
+   GArray *vmci, BIOSLinker *linker);
+void vmcoreinfo_add_fw_cfg(VMCoreInfoState *vis, FWCfgState *s, GArray *vmci);
+bool vmcoreinfo_get(VMCoreInfoState *vis, uint64_t *paddr, uint32_t *size,
+Error **errp);
+
+#endif
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 36a6cc450e..47043ade4a 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1561,6 +1561,7 @@ void acpi_build_tables_init(AcpiBuildTables *tables)
 tables->table_data = g_array_new(false, true /* clear */, 1);
 tables->tcpalog = g_array_new(false, true /* clear */, 1);
 tables->vmgenid = g_array_new(false, true /* clear */, 1);
+tables->vmcoreinfo = g_array_new(false, true /* clear */, 1);
 tables->linker = bios_linker_loader_init();
 }
 
@@ -1571,6 +1572,7 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, 
bool mfre)
 g_array_free(tables->table_data, true);
 g_array_free(tables->tcpalog, mfre);
 g_array_free(tables->vmgenid, mfre);
+g_array_free(tables->vmcoreinfo, mfre);
 }
 
 /* Build rsdt table */
diff --git a/hw/acpi/vmcoreinfo.c b/hw/acpi/vmcoreinfo.c
new file mode 100644
index 00..0ea41de8d9
--- /dev/null
+++ b/hw/acpi/vmcoreinfo.c
@@ -0,0 +1,208 @@
+/*
+ *  Virtual Machine coreinfo device
+ *  (based on Virtual Machine Generation ID Device)
+ *
+ *  Copyright (C) 2017 Red Hat, Inc.
+ *  Copyright (C) 2017 Skyport Systems.
+ *
+ *  Authors: Marc-André Lureau 
+ *   Ben Warren 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include "qemu/osdep.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/vmcoreinfo.h"
+#include "hw/nvram/fw_cfg.h"
+#include "sysemu/sysemu.h"
+#include "qapi/error.h"
+
+void vmcoreinfo_build_acpi(VMCoreInfoState *vis, GArray *table_data,
+   GArray *vmci, BIOSLinker *linker)
+{
+Aml *ssdt, *dev, *scope, *method, *addr, *if_ctx;
+uint32_t vcia_offset;
+
+g_array_set_size(vmci, VMCOREINFO_FW_CFG_SIZE);
+
+/* Put this in a separate SSDT table */
+ssdt = init_aml_allocator();
+
+/* Reserve s

[Qemu-devel] [PATCH v2 3/7] tests: add simple vmcoreinfo test

[Marc-André Lureau]

This test is based off vmgenid test from Ben Warren
. It simply checks the vmcoreinfo ACPI device
is present and that the memory region associated can be read.

Signed-off-by: Marc-André Lureau 
---
 tests/vmcoreinfo-test.c | 130 
 tests/Makefile.include  |   2 +
 2 files changed, 132 insertions(+)
 create mode 100644 tests/vmcoreinfo-test.c

diff --git a/tests/vmcoreinfo-test.c b/tests/vmcoreinfo-test.c
new file mode 100644
index 00..c8b073366e
--- /dev/null
+++ b/tests/vmcoreinfo-test.c
@@ -0,0 +1,130 @@
+/*
+ * QTest testcase for VM coreinfo device
+ *
+ * Copyright (c) 2017 Red Hat, Inc.
+ * Copyright (c) 2017 Skyport Systems
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include 
+#include 
+#include 
+#include "qemu/osdep.h"
+#include "qemu/bitmap.h"
+#include "qemu/uuid.h"
+#include "hw/acpi/acpi-defs.h"
+#include "acpi-utils.h"
+#include "libqtest.h"
+
+#define VMCOREINFO_OFFSET 40 /* allow space for
+  * OVMF SDT Header Probe Supressor
+  */
+#define RSDP_ADDR_INVALID 0x10 /* RSDP must be below this address */
+#define RSDP_SLEEP_US 10   /* Sleep for 100ms between tries */
+#define RSDP_TRIES_MAX100  /* Max total time is 10 seconds */
+
+typedef struct {
+AcpiTableHeader header;
+gchar name_op;
+gchar vcia[4];
+gchar val_op;
+uint32_t vcia_val;
+} QEMU_PACKED VgidTable;
+
+static uint32_t acpi_find_vcia(void)
+{
+uint32_t off;
+AcpiRsdpDescriptor rsdp_table;
+uint32_t rsdt;
+AcpiRsdtDescriptorRev1 rsdt_table;
+int tables_nr;
+uint32_t *tables;
+AcpiTableHeader ssdt_table;
+VgidTable vgid_table;
+int i;
+
+/* Tables may take a short time to be set up by the guest */
+for (i = 0; i < RSDP_TRIES_MAX; i++) {
+off = acpi_find_rsdp_address();
+if (off < RSDP_ADDR_INVALID) {
+break;
+}
+g_usleep(RSDP_SLEEP_US);
+}
+g_assert_cmphex(off, <, RSDP_ADDR_INVALID);
+
+acpi_parse_rsdp_table(off, &rsdp_table);
+
+rsdt = rsdp_table.rsdt_physical_address;
+/* read the header */
+ACPI_READ_TABLE_HEADER(&rsdt_table, rsdt);
+ACPI_ASSERT_CMP(rsdt_table.signature, "RSDT");
+
+/* compute the table entries in rsdt */
+tables_nr = (rsdt_table.length - sizeof(AcpiRsdtDescriptorRev1)) /
+sizeof(uint32_t);
+g_assert_cmpint(tables_nr, >, 0);
+
+/* get the addresses of the tables pointed by rsdt */
+tables = g_new0(uint32_t, tables_nr);
+ACPI_READ_ARRAY_PTR(tables, tables_nr, rsdt);
+
+for (i = 0; i < tables_nr; i++) {
+ACPI_READ_TABLE_HEADER(&ssdt_table, tables[i]);
+if (!strncmp((char *)ssdt_table.oem_table_id, "VMCOREIN", 8)) {
+/* the first entry in the table should be VCIA
+ * That's all we need
+ */
+ACPI_READ_FIELD(vgid_table.name_op, tables[i]);
+g_assert(vgid_table.name_op == 0x08);  /* name */
+ACPI_READ_ARRAY(vgid_table.vcia, tables[i]);
+g_assert(memcmp(vgid_table.vcia, "VCIA", 4) == 0);
+ACPI_READ_FIELD(vgid_table.val_op, tables[i]);
+g_assert(vgid_table.val_op == 0x0C);  /* dword */
+ACPI_READ_FIELD(vgid_table.vcia_val, tables[i]);
+/* The GUID is written at a fixed offset into the fw_cfg file
+ * in order to implement the "OVMF SDT Header probe suppressor"
+ * see docs/specs/vmgenid.txt for more details
+ */
+g_free(tables);
+return vgid_table.vcia_val;
+}
+}
+g_free(tables);
+return 0;
+}
+
+static void vmcoreinfo_test(void)
+{
+gchar *cmd;
+uint32_t vmci_addr;
+int i;
+
+cmd = g_strdup_printf("-machine accel=tcg -device vmcoreinfo,id=vmci");
+qtest_start(cmd);
+
+vmci_addr = acpi_find_vcia();
+g_assert(vmci_addr);
+
+for (i = 0; i < 4096; i++) {
+/* check the memory region can be read */
+readb(vmci_addr + i);
+}
+
+qtest_quit(global_qtest);
+g_free(cmd);
+}
+
+int main(int argc, char **argv)
+{
+int ret;
+
+g_test_init(&argc, &argv, NULL);
+
+qtest_add_func("/vmcoreinfo/test", vmcoreinfo_test);
+ret = g_test_run();
+
+return ret;
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 18cd06a6b3..f573c1d3c8 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -251,6 +251,7 @@ gcov-files-i386-y += hw/usb/hcd-xhci.c
 check-qtest-i386-y += tests/pc-cpu-test$(EXESUF)
 check-qtest-i386-y += tests/q35-test$(EXESUF)
 check-qtest-i386-y += tests/vmgenid-test$(EXESUF)
+check-qtest-i386-y += tests/vmcoreinfo-test$(EXESUF)
 gcov-files-i386-y += hw/pci-host/q35.c
 check-qtest-i386-$(CONFIG_VHOST_NET_TEST_i386) += 
tests/vhost-user-test$(EXESUF)
 ifeq ($(CONFIG_VHOST_NET_TEST_i38

[Qemu-devel] [PATCH v2 6/7] scripts/dump-guest-memory.py: add vmcoreinfo

[Marc-André Lureau]

Add vmcoreinfo ELF note if vmcoreinfo device is ready.

To help the python script, add a little global vmcoreinfo_gdb
structure, that is populated with vmcoreinfo_gdb_update().

Signed-off-by: Marc-André Lureau 
---
 scripts/dump-guest-memory.py | 40 
 hw/acpi/vmcoreinfo.c |  3 +++
 2 files changed, 43 insertions(+)

diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index f7c6635f15..2dd2ed6983 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -14,6 +14,7 @@ the COPYING file in the top-level directory.
 """
 
 import ctypes
+import struct
 
 UINTPTR_T = gdb.lookup_type("uintptr_t")
 
@@ -120,6 +121,20 @@ class ELF(object):
 self.segments[0].p_filesz += ctypes.sizeof(note)
 self.segments[0].p_memsz += ctypes.sizeof(note)
 
+
+def add_vmcoreinfo_note(self, vmcoreinfo):
+"""Adds a vmcoreinfo note to the ELF dump."""
+chead = type(get_arch_note(self.endianness, 0, 0))
+header = chead.from_buffer_copy(vmcoreinfo[0:ctypes.sizeof(chead)])
+note = get_arch_note(self.endianness,
+ header.n_namesz - 1, header.n_descsz)
+ctypes.memmove(ctypes.pointer(note), vmcoreinfo, ctypes.sizeof(note))
+header_size = ctypes.sizeof(note) - header.n_descsz
+
+self.notes.append(note)
+self.segments[0].p_filesz += ctypes.sizeof(note)
+self.segments[0].p_memsz += ctypes.sizeof(note)
+
 def add_segment(self, p_type, p_paddr, p_size):
 """Adds a segment to the elf."""
 
@@ -505,6 +520,30 @@ shape and this command should mostly work."""
 cur += chunk_size
 left -= chunk_size
 
+def phys_memory_read(self, addr, size):
+qemu_core = gdb.inferiors()[0]
+for block in self.guest_phys_blocks:
+if block["target_start"] <= addr < block["target_end"]:
+haddr = block["host_addr"] + (addr - block["target_start"])
+return qemu_core.read_memory(haddr, size)
+
+def add_vmcoreinfo(self):
+if not gdb.parse_and_eval("vmcoreinfo_gdb_helper"):
+return
+
+addr = gdb.parse_and_eval("vmcoreinfo_gdb_helper.vmcoreinfo_addr_le")
+addr = bytes([addr[i] for i in range(4)])
+addr = struct.unpack("vmcoreinfo_addr_le, 0, ARRAY_SIZE(vis->vmcoreinfo_addr_le));
 }
 
+static VMCoreInfoState *vmcoreinfo_gdb_helper;
+
 static void vmcoreinfo_realize(DeviceState *dev, Error **errp)
 {
 if (!bios_linker_loader_can_write_pointer()) {
@@ -181,6 +183,7 @@ static void vmcoreinfo_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+vmcoreinfo_gdb_helper = VMCOREINFO(dev);
 qemu_register_reset(vmcoreinfo_handle_reset, dev);
 }
 
-- 
2.13.1.395.gf7b71de06

Re: [Qemu-devel] [PATCH 7/7] MAINTAINERS: add Dump maintainers

[Laszlo Ersek]

On 07/06/17 11:54, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
>> On 06/29/17 15:23, Marc-André Lureau wrote:
>>> Proposing myself, since I have some familiarity with the code now.
>>>
>>> Signed-off-by: Marc-André Lureau 
>>> ---
>>>  MAINTAINERS | 7 +++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 839f7ca063..45a0eb4cb0 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1272,6 +1272,13 @@ S: Maintained
>>>  F: device_tree.c
>>>  F: include/sysemu/device_tree.h
>>>  
>>> +Dump
>>> +S: Supported
>>> +M: Marc-André Lureau 
>>> +F: dump.c
>>> +F: include/sysemu/dump.h
>>> +F: include/sysemu/dump-arch.h
>>> +
>>>  Error reporting
>>>  M: Markus Armbruster 
>>>  S: Supported
>>>
>>
>> That's very kind of you, thanks!
>>
>> Do you have suggestions for the following files?
>>
>> scripts/dump-guest-memory.py
> 
> This one is yours, no? :) But I am ok to "support" it, meaning I'll take time 
> to review the patches, and eventually make the pull-requests.

It used to be "mine" until (a) it got rewritten in object-oriented
Python, and (b) it received multi-arch support ;)

>  
>> stubs/dump.c
> 
> I can add that one, although it is also maintained by Paolo
> 
>> target/arm/arch_dump.c
>> target/i386/arch_dump.c
>> target/ppc/arch_dump.c
>> target/s390x/arch_dump.c
> 
> I'd rather have those maintained by the respective arch maintainers, as they 
> are. But I imagine it would make sense to also cover them with the rest of 
> dump.

Yeah there's an overlap here -- I'm not suggesting that you take on
everything, just curious what you think. I'm fine with this patch as it is.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH 4/7] dump: add vmcoreinfo ELF note

[Laszlo Ersek]

On 07/05/17 23:52, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 5, 2017 at 1:48 AM, Laszlo Ersek  wrote:
>> On 06/29/17 15:23, Marc-André Lureau wrote:

>>> @@ -258,6 +280,8 @@ static void write_elf64_notes(WriteCoreDumpFunction f, 
>>> DumpState *s,
>>>  return;
>>>  }
>>>  }
>>> +
>>> +write_vmcoreinfo_note(f, s, errp);
>>>  }
>>>
>>>  static void write_elf32_note(DumpState *s, Error **errp)
>>> @@ -303,6 +327,8 @@ static void write_elf32_notes(WriteCoreDumpFunction f, 
>>> DumpState *s,
>>>  return;
>>>  }
>>>  }
>>> +
>>> +write_vmcoreinfo_note(f, s, errp);
>>>  }
>>>
>>
>> Wait, I'm confused again. You explained why it was OK to hook this logic
>> into the kdump handling too, but I don't think I understand your
>> explanation, so let me repeat my confusion below :)
>>
>> In the ELF case, this code works fine, I think. As long as the guest
>> provided us with a well-formed note, a well-formed note will be appended
>> to the ELF dump.
>>
>> But, this code is also invoked in the kdump case, and I don't understand
>> why that's a good thing. If I understand the next patch correctly, the
>> kdump format already provides crash with a (trimmed) copy of the guest
>> kernels vmcoreinfo note. So in the kdump case, why do we have to create
>> yet another copy of the guest kernel's vmcoreinfo note?
>>
>> Thus, my confusion persists, and I can only think (again) that
>> write_vmcoreinfo_note() should be called from dump_begin() only (at the
>> end). (And the s->note_size adjustment should take that into account.)
>>
>> Alternatively, we should keep this logic, and drop patch #5.
> 
> You are right, sorry for my misunderstanding, although crash is seems
> fine as long as size/offsets are ok.
> 
> So, instead of duplicating the info, let's keep the complete ELF note
> (with the header) in the dump, and simply adjust kdump header to point
> to the adjusted offset/size. This has the advantage of not making the
> code unnecessarily more complicated wrt s->note_size handling etc, and
> is consistent with the rest of the elf notes.

I guess that's OK, although I don't really see why even that is
necessary. The vmcoreinfo note should be possible to find in the kdump
output with the rest of the ELF notes, even without pointing some kdump
header fields into that specific note.

Snipping the rest because I'm going to see updates on those things in v2
anyway (which you've just posted).

Thanks
Laszlo

Re: [Qemu-devel] change x86 default machine type to Q35?

[Stefan Hajnoczi]

On Wed, Jul 05, 2017 at 12:07:57PM +0100, Daniel P. Berrange wrote:
> On Wed, Jul 05, 2017 at 10:14:23AM +0200, Thomas Huth wrote:
> >  Hi,
> > 
> > On 05.07.2017 08:57, Chao Peng wrote:
> > > 
> > > Q35 has been in QEMU for quite a while. Compared to the current default
> > > i440FX, Q35 is probably not that mature and not widely used, however in
> > > some case, Q35 has advantages, for example, in supporting new features.
> > > For instance, we have some features require PCI-e support which is only
> > > available on Q35 and some others need it for EFI support. It is of
> > > course not necessary to change it as the default but if more and more
> > > features have dependencies on Q35 because of requiring much more modern
> > > features then I think it may be worth to do so. In such case we can have
> > > more people to use it and find problems we may know or not know.
> > 
> > Yes, IMHO at one point in time, we should switch the default machine
> > type to q35. The i440FX is really quite old...
> > 
> > > There are certainly some drawbacks:
> > > -Compatibility: current code or script may need adjustment
> > 
> > That might be a real concern ... so I think a good point in time to
> > switch to the q35 machine type would be when we switch to the next major
> > version number of QEMU, i.e. when we switch to version 3.0. If the users
> > see a new major version number, they might be more willing to accept
> > such major changes (yeah, I know, we've discussed in the past that
> > version numbers are just numbers ... but still, there is some kind of
> > psychological aspect to this, too, I think)
> 
> Most users aren't even aware of version numbers of QEMU - they'll just
> take whatever their distro has provided run it. The notion that their
> latest distro version happened to pull in a "major" version instead of
> previously pulling in a "minor" version is invisible to everyone,
> except the minority of people who care about the low level details.

When user-visible changes break existing setups it's common for
packagers to ship a new family of packages that includes the major
version number (e.g. apache2, postgresql-9.6).

The last QEMU 2.x version would be considered stable and still available
from package repos.  Only critical bug and security fixes could be
released for another, say, 2 years.

This way users don't have to migrate to QEMU 3.x until they are ready.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v4 04/21] stream: Drop reached_end for stream_complete()

[Kevin Wolf]

Am 05.07.2017 um 23:08 hat Eric Blake geschrieben:
> stream_complete() skips the work of rewriting the backing file if
> the job was cancelled, if data->reached_end is false, or if there
> was an error detected (non-zero data->ret) during the streaming.
> But note that in stream_run(), data->reached_end is only set if the
> loop ran to completion, and data->ret is only 0 in two cases:
> either the loop ran to completion (possibly by cancellation, but
> stream_complete checks for that), or we took an early goto out
> because there is no bs->backing.  Thus, we can preserve the same
> semantics without the use of reached_end, by merely checking for
> bs->backing (and logically, if there was no backing file, streaming
> is a no-op, so there is no backing file to rewrite).
> 
> Suggested-by: Kevin Wolf 
> Signed-off-by: Eric Blake 

Reviewed-by: Kevin Wolf 

Re: [Qemu-devel] [PATCH v4 05/21] stream: Switch stream_run() to byte-based

[Kevin Wolf]

Am 05.07.2017 um 23:08 hat Eric Blake geschrieben:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Change the internal
> loop iteration of streaming to track by bytes instead of sectors
> (although we are still guaranteed that we iterate by steps that
> are sector-aligned).
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: John Snow 
> Reviewed-by: Jeff Cody 

Reviewed-by: Kevin Wolf 

Re: [Qemu-devel] change x86 default machine type to Q35?

[Daniel P. Berrange]

On Thu, Jul 06, 2017 at 11:30:43AM +0100, Stefan Hajnoczi wrote:
> On Wed, Jul 05, 2017 at 12:07:57PM +0100, Daniel P. Berrange wrote:
> > On Wed, Jul 05, 2017 at 10:14:23AM +0200, Thomas Huth wrote:
> > >  Hi,
> > > 
> > > On 05.07.2017 08:57, Chao Peng wrote:
> > > > 
> > > > Q35 has been in QEMU for quite a while. Compared to the current default
> > > > i440FX, Q35 is probably not that mature and not widely used, however in
> > > > some case, Q35 has advantages, for example, in supporting new features.
> > > > For instance, we have some features require PCI-e support which is only
> > > > available on Q35 and some others need it for EFI support. It is of
> > > > course not necessary to change it as the default but if more and more
> > > > features have dependencies on Q35 because of requiring much more modern
> > > > features then I think it may be worth to do so. In such case we can have
> > > > more people to use it and find problems we may know or not know.
> > > 
> > > Yes, IMHO at one point in time, we should switch the default machine
> > > type to q35. The i440FX is really quite old...
> > > 
> > > > There are certainly some drawbacks:
> > > > -Compatibility: current code or script may need adjustment
> > > 
> > > That might be a real concern ... so I think a good point in time to
> > > switch to the q35 machine type would be when we switch to the next major
> > > version number of QEMU, i.e. when we switch to version 3.0. If the users
> > > see a new major version number, they might be more willing to accept
> > > such major changes (yeah, I know, we've discussed in the past that
> > > version numbers are just numbers ... but still, there is some kind of
> > > psychological aspect to this, too, I think)
> > 
> > Most users aren't even aware of version numbers of QEMU - they'll just
> > take whatever their distro has provided run it. The notion that their
> > latest distro version happened to pull in a "major" version instead of
> > previously pulling in a "minor" version is invisible to everyone,
> > except the minority of people who care about the low level details.
> 
> When user-visible changes break existing setups it's common for
> packagers to ship a new family of packages that includes the major
> version number (e.g. apache2, postgresql-9.6).
> 
> The last QEMU 2.x version would be considered stable and still available
> from package repos.  Only critical bug and security fixes could be
> released for another, say, 2 years.
> 
> This way users don't have to migrate to QEMU 3.x until they are ready.

Given the number of security vulnerabilities packagers have to deal with
for QEMU, maintaining multiple QEMU streams in parallel is a pretty
major undertaking. I can't say that would be appealing from a Fedora
maintainer POV - it'd likely just end up shipping only the newest version
to avoid that extra maint burden. I doubt enterprise distros would ship
both versions in parallel in this way [1].

Even if both versions are available, the people affected by the breakage
still need to figure out that the breakage they're seeing is caused by
the change in machine type, and that installing the old version will
fix it. This is still an unpleasant experiance IMHO. 

Regards,
Daniel

[1] RHEL does already ship 2 QEMU streams in parallel, but they are
differentiated for other reasons, one being a feature limited
version of the other, and not as frequently rebased.
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v4 08/21] mirror: Switch MirrorBlockJob to byte-based

[Kevin Wolf]

Am 05.07.2017 um 23:08 hat Eric Blake geschrieben:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Continue by converting an
> internal structure (no semantic change), and all references to the
> buffer size.
> 
> Add an assertion that our use of s->granularity >> BDRV_SECTOR_BITS
> (necessary for interaction with sector-based dirty bitmaps, until
> a later patch converts those to be byte-based) does not suffer from
> truncation problems.
> 
> [checkpatch has a false positive on use of MIN() in this patch]
> 
> Signed-off-by: Eric Blake 

Reviewed-by: Kevin Wolf 

Re: [Qemu-devel] [PULL 00/42] Misc changes for 2017-07-05

[Peter Maydell]

On 5 July 2017 at 08:14, Paolo Bonzini  wrote:
> The following changes since commit 0c7a8b9baa744ae4323bb46cb4fe942355beaa85:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
> (2017-07-04 11:17:02 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 44eff673411381062b826d048ba9d6630d2b2bdb:
>
>   target/i386: add the CONFIG_TCG into Makefiles (2017-07-05 09:12:44 +0200)
>
> 
> * qemu-thread portability improvement (Fam)
> * virtio-scsi IOMMU fix (Jason)
> * poisoning and common-obj-y cleanups (Thomas)
> * initial Hypervisor.framework refactoring (Sergio)
> * x86 TCG interrupt injection fixes (Wu Xiang, me)
> * --disable-tcg support for x86 (Yang Zhong, me)
> * various other bugfixes and cleanups (Daniel, Peter, Thomas)
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] change x86 default machine type to Q35?

[Daniel P. Berrange]

On Wed, Jul 05, 2017 at 08:22:00PM +0800, Fam Zheng wrote:
> On Wed, 07/05 13:58, Thomas Huth wrote:
> > On 05.07.2017 13:44, Fam Zheng wrote:
> > > On Wed, 07/05 12:04, Daniel P. Berrange wrote:
> > >> While you can say people should just add '-M pc' that isn't a nice
> > >> user experiance, because it makes the assumption that people actually
> > >> understand what caused their breakage. When the incompatibilities
> > >> arise the error messages are unlikely to give any hint to users that
> > >> the problems are caused by the machine type change. So unless someone
> > >> is very familiar with debugging QEMU, they're not going to realize
> > >> that '-M pc' will fix their problem.
> > > 
> > > While we change the default, can we start to print a message like 
> > > 'machine type
> > > ("-M" option) not specified, default to Q35. If you find compatibility 
> > > issues,
> > > try "-M pc"'?
> > 
> > Or we could simply remove the default machine type completely for a
> > couple of releases? That would force people to update their script with
> > "-M pc" ... and once this has been established, we can finally make q35
> > the default?
> 
> I wouldn't do that.. Defaults are for those who don't care about setting it, 
> or
> want something that just works, by default. It's fine not to provide a default
> machine type if we decide there isn't a clear win, just like arm, but IMHO
> removing it just to restore it later in order to educate people makes a very
> poor experience.

Since the idea of removing the default machine type was brought up, we should
perhaps consider that as a distinct solution. Going from using 'pc' by defailt
to having no default machine type, has significantly better user experiance
than changing the default machine to 'q35' IMHO.

eg if we killed the default, currently QEMU would say something like

   qemu-system-x86_64: No machine specified, and there is no default
   Use -machine help to list supported machines

We could augment that with further details:

   qemu-system-x86_64: No machine specified, and there is no default
   Use -machine help to list supported machines

   For compatibility with previous releases of QEMU, pick the 'pc'
   machine type. New deployments are suggested to use the 'q35'
   machine type.

This avoids the user having problems with guest ABI silently changing
behind their back, or migration suddenly failing to load vm state,
and gives them clear immediate guidance on how to resolve the problem
they're facing, as well as encouraging usage of 'q35' going forward,
without having to actually make it the default.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] change x86 default machine type to Q35?

[Peter Maydell]

On 6 July 2017 at 11:49, Daniel P. Berrange  wrote:
> eg if we killed the default, currently QEMU would say something like
>
>qemu-system-x86_64: No machine specified, and there is no default
>Use -machine help to list supported machines

Incidentally I really should get round to figuring out how to make
command lines like
 qemu-system-arm -device help
 qemu-system-arm -cpu help
work without the user having to specify "-M virt" too to avoid the
"no default machine" error taking precedence over the help output...

thanks
-- PMM

Re: [Qemu-devel] change x86 default machine type to Q35?

[Thomas Huth]

On 06.07.2017 12:49, Daniel P. Berrange wrote:
[...]
> This avoids the user having problems with guest ABI silently changing
> behind their back, or migration suddenly failing to load vm state,

For proper migration, you've got to specify the right versioned machine
type anyway, so that should not be an issue here.

 Thomas

Re: [Qemu-devel] change x86 default machine type to Q35?

[Daniel P. Berrange]

On Thu, Jul 06, 2017 at 01:00:53PM +0200, Thomas Huth wrote:
> On 06.07.2017 12:49, Daniel P. Berrange wrote:
> [...]
> > This avoids the user having problems with guest ABI silently changing
> > behind their back, or migration suddenly failing to load vm state,
> 
> For proper migration, you've got to specify the right versioned machine
> type anyway, so that should not be an issue here.

That's only true if you have mis-matched QEMU releases on each host.
If you know you have the same QEMU release everywhere, there's no
need for versioned machine types. Though obviously we'd still recommend
people us versioned machine type, its not unreasonable that some people
will not

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 05/11] linux-user/sh4: Notice gUSA regions during signal delivery

[John Paul Adrian Glaubitz]

On Thu, Jul 06, 2017 at 03:09:59AM +0200, Laurent Vivier wrote:
> Reviewed-by: Laurent Vivier 

I have identified this patch as reponsible for the
segfaults. Reverting this patch fixes the issue. The crashes are
random. Sometimes it crashes directly when entering the chroot,
sometimes only after a command like running "apt
update". Interestingly, the issue does not reproduce on an older
chroot from 2015.

I have uploaded two chroots for testing, one from 2015 [1] and one
freshly generated [2] which shows the crashes with patch nr. 5
applied.

Adrian

> [1] https://people.debian.org/~glaubitz/chroots/unstable-sh4-20150315.tar.gz
> [2] https://people.debian.org/~glaubitz/chroots/unstable-sh4-20170706.tgz

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [Qemu-devel] change x86 default machine type to Q35?

[Thomas Huth]

On 06.07.2017 13:06, Daniel P. Berrange wrote:
> On Thu, Jul 06, 2017 at 01:00:53PM +0200, Thomas Huth wrote:
>> On 06.07.2017 12:49, Daniel P. Berrange wrote:
>> [...]
>>> This avoids the user having problems with guest ABI silently changing
>>> behind their back, or migration suddenly failing to load vm state,
>>
>> For proper migration, you've got to specify the right versioned machine
>> type anyway, so that should not be an issue here.
> 
> That's only true if you have mis-matched QEMU releases on each host.
> If you know you have the same QEMU release everywhere, there's no
> need for versioned machine types.
Sure, but in case you use the same QEMU release on both ends, there
should also be no clash with the default machine type here.

 Thomas

Re: [Qemu-devel] change x86 default machine type to Q35?

[Dr. David Alan Gilbert]

* Daniel P. Berrange (berra...@redhat.com) wrote:
> On Thu, Jul 06, 2017 at 01:00:53PM +0200, Thomas Huth wrote:
> > On 06.07.2017 12:49, Daniel P. Berrange wrote:
> > [...]
> > > This avoids the user having problems with guest ABI silently changing
> > > behind their back, or migration suddenly failing to load vm state,
> > 
> > For proper migration, you've got to specify the right versioned machine
> > type anyway, so that should not be an issue here.
> 
> That's only true if you have mis-matched QEMU releases on each host.
> If you know you have the same QEMU release everywhere, there's no
> need for versioned machine types. Though obviously we'd still recommend
> people us versioned machine type, its not unreasonable that some people
> will not

We have got protection against machine type screwups; e.g going from -M
pc to -M q35 we get:

qemu-system-x86_64: Machine type received is 'pc-i440fx-2.9' and local is 
'pc-q35-2.9'

Dave

> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 11/21] mirror: Switch mirror_cow_align() to byte-based

[Kevin Wolf]

Am 05.07.2017 um 23:08 hat Eric Blake geschrieben:
> We are gradually converting to byte-based interfaces, as they are
> easier to reason about than sector-based.  Convert another internal
> function (no semantic change), and add mirror_clip_bytes() as a
> counterpart to mirror_clip_sectors().  Some of the conversion is
> a bit tricky, requiring temporaries to convert between units; it
> will be cleared up in a following patch.
> 
> Signed-off-by: Eric Blake 
> Reviewed-by: John Snow 
> Reviewed-by: Jeff Cody 

> -if (align_nb_sectors > max_sectors) {
> -align_nb_sectors = max_sectors;
> +if (align_bytes > max_bytes) {
> +align_bytes = max_bytes;
>  if (need_cow) {
> -align_nb_sectors = QEMU_ALIGN_DOWN(align_nb_sectors,
> -   s->target_cluster_size >>
> -   BDRV_SECTOR_BITS);
> +align_bytes = QEMU_ALIGN_DOWN(align_bytes,
> +  s->target_cluster_size);

This would fit in a single line, if you have to respin for some reason.

Reviewed-by: Kevin Wolf 

[Qemu-devel] [Bug 1702621] Re: colo: secondary vm crash during loadvm

[Dr. David Alan Gilbert]

I haven't tried COLO for a while; I've got a note I hit a similar error in  the 
past - 
I think I came to the conclusion it was the e1000 that was unhappy - probably 
sending an interrupt after it had stopped.
Try a different NIC.  I flipped to using a virtio-net at one point (but I think 
I had to disable vhost, there are some patches for this recently).

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

Title:
  colo: secondary vm crash during loadvm

Status in QEMU:
  New

Bug description:
  Following document 'COLO-FT.txt', I test colo feature on my hosts. It seems 
goes well. But after a while the secondary vm crash.  The stack is as follows:
  #0  0x7f191456dc37 in raise () from /lib/x86_64-linux-gnu/libc.so.6
  #1  0x7f1914571028 in abort () from /lib/x86_64-linux-gnu/libc.so.6
  #2  0x7f1914566bf6 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
  #3  0x7f1914566ca2 in __assert_fail () from 
/lib/x86_64-linux-gnu/libc.so.6
  #4  0x564154ad9147 in pcibus_reset (qbus=0x564156760d10) at 
../hw/pci/pci.c:311
  #5  0x564154a07cdb in qbus_reset_one (bus=0x564156760d10, opaque=0x0) at 
hw/core/qdev.c:319
  #6  0x564154a0d721 in qbus_walk_children (bus=0x564156760d10, 
pre_devfn=0, pre_busfn=0, 
  post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
  at hw/core/bus.c:68
  #7  0x564154a08b4d in qdev_walk_children (dev=0x56415675f2b0, 
pre_devfn=0, pre_busfn=0, 
  post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
  at hw/core/qdev.c:617
  #8  0x564154a0d6e5 in qbus_walk_children (bus=0x564156594d30, 
pre_devfn=0, pre_busfn=0, 
  post_devfn=0x564154a07c26 , post_busfn=0x564154a07c6c 
, opaque=0x0)
  at hw/core/bus.c:59
  #9  0x564154a07df5 in qbus_reset_all (bus=0x564156594d30) at 
hw/core/qdev.c:336
  #10 0x564154a07e3a in qbus_reset_all_fn (opaque=0x564156594d30) at 
hw/core/qdev.c:342
  #11 0x564154a0e222 in qemu_devices_reset () at hw/core/reset.c:69
  #12 0x5641548b3b47 in pc_machine_reset () at 
/vms/git/qemu/hw/i386/pc.c:2234
  #13 0x564154972ca7 in qemu_system_reset (report=false) at vl.c:1697
  #14 0x564154b9d007 in colo_process_incoming_thread 
(opaque=0x5641553c1280) at migration/colo.c:617
  #15 0x7f1914907184 in start_thread () from 
/lib/x86_64-linux-gnu/libpthread.so.0
  #16 0x7f1914634bed in clone () from /lib/x86_64-linux-gnu/libc.so.6

  (gdb) frame 4
  #4  0x564154ad9147 in pcibus_reset (qbus=0x564156760d10) at 
../hw/pci/pci.c:311
  warning: Source file is more recent than executable.
  311 assert(bus->irq_count[i] == 0);
  (gdb) ^CQuit
  (gdb) p bus->irq_count[i]
  $1 = -1

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

Re: [Qemu-devel] [RFC v2 0/8] VIRTIO-IOMMU device

[Bharat Bhushan]

> -Original Message-
> From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com]
> Sent: Thursday, July 06, 2017 3:33 PM
> To: Bharat Bhushan ; Auger Eric
> ; eric.auger@gmail.com;
> peter.mayd...@linaro.org; alex.william...@redhat.com; m...@redhat.com;
> qemu-...@nongnu.org; qemu-devel@nongnu.org
> Cc: w...@redhat.com; kevin.t...@intel.com; marc.zyng...@arm.com;
> t...@semihalf.com; will.dea...@arm.com; drjo...@redhat.com;
> robin.mur...@arm.com; christoffer.d...@linaro.org
> Subject: Re: [Qemu-devel] [RFC v2 0/8] VIRTIO-IOMMU device
> 
> On 05/07/17 09:49, Bharat Bhushan wrote:>>> Also when setup msi-route
> kvm_irqchip_add_msi_route() we needed to
> >> provide the translated address.
> >>> According to my understanding this is required because kernel does
> >>> no go
> >> through viommu translation when generating interrupt, no?
> >>
> >> yes this is needed when KVM MSI routes are set up, ie. along with GICV3
> ITS.
> >> With GICv2M, qemu direct gsi mapping is used and this is not needed.
> >>
> >> So I do not understand your previous sentence saying "MSI interrupts
> >> works without any change".
> >
> > I have almost completed vfio integration with virtio-iommu and now
> > testing the changes by assigning e1000 device to VM. For this I have
> > changed virtio-iommu driver to use IOMMU_RESV_MSI rather than sw-msi
> > and this does not need changed in vfio_get_addr()  and
> > kvm_irqchip_add_msi_route()
> 
> I understand you're reserving region 0x0800-0x0810 as
> IOMMU_RESV_MSI instead of IOMMU_RESV_SW_MSI? I think this only
> works because Qemu places the vgic in that area as well (in hw/arm/virt.c).
> It's not a coincidence if the addresses are the same, because Eric chose them
> for the Linux SMMU drivers and I copied them.
> 
> We can't rely on that behavior, though, it will break MSIs in emulated
> devices. And if Qemu happens to move the MSI doorbell in future machine
> revisions, then it would also break VFIO.

Yes, make sense to me

> 
> Just for my own understanding -- what happens, I think, is that in Linux
> iova_reserve_iommu_regions initially reserves the guest-physical doorbell
> 0x0800-0x0810. Then much later, when the device driver requests
> an MSI, the irqchip driver calls iommu_dma_map_msi_msg with the guest-
> physical gicv2m address 0x0802. The function finds the right page in
> msi_page_list, which was added by cookie_init_hw_msi_region, therefore
> bypassing the viommu and the GPA gets written in the MSI-X table.

This means in case tomorrow when qemu changes virt machine address map and 
vgic-its (its-translator register address) address range does not fall in the 
msi_page_list then it will allocate a new iova, create mapping in iommu. So 
this will no longer be identity mapped and fail to work with new qemu?

> 
> If an emulated device such as virtio-net-pci were to generate an MSI, then
> Qemu would attempt to access the doorbell written by Linux into the MSI-X
> table, 0x0802, and fault because that address wasn't mapped in the
> viommu.
> 
> So for VFIO, you either need to translate the MSI-X entry using the viommu,
> or just assume that the vaddr corresponds to the only MSI doorbell
> accessible by this device (because how can we be certain that the guest
> already mapped the doorbell before writing the entry?)
> 
> For ARM machines it's probably best to stick with IOMMU_RESV_SW_MSI.
> However, a nice way to use IOMMU_RESV_MSI would be for the virtio-
> iommu device to advertise identity-mapped/reserved regions, and bypass
> translation on these regions. Then the driver could reserve those with
> IOMMU_RESV_MSI.

Correct me if I did not understood you correctly, today iommu-driver decides 
msi-reserved region, what if we change this and virtio-iommu device will 
provide the reserved msi region as per the emulated machine (virt/intel). So 
virtio-iommu driver will use the address advertised by virtio-iommu device as 
IOMMU_RESV_MSI. In this case msi-page-list will always have the reserved region 
for MSI.
On qemu side, for emulated devices we will let virtio-iommu return same address 
as translated address as it falls in MSI-reserved page already known to it.


> For x86 we will need such a system, with an added IRQ
> remapping feature.

I do not understand x86 MSI interrupt generation, but If above understand is 
correct, then why we need IRQ remapping for x86?
Will the x86 machine emulated in QEMU provides a big address range for MSIs and 
when actually generating MSI it needed some extra processing (IRQ-remapping 
processing) before actually generating write transaction for MSI interrupt ? 

Thanks
-Bharat

> 
> Thanks,
> Jean

Re: [Qemu-devel] Managing architectural restrictions with -device and libvirt

[Markus Armbruster]

Marcel Apfelbaum  writes:

> On 05/07/2017 21:05, Markus Armbruster wrote:
>> Mark Cave-Ayland  writes:
[...]
>>> It seems like limiting the size of the bus would solve the majority of
>>> the problem. I've had a quick look around pci.c and while I can see that
>>> the PCIBus creation functions take a devfn_min parameter, I can't see
>>> anything that limits the number of slots available on the bus?
>>
>> Marcel?
>>
>
> Hi Markus,
> Sorry for my late reply.
>
> Indeed, we don't have currently a restriction on the number of usable
> slots on a bus, however deriving from PCIBus class and implementing
> the new policy should not be much trouble.
>
>
>>> And presumably if the user did try and coldplug something into a full
>>> bus then they would get the standard "PCI: no slot/function
>>> available..." error?
>>
>> That's what I'd expect.
>>
> My understanding from reading various bits of documentation is that the
> the empty simba bridge (bus B) can hold a maximum of 4 devices, whilst
> the non-empty simba bridge (bus A) can hold a maximum of 2 devices
> (presumably due to the on-board hardware). And in order to make sure
> OpenBIOS maps the PCI IO ranges correctly, the ebus must be the first
> on-board device found during a PCI bus scan which means slot 0 on bus A
> must be blacklisted.

 Assuming init() plugs in the device providing ebus: plug it into slot 0,
 mark it not hotpluggable, done.
>>>
>>> That is good solution in theory except that I'd like to keep the ebus in
>>> slot 1 so that it matches the real DT as much as possible. In the future
>>> it could be possible for people to boot using PROMs from a real Sun and
>>> I'm not yet convinced that there aren't hardcoded references to some of
>>> the onboard legacy devices in a real PROM.
>>
>> Misunderstanding on my part!  You don't have to blacklist slot 0 to have
>> the PCI core put ebus in slot 1.  Simply ask for slot 1 by passing
>> PCI_DEVFN(1, 0) to pci_create() or similar.
>>
>
> Right, hard-coding the device creation in machine init will solve that,
> however it will be against our long-term goal to create the machine as
> a puzzle, and for that, the devices should be created in some
> order. I suspect the task would not be easy to integrate as
> part of this project though.

Even in a world where we start with an empty board, then plug in stuff
directed by configuration file, an onboard device's PCI address should
be explicit.  Having to count PCI devices to figure out their address
sucks.

> I guess what I'm looking for is some kind of hook that runs after both
> machine init and all the devices have been specified on the command
> line, which I can use to validate the configuration and provide a
> suitable error message/hint if the configuration is invalid?

 You should be able to construct the machine you want, and protect the
 parts the user shouldn't mess with from messing users.  No need to
 validate the mess afterwards then.
>>>
>>> Unfortunately there would be issues if the user was allowed to construct
>>> a machine with more PCI devices than slots in real hardware, since the
>>> PCI interrupt number is limited to 4 bits - 2 bits for the PCI interrupt
>>> number (A to D), and 2 bits for the slot. So if a user tries to plug in
>>> more than 4 devices into each simba bus then the interrupts won't be
>>> mapped correctly.
>>>
>>> My feeling is that it makes more sense to error out if the user tries to
>>> add too many devices to the bus and/or in the wrong slots rather than
>>> let them carry on and wonder why the virtual devices don't work
>>> correctly, but I'm open to other options.
>>
>> My advice is to model the physical hardware faithfully.  If it has four
>> PCI slots on a certain PCI bus, provide exactly four.  If it has onboard
>> devices hardwired into a certain slot, put them exactly there, and
>> disable unplug.  Make it impossible to plug too many devices into a bus,
>> or into the wrong slots.
>>
>
> I agree, but still the user will see an error. However the error would
> be "slot x does not exist" which is clean.
>
>
> I see two ways to continue:
>  1. A new kind of pci-bridge should be created with a "special"
> secondary bus that has less slots. (harder to implement)
>  2. Add the limitation of the number of slots to the PCIBus class,
> (simpler to implement, but since is not a widely used case maybe
> is not the best way to go.

I suspect (2) would be trivial.  I like trivial.

Re: [Qemu-devel] [RFC v3 2/3] qemu-error: Implement a more generic error reporting

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> On Thu, Jul 06, 2017 at 08:15:54AM +0200, Markus Armbruster wrote:
>> Alistair Francis  writes:
>> 
>> > This patch converts the existing error_vreport() function into a generic
>> > qmesg_vreport() function that takes an enum describing the
>> > information to be reported.
>> >
>> > As part of this change a new qmesg_report() function is added as well with 
>> > the
>> > same capability.
>> >
>> > To maintain full compatibility the original error_report() function is
>> > maintained and no changes to the way errors are printed have been made.
>> > To improve access to the new informaiton and warning options wrapper 
>> > functions
>> > similar to error_report() have been added for warnings and information
>> > printing.
>> >
>> > Signed-off-by: Alistair Francis 
>
>> > diff --git a/util/qemu-error.c b/util/qemu-error.c
>> > index 1c5e35ecdb..63fdc0e174 100644
>> > --- a/util/qemu-error.c
>> > +++ b/util/qemu-error.c
>> > @@ -179,17 +179,29 @@ static void print_loc(void)
>> >  
>> >  bool enable_timestamp_msg;
>> >  /*
>> > - * Print an error message to current monitor if we have one, else to 
>> > stderr.
>> > + * Print a message to current monitor if we have one, else to stderr.
>> >   * Format arguments like vsprintf().  The resulting message should be
>> >   * a single phrase, with no newline or trailing punctuation.
>> >   * Prepend the current location and append a newline.
>> >   * It's wrong to call this in a QMP monitor.  Use error_setg() there.
>> >   */
>> > -void error_vreport(const char *fmt, va_list ap)
>> > +void qmsg_vreport(report_type type, const char *fmt, va_list ap)
>> >  {
>> >  GTimeVal tv;
>> >  gchar *timestr;
>> >  
>> > +switch (type) {
>> > +case REPORT_TYPE_ERROR:
>> > +/* To maintain compatibility we don't add anything here */
>> 
>> I feel the comment isn't going to be useful in the future.  Let's drop
>> it.
>
> Do we really need to care about compatibility of the precise way we output
> error messages. It has never been something we call a "stable API", as we
> don't guarantee error message text will remain the same across releases. So
> anyone relying on scraping QEMU stderr to match some error message has always
> been liable to break.
>
> IOW, just add an "error: " prefix to the text

I agree the error message format isn't ABI.

But what would adding "error: " buy us?

Re: [Qemu-devel] [RFC v3 2/3] qemu-error: Implement a more generic error reporting

[Daniel P. Berrange]

On Thu, Jul 06, 2017 at 01:27:15PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange"  writes:
> 
> > On Thu, Jul 06, 2017 at 08:15:54AM +0200, Markus Armbruster wrote:
> >> Alistair Francis  writes:
> >> 
> >> > This patch converts the existing error_vreport() function into a generic
> >> > qmesg_vreport() function that takes an enum describing the
> >> > information to be reported.
> >> >
> >> > As part of this change a new qmesg_report() function is added as well 
> >> > with the
> >> > same capability.
> >> >
> >> > To maintain full compatibility the original error_report() function is
> >> > maintained and no changes to the way errors are printed have been made.
> >> > To improve access to the new informaiton and warning options wrapper 
> >> > functions
> >> > similar to error_report() have been added for warnings and information
> >> > printing.
> >> >
> >> > Signed-off-by: Alistair Francis 
> >
> >> > diff --git a/util/qemu-error.c b/util/qemu-error.c
> >> > index 1c5e35ecdb..63fdc0e174 100644
> >> > --- a/util/qemu-error.c
> >> > +++ b/util/qemu-error.c
> >> > @@ -179,17 +179,29 @@ static void print_loc(void)
> >> >  
> >> >  bool enable_timestamp_msg;
> >> >  /*
> >> > - * Print an error message to current monitor if we have one, else to 
> >> > stderr.
> >> > + * Print a message to current monitor if we have one, else to stderr.
> >> >   * Format arguments like vsprintf().  The resulting message should be
> >> >   * a single phrase, with no newline or trailing punctuation.
> >> >   * Prepend the current location and append a newline.
> >> >   * It's wrong to call this in a QMP monitor.  Use error_setg() there.
> >> >   */
> >> > -void error_vreport(const char *fmt, va_list ap)
> >> > +void qmsg_vreport(report_type type, const char *fmt, va_list ap)
> >> >  {
> >> >  GTimeVal tv;
> >> >  gchar *timestr;
> >> >  
> >> > +switch (type) {
> >> > +case REPORT_TYPE_ERROR:
> >> > +/* To maintain compatibility we don't add anything here */
> >> 
> >> I feel the comment isn't going to be useful in the future.  Let's drop
> >> it.
> >
> > Do we really need to care about compatibility of the precise way we output
> > error messages. It has never been something we call a "stable API", as we
> > don't guarantee error message text will remain the same across releases. So
> > anyone relying on scraping QEMU stderr to match some error message has 
> > always
> > been liable to break.
> >
> > IOW, just add an "error: " prefix to the text
> 
> I agree the error message format isn't ABI.
> 
> But what would adding "error: " buy us?

It would clearly distinguish errors from any other output on stderr, which
may not be error related (for example SPICE commonly pollutes stderr with
lots of messages).

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC v2 0/8] VIRTIO-IOMMU device

[Jean-Philippe Brucker]

On 06/07/17 12:24, Bharat Bhushan wrote:
> 
> 
>> -Original Message-
>> From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com]
>> Sent: Thursday, July 06, 2017 3:33 PM
>> To: Bharat Bhushan ; Auger Eric
>> ; eric.auger@gmail.com;
>> peter.mayd...@linaro.org; alex.william...@redhat.com; m...@redhat.com;
>> qemu-...@nongnu.org; qemu-devel@nongnu.org
>> Cc: w...@redhat.com; kevin.t...@intel.com; marc.zyng...@arm.com;
>> t...@semihalf.com; will.dea...@arm.com; drjo...@redhat.com;
>> robin.mur...@arm.com; christoffer.d...@linaro.org
>> Subject: Re: [Qemu-devel] [RFC v2 0/8] VIRTIO-IOMMU device
>>
>> On 05/07/17 09:49, Bharat Bhushan wrote:>>> Also when setup msi-route
>> kvm_irqchip_add_msi_route() we needed to
 provide the translated address.
> According to my understanding this is required because kernel does
> no go
 through viommu translation when generating interrupt, no?

 yes this is needed when KVM MSI routes are set up, ie. along with GICV3
>> ITS.
 With GICv2M, qemu direct gsi mapping is used and this is not needed.

 So I do not understand your previous sentence saying "MSI interrupts
 works without any change".
>>>
>>> I have almost completed vfio integration with virtio-iommu and now
>>> testing the changes by assigning e1000 device to VM. For this I have
>>> changed virtio-iommu driver to use IOMMU_RESV_MSI rather than sw-msi
>>> and this does not need changed in vfio_get_addr()  and
>>> kvm_irqchip_add_msi_route()
>>
>> I understand you're reserving region 0x0800-0x0810 as
>> IOMMU_RESV_MSI instead of IOMMU_RESV_SW_MSI? I think this only
>> works because Qemu places the vgic in that area as well (in hw/arm/virt.c).
>> It's not a coincidence if the addresses are the same, because Eric chose them
>> for the Linux SMMU drivers and I copied them.
>>
>> We can't rely on that behavior, though, it will break MSIs in emulated
>> devices. And if Qemu happens to move the MSI doorbell in future machine
>> revisions, then it would also break VFIO.
> 
> Yes, make sense to me
> 
>>
>> Just for my own understanding -- what happens, I think, is that in Linux
>> iova_reserve_iommu_regions initially reserves the guest-physical doorbell
>> 0x0800-0x0810. Then much later, when the device driver requests
>> an MSI, the irqchip driver calls iommu_dma_map_msi_msg with the guest-
>> physical gicv2m address 0x0802. The function finds the right page in
>> msi_page_list, which was added by cookie_init_hw_msi_region, therefore
>> bypassing the viommu and the GPA gets written in the MSI-X table.
> 
> This means in case tomorrow when qemu changes virt machine address map and 
> vgic-its (its-translator register address) address range does not fall in the 
> msi_page_list then it will allocate a new iova, create mapping in iommu. So 
> this will no longer be identity mapped and fail to work with new qemu?

Precisely

>>
>> If an emulated device such as virtio-net-pci were to generate an MSI, then
>> Qemu would attempt to access the doorbell written by Linux into the MSI-X
>> table, 0x0802, and fault because that address wasn't mapped in the
>> viommu.
>>
>> So for VFIO, you either need to translate the MSI-X entry using the viommu,
>> or just assume that the vaddr corresponds to the only MSI doorbell
>> accessible by this device (because how can we be certain that the guest
>> already mapped the doorbell before writing the entry?)
>>
>> For ARM machines it's probably best to stick with IOMMU_RESV_SW_MSI.
>> However, a nice way to use IOMMU_RESV_MSI would be for the virtio-
>> iommu device to advertise identity-mapped/reserved regions, and bypass
>> translation on these regions. Then the driver could reserve those with
>> IOMMU_RESV_MSI.
> 
> Correct me if I did not understood you correctly, today iommu-driver decides 
> msi-reserved region, what if we change this and virtio-iommu device will 
> provide the reserved msi region as per the emulated machine (virt/intel). So 
> virtio-iommu driver will use the address advertised by virtio-iommu device as 
> IOMMU_RESV_MSI. In this case msi-page-list will always have the reserved 
> region for MSI.
> On qemu side, for emulated devices we will let virtio-iommu return same 
> address as translated address as it falls in MSI-reserved page already known 
> to it.

Yes that's it. For example on x86, the virtio-iommu device will advertise
range 0xfee0-0xfeef as IOMMU_RESV_MSI, which is the IOAPIC
address. And it will leave any access to this region go through
untranslated. As far as I understand, that's what the AMD vIOMMU does.

In the x86 world this fee0 window is known implicitly to be the MSI
window. But the virtio-iommu device needs a generic method to declare
which bypassed windows it implements (and actually implement it), and I
still have to think about that. That way an emulated ARM system could also
advertise the GIC doorbell as bypassed.

>> For x86 we will need such a system, wi

Re: [Qemu-devel] [PULL 0/8] s390x/kvm/migration: fixes, enhancements and cleanups

[Peter Maydell]

On 6 July 2017 at 08:23, Christian Borntraeger  wrote:
> Peter,
>
> the following changes since commit 2185c93ba80f81bfa27ce6f259c7f2ef4f08b668:
>
>   Merge remote-tracking branch 
> 'remotes/edgar/tags/edgar/xilinx-next.for-upstream' into staging (2017-07-04 
> 13:05:30 +0100)
>
> are available in the git repository at:
>
>   git://github.com/borntraeger/qemu.git tags/s390x-20170706
>
> for you to fetch changes up to 1045e3cdafaf1218d9f771080ba5c9d4f64346e4:
>
>   hw/s390x/ipl: Fix endianness problem with netboot_start_addr (2017-07-05 
> 19:46:30 +0200)
>
> 
> s390x/kvm/migration: fixes, enhancements and cleanups
>
> - new email address for Cornelia
> - Fixes: 3270, flic, virtio-scsi-ccw, ipl
> - Enhancements, cpumodel, migration
>
> 
> Cornelia Huck (1):
>   s390x/MAINTAINERS: Update my email address
>
> Dong Jia Shi (1):
>   s390x/3270: fix instruction interception handler
>
> Halil Pasic (3):
>   s390x: vmstatify config migration for virtio-ccw
>   s390x: fix error propagation in kvm-flic's realize
>   s390x: fix realize inheritance for kvm-flic
>
> QingFeng Hao (1):
>   virtio-scsi-ccw: use ioeventfd even when KVM is disabled
>
> Thomas Huth (1):
>   hw/s390x/ipl: Fix endianness problem with netboot_start_addr
>
> Viktor Mihajlovski (1):
>   s390x: return unavailable features via query-cpu-definitions
>

Applied, thanks.

-- PMM

[Qemu-devel] [PATCH v6 00/13] chardevice hotswap

[Anton Nefedov]

Changed in v6:
  - patch 3: chr_lock mutex removed (as discussed, thread safety can be left to
the front-ends, and the front-ends currently supported are good as is).
  - patch 5 now includes related changes for serial and virtio-serial 
  - patch 13 commit message extended



Changed in v5:
  - rebased
  - patch 6 fixed (wouldn't compile until patch 7 (broken by previous rebase))
  - patch 11 commit message added



Changed in v4:
  - rebased on top of the latest chardev changes
  - remarks applied
  - patch 1 fixed so it works with alias names



Changed in v3:
  - minor remarks to patch 1 applied
  - patch 3: avoid using bottom-half, handle syncronously
As mentioned, it gets thing complicated and is only a problem for
a monitor-connected chardev hotswap and that is not supported for now
  - tests added (patches 6-9)



This serie is a v2 of the February submit
http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg01989.html

The interface is changed as requested and the changes are slightly reworked
and split into separate patches.



The patchset adds support of the character device change without
a frontend device removal.
Yet isa-serial and virtio-serial frontends are supported.

The feature can be helpful for e.g. Windows debug allowing to
establish connection to a live VM from VM with WinDbg.

Anton Nefedov (13):
  char: move QemuOpts->ChardevBackend translation to a separate func
  char: add backend hotswap handler
  char: chardevice hotswap
  char: forbid direct chardevice access for hotswap devices
  char: avoid chardevice direct access
  test-char: destroy chardev-udp after test
  test-char: split char_udp_test
  test-char: split char_file_test
  test-char: add hotswap test
  hmp: add hmp analogue for qmp-chardev-change
  virtio-console: chardev hotswap support
  serial: move TIOCM update to a separate function
  serial: chardev hotswap support

 include/chardev/char-fe.h   |  22 
 include/chardev/char.h  |  19 +++
 hmp.h   |   1 +
 backends/rng-egd.c  |   2 +-
 chardev/char-fe.c   |  16 ++-
 chardev/char-mux.c  |   1 +
 chardev/char.c  | 164 +-
 gdbstub.c   |   2 +-
 hmp-commands.hx |  18 ++-
 hmp.c   |  34 ++
 hw/arm/pxa2xx.c |   3 +-
 hw/arm/strongarm.c  |   4 +-
 hw/char/bcm2835_aux.c   |   2 +-
 hw/char/cadence_uart.c  |   4 +-
 hw/char/debugcon.c  |   4 +-
 hw/char/digic-uart.c|   2 +-
 hw/char/escc.c  |   8 +-
 hw/char/etraxfs_ser.c   |   2 +-
 hw/char/exynos4210_uart.c   |   4 +-
 hw/char/grlib_apbuart.c |   4 +-
 hw/char/imx_serial.c|   2 +-
 hw/char/ipoctal232.c|   4 +-
 hw/char/lm32_juart.c|   2 +-
 hw/char/lm32_uart.c |   2 +-
 hw/char/mcf_uart.c  |   2 +-
 hw/char/milkymist-uart.c|   2 +-
 hw/char/parallel.c  |   2 +-
 hw/char/pl011.c |   2 +-
 hw/char/sclpconsole-lm.c|   4 +-
 hw/char/sclpconsole.c   |   4 +-
 hw/char/serial.c|  63 +++---
 hw/char/sh_serial.c |   4 +-
 hw/char/spapr_vty.c |   4 +-
 hw/char/stm32f2xx_usart.c   |   3 +-
 hw/char/terminal3270.c  |   4 +-
 hw/char/virtio-console.c|  35 +-
 hw/char/xen_console.c   |   4 +-
 hw/char/xilinx_uartlite.c   |   2 +-
 hw/ipmi/ipmi_bmc_extern.c   |   4 +-
 hw/mips/boston.c|   2 +-
 hw/mips/mips_malta.c|   2 +-
 hw/misc/ivshmem.c   |   6 +-
 hw/usb/ccid-card-passthru.c |   6 +-
 hw/usb/dev-serial.c |   7 +-
 hw/usb/redirect.c   |   7 +-
 monitor.c   |   4 +-
 net/colo-compare.c  |  10 +-
 net/filter-mirror.c |   8 +-
 net/slirp.c |   2 +-
 net/vhost-user.c|   7 +-
 qapi-schema.json|  40 +++
 qtest.c |   2 +-
 tests/test-char.c   | 275 
 tests/test-hmp.c|   1 +
 tests/vhost-user-test.c |   2 +-
 55 files changed, 647 insertions(+), 199 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v6 02/13] char: add backend hotswap handler

[Anton Nefedov]

Frontends should have an interface to setup the handler of a backend change.
The interface will be used in the next commits

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Marc-André Lureau 
---
 include/chardev/char-fe.h   |  5 +
 backends/rng-egd.c  |  2 +-
 chardev/char-fe.c   |  4 +++-
 chardev/char-mux.c  |  1 +
 gdbstub.c   |  2 +-
 hw/arm/pxa2xx.c |  3 ++-
 hw/arm/strongarm.c  |  2 +-
 hw/char/bcm2835_aux.c   |  2 +-
 hw/char/cadence_uart.c  |  2 +-
 hw/char/debugcon.c  |  2 +-
 hw/char/digic-uart.c|  2 +-
 hw/char/escc.c  |  2 +-
 hw/char/etraxfs_ser.c   |  2 +-
 hw/char/exynos4210_uart.c   |  2 +-
 hw/char/grlib_apbuart.c |  2 +-
 hw/char/imx_serial.c|  2 +-
 hw/char/ipoctal232.c|  2 +-
 hw/char/lm32_juart.c|  2 +-
 hw/char/lm32_uart.c |  2 +-
 hw/char/mcf_uart.c  |  2 +-
 hw/char/milkymist-uart.c|  2 +-
 hw/char/pl011.c |  2 +-
 hw/char/sclpconsole-lm.c|  2 +-
 hw/char/sclpconsole.c   |  2 +-
 hw/char/serial.c|  2 +-
 hw/char/sh_serial.c |  2 +-
 hw/char/spapr_vty.c |  2 +-
 hw/char/stm32f2xx_usart.c   |  3 ++-
 hw/char/terminal3270.c  |  2 +-
 hw/char/virtio-console.c|  4 ++--
 hw/char/xen_console.c   |  2 +-
 hw/char/xilinx_uartlite.c   |  2 +-
 hw/ipmi/ipmi_bmc_extern.c   |  2 +-
 hw/mips/boston.c|  2 +-
 hw/mips/mips_malta.c|  2 +-
 hw/misc/ivshmem.c   |  2 +-
 hw/usb/ccid-card-passthru.c |  2 +-
 hw/usb/dev-serial.c |  2 +-
 hw/usb/redirect.c   |  2 +-
 monitor.c   |  4 ++--
 net/colo-compare.c  | 10 ++
 net/filter-mirror.c |  6 +++---
 net/slirp.c |  2 +-
 net/vhost-user.c|  7 ---
 qtest.c |  2 +-
 tests/test-char.c   | 14 ++
 tests/vhost-user-test.c |  2 +-
 47 files changed, 76 insertions(+), 57 deletions(-)

diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 2cbb262..9f38060 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -4,6 +4,7 @@
 #include "chardev/char.h"
 
 typedef void IOEventHandler(void *opaque, int event);
+typedef int BackendChangeHandler(void *opaque);
 
 /* This is the backend as seen by frontend, the actual backend is
  * Chardev */
@@ -12,6 +13,7 @@ struct CharBackend {
 IOEventHandler *chr_event;
 IOCanReadHandler *chr_can_read;
 IOReadHandler *chr_read;
+BackendChangeHandler *chr_be_change;
 void *opaque;
 int tag;
 int fe_open;
@@ -54,6 +56,8 @@ Chardev *qemu_chr_fe_get_driver(CharBackend *be);
  *   receive
  * @fd_read: callback to receive data from char
  * @fd_event: event callback
+ * @be_change: backend change callback; passing NULL means hot backend change
+ * is not supported and will not be attempted
  * @opaque: an opaque pointer for the callbacks
  * @context: a main loop context or NULL for the default
  * @set_open: whether to call qemu_chr_fe_set_open() implicitely when
@@ -68,6 +72,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
   IOCanReadHandler *fd_can_read,
   IOReadHandler *fd_read,
   IOEventHandler *fd_event,
+  BackendChangeHandler *be_change,
   void *opaque,
   GMainContext *context,
   bool set_open);
diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index e7ce2ca..d2b9ce6 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -106,7 +106,7 @@ static void rng_egd_opened(RngBackend *b, Error **errp)
 
 /* FIXME we should resubmit pending requests when the CDS reconnects. */
 qemu_chr_fe_set_handlers(&s->chr, rng_egd_chr_can_read,
- rng_egd_chr_read, NULL, s, NULL, true);
+ rng_egd_chr_read, NULL, NULL, s, NULL, true);
 }
 
 static void rng_egd_set_chardev(Object *obj, const char *value, Error **errp)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 3f90f05..7054863 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -216,7 +216,7 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del)
 assert(b);
 
 if (b->chr) {
-qemu_chr_fe_set_handlers(b, NULL, NULL, NULL, NULL, NULL, true);
+qemu_chr_fe_set_handlers(b, NULL, NULL, NULL, NULL, NULL, NULL, true);
 if (b->chr->be == b) {
 b->chr->be = NULL;
 }
@@ -235,6 +235,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
   IOCanReadHandler *fd_can_read,
   IOReadHandler *fd_read,
   IOEventHandler *fd_event,
+  BackendChangeHandler *be_change,
  

[Qemu-devel] [PATCH v6 03/13] char: chardevice hotswap

[Anton Nefedov]

This patch adds a possibility to change a char device without a frontend
removal.

Ideally, it would have to happen transparently to a frontend, i.e.
frontend would continue its regular operation.
However, backends are not stateless and are set up by the frontends
via qemu_chr_fe_<> functions, and it's not (generally) possible to replay
that setup entirely in a backend code, as different chardevs respond
to the setup calls differently, so do frontends work differently basing
on those setup responses.
Moreover, some frontend can generally get and save the backend pointer
(qemu_chr_fe_get_driver()), and it will become invalid after backend change.

So, a frontend which would like to support chardev hotswap has to register
a "backend change" handler, and redo its backend setup there.

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 include/chardev/char.h |  9 ++
 chardev/char.c | 83 ++
 qapi-schema.json   | 40 
 3 files changed, 132 insertions(+)

diff --git a/include/chardev/char.h b/include/chardev/char.h
index 8a9ade4..22fd734 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -93,6 +93,15 @@ void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon 
*backend);
 Chardev *qemu_chr_new(const char *label, const char *filename);
 
 /**
+ * @qemu_chr_change:
+ *
+ * Change an existing character backend
+ *
+ * @opts the new backend options
+ */
+void qemu_chr_change(QemuOpts *opts, Error **errp);
+
+/**
  * @qemu_chr_cleanup:
  *
  * Delete all chardevs (when leaving qemu)
diff --git a/chardev/char.c b/chardev/char.c
index 839eff6..d6b9d89 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -951,6 +951,89 @@ ChardevReturn *qmp_chardev_add(const char *id, 
ChardevBackend *backend,
 return ret;
 }
 
+ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
+  Error **errp)
+{
+CharBackend *be;
+const ChardevClass *cc;
+Chardev *chr, *chr_new;
+bool closed_sent = false;
+ChardevReturn *ret;
+
+chr = qemu_chr_find(id);
+if (!chr) {
+error_setg(errp, "Chardev '%s' does not exist", id);
+return NULL;
+}
+
+if (CHARDEV_IS_MUX(chr)) {
+error_setg(errp, "Mux device hotswap not supported yet");
+return NULL;
+}
+
+if (qemu_chr_replay(chr)) {
+error_setg(errp,
+"Chardev '%s' cannot be changed in record/replay mode", id);
+return NULL;
+}
+
+be = chr->be;
+if (!be) {
+/* easy case */
+object_unparent(OBJECT(chr));
+return qmp_chardev_add(id, backend, errp);
+}
+
+if (!be->chr_be_change) {
+error_setg(errp, "Chardev user does not support chardev hotswap");
+return NULL;
+}
+
+cc = char_get_class(ChardevBackendKind_lookup[backend->type], errp);
+if (!cc) {
+return NULL;
+}
+
+chr_new = qemu_chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)),
+   backend, errp);
+if (!chr_new) {
+return NULL;
+}
+chr_new->label = g_strdup(id);
+
+if (chr->be_open && !chr_new->be_open) {
+qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
+closed_sent = true;
+}
+
+chr->be = NULL;
+qemu_chr_fe_init(be, chr_new, &error_abort);
+
+if (be->chr_be_change(be->opaque) < 0) {
+error_setg(errp, "Chardev '%s' change failed", chr_new->label);
+chr_new->be = NULL;
+qemu_chr_fe_init(be, chr, &error_abort);
+if (closed_sent) {
+qemu_chr_be_event(chr, CHR_EVENT_OPENED);
+}
+object_unref(OBJECT(chr_new));
+return NULL;
+}
+
+object_unparent(OBJECT(chr));
+object_property_add_child(get_chardevs_root(), chr_new->label,
+  OBJECT(chr_new), &error_abort);
+object_unref(OBJECT(chr_new));
+
+ret = g_new0(ChardevReturn, 1);
+if (CHARDEV_IS_PTY(chr_new)) {
+ret->pty = g_strdup(chr_new->filename + 4);
+ret->has_pty = true;
+}
+
+return ret;
+}
+
 void qmp_chardev_remove(const char *id, Error **errp)
 {
 Chardev *chr;
diff --git a/qapi-schema.json b/qapi-schema.json
index 37c4b95..0090fbf 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -5098,6 +5098,46 @@
   'returns': 'ChardevReturn' }
 
 ##
+# @chardev-change:
+#
+# Change a character device backend
+#
+# @id: the chardev's ID, must exist
+# @backend: new backend type and parameters
+#
+# Returns: ChardevReturn.
+#
+# Since: 2.10
+#
+# Example:
+#
+# -> { "execute" : "chardev-change",
+#  "arguments" : { "id" : "baz",
+#  "backend" : { "type" : "pty", "data" : {} } } }
+# <- { "return": { "pty" : "/dev/pty/42" } }
+#
+# -> {"execute" : "chardev-change",
+# "arguments" : {
+# "id" : "charchannel2",
+# "backend" : {
+# "type" : "socket",
+# "dat

Re: [Qemu-devel] [PATCH 05/11] linux-user/sh4: Notice gUSA regions during signal delivery

[Laurent Vivier]

Le 06/07/2017 à 02:23, Richard Henderson a écrit :
> We translate gUSA regions atomically in a parallel context.
> But in a serial context a gUSA region may be interrupted.
> In that case, restart the region as the kernel would.
> 
> Signed-off-by: Richard Henderson 
> ---
>  linux-user/signal.c | 21 +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 3d18d1b..1e716a9 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -3471,6 +3471,23 @@ static abi_ulong get_sigframe(struct target_sigaction 
> *ka,
>  return (sp - frame_size) & -8ul;
>  }
>  
> +/* Notice when we're in the middle of a gUSA region and reset.
> +   Note that this will only occur for !parallel_cpus, as we will
> +   translate such sequences differently in a parallel context.  */
> +static void unwind_gusa(CPUSH4State *regs)
> +{
> +/* If the stack pointer is sufficiently negative... */
> +if ((regs->gregs[15] & 0xc000u) == 0xc000u) {

kernel also checks PC < gUSA region end point,
try this:

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 1e716a9..4e1e4f0 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -3477,7 +3477,8 @@ static abi_ulong get_sigframe(struct
target_sigaction *ka,
 static void unwind_gusa(CPUSH4State *regs)
 {
 /* If the stack pointer is sufficiently negative... */
-if ((regs->gregs[15] & 0xc000u) == 0xc000u) {
+if ((regs->gregs[15] & 0xc000u) == 0xc000u &&
+regs->pc < regs->gregs[0]) {
 /* Reset the PC to before the gUSA region, as computed from
R0 = region end, SP = -(region size), plus one more insn
that actually sets SP to the region size.  */

Laurent

[Qemu-devel] [PATCH v6 01/13] char: move QemuOpts->ChardevBackend translation to a separate func

[Anton Nefedov]

parse function will be used by the following patch

Signed-off-by: Anton Nefedov 
---
 chardev/char.c | 81 --
 1 file changed, 51 insertions(+), 30 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index 2b679a2..839eff6 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -556,17 +556,23 @@ help_string_append(const char *name, void *opaque)
 g_string_append_printf(str, "\n%s", name);
 }
 
-Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
-Error **errp)
+static const char *chardev_alias_translate(const char *name)
+{
+int i;
+for (i = 0; i < (int)ARRAY_SIZE(chardev_alias_table); i++) {
+if (g_strcmp0(chardev_alias_table[i].alias, name) == 0) {
+return chardev_alias_table[i].typename;
+}
+}
+return name;
+}
+
+static ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts, Error **errp)
 {
 Error *local_err = NULL;
 const ChardevClass *cc;
-Chardev *chr;
-int i;
 ChardevBackend *backend = NULL;
-const char *name = qemu_opt_get(opts, "backend");
-const char *id = qemu_opts_id(opts);
-char *bid = NULL;
+const char *name = chardev_alias_translate(qemu_opt_get(opts, "backend"));
 
 if (name == NULL) {
 error_setg(errp, "chardev: \"%s\" missing backend",
@@ -574,7 +580,40 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
 return NULL;
 }
 
-if (is_help_option(name)) {
+cc = char_get_class(name, errp);
+if (cc == NULL) {
+return NULL;
+}
+
+backend = g_new0(ChardevBackend, 1);
+backend->type = CHARDEV_BACKEND_KIND_NULL;
+
+if (cc->parse) {
+cc->parse(opts, backend, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+qapi_free_ChardevBackend(backend);
+return NULL;
+}
+} else {
+ChardevCommon *ccom = g_new0(ChardevCommon, 1);
+qemu_chr_parse_common(opts, ccom);
+backend->u.null.data = ccom; /* Any ChardevCommon member would work */
+}
+
+return backend;
+}
+
+Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error **errp)
+{
+const ChardevClass *cc;
+Chardev *chr = NULL;
+ChardevBackend *backend = NULL;
+const char *name = chardev_alias_translate(qemu_opt_get(opts, "backend"));
+const char *id = qemu_opts_id(opts);
+char *bid = NULL;
+
+if (name && is_help_option(name)) {
 GString *str = g_string_new("");
 
 chardev_name_foreach(help_string_append, str);
@@ -589,38 +628,20 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
 return NULL;
 }
 
-for (i = 0; i < (int)ARRAY_SIZE(chardev_alias_table); i++) {
-if (g_strcmp0(chardev_alias_table[i].alias, name) == 0) {
-name = chardev_alias_table[i].typename;
-break;
-}
+backend = qemu_chr_parse_opts(opts, errp);
+if (backend == NULL) {
+return NULL;
 }
 
 cc = char_get_class(name, errp);
 if (cc == NULL) {
-return NULL;
+goto out;
 }
 
-backend = g_new0(ChardevBackend, 1);
-backend->type = CHARDEV_BACKEND_KIND_NULL;
-
 if (qemu_opt_get_bool(opts, "mux", 0)) {
 bid = g_strdup_printf("%s-base", id);
 }
 
-chr = NULL;
-if (cc->parse) {
-cc->parse(opts, backend, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-goto out;
-}
-} else {
-ChardevCommon *ccom = g_new0(ChardevCommon, 1);
-qemu_chr_parse_common(opts, ccom);
-backend->u.null.data = ccom; /* Any ChardevCommon member would work */
-}
-
 chr = qemu_chardev_new(bid ? bid : id,
object_class_get_name(OBJECT_CLASS(cc)),
backend, errp);
-- 
2.7.4

[Qemu-devel] [PATCH v6 04/13] char: forbid direct chardevice access for hotswap devices

[Anton Nefedov]

qemu_chr_fe_get_driver() is unsafe, frontends with hotswap support
should not access CharDriver ptr directly as CharDriver might change.

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Marc-André Lureau 
---
 include/chardev/char-fe.h | 10 ++
 chardev/char-fe.c |  7 +++
 2 files changed, 17 insertions(+)

diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 9f38060..588abed 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -46,10 +46,20 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del);
  *
  * Returns the driver associated with a CharBackend or NULL if no
  * associated Chardev.
+ * Note: avoid this function as the driver should never be accessed directly,
+ *   especially by the frontends that support chardevice hotswap.
+ *   Consider qemu_chr_fe_backend_connected() to check for driver existence
  */
 Chardev *qemu_chr_fe_get_driver(CharBackend *be);
 
 /**
+ * @qemu_chr_fe_backend_connected:
+ *
+ * Returns true if there is a chardevice associated with @be.
+ */
+bool qemu_chr_fe_backend_connected(CharBackend *be);
+
+/**
  * @qemu_chr_fe_set_handlers:
  * @b: a CharBackend
  * @fd_can_read: callback to get the amount of data the frontend may
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 7054863..de5ba3c 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -179,9 +179,16 @@ void qemu_chr_fe_printf(CharBackend *be, const char *fmt, 
...)
 
 Chardev *qemu_chr_fe_get_driver(CharBackend *be)
 {
+/* this is unsafe for the users that support chardev hotswap */
+assert(be->chr_be_change == NULL);
 return be->chr;
 }
 
+bool qemu_chr_fe_backend_connected(CharBackend *be)
+{
+return !!be->chr;
+}
+
 bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp)
 {
 int tag = 0;
-- 
2.7.4