Re: [Qemu-devel] [PATCH 00/18] virgl: use a seperate rendering thread

[Gerd Hoffmann]

  Hi,

> The benchmarks are quite encouraging, since I get from +-25% for
> xonotic up to +-100% for glmark. (fwiw, vhost-user-gpu had similar
> results too). Finally, I tried to make it acceptable for upstream.

Hmm, I don't feel like adding too many modes to virgl ...

With vhost-user-gpu giving us simliar benefits I'd prefer to focus on
that, for the additional security benefits (sandboxing the separate
process).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/3] virtio: add virtqueue_rewind()

[Roman Kagan]

On Wed, Sep 07, 2016 at 05:20:48PM +0200, Ladi Prosek wrote:
> From: Stefan Hajnoczi 
> 
> virtqueue_discard() requires a VirtQueueElement but virtio-balloon does
> not migrate its in-use element.  Introduce a new function that is
> similar to virtqueue_discard() but doesn't require a VirtQueueElement.
> 
> This will allow virtio-balloon to access element again after migration
> with the usual proviso that the guest may have modified the vring since
> last time.
> 
> Cc: Michael S. Tsirkin 
> Cc: Roman Kagan 
> Cc: Stefan Hajnoczi 
> Signed-off-by: Ladi Prosek 
> ---
>  hw/virtio/virtio.c | 22 ++
>  include/hw/virtio/virtio.h |  1 +
>  2 files changed, 23 insertions(+)

Reviewed-by: Roman Kagan 

One question though:

> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 74c085c..3de6029 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -272,6 +272,28 @@ void virtqueue_discard(VirtQueue *vq, const 
> VirtQueueElement *elem,
>  virtqueue_unmap_sg(vq, elem, len);
>  }
>  
> +/* virtqueue_rewind:
> + * @vq: The #VirtQueue
> + * @num: Number of elements to push back
> + *
> + * Pretend that elements weren't popped from the virtqueue.  The next
> + * virtqueue_pop() will refetch the oldest element.
> + *
> + * Use virtqueue_discard() instead if you have a VirtQueueElement.
> + *
> + * Returns: true on success, false if @num is greater than the number of in 
> use
> + * elements.
> + */
> +bool virtqueue_rewind(VirtQueue *vq, unsigned int num)
> +{
> +if (num > vq->inuse) {
> +return false;
> +}
> +vq->last_avail_idx -= num;
> +vq->inuse -= num;
> +return true;
> +}
> +

Presumably you envision rewinding by something other than ->inuse.  Do
you have in mind a usecase for that, or is it just a matter of API
symmetry or whatnot?

Thanks,
Roman.

Re: [Qemu-devel] [PATCH 2/3] virtio: add virtqueue_rewind()

[Ladi Prosek]

On Thu, Sep 8, 2016 at 8:44 AM, Roman Kagan  wrote:
> On Wed, Sep 07, 2016 at 05:20:48PM +0200, Ladi Prosek wrote:
>> From: Stefan Hajnoczi 
>>
>> virtqueue_discard() requires a VirtQueueElement but virtio-balloon does
>> not migrate its in-use element.  Introduce a new function that is
>> similar to virtqueue_discard() but doesn't require a VirtQueueElement.
>>
>> This will allow virtio-balloon to access element again after migration
>> with the usual proviso that the guest may have modified the vring since
>> last time.
>>
>> Cc: Michael S. Tsirkin 
>> Cc: Roman Kagan 
>> Cc: Stefan Hajnoczi 
>> Signed-off-by: Ladi Prosek 
>> ---
>>  hw/virtio/virtio.c | 22 ++
>>  include/hw/virtio/virtio.h |  1 +
>>  2 files changed, 23 insertions(+)
>
> Reviewed-by: Roman Kagan 
>
> One question though:
>
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 74c085c..3de6029 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -272,6 +272,28 @@ void virtqueue_discard(VirtQueue *vq, const 
>> VirtQueueElement *elem,
>>  virtqueue_unmap_sg(vq, elem, len);
>>  }
>>
>> +/* virtqueue_rewind:
>> + * @vq: The #VirtQueue
>> + * @num: Number of elements to push back
>> + *
>> + * Pretend that elements weren't popped from the virtqueue.  The next
>> + * virtqueue_pop() will refetch the oldest element.
>> + *
>> + * Use virtqueue_discard() instead if you have a VirtQueueElement.
>> + *
>> + * Returns: true on success, false if @num is greater than the number of in 
>> use
>> + * elements.
>> + */
>> +bool virtqueue_rewind(VirtQueue *vq, unsigned int num)
>> +{
>> +if (num > vq->inuse) {
>> +return false;
>> +}
>> +vq->last_avail_idx -= num;
>> +vq->inuse -= num;
>> +return true;
>> +}
>> +
>
> Presumably you envision rewinding by something other than ->inuse.  Do
> you have in mind a usecase for that, or is it just a matter of API
> symmetry or whatnot?

It may not always be correct to rewind by ->inuse. The elements that
are in use do not necessarily have to be at the end of the available
ring. Example:

elem1 = virtqueue_pop();
elem2 = virtqueue_pop();
elem3 = virtqueue_pop();

virtqueue_push(elem2);

Now inuse == 2 but rewinding by 2 would be incorrect because elem2 has
already been pushed to the used ring.

So it is a dangerous API, which I believe is why Stefan added the num
parameter even though it is currently not needed.

Thanks,
Ladi

Re: [Qemu-devel] [PATCH 0/3] iscsi: add -drive ..., iscsi= parameter

[Daniel P. Berrange]

On Wed, Sep 07, 2016 at 05:23:09PM -0400, Stefan Hajnoczi wrote:
> Add a parameter to associate an iSCSI block driver instance with an -iscsi
> object.  This is more powerful than relying on the implicit target IQN naming
> convention since that only allows one -iscsi object per target.

I'm inclined to say we should *not* do this, as it just serves to perpetuate
usage of the insane -iscsi arg. Instead we could focus on fixing the iSCSI
blockdev for 2.8 so that it directly supports properties for everything that
you can set via -iscsi and deprecate -iscsi entirely. We've got enough time
during 2.8 to get this entirely converted I think.

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] ppc: restrict the use of the rfi instruction

[Thomas Huth]

On 08.09.2016 09:32, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt 
> 
> Power ISA 2.x has deleted the rfi instruction and rfid shoud be used
> instead on cpus following this instruction set or later.
> 
> This will raise an invalid exception when rfi is used on such
> processors: Book3S 64-bit processors.
> 
> Signed-off-by: Benjamin Herrenschmidt 
> Reviewed-by: David Gibson 
> [clg: the required fix in openbios, commit b747b6acc272 ('ppc: use
>   rfid when running under a CPU from the 970 family.'), is now
>   merged in qemu under commit 5cebd885d0d2 ('Update OpenBIOS
>   images to b747b6a built from submodule.') ]
> Signed-off-by: Cédric Le Goater 
> ---
>  target-ppc/translate.c |9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> Index: qemu-dgibson-for-2.8.git/target-ppc/translate.c
> ===
> --- qemu-dgibson-for-2.8.git.orig/target-ppc/translate.c
> +++ qemu-dgibson-for-2.8.git/target-ppc/translate.c
> @@ -3585,10 +3585,13 @@ static void gen_rfi(DisasContext *ctx)
>  #if defined(CONFIG_USER_ONLY)
>  GEN_PRIV;
>  #else
> -/* FIXME: This instruction doesn't exist anymore on 64-bit server
> - * processors compliant with arch 2.x, we should remove it there,
> - * but we need to fix OpenBIOS not to use it on 970 first
> +/* This instruction doesn't exist anymore on 64-bit server
> + * processors compliant with arch 2.x
>   */
> +if (ctx->insns_flags & PPC_SEGMENT_64B) {
> +gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
> +return;
> +}
>  /* Restore CPU state */
>  CHK_SV;
>  gen_update_cfar(ctx, ctx->nip - 4);
> 

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH] virtio: zero vq->inuse in virtio_reset()

[Ladi Prosek]

On Wed, Sep 7, 2016 at 5:54 PM, Stefan Hajnoczi  wrote:
> On Wed, Sep 7, 2016 at 11:51 AM, Stefan Hajnoczi  wrote:
>> vq->inuse must be zeroed upon device reset like most other virtqueue
>> fields.
>>
>> In theory, virtio_reset() just needs assert(vq->inuse == 0) since
>> devices must clean up in-flight requests during reset (requests cannot
>> not be leaked!).
>>
>> In practice, it is difficult to achieve vq->inuse == 0 across reset
>> because balloon, blk, 9p, etc implement various different strategies for
>> cleaning up requests.  Most devices call g_free(elem) directly without
>> telling virtio.c that the VirtQueueElement is cleaned up.  Therefore
>> vq->inuse is not decremented during reset.
>>
>> This patch zeroes vq->inuse and trusts that devices are not leaking
>> VirtQueueElements across reset.
>>
>> I will send a follow-up series that refactors request life-cycle across
>> all devices and converts vq->inuse = 0 into assert(vq->inuse == 0) but
>> this more invasive approach is not appropriate for stable trees.
>>
>> Signed-off-by: Stefan Hajnoczi 
>> ---
>>  hw/virtio/virtio.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 74c085c..e8a13a5 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -822,6 +822,7 @@ void virtio_reset(void *opaque)
>>  vdev->vq[i].signalled_used_valid = false;
>>  vdev->vq[i].notification = true;
>>  vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
>> +vdev->vq[i].inuse = 0;
>>  }
>>  }
>>
>> --
>> 2.7.4
>
> CCing qemu-stable

Reviewed-by: Ladi Prosek 

Re: [Qemu-devel] [PATCH] vhost_net: don't enable vring if backend lack this feature

[Chen Hanxiao]

At 2016-09-03 16:44:47, "Chen Hanxiao"  wrote:
>From: Chen Hanxiao 
>
>If backend(such as dpdk) lack this feature,
>don't assume it and mark it in vring_enable.
>Or we may fail in vhost_net_start,
>then we can't use vhost net.
>This will bring compat issue with old version backend.
>
>Signed-off-by: Chen Hanxiao 
>---

Hi,

Any comments?

Regards,
- Chen

Re: [Qemu-devel] [PATCH] virtio: zero vq->inuse in virtio_reset()

[Cornelia Huck]

On Wed,  7 Sep 2016 11:51:25 -0400
Stefan Hajnoczi  wrote:

> vq->inuse must be zeroed upon device reset like most other virtqueue
> fields.
> 
> In theory, virtio_reset() just needs assert(vq->inuse == 0) since
> devices must clean up in-flight requests during reset (requests cannot
> not be leaked!).
> 
> In practice, it is difficult to achieve vq->inuse == 0 across reset
> because balloon, blk, 9p, etc implement various different strategies for
> cleaning up requests.  Most devices call g_free(elem) directly without
> telling virtio.c that the VirtQueueElement is cleaned up.  Therefore
> vq->inuse is not decremented during reset.
> 
> This patch zeroes vq->inuse and trusts that devices are not leaking
> VirtQueueElements across reset.
> 
> I will send a follow-up series that refactors request life-cycle across
> all devices and converts vq->inuse = 0 into assert(vq->inuse == 0) but
> this more invasive approach is not appropriate for stable trees.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  hw/virtio/virtio.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 74c085c..e8a13a5 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -822,6 +822,7 @@ void virtio_reset(void *opaque)
>  vdev->vq[i].signalled_used_valid = false;
>  vdev->vq[i].notification = true;
>  vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
> +vdev->vq[i].inuse = 0;
>  }
>  }
> 

This looks sane as a minimal change, and survives my smoketests.

Reviewed-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH RFC] docs: add PCIe devices placement guidelines

[Gerd Hoffmann]

  Hi,

> I had understood that the xhci could be a legacy PCI device or a PCI 
> Express device depending on the socket it was plugged into (or was that 
> possibly just someone doing some hand-waving over the fact that 
> obscuring the PCI Express capabilities effectively turns it into a 
> legacy PCI device?).

That is correct, it'll work both ways.

> If that's the case, why do you prefer the default 
> USB controller to be added in a root-port rather than as an integrated 
> device (which is what we do with the group of USB2 controllers, as well 
> as the primary video device)

Trying to mimic real hardware as close as possible.  The ich9 uhci/ehci
controllers are actually integrated chipset devices.  The nec xhci is a
express device in physical hardware.

That is more a personal preference though, there are no strong technical
reasons to do it that way.

cheers,
  Gerd

[Qemu-devel] [PATCH] ppc: restrict the use of the rfi instruction

[Cédric Le Goater]

From: Benjamin Herrenschmidt 

Power ISA 2.x has deleted the rfi instruction and rfid shoud be used
instead on cpus following this instruction set or later.

This will raise an invalid exception when rfi is used on such
processors: Book3S 64-bit processors.

