Re: [Qemu-devel] [PATCH] Use macro instead of plain text

[Cao jin]

Sorry, it can`t pass compilation...
I will give v2 soon.

On 12/18/2015 03:57 PM, Cao jin wrote:

There is TYPE_ICH9_AHCI definition in ahci.h when QOMify it, seems these two
places are missed.

Signed-off-by: Cao jin 
---
  hw/i386/pc_q35.c | 2 +-
  qdev-monitor.c   | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 9a12068..aa34a07 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -249,7 +249,7 @@ static void pc_q35_init(MachineState *machine)
  ahci = pci_create_simple_multifunction(host_bus,
 PCI_DEVFN(ICH9_SATA1_DEV,
   ICH9_SATA1_FUNC),
-   true, "ich9-ahci");
+   true, TYPE_ICH9_AHCI);
  idebus[0] = qdev_get_child_bus(>qdev, "ide.0");
  idebus[1] = qdev_get_child_bus(>qdev, "ide.1");
  g_assert(MAX_SATA_PORTS == ICH_AHCI(ahci)->ahci.ports);
diff --git a/qdev-monitor.c b/qdev-monitor.c
index a35098f..f249603 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -48,7 +48,7 @@ static const QDevAlias qdev_alias_table[] = {
  { "virtio-net-ccw", "virtio-net", QEMU_ARCH_S390X },
  { "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_S390X },
  { "lsi53c895a", "lsi" },
-{ "ich9-ahci", "ahci" },
+{ TYPE_ICH9_AHCI, "ahci" },
  { "kvm-pci-assign", "pci-assign" },
  { "e1000", "e1000-82540em" },
  { }



--
Yours Sincerely,

Cao Jin

Re: [Qemu-devel] [PATCH v2 04/23] error: Use error_report_err() instead of ad hoc prints

[Markus Armbruster]

Eric Blake  writes:

> On 12/17/2015 09:49 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster 
>> ---
>>  contrib/ivshmem-server/main.c | 4 +---
>>  qdev-monitor.c| 3 +--
>>  qemu-nbd.c| 3 +--
>>  3 files changed, 3 insertions(+), 7 deletions(-)
>> 
>> diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
>> index 54ff001..00508b5 100644
>> --- a/contrib/ivshmem-server/main.c
>> +++ b/contrib/ivshmem-server/main.c
>> @@ -106,9 +106,7 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int 
>> argc, char *argv[])
>>  case 'l': /* shm_size */
>>  parse_option_size("shm_size", optarg, >shm_size, );
>
> Idea for a followup patch: The name 'errp' is most often associated with
> type 'Error **'; but here it is 'Error *', which is confusing.

Yes.  Another round of these:

e940f54 qmp hmp: Consistently name Error * objects err, and not errp
2f719f1 hw: Consistently name Error * objects err, and not errp
4399c43 qemu-img: Consistently name Error * objects err, and not errp

> Other offenders in hmp.c, hw/core/nmi.c, include/qemu/sockets.h, and
> tests/test-string-output-visitor.c.
>
>>  if (errp) {
>> -fprintf(stderr, "cannot parse shm size: %s\n",
>> -error_get_pretty(errp));
>> -error_free(errp);
>> +error_report_err(errp);
>
> This loses the "cannot parse shm size: " prefix; but I don't think that
> hurts.  Could use a mention in the commit message, though.

Losing the prefix is fine, because the error messages always mention the
parameter name.  For instance,

cannot parse shm size: Parameter 'shm_size' expects a size

becomes

Parameter 'shm_size' expects a size
You may use k, M, G or T suffixes for kilobytes, megabytes, gigabytes and 
terabytes.

Note we now get the hint.

Including the error location would be even nicer, but is out of scope
for this series.

The program shows its complete usage help afterwards, which is annoying,
but out of scope for this series.

I'll amend the commit message.

> Reviewed-by: Eric Blake 

Thanks!

[Qemu-devel] [PATCH v2] Use macro instead of plain text

[Cao jin]

There is TYPE_ICH9_AHCI definition in ahci.h when QOMify it, seems it
is missed.

Signed-off-by: Cao jin 
---
 hw/i386/pc_q35.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 9a12068..aa34a07 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -249,7 +249,7 @@ static void pc_q35_init(MachineState *machine)
 ahci = pci_create_simple_multifunction(host_bus,
PCI_DEVFN(ICH9_SATA1_DEV,
  ICH9_SATA1_FUNC),
-   true, "ich9-ahci");
+   true, TYPE_ICH9_AHCI);
 idebus[0] = qdev_get_child_bus(>qdev, "ide.0");
 idebus[1] = qdev_get_child_bus(>qdev, "ide.1");
 g_assert(MAX_SATA_PORTS == ICH_AHCI(ahci)->ahci.ports);
-- 
2.1.0

[Qemu-devel] [PATCH] Convert to realize()

[Cao jin]

for educational PCI device

Signed-off-by: Cao jin 
---
 hw/misc/edu.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index fe50b42..43d5b18 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -327,7 +327,7 @@ static void *edu_fact_thread(void *opaque)
 return NULL;
 }
 
-static int pci_edu_init(PCIDevice *pdev)
+static void pci_edu_realize(PCIDevice *pdev, Error **errp)
 {
 EduState *edu = DO_UPCAST(EduState, pdev, pdev);
 uint8_t *pci_conf = pdev->config;
@@ -344,8 +344,6 @@ static int pci_edu_init(PCIDevice *pdev)
 memory_region_init_io(>mmio, OBJECT(edu), _mmio_ops, edu,
 "edu-mmio", 1 << 20);
 pci_register_bar(pdev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, >mmio);
-
-return 0;
 }
 
 static void pci_edu_uninit(PCIDevice *pdev)
@@ -385,7 +383,7 @@ static void edu_class_init(ObjectClass *class, void *data)
 {
 PCIDeviceClass *k = PCI_DEVICE_CLASS(class);
 
-k->init = pci_edu_init;
+k->realize = pci_edu_realize;
 k->exit = pci_edu_uninit;
 k->vendor_id = PCI_VENDOR_ID_QEMU;
 k->device_id = 0x11e8;
-- 
2.1.0

[Qemu-devel] [PATCH] hw/ppc/spapr_rtc: Remove bad class_size value

[Thomas Huth]

class_size = sizeof(XICSStateClass) does not make much sense
in the RTC code and likely was just a copy-n-paste error.
Let's simply remove it.

Signed-off-by: Thomas Huth 
---
 hw/ppc/spapr_rtc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/ppc/spapr_rtc.c b/hw/ppc/spapr_rtc.c
index 34b27db..b591a8e 100644
--- a/hw/ppc/spapr_rtc.c
+++ b/hw/ppc/spapr_rtc.c
@@ -200,7 +200,6 @@ static const TypeInfo spapr_rtc_info = {
 .name  = TYPE_SPAPR_RTC,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(sPAPRRTCState),
-.class_size = sizeof(XICSStateClass),
 .class_init= spapr_rtc_class_init,
 };
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/2] compat: Introduce HW_COMPAT_2_5

[Cornelia Huck]

On Fri, 18 Dec 2015 09:30:02 +0200
Shmulik Ladkani  wrote:

> Introduce the place-holder for 2.5 back-compat properties, and the
> accompanying PC_COMPAT_2_5, CCW_COMPAT_2_5, SPAPR_COMPAT_2_5.
> 
> Signed-off-by: Shmulik Ladkani 
> ---
>  hw/i386/pc_piix.c  | 1 +
>  hw/i386/pc_q35.c   | 1 +
>  hw/ppc/spapr.c | 9 +
>  hw/s390x/s390-virtio-ccw.c | 9 +
>  include/hw/compat.h| 3 +++
>  include/hw/i386/pc.h   | 4 
>  6 files changed, 27 insertions(+)
> 

> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 5a52ff2..3d79654 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -235,7 +235,11 @@ static const TypeInfo ccw_machine_info = {
>  },
>  };
> 
> +#define CCW_COMPAT_2_5 \
> +HW_COMPAT_2_5
> +
>  #define CCW_COMPAT_2_4 \
> +CCW_COMPAT_2_5 \
>  HW_COMPAT_2_4 \
>  {\
>  .driver   = TYPE_S390_SKEYS,\
> @@ -296,10 +300,15 @@ static const TypeInfo ccw_machine_2_4_info = {
>  static void ccw_machine_2_5_class_init(ObjectClass *oc, void *data)
>  {
>  MachineClass *mc = MACHINE_CLASS(oc);
> +static GlobalProperty compat_props[] = {
> +CCW_COMPAT_2_5
> +{ /* end of list */ }
> +};
> 
>  mc->alias = "s390-ccw-virtio";
>  mc->desc = "VirtIO-ccw based S390 machine v2.5";
>  mc->is_default = 1;
> +mc->compat_props = compat_props;
>  }
> 
>  static const TypeInfo ccw_machine_2_5_info = {

s390x part:

Acked-by: Cornelia Huck 

Re: [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions

[Gerd Hoffmann]

On Do, 2015-12-17 at 15:27 -0200, Eduardo Habkost wrote:
> Migration with q35 was not possible before commit
> 04329029a8c539eb5f75dcb6d8b016f0c53a031a, because q35 unconditionally
> creates an ich9-ahci device, that was marked as unmigratable. So all q35
> machine classes before pc-q35-2.4 were not migratable, so there's no
> point in keeping compatibility code for them.
> 
> Remove all old pc-q35 machine classes and keep only pc-q35-2.4.

> -static void pc_compat_1_6(MachineState *machine)
> -{
> -pc_compat_1_7(machine);
> -rom_file_has_mr = false;
> -has_acpi_build = false;

After applying this patch has_acpi_build is always true and can be
dropped, together with some other code elsewhere which depends on
has_acpi_build.  The same is probably true for other variables as well
(gigabyte_align?).  Guess it's best to do this as one-per-variable
incremental patches.

Reviewed-by: Gerd Hoffmann 

Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl

[Laurent Vivier]

Le 18/12/2015 07:51, Chen Gang a écrit :
> 
> I found this issue during my working time, it is about sw_64 (almost the
> same as alpha) host running i386 wine programs.
> 
> I also found another issue, but I am not quite sure whether it is worth
> enough for our upstream: The related fix patch is below, which will let
> the initialization slower, but for most archs, they have no this issue.
> 
> linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in target_mmap()
> 
> In some architectures, they have no policy to zero MAP_ANONYMOUS memory,
> which will cause issue for qemu target_mmap.
> 
> Signed-off-by: Chen Gang 
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index 7b459d5..9c9152d 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>  printf("\n");
>  #endif
>  tb_invalidate_phys_range(start, start + len);
> +if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS)
> +&& ((flags & MAP_PRIVATE) || (fd == -1))) {
> +memset(g2h(start), 0, len);
> +}

IMHO, their kernel needs a fix, mmap(2):

MAP_ANONYMOUS
The mapping is not backed by any file; its contents are initial‐
ized to zero.


>  mmap_unlock();
>  return start;
>  fail:
> 
> 
> Thanks.

Laurent

Re: [Qemu-devel] [PATCH v2 16/23] error: Strip trailing '\n' from error string arguments (again)

[Markus Armbruster]

Eric Blake  writes:

> On 12/17/2015 09:49 AM, Markus Armbruster wrote:
>> Commit 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but they
>> keep coming back.  Tracked down with the Coccinelle semantic patch
>> from commit 312fd5f.
>
> Don't forget to rerun this to pick up stragglers exposed by 1/23 :)

Just two:

qemu-nbd.c:574:76:"Shared device number must be greater than 0\n"
qemu-nbd.c:557:61:"socket path must be absolute\n"

>> Signed-off-by: Markus Armbruster 
>> Reviewed-by: Dr. David Alan Gilbert 
>> Acked-by: Cornelia Huck 
>> Acked-by: Bharata B Rao 
>> Acked-by: Fam Zheng 
>
> If you want to add to the list:
> Reviewed-by: Eric Blake 
>
>> +++ b/hw/s390x/s390-skeys.c
>> @@ -191,8 +191,8 @@ static int qemu_s390_skeys_set(S390SKeysState *ss, 
>> uint64_t start_gfn,
>>  /* Check for uint64 overflow and access beyond end of key data */
>>  if (start_gfn + count > skeydev->key_count || start_gfn + count < 
>> count) {
>>  error_report("Error: Setting storage keys for page beyond the end "
>> -"of memory: gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
>> -count);
>> + "of memory: gfn=%" PRIx64 " count=%" PRId64,
>> + start_gfn, count);
>
> Do we want a separate patch cleaning up 'Error: ' prefixes?

After my Christmas break, in a separate series probably.

Re: [Qemu-devel] [PATCH v2 17/23] vmdk: Clean up control flow in vmdk_parse_extents() a bit

[Markus Armbruster]

Eric Blake  writes:

> On 12/17/2015 09:49 AM, Markus Armbruster wrote:
>> Cc: Fam Zheng 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  block/vmdk.c | 28 +++-
>>  1 file changed, 15 insertions(+), 13 deletions(-)
>
> Could have mentioned what the change was: factoring out a common
> next_line() helper to let you drop an end-of-loop label.

Adding:

Factor out loop stepping to turn a while-loop with goto into a
for-loop with continue.

> Reviewed-by: Eric Blake 

Thanks!

Re: [Qemu-devel] [PATCH v2 18/23] vmdk: Clean up "Invalid extent lines" error message

[Markus Armbruster]

Fam Zheng  writes:

> On Thu, 12/17 17:49, Markus Armbruster wrote:
>> vmdk_parse_extents() reports parse errors like this:
>> 
>> error_setg(errp, "Invalid extent lines:\n%s", p);
>> 
>> where p points to the beginning of the malformed line in the image
>> descriptor.  This results in a multi-line error message
>> 
>> Invalid extent lines:
>> 
>> 
>> 
>> Error messages should not have newlines embedded.  Since the remaining
>> text is not helpful, we can simply report:
>> 
>> Invalid extent line: 
>> 
>> Cc: Fam Zheng 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  block/vmdk.c   | 19 ---
>>  tests/qemu-iotests/059.out |  4 +---
>>  2 files changed, 13 insertions(+), 10 deletions(-)
>> 
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index 08fa3f3..04b47e3 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -780,7 +780,7 @@ static int vmdk_parse_extents(const char *desc, 
>> BlockDriverState *bs,
>>  char access[11];
>>  char type[11];
>>  char fname[512];
>> -const char *p;
>> +const char *p, *np;
>>  int64_t sectors = 0;
>>  int64_t flat_offset;
>>  char *extent_path;
>> @@ -805,19 +805,16 @@ static int vmdk_parse_extents(const char *desc, 
>> BlockDriverState *bs,
>>  continue;
>>  } else if (!strcmp(type, "FLAT")) {
>>  if (matches != 5 || flat_offset < 0) {
>> -error_setg(errp, "Invalid extent lines: \n%s", p);
>> -return -EINVAL;
>> +goto invalid;
>>  }
>>  } else if (!strcmp(type, "VMFS")) {
>>  if (matches == 4) {
>>  flat_offset = 0;
>>  } else {
>> -error_setg(errp, "Invalid extent lines:\n%s", p);
>> -return -EINVAL;
>> +goto invalid;
>>  }
>>  } else if (matches != 4) {
>> -error_setg(errp, "Invalid extent lines:\n%s", p);
>> -return -EINVAL;
>> +goto invalid;
>>  }
>>  
>>  if (sectors <= 0 ||
>> @@ -883,6 +880,14 @@ static int vmdk_parse_extents(const char *desc, 
>> BlockDriverState *bs,
>>  extent->type = g_strdup(type);
>>  }
>>  return 0;
>> +
>> +invalid:
>> +np = next_line(p);
>> +assert(np != p);
>> +if (np[-1] == '\n')
>> +np--;
>
> Braces are required.

Fixing...

>> +error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p);
>> +return -EINVAL;
>>  }
>>  
>>  static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
>> diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
>> index d28df5b..9d506cb 100644
>> --- a/tests/qemu-iotests/059.out
>> +++ b/tests/qemu-iotests/059.out
>> @@ -2038,9 +2038,7 @@ Format specific information:
>>  format: FLAT
>>  
>>  === Testing malformed VMFS extent description line ===
>> -qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent lines:
>> -RW 12582912 VMFS "dummy.IMGFMT" 1
>> -
>> +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent line: RW 
>> 12582912 VMFS "dummy.IMGFMT" 1
>>  
>>  === Testing truncated sparse ===
>>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400 
>> subformat=monolithicSparse
>> -- 
>> 2.4.3
>> 

Re: [Qemu-devel] [PATCH v2 21/23] error: Clean up errors with embedded newlines (again)

[Markus Armbruster]

Eric Blake  writes:

> On 12/17/2015 09:50 AM, Markus Armbruster wrote:
>> The arguments of error_report() should yield a short error string
>> without newlines.
>> 
>> A few places try to print additional help after the error message by
>> embedding newlines in the error string.  That's nice, but let's do it
>> the right way.  Commit 474c213 cleaned up some, but they keep coming
>> back.  Offenders tracked down with the Coccinelle semantic patch from
>> commit 312fd5f.
>
> Not sure if you'll have any stragglers from 1/23 here, after fixing 16/23.

Just one:
qemu-nbd.c:604:28:"Invalid number of argument.\n" "Try `%s --help' for more 
information."

>> 
>> Cc: Laszlo Ersek 
>> Cc: Pavel Fedin 
>> Signed-off-by: Markus Armbruster 
>> Reviewed-by: Laszlo Ersek 
>> ---
>>  hw/i386/pc.c | 4 ++--
>>  kvm-all.c| 6 +++---
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>> 
>
> Reviewed-by: Eric Blake 

Thanks!

[Qemu-devel] [PATCH] misc: spelling

[marcandre . lureau]

From: Marc-André Lureau 

---
 monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index e7e7ae2..51ec4c3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -311,7 +311,7 @@ static void monitor_flush_locked(Monitor *mon)
 return;
 }
 if (rc > 0) {
-/* partinal write */
+/* partial write */
 QString *tmp = qstring_from_str(buf + rc);
 QDECREF(mon->outbuf);
 mon->outbuf = tmp;
-- 
2.5.0

[Qemu-devel] [PATCH 2/7] target-ppc: rename and export maybe_bswap_register()

[Greg Kurz]

This helper will be used to support FP, Altivec and VSX registers when
the guest is little-endian.

Signed-off-by: Greg Kurz 
---
 target-ppc/cpu.h |1 +
 target-ppc/gdbstub.c |   10 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 9706000f8bf1..1e2516e071b7 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -2355,4 +2355,5 @@ int ppc_get_vcpu_dt_id(PowerPCCPU *cpu);
  */
 PowerPCCPU *ppc_get_vcpu_by_dt_id(int cpu_dt_id);
 
+void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len);
 #endif /* !defined (__CPU_PPC_H__) */
diff --git a/target-ppc/gdbstub.c b/target-ppc/gdbstub.c
index 14675f45653d..b20bb0c80ef3 100644
--- a/target-ppc/gdbstub.c
+++ b/target-ppc/gdbstub.c
@@ -88,7 +88,7 @@ static int ppc_gdb_register_len(int n)
the proper ordering for the binary, and cannot be changed.
For system mode, TARGET_WORDS_BIGENDIAN is always set, and we must check
the current mode of the chip to see if we're running in little-endian.  */
-static void maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len)
+void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len)
 {
 #ifndef CONFIG_USER_ONLY
 if (!msr_le) {
@@ -158,7 +158,7 @@ int ppc_cpu_gdb_read_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 break;
 }
 }
-maybe_bswap_register(env, mem_buf, r);
+ppc_maybe_bswap_register(env, mem_buf, r);
 return r;
 }
 
@@ -214,7 +214,7 @@ int ppc_cpu_gdb_read_register_apple(CPUState *cs, uint8_t 
*mem_buf, int n)
 break;
 }
 }
-maybe_bswap_register(env, mem_buf, r);
+ppc_maybe_bswap_register(env, mem_buf, r);
 return r;
 }
 
@@ -227,7 +227,7 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 if (!r) {
 return r;
 }
-maybe_bswap_register(env, mem_buf, r);
+ppc_maybe_bswap_register(env, mem_buf, r);
 if (n < 32) {
 /* gprs */
 env->gpr[n] = ldtul_p(mem_buf);
@@ -277,7 +277,7 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cs, uint8_t 
*mem_buf, int n)
 if (!r) {
 return r;
 }
-maybe_bswap_register(env, mem_buf, r);
+ppc_maybe_bswap_register(env, mem_buf, r);
 if (n < 32) {
 /* gprs */
 env->gpr[n] = ldq_p(mem_buf);

[Qemu-devel] [PATCH 0/7] target-ppc: endian fixes for KVM and gdbstub

[Greg Kurz]

Hi,

This series is a sequel to Anton's tentative at bringing VSX support in
our gdbstub:

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

Indeed, FP, SPE and Altivec registers need to be copied to memory with
the appropriate ordering, like we already do for core registers. This
series reuses the maybe_bswap_register() helper to do the job, since
it already handles the user mode case where the target endianness is
known at build time and don't need byteswap. This is covered by
patches 2 to 6.

I also found more serious issues that probably break more than gdbstub.

First one is a bug in KVM, that completely breaks the KVM_GET_ONE_REG
and KVM_SET_ONE_REG ioctls for Altivec registers. I've already sent a
patch:

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

Second one is a bug in QEMU that breaks synchronisation with KVM for FP,
Altivec and VSX registers on little-endian hosts. I pushed the fix to
patch 1 since it is needed for the gdbstub fixes to actually work, but it
could even be handled separately.

And finally, patch 7 is Anton's + the byteswapping for little-endian
guests.

Cheers.

---

Anton Blanchard (1):
  target-ppc: gdbstub: Add VSX support

Greg Kurz (6):
  target-ppc: kvm: fix floating point registers sync on little-endian hosts
  target-ppc: rename and export maybe_bswap_register()
  target-ppc: gdbstub: fix float registers for little-endian guests
  target-ppc: gdbstub: introduce avr_need_swap()
  target-ppc: gdbstub: fix altivec registers for little-endian guests
  target-ppc: gdbstub: fix spe registers for little-endian guests


 configure   |6 ++-
 gdb-xml/power-vsx.xml   |   44 +++
 target-ppc/cpu.h|1 +
 target-ppc/gdbstub.c|   10 +++--
 target-ppc/kvm.c|   12 ++
 target-ppc/translate_init.c |   84 +++
 6 files changed, 134 insertions(+), 23 deletions(-)
 create mode 100644 gdb-xml/power-vsx.xml

[Qemu-devel] [PATCH 3/7] target-ppc: gdbstub: fix float registers for little-endian guests

[Greg Kurz]

Let's reuse the ppc_maybe_bswap_register() helper, like we already do
with the general registers.

Signed-off-by: Greg Kurz 
---
 target-ppc/translate_init.c |4 
 1 file changed, 4 insertions(+)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index e88dc7fc7aa3..d31d7f6e9bd8 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8755,10 +8755,12 @@ static int gdb_get_float_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 {
 if (n < 32) {
 stfq_p(mem_buf, env->fpr[n]);
+ppc_maybe_bswap_register(env, mem_buf, 8);
 return 8;
 }
 if (n == 32) {
 stl_p(mem_buf, env->fpscr);
+ppc_maybe_bswap_register(env, mem_buf, 4);
 return 4;
 }
 return 0;
@@ -8767,10 +8769,12 @@ static int gdb_get_float_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 static int gdb_set_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
 {
 if (n < 32) {
+ppc_maybe_bswap_register(env, mem_buf, 8);
 env->fpr[n] = ldfq_p(mem_buf);
 return 8;
 }
 if (n == 32) {
+ppc_maybe_bswap_register(env, mem_buf, 4);
 helper_store_fpscr(env, ldl_p(mem_buf), 0x);
 return 4;
 }

[Qemu-devel] [PATCH 1/7] target-ppc: kvm: fix floating point registers sync on little-endian hosts

[Greg Kurz]

On VSX capable CPUs, the 32 FP registers are mapped to the high-bits
of the 32 first VSX registers. So if you have:

VSR31 = (uint128) 0x0102030405060708090a0b0c0d0e0f00

then

FPR31 = (uint64) 0x0102030405060708

The kernel stores the VSX registers in the fp_state struct following the
host endian element ordering.

On big-endian:

fp_state.fpr[31][0] = 0x0102030405060708
fp_state.fpr[31][1] = 0x090a0b0c0d0e0f00

On little-endian:

fp_state.fpr[31][0] = 0x090a0b0c0d0e0f00
fp_state.fpr[31][1] = 0x0102030405060708

The KVM_GET_ONE_REG and KVM_SET_ONE_REG ioctls preserve this ordering, but
QEMU considers it as big-endian and always copies element [0] to the
fpr[] array and element [1] to the vsr[] array. This does not work with
little-endian hosts, and you will get:

(qemu) p $f31
0x90a0b0c0d0e0f00

instead of:

(qemu) p $f31
0x102030405060708

This patch fixes the element ordering for little-endian hosts.

Signed-off-by: Greg Kurz 
---
 target-ppc/kvm.c |   12 
 1 file changed, 12 insertions(+)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index ac70f0897b35..acd327538ce2 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -650,8 +650,13 @@ static int kvm_put_fp(CPUState *cs)
 for (i = 0; i < 32; i++) {
 uint64_t vsr[2];
 
+#ifdef HOST_WORDS_BIGENDIAN
 vsr[0] = float64_val(env->fpr[i]);
 vsr[1] = env->vsr[i];
+#else
+vsr[0] = env->vsr[i];
+vsr[1] = float64_val(env->fpr[i]);
+#endif
 reg.addr = (uintptr_t) 
 reg.id = vsx ? KVM_REG_PPC_VSR(i) : KVM_REG_PPC_FPR(i);
 
@@ -721,10 +726,17 @@ static int kvm_get_fp(CPUState *cs)
 vsx ? "VSR" : "FPR", i, strerror(errno));
 return ret;
 } else {
+#ifdef HOST_WORDS_BIGENDIAN
 env->fpr[i] = vsr[0];
 if (vsx) {
 env->vsr[i] = vsr[1];
 }
+#else
+env->fpr[i] = vsr[1];
+if (vsx) {
+env->vsr[i] = vsr[0];
+}
+#endif
 }
 }
 }

[Qemu-devel] [PATCH 4/7] target-ppc: gdbstub: introduce avr_need_swap()

[Greg Kurz]

This helper will be used to support Altivec registers in little-endian guests.
This patch does not change functionnality.

Note: I had to put the helper some lines away from the gdb_*_avr_reg()
routines to get a more readable patch.

Signed-off-by: Greg Kurz 
---
 target-ppc/translate_init.c |   37 +++--
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index d31d7f6e9bd8..18e9e561561f 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8751,6 +8751,15 @@ static void dump_ppc_insns (CPUPPCState *env)
 }
 #endif
 
