[Qemu-devel] [PATCH v1 0/7] KVM: Hyper-V SynIC timers

[Andrey Smetanin]

Per Hyper-V specification (and as required by Hyper-V-aware guests),
SynIC provides 4 per-vCPU timers.  Each timer is programmed via a pair
of MSRs, and signals expiration by delivering a special format message
to the configured SynIC message slot and triggering the corresponding
synthetic interrupt.

Note: as implemented by this patch, all periodic timers are "lazy"
(i.e. if the vCPU wasn't scheduled for more than the timer period the
timer events are lost), regardless of the corresponding configuration
MSR.  If deemed necessary, the "catch up" mode (the timer period is
shortened until the timer catches up) will be implemented later.

The Hyper-V SynIC timers support is required to load winhv.sys
inside Windows guest on which guest VMBus devices depends on.

This patches depends on Hyper-V SynIC patches previosly sent.

Signed-off-by: Andrey Smetanin 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org

Andrey Smetanin (7):
  drivers/hv: Move HV_SYNIC_STIMER_COUNT into Hyper-V UAPI x86 header
  drivers/hv: Move struct hv_message into UAPI Hyper-V x86 header
  kvm/x86: Rearrange func's declarations inside Hyper-V header
  kvm/x86: Added Hyper-V vcpu_to_hv_vcpu()/hv_vcpu_to_vcpu() helpers
  kvm/x86: Hyper-V internal helper to read MSR HV_X64_MSR_TIME_REF_COUNT
  kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
  kvm/x86: Hyper-V SynIC timers

 arch/x86/include/asm/kvm_host.h|  13 ++
 arch/x86/include/uapi/asm/hyperv.h |  99 ++
 arch/x86/kvm/hyperv.c  | 367 -
 arch/x86/kvm/hyperv.h  |  54 --
 arch/x86/kvm/x86.c |   9 +
 drivers/hv/hyperv_vmbus.h  |  93 --
 include/linux/kvm_host.h   |   3 +
 7 files changed, 527 insertions(+), 111 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack

[Andrey Smetanin]

The SynIC message protocol mandates that the message slot is claimed
by atomically setting message type to something other than HVMSG_NONE.
If another message is to be delivered while the slot is still busy,
message pending flag is asserted to indicate to the guest that the
hypervisor wants to be notified when the slot is released.

To make sure the protocol works regardless of where the message
sources are (kernel or userspace), clear the pending flag on SINT ACK
notification, and let the message sources compete for the slot again.

Signed-off-by: Andrey Smetanin 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.c| 31 +++
 include/linux/kvm_host.h |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 9958926..6412b6b 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -27,6 +27,7 @@
 #include "hyperv.h"
 
 #include 
+#include 
 #include 
 #include 
 
@@ -116,13 +117,43 @@ static struct kvm_vcpu_hv_synic *synic_get(struct kvm 
*kvm, u32 vcpu_id)
return (synic->active) ? synic : NULL;
 }
 
+static void synic_clear_sint_msg_pending(struct kvm_vcpu_hv_synic *synic,
+   u32 sint)
+{
+   struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
+   struct page *page;
+   gpa_t gpa;
+   struct hv_message *msg;
+   struct hv_message_page *msg_page;
+
+   gpa = synic->msg_page & PAGE_MASK;
+   page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
+   if (is_error_page(page)) {
+   vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
+gpa);
+   return;
+   }
+   msg_page = kmap_atomic(page);
+
+   msg = _page->sint_message[sint];
+   msg->header.message_flags.msg_pending = 0;
+
+   kunmap_atomic(msg_page);
+   kvm_release_page_dirty(page);
+   kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
+}
+
 static void kvm_hv_notify_acked_sint(struct kvm_vcpu *vcpu, u32 sint)
 {
struct kvm *kvm = vcpu->kvm;
+   struct kvm_vcpu_hv_synic *synic = vcpu_to_synic(vcpu);
int gsi, idx;
 
vcpu_debug(vcpu, "Hyper-V SynIC acked sint %d\n", sint);
 
+   if (synic->msg_page & HV_SYNIC_SIMP_ENABLE)
+   synic_clear_sint_msg_pending(synic, sint);
+
idx = srcu_read_lock(>irq_srcu);
gsi = atomic_read(_to_synic(vcpu)->sint_to_gsi[sint]);
if (gsi != -1)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2911919..9b64c8c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -450,6 +450,8 @@ struct kvm {
 
 #define vcpu_debug(vcpu, fmt, ...) \
kvm_debug("vcpu%i " fmt, (vcpu)->vcpu_id, ## __VA_ARGS__)
+#define vcpu_err(vcpu, fmt, ...)   \
+   kvm_err("vcpu%i " fmt, (vcpu)->vcpu_id, ## __VA_ARGS__)
 
 static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i)
 {
-- 
2.4.3

Re: [Qemu-devel] [RESEND RFC 2/6] device_tree: introduce load_device_tree_from_sysfs

[Alex Bennée]

Eric Auger  writes:

> This function returns the host device tree blob from sysfs
> (/sys/firmware/devicetree/base).
>
> This has a runtime dependency on the dtc binary. This functionality
> is useful for platform device passthrough where the host device tree
> needs to be parsed to feed information into the guest device tree.
>
> Signed-off-by: Eric Auger 
> ---
>  device_tree.c| 40 
>  include/sysemu/device_tree.h |  1 +
>  2 files changed, 41 insertions(+)
>
> diff --git a/device_tree.c b/device_tree.c
> index a9f5f8e..58a5329 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -117,6 +117,46 @@ fail:
>  return NULL;
>  }
>
> +/**
> + * load_device_tree_from_sysfs
> + *
> + * extract the dt blob from host sysfs
> + * this has a runtime dependency on the dtc binary
> + */
> +void *load_device_tree_from_sysfs(void)
> +{
> +char cmd[] = "dtc -I fs -O dtb /sys/firmware/devicetree/base";
> +FILE *pipe;
> +void *fdt;
> +int ret, actual_dt_size;
> +
> +pipe = popen(cmd, "r");
> +if (!pipe) {
> +error_report("%s: Error when executing dtc", __func__);
> +return NULL;
> +}
> +fdt = g_malloc0(FDT_MAX_SIZE);
> +actual_dt_size = fread(fdt, 1, FDT_MAX_SIZE, pipe);
> +pclose(pipe);

I think this looks OK but I'm wary of anything that calls out to a
external script. However it seems the other popen() case is for
migration and that looks as though it will get removed by the IO Channel
framework stuff.

There may be millage in using g_spawn_sync() as it will return you an
automatically sized buffer.

> +
> +if (actual_dt_size == 0) {
> +error_report("%s: could not copy host device tree in memory: %m",
> + __func__);
> +goto fail;
> +}
> +ret = fdt_check_header(fdt);
> +if (ret) {
> +error_report("%s: Host dt file loaded into memory is invalid: %s",
> + __func__, fdt_strerror(ret));
> +goto fail;
> +}
> +return fdt;
> +
> +fail:
> +g_free(fdt);
> +return NULL;
> +}
> +
>  static int findnode_nofail(void *fdt, const char *node_path)
>  {
>  int offset;
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index 359e143..307e53d 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -16,6 +16,7 @@
>
>  void *create_device_tree(int *sizep);
>  void *load_device_tree(const char *filename_path, int *sizep);
> +void *load_device_tree_from_sysfs(void);
>
>  int qemu_fdt_setprop(void *fdt, const char *node_path,
>   const char *property, const void *val, int size);


--
Alex Bennée

Re: [Qemu-devel] [PATCH v2 14/14] iotests: Add "qemu-img map" test for VMDK extents

[Eric Blake]

On 11/25/2015 12:39 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  tests/qemu-iotests/059 | 10 ++
>  tests/qemu-iotests/059.out | 38 ++
>  2 files changed, 48 insertions(+)

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] [RFC PATCH V2 09/10] Qemu/VFIO: Add SRIOV VF migration support

[Michael S. Tsirkin]

On Wed, Nov 25, 2015 at 11:32:23PM +0800, Lan, Tianyu wrote:
> 
> On 11/25/2015 5:03 AM, Michael S. Tsirkin wrote:
> >>>+void vfio_migration_cap_handle(PCIDevice *pdev, uint32_t addr,
> >>>+  uint32_t val, int len)
> >>>+{
> >>>+VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
> >>>+
> >>>+if (addr == vdev->migration_cap + PCI_VF_MIGRATION_VF_STATUS
> >>>+&& val == PCI_VF_READY_FOR_MIGRATION) {
> >>>+qemu_event_set(_event);
> >This would wake migration so it can proceed -
> >except it needs QEMU lock to run, and that's
> >taken by the migration thread.
> 
> Sorry, I seem to miss something.
> Which lock may cause dead lock when calling vfio_migration_cap_handle()
> and run migration?

qemu_global_mutex.

> The function is called when VF accesses faked PCI capability.
> 
> >
> >It seems unlikely that this ever worked - how
> >did you test this?
> >

Re: [Qemu-devel] [PATCH v7 12/24] virtio-blk: Functions for op blocker management

[Max Reitz]

On 25.11.2015 17:18, Kevin Wolf wrote:
> Am 25.11.2015 um 17:03 hat Max Reitz geschrieben:
>> On 25.11.2015 16:57, Kevin Wolf wrote:
>>> Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
 Put the code for setting up and removing op blockers into an own
 function, respectively. Then, we can invoke those functions whenever a
 BDS is removed from an virtio-blk BB or inserted into it.

 Signed-off-by: Max Reitz 
>>>
>>> Do you know of a case where this is observable?
>>
>> Actually, no.
>>
>>> I don't think you can
>>> change the medium of a virtio-blk device, and blk_set_bs() isn't
>>> converted to a wrapper around blk_remove/insert_bs() yet. If this patch
>>> is necessary to fix something observable, the latter is probably a bug.
>>
>> So I guess that implies "Otherwise, this patch should be dropped"?
> 
> I'm not sure. I guess you had a reason to include these patches other
> than putting the notifiers to use?

I'm not sure, it has been so long. :-)

I can very well imagine having included it because a similar change is
necessary to virtio-scsi, so I included it just because I was already
doing work for virtio. Maybe I imagined that virtio-blk may reasonably
offer tray devices in the future, but I just now read you saying
somewhere else:

> Please write your code so that it makes sense today.

So that doesn't hold up. :-)

Maybe I just saw it unblocking the EJECT op blocker, and so I thought
there might be a reason for that.

> With blk_set_bs() changed, I think it would have an effect: The op
> blockers would move from the old BDS to the new top-level one. This
> sounds desirable to me.

Hm, thanks for saving me. Yes, that seems useful indeed.

[...]

>>> This makes me wonder: What do we even block here any more? If I didn't
>>> miss anything, it's only BLOCK_OP_TYPE_BACKUP_TARGET, and I'm not sure
>>> why this needs to be blocked, or if we simply forgot to enable it.
>>
>> Well, even though in practice this wall of code doesn't make much sense,
>> of course it will be safe for potential additions of new op blockers.
>>
>> And of course we actually don't want these blockers at all anymore...
> 
> Yes, but dataplane shouldn't really be special enough any more that we
> want to disable features for it initially. By now it sounds more like an
> easy way to forget unblocking a new feature even though it would work.
> 
> So perhaps we should really just remove the blockers from dataplane.
> Then we don't have to answer the question above...

Well, maybe. I guess this is up to Stefan.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Paolo Bonzini]

On 25/11/2015 16:49, Eduardo Habkost wrote:
> Instead of silently clearing mcg_cap bits when the host doesn't
> support them, print a warning when doing that.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  target-i386/kvm.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index d63a85b..446bdfc 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -774,7 +774,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  && (env->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
> (CPUID_MCE | CPUID_MCA)
>  && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
> -uint64_t mcg_cap;
> +uint64_t mcg_cap, unsupported_caps;
>  int banks;
>  int ret;
>  
> @@ -790,6 +790,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  return -ENOTSUP;
>  }
>  
> +unsupported_caps = env->mcg_cap & ~(mcg_cap | MCG_CAP_BANKS_MASK);
> +if (unsupported_caps) {
> +error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64 
> "\n",

\n should not be at end of error_report.

Fixed and applied.

Paolo

> + unsupported_caps);
> +}
> +
>  env->mcg_cap &= mcg_cap | MCG_CAP_BANKS_MASK;
>  ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, >mcg_cap);
>  if (ret < 0) {
> 

Re: [Qemu-devel] [PATCH v3 0/2] qga: flush explicitly when needed

[Michael Roth]

Quoting marcandre.lur...@redhat.com (2015-11-25 06:59:10)
> From: Marc-André Lureau 
> 
> Without this change, a write() followed by a read() may lose the
> previously written content, as shown in the following test.
> 
> v2->v3:
> - use a RwState tristate enum
> - reset the state on flush & seek
> 
> v1->v2:
> - replace guchar with unsigned char
> - fix implicitly/explictly
> - comment space fix
> 
> Marc-André Lureau (2):
>   qga: flush explicitly when needed
>   tests: add file-write-read test

Thanks, applied with fix-ups suggested by Lazlo:

  https://github.com/mdroth/qemu/commits/qga

> 
>  qga/commands-posix.c | 37 
>  tests/test-qga.c | 95 
> ++--
>  2 files changed, 130 insertions(+), 2 deletions(-)
> 
> -- 
> 2.5.0
> 

Re: [Qemu-devel] [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Borislav Petkov]

On Wed, Nov 25, 2015 at 01:49:49PM -0200, Eduardo Habkost wrote:
> Instead of silently clearing mcg_cap bits when the host doesn't
> support them, print a warning when doing that.

Why the host? Why would we want there to be any relation between the MCA
capabilities of the host and what qemu is emulating?

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

Re: [Qemu-devel] [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Paolo Bonzini]

On 25/11/2015 18:21, Borislav Petkov wrote:
>> Instead of silently clearing mcg_cap bits when the host doesn't
>> > support them, print a warning when doing that.
> Why the host? Why would we want there to be any relation between the MCA
> capabilities of the host and what qemu is emulating?

He means the hypervisor. :)

Paolo

Re: [Qemu-devel] [PATCH v7 12/24] virtio-blk: Functions for op blocker management

[Kevin Wolf]

Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
> Put the code for setting up and removing op blockers into an own
> function, respectively. Then, we can invoke those functions whenever a
> BDS is removed from an virtio-blk BB or inserted into it.
> 
> Signed-off-by: Max Reitz 

Do you know of a case where this is observable? I don't think you can
change the medium of a virtio-blk device, and blk_set_bs() isn't
converted to a wrapper around blk_remove/insert_bs() yet. If this patch
is necessary to fix something observable, the latter is probably a bug.

> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index c42ddeb..4c95d5b 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -39,6 +39,8 @@ struct VirtIOBlockDataPlane {
>  EventNotifier *guest_notifier;  /* irq */
>  QEMUBH *bh; /* bh for guest notification */
>  
> +Notifier insert_notifier, remove_notifier;
> +
>  /* Note that these EventNotifiers are assigned by value.  This is
>   * fine as long as you do not call event_notifier_cleanup on them
>   * (because you don't own the file descriptor or handle; you just
> @@ -137,6 +139,54 @@ static void handle_notify(EventNotifier *e)
>  blk_io_unplug(s->conf->conf.blk);
>  }
>  
> +static void data_plane_set_up_op_blockers(VirtIOBlockDataPlane *s)
> +{
> +assert(!s->blocker);
> +error_setg(>blocker, "block device is in use by data plane");
> +blk_op_block_all(s->conf->conf.blk, s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_RESIZE, s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_DRIVE_DEL, s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_BACKUP_SOURCE, 
> s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_CHANGE, s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_SOURCE, 
> s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_TARGET, 
> s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EJECT, s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
> +   s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT,
> +   s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
> +   s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_MIRROR, s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker);
> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker);
> +}

This makes me wonder: What do we even block here any more? If I didn't
miss anything, it's only BLOCK_OP_TYPE_BACKUP_TARGET, and I'm not sure
why this needs to be blocked, or if we simply forgot to enable it.

Kevin

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

[Eric Blake]

On 11/25/2015 05:59 AM, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> This test exhibits a POSIX behaviour regarding switching between write
> and read. It's undefined result if the application doesn't ensure a
> flush between the two operations (with glibc, the flush can be implicit
> when the buffer size is relatively small). The previous commit fixes
> this test.
> 
> Related to:
> https://bugzilla.redhat.com/show_bug.cgi?id=1210246
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  tests/test-qga.c | 95 
> ++--
>  1 file changed, 93 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake 

> +/* seek to 0 */
> +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
> +  " 'arguments': { 'handle': %" PRId64 ", "
> +  " 'offset': %d, 'whence': %d } }",
> +  id, 0, SEEK_SET);

We still have a conflict between this series and my proposal to codify 0
rather than relying on platform-specific SEEK_SET; Markus had the
suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
both your series and my v2 patch into 2.5?  Knowing that will help me
decide whether my v2 should be rebased on top of your patches.

-- 
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 v7 12/24] virtio-blk: Functions for op blocker management

[Max Reitz]

On 25.11.2015 16:57, Kevin Wolf wrote:
> Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
>> Put the code for setting up and removing op blockers into an own
>> function, respectively. Then, we can invoke those functions whenever a
>> BDS is removed from an virtio-blk BB or inserted into it.
>>
>> Signed-off-by: Max Reitz 
> 
> Do you know of a case where this is observable?

Actually, no.

> I don't think you can
> change the medium of a virtio-blk device, and blk_set_bs() isn't
> converted to a wrapper around blk_remove/insert_bs() yet. If this patch
> is necessary to fix something observable, the latter is probably a bug.

So I guess that implies "Otherwise, this patch should be dropped"?

>> diff --git a/hw/block/dataplane/virtio-blk.c 
>> b/hw/block/dataplane/virtio-blk.c
>> index c42ddeb..4c95d5b 100644
>> --- a/hw/block/dataplane/virtio-blk.c
>> +++ b/hw/block/dataplane/virtio-blk.c
>> @@ -39,6 +39,8 @@ struct VirtIOBlockDataPlane {
>>  EventNotifier *guest_notifier;  /* irq */
>>  QEMUBH *bh; /* bh for guest notification */
>>  
>> +Notifier insert_notifier, remove_notifier;
>> +
>>  /* Note that these EventNotifiers are assigned by value.  This is
>>   * fine as long as you do not call event_notifier_cleanup on them
>>   * (because you don't own the file descriptor or handle; you just
>> @@ -137,6 +139,54 @@ static void handle_notify(EventNotifier *e)
>>  blk_io_unplug(s->conf->conf.blk);
>>  }
>>  
>> +static void data_plane_set_up_op_blockers(VirtIOBlockDataPlane *s)
>> +{
>> +assert(!s->blocker);
>> +error_setg(>blocker, "block device is in use by data plane");
>> +blk_op_block_all(s->conf->conf.blk, s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_RESIZE, s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_DRIVE_DEL, s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_BACKUP_SOURCE, 
>> s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_CHANGE, s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_SOURCE, 
>> s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_TARGET, 
>> s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EJECT, s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
>> +   s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT,
>> +   s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, 
>> BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
>> +   s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_MIRROR, s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker);
>> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker);
>> +}
> 
> This makes me wonder: What do we even block here any more? If I didn't
> miss anything, it's only BLOCK_OP_TYPE_BACKUP_TARGET, and I'm not sure
> why this needs to be blocked, or if we simply forgot to enable it.

Well, even though in practice this wall of code doesn't make much sense,
of course it will be safe for potential additions of new op blockers.

And of course we actually don't want these blockers at all anymore...

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v7 13/24] virtio-scsi: Catch BDS-BB removal/insertion

[Kevin Wolf]

Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
> Make use of the BDS-BB removal and insertion notifiers to remove or set
> up, respectively, virtio-scsi's op blockers.
> 
> Signed-off-by: Max Reitz 

> @@ -797,6 +830,29 @@ static void virtio_scsi_hotunplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>  if (s->ctx) {
>  blk_op_unblock_all(sd->conf.blk, s->blocker);
>  }
> +
> +QTAILQ_FOREACH(insert_notifier, >insert_notifiers, next) {
> +if (insert_notifier->sd == sd) {
> +break;
> +}
> +}
> +if (insert_notifier) {
> +notifier_remove(_notifier->n);
> +QTAILQ_REMOVE(>insert_notifiers, insert_notifier, next);
> +g_free(insert_notifier);
> +}

Why a separate if block instead of just doing that inside the loop?

> +QTAILQ_FOREACH(remove_notifier, >remove_notifiers, next) {
> +if (remove_notifier->sd == sd) {
> +break;
> +}
> +}
> +if (remove_notifier) {
> +notifier_remove(_notifier->n);
> +QTAILQ_REMOVE(>remove_notifiers, remove_notifier, next);
> +g_free(remove_notifier);
> +}
> +
>  qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
>  }

Kevin

Re: [Qemu-devel] [RFC PATCH V2 3/3] Ixgbevf: Add migration support for ixgbevf driver

[Lan, Tianyu]

On 11/25/2015 8:28 PM, Michael S. Tsirkin wrote:

Frankly, I don't really see what this short term hack buys us,
and if it goes in, we'll have to maintain it forever.



The framework of how to notify VF about migration status won't be
changed regardless of stopping VF or not before doing migration.
We hope to reach agreement on this first. Tracking dirty memory still
need to more discussions and we will continue working on it. Stop VF may
help to work around the issue and make tracking easier.



Also, assuming you just want to do ifdown/ifup for some reason, it's
easy enough to do using a guest agent, in a completely generic way.



Just ifdown/ifup is not enough for migration. It needs to restore some 
PCI settings before doing ifup on the target machine

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

[Eric Blake]

On 11/25/2015 09:46 AM, Michael Roth wrote:
> Quoting Eric Blake (2015-11-25 10:41:35)
>> On 11/25/2015 09:21 AM, Michael Roth wrote:
>>
> +/* seek to 0 */
> +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
> +  " 'arguments': { 'handle': %" PRId64 ", "
> +  " 'offset': %d, 'whence': %d } }",
> +  id, 0, SEEK_SET);

 We still have a conflict between this series and my proposal to codify 0
 rather than relying on platform-specific SEEK_SET; Markus had the
 suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
 both your series and my v2 patch into 2.5?  Knowing that will help me
 decide whether my v2 should be rebased on top of your patches.
>>>
>>> I was planning on pulling in your patch on top of this for the next
>>> 2.5 pull, so rebasing on top of this series is probably best.
>>
>> Okay, will do, and will do quickly so I'm not holding up your pull request.
> 
> Thanks! FYI I now have this series applied here if you'd like to base
> on that:
> 
>   https://github.com/mdroth/qemu/commits/qga

On that branch, commit 7a38932 has two typos:
s/is commit msg (Lazlo)/in commit msg (Laszlo)/

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL 7/9] target-i386: kvm: Abort if MCE bank count is not supported by host

[Paolo Bonzini]

From: Eduardo Habkost 

Instead of silently changing the number of banks in mcg_cap based
on kvm_get_mce_cap_supported(), abort initialization if the host
doesn't support MCE_BANKS_DEF banks.

Note that MCE_BANKS_DEF was always 10 since it was introduced in
QEMU, and Linux always returned 32 at KVM_CAP_MCE since
KVM_CAP_MCE was introduced, so no behavior is being changed and
the error can't be triggered by any Linux version. The point of
the new check is to ensure we won't silently change the bank
count if we change MCE_BANKS_DEF or make the bank count
configurable in the future.

Signed-off-by: Eduardo Habkost 
[Avoid Yoda condition and \n at end of error_report. - Paolo]
Signed-off-by: Paolo Bonzini 
---
 target-i386/kvm.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 2a9953b..93d1f5e 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -784,11 +784,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
 return ret;
 }
 
-if (banks > MCE_BANKS_DEF) {
-banks = MCE_BANKS_DEF;
+if (banks < MCE_BANKS_DEF) {
+error_report("kvm: Unsupported MCE bank count (QEMU = %d, KVM = 
%d)",
+ MCE_BANKS_DEF, banks);
+return -ENOTSUP;
 }
