Re: [Qemu-devel] [PATCH v2 1/5] net/dump: Add support for receive_iov function

[Yang Hongyang]

On 10/13/2015 06:39 PM, Thomas Huth wrote:

Adding a proper receive_iov function to the net dump module.
This will make it easier to support the dump filter feature for
the -netdev option in later patches.

Signed-off-by: Thomas Huth 


Reviewed-by: Yang Hongyang 


---
  net/dump.c | 24 +---
  1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index 08259af..aa0d45d 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -25,6 +25,7 @@
  #include "clients.h"
  #include "qemu-common.h"
  #include "qemu/error-report.h"
+#include "qemu/iov.h"
  #include "qemu/log.h"
  #include "qemu/timer.h"
  #include "hub.h"
@@ -57,12 +58,15 @@ struct pcap_sf_pkthdr {
  uint32_t len;
  };

-static ssize_t dump_receive(NetClientState *nc, const uint8_t *buf, size_t 
size)
+static ssize_t dump_receive_iov(NetClientState *nc, const struct iovec *iov,
+int cnt)
  {
  DumpState *s = DO_UPCAST(DumpState, nc, nc);
  struct pcap_sf_pkthdr hdr;
  int64_t ts;
  int caplen;
+size_t size = iov_size(iov, cnt);
+struct iovec dumpiov[cnt + 1];

  /* Early return in case of previous error. */
  if (s->fd < 0) {
@@ -76,8 +80,12 @@ static ssize_t dump_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
  hdr.ts.tv_usec = ts % 100;
  hdr.caplen = caplen;
  hdr.len = size;
-if (write(s->fd, , sizeof(hdr)) != sizeof(hdr) ||
-write(s->fd, buf, caplen) != caplen) {
+
+dumpiov[0].iov_base = 
+dumpiov[0].iov_len = sizeof(hdr);
+cnt = iov_copy([1], cnt, iov, cnt, 0, caplen);
+
+if (writev(s->fd, dumpiov, cnt + 1) != sizeof(hdr) + caplen) {
  qemu_log("-net dump write error - stop dump\n");
  close(s->fd);
  s->fd = -1;
@@ -86,6 +94,15 @@ static ssize_t dump_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
  return size;
  }

+static ssize_t dump_receive(NetClientState *nc, const uint8_t *buf, size_t 
size)
+{
+struct iovec iov = {
+.iov_base = (void *)buf,
+.iov_len = size
+};
+return dump_receive_iov(nc, , 1);
+}
+
  static void dump_cleanup(NetClientState *nc)
  {
  DumpState *s = DO_UPCAST(DumpState, nc, nc);
@@ -97,6 +114,7 @@ static NetClientInfo net_dump_info = {
  .type = NET_CLIENT_OPTIONS_KIND_DUMP,
  .size = sizeof(DumpState),
  .receive = dump_receive,
+.receive_iov = dump_receive_iov,
  .cleanup = dump_cleanup,
  };




--
Thanks,
Yang.

Re: [Qemu-devel] [PATCH] net: cadence_gem: Set initial MAC address

[Jason Wang]

On 10/12/2015 04:25 PM, Sebastian Huber wrote:
> Set initial MAC address to the one specified by the command line.
>
> Signed-off-by: Sebastian Huber 
> Reviewed-by: Jason Wang 
> Reviewed-by: Peter Crosthwaite 
> ---
>  hw/net/cadence_gem.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 1127223..3639fc1 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -964,6 +964,7 @@ static void gem_reset(DeviceState *d)
>  {
>  int i;
>  CadenceGEMState *s = CADENCE_GEM(d);
> +const uint8_t *a;
>  
>  DB_PRINT("\n");
>  
> @@ -982,6 +983,11 @@ static void gem_reset(DeviceState *d)
>  s->regs[GEM_DESCONF5] = 0x002f2145;
>  s->regs[GEM_DESCONF6] = 0x0200;
>  
> +/* Set MAC address */
> +a = >conf.macaddr.a[0];
> +s->regs[GEM_SPADDR1LO] = a[0] | (a[1] << 8) | (a[2] << 16) | (a[3] << 
> 24);
> +s->regs[GEM_SPADDR1HI] = a[4] | (a[5] << 8);
> +
>  for (i = 0; i < 4; i++) {
>  s->sar_active[i] = false;
>  }

Applied in https://github.com/jasowang/qemu/commits/net

Thanks

Re: [Qemu-devel] [PATCH v4 3/5] acpi: pc: add fw_cfg device node to ssdt

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 07:43:00PM -0300, Eduardo Habkost wrote:
> On Wed, Oct 14, 2015 at 12:18:10AM +0300, Michael S. Tsirkin wrote:
> > On Tue, Oct 13, 2015 at 04:10:03PM -0300, Eduardo Habkost wrote:
> > > One of the things that may break if guest-visible bits of the machine
> > > change is Windows license activation, but the rules Windows use to
> > > trigger reactivation aren't very clear.
> > 
> > They are easy to find on the internet.
> 
> I couldn't find them[1]. If you have a pointer to a clear description of
> the rules for all Windows versions, I would love to see it.


The detailed info is not hard to find:
http://en.wikipedia.org/wiki/Microsoft_Product_Activation
links to:
http://technet.microsoft.com/en-us/library/bb457054.aspx


1
Display Adapter
00010 (5)
2
SCSI Adapter
00011 (5)
3
IDE Adapter
0011 (4)
4
Network Adapter MAC Address
1001011000 (10)
5
RAM Amount Range (i.e. 0-64mb, 64-128mb, etc)
101 (3)
6
Processor Type
011 (3)
7
Processor Serial Number
00 (6)
8
Hard Drive Device
1101100 (7)
9
Hard Drive Volume Serial Number
100101 (10)
10
CD—ROM / CD-RW / DVD-ROM
010111 (6)
-
"Dockable"
0 (1)
-
Hardware Hash version (version of algorithm used)
001 (3)

Also:
Microsoft defines "substantially different" hardware differently for PCs
that are configured to be dockable. Additionally, the network adapter is
given a superior "weighting." If the PC is not dockable and a network
adapter exists and is not changed, 6 or more of the other above values
would have to change before reactivation was required. If a network
adapter existed but is changed or never existed at all, 4 or more
changes (including the changed network adapter if it previously existed)
will result in a requirement to reactivate.

So it's also not hard to try it out with kvm.  Apply a patch, change
3 other things: nic mac, ram size (by a factor of >2), cpu type and boot.



> >
> >___
> >Xen-devel mailing list
> >xen-de...@lists.xen.org
> >http://lists.xen.org/xen-devel
> >
> >-
> >No virus found in this message.
> >Checked by AVG - www.avg.com
> >Version: 2014.0.4592 / Virus Database: 3986/7769 - Release Date: 06/30/14
> >
> 
> 
> -- 
> Ross Philipson

Re: [Qemu-devel] [PATCH v2 3/5] net/dump: Separate the NetClientState from the DumpState

[Yang Hongyang]

On 10/13/2015 06:40 PM, Thomas Huth wrote:

With the upcoming dumping-via-netfilter patch, the DumpState
should not be related to NetClientState anymore, so move the
related information to a new struct called DumpNetClient.

Signed-off-by: Thomas Huth 


Reviewed-by: Yang Hongyang 


---
  net/dump.c | 74 +-
  1 file changed, 49 insertions(+), 25 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index e6f6be0..e3e82bd 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -31,7 +31,6 @@
  #include "hub.h"

  typedef struct DumpState {
-NetClientState nc;
  int64_t start_ts;
  int fd;
  int pcap_caplen;
@@ -58,10 +57,8 @@ struct pcap_sf_pkthdr {
  uint32_t len;
  };

-static ssize_t dump_receive_iov(NetClientState *nc, const struct iovec *iov,
-int cnt)
+static ssize_t dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt)
  {
-DumpState *s = DO_UPCAST(DumpState, nc, nc);
  struct pcap_sf_pkthdr hdr;
  int64_t ts;
  int caplen;
@@ -94,30 +91,12 @@ static ssize_t dump_receive_iov(NetClientState *nc, const 
struct iovec *iov,
  return size;
  }

-static ssize_t dump_receive(NetClientState *nc, const uint8_t *buf, size_t 
size)
-{
-struct iovec iov = {
-.iov_base = (void *)buf,
-.iov_len = size
-};
-return dump_receive_iov(nc, , 1);
-}
-
-static void dump_cleanup(NetClientState *nc)
+static void dump_cleanup(DumpState *s)
  {
-DumpState *s = DO_UPCAST(DumpState, nc, nc);
-
  close(s->fd);
+s->fd = -1;
  }

-static NetClientInfo net_dump_info = {
-.type = NET_CLIENT_OPTIONS_KIND_DUMP,
-.size = sizeof(DumpState),
-.receive = dump_receive,
-.receive_iov = dump_receive_iov,
-.cleanup = dump_cleanup,
-};
-
  static int net_dump_state_init(DumpState *s, const char *filename,
 int len, Error **errp)
  {
@@ -154,6 +133,49 @@ static int net_dump_state_init(DumpState *s, const char 
*filename,
  return 0;
  }

+/* Dumping via VLAN netclient */
+
+struct DumpNetClient {
+NetClientState nc;
+DumpState ds;
+};
+typedef struct DumpNetClient DumpNetClient;
+
+static ssize_t dumpclient_receive(NetClientState *nc, const uint8_t *buf,
+  size_t size)
+{
+DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
+struct iovec iov = {
+.iov_base = (void *)buf,
+.iov_len = size
+};
+
+return dump_receive_iov(>ds, , 1);
+}
+
+static ssize_t dumpclient_receive_iov(NetClientState *nc,
+  const struct iovec *iov, int cnt)
+{
+DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
+
+return dump_receive_iov(>ds, iov, cnt);
+}
+
+static void dumpclient_cleanup(NetClientState *nc)
+{
+DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
+
+dump_cleanup(>ds);
+}
+
+static NetClientInfo net_dump_info = {
+.type = NET_CLIENT_OPTIONS_KIND_DUMP,
+.size = sizeof(DumpNetClient),
+.receive = dumpclient_receive,
+.receive_iov = dumpclient_receive_iov,
+.cleanup = dumpclient_cleanup,
+};
+
  int net_init_dump(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
  {
@@ -162,6 +184,7 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
  char def_file[128];
  const NetdevDumpOptions *dump;
  NetClientState *nc;
+DumpNetClient *dnc;

  assert(opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP);
  dump = opts->dump;
@@ -195,7 +218,8 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
  snprintf(nc->info_str, sizeof(nc->info_str),
   "dump to %s (len=%d)", file, len);

-rc = net_dump_state_init(DO_UPCAST(DumpState, nc, nc), file, len, errp);
+dnc = DO_UPCAST(DumpNetClient, nc, nc);
+rc = net_dump_state_init(>ds, file, len, errp);
  if (rc) {
  qemu_del_net_client(nc);
  }



--
Thanks,
Yang.

Re: [Qemu-devel] [PATCH v3 00/32] implement vNVDIMM

[Xiao Guangrong]

On 10/13/2015 02:36 PM, Dan Williams wrote:

On Mon, Oct 12, 2015 at 10:49 PM, Xiao Guangrong
 wrote:



On 10/13/2015 11:38 AM, Dan Williams wrote:


On Mon, Oct 12, 2015 at 8:14 PM, Xiao Guangrong
 wrote:


On 10/13/2015 12:36 AM, Dan Williams wrote:


Static namespaces can be emitted without a label.  Linux needs this to
support existing "label-less" bare metal NVDIMMs.




This is Linux specific? As i did not see it has been documented in the
spec...



I expect most NVDIMMs, especially existing ones available today, do
not have a label area.  This is not Linux specific and ACPI 6 does not
specify a label area, only the Intel DSM Interface Example.



Yup, label data is accessed via DSM interface, the spec I mentioned
is Intel DSM Interface Example.

However, IIRC Linux NVDIMM driver refused to use the device if no
DSM GET_LABEL support, are you going to update it?


Label-less DIMMs are tested as part of the unit test [1] and the
"memmap=nn!ss" kernel parameter that registers a persistent-memory
address range without a DIMM.  What error do you see when label
support is disabled?

[1]: https://github.com/pmem/ndctl/blob/master/README.md



After revert my commits on NVDIMM driver, yeah, it works.

Okay, i will drop the namespace part and make it as label-less
instead.

Thank you, Dan!

Re: [Qemu-devel] [PATCH v2 4/5] net/dump: Provide the dumping facility as a net-filter

[Yang Hongyang]

On 10/13/2015 06:40 PM, Thomas Huth wrote:

Use the net-filter infrastructure to provide the dumping
functions for netdev devices, too.

Signed-off-by: Thomas Huth 


Reviewed-by: Yang Hongyang 



---
  net/dump.c | 129 -
  vl.c   |   7 +++-
  2 files changed, 134 insertions(+), 2 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index e3e82bd..dd0555f 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -28,7 +28,8 @@
  #include "qemu/iov.h"
  #include "qemu/log.h"
  #include "qemu/timer.h"
-#include "hub.h"
+#include "qapi/visitor.h"
+#include "net/filter.h"

  typedef struct DumpState {
  int64_t start_ts;
@@ -225,3 +226,129 @@ int net_init_dump(const NetClientOptions *opts, const 
char *name,
  }
  return rc;
  }
+
+/* Dumping via filter */
+
+#define TYPE_FILTER_DUMP "filter-dump"
+
+#define FILTER_DUMP(obj) \
+OBJECT_CHECK(NetFilterDumpState, (obj), TYPE_FILTER_DUMP)
+
+struct NetFilterDumpState {
+NetFilterState nfs;
+DumpState ds;
+char *filename;
+uint32_t maxlen;
+};
+typedef struct NetFilterDumpState NetFilterDumpState;
+
+static ssize_t filter_dump_receive_iov(NetFilterState *nf, NetClientState 
*sndr,
+   unsigned flags, const struct iovec *iov,
+   int iovcnt, NetPacketSent *sent_cb)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(nf);
+
+dump_receive_iov(>ds, iov, iovcnt);
+return 0;
+}
+
+static void filter_dump_cleanup(NetFilterState *nf)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(nf);
+
+dump_cleanup(>ds);
+}
+
+static void filter_dump_setup(NetFilterState *nf, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(nf);
+
+if (!nfds->filename) {
+error_setg(errp, "dump filter needs 'file' property set!");
+return;
+}
+
+net_dump_state_init(>ds, nfds->filename, nfds->maxlen, errp);
+}
+
+static void filter_dump_get_maxlen(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+uint32_t value = nfds->maxlen;
+
+visit_type_uint32(v, , name, errp);
+}
+
+static void filter_dump_set_maxlen(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+Error *local_err = NULL;
+uint32_t value;
+
+visit_type_uint32(v, , name, _err);
+if (local_err) {
+goto out;
+}
+if (value == 0) {
+error_setg(_err, "Property '%s.%s' doesn't take value '%u'",
+   object_get_typename(obj), name, value);
+goto out;
+}
+nfds->maxlen = value;
+
+out:
+error_propagate(errp, local_err);
+}
+
+static char *file_dump_get_filename(Object *obj, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+
+return g_strdup(nfds->filename);
+}
+
+static void file_dump_set_filename(Object *obj, const char *value, Error 
**errp)
+{
+   NetFilterDumpState *nfds = FILTER_DUMP(obj);
+
+g_free(nfds->filename);
+nfds->filename = g_strdup(value);
+}
+
+static void filter_dump_instance_init(Object *obj)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+
+nfds->maxlen = 65536;
+
+object_property_add(obj, "maxlen", "int", filter_dump_get_maxlen,
+filter_dump_set_maxlen, NULL, NULL, NULL);
+object_property_add_str(obj, "file", file_dump_get_filename,
+file_dump_set_filename, NULL);
+}
+
+static void filter_dump_class_init(ObjectClass *oc, void *data)
+{
+NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+nfc->setup = filter_dump_setup;
+nfc->cleanup = filter_dump_cleanup;
+nfc->receive_iov = filter_dump_receive_iov;
+}
+
+static const TypeInfo filter_dump_info = {
+.name = TYPE_FILTER_DUMP,
+.parent = TYPE_NETFILTER,
+.class_init = filter_dump_class_init,
+.instance_init = filter_dump_instance_init,
+.instance_size = sizeof(NetFilterDumpState),
+};
+
+static void filter_dump_register_types(void)
+{
+type_register_static(_dump_info);
+}
+
+type_init(filter_dump_register_types);
diff --git a/vl.c b/vl.c
index 7c806a2..b54a800 100644
--- a/vl.c
+++ b/vl.c
@@ -2748,7 +2748,12 @@ static bool object_create_initial(const char *type)
  return false;
  }

-if (g_str_equal(type, "filter-buffer")) {
+/*
+ * return false for concrete netfilters since
+ * they depend on netdevs already existing
+ */
+if (g_str_equal(type, "filter-buffer") ||
+g_str_equal(type, "filter-dump")) {
  return false;
  }




--
Thanks,
Yang.

[Qemu-devel] [PATCH 0/2] target-xtensa: two new instructions

[Max Filippov]

Hi,

this series adds two new xtensa instructions:
- s32nb is a new core instruction;
- depbits is enabled by dedicated option.

Max Filippov (2):
  target-xtensa: implement depbits instruction
  target-xtensa: implement S32NB

 target-xtensa/cpu.h  |  1 +
 target-xtensa/overlay_tool.h |  5 +
 target-xtensa/translate.c| 31 +++
 3 files changed, 37 insertions(+)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/2] target-xtensa: implement depbits instruction

[Max Filippov]

This option provides an instruction for depositing a bit field from the
least significant position of one register to an arbitrary position in
another register.

Signed-off-by: Max Filippov 
---
 target-xtensa/cpu.h  |  1 +
 target-xtensa/overlay_tool.h |  5 +
 target-xtensa/translate.c| 20 
 3 files changed, 26 insertions(+)

diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index 96bfc82..a45cb39 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -65,6 +65,7 @@ enum {
 XTENSA_OPTION_MP_SYNCHRO,
 XTENSA_OPTION_CONDITIONAL_STORE,
 XTENSA_OPTION_ATOMCTL,
+XTENSA_OPTION_DEPBITS,
 
 /* Interrupts and exceptions */
 XTENSA_OPTION_EXCEPTION,
diff --git a/target-xtensa/overlay_tool.h b/target-xtensa/overlay_tool.h
index eda03aa..e8a7fda 100644
--- a/target-xtensa/overlay_tool.h
+++ b/target-xtensa/overlay_tool.h
@@ -30,6 +30,10 @@
 { .targno = (no), .type = (typ), .group = (grp), .size = (sz) },
 #define XTREG_END { .targno = -1 },
 
+#ifndef XCHAL_HAVE_DEPBITS
+#define XCHAL_HAVE_DEPBITS 0
+#endif
+
 #ifndef XCHAL_HAVE_DIV32
 #define XCHAL_HAVE_DIV32 0
 #endif
@@ -69,6 +73,7 @@
 XCHAL_OPTION(XCHAL_HAVE_S32C1I, XTENSA_OPTION_CONDITIONAL_STORE) | \
 XCHAL_OPTION(XCHAL_HAVE_S32C1I && XCHAL_HW_MIN_VERSION >= 23, \
 XTENSA_OPTION_ATOMCTL) | \
+XCHAL_OPTION(XCHAL_HAVE_DEPBITS, XTENSA_OPTION_DEPBITS) | \
 /* Interrupts and exceptions */ \
 XCHAL_OPTION(XCHAL_HAVE_EXCEPTIONS, XTENSA_OPTION_EXCEPTION) | \
 XCHAL_OPTION(XCHAL_HAVE_VECBASE, XTENSA_OPTION_RELOCATABLE_VECTOR) | \
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index f2118c2..cd2148e 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -1970,6 +1970,16 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 break;
 
 case 10: /*FP0*/
+/*DEPBITS*/
+if (option_enabled(dc, XTENSA_OPTION_DEPBITS)) {
+if (!gen_window_check2(dc, RRR_S, RRR_T)) {
+break;
+}
+tcg_gen_deposit_i32(cpu_R[RRR_T], cpu_R[RRR_T], cpu_R[RRR_S],
+OP2, RRR_R + 1);
+break;
+}
+
 HAS_OPTION(XTENSA_OPTION_FP_COPROCESSOR);
 switch (OP2) {
 case 0: /*ADD.Sf*/
@@ -2104,6 +2114,16 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 break;
 
 case 11: /*FP1*/
+/*DEPBITS*/
+if (option_enabled(dc, XTENSA_OPTION_DEPBITS)) {
+if (!gen_window_check2(dc, RRR_S, RRR_T)) {
+break;
+}
+tcg_gen_deposit_i32(cpu_R[RRR_T], cpu_R[RRR_T], cpu_R[RRR_S],
+OP2 + 16, RRR_R + 1);
+break;
+}
+
 HAS_OPTION(XTENSA_OPTION_FP_COPROCESSOR);
 
 #define gen_compare(rel, br, a, b) \
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/2] target-xtensa: implement S32NB

[Max Filippov]

S32NB provides the same functionality as S32I with two exceptions.
First, when its operation leaves the processor, the external transaction
is marked Non-Bufferable. Second, it may not be used to write to
Instruction RAM.
In QEMU S32NB is equivalent to S32I.

Signed-off-by: Max Filippov 
---
 target-xtensa/translate.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index cd2148e..9b6033d 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -1963,6 +1963,17 @@ static void disas_xtensa_insn(CPUXtensaState *env, 
DisasContext *dc)
 }
 break;
 
+case 5: /*S32N*/
+if (gen_window_check2(dc, RRI4_S, RRI4_T)) {
+TCGv_i32 addr = tcg_temp_new_i32();
+
+tcg_gen_addi_i32(addr, cpu_R[RRI4_S], RRI4_IMM4 << 2);
+gen_load_store_alignment(dc, 2, addr, false);
+tcg_gen_qemu_st32(cpu_R[RRI4_T], addr, dc->cring);
+tcg_temp_free(addr);
+}
+break;
+
 default:
 RESERVED();
 break;
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 2/5] net/dump: Rework net-dump init functions

[Yang Hongyang]

On 10/13/2015 06:39 PM, Thomas Huth wrote:

Move the creation of the dump client from net_dump_init() into
net_init_dump(), so we can later use the former function for
dump via netfilter, too. Also rename net_dump_init() to
net_dump_state_init() to make it easier distinguishable from
net_init_dump().

Signed-off-by: Thomas Huth 



Reviewed-by: Yang Hongyang 


---
  net/dump.c | 27 +--
  1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index aa0d45d..e6f6be0 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -118,13 +118,10 @@ static NetClientInfo net_dump_info = {
  .cleanup = dump_cleanup,
  };

-static int net_dump_init(NetClientState *peer, const char *device,
- const char *name, const char *filename, int len,
- Error **errp)
+static int net_dump_state_init(DumpState *s, const char *filename,
+   int len, Error **errp)
  {
  struct pcap_file_hdr hdr;
-NetClientState *nc;
-DumpState *s;
  struct tm tm;
  int fd;

@@ -148,13 +145,6 @@ static int net_dump_init(NetClientState *peer, const char 
*device,
  return -1;
  }

-nc = qemu_new_net_client(_dump_info, peer, device, name);
-
-snprintf(nc->info_str, sizeof(nc->info_str),
- "dump to %s (len=%d)", filename, len);
-
-s = DO_UPCAST(DumpState, nc, nc);
-
  s->fd = fd;
  s->pcap_caplen = len;

@@ -167,10 +157,11 @@ static int net_dump_init(NetClientState *peer, const char 
*device,
  int net_init_dump(const NetClientOptions *opts, const char *name,
NetClientState *peer, Error **errp)
  {
-int len;
+int len, rc;
  const char *file;
  char def_file[128];
  const NetdevDumpOptions *dump;
+NetClientState *nc;

  assert(opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP);
  dump = opts->dump;
@@ -200,5 +191,13 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
  len = 65536;
  }

-return net_dump_init(peer, "dump", name, file, len, errp);
+nc = qemu_new_net_client(_dump_info, peer, "dump", name);
+snprintf(nc->info_str, sizeof(nc->info_str),
+ "dump to %s (len=%d)", file, len);
+
+rc = net_dump_state_init(DO_UPCAST(DumpState, nc, nc), file, len, errp);
+if (rc) {
+qemu_del_net_client(nc);
+}
+return rc;
  }



--
Thanks,
Yang.

Re: [Qemu-devel] [PATCH v3 0/2] PCI-e device multi-function hot-add support

[Cao jin]

On 10/13/2015 09:10 PM, Michael S. Tsirkin wrote:

On Tue, Oct 13, 2015 at 07:54:07PM +0800, Cao jin wrote:



On 10/13/2015 04:49 PM, Michael S. Tsirkin wrote:

On Tue, Oct 13, 2015 at 04:41:33PM +0800, Cao jin wrote:

Support PCI-e device hot-add multi-function via device_add, just ensure
add the function 0 is added last. While allow user to roll back in the
middle via device_del, in case user regret.


This patch doesn't seem to account of AIR though.



Yes, but the AIR function seems never be used(nobody calls the function
pcie_ari_init()), so I am a little confused about should it be consindered?


Yes please - we'll likely use that in the future. Pls add an API
that takes ari into account.


Ok, I am on it


changelog:
1. Flag device as unexposed when func 0 doesn`t exist, via return 0xFF
in case of gratuitous pci bus scan.
2. Since device is unexposed to guest, can remove function individually,
without interaction with the guest.

Cao jin (2):
   enable multi-function hot-add
   remove function during multi-function hot-add

  hw/pci/pci.c  | 10 ++
  hw/pci/pci_host.c |  6 +-
  hw/pci/pcie.c | 38 +-
  3 files changed, 40 insertions(+), 14 deletions(-)

--
2.1.0

.



--
Yours Sincerely,

Cao Jin

.



--
Yours Sincerely,

Cao Jin

Re: [Qemu-devel] [PATCH v3 2/2] remove function during multi-function hot-add

[Cao jin]

Hi, Alex

On 10/13/2015 11:27 PM, Alex Williamson wrote:

On Tue, 2015-10-13 at 16:41 +0800, Cao jin wrote:

In case user regret when hot-adding multi-function, should roll back,
device_del the function added but not exposed to the guest.


As Michael suggests, this patch should come first, before we actually
enable multi-function hot-add.


OK.

Signed-off-by: Cao jin 
---
  hw/pci/pci_host.c |  6 +-
  hw/pci/pcie.c | 22 +-
  2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 3e26f92..35e5cf3 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -20,6 +20,7 @@

  #include "hw/pci/pci.h"
  #include "hw/pci/pci_host.h"
+#include "hw/pci/pci_bus.h"
  #include "trace.h"

  /* debug PCI */
@@ -88,10 +89,13 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, 
int len)
  uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
  {
  PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
+PCIDevice *f0 = NULL;
  uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
  uint32_t val;
+uint8_t slot = (addr >> 11) & 0x1F;

-if (!pci_dev) {
+f0 = s->devices[PCI_DEVFN(slot, 0)];
+if (!pci_dev || (!f0 && pci_dev)) {



This uses a lot more variables and operations than it needs to:

if (!pci_dev || !s->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]) {


Ok. variables is intended to make the line shorter.


Shouldn't we do the same on pci_data_write()?  A well behaved guest
won't blindly write to config space, but not all guests are well
behaved.



Yup, agree. I missed the consideration of bad behavior. I thought anyone 
use the device should read the vendor ID first(good behavior), then do 
anything he/she want. Thanks for reminding



Comments in the code would be nice here to explain that non-zero
functions are only exposed when function zero is present, allowing
direct removal of unexposed devices.


OK


I imagine that due to qemu locking that we don't have a race here, but
note that devices[] is populated early in the core pci realize function,
prior to the device initialize function, and there are any number of
reasons that failure could still occur, which would create a window
where the function is accessible.  I doubt this is an issue, but simply
note it for completeness.


Ok, will consider the "function access window" condition, to see what I 
can do with it



  return ~0x0;
  }

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 89bf61b..58d2153 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -261,13 +261,30 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler 
*hotplug_dev, DeviceState *dev,
  }
  }

+static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
+{
+object_unparent(OBJECT(dev));
+}
+
  void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
   DeviceState *dev, Error **errp)
  {
  uint8_t *exp_cap;
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+PCIBus *bus = pci_dev->bus;

  pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, _cap, 
errp);

+/* In case user regret when hot-adding multi function, remove the function
+ * that is unexposed to guest individually, without interaction with guest.
+ */
+if (PCI_FUNC(pci_dev->devfn) > 0 &&
+bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)] == NULL) {


Similarly,

if (PCI_FUNC(pci_dev->devfn) && 
!bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]) {



Ok


+pcie_unplug_device(bus, pci_dev, NULL);
+
+return;
+}
+
  pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
  }

@@ -378,11 +395,6 @@ void pcie_cap_slot_reset(PCIDevice *dev)
  hotplug_event_update_event_status(dev);
  }

-static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
-{
-object_unparent(OBJECT(dev));
-}
-
  void pcie_cap_slot_write_config(PCIDevice *dev,
  uint32_t addr, uint32_t val, int len)
  {




.



--
Yours Sincerely,

Cao Jin

Re: [Qemu-devel] [PATCH v3 00/32] implement vNVDIMM

[Dan Williams]

On Mon, Oct 12, 2015 at 10:49 PM, Xiao Guangrong
 wrote:
>
>
> On 10/13/2015 11:38 AM, Dan Williams wrote:
>>
>> On Mon, Oct 12, 2015 at 8:14 PM, Xiao Guangrong
>>  wrote:
>>>
>>> On 10/13/2015 12:36 AM, Dan Williams wrote:

 Static namespaces can be emitted without a label.  Linux needs this to
 support existing "label-less" bare metal NVDIMMs.
>>>
>>>
>>>
>>> This is Linux specific? As i did not see it has been documented in the
>>> spec...
>>
>>
>> I expect most NVDIMMs, especially existing ones available today, do
>> not have a label area.  This is not Linux specific and ACPI 6 does not
>> specify a label area, only the Intel DSM Interface Example.
>>
>
> Yup, label data is accessed via DSM interface, the spec I mentioned
> is Intel DSM Interface Example.
>
> However, IIRC Linux NVDIMM driver refused to use the device if no
> DSM GET_LABEL support, are you going to update it?

Label-less DIMMs are tested as part of the unit test [1] and the
"memmap=nn!ss" kernel parameter that registers a persistent-memory
address range without a DIMM.  What error do you see when label
support is disabled?

[1]: https://github.com/pmem/ndctl/blob/master/README.md

Re: [Qemu-devel] [Bug 1504513] [NEW] Socket leak on each call to qemu_socket()

[Markus Armbruster]

Mark Pizzolato - Info Comm  writes:

> On Sunday, October 11, 2015 at 11:58 PM. Markus Armbruster wrote:
>> Mark Pizzolato  writes:
>> 
>> > Public bug reported:
>> >
>> > On any host platform where SOCK_CLOEXEC is defined (Linux at least), a
>> > socket is leaked on each call to qemu_socket() AND the socket returned
>> > hasn't been created with the desired SOCK_CLOEXEC attribute.  The
>> > qemu_socket routine is:
>> >
>> > Line 272 of util/osdep.c:
>> > /*
>> >  * Opens a socket with FD_CLOEXEC set
>> >  */
>> > int qemu_socket(int domain, int type, int protocol)
>> > {
>> > int ret;
>> >
>> > #ifdef SOCK_CLOEXEC
>> > ret = socket(domain, type | SOCK_CLOEXEC, protocol);
>> > if (ret != -1 || errno != EINVAL) {
>> > return ret;
>> 
>> If socket() succeeded (ret != -1), we return the socket.
>> 
>> If socket() failed with anything but EINVAL (ret == -1 && errno !=
>> EINVAL), we return -1 with errno set.
>> 
>> > }
>> 
>> Here, ret == -1 && errno == EINVAL.
>> 
>> > #endif
>> > ret = socket(domain, type, protocol);
>> > if (ret >= 0) {
>> > qemu_set_cloexec(ret);
>> > }
>> >
>> > return ret;
>> > }
>> 
>> How can this leak a socket?
>> 
>> How can this return a socket with FD_CLOEXEC not set?
>
> All I can say is "OOPS!!"  Sorry for bothering you.  I misread the
> status check after the first socket() call.
>
> I'm in the process of lifting qemu's slirp code and dropping it into
> another open source project.  Since I'm trying to use all the code in
> the slirp directory without modification I'm digging through where it
> now depends on other qemu code.  I quickly looked at the qemu_socket()
> routine and read it wrong.
>
> Once again, sorry.

Happens to all of us from time to time :)

Re: [Qemu-devel] [PATCH v3 2/4] block: auto-generated node-names

[Markus Armbruster]

Jeff Cody  writes:

> If a node-name is not specified, automatically generate the node-name.
>
> Generated node-names will use the "block" sub-system identifier.
>
> Reviewed-by: Eric Blake 
> Reviewed-by: John Snow 
> Signed-off-by: Jeff Cody 
> ---
>  block.c | 19 ---
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/block.c b/block.c
> index 1f90b47..5947704 100644
> --- a/block.c
> +++ b/block.c
> @@ -763,12 +763,15 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
>const char *node_name,
>Error **errp)
>  {
> +char *gen_node_name = NULL;
> +
>  if (!node_name) {
> -return;
> -}
> -
> -/* Check for empty string or invalid characters */
> -if (!id_wellformed(node_name)) {
> +node_name = gen_node_name = id_generate(ID_BLOCK);
> +} else if (!id_wellformed(node_name)) {
> +/*
> + * Check for empty string or invalid characters, but not if it is
> + * generated (generated names use characters not available to the 
> user)
> + */
>  error_setg(errp, "Invalid node name");
>  return;
>  }

I'd drop the comment, as the code is obvious enough now.  Could be done
on commit.

> @@ -777,18 +780,20 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
>  if (blk_by_name(node_name)) {
>  error_setg(errp, "node-name=%s is conflicting with a device id",
> node_name);
> -return;
> +goto out;
>  }
>  
>  /* takes care of avoiding duplicates node names */
>  if (bdrv_find_node(node_name)) {
>  error_setg(errp, "Duplicate node name");
> -return;
> +goto out;
>  }
>  
>  /* copy node name into the bs and insert it into the graph list */
>  pstrcpy(bs->node_name, sizeof(bs->node_name), node_name);
>  QTAILQ_INSERT_TAIL(_bdrv_states, bs, node_list);
> +out:
> +g_free(gen_node_name);
>  }
>  
>  static QemuOptsList bdrv_runtime_opts = {

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH v3 00/32] implement vNVDIMM

[Dan Williams]

On Mon, Oct 12, 2015 at 8:14 PM, Xiao Guangrong
 wrote:
> On 10/13/2015 12:36 AM, Dan Williams wrote:
>> Static namespaces can be emitted without a label.  Linux needs this to
>> support existing "label-less" bare metal NVDIMMs.
>
>
> This is Linux specific? As i did not see it has been documented in the
> spec...

I expect most NVDIMMs, especially existing ones available today, do
not have a label area.  This is not Linux specific and ACPI 6 does not
specify a label area, only the Intel DSM Interface Example.

[Qemu-devel] [PATCH] hw/arm/virt: Allow zero address for PCI IO space

[Alexander Gordeev]

Currently PCI IO address 0 is not allowed even though
the IO space starts from 0. As result, PCI IO is not
possible to use at all.

CC: Peter Maydell 
CC: Andrew Jones 
Signed-off-by: Alexander Gordeev 
---
 hw/arm/virt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d25d6cf..3620489 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1157,6 +1157,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
 mc->has_dynamic_sysbus = true;
 mc->block_default_type = IF_VIRTIO;
 mc->no_cdrom = 1;
+mc->pci_allow_0_address = true;
 }
 
 static const TypeInfo machvirt_info = {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/2] kvm/x86: Hyper-V kvm exit

[KY Srinivasan]

> -Original Message-
> From: Denis V. Lunev [mailto:d...@openvz.org]
> Sent: Monday, October 12, 2015 6:46 AM
> To: Eric Blake 
> Cc: Gleb Natapov ; qemu-devel@nongnu.org;
> virtualizat...@lists.linux-foundation.org; rka...@virtuozzo.com; Paolo
> Bonzini ; Andrey Smetanin
> ; Vitaly Kuznetsov ; KY
> Srinivasan 
> Subject: Re: [Qemu-devel] [PATCH 2/2] kvm/x86: Hyper-V kvm exit
> 
> On 10/12/2015 04:42 PM, Eric Blake wrote:
> > On 10/09/2015 07:39 AM, Denis V. Lunev wrote:
> >> From: Andrey Smetanin 
> >>
> >> A new vcpu exit is introduced to notify the userspace of the
> >> changes in Hyper-V synic configuraion triggered by guest writing to the
> > s/configuraion/configuration/
> > Is 'synic' intended?  Is it short for something (if so, spelling it out
> > may help)?
> >
> >
> >> +++ b/Documentation/virtual/kvm/api.txt
> >> @@ -3331,6 +3331,12 @@ the userspace IOAPIC should process the EOI
> and retrigger the interrupt if
> >>   it is still asserted.  Vector is the LAPIC interrupt vector for which the
> >>   EOI was received.
> >>
> >> +  /* KVM_EXIT_HYPERV */
> >> +struct kvm_hyperv_exit hyperv;
> >> +Indicates that the VCPU's exits into userspace to process some tasks
> > s/VCPU's/VCPU/
> >
> >> +related with Hyper-V emulation. Currently used to synchronize modified
> >> +Hyper-V synic state with userspace.
> > Again, is 'synic' intended?  Hmm, I see it throughout the patch, so it
> > looks intentional, but I keep trying to read it as a typo for 'sync'.
> >
> this is not a typo :)

Yes; the Hyper-V public functional spec has chosen this name;
it stands for Synthetic Interrupt Controller.

K. Y

Re: [Qemu-devel] [PATCH RFC] hw/virtio: Add PCIe capability to virtio devices

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 04:41:35PM +0800, Cao jin wrote:
> In case user regret when hot-adding multi-function, should roll back,
> device_del the function added but not exposed to the guest.
> 
> Signed-off-by: Cao jin 
> ---
>  hw/pci/pci_host.c |  6 +-
>  hw/pci/pcie.c | 22 +-
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index 3e26f92..35e5cf3 100644
> --- a/hw/pci/pci_host.c
> +++ b/hw/pci/pci_host.c
> @@ -20,6 +20,7 @@
>  
>  #include "hw/pci/pci.h"
>  #include "hw/pci/pci_host.h"
> +#include "hw/pci/pci_bus.h"
>  #include "trace.h"
>  
>  /* debug PCI */
> @@ -88,10 +89,13 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t 
> val, int len)
>  uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
>  {
>  PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
> +PCIDevice *f0 = NULL;
>  uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
>  uint32_t val;
> +uint8_t slot = (addr >> 11) & 0x1F;
>  
> -if (!pci_dev) {
> +f0 = s->devices[PCI_DEVFN(slot, 0)];
> +if (!pci_dev || (!f0 && pci_dev)) {
>  return ~0x0;
>  }
>

This seems to belong to the previous patch?
  
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 89bf61b..58d2153 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -261,13 +261,30 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>  }
>  }
>  
> +static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> +{
> +object_unparent(OBJECT(dev));
> +}
> +
>  void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
>   DeviceState *dev, Error **errp)
>  {
>  uint8_t *exp_cap;
> +PCIDevice *pci_dev = PCI_DEVICE(dev);
> +PCIBus *bus = pci_dev->bus;
>  
>  pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, _cap, 
> errp);
>  
> +/* In case user regret when hot-adding multi function, remove the 
> function
> + * that is unexposed to guest individually, without interaction with 
> guest.
> + */
> +if (PCI_FUNC(pci_dev->devfn) > 0 &&
> +bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)] == NULL) {

Shorter: PCI_FUNC(pci_dev->devfn) &&
!bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)]


> +pcie_unplug_device(bus, pci_dev, NULL);
> +
> +return;
> +}
> +
>  pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
>  }
>  
> @@ -378,11 +395,6 @@ void pcie_cap_slot_reset(PCIDevice *dev)
>  hotplug_event_update_event_status(dev);
>  }
>  
> -static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> -{
> -object_unparent(OBJECT(dev));
> -}
> -
>  void pcie_cap_slot_write_config(PCIDevice *dev,
>  uint32_t addr, uint32_t val, int len)
>  {
> -- 
> 2.1.0

Re: [Qemu-devel] [PATCH v3 13/16] block: Implement bdrv_append() without bdrv_swap()

[Alberto Garcia]

On Tue 13 Oct 2015 10:39:22 AM CEST, Kevin Wolf  wrote:

>> > +static void change_parent_backing_link(BlockDriverState *from,
>> > +   BlockDriverState *to)
>> > +{
>> > +BdrvChild *c, *next;
>> > +
>> > +QLIST_FOREACH_SAFE(c, >parents, next_parent, next) {
>> > +assert(c->role != _backing);
>> > +c->bs = to;
>> > +QLIST_REMOVE(c, next_parent);
>> > +QLIST_INSERT_HEAD(>parents, c, next_parent);
>> > +bdrv_ref(to);
>> > +bdrv_unref(from);
>> > +}
>> > +if (from->blk) {
>> > +blk_set_bs(from->blk, to);
>> > +if (!to->device_list.tqe_prev) {
>> > +QTAILQ_INSERT_BEFORE(from, to, device_list);
>> > +}
>> 
>> Is it even possible that this last condition is false? In what case
>> would 'to' be already in bdrv_states?
>> 
>> I understand that it would mean that it would already be attached to
>> a BlockBackend, but that's not possible in this case.
>
> Yes, I think it's not possible currently (hopefully, because that
> would cause other bugs), just being careful. Eventually we'll allow
> more than one BlockBackend pointing to the same BDS, and then this is
> a scenario that could happen.

blk_set_bs() already asserts that to->blk == NULL, so if this is not
possible here wouldn't it be better to use another assertion instead?
When I was reviewing the code I kept wondering in what kind of scenario
this condition could be false.

Berto

Re: [Qemu-devel] [PATCH v7 03/14] qapi: Drop redundant alternate-good test

[Markus Armbruster]

Eric Blake  writes:

> On 10/07/2015 10:15 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> The alternate-good.json test was already covered by
>>> qapi-schema-test.json.  As future commits will be tweaking
>>> how alternates are laid out, removing the duplicate test now
>>> reduces churn.
>> 
>> Do we have more positive tests?  They should probably all live in
>> qapi-schema-test.json, because there their generated code actually gets
>> compiled.
>
> Hmm, any test that has an empty .err and non-empty .out, but which does
> not also have an TODO/FIXME stating that it is a bug, is worth checking.
>  So first, here's the list of non-empty .out files:
>
> empty.out [1]
> enum-empty.out
> event-case.out [4]
> union-empty.out [2]
> include-simple.out
> include-repetition.out
> include-relpath.out
> comments.out
> alternate-empty.out [2]
> returns-int.out
> struct-base-clash-base.out [3]
> flat-union-empty.out [2]
> indented-expr.out
> union-clash-data.out [3]
> ident-with-escape.out
> args-member-array.out
> flat-union-reverse-define.out
>
> [1] must stay separate, because we can't really make qapi-schema-test
> empty :)
> [2] marked FIXME, and turns into proper error message later in the v5 series
> [3] marked FIXME, and already removed later in the v5 series once a
> positive test is inlined into qapi-schema-test
> [4] marked TODO
>
> For the ones I didn't tag with a footnote, we could probably cover
> things directly in qapi-schema-test instead; the question then becomes
> which ones are already covered, and which ones need content moved.

Actually, the point isn't to move the positive test to
qapi-schema-test.json, the point is to compile-test its generated code.
Moving it to qapi-schema-test.json accomplishes that.  However, we may
not want a single, monolithic positive test.  Should we split up
qapi-schema-test.json instead?  I don't know.  Anyway, let's flush our
queue first.

Third case: the generated code isn't worth compile-testing; comparing
actual to expected .out suffices.

Let's sort your untagged tests into buckets:

Not worth compile-testing:
* comments.json
* include-simple.json
* include-repetition.json
* include-relpath.json
* indented-expr.json
* ident-with-escape.json

Not (completely) covered in qapi-schema-test.json:
* enum-empty.json
  Not covered, but should be.
* flat-union-reverse-define.json
  UserDefOne covers forward reference to struct base, UserDefFlatUnion
  covers forward reference to union base, and UserDefFlatUnion2 covers
  forward reference to member.  We may want to cover forward reference
  to the tag member's type.

Covered:
* returns-int.json
  'user_def_cmd3' does the job.
* args-member-array.json
  '__org.qemu_x-command' seems good enough.

[Qemu-devel] [PATCH v3 1/2] enable multi-function hot-add

[Cao jin]

Just ensure the function 0 added last, then driver will got the notification
to scan all the function in the slot.

Signed-off-by: Cao jin 
---
 hw/pci/pci.c  | 10 ++
 hw/pci/pcie.c | 18 +-
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ccea628..15b53d9 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -847,6 +847,7 @@ static PCIDevice *do_pci_register_device(PCIDevice 
*pci_dev, PCIBus *bus,
 PCIConfigWriteFunc *config_write = pc->config_write;
 Error *local_err = NULL;
 AddressSpace *dma_as;
+DeviceState *dev = DEVICE(pci_dev);
 
 if (devfn < 0) {
 for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
@@ -864,6 +865,15 @@ static PCIDevice *do_pci_register_device(PCIDevice 
*pci_dev, PCIBus *bus,
PCI_SLOT(devfn), PCI_FUNC(devfn), name,
bus->devices[devfn]->name);
 return NULL;
+} else if (dev->hotplugged &&
+   bus->devices[PCI_DEVFN(PCI_SLOT(devfn), 0)]) {
+error_setg(errp, "PCI: slot %d function 0 already ocuppied by %s,"
+   " new func %s cannot be exposed to guest.",
+   PCI_SLOT(devfn),
+   bus->devices[PCI_DEVFN(PCI_SLOT(devfn), 0)]->name,
+   name);
+
+   return NULL;
 }
 
 pci_dev->bus = bus;
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 6e28985..89bf61b 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -249,16 +249,16 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 return;
 }
 
-/* TODO: multifunction hot-plug.
- * Right now, only a device of function = 0 is allowed to be
- * hot plugged/unplugged.
+/* To enable multifunction hot-plug, we just ensure the function
+ * 0 added last. Until function 0 added, we set the sltsta and
+ * inform OS via event notification.
  */
-assert(PCI_FUNC(pci_dev->devfn) == 0);
-
-pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
-   PCI_EXP_SLTSTA_PDS);
-pcie_cap_slot_event(PCI_DEVICE(hotplug_dev),
-PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP);
+if (PCI_FUNC(pci_dev->devfn) == 0) {
+pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
+   PCI_EXP_SLTSTA_PDS);
+pcie_cap_slot_event(PCI_DEVICE(hotplug_dev),
+PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP);
+}
 }
 
 void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
-- 
2.1.0

[Qemu-devel] [PATCH v3 0/2] PCI-e device multi-function hot-add support

[Cao jin]

Support PCI-e device hot-add multi-function via device_add, just ensure
add the function 0 is added last. While allow user to roll back in the
middle via device_del, in case user regret.

changelog:
1. Flag device as unexposed when func 0 doesn`t exist, via return 0xFF
   in case of gratuitous pci bus scan.
2. Since device is unexposed to guest, can remove function individually,
   without interaction with the guest.

Cao jin (2):
  enable multi-function hot-add
  remove function during multi-function hot-add

 hw/pci/pci.c  | 10 ++
 hw/pci/pci_host.c |  6 +-
 hw/pci/pcie.c | 38 +-
 3 files changed, 40 insertions(+), 14 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v3 2/2] remove function during multi-function hot-add

[Cao jin]

In case user regret when hot-adding multi-function, should roll back,
device_del the function added but not exposed to the guest.

Signed-off-by: Cao jin 
---
 hw/pci/pci_host.c |  6 +-
 hw/pci/pcie.c | 22 +-
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 3e26f92..35e5cf3 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -20,6 +20,7 @@
 
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_host.h"
+#include "hw/pci/pci_bus.h"
 #include "trace.h"
 
 /* debug PCI */
@@ -88,10 +89,13 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, 
int len)
 uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
 {
 PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
+PCIDevice *f0 = NULL;
 uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
 uint32_t val;
+uint8_t slot = (addr >> 11) & 0x1F;
 
-if (!pci_dev) {
+f0 = s->devices[PCI_DEVFN(slot, 0)];
+if (!pci_dev || (!f0 && pci_dev)) {
 return ~0x0;
 }
 
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 89bf61b..58d2153 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -261,13 +261,30 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 }
 }
 
+static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
+{
+object_unparent(OBJECT(dev));
+}
+
 void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
  DeviceState *dev, Error **errp)
 {
 uint8_t *exp_cap;
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+PCIBus *bus = pci_dev->bus;
 
 pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, _cap, errp);
 
+/* In case user regret when hot-adding multi function, remove the function
+ * that is unexposed to guest individually, without interaction with guest.
+ */
+if (PCI_FUNC(pci_dev->devfn) > 0 &&
+bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)] == NULL) {
+pcie_unplug_device(bus, pci_dev, NULL);
+
+return;
+}
+
 pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
 }
 