+static bool avr_need_swap(CPUPPCState *env)
+{
+#ifdef HOST_WORDS_BIGENDIAN
+return false;
+#else
+return true;
+#endif
+}
+
 static int gdb_get_float_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
 {
 if (n < 32) {
@@ -8784,13 +8793,13 @@ static int gdb_set_float_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 static int gdb_get_avr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
 {
 if (n < 32) {
-#ifdef HOST_WORDS_BIGENDIAN
-stq_p(mem_buf, env->avr[n].u64[0]);
-stq_p(mem_buf+8, env->avr[n].u64[1]);
-#else
-stq_p(mem_buf, env->avr[n].u64[1]);
-stq_p(mem_buf+8, env->avr[n].u64[0]);
-#endif
+if (!avr_need_swap(env)) {
+stq_p(mem_buf, env->avr[n].u64[0]);
+stq_p(mem_buf+8, env->avr[n].u64[1]);
+} else {
+stq_p(mem_buf, env->avr[n].u64[1]);
+stq_p(mem_buf+8, env->avr[n].u64[0]);
+}
 return 16;
 }
 if (n == 32) {
@@ -8807,13 +8816,13 @@ static int gdb_get_avr_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 static int gdb_set_avr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
 {
 if (n < 32) {
-#ifdef HOST_WORDS_BIGENDIAN
-env->avr[n].u64[0] = ldq_p(mem_buf);
-env->avr[n].u64[1] = ldq_p(mem_buf+8);
-#else
-env->avr[n].u64[1] = ldq_p(mem_buf);
-env->avr[n].u64[0] = ldq_p(mem_buf+8);
-#endif
+if (!avr_need_swap(env)) {
+env->avr[n].u64[0] = ldq_p(mem_buf);
+env->avr[n].u64[1] = ldq_p(mem_buf+8);
+} else {
+env->avr[n].u64[1] = ldq_p(mem_buf);
+env->avr[n].u64[0] = ldq_p(mem_buf+8);
+}
 return 16;
 }
 if (n == 32) {

[Qemu-devel] [PATCH 6/7] target-ppc: gdbstub: fix spe registers for little-endian guests

[Greg Kurz]

Let's reuse the ppc_maybe_bswap_register() helper, like we already do
with the general registers.

Signed-off-by: Greg Kurz 
---
 target-ppc/translate_init.c |   11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 80d53e4dcf5a..5ea168c19eae 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8849,6 +8849,7 @@ static int gdb_get_spe_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 if (n < 32) {
 #if defined(TARGET_PPC64)
 stl_p(mem_buf, env->gpr[n] >> 32);
+ppc_maybe_bswap_register(env, mem_buf, 4);
 #else
 stl_p(mem_buf, env->gprh[n]);
 #endif
@@ -8856,10 +8857,12 @@ static int gdb_get_spe_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 }
 if (n == 32) {
 stq_p(mem_buf, env->spe_acc);
+ppc_maybe_bswap_register(env, mem_buf, 8);
 return 8;
 }
 if (n == 33) {
 stl_p(mem_buf, env->spe_fscr);
+ppc_maybe_bswap_register(env, mem_buf, 4);
 return 4;
 }
 return 0;
@@ -8870,7 +8873,11 @@ static int gdb_set_spe_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 if (n < 32) {
 #if defined(TARGET_PPC64)
 target_ulong lo = (uint32_t)env->gpr[n];
-target_ulong hi = (target_ulong)ldl_p(mem_buf) << 32;
+target_ulong hi;
+
+ppc_maybe_bswap_register(env, mem_buf, 4);
+
+hi = (target_ulong)ldl_p(mem_buf) << 32;
 env->gpr[n] = lo | hi;
 #else
 env->gprh[n] = ldl_p(mem_buf);
@@ -8878,10 +8885,12 @@ static int gdb_set_spe_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 return 4;
 }
 if (n == 32) {
+ppc_maybe_bswap_register(env, mem_buf, 8);
 env->spe_acc = ldq_p(mem_buf);
 return 8;
 }
 if (n == 33) {
+ppc_maybe_bswap_register(env, mem_buf, 4);
 env->spe_fscr = ldl_p(mem_buf);
 return 4;
 }

[Qemu-devel] [PATCH 5/7] target-ppc: gdbstub: fix altivec registers for little-endian guests

[Greg Kurz]

Altivec registers are 128-bit wide. They are stored in memory as two
64-bit values that must be byteswapped when the guest is little-endian.
Let's reuse the ppc_maybe_bswap_register() helper for this.

We also need to fix the ordering of the 64-bit elements according to
the target endianness, for both system and user mode.

Signed-off-by: Greg Kurz 
---
 target-ppc/translate_init.c |   12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 18e9e561561f..80d53e4dcf5a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8754,9 +8754,9 @@ static void dump_ppc_insns (CPUPPCState *env)
 static bool avr_need_swap(CPUPPCState *env)
 {
 #ifdef HOST_WORDS_BIGENDIAN
-return false;
+return msr_le;
 #else
-return true;
+return !msr_le;
 #endif
 }
 
@@ -8800,14 +8800,18 @@ static int gdb_get_avr_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 stq_p(mem_buf, env->avr[n].u64[1]);
 stq_p(mem_buf+8, env->avr[n].u64[0]);
 }
+ppc_maybe_bswap_register(env, mem_buf, 8);
+ppc_maybe_bswap_register(env, mem_buf + 8, 8);
 return 16;
 }
 if (n == 32) {
 stl_p(mem_buf, env->vscr);
+ppc_maybe_bswap_register(env, mem_buf, 4);
 return 4;
 }
 if (n == 33) {
 stl_p(mem_buf, (uint32_t)env->spr[SPR_VRSAVE]);
+ppc_maybe_bswap_register(env, mem_buf, 4);
 return 4;
 }
 return 0;
@@ -8816,6 +8820,8 @@ static int gdb_get_avr_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 static int gdb_set_avr_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
 {
 if (n < 32) {
+ppc_maybe_bswap_register(env, mem_buf, 8);
+ppc_maybe_bswap_register(env, mem_buf + 8, 8);
 if (!avr_need_swap(env)) {
 env->avr[n].u64[0] = ldq_p(mem_buf);
 env->avr[n].u64[1] = ldq_p(mem_buf+8);
@@ -8826,10 +8832,12 @@ static int gdb_set_avr_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 return 16;
 }
 if (n == 32) {
+ppc_maybe_bswap_register(env, mem_buf, 4);
 env->vscr = ldl_p(mem_buf);
 return 4;
 }
 if (n == 33) {
+ppc_maybe_bswap_register(env, mem_buf, 4);
 env->spr[SPR_VRSAVE] = (target_ulong)ldl_p(mem_buf);
 return 4;
 }

[Qemu-devel] [PATCH 7/7] target-ppc: gdbstub: Add VSX support

[Greg Kurz]

From: Anton Blanchard 

Add the XML and functions to get and set VSX registers.

Signed-off-by: Anton Blanchard 
(fixed little-endian guests)
Signed-off-by: Greg Kurz 
---
 configure   |6 +++---
 gdb-xml/power-vsx.xml   |   44 +++
 target-ppc/translate_init.c |   24 +++
 3 files changed, 71 insertions(+), 3 deletions(-)
 create mode 100644 gdb-xml/power-vsx.xml

diff --git a/configure b/configure
index b9552fda6f7f..78111801752f 100755
--- a/configure
+++ b/configure
@@ -5617,20 +5617,20 @@ case "$target_name" in
   ppc64)
 TARGET_BASE_ARCH=ppc
 TARGET_ABI_DIR=ppc
-gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml"
+gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml power-vsx.xml"
   ;;
   ppc64le)
 TARGET_ARCH=ppc64
 TARGET_BASE_ARCH=ppc
 TARGET_ABI_DIR=ppc
-gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml"
+gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml power-vsx.xml"
   ;;
   ppc64abi32)
 TARGET_ARCH=ppc64
 TARGET_BASE_ARCH=ppc
 TARGET_ABI_DIR=ppc
 echo "TARGET_ABI32=y" >> $config_target_mak
-gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml"
+gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml power-vsx.xml"
   ;;
   sh4|sh4eb)
 TARGET_ARCH=sh4
diff --git a/gdb-xml/power-vsx.xml b/gdb-xml/power-vsx.xml
new file mode 100644
index ..fd290e970b41
--- /dev/null
+++ b/gdb-xml/power-vsx.xml
@@ -0,0 +1,44 @@
+
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 5ea168c19eae..8069f3c27956 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8897,6 +8897,26 @@ static int gdb_set_spe_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 return 0;
 }
 
+static int gdb_get_vsx_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+{
+if (n < 32) {
+stq_p(mem_buf, env->vsr[n]);
+ppc_maybe_bswap_register(env, mem_buf, 8);
+return 8;
+}
+return 0;
+}
+
+static int gdb_set_vsx_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+{
+if (n < 32) {
+ppc_maybe_bswap_register(env, mem_buf, 8);
+env->vsr[n] = ldq_p(mem_buf);
+return 8;
+}
+return 0;
+}
+
 static int ppc_fixup_cpu(PowerPCCPU *cpu)
 {
 CPUPPCState *env = >env;
@@ -9002,6 +9022,10 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 gdb_register_coprocessor(cs, gdb_get_spe_reg, gdb_set_spe_reg,
  34, "power-spe.xml", 0);
 }
+if (pcc->insns_flags2 & PPC2_VSX) {
+gdb_register_coprocessor(cs, gdb_get_vsx_reg, gdb_set_vsx_reg,
+ 32, "power-vsx.xml", 0);
+}
 
 qemu_init_vcpu(cs);
 

Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint()

[Markus Armbruster]

Eric Blake  writes:

> On 12/17/2015 12:00 PM, Markus Armbruster wrote:
>
>> @@ -128,6 +138,7 @@ ErrorClass error_get_class(const Error *err);
>>   * If @errp is anything else, *@errp must be NULL.
>>   * The new error's class is ERROR_CLASS_GENERIC_ERROR, and its
>>   * human-readable error message is made from printf-style @fmt, ...
>> + * The resulting message should not contain newlines.
>
> Should we also discourage trailing punctuation?

 Yes.  How to best phrase it?
>>>
>>> Maybe:
>>>
>>> The resulting message should be a single phrase, with no newline or
>>> trailing punctuation.
>> 
>> What about ending the message with an exclamation mark?
>
> Very few current users do that! An exclamation mark is still trailing
> punctuation! And I don't like shouting at users!
>
> :)

Okay, okay, I applied it %-)

Re: [Qemu-devel] [PATCH v2 09/23] error: New error_prepend(), error_reportf_err()

[Markus Armbruster]

Eric Blake  writes:

> On 12/17/2015 09:49 AM, Markus Armbruster wrote:
>> Instead of simply propagating an error verbatim, we sometimes want to
>> add to its message, like this:
>> 
>> frobnicate(arg, );
>> error_setg(errp, "Can't frobnicate %s: %s",
>>   arg, error_get_pretty(err));
>
> Did you intend to have literal TABs in the commit message?

Editing accident, fixed.

>> error_free(err);
>> 
>> This is suboptimal, because it loses err's hint (if any).  Moreover,
>> when errp is _abort or is subsequently propagated to
>> _abort, the abort message points to the place where we last
>> added to the error, not to the place where it originated.
>> 
>> To avoid these issues, provide means to add to an error's message in
>> place:
>> 
>> frobnicate(arg, errp);
>> error_prepend(errp, "Can't frobnicate %s: ", arg);
>> 
>> Likewise, reporting an error like
>> 
>> frobnicate(arg, );
>> error_report("Can't frobnicate %s: %s", arg, error_get_pretty(err));
>> 
>> can lose err's hint.  To avoid:
>> 
>> error_reportf_err(err, "Can't frobnicate %s: ", arg);
>> 
>> The next commits will put these functions to use.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  include/qapi/error.h | 31 +--
>>  util/error.c | 33 +
>>  2 files changed, 62 insertions(+), 2 deletions(-)
>> 
>
> Reviewed-by: Eric Blake 

Thanks!

Re: [Qemu-devel] [PATCH v2 11/23] error: Use error_reportf_err() where it makes obvious sense

[Markus Armbruster]

Eric Blake  writes:

> On 12/17/2015 09:49 AM, Markus Armbruster wrote:
>> Done with this Coccinelle semantic patch
>> 
>> @@
>> expression FMT, E;
>> expression list ARGS;
>> @@
>> -error_report(FMT, ARGS, error_get_pretty(E));
>> +error_reportf_err(E, FMT/*@@@*/, ARGS);
>> (
>> -error_free(E);
>> |
>>   exit(S);
>
> Does S have to be declared an expression, for this branch to work
> correctly?  I guess not, since...

Pasto, fixed.

>> |
>>   abort();
>> )
>> 
>> followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping,
>> because I can't figure out how to make Coccinelle transform strings.
>>
>
> Hey - you can already make it do more than I can.
>
>
>> Signed-off-by: Markus Armbruster 
>> ---
>> +++ b/arch_init.c
>> @@ -258,9 +258,7 @@ void do_acpitable_option(const QemuOpts *opts)
>>  
>>  acpi_table_add(opts, );
>>  if (err) {
>> -error_report("Wrong acpi table provided: %s",
>> - error_get_pretty(err));
>> -error_free(err);
>> +error_reportf_err(err, "Wrong acpi table provided: ");
>>  exit(1);
>
> ...you properly found this spot.
>
>> +++ b/blockdev.c
>> @@ -1583,13 +1583,10 @@ static void
>> internal_snapshot_abort(BlkActionState *common)
>>  }
>>  
>>  if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, _error) < 0) {
>> - error_report("Failed to delete snapshot with id '%s' and name '%s'
>> on "
>> - "device '%s' in abort: %s",
>> - sn->id_str,
>> - sn->name,
>> - bdrv_get_device_name(bs),
>> - error_get_pretty(local_error));
>> -error_free(local_error);
>> +error_reportf_err(local_error,
>> + "Failed to delete snapshot with id '%s' and name '%s' on " "device
>> '%s' in abort: ",
>
> Whoops; line rewrapping touchups missed here.
>
> With that fixed,
> Reviewed-by: Eric Blake 

Thanks!

Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl

[Laurent Vivier]

Le 18/12/2015 07:26, Chen Gang a écrit :
> 
> For fcntl, it always needs to notice about it, just like do_fcntl() has
> done, or it will cause issue (e.g. alpha host run i386 guest).
> 
> Signed-off-by: Chen Gang 
> ---
>  linux-user/syscall.c |   18 --
>  1 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 0f8adeb..1a60e6f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9007,7 +9007,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  if (((CPUARMState *)cpu_env)->eabi) {
>  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>  goto efault;
> -fl.l_type = tswap16(target_efl->l_type);
> +fl.l_type = 
> target_to_host_bitmask(tswap16(target_fl->l_type),
> +   flock_tbl);
>  fl.l_whence = tswap16(target_efl->l_whence);
>  fl.l_start = tswap64(target_efl->l_start);
>  fl.l_len = tswap64(target_efl->l_len);
> @@ -9018,7 +9019,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  {
>  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>  goto efault;
> -fl.l_type = tswap16(target_fl->l_type);
> +fl.l_type = 
> target_to_host_bitmask(tswap16(target_fl->l_type),
> +   flock_tbl);
>  fl.l_whence = tswap16(target_fl->l_whence);
>  fl.l_start = tswap64(target_fl->l_start);
>  fl.l_len = tswap64(target_fl->l_len);
> @@ -9031,7 +9033,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  if (((CPUARMState *)cpu_env)->eabi) {
>  if (!lock_user_struct(VERIFY_WRITE, target_efl, arg3, 
> 0)) 
>  goto efault;
> -target_efl->l_type = tswap16(fl.l_type);
> +target_efl->l_type = host_to_target_bitmask(
> + tswap16(fl.l_type), 
> flock_tbl);
>  target_efl->l_whence = tswap16(fl.l_whence);
>  target_efl->l_start = tswap64(fl.l_start);
>  target_efl->l_len = tswap64(fl.l_len);
> @@ -9042,7 +9045,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  {
>  if (!lock_user_struct(VERIFY_WRITE, target_fl, arg3, 0)) 
>  goto efault;
> -target_fl->l_type = tswap16(fl.l_type);
> +target_fl->l_type = host_to_target_bitmask(
> + tswap16(fl.l_type), 
> flock_tbl);
>  target_fl->l_whence = tswap16(fl.l_whence);
>  target_fl->l_start = tswap64(fl.l_start);
>  target_fl->l_len = tswap64(fl.l_len);
> @@ -9058,7 +9062,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  if (((CPUARMState *)cpu_env)->eabi) {
>  if (!lock_user_struct(VERIFY_READ, target_efl, arg3, 1)) 
>  goto efault;
> -fl.l_type = tswap16(target_efl->l_type);
> +fl.l_type = 
> target_to_host_bitmask(tswap16(target_fl->l_type),
> +   flock_tbl);
>  fl.l_whence = tswap16(target_efl->l_whence);
>  fl.l_start = tswap64(target_efl->l_start);
>  fl.l_len = tswap64(target_efl->l_len);
> @@ -9069,7 +9074,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  {
>  if (!lock_user_struct(VERIFY_READ, target_fl, arg3, 1)) 
>  goto efault;
> -fl.l_type = tswap16(target_fl->l_type);
> +fl.l_type = 
> target_to_host_bitmask(tswap16(target_fl->l_type),
> +   flock_tbl);
>  fl.l_whence = tswap16(target_fl->l_whence);
>  fl.l_start = tswap64(target_fl->l_start);
>  fl.l_len = tswap64(target_fl->l_len);
> 

This patch looks good to me, except that script/checkpatch.pl complains
about "DOS line ending" and "line over 80 characters".

Reviewed-by: Laurent Vivier 

[Qemu-devel] [PATCH] ccid: use libcacard.h from 2.5.1

[marcandre . lureau]

From: Marc-André Lureau 

This cleans up a bit libcacard headers inclusion.

Signed-off-by: Marc-André Lureau 
---
 configure   | 7 ---
 hw/usb/ccid-card-emulated.c | 5 +
 hw/usb/ccid-card-passthru.c | 2 +-
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index b9552fd..65dc5ed 100755
--- a/configure
+++ b/configure
@@ -3981,17 +3981,18 @@ EOF
 fi
 
 # check for smartcard support
+libcacard_version=2.5.1
 smartcard_cflags=""
 if test "$smartcard" != "no"; then
-if $pkg_config libcacard; then
+if $pkg_config "libcacard >= $libcacard_version"; then
 libcacard_cflags=$($pkg_config --cflags libcacard)
 libcacard_libs=$($pkg_config --libs libcacard)
-QEMU_CFLAGS="$QEMU_CFLAGS $libcacard_cflags"
+QEMU_CFLAGS= $QEMU_CFLAGS $libcacard_cflags""
 libs_softmmu="$libs_softmmu $libcacard_libs"
 smartcard="yes"
 else
 if test "$smartcard" = "yes"; then
-feature_not_found "smartcard" "Install libcacard devel"
+feature_not_found "smartcard" "Install 
libcacard(>=$libcacard_version) devel"
 fi
 smartcard="no"
 fi
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index 869a63c..25d3e3f 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -26,10 +26,7 @@
  * the db parameter.
  */
 
-#include 
-#include 
-#include 
-#include 
+#include 
 
 #include "qemu/thread.h"
 #include "sysemu/char.h"
diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
index 9f49c05..f4c276f 100644
--- a/hw/usb/ccid-card-passthru.c
+++ b/hw/usb/ccid-card-passthru.c
@@ -12,7 +12,7 @@
 #include "qemu/error-report.h"
 #include "qemu/sockets.h"
 #include "ccid.h"
-#include "cacard/vscard_common.h"
+#include 
 
 #define DPRINTF(card, lvl, fmt, ...)\
 do {\
-- 
2.5.0

Re: [Qemu-devel] [PATCH 06/10] sdhci_sysbus: Create SD card device in users, not the device itself

[Peter Maydell]

On 18 December 2015 at 00:18, Alistair Francis
 wrote:
> On Fri, Dec 11, 2015 at 8:37 AM, Peter Maydell  
> wrote:
>>
>> +/* Create and plug in the SD cards */
>> +for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) {
>> +BusState *bus;
>> +DriveInfo *di = drive_get_next(IF_SD);
>> +BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL;
>> +DeviceState *carddev;
>> +
>> +bus = qdev_get_child_bus(DEVICE(>soc.sdhci[i]), "sd-bus");
>
> This looks like the same thing I was trying to avoid with my SPI
> patches. We were trying to avoid the machine reaching into the SoC
> when getting the child busses. Instead expose the bus to the SoC so
> the board can just get it straight from there.

Yes, I wrote this code first and then saw your patches second.
Whatever we do, we should deal with the problem the same way.

thanks
-- PMM

[Qemu-devel] [PATCH v4 1/7] mips/kvm: Remove a couple of noisy DPRINTFs

[James Hogan]

The DPRINTFs in cpu_mips_io_interrupts_pending() and kvm_arch_pre_run()
are particularly noisy during normal execution, and also not
particularly helpful. Remove them so that more important debug messages
can be more easily seen.

Signed-off-by: James Hogan 
Reviewed-by: Leon Alrae 
Cc: Paolo Bonzini 
Cc: Aurelien Jarno 
---
 target-mips/kvm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index 12d7db311ece..5cd65ad201ca 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -88,7 +88,6 @@ static inline int cpu_mips_io_interrupts_pending(MIPSCPU *cpu)
 {
 CPUMIPSState *env = >env;
 
-DPRINTF("%s: %#x\n", __func__, env->CP0_Cause & (1 << (2 + CP0Ca_IP)));
 return env->CP0_Cause & (0x1 << (2 + CP0Ca_IP));
 }
 
@@ -117,7 +116,6 @@ void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
 
 MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
 {
-DPRINTF("%s\n", __func__);
 return MEMTXATTRS_UNSPECIFIED;
 }
 
-- 
2.4.10

Re: [Qemu-devel] [RFC PATCH v0 2/9] cpu: Store CPU typename in MachineState

[Igor Mammedov]

On Thu, 17 Dec 2015 16:09:23 -0200
Eduardo Habkost  wrote:

> On Wed, Dec 16, 2015 at 11:26:20PM +0100, Igor Mammedov wrote:
> > On Wed, 16 Dec 2015 17:39:02 -0200
> > Eduardo Habkost  wrote:
> > 
> > > On Wed, Dec 16, 2015 at 05:54:25PM +0100, Igor Mammedov wrote:
> > > > On Tue, 15 Dec 2015 14:08:09 +0530
> > > > Bharata B Rao  wrote:
> > > > 
> > > > > On Mon, Dec 14, 2015 at 03:29:49PM -0200, Eduardo Habkost wrote:
> > > > > > On Thu, Dec 10, 2015 at 11:45:37AM +0530, Bharata B Rao wrote:
> > > > > > > Storing CPU typename in MachineState lets us to create CPU
> > > > > > > threads for all architectures in uniform manner from
> > > > > > > arch-neutral code.
> > > > > > > 
> > > > > > > TODO: Touching only i386 and spapr targets for now
> > > > > > > 
> > > > > > > Signed-off-by: Bharata B Rao 
> > > > > > 
> > > > > > Suggestions:
> > > > > > 
> > > > > > * Name the field "cpu_base_type" to indicate it is the base CPU
> > > > > >   class name, not the actual CPU class name used when creating
> > > > > >   CPUs.
> > > > > > * Put it in MachineClass, as it may be useful for code that
> > > > > >   runs before machine->init(), in the future.
> > > > > 
> > > > > Ok.
> > > > > 
> > > > > > * Maybe make it a CPUClass* field instead of a string?
> > > > > 
> > > > > In the current use case, this base cpu type string is being passed
> > > > > to cpu_generic_init(const char *typename, const char *cpu_model)
> > > > > to create boot time CPUs with given typename and cpu_mode. So for
> > > > > now the string makes sense for use case.
> > > > > 
> > > > > Making it CPUClass* would necessiate more changes to
> > > > > cpu_generic_init().
> > > > how about actually leaving it as "cpu_type" and putting in it
> > > > actual cpu type that could be used with device_add().
> > > > 
> > > > that would get rid of keeping and passing around intermediate
> > > > cpu_model.
> > > 
> > > Makes sense. We only need to save both typename and cpu_model
> > > today because cpu_generic_init() currently encapsulates three
> > > steps: CPU class lookup + CPU creation + CPU feature parsing. But
> > > we shouldn't need to redo CPU class lookup every time.
> > BTW: Eduardo do you know if QEMU could somehow provide a list of
> > supported CPU types (i.e. not cpumodels) to libvirt?
> 
> Not sure I understand the question. Could you clarify what you
> mean by "supported CPU types", and what's the problem it would
> solve?
device_add TYPE, takes only type name so I'd like to kep it that way
and make sure that libvirt/user can list cpu types that hi would
be able to use with device_add/-device.

for PC they are generated from cpu_model with help of x86_cpu_type_name()

> > 
> > > 
> > > We could just split cpu_model once, and save the resulting
> > > CPUClass* + featurestr, instead of saving the full cpu_model
> > > string and parsing it again every time.
> > isn't featurestr as x86/sparc specific?
> > 
> > Could we have field in  x86_cpu_class/sparc_cpu_class for it and set it
> > when cpu_model is parsed?
> > That way generic cpu_model parser would handle only cpu names and
> > target specific overrides would handle both.
> 
> I always assumed we want to have a generic CPU model + featurestr
> mechanism that could be reused by multiple architectures.

I've thought the opposite way, that we wanted to faze out featurestr
in favor of generic option parsing of generic device, i.e.
 -device TYPE,option=X,...
but we would have to keep compatibility with old CLI
that supplies cpu definition via -cpu cpu_model,featurestr
so cpu_model translated into "cpu_type" field make sense for every
target but featurestr is x86/sparc specific and I'd prefer to
keep it that way and do not introduce it to other targets.

[Qemu-devel] [PATCH v4 4/7] mips/kvm: Support unsigned KVM registers

[James Hogan]

Add KVM register access functions for the uint32_t type. This is
required for FP and MSA control registers, which are represented as
unsigned 32-bit integers.

Signed-off-by: James Hogan 
Cc: Paolo Bonzini 
Cc: Leon Alrae 
Cc: Aurelien Jarno 
---
Changes in v3:
- Fix big endian (the pointer passed to the kernel must be for the
  actual 32-bit value, not a temporary 64-bit value, otherwise on big
  endian systems the kernel will only interpret the upper half).
---
 target-mips/kvm.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index b777a5e93fb1..a11095f273f0 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -248,6 +248,17 @@ static inline int kvm_mips_put_one_reg(CPUState *cs, 
uint64_t reg_id,
 return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
 }
 
+static inline int kvm_mips_put_one_ureg(CPUState *cs, uint64_t reg_id,
+uint32_t *addr)
+{
+struct kvm_one_reg cp0reg = {
+.id = reg_id,
+.addr = (uintptr_t)addr
+};
+
+return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+}
+
 static inline int kvm_mips_put_one_ulreg(CPUState *cs, uint64_t reg_id,
  target_ulong *addr)
 {
@@ -282,6 +293,17 @@ static inline int kvm_mips_get_one_reg(CPUState *cs, 
uint64_t reg_id,
 return kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
 }
 
+static inline int kvm_mips_get_one_ureg(CPUState *cs, uint64_t reg_id,
+uint32_t *addr)
+{
+struct kvm_one_reg cp0reg = {
+.id = reg_id,
+.addr = (uintptr_t)addr
+};
+
+return kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+}
+
 static inline int kvm_mips_get_one_ulreg(CPUState *cs, uint64 reg_id,
  target_ulong *addr)
 {
-- 
2.4.10

[Qemu-devel] [PATCH v4 0/7] mips/kvm: Support FPU & SIMD (MSA) in MIPS KVM guests

[James Hogan]

Here's a v4 refresh of my FPU/MSA patchset for v2.6. Thanks to all who
have taken the time to review it so far.

This patchset primarily adds support for FPU and MIPS SIMD Architecture
(MSA) in MIPS KVM guests to QEMU. It depends on Linux v4.1, specifically
my KVM patchset to add the corresponding hypervisor support to KVM
("[PATCH 00/20] MIPS: KVM: Guest FPU & SIMD (MSA) support").

All comments welcome.

Changes in v4:
- Rebase on master (dropped patch 1 & 2).

Changes in v3 (patch 6 only):
- Fix big endian (the pointer passed to the kernel must be for the
  actual 32-bit value, not a temporary 64-bit value, otherwise on big
  endian systems the kernel will only interpret the upper half).

Changes in v2:
- Moved most of patch 7 and updates to linux-headers/linux/kvm.h from
  patches 8 and 9 into a new patch 1, which is purely for reference
  (Paolo).
- Add the changes to MIPS_CP0_{32,64} macros from v1 patch 7 to patch 2,
  since the rest of that patch is now unnecessary and the change is
  along the same lines as patch 2 (not added Leon's Reviewed-by to this
  patch due to that non-reviewed change).
- Fix line wrapping of kvm_mips_get_one_reg() calls from Config4 and
  Config5 in patch 5 (Leon).
- Change (1 << x) to (1U << x) in important places in patch 5, 8 & 9 to
  avoid compiler undefined behaviour (Leon).

James Hogan (7):
  mips/kvm: Remove a couple of noisy DPRINTFs
  mips/kvm: Implement PRid CP0 register
  mips/kvm: Implement Config CP0 registers
  mips/kvm: Support unsigned KVM registers
  mips/kvm: Support signed 64-bit KVM registers
  mips/kvm: Support FPU in MIPS KVM guests
  mips/kvm: Support MSA in MIPS KVM guests

 target-mips/kvm.c | 388 --
 1 file changed, 375 insertions(+), 13 deletions(-)

-- 
2.4.10

[Qemu-devel] [PATCH v4 2/7] mips/kvm: Implement PRid CP0 register

[James Hogan]

Implement saving and restoring to KVM state of the Processor ID (PRid)
CP0 register. This allows QEMU to control the PRid exposed to the guest
instead of using the default set by KVM.

Signed-off-by: James Hogan 
Reviewed-by: Leon Alrae 
Cc: Paolo Bonzini 
Cc: Aurelien Jarno 
---
 target-mips/kvm.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index 5cd65ad201ca..41abc8709d96 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -228,6 +228,7 @@ int kvm_mips_set_ipi_interrupt(MIPSCPU *cpu, int irq, int 
level)
 #define KVM_REG_MIPS_CP0_STATUS MIPS_CP0_32(12, 0)
 #define KVM_REG_MIPS_CP0_CAUSE  MIPS_CP0_32(13, 0)
 #define KVM_REG_MIPS_CP0_EPCMIPS_CP0_64(14, 0)
+#define KVM_REG_MIPS_CP0_PRID   MIPS_CP0_32(15, 0)
 #define KVM_REG_MIPS_CP0_ERROREPC   MIPS_CP0_64(30, 0)
 
 static inline int kvm_mips_put_one_reg(CPUState *cs, uint64_t reg_id,
@@ -520,6 +521,11 @@ static int kvm_mips_put_cp0_registers(CPUState *cs, int 
level)
 DPRINTF("%s: Failed to put CP0_EPC (%d)\n", __func__, err);
 ret = err;
 }
+err = kvm_mips_put_one_reg(cs, KVM_REG_MIPS_CP0_PRID, >CP0_PRid);
+if (err < 0) {
+DPRINTF("%s: Failed to put CP0_PRID (%d)\n", __func__, err);
+ret = err;
+}
 err = kvm_mips_put_one_ulreg(cs, KVM_REG_MIPS_CP0_ERROREPC,
  >CP0_ErrorEPC);
 if (err < 0) {
@@ -606,6 +612,11 @@ static int kvm_mips_get_cp0_registers(CPUState *cs)
 DPRINTF("%s: Failed to get CP0_EPC (%d)\n", __func__, err);
 ret = err;
 }
+err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_PRID, >CP0_PRid);
+if (err < 0) {
+DPRINTF("%s: Failed to get CP0_PRID (%d)\n", __func__, err);
+ret = err;
+}
 err = kvm_mips_get_one_ulreg(cs, KVM_REG_MIPS_CP0_ERROREPC,
  >CP0_ErrorEPC);
 if (err < 0) {
-- 
2.4.10

[Qemu-devel] [PATCH v4 6/7] mips/kvm: Support FPU in MIPS KVM guests

[James Hogan]

Support the new KVM_CAP_MIPS_FPU capability, which allows the host's FPU
to be exposed to the KVM guest.

The capability is enabled if the guest core has an FPU according to its
Config1 register. Various config bits are now writeable so that KVM is
aware of the configuration (Config1.FP) and so that QEMU can
save/restore the guest modifiable bits (Config5.FRE, Config5.UFR,
Config5.UFE). The FCSR/FIR registers and the floating point registers
are now saved/restored (depending on the FR mode bit).

Signed-off-by: James Hogan 
Cc: Paolo Bonzini 
Cc: Leon Alrae 
Cc: Aurelien Jarno 
---
Changes in v2:
- Change (1 << x) to (1U << x) in important places to avoid compiler
  undefined behaviour (Leon).
- Removed update of linux-headers/linux/kvm.h (Paolo).
---
 target-mips/kvm.c | 124 --
 1 file changed, 120 insertions(+), 4 deletions(-)

diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index 284b7a954ba2..f66347b8250a 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -30,6 +30,8 @@
 #define DPRINTF(fmt, ...) \
 do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
 
+static int kvm_mips_fpu_cap;
+
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 KVM_CAP_LAST_INFO
 };
@@ -46,16 +48,29 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 /* MIPS has 128 signals */
 kvm_set_sigmask_len(s, 16);
 
+kvm_mips_fpu_cap = kvm_check_extension(s, KVM_CAP_MIPS_FPU);
+
 DPRINTF("%s\n", __func__);
 return 0;
 }
 
 int kvm_arch_init_vcpu(CPUState *cs)
 {
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = >env;
 int ret = 0;
 
 qemu_add_vm_change_state_handler(kvm_mips_update_state, cs);
 
+if (kvm_mips_fpu_cap && env->CP0_Config1 & (1 << CP0C1_FP)) {
+ret = kvm_vcpu_enable_cap(cs, KVM_CAP_MIPS_FPU, 0, 0);
+if (ret < 0) {
+/* mark unsupported so it gets disabled on reset */
+kvm_mips_fpu_cap = 0;
+ret = 0;
+}
+}
+
 DPRINTF("%s\n", __func__);
 return ret;
 }
@@ -64,8 +79,8 @@ void kvm_mips_reset_vcpu(MIPSCPU *cpu)
 {
 CPUMIPSState *env = >env;
 
-if (env->CP0_Config1 & (1 << CP0C1_FP)) {
-fprintf(stderr, "Warning: FPU not supported with KVM, disabling\n");
+if (!kvm_mips_fpu_cap && env->CP0_Config1 & (1 << CP0C1_FP)) {
+fprintf(stderr, "Warning: KVM does not support FPU, disabling\n");
 env->CP0_Config1 &= ~(1 << CP0C1_FP);
 }
 
@@ -355,11 +370,14 @@ static inline int kvm_mips_get_one_ureg64(CPUState *cs, 
uint64 reg_id,
 }
 
 #define KVM_REG_MIPS_CP0_CONFIG_MASK(1U << CP0C0_M)
-#define KVM_REG_MIPS_CP0_CONFIG1_MASK   (1U << CP0C1_M)
+#define KVM_REG_MIPS_CP0_CONFIG1_MASK   ((1U << CP0C1_M) | \
+ (1U << CP0C1_FP))
 #define KVM_REG_MIPS_CP0_CONFIG2_MASK   (1U << CP0C2_M)
 #define KVM_REG_MIPS_CP0_CONFIG3_MASK   (1U << CP0C3_M)
 #define KVM_REG_MIPS_CP0_CONFIG4_MASK   (1U << CP0C4_M)
-#define KVM_REG_MIPS_CP0_CONFIG5_MASK   0
+#define KVM_REG_MIPS_CP0_CONFIG5_MASK   ((1U << CP0C5_UFE) | \
+ (1U << CP0C5_FRE) | \
+ (1U << CP0C5_UFR))
 
 static inline int kvm_mips_change_one_reg(CPUState *cs, uint64_t reg_id,
   int32_t *addr, int32_t mask)
@@ -521,6 +539,98 @@ static void kvm_mips_update_state(void *opaque, int 
running, RunState state)
 }
 }
 
+static int kvm_mips_put_fpu_registers(CPUState *cs, int level)
+{
+MIPSCPU *cpu = MIPS_CPU(cs);
+CPUMIPSState *env = >env;
+int err, ret = 0;
+unsigned int i;
+
+/* Only put FPU state if we're emulating a CPU with an FPU */
+if (env->CP0_Config1 & (1 << CP0C1_FP)) {
+/* FPU Control Registers */
+if (level == KVM_PUT_FULL_STATE) {
+err = kvm_mips_put_one_ureg(cs, KVM_REG_MIPS_FCR_IR,
+>active_fpu.fcr0);
+if (err < 0) {
+DPRINTF("%s: Failed to put FCR_IR (%d)\n", __func__, err);
+ret = err;
+}
+}
+err = kvm_mips_put_one_ureg(cs, KVM_REG_MIPS_FCR_CSR,
+>active_fpu.fcr31);
+if (err < 0) {
+DPRINTF("%s: Failed to put FCR_CSR (%d)\n", __func__, err);
+ret = err;
+}
+
+/* Floating point registers */
+for (i = 0; i < 32; ++i) {
+if (env->CP0_Status & (1 << CP0St_FR)) {
+err = kvm_mips_put_one_ureg64(cs, KVM_REG_MIPS_FPR_64(i),
+  >active_fpu.fpr[i].d);
+} else {
+err = kvm_mips_get_one_ureg(cs, KVM_REG_MIPS_FPR_32(i),
+  
>active_fpu.fpr[i].w[FP_ENDIAN_IDX]);
+}
+if (err < 0) {
+  

Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Notice about lock bitmask translation for fcntl

[Chen Gang]

On 2015年12月18日 17:41, Laurent Vivier wrote:
> 
> 
> Le 18/12/2015 07:51, Chen Gang a écrit :
>>
>> I found this issue during my working time, it is about sw_64 (almost the
>> same as alpha) host running i386 wine programs.
>>
>> I also found another issue, but I am not quite sure whether it is worth
>> enough for our upstream: The related fix patch is below, which will let
>> the initialization slower, but for most archs, they have no this issue.
>>
>> linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in target_mmap()
>> 
>> In some architectures, they have no policy to zero MAP_ANONYMOUS memory,
>> which will cause issue for qemu target_mmap.
>> 
>> Signed-off-by: Chen Gang 
>>
>> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
>> index 7b459d5..9c9152d 100644
>> --- a/linux-user/mmap.c
>> +++ b/linux-user/mmap.c
>> @@ -567,6 +567,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
>> int prot,
>>  printf("\n");
>>  #endif
>>  tb_invalidate_phys_range(start, start + len);
>> +if ((prot & PROT_WRITE) && (flags & MAP_ANONYMOUS)
>> +&& ((flags & MAP_PRIVATE) || (fd == -1))) {
>> +memset(g2h(start), 0, len);
>> +}
> 
> IMHO, their kernel needs a fix, mmap(2):
> 
> MAP_ANONYMOUS
> The mapping is not backed by any file; its contents are initial‐
> ized to zero.
> 

OK, Thanks.


-- 
Chen Gang (陈刚)

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

[Qemu-devel] [PATCH v4 5/7] mips/kvm: Support signed 64-bit KVM registers

[James Hogan]

Rename kvm_mips_{get,put}_one_reg64() to kvm_mips_{get,put}_one_ureg64()
since they take an int64_t pointer, and add separate signed 64-bit
accessors. These will be used for double precision floating point
registers.

Signed-off-by: James Hogan 
Cc: Paolo Bonzini 
Cc: Leon Alrae 
Cc: Aurelien Jarno 
---
 target-mips/kvm.c | 40 +++-
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index a11095f273f0..284b7a954ba2 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -272,7 +272,18 @@ static inline int kvm_mips_put_one_ulreg(CPUState *cs, 
uint64_t reg_id,
 }
 
 static inline int kvm_mips_put_one_reg64(CPUState *cs, uint64_t reg_id,
- uint64_t *addr)
+ int64_t *addr)
+{
+struct kvm_one_reg cp0reg = {
+.id = reg_id,
+.addr = (uintptr_t)addr
+};
+
+return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+}
+
+static inline int kvm_mips_put_one_ureg64(CPUState *cs, uint64_t reg_id,
+  uint64_t *addr)
 {
 struct kvm_one_reg cp0reg = {
 .id = reg_id,
@@ -322,7 +333,18 @@ static inline int kvm_mips_get_one_ulreg(CPUState *cs, 
uint64 reg_id,
 }
 
 static inline int kvm_mips_get_one_reg64(CPUState *cs, uint64 reg_id,
- uint64_t *addr)
+ int64_t *addr)
+{
+struct kvm_one_reg cp0reg = {
+.id = reg_id,
+.addr = (uintptr_t)addr
+};
+
+return kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+}
+
+static inline int kvm_mips_get_one_ureg64(CPUState *cs, uint64 reg_id,
+  uint64_t *addr)
 {
 struct kvm_one_reg cp0reg = {
 .id = reg_id,
@@ -377,13 +399,13 @@ static int kvm_mips_save_count(CPUState *cs)
 int err, ret = 0;
 
 /* freeze KVM timer */
-err = kvm_mips_get_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, _ctl);
+err = kvm_mips_get_one_ureg64(cs, KVM_REG_MIPS_COUNT_CTL, _ctl);
 if (err < 0) {
 DPRINTF("%s: Failed to get COUNT_CTL (%d)\n", __func__, err);
 ret = err;
 } else if (!(count_ctl & KVM_REG_MIPS_COUNT_CTL_DC)) {
 count_ctl |= KVM_REG_MIPS_COUNT_CTL_DC;
-err = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, _ctl);
+err = kvm_mips_put_one_ureg64(cs, KVM_REG_MIPS_COUNT_CTL, _ctl);
 if (err < 0) {
 DPRINTF("%s: Failed to set COUNT_CTL.DC=1 (%d)\n", __func__, err);
 ret = err;
@@ -419,14 +441,14 @@ static int kvm_mips_restore_count(CPUState *cs)
 int err_dc, err, ret = 0;
 
 /* check the timer is frozen */
-err_dc = kvm_mips_get_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, _ctl);
+err_dc = kvm_mips_get_one_ureg64(cs, KVM_REG_MIPS_COUNT_CTL, _ctl);
 if (err_dc < 0) {
 DPRINTF("%s: Failed to get COUNT_CTL (%d)\n", __func__, err_dc);
 ret = err_dc;
 } else if (!(count_ctl & KVM_REG_MIPS_COUNT_CTL_DC)) {
 /* freeze timer (sets COUNT_RESUME for us) */
 count_ctl |= KVM_REG_MIPS_COUNT_CTL_DC;
-err = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, _ctl);
+err = kvm_mips_put_one_ureg64(cs, KVM_REG_MIPS_COUNT_CTL, _ctl);
 if (err < 0) {
 DPRINTF("%s: Failed to set COUNT_CTL.DC=1 (%d)\n", __func__, err);
 ret = err;
@@ -450,7 +472,7 @@ static int kvm_mips_restore_count(CPUState *cs)
 /* resume KVM timer */
 if (err_dc >= 0) {
 count_ctl &= ~KVM_REG_MIPS_COUNT_CTL_DC;
-err = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, _ctl);
+err = kvm_mips_put_one_ureg64(cs, KVM_REG_MIPS_COUNT_CTL, _ctl);
 if (err < 0) {
 DPRINTF("%s: Failed to set COUNT_CTL.DC=0 (%d)\n", __func__, err);
 ret = err;
@@ -483,8 +505,8 @@ static void kvm_mips_update_state(void *opaque, int 
running, RunState state)
 } else {
 /* Set clock restore time to now */
 count_resume = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
-ret = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_RESUME,
- _resume);
+ret = kvm_mips_put_one_ureg64(cs, KVM_REG_MIPS_COUNT_RESUME,
+  _resume);
 if (ret < 0) {
 fprintf(stderr, "Failed setting COUNT_RESUME\n");
 return;
-- 
2.4.10

[Qemu-devel] [PATCH v4 3/7] mips/kvm: Implement Config CP0 registers

[James Hogan]

Implement saving and restoring to KVM state of the Config CP0 registers
(namely Config, Config1, Config2, Config3, Config4, and Config5). These
control the features available to a guest, and a few of the fields will
soon be writeable by a guest so QEMU needs to know about them so as not
to clobber them on migration/savevm.

Signed-off-by: James Hogan 
Cc: Paolo Bonzini 
Cc: Leon Alrae 
Cc: Aurelien Jarno 
---
Changes in v2:
- Fix line wrapping of kvm_mips_get_one_reg() calls from Config4 and
  Config5 (Leon).
- Change (1 << x) to (1U << x) in important places to avoid compiler
  defined behaviour (Leon).
---
 target-mips/kvm.c | 106 ++
 1 file changed, 106 insertions(+)

diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index 41abc8709d96..b777a5e93fb1 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -229,6 +229,12 @@ int kvm_mips_set_ipi_interrupt(MIPSCPU *cpu, int irq, int 
level)
 #define KVM_REG_MIPS_CP0_CAUSE  MIPS_CP0_32(13, 0)
 #define KVM_REG_MIPS_CP0_EPCMIPS_CP0_64(14, 0)
 #define KVM_REG_MIPS_CP0_PRID   MIPS_CP0_32(15, 0)
+#define KVM_REG_MIPS_CP0_CONFIG MIPS_CP0_32(16, 0)
+#define KVM_REG_MIPS_CP0_CONFIG1MIPS_CP0_32(16, 1)
+#define KVM_REG_MIPS_CP0_CONFIG2MIPS_CP0_32(16, 2)
+#define KVM_REG_MIPS_CP0_CONFIG3MIPS_CP0_32(16, 3)
+#define KVM_REG_MIPS_CP0_CONFIG4MIPS_CP0_32(16, 4)
+#define KVM_REG_MIPS_CP0_CONFIG5MIPS_CP0_32(16, 5)
 #define KVM_REG_MIPS_CP0_ERROREPC   MIPS_CP0_64(30, 0)
 
 static inline int kvm_mips_put_one_reg(CPUState *cs, uint64_t reg_id,
@@ -304,6 +310,34 @@ static inline int kvm_mips_get_one_reg64(CPUState *cs, 
uint64 reg_id,
 return kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
 }
 
+#define KVM_REG_MIPS_CP0_CONFIG_MASK(1U << CP0C0_M)
+#define KVM_REG_MIPS_CP0_CONFIG1_MASK   (1U << CP0C1_M)
+#define KVM_REG_MIPS_CP0_CONFIG2_MASK   (1U << CP0C2_M)
+#define KVM_REG_MIPS_CP0_CONFIG3_MASK   (1U << CP0C3_M)
+#define KVM_REG_MIPS_CP0_CONFIG4_MASK   (1U << CP0C4_M)
+#define KVM_REG_MIPS_CP0_CONFIG5_MASK   0
+
+static inline int kvm_mips_change_one_reg(CPUState *cs, uint64_t reg_id,
+  int32_t *addr, int32_t mask)
+{
+int err;
+int32_t tmp, change;
+
+err = kvm_mips_get_one_reg(cs, reg_id, );
+if (err < 0) {
+return err;
+}
+
+/* only change bits in mask */
+change = (*addr ^ tmp) & mask;
+if (!change) {
+return 0;
+}
+
+tmp = tmp ^ change;
+return kvm_mips_put_one_reg(cs, reg_id, );
+}
+
 /*
  * We freeze the KVM timer when either the VM clock is stopped or the state is
  * saved (the state is dirty).
@@ -526,6 +560,48 @@ static int kvm_mips_put_cp0_registers(CPUState *cs, int 
level)
 DPRINTF("%s: Failed to put CP0_PRID (%d)\n", __func__, err);
 ret = err;
 }
+err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG,
+  >CP0_Config0,
+  KVM_REG_MIPS_CP0_CONFIG_MASK);
+if (err < 0) {
+DPRINTF("%s: Failed to change CP0_CONFIG (%d)\n", __func__, err);
+ret = err;
+}
+err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG1,
+  >CP0_Config1,
+  KVM_REG_MIPS_CP0_CONFIG1_MASK);
+if (err < 0) {
+DPRINTF("%s: Failed to change CP0_CONFIG1 (%d)\n", __func__, err);
+ret = err;
+}
+err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG2,
+  >CP0_Config2,
+  KVM_REG_MIPS_CP0_CONFIG2_MASK);
+if (err < 0) {
+DPRINTF("%s: Failed to change CP0_CONFIG2 (%d)\n", __func__, err);
+ret = err;
+}
+err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG3,
+  >CP0_Config3,
+  KVM_REG_MIPS_CP0_CONFIG3_MASK);
+if (err < 0) {
+DPRINTF("%s: Failed to change CP0_CONFIG3 (%d)\n", __func__, err);
+ret = err;
+}
+err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG4,
+  >CP0_Config4,
+  KVM_REG_MIPS_CP0_CONFIG4_MASK);
+if (err < 0) {
+DPRINTF("%s: Failed to change CP0_CONFIG4 (%d)\n", __func__, err);
+ret = err;
+}
+err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG5,
+  >CP0_Config5,
+  KVM_REG_MIPS_CP0_CONFIG5_MASK);
+if (err < 0) {
+DPRINTF("%s: Failed to change CP0_CONFIG5 (%d)\n", __func__, err);
+ret = err;
+}
 err = kvm_mips_put_one_ulreg(cs, KVM_REG_MIPS_CP0_ERROREPC,
  >CP0_ErrorEPC);
 if (err < 0) {
@@ -617,6 +693,36 @@ static int 

[Qemu-devel] [PATCH v4 7/7] mips/kvm: Support MSA in MIPS KVM guests

[James Hogan]

Support the new KVM_CAP_MIPS_MSA capability, which allows MIPS SIMD
Architecture (MSA) to be exposed to the KVM guest.

The capability is enabled if the guest core has MSA according to its
Config3 register. Various config bits are now writeable so that KVM is
aware of the configuration (Config3.MSAP) and so that QEMU can
save/restore the guest modifiable bits (Config5.MSAEn). The MSACSR/MSAIR
registers and the MSA vector registers are now saved/restored. Since the
FP registers are a subset of the vector registers, they are omitted if
the guest has MSA.

Signed-off-by: James Hogan 
Cc: Paolo Bonzini 
Cc: Leon Alrae 
Cc: Aurelien Jarno 
---
Changes in v2:
- Change (1 << x) to (1U << x) in important places to avoid compiler
  undefined behaviour (Leon).
- Removed update of linux-headers/linux/kvm.h (Paolo).
---
 target-mips/kvm.c | 127 +-
 1 file changed, 107 insertions(+), 20 deletions(-)

diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index f66347b8250a..4404797261fd 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -31,6 +31,7 @@
 do { if (DEBUG_KVM) { fprintf(stderr, fmt, ## __VA_ARGS__); } } while (0)
 
 static int kvm_mips_fpu_cap;
+static int kvm_mips_msa_cap;
 
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 KVM_CAP_LAST_INFO
@@ -49,6 +50,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
 kvm_set_sigmask_len(s, 16);
 
 kvm_mips_fpu_cap = kvm_check_extension(s, KVM_CAP_MIPS_FPU);
+kvm_mips_msa_cap = kvm_check_extension(s, KVM_CAP_MIPS_MSA);
 
 DPRINTF("%s\n", __func__);
 return 0;
@@ -71,6 +73,15 @@ int kvm_arch_init_vcpu(CPUState *cs)
 }
 }
 
+if (kvm_mips_msa_cap && env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+ret = kvm_vcpu_enable_cap(cs, KVM_CAP_MIPS_MSA, 0, 0);
+if (ret < 0) {
+/* mark unsupported so it gets disabled on reset */
+kvm_mips_msa_cap = 0;
+ret = 0;
+}
+}
+
 DPRINTF("%s\n", __func__);
 return ret;
 }
@@ -83,6 +94,10 @@ void kvm_mips_reset_vcpu(MIPSCPU *cpu)
 fprintf(stderr, "Warning: KVM does not support FPU, disabling\n");
 env->CP0_Config1 &= ~(1 << CP0C1_FP);
 }
+if (!kvm_mips_msa_cap && env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+fprintf(stderr, "Warning: KVM does not support MSA, disabling\n");
+env->CP0_Config3 &= ~(1 << CP0C3_MSAP);
+}
 
 DPRINTF("%s\n", __func__);
 }
@@ -373,9 +388,11 @@ static inline int kvm_mips_get_one_ureg64(CPUState *cs, 
uint64 reg_id,
 #define KVM_REG_MIPS_CP0_CONFIG1_MASK   ((1U << CP0C1_M) | \
  (1U << CP0C1_FP))
 #define KVM_REG_MIPS_CP0_CONFIG2_MASK   (1U << CP0C2_M)
-#define KVM_REG_MIPS_CP0_CONFIG3_MASK   (1U << CP0C3_M)
+#define KVM_REG_MIPS_CP0_CONFIG3_MASK   ((1U << CP0C3_M) | \
+ (1U << CP0C3_MSAP))
 #define KVM_REG_MIPS_CP0_CONFIG4_MASK   (1U << CP0C4_M)
-#define KVM_REG_MIPS_CP0_CONFIG5_MASK   ((1U << CP0C5_UFE) | \
+#define KVM_REG_MIPS_CP0_CONFIG5_MASK   ((1U << CP0C5_MSAEn) | \
+ (1U << CP0C5_UFE) | \
  (1U << CP0C5_FRE) | \
  (1U << CP0C5_UFR))
 
@@ -564,17 +581,53 @@ static int kvm_mips_put_fpu_registers(CPUState *cs, int 
level)
 ret = err;
 }
 
-/* Floating point registers */
+/*
+ * FPU register state is a subset of MSA vector state, so don't put FPU
+ * registers if we're emulating a CPU with MSA.
+ */
+if (!(env->CP0_Config3 & (1 << CP0C3_MSAP))) {
+/* Floating point registers */
+for (i = 0; i < 32; ++i) {
+if (env->CP0_Status & (1 << CP0St_FR)) {
+err = kvm_mips_put_one_ureg64(cs, KVM_REG_MIPS_FPR_64(i),
+  >active_fpu.fpr[i].d);
+} else {
+err = kvm_mips_get_one_ureg(cs, KVM_REG_MIPS_FPR_32(i),
+>active_fpu.fpr[i].w[FP_ENDIAN_IDX]);
+}
+if (err < 0) {
+DPRINTF("%s: Failed to put FPR%u (%d)\n", __func__, i, 
err);
+ret = err;
+}
+}
+}
+}
+
+/* Only put MSA state if we're emulating a CPU with MSA */
+if (env->CP0_Config3 & (1 << CP0C3_MSAP)) {
+/* MSA Control Registers */
+if (level == KVM_PUT_FULL_STATE) {
+err = kvm_mips_put_one_reg(cs, KVM_REG_MIPS_MSA_IR,
+   >msair);
+if (err < 0) {
+DPRINTF("%s: Failed to put MSA_IR (%d)\n", __func__, err);
+ret = err;
+}
+}
+err = kvm_mips_put_one_reg(cs, KVM_REG_MIPS_MSA_CSR,

Re: [Qemu-devel] [PATCH COLO-Frame v12 31/38] COLO: Separate the process of saving/loading ram and device state

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> We separate the process of saving/loading ram and device state when do 
> checkpoint,
> we add new helpers for save/load ram/device. With this change, we can directly
> transfer ram from master to slave without using QEMUSizeBuffer as assistant,
> which also reduce the size of extra memory been used during checkpoint.
> 
> Besides, we move the colo_flush_ram_cache to the proper position after the
> above change.
> 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Li Zhijian 
> ---
> v11:
> - Remove load configuration section in qemu_loadvm_state_begin()
> 
> Signed-off-by: zhanghailiang 
> ---
>  include/sysemu/sysemu.h |   6 +++
>  migration/colo.c|  43 
>  migration/ram.c |   5 --
>  migration/savevm.c  | 132 
> ++--
>  4 files changed, 168 insertions(+), 18 deletions(-)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 91eeda3..5deae53 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -133,7 +133,13 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, 
> const char *name,
> uint64_t *start_list,
> uint64_t *length_list);
>  
> +int qemu_save_ram_precopy(QEMUFile *f);
> +int qemu_save_device_state(QEMUFile *f);
> +
>  int qemu_loadvm_state(QEMUFile *f);
> +int qemu_loadvm_state_begin(QEMUFile *f);
> +int qemu_load_ram_state(QEMUFile *f);
> +int qemu_load_device_state(QEMUFile *f);
>  
>  typedef enum DisplayType
>  {
> diff --git a/migration/colo.c b/migration/colo.c
> index 62a0444..d253d64 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -272,21 +272,32 @@ static int 
> colo_do_checkpoint_transaction(MigrationState *s,
>  goto out;
>  }
>  
> +ret = colo_put_cmd(s->to_dst_file, COLO_COMMAND_VMSTATE_SEND);
> +if (ret < 0) {
> +goto out;
> +}
>  /* Disable block migration */
>  s->params.blk = 0;
>  s->params.shared = 0;
> -qemu_savevm_state_header(trans);
> -qemu_savevm_state_begin(trans, >params);
> -qemu_mutex_lock_iothread();
> -qemu_savevm_state_complete_precopy(trans, false);
> -qemu_mutex_unlock_iothread();
> -
> -qemu_fflush(trans);
> +qemu_savevm_state_begin(s->to_dst_file, >params);
> +ret = qemu_file_get_error(s->to_dst_file);
> +if (ret < 0) {
> +error_report("save vm state begin error\n");

You don't need \n in error_report (Markus is trying to get rid
of all the existing cases where people do that!)

> +goto out;
> +}
>  
> -ret = colo_put_cmd(s->to_dst_file, COLO_COMMAND_VMSTATE_SEND);
> +qemu_mutex_lock_iothread();
> +/* Note: device state is saved into buffer */
> +ret = qemu_save_device_state(trans);
>  if (ret < 0) {
> +error_report("save device state error\n");
> +qemu_mutex_unlock_iothread();
>  goto out;
>  }
> +qemu_fflush(trans);
> +qemu_save_ram_precopy(s->to_dst_file);
> +qemu_mutex_unlock_iothread();
> +

It's interesting you're saving the devices and then saving the RAM,
where as in a normal migration we always save the RAM first and then
the devices;  I don't _think_ it makes any difference but I thought
I'd point it out.

>  /* we send the total size of the vmstate first */
>  size = qsb_get_length(buffer);
>  ret = colo_put_cmd_value(s->to_dst_file, COLO_COMMAND_VMSTATE_SIZE, 
> size);
> @@ -545,6 +556,16 @@ void *colo_process_incoming_thread(void *opaque)
>  goto out;
>  }
>  
> +ret = qemu_loadvm_state_begin(mis->from_src_file);
> +if (ret < 0) {
> +error_report("load vm state begin error, ret=%d", ret);
> +goto out;
> +}
> +ret = qemu_load_ram_state(mis->from_src_file);
> +if (ret < 0) {
> +error_report("load ram state error");
> +goto out;
> +}
>  /* read the VM state total size first */
>  ret = colo_get_cmd_value(mis->from_src_file,
>   COLO_COMMAND_VMSTATE_SIZE, );
> @@ -577,8 +598,10 @@ void *colo_process_incoming_thread(void *opaque)
>  qemu_mutex_lock_iothread();
>  qemu_system_reset(VMRESET_SILENT);
>  vmstate_loading = true;
> -if (qemu_loadvm_state(fb) < 0) {
> -error_report("COLO: loadvm failed");
> +colo_flush_ram_cache();
> +ret = qemu_load_device_state(fb);
> +if (ret < 0) {
> +error_report("COLO: load device state failed\n");
>  vmstate_loading = false;
>  qemu_mutex_unlock_iothread();
>  goto out;
> diff --git a/migration/ram.c b/migration/ram.c
> index 8ff7f7c..45d9332 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ 

[Qemu-devel] [PATCH 0/5] Convert to realize()

[Cao jin]

some simple case

Cao jin (5):
  SH PCI Host: convert to realize()
  igd-passthrough-i440FX: convert to realize()
  PXB: convert to realize()
  gt64120: convert to realize()
  xen-pvdevice: convert to realize()

 hw/i386/xen/xen_pvdevice.c  | 12 ++--
 hw/mips/gt64xxx_pci.c   |  6 ++
 hw/pci-bridge/pci_expander_bridge.c | 24 ++--
 hw/pci-host/piix.c  | 16 +---
 hw/sh4/sh_pci.c |  5 ++---
 5 files changed, 33 insertions(+), 30 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH 2/5] igd-passthrough-i440FX: convert to realize()

[Cao jin]

Signed-off-by: Cao jin 
---
 hw/pci-host/piix.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 715208b..e3840f0 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
 {0xa8, 4},  /* SNB: base of GTT stolen memory */
 };
 
-static int host_pci_config_read(int pos, int len, uint32_t val)
+static int host_pci_config_read(int pos, int len, uint32_t val, Error **errp)
 {
 char path[PATH_MAX];
 int config_fd;
@@ -772,15 +772,18 @@ static int host_pci_config_read(int pos, int len, 
uint32_t val)
 int ret = 0;
 
 if (rc >= size || rc < 0) {
+error_setg(errp, "No such device");
 return -ENODEV;
 }
 
 config_fd = open(path, O_RDWR);
 if (config_fd < 0) {
+error_setg(errp, "No such device");
 return -ENODEV;
 }
 
 if (lseek(config_fd, pos, SEEK_SET) != pos) {
+error_setg(errp, "lseek err: %s", strerror(errno));
 ret = -errno;
 goto out;
 }
@@ -789,13 +792,14 @@ static int host_pci_config_read(int pos, int len, 
uint32_t val)
 } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
 if (rc != len) {
 ret = -errno;
+error_setg(errp, "read err: %s", strerror(errno));
 }
 out:
 close(config_fd);
 return ret;
 }
 
-static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
+static void igd_pt_i440fx_realize(struct PCIDevice *pci_dev, Error **errp)
 {
 uint32_t val = 0;
 int rc, i, num;
@@ -805,14 +809,12 @@ static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
 for (i = 0; i < num; i++) {
 pos = igd_host_bridge_infos[i].offset;
 len = igd_host_bridge_infos[i].len;
-rc = host_pci_config_read(pos, len, val);
+rc = host_pci_config_read(pos, len, val, errp);
 if (rc) {
-return -ENODEV;
+return;
 }
 pci_default_write_config(pci_dev, pos, val, len);
 }
-
-return 0;
 }
 
 static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
@@ -820,7 +822,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass 
*klass, void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-k->init = igd_pt_i440fx_initfn;
+k->realize = igd_pt_i440fx_realize;
 dc->desc = "IGD Passthrough Host bridge";
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH 3/5] PXB: convert to realize()

[Cao jin]

Signed-off-by: Cao jin 
---
 hw/pci-bridge/pci_expander_bridge.c | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/hw/pci-bridge/pci_expander_bridge.c 
b/hw/pci-bridge/pci_expander_bridge.c
index 57f8a37..cc975f6 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -145,19 +145,19 @@ static const TypeInfo pxb_host_info = {
  * Returns 0 on successs, -1 if i440fx host was not
  * found or the bus number is already in use.
  */
-static int pxb_register_bus(PCIDevice *dev, PCIBus *pxb_bus)
+static int pxb_register_bus(PCIDevice *dev, PCIBus *pxb_bus, Error **errp)
 {
 PCIBus *bus = dev->bus;
 int pxb_bus_num = pci_bus_num(pxb_bus);
 
 if (bus->parent_dev) {
-error_report("PXB devices can be attached only to root bus.");
+error_setg(errp, "PXB devices can be attached only to root bus.");
 return -1;
 }
 
 QLIST_FOREACH(bus, >child, sibling) {
 if (pci_bus_num(bus) == pxb_bus_num) {
-error_report("Bus %d is already in use.", pxb_bus_num);
+error_setg(errp, "Bus %d is already in use.", pxb_bus_num);
 return -1;
 }
 }
@@ -193,7 +193,7 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
0;
 }
 
-static int pxb_dev_initfn(PCIDevice *dev)
+static void pxb_dev_realize(PCIDevice *dev, Error **errp)
 {
 PXBDev *pxb = PXB_DEV(dev);
 DeviceState *ds, *bds;
@@ -202,8 +202,8 @@ static int pxb_dev_initfn(PCIDevice *dev)
 
 if (pxb->numa_node != NUMA_NODE_UNASSIGNED &&
 pxb->numa_node >= nb_numa_nodes) {
-error_report("Illegal numa node %d.", pxb->numa_node);
-return -EINVAL;
+error_setg(errp, "Illegal numa node %d.", pxb->numa_node);
+return;
 }
 
 if (dev->qdev.id && *dev->qdev.id) {
@@ -225,8 +225,8 @@ static int pxb_dev_initfn(PCIDevice *dev)
 
 PCI_HOST_BRIDGE(ds)->bus = bus;
 
-if (pxb_register_bus(dev, bus)) {
-return -EINVAL;
+if (pxb_register_bus(dev, bus, errp)) {
+goto err_register_bus;
 }
 
 qdev_init_nofail(ds);
@@ -237,7 +237,11 @@ static int pxb_dev_initfn(PCIDevice *dev)
 pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_HOST);
 
 pxb_dev_list = g_list_insert_sorted(pxb_dev_list, pxb, pxb_compare);
-return 0;
+
+err_register_bus:
+object_unref(OBJECT(ds));
+object_unref(OBJECT(bds));
+object_unref(OBJECT(bus));
 }
 
 static void pxb_dev_exitfn(PCIDevice *pci_dev)
@@ -259,7 +263,7 @@ static void pxb_dev_class_init(ObjectClass *klass, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-k->init = pxb_dev_initfn;
+k->realize = pxb_dev_realize;
 k->exit = pxb_dev_exitfn;
 k->vendor_id = PCI_VENDOR_ID_REDHAT;
 k->device_id = PCI_DEVICE_ID_REDHAT_PXB;
-- 
2.1.0

[Qemu-devel] [PATCH 1/5] SH PCI Host: convert to realize()

[Cao jin]

Signed-off-by: Cao jin 
---
 hw/sh4/sh_pci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/sh4/sh_pci.c b/hw/sh4/sh_pci.c
index a2f6d9e..4509053 100644
--- a/hw/sh4/sh_pci.c
+++ b/hw/sh4/sh_pci.c
@@ -151,12 +151,11 @@ static int sh_pci_device_init(SysBusDevice *dev)
 return 0;
 }
 
-static int sh_pci_host_init(PCIDevice *d)
+static void sh_pci_host_realize(PCIDevice *d, Error **errp)
 {
 pci_set_word(d->config + PCI_COMMAND, PCI_COMMAND_WAIT);
 pci_set_word(d->config + PCI_STATUS, PCI_STATUS_CAP_LIST |
  PCI_STATUS_FAST_BACK | PCI_STATUS_DEVSEL_MEDIUM);
-return 0;
 }
 
 static void sh_pci_host_class_init(ObjectClass *klass, void *data)
@@ -164,7 +163,7 @@ static void sh_pci_host_class_init(ObjectClass *klass, void 
*data)
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 
-k->init = sh_pci_host_init;
+k->realize = sh_pci_host_realize;
 k->vendor_id = PCI_VENDOR_ID_HITACHI;
 k->device_id = PCI_DEVICE_ID_HITACHI_SH7751R;
 /*
-- 
2.1.0

[Qemu-devel] [PATCH 5/5] xen-pvdevice: convert to realize()

[Cao jin]

Signed-off-by: Cao jin 
---
 hw/i386/xen/xen_pvdevice.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/i386/xen/xen_pvdevice.c b/hw/i386/xen/xen_pvdevice.c
index c218947..a6c93d0 100644
--- a/hw/i386/xen/xen_pvdevice.c
+++ b/hw/i386/xen/xen_pvdevice.c
@@ -69,14 +69,16 @@ static const MemoryRegionOps xen_pv_mmio_ops = {
 .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static int xen_pv_init(PCIDevice *pci_dev)
+static void xen_pv_realize(PCIDevice *pci_dev, Error **errp)
 {
 XenPVDevice *d = XEN_PV_DEVICE(pci_dev);
 uint8_t *pci_conf;
 
 /* device-id property must always be supplied */
-if (d->device_id == 0x)
-   return -1;
+if (d->device_id == 0x) {
+error_setg(errp, "Device ID invalid, it must always be supplied")
+   return;
+}
 
 pci_conf = pci_dev->config;
 
@@ -97,8 +99,6 @@ static int xen_pv_init(PCIDevice *pci_dev)
 
 pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_MEM_PREFETCH,
  >mmio);
-
-return 0;
 }
 
 static Property xen_pv_props[] = {
@@ -114,7 +114,7 @@ static void xen_pv_class_init(ObjectClass *klass, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-k->init = xen_pv_init;
+k->realize = xen_pv_realize;
 k->class_id = PCI_CLASS_SYSTEM_OTHER;
 dc->desc = "Xen PV Device";
 dc->props = xen_pv_props;
-- 
2.1.0

[Qemu-devel] [PATCH 4/5] gt64120: convert to realize()

[Cao jin]

Signed-off-by: Cao jin 
---
 hw/mips/gt64xxx_pci.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index f76a9fd..c1f3c9c 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1193,7 +1193,7 @@ static int gt64120_init(SysBusDevice *dev)
 return 0;
 }
 
-static int gt64120_pci_init(PCIDevice *d)
+static void gt64120_pci_realize(PCIDevice *d, Error **errp)
 {
 /* FIXME: Malta specific hw assumptions ahead */
 pci_set_word(d->config + PCI_COMMAND, 0);
@@ -1207,8 +1207,6 @@ static int gt64120_pci_init(PCIDevice *d)
 pci_set_long(d->config + PCI_BASE_ADDRESS_4, 0x1400);
 pci_set_long(d->config + PCI_BASE_ADDRESS_5, 0x1401);
 pci_set_byte(d->config + 0x3d, 0x01);
-
-return 0;
 }
 
 static void gt64120_pci_class_init(ObjectClass *klass, void *data)
@@ -1216,7 +1214,7 @@ static void gt64120_pci_class_init(ObjectClass *klass, 
void *data)
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 
-k->init = gt64120_pci_init;
+k->realize = gt64120_pci_realize;
 k->vendor_id = PCI_VENDOR_ID_MARVELL;
 k->device_id = PCI_DEVICE_ID_MARVELL_GT6412X;
 k->revision = 0x10;
-- 
2.1.0

Re: [Qemu-devel] [PATCH 0/5] Convert to realize()

[Cao jin]

forgot to CC to: Markus

On 12/18/2015 07:03 PM, Cao jin wrote:

some simple case

Cao jin (5):
   SH PCI Host: convert to realize()
   igd-passthrough-i440FX: convert to realize()
   PXB: convert to realize()
   gt64120: convert to realize()
   xen-pvdevice: convert to realize()

  hw/i386/xen/xen_pvdevice.c  | 12 ++--
  hw/mips/gt64xxx_pci.c   |  6 ++
  hw/pci-bridge/pci_expander_bridge.c | 24 ++--
  hw/pci-host/piix.c  | 16 +---
  hw/sh4/sh_pci.c |  5 ++---
  5 files changed, 33 insertions(+), 30 deletions(-)



--
Yours Sincerely,

Cao Jin

Re: [Qemu-devel] [PATCH] gtk: use setlocale() for LC_MESSAGES only

[Kevin Wolf]

Am 10.09.2015 um 17:19 hat Alberto Garcia geschrieben:
> The QEMU code is not internationalized and assumes that it runs under
> the C locale, but if we use the GTK+ UI we'll end up importing the
> locale settings from the environment. This can break things, such as
> the JSON generator and iotest 120 in locales that use a decimal comma.
> 
> We do however have translations for a few simple strings for the GTK+
> menu items, so in order to run QEMU using the C locale, and yet have a
> translated UI let's use setlocale() for LC_MESSAGES only.
> 
> Signed-off-by: Alberto Garcia 

Not sure why I noticed it only now and if it's related to any recent
package upgrade on my side (using RHEL 7), but I noticed that non-ASCII
characters in the GTK UI strings are broken for me and git bisect
pointed to this commit.

Kevin

>  ui/gtk.c | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/gtk.c b/ui/gtk.c
> index df2a79e..11ea2cf 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1941,7 +1941,8 @@ void gtk_display_init(DisplayState *ds, bool 
> full_screen, bool grab_on_hover)
>  
>  s->free_scale = FALSE;
>  
> -setlocale(LC_ALL, "");
> +/* LC_MESSAGES only. See early_gtk_display_init() for details */
> +setlocale(LC_MESSAGES, "");
>  bindtextdomain("qemu", CONFIG_QEMU_LOCALEDIR);
>  textdomain("qemu");
>  
> @@ -2010,6 +2011,24 @@ void gtk_display_init(DisplayState *ds, bool 
> full_screen, bool grab_on_hover)
>  
>  void early_gtk_display_init(int opengl)
>  {
> +/* The QEMU code relies on the assumption that it's always run in
> + * the C locale. Therefore it is not prepared to deal with
> + * operations that produce different results depending on the
> + * locale, such as printf's formatting of decimal numbers, and
> + * possibly others.
> + *
> + * Since GTK+ calls setlocale() by default -importing the locale
> + * settings from the environment- we must prevent it from doing so
> + * using gtk_disable_setlocale().
> + *
> + * QEMU's GTK+ UI, however, _does_ have translations for some of
> + * the menu items. As a trade-off between a functionally correct
> + * QEMU and a fully internationalized UI we support importing
> + * LC_MESSAGES from the environment (see the setlocale() call
> + * earlier in this file). This allows us to display translated
> + * messages leaving everything else untouched.
> + */
> +gtk_disable_setlocale();
>  gtkinit = gtk_init_check(NULL, NULL);
>  if (!gtkinit) {
>  /* don't exit yet, that'll break -help */
> -- 
> 2.5.1
> 
> 

Re: [Qemu-devel] [PATCH v2 0/2] Clean up the remainders of bdrv_swap

[Kevin Wolf]

Am 18.12.2015 um 02:04 hat Fam Zheng geschrieben:
> v2: Update comment in patch 2. [Kevin]
> 
> 
> 
> Fam Zheng (2):
>   block: Remove prototype of bdrv_swap from header
>   iotests: Update comments for bdrv_swap() in 094
> 
>  include/block/block.h  | 1 -
>  tests/qemu-iotests/094 | 8 +---
>  2 files changed, 5 insertions(+), 4 deletions(-)

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH v3 8/9] elf: add arm note types

[Peter Maydell]

On 15 December 2015 at 22:51, Andrew Jones  wrote:
> Signed-off-by: Andrew Jones 
> ---
>  include/elf.h | 5 +
>  1 file changed, 5 insertions(+)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 9/9] target-arm: dump-guest-memory: add vfp notes for arm

[Peter Maydell]

On 15 December 2015 at 22:51, Andrew Jones  wrote:
> gdb won't actually dump these with 'info all-registers' since
> it first tries to confirm that it should by checking the VFP
> hwcap in the .auxv note. Well, we don't generate an .auxv note.
>
> Signed-off-by: Andrew Jones 
> ---

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PULL v4 3/9] io: add QIOTask class for async operations

[Daniel P. Berrange]

A number of I/O operations need to be performed asynchronously
to avoid blocking the main loop. The caller of such APIs need
to provide a callback to be invoked on completion/error and
need access to the error, if any. The small QIOTask provides
a simple framework for dealing with such probes. The API
docs inline provide an outline of how this is to be used.

Some functions don't have the ability to run asynchronously
(eg getaddrinfo always blocks), so to facilitate their use,
the task class provides a mechanism to run a blocking
function in a thread, while triggering the completion
callback in the main event loop thread. This easily allows
any synchronous function to be made asynchronous, albeit
at the cost of spawning a thread.

In this series, the QIOTask class will be used for things like
the TLS handshake, the websockets handshake and TCP connect()
progress.

The concept of QIOTask is inspired by the GAsyncResult
interface / GTask class in the GIO libraries. The min
version requirements on glib don't allow those to be
used from QEMU, so QIOTask provides a facsimilie which
can be easily switched to GTask in the future if the
min version is increased.

Signed-off-by: Daniel P. Berrange 
---
 include/io/task.h| 256 
 io/Makefile.objs |   1 +
 io/task.c| 159 ++
 tests/.gitignore |   1 +
 tests/Makefile   |   3 +
 tests/test-io-task.c | 268 +++
 trace-events |   9 ++
 7 files changed, 697 insertions(+)
 create mode 100644 include/io/task.h
 create mode 100644 io/task.c
 create mode 100644 tests/test-io-task.c

diff --git a/include/io/task.h b/include/io/task.h
new file mode 100644
index 000..2418714
--- /dev/null
+++ b/include/io/task.h
@@ -0,0 +1,256 @@
+/*
+ * QEMU I/O task
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_TASK_H__
+#define QIO_TASK_H__
+
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "qom/object.h"
+
+typedef struct QIOTask QIOTask;
+
+typedef void (*QIOTaskFunc)(Object *source,
+Error *err,
+gpointer opaque);
+
+typedef int (*QIOTaskWorker)(QIOTask *task,
+ Error **errp,
+ gpointer opaque);
+
+/**
+ * QIOTask:
+ *
+ * The QIOTask object provides a simple mechanism for reporting
+ * success / failure of long running background operations.
+ *
+ * A object on which the operation is to be performed could have
+ * a public API which accepts a task callback:
+ *
+ * 
+ *   Task callback function signature
+ *   
+ *  void myobject_operation(QMyObject *obj,
+ *  QIOTaskFunc *func,
+ *  gpointer opaque,
+ *  GDestroyNotify *notify);
+ *   
+ * 
+ *
+ * The 'func' parameter is the callback to be invoked, and 'opaque'
+ * is data to pass to it. The optional 'notify' function is used
+ * to free 'opaque' when no longer needed.
+ *
+ * Now, lets say the implementation of this method wants to set
+ * a timer to run once a second checking for completion of some
+ * activity. It would do something like
+ *
+ * 
+ *   Task callback function implementation
+ *   
+ *void myobject_operation(QMyObject *obj,
+ *QIOTaskFunc *func,
+ *gpointer opaque,
+ *GDestroyNotify *notify)
+ *{
+ *  QIOTask *task;
+ *
+ *  task = qio_task_new(OBJECT(obj), func, opaque, notify);
+ *
+ *  g_timeout_add_full(G_PRIORITY_DEFAULT,
+ * 1000,
+ * myobject_operation_timer,
+ * task,
+ * NULL);
+ *}
+ *   
+ * 
+ *
+ * It could equally have setup a watch on a file descriptor or
+ * created a background thread, or something else entirely.
+ * Notice that the source object is passed to the task, and
+ * QIOTask will hold a reference on that. This ensure that
+ * the QMyObject instance cannot be garbage collected while
+ * the async task is still in progress.
+ *
+ * In this case, myobject_operation_timer will fire after
+ * 3 secs and do
+ *
+ * 
+ *   Task timer 

[Qemu-devel] [PULL v4 5/9] io: add QIOChannelFile class

[Daniel P. Berrange]

Add a QIOChannel subclass that is capable of operating on things
that are files, such as plain files, pipes, character/block
devices, but notably not sockets.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-file.h|  93 ++
 io/Makefile.objs |   1 +
 io/channel-file.c| 225 +++
 tests/.gitignore |   2 +
 tests/Makefile   |   3 +
 tests/test-io-channel-file.c | 100 +++
 trace-events |   4 +
 7 files changed, 428 insertions(+)
 create mode 100644 include/io/channel-file.h
 create mode 100644 io/channel-file.c
 create mode 100644 tests/test-io-channel-file.c

diff --git a/include/io/channel-file.h b/include/io/channel-file.h
new file mode 100644
index 000..308e6d4
--- /dev/null
+++ b/include/io/channel-file.h
@@ -0,0 +1,93 @@
+/*
+ * QEMU I/O channels files driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_FILE_H__
+#define QIO_CHANNEL_FILE_H__
+
+#include "io/channel.h"
+
+#define TYPE_QIO_CHANNEL_FILE "qio-channel-file"
+#define QIO_CHANNEL_FILE(obj) \
+OBJECT_CHECK(QIOChannelFile, (obj), TYPE_QIO_CHANNEL_FILE)
+
+typedef struct QIOChannelFile QIOChannelFile;
+
+/**
+ * QIOChannelFile:
+ *
+ * The QIOChannelFile object provides a channel implementation
+ * that is able to perform I/O on block devices, character
+ * devices, FIFOs, pipes and plain files. While it is technically
+ * able to work on sockets too on the UNIX platform, this is not
+ * portable to Windows and lacks some extra sockets specific
+ * functionality. So the QIOChannelSocket object is recommended
+ * for that use case.
+ *
+ */
+
+struct QIOChannelFile {
+QIOChannel parent;
+int fd;
+};
+
+
+/**
+ * qio_channel_file_new_fd:
+ * @fd: the file descriptor
+ *
+ * Create a new IO channel object for a file represented
+ * by the @fd parameter. @fd can be associated with a
+ * block device, character device, fifo, pipe, or a
+ * regular file. For sockets, the QIOChannelSocket class
+ * should be used instead, as this provides greater
+ * functionality and cross platform portability.
+ *
+ * The channel will own the passed in file descriptor
+ * and will take responsibility for closing it, so the
+ * caller must not close it. If appropriate the caller
+ * should dup() its FD before opening the channel.
+ *
+ * Returns: the new channel object
+ */
+QIOChannelFile *
+qio_channel_file_new_fd(int fd);
+
+/**
+ * qio_channel_file_new_path:
+ * @fd: the file descriptor
+ * @flags: the open flags (O_RDONLY|O_WRONLY|O_RDWR, etc)
+ * @mode: the file creation mode if O_WRONLY is set in @flags
+ * @errp: pointer to initialized error object
+ *
+ * Create a new IO channel object for a file represented
+ * by the @path parameter. @path can point to any
+ * type of file on which sequential I/O can be
+ * performed, whether it be a plain file, character
+ * device or block device.
+ *
+ * Returns: the new channel object
+ */
+QIOChannelFile *
+qio_channel_file_new_path(const char *path,
+  int flags,
+  mode_t mode,
+  Error **errp);
+
+#endif /* QIO_CHANNEL_FILE_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
index e9d77aa..3d2f232 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -1,4 +1,5 @@
 io-obj-y = channel.o
+io-obj-y += channel-file.o
 io-obj-y += channel-socket.o
 io-obj-y += channel-watch.o
 io-obj-y += task.o
diff --git a/io/channel-file.c b/io/channel-file.c
new file mode 100644
index 000..1360900
--- /dev/null
+++ b/io/channel-file.c
@@ -0,0 +1,225 @@
+/*
+ * QEMU I/O channels files driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more 

[Qemu-devel] [PULL v4 4/9] io: add QIOChannelSocket class

[Daniel P. Berrange]

Implement a QIOChannel subclass that supports sockets I/O.
The implementation is able to manage a single socket file
descriptor, whether a TCP/UNIX listener, TCP/UNIX connection,
or a UDP datagram. It provides APIs which can listen and
connect either asynchronously or synchronously. Since there
is no asynchronous DNS lookup API available, it uses the
QIOTask helper for spawning a background thread to ensure
non-blocking operation.

Signed-off-by: Daniel P. Berrange 
---
 configure  |  11 +
 include/io/channel-socket.h| 244 ++
 include/qemu/sockets.h |  19 ++
 io/Makefile.objs   |   1 +
 io/channel-socket.c| 741 +
 scripts/create_config  |   9 +
 tests/.gitignore   |   1 +
 tests/Makefile |   3 +
 tests/io-channel-helpers.c | 246 ++
 tests/io-channel-helpers.h |  42 +++
 tests/test-io-channel-socket.c | 399 ++
 trace-events   |  19 ++
 util/qemu-sockets.c|   2 +-
 13 files changed, 1736 insertions(+), 1 deletion(-)
 create mode 100644 include/io/channel-socket.h
 create mode 100644 io/channel-socket.c
 create mode 100644 tests/io-channel-helpers.c
 create mode 100644 tests/io-channel-helpers.h
 create mode 100644 tests/test-io-channel-socket.c

diff --git a/configure b/configure
index b9552fd..375f103 100755
--- a/configure
+++ b/configure
@@ -2427,6 +2427,14 @@ fi
 
 
 ##
+# getifaddrs (for tests/test-io-channel-socket )
+
+have_ifaddrs_h=yes
+if ! check_include "ifaddrs.h" ; then
+  have_ifaddrs_h=no
+fi
+
+##
 # VTE probe
 
 if test "$vte" != "no"; then
@@ -5137,6 +5145,9 @@ fi
 if test "$tasn1" = "yes" ; then
   echo "CONFIG_TASN1=y" >> $config_host_mak
 fi
+if test "$have_ifaddrs_h" = "yes" ; then
+echo "HAVE_IFADDRS_H=y" >> $config_host_mak
+fi
 if test "$vte" = "yes" ; then
   echo "CONFIG_VTE=y" >> $config_host_mak
   echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak
diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
new file mode 100644
index 000..0719757
--- /dev/null
+++ b/include/io/channel-socket.h
@@ -0,0 +1,244 @@
+/*
+ * QEMU I/O channels sockets driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_SOCKET_H__
+#define QIO_CHANNEL_SOCKET_H__
+
+#include "io/channel.h"
+#include "io/task.h"
+#include "qemu/sockets.h"
+
+#define TYPE_QIO_CHANNEL_SOCKET "qio-channel-socket"
+#define QIO_CHANNEL_SOCKET(obj) \
+OBJECT_CHECK(QIOChannelSocket, (obj), TYPE_QIO_CHANNEL_SOCKET)
+
+typedef struct QIOChannelSocket QIOChannelSocket;
+
+/**
+ * QIOChannelSocket:
+ *
+ * The QIOChannelSocket class provides a channel implementation
+ * that can transport data over a UNIX socket or TCP socket.
+ * Beyond the core channel API, it also provides functionality
+ * for accepting client connections, tuning some socket
+ * parameters and getting socket address strings.
+ */
+
+struct QIOChannelSocket {
+QIOChannel parent;
+int fd;
+struct sockaddr_storage localAddr;
+socklen_t localAddrLen;
+struct sockaddr_storage remoteAddr;
+socklen_t remoteAddrLen;
+};
+
+
+/**
+ * qio_channel_socket_new:
+ *
+ * Create a channel for performing I/O on a socket
+ * connection, that is initially closed. After
+ * creating the socket, it must be setup as a client
+ * connection or server.
+ *
+ * Returns: the socket channel object
+ */
+QIOChannelSocket *
+qio_channel_socket_new(void);
+
+/**
+ * qio_channel_socket_new_fd:
+ * @fd: the socket file descriptor
+ * @errp: pointer to an uninitialized error object
+ *
+ * Create a channel for performing I/O on the socket
+ * connection represented by the file descriptor @fd.
+ *
+ * Returns: the socket channel object, or NULL on error
+ */
+QIOChannelSocket *
+qio_channel_socket_new_fd(int fd,
+  Error **errp);
+
+
+/**
+ * qio_channel_socket_connect_sync:
+ * @ioc: the socket channel object
+ * @addr: the address to connect to
+ * @errp: pointer to an uninitialized error object
+ *
+ * Attempt to connect to the address @addr. This method
+ * will run in the foreground 

[Qemu-devel] [PULL v4 1/9] io: add abstract QIOChannel classes

[Daniel P. Berrange]

Start the new generic I/O channel framework by defining a
QIOChannel abstract base class. This is designed to feel
similar to GLib's GIOChannel, but with the addition of
support for using iovecs, qemu error reporting, file
descriptor passing, coroutine integration and use of
the QOM framework for easier sub-classing.

The intention is that anywhere in QEMU that almost
anywhere that deals with sockets will use this new I/O
infrastructure, so that it becomes trivial to then layer
in support for TLS encryption. This will at least include
the VNC server, char device backend and migration code.

Signed-off-by: Daniel P. Berrange 
---
 MAINTAINERS  |   7 +
 Makefile |   2 +
 Makefile.objs|   5 +
 Makefile.target  |   2 +
 include/io/channel.h | 502 +++
 io/Makefile.objs |   1 +
 io/channel.c | 291 +
 7 files changed, 810 insertions(+)
 create mode 100644 include/io/channel.h
 create mode 100644 io/Makefile.objs
 create mode 100644 io/channel.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e8cee1e..55a0fd8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1243,6 +1243,13 @@ S: Odd fixes
 F: util/buffer.c
 F: include/qemu/buffer.h
 
+I/O Channels
+M: Daniel P. Berrange 
+S: Maintained
+F: io/
+F: include/io/
+F: tests/test-io-*
+
 Usermode Emulation
 --
 Overall
diff --git a/Makefile b/Makefile
index 930ac27..af3e5f1 100644
--- a/Makefile
+++ b/Makefile
@@ -159,6 +159,7 @@ dummy := $(call unnest-vars,, \
 crypto-obj-y \
 crypto-aes-obj-y \
 qom-obj-y \
+io-obj-y \
 common-obj-y \
 common-obj-m)
 
@@ -178,6 +179,7 @@ SOFTMMU_SUBDIR_RULES=$(filter %-softmmu,$(SUBDIR_RULES))
 
 $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(crypto-obj-y)
+$(SOFTMMU_SUBDIR_RULES): $(io-obj-y)
 $(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
 
 subdir-%:
diff --git a/Makefile.objs b/Makefile.objs
index 77be052..dac2c02 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -28,6 +28,11 @@ crypto-aes-obj-y = crypto/
 
 qom-obj-y = qom/
 
+###
+# io-obj-y is code used by both qemu system emulation and qemu-img
+
+io-obj-y = io/
+
 ##
 # Target independent part of system emulation. The long term path is to
 # suppress *all* target specific code in case of system emulation, i.e. a
diff --git a/Makefile.target b/Makefile.target
index 962d004..34ddb7e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -176,6 +176,7 @@ dummy := $(call unnest-vars,.., \
crypto-obj-y \
crypto-aes-obj-y \
qom-obj-y \
+   io-obj-y \
common-obj-y \
common-obj-m)
 target-obj-y := $(target-obj-y-save)
@@ -185,6 +186,7 @@ all-obj-y += $(qom-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
 all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
+all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
 
 $(QEMU_PROG_BUILD): config-devices.mak
 
diff --git a/include/io/channel.h b/include/io/channel.h
new file mode 100644
index 000..4ecec98
--- /dev/null
+++ b/include/io/channel.h
@@ -0,0 +1,502 @@
+/*
+ * QEMU I/O channels
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_H__
+#define QIO_CHANNEL_H__
+
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "qom/object.h"
+
+#define TYPE_QIO_CHANNEL "qio-channel"
+#define QIO_CHANNEL(obj)\
+OBJECT_CHECK(QIOChannel, (obj), TYPE_QIO_CHANNEL)
+#define QIO_CHANNEL_CLASS(klass)\
+OBJECT_CLASS_CHECK(QIOChannelClass, klass, TYPE_QIO_CHANNEL)
+#define QIO_CHANNEL_GET_CLASS(obj)  \
+OBJECT_GET_CLASS(QIOChannelClass, obj, TYPE_QIO_CHANNEL)
+
+typedef struct QIOChannel QIOChannel;
+typedef struct QIOChannelClass QIOChannelClass;
+
+#define QIO_CHANNEL_ERR_BLOCK -2
+
+typedef enum QIOChannelFeature QIOChannelFeature;
+
+enum QIOChannelFeature {
+QIO_CHANNEL_FEATURE_FD_PASS  

[Qemu-devel] [PULL v4 9/9] io: add QIOChannelBuffer class

[Daniel P. Berrange]

Add a QIOChannel subclass that is capable of performing I/O
to/from a memory buffer. This implementation does not attempt
to support concurrent readers & writers. It is designed for
serialized access where by a single thread at a time may write
data, seek and then read data back out.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-buffer.h|  60 ++
 io/Makefile.objs   |   1 +
 io/channel-buffer.c| 248 +
 tests/.gitignore   |   1 +
 tests/Makefile |   3 +
 tests/test-io-channel-buffer.c |  50 +
 6 files changed, 363 insertions(+)
 create mode 100644 include/io/channel-buffer.h
 create mode 100644 io/channel-buffer.c
 create mode 100644 tests/test-io-channel-buffer.c

diff --git a/include/io/channel-buffer.h b/include/io/channel-buffer.h
new file mode 100644
index 000..91a52b3
--- /dev/null
+++ b/include/io/channel-buffer.h
@@ -0,0 +1,60 @@
+/*
+ * QEMU I/O channels memory buffer driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_BUFFER_H__
+#define QIO_CHANNEL_BUFFER_H__
+
+#include "io/channel.h"
+
+#define TYPE_QIO_CHANNEL_BUFFER "qio-channel-buffer"
+#define QIO_CHANNEL_BUFFER(obj) \
+OBJECT_CHECK(QIOChannelBuffer, (obj), TYPE_QIO_CHANNEL_BUFFER)
+
+typedef struct QIOChannelBuffer QIOChannelBuffer;
+
+/**
+ * QIOChannelBuffer:
+ *
+ * The QIOChannelBuffer object provides a channel implementation
+ * that is able to perform I/O to/from a memory buffer.
+ *
+ */
+
+struct QIOChannelBuffer {
+QIOChannel parent;
+size_t capacity; /* Total allocated memory */
+size_t usage;/* Current size of data */
+size_t offset;   /* Offset for future I/O ops */
+char *data;
+};
+
+
+/**
+ * qio_channel_buffer_new:
+ * @capacity: the initial buffer capacity to allocate
+ *
+ * Allocate a new buffer which is initially empty
+ *
+ * Returns: the new channel object
+ */
+QIOChannelBuffer *
+qio_channel_buffer_new(size_t capacity);
+
+#endif /* QIO_CHANNEL_BUFFER_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
index 1a58ccb..0e3de31 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -1,4 +1,5 @@
 io-obj-y = channel.o
+io-obj-y += channel-buffer.o
 io-obj-y += channel-command.o
 io-obj-y += channel-file.o
 io-obj-y += channel-socket.o
diff --git a/io/channel-buffer.c b/io/channel-buffer.c
new file mode 100644
index 000..daebc92
--- /dev/null
+++ b/io/channel-buffer.c
@@ -0,0 +1,248 @@
+/*
+ * QEMU I/O channels memory buffer driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "io/channel-buffer.h"
+#include "io/channel-watch.h"
+#include "qemu/sockets.h"
+#include "trace.h"
+
+QIOChannelBuffer *
+qio_channel_buffer_new(size_t capacity)
+{
+QIOChannelBuffer *ioc;
+
+ioc = QIO_CHANNEL_BUFFER(object_new(TYPE_QIO_CHANNEL_BUFFER));
+
+if (capacity) {
+ioc->data = g_new0(char, capacity);
+ioc->capacity = capacity;
+}
+
+return ioc;
+}
+
+
+static void qio_channel_buffer_finalize(Object *obj)
+{
+QIOChannelBuffer *ioc = QIO_CHANNEL_BUFFER(obj);
+g_free(ioc->data);
+ioc->capacity = ioc->usage = ioc->offset = 0;
+}
+
+
+static ssize_t qio_channel_buffer_readv(QIOChannel *ioc,
+const struct iovec *iov,
+size_t niov,
+int **fds,
+size_t *nfds,
+Error **errp)
+{
+QIOChannelBuffer *bioc 

[Qemu-devel] [PATCH] scripts: provide a script for checking glib symbol usage

[Daniel P. Berrange]

There are frequently patches submitted for merge which use
symbols only defined by newer glib versions that the max
version currently permitted by QEMU (ie glib==2.22). The
glib 2.32 release introduced GLIB_VERSION_MIN_REQUIRED and
GLIB_VERSION_MAX_ALLOWED macros to allow apps to declare
what they want. Unfortunately QEMU has a glib-compat.h
file which backports certain functions from newer glib
which confuses the glib version checking macros. This is
effectively preventing their use in QEMU. The version check
macros also don't cover all versions back to 2.22 so any
checks are incomplete.

This patch takes a different approach, by providing a script
that can validate that source files are only using glib
functions & macros present in the minimum declared glib version,
by doing static analysis of the source and comparing to a
previously defined list of permitted symbols.

The list of functions/macros was extracted from the glib 2.22
HTML docs symbol index using

 $ grep '' /usr/share/gtk-doc/html/glib/ix01.html | \
awk {'print $1}' | \
sed -e 's///' -e 's/,//' | \
grep -E -i '^g_' | \
sort > scripts/glib-syms.txt

The script works by simply searching through the .c and .h
files looking for anything that looks like a function or
macro call, starting with a 'G_' or 'g_' prefix. This is
not a 100% exact science, but with a few override lists it
provides a practical/usable level of detection of mistakes.

This patch creates a 'scripts/Makefile' file which is intended
to accumulate various other style checks, similar to those done
by 'checkpatch.pl', but operating against the entire source
tree rather than just new patches.

The check can be invoked using 'make check-style' in the top
level directory.

Example output when the check fails would look like:

$ make check-style
tests/test-io-task.c:151: 'g_thread_ref' symbol not provided in min glib version
tests/test-io-task.c:172: 'g_thread_ref' symbol not provided in min glib version
tests/test-io-task.c:216: 'g_thread_unref' symbol not provided in min glib 
version
tests/test-io-task.c:217: 'g_thread_unref' symbol not provided in min glib 
version
tests/test-io-task.c:260: 'g_thread_unref' symbol not provided in min glib 
version
tests/test-io-task.c:261: 'g_thread_unref' symbol not provided in min glib 
version

Signed-off-by: Daniel P. Berrange 
---
 Makefile  |1 +
 scripts/Makefile  |   24 +
 scripts/glib-syms.pl  |  131 +
 scripts/glib-syms.txt | 1513 +
 4 files changed, 1669 insertions(+)
 create mode 100644 scripts/Makefile
 create mode 100644 scripts/glib-syms.pl
 create mode 100644 scripts/glib-syms.txt

diff --git a/Makefile b/Makefile
index af3e5f1..88a7649 100644
--- a/Makefile
+++ b/Makefile
@@ -165,6 +165,7 @@ dummy := $(call unnest-vars,, \
 
 ifneq ($(wildcard config-host.mak),)
 include $(SRC_PATH)/tests/Makefile
+include $(SRC_PATH)/scripts/Makefile
 endif
 
 all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all modules
diff --git a/scripts/Makefile b/scripts/Makefile
new file mode 100644
index 000..162e7e9
--- /dev/null
+++ b/scripts/Makefile
@@ -0,0 +1,24 @@
+#
+# This makefile runs various style checks across the entire
+# source tree.
+#
+# This is similar in concept of checkpatch.pl, but enforces
+# rules across the entire codebase, not just new patches
+#
+
+STYLE_CHECKS = \
+   cs-glib-syms
+
+ALL_FILES = \
+   $(shell git ls-tree -r HEAD . | awk '{print $$4}')
+
+C_CODE_FILES = $(filter %.c %.h, $(ALL_FILES))
+
+check-style: $(STYLE_CHECKS)
+
+# Check that we only use glib symbols present in our
+# minimum declared glib version
+GLIB_SYMS_LIST = scripts/glib-syms.txt
+
+cs-glib-syms:
+   @perl scripts/glib-syms.pl $(GLIB_SYMS_LIST) $(C_CODE_FILES)
diff --git a/scripts/glib-syms.pl b/scripts/glib-syms.pl
new file mode 100644
index 000..e4b50c1
--- /dev/null
+++ b/scripts/glib-syms.pl
@@ -0,0 +1,131 @@
+#!/usr/bin/perl
+#
+# Copyright (C) 2015 Red Hat Inc.
+#
+# This work is licensed under the terms of the GNU GPL, version 2
+# or later. See the COPYING file in the top-level directory.
+#
+# This parses *.c and *.h files in QEMU source tree to
+# extract a list of functions used from the glib
+# library and compares them to the list of symbols
+# present in the minimum required glib version.
+
+use strict;
+use warnings;
+
+# The file containing the list of glib symbols
+# Generated by from the glib 2.22 HTML docs index
+# of symbols file using
+#
+# $ grep '' /usr/share/gtk-doc/html/glib/ix01.html | \
+#awk {'print $1}' | \
+#sed -e 's///' -e 's/,//' | \
+#grep -E -i '^g_' | \
+#sort > scripts/glib-syms.txt
+#
+# Newer versions might need different grep/awk/sed rules
+my $syms = shift @ARGV;
+
+
+# Symbols not present in the release that we depend
+# on, but which have wrappers in include/glib-compat.h
+my @compatsyms = qw(
+g_get_monotonic_time
+
+g_assert_true
+g_assert_false
+

Re: [Qemu-devel] [PATCH v3 6/9] target-arm: support QMP dump-guest-memory

[Peter Maydell]

On 15 December 2015 at 22:51, Andrew Jones  wrote:
> Add the support needed for creating prstatus elf notes. This
> allows us to use QMP dump-guest-memory.
>
> Signed-off-by: Andrew Jones 

> +int arm_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> + int cpuid, void *opaque)
> +{
> +struct aarch64_note note;
> +CPUARMState *env = _CPU(cs)->env;
> +DumpState *s = opaque;
> +uint64_t pstate, sp;
> +int ret, i;
> +
> +aarch64_note_init(, s, "CORE", 5, NT_PRSTATUS, 
> sizeof(note.prstatus));
> +
> +note.prstatus.pr_pid = cpu_to_dump32(s, cpuid);
> +
> +if (!is_a64(env)) {
> +aarch64_sync_32_to_64(env);
> +pstate = cpsr_read(env);
> +sp = aarch64_compat_sp(env);

I don't understand why we need to do this. If this is an
AArch64 dump then we should just treat it as an AArch64
dump, and presumably the consumer of the dump knows enough
to know what the "hypervisor view" of a CPU that's currently
in 32-bit mode is. It has to anyway to be able to figure
out where all the other registers are, so why can't it
also figure out what mode the CPU is currently in and thus
where r13 is in the xregs array?

> +} else {
> +pstate = pstate_read(env);
> +sp = env->xregs[31];
> +}

thanks
-- PMM

[Qemu-devel] [PATCH] virtio-gpu: fix memory leak in error path

[Gerd Hoffmann]

Found by Coverity Scan, buf not freed on error.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/virtio-gpu-3d.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index 527ac90..bd88d45 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -197,7 +197,7 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
 qemu_log_mask(LOG_GUEST_ERROR, "%s: size mismatch (%zd/%d)",
   __func__, s, cs.size);
 cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
-return;
+goto out;
 }
 
 if (virtio_gpu_stats_enabled(g->conf)) {
@@ -207,6 +207,7 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
 
 virgl_renderer_submit_cmd(buf, cs.hdr.ctx_id, cs.size / 4);
 
+out:
 g_free(buf);
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH COLO-Frame v12 32/38] COLO: Split qemu_savevm_state_begin out of checkpoint process

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> It is unnecessary to call qemu_savevm_state_begin() in every checkponit 
> process.
> It mainly sets up devices and does the first device state pass. These data 
> will
> not change during the later checkpoint process. So, we split it out of
> colo_do_checkpoint_transaction(), in this way, we can reduce these data
> transferring in the later checkpoint.
> 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Li Zhijian 
> ---
>  migration/colo.c | 51 +--
>  1 file changed, 37 insertions(+), 14 deletions(-)
> 
> diff --git a/migration/colo.c b/migration/colo.c
> index d253d64..4571359 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -276,15 +276,6 @@ static int colo_do_checkpoint_transaction(MigrationState 
> *s,
>  if (ret < 0) {
>  goto out;
>  }
> -/* Disable block migration */
> -s->params.blk = 0;
> -s->params.shared = 0;
> -qemu_savevm_state_begin(s->to_dst_file, >params);
> -ret = qemu_file_get_error(s->to_dst_file);
> -if (ret < 0) {
> -error_report("save vm state begin error\n");
> -goto out;
> -}
>  
>  qemu_mutex_lock_iothread();
>  /* Note: device state is saved into buffer */
> @@ -348,6 +339,21 @@ out:
>  return ret;
>  }
>  
> +static int colo_prepare_before_save(MigrationState *s)
> +{
> +int ret;
> +/* Disable block migration */
> +s->params.blk = 0;
> +s->params.shared = 0;
> +qemu_savevm_state_begin(s->to_dst_file, >params);
> +ret = qemu_file_get_error(s->to_dst_file);
> +if (ret < 0) {
> +error_report("save vm state begin error\n");

 '\n' again not needed.

> +return ret;
> +}
> +return 0;
> +}
> +
>  static void colo_process_checkpoint(MigrationState *s)
>  {
>  QEMUSizedBuffer *buffer = NULL;
> @@ -363,6 +369,11 @@ static void colo_process_checkpoint(MigrationState *s)
>  goto out;
>  }
>  
> +ret = colo_prepare_before_save(s);
> +if (ret < 0) {
> +goto out;
> +}
> +
>  /*
>   * Wait for Secondary finish loading vm states and enter COLO
>   * restore.
> @@ -484,6 +495,18 @@ static int colo_wait_handle_cmd(QEMUFile *f, int 
> *checkpoint_request)
>  }
>  }
>  
> +static int colo_prepare_before_load(QEMUFile *f)
> +{
> +int ret;
> +
> +ret = qemu_loadvm_state_begin(f);
> +if (ret < 0) {
> +error_report("load vm state begin error, ret=%d", ret);
> +return ret;

You can simplify these returns; remove this line.

> +}
> +return 0;

and make this return ret; same in a few places.


Other than those minor issues;

Reviewed-by: Dr. David Alan Gilbert 


> +}
> +
>  void *colo_process_incoming_thread(void *opaque)
>  {
>  MigrationIncomingState *mis = opaque;
> @@ -522,6 +545,11 @@ void *colo_process_incoming_thread(void *opaque)
>  goto out;
>  }
>  
> +ret = colo_prepare_before_load(mis->from_src_file);
> +if (ret < 0) {
> +goto out;
> +}
> +
>  ret = colo_put_cmd(mis->to_src_file, COLO_COMMAND_CHECKPOINT_READY);
>  if (ret < 0) {
>  goto out;
> @@ -556,11 +584,6 @@ void *colo_process_incoming_thread(void *opaque)
>  goto out;
>  }
>  
> -ret = qemu_loadvm_state_begin(mis->from_src_file);
> -if (ret < 0) {
> -error_report("load vm state begin error, ret=%d", ret);
> -goto out;
> -}
>  ret = qemu_load_ram_state(mis->from_src_file);
>  if (ret < 0) {
>  error_report("load ram state error");
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 7/9] target-arm: dump-guest-memory: add prfpreg notes for aarch64

[Peter Maydell]

On 15 December 2015 at 22:51, Andrew Jones  wrote:
> Signed-off-by: Andrew Jones 
> ---
>  target-arm/arch_dump.c | 79 
> +-
>  1 file changed, 71 insertions(+), 8 deletions(-)

> +static int

We don't usually do the 'linebreak before function name' style.

> +aarch64_write_elf64_prfpreg(WriteCoreDumpFunction f, CPUARMState *env,
> +int cpuid, DumpState *s)
> +{
> +struct aarch64_note note;
> +int ret, i;
> +
> +aarch64_note_init(, s, "CORE", 5, NT_PRFPREG, sizeof(note.vfp));
> +
> +for (i = 0; i < 64; ++i) {
> +note.vfp.vregs[i] = cpu_to_dump64(s, float64_val(env->vfp.regs[i]));
> +}
> +
> +if (s->dump_info.d_endian == ELFDATA2MSB) {
> +/* For AArch4 we must always swap the vfp.regs's 2n and 2n+1

"AArch64".

> + * entries when generating BE notes, because even big endian
> + * hosts use 2n+1 for the high half.
> + */

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH] block: use drained section around bdrv_snapshot_delete

[Kevin Wolf]

[ Cc: qemu-block ]

Am 17.12.2015 um 02:55 hat Fam Zheng geschrieben:
> On Wed, 12/16 19:33, Paolo Bonzini wrote:
> > Do not use bdrv_drain, since by itself it does not guarantee
> > anything.
> > 
> > Signed-off-by: Paolo Bonzini 

Thanks, applied to the block branch.

Kevin

> >  block/snapshot.c | 23 ++-
> >  1 file changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/block/snapshot.c b/block/snapshot.c
> > index 6e9fa8d..2d86b88 100644
> > --- a/block/snapshot.c
> > +++ b/block/snapshot.c
> > @@ -229,6 +229,8 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
> >   Error **errp)
> >  {
> >  BlockDriver *drv = bs->drv;
> > +int ret;
> > +
> >  if (!drv) {
> >  error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, 
> > bdrv_get_device_name(bs));
> >  return -ENOMEDIUM;
> > @@ -239,18 +241,21 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
> >  }
> >  
> >  /* drain all pending i/o before deleting snapshot */
> > -bdrv_drain(bs);
> > +bdrv_drained_begin(bs);
> >  
> >  if (drv->bdrv_snapshot_delete) {
> > -return drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
> > +ret = drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
> > +} else if (bs->file) {
> > +ret = bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp);
> > +} else {
> > +error_setg(errp, "Block format '%s' used by device '%s' "
> > +   "does not support internal snapshot deletion",
> > +   drv->format_name, bdrv_get_device_name(bs));
> > +ret = -ENOTSUP;
> >  }
> > -if (bs->file) {
> > -return bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp);
> > -}
> > -error_setg(errp, "Block format '%s' used by device '%s' "
> > -   "does not support internal snapshot deletion",
> > -   drv->format_name, bdrv_get_device_name(bs));
> > -return -ENOTSUP;
> > +
> > +bdrv_drained_end(bs);
> > +return ret;
> >  }
> >  
> >  int bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
> > -- 
> > 2.5.0
> > 
> > 
> 
> Reviewed-by: Fam Zheng 
> 