+
 mcg_cap &= MCE_CAP_DEF;
-mcg_cap |= banks;
+mcg_cap |= MCE_BANKS_DEF;
 ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, _cap);
 if (ret < 0) {
 fprintf(stderr, "KVM_X86_SETUP_MCE: %s", strerror(-ret));
-- 
1.8.3.1

[Qemu-devel] [PULL 8/9] target-i386: kvm: Use env->mcg_cap when setting up MCE

[Paolo Bonzini]

From: Eduardo Habkost 

When setting up MCE, instead of using the MCE_*_DEF macros
directly, just filter the existing env->mcg_cap value.

As env->mcg_cap is already initialized as
MCE_CAP_DEF|MCE_BANKS_DEF at target-i386/cpu.c:mce_init(), this
doesn't change any behavior. But it will allow us to change
mce_init() in the future, to implement different defaults
depending on CPU model, machine-type or command-line parameters.

Signed-off-by: Eduardo Habkost 
Signed-off-by: Paolo Bonzini 
---
 target-i386/cpu.h |  2 ++
 target-i386/kvm.c | 11 ---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index fc4a605..84edfd0 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -286,6 +286,8 @@
 #define MCE_CAP_DEF (MCG_CTL_P|MCG_SER_P)
 #define MCE_BANKS_DEF   10
 
+#define MCG_CAP_BANKS_MASK 0xff
+
 #define MCG_STATUS_RIPV (1ULL<<0)   /* restart ip valid */
 #define MCG_STATUS_EIPV (1ULL<<1)   /* ip points to correct instruction */
 #define MCG_STATUS_MCIP (1ULL<<2)   /* machine check in progress */
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 93d1f5e..90bd447 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -784,21 +784,18 @@ int kvm_arch_init_vcpu(CPUState *cs)
 return ret;
 }
 
-if (banks < MCE_BANKS_DEF) {
+if (banks < (env->mcg_cap & MCG_CAP_BANKS_MASK)) {
 error_report("kvm: Unsupported MCE bank count (QEMU = %d, KVM = 
%d)",
- MCE_BANKS_DEF, banks);
+ (int)(env->mcg_cap & MCG_CAP_BANKS_MASK), banks);
 return -ENOTSUP;
 }
 
-mcg_cap &= MCE_CAP_DEF;
-mcg_cap |= MCE_BANKS_DEF;
-ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, _cap);
+env->mcg_cap &= mcg_cap | MCG_CAP_BANKS_MASK;
+ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, >mcg_cap);
 if (ret < 0) {
 fprintf(stderr, "KVM_X86_SETUP_MCE: %s", strerror(-ret));
 return ret;
 }
-
-env->mcg_cap = mcg_cap;
 }
 
 qemu_add_vm_change_state_handler(cpu_update_state, env);
-- 
1.8.3.1

[Qemu-devel] [PULL 9/9] target-i386: kvm: Print warning when clearing mcg_cap bits

[Paolo Bonzini]

From: Eduardo Habkost 

Instead of silently clearing mcg_cap bits when the host doesn't
support them, print a warning when doing that.

Signed-off-by: Eduardo Habkost 
[Avoid \n at end of error_report. - Paolo]
Signed-off-by: Paolo Bonzini 
---
 target-i386/kvm.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 90bd447..6dc9846 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -774,7 +774,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 && (env->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
(CPUID_MCE | CPUID_MCA)
 && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
-uint64_t mcg_cap;
+uint64_t mcg_cap, unsupported_caps;
 int banks;
 int ret;
 
@@ -790,6 +790,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
 return -ENOTSUP;
 }
 