Signed-off-by: Benjamin Herrenschmidt 
Reviewed-by: David Gibson 
[clg: the required fix in openbios, commit b747b6acc272 ('ppc: use
  rfid when running under a CPU from the 970 family.'), is now
  merged in qemu under commit 5cebd885d0d2 ('Update OpenBIOS
  images to b747b6a built from submodule.') ]
Signed-off-by: Cédric Le Goater 
---
 target-ppc/translate.c |9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Index: qemu-dgibson-for-2.8.git/target-ppc/translate.c
===
--- qemu-dgibson-for-2.8.git.orig/target-ppc/translate.c
+++ qemu-dgibson-for-2.8.git/target-ppc/translate.c
@@ -3585,10 +3585,13 @@ static void gen_rfi(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
 GEN_PRIV;
 #else
-/* FIXME: This instruction doesn't exist anymore on 64-bit server
- * processors compliant with arch 2.x, we should remove it there,
- * but we need to fix OpenBIOS not to use it on 970 first
+/* This instruction doesn't exist anymore on 64-bit server
+ * processors compliant with arch 2.x
  */
+if (ctx->insns_flags & PPC_SEGMENT_64B) {
+gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+return;
+}
 /* Restore CPU state */
 CHK_SV;
 gen_update_cfar(ctx, ctx->nip - 4);

Re: [Qemu-devel] [RFC v2 0/4] adding mdev bus and vfio support

[Jike Song]

On 09/08/2016 12:56 AM, Alex Williamson wrote:
> On Wed, 07 Sep 2016 14:42:58 +0800
> Jike Song  wrote:
> 
>> On 09/07/2016 11:38 AM, Neo Jia wrote:
>>> On Wed, Sep 07, 2016 at 10:22:26AM +0800, Jike Song wrote:  
 On 09/02/2016 11:03 PM, Alex Williamson wrote:  
> On Fri,  2 Sep 2016 16:16:08 +0800
> Jike Song  wrote:
>  
>> This patchset is based on NVidia's "Add Mediated device support" series, 
>> version 6:
>>
>>  http://www.spinics.net/lists/kvm/msg136472.html  
>
>
> Hi Jike,
>
> I'm thrilled by your active participation here, but I'm confused which
> versions I should be reviewing and where the primary development is
> going.  Kirti sent v7 a week ago, so I would have expected a revision
> based on that rather than a re-write based on v6 plus incorporation of a
> few of Kirti's patches directly.  

 Hi Alex,

 [Sorry! replied this on Monday but it was silently dropped by the our 
 firewall]



 The v1 of this patchset was send as incremental ones, basing on Nvidia's 
 v6, to
 demonstrate how is it possible and beneficial to:

1, Introduce an independent device between physical and mdev;
2, Simplify vfio-mdev and make it the most flexible for vendor drivers;

 Unfortunately neither was understood or adopted in v7:

http://www.spinics.net/lists/kvm/msg137081.html

 So here came the v2, as a standalone series, to give a whole and straight
 demonstration. The reason of still basing on v6:

- Addressed all v6 comments (except the iommu part);
- There is no comments yet for v7 (except the sysfs ones);


  
> I liked the last version of these
> changes a lot, but we need to figure out how to combine development
> because we do not have infinite cycles for review available :-\  Thanks!  

 Fully understand.

 Here is the dilemma: v6 is an obsolete version to work upon, v7 is still 
 not
 at the direction we prefer.   
>>>
>>> Hi Jike,
>>>
>>> I wish I could meet you in person in KVM forum couple weeks ago so we can 
>>> have a
>>> better discussion.  
>>
>> I wish I could have that opportunity, too!
>>
>>> We are trying our best to accommodate almost all requirements / comments 
>>> from 
>>> use cases and code reviews while keeping little (or none) architectural 
>>> changes
>>> between revisions.  
>>
>> Yes I saw that, there was little architectural change from v6 to v7,
>> that's what I will argue for :)
>>
 We would be highly glad and thankful if Neo/Kirti
 would adopt the code in their next version, which will certainly form a
 more simple and consolidated base for future co-development; otherwise
 and we could at least discuss the concerns, in case of any.
  
>>>
>>> As I have said in my previous response to you, if you have any questions 
>>> about
>>> adopting the framework that we have developed, you are very welcome to
>>> comment/speak out on the code review thread like others. And if it is 
>>> reasonable
>>> request and won't break other vendors' use case, we will adopt it (one 
>>> example
>>> is the online file and removing the mdev pci dependency).
>>>   
>>
>> Not limited to having questions about adoption, right? :)
>>
>> We do think the framework itself had too much unnecessary logic and
>> imposed limitations to vendor drivers, also it's clearly possible to be
>> simplified.
>>
>>> Just some update for you regarding the v7 patches, currently we are very
>>> actively trying to lock down the sysfs and management interfaces discussion.
>>>
>>> So, if you would like to make the upstream happen sooner, please join us in 
>>> the
>>> v7 and following patch discussion instead of rewriting them.
>>>   
>>
>> So as you said, I would comment on the v7 series to propose both 
>> architectural
>> and implementation changes, hoping this will help more.
> 
> Please do, I definitely like where you're heading and I want to see
> Kirti and Neo include your ideas.  The challenge is to try to focus our
> efforts into a single development stream.  Please continue to comment
> and even submit patches, but let's consider NVIDIA's latest patches to
> be the main development stream and request changes against that.  As
> you would do upstream, propose changes in small increments where we can
> evaluate each step.  It's very difficult for Neo and Kirti to
> incorporate bulk rewrites.  Thanks,
> 

Thanks - We'll try our best to meet that!


--
Thanks,
Jike

Re: [Qemu-devel] [PATCH v3] Move max-bandwidth and downtime-limit into migrate_set_parameter for both hmp and qmp

[Ashijeet Acharya]

On Thu, Sep 8, 2016 at 3:33 AM, Eric Blake  wrote:
> On 09/06/2016 08:39 AM, Ashijeet Acharya wrote:
>> Mark old-commands for speed and downtime as deprecated.
>> Move max-bandwidth and downtime-limit into migrate-set-parameters for
>> setting maximum migration speed and expected downtime limit parameters
>> respectively.
>> Change downtime units to milliseconds (only for new-command) and update
>> the query part in both hmp and qmp qemu control interfaces.
>>
>> Signed-off-by: Ashijeet Acharya 
>> ---
>
>> +++ b/migration/migration.c
>> @@ -44,6 +44,9 @@
>>  #define BUFFER_DELAY 100
>>  #define XFER_LIMIT_RATIO (1000 / BUFFER_DELAY)
>>
>> +/* Time in nanoseconds we are allowed to stop the source for to send last 
>> part */
>
> Long line.  Also, a grammar issue:
>
> s/source for to send/source, for sending the/

Okay, I will change it.

>
>> @@ -832,6 +848,21 @@ void qmp_migrate_set_parameters(bool has_compress_level,
>>  g_free(s->parameters.tls_hostname);
>>  s->parameters.tls_hostname = g_strdup(tls_hostname);
>>  }
>> +if (has_max_bandwidth) {
>> +s->parameters.max_bandwidth = max_bandwidth;
>> +if (s->to_dst_file) {
>> +qemu_file_set_rate_limit(s->to_dst_file,
>> +s->parameters.max_bandwidth / 
>> XFER_LIMIT_RATIO);
>> +}
>> +}
>> +if (has_downtime_limit) {
>> +downtime_limit *= 100; /* convert to nanoseconds */
>
> Are you sure this won't overflow?

Should I explicitly cast it? Like;
downtime_limit *= (int64_t)100;
This should work right?

>> +void qmp_migrate_set_speed(int64_t valuebw, Error **errp)
>> +{
>> +bool has_compress_level = false;
>> +bool has_compress_threads = false;
>> +bool has_decompress_threads = false;
>> +bool has_cpu_throttle_initial = false;
>> +bool has_cpu_throttle_increment = false;
>> +bool has_tls_creds = false;
>> +bool has_tls_hostname = false;
>> +bool has_max_bandwidth = true;
>> +bool has_downtime_limit = false;
>> +const char *valuestr = NULL;
>> +long valueint = 0;
>> +Error *err = NULL;
>> +
>> +qmp_migrate_set_parameters(has_compress_level, valueint,
>> +   has_compress_threads, valueint,
>
> Ugg. This looks gross.  No need to name a bunch of variables set to
> false, when you can instead use QAPI's boxed conventions to pass a
> pointer to a struct, and rely on 0-initialization of the struct to leave
> all the parameters that you don't care about unmentioned.
>

I know. But I was not sure if I can do something else.
'hmp.c' does it in a similar way. I am not aware of the method you described.
Maybe you can direct me to some place where I can code by example.
(Sorry! I am a newbie)

>> +   has_decompress_threads, valueint,
>> +   has_cpu_throttle_initial, valueint,
>> +   has_cpu_throttle_increment, valueint,
>> +   has_tls_creds, valuestr,
>> +   has_tls_hostname, valuestr,
>> +   has_max_bandwidth, valuebw,
>> +   has_downtime_limit, valueint,
>> +   );
>> +
>> +}
>> +
>> +void qmp_migrate_set_downtime(double valuedowntime, Error **errp)
>> +{
>> +bool has_compress_level = false;
>> +bool has_compress_threads = false;
>> +bool has_decompress_threads = false;
>> +bool has_cpu_throttle_initial = false;
>> +bool has_cpu_throttle_increment = false;
>> +bool has_tls_creds = false;
>> +bool has_tls_hostname = false;
>> +bool has_max_bandwidth = false;
>> +bool has_downtime_limit = true;
>
> Again, gross.
>
>> +const char *valuestr = NULL;
>> +long valueint = 0;
>> +int64_t valuebw = 0;
>> +valuedowntime *= 1000; /* Convert to milliseconds */
>> +valuedowntime = MAX(0, MIN(INT64_MAX, valuedowntime));
>> +valuedowntime = (int64_t)valuedowntime;
>
> Useless statement; the cast would already implicitly happen when passing
> it to the function call.

Okay, I will remove that.

>
>> +Error *err = NULL;
>> +
>> +qmp_migrate_set_parameters(has_compress_level, valueint,
>> +   has_compress_threads, valueint,
>> +   has_decompress_threads, valueint,
>> +   has_cpu_throttle_initial, valueint,
>> +   has_cpu_throttle_increment, valueint,
>> +   has_tls_creds, valuestr,
>> +   has_tls_hostname, valuestr,
>> +   has_max_bandwidth, valuebw,
>> +   has_downtime_limit, valuedowntime,
>> +   );
>>  }
>>
>>  bool migrate_postcopy_ram(void)
>> @@ -1858,7 +1922,7 @@ void migrate_fd_connect(MigrationState *s)
>>
>> 

Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic

[Laurent Vivier]

On 08/09/2016 04:04, David Gibson wrote:
> On Tue, Sep 06, 2016 at 03:17:56PM +0200, Laurent Vivier wrote:
>> And add support for ppc64.
>>
>> Signed-off-by: Laurent Vivier 
> 
> Some of my coments may be obsoleted by the discussion with Greg.
> 
>> ---
>> v2:
>> - remove useless parenthesis, inline
>>
>>  tests/Makefile.include  |  3 ++-
>>  tests/libqos/libqos.h   |  2 +-
>>  tests/libqos/malloc-ppc64.c | 38 ++
>>  tests/libqos/malloc-ppc64.h | 17 +
>>  tests/libqos/malloc.c   | 42 ++
>>  tests/libqos/malloc.h   |  3 +++
>>  6 files changed, 103 insertions(+), 2 deletions(-)
>>  create mode 100644 tests/libqos/malloc-ppc64.c
>>  create mode 100644 tests/libqos/malloc-ppc64.h
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 14be491..a286848 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -557,8 +557,9 @@ tests/test-crypto-block$(EXESUF): 
>> tests/test-crypto-block.o $(test-crypto-obj-y)
>>  
>>  libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o 
>> tests/libqos/malloc.o
>>  libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
>> +libqos-obj-y += tests/libqos/malloc-ppc64.o tests/libqos/malloc-pc.o
>>  libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
>> -libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
>> +libqos-pc-obj-y += tests/libqos/libqos-pc.o
>>  libqos-pc-obj-y += tests/libqos/ahci.o
>>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
>>  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
>> diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
>> index 604980d..7b71607 100644
>> --- a/tests/libqos/libqos.h
>> +++ b/tests/libqos/libqos.h
>> @@ -3,7 +3,7 @@
>>  
>>  #include "libqtest.h"
>>  #include "libqos/pci.h"
>> -#include "libqos/malloc-pc.h"
>> +#include "libqos/malloc.h"
>>  
>>  typedef struct QOSOps {
>>  QGuestAllocator *(*init_allocator)(QAllocOpts);
>> diff --git a/tests/libqos/malloc-ppc64.c b/tests/libqos/malloc-ppc64.c
>> new file mode 100644
>> index 000..1b31e33
>> --- /dev/null
>> +++ b/tests/libqos/malloc-ppc64.c
>> @@ -0,0 +1,38 @@
>> +/*
>> + * libqos malloc support for PPC64
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "libqos/malloc-ppc64.h"
>> +
>> +#include "qemu-common.h"
>> +
>> +#define PAGE_SIZE 4096
>> +
>> +/* Memory must be a multiple of 256 MB,
>> + * so we have at least 256MB
>> + */
>> +#define PPC64_MIN_SIZE 0x1000
> 
> So, this is really a "pseries" machine type fact rather than a ppc64
> fact.  Is there a way to make this dependent on machine type rather
> than target architecture?  Naming this based on machine type would
> seem to better match the "pc" things these are based on ("pc" rather
> than "i386" or "x86_64").

I've changed all my "ppc64" by "spapr". Is "pseries" better?

>> +
>> +void ppc64_alloc_uninit(QGuestAllocator *allocator)
>> +{
>> +alloc_uninit(allocator);
>> +}
>> +
>> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags)
>> +{
>> +QGuestAllocator *s;
>> +
>> +s = alloc_init_flags(flags, 1 << 20, PPC64_MIN_SIZE);
>> +alloc_set_page_size(s, PAGE_SIZE);
>> +
>> +return s;
>> +}
>> +
>> +QGuestAllocator *ppc64_alloc_init(void)
>> +{
>> +return ppc64_alloc_init_flags(ALLOC_NO_FLAGS);
>> +}
>> diff --git a/tests/libqos/malloc-ppc64.h b/tests/libqos/malloc-ppc64.h
>> new file mode 100644
>> index 000..c2b2dff
>> --- /dev/null
>> +++ b/tests/libqos/malloc-ppc64.h
>> @@ -0,0 +1,17 @@
>> +/*
>> + * libqos malloc support for PPC64
>> + *
>> + * 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 LIBQOS_MALLOC_PPC64_H
>> +#define LIBQOS_MALLOC_PPC64_H
>> +
>> +#include "libqos/malloc.h"
>> +
>> +QGuestAllocator *ppc64_alloc_init(void);
>> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags);
>> +void ppc64_alloc_uninit(QGuestAllocator *allocator);
>> +
>> +#endif
>> diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
>> index b8eff5f..6a02345 100644
>> --- a/tests/libqos/malloc.c
>> +++ b/tests/libqos/malloc.c
>> @@ -12,6 +12,9 @@
>>  
>>  #include "qemu/osdep.h"
>>  #include "libqos/malloc.h"
>> +#include "libqos/malloc-pc.h"
>> +#include "libqos/malloc-ppc64.h"
>> +#include "libqtest.h"
>>  #include "qemu-common.h"
>>  #include "qemu/host-utils.h"
>>  
>> @@ -375,3 +378,42 @@ void migrate_allocator(QGuestAllocator *src,
>>  QTAILQ_INSERT_HEAD(src->free, node, MLIST_ENTNAME);
>>  return;
>>  }
>> +
>> +QGuestAllocator *machine_alloc_init(void)
>> +{
>> +const char *arch = qtest_get_arch();
> 
> Maybe we need to add a qtest_get_machine_type().

I'm working on that...

>> +if 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX()

[Greg Kurz]

On Tue,  6 Sep 2016 15:17:55 +0200
Laurent Vivier  wrote:

> Signed-off-by: Laurent Vivier 
> ---

FWIW,

Reviewed-by: Greg Kurz 

> v4:
> - add this patch in the series to change all strtoXX() in qtest.c
> 
>  qtest.c | 49 ++---
>  1 file changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/qtest.c b/qtest.c
> index da4826c..4c94708 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -27,6 +27,7 @@
>  #include "qemu/config-file.h"
>  #include "qemu/option.h"
>  #include "qemu/error-report.h"
> +#include "qemu/cutils.h"
>  
>  #define MAX_IRQ 256
>  
> @@ -324,12 +325,13 @@ static void qtest_process_command(CharDriverState *chr, 
> gchar **words)
>  } else if (strcmp(words[0], "outb") == 0 ||
> strcmp(words[0], "outw") == 0 ||
> strcmp(words[0], "outl") == 0) {
> -uint16_t addr;
> -uint32_t value;
> +unsigned long addr;
> +unsigned long value;
>  
>  g_assert(words[1] && words[2]);
> -addr = strtoul(words[1], NULL, 0);
> -value = strtoul(words[2], NULL, 0);
> +g_assert(qemu_strtoul(words[1], NULL, 0, ) == 0);
> +g_assert(qemu_strtoul(words[2], NULL, 0, ) == 0);
> +g_assert(addr <= 0x);
>  
>  if (words[0][3] == 'b') {
>  cpu_outb(addr, value);
> @@ -343,11 +345,12 @@ static void qtest_process_command(CharDriverState *chr, 
> gchar **words)
>  } else if (strcmp(words[0], "inb") == 0 ||
>  strcmp(words[0], "inw") == 0 ||
>  strcmp(words[0], "inl") == 0) {
> -uint16_t addr;
> +unsigned long addr;
>  uint32_t value = -1U;
>  
>  g_assert(words[1]);
> -addr = strtoul(words[1], NULL, 0);
> +g_assert(qemu_strtoul(words[1], NULL, 0, ) == 0);
> +g_assert(addr <= 0x);
>  
>  if (words[0][2] == 'b') {
>  value = cpu_inb(addr);
> @@ -366,8 +369,8 @@ static void qtest_process_command(CharDriverState *chr, 
> gchar **words)
>  uint64_t value;
>  
>  g_assert(words[1] && words[2]);
> -addr = strtoull(words[1], NULL, 0);
> -value = strtoull(words[2], NULL, 0);
> +g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
> +g_assert(qemu_strtoull(words[2], NULL, 0, ) == 0);
>  
>  if (words[0][5] == 'b') {
>  uint8_t data = value;
> @@ -395,7 +398,7 @@ static void qtest_process_command(CharDriverState *chr, 
> gchar **words)
>  uint64_t value = UINT64_C(-1);
>  
>  g_assert(words[1]);
> -addr = strtoull(words[1], NULL, 0);
> +g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
>  
>  if (words[0][4] == 'b') {
>  uint8_t data;
> @@ -421,8 +424,8 @@ static void qtest_process_command(CharDriverState *chr, 
> gchar **words)
>  char *enc;
>  
>  g_assert(words[1] && words[2]);
> -addr = strtoull(words[1], NULL, 0);
> -len = strtoull(words[2], NULL, 0);
> +g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
> +g_assert(qemu_strtoull(words[2], NULL, 0, ) == 0);
>  
>  data = g_malloc(len);
>  cpu_physical_memory_read(addr, data, len);
> @@ -443,8 +446,8 @@ static void qtest_process_command(CharDriverState *chr, 
> gchar **words)
>  gchar *b64_data;
>  
>  g_assert(words[1] && words[2]);
> -addr = strtoull(words[1], NULL, 0);
> -len = strtoull(words[2], NULL, 0);
> +g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
> +g_assert(qemu_strtoull(words[2], NULL, 0, ) == 0);
>  
>  data = g_malloc(len);
>  cpu_physical_memory_read(addr, data, len);
> @@ -460,8 +463,8 @@ static void qtest_process_command(CharDriverState *chr, 
> gchar **words)
>  size_t data_len;
>  
>  g_assert(words[1] && words[2] && words[3]);
> -addr = strtoull(words[1], NULL, 0);
> -len = strtoull(words[2], NULL, 0);
> +g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
> +g_assert(qemu_strtoull(words[2], NULL, 0, ) == 0);
>  
>  data_len = strlen(words[3]);
>  if (data_len < 3) {
> @@ -486,12 +489,12 @@ static void qtest_process_command(CharDriverState *chr, 
> gchar **words)
>  } else if (strcmp(words[0], "memset") == 0) {
>  uint64_t addr, len;
>  uint8_t *data;
> -uint8_t pattern;
> +unsigned long pattern;
>  
>  g_assert(words[1] && words[2] && words[3]);
> -addr = strtoull(words[1], NULL, 0);
> -len = strtoull(words[2], NULL, 0);
> -pattern = strtoull(words[3], NULL, 0);
> +g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
> +g_assert(qemu_strtoull(words[2], NULL, 0, ) == 0);
> +g_assert(qemu_strtoul(words[3], NULL, 0, ) == 0);
>  
>  data = g_malloc(len);
>  memset(data, pattern, len);
> @@ -507,8 +510,8 @@ static void 

Re: [Qemu-devel] [PATCH] 9pfs: add support for IO limits to 9p-local driver

[Greg Kurz]

On Wed, 7 Sep 2016 16:36:12 -0500
Eric Blake  wrote:

> On 09/07/2016 07:39 AM, Pradeep wrote:
> > Uses throttling APIs to limit I/O bandwidth and number of operations on the 
> > devices which use 9p-local driver.
> > 
> > Signed-off-by: Pradeep 
> >  fsdev/file-op-9p.h  |   3 +
> >  fsdev/qemu-fsdev-opts.c |  52 +
> >  hw/9pfs/9p-local.c  |  18 -
> >  hw/9pfs/9p-throttle.c   | 199 
> > 
> >  hw/9pfs/9p-throttle.h   |  55 +
> >  hw/9pfs/9p.c|   7 ++
> >  hw/9pfs/Makefile.objs   |   1 +
> >  7 files changed, 333 insertions(+), 2 deletions(-)
> >  create mode 100644 hw/9pfs/9p-throttle.c
> >  create mode 100644 hw/9pfs/9p-throttle.h
> > 
> > Hi All,
> > 
> > This adds the support for the 9p-local driver.
> > For now this functionality can be enabled only through qemu cli options.
> > QMP interface and support to other drivers need further extensions.
> > To make it simple for other drivers, the throttle code has been put in
> > separate files.  
> 
> CLI-only extensions mean that we can't use it during hotplug.  Are you
> sure that blockdev-add won't automatically enable these options, given
> that it tries to convert the dictionary passed through QMP back into the
> QemuOpts form that matches command line parsing?  Also, aren't these

9p-local is fsdev, not blockdev: it has its own QemuOpts and cannot be
passed to blockdev-add (and BTW, there isn't any fsdev-add either).

> limits very similar to what is already available in the throttle driver?

Yes, IIUC this patch tries to implement similar throttling concepts to the
read/write operations in fsdev.

More details on the motivation here:

https://lists.gnu.org/archive/html/qemu-discuss/2016-04/msg00064.html

> Why are we repeating them at the 9p driver, instead of putting a
> throttle group BDS in the mix?
> 

Not sure if it is possible to mix fsdev and blockdev... to be honest, I
really don't know how one would achieve that :)

> > +++ b/fsdev/qemu-fsdev-opts.c
> > @@ -37,6 +37,58 @@ static QemuOptsList qemu_fsdev_opts = {
> >  }, {
> >  .name = "sock_fd",
> >  .type = QEMU_OPT_NUMBER,
> > +}, {
> > +.name = "bps",
> > +.type = QEMU_OPT_NUMBER,
> > +.help = "total bytes burst",
> > +}, {
> > +.name = "bps_rd",
> > +.type = QEMU_OPT_NUMBER,
> > +.help = "total bytes read burst",
> > +}, {  
> 
> 
> At the very least, if we must have multiple throttle implementations,
> can they at least share code so that we don't forget to update one when
> adding another option to the other?
> 

This is experimental work. If it turns out that fsdev and blockdev
throttling are close enough, I agree that they should probably share
code. But at this point, I guess they should remain distinct.

Cheers.

--
Greg


pgpYDWu88aig9.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] virtio-vga: adapt to page-per-vq=off

[no-reply]

Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Subject: [Qemu-devel] [PATCH] virtio-vga: adapt to page-per-vq=off
Type: series
Message-id: 1473319012-27560-1-git-send-email-kra...@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
7bf2df3 virtio-vga: adapt to page-per-vq=off

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY RUNNER
  RUN test-quick in centos6
No C++ compiler available; disabling C++ specific optional code
Install prefix/tmp/qemu-test/src/tests/docker/install
BIOS directory/tmp/qemu-test/src/tests/docker/install/share/qemu
binary directory  /tmp/qemu-test/src/tests/docker/install/bin
library directory /tmp/qemu-test/src/tests/docker/install/lib
module directory  /tmp/qemu-test/src/tests/docker/install/lib/qemu
libexec directory /tmp/qemu-test/src/tests/docker/install/libexec
include directory /tmp/qemu-test/src/tests/docker/install/include
config directory  /tmp/qemu-test/src/tests/docker/install/etc
local state directory   /tmp/qemu-test/src/tests/docker/install/var
Manual directory  /tmp/qemu-test/src/tests/docker/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread 
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -g 
QEMU_CFLAGS   -I/usr/include/pixman-1-fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno 
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   yes
vde support   no
netmap supportno
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
RDMA support  no
TCG interpreter   no
fdt support   yes
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
uuid support  no
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
Trace backendslog
spice support no 
rbd support   no
xfsctl supportno
smartcard support no
libusbno
usb net redir no
OpenGL supportno
OpenGL dmabufsno
libiscsi support  no
libnfs supportno
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine poolyes
GlusterFS support no
Archipelago support no
gcov  gcov
gcov enabled  no
TPM support   yes
libssh2 support   no
TPM passthrough   yes
QOM debugging yes
vhdx  no
lzo support   no
snappy supportno
bzip2 support no
NUMA host support no
tcmalloc support  no
jemalloc support  no
avx2 optimization no
  GEN   x86_64-softmmu/config-devices.mak.tmp
  GEN   aarch64-softmmu/config-devices.mak.tmp
  GEN   config-host.h
  GEN   qemu-options.def
  GEN   qmp-commands.h
  GEN   qapi-types.h
  GEN   qapi-visit.h
  GEN   qapi-event.h
  GEN   x86_64-softmmu/config-devices.mak
  GEN   aarch64-softmmu/config-devices.mak
  GEN   qmp-introspect.h
  GEN   tests/test-qapi-types.h
  GEN   tests/test-qapi-visit.h
  GEN   

Re: [Qemu-devel] [PATCH RFC] docs: add PCIe devices placement guidelines

[Gerd Hoffmann]

  Hi,

> > Good point, maybe libvirt can avoid adding switches unless the user
> > explicitly
> > asked for them. I checked and it a actually works fine in QEMU.

> So, *is* there any downside to doing this?

I don't think so.

The only issue I can think of when it comes to multifunction is hotplug,
because hotplug works at slot level in pci so you can't hotplug single
functions.

But as you can't hotplug the root ports in the first place this is
nothing we have to worry about in this specific case.

cheers,
  Gerd

Re: [Qemu-devel] KVM Forum 2016 videos now online

[Christian Borntraeger]

On 09/08/2016 05:28 AM, Pranith Kumar wrote:
> FYI,
> 
> The KVM Forum 2016 videos are now online on youtube. You can find them here:
> 
> https://www.youtube.com/playlist?list=PLW3ep1uCIRfzQoZ0SlniYE8nz1ZRobjH7

The videos are not complete yet, some more still to come as far as I can see.

Re: [Qemu-devel] [PATCH 1/2] SDL2: only show consoles by shortcuts and not hide.

[Gerd Hoffmann]

On Mi, 2016-09-07 at 15:24 +0300, Andrei Karas wrote:
> >Среда,  7 сентября 2016, 14:27 +03:00 от Gerd Hoffmann :
> >
> >On Di, 2016-08-23 at 23:36 +0300, Andrei Karas wrote:
> >> This fix issue with stuck keys in SDL2 if press one of shortcuts
> >> for show/hide consoles.
> >
> >More detailed description please.  How exactly do you end up with stuck
> >keys?
> For example i using ctrl_grab=on option.
> Without patch if press rctrl+2 cosole window can be drawed and hidden at same 
> time many times.
> Console drawed or hidden depend only how long keys was pressed.

Ah, so you hold down the hotkey and key autorepeat will show/hide the
window?

> >So, the hotkey works only in case the window is hidden, so you can't
> >hide windows with the shortcut any more.  Is that intentional?  Why?
> Because previous explanation i found only solution is limit this keys to
> always show windows.

You can check ev->key.repeat to detect whenever the keypress is a real
one or autorepeat event.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 00/18] virgl: use a seperate rendering thread

[Marc-André Lureau]

Hi

- Original Message -
> Hi,
> 
> > The benchmarks are quite encouraging, since I get from +-25% for
> > xonotic up to +-100% for glmark. (fwiw, vhost-user-gpu had similar
> > results too). Finally, I tried to make it acceptable for upstream.
> 
> Hmm, I don't feel like adding too many modes to virgl ...
> 
> With vhost-user-gpu giving us simliar benefits I'd prefer to focus on
> that, for the additional security benefits (sandboxing the separate
> process).

Makes sense, I wanted to have a fair comparison between the two. vhost-user-gpu 
will probably take longer to happen, since it changes the way qemu must be 
managed. I also expect a lag in vhost-user whenever virtio ring/dataplane gets 
optimization in qemu, that will need to be adapted for vhost-user.
 So the benefit of having a virgl thread in qemu is that it is less complicated 
than vhost-user-gpu, it brings immediate performance boost, and it serves as a 
comparison point. I can keep it in a branch, but it would be nice to consider 
this as an "experimental" option too. 

Re: [Qemu-devel] Help: Does Qemu support virtio-pci for net-device and disk device?

[Kevin Zhao]

Hi  All,
   As discussed before, upstream are working about PCIE instead of PCI in
AArch64.
   Thanks for your efforts about this on AArch64 :-)
   If it convenient, could you tell me when we plan  to finish this task ?
and which qemu version will support this functions in the future?
   Big Thanks~

Best Regards,
Kevin

On 24 August 2016 at 09:52, Kevin Zhao  wrote:

> Great ~ Thanks for your valuable information~
> I will try with the xml and any update I will post here.
>
> On 18 August 2016 at 21:51, Andrea Bolognani  wrote:
>
>> On Thu, 2016-08-18 at 20:43 +0800, Kevin Zhao wrote:
>> > What's the minimum version of  Qemu that support virito-1.0?
>> > Does Qemu 2.6 works?
>>
>> 2.6 definitely has virtio 1.0 support, however libvirt does
>> not yet allow you to control whether a device uses 0.9, 1.0
>> or both. The default for 2.6 should be both IIRC.
>>
>> > Now I will manually add the slots and bus to pcie. Because
>> > I am not familiar with it,  if it convenient, could you give
>> > me an available xml file which PCIE disk and PCIE
>> > net device can work for machine virt ?
>>
>> The XML you're looking for is at the end of this message.
>>
>> Note that a Fedora 24 guest configured this way will not
>> boot at all if the machine type is virt-2.6; on the other
>> hand, an identically-configured RHEL 7.3 guest will boot
>> even with virt-2.6, but both the disk and the network
>> adapter will be legacy PCI instead of PCIe.
>>
>>
>> 
>>   abologna-f24
>>   f6d0428b-a034-4c4e-8ef2-f12f6aa9cab0
>>   2097152
>>   2097152
>>   4
>>   
>> hvm
>> /usr/share/AAVMF
>> /AAVMF_CODE.fd
>> /var/lib/libvirt/qemu/nvram/abologna-f24_VARS.fd
>> 
>>   
>>   
>> 
>>   
>>   
>>   
>>   destroy
>>   restart
>>   restart
>>   
>> /usr/libexec/abologna-qemu-kvm
>> 
>>   
>>   
>>   
>>   > function='0x0'/>
>> 
>> 
>> 
>>   
>>   
>>   > function='0x0'/>
>> 
>> 
>>   
>>   
>>   > function='0x0'/>
>> 
>> 
>>   
>> 
>> 
>>   
>>   
>>   
>>   
>>   > function='0x0'/>
>> 
>> 
>>   
>> 
>> 
>>   
>> 
>> 
>>   
>>   
>>   
>> 
>>   
>> 
>>
>> --
>> Andrea Bolognani / Red Hat / Virtualization
>>
>
>

[Qemu-devel] [PATCH] target-mips: generate fences

[Leon Alrae]

Make use of memory barrier TCG opcode in MIPS front end.

Signed-off-by: Leon Alrae 
---
This patch complements the following series:
https://lists.nongnu.org/archive/html/qemu-devel/2016-07/msg03283.html
---
 target-mips/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index c212e4f..f4513bf 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -13384,7 +13384,7 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 case 0x2d:
 switch (minor) {
 case SYNC:
-/* NOP */
+tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
 break;
 case SYSCALL:
 generate_exception_end(ctx, EXCP_SYSCALL);
@@ -17201,7 +17201,7 @@ static void decode_opc_special(CPUMIPSState *env, 
DisasContext *ctx)
 break;
 case OPC_SYNC:
 check_insn(ctx, ISA_MIPS2);
-/* Treat as NOP. */
+tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
 break;
 
 #if defined(TARGET_MIPS64)
-- 
2.7.4

Re: [Qemu-devel] [virtio-dev][RFC v3] virtio-sdm: new device specification

[Christian Pinto]

Hello Edgar,


On 07/09/2016 18:02, Edgar E. Iglesias wrote:

On Wed, Sep 07, 2016 at 05:39:08PM +0200, Christian Pinto wrote:

On 07/09/2016 09:51, Edgar E. Iglesias wrote:

On Wed, Sep 07, 2016 at 09:24:39AM +0200, Christian Pinto wrote:

Hello Edgar,

thanks for your comments.


Thanks for the clarification, I have a few follow-up questions/comments.

On 06/09/2016 23:43, Edgar E. Iglesias wrote:

Hi,

Sorry for the delay. I have a few questions.

I don't fully understand the purpose of this. Could you elaborate a little on 
that?
You need a real IPI signal to drive this I guess, so it's a little bit of a
chicken and egg problem.

Usually in AMP-like systems CPUs signal each other with IPIs by exploiting
a dedicated hardware peripheral, like a hardware mailbox. So I see the
concept of the SDM (master, slave and communication channels) as the
hardware device to actually implement and deliver the IPIs.
In this case the SDM is designed in a way to be able to deliver multiple
signals (e.g., BOOT, RESET) and not just a single interrupt.


A useful feature I can see is multiplexing a single underlying IPI (driving
an sdm channel) into multiple "virtual" IPI signals. Is that the main
use-case?

In the SDM multiple slaves can connect to the same SDM channel (e.g.,
socket)
and the master uses the channel to deliver the signal (e.g., IRQ) to the
destination slave device. The other way around is also possible, slaves
signaling
the master.
I guess this is the multiplexing features you were talking about.

Right, but for this to be more useful, I think you should pass on some
payload with the IRQ signal. That will allow this channel to be used
to enable notification mechanisms for many other channels over a single
HW irq (e.g the payload would be like the irq vector or irq line

All signals carry a 64 bits payload that for the time being is ignored
for the IRQ signal. I now see your point, thanks for the clarification.
The payload could definitely be used as an IRQ vector, so that SW
can register to a specific IRQ line of the SDM HW interrupt.

Thanks Christian,

Perhaps we could update the spec and elaborate a little on this and
change the payload from ignored to provide and notifier/irq index?



Sure. I will improve this part in the next release.


Another question:
For the reverese path, I don't see how it will be reasonably implemented
in the remote cpus various sw stacks sharing a single virtio queue.
It would seem like you would need a reverese channel per remoteproc
to avoid locking (very messy in AMP systems) to safely update
the reverese gh_vq queue?

Or how do you see multiple AMP systems safely updating a single virtio
queue in parallel?

The AMP systems are not all interacting with the same virtio queues.
As described in the specification the device can be a master or a slave.
The master processor will interact with a master SDM instance,
while remote processors with slave SDM instances.
Master/slave instances should be seen as multiple ports of the same
SDM.
Master and slave SDM instances communicate using the SDM Channel.
So, no need for locking or to handle concurrent updates from
multiple processors to the same virtio queue.



These are the parts that I find a little confusing:

\begin{description}
\item[0] hg_vq
\item[1] gh_vq
\end{description}

Queue 0 is used for device-to-driver communication (i.e., notification of a
signal), while queue 1 for driver-to-device communication.

...

The master slave behavior is meant on purpose to reflect the AMP like type of 
communication
where a master processor controls one or more slave processors.
A master SDM instance can send a signal to any of the slaves instances,
while slaves can only signal the master.

...

The \texttt{SDMSignalData} structure is first filled by the source SDM kernel
virtio driver and sent over the gh_vq.


I interpret that as there being a single gh_vq queue and multiple slaves 
sending events
on that queue. Do you mean that there are multiple gh_vq's?
Am I missing something?


When I talk about master/slave instance I mean two different SDM
devices, one with master and the other with slave behavior.
This means that each SDM device will have its own pair of virtio
queues (gh_vq and hg_vq). Each remote processor (or slave)
will use its own SDM slave device and update only the related virtio
queues. The same is true for the master.
All the instances of the SDM interact between each other through
the SDM Channel (e.g., shared memory or network sockets in case
multiple nodes are involved in the virtualization of the AMP).

So, as an example, if your system has 1 master processor and 2
remote processors, you will have to instantiate three SDM devices
(1 master and 2 slaves). This means three different virtio devices,
each one interacting with a different processor.

Probably this aspect is not explicitly highlighted in the text, and I
can clarify it a bit in the next release.

Please let me know whether you think that further 

Re: [Qemu-devel] [PATCH 1/2] virtio-9p: print error message and exit instead of BUG_ON()

[Markus Armbruster]

Greg Kurz  writes:

> Calling assert() really makes sense when hitting a genuine bug, which calls
> for a fix in QEMU. However, when something goes wrong because the guest
> sends a malformed message, it is better to write down a more meaningul
> error message and exit.
>
> Signed-off-by: Greg Kurz 
> ---
>  hw/9pfs/virtio-9p-device.c |   20 ++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 009b43f6d045..67059182645a 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -19,6 +19,7 @@
>  #include "coth.h"
>  #include "hw/virtio/virtio-access.h"
>  #include "qemu/iov.h"
> +#include "qemu/error-report.h"
>  
>  void virtio_9p_push_and_notify(V9fsPDU *pdu)
>  {
> @@ -35,6 +36,11 @@ void virtio_9p_push_and_notify(V9fsPDU *pdu)
>  virtio_notify(VIRTIO_DEVICE(v), v->vq);
>  }
>  
> +static void virtio_9p_error(const char *msg)
> +{
> +error_report("The virtio-9p driver in the guest has an issue: %s", msg);
> +}
> +
>  static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
>  {
>  V9fsVirtioState *v = (V9fsVirtioState *)vdev;
> @@ -56,13 +62,23 @@ static void handle_9p_output(VirtIODevice *vdev, 
> VirtQueue *vq)
>  break;
>  }
>  
> -BUG_ON(elem->out_num == 0 || elem->in_num == 0);
> +if (elem->out_num == 0) {
> +virtio_9p_error("missing VirtFS request's header");
> +exit(1);
> +}

Can the guest trigger this?

> +if (elem->in_num == 0) {
> +virtio_9p_error("missing VirtFS reply's header");
> +exit(1);
> +}

Same question.

>  QEMU_BUILD_BUG_ON(sizeof out != 7);
>  
>  v->elems[pdu->idx] = elem;
>  len = iov_to_buf(elem->out_sg, elem->out_num, 0,
>   , sizeof out);
> -BUG_ON(len != sizeof out);
> +if (len != sizeof out) {
> +virtio_9p_error("malformed VirtFS request");
> +exit(1);
> +}

Same question.

>  
>  pdu->size = le32_to_cpu(out.size_le);
>  

[Qemu-devel] [PATCH] virtio-gpu-pci: tag as not hotpluggable

[Gerd Hoffmann]

We can't hotplug display adapters in qemu, tag virtio-gpu-pci
accordingly (virtio-vga already has this).

Signed-off-by: Gerd Hoffmann 
---
 hw/display/virtio-gpu-pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index 34a724c..ef92c4a 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -48,6 +48,7 @@ static void virtio_gpu_pci_class_init(ObjectClass *klass, 
void *data)
 
 set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
 dc->props = virtio_gpu_pci_properties;
+dc->hotpluggable = false;
 k->realize = virtio_gpu_pci_realize;
 pcidev_k->class_id = PCI_CLASS_DISPLAY_OTHER;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/2] virtio-pci: error out when both legacy and modern modes are disabled

[Markus Armbruster]

Greg Kurz  writes:

> From: Greg Kurz 
>
> Without presuming if we got there because of a user mistake or some
> more subtle bug in the tooling, it really does not make sense to
> implement a non-functional device.
>
> Signed-off-by: Greg Kurz 
> Reviewed-by: Marcel Apfelbaum 
> Reviewed-by: Cornelia Huck 
> Signed-off-by: Greg Kurz 
> ---
>  hw/virtio/virtio-pci.c |8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 268fd8ebb219..4b6a8a356621 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1842,6 +1842,14 @@ static void virtio_pci_dc_realize(DeviceState *qdev, 
> Error **errp)
>  VirtIOPCIProxy *proxy = VIRTIO_PCI(qdev);
>  PCIDevice *pci_dev = >pci_dev;
>  
> +if (!(virtio_pci_modern(proxy) || virtio_pci_legacy(proxy))) {
> +error_setg(errp, "device cannot work as neither modern nor legacy 
> mode"
> +   " is enabled");
> +error_append_hint(errp, "Set either disable-modern or disable-legacy"
> +  " to off\n");
> +return;
> +}
> +
>  if (!(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_PCIE) &&
>  virtio_pci_modern(proxy)) {
>  pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;

Pardon my ignorance... is this a device-specific restriction, or is it
the same for more (all?) virtio devices?

Re: [Qemu-devel] [PATCH v2 01/10] ppc: Fix rfi/rfid/hrfi/... emulation

[Cédric Le Goater]

On 09/07/2016 11:48 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2016-09-07 at 14:13 +0200, Cédric Le Goater wrote:
>> On 09/07/2016 01:08 PM, Benjamin Herrenschmidt wrote:
>>>
>>> On Wed, 2016-09-07 at 12:50 +0200, Cédric Le Goater wrote:

 This is a bit broader than Ben's patch which used
 PPC_SEGMENT_64B. 
 it's basically !PPC_64B which includes the e5500.

 If so, here is a proposal below adding a new PPC_RFI in the 
 "PowerPC Instructions types definitions" enum for that purpose. 
 Not much bits left there.
>>>
>>> Why not stick to PPC_SEGMENT_64B ?
>>
>> I am trying to remove the rfi instruction from the set of the CPU 
>> and I think we need to introduce a new PPC_* bit for GEN_HANDLER to :
> 
> What does it buy you instead of just having the test in the handler ?

not much. I will send the remaining bit of the original patch.

Thanks,
C.


>> +GEN_HANDLER(rfi, 0x13, 0x12, 0x01, 0x03FF8001, PPC_RFI),
>>
>> we can also keep the test on PPC_SEGMENT_64B in the handler which 
>> works perfectly fine. 
>>
>>>
>>> rfi exists on all 32-bit processors and all non-Book3S (aka server
>>> aka
>>> segment/hash) 64-bit. So PPC_SEGMENT_64B is the test we want.
>>>
>>> IE. rfi does exist on e5500
>>
>> ok.
>>
>> Cheers,
>> C.

[Qemu-devel] [PATCH] virtio-vga: adapt to page-per-vq=off

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 hw/display/virtio-vga.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index 5b510a1..f77b401 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -122,6 +122,17 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, 
Error **errp)
  */
 vpci_dev->modern_mem_bar = 2;
 vpci_dev->msix_bar = 4;
+
+if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
+/*
+ * with page-per-vq=off there is no padding space we can use
+ * for the stdvga registers.  Make the common and isr regions
+ * smaller then.
+ */
+vpci_dev->common.size /= 2;
+vpci_dev->isr.size /= 2;
+}
+
 offset = memory_region_size(_dev->modern_bar);
 offset -= vpci_dev->notify.size;
 vpci_dev->notify.offset = offset;
-- 
1.8.3.1

[Qemu-devel] [PATCH 0/2] vhost-user: set_mem_table updates

[Maxime Coquelin]

The series first restore the GET_FEATURES call in vhost_set_mem_table(),
as the hang it introduced was due to the use of TCG in the vhost-user-test,
mode which is no more used since commit cdafe929 ("vhost-user-test: Use libqos
instead of pxe-virtio.rom").

Second patch aims at being as close as possible to original behaviour,
and so only request a sync after having updated the mem table when necessary.

I'll be glad to get your feedback, especially on patch 2, to confirm my
understanding is correct, and the sync is not required in the listed cases.

Thanks,
Maxime

Maxime Coquelin (2):
  Revert "Revert "vhost-user: Attempt to fix a race with
set_mem_table.""
  vhost-user: only seek a reply if needed in set_mem_table

 hw/virtio/vhost-user.c| 134 +-
 hw/virtio/vhost.c |  10 
 include/hw/virtio/vhost.h |   1 +
 3 files changed, 85 insertions(+), 60 deletions(-)

-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 07/27] tcg: Add EXCP_ATOMIC

[Alex Bennée]

Richard Henderson  writes:

> When we cannot emulate an atomic operation within a parallel
> context, this exception allows us to stop the world and try
> again in a serial context.
>
> Signed-off-by: Richard Henderson 
> ---
>  cpu-exec-common.c   |  6 +
>  cpu-exec.c  | 23 +++
>  cpus.c  |  6 +
>  include/exec/cpu-all.h  |  1 +
>  include/exec/exec-all.h |  1 +
>  include/qemu-common.h   |  1 +
>  linux-user/main.c   | 59 
> -
>  tcg/tcg.h   |  1 +
>  translate-all.c |  1 +
>  9 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/cpu-exec-common.c b/cpu-exec-common.c
> index 0cb4ae6..767d9c6 100644
> --- a/cpu-exec-common.c
> +++ b/cpu-exec-common.c
> @@ -77,3 +77,9 @@ void cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc)
>  }
>  siglongjmp(cpu->jmp_env, 1);
>  }
> +
> +void cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc)
> +{
> +cpu->exception_index = EXCP_ATOMIC;
> +cpu_loop_exit_restore(cpu, pc);
> +}
> diff --git a/cpu-exec.c b/cpu-exec.c
> index b840e1d..041f8b7 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -225,6 +225,29 @@ static void cpu_exec_nocache(CPUState *cpu, int 
> max_cycles,
>  }
>  #endif
>
> +void cpu_exec_step(CPUState *cpu)
> +{
> +CPUArchState *env = (CPUArchState *)cpu->env_ptr;
> +TranslationBlock *tb;
> +target_ulong cs_base, pc;
> +uint32_t flags;
> +bool old_tb_flushed;
> +
> +old_tb_flushed = cpu->tb_flushed;
> +cpu->tb_flushed = false;
> +
> +cpu_get_tb_cpu_state(env, , _base, );
> +tb = tb_gen_code(cpu, pc, cs_base, flags,
> + 1 | CF_NOCACHE | CF_IGNORE_ICOUNT);
> +tb->orig_tb = NULL;
> +cpu->tb_flushed |= old_tb_flushed;
> +/* execute the generated code */
> +trace_exec_tb_nocache(tb, pc);
> +cpu_tb_exec(cpu, tb);
> +tb_phys_invalidate(tb, -1);
> +tb_free(tb);
> +}
> +
>  struct tb_desc {
>  target_ulong pc;
>  target_ulong cs_base;
> diff --git a/cpus.c b/cpus.c
> index 84c3520..a01bbbd 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1575,6 +1575,12 @@ static void tcg_exec_all(void)
>  if (r == EXCP_DEBUG) {
>  cpu_handle_guest_debug(cpu);
>  break;
> +} else if (r == EXCP_ATOMIC) {
> +/* ??? When we begin running cpus in parallel, we should
> +   stop all cpus, clear parallel_cpus, and interpret a
> +   single insn with cpu_exec_step.  In the meantime,
> +   we should never get here.  */
> +abort();

Pranith has been hitting this abort in the latest merged tree with MTTCG
but I'm a little unclear how it got here. So is the plan the MTTCG
thread function should do a step_atomic a-la user mode but we'll never
get here in the single threaded case?

>  }
>  } else if (cpu->stop || cpu->stopped) {
>  if (cpu->unplug) {
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 8007abb..d2aac4b 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -31,6 +31,7 @@
>  #define EXCP_DEBUG  0x10002 /* cpu stopped after a breakpoint or 
> singlestep */
>  #define EXCP_HALTED 0x10003 /* cpu is halted (waiting for external 
> event) */
>  #define EXCP_YIELD  0x10004 /* cpu wants to yield timeslice to another */
> +#define EXCP_ATOMIC 0x10005 /* stop-the-world and emulate atomic */
>
>  /* some important defines:
>   *
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index c1f59fa..ec72c5a 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -59,6 +59,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>  void cpu_exec_init(CPUState *cpu, Error **errp);
>  void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
>  void QEMU_NORETURN cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
> +void QEMU_NORETURN cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc);
>
>  #if !defined(CONFIG_USER_ONLY)
>  void cpu_reloading_memory_map(void);
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 1f2cb94..77e379d 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -76,6 +76,7 @@ void tcg_exec_init(unsigned long tb_size);
>  bool tcg_enabled(void);
>
>  void cpu_exec_init_all(void);
> +void cpu_exec_step(CPUState *cpu);
>
>  /**
>   * Sends a (part of) iovec down a socket, yielding when the socket is full, 
> or
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 78d8d04..54df300 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -179,13 +179,25 @@ static inline void start_exclusive(void)
>  }
>
>  /* Finish an exclusive operation.  */
> -static inline void __attribute__((unused)) end_exclusive(void)
> +static inline void end_exclusive(void)
>  {
>  pending_cpus = 0;
>  pthread_cond_broadcast(_resume);
>  

[Qemu-devel] [PATCH v5 3/3] tests: add RTAS command in the protocol

[Laurent Vivier]

Add a first test to validate the protocol:

- rtas/get-time-of-day compares the time
  from the guest with the time from the host.

Signed-off-by: Laurent Vivier 
---
v5:
- use qtest_spapr_boot() instead of machine_alloc_init()

v4:
- use qemu_strtoXXX() instead strtoXX()

v3:
- use mktimegm() instead of timegm()

v2:
- add a missing space in qrtas_call() prototype

 hw/ppc/spapr_rtas.c | 19 
 include/hw/ppc/spapr_rtas.h | 10 +++
 qtest.c | 17 +++
 tests/Makefile.include  |  3 ++
 tests/libqos/rtas.c | 71 +
 tests/libqos/rtas.h | 11 +++
 tests/libqtest.c| 10 +++
 tests/libqtest.h| 15 ++
 tests/rtas-test.c   | 40 +
 9 files changed, 196 insertions(+)
 create mode 100644 include/hw/ppc/spapr_rtas.h
 create mode 100644 tests/libqos/rtas.c
 create mode 100644 tests/libqos/rtas.h
 create mode 100644 tests/rtas-test.c

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index dc058e5..8aed56f 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -36,6 +36,7 @@
 
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_vio.h"
+#include "hw/ppc/spapr_rtas.h"
 #include "hw/ppc/ppc.h"
 #include "qapi-event.h"
 #include "hw/boards.h"
@@ -691,6 +692,24 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 return H_PARAMETER;
 }
 
+uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args,
+ uint32_t nret, uint64_t rets)
+{
+int token;
+
+for (token = 0; token < RTAS_TOKEN_MAX - RTAS_TOKEN_BASE; token++) {
+if (strcmp(cmd, rtas_table[token].name) == 0) {
+sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+
+rtas_table[token].fn(cpu, spapr, token + RTAS_TOKEN_BASE,
+ nargs, args, nret, rets);
+return H_SUCCESS;
+}
+}
+return H_PARAMETER;
+}
+
 void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn)
 {
 assert((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX));
diff --git a/include/hw/ppc/spapr_rtas.h b/include/hw/ppc/spapr_rtas.h
new file mode 100644
index 000..383611f
--- /dev/null
+++ b/include/hw/ppc/spapr_rtas.h
@@ -0,0 +1,10 @@
+#ifndef HW_SPAPR_RTAS_H
+#define HW_SPAPR_RTAS_H
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args,
+ uint32_t nret, uint64_t rets);
+#endif /* HW_SPAPR_RTAS_H */
diff --git a/qtest.c b/qtest.c
index 4c94708..beb62b4 100644
--- a/qtest.c
+++ b/qtest.c
@@ -28,6 +28,9 @@
 #include "qemu/option.h"
 #include "qemu/error-report.h"
 #include "qemu/cutils.h"