Re: [Qemu-devel] [PATCH v3 3/9] dump: allow target to set the page size

[Peter Maydell]

On 15 December 2015 at 22:51, Andrew Jones  wrote:
> This is necessary for targets that don't have TARGET_PAGE_SIZE ==
> real-target-page-size. The target should set the page size to the
> correct one, if known, or, if not known, to the maximum page size
> it supports.
>
> (No functional change.)
>
> Signed-off-by: Andrew Jones 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH] block: fix bdrv_ioctl called from coroutine

[Kevin Wolf]

[ Cc: qemu-block ]

Am 16.12.2015 um 19:33 hat Paolo Bonzini geschrieben:
> When called from a coroutine, bdrv_ioctl must be asynchronous just like
> e.g. bdrv_flush.  The code was incorrectly making it synchronous, fix
> it.
> 
> Signed-off-by: Paolo Bonzini 

Thanks, applied to the block branch.

Kevin

[Qemu-devel] [PULL v4 6/9] io: add QIOChannelTLS class

[Daniel P. Berrange]

Add a QIOChannel subclass that can run the TLS protocol over
the top of another QIOChannel instance. The object provides a
simplified API to perform the handshake when starting the TLS
session. The layering of TLS over the underlying channel does
not have to be setup immediately. It is possible to take an
existing QIOChannel that has done some handshake and then swap
in the QIOChannelTLS layer. This allows for use with protocols
which start TLS right away, and those which start plain text
and then negotiate TLS.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-tls.h| 142 
 io/Makefile.objs|   1 +
 io/channel-tls.c| 393 
 tests/.gitignore|   1 +
 tests/Makefile  |   4 +
 tests/test-io-channel-tls.c | 342 ++
 trace-events|  10 ++
 7 files changed, 893 insertions(+)
 create mode 100644 include/io/channel-tls.h
 create mode 100644 io/channel-tls.c
 create mode 100644 tests/test-io-channel-tls.c