+unsupported_caps = env->mcg_cap & ~(mcg_cap | MCG_CAP_BANKS_MASK);
+if (unsupported_caps) {
+error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64,
+ unsupported_caps);
+}
+
 env->mcg_cap &= mcg_cap | MCG_CAP_BANKS_MASK;
 ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, >mcg_cap);
 if (ret < 0) {
-- 
1.8.3.1

[Qemu-devel] [PULL 3/9] call bdrv_drain_all() even if the vm is stopped

[Paolo Bonzini]

From: Wen Congyang 

There are still I/O operations when the vm is stopped. For example,
stop the vm, and do block migration. In this case, we don't drain all
I/O operation, and may meet the following problem:

qemu-system-x86_64: migration/block.c:731: block_save_complete: Assertion 
`block_mig_state.submitted == 0' failed.

Signed-off-by: Wen Congyang 
Message-Id: <564ee92e.4070...@cn.fujitsu.com>
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
---
 cpus.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cpus.c b/cpus.c
index 877bd70..43676fa 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1415,6 +1415,8 @@ int vm_stop_force_state(RunState state)
 return vm_stop(state);
 } else {
 runstate_set(state);
+
+bdrv_drain_all();
 /* Make sure to return an error if the flush in a previous vm_stop()
  * failed. */
 return bdrv_flush_all();
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Borislav Petkov]

On Wed, Nov 25, 2015 at 06:29:25PM +0100, Paolo Bonzini wrote:
> On 25/11/2015 18:21, Borislav Petkov wrote:
> >> Instead of silently clearing mcg_cap bits when the host doesn't
> >> > support them, print a warning when doing that.
> > Why the host? Why would we want there to be any relation between the MCA
> > capabilities of the host and what qemu is emulating?
> 
> He means the hypervisor. :)

Ah, ok. :)

Then they look good to me, a step in the right direction.

Acked-by: Borislav Petkov 

Thanks!

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

[Qemu-devel] [PATCH v1 0/2] QEMU: Hyper-V SynIC timers MSR's support

[Andrey Smetanin]

Hyper-V SynIC timers are host timers that are configurable
by guest through corresponding MSR's (HV_X64_MSR_STIMER*).
Guest setup and use fired by host events(SynIC interrupt
and appropriate timer expiration message) as guest clock
events.

The state of Hyper-V SynIC timers are stored in corresponding
MSR's. This patch seria implements such MSR's support and migration.

Signed-off-by: Andrey Smetanin 
CC: Paolo Bonzini 
CC: Richard Henderson 
CC: Eduardo Habkost 
CC: "Andreas Färber" 
CC: Marcelo Tosatti 
CC: Denis V. Lunev 
CC: Roman Kagan 
CC: k...@vger.kernel.org

Andrey Smetanin (2):
  include: update Hyper-V header to include SynIC timers defines
  target-i386/kvm: Hyper-V SynIC timers MSR's support

 include/standard-headers/asm-x86/hyperv.h | 99 +++
 target-i386/cpu-qom.h |  1 +
 target-i386/cpu.c |  1 +
 target-i386/cpu.h |  2 +
 target-i386/kvm.c | 50 +++-
 target-i386/machine.c | 29 +
 6 files changed, 181 insertions(+), 1 deletion(-)

-- 
2.4.3

Re: [Qemu-devel] [PATCH v2 13/14] qemu-img: Use QAPI visitor to generate JSON

[Eric Blake]

On 11/25/2015 12:39 AM, Fam Zheng wrote:
> A visible improvement is that "filename" is now included in the output
> if it's valid.
> 
> Signed-off-by: Fam Zheng 
> ---
>  qemu-img.c | 39 ---
>  tests/qemu-iotests/122.out | 96 
> ++
>  2 files changed, 79 insertions(+), 56 deletions(-)
> 

> @@ -2155,21 +2161,26 @@ static void dump_map_entry(OutputFormat 
> output_format, MapEntry *e,
>  }
>  break;
>  case OFORMAT_JSON:
> -printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64","
> -   " \"depth\": %ld,"
> -   " \"zero\": %s, \"data\": %s",
> -   (e->start == 0 ? "[" : ",\n"),
> -   e->start, e->length, e->depth,
> -   e->zero ? "true" : "false",
> -   e->data ? "true" : "false");
> -if (e->has_offset) {
> -printf(", \"offset\": %"PRId64"", e->offset);
> -}
> -putchar('}');
> +ov = qmp_output_visitor_new();
> +visit_type_MapEntry(qmp_output_get_visitor(ov),
> +, NULL, _abort);
> +obj = qmp_output_get_qobject(ov);
> +str = qobject_to_json(obj);

It would be nice to someday add a visitor that goes directly to json,
instead of through an intermediate QObject. But that's not for this
series; your conversion here is sane.

> @@ -2213,9 +2224,9 @@ static int get_block_status(BlockDriverState *bs, 
> int64_t sector_num,
>  e->offset = ret & BDRV_BLOCK_OFFSET_MASK;
>  e->has_offset = !!(ret & BDRV_BLOCK_OFFSET_VALID);
>  e->depth = depth;
> -if (e->has_offset) {
> +if (file && e->has_offset) {
>  e->has_filename = true;
> -e->filename = bs->filename;
> +e->filename = file->filename;

Does this hunk belong in a different patch?

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 v2 2/4] qemu-iotests: s390x: fix test 051

[Max Reitz]

On 24.11.2015 22:17, Sascha Silbe wrote:
> Dear Max,

Hi! :-)

> Max Reitz  writes:
> 
>> OK, so it is expected for s390x; however, this is strictly speaking not
>> the output file for s390x but for any platform but PC. That's why I'd
>> rather not have it in this “generic” reference output.
>>
>> This patch already assumes that the iotests only support s390 and PC
>> (hunk @@ -251,28 +273,37 @@ in 051 adds a case statement which knows
>> only these two cases). Therefore, I would be fine with renaming this
>> reference output file to "051.s390.out" with a copy
>> "051.s390-ccw-virtio.out" and just removing the generic "051.out". Then
>> you can keep the warnings in.
>>
>> (Or you filter the warnings out and keep it as "051.out".)
> 
> This PC/s390x-only hunk looks like an oversight to me.

Not really, see
http://lists.nongnu.org/archive/html/qemu-devel/2015-02/msg01906.html
and
http://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg02851.html

I noticed, but I am fine with it since the tests probably won't run on
anything but x86/pc and s390 anyway (without modifications; most of the
changes this series is making to make the iotests work on s390 are
necessary for other non-pc platforms as well, and that shows to me that
apparently nobody tried to run the iotests on non-pc platforms before
s390, or didn't care enough about them to fix them).

>We should make
> one of the options the default. I'd prefer defaulting to virtio (see
> below), but since the test previously hard-coded IDE that would be fine,
> too.

In my first reply above, I noted that virtio0 may not be available on
all platforms either. Therefore, I'd rather have an explicit list of
platforms there than an asterisk where it does not belong.

However, my second reply above spawned a bit of a discussion, where
Kevin simply proposed to change the ID of the drive to something known,
i.e. just set the ID by adding an id=drive0 or something to the -drive
parameter.

Thanks for reminding me of the above, I had already forgotten. Indeed,
we should just add id=drive0 to the -drive parameter and use drive0. A
similar solution may be possible in most other places as well where PC
and s390 differ due to the names of the default devices available.

> For the other cases, IIRC it's really PC that's the odd one out, that's
> why I suggested adding a PC-specific output file and patching the
> generic output file to look like the output on most other architectures.

Actually, I remember it was me:
http://lists.nongnu.org/archive/html/qemu-devel/2015-02/msg00862.html ;-)

(Not that it really matters, of course)

> But I'm fine with anything that gets the job done on both PC and s390x
> today.

As am I.

>I'll have a closer look again once things settle down a
> bit. While the support for one output file per architecture is a very
> useful generic solution, we should make use of it only sparingly. The
> git log already shows that people forget to update test output
> files. This will get worse with multiple output files.

Well, maintaining the iotests for s390 may be a difficult task in
itself. Most people (including myself) don't have an s390 machine, so we
can't test whether modifications or additions we make to the iotests
work there.

> One of the things I'm considering is splitting off the IDE tests to a
> separate test case and skipping it entirely on machines that do not
> support IDE. Another is using virtio-blk on all architectures whenever
> the test case doesn't care about the device type. (I doubt the tests
> currently work on architectures that don't support virtio, but will of
> course check this).

The question is whether we really do have IDE test cases in the iotests.
I don't think so, the actual IDE tests should be part of qtest. The
iotests only test the block layer itself, so as I noted above we should
most of the time get around the issues addressed in this series by
simply manually setting the ID of the drive we are creating (instead of
relying on a certain default name like ide0-hd0 or virtio0).

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 0/6] Block patches for 2.5.0-rc2

[Peter Maydell]

On 25 November 2015 at 14:10, Kevin Wolf  wrote:
> The following changes since commit 1aae36df4b8ed884c6ef6995e70c67fad79b49df:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-ivshmem-2015-11-25' 
> into staging (2015-11-25 11:38:03 +)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 8c34d891b1594840d8394a3c9b92236c13254fd8:
>
>   Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-25' 
> into queue-block (2015-11-25 14:33:01 +0100)
>
> 
>
> Block layer patches
>
> 
> Fam Zheng (1):
>   qemu-iotests: Add -nographic when starting QEMU in 119 and 120
>
> Kevin Wolf (3):
>   tests/Makefile: Add more dependencies for test-timed-average
>   test-aio: Fix event notifier cleanup
>   Merge remote-tracking branch 
> 'mreitz/tags/pull-block-for-kevin-2015-11-25' into queue-block
>
> Markus Armbruster (1):
>   block/qapi: Plug memory leak on query-block error path
>
> Programmingkid (1):
>   raw-posix.c: Make GetBSDPath() handle caching options
>
> Ricard Wanderlof (1):
>   nand: fix flash erase when oob is in memory
>
>  block/qapi.c   |  8 +++-
>  block/raw-posix.c  | 15 +--
>  hw/block/nand.c|  2 +-
>  tests/Makefile |  3 +--
>  tests/qemu-iotests/119 |  2 +-
>  tests/qemu-iotests/120 |  2 +-
>  tests/test-aio.c   |  1 +
>  7 files changed, 17 insertions(+), 16 deletions(-)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2 07/14] qed: Assign bs->file->bs to file in bdrv_qed_co_get_block_status

[Eric Blake]

On 11/25/2015 12:39 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  block/qed.c | 3 +++
>  1 file changed, 3 insertions(+)
> 

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] [RFC PATCH V2 3/3] Ixgbevf: Add migration support for ixgbevf driver

[Michael S. Tsirkin]

On Wed, Nov 25, 2015 at 08:24:38AM -0800, Alexander Duyck wrote:
> >> Also, assuming you just want to do ifdown/ifup for some reason, it's
> >> easy enough to do using a guest agent, in a completely generic way.
> >>
> >
> > Just ifdown/ifup is not enough for migration. It needs to restore some PCI
> > settings before doing ifup on the target machine
> 
> That is why I have been suggesting making use of suspend/resume logic
> that is already in place for PCI power management.  In the case of a
> suspend/resume we already have to deal with the fact that the device
> will go through a D0->D3->D0 reset so we have to restore all of the
> existing state.  It would take a significant load off of Qemu since
> the guest would be restoring its own state instead of making Qemu have
> to do all of the device migration work.

That can work, though again, the issue is you need guest
cooperation to migrate.

If you reset device on destination instead of restoring state,
then that issue goes away, but maybe the downtime
will be increased.

Will it really? I think it's worth it to start with the
simplest solution (reset on destination) and see
what the effect is, then add optimizations.


One thing that I've been thinking about for a while, is saving (some)
state speculatively.  For example, notify guest a bit before migration
is done, so it can save device state. If guest responds quickly, you
have state that can be restored.  If it doesn't, still migrate, and it
will have to reset on destination.


-- 
MST

Re: [Qemu-devel] [PATCH for 2.5 1/1] qga: gspawn() console helper to Windows guest agent msi build

[Michael Roth]

Quoting Denis V. Lunev (2015-11-19 06:20:37)
> From: Yuri Pudgorodskiy 
> 
> This helper, gspawn-win64-helper-console.exe for 64-bit and
> gspawn-win32-helper-console.exe for 32-bit environment,
> is needed for gspawn() mingw implementation, used by guest-exec command.
> 
> Without these files guest-exec command on Windows will not
> work with "file not found" diagnostic message.
> 
> Signed-off-by: Yuri Pudgorodskiy 
> Signed-off-by: Denis V. Lunev 
> CC: Michael Roth 

Thanks, applied to qga tree:

  https://github.com/mdroth/qemu/commits/qga

> ---
>  qga/installer/qemu-ga.wxs | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> index 6804f02..f25afdd 100644
> --- a/qga/installer/qemu-ga.wxs
> +++ b/qga/installer/qemu-ga.wxs
> @@ -91,6 +91,16 @@
>   Source="$(env.BUILD_DIR)/qga/vss-win32/qga-vss.tlb" KeyPath="yes" DiskId="1"/>
>
>
> +  
> +   Guid="{446185B3-87BE-43D2-96B8-0FEFD9E8696D}">
> + Name="gspawn-win32-helper-console.exe" 
> Source="$(var.Mingw_bin)/gspawn-win32-helper-console.exe" KeyPath="yes" 
> DiskId="1"/>
> +  
> +  
> +  
> +   Guid="{9E615A9F-349A-4992-A5C2-C10BAD173660}">
> + Name="gspawn-win64-helper-console.exe" 
> Source="$(var.Mingw_bin)/gspawn-win64-helper-console.exe" KeyPath="yes" 
> DiskId="1"/>
> +  
> +  
> Guid="{35EE3558-D34B-4F0A-B8BD-430FF0775246}">
>   Source="$(var.Mingw_bin)/iconv.dll" KeyPath="yes" DiskId="1"/>
>
> @@ -148,6 +158,7 @@
>
>
>
> +  
>
>
>
> -- 
> 2.1.4
> 

Re: [Qemu-devel] [PATCH 1/3] target-i386: kvm: Abort if MCE bank count is not supported by host

[Paolo Bonzini]

On 25/11/2015 18:26, Eduardo Habkost wrote:
>> > Yoda conditions?
>> > 
>> > if (banks < MCE_BANKS_DEF) {
>> > error_report("kvm: Unsupported MCE bank count (QEMU = %d, KVM 
>> > = %d)",
>> >  MCE_BANKS_DEF, banks);
> This was on purpose, because MCE_BANKS_DEF is replaced by
> (env->mcg_caps & MCG_CAPS_COUNT_MASK) in the next patch.

Yeah, I noticed it later.

Paolo

Re: [Qemu-devel] [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Paolo Bonzini]

On 25/11/2015 18:29, Eduardo Habkost wrote:
>>> > >  
>>> > > +unsupported_caps = env->mcg_cap & ~(mcg_cap | 
>>> > > MCG_CAP_BANKS_MASK);
>>> > > +if (unsupported_caps) {
>>> > > +error_report("warning: Unsupported MCG_CAP bits: 0x%" 
>>> > > PRIx64 "\n",
>> > 
>> > \n should not be at end of error_report.
>> > 
>> > Fixed and applied.
> MCG_CAP_BANKS_MASK is defined by patch 2/3. Have you applied the
> whole series?

Yes, of course.

Paolo

[Qemu-devel] [PATCH for-2.5 v2] qga: Better mapping of SEEK_* in guest-file-seek

[Eric Blake]

Exposing OS-specific SEEK_ constants in our qapi was a mistake
(if the host has SEEK_CUR as 1, but the guest has it as 2, then
the semantics are unclear what should happen); if we had a time
machine, we would instead expose only a symbolic enum.  It's too
late to change the fact that we have an integer in qapi, but we
can at least document what mapping we want to enforce for all
qga clients (and luckily, it happens to be the mapping that both
Linux and Windows use); then fix the code to match that mapping.
It also helps us filter out unsupported SEEK_DATA and SEEK_HOLE.

In the future, we may wish to move our QGA_SEEK_* constants into
qga/qapi-schema.json, along with updating the schema to take an
alternate type (either the integer, or the string value of the
enum name) - but that's too much risk during hard freeze.

Signed-off-by: Eric Blake 

---
v2: rebase to mdroth/qga, add QGA_SEEK_* constants instead of magic
numbers
---
 qga/commands-posix.c   | 19 ++-
 qga/commands-win32.c   | 20 +++-
 qga/guest-agent-core.h |  7 +++
 qga/qapi-schema.json   |  4 ++--
 tests/test-qga.c   |  5 +++--
 5 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index cf1d7ec..c2ff970 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -553,17 +553,34 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, 
const char *buf_b64,
 }

 struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset,
-  int64_t whence, Error **errp)
+  int64_t whence_code, Error **errp)
 {
 GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
 GuestFileSeek *seek_data = NULL;
 FILE *fh;
 int ret;
+int whence;

 if (!gfh) {
 return NULL;
 }

+/* We stupidly exposed 'whence':'int' in our qapi */
+switch (whence_code) {
+case QGA_SEEK_SET:
+whence = SEEK_SET;
+break;
+case QGA_SEEK_CUR:
+whence = SEEK_CUR;
+break;
+case QGA_SEEK_END:
+whence = SEEK_END;
+break;
+default:
+error_setg(errp, "invalid whence code %"PRId64, whence_code);
+return NULL;
+}
+
 fh = gfh->fh;
 ret = fseek(fh, offset, whence);
 if (ret == -1) {
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 41f6dd9..0654fe4 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -382,7 +382,7 @@ done:
 }

 GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset,
-   int64_t whence, Error **errp)
+   int64_t whence_code, Error **errp)
 {
 GuestFileHandle *gfh;
 GuestFileSeek *seek_data;
@@ -390,11 +390,29 @@ GuestFileSeek *qmp_guest_file_seek(int64_t handle, 
int64_t offset,
 LARGE_INTEGER new_pos, off_pos;
 off_pos.QuadPart = offset;
 BOOL res;
+int whence;
+
 gfh = guest_file_handle_find(handle, errp);
 if (!gfh) {
 return NULL;
 }

+/* We stupidly exposed 'whence':'int' in our qapi */
+switch (whence_code) {
+case QGA_SEEK_SET:
+whence = SEEK_SET;
+break;
+case QGA_SEEK_CUR:
+whence = SEEK_CUR;
+break;
+case QGA_SEEK_END:
+whence = SEEK_END;
+break;
+default:
+error_setg(errp, "invalid whence code %"PRId64, whence_code);
+return NULL;
+}
+
 fh = gfh->fh;
 res = SetFilePointerEx(fh, off_pos, _pos, whence);
 if (!res) {
diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
index e92c6ab..238dc6b 100644
--- a/qga/guest-agent-core.h
+++ b/qga/guest-agent-core.h
@@ -15,6 +15,13 @@

 #define QGA_READ_COUNT_DEFAULT 4096

+/* Mapping of whence codes used by guest-file-seek. */
+enum {
+QGA_SEEK_SET = 0,
+QGA_SEEK_CUR = 1,
+QGA_SEEK_END = 2,
+};
+
 typedef struct GAState GAState;
 typedef struct GACommandState GACommandState;
 extern GAState *ga_state;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 78362e0..01c9ee4 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -318,13 +318,13 @@
 #
 # Seek to a position in the file, as with fseek(), and return the
 # current file position afterward. Also encapsulates ftell()'s
-# functionality, just Set offset=0, whence=SEEK_CUR.
+# functionality, with offset=0 and whence=1.
 #
 # @handle: filehandle returned by guest-file-open
 #
 # @offset: bytes to skip over in the file stream
 #
-# @whence: SEEK_SET, SEEK_CUR, or SEEK_END, as with fseek()
+# @whence: 0 for SEEK_SET, 1 for SEEK_CUR, or 2 for SEEK_END
 #
 # Returns: @GuestFileSeek on success.
 #
diff --git a/tests/test-qga.c b/tests/test-qga.c
index 3b99d9d..e6a84d1 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -13,6 +13,7 @@

 #include "libqtest.h"
 #include "config-host.h"
+#include "qga/guest-agent-core.h"

 typedef struct {
 char *test_dir;
@@ 

[Qemu-devel] [PATCH v1 7/7] kvm/x86: Hyper-V SynIC timers

[Andrey Smetanin]

Per Hyper-V specification (and as required by Hyper-V-aware guests),
SynIC provides 4 per-vCPU timers.  Each timer is programmed via a pair
of MSRs, and signals expiration by delivering a special format message
to the configured SynIC message slot and triggering the corresponding
synthetic interrupt.

Note: as implemented by this patch, all periodic timers are "lazy"
(i.e. if the vCPU wasn't scheduled for more than the timer period the
timer events are lost), regardless of the corresponding configuration
MSR.  If deemed necessary, the "catch up" mode (the timer period is
shortened until the timer catches up) will be implemented later.

Signed-off-by: Andrey Smetanin 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/include/asm/kvm_host.h|  13 ++
 arch/x86/include/uapi/asm/hyperv.h |   6 +
 arch/x86/kvm/hyperv.c  | 325 -
 arch/x86/kvm/hyperv.h  |  24 +++
 arch/x86/kvm/x86.c |   9 +
 include/linux/kvm_host.h   |   1 +
 6 files changed, 375 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f608e17..e35c5ca 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -375,6 +375,17 @@ struct kvm_mtrr {
struct list_head head;
 };
 
+/* Hyper-V SynIC timer */
+struct kvm_vcpu_hv_stimer {
+   struct hrtimer timer;
+   int index;
+   u64 config;
+   u64 count;
+   u64 exp_time;
+   struct hv_message msg;
+   bool msg_pending;
+};
+
 /* Hyper-V synthetic interrupt controller (SynIC)*/
 struct kvm_vcpu_hv_synic {
u64 version;
@@ -394,6 +405,8 @@ struct kvm_vcpu_hv {
s64 runtime_offset;
struct kvm_vcpu_hv_synic synic;
struct kvm_hyperv_exit exit;
+   struct kvm_vcpu_hv_stimer stimer[HV_SYNIC_STIMER_COUNT];
+   DECLARE_BITMAP(stimer_pending_bitmap, HV_SYNIC_STIMER_COUNT);
 };
 
 struct kvm_vcpu_arch {
diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index e86d77e..f9d3349 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -362,4 +362,10 @@ struct hv_message_page {
struct hv_message sint_message[HV_SYNIC_SINT_COUNT];
 };
 
+#define HV_STIMER_ENABLE   (1ULL << 0)
+#define HV_STIMER_PERIODIC (1ULL << 1)
+#define HV_STIMER_LAZY (1ULL << 2)
+#define HV_STIMER_AUTOENABLE   (1ULL << 3)
+#define HV_STIMER_SINT(config) (__u8)(((config) >> 16) & 0x0F)
+
 #endif
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 6412b6b..9f8eb82 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -147,15 +147,32 @@ static void kvm_hv_notify_acked_sint(struct kvm_vcpu 
*vcpu, u32 sint)
 {
struct kvm *kvm = vcpu->kvm;
struct kvm_vcpu_hv_synic *synic = vcpu_to_synic(vcpu);
-   int gsi, idx;
+   struct kvm_vcpu_hv *hv_vcpu = vcpu_to_hv_vcpu(vcpu);
+   struct kvm_vcpu_hv_stimer *stimer;
+   int gsi, idx, stimers_pending;
 
vcpu_debug(vcpu, "Hyper-V SynIC acked sint %d\n", sint);
 
if (synic->msg_page & HV_SYNIC_SIMP_ENABLE)
synic_clear_sint_msg_pending(synic, sint);
 
+   /* Try to deliver pending Hyper-V SynIC timers messages */
+   stimers_pending = 0;
+   for (idx = 0; idx < ARRAY_SIZE(hv_vcpu->stimer); idx++) {
+   stimer = _vcpu->stimer[idx];
+   if (stimer->msg_pending &&
+   (stimer->config & HV_STIMER_ENABLE) &&
+   HV_STIMER_SINT(stimer->config) == sint) {
+   set_bit(stimer->index,
+   hv_vcpu->stimer_pending_bitmap);
+   stimers_pending++;
+   }
+   }
+   if (stimers_pending)
+   kvm_make_request(KVM_REQ_HV_STIMER, vcpu);
+
idx = srcu_read_lock(>irq_srcu);
-   gsi = atomic_read(_to_synic(vcpu)->sint_to_gsi[sint]);
+   gsi = atomic_read(>sint_to_gsi[sint]);
if (gsi != -1)
kvm_notify_acked_gsi(kvm, gsi);
srcu_read_unlock(>irq_srcu, idx);
@@ -371,9 +388,275 @@ static u64 get_time_ref_counter(struct kvm *kvm)
return div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
 }
 
+static void stimer_mark_expired(struct kvm_vcpu_hv_stimer *stimer,
+   bool vcpu_kick)
+{
+   struct kvm_vcpu *vcpu = stimer_to_vcpu(stimer);
+
+   set_bit(stimer->index,
+   vcpu_to_hv_vcpu(vcpu)->stimer_pending_bitmap);
+   kvm_make_request(KVM_REQ_HV_STIMER, vcpu);
+   if (vcpu_kick)
+   

[Qemu-devel] [PATCH v1 2/7] drivers/hv: Move struct hv_message into UAPI Hyper-V x86 header

[Andrey Smetanin]

This struct is required for Hyper-V SynIC timers implementation inside KVM
and for upcoming Hyper-V VMBus support by userspace(QEMU). So place it into
Hyper-V UAPI header.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/include/uapi/asm/hyperv.h | 91 ++
 drivers/hv/hyperv_vmbus.h  | 91 --
 2 files changed, 91 insertions(+), 91 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 07981f0..e86d77e 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -271,4 +271,95 @@ typedef struct _HV_REFERENCE_TSC_PAGE {
 
 #define HV_SYNIC_STIMER_COUNT  (4)
 
+/* Define synthetic interrupt controller message constants. */
+#define HV_MESSAGE_SIZE(256)
+#define HV_MESSAGE_PAYLOAD_BYTE_COUNT  (240)
+#define HV_MESSAGE_PAYLOAD_QWORD_COUNT (30)
+
+/* Define hypervisor message types. */
+enum hv_message_type {
+   HVMSG_NONE  = 0x,
+
+   /* Memory access messages. */
+   HVMSG_UNMAPPED_GPA  = 0x8000,
+   HVMSG_GPA_INTERCEPT = 0x8001,
+
+   /* Timer notification messages. */
+   HVMSG_TIMER_EXPIRED = 0x8010,
+
+   /* Error messages. */
+   HVMSG_INVALID_VP_REGISTER_VALUE = 0x8020,
+   HVMSG_UNRECOVERABLE_EXCEPTION   = 0x8021,
+   HVMSG_UNSUPPORTED_FEATURE   = 0x8022,
+
+   /* Trace buffer complete messages. */
+   HVMSG_EVENTLOG_BUFFERCOMPLETE   = 0x8040,
+
+   /* Platform-specific processor intercept messages. */
+   HVMSG_X64_IOPORT_INTERCEPT  = 0x8001,
+   HVMSG_X64_MSR_INTERCEPT = 0x80010001,
+   HVMSG_X64_CPUID_INTERCEPT   = 0x80010002,
+   HVMSG_X64_EXCEPTION_INTERCEPT   = 0x80010003,
+   HVMSG_X64_APIC_EOI  = 0x80010004,
+   HVMSG_X64_LEGACY_FP_ERROR   = 0x80010005
+};
+
+/* Define synthetic interrupt controller message flags. */
+union hv_message_flags {
+   __u8 asu8;
+   struct {
+   __u8 msg_pending:1;
+   __u8 reserved:7;
+   };
+};
+
+/* Define port identifier type. */
+union hv_port_id {
+   __u32 asu32;
+   struct {
+   __u32 id:24;
+   __u32 reserved:8;
+   } u;
+};
+
+/* Define port type. */
+enum hv_port_type {
+   HVPORT_MSG  = 1,
+   HVPORT_EVENT= 2,
+   HVPORT_MONITOR  = 3
+};
+
+/* Define synthetic interrupt controller message header. */
+struct hv_message_header {
+   enum hv_message_type message_type;
+   __u8 payload_size;
+   union hv_message_flags message_flags;
+   __u8 reserved[2];
+   union {
+   __u64 sender;
+   union hv_port_id port;
+   };
+};
+
+/* Define timer message payload structure. */
+struct hv_timer_message_payload {
+   __u32 timer_index;
+   __u32 reserved;
+   __u64 expiration_time;  /* When the timer expired */
+   __u64 delivery_time;/* When the message was delivered */
+};
+
+/* Define synthetic interrupt controller message format. */
+struct hv_message {
+   struct hv_message_header header;
+   union {
+   __u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
+   } u;
+};
+
+/* Define the synthetic interrupt message page layout. */
+struct hv_message_page {
+   struct hv_message sint_message[HV_SYNIC_SINT_COUNT];
+};
+
 #endif
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 46e23d1..d22230c 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -63,10 +63,6 @@ enum hv_cpuid_function {
 /* Define version of the synthetic interrupt controller. */
 #define HV_SYNIC_VERSION   (1)
 
-/* Define synthetic interrupt controller message constants. */
-#define HV_MESSAGE_SIZE(256)
-#define HV_MESSAGE_PAYLOAD_BYTE_COUNT  (240)
-#define HV_MESSAGE_PAYLOAD_QWORD_COUNT (30)
 #define HV_ANY_VP  (0x)
 
 /* Define synthetic interrupt controller flag constants. */
@@ -74,53 +70,9 @@ enum hv_cpuid_function {
 #define HV_EVENT_FLAGS_BYTE_COUNT  (256)
 #define HV_EVENT_FLAGS_DWORD_COUNT (256 / sizeof(u32))
 
-/* Define hypervisor message types. */
-enum hv_message_type {
-   HVMSG_NONE  = 0x,
-
-   /* Memory access messages. */
-   HVMSG_UNMAPPED_GPA  = 0x8000,
-   HVMSG_GPA_INTERCEPT = 0x8001,
-
-  

Re: [Qemu-devel] [PATCH v7 13/24] virtio-scsi: Catch BDS-BB removal/insertion

[Max Reitz]

On 25.11.2015 17:03, Kevin Wolf wrote:
> Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
>> Make use of the BDS-BB removal and insertion notifiers to remove or set
>> up, respectively, virtio-scsi's op blockers.
>>
>> Signed-off-by: Max Reitz 
> 
>> @@ -797,6 +830,29 @@ static void virtio_scsi_hotunplug(HotplugHandler 
>> *hotplug_dev, DeviceState *dev,
>>  if (s->ctx) {
>>  blk_op_unblock_all(sd->conf.blk, s->blocker);
>>  }
>> +
>> +QTAILQ_FOREACH(insert_notifier, >insert_notifiers, next) {
>> +if (insert_notifier->sd == sd) {
>> +break;
>> +}
>> +}
>> +if (insert_notifier) {
>> +notifier_remove(_notifier->n);
>> +QTAILQ_REMOVE(>insert_notifiers, insert_notifier, next);
>> +g_free(insert_notifier);
>> +}
> 
> Why a separate if block instead of just doing that inside the loop?

Because I unconsciously try to reduce indentation.

Also, because when I wrote the code, to me it was "Find the relevant
notifier -- destroy that notifier", and that's how this pattern came about.

I personally still like it more the way I did it, but I can very well
imagine that I'm the only one who thinks so. Therefore, I wouldn't mind
changing it (besides the effort involved to change it).

Max

>> +QTAILQ_FOREACH(remove_notifier, >remove_notifiers, next) {
>> +if (remove_notifier->sd == sd) {
>> +break;
>> +}
>> +}
>> +if (remove_notifier) {
>> +notifier_remove(_notifier->n);
>> +QTAILQ_REMOVE(>remove_notifiers, remove_notifier, next);
>> +g_free(remove_notifier);
>> +}
>> +
>>  qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
>>  }
> 
> Kevin
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

[Eric Blake]

On 11/25/2015 09:21 AM, Michael Roth wrote:

>>> +/* seek to 0 */
>>> +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
>>> +  " 'arguments': { 'handle': %" PRId64 ", "
>>> +  " 'offset': %d, 'whence': %d } }",
>>> +  id, 0, SEEK_SET);
>>
>> We still have a conflict between this series and my proposal to codify 0
>> rather than relying on platform-specific SEEK_SET; Markus had the
>> suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
>> both your series and my v2 patch into 2.5?  Knowing that will help me
>> decide whether my v2 should be rebased on top of your patches.
> 
> I was planning on pulling in your patch on top of this for the next
> 2.5 pull, so rebasing on top of this series is probably best.

Okay, will do, and will do quickly so I'm not holding up your pull request.

-- 
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 v3 2/2] tests: add file-write-read test

[Eric Blake]

On 11/25/2015 10:14 AM, Michael Roth wrote:
>>> Thanks! FYI I now have this series applied here if you'd like to base
>>> on that:
>>>
>>>   https://github.com/mdroth/qemu/commits/qga
>>
>> On that branch, commit 7a38932 has two typos:
>> s/is commit msg (Lazlo)/in commit msg (Laszlo)/
> 
> Thanks, fixed now.

Except that in e8c9ea9, the message still references write()/read()
instead of the intended fwrite()/fread().  We'll get it eventually :)

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL 5/9] exec: remove warning about mempath and hugetlbfs

[Paolo Bonzini]

From: "Daniel P. Berrange" 

The gethugepagesize() method in exec.c printed a warning if
the file path for "-mem-path" or "-object memory-backend-file"
was not on a hugetlbfs filesystem. This warning is bogus, because
QEMU functions perfectly well with the path on a regular tmpfs
filesystem. Use of hugetlbfs vs tmpfs is a choice for the management
application or end user to make as best fits their needs. As such it
is inappropriate for QEMU to have an opinion on whether the user's
choice is right or wrong in this case.

Signed-off-by: Daniel P. Berrange 
Message-Id: <1448448749-1332-3-git-send-email-berra...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 exec.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/exec.c b/exec.c
index b09f18b..de1cf19 100644
--- a/exec.c
+++ b/exec.c
@@ -1196,9 +1196,6 @@ static long gethugepagesize(const char *path, Error 
**errp)
 return 0;
 }
 
-if (fs.f_type != HUGETLBFS_MAGIC)
-fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path);
-
 return fs.f_bsize;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 4/9] Revert "exec: silence hugetlbfs warning under qtest"

[Paolo Bonzini]

From: "Daniel P. Berrange" 

This reverts commit 1c7ba94a184df1eddd589d5400d879568d3e5d08.

That commit changed QEMU initialization order from

 - object-initial, chardev, qtest, object-late

to

 - chardev, qtest, object-initial, object-late

This breaks chardev setups which need to rely on objects
having been created. For example, when chardevs use TLS
encryption in the future, they need to have tls credential
objects created first.

This revert, restores the ordering introduced in

  commit f08f9271bfe3f19a5eb3d7a2f48532065304d5c8
  Author: Daniel P. Berrange 
  Date:   Wed May 13 17:14:04 2015 +0100

vl: Create (most) objects before creating chardev backends

Signed-off-by: Daniel P. Berrange 
Message-Id: <1448448749-1332-2-git-send-email-berra...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 exec.c |  5 +
 vl.c   | 28 ++--
 2 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/exec.c b/exec.c
index acbd4a2..b09f18b 100644
--- a/exec.c
+++ b/exec.c
@@ -51,7 +51,6 @@
 #include "qemu/main-loop.h"
 #include "translate-all.h"
 #include "sysemu/replay.h"
-#include "sysemu/qtest.h"
 
 #include "exec/memory-internal.h"
 #include "exec/ram_addr.h"
@@ -1197,10 +1196,8 @@ static long gethugepagesize(const char *path, Error 
**errp)
 return 0;
 }
 
-if (!qtest_driver() &&
-fs.f_type != HUGETLBFS_MAGIC) {
+if (fs.f_type != HUGETLBFS_MAGIC)
 fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path);
-}
 
 return fs.f_bsize;
 }
diff --git a/vl.c b/vl.c
index 525929b..4211ff1 100644
--- a/vl.c
+++ b/vl.c
@@ -4291,26 +4291,17 @@ int main(int argc, char **argv, char **envp)
 page_size_init();
 socket_init();
 
-if (qemu_opts_foreach(qemu_find_opts("chardev"),
-  chardev_init_func, NULL, NULL)) {
-exit(1);
-}
-
-if (qtest_chrdev) {
-Error *local_err = NULL;
-qtest_init(qtest_chrdev, qtest_log, _err);
-if (local_err) {
-error_report_err(local_err);
-exit(1);
-}
-}
-
 if (qemu_opts_foreach(qemu_find_opts("object"),
   object_create,
   object_create_initial, NULL)) {
 exit(1);
 }
 
+if (qemu_opts_foreach(qemu_find_opts("chardev"),
+  chardev_init_func, NULL, NULL)) {
+exit(1);
+}
+
 #ifdef CONFIG_VIRTFS
 if (qemu_opts_foreach(qemu_find_opts("fsdev"),
   fsdev_init_func, NULL, NULL)) {
@@ -4337,6 +4328,15 @@ int main(int argc, char **argv, char **envp)
 
 configure_accelerator(current_machine);
 
+if (qtest_chrdev) {
+Error *local_err = NULL;
+qtest_init(qtest_chrdev, qtest_log, _err);
+if (local_err) {
+error_report_err(local_err);
+exit(1);
+}
+}
+
 machine_opts = qemu_get_machine_opts();
 kernel_filename = qemu_opt_get(machine_opts, "kernel");
 initrd_filename = qemu_opt_get(machine_opts, "initrd");
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 12/14] qemu-img: Make MapEntry a QAPI struct

[Eric Blake]

On 11/25/2015 12:39 AM, Fam Zheng wrote:
> The "flags" bit mask is expanded to two booleans, "data" and "zero";
> "bs" is replaced with "filename" string.
> 
> Signed-off-by: Fam Zheng 
> ---
>  qapi/block-core.json | 28 
>  qemu-img.c   | 48 ++--
>  2 files changed, 50 insertions(+), 26 deletions(-)
> 

> +##
> +
> +{ 'struct': 'MapEntry',

Blank line is not typical here.

> +  'data': {'start': 'int', 'length': 'int', 'data': 'bool',
> +   'zero': 'bool', 'depth': 'int', '*offset': 'int',
> +   '*filename': 'str' } }

Struct looks fine.

> 
> -if ((e->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) == 
> BDRV_BLOCK_DATA) {
> +if (e->data && !e->zero) {
>  printf("%#-16"PRIx64"%#-16"PRIx64"%#-16"PRIx64"%s\n",
> -   e->start, e->length, e->offset, e->bs->filename);
> +   e->start, e->length, e->offset,
> +   e->has_filename ? e->filename : "");
>  }

This blindly prints e->offset, even though...[1]

>  case OFORMAT_JSON:
> -printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64", \"depth\": 
> %d,"
> +printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64","
> +   " \"depth\": %ld,"

%ld is wrong; it needs to be PRId64.

> " \"zero\": %s, \"data\": %s",

Worth joining the two short lines into one?

> @@ -2219,10 +2208,15 @@ static int get_block_status(BlockDriverState *bs, 
> int64_t sector_num,
>  
>  e->start = sector_num * BDRV_SECTOR_SIZE;
>  e->length = nb_sectors * BDRV_SECTOR_SIZE;
> -e->flags = ret & ~BDRV_BLOCK_OFFSET_MASK;
> +e->data = !!(ret & BDRV_BLOCK_DATA);
> +e->zero = !!(ret & BDRV_BLOCK_ZERO);
>  e->offset = ret & BDRV_BLOCK_OFFSET_MASK;
> +e->has_offset = !!(ret & BDRV_BLOCK_OFFSET_VALID);

[1]... offset might be garbage if has_offset is false.

>  e->depth = depth;
> -e->bs = bs;
> +if (e->has_offset) {
> +e->has_filename = true;
> +e->filename = bs->filename;
> +}
>  return 0;
>  }

Are we guaranteed that bs->filename is non-NULL when offset is set?  Or
does this need to be if (e->has_offset && bs->filename)?

>  
> @@ -2307,9 +2301,11 @@ static int img_map(int argc, char **argv)
>  goto out;
>  }
>  
> -if (curr.length != 0 && curr.flags == next.flags &&
> +if (curr.length != 0 && curr.zero == next.zero &&
> +curr.data == next.data &&
>  curr.depth == next.depth &&
> -((curr.flags & BDRV_BLOCK_OFFSET_VALID) == 0 ||
> +!strcmp(curr.filename, next.filename) &&

Is this strcmp valid even when !has_filename?

> +(!curr.has_offset ||
>   curr.offset + curr.length == next.offset)) {
>  curr.length += next.length;
>  continue;
> 

-- 
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] qemu_mutex_iothread_locked not correctly synchronized

[David Engraf]

Am 25.11.2015 um 15:36 schrieb Paolo Bonzini:



On 25/11/2015 15:04, David Engraf wrote:




No, you don't.  Who is reading iothread_locked during
qemu_cond_wait_iothread?  No one, because it is a thread-local variable
whose address is never taken.


prepare_mmio_access is reading iothread_locked by using
qemu_mutex_iothread_locked after qemu_tcg_wait_io_event calls
qemu_cond_wait. All one the same thread.


Sure, but who has set iothread_locked to false during the execution of
qemu_cond_wait?  No one, because it's a thread-local variable.  If it's
true before qemu_cond_wait, it will be true after qemu_cond_wait and you
don't need qemu_cond_wait_iothread... unless your compiler is broken and
doesn't generate TLS properly.


Indeed, TLS handling is broken. The address of iothread_locked is always 
the same between threads and I can see that a different thread sets 
iothread_locked to false, thus my current thread uses an invalid state. 
I will have to check why my compiler produces invalid TLS code.


David



Can you compile cpus.c with -S and attach it?

Paolo


qemu_tcg_cpu_thread_fn
-> qemu_tcg_wait_io_event
-> qemu_cond_wait acquires the mutex

qemu_tcg_cpu_thread_fn
-> tcg_exec_all -> tcg_cpu_exec -> cpu_exec
-> cpu_exec ends up in calling prepare_mmio_access

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

[Michael Roth]

Quoting Eric Blake (2015-11-25 10:02:55)
> On 11/25/2015 05:59 AM, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> > 
> > This test exhibits a POSIX behaviour regarding switching between write
> > and read. It's undefined result if the application doesn't ensure a
> > flush between the two operations (with glibc, the flush can be implicit
> > when the buffer size is relatively small). The previous commit fixes
> > this test.
> > 
> > Related to:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1210246
> > 
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  tests/test-qga.c | 95 
> > ++--
> >  1 file changed, 93 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Eric Blake 
> 
> > +/* seek to 0 */
> > +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
> > +  " 'arguments': { 'handle': %" PRId64 ", "
> > +  " 'offset': %d, 'whence': %d } }",
> > +  id, 0, SEEK_SET);
> 
> We still have a conflict between this series and my proposal to codify 0
> rather than relying on platform-specific SEEK_SET; Markus had the
> suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
> both your series and my v2 patch into 2.5?  Knowing that will help me
> decide whether my v2 should be rebased on top of your patches.

I was planning on pulling in your patch on top of this for the next
2.5 pull, so rebasing on top of this series is probably best.

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

Re: [Qemu-devel] [RFC PATCH V2 3/3] Ixgbevf: Add migration support for ixgbevf driver

[Michael S. Tsirkin]

On Thu, Nov 26, 2015 at 12:02:33AM +0800, Lan, Tianyu wrote:
> On 11/25/2015 8:28 PM, Michael S. Tsirkin wrote:
> >Frankly, I don't really see what this short term hack buys us,
> >and if it goes in, we'll have to maintain it forever.
> >
> 
> The framework of how to notify VF about migration status won't be
> changed regardless of stopping VF or not before doing migration.
> We hope to reach agreement on this first.

Well it's bi-directional, the framework won't work if it's
uni-directional.
Further, if you use this interface to stop the interface
at the moment, you won't be able to do anything else
with it, and will need a new one down the road.


> Tracking dirty memory still
> need to more discussions and we will continue working on it. Stop VF may
> help to work around the issue and make tracking easier.
> 
> 
> >Also, assuming you just want to do ifdown/ifup for some reason, it's
> >easy enough to do using a guest agent, in a completely generic way.
> >
> 
> Just ifdown/ifup is not enough for migration. It needs to restore some PCI
> settings before doing ifup on the target machine

I'd focus on just restoring then.

-- 
MST

Re: [Qemu-devel] [PATCH v3 0/2] qga: flush explicitly when needed

[Marc-André Lureau]

Hi

- Original Message -
> Quoting marcandre.lur...@redhat.com (2015-11-25 06:59:10)
> > From: Marc-André Lureau 
> > 
> > Without this change, a write() followed by a read() may lose the
> > previously written content, as shown in the following test.
> > 
> > v2->v3:
> > - use a RwState tristate enum
> > - reset the state on flush & seek
> > 
> > v1->v2:
> > - replace guchar with unsigned char
> > - fix implicitly/explictly
> > - comment space fix
> > 
> > Marc-André Lureau (2):
> >   qga: flush explicitly when needed
> >   tests: add file-write-read test
> 
> Thanks, applied with fix-ups suggested by Lazlo:
> 
>   https://github.com/mdroth/qemu/commits/qga

thanks Michael!

Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack

[Paolo Bonzini]

On 25/11/2015 16:20, Andrey Smetanin wrote:
> +static void synic_clear_sint_msg_pending(struct kvm_vcpu_hv_synic *synic,
> + u32 sint)
> +{
> + struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
> + struct page *page;
> + gpa_t gpa;
> + struct hv_message *msg;
> + struct hv_message_page *msg_page;
> +
> + gpa = synic->msg_page & PAGE_MASK;
> + page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
> + if (is_error_page(page)) {
> + vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
> +  gpa);
> + return;
> + }
> + msg_page = kmap_atomic(page);

But the message page is not being pinned, is it?

Paolo

> + msg = _page->sint_message[sint];
> + msg->header.message_flags.msg_pending = 0;
> +
> + kunmap_atomic(msg_page);
> + kvm_release_page_dirty(page);
> + kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
> +}
> +

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

[Michael Roth]

Quoting Eric Blake (2015-11-25 11:10:46)
> On 11/25/2015 09:46 AM, Michael Roth wrote:
> > Quoting Eric Blake (2015-11-25 10:41:35)
> >> On 11/25/2015 09:21 AM, Michael Roth wrote:
> >>
> > +/* seek to 0 */
> > +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
> > +  " 'arguments': { 'handle': %" PRId64 ", "
> > +  " 'offset': %d, 'whence': %d } }",
> > +  id, 0, SEEK_SET);
> 
>  We still have a conflict between this series and my proposal to codify 0
>  rather than relying on platform-specific SEEK_SET; Markus had the
>  suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
>  both your series and my v2 patch into 2.5?  Knowing that will help me
>  decide whether my v2 should be rebased on top of your patches.
> >>>
> >>> I was planning on pulling in your patch on top of this for the next
> >>> 2.5 pull, so rebasing on top of this series is probably best.
> >>
> >> Okay, will do, and will do quickly so I'm not holding up your pull request.
> > 
> > Thanks! FYI I now have this series applied here if you'd like to base
> > on that:
> > 
> >   https://github.com/mdroth/qemu/commits/qga
> 
> On that branch, commit 7a38932 has two typos:
> s/is commit msg (Lazlo)/in commit msg (Laszlo)/

Thanks, fixed now.

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

Re: [Qemu-devel] [trivial for-2.6] util/id: fully allocate names table

[John Snow]

On 11/25/2015 03:18 AM, Markus Armbruster wrote:
> John Snow  writes:
> 
>> Trivial: this array should be allocated to have ID_MAX entries always.
>> Otherwise if someone were to forget to expand this table, the assertion
>> in the id generator won't actually trigger; it will read junk data.
> 
> You mean this one:
> 
> assert(id < ID_MAX);
> 

Well, sort of. I meant 'assert(id_subsys_str[id])' itself. If you forget
to expand the list (It happened to a friend of mine) this assert will
pass because it reads garbage.

If you just always expand the full table, though, it will catch you
(Err, my friend) being a dummy a little more nicely.

My thought is we need both the range and presence checks.
I'll v2 it, thanks.

--js

> The assertion is crap, because it fails to protect array access
> id_subsys_str[id].  Here's one that does:
> 
> assert(0 <= id && id < ARRAY_SIZE(id_subsys_str));
> 
>> Signed-off-by: John Snow 
>> ---
>>  util/id.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/util/id.c b/util/id.c
>> index bcc64d8..b7ca4d2 100644
>> --- a/util/id.c
>> +++ b/util/id.c
>> @@ -29,7 +29,7 @@ bool id_wellformed(const char *id)
>>  
>>  #define ID_SPECIAL_CHAR '#'
>>  
>> -static const char *const id_subsys_str[] = {
>> +static const char *const id_subsys_str[ID_MAX] = {
>>  [ID_QDEV]  = "qdev",
>>  [ID_BLOCK] = "block",
>>  };

[Qemu-devel] [PATCH v1 2/2] target-i386/kvm: Hyper-V SynIC timers MSR's support

[Andrey Smetanin]

Hyper-V SynIC timers are host timers that are configurable
by guest through corresponding MSR's (HV_X64_MSR_STIMER*).
Guest setup and use fired by host events(SynIC interrupt
and appropriate timer expiration message) as guest clock
events.

The state of Hyper-V SynIC timers are stored in corresponding
MSR's. This patch seria implements such MSR's support and migration.

Signed-off-by: Andrey Smetanin 
CC: Paolo Bonzini 
CC: Richard Henderson 
CC: Eduardo Habkost 
CC: "Andreas Färber" 
CC: Marcelo Tosatti 
CC: Denis V. Lunev 
CC: Roman Kagan 
CC: k...@vger.kernel.org

---
 target-i386/cpu-qom.h |  1 +
 target-i386/cpu.c |  1 +
 target-i386/cpu.h |  2 ++
 target-i386/kvm.c | 50 +-
 target-i386/machine.c | 29 +
 5 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 7ea5b34..5f9d960 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -95,6 +95,7 @@ typedef struct X86CPU {
 bool hyperv_vpindex;
 bool hyperv_runtime;
 bool hyperv_synic;
+bool hyperv_stimer;
 bool check_cpuid;
 bool enforce_cpuid;
 bool expose_kvm;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1462e19..31407f1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3143,6 +3143,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("hv-vpindex", X86CPU, hyperv_vpindex, false),
 DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
 DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
+DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
 DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
 DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
 DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 8cf33df..2376a55 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -923,6 +923,8 @@ typedef struct CPUX86State {
 uint64_t msr_hv_synic_evt_page;
 uint64_t msr_hv_synic_msg_page;
 uint64_t msr_hv_synic_sint[HV_SYNIC_SINT_COUNT];
+uint64_t msr_hv_stimer_config[HV_SYNIC_STIMER_COUNT];
+uint64_t msr_hv_stimer_count[HV_SYNIC_STIMER_COUNT];
 
 /* exception/interrupt handling */
 int error_code;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7513ef6..cb419ea 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -90,6 +90,7 @@ static bool has_msr_hv_reset;
 static bool has_msr_hv_vpindex;
 static bool has_msr_hv_runtime;
 static bool has_msr_hv_synic;
+static bool has_msr_hv_stimer;
 static bool has_msr_mtrr;
 static bool has_msr_xss;
 
@@ -526,7 +527,8 @@ static bool hyperv_enabled(X86CPU *cpu)
 cpu->hyperv_reset ||
 cpu->hyperv_vpindex ||
 cpu->hyperv_runtime ||
-cpu->hyperv_synic);
+cpu->hyperv_synic ||
+cpu->hyperv_stimer);
 }
 
 static Error *invtsc_mig_blocker;
@@ -630,6 +632,13 @@ int kvm_arch_init_vcpu(CPUState *cs)
 env->msr_hv_synic_sint[sint] = HV_SYNIC_SINT_MASKED;
 }
 }
+if (cpu->hyperv_stimer) {
+if (!has_msr_hv_stimer) {
+fprintf(stderr, "Hyper-V timers aren't supported by kernel\n");
+return -ENOSYS;
+}
+c->eax |= HV_X64_MSR_SYNTIMER_AVAILABLE;
+}
 c = _data.entries[cpuid_i++];
 c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
 if (cpu->hyperv_relaxed_timing) {
@@ -974,6 +983,10 @@ static int kvm_get_supported_msrs(KVMState *s)
 has_msr_hv_synic = true;
 continue;
 }
+if (kvm_msr_list->indices[i] == HV_X64_MSR_STIMER0_CONFIG) {
+has_msr_hv_stimer = true;
+continue;
+}
 }
 }
 
@@ -1552,6 +1565,19 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
   env->msr_hv_synic_sint[j]);
 }
 }
+if (has_msr_hv_stimer) {
+int j;
+
+for (j = 0; j < ARRAY_SIZE(env->msr_hv_stimer_config); j++) {
+kvm_msr_entry_set([n++], HV_X64_MSR_STIMER0_CONFIG + j*2,
+env->msr_hv_stimer_config[j]);
+}
+
+for (j = 0; j < ARRAY_SIZE(env->msr_hv_stimer_count); j++) {
+kvm_msr_entry_set([n++], HV_X64_MSR_STIMER0_COUNT + j*2,
+env->msr_hv_stimer_count[j]);
+}
+}
 if (has_msr_mtrr) {
 kvm_msr_entry_set([n++], MSR_MTRRdefType, env->mtrr_deftype);
 kvm_msr_entry_set([n++],
@@ -1931,6 +1957,14 @@ static int kvm_get_msrs(X86CPU *cpu)
 

Re: [Qemu-devel] [RFC PATCH V2 09/10] Qemu/VFIO: Add SRIOV VF migration support

[Lan, Tianyu]

On 11/25/2015 5:03 AM, Michael S. Tsirkin wrote:

>+void vfio_migration_cap_handle(PCIDevice *pdev, uint32_t addr,
>+  uint32_t val, int len)
>+{
>+VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
>+
>+if (addr == vdev->migration_cap + PCI_VF_MIGRATION_VF_STATUS
>+&& val == PCI_VF_READY_FOR_MIGRATION) {
>+qemu_event_set(_event);

This would wake migration so it can proceed -
except it needs QEMU lock to run, and that's
taken by the migration thread.


Sorry, I seem to miss something.
Which lock may cause dead lock when calling vfio_migration_cap_handle()
and run migration?
The function is called when VF accesses faked PCI capability.



It seems unlikely that this ever worked - how
did you test this?

Re: [Qemu-devel] [PATCH v3 1/2] qga: flush explicitly when needed

[Eric Blake]

On 11/25/2015 05:59 AM, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> According to the specification:
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html
> 
> "the application shall ensure that output is not directly followed by
> input without an intervening call to fflush() or to a file positioning
> function (fseek(), fsetpos(), or rewind()), and input is not directly
> followed by output without an intervening call to a file positioning
> function, unless the input operation encounters end-of-file."
> 
> Without this change, a write() followed by a read() may lose the
> previously written content, as shown in the following test.
> 
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1210246
> ---
>  qga/commands-posix.c | 37 +
>  1 file changed, 37 insertions(+)

With Laszlo's suggested commit message improvements,
Reviewed-by: Eric Blake 

> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 0ebd473..cf1d7ec 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -216,9 +216,16 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, 
> Error **errp)
>  }
>  }
>  
> +typedef enum {
> +RW_STATE_NEW,

Bikeshedding: NEW really only sounds right after open(), and doesn't
quite right after a flush; maybe CLEAN or WAITING would be a better name?

-- 
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 v7 for-2.6 00/24] block: Rework bdrv_close_all()

[Kevin Wolf]

Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
> Currently, bdrv_close_all() force-closes all BDSs with a BlockBackend,
> which can lead to data corruption (see the iotest added in the final
> patch of this series) and is most certainly very ugly.
> 
> This series reworks bdrv_close_all() to instead eject the BDS trees from
> all BlockBackends and then close the monitor-owned BDS trees, which are
> the only BDSs without a BB. In effect, all BDSs are closed just by
> getting closed automatically due to their reference count becoming 0.
> 
> Note that the approach taken here leaks all BlockBackends. This does not
> really matter, however, since qemu is about to exit anyway.

Patches 4-11:
Reviewed-by: Kevin Wolf 

Re: [Qemu-devel] [PATCH v2] qemu_mutex_iothread_locked not correctly synchronized

[Paolo Bonzini]

On 25/11/2015 16:48, David Engraf wrote:
> 
> Indeed, TLS handling is broken. The address of iothread_locked is always
> the same between threads and I can see that a different thread sets
> iothread_locked to false, thus my current thread uses an invalid state.
> I will have to check why my compiler produces invalid TLS code.

That rings a bell, I think there are different CRT libraries or
something like that.  Stefan, what would break TLS under Windows?

Paolo

Re: [Qemu-devel] [RFC PATCH V2 3/3] Ixgbevf: Add migration support for ixgbevf driver

[Alexander Duyck]

On Wed, Nov 25, 2015 at 8:02 AM, Lan, Tianyu  wrote:
> On 11/25/2015 8:28 PM, Michael S. Tsirkin wrote:
>>
>> Frankly, I don't really see what this short term hack buys us,
>> and if it goes in, we'll have to maintain it forever.
>>
>
> The framework of how to notify VF about migration status won't be
> changed regardless of stopping VF or not before doing migration.
> We hope to reach agreement on this first. Tracking dirty memory still
> need to more discussions and we will continue working on it. Stop VF may
> help to work around the issue and make tracking easier.

The problem is you still have to stop the device at some point for the
same reason why you have to halt the VM.  You seem to think you can
get by without doing that but you can't.  All you do is open the
system up to multiple races if you leave the device running.  The goal
should be to avoid stopping the device until the last possible moment,
however it will still have to be stopped eventually.  It isn't as if
you can migrate memory and leave the device doing DMA and expect to
get a clean state.

I agree with Michael.  The focus needs to be on first addressing dirty
page tracking.  Once you have that you could use a variation on the
bonding solution where you postpone the hot-plug event until near the
end of the migration just before you halt the guest instead of having
to do it before you start the migration.  Then after that we could
look at optimizing things further by introducing a variation that you
could further improve on things by introducing a variation of hot-plug
that would pause the device as I suggested instead of removing it.  At
that point you should be able to have almost all of the key issues
addresses so that you could drop the bond interface entirely.

>> Also, assuming you just want to do ifdown/ifup for some reason, it's
>> easy enough to do using a guest agent, in a completely generic way.
>>
>
> Just ifdown/ifup is not enough for migration. It needs to restore some PCI
> settings before doing ifup on the target machine

That is why I have been suggesting making use of suspend/resume logic
that is already in place for PCI power management.  In the case of a
suspend/resume we already have to deal with the fact that the device
will go through a D0->D3->D0 reset so we have to restore all of the
existing state.  It would take a significant load off of Qemu since
the guest would be restoring its own state instead of making Qemu have
to do all of the device migration work.

Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack

[Paolo Bonzini]

On 25/11/2015 17:55, Andrey Smetanin wrote:
>>
>> +gpa = synic->msg_page & PAGE_MASK;
>> +page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
>> +if (is_error_page(page)) {
>> +vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
>> + gpa);
>> +return;
>> +}
>> +msg_page = kmap_atomic(page);
>
> But the message page is not being pinned, is it?
>
> Actually I don't know anything about pinning.
> Is it pinning against page swapping ?

Yes.  Unless the page is pinned, kmap_atomic can fail.

However, I don't think that kvm_hv_notify_acked_sint is called from
atomic context.  It is only called from apic_set_eoi.  Could you just
use kvm_vcpu_write_guest_page?

By the way, do you need to do this also in kvm_get_apic_interrupt, for
auto EOI interrupts?

Thanks,

Paolo

> Could you please clarify and provide an API to use in this case ?

Re: [Qemu-devel] [PATCH 1/3] target-i386: kvm: Abort if MCE bank count is not supported by host

[Eduardo Habkost]

On Wed, Nov 25, 2015 at 05:46:38PM +0100, Paolo Bonzini wrote:
> 
> 
> On 25/11/2015 16:49, Eduardo Habkost wrote:
> > Instead of silently changing the number of banks in mcg_cap based
> > on kvm_get_mce_cap_supported(), abort initialization if the host
> > doesn't support MCE_BANKS_DEF banks.
> > 
> > Note that MCE_BANKS_DEF was always 10 since it was introduced in
> > QEMU, and Linux always returned 32 at KVM_CAP_MCE since
> > KVM_CAP_MCE was introduced, so no behavior is being changed and
> > the error can't be triggered by any Linux version. The point of
> > the new check is to ensure we won't silently change the bank
> > count if we change MCE_BANKS_DEF or make the bank count
> > configurable in the future.
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  target-i386/kvm.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 2a9953b..ee7bc69 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -784,11 +784,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >  return ret;
> >  }
> >  
> > -if (banks > MCE_BANKS_DEF) {
> > -banks = MCE_BANKS_DEF;
> > +if (MCE_BANKS_DEF > banks) {
> > +error_report("kvm: Unsupported MCE bank count: %d > %d\n",
> > + MCE_BANKS_DEF, banks);
> 
> Yoda conditions?
> 
> if (banks < MCE_BANKS_DEF) {
> error_report("kvm: Unsupported MCE bank count (QEMU = %d, KVM = 
> %d)",
>  MCE_BANKS_DEF, banks);

This was on purpose, because MCE_BANKS_DEF is replaced by
(env->mcg_caps & MCG_CAPS_COUNT_MASK) in the next patch.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

[Michael Roth]

Quoting Eric Blake (2015-11-25 11:18:24)
> On 11/25/2015 10:14 AM, Michael Roth wrote:
> >>> Thanks! FYI I now have this series applied here if you'd like to base
> >>> on that:
> >>>
> >>>   https://github.com/mdroth/qemu/commits/qga
> >>
> >> On that branch, commit 7a38932 has two typos:
> >> s/is commit msg (Lazlo)/in commit msg (Laszlo)/
> > 
> > Thanks, fixed now.
> 
> Except that in e8c9ea9, the message still references write()/read()
> instead of the intended fwrite()/fread().  We'll get it eventually :)

Argh! Sorry, fixed now.

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

[Qemu-devel] [PATCH v1 1/2] include: update Hyper-V header to include SynIC timers defines

[Andrey Smetanin]

This patch brings in the necessary changes from the corresponding kernel
patchset.  It's included only for completeness; ideally these changes
should arrive via the standard kernel header pull.

Signed-off-by: Andrey Smetanin 
CC: Paolo Bonzini 
CC: Richard Henderson 
CC: Eduardo Habkost 
CC: "Andreas Färber" 
CC: Marcelo Tosatti 
CC: Denis V. Lunev 
CC: Roman Kagan 
CC: k...@vger.kernel.org

---
 include/standard-headers/asm-x86/hyperv.h | 99 +++
 1 file changed, 99 insertions(+)

diff --git a/include/standard-headers/asm-x86/hyperv.h 
b/include/standard-headers/asm-x86/hyperv.h
index f9780f1..3684610 100644
--- a/include/standard-headers/asm-x86/hyperv.h
+++ b/include/standard-headers/asm-x86/hyperv.h
@@ -269,4 +269,103 @@ typedef struct _HV_REFERENCE_TSC_PAGE {
 #define HV_SYNIC_SINT_AUTO_EOI (1ULL << 17)
 #define HV_SYNIC_SINT_VECTOR_MASK  (0xFF)
 
+#define HV_SYNIC_STIMER_COUNT  (4)
+
+/* Define synthetic interrupt controller message constants. */
+#define HV_MESSAGE_SIZE(256)
+#define HV_MESSAGE_PAYLOAD_BYTE_COUNT  (240)
+#define HV_MESSAGE_PAYLOAD_QWORD_COUNT (30)
+
+/* Define hypervisor message types. */
+enum hv_message_type {
+   HVMSG_NONE  = 0x,
+
+   /* Memory access messages. */
+   HVMSG_UNMAPPED_GPA  = 0x8000,
+   HVMSG_GPA_INTERCEPT = 0x8001,
+
+   /* Timer notification messages. */
+   HVMSG_TIMER_EXPIRED = 0x8010,
+
+   /* Error messages. */
+   HVMSG_INVALID_VP_REGISTER_VALUE = 0x8020,
+   HVMSG_UNRECOVERABLE_EXCEPTION   = 0x8021,
+   HVMSG_UNSUPPORTED_FEATURE   = 0x8022,
+
+   /* Trace buffer complete messages. */
+   HVMSG_EVENTLOG_BUFFERCOMPLETE   = 0x8040,
+
+   /* Platform-specific processor intercept messages. */
+   HVMSG_X64_IOPORT_INTERCEPT  = 0x8001,
+   HVMSG_X64_MSR_INTERCEPT = 0x80010001,
+   HVMSG_X64_CPUID_INTERCEPT   = 0x80010002,
+   HVMSG_X64_EXCEPTION_INTERCEPT   = 0x80010003,
+   HVMSG_X64_APIC_EOI  = 0x80010004,
+   HVMSG_X64_LEGACY_FP_ERROR   = 0x80010005
+};
+
+/* Define synthetic interrupt controller message flags. */
+union hv_message_flags {
+   uint8_t asu8;
+   struct {
+   uint8_t msg_pending:1;
+   uint8_t reserved:7;
+   };
+};
+
+/* Define port identifier type. */
+union hv_port_id {
+   uint32_t asu32;
+   struct {
+   uint32_t id:24;
+   uint32_t reserved:8;
+   } u;
+};
+
+/* Define port type. */
+enum hv_port_type {
+   HVPORT_MSG  = 1,
+   HVPORT_EVENT= 2,
+   HVPORT_MONITOR  = 3
+};
+
+/* Define synthetic interrupt controller message header. */
+struct hv_message_header {
+   enum hv_message_type message_type;
+   uint8_t payload_size;
+   union hv_message_flags message_flags;
+   uint8_t reserved[2];
+   union {
+   uint64_t sender;
+   union hv_port_id port;
+   };
+};
+
+/* Define timer message payload structure. */
+struct hv_timer_message_payload {
+   uint32_t timer_index;
+   uint32_t reserved;
+   uint64_t expiration_time;   /* When the timer expired */
+   uint64_t delivery_time; /* When the message was delivered */
+};
+
+/* Define synthetic interrupt controller message format. */
+struct hv_message {
+   struct hv_message_header header;
+   union {
+   uint64_t payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
+   } u;
+};
+
+/* Define the synthetic interrupt message page layout. */
+struct hv_message_page {
+   struct hv_message sint_message[HV_SYNIC_SINT_COUNT];
+};
+
+#define HV_STIMER_ENABLE   (1ULL << 0)
+#define HV_STIMER_PERIODIC (1ULL << 1)
+#define HV_STIMER_LAZY (1ULL << 2)
+#define HV_STIMER_AUTOENABLE   (1ULL << 3)
+#define HV_STIMER_SINT(config) (uint8_t)(((config) >> 16) & 0x0F)
+
 #endif
-- 
2.4.3

[Qemu-devel] [PATCH v1 3/7] kvm/x86: Rearrange func's declarations inside Hyper-V header

[Andrey Smetanin]

This rearrangement places functions declarations together
according to their functionality, so future additions
will be simplier.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.h | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 315af4b..9483d49 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -24,14 +24,6 @@
 #ifndef __ARCH_X86_KVM_HYPERV_H__
 #define __ARCH_X86_KVM_HYPERV_H__
 
-int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
-int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
-bool kvm_hv_hypercall_enabled(struct kvm *kvm);
-int kvm_hv_hypercall(struct kvm_vcpu *vcpu);
-
-int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vcpu_id, u32 sint);
-void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int vector);
-
 static inline struct kvm_vcpu_hv_synic *vcpu_to_synic(struct kvm_vcpu *vcpu)
 {
return >arch.hyperv.synic;
@@ -46,10 +38,18 @@ static inline struct kvm_vcpu *synic_to_vcpu(struct 
kvm_vcpu_hv_synic *synic)
arch = container_of(hv, struct kvm_vcpu_arch, hyperv);
return container_of(arch, struct kvm_vcpu, arch);
 }
-void kvm_hv_irq_routing_update(struct kvm *kvm);
 
-void kvm_hv_vcpu_init(struct kvm_vcpu *vcpu);
+int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
+int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
+
+bool kvm_hv_hypercall_enabled(struct kvm *kvm);
+int kvm_hv_hypercall(struct kvm_vcpu *vcpu);
 
+void kvm_hv_irq_routing_update(struct kvm *kvm);
+int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vcpu_id, u32 sint);
+void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int vector);
 int kvm_hv_activate_synic(struct kvm_vcpu *vcpu);
 
+void kvm_hv_vcpu_init(struct kvm_vcpu *vcpu);
+
 #endif
-- 
2.4.3

Re: [Qemu-devel] [PATCH] Revert "vhost: send SET_VRING_ENABLE at start/stop"

[Thibaut Collet]

On Wed, Nov 25, 2015 at 1:42 PM, Michael S. Tsirkin  wrote:
> This reverts commit 3a12f32229a046f4d4ab0a3a52fb01d2d5a1ab76.
>
> In case of live migration several queues can be enabled and not only the
> first one. So informing backend that only the first queue is enabled is
> wrong.
>
> Reported-by: Thibaut Collet 
> Cc: Yuanhan Liu 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  hw/virtio/vhost.c | 9 -
>  1 file changed, 9 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 1794f0d..de29968 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1226,11 +1226,6 @@ int vhost_dev_start(struct vhost_dev *hdev, 
> VirtIODevice *vdev)
>  }
>  }
>
> -if (hdev->vhost_ops->vhost_set_vring_enable) {
> -/* only enable first vq pair by default */
> -hdev->vhost_ops->vhost_set_vring_enable(hdev, hdev->vq_index == 0);
> -}
> -
>  return 0;
>  fail_log:
>  vhost_log_put(hdev, false);
> @@ -1261,10 +1256,6 @@ void vhost_dev_stop(struct vhost_dev *hdev, 
> VirtIODevice *vdev)
>   hdev->vq_index + i);
>  }
>
> -if (hdev->vhost_ops->vhost_set_vring_enable) {
> -hdev->vhost_ops->vhost_set_vring_enable(hdev, 0);
> -}
> -
>  vhost_log_put(hdev, true);
>  hdev->started = false;
>  hdev->log = NULL;
> --

Ack

> MST

[Qemu-devel] [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Eduardo Habkost]

Instead of silently clearing mcg_cap bits when the host doesn't
support them, print a warning when doing that.

Signed-off-by: Eduardo Habkost 
---
 target-i386/kvm.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index d63a85b..446bdfc 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -774,7 +774,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 && (env->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
(CPUID_MCE | CPUID_MCA)
 && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
-uint64_t mcg_cap;
+uint64_t mcg_cap, unsupported_caps;
 int banks;
 int ret;
 
@@ -790,6 +790,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
 return -ENOTSUP;
 }
 
+unsupported_caps = env->mcg_cap & ~(mcg_cap | MCG_CAP_BANKS_MASK);
+if (unsupported_caps) {
+error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64 "\n",
+ unsupported_caps);
+}
+
 env->mcg_cap &= mcg_cap | MCG_CAP_BANKS_MASK;
 ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, >mcg_cap);
 if (ret < 0) {
-- 
2.1.0

Re: [Qemu-devel] [PATCH 1/3] target-i386: kvm: Abort if MCE bank count is not supported by host

[Paolo Bonzini]

On 25/11/2015 16:49, Eduardo Habkost wrote:
> Instead of silently changing the number of banks in mcg_cap based
> on kvm_get_mce_cap_supported(), abort initialization if the host
> doesn't support MCE_BANKS_DEF banks.
> 
> Note that MCE_BANKS_DEF was always 10 since it was introduced in
> QEMU, and Linux always returned 32 at KVM_CAP_MCE since
> KVM_CAP_MCE was introduced, so no behavior is being changed and
> the error can't be triggered by any Linux version. The point of
> the new check is to ensure we won't silently change the bank
> count if we change MCE_BANKS_DEF or make the bank count
> configurable in the future.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  target-i386/kvm.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 2a9953b..ee7bc69 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -784,11 +784,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  return ret;
>  }
>  
> -if (banks > MCE_BANKS_DEF) {
> -banks = MCE_BANKS_DEF;
> +if (MCE_BANKS_DEF > banks) {
> +error_report("kvm: Unsupported MCE bank count: %d > %d\n",
> + MCE_BANKS_DEF, banks);

Yoda conditions?

if (banks < MCE_BANKS_DEF) {
error_report("kvm: Unsupported MCE bank count (QEMU = %d, KVM = 
%d)",
 MCE_BANKS_DEF, banks);

Paolo

> +return -ENOTSUP;
>  }
> +
>  mcg_cap &= MCE_CAP_DEF;
> -mcg_cap |= banks;
> +mcg_cap |= MCE_BANKS_DEF;
>  ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, _cap);
>  if (ret < 0) {
>  fprintf(stderr, "KVM_X86_SETUP_MCE: %s", strerror(-ret));
> 

Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack

[Andrey Smetanin]

On 11/25/2015 07:52 PM, Paolo Bonzini wrote:



On 25/11/2015 16:20, Andrey Smetanin wrote:

+static void synic_clear_sint_msg_pending(struct kvm_vcpu_hv_synic *synic,
+   u32 sint)
+{
+   struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
+   struct page *page;
+   gpa_t gpa;
+   struct hv_message *msg;
+   struct hv_message_page *msg_page;
+
+   gpa = synic->msg_page & PAGE_MASK;
+   page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
+   if (is_error_page(page)) {
+   vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
+gpa);
+   return;
+   }
+   msg_page = kmap_atomic(page);


But the message page is not being pinned, is it?


Actually I don't know anything about pinning.
Is it pinning against page swapping ?
Could you please clarify and provide an API to use in this case ?

Paolo


+   msg = _page->sint_message[sint];
+   msg->header.message_flags.msg_pending = 0;
+
+   kunmap_atomic(msg_page);
+   kvm_release_page_dirty(page);
+   kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
+}
+

[Qemu-devel] [PULL 2/9] QEMU does not care about left shifts of signed negative values

[Paolo Bonzini]

It seems like there's no good reason for the compiler to exploit the
undefinedness of left shifts.  GCC explicitly documents that they do not
use at all this possibility and, while they also say this is subject
to change, they have been saying this for 10 years (since the wording
appeared in the GCC 4.0 manual).

Any workaround for this particular case of undefined behavior uglifies the
code.  Using unsigned is unsafe (https://github.com/madler/zlib/pull/112
is the proof) because the value becomes positive when extended; -(a << b)
works but does not express that the intention is to compute -a * 2^N,
especially if "a" is a constant.


The straw that broke the camel back is Clang's latest obnoxious,
pointless, unsafe---and did I mention *totally* useless---warning about
left shifting a negative integer.  It's obnoxious and pointless because
the compiler is not using the latitude that the standard gives it, so
the warning just adds noise.  It is useless and unsafe because it does
not catch the widely more common case where the LHS is a variable, and
thus gives a false sense of security.  The noisy nature of the warning
means that it should have never been added to -Wall.  The uselessness
means that it probably should not have even been added to -Wextra.

(It would have made sense to enable the warning for -fsanitize=shift,
as the program would always crash if the point is reached.  But this was
probably too sophisticated to come up with, when you're so busy giving
birth to gems such as -Wabsolute-value).


Ubsan also has warnings for undefined behavior of left shifts.  Checks for
left shift overflow and left shift of negative numbers, unfortunately,
cannot be silenced without also silencing the useful ones about out-of-range
shift amounts. -fwrapv ought to shut them up, but doesn't yet
(https://llvm.org/bugs/show_bug.cgi?id=25552; I am taking care of fixing
the same issues in GCC).  Luckily ubsan is optional, and the easy
workaround is to use -fsanitize-recover.

Anyhow, this patch documents our assumptions explicitly, and shuts up the
stupid warning.  -fwrapv is a bit of a heavy hammer, but it is the safest
option and it ought to just work long term as the compilers improve.
Note that -fstrict-overflow does not silence ubsan's overflow warnings,
hence it's reasonable to assume that it won't silence the left shift
warnings either.  QEMU doesn't rely on pointer overflow anyway, and
that's the other major difference between -fwrapv (which only cares
about integer overflow) and -fstrict-overflow.

Thanks to everyone involved in the discussion!

Cc: Peter Maydell 
Reviewed-by: Markus Armbruster 
Grudgingly-reviewed-by: Laszlo Ersek 
Signed-off-by: Paolo Bonzini 
---
 HACKING   | 6 ++
 configure | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/HACKING b/HACKING
index 12fbc8a..71ad23b 100644
--- a/HACKING
+++ b/HACKING
@@ -157,3 +157,9 @@ painful. These are:
  * you may assume that integers are 2s complement representation
  * you may assume that right shift of a signed integer duplicates
the sign bit (ie it is an arithmetic shift, not a logical shift)
+
+In addition, QEMU assumes that the compiler does not use the latitude
+given in C99 and C11 to treat aspects of signed '<<' as undefined, as
+documented in the GNU Compiler Collection manual starting at version 4.0.
+If a compiler does not respect this when passed the -fwrapv option,
+it is not supported for compilation of QEMU.
diff --git a/configure b/configure
index 71d6cbc..5bb8187 100755
--- a/configure
+++ b/configure
@@ -413,7 +413,7 @@ sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
 ARFLAGS="${ARFLAGS-rv}"
 
 # default flags for all hosts
-QEMU_CFLAGS="-fno-strict-aliasing -fno-common $QEMU_CFLAGS"
+QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
$QEMU_CFLAGS"
@@ -1461,7 +1461,7 @@ fi
 gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
 gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers 
$gcc_flags"
 gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
-gcc_flags="-Wendif-labels $gcc_flags"
+gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags"
 gcc_flags="-Wno-initializer-overrides $gcc_flags"
 gcc_flags="-Wno-string-plus-int $gcc_flags"
 # Note that we do not add -Werror to gcc_flags here, because that would
-- 
1.8.3.1

[Qemu-devel] [PULL 0/9] Misc patches for QEMU 2.5-rc2 (2015-11-25)

[Paolo Bonzini]

The following changes since commit 4b6eda626fdb8bf90472c6868d502a2ac09abeeb:

  Merge remote-tracking branch 'remotes/lalrae/tags/mips-20151124' into staging 
(2015-11-24 17:05:06 +)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 6ac504d20a0967da06caadf595388f753908328a:

  target-i386: kvm: Print warning when clearing mcg_cap bits (2015-11-25 
17:48:46 +0100)


Small patches, most notably the introduction of -fwrapv.


Daniel P. Berrange (2):
  Revert "exec: silence hugetlbfs warning under qtest"
  exec: remove warning about mempath and hugetlbfs

Eduardo Habkost (3):
  target-i386: kvm: Abort if MCE bank count is not supported by host
  target-i386: kvm: Use env->mcg_cap when setting up MCE
  target-i386: kvm: Print warning when clearing mcg_cap bits

Paolo Bonzini (3):
  MAINTAINERS: Update TCG CPU cores section
  QEMU does not care about left shifts of signed negative values
  target-sparc: fix 32-bit truncation in fpackfix

Wen Congyang (1):
  call bdrv_drain_all() even if the vm is stopped

 HACKING   |  6 ++
 MAINTAINERS   | 17 +
 configure |  4 ++--
 cpus.c|  2 ++
 exec.c|  6 --
 target-i386/cpu.h |  2 ++
 target-i386/kvm.c | 22 ++
 target-sparc/vis_helper.c |  2 +-
 vl.c  | 28 ++--
 9 files changed, 54 insertions(+), 35 deletions(-)
-- 
1.8.3.1

[Qemu-devel] [PATCH 1/3] target-i386: kvm: Abort if MCE bank count is not supported by host

[Eduardo Habkost]

Instead of silently changing the number of banks in mcg_cap based
on kvm_get_mce_cap_supported(), abort initialization if the host
doesn't support MCE_BANKS_DEF banks.

Note that MCE_BANKS_DEF was always 10 since it was introduced in
QEMU, and Linux always returned 32 at KVM_CAP_MCE since
KVM_CAP_MCE was introduced, so no behavior is being changed and
the error can't be triggered by any Linux version. The point of
the new check is to ensure we won't silently change the bank
count if we change MCE_BANKS_DEF or make the bank count
configurable in the future.

Signed-off-by: Eduardo Habkost 
---
 target-i386/kvm.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 2a9953b..ee7bc69 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -784,11 +784,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
 return ret;
 }
 
-if (banks > MCE_BANKS_DEF) {
-banks = MCE_BANKS_DEF;
+if (MCE_BANKS_DEF > banks) {
+error_report("kvm: Unsupported MCE bank count: %d > %d\n",
+ MCE_BANKS_DEF, banks);
+return -ENOTSUP;
 }
+
 mcg_cap &= MCE_CAP_DEF;
-mcg_cap |= banks;
+mcg_cap |= MCE_BANKS_DEF;
 ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, _cap);
 if (ret < 0) {
 fprintf(stderr, "KVM_X86_SETUP_MCE: %s", strerror(-ret));
-- 
2.1.0

[Qemu-devel] [PATCH 2/3] target-i386: kvm: Use env->mcg_cap when setting up MCE

[Eduardo Habkost]

When setting up MCE, instead of using the MCE_*_DEF macros
directly, just filter the existing env->mcg_cap value.

As env->mcg_cap is already initialized as
MCE_CAP_DEF|MCE_BANKS_DEF at target-i386/cpu.c:mce_init(), this
doesn't change any behavior. But it will allow us to change
mce_init() in the future, to implement different defaults
depending on CPU model, machine-type or command-line parameters.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.h |  2 ++
 target-i386/kvm.c | 11 ---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index fc4a605..84edfd0 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -286,6 +286,8 @@
 #define MCE_CAP_DEF (MCG_CTL_P|MCG_SER_P)
 #define MCE_BANKS_DEF   10
 
+#define MCG_CAP_BANKS_MASK 0xff
+
 #define MCG_STATUS_RIPV (1ULL<<0)   /* restart ip valid */
 #define MCG_STATUS_EIPV (1ULL<<1)   /* ip points to correct instruction */
 #define MCG_STATUS_MCIP (1ULL<<2)   /* machine check in progress */
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index ee7bc69..d63a85b 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -784,21 +784,18 @@ int kvm_arch_init_vcpu(CPUState *cs)
 return ret;
 }
 
-if (MCE_BANKS_DEF > banks) {
+if ((env->mcg_cap & MCG_CAP_BANKS_MASK) > banks) {
 error_report("kvm: Unsupported MCE bank count: %d > %d\n",
- MCE_BANKS_DEF, banks);
+ (int)(env->mcg_cap & MCG_CAP_BANKS_MASK), banks);
 return -ENOTSUP;
 }
 
-mcg_cap &= MCE_CAP_DEF;
-mcg_cap |= MCE_BANKS_DEF;
-ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, _cap);
+env->mcg_cap &= mcg_cap | MCG_CAP_BANKS_MASK;
+ret = kvm_vcpu_ioctl(cs, KVM_X86_SETUP_MCE, >mcg_cap);
 if (ret < 0) {
 fprintf(stderr, "KVM_X86_SETUP_MCE: %s", strerror(-ret));
 return ret;
 }
-
-env->mcg_cap = mcg_cap;
 }
 
 qemu_add_vm_change_state_handler(cpu_update_state, env);
-- 
2.1.0

[Qemu-devel] [PATCH 0/3] target-i386: kvm: Use env->mcg_cap when setting up MCE

[Eduardo Habkost]

Instead of overwriting env->mcg_cap, make kvm_arch_init_vcpu(),
use the value already set at the CPU object when initializing
MCE.

Except for the new "unsupported MCG_CAPS bits" warning, this
patch doesn't change any of the existing QEMU behavior. The
previous code set env->mcg_cap to:
  (MCE_CAP_DEF & ioctl(KVM_X86_GET_MCE_CAP_SUPPORTED)) | MCE_BANKS_DEF
and the new code still keeps it exactly the same, as env->mcg_cap
is already initialized as MCE_CAP_DEF|MCE_BANKS_DEF at
mce_init().

This will allow us to change mce_init() in the future, to
implement different defaults depending on CPU model, machine-type
or command-line parameters.

Eduardo Habkost (3):
  target-i386: kvm: Abort if MCE bank count is not supported by host
  target-i386: kvm: Use env->mcg_cap when setting up MCE
  target-i386: kvm: Print warning when clearing mcg_cap bits

 target-i386/cpu.h |  2 ++
 target-i386/kvm.c | 22 ++
 2 files changed, 16 insertions(+), 8 deletions(-)

-- 
2.1.0

Re: [Qemu-devel] [PATCH v7 12/24] virtio-blk: Functions for op blocker management

[Kevin Wolf]

Am 25.11.2015 um 17:03 hat Max Reitz geschrieben:
> On 25.11.2015 16:57, Kevin Wolf wrote:
> > Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
> >> Put the code for setting up and removing op blockers into an own
> >> function, respectively. Then, we can invoke those functions whenever a
> >> BDS is removed from an virtio-blk BB or inserted into it.
> >>
> >> Signed-off-by: Max Reitz 
> > 
> > Do you know of a case where this is observable?
> 
> Actually, no.
> 
> > I don't think you can
> > change the medium of a virtio-blk device, and blk_set_bs() isn't
> > converted to a wrapper around blk_remove/insert_bs() yet. If this patch
> > is necessary to fix something observable, the latter is probably a bug.
> 
> So I guess that implies "Otherwise, this patch should be dropped"?

I'm not sure. I guess you had a reason to include these patches other
than putting the notifiers to use?

With blk_set_bs() changed, I think it would have an effect: The op
blockers would move from the old BDS to the new top-level one. This
sounds desirable to me.

> >> diff --git a/hw/block/dataplane/virtio-blk.c 
> >> b/hw/block/dataplane/virtio-blk.c
> >> index c42ddeb..4c95d5b 100644
> >> --- a/hw/block/dataplane/virtio-blk.c
> >> +++ b/hw/block/dataplane/virtio-blk.c
> >> @@ -39,6 +39,8 @@ struct VirtIOBlockDataPlane {
> >>  EventNotifier *guest_notifier;  /* irq */
> >>  QEMUBH *bh; /* bh for guest notification */
> >>  
> >> +Notifier insert_notifier, remove_notifier;
> >> +
> >>  /* Note that these EventNotifiers are assigned by value.  This is
> >>   * fine as long as you do not call event_notifier_cleanup on them
> >>   * (because you don't own the file descriptor or handle; you just
> >> @@ -137,6 +139,54 @@ static void handle_notify(EventNotifier *e)
> >>  blk_io_unplug(s->conf->conf.blk);
> >>  }
> >>  
> >> +static void data_plane_set_up_op_blockers(VirtIOBlockDataPlane *s)
> >> +{
> >> +assert(!s->blocker);
> >> +error_setg(>blocker, "block device is in use by data plane");
> >> +blk_op_block_all(s->conf->conf.blk, s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_RESIZE, s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_DRIVE_DEL, 
> >> s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_BACKUP_SOURCE, 
> >> s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_CHANGE, s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_SOURCE, 
> >> s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_TARGET, 
> >> s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EJECT, s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
> >> +   s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT,
> >> +   s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, 
> >> BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
> >> +   s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_MIRROR, s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker);
> >> +blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker);
> >> +}
> > 
> > This makes me wonder: What do we even block here any more? If I didn't
> > miss anything, it's only BLOCK_OP_TYPE_BACKUP_TARGET, and I'm not sure
> > why this needs to be blocked, or if we simply forgot to enable it.
> 
> Well, even though in practice this wall of code doesn't make much sense,
> of course it will be safe for potential additions of new op blockers.
> 
> And of course we actually don't want these blockers at all anymore...

Yes, but dataplane shouldn't really be special enough any more that we
want to disable features for it initially. By now it sounds more like an
easy way to forget unblocking a new feature even though it would work.

So perhaps we should really just remove the blockers from dataplane.
Then we don't have to answer the question above...

Kevin


pgpmUap73ysl6.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 06/14] parallels: Assign bs->file->bs to file in parallels_co_get_block_status

[Eric Blake]

On 11/25/2015 12:39 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  block/parallels.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Eric Blake 

> 
> diff --git a/block/parallels.c b/block/parallels.c
> index d1146f1..6552f32 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -273,6 +273,7 @@ static int64_t coroutine_fn 
> parallels_co_get_block_status(BlockDriverState *bs,
>  return 0;
>  }
>  
> +*file = bs->file->bs;
>  return (offset << BDRV_SECTOR_BITS) |
>  BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID;
>  }
> 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH 0/2] import nvme fix

[Keith Busch]

On Wed, Nov 25, 2015 at 10:28:42AM +0100, Kevin Wolf wrote:
> Am 18.11.2015 um 19:53 hat Christoph Hellwig geschrieben:
> > First one fixes Identify to behave as mandated by the spec, and the
> > second bumps the PCI revision so that guest drivers can detect
> > the fixed version of the device so that only the old version has
> > to be blacklisted.
> 
> Keith, this looks to me like a fix that should still be merged for 2.5,
> would you agree? Can you please have a look at the series and either
> give your Acked-by or comment?

The series looks good to me. I had some difficulty finding the right
patches in the midst of Christoph's Linux patch bombs. :)

Acked-by: Keith Busch 

[Qemu-devel] [PULL 6/9] target-sparc: fix 32-bit truncation in fpackfix

[Paolo Bonzini]

This is reported by Coverity.  The algorithm description at
ftp://ftp.icm.edu.pl/packages/ggi/doc/hw/sparc/Sparc.pdf suggests
that the 32-bit parts of rs2, after the left shift, is treated
as a 64-bit integer.  Bits 32 and above are used to do the
saturating truncation.

Message-Id: <1446473134-4330-1-git-send-email-pbonz...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 target-sparc/vis_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-sparc/vis_helper.c b/target-sparc/vis_helper.c
index 383cc8b..45fc7db 100644
--- a/target-sparc/vis_helper.c
+++ b/target-sparc/vis_helper.c
@@ -447,7 +447,7 @@ uint32_t helper_fpackfix(uint64_t gsr, uint64_t rs2)
 for (word = 0; word < 2; word++) {
 uint32_t val;
 int32_t src = rs2 >> (word * 32);
-int64_t scaled = src << scale;
+int64_t scaled = (int64_t)src << scale;
 int64_t from_fixed = scaled >> 16;
 
 val = (from_fixed < -32768 ? -32768 :
-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH V2 3/3] Ixgbevf: Add migration support for ixgbevf driver

[Alexander Duyck]

On Wed, Nov 25, 2015 at 8:39 AM, Michael S. Tsirkin  wrote:
> On Wed, Nov 25, 2015 at 08:24:38AM -0800, Alexander Duyck wrote:
>> >> Also, assuming you just want to do ifdown/ifup for some reason, it's
>> >> easy enough to do using a guest agent, in a completely generic way.
>> >>
>> >
>> > Just ifdown/ifup is not enough for migration. It needs to restore some PCI
>> > settings before doing ifup on the target machine
>>
>> That is why I have been suggesting making use of suspend/resume logic
>> that is already in place for PCI power management.  In the case of a
>> suspend/resume we already have to deal with the fact that the device
>> will go through a D0->D3->D0 reset so we have to restore all of the
>> existing state.  It would take a significant load off of Qemu since
>> the guest would be restoring its own state instead of making Qemu have
>> to do all of the device migration work.
>
> That can work, though again, the issue is you need guest
> cooperation to migrate.

Right now the problem is you need to have guest cooperation anyway as
you need to have some way of tracking the dirty pages.  If the IOMMU
on the host were to provide some sort of dirty page tracking then we
could exclude the guest from the equation, but until then we need the
guest to notify us of what pages it is letting the device dirty.  I'm
still of the opinion that the best way to go there is to just modify
the DMA API that is used in the guest so that it supports some sort of
page flag modification or something along those lines so we can track
all of the pages that might be written to by the device.

> If you reset device on destination instead of restoring state,
> then that issue goes away, but maybe the downtime
> will be increased.

Yes, the downtime will be increased, but it shouldn't be by much.
Depending on the setup a VF with a single queue can have about 3MB of
data outstanding when you move the driver over.  After that it is just
a matter of bringing the interface back up which should take only a
few hundred milliseconds assuming the PF is fairly responsive.

> Will it really? I think it's worth it to start with the
> simplest solution (reset on destination) and see
> what the effect is, then add optimizations.

Agreed.  My thought would be to start with something like
dma_mark_clean() that could be used to take care of marking the pages
for migration when they are unmapped or synced.

> One thing that I've been thinking about for a while, is saving (some)
> state speculatively.  For example, notify guest a bit before migration
> is done, so it can save device state. If guest responds quickly, you
> have state that can be restored.  If it doesn't, still migrate, and it
> will have to reset on destination.

I'm not sure how much more device state we really need to save.  The
driver in the guest has to have enough state to recover in the event
of a device failure resulting in a slot reset.  To top it off the
driver is able to reconfigure things probably as quick as we could if
we were restoring the state.

[Qemu-devel] [PULL 1/9] MAINTAINERS: Update TCG CPU cores section

[Paolo Bonzini]

These are the people that I think have been touching it lately
or reviewing patches.

Signed-off-by: Paolo Bonzini 
---
 MAINTAINERS | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 28f0139..bb1f3e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -62,14 +62,22 @@ Guest CPU cores (TCG):
 --
 Overall
 L: qemu-devel@nongnu.org
-S: Odd fixes
+M: Paolo Bonzini 
+M: Peter Crosthwaite 
+M: Richard Henderson 
+S: Maintained
 F: cpu-exec.c
+F: cpu-exec-common.c
+F: cpus.c
 F: cputlb.c
+F: exec.c
 F: softmmu_template.h
-F: translate-all.c
-F: include/exec/cpu_ldst.h
-F: include/exec/cpu_ldst_template.h
+F: translate-all.*
+F: translate-common.c
+F: include/exec/cpu*.h
+F: include/exec/exec-all.h
 F: include/exec/helper*.h
+F: include/exec/tb-hash.h
 
 Alpha
 M: Richard Henderson 
@@ -1042,6 +1050,7 @@ S: Supported
 F: include/exec/ioport.h
 F: ioport.c
 F: include/exec/memory.h
+F: include/exec/ram_addr.h
 F: memory.c
 F: include/exec/memory-internal.h
 F: exec.c
-- 
1.8.3.1

[Qemu-devel] [PATCH v1 1/7] drivers/hv: Move HV_SYNIC_STIMER_COUNT into Hyper-V UAPI x86 header

[Andrey Smetanin]

This constant is required for Hyper-V SynIC timers MSR's
support by userspace(QEMU).

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/include/uapi/asm/hyperv.h | 2 ++
 drivers/hv/hyperv_vmbus.h  | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 040d408..07981f0 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -269,4 +269,6 @@ typedef struct _HV_REFERENCE_TSC_PAGE {
 #define HV_SYNIC_SINT_AUTO_EOI (1ULL << 17)
 #define HV_SYNIC_SINT_VECTOR_MASK  (0xFF)
 
+#define HV_SYNIC_STIMER_COUNT  (4)
+
 #endif
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 3782636..46e23d1 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -102,8 +102,6 @@ enum hv_message_type {
HVMSG_X64_LEGACY_FP_ERROR   = 0x80010005
 };
 
-#define HV_SYNIC_STIMER_COUNT  (4)
-
 /* Define invalid partition identifier. */
 #define HV_PARTITION_ID_INVALID((u64)0x0)
 
-- 
2.4.3

[Qemu-devel] [PATCH v1 4/7] kvm/x86: Added Hyper-V vcpu_to_hv_vcpu()/hv_vcpu_to_vcpu() helpers

[Andrey Smetanin]

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.h | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index 9483d49..d5d8217 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -24,21 +24,29 @@
 #ifndef __ARCH_X86_KVM_HYPERV_H__
 #define __ARCH_X86_KVM_HYPERV_H__
 
-static inline struct kvm_vcpu_hv_synic *vcpu_to_synic(struct kvm_vcpu *vcpu)
+static inline struct kvm_vcpu_hv *vcpu_to_hv_vcpu(struct kvm_vcpu *vcpu)
 {
-   return >arch.hyperv.synic;
+   return >arch.hyperv;
 }
 
-static inline struct kvm_vcpu *synic_to_vcpu(struct kvm_vcpu_hv_synic *synic)
+static inline struct kvm_vcpu *hv_vcpu_to_vcpu(struct kvm_vcpu_hv *hv_vcpu)
 {
-   struct kvm_vcpu_hv *hv;
struct kvm_vcpu_arch *arch;
 
-   hv = container_of(synic, struct kvm_vcpu_hv, synic);
-   arch = container_of(hv, struct kvm_vcpu_arch, hyperv);
+   arch = container_of(hv_vcpu, struct kvm_vcpu_arch, hyperv);
return container_of(arch, struct kvm_vcpu, arch);
 }
 
+static inline struct kvm_vcpu_hv_synic *vcpu_to_synic(struct kvm_vcpu *vcpu)
+{
+   return >arch.hyperv.synic;
+}
+
+static inline struct kvm_vcpu *synic_to_vcpu(struct kvm_vcpu_hv_synic *synic)
+{
+   return hv_vcpu_to_vcpu(container_of(synic, struct kvm_vcpu_hv, synic));
+}
+
 int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
 int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
 
-- 
2.4.3

[Qemu-devel] [PATCH v1 5/7] kvm/x86: Hyper-V internal helper to read MSR HV_X64_MSR_TIME_REF_COUNT

[Andrey Smetanin]

This helper will be used also in Hyper-V SynIC timers implementation.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Vitaly Kuznetsov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 41869a9..9958926 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -335,6 +335,11 @@ static void synic_init(struct kvm_vcpu_hv_synic *synic)
}
 }
 
+static u64 get_time_ref_counter(struct kvm *kvm)
+{
+   return div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
+}
+
 void kvm_hv_vcpu_init(struct kvm_vcpu *vcpu)
 {
synic_init(vcpu_to_synic(vcpu));
@@ -576,11 +581,9 @@ static int kvm_hv_get_msr_pw(struct kvm_vcpu *vcpu, u32 
msr, u64 *pdata)
case HV_X64_MSR_HYPERCALL:
data = hv->hv_hypercall;
break;
-   case HV_X64_MSR_TIME_REF_COUNT: {
-   data =
-div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
+   case HV_X64_MSR_TIME_REF_COUNT:
+   data = get_time_ref_counter(kvm);
break;
-   }
case HV_X64_MSR_REFERENCE_TSC:
data = hv->hv_tsc_page;
break;
-- 
2.4.3

Re: [Qemu-devel] [RFC PATCH V2 0/3] IXGBE/VFIO: Add live migration support for SRIOV NIC

[Alexander Duyck]

On Wed, Nov 25, 2015 at 12:21 AM, Lan Tianyu  wrote:
> On 2015年11月25日 13:30, Alexander Duyck wrote:
>> No, what I am getting at is that you can't go around and modify the
>> configuration space for every possible device out there.  This
>> solution won't scale.
>
>
> PCI config space regs are emulation by Qemu and so We can find the free
> PCI config space regs for the faked PCI capability. Its position can be
> not permanent.

Yes, but do you really want to edit every driver on every OS that you
plan to support this on.  What about things like direct assignment of
regular Ethernet ports?  What you really need is a solution that will
work generically on any existing piece of hardware out there.

>>  If you instead moved the logic for notifying
>> the device into a separate mechanism such as making it a part of the
>> hot-plug logic then you only have to write the code once per OS in
>> order to get the hot-plug capability to pause/resume the device.  What
>> I am talking about is not full hot-plug, but rather to extend the
>> existing hot-plug in Qemu and the Linux kernel to support a
>> "pause/resume" functionality.  The PCI hot-plug specification calls
>> out the option of implementing something like this, but we don't
>> currently have support for it.
>>
>
> Could you elaborate the part of PCI hot-plug specification you mentioned?
>
> My concern is whether it needs to change PCI spec or not.


In the PCI Hot-Plug Specification 1.1, in section 4.1.2 it states:
 In addition to quiescing add-in card activity, an operating-system
vendor may optionally implement a less drastic “pause” capability, in
anticipation of the same or a similar add-in card being reinserted.

The idea I had was basically if we were to implement something like
that in Linux then we could pause/resume the device instead of
outright removing it.  The pause functionality could make use of the
suspend/resume functionality most drivers already have for PCI power
management.

- Alex

Re: [Qemu-devel] [PATCH v3 2/2] tests: add file-write-read test

[Michael Roth]

Quoting Eric Blake (2015-11-25 10:41:35)
> On 11/25/2015 09:21 AM, Michael Roth wrote:
> 
> >>> +/* seek to 0 */
> >>> +cmd = g_strdup_printf("{'execute': 'guest-file-seek',"
> >>> +  " 'arguments': { 'handle': %" PRId64 ", "
> >>> +  " 'offset': %d, 'whence': %d } }",
> >>> +  id, 0, SEEK_SET);
> >>
> >> We still have a conflict between this series and my proposal to codify 0
> >> rather than relying on platform-specific SEEK_SET; Markus had the
> >> suggestion of using QGA_SET (or QGA_SEEK_SET).  Are we trying to get
> >> both your series and my v2 patch into 2.5?  Knowing that will help me
> >> decide whether my v2 should be rebased on top of your patches.
> > 
> > I was planning on pulling in your patch on top of this for the next
> > 2.5 pull, so rebasing on top of this series is probably best.
> 
> Okay, will do, and will do quickly so I'm not holding up your pull request.

Thanks! FYI I now have this series applied here if you'd like to base
on that:

  https://github.com/mdroth/qemu/commits/qga

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

Re: [Qemu-devel] [PATCH 3/3] target-i386: kvm: Print warning when clearing mcg_cap bits

[Eduardo Habkost]

On Wed, Nov 25, 2015 at 05:45:20PM +0100, Paolo Bonzini wrote:
> On 25/11/2015 16:49, Eduardo Habkost wrote:
> > Instead of silently clearing mcg_cap bits when the host doesn't
> > support them, print a warning when doing that.
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  target-i386/kvm.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index d63a85b..446bdfc 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -774,7 +774,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >  && (env->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
> > (CPUID_MCE | CPUID_MCA)
> >  && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
> > -uint64_t mcg_cap;
> > +uint64_t mcg_cap, unsupported_caps;
> >  int banks;
> >  int ret;
> >  
> > @@ -790,6 +790,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >  return -ENOTSUP;
> >  }
> >  
> > +unsupported_caps = env->mcg_cap & ~(mcg_cap | MCG_CAP_BANKS_MASK);
> > +if (unsupported_caps) {
> > +error_report("warning: Unsupported MCG_CAP bits: 0x%" PRIx64 
> > "\n",
> 
> \n should not be at end of error_report.
> 
> Fixed and applied.

MCG_CAP_BANKS_MASK is defined by patch 2/3. Have you applied the
whole series?

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 for-2.5 11/12] qjson: surprise, allocating 6 QObjects per token is expensive

[Eric Blake]

On 11/25/2015 02:23 PM, Markus Armbruster wrote:
> From: Paolo Bonzini 
> 
> Replace the contents of the tokens GQueue with a simple struct.  This cuts
> the amount of memory allocated by tests/check-qjson from ~500MB to ~20MB,
> and the execution time from 600ms to 80ms on my laptop.  Still a lot (some
> could be saved by using an intrusive list, such as QSIMPLEQ, instead of
> the GQueue), but the savings are already massive and the right thing to
> do would probably be to get rid of json-streamer completely.
> 
> Signed-off-by: Paolo Bonzini 
> Message-Id: <1448300659-23559-5-git-send-email-pbonz...@redhat.com>
> [Straightforwardly rebased on my patches]
> Signed-off-by: Markus Armbruster 
> ---
>  include/qapi/qmp/json-streamer.h |   7 +++
>  qobject/json-parser.c| 115 
> ---
>  qobject/json-streamer.c  |  19 +++
>  3 files changed, 63 insertions(+), 78 deletions(-)
> 

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

[Qemu-devel] [PULL for-2.5 3/6] qga: flush explicitly when needed

[Michael Roth]

From: Marc-André Lureau 

According to the specification:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html

"the application shall ensure that output is not directly followed by
input without an intervening call to fflush() or to a file positioning
function (fseek(), fsetpos(), or rewind()), and input is not directly
followed by output without an intervening call to a file positioning
function, unless the input operation encounters end-of-file."

Without this change, an fwrite() followed by an fread() may lose the
previously written content, as shown in the following test.

Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1210246

Reviewed-by: Eric Blake 
* don't confuse {write,read}() with f{write,read}() in
  commit msg (Laszlo)
Signed-off-by: Michael Roth 
---
 qga/commands-posix.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0ebd473..cf1d7ec 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -216,9 +216,16 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, 
Error **errp)
 }
 }
 
+typedef enum {
+RW_STATE_NEW,
+RW_STATE_READING,
+RW_STATE_WRITING,
+} RwState;
+
 typedef struct GuestFileHandle {
 uint64_t id;
 FILE *fh;
+RwState state;
 QTAILQ_ENTRY(GuestFileHandle) next;
 } GuestFileHandle;
 
@@ -460,6 +467,17 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, 
bool has_count,
 }
 
 fh = gfh->fh;
+
+/* explicitly flush when switching from writing to reading */
+if (gfh->state == RW_STATE_WRITING) {
+int ret = fflush(fh);
+if (ret == EOF) {
+error_setg_errno(errp, errno, "failed to flush file");
+return NULL;
+}
+gfh->state = RW_STATE_NEW;
+}
+
 buf = g_malloc0(count+1);
 read_count = fread(buf, 1, count, fh);
 if (ferror(fh)) {
@@ -473,6 +491,7 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, 
bool has_count,
 if (read_count) {
 read_data->buf_b64 = g_base64_encode(buf, read_count);
 }
+gfh->state = RW_STATE_READING;
 }
 g_free(buf);
 clearerr(fh);
@@ -496,6 +515,16 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const 
char *buf_b64,
 }
 
 fh = gfh->fh;
+
+if (gfh->state == RW_STATE_READING) {
+int ret = fseek(fh, 0, SEEK_CUR);
+if (ret == -1) {
+error_setg_errno(errp, errno, "failed to seek file");
+return NULL;
+}
+gfh->state = RW_STATE_NEW;
+}
+
 buf = g_base64_decode(buf_b64, _len);
 
 if (!has_count) {
@@ -515,6 +544,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const 
char *buf_b64,
 write_data = g_new0(GuestFileWrite, 1);
 write_data->count = write_count;
 write_data->eof = feof(fh);
+gfh->state = RW_STATE_WRITING;
 }
 g_free(buf);
 clearerr(fh);
@@ -538,10 +568,15 @@ struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, 
int64_t offset,
 ret = fseek(fh, offset, whence);
 if (ret == -1) {
 error_setg_errno(errp, errno, "failed to seek file");
+if (errno == ESPIPE) {
+/* file is non-seekable, stdio shouldn't be buffering anyways */
+gfh->state = RW_STATE_NEW;
+}
 } else {
 seek_data = g_new0(GuestFileSeek, 1);
 seek_data->position = ftell(fh);
 seek_data->eof = feof(fh);
+gfh->state = RW_STATE_NEW;
 }
 clearerr(fh);
 
@@ -562,6 +597,8 @@ void qmp_guest_file_flush(int64_t handle, Error **errp)
 ret = fflush(fh);
 if (ret == EOF) {
 error_setg_errno(errp, errno, "failed to flush file");
+} else {
+gfh->state = RW_STATE_NEW;
 }
 }
 
-- 
1.9.1

[Qemu-devel] [PULL v2 for-2.5 6/6] qga: added another non-interactive gspawn() helper file.

[Michael Roth]

From: Yuri Pudgorodskiy 

With previous commit we added gspawn-win64-helper-console.exe,
required for gspawn() mingw implementation.
Unfortunatly when running as a service without interactive
desktop, gspawn() also requires another helper app.

Added gspawn-win64-helper.exe and gspawn-win32-helper.exe
for corresponding architectures.

Signed-off-by: Yuri Pudgorodskiy 
Signed-off-by: Denis V. Lunev 
CC: Michael Roth 
* remove trailing whitespace
Signed-off-by: Michael Roth 
---
 qga/installer/qemu-ga.wxs | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index f25afdd..9473875 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -95,11 +95,17 @@
   
 
   
+  
+
+  
   
   
   
 
   
+  
+
+  
   
   
 
@@ -159,6 +165,7 @@
   
   
   
+  
   
   
   
-- 
1.9.1

[Qemu-devel] [PATCH v6 05/23] qmp: Fix reference-counting of qnull on empty output visit

[Eric Blake]

Commit 6c2f9a15 ensured that we would not return NULL when the
caller used an output visitor but had nothing to visit. But
in doing so, it added a FIXME about a reference count leak
that could abort qemu in the (unlikely) case of SIZE_MAX such
visits (more plausible on 32-bit).

This fixes things by documenting the internal contracts, and
explaining why the internal function can return NULL and only
the public facing interface needs to worry about qnull(),
thus avoiding over-referencing the qnull_ global object.

It does not, however, fix the stupidity of the stack mixing
up two separate pieces of information; add a FIXME to explain
that issue.

Signed-off-by: Eric Blake 

---
v6: no change
---
 qapi/qmp-output-visitor.c   | 30 --
 tests/test-qmp-output-visitor.c |  2 ++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index e66ab3c..9d0f9d1 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -29,6 +29,12 @@ typedef QTAILQ_HEAD(QStack, QStackEntry) QStack;
 struct QmpOutputVisitor
 {
 Visitor visitor;
+/* FIXME: we are abusing stack to hold two separate pieces of
+ * information: the current root object, and the stack of objects
+ * still being built.  Worse, our behavior is inconsistent:
+ * visiting two top-level scalars in a row discards the first in
+ * favor of the second, but visiting two top-level objects in a
+ * row tries to append the second object into the first.  */
 QStack stack;
 };

@@ -41,10 +47,12 @@ static QmpOutputVisitor *to_qov(Visitor *v)
 return container_of(v, QmpOutputVisitor, visitor);
 }

+/* Push @value onto the stack of current QObjects being built */
 static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value)
 {
 QStackEntry *e = g_malloc0(sizeof(*e));

+assert(value);
 e->value = value;
 if (qobject_type(e->value) == QTYPE_QLIST) {
 e->is_list_head = true;
@@ -52,16 +60,20 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov, 
QObject *value)
 QTAILQ_INSERT_HEAD(>stack, e, node);
 }

+/* Grab and remove the most recent QObject from the stack */
 static QObject *qmp_output_pop(QmpOutputVisitor *qov)
 {
 QStackEntry *e = QTAILQ_FIRST(>stack);
 QObject *value;
+
+assert(e);
 QTAILQ_REMOVE(>stack, e, node);
 value = e->value;
 g_free(e);
 return value;
 }

+/* Grab the root QObject, if any, in preparation to empty the stack */
 static QObject *qmp_output_first(QmpOutputVisitor *qov)
 {
 QStackEntry *e = QTAILQ_LAST(>stack, QStack);
@@ -72,24 +84,32 @@ static QObject *qmp_output_first(QmpOutputVisitor *qov)
  * handle null.
  */
 if (!e) {
-return qnull();
+/* No root */
+return NULL;
 }
-
+assert(e->value);
 return e->value;
 }

+/* Grab the most recent QObject from the stack, which must exist */
 static QObject *qmp_output_last(QmpOutputVisitor *qov)
 {
 QStackEntry *e = QTAILQ_FIRST(>stack);
+
+assert(e);
 return e->value;
 }

+/* Add @value to the current QObject being built.
+ * If the stack is visiting a dictionary or list, @value is now owned
+ * by that container. Otherwise, @value is now the root.  */
 static void qmp_output_add_obj(QmpOutputVisitor *qov, const char *name,
QObject *value)
 {
 QObject *cur;

 if (QTAILQ_EMPTY(>stack)) {
+/* Stack was empty, track this object as root */
 qmp_output_push_obj(qov, value);
 return;
 }
@@ -98,13 +118,16 @@ static void qmp_output_add_obj(QmpOutputVisitor *qov, 
const char *name,

 switch (qobject_type(cur)) {
 case QTYPE_QDICT:
+assert(name);
 qdict_put_obj(qobject_to_qdict(cur), name, value);
 break;
 case QTYPE_QLIST:
 qlist_append_obj(qobject_to_qlist(cur), value);
 break;
 default:
+/* The previous root was a scalar, replace it with a new root */
 qobject_decref(qmp_output_pop(qov));
+assert(QTAILQ_EMPTY(>stack));
 qmp_output_push_obj(qov, value);
 break;
 }
@@ -205,11 +228,14 @@ static void qmp_output_type_any(Visitor *v, QObject 
**obj, const char *name,
 qmp_output_add_obj(qov, name, *obj);
 }

+/* Finish building, and return the root object. Will not be NULL. */
 QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
 {
 QObject *obj = qmp_output_first(qov);
 if (obj) {
 qobject_incref(obj);
+} else {
+obj = qnull();
 }
 return obj;
 }
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 3078442..8e6fc33 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -461,6 +461,8 @@ static void test_visitor_out_empty(TestOutputVisitorData 
*data,

 arg = qmp_output_get_qobject(data->qov);
 g_assert(qobject_type(arg) == QTYPE_QNULL);
+/* 

[Qemu-devel] [PATCH v6 06/23] qapi: Don't abuse stack to track qmp-output root

[Eric Blake]

The previous commit documented an inconsistency in how we are
using the stack of qmp-output-visitor.  Normally, pushing a
single top-level object puts the object on the stack twice:
once as the root, and once as the current container being
appended to; but popping that struct only pops once.  However,
qmp_ouput_add() was trying to either set up the added object
as the new root (works if you parse two top-level scalars in a
row: the second replaces the first as the root) or as a member
of the current container (works as long as you have an open
container on the stack; but if you have popped the first
top-level container, it then resolves to the root and still
tries to add into that existing container).

Fix the stupidity by not tracking two separate things in the
stack.

Not done here: maybe qmp_output_get_object() should assert that
the stack is empty, rather than letting users look at the current
root even while the root is still being visited.

Signed-off-by: Eric Blake 

---
v6: no change
---
 qapi/qmp-output-visitor.c | 70 +++
 1 file changed, 22 insertions(+), 48 deletions(-)

diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 9d0f9d1..4a28ce3 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -29,13 +29,8 @@ typedef QTAILQ_HEAD(QStack, QStackEntry) QStack;
 struct QmpOutputVisitor
 {
 Visitor visitor;
-/* FIXME: we are abusing stack to hold two separate pieces of
- * information: the current root object, and the stack of objects
- * still being built.  Worse, our behavior is inconsistent:
- * visiting two top-level scalars in a row discards the first in
- * favor of the second, but visiting two top-level objects in a
- * row tries to append the second object into the first.  */
-QStack stack;
+QStack stack; /* Stack of containers still growing */
+QObject *root; /* Root of the output visit */
 };

 #define qmp_output_add(qov, name, value) \
@@ -52,6 +47,7 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov, 
QObject *value)
 {
 QStackEntry *e = g_malloc0(sizeof(*e));

+assert(qov->root);
 assert(value);
 e->value = value;
 if (qobject_type(e->value) == QTYPE_QLIST) {
@@ -76,28 +72,15 @@ static QObject *qmp_output_pop(QmpOutputVisitor *qov)
 /* Grab the root QObject, if any, in preparation to empty the stack */
 static QObject *qmp_output_first(QmpOutputVisitor *qov)
 {
-QStackEntry *e = QTAILQ_LAST(>stack, QStack);
-
-/*
- * FIXME Wrong, because qmp_output_get_qobject() will increment
- * the refcnt *again*.  We need to think through how visitors
- * handle null.
- */
-if (!e) {
-/* No root */
-return NULL;
-}
-assert(e->value);
-return e->value;
+return qov->root;
 }

-/* Grab the most recent QObject from the stack, which must exist */
+/* Grab the most recent QObject from the stack, if any */
 static QObject *qmp_output_last(QmpOutputVisitor *qov)
 {
 QStackEntry *e = QTAILQ_FIRST(>stack);

-assert(e);
-return e->value;
+return e ? e->value : NULL;
 }

 /* Add @value to the current QObject being built.
@@ -108,28 +91,23 @@ static void qmp_output_add_obj(QmpOutputVisitor *qov, 
const char *name,
 {
 QObject *cur;

-if (QTAILQ_EMPTY(>stack)) {
-/* Stack was empty, track this object as root */
-qmp_output_push_obj(qov, value);
-return;
-}
-
 cur = qmp_output_last(qov);

-switch (qobject_type(cur)) {
-case QTYPE_QDICT:
-assert(name);
-qdict_put_obj(qobject_to_qdict(cur), name, value);
-break;
-case QTYPE_QLIST:
-qlist_append_obj(qobject_to_qlist(cur), value);
-break;
-default:
-/* The previous root was a scalar, replace it with a new root */
-qobject_decref(qmp_output_pop(qov));
-assert(QTAILQ_EMPTY(>stack));
-qmp_output_push_obj(qov, value);
-break;
+if (!cur) {
+qobject_decref(qov->root);
+qov->root = value;
+} else {
+switch (qobject_type(cur)) {
+case QTYPE_QDICT:
+assert(name);
+qdict_put_obj(qobject_to_qdict(cur), name, value);
+break;
+case QTYPE_QLIST:
+qlist_append_obj(qobject_to_qlist(cur), value);
+break;
+default:
+g_assert_not_reached();
+}
 }
 }

@@ -249,16 +227,12 @@ void qmp_output_visitor_cleanup(QmpOutputVisitor *v)
 {
 QStackEntry *e, *tmp;

-/* The bottom QStackEntry, if any, owns the root QObject. See the
- * qmp_output_push_obj() invocations in qmp_output_add_obj(). */
-QObject *root = QTAILQ_EMPTY(>stack) ? NULL : qmp_output_first(v);
-
 QTAILQ_FOREACH_SAFE(e, >stack, node, tmp) {
 QTAILQ_REMOVE(>stack, e, node);
 g_free(e);
 }

-qobject_decref(root);
+qobject_decref(v->root);
 g_free(v);
 }

-- 
2.4.3

[Qemu-devel] [PATCH v6 23/23] qapi: Change visit_type_FOO() to no longer return partial objects

[Eric Blake]

Returning a partial object on error is an invitation for a careless
caller to leak memory.  As no one outside the testsuite was actually
relying on these semantics, it is cleaner to just document and
guarantee that ALL visit_type_FOO() functions leave a safe value
in *obj when an error is encountered during an input visitor.

Since input visitors have blind assignment semantics, we have to
track the result of whether an assignment is made all the way down
to each visitor callback implementation.

Signed-off-by: Eric Blake 

---
v6: rebase on top of earlier doc and formatting improvements, mention
that *obj can be uninitialized on entry to an input visitor, rework
semantics to keep valgrind happy on uninitialized input, break some
long lines
---
 include/qapi/visitor-impl.h|  6 ++---
 include/qapi/visitor.h | 52 +++---
 qapi/opts-visitor.c|  8 ---
 qapi/qapi-dealloc-visitor.c|  9 +---
 qapi/qapi-visit-core.c | 13 ++-
 qapi/qmp-input-visitor.c   | 17 +-
 qapi/qmp-output-visitor.c  |  6 +++--
 qapi/string-input-visitor.c|  3 ++-
 qapi/string-output-visitor.c   |  3 ++-
 scripts/qapi-visit.py  | 40 
 tests/test-qmp-commands.c  | 13 +--
 tests/test-qmp-input-strict.c  | 19 +++
 tests/test-qmp-input-visitor.c | 10 ++--
 13 files changed, 125 insertions(+), 74 deletions(-)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 5350bdf..a51aa2a 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -26,7 +26,7 @@ struct Visitor
 {
 /* Must be provided to visit structs (the string visitors do not
  * currently visit structs). */
-void (*start_struct)(Visitor *v, void **obj, const char *kind,
+bool (*start_struct)(Visitor *v, void **obj, const char *kind,
  const char *name, size_t size, Error **errp);
 /* May be NULL; most useful for input visitors. */
 void (*check_struct)(Visitor *v, Error **errp);
@@ -34,13 +34,13 @@ struct Visitor
 void (*end_struct)(Visitor *v);

 /* May be NULL; most useful for input visitors. */
-void (*start_implicit_struct)(Visitor *v, void **obj, size_t size,
+bool (*start_implicit_struct)(Visitor *v, void **obj, size_t size,
   Error **errp);
 /* May be NULL */
 void (*end_implicit_struct)(Visitor *v);

 /* Must be set */
-void (*start_list)(Visitor *v, const char *name, Error **errp);
+bool (*start_list)(Visitor *v, const char *name, Error **errp);
 /* Must be set */
 GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp);
 /* Must be set */
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index f9ea325..0b63a7a 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -30,6 +30,27 @@
  * visitor-impl.h.
  */

+/* All qapi types have a corresponding function with a signature
+ * roughly compatible with this:
+ *
+ * void visit_type_FOO(Visitor *v, void *obj, const char *name, Error **errp);
+ *
+ * where *@obj is itself a pointer or a scalar.  The visit functions for
+ * built-in types are declared here, while the functions for qapi-defined
+ * struct, union, enum, and list types are generated (see qapi-visit.h).
+ * Input visitors may receive an uninitialized *@obj, and guarantee a
+ * safe value is assigned (a new object on success, or NULL on failure).
+ * Output visitors expect *@obj to be properly initialized on entry.
+ *
+ * Additionally, all qapi structs have a generated function compatible
+ * with this:
+ *
+ * void qapi_free_FOO(void *obj);
+ *
+ * which behaves like free(), even if @obj is NULL or was only partially
+ * allocated before encountering an error.
+ */
+
 /* This struct is layout-compatible with all other *List structs
  * created by the qapi generator. */
 typedef struct GenericList
@@ -51,10 +72,12 @@ typedef struct GenericList
  * bytes, without regards to the previous value of *@obj. For other
  * visitors, *@obj is the object to visit. Set *@errp on failure.
  *
- * FIXME: *@obj can be modified even on error; this can lead to
- * memory leaks if clients aren't careful.
+ * Returns true if *@obj was allocated; if that happens, and an error
+ * occurs any time before the matching visit_end_struct(), then the
+ * caller (usually a visit_type_FOO() function) knows to undo the
+ * allocation before returning control further.
  */
-void visit_start_struct(Visitor *v, void **obj, const char *kind,
+bool visit_start_struct(Visitor *v, void **obj, const char *kind,
 const char *name, size_t size, Error **errp);
 /**
  * Prepare for completing a struct visit.
@@ -73,14 +96,12 @@ void visit_end_struct(Visitor *v);

 /**
  * Prepare to visit an implicit struct.
- * Similar to visit_start_struct(), except that in implicit struct
- * 

[Qemu-devel] [PATCH v6 16/23] qapi: Track all failures between visit_start/stop

[Eric Blake]

Inside the generated code between visit_start_struct() and
visit_end_struct(), we were blindly setting the error into
the caller's errp parameter.  But a future patch to split
visit_end_struct() will require that we take action based
on whether an error has occurred, which requires us to track
all actions through a local err.  Rewrite the visits to be
more in line with the other generated calls.

Signed-off-by: Eric Blake 

---
v6: based loosely on v5 7/46, but mostly a rewrite to get the last
generated code in the same form as all the others, so that the
later conversion to split visit_check_struct() will be easier
---
 scripts/qapi-visit.py | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 8c964b5..391bdfb 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -123,12 +123,18 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error
 Error *err = NULL;

 visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), 
);
-if (!err) {
-if (*obj) {
-visit_type_%(c_name)s_fields(v, obj, errp);
-}
-visit_end_struct(v, );
+if (err) {
+goto out;
 }
+if (!*obj) {
+goto out_obj;
+}
+visit_type_%(c_name)s_fields(v, obj, );
+out_obj:
+error_propagate(errp, err);
+err = NULL;
+visit_end_struct(v, );
+out:
 error_propagate(errp, err);
 }
 ''',
-- 
2.4.3

[Qemu-devel] [PATCH v6 21/23] qapi: Simplify extra member error reporting in input visitors

[Eric Blake]

When reporting that an unvisited member remains at the end of an
input visit for a struct, we were using g_hash_table_find()
coupled with a callback function that always returns true, to
locate an arbitrary member of the hash table.  But if all we
need is one entry, we can get that from a single iteration on an
iterator, without needing to split logic to a callback function.

Suggested-by: Markus Armbruster 
Signed-off-by: Eric Blake 

---
v6: new patch, based on comments on RFC against v5 7/46
---
 qapi/opts-visitor.c  | 12 +++-
 qapi/qmp-input-visitor.c | 14 +-
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 37f172d..46dd9fe 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -150,17 +150,11 @@ opts_start_struct(Visitor *v, void **obj, const char 
*kind,
 }


-static gboolean
-ghr_true(gpointer ign_key, gpointer ign_value, gpointer ign_user_data)
-{
-return TRUE;
-}
-
-
 static void
 opts_end_struct(Visitor *v, Error **errp)
 {
 OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
+GHashTableIter iter;
 GQueue *any;

 if (--ov->depth > 0) {
@@ -168,8 +162,8 @@ opts_end_struct(Visitor *v, Error **errp)
 }

 /* we should have processed all (distinct) QemuOpt instances */
-any = g_hash_table_find(ov->unprocessed_opts, _true, NULL);
-if (any) {
+g_hash_table_iter_init(, ov->unprocessed_opts);
+if (g_hash_table_iter_next(, NULL, (void **))) {
 const QemuOpt *first;

 first = g_queue_peek_head(any);
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 9ff1e75..9dbe025 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -88,12 +88,6 @@ static void qmp_input_push(QmpInputVisitor *qiv, QObject 
*obj, Error **errp)
 qiv->nb_stack++;
 }

-/** Only for qmp_input_pop. */
-static gboolean always_true(gpointer key, gpointer val, gpointer user_pkey)
-{
-*(const char **)user_pkey = (const char *)key;
-return TRUE;
-}

 static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
 {
@@ -102,9 +96,11 @@ static void qmp_input_pop(QmpInputVisitor *qiv, Error 
**errp)
 if (qiv->strict) {
 GHashTable * const top_ht = qiv->stack[qiv->nb_stack - 1].h;
 if (top_ht) {
-if (g_hash_table_size(top_ht)) {
-const char *key;
-g_hash_table_find(top_ht, always_true, );
+GHashTableIter iter;
+const char *key;
+
+g_hash_table_iter_init(, top_ht);
+if (g_hash_table_iter_next(, (void **), NULL)) {
 error_setg(errp, QERR_QMP_EXTRA_MEMBER, key);
 }
 g_hash_table_unref(top_ht);
-- 
2.4.3

Re: [Qemu-devel] [PATCH v8] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Eric Blake]

On 11/25/2015 05:24 PM, Programmingkid wrote:
> Mac OS X can be picky when it comes to allowing the user
> to use physical devices in QEMU. Most mounted volumes
> appear to be off limits to QEMU. If an issue is detected,
> a message is displayed showing the user how to unmount a
> volume.
> 
> Signed-off-by: John Arbuckle 
> 
> ---

Right here (between the --- and diffstat) it's nice to post a changelog
of how v8 differs from v7, to help earlier reviewers focus on the
improvements.

>  block/raw-posix.c |   98 
> +++--
>  1 files changed, 72 insertions(+), 26 deletions(-)

> +++ b/block/raw-posix.c
> @@ -42,9 +42,8 @@
>  #include 
>  #include 
>  #include 
> -//#include 
>  #include 
> -#endif
> +#endif /* (__APPLE__) && (__MACH__) */
>  

This hunk looks to be an unrelated cleanup; you might want to just
propose it separately through the qemu-trivial queue (but don't forget
that even trivial patches must cc qemu-devel).

> +
> +/* look for a working partition */
> +for (index = 0; index < num_of_test_partitions; index++) {
> +snprintf(test_partition, sizeof(test_partition), "%ss%d", bsd_path,
> + 
> index);

Unusual indentation.  More typical would be:

snprintf(test_partition, sizeof(test_partition), "%ss%d",
 bsd_path, index);

with the second line flush to the character after the ( of the first line.

> +fd = qemu_open(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE);

Isn't qemu_open() supposed to provide O_LARGEFILE for ALL users
automatically?  (That is, why would we ever _want_ to handle a file
using only 32-bit off_t?)  But that's a separate issue; it looks like
you are copy-and-pasting from existing use of this idiom already in
raw-posix.c.

> +if (fd >= 0) {
> +partition_found = true;
> +qemu_close(fd);
> +break;
> +}
> +}
> +
> +/* if a working partition on the device was not found */
> +if (partition_found == false) {
> +error_setg(errp, "Error: Failed to find a working partition on "
> + 
> "disc!\n");

Several violations of convention.  error_setg() does not need a
redundant "Error: " prefix, should not end in '!' (we aren't shouting),
and should not end in newline.  And with those fixes, you won't even
need the weird indentation.

error_setg(errp, "failed to find a working partition on disk");

>  
> +/* Prints directions on mounting and unmounting a device */
> +static void printUnmountingDirections(const char *file_name)

Elsewhere, we use 'function_name', not 'functionName'.

> +{
> +error_report("Error: If device %s is mounted on the desktop, unmount"
> + " it first before using it in QEMU.\n", 
> file_name);
> +error_report("Command to unmount device: diskutil unmountDisk %s\n",
> + 
> file_name);
> +error_report("Command to mount device: diskutil mountDisk %s\n",
> + 
> file_name);

Again, weird indentation. And don't use \n at the end of error_report().

> @@ -2123,32 +2162,32 @@ static int hdev_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  int ret;
>  
>  #if defined(__APPLE__) && defined(__MACH__)
> -const char *filename = qdict_get_str(options, "filename");
> +const char *file_name = qdict_get_str(options, "filename");

No need to rename this variable.

> +
> +/* If a real optical drive was not found */
> +if (bsd_path[0] == '\0') {
> +error_setg(errp, "Error: failed to obtain bsd path for optical"
> +   " 
> drive!\n");

Again, weird indentation, redundant "Error: ", and no "!\n" at the end.

>  
> +#if defined(__APPLE__) && defined(__MACH__)
> +/* if a physical device experienced an error while being opened */
> +if (strncmp(file_name, "/dev/", 5) == 0 && ret != 0) {
> +printUnmountingDirections(file_name);

Is this advice appropriate to ALL things under /dev/, or just cdroms?

> +return -1;
> +}
> +#endif /* defined(__APPLE__) && defined(__MACH__) */
> +
>  /* Since this does ioctl the device must be already opened */
>  bs->sg = hdev_is_sg(bs);
>  
> 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL for-2.5 0/6] qemu-ga patch queue for 2.5

[Michael Roth]

The following changes since commit 1a4dab849d5d06191ab5e5850f6b8bfcad8ceb47:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
(2015-11-25 14:47:06 +)

are available in the git repository at:


  git://github.com/mdroth/qemu.git tags/qga-pull-2015-11-25-tag

for you to fetch changes up to 35f8c32a200c8133c66642ce0dac721b3480178a:

  qga: added another non-interactive gspawn() helper file. (2015-11-25 15:34:48 
-0600)


qemu-ga patch queue for 2.5

* include additional w32 MSI install components needed for
  guest-exec
* fix 'make install' when compiling with --disable-tools
* fix potential data corruption/loss when accessing files
  bi-directionally via guest-file-{read,write}
* explicitly document how integer args for guest-file-seek map to
  SEEK_SET/SEEK_CUR/etc to avoid platform-specific differences


Eric Blake (1):
  qga: Better mapping of SEEK_* in guest-file-seek

Marc-André Lureau (2):
  qga: flush explicitly when needed
  tests: add file-write-read test

Michael Roth (1):
  makefile: fix qemu-ga make install for --disable-tools

Yuri Pudgorodskiy (2):
  qga: gspawn() console helper to Windows guest agent msi build
  qga: added another non-interactive gspawn() helper file.

 Makefile  |  5 +--
 qga/commands-posix.c  | 56 ++-
 qga/commands-win32.c  | 20 +-
 qga/guest-agent-core.h|  7 
 qga/installer/qemu-ga.wxs | 18 +
 qga/qapi-schema.json  |  4 +-
 tests/test-qga.c  | 98 +--
 7 files changed, 197 insertions(+), 11 deletions(-)

[Qemu-devel] [PULL for-2.5 1/6] makefile: fix qemu-ga make install for --disable-tools

[Michael Roth]

ab59e3e introduced a fix for `make install` on w32 that involved
filtering out qemu-ga from $TOOLS install recipe so that we could
append $(EXESUF) to it before attempting to install the binary
via install-prog function.

install-prog takes a list of binaries to install to a particular
directory. If the list is empty it breaks. We guard against this
by ensuring $TOOLS is not empty prior to calling.

However, ab59e3e introduces extra filtering after this check which
can still result on us attempting to call install-prog with an
empty list of binaries. In particular, this occurs if we
build with the --disable-tools configure option, which results
in qemu-ga being the only member of $TOOLS.

Fix this by doing a simple s/qemu-ga/qemu-ga$(EXESUF)/ pass through
$TOOLS instead of filtering out qemu-ga to handle it seperately.

Reported-by: Steve Ellcey 
Cc: Stefan Weil 
Signed-off-by: Michael Roth 
---
 Makefile | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index c7fa427..930ac27 100644
--- a/Makefile
+++ b/Makefile
@@ -440,10 +440,7 @@ endif
 install: all $(if $(BUILD_DOCS),install-doc) \
 install-datadir install-localstatedir
 ifneq ($(TOOLS),)
-   $(call install-prog,$(filter-out qemu-ga,$(TOOLS)),$(DESTDIR)$(bindir))
-ifneq (,$(findstring qemu-ga,$(TOOLS)))
-   $(call install-prog,qemu-ga$(EXESUF),$(DESTDIR)$(bindir))
-endif
+   $(call install-prog,$(subst 
qemu-ga,qemu-ga$(EXESUF),$(TOOLS)),$(DESTDIR)$(bindir))
 endif
 ifneq ($(CONFIG_MODULES),)
$(INSTALL_DIR) "$(DESTDIR)$(qemu_moddir)"
-- 
1.9.1

Re: [Qemu-devel] [PULL for-2.5 0/6] qemu-ga patch queue for 2.5

[Michael Roth]

Quoting Eric Blake (2015-11-25 17:47:00)
> On 11/25/2015 03:47 PM, Michael Roth wrote:
> > The following changes since commit 1a4dab849d5d06191ab5e5850f6b8bfcad8ceb47:
> > 
> >   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into 
> > staging (2015-11-25 14:47:06 +)
> > 
> > are available in the git repository at:
> > 
> > 
> >   git://github.com/mdroth/qemu.git tags/qga-pull-2015-11-25-tag
> > 
> > for you to fetch changes up to 35f8c32a200c8133c66642ce0dac721b3480178a:
> > 
> >   qga: added another non-interactive gspawn() helper file. (2015-11-25 
> > 15:34:48 -0600)
> > 
> > 
> > qemu-ga patch queue for 2.5
> > 
> > * include additional w32 MSI install components needed for
> >   guest-exec
> > * fix 'make install' when compiling with --disable-tools
> > * fix potential data corruption/loss when accessing files
> >   bi-directionally via guest-file-{read,write}
> > * explicitly document how integer args for guest-file-seek map to
> >   SEEK_SET/SEEK_CUR/etc to avoid platform-specific differences
> > 
> > 
> > Eric Blake (1):
> >   qga: Better mapping of SEEK_* in guest-file-seek
> > 
> > Marc-André Lureau (2):
> >   qga: flush explicitly when needed
> >   tests: add file-write-read test
> 
> A v2 would be wise to ensure we have all the required S-o-b tags (see 3/6)

v2 sent. Not sure why I can't seem to get that patch right. Sorry for the noise.

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

[Qemu-devel] [PATCH v6 00/23] qapi visitor cleanups (post-introspection cleanups subset E)

[Eric Blake]

Pending prerequisites:
+ Markus' "typedefs: Put them back into alphabetical order"
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg04417.html
+ Markus' qapi-next branch
http://repo.or.cz/qemu/armbru.git/shortlog/refs/heads/qapi-next
+ My v13 subset D patches:
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg04732.html

Also available as a tag at this location:
git fetch git://repo.or.cz/qemu/ericb.git qapi-cleanupv6e

and will soon be part of my branch with the rest of the v5 series, at:
http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi

v6 notes:
My set of patches related to qapi visitors has grown, and it's time
that I post it on list again.  Of course, since this is all 2.6
material, and there's already lots of patches earlier in the queue,
I may need a v7 to pick up rebase changes.

A lot of the new patches in this series are based on fallout from
implementing an early RFC posted against a v5 review:
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg06878.html

Backport diff from v5:

001/23:[down] 'qapi: Make all visitors supply int64/uint64 callbacks'
002/23:[down] 'qapi: Require int64/uint64 implementation'
003/23:[down] 'qapi: Consolidate visitor integer callbacks'
004/23:[down] 'qapi: Don't cast Enum* to int*'
005/23:[] [--] 'qmp: Fix reference-counting of qnull on empty output visit'
006/23:[] [--] 'qapi: Don't abuse stack to track qmp-output root'
007/23:[0100] [FC] 'qapi: Document visitor interfaces'
008/23:[down] 'qapi: Drop unused error argument for list and implicit struct'
009/23:[down] 'hmp: Improve use of qapi visitor'
010/23:[down] 'vl: Improve use of qapi visitor'
011/23:[down] 'ppc: Improve use of qapi visitors'
012/23:[down] 'balloon: Improve use of qapi visitor'
013/23:[down] 'qapi: Add type.is_empty() helper'
014/23:[down] 'qapi: Fix command with named empty argument type'
015/23:[down] 'qapi: Improve generated event use of qapi visitor'
016/23:[down] 'qapi: Track all failures between visit_start/stop'
017/23:[down] 'qapi: Eliminate empty visit_type_FOO_fields'
018/23:[down] 'qapi: Canonicalize missing object to :empty'
019/23:[down] 'qapi-visit: Unify struct and union visit'
020/23:[0029] [FC] 'qapi: Rework deallocation of partial struct'
021/23:[down] 'qapi: Simplify extra member error reporting in input visitors'
022/23:[down] 'qapi: Split visit_end_struct() into pieces'
023/23:[0174] [FC] 'qapi: Change visit_type_FOO() to no longer return partial 
objects'

Subset F (and more?) will come later.

In v5:
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05410.html
I _did_ rearrange patches to try and group related features:

1-2: Groundwork cleanups
3-5: Add more test cases
6-16: Front-end cleanups
17-18: Introspection output cleanups
19-20: 'alternate' type cleanups
21-29: qapi visitor cleanups
30-45: qapi-ify netdev_add
46: add qapi shorthand for flat unions

Lots of fixes based on additional testing, and rebased to
track other changes that happened in the meantime.  The series
is huge; I can split off smaller portions as requested.

In v4:
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02580.html
add some more clean up patches
rebase to Markus' recent work
pull in part of Zoltán's work to make netdev_add a flat union,
further enhancing it to be introspectible

I might be able to rearrange some of these patches, or separate
it into smaller independent series, if requested; but I'm
posting now to get review started.

In v3:
https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02059.html
redo cleanup of dealloc of partial struct
add patches to make all visit_type_*() avoid leaks on failure
add patches to allow boxed command arguments and events

In v2:
https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00900.html
rebase to Markus' v3 series
rework how comments are emitted for fields inherited from base
additional patches added for deleting colliding 'void *data'
documentation updates to match code changes

v1 was here:
https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05266.html
https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05325.html

Eric Blake (23):
  qapi: Make all visitors supply int64/uint64 callbacks
  qapi: Require int64/uint64 implementation
  qapi: Consolidate visitor integer callbacks
  qapi: Don't cast Enum* to int*
  qmp: Fix reference-counting of qnull on empty output visit
  qapi: Don't abuse stack to track qmp-output root
  qapi: Document visitor interfaces
  qapi: Drop unused error argument for list and implicit struct
  hmp: Improve use of qapi visitor
  vl: Improve use of qapi visitor
  ppc: Improve use of qapi visitors
  balloon: Improve use of qapi visitor
  qapi: Add type.is_empty() helper
  qapi: Fix command with named empty argument type
  qapi: Improve generated event use of qapi visitor
  qapi: Track all failures between visit_start/stop
  qapi: Eliminate empty visit_type_FOO_fields
  qapi: Canonicalize missing object to :empty
  qapi-visit: Unify struct and union visit
 

[Qemu-devel] [PATCH v6 03/23] qapi: Consolidate visitor integer callbacks

[Eric Blake]

Commit 4e27e819 introduced optional visitor callbacks for all
sorts of int types, but except for type_uint64() and type_size(),
none of them have ever been supplied (the generic implementation
based on using type_[u]int64() then bounds-checking works just
fine). In the interest of simplicity, it's easier to make the
visitor callback interface not have to worry about the other
sizes.

Signed-off-by: Eric Blake 

---
v6: split off from v5 23/46
original version also appeared in v6-v9 of subset D
---
 include/qapi/visitor-impl.h |  22 -
 qapi/qapi-visit-core.c  | 117 ++--
 2 files changed, 59 insertions(+), 80 deletions(-)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 70326e0..d071c06 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -1,7 +1,7 @@
 /*
  * Core Definitions for QAPI Visitor implementations
  *
- * Copyright (C) 2012 Red Hat, Inc.
+ * Copyright (C) 2012, 2015 Red Hat, Inc.
  *
  * Author: Paolo Bonizni 
  *
@@ -36,6 +36,16 @@ struct Visitor
 void (*get_next_type)(Visitor *v, QType *type, bool promote_int,
   const char *name, Error **errp);

+/* Must be set. */
+void (*type_int64)(Visitor *v, int64_t *obj, const char *name,
+   Error **errp);
+/* Must be set. */
+void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name,
+Error **errp);
+/* Only required to visit sizes differently than (*type_uint64)().  */
+void (*type_size)(Visitor *v, uint64_t *obj, const char *name,
+  Error **errp);
+/* Must be set. */
 void (*type_bool)(Visitor *v, bool *obj, const char *name, Error **errp);
 void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
 void (*type_number)(Visitor *v, double *obj, const char *name,
@@ -46,16 +56,6 @@ struct Visitor
 /* May be NULL; most useful for input visitors. */
 void (*optional)(Visitor *v, bool *present, const char *name);

-void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error 
**errp);
-void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error 
**errp);
-void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error 
**errp);
-void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name, Error 
**errp);
-void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error **errp);
-void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error 
**errp);
-void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error 
**errp);
-void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error 
**errp);
-/* visit_type_size() falls back to (*type_uint64)() if type_size is unset 
*/
-void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error 
**errp);
 bool (*start_union)(Visitor *v, bool data_present, Error **errp);
 void (*end_union)(Visitor *v, bool data_present, Error **errp);
 };
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 88bed9c..be1fcdd 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -104,57 +104,48 @@ void visit_type_uint8(Visitor *v, uint8_t *obj, const 
char *name, Error **errp)
 {
 uint64_t value;

-if (v->type_uint8) {
-v->type_uint8(v, obj, name, errp);
-} else {
-value = *obj;
-v->type_uint64(v, , name, errp);
-if (value > UINT8_MAX) {
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-   name ? name : "null", "uint8_t");
-return;
-}
-*obj = value;
+value = *obj;
+v->type_uint64(v, , name, errp);
+if (value > UINT8_MAX) {
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+   name ? name : "null", "uint8_t");
+return;
 }
+*obj = value;
 }

-void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error 
**errp)
+void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name,
+   Error **errp)
 {
 uint64_t value;

-if (v->type_uint16) {
-v->type_uint16(v, obj, name, errp);
-} else {
-value = *obj;
-v->type_uint64(v, , name, errp);
-if (value > UINT16_MAX) {
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-   name ? name : "null", "uint16_t");
-return;
-}
-*obj = value;
+value = *obj;
+v->type_uint64(v, , name, errp);
+if (value > UINT16_MAX) {
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+   name ? name : "null", "uint16_t");
+return;
 }
+*obj = value;
 }

-void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error 
**errp)
+void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name,
+   Error **errp)
 {
 uint64_t 

[Qemu-devel] [PATCH v6 15/23] qapi: Improve generated event use of qapi visitor

[Eric Blake]

All other successful clients of visit_start_struct() were paired
with an unconditional visit_end_struct(); but the generated
code for events was relying on qmp_output_visitor_cleanup() to
work on an incomplete visit.  Alter the code to guarantee that
the struct is completed, which will make a future patch to
split visit_end_struct() easier to reason about.  While at it,
drop some assertions and comments that are not present in other
uses of the qmp output visitor, and rearrange the declaration
to make it easier for a future patch to introduce the notion of
a boxed event visit.

Signed-off-by: Eric Blake 

---
v6: new patch

If desired, I can defer the hunk re-ordering the declaration of
obj to later in the series where it actually comes in handy.
---
 scripts/qapi-event.py | 19 ++-
 scripts/qapi.py   |  5 +++--
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 51128f4..5dc9726 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -41,9 +41,9 @@ def gen_event_send(name, arg_type):

 if arg_type and not arg_type.is_empty():
 ret += mcgen('''
+QObject *obj;
 QmpOutputVisitor *qov;
 Visitor *v;
-QObject *obj;

 ''')

@@ -59,27 +59,28 @@ def gen_event_send(name, arg_type):
  name=name)

 if arg_type and not arg_type.is_empty():
+c_name = 'NULL'
+if not arg_type.is_implicit():
+c_name = '"%s"' % arg_type.c_name()
 ret += mcgen('''
 qov = qmp_output_visitor_new();
-g_assert(qov);
-
 v = qmp_output_get_visitor(qov);
-g_assert(v);

-/* Fake visit, as if all members are under a structure */
-visit_start_struct(v, NULL, "", "%(name)s", 0, );
+visit_start_struct(v, NULL, %(c_name)s, "%(name)s", 0, );
 ''',
- name=name)
+ c_name=c_name, name=name)
 ret += gen_err_check()
-ret += gen_visit_fields(arg_type.members, need_cast=True)
+ret += gen_visit_fields(arg_type.members, need_cast=True,
+label='out_obj')
 ret += mcgen('''
+out_obj:
 visit_end_struct(v, );
 if (err) {
 goto out;
 }

 obj = qmp_output_get_qobject(qov);
-g_assert(obj != NULL);
+g_assert(obj);

 qdict_put_obj(qmp, "data", obj);
 ''')
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 45bc5a7..ed2a063 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1637,7 +1637,8 @@ def gen_err_check(label='out', skiperr=False):
  label=label)


-def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False):
+def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False,
+ label='out'):
 ret = ''
 if skiperr:
 errparg = 'NULL'
@@ -1665,7 +1666,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, 
skiperr=False):
  c_type=memb.type.c_name(), prefix=prefix, cast=cast,
  c_name=c_name(memb.name), name=memb.name,
  errp=errparg)
-ret += gen_err_check(skiperr=skiperr)
+ret += gen_err_check(skiperr=skiperr, label=label)

 if memb.optional:
 pop_indent()
-- 
2.4.3

[Qemu-devel] [PATCH v6 10/23] vl: Improve use of qapi visitor

[Eric Blake]

Cache the visitor in a local variable instead of repeatedly
calling the accessor.  Pass NULL for the visit_start_struct()
object (which matches the fact that we were already passing 0
for the size argument, because we aren't using the visit to
allocate a qapi struct).  Pass "object" for the struct name,
for better error messages. Reflow the logic so that we don't
have to undo an object_add().

A later patch will then split the error detection currently
in visit_struct_end(), at which point we can again hoist the
object_add() to occur before the label as one of the cleanups
enabled by that split.

Signed-off-by: Eric Blake 

---
v6: new patch, split from RFC on v5 7/46
---
 vl.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/vl.c b/vl.c
index 4d6263d..4e69815 100644
--- a/vl.c
+++ b/vl.c
@@ -2828,43 +2828,42 @@ static bool object_create_delayed(const char *type)
 static int object_create(void *opaque, QemuOpts *opts, Error **errp)
 {
 Error *err = NULL;
+Error *err_end = NULL;
 char *type = NULL;
 char *id = NULL;
-void *dummy = NULL;
 OptsVisitor *ov;
 QDict *pdict;
 bool (*type_predicate)(const char *) = opaque;
+Visitor *v;

 ov = opts_visitor_new(opts);
 pdict = qemu_opts_to_qdict(opts, NULL);
+v = opts_get_visitor(ov);

-visit_start_struct(opts_get_visitor(ov), , NULL, NULL, 0, );
+visit_start_struct(v, NULL, "object", NULL, 0, );
 if (err) {
 goto out;
 }

 qdict_del(pdict, "qom-type");
-visit_type_str(opts_get_visitor(ov), , "qom-type", );
+visit_type_str(v, , "qom-type", );
 if (err) {
 goto out;
 }
 if (!type_predicate(type)) {
-goto out;
+goto out_end;
 }

 qdict_del(pdict, "id");
-visit_type_str(opts_get_visitor(ov), , "id", );
+visit_type_str(v, , "id", );
 if (err) {
-goto out;
+goto out_end;
 }

-object_add(type, id, pdict, opts_get_visitor(ov), );
-if (err) {
-goto out;
-}
-visit_end_struct(opts_get_visitor(ov), );
-if (err) {
-qmp_object_del(id, NULL);
+out_end:
+visit_end_struct(v, _end);
+if (!err && !err_end) {
+object_add(type, id, pdict, v, );
 }

 out:
@@ -2873,7 +2872,6 @@ out:
 QDECREF(pdict);
 g_free(id);
 g_free(type);
-g_free(dummy);
 if (err) {
 error_report_err(err);
 return -1;
-- 
2.4.3

[Qemu-devel] [PATCH v6 12/23] balloon: Improve use of qapi visitor

[Eric Blake]

Rework the control flow of balloon_stats_get_all() to make it
easier for a later patch to split visit_end_struct().  Also
switch to the uint64 visitor to match the data type.

Signed-off-by: Eric Blake 

---
v6: new patch, split from RFC on v5 7/46
---
 hw/virtio/virtio-balloon.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 9671635..1ce987a 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -130,10 +130,13 @@ static void balloon_stats_get_all(Object *obj, struct 
Visitor *v,
 if (err) {
 goto out_end;
 }
-for (i = 0; !err && i < VIRTIO_BALLOON_S_NR; i++) {
-visit_type_int64(v, (int64_t *) >stats[i], balloon_stat_names[i],
- );
+for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
+visit_type_uint64(v, >stats[i], balloon_stat_names[i], );
+if (err) {
+goto out_nested;
+}
 }
+out_nested:
 error_propagate(errp, err);
 err = NULL;
 visit_end_struct(v, );
-- 
2.4.3

[Qemu-devel] [PATCH v6 17/23] qapi: Eliminate empty visit_type_FOO_fields

[Eric Blake]

For empty structs, such as the 'Abort' helper type used as part
of the 'transaction' command, we were emitting a no-op
visit_type_FOO_fields().  Optimize things to instead omit calls
for empty structs.  Generated code changes resemble:

|-static void visit_type_Abort_fields(Visitor *v, Abort **obj, Error **errp)
|-{
|-Error *err = NULL;
|-
|-error_propagate(errp, err);
|-}
|-
| void visit_type_Abort(Visitor *v, Abort **obj, const char *name, Error **errp)
| {
| Error *err = NULL;
|@@ -112,7 +105,6 @@ void visit_type_Abort(Visitor *v, Abort
| if (!*obj) {
| goto out_obj;
| }
|-visit_type_Abort_fields(v, obj, );
| out_obj:
| error_propagate(errp, err);

Another reason for doing this optimization is that it gets us
closer to merging the code for visiting structs and unions:
since flat unions have no local members, they do not need to
have a visit_type_UNION_fields() emitted, even when they start
sharing the code used to visit structs.

Signed-off-by: Eric Blake 

---
v6: new patch
---
 scripts/qapi-visit.py | 53 ---
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 391bdfb..ed4fb20 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -35,22 +35,22 @@ void visit_type_%(c_name)s(Visitor *v, %(c_type)sobj, const 
char *name, Error **


 def gen_visit_fields_decl(typ):
-ret = ''
-if typ.name not in struct_fields_seen:
-ret += mcgen('''
+if typ.is_empty() or typ.name in struct_fields_seen:
+return ''
+
+struct_fields_seen.add(typ.name)
+return mcgen('''

 static void visit_type_%(c_type)s_fields(Visitor *v, %(c_type)s **obj, Error 
**errp);
 ''',
- c_type=typ.c_name())
-struct_fields_seen.add(typ.name)
-return ret
+ c_type=typ.c_name())


 def gen_visit_implicit_struct(typ):
-if typ in implicit_structs_seen:
+if typ.is_empty() or typ in implicit_structs_seen:
 return ''
+
 implicit_structs_seen.add(typ)
-
 ret = gen_visit_fields_decl(typ)

 ret += mcgen('''
@@ -74,7 +74,10 @@ static void visit_type_implicit_%(c_type)s(Visitor *v, 
%(c_type)s **obj, Error *
 def gen_visit_struct_fields(name, base, members):
 ret = ''

-if base:
+if (not base or base.is_empty()) and not members:
+return ret
+
+if base and not base.is_empty():
 ret += gen_visit_fields_decl(base)

 struct_fields_seen.add(name)
@@ -87,7 +90,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, 
%(c_name)s **obj, Error **e
 ''',
  c_name=c_name(name))

-if base:
+if base and not base.is_empty():
 ret += mcgen('''
 visit_type_%(c_type)s_fields(v, (%(c_type)s **)obj, );
 ''',
@@ -96,13 +99,9 @@ static void visit_type_%(c_name)s_fields(Visitor *v, 
%(c_name)s **obj, Error **e

 ret += gen_visit_fields(members, prefix='(*obj)->')

-# 'goto out' produced for base, and by gen_visit_fields() for each member
-if base or members:
-ret += mcgen('''
+ret += mcgen('''

 out:
-''')
-ret += mcgen('''
 error_propagate(errp, err);
 }
 ''')
@@ -129,16 +128,22 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error
 if (!*obj) {
 goto out_obj;
 }
-visit_type_%(c_name)s_fields(v, obj, );
-out_obj:
-error_propagate(errp, err);
-err = NULL;
-visit_end_struct(v, );
-out:
-error_propagate(errp, err);
-}
 ''',
  name=name, c_name=c_name(name))
+if (base and not base.is_empty()) or members:
+ret += mcgen('''
+visit_type_%(c_name)s_fields(v, obj, );
+''',
+ c_name=c_name(name))
+ret += mcgen('''
+out_obj:
+error_propagate(errp, err);
+err = NULL;
+visit_end_struct(v, );
+out:
+error_propagate(errp, err);
+}
+''')

 return ret

@@ -300,7 +305,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, 
const char *name, Error
 ''',
  c_type=simple_union_type.c_name(),
  c_name=c_name(var.name))
-else:
+elif not var.type.is_empty():
 ret += mcgen('''
 visit_type_implicit_%(c_type)s(v, &(*obj)->u.%(c_name)s, );
 ''',
-- 
2.4.3

[Qemu-devel] [PATCH v6 18/23] qapi: Canonicalize missing object to :empty

[Eric Blake]

Now that we elide unnecessary visits of empty types, we can
start using the special ':empty' type in more places.  By using
the empty type as the base class of every explicit struct or
union, and as the default data for any command or event, we can
simplify later logic in qapi-{visit,commands,event} by merely
checking whether the type is empty, without also having to worry
whether a type was even supplied.

Note that gen_object() in qapi-types still has to check for a
base, because it is also called for alternates (which have no
base).

No change to generated code.

Signed-off-by: Eric Blake 

---
v6: new patch
---
 scripts/qapi-commands.py|  7 ++---
 scripts/qapi-event.py   |  7 ++---
 scripts/qapi-types.py   |  4 +--
 scripts/qapi-visit.py   | 12 +
 scripts/qapi.py | 22 
 tests/qapi-schema/event-case.out|  2 +-
 tests/qapi-schema/flat-union-empty.out  |  1 +
 tests/qapi-schema/ident-with-escape.out |  1 +
 tests/qapi-schema/indented-expr.out |  4 +--
 tests/qapi-schema/qapi-schema-test.out  | 45 ++---
 tests/qapi-schema/union-clash-data.out  |  2 ++
 tests/qapi-schema/union-empty.out   |  1 +
 12 files changed, 78 insertions(+), 30 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 38cbffc..0f3cc57 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -65,7 +65,8 @@ def gen_marshal_vars(arg_type, ret_type):
 ''',
  c_type=ret_type.c_type())

-if arg_type and not arg_type.is_empty():
+assert arg_type
+if not arg_type.is_empty():
 ret += mcgen('''
 QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
 QapiDeallocVisitor *qdv;
@@ -97,7 +98,7 @@ def gen_marshal_vars(arg_type, ret_type):
 def gen_marshal_input_visit(arg_type, dealloc=False):
 ret = ''

-if not arg_type or arg_type.is_empty():
+if arg_type.is_empty():
 return ret

 if dealloc:
@@ -177,7 +178,7 @@ def gen_marshal(name, arg_type, ret_type):

 # 'goto out' produced by gen_marshal_input_visit->gen_visit_fields()
 # for each arg_type member, and by gen_call() for ret_type
-if (arg_type and not arg_type.is_empty()) or ret_type:
+if not arg_type.is_empty() or ret_type:
 ret += mcgen('''

 out:
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 5dc9726..7ee74a5 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -39,7 +39,8 @@ def gen_event_send(name, arg_type):
 ''',
 proto=gen_event_send_proto(name, arg_type))

-if arg_type and not arg_type.is_empty():
+assert arg_type
+if not arg_type.is_empty():
 ret += mcgen('''
 QObject *obj;
 QmpOutputVisitor *qov;
@@ -58,7 +59,7 @@ def gen_event_send(name, arg_type):
 ''',
  name=name)

-if arg_type and not arg_type.is_empty():
+if not arg_type.is_empty():
 c_name = 'NULL'
 if not arg_type.is_implicit():
 c_name = '"%s"' % arg_type.c_name()
@@ -91,7 +92,7 @@ out_obj:
 ''',
  c_enum=c_enum_const(event_enum_name, name))

-if arg_type and not arg_type.is_empty():
+if not arg_type.is_empty():
 ret += mcgen('''
 out:
 qmp_output_visitor_cleanup(qov);
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index af6b324..795a2bf 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -58,7 +58,7 @@ struct %(c_name)s {
 ''',
 c_name=c_name(name))

-if base:
+if base and not base.is_empty():
 ret += mcgen('''
 /* Members inherited from %(c_name)s: */
 ''',
@@ -226,7 +226,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
 def visit_object_type(self, name, info, base, members, variants):
 self._fwdecl += gen_fwd_object_or_array(name)
 self.decl += gen_object(name, base, members, variants)
-if base:
+if not base.is_implicit():
 self.decl += gen_upcast(name, base)
 self._gen_type_cleanup(name)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index ed4fb20..421a5b5 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -74,10 +74,11 @@ static void visit_type_implicit_%(c_type)s(Visitor *v, 
%(c_type)s **obj, Error *
 def gen_visit_struct_fields(name, base, members):
 ret = ''

-if (not base or base.is_empty()) and not members:
+assert base
+if base.is_empty() and not members:
 return ret

-if base and not base.is_empty():
+if not base.is_empty():
 ret += gen_visit_fields_decl(base)

 struct_fields_seen.add(name)
@@ -90,7 +91,7 @@ static void visit_type_%(c_name)s_fields(Visitor *v, 
%(c_name)s **obj, Error **e
 ''',
  c_name=c_name(name))

-if base and not base.is_empty():
+if not base.is_empty():
 ret += mcgen('''
 

[Qemu-devel] [PATCH v6 11/23] ppc: Improve use of qapi visitors

[Eric Blake]

The implementation of prop_get_fdt() is taking some shortcuts
with the qapi visitor functions.  Document them, and use
error_abort rather than NULL to ensure that any changes to
the visitors do not break our use of shortcuts.

Signed-off-by: Eric Blake 

---
v6: new patch, split from RFC on v5 7/46
---
 hw/ppc/spapr_drc.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 1846b4f..03a1879 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -276,21 +276,26 @@ static void prop_get_fdt(Object *obj, Visitor *v, void 
*opaque,
 case FDT_BEGIN_NODE:
 fdt_depth++;
 name = fdt_get_name(fdt, fdt_offset, _len);
-visit_start_struct(v, NULL, NULL, name, 0, NULL);
+visit_start_struct(v, NULL, "fdt", name, 0, _abort);
 break;
 case FDT_END_NODE:
 /* shouldn't ever see an FDT_END_NODE before FDT_BEGIN_NODE */
 g_assert(fdt_depth > 0);
-visit_end_struct(v, NULL);
+visit_end_struct(v, _abort);
 fdt_depth--;
 break;
 case FDT_PROP: {
 int i;
 prop = fdt_get_property_by_offset(fdt, fdt_offset, _len);
 name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
-visit_start_list(v, name, NULL);
+/* Note: since v is an output visitor, we can get away
+ * with just visiting a direct sequence of uint8 into the
+ * output array, instead of creating a uint8List and using
+ * visit_type_uint8List(). */
+visit_start_list(v, name, _abort);
 for (i = 0; i < prop_len; i++) {
-visit_type_uint8(v, (uint8_t *)>data[i], NULL, NULL);
+visit_type_uint8(v, (uint8_t *)>data[i], NULL,
+ _abort);

 }
 visit_end_list(v);
-- 
2.4.3

Re: [Qemu-devel] RFC: raspberry pi / pi2 / Windows-on-ARM support

[Andrew Baumann]

> From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Tuesday, 24 November 2015 22:04
> On Tue, Nov 24, 2015 at 4:00 PM, Andrew Baumann
>  wrote:
> > I am working on refactoring the Pi support code as you suggested. I have
> split the Pi SOCs into separate objects (bcm2835 and bcm2836) which both
> instantiate a third common bcm2835_peripherals device that in turn contains
> all the common devices. I have also switched the code to use object
> properties rather than global variables to communicate state where devices
> interact with each other.
> >
> 
> Should this be an inheritance of a common SoC rather than an
> instantiation of a common container?

I considered that, but it doesn't seem to buy much. To avoid lots of 
conditional code in the common soc, I would still need three classes: the 
common stuff, bcm2835 (which inherits from the common stuff and adds the 
arm1176 cpu), and bcm2836 (which also inherits the common stuff, but adds up to 
four a7 cores, and a new interrupt controller with a more complex routing). I'd 
have to overload realize in both of those subclasses. And aside from the issue 
of properties (which aliases seem to solve -- thanks!) the code would be about 
the same complexity overall, with the downside that the interface between the 
common stuff and the parent soc device is now much wider through inheritance of 
all its internal stuff (like the private IO bus) that the parent doesn't need 
to see.

Andrew

[Qemu-devel] [PATCH for 2.5 1/1] qga: added another non-interactive gspawn() helper file.

[Denis V. Lunev]

From: Yuri Pudgorodskiy 

With previous commit we added gspawn-win64-helper-console.exe,
required for gspawn() mingw implementation.
Unfortunatly when running as a service without interactive
desktop, gspawn() also requires another helper app.

Added gspawn-win64-helper.exe and gspawn-win32-helper.exe
for corresponding architectures.

Signed-off-by: Yuri Pudgorodskiy 
Signed-off-by: Denis V. Lunev 
CC: Michael Roth 
---
 qga/installer/qemu-ga.wxs | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index f25afdd..7c59972 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -95,11 +95,17 @@
   
 
   
+   
+
+  
   
   
   
 
   
+  
+
+  
   
   
 
@@ -159,6 +165,7 @@
   
   
   
+  
   
   
   
-- 
2.1.4