@@ -378,11 +395,6 @@ void pcie_cap_slot_reset(PCIDevice *dev)
 hotplug_event_update_event_status(dev);
 }
 
-static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
-{
-object_unparent(OBJECT(dev));
-}
-
 void pcie_cap_slot_write_config(PCIDevice *dev,
 uint32_t addr, uint32_t val, int len)
 {
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3 0/2] PCI-e device multi-function hot-add support

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 04:41:33PM +0800, Cao jin wrote:
> Support PCI-e device hot-add multi-function via device_add, just ensure
> add the function 0 is added last. While allow user to roll back in the
> middle via device_del, in case user regret.

s/regret/cancels the operation/


> changelog:
> 1. Flag device as unexposed when func 0 doesn`t exist, via return 0xFF
>in case of gratuitous pci bus scan.
> 2. Since device is unexposed to guest, can remove function individually,
>without interaction with the guest.
> 
> Cao jin (2):
>   enable multi-function hot-add
>   remove function during multi-function hot-add
> 
>  hw/pci/pci.c  | 10 ++
>  hw/pci/pci_host.c |  6 +-
>  hw/pci/pcie.c | 38 +-
>  3 files changed, 40 insertions(+), 14 deletions(-)
> 
> -- 
> 2.1.0

Re: [Qemu-devel] [RFH PATCH 0/4] record/replay fixups and doubts

[Pavel Dovgaluk]

There is one more fix.
Sometimes replay cannot continue after stopping/restarting of the virtual 
machine.
This happens because warp on stopped machine and on running machine behaves 
differently.
Timers deadline calculation depends on enabled flag of the virtual timer.
The following patch fixes the problem - it disables warp when machine is 
stopped.

index 5130806..7a337d9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -411,7 +411,7 @@ void qemu_clock_warp(QEMUClockType type)
 }

 /* warp clock deterministically in record/replay mode */
-if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP)) {
+if (!runstate_is_running() || !replay_checkpoint(CHECKPOINT_CLOCK_WARP)) {
 return;
 }

Pavel Dovgalyuk


> -Original Message-
> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo 
> Bonzini
> Sent: Tuesday, October 06, 2015 11:01 PM
> To: qemu-devel@nongnu.org
> Cc: pavel.dovga...@ispras.ru
> Subject: [RFH PATCH 0/4] record/replay fixups and doubts
> 
> These are some comments I have about the record/replay code.  I can
> integrate these in your patches myself, but I need an ack/tested-by and
> in some case more answers...  Please take a look.
> 
> Paolo Bonzini (4):
>   replay: generalize ptimer event to bottom halves
>   more replay fixes
>   why is runstate_is_running needed?
>   events doubts
> 
>  Makefile.objs   |  2 ++
>  Makefile.target |  1 -
>  cpu-exec.c  |  2 +-
>  cpus.c  |  2 +-
>  exec.c  |  2 +-
>  hw/bt/hci.c |  4 ++--
>  hw/core/ptimer.c|  8 ++--
>  include/qapi/qmp/qerror.h   |  2 +-
>  {replay => include/sysemu}/replay.h |  4 ++--
>  qapi/common.json|  6 +-
>  qemu-timer.c| 23 +--
>  replay/Makefile.objs| 11 +--
>  replay/replay-events.c  | 24 +++-
>  replay/replay-input.c   |  2 +-
>  replay/replay-internal.c|  4 ++--
>  replay/replay-internal.h|  2 +-
>  replay/replay-time.c|  2 +-
>  replay/replay.c |  2 +-
>  stubs/Makefile.objs |  1 +
>  {replay => stubs}/replay-user.c |  6 +-
>  stubs/replay.c  | 10 +-
>  ui/input.c  |  2 +-
>  vl.c|  6 +++---
>  23 files changed, 59 insertions(+), 69 deletions(-)
>  rename {replay => include/sysemu}/replay.h (97%)
>  mode change 100755 => 100644 replay/Makefile.objs
>  mode change 100755 => 100644 replay/replay-events.c
>  mode change 100755 => 100644 replay/replay-input.c
>  mode change 100755 => 100644 replay/replay-internal.c
>  mode change 100755 => 100644 replay/replay-internal.h
>  mode change 100755 => 100644 replay/replay-time.c
>  mode change 100755 => 100644 replay/replay.c
>  rename {replay => stubs}/replay-user.c (90%)
> 
> --
> 2.5.0

Re: [Qemu-devel] [PATCH v3 07/16] block: Convert bs->backing_hd to BdrvChild

[Kevin Wolf]

Am 13.10.2015 um 03:53 hat Jeff Cody geschrieben:
> On Fri, Oct 09, 2015 at 02:15:32PM +0200, Kevin Wolf wrote:
> > This is the final step in converting all of the BlockDriverState
> > pointers that block drivers use to BdrvChild.
> > 
> > After this patch, bs->children contains the full list of child nodes
> > that are referenced by a given BDS, and these children are only
> > referenced through BdrvChild, so that updating the pointer in there is
> > enough for changing edges in the graph.
> > 
> > Signed-off-by: Kevin Wolf 
> > ---
> >  block.c   | 105 
> > +++---
> >  block/io.c|  24 +--
> >  block/mirror.c|   6 +--
> >  block/qapi.c  |   8 ++--
> >  block/qcow.c  |   4 +-
> >  block/qcow2-cluster.c |   4 +-
> >  block/qcow2.c |   6 +--
> >  block/qed.c   |  12 +++---
> >  block/stream.c|   8 ++--
> >  block/vmdk.c  |  21 +-
> >  block/vvfat.c |   6 +--
> >  blockdev.c|   4 +-
> >  include/block/block_int.h |  12 --
> >  qemu-img.c|   4 +-
> >  14 files changed, 115 insertions(+), 109 deletions(-)
> > 
> 
> [...]
> 
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index 98936c9..90971c0 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -378,8 +378,7 @@ struct BlockDriverState {
> >  QDict *full_open_options;
> >  char exact_filename[PATH_MAX];
> >  
> > -BlockDriverState *backing_hd;
> > -BdrvChild *backing_child;
> > +BdrvChild *backing;
> >  BdrvChild *file;
> >  
> >  NotifierList close_notifiers;
> > @@ -458,6 +457,11 @@ struct BlockDriverState {
> >  NotifierWithReturn write_threshold_notifier;
> >  };
> >  
> > +static inline BlockDriverState *backing_bs(BlockDriverState *bs)
> > +{
> > +return bs->backing ? bs->backing->bs : NULL;
> > +}
> > +
> 
> Is there a good guideline regarding when you prefer backing_bs() to be
> used, over accessing bs->backing->bs directly?  There seems to be a
> lot of mixed usage left in this patch, and I'm not sure if I am just
> missing the intended distinction.

Essentially, I'm using backing_bs() whenever bs->backing could be NULL,
because bs->backing->bs would segfault then. When I know that it's not
NULL, I prefer the direct access.

Kevin

Re: [Qemu-devel] [PATCH v3 2/2] remove function during multi-function hot-add

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 04:41:35PM +0800, Cao jin wrote:
> In case user regret when hot-adding multi-function, should roll back,
> device_del the function added but not exposed to the guest.
> 
> Signed-off-by: Cao jin 

I think this patch should come first, before we enable the
functionality that depends on it.


> ---
>  hw/pci/pci_host.c |  6 +-
>  hw/pci/pcie.c | 22 +-
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index 3e26f92..35e5cf3 100644
> --- a/hw/pci/pci_host.c
> +++ b/hw/pci/pci_host.c
> @@ -20,6 +20,7 @@
>  
>  #include "hw/pci/pci.h"
>  #include "hw/pci/pci_host.h"
> +#include "hw/pci/pci_bus.h"
>  #include "trace.h"
>  
>  /* debug PCI */
> @@ -88,10 +89,13 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t 
> val, int len)
>  uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
>  {
>  PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
> +PCIDevice *f0 = NULL;
>  uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
>  uint32_t val;
> +uint8_t slot = (addr >> 11) & 0x1F;
>  
> -if (!pci_dev) {
> +f0 = s->devices[PCI_DEVFN(slot, 0)];
> +if (!pci_dev || (!f0 && pci_dev)) {
>  return ~0x0;
>  }
>  
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 89bf61b..58d2153 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -261,13 +261,30 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>  }
>  }
>  
> +static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> +{
> +object_unparent(OBJECT(dev));
> +}
> +
>  void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
>   DeviceState *dev, Error **errp)
>  {
>  uint8_t *exp_cap;
> +PCIDevice *pci_dev = PCI_DEVICE(dev);
> +PCIBus *bus = pci_dev->bus;
>  
>  pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, _cap, 
> errp);
>  
> +/* In case user regret when hot-adding multi function, remove the 
> function
> + * that is unexposed to guest individually, without interaction with 
> guest.
> + */
> +if (PCI_FUNC(pci_dev->devfn) > 0 &&
> +bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)] == NULL) {
> +pcie_unplug_device(bus, pci_dev, NULL);
> +
> +return;
> +}
> +
>  pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
>  }
>  
> @@ -378,11 +395,6 @@ void pcie_cap_slot_reset(PCIDevice *dev)
>  hotplug_event_update_event_status(dev);
>  }
>  
> -static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> -{
> -object_unparent(OBJECT(dev));
> -}
> -
>  void pcie_cap_slot_write_config(PCIDevice *dev,
>  uint32_t addr, uint32_t val, int len)
>  {
> -- 
> 2.1.0

Re: [Qemu-devel] [PATCH v3 05/16] block: Convert bs->file to BdrvChild

[Kevin Wolf]

Am 13.10.2015 um 03:33 hat Jeff Cody geschrieben:
> On Fri, Oct 09, 2015 at 02:15:30PM +0200, Kevin Wolf wrote:
> > This patch removes the temporary duplication between bs->file and
> > bs->file_child by converting everything to BdrvChild.
> > 
> > Signed-off-by: Kevin Wolf 
> > Reviewed-by: Max Reitz 
> > Reviewed-by: Alberto Garcia 
> > Reviewed-by: Fam Zheng 
> > ---
> >  block.c   | 63 
> > ++-
> >  block/blkdebug.c  | 32 +---
> >  block/blkverify.c | 33 ++---
> >  block/bochs.c |  8 +++---
> >  block/cloop.c | 10 
> >  block/dmg.c   | 20 +++
> >  block/io.c| 50 ++---
> >  block/parallels.c | 38 ++--
> >  block/qapi.c  |  2 +-
> >  block/qcow.c  | 42 ---
> >  block/qcow2-cache.c   | 11 +
> >  block/qcow2-cluster.c | 38 
> >  block/qcow2-refcount.c| 45 +
> >  block/qcow2-snapshot.c| 30 +++---
> >  block/qcow2.c | 62 
> > --
> >  block/qed-table.c |  4 +--
> >  block/qed.c   | 32 
> >  block/raw_bsd.c   | 40 +++---
> >  block/snapshot.c  | 12 -
> >  block/vdi.c   | 17 +++--
> >  block/vhdx-log.c  | 25 ++-
> >  block/vhdx.c  | 36 ++-
> >  block/vmdk.c  | 27 ++--
> >  block/vpc.c   | 34 +
> >  include/block/block.h |  8 +-
> >  include/block/block_int.h |  3 +--
> >  26 files changed, 378 insertions(+), 344 deletions(-)
> >
> 
> [snip lots of code]
> 
> > diff --git a/block/blkdebug.c b/block/blkdebug.c
> > index bc247f4..117fce6 100644
> > --- a/block/blkdebug.c
> > +++ b/block/blkdebug.c
> > @@ -427,10 +427,10 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >  s->state = 1;
> >  
> >  /* Open the backing file */
> > -assert(bs->file == NULL);
> > -ret = bdrv_open_image(>file, qemu_opt_get(opts, "x-image"), 
> > options, "image",
> > -  bs, _file, false, _err);
> > -if (ret < 0) {
> > +bs->file = bdrv_open_child(qemu_opt_get(opts, "x-image"), options, 
> > "image",
> > +   bs, _file, false, _err);
> > +if (local_err) {
> > +ret = -EINVAL;
> >  error_propagate(errp, local_err);
> >  goto out;
> >  }
> > @@ -449,7 +449,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict 
> > *options, int flags,
> >  goto out;
> >  
> >  fail_unref:
> > -bdrv_unref(bs->file);
> > +bdrv_unref(bs->file->bs);
> 
> 
> Would it be better to use bdrv_unref_child() here?

Good catch, without it the BdrvChild object is leaked here. (And
blkverify already leaks the whole BDS on .bdrv_open failure. I'll send a
separate patch on top of this series for that.)

I'll just fix this (and your coding style points) while merging the
series; unless something major comes up, I'm not going to send another
version.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH v10 08/10] Implement new driver for block replication

[Wen Congyang]

On 10/13/2015 12:31 AM, Stefan Hajnoczi wrote:
> On Fri, Sep 25, 2015 at 02:17:36PM +0800, Wen Congyang wrote:
>> +static void replication_start(BlockDriverState *bs, ReplicationMode mode,
>> +  Error **errp)
>> +{
>> +BDRVReplicationState *s = bs->opaque;
>> +int64_t active_length, hidden_length, disk_length;
>> +AioContext *aio_context;
>> +Error *local_err = NULL;
>> +
>> +if (s->replication_state != BLOCK_REPLICATION_NONE) {
>> +error_setg(errp, "Block replication is running or done");
>> +return;
>> +}
>> +
>> +if (s->mode != mode) {
>> +error_setg(errp, "The parameter mode's value is invalid, needs %d,"
>> +   " but receives %d", s->mode, mode);
>> +return;
>> +}
>> +
>> +switch (s->mode) {
>> +case REPLICATION_MODE_PRIMARY:
>> +break;
>> +case REPLICATION_MODE_SECONDARY:
>> +s->active_disk = bs->file;
>> +if (!bs->file->backing_hd) {
>> +error_setg(errp, "Active disk doesn't have backing file");
>> +return;
>> +}
>> +
>> +s->hidden_disk = s->active_disk->backing_hd;
>> +if (!s->hidden_disk->backing_hd) {
>> +error_setg(errp, "Hidden disk doesn't have backing file");
>> +return;
>> +}
>> +
>> +s->secondary_disk = s->hidden_disk->backing_hd;
>> +if (!s->secondary_disk->blk) {
>> +error_setg(errp, "The secondary disk doesn't have block 
>> backend");
>> +return;
>> +}
> ...
>> +aio_context = bdrv_get_aio_context(bs);
>> +aio_context_acquire(aio_context);
>> +bdrv_set_aio_context(s->secondary_disk, aio_context);
> 
> Why is this bdrv_set_aio_context() call necessary?
> 
> Child BDS nodes are in the same AioContext as their parents.  Other
> block jobs need something like this because they operate on a second BDS
> which is not bs' backing file chain.  I think you have a different
> situation here so it's not needed.

I think you are right. I will check it and remove it.

Thanks
Wen Congyang

> .
> 

Re: [Qemu-devel] [PATCH v3] spice: Allow to set password even if disable-ticketing was used

[Gerd Hoffmann]

  Hi,

> > If there is an actual use case where switching authentication methods at
> > runtime is needed we can discuss that.  But we'll be doing that as
> > explicit monitor command, not as side-effect of something else.
> 
> Yeah, I'm not saying this patch is a good idea. However, there was a
> silent change (by reading the commit log which introduced it, this
> change is not mentioned at all, so may have been unintentional)

Well, sort of.  The check was mainly added to avoid set_ticket being
called with sasl auth being active.  And as setting the ticket only
makes sense with spice auth being active (not knowing set_ticket can
switch auth mode) the check added verifies we are in spice auth mode.

> in
> behaviour in QEMU, and this change causes breakage in existing apps
> using QEMU.

Undocumented behavior.

> This patch only attempts to fix this regression, I'm not
> saying one behaviour or the other is better.

Fair enough.  I still prefer to keep the current behavior.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] hw/arm/virt: Allow zero address for PCI IO space

[Peter Maydell]

On 13 October 2015 at 07:31, Alexander Gordeev  wrote:
> On Mon, Oct 12, 2015 at 10:03:03PM +0100, Peter Maydell wrote:
>> On 12 October 2015 at 21:55, Alexander Gordeev  wrote:
>> > Currently PCI IO address 0 is not allowed even though
>> > the IO space starts from 0. As result, PCI IO is not
>> > possible to use at all.
>>
>> I don't see any reason for us not to allow 0 IO addresses,
>> but I'm not sure how your your conclusion follows. It
>> should be entirely possible to map PCI IO to some other
>> address than zero in the IO window, which is what I would
>> have expected the guest to do.
>
> You are right - my changelog is incorrect. The rest of IO
> space should be alright.
>
> However, as 0 IO address is exposed via "ranges" to the guest,
> it must be usable - isn't it? So it either should be allowed
> or the range should be different.

Depends on your point of view. If you take the Linus Torvalds
view that 0 is always an invalid PCI address, then you'll
never try to use it anyway.

In any case, setting pci_allow_0_address is the right thing,
so we can just change the commit message in this patch.

Incidentally, why is this a property on the machine
and not on the PCI controller device?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] hw/arm/virt: Allow zero address for PCI IO space

[Alexander Gordeev]

On Mon, Oct 12, 2015 at 10:03:03PM +0100, Peter Maydell wrote:
> On 12 October 2015 at 21:55, Alexander Gordeev  wrote:
> > Currently PCI IO address 0 is not allowed even though
> > the IO space starts from 0. As result, PCI IO is not
> > possible to use at all.
> 
> I don't see any reason for us not to allow 0 IO addresses,
> but I'm not sure how your your conclusion follows. It
> should be entirely possible to map PCI IO to some other
> address than zero in the IO window, which is what I would
> have expected the guest to do.

You are right - my changelog is incorrect. The rest of IO
space should be alright.

However, as 0 IO address is exposed via "ranges" to the guest,
it must be usable - isn't it? So it either should be allowed
or the range should be different.

Thanks!

> thanks
> -- PMM

-- 
Regards,
Alexander Gordeev
agord...@redhat.com

Re: [Qemu-devel] [PATCH v3 13/16] block: Implement bdrv_append() without bdrv_swap()

[Kevin Wolf]

Am 12.10.2015 um 16:27 hat Alberto Garcia geschrieben:
> On Fri 09 Oct 2015 02:15:38 PM CEST, Kevin Wolf wrote:
> > +static void change_parent_backing_link(BlockDriverState *from,
> > +   BlockDriverState *to)
> > +{
> > +BdrvChild *c, *next;
> > +
> > +QLIST_FOREACH_SAFE(c, >parents, next_parent, next) {
> > +assert(c->role != _backing);
> > +c->bs = to;
> > +QLIST_REMOVE(c, next_parent);
> > +QLIST_INSERT_HEAD(>parents, c, next_parent);
> > +bdrv_ref(to);
> > +bdrv_unref(from);
> > +}
> > +if (from->blk) {
> > +blk_set_bs(from->blk, to);
> > +if (!to->device_list.tqe_prev) {
> > +QTAILQ_INSERT_BEFORE(from, to, device_list);
> > +}
> 
> Is it even possible that this last condition is false? In what case
> would 'to' be already in bdrv_states?
> 
> I understand that it would mean that it would already be attached to a
> BlockBackend, but that's not possible in this case.

Yes, I think it's not possible currently (hopefully, because that would
cause other bugs), just being careful. Eventually we'll allow more than
one BlockBackend pointing to the same BDS, and then this is a scenario
that could happen.

Kevin

Re: [Qemu-devel] [PATCH RFC] hw/virtio: Add PCIe capability to virtio devices

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 10:13:37AM +0200, Gerd Hoffmann wrote:
> On Mo, 2015-10-12 at 18:42 +0300, Michael S. Tsirkin wrote:
> > On Mon, Oct 12, 2015 at 06:17:54PM +0300, Marcel Apfelbaum wrote:
> > > The virtio devices are converted to PCI-Express
> > > if they are plugged into a PCI-Express bus and
> > > the 'modern' protocol is enabled.
> > > 
> > > Signed-off-by: Marcel Apfelbaum 
> > > ---
> > > 
> > > This is an RFC because all it does it adds the PCIe capability and 
> > > nothing more.
> > 
> > Express capability is easy.
> > But if you go over express space you will see that a bunch of
> > other capabilities are required, such as PM capability etc.
> > These might need more work.
> 
> Also what about the legacy io bar?  I guess we'd better avoid that for
> express devices.

This needs some thought.  We definitely still want a way to enable it I
think, e.g. for old guests.

> Maybe it makes sense to add virtio-*-pcie devices (virtio-1.0 only, with
> pcie caps)?
> 
> cheers,
>   Gerd

Personally I think it's ugly enough to remember the need to specify -pci.
OTOH the need to specify upstream and downstream ports is even uglier.
Any idea how to address both issues at the same time?

-- 
MST

[Qemu-devel] [PATCH] gdbstub: Fix buffer overflows in gdb_handle_packet()

[Kevin Wolf]

Some places in gdb_handle_packet() can get an arbitrary length (most
times directly from the client) and either didn't check it at all or
checked against the wrong value, potentially causing buffer overflows.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf 
---
 gdbstub.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index d2c95b5..9c29aa0 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -956,6 +956,13 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 if (*p == ',')
 p++;
 len = strtoull(p, NULL, 16);
+
+/* memtohex() doubles the required space */
+if (len > MAX_PACKET_LENGTH / 2) {
+put_packet (s, "E22");
+break;
+}
+
 if (target_memory_rw_debug(s->g_cpu, addr, mem_buf, len, false) != 0) {
 put_packet (s, "E14");
 } else {
@@ -970,6 +977,12 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 len = strtoull(p, (char **), 16);
 if (*p == ':')
 p++;
+
+/* hextomem() reads 2*len bytes */
+if (len > strlen(p) / 2) {
+put_packet (s, "E22");
+break;
+}
 hextomem(mem_buf, p, len);
 if (target_memory_rw_debug(s->g_cpu, addr, mem_buf, len,
true) != 0) {
@@ -1107,7 +1120,8 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 cpu = find_cpu(thread);
 if (cpu != NULL) {
 cpu_synchronize_state(cpu);
-len = snprintf((char *)mem_buf, sizeof(mem_buf),
+/* memtohex() doubles the required space */
+len = snprintf((char *)mem_buf, sizeof(buf) / 2,
"CPU#%d [%s]", cpu->cpu_index,
cpu->halted ? "halted " : "running");
 memtohex(buf, mem_buf, len);
@@ -1136,8 +1150,8 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 put_packet(s, "E01");
 break;
 }
-hextomem(mem_buf, p + 5, len);
 len = len / 2;
+hextomem(mem_buf, p + 5, len);
 mem_buf[len++] = 0;
 qemu_chr_be_write(s->mon_chr, mem_buf, len);
 put_packet(s, "OK");
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] target-i386: fix pcmpxstrx equal-ordered (strstr) mode

[Paolo Bonzini]

On 13/10/2015 00:16, Richard Henderson wrote:
> On 10/12/2015 08:50 PM, Paolo Bonzini wrote:
>> In this mode, referring an invalid element of the source forces the
>> result to false (table 4-7, last column) but referring an invalid
>> element of the destination forces the result to true, so the outer
>> loop should still be run even if some elements of the destination
>> will be invalid.  They will be culled in the inner loop, which
>> correctly bounds "i" to validd.
>>
>> This fix tst_strstr in glibc 2.17.
>>
>> Reported-by: Florian Weimer 
>> Cc: Richard Henderson 
>> Cc: Eduardo Habkost 
>> Signed-off-by: Paolo Bonzini 
>> ---
>>   target-i386/ops_sse.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target-i386/ops_sse.h b/target-i386/ops_sse.h
>> index 7aa693a..268f3e1 100644
>> --- a/target-i386/ops_sse.h
>> +++ b/target-i386/ops_sse.h
>> @@ -2037,7 +2037,7 @@ static inline unsigned pcmpxstrx(CPUX86State
>> *env, Reg *d, Reg *s,
>>   }
>>   break;
>>   case 3:
>> -for (j = valids - validd; j >= 0; j--) {
>> +for (j = valids; j >= 0; j--) {
>>   res <<= 1;
>>   v = 1;
>>   for (i = MIN(upper - j, validd); i >= 0; i--) {
> 
> I don't see how the bounding is properly done.  In particular,
> 
>> v &= (pcmp_val(s, ctrl, i + j) == pcmp_val(d, ctrl, i));
> 
> We're bounding j by valids, but accessing i+j?

You're absolutely right, the second loop also needs s/upper/valids/.

Paolo

Re: [Qemu-devel] [PATCH v3 4/4] qemu-iotests: update tests for generated node-names

[Markus Armbruster]

Shouldn't PATCH 3+4 be squashed into PATCH 2 to keep bisection working?

[Qemu-devel] [Bug 1465935] Re: kvm_irqchip_commit_routes: Assertion `ret == 0' failed