diff --git a/include/io/channel-tls.h b/include/io/channel-tls.h
new file mode 100644
index 000..0298b17
--- /dev/null
+++ b/include/io/channel-tls.h
@@ -0,0 +1,142 @@
+/*
+ * QEMU I/O channels TLS driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_TLS_H__
+#define QIO_CHANNEL_TLS_H__
+
+#include "io/channel.h"
+#include "io/task.h"
+#include "crypto/tlssession.h"
+
+#define TYPE_QIO_CHANNEL_TLS "qio-channel-tls"
+#define QIO_CHANNEL_TLS(obj) \
+OBJECT_CHECK(QIOChannelTLS, (obj), TYPE_QIO_CHANNEL_TLS)
+
+typedef struct QIOChannelTLS QIOChannelTLS;
+
+/**
+ * QIOChannelTLS
+ *
+ * The QIOChannelTLS class provides a channel wrapper which
+ * can transparently run the TLS encryption protocol. It is
+ * usually used over a TCP socket, but there is actually no
+ * technical restriction on which type of master channel is
+ * used as the transport.
+ *
+ * This channel object is capable of running as either a
+ * TLS server or TLS client.
+ */
+
+struct QIOChannelTLS {
+QIOChannel parent;
+QIOChannel *master;
+QCryptoTLSSession *session;
+};
+
+/**
+ * qio_channel_tls_new_server:
+ * @master: the underlying channel object
+ * @creds: the credentials to use for TLS handshake
+ * @aclname: the access control list for validating clients
+ * @errp: pointer to an uninitialized error object
+ *
+ * Create a new TLS channel that runs the server side of
+ * a TLS session. The TLS session handshake will use the
+ * credentials provided in @creds. If the @aclname parameter
+ * is non-NULL, then the client will have to provide
+ * credentials (ie a x509 client certificate) which will
+ * then be validated against the ACL.
+ *
+ * After creating the channel, it is mandatory to call
+ * the qio_channel_tls_handshake() method before attempting
+ * todo any I/O on the channel.
+ *
+ * Once the handshake has completed, all I/O should be done
+ * via the new TLS channel object and not the original
+ * master channel
+ *
+ * Returns: the new TLS channel object, or NULL
+ */
+QIOChannelTLS *
+qio_channel_tls_new_server(QIOChannel *master,
+   QCryptoTLSCreds *creds,
+   const char *aclname,
+   Error **errp);
+
+/**
+ * qio_channel_tls_new_client:
+ * @master: the underlying channel object
+ * @creds: the credentials to use for TLS handshake
+ * @hostname: the user specified server hostname
+ * @errp: pointer to an uninitialized error object
+ *
+ * Create a new TLS channel that runs the client side of
+ * a TLS session. The TLS session handshake will use the
+ * credentials provided in @creds. The @hostname parameter
+ * should provide the user specified hostname of the server
+ * and will be validated against the server's credentials
+ * (ie CommonName of the x509 certificate)
+ *
+ * After creating the channel, it is mandatory to call
+ * the qio_channel_tls_handshake() method before attempting
+ * todo any I/O on the channel.
+ *
+ * Once the handshake has completed, all I/O should be done
+ * via the new TLS channel object and not the original
+ * master channel
+ *
+ * Returns: the new TLS channel object, or NULL
+ */