+#ifdef TARGET_PPC64
+#include "hw/ppc/spapr_rtas.h"
+#endif
 
 #define MAX_IRQ 256
 
@@ -531,6 +534,20 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 
 qtest_send_prefix(chr);
 qtest_send(chr, "OK\n");
+#ifdef TARGET_PPC64
+} else if (strcmp(words[0], "rtas") == 0) {
+uint64_t res, args, ret;
+unsigned long nargs, nret;
+
+g_assert(qemu_strtoul(words[2], NULL, 0, ) == 0);
+g_assert(qemu_strtoull(words[3], NULL, 0, ) == 0);
+g_assert(qemu_strtoul(words[4], NULL, 0, ) == 0);
+g_assert(qemu_strtoull(words[5], NULL, 0, ) == 0);
+res = qtest_rtas_call(words[1], nargs, args, nret, ret);
+
+qtest_send_prefix(chr);
+qtest_sendf(chr, "OK %"PRIu64"\n", res);
+#endif
 } else if (qtest_enabled() && strcmp(words[0], "clock_step") == 0) {
 int64_t ns;
 
diff --git a/tests/Makefile.include b/tests/Makefile.include
index da17b9b..a2cd446 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -272,6 +272,7 @@ check-qtest-sparc-y += tests/prom-env-test$(EXESUF)
 check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
 check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
 check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
+check-qtest-ppc64-y += tests/rtas-test$(EXESUF)
 
 check-qtest-generic-y += tests/qom-test$(EXESUF)
 
@@ -559,6 +560,7 @@ libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o 
tests/libqos/malloc.o
 libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
 libqos-spapr-obj-y = $(libqos-obj-y) tests/libqos/malloc-spapr.o
 libqos-spapr-obj-y += tests/libqos/libqos-spapr.o
+libqos-spapr-obj-y += tests/libqos/rtas.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
 libqos-pc-obj-y += tests/libqos/ahci.o
@@ -573,6 +575,7 @@ tests/m48t59-test$(EXESUF): tests/m48t59-test.o
 tests/endianness-test$(EXESUF): tests/endianness-test.o
 tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o 

[Qemu-devel] [PATCH v5 0/3] tests: add RTAS protocol

[Laurent Vivier]

This series allows to call RTAS commands from the qtest framework,
and defines a first test to call RTAS command "get-time-of-day"
to validate the protocol and test RTAS.

RTAS command parameters are passed to the guest via the
guest memory, so we also need to implement the guest memory
management functions for SPAPR target.

RTAS commands will be needed later to test PCI from the qtest framework
with SPAPR virtual machines: PCI configuration is read/written with
RTAS commands "ibm,read-pci-config", "ibm,write-pci-config".

v5:
- replace "ppc64" by "spapr"
- define and use qtest_spapr_vboot()/qtest_spapr_boot()/qtest_spapr_shutdown()

v4:
- use qemu_strtoXXX() instead strtoXX(),
  add a patch in the series to change all strtoXX() in qtest.c

v3:
- use mktimegm() instead of timegm()

v2:
- remove useless parenthesis, inline
- add a missing space in qrtas_call() prototype

Laurent Vivier (3):
  qtest: replace strtoXX() by qemu_strtoXX()
  libqos: define SPAPR libqos functions
  tests: add RTAS command in the protocol

 hw/ppc/spapr_rtas.c | 19 
 include/hw/ppc/spapr_rtas.h | 10 +++
 qtest.c | 66 ++---
 tests/Makefile.include  |  5 
 tests/libqos/libqos-pc.c|  2 ++
 tests/libqos/libqos-spapr.c | 30 +++
 tests/libqos/libqos-spapr.h | 10 +++
 tests/libqos/libqos.c   |  1 -
 tests/libqos/libqos.h   |  1 +
 tests/libqos/malloc-spapr.c | 38 
 tests/libqos/malloc-spapr.h | 17 +++
 tests/libqos/rtas.c | 71 +
 tests/libqos/rtas.h | 11 +++
 tests/libqtest.c| 10 +++
 tests/libqtest.h| 15 ++
 tests/rtas-test.c   | 40 +
 16 files changed, 322 insertions(+), 24 deletions(-)
 create mode 100644 include/hw/ppc/spapr_rtas.h
 create mode 100644 tests/libqos/libqos-spapr.c
 create mode 100644 tests/libqos/libqos-spapr.h
 create mode 100644 tests/libqos/malloc-spapr.c
 create mode 100644 tests/libqos/malloc-spapr.h
 create mode 100644 tests/libqos/rtas.c
 create mode 100644 tests/libqos/rtas.h
 create mode 100644 tests/rtas-test.c

-- 
2.5.5

Re: [Qemu-devel] [PATCH v7 1/4] vfio: Mediated device Core driver

[Jike Song]

On 08/25/2016 11:53 AM, Kirti Wankhede wrote:
> Design for Mediated Device Driver:
> Main purpose of this driver is to provide a common interface for mediated
> device management that can be used by different drivers of different
> devices.
> 
> This module provides a generic interface to create the device, add it to
> mediated bus, add device to IOMMU group and then add it to vfio group.
> 
> Below is the high Level block diagram, with Nvidia, Intel and IBM devices
> as example, since these are the devices which are going to actively use
> this module as of now.
> 
>  +---+
>  |   |
>  | +---+ |  mdev_register_driver() +--+
>  | |   | +<+ __init() |
>  | |  mdev | | |  |
>  | |  bus  | +>+  |<-> VFIO user
>  | |  driver   | | probe()/remove()| vfio_mdev.ko |APIs
>  | |   | | |  |
>  | +---+ | +--+
>  |   |

This aimed to have only one single vfio bus driver for all mediated devices,
right?

>  |  MDEV CORE|
>  |   MODULE  |
>  |   mdev.ko |
>  | +---+ |  mdev_register_device() +--+
>  | |   | +<+  |
>  | |   | | |  nvidia.ko   |<-> physical
>  | |   | +>+  |device
>  | |   | |callback +--+
>  | | Physical  | |
>  | |  device   | |  mdev_register_device() +--+
>  | | interface | |<+  |
>  | |   | | |  i915.ko |<-> physical
>  | |   | +>+  |device
>  | |   | |callback +--+
>  | |   | |
>  | |   | |  mdev_register_device() +--+
>  | |   | +<+  |
>  | |   | | | ccw_device.ko|<-> physical
>  | |   | +>+  |device
>  | |   | |callback +--+
>  | +---+ |
>  +---+
> 
> Core driver provides two types of registration interfaces:
> 1. Registration interface for mediated bus driver:
> 
> /**
>   * struct mdev_driver - Mediated device's driver
>   * @name: driver name
>   * @probe: called when new device created
>   * @remove:called when device removed
>   * @driver:device driver structure
>   *
>   **/
> struct mdev_driver {
>  const char *name;
>  int  (*probe)  (struct device *dev);
>  void (*remove) (struct device *dev);
>  struct device_driverdriver;
> };
> 
> int  mdev_register_driver(struct mdev_driver *drv, struct module *owner);
> void mdev_unregister_driver(struct mdev_driver *drv);
> 
> Mediated device's driver for mdev, vfio_mdev, uses this interface to
> register with Core driver. vfio_mdev module adds mediated device to VFIO
> group.
> 
> 2. Physical device driver interface
> This interface provides vendor driver the set APIs to manage physical
> device related work in their own driver. APIs are :
> - supported_config: provide supported configuration list by the vendor
>   driver
> - create: to allocate basic resources in vendor driver for a mediated
> device.
> - destroy: to free resources in vendor driver when mediated device is
>  destroyed.
> - reset: to free and reallocate resources in vendor driver during device
>reset.
> - set_online_status: to change online status of mediated device.
> - get_online_status: to get current (online/offline) status of mediated
>device.
> - read : read emulation callback.
> - write: write emulation callback.
> - mmap: mmap emulation callback.
> - get_irq_info: to retrieve information about mediated device's IRQ.
> - set_irqs: send interrupt configuration information that VMM sets.
> - get_device_info: to retrieve VFIO device related flags, number of regions
>  and number of IRQs supported.
> - get_region_info: to provide region size and its flags for the mediated
>  device.
> 
> This registration interface should be used by vendor drivers to register
> each physical device to mdev core driver.
> Locks to serialize above callbacks are removed. If required, vendor driver
> can have locks to serialize above APIs in their driver.
> 
> Signed-off-by: Kirti Wankhede 
> Signed-off-by: Neo Jia 
> Change-Id: I73a5084574270b14541c529461ea2f03c292d510
> Reviewed-on: http://git-master/r/1175705
> Reviewed-by: Automatic_Commit_Validation_User
> ---
>  drivers/vfio/Kconfig |   1 +
>  drivers/vfio/Makefile|   1 +
>  drivers/vfio/mdev/Kconfig|  12 +
>  

Re: [Qemu-devel] [PATCH v2 08/27] HACK: Always enable parallel_cpus

[Alex Bennée]

Richard Henderson  writes:

> This is really just a placeholder for an actual
> command-line switch for mttcg.
> ---
>  translate-all.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/translate-all.c b/translate-all.c
> index 99ae7f9..a10fa06 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -119,7 +119,7 @@ static void *l1_map[V_L1_SIZE];
>
>  /* code generation context */
>  TCGContext tcg_ctx;
> -bool parallel_cpus;
> +bool parallel_cpus = 1;

I appreciate this is currently a hack but for CONFIG_USER it should
always be true anyway.

>
>  /* translation block context */
>  #ifdef CONFIG_USER_ONLY


--
Alex Bennée

[Qemu-devel] [PATCHv5] tests: add qtest_add_data_func_full

[Marc-André Lureau]

Allows one to specify a destroy function for the test data.

Add a fallback using glib g_test_add_vtable() internal function, whose
signature changed over time. Tested with glib 2.22, 2.26 and 2.48, which
according to git log should be enough to cover all variations.

Signed-off-by: Marc-André Lureau 
---
 tests/libqtest.c | 19 +++
 tests/libqtest.h | 17 +
 2 files changed, 36 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index eb00f13..5f4450f 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -758,6 +758,25 @@ void qtest_add_func(const char *str, void (*fn)(void))
 g_free(path);
 }
 