[Stefan Bader]

@Li Chengyuan, thank you for the clarification. So just formally I will
mark the Precise task of this report as invalid (since the qemu in
Precise is actually a different source package and also not affected as
far as I can tell). I will need to figure out how to ensure this fix is
also pulled into the cloud-archive after it landed in the Trusty/Vivid
main archive.

** Changed in: qemu (Ubuntu Precise)
   Status: New => Invalid

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

Title:
  kvm_irqchip_commit_routes: Assertion `ret == 0' failed

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Precise:
  Invalid
Status in qemu source package in Trusty:
  New
Status in qemu source package in Utopic:
  Won't Fix
Status in qemu source package in Vivid:
  New

Bug description:
  Several my QEMU instances crashed, and in the  qemu log, I can see
  this assertion failure,

 qemu-system-x86_64: /build/buildd/qemu-2.0.0+dfsg/kvm-all.c:984:
  kvm_irqchip_commit_routes: Assertion `ret == 0' failed.

  The QEMU version is 2.0.0, HV OS is ubuntu 12.04, kernel 3.2.0-38.
  Guest OS is RHEL 6.3.

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

[Qemu-devel] [Bug 1465935] Re: kvm_irqchip_commit_routes: Assertion `ret == 0' failed

[Stefan Bader]

** Changed in: qemu (Ubuntu Vivid)
   Status: New => In Progress

** Changed in: qemu (Ubuntu Trusty)
   Status: New => In Progress

** Changed in: qemu (Ubuntu Trusty)
 Assignee: (unassigned) => Stefan Bader (smb)

** Changed in: qemu (Ubuntu Vivid)
 Assignee: (unassigned) => Stefan Bader (smb)

** Changed in: qemu (Ubuntu)
 Assignee: Stefan Bader (smb) => (unassigned)

** Changed in: qemu (Ubuntu Trusty)
   Importance: Undecided => High

** Changed in: qemu (Ubuntu Vivid)
   Importance: Undecided => High

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

Title:
  kvm_irqchip_commit_routes: Assertion `ret == 0' failed

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Precise:
  Invalid
Status in qemu source package in Trusty:
  In Progress
Status in qemu source package in Utopic:
  Won't Fix
Status in qemu source package in Vivid:
  In Progress

Bug description:
  Several my QEMU instances crashed, and in the  qemu log, I can see
  this assertion failure,

 qemu-system-x86_64: /build/buildd/qemu-2.0.0+dfsg/kvm-all.c:984:
  kvm_irqchip_commit_routes: Assertion `ret == 0' failed.

  The QEMU version is 2.0.0, HV OS is ubuntu 12.04, kernel 3.2.0-38.
  Guest OS is RHEL 6.3.

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

Re: [Qemu-devel] [PATCH v3 0/2] PCI-e device multi-function hot-add support

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 04:41:33PM +0800, Cao jin wrote:
> Support PCI-e device hot-add multi-function via device_add, just ensure
> add the function 0 is added last. While allow user to roll back in the
> middle via device_del, in case user regret.

This patch doesn't seem to account of AIR though.

> changelog:
> 1. Flag device as unexposed when func 0 doesn`t exist, via return 0xFF
>in case of gratuitous pci bus scan.
> 2. Since device is unexposed to guest, can remove function individually,
>without interaction with the guest.
> 
> Cao jin (2):
>   enable multi-function hot-add
>   remove function during multi-function hot-add
> 
>  hw/pci/pci.c  | 10 ++
>  hw/pci/pci_host.c |  6 +-
>  hw/pci/pcie.c | 38 +-
>  3 files changed, 40 insertions(+), 14 deletions(-)
> 
> -- 
> 2.1.0