[Qemu-devel] [PULL v4 0/9] Introduce I/O channels framework

[Daniel P. Berrange]

The following changes since commit 6a6533213d78dea4407fe6933ad489796b582599:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2015-12-17 18:07:09 +)

are available in the git repository at:

  git://github.com/berrange/qemu tags/pull-io-channel-base-2015-12-18-1

for you to fetch changes up to d98e4eb7de93290f7921b0dbe869c7dd3c567945:

  io: add QIOChannelBuffer class (2015-12-18 12:18:31 +)


Merge I/O channels base classes


Daniel P. Berrange (9):
  io: add abstract QIOChannel classes
  io: add helper module for creating watches on FDs
  io: add QIOTask class for async operations
  io: add QIOChannelSocket class
  io: add QIOChannelFile class
  io: add QIOChannelTLS class
  io: add QIOChannelWebsock class
  io: add QIOChannelCommand class
  io: add QIOChannelBuffer class

 MAINTAINERS |   7 +
 Makefile|   2 +
 Makefile.objs   |   5 +
 Makefile.target |   2 +
 configure   |  11 +
 include/io/channel-buffer.h |  60 +++
 include/io/channel-command.h|  91 
 include/io/channel-file.h   |  93 
 include/io/channel-socket.h | 244 ++
 include/io/channel-tls.h| 142 ++
 include/io/channel-watch.h  |  72 +++
 include/io/channel-websock.h| 108 +
 include/io/channel.h| 502 +
 include/io/task.h   | 256 +++
 include/qemu/sockets.h  |  19 +
 io/Makefile.objs|   9 +
 io/channel-buffer.c | 248 +++
 io/channel-command.c| 357 +++
 io/channel-file.c   | 225 ++
 io/channel-socket.c | 741 +++
 io/channel-tls.c| 393 
 io/channel-watch.c  | 198 +
 io/channel-websock.c| 962 
 io/channel.c| 291 
 io/task.c   | 159 +++
 scripts/create_config   |   9 +
 tests/.gitignore|   8 +
 tests/Makefile  |  19 +
 tests/io-channel-helpers.c  | 246 ++
 tests/io-channel-helpers.h  |  42 ++
 tests/test-io-channel-buffer.c  |  50 +++
 tests/test-io-channel-command.c | 129 ++
 tests/test-io-channel-file.c| 100 +
 tests/test-io-channel-socket.c  | 399 +
 tests/test-io-channel-tls.c | 342 ++
 tests/test-io-task.c| 268 +++
 trace-events|  56 +++
 util/qemu-sockets.c |   2 +-
 38 files changed, 6866 insertions(+), 1 deletion(-)
 create mode 100644 include/io/channel-buffer.h
 create mode 100644 include/io/channel-command.h
 create mode 100644 include/io/channel-file.h
 create mode 100644 include/io/channel-socket.h
 create mode 100644 include/io/channel-tls.h
 create mode 100644 include/io/channel-watch.h
 create mode 100644 include/io/channel-websock.h
 create mode 100644 include/io/channel.h
 create mode 100644 include/io/task.h
 create mode 100644 io/Makefile.objs
 create mode 100644 io/channel-buffer.c
 create mode 100644 io/channel-command.c
 create mode 100644 io/channel-file.c
 create mode 100644 io/channel-socket.c
 create mode 100644 io/channel-tls.c
 create mode 100644 io/channel-watch.c
 create mode 100644 io/channel-websock.c
 create mode 100644 io/channel.c
 create mode 100644 io/task.c
 create mode 100644 tests/io-channel-helpers.c
 create mode 100644 tests/io-channel-helpers.h
 create mode 100644 tests/test-io-channel-buffer.c
 create mode 100644 tests/test-io-channel-command.c
 create mode 100644 tests/test-io-channel-file.c
 create mode 100644 tests/test-io-channel-socket.c
 create mode 100644 tests/test-io-channel-tls.c
 create mode 100644 tests/test-io-task.c

-- 
2.5.0

[Qemu-devel] [PULL v4 2/9] io: add helper module for creating watches on FDs

[Daniel P. Berrange]

A number of the channel implementations will require the
ability to create watches on file descriptors / sockets.
To avoid duplicating this code in each channel, provide a
helper API for dealing with file descriptor watches.

There are two watch implementations provided. The first
is useful for bi-directional file descriptors such as
sockets, regular files, character devices, etc. The
second works with a pair of unidirectional file descriptors
such as pipes.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-watch.h |  72 +
 io/Makefile.objs   |   1 +
 io/channel-watch.c | 198 +
 3 files changed, 271 insertions(+)
 create mode 100644 include/io/channel-watch.h
 create mode 100644 io/channel-watch.c

diff --git a/include/io/channel-watch.h b/include/io/channel-watch.h
new file mode 100644
index 000..656358a
--- /dev/null
+++ b/include/io/channel-watch.h
@@ -0,0 +1,72 @@
+/*
+ * QEMU I/O channels watch helper APIs
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_WATCH_H__
+#define QIO_CHANNEL_WATCH_H__
+
+#include "io/channel.h"
+
+/*
+ * This module provides helper functions that will be needed by
+ * the various QIOChannel implementations, for creating watches
+ * on file descriptors / sockets
+ */
+
+/**
+ * qio_channel_create_fd_watch:
+ * @ioc: the channel object
+ * @fd: the file descriptor
+ * @condition: the I/O condition
+ *
+ * Create a new main loop source that is able to
+ * monitor the file descriptor @fd for the
+ * I/O conditions in @condition. This is able
+ * monitor block devices, character devices,
+ * sockets, pipes but not plain files.
+ *
+ * Returns: the new main loop source
+ */
+GSource *qio_channel_create_fd_watch(QIOChannel *ioc,
+ int fd,
+ GIOCondition condition);
+
+/**
+ * qio_channel_create_fd_pair_watch:
+ * @ioc: the channel object
+ * @fdread: the file descriptor for reading
+ * @fdwrite: the file descriptor for writing
+ * @condition: the I/O condition
+ *
+ * Create a new main loop source that is able to
+ * monitor the pair of file descriptors @fdread
+ * and @fdwrite for the I/O conditions in @condition.
+ * This is intended for monitoring unidirectional
+ * file descriptors such as pipes, where a pair
+ * of descriptors is required for bidirectional
+ * I/O
+ *
+ * Returns: the new main loop source
+ */
+GSource *qio_channel_create_fd_pair_watch(QIOChannel *ioc,
+  int fdread,
+  int fdwrite,
+  GIOCondition condition);
+
+#endif /* QIO_CHANNEL_WATCH_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
index a6ed361..b02ea90 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -1 +1,2 @@
 io-obj-y = channel.o
+io-obj-y += channel-watch.o
diff --git a/io/channel-watch.c b/io/channel-watch.c
new file mode 100644
index 000..2f745f1
--- /dev/null
+++ b/io/channel-watch.c
@@ -0,0 +1,198 @@
+/*
+ * QEMU I/O channels watch helper APIs
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "io/channel-watch.h"
+
+typedef struct QIOChannelFDSource QIOChannelFDSource;
+struct QIOChannelFDSource {
+GSource parent;
+GPollFD fd;
+QIOChannel *ioc;
+GIOCondition condition;
+};
+
+
+typedef struct QIOChannelFDPairSource QIOChannelFDPairSource;
+struct QIOChannelFDPairSource {
+GSource parent;
+GPollFD fdread;
+GPollFD fdwrite;
+QIOChannel *ioc;
+GIOCondition condition;
+};
+

[Qemu-devel] [PULL v4 8/9] io: add QIOChannelCommand class

[Daniel P. Berrange]

Add a QIOChannel subclass that is capable of performing I/O
to/from a separate process, via a pair of pipes. The command
can be used for unidirectional or bi-directional I/O.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-command.h|  91 ++
 io/Makefile.objs|   1 +
 io/channel-command.c| 357 
 tests/.gitignore|   2 +
 tests/Makefile  |   3 +
 tests/test-io-channel-command.c | 129 +++
 trace-events|   6 +
 7 files changed, 589 insertions(+)
 create mode 100644 include/io/channel-command.h
 create mode 100644 io/channel-command.c
 create mode 100644 tests/test-io-channel-command.c

diff --git a/include/io/channel-command.h b/include/io/channel-command.h
new file mode 100644
index 000..bd3c599
--- /dev/null
+++ b/include/io/channel-command.h
@@ -0,0 +1,91 @@
+/*
+ * QEMU I/O channels external command driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_COMMAND_H__
+#define QIO_CHANNEL_COMMAND_H__
+
+#include "io/channel.h"
+
+#define TYPE_QIO_CHANNEL_COMMAND "qio-channel-command"
+#define QIO_CHANNEL_COMMAND(obj) \
+OBJECT_CHECK(QIOChannelCommand, (obj), TYPE_QIO_CHANNEL_COMMAND)
+
+typedef struct QIOChannelCommand QIOChannelCommand;
+
+
+/**
+ * QIOChannelCommand:
+ *
+ * The QIOChannelCommand class provides a channel implementation
+ * that can transport data with an externally running command
+ * via its stdio streams.
+ */
+
+struct QIOChannelCommand {
+QIOChannel parent;
+int writefd;
+int readfd;
+pid_t pid;
+};
+
+
+/**
+ * qio_channel_command_new_pid:
+ * @writefd: the FD connected to the command's stdin
+ * @readfd: the FD connected to the command's stdout
+ * @pid: the PID of the running child command
+ * @errp: pointer to an uninitialized error object
+ *
+ * Create a channel for performing I/O with the
+ * previously spawned command identified by @pid.
+ * The two file descriptors provide the connection
+ * to command's stdio streams, either one or which
+ * may be -1 to indicate that stream is not open.
+ *
+ * The channel will take ownership of the process
+ * @pid and will kill it when closing the channel.
+ * Similarly it will take responsibility for
+ * closing the file descriptors @writefd and @readfd.
+ *
+ * Returns: the command channel object, or NULL on error
+ */
+QIOChannelCommand *
+qio_channel_command_new_pid(int writefd,
+int readfd,
+pid_t pid);
+
+/**
+ * qio_channel_command_new_spawn:
+ * @argv: the NULL terminated list of command arguments
+ * @flags: the I/O mode, one of O_RDONLY, O_WRONLY, O_RDWR
+ * @errp: pointer to an uninitialized error object
+ *
+ * Create a channel for performing I/O with the
+ * command to be spawned with arguments @argv.
+ *
+ * Returns: the command channel object, or NULL on error
+ */
+QIOChannelCommand *
+qio_channel_command_new_spawn(const char *const argv[],
+  int flags,
+  Error **errp);
+
+
+#endif /* QIO_CHANNEL_COMMAND_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
index e3771b1..1a58ccb 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -1,4 +1,5 @@
 io-obj-y = channel.o
+io-obj-y += channel-command.o
 io-obj-y += channel-file.o
 io-obj-y += channel-socket.o
 io-obj-y += channel-tls.o
diff --git a/io/channel-command.c b/io/channel-command.c
new file mode 100644
index 000..598fdab
--- /dev/null
+++ b/io/channel-command.c
@@ -0,0 +1,357 @@
+/*
+ * QEMU I/O channels external command driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy 

[Qemu-devel] [PULL v4 7/9] io: add QIOChannelWebsock class

[Daniel P. Berrange]

Add a QIOChannel subclass that can run the websocket protocol over
the top of another QIOChannel instance. This initial implementation
is only capable of acting as a websockets server. There is no support
for acting as a websockets client yet.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-websock.h | 108 +
 io/Makefile.objs |   1 +
 io/channel-websock.c | 962 +++
 trace-events |   8 +
 4 files changed, 1079 insertions(+)
 create mode 100644 include/io/channel-websock.h
 create mode 100644 io/channel-websock.c

diff --git a/include/io/channel-websock.h b/include/io/channel-websock.h
new file mode 100644
index 000..0dc21cc
--- /dev/null
+++ b/include/io/channel-websock.h
@@ -0,0 +1,108 @@
+/*
+ * QEMU I/O channels driver websockets
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_WEBSOCK_H__
+#define QIO_CHANNEL_WEBSOCK_H__
+
+#include "io/channel.h"
+#include "qemu/buffer.h"
+#include "io/task.h"
+
+#define TYPE_QIO_CHANNEL_WEBSOCK "qio-channel-websock"
+#define QIO_CHANNEL_WEBSOCK(obj) \
+OBJECT_CHECK(QIOChannelWebsock, (obj), TYPE_QIO_CHANNEL_WEBSOCK)
+
+typedef struct QIOChannelWebsock QIOChannelWebsock;
+typedef union QIOChannelWebsockMask QIOChannelWebsockMask;
+
+union QIOChannelWebsockMask {
+char c[4];
+uint32_t u;
+};
+
+/**
+ * QIOChannelWebsock
+ *
+ * The QIOChannelWebsock class provides a channel wrapper which
+ * can transparently run the HTTP websockets protocol. This is
+ * usually used over a TCP socket, but there is actually no
+ * technical restriction on which type of master channel is
+ * used as the transport.
+ *
+ * This channel object is currently only capable of running as
+ * a websocket server and is a pretty crude implementation
+ * of it, not supporting the full websockets protocol feature
+ * set. It is sufficient to use with a simple websockets
+ * client for encapsulating VNC for noVNC in-browser client.
+ */
+
+struct QIOChannelWebsock {
+QIOChannel parent;
+QIOChannel *master;
+Buffer encinput;
+Buffer encoutput;
+Buffer rawinput;
+Buffer rawoutput;
+size_t payload_remain;
+QIOChannelWebsockMask mask;
+guint io_tag;
+Error *io_err;
+gboolean io_eof;
+};
+
+/**
+ * qio_channel_websock_new_server:
+ * @master: the underlying channel object
+ *
+ * Create a new websockets channel that runs the server
+ * side of the protocol.
+ *
+ * After creating the channel, it is mandatory to call
+ * the qio_channel_websock_handshake() method before attempting
+ * todo any I/O on the channel.
+ *
+ * Once the handshake has completed, all I/O should be done
+ * via the new websocket channel object and not the original
+ * master channel
+ *
+ * Returns: the new websockets channel object
+ */
+QIOChannelWebsock *
+qio_channel_websock_new_server(QIOChannel *master);
+
+/**
+ * qio_channel_websock_handshake:
+ * @ioc: the websocket channel object
+ * @func: the callback to invoke when completed
+ * @opaque: opaque data to pass to @func
+ * @destroy: optional callback to free @opaque
+ *
+ * Perform the websocket handshake. This method
+ * will return immediately and the handshake will
+ * continue in the background, provided the main
+ * loop is running. When the handshake is complete,
+ * or fails, the @func callback will be invoked.
+ */
+void qio_channel_websock_handshake(QIOChannelWebsock *ioc,
+   QIOTaskFunc func,
+   gpointer opaque,
+   GDestroyNotify destroy);
+
+#endif /* QIO_CHANNEL_WEBSOCK_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
index a48011b..e3771b1 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -3,4 +3,5 @@ io-obj-y += channel-file.o
 io-obj-y += channel-socket.o
 io-obj-y += channel-tls.o
 io-obj-y += channel-watch.o
+io-obj-y += channel-websock.o
 io-obj-y += task.o
diff --git a/io/channel-websock.c b/io/channel-websock.c
new file mode 100644
index 000..9273a8b
--- /dev/null
+++ b/io/channel-websock.c
@@ -0,0 +1,962 @@
+/*
+ * QEMU I/O channels driver websockets
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can 

[Qemu-devel] [Bug 1525682] Re: configure: fix POSIX compatibility issue

[Thorsten Glaser]

Note that mksh is virtually a superset of OpenBSD ksh and accepts this
construct, for a quick fix.

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

Title:
  configure: fix POSIX compatibility issue

Status in QEMU:
  New

Bug description:
  When running configure script from 2.5.0-rc4 on OpenBSD-current
  (amd64), I get the following error:

./configure[4756]: ${nettle:+($nettle_version)}": bad substitution
*** Error 1 in . (/usr/ports/infrastructure/mk/bsd.port.mk:2747 
'/usr/ports/pobj/qemu-2.5.0rc4/.configure_done')
*** Error 1 in /usr/ports/openbsd-wip/emulators/qemu 
(/usr/ports/infrastructure/mk/bsd.port.mk:2491 'configure')

  Indeed, construct "${nettle:+($nettle_version)}" does not conform to
  POSIX Shell Command Language. The attached patch fixes the issue.

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

Re: [Qemu-devel] [PATCH] scripts: provide a script for checking glib symbol usage

[Paolo Bonzini]

On 18/12/2015 12:36, Daniel P. Berrange wrote:
> diff --git a/scripts/Makefile b/scripts/Makefile
> new file mode 100644
> index 000..162e7e9
> --- /dev/null
> +++ b/scripts/Makefile
> @@ -0,0 +1,24 @@
> +#
> +# This makefile runs various style checks across the entire
> +# source tree.
> +#
> +# This is similar in concept of checkpatch.pl, but enforces
> +# rules across the entire codebase, not just new patches
> +#
> +
> +STYLE_CHECKS = \
> + cs-glib-syms
> +
> +ALL_FILES = \
> + $(shell git ls-tree -r HEAD . | awk '{print $$4}')
> +
> +C_CODE_FILES = $(filter %.c %.h, $(ALL_FILES))
> +
> +check-style: $(STYLE_CHECKS)
> +
> +# Check that we only use glib symbols present in our
> +# minimum declared glib version
> +GLIB_SYMS_LIST = scripts/glib-syms.txt
> +
> +cs-glib-syms:
> + @perl scripts/glib-syms.pl $(GLIB_SYMS_LIST) $(C_CODE_FILES)


Does this need to be included, or could it be a separate Makefile
invoked with e.g. make -f scripts/Makefile.style?

> +# Symbols not present in the release that we depend
> +# on, but which have wrappers in include/glib-compat.h
> +my @compatsyms = qw(
> +g_get_monotonic_time
> +
> +g_assert_true
> +g_assert_false
> +g_assert_null
> +g_assert_nonnull
> +g_assert_cmpmem
> +
> +g_hash_table_add
> +
> +g_cond_clear
> +g_cond_init
> +g_cond_wait_until
> +
> +g_mutex_init
> +g_mutex_clear
> +
> +g_thread_new
> +
> +g_private_replace
> +G_PRIVATE_INIT
> +
> +G_TIME_SPAN_SECOND
> +);
> +
> +
> +# Functions defined inside QEMU which are using the
> +# the same "g_" function name prefix as glib, so
> +# get mis-detected as glib symbols
> +my @blacklist = qw(
> +g_to_float64
> +g_assert_no_errno
> +g_cclosure_new_swap
> +g_free_rcu
> +g_test_trap_subprocess
> +g_poll_fixed
> +g_list_insert_sorted_merged
> +G_BYTE
> +);
> +
> +# GObject stuff used by gtk frontend
> +my @gobjectsums = qw(
> +g_object_ref
> +g_object_unref
> +g_object_set_data
> +g_signal_connect
> +G_CALLBACK
> +);
> +
> +# Functions defined by glib which are strangely
> +# missing from their docs header index
> +my @missingindex = qw(
> +g_assertion_message
> +g_assertion_message_expr
> +g_assertion_message_cmpstr
> +g_assertion_message_cmpnum
> +g_assertion_message_error
> +);
> +

Can we "parse" #.* as comments, and put these in a glib-syms-extra.txt
file?  Then we can hypothetically do the same tests in checkpatch.pl too.

I'm not opposing the scripts/Makefile concept, and I'm not asking you to
patch checkpatch.pl; I just would like to keep the door open for that.

Paolo

Re: [Qemu-devel] [PATCH] ccid: use libcacard.h from 2.5.1

[Paolo Bonzini]

On 18/12/2015 10:49, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> This cleans up a bit libcacard headers inclusion.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  configure   | 7 ---
>  hw/usb/ccid-card-emulated.c | 5 +
>  hw/usb/ccid-card-passthru.c | 2 +-
>  3 files changed, 6 insertions(+), 8 deletions(-)

Are there any distros that have not packaged libcacard yet?  This will
break compatibility with QEMU 2.4's libcacard, I think.

Paolo

Re: [Qemu-devel] [PATCH] ccid: use libcacard.h from 2.5.1

[Marc-André Lureau]

Hi

- Original Message -
> 
> 
> On 18/12/2015 10:49, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> > 
> > This cleans up a bit libcacard headers inclusion.
> > 
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  configure   | 7 ---
> >  hw/usb/ccid-card-emulated.c | 5 +
> >  hw/usb/ccid-card-passthru.c | 2 +-
> >  3 files changed, 6 insertions(+), 8 deletions(-)
> 
> Are there any distros that have not packaged libcacard yet?  This will
> break compatibility with QEMU 2.4's libcacard, I think.

I bumped libcacard requirement to 2.5.1 in the patch.

Re: [Qemu-devel] [PATCH v2] Use macro instead of plain text

[Markus Armbruster]

Cao jin  writes:

> There is TYPE_ICH9_AHCI definition in ahci.h when QOMify it, seems it
> is missed.
>
> Signed-off-by: Cao jin 
> ---
>  hw/i386/pc_q35.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 9a12068..aa34a07 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -249,7 +249,7 @@ static void pc_q35_init(MachineState *machine)
>  ahci = pci_create_simple_multifunction(host_bus,
> PCI_DEVFN(ICH9_SATA1_DEV,
>   ICH9_SATA1_FUNC),
> -   true, "ich9-ahci");
> +   true, TYPE_ICH9_AHCI);
>  idebus[0] = qdev_get_child_bus(>qdev, "ide.0");
>  idebus[1] = qdev_get_child_bus(>qdev, "ide.1");
>  g_assert(MAX_SATA_PORTS == ICH_AHCI(ahci)->ahci.ports);

I suspect there are more instances of the same problem.

To find all similar macro definitions:

$ git-grep -Eh '^ *# *define +TYPE_[A-Za-z0-9_]+ +"'

Extract the strings:

$ git-grep -Eh '^ *# *define +TYPE_[A-Za-z0-9_]+ +"' | sed 
's/^[^"]*\("[^"]*"\).*/\1/'