+void qtest_add_data_func_full(const char *str, void *data,
+  void (*fn)(const void *),
+  GDestroyNotify data_free_func)
+{
+gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
+#if GLIB_CHECK_VERSION(2, 34, 0)
+g_test_add_data_func_full(path, data, fn, data_free_func);
+#elif GLIB_CHECK_VERSION(2, 26, 0)
+/* back-compat casts, remove this once we can require new-enough glib */
+g_test_add_vtable(path, 0, data, NULL,
+  (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
+#elif GLIB_CHECK_VERSION(2, 22, 0)
+/* back-compat casts, remove this once we can require new-enough glib */
+g_test_add_vtable(path, 0, data, NULL,
+  (void (*)(void)) fn, (void (*)(void)) data_free_func);
+#endif
+g_free(path);
+}
+
 void qtest_add_data_func(const char *str, const void *data,
  void (*fn)(const void *))
 {
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 37f37ad..d2b4853 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -425,6 +425,23 @@ void qtest_add_func(const char *str, void (*fn)(void));
 void qtest_add_data_func(const char *str, const void *data,
  void (*fn)(const void *));
 
+/**
+ * qtest_add_data_func_full:
+ * @str: Test case path.
+ * @data: Test case data
+ * @fn: Test case function
+ * @data_free_func: GDestroyNotify for data
+ *
+ * Add a GTester testcase with the given name, data and function.
+ * The path is prefixed with the architecture under test, as
+ * returned by qtest_get_arch().
+ *
+ * @data is passed to @data_free_func() on test completion.
+ */
+void qtest_add_data_func_full(const char *str, void *data,
+  void (*fn)(const void *),
+  GDestroyNotify data_free_func);
+
 /**
  * qtest_add:
  * @testpath: Test case path
-- 
2.10.0

[Qemu-devel] [PATCH v5 2/3] libqos: define SPAPR libqos functions

[Laurent Vivier]

Define spapr_alloc_init()/spapr_alloc_init_flags()/spapr_alloc_uninit()

  to allocate and use SPAPR guest memory

Define qtest_spapr_vboot()/qtest_spapr_boot()/qtest_spapr_shutdown()

  to start SPAPR guest with QOSState initialized for it (memory management)

Move qtest_irq_intercept_in() from generic part to PC part.

Signed-off-by: Laurent Vivier 
---
v5:
- replace "ppc64" by "spapr"
- Add test_spapr_vboot()/qtest_spapr_boot()/qtest_spapr_shutdown()
  and remove machine_alloc_XXX() fuctions

 tests/Makefile.include  |  2 ++
 tests/libqos/libqos-pc.c|  2 ++
 tests/libqos/libqos-spapr.c | 30 ++
 tests/libqos/libqos-spapr.h | 10 ++
 tests/libqos/libqos.c   |  1 -
 tests/libqos/libqos.h   |  1 +
 tests/libqos/malloc-spapr.c | 38 ++
 tests/libqos/malloc-spapr.h | 17 +
 8 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 tests/libqos/libqos-spapr.c
 create mode 100644 tests/libqos/libqos-spapr.h
 create mode 100644 tests/libqos/malloc-spapr.c
 create mode 100644 tests/libqos/malloc-spapr.h

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 14be491..da17b9b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -557,6 +557,8 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o 
$(test-crypto-obj-y)
 
 libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
 libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
+libqos-spapr-obj-y = $(libqos-obj-y) tests/libqos/malloc-spapr.o
+libqos-spapr-obj-y += tests/libqos/libqos-spapr.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
 libqos-pc-obj-y += tests/libqos/ahci.o
diff --git a/tests/libqos/libqos-pc.c b/tests/libqos/libqos-pc.c
index 72b5e3b..df34092 100644
--- a/tests/libqos/libqos-pc.c
+++ b/tests/libqos/libqos-pc.c
@@ -21,6 +21,8 @@ QOSState *qtest_pc_boot(const char *cmdline_fmt, ...)
 qs = qtest_vboot(_ops, cmdline_fmt, ap);
 va_end(ap);
 
+qtest_irq_intercept_in(global_qtest, "ioapic");
+
 return qs;
 }
 
diff --git a/tests/libqos/libqos-spapr.c b/tests/libqos/libqos-spapr.c
new file mode 100644
index 000..f19408b
--- /dev/null
+++ b/tests/libqos/libqos-spapr.c
@@ -0,0 +1,30 @@
+#include "qemu/osdep.h"
+#include "libqos/libqos-spapr.h"
+#include "libqos/malloc-spapr.h"
+
+static QOSOps qos_ops = {
+.init_allocator = spapr_alloc_init_flags,
+.uninit_allocator = spapr_alloc_uninit
+};
+
+QOSState *qtest_spapr_vboot(const char *cmdline_fmt, va_list ap)
+{
+return qtest_vboot(_ops, cmdline_fmt, ap);
+}
+
+QOSState *qtest_spapr_boot(const char *cmdline_fmt, ...)
+{
+QOSState *qs;
+va_list ap;
+
+va_start(ap, cmdline_fmt);
+qs = qtest_vboot(_ops, cmdline_fmt, ap);
+va_end(ap);
+
+return qs;
+}
+
+void qtest_spapr_shutdown(QOSState *qs)
+{
+return qtest_shutdown(qs);
+}
diff --git a/tests/libqos/libqos-spapr.h b/tests/libqos/libqos-spapr.h
new file mode 100644
index 000..dcb5c43
--- /dev/null
+++ b/tests/libqos/libqos-spapr.h
@@ -0,0 +1,10 @@
+#ifndef LIBQOS_SPAPR_H
+#define LIBQOS_SPAPR_H
+
+#include "libqos/libqos.h"
+
+QOSState *qtest_spapr_vboot(const char *cmdline_fmt, va_list ap);
+QOSState *qtest_spapr_boot(const char *cmdline_fmt, ...);
+void qtest_spapr_shutdown(QOSState *qs);
+
+#endif
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index c7ba441..a852dc5 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -20,7 +20,6 @@ QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, 
va_list ap)
 cmdline = g_strdup_vprintf(cmdline_fmt, ap);
 qs->qts = qtest_start(cmdline);
 qs->ops = ops;
-qtest_irq_intercept_in(global_qtest, "ioapic");
 if (ops && ops->init_allocator) {
 qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
 }
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index 604980d..5022ce3 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -4,6 +4,7 @@
 #include "libqtest.h"
 #include "libqos/pci.h"
 #include "libqos/malloc-pc.h"
+#include "libqos/malloc-spapr.h"
 
 typedef struct QOSOps {
 QGuestAllocator *(*init_allocator)(QAllocOpts);
diff --git a/tests/libqos/malloc-spapr.c b/tests/libqos/malloc-spapr.c
new file mode 100644
index 000..006404a
--- /dev/null
+++ b/tests/libqos/malloc-spapr.c
@@ -0,0 +1,38 @@
+/*
+ * libqos malloc support for SPAPR
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqos/malloc-spapr.h"
+
+#include "qemu-common.h"
+
+#define PAGE_SIZE 4096
+
+/* Memory must be a multiple of 256 MB,
+ * so we have at least 256MB
+ */
+#define SPAPR_MIN_SIZE 0x1000
+
+void spapr_alloc_uninit(QGuestAllocator *allocator)
+{
+alloc_uninit(allocator);
+}
+
+QGuestAllocator 

[Qemu-devel] [PATCH v5 1/3] qtest: replace strtoXX() by qemu_strtoXX()

[Laurent Vivier]

Check the result of qemu_strtoXX() and assert
if the string cannot be converted.

Signed-off-by: Laurent Vivier 
Reviewed-by: David Gibson 
---
v5:
- update log message about result checking
- add David's Rb

v4:
- add this patch in the series to change all strtoXX() in qtest.c

 qtest.c | 49 ++---
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/qtest.c b/qtest.c
index da4826c..4c94708 100644
--- a/qtest.c
+++ b/qtest.c
@@ -27,6 +27,7 @@
 #include "qemu/config-file.h"
 #include "qemu/option.h"
 #include "qemu/error-report.h"
+#include "qemu/cutils.h"
 
 #define MAX_IRQ 256
 
@@ -324,12 +325,13 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 } else if (strcmp(words[0], "outb") == 0 ||
strcmp(words[0], "outw") == 0 ||
strcmp(words[0], "outl") == 0) {
-uint16_t addr;
-uint32_t value;
+unsigned long addr;
+unsigned long value;
 
 g_assert(words[1] && words[2]);
-addr = strtoul(words[1], NULL, 0);
-value = strtoul(words[2], NULL, 0);
+g_assert(qemu_strtoul(words[1], NULL, 0, ) == 0);
+g_assert(qemu_strtoul(words[2], NULL, 0, ) == 0);
+g_assert(addr <= 0x);
 
 if (words[0][3] == 'b') {
 cpu_outb(addr, value);
@@ -343,11 +345,12 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 } else if (strcmp(words[0], "inb") == 0 ||
 strcmp(words[0], "inw") == 0 ||
 strcmp(words[0], "inl") == 0) {
-uint16_t addr;
+unsigned long addr;
 uint32_t value = -1U;
 
 g_assert(words[1]);
-addr = strtoul(words[1], NULL, 0);
+g_assert(qemu_strtoul(words[1], NULL, 0, ) == 0);
+g_assert(addr <= 0x);
 
 if (words[0][2] == 'b') {
 value = cpu_inb(addr);
@@ -366,8 +369,8 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 uint64_t value;
 
 g_assert(words[1] && words[2]);
-addr = strtoull(words[1], NULL, 0);
-value = strtoull(words[2], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
+g_assert(qemu_strtoull(words[2], NULL, 0, ) == 0);
 
 if (words[0][5] == 'b') {
 uint8_t data = value;
@@ -395,7 +398,7 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 uint64_t value = UINT64_C(-1);
 
 g_assert(words[1]);
-addr = strtoull(words[1], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
 
 if (words[0][4] == 'b') {
 uint8_t data;
@@ -421,8 +424,8 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 char *enc;
 
 g_assert(words[1] && words[2]);
-addr = strtoull(words[1], NULL, 0);
-len = strtoull(words[2], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
+g_assert(qemu_strtoull(words[2], NULL, 0, ) == 0);
 
 data = g_malloc(len);
 cpu_physical_memory_read(addr, data, len);
@@ -443,8 +446,8 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 gchar *b64_data;
 
 g_assert(words[1] && words[2]);
-addr = strtoull(words[1], NULL, 0);
-len = strtoull(words[2], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
+g_assert(qemu_strtoull(words[2], NULL, 0, ) == 0);
 
 data = g_malloc(len);
 cpu_physical_memory_read(addr, data, len);
@@ -460,8 +463,8 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 size_t data_len;
 
 g_assert(words[1] && words[2] && words[3]);
-addr = strtoull(words[1], NULL, 0);
-len = strtoull(words[2], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
+g_assert(qemu_strtoull(words[2], NULL, 0, ) == 0);
 
 data_len = strlen(words[3]);
 if (data_len < 3) {
@@ -486,12 +489,12 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 } else if (strcmp(words[0], "memset") == 0) {
 uint64_t addr, len;
 uint8_t *data;
-uint8_t pattern;
+unsigned long pattern;
 
 g_assert(words[1] && words[2] && words[3]);
-addr = strtoull(words[1], NULL, 0);
-len = strtoull(words[2], NULL, 0);
-pattern = strtoull(words[3], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, ) == 0);
+g_assert(qemu_strtoull(words[2], NULL, 0, ) == 0);
+g_assert(qemu_strtoul(words[3], NULL, 0, ) == 0);
 
 data = g_malloc(len);
 memset(data, pattern, len);
@@ -507,8 +510,8 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 gsize out_len;
 
 g_assert(words[1] && words[2] && words[3]);
-addr = strtoull(words[1], NULL, 0);
-  

[Qemu-devel] [PATCH 2/2] vhost-user: only seek a reply if needed in set_mem_table

[Maxime Coquelin]

The goal of this patch is to only request a sync (reply_ack,
or get_features) in set_mem_table only when necessary.

It should not be necessary the first time we set the table,
or when we add a new regions which hadn't been merged with an
existing ones.

Suggested-by: Michael S. Tsirkin 
Cc: Prerna Saxena 
Cc: Marc-André Lureau 
Signed-off-by: Maxime Coquelin 
---
 hw/virtio/vhost-user.c|  7 +++
 hw/virtio/vhost.c | 10 ++
 include/hw/virtio/vhost.h |  1 +
 3 files changed, 18 insertions(+)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 1a7d53c..ca41728 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -531,6 +531,11 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
 
 vhost_user_write(dev, , fds, fd_num);
 
+if (!dev->mem_changed_req_sync) {
+/* The update only add regions, skip the sync */
+return 0;
+}
+
 if (reply_supported) {
 return process_message_reply(dev, msg.request);
 } else {
@@ -541,6 +546,8 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
 vhost_user_get_features(dev, );
 }
 
+dev->mem_changed_req_sync = false;
+
 return 0;
 }
 
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 3d0c807..e653067 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -303,7 +303,11 @@ static void vhost_dev_assign_memory(struct vhost_dev *dev,
 reg->guest_phys_addr = start_addr;
 reg->userspace_addr = uaddr;
 ++to;
+} else {
+/* Existing mapping updated, sync is required */
+dev->mem_changed_req_sync = true;
 }
+
 assert(to <= dev->mem->nregions + 1);
 dev->mem->nregions = to;
 }
@@ -533,6 +537,7 @@ static void vhost_set_memory(MemoryListener *listener,
 } else {
 /* Remove old mapping for this memory, if any. */
 vhost_dev_unassign_memory(dev, start_addr, size);
+dev->mem_changed_req_sync = true;
 }
 dev->mem_changed_start_addr = MIN(dev->mem_changed_start_addr, start_addr);
 dev->mem_changed_end_addr = MAX(dev->mem_changed_end_addr, start_addr + 
size - 1);
@@ -1126,6 +1131,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
 hdev->log_enabled = false;
 hdev->started = false;
 hdev->memory_changed = false;
+hdev->mem_changed_req_sync = false;
 memory_listener_register(>memory_listener, _space_memory);
 QLIST_INSERT_HEAD(_devices, hdev, entry);
 return 0;
@@ -1301,6 +1307,10 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice 
*vdev)
 if (r < 0) {
 goto fail_features;
 }
+
+/* First time the mem table is set, skip sync for completion */
+hdev->mem_changed_req_sync = false;
+
 r = hdev->vhost_ops->vhost_set_mem_table(hdev, hdev->mem);
 if (r < 0) {
 VHOST_OPS_DEBUG("vhost_set_mem_table failed");
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index e433089..4bbf36a 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -55,6 +55,7 @@ struct vhost_dev {
 uint64_t log_size;
 Error *migration_blocker;
 bool memory_changed;
+bool mem_changed_req_sync;
 hwaddr mem_changed_start_addr;
 hwaddr mem_changed_end_addr;
 const VhostOps *vhost_ops;
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 3/7] ppc/pnv: Add XSCOM infrastructure

[Cédric Le Goater]

On 09/07/2016 11:53 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2016-09-07 at 17:47 +0200, Cédric Le Goater wrote:
>>  
>> +static uint64_t pnv_lpc_xscom_mr_read(void *opaque, hwaddr addr,
>> unsigned size)
>> +{
>> +XScomDevice *xd = XSCOM_DEVICE(opaque);
>> +uint64_t val = 0;
>> +
>> +pnv_lpc_xscom_read(xd, 0, xscom_to_pcb_addr(xd->chip_type,
>> addr), );
>> +return val;
>> +}
>> +
>> +static void pnv_lpc_xscom_mr_write(void *opaque, hwaddr addr,
>> +   uint64_t val, unsigned size)
>> +{
>> +XScomDevice *xd = XSCOM_DEVICE(opaque);
>> +pnv_lpc_xscom_write(xd, 0, xscom_to_pcb_addr(xd->chip_type,
>> addr), val);
>> +}
>>
> 
> I don't understand. That's not at all why I suggested or I'm missing
> something.

This was a preliminary hack on the full powernv tree to study the 
question. I made the two options cohabitate in the same qemu to see 
what were the issues and possible solutions.

The result is very much what you describe below. I need to start 
from the beginning now, and not the end, to make something cleaner.

> What I suggest is that you have a memory region per-chip (which is NOT
> hooked onto the main address space) which represents the PCB space.
> Calling xscom_region. Hook it up to its own address_space.
>
> Thus, the various devices (LPC, OCC, etc...) all just register a sub-
> region of that address space using the PCB addresses directly (well,
> shifted left by 3 because it's 8 bytes registers but that's it).
> 
> The XSCOM "controller" AKA ADU is the one doing the bridge. It
> registers an MMIO region in the main address space (SysBusDevice ?)
> and from the MMIO accessors, it does the address mangling, and use
> address_space_rw() to trigger accesses onto that chip's xscom_region.

yes. this is what your XSCOM bridge does already.

name = g_strdup_printf("xscom-%x", s->chip_id);
memory_region_init_io(>mem, OBJECT(s), _ops, s, name, XSCOM_SIZE);
sysbus_init_mmio(sbd, >mem);
sysbus_mmio_map(sbd, 0, XSCOM_BASE(s->chip_id));


Thanks,

C. 

[Qemu-devel] [PATCH 1/2] Revert "Revert "vhost-user: Attempt to fix a race with set_mem_table.""

[Maxime Coquelin]

This reverts commit 94c9cb31c04737f86be29afefbff401cd23bc24d.

Analysis of the race shows that it would happen only when QEMU relies
on TCG.

Since commit cdafe929 ("vhost-user-test: Use libqos instead of
pxe-virtio.rom"), vhost-user-test don't use TCG, so the race no more
appear.

Cc: Michael S. Tsirkin 
Cc: Marc-André Lureau 
Cc: Prerna Saxena 
Cc: Peter Maydell 
Signed-off-by: Maxime Coquelin 
---
 hw/virtio/vhost-user.c | 127 ++---
 1 file changed, 67 insertions(+), 60 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index b57454a..1a7d53c 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -263,66 +263,6 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, 
uint64_t base,
 return 0;
 }
 
-static int vhost_user_set_mem_table(struct vhost_dev *dev,
-struct vhost_memory *mem)
-{
-int fds[VHOST_MEMORY_MAX_NREGIONS];
-int i, fd;
-size_t fd_num = 0;
-bool reply_supported = virtio_has_feature(dev->protocol_features,
-  VHOST_USER_PROTOCOL_F_REPLY_ACK);
-
-VhostUserMsg msg = {
-.request = VHOST_USER_SET_MEM_TABLE,
-.flags = VHOST_USER_VERSION,
-};
-
-if (reply_supported) {
-msg.flags |= VHOST_USER_NEED_REPLY_MASK;
-}
-
-for (i = 0; i < dev->mem->nregions; ++i) {
-struct vhost_memory_region *reg = dev->mem->regions + i;
-ram_addr_t offset;
-MemoryRegion *mr;
-
-assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
-mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr,
- );
-fd = memory_region_get_fd(mr);
-if (fd > 0) {
-msg.payload.memory.regions[fd_num].userspace_addr = 
reg->userspace_addr;
-msg.payload.memory.regions[fd_num].memory_size  = reg->memory_size;
-msg.payload.memory.regions[fd_num].guest_phys_addr = 
reg->guest_phys_addr;
-msg.payload.memory.regions[fd_num].mmap_offset = offset;
-assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
-fds[fd_num++] = fd;
-}
-}
-
-msg.payload.memory.nregions = fd_num;
-
-if (!fd_num) {
-error_report("Failed initializing vhost-user memory map, "
- "consider using -object memory-backend-file share=on");
-return -1;
-}
-
-msg.size = sizeof(msg.payload.memory.nregions);
-msg.size += sizeof(msg.payload.memory.padding);
-msg.size += fd_num * sizeof(VhostUserMemoryRegion);
-
-if (vhost_user_write(dev, , fds, fd_num) < 0) {
-return -1;
-}
-
-if (reply_supported) {
-return process_message_reply(dev, msg.request);
-}
-
-return 0;
-}
-
 static int vhost_user_set_vring_addr(struct vhost_dev *dev,
  struct vhost_vring_addr *addr)
 {
@@ -537,6 +477,73 @@ static int vhost_user_get_features(struct vhost_dev *dev, 
uint64_t *features)
 return vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features);
 }
 
+static int vhost_user_set_mem_table(struct vhost_dev *dev,
+struct vhost_memory *mem)
+{
+int fds[VHOST_MEMORY_MAX_NREGIONS];
+int i, fd;
+size_t fd_num = 0;
+uint64_t features;
+bool reply_supported = virtio_has_feature(dev->protocol_features,
+  VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+VhostUserMsg msg = {
+.request = VHOST_USER_SET_MEM_TABLE,
+.flags = VHOST_USER_VERSION,
+};
+
+if (reply_supported) {
+msg.flags |= VHOST_USER_NEED_REPLY_MASK;
+}
+
+for (i = 0; i < dev->mem->nregions; ++i) {
+struct vhost_memory_region *reg = dev->mem->regions + i;
+ram_addr_t offset;
+MemoryRegion *mr;
+
+assert((uintptr_t)reg->userspace_addr == reg->userspace_addr);
+mr = memory_region_from_host((void *)(uintptr_t)reg->userspace_addr,
+ );
+fd = memory_region_get_fd(mr);
+if (fd > 0) {
+msg.payload.memory.regions[fd_num].userspace_addr
+ = reg->userspace_addr;
+msg.payload.memory.regions[fd_num].memory_size  = reg->memory_size;
+msg.payload.memory.regions[fd_num].guest_phys_addr
+ = reg->guest_phys_addr;
+msg.payload.memory.regions[fd_num].mmap_offset = offset;
+assert(fd_num < VHOST_MEMORY_MAX_NREGIONS);
+fds[fd_num++] = fd;
+}
+}
+
+msg.payload.memory.nregions = fd_num;
+
+if (!fd_num) {
+error_report("Failed initializing vhost-user memory map, "
+ "consider using -object 

Re: [Qemu-devel] [PATCH v4 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support

[Daniel P. Berrange]

On Thu, Sep 08, 2016 at 10:43:19AM +0200, Kevin Wolf wrote:
> Am 08.09.2016 um 00:32 hat ashish mittal geschrieben:
> > >> +
> > >> +void vxhs_set_acb_buffer(void *ptr, void *buffer)
> > >> +{
> > >> +VXHSAIOCB *acb = ptr;
> > >> +
> > >> +acb->buffer = buffer;
> > >> +}
> > >> +
> > >
> > > Unused function?
> > 
> > This is called from within libqnio.
> 
> Wait, you mean the library references a symbol in the qemu binary? This
> sounds completely wrong to me. I wouldn't even do something like this if
> it were an internal qemu library because I think libraries should be
> self-contained, but it's a much larger problem in something that is
> supposed to be an independent library.

I'd be surprised if that actually worked. If an external library wants
to refrence symbols in the QEMU binary, then QEMU would ned to be using
the -export-dynamic / -rdynamic linker flags to export all its symbols,
and AFAIK, we're not doing that.


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 v4 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support

[Kevin Wolf]

Am 08.09.2016 um 00:32 hat ashish mittal geschrieben:
> >> +
> >> +void vxhs_set_acb_buffer(void *ptr, void *buffer)
> >> +{
> >> +VXHSAIOCB *acb = ptr;
> >> +
> >> +acb->buffer = buffer;
> >> +}
> >> +
> >
> > Unused function?
> 
> This is called from within libqnio.

Wait, you mean the library references a symbol in the qemu binary? This
sounds completely wrong to me. I wouldn't even do something like this if
it were an internal qemu library because I think libraries should be
self-contained, but it's a much larger problem in something that is
supposed to be an independent library.

If the library has to call into qemu, it should only do so through
callbacks that qemu registered with the library (and then the function
wouldn't look unused in qemu).

In an earlier version of the series I suggested moving the part directly
related to qemu (at least the functions called qemu_*) from the library
to the block driver. Did you consider this?

Kevin

Re: [Qemu-devel] [PATCH 0/7] MIPS Boston board support

[Leon Alrae]

On Fri, Aug 19, 2016 at 08:40:32PM +0100, Paul Burton wrote:
> On 19/08/16 20:25, no-re...@patchew.org wrote:
> > Hi,
> > 
> > Your series failed automatic build test. Please find the testing commands 
> > and
> > their output below. If you have docker installed, you can probably 
> > reproduce it
> > locally.
> > 
> > Message-id: 20160819190903.10974-1-paul.bur...@imgtec.com
> > Subject: [Qemu-devel] [PATCH 0/7] MIPS Boston board support
> > Type: series
> 
> FYI, this build failure occurs because dtc/libfdt is missing this commit:
> 
> 
> https://git.kernel.org/cgit/utils/dtc/dtc.git/commit/libfdt/version.lds?id=a4b093f7366fdb429ca1781144d3985fa50d0fbb
> 
> Unfortunately the last release of dtc seems to be very old despite there
> being fixes like that present in it for well over a year. I'm open to
> suggestions about how to handle that...

Looks like 1.4.2 has appeared recently and it contains above commit.

One way would be just to bump our minimal dtc version requirement from
1.4.0 to 1.4.2 (and update qemu's dtc submodule as well)...

Thanks,
Leon

> 
> Thanks,
> Paul
> 
> > 
> > === TEST SCRIPT BEGIN ===
> > #!/bin/bash
> > set -e
> > git submodule update --init dtc
> > make J=8 docker-test-quick@centos6
> > 
> > # we need CURL DPRINTF patch
> > # 
> > http://patchew.org/QEMU/1470027888-24381-1-git-send-email-famz%40redhat.com/
> > #make J=8 docker-test-mingw@fedora
> > === TEST SCRIPT END ===
> > 
> > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> > Switched to a new branch 'test'
> > 1821581 hw/mips: MIPS Boston board support
> > 9b44da9 hw: xilinx-pcie: Add support for Xilinx AXI PCIe Controller
> > fe513d4 loader: Support Flattened Image Trees (FIT images)
> > d0296cc target-mips: Provide function to test if a CPU supports an ISA
> > 52c4cc5 hw/mips_gic: Update pin state on mask changes
> > 402000d hw/mips_gictimer: provide API for retrieving frequency
> > a80d326 hw/mips_cmgcr: allow GCR base to be moved
> > 
> > === OUTPUT BEGIN ===
> > Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 
> > 'dtc'
> > Cloning into 'dtc'...
> > Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
> >   BUILD centos6
> >   ARCHIVE qemu.tgz
> >   ARCHIVE dtc.tgz
> >   COPY RUNNER
> >   RUN test-quick in centos6
> > No C++ compiler available; disabling C++ specific optional code
> > Install prefix/tmp/qemu-test/src/tests/docker/install
> > BIOS directory/tmp/qemu-test/src/tests/docker/install/share/qemu
> > binary directory  /tmp/qemu-test/src/tests/docker/install/bin
> > library directory /tmp/qemu-test/src/tests/docker/install/lib
> > module directory  /tmp/qemu-test/src/tests/docker/install/lib/qemu
> > libexec directory /tmp/qemu-test/src/tests/docker/install/libexec
> > include directory /tmp/qemu-test/src/tests/docker/install/include
> > config directory  /tmp/qemu-test/src/tests/docker/install/etc
> > local state directory   /tmp/qemu-test/src/tests/docker/install/var
> > Manual directory  /tmp/qemu-test/src/tests/docker/install/share/man
> > ELF interp prefix /usr/gnemul/qemu-%M
> > Source path   /tmp/qemu-test/src
> > C compilercc
> > Host C compiler   cc
> > C++ compiler  
> > Objective-C compiler cc
> > ARFLAGS   rv
> > CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread 
> > -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -g 
> > QEMU_CFLAGS   -I/usr/include/pixman-1-fPIE -DPIE -m64 -D_GNU_SOURCE 
> > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
> > -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
> > -fno-strict-aliasing -fno-common  -Wendif-labels -Wmissing-include-dirs 
> > -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
> > -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
> > -Wtype-limits -fstack-protector-all
> > LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
> > make  make
> > install   install
> > pythonpython -B
> > smbd  /usr/sbin/smbd
> > module supportno
> > host CPU  x86_64
> > host big endian   no
> > target list   x86_64-softmmu aarch64-softmmu
> > tcg debug enabled no
> > gprof enabled no
> > sparse enabledno
> > strip binariesyes
> > profiler  no
> > static build  no
> > pixmansystem
> > SDL support   yes (1.2.14)
> > GTK support   no 
> > GTK GL supportno
> > VTE support   no 
> > TLS priority  NORMAL
> > GNUTLS supportno
> > GNUTLS rndno
> > libgcrypt no
> > libgcrypt kdf no
> > nettleno 
> > nettle kdfno
> > libtasn1  no
> > curses supportno
> > virgl support no
> > curl support  no
> > mingw32 support   no
> > Audio drivers oss
> > Block whitelist (rw) 
> > Block whitelist (ro) 
> > VirtFS supportno
> > VNC support   yes
> > VNC SASL support  no
> > VNC JPEG 

Re: [Qemu-devel] [PATCH v4 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support

[Daniel P. Berrange]

On Thu, Sep 08, 2016 at 11:29:41AM +0200, Kevin Wolf wrote:
> Am 08.09.2016 um 10:49 hat Daniel P. Berrange geschrieben:
> > On Thu, Sep 08, 2016 at 10:43:19AM +0200, Kevin Wolf wrote:
> > > Am 08.09.2016 um 00:32 hat ashish mittal geschrieben:
> > > > >> +
> > > > >> +void vxhs_set_acb_buffer(void *ptr, void *buffer)
> > > > >> +{
> > > > >> +VXHSAIOCB *acb = ptr;
> > > > >> +
> > > > >> +acb->buffer = buffer;
> > > > >> +}
> > > > >> +
> > > > >
> > > > > Unused function?
> > > > 
> > > > This is called from within libqnio.
> > > 
> > > Wait, you mean the library references a symbol in the qemu binary? This
> > > sounds completely wrong to me. I wouldn't even do something like this if
> > > it were an internal qemu library because I think libraries should be
> > > self-contained, but it's a much larger problem in something that is
> > > supposed to be an independent library.
> > 
> > I'd be surprised if that actually worked. If an external library wants
> > to refrence symbols in the QEMU binary, then QEMU would ned to be using
> > the -export-dynamic / -rdynamic linker flags to export all its symbols,
> > and AFAIK, we're not doing that.
> 
> Hm, but if the function is really used by the library, how else would it
> be when its name isn't mentioned anywhere in the patch except in its
> declaration? And it appears to be called there directly:
> 
> https://github.com/MittalAshish/libqnio/search?q=vxhs_set_acb_buffer
> 
> Anyway, something is fishy here.

Oh, notice that .c file is actually part of the shim library, not the
main libqnio.so that QEMU would link against.

So, it really is unused and should be deleted from this block/vxhs.c
file.

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 1/2] virtio-9p: print error message and exit instead of BUG_ON()

[Greg Kurz]

On Thu, 8 Sep 2016 10:59:26 +0200
Cornelia Huck  wrote:

> On Wed, 07 Sep 2016 19:19:24 +0200
> Greg Kurz  wrote:
> 
> > Calling assert() really makes sense when hitting a genuine bug, which calls
> > for a fix in QEMU. However, when something goes wrong because the guest
> > sends a malformed message, it is better to write down a more meaningul
> > error message and exit.
> > 
> > Signed-off-by: Greg Kurz 
> > ---
> >  hw/9pfs/virtio-9p-device.c |   20 ++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)  
> 
> While this is an improvement over the current state, I don't think the
> guest should be able to kill qemu just by doing something stupid.
> 

Hi Connie,

I'm glad you're pointing this out... this was also my impression, but
since there are a bunch of sanity checks in the virtio code that cause
QEMU to exit (even recently added like 1e7aed70144b), I did not dare
stand up :)

> The right way to go is to mark the virtio device as broken and stop
> doing any processing until the guest resets it. I think Stefan had a
> patch series doing that for some base virtio errors, but I'd have to
> search for it.
> 

I'd be glad to have a look and try to address this issue.

Thanks !

--
Greg

Re: [Qemu-devel] [PATCH] virtio-gpu-pci: tag as not hotpluggable

[Marc-André Lureau]

On Thu, Sep 8, 2016 at 11:29 AM Gerd Hoffmann  wrote:

> We can't hotplug display adapters in qemu, tag virtio-gpu-pci
> accordingly (virtio-vga already has this).
>
> Signed-off-by: Gerd Hoffmann 
>

 Reviewed-by: Marc-André Lureau 

---
>  hw/display/virtio-gpu-pci.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
> index 34a724c..ef92c4a 100644
> --- a/hw/display/virtio-gpu-pci.c
> +++ b/hw/display/virtio-gpu-pci.c
> @@ -48,6 +48,7 @@ static void virtio_gpu_pci_class_init(ObjectClass
> *klass, void *data)
>
>  set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
>  dc->props = virtio_gpu_pci_properties;
> +dc->hotpluggable = false;
>  k->realize = virtio_gpu_pci_realize;
>  pcidev_k->class_id = PCI_CLASS_DISPLAY_OTHER;
>  }
> --
> 1.8.3.1
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH 1/2] virtio-9p: print error message and exit instead of BUG_ON()

[Cornelia Huck]

On Wed, 07 Sep 2016 19:19:24 +0200
Greg Kurz  wrote:

> Calling assert() really makes sense when hitting a genuine bug, which calls
> for a fix in QEMU. However, when something goes wrong because the guest
> sends a malformed message, it is better to write down a more meaningul
> error message and exit.
> 
> Signed-off-by: Greg Kurz 
> ---
>  hw/9pfs/virtio-9p-device.c |   20 ++--
>  1 file changed, 18 insertions(+), 2 deletions(-)

While this is an improvement over the current state, I don't think the
guest should be able to kill qemu just by doing something stupid.

The right way to go is to mark the virtio device as broken and stop
doing any processing until the guest resets it. I think Stefan had a
patch series doing that for some base virtio errors, but I'd have to
search for it.

Re: [Qemu-devel] [PATCH 1/2] virtio-9p: print error message and exit instead of BUG_ON()

[Greg Kurz]

On Thu, 08 Sep 2016 09:14:05 +0200
Markus Armbruster  wrote:

> Greg Kurz  writes:
> 
> > Calling assert() really makes sense when hitting a genuine bug, which calls
> > for a fix in QEMU. However, when something goes wrong because the guest
> > sends a malformed message, it is better to write down a more meaningul
> > error message and exit.
> >
> > Signed-off-by: Greg Kurz 
> > ---
> >  hw/9pfs/virtio-9p-device.c |   20 ++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > index 009b43f6d045..67059182645a 100644
> > --- a/hw/9pfs/virtio-9p-device.c
> > +++ b/hw/9pfs/virtio-9p-device.c
> > @@ -19,6 +19,7 @@
> >  #include "coth.h"
> >  #include "hw/virtio/virtio-access.h"
> >  #include "qemu/iov.h"
> > +#include "qemu/error-report.h"
> >  
> >  void virtio_9p_push_and_notify(V9fsPDU *pdu)
> >  {
> > @@ -35,6 +36,11 @@ void virtio_9p_push_and_notify(V9fsPDU *pdu)
> >  virtio_notify(VIRTIO_DEVICE(v), v->vq);
> >  }
> >  
> > +static void virtio_9p_error(const char *msg)
> > +{
> > +error_report("The virtio-9p driver in the guest has an issue: %s", 
> > msg);
> > +}
> > +
> >  static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
> >  {
> >  V9fsVirtioState *v = (V9fsVirtioState *)vdev;
> > @@ -56,13 +62,23 @@ static void handle_9p_output(VirtIODevice *vdev, 
> > VirtQueue *vq)
> >  break;
> >  }
> >  
> > -BUG_ON(elem->out_num == 0 || elem->in_num == 0);
> > +if (elem->out_num == 0) {
> > +virtio_9p_error("missing VirtFS request's header");
> > +exit(1);
> > +}  
> 
> Can the guest trigger this?
> 

Yes it can in theory if it pushes an empty buffer... but this "recent"
commit changed the outcome:

commit 1e7aed70144b4673fc26e73062064b6724795e5f
Author: Prasad J Pandit 
Date:   Wed Jul 27 21:07:56 2016 +0530

virtio: check vring descriptor buffer length

And now, the error is caught in virtqueue_map_desc():

if (!sz) {
error_report("virtio: zero sized buffers are not allowed");
exit(1);
}

So I guess we should keep the BUG_ON() then.

BTW, there are similar checks in virtio-blk and virtio-net leading to a QEMU
exit... which seem to be obsoleted by the above commit. I'll have a closer
look.

> > +if (elem->in_num == 0) {
> > +virtio_9p_error("missing VirtFS reply's header");
> > +exit(1);
> > +}  
> 
> Same question.
> 

Same answer. :)

> >  QEMU_BUILD_BUG_ON(sizeof out != 7);
> >  
> >  v->elems[pdu->idx] = elem;
> >  len = iov_to_buf(elem->out_sg, elem->out_num, 0,
> >   , sizeof out);
> > -BUG_ON(len != sizeof out);
> > +if (len != sizeof out) {
> > +virtio_9p_error("malformed VirtFS request");
> > +exit(1);
> > +}  
> 
> Same question.
> 

Here this is different: the guest can put a bogus len in the vring_desc
structure, and this doesn't get checked earlier.

> >  
> >  pdu->size = le32_to_cpu(out.size_le);
> >

Cheers.

--
Greg

[Qemu-devel] [PATCH V14 04/12] Jhash: add linux kernel jhashtable in qemu

[Zhang Chen]

Jhash will be used by colo-compare and filter-rewriter
to save and lookup net connection info

Signed-off-by: Zhang Chen 
Signed-off-by: Li Zhijian 
Signed-off-by: Wen Congyang 
---
 include/qemu/jhash.h | 59 
 net/colo.h   |  1 +
 2 files changed, 60 insertions(+)
 create mode 100644 include/qemu/jhash.h

diff --git a/include/qemu/jhash.h b/include/qemu/jhash.h
new file mode 100644
index 000..742
--- /dev/null
+++ b/include/qemu/jhash.h
@@ -0,0 +1,59 @@
+/* jhash.h: Jenkins hash support.
+  *
+  * Copyright (C) 2006. Bob Jenkins (bob_jenk...@burtleburtle.net)
+  *
+  * http://burtleburtle.net/bob/hash/
+  *
+  * These are the credits from Bob's sources:
+  *
+  * lookup3.c, by Bob Jenkins, May 2006, Public Domain.
+  *
+  * These are functions for producing 32-bit hashes for hash table lookup.
+  * hashword(), hashlittle(), hashlittle2(), hashbig(), mix(), and final()
+  * are externally useful functions.  Routines to test the hash are included
+  * if SELF_TEST is defined.  You can use this free for any purpose. It's in
+  * the public domain.  It has no warranty.
+  *
+  * Copyright (C) 2009-2010 Jozsef Kadlecsik (kad...@blackhole.kfki.hu)
+  *
+  * I've modified Bob's hash to be useful in the Linux kernel, and
+  * any bugs present are my fault.
+  * Jozsef
+  */
+
+#ifndef QEMU_JHASH_H__
+#define QEMU_JHASH_H__
+
+#include "qemu/bitops.h"
+
+/*
+ * hashtable relation copy from linux kernel jhash
+ */
+
+/* __jhash_mix -- mix 3 32-bit values reversibly. */
+#define __jhash_mix(a, b, c)\
+{   \
+a -= c;  a ^= rol32(c, 4);  c += b; \
+b -= a;  b ^= rol32(a, 6);  a += c; \
+c -= b;  c ^= rol32(b, 8);  b += a; \
+a -= c;  a ^= rol32(c, 16); c += b; \
+b -= a;  b ^= rol32(a, 19); a += c; \
+c -= b;  c ^= rol32(b, 4);  b += a; \
+}
+
+/* __jhash_final - final mixing of 3 32-bit values (a,b,c) into c */
+#define __jhash_final(a, b, c)  \
+{   \
+c ^= b; c -= rol32(b, 14);  \
+a ^= c; a -= rol32(c, 11);  \
+b ^= a; b -= rol32(a, 25);  \
+c ^= b; c -= rol32(b, 16);  \
+a ^= c; a -= rol32(c, 4);   \
+b ^= a; b -= rol32(a, 14);  \
+c ^= b; c -= rol32(b, 24);  \
+}
+
+/* An arbitrary initial parameter */
+#define JHASH_INITVAL   0xdeadbeef
+
+#endif /* QEMU_JHASH_H__ */
diff --git a/net/colo.h b/net/colo.h
index e211eda..05dc0b6 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -16,6 +16,7 @@
 #define QEMU_COLO_PROXY_H
 
 #include "slirp/slirp.h"
+#include "qemu/jhash.h"
 
 #define HASHTABLE_MAX_SIZE 16384
 
-- 
2.7.4

[Qemu-devel] [PATCH V14 00/12] Introduce COLO-Proxy

[Zhang Chen]

COLO-proxy is a part of COLO project. COLO project is
composed of COLO-frame, COLO-proxy and block-replication.
It is used to compare the network package to help COLO
decide whether to do checkpoint. With COLO-proxy's help,
COLO greatly improves the performance.

The filter-redirector, filter-mirror, colo-compare
and filter-rewriter compose the COLO-proxy.

COLO-compare
It is used to compare the network package to help COLO decide
whether to do checkpoint. 

Filter-rewriter
It will rewrite some of secondary packet to make
secondary guest's connection established successfully.
In this module we will rewrite tcp packet's ack to the secondary
from primary,and rewrite tcp packet's seq to the primary from
secondary.

The full version in this github:
https://github.com/zhangckid/qemu/tree/colo-v2.7-proxy-mode-compare-and-rewriter-sep8

v14:
  - fix some coding style problems
  - move patch "add docs/colo-proxy.txt" to the end of this series
  - fix the mingw build issue

v13:
  - add docs/colo-proxy.txt
  - add MAINTAINERS
  - remove unnecessary .h
  - remove QTAILQ_ENTRY(CompareState)
  - fix some comments
  - add find_and_check_chardev() to avoid code duplication
  - remove the "is_unix" in patch3
  - change error_report() to trace in patch4
  - use l2hdr_len here instead of ETH_HLEP
  - fix code style
  - remove colo_rm_connection()
  - remove hashtable_size
  - change g_queue_foreach() to g_queue_find_custom() in patch7
  - change trace_colo_compare_tcp_miscompare() to fprintf() in patch8
  - add codes not queue vlan packets

v12:
  - add qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext
to this series as the first patch.
  - update COLO net ascii figure.
  - add chardev socket check.
  - fix some typo.
  - add some comments.
  - rename net/colo-base.c to net/colo.c
  - rename network/transport_layer to network/transport_header.
  - move the job that clear coon_list when hashtable_size oversize
to connection_get.
  - reuse connection_destroy() do colo_rm_connection().
  - fix pkt mem leak in colo_compare_connection().
(result be released in g_queue_remove(), so it were not leak)
  - rename thread_name "compare" to "colo-compare".
  - change icmp compare to memcmp().

v11:
  - Make patch 5 to a independent patch series.
[PATCH V3] qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext
  - For Jason's comments, merge filter-rewriter to this series.
(patch 7,8,9)
  - Add reverse_connection_key()
  - remove conn_list in filter-rewriter
  - remove unprocessed_connections
  - add some comments

v10:
  - fix typo
  - Should we make patch 5 independent with this series?
This patch just add a API for qemu-char.

v9:
 p5:
  - use chr_update_read_handler_full() replace
the chr_update_read_handler()
  - use io_watch_poll_prepare_full() replace
the io_watch_poll_prepare()
  - use io_watch_poll_funcs_full replace
the io_watch_poll_funcs
  - avoid code duplication

v8:
 p5:
  - add new patch:
qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext

v7:
 p5:
   - add [PATCH]qemu-char: Fix context for g_source_attach()
 in this patch series.

v6: 
 p6:
   - add more commit log.
   - fix icmp comparison to compare all packet.

 p5:
   - add more cpmments in commit log.
   - change REGULAR_CHECK_MS to REGULAR_PACKET_CHECK_MS
   - make check old packet independent to compare thread
   - remove thread_status

 p4:
   - change this patch only about
 Connection and ConnectionKey.
   - add some comments in commit log.
   - remove mode in fill_connection_key().
   - fix some comments and bug.
   - move colo_conn_state to patch of
 "work with colo-frame"
   - remove conn_list_lock.
   - add MAX_QUEUE_SIZE, if primary_list or
 secondary_list biger than MAX_QUEUE_SIZE
 we will drop packet. 

 p3:
   - add new independent kernel jhash patch.

 p2:
   - add new independent colo-base patch.

 p1:
   - add a ascii figure and some comments to explain it
   - move trace.h to p2
   - move QTAILQ_HEAD(, CompareState) net_compares to
 patch of "work with colo-frame"
   - add some comments in qemu-option.hx


v5:
 p3:
- comments from Jason
  we poll and handle chardev in comapre thread,
  Through this way, there's no need for extra 
  synchronization with main loop
  this depend on another patch:
  qemu-char: Fix context for g_source_attach()
- remove QemuEvent
 p2:
- remove conn->list_lock
 p1:
- move compare_pri/sec_chr_in to p3
- move compare_chr_send to p2

v4:
 p4:
- add some comments
- fix some trace-events
- fix tcp compare error
 p3:
- add rcu_read_lock().
- fix trace name
- fix jason's other comments
- rebase some Dave's branch function
 p2:
- colo_compare_connection() change g_queue_push_head() to
- g_queue_push_tail() match to sorted order.
- remove pkt->s
- move data structure to colo-base.h
- add colo-base.c reuse codes for filter-rewriter
- add some 

Re: [Qemu-devel] [PATCH 2/3] memory: add iommu_notify_flag

[Peter Xu]

On Wed, Sep 07, 2016 at 04:41:54PM +1000, David Gibson wrote:
> On Wed, Sep 07, 2016 at 02:34:19PM +0800, Peter Xu wrote:
> > On Wed, Sep 07, 2016 at 03:44:19PM +1000, David Gibson wrote:
> > > > For "CHANGE", it sounds like a unmap() + a map(). However I'd say
> > > > "ADDITION" is nowhere better...
> > > 
> > > Right.. this brings up a good point.
> > > 
> > > Changing a mapping (i.e. overwriting an existing mapping with a
> > > different one) would also need notification, even on x86, no?  Since
> > > it implicitly invalidates the previous mapping.
> > > 
> > > I'm guessing the guest will avoid this by always unmapping before it
> > > maps.  We still need to consider this possibility when designing the
> > > notifier interface though.
> > > 
> > > It seems the real notification triggers here are:
> > > * map - something is mapped which previously wasn't
> > > * unmap - something is no longer mapped which was before
> > > 
> > > Note that whether the second needs to be triggered depends on the
> > > *previous* state of that IOBA range, *not* on the permissions of the
> > > new mapping (if any).
> > > 
> > > A "change" - replacing one mapping with another should count as both a
> > > "map" and "unmap" event.
> > 
> > Yeah... For MAP/UNMAP, it is strange in another way: e.g. for vhost,
> > it doesn't care about map/unmap, it cares about invalidated cache.
> 
> I think caring about invalidated cache *is* caring about unmap.  It
> doesn't matter whether the new mapping is something or nothing - if
> the old mapping is no longer valid, you need to invalidate the cache,
> yes?

Yes, I think these two are exactly the same in implementation (vhost
needs UNMAP events of course). So that's why I called it "a naming
issue". :)

> 
> > So
> > IIUC this is a question about "naming" but not the implementations...
> > I suppose it is really a matter of taste, and both work for me (either
> > INVALIDATION/CHANGE or UNMAP/MAP).
> 
> No.. it is a question of implementation.  My point is that I don't
> think the new permission is sufficient information to let you know if
> a notification is necessary.  You need to know if there was an
> existing mapping at that IOBA.

My understanding is that we don't need to know that. Because IIUC
there are only map_page() and unmap_page() in guest IOMMU driver
(please check dma_map_ops in kernel). There is no chance for anyone to
"change" the content of the mapping, unless it calls unmap_page() then
with a map_page(). In that case, we'll have two IOTLB invalidation
requests.

Please kindly correct me if I am wrong.

Thanks,

-- peterx

Re: [Qemu-devel] [PATCH] memory: Use standard log messages for unassigned memory read / write

[Paolo Bonzini]

On 08/09/2016 07:43, Stefan Weil wrote:
> Am 07.09.2016 um 23:29 schrieb Peter Maydell:
>> On 7 September 2016 at 20:13, Stefan Weil  wrote:
>>> The old log messages are implemented by conditional compilation
>>> and not available by default.
>>>
>>> The new log messages can be enabled either by a command line option
>>> (-d unimp) or in the QEMU monitor (log unimp).
>>>
>>> Signed-off-by: Stefan Weil 
>>> ---
>>>
>>> The new code is very useful when implementing new platforms or
>>> looking for problems with existing platforms which are only
>>> partially emulated (I use it for Raspberry Pi).
>>>
>>> target-sparc/ldst_helper.c also uses DEBUG_UNASSIGNED
>>> and could get similar code.
>>>
>>> Regards,
>>> Stefan
>>>
>>>
>>>  memory.c | 12 
>>>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> [...]
> 
>>> -#ifdef DEBUG_UNASSIGNED
>>> -printf("Unassigned mem write " TARGET_FMT_plx " =
>>> 0x%"PRIx64"\n", addr, val);
>>> -#endif
>>> +qemu_log_mask(LOG_UNIMP, "%s " TARGET_FMT_plx " = 0x%" PRIx64 "\n",
>>> +  __func__, addr, val);
>>
>> Maybe this should be LOG_GUEST_ERROR rather than LOG_UNIMP ?
> 
> My first code used LOG_GUEST_ERROR. Of course a guest can try to access
> memory which is not available for the real hardware.
> 
> For my test object (Raspberry Pi), all unassigned memory accesses were
> not guest errors but unimplemented emulation in QEMU – that's why I
> changed from LOG_GUEST_ERROR to LOG_UNIMP.
> 
> A short test with i386 / x86_64 emulation also detected unassigned
> memory accesses (from BIOS) caused by an unimplemented debug device, so
> that was also not a guest error.

FWIW, the debug device is optional (-device isa-debugcon), not
unimplemented.  Similarly it's common, or at least it was before ACPI
was introduced, to poke at known ISA ports to detect legacy devices;
it's neither unimplemented nor a guest error in that case.

Perhaps it should just be a tracepoint?

Paolo

[Qemu-devel] [PATCH V14 10/12] filter-rewriter: rewrite tcp packet to keep secondary connection

[Zhang Chen]

We will rewrite tcp packet secondary received and sent.
When colo guest is a tcp server.

Firstly, client start a tcp handshake. the packet's seq=client_seq,
ack=0,flag=SYN. COLO primary guest get this pkt and mirror(filter-mirror)
to secondary guest, secondary get it use filter-redirector.
Then,primary guest response pkt
(seq=primary_seq,ack=client_seq+1,flag=ACK|SYN).
secondary guest response pkt
(seq=secondary_seq,ack=client_seq+1,flag=ACK|SYN).
In here,we use filter-rewriter save the secondary_seq to it's tcp connection.
Finally handshake,client send pkt
(seq=client_seq+1,ack=primary_seq+1,flag=ACK).
Here,filter-rewriter can get primary_seq, and rewrite ack from primary_seq+1
to secondary_seq+1, recalculate checksum. So the secondary tcp connection
kept good.

When we send/recv packet.
client send pkt(seq=client_seq+1+data_len,ack=primary_seq+1,flag=ACK|PSH).
filter-rewriter rewrite ack and send to secondary guest.

primary guest response pkt
(seq=primary_seq+1,ack=client_seq+1+data_len,flag=ACK)
secondary guest response pkt
(seq=secondary_seq+1,ack=client_seq+1+data_len,flag=ACK)
we rewrite secondary guest seq from secondary_seq+1 to primary_seq+1.
So tcp connection kept good.

In code We use offset( = secondary_seq - primary_seq )
to rewrite seq or ack.
handle_primary_tcp_pkt: tcp_pkt->th_ack += offset;
handle_secondary_tcp_pkt: tcp_pkt->th_seq -= offset;

Signed-off-by: Zhang Chen 
Signed-off-by: Li Zhijian 
Signed-off-by: Wen Congyang 
---
 net/colo.c|   2 +
 net/colo.h|   7 
 net/filter-rewriter.c | 112 +-
 trace-events  |   5 +++
 4 files changed, 124 insertions(+), 2 deletions(-)

diff --git a/net/colo.c b/net/colo.c
index dc4e4a4..9f469e6 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -135,6 +135,8 @@ Connection *connection_new(ConnectionKey *key)
 
 conn->ip_proto = key->ip_proto;
 conn->processing = false;
+conn->offset = 0;
+conn->syn_flag = 0;
 g_queue_init(>primary_list);
 g_queue_init(>secondary_list);
 
diff --git a/net/colo.h b/net/colo.h
index 6720a3a..7c524f3 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -62,6 +62,13 @@ typedef struct Connection {
 /* flag to enqueue unprocessed_connections */
 bool processing;
 uint8_t ip_proto;
+/* offset = secondary_seq - primary_seq */
+tcp_seq  offset;
+/*
+ * we use this flag update offset func
+ * run once in independent tcp connection
+ */
+int syn_flag;
 } Connection;
 
 uint32_t connection_key_hash(const void *opaque);
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index 1d49d04..cd0dc54 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -10,6 +10,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "trace.h"
 #include "net/colo.h"
 #include "net/filter.h"
 #include "net/net.h"
@@ -58,6 +59,93 @@ static int is_tcp_packet(Packet *pkt)
 }
 }
 
+/* handle tcp packet from primary guest */
+static int handle_primary_tcp_pkt(NetFilterState *nf,
+  Connection *conn,
+  Packet *pkt)
+{
+struct tcphdr *tcp_pkt;
+
+tcp_pkt = (struct tcphdr *)pkt->transport_header;
+if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
+char *sdebug, *ddebug;
+sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
+ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
+trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
+ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
+tcp_pkt->th_flags);
+trace_colo_filter_rewriter_conn_offset(conn->offset);
+g_free(sdebug);
+g_free(ddebug);
+}
+
+if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
+/*
+ * we use this flag update offset func
+ * run once in independent tcp connection
+ */
+conn->syn_flag = 1;
+}
+
+if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK)) {
+if (conn->syn_flag) {
+/*
+ * offset = secondary_seq - primary seq
+ * ack packet sent by guest from primary node,
+ * so we use th_ack - 1 get primary_seq
+ */
+conn->offset -= (ntohl(tcp_pkt->th_ack) - 1);
+conn->syn_flag = 0;
+}
+/* handle packets to the secondary from the primary */
+tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset);
+
+net_checksum_calculate((uint8_t *)pkt->data, pkt->size);
+}
+
+return 0;
+}
+
+/* handle tcp packet from secondary guest */
+static int handle_secondary_tcp_pkt(NetFilterState *nf,
+Connection *conn,
+Packet *pkt)
+{
+struct tcphdr *tcp_pkt;
+
+tcp_pkt = (struct tcphdr *)pkt->transport_header;
+
+if 

Re: [Qemu-devel] [PATCH 2/2] virtio-pci: error out when both legacy and modern modes are disabled

[Greg Kurz]

On Thu, 08 Sep 2016 09:15:28 +0200
Markus Armbruster  wrote:

> Greg Kurz  writes:
> 
> > From: Greg Kurz 
> >
> > Without presuming if we got there because of a user mistake or some
> > more subtle bug in the tooling, it really does not make sense to
> > implement a non-functional device.
> >
> > Signed-off-by: Greg Kurz 
> > Reviewed-by: Marcel Apfelbaum 
> > Reviewed-by: Cornelia Huck 
> > Signed-off-by: Greg Kurz 
> > ---
> >  hw/virtio/virtio-pci.c |8 
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 268fd8ebb219..4b6a8a356621 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -1842,6 +1842,14 @@ static void virtio_pci_dc_realize(DeviceState *qdev, 
> > Error **errp)
> >  VirtIOPCIProxy *proxy = VIRTIO_PCI(qdev);
> >  PCIDevice *pci_dev = >pci_dev;
> >  
> > +if (!(virtio_pci_modern(proxy) || virtio_pci_legacy(proxy))) {
> > +error_setg(errp, "device cannot work as neither modern nor legacy 
> > mode"
> > +   " is enabled");
> > +error_append_hint(errp, "Set either disable-modern or 
> > disable-legacy"
> > +  " to off\n");
> > +return;
> > +}
> > +
> >  if (!(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_PCIE) &&
> >  virtio_pci_modern(proxy)) {
> >  pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;  
> 
> Pardon my ignorance... is this a device-specific restriction, or is it
> the same for more (all?) virtio devices?

Yes this for all virtio devices.

But wait, I now realize that the check isn't done in the right place because
if we don't pass an explicit disable-legacy=on then proxy->disable_legacy gets
its final value in virtio_pci_realize() which gets called after 
virtio_pci_dc_realize()...

I'll fix that.

Thanks !

--
Greg

[Qemu-devel] [PATCH V14 06/12] colo-compare: introduce packet comparison thread

[Zhang Chen]

If primary packet is same with secondary packet,
we will send primary packet and drop secondary
packet, otherwise notify COLO frame to do checkpoint.
If primary packet comes but secondary packet does not,
after REGULAR_PACKET_CHECK_MS milliseconds we set
the primary packet as old_packet,then do a checkpoint.

Signed-off-by: Zhang Chen 
Signed-off-by: Li Zhijian 
Signed-off-by: Wen Congyang 
---
 net/colo-compare.c | 233 +
 net/colo.c |   1 +
 net/colo.h |   3 +
 trace-events   |   2 +
 4 files changed, 239 insertions(+)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 231654c..645126e 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -33,8 +33,12 @@
 #define COLO_COMPARE(obj) \
 OBJECT_CHECK(CompareState, (obj), TYPE_COLO_COMPARE)
 
+#define COMPARE_READ_LEN_MAX NET_BUFSIZE
 #define MAX_QUEUE_SIZE 1024
 
+/* TODO: Should be configurable */
+#define REGULAR_PACKET_CHECK_MS 3000
+
 /*
   + CompareState ++
   |   |
@@ -76,6 +80,11 @@ typedef struct CompareState {
 GQueue conn_list;
 /* hashtable to save connection */
 GHashTable *connection_track_table;
+/* compare thread, a thread for each NIC */
+QemuThread thread;
+/* Timer used on the primary to find packets that are never matched */
+QEMUTimer *timer;
+QemuMutex timer_check_lock;
 } CompareState;
 
 typedef struct CompareClass {
@@ -148,6 +157,118 @@ static int packet_enqueue(CompareState *s, int mode)
 return 0;
 }
 
+/*
+ * The IP packets sent by primary and secondary
+ * will be compared in here
+ * TODO support ip fragment, Out-Of-Order
+ * return:0  means packet same
+ *> 0 || < 0 means packet different
+ */
+static int colo_packet_compare(Packet *ppkt, Packet *spkt)
+{
+trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
+   inet_ntoa(ppkt->ip->ip_dst), spkt->size,
+   inet_ntoa(spkt->ip->ip_src),
+   inet_ntoa(spkt->ip->ip_dst));
+
+if (ppkt->size == spkt->size) {
+return memcmp(ppkt->data, spkt->data, spkt->size);
+} else {
+return -1;
+}
+}
+
+static int colo_packet_compare_all(Packet *spkt, Packet *ppkt)
+{
+trace_colo_compare_main("compare all");
+return colo_packet_compare(ppkt, spkt);
+}
+
+static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
+{
+int64_t now = qemu_clock_get_ms(QEMU_CLOCK_HOST);
+
+if ((now - pkt->creation_ms) > (*check_time)) {
+trace_colo_old_packet_check_found(pkt->creation_ms);
+return 0;
+} else {
+return 1;
+}
+}
+
+static void colo_old_packet_check_one_conn(void *opaque,
+   void *user_data)
+{
+Connection *conn = opaque;
+GList *result = NULL;
+int64_t check_time = REGULAR_PACKET_CHECK_MS;
+
+result = g_queue_find_custom(>primary_list,
+ _time,
+ (GCompareFunc)colo_old_packet_check_one);
+
+if (result) {
+/* do checkpoint will flush old packet */
+/* TODO: colo_notify_checkpoint();*/
+}
+}
+
+/*
+ * Look for old packets that the secondary hasn't matched,
+ * if we have some then we have to checkpoint to wake
+ * the secondary up.
+ */
+static void colo_old_packet_check(void *opaque)
+{
+CompareState *s = opaque;
+
+g_queue_foreach(>conn_list, colo_old_packet_check_one_conn, NULL);
+}
+
+/*
+ * Called from the compare thread on the primary
+ * for compare connection
+ */
+static void colo_compare_connection(void *opaque, void *user_data)
+{
+CompareState *s = user_data;
+Connection *conn = opaque;
+Packet *pkt = NULL;
+GList *result = NULL;
+int ret;
+
+while (!g_queue_is_empty(>primary_list) &&
+   !g_queue_is_empty(>secondary_list)) {
+qemu_mutex_lock(>timer_check_lock);
+pkt = g_queue_pop_tail(>primary_list);
+qemu_mutex_unlock(>timer_check_lock);
+result = g_queue_find_custom(>secondary_list,
+  pkt, (GCompareFunc)colo_packet_compare_all);
+
+if (result) {
+ret = compare_chr_send(s->chr_out, pkt->data, pkt->size);
+if (ret < 0) {
+error_report("colo_send_primary_packet failed");
+}
+trace_colo_compare_main("packet same and release packet");
+g_queue_remove(>secondary_list, result->data);
+packet_destroy(pkt, NULL);
+} else {
+/*
+ * If one packet arrive late, the secondary_list or
+ * primary_list will be empty, so we can't compare it
+ * until next comparison.
+ */
+trace_colo_compare_main("packet different");
+qemu_mutex_lock(>timer_check_lock);
+

Re: [Qemu-devel] [PATCH v4 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support

[Daniel P. Berrange]

On Thu, Sep 08, 2016 at 11:29:41AM +0200, Kevin Wolf wrote:
> Am 08.09.2016 um 10:49 hat Daniel P. Berrange geschrieben:
> > On Thu, Sep 08, 2016 at 10:43:19AM +0200, Kevin Wolf wrote:
> > > Am 08.09.2016 um 00:32 hat ashish mittal geschrieben:
> > > > >> +
> > > > >> +void vxhs_set_acb_buffer(void *ptr, void *buffer)
> > > > >> +{
> > > > >> +VXHSAIOCB *acb = ptr;
> > > > >> +
> > > > >> +acb->buffer = buffer;
> > > > >> +}
> > > > >> +
> > > > >
> > > > > Unused function?
> > > > 
> > > > This is called from within libqnio.
> > > 
> > > Wait, you mean the library references a symbol in the qemu binary? This
> > > sounds completely wrong to me. I wouldn't even do something like this if
> > > it were an internal qemu library because I think libraries should be
> > > self-contained, but it's a much larger problem in something that is
> > > supposed to be an independent library.
> > 
> > I'd be surprised if that actually worked. If an external library wants
> > to refrence symbols in the QEMU binary, then QEMU would ned to be using
> > the -export-dynamic / -rdynamic linker flags to export all its symbols,
> > and AFAIK, we're not doing that.
> 
> Hm, but if the function is really used by the library, how else would it
> be when its name isn't mentioned anywhere in the patch except in its
> declaration? And it appears to be called there directly:
> 
> https://github.com/MittalAshish/libqnio/search?q=vxhs_set_acb_buffer
> 
> Anyway, something is fishy here.

Yeah, and as you say, this approach is just plain wrong, and absolutely
can never be merged as is. The external library must never use anything
in QEMU that it wasn't explicitly given via a callback, otherwise that
library is defacto part of QEMU.

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 :|

[Qemu-devel] [PATCH V14 07/12] colo-compare: add TCP, UDP, ICMP packet comparison

[Zhang Chen]

We add TCP,UDP,ICMP packet comparison to replace
IP packet comparison. This can increase the
accuracy of the package comparison.
Less checkpoint more efficiency.

Signed-off-by: Zhang Chen 
Signed-off-by: Li Zhijian 
Signed-off-by: Wen Congyang 
---
 net/colo-compare.c | 147 +++--
 trace-events   |   3 ++
 2 files changed, 146 insertions(+), 4 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 645126e..3328515 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -19,6 +19,7 @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/error.h"
 #include "net/net.h"
+#include "net/eth.h"
 #include "qom/object_interfaces.h"
 #include "qemu/iov.h"
 #include "qom/object.h"
@@ -178,9 +179,131 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
 }
 }
 
-static int colo_packet_compare_all(Packet *spkt, Packet *ppkt)
+/*
+ * Called from the compare thread on the primary
+ * for compare tcp packet
+ * compare_tcp copied from Dr. David Alan Gilbert's branch
+ */
+static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
+{
+struct tcphdr *ptcp, *stcp;
+int res;
+char *sdebug, *ddebug;
+
+trace_colo_compare_main("compare tcp");
+if (ppkt->size != spkt->size) {
+if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+trace_colo_compare_main("pkt size not same");
+}
+return -1;
+}
+
+ptcp = (struct tcphdr *)ppkt->transport_header;
+stcp = (struct tcphdr *)spkt->transport_header;
+
+/*
+ * The 'identification' field in the IP header is *very* random
+ * it almost never matches.  Fudge this by ignoring differences in
+ * unfragmented packets; they'll normally sort themselves out if different
+ * anyway, and it should recover at the TCP level.
+ * An alternative would be to get both the primary and secondary to rewrite
+ * somehow; but that would need some sync traffic to sync the state
+ */
+if (ntohs(ppkt->ip->ip_off) & IP_DF) {
+spkt->ip->ip_id = ppkt->ip->ip_id;
+/* and the sum will be different if the IDs were different */
+spkt->ip->ip_sum = ppkt->ip->ip_sum;
+}
+
+res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
+(spkt->size - ETH_HLEN));
+
+if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+sdebug = strdup(inet_ntoa(ppkt->ip->ip_src));
+ddebug = strdup(inet_ntoa(ppkt->ip->ip_dst));
+fprintf(stderr, "%s: src/dst: %s/%s p: seq/ack=%u/%u"
+" s: seq/ack=%u/%u res=%d flags=%x/%x\n",
+__func__, sdebug, ddebug,
+(unsigned int)ntohl(ptcp->th_seq),
+(unsigned int)ntohl(ptcp->th_ack),
+(unsigned int)ntohl(stcp->th_seq),
+(unsigned int)ntohl(stcp->th_ack),
+res, ptcp->th_flags, stcp->th_flags);
+
+fprintf(stderr, "Primary len = %d\n", ppkt->size);
+qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
+fprintf(stderr, "Secondary len = %d\n", spkt->size);
+qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
+
+g_free(sdebug);
+g_free(ddebug);
+}
+
+return res;
+}
+
+/*
+ * Called from the compare thread on the primary
+ * for compare udp packet
+ */
+static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
+{
+int ret;
+
+trace_colo_compare_main("compare udp");
+ret = colo_packet_compare(ppkt, spkt);
+
+if (ret) {
+trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
+qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
+trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
+qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
+}
+
+return ret;
+}
+
+/*
+ * Called from the compare thread on the primary
+ * for compare icmp packet
+ */
+static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
 {
-trace_colo_compare_main("compare all");
+int network_length;
+
+trace_colo_compare_main("compare icmp");
+network_length = ppkt->ip->ip_hl * 4;
+if (ppkt->size != spkt->size ||
+ppkt->size < network_length + ETH_HLEN) {
+return -1;
+}
+
+if (colo_packet_compare(ppkt, spkt)) {
+trace_colo_compare_icmp_miscompare("primary pkt size",
+   ppkt->size);
+qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
+ ppkt->size);
+trace_colo_compare_icmp_miscompare("Secondary pkt size",
+   spkt->size);
+qemu_hexdump((char *)spkt->data, stderr, "colo-compare",
+ spkt->size);
+return -1;
+} else {
+return 0;
+}
+}
+
+/*
+ * 

Re: [Qemu-devel] [PATCH v4 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support

[Kevin Wolf]

Am 08.09.2016 um 11:43 hat Daniel P. Berrange geschrieben:
> On Thu, Sep 08, 2016 at 11:29:41AM +0200, Kevin Wolf wrote:
> > Am 08.09.2016 um 10:49 hat Daniel P. Berrange geschrieben:
> > > On Thu, Sep 08, 2016 at 10:43:19AM +0200, Kevin Wolf wrote:
> > > > Am 08.09.2016 um 00:32 hat ashish mittal geschrieben:
> > > > > >> +
> > > > > >> +void vxhs_set_acb_buffer(void *ptr, void *buffer)
> > > > > >> +{
> > > > > >> +VXHSAIOCB *acb = ptr;
> > > > > >> +
> > > > > >> +acb->buffer = buffer;
> > > > > >> +}
> > > > > >> +
> > > > > >
> > > > > > Unused function?
> > > > > 
> > > > > This is called from within libqnio.
> > > > 
> > > > Wait, you mean the library references a symbol in the qemu binary? This
> > > > sounds completely wrong to me. I wouldn't even do something like this if
> > > > it were an internal qemu library because I think libraries should be
> > > > self-contained, but it's a much larger problem in something that is
> > > > supposed to be an independent library.
> > > 
> > > I'd be surprised if that actually worked. If an external library wants
> > > to refrence symbols in the QEMU binary, then QEMU would ned to be using
> > > the -export-dynamic / -rdynamic linker flags to export all its symbols,
> > > and AFAIK, we're not doing that.
> > 
> > Hm, but if the function is really used by the library, how else would it
> > be when its name isn't mentioned anywhere in the patch except in its
> > declaration? And it appears to be called there directly:
> > 
> > https://github.com/MittalAshish/libqnio/search?q=vxhs_set_acb_buffer
> > 
> > Anyway, something is fishy here.
> 
> Oh, notice that .c file is actually part of the shim library, not the
> main libqnio.so that QEMU would link against.
> 
> So, it really is unused and should be deleted from this block/vxhs.c
> file.

I haven't fully understood yet what's the deal with this shim library,
but this patch links against both.

Kevin

[Qemu-devel] [PATCH v9 2/2] virtio-crypto: Add conformance clauses

[Gonglei]

Add the conformance targets and clauses for
virtio-crypto device.

Signed-off-by: Gonglei 
---
 conformance.tex | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/conformance.tex b/conformance.tex
index f59e360..039f44e 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -146,6 +146,21 @@ An SCSI host driver MUST conform to the following 
normative statements:
 \item \ref{drivernormative:Device Types / SCSI Host Device / Device Operation 
/ Device Operation: eventq}
 \end{itemize}
 
+\subsection{Crypto Driver Conformance}\label{sec:Conformance / Driver 
Conformance / Crypto Driver Conformance}
+
+An Crypto driver MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{drivernormative:Device Types / Crypto Device / Device configuration 
layout / Driver Requirements: Device configuration layout}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / 
Session operation / Session operation: create session / Driver Requirements: 
Session operation: create session}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / 
Session operation / Session operation: destroy session / Driver Requirements: 
Session operation: destroy session}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / 
Crypto operation / Crypto operation: HASH operation / Driver Requirements: 
Crypto operation: HASH operation}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / 
Crypto operation / Crypto operation: MAC operation / Driver Requirements: 
Crypto operation: MAC operation}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / 
Crypto operation / Crypto operation: Symmetric algorithms operation / Driver 
Requirements: Crypto operation: Symmetric algorithms encryption}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / 
Crypto operation / Crypto operation: AEAD operation / Driver Requirements: 
Crypto operation: AEAD encryption}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / 
Crypto operation / Crypto operation: AEAD operation / Driver Requirements: 
Crypto operation: AEAD decryption}
+\end{itemize}
+
 \section{Device Conformance}\label{sec:Conformance / Device Conformance}
 
 A device MUST conform to the following normative statements:
@@ -267,6 +282,22 @@ An SCSI host device MUST conform to the following 
normative statements:
 \item \ref{devicenormative:Device Types / SCSI Host Device / Device Operation 
/ Device Operation: eventq}
 \end{itemize}
 
+\subsection{Crypto Device Conformance}\label{sec:Conformance / Device 
Conformance / Crypto Device Conformance}
+
+An Crypto device MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{devicenormative:Device Types / Crypto Device / Device configuration 
layout / Device Requirements: Device configuration layout}
+\item \ref{devicenormative:Device Types / Crypto Device / Device 
Initialization / Device Requirements: Device Initialization}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / 
Control Virtqueue / Session operation / Session operation: create session / 
Device Requirements: Session operation: create session}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / 
Control Virtqueue / Session operation / Session operation: destroy session / 
Device Requirements: Session operation: destroy session}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / 
Data Virtqueue / HASH Service operation / Device Requirements: Crypto 
operation: HASH operation}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / 
Data Virtqueue / MAC Service operation / Device Requirements: Crypto operation: 
MAC operation}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / 
Data Virtqueue / Symmetric algorithms Operation / Device Requirements: Crypto 
operation: Symmetric algorithms Encryption}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / 
Data Virtqueue / AEAD Service operation / Device Requirements: AEAD Service 
Encryption}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / 
Data Virtqueue / AEAD Service operation / Device Requirements: AEAD Service 
Decryption}
+\end{itemize}
+
 \section{Legacy Interface: Transitional Device and
 Transitional Driver Conformance}\label{sec:Conformance / Legacy
 Interface: Transitional Device and 
-- 
1.7.12.4

[Qemu-devel] [PATCH] docs/xbzrle: correction

[Cao jin]

1. Default cache size is 64MB.
2. Semantics correction.

Signed-off-by: Cao jin 
---
 docs/xbzrle.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/xbzrle.txt b/docs/xbzrle.txt
index 52c8511..c0a7dfd 100644
--- a/docs/xbzrle.txt
+++ b/docs/xbzrle.txt
@@ -42,7 +42,7 @@ nzrun = length byte...
 length = uleb128 encoded integer
 
 On the sender side XBZRLE is used as a compact delta encoding of page updates,
-retrieving the old page content from the cache (default size of 512 MB). The
+retrieving the old page content from the cache (default size of 64MB). The
 receiving side uses the existing page's content and XBZRLE to decode the new
 page's content.
 
@@ -73,7 +73,7 @@ e9 07 0f 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 03 01 
67 01 01 69
 
 Cache update strategy
 =
-Keeping the hot pages in the cache is effective for decreased cache
+Keeping the hot pages in the cache is effective for decreasing cache
 misses. XBZRLE uses a counter as the age of each page. The counter will
 increase after each ram dirty bitmap sync. When a cache conflict is
 detected, XBZRLE will only evict pages in the cache that are older than
-- 
2.1.0

[Qemu-devel] [PATCH] iothread: Stop threads before main() quits

[Fam Zheng]

Right after main_loop ends, we release various things but keep iothread
alive. The latter is not prepared to the sudden change of resources.

Specifically, after bdrv_close_all(), virtio-scsi dataplane get a
surprise at the empty BlockBackend:

(gdb) bt
at /usr/src/debug/qemu-2.6.0/hw/scsi/virtio-scsi.c:543
at /usr/src/debug/qemu-2.6.0/hw/scsi/virtio-scsi.c:577

It is because the d->conf.blk->root is set to NULL, then
blk_get_aio_context() returns qemu_aio_context, whereas s->ctx is still
pointing to the iothread:

hw/scsi/virtio-scsi.c:543:

if (s->dataplane_started) {
assert(blk_get_aio_context(d->conf.blk) == s->ctx);
}

To fix this, let's stop iothreads before doing bdrv_close_all().

Cc: qemu-sta...@nongnu.org
Signed-off-by: Fam Zheng 
---
 include/sysemu/iothread.h |  1 +
 iothread.c| 24 
 vl.c  |  2 ++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index 2eefea1..68ac2de 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -35,5 +35,6 @@ typedef struct {
 
 char *iothread_get_id(IOThread *iothread);
 AioContext *iothread_get_aio_context(IOThread *iothread);
+void iothread_stop_all(void);
 
 #endif /* IOTHREAD_H */
diff --git a/iothread.c b/iothread.c
index f183d38..fb08a60 100644
--- a/iothread.c
+++ b/iothread.c
@@ -54,16 +54,25 @@ static void *iothread_run(void *opaque)
 return NULL;
 }
 