Re: [Qemu-devel] [Qemu-block] [PATCH v10 08/10] Implement new driver for block replication

[Wen Congyang]

On 10/13/2015 12:25 AM, Stefan Hajnoczi wrote:
> On Fri, Sep 25, 2015 at 02:17:36PM +0800, Wen Congyang wrote:
>> +static void backup_job_completed(void *opaque, int ret)
>> +{
>> +BDRVReplicationState *s = opaque;
>> +
>> +if (s->replication_state != BLOCK_REPLICATION_DONE) {
>> +/* The backup job is cancelled unexpectedly */
>> +s->error = -EIO;
>> +}
>> +
>> +bdrv_op_block(s->hidden_disk, BLOCK_OP_TYPE_BACKUP_TARGET,
>> +  s->active_disk->backing_blocker);
>> +bdrv_op_block(s->secondary_disk, BLOCK_OP_TYPE_BACKUP_SOURCE,
>> +  s->hidden_disk->backing_blocker);
>> +
>> +bdrv_put_ref_bh_schedule(s->secondary_disk);
> 
> Why is bdrv_put_ref_bh_schedule() necessary?

It is copied from block_job_cb(). According to the comments in 
bdrv_put_ref_bh_schedule():
/*
 * Release a BDS reference in a BH
 *
 * It is not safe to use bdrv_unref() from a callback function when the callers
 * still need the BlockDriverState.  In such cases we schedule a BH to release
 * the reference.
 */

If the comment is right, I think it is necessary to call 
bdrv_put_ref_bh_schedule() here.
Because the job is created on the BDS s->secondary disk, backup_job_completed() 
is
called in block_job_completed(), and we will still use s->secondary_disk in 
block_job_release().

Thanks
Wen Congyang

> .
> 

Re: [Qemu-devel] [Qemu-block] [PATCH v10 08/10] Implement new driver for block replication

[Wen Congyang]

On 10/13/2015 12:27 AM, Stefan Hajnoczi wrote:
> On Fri, Sep 25, 2015 at 02:17:36PM +0800, Wen Congyang wrote:
>> +/* start backup job now */
>> +bdrv_op_unblock(s->hidden_disk, BLOCK_OP_TYPE_BACKUP_TARGET,
>> +s->active_disk->backing_blocker);
>> +bdrv_op_unblock(s->secondary_disk, BLOCK_OP_TYPE_BACKUP_SOURCE,
>> +s->hidden_disk->backing_blocker);
> 
> Why is it safe to unblock these operations?
> 
> Why do they have to be blocked for non-replication users?

hidden_disk and secondary disk are opened as backing file, so it is blocked for
non-replication users.
What can I do if I don't unblock it and want to do backup?

Thanks
Wen Congyang

> 
> Stefan
> .
> 

Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check()

[Markus Armbruster]

Eric Blake  writes:

> On 10/12/2015 09:53 AM, Markus Armbruster wrote:
[...]
>> It would be simpler if we could make C clash only when QMP clashes.
>
> If a QMP clash always caused a C clash, life would be a bit simpler. We
> aren't going to get there (because in a flat union, hiding the members
> of the branch type behind a single pointer means those members don't
> clash with any C names of the overall union), but I can certainly try to
> make the comments explain what is going on.

(1) If QMP clashed exactly when C clashed, we'd have to think only about
one of them, and the C compiler would check for clashes statically.

(2) If a QMP clash implied a C clash, we'd only have to think about C,
and the C compiler would check statically.

(3) If a C clash implied a QMP clash, we'd only have to think about QMP.

(4) If they can clash independently, we need to think about both
independently.

We currently have (4), and having to juggle both QMP and C in my mind
has been taxing.

My remark was about (3): C clashes only when QMP clashes <=> C clash
implies QMP clash.

Your remark was about (2).  More useful than (3), but as you say not
feasible.

Do you think we can get to (3)?

>> We might want to separate out alternates.  Dunno.
>
> And to some extent, subset C already does some of that separation
> (because I try to convert from 'FooKind type' to 'qtype_code type'
> without even creating an implicit 'FooKind' enum).  It sounds like
> you're okay with any well-documented TODO warts related to alternates
> for the purposes of this subset B, especially if I can then clean those
> warts back up in subset C.  But what v8 of subset B needs to avoid is
> any warts based on simple vs. flat unions.  I can live with that.

I'm prepared to accept rephrased existing warts and additional temporary
warts when I understand why simply fixing them isn't practical or
economical.  A certain amount of hand-waving is okay.

> I'm still trying to figure out if hoisting the kind=>type rename into
> subset B makes life easier or harder (it certainly causes me more rebase
> churn, but that's not necessarily a bad thing).

Thanks!

Re: [Qemu-devel] [RFC] Block device size rounding

[Markus Armbruster]

John Snow  writes:

> On 10/12/2015 02:09 PM, Peter Crosthwaite wrote:
>> On Mon, Oct 12, 2015 at 9:26 AM, Eric Blake  wrote:
>>> On 10/12/2015 09:56 AM, John Snow wrote:
>>>
> What is the correct action here though? If the file is writeable should
> we just allow the device to extend its size? Is that possible already?
> Just zero-pad read-only?
>

 Read-only seems like an easy case of append zeroes.
>>>
>>> Yes, allowing read-only with append-zero behavior seems sane.

This is tolerable.  Do we want to warn?

A reopen can bring in the read/write case.

 Read-write ... well, we can't write-protect just half of a 512k block.
>>>
 Probably just forcibly increasing the size on RW or refusing to use the
 file altogether are probably the sane deterministic things we want.
>>>
>>> I'd lean towards outright rejection if the file size isn't up to snuff
>>> for use as read-write.  Forcibly increasing the size (done
>>> unconditionally) still feels like magic, and may not be possible if the
>>> size is due to something backed by a block device rather than a file.

Concur.

>> Inability to extend is easily detectable and can become a failure mode
>> in it's own right. If we cant extend the file perhaps we can just
>> LOG_UNIMP the data writes? Having to include in your user instructions
>> "dd your already-on-SATA file system to this container just so it can
>> work for SD" is a pain.

Whenever QEMU proper can extend to the right size, qemu-img should be
able to do so as well, shouldn't it?  QEMU's error message could even
explain how.

> Fits within my "Always extend the size" answer. Failing to do so is a
> good cause to fail.
>
> I'm not sure if this is the sort of thing that might require an extra
> flag or option for compatibility reasons or not, though. If there is no
> precedent for QEMU resizing a block device to make it compatible with a
> particular device model, it's probably reasonable that no management
> tool is expecting this to happen automatically either.
>
> Then again, it's still annoying that the current default is definitely
> broken.
>
> I think this is going to boil down into an interface-and-expectations
> argument. I am otherwise in favor of just forcing the resize whenever
> possible and failing when it isn't.

I agree it's about expectations.

When I give QEMU read/write access to an image, I expect it to modify
the image, but I don't expect it to resize it on its own.  Perhaps my
expectation is wrong.  Do we have precedence?

Re: [Qemu-devel] [PATCH v3 1/4] util - add automated ID generation utility

[Markus Armbruster]

Jeff Cody  writes:

> Multiple sub-systems in QEMU may find it useful to generate IDs
> for objects that a user may reference via QMP or HMP.  This patch
> presents a standardized way to do it, so that automatic ID generation
> follows the same rules.
>
> This patch enforces the following rules when generating an ID:
>
> 1.) Guarantee no collisions with a user-specified ID
> 2.) Identify the sub-system the ID belongs to
> 3.) Guarantee of uniqueness
> 4.) Spoiling predictability, to avoid creating an assumption
> of object ordering and parsing (i.e., we don't want users to think
> they can guess the next ID based on prior behavior).
>
> The scheme for this is as follows (no spaces):
>
> # subsys D RR
> Reserved char --||   | |
> Subsystem String |   | |
> Unique number (64-bit) --| |
> Two-digit random number ---|
>
> For example, a generated node-name for the block sub-system may look
> like this:
>
> #block076
>
> The caller of id_generate() is responsible for freeing the generated
> node name string with g_free().
>
> Reviewed-by: John Snow 
> Reviewed-by: Eric Blake 
> Reviewed-by: Alberto Garcia 
> Signed-off-by: Jeff Cody 
> ---
>  include/qemu-common.h |  8 
>  util/id.c | 37 +
>  2 files changed, 45 insertions(+)
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 0bd212b..2f74540 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -246,6 +246,14 @@ int64_t qemu_strtosz_suffix_unit(const char *nptr, char 
> **end,
>  #define STR_OR_NULL(str) ((str) ? (str) : "null")
>  
>  /* id.c */
> +
> +typedef enum IdSubSystems {
> +ID_QDEV,

ID_QDEV is not used in this series.  Do you intend to use it in a
followup-series?  Can we reasonably expect that series will be accepted?

You could sidestep these questions by making id_generate() take a string
argument ;)

> +ID_BLOCK,
> +ID_MAX  /* last element, used as array size */
> +} IdSubSystems;
> +
> +char *id_generate(IdSubSystems id);
>  bool id_wellformed(const char *id);
>  
>  /* path.c */
[...]

Re: [Qemu-devel] [PATCH RFC] hw/virtio: Add PCIe capability to virtio devices

[Gerd Hoffmann]

On Mo, 2015-10-12 at 18:42 +0300, Michael S. Tsirkin wrote:
> On Mon, Oct 12, 2015 at 06:17:54PM +0300, Marcel Apfelbaum wrote:
> > The virtio devices are converted to PCI-Express
> > if they are plugged into a PCI-Express bus and
> > the 'modern' protocol is enabled.
> > 
> > Signed-off-by: Marcel Apfelbaum 
> > ---
> > 
> > This is an RFC because all it does it adds the PCIe capability and nothing 
> > more.
> 
> Express capability is easy.
> But if you go over express space you will see that a bunch of
> other capabilities are required, such as PM capability etc.
> These might need more work.

Also what about the legacy io bar?  I guess we'd better avoid that for
express devices.

Maybe it makes sense to add virtio-*-pcie devices (virtio-1.0 only, with
pcie caps)?

cheers,
  Gerd

Re: [Qemu-devel] [PULL 04/26] target-*: Introduce and use cpu_breakpoint_test

[Peter Maydell]

On 13 October 2015 at 01:13, Richard Henderson  wrote:
> Why do you believe that a zero-length TB won't be cleared?
> The TB still has a start address, which is contained within
> a given page, which is invalidated.
>
> Some target-*/translate.c takes care to advance the PC, but I believe that
> this is only required when the breakpoint instruction *itself* could span a
> page boundary.  I.e. the TB needs to be marked to span two pages.  This
> situation can never be true for many RISC targets.

If this is the reason for the logic it would be good to have
a comment explaining it in the code. I've never really understood
that part...

-- PMM

Re: [Qemu-devel] [RFC] Block device size rounding

[Kevin Wolf]

Am 12.10.2015 um 20:26 hat John Snow geschrieben:
> 
> 
> On 10/12/2015 02:09 PM, Peter Crosthwaite wrote:
> > On Mon, Oct 12, 2015 at 9:26 AM, Eric Blake  wrote:
> >> On 10/12/2015 09:56 AM, John Snow wrote:
> >>
>  What is the correct action here though? If the file is writeable should
>  we just allow the device to extend its size? Is that possible already?
>  Just zero-pad read-only?
> 
> >>>
> >>> Read-only seems like an easy case of append zeroes.
> >>
> >> Yes, allowing read-only with append-zero behavior seems sane.
> >>
> >>>
> >>> Read-write ... well, we can't write-protect just half of a 512k block.
> >>
> >>> Probably just forcibly increasing the size on RW or refusing to use the
> >>> file altogether are probably the sane deterministic things we want.
> >>
> >> I'd lean towards outright rejection if the file size isn't up to snuff
> >> for use as read-write.  Forcibly increasing the size (done
> >> unconditionally) still feels like magic, and may not be possible if the
> >> size is due to something backed by a block device rather than a file.

Agreed, let's just reject the image for r/w. Image resize should always
been an explicit action invoked by the user, not a side effect of using
the image with a specific device.

> > Inability to extend is easily detectable and can become a failure mode
> > in it's own right. If we cant extend the file perhaps we can just
> > LOG_UNIMP the data writes? Having to include in your user instructions
> > "dd your already-on-SATA file system to this container just so it can
> > work for SD" is a pain.
> > 
> > Regards,
> > Peter
> > 
> 
> Fits within my "Always extend the size" answer. Failing to do so is a
> good cause to fail.
> 
> I'm not sure if this is the sort of thing that might require an extra
> flag or option for compatibility reasons or not, though. If there is no
> precedent for QEMU resizing a block device to make it compatible with a
> particular device model, it's probably reasonable that no management
> tool is expecting this to happen automatically either.
> 
> Then again, it's still annoying that the current default is definitely
> broken.

That's not so clear to me. Strictly speaking, this is really a user
error because the user passed an image that isn't suitable for the
device. All we're discussing is handling this user error friendlier.

Maybe we should take a step back: What's the specific use case here,
i.e. where does the misaligned image come from and what is it used for?
I assume this is not an image created with qemu-img, because then the
obvious options would already result in an aligned size.

> I think this is going to boil down into an interface-and-expectations
> argument. I am otherwise in favor of just forcing the resize whenever
> possible and failing when it isn't.

I'm strongly objecting to any automagic resizing of images.

Kevin

[Qemu-devel] [Bug 1359383] Re: kernel panic at smpboot.c:134 when rebooting qemu with multiple cores

[Dexuan Cui]

The bug is probably fixed by
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=dd9d3843755da95f63dd3a376f62b3e45c011210

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

Title:
  kernel panic at smpboot.c:134 when rebooting qemu with multiple cores

Status in QEMU:
  New

Bug description:
  Hi all,

  I can reproduce this with kernel 3.14 and 3.17rc1. I suspect it is a
  qemu issue, but I'm not sure. The test case is the following script:

  qemu-system-x86_64 -machine accel=kvm -pidfile /tmp/pid$$ -m 512M -smp
  8,sockets=8 -kernel vmlinuz -append "init=/sbin/reboot -f
  console=ttyS0,115200 kgdboc=ttyS2,115200 root=/dev/sda rw" -nographic
  -serial stdio -drive format=raw,snapshot=on,file=/var/lib/ktest/root

  Note that we pass /sbin/reboot as the init program so it just reboots
  forever. After a dozen or so iterations, I hit this:

  [0.00] Initializing cgroup subsys cpuset
  [0.00] Initializing cgroup subsys cpu
  [0.00] Initializing cgroup subsys cpuacct
  [0.00] Linux version 3.17.0-rc1-0-2014.sp (sp@vodka) (gcc version 
4.8.2 20140120 (Red Hat 4.8.2-16) (GCC) ) #209 SMP Wed Aug 20 20:17:46 UTC 2014
  [0.00] Command line: init=/sbin/reboot -f console=ttyS0,115200 
kgdboc=ttyS2,115200 root=/dev/sda rw ktest.priority=9
  [0.00] e820: BIOS-provided physical RAM map:
  [0.00] BIOS-e820: [mem 0x-0x0009fbff] usable
  [0.00] BIOS-e820: [mem 0x0009fc00-0x0009] reserved
  [0.00] BIOS-e820: [mem 0x000f-0x000f] reserved
  [0.00] BIOS-e820: [mem 0x0010-0x1fffcfff] usable
  [0.00] BIOS-e820: [mem 0x1fffd000-0x1fff] reserved
  [0.00] BIOS-e820: [mem 0xfeffc000-0xfeff] reserved
  [0.00] BIOS-e820: [mem 0xfffc-0x] reserved
  [0.00] process: using polling idle threads
  [0.00] NX (Execute Disable) protection: active
  [0.00] SMBIOS 2.4 present.
  [0.00] Hypervisor detected: KVM
  [0.00] e820: last_pfn = 0x1fffd max_arch_pfn = 0x4
  [0.00] PAT not supported by CPU.
  [0.00] init_memory_mapping: [mem 0x-0x000f]
  [0.00] init_memory_mapping: [mem 0x1fc0-0x1fdf]
  [0.00] init_memory_mapping: [mem 0x1c00-0x1fbf]
  [0.00] init_memory_mapping: [mem 0x0010-0x1bff]
  [0.00] init_memory_mapping: [mem 0x1fe0-0x1fffcfff]
  [0.00] ACPI: Early table checksum verification disabled
  [0.00] ACPI: RSDP 0x000F0A90 14 (v00 BOCHS )
  [0.00] ACPI: RSDT 0x1C21 34 (v01 BOCHS  BXPCRSDT 
0001 BXPC 0001)
  [0.00] ACPI: FACP 0x1FFFEF40 74 (v01 BOCHS  BXPCFACP 
0001 BXPC 0001) 
  
  [0.00] ACPI: DSDT 0x1FFFDDC0 001180 (v01 BOCHS  BXPCDSDT 
0001 BXPC 0001) 
  
  [0.00] ACPI: FACS 0x1FFFDD80 40   

 
  [0.00] ACPI: SSDT 0x1FFFEFB4 000B85 (v01 BOCHS  BXPCSSDT 
0001 BXPC 0001) 
  
  [0.00] ACPI: APIC 0x1B39 B0 (v01 BOCHS  BXPCAPIC 
0001 BXPC 0001) 
  
  [0.00] ACPI: HPET 0x1BE9 38 (v01 BOCHS  BXPCHPET 
0001 BXPC 0001) 
  
  [0.00] No NUMA configuration found

 
  [0.00] Faking a node at [mem 0x-0x1fffcfff]   

 
  [0.00] Initmem setup node 0 [mem 0x-0x1fffcfff]   

 
  [0.00]   NODE_DATA [mem 0x1fffa000-0x1fffcfff]

 
  [0.00] kvm-clock: Using msrs 4b564d01 and 4b564d00
  

Re: [Qemu-devel] [PATCH v3 2/2] remove function during multi-function hot-add

[Cao jin]

Hi Michael

On 10/13/2015 04:51 PM, Michael S. Tsirkin wrote:

On Tue, Oct 13, 2015 at 04:41:35PM +0800, Cao jin wrote:

In case user regret when hot-adding multi-function, should roll back,
device_del the function added but not exposed to the guest.

Signed-off-by: Cao jin 


I think this patch should come first, before we enable the
functionality that depends on it.

Do you mean, the function should be removed individually in any 
condition? Because as you know, device_del pci_dev will remove all the 
functions in the slot that are fulled exposed to the guest, Alex also 
mentioned this limitation before.



---
  hw/pci/pci_host.c |  6 +-
  hw/pci/pcie.c | 22 +-
  2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 3e26f92..35e5cf3 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -20,6 +20,7 @@

  #include "hw/pci/pci.h"
  #include "hw/pci/pci_host.h"
+#include "hw/pci/pci_bus.h"
  #include "trace.h"

  /* debug PCI */
@@ -88,10 +89,13 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, 
int len)
  uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
  {
  PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
+PCIDevice *f0 = NULL;
  uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
  uint32_t val;
+uint8_t slot = (addr >> 11) & 0x1F;

-if (!pci_dev) {
+f0 = s->devices[PCI_DEVFN(slot, 0)];
+if (!pci_dev || (!f0 && pci_dev)) {
  return ~0x0;
  }

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 89bf61b..58d2153 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -261,13 +261,30 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler 
*hotplug_dev, DeviceState *dev,
  }
  }

+static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
+{
+object_unparent(OBJECT(dev));
+}
+
  void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
   DeviceState *dev, Error **errp)
  {
  uint8_t *exp_cap;
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+PCIBus *bus = pci_dev->bus;

  pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, _cap, 
errp);

+/* In case user regret when hot-adding multi function, remove the function
+ * that is unexposed to guest individually, without interaction with guest.
+ */
+if (PCI_FUNC(pci_dev->devfn) > 0 &&
+bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)] == NULL) {
+pcie_unplug_device(bus, pci_dev, NULL);
+
+return;
+}
+
  pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
  }

@@ -378,11 +395,6 @@ void pcie_cap_slot_reset(PCIDevice *dev)
  hotplug_event_update_event_status(dev);
  }

-static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
-{
-object_unparent(OBJECT(dev));
-}
-
  void pcie_cap_slot_write_config(PCIDevice *dev,
  uint32_t addr, uint32_t val, int len)
  {
--
2.1.0

.



--
Yours Sincerely,

Cao Jin

Re: [Qemu-devel] [PATCH v2 01/22] xen_disk: Account for flush operations

[Stefan Hajnoczi]

On Fri, Oct 02, 2015 at 05:26:11PM +0300, Alberto Garcia wrote:
> Currently both BLKIF_OP_WRITE and BLKIF_OP_FLUSH_DISKCACHE are being
> accounted as write operations.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  hw/block/xen_disk.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 

[Qemu-devel] [PATCH v2 3/5] net/dump: Separate the NetClientState from the DumpState

[Thomas Huth]

With the upcoming dumping-via-netfilter patch, the DumpState
should not be related to NetClientState anymore, so move the
related information to a new struct called DumpNetClient.

Signed-off-by: Thomas Huth 
---
 net/dump.c | 74 +-
 1 file changed, 49 insertions(+), 25 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index e6f6be0..e3e82bd 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -31,7 +31,6 @@
 #include "hub.h"
 
 typedef struct DumpState {
-NetClientState nc;
 int64_t start_ts;
 int fd;
 int pcap_caplen;
@@ -58,10 +57,8 @@ struct pcap_sf_pkthdr {
 uint32_t len;
 };
 
-static ssize_t dump_receive_iov(NetClientState *nc, const struct iovec *iov,
-int cnt)
+static ssize_t dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt)
 {
-DumpState *s = DO_UPCAST(DumpState, nc, nc);
 struct pcap_sf_pkthdr hdr;
 int64_t ts;
 int caplen;
@@ -94,30 +91,12 @@ static ssize_t dump_receive_iov(NetClientState *nc, const 
struct iovec *iov,
 return size;
 }
 
-static ssize_t dump_receive(NetClientState *nc, const uint8_t *buf, size_t 
size)
-{
-struct iovec iov = {
-.iov_base = (void *)buf,
-.iov_len = size
-};
-return dump_receive_iov(nc, , 1);
-}
-
-static void dump_cleanup(NetClientState *nc)
+static void dump_cleanup(DumpState *s)
 {
-DumpState *s = DO_UPCAST(DumpState, nc, nc);
-
 close(s->fd);
+s->fd = -1;
 }
 
-static NetClientInfo net_dump_info = {
-.type = NET_CLIENT_OPTIONS_KIND_DUMP,
-.size = sizeof(DumpState),
-.receive = dump_receive,
-.receive_iov = dump_receive_iov,
-.cleanup = dump_cleanup,
-};
-
 static int net_dump_state_init(DumpState *s, const char *filename,
int len, Error **errp)
 {
@@ -154,6 +133,49 @@ static int net_dump_state_init(DumpState *s, const char 
*filename,
 return 0;
 }
 
+/* Dumping via VLAN netclient */
+
+struct DumpNetClient {
+NetClientState nc;
+DumpState ds;
+};
+typedef struct DumpNetClient DumpNetClient;
+
+static ssize_t dumpclient_receive(NetClientState *nc, const uint8_t *buf,
+  size_t size)
+{
+DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
+struct iovec iov = {
+.iov_base = (void *)buf,
+.iov_len = size
+};
+
+return dump_receive_iov(>ds, , 1);
+}
+
+static ssize_t dumpclient_receive_iov(NetClientState *nc,
+  const struct iovec *iov, int cnt)
+{
+DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
+
+return dump_receive_iov(>ds, iov, cnt);
+}
+
+static void dumpclient_cleanup(NetClientState *nc)
+{
+DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
+
+dump_cleanup(>ds);
+}
+
+static NetClientInfo net_dump_info = {
+.type = NET_CLIENT_OPTIONS_KIND_DUMP,
+.size = sizeof(DumpNetClient),
+.receive = dumpclient_receive,
+.receive_iov = dumpclient_receive_iov,
+.cleanup = dumpclient_cleanup,
+};
+
 int net_init_dump(const NetClientOptions *opts, const char *name,
   NetClientState *peer, Error **errp)
 {
@@ -162,6 +184,7 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
 char def_file[128];
 const NetdevDumpOptions *dump;
 NetClientState *nc;
+DumpNetClient *dnc;
 
 assert(opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP);
 dump = opts->dump;
@@ -195,7 +218,8 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
 snprintf(nc->info_str, sizeof(nc->info_str),
  "dump to %s (len=%d)", file, len);
 
-rc = net_dump_state_init(DO_UPCAST(DumpState, nc, nc), file, len, errp);
+dnc = DO_UPCAST(DumpNetClient, nc, nc);
+rc = net_dump_state_init(>ds, file, len, errp);
 if (rc) {
 qemu_del_net_client(nc);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 4/5] net/dump: Provide the dumping facility as a net-filter

[Thomas Huth]

Use the net-filter infrastructure to provide the dumping
functions for netdev devices, too.

Signed-off-by: Thomas Huth 
---
 net/dump.c | 129 -
 vl.c   |   7 +++-
 2 files changed, 134 insertions(+), 2 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index e3e82bd..dd0555f 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -28,7 +28,8 @@
 #include "qemu/iov.h"
 #include "qemu/log.h"
 #include "qemu/timer.h"
-#include "hub.h"
+#include "qapi/visitor.h"
+#include "net/filter.h"
 
 typedef struct DumpState {
 int64_t start_ts;
@@ -225,3 +226,129 @@ int net_init_dump(const NetClientOptions *opts, const 
char *name,
 }
 return rc;
 }
+
+/* Dumping via filter */
+
+#define TYPE_FILTER_DUMP "filter-dump"
+
+#define FILTER_DUMP(obj) \
+OBJECT_CHECK(NetFilterDumpState, (obj), TYPE_FILTER_DUMP)
+
+struct NetFilterDumpState {
+NetFilterState nfs;
+DumpState ds;
+char *filename;
+uint32_t maxlen;
+};
+typedef struct NetFilterDumpState NetFilterDumpState;
+
+static ssize_t filter_dump_receive_iov(NetFilterState *nf, NetClientState 
*sndr,
+   unsigned flags, const struct iovec *iov,
+   int iovcnt, NetPacketSent *sent_cb)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(nf);
+
+dump_receive_iov(>ds, iov, iovcnt);
+return 0;
+}
+
+static void filter_dump_cleanup(NetFilterState *nf)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(nf);
+
+dump_cleanup(>ds);
+}
+
+static void filter_dump_setup(NetFilterState *nf, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(nf);
+
+if (!nfds->filename) {
+error_setg(errp, "dump filter needs 'file' property set!");
+return;
+}
+
+net_dump_state_init(>ds, nfds->filename, nfds->maxlen, errp);
+}
+
+static void filter_dump_get_maxlen(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+uint32_t value = nfds->maxlen;
+
+visit_type_uint32(v, , name, errp);
+}
+
+static void filter_dump_set_maxlen(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+Error *local_err = NULL;
+uint32_t value;
+
+visit_type_uint32(v, , name, _err);
+if (local_err) {
+goto out;
+}
+if (value == 0) {
+error_setg(_err, "Property '%s.%s' doesn't take value '%u'",
+   object_get_typename(obj), name, value);
+goto out;
+}
+nfds->maxlen = value;
+
+out:
+error_propagate(errp, local_err);
+}
+
+static char *file_dump_get_filename(Object *obj, Error **errp)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+
+return g_strdup(nfds->filename);
+}
+
+static void file_dump_set_filename(Object *obj, const char *value, Error 
**errp)
+{
+   NetFilterDumpState *nfds = FILTER_DUMP(obj);
+
+g_free(nfds->filename);
+nfds->filename = g_strdup(value);
+}
+
+static void filter_dump_instance_init(Object *obj)
+{
+NetFilterDumpState *nfds = FILTER_DUMP(obj);
+
+nfds->maxlen = 65536;
+
+object_property_add(obj, "maxlen", "int", filter_dump_get_maxlen,
+filter_dump_set_maxlen, NULL, NULL, NULL);
+object_property_add_str(obj, "file", file_dump_get_filename,
+file_dump_set_filename, NULL);
+}
+
+static void filter_dump_class_init(ObjectClass *oc, void *data)
+{
+NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+nfc->setup = filter_dump_setup;
+nfc->cleanup = filter_dump_cleanup;
+nfc->receive_iov = filter_dump_receive_iov;
+}
+
+static const TypeInfo filter_dump_info = {
+.name = TYPE_FILTER_DUMP,
+.parent = TYPE_NETFILTER,
+.class_init = filter_dump_class_init,
+.instance_init = filter_dump_instance_init,
+.instance_size = sizeof(NetFilterDumpState),
+};
+
+static void filter_dump_register_types(void)
+{
+type_register_static(_dump_info);
+}
+
+type_init(filter_dump_register_types);
diff --git a/vl.c b/vl.c
index 7c806a2..b54a800 100644
--- a/vl.c
+++ b/vl.c
@@ -2748,7 +2748,12 @@ static bool object_create_initial(const char *type)
 return false;
 }
 