Search the source for them:

$ for i in `git-grep -Eh '^ *# *define +TYPE_[A-Za-z0-9_]+ +"' | sed 
's/^[^"]*\("[^"]*"\).*/\1/'`; do echo "= $i ="; git-grep -F "$i"; done

Results in more than 1000 hits for me.  Names like "device" are of
course very prone to false positives.

Not sure how interested we are in consistent use of these macros...

Re: [Qemu-devel] [PATCH] Convert to realize()

[Markus Armbruster]

Copying qemu-trivial.

Cao jin  writes:

> for educational PCI device
>
> Signed-off-by: Cao jin 
> ---
>  hw/misc/edu.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index fe50b42..43d5b18 100644
> --- a/hw/misc/edu.c
> +++ b/hw/misc/edu.c
> @@ -327,7 +327,7 @@ static void *edu_fact_thread(void *opaque)
>  return NULL;
>  }
>  
> -static int pci_edu_init(PCIDevice *pdev)
> +static void pci_edu_realize(PCIDevice *pdev, Error **errp)
>  {
>  EduState *edu = DO_UPCAST(EduState, pdev, pdev);
>  uint8_t *pci_conf = pdev->config;
> @@ -344,8 +344,6 @@ static int pci_edu_init(PCIDevice *pdev)
>  memory_region_init_io(>mmio, OBJECT(edu), _mmio_ops, edu,
>  "edu-mmio", 1 << 20);
>  pci_register_bar(pdev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, >mmio);
> -
> -return 0;
>  }
>  
>  static void pci_edu_uninit(PCIDevice *pdev)
> @@ -385,7 +383,7 @@ static void edu_class_init(ObjectClass *class, void *data)
>  {
>  PCIDeviceClass *k = PCI_DEVICE_CLASS(class);
>  
> -k->init = pci_edu_init;
> +k->realize = pci_edu_realize;
>  k->exit = pci_edu_uninit;
>  k->vendor_id = PCI_VENDOR_ID_QEMU;
>  k->device_id = 0x11e8;

Reviewed-by: Markus Armbruster 

[Qemu-devel] [PATCH] scripts/checkpatch.pl: Don't allow special cases of unspaced operators

[Peter Maydell]

The checkpatch.pl script has a special case to permit the following
operators to have no spaces around them:
 <<  >>  &  ^  |  +  -  *  /  %

QEMU style prefers all operators to consistently have spacing around
them, so remove this special case handling. This avoids reviewers
having to manually note it during code review.

Signed-off-by: Peter Maydell 
---
I actually thought failing to point out the lack of spaces was
a checkpatch parsing bug until I looked in the source and found it
was deliberate...

I say "QEMU style prefers", but possibly what I actually mean is
"I prefer" ? Does anybody want to defend the unspaced versions?

 scripts/checkpatch.pl | 13 -
 1 file changed, 13 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b0f6e11..efca817 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1890,19 +1890,6 @@ sub process {
ERROR("space prohibited after 
that '$op' $at\n" . $hereptr);
}
 
-
-   # << and >> may either have or not have spaces 
both sides
-   } elsif ($op eq '<<' or $op eq '>>' or
-$op eq '&' or $op eq '^' or $op eq '|' 
or
-$op eq '+' or $op eq '-' or
-$op eq '*' or $op eq '/' or
-$op eq '%')
-   {
-   if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) {
-   ERROR("need consistent spacing 
around '$op' $at\n" .
-   $hereptr);
-   }
-
# A colon needs no spaces before when it is
# terminating a case value or a label.
} elsif ($opv eq ':C' || $opv eq ':L') {
-- 
1.9.1

Re: [Qemu-devel] [PATCH] misc: spelling

[Markus Armbruster]

Copying maintainer just for completeness.  I'm sure Luiz won't at all
mind this going through trivial ;)

Let's clarify the commit message:

monitor: Spelling fix

With that:

Reviewed-by: Markus Armbruster 

marcandre.lur...@redhat.com writes:

> From: Marc-André Lureau 
>
> ---
>  monitor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/monitor.c b/monitor.c
> index e7e7ae2..51ec4c3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -311,7 +311,7 @@ static void monitor_flush_locked(Monitor *mon)
>  return;
>  }
>  if (rc > 0) {
> -/* partinal write */
> +/* partial write */
>  QString *tmp = qstring_from_str(buf + rc);
>  QDECREF(mon->outbuf);
>  mon->outbuf = tmp;

Re: [Qemu-devel] [PATCH 1/2] compat: Introduce HW_COMPAT_2_5

[Paolo Bonzini]

On 18/12/2015 10:37, Cornelia Huck wrote:
> On Fri, 18 Dec 2015 09:30:02 +0200
> Shmulik Ladkani  wrote:
> 
>> Introduce the place-holder for 2.5 back-compat properties, and the
>> accompanying PC_COMPAT_2_5, CCW_COMPAT_2_5, SPAPR_COMPAT_2_5.
>>
>> Signed-off-by: Shmulik Ladkani 
>> ---
>>  hw/i386/pc_piix.c  | 1 +
>>  hw/i386/pc_q35.c   | 1 +
>>  hw/ppc/spapr.c | 9 +
>>  hw/s390x/s390-virtio-ccw.c | 9 +
>>  include/hw/compat.h| 3 +++
>>  include/hw/i386/pc.h   | 4 
>>  6 files changed, 27 insertions(+)
>>
> 
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 5a52ff2..3d79654 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -235,7 +235,11 @@ static const TypeInfo ccw_machine_info = {
>>  },
>>  };
>>
>> +#define CCW_COMPAT_2_5 \
>> +HW_COMPAT_2_5
>> +
>>  #define CCW_COMPAT_2_4 \
>> +CCW_COMPAT_2_5 \
>>  HW_COMPAT_2_4 \
>>  {\
>>  .driver   = TYPE_S390_SKEYS,\
>> @@ -296,10 +300,15 @@ static const TypeInfo ccw_machine_2_4_info = {
>>  static void ccw_machine_2_5_class_init(ObjectClass *oc, void *data)
>>  {
>>  MachineClass *mc = MACHINE_CLASS(oc);
>> +static GlobalProperty compat_props[] = {
>> +CCW_COMPAT_2_5
>> +{ /* end of list */ }
>> +};
>>
>>  mc->alias = "s390-ccw-virtio";
>>  mc->desc = "VirtIO-ccw based S390 machine v2.5";
>>  mc->is_default = 1;
>> +mc->compat_props = compat_props;
>>  }
>>
>>  static const TypeInfo ccw_machine_2_5_info = {
> 
> s390x part:
> 
> Acked-by: Cornelia Huck 
> 
>  machine, but nothing that isn't sorted out easily>
> 

Feel free to include these in your pull request if it's coming before
say January 6th.

Paolo

Re: [Qemu-devel] [PATCH v2] Use macro instead of plain text

[Peter Maydell]

On 18 December 2015 at 12:52, Markus Armbruster  wrote:
> Cao jin  writes:
>
>> There is TYPE_ICH9_AHCI definition in ahci.h when QOMify it, seems it
>> is missed.

> I suspect there are more instances of the same problem.

> Not sure how interested we are in consistent use of these macros...

In a lot of cases we'd need to find somewhere appropriate to put the
TYPE_* macro which might well mean creating a new header file just
for the purpose. That seems like overkill...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Don't allow special cases of unspaced operators

[Paolo Bonzini]

On 18/12/2015 13:59, Peter Maydell wrote:
> The checkpatch.pl script has a special case to permit the following
> operators to have no spaces around them:
>  <<  >>  &  ^  |  +  -  *  /  %
> 
> QEMU style prefers all operators to consistently have spacing around
> them, so remove this special case handling. This avoids reviewers
> having to manually note it during code review.
> 
> Signed-off-by: Peter Maydell 
> ---
> I actually thought failing to point out the lack of spaces was
> a checkpatch parsing bug until I looked in the source and found it
> was deliberate...
> 
> I say "QEMU style prefers", but possibly what I actually mean is
> "I prefer" ? Does anybody want to defend the unspaced versions?

No thanks.  Let's save to bikeshedding for important stuff such as line
lengths.  :)

Paolo

>  scripts/checkpatch.pl | 13 -
>  1 file changed, 13 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index b0f6e11..efca817 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1890,19 +1890,6 @@ sub process {
>   ERROR("space prohibited after 
> that '$op' $at\n" . $hereptr);
>   }
>  
> -
> - # << and >> may either have or not have spaces 
> both sides
> - } elsif ($op eq '<<' or $op eq '>>' or
> -  $op eq '&' or $op eq '^' or $op eq '|' 
> or
> -  $op eq '+' or $op eq '-' or
> -  $op eq '*' or $op eq '/' or
> -  $op eq '%')
> - {
> - if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) {
> - ERROR("need consistent spacing 
> around '$op' $at\n" .
> - $hereptr);
> - }
> -
>   # A colon needs no spaces before when it is
>   # terminating a case value or a label.
>   } elsif ($opv eq ':C' || $opv eq ':L') {
> 

Re: [Qemu-devel] [PATCH] ccid: use libcacard.h from 2.5.1

[Paolo Bonzini]

On 18/12/2015 13:50, Marc-André Lureau wrote:
>>> > >  configure   | 7 ---
>>> > >  hw/usb/ccid-card-emulated.c | 5 +
>>> > >  hw/usb/ccid-card-passthru.c | 2 +-
>>> > >  3 files changed, 6 insertions(+), 8 deletions(-)
>> > 
>> > Are there any distros that have not packaged libcacard yet?  This will
>> > break compatibility with QEMU 2.4's libcacard, I think.
> I bumped libcacard requirement to 2.5.1 in the patch.

Yes, the question is whether distributions would have problems with that.

Paolo

Re: [Qemu-devel] [PATCH] scripts: provide a script for checking glib symbol usage

[Daniel P. Berrange]

On Fri, Dec 18, 2015 at 01:36:00PM +0100, Paolo Bonzini wrote:
> 
> 
> On 18/12/2015 12:36, Daniel P. Berrange wrote:
> > diff --git a/scripts/Makefile b/scripts/Makefile
> > new file mode 100644
> > index 000..162e7e9
> > --- /dev/null
> > +++ b/scripts/Makefile
> > @@ -0,0 +1,24 @@
> > +#
> > +# This makefile runs various style checks across the entire
> > +# source tree.
> > +#
> > +# This is similar in concept of checkpatch.pl, but enforces
> > +# rules across the entire codebase, not just new patches
> > +#
> > +
> > +STYLE_CHECKS = \
> > +   cs-glib-syms
> > +
> > +ALL_FILES = \
> > +   $(shell git ls-tree -r HEAD . | awk '{print $$4}')
> > +
> > +C_CODE_FILES = $(filter %.c %.h, $(ALL_FILES))
> > +
> > +check-style: $(STYLE_CHECKS)
> > +
> > +# Check that we only use glib symbols present in our
> > +# minimum declared glib version
> > +GLIB_SYMS_LIST = scripts/glib-syms.txt
> > +
> > +cs-glib-syms:
> > +   @perl scripts/glib-syms.pl $(GLIB_SYMS_LIST) $(C_CODE_FILES)
> 
> 
> Does this need to be included, or could it be a separate Makefile
> invoked with e.g. make -f scripts/Makefile.style?

Any particular reason to favour that over include ? I did it this
way because QEMU in general seems to be biased towards includes
and not recursive make

> > +# Symbols not present in the release that we depend
> > +# on, but which have wrappers in include/glib-compat.h
> > +my @compatsyms = qw(
> > +g_get_monotonic_time
> > +
> > +g_assert_true
> > +g_assert_false
> > +g_assert_null
> > +g_assert_nonnull
> > +g_assert_cmpmem
> > +
> > +g_hash_table_add
> > +
> > +g_cond_clear
> > +g_cond_init
> > +g_cond_wait_until
> > +
> > +g_mutex_init
> > +g_mutex_clear
> > +
> > +g_thread_new
> > +
> > +g_private_replace
> > +G_PRIVATE_INIT
> > +
> > +G_TIME_SPAN_SECOND
> > +);
> > +
> > +
> > +# Functions defined inside QEMU which are using the
> > +# the same "g_" function name prefix as glib, so
> > +# get mis-detected as glib symbols
> > +my @blacklist = qw(
> > +g_to_float64
> > +g_assert_no_errno
> > +g_cclosure_new_swap
> > +g_free_rcu
> > +g_test_trap_subprocess
> > +g_poll_fixed
> > +g_list_insert_sorted_merged
> > +G_BYTE
> > +);
> > +
> > +# GObject stuff used by gtk frontend
> > +my @gobjectsums = qw(
> > +g_object_ref
> > +g_object_unref
> > +g_object_set_data
> > +g_signal_connect
> > +G_CALLBACK
> > +);
> > +
> > +# Functions defined by glib which are strangely
> > +# missing from their docs header index
> > +my @missingindex = qw(
> > +g_assertion_message
> > +g_assertion_message_expr
> > +g_assertion_message_cmpstr
> > +g_assertion_message_cmpnum
> > +g_assertion_message_error
> > +);
> > +
> 
> Can we "parse" #.* as comments, and put these in a glib-syms-extra.txt
> file?  Then we can hypothetically do the same tests in checkpatch.pl too.

Yep, that works.

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

Re: [Qemu-devel] [PATCH] scripts/checkpatch.pl: Don't allow special cases of unspaced operators

[Stefan Weil]

Am 18.12.2015 um 13:59 schrieb Peter Maydell:
> The checkpatch.pl script has a special case to permit the following
> operators to have no spaces around them:
>  <<  >>  &  ^  |  +  -  *  /  %
> 
> QEMU style prefers all operators to consistently have spacing around
> them, so remove this special case handling. This avoids reviewers
> having to manually note it during code review.
> 
> Signed-off-by: Peter Maydell 
> ---
> I actually thought failing to point out the lack of spaces was
> a checkpatch parsing bug until I looked in the source and found it
> was deliberate...
> 
> I say "QEMU style prefers", but possibly what I actually mean is
> "I prefer" ? Does anybody want to defend the unspaced versions?


I don't. Therefore

Reviewed-by: Stefan Weil 

Cc'ing QEMU Trivial, maybe it can be applied there.


> 
>  scripts/checkpatch.pl | 13 -
>  1 file changed, 13 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index b0f6e11..efca817 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1890,19 +1890,6 @@ sub process {
>   ERROR("space prohibited after 
> that '$op' $at\n" . $hereptr);
>   }
>  
> -
> - # << and >> may either have or not have spaces 
> both sides
> - } elsif ($op eq '<<' or $op eq '>>' or
> -  $op eq '&' or $op eq '^' or $op eq '|' 
> or
> -  $op eq '+' or $op eq '-' or
> -  $op eq '*' or $op eq '/' or
> -  $op eq '%')
> - {
> - if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) {
> - ERROR("need consistent spacing 
> around '$op' $at\n" .
> - $hereptr);
> - }
> -
>   # A colon needs no spaces before when it is
>   # terminating a case value or a label.
>   } elsif ($opv eq ':C' || $opv eq ':L') {
> 

Re: [Qemu-devel] [PATCH] scripts: provide a script for checking glib symbol usage

[Paolo Bonzini]

On 18/12/2015 14:05, Daniel P. Berrange wrote:
> > > +
> > > +cs-glib-syms:
> > > + @perl scripts/glib-syms.pl $(GLIB_SYMS_LIST) $(C_CODE_FILES)
> > 
> > 
> > Does this need to be included, or could it be a separate Makefile
> > invoked with e.g. make -f scripts/Makefile.style?
> 
> Any particular reason to favour that over include ? I did it this
> way because QEMU in general seems to be biased towards includes
> and not recursive make

That would not be recursive make, but rather a completely separate
Makefile to be manually invoked with -f.

Paolo

[Qemu-devel] Minutes from the "Stuttgart block Gipfele"

[Markus Armbruster]

Kevin, Max and I used an opportunity to meet and discuss block layer
matters.  We examined two topics in some depth: BlockBackend, and block
filters and dynamic reconfiguration.

Not nearly enough people to call it a block summit.  But the local
dialect is known for its use of diminutives, and "Gipfele" is the
diminutive of "summit" :)


= BlockBackend =

Background: BlockBackend (BB) was split off BlockDriverState (BDS) to
separate the block layer's external interface (BB) from its internal
building block (BDS).  Block layer clients such as device models and the
NBD server attach to a BB by BB name.  A BB has zero or one BDS (zero
means no medium).

Multiple device models using the same BB is dangerous, so we allow
attaching only one.  We don't currently enforce an "only one"
restriction for other clients.  This is problematic, because

* Different clients may want to configure the BB in conflicting ways,
  e.g. writeback caching mode (still to be moved from the BDS's
  enable_write_cache to the BB).

* When the BDS graph gets dynamically reconfigured, say when a block
  filter gets spliced in, clients that started out in the same spot may
  need to move differently.

Instead, each client should connect to its own BB.

This leads to the next question: how should this BB be created?

Initially, what is now the BB was mashed into the BDS.  In a way, the BB
got created along with the BDS.

The current code lets you create a BB along with a BDS when you need
one, or create a new BB for an existing BDS.  The BB has a name, and the
BDS may have a node-name.

The obvious low-level building blocks would be "create BB", "connect BB
to a BDS" (we have that as x-blockdev-insert-medium), "disconnect BB
from a BDS" (x-blockdev-remove-medium) and "destroy BB"
(x-blockdev-del).

Management applications probably don't mind having to work at this low
level, but for human users, it's cumbersome.  Perhaps the BB should be
created along with the client, at least optionally.

Means to create BBs separately are mostly useful when the BB needs to be
configured by the user: instead of duplicating the BB configuration
within each client, we keep it neatly separate.  We're not aware of
user-configurable knobs, though.

Currently, a client is configured to attach to a BB by specifying a BB
name.  For instance, a device model has a "drive" property that names a
BB.  If we create the BB automatically, we need client configuration to
name a BDS instead, i.e. we need a node-name instead of a BB name.

Of course, we'll have to keep the legacy configuration working.  The
"drive" property will have to refer to a BDS, like it did before BBs
were invented.  We could:

* Move the BB name back into the BDS.

* Move the BB name into DriveInfo, where the other legacy stuff lives.
  DriveInfo needs to be changed to hang off BDS rather than BB.

Regardless, dynamic reconfiguration may have to move the name / the
DriveInfo to a different BDS.

Not entirely sure automatic creation of BB is worthwhile or not.

Next steps:

* Support multiple BBs sharing the same BDS.

* Restrict BB to only one client of any kind instead of special-casing
  device models.

* Block jobs should go through BB.

* Investigate automatic creation of BB.


= Block filters =

We already have a few block filters:

* blkdebug, blkverify, quorum

Encryption should become another one.

Moreover, we have a few things mashed into BDS that should be filters:

* throttle (only at a root, i.e. right below a BB), copy-on-read,
  notifier (for backup block job), detect-zero

Dynamic reconfiguration means altering the BDS graph while it's in use.
Existing mutators:

* snaphot, mirror-complete, commit-complete, x-blockdev-change.

Things become interesting when nodes get implicitly inserted into the
graph, e.g.:

* A backup job inserts its notifier filter

* We create an implicit throttle filter to implement legacy throttling
  configuration

And so forth.  Nothing of the sort exists just yet.

What should happen when the user asks for a mutation at a place where we
have implicit filter(s)?

First, let's examine how such a chain could look like.  If we read the
current code correctly, it behaves as if we had a chain

BB
 |
  throttle
 |
 detect-zero
 |
copy-on-read
 |
BDS

Except for the backup job, which behaves as if we had

   backup job
  /
  notifier
 |
 detect-zero
 |
BDS

We believe that the following cleaned up filter stack should work:

BB
 |
  throttle  \
 |   \
copy-on-read  ) fixed at creation time
 |   /
 detect-zero/
 |
 | backup job
 |/
  notifier  ) dynamically inserted by the job
 |
BDS

Clients (device model, NBD server) connect through a BB on top.

Snapshot cuts in between the BDS and its implicit filters, like this:

Re: [Qemu-devel] [RFC v6 01/14] exec.c: Add new exclusive bitmap to ram_list

[Alex Bennée]

Alvise Rigo  writes:

> The purpose of this new bitmap is to flag the memory pages that are in
> the middle of LL/SC operations (after a LL, before a SC) on a per-vCPU
> basis.
> For all these pages, the corresponding TLB entries will be generated
> in such a way to force the slow-path if at least one vCPU has the bit
> not set.
> When the system starts, the whole memory is dirty (all the bitmap is
> set). A page, after being marked as exclusively-clean, will be
> restored as dirty after the SC.
>
> For each page we keep 8 bits to be shared among all the vCPUs available
> in the system. In general, the to the vCPU n correspond the bit n % 8.
>
> Suggested-by: Jani Kokkonen 
> Suggested-by: Claudio Fontana 
> Signed-off-by: Alvise Rigo 
> ---
>  exec.c  |  8 --
>  include/exec/memory.h   |  3 +-
>  include/exec/ram_addr.h | 76 
> +
>  3 files changed, 84 insertions(+), 3 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 0bf0a6e..e66d232 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1548,11 +1548,15 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, 
> Error **errp)
>  int i;
>
>  /* ram_list.dirty_memory[] is protected by the iothread lock.  */
> -for (i = 0; i < DIRTY_MEMORY_NUM; i++) {
> +for (i = 0; i < DIRTY_MEMORY_EXCLUSIVE; i++) {
>  ram_list.dirty_memory[i] =
>  bitmap_zero_extend(ram_list.dirty_memory[i],
> old_ram_size, new_ram_size);
> -   }
> +}
> +ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE] = bitmap_zero_extend(
> +ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE],
> +old_ram_size * EXCL_BITMAP_CELL_SZ,
> +new_ram_size * EXCL_BITMAP_CELL_SZ);
>  }

I'm wondering is old/new_ram_size should be renamed to
old/new_ram_pages?

So as I understand it we have created a bitmap which has
EXCL_BITMAP_CELL_SZ bits per page.

