Re: [Qemu-devel] [PATCH v8 03/11] netfilter: add netfilter_{add|del} commands

[Eric Blake]

On 08/26/2015 09:17 AM, Markus Armbruster wrote:
 Only reviewing QAPI/QMP and HMP interface parts for now.
 
 I apologize for not having reviewed this series earlier.  v8 is awfully
 late for the kind of review comments I have.

And I've also been behind the curve, intending to review this since it
touches API, but not getting there yet.


 +##
 +{ 'command': 'netfilter_add',
 +  'data': {
 +'type': 'str',
 +'id':   'str',
 +'netdev': 'str',
 +'*chain': 'str',
 +'*props': '**'}, 'gen': false }
 
 I figure you're merely following netdev_add precedence here (can't fault
 you for that), but netdev_add cheats, and we shouldn't add more cheats.
 
 First, 'gen': false is best avoided.  It suppresses the generated
 marshaller, and that lets you cheat.  There are cases where we can't do
 without, but I don't think this is one.
 
 When we suppress the generated marshaller, 'data' is at best a
 declaration of intent; the code can do something else entirely.  For
 instance, netdev_add declares
 
 { 'command': 'netdev_add',
   'data': {'type': 'str', 'id': 'str', '*props': '**'},
   'gen': false }
 
 but the '*props' part is a bald-faced lie: it doesn't take a 'props'
 argument.  See
 http://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00460.html
 and maybe even slides 37-38 of
 https://events.linuxfoundation.org/sites/events/files/slides/armbru-qemu-introspection.pdf
 
 I didn't check your code, but I suspect yours is a lie, too.
 
 I intend to clean up netdev_add, hopefully soon.
 
 You should use a proper QAPI data type here.  I guess the flat union I
 sketched in my reply to PATCH 2 would do nicely, except we don't support
 commands with union type data, yet.  I expect to add support to clean up
 netdev_del.

In fact, I've already proposed adding such support:

http://thread.gmane.org/gmane.comp.emulators.qemu/356265/focus=356291

 
 If you don't want to wait for that (understandable!), then I suggest you
 keep 'NetFilter' a struct type for now, use it as command data here, and
 we convert it to a flat union later.  Must be done before the release,
 to avoid backward incompatibility.
 
 Then this becomes something like:
 
 { 'command': 'netfilter-add', 'data': 'NetFilter' }

or use NetFilter as a union, but have the command be:

{ 'command': 'netfilter-add',
  'data': { 'data': 'NetFilter' } }

where you have to pass an extra layer of nesting at the QMP layer.

 
 If you need the command to take arguments you don't want to put into
 NetFilter for some reason, I can help you achieve that cleanly.

In fact, having the 'NetFilter' union be a single argument of a larger
struct makes that larger struct the nice place to stick any additional
arguments that don't need to be part of the union.

 
 +
 +##
 +# @netfilter_del:
 +#
 +# Remove a netfilter.
 +#
 +# @id: the name of the netfilter to remove
 +#
 +# Returns: Nothing on success
 +#  If @id is not a valid netfilter, DeviceNotFound
 +#
 +# Since: 2.5
 +##
 +{ 'command': 'netfilter_del', 'data': {'id': 'str'} }

Please name new QMP commands with '-' not '_'; this should be
'netfilter-del'.

-- 
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 1/2] memory: allow zero size for adjust_endianness()

[Greg Kurz]

On Wed, 26 Aug 2015 15:21:59 +0100
Peter Maydell peter.mayd...@linaro.org wrote:

 On 26 August 2015 at 11:04, Jason Wang jasow...@redhat.com wrote:
  Wildcard mmio eventfd use zero size, but it will lead abort() since it
  was illegal in adjust_endianness(). Fix this by allowing zero size.
 
  Cc: Greg Kurz gk...@linux.vnet.ibm.com
  Cc: Paolo Bonzini pbonz...@redhat.com
  Signed-off-by: Jason Wang jasow...@redhat.com
 
 This seems to me like a bug in the caller. Why would anything
 try to call into the memory subsystem to do a zero-size
 transaction?
 
 thanks
 -- PMM
 

Here's the patch which needs zero-size eventfd:

http://patchwork.ozlabs.org/patch/509428/

Cheers.

--
Greg

Re: [Qemu-devel] [PATCH v5 2/9] make: ensure all members of libqemuutil.a are linked

[Daniel P. Berrange]