-if (g_str_equal(type, "filter-buffer")) {
+/*
+ * return false for concrete netfilters since
+ * they depend on netdevs already existing
+ */
+if (g_str_equal(type, "filter-buffer") ||
+g_str_equal(type, "filter-dump")) {
 return false;
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [Qemu-block] [PATCH v10 02/10] Backup: clear all bitmap when doing block checkpoint

[Wen Congyang]

On 10/12/2015 09:45 PM, Stefan Hajnoczi wrote:
> On Fri, Sep 25, 2015 at 02:17:30PM +0800, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang 
>> Signed-off-by: zhanghailiang 
>> Signed-off-by: Gonglei 
>> Reviewed-by: Jeff Cody 
>> ---
>>  block/backup.c   | 14 ++
>>  blockjob.c   | 11 +++
>>  include/block/blockjob.h | 12 
>>  3 files changed, 37 insertions(+)
>>
>> diff --git a/block/backup.c b/block/backup.c
>> index c61e4c3..5e5995e 100644
>> --- a/block/backup.c
>> +++ b/block/backup.c
>> @@ -214,11 +214,25 @@ static void backup_iostatus_reset(BlockJob *job)
>>  }
>>  }
>>  
>> +static void backup_do_checkpoint(BlockJob *job, Error **errp)
>> +{
>> +BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common);
>> +
>> +if (backup_job->sync_mode != MIRROR_SYNC_MODE_NONE) {
>> +error_setg(errp, "The backup job only supports block checkpoint in"
>> +   " sync=none mode");
>> +return;
>> +}
>> +
>> +hbitmap_reset_all(backup_job->bitmap);
>> +}
> 
> Is this a fast way to stop and then start a new backup blockjob without
> emitting block job lifecycle events?
> 
> Not sure the blockjob_do_checkpoint() interface is appropriate.  Is
> there any other block job type that will implement .do_checkpoint()?

Currently, the answer is no.

> 
> COLO block replication could call a public backup_do_checkpoint()
> function.  That way the direct coupling between COLO and the backup
> block job is obvious.  I'm not convinced a generic interface like
> blockjob_do_checkpoint() makes sense since it's really not a generic
> operation that makes sense for other block job types.
> 
> void backup_do_checkpoint(BlockJob *job, Error **errp)
> {
> BackupBlockJob *s;
> 
> if (job->driver != backup_job_driver) {
> error_setg(errp, "expected backup block job type for "
>  "checkpoint, got %d", job->driver->job_type);
> return;
> }
> 
> s = container_of(job, BackupBlockJob, common);
> ...
> }

In a older version, I implement it like this, but Paolo didn't like it.

> 
> Please also make the function name and documentation more specific.
> Instead of "do" maybe this should be "pre" or "post" to indicate whether
> this happens before or after the checkpoint commit.  What happens if

OK

> this function returns an error?

We just return this error to COLO, and COLO will do failover.

Thanks
Wen Congyang

> .
> 

Re: [Qemu-devel] [Qemu-block] [PATCH v10 08/10] Implement new driver for block replication

[Fam Zheng]

On Tue, 10/13 17:46, Wen Congyang wrote:
> On 10/13/2015 05:41 PM, Fam Zheng wrote:
> > On Tue, 10/13 16:59, Wen Congyang wrote:
> >> On 10/13/2015 12:25 AM, Stefan Hajnoczi wrote:
> >>> On Fri, Sep 25, 2015 at 02:17:36PM +0800, Wen Congyang wrote:
>  +static void backup_job_completed(void *opaque, int ret)
>  +{
>  +BDRVReplicationState *s = opaque;
>  +
>  +if (s->replication_state != BLOCK_REPLICATION_DONE) {
>  +/* The backup job is cancelled unexpectedly */
>  +s->error = -EIO;
>  +}
>  +
>  +bdrv_op_block(s->hidden_disk, BLOCK_OP_TYPE_BACKUP_TARGET,
>  +  s->active_disk->backing_blocker);
>  +bdrv_op_block(s->secondary_disk, BLOCK_OP_TYPE_BACKUP_SOURCE,
>  +  s->hidden_disk->backing_blocker);
>  +
>  +bdrv_put_ref_bh_schedule(s->secondary_disk);
> >>>
> >>> Why is bdrv_put_ref_bh_schedule() necessary?
> >>
> >> It is copied from block_job_cb(). According to the comments in 
> >> bdrv_put_ref_bh_schedule():
> >> /*
> >>  * Release a BDS reference in a BH
> >>  *
> >>  * It is not safe to use bdrv_unref() from a callback function when the 
> >> callers
> >>  * still need the BlockDriverState.  In such cases we schedule a BH to 
> >> release
> >>  * the reference.
> >>  */
> >>
> >> If the comment is right, I think it is necessary to call 
> >> bdrv_put_ref_bh_schedule() here.
> >> Because the job is created on the BDS s->secondary disk, 
> >> backup_job_completed() is
> >> called in block_job_completed(), and we will still use s->secondary_disk 
> >> in block_job_release().
> > 
> > Where is the matching bdrv_ref called?
> 
> It is in block_job_create()
> 
> source: we call in bdrv_ref() in block_job_create(), and the user should 
> unref it.
> target: the user call bdrv_ref(), and we will unref it in the job
> 
> I don't know why we design it like this...
> 

Maybe it's better to unref it in block_job_release. Then we can simply drop
bdrv_put_ref_bh_schedule.

Fam

Re: [Qemu-devel] [PATCH v3 00/16] block: Get rid of bdrv_swap()

[Stefan Hajnoczi]

On Fri, Oct 09, 2015 at 02:15:25PM +0200, Kevin Wolf wrote:
> bdrv_swap() has always been an ugly hack that we would rather have
> avoided.  When it was introduced, we simply didn't have the
> infrastructure to update pointers instead of transplanting the contents
> of BDS object, so we grudgingly added bdrv_swap() as a quick solution.
> Meanwhile, most of the infrastructure exists and this series implements
> the final step necessary to implement the required functionality in a
> less adventurous way.
> 
> 
> v3:
> - Patch 4 ('quorum: Convert to BdrvChild')
>   Fixed stale comment [Max]
> 
> - Patch 7 ('block: Convert bs->backing_hd to BdrvChild')
>   Simplified bdrv_find_overlay() [Fam]
> 
> - Patch 8 ('block: Manage backing file references in bdrv_set_backing_hd()')
>   Fixed leaks in bdrv_drop_intermediate() [Berto]
> 
> 
> v2:
> - Patch 4 ('quorum: Convert to BdrvChild')
>   Use bdrv_unref_child() instead of bdrv_unref() [Berto]
> 
> - Patch 5 ('block: Convert bs->file to BdrvChild')
>   bdrv_close: Set bs->drv = NULL immediately after .bdrv_close [Berto]
>   bdrv_close: bdrv_unref_child() instead of bdrv_unref() for bs->file [Max]
> 
> - Patch 7 ('block: Convert bs->backing_hd to BdrvChild')
>   Use backing_bs() more consistently instead of open-coding and introducing
>   additional bugs while doing so [Max]
> 
> - Patch 8 ('block: Manage backing file references in bdrv_set_backing_hd()')
>   mirror: Use backing instead of s->base->backing->bs [Max]
>   stream: More simplication, remove close_unused_images() altogether [Berto]
> 
> - Patch 11 ('block-backend: Add blk_set_bs()')
>   assert(bs->blk == NULL) [Max]
> 
> - Patch 13 ('block: Implement bdrv_append() without bdrv_swap()')
>   bdrv_ref/unref in change_parent_backing_link [Max]
>   Improved throttle state handling in swap_feature_fields() [Berto]
>   Fixed external_snapshot_commit() [Berto]
> 
> - Patch 14 ('blockjob: Store device name at job creation')
>   Use new job->id more consistently [Max]
> 
> 
> Kevin Wolf (16):
>   block: Introduce BDS.file_child
>   vmdk: Use BdrvChild instead of BDS for references to extents
>   blkverify: Convert s->test_file to BdrvChild
>   quorum: Convert to BdrvChild
>   block: Convert bs->file to BdrvChild
>   block: Remove bdrv_open_image()
>   block: Convert bs->backing_hd to BdrvChild
>   block: Manage backing file references in bdrv_set_backing_hd()
>   block: Split bdrv_move_feature_fields()
>   block/io: Make bdrv_requests_pending() public
>   block-backend: Add blk_set_bs()
>   block: Introduce parents list
>   block: Implement bdrv_append() without bdrv_swap()
>   blockjob: Store device name at job creation
>   block: Add and use bdrv_replace_in_backing_chain()
>   block: Remove bdrv_swap()
> 
>  block.c   | 486 
> ++
>  block/blkdebug.c  |  32 +--
>  block/blkverify.c |  68 ---
>  block/block-backend.c |  17 ++
>  block/bochs.c |   8 +-
>  block/cloop.c |  10 +-
>  block/dmg.c   |  20 +-
>  block/io.c|  76 
>  block/mirror.c|  22 +--
>  block/parallels.c |  38 ++--
>  block/qapi.c  |  10 +-
>  block/qcow.c  |  46 ++---
>  block/qcow2-cache.c   |  11 +-
>  block/qcow2-cluster.c |  42 ++--
>  block/qcow2-refcount.c|  45 +++--
>  block/qcow2-snapshot.c|  30 +--
>  block/qcow2.c |  68 +++
>  block/qed-table.c |   4 +-
>  block/qed.c   |  51 +++--
>  block/quorum.c|  65 ---
>  block/raw_bsd.c   |  40 ++--
>  block/snapshot.c  |  12 +-
>  block/stream.c|  34 +---
>  block/vdi.c   |  17 +-
>  block/vhdx-log.c  |  25 +--
>  block/vhdx.c  |  36 ++--
>  block/vmdk.c  | 133 +++--
>  block/vpc.c   |  34 ++--
>  block/vvfat.c |  19 +-
>  blockdev.c|   6 +-
>  blockjob.c|  15 +-
>  include/block/block.h |  15 +-
>  include/block/block_int.h |  20 +-
>  include/block/blockjob.h  |   8 +
>  include/qemu/queue.h  |   6 -
>  qemu-img.c|  20 +-
>  36 files changed, 754 insertions(+), 835 deletions(-)
> 
> -- 
> 1.8.3.1
> 

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH v2 05/12] block: Introduce "drained begin/end" API

[Paolo Bonzini]

On 13/10/2015 11:31, Kevin Wolf wrote:
> This would mean that once you've sent an I/O request inside a drain
> section, you have to expect that more internal I/O might be going on
> after the request has completed. If you don't want this, you have to
> issue another bdrv_drain() or use a nested bdrv_drained_begin/end()
> section.

Yes.

> Sounds reasonable enough to me, but I guess this should be explicitly
> documented.

I agree.  Perhaps bdrv_drained_begin/end() could be renamed to
bdrv_drain_and_lock() / bdrv_unlock()?

Paolo

Re: [Qemu-devel] Live migration sequence

[Dr. David Alan Gilbert]

* Pavel Fedin (p.fe...@samsung.com) wrote:
>  Hello!
> 
>  Sorry for the delayed reply.

> > What's an ITS ?
> 
>  Interrupt Translation Service. In a short, it's a thing responsible for 
> handling PCIe MSI-X
> interrupts on ARM64 architecture.

OK; I asked Peter (cc'd) to explain a bit more about the ITS to me.

> > With a related question, how big are the tables and can it change during 
> > the iterated part
> > of the migrate?
> 
>  Tables are something like 64K each. They hold mappings between device/event 
> IDs and actual IRQ
> numbers.
>
>  Unfortunately i don't know how to answer the second part of the question, 
> about iterated part. Can
> you explain in details, what is it and how does it work?

QEMU migrates stuff in two different ways:
   a) Like a device where at the end of the migration it transmits all the 
information
   b) Like RAM, (iterated) - in this it sends the contents across but does this 
while the guest
 is still running, changes that are made to the RAM are then transmitted 
over and over again
 until the amount of changed RAM is small; then we stop the guest, transmit 
those last few
 changes and then do (a).

>  Or, well, we could put the question the other way: imagine that in pre_save 
> i tell my emulated
> device to flush its cached state into RAM-based tables. In post_load i could 
> tell the device to
> re-read data from RAM into its cache. So, what do i need in order to make 
> these tables in RAM to
> migrate correctly?

At 64k that's pretty small; however Peter explained to me that's per-cpu so 
that potentially
could be huge.  If it was only small I'd do it just like a device which is nice
and simple; but since it can get big then it's going to be more interesting, and
since it's part of guest RAM you need to be careful.

The pre_load/post_load are all relative to a particular device; they're
not hooked around when other stuff (RAM) gets migrated; it sounds like you
need another hook.

If I understand correctly what you need is to find a hook to dump the state
into guest ram, but then you also need to keep the state updated in guest
RAM during the migration.

Some thoughts:
  a) There is a migration state notifier list - see 
add_migration_state_change_notifier (spice calls it)
 - but I don't think it's called in the right places for your needs;  we
 could add some more places that gets called.

  b) Once you're in the device state saving (a above) you must not change guest 
RAM,
 because at that point the migration code won't send any new changes across
 to the destination. So any sync's you're going to do have to happen 
before/at
 the time we stop the CPU and do the final RAM sync.  On the plus side, when
 you're loading the device state in (a) you can be sure the RAM contents 
are there.

  c) Watch out for the size of that final sync; if you have lots of these ITS
 and they all update their 64k page at the point we stop the CPU then you're
 going to generate a lot of RAM that needs syncing.

Dave

> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] Live migration sequence

[Pavel Fedin]

 Hello!

 Sorry for the delayed reply.

> What's an ITS ?

 Interrupt Translation Service. In a short, it's a thing responsible for 
handling PCIe MSI-X
interrupts on ARM64 architecture.

> With a related question, how big are the tables and can it change during the 
> iterated part
> of the migrate?

 Tables are something like 64K each. They hold mappings between device/event 
IDs and actual IRQ
numbers.
 Unfortunately i don't know how to answer the second part of the question, 
about iterated part. Can
you explain in details, what is it and how does it work?
 Or, well, we could put the question the other way: imagine that in pre_save i 
tell my emulated
device to flush its cached state into RAM-based tables. In post_load i could 
tell the device to
re-read data from RAM into its cache. So, what do i need in order to make these 
tables in RAM to
migrate correctly?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PATCH 0/3] qemu-gdb: add functionality for inspecting core dumps

[Stefan Hajnoczi]

On Mon, Oct 12, 2015 at 10:02:51AM +0200, Paolo Bonzini wrote:
> Currently it is very hard to inspect coroutine in core dumps, because
> none of the qemu-gdb functionality works.  Fixing this is not really
> possible because "bt" only works on the core dump's stack pointer and
> program counter, but the situation would improve noticeably if only
> a coroutine's stack pointer and program counter were accessible at all;
> that would allow inspecting the coroutine's stack and building a
> stack trace from the hex dump of the stack.
> 
> The main hurdle is that glibc_pointer_guard() cannot be run on a core
> dump, because get_fs_base() uses the arch_prctl system call.  The first
> patch modifies that to use the gdb API instead.  The second and third
> patch then add the new functions.
> 
> Paolo
> 
> Paolo Bonzini (3):
>   qemu-gdb: allow using glibc_pointer_guard() on core dumps
>   qemu-gdb: extract parts of "qemu coroutine" implementation
>   qemu-gdb: add $qemu_coroutine_sp and $qemu_coroutine_pc
> 
>  scripts/qemu-gdb.py  |  3 ++
>  scripts/qemugdb/coroutine.py | 90 
> +---
>  2 files changed, 62 insertions(+), 31 deletions(-)

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

Stefan

[Qemu-devel] [PATCH v2 5/5] options: Add documentation for filter-dump

[Thomas Huth]

Add a short description for the filter-dump command line options.

Signed-off-by: Thomas Huth 
---
 qemu-options.hx | 8 
 1 file changed, 8 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index 2485b94..3ab753d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2012,6 +2012,7 @@ qemu -m 512 -object 
memory-backend-file,id=mem,size=512M,mem-path=/hugetlbfs,sha
 Dump network traffic on VLAN @var{n} to file @var{file} 
(@file{qemu-vlan0.pcap} by default).
 At most @var{len} bytes (64k by default) per packet are stored. The file 
format is
 libpcap, so it can be analyzed with tools such as tcpdump or Wireshark.
+Note: For devices created with '-netdev', use '-object filter-dump,...' 
instead.
 
 @item -net none
 Indicate that no network devices should be configured. It is used to
@@ -3660,6 +3661,13 @@ queue @var{all|rx|tx} is an option that can be applied 
to any netfilter.
 @option{tx}: the filter is attached to the transmit queue of the netdev,
  where it will receive packets sent by the netdev.
 
+@item -object 
filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}]
+
+Dump the network traffic on netdev @var{dev} to the file specified by
+@var{filename}. At most @var{len} bytes (64k by default) per packet are stored.
+The file format is libpcap, so it can be analyzed with tools such as tcpdump
+or Wireshark.
+
 @end table
 
 ETEXI
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v4 0/3] vhost: check if vhost has capacity for hotplugged memory

[Igor Mammedov]

On Tue,  6 Oct 2015 10:37:26 +0200
Igor Mammedov  wrote:

> v3->v4:
>   * rescan all vhost backends when checking for limit (Michael S. Tsirkin)
>   * check that used slots are less than limit of new backend (Michael S. 
> Tsirkin)
ping

> v2->v3:
>   * add refcounting of limit by # vhost devices,
> and make vhost_has_free_slot() return true if no vhost devices
> are present at current moment
>   * move limit initialization to vhost_dev_init()
>   * fail vhost backend intialization if memslots number is more than its 
> supported limit
> 
> v1->v2:
>   * replace probbing with checking for
> /sys/module/vhost/parameters/max_mem_regions and
> if it's missing has non wrong value return
> hardcoded legacy limit (64 slots).
> 
> it's defensive patchset which helps to avoid QEMU crashing
> at memory hotplug time by checking that vhost has free capacity
> for an additional memory slot.
> 
> Igor Mammedov (3):
>   vhost: add vhost_has_free_slot() interface
>   pc-dimm: add vhost slots limit check before commiting to hotplug
>   vhost: fail backend intialization if memslots number is more than its
> supported limit
> 
>  hw/mem/pc-dimm.c  |  7 +++
>  hw/virtio/vhost-backend.c | 19 +++
>  hw/virtio/vhost-user.c|  6 ++
>  hw/virtio/vhost.c | 27 +++
>  include/hw/virtio/vhost-backend.h |  2 ++
>  include/hw/virtio/vhost.h |  2 ++
>  stubs/Makefile.objs   |  1 +
>  stubs/vhost.c |  6 ++
>  8 files changed, 70 insertions(+)
>  create mode 100644 stubs/vhost.c
> 

Re: [Qemu-devel] [PULL v2 00/20] QAPI patches

[Peter Maydell]