>  cpu_physical_memory_set_dirty_range(new_block->offset,
>  new_block->used_length,
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 0f07159..2782c77 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -19,7 +19,8 @@
>  #define DIRTY_MEMORY_VGA   0
>  #define DIRTY_MEMORY_CODE  1
>  #define DIRTY_MEMORY_MIGRATION 2
> -#define DIRTY_MEMORY_NUM   3/* num of dirty bits */
> +#define DIRTY_MEMORY_EXCLUSIVE 3
> +#define DIRTY_MEMORY_NUM   4/* num of dirty bits */
>
>  #include 
>  #include 
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 7115154..b48af27 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -21,6 +21,7 @@
>
>  #ifndef CONFIG_USER_ONLY
>  #include "hw/xen/xen.h"
> +#include "sysemu/sysemu.h"
>
>  struct RAMBlock {
>  struct rcu_head rcu;
> @@ -82,6 +83,13 @@ int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, 
> Error **errp);
>  #define DIRTY_CLIENTS_ALL ((1 << DIRTY_MEMORY_NUM) - 1)
>  #define DIRTY_CLIENTS_NOCODE  (DIRTY_CLIENTS_ALL & ~(1 << DIRTY_MEMORY_CODE))
>
> +/* Exclusive bitmap support. */
> +#define EXCL_BITMAP_CELL_SZ 8
> +#define EXCL_BITMAP_GET_BIT_OFFSET(addr) \
> +(EXCL_BITMAP_CELL_SZ * (addr >> TARGET_PAGE_BITS))
> +#define EXCL_BITMAP_GET_BYTE_OFFSET(addr) (addr >> TARGET_PAGE_BITS)
> +#define EXCL_IDX(cpu) (cpu % EXCL_BITMAP_CELL_SZ)
> +

I think some of the explanation of what CELL_SZ means from your commit
message needs to go here.

>  static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
>   ram_addr_t length,
>   unsigned client)
> @@ -173,6 +181,11 @@ static inline void 
> cpu_physical_memory_set_dirty_range(ram_addr_t start,
>  if (unlikely(mask & (1 << DIRTY_MEMORY_CODE))) {
>  bitmap_set_atomic(d[DIRTY_MEMORY_CODE], page, end - page);
>  }
> +if (unlikely(mask & (1 << DIRTY_MEMORY_EXCLUSIVE))) {
> +bitmap_set_atomic(d[DIRTY_MEMORY_EXCLUSIVE],
> +page * EXCL_BITMAP_CELL_SZ,
> +(end - page) * EXCL_BITMAP_CELL_SZ);
> +}
>  xen_modified_memory(start, length);
>  }
>
> @@ -288,5 +301,68 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned 
> long *dest,
>  }
>
>  void migration_bitmap_extend(ram_addr_t old, ram_addr_t new);
> +
> +/* One cell for each page. The n-th bit of a cell describes all the i-th 
> vCPUs
> + * such that (i % EXCL_BITMAP_CELL_SZ) == n.
> + * A bit set to zero ensures that all the vCPUs described by the bit have the
> + * EXCL_BIT set for the page. */
> +static inline void cpu_physical_memory_unset_excl(ram_addr_t addr, uint32_t 
> cpu)
> +{
> +set_bit_atomic(EXCL_BITMAP_GET_BIT_OFFSET(addr) + 

Re: [Qemu-devel] [PATCH] misc: spelling

[Luiz Capitulino]

On Fri, 18 Dec 2015 13:58:45 +0100
Markus Armbruster  wrote:

> Copying maintainer just for completeness.  I'm sure Luiz won't at all
> mind this going through trivial ;)
> 
> Let's clarify the commit message:
> 
> monitor: Spelling fix
> 
> With that:
> 
> Reviewed-by: Markus Armbruster 

Reviewed-by: Luiz Capitulino 

> 
> marcandre.lur...@redhat.com writes:
> 
> > From: Marc-André Lureau 
> >
> > ---
> >  monitor.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index e7e7ae2..51ec4c3 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -311,7 +311,7 @@ static void monitor_flush_locked(Monitor *mon)
> >  return;
> >  }
> >  if (rc > 0) {
> > -/* partinal write */
> > +/* partial write */
> >  QString *tmp = qstring_from_str(buf + rc);
> >  QDECREF(mon->outbuf);
> >  mon->outbuf = tmp;
> 

Re: [Qemu-devel] [PATCH] scripts: provide a script for checking glib symbol usage

[Daniel P. Berrange]

On Fri, Dec 18, 2015 at 01:35:13PM +, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonz...@redhat.com) wrote:
> > 
> > 
> > On 18/12/2015 14:05, Daniel P. Berrange wrote:
> > > > > +
> > > > > +cs-glib-syms:
> > > > > + @perl scripts/glib-syms.pl $(GLIB_SYMS_LIST) $(C_CODE_FILES)
> > > > 
> > > > 
> > > > Does this need to be included, or could it be a separate Makefile
> > > > invoked with e.g. make -f scripts/Makefile.style?
> > > 
> > > Any particular reason to favour that over include ? I did it this
> > > way because QEMU in general seems to be biased towards includes
> > > and not recursive make
> > 
> > That would not be recursive make, but rather a completely separate
> > Makefile to be manually invoked with -f.
> 
> Hmm but wouldn't this Makefile also be a good place for small-fast
> style check scripts that could be included in  make check  ?

Nothing about "make check" is fast, so we could probably just wire it
all into make check by default, as qtest & the block tests take some
considerable time to run. IOW people who want speed will be running
"make check-unit" or individual tests already.

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

Re: [Qemu-devel] [RFC v6 01/14] exec.c: Add new exclusive bitmap to ram_list

[alvise rigo]

On Fri, Dec 18, 2015 at 2:18 PM, Alex Bennée  wrote:
>
> Alvise Rigo  writes:
>
>> The purpose of this new bitmap is to flag the memory pages that are in
>> the middle of LL/SC operations (after a LL, before a SC) on a per-vCPU
>> basis.
>> For all these pages, the corresponding TLB entries will be generated
>> in such a way to force the slow-path if at least one vCPU has the bit
>> not set.
>> When the system starts, the whole memory is dirty (all the bitmap is
>> set). A page, after being marked as exclusively-clean, will be
>> restored as dirty after the SC.
>>
>> For each page we keep 8 bits to be shared among all the vCPUs available
>> in the system. In general, the to the vCPU n correspond the bit n % 8.
>>
>> Suggested-by: Jani Kokkonen 
>> Suggested-by: Claudio Fontana 
>> Signed-off-by: Alvise Rigo 
>> ---
>>  exec.c  |  8 --
>>  include/exec/memory.h   |  3 +-
>>  include/exec/ram_addr.h | 76 
>> +
>>  3 files changed, 84 insertions(+), 3 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 0bf0a6e..e66d232 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1548,11 +1548,15 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, 
>> Error **errp)
>>  int i;
>>
>>  /* ram_list.dirty_memory[] is protected by the iothread lock.  */
>> -for (i = 0; i < DIRTY_MEMORY_NUM; i++) {
>> +for (i = 0; i < DIRTY_MEMORY_EXCLUSIVE; i++) {
>>  ram_list.dirty_memory[i] =
>>  bitmap_zero_extend(ram_list.dirty_memory[i],
>> old_ram_size, new_ram_size);
>> -   }
>> +}
>> +ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE] = bitmap_zero_extend(
>> +ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE],
>> +old_ram_size * EXCL_BITMAP_CELL_SZ,
>> +new_ram_size * EXCL_BITMAP_CELL_SZ);
>>  }
>
> I'm wondering is old/new_ram_size should be renamed to
> old/new_ram_pages?

Yes, I think it make more sense.

>
> So as I understand it we have created a bitmap which has
> EXCL_BITMAP_CELL_SZ bits per page.
>
>>  cpu_physical_memory_set_dirty_range(new_block->offset,
>>  new_block->used_length,
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index 0f07159..2782c77 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -19,7 +19,8 @@
>>  #define DIRTY_MEMORY_VGA   0
>>  #define DIRTY_MEMORY_CODE  1
>>  #define DIRTY_MEMORY_MIGRATION 2
>> -#define DIRTY_MEMORY_NUM   3/* num of dirty bits */
>> +#define DIRTY_MEMORY_EXCLUSIVE 3
>> +#define DIRTY_MEMORY_NUM   4/* num of dirty bits */
>>
>>  #include 
>>  #include 
>> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
>> index 7115154..b48af27 100644
>> --- a/include/exec/ram_addr.h
>> +++ b/include/exec/ram_addr.h
>> @@ -21,6 +21,7 @@
>>
>>  #ifndef CONFIG_USER_ONLY
>>  #include "hw/xen/xen.h"
>> +#include "sysemu/sysemu.h"
>>
>>  struct RAMBlock {
>>  struct rcu_head rcu;
>> @@ -82,6 +83,13 @@ int qemu_ram_resize(ram_addr_t base, ram_addr_t newsize, 
>> Error **errp);
>>  #define DIRTY_CLIENTS_ALL ((1 << DIRTY_MEMORY_NUM) - 1)
>>  #define DIRTY_CLIENTS_NOCODE  (DIRTY_CLIENTS_ALL & ~(1 << 
>> DIRTY_MEMORY_CODE))
>>
>> +/* Exclusive bitmap support. */
>> +#define EXCL_BITMAP_CELL_SZ 8
>> +#define EXCL_BITMAP_GET_BIT_OFFSET(addr) \
>> +(EXCL_BITMAP_CELL_SZ * (addr >> TARGET_PAGE_BITS))
>> +#define EXCL_BITMAP_GET_BYTE_OFFSET(addr) (addr >> TARGET_PAGE_BITS)
>> +#define EXCL_IDX(cpu) (cpu % EXCL_BITMAP_CELL_SZ)
>> +
>
> I think some of the explanation of what CELL_SZ means from your commit
> message needs to go here.

OK.

>
>>  static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
>>   ram_addr_t length,
>>   unsigned client)
>> @@ -173,6 +181,11 @@ static inline void 
>> cpu_physical_memory_set_dirty_range(ram_addr_t start,
>>  if (unlikely(mask & (1 << DIRTY_MEMORY_CODE))) {
>>  bitmap_set_atomic(d[DIRTY_MEMORY_CODE], page, end - page);
>>  }
>> +if (unlikely(mask & (1 << DIRTY_MEMORY_EXCLUSIVE))) {
>> +bitmap_set_atomic(d[DIRTY_MEMORY_EXCLUSIVE],
>> +page * EXCL_BITMAP_CELL_SZ,
>> +(end - page) * EXCL_BITMAP_CELL_SZ);
>> +}
>>  xen_modified_memory(start, length);
>>  }
>>
>> @@ -288,5 +301,68 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned 
>> long *dest,
>>  }
>>
>>  void migration_bitmap_extend(ram_addr_t old, ram_addr_t new);
>> +
>> +/* One cell for each page. The n-th bit of a cell describes all the i-th 
>> vCPUs
>> + * such that (i % EXCL_BITMAP_CELL_SZ) == n.
>> + * A bit set to 

Re: [Qemu-devel] RFC: Operation Blockers in QEMU Block Nodes

[Kevin Wolf]

Am 16.12.2015 um 07:25 hat Jeff Cody geschrieben:
> Background:
> 
> Block jobs, and other QAPI operations, may modify and impact the
> BlockDriverState graph in QEMU.  In order to support multiple
> operations safely, we need a mechanism to block and gate operations,
> 
> We currently have op blockers, that are attached to each BDS.
> However, in practice we check this on the device level, rather than on
> the granularity of the individual BDS.  Also, due to limitations of
> the current system, we only allow a single block job at a time.

As I already said on IRC, I think a really important part in order to
make proper op blockers workable is that we actually enforce that you
have to request permission before you can operate on an image. That is,
if you were to write to an image, but you haven't requested write
permission from the op blocker system before, you'd get an assertion
failure.

The reason for this is that it's a long standing issue of the current
system that it's not only possible, but even easy to forget adding the
blocker code or updating it when the surrounding code changes. The
result is a mechanism that is too restrictive for many cases on the one
hand, but fails to actually provide the protection we need on the other
hand (for example, bs->file is still unprotected).

Blockers are subtle and at the same time pervasive enough that I'm
almost sure we wouldn't implement them correctly if we don't force
ourselves to do it right by letting qemu crash whenever we don't.

> Proposed New Design Features:
> --
> This design would supersede the current op blocker system.
> 
> Rather than have the op blockers attached directly to each BDS, Block
> Job access will be done through a separate Node Access Control system.
> 
> This new system will:
> 
> * Allow / Disallow operations to BDSs, (generally initiated by QAPI
>   actions; i.e. BDS node operations other than guest read/writes)
> 
> * Not be limited to "block jobs" (e.g. block-commit, block-stream,
>   etc..) - will also apply to other operations, such as
>   blockdev-add, change-backing-file, etc.
> 
> * Allow granularity in options, and provide the safety needed to
>   support multiple block jobs and operations.
> 
> * Reference each BDS by its node-name
> 
> * Be independent of the bdrv_states graph (i.e. does not reside in
>   the BDS structure)

I agree that the BDS structure is probably not the right place (except
possibly for caching the permission bitmasks), but I also wouldn't say
that it should be completely independent structures. Otherwise keeping
things in sync when the graph changes might turn out hard.

At the moment I can see two fundamentally different kinds of operations
that are interesting for op blockers:

1. I/O requests

   We need a place to check I/O requests that knows both the user (and
   the permissions it has acquired) and the node that is accessed.

   Any I/O request from outside the block layer (which includes guest
   devices, NBD servers and block jobs) should come through a
   BlockBackend. Even though we don't have it yet, we want to have a
   single BB per user. These requests could be dealt with by calling
   into the NAC whenever a BDS is attach to or detached from a BB;
   assertions can be checked in the appropriate blk_* functions.

   But there are also references between BDSes in the middle of the
   graph, like a qcow2 image referencing (and sending I/O requests to)
   its bs->file and bs->backing. It's important that we represent the
   requirements of such nodes in the NAC. As permissions are always for
   a specific user for a specific node, neither the parent node nor the
   child node are the correct place to manage the permissions. It seems
   that instead BdrvChild might be where they belong. The NAC needs to
   be informed whenever a child node is attached or detached from a BDS.

   As it happens, I've already been considering for a while to convert
   BlockBackend to using BdrvChild as well, so we could possibly unify
   both cases.

   The problem we have with BdrvChild is that I/O requests don't go
   through BdrvChild functions, so there is no place to add the
   assertions we need. We could possibly still add them to blk_*, but
   that would leave internal I/O requests unchecked.

2. Graph reconfiguration

   The problematic part here is that it's generally done by users who
   don't have a direct reference to all the nodes that are affected from
   the changes to the graph.

   For example, the commit job wants to drop all intermediate nodes
   after completing the copy - but even after properly converting it to
   BlockBackend, it would still only have a BB for the top and the base
   node, but not to any of the intermediate nodes that it drops.

   We don't seem to have convenient places like attaching/detaching from
   nodes where we could put the permission requests. Also we don't seem
   to have a 

[Qemu-devel] [PATCH] gtk: fix utf8 strings in the ui

[Gerd Hoffmann]

Commit "2cb5d2a gtk: use setlocale() for LC_MESSAGES only" restricts
locate settings to LC_MESSAGES, to avoid bugs caused by locale-specific
number printing (LC_NUMERIC) and possibly others.

We need LC_CTYPE too to make messages with chars outside us-ascii work
correctly.  Add it.

Reported-by: Kevin Wolf 
Tested-by: Kevin Wolf 
Signed-off-by: Gerd Hoffmann 
---
 ui/gtk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 47b37e1..30407a5 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2044,8 +2044,9 @@ void gtk_display_init(DisplayState *ds, bool full_screen, 
bool grab_on_hover)
 
 s->free_scale = FALSE;
 
-/* LC_MESSAGES only. See early_gtk_display_init() for details */
+/* LC_MESSAGES+LC_CTYPE only. See early_gtk_display_init() for details */
 setlocale(LC_MESSAGES, "");
+setlocale(LC_CTYPE, "");
 bindtextdomain("qemu", CONFIG_QEMU_LOCALEDIR);
 textdomain("qemu");
 
-- 
1.8.3.1

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

[Denis V. Lunev]

On 12/18/2015 06:19 PM, Pavel Fedin wrote:

  Hello!

  I realize that it's perhaps too late, because patches are already on 
Linux-next, but i have one concern... May be it's not too
late...

  I dislike implementing architecture-dependent exit code where we could 
implement an architecture-independent one.

  As far as i understand this code, KVM_EXIT_HYPERV is called when one of three 
MSRs are accessed. But, shouldn't we have implemented
instead something more generic, like KVM_EXIT_REG_IO, which would work similar 
to KVM_EXIT_PIO or KVM_EXIT_MMIO, but carry register
code and value?

  This would allow us to solve the same task which we have done here, but this 
solution would be reusable for other devices and other
archirectures. What if in future we have more system registers to emulate in 
userspace?

  I write this because at one point i suggested similar thing for ARM64 (but i 
never actually wrote it), to emulate physical CP15
timer. And it would require exactly the same capability - process some trapped 
system register accesses in userspace.

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



we have discussed this AFAIR. HyperV guest implementation
is available in Linux kernel and thus technically we can have
this stuff on any platform.

Den

[Qemu-devel] [PULL 37/48] block: Remove prototype of bdrv_swap from header

[Kevin Wolf]

From: Fam Zheng 

The function has gone.

Signed-off-by: Fam Zheng 
Signed-off-by: Kevin Wolf 
---
 include/block/block.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/block/block.h b/include/block/block.h
index bce0d86..db8e096 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -198,7 +198,6 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, 
Error **errp);
 BlockDriverState *bdrv_new_root(void);
 BlockDriverState *bdrv_new(void);
 void bdrv_make_anon(BlockDriverState *bs);
-void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
 void bdrv_replace_in_backing_chain(BlockDriverState *old,
BlockDriverState *new);
-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH v0 2/9] cpu: Store CPU typename in MachineState

[Igor Mammedov]

On Fri, 18 Dec 2015 13:51:49 -0200
Eduardo Habkost  wrote:

> On Fri, Dec 18, 2015 at 11:46:05AM +0100, Igor Mammedov wrote:
> > On Thu, 17 Dec 2015 16:09:23 -0200
> > Eduardo Habkost  wrote:
> > 
> > > On Wed, Dec 16, 2015 at 11:26:20PM +0100, Igor Mammedov wrote:
> > > > On Wed, 16 Dec 2015 17:39:02 -0200
> > > > Eduardo Habkost  wrote:
> > > > 
> > > > > On Wed, Dec 16, 2015 at 05:54:25PM +0100, Igor Mammedov wrote:
> > > > > > On Tue, 15 Dec 2015 14:08:09 +0530
> > > > > > Bharata B Rao  wrote:
> > > > > > 
> > > > > > > On Mon, Dec 14, 2015 at 03:29:49PM -0200, Eduardo Habkost wrote:
> > > > > > > > On Thu, Dec 10, 2015 at 11:45:37AM +0530, Bharata B Rao wrote:
> > > > > > > > > Storing CPU typename in MachineState lets us to create CPU
> > > > > > > > > threads for all architectures in uniform manner from
> > > > > > > > > arch-neutral code.
> > > > > > > > > 
> > > > > > > > > TODO: Touching only i386 and spapr targets for now
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Bharata B Rao 
> > > > > > > > 
> > > > > > > > Suggestions:
> > > > > > > > 
> > > > > > > > * Name the field "cpu_base_type" to indicate it is the base CPU
> > > > > > > >   class name, not the actual CPU class name used when creating
> > > > > > > >   CPUs.
> > > > > > > > * Put it in MachineClass, as it may be useful for code that
> > > > > > > >   runs before machine->init(), in the future.
> > > > > > > 
> > > > > > > Ok.
> > > > > > > 
> > > > > > > > * Maybe make it a CPUClass* field instead of a string?
> > > > > > > 
> > > > > > > In the current use case, this base cpu type string is being passed
> > > > > > > to cpu_generic_init(const char *typename, const char *cpu_model)
> > > > > > > to create boot time CPUs with given typename and cpu_mode. So for
> > > > > > > now the string makes sense for use case.
> > > > > > > 
> > > > > > > Making it CPUClass* would necessiate more changes to
> > > > > > > cpu_generic_init().
> > > > > > how about actually leaving it as "cpu_type" and putting in it
> > > > > > actual cpu type that could be used with device_add().
> > > > > > 
> > > > > > that would get rid of keeping and passing around intermediate
> > > > > > cpu_model.
> > > > > 
> > > > > Makes sense. We only need to save both typename and cpu_model
> > > > > today because cpu_generic_init() currently encapsulates three
> > > > > steps: CPU class lookup + CPU creation + CPU feature parsing. But
> > > > > we shouldn't need to redo CPU class lookup every time.
> > > > BTW: Eduardo do you know if QEMU could somehow provide a list of
> > > > supported CPU types (i.e. not cpumodels) to libvirt?
> > > 
> > > Not sure I understand the question. Could you clarify what you
> > > mean by "supported CPU types", and what's the problem it would
> > > solve?
> > device_add TYPE, takes only type name so I'd like to kep it that way
> > and make sure that libvirt/user can list cpu types that hi would
> > be able to use with device_add/-device.
> > 
> > for PC they are generated from cpu_model with help of x86_cpu_type_name()
> 
> What about adding a "qom-type" field to query-cpu-definitions?
Sounds like good idea to me.

> 
> > 
> > > > 
> > > > > 
> > > > > We could just split cpu_model once, and save the resulting
> > > > > CPUClass* + featurestr, instead of saving the full cpu_model
> > > > > string and parsing it again every time.
> > > > isn't featurestr as x86/sparc specific?
> > > > 
> > > > Could we have field in  x86_cpu_class/sparc_cpu_class for it and set it
> > > > when cpu_model is parsed?
> > > > That way generic cpu_model parser would handle only cpu names and
> > > > target specific overrides would handle both.
> > > 
> > > I always assumed we want to have a generic CPU model + featurestr
> > > mechanism that could be reused by multiple architectures.
> > 
> > I've thought the opposite way, that we wanted to faze out featurestr
> > in favor of generic option parsing of generic device, i.e.
> >  -device TYPE,option=X,...
> > but we would have to keep compatibility with old CLI
> > that supplies cpu definition via -cpu cpu_model,featurestr
> > so cpu_model translated into "cpu_type" field make sense for every
> > target but featurestr is x86/sparc specific and I'd prefer to
> > keep it that way and do not introduce it to other targets.
> 
> I see, and it may make sense long term. But do you really think
> we will be able to deprecate "-cpu" and "-smp" soon?
> 
> We already have CPUClass::parse_features, and cpu_generic_init()
> already makes the model/featurestr split. Do you propose we
> remove that generic code and move it back to x86/sparc?
> 
> Also, are you sure no other architectures support options in
> "-cpu"? cpu_common_parse_features() already supports setting QOM
> properties, and it is already available on every architecture
> that calls CPUClass::parse_features or uses cpu_generic_init(). I

Re: [Qemu-devel] [PATCH v3 11/24] error: Use error_reportf_err() where it makes obvious sense

[Eric Blake]

On 12/18/2015 08:35 AM, Markus Armbruster wrote:
> Done with this Coccinelle semantic patch
> 
> @@
> expression FMT, E, S;
> expression list ARGS;
> @@
> -error_report(FMT, ARGS, error_get_pretty(E));
> +error_reportf_err(E, FMT/*@@@*/, ARGS);
> (
> -error_free(E);
> |
>exit(S);
> |
>abort();
> )
> 
> followed by a replace of '%s"/*@@@*/' by '"' and some line rewrapping,
> because I can't figure out how to make Coccinelle transform strings.
> 
> We now use the error whole instead of just its message obtained with
> error_get_pretty().  This avoids suppressing its hint (see commit
> 50b7b00), but I can't see how the errors touched in this commit could
> come with hints.
> 
> Signed-off-by: Markus Armbruster 
> ---

> +++ b/arch_init.c
> @@ -258,9 +258,7 @@ void do_acpitable_option(const QemuOpts *opts)
>  
>  acpi_table_add(opts, );
>  if (err) {
> -error_report("Wrong acpi table provided: %s",
> - error_get_pretty(err));
> -error_free(err);
> +error_reportf_err(err, "Wrong acpi table provided: ");

Bikeshedding: should error_reportf_err() automatically add the trailing
": " to the prefix, instead of having every caller express it?  Would
affect 10/24 as well.  But I can't see a strong reason to add the churn
it would cause for a respin, so I won't insist.

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] [PULL v4 0/9] Introduce I/O channels framework

[Peter Maydell]

On 18 December 2015 at 12:20, Daniel P. Berrange  wrote:
> The following changes since commit 6a6533213d78dea4407fe6933ad489796b582599:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2015-12-17 18:07:09 +)
>
> are available in the git repository at:
>
>   git://github.com/berrange/qemu tags/pull-io-channel-base-2015-12-18-1
>
> for you to fetch changes up to d98e4eb7de93290f7921b0dbe869c7dd3c567945:
>
>   io: add QIOChannelBuffer class (2015-12-18 12:18:31 +)
>
> 
> Merge I/O channels base classes
>
> 

Applied this version, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] scripts: provide a script for checking glib symbol usage

[Peter Maydell]

On 18 December 2015 at 13:41, Paolo Bonzini  wrote:
>
>
> On 18/12/2015 14:35, Dr. David Alan Gilbert wrote:
>> > That would not be recursive make, but rather a completely separate
>> > Makefile to be manually invoked with -f.
>>
>> Hmm but wouldn't this Makefile also be a good place for small-fast
>> style check scripts that could be included in  make check  ?
>
> Yes, but there is no benefit from inclusion, since these tests do not
> depend on anything having been built already.

In particular, if it doesn't require a preceding configure or
build I can just run it once as part of my merge testing rather
than farming it out to be run in parallel on five machines and
ten different configs :-)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/6] hw/vfio/platform: amd-xgbe device

[Peter Maydell]

On 17 December 2015 at 12:29, Eric Auger  wrote:
> This patch introduces the amd-xgbe VFIO platform device. It
> allows the guest to do passthrough on a device exposing an
> "amd,xgbe-seattle-v1a" compat string.
>
> Signed-off-by: Eric Auger 
> Reviewed-by: Alex Benné

You've typo'd Alex's name here ("Bennée"); would be good to
fix if you need to do another round.

thanks
-- PMM

[Qemu-devel] [PULL 46/48] block/qapi: explicitly warn if !has_full_backing_filename

[Kevin Wolf]

From: John Snow 

Disambiguate "Backing filename and full backing filename are equivalent"
from "full backing filename could not be determined."

Signed-off-by: John Snow 
Message-id: 1450122916-4706-4-git-send-email-js...@redhat.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 block/qapi.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 3d8e434..5587c64 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -677,9 +677,10 @@ void bdrv_image_info_dump(fprintf_function func_fprintf, 
void *f,
 
 if (info->has_backing_filename) {
 func_fprintf(f, "backing file: %s", info->backing_filename);
-if (info->has_full_backing_filename &&
-(strcmp(info->backing_filename,
-info->full_backing_filename) != 0)) {
+if (!info->has_full_backing_filename) {
+func_fprintf(f, " (cannot determine actual path)");
+} else if (strcmp(info->backing_filename,
+  info->full_backing_filename) != 0) {
 func_fprintf(f, " (actual path: %s)", info->full_backing_filename);
 }
 func_fprintf(f, "\n");
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] scripts: provide a script for checking glib symbol usage

[Paolo Bonzini]

On 18/12/2015 14:35, Dr. David Alan Gilbert wrote:
> > That would not be recursive make, but rather a completely separate
> > Makefile to be manually invoked with -f.
>
> Hmm but wouldn't this Makefile also be a good place for small-fast
> style check scripts that could be included in  make check  ?

Yes, but there is no benefit from inclusion, since these tests do not
depend on anything having been built already.

Paolo

Re: [Qemu-devel] [PATCH] gtk: use setlocale() for LC_MESSAGES only

[Kevin Wolf]

Am 18.12.2015 um 14:23 hat Gerd Hoffmann geschrieben:
> On Fr, 2015-12-18 at 12:38 +0100, Kevin Wolf wrote:
> > Am 10.09.2015 um 17:19 hat Alberto Garcia geschrieben:
> > > The QEMU code is not internationalized and assumes that it runs under
> > > the C locale, but if we use the GTK+ UI we'll end up importing the
> > > locale settings from the environment. This can break things, such as
> > > the JSON generator and iotest 120 in locales that use a decimal comma.
> > > 
> > > We do however have translations for a few simple strings for the GTK+
> > > menu items, so in order to run QEMU using the C locale, and yet have a
> > > translated UI let's use setlocale() for LC_MESSAGES only.
> > > 
> > > Signed-off-by: Alberto Garcia 
> > 
> > Not sure why I noticed it only now and if it's related to any recent
> > package upgrade on my side (using RHEL 7), but I noticed that non-ASCII
> > characters in the GTK UI strings are broken for me and git bisect
> > pointed to this commit.
> 
> I guess we need to set LC_CTYPE too.
> Can you try whenever the attached patch fixes the issue?

Yes, that works for me.

Tested-by: Kevin Wolf 

Re: [Qemu-devel] [PATCH v3 05/24] error: Improve documentation

[Eric Blake]

On 12/18/2015 08:35 AM, Markus Armbruster wrote:
> While there, tighten error_append_hint()'s assertion.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  include/qapi/error.h | 20 ++--
>  util/error.c |  2 +-
>  util/qemu-error.c|  8 
>  3 files changed, 23 insertions(+), 7 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