On Wed, Aug 26, 2015 at 09:25:39AM -0600, Eric Blake wrote:
 On 08/26/2015 09:05 AM, Daniel P. Berrange wrote:
  The libqemuutil.a archive may contain QOM objects which are
  only indirectly referenced via __attribute__((constructor))
  annotations. Despite the constructor annotation the linker
  will think these objects are unused by the executable and
  so drop them when linking, to the system emulator. As a
  result the objects in question will be missing from QOM.
  
  Using the -Wl,--whole-archive flag instructs the linker
  to pull in everything in libqemuutil.a regardless of
  whether it thinks it is used or not.
  
  Signed-off-by: Daniel P. Berrange berra...@redhat.com
  ---
   Makefile.target | 7 ++-
   1 file changed, 6 insertions(+), 1 deletion(-)
  
  diff --git a/Makefile.target b/Makefile.target
  index 3e7aafd..5b08f0f 100644
  --- a/Makefile.target
  +++ b/Makefile.target
  @@ -180,8 +180,13 @@ all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
   $(QEMU_PROG_BUILD): config-devices.mak
   
   # build either PROG or PROGW
  +# We must use --whole-archive with libqemuutil.a otherwise the
  +# linker will drop any objects from the archive which are only
  +# indirectly referenced via __attribute__(constructor) annotations
   $(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
  -   $(call LINK, $(filter-out %.mak, $^))
  +   $(call LINK, $(filter-out %.mak, $(sort $(all-obj-y \
 
 Why the added $(sort)? Should that be a separate patch?

$(all-obj-y) has some .o's listed multiple times. $^ removes
duplicates, avoiding this problem, but if we use $(all-obj-y)
we must now remove duplicates ourselves. $(sort) removes
duplicates and AFAICT, there's no built-in make function
that can remove duplicates without sorting.

 
  +   -Wl,--whole-archive ../libqemuutil.a -Wl,--no-whole-archive \
  +   ../libqemustub.a
 
 Otherwise makes sense to me.
 Reviewed-by: Eric Blake ebl...@redhat.com



Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] block/nfs: fix calculation of allocated file size

[Jeff Cody]

On Thu, Aug 20, 2015 at 12:46:47PM +0200, Peter Lieven wrote:
 st.st_blocks is always counted in 512 byte units. Do not
 use st.st_blksize as multiplicator which may be larger.
 
 Cc: qemu-sta...@nongnu.org
 Signed-off-by: Peter Lieven p...@kamp.de
 ---
  block/nfs.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/block/nfs.c b/block/nfs.c
 index c026ff6..02eb4e4 100644
 --- a/block/nfs.c
 +++ b/block/nfs.c
 @@ -475,7 +475,7 @@ static int64_t 
 nfs_get_allocated_file_size(BlockDriverState *bs)
  aio_poll(client-aio_context, true);
  }
  
 -return (task.ret  0 ? task.ret : st.st_blocks * st.st_blksize);
 +return (task.ret  0 ? task.ret : st.st_blocks * 512);
  }
  
  static int nfs_file_truncate(BlockDriverState *bs, int64_t offset)
 -- 
 1.9.1
 

Reviewed-by: Jeff Cody jc...@redhat.com

[Qemu-devel] [PATCH RFC 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 th...@redhat.com
---
 net/dump.c | 73 +-
 1 file changed, 48 insertions(+), 25 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index 27083d4..8317102 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)
+static void dump_cleanup(DumpState *s)
 {
-struct iovec iov = {
-.iov_base = (void *)buf,
-.iov_len = size
-};
-return dump_receive_iov(nc, iov, 1);
-}
-
-static void dump_cleanup(NetClientState *nc)
-{
-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,48 @@ static int net_dump_state_init(DumpState *s, const char 
*filename,
 return 0;
 }
 
+/* Dumping via VLAN netclient */
+
+typedef struct DumpNetClient {
+NetClientState nc;
+DumpState ds;
+} 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(dc-ds, iov, 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(dc-ds, iov, cnt);
+}
+
+static void dumpclient_cleanup(NetClientState *nc)
+{
+DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
+
+dump_cleanup(dc-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 +183,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 +217,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(dnc-ds, file, len, errp);
 if (rc) {
 qemu_del_net_client(nc);
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v8 01/11] net: add a new object netfilter

[Yang Hongyang]

On 08/26/2015 10:04 PM, Markus Armbruster wrote:

Missed a bunch of revisions of this series, please excuse gaps in my
understanding.


Thank you for the review.



Yang Hongyang yan...@cn.fujitsu.com writes:


Add the framework for a new netfilter object and a new
-netfilter CLI option as a basis for the following patches.

Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com
CC: Paolo Bonzini pbonz...@redhat.com
CC: Eric Blake ebl...@redhat.com
Reviewed-by: Thomas Huth th...@redhat.com
---
  include/net/filter.h| 15 +++
  include/sysemu/sysemu.h |  1 +
  net/Makefile.objs   |  1 +
  net/filter.c| 27 +++
  qemu-options.hx |  1 +
  vl.c| 13 +
  6 files changed, 58 insertions(+)
  create mode 100644 include/net/filter.h
  create mode 100644 net/filter.c

diff --git a/include/net/filter.h b/include/net/filter.h
new file mode 100644
index 000..4242ded
--- /dev/null
+++ b/include/net/filter.h
@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2015 FUJITSU LIMITED
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_NET_FILTER_H
+#define QEMU_NET_FILTER_H
+
+#include qemu-common.h
+
+int net_init_filters(void);
+
+#endif /* QEMU_NET_FILTER_H */
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 44570d1..15d6d00 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -212,6 +212,7 @@ extern QemuOptsList qemu_chardev_opts;
  extern QemuOptsList qemu_device_opts;
  extern QemuOptsList qemu_netdev_opts;
  extern QemuOptsList qemu_net_opts;
+extern QemuOptsList qemu_netfilter_opts;
  extern QemuOptsList qemu_global_opts;
  extern QemuOptsList qemu_mon_opts;

diff --git a/net/Makefile.objs b/net/Makefile.objs
index ec19cb3..914aec0 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -13,3 +13,4 @@ common-obj-$(CONFIG_HAIKU) += tap-haiku.o
  common-obj-$(CONFIG_SLIRP) += slirp.o
  common-obj-$(CONFIG_VDE) += vde.o
  common-obj-$(CONFIG_NETMAP) += netmap.o
+common-obj-y += filter.o
diff --git a/net/filter.c b/net/filter.c
new file mode 100644
index 000..4e40f08
--- /dev/null
+++ b/net/filter.c
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2015 FUJITSU LIMITED
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include qemu-common.h
+#include net/filter.h
+
+int net_init_filters(void)
+{
+return 0;
+}
+
+QemuOptsList qemu_netfilter_opts = {
+.name = netfilter,
+.implied_opt_name = type,
+.head = QTAILQ_HEAD_INITIALIZER(qemu_netfilter_opts.head),
+.desc = {
+/*
+ * no elements = accept any params
+ * validation will happen later
+ */
+{ /* end of list */ }
+},
+};


Ignorant question: why dynamic validation (empty .desc) instead of
statically defined parameters?  The documentation you add in PATCH 10
suggests they're not actually dynamic.


Actually I was following the netdev stuff. There might be bunch of filters,
and I don't know the params of each filter until it is realized.




diff --git a/qemu-options.hx b/qemu-options.hx
index 77f5853..0d52d02 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1575,6 +1575,7 @@ DEF(net, HAS_ARG, QEMU_OPTION_net,
  socket][,vlan=n][,option][,option][,...]\n
  old way to initialize a host network interface\n
  (use the -netdev option if possible instead)\n, 
QEMU_ARCH_ALL)
+DEF(netfilter, HAS_ARG, QEMU_OPTION_netfilter, , QEMU_ARCH_ALL)
  STEXI
  @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] 
[,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
  @findex -net


Wrong spot: this is between DEF(net, ...) and its STEXI..ETEXI stanza.
Suggest to move it behind the ETEXI.

Missing help string.  You add the help in PATCH 10.  What about adding
it here already?  Would serve as a hint of the things to come later in
your series.

Missing STEXI..ETEXI stanza for the user manual.


If I understand correctly, you are suggesting separate the netfilter from
net, seems more reasonable, so I should add something like:
DEF(netfilter, HAS_ARG, QEMU_OPTION_netfilter, , QEMU_ARCH_ALL)
STEXI..ETEXI

after the DEF(net, ...) and its STEXI..ETEXI stanza, am I right?




diff --git a/vl.c b/vl.c
index 584ca88..aee931a 100644
--- a/vl.c
+++ b/vl.c
@@ -75,6 +75,7 @@ int main(int argc, char **argv)
  #include monitor/qdev.h
  #include sysemu/bt.h
  #include net/net.h
+#include net/filter.h
  #include net/slirp.h
  #include monitor/monitor.h
  #include ui/console.h
@@ -2998,6 +2999,7 @@ int main(int argc, char **argv, char **envp)
  qemu_add_opts(qemu_device_opts);
  qemu_add_opts(qemu_netdev_opts);
  qemu_add_opts(qemu_net_opts);
+qemu_add_opts(qemu_netfilter_opts);
  qemu_add_opts(qemu_rtc_opts);
  qemu_add_opts(qemu_global_opts);
  

[Qemu-devel] [PATCH RFC 5/5] net/dump: Add documentation

[Thomas Huth]

Add a short description for the command line options.

Signed-off-by: Thomas Huth th...@redhat.com
---
 qemu-options.hx | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 390e055..21cf129 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1578,7 +1578,10 @@ DEF(net, HAS_ARG, QEMU_OPTION_net,
 DEF(netfilter, HAS_ARG, QEMU_OPTION_netfilter,
 -netfilter buffer,id=str,netdev=str[,chain=in|out|all,interval=n]\n
 buffer netdev in/out packets. if interval provided, will 
release\n
-packets by interval. interval scale: microsecond\n, 
QEMU_ARCH_ALL)
+packets by interval. interval scale: microsecond\n
+-netfilter dump,id=str,netdev=str,file=f[,maxlen=n,chain=in|out|all]\n
+dump network traffic to file 'f' (max n bytes per 
packet)\n,
+ QEMU_ARCH_ALL)
 STEXI
 @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] 
[,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
 @findex -net
@@ -1984,10 +1987,12 @@ qemu -m 512 -object 
memory-backend-file,id=mem,size=512M,mem-path=/hugetlbfs,sha
  -device virtio-net-pci,netdev=net0
 @end example
 
-@item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
-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.
+@item -netfilter 
dump,id=@var{idstr},netdev=@var{dev},file=@var{file}[,maxlen=@var{len},chain=in|out|all]
+@itemx -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
+Dump network traffic on netdev @var{dev} (or VLAN @var{n} reciprocally) to
+file @var{file} (@file{qemu-vlan0.pcap} for '-net dump' 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.
 
 @item -net none
 Indicate that no network devices should be configured. It is used to
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2] i386: keep cpu_model field in MachineState uptodate

[Zhu Guihua]

On 08/26/2015 11:11 PM, Eduardo Habkost wrote:

On Mon, Aug 24, 2015 at 05:42:09PM +0800, Zhu Guihua wrote:

Update cpu_model in MachineState for i386, so that the field can be used
for cpu hotplug, instead of using a static variable.

Signed-off-by: Zhu Guihua zhugh.f...@cn.fujitsu.com

[...]

-void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
+void pc_cpus_init(MachineState *machine, DeviceState *icc_bridge)

This is a PC initialization function, so a PCMachineState argument makes
more sense. See pc_cmos_init(), load_linux(), pc_guest_info_init(),
pc_memory_init(), etc.


Oh, I know how to do this. Thanks for your suggestion.

Thanks,
Zhu



The rest looks good.

Re: [Qemu-devel] [PATCH RFC 0/5] Network traffic dumping via netfilter

[Jason Wang]

On 08/27/2015 10:33 AM, Thomas Huth wrote:
 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. It can be used
 like this for example:

  qemu-system-ppc64 -nographic -vga none -device virtio-net,netdev=mynet \
-netdev user,id=mynet,tftp=/tmp/tftp,bootfile=zImage -boot n \
-netfilter dump,id=f0,netdev=mynet,file=/tmp/filterdump.dat

 This series is based on v8 of Yang Hongyang's netfilter patches
 (i.e. these have to be applied before the dumping patches can be
 used). Since these netfilter patches are not upstream yet and still
 might change the API in future versions, I'm sending my patches as
 RFC (but since the integration with the current netfilter patches
 was pretty easy already, they should be ready as normal patches very
 soon after the netfilter patches have been finalized).

 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
   net/dump: Add documentation

  net/dump.c   | 152 
 ---
  net/filter.c |   1 +
  net/filters.h|   2 +
  qapi-schema.json |  20 +++-
  qemu-options.hx  |  15 --
  5 files changed, 155 insertions(+), 35 deletions(-)


Looks good to me.

Thanks

Re: [Qemu-devel] Plan for using softmmu with linux-user

[Chen Gang]

On 2015年08月19日 16:01, gchen gchen wrote:
 On 2015年08月15日 04:45, Chen Gang wrote:
 On 8/14/15 22:44, Richard Henderson wrote:
 On 08/14/2015 02:37 AM, gchen gchen wrote:
  - If I implement SW64 tcg backend, I guess, I cann't get help from qemu
upstream: I don't think SW64 is valuable enough for upstream (either
I am not sure that I can implment Alpha tcg backend in working time).

 It'll need some updating to apply to master, but I started an alpha backend 
 a
 couple of years ago.  It looks like it was last rebased in May 2014.

   git://github.com/rth7680/qemu.git tcg-alpha-2


Really thank you very much again!!

I have implemented sw_64 tcg back end, its performance is much better
than tci (10 times faster for testing i386 bash). I guess at present,
our qemu performance should be enough for my current using. :-)

And next, I shall try to merge alpha related code to the latest qemu,
and send patches for it. Hope it is useful for our qemu (since no any
additional reply for it, I guess I should try to send patches for it).


Thanks.



 After get sw_64 pc, it is much slower than I guess (it is much slower
 than my intel x86_64 laptop which was made in 2007-2008). So I have to
 use tcg backend instead of tci.

 Will qemu upstream accept alpha tcg backend to master tree? If possible
 I can do it in my working time (after simply trying, I am sure, alpha
 tcg backend can not work if only simply merge and let it pass building).

 By the way, for me, I have to process alpha/sw_64 tcg backend firstly,
 then process softmmu + linux-user.


 Welcome any ideas, suggestions, and completions.

 Thanks.

 Thank you very much, I shall clone it. I guess, it will save my much
 time resources.

 If possible, it will be better to merge the alpha tcg backend to qemu
 master main branch (at least for me, it is useful).
 :-)


 --
 Chen Gang

 Open, share, and attitude like air, water, and life which God blessed



--
Chen Gang

Open, share, and attitude like air, water, and life which God blessed
  

Re: [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter

[Yang Hongyang]

On 08/27/2015 09:05 AM, Thomas Huth wrote:

On 26/08/15 11:59, Yang Hongyang wrote:

This patch add a new object netfilter, capture all network packets.
Also implement a netbuffer based on this object.
the buffer netfilter could be used by VM FT solutions like
MicroCheckpointing, to buffer/release packets. Or to simulate
packet delay.

You can also get the series from:
https://github.com/macrosheep/qemu/tree/netfilter-v8

Usage:
  -netdev tap,id=bn0
  -netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000
  -device e1000,netdev=bn0

dynamically add/remove netfilters:
  netfilter_add buffer,id=f0,netdev=bn0,chain=in,interval=1000
  netfilter_del f0

NOTE:
  interval's scale is microsecond.
  chain is optional, and is one of in|out|all, default is all.
in means this filter will receive packets sent to the @netdev
out means this filter will receive packets sent from the @netdev
all means this filter will receive packets both sent to/from
  the @netdev

TODO:
  - dump


FYI, I've now reworked my dump patch series to use your netfilter
infrastructure - worked out fine and it was pretty easy since your
netfilter infrastructure is very usable! I'll polish my patches a little
bit more, then I'll send them out, too. So I am looking forward to see
your netfilter infrastructure included in upstream soon :-)


That's great! Thank you! Seems the patchset still needs some work on the QAPI
part, I will address it as soon as possiable.



  Thomas

.



--
Thanks,
Yang.

Re: [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter

[Yang Hongyang]

On 08/26/2015 11:58 PM, Markus Armbruster wrote:

Yang Hongyang yan...@cn.fujitsu.com writes:


This patch add a new object netfilter, capture all network packets.
Also implement a netbuffer based on this object.
the buffer netfilter could be used by VM FT solutions like
MicroCheckpointing, to buffer/release packets. Or to simulate
packet delay.


I like the idea of having a generic netfilter infrastructure.  The
external interfaces need a bit more work.  I'm sure you can get them
right with a little help.


Thanks a lot for the review and help!



Thanks for tackling this!
.



--
Thanks,
Yang.

Re: [Qemu-devel] [PATCH v8 10/11] filter/buffer: update command description and help

[Yang Hongyang]

On 08/26/2015 11:55 PM, Markus Armbruster wrote:

Yang Hongyang yan...@cn.fujitsu.com writes:


now that we have a buffer netfilter, update the command
description and help.

Signed-off-by: Yang Hongyang yan...@cn.fujitsu.com
CC: Luiz Capitulino lcapitul...@redhat.com
CC: Markus Armbruster arm...@redhat.com
---
v8: add more description for the filter to the TEXI section
---
  hmp-commands.hx |  2 +-
  qemu-options.hx | 18 +-
  qmp-commands.hx |  2 +-
  3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 902e2d1..63177a8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1255,7 +1255,7 @@ ETEXI
  {
  .name   = netfilter_add,
  .args_type  = netfilter:O,
-.params = 
[type],id=str,netdev=str[,chain=in|out|all,prop=value][,...],
+.params = 
[buffer],id=str,netdev=str[,chain=in|out|all,prop=value][,...],
  .help   = add netfilter,
  .mhandler.cmd = hmp_netfilter_add,
  .command_completion = netfilter_add_completion,


This change looks odd.  Actually, params look odd before and after the
change :)

I suspect you're trying to follow netdev_add precedence.  Its params:

 .params = 
[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...],

Neglects to mention the long form type=, but that's pretty common.

Uses square brackets both for grouping and for optionalness.  We suck.

Users can probably figure out that the first [ ] is just grouping, and
the others are optionalness.

Your params are even more confusing, because the first [ ] is again
grouping, but there's just one alternative: buffer.

What about not simply writing

 .params = 
buffer,id=str,netdev=str[,chain=in|out|all,prop=value][,...],

for now?


Sure, I agree it is confusing when it has only one alternative now...When we
added more filters, we can change it later.




diff --git a/qemu-options.hx b/qemu-options.hx
index 0d52d02..390e055 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1575,7 +1575,10 @@ DEF(net, HAS_ARG, QEMU_OPTION_net,
  socket][,vlan=n][,option][,option][,...]\n
  old way to initialize a host network interface\n
  (use the -netdev option if possible instead)\n, 
QEMU_ARCH_ALL)
-DEF(netfilter, HAS_ARG, QEMU_OPTION_netfilter, , QEMU_ARCH_ALL)
+DEF(netfilter, HAS_ARG, QEMU_OPTION_netfilter,
+-netfilter buffer,id=str,netdev=str[,chain=in|out|all,interval=n]\n
+buffer netdev in/out packets. if interval provided, will 
release\n
+packets by interval. interval scale: microsecond\n, 
QEMU_ARCH_ALL)


If I understand your intent correctly, interval is the amount of time
to delay packets.  What about calling it delay?


You can take it as the amount of time to delay packets, but for other usecase
like MC, you can also take it as an interval to release packets, that is to
say, to release packets every {interval} time.



Suggest something like

 buffer network packets, delaying them for 'delay' microseconds
 (default 0us)


  STEXI
  @item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] 