-static void iothread_instance_finalize(Object *obj)
+static int iothread_stop(Object *object, void *opaque)
 {
-IOThread *iothread = IOTHREAD(obj);
+IOThread *iothread;
 
-if (!iothread->ctx) {
-return;
+iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
+if (!iothread || !iothread->ctx) {
+return 0;
 }
 iothread->stopping = true;
 aio_notify(iothread->ctx);
 qemu_thread_join(>thread);
+return 0;
+}
+
+static void iothread_instance_finalize(Object *obj)
+{
+IOThread *iothread = IOTHREAD(obj);
+
+iothread_stop(obj, NULL);
 qemu_cond_destroy(>init_done_cond);
 qemu_mutex_destroy(>init_done_lock);
 aio_context_unref(iothread->ctx);
@@ -174,3 +183,10 @@ IOThreadInfoList *qmp_query_iothreads(Error **errp)
 object_child_foreach(container, query_one_iothread, );
 return head;
 }
+
+void iothread_stop_all(void)
+{
+Object *container = object_get_objects_root();
+
+object_child_foreach(container, iothread_stop, NULL);
+}
diff --git a/vl.c b/vl.c
index ee557a1..6a218ce 100644
--- a/vl.c
+++ b/vl.c
@@ -121,6 +121,7 @@ int main(int argc, char **argv)
 #include "crypto/init.h"
 #include "sysemu/replay.h"
 #include "qapi/qmp/qerror.h"
+#include "sysemu/iothread.h"
 
 #define MAX_VIRTIO_CONSOLES 1
 #define MAX_SCLP_CONSOLES 1
@@ -4616,6 +4617,7 @@ int main(int argc, char **argv, char **envp)
 trace_init_vcpu_events();
 main_loop();
 replay_disable_events();
+iothread_stop_all();
 
 bdrv_close_all();
 pause_all_vcpus();
-- 
2.7.4

Re: [Qemu-devel] [PATCH] iothread: Stop threads before main() quits

[Paolo Bonzini]

On 08/09/2016 11:28, Fam Zheng wrote:
> Right after main_loop ends, we release various things but keep iothread
> alive. The latter is not prepared to the sudden change of resources.
> 
> Specifically, after bdrv_close_all(), virtio-scsi dataplane get a
> surprise at the empty BlockBackend:
> 
> (gdb) bt
> at /usr/src/debug/qemu-2.6.0/hw/scsi/virtio-scsi.c:543
> at /usr/src/debug/qemu-2.6.0/hw/scsi/virtio-scsi.c:577
> 
> It is because the d->conf.blk->root is set to NULL, then
> blk_get_aio_context() returns qemu_aio_context, whereas s->ctx is still
> pointing to the iothread:
> 
> hw/scsi/virtio-scsi.c:543:
> 
> if (s->dataplane_started) {
> assert(blk_get_aio_context(d->conf.blk) == s->ctx);
> }
> 
> To fix this, let's stop iothreads before doing bdrv_close_all().
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Fam Zheng 
> ---
>  include/sysemu/iothread.h |  1 +
>  iothread.c| 24 
>  vl.c  |  2 ++
>  3 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
> index 2eefea1..68ac2de 100644
> --- a/include/sysemu/iothread.h
> +++ b/include/sysemu/iothread.h
> @@ -35,5 +35,6 @@ typedef struct {
>  
>  char *iothread_get_id(IOThread *iothread);
>  AioContext *iothread_get_aio_context(IOThread *iothread);
> +void iothread_stop_all(void);
>  
>  #endif /* IOTHREAD_H */
> diff --git a/iothread.c b/iothread.c
> index f183d38..fb08a60 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -54,16 +54,25 @@ static void *iothread_run(void *opaque)
>  return NULL;
>  }
>  
> -static void iothread_instance_finalize(Object *obj)
> +static int iothread_stop(Object *object, void *opaque)
>  {
> -IOThread *iothread = IOTHREAD(obj);
> +IOThread *iothread;
>  
> -if (!iothread->ctx) {
> -return;
> +iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
> +if (!iothread || !iothread->ctx) {
> +return 0;
>  }
>  iothread->stopping = true;
>  aio_notify(iothread->ctx);
>  qemu_thread_join(>thread);
> +return 0;
> +}
> +
> +static void iothread_instance_finalize(Object *obj)
> +{
> +IOThread *iothread = IOTHREAD(obj);
> +
> +iothread_stop(obj, NULL);
>  qemu_cond_destroy(>init_done_cond);
>  qemu_mutex_destroy(>init_done_lock);
>  aio_context_unref(iothread->ctx);
> @@ -174,3 +183,10 @@ IOThreadInfoList *qmp_query_iothreads(Error **errp)
>  object_child_foreach(container, query_one_iothread, );
>  return head;
>  }
> +
> +void iothread_stop_all(void)
> +{
> +Object *container = object_get_objects_root();
> +
> +object_child_foreach(container, iothread_stop, NULL);
> +}
> diff --git a/vl.c b/vl.c
> index ee557a1..6a218ce 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -121,6 +121,7 @@ int main(int argc, char **argv)
>  #include "crypto/init.h"
>  #include "sysemu/replay.h"
>  #include "qapi/qmp/qerror.h"
> +#include "sysemu/iothread.h"
>  
>  #define MAX_VIRTIO_CONSOLES 1
>  #define MAX_SCLP_CONSOLES 1
> @@ -4616,6 +4617,7 @@ int main(int argc, char **argv, char **envp)
>  trace_init_vcpu_events();
>  main_loop();
>  replay_disable_events();
> +iothread_stop_all();
>  
>  bdrv_close_all();
>  pause_all_vcpus();
> 

Reviewed-by: Paolo Bonzini 

Paolo

[Qemu-devel] [PATCH V14 11/12] MAINTAINERS: add maintainer for COLO-proxy

[Zhang Chen]

add Zhang Chen and Li zhijian as co-maintainers of COLO-proxy.

Signed-off-by: Zhang Chen 
Signed-off-by: Li Zhijian 
Signed-off-by: Wen Congyang 
---
 MAINTAINERS | 9 +
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b6fb84e..4781f9f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1330,6 +1330,15 @@ F: include/qemu/throttle.h
 F: util/throttle.c
 L: qemu-bl...@nongnu.org
 
+COLO Proxy
+M: Zhang Chen 
+M: Li Zhijian 
+S: Supported
+F: docs/colo-proxy.txt
+F: net/colo*
+F: net/filter-rewriter.c
+F: net/filter-mirror.c
+
 Usermode Emulation
 --
 Overall
-- 
2.7.4

[Qemu-devel] [PATCH V14 05/12] colo-compare: track connection and enqueue packet

[Zhang Chen]

In this patch we use kernel jhash table to track
connection, and then enqueue net packet like this:

+ CompareState ++
|   |
+---+   +---+ +---+
|conn list  +--->conn   +->conn   |
+---+   +---+ +---+
|   | |   | |  |
+---+ +---v+  +---v++---v+ +---v+
  |primary |  |secondary|primary | |secondary
  |packet  |  |packet  +|packet  | |packet  +
  ++  ++++ ++
  |   | |  |
  +---v+  +---v++---v+ +---v+
  |primary |  |secondary|primary | |secondary
  |packet  |  |packet  +|packet  | |packet  +
  ++  ++++ ++
  |   | |  |
  +---v+  +---v++---v+ +---v+
  |primary |  |secondary|primary | |secondary
  |packet  |  |packet  +|packet  | |packet  +
  ++  ++++ ++

We use conn_list to record connection info.
When we want to enqueue a packet, firstly get the
connection from connection_track_table. then push
the packet to g_queue(pri/sec) in it's own conn.

Signed-off-by: Zhang Chen 
Signed-off-by: Li Zhijian 
Signed-off-by: Wen Congyang 
---
 net/colo-compare.c |  53 +-
 net/colo.c | 109 +
 net/colo.h |  39 +++
 3 files changed, 191 insertions(+), 10 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index cea9b27..231654c 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -33,6 +33,8 @@
 #define COLO_COMPARE(obj) \
 OBJECT_CHECK(CompareState, (obj), TYPE_COLO_COMPARE)
 
+#define MAX_QUEUE_SIZE 1024
+
 /*
   + CompareState ++
   |   |
@@ -67,6 +69,11 @@ typedef struct CompareState {
 SocketReadState pri_rs;
 SocketReadState sec_rs;
 
+/* connection list: the connections belonged to this NIC could be found
+ * in this list.
+ * element type: Connection
+ */
+GQueue conn_list;
 /* hashtable to save connection */
 GHashTable *connection_track_table;
 } CompareState;
@@ -94,7 +101,9 @@ static int compare_chr_send(CharDriverState *out,
  */
 static int packet_enqueue(CompareState *s, int mode)
 {
+ConnectionKey key = { 0 };
 Packet *pkt = NULL;
+Connection *conn;
 
 if (mode == PRIMARY_IN) {
 pkt = packet_new(s->pri_rs.buf, s->pri_rs.packet_len);
@@ -107,17 +116,34 @@ static int packet_enqueue(CompareState *s, int mode)
 pkt = NULL;
 return -1;
 }
-/* TODO: get connection key from pkt */
+fill_connection_key(pkt, );
 
-/*
- * TODO: use connection key get conn from
- * connection_track_table
- */
+conn = connection_get(s->connection_track_table,
+  ,
+  >conn_list);
 
-/*
- * TODO: insert pkt to it's conn->primary_list
- * or conn->secondary_list
- */
+if (!conn->processing) {
+g_queue_push_tail(>conn_list, conn);
+conn->processing = true;
+}
+
+if (mode == PRIMARY_IN) {
+if (g_queue_get_length(>primary_list) <=
+   MAX_QUEUE_SIZE) {
+g_queue_push_tail(>primary_list, pkt);
+} else {
+error_report("colo compare primary queue size too big,"
+ "drop packet");
+}
+} else {
+if (g_queue_get_length(>secondary_list) <=
+   MAX_QUEUE_SIZE) {
+g_queue_push_tail(>secondary_list, pkt);
+} else {
+error_report("colo compare secondary queue size too big,"
+ "drop packet");
+}
+}
 
 return 0;
 }
@@ -308,7 +334,12 @@ static void colo_compare_complete(UserCreatable *uc, Error 
**errp)
 net_socket_rs_init(>pri_rs, compare_pri_rs_finalize);
 net_socket_rs_init(>sec_rs, compare_sec_rs_finalize);
 
-/* use g_hash_table_new_full() to new a hashtable */
+g_queue_init(>conn_list);
+
+s->connection_track_table = g_hash_table_new_full(connection_key_hash,
+  connection_key_equal,
+  g_free,
+  connection_destroy);
 
 return;
 }
@@ -349,6 +380,8 @@ static void colo_compare_finalize(Object *obj)
 qemu_chr_fe_release(s->chr_out);
 }
 