On 12 October 2015 at 19:19, Markus Armbruster  wrote:
> The following changes since commit 5451316ed07b758a187dedf21047bed8f843f7f1:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' 
> into staging (2015-10-12 15:52:54 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2015-10-12
>
> for you to fetch changes up to 18bdbc3ac8b477e160d56aa6ecd6942495ce44d0:
>
>   qapi: Simplify gen_visit_fields() error handling (2015-10-12 18:46:50 +0200)
>
> 
> QAPI patches
>

Applied, thanks.

-- PMM

[Qemu-devel] [PATCH] Revert "blockdev: add note that block_job_cb() must be thread-safe"

[Fam Zheng]

This reverts commit 723c5d93c51bdb3adbc238ce90195c0864aa6cd5.

block_job_cb is called by block_job_completed, which is always called in
a main loop bottom half in existing block jobs. So we don't need to
worry about thread-safety here.

Signed-off-by: Fam Zheng 
---
 blockdev.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 32b04b4..130b7fb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2248,11 +2248,6 @@ out:
 
 static void block_job_cb(void *opaque, int ret)
 {
-/* Note that this function may be executed from another AioContext besides
- * the QEMU main loop.  If you need to access anything that assumes the
- * QEMU global mutex, use a BH or introduce a mutex.
- */
-
 BlockDriverState *bs = opaque;
 const char *msg = NULL;
 
-- 
2.6.1

Re: [Qemu-devel] [PATCH v3 2/2] remove function during multi-function hot-add

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 05:54:34PM +0800, Cao jin wrote:
> Hi Michael
> 
> On 10/13/2015 04:51 PM, Michael S. Tsirkin wrote:
> >On Tue, Oct 13, 2015 at 04:41:35PM +0800, Cao jin wrote:
> >>In case user regret when hot-adding multi-function, should roll back,
> >>device_del the function added but not exposed to the guest.
> >>
> >>Signed-off-by: Cao jin 
> >
> >I think this patch should come first, before we enable the
> >functionality that depends on it.
> >
> Do you mean, the function should be removed individually in any condition?

I just mean the patches in the series should be reordered.

> Because as you know, device_del pci_dev will remove all the functions in the
> slot that are fulled exposed to the guest, Alex also mentioned this
> limitation before.
> >
> >>---
> >>  hw/pci/pci_host.c |  6 +-
> >>  hw/pci/pcie.c | 22 +-
> >>  2 files changed, 22 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> >>index 3e26f92..35e5cf3 100644
> >>--- a/hw/pci/pci_host.c
> >>+++ b/hw/pci/pci_host.c
> >>@@ -20,6 +20,7 @@
> >>
> >>  #include "hw/pci/pci.h"
> >>  #include "hw/pci/pci_host.h"
> >>+#include "hw/pci/pci_bus.h"
> >>  #include "trace.h"
> >>
> >>  /* debug PCI */
> >>@@ -88,10 +89,13 @@ void pci_data_write(PCIBus *s, uint32_t addr, uint32_t 
> >>val, int len)
> >>  uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
> >>  {
> >>  PCIDevice *pci_dev = pci_dev_find_by_addr(s, addr);
> >>+PCIDevice *f0 = NULL;
> >>  uint32_t config_addr = addr & (PCI_CONFIG_SPACE_SIZE - 1);
> >>  uint32_t val;
> >>+uint8_t slot = (addr >> 11) & 0x1F;
> >>
> >>-if (!pci_dev) {
> >>+f0 = s->devices[PCI_DEVFN(slot, 0)];
> >>+if (!pci_dev || (!f0 && pci_dev)) {
> >>  return ~0x0;
> >>  }
> >>
> >>diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> >>index 89bf61b..58d2153 100644
> >>--- a/hw/pci/pcie.c
> >>+++ b/hw/pci/pcie.c
> >>@@ -261,13 +261,30 @@ void pcie_cap_slot_hotplug_cb(HotplugHandler 
> >>*hotplug_dev, DeviceState *dev,
> >>  }
> >>  }
> >>
> >>+static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> >>+{
> >>+object_unparent(OBJECT(dev));
> >>+}
> >>+
> >>  void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> >>   DeviceState *dev, Error **errp)
> >>  {
> >>  uint8_t *exp_cap;
> >>+PCIDevice *pci_dev = PCI_DEVICE(dev);
> >>+PCIBus *bus = pci_dev->bus;
> >>
> >>  pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, _cap, 
> >> errp);
> >>
> >>+/* In case user regret when hot-adding multi function, remove the 
> >>function
> >>+ * that is unexposed to guest individually, without interaction with 
> >>guest.
> >>+ */
> >>+if (PCI_FUNC(pci_dev->devfn) > 0 &&
> >>+bus->devices[PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 0)] == NULL) {
> >>+pcie_unplug_device(bus, pci_dev, NULL);
> >>+
> >>+return;
> >>+}
> >>+
> >>  pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
> >>  }
> >>
> >>@@ -378,11 +395,6 @@ void pcie_cap_slot_reset(PCIDevice *dev)
> >>  hotplug_event_update_event_status(dev);
> >>  }
> >>
> >>-static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> >>-{
> >>-object_unparent(OBJECT(dev));
> >>-}
> >>-
> >>  void pcie_cap_slot_write_config(PCIDevice *dev,
> >>  uint32_t addr, uint32_t val, int len)
> >>  {
> >>--
> >>2.1.0
> >.
> >
> 
> -- 
> Yours Sincerely,
> 
> Cao Jin

[Qemu-devel] [PATCH v2 0/5] Network traffic dumping via netfilter

[Thomas Huth]

The "-net dump" option only works with the "-net" option. So far, it
is not possible to dump network traffic with the "-netdev" option yet.
This patch series now fixes this ugliness by enabling dumping for the
netdev devices via the new netfilter infrastructure from Yang Hongyang.
The dumping filter can be used like this for example:

 ppc64-softmmu/qemu-system-ppc64 -device virtio-net,netdev=mynet \
 -netdev user,id=mynet,tftp=/tmp/tftp,bootfile=zImage \
 -object filter-dump,id=f0,netdev=mynet,file=/tmp/dumpfile.dat

Changes in v2:
- Only rebased to master branch (to suit the final version of the
  netfilter patches that are now merged into master)

Thomas Huth (5):
  net/dump: Add support for receive_iov function
  net/dump: Rework net-dump init functions
  net/dump: Separate the NetClientState from the DumpState
  net/dump: Provide the dumping facility as a net filter
  options: Add documentation for filter-dump

 net/dump.c  | 228 
 qemu-options.hx |   8 ++
 vl.c|   7 +-
 3 files changed, 212 insertions(+), 31 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 2/5] net/dump: Rework net-dump init functions

[Thomas Huth]

Move the creation of the dump client from net_dump_init() into
net_init_dump(), so we can later use the former function for
dump via netfilter, too. Also rename net_dump_init() to
net_dump_state_init() to make it easier distinguishable from
net_init_dump().

Signed-off-by: Thomas Huth 
---
 net/dump.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index aa0d45d..e6f6be0 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -118,13 +118,10 @@ static NetClientInfo net_dump_info = {
 .cleanup = dump_cleanup,
 };
 
-static int net_dump_init(NetClientState *peer, const char *device,
- const char *name, const char *filename, int len,
- Error **errp)
+static int net_dump_state_init(DumpState *s, const char *filename,
+   int len, Error **errp)
 {
 struct pcap_file_hdr hdr;
-NetClientState *nc;
-DumpState *s;
 struct tm tm;
 int fd;
 
@@ -148,13 +145,6 @@ static int net_dump_init(NetClientState *peer, const char 
*device,
 return -1;
 }
 
-nc = qemu_new_net_client(_dump_info, peer, device, name);
-
-snprintf(nc->info_str, sizeof(nc->info_str),
- "dump to %s (len=%d)", filename, len);
-
-s = DO_UPCAST(DumpState, nc, nc);
-
 s->fd = fd;
 s->pcap_caplen = len;
 
@@ -167,10 +157,11 @@ static int net_dump_init(NetClientState *peer, const char 
*device,
 int net_init_dump(const NetClientOptions *opts, const char *name,
   NetClientState *peer, Error **errp)
 {
-int len;
+int len, rc;
 const char *file;
 char def_file[128];
 const NetdevDumpOptions *dump;
+NetClientState *nc;
 
 assert(opts->kind == NET_CLIENT_OPTIONS_KIND_DUMP);
 dump = opts->dump;
@@ -200,5 +191,13 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
 len = 65536;
 }
 
-return net_dump_init(peer, "dump", name, file, len, errp);
+nc = qemu_new_net_client(_dump_info, peer, "dump", name);
+snprintf(nc->info_str, sizeof(nc->info_str),
+ "dump to %s (len=%d)", file, len);
+
+rc = net_dump_state_init(DO_UPCAST(DumpState, nc, nc), file, len, errp);
+if (rc) {
+qemu_del_net_client(nc);
+}
+return rc;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/5] net/dump: Add support for receive_iov function

[Thomas Huth]

Adding a proper receive_iov function to the net dump module.
This will make it easier to support the dump filter feature for
the -netdev option in later patches.

Signed-off-by: Thomas Huth 
---
 net/dump.c | 24 +---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index 08259af..aa0d45d 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -25,6 +25,7 @@
 #include "clients.h"
 #include "qemu-common.h"
 #include "qemu/error-report.h"
+#include "qemu/iov.h"
 #include "qemu/log.h"
 #include "qemu/timer.h"
 #include "hub.h"
@@ -57,12 +58,15 @@ struct pcap_sf_pkthdr {
 uint32_t len;
 };
 
-static ssize_t dump_receive(NetClientState *nc, const uint8_t *buf, size_t 
size)
+static ssize_t dump_receive_iov(NetClientState *nc, const struct iovec *iov,
+int cnt)
 {
 DumpState *s = DO_UPCAST(DumpState, nc, nc);
 struct pcap_sf_pkthdr hdr;
 int64_t ts;
 int caplen;
+size_t size = iov_size(iov, cnt);
+struct iovec dumpiov[cnt + 1];
 
 /* Early return in case of previous error. */
 if (s->fd < 0) {
@@ -76,8 +80,12 @@ static ssize_t dump_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
 hdr.ts.tv_usec = ts % 100;
 hdr.caplen = caplen;
 hdr.len = size;
-if (write(s->fd, , sizeof(hdr)) != sizeof(hdr) ||
-write(s->fd, buf, caplen) != caplen) {
+
+dumpiov[0].iov_base = 
+dumpiov[0].iov_len = sizeof(hdr);
+cnt = iov_copy([1], cnt, iov, cnt, 0, caplen);
+
+if (writev(s->fd, dumpiov, cnt + 1) != sizeof(hdr) + caplen) {
 qemu_log("-net dump write error - stop dump\n");
 close(s->fd);
 s->fd = -1;
@@ -86,6 +94,15 @@ static ssize_t dump_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
 return size;
 }
 
+static ssize_t dump_receive(NetClientState *nc, const uint8_t *buf, size_t 
size)
+{
+struct iovec iov = {
+.iov_base = (void *)buf,
+.iov_len = size
+};
+return dump_receive_iov(nc, , 1);
+}
+
 static void dump_cleanup(NetClientState *nc)
 {
 DumpState *s = DO_UPCAST(DumpState, nc, nc);
@@ -97,6 +114,7 @@ static NetClientInfo net_dump_info = {
 .type = NET_CLIENT_OPTIONS_KIND_DUMP,
 .size = sizeof(DumpState),
 .receive = dump_receive,
+.receive_iov = dump_receive_iov,
 .cleanup = dump_cleanup,
 };
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 02/22] ide: Account for write operations correctly

[Stefan Hajnoczi]

On Fri, Oct 02, 2015 at 05:26:12PM +0300, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia 
> ---
>  hw/ide/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PULL v2 00/50] Ivshmem patches

[Peter Maydell]

On 12 October 2015 at 17:40,   wrote:
> From: Marc-André Lureau 
>
> The following changes since commit 0bf224d5da41967a775b328234cda2d19f303908:
>
>   Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into 
> staging (2015-10-12 14:29:29 +0100)
>
> are available in the git repository at:
>
>   g...@github.com:elmarco/qemu.git tags/ivshmem-pull-request
>
> for you to fetch changes up to a3920e445af166ae95651a63bcd0d3b4694eed14:
>
>   doc: document ivshmem & hugepages (2015-10-12 18:02:51 +0200)
>
> 
> This new pull request includes Andreas qtest patch, a doc patch, and
> fixes some new glib API usage in tests.
> 

Hi. I'm afraid this fails to build on OSX:

/Users/pm215/src/qemu-for-merges/contrib/ivshmem-server/ivshmem-server.c:13:10:
fatal error: 'sys/even
tfd.h' file not found
#include 
 ^
  CCcontrib/ivshmem-server/main.o
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:111:48:
warning: format speci
fies type 'long' but the argument has type 'int64_t' (aka 'long long')
[-Wformat]
 "peer %ld\n", peer_id);
   ~~~ ^~~
   %lld
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:21:28:
note: expanded from ma
cro 'IVSHMEM_CLIENT_DEBUG'
printf(fmt, ## __VA_ARGS__); \
   ^
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:115:64:
warning: format speci
fies type 'long' but the argument has type 'int64_t' (aka 'long long')
[-Wformat]
IVSHMEM_CLIENT_DEBUG(client, "delete peer id = %ld\n", peer_id);
   ~~~ ^~~
   %lld
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:21:28:
note: expanded from ma
cro 'IVSHMEM_CLIENT_DEBUG'
printf(fmt, ## __VA_ARGS__); \
   ^
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:126:61:
warning: format speci
fies type 'long' but the argument has type 'int64_t' (aka 'long long')
[-Wformat]
IVSHMEM_CLIENT_DEBUG(client, "new peer id = %ld\n", peer_id);
~~~ ^~~
%lld
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:21:28:
note: expanded from ma
cro 'IVSHMEM_CLIENT_DEBUG'
printf(fmt, ## __VA_ARGS__); \
   ^
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:131:51:
warning: format speci
fies type 'long' but the argument has type 'int64_t' (aka 'long long')
[-Wformat]
 peer->vectors_count, fd, peer->id);
  ^~~~
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:21:28:
note: expanded from ma
cro 'IVSHMEM_CLIENT_DEBUG'
printf(fmt, ## __VA_ARGS__); \
   ^
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:223:50:
warning: format speci
fies type 'long' but the argument has type 'int64_t' (aka 'long long')
[-Wformat]
IVSHMEM_CLIENT_DEBUG(client, "our_id=%ld\n", client->local.id);
 ~~~ ^~~~
 %lld
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:21:28:
note: expanded from macro 'IVSHMEM_CLIENT_DEBUG'
printf(fmt, ## __VA_ARGS__); \
   ^
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:318:51:
warning: format specifies type 'long' but the argument has type
'uint64_t' (aka 'unsigned long long') [-Wformat]
 peer->vectors[i], i, kick);
  ^~~~
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:21:28:
note: expanded from macro 'IVSHMEM_CLIENT_DEBUG'
printf(fmt, ## __VA_ARGS__); \
   ^
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:355:30:
warning: format specifies type 'long' but the argument has type
'int64_t' (aka 'long long') [-Wformat]
 peer->id);
 ^~~~
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:21:28:
note: expanded from macro 'IVSHMEM_CLIENT_DEBUG'
printf(fmt, ## __VA_ARGS__); \
   ^
/Users/pm215/src/qemu-for-merges/contrib/ivshmem-client/ivshmem-client.c:360:26:
warning: format specifies type 'long' but the argument has type
'int64_t' (aka 'long long') [-Wformat]

[Qemu-devel] [PATCH v3 2/2] target-arm: Fix CPU breakpoint handling

[Sergey Fedorov]

A QEMU breakpoint match is not definitely an architectural breakpoint
match. If an exception is generated unconditionally during translation,
it is hardly possible to ignore it in the debug exception handler.

Generate a call to a helper to check CPU breakpoints and raise an
exception only if any breakpoint matches architecturally.

Signed-off-by: Sergey Fedorov 
---
 target-arm/helper.h|  2 ++
 target-arm/op_helper.c | 29 ++---
 target-arm/translate-a64.c | 17 -
 target-arm/translate.c | 19 ++-
 4 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/target-arm/helper.h b/target-arm/helper.h
index 827b33d..c2a85c7 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -54,6 +54,8 @@ DEF_HELPER_1(yield, void, env)
 DEF_HELPER_1(pre_hvc, void, env)
 DEF_HELPER_2(pre_smc, void, env, i32)
 
+DEF_HELPER_1(check_breakpoints, void, env)
+
 DEF_HELPER_3(cpsr_write, void, env, i32, i32)
 DEF_HELPER_1(cpsr_read, i32, env)
 
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 67b18c0..7929c71 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -867,6 +867,15 @@ static bool check_breakpoints(ARMCPU *cpu)
 return false;
 }
 
+void HELPER(check_breakpoints)(CPUARMState *env)
+{
+ARMCPU *cpu = arm_env_get_cpu(env);
+
+if (check_breakpoints(cpu)) {
+HELPER(exception_internal(env, EXCP_DEBUG));
+}
+}
+
 void arm_debug_excp_handler(CPUState *cs)
 {
 /* Called by core code when a watchpoint or breakpoint fires;
@@ -898,23 +907,21 @@ void arm_debug_excp_handler(CPUState *cs)
 }
 } else {
 uint64_t pc = is_a64(env) ? env->pc : env->regs[15];
+bool same_el = (arm_debug_target_el(env) == arm_current_el(env));
 
 if (cpu_breakpoint_test(cs, pc, BP_GDB)) {
 return;
 }
 
-if (check_breakpoints(cpu)) {
-bool same_el = (arm_debug_target_el(env) == arm_current_el(env));
-if (extended_addresses_enabled(env)) {
-env->exception.fsr = (1 << 9) | 0x22;
-} else {
-env->exception.fsr = 0x2;
-}
-/* FAR is UNKNOWN, so doesn't need setting */
-raise_exception(env, EXCP_PREFETCH_ABORT,
-syn_breakpoint(same_el),
-arm_debug_target_el(env));
+if (extended_addresses_enabled(env)) {
+env->exception.fsr = (1 << 9) | 0x22;
+} else {
+env->exception.fsr = 0x2;
 }
+/* FAR is UNKNOWN, so doesn't need setting */
+raise_exception(env, EXCP_PREFETCH_ABORT,
+syn_breakpoint(same_el),
+arm_debug_target_el(env));
 }
 }
 
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index e65e309..09a5dde 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -11084,11 +11084,18 @@ void gen_intermediate_code_a64(ARMCPU *cpu, 
TranslationBlock *tb)
 CPUBreakpoint *bp;
 QTAILQ_FOREACH(bp, >breakpoints, entry) {
 if (bp->pc == dc->pc) {
-gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
-/* Advance PC so that clearing the breakpoint will
-   invalidate this TB.  */
-dc->pc += 2;
-goto done_generating;
+if (bp->flags & BP_CPU) {
+gen_helper_check_breakpoints(cpu_env);
+/* End the TB early; it likely won't be executed */
+dc->is_jmp = DISAS_UPDATE;
+} else {
+gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
+/* Advance PC so that clearing the breakpoint will
+   invalidate this TB.  */
+dc->pc += 4;
+goto done_generating;
+}
+break;
 }
 }
 }
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 22c3587..0652721 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11329,11 +11329,20 @@ void gen_intermediate_code(CPUARMState *env, 
TranslationBlock *tb)
 CPUBreakpoint *bp;
 QTAILQ_FOREACH(bp, >breakpoints, entry) {
 if (bp->pc == dc->pc) {
-gen_exception_internal_insn(dc, 0, EXCP_DEBUG);
-/* Advance PC so that clearing the breakpoint will
-   invalidate this TB.  */
-dc->pc += 2;
-goto done_generating;
+if (bp->flags & BP_CPU) {
+gen_helper_check_breakpoints(cpu_env);
+/* End the TB early; it's likely not going to be 
executed */
+dc->is_jmp = 

[Qemu-devel] [PATCH v3] target-arm: Fix breakpoint handling

[Sergey Fedorov]

This series is intended to fix ARM breakpoint emulation misbehavior.
QEMU hangs when QEMU breakpoint fires but it does not pass additional
architectural checks in ARM CPU debug exception handler. For details,
please see individual patches. The most relevant parts of the original
discussion about ARM breakpoint emulation misbehavior can be found at:
https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02715.html

Changes in v3:
 * GDB breakpoint check in arm_debug_excp_handler() simplified
 * Debug exception generating fixed in translation code

Changes in v2:
 * The initial commit split into two parts
 * Extra block eliminated in arm_debug_excp_handler()
 * Just one instruction always translated in case of breakpoint PC match

Sergey Fedorov (1):
  target-arm: Fix CPU breakpoint handling

 target-arm/helper.h|  2 ++
 target-arm/op_helper.c | 29 ++---
 target-arm/translate-a64.c | 17 -
 target-arm/translate.c | 17 -
 4 files changed, 44 insertions(+), 21 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v3 1/2] target-arm: Fix GDB breakpoint handling

[Sergey Fedorov]

GDB breakpoints have higher priority so they have to be checked first.
Should GDB breakpoint match, just return from the debug exception
handler.

Signed-off-by: Sergey Fedorov 
---
 target-arm/op_helper.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 1425a1d..67b18c0 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -897,6 +897,12 @@ void arm_debug_excp_handler(CPUState *cs)
 }
 }
 } else {
+uint64_t pc = is_a64(env) ? env->pc : env->regs[15];
+
+if (cpu_breakpoint_test(cs, pc, BP_GDB)) {
+return;
+}
+
 if (check_breakpoints(cpu)) {
 bool same_el = (arm_debug_target_el(env) == arm_current_el(env));
 if (extended_addresses_enabled(env)) {
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 05/12] block: Introduce "drained begin/end" API

[Kevin Wolf]

Am 12.10.2015 um 16:27 hat Paolo Bonzini geschrieben:
> 
> 
> On 12/10/2015 13:50, Fam Zheng wrote:
> > +void bdrv_drained_begin(BlockDriverState *bs)
> > +{
> > +if (bs->quiesce_counter++) {
> > +return;
> > +}
> > +aio_disable_external(bdrv_get_aio_context(bs));
> > +bdrv_drain(bs);
> > +}
> 
> I think bdrv_drain should be called unconditionally, i.e. before the
> "if".  This should also solve Kevin's doubt about new allocating write
> request reenabling the timer: any write request from the drained section
> happens normally, until you get a nested drain request and then the
> callback completes the requests.

This would mean that once you've sent an I/O request inside a drain
section, you have to expect that more internal I/O might be going on
after the request has completed. If you don't want this, you have to
issue another bdrv_drain() or use a nested bdrv_drained_begin/end()
section.

Sounds reasonable enough to me, but I guess this should be explicitly
documented.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH v10 08/10] Implement new driver for block replication

[Fam Zheng]

On Tue, 10/13 16:59, Wen Congyang wrote:
> On 10/13/2015 12:25 AM, Stefan Hajnoczi wrote:
> > On Fri, Sep 25, 2015 at 02:17:36PM +0800, Wen Congyang wrote:
> >> +static void backup_job_completed(void *opaque, int ret)
> >> +{
> >> +BDRVReplicationState *s = opaque;
> >> +
> >> +if (s->replication_state != BLOCK_REPLICATION_DONE) {
> >> +/* The backup job is cancelled unexpectedly */
> >> +s->error = -EIO;
> >> +}
> >> +
> >> +bdrv_op_block(s->hidden_disk, BLOCK_OP_TYPE_BACKUP_TARGET,
> >> +  s->active_disk->backing_blocker);
> >> +bdrv_op_block(s->secondary_disk, BLOCK_OP_TYPE_BACKUP_SOURCE,
> >> +  s->hidden_disk->backing_blocker);
> >> +
> >> +bdrv_put_ref_bh_schedule(s->secondary_disk);
> > 
> > Why is bdrv_put_ref_bh_schedule() necessary?
> 
> It is copied from block_job_cb(). According to the comments in 
> bdrv_put_ref_bh_schedule():
> /*
>  * Release a BDS reference in a BH
>  *
>  * It is not safe to use bdrv_unref() from a callback function when the 
> callers
>  * still need the BlockDriverState.  In such cases we schedule a BH to release
>  * the reference.
>  */
> 
> If the comment is right, I think it is necessary to call 
> bdrv_put_ref_bh_schedule() here.
> Because the job is created on the BDS s->secondary disk, 
> backup_job_completed() is
> called in block_job_completed(), and we will still use s->secondary_disk in 
> block_job_release().

Where is the matching bdrv_ref called?

Fam

Re: [Qemu-devel] [Qemu-block] [PATCH v10 08/10] Implement new driver for block replication

[Wen Congyang]

On 10/13/2015 05:41 PM, Fam Zheng wrote:
> On Tue, 10/13 16:59, Wen Congyang wrote:
>> On 10/13/2015 12:25 AM, Stefan Hajnoczi wrote:
>>> On Fri, Sep 25, 2015 at 02:17:36PM +0800, Wen Congyang wrote:
 +static void backup_job_completed(void *opaque, int ret)
 +{
 +BDRVReplicationState *s = opaque;
 +
 +if (s->replication_state != BLOCK_REPLICATION_DONE) {
 +/* The backup job is cancelled unexpectedly */
 +s->error = -EIO;
 +}
 +
 +bdrv_op_block(s->hidden_disk, BLOCK_OP_TYPE_BACKUP_TARGET,
 +  s->active_disk->backing_blocker);
 +bdrv_op_block(s->secondary_disk, BLOCK_OP_TYPE_BACKUP_SOURCE,
 +  s->hidden_disk->backing_blocker);
 +
 +bdrv_put_ref_bh_schedule(s->secondary_disk);
>>>
>>> Why is bdrv_put_ref_bh_schedule() necessary?
>>
>> It is copied from block_job_cb(). According to the comments in 
>> bdrv_put_ref_bh_schedule():
>> /*
>>  * Release a BDS reference in a BH
>>  *
>>  * It is not safe to use bdrv_unref() from a callback function when the 
>> callers
>>  * still need the BlockDriverState.  In such cases we schedule a BH to 
>> release
>>  * the reference.
>>  */
>>
>> If the comment is right, I think it is necessary to call 
>> bdrv_put_ref_bh_schedule() here.
>> Because the job is created on the BDS s->secondary disk, 
>> backup_job_completed() is
>> called in block_job_completed(), and we will still use s->secondary_disk in 
>> block_job_release().
> 
> Where is the matching bdrv_ref called?

It is in block_job_create()

source: we call in bdrv_ref() in block_job_create(), and the user should unref 
it.
target: the user call bdrv_ref(), and we will unref it in the job

I don't know why we design it like this...

Thanks
Wen Congyang

> 
> Fam
> .
> 

Re: [Qemu-devel] [PATCH] block: qemu-iotests - fix vmdk test 059.out

[Kevin Wolf]

Am 13.10.2015 um 03:21 hat Fam Zheng geschrieben:
> On Mon, 10/12 20:18, Jeff Cody wrote:
> > In commit fe646693acc13ac48b98435d14149ab04dc597bc, the option
> > printout format changed.
> > 
> > This updates the VMDK test 059.out to the correct output.
> > 
> > Signed-off-by: Jeff Cody 
> 
> Reviewed-by: Fam Zheng 

Thanks, applied to the block branch.

Kevin

[Qemu-devel] [PATCH v2 2/2] aio: Introduce aio-epoll.c

[Fam Zheng]

To minimize code duplication, epoll is hooked into aio-posix's
aio_poll() instead of rolling its own. This approach also has both
compile-time and run-time switchability.

1) When QEMU starts with a small number of fds in the event loop, ppoll
is used.

2) When QEMU starts with a big number of fds, or when more devices are
hot plugged, epoll kicks in when the number of fds hits the threshold.

3) Some fds may not support epoll, such as tty based stdio. In this
case, it falls back to ppoll.

A rough benchmark with scsi-disk on virtio-scsi dataplane (epoll gets
enabled from 64 onward). Numbers are in MB/s.

===
 | master | epoll
 ||
scsi disks # | readrandrw | readrandrw
-||
1| 74  36 | 75  37
8| 74  36 | 73  37
32   | 65  32 | 63  32
64   | 52  25 | 66  32
128  | 42  21 | 54  27
256  | 26  15 | 38  19
===

Signed-off-by: Fam Zheng 
---
 aio-posix.c | 173 +++-
 include/block/aio.h |   6 ++
 2 files changed, 177 insertions(+), 2 deletions(-)

diff --git a/aio-posix.c b/aio-posix.c
index 597cb35..3e88d7f 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -17,6 +17,9 @@
 #include "block/block.h"
 #include "qemu/queue.h"
 #include "qemu/sockets.h"
+#ifdef CONFIG_EPOLL
+#include 
+#endif
 
 struct AioHandler
 {
@@ -28,6 +31,150 @@ struct AioHandler
 QLIST_ENTRY(AioHandler) node;
 };
 
+#ifdef CONFIG_EPOLL
+
+/* The fd number threashold to switch to epoll */
+#define EPOLL_ENABLE_THRESHOLD 64
+
+static void aio_epoll_disable(AioContext *ctx)
+{
+ctx->epoll_available = false;
+if (!ctx->epoll_enabled) {
+return;
+}
+ctx->epoll_enabled = false;
+close(ctx->epollfd);
+}
+
+static inline int epoll_events_from_pfd(int pfd_events)
+{
+return (pfd_events & G_IO_IN ? EPOLLIN : 0) |
+   (pfd_events & G_IO_OUT ? EPOLLOUT : 0) |
+   (pfd_events & G_IO_HUP ? EPOLLHUP : 0) |
+   (pfd_events & G_IO_ERR ? EPOLLERR : 0);
+}
+
+static bool aio_epoll_try_enable(AioContext *ctx)
+{
+AioHandler *node;
+struct epoll_event event;
+if (!ctx->epoll_available) {
+return false;
+}
+
+QLIST_FOREACH(node, >aio_handlers, node) {
+int r;
+if (node->deleted || !node->pfd.events) {
+continue;
+}
+event.events = epoll_events_from_pfd(node->pfd.events);
+event.data.ptr = node;
+r = epoll_ctl(ctx->epollfd, EPOLL_CTL_ADD, node->pfd.fd, );
+if (r) {
+return false;
+}
+}
+ctx->epoll_enabled = true;
+return true;
+}
+
+static void aio_epoll_update(AioContext *ctx, AioHandler *node, bool is_new)
+{
+struct epoll_event event;
+int r;
+
+if (!ctx->epoll_enabled) {
+return;
+}
+if (!node->pfd.events) {
+r = epoll_ctl(ctx->epollfd, EPOLL_CTL_DEL, node->pfd.fd, );
+assert(!r);
+} else {
+event.data.ptr = node;
+event.events = epoll_events_from_pfd(node->pfd.events);
+if (is_new) {
+r = epoll_ctl(ctx->epollfd, EPOLL_CTL_ADD, node->pfd.fd, );
+if (r) {
+aio_epoll_disable(ctx);
+}
+} else {
+r = epoll_ctl(ctx->epollfd, EPOLL_CTL_MOD, node->pfd.fd, );
+assert(!r);
+}
+}
+}
+
+static int aio_epoll(AioContext *ctx, GPollFD *pfds,
+ unsigned npfd, int64_t timeout)
+{
+AioHandler *node;
+int i, ret = 0;
+struct epoll_event events[128];
+
+assert(npfd == 1);
+assert(pfds[0].fd == ctx->epollfd);
+if (timeout > 0) {
+ret = qemu_poll_ns(pfds, npfd, timeout);
+}
+if (timeout <= 0 || ret > 0) {
+ret = epoll_wait(ctx->epollfd, events,
+ sizeof(events) / sizeof(events[0]),
+ timeout);
+if (ret <= 0) {
+goto out;
+}
+for (i = 0; i < ret; i++) {
+int ev = events[i].events;
+node = events[i].data.ptr;
+node->pfd.revents = (ev & EPOLLIN ? G_IO_IN : 0) |
+(ev & EPOLLOUT ? G_IO_OUT : 0) |
+(ev & EPOLLHUP ? G_IO_HUP : 0) |
+(ev & EPOLLERR ? G_IO_ERR : 0);
+}
+}
+out:
+return ret;
+}
+
+static bool aio_epoll_check_poll(AioContext *ctx, GPollFD *pfds,
+ unsigned npfd, int64_t timeout)
+{
+if (!ctx->epoll_available) {
+return false;
+}
+if (ctx->epoll_enabled) {
+return true;
+}
+if (npfd >= EPOLL_ENABLE_THRESHOLD) {
+if (aio_epoll_try_enable(ctx)) {
+return true;
+} else {
+aio_epoll_disable(ctx);
+ 

[Qemu-devel] [PATCH v2 1/2] aio: Introduce aio_context_setup

[Fam Zheng]

This is the place to initialize platform specific bits of AioContext.

Signed-off-by: Fam Zheng 
---
 aio-posix.c |  4 
 aio-win32.c |  4 
 async.c | 13 +++--
 include/block/aio.h |  8 
 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/aio-posix.c b/aio-posix.c
index d477033..597cb35 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -297,3 +297,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
 
 return progress;
 }
+
+void aio_context_setup(AioContext *ctx, Error **errp)
+{
+}
diff --git a/aio-win32.c b/aio-win32.c
index 50a6867..4742af3 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -363,3 +363,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
 aio_context_release(ctx);
 return progress;
 }
+
+void aio_context_setup(AioContext *ctx, Error **errp)
+{
+}
diff --git a/async.c b/async.c
index efce14b..f4b1410 100644
--- a/async.c
+++ b/async.c
@@ -320,12 +320,18 @@ AioContext *aio_context_new(Error **errp)
 {
 int ret;
 AioContext *ctx;
+Error *local_err = NULL;
+
 ctx = (AioContext *) g_source_new(_source_funcs, sizeof(AioContext));
+aio_context_setup(ctx, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+goto fail;
+}
 ret = event_notifier_init(>notifier, false);
 if (ret < 0) {
-g_source_destroy(>source);
 error_setg_errno(errp, -ret, "Failed to initialize event notifier");
-return NULL;
+goto fail;
 }
 g_source_set_can_recurse(>source, true);
 aio_set_event_notifier(ctx, >notifier,
@@ -339,6 +345,9 @@ AioContext *aio_context_new(Error **errp)
 ctx->notify_dummy_bh = aio_bh_new(ctx, notify_dummy_bh, NULL);
 
 return ctx;
+fail:
+g_source_destroy(>source);
+return NULL;
 }
 
 void aio_context_ref(AioContext *ctx)
diff --git a/include/block/aio.h b/include/block/aio.h
index 400b1b0..a6fd5ad 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -373,4 +373,12 @@ static inline void aio_timer_init(AioContext *ctx,
  */
 int64_t aio_compute_timeout(AioContext *ctx);
 
+/**
+ * aio_context_setup:
+ * @ctx: the aio context
+ *
+ * Initialize the aio context.
+ */
+void aio_context_setup(AioContext *ctx, Error **errp);
+
 #endif
-- 
2.6.1

Re: [Qemu-devel] [PATCH v8 13/18] qapi: Track owner of each object member

[Markus Armbruster]

Eric Blake  writes:

> Future commits will migrate semantic checking away from parsing
> and over to the various QAPISchema*.check() methods.  But to
> report an error message about an incorrect semantic use of a
> member of an object type, it helps to know which type, command,
> or event owns the member.  In particular, when a member is
> inherited from a base type, it is desirable to associate the
> member name with the base type (and not the type calling
> member.check()).
>
> Rather than packing additional information into the seen array
> passed to each member.check() (as in seen[m.name] = {'member':m,
> 'owner':type}), it is easier to have each member track the name
> of the owner type in the first place (keeping things simpler
> with the existing seen[m.name] = m).  The new member.owner field
> is set via a new set_owner() function, called when registering

method

> the members and variants arrays with an object or variant type.
> Track only a name, and not the actual type object, to avoid
> creating a circular python reference chain.
>
> The source information is intended for human consumption in
> error messages, and a new describe() method is added to access
> the resulting information.  For example, given the qapi:
>   { 'command': 'foo', 'data': { 'string': 'str' } }
> an implementation of visit_command() that calls
>   arg_type.members[0].describe()
> will see "'string' (argument of foo)".
>
> To make the human-readable name of implicit types work without
> duplicating efforts, the describe() method has to reverse the
> name of implicit types.
>
> No change to generated code.
>
> Signed-off-by: Eric Blake 
>
> ---
> v8: don't munge implicit type names [except for event data], and
> instead make describe() create nicer messages. Add set_owner(), and
> use field 'role' instead of method _describe()
> v7: total rewrite: rework implicit object names, assign owner
> when initializing owner type rather than when creating member
> python object
> v6: rebase on new lazy array creation and simple union 'type'
> motion; tweak commit message
> ---
>  scripts/qapi.py| 48 
> +++---
>  tests/qapi-schema/qapi-schema-test.out |  8 +++---
>  2 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index c9ce9ee..8b29e11 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -952,8 +952,10 @@ class QAPISchemaObjectType(QAPISchemaType):
>  assert base is None or isinstance(base, str)
>  for m in local_members:
>  assert isinstance(m, QAPISchemaObjectTypeMember)
> -assert (variants is None or
> -isinstance(variants, QAPISchemaObjectTypeVariants))
> +m.set_owner(name)
> +if variants is not None:
> +assert isinstance(variants, QAPISchemaObjectTypeVariants)
> +variants.set_owner(name)
>  self._base_name = base
>  self.base = None
>  self.local_members = local_members
> @@ -1014,8 +1016,14 @@ class QAPISchemaObjectTypeMember(object):
>  self._type_name = typ
>  self.type = None
>  self.optional = optional
> +self.owner = None
> +
> +def set_owner(self, name):
> +assert not self.owner
> +self.owner = name
>
>  def check(self, schema, all_members, seen):
> +assert self.owner
>  assert self.name not in seen
>  self.type = schema.lookup_type(self._type_name)
>  assert self.type
> @@ -1025,6 +1033,25 @@ class QAPISchemaObjectTypeMember(object):
>  def c_name(self):
>  return c_name(self.name)
>
> +def describe(self):
> +owner = self.owner
> +# See QAPISchema._make_implicit_object_type() - reverse the
> +# mapping there to create a nice human-readable description
> +if owner.startswith(':obj-'):
> +owner = owner[5:]
> +if owner.endswith('-arg'):
> +source = '(argument of %s)' % owner[:4]
> +elif owner.endswith('-data'):
> +source = '(data of %s)' % owner[:5]
> +else:
> +assert owner.endswith('-wrapper')
> +source = '(branch of %s)' % owner[:8]
> +else:
> +source = '(%s of %s)' % (self.role, owner)
> +return "'%s' %s" % (self.name, source)

Perhaps not exactly pretty, but it gets the job done with minimal fuss.
Sold.

> +
> +role = 'member'
> +
>

Class variables are usually defined first, before methods.

>  # TODO Drop this class once we no longer have the 'type'/'kind' mismatch
>  class QAPISchemaObjectTypeUnionTag(QAPISchemaObjectTypeMember):
> @@ -1034,6 +1061,11 @@ class 
> QAPISchemaObjectTypeUnionTag(QAPISchemaObjectTypeMember):
>  assert self.type.is_implicit()
>  return 'kind'
>
> +def describe(self):
> +# Must override superclass describe() because 

Re: [Qemu-devel] [PATCH] hw/arm/virt: Allow zero address for PCI IO space

[Peter Maydell]

On 13 October 2015 at 14:04, Michael S. Tsirkin  wrote:
> On Tue, Oct 13, 2015 at 02:47:33PM +0200, Laurent Vivier wrote:
>> MST asked for a global flag:
>>
>> https://lists.gnu.org/archive/html/qemu-ppc/2015-07/msg00364.html
>>
>> But perhaps the machine is not the good place for this global flag ?
>>
>> CC: Michael for that.
>
> Whether controllers treat zero specially should be expressed
> using priorities in memory APIs.

Mmm, I guess it's a bug in how the machine model
wires up the controller rather than in the controller
model itself.

> This is just about buggy machine types that do
> not specify priority for overlapping regions correctly.
>
> As a temporary hacky work-around, a global seems sufficient.

Well, if we're going to go around and fix the machines which
don't get things right, I guess. It's a shame the default for
the global is "this machine is broken", because now every
new machine will default unnecessarily to broken, and there's
no way to grep the source tree for machines which need fixing.

-- PMM

Re: [Qemu-devel] [PATCH v4 7/7] qom: allow properties to be registered against classes

[Pavel Fedin]

 Hello!

> -Original Message-
> From: qemu-devel-bounces+p.fedin=samsung@nongnu.org [mailto:qemu-devel-
> bounces+p.fedin=samsung@nongnu.org] On Behalf Of Daniel P. Berrange
> Sent: Tuesday, October 13, 2015 3:38 PM
> To: qemu-devel@nongnu.org
> Cc: Pavel Fedin; Markus Armbruster; Paolo Bonzini; Andreas Färber
> Subject: [Qemu-devel] [PATCH v4 7/7] qom: allow properties to be registered 
> against classes
> 
> When there are many instances of a given class, registering
> properties against the instance is wasteful of resources. The
> majority of objects have a statically defined list of possible
> properties, so most of the properties are easily registerable
> against the class. Only those properties which are conditionally
> registered at runtime need be recorded against the klass.
> 
> Registering properties against classes also makes it possible
> to provide static introspection of QOM - currently introspection
> is only possible after creating an instance of a class, which
> severely limits its usefulness.
> 
> This impl only supports simple scalar properties. It does not
> attempt to allow child object / link object properties against
> the class. There are ways to support those too, but it would
> make this patch more complicated, so it is left as an exercise
> for the future.
> 
> There is no equivalent to object_property_del provided, since
> classes must be immutable once they are defined.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  include/qom/object.h   |  50 ++
>  qom/object.c   | 237 
> ++---
>  tests/check-qom-proplist.c |  31 --
>  3 files changed, 293 insertions(+), 25 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 2a54515..38f41d3 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -381,6 +381,8 @@ struct ObjectClass
>  const char *class_cast_cache[OBJECT_CLASS_CAST_CACHE];
> 
>  ObjectUnparent *unparent;
> +
> +GHashTable *properties;
>  };
> 
>  /**
> @@ -947,6 +949,13 @@ ObjectProperty *object_property_add(Object *obj, const 
> char *name,
> 
>  void object_property_del(Object *obj, const char *name, Error **errp);
> 
> +ObjectProperty *object_class_property_add(ObjectClass *klass, const char 
> *name,
> +  const char *type,
> +  ObjectPropertyAccessor *get,
> +  ObjectPropertyAccessor *set,
> +  ObjectPropertyRelease *release,
> +  void *opaque, Error **errp);
> +
>  /**
>   * object_property_find:
>   * @obj: the object
> @@ -957,6 +966,8 @@ void object_property_del(Object *obj, const char *name, 
> Error **errp);
>   */
>  ObjectProperty *object_property_find(Object *obj, const char *name,
>   Error **errp);
> +ObjectProperty *object_class_property_find(ObjectClass *klass, const char 
> *name,
> +   Error **errp);
> 
>  typedef struct ObjectPropertyIterator ObjectPropertyIterator;
> 
> @@ -964,8 +975,14 @@ typedef struct ObjectPropertyIterator 
> ObjectPropertyIterator;
>   * object_property_iter_init:
>   * @obj: the object
>   *
> +<<< HEAD
>   * Initializes an iterator for traversing all properties
>   * registered against an object instance.
> +===
> + * Iterates over all properties defined against the object
> + * instance, its class and all parent classes, calling
> + * @iter for each property.
> +>>> 0ca9307... qom: allow properties to be registered against classes

 Conflict markers slipped in the comment

>   *
>   * It is forbidden to modify the property list while iterating,
>   * whether removing or adding properties.
> @@ -1375,6 +1392,12 @@ void object_property_add_str(Object *obj, const char 
> *name,
>   void (*set)(Object *, const char *, Error **),
>   Error **errp);
> 
> +void object_class_property_add_str(ObjectClass *klass, const char *name,
> +   char *(*get)(Object *, Error **),
> +   void (*set)(Object *, const char *,
> +   Error **),
> +   Error **errp);
> +
>  /**
>   * object_property_add_bool:
>   * @obj: the object to add a property to
> @@ -1391,6 +1414,11 @@ void object_property_add_bool(Object *obj, const char 
> *name,
>void (*set)(Object *, bool, Error **),
>Error **errp);
> 
> +void object_class_property_add_bool(ObjectClass *klass, const char *name,
> +bool (*get)(Object *, Error **),
> +void (*set)(Object *, bool, Error **),
> +

Re: [Qemu-devel] [PATCH 1/1] log hmp/qmp command

[Denis V. Lunev]

On 10/13/2015 04:05 PM, Luiz Capitulino wrote:

On Mon, 12 Oct 2015 11:41:35 +0300
"Denis V. Lunev"  wrote:


From: Pavel Butsykin 

This log would be very welcome for long-term diagnostics of the system
in the production. This log is at least necessary to understand what
has been happened on the system and to identify issues at higher-level
subsystems (libvirt, etc).

This is good to have, and probably we should have had it since day one.
But I have the following considerations:

  - libvirt would need support for this to use it the way you describe,
although libvirt already has its logging facility (well, one could
wrap qemu in bash and pass this by default but this is not ideal)

OK, I'll add task to the queue for our libvirt guys


  - A true logging facility would also log responses and events

responses are done alreade internally in the scope of preparations
to the next submission, events are missed. Thank you for pointing
this out


  - I don't think it's important to log HMP


OK, but I do not that that this will hurt us :)

Den


Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Markus Armbruster 
CC: Luiz Capitulino 
CC: Eric Blake 
---
  include/qemu/log.h | 1 +
  monitor.c  | 4 
  qemu-log.c | 1 +
  3 files changed, 6 insertions(+)

diff --git a/include/qemu/log.h b/include/qemu/log.h
index f880e66..dfb587e 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -41,6 +41,7 @@ static inline bool qemu_log_enabled(void)
  #define LOG_UNIMP  (1 << 10)
  #define LOG_GUEST_ERROR(1 << 11)
  #define CPU_LOG_MMU(1 << 12)
+#define LOG_CMD(1 << 13)
  
  /* Returns true if a bit is set in the current loglevel mask

   */
diff --git a/monitor.c b/monitor.c
index 4f1ba2f..a22a798 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2848,6 +2848,8 @@ static void handle_hmp_command(Monitor *mon, const char 
*cmdline)
  QDict *qdict;
  const mon_cmd_t *cmd;
  
+qemu_log_mask(LOG_CMD, "hmp \"%s\" requested\n", cmdline);

+
  cmd = monitor_parse_command(mon, , mon->cmd_table);
  if (!cmd) {
  return;
@@ -3822,6 +3824,8 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
  error_setg(_err, QERR_JSON_PARSING);
  goto err_out;
  }
+qemu_log_mask(LOG_CMD, "qmp \"%s\" requested\n",
+  qobject_to_json(obj)->string);
  
  input = qmp_check_input_obj(obj, _err);

  if (!input) {
diff --git a/qemu-log.c b/qemu-log.c
index 13f3813..66b15ec 100644
--- a/qemu-log.c
+++ b/qemu-log.c
@@ -119,6 +119,7 @@ const QEMULogItem qemu_log_items[] = {
  { LOG_GUEST_ERROR, "guest_errors",
"log when the guest OS does something invalid (eg accessing a\n"
"non-existent register)" },
+{ LOG_CMD, "cmd", "log the hmp/qmp commands execution" },
  { 0, NULL, NULL },
  };
  

Re: [Qemu-devel] [PATCH] hw/arm/virt: Allow zero address for PCI IO space

[Peter Maydell]

On 13 October 2015 at 14:19, Michael S. Tsirkin  wrote:
> On Tue, Oct 13, 2015 at 02:12:34PM +0100, Peter Maydell wrote:
>> Well, if we're going to go around and fix the machines which
>> don't get things right, I guess. It's a shame the default for
>> the global is "this machine is broken", because now every
>> new machine will default unnecessarily to broken, and there's
>> no way to grep the source tree for machines which need fixing.

> It'd be easy enough to revert the logic if someone's willing to start on
> this.  I'm reluctant to make this patchset depend on changing all
> machines, but if you think I'm wrong, pls let me know.

Most machines don't have a PCI controller, luckily.
I'll have a look at how many files it would touch...

-- PMM

Re: [Qemu-devel] [PATCH v3 04/32] acpi: add aml_mutex, aml_acquire, aml_release

[Igor Mammedov]

On Sun, 11 Oct 2015 11:52:36 +0800
Xiao Guangrong  wrote:

> Implement Mutex, Acquire and Release terms which are used by NVDIMM _DSM 
> method
> in later patch
> 
> Signed-off-by: Xiao Guangrong 
> ---
>  hw/acpi/aml-build.c | 32 
>  include/hw/acpi/aml-build.h |  3 +++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 9fe5e7b..ab52692 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1164,6 +1164,38 @@ Aml *aml_create_field(Aml *srcbuf, Aml *index, Aml 
> *len, const char *name)
>  return var;
>  }
>  
> +/* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefMutex */
> +Aml *aml_mutex(const char *name, uint8_t flags)
s/flags/sync_level/

> +{
> +Aml *var = aml_alloc();
> +build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
> +build_append_byte(var->buf, 0x01); /* MutexOp */
> +build_append_namestring(var->buf, "%s", name);

add assert here to check that reserved bits are 0
> +build_append_byte(var->buf, flags);
> +return var;
> +}
> +
> +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefAcquire */
> +Aml *aml_acquire(Aml *mutex, uint16_t timeout)
> +{
> +Aml *var = aml_alloc();
> +build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
> +build_append_byte(var->buf, 0x23); /* AcquireOp */
> +aml_append(var, mutex);
> +build_append_int_noprefix(var->buf, timeout, sizeof(timeout));
> +return var;
> +}
> +
> +/* ACPI 1.0b: 16.2.5.3 Type 1 Opcodes Encoding: DefRelease */
> +Aml *aml_release(Aml *mutex)
> +{
> +Aml *var = aml_alloc();
> +build_append_byte(var->buf, 0x5B); /* ExtOpPrefix */
> +build_append_byte(var->buf, 0x27); /* ReleaseOp */
> +aml_append(var, mutex);
> +return var;
> +}
> +
>  void
>  build_header(GArray *linker, GArray *table_data,
>   AcpiTableHeader *h, const char *sig, int len, uint8_t rev)
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 7e1c43b..d494c0c 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -277,6 +277,9 @@ Aml *aml_unicode(const char *str);
>  Aml *aml_derefof(Aml *arg);
>  Aml *aml_sizeof(Aml *arg);
>  Aml *aml_create_field(Aml *srcbuf, Aml *index, Aml *len, const char *name);
> +Aml *aml_mutex(const char *name, uint8_t flags);
> +Aml *aml_acquire(Aml *mutex, uint16_t timeout);
> +Aml *aml_release(Aml *mutex);
>  
>  void
>  build_header(GArray *linker, GArray *table_data,

Re: [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection

[Denis V. Lunev]

On 10/13/2015 07:05 AM, Michael Roth wrote:

Quoting Denis V. Lunev (2015-10-07 05:32:21)

From: Yuri Pudgorodskiy 

Implemented with base64-encoded strings in qga json protocol.
Glib portable GIOChannel is used for data I/O.

Optinal stdin parameter of guest-exec command is now used as
stdin content for spawned subprocess.

If capture-output bool flag is specified, guest-exec redirects out/err
file descriptiors internally to pipes and collects subprocess
output.

Guest-exe-status is modified to return this collected data to requestor
in base64 encoding.

Signed-off-by: Yuri Pudgorodskiy 
Signed-off-by: Denis V. Lunev 
CC: Michael Roth 

For capture-output mode, win32 guests appear to spin forever on EOF and
the exec'd process never gets reaped as a result. The below patch
seems to fix this issue. If this seems reasonable I can squash it
into the patch directly, but you might want to check I didn't break one
of your !capture-output use-cases (I assume this is still mainly focused
on redirecting to virtio-serial for stdio).

Another issue I noticed is that glib relies on
gspawn-win{32,64}-helper[-console].exe for g_spawn*() interface, so guest
exec won't work for .msi distributables unless those are added. This can
probably be addressed during soft-freeze though.

diff --git a/qga/commands.c b/qga/commands.c
index 27c06c5..fbf8abd 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -318,7 +318,7 @@ static gboolean guest_exec_output_watch(GIOChannel *ch,
  struct GuestExecIOData *p = (struct GuestExecIOData *)p_;
  gsize bytes_read = 0;
  
-if (cond == G_IO_HUP) {

+if (cond == G_IO_HUP || cond == G_IO_ERR) {
  g_io_channel_unref(ch);
  g_atomic_int_set(>closed, 1);
  return FALSE;
@@ -330,10 +330,18 @@ static gboolean guest_exec_output_watch(GIOChannel *ch,
  t = g_try_realloc(p->data, p->size + GUEST_EXEC_IO_SIZE);
  }
  if (t == NULL) {
+GIOStatus gstatus = 0;
  p->truncated = true;
  /* ignore truncated output */
  gchar buf[GUEST_EXEC_IO_SIZE];
-g_io_channel_read_chars(ch, buf, sizeof(buf), _read, NULL);
+gstatus = g_io_channel_read_chars(ch, buf, sizeof(buf),
+  _read, NULL);
+if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) {
+g_io_channel_unref(ch);
+g_atomic_int_set(>closed, 1);
+return false;
+}
+
  return TRUE;
  }
  p->size += GUEST_EXEC_IO_SIZE;
@@ -342,8 +350,14 @@ static gboolean guest_exec_output_watch(GIOChannel *ch,
  
  /* Calling read API once.

   * On next available data our callback will be called again */
-g_io_channel_read_chars(ch, (gchar *)p->data + p->length,
+GIOStatus gstatus = 0;
+gstatus = g_io_channel_read_chars(ch, (gchar *)p->data + p->length,
  p->size - p->length, _read, NULL);
+if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) {
+g_io_channel_unref(ch);
+g_atomic_int_set(>closed, 1);
+return false;
+}


not completely. we have tested your code and found that minor
change is required

Signed-off-by: Yuri Pudgorodskiy
---
 qga/commands.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/commands.c b/qga/commands.c
index 1811ce6..deb041e 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -413,7 +413,7 @@ GuestExec *qmp_guest_exec(const char *path,
 if (has_inp_data) {
 gei->in.data = g_base64_decode(inp_data, >in.size);
 #ifdef G_OS_WIN32
-in_ch = g_io_channel_win32_new_fd(in_ch);
+in_ch = g_io_channel_win32_new_fd(in_fd);
 #else
 in_ch = g_io_channel_unix_new(in_fd);
 #endif

I'll send a patch with this change and minor cosmetic
improvements soon (to make code shorter), but you can
take your version with this fix applied at your taste.

Den

Re: [Qemu-devel] [PATCH v8 11/18] qapi: Simplify gen_struct_field()

[Eric Blake]

On 10/13/2015 06:12 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> Rather than having all callers pass a name, type, and optional
>> flag, have them instead pass a QAPISchemaObjectTypeMember which
>> already has all that information.
>>
>> This will allow a future patch to simplify alternates to use
>> a special tag 'qtype_code type'.  In the meantime, it requires
>> a hack to create a temporary member 'base' for struct base
>> classes; this temporary member will go away in a later patch
>> that flattens structs in the same way that flat union base
>> classes were flattened in commit 1e6c1616.
>>
>> No change to generated code.
>>
>> Signed-off-by: Eric Blake 
>>

>>
>>  if base:
>> -ret += gen_struct_field('base', base, False)
>> +# TODO Flatten this struct, similar to flat union bases. Until
>> +# then, hack around it with a temporary member.
>> +member = QAPISchemaObjectTypeMember('base', base.name, False)
>> +member.type = base
>> +ret += gen_struct_field(member)
>>
>>  ret += gen_struct_fields(members)
> 
> Uff!  Are you really, really sure this is the right spot in the series
> for this transformation?

Serves me right for sending the series late at night. I can probably
delay this transformation to a later subset, after I have unboxed struct
base members, so I won't need this hack.  And I just confirmed that
nothing else in subset B depends on this patch, so let's defer it to
subset C or later (dropping from this series is not sufficient reason
for a v9 respin, but if something else turns up that needs a v9, this
patch has been moved out).

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] util/qemu-config: fix missing machine command line options

[Marcel Apfelbaum]

On 10/13/2015 02:51 PM, Cornelia Huck wrote:

On Mon, 12 Oct 2015 18:42:04 +0300
Marcel Apfelbaum  wrote:


On 10/12/2015 06:36 PM, Tony Krowiak wrote:

Commit 0a7cf217 ("util/qemu-config: fix regression of
qmp_query_command_line_options") aimed to restore parsing of global
machine options, but missed two: "aes-key-wrap" and
"dea-key-wrap" (which were present in the initial version of that
patch). Let's add them to the machine_opts again.


Adding qemu-devel

Hi,

I thought I hunted them all, anyway, thanks for adding them.

Reviewed-by: Marcel Apfelbaum 


Thanks.

util/qemu-config.c yields -ENOMAINTAINER; any obvious tree for this to
go through, or should I simply include this with the next batch of
s390x patches (as these are s390x options)?



Just take it to your tree.
Thanks!
Marcel






Fixes: 0a7cf217
CC: Marcel Apfelbaum 
CC: qemu-sta...@nongnu.org
Signed-off-by: Tony Krowiak 
---
   util/qemu-config.c |8 
   1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/util/qemu-config.c b/util/qemu-config.c
index 5fcfd0e..687fd34 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -219,6 +219,14 @@ static QemuOptsList machine_opts = {
   .name = "suppress-vmdesc",
   .type = QEMU_OPT_BOOL,
   .help = "Set on to disable self-describing migration",
+},{
+.name = "aes-key-wrap",
+.type = QEMU_OPT_BOOL,
+.help = "enable/disable AES key wrapping using the CPACF wrapping 
key",
+},{
+.name = "dea-key-wrap",
+.type = QEMU_OPT_BOOL,
+.help = "enable/disable DEA key wrapping using the CPACF wrapping 
key",
   },
   { /* End of list */ }
   }

Re: [Qemu-devel] [PATCH 0/3] Make KVM/MSI code device-ID-aware

[Pavel Fedin]

 Hello!

> I'm not at all sure we want to keep extending pci-assign with
> more functionality. Why not add it to vfio instead?

 pci-assign? What exactly do you mean?

 hw/i386/kvm/pci-assign.c is modified only because kvm_irqchip_add_msi_route() 
now wants pci_dev pointer. Since x86 platform doesn't
use device IDs, it could very well be NULL. Just i decided to keep it 
consistent with the rest of the code.
 Actually, patch 0003 is all about that - we add pci_dev pointer to KVM GSI 
routing functions and make callers passing it, that's
all. Currently it's not used because KVM API is not released yet. See my full 
vITS patchset for information on how it will be used.
And yes, the patchset is a bit obsolete, so it doesn't have msi_device_id() 
inline, there's copypasted calculation instead.
 What exactly do you suggest to move to vfio?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

[Qemu-devel] [PATCH 1/3] block: Add blk_get_refcnt()

[Alberto Garcia]

This function returns the reference count of a given BlockBackend.
For convenience, it returns 0 if the BlockBackend pointer is NULL.

Signed-off-by: Alberto Garcia 
---
 block/block-backend.c  | 5 +
 include/sysemu/block-backend.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 10e4d71..5f1e395 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -189,6 +189,11 @@ static void drive_info_del(DriveInfo *dinfo)
 g_free(dinfo);
 }
 
+int blk_get_refcnt(BlockBackend *blk)
+{
+return blk ? blk->refcnt : 0;
+}
+
 /*
  * Increment @blk's reference count.
  * @blk must not be null.
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 14a6d32..8a6413d 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -65,6 +65,7 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp);
 BlockBackend *blk_new_open(const char *name, const char *filename,
const char *reference, QDict *options, int flags,
Error **errp);
+int blk_get_refcnt(BlockBackend *blk);
 void blk_ref(BlockBackend *blk);
 void blk_unref(BlockBackend *blk);
 const char *blk_name(BlockBackend *blk);
-- 
2.6.1

[Qemu-devel] [PATCH 3/3] iotests: Add test for the blockdev-del command

[Alberto Garcia]

Signed-off-by: Alberto Garcia 
---
 tests/qemu-iotests/139 | 189 +
 tests/qemu-iotests/139.out |   5 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 195 insertions(+)
 create mode 100644 tests/qemu-iotests/139
 create mode 100644 tests/qemu-iotests/139.out

diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
new file mode 100644
index 000..325ac7a
--- /dev/null
+++ b/tests/qemu-iotests/139
@@ -0,0 +1,189 @@
+#!/usr/bin/env python
+#
+# Test cases for the QMP 'blockdev-del' command
+#
+# Copyright (C) 2015 Igalia, S.L.
+# Author: Alberto Garcia 
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import iotests
+import time
+
+base_img = os.path.join(iotests.test_dir, 'base.img')
+new_img = os.path.join(iotests.test_dir, 'new.img')
+
+class TestBlockdevDel(iotests.QMPTestCase):
+
+def setUp(self):
+iotests.qemu_img('create', '-f', iotests.imgfmt, base_img, '1M')
+self.vm = iotests.VM()
+self.vm.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(base_img)
+if os.path.isfile(new_img):
+os.remove(new_img)
+
+def addBlockDev(self, block_backend = True):
+opts = {'node-name': 'node0',
+'driver': iotests.imgfmt,
+'file': {'filename': base_img,
+ 'driver': 'file'}}
+if block_backend:
+opts['id'] = 'drive0'
+result = self.vm.qmp('blockdev-add', conv_keys = False, options = opts)
+self.assert_qmp(result, 'return', {})
+
+def delBlockDev(self, use_node_name = False, expect_error = False):
+if use_node_name:
+result = self.vm.qmp('blockdev-del', device = 'node0')
+else:
+result = self.vm.qmp('blockdev-del', device = 'drive0')
+if expect_error:
+self.assert_qmp(result, 'error/class', 'GenericError')
+else:
+self.assert_qmp(result, 'return', {})
+
+def addDeviceModel(self):
+result = self.vm.qmp('device_add', id = 'device0',
+ driver = 'virtio-blk-pci', drive = 'drive0')
+self.assert_qmp(result, 'return', {})
+
+def delDeviceModel(self):
+result = self.vm.qmp('device_del', id = 'device0')
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('system_reset')
+self.assert_qmp(result, 'return', {})
+
+device_path = '/machine/peripheral/device0/virtio-backend'
+event = self.vm.event_wait(name="DEVICE_DELETED",
+   match={'data': {'path': device_path}})
+self.assertNotEqual(event, None)
+
+event = self.vm.event_wait(name="DEVICE_DELETED",
+   match={'data': {'device': 'device0'}})
+self.assertNotEqual(event, None)
+
+def ejectDrive(self, expect_error = False):
+result = self.vm.qmp('eject', device = 'drive0')
+if expect_error:
+self.assert_qmp(result, 'error/class', 'GenericError')
+else:
+self.assert_qmp(result, 'return', {})
+
+def createSnapshot(self):
+opts = {'node-name': 'node0',
+'snapshot-file': new_img,
+'snapshot-node-name': 'overlay0',
+'format': iotests.imgfmt}
+result = self.vm.qmp('blockdev-snapshot-sync', conv_keys=False, **opts)
+self.assert_qmp(result, 'return', {})
+
+def createMirror(self):
+opts = {'device': 'drive0',
+'target': new_img,
+'sync': 'top',
+'format': iotests.imgfmt}
+result = self.vm.qmp('drive-mirror', conv_keys=False, **opts)
+self.assert_qmp(result, 'return', {})
+
+def completeBlockJob(self):
+result = self.vm.qmp('block-job-complete', device='drive0')
+self.assert_qmp(result, 'return', {})
+
+def addQuorumDrive(self):
+iotests.qemu_img('create', '-f', iotests.imgfmt, new_img, '1M')
+drive_0 = {'driver': iotests.imgfmt,
+   'node-name': 'node0',
+   'file': {'driver': 'file',
+'filename': base_img}}
+drive_1 = {'driver': iotests.imgfmt,
+   'node-name': 'node1',
+   'file': {'driver': 

Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check()

[Eric Blake]

On 10/13/2015 01:08 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> On 10/12/2015 09:53 AM, Markus Armbruster wrote:
> [...]
>>> It would be simpler if we could make C clash only when QMP clashes.
>>
>> If a QMP clash always caused a C clash, life would be a bit simpler. We
>> aren't going to get there (because in a flat union, hiding the members
>> of the branch type behind a single pointer means those members don't
>> clash with any C names of the overall union), but I can certainly try to
>> make the comments explain what is going on.
> 
> (1) If QMP clashed exactly when C clashed, we'd have to think only about
> one of them, and the C compiler would check for clashes statically.

Although deferring the error about the clash until compile time is a bit
long, compared to reporting the clash at generator time.

> 
> (2) If a QMP clash implied a C clash, we'd only have to think about C,
> and the C compiler would check statically.
> 
> (3) If a C clash implied a QMP clash, we'd only have to think about QMP.
> 
> (4) If they can clash independently, we need to think about both
> independently.
> 
> We currently have (4), and having to juggle both QMP and C in my mind
> has been taxing.
> 
> My remark was about (3): C clashes only when QMP clashes <=> C clash
> implies QMP clash.
> 
> Your remark was about (2).  More useful than (3), but as you say not
> feasible.
> 
> Do you think we can get to (3)?

C clash that does not imply a QMP clash:

{ 'union':'U', 'data':{ 'a':'int', 'A':'str' } }

C clash (the implicit enum has two U_KIND_A values) but not a QMP clash
('a' and 'A' don't even appear in QMP; and even if they did, they are
distinct in C as branch names).  Might be possible to avoid the C clash
by munging case labels of the generated enum differently, but I'm not
sure it is worth the effort.

QMP clash that does not (currently) imply a C clash (using abbreviated
notation):

{ 'union':'U', 'base':{ 'type':'Enum', 'member':'int' },
  'discriminator':'type',
  'data':{ 'branch':{ 'member':'str' } } }

QMP clash (the field 'member' is part of the base type, and also part of
the 'branch' variant type), but not a C clash (because the C layout
accesses the branch through a box named 'branch').

But we cannot just remove the boxing, either, because then we'd have a
clash in C that is NOT a clash in QMP:

{ 'union':'U', 'base':{ 'type':'Enum' }, 'discriminator':'type',
'data':{ 'branch1':{ 'member':'str' }, 'branch2': { 'member':'int' } } }

If the two 'member' fields were at the same C level as 'type', then we
could use C collisions to detect a QMP clash between a variant's members
and the non-variant fields - but it also has the drawback of declaring a
C collision between the two branches.

So no, I don't think we can get to (3).

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] hw/arm/virt: Allow zero address for PCI IO space

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 02:12:34PM +0100, Peter Maydell wrote:
> On 13 October 2015 at 14:04, Michael S. Tsirkin  wrote:
> > On Tue, Oct 13, 2015 at 02:47:33PM +0200, Laurent Vivier wrote:
> >> MST asked for a global flag:
> >>
> >> https://lists.gnu.org/archive/html/qemu-ppc/2015-07/msg00364.html
> >>
> >> But perhaps the machine is not the good place for this global flag ?
> >>
> >> CC: Michael for that.
> >
> > Whether controllers treat zero specially should be expressed
> > using priorities in memory APIs.
> 
> Mmm, I guess it's a bug in how the machine model
> wires up the controller rather than in the controller
> model itself.
> 
> > This is just about buggy machine types that do
> > not specify priority for overlapping regions correctly.
> >
> > As a temporary hacky work-around, a global seems sufficient.
> 
> Well, if we're going to go around and fix the machines which
> don't get things right, I guess. It's a shame the default for
> the global is "this machine is broken", because now every
> new machine will default unnecessarily to broken, and there's
> no way to grep the source tree for machines which need fixing.
> 
> -- PMM

It'd be easy enough to revert the logic if someone's willing to start on
this.  I'm reluctant to make this patchset depend on changing all
machines, but if you think I'm wrong, pls let me know.

-- 
MST

Re: [Qemu-devel] [PATCH 2/3] hw/pci: Introduce msi_device_id()

[Pavel Fedin]

 Hello!

> What make this specific format an msi id?
> I think this is some kvm specific hack.

 It is actually architecture-specific. But, we don't have any other arches 
using it (and probably won't), so at least for now i
decided to put it there in order to keep things simpler. If this ever pops up 
on any other arch, we'll be able to easily move it to
target-XXX.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

Re: [Qemu-devel] [PULL 04/26] target-*: Introduce and use cpu_breakpoint_test

[Sergey Fedorov]

On 13.10.2015 03:13, Richard Henderson wrote:
> On 10/10/2015 12:34 AM, Sergey Fedorov wrote:
>>> @@ -2936,6 +2927,10 @@ static inline void
>>> gen_intermediate_code_internal(AlphaCPU *cpu,
>>>   tcg_gen_insn_start(ctx.pc);
>>>   num_insns++;
>>>
>>> +if (unlikely(cpu_breakpoint_test(cs, ctx.pc, BP_ANY))) {
>>> +gen_excp(, EXCP_DEBUG, 0);
>>> +break;
>>> +}
>>
>> Actually, control logic has changed here. The old code used a break
>> statement to exit from QTAILQ_FOREACH loop and continue with instruction
>> translation thus translating at least one instruction. The break
>> statement in the new code makes exit from the translation loop itself,
>> effectively producing zero-length TB which won't get invalidated when
>> clearing the breakpoint. Seems like we should remove the break statement
>> here and in similar cases below, right?
>
> Why do you believe that a zero-length TB won't be cleared?
> The TB still has a start address, which is contained within
> a given page, which is invalidated.
>
> Some target-*/translate.c takes care to advance the PC, but I believe
> that this is only required when the breakpoint instruction *itself*
> could span a page boundary.  I.e. the TB needs to be marked to span
> two pages.  This situation can never be true for many RISC targets.
>
> We did discuss this exact situation during review of the patch set,
> though it's probably true that there are outstanding errors in some
> translators.

I see your point, but what I am actually concerned about is the
following scenario.

Lets suppose we are going to remove a breakpoint. Eventually we can get
the following function call stack:

#0  tb_invalidate_phys_page_range()
#1  tb_invalidate_phys_addr()
#2  breakpoint_invalidate()
#3  cpu_breakpoint_remove_by_ref()
...

Then, if we come across a zero-sized TB then 'tb_start' would be equal
to 'tb_end'. That is not a big deal unless 'start' is not equal to
'tb_start'. Otherwise, "!(tb_end <= start || tb_start >= end)" condition
check will fail and that TB won't get invalidated as it should be. As I
can see this is a case when we try to remove a breakpoint which is
happened to be set at the very first instruction of a TB. So we either
need to change the condition in tb_invalidate_phys_page_range() or do
the PC advancement trick during translation, no matter can instructions
cross a page boundary or not.

Best regards,
Sergey

[Qemu-devel] [PATCH 2/3] block: Add 'blockdev-del' QMP command

[Alberto Garcia]

This is the companion to 'blockdev-add'. It allows deleting a
BlockBackend with its associated BlockDriverState tree, or a
BlockDriverState that is not attached to any backend.

In either case, the command fails if the reference count is greater
than 1 or the BlockDriverState has any parents.

Signed-off-by: Alberto Garcia 
---
 blockdev.c   | 42 ++
 qapi/block-core.json | 21 +
 qmp-commands.hx  | 36 
 3 files changed, 99 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 2360c1f..c448a0b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3402,6 +3402,48 @@ fail:
 qmp_output_visitor_cleanup(ov);
 }
 
+void qmp_blockdev_del(const char *device, Error **errp)
+{
+AioContext *aio_context;
+BlockBackend *blk;
+BlockDriverState *bs;
+
+blk = blk_by_name(device);
+if (blk) {
+bs = blk_bs(blk);
+aio_context = blk_get_aio_context(blk);
+} else {
+bs = bdrv_find_node(device);
+if (!bs) {
+error_setg(errp, "Cannot find block device %s", device);
+return;
+}
+blk = bs->blk;
+aio_context = bdrv_get_aio_context(bs);
+}
+
+aio_context_acquire(aio_context);
+
+if (bs && bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, errp)) {
+goto out;
+}
+
+if (blk_get_refcnt(blk) > 1 ||
+(bs && (bs->refcnt > 1 || !QLIST_EMPTY(>parents {
+error_setg(errp, "Block device %s is being used", device);
+goto out;
+}
+
+if (blk) {
+blk_unref(blk);
+} else {
+bdrv_unref(bs);
+}
+
+out:
+aio_context_release(aio_context);
+}
+
 BlockJobInfoList *qmp_query_block_jobs(Error **errp)
 {
 BlockJobInfoList *head = NULL, **p_next = 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5f12af7..20897eb 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1877,6 +1877,27 @@
 { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
 
 ##
+# @blockdev-del:
+#
+# Deletes a block device. The selected device can be either a block
+# backend or a graph node.
+#
+# In the former case the backend will be destroyed, along with its
+# inserted medium if there's any.
+#
+# In the latter case the node will be destroyed, along with the
+# backend it is attached to if there's any.
+#
+# The command will fail if the selected block device is still being
+# used.
+#
+# @device: Name of the block backend or the graph node to be deleted.
+#
+# Since: 2.5
+##
+{ 'command': 'blockdev-del', 'data': { 'device': 'str' } }
+
+##
 # @blockdev-open-tray:
 #
 # Opens a block device's tray. If there is a block driver state tree inserted 
as
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4f03d11..b8b133e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3921,6 +3921,42 @@ Example (2):
 EQMP
 
 {
+.name   = "blockdev-del",
+.args_type  = "device:s",
+.mhandler.cmd_new = qmp_marshal_blockdev_del,
+},
+
+SQMP
+blockdev-del
+
+Since 2.5
+
+Deletes a block device. The selected device can be either a block
+backend or a graph node.
+
+In the former case the backend will be destroyed, along with its
+inserted medium if there's any.
+
+In the latter case the node will be destroyed, along with the
+backend it is attached to if there's any.
+
+The command will fail if the selected block device is still being
+used.
+
+Arguments:
+
+- "device": Identifier of the block device or graph node name (json-string)
+
+Example:
+
+-> { "execute": "blockdev-del",
+"arguments": { "device": "virtio0" }
+   }
+<- { "return": {} }
+
+EQMP
+
+{
 .name   = "blockdev-open-tray",
 .args_type  = "device:s,force:b?",
 .mhandler.cmd_new = qmp_marshal_blockdev_open_tray,
-- 
2.6.1

[Qemu-devel] [PATCH 0/3] Add 'blockdev-del' command

[Alberto Garcia]

Here's my first attempt at the 'blockdev-del' command.

This series goes on top of Max's "BlockBackend and media" v6:

https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02810.html

With Max's code, 'blockdev-add' can now create a BlockDriverState with
or without a BlockBackend (depending on whether the 'id' parameter is
passed).

Therefore, 'blockdev-del' can be used to delete a backend and/or a
BDS.

The way it works is simple: it receives a single 'device' parameter
which can refer to a block backend or a node name.

 - If it's a block backend, it will delete it along with its attached
   BDS (if any).

 - If it's a node name, it will delete the BDS along with the backend
   it is attached to (if any).

 - If you're wondering whether it's possible to delete the BDS but not
   the backend it is attached to, there is already eject or
   blockdev-remove-medium for that.

If either the backend or the BDS are being used (refcount > 1, or if
the BDS has any parents) the command fails.

The series includes bunch of test cases with different scenarios
(nodes with and without backend, empty backend, backing images, block
jobs, Quorum).

Regards,

Berto

Alberto Garcia (3):
  block: Add blk_get_refcnt()
  block: Add 'blockdev-del' QMP command
  iotests: Add test for the blockdev-del command

 block/block-backend.c  |   5 ++
 blockdev.c |  42 +
 include/sysemu/block-backend.h |   1 +
 qapi/block-core.json   |  21 +
 qmp-commands.hx|  36 
 tests/qemu-iotests/139 | 189 +
 tests/qemu-iotests/139.out |   5 ++
 tests/qemu-iotests/group   |   1 +
 8 files changed, 300 insertions(+)
 create mode 100644 tests/qemu-iotests/139
 create mode 100644 tests/qemu-iotests/139.out

-- 
2.6.1

Re: [Qemu-devel] [PATCH] hw/arm/virt: Allow zero address for PCI IO space

[Peter Maydell]

On 13 October 2015 at 14:25, Peter Maydell  wrote:
> On 13 October 2015 at 14:19, Michael S. Tsirkin  wrote:
>> On Tue, Oct 13, 2015 at 02:12:34PM +0100, Peter Maydell wrote:
>>> Well, if we're going to go around and fix the machines which
>>> don't get things right, I guess. It's a shame the default for
>>> the global is "this machine is broken", because now every
>>> new machine will default unnecessarily to broken, and there's
>>> no way to grep the source tree for machines which need fixing.
>
>> It'd be easy enough to revert the logic if someone's willing to start on
>> this.  I'm reluctant to make this patchset depend on changing all
>> machines, but if you think I'm wrong, pls let me know.
>
> Most machines don't have a PCI controller, luckily.
> I'll have a look at how many files it would touch...

So, first up I'm happy that the gpex and versatile
pci controllers don't have this problem (the way they
set up the mmio and io windows means there won't be
overlaps). That leaves the following pci controllers,
which I've listed with the source files which define machines
that use them:

hw/pci-host/pbm
 hw/sparc64/sun4u.c
hw/pci-host/bonito
 hw/mips/mips_fulong2e.c
hw/pci-host/grackle
 hw/pc/mac_oldworld.c
hw/pci-host/piix
 [the x86 systems]
hw/pci-host/ppce500
 hw/ppc/e500.c
hw/pci-host/prep
 hw/ppc/prep.c
hw/pci-host/q35
 [x86 systems]
hw/pci-host/uninorth
 hw/ppc/mac_newworld.c
hw/alpha/typhoon
 used in hw/alpha/dp264.c
hw/mips/gt64xxx_pci
 hw/mips/mips_malta.c
hw/ppc/ppc4xx_pci
 hw/ppc/ppc440_bamboo.c
hw/ppc/spapr_pci
 hw/ppc/spapr.c [already marked as '0 addrs ok']
hw/s390/s390-pci-bus
 hw/s390/s390-virtio-ccw.c
hw/sh4/sh_pci
 hw/sh4/rd2.c

So we'd need to touch perhaps fifteen files in total.

I don't insist we do that rather than applying this particular
patch, but I don't think it's a huge effort.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] Make KVM/MSI code device-ID-aware

[Michael S. Tsirkin]

On Tue, Oct 13, 2015 at 04:49:17PM +0300, Pavel Fedin wrote:
>  Hello!
> 
> > I'm not at all sure we want to keep extending pci-assign with
> > more functionality. Why not add it to vfio instead?
> 
>  pci-assign? What exactly do you mean?
> 
>  hw/i386/kvm/pci-assign.c is modified only because 
> kvm_irqchip_add_msi_route() now wants pci_dev pointer. Since x86 platform 
> doesn't
> use device IDs, it could very well be NULL. Just i decided to keep it 
> consistent with the rest of the code.
>  Actually, patch 0003 is all about that - we add pci_dev pointer to KVM GSI 
> routing functions and make callers passing it, that's
> all.

I see, I didn't get that. Pls make this explicit in the commit log.

> Currently it's not used because KVM API is not released yet. See my full vITS 
> patchset for information on how it will be used.
> And yes, the patchset is a bit obsolete, so it doesn't have msi_device_id() 
> inline, there's copypasted calculation instead.
>  What exactly do you suggest to move to vfio?

msi_device_id seems like something kvm specific.
Maybe not vfio, just out of pci. 

> 
> Kind regards,
> Pavel Fedin
> Expert Engineer
> Samsung Electronics Research center Russia
>