[,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
  @findex -net
@@ -1990,6 +1993,19 @@ libpcap, so it can be analyzed with tools such as 
tcpdump or Wireshark.
  Indicate that no network devices should be configured. It is used to
  override the default configuration (@option{-net nic -net user}) which
  is activated if no @option{-net} options are provided.
+
+@item -netfilter 
buffer,id=@var{id},netdev=@var{netdevid}[,chain=@var{in/out/all}][,interval=@var{n}]


'n' is an unusual choice for a time delay.  What about 't', 'dt', or
'delay'?


't' is better, thanks!




+Buffer netdev @var{netdevid} input or output packets.


Are there any other kinds of packets?


No, only input/output packets now.




if interval @var{n}
+provided, will release packets by interval. interval scale: microsecond.


Please start sentences with a capital letter.

Suggest something like

 Packets are delayed by @var{n} microseconds.  Defaults to 0us,
 i.e. no delay.


+
+chain @var{in/out/all} is an option that can be applied to any netfilters, if
+not provided, default is @option{all}.


to any netfilter (singular)

Drop if not provided,


Ok, thanks!




+
+@option{in} means this filter will receive packets sent to the netdev
+
+@option{out} means this filter will receive packets sent from the netdev
+
+@option{all} means this filter will receive packets both sent to/from the 
netdev
  ETEXI

  STEXI
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4f0dc98..9419a6f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -947,7 +947,7 @@ Arguments:
  Example:

  - { execute: netfilter_add,
-arguments: { type: type, id: nf0,
+arguments: { type: buffer, id: nf0,
   

[Qemu-devel] [PATCH v2] sheepdog: add reopen support

[Liu Yuan]

From: Liu Yuan liuy...@cmss.chinamobile.com

With reopen supported, block-commit (and offline commit) is now supported for
image files whose base image uses the Sheepdog protocol driver.

Cc: qemu-devel@nongnu.org
Cc: Jeff Cody jc...@redhat.com
Cc: Kevin Wolf kw...@redhat.com
Cc: Stefan Hajnoczi stefa...@redhat.com
Signed-off-by: Liu Yuan liuy...@cmss.chinamobile.com
---
v2:
 - free AioHandler [Jeff Cody]

 block/sheepdog.c | 75 
 1 file changed, 75 insertions(+)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 9585beb..e54add4 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -377,6 +377,11 @@ typedef struct BDRVSheepdogState {
 QLIST_HEAD(inflight_aiocb_head, SheepdogAIOCB) inflight_aiocb_head;
 } BDRVSheepdogState;
 
+typedef struct BDRVSheepdogReopenState {
+int fd;
+int cache_flags;
+} BDRVSheepdogReopenState;
+
 static const char * sd_strerror(int err)
 {
 int i;
@@ -1486,6 +1491,67 @@ out:
 return ret;
 }
 
+static int sd_reopen_prepare(BDRVReopenState *state, BlockReopenQueue *queue,
+ Error **errp)
+{
+BDRVSheepdogState *s = state-bs-opaque;
+BDRVSheepdogReopenState *re_s;
+int ret = 0;
+
+re_s = state-opaque = g_new0(BDRVSheepdogReopenState, 1);
+
+if (state-flags  BDRV_O_NOCACHE) {
+re_s-cache_flags = SD_FLAG_CMD_DIRECT;
+}
+
+re_s-fd = get_sheep_fd(s, errp);
+if (re_s-fd  0) {
+ret = re_s-fd;
+return ret;
+}
+
+return ret;
+}
+
+static void sd_reopen_commit(BDRVReopenState *state)
+{
+BDRVSheepdogReopenState *re_s = state-opaque;
+BDRVSheepdogState *s = state-bs-opaque;
+
+if (s-fd) {
+aio_set_fd_handler(s-aio_context, s-fd, NULL, NULL, NULL);
+closesocket(s-fd);
+}
+
+s-fd = re_s-fd;
+s-cache_flags = re_s-cache_flags;
+
+g_free(state-opaque);
+state-opaque = NULL;
+
+return;
+}
+
+static void sd_reopen_abort(BDRVReopenState *state)
+{
+BDRVSheepdogReopenState *re_s = state-opaque;
+BDRVSheepdogState *s = state-bs-opaque;
+
+if (re_s == NULL) {
+return;
+}
+
+if (re_s-fd) {
+aio_set_fd_handler(s-aio_context, re_s-fd, NULL, NULL, NULL);
+closesocket(re_s-fd);
+}
+
+g_free(state-opaque);
+state-opaque = NULL;
+
+return;
+}
+
 static int do_sd_create(BDRVSheepdogState *s, uint32_t *vdi_id, int snapshot,
 Error **errp)
 {
@@ -2703,6 +2769,9 @@ static BlockDriver bdrv_sheepdog = {
 .instance_size  = sizeof(BDRVSheepdogState),
 .bdrv_needs_filename = true,
 .bdrv_file_open = sd_open,
+.bdrv_reopen_prepare= sd_reopen_prepare,
+.bdrv_reopen_commit = sd_reopen_commit,
+.bdrv_reopen_abort  = sd_reopen_abort,
 .bdrv_close = sd_close,
 .bdrv_create= sd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
@@ -2736,6 +2805,9 @@ static BlockDriver bdrv_sheepdog_tcp = {
 .instance_size  = sizeof(BDRVSheepdogState),
 .bdrv_needs_filename = true,
 .bdrv_file_open = sd_open,
+.bdrv_reopen_prepare= sd_reopen_prepare,
+.bdrv_reopen_commit = sd_reopen_commit,
+.bdrv_reopen_abort  = sd_reopen_abort,
 .bdrv_close = sd_close,
 .bdrv_create= sd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
@@ -2769,6 +2841,9 @@ static BlockDriver bdrv_sheepdog_unix = {
 .instance_size  = sizeof(BDRVSheepdogState),
 .bdrv_needs_filename = true,
 .bdrv_file_open = sd_open,
+.bdrv_reopen_prepare= sd_reopen_prepare,
+.bdrv_reopen_commit = sd_reopen_commit,
+.bdrv_reopen_abort  = sd_reopen_abort,
 .bdrv_close = sd_close,
 .bdrv_create= sd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
-- 
1.9.1

Re: [Qemu-devel] Should we auto-generate IDs?

[Programmingkid]

On Aug 26, 2015, at 6:01 PM, Jeff Cody wrote:

 On Wed, Aug 26, 2015 at 02:17:17PM -0400, Programmingkid wrote:
 
 On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote:
 
 On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote:
 
 On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:
 
 On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
 Did you drop cc's intentionally?  I put them right back.
 
 Programmingkid programmingk...@gmail.com writes:
 
 On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
 
 You're proposing to revise a qdev design decision, namely the purpose 
 of
 IDs.  This has been discussed before, and IDs remained unchanged.
 Perhaps it's time to revisit this issue.  Cc'ing a few more people.
 
 Relevant prior threads:
 * [PATCH] qdev: Reject duplicate and anti-social device IDs
 http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
 * [PATCH 6/6] qdev: Generate IDs for anonymous devices
 http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
 * [PATCH] qdev: Assign a default device ID when none is provided.
 http://thread.gmane.org/gmane.comp.emulators.qemu/249702
 * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from 
 QemuOpt
 http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
 
 
 After reading all the threads, I realize why all the attempts to
 accept a device ID patch failed.
 It is because it was assumed everyone would agree on one patch to
 accept. This is
 very unlikely. It would take someone in a leadership position to
 decide which patch
 should be accepted. From one of the threads above, I saw Anthony
 Liguori participate.
 He was in the perfect position to make the choice. The person who is
 in his position now
 is Peter Maydell. Maybe we should just ask him to look at all the
 candidate patches and
 have him pick one to use. 
 
 Yes, when no consensus emerges, problems tend to go unsolved.
 
 Before we appeal to authority to break the deadlock, we should make
 another attempt at finding consensus.
 
 I know that we've entertained the idea of automatically generated IDs
 for block layer objects (that's why I cc'ed some block guys).
 
 Yeah, I was one of the ones that proposed some auto-generated IDs for
 the block layer, specifically for BlockDriverState, making use of the
 node-name field that Benoit introduced a while ago.  Here is my patch
 (not sure if this is the latest version, but it is sufficient for this
 discussion):
 
 http://patchwork.ozlabs.org/patch/355990/
 
 I'm not sure about the requirements needed by device ID names, and
 they may of course differ from what I was thinking for BDS entries.
 
 Here is what I was after with my patch for node-name auto-generation:
 
 * Identifiable as QEMU generated / reserved namespace
 
 * Guaranteed uniqueness
 
 * Non-predictable (don't want users trying to guess / assume
 generated node-names)
 
 My approach was overkill in some ways (24 characters!).  But for
 better or worse, what I had was:
 
  __qemu##IAIYNXXR
  
 QEMU namespace |
| ^
 Increment counter, unique | |
  |
 Random string, to spoil prediction  |
 
 Yikes! 24 characters long. That is a bit much to type. Thank you very much
 for your effort.
 
 IMO, the number of characters to type is pretty low on the list of
 requirements, although it can still be addressed secondary to other
 concerns.
 
 I should have made this in reply to Markus' other email, because the
 important part of this is try and address his point #2:
 
 (from Markus' other email):
 2. The ID must be well-formed.
 
 To have a well-formed ID, we need to have know requirements of the ID
 structure (i.e. the why and what of it all)
 
 I don't know if the three requirements I had above apply to all areas
 in QEMU, but I expect they do, in varying degrees of importance.  The
 length itself can be tweaked.
 
 Talking with John Snow over IRC (added to the CC), one thing he
 suggested was adding in sub-domain spaces; e.g.:
 
 __qemu#bn##IAIYNXXR
 
 Where the 'bn' in this case would be for Block Nodes, etc..
 
 This may make the scheme extensible through QEMU, where auto-generated
 IDs are desired.
 
 (sorry to say, this lengthens things, rather than shortening them!)
 
 We can, of course, make the string shorter - if the random characters
 are just there for spoiling predictability, then 2-3 should be
 sufficient. We could then end up with something like this:
 
 __qemu#bn##XR
 
 The __qemu part of the namespace could be shortened as well, but it
 would be nice if it was easy recognizable as being from QEMU.
 
 If this ID format was supported, I'm thinking being able to copy and paste 
 from
 the monitor is a necessary feature. 
 
 Any way it could be shorted? I was hoping no more than three characters 
 long. 
 
 
 Likely could be shorter, but something in the realm of three
 characters 

[Qemu-devel] [PATCH RFC 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 th...@redhat.com
---
 net/dump.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index f8afb83..27083d4 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(net_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(net_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 RFC 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 th...@redhat.com
---
 net/dump.c | 24 +---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index 02c8064..f8afb83 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, hdr, sizeof(hdr)) != sizeof(hdr) ||
-write(s-fd, buf, caplen) != caplen) {
+
+dumpiov[0].iov_base = hdr;
+dumpiov[0].iov_len = sizeof(hdr);
+cnt = iov_copy(dumpiov[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, iov, 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

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

[Thomas Huth]

Add glue code to use the dumping functions as a netdev
filter, too.

Signed-off-by: Thomas Huth th...@redhat.com
---
 net/dump.c   | 54 ++
 net/filter.c |  1 +
 net/filters.h|  2 ++
 qapi-schema.json | 20 +++-
 4 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/net/dump.c b/net/dump.c
index 8317102..b09d2b9 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -28,7 +28,9 @@
 #include qemu/iov.h
 #include qemu/log.h
 #include qemu/timer.h
+#include net/filter.h
 #include hub.h
+#include filters.h
 
 typedef struct DumpState {
 int64_t start_ts;
@@ -224,3 +226,55 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
 }
 return rc;
 }
+
+/* Dumping via filter */
+
+typedef struct NetFilterDumpState {
+NetFilterState nf;
+DumpState ds;
+} 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 = DO_UPCAST(NetFilterDumpState, nf, nf);
+
+dump_receive_iov(nfds-ds, iov, iovcnt);
+return 0;
+}
+
+static void filter_dump_cleanup(NetFilterState *nf)
+{
+NetFilterDumpState *nfds = DO_UPCAST(NetFilterDumpState, nf, nf);
+
+dump_cleanup(nfds-ds);
+}
+
+static NetFilterInfo net_filter_dump_info = {
+.type = NET_FILTER_OPTIONS_KIND_DUMP,
+.size = sizeof(NetFilterDumpState),
+.receive_iov = filter_dump_receive_iov,
+.cleanup = filter_dump_cleanup,
+};
+
+int net_init_filter_dump(const NetFilterOptions *opts, const char *name,
+ int chain, NetClientState *netdev, Error **errp)
+{
+NetFilterState *nf;
+NetFilterDumpState *nfds;
+const NetFilterDumpOptions *dumpopt;
+int dump_len = 65536;
+
+assert(opts-kind == NET_FILTER_OPTIONS_KIND_DUMP);
+dumpopt = opts-dump;
+if (dumpopt-has_maxlen) {
+dump_len = dumpopt-maxlen;
+}
+
+nf = qemu_new_net_filter(net_filter_dump_info, netdev, name, chain);
+nfds = DO_UPCAST(NetFilterDumpState, nf, nf);
+
+return net_dump_state_init(nfds-ds, dumpopt-file, dump_len, errp);
+}
diff --git a/net/filter.c b/net/filter.c
index f23fc7f..536d236 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -216,6 +216,7 @@ typedef int (NetFilterInit)(const NetFilterOptions *opts,
 static
 NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
 [NET_FILTER_OPTIONS_KIND_BUFFER] = net_init_filter_buffer,
+[NET_FILTER_OPTIONS_KIND_DUMP] = net_init_filter_dump,
 };
 
 static int net_filter_init1(const NetFilter *netfilter, Error **errp)
diff --git a/net/filters.h b/net/filters.h
index 3b546db..f63bde1 100644
--- a/net/filters.h
+++ b/net/filters.h
@@ -13,5 +13,7 @@
 
 int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
int chain, NetClientState *netdev, Error **errp);
+int net_init_filter_dump(const NetFilterOptions *opts, const char *name,
+   int chain, NetClientState *netdev, Error **errp);
 
 #endif /* QEMU_NET_FILTERS_H */
diff --git a/qapi-schema.json b/qapi-schema.json
index 7882641..71caca9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2599,6 +2599,23 @@
 '*interval': 'uint32' } }
 
 ##
+# @NetFilterDumpOptions
+#
+# a dumping filter for network backends.
+#
+# @file: Name of the file where the dump data should be written into.
+#
+# @maxlen: Crop big packets to this size before writing them into the
+#  dump file.
+#
+# Since 2.5
+##
+{ 'struct': 'NetFilterDumpOptions',
+  'data': {
+'file': 'str',
+'*maxlen':  'int32' } }
+
+##
 # @NetFilterOptions
 #
 # A discriminated record of network filters.
@@ -2608,7 +2625,8 @@
 ##
 { 'union': 'NetFilterOptions',
   'data': {
-'buffer': 'NetFilterBufferOptions'} }
+'buffer': 'NetFilterBufferOptions',
+'dump':   'NetFilterDumpOptions' } }
 
 ##
 # @NetFilter
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC 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. It can be used
like this for example:

 qemu-system-ppc64 -nographic -vga none -device virtio-net,netdev=mynet \
   -netdev user,id=mynet,tftp=/tmp/tftp,bootfile=zImage -boot n \
   -netfilter dump,id=f0,netdev=mynet,file=/tmp/filterdump.dat

This series is based on v8 of Yang Hongyang's netfilter patches
(i.e. these have to be applied before the dumping patches can be
used). Since these netfilter patches are not upstream yet and still
might change the API in future versions, I'm sending my patches as
RFC (but since the integration with the current netfilter patches
was pretty easy already, they should be ready as normal patches very
soon after the netfilter patches have been finalized).

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
  net/dump: Add documentation

 net/dump.c   | 152 ---
 net/filter.c |   1 +
 net/filters.h|   2 +
 qapi-schema.json |  20 +++-
 qemu-options.hx  |  15 --
 5 files changed, 155 insertions(+), 35 deletions(-)

-- 
1.8.3.1

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

[Yang Hongyang]

On 08/27/2015 10:33 AM, Thomas Huth wrote:

Add glue code to use the dumping functions as a netdev
filter, too.


Overall looks good to me, thanks.

Note that the QAPI part might change later, so there might be some rebase
work later, thanks again!



Signed-off-by: Thomas Huth th...@redhat.com
---
  net/dump.c   | 54 ++
  net/filter.c |  1 +
  net/filters.h|  2 ++
  qapi-schema.json | 20 +++-
  4 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/net/dump.c b/net/dump.c
index 8317102..b09d2b9 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -28,7 +28,9 @@
  #include qemu/iov.h
  #include qemu/log.h
  #include qemu/timer.h
+#include net/filter.h
  #include hub.h
+#include filters.h

  typedef struct DumpState {
  int64_t start_ts;
@@ -224,3 +226,55 @@ int net_init_dump(const NetClientOptions *opts, const char 
*name,
  }
  return rc;
  }
+
+/* Dumping via filter */
+
+typedef struct NetFilterDumpState {
+NetFilterState nf;
+DumpState ds;
+} 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 = DO_UPCAST(NetFilterDumpState, nf, nf);
+
+dump_receive_iov(nfds-ds, iov, iovcnt);
+return 0;
+}
+
+static void filter_dump_cleanup(NetFilterState *nf)
+{
+NetFilterDumpState *nfds = DO_UPCAST(NetFilterDumpState, nf, nf);
+
+dump_cleanup(nfds-ds);
+}
+
+static NetFilterInfo net_filter_dump_info = {
+.type = NET_FILTER_OPTIONS_KIND_DUMP,
+.size = sizeof(NetFilterDumpState),
+.receive_iov = filter_dump_receive_iov,
+.cleanup = filter_dump_cleanup,
+};
+
+int net_init_filter_dump(const NetFilterOptions *opts, const char *name,
+ int chain, NetClientState *netdev, Error **errp)
+{
+NetFilterState *nf;
+NetFilterDumpState *nfds;
+const NetFilterDumpOptions *dumpopt;
+int dump_len = 65536;
+
+assert(opts-kind == NET_FILTER_OPTIONS_KIND_DUMP);
+dumpopt = opts-dump;
+if (dumpopt-has_maxlen) {
+dump_len = dumpopt-maxlen;
+}
+
+nf = qemu_new_net_filter(net_filter_dump_info, netdev, name, chain);
+nfds = DO_UPCAST(NetFilterDumpState, nf, nf);
+
+return net_dump_state_init(nfds-ds, dumpopt-file, dump_len, errp);
+}
diff --git a/net/filter.c b/net/filter.c
index f23fc7f..536d236 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -216,6 +216,7 @@ typedef int (NetFilterInit)(const NetFilterOptions *opts,
  static
  NetFilterInit * const net_filter_init_fun[NET_FILTER_OPTIONS_KIND_MAX] = {
  [NET_FILTER_OPTIONS_KIND_BUFFER] = net_init_filter_buffer,
+[NET_FILTER_OPTIONS_KIND_DUMP] = net_init_filter_dump,
  };

  static int net_filter_init1(const NetFilter *netfilter, Error **errp)
diff --git a/net/filters.h b/net/filters.h
index 3b546db..f63bde1 100644
--- a/net/filters.h
+++ b/net/filters.h
@@ -13,5 +13,7 @@

  int net_init_filter_buffer(const NetFilterOptions *opts, const char *name,
 int chain, NetClientState *netdev, Error **errp);
+int net_init_filter_dump(const NetFilterOptions *opts, const char *name,
+   int chain, NetClientState *netdev, Error **errp);


wrong alignment.



  #endif /* QEMU_NET_FILTERS_H */
diff --git a/qapi-schema.json b/qapi-schema.json
index 7882641..71caca9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2599,6 +2599,23 @@
  '*interval': 'uint32' } }

  ##
+# @NetFilterDumpOptions
+#
+# a dumping filter for network backends.
+#
+# @file: Name of the file where the dump data should be written into.
+#
+# @maxlen: Crop big packets to this size before writing them into the
+#  dump file.
+#
+# Since 2.5
+##
+{ 'struct': 'NetFilterDumpOptions',
+  'data': {
+'file': 'str',
+'*maxlen':  'int32' } }
+
+##
  # @NetFilterOptions
  #
  # A discriminated record of network filters.
@@ -2608,7 +2625,8 @@
  ##
  { 'union': 'NetFilterOptions',
'data': {
-'buffer': 'NetFilterBufferOptions'} }
+'buffer': 'NetFilterBufferOptions',
+'dump':   'NetFilterDumpOptions' } }

  ##
  # @NetFilter



--
Thanks,
Yang.

Re: [Qemu-devel] [PATCH v8 00/11] Add a netfilter object and netbuffer filter

[Jason Wang]

On 08/26/2015 05:59 PM, Yang Hongyang wrote:
 This patch add a new object netfilter, capture all network packets.
 Also implement a netbuffer based on this object.
 the buffer netfilter could be used by VM FT solutions like
 MicroCheckpointing, to buffer/release packets. Or to simulate
 packet delay.

 You can also get the series from:
 https://github.com/macrosheep/qemu/tree/netfilter-v8

 Usage:
  -netdev tap,id=bn0
  -netfilter buffer,id=f0,netdev=bn0,chain=in,interval=1000
  -device e1000,netdev=bn0

 dynamically add/remove netfilters:
  netfilter_add buffer,id=f0,netdev=bn0,chain=in,interval=1000
  netfilter_del f0

 NOTE:
  interval's scale is microsecond.
  chain is optional, and is one of in|out|all, default is all.
in means this filter will receive packets sent to the @netdev
out means this filter will receive packets sent from the @netdev
all means this filter will receive packets both sent to/from
  the @netdev

 TODO:
  - dump

 v8:
  - some minor fixes according to Thomas's comments
  - rebased to the latest master branch

 v7:
  - print filter info when execute 'info network'
  - addressed Jason's comments

 v6:
  - add multiqueue support, please see individual patch for detail

 v5:
  - add a sent_cb param to filter receive_iov api
  - squash the 4th patch into patch 3
  - remove dummy sent_cb (buffer filter)
  - addressed Jason's other comments, see individual patches for detail

 v4:
  - get rid of struct Filter
  - squash the 4th patch into patch 2
  - fix qemu_netfilter_pass_to_next_iov
  - get rid of bh (buffer filter)
  - release the packet to next filter instead of to receiver (buffer filter)

 v3:
  - add an api to pass the packet to next filter
  - remove netfilters when delete netdev
  - add qtest testcases for netfilter
  - addressed comments from Jason

 v2:
  - add a chain option to netfilter object
  - move the hook place earlier, before net_queue_send
  - drop the unused api in buffer filter
  - squash buffer filter patches into one
  - remove receive() api from netfilter, only receive_iov() is enough
  - addressed comments from JasonThomas

 v1:
  initial patch.

 Yang Hongyang (11):
   net: add a new object netfilter
   init/cleanup of netfilter object
   netfilter: add netfilter_{add|del} commands
   netfilter: hook packets before net queue send
   move out net queue structs define
   netfilter: add an API to pass the packet to next filter
   netfilter: print filter info associate with the netdev
   net/queue: export qemu_net_queue_append_iov
   netfilter: add a netbuffer filter
   filter/buffer: update command description and help
   tests: add test cases for netfilter object

  hmp-commands.hx |  30 +
  hmp.c   |  29 +
  hmp.h   |   4 +
  include/net/filter.h|  64 ++
  include/net/net.h   |   1 +
  include/net/queue.h |  26 
  include/qemu/typedefs.h |   1 +
  include/sysemu/sysemu.h |   1 +
  monitor.c   |  33 +
  net/Makefile.objs   |   2 +
  net/filter-buffer.c | 125 ++
  net/filter.c| 332 
 
  net/filters.h   |  17 +++
  net/net.c   |  85 +
  net/queue.c |  31 +
  qapi-schema.json| 100 +++
  qemu-options.hx |  17 +++
  qmp-commands.hx |  57 +
  tests/.gitignore|   1 +
  tests/Makefile  |   2 +
  tests/test-netfilter.c  | 194 
  vl.c|  13 ++
  22 files changed, 1140 insertions(+), 25 deletions(-)
  create mode 100644 include/net/filter.h
  create mode 100644 net/filter-buffer.c
  create mode 100644 net/filter.c
  create mode 100644 net/filters.h
  create mode 100644 tests/test-netfilter.c


Looks good to me. After addressing comments of interfaces, I think it
was pretty ready to be merged.

Thanks

Re: [Qemu-devel] Should we auto-generate IDs?

[Programmingkid]

On Aug 26, 2015, at 6:04 PM, John Snow wrote:

 
 
 On 08/26/2015 06:01 PM, Jeff Cody wrote:
 On Wed, Aug 26, 2015 at 02:17:17PM -0400, Programmingkid wrote:
 
 On Aug 26, 2015, at 2:08 PM, Jeff Cody wrote:
 
 On Wed, Aug 26, 2015 at 01:29:04PM -0400, Programmingkid wrote:
 
 On Aug 26, 2015, at 1:25 PM, Jeff Cody wrote:
 
 On Wed, Aug 26, 2015 at 06:31:57PM +0200, Markus Armbruster wrote:
 Did you drop cc's intentionally?  I put them right back.
 
 Programmingkid programmingk...@gmail.com writes:
 
 On Aug 25, 2015, at 8:38 AM, Markus Armbruster wrote:
 
 You're proposing to revise a qdev design decision, namely the purpose 
 of
 IDs.  This has been discussed before, and IDs remained unchanged.
 Perhaps it's time to revisit this issue.  Cc'ing a few more people.
 
 Relevant prior threads:
 * [PATCH] qdev: Reject duplicate and anti-social device IDs
 http://thread.gmane.org/gmane.comp.emulators.qemu/71230/focus=72272
 * [PATCH 6/6] qdev: Generate IDs for anonymous devices
 http://thread.gmane.org/gmane.comp.emulators.qemu/114853/focus=114858
 * [PATCH] qdev: Assign a default device ID when none is provided.
 http://thread.gmane.org/gmane.comp.emulators.qemu/249702
 * IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from 
 QemuOpt
 http://thread.gmane.org/gmane.comp.emulators.qemu/299945/focus=300381
 
 
 After reading all the threads, I realize why all the attempts to
 accept a device ID patch failed.
 It is because it was assumed everyone would agree on one patch to
 accept. This is
 very unlikely. It would take someone in a leadership position to
 decide which patch
 should be accepted. From one of the threads above, I saw Anthony
 Liguori participate.
 He was in the perfect position to make the choice. The person who is
 in his position now
 is Peter Maydell. Maybe we should just ask him to look at all the
 candidate patches and
 have him pick one to use. 
 
 Yes, when no consensus emerges, problems tend to go unsolved.
 
 Before we appeal to authority to break the deadlock, we should make
 another attempt at finding consensus.
 
 I know that we've entertained the idea of automatically generated IDs
 for block layer objects (that's why I cc'ed some block guys).
 
 Yeah, I was one of the ones that proposed some auto-generated IDs for
 the block layer, specifically for BlockDriverState, making use of the
 node-name field that Benoit introduced a while ago.  Here is my patch
 (not sure if this is the latest version, but it is sufficient for this
 discussion):
 
 http://patchwork.ozlabs.org/patch/355990/
 
 I'm not sure about the requirements needed by device ID names, and
 they may of course differ from what I was thinking for BDS entries.
 
 Here is what I was after with my patch for node-name auto-generation:
 
 * Identifiable as QEMU generated / reserved namespace
 
 * Guaranteed uniqueness
 
 * Non-predictable (don't want users trying to guess / assume
 generated node-names)
 
 My approach was overkill in some ways (24 characters!).  But for
 better or worse, what I had was:
 
  __qemu##IAIYNXXR
  
 QEMU namespace |
| ^
 Increment counter, unique | |
  |
 Random string, to spoil prediction  |
 
 Yikes! 24 characters long. That is a bit much to type. Thank you very much
 for your effort.
 
 IMO, the number of characters to type is pretty low on the list of
 requirements, although it can still be addressed secondary to other
 concerns.
 
 I should have made this in reply to Markus' other email, because the
 important part of this is try and address his point #2:
 
 (from Markus' other email):
 2. The ID must be well-formed.
 
 To have a well-formed ID, we need to have know requirements of the ID
 structure (i.e. the why and what of it all)
 
 I don't know if the three requirements I had above apply to all areas
 in QEMU, but I expect they do, in varying degrees of importance.  The
 length itself can be tweaked.
 
 Talking with John Snow over IRC (added to the CC), one thing he
 suggested was adding in sub-domain spaces; e.g.:
 
 __qemu#bn##IAIYNXXR
 
 Where the 'bn' in this case would be for Block Nodes, etc..
 
 This may make the scheme extensible through QEMU, where auto-generated
 IDs are desired.
 
 (sorry to say, this lengthens things, rather than shortening them!)
 
 We can, of course, make the string shorter - if the random characters
 are just there for spoiling predictability, then 2-3 should be
 sufficient. We could then end up with something like this:
 
 __qemu#bn##XR
 
 The __qemu part of the namespace could be shortened as well, but it
 would be nice if it was easy recognizable as being from QEMU.
 
 If this ID format was supported, I'm thinking being able to copy and paste 
 from
 the monitor is a necessary feature. 
 
 Any way it could be shorted? I was hoping no more than three characters 
 long. 
 
 
 Likely could be shorter, but 

Re: [Qemu-devel] Should we auto-generate IDs?

[Programmingkid]

On Aug 26, 2015, at 6:08 PM, John Snow wrote:

 
 
 On 08/26/2015 05:48 PM, Programmingkid wrote:
 
 On Aug 26, 2015, at 2:45 PM, Peter Maydell wrote:
 
 On 26 August 2015 at 18:16, Programmingkid programmingk...@gmail.com 
 wrote:
 That is assuming they have the time and/or the interest in solving this 
 problem. I
 suppose giving them some time to respond would be reasonable. I'm thinking 
 if
 no consensus has been reached in one weeks time (starting today), we turn 
 to
 Peter Maydell for the answer. Hopefully he will just pick which of the 
 patches he
 likes the best. Judging by how long this problem has been ongoing, someone
 pick the answer is probably the best we can expect.
 
 This is the kind of thing I strongly prefer to leave to the
 relevant subsystem maintainer(s). My opinion is not worth
 a great deal since I don't have a strong familiarity with
 this bit of QEMU.
 
 It looks unreasonable to assume any consensus can be reached over this issue.
 The easy thing to do is to just let each maintainer deal with this problem
 his own way. 
 
 
 What feedback was there that seemed insurmountable? Last I talked to
 Jeff Cody he said there was no overwhelming pushback against his
 patches, just a list of concerns.

Markus Armbruster sent me four different threads each trying to solve this 
problem.
Some of those threads were many years old. The situation is the same then as it
is now. There is no judicator to decide how this problem is to be solved. 
Expecting
all the maintainers to agree on one patch is unrealistic. 

 This doesn't sound like a dead end so much as it sounds like we haven't
 planned the feature enough yet.

The threads did have some really good patches that did seem to solve the 
problem.
I could send you the threads if you haven't read them yet.

 
 Markus:
 I know you really wanted a single ID generating system, but it just isn't 
 going
 to happen. I will make a patch that would only effect USB devices. All other
 devices would be untouched. At least the device_del problem will be solved.
 
 
 I think this is being unnecessarily hasty. We should make sure that an
 auto-generated ID system does not create problems for other areas of
 code before we rush ahead with one to solve a single problem.

I would make sure my patch only affects USB devices. No other systems
would be affected. 

 Let's give the universal approach some more time before we jump to the
 conclusion that it's impossible.

I suppose 5 more years will do ;)

Maybe that's too soon...

Re: [Qemu-devel] [PATCH 1/2] memory: allow zero size for adjust_endianness()

[Jason Wang]

On 08/26/2015 10:51 PM, Greg Kurz wrote:
 On Wed, 26 Aug 2015 15:21:59 +0100
 Peter Maydell peter.mayd...@linaro.org wrote:

 On 26 August 2015 at 11:04, Jason Wang jasow...@redhat.com wrote:
 Wildcard mmio eventfd use zero size, but it will lead abort() since it
 was illegal in adjust_endianness(). Fix this by allowing zero size.

 Cc: Greg Kurz gk...@linux.vnet.ibm.com
 Cc: Paolo Bonzini pbonz...@redhat.com
 Signed-off-by: Jason Wang jasow...@redhat.com
 This seems to me like a bug in the caller. Why would anything
 try to call into the memory subsystem to do a zero-size
 transaction?

 thanks
 -- PMM

 Here's the patch which needs zero-size eventfd:

 http://patchwork.ozlabs.org/patch/509428/

 Cheers.

 --
 Greg


Yes, this is because we want to use wildcard mmio eventfd (which
requires size to be zero) to speed up virtio 1.0 mmio.