+g_queue_free(>conn_list);
+
 g_free(s->pri_indev);
 

Re: [Qemu-devel] [PATCH RESEND v4 1/2] pc: Add 2.8 machine

[Christian Borntraeger]

On 09/05/2016 09:53 AM, Longpeng(Mike) wrote:

> +++ b/include/hw/compat.h
> @@ -1,6 +1,8 @@
>  #ifndef HW_COMPAT_H
>  #define HW_COMPAT_H
> 
> +#define HW_COMPAT_2_7
> +

This is already defined (due to the s390 2.8 machine)

[Qemu-devel] [PATCH V14 12/12] docs: Add documentation for COLO-proxy

[Zhang Chen]

Introduce the design of COLO-proxy, and how to use it.

Signed-off-by: Zhang Chen 
---
 docs/colo-proxy.txt | 188 
 1 file changed, 188 insertions(+)
 create mode 100644 docs/colo-proxy.txt

diff --git a/docs/colo-proxy.txt b/docs/colo-proxy.txt
new file mode 100644
index 000..76767cb
--- /dev/null
+++ b/docs/colo-proxy.txt
@@ -0,0 +1,188 @@
+COLO-proxy
+--
+Copyright (c) 2016 Intel Corporation
+Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+Copyright (c) 2016 Fujitsu, Corp.
+
+This work is licensed under the terms of the GNU GPL, version 2 or later.
+See the COPYING file in the top-level directory.
+
+This document gives an overview of COLO proxy's design.
+
+== Background ==
+COLO-proxy is a part of COLO project. It is used
+to compare the network package to help COLO decide
+whether to do checkpoint. With COLO-proxy's help,
+COLO greatly improves the performance.
+
+The filter-redirector, filter-mirror, colo-compare
+and filter-rewriter compose the COLO-proxy.
+
+== Architecture ==
+
+COLO-Proxy is based on qemu netfilter and it's a plugin for qemu netfilter
+(except colo-compare). It keep Secondary VM connect normally to
+client and compare packets sent by PVM with sent by SVM.
+If the packet difference, notify COLO-frame to do checkpoint and send
+all primary packet has queued. Otherwise just send the queued primary
+packet and drop the queued secondary packet.
+
+Below is a COLO proxy ascii figure:
+
+ Primary qemu   
Secondary qemu
++--+   
++
+| +--+ |   |  
+---+ |
+| |  | |   |  |
   | |
+| |guest | |   |  |
guest  | |
+| |  | |   |  |
   | |
+| +---^--+---+ |   |  
+-+++ |
+| |  | |   |   
 ^|  |
+| |  | |   |   
 ||  |
+| |  +--+  |   
 ||  |
+|netfilter|  |   | ||  |   
netfilter||  |
+| +--+ ++  ||  |  
+---+ |
+| |   |  |   |  |out   ||  |  |
 ||  filter excute order   | |
+| |   |  |  +-+||  |  |
 || +--->  | |
+| |   |  |  ||  | |||  |  |
 ||   TCP  | |
+| | +-+--+-+  +-v+ +-v+ |pri +++sec||  |  | 
++  +---++---v+rewriter++  ++ | |
+| | |  |  |  | |  | |in  | |in ||  |  | |  
  |  ||  |  || | |
+| | |  filter  |  |  filter  | |  filter  +-->  colo   <--+ +> 
 filter   +--> adjust |   adjust +-->   filter   | | |
+| | |  mirror  |  |redirector| |redirector| || compare |   |  ||  | | 
redirector |  | ack|   seq|  | redirector | | |
+| | |  |  |  | |  | || |   |  ||  | |  
  |  ||  |  || | |
+| | +^-+  ++-+ +--+ |+-+   |  ||  | 
++  ++--+  +---++ | |
+| |  |   tx|   rx   rx  |  |  ||  |
txall   |  rx  | |
+| |  | ||  |  ||  
+---+ |
+| |  | +--+ |  |  ||   
||
+| |  |   filter excute order  | |  |  ||   
||
+| |  |  

Re: [Qemu-devel] [PATCH v4 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support

[Kevin Wolf]

Am 08.09.2016 um 10:49 hat Daniel P. Berrange geschrieben:
> On Thu, Sep 08, 2016 at 10:43:19AM +0200, Kevin Wolf wrote:
> > Am 08.09.2016 um 00:32 hat ashish mittal geschrieben:
> > > >> +
> > > >> +void vxhs_set_acb_buffer(void *ptr, void *buffer)
> > > >> +{
> > > >> +VXHSAIOCB *acb = ptr;
> > > >> +
> > > >> +acb->buffer = buffer;
> > > >> +}
> > > >> +
> > > >
> > > > Unused function?
> > > 
> > > This is called from within libqnio.
> > 
> > Wait, you mean the library references a symbol in the qemu binary? This
> > sounds completely wrong to me. I wouldn't even do something like this if
> > it were an internal qemu library because I think libraries should be
> > self-contained, but it's a much larger problem in something that is
> > supposed to be an independent library.
> 
> I'd be surprised if that actually worked. If an external library wants
> to refrence symbols in the QEMU binary, then QEMU would ned to be using
> the -export-dynamic / -rdynamic linker flags to export all its symbols,
> and AFAIK, we're not doing that.

Hm, but if the function is really used by the library, how else would it
be when its name isn't mentioned anywhere in the patch except in its
declaration? And it appears to be called there directly:

https://github.com/MittalAshish/libqnio/search?q=vxhs_set_acb_buffer

Anyway, something is fishy here.

Kevin

Re: [Qemu-devel] [PATCH 0/7] MIPS Boston board support

[Paul Burton]

On 08/09/16 09:57, Leon Alrae wrote:
> On Fri, Aug 19, 2016 at 08:40:32PM +0100, Paul Burton wrote:
>> On 19/08/16 20:25, no-re...@patchew.org wrote:
>>> Hi,
>>>
>>> Your series failed automatic build test. Please find the testing commands 
>>> and
>>> their output below. If you have docker installed, you can probably 
>>> reproduce it
>>> locally.
>>>
>>> Message-id: 20160819190903.10974-1-paul.bur...@imgtec.com
>>> Subject: [Qemu-devel] [PATCH 0/7] MIPS Boston board support
>>> Type: series
>>
>> FYI, this build failure occurs because dtc/libfdt is missing this commit:
>>
>>
>> https://git.kernel.org/cgit/utils/dtc/dtc.git/commit/libfdt/version.lds?id=a4b093f7366fdb429ca1781144d3985fa50d0fbb
>>
>> Unfortunately the last release of dtc seems to be very old despite there
>> being fixes like that present in it for well over a year. I'm open to
>> suggestions about how to handle that...
> 
> Looks like 1.4.2 has appeared recently and it contains above commit.

Hi Leon,

Great - that's good timing! :)

> 
> One way would be just to bump our minimal dtc version requirement from
> 1.4.0 to 1.4.2 (and update qemu's dtc submodule as well)...

Would you like me to submit a v2 which does that & removes the
extraneous semicolon from patch 1?

Thanks for reviewing some of this,

Paul

> 
> Thanks,
> Leon
> 
>>
>> Thanks,
>> Paul
>>
>>>
>>> === TEST SCRIPT BEGIN ===
>>> #!/bin/bash
>>> set -e
>>> git submodule update --init dtc
>>> make J=8 docker-test-quick@centos6
>>>
>>> # we need CURL DPRINTF patch
>>> # 
>>> http://patchew.org/QEMU/1470027888-24381-1-git-send-email-famz%40redhat.com/
>>> #make J=8 docker-test-mingw@fedora
>>> === TEST SCRIPT END ===
>>>
>>> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
>>> Switched to a new branch 'test'
>>> 1821581 hw/mips: MIPS Boston board support
>>> 9b44da9 hw: xilinx-pcie: Add support for Xilinx AXI PCIe Controller
>>> fe513d4 loader: Support Flattened Image Trees (FIT images)
>>> d0296cc target-mips: Provide function to test if a CPU supports an ISA
>>> 52c4cc5 hw/mips_gic: Update pin state on mask changes
>>> 402000d hw/mips_gictimer: provide API for retrieving frequency
>>> a80d326 hw/mips_cmgcr: allow GCR base to be moved
>>>
>>> === OUTPUT BEGIN ===
>>> Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 
>>> 'dtc'
>>> Cloning into 'dtc'...
>>> Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
>>>   BUILD centos6
>>>   ARCHIVE qemu.tgz
>>>   ARCHIVE dtc.tgz
>>>   COPY RUNNER
>>>   RUN test-quick in centos6
>>> No C++ compiler available; disabling C++ specific optional code
>>> Install prefix/tmp/qemu-test/src/tests/docker/install
>>> BIOS directory/tmp/qemu-test/src/tests/docker/install/share/qemu
>>> binary directory  /tmp/qemu-test/src/tests/docker/install/bin
>>> library directory /tmp/qemu-test/src/tests/docker/install/lib
>>> module directory  /tmp/qemu-test/src/tests/docker/install/lib/qemu
>>> libexec directory /tmp/qemu-test/src/tests/docker/install/libexec
>>> include directory /tmp/qemu-test/src/tests/docker/install/include
>>> config directory  /tmp/qemu-test/src/tests/docker/install/etc
>>> local state directory   /tmp/qemu-test/src/tests/docker/install/var
>>> Manual directory  /tmp/qemu-test/src/tests/docker/install/share/man
>>> ELF interp prefix /usr/gnemul/qemu-%M
>>> Source path   /tmp/qemu-test/src
>>> C compilercc
>>> Host C compiler   cc
>>> C++ compiler  
>>> Objective-C compiler cc
>>> ARFLAGS   rv
>>> CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread 
>>> -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -g 
>>> QEMU_CFLAGS   -I/usr/include/pixman-1-fPIE -DPIE -m64 -D_GNU_SOURCE 
>>> -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
>>> -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
>>> -fno-strict-aliasing -fno-common  -Wendif-labels -Wmissing-include-dirs 
>>> -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
>>> -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
>>> -Wtype-limits -fstack-protector-all
>>> LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
>>> make  make
>>> install   install
>>> pythonpython -B
>>> smbd  /usr/sbin/smbd
>>> module supportno
>>> host CPU  x86_64
>>> host big endian   no
>>> target list   x86_64-softmmu aarch64-softmmu
>>> tcg debug enabled no
>>> gprof enabled no
>>> sparse enabledno
>>> strip binariesyes
>>> profiler  no
>>> static build  no
>>> pixmansystem
>>> SDL support   yes (1.2.14)
>>> GTK support   no 
>>> GTK GL supportno
>>> VTE support   no 
>>> TLS priority  NORMAL
>>> GNUTLS supportno
>>> GNUTLS rndno
>>> libgcrypt no
>>> libgcrypt kdf no
>>> nettleno 
>>> nettle kdfno
>>> libtasn1  no
>>> curses support

Re: [Qemu-devel] [PATCH v7 1/4] vfio: Mediated device Core driver

[Neo Jia]

On Thu, Sep 08, 2016 at 04:09:39PM +0800, Jike Song wrote:
> On 08/25/2016 11:53 AM, Kirti Wankhede wrote:
> > +
> > +/**
> > + * struct parent_ops - Structure to be registered for each parent device to
> > + * register the device to mdev module.
> > + *
> > + * @owner: The module owner.
> > + * @dev_attr_groups:   Default attributes of the parent device.
> > + * @mdev_attr_groups:  Default attributes of the mediated device.
> > + * @supported_config:  Called to get information about supported types.
> > + * @dev : device structure of parent device.
> > + * @config: should return string listing supported config
> > + * Returns integer: success (0) or error (< 0)
> > + * @create:Called to allocate basic resources in parent 
> > device's
> > + * driver for a particular mediated device. It is
> > + * mandatory to provide create ops.
> > + * @mdev: mdev_device structure on of mediated device
> > + *   that is being created
> > + * @mdev_params: extra parameters required by parent
> > + * device's driver.
> > + * Returns integer: success (0) or error (< 0)
> > + * @destroy:   Called to free resources in parent device's 
> > driver for a
> > + * a mediated device. It is mandatory to provide destroy
> > + * ops.
> > + * @mdev: mdev_device device structure which is being
> > + *destroyed
> > + * Returns integer: success (0) or error (< 0)
> > + * If VMM is running and destroy() is called that means the
> > + * mdev is being hotunpluged. Return error if VMM is
> > + * running and driver doesn't support mediated device
> > + * hotplug.
> > + * @reset: Called to reset mediated device.
> > + * @mdev: mdev_device device structure.
> > + * Returns integer: success (0) or error (< 0)
> > + * @set_online_status: Called to change to status of mediated device.
> > + * @mdev: mediated device.
> > + * @online: set true or false to make mdev device online or
> > + * offline.
> > + * Returns integer: success (0) or error (< 0)
> > + * @get_online_status: Called to get online/offline status of  
> > mediated device
> > + * @mdev: mediated device.
> > + * @online: Returns status of mediated device.
> > + * Returns integer: success (0) or error (< 0)
> > + * @read:  Read emulation callback
> > + * @mdev: mediated device structure
> > + * @buf: read buffer
> > + * @count: number of bytes to read
> > + * @pos: address.
> > + * Retuns number on bytes read on success or error.
> > + * @write: Write emulation callback
> > + * @mdev: mediated device structure
> > + * @buf: write buffer
> > + * @count: number of bytes to be written
> > + * @pos: address.
> > + * Retuns number on bytes written on success or error.
> > + * @get_irq_info:  Called to retrieve information about mediated device IRQ
> > + * @mdev: mediated device structure
> > + * @irq_info: VFIO IRQ flags and count.
> > + * Returns integer: success (0) or error (< 0)
> > + * @set_irqs:  Called to send about interrupts configuration
> > + * information that VMM sets.
> > + * @mdev: mediated device structure
> > + * @flags, index, start, count and *data : same as that of
> > + * struct vfio_irq_set of VFIO_DEVICE_SET_IRQS API.
> > + * @get_device_info:   Called to get VFIO device information for a 
> > mediated
> > + * device.
> > + * @vfio_device_info: VFIO device info.
> > + * Returns integer: success (0) or error (< 0)
> > + * @get_region_info:   Called to get VFIO region size and flags of 
> > mediated
> > + * device.
> > + * @mdev: mediated device structure
> > + * @region_info: output, returns size and flags of
> > + *   requested region.
> > + * @cap_type_id: returns id of capability.
> > + * @cap_type: returns pointer to capability structure
> > + * corresponding to capability id.
> > + * Returns integer: success (0) or error (< 0)
> > + *
> > + * Parent device that support mediated device should be registered with 
> > mdev
> > + * module with parent_ops structure.
> > + */
> > +
> > +struct parent_ops {
> > +   struct module   *owner;
> > +   const struct 

[Qemu-devel] [PATCH V14 02/12] colo-compare: introduce colo compare initialization

[Zhang Chen]

This a COLO net ascii figure:

 Primary qemu   
Secondary qemu
+--+   
++
| +--+ |   |  
+---+ |
| |  | |   |  | 
  | |
| |guest | |   |  | 
   guest  | |
| |  | |   |  | 
  | |
| +---^--+---+ |   |  
+-+++ |
| |  | |   |
^|  |
| |  | |   |
||  |
| |  +--+  |
||  |
|netfilter|  |   | ||  |   
netfilter||  |
| +--+ ++  ||  |  
+---+ |
| |   |  |   |  |out   ||  |  | 
||  filter excute order   | |
| |   |  |  +-+||  |  | 
|| +--->  | |
| |   |  |  ||  | |||  |  | 
||   TCP  | |
| | +-+--+-+  +-v+ +-v+ |pri +++sec||  |  | 
++  +---++---v+rewriter++  ++ | |
| | |  |  |  | |  | |in  | |in ||  |  | |   
 |  ||  |  || | |
| | |  filter  |  |  filter  | |  filter  +-->  colo   <--+ +>  
filter   +--> adjust |   adjust +-->   filter   | | |
| | |  mirror  |  |redirector| |redirector| || compare |   |  ||  | | 
redirector |  | ack|   seq|  | redirector | | |
| | |  |  |  | |  | || |   |  ||  | |   
 |  ||  |  || | |
| | +^-+  ++-+ +--+ |+-+   |  ||  | 
++  ++--+  +---++ | |
| |  |   tx|   rx   rx  |  |  ||  | 
   txall   |  rx  | |
| |  | ||  |  ||  
+---+ |
| |  | +--+ |  |  ||
   ||
| |  |   filter excute order  | |  |  ||
   ||
| |  |  +>| |  |  
++|
| +-+  |   |
|
||||   |
|
+--+   
++
 |guest receive   | guest send
 ||
++v+
|  |
  NOTE: filter direction is rx/tx/all
| tap  |
  rx:receive packets sent to the netdev
|  |
  tx:receive packets sent by the netdev
+--+

In COLO-compare, we do packet comparing job.
Packets coming from the primary char indev will be sent to outdev.
Packets coming from the secondary char dev will be dropped after comparing.
colo-comapre need two input chardev and one output chardev:
primary_in=chardev1-id (source: primary send packet)
secondary_in=chardev2-id (source: secondary send packet)
outdev=chardev3-id

[Qemu-devel] [PATCH] vmsvga: correct bitmap and pixmap size checks

[P J P]

From: Prasad J Pandit 

When processing svga command DEFINE_CURSOR in vmsvga_fifo_run,
the computed BITMAP and PIXMAP size are checked against the
'cursor.mask[]' and 'cursor.image[]' array sizes in bytes.
Correct these checks to avoid OOB memory access.

Reported-by: Qinghao Tang 
Reported-by: Li Qiang 
Signed-off-by: Prasad J Pandit 
---
 hw/display/vmware_vga.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index e51a05e..6599cf0 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -676,11 +676,13 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
 cursor.bpp = vmsvga_fifo_read(s);
 
 args = SVGA_BITMAP_SIZE(x, y) + SVGA_PIXMAP_SIZE(x, y, cursor.bpp);
-if (cursor.width > 256 ||
-cursor.height > 256 ||
-cursor.bpp > 32 ||
-SVGA_BITMAP_SIZE(x, y) > sizeof cursor.mask ||
-SVGA_PIXMAP_SIZE(x, y, cursor.bpp) > sizeof cursor.image) {
+if (cursor.width > 256
+|| cursor.height > 256
+|| cursor.bpp > 32
+|| SVGA_BITMAP_SIZE(x, y)
+> sizeof(cursor.mask) / sizeof(cursor.mask[0])
+|| SVGA_PIXMAP_SIZE(x, y, cursor.bpp)
+> sizeof(cursor.image) / sizeof(cursor.image[0])) {
 goto badcmd;
 }
 
-- 
2.5.5

Re: [Qemu-devel] [PATCH 02/10] tcg: Pass last_tb by value to tb_find_fast()

[Alex Bennée]

Paolo Bonzini  writes:

> From: Sergey Fedorov 
>
> This is a small clean up. tb_find_fast() is a final consumer of this
> variable so no need to pass it by reference. 'last_tb' is always updated
> by subsequent cpu_loop_exec_tb() in cpu_exec().
>

> @@ -621,7 +620,7 @@ int cpu_exec(CPUState *cpu)
>  cpu->tb_flushed = false; /* reset before first TB lookup */
>  for(;;) {
>  cpu_handle_interrupt(cpu, _tb);
> -tb = tb_find_fast(cpu, _tb, tb_exit);
> +tb = tb_find_fast(cpu, last_tb, tb_exit);

Maybe a comment here for those that missed the subtly in the commit
message?

   /* cpu_loop_exec_tb updates a to a new last_tb */

>  cpu_loop_exec_tb(cpu, tb, _tb, _exit, );

You could even make it explicit and change cpu_loop_exec_tb to return
last_tb instead of passing by reference. Then it would be even clearer
when reading the code.

>  /* Try to align the host and virtual clocks
> if the guest is in advance */



--
Alex Bennée

Re: [Qemu-devel] KVM Forum 2016 videos now online

[Daniel P. Berrange]

On Thu, Sep 08, 2016 at 09:42:33AM +0200, Christian Borntraeger wrote:
> On 09/08/2016 05:28 AM, Pranith Kumar wrote:
> > FYI,
> > 
> > The KVM Forum 2016 videos are now online on youtube. You can find them here:
> > 
> > https://www.youtube.com/playlist?list=PLW3ep1uCIRfzQoZ0SlniYE8nz1ZRobjH7
> 
> The videos are not complete yet, some more still to come as far as I can see.

Yep, they have been uploaded incrementally over the past few days as each
video had its editting completed and this (time consuming !) work is still
ongoing.

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 :|

[Qemu-devel] [PATCH V14 01/12] qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext

[Zhang Chen]

Add qemu_chr_add_handlers_full() API, we can use
this API pass in a GMainContext,make handler run
in the context rather than main_loop.
This comments from Daniel P . Berrange.

Signed-off-by: Zhang Chen 
Signed-off-by: Li Zhijian 
Signed-off-by: Wen Congyang 
Reviewed-by: Daniel P. Berrange 
---
 include/sysemu/char.h | 11 +++-
 qemu-char.c   | 77 +++
 2 files changed, 63 insertions(+), 25 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index ee7e554..0d0465a 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -65,7 +65,8 @@ struct CharDriverState {
 int (*chr_sync_read)(struct CharDriverState *s,
  const uint8_t *buf, int len);
 GSource *(*chr_add_watch)(struct CharDriverState *s, GIOCondition cond);
-void (*chr_update_read_handler)(struct CharDriverState *s);
+void (*chr_update_read_handler)(struct CharDriverState *s,
+GMainContext *context);
 int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
 int (*get_msgfds)(struct CharDriverState *s, int* fds, int num);
 int (*set_msgfds)(struct CharDriverState *s, int *fds, int num);
@@ -422,6 +423,14 @@ void qemu_chr_add_handlers(CharDriverState *s,
IOEventHandler *fd_event,
void *opaque);
 
+/* This API can make handler run in the context what you pass to. */
+void qemu_chr_add_handlers_full(CharDriverState *s,
+IOCanReadHandler *fd_can_read,
+IOReadHandler *fd_read,
+IOEventHandler *fd_event,
+void *opaque,
+GMainContext *context);
+
 void qemu_chr_be_generic_open(CharDriverState *s);
 void qemu_chr_accept_input(CharDriverState *s);
 int qemu_chr_add_client(CharDriverState *s, int fd);
diff --git a/qemu-char.c b/qemu-char.c
index 5f82ebb..100aada 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -446,11 +446,12 @@ void qemu_chr_fe_printf(CharDriverState *s, const char 
*fmt, ...)
 
 static void remove_fd_in_watch(CharDriverState *chr);
 
-void qemu_chr_add_handlers(CharDriverState *s,
-   IOCanReadHandler *fd_can_read,
-   IOReadHandler *fd_read,
-   IOEventHandler *fd_event,
-   void *opaque)
+void qemu_chr_add_handlers_full(CharDriverState *s,
+IOCanReadHandler *fd_can_read,
+IOReadHandler *fd_read,
+IOEventHandler *fd_event,
+void *opaque,
+GMainContext *context)
 {
 int fe_open;
 
@@ -464,8 +465,9 @@ void qemu_chr_add_handlers(CharDriverState *s,
 s->chr_read = fd_read;
 s->chr_event = fd_event;
 s->handler_opaque = opaque;
-if (fe_open && s->chr_update_read_handler)
-s->chr_update_read_handler(s);
+if (fe_open && s->chr_update_read_handler) {
+s->chr_update_read_handler(s, context);
+}
 
 if (!s->explicit_fe_open) {
 qemu_chr_fe_set_open(s, fe_open);
@@ -478,6 +480,16 @@ void qemu_chr_add_handlers(CharDriverState *s,
 }
 }
 
+void qemu_chr_add_handlers(CharDriverState *s,
+   IOCanReadHandler *fd_can_read,
+   IOReadHandler *fd_read,
+   IOEventHandler *fd_event,
+   void *opaque)
+{
+qemu_chr_add_handlers_full(s, fd_can_read, fd_read,
+   fd_event, opaque, NULL);
+}
+
 static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
 {
 return len;
@@ -715,7 +727,8 @@ static void mux_chr_event(void *opaque, int event)
 mux_chr_send_event(d, i, event);
 }
 
-static void mux_chr_update_read_handler(CharDriverState *chr)
+static void mux_chr_update_read_handler(CharDriverState *chr,
+GMainContext *context)
 {
 MuxDriver *d = chr->opaque;
 
@@ -729,8 +742,10 @@ static void mux_chr_update_read_handler(CharDriverState 
*chr)
 d->chr_event[d->mux_cnt] = chr->chr_event;
 /* Fix up the real driver with mux routines */
 if (d->mux_cnt == 0) {
-qemu_chr_add_handlers(d->drv, mux_chr_can_read, mux_chr_read,
-  mux_chr_event, chr);
+qemu_chr_add_handlers_full(d->drv, mux_chr_can_read,
+   mux_chr_read,
+   mux_chr_event,
+   chr, context);
 }
 if (d->focus != -1) {
 mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
@@ -846,6 +861,7 @@ typedef struct IOWatchPoll
 IOCanReadHandler *fd_can_read;
 

[Qemu-devel] [PATCH V14 08/12] filter-rewriter: introduce filter-rewriter initialization

[Zhang Chen]

Filter-rewriter is a part of COLO project.
It will rewrite some of secondary packet to make
secondary guest's tcp connection established successfully.
In this module we will rewrite tcp packet's ack to the secondary
from primary,and rewrite tcp packet's seq to the primary from
secondary.

usage:

colo secondary:
-object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0
-object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
-object filter-rewriter,id=rew0,netdev=hn0,queue=all

Signed-off-by: Zhang Chen 
Signed-off-by: Li Zhijian 
Signed-off-by: Wen Congyang 
---
 net/Makefile.objs |   1 +
 net/filter-rewriter.c | 105 ++
 qemu-options.hx   |  13 +++
 vl.c  |   3 +-
 4 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 net/filter-rewriter.c

diff --git a/net/Makefile.objs b/net/Makefile.objs
index beb504b..2a80df5 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -18,3 +18,4 @@ common-obj-y += filter-buffer.o
 common-obj-y += filter-mirror.o
 common-obj-y += colo-compare.o
 common-obj-y += colo.o
+common-obj-y += filter-rewriter.o
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
new file mode 100644
index 000..de29f07
--- /dev/null
+++ b/net/filter-rewriter.c
@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ * Copyright (c) 2016 FUJITSU LIMITED
+ * Copyright (c) 2016 Intel Corporation
+ *
+ * Author: Zhang Chen 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "net/colo.h"
+#include "net/filter.h"
+#include "net/net.h"
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qerror.h"
+#include "qapi-visit.h"
+#include "qom/object.h"
+#include "qemu/main-loop.h"
+#include "qemu/iov.h"
+#include "net/checksum.h"
+
+#define FILTER_COLO_REWRITER(obj) \
+OBJECT_CHECK(RewriterState, (obj), TYPE_FILTER_REWRITER)
+
+#define TYPE_FILTER_REWRITER "filter-rewriter"
+
+typedef struct RewriterState {
+NetFilterState parent_obj;
+NetQueue *incoming_queue;
+/* hashtable to save connection */
+GHashTable *connection_track_table;
+} RewriterState;
+
+static void filter_rewriter_flush(NetFilterState *nf)
+{
+RewriterState *s = FILTER_COLO_REWRITER(nf);
+
+if (!qemu_net_queue_flush(s->incoming_queue)) {
+/* Unable to empty the queue, purge remaining packets */
+qemu_net_queue_purge(s->incoming_queue, nf->netdev);
+}
+}
+
+static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
+ NetClientState *sender,
+ unsigned flags,
+ const struct iovec *iov,
+ int iovcnt,
+ NetPacketSent *sent_cb)
+{
+/*
+ * if we get tcp packet
+ * we will rewrite it to make secondary guest's
+ * connection established successfully
+ */
+return 0;
+}
+
+static void colo_rewriter_cleanup(NetFilterState *nf)
+{
+RewriterState *s = FILTER_COLO_REWRITER(nf);
+
+/* flush packets */
+if (s->incoming_queue) {
+filter_rewriter_flush(nf);
+g_free(s->incoming_queue);
+}
+}
+
+static void colo_rewriter_setup(NetFilterState *nf, Error **errp)
+{
+RewriterState *s = FILTER_COLO_REWRITER(nf);
+
+s->connection_track_table = g_hash_table_new_full(connection_key_hash,
+  connection_key_equal,
+  g_free,
+  connection_destroy);
+s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
+}
+
+static void colo_rewriter_class_init(ObjectClass *oc, void *data)
+{
+NetFilterClass *nfc = NETFILTER_CLASS(oc);
+
+nfc->setup = colo_rewriter_setup;
+nfc->cleanup = colo_rewriter_cleanup;
+nfc->receive_iov = colo_rewriter_receive_iov;
+}
+
+static const TypeInfo colo_rewriter_info = {
+.name = TYPE_FILTER_REWRITER,
+.parent = TYPE_NETFILTER,
+.class_init = colo_rewriter_class_init,
+.instance_size = sizeof(RewriterState),
+};
+
+static void register_types(void)
+{
+type_register_static(_rewriter_info);
+}
+
+type_init(register_types);
diff --git a/qemu-options.hx b/qemu-options.hx
index 87fbba3..5cd50a8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3880,6 +3880,19 @@ Create a filter-redirector we need to differ outdev id 
from indev id, id can not
 be the same. we can just use indev or outdev, but at least one of indev or 
outdev
 need to be specified.
 
+@item -object 

Re: [Qemu-devel] [PULL v2 0/4] target-arm queue

[Peter Maydell]

On 6 September 2016 at 20:03, Peter Maydell  wrote:
> v2 pull:
>  * dropped the ast2500 patches
>  * fix ast2400 memory controller format string bug
>
> thanks
> -- PMM
>
>
> The following changes since commit 2926375cffce464fde6b4dabaed1e133d549af39:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
> (2016-09-06 17:18:17 +0100)
>
> are available in the git repository at:
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20160906-1
>
> for you to fetch changes up to c827c06a4dd6c768eeb3aaa6af6cfd29663116af:
>
>   block: m25p80: Fix vmstate structure name (2016-09-06 19:52:18 +0100)
>
> 
> target-arm queue:
>  * fix incorrect LPAE bit in FSR for alignment faults
>  * ACPI: fix the AML ID format for CPU devices to work for
>large numbers of CPUs
>  * ast2400: add memory controller device model
>  * m25p80: fix the vmstate structure name (migration break)
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCHv5] tests: add qtest_add_data_func_full

[Eric Blake]

On 09/08/2016 03:40 AM, Marc-André Lureau wrote:
> Allows one to specify a destroy function for the test data.
> 
> Add a fallback using glib g_test_add_vtable() internal function, whose
> signature changed over time. Tested with glib 2.22, 2.26 and 2.48, which
> according to git log should be enough to cover all variations.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  tests/libqtest.c | 19 +++
>  tests/libqtest.h | 17 +
>  2 files changed, 36 insertions(+)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index eb00f13..5f4450f 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -758,6 +758,25 @@ void qtest_add_func(const char *str, void (*fn)(void))
>  g_free(path);
>  }
>  
> +void qtest_add_data_func_full(const char *str, void *data,
> +  void (*fn)(const void *),
> +  GDestroyNotify data_free_func)
> +{
> +gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
> +#if GLIB_CHECK_VERSION(2, 34, 0)
> +g_test_add_data_func_full(path, data, fn, data_free_func);
> +#elif GLIB_CHECK_VERSION(2, 26, 0)
> +/* back-compat casts, remove this once we can require new-enough glib */
> +g_test_add_vtable(path, 0, data, NULL,
> +  (GTestFixtureFunc)fn, (GTestFixtureFunc) 
> data_free_func);
> +#elif GLIB_CHECK_VERSION(2, 22, 0)

Perhaps this could be written as #else, since we have no further
alternatives (2.22.0 being our minimum).  Up to the maintainer if it is
worth changing.  Otherwise I think the end result is reasonable, and you
were able to avoid the leaks.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support

[Paolo Bonzini]

On 08/09/2016 12:18, Kevin Wolf wrote:
> Am 08.09.2016 um 11:43 hat Daniel P. Berrange geschrieben:
>> On Thu, Sep 08, 2016 at 11:29:41AM +0200, Kevin Wolf wrote:
>>> Am 08.09.2016 um 10:49 hat Daniel P. Berrange geschrieben:
 On Thu, Sep 08, 2016 at 10:43:19AM +0200, Kevin Wolf wrote:
> Am 08.09.2016 um 00:32 hat ashish mittal geschrieben:
 +
 +void vxhs_set_acb_buffer(void *ptr, void *buffer)
 +{
 +VXHSAIOCB *acb = ptr;
 +
 +acb->buffer = buffer;
 +}
 +
>>>
>>> Unused function?
>>
>> This is called from within libqnio.
>
> Wait, you mean the library references a symbol in the qemu binary? This
> sounds completely wrong to me. I wouldn't even do something like this if
> it were an internal qemu library because I think libraries should be
> self-contained, but it's a much larger problem in something that is
> supposed to be an independent library.

 I'd be surprised if that actually worked. If an external library wants
 to refrence symbols in the QEMU binary, then QEMU would ned to be using
 the -export-dynamic / -rdynamic linker flags to export all its symbols,
 and AFAIK, we're not doing that.
>>>
>>> Hm, but if the function is really used by the library, how else would it
>>> be when its name isn't mentioned anywhere in the patch except in its
>>> declaration? And it appears to be called there directly:
>>>
>>> https://github.com/MittalAshish/libqnio/search?q=vxhs_set_acb_buffer
>>>
>>> Anyway, something is fishy here.
>>
>> Oh, notice that .c file is actually part of the shim library, not the
>> main libqnio.so that QEMU would link against.
>>
>> So, it really is unused and should be deleted from this block/vxhs.c
>> file.
> 
> I haven't fully understood yet what's the deal with this shim library,
> but this patch links against both.

Indeed, I suspect that the "shim library" should really be part of QEMU.
 It's just 700 lines of code, and some parts (e.g.
qnio_ck_initialize_lock seem unused even).  The main thing to do is
convert it from cJSON to QEMU's internal libraries for the same thing.

Paolo

[Qemu-devel] [PATCH V14 03/12] net/colo.c: add colo.c to define and handle packet

[Zhang Chen]

The net/colo.c is used by colo-compare and filter-rewriter.
this can share common data structure like net packet,
and other functions.

Signed-off-by: Zhang Chen 
Signed-off-by: Li Zhijian 
Signed-off-by: Wen Congyang 
---
 net/Makefile.objs  |   1 +
 net/colo-compare.c | 114 +++--
 net/colo.c |  86 
 net/colo.h |  37 +
 trace-events   |   6 +++
 5 files changed, 240 insertions(+), 4 deletions(-)
 create mode 100644 net/colo.c
 create mode 100644 net/colo.h

diff --git a/net/Makefile.objs b/net/Makefile.objs
index ba92f73..beb504b 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -17,3 +17,4 @@ common-obj-y += filter.o
 common-obj-y += filter-buffer.o
 common-obj-y += filter-mirror.o
 common-obj-y += colo-compare.o
+common-obj-y += colo.o
diff --git a/net/colo-compare.c b/net/colo-compare.c
index dc5f70c..cea9b27 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -14,6 +14,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
+#include "trace.h"
 #include "qemu-common.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/error.h"
@@ -26,13 +27,34 @@
 #include "sysemu/char.h"
 #include "qemu/sockets.h"
 #include "qapi-visit.h"
+#include "net/colo.h"
 
 #define TYPE_COLO_COMPARE "colo-compare"
 #define COLO_COMPARE(obj) \
 OBJECT_CHECK(CompareState, (obj), TYPE_COLO_COMPARE)
 
-#define COMPARE_READ_LEN_MAX NET_BUFSIZE
-
+/*
+  + CompareState ++
+  |   |
+  +---+   +---+ +---+
+  |conn list  +--->conn   +->conn   |
+  +---+   +---+ +---+
+  |   | |   | |  |
+  +---+ +---v+  +---v++---v+ +---v+
+|primary |  |secondary|primary | |secondary
+|packet  |  |packet  +|packet  | |packet  +
+++  ++++ ++
+|   | |  |
++---v+  +---v++---v+ +---v+
+|primary |  |secondary|primary | |secondary
+|packet  |  |packet  +|packet  | |packet  +
+++  ++++ ++
+|   | |  |
++---v+  +---v++---v+ +---v+
+|primary |  |secondary|primary | |secondary
+|packet  |  |packet  +|packet  | |packet  +
+++  ++++ ++
+*/
 typedef struct CompareState {
 Object parent;
 
@@ -44,6 +66,9 @@ typedef struct CompareState {
 CharDriverState *chr_out;
 SocketReadState pri_rs;
 SocketReadState sec_rs;
+
+/* hashtable to save connection */
+GHashTable *connection_track_table;
 } CompareState;
 
 typedef struct CompareClass {
@@ -54,6 +79,76 @@ typedef struct CompareChardevProps {
 bool is_socket;
 } CompareChardevProps;
 
+enum {
+PRIMARY_IN = 0,
+SECONDARY_IN,
+};
+
+static int compare_chr_send(CharDriverState *out,
+const uint8_t *buf,
+uint32_t size);
+
+/*
+ * Return 0 on success, if return -1 means the pkt
+ * is unsupported(arp and ipv6) and will be sent later
+ */
+static int packet_enqueue(CompareState *s, int mode)
+{
+Packet *pkt = NULL;
+
+if (mode == PRIMARY_IN) {
+pkt = packet_new(s->pri_rs.buf, s->pri_rs.packet_len);
+} else {
+pkt = packet_new(s->sec_rs.buf, s->sec_rs.packet_len);
+}
+
+if (parse_packet_early(pkt)) {
+packet_destroy(pkt, NULL);
+pkt = NULL;
+return -1;
+}
+/* TODO: get connection key from pkt */
+
+/*
+ * TODO: use connection key get conn from
+ * connection_track_table
+ */
+
+/*
+ * TODO: insert pkt to it's conn->primary_list
+ * or conn->secondary_list
+ */
+
+return 0;
+}
+
+static int compare_chr_send(CharDriverState *out,
+const uint8_t *buf,
+uint32_t size)
+{
+int ret = 0;
+uint32_t len = htonl(size);
+
+if (!size) {
+return 0;
+}
+
+ret = qemu_chr_fe_write_all(out, (uint8_t *), sizeof(len));
+if (ret != sizeof(len)) {
+goto err;
+}
+
+ret = qemu_chr_fe_write_all(out, (uint8_t *)buf, size);
+if (ret != size) {
+goto err;
+}
+
+return 0;
+
+err:
+return ret < 0 ? ret : -EIO;
+}
+
 static char *compare_get_pri_indev(Object *obj, Error **errp)
 {
 CompareState *s = COLO_COMPARE(obj);
@@ -101,12 +196,21 @@ static void compare_set_outdev(Object *obj, const char 
*value, Error **errp)
 
 

Re: [Qemu-devel] [PATCH v3 1/3] memory: introduce IOMMUNotifier and its caps

[Peter Xu]

On Wed, Sep 07, 2016 at 08:20:35PM +1000, David Gibson wrote:
> On Wed, Sep 07, 2016 at 03:09:16PM +0800, Peter Xu wrote:
> > On Wed, Sep 07, 2016 at 04:02:39PM +1000, David Gibson wrote:
> > > On Wed, Sep 07, 2016 at 01:32:22PM +0800, Peter Xu wrote:
> > > > IOMMU Notifier list is used for notifying IO address mapping changes.
> > > > Currently VFIO is the only user.
> > > > 
> > > > However it is possible that future consumer like vhost would like to
> > > > only listen to part of its notifications (e.g., cache invalidations).
> > > > 
> > > > This patch introduced IOMMUNotifier and IOMMUNotfierCap bits for a finer
> > > > grained control of it.
> > > > 
> > > > IOMMUNotifier contains a bitfield for the notify consumer describing
> > > > what kind of notification it is interested in. Currently two kinds of
> > > > notifications are defined:
> > > > 
> > > > - IOMMU_NOTIFIER_CHANGE:   for entry changes (additions)
> > > > - IOMMU_NOTIFIER_INVALIDATION: for entry removals (cache invalidates)
> > > 
> > > As noted on the other thread, I think the correct options for your
> > > bitmap here are "map" and "unmap".  Which are triggered depends on the
> > > permissions / existence of the *previous* mapping, as well as the new
> > > one.
> > 
> > As mentioned in previous reply, both work for me. :)
> > 
> > If you insist on changing it (without anyone that strongly
> > disagree...), I can do it in next spin.
> 
> This is not just a name change I'm proposing, but a semantic change
> (or at least a clarification).

I see that kernel IOMMU driver is using map_page() and unmap_page()
for its interface. Now I prefer MAP/UNMAP.

> 
> > > You could in fact have "map-read", "map-write", "unmap-read",
> > > "unmap-write" as separate bitmap options (e.g. changing a mapping from
> > > RO to WO would be both a read-unmap and write-map event).  I can't see
> > > any real use for that though, so just "map" and "unmap" are probably
> > > sufficient.
> > 
> > Agreed. We can enhance it in the future if there is any real
> > requirement. Before that, it would be over-engineering.
> > 
> > (Btw, we should not need {read|write}_unmap in all cases. IIUC unmap
> >  should not need any permission check.)
> 
> In practice probably not, but they are distinct operations.
> read_unmap means a readable mapping has been removed, write_unmap
> means a writable mapping has been removed.  Again - the permissions on
> the *old* mapping are what matters here.

Why they are distinct operations? Or could you help explain in what
case would we need this flag (read/write) for an unmap() operation?

> 
> > 
> > [...]
> > 
> > > > -static void vfio_iommu_map_notify(Notifier *n, void *data)
> > > > +static void vfio_iommu_map_notify(IOMMUNotifier *n, void *data)
> > > >  {
> > > >  VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> > > >  VFIOContainer *container = giommu->container;
> > > > @@ -454,6 +454,7 @@ static void vfio_listener_region_add(MemoryListener 
> > > > *listener,
> > > > section->offset_within_region;
> > > >  giommu->container = container;
> > > >  giommu->n.notify = vfio_iommu_map_notify;
> > > > +giommu->n.notifier_caps = IOMMU_NOTIFIER_ALL;
> > > 
> > > "caps" isn't really right.  It's a *requirement* that VFIO get all the
> > > notifications, not a capability.  "caps" would only make sense on the
> > > other side (the vIOMMU implementation).
> > 
> > Sounds reasonable. How about "flags"? Or any suggestion?
> 
> "flags" would do.  I feel like there should be a better name, but I
> can't think of it.

Sure. I can switch.

> 
> > [...]
> > 
> > > >  /**
> > > > @@ -620,11 +642,12 @@ void memory_region_notify_iommu(MemoryRegion *mr,
> > > >   * IOMMU translation entries.
> > > >   *
> > > >   * @mr: the memory region to observe
> > > > - * @n: the notifier to be added; the notifier receives a pointer to an
> > > > - * #IOMMUTLBEntry as the opaque value; the pointer ceases to be
> > > > - * valid on exit from the notifier.
> > > > + * @n: the IOMMUNotifier to be added; the notify callback receives a
> > > > + * pointer to an #IOMMUTLBEntry as the opaque value; the pointer
> > > > + * ceases to be valid on exit from the notifier.
> > > >   */
> > > > -void memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier 
> > > > *n);
> > > > +void memory_region_register_iommu_notifier(MemoryRegion *mr,
> > > > +   IOMMUNotifier *n);
> > > 
> > > It seems to me that this should be allowed to fail, if the notifier
> > > you're trying to register requires notifications that the MR
> > > implementation can't supply.  That seems cleaner than delaying the
> > > checking until the notification actually happens.
> > 
> > Do we have real use case for this one? For example, do we have case
> > that VM "registering required notifications that MR cannot support"
> > can still work?
> > 
> > Currently there is 

Re: [Qemu-devel] [PATCH v3 0/8] docker tests fixes

[Fam Zheng]

On Tue, 09/06 22:05, Sascha Silbe wrote:
> A couple of fixes for issues encountered while trying out the new
> docker test support.
> 
> v2→v3:
>   - fix non-portable sort -V usage
>   - send debootstrap.pre error messages to stderr
>   - whitespace changes
> 
> v1→v2:
>   - found a good place to stick the warning about EXECUTABLE
>   - additional fixes for the debian-debootstrap image
> 
> Sascha Silbe (8):
>   docker.py: don't hang on large docker output
>   docker: avoid dependency on 'realpath' package
>   docker: debian-bootstrap.pre: print error messages to stderr
>   docker: debian-bootstrap.pre: print helpful message if
> DEB_ARCH/DEB_TYPE unset
>   docker: print warning if EXECUTABLE is not set when building
> debootstrap image
>   docker: make sure debootstrap is at least 1.0.67
>   docker: build debootstrap after cloning
>   docker: silence debootstrap when --quiet is given
> 
>  tests/docker/Makefile.include |  5 -
>  tests/docker/docker.py| 12 ++
>  tests/docker/dockerfiles/debian-bootstrap.pre | 32 
> +++
>  3 files changed, 40 insertions(+), 9 deletions(-)
> 
> -- 
> 1.9.1
> 

Thanks. Fixed the 'sort -C' portability issue and applied to my docker branch:

http://github.com/famz/qemu/tree/docker

Fam

Re: [Qemu-devel] [PULL 35/38] qmp: add QMP interface "query-cpu-model-baseline"

[Eric Blake]

On 09/06/2016 02:47 AM, Cornelia Huck wrote:
> From: David Hildenbrand 
> 
> Let's provide a standardized interface to baseline two CPU models, to
> create a third, compatible one. This is especially helpful when two
> CPU models are not identical, but a CPU model is required that is
> guaranteed to run under both configurations, where the original models run.
> 

> +#
> +# The result returned by this command may be affected by:
> +#
> +# * QEMU version: CPU models may look different depending on the QEMU 
> version.
> +#   (Except for CPU models reported as "static" in query-cpu-definitions.)
> +# * machine-type: CPU model  may look different depending on the 
> machine-type.

s/model  may/model may/

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH V14 09/12] filter-rewriter: track connection and parse packet

[Zhang Chen]

We use net/colo.h to track connection and parse packet

Signed-off-by: Zhang Chen 
Signed-off-by: Li Zhijian 
Signed-off-by: Wen Congyang 
---
 net/colo.c| 14 ++
 net/colo.h|  1 +
 net/filter-rewriter.c | 50 ++
 3 files changed, 65 insertions(+)

diff --git a/net/colo.c b/net/colo.c
index e517521..dc4e4a4 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -115,6 +115,20 @@ void fill_connection_key(Packet *pkt, ConnectionKey *key)
 }
 }
 
+void reverse_connection_key(ConnectionKey *key)
+{
+struct in_addr tmp_ip;
+uint16_t tmp_port;
+
+tmp_ip = key->src;
+key->src = key->dst;
+key->dst = tmp_ip;
+
+tmp_port = key->src_port;
+key->src_port = key->dst_port;
+key->dst_port = tmp_port;
+}
+
 Connection *connection_new(ConnectionKey *key)
 {
 Connection *conn = g_slice_new(Connection);
diff --git a/net/colo.h b/net/colo.h
index 9a7d5e0..6720a3a 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -68,6 +68,7 @@ uint32_t connection_key_hash(const void *opaque);
 int connection_key_equal(const void *opaque1, const void *opaque2);
 int parse_packet_early(Packet *pkt);
 void fill_connection_key(Packet *pkt, ConnectionKey *key);
+void reverse_connection_key(ConnectionKey *key);
 Connection *connection_new(ConnectionKey *key);
 void connection_destroy(void *opaque);
 Connection *connection_get(GHashTable *connection_track_table,
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index de29f07..1d49d04 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -44,6 +44,20 @@ static void filter_rewriter_flush(NetFilterState *nf)
 }
 }
 
+/*
+ * Return 1 on success, if return 0 means the pkt
+ * is not TCP packet
+ */
+static int is_tcp_packet(Packet *pkt)
+{
+if (!parse_packet_early(pkt) &&
+pkt->ip->ip_p == IPPROTO_TCP) {
+return 1;
+} else {
+return 0;
+}
+}
+
 static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
  NetClientState *sender,
  unsigned flags,
@@ -51,11 +65,47 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
  int iovcnt,
  NetPacketSent *sent_cb)
 {
+RewriterState *s = FILTER_COLO_REWRITER(nf);
+Connection *conn;
+ConnectionKey key = { 0 };
+Packet *pkt;
+ssize_t size = iov_size(iov, iovcnt);
+char *buf = g_malloc0(size);
+
+iov_to_buf(iov, iovcnt, 0, buf, size);
+pkt = packet_new(buf, size);
+
 /*
  * if we get tcp packet
  * we will rewrite it to make secondary guest's
  * connection established successfully
  */
+if (pkt && is_tcp_packet(pkt)) {
+
+fill_connection_key(pkt, );
+
+if (sender == nf->netdev) {
+/*
+ * We need make tcp TX and RX packet
+ * into one connection.
+ */
+reverse_connection_key();
+}
+conn = connection_get(s->connection_track_table,
+  ,
+  NULL);
+
+if (sender == nf->netdev) {
+/* NET_FILTER_DIRECTION_TX */
+/* handle_primary_tcp_pkt */
+} else {
+/* NET_FILTER_DIRECTION_RX */
+/* handle_secondary_tcp_pkt */
+}
+}
+
+packet_destroy(pkt, NULL);
+pkt = NULL;
 return 0;
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH 3/3] virtio-balloon: fix stats vq migration

[Roman Kagan]

On Wed, Sep 07, 2016 at 05:20:49PM +0200, Ladi Prosek wrote:
> The statistics virtqueue is not migrated properly because virtio-balloon
> does not include s->stats_vq_elem in the migration stream.
> 
> After migration the statistics virtqueue hangs because the host never
> completes the last element (s->stats_vq_elem is NULL on the destination
> QEMU).  Therefore the guest never submits new elements and the virtqueue
> is hung.
> 
> Instead of changing the migration stream format in an incompatible way,
> detect the migration case and rewind the virtqueue so the last element
> can be completed.
> 
> Cc: Michael S. Tsirkin 
> Cc: Roman Kagan 
> Cc: Stefan Hajnoczi 
> Suggested-by: Roman Kagan 
> Signed-off-by: Ladi Prosek 
> ---
>  hw/virtio/virtio-balloon.c | 13 +
>  1 file changed, 13 insertions(+)

Reviewed-by: Roman Kagan 

Re: [Qemu-devel] [PATCH for-2.8 v1 00/60] Modular build of trace event files

[Daniel P. Berrange]

On Thu, Sep 08, 2016 at 03:23:26PM +0200, Lluís Vilanova wrote:
> Daniel P Berrange writes:
> 
> > I previously split the global trace-events file up into one file
> > per-subdirectory to avoid merge conflict hell.
> [...]
> 
> Sorry, I could not find the message where the infrastructure is modified to
> provide this. But I think there's a more efficient way to provide modular
> auto-generated tracing code without the hierarchical indexing you proposed.

[snip]

>   struct TraceEvent ___trace_events[] = {
>   {
>   .name = "eventname",
>   .sstate = 1,
>   .dstate = ___trace_eventname_dstate;
>   }
>   }
> 
>   TraceEvent *TRACE_EVENTNAME = &___trace_events[...];

Life would be simpler if we had the 'bool dstate' as part of the
TraceEvent struct, but doing so would essentially be reverting this
previous change:

  commit 585ec7273e6fdab902b2128bc6c2a8136aafef04
  Author: Paolo Bonzini 
  Date:   Wed Oct 28 07:06:27 2015 +0100

trace: track enabled events in a separate array

This is more cache friendly on the fast path, where we already have
the event id available.

I asked Paolo about this previously and he indicated it was a notable
performance improvement, so we can't put dstate back into the TraceEvent
struct :-(

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] [PATCHv5] tests: add qtest_add_data_func_full

[Marc-André Lureau]

Hi

- Original Message -
> On 09/08/2016 03:40 AM, Marc-André Lureau wrote:
> > Allows one to specify a destroy function for the test data.
> > 
> > Add a fallback using glib g_test_add_vtable() internal function, whose
> > signature changed over time. Tested with glib 2.22, 2.26 and 2.48, which
> > according to git log should be enough to cover all variations.
> > 
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  tests/libqtest.c | 19 +++
> >  tests/libqtest.h | 17 +
> >  2 files changed, 36 insertions(+)
> > 
> > diff --git a/tests/libqtest.c b/tests/libqtest.c
> > index eb00f13..5f4450f 100644
> > --- a/tests/libqtest.c
> > +++ b/tests/libqtest.c
> > @@ -758,6 +758,25 @@ void qtest_add_func(const char *str, void (*fn)(void))
> >  g_free(path);
> >  }
> >  
> > +void qtest_add_data_func_full(const char *str, void *data,
> > +  void (*fn)(const void *),
> > +  GDestroyNotify data_free_func)
> > +{
> > +gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
> > +#if GLIB_CHECK_VERSION(2, 34, 0)
> > +g_test_add_data_func_full(path, data, fn, data_free_func);
> > +#elif GLIB_CHECK_VERSION(2, 26, 0)
> > +/* back-compat casts, remove this once we can require new-enough glib
> > */
> > +g_test_add_vtable(path, 0, data, NULL,
> > +  (GTestFixtureFunc)fn, (GTestFixtureFunc)
> > data_free_func);
> > +#elif GLIB_CHECK_VERSION(2, 22, 0)
> 
> Perhaps this could be written as #else, since we have no further
> alternatives (2.22.0 being our minimum).  Up to the maintainer if it is
> worth changing.  Otherwise I think the end result is reasonable, and you
> were able to avoid the leaks.
> 
> Reviewed-by: Eric Blake 

Ack, I'll change it.

thanks

[Qemu-devel] [PULLv2 15/25] acpi-build: fix array leak

[Marc-André Lureau]

The free_ranges array is used as a temporary pointer array, the segment
should still be freed, however, it shouldn't free the elements themself.

Signed-off-by: Marc-André Lureau 
Tested-by: Marcel Apfelbaum 
Reviewed-by: Marcel Apfelbaum 
---
 hw/i386/acpi-build.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a26a4bb..433feba 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -789,7 +789,7 @@ static gint crs_range_compare(gconstpointer a, 
gconstpointer b)
 static void crs_replace_with_free_ranges(GPtrArray *ranges,
  uint64_t start, uint64_t end)
 {
-GPtrArray *free_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+GPtrArray *free_ranges = g_ptr_array_new();
 uint64_t free_base = start;
 int i;
 
@@ -813,7 +813,7 @@ static void crs_replace_with_free_ranges(GPtrArray *ranges,
 g_ptr_array_add(ranges, g_ptr_array_index(free_ranges, i));
 }
 
-g_ptr_array_free(free_ranges, false);
+g_ptr_array_free(free_ranges, true);
 }
 
 /*
-- 
2.10.0

[Qemu-devel] [PATCH 2/6] crypto: clear out buffer after timing pbkdf algorithm

[Daniel P. Berrange]

The 'out' buffer will hold a key derived from master
password, so it is best practice to clear this buffer
when no longer required.

Signed-off-by: Daniel P. Berrange 
---
 crypto/pbkdf.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/crypto/pbkdf.c b/crypto/pbkdf.c
index 695cc35..35dccc2 100644
--- a/crypto/pbkdf.c
+++ b/crypto/pbkdf.c
@@ -67,13 +67,14 @@ int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
const uint8_t *salt, size_t nsalt,
Error **errp)
 {
+int ret = -1;
 uint8_t out[32];
 long long int iterations = (1 << 15);
 unsigned long long delta_ms, start_ms, end_ms;
 
 while (1) {
 if (qcrypto_pbkdf2_get_thread_cpu(_ms, errp) < 0) {
-return -1;
+goto cleanup;
 }
 if (qcrypto_pbkdf2(hash,
key, nkey,
@@ -81,10 +82,10 @@ int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
iterations,
out, sizeof(out),
errp) < 0) {
-return -1;
+goto cleanup;
 }
 if (qcrypto_pbkdf2_get_thread_cpu(_ms, errp) < 0) {
-return -1;
+goto cleanup;
 }
 
 delta_ms = end_ms - start_ms;
@@ -103,8 +104,12 @@ int qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
 if (iterations > INT32_MAX) {
 error_setg(errp, "Iterations %lld too large for a 32-bit int",
iterations);
-return -1;
+goto cleanup;
 }
 
-return iterations;
+ret = iterations;
+
+ cleanup:
+memset(out, 0, sizeof(out));
+return ret;
 }
-- 
2.7.4

[Qemu-devel] [PULLv2 17/25] pc: free i8259

[Marc-André Lureau]

Simiarly to 2ba154cf4eb8636cdd3aa90f392ca9e77206ca39

Signed-off-by: Marc-André Lureau 
Reviewed-by: Marcel Apfelbaum 
---
 hw/i386/pc_q35.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c0b9961..c5e8367 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -213,6 +213,8 @@ static void pc_q35_init(MachineState *machine)
 for (i = 0; i < ISA_NUM_IRQS; i++) {
 gsi_state->i8259_irq[i] = i8259[i];
 }
+g_free(i8259);
+
 if (pcmc->pci_enabled) {
 ioapic_init_gsi(gsi_state, "q35");
 }
-- 
2.10.0

[Qemu-devel] [PULLv2 22/25] tests: add qtest_add_data_func_full

[Marc-André Lureau]

Allows one to specify a destroy function for the test data.

Add a fallback using glib g_test_add_vtable() internal function, whose
signature changed over time. Tested with glib 2.22, 2.26 and 2.48, which
according to git log should be enough to cover all variations.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
---
 tests/libqtest.c | 19 +++
 tests/libqtest.h | 17 +
 2 files changed, 36 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index eb00f13..42ccb62 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -758,6 +758,25 @@ void qtest_add_func(const char *str, void (*fn)(void))
 g_free(path);
 }
 
+void qtest_add_data_func_full(const char *str, void *data,
+  void (*fn)(const void *),
+  GDestroyNotify data_free_func)
+{
+gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
+#if GLIB_CHECK_VERSION(2, 34, 0)
+g_test_add_data_func_full(path, data, fn, data_free_func);
+#elif GLIB_CHECK_VERSION(2, 26, 0)
+/* back-compat casts, remove this once we can require new-enough glib */
+g_test_add_vtable(path, 0, data, NULL,
+  (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
+#else
+/* back-compat casts, remove this once we can require new-enough glib */
+g_test_add_vtable(path, 0, data, NULL,
+  (void (*)(void)) fn, (void (*)(void)) data_free_func);
+#endif
+g_free(path);
+}
+
 void qtest_add_data_func(const char *str, const void *data,
  void (*fn)(const void *))
 {
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 37f37ad..d2b4853 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -425,6 +425,23 @@ void qtest_add_func(const char *str, void (*fn)(void));
 void qtest_add_data_func(const char *str, const void *data,
  void (*fn)(const void *));
 
+/**
+ * qtest_add_data_func_full:
+ * @str: Test case path.
+ * @data: Test case data
+ * @fn: Test case function
+ * @data_free_func: GDestroyNotify for data
+ *
+ * Add a GTester testcase with the given name, data and function.
+ * The path is prefixed with the architecture under test, as
+ * returned by qtest_get_arch().
+ *
+ * @data is passed to @data_free_func() on test completion.
+ */
+void qtest_add_data_func_full(const char *str, void *data,
+  void (*fn)(const void *),
+  GDestroyNotify data_free_func);
+
 /**
  * qtest_add:
  * @testpath: Test case path
-- 
2.10.0

[Qemu-devel] [PULLv2 23/25] tests: pc-cpu-test leaks fixes

[Marc-André Lureau]

The path is allocated and should be freed.

The qmp response should be unref, but then 'machine' must be duplicated.

Use a destroy function for the PCTestData.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
---
 tests/pc-cpu-test.c | 24 +++-
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/tests/pc-cpu-test.c b/tests/pc-cpu-test.c
index 4428cea..c3a2633 100644
--- a/tests/pc-cpu-test.c
+++ b/tests/pc-cpu-test.c
@@ -14,7 +14,7 @@
 #include "qapi/qmp/types.h"
 
 struct PCTestData {
-const char *machine;
+char *machine;
 const char *cpu_model;
 unsigned sockets;
 unsigned cores;
@@ -71,6 +71,14 @@ static void test_pc_without_cpu_add(gconstpointer data)
 g_free(args);
 }
 
+static void test_data_free(gpointer data)
+{
+PCTestData *pc = data;
+
+g_free(pc->machine);
+g_free(pc);
+}
+
 static void add_pc_test_cases(void)
 {
 QDict *response, *minfo;
@@ -78,7 +86,8 @@ static void add_pc_test_cases(void)
 const QListEntry *p;
 QObject *qobj;
 QString *qstr;
-const char *mname, *path;
+const char *mname;
+char *path;
 PCTestData *data;
 
 qtest_start("-machine none");
@@ -99,7 +108,7 @@ static void add_pc_test_cases(void)
 continue;
 }
 data = g_malloc(sizeof(PCTestData));
-data->machine = mname;
+data->machine = g_strdup(mname);
 data->cpu_model = "Haswell"; /* 1.3+ theoretically */
 data->sockets = 1;
 data->cores = 3;
@@ -119,14 +128,19 @@ static void add_pc_test_cases(void)
 path = g_strdup_printf("cpu/%s/init/%ux%ux%u=%u",
mname, data->sockets, data->cores,
data->threads, data->maxcpus);
-qtest_add_data_func(path, data, test_pc_without_cpu_add);
+qtest_add_data_func_full(path, data, test_pc_without_cpu_add,
+ test_data_free);
+g_free(path);
 } else {
 path = g_strdup_printf("cpu/%s/add/%ux%ux%u=%u",
mname, data->sockets, data->cores,
data->threads, data->maxcpus);
-qtest_add_data_func(path, data, test_pc_with_cpu_add);
+qtest_add_data_func_full(path, data, test_pc_with_cpu_add,
+ test_data_free);
+g_free(path);
 }
 }
+QDECREF(response);
 qtest_end();
 }
 
-- 
2.10.0

[Qemu-devel] [PATCH v9 1/2] virtio-crypto: Add virtio crypto device specification

[Gonglei]

The virtio crypto device is a virtual crypto device (ie. hardware
crypto accelerator card). The virtio crypto device can provide
five crypto services: CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.

In this patch, CIPHER, MAC, HASH, AEAD services are introduced.

Signed-off-by: Gonglei 
CC: Michael S. Tsirkin 
CC: Cornelia Huck 
CC: Stefan Hajnoczi 
CC: Lingli Deng 
CC: Jani Kokkonen 
CC: Ola Liljedahl 
CC: Varun Sethi 
CC: Zeng Xin 
CC: Keating Brian 
CC: Ma Liang J 
CC: Griffin John 
CC: Hanweidong 
CC: Mihai Claudiu Caraman 
---
 content.tex   |   2 +
 virtio-crypto.tex | 926 ++
 2 files changed, 928 insertions(+)
 create mode 100644 virtio-crypto.tex

diff --git a/content.tex b/content.tex
index 4b45678..ab75f78 100644
--- a/content.tex
+++ b/content.tex
@@ -5750,6 +5750,8 @@ descriptor for the \field{sense_len}, \field{residual},
 \field{status_qualifier}, \field{status}, \field{response} and
 \field{sense} fields.
 
+\input{virtio-crypto.tex}
+
 \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
 
 Currently there are three device-independent feature bits defined:
diff --git a/virtio-crypto.tex b/virtio-crypto.tex
new file mode 100644
index 000..eec4741
--- /dev/null
+++ b/virtio-crypto.tex
@@ -0,0 +1,926 @@
+\section{Crypto Device}\label{sec:Device Types / Crypto Device}
+
+The virtio crypto device is a virtual crypto device, and is a kind of
+virtual hardware accelerator for virtual machines.  The encryption and
+decryption requests are placed in the data queue, and handled by the
+real crypto accelerators finally. The second queue is the control queue,
+which is used to create or destroy sessions for symmetric algorithms, and
+control some advanced features in the future. The virtio crypto
+device can provide seven crypto services: CIPHER, MAC, HASH, AEAD,
+KDF, ASYM, PRIMITIVE.
+
+\subsection{Device ID}\label{sec:Device Types / Crypto Device / Device ID}
+
+20
+
+\subsection{Virtqueues}\label{sec:Device Types / Crypto Device / Virtqueues}
+
+\begin{description}
+\item[0] dataq1
+\item[\ldots]
+\item[N-1] dataqN
+\item[N] controlq
+\end{description}
+
+N is set by \field{max_dataqueues}.
+
+\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature 
bits}
+  None currently defined
+
+\subsection{Device configuration layout}\label{sec:Device Types / Crypto 
Device / Device configuration layout}
+
+The following driver-read-only configuration fields are currently defined.
+
+\begin{lstlisting}
+struct virtio_crypto_config {
+le32  status;
+le32  max_dataqueues;
+le32  crypto_services;
+/* detailed algorithms mask */
+le32 cipher_algo_l;
+le32 cipher_algo_h;
+le32 hash_algo;
+le32 mac_algo_l;
+le32 mac_algo_h;
+le32 asym_algo;
+le32 kdf_algo;
+le32 aead_algo;
+le32 primitive_algo;
+};
+\end{lstlisting}
+
+The first field, \field{status} is currently defined: VIRTIO_CRYPTO_S_HW_READY
+and VIRTIO_CRYPTO_S_STARTED.
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_S_HW_READY  (1 << 0)
+#define VIRTIO_CRYPTO_S_STARTED  (1 << 1)
+\end{lstlisting}
+
+The following driver-read-only field, \field{max_dataqueuess} specifies the
+maximum number of data virtqueues (dataq1\ldots dataqN). The 
\field{crypto_services}
+shows the crypto service the virtio crypto supports. The service currently
+defined are:
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_SERVICE_CIPHER (0) /* cipher service */
+#define VIRTIO_CRYPTO_SERVICE_HASH   (1) /* hash service */
+#define VIRTIO_CRYPTO_SERVICE_MAC(2) /* MAC (Message Authentication Codes) 
service */
+#define VIRTIO_CRYPTO_SERVICE_AEAD   (3) /* AEAD (Authenticated Encryption 
with Associated Data) service */
+\end{lstlisting}
+
+The last driver-read-only fields specify detailed algorithms masks which
+the device offers for corresponding services. The below CIPHER algorithms
+are defined currently:
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_NO_CIPHER 0
+#define VIRTIO_CRYPTO_CIPHER_ARC4   1
+#define VIRTIO_CRYPTO_CIPHER_AES_ECB2
+#define VIRTIO_CRYPTO_CIPHER_AES_CBC3
+#define VIRTIO_CRYPTO_CIPHER_AES_CTR4
+#define VIRTIO_CRYPTO_CIPHER_DES_ECB5
+#define VIRTIO_CRYPTO_CIPHER_DES_CBC6
+#define VIRTIO_CRYPTO_CIPHER_3DES_ECB   7
+#define VIRTIO_CRYPTO_CIPHER_3DES_CBC   8
+#define VIRTIO_CRYPTO_CIPHER_3DES_CTR   9
+#define VIRTIO_CRYPTO_CIPHER_KASUMI_F8  10
+#define VIRTIO_CRYPTO_CIPHER_SNOW3G_UEA211
+#define VIRTIO_CRYPTO_CIPHER_AES_F8 12
+#define VIRTIO_CRYPTO_CIPHER_AES_XTS   

Re: [Qemu-devel] [PATCH for-2.7] qtest.c: Allow zero size in memset qtest commands

[Eric Blake]

On 08/05/2016 05:43 AM, Peter Maydell wrote:
> Some tests use the qtest protocol "memset" command with a zero
> size, expecting it to do nothing. However in the current code this
> will result in calling memset() with a NULL pointer, which is
> undefined behaviour. Detect and specially handle zero sizes to
> avoid this.
> 
> Signed-off-by: Peter Maydell 
> ---
> Looking at the code for the other commands that take a size
> ('read', 'write', 'b64read' and 'b64write' they all assume a
> non-zero size. I've left those alone though, somebody else can
> make them do nothing on zero size if they feel it's important.)

I obviously missed reviewing this in time for 2.7, but looks reasonable
to me.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support

[Jeff Cody]

On Thu, Sep 08, 2016 at 10:34:24AM +0100, Daniel P. Berrange wrote:
> On Thu, Sep 08, 2016 at 11:29:41AM +0200, Kevin Wolf wrote:
> > Am 08.09.2016 um 10:49 hat Daniel P. Berrange geschrieben:
> > > On Thu, Sep 08, 2016 at 10:43:19AM +0200, Kevin Wolf wrote:
> > > > Am 08.09.2016 um 00:32 hat ashish mittal geschrieben:
> > > > > >> +
> > > > > >> +void vxhs_set_acb_buffer(void *ptr, void *buffer)
> > > > > >> +{
> > > > > >> +VXHSAIOCB *acb = ptr;
> > > > > >> +
> > > > > >> +acb->buffer = buffer;
> > > > > >> +}
> > > > > >> +
> > > > > >
> > > > > > Unused function?
> > > > > 
> > > > > This is called from within libqnio.
> > > > 
> > > > Wait, you mean the library references a symbol in the qemu binary? This
> > > > sounds completely wrong to me. I wouldn't even do something like this if
> > > > it were an internal qemu library because I think libraries should be
> > > > self-contained, but it's a much larger problem in something that is
> > > > supposed to be an independent library.
> > > 
> > > I'd be surprised if that actually worked. If an external library wants
> > > to refrence symbols in the QEMU binary, then QEMU would ned to be using
> > > the -export-dynamic / -rdynamic linker flags to export all its symbols,
> > > and AFAIK, we're not doing that.
> > 
> > Hm, but if the function is really used by the library, how else would it
> > be when its name isn't mentioned anywhere in the patch except in its
> > declaration? And it appears to be called there directly:
> > 
> > https://github.com/MittalAshish/libqnio/search?q=vxhs_set_acb_buffer
> > 
> > Anyway, something is fishy here.
> 
> Yeah, and as you say, this approach is just plain wrong, and absolutely
> can never be merged as is. The external library must never use anything
> in QEMU that it wasn't explicitly given via a callback, otherwise that
> library is defacto part of QEMU.
>

Looking at the qnio test server, it does not link against the shim; it looks
like only QEMU links against the shim.

There are at least 3 qemu symbols referenced by the shim library (as of v5):
vxhs_dec_acb_segment_count, vxhs_inc_acb_segment_count, vxhs_set_acb_buffer.

So given that:

1. QEMU is the only user of the shim,
2. The shim calls QEMU functions directly,
3. The shim only exists to bridge QEMU and the QNIO library,
   (From the shim .c file:
   "Network IO library for VxHS QEMU block driver ")
4. The shim makes assumptions about QEMU (e.g. sector size alignment)

I strongly agree with everyone that the shim should just be part of QEMU,
and is the better choice over simply refactoring out QEMU calls from the
shim.


Jeff

Re: [Qemu-devel] [PATCH v3] Move max-bandwidth and downtime-limit into migrate_set_parameter for both hmp and qmp

[Ashijeet Acharya]

On Thu, Sep 8, 2016 at 8:11 PM, Juan Quintela  wrote:
> Eric Blake  wrote:
>
>>> +if (has_max_bandwidth) {
>>> +s->parameters.max_bandwidth = max_bandwidth;
>>> +if (s->to_dst_file) {
>>> +qemu_file_set_rate_limit(s->to_dst_file,
>>> +s->parameters.max_bandwidth / 
>>> XFER_LIMIT_RATIO);
>>> +}
>>> +}
>>> +if (has_downtime_limit) {
>>> +downtime_limit *= 100; /* convert to nanoseconds */
>>
>> Are you sure this won't overflow?
>
> Ashijeet, Eric mere means that if downtime_limit is bigger that
> INT64_MAX/100, then you get an overflow with the multiplication.

Oh I get it now.

> Notice that if it overflows, the value is already quite big O:-)
>
> 2^63/1000/1000/1000/3600/24/365
> 292.47
>
> Allowing "only" 292 years of downtime should be enough for the time
> being (famous last words) O:-)
>

Haha! 292 years seems sufficient. :-)

>
>>
>>> +void qmp_migrate_set_speed(int64_t valuebw, Error **errp)
>>> +{
>>> +bool has_compress_level = false;
>>> +bool has_compress_threads = false;
>>> +bool has_decompress_threads = false;
>>> +bool has_cpu_throttle_initial = false;
>>> +bool has_cpu_throttle_increment = false;
>>> +bool has_tls_creds = false;
>>> +bool has_tls_hostname = false;
>>> +bool has_max_bandwidth = true;
>>> +bool has_downtime_limit = false;
>>> +const char *valuestr = NULL;
>>> +long valueint = 0;
>>> +Error *err = NULL;
>>> +
>>> +qmp_migrate_set_parameters(has_compress_level, valueint,
>>> +   has_compress_threads, valueint,
>>
>> Ugg. This looks gross.  No need to name a bunch of variables set to
>> false, when you can instead use QAPI's boxed conventions to pass a
>> pointer to a struct, and rely on 0-initialization of the struct to leave
>> all the parameters that you don't care about unmentioned.
>
> We should change qmp_migrate_set_parameters to your new api.  I fully
> agree that this is gross, but it is the way it was written, nothing to
> do with this patch, really.

Yeah, you expressed your disgust about this in your 'to-do list of
migration mail' too I remember.

>
> Ashijeet, if you want to do this change in a different patch before this
> change, I am all for it, but that is independent of your change.

I can try, but I am not sure about it as I am very new to QAPI's methods.
Its better if I embarked upon such a task later with more knowledge of QAPI.

> With all the other suggestions of Eric, I agree, or I don't care.
> (If time is an int in milliseconds or a float, I don't really care one
> way or another.  Whatever libvirt preffers).

I am keeping it to milliseconds for now then!

Juan, do not apply the patch just yet. Let me rectify the small
mistakes regarding grammatical errors in comments and send the updated
patch in a few. Accept it after that.

Thanks
Ashijeet
>
> Thanks, Juan.

Re: [Qemu-devel] [PATCH for-2.8 v1 00/60] Modular build of trace event files

[Lluís Vilanova]

Daniel P Berrange writes:

> I previously split the global trace-events file up into one file
> per-subdirectory to avoid merge conflict hell.
[...]

Sorry, I could not find the message where the infrastructure is modified to
provide this. But I think there's a more efficient way to provide modular
auto-generated tracing code without the hierarchical indexing you proposed.

What about using global variables? Instead of the dstate array, each event could
have this on the "public" header:

  /* define for static state */
  #define TRACE_EVENTNAME_ENABLED 1
  /* pointer to event descriptor */
  extern TraceEvent *TRACE_EVENTNAME;
  /* variable with dynamic state */
  extern bool ___TRACE_EVENTNAME_DSTATE;

  void trace_eventname(...) {
  if (trace_event_get_stateTRACE_EVENTNAME_ENABLED && 
___trace_eventname_dstate) {
  /* ... */
  }
  }

The use of event IDs on generic code can be adapted like this:

  #define trace_event_get_state_dynamic_by_id(id) \
  (unlikely(trace_events_enabled_count) && \
   (___ ## id ## _DSTATE))

And then we can concatenate all "trace-events" files to generate the .c files:

  struct TraceEvent {
  /* ... */
  bool *dstate;
  };

  bool ___TRACE_EVENTNAME_DSTATE;

  struct TraceEvent ___trace_events[] = {
  {
  .name = "eventname",
  .sstate = 1,
  .dstate = ___trace_eventname_dstate;
  }
  }

  TraceEvent *TRACE_EVENTNAME = &___trace_events[...];

So updating a single "trace-events" file does not force a recompile of the whole
QEMU, but we retain the performance of the flat dstate array (now a per-event
pointer) and the simpler flat structure for iteration based on event names.


Cheers,
  Lluis

Re: [Qemu-devel] [PATCH 2/2] vhost-user: only seek a reply if needed in set_mem_table

[Michael S. Tsirkin]

On Thu, Sep 08, 2016 at 07:56:38AM -0400, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
> > Regarding Patch 2/2:
> > This patch seems to filter responses from set_mem_table only for certain
> > updates of memory regions. It violates the definition of the REPLY_ACK
> > feature. This feature expects the client to send a response for every call
> > of set_mem_table. And here, qemu exits the set_mem_table() function in some
> > cases without even waiting for the reply that is going to come in.
> > 
> 
> Agreed with Prerna here,
> 
> Furthermore, I haven't followed closely the recents developments, and the 
> commit message doesn't explain why it should not be necessary to sync the 
> first time set-mem-table is called. Could you develop that?
> 
> thanks

Basically if we send set mem table when backend is not started,
there is no reason to wait for a response.

-- 
MST

Re: [Qemu-devel] [PATCH for-2.8 v1 04/60] trace: move hw/virtio/virtio-balloon.c trace points into correct file

[Eric Blake]

On 08/09/2016 10:31 AM, Daniel P. Berrange wrote:
> The trace points for hw/virtio/virtio-balloon.c were mistakenly put
> in the top level trace-events file, instead of util/trace-events.

Is it worth updating the commit messages to commit ids that caused that
need for 1-4 in your series?

-- 
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 v2] iscsi: Fix divide-by-zero regression on raw SG devices

[Holger Schranz]

Hi Eric,

Thanks a lot, it seems the patch works. The VM starting.
Unfortunately we run into the next issue. By the accessing the
megasas controller we got a SIGSEGV.

See the trave below.

Best regards

Holger

===

Thread 4 "CPU 1/KVM" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fe6c622b700 (LWP 18444)]
0x559049a34965 in megasas_handle_frame (s=0x7fe6c5658010,
frame_addr=935342080, frame_count=0)
at /home/shl/Install/qemu-2.7.0/hw/scsi/megasas.c:1984
1984cmd->frame->header.cmd_status = frame_status;
(gdb) bt
#0  0x559049a34965 in megasas_handle_frame (s=0x7fe6c5658010,
frame_addr=935342080, frame_count=0) at
/home/shl/Install/qemu-2.7.0/hw/scsi/megasas.c:1984
#1  0x559049a34f90 in megasas_mmio_write (opaque=0x7fe6c5658010,
addr=64, val=935342081, size=8) at
/home/shl/Install/qemu-2.7.0/hw/scsi/megasas.c:2125
#2  0x559049743cbc in memory_region_write_accessor
(mr=0x7fe6c56588a0, addr=64, value=0x7fe6c622a7b8, size=8, shift=0,
mask=18446744073709551615, attrs=...) at
/home/shl/Install/qemu-2.7.0/memory.c:525
#3  0x559049743f36 in access_with_adjusted_size (addr=64,
value=0x7fe6c622a7b8, size=4, access_size_min=8, access_size_max=8, access=
0x559049743bc5 , mr=0x7fe6c56588a0,
attrs=...) at /home/shl/Install/qemu-2.7.0/memory.c:591
#4  0x559049746e62 in memory_region_dispatch_write
(mr=0x7fe6c56588a0, addr=64, data=935342081, size=4, attrs=...)
at /home/shl/Install/qemu-2.7.0/memory.c:1262
#5  0x5590496eb4f8 in address_space_write_continue
(as=0x55904a25bc00 , addr=4272095296, attrs=...,
buf=0x7fe6da415028 "\001\060\300\067", len=4, addr1=64, l=4,
mr=0x7fe6c56588a0)
at /home/shl/Install/qemu-2.7.0/exec.c:2544
#6  0x5590496eb6d8 in address_space_write (as=0x55904a25bc00
, addr=4272095296, attrs=..., buf=0x7fe6da415028
"\001\060\300\067", len=4) at /home/shl/Install/qemu-2.7.0/exec.c:2601
#7  0x5590496eba88 in address_space_rw (as=0x55904a25bc00
, addr=4272095296, attrs=..., buf=0x7fe6da415028
"\001\060\300\067", len=4, ---Type  to continue, or q 
to quit---q
Python Exception  Quit:
#8  0x55904973fa2a in kvm_cpu_exec (cpu=0x55904c7df620)
at /home/shl/Install/qemu-2.7.0/kvm-all.c:1965
#9  0x559049723116 in qemu_kvm_cpu_thread_fn (arg=0x55904c7df620)
at /home/shl/Install/qemu-2.7.0/cpus.c:1078
#10 0x7fe6d76440a4 in start_thread () at /lib64/libpthread.so.0
#11 0x7fe6d335202d in clone () at /lib64/libc.so.6
(gdb) print cmd
$1 = (MegasasCmd *) 0x7fe6c5658c20
(gdb) print *cmd
$2 = {index = 0, flags = 1, count = 0, context = 0, pa = 0, pa_size = 1024,
  frame = 0x0, req = 0x0, qsg = {sg = 0x0, nsg = 0, nalloc = 0, size = 0,
dev = 0x0, as = 0x0}, iov_buf = 0x0, iov_size = 512, iov_offset = 0,
  state = 0x7fe6c5658010}
(gdb) print cmd->cmd
There is no member named cmd.
(gdb) print cmd->frame
$3 = (union mfi_frame *) 0x0
(gdb)

=

megasas:

(gdb) bt
#0  0x559049a34965 in megasas_handle_frame (s=0x7fe6c5658010,
frame_addr=935342080, frame_count=0) at
/home/shl/Install/qemu-2.7.0/hw/scsi/megasas.c:1984
#1  0x559049a34f90 in megasas_mmio_write (opaque=0x7fe6c5658010,
addr=64, val=935342081, size=8) at
/home/shl/Install/qemu-2.7.0/hw/scsi/megasas.c:2125
:

/home/kvm/SOURCES/qemu.git/qemu/hw/scsi> diff megasas.c
/home/kvm/SOURCES/qemu-2.6.1/hw/scsi/megasas.c
32c32
< #include "qapi/error.h"
---
>
51c51,55
:
413a422,423
>
> return bcd_time;
1984c1994,1998
< cmd->frame->header.cmd_status = frame_status;
---
> if (cmd->frame) {
> cmd->frame->header.cmd_status = frame_status;
> } else {
> megasas_frame_set_cmd_status(s, frame_addr, frame_status);
> }
2301c2315,2317
< msi_uninit(d);
---
:


Am 07.09.2016 um 23:27 schrieb Eric Blake:

When qemu uses iscsi devices in sg mode, iscsilun->block_size
is left at 0.  Prior to commits cf081fca and similar, when
block limits were tracked in sectors, this did not matter:
various block limits were just left at 0.  But when we started
scaling by block size, this caused SIGFPE.

Then, in a later patch, commit a5b8dd2c added an assertion to
bdrv_open_common() that request_alignment is always non-zero;
which was not true for SG mode.  Rather than relax that assertion,
we can just provide a sane value (we don't know of any SG device
with a block size smaller than qemu's default sizing of 512 bytes).

One possible solution for SG mode is to just blindly skip ALL
of iscsi_refresh_limits(), since we already short circuit so
many other things in sg mode.  But this patch takes a slightly
more conservative approach, and merely guarantees that scaling
will succeed, while still using multiples of the original size
where possible.  Resulting limits may still be zero in SG mode
(that is, we mostly only fix block_size used as a denominator
or which affect assertions, not all uses).

Re: [Qemu-devel] [PATCH RFC v2 09/22] block/pcache: separation AIOCB on requests

[Pavel Butsykin]

On 02.09.2016 12:10, Kevin Wolf wrote:

Am 29.08.2016 um 19:10 hat Pavel Butsykin geschrieben:

for case when the cache partially covers request we are part of the request
is filled from the cache, and the other part request from disk. Also add
reference counting for nodes, as way to maintain multithreading.

There is still no full synchronization in multithreaded mode.

Signed-off-by: Pavel Butsykin 
---
  block/pcache.c | 169 -
  1 file changed, 155 insertions(+), 14 deletions(-)

diff --git a/block/pcache.c b/block/pcache.c
index 28bd056..6114289 100644
--- a/block/pcache.c
+++ b/block/pcache.c
@@ -58,7 +58,10 @@ typedef struct BlockNode {
  typedef struct PCNode {
  BlockNode cm;

+uint32_t status;


I guess this is NODE_*_STATUS. Make it a named enum then instead of
uint32_t so that it's obvious what this field means.


OK


+uint32_t ref;
  uint8_t  *data;
+CoMutex  lock;
  } PCNode;

  typedef struct ReqStor {
@@ -95,9 +98,23 @@ typedef struct PrefCacheAIOCB {
  uint64_t sector_num;
  uint32_t nb_sectors;
  int  aio_type;
+struct {
+QTAILQ_HEAD(req_head, PrefCachePartReq) list;
+CoMutex lock;
+} requests;
  int  ret;
  } PrefCacheAIOCB;

+typedef struct PrefCachePartReq {
+uint64_t sector_num;
+uint32_t nb_sectors;


Should be byte-based, like everything.


+QEMUIOVector qiov;
+PCNode *node;
+PrefCacheAIOCB *acb;
+QTAILQ_ENTRY(PrefCachePartReq) entry;
+} PrefCachePartReq;
+
  static const AIOCBInfo pcache_aiocb_info = {
  .aiocb_size = sizeof(PrefCacheAIOCB),
  };
@@ -126,8 +143,39 @@ static QemuOptsList runtime_opts = {
  #define MB_BITS 20
  #define PCACHE_DEFAULT_CACHE_SIZE (4 << MB_BITS)

+enum {
+NODE_SUCCESS_STATUS = 0,
+NODE_WAIT_STATUS= 1,
+NODE_REMOVE_STATUS  = 2,
+NODE_GHOST_STATUS   = 3 /* only for debugging */


NODE_DELETED_STATUS?


Yes :)


+};
+
  #define PCNODE(_n) ((PCNode *)(_n))

+static inline void pcache_node_unref(PCNode *node)
+{
+assert(node->status == NODE_SUCCESS_STATUS ||
+   node->status == NODE_REMOVE_STATUS);
+
+if (atomic_fetch_dec(>ref) == 0) {


Atomics imply concurrency, which we don't have.


+assert(node->status == NODE_REMOVE_STATUS);
+
+node->status = NODE_GHOST_STATUS;
+g_free(node->data);
+g_slice_free1(sizeof(*node), node);


When you switch to plain g_malloc(), this needs to be updated.


+}
+}
+
+static inline PCNode *pcache_node_ref(PCNode *node)
+{
+assert(node->status == NODE_SUCCESS_STATUS ||
+   node->status == NODE_WAIT_STATUS);
+assert(atomic_read(>ref) == 0);/* XXX: only for sequential requests 
*/
+atomic_inc(>ref);


Do you expect concurrent accesses or not? Because if you don't, there is
no need for atomics, but if you do, this is buggy because each of the
lines is atomic for itself, but the assertion isn't atomic with the
refcount increment.


Well, about concurrent accesses, we've already figured out.


A ref() function that can take only a single reference feels odd anyway
and this restriction seems to be lifted later. Why have it here?


No, this is a temporary assert(). In fact, it is not necessary, but the
assert helps to check the correct functioning on the current patch,
because not yet implemented reading of nodes and rescheduling requests.


+
+return node;
+}


